diff --git a/docs/review-roadmap.md b/docs/review-roadmap.md index 57d4ca8..38c966b 100644 --- a/docs/review-roadmap.md +++ b/docs/review-roadmap.md @@ -84,6 +84,10 @@ The current implementation adds: - Public PR preview for uninstalled public repositories via GitHub REST. - GitHub OAuth sign-in for user-scoped review actions. - GitHub-native comment, reply, approval, thread-resolution, and viewed-file helpers. +- One review action module owns native writes and subsequent index updates. If GitHub accepts an + action but indexing fails, the response preserves native success and includes an index warning. + Missing native thread references prevent local-only replies or resolution; viewed-file mirroring + remains best effort after the patchset-aware mark is saved. - Hidden thread/comment markers on mirrored review comments. - Pull request webhook indexing for patchsets and per-file patch snapshots. - Recovery of Clearance-marked GitHub review comments from review-comment webhooks. diff --git a/docs/testing.md b/docs/testing.md index 5797ca9..8cce87f 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -65,6 +65,9 @@ Covered areas: - submitted review flow - invalid config behavior - scoped stale approval retention and invalidation +- review actions through `createReviewActions`, using mocked GitHub and review-index adapters: + native-write ordering, partial index failures, imported thread references, attention after review, + and best-effort viewed-file mirroring; a thin HTTP harness covers routing, validation, and CSRF. - escalation action application ### Webhook handler tests diff --git a/src/github/review-native.ts b/src/github/review-native.ts index 1351682..79f2c9f 100644 --- a/src/github/review-native.ts +++ b/src/github/review-native.ts @@ -125,10 +125,15 @@ export async function createGithubReviewThreadComment( "POST /repos/{owner}/{repo}/pulls/{pull_number}/comments", parameters, ); - const threadNodeId = - response.data.node_id === undefined - ? undefined - : await findGithubReviewThreadNodeId(octokit, ref, response.data.node_id); + let threadNodeId: string | undefined; + if (response.data.node_id !== undefined) { + try { + threadNodeId = await findGithubReviewThreadNodeId(octokit, ref, response.data.node_id); + } catch { + // The comment already exists. Resolution can recover this optional reference + // from its persisted comment id; failing here would invite a duplicate write. + } + } return { id: response.data.id, diff --git a/src/review/actions.ts b/src/review/actions.ts new file mode 100644 index 0000000..ab40cc7 --- /dev/null +++ b/src/review/actions.ts @@ -0,0 +1,187 @@ +import { randomUUID } from "node:crypto"; + +import { + createGithubReviewThreadComment, + findGithubPullRequestNodeId, + findGithubReviewThreadNodeIdForComment, + markGithubFileViewed, + replyToGithubReviewThread, + resolveGithubReviewThread, + serializeReviewThreadMarker, + submitGithubPullRequestReview, + type GithubNativeReviewOctokit, + type GithubPullRequestReviewEvent, +} from "../github/review-native.js"; +import { getPublicThreadRootCommentId } from "./public-snapshot.js"; +import type { DrizzleReviewStore, ReviewPullRequestRef } from "./store.js"; +import type { + AttentionPassRequest, + CreateThreadRequest, + MarkReviewedRequest, + ReplyThreadRequest, +} from "./types.js"; + +export type ReviewActionStore = Pick< + DrizzleReviewStore, + | "recordCreatedThread" + | "recordThreadReply" + | "loadThreadGithubRef" + | "resolveThread" + | "markFileReviewed" + | "passAttention" + | "markNotMyTurn" +>; + +export type ReviewActionResult = + | { + ok: true; + persisted: boolean; + githubMirrored?: boolean; + githubResolved?: boolean; + threadId?: string; + warning?: string; + } + | { ok: false; reason: "sign-in-required" | "thread-reference-unavailable"; error: string }; + +/** Native review writes precede index updates; a failed index cannot undo a GitHub success. */ +export function createReviewActions(input: { + actorLogin: string; + ref: ReviewPullRequestRef; + store: ReviewActionStore; + github?: GithubNativeReviewOctokit; +}) { + const { actorLogin, ref, store, github } = input; + + return { + // Patchset-aware marks are Clearance-owned; native viewed state is best effort. + async markFileReviewed(request: MarkReviewedRequest): Promise { + const persisted = await store.markFileReviewed(ref, actorLogin, request); + if (github === undefined) return { ok: true, persisted, githubMirrored: false }; + try { + const nodeId = + request.pullRequestNodeId ?? (await findGithubPullRequestNodeId(github, ref)); + if (nodeId !== undefined) { + await markGithubFileViewed(github, nodeId, request.filePath); + return { ok: true, persisted, githubMirrored: true }; + } + } catch { + // The local mark has already committed. Do not make callers retry it. + } + return { + ok: true, + persisted, + githubMirrored: false, + warning: "GitHub viewed-file state could not be updated.", + }; + }, + async passAttention(request: AttentionPassRequest): Promise { + return { ok: true, persisted: await store.passAttention(ref, actorLogin, request) }; + }, + async markNotMyTurn(): Promise { + return { ok: true, persisted: await store.markNotMyTurn(ref, actorLogin) }; + }, + async submitReview(request: { + body?: string; + event: GithubPullRequestReviewEvent; + }): Promise { + if (github === undefined) return signInRequired(); + await submitGithubPullRequestReview(github, ref, { + event: request.event, + body: request.body ?? (request.event === "APPROVE" ? "Reviewed in Clearance." : undefined), + }); + return { + ok: true, + githubMirrored: true, + ...(await indexNativeWrite(() => store.markNotMyTurn(ref, actorLogin))), + }; + }, + async resolveThread(threadId: string): Promise { + if (github === undefined) return signInRequired(); + const githubRef = await store.loadThreadGithubRef(ref, threadId); + const firstCommentId = githubRef?.firstCommentId ?? getPublicThreadRootCommentId(threadId); + const threadNodeId = + githubRef?.threadNodeId ?? + (firstCommentId === undefined + ? undefined + : await findGithubReviewThreadNodeIdForComment(github, ref, firstCommentId)); + if (threadNodeId === undefined) return missingThreadReference(); + await resolveGithubReviewThread(github, threadNodeId); + return { + ok: true, + githubResolved: true, + ...(await indexNativeWrite(() => store.resolveThread(ref, actorLogin, threadId))), + }; + }, + async replyToThread( + threadId: string, + request: ReplyThreadRequest, + ): Promise { + if (github === undefined) return signInRequired(); + const githubRef = await store.loadThreadGithubRef(ref, threadId); + const firstCommentId = githubRef?.firstCommentId ?? getPublicThreadRootCommentId(threadId); + if (firstCommentId === undefined) return missingThreadReference(); + const commentId = randomUUID(); + const marker = { commentId, threadId }; + const mirrored = await replyToGithubReviewThread( + github, + ref, + firstCommentId, + request.body, + marker, + ); + const indexed = await indexNativeWrite(() => + store.recordThreadReply(ref, actorLogin, threadId, request, { + commentId, + github: mirrored, + marker: serializeReviewThreadMarker(marker), + }), + ); + return { ok: true, githubMirrored: true, ...indexed }; + }, + async createThread(request: CreateThreadRequest): Promise { + if (github === undefined) return signInRequired(); + const threadId = randomUUID(); + const commentId = randomUUID(); + const marker = { commentId, threadId }; + const mirrored = await createGithubReviewThreadComment(github, ref, request, marker); + const indexed = await indexNativeWrite(() => + store.recordCreatedThread(ref, actorLogin, request, { + commentId, + github: mirrored, + marker: serializeReviewThreadMarker(marker), + threadId, + threadMarker: serializeReviewThreadMarker({ threadId }), + }), + ); + return { ok: true, githubMirrored: true, threadId, ...indexed }; + }, + }; +} + +function signInRequired(): ReviewActionResult { + return { + ok: false, + reason: "sign-in-required", + error: "Sign in with GitHub to change native review state.", + }; +} + +function missingThreadReference(): ReviewActionResult { + return { + ok: false, + reason: "thread-reference-unavailable", + error: "The GitHub review-thread reference is unavailable. Refresh the review before retrying.", + }; +} + +async function indexNativeWrite(persist: () => Promise) { + try { + return { persisted: await persist() }; + } catch { + return { + persisted: false, + warning: + "Saved on GitHub, but the Clearance review index could not be updated. Refresh to reload review state.", + }; + } +} diff --git a/src/server.ts b/src/server.ts index 568c314..7e67a97 100644 --- a/src/server.ts +++ b/src/server.ts @@ -2,7 +2,6 @@ import { createReadStream } from "node:fs"; import { stat } from "node:fs/promises"; import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; import { extname, join, normalize, sep } from "node:path"; -import { randomUUID } from "node:crypto"; import { fileURLToPath } from "node:url"; import { App } from "@octokit/app"; @@ -14,24 +13,12 @@ import dotenv from "dotenv"; import { createDatabaseClient, DrizzleClearanceStore } from "./db/index.js"; import { readEnv } from "./env.js"; import { registerGithubHandlers, type GithubWorkflowOctokit } from "./github/handlers.js"; -import { - createGithubReviewThreadComment, - findGithubPullRequestNodeId, - findGithubReviewThreadNodeIdForComment, - markGithubFileViewed, - replyToGithubReviewThread, - resolveGithubReviewThread, - serializeReviewThreadMarker, - submitGithubPullRequestReview, - type GithubPullRequestReviewEvent, - type GithubReviewCommentMirror, -} from "./github/index.js"; +import type { GithubPullRequestReviewEvent } from "./github/index.js"; import { createInstallationOctokit } from "./github/installation-client.js"; import { createRandomToken, DrizzleReviewAuthStore, DrizzleReviewStore, - getPublicThreadRootCommentId, loadReviewSnapshot, type AttentionPassRequest, type CreateThreadRequest, @@ -39,6 +26,7 @@ import { type ReplyThreadRequest, type SubmitReviewRequest, } from "./review/index.js"; +import { createReviewActions, type ReviewActionResult } from "./review/actions.js"; dotenv.config(); @@ -352,6 +340,13 @@ async function handleReviewApiRequest( return; } + const actions = createReviewActions({ + actorLogin, + ref: route, + store: reviewStore, + github: actor.accessToken === undefined ? undefined : createUserOctokit(actor.accessToken), + }); + if (request.method === "POST" && route.action === "marks") { const body = await readJsonBody(request); if ( @@ -363,19 +358,7 @@ async function handleReviewApiRequest( return; } - const persisted = await reviewStore.markFileReviewed(route, actorLogin, body); - let githubMirrored = false; - if (actor.accessToken !== undefined) { - const octokit = createUserOctokit(actor.accessToken); - const pullRequestNodeId = - body.pullRequestNodeId ?? (await findGithubPullRequestNodeId(octokit, route)); - if (pullRequestNodeId !== undefined) { - await markGithubFileViewed(octokit, pullRequestNodeId, body.filePath); - githubMirrored = true; - } - } - - writeJson(response, 202, { ok: true, githubMirrored, persisted }); + writeReviewActionResult(response, 202, await actions.markFileReviewed(body)); return; } @@ -386,14 +369,12 @@ async function handleReviewApiRequest( return; } - const persisted = await reviewStore.passAttention(route, actorLogin, body); - writeJson(response, 202, { ok: true, persisted }); + writeReviewActionResult(response, 202, await actions.passAttention(body)); return; } if (request.method === "POST" && route.action === "attention/not-my-turn") { - const persisted = await reviewStore.markNotMyTurn(route, actorLogin); - writeJson(response, 202, { ok: true, persisted }); + writeReviewActionResult(response, 202, await actions.markNotMyTurn()); return; } @@ -414,32 +395,7 @@ async function handleReviewApiRequest( return; } - const threadId = randomUUID(); - const commentId = randomUUID(); - const marker = { commentId, threadId }; - const github = await mirrorCreateThread(actor.accessToken, route, body, marker); - const persisted = await reviewStore.recordCreatedThread(route, actorLogin, body, { - commentId, - github, - marker: serializeReviewThreadMarker(marker), - threadId, - threadMarker: serializeReviewThreadMarker({ threadId }), - }); - - if (github === undefined && !persisted) { - writeJson(response, 401, { - error: - "Public pull request comments require GitHub sign-in or an indexed Clearance database.", - }); - return; - } - - writeJson(response, 201, { - githubMirrored: github !== undefined, - ok: true, - persisted, - threadId, - }); + writeReviewActionResult(response, 201, await actions.createThread(body)); return; } @@ -454,27 +410,7 @@ async function handleReviewApiRequest( return; } - const commentId = randomUUID(); - const marker = { commentId, threadId: route.threadId }; - const github = await mirrorThreadReply(actor.accessToken, route, route.threadId, body, marker); - const persisted = await reviewStore.recordThreadReply(route, actorLogin, route.threadId, body, { - commentId, - github, - marker: serializeReviewThreadMarker(marker), - }); - - if (github === undefined && !persisted) { - writeJson(response, 401, { - error: "Public pull request replies require GitHub sign-in or an indexed Clearance thread.", - }); - return; - } - - writeJson(response, 201, { - githubMirrored: github !== undefined, - ok: true, - persisted, - }); + writeReviewActionResult(response, 201, await actions.replyToThread(route.threadId, body)); return; } @@ -483,16 +419,7 @@ async function handleReviewApiRequest( route.threadId !== undefined && route.action === "threads/resolve" ) { - const githubResolved = await mirrorThreadResolution(actor.accessToken, route, route.threadId); - const persisted = await reviewStore.resolveThread(route, actorLogin, route.threadId); - if (!githubResolved && !persisted) { - writeJson(response, 409, { - error: "Resolving imported public threads requires indexed GitHub review-thread state.", - }); - return; - } - - writeJson(response, 202, { githubResolved, ok: true, persisted }); + writeReviewActionResult(response, 202, await actions.resolveThread(route.threadId)); return; } @@ -517,19 +444,22 @@ async function handleReviewApiRequest( return; } - await submitGithubPullRequestReview(createUserOctokit(actor.accessToken), route, { - body: body?.body ?? (event === "APPROVE" ? "Reviewed in Clearance." : undefined), - event, - }); - await reviewStore.markNotMyTurn(route, actorLogin); - - writeJson(response, 202, { githubMirrored: true, ok: true }); + writeReviewActionResult(response, 202, await actions.submitReview({ body: body?.body, event })); return; } writeJson(response, 404, { error: "review action not found" }); } +function writeReviewActionResult( + response: ServerResponse, + successStatus: number, + result: ReviewActionResult, +): void { + const status = result.ok ? successStatus : result.reason === "sign-in-required" ? 401 : 409; + writeJson(response, status, result); +} + /* * The public snapshot path uses GitHub REST directly. It intentionally does not * require a GitHub App installation; OAuth is used only when the viewer has @@ -695,76 +625,6 @@ async function requireReviewCsrf( return true; } -async function mirrorCreateThread( - accessToken: string | undefined, - ref: ReviewPullRequestApiRoute, - body: CreateThreadRequest, - marker: { commentId: string; threadId: string }, -): Promise { - if (accessToken === undefined) { - return undefined; - } - - return createGithubReviewThreadComment(createUserOctokit(accessToken), ref, body, marker); -} - -async function mirrorThreadReply( - accessToken: string | undefined, - ref: ReviewPullRequestApiRoute, - threadId: string, - body: ReplyThreadRequest, - marker: { commentId: string; threadId: string }, -): Promise { - if (accessToken === undefined) { - return undefined; - } - - const githubRef = await reviewStore.loadThreadGithubRef(ref, threadId); - const firstCommentId = githubRef?.firstCommentId ?? getPublicThreadRootCommentId(threadId); - if (firstCommentId === undefined) { - return undefined; - } - - return replyToGithubReviewThread( - createUserOctokit(accessToken), - ref, - firstCommentId, - body.body, - marker, - ); -} - -async function mirrorThreadResolution( - accessToken: string | undefined, - ref: ReviewPullRequestApiRoute, - threadId: string, -): Promise { - if (accessToken === undefined) { - return false; - } - - const githubRef = await reviewStore.loadThreadGithubRef(ref, threadId); - let threadNodeId = githubRef?.threadNodeId; - if (threadNodeId === undefined) { - const rootCommentId = getPublicThreadRootCommentId(threadId); - threadNodeId = - rootCommentId === undefined - ? undefined - : await findGithubReviewThreadNodeIdForComment( - createUserOctokit(accessToken), - ref, - rootCommentId, - ); - } - - if (threadNodeId === undefined) { - return false; - } - - await resolveGithubReviewThread(createUserOctokit(accessToken), threadNodeId); - return true; -} - function createUserOctokit(accessToken: string): Octokit { return new Octokit({ auth: accessToken }); } diff --git a/test/review/action-routes.test.ts b/test/review/action-routes.test.ts new file mode 100644 index 0000000..4a922e5 --- /dev/null +++ b/test/review/action-routes.test.ts @@ -0,0 +1,152 @@ +import { Socket } from "node:net"; +import { IncomingMessage, type RequestListener, type ServerResponse } from "node:http"; +import { afterAll, beforeAll, beforeEach, expect, it, vi } from "vitest"; + +import type { GithubNativeReviewOctokit } from "../../src/github/review-native.js"; +import type { ReviewActionStore } from "../../src/review/actions.js"; +import type { DrizzleReviewAuthStore } from "../../src/review/auth.js"; + +const harness = vi.hoisted(() => ({ + listener: undefined as RequestListener | undefined, + request: vi.fn(), + graphql: vi.fn(), + loadSession: vi.fn(), + verifyCsrf: vi.fn(), + recordCreatedThread: vi.fn(), + loadThreadGithubRef: vi.fn(), + resolveThread: vi.fn(), +})); + +vi.mock("node:http", async (importOriginal) => ({ + ...(await importOriginal()), + createServer: (listener: RequestListener) => { + harness.listener = listener; + return { listen: vi.fn<() => void>(), close: vi.fn<() => void>() }; + }, +})); +vi.mock("@octokit/core", () => ({ + Octokit: class { + request = harness.request; + graphql = harness.graphql; + }, +})); +vi.mock("../../src/db/index.js", () => ({ + createDatabaseClient: () => ({ db: {}, close: vi.fn<() => Promise>() }), + DrizzleClearanceStore: function () { + return {}; + }, +})); +vi.mock("../../src/review/store.js", () => ({ + DrizzleReviewStore: class { + recordCreatedThread = harness.recordCreatedThread; + loadThreadGithubRef = harness.loadThreadGithubRef; + resolveThread = harness.resolveThread; + }, +})); +vi.mock("../../src/review/auth.js", () => ({ + createRandomToken: vi.fn<() => string>(), + DrizzleReviewAuthStore: class { + loadSession = harness.loadSession; + verifyCsrf = harness.verifyCsrf; + }, +})); + +let initialSigint: ReturnType; +let initialSigterm: ReturnType; + +beforeAll(async () => { + vi.stubEnv("DATABASE_URL", "postgres://unused"); + vi.stubEnv("GITHUB_APP_ID", "123"); + vi.stubEnv("GITHUB_PRIVATE_KEY", "unused"); + vi.stubEnv("GITHUB_WEBHOOK_SECRET", "test-secret"); + initialSigint = process.listeners("SIGINT"); + initialSigterm = process.listeners("SIGTERM"); + await import("../../src/server.js"); +}); + +afterAll(() => { + for (const listener of process.listeners("SIGINT")) { + if (!initialSigint.includes(listener)) process.removeListener("SIGINT", listener); + } + for (const listener of process.listeners("SIGTERM")) { + if (!initialSigterm.includes(listener)) process.removeListener("SIGTERM", listener); + } + vi.unstubAllEnvs(); +}); + +beforeEach(() => { + vi.clearAllMocks(); + harness.loadSession.mockResolvedValue({ id: "alice-id", login: "alice", accessToken: "token" }); + harness.verifyCsrf.mockResolvedValue(true); + harness.request.mockResolvedValue({ data: { id: 42 } }); + harness.recordCreatedThread.mockResolvedValue(true); + harness.loadThreadGithubRef.mockResolvedValue(undefined); +}); + +it("returns native success and an index warning through the real comment route", async () => { + harness.recordCreatedThread.mockRejectedValue(new Error("index unavailable")); + const response = await post("threads", { + body: "Comment", + commitSha: "head", + filePath: "src/index.ts", + }); + expect(response.status).toBe(201); + expect(response.body).toMatchObject({ + ok: true, + githubMirrored: true, + persisted: false, + warning: expect.any(String), + }); +}); + +it("rejects unauthenticated mutation requests before writing", async () => { + harness.loadSession.mockResolvedValue(undefined); + const response = await post("threads", { + body: "Comment", + commitSha: "head", + filePath: "src/index.ts", + }); + expect(response.status).toBe(401); + expect(harness.request).not.toHaveBeenCalled(); + expect(harness.recordCreatedThread).not.toHaveBeenCalled(); +}); + +it("rejects a failed CSRF check before calling a review action", async () => { + harness.verifyCsrf.mockResolvedValue(false); + expect((await post("threads", {})).status).toBe(403); + expect(harness.request).not.toHaveBeenCalled(); +}); + +it("retains request validation before native writes", async () => { + expect((await post("threads", { body: "Comment" })).status).toBe(400); + expect(harness.request).not.toHaveBeenCalled(); +}); + +it("adapts a missing native resolution reference to conflict without changing the index", async () => { + expect((await post("threads/missing-thread/resolve", {})).status).toBe(409); + expect(harness.resolveThread).not.toHaveBeenCalled(); + expect(harness.graphql).not.toHaveBeenCalled(); +}); + +async function post(action: string, body: unknown): Promise<{ status: number; body: unknown }> { + const request = new IncomingMessage(new Socket()); + request.method = "POST"; + request.url = `/api/review/acme/widget/pull/17/${action}`; + request.headers = { cookie: "clearance_review_session=session", "x-clearance-csrf": "csrf" }; + request.push(Buffer.from(JSON.stringify(body))); + request.push(null); + return new Promise((resolve) => { + let status = 0; + const response = { + writeHead(code: number) { + status = code; + }, + end(responseBody: string) { + resolve({ status, body: JSON.parse(responseBody) }); + }, + } as ServerResponse; + if (harness.listener === undefined) + throw new Error("server did not register a request listener"); + harness.listener(request, response); + }); +} diff --git a/test/review/actions.test.ts b/test/review/actions.test.ts new file mode 100644 index 0000000..031018e --- /dev/null +++ b/test/review/actions.test.ts @@ -0,0 +1,266 @@ +import { describe, expect, it, vi } from "vitest"; + +import { createReviewActions, type ReviewActionStore } from "../../src/review/actions.js"; +import { + type GithubNativeReviewOctokit, + parseReviewThreadMarker, +} from "../../src/github/review-native.js"; + +const ref = { owner: "acme", repo: "widget", pullNumber: 17 }; +const thread = { + body: "Please simplify this.", + commitSha: "head", + filePath: "src/index.ts", + line: 12, +}; + +function setup() { + const store = { + recordCreatedThread: vi.fn().mockResolvedValue(true), + recordThreadReply: vi.fn().mockResolvedValue(true), + loadThreadGithubRef: vi + .fn() + .mockResolvedValue(undefined), + resolveThread: vi.fn().mockResolvedValue(true), + markFileReviewed: vi.fn().mockResolvedValue(true), + passAttention: vi.fn().mockResolvedValue(true), + markNotMyTurn: vi.fn().mockResolvedValue(true), + }; + const request = vi + .fn() + .mockResolvedValue({ data: { id: 42, html_url: "https://github.com/comment/42" } }); + const graphql = vi.fn().mockResolvedValue({}); + const github: GithubNativeReviewOctokit = { + request: request as GithubNativeReviewOctokit["request"], + graphql: graphql as GithubNativeReviewOctokit["graphql"], + }; + return { + actions: createReviewActions({ actorLogin: "alice", ref, store, github }), + store, + request, + graphql, + }; +} + +describe("review actions", () => { + it("reports a successful GitHub comment when updating the review index fails", async () => { + const { actions, store, request } = setup(); + store.recordCreatedThread.mockRejectedValue(new Error("database unavailable")); + + const result = await actions.createThread(thread); + + expect(result).toMatchObject({ + ok: true, + githubMirrored: true, + persisted: false, + warning: expect.stringContaining("GitHub"), + }); + const marker = parseReviewThreadMarker(String(request.mock.calls[0]?.[1].body)); + expect(marker?.threadId).toBe(result.ok ? result.threadId : undefined); + expect(marker?.commentId).toEqual(expect.any(String)); + expect(store.recordCreatedThread).toHaveBeenCalledWith( + ref, + "alice", + thread, + expect.objectContaining({ + threadId: marker?.threadId, + commentId: marker?.commentId, + github: expect.objectContaining({ id: 42 }), + }), + ); + }); +}); + +it("does not record a local reply when the GitHub root comment is unknown", async () => { + const { actions, store, request } = setup(); + expect(await actions.replyToThread("missing-thread", { body: "Reply" })).toMatchObject({ + ok: false, + reason: "thread-reference-unavailable", + }); + expect(request).not.toHaveBeenCalled(); + expect(store.recordThreadReply).not.toHaveBeenCalled(); +}); + +it("finds the GitHub thread through an indexed root comment before resolving locally", async () => { + const { actions, store, request, graphql } = setup(); + store.loadThreadGithubRef.mockResolvedValue({ firstCommentId: 42 }); + request.mockResolvedValue({ data: { node_id: "COMMENT_42" } }); + graphql + .mockResolvedValueOnce({ + repository: { + pullRequest: { + reviewThreads: { + pageInfo: { hasNextPage: false }, + nodes: [ + { + id: "THREAD_42", + comments: { pageInfo: { hasNextPage: false }, nodes: [{ id: "COMMENT_42" }] }, + }, + ], + }, + }, + }, + }) + .mockResolvedValueOnce({}); + store.resolveThread.mockImplementation(async () => { + expect(graphql).toHaveBeenLastCalledWith(expect.stringContaining("resolveReviewThread"), { + threadId: "THREAD_42", + }); + return true; + }); + expect(await actions.resolveThread("indexed-thread")).toMatchObject({ + ok: true, + githubResolved: true, + persisted: true, + }); +}); + +it("preserves a submitted review when clearing attention fails", async () => { + const { actions, store, request } = setup(); + store.markNotMyTurn.mockImplementation(async () => { + expect(request).toHaveBeenCalledWith("POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews", { + owner: "acme", + repo: "widget", + pull_number: 17, + event: "APPROVE", + body: "Reviewed in Clearance.", + }); + throw new Error("attention update failed"); + }); + expect(await actions.submitReview({ event: "APPROVE" })).toMatchObject({ + ok: true, + githubMirrored: true, + persisted: false, + warning: expect.any(String), + }); +}); + +it("keeps the patchset-aware mark when GitHub viewed-file mirroring fails", async () => { + const { actions, store, graphql } = setup(); + graphql.mockImplementation(async () => { + expect(store.markFileReviewed).toHaveBeenCalledWith(ref, "alice", { + filePath: "src/index.ts", + patchsetNumber: 2, + }); + throw new Error("GitHub unavailable"); + }); + expect( + await actions.markFileReviewed({ filePath: "src/index.ts", patchsetNumber: 2 }), + ).toMatchObject({ + ok: true, + persisted: true, + githubMirrored: false, + warning: expect.any(String), + }); +}); + +it("still indexes an accepted comment when GitHub thread discovery fails", async () => { + const { actions, store, request, graphql } = setup(); + request.mockResolvedValue({ data: { id: 42, node_id: "COMMENT_42" } }); + graphql.mockRejectedValue(new Error("GitHub read failed after write")); + expect(await actions.createThread(thread)).toMatchObject({ + ok: true, + githubMirrored: true, + persisted: true, + }); + expect(store.recordCreatedThread).toHaveBeenCalledWith( + ref, + "alice", + thread, + expect.objectContaining({ + github: expect.objectContaining({ id: 42, nodeId: "COMMENT_42", threadNodeId: undefined }), + }), + ); +}); + +it("does not resolve the index when an imported thread has no recoverable GitHub reference", async () => { + const { actions, store, graphql } = setup(); + expect(await actions.resolveThread("missing-thread")).toMatchObject({ + ok: false, + reason: "thread-reference-unavailable", + }); + expect(graphql).not.toHaveBeenCalled(); + expect(store.resolveThread).not.toHaveBeenCalled(); +}); + +it("replies to a public GitHub thread without requiring it in the index", async () => { + const { actions, store, request } = setup(); + store.recordThreadReply.mockResolvedValue(false); + expect(await actions.replyToThread("github-comment-42", { body: "Reply" })).toMatchObject({ + ok: true, + githubMirrored: true, + persisted: false, + }); + expect(request).toHaveBeenCalledWith( + expect.stringContaining("/replies"), + expect.objectContaining({ comment_id: 42 }), + ); +}); + +it("leaves attention unchanged when GitHub rejects a submitted review", async () => { + const { actions, store, request } = setup(); + request.mockRejectedValue(new Error("review rejected")); + await expect( + actions.submitReview({ event: "REQUEST_CHANGES", body: "Please fix this." }), + ).rejects.toThrow("review rejected"); + expect(store.markNotMyTurn).not.toHaveBeenCalled(); +}); + +it("does not record a native comment locally without a signed-in GitHub adapter", async () => { + const { store } = setup(); + const actions = createReviewActions({ actorLogin: "alice", ref, store }); + expect(await actions.createThread(thread)).toMatchObject({ + ok: false, + reason: "sign-in-required", + }); + expect(store.recordCreatedThread).not.toHaveBeenCalled(); +}); + +it("does not index a comment rejected by GitHub", async () => { + const { actions, store, request } = setup(); + request.mockRejectedValue(new Error("invalid diff line")); + await expect(actions.createThread(thread)).rejects.toThrow("invalid diff line"); + expect(store.recordCreatedThread).not.toHaveBeenCalled(); +}); + +it("mirrors a reviewed file after its local mark is persisted", async () => { + const { actions, store, graphql } = setup(); + graphql + .mockResolvedValueOnce({ repository: { pullRequest: { id: "PR_17" } } }) + .mockImplementationOnce(async () => { + expect(store.markFileReviewed).toHaveBeenCalled(); + return {} as TResponse; + }); + expect( + await actions.markFileReviewed({ filePath: "src/index.ts", patchsetNumber: 2 }), + ).toMatchObject({ ok: true, persisted: true, githubMirrored: true }); + expect(graphql).toHaveBeenLastCalledWith(expect.stringContaining("markFileAsViewed"), { + path: "src/index.ts", + pullRequestId: "PR_17", + }); +}); + +it("reports an accepted GitHub resolution even when the index write fails", async () => { + const { actions, store } = setup(); + store.loadThreadGithubRef.mockResolvedValue({ threadNodeId: "THREAD_42" }); + store.resolveThread.mockRejectedValue(new Error("index unavailable")); + expect(await actions.resolveThread("thread-42")).toMatchObject({ + ok: true, + githubResolved: true, + persisted: false, + warning: expect.any(String), + }); +}); + +it("passes and clears Clearance-owned attention through the review index", async () => { + const { actions, store, request, graphql } = setup(); + expect(await actions.passAttention({ targetLogin: "bob" })).toEqual({ + ok: true, + persisted: true, + }); + expect(await actions.markNotMyTurn()).toEqual({ ok: true, persisted: true }); + expect(store.passAttention).toHaveBeenCalledWith(ref, "alice", { targetLogin: "bob" }); + expect(store.markNotMyTurn).toHaveBeenCalledWith(ref, "alice"); + expect(request).not.toHaveBeenCalled(); + expect(graphql).not.toHaveBeenCalled(); +});