mirror of
https://github.com/block/buzz.git
synced 2026-11-08 18:31:56 +00:00
Improve project review check lifecycle
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
This commit is contained in:
@@ -62,6 +62,8 @@ export function buildProjectReviewCheckPrompt(input: {
|
||||
commit: string | null;
|
||||
branchName: string | null;
|
||||
targetBranch: string | null;
|
||||
supersededRequestId?: string;
|
||||
supersededEventId?: string;
|
||||
}): string;
|
||||
|
||||
export function projectReviewChecksStorageKey(input: {
|
||||
|
||||
@@ -12,8 +12,9 @@ export const DEFAULT_PROJECT_REVIEW_CHECKS = [
|
||||
description:
|
||||
"Reviews the user-facing experience and frontend implementation as one surface.",
|
||||
instructions: [
|
||||
"Inspect the user-facing behavior and frontend implementation changed by this review.",
|
||||
"Trace the user-visible flow through the frontend state and backend contract changed by this review.",
|
||||
"Check design-system usage, interaction states, accessibility, loading and error states, performance, and visual consistency.",
|
||||
"Probe alternate input modes, empty and partial data, permission boundaries, and failed or interrupted interactions where relevant.",
|
||||
"Flag hand-rolled UI where the project design system already provides the intended primitive.",
|
||||
],
|
||||
},
|
||||
@@ -25,6 +26,7 @@ export const DEFAULT_PROJECT_REVIEW_CHECKS = [
|
||||
instructions: [
|
||||
"Trace the changed behavior through its callers, data flow, and external contracts.",
|
||||
"Look for logic errors, invalid state transitions, missed edge cases, unsafe error handling, races, and resource-lifecycle defects.",
|
||||
"Probe hidden assumptions, security and authorization boundaries, concurrency, persistence, and compatibility where relevant.",
|
||||
"Report behavior-level defects introduced by or made materially worse by this review; leave codebase conventions to their own check.",
|
||||
],
|
||||
},
|
||||
@@ -36,6 +38,7 @@ export const DEFAULT_PROJECT_REVIEW_CHECKS = [
|
||||
instructions: [
|
||||
"Compare the change with contributor guidance and analogous code already in the repository.",
|
||||
"Check architectural boundaries, shared abstractions, naming, error handling, and framework idioms for consistency with established patterns.",
|
||||
"Prefer the smallest safe implementation that fully solves the problem; flag opportunistic refactors and speculative abstractions that add material risk.",
|
||||
"Report departures that create meaningful maintenance or integration risk; leave functional defects to the code correctness check.",
|
||||
],
|
||||
},
|
||||
@@ -47,7 +50,8 @@ export const DEFAULT_PROJECT_REVIEW_CHECKS = [
|
||||
instructions: [
|
||||
"Inspect relevant line and path history with git blame and git log, then follow related commits into pull requests or review discussion when available.",
|
||||
"Look for prior design rationale, analogous changes, reversions, and decisions from maintainers or frequent contributors to this area.",
|
||||
"Evaluate whether this review continues, deliberately revises, or accidentally contradicts the philosophy behind the existing implementation; clearly distinguish historical evidence from inference.",
|
||||
"Cite the exact commits, reviews, links, files, or lines supporting the historical interpretation, and clearly distinguish evidence from inference.",
|
||||
"Evaluate whether this review continues, deliberately revises, or accidentally contradicts the philosophy behind the existing implementation.",
|
||||
"Do not reject a change merely because it differs from precedent. Recommend a fix only when an unexplained philosophical shift creates material product, architecture, or maintenance risk.",
|
||||
],
|
||||
},
|
||||
@@ -59,6 +63,7 @@ export const DEFAULT_PROJECT_REVIEW_CHECKS = [
|
||||
instructions: [
|
||||
"Compare the tests with the production behavior changed by this review.",
|
||||
"Look for missing behavioral coverage, assertions that cannot catch regressions, and over-mocked paths.",
|
||||
"Attribute commands and verification results to the exact reviewed commit and working-tree state; do not claim a check ran without evidence that it did.",
|
||||
"Do not treat a green test command by itself as evidence of adequate test quality.",
|
||||
],
|
||||
},
|
||||
@@ -255,6 +260,8 @@ export function buildProjectReviewCheckPrompt({
|
||||
commit,
|
||||
branchName,
|
||||
targetBranch,
|
||||
supersededRequestId,
|
||||
supersededEventId,
|
||||
}) {
|
||||
const scope = [
|
||||
`Project: ${JSON.stringify(projectName)}`,
|
||||
@@ -271,14 +278,25 @@ export function buildProjectReviewCheckPrompt({
|
||||
`Run the project review check ${JSON.stringify(check.name)}.`,
|
||||
"THREAD CONTRACT: This message is one independent project review check request.",
|
||||
"The enclosing Buzz event's Event ID (shown by the agent harness immediately above this message content) is THIS_REQUEST_EVENT_ID and is this request's thread root.",
|
||||
"If one turn contains multiple project review check events, process every event independently. A newer event must never replace, cancel, combine with, or stand in for an earlier event.",
|
||||
`For every check event, publish exactly one final result in that event's own thread by providing the marker and JSON below on stdin to: buzz messages send --channel ${JSON.stringify(channelId)} --content - --reply-to <THIS_REQUEST_EVENT_ID>`,
|
||||
"If one turn contains multiple project review check events, process every event independently unless this request contains an INTERRUPT DIRECTIVE naming the exact request it supersedes.",
|
||||
"A newer event without that directive must never replace, cancel, combine with, or stand in for an earlier event.",
|
||||
`For every active, non-superseded check event, publish exactly one final result in that event's own thread by providing the marker and JSON below on stdin to: buzz messages send --channel ${JSON.stringify(channelId)} --content - --reply-to <THIS_REQUEST_EVENT_ID>`,
|
||||
"Replace <THIS_REQUEST_EVENT_ID> with the exact 64-character Event ID enclosing this request, without angle brackets. Never use the newest or another check event's ID as a shortcut.",
|
||||
"Never post a check result at the DM top level or in another check's thread. Do not finish the turn after answering only the last event; every request_id delivered in the turn needs its own threaded result.",
|
||||
"Never post a check result at the DM top level or in another check's thread. Do not finish the turn after answering only the last event; every non-superseded request_id delivered in the turn needs its own threaded result.",
|
||||
"Review only; do not modify code, push commits, or change the review.",
|
||||
"Use the repository and Buzz tools available to you to inspect the exact review commit.",
|
||||
"",
|
||||
...scope,
|
||||
...(supersededRequestId && supersededEventId
|
||||
? [
|
||||
"",
|
||||
"INTERRUPT DIRECTIVE: This replacement request supersedes exactly one earlier check request.",
|
||||
`Superseded request id: ${JSON.stringify(supersededRequestId)}`,
|
||||
`Superseded event id: ${supersededEventId}`,
|
||||
"Stop work on that superseded request and do not publish its result. Continue with this replacement request instead.",
|
||||
"This directive does not cancel or combine any other check request.",
|
||||
]
|
||||
: []),
|
||||
"",
|
||||
"Check definition:",
|
||||
...check.instructions.map((instruction) => `- ${instruction}`),
|
||||
@@ -336,9 +354,18 @@ export function readProjectReviewCheckRuns(storage, key) {
|
||||
if (!storage || !key) return {};
|
||||
try {
|
||||
const parsed = JSON.parse(storage.getItem(key) ?? "{}");
|
||||
return parsed && typeof parsed === "object" && !Array.isArray(parsed)
|
||||
? parsed
|
||||
: {};
|
||||
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
|
||||
return {};
|
||||
}
|
||||
return Object.fromEntries(
|
||||
Object.entries(parsed).filter(
|
||||
([, run]) =>
|
||||
run &&
|
||||
typeof run === "object" &&
|
||||
!Array.isArray(run) &&
|
||||
run.status === "completed",
|
||||
),
|
||||
);
|
||||
} catch {
|
||||
return {};
|
||||
}
|
||||
|
||||
@@ -33,10 +33,33 @@ test("historical intent check investigates precedent without treating it as a ve
|
||||
const instructions = check.instructions.join("\n");
|
||||
assert.match(instructions, /git blame and git log/);
|
||||
assert.match(instructions, /pull requests or review discussion/);
|
||||
assert.match(instructions, /historical evidence from inference/);
|
||||
assert.match(instructions, /clearly distinguish evidence from inference/);
|
||||
assert.match(instructions, /Do not reject a change merely because/);
|
||||
});
|
||||
|
||||
test("review guidance is assigned to the check that owns the concern", () => {
|
||||
const instructionsFor = (id) => {
|
||||
const check = DEFAULT_PROJECT_REVIEW_CHECKS.find(
|
||||
(candidate) => candidate.id === id,
|
||||
);
|
||||
assert.ok(check);
|
||||
return check.instructions.join("\n");
|
||||
};
|
||||
|
||||
assert.match(
|
||||
instructionsFor("code-correctness"),
|
||||
/security and authorization boundaries, concurrency, persistence, and compatibility/,
|
||||
);
|
||||
assert.match(
|
||||
instructionsFor("codebase-patterns"),
|
||||
/smallest safe implementation/,
|
||||
);
|
||||
assert.match(
|
||||
instructionsFor("test-quality"),
|
||||
/exact reviewed commit and working-tree state/,
|
||||
);
|
||||
});
|
||||
|
||||
test("parses an approved structured agent result", () => {
|
||||
const result = parseProjectReviewCheckResult(`Analysis first.
|
||||
|
||||
@@ -221,7 +244,7 @@ test("does not treat ordinary agent prose as a completed check", () => {
|
||||
});
|
||||
|
||||
test("builds a review-only prompt pinned to the exact commit", () => {
|
||||
const prompt = buildProjectReviewCheckPrompt({
|
||||
const input = {
|
||||
check: DEFAULT_PROJECT_REVIEW_CHECKS[1],
|
||||
projectName: "Buzz",
|
||||
repoAddress: "30617:owner:buzz",
|
||||
@@ -234,11 +257,13 @@ test("builds a review-only prompt pinned to the exact commit", () => {
|
||||
commit: "abc123",
|
||||
branchName: "checks",
|
||||
targetBranch: "main",
|
||||
});
|
||||
};
|
||||
const prompt = buildProjectReviewCheckPrompt(input);
|
||||
|
||||
assert.match(prompt, /Review only; do not modify code/);
|
||||
assert.match(prompt, /Commit under review: abc123/);
|
||||
assert.match(prompt, /Code correctness/);
|
||||
assert.match(prompt, /security and authorization boundaries/);
|
||||
assert.match(prompt, /"request_id":"request-123"/);
|
||||
assert.match(prompt, /Echo request_id exactly/);
|
||||
assert.match(prompt, /process every event independently/);
|
||||
@@ -253,9 +278,25 @@ test("builds a review-only prompt pinned to the exact commit", () => {
|
||||
assert.match(prompt, /"diff_event_id":null/);
|
||||
assert.match(prompt, /copy its exact 64-character event_id/);
|
||||
assert.match(prompt, new RegExp(PROJECT_REVIEW_CHECK_RESULT_MARKER));
|
||||
assert.doesNotMatch(prompt, /Superseded request id:/);
|
||||
|
||||
const replacementPrompt = buildProjectReviewCheckPrompt({
|
||||
...input,
|
||||
requestId: "request-456",
|
||||
supersededRequestId: "request-123",
|
||||
supersededEventId: "a".repeat(64),
|
||||
});
|
||||
assert.match(replacementPrompt, /INTERRUPT DIRECTIVE: This replacement/);
|
||||
assert.match(replacementPrompt, /Superseded request id: "request-123"/);
|
||||
assert.match(
|
||||
replacementPrompt,
|
||||
new RegExp(`Superseded event id: ${"a".repeat(64)}`),
|
||||
);
|
||||
assert.match(replacementPrompt, /do not publish its result/);
|
||||
assert.match(replacementPrompt, /does not cancel or combine any other/);
|
||||
});
|
||||
|
||||
test("local check state is isolated by relay, signer, repo, and review", () => {
|
||||
test("local check state restores only completed runs within its scope", () => {
|
||||
const base = {
|
||||
relayUrl: "wss://relay.example",
|
||||
signerPubkey: "A".repeat(64),
|
||||
@@ -275,9 +316,27 @@ test("local check state is isolated by relay, signer, repo, and review", () => {
|
||||
setItem: (key, value) => entries.set(key, value),
|
||||
};
|
||||
const runs = {
|
||||
"code-correctness": { agentPubkey: "agent", status: "idle" },
|
||||
interface: {
|
||||
agentPubkey: "agent",
|
||||
status: "completed",
|
||||
result: {
|
||||
request_id: "completed-request",
|
||||
conclusion: "approved",
|
||||
summary: "No material issues found.",
|
||||
diff_event_id: null,
|
||||
findings: [],
|
||||
},
|
||||
},
|
||||
"code-correctness": { agentPubkey: "agent", status: "running" },
|
||||
"codebase-patterns": {
|
||||
agentPubkey: "agent",
|
||||
status: "failed",
|
||||
error: "Agent unavailable.",
|
||||
},
|
||||
};
|
||||
writeProjectReviewCheckRuns(storage, first, runs);
|
||||
assert.deepEqual(readProjectReviewCheckRuns(storage, first), runs);
|
||||
assert.deepEqual(readProjectReviewCheckRuns(storage, first), {
|
||||
interface: runs.interface,
|
||||
});
|
||||
assert.deepEqual(readProjectReviewCheckRuns(storage, second), {});
|
||||
});
|
||||
|
||||
@@ -81,15 +81,6 @@ type ReviewCheckRun = {
|
||||
|
||||
type ReviewCheckRuns = Record<string, ReviewCheckRun>;
|
||||
|
||||
function isReviewCheckStatus(value: unknown): value is ReviewCheckStatus {
|
||||
return (
|
||||
value === "idle" ||
|
||||
value === "running" ||
|
||||
value === "completed" ||
|
||||
value === "failed"
|
||||
);
|
||||
}
|
||||
|
||||
function normalizeStoredProposal(
|
||||
value: unknown,
|
||||
agentPubkey: string | null,
|
||||
@@ -148,13 +139,13 @@ function normalizeStoredRuns(value: Record<string, unknown>): ReviewCheckRuns {
|
||||
const raw = candidate as Record<string, unknown>;
|
||||
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" &&
|
||||
@@ -167,7 +158,7 @@ function normalizeStoredRuns(value: Record<string, unknown>): ReviewCheckRuns {
|
||||
: undefined;
|
||||
runs[checkId] = {
|
||||
agentPubkey,
|
||||
status: status === "completed" && !parsedResult ? "idle" : status,
|
||||
status: "completed",
|
||||
...(typeof raw.targetCommit === "string" || raw.targetCommit === null
|
||||
? { targetCommit: raw.targetCommit as string | null }
|
||||
: {}),
|
||||
@@ -178,7 +169,7 @@ function normalizeStoredRuns(value: Record<string, unknown>): ReviewCheckRuns {
|
||||
? { requestId: raw.requestId }
|
||||
: {}),
|
||||
...(opener ? { opener } : {}),
|
||||
...(parsedResult ? { result: parsedResult } : {}),
|
||||
result: parsedResult,
|
||||
...(proposal ? { proposal } : {}),
|
||||
...(proposal && raw.acceptedProposalEventId === proposal.eventId
|
||||
? { acceptedProposalEventId: proposal.eventId }
|
||||
@@ -523,7 +514,11 @@ export function ProjectReviewChecks({
|
||||
);
|
||||
|
||||
const runCheck = React.useCallback(
|
||||
async (check: ProjectReviewCheckDefinition, agent: ReviewCheckAgent) => {
|
||||
async (
|
||||
check: ProjectReviewCheckDefinition,
|
||||
agent: ReviewCheckAgent,
|
||||
supersededRun?: ReviewCheckRun,
|
||||
) => {
|
||||
const expectedStorageKey = storageKey;
|
||||
if (!expectedStorageKey || !relayScope || !signerScope) return;
|
||||
const requestId = crypto.randomUUID();
|
||||
@@ -603,6 +598,8 @@ export function ProjectReviewChecks({
|
||||
commit: targetCommit,
|
||||
branchName: pullRequest.branchName,
|
||||
targetBranch: pullRequest.targetBranch,
|
||||
supersededRequestId: supersededRun?.requestId,
|
||||
supersededEventId: supersededRun?.opener?.eventId,
|
||||
});
|
||||
const sent = await sendChannelMessage(
|
||||
channel.id,
|
||||
@@ -640,13 +637,15 @@ export function ProjectReviewChecks({
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: "Failed to run review check.";
|
||||
updateRun(expectedStorageKey, check.id, (current) => ({
|
||||
...current,
|
||||
error: message,
|
||||
result: undefined,
|
||||
status: "failed",
|
||||
targetCommit,
|
||||
}));
|
||||
if (!supersededRun) {
|
||||
updateRun(expectedStorageKey, check.id, (current) => ({
|
||||
...current,
|
||||
error: message,
|
||||
result: undefined,
|
||||
status: "failed",
|
||||
targetCommit,
|
||||
}));
|
||||
}
|
||||
toast.error(message);
|
||||
} finally {
|
||||
if (storageKeyRef.current === expectedStorageKey) {
|
||||
@@ -704,13 +703,23 @@ export function ProjectReviewChecks({
|
||||
agentPubkey: null,
|
||||
status: "idle" as const,
|
||||
};
|
||||
const runAgent = run.agentPubkey
|
||||
? (reviewCheckAgents.candidates.find(
|
||||
(candidate) =>
|
||||
normalizePubkey(candidate.pubkey) ===
|
||||
normalizePubkey(run.agentPubkey ?? ""),
|
||||
) ?? null)
|
||||
: null;
|
||||
const runningAgent =
|
||||
run.status === "running" && run.agentPubkey
|
||||
? (reviewCheckAgents.candidates.find(
|
||||
(candidate) =>
|
||||
normalizePubkey(candidate.pubkey) ===
|
||||
normalizePubkey(run.agentPubkey ?? ""),
|
||||
) ?? null)
|
||||
: null;
|
||||
const actionAgent =
|
||||
run.status === "running" ? runningAgent : selectedAgent;
|
||||
const rerunsCheck = run.status !== "idle";
|
||||
const actionLabel =
|
||||
run.status === "running"
|
||||
? "Interrupt and rerun check"
|
||||
: rerunsCheck
|
||||
? "Run check again"
|
||||
: undefined;
|
||||
const isPending = pendingCheckIds.has(check.id);
|
||||
const isStale =
|
||||
run.status === "completed" && run.targetCommit !== targetCommit;
|
||||
@@ -749,38 +758,33 @@ export function ProjectReviewChecks({
|
||||
</div>
|
||||
<div className="flex flex-wrap items-center justify-end gap-2">
|
||||
<Button
|
||||
disabled={
|
||||
!selectedAgent ||
|
||||
isPending ||
|
||||
run.status === "running" ||
|
||||
!storageKey
|
||||
}
|
||||
aria-label={actionLabel}
|
||||
disabled={!actionAgent || isPending || !storageKey}
|
||||
onClick={() => {
|
||||
if (selectedAgent) void runCheck(check, selectedAgent);
|
||||
if (actionAgent) {
|
||||
void runCheck(
|
||||
check,
|
||||
actionAgent,
|
||||
run.status === "running" ? run : undefined,
|
||||
);
|
||||
}
|
||||
}}
|
||||
size="sm"
|
||||
size={rerunsCheck ? "icon" : "sm"}
|
||||
title={actionLabel}
|
||||
type="button"
|
||||
variant="secondary"
|
||||
>
|
||||
{run.status === "completed" || run.status === "failed" ? (
|
||||
<RotateCcw />
|
||||
) : isPending || run.status === "running" ? (
|
||||
{isPending ? (
|
||||
<LoaderCircle className="animate-spin" />
|
||||
) : rerunsCheck ? (
|
||||
<RotateCcw />
|
||||
) : (
|
||||
<Play />
|
||||
)}
|
||||
{run.status === "completed" || run.status === "failed"
|
||||
? "Run again"
|
||||
: "Run"}
|
||||
{rerunsCheck ? null : "Run"}
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
{run.status === "running" ? (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Waiting for {runAgent?.name ?? "the assigned agent"} to return
|
||||
a structured conclusion…
|
||||
</p>
|
||||
) : null}
|
||||
{run.error ? (
|
||||
<p className="text-xs text-destructive">{run.error}</p>
|
||||
) : null}
|
||||
|
||||
@@ -425,6 +425,226 @@ test("review checks dispatch to a running local managed agent", async ({
|
||||
expect(commands).not.toContain("start_managed_agent");
|
||||
});
|
||||
|
||||
test("running review check can be interrupted with a replacement request", async ({
|
||||
page,
|
||||
}) => {
|
||||
await enableProjectsFeature(page);
|
||||
await installMockBridge(page, {
|
||||
managedAgents: [
|
||||
{
|
||||
pubkey: RELAY_REVIEW_AGENT_PUBKEY,
|
||||
name: "Roof",
|
||||
status: "running",
|
||||
respondTo: "allowlist",
|
||||
respondToAllowlist: [],
|
||||
},
|
||||
],
|
||||
});
|
||||
await openBuzzProject(page);
|
||||
|
||||
await page.getByRole("tab", { name: "Review" }).click();
|
||||
const aliceRow = pullRequestRowByAuthor(page, "alice").first();
|
||||
await expect(aliceRow).toBeVisible({ timeout: 10_000 });
|
||||
await aliceRow.getByRole("button", { name: /^#/ }).click();
|
||||
await page.getByRole("button", { name: "Checks", exact: true }).click();
|
||||
|
||||
const interfaceCheck = page.getByTestId("project-review-check").first();
|
||||
await interfaceCheck
|
||||
.getByRole("button", { name: "Run", exact: true })
|
||||
.click();
|
||||
const checkPrompts = () =>
|
||||
page.evaluate(
|
||||
() =>
|
||||
window.__BUZZ_E2E_COMMAND_PAYLOADS__
|
||||
?.filter(
|
||||
(entry) =>
|
||||
entry.command === "send_channel_message" &&
|
||||
typeof (entry.payload as { content?: unknown })?.content ===
|
||||
"string" &&
|
||||
(entry.payload as { content: string }).content.includes(
|
||||
"BUZZ_CHECK_RESULT_V1",
|
||||
),
|
||||
)
|
||||
.map((entry) => (entry.payload as { content: string }).content) ?? [],
|
||||
);
|
||||
await expect.poll(async () => (await checkPrompts()).length).toBe(1);
|
||||
const firstPrompt = (await checkPrompts())[0];
|
||||
const firstRequestId = requestIdFromCheckPrompt(firstPrompt);
|
||||
const firstOpenerEventId = await checkOpenerEventId(page, firstRequestId);
|
||||
|
||||
const interruptButton = interfaceCheck.getByRole("button", {
|
||||
name: "Interrupt and rerun check",
|
||||
exact: true,
|
||||
});
|
||||
await expect(interruptButton).toBeEnabled();
|
||||
await expect(interruptButton).toHaveText("");
|
||||
await interruptButton.click();
|
||||
|
||||
await expect.poll(async () => (await checkPrompts()).length).toBe(2);
|
||||
const replacementPrompt = (await checkPrompts())[1];
|
||||
const replacementRequestId = requestIdFromCheckPrompt(replacementPrompt);
|
||||
expect(replacementRequestId).not.toBe(firstRequestId);
|
||||
expect(replacementPrompt).toContain(
|
||||
`Superseded request id: ${JSON.stringify(firstRequestId)}`,
|
||||
);
|
||||
expect(replacementPrompt).toContain(
|
||||
`Superseded event id: ${firstOpenerEventId}`,
|
||||
);
|
||||
const replacementOpenerEventId = await checkOpenerEventId(
|
||||
page,
|
||||
replacementRequestId,
|
||||
);
|
||||
expect(replacementOpenerEventId).not.toBe(firstOpenerEventId);
|
||||
|
||||
await waitForMockLiveSubscription(page, "DM");
|
||||
await page.evaluate(
|
||||
({
|
||||
agentPubkey,
|
||||
firstOpenerEventId,
|
||||
firstRequestId,
|
||||
replacementOpenerEventId,
|
||||
replacementRequestId,
|
||||
}) => {
|
||||
window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({
|
||||
channelName: "DM",
|
||||
content: `BUZZ_CHECK_RESULT_V1\n${JSON.stringify({
|
||||
request_id: firstRequestId,
|
||||
conclusion: "approved",
|
||||
summary: "Superseded result must be ignored.",
|
||||
diff_event_id: null,
|
||||
findings: [],
|
||||
})}`,
|
||||
createdAt: Math.floor(Date.now() / 1_000) + 1,
|
||||
kind: 9,
|
||||
parentEventId: firstOpenerEventId,
|
||||
pubkey: agentPubkey,
|
||||
});
|
||||
window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({
|
||||
channelName: "DM",
|
||||
content: `BUZZ_CHECK_RESULT_V1\n${JSON.stringify({
|
||||
request_id: replacementRequestId,
|
||||
conclusion: "approved",
|
||||
summary: "Replacement result accepted.",
|
||||
diff_event_id: null,
|
||||
findings: [],
|
||||
})}`,
|
||||
createdAt: Math.floor(Date.now() / 1_000) + 2,
|
||||
kind: 9,
|
||||
parentEventId: replacementOpenerEventId,
|
||||
pubkey: agentPubkey,
|
||||
});
|
||||
},
|
||||
{
|
||||
agentPubkey: RELAY_REVIEW_AGENT_PUBKEY,
|
||||
firstOpenerEventId,
|
||||
firstRequestId,
|
||||
replacementOpenerEventId,
|
||||
replacementRequestId,
|
||||
},
|
||||
);
|
||||
|
||||
await expect(interfaceCheck).toContainText("Replacement result accepted.");
|
||||
await expect(interfaceCheck).not.toContainText(
|
||||
"Superseded result must be ignored.",
|
||||
);
|
||||
});
|
||||
|
||||
test("fresh review load restores completed checks but ignores unfinished runs", async ({
|
||||
page,
|
||||
}) => {
|
||||
await enableProjectsFeature(page);
|
||||
await installMockBridge(page);
|
||||
await openBuzzProject(page);
|
||||
|
||||
await page.getByRole("tab", { name: "Review" }).click();
|
||||
const aliceRow = pullRequestRowByAuthor(page, "alice").first();
|
||||
await expect(aliceRow).toBeVisible({ timeout: 10_000 });
|
||||
await aliceRow.getByRole("button", { name: /^#/ }).click();
|
||||
await page.getByRole("button", { name: "Checks", exact: true }).click();
|
||||
|
||||
const checkRows = page.getByTestId("project-review-check");
|
||||
const interfaceCheck = checkRows.nth(0);
|
||||
await page.getByTestId("project-review-debug-harness-trigger").click();
|
||||
await page
|
||||
.getByTestId("project-review-debug-agent-select")
|
||||
.selectOption(RELAY_REVIEW_AGENT_PUBKEY);
|
||||
await interfaceCheck
|
||||
.getByRole("button", { name: "Run", exact: true })
|
||||
.click();
|
||||
await expect(interfaceCheck).toContainText("Running");
|
||||
await expect
|
||||
.poll(() =>
|
||||
page.evaluate(() => {
|
||||
for (let index = 0; index < window.localStorage.length; index += 1) {
|
||||
const key = window.localStorage.key(index);
|
||||
if (!key?.startsWith("buzz.projects.review-checks.v1:")) continue;
|
||||
const runs = JSON.parse(window.localStorage.getItem(key) ?? "{}");
|
||||
if (runs.interface?.status === "running") return true;
|
||||
}
|
||||
return false;
|
||||
}),
|
||||
)
|
||||
.toBe(true);
|
||||
|
||||
await page.evaluate(() => {
|
||||
const key = Object.keys(window.localStorage).find((candidate) =>
|
||||
candidate.startsWith("buzz.projects.review-checks.v1:"),
|
||||
);
|
||||
if (!key) throw new Error("Review check storage key was not created.");
|
||||
const runs = JSON.parse(window.localStorage.getItem(key) ?? "{}");
|
||||
const activeRun = runs.interface;
|
||||
runs.interface = {
|
||||
...activeRun,
|
||||
status: "completed",
|
||||
result: {
|
||||
request_id: activeRun.requestId,
|
||||
conclusion: "approved",
|
||||
summary: "Persisted completed result.",
|
||||
diff_event_id: null,
|
||||
findings: [],
|
||||
},
|
||||
};
|
||||
runs["code-correctness"] = {
|
||||
...activeRun,
|
||||
requestId: "unfinished-request",
|
||||
status: "running",
|
||||
};
|
||||
runs["codebase-patterns"] = {
|
||||
agentPubkey: activeRun.agentPubkey,
|
||||
error: "Transient failure.",
|
||||
status: "failed",
|
||||
};
|
||||
window.localStorage.setItem(key, JSON.stringify(runs));
|
||||
});
|
||||
|
||||
await page
|
||||
.getByRole("navigation", { name: "Project breadcrumb" })
|
||||
.getByRole("button", { name: "Review", exact: true })
|
||||
.click();
|
||||
const reloadedAliceRow = pullRequestRowByAuthor(page, "alice").first();
|
||||
await expect(reloadedAliceRow).toBeVisible({ timeout: 10_000 });
|
||||
await reloadedAliceRow.getByRole("button", { name: /^#/ }).click();
|
||||
await page.getByRole("button", { name: "Checks", exact: true }).click();
|
||||
|
||||
const reloadedRows = page.getByTestId("project-review-check");
|
||||
await expect(reloadedRows.nth(0)).toContainText("Approved");
|
||||
await expect(reloadedRows.nth(0)).toContainText(
|
||||
"Persisted completed result.",
|
||||
);
|
||||
await expect(
|
||||
reloadedRows.nth(0).getByRole("button", { name: "Run check again" }),
|
||||
).toHaveText("");
|
||||
await expect(
|
||||
reloadedRows.nth(1).getByRole("button", { name: "Run", exact: true }),
|
||||
).toBeEnabled();
|
||||
await expect(reloadedRows.nth(1)).not.toContainText("Running");
|
||||
await expect(
|
||||
reloadedRows.nth(2).getByRole("button", { name: "Run", exact: true }),
|
||||
).toBeEnabled();
|
||||
await expect(reloadedRows.nth(2)).not.toContainText("Couldn’t run");
|
||||
await expect(reloadedRows.nth(2)).not.toContainText("Transient failure.");
|
||||
});
|
||||
|
||||
test("concurrent review checks only accept their correlated result", async ({
|
||||
page,
|
||||
}) => {
|
||||
|
||||
Reference in New Issue
Block a user