Q115: a goal closes only with its tasks done and nothing its verdict rests on changed during the audit
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
abfcbf4824
commit
938c8cad79
4 files changed
+244
-11
No files matched your search
@@ -11,7 +11,7 @@ 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 { goalAuditor, resultPointers, type GoalAuditor, type ResultPointers } from "./goal-audit-inputs";
|
||||
import { AuditBasis, goalAuditor, resultPointers, unfinishedGoalTasks, type GoalAuditor, type ResultPointers } from "./goal-audit-inputs";
|
||||
import { DOCUMENT_TOOLS, listDocumentsTool, readDocumentTool } from "./agent-document-tools";
|
||||
|
||||
const AUDIT_MAX_ITERATIONS = 6;
|
||||
@@ -84,16 +84,28 @@ export async function completeGoalTool(args: GoalCompletionArgs): Promise<string
|
||||
}
|
||||
if (goal.status !== "active") return `Goal "${goal.title}" is already ${goal.status}.`;
|
||||
if (heldWorkTitles(deps.db, { goalId: goal.id }).length > 0) return heldGoalRefusal(deps, goal);
|
||||
const unfinished = unfinishedGoalTasks(deps.db, goal.id, ctx.taskId);
|
||||
if (unfinished.length > 0) return unfinishedRefusal(goal, unfinished);
|
||||
const pointers = resultPointers(deps.db, ctx, call.arguments);
|
||||
const auditor = goalAuditor(deps.db, ctx, args.providerId);
|
||||
const basis = new AuditBasis(deps.db, goal.id, ctx.workspace?.localPath ?? null, pointers);
|
||||
|
||||
const verdict = await auditGoalCompletion(args, goal, pointers, auditor);
|
||||
const verdict = await auditGoalCompletion(args, goal, pointers, auditor, basis);
|
||||
if (!verdict.approved) {
|
||||
return `The independent audit rejected closing the goal: ${verdict.reason} The goal stays active — close that gap or create the tasks that will.`;
|
||||
}
|
||||
throwIfRunStopped();
|
||||
// Work held for review while the audit ran keeps the goal open too.
|
||||
// Work held for review, tasks opened, or the goal or result changed while the audit ran: the verdict no longer covers what exists.
|
||||
if (heldWorkTitles(deps.db, { goalId: goal.id }).length > 0) return heldGoalRefusal(deps, goal);
|
||||
const openedMeanwhile = unfinishedGoalTasks(deps.db, goal.id, ctx.taskId);
|
||||
if (openedMeanwhile.length > 0) return unfinishedRefusal(goal, openedMeanwhile);
|
||||
const edited = basis.editedInCopy();
|
||||
if (edited.length > 0) {
|
||||
return `ERROR: The auditor's commands changed ${edited.slice(0, 5).join(", ")} in its copy of the workspace, so its verdict is not about the real work; call complete_goal again.`;
|
||||
}
|
||||
if (basis.changed()) {
|
||||
return "ERROR: The goal or the result it points to changed during the audit; call complete_goal again so the audit judges what exists now.";
|
||||
}
|
||||
deps.db.prepare("update goals set status = 'achieved', stalled_reason = null, updated_at = ? where id = ? and org_id = ?")
|
||||
.run(new Date().toISOString(), goal.id, ctx.orgId);
|
||||
new AuditService(deps.db).record({
|
||||
@@ -111,6 +123,10 @@ export async function completeGoalTool(args: GoalCompletionArgs): Promise<string
|
||||
}
|
||||
}
|
||||
|
||||
function unfinishedRefusal(goal: { title: string }, titles: string[]): string {
|
||||
return `ERROR: Goal "${goal.title}" still has unfinished tasks (${titles.join(", ")}); finish or close them first.`;
|
||||
}
|
||||
|
||||
function heldGoalRefusal(deps: GoalCompletionArgs["deps"], goal: { id: string; title: string }): string {
|
||||
const held = heldWorkTitles(deps.db, { goalId: goal.id });
|
||||
return `ERROR: Goal "${goal.title}" has work awaiting board review (${held.join(", ")}); it is not written yet, so the goal cannot be achieved.`;
|
||||
@@ -126,15 +142,20 @@ async function auditGoalCompletion(
|
||||
goal: { title: string; description: string },
|
||||
pointers: ResultPointers,
|
||||
auditor: GoalAuditor,
|
||||
basis: AuditBasis,
|
||||
): Promise<AuditVerdict> {
|
||||
const workspace = args.ctx.workspace;
|
||||
// With the operator's file-access approval the auditor may build and test, but only in a throwaway copy.
|
||||
const scratch = workspace?.writeAccess ? copyWorkspace(workspace.localPath) : null;
|
||||
try {
|
||||
if (scratch) basis.auditInCopy(scratch);
|
||||
const auditWorkspace = workspace ? { ...workspace, localPath: scratch ?? workspace.localPath, writeAccess: scratch !== null } : null;
|
||||
return await runAudit(args, goal, pointers, auditor, auditWorkspace);
|
||||
return await runAudit(args, goal, pointers, auditor, basis, auditWorkspace);
|
||||
} finally {
|
||||
if (scratch) rmSync(scratch, { recursive: true, force: true });
|
||||
if (scratch) {
|
||||
basis.closeCopy();
|
||||
rmSync(scratch, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -155,6 +176,7 @@ async function runAudit(
|
||||
goal: { title: string; description: string },
|
||||
pointers: ResultPointers,
|
||||
auditor: GoalAuditor,
|
||||
basis: AuditBasis,
|
||||
workspace: AgentToolContext["workspace"],
|
||||
): Promise<AuditVerdict> {
|
||||
const canRun = workspace?.writeAccess === true;
|
||||
@@ -175,7 +197,7 @@ async function runAudit(
|
||||
"The executor points to these as the result. Verify them yourself:",
|
||||
`Files: ${pointers.files.length > 0 ? pointers.files.join(", ") : "none"}`,
|
||||
`Documents: ${pointers.documents.length > 0 ? pointers.documents.map((doc) => `${doc.title} (doc_id=${doc.id})`).join(", ") : "none"}`,
|
||||
`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. ${canRun ? "run_command runs in a throwaway copy of the workspace: build and run the tests yourself instead of trusting claims that they pass; nothing you do there reaches the real workspace." : "You cannot run commands here, so a claim that something was run or passes is unverified unless the workspace holds its results."} 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. ${canRun ? "run_command runs in a throwaway copy of the workspace: build and run the tests yourself instead of trusting claims that they pass; nothing you do there reaches the real workspace. Never edit the work there: if your commands change any file that was already in the workspace, the verdict is void." : "You cannot run commands here, so a claim that something was run or passes is unverified unless the workspace holds its results."} 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.`,
|
||||
"Keep your reasoning apart from your decision. End your final message with the decision alone on its last line: <approved/> by itself if the goal is genuinely achieved, or <disapproved/> followed by one short sentence naming the decisive gap. Use no marker anywhere else.",
|
||||
].join("\n\n"),
|
||||
}];
|
||||
@@ -207,7 +229,11 @@ async function runAudit(
|
||||
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 runAuditTool(args.deps, auditCtx, toolCall);
|
||||
if (readsEvidence(toolCall) && !output.startsWith("ERROR")) evidenceRead = true;
|
||||
if (readsEvidence(toolCall) && !output.startsWith("ERROR")) {
|
||||
evidenceRead = true;
|
||||
if (toolCall.name === "read_file") basis.noteFile(String(toolCall.arguments.path));
|
||||
else basis.noteDocument(String(toolCall.arguments.doc_id));
|
||||
}
|
||||
results.push({ toolCallId: toolCall.id, toolName: toolCall.name, output });
|
||||
}
|
||||
messages.push({ role: "tool", results });
|
||||
|
||||
@@ -9,7 +9,7 @@ import { PAGE_TOOL_PROPERTIES, pageText, readPageArgs, type PageRequest } from "
|
||||
|
||||
const LIST_FILES_LIMIT = 400;
|
||||
/** Dependency and VCS internals: huge, machine-managed, never what an agent is looking for. */
|
||||
const LIST_SKIPPED_DIRECTORIES = new Set([".git", "node_modules", ".pnpm-store", "out", "dist"]);
|
||||
export const LIST_SKIPPED_DIRECTORIES = new Set([".git", "node_modules", ".pnpm-store", "out", "dist"]);
|
||||
|
||||
/**
|
||||
* The workspace tools turn agents from document writers into workers: they can
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { existsSync } from "node:fs";
|
||||
import { createHash } from "node:crypto";
|
||||
import { existsSync, readdirSync, readFileSync, realpathSync } from "node:fs";
|
||||
import { join, relative, sep } from "node:path";
|
||||
import type { DatabaseSync } from "node:sqlite";
|
||||
import { optionalStringArray } from "./agent-tool-args";
|
||||
import { LIST_SKIPPED_DIRECTORIES } from "./agent-workspace-tools";
|
||||
import { inputBudgetTokens } from "./context-budget";
|
||||
import type { AgentToolContext } from "./agent-tools";
|
||||
import { resolveWorkspacePath } from "./files/workspace-path";
|
||||
@@ -9,6 +12,7 @@ import { currentRunId } from "./run-control";
|
||||
import { effectiveModel } from "./task-run-preparation";
|
||||
|
||||
const AUDITOR_SETTING = "Settings > Advanced > Goal audit";
|
||||
const UNFINISHED_SHOWN = 5;
|
||||
|
||||
export interface GoalAuditor {
|
||||
providerId: string;
|
||||
@@ -53,6 +57,116 @@ export function setGoalAuditor(db: DatabaseSync, input: { orgId: string; provide
|
||||
console.info(`[goal-audit] org ${input.orgId} audits goals with ${model} on ${input.providerId}`);
|
||||
}
|
||||
|
||||
/** Unfinished work of a goal besides the task asking to close it; bookkeeping tasks (chat wakes, reviews) do not count. */
|
||||
export function unfinishedGoalTasks(db: DatabaseSync, goalId: string, exceptTaskId: string | null): string[] {
|
||||
const rows = db.prepare(`
|
||||
select title from tasks where goal_id = ? and status <> 'done' and origin is null and (? is null or id <> ?)
|
||||
order by rowid limit ?
|
||||
`).all(goalId, exceptTaskId, exceptTaskId, UNFINISHED_SHOWN) as Array<{ title: string }>;
|
||||
return rows.map((row) => row.title);
|
||||
}
|
||||
|
||||
/** What a verdict rests on, each stamped as it entered the audit: the goal, the pointers, and what the auditor read in the real workspace. */
|
||||
export class AuditBasis {
|
||||
private readonly stamps = new Map<string, string>();
|
||||
/** The audit's copy and its file hashes taken before any command ran, i.e. the workspace as the audit found it. */
|
||||
private copy: { root: string; atStart: Map<string, string> } | null = null;
|
||||
/** Files that existed at the start and that the auditor's own commands changed or removed in the copy. */
|
||||
private editedFiles: string[] = [];
|
||||
|
||||
constructor(private readonly db: DatabaseSync, private readonly goalId: string, private readonly workspaceRoot: string | null, pointers: ResultPointers) {
|
||||
this.note("goal:");
|
||||
for (const file of pointers.files) this.noteFile(file);
|
||||
for (const document of pointers.documents) this.noteDocument(document.id);
|
||||
}
|
||||
|
||||
/** When the auditor works in a copy, a read file is stamped as the copy held it at the start: what the auditor saw. */
|
||||
auditInCopy(copyRoot: string): void {
|
||||
this.copy = { root: copyRoot, atStart: hashTree(copyRoot) };
|
||||
}
|
||||
|
||||
noteFile(path: string): void {
|
||||
if (!this.workspaceRoot) return;
|
||||
const relative = canonicalRelative(this.copy?.root ?? this.workspaceRoot, path.trim());
|
||||
const key = `file:${relative}`;
|
||||
if (this.stamps.has(key)) return;
|
||||
this.stamps.set(key, this.copy ? this.copy.atStart.get(relative) ?? this.absentStamp(relative) : this.stamp(key));
|
||||
}
|
||||
|
||||
/** Called before the copy is deleted: any file the audit found that its own commands changed means it judged its own edits. */
|
||||
closeCopy(): void {
|
||||
if (!this.copy) return;
|
||||
const now = hashTree(this.copy.root);
|
||||
this.editedFiles = [...this.copy.atStart].filter(([path, hash]) => now.get(path) !== hash).map(([path]) => path);
|
||||
}
|
||||
|
||||
editedInCopy(): string[] {
|
||||
return this.editedFiles;
|
||||
}
|
||||
|
||||
noteDocument(id: string): void {
|
||||
this.note(`doc:${id.trim()}`);
|
||||
}
|
||||
|
||||
/** Whether anything the verdict rests on is different now. */
|
||||
changed(): boolean {
|
||||
return [...this.stamps].some(([key, stamp]) => this.stamp(key) !== stamp);
|
||||
}
|
||||
|
||||
private note(key: string): void {
|
||||
if (!this.stamps.has(key)) this.stamps.set(key, this.stamp(key));
|
||||
}
|
||||
|
||||
private stamp(key: string): string {
|
||||
if (key === "goal:") return JSON.stringify(this.db.prepare("select title, description, status from goals where id = ?").get(this.goalId));
|
||||
if (key.startsWith("doc:")) {
|
||||
const row = this.db.prepare("select version from documents where id = ?").get(key.slice(4)) as { version: number } | undefined;
|
||||
return row ? `v${row.version}` : "missing";
|
||||
}
|
||||
const relative = key.slice(5);
|
||||
if (inSkippedDirectory(relative)) return "untracked";
|
||||
const absolute = resolveWorkspacePath(this.workspaceRoot!, relative).absolute;
|
||||
return existsSync(absolute) ? hashFile(absolute) : "missing";
|
||||
}
|
||||
|
||||
private absentStamp(relative: string): string {
|
||||
return inSkippedDirectory(relative) ? "untracked" : "missing";
|
||||
}
|
||||
}
|
||||
|
||||
/** A path as the disk spells it, so differently cased reads of one file on a case-insensitive disk share one stamp. */
|
||||
function canonicalRelative(root: string, path: string): string {
|
||||
const { absolute, relativePosix } = resolveWorkspacePath(root, path);
|
||||
if (!existsSync(absolute)) return relativePosix;
|
||||
const actual = relative(realpathSync.native(root), realpathSync.native(absolute)).split(sep).join("/");
|
||||
return actual.startsWith("..") ? relativePosix : actual;
|
||||
}
|
||||
|
||||
/** Build and dependency folders are not evidence of record; they are left out of stamps, as list_files leaves them out. */
|
||||
function inSkippedDirectory(relative: string): boolean {
|
||||
return relative.split("/").slice(0, -1).some((part) => LIST_SKIPPED_DIRECTORIES.has(part));
|
||||
}
|
||||
|
||||
function hashFile(absolute: string): string {
|
||||
return createHash("sha256").update(readFileSync(absolute)).digest("hex");
|
||||
}
|
||||
|
||||
function hashTree(root: string): Map<string, string> {
|
||||
const hashes = new Map<string, string>();
|
||||
const walk = (directory: string, prefix: string): void => {
|
||||
for (const entry of readdirSync(directory, { withFileTypes: true })) {
|
||||
const relative = prefix ? `${prefix}/${entry.name}` : entry.name;
|
||||
if (entry.isDirectory()) {
|
||||
if (!LIST_SKIPPED_DIRECTORIES.has(entry.name)) walk(join(directory, entry.name), relative);
|
||||
} else if (entry.isFile()) {
|
||||
hashes.set(relative, hashFile(join(directory, entry.name)));
|
||||
}
|
||||
}
|
||||
};
|
||||
walk(root, "");
|
||||
return hashes;
|
||||
}
|
||||
|
||||
/** Checks the executor's pointers before any audit spend: every file in the workspace and every document must exist. */
|
||||
export function resultPointers(db: DatabaseSync, ctx: AgentToolContext, args: JsonRecord): ResultPointers {
|
||||
const files = optionalStringArray(args.files, "files");
|
||||
|
||||
Reference in new issue
Block a user