Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ Every agent launched by Dispatch gets access to MCP tools via an agent-scoped en

### Persona agents

Persona review agents get a narrower set focused on reviewing their parent's work: `dispatch_review_submit`, `dispatch_review_add_feedback`, `dispatch_review_list_feedback`, `dispatch_review_add_message`, `dispatch_event`, `dispatch_pin`, `dispatch_share`, and `get_parent_context`.
Persona review agents get a narrower set focused on reviewing their parent's work: `dispatch_review_submit`, `dispatch_review_add_feedback`, `dispatch_review_list_feedback`, `dispatch_review_add_message`, `dispatch_review_resolve`, `dispatch_event`, `dispatch_pin`, `dispatch_share`, and `get_parent_context`. After the parent reports a fix in the feedback thread, the reviewer re-inspects it and either resolves the item or replies with further instructions.

### Job agents

Expand Down
52 changes: 49 additions & 3 deletions apps/server/src/agents/reviews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ export type ReviewFeedbackItemRecord = {
updatedAt: string;
};

export type ReviewFeedbackResolverRole = "reviewer" | "assignee" | "human";

export class ReviewFeedbackResolutionConflictError extends Error {
constructor(public readonly item: ReviewFeedbackItemRecord) {
const resolution = item.resolution ? ` (${item.resolution})` : "";
super(
`Review feedback item #${item.id} is already ${item.status}${resolution}; refresh the review before acting.`
);
this.name = "ReviewFeedbackResolutionConflictError";
}
}

