From fc3bc1b4d19c598e1645dab5acf273191c614b0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 8 Oct 2026 18:08:00 -0400 Subject: [PATCH] Q111: a goal audit approves only after reading the work itself Co-Authored-By: Claude Opus 5.5 --- src/main/services/agent-goal-tools.ts | 38 +++++++++++++++--- tests/unit/agent-goal-tools.test.ts | 55 ++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/main/services/agent-goal-tools.ts b/src/main/services/agent-goal-tools.ts index 142dcbe4..58166d9d 100644 --- a/src/main/services/agent-goal-tools.ts +++ b/src/main/services/agent-goal-tools.ts @@ -8,8 +8,11 @@ import type { ProviderService } from "./provider-service"; import { unreadableArguments, type AgentTurnMessage, type ToolCall, type ToolDefinition } from "./provider-tool-turn"; import { MAX_TOOL_CALLS_PER_TURN, throwIfRunStopped, untilStopped } from "./run-control"; import { heldWorkTitles } from "./held-deliverables"; +import { DOCUMENT_TOOLS, listDocumentsTool, readDocumentTool } from "./agent-document-tools"; const AUDIT_MAX_ITERATIONS = 6; +const AUDIT_DOCUMENT_TOOLS = new Set(["list_documents", "read_document"]); +const UNREAD_APPROVAL = "You approved without reading any evidence. Open the work itself with read_file or read_document, then decide again."; const STATUS_TASK_LINES = 8; export const GOAL_TOOLS: ToolDefinition[] = [ @@ -121,8 +124,12 @@ async function auditGoalCompletion( goal: { title: string; description: string }, evidence: string, ): Promise { - // Read-only inspection: the auditor may look at anything but change nothing. - const auditTools = WORKSPACE_TOOLS.filter((tool) => tool.name !== "run_command"); + // Read-only inspection of the workspace and the library: the auditor may look at anything but change nothing. + const auditTools = [ + ...WORKSPACE_TOOLS.filter((tool) => tool.name !== "run_command"), + ...DOCUMENT_TOOLS.filter((tool) => AUDIT_DOCUMENT_TOOLS.has(tool.name)), + ]; + let evidenceRead = false; const auditCtx: AgentToolContext = { ...args.ctx, workspace: args.ctx.workspace ? { ...args.ctx.workspace, writeAccess: false } : null, @@ -135,7 +142,7 @@ async function auditGoalCompletion( `Goal detail: ${goal.description}`, "The executor's claim:", evidence, - "Inspect the workspace with list_files and read_file before deciding. Judge outcomes, not artifacts about outcomes: plans, specs, status reports, and skeletons do not make a goal achieved; working deliverables do. If the goal promises software, look for the actual implementation, not documents describing it.", + "Inspect the work before deciding: list_files and read_file for the workspace, list_documents and read_document for the library. An approval counts only after you have read some of it. Judge outcomes, not artifacts about outcomes: plans, specs, status reports, and skeletons do not make a goal achieved; working deliverables do. If the goal promises software, look for the actual implementation, not documents describing it.", "End your final message with exactly one marker: if the goal is genuinely achieved, or followed by one short sentence naming the decisive gap.", ].join("\n\n"), }]; @@ -152,14 +159,21 @@ async function auditGoalCompletion( messages, tools: auditTools, })); - if (turn.toolCalls.length === 0) return parseVerdict(turn.text); + if (turn.toolCalls.length === 0) { + const verdict = parseVerdict(turn.text); + if (!verdict.approved || evidenceRead) return verdict; + // An approval reached without opening anything is not a judgment of the work; the auditor must look first. + messages.push({ role: "assistant", text: turn.text, toolCalls: [], ...(turn.replay ? { replay: turn.replay } : {}) }, { role: "user", text: UNREAD_APPROVAL }); + continue; + } messages.push({ role: "assistant", text: turn.text, toolCalls: turn.toolCalls, ...(turn.replay ? { replay: turn.replay } : {}) }); const results: Array<{ toolCallId: string; toolName: string; output: string }> = []; for (const [index, toolCall] of turn.toolCalls.entries()) { throwIfRunStopped(); const output = index >= MAX_TOOL_CALLS_PER_TURN ? `ERROR: not run: this turn asked for ${turn.toolCalls.length} tool calls; the limit is ${MAX_TOOL_CALLS_PER_TURN}.` - : toolCall.argumentsError ? unreadableArguments(toolCall) : await untilStopped(executeWorkspaceTool(auditCtx, toolCall)); + : toolCall.argumentsError ? unreadableArguments(toolCall) : await runAuditTool(args.deps, auditCtx, toolCall); + if (readsEvidence(toolCall) && !output.startsWith("ERROR")) evidenceRead = true; results.push({ toolCallId: toolCall.id, toolName: toolCall.name, output }); } messages.push({ role: "tool", results }); @@ -167,6 +181,20 @@ async function auditGoalCompletion( return { approved: false, reason: "the audit did not reach a verdict within its budget, which counts as not achieved" }; } +/** Whether a call opens the work itself; listings and a document's version history do not. */ +function readsEvidence(call: ToolCall): boolean { + return call.name === "read_file" || (call.name === "read_document" && call.arguments.history !== true); +} + +async function runAuditTool(deps: AgentToolDeps, ctx: AgentToolContext, call: ToolCall): Promise { + if (!AUDIT_DOCUMENT_TOOLS.has(call.name)) return untilStopped(executeWorkspaceTool(ctx, call)); + try { + return call.name === "read_document" ? readDocumentTool(deps, ctx, call.arguments) : listDocumentsTool(deps, ctx, call.arguments); + } catch (error) { + return `ERROR: ${error instanceof Error ? error.message : String(error)}`; + } +} + function parseVerdict(text: string): AuditVerdict { const reason = text.replace(/<\/?(approved|disapproved)\s*\/?>/gi, " ").replace(/\s+/g, " ").trim(); if (//i.test(text)) return { approved: true, reason: reason || "no gaps found" }; diff --git a/tests/unit/agent-goal-tools.test.ts b/tests/unit/agent-goal-tools.test.ts index c782ffb1..6ed55895 100644 --- a/tests/unit/agent-goal-tools.test.ts +++ b/tests/unit/agent-goal-tools.test.ts @@ -1,3 +1,4 @@ +import { DocumentService } from "../../src/main/services/document-service"; import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import { randomUUID } from "node:crypto"; import { tmpdir } from "node:os"; @@ -123,7 +124,10 @@ describe("complete_goal with independent audit", () => { insert into held_deliverables(id, org_id, approval_id, kind, goal_id, title, doc_type, content, state, created_at) values (?, ?, 'app_x', 'document_create', ?, 'Launch memo', 'memo', 'draft', 'held', ?) `).run(`hld_${randomUUID()}`, h.orgId, h.goalId, new Date().toISOString()); + let turns = 0; const auditor = async (): Promise => { + turns += 1; + if (turns === 1) return turnOf("Reading.", [{ id: "c1", name: "read_file", arguments: { path: "src/index.ts" } }]); hold(); return turnOf(""); }; @@ -133,6 +137,53 @@ describe("complete_goal with independent audit", () => { h.connection.close(); }); + test("an approval that read nothing is sent back to inspect, and alone never closes the goal", async () => { + const h = createHarness(); + const prompts: string[] = []; + const auditor = async (input: AuditorInput): Promise => { + const last = input.messages.at(-1); + prompts.push(last && "text" in last ? String(last.text) : ""); + return turnOf("Looks done. "); + }; + const output = await completeGoalTool(argsFor(h, h.ceoId, auditor, call({ evidence: "Trust me." }))); + expect(output).toContain("audit rejected"); + expect(goalStatus(h)).toBe("active"); + expect(prompts[1]).toBe("You approved without reading any evidence. Open the work itself with read_file or read_document, then decide again."); + h.connection.close(); + }); + + test("a failed read is not inspection; reading the document the work lives in is", async () => { + const h = createHarness(); + const docId = h.connection.transaction(() => new DocumentService(h.connection.db).create({ + orgId: h.orgId, title: "Launch report", docType: "report", content: "Shipped to 40 customers.", authorAgentId: h.ceoId, + reviewerAgentId: null, projectId: null, goalId: h.goalId, taskId: null, + })); + const replay = { format: "anthropic" as const, content: [{ type: "thinking", thinking: "unsure", signature: "SIG" }] }; + const turns = [ + turnOf("Reading.", [{ id: "c1", name: "read_file", arguments: { path: "no-such-file.md" } }]), + { ...turnOf(""), replay }, + turnOf("History only.", [{ id: "c3", name: "read_document", arguments: { doc_id: docId, history: true } }]), + turnOf(""), + turnOf("Reading the report.", [{ id: "c2", name: "read_document", arguments: { doc_id: docId } }]), + turnOf("The report shows customers. "), + ]; + const seen: string[][] = []; + const histories: AuditorInput["messages"][] = []; + const auditor = async (input: AuditorInput): Promise => { + seen.push(input.tools.map((tool) => tool.name)); + histories.push([...input.messages]); + return turns.shift()!; + }; + const output = await completeGoalTool(argsFor(h, h.ceoId, auditor, call({ evidence: "See the launch report." }))); + expect(output).toContain("achieved"); + expect(goalStatus(h)).toBe("achieved"); + expect(seen[0]).toEqual(expect.arrayContaining(["read_file", "list_documents", "read_document"])); + expect(seen[0]).not.toContain("update_document"); + // The sent-back approval keeps its reasoning for the next turn. + expect(histories[2].at(-2)).toMatchObject({ role: "assistant", replay }); + h.connection.close(); + }); + test("no explicit approval means not achieved", async () => { const h = createHarness(); const auditor = async (): Promise => turnOf("Looks plausible I guess."); @@ -147,7 +198,7 @@ describe("complete_goal with independent audit", () => { let auditorCalls = 0; const auditor = async (): Promise => { auditorCalls += 1; - return turnOf(""); + return auditorCalls % 2 === 1 ? turnOf("Reading.", [{ id: `c${auditorCalls}`, name: "read_file", arguments: { path: "src/index.ts" } }]) : turnOf(""); }; // Staff cannot complete a goal they do not own. @@ -166,7 +217,7 @@ describe("complete_goal with independent audit", () => { await completeGoalTool(argsFor(h, h.ceoId, auditor, call({ evidence: "src/index.ts works" }))); expect(goalStatus(h)).toBe("achieved"); expect(await completeGoalTool(argsFor(h, h.ceoId, auditor, call({ evidence: "again" })))).toContain("already achieved"); - expect(auditorCalls).toBe(1); + expect(auditorCalls).toBe(2); h.connection.close(); }); });