From 2c346ee2b8e0baaf82f52716c11ff3b265694289 Mon Sep 17 00:00:00 2001 From: Tim Lingo <1timlingo@gmail.com> Date: Thu, 16 Jul 2026 23:02:07 -0500 Subject: [PATCH] =?UTF-8?q?fix(engine):=20BUG-6=20=E2=80=94=20approved=20w?= =?UTF-8?q?rites=20must=20land,=20and=20say=20where=20(false-receipt=20kil?= =?UTF-8?q?l)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two compounding defects made every pause->approve write_file report success while writing NOTHING: 1. The naive json_get scanner matches "content" anywhere in the approve body — including INSIDE tool_input, which for write_file always carries a content field. The handler therefore treated every approved builtin write as already-client-executed, skipped dispatch entirely, and handed the model the file's own content as the 'tool result'. The model then narrated 'Done, created' — a false receipt with no file. Builtin tools now ALWAYS dispatch server-side; client content is only honored for non-builtin (MCP/client-executed) tools. Stricter only. 2. write_file returned {"ok":true} unconditionally — fs_write's outcome was never checked, so any failed write also reported success. The write now verifies the file landed (fs_exists) and returns the RESOLVED path in the ok payload; failures return a real error naming the destination. E2E on the test brain (boot 38): approve-path write lands byte-exact and the result carries the resolved path; auto-run writes unchanged; denied writes execute nothing. BUG-5 (approve wire lacked tool_name) had been masking this one — two stacked bugs on the same path. NOTE for review: the deeper cure is a nesting-aware json reader; this fix removes the dangerous consequence at the two spots that lie about disk. Co-Authored-By: Claude Fable 5 --- chat.el | 12 ++++++++++-- sessions.el | 8 +++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/chat.el b/chat.el index 776a636..05a97bd 100644 --- a/chat.el +++ b/chat.el @@ -1680,8 +1680,16 @@ 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) - return json_safe("{\"ok\":true}") + // BUG-6 fix (2026-07-17): never claim ok without disk truth. fs_write's result was + // never checked, so a failed write reported ok — the exact false-receipt failure + // the run guards exist to kill. Verify the file landed and return the RESOLVED + // path so callers and the model can only narrate what is really on disk. + let dest: String = resolve_in_root(path, root) + fs_write(dest, content) + if !fs_exists(dest) { + return json_safe("{\"error\":\"write failed - nothing landed at " + dest + "\"}") + } + return json_safe("{\"ok\":true,\"path\":\"" + dest + "\"}") } if str_eq(tool_name, "web_get") { let url: String = json_get(tool_input, "url") diff --git a/sessions.el b/sessions.el index 6e31343..d125f00 100644 --- a/sessions.el +++ b/sessions.el @@ -713,7 +713,13 @@ fn handle_session_approve(session_id: String, body: String) -> String { // For builtin tools with no client-provided content: fall back to // dispatch_tool so those tools still execute correctly. let client_content: String = json_get(body, "content") - let use_client_content: Bool = !str_eq(client_content, "") + // BUG-6 fix (2026-07-17): the naive json_get scanner matches "content" ANYWHERE + // in the body — including INSIDE tool_input — so every approved write_file (whose + // input always carries a content field) was mistaken for client-executed, never + // dispatched, and narrated as done: a false receipt with no file on disk. Builtin + // tools now ALWAYS dispatch server-side; client content is only honored for + // non-builtin (MCP/client-executed) tools. Stricter only. + let use_client_content: Bool = !str_eq(client_content, "") && !is_builtin_tool(approve_tool_name) let use_dispatch: Bool = is_builtin_tool(approve_tool_name) && !use_client_content let raw_input: String = json_get_raw(body, "tool_input") let eff_input: String = if str_eq(raw_input, "") { "{}" } else { raw_input }