diff --git a/desktop/src/features/projects/lib/projectReviewChecks.mjs b/desktop/src/features/projects/lib/projectReviewChecks.mjs index 1c8b910bff..e844728f00 100644 --- a/desktop/src/features/projects/lib/projectReviewChecks.mjs +++ b/desktop/src/features/projects/lib/projectReviewChecks.mjs @@ -359,11 +359,7 @@ export function readProjectReviewCheckRuns(storage, key) { } return Object.fromEntries( Object.entries(parsed).filter( - ([, run]) => - run && - typeof run === "object" && - !Array.isArray(run) && - run.status === "completed", + ([, run]) => run && typeof run === "object" && !Array.isArray(run), ), ); } catch { diff --git a/desktop/src/features/projects/lib/projectReviewChecks.test.mjs b/desktop/src/features/projects/lib/projectReviewChecks.test.mjs index 542f496beb..e979eff89f 100644 --- a/desktop/src/features/projects/lib/projectReviewChecks.test.mjs +++ b/desktop/src/features/projects/lib/projectReviewChecks.test.mjs @@ -296,7 +296,7 @@ test("builds a review-only prompt pinned to the exact commit", () => { assert.match(replacementPrompt, /does not cancel or combine any other/); }); -test("local check state restores only completed runs within its scope", () => { +test("local check lifecycle is restored within its scope", () => { const base = { relayUrl: "wss://relay.example", signerPubkey: "A".repeat(64), @@ -335,8 +335,6 @@ test("local check state restores only completed runs within its scope", () => { }, }; writeProjectReviewCheckRuns(storage, first, runs); - assert.deepEqual(readProjectReviewCheckRuns(storage, first), { - interface: runs.interface, - }); + assert.deepEqual(readProjectReviewCheckRuns(storage, first), runs); assert.deepEqual(readProjectReviewCheckRuns(storage, second), {}); }); diff --git a/desktop/src/features/projects/lib/projectReviewMemory.test.mjs b/desktop/src/features/projects/lib/projectReviewMemory.test.mjs index c9df5f2d16..882ff8352c 100644 --- a/desktop/src/features/projects/lib/projectReviewMemory.test.mjs +++ b/desktop/src/features/projects/lib/projectReviewMemory.test.mjs @@ -151,14 +151,15 @@ test("context prompt requests previous reviews and future-looking Buzz evidence" assert.match(prompt, /"request_id":"request-2"/); }); -test("only completed generated context is restored", () => { +test("generated context lifecycle is restored", () => { const storage = storageFixture(); const key = projectReviewContextStorageKey(scope); - writeProjectReviewContextRun(storage, key, { + const running = { agentPubkey: "agent", status: "running", - }); - assert.equal(readProjectReviewContextRun(storage, key), null); + }; + writeProjectReviewContextRun(storage, key, running); + assert.deepEqual(readProjectReviewContextRun(storage, key), running); const completed = { agentPubkey: "agent", diff --git a/desktop/src/features/projects/lib/projectReviewMemory.ts b/desktop/src/features/projects/lib/projectReviewMemory.ts index 13dee5c80a..97d2d8028c 100644 --- a/desktop/src/features/projects/lib/projectReviewMemory.ts +++ b/desktop/src/features/projects/lib/projectReviewMemory.ts @@ -272,10 +272,7 @@ export function readProjectReviewContextRun( if (!storage || !key) return null; try { const parsed: unknown = JSON.parse(storage.getItem(key) ?? "null"); - return parsed && - typeof parsed === "object" && - !Array.isArray(parsed) && - (parsed as Record).status === "completed" + return parsed && typeof parsed === "object" && !Array.isArray(parsed) ? (parsed as Record) : null; } catch { diff --git a/desktop/src/features/projects/ui/ProjectReviewChecks.tsx b/desktop/src/features/projects/ui/ProjectReviewChecks.tsx index 35d57976cc..4cbda6c2e7 100644 --- a/desktop/src/features/projects/ui/ProjectReviewChecks.tsx +++ b/desktop/src/features/projects/ui/ProjectReviewChecks.tsx @@ -71,6 +71,15 @@ type ReviewCheckRun = { type ReviewCheckRuns = Record; +function isReviewCheckStatus(value: unknown): value is ReviewCheckStatus { + return ( + value === "idle" || + value === "running" || + value === "completed" || + value === "failed" + ); +} + function normalizeStoredProposal( value: unknown, agentPubkey: string | null, @@ -129,13 +138,13 @@ function normalizeStoredRuns(value: Record): ReviewCheckRuns { const raw = candidate as Record; const agentPubkey = typeof raw.agentPubkey === "string" ? raw.agentPubkey : null; + const proposal = normalizeStoredProposal(raw.proposal, agentPubkey); + const status = isReviewCheckStatus(raw.status) ? raw.status : "idle"; const parsedResult = parseProjectReviewCheckResult( raw.result && typeof raw.result === "object" ? `BUZZ_CHECK_RESULT_V1\n${JSON.stringify(raw.result)}` : "", ); - if (raw.status !== "completed" || !parsedResult) continue; - const proposal = normalizeStoredProposal(raw.proposal, agentPubkey); const opener = raw.opener && typeof raw.opener === "object" && @@ -148,7 +157,7 @@ function normalizeStoredRuns(value: Record): ReviewCheckRuns { : undefined; runs[checkId] = { agentPubkey, - status: "completed", + status: status === "completed" && !parsedResult ? "idle" : status, ...(typeof raw.targetCommit === "string" || raw.targetCommit === null ? { targetCommit: raw.targetCommit as string | null } : {}), @@ -159,7 +168,7 @@ function normalizeStoredRuns(value: Record): ReviewCheckRuns { ? { requestId: raw.requestId } : {}), ...(opener ? { opener } : {}), - result: parsedResult, + ...(parsedResult ? { result: parsedResult } : {}), ...(proposal ? { proposal } : {}), ...(proposal && raw.acceptedProposalEventId === proposal.eventId ? { acceptedProposalEventId: proposal.eventId } diff --git a/desktop/src/features/projects/ui/ProjectReviewMemory.tsx b/desktop/src/features/projects/ui/ProjectReviewMemory.tsx index d1f1626646..89258c0332 100644 --- a/desktop/src/features/projects/ui/ProjectReviewMemory.tsx +++ b/desktop/src/features/projects/ui/ProjectReviewMemory.tsx @@ -64,16 +64,25 @@ type ReviewContextRun = { targetCommit?: string | null; }; +function isReviewContextStatus(value: unknown): value is ReviewContextStatus { + return ( + value === "idle" || + value === "running" || + value === "completed" || + value === "failed" + ); +} + function normalizeStoredRun( value: Record | null, ): ReviewContextRun | null { - if (value?.status !== "completed") return null; + if (!value) return null; + const status = isReviewContextStatus(value.status) ? value.status : "idle"; const result = parseProjectReviewContextResult( value.result && typeof value.result === "object" ? `${PROJECT_REVIEW_CONTEXT_RESULT_MARKER}\n${JSON.stringify(value.result)}` : "", ); - if (!result) return null; const rawOpener = value.opener && typeof value.opener === "object" && @@ -97,11 +106,11 @@ function normalizeStoredRun( ...(typeof value.requestId === "string" ? { requestId: value.requestId } : {}), - result, + ...(result ? { result } : {}), ...(typeof value.resultCreatedAt === "number" ? { resultCreatedAt: value.resultCreatedAt } : {}), - status: "completed", + status: status === "completed" && !result ? "idle" : status, ...(typeof value.targetCommit === "string" || value.targetCommit === null ? { targetCommit: value.targetCommit as string | null } : {}), diff --git a/desktop/tests/e2e/project-pr-memory.spec.ts b/desktop/tests/e2e/project-pr-memory.spec.ts index e3445737b6..f24a653715 100644 --- a/desktop/tests/e2e/project-pr-memory.spec.ts +++ b/desktop/tests/e2e/project-pr-memory.spec.ts @@ -141,6 +141,21 @@ test("review context is discovered by an agent and restored as sourced evidence" const requestId = requestIdFromPrompt(sentRequest?.content); const openerId = await openerEventId(page, requestId); + + await page.getByRole("button", { name: "Review", exact: true }).click(); + await openAliceReview(page); + await page + .getByRole("button", { name: "Prior art and future vision", exact: true }) + .click(); + const resumed = page.getByTestId("project-review-memory"); + await expect(resumed).toContainText("Searching"); + await expect( + resumed.getByRole("button", { + name: "Interrupt and find context again", + exact: true, + }), + ).toBeEnabled(); + await waitForDmSubscription(page); await page.evaluate( ({ agentPubkey, channelId, openerId, requestId }) => { @@ -182,7 +197,7 @@ test("review context is discovered by an agent and restored as sourced evidence" }, ); - const result = memory.getByTestId("project-review-context-result"); + const result = resumed.getByTestId("project-review-context-result"); await expect(result).toContainText("Keep review checks advisory"); await expect(result).toContainText("Turn review history into team memory"); await expect(result).toContainText("#engineering"); diff --git a/desktop/tests/e2e/project-pr-review.spec.ts b/desktop/tests/e2e/project-pr-review.spec.ts index 3cc05b9e29..c1f2d56d88 100644 --- a/desktop/tests/e2e/project-pr-review.spec.ts +++ b/desktop/tests/e2e/project-pr-review.spec.ts @@ -549,7 +549,7 @@ test("running review check can be interrupted with a replacement request", async ); }); -test("fresh review load restores completed checks but ignores unfinished runs", async ({ +test("fresh review load resumes unfinished checks and captures late results", async ({ page, }) => { await enableProjectsFeature(page); @@ -586,7 +586,7 @@ test("fresh review load restores completed checks but ignores unfinished runs", ) .toBe(true); - await page.evaluate(() => { + const unfinishedRun = await page.evaluate(() => { const key = Object.keys(window.localStorage).find((candidate) => candidate.startsWith("buzz.projects.review-checks.v1:"), ); @@ -615,6 +615,11 @@ test("fresh review load restores completed checks but ignores unfinished runs", status: "failed", }; window.localStorage.setItem(key, JSON.stringify(runs)); + return { + agentPubkey: activeRun.agentPubkey as string, + openerEventId: activeRun.opener.eventId as string, + requestId: "unfinished-request", + }; }); await page @@ -635,14 +640,42 @@ test("fresh review load restores completed checks but ignores unfinished runs", reloadedRows.nth(0).getByRole("button", { name: "Run check again" }), ).toHaveText(""); await expect( - reloadedRows.nth(1).getByRole("button", { name: "Run", exact: true }), + reloadedRows.nth(1).getByRole("button", { + name: "Interrupt and rerun check", + exact: true, + }), ).toBeEnabled(); - await expect(reloadedRows.nth(1)).not.toContainText("Running"); + await expect(reloadedRows.nth(1)).toContainText("Running"); await expect( - reloadedRows.nth(2).getByRole("button", { name: "Run", exact: true }), + reloadedRows.nth(2).getByRole("button", { + name: "Run check again", + exact: true, + }), ).toBeEnabled(); - await expect(reloadedRows.nth(2)).not.toContainText("Couldn’t run"); - await expect(reloadedRows.nth(2)).not.toContainText("Transient failure."); + await expect(reloadedRows.nth(2)).toContainText("Couldn’t run"); + await expect(reloadedRows.nth(2)).toContainText("Transient failure."); + + await waitForMockLiveSubscription(page, "DM"); + await page.evaluate(({ agentPubkey, openerEventId, requestId }) => { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "DM", + content: `BUZZ_CHECK_RESULT_V1\n${JSON.stringify({ + request_id: requestId, + conclusion: "approved", + summary: "Late result captured after the review remounted.", + diff_event_id: null, + findings: [], + })}`, + createdAt: Math.floor(Date.now() / 1_000) + 1, + kind: 9, + parentEventId: openerEventId, + pubkey: agentPubkey, + }); + }, unfinishedRun); + await expect(reloadedRows.nth(1)).toContainText("Approved"); + await expect(reloadedRows.nth(1)).toContainText( + "Late result captured after the review remounted.", + ); }); test("concurrent review checks only accept their correlated result", async ({