diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index bca7e13b10..a074008bd9 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -14405,6 +14405,96 @@ "summary", "outcomes" ] + }, + "ContributorNotificationFeed": { + "type": "object", + "properties": { + "login": { + "type": "string" + }, + "unreadCount": { + "type": "number" + }, + "summary": { + "type": "string" + }, + "notifications": { + "type": "array", + "items": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "eventType": { + "type": "string" + }, + "repoFullName": { + "type": "string" + }, + "pullNumber": { + "type": "number", + "nullable": true + }, + "title": { + "type": "string" + }, + "body": { + "type": "string" + }, + "deeplink": { + "type": "string" + }, + "status": { + "type": "string", + "enum": [ + "delivered", + "read" + ] + }, + "createdAt": { + "type": "string" + } + }, + "required": [ + "id", + "eventType", + "repoFullName", + "pullNumber", + "title", + "body", + "deeplink", + "status", + "createdAt" + ] + } + } + }, + "required": [ + "login", + "unreadCount", + "summary", + "notifications" + ] + }, + "ContributorNotificationsMarkRead": { + "type": "object", + "properties": { + "login": { + "type": "string" + }, + "marked": { + "type": "number" + }, + "summary": { + "type": "string" + } + }, + "required": [ + "login", + "marked", + "summary" + ] } }, "parameters": {}, @@ -18694,6 +18784,94 @@ } ] } + }, + "/v1/contributors/{login}/notifications": { + "get": { + "summary": "Contributor notification feed", + "parameters": [ + { + "schema": { + "type": "string" + }, + "required": true, + "name": "login", + "in": "path" + } + ], + "responses": { + "200": { + "description": "Self-scoped badge notification feed with unread count (mirrors loopover_list_notifications).", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ContributorNotificationFeed" + } + } + } + } + }, + "security": [ + { + "LoopOverBearer": [] + }, + { + "LoopOverSessionCookie": [] + } + ] + } + }, + "/v1/contributors/{login}/notifications/read": { + "post": { + "summary": "Mark contributor notifications read", + "parameters": [ + { + "schema": { + "type": "string" + }, + "required": true, + "name": "login", + "in": "path" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "ids": { + "type": "array", + "items": { + "type": "string" + }, + "maxItems": 100 + } + } + } + } + } + }, + "responses": { + "200": { + "description": "Count of notifications marked read (mirrors loopover_mark_notifications_read).", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ContributorNotificationsMarkRead" + } + } + } + } + }, + "security": [ + { + "LoopOverBearer": [] + }, + { + "LoopOverSessionCookie": [] + } + ] + } } }, "servers": [ diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 4a8d130bac..6ee6f0200d 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -90,6 +90,8 @@ const CLI_COMMAND_SPEC = { "contributor-profile": [], "monitor-open-prs": [], "pr-outcomes": [], + notifications: [], + "mark-notifications-read": [], "analyze-branch": [], preflight: [], "review-pr": [], @@ -1137,6 +1139,18 @@ const STDIO_TOOL_DESCRIPTORS = [ description: "Return a contributor's own post-merge outcome records — for each merged PR, a public-safe attribution of what it did for their standing on the repo. Self-scoped: only the authenticated login's outcomes.", }, + { + name: "loopover_list_notifications", + category: "utility", + description: + "Return a contributor's own LoopOver notifications (e.g. changes requested on their PRs) and unread badge count. Self-scoped: only the authenticated login's notifications.", + }, + { + name: "loopover_mark_notifications_read", + category: "utility", + description: + "Mark a contributor's own delivered notifications as read (clears the badge). Self-scoped; pass `ids` to clear specific notifications or omit to clear all.", + }, { name: "loopover_compare_pr_variants", category: "branch", @@ -2092,6 +2106,33 @@ registerStdioTool( }, ); +registerStdioTool( + "loopover_list_notifications", + { + description: stdioToolDescription("loopover_list_notifications"), + inputSchema: loginShape, + }, + async ({ login }) => { + const payload = await getNotifications(login); + return toolResult(notificationsToolSummary(login, payload), payload); + }, +); + +registerStdioTool( + "loopover_mark_notifications_read", + { + description: stdioToolDescription("loopover_mark_notifications_read"), + inputSchema: { + login: z.string().min(1), + ids: z.array(z.string().min(1).max(128)).max(100).optional(), + }, + }, + async ({ login, ids }) => { + const payload = await markNotificationsRead(login, ids); + return toolResult(markNotificationsReadToolSummary(login, payload), payload); + }, +); + registerStdioTool( "loopover_compare_pr_variants", { @@ -3399,6 +3440,8 @@ async function runCli(args) { if (command === "contributor-profile") return contributorProfileCli(options); if (command === "monitor-open-prs") return monitorOpenPrsCli(options); if (command === "pr-outcomes") return prOutcomesCli(options); + if (command === "notifications") return notificationsCli(options); + if (command === "mark-notifications-read") return markNotificationsReadCli(options); if (command === "review-pr") return reviewPrCli(options); if (command !== "analyze-branch" && command !== "preflight") { const suggestion = suggestCommand(command); @@ -3900,6 +3943,63 @@ async function prOutcomesCli(options) { } } +function printNotificationsHelp() { + process.stdout.write( + [ + "Usage: loopover-mcp notifications --login [--json]", + "", + "List your LoopOver notification feed and unread badge count.", + "Mirrors the loopover_list_notifications MCP tool and GET /v1/contributors/{login}/notifications. No source upload.", + "", + "Pass --json for machine-readable output.", + ].join("\n") + "\n", + ); +} + +async function notificationsCli(options) { + if (options.help === true) return printNotificationsHelp(); + const login = options.login ?? process.env.LOOPOVER_LOGIN ?? process.env.GITHUB_LOGIN; + if (!login) throw new Error("Pass --login or set LOOPOVER_LOGIN."); + const payload = await getNotifications(login); + if (options.json) { + process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); + return; + } + process.stdout.write(`${sanitizePlainTextTerminalOutput(notificationsToolSummary(login, payload))}\n`); + for (const item of payload?.notifications ?? []) { + const heading = `${item.status} ${item.repoFullName}${item.pullNumber != null ? `#${item.pullNumber}` : ""} — ${item.title}`; + process.stdout.write(`${sanitizePlainTextTerminalOutput(heading)}\n`); + } +} + +function printMarkNotificationsReadHelp() { + process.stdout.write( + [ + "Usage: loopover-mcp mark-notifications-read --login [--id ]... [--json]", + "", + "Mark your delivered LoopOver notifications as read (clears the badge).", + "Mirrors the loopover_mark_notifications_read MCP tool and POST /v1/contributors/{login}/notifications/read.", + "Omit --id to mark all; pass --id repeatedly to mark specific deliveries.", + "", + "Pass --json for machine-readable output.", + ].join("\n") + "\n", + ); +} + +async function markNotificationsReadCli(options) { + if (options.help === true) return printMarkNotificationsReadHelp(); + const login = options.login ?? process.env.LOOPOVER_LOGIN ?? process.env.GITHUB_LOGIN; + if (!login) throw new Error("Pass --login or set LOOPOVER_LOGIN."); + const rawIds = options.id; + const ids = rawIds === undefined || rawIds === true ? undefined : Array.isArray(rawIds) ? rawIds : [rawIds]; + const payload = await markNotificationsRead(login, ids); + if (options.json) { + process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); + return; + } + process.stdout.write(`${sanitizePlainTextTerminalOutput(markNotificationsReadToolSummary(login, payload))}\n`); +} + function printRepoDecisionHelp() { process.stdout.write( [ @@ -4380,6 +4480,8 @@ function printHelp() { loopover-mcp repo-decision --login --repo owner/repo [--json] loopover-mcp monitor-open-prs --login [--json] loopover-mcp pr-outcomes --login [--limit N] [--json] + loopover-mcp notifications --login [--json] + loopover-mcp mark-notifications-read --login [--id ]... [--json] loopover-mcp analyze-branch --login [--repo owner/repo] [--base origin/main] [--branch-eligibility eligible|ineligible|unknown] [--pending-merged-prs 3] [--expected-open-prs 0] [--projected-credibility 0.8] [--scenario-note "..."] [--validation "passed|npm test|summary"] [--format table] [--json] loopover-mcp preflight --login [--repo owner/repo] [--base origin/main] [--branch-eligibility eligible|ineligible|unknown] [--pending-merged-prs 3] [--expected-open-prs 0] [--projected-credibility 0.8] [--validation "passed|npm test|summary"] [--format table] [--json] loopover-mcp review-pr --login [--repo owner/repo] [--base origin/main] [--commit ]... [--body ] [--body-file ] [--linked-issue ] [--json] @@ -4398,7 +4500,7 @@ function printHelp() { LOOPOVER_PROFILE LOOPOVER_CONFIG_PATH or LOOPOVER_CONFIG_DIR LOOPOVER_API_TOKEN, LOOPOVER_MCP_TOKEN, LOOPOVER_TOKEN, or a session from loopover-mcp login - LOOPOVER_LOGIN or GITHUB_LOGIN (default --login for analyze-branch, preflight, review-pr, decision-pack, repo-decision, monitor-open-prs, pr-outcomes, and agent plan/packet) + LOOPOVER_LOGIN or GITHUB_LOGIN (default --login for analyze-branch, preflight, review-pr, decision-pack, repo-decision, monitor-open-prs, pr-outcomes, notifications, mark-notifications-read, and agent plan/packet) GITHUB_TOKEN for non-interactive login bootstrap GITTENSOR_SCORE_PREVIEW_CMD GITTENSOR_ROOT @@ -4443,7 +4545,7 @@ Use --profile or LOOPOVER_PROFILE to run login, logout, whoami, status, d function parseOptions(args) { const options = {}; - const repeatable = new Set(["label", "issue", "commit", "changedFile", "test", "testFile", "validation", "validationCommand", "validationStatus", "validationSummary", "validationDuration", "scenarioNote"]); + const repeatable = new Set(["label", "issue", "commit", "changedFile", "test", "testFile", "validation", "validationCommand", "validationStatus", "validationSummary", "validationDuration", "scenarioNote", "id"]); for (let index = 0; index < args.length; index += 1) { const arg = args[index]; if (arg === "--json") { @@ -5533,6 +5635,14 @@ function getPrOutcomes(login, limit) { return apiGet(`/v1/contributors/${encodeURIComponent(login)}/pr-outcomes${suffix}`); } +function getNotifications(login) { + return apiGet(`/v1/contributors/${encodeURIComponent(login)}/notifications`); +} + +function markNotificationsRead(login, ids) { + return apiPost(`/v1/contributors/${encodeURIComponent(login)}/notifications/read`, ids ? { ids } : {}); +} + // Mirror the API's own `summary` when it sends one, so the CLI and the loopover_monitor_open_prs MCP // tool (which returns monitor.summary verbatim) never drift into two different sentences for one payload. function openPrMonitorToolSummary(login, payload) { @@ -5547,6 +5657,18 @@ function prOutcomesToolSummary(login, payload) { return `LoopOver post-merge outcomes for ${login}.`; } +function notificationsToolSummary(login, payload) { + const summary = typeof payload?.summary === "string" ? payload.summary.trim() : ""; + if (summary) return summary; + return `LoopOver notifications for ${login}.`; +} + +function markNotificationsReadToolSummary(login, payload) { + const summary = typeof payload?.summary === "string" ? payload.summary.trim() : ""; + if (summary) return summary; + return `Marked LoopOver notification(s) read for ${login}.`; +} + function isCacheableDecisionPack(payload, login) { return payload?.status === "ready" && typeof payload.login === "string" && payload.login.toLowerCase() === login.toLowerCase(); } diff --git a/src/api/routes.ts b/src/api/routes.ts index 121428092c..316cb3be4c 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -107,6 +107,8 @@ import { upsertContributorEvidence, upsertContributorScoringProfile, upsertRepositorySettings, + MAX_NOTIFICATION_DELIVERY_ID_LENGTH, + MAX_NOTIFICATION_MARK_READ_IDS, getRepositoryAiKeyStatus, upsertRepositoryAiKey, deleteRepositoryAiKey, @@ -271,6 +273,7 @@ import { import { attachDataQuality, buildCoreSignalFidelity, buildFreshnessSloReport, buildRepoDataQuality, buildSignalFidelity } from "../signals/data-quality"; import { buildContributorOpenPrMonitor } from "../signals/contributor-open-pr-monitor"; import { buildContributorPrOutcomes } from "../signals/contributor-pr-outcomes"; +import { loadContributorNotificationFeed, markContributorNotificationsRead } from "../notifications/service"; import { buildPullRequestReviewability, type PullRequestReviewability } from "../signals/reward-risk"; import { buildLocalBranchAnalysis, findCurrentBranchPullRequest } from "../signals/local-branch"; import { buildIssueSlopAssessment, ISSUE_SLOP_RUBRIC_MARKDOWN } from "../signals/issue-slop"; @@ -583,6 +586,14 @@ const slopRiskSchema = z.object({ issueDiscoveryLane: z.boolean().optional(), }); +// #6745: mirrors markNotificationsReadShape in src/mcp/server.ts (ids optional; omit = mark all). +const markNotificationsReadBodySchema = z.object({ + ids: z + .array(z.string().min(1).max(MAX_NOTIFICATION_DELIVERY_ID_LENGTH)) + .max(MAX_NOTIFICATION_MARK_READ_IDS) + .optional(), +}); + // #6748: mirrors checkImprovementPotentialShape in src/mcp/server.ts VERBATIM (same bounds, same optionality) // so the REST surface can never accept an input the MCP tool would reject, or vice versa. const improvementPotentialSchema = z.object({ @@ -3343,6 +3354,24 @@ export function createApp() { return c.json(await buildContributorPrOutcomes(c.env, login, limit)); }); + // #6745: REST mirrors of loopover_list_notifications / loopover_mark_notifications_read. + app.get("/v1/contributors/:login/notifications", async (c) => { + const login = c.req.param("login"); + const unauthorized = await requireContributorAccess(c, login); + if (unauthorized) return unauthorized; + return c.json(await loadContributorNotificationFeed(c.env, login)); + }); + + app.post("/v1/contributors/:login/notifications/read", async (c) => { + const login = c.req.param("login"); + const unauthorized = await requireContributorAccess(c, login); + if (unauthorized) return unauthorized; + const body = await c.req.json().catch(() => null); + const parsed = markNotificationsReadBodySchema.safeParse(body ?? {}); + if (!parsed.success) return c.json({ error: "invalid_mark_notifications_read", issues: parsed.error.issues }, 400); + return c.json(await markContributorNotificationsRead(c.env, login, parsed.data.ids)); + }); + app.get("/v1/contributors/:login/repos/:owner/:repo/decision", async (c) => { const login = c.req.param("login"); const unauthorized = await requireContributorAccess(c, login); diff --git a/src/mcp/server.ts b/src/mcp/server.ts index d77180447b..2404d669e9 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -59,7 +59,6 @@ import { listIssues, deleteIssueWatchSubscription, listIssueWatchSubscriptionsForLogin, - listNotificationDeliveriesForRecipient, upsertIssueWatchSubscription, upsertRepositorySettings, listOpenPullRequests, @@ -73,13 +72,12 @@ import { listRepositories, MAX_NOTIFICATION_DELIVERY_ID_LENGTH, MAX_NOTIFICATION_MARK_READ_IDS, - markNotificationDeliveriesRead, recordProductUsageEvent, } from "../db/repositories"; import { decidePendingAgentAction } from "../services/agent-approval-queue"; import { automationStateSummary, buildAutomationState } from "../services/automation-state"; import { nowIso } from "../utils/json"; -import { buildNotificationFeed } from "../notifications/service"; +import { loadContributorNotificationFeed, markContributorNotificationsRead } from "../notifications/service"; import { contributorRepoStatsFromGittensor, fetchGittensorContributorSnapshot } from "../gittensor/api"; import { getRepositoryCollaboratorPermission } from "../github/app"; import { performRepoDocRefresh } from "../github/repo-doc-refresh-runner"; @@ -3797,10 +3795,9 @@ export class LoopoverMcp { private async listNotifications(login: string): Promise { this.requireContributorAccess(login); - const deliveries = await listNotificationDeliveriesForRecipient(this.env, login, { channel: "badge", limit: 50 }); - const feed = buildNotificationFeed(login, deliveries); + const feed = await loadContributorNotificationFeed(this.env, login); return { - summary: `LoopOver notifications for ${login}: ${feed.unreadCount} unread.`, + summary: feed.summary, data: feed as unknown as Record, }; } @@ -3829,10 +3826,10 @@ export class LoopoverMcp { private async markNotificationsRead(login: string, ids?: string[]): Promise { this.requireContributorAccess(login); - const marked = await markNotificationDeliveriesRead(this.env, login, ids); + const payload = await markContributorNotificationsRead(this.env, login, ids); return { - summary: `Marked ${marked} LoopOver notification(s) read for ${login}.`, - data: { login: login.toLowerCase(), marked }, + summary: payload.summary, + data: payload as unknown as Record, }; } diff --git a/src/notifications/service.ts b/src/notifications/service.ts index 826e2de800..eeb79d0f71 100644 --- a/src/notifications/service.ts +++ b/src/notifications/service.ts @@ -5,7 +5,9 @@ import { getRepository, insertNotificationDeliveryIfAbsent, listIssueWatchersForRepo, + listNotificationDeliveriesForRecipient, listNotificationSubscriptionsForLogin, + markNotificationDeliveriesRead, markNotificationDeliveryDelivered, } from "../db/repositories"; import { isGrabbableHighMultiplierIssue } from "../signals/engine"; @@ -187,6 +189,38 @@ export function buildNotificationFeed(login: string, deliveries: NotificationDel return { login: login.toLowerCase(), unreadCount, notifications }; } +export type ContributorNotificationFeed = NotificationFeed & { summary: string }; + +export type ContributorNotificationsMarkRead = { + login: string; + marked: number; + summary: string; +}; + +/** #6745: shared payload for loopover_list_notifications + GET /v1/contributors/:login/notifications. */ +export async function loadContributorNotificationFeed(env: Env, login: string): Promise { + const deliveries = await listNotificationDeliveriesForRecipient(env, login, { channel: "badge", limit: 50 }); + const feed = buildNotificationFeed(login, deliveries); + return { + ...feed, + summary: `LoopOver notifications for ${login}: ${feed.unreadCount} unread.`, + }; +} + +/** #6745: shared payload for loopover_mark_notifications_read + POST .../notifications/read. */ +export async function markContributorNotificationsRead( + env: Env, + login: string, + ids?: string[], +): Promise { + const marked = await markNotificationDeliveriesRead(env, login, ids); + return { + login: login.toLowerCase(), + marked, + summary: `Marked ${marked} LoopOver notification(s) read for ${login}.`, + }; +} + // Badge delivery is pull-based: "delivering" just makes the row visible to the recipient's feed (status // pending -> delivered). Email/web-push (#570) would perform an outbound send here for their channel. export async function deliverNotification(env: Env, deliveryId: string): Promise { diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index 20234bf962..3be3fb0705 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -481,6 +481,35 @@ export const ContributorPrOutcomesSchema = z }) .openapi("ContributorPrOutcomes"); +export const ContributorNotificationFeedSchema = z + .object({ + login: z.string(), + unreadCount: z.number(), + summary: z.string(), + notifications: z.array( + z.object({ + id: z.string(), + eventType: z.string(), + repoFullName: z.string(), + pullNumber: z.number().nullable(), + title: z.string(), + body: z.string(), + deeplink: z.string(), + status: z.enum(["delivered", "read"]), + createdAt: z.string(), + }), + ), + }) + .openapi("ContributorNotificationFeed"); + +export const ContributorNotificationsMarkReadSchema = z + .object({ + login: z.string(), + marked: z.number(), + summary: z.string(), + }) + .openapi("ContributorNotificationsMarkRead"); + export const ContributorOpportunitySchema = z .object({ repoFullName: z.string(), diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 6dca754355..4c4350a40b 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -25,6 +25,8 @@ import { ContributorDecisionPackSchema, ContributorOpenPrMonitorSchema, ContributorPrOutcomesSchema, + ContributorNotificationFeedSchema, + ContributorNotificationsMarkReadSchema, ContributorRewardRiskStrategySchema, ContributorProfileSchema, ContributorScoringProfileSchema, @@ -792,6 +794,41 @@ export function buildOpenApiSpec() { }, }, }); + registry.registerPath({ + method: "get", + path: "/v1/contributors/{login}/notifications", + summary: "Contributor notification feed", + request: { params: z.object({ login: z.string() }) }, + responses: { + 200: { + description: "Self-scoped badge notification feed with unread count (mirrors loopover_list_notifications).", + content: { "application/json": { schema: ContributorNotificationFeedSchema } }, + }, + }, + }); + registry.registerPath({ + method: "post", + path: "/v1/contributors/{login}/notifications/read", + summary: "Mark contributor notifications read", + request: { + params: z.object({ login: z.string() }), + body: { + content: { + "application/json": { + schema: z.object({ + ids: z.array(z.string()).max(100).optional(), + }), + }, + }, + }, + }, + responses: { + 200: { + description: "Count of notifications marked read (mirrors loopover_mark_notifications_read).", + content: { "application/json": { schema: ContributorNotificationsMarkReadSchema } }, + }, + }, + }); registry.registerPath({ method: "get", path: "/v1/contributors/{login}/repos/{owner}/{repo}/decision", diff --git a/test/integration/routes-errors.test.ts b/test/integration/routes-errors.test.ts index abcbb2fce5..a081aebfca 100644 --- a/test/integration/routes-errors.test.ts +++ b/test/integration/routes-errors.test.ts @@ -233,6 +233,21 @@ describe("api route guards and error branches", () => { expect(ownPrOutcomes.status).toBe(200); await expect(ownPrOutcomes.json()).resolves.toMatchObject({ login: "attacker", outcomes: expect.any(Array) }); + const victimNotifications = await app.request("/v1/contributors/victim/notifications", { headers: sessionHeaders }, env); + expect(victimNotifications.status).toBe(403); + await expect(victimNotifications.json()).resolves.toMatchObject({ error: "forbidden_contributor" }); + + const ownNotifications = await app.request("/v1/contributors/attacker/notifications", { headers: sessionHeaders }, env); + expect(ownNotifications.status).toBe(200); + await expect(ownNotifications.json()).resolves.toMatchObject({ login: "attacker", notifications: expect.any(Array) }); + + const victimMarkRead = await app.request("/v1/contributors/victim/notifications/read", { + method: "POST", + headers: sessionHeaders, + body: JSON.stringify({}), + }, env); + expect(victimMarkRead.status).toBe(403); + const victimRepoDecision = await app.request("/v1/contributors/victim/repos/owner/private-repo/decision", { headers: sessionHeaders }, env); expect(victimRepoDecision.status).toBe(403); await expect(victimRepoDecision.json()).resolves.toMatchObject({ error: "forbidden_contributor" }); diff --git a/test/unit/mcp-cli-notifications.test.ts b/test/unit/mcp-cli-notifications.test.ts new file mode 100644 index 0000000000..8e58350c6f --- /dev/null +++ b/test/unit/mcp-cli-notifications.test.ts @@ -0,0 +1,151 @@ +// #6745: CLI + stdio mirrors for loopover_list_notifications / loopover_mark_notifications_read. +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { + closeFixtureServer, + markNotificationsReadFixture, + notificationsFixture, + run, + runAsync, + runExpectingFailure, + startFixtureServer, +} from "./support/mcp-cli-harness"; + +const bin = join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js"); + +let client: Client; +let transport: StdioClientTransport; +let configDir: string; +let apiUrl: string; +let capturedRequests: Array<{ url: string; method: string; body?: unknown }>; + +async function connect() { + configDir = mkdtempSync(join(tmpdir(), "loopover-notifications-")); + capturedRequests = []; + apiUrl = await startFixtureServer({ + onApiRequest: (request) => { + if (request.url && request.url.includes("/notifications")) { + capturedRequests.push({ url: request.url ?? "", method: request.method ?? "GET" }); + } + }, + }); + transport = new StdioClientTransport({ + command: "node", + args: [bin, "--stdio"], + env: { + ...process.env, + LOOPOVER_CONFIG_DIR: configDir, + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + LOOPOVER_API_TIMEOUT_MS: "5000", + }, + }); + client = new Client({ name: "notifications-cli-test", version: "0.0.1" }); + await client.connect(transport); +} + +async function disconnect() { + await client.close().catch(() => undefined); + await closeFixtureServer(); + if (configDir) rmSync(configDir, { recursive: true, force: true }); +} + +describe("loopover_list_notifications / mark-read stdio (#6745)", () => { + beforeEach(connect); + afterEach(disconnect); + + it("registers both tools", async () => { + const { tools } = await client.listTools(); + const names = tools.map((t) => t.name); + expect(names).toContain("loopover_list_notifications"); + expect(names).toContain("loopover_mark_notifications_read"); + }); + + it("proxies list + mark-read to the REST routes", async () => { + const listed = await client.callTool({ name: "loopover_list_notifications", arguments: { login: "JSONbored" } }); + expect(capturedRequests.some((r) => r.method === "GET" && r.url.includes("/notifications"))).toBe(true); + expect(JSON.stringify(listed)).toContain(notificationsFixture().summary); + + const marked = await client.callTool({ + name: "loopover_mark_notifications_read", + arguments: { login: "JSONbored", ids: ["n-1"] }, + }); + expect(capturedRequests.some((r) => r.method === "POST" && r.url.includes("/notifications/read"))).toBe(true); + expect(JSON.stringify(marked)).toContain("Marked 1"); + }); +}); + +describe("loopover-mcp notifications CLI (#6745)", () => { + beforeEach(connect); + afterEach(disconnect); + + it("--json list/mark mirrors the MCP tools for the same login", async () => { + const viaTool = await client.callTool({ name: "loopover_list_notifications", arguments: { login: "JSONbored" } }); + const toolData = (viaTool as { structuredContent?: unknown }).structuredContent; + const viaCli = JSON.parse( + await runAsync(["notifications", "--login", "JSONbored", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }), + ); + expect(viaCli).toEqual(notificationsFixture()); + if (toolData !== undefined) expect(viaCli).toEqual(toolData); + + const markTool = await client.callTool({ name: "loopover_mark_notifications_read", arguments: { login: "JSONbored" } }); + const markCli = JSON.parse( + await runAsync(["mark-notifications-read", "--login", "JSONbored", "--json"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }), + ); + expect(markCli).toEqual(markNotificationsReadFixture()); + const markData = (markTool as { structuredContent?: unknown }).structuredContent; + if (markData !== undefined) expect(markCli).toEqual(markData); + }); + + it("prints summaries and forwards --id filters", async () => { + const listOut = await runAsync(["notifications", "--login", "JSONbored"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }); + expect(listOut).toContain(notificationsFixture().summary); + expect(listOut).toContain("delivered JSONbored/loopover#42"); + + const markOut = await runAsync(["mark-notifications-read", "--login", "JSONbored", "--id", "n-1", "--id", "n-2"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }); + expect(markOut).toContain("Marked 2"); + }); + + it("falls back when the API omits summary", async () => { + await closeFixtureServer(); + const sparseUrl = await startFixtureServer({ + notifications: { summary: " ", notifications: [] }, + markNotificationsRead: { summary: "", marked: 0 }, + }); + const env = { LOOPOVER_API_URL: sparseUrl, LOOPOVER_TOKEN: "session-token" }; + expect(await runAsync(["notifications", "--login", "JSONbored"], env)).toContain("LoopOver notifications for JSONbored."); + expect(await runAsync(["mark-notifications-read", "--login", "JSONbored"], env)).toContain( + "Marked LoopOver notification(s) read for JSONbored.", + ); + }); + + it("requires login and documents --help / completion", () => { + const failure = runExpectingFailure(["notifications"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + LOOPOVER_LOGIN: "", + GITHUB_LOGIN: "", + }); + expect(failure.status).toBe(1); + + expect(run(["--help"])).toContain("loopover-mcp notifications --login [--json]"); + expect(run(["--help"])).toContain("mark-notifications-read"); + expect(run(["notifications", "--help"])).toContain("Mirrors the loopover_list_notifications MCP tool"); + expect(run(["mark-notifications-read", "--help"])).toContain("Mirrors the loopover_mark_notifications_read MCP tool"); + expect(run(["completion", "bash"])).toContain("notifications"); + expect(run(["completion", "bash"])).toContain("mark-notifications-read"); + }); +}); diff --git a/test/unit/mcp-tool-rename-aliases.test.ts b/test/unit/mcp-tool-rename-aliases.test.ts index 8e8be0c06f..ec809b5d35 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -20,6 +20,7 @@ // (#6740 registered the loopover_explain_gate_disposition CLI mirror, taking the count from 75 to 76.) // (#6741 registered the loopover_draft_pr_body CLI mirror, taking the count from 76 to 77.) // (#6747 registered the loopover_pr_outcome CLI mirror, taking the count from 77 to 78.) +// (#6745 registered the notifications list + mark-read CLI mirrors, taking the count from 78 to 80.) import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { mkdtempSync, rmSync } from "node:fs"; @@ -67,14 +68,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 78 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 80 loopover_ tools and zero gittensory_-prefixed aliases", async () => { const { tools } = await client.listTools(); const names = tools.map((t) => t.name); const primary = names.filter((n) => n.startsWith("loopover_")); const legacy = names.filter((n) => n.startsWith("gittensory_")); - expect(primary.length).toBe(78); + expect(primary.length).toBe(80); expect(legacy.length).toBe(0); - expect(names.length).toBe(78); + expect(names.length).toBe(80); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -86,14 +87,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 78-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 80-tool count the live server registers", async () => { const { tools } = await client.listTools(); const payload = JSON.parse(run(["tools", "--json"])) as { count: number; tools: Array<{ name: string }>; }; expect(payload.count).toBe(tools.length); - expect(payload.count).toBe(78); + expect(payload.count).toBe(80); expect([...payload.tools.map((t) => t.name)].sort()).toEqual( [...tools.map((t) => t.name)].sort(), ); diff --git a/test/unit/openapi.test.ts b/test/unit/openapi.test.ts index 2892ad63c0..06d69a9897 100644 --- a/test/unit/openapi.test.ts +++ b/test/unit/openapi.test.ts @@ -25,6 +25,8 @@ describe("OpenAPI contract", () => { expect(spec.paths["/v1/contributors/{login}/decision-pack"]).toBeDefined(); expect(spec.paths["/v1/contributors/{login}/open-pr-monitor"]).toBeDefined(); expect(spec.paths["/v1/contributors/{login}/pr-outcomes"]).toBeDefined(); + expect(spec.paths["/v1/contributors/{login}/notifications"]).toBeDefined(); + expect(spec.paths["/v1/contributors/{login}/notifications/read"]).toBeDefined(); expect(spec.paths["/v1/contributors/{login}/repos/{owner}/{repo}/decision"]).toBeDefined(); expect(spec.paths["/v1/preflight/pr"]).toBeDefined(); expect(spec.paths["/v1/preflight/local-diff"]).toBeDefined(); diff --git a/test/unit/routes-notifications.test.ts b/test/unit/routes-notifications.test.ts new file mode 100644 index 0000000000..03f354a900 --- /dev/null +++ b/test/unit/routes-notifications.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from "vitest"; +import { createApp } from "../../src/api/routes"; +import { LoopoverMcp } from "../../src/mcp/server"; +import { createSessionForGitHubUser } from "../../src/auth/security"; +import { + MAX_NOTIFICATION_DELIVERY_ID_LENGTH, + MAX_NOTIFICATION_MARK_READ_IDS, + insertNotificationDeliveryIfAbsent, + markNotificationDeliveryDelivered, +} from "../../src/db/repositories"; +import { loadContributorNotificationFeed, markContributorNotificationsRead } from "../../src/notifications/service"; +import { createTestEnv } from "../helpers/d1"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; + +const apiHeaders = (env: Env) => ({ authorization: `Bearer ${env.LOOPOVER_API_TOKEN}` }); + +async function seedDelivered(env: Env, recipientLogin: string, dedupKey: string) { + const { delivery } = await insertNotificationDeliveryIfAbsent(env, { + dedupKey, + channel: "badge", + recipientLogin, + eventType: "pull_request_changes_requested", + repoFullName: "owner/repo", + pullNumber: 7, + title: "Changes requested on owner/repo#7", + body: "A reviewer requested changes on your pull request owner/repo#7.", + deeplink: "https://github.com/owner/repo/pull/7", + actorLogin: "reviewer", + }); + await markNotificationDeliveryDelivered(env, delivery.id); + return delivery.id; +} + +describe("GET/POST /v1/contributors/:login/notifications (#6745)", () => { + it("lists unread notifications for the authenticated contributor", async () => { + const app = createApp(); + const env = createTestEnv(); + await seedDelivered(env, "miner", "k1"); + + const response = await app.request("/v1/contributors/miner/notifications", { headers: apiHeaders(env) }, env); + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ + login: "miner", + unreadCount: 1, + summary: "LoopOver notifications for miner: 1 unread.", + notifications: [{ repoFullName: "owner/repo", pullNumber: 7, status: "delivered" }], + }); + }); + + it("marks all notifications read, then specific ids", async () => { + const app = createApp(); + const env = createTestEnv(); + await seedDelivered(env, "miner", "k2"); + const idB = await seedDelivered(env, "miner", "k3"); + + const markAll = await app.request("/v1/contributors/miner/notifications/read", { + method: "POST", + headers: { ...apiHeaders(env), "content-type": "application/json" }, + body: JSON.stringify({}), + }, env); + expect(markAll.status).toBe(200); + await expect(markAll.json()).resolves.toMatchObject({ login: "miner", marked: 2 }); + + const idC = await seedDelivered(env, "miner", "k4"); + const markOne = await app.request("/v1/contributors/miner/notifications/read", { + method: "POST", + headers: { ...apiHeaders(env), "content-type": "application/json" }, + body: JSON.stringify({ ids: [idC] }), + }, env); + expect(markOne.status).toBe(200); + await expect(markOne.json()).resolves.toMatchObject({ login: "miner", marked: 1 }); + expect(idB).toBeTruthy(); + }); + + it("rejects invalid mark-read payloads", async () => { + const app = createApp(); + const env = createTestEnv(); + + const tooMany = await app.request("/v1/contributors/miner/notifications/read", { + method: "POST", + headers: { ...apiHeaders(env), "content-type": "application/json" }, + body: JSON.stringify({ ids: Array.from({ length: MAX_NOTIFICATION_MARK_READ_IDS + 1 }, (_, i) => `id-${i}`) }), + }, env); + expect(tooMany.status).toBe(400); + + const tooLong = await app.request("/v1/contributors/miner/notifications/read", { + method: "POST", + headers: { ...apiHeaders(env), "content-type": "application/json" }, + body: JSON.stringify({ ids: ["x".repeat(MAX_NOTIFICATION_DELIVERY_ID_LENGTH + 1)] }), + }, env); + expect(tooLong.status).toBe(400); + + const badJson = await app.request("/v1/contributors/miner/notifications/read", { + method: "POST", + headers: { ...apiHeaders(env), "content-type": "application/json" }, + body: "{", + }, env); + // malformed JSON falls through to {} → mark-all of zero + expect(badJson.status).toBe(200); + await expect(badJson.json()).resolves.toMatchObject({ marked: 0 }); + }); + + it("rejects unauthenticated callers and forbids cross-login sessions", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "miner" }); + const unauth = await app.request("/v1/contributors/miner/notifications", {}, env); + expect(unauth.status).toBeGreaterThanOrEqual(401); + + const { token } = await createSessionForGitHubUser(env, { login: "miner", id: 1 }); + const forbidden = await app.request("/v1/contributors/other/notifications", { + headers: { authorization: `Bearer ${token}` }, + }, env); + expect(forbidden.status).toBe(403); + await expect(forbidden.json()).resolves.toMatchObject({ error: "forbidden_contributor" }); + }); + + it("matches the host MCP tool payloads for the same login (mirror parity)", async () => { + const env = createTestEnv(); + await seedDelivered(env, "miner", "parity"); + + const viaBuilder = await loadContributorNotificationFeed(env, "miner"); + const app = createApp(); + const viaRest = await (await app.request("/v1/contributors/miner/notifications", { headers: apiHeaders(env) }, env)).json(); + + const server = new LoopoverMcp(env).createServer(); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + const client = new Client({ name: "notifications-parity", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + const viaMcp = await client.callTool({ name: "loopover_list_notifications", arguments: { login: "miner" } }); + + expect(viaRest).toEqual(viaBuilder); + expect((viaMcp as { structuredContent?: unknown }).structuredContent).toEqual(viaBuilder); + + const markEnv = createTestEnv(); + await seedDelivered(markEnv, "miner", "parity-mark"); + const markBuilder = await markContributorNotificationsRead(markEnv, "miner"); + + const markEnv2 = createTestEnv(); + await seedDelivered(markEnv2, "miner", "parity-mark-rest"); + const markRest = await ( + await app.request( + "/v1/contributors/miner/notifications/read", + { + method: "POST", + headers: { ...apiHeaders(markEnv2), "content-type": "application/json" }, + body: JSON.stringify({}), + }, + markEnv2, + ) + ).json(); + + const markEnv3 = createTestEnv(); + await seedDelivered(markEnv3, "miner", "parity-mark-mcp"); + const markServer = new LoopoverMcp(markEnv3).createServer(); + const [ct, st] = InMemoryTransport.createLinkedPair(); + await markServer.connect(st); + const markClient = new Client({ name: "notifications-mark-parity", version: "0.1.0" }, { capabilities: {} }); + await markClient.connect(ct); + const markMcp = await markClient.callTool({ name: "loopover_mark_notifications_read", arguments: { login: "miner" } }); + + expect(markRest).toEqual(markBuilder); + expect((markMcp as { structuredContent?: unknown }).structuredContent).toEqual(markBuilder); + }); +}); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index b58c1bccf9..bae6849e72 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -177,6 +177,8 @@ export async function startFixtureServer( validateConfigWarnings?: string[]; openPrMonitor?: Record; prOutcomes?: Record; + notifications?: Record; + markNotificationsRead?: Record; intakeStatus?: number; localBranchAnalysisStatus?: number; /** #6743: overrides the repo-doc refresh route's default "opened a new PR" response, e.g. to exercise @@ -318,6 +320,19 @@ export async function startFixtureServer( response.end(JSON.stringify({ ...prOutcomesFixture(login), ...(options.prOutcomes ?? {}) })); return; } + const notificationsMatch = request.url?.match(/^\/v1\/contributors\/([^/]+)\/notifications$/); + if (notificationsMatch && request.method === "GET") { + const login = decodeURIComponent(notificationsMatch[1]!); + response.end(JSON.stringify({ ...notificationsFixture(login), ...(options.notifications ?? {}) })); + return; + } + const notificationsReadMatch = request.url?.match(/^\/v1\/contributors\/([^/]+)\/notifications\/read$/); + if (notificationsReadMatch && request.method === "POST") { + const login = decodeURIComponent(notificationsReadMatch[1]!); + const body = (await readJsonRequest(request)) as { ids?: string[] }; + response.end(JSON.stringify({ ...markNotificationsReadFixture(login, body?.ids), ...(options.markNotificationsRead ?? {}) })); + return; + } if (request.url === "/v1/contributors/JSONbored/repos/JSONbored/loopover/decision" && request.method === "GET") { if (options.repoDecisionStatus && options.repoDecisionStatus >= 400) { response.statusCode = options.repoDecisionStatus; @@ -901,6 +916,37 @@ export function prOutcomesFixture(login = "JSONbored") { }; } +/** Mirrors GET /v1/contributors/:login/notifications / loadContributorNotificationFeed. */ +export function notificationsFixture(login = "JSONbored") { + return { + login: login.toLowerCase(), + unreadCount: 1, + summary: `LoopOver notifications for ${login}: 1 unread.`, + notifications: [ + { + id: "n-1", + eventType: "pull_request_changes_requested", + repoFullName: "JSONbored/loopover", + pullNumber: 42, + title: "Changes requested on JSONbored/loopover#42", + body: "A reviewer requested changes on your pull request JSONbored/loopover#42.", + deeplink: "https://github.com/JSONbored/loopover/pull/42", + status: "delivered" as const, + createdAt: "2026-06-01T00:00:00.000Z", + }, + ], + }; +} + +export function markNotificationsReadFixture(login = "JSONbored", ids?: string[]) { + const marked = ids?.length ? ids.length : 1; + return { + login: login.toLowerCase(), + marked, + summary: `Marked ${marked} LoopOver notification(s) read for ${login}.`, + }; +} + export function decisionPackFixture() { return { status: "ready",