export type ReviewThreadMessageRecord = {
id: number;
feedbackItemId: number;
Expand Down Expand Up @@ -363,7 +375,8 @@ export async function resolveReviewFeedbackItem(
note?: string | null;
resolvedBy?: string | null;
authorType?: "human" | "agent";
} = {}
resolverRole: ReviewFeedbackResolverRole;
}
): Promise<{
item: ReviewFeedbackItemRecord;
reviewId: number;
Expand All @@ -381,7 +394,12 @@ export async function resolveReviewFeedbackItem(
resolved_by = $3, resolved_at = NOW(), updated_at = NOW()
FROM reviews r
WHERE fi.id = $4 AND fi.review_id = r.id
AND (r.agent_id = $5 OR r.assigned_agent_id = $5)
AND fi.status = 'open'
AND (
(($6 = 'human' OR $6 = 'assignee')
AND (r.agent_id = $5 OR r.assigned_agent_id = $5))
OR ($6 = 'reviewer' AND r.reviewer_type = 'agent' AND r.reviewer_agent_id = $5)
)
RETURNING
fi.id, fi.review_id AS "reviewId", fi.file_path AS "filePath",
fi.line_start AS "lineStart", fi.line_end AS "lineEnd",
Expand All @@ -390,10 +408,38 @@ export async function resolveReviewFeedbackItem(
fi.resolved_by AS "resolvedBy", fi.resolved_at AS "resolvedAt",
fi.created_at AS "createdAt", fi.updated_at AS "updatedAt",
r.agent_id AS "agentId"`,
[resolution, opts.note ?? null, opts.resolvedBy ?? null, itemId, agentId]
[
resolution,
opts.note ?? null,
opts.resolvedBy ?? null,
itemId,
agentId,
opts.resolverRole,
]
);
const item = itemResult.rows[0];
if (!item) {
const currentResult = await client.query<ReviewFeedbackItemRecord>(
`SELECT fi.id, fi.review_id AS "reviewId", fi.file_path AS "filePath",
fi.line_start AS "lineStart", fi.line_end AS "lineEnd",
fi.diff_snapshot AS "diffSnapshot", fi.base_ref AS "baseRef",
fi.status, fi.resolution, fi.resolution_note AS "resolutionNote",
fi.resolved_by AS "resolvedBy", fi.resolved_at AS "resolvedAt",
fi.created_at AS "createdAt", fi.updated_at AS "updatedAt"
FROM review_feedback_items fi
JOIN reviews r ON r.id = fi.review_id
WHERE fi.id = $1
AND (
(($3 = 'human' OR $3 = 'assignee')
AND (r.agent_id = $2 OR r.assigned_agent_id = $2))
OR ($3 = 'reviewer' AND r.reviewer_type = 'agent' AND r.reviewer_agent_id = $2)
)`,
[itemId, agentId, opts.resolverRole]
);
const currentItem = currentResult.rows[0];
if (currentItem && currentItem.status !== "open") {
throw new ReviewFeedbackResolutionConflictError(currentItem);
}
await client.query("ROLLBACK");
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/agents/tmux/command-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ export function buildLaunchGuidance(
);
if (autoReview) {
rules.push(
"Autonomous Review is enabled. Before emitting done: commit and push your branch, open a draft PR via create_pr (don't override baseBranch — it defaults correctly), call list_personas, then launch 1 relevant reviewer via dispatch_launch_persona. Wait for the structured REVIEW SUBMITTED prompt instead of polling. If feedback exists, call dispatch_review_list_feedback with the supplied review ID, keep all discussion in item threads via dispatch_review_add_message, and set each outcome with dispatch_review_resolve. Use dispatch_review_reopen if more work is needed. A clean zero-item approval requires no action. Don't emit done until all submitted reviews are resolved."
"Autonomous Review is enabled. Before emitting done: commit and push your branch, open a draft PR via create_pr (don't override baseBranch — it defaults correctly), call list_personas, then launch 1 relevant reviewer via dispatch_launch_persona. After launch, do not poll, sleep, call list_agents, or schedule a wakeup; end the turn and let Dispatch inject the structured REVIEW SUBMITTED prompt when ready. If feedback exists, call dispatch_review_list_feedback with the supplied review ID and keep all discussion in item threads via dispatch_review_add_message. After fixing an item, ask the reviewer to verify it instead of resolving it yourself. The reviewer will resolve verified fixes or reply with further instructions. A clean zero-item approval requires no action. Don't emit done until all submitted reviews are resolved."
);
}
}
Expand Down
11 changes: 9 additions & 2 deletions apps/server/src/reviews/injection-prompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ export function buildReviewSubmittedPrompt(input: {
lines.push(`- #${item.id} ${location} — ${item.body}`);
}
lines.push(
"Call dispatch_review_list_feedback with this reviewId before acting. Use dispatch_review_add_message for questions or explanations, dispatch_review_resolve when an item is fixed or dismissed, and dispatch_review_reopen if a resolved item needs more work. Keep all review discussion in its feedback-item thread."
input.reviewerAgentId
? "Call dispatch_review_list_feedback with this reviewId before acting. Use dispatch_review_add_message for questions and explanations. After fixing an item, post a concise message asking the reviewer to verify the fix; do not resolve persona-review feedback yourself. The reviewer will re-inspect the fix and resolve the item if complete, or leave it open and reply with further instructions. Keep all review discussion in its feedback-item thread."
: "Call dispatch_review_list_feedback with this reviewId before acting. Use dispatch_review_add_message for questions or explanations, dispatch_review_resolve when an item is fixed or dismissed, and dispatch_review_reopen if a resolved item needs more work. Keep all review discussion in its feedback-item thread."
);
}
return buildReviewPromptBlock("REVIEW SUBMITTED", lines);
Expand All @@ -86,13 +88,18 @@ export function buildReviewThreadUpdatePrompt(input: {
itemId: number;
from: string;
body: string;
recipient: "reviewer" | "assignee";
}): string {
const nextStep =
input.recipient === "reviewer"
? "Re-inspect the claimed fix before deciding. If it fully addresses this item, call dispatch_review_resolve with resolution 'fixed'. If it is incomplete, leave the item open and reply with specific instructions using dispatch_review_add_message. Do not resolve based only on the assignee's claim."
: "Address any requested work, then use dispatch_review_add_message to ask the reviewer to verify the fix. Do not resolve persona-review feedback yourself.";
return buildReviewPromptBlock("REVIEW THREAD UPDATE", [
`Review ID: ${input.reviewId}`,
`Feedback item ID: ${input.itemId}`,
`From: ${input.from}`,
`Message: ${input.body}`,
"Call dispatch_event with type 'working' before handling this update, then call dispatch_review_list_feedback with this reviewId for full context. Reply with dispatch_review_add_message only when useful; do not move review discussion to direct agent messages. Finish with dispatch_event type 'done', or 'waiting_user' only after posting a tracked question that needs a reply.",
`Call dispatch_event with type 'working' before handling this update, then call dispatch_review_list_feedback with this reviewId for full context. ${nextStep} Do not move review discussion to direct agent messages. Finish with dispatch_event type 'done', or 'waiting_user' only after posting a tracked question that needs a reply.`,
]);
}

Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/routes/personas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export async function registerPersonaRoutes(
`Use the dispatch_launch_persona MCP tool to launch the "${body.persona}" persona on your current work.`,
`Use agentType: "${body.agentType}" and includeDiff: ${includeDiff ? "true" : "false"}.`,
"Treat this as an author-requested review for the current worktree/branch.",
"Wait for the structured REVIEW SUBMITTED prompt before acting on findings. Keep all review discussion in feedback-item threads with the dispatch_review_* tools.",
"After launch, do not poll, sleep, call list_agents, or schedule a wakeup. End the turn and wait for Dispatch to inject the structured REVIEW SUBMITTED prompt. Keep all review discussion in feedback-item threads with the dispatch_review_* tools. After fixing an item, ask the reviewer to verify it instead of resolving it yourself.",
"Provide a detailed context briefing covering what you built, key files changed, and any areas that need extra attention.",
].join(" ");

Expand Down
11 changes: 10 additions & 1 deletion apps/server/src/routes/reviews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export async function registerReviewRoutes(
itemId,
agentId,
body.resolution as "fixed" | "dismissed",
{ note, authorType: "human" }
{ note, authorType: "human", resolverRole: "human" }
);
if (!result) {
return reply.code(404).send({ error: "Feedback item not found." });
Expand Down Expand Up @@ -319,6 +319,14 @@ export async function registerReviewRoutes(

return { item: result.item };
} catch (error) {
if (
error instanceof reviewQueries.ReviewFeedbackResolutionConflictError
) {
return reply.code(409).send({
error: error.message,
item: error.item,
});
}
return deps.handleAgentError(reply, error);
}
}
Expand Down Expand Up @@ -387,6 +395,7 @@ export async function registerReviewRoutes(
itemId,
from: "Human collaborator",
body: body.body.trim(),
recipient: review?.reviewerAgentId ? "reviewer" : "assignee",
})
);
} catch (error) {
Expand Down
18 changes: 14 additions & 4 deletions apps/server/src/server/mcp-review-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,8 @@ export function createReviewHandlers(deps: CreateReviewHandlersDeps) {
};
reviewStatus: string;
}> {
const resolver = await agentManager.getAgent(agentId);
if (!resolver) throw new Error("Resolving agent not found.");
const result = await resolveReviewFeedbackItem(
pool,
itemId,
Expand All @@ -310,27 +312,33 @@ export function createReviewHandlers(deps: CreateReviewHandlersDeps) {
note: opts.note ?? null,
resolvedBy: agentId,
authorType: "agent",
resolverRole: resolver.role === "review" ? "reviewer" : "assignee",
}
);
if (!result) {
throw new Error(
`Review feedback item #${itemId} not found or not owned by this agent.`
);
}
const review = await getReviewRecord(pool, result.reviewId);
const eventAgentId = review?.agentId ?? agentId;
publishUiEvent({
type: "review_feedback.updated",
agentId,
agentId: eventAgentId,
feedbackItemId: itemId,
});
publishUiEvent({
type: "review.updated",
agentId,
agentId: eventAgentId,
reviewId: result.reviewId,
status: result.reviewStatus,
});
const review = await getReviewRecord(pool, result.reviewId);
const targetAgentId =
review?.reviewerAgentId === agentId
? (review.assignedAgentId ?? review.agentId)
: (review?.reviewerAgentId ?? null);
await sendPromptBestEffort(
review?.reviewerAgentId ?? null,
targetAgentId,
buildReviewItemStatePrompt({
reviewId: result.reviewId,
itemId,
Expand Down Expand Up @@ -441,6 +449,8 @@ export function createReviewHandlers(deps: CreateReviewHandlersDeps) {
itemId,
from: actor?.persona ?? actor?.name ?? agentId,
body,
recipient:
review?.reviewerAgentId === targetAgentId ? "reviewer" : "assignee",
})
);
return {
Expand Down
8 changes: 5 additions & 3 deletions apps/server/src/shared/mcp/persona-interaction-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ export function registerPersonaInteractionTools(
"dispatch_review_resolve",
{
description:
"Parent-only. Resolve a review feedback item as fixed or dismissed. Review status is automatically derived from the current feedback item states.",
"Resolve a review feedback item as fixed or dismissed. For persona reviews, the reviewer uses this only after re-inspecting the assignee's fix; the assignee should request verification with dispatch_review_add_message instead of resolving the item. Review status is automatically derived from the current feedback item states.",
inputSchema: {
itemId: z
.number()
Expand Down Expand Up @@ -411,7 +411,7 @@ export function registerPersonaInteractionTools(
server.registerTool(
"dispatch_review_add_message",
{
description: `Add a concise message to a review feedback item's thread. Use it only for a necessary clarifying question or essential explanation before resolving. ${AGENT_REVIEW_REPLY_GUIDANCE}`,
description: `Add a concise message to a review feedback item's thread. For persona feedback, the assignee uses this after fixing an item to request reviewer verification; the reviewer uses it to give further instructions when a fix is incomplete. ${AGENT_REVIEW_REPLY_GUIDANCE}`,
inputSchema: {
itemId: z
.number()
Expand Down Expand Up @@ -460,7 +460,9 @@ export function buildLaunchPersonaResponseText(

The reviewer will inspect the target and create a review only when it calls dispatch_review_submit. Dispatch will inject a structured REVIEW SUBMITTED block here with the review summary and any feedback item IDs.

If the review has feedback, call dispatch_review_list_feedback with its reviewId before acting. Keep all questions and explanations tracked in the corresponding item thread with dispatch_review_add_message. Resolve an item with dispatch_review_resolve when it is fixed or intentionally dismissed, and use dispatch_review_reopen if it needs more work.
Do not poll, sleep, call list_agents, or schedule a wakeup while waiting for the review. End this turn after the launch; Dispatch will notify you with a new injected REVIEW SUBMITTED block when the reviewer submits.

If the review has feedback, call dispatch_review_list_feedback with its reviewId before acting. Keep all questions and explanations tracked in the corresponding item thread with dispatch_review_add_message. After fixing an item, post a concise message asking the reviewer to verify it; do not call dispatch_review_resolve on persona feedback yourself. The reviewer will re-inspect the fix and resolve it if complete, or leave it open and reply with further instructions.

A clean approval is also recorded: the reviewer submits a required summary with an empty feedback array, creating a resolved review with no items. No legacy round or recheck lifecycle is required.`;
}
1 change: 1 addition & 0 deletions apps/server/src/shared/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ const REVIEW_AGENT_TOOLS = new Set([
"dispatch_review_add_feedback",
"dispatch_review_list_feedback",
"dispatch_review_add_message",
"dispatch_review_resolve",
"get_parent_context",
]);

Expand Down
5 changes: 5 additions & 0 deletions apps/server/test/db/agent-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,11 @@ describe("AgentManager", () => {
expect(setupScript).toContain("dispatch_review_list_feedback");
expect(setupScript).toContain("structured REVIEW SUBMITTED prompt");
expect(setupScript).toContain("launch 1 relevant reviewer");
expect(setupScript).toContain("do not poll, sleep");
expect(setupScript).toContain("ask the reviewer to verify it");
expect(setupScript).not.toContain(
"set each outcome with dispatch_review_resolve"
);
expect(setupScript).not.toContain("dispatch_get_feedback");
expect(setupScript).not.toContain("Only launch additional reviewers");
});
Expand Down
33 changes: 33 additions & 0 deletions apps/server/test/injection-prompts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,47 @@ describe("unified review prompt blocks", () => {
itemId: 9,
from: "Parent",
body: "Can you clarify?",
recipient: "reviewer",
});
expect(text).toContain("Review ID: 7");
expect(text).toContain("Feedback item ID: 9");
expect(text).toContain("dispatch_review_add_message");
expect(text).toContain("dispatch_review_resolve");
expect(text).toMatch(/re-inspect/i);
expect(text).toContain("type 'working'");
expect(text).toContain("type 'done'");
});

it("tells the assignee to request verification instead of resolving", () => {
const text = buildReviewSubmittedPrompt({
reviewId: 7,
reviewerName: "Security",
reviewerAgentId: "agt_reviewer",
summary: null,
items: [{ id: 9, filePath: null, lineStart: null, body: "Finding" }],
});

expect(text).toMatch(/asking the reviewer to verify/i);
expect(text).toMatch(/do not resolve persona-review feedback yourself/i);
});

it("keeps resolution guidance for human reviews", () => {
const text = buildReviewSubmittedPrompt({
reviewId: 7,
reviewerName: "Human reviewer",
reviewerAgentId: null,
summary: null,
items: [{ id: 9, filePath: null, lineStart: null, body: "Finding" }],
});

expect(text).toMatch(/dispatch_review_resolve when an item is fixed/i);
expect(text).toContain("dispatch_review_reopen");
expect(text).not.toMatch(/asking the reviewer to verify/i);
expect(text).not.toMatch(
/do not resolve persona-review feedback yourself/i
);
});

it("omits an absent summary when feedback items carry the review", () => {
const text = buildReviewSubmittedPrompt({
reviewId: 7,
Expand Down
1 change: 1 addition & 0 deletions apps/server/test/mcp-auth-integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ describe("MCP auth integration", () => {
expect(response.body).toContain("dispatch_review_add_feedback");
expect(response.body).toContain("dispatch_review_list_feedback");
expect(response.body).toContain("dispatch_review_add_message");
expect(response.body).toContain("dispatch_review_resolve");
expect(response.body).toContain("get_parent_context");
expect(response.body).not.toContain("dispatch_get_recheck_context");
expect(response.body).not.toContain("dispatch_complete_review");
Expand Down
8 changes: 7 additions & 1 deletion apps/server/test/mcp-handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,7 @@ describe("createMcpHandlers", () => {
{
authorType: "agent",
note: "addressed in latest commit",
resolverRole: "assignee",
resolvedBy: "agt_test1",
}
);
Expand Down Expand Up @@ -1638,7 +1639,12 @@ describe("createMcpHandlers", () => {
10,
"agt_test1",
"ignored",
{ authorType: "agent", note: null, resolvedBy: "agt_test1" }
{
authorType: "agent",
note: null,
resolverRole: "assignee",
resolvedBy: "agt_test1",
}
);
});
});
Expand Down
Loading
Loading