Skip to content

Commit 2667ba5

Browse files
junkyard22claude
andcommitted
fix: targeted write_file repair nudge and agent loop hardening
- repairLoop: inject specific write_file directive when TOOL_MISSING is for write_file, rather than generic "call tools" nudge that Brain interprets as permission to keep analyzing - ReactAgentAdapter: fix needsFileWrite and task guards to check intent + goals (repair tasks use intent="repair", file info lives in goals) - ReactAgentAdapter: fix iteration-ceiling warning to fire 3 turns before end instead of requiring 30 cumulative tool calls (never triggered) - ReactAgentAdapter: add post-loop write rescue — if model produced a FINAL ANSWER but refused to call write_file, rescue writes the file - pappy evaluator: tool_event ACs now check filesChanged instead of keyword-matching output text - toolResults: write_file TOOL_MISSING is now HIGH severity (forces repair) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c26346c commit 2667ba5

4 files changed

Lines changed: 125 additions & 25 deletions

File tree

apps/desktop/src/agents/ReactAgentAdapter.ts

Lines changed: 77 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,10 @@ export class ReactAgentAdapter implements AgentAdapter {
277277
let cumulativeToolCallCount = 0;
278278
let toolLimitWarningInjected = false;
279279
let finalAnswerFound = false;
280+
// Capture the most recent FINAL ANSWER that was rejected by the write_file guard.
281+
// Used by the post-loop rescue to save the model's analysis even when it never
282+
// called write_file on its own.
283+
let lastRejectedFinalAnswer = '';
280284

281285
const availableTools = tools;
282286
const toolSchemaLines = availableTools.flatMap((tool) => {
@@ -314,7 +318,11 @@ export class ReactAgentAdapter implements AgentAdapter {
314318

315319
// When the task involves writing a file, put a hard imperative first so
316320
// even models that skip the system prompt can't miss it.
317-
const needsFileWrite = /\.[a-z]{2,4}(\s|$)/i.test(task.intent);
321+
// Check both intent AND goals — repair tasks use intent="repair" so the
322+
// file extension only appears in the goals list.
323+
const goalText = task.goals.join(' ');
324+
const intentAndGoals = `${task.intent} ${goalText}`;
325+
const needsFileWrite = /\.[a-z]{2,4}(\s|$)/i.test(intentAndGoals);
318326
const fileWriteDirective = needsFileWrite
319327
? "**ACTION REQUIRED: Call write_file to save the file content to disk. Do NOT output the file content inline — use the tool.**\n\n"
320328
: "";
@@ -521,34 +529,39 @@ export class ReactAgentAdapter implements AgentAdapter {
521529
finalAnswerFound = true;
522530
}
523531

524-
// Tool Use Discipline: warn when approaching the tool ceiling so the model wraps up.
525-
if (cumulativeToolCallCount >= this.maxIterations * 3 && !toolLimitWarningInjected && !finalAnswerFound) {
526-
// Inject warning message - do NOT execute any more tools this iteration
532+
// Tool Use Discipline: warn when approaching the iteration ceiling so the model wraps up.
533+
// Fire 3 iterations before the end so the model has time to call write_file AND produce
534+
// a FINAL ANSWER — two distinct operations that each need their own turn.
535+
// (The old cumulativeToolCallCount-based trigger never fired for 10-iteration runs
536+
// because it required 30 tool calls, which is 3× the iteration budget.)
537+
const iterationsRemaining = this.maxIterations - iteration - 1;
538+
if (iterationsRemaining === 2 && !toolLimitWarningInjected && !finalAnswerFound) {
527539
toolLimitWarningInjected = true;
528-
messages.push({ role: "user", content: "You have used many tools without producing a final answer. Do not call any more tools. Your next response must be your final answer." });
529-
continue; // Skip tool execution, wait for model's final answer next iteration
530-
}
531-
532-
// If warning was already injected and this is the next iteration, extract output as final
533-
if (toolLimitWarningInjected && !finalAnswerFound) {
534-
// This is the iteration after warning injection - extract whatever we have as final answer
535-
currentOutputText = stripThoughtBlocks(cleanedOutput);
536-
stoppedBecause = 'done';
537-
ctx.recordTrace?.("agent.final_answer.fallback", {
538-
role: this.role,
539-
iteration: iterationCount,
540-
outputText: currentOutputText,
540+
const allTaskTextForWarn = `${task.intent} ${task.goals.join(' ')} ${task.doneCriteria.join(' ')}`;
541+
const taskImpliesFileSaveForWarn =
542+
/\b(save|write|creat)\w*.{0,40}\.\w{2,6}/i.test(allTaskTextForWarn) ||
543+
/\b(save|write).{0,20}(report|file|output|result|summary|plan|audit)/i.test(allTaskTextForWarn);
544+
const writeFileCalledForWarn = toolsUsed.some(e => e.tool === 'write_file');
545+
const writeFileReminder = taskImpliesFileSaveForWarn && !writeFileCalledForWarn
546+
? '\n\nCRITICAL: Your task requires saving a file. You MUST call write_file with the complete file content in your NEXT response — before writing your final answer.'
547+
: '';
548+
messages.push({
549+
role: "user",
550+
content: `You have 2 turns remaining.${writeFileReminder}\nAfter any required tool call (e.g. write_file), your next response MUST be your FINAL ANSWER using the FINAL ANSWER: marker.`,
541551
});
542-
break;
552+
continue; // give the model a clean turn to act on this warning
543553
}
544554

545555
if (toolCalls.length === 0) {
546556
if (finalAnswer) {
547557
// Guard: if the task clearly requires file/repo investigation and no tools
548558
// were called at all, the model skipped the work — reject the premature answer.
559+
// Check both intent and goals — repair tasks use intent="repair", so file/tool
560+
// indicators only appear in the goals list.
561+
const allTaskText = `${task.intent} ${task.goals.join(' ')}`;
549562
const taskImpliesToolUse = (
550-
/\b(analyz|investigat|examin|read|list|search|find|save|write|creat)\w*/i.test(task.intent) ||
551-
/\b\w+\.\w{2,5}\b/.test(task.intent) // explicit filename with extension
563+
/\b(analyz|investigat|examin|read|list|search|find|save|write|creat)\w*/i.test(allTaskText) ||
564+
/\b\w+\.\w{2,5}\b/.test(allTaskText) // explicit filename with extension
552565
);
553566
if (taskImpliesToolUse && cumulativeToolCallCount === 0) {
554567
messages.push({
@@ -565,11 +578,14 @@ export class ReactAgentAdapter implements AgentAdapter {
565578
// Guard: if the task explicitly asks to save output to a file, write_file
566579
// must have been called before we accept a FINAL ANSWER.
567580
const taskImpliesFileSave = (
568-
/\b(save|write|creat)\w*.{0,40}\.\w{2,6}/i.test(task.intent) ||
569-
/\b(save|write).{0,20}(report|file|output|result|summary|plan|audit)/i.test(task.intent)
581+
/\b(save|write|creat)\w*.{0,40}\.\w{2,6}/i.test(allTaskText) ||
582+
/\b(save|write).{0,20}(report|file|output|result|summary|plan|audit)/i.test(allTaskText)
570583
);
571584
const writeFileCalled = toolsUsed.some(e => e.tool === 'write_file');
572585
if (taskImpliesFileSave && !writeFileCalled) {
586+
// Save this final answer — if the model keeps refusing to call write_file,
587+
// the post-loop rescue will use this content to write the file directly.
588+
lastRejectedFinalAnswer = finalAnswer;
573589
messages.push({
574590
role: 'user',
575591
content:
@@ -603,7 +619,10 @@ export class ReactAgentAdapter implements AgentAdapter {
603619
&& !/\n/.test(trimmed)
604620
&& /^(Good|OK|Alright|Let me|I('ll| will| need| want| should)|Now |First)/im.test(trimmed);
605621
const isSubstantive = trimmed.length > 0 && !looksLikePureThinking;
606-
if (isSubstantive) {
622+
// Only overwrite currentOutputText if the new text is longer — prevents
623+
// a short thinking sentence like "I've gathered enough information" from
624+
// clobbering a longer analysis the model produced in an earlier iteration.
625+
if (isSubstantive && trimmed.length > currentOutputText.trim().length) {
607626
currentOutputText = extractedText;
608627
}
609628
// Do not exit — continue to next iteration to allow model to retry
@@ -821,6 +840,41 @@ export class ReactAgentAdapter implements AgentAdapter {
821840
}
822841
}
823842

843+
// ── Post-loop write rescue ────────────────────────────────────────────────
844+
// If the task required a file write, the model never called write_file, but
845+
// it DID produce a FINAL ANSWER (which was rejected by the guard), call
846+
// write_file now using that content. This handles models that understand the
847+
// task but consistently refuse to emit <tool_call> blocks on their own.
848+
const allTaskTextForRescue = `${task.intent} ${task.goals.join(' ')} ${task.doneCriteria.join(' ')}`;
849+
const taskImpliesFileSaveRescue =
850+
/\b(save|write|creat)\w*.{0,40}\.\w{2,6}/i.test(allTaskTextForRescue) ||
851+
/\b(save|write).{0,20}(report|file|output|result|summary|plan|audit)/i.test(allTaskTextForRescue);
852+
const writeFileCalledRescue = toolsUsed.some(e => e.tool === 'write_file');
853+
854+
if (taskImpliesFileSaveRescue && !writeFileCalledRescue && lastRejectedFinalAnswer.trim().length > 100) {
855+
// Extract the target file path from doneCriteria or task text.
856+
const pathMatch =
857+
allTaskTextForRescue.match(/\b([\w.\-/]+\.(?:md|txt|ts|js|json|yaml|yml|py|sh|csv))\b/i);
858+
const targetPath = pathMatch?.[1] ?? '';
859+
const writeTool = availableTools.find(t => t.name === 'write_file');
860+
if (targetPath && writeTool) {
861+
try {
862+
ctx.recordTrace?.("agent.postloop_write_rescue", { targetPath, contentLength: lastRejectedFinalAnswer.length });
863+
const writeResult = await writeTool.execute(
864+
{ path: targetPath, content: lastRejectedFinalAnswer },
865+
{ workspaceRoot: ctx.workspaceRoot },
866+
);
867+
toolsUsed.push({ tool: 'write_file', ok: writeResult.ok, summary: writeResult.ok ? `Rescued: wrote ${targetPath}` : writeResult.error ?? 'write failed' });
868+
if (writeResult.ok) {
869+
filesChanged.push({ path: targetPath, changeType: 'A' });
870+
currentOutputText = lastRejectedFinalAnswer;
871+
}
872+
} catch (rescueErr) {
873+
ctx.recordTrace?.("agent.postloop_write_rescue.error", { error: String(rescueErr) });
874+
}
875+
}
876+
}
877+
824878
const cleanedLastOutput = stripToolCalls(lastModelOutput);
825879
// Prefer accumulated output (set on each iteration) over a fresh strip of
826880
// the last response — if stripping left nothing, fall back to the raw

packages/orca-core/src/repairLoop.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,18 @@ export async function handleRepairLoop(
8181
const hasToolMissing = currentQC.issues.some(
8282
(i) => i.code === 'TOOL_MISSING' || /tool.*(missing|not.*call)/i.test(i.message ?? i.description ?? ''),
8383
);
84-
const toolNudge = hasToolMissing
84+
// Detect the specific case where write_file was the missing tool.
85+
// The generic nudge lets Brain interpret "call tools" as permission to call
86+
// read_file again and keep analyzing. write_file needs a targeted directive.
87+
const hasWriteFileMissing = currentQC.issues.some(
88+
(i) => (i.code === 'TOOL_MISSING') && /write_file/.test(i.description ?? i.message ?? i.evidence ?? ''),
89+
);
90+
const toolNudge = hasWriteFileMissing
91+
? `\n\nCRITICAL: This task requires writing a file to disk. You MUST call write_file as your NEXT action. ` +
92+
`Do not analyze, do not explain, do not summarize. Your ONLY valid next action is:\n\n` +
93+
`<tool_call>\n{"tool": "write_file", "path": "[target file from task]", "content": "[your complete report content]"}\n</tool_call>\n\n` +
94+
`Call write_file NOW. Nothing else is acceptable.`
95+
: hasToolMissing
8596
? `\n\nCRITICAL: The previous attempt completed WITHOUT calling any tools. ` +
8697
`You MUST use your available tools (read_file, write_file, list_directory, etc.) ` +
8798
`to actually perform the work. Do NOT just describe what you would do — ` +
@@ -94,6 +105,15 @@ export async function handleRepairLoop(
94105
goals: [
95106
...originalTask.goals,
96107
"Fix all issues identified in the quality check — produce the corrected output, not a description of fixes",
108+
// Surface the tool nudge in goals so the specialist agent sees it directly
109+
// (originalUserMessage is only used for brain routing, not passed to the agent).
110+
...(hasWriteFileMissing ? [
111+
"CRITICAL: Your ONLY valid next action is to call write_file. " +
112+
"Do NOT read files, do NOT analyze, do NOT explain. Call write_file immediately.",
113+
] : hasToolMissing ? [
114+
"CRITICAL: You MUST call write_file and any other required tools. " +
115+
"Do NOT describe what you would do — use <tool_call> blocks to actually call the tools.",
116+
] : []),
97117
],
98118
constraints: originalTask.constraints,
99119
permissions: originalTask.permissions,

packages/pappy-core/src/ahp/evaluator.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,28 @@ function checkACAgainstOutput(
7878
return { passed: false, evidence: "no output" };
7979
}
8080

81+
// tool_event requirement: AC asserts a specific tool was called (Brain done_criteria pattern).
82+
// Keyword matching on outputText is insufficient — the model often MENTIONS the tool name
83+
// without actually calling it. Require the file to appear in filesChanged instead.
84+
if (/tool_event/i.test(acText)) {
85+
const filePathMatch =
86+
acText.match(/creating\s+([\w.\-\/]+)/i) ??
87+
acText.match(/\b([\w.\-]+\.[\w]{1,6})\b/);
88+
const p = filePathMatch?.[1] ?? "";
89+
const touched = (input.filesChanged ?? []).map((f) => f.path);
90+
if (p) {
91+
if (touched.some((t) => t.endsWith(p) || t === p)) {
92+
return { passed: true, evidence: `file present: ${p}` };
93+
}
94+
return { passed: false, evidence: `tool_event AC: file not created: ${p}` };
95+
}
96+
// No specific file — require at least one file change
97+
if (touched.length > 0) {
98+
return { passed: true, evidence: "files were changed" };
99+
}
100+
return { passed: false, evidence: "tool_event AC: no files changed" };
101+
}
102+
81103
// File-path requirement: `File "x" must be ...`
82104
const fileReq = acText.match(/[Ff]ile ["`']?([^"`'\s,]+)["`']?/);
83105
if (fileReq) {

packages/pappy-core/src/checks/toolResults.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,12 @@ export function runToolResultChecks(input: PappyInput): Omit<Issue, "issueId">[]
108108
const satisfiedByFiles = expected.satisfiedByFilesChanged === true && hasFilesChanged;
109109

110110
if (!toolWasCalled && !satisfiedByFiles) {
111+
// write_file missing is HIGH — the task explicitly required a file to be
112+
// created and it wasn't. This forces FAIL → repair rather than WARN → ship.
113+
// Other missing tools stay MEDIUM (heuristic, may have false positives).
114+
const severity = expected.tool === "write_file" ? "HIGH" : "MEDIUM";
111115
issues.push({
112-
severity: "MEDIUM",
116+
severity,
113117
code: "TOOL_MISSING",
114118
category: "Tooling",
115119
description: `Task implies using "${expected.tool}" but no such tool was called.`,

0 commit comments

Comments
 (0)