feat(KB-172): add spec review loop to triage agent
- Extend reviewer with 'spec' review type and spec review format template - Add review_spec tool to triage agent for independent spec quality evaluation - Implement APPROVE/REVISE/RETHINK loop with conversation rewind on RETHINK - Add post-session gate to block tasks with unresolved REVISE verdicts from moving to todo - Add tests for reviewer spec support and triage review loop (APPROVE, REVISE, RETHINK paths)
This commit is contained in:
@@ -80,6 +80,121 @@ describe("reviewStep — model settings threading", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("reviewStep — spec review type", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("extracts verdict correctly for spec reviews", async () => {
|
||||
mockedCreateHaiAgent.mockResolvedValue(
|
||||
createMockSession("## Spec Review: KB-050\n\n### Verdict: APPROVE\n### Summary\nSpec looks complete and well-structured."),
|
||||
);
|
||||
|
||||
const result = await reviewStep(
|
||||
"/tmp/worktree", "KB-050", 0, "Spec Review", "spec", "# Task: KB-050\n\n## Mission\nDo something",
|
||||
);
|
||||
|
||||
expect(result.verdict).toBe("APPROVE");
|
||||
expect(result.summary).toContain("well-structured");
|
||||
});
|
||||
|
||||
it("extracts REVISE verdict for spec reviews", async () => {
|
||||
mockedCreateHaiAgent.mockResolvedValue(
|
||||
createMockSession("## Spec Review: KB-050\n\n### Verdict: REVISE\n### Summary\nMissing test requirements."),
|
||||
);
|
||||
|
||||
const result = await reviewStep(
|
||||
"/tmp/worktree", "KB-050", 0, "Spec Review", "spec", "# Task: KB-050",
|
||||
);
|
||||
|
||||
expect(result.verdict).toBe("REVISE");
|
||||
});
|
||||
|
||||
it("extracts RETHINK verdict for spec reviews", async () => {
|
||||
mockedCreateHaiAgent.mockResolvedValue(
|
||||
createMockSession("## Spec Review: KB-050\n\n### Verdict: RETHINK\n### Summary\nFundamentally wrong approach."),
|
||||
);
|
||||
|
||||
const result = await reviewStep(
|
||||
"/tmp/worktree", "KB-050", 0, "Spec Review", "spec", "# Task: KB-050",
|
||||
);
|
||||
|
||||
expect(result.verdict).toBe("RETHINK");
|
||||
});
|
||||
|
||||
it("calls createKbAgent with readonly tools and correct system prompt", async () => {
|
||||
mockedCreateHaiAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nGood spec."),
|
||||
);
|
||||
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "KB-050", 0, "Spec Review", "spec", "# Task: KB-050",
|
||||
);
|
||||
|
||||
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(1);
|
||||
const opts = mockedCreateHaiAgent.mock.calls[0][0];
|
||||
expect(opts.tools).toBe("readonly");
|
||||
expect(opts.systemPrompt).toContain("Spec Review Format");
|
||||
expect(opts.systemPrompt).toContain("Mission clarity");
|
||||
});
|
||||
|
||||
it("builds review request with spec-specific instructions", async () => {
|
||||
let capturedPrompt = "";
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async (prompt: string) => {
|
||||
capturedPrompt = prompt;
|
||||
}),
|
||||
subscribe: vi.fn().mockImplementation((cb: any) => {
|
||||
cb({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: "### Verdict: APPROVE\n### Summary\nOK" },
|
||||
});
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "KB-050", 0, "Spec Review", "spec",
|
||||
"# Task: KB-050\n\n## Mission\nDo something great",
|
||||
);
|
||||
|
||||
expect(capturedPrompt).toContain("Evaluate this PROMPT.md specification");
|
||||
expect(capturedPrompt).toContain("spec quality criteria");
|
||||
expect(capturedPrompt).toContain("# Task: KB-050");
|
||||
// Spec reviews should NOT contain git diff instructions
|
||||
expect(capturedPrompt).not.toContain("git diff");
|
||||
});
|
||||
|
||||
it("does not include git diff instructions for spec reviews", async () => {
|
||||
let capturedPrompt = "";
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async (prompt: string) => {
|
||||
capturedPrompt = prompt;
|
||||
}),
|
||||
subscribe: vi.fn().mockImplementation((cb: any) => {
|
||||
cb({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: "### Verdict: APPROVE\n### Summary\nOK" },
|
||||
});
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
|
||||
// Pass a baseline — should be ignored for spec reviews
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "KB-050", 0, "Spec Review", "spec",
|
||||
"# Task: KB-050", "abc123",
|
||||
);
|
||||
|
||||
expect(capturedPrompt).not.toContain("git diff");
|
||||
expect(capturedPrompt).not.toContain("abc123");
|
||||
});
|
||||
});
|
||||
|
||||
describe("reviewStep — exhausted-retry error detection", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
@@ -87,6 +87,32 @@ access to the codebase and can run commands to inspect code.
|
||||
- [Optional improvements, not blocking]
|
||||
\`\`\`
|
||||
|
||||
## Spec Review Format
|
||||
|
||||
\`\`\`markdown
|
||||
## Spec Review: [Task ID]
|
||||
|
||||
### Verdict: [APPROVE | REVISE | RETHINK]
|
||||
|
||||
### Summary
|
||||
[2-3 sentence assessment of the specification quality]
|
||||
|
||||
### Issues Found
|
||||
1. **[Severity: critical/important/minor]** — [Description and suggested fix]
|
||||
|
||||
### Criteria Assessment
|
||||
- **Mission clarity:** [Clear, unambiguous mission statement?]
|
||||
- **Step specificity:** [Steps have verifiable, concrete outcomes?]
|
||||
- **File scope accuracy:** [All affected files listed? No extras?]
|
||||
- **Dependency correctness:** [Dependencies exist and are appropriate?]
|
||||
- **Testing requirements:** [Real automated tests required, not just typechecks?]
|
||||
- **Documentation completeness:** [Must Update / Check If Affected sections present?]
|
||||
- **Sizing & review level:** [Size and review level appropriate for the work?]
|
||||
|
||||
### Suggestions
|
||||
- [Optional improvements, not blocking]
|
||||
\`\`\`
|
||||
|
||||
## Plan Granularity
|
||||
|
||||
When reviewing plans, assess whether the approach achieves the step's OUTCOMES —
|
||||
@@ -103,7 +129,7 @@ Do NOT demand function-level implementation checklists.
|
||||
- Output your review as plain text (not to a file)
|
||||
`;
|
||||
|
||||
export type ReviewType = "plan" | "code";
|
||||
export type ReviewType = "plan" | "code" | "spec";
|
||||
export type ReviewVerdict = "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE";
|
||||
|
||||
export interface ReviewResult {
|
||||
@@ -218,7 +244,19 @@ function buildReviewRequest(
|
||||
"",
|
||||
];
|
||||
|
||||
if (reviewType === "plan") {
|
||||
if (reviewType === "spec") {
|
||||
parts.push(
|
||||
"## What to review",
|
||||
"Evaluate this PROMPT.md specification for completeness and quality.",
|
||||
"Assess against the spec quality criteria: mission clarity, step specificity/verifiability,",
|
||||
"file scope accuracy, dependency correctness, testing requirements, documentation completeness,",
|
||||
"and appropriate sizing/review level.",
|
||||
"",
|
||||
"Read relevant source files to verify the spec references real files, functions, and patterns.",
|
||||
"Check that steps have concrete, verifiable outcomes — not vague instructions.",
|
||||
"Ensure testing requirements demand real automated tests with assertions.",
|
||||
);
|
||||
} else if (reviewType === "plan") {
|
||||
parts.push(
|
||||
"## What to review",
|
||||
`The worker is about to implement Step ${stepNumber} (${stepName}).`,
|
||||
|
||||
@@ -5,15 +5,22 @@ import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { AgentSemaphore } from "./concurrency.js";
|
||||
|
||||
// Mock createKbAgent before importing TriageProcessor
|
||||
// Mock createKbAgent and reviewStep before importing TriageProcessor
|
||||
vi.mock("./pi.js", () => ({
|
||||
createKbAgent: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("./reviewer.js", () => ({
|
||||
reviewStep: vi.fn(),
|
||||
}));
|
||||
|
||||
import { TriageProcessor, buildSpecificationPrompt, type AttachmentContent } from "./triage.js";
|
||||
import { createKbAgent } from "./pi.js";
|
||||
import { reviewStep } from "./reviewer.js";
|
||||
import type { TaskDetail } from "@kb/core";
|
||||
|
||||
const mockedReviewStep = vi.mocked(reviewStep);
|
||||
|
||||
const mockedCreateHaiAgent = vi.mocked(createKbAgent);
|
||||
|
||||
function createMockStore(tasks: any[] = []) {
|
||||
@@ -1929,3 +1936,346 @@ describe("TriageProcessor enginePaused agent termination", () => {
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", { status: null });
|
||||
});
|
||||
});
|
||||
|
||||
// ── Triage spec review loop tests ──────────────────────────────────
|
||||
|
||||
describe("TriageProcessor review_spec tool", () => {
|
||||
let tmpDir: string;
|
||||
|
||||
const makeTask = (id = "KB-001") => ({
|
||||
id,
|
||||
title: "Test",
|
||||
description: "Test task",
|
||||
column: "triage" as const,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
tmpDir = mkdtempSync(join(tmpdir(), "kb-triage-review-"));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function writePromptMd(rootDir: string, taskId: string, content: string) {
|
||||
const dir = join(rootDir, ".kb", "tasks", taskId);
|
||||
await mkdir(dir, { recursive: true });
|
||||
await writeFile(join(dir, "PROMPT.md"), content);
|
||||
}
|
||||
|
||||
it("registers review_spec as a custom tool on createKbAgent calls", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
},
|
||||
} as any);
|
||||
|
||||
const triage = new TriageProcessor(store, tmpDir);
|
||||
await triage.specifyTask(makeTask());
|
||||
|
||||
expect(mockedCreateHaiAgent).toHaveBeenCalledOnce();
|
||||
const callArgs = mockedCreateHaiAgent.mock.calls[0][0];
|
||||
const tools = callArgs.customTools as any[];
|
||||
const reviewTool = tools.find((t: any) => t.name === "review_spec");
|
||||
expect(reviewTool).toBeDefined();
|
||||
expect(reviewTool.name).toBe("review_spec");
|
||||
expect(reviewTool.description).toContain("reviewer");
|
||||
});
|
||||
|
||||
it("system prompt contains instructions for calling review_spec", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
},
|
||||
} as any);
|
||||
|
||||
const triage = new TriageProcessor(store, tmpDir);
|
||||
await triage.specifyTask(makeTask());
|
||||
|
||||
const callArgs = mockedCreateHaiAgent.mock.calls[0][0];
|
||||
const systemPrompt = callArgs.systemPrompt as string;
|
||||
expect(systemPrompt).toContain("review_spec()");
|
||||
expect(systemPrompt).toContain("APPROVE");
|
||||
expect(systemPrompt).toContain("REVISE");
|
||||
expect(systemPrompt).toContain("RETHINK");
|
||||
});
|
||||
|
||||
it("when reviewer returns APPROVE, task proceeds to todo normally", async () => {
|
||||
const store = createMockStore();
|
||||
store.parseDependenciesFromPrompt.mockResolvedValue([]);
|
||||
|
||||
const promptContent = "# Task: KB-001\n\n**Size:** S\n\n## Review Level: 0\n\n## Steps\n";
|
||||
await writePromptMd(tmpDir, "KB-001", promptContent);
|
||||
|
||||
let reviewSpecTool: any;
|
||||
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||
reviewSpecTool = opts.customTools?.find((t: any) => t.name === "review_spec");
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
// Simulate the agent calling review_spec
|
||||
if (reviewSpecTool) {
|
||||
await reviewSpecTool.execute("call-1", {});
|
||||
}
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
mockedReviewStep.mockResolvedValue({
|
||||
verdict: "APPROVE",
|
||||
review: "Good spec",
|
||||
summary: "Looks good",
|
||||
});
|
||||
|
||||
const triage = new TriageProcessor(store, tmpDir);
|
||||
await triage.specifyTask(makeTask());
|
||||
|
||||
// Task should move to todo
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo");
|
||||
});
|
||||
|
||||
it("review_spec tool calls reviewStep with reviewType spec", async () => {
|
||||
const store = createMockStore();
|
||||
store.parseDependenciesFromPrompt.mockResolvedValue([]);
|
||||
|
||||
const promptContent = "# Task: KB-001\n\n**Size:** S\n\n## Review Level: 0\n\n## Steps\n";
|
||||
await writePromptMd(tmpDir, "KB-001", promptContent);
|
||||
|
||||
let reviewSpecTool: any;
|
||||
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||
reviewSpecTool = opts.customTools?.find((t: any) => t.name === "review_spec");
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
if (reviewSpecTool) {
|
||||
await reviewSpecTool.execute("call-1", {});
|
||||
}
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
mockedReviewStep.mockResolvedValue({
|
||||
verdict: "APPROVE",
|
||||
review: "Good spec",
|
||||
summary: "Looks good",
|
||||
});
|
||||
|
||||
const triage = new TriageProcessor(store, tmpDir);
|
||||
await triage.specifyTask(makeTask());
|
||||
|
||||
expect(mockedReviewStep).toHaveBeenCalledTimes(1);
|
||||
const reviewArgs = mockedReviewStep.mock.calls[0];
|
||||
expect(reviewArgs[0]).toBe(tmpDir); // cwd = rootDir
|
||||
expect(reviewArgs[1]).toBe("KB-001"); // taskId
|
||||
expect(reviewArgs[2]).toBe(0); // stepNumber
|
||||
expect(reviewArgs[3]).toBe("Specification"); // stepName
|
||||
expect(reviewArgs[4]).toBe("spec"); // reviewType
|
||||
expect(reviewArgs[5]).toBe(promptContent); // promptContent
|
||||
});
|
||||
|
||||
it("logs review verdict via store.logEntry", async () => {
|
||||
const store = createMockStore();
|
||||
store.parseDependenciesFromPrompt.mockResolvedValue([]);
|
||||
|
||||
const promptContent = "# Task: KB-001\n\n**Size:** S\n\n## Steps\n";
|
||||
await writePromptMd(tmpDir, "KB-001", promptContent);
|
||||
|
||||
let reviewSpecTool: any;
|
||||
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||
reviewSpecTool = opts.customTools?.find((t: any) => t.name === "review_spec");
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
if (reviewSpecTool) {
|
||||
await reviewSpecTool.execute("call-1", {});
|
||||
}
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
mockedReviewStep.mockResolvedValue({
|
||||
verdict: "APPROVE",
|
||||
review: "Good spec",
|
||||
summary: "Looks good",
|
||||
});
|
||||
|
||||
const triage = new TriageProcessor(store, tmpDir);
|
||||
await triage.specifyTask(makeTask());
|
||||
|
||||
// Check logEntry calls for review-related entries
|
||||
const logCalls = store.logEntry.mock.calls;
|
||||
const reviewRequestLog = logCalls.find((c: any[]) => c[1] === "Spec review requested");
|
||||
expect(reviewRequestLog).toBeDefined();
|
||||
|
||||
const verdictLog = logCalls.find((c: any[]) => c[1] === "Spec review: APPROVE");
|
||||
expect(verdictLog).toBeDefined();
|
||||
expect(verdictLog![2]).toBe("Looks good"); // summary as outcome
|
||||
});
|
||||
|
||||
it("reviewer failure returns UNAVAILABLE to the agent", async () => {
|
||||
const store = createMockStore();
|
||||
store.parseDependenciesFromPrompt.mockResolvedValue([]);
|
||||
|
||||
const promptContent = "# Task: KB-001\n\n**Size:** S\n\n## Steps\n";
|
||||
await writePromptMd(tmpDir, "KB-001", promptContent);
|
||||
|
||||
let reviewResult: any;
|
||||
let reviewSpecTool: any;
|
||||
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||
reviewSpecTool = opts.customTools?.find((t: any) => t.name === "review_spec");
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
if (reviewSpecTool) {
|
||||
reviewResult = await reviewSpecTool.execute("call-1", {});
|
||||
}
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
mockedReviewStep.mockRejectedValue(new Error("API connection failed"));
|
||||
|
||||
const triage = new TriageProcessor(store, tmpDir);
|
||||
await triage.specifyTask(makeTask());
|
||||
|
||||
expect(reviewResult.content[0].text).toContain("UNAVAILABLE");
|
||||
expect(reviewResult.content[0].text).toContain("API connection failed");
|
||||
});
|
||||
|
||||
it("returns UNAVAILABLE when PROMPT.md file does not exist", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
let reviewResult: any;
|
||||
let reviewSpecTool: any;
|
||||
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||
reviewSpecTool = opts.customTools?.find((t: any) => t.name === "review_spec");
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
if (reviewSpecTool) {
|
||||
reviewResult = await reviewSpecTool.execute("call-1", {});
|
||||
}
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
const triage = new TriageProcessor(store, tmpDir);
|
||||
await triage.specifyTask(makeTask());
|
||||
|
||||
expect(reviewResult.content[0].text).toContain("UNAVAILABLE");
|
||||
expect(reviewResult.content[0].text).toContain("not found or empty");
|
||||
});
|
||||
|
||||
it("post-session REVISE gate prevents moving to todo when last verdict is REVISE", async () => {
|
||||
const store = createMockStore();
|
||||
store.parseDependenciesFromPrompt.mockResolvedValue([]);
|
||||
|
||||
const promptContent = "# Task: KB-001\n\n**Size:** S\n\n## Steps\n";
|
||||
await writePromptMd(tmpDir, "KB-001", promptContent);
|
||||
|
||||
let reviewSpecTool: any;
|
||||
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||
reviewSpecTool = opts.customTools?.find((t: any) => t.name === "review_spec");
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
// Simulate the agent calling review_spec but getting REVISE and then stopping
|
||||
if (reviewSpecTool) {
|
||||
await reviewSpecTool.execute("call-1", {});
|
||||
}
|
||||
// Agent finishes without APPROVE
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
mockedReviewStep.mockResolvedValue({
|
||||
verdict: "REVISE",
|
||||
review: "Missing test requirements",
|
||||
summary: "Spec needs work",
|
||||
});
|
||||
|
||||
const triage = new TriageProcessor(store, tmpDir);
|
||||
await triage.specifyTask(makeTask());
|
||||
|
||||
// Task should NOT move to todo
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
// Status should be cleared
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", { status: null });
|
||||
// Should log the REVISE gate
|
||||
const logCalls = store.logEntry.mock.calls;
|
||||
const reviseGateLog = logCalls.find((c: any[]) =>
|
||||
typeof c[1] === "string" && c[1].includes("not approved"),
|
||||
);
|
||||
expect(reviseGateLog).toBeDefined();
|
||||
});
|
||||
|
||||
it("REVISE tool response includes review feedback", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
const promptContent = "# Task: KB-001\n\n**Size:** S\n\n## Steps\n";
|
||||
await writePromptMd(tmpDir, "KB-001", promptContent);
|
||||
|
||||
let reviewResult: any;
|
||||
let reviewSpecTool: any;
|
||||
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||
reviewSpecTool = opts.customTools?.find((t: any) => t.name === "review_spec");
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
if (reviewSpecTool) {
|
||||
reviewResult = await reviewSpecTool.execute("call-1", {});
|
||||
}
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
mockedReviewStep.mockResolvedValue({
|
||||
verdict: "REVISE",
|
||||
review: "Missing test requirements\n\nAdd real tests.",
|
||||
summary: "Spec needs work",
|
||||
});
|
||||
|
||||
const triage = new TriageProcessor(store, tmpDir);
|
||||
await triage.specifyTask(makeTask());
|
||||
|
||||
expect(reviewResult.content[0].text).toContain("REVISE");
|
||||
expect(reviewResult.content[0].text).toContain("Missing test requirements");
|
||||
expect(reviewResult.content[0].text).toContain("call review_spec() again");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
import type { TaskStore, Task, TaskDetail, TaskAttachment, Settings } from "@kb/core";
|
||||
import type { ImageContent } from "@mariozechner/pi-ai";
|
||||
import { Type, type Static } from "@mariozechner/pi-ai";
|
||||
import type { ToolDefinition } from "@mariozechner/pi-coding-agent";
|
||||
import type { ToolDefinition, AgentSession } from "@mariozechner/pi-coding-agent";
|
||||
import { createKbAgent } from "./pi.js";
|
||||
import { reviewStep, type ReviewVerdict } from "./reviewer.js";
|
||||
import { PRIORITY_SPECIFY, type AgentSemaphore } from "./concurrency.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
import { triageLog } from "./logger.js";
|
||||
import { triageLog, reviewerLog } from "./logger.js";
|
||||
import { isUsageLimitError, checkSessionError, type UsageLimitPauser } from "./usage-limit-detector.js";
|
||||
|
||||
const TRIAGE_SYSTEM_PROMPT = `You are a task specification agent for "kb", an AI-orchestrated task board.
|
||||
@@ -152,8 +153,18 @@ commands, use those EXACT commands in the testing/verification steps and anywher
|
||||
the spec references running tests or builds. Do NOT guess or infer commands from
|
||||
package.json when explicit commands are provided.
|
||||
|
||||
## Spec Review
|
||||
|
||||
After writing the PROMPT.md, call \`review_spec()\` to get an independent quality review.
|
||||
|
||||
- **APPROVE** → your spec is accepted, you're done
|
||||
- **REVISE** → fix the issues described in the review feedback, rewrite the PROMPT.md, and call \`review_spec()\` again. Repeat until approved.
|
||||
- **RETHINK** → your approach was fundamentally rejected. The conversation will rewind. Read the feedback carefully and take a completely different approach. Do NOT repeat the rejected strategy.
|
||||
|
||||
You MUST call \`review_spec()\` after writing the PROMPT.md. Do not finish without getting an APPROVE verdict.
|
||||
|
||||
## Output
|
||||
Write the PROMPT.md directly using the write tool. Nothing else.`;
|
||||
Write the PROMPT.md directly using the write tool, then call \`review_spec()\` for review.`;
|
||||
|
||||
export interface TriageProcessorOptions {
|
||||
pollIntervalMs?: number;
|
||||
@@ -346,6 +357,19 @@ export class TriageProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Specify a triage task by spawning an AI agent to generate a PROMPT.md.
|
||||
*
|
||||
* After the agent writes the PROMPT.md, it calls `review_spec()` to spawn
|
||||
* an independent reviewer agent that evaluates the specification quality.
|
||||
* The review loop works as follows:
|
||||
* - **APPROVE**: the spec is accepted and the task moves to `todo`
|
||||
* - **REVISE**: the agent revises the spec and calls `review_spec()` again.
|
||||
* If the agent finishes without getting APPROVE, the task is NOT moved to
|
||||
* `todo` — a post-session gate checks the last verdict.
|
||||
* - **RETHINK**: the conversation rewinds to a pre-specification checkpoint
|
||||
* and the agent starts over with a fundamentally different approach.
|
||||
*/
|
||||
async specifyTask(task: Task): Promise<void> {
|
||||
if (this.processing.has(task.id)) return;
|
||||
this.processing.add(task.id);
|
||||
@@ -375,11 +399,25 @@ export class TriageProcessor {
|
||||
},
|
||||
});
|
||||
|
||||
// Mutable ref — populated after createKbAgent, tools access lazily via closure
|
||||
const sessionRef: { current: AgentSession | null } = { current: null };
|
||||
// Checkpoint for RETHINK rewind — captured lazily on first review_spec call
|
||||
const checkpointRef: { current: string | null } = { current: null };
|
||||
// Track the last spec review verdict for post-session enforcement
|
||||
const specReviewVerdictRef: { current: ReviewVerdict | null } = { current: null };
|
||||
|
||||
const customTools = [
|
||||
...this.createTriageTools(),
|
||||
this.createReviewSpecTool(
|
||||
task.id, promptPath, sessionRef, checkpointRef, specReviewVerdictRef, settings,
|
||||
),
|
||||
];
|
||||
|
||||
const { session } = await createKbAgent({
|
||||
cwd: this.rootDir,
|
||||
systemPrompt: TRIAGE_SYSTEM_PROMPT,
|
||||
tools: "coding",
|
||||
customTools: this.createTriageTools(),
|
||||
customTools,
|
||||
onText: agentLogger.onText,
|
||||
onThinking: agentLogger.onThinking,
|
||||
onToolStart: agentLogger.onToolStart,
|
||||
@@ -389,6 +427,9 @@ export class TriageProcessor {
|
||||
defaultThinkingLevel: settings.defaultThinkingLevel,
|
||||
});
|
||||
|
||||
// Make session available to review_spec tool (for RETHINK rewind)
|
||||
sessionRef.current = session;
|
||||
|
||||
// Register session so the global pause listener can terminate it
|
||||
this.activeSessions.set(task.id, session);
|
||||
|
||||
@@ -404,6 +445,15 @@ export class TriageProcessor {
|
||||
// Re-raise errors that pi-coding-agent swallowed after exhausting retries.
|
||||
checkSessionError(session);
|
||||
|
||||
// Post-session REVISE gate: if the last review_spec verdict was REVISE
|
||||
// and the agent finished without getting APPROVE, don't move to todo.
|
||||
if (specReviewVerdictRef.current === "REVISE") {
|
||||
triageLog.log(`${task.id} spec review ended with REVISE — not moving to todo`);
|
||||
await this.store.logEntry(task.id, "Spec review ended with REVISE verdict — specification not approved");
|
||||
await this.store.updateTask(task.id, { status: null });
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if the agent flagged a duplicate
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
const { join } = await import("node:path");
|
||||
@@ -549,6 +599,146 @@ export class TriageProcessor {
|
||||
|
||||
return [taskList, taskGet];
|
||||
}
|
||||
|
||||
/**
|
||||
* Create the `review_spec` tool for the triage agent.
|
||||
*
|
||||
* Spawns an independent reviewer agent to evaluate the generated PROMPT.md.
|
||||
* Verdict handling:
|
||||
* - **APPROVE**: returns "APPROVE" — the triage agent's work is done.
|
||||
* - **REVISE**: returns the review feedback. The triage agent must fix the
|
||||
* PROMPT.md and call `review_spec` again. A post-session gate in
|
||||
* `specifyTask()` prevents moving to `todo` if the last verdict is REVISE.
|
||||
* - **RETHINK**: rewinds the conversation to a pre-specification checkpoint
|
||||
* using `session.navigateTree()`. Returns a re-prompt instructing the agent
|
||||
* to take a fundamentally different approach.
|
||||
*/
|
||||
private createReviewSpecTool(
|
||||
taskId: string,
|
||||
promptPath: string,
|
||||
sessionRef: { current: AgentSession | null },
|
||||
checkpointRef: { current: string | null },
|
||||
specReviewVerdictRef: { current: ReviewVerdict | null },
|
||||
settings: { defaultProvider?: string; defaultModelId?: string; defaultThinkingLevel?: string },
|
||||
): ToolDefinition {
|
||||
const store = this.store;
|
||||
const rootDir = this.rootDir;
|
||||
const options = this.options;
|
||||
|
||||
return {
|
||||
name: "review_spec",
|
||||
label: "Review Specification",
|
||||
description:
|
||||
"Spawn a reviewer agent to evaluate the generated PROMPT.md specification. " +
|
||||
"Returns APPROVE, REVISE, RETHINK, or UNAVAILABLE. " +
|
||||
"Call after writing the PROMPT.md.",
|
||||
parameters: Type.Object({}),
|
||||
execute: async () => {
|
||||
reviewerLog.log(`${taskId}: spec review requested`);
|
||||
await store.logEntry(taskId, "Spec review requested");
|
||||
|
||||
// Capture checkpoint lazily on first call — at this point the session
|
||||
// has already started and has a valid conversation state to rewind to.
|
||||
if (!checkpointRef.current && sessionRef.current) {
|
||||
checkpointRef.current = sessionRef.current.sessionManager.getLeafId() ?? null;
|
||||
}
|
||||
|
||||
try {
|
||||
// Read the generated PROMPT.md from disk
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
const { join } = await import("node:path");
|
||||
const promptContent = await readFile(
|
||||
join(rootDir, promptPath), "utf-8",
|
||||
).catch(() => "");
|
||||
|
||||
if (!promptContent) {
|
||||
return {
|
||||
content: [{
|
||||
type: "text" as const,
|
||||
text: "UNAVAILABLE — PROMPT.md file not found or empty. Write the specification first, then call review_spec.",
|
||||
}],
|
||||
details: {},
|
||||
};
|
||||
}
|
||||
|
||||
const result = await reviewStep(
|
||||
rootDir, taskId, 0, "Specification",
|
||||
"spec", promptContent, undefined,
|
||||
{
|
||||
onText: (delta) => options.onAgentText?.(taskId, delta),
|
||||
defaultProvider: settings.defaultProvider,
|
||||
defaultModelId: settings.defaultModelId,
|
||||
defaultThinkingLevel: settings.defaultThinkingLevel,
|
||||
store,
|
||||
taskId,
|
||||
},
|
||||
);
|
||||
|
||||
// Track verdict for post-session enforcement
|
||||
specReviewVerdictRef.current = result.verdict;
|
||||
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`Spec review: ${result.verdict}`,
|
||||
result.summary,
|
||||
);
|
||||
reviewerLog.log(`${taskId}: spec review → ${result.verdict}`);
|
||||
|
||||
let text: string;
|
||||
switch (result.verdict) {
|
||||
case "APPROVE":
|
||||
text = "APPROVE";
|
||||
break;
|
||||
case "REVISE":
|
||||
text = `REVISE — fix the issues below, rewrite the PROMPT.md, and call review_spec() again.\n\n${result.review}`;
|
||||
break;
|
||||
case "RETHINK": {
|
||||
// Rewind conversation to pre-specification checkpoint
|
||||
const checkpointId = checkpointRef.current;
|
||||
if (checkpointId && sessionRef.current) {
|
||||
try {
|
||||
await sessionRef.current.navigateTree(checkpointId, { summarize: false });
|
||||
triageLog.log(`${taskId}: RETHINK — session rewound to checkpoint ${checkpointId}`);
|
||||
} catch {
|
||||
// Fallback to branchWithSummary
|
||||
try {
|
||||
sessionRef.current.sessionManager.branchWithSummary(
|
||||
checkpointId,
|
||||
`RETHINK: ${result.summary || "Approach rejected by reviewer"}`,
|
||||
);
|
||||
triageLog.log(`${taskId}: RETHINK — branched from checkpoint ${checkpointId}`);
|
||||
} catch (branchErr: any) {
|
||||
triageLog.error(`${taskId}: RETHINK session rewind failed: ${branchErr.message}`);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
triageLog.log(`${taskId}: RETHINK — no session checkpoint, skipping rewind`);
|
||||
}
|
||||
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`RETHINK: spec rewound — session checkpoint ${checkpointId || "N/A"}`,
|
||||
result.summary,
|
||||
);
|
||||
text = `RETHINK\n\nYour specification was rejected. Here is why:\n\n${result.review}\n\nTake a completely different approach to writing this specification. Do NOT repeat the rejected strategy.`;
|
||||
break;
|
||||
}
|
||||
default:
|
||||
text = "UNAVAILABLE — reviewer did not produce a usable verdict.";
|
||||
}
|
||||
|
||||
return { content: [{ type: "text" as const, text }], details: {} };
|
||||
} catch (err: any) {
|
||||
reviewerLog.error(`${taskId}: spec review failed: ${err.message}`);
|
||||
await store.logEntry(taskId, `Spec review failed: ${err.message}`);
|
||||
return {
|
||||
content: [{ type: "text" as const, text: `UNAVAILABLE — reviewer error: ${err.message}` }],
|
||||
details: {},
|
||||
};
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
/** Content read from an attachment file for inlining in the prompt. */
|
||||
|
||||
Reference in New Issue
Block a user