From b10a1e591a4a875f3c764008b620ad352e7cbb56 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Wed, 22 Apr 2026 13:59:08 -0600 Subject: [PATCH 1/3] Add persona review recheck state machine --- apps/server/src/agents/manager.ts | 371 ++++++++++++++++++++-- apps/server/src/reviews/poll-cadence.ts | 20 ++ apps/server/src/server.ts | 74 +++++ apps/server/src/shared/mcp/server.ts | 190 ++++++++++- apps/server/test/db/agent-manager.test.ts | 244 +++++++++++++- apps/server/test/poll-cadence.test.ts | 46 +++ 6 files changed, 897 insertions(+), 48 deletions(-) create mode 100644 apps/server/src/reviews/poll-cadence.ts create mode 100644 apps/server/test/poll-cadence.test.ts diff --git a/apps/server/src/agents/manager.ts b/apps/server/src/agents/manager.ts index 00366717..85e0a181 100644 --- a/apps/server/src/agents/manager.ts +++ b/apps/server/src/agents/manager.ts @@ -25,6 +25,10 @@ import { } from "../shared/git/worktree.js"; import { runCommand } from "../shared/lib/run-command.js"; import { loadRepoHooks } from "../shared/mcp/repo-tools.js"; +import { + RECHECK_POLL_TIMEOUT, + pollCadenceSeconds, +} from "../reviews/poll-cadence.js"; import { harvestTokenUsage } from "./token-harvester.js"; type AgentStatus = @@ -226,6 +230,30 @@ export type PersonaReviewResolutionRecord = { submittedAt: string; }; +export type PersonaReviewResolutionItem = { + feedbackId: number; + originalDescription: string; + originalSeverity: string; + status: string; + reason: string | null; + filePath: string | null; + lineNumber: number | null; + suggestion: string | null; + resolutionCommit: string | null; + resolvedAt: string | null; + roundNumber: number; +}; + +export type AwaitRecheckResult = + | { status: "pending"; pollAgainInSeconds: number } + | { + status: "ready"; + review: PersonaReviewRecord; + resolution: PersonaReviewResolutionRecord; + resolutions: PersonaReviewResolutionItem[]; + } + | { status: "cancelled" }; + type AgentLatestEventInput = { type: AgentLatestEventType; message: string; @@ -2490,32 +2518,85 @@ export class AgentManager { } } } - const result = await this.pool.query( - `UPDATE persona_reviews - SET status = 'complete', verdict = $2, summary = $3, - files_reviewed = $4::jsonb, message = $5, - last_reviewed_commit = COALESCE($6, last_reviewed_commit), - updated_at = NOW() - WHERE agent_id = $1 - RETURNING id, agent_id AS "agentId", parent_agent_id AS "parentAgentId", - persona, status, message, verdict, summary, - files_reviewed AS "filesReviewed", - last_reviewed_commit AS "lastReviewedCommit", - round_number AS "roundNumber", - allow_recheck AS "allowRecheck", - created_at AS "createdAt", updated_at AS "updatedAt"`, - [ - agentId, - input.verdict, - input.summary, - JSON.stringify(input.filesReviewed ?? []), - input.message ?? null, - input.lastReviewedCommit ?? null, - ] - ); - if (result.rowCount === 0) - throw new AgentError("No persona review found for agent.", 404); - return result.rows[0]!; + const client = await this.pool.connect(); + try { + await client.query("BEGIN"); + + const currentResult = await client.query( + `SELECT id, agent_id AS "agentId", parent_agent_id AS "parentAgentId", + persona, status, message, verdict, summary, + files_reviewed AS "filesReviewed", + last_reviewed_commit AS "lastReviewedCommit", + round_number AS "roundNumber", + allow_recheck AS "allowRecheck", + created_at AS "createdAt", updated_at AS "updatedAt" + FROM persona_reviews + WHERE agent_id = $1 + FOR UPDATE`, + [agentId] + ); + const current = currentResult.rows[0]; + if (!current) { + throw new AgentError("No persona review found for agent.", 404); + } + + let nextRoundNumber = current.roundNumber; + if (current.status === "reviewing") { + nextRoundNumber = 1; + } else if (current.status === "awaiting_recheck") { + if (current.roundNumber >= 2) { + throw new AgentError( + "Review has already completed round 2; further review completion is not allowed in v1.", + 409 + ); + } + nextRoundNumber = current.roundNumber + 1; + } else if (current.status === "complete" && current.roundNumber >= 2) { + throw new AgentError( + "Review has already completed round 2; a third completion is not allowed in v1.", + 409 + ); + } else { + throw new AgentError( + `Review can only be completed from 'reviewing' or 'awaiting_recheck' (current: ${current.status}).`, + 409 + ); + } + + const result = await client.query( + `UPDATE persona_reviews + SET status = 'complete', verdict = $2, summary = $3, + files_reviewed = $4::jsonb, message = $5, + last_reviewed_commit = COALESCE($6, last_reviewed_commit), + round_number = $7, + updated_at = NOW() + WHERE agent_id = $1 + RETURNING id, agent_id AS "agentId", parent_agent_id AS "parentAgentId", + persona, status, message, verdict, summary, + files_reviewed AS "filesReviewed", + last_reviewed_commit AS "lastReviewedCommit", + round_number AS "roundNumber", + allow_recheck AS "allowRecheck", + created_at AS "createdAt", updated_at AS "updatedAt"`, + [ + agentId, + input.verdict, + input.summary, + JSON.stringify(input.filesReviewed ?? []), + input.message ?? null, + input.lastReviewedCommit ?? null, + nextRoundNumber, + ] + ); + + await client.query("COMMIT"); + return result.rows[0]!; + } catch (err) { + await client.query("ROLLBACK"); + throw err; + } finally { + client.release(); + } } async getPersonaReview(agentId: string): Promise { @@ -3233,9 +3314,25 @@ export class AgentManager { agentId: string, feedback: FeedbackInput ): Promise { + const reviewResult = await this.pool.query<{ + status: string; + roundNumber: number; + }>( + `SELECT status, round_number AS "roundNumber" + FROM persona_reviews + WHERE agent_id = $1`, + [agentId] + ); + const review = reviewResult.rows[0]; + const feedbackRoundNumber = review + ? review.status === "awaiting_recheck" + ? review.roundNumber + 1 + : review.roundNumber + : 1; + const result = await this.pool.query( - `INSERT INTO agent_feedback (agent_id, severity, file_path, line_number, description, suggestion, media_ref) - VALUES ($1, $2, $3, $4, $5, $6, $7) + `INSERT INTO agent_feedback (agent_id, severity, file_path, line_number, description, suggestion, media_ref, round_number) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING id, agent_id AS "agentId", severity, file_path AS "filePath", line_number AS "lineNumber", description, suggestion, media_ref AS "mediaRef", status, resolution_reason AS "resolutionReason", @@ -3252,6 +3349,7 @@ export class AgentManager { feedback.description, feedback.suggestion ?? null, feedback.mediaRef ?? null, + feedbackRoundNumber, ] ); return result.rows[0]!; @@ -3489,6 +3587,18 @@ export class AgentManager { 404 ); } + if (review.status === "awaiting_recheck") { + throw new AgentError( + "Review is already awaiting recheck; dispatch_submit_resolution cannot be called again in that state.", + 409 + ); + } + if (review.status === "complete" && review.roundNumber >= 2) { + throw new AgentError( + "Review is already complete for round 2; no further resolution submission is allowed in v1.", + 409 + ); + } if (review.status !== "complete") { throw new AgentError( `Review must be in status 'complete' to submit a resolution (current: ${review.status}).`, @@ -3532,7 +3642,7 @@ export class AgentManager { ); } - const roundNumber = 1; + const roundNumber = review.roundNumber; const inserted = await client.query( `INSERT INTO persona_review_resolutions (review_id, round_number, summary, resolution_commit) @@ -3550,8 +3660,25 @@ export class AgentManager { [review.id, roundNumber, summary, input.resolutionCommit ?? null] ); + const updatedReviewResult = await client.query( + `UPDATE persona_reviews + SET status = 'awaiting_recheck', updated_at = NOW() + WHERE id = $1 + RETURNING id, agent_id AS "agentId", parent_agent_id AS "parentAgentId", + persona, status, message, verdict, summary, + files_reviewed AS "filesReviewed", + last_reviewed_commit AS "lastReviewedCommit", + round_number AS "roundNumber", + allow_recheck AS "allowRecheck", + created_at AS "createdAt", updated_at AS "updatedAt"`, + [review.id] + ); + await client.query("COMMIT"); - return { review, resolution: inserted.rows[0]! }; + return { + review: updatedReviewResult.rows[0]!, + resolution: inserted.rows[0]!, + }; } catch (err) { await client.query("ROLLBACK"); throw err; @@ -3575,6 +3702,190 @@ export class AgentManager { return result.rows; } + async cancelReviewRecheck(input: { + parentAgentId: string; + personaAgentId: string; + reason?: string | null; + }): Promise { + const client = await this.pool.connect(); + try { + await client.query("BEGIN"); + + const reviewResult = await client.query( + `SELECT id, agent_id AS "agentId", parent_agent_id AS "parentAgentId", + persona, status, message, verdict, summary, + files_reviewed AS "filesReviewed", + last_reviewed_commit AS "lastReviewedCommit", + round_number AS "roundNumber", + allow_recheck AS "allowRecheck", + created_at AS "createdAt", updated_at AS "updatedAt" + FROM persona_reviews + WHERE agent_id = $1 AND parent_agent_id = $2 + FOR UPDATE`, + [input.personaAgentId, input.parentAgentId] + ); + const review = reviewResult.rows[0]; + if (!review) { + throw new AgentError( + `No persona review found for agent ${input.personaAgentId} under parent ${input.parentAgentId}.`, + 404 + ); + } + if (review.status === "complete" && review.roundNumber >= 2) { + throw new AgentError( + "Cannot cancel recheck after round 2 is already complete.", + 409 + ); + } + if (review.status === "cancelled") { + await client.query("COMMIT"); + return review; + } + + const updatedResult = await client.query( + `UPDATE persona_reviews + SET status = 'cancelled', + message = COALESCE($2, message), + updated_at = NOW() + WHERE id = $1 + RETURNING id, agent_id AS "agentId", parent_agent_id AS "parentAgentId", + persona, status, message, verdict, summary, + files_reviewed AS "filesReviewed", + last_reviewed_commit AS "lastReviewedCommit", + round_number AS "roundNumber", + allow_recheck AS "allowRecheck", + created_at AS "createdAt", updated_at AS "updatedAt"`, + [review.id, input.reason ?? null] + ); + + await client.query("COMMIT"); + return updatedResult.rows[0]!; + } catch (err) { + await client.query("ROLLBACK"); + throw err; + } finally { + client.release(); + } + } + + async awaitReviewRecheck( + personaAgentId: string, + now = new Date() + ): Promise { + const client = await this.pool.connect(); + try { + await client.query("BEGIN"); + + const reviewResult = await client.query( + `SELECT id, agent_id AS "agentId", parent_agent_id AS "parentAgentId", + persona, status, message, verdict, summary, + files_reviewed AS "filesReviewed", + last_reviewed_commit AS "lastReviewedCommit", + round_number AS "roundNumber", + allow_recheck AS "allowRecheck", + created_at AS "createdAt", updated_at AS "updatedAt" + FROM persona_reviews + WHERE agent_id = $1 + FOR UPDATE`, + [personaAgentId] + ); + const review = reviewResult.rows[0]; + if (!review) { + throw new AgentError("No persona review found for agent.", 404); + } + if (!review.allowRecheck) { + throw new AgentError( + "dispatch_await_recheck is only available for reviews launched with allowRecheck: true.", + 409 + ); + } + if (review.status === "cancelled") { + await client.query("COMMIT"); + return { status: "cancelled" }; + } + if (review.status === "awaiting_recheck") { + const [resolutionResult, resolutionItemsResult] = await Promise.all([ + client.query( + `SELECT id, review_id AS "reviewId", round_number AS "roundNumber", + summary, resolution_commit AS "resolutionCommit", + submitted_at AS "submittedAt" + FROM persona_review_resolutions + WHERE review_id = $1 AND round_number = $2 + ORDER BY submitted_at DESC + LIMIT 1`, + [review.id, review.roundNumber] + ), + client.query( + `SELECT id AS "feedbackId", + description AS "originalDescription", + severity AS "originalSeverity", + status, + resolution_reason AS reason, + file_path AS "filePath", + line_number AS "lineNumber", + suggestion, + resolution_commit AS "resolutionCommit", + resolved_at AS "resolvedAt", + round_number AS "roundNumber" + FROM agent_feedback + WHERE agent_id = $1 AND round_number = $2 + ORDER BY id ASC`, + [personaAgentId, review.roundNumber] + ), + ]); + const resolution = resolutionResult.rows[0]; + if (!resolution) { + throw new AgentError( + `No resolution found for review ${review.id} round ${review.roundNumber}.`, + 409 + ); + } + await client.query("COMMIT"); + return { + status: "ready", + review, + resolution, + resolutions: resolutionItemsResult.rows, + }; + } + if (review.status !== "complete") { + throw new AgentError( + `Review must be complete before awaiting recheck (current: ${review.status}).`, + 409 + ); + } + if (review.roundNumber >= 2) { + await client.query("COMMIT"); + return { status: "cancelled" }; + } + + const pollCadence = pollCadenceSeconds(new Date(review.updatedAt), now); + if (pollCadence === RECHECK_POLL_TIMEOUT) { + await client.query( + `UPDATE persona_reviews + SET status = 'cancelled', + message = 'Recheck window expired.', + updated_at = NOW() + WHERE id = $1`, + [review.id] + ); + await client.query("COMMIT"); + return { status: "cancelled" }; + } + + await client.query("COMMIT"); + return { + status: "pending", + pollAgainInSeconds: pollCadence, + }; + } catch (err) { + await client.query("ROLLBACK"); + throw err; + } finally { + client.release(); + } + } + private baseAgentSelectSql(): string { return ` SELECT diff --git a/apps/server/src/reviews/poll-cadence.ts b/apps/server/src/reviews/poll-cadence.ts new file mode 100644 index 00000000..26f5e51e --- /dev/null +++ b/apps/server/src/reviews/poll-cadence.ts @@ -0,0 +1,20 @@ +export const RECHECK_POLL_TIMEOUT = "cancelled" as const; + +export function pollCadenceSeconds( + submittedAt: Date, + now: Date +): number | typeof RECHECK_POLL_TIMEOUT { + const elapsedMs = now.getTime() - submittedAt.getTime(); + const elapsedSeconds = elapsedMs / 1000; + + if (elapsedSeconds > 2 * 60 * 60) { + return RECHECK_POLL_TIMEOUT; + } + if (elapsedSeconds < 9 * 60) { + return 180; + } + if (elapsedSeconds < 24 * 60) { + return 300; + } + return 600; +} diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index b6aece46..88a53a07 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -1702,6 +1702,8 @@ async function registerRoutes() { getFeedback: mcpGetFeedback, resolveFeedback: mcpResolveFeedback, submitResolution: mcpSubmitResolution, + awaitRecheck: mcpAwaitRecheck, + cancelRecheck: mcpCancelRecheck, upsertPin: mcpUpsertPin, deletePin: mcpDeletePin, getParentContext: mcpGetParentContext, @@ -5309,6 +5311,61 @@ async function mcpSubmitResolution( return result; } +async function mcpAwaitRecheck( + agentId: string +): Promise { + const reviewer = await agentManager.getAgent(agentId); + if (!reviewer?.persona) { + throw new Error( + "dispatch_await_recheck is only available to reviewer agents." + ); + } + + const result = await agentManager.awaitReviewRecheck(agentId); + if (result.status !== "ready") { + return result; + } + + const diffSincePreviousRound = + reviewer.cwd && result.review.lastReviewedCommit + ? await diffSinceCommit(reviewer.cwd, result.review.lastReviewedCommit) + : ""; + + return { + status: "ready", + summary: result.resolution.summary, + resolutions: result.resolutions, + diffSincePreviousRound, + }; +} + +async function mcpCancelRecheck( + agentId: string, + input: { personaAgentId: string; reason?: string } +): Promise { + const review = await agentManager.cancelReviewRecheck({ + parentAgentId: agentId, + personaAgentId: input.personaAgentId, + reason: input.reason ?? null, + }); + const [child, parent] = await Promise.all([ + agentManager.getAgent(input.personaAgentId), + agentManager.getAgent(review.parentAgentId), + ]); + if (child) { + uiEventBroker.publish({ + type: "agent.upsert", + agent: withStreamFlag(child), + }); + } + if (parent) { + uiEventBroker.publish({ + type: "agent.upsert", + agent: withStreamFlag(parent), + }); + } +} + async function mcpUpsertPin( agentId: string, pin: { label: string; value: string; type: string } @@ -5482,6 +5539,7 @@ async function mcpLaunchPersona( persona: string; context: string; agentType?: (typeof AGENT_TYPES)[number]; + allowRecheck?: boolean; } ): Promise<{ agentId: string; persona: string; parentAgentId: string }> { const parent = await agentManager.getAgent(agentId); @@ -5573,6 +5631,7 @@ async function mcpLaunchPersona( parentAgentId: agentId, persona: opts.persona, lastReviewedCommit: launchCommit, + allowRecheck: opts.allowRecheck, }); // Re-fetch so the SSE event includes the review subquery data @@ -5607,6 +5666,21 @@ async function mcpLaunchPersona( return { agentId: agent.id, persona: opts.persona, parentAgentId: agentId }; } +async function diffSinceCommit( + cwd: string, + baseCommit: string +): Promise { + const result = await runCommand( + "git", + ["-C", cwd, "diff", `${baseCommit}...HEAD`], + { allowedExitCodes: [0, 128] } + ); + if (result.exitCode !== 0) { + return ""; + } + return result.stdout; +} + async function mcpShareMedia( agentId: string, opts: { diff --git a/apps/server/src/shared/mcp/server.ts b/apps/server/src/shared/mcp/server.ts index 90ec1683..57964549 100644 --- a/apps/server/src/shared/mcp/server.ts +++ b/apps/server/src/shared/mcp/server.ts @@ -42,9 +42,34 @@ export type FeedbackItem = { suggestion: string | null; mediaRef: string | null; status: string; + roundNumber: number; createdAt: string; }; +export type ReviewResolutionItem = { + feedbackId: number; + originalDescription: string; + originalSeverity: string; + status: string; + reason: string | null; + filePath: string | null; + lineNumber: number | null; + suggestion: string | null; + resolutionCommit: string | null; + resolvedAt: string | null; + roundNumber: number; +}; + +export type AwaitRecheckResponse = + | { status: "pending"; pollAgainInSeconds: number } + | { + status: "ready"; + summary: string; + resolutions: ReviewResolutionItem[]; + diffSincePreviousRound: string; + } + | { status: "cancelled" }; + export type PersonaFeedbackGroup = { persona: string; agentId: string; @@ -154,6 +179,7 @@ const AGENT_TOOLS = new Set([ "dispatch_get_feedback", "dispatch_resolve_feedback", "dispatch_submit_resolution", + "dispatch_cancel_recheck", "get_activity_summary", "get_agent_history", "get_feedback_summary", @@ -182,6 +208,8 @@ const JOB_TOOLS = new Set([ const PERSONA_TOOLS = new Set([ "review_status", + "dispatch_complete_review", + "dispatch_await_recheck", "dispatch_pin", "dispatch_share", "dispatch_feedback", @@ -281,6 +309,7 @@ export type McpRequestContext = { persona: string; context: string; agentType?: LaunchPersonaAgentType; + allowRecheck?: boolean; } ) => Promise<{ agentId: string; persona: string; parentAgentId: string }>; getFeedback?: ( @@ -326,6 +355,11 @@ export type McpRequestContext = { message?: string; } ) => Promise; + awaitRecheck?: (agentId: string) => Promise; + cancelRecheck?: ( + agentId: string, + input: { personaAgentId: string; reason?: string } + ) => Promise; getActivitySummary?: (params: { start: Date; end: Date; @@ -474,6 +508,103 @@ async function createDispatchMcpServer( ); } + if (allowed.has("dispatch_complete_review") && context.completeReview) { + const agentId = context.agent!.id; + const completeReview = context.completeReview; + + server.registerTool( + "dispatch_complete_review", + { + description: + "Reviewer-only. Complete the current review round with a verdict and summary. Valid from round 1 ('reviewing') and round 2 ('awaiting_recheck'). A third completion is rejected in v1.", + inputSchema: { + verdict: z + .enum(["approve", "request_changes"]) + .describe("Review verdict for this round."), + summary: z + .string() + .min(1) + .describe("Summary of the review findings for this round."), + filesReviewed: z + .array(z.string()) + .optional() + .describe("List of file paths reviewed in this round."), + message: z + .string() + .optional() + .describe("Optional short status note to store with the review."), + }, + }, + async (args) => { + try { + await completeReview(agentId, { + verdict: args.verdict, + summary: args.summary, + filesReviewed: args.filesReviewed, + message: args.message, + }); + return { + content: [ + { + type: "text", + text: `Review complete: ${args.verdict}. ${args.summary}`, + }, + ], + }; + } catch (error) { + return toToolError(error); + } + } + ); + } + + if (allowed.has("dispatch_await_recheck") && context.awaitRecheck) { + const agentId = context.agent!.id; + const awaitRecheck = context.awaitRecheck; + + server.registerTool( + "dispatch_await_recheck", + { + description: + "Reviewer-only. Call this after dispatch_complete_review when the review was launched with allowRecheck: true. Returns pending with pollAgainInSeconds, ready with the parent resolution summary and diff, or cancelled when no round 2 is expected.", + inputSchema: {}, + }, + async () => { + try { + const result = await awaitRecheck(agentId); + if (result.status === "pending") { + return { + content: [ + { + type: "text", + text: `Recheck pending. Poll again in ${result.pollAgainInSeconds} seconds.`, + }, + ], + structuredContent: result, + }; + } + if (result.status === "cancelled") { + return { + content: [{ type: "text", text: "Recheck cancelled." }], + structuredContent: result, + }; + } + return { + content: [ + { + type: "text", + text: "Recheck ready. Review the supplied resolutions and diff, then submit round 2.", + }, + ], + structuredContent: result, + }; + } catch (error) { + return toToolError(error); + } + } + ); + } + // ── get_parent_context (persona) ──────────────────────────────────── if ( allowed.has("get_parent_context") && @@ -522,6 +653,53 @@ async function createDispatchMcpServer( ); } + if ( + allowed.has("dispatch_cancel_recheck") && + context.agent && + context.cancelRecheck + ) { + const agentId = context.agent.id; + const cancelRecheck = context.cancelRecheck; + + server.registerTool( + "dispatch_cancel_recheck", + { + description: + "Parent-only. Cancel a pending recheck loop so the reviewer exits cleanly on its next poll. Rejected after round 2 is already complete.", + inputSchema: { + personaAgentId: z + .string() + .describe( + "The persona agent ID whose recheck should be cancelled." + ), + reason: z + .string() + .max(10_000) + .optional() + .describe("Optional reason surfaced to the reviewer."), + }, + }, + async (args) => { + try { + await cancelRecheck(agentId, { + personaAgentId: args.personaAgentId, + reason: args.reason, + }); + return { + content: [ + { + type: "text", + text: `Cancelled recheck for persona agent ${args.personaAgentId}.`, + }, + ], + }; + } catch (error) { + return toToolError(error); + } + } + ); + } + // ── create_pr ───────────────────────────────────────────────────── if (allowed.has("create_pr")) { const agentBaseBranch = context.agent?.baseBranch; @@ -877,6 +1055,12 @@ async function createDispatchMcpServer( .describe( "Optional agent runtime override for the persona launch." ), + allowRecheck: z + .boolean() + .default(false) + .describe( + "Whether the reviewer should stay alive for a single opt-in recheck pass after the parent submits resolutions." + ), }, }, async (args) => { @@ -885,12 +1069,16 @@ async function createDispatchMcpServer( persona: args.persona, context: args.context, agentType: args.agentType, + allowRecheck: args.allowRecheck, }); + const guidance = args.allowRecheck + ? " After resolving every finding, call dispatch_submit_resolution so the reviewer can perform its single recheck pass." + : ""; return { content: [ { type: "text", - text: `Launched persona "${result.persona}" as agent ${result.agentId}.`, + text: `Launched persona "${result.persona}" as agent ${result.agentId}.${guidance}`, }, ], }; diff --git a/apps/server/test/db/agent-manager.test.ts b/apps/server/test/db/agent-manager.test.ts index 3d94b7c8..ebaa9589 100644 --- a/apps/server/test/db/agent-manager.test.ts +++ b/apps/server/test/db/agent-manager.test.ts @@ -1650,6 +1650,26 @@ describe("AgentManager", () => { return { parent, child }; } + async function seedCompletedReviewWithRecheck(): Promise<{ + parent: Awaited>; + child: Awaited>; + }> { + const { parent, child } = await seedParentChild(); + await manager.createPersonaReview({ + agentId: child.id, + parentAgentId: parent.id, + persona: "security-review", + lastReviewedCommit: "launchsha", + allowRecheck: true, + }); + await manager.completePersonaReview(child.id, { + verdict: "approve", + summary: "All good", + lastReviewedCommit: "round1sha", + }); + return { parent, child }; + } + it("rejects an empty summary", async () => { const { parent, child } = await seedCompletedReview(); @@ -1763,7 +1783,7 @@ describe("AgentManager", () => { }); it("persists summary and resolution_commit on the happy path", async () => { - const { parent, child } = await seedCompletedReview(); + const { parent, child } = await seedCompletedReviewWithRecheck(); const item = await manager.submitFeedback(child.id, { description: "a thing", }); @@ -1781,6 +1801,7 @@ describe("AgentManager", () => { expect(result.resolution.summary).toBe("Accepted one, rejected one."); expect(result.resolution.resolutionCommit).toBe("headsha1"); expect(result.resolution.roundNumber).toBe(1); + expect(result.review.status).toBe("awaiting_recheck"); // Confirm via a separate read path so the test also covers read APIs. const resolutions = await manager.getReviewResolutions( @@ -1791,29 +1812,24 @@ describe("AgentManager", () => { expect(resolutions[0].resolutionCommit).toBe("headsha1"); }); - it("upserts on repeat submit, replacing summary + commit", async () => { - const { parent, child } = await seedCompletedReview(); + it("rejects repeat submit once the review is awaiting_recheck", async () => { + const { parent, child } = await seedCompletedReviewWithRecheck(); - const first = await manager.submitReviewResolution({ + await manager.submitReviewResolution({ parentAgentId: parent.id, personaAgentId: child.id, summary: "v1", resolutionCommit: "sha-v1", }); - const second = await manager.submitReviewResolution({ - parentAgentId: parent.id, - personaAgentId: child.id, - summary: "v2 — revised", - resolutionCommit: "sha-v2", - }); - expect(second.resolution.id).toBe(first.resolution.id); - const resolutions = await manager.getReviewResolutions( - second.review.id - ); - expect(resolutions).toHaveLength(1); - expect(resolutions[0].summary).toBe("v2 — revised"); - expect(resolutions[0].resolutionCommit).toBe("sha-v2"); + await expect( + manager.submitReviewResolution({ + parentAgentId: parent.id, + personaAgentId: child.id, + summary: "v2 — revised", + resolutionCommit: "sha-v2", + }) + ).rejects.toThrow(/already awaiting recheck/); }); it("trims leading/trailing whitespace from the stored summary", async () => { @@ -1910,6 +1926,200 @@ describe("AgentManager", () => { expect(completed.lastReviewedCommit).toBe("launchsha"); }); }); + + describe("round-trip review state machine", () => { + async function seedCompletedReview(): Promise<{ + parent: Awaited>; + child: Awaited>; + }> { + const { parent, child } = await seedParentChild(); + await manager.createPersonaReview({ + agentId: child.id, + parentAgentId: parent.id, + persona: "security-review", + lastReviewedCommit: "launchsha", + }); + await manager.completePersonaReview(child.id, { + verdict: "approve", + summary: "All good", + }); + return { parent, child }; + } + + async function seedCompletedReviewWithRecheck(): Promise<{ + parent: Awaited>; + child: Awaited>; + }> { + const { parent, child } = await seedParentChild(); + await manager.createPersonaReview({ + agentId: child.id, + parentAgentId: parent.id, + persona: "security-review", + lastReviewedCommit: "launchsha", + allowRecheck: true, + }); + await manager.completePersonaReview(child.id, { + verdict: "approve", + summary: "All good", + lastReviewedCommit: "round1sha", + }); + return { parent, child }; + } + + async function seedAwaitingRecheckReview() { + const { parent, child } = await seedCompletedReviewWithRecheck(); + const original = await manager.submitFeedback(child.id, { + description: "round 1 finding", + severity: "high", + filePath: "apps/server/src/server.ts", + lineNumber: 42, + }); + await manager.updateFeedbackStatus(original.id, child.id, "fixed", { + reason: "patched", + resolutionCommit: "fixsha", + }); + await manager.submitReviewResolution({ + parentAgentId: parent.id, + personaAgentId: child.id, + summary: "Patched the finding.", + resolutionCommit: "parentsha", + }); + return { parent, child, original }; + } + + it("allows round 2 completion from awaiting_recheck and increments round_number", async () => { + const { child } = await seedAwaitingRecheckReview(); + + const completed = await manager.completePersonaReview(child.id, { + verdict: "approve", + summary: "Round 2 complete", + lastReviewedCommit: "round2sha", + }); + + expect(completed.status).toBe("complete"); + expect(completed.roundNumber).toBe(2); + expect(completed.lastReviewedCommit).toBe("round2sha"); + }); + + it("rejects a third completion attempt in v1", async () => { + const { child } = await seedAwaitingRecheckReview(); + await manager.completePersonaReview(child.id, { + verdict: "approve", + summary: "Round 2 complete", + }); + + await expect( + manager.completePersonaReview(child.id, { + verdict: "approve", + summary: "Round 3", + }) + ).rejects.toThrow(/third completion is not allowed/i); + }); + + it("returns pending cadence while the reviewer waits for resolution", async () => { + const { child } = await seedCompletedReviewWithRecheck(); + const review = await manager.getPersonaReview(child.id); + const eightMinutesFiftyNineSecondsLater = new Date( + new Date(review!.updatedAt).getTime() + (8 * 60 + 59) * 1000 + ); + + await expect( + manager.awaitReviewRecheck( + child.id, + eightMinutesFiftyNineSecondsLater + ) + ).resolves.toEqual({ + status: "pending", + pollAgainInSeconds: 180, + }); + }); + + it("returns ready with the stored resolution payload after submitResolution", async () => { + const { child, original } = await seedAwaitingRecheckReview(); + + const result = await manager.awaitReviewRecheck(child.id); + + expect(result.status).toBe("ready"); + if (result.status !== "ready") { + throw new Error("expected ready"); + } + expect(result.resolution.summary).toBe("Patched the finding."); + expect(result.resolutions).toEqual([ + expect.objectContaining({ + feedbackId: original.id, + originalDescription: "round 1 finding", + originalSeverity: "high", + status: "fixed", + reason: "patched", + filePath: "apps/server/src/server.ts", + lineNumber: 42, + resolutionCommit: "fixsha", + roundNumber: 1, + }), + ]); + }); + + it("times out awaitReviewRecheck after two hours and cancels the review", async () => { + const { child } = await seedCompletedReviewWithRecheck(); + const review = await manager.getPersonaReview(child.id); + const twoHoursAndOneSecondLater = new Date( + new Date(review!.updatedAt).getTime() + (2 * 60 * 60 + 1) * 1000 + ); + + const result = await manager.awaitReviewRecheck( + child.id, + twoHoursAndOneSecondLater + ); + + expect(result).toEqual({ status: "cancelled" }); + const cancelledReview = await manager.getPersonaReview(child.id); + expect(cancelledReview!.status).toBe("cancelled"); + }); + + it("rejects awaitReviewRecheck when allowRecheck is false", async () => { + const { child } = await seedCompletedReview(); + + await expect(manager.awaitReviewRecheck(child.id)).rejects.toThrow( + /allowRecheck: true/ + ); + }); + + it("cancels recheck from the parent and rejects cancelling after round 2 completes", async () => { + const { parent, child } = await seedAwaitingRecheckReview(); + + const cancelled = await manager.cancelReviewRecheck({ + parentAgentId: parent.id, + personaAgentId: child.id, + reason: "shipping without recheck", + }); + expect(cancelled.status).toBe("cancelled"); + expect(cancelled.message).toBe("shipping without recheck"); + + const { parent: parent2, child: child2 } = + await seedAwaitingRecheckReview(); + await manager.completePersonaReview(child2.id, { + verdict: "approve", + summary: "Round 2 complete", + }); + + await expect( + manager.cancelReviewRecheck({ + parentAgentId: parent2.id, + personaAgentId: child2.id, + }) + ).rejects.toThrow(/after round 2 is already complete/); + }); + + it("records round 2 findings with round_number = 2", async () => { + const { child } = await seedAwaitingRecheckReview(); + + const round2Feedback = await manager.submitFeedback(child.id, { + description: "round 2 follow-up", + }); + + expect(round2Feedback.roundNumber).toBe(2); + }); + }); }); describe("listRecentPersonaReviews", () => { diff --git a/apps/server/test/poll-cadence.test.ts b/apps/server/test/poll-cadence.test.ts new file mode 100644 index 00000000..12a0b61c --- /dev/null +++ b/apps/server/test/poll-cadence.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest"; + +import { + RECHECK_POLL_TIMEOUT, + pollCadenceSeconds, +} from "../src/reviews/poll-cadence.js"; + +describe("pollCadenceSeconds", () => { + const submittedAt = new Date("2026-04-22T00:00:00.000Z"); + + it("returns 180 seconds before 9 minutes", () => { + expect( + pollCadenceSeconds(submittedAt, new Date("2026-04-22T00:08:59.000Z")) + ).toBe(180); + }); + + it("returns 300 seconds at 9 minutes", () => { + expect( + pollCadenceSeconds(submittedAt, new Date("2026-04-22T00:09:00.000Z")) + ).toBe(300); + }); + + it("returns 300 seconds before 24 minutes", () => { + expect( + pollCadenceSeconds(submittedAt, new Date("2026-04-22T00:23:59.000Z")) + ).toBe(300); + }); + + it("returns 600 seconds at 24 minutes", () => { + expect( + pollCadenceSeconds(submittedAt, new Date("2026-04-22T00:24:00.000Z")) + ).toBe(600); + }); + + it("returns 600 seconds at exactly two hours", () => { + expect( + pollCadenceSeconds(submittedAt, new Date("2026-04-22T02:00:00.000Z")) + ).toBe(600); + }); + + it("returns the timeout marker after two hours", () => { + expect( + pollCadenceSeconds(submittedAt, new Date("2026-04-22T02:00:01.000Z")) + ).toBe(RECHECK_POLL_TIMEOUT); + }); +}); From 0ace5e7e768fa4bed4fd97b095ea0f277884f46e Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Wed, 22 Apr 2026 14:09:48 -0600 Subject: [PATCH 2/3] Fix recheck resolution edge cases --- apps/server/src/agents/manager.ts | 5 +++-- apps/server/src/personas/loader.ts | 24 +++++++++++++---------- apps/server/src/server.ts | 3 ++- apps/server/test/db/agent-manager.test.ts | 14 +++++++++++++ apps/server/test/poll-cadence.test.ts | 14 +++++++++++++ 5 files changed, 47 insertions(+), 13 deletions(-) diff --git a/apps/server/src/agents/manager.ts b/apps/server/src/agents/manager.ts index 85e0a181..1c6ec10d 100644 --- a/apps/server/src/agents/manager.ts +++ b/apps/server/src/agents/manager.ts @@ -3660,9 +3660,10 @@ export class AgentManager { [review.id, roundNumber, summary, input.resolutionCommit ?? null] ); + const nextStatus = review.allowRecheck ? "awaiting_recheck" : "complete"; const updatedReviewResult = await client.query( `UPDATE persona_reviews - SET status = 'awaiting_recheck', updated_at = NOW() + SET status = $2, updated_at = NOW() WHERE id = $1 RETURNING id, agent_id AS "agentId", parent_agent_id AS "parentAgentId", persona, status, message, verdict, summary, @@ -3671,7 +3672,7 @@ export class AgentManager { round_number AS "roundNumber", allow_recheck AS "allowRecheck", created_at AS "createdAt", updated_at AS "updatedAt"`, - [review.id] + [review.id, nextStatus] ); await client.query("COMMIT"); diff --git a/apps/server/src/personas/loader.ts b/apps/server/src/personas/loader.ts index 1404b341..8d0b3c62 100644 --- a/apps/server/src/personas/loader.ts +++ b/apps/server/src/personas/loader.ts @@ -21,7 +21,19 @@ type PersonaFrontmatter = { }; const PERSONAS_DIR = ".dispatch/personas"; -const MAX_DIFF_BYTES = 50 * 1024; +export const MAX_DIFF_BYTES = 50 * 1024; + +export function truncateDiffForPrompt(diff: string): string { + if (Buffer.byteLength(diff, "utf-8") <= MAX_DIFF_BYTES) { + return diff; + } + + const decoder = new TextDecoder("utf-8", { fatal: false }); + return ( + decoder.decode(Buffer.from(diff, "utf-8").subarray(0, MAX_DIFF_BYTES)) + + "\n\n[... diff truncated at 50KB ...]" + ); +} export function parseFrontmatter(content: string): { frontmatter: PersonaFrontmatter; @@ -144,14 +156,6 @@ export function assemblePersonaPrompt( context: string, diff: string ): string { - let truncatedDiff = diff; - if (Buffer.byteLength(diff, "utf-8") > MAX_DIFF_BYTES) { - const decoder = new TextDecoder("utf-8", { fatal: false }); - truncatedDiff = - decoder.decode(Buffer.from(diff, "utf-8").subarray(0, MAX_DIFF_BYTES)) + - "\n\n[... diff truncated at 50KB ...]"; - } - // Strip legacy {{context}} and {{diff}} placeholders if present — Dispatch // now appends these sections automatically so persona files don't need them. const personaBody = persona.body @@ -162,6 +166,6 @@ export function assemblePersonaPrompt( personaBody.trimEnd(), STANDARD_FEEDBACK_GUIDANCE, `## Context from parent agent\n${context}`, - `## Changes to review\n${truncatedDiff}`, + `## Changes to review\n${truncateDiffForPrompt(diff)}`, ].join("\n\n"); } diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 88a53a07..0a186b21 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -56,6 +56,7 @@ import { assemblePersonaPrompt, } from "./personas/loader.js"; import { buildPersonaReviewDiff } from "./personas/review-diff.js"; +import { truncateDiffForPrompt } from "./personas/loader.js"; import { isPasswordSet, setPassword, @@ -5335,7 +5336,7 @@ async function mcpAwaitRecheck( status: "ready", summary: result.resolution.summary, resolutions: result.resolutions, - diffSincePreviousRound, + diffSincePreviousRound: truncateDiffForPrompt(diffSincePreviousRound), }; } diff --git a/apps/server/test/db/agent-manager.test.ts b/apps/server/test/db/agent-manager.test.ts index ebaa9589..48eb41ed 100644 --- a/apps/server/test/db/agent-manager.test.ts +++ b/apps/server/test/db/agent-manager.test.ts @@ -1812,6 +1812,20 @@ describe("AgentManager", () => { expect(resolutions[0].resolutionCommit).toBe("headsha1"); }); + it("keeps non-recheck reviews in complete after resolution submission", async () => { + const { parent, child } = await seedCompletedReview(); + + const result = await manager.submitReviewResolution({ + parentAgentId: parent.id, + personaAgentId: child.id, + summary: "Recorded the resolution without a recheck.", + resolutionCommit: "headsha1", + }); + + expect(result.review.status).toBe("complete"); + expect(result.review.allowRecheck).toBe(false); + }); + it("rejects repeat submit once the review is awaiting_recheck", async () => { const { parent, child } = await seedCompletedReviewWithRecheck(); diff --git a/apps/server/test/poll-cadence.test.ts b/apps/server/test/poll-cadence.test.ts index 12a0b61c..095ad514 100644 --- a/apps/server/test/poll-cadence.test.ts +++ b/apps/server/test/poll-cadence.test.ts @@ -4,6 +4,10 @@ import { RECHECK_POLL_TIMEOUT, pollCadenceSeconds, } from "../src/reviews/poll-cadence.js"; +import { + MAX_DIFF_BYTES, + truncateDiffForPrompt, +} from "../src/personas/loader.js"; describe("pollCadenceSeconds", () => { const submittedAt = new Date("2026-04-22T00:00:00.000Z"); @@ -43,4 +47,14 @@ describe("pollCadenceSeconds", () => { pollCadenceSeconds(submittedAt, new Date("2026-04-22T02:00:01.000Z")) ).toBe(RECHECK_POLL_TIMEOUT); }); + + it("truncates oversized recheck diffs to the shared 50KB limit", () => { + const largeDiff = "a".repeat(MAX_DIFF_BYTES + 1024); + const truncated = truncateDiffForPrompt(largeDiff); + + expect(Buffer.byteLength(truncated, "utf-8")).toBeGreaterThan( + MAX_DIFF_BYTES + ); + expect(truncated).toContain("[... diff truncated at 50KB ...]"); + }); }); From 7afa1e868a7a823daa10bf59ea941bb60030e44a Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Wed, 22 Apr 2026 16:19:48 -0600 Subject: [PATCH 3/3] Tighten recheck tool contracts --- apps/server/src/agents/manager.ts | 101 +++++++++++------- apps/server/src/server.ts | 13 ++- apps/server/src/shared/mcp/server.ts | 16 ++- apps/server/test/db/agent-manager.test.ts | 30 ++++++ apps/server/test/mcp-auth-integration.test.ts | 49 +++++++++ 5 files changed, 167 insertions(+), 42 deletions(-) diff --git a/apps/server/src/agents/manager.ts b/apps/server/src/agents/manager.ts index 1c6ec10d..84beed86 100644 --- a/apps/server/src/agents/manager.ts +++ b/apps/server/src/agents/manager.ts @@ -252,6 +252,7 @@ export type AwaitRecheckResult = resolution: PersonaReviewResolutionRecord; resolutions: PersonaReviewResolutionItem[]; } + | { status: "complete" } | { status: "cancelled" }; type AgentLatestEventInput = { @@ -3314,45 +3315,58 @@ export class AgentManager { agentId: string, feedback: FeedbackInput ): Promise { - const reviewResult = await this.pool.query<{ - status: string; - roundNumber: number; - }>( - `SELECT status, round_number AS "roundNumber" - FROM persona_reviews - WHERE agent_id = $1`, - [agentId] - ); - const review = reviewResult.rows[0]; - const feedbackRoundNumber = review - ? review.status === "awaiting_recheck" - ? review.roundNumber + 1 - : review.roundNumber - : 1; + const client = await this.pool.connect(); + try { + await client.query("BEGIN"); - const result = await this.pool.query( - `INSERT INTO agent_feedback (agent_id, severity, file_path, line_number, description, suggestion, media_ref, round_number) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) - RETURNING id, agent_id AS "agentId", severity, file_path AS "filePath", line_number AS "lineNumber", - description, suggestion, media_ref AS "mediaRef", status, - resolution_reason AS "resolutionReason", - resolution_commit AS "resolutionCommit", - resolved_at AS "resolvedAt", - round_number AS "roundNumber", - responds_to_feedback_id AS "respondsToFeedbackId", - created_at AS "createdAt"`, - [ - agentId, - feedback.severity ?? "info", - feedback.filePath ?? null, - feedback.lineNumber ?? null, - feedback.description, - feedback.suggestion ?? null, - feedback.mediaRef ?? null, - feedbackRoundNumber, - ] - ); - return result.rows[0]!; + const reviewResult = await client.query<{ + status: string; + roundNumber: number; + }>( + `SELECT status, round_number AS "roundNumber" + FROM persona_reviews + WHERE agent_id = $1 + FOR UPDATE`, + [agentId] + ); + const review = reviewResult.rows[0]; + const feedbackRoundNumber = review + ? review.status === "awaiting_recheck" + ? review.roundNumber + 1 + : review.roundNumber + : 1; + + const result = await client.query( + `INSERT INTO agent_feedback (agent_id, severity, file_path, line_number, description, suggestion, media_ref, round_number) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + RETURNING id, agent_id AS "agentId", severity, file_path AS "filePath", line_number AS "lineNumber", + description, suggestion, media_ref AS "mediaRef", status, + resolution_reason AS "resolutionReason", + resolution_commit AS "resolutionCommit", + resolved_at AS "resolvedAt", + round_number AS "roundNumber", + responds_to_feedback_id AS "respondsToFeedbackId", + created_at AS "createdAt"`, + [ + agentId, + feedback.severity ?? "info", + feedback.filePath ?? null, + feedback.lineNumber ?? null, + feedback.description, + feedback.suggestion ?? null, + feedback.mediaRef ?? null, + feedbackRoundNumber, + ] + ); + + await client.query("COMMIT"); + return result.rows[0]!; + } catch (err) { + await client.query("ROLLBACK"); + throw err; + } finally { + client.release(); + } } async listFeedback(agentId: string): Promise { @@ -3742,6 +3756,15 @@ export class AgentManager { await client.query("COMMIT"); return review; } + if ( + review.status !== "awaiting_recheck" && + !(review.status === "complete" && review.allowRecheck) + ) { + throw new AgentError( + `Recheck can only be cancelled while awaiting round 2 or while the reviewer is polling after round 1 (current: ${review.status}).`, + 409 + ); + } const updatedResult = await client.query( `UPDATE persona_reviews @@ -3857,7 +3880,7 @@ export class AgentManager { } if (review.roundNumber >= 2) { await client.query("COMMIT"); - return { status: "cancelled" }; + return { status: "complete" }; } const pollCadence = pollCadenceSeconds(new Date(review.updatedAt), now); diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 0a186b21..555beff9 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -1614,7 +1614,14 @@ async function registerRoutes() { reply.hijack(); await handleMcpRequest(request.raw, reply.raw, request.body, { - agent, + agent: { + id: agent.id, + cwd: agent.cwd, + persona: agent.persona, + parentAgentId: agent.parentAgentId, + baseBranch: agent.baseBranch, + review: null, + }, repoRoot, worktreeRoot, sendNotify: mcpSendNotify, @@ -1665,6 +1672,9 @@ async function registerRoutes() { if (!agent) { return reply.code(404).send({ error: "Agent not found." }); } + const review = agent.persona + ? await agentManager.getPersonaReview(agentId) + : null; const activeJobRun = await jobService.getActiveRunForAgent(agentId); if (activeJobRun) { return reply @@ -1689,6 +1699,7 @@ async function registerRoutes() { persona: agent.persona, parentAgentId: agent.parentAgentId, baseBranch: agent.baseBranch, + review: review ? { allowRecheck: review.allowRecheck } : null, }, repoRoot, worktreeRoot, diff --git a/apps/server/src/shared/mcp/server.ts b/apps/server/src/shared/mcp/server.ts index 57964549..5d14906c 100644 --- a/apps/server/src/shared/mcp/server.ts +++ b/apps/server/src/shared/mcp/server.ts @@ -14,6 +14,9 @@ export type McpAgent = { persona?: string | null; parentAgentId?: string | null; baseBranch?: string | null; + review?: { + allowRecheck?: boolean; + } | null; }; export type MediaResult = { @@ -68,6 +71,7 @@ export type AwaitRecheckResponse = resolutions: ReviewResolutionItem[]; diffSincePreviousRound: string; } + | { status: "complete" } | { status: "cancelled" }; export type PersonaFeedbackGroup = { @@ -209,7 +213,6 @@ const JOB_TOOLS = new Set([ const PERSONA_TOOLS = new Set([ "review_status", "dispatch_complete_review", - "dispatch_await_recheck", "dispatch_pin", "dispatch_share", "dispatch_feedback", @@ -422,7 +425,10 @@ async function createDispatchMcpServer( : context.jobTools ? "job" : "agent"; - const allowed = TOOL_SETS[agentType]; + const allowed = new Set(TOOL_SETS[agentType]); + if (context.agent?.persona && context.agent.review?.allowRecheck) { + allowed.add("dispatch_await_recheck"); + } // ── review_status (persona) ─────────────────────────────────────── if ( @@ -589,6 +595,12 @@ async function createDispatchMcpServer( structuredContent: result, }; } + if (result.status === "complete") { + return { + content: [{ type: "text", text: "Recheck complete." }], + structuredContent: result, + }; + } return { content: [ { diff --git a/apps/server/test/db/agent-manager.test.ts b/apps/server/test/db/agent-manager.test.ts index 48eb41ed..a8829092 100644 --- a/apps/server/test/db/agent-manager.test.ts +++ b/apps/server/test/db/agent-manager.test.ts @@ -2124,6 +2124,24 @@ describe("AgentManager", () => { ).rejects.toThrow(/after round 2 is already complete/); }); + it("rejects cancelling while round 1 review is still in progress", async () => { + const { parent, child } = await seedParentChild(); + await manager.createPersonaReview({ + agentId: child.id, + parentAgentId: parent.id, + persona: "security-review", + lastReviewedCommit: "launchsha", + allowRecheck: true, + }); + + await expect( + manager.cancelReviewRecheck({ + parentAgentId: parent.id, + personaAgentId: child.id, + }) + ).rejects.toThrow(/can only be cancelled while awaiting round 2/i); + }); + it("records round 2 findings with round_number = 2", async () => { const { child } = await seedAwaitingRecheckReview(); @@ -2133,6 +2151,18 @@ describe("AgentManager", () => { expect(round2Feedback.roundNumber).toBe(2); }); + + it("returns complete once round 2 has already been submitted", async () => { + const { child } = await seedAwaitingRecheckReview(); + await manager.completePersonaReview(child.id, { + verdict: "approve", + summary: "Round 2 complete", + }); + + await expect(manager.awaitReviewRecheck(child.id)).resolves.toEqual({ + status: "complete", + }); + }); }); }); diff --git a/apps/server/test/mcp-auth-integration.test.ts b/apps/server/test/mcp-auth-integration.test.ts index fbbdaf4a..229e56b9 100644 --- a/apps/server/test/mcp-auth-integration.test.ts +++ b/apps/server/test/mcp-auth-integration.test.ts @@ -184,6 +184,55 @@ describe("MCP auth integration", () => { expect(jobResponse.json()).toEqual({ error: "Agent not found." }); }); + it("only exposes dispatch_await_recheck to reviewers launched with allowRecheck", async () => { + await pool.query( + `INSERT INTO agents (id, name, type, status, cwd, persona, parent_agent_id, full_access) + VALUES + ('agt_parentreview', 'parent', 'codex', 'running', '/tmp', null, null, false), + ('agt_persona_plain', 'plain-reviewer', 'codex', 'running', '/tmp', 'backend-security-review', 'agt_parentreview', false), + ('agt_persona_recheck', 'recheck-reviewer', 'codex', 'running', '/tmp', 'backend-security-review', 'agt_parentreview', false)` + ); + await pool.query( + `INSERT INTO persona_reviews ( + agent_id, parent_agent_id, persona, status, round_number, allow_recheck + ) + VALUES + ('agt_persona_plain', 'agt_parentreview', 'backend-security-review', 'reviewing', 1, false), + ('agt_persona_recheck', 'agt_parentreview', 'backend-security-review', 'reviewing', 1, true)` + ); + + const authTokenResult = await pool.query<{ value: string }>( + "SELECT value FROM settings WHERE key = 'auth_token'" + ); + const authToken = authTokenResult.rows[0]!.value; + + const plainResponse = await app.inject({ + method: "POST", + url: "/api/mcp/agt_persona_plain", + headers: { + authorization: `Bearer ${createAgentMcpToken(authToken, "agt_persona_plain")}`, + accept: "application/json, text/event-stream", + "content-type": "application/json", + }, + payload: { jsonrpc: "2.0", id: 1, method: "tools/list", params: {} }, + }); + expect(plainResponse.statusCode).toBe(200); + expect(plainResponse.body).not.toContain("dispatch_await_recheck"); + + const recheckResponse = await app.inject({ + method: "POST", + url: "/api/mcp/agt_persona_recheck", + headers: { + authorization: `Bearer ${createAgentMcpToken(authToken, "agt_persona_recheck")}`, + accept: "application/json, text/event-stream", + "content-type": "application/json", + }, + payload: { jsonrpc: "2.0", id: 1, method: "tools/list", params: {} }, + }); + expect(recheckResponse.statusCode).toBe(200); + expect(recheckResponse.body).toContain("dispatch_await_recheck"); + }); + it("exposes dispatch_event and dispatch_rename_session on the job-scoped MCP route", async () => { await pool.query( `INSERT INTO agents (id, name, type, status, cwd, full_access)