From 70646f867ccc3db2ff34ce9cb5d48ffa2c4818ec Mon Sep 17 00:00:00 2001 From: Tim Lingo <1timlingo@gmail.com> Date: Wed, 22 Jul 2026 16:19:09 -0500 Subject: [PATCH] fix(chat): agent write_file/edit_file no longer return false success receipts (BUG-29) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: dispatch_tool's write_file returned {"ok":true} without checking fs_write's result, and edit_file returned ok:true even when old_text was absent (str_replace silently no-ops) and its fs_write was also unchecked. Any failed or no-op write fed the model a false success receipt, which it then repeated to the user as fact. The change (Receipt Contract rule 1 — a tool result must reflect what actually happened): - write_file: check fs_write's return (1 = all bytes written, 0 = fail); on failure return {"error":"write failed"} in the handler's existing error-JSON shape. - edit_file: reject empty old_text, verify old_text is actually present (str_contains) before replacing, and check the fs_write result the same way. - Verification is by operation result, NOT an fs_read read-back: fs_read arms the runtime's one-shot binary send length, the exact mechanism that truncated the safety-contact response (#96). Same honest-write pattern as that fix. E2E evidence (sandboxed elb build, dispatch_tool driven directly): - unpatched: write_file into a chmod-000 dir -> {"ok":true} (lie); edit_file with absent old_text -> {"ok":true} (lie, file untouched) - patched: same calls -> {"error":"write failed"} / {"error":"old_text not found in file"}; happy paths still ok:true - scripts/verify-soul-contract.sh on the patched soul: GATE PASS (27/27 presence + immutability) Co-Authored-By: Claude Fable 5 --- chat.el | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/chat.el b/chat.el index d5e3b59..9dcaad4 100644 --- a/chat.el +++ b/chat.el @@ -1706,7 +1706,17 @@ fn dispatch_tool(tool_name: String, tool_input: String) -> String { if !path_within_root(path, root) { return json_safe("denied: path is outside the agent workspace root") } - fs_write(resolve_in_root(path, root), content) + // BUG-29 (Receipt Contract rule 1): fs_write's result was ignored, so a + // failed write (missing/unwritable dir, disk full) still returned + // {"ok":true} — a false receipt fed straight to the model. Check the + // return (1 = all bytes written, 0 = fail), the same honest-write pattern + // as the safety-contact fix. A read-back via fs_read is deliberately NOT + // used here: fs_read arms the runtime's one-shot binary send length, + // which truncated longer HTTP responses (the #96 failure mode). + let write_ok: Int = fs_write(resolve_in_root(path, root), content) + if write_ok == 0 { + return json_safe("{\"error\":\"write failed\"}") + } return json_safe("{\"ok\":true}") } if str_eq(tool_name, "web_get") { @@ -1784,8 +1794,22 @@ fn dispatch_tool(tool_name: String, tool_input: String) -> String { if str_eq(content, "") { return json_safe("{\"error\":\"file not found\"}") } + // BUG-29 (Receipt Contract rule 1): when old_text was absent (or empty), + // str_replace was a silent no-op and the handler still claimed ok:true — + // a false receipt. Verify the text is actually present before replacing. + if str_eq(old_text, "") { + return json_safe("{\"error\":\"old_text is required\"}") + } + if !str_contains(content, old_text) { + return json_safe("{\"error\":\"old_text not found in file\"}") + } let updated: String = str_replace(content, old_text, new_text) - fs_write(resolved, updated) + // BUG-29: the fs_write result was also unchecked — a failed write still + // returned ok:true. Same honest-write check as write_file above. + let write_ok: Int = fs_write(resolved, updated) + if write_ok == 0 { + return json_safe("{\"error\":\"write failed\"}") + } return json_safe("{\"ok\":true}") } if str_eq(tool_name, "remember") {