From 4a2383cdacfefd2f90c515275d008a75886f955d Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 3 Sep 2026 10:37:02 +0530 Subject: [PATCH] fix: unwrap flare-gateway calls before completion-gate classification item done/check_merge's verification gate, the mutating-tool branch guard, and destructive-command detection all classify raw tool_name/tool_input as Claude Code sends it. When a verification command or a ctx_patch/ctx_edit edit is routed through the flare gateway (mcp__flare__tool(action="execute", server="leanctx", tool=..., args={...})) -- which apply_gateway_permissions actively steers agents toward by stripping direct mcp__lean-ctx__* permissions -- the real command/tool sits one level deeper, under tool_input.args, where none of these classifiers looked. Result: a verification command run via ctx_shell was never recorded as evidence, so the completion gate rejected item done/check_merge on every retry even seconds after a passing test run. Gateway-routed ctx_patch/ ctx_edit edits were also invisible to the branch guard and to invalidate_verification. Adds hook_redirect::unwrap_gateway_call to unwrap a gateway action="execute" call to its real tool+args before classification, wired into both hook.rs::parse_pre_tool_use and hook_completion_gate.rs::parse_post_tool_use. Also covers ctx_shell in the destructive-command guard's Bash-family match arm, and makes output-text extraction unwrap the MCP content-block envelope so a genuine failing gateway-routed run isn't defaulted to "passed". Agentflare-Agent: claude-code_2-1-259_agent Agentflare-Branch: task/559-completion-gate-misses-verification-edit Agentflare-Item: 559 Agentflare-Session: 9ce40d77-1229-4fa6-ad25-00af35e38470 --- src/hook.rs | 10 ++- src/hook_completion_gate.rs | 65 +++++++++++++++++-- src/hook_redirect.rs | 120 +++++++++++++++++++++++++++++++++++- 3 files changed, 186 insertions(+), 9 deletions(-) diff --git a/src/hook.rs b/src/hook.rs index ad156d2ac..84f0e622e 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -229,8 +229,14 @@ struct PreToolUseInput { fn parse_pre_tool_use(input: &str) -> Option { let v: serde_json::Value = serde_json::from_str(input).ok()?; let session_id = v.get("session_id")?.as_str()?.to_string(); - let tool_name = v.get("tool_name")?.as_str()?.to_string(); - let tool_input = v.get("tool_input").cloned(); + let raw_tool_name = v.get("tool_name")?.as_str()?.to_string(); + let raw_tool_input = v.get("tool_input").cloned(); + // Unwrap flare-gateway `action="execute"` calls to the real tool they + // forward -- every classifier downstream (branch guard, completion + // gate) needs to see the actual tool/args, not the gateway envelope + // (item #559). + let (tool_name, tool_input) = + crate::hook_redirect::unwrap_gateway_call(&raw_tool_name, raw_tool_input.as_ref()); let delay_seconds = tool_input .as_ref() .and_then(|ti| ti.get("delaySeconds")) diff --git a/src/hook_completion_gate.rs b/src/hook_completion_gate.rs index 4e777a3c7..484f8689c 100644 --- a/src/hook_completion_gate.rs +++ b/src/hook_completion_gate.rs @@ -72,8 +72,17 @@ fn item_action_succeeded(action: &str, response: Option<&Value>) -> Option fn parse_post_tool_use(input: &str) -> Option { let v: serde_json::Value = serde_json::from_str(input).ok()?; let session_id = v.get("session_id")?.as_str()?.to_string(); - let tool_name = v.get("tool_name")?.as_str()?.to_string(); - let tool_input = v.get("tool_input"); + let raw_tool_name = v.get("tool_name")?.as_str()?.to_string(); + let raw_tool_input = v.get("tool_input").cloned(); + // Unwrap flare-gateway `action="execute"` calls (item #559) -- a + // verification command run via `mcp__flare__tool(action="execute", + // server="leanctx", tool="ctx_shell", args={"command": ...})` must be + // recorded exactly like a direct Bash call, or the completion gate + // never sees it as evidence and rejects `item done`/`check_merge` even + // right after a passing run. + let (tool_name, tool_input) = + crate::hook_redirect::unwrap_gateway_call(&raw_tool_name, raw_tool_input.as_ref()); + let tool_input = tool_input.as_ref(); let command = tool_input .and_then(|ti| { ti.get("command") @@ -99,11 +108,26 @@ fn parse_post_tool_use(input: &str) -> Option { .map(|r| { let stdout = r.get("stdout").and_then(Value::as_str).unwrap_or(""); let stderr = r.get("stderr").and_then(Value::as_str).unwrap_or(""); - if stdout.is_empty() && stderr.is_empty() { - r.as_str().unwrap_or_default().to_string() - } else { - format!("{stdout}\n{stderr}") + if !stdout.is_empty() || !stderr.is_empty() { + return format!("{stdout}\n{stderr}"); } + if let Some(s) = r.as_str() { + return s.to_string(); + } + // A gateway-forwarded response (item #559) comes back as the + // MCP content-block envelope (`{"content":[{"type":"text", + // "text":"..."}]}`), same shape `response_as_json` unwraps for + // item actions -- without this, exit_code and output_text are + // both blank for every gateway-routed verification command, so + // a genuine failure would default to "passed" via + // `verification_passed`'s permissive fallback below. + r.get("content") + .and_then(Value::as_array) + .and_then(|blocks| blocks.first()) + .and_then(|block| block.get("text")) + .and_then(Value::as_str) + .unwrap_or_default() + .to_string() }) .unwrap_or_default(); let item_success = item_action @@ -351,6 +375,35 @@ mod tests { assert!(parse_post_tool_use("not json").is_none()); } + #[test] + fn parse_post_tool_use_unwraps_gateway_routed_verification_command() { + // Regression for item #559: a verification command run through the + // flare gateway (`mcp__flare__tool(action="execute", server= + // "leanctx", tool="ctx_shell", args={"command": ...})`) must be + // recognized exactly like a direct Bash call. + let input = r#"{"session_id":"s1","tool_name":"mcp__flare__tool","tool_input":{"action":"execute","server":"leanctx","tool":"ctx_shell","args":{"command":"cargo test"}},"tool_response":{"content":[{"type":"text","text":"test result: ok"}]}}"#; + let parsed = parse_post_tool_use(input).unwrap(); + assert_eq!(parsed.tool_name, "mcp__lean-ctx__ctx_shell"); + assert_eq!(parsed.command.as_deref(), Some("cargo test")); + assert_eq!(parsed.output_text, "test result: ok"); + } + + #[test] + fn parse_post_tool_use_gateway_search_action_has_no_command() { + let input = r#"{"session_id":"s1","tool_name":"mcp__flare__tool","tool_input":{"action":"search","query":"ctx_shell"},"tool_response":{}}"#; + let parsed = parse_post_tool_use(input).unwrap(); + assert_eq!(parsed.tool_name, "mcp__flare__tool"); + assert!(parsed.command.is_none()); + } + + #[test] + fn output_text_falls_back_to_mcp_content_block_envelope() { + let input = r#"{"session_id":"s1","tool_name":"Bash","tool_input":{"command":"cargo test"},"tool_response":{"content":[{"type":"text","text":"3 failures:\n test_foo"}]}}"#; + let parsed = parse_post_tool_use(input).unwrap(); + assert_eq!(parsed.output_text, "3 failures:\n test_foo"); + assert!(!verification_passed(parsed.exit_code, &parsed.output_text)); + } + #[test] fn verification_passed_trusts_zero_exit_code() { assert!(verification_passed(Some(0), "")); diff --git a/src/hook_redirect.rs b/src/hook_redirect.rs index 17245b266..0fd82ccee 100644 --- a/src/hook_redirect.rs +++ b/src/hook_redirect.rs @@ -216,6 +216,54 @@ fn branch_guard_reason_for(branch: Option<&str>, default: Option<&str>) -> Optio }) } +/// MCP tool name Claude Code sends for the flare gateway's own `tool` MCP +/// tool. `apply_gateway_permissions` (components.rs) steers agents toward +/// calling lean-ctx exclusively through this gateway (`action="execute"`) +/// and actively strips their direct `mcp__lean-ctx__*` permissions -- so +/// once that's applied, every `ctx_shell`/`ctx_patch`/`ctx_edit` call this +/// module or `hook_completion_gate` needs to classify arrives wrapped in +/// this envelope instead of at the top level. +const GATEWAY_TOOL_NAME: &str = "mcp__flare__tool"; + +/// Unwraps a flare-gateway `action="execute"` call (`mcp__flare__tool( +/// action="execute", server="leanctx", tool="ctx_shell", args={"command": +/// "..."})`) to the real tool name and arguments it forwards, so +/// `is_verification_command`, `MUTATING_TOOLS`, and +/// `destructive_data_file_reason` all see the actual command/tool instead of +/// the gateway envelope one level up. Every other call (non-gateway, or +/// gateway `action="search"`, or `server` other than `"leanctx"`) passes +/// through with `tool_name`/`tool_input` unchanged. +/// +/// Root cause of item #559: without this, a verification command (or a +/// `ctx_patch`/`ctx_edit` edit) run through the gateway was invisible to +/// every classifier in this module -- `is_verification_command` never saw +/// the real command, so the completion gate (`completion_gate_reason`) +/// rejected `item done`/`check_merge` even seconds after a passing test run, +/// on every single retry, once gateway routing was in effect. +pub(crate) fn unwrap_gateway_call( + tool_name: &str, + tool_input: Option<&Value>, +) -> (String, Option) { + let passthrough = || (tool_name.to_string(), tool_input.cloned()); + if tool_name != GATEWAY_TOOL_NAME { + return passthrough(); + } + let Some(input) = tool_input else { + return passthrough(); + }; + if input.get("action").and_then(Value::as_str) != Some("execute") { + return passthrough(); + } + if input.get("server").and_then(Value::as_str) != Some("leanctx") { + return passthrough(); + } + let Some(real_tool) = input.get("tool").and_then(Value::as_str) else { + return passthrough(); + }; + let args = input.get("args").cloned().unwrap_or_else(|| json!({})); + (format!("mcp__lean-ctx__{real_tool}"), Some(args)) +} + /// MCP tool name Claude Code sends for the `flare` server's `item` tool /// (`mcp____`). opencode's MCP bridge is expected to use the /// same convention -- if a harness turns out to send a bare `item` instead, @@ -290,7 +338,12 @@ fn classify( ) }) } - "Bash" | "bash" | "PowerShell" | "powershell" | "shell" => { + "Bash" | "bash" | "PowerShell" | "powershell" | "shell" | "mcp__lean-ctx__ctx_shell" => { + // Includes ctx_shell (direct or gateway-unwrapped, see + // `unwrap_gateway_call`) -- item #559: a destructive `rm + // ~/.agentflare/*.db` run via ctx_shell must be caught exactly + // like the same command run via Bash. + // // "command" is Claude Code's and (by convention) opencode's bash // tool field; "cmd"/"script" are cheap insurance against a // harness using a different name rather than a hard dependency @@ -412,6 +465,15 @@ mod tests { assert!(classify("Bash", Some(&input), NOT_A_REPO).is_some()); } + #[test] + fn classify_blocks_rm_of_agentflare_db_via_ctx_shell() { + // item #559: ctx_shell (direct tool name, matching what + // unwrap_gateway_call produces for a gateway-routed call) must be + // covered by the same destructive-command guard as Bash. + let input = json!({ "command": "rm ~/.agentflare/store.db" }); + assert!(classify("mcp__lean-ctx__ctx_shell", Some(&input), NOT_A_REPO).is_some()); + } + #[test] fn classify_allows_unrelated_rm_commands() { let input = json!({ "command": "rm /tmp/scratch.txt" }); @@ -682,6 +744,62 @@ mod tests { } } + #[test] + fn unwrap_gateway_call_unwraps_leanctx_execute() { + let input = json!({ + "action": "execute", + "server": "leanctx", + "tool": "ctx_shell", + "args": {"command": "cargo test"}, + }); + let (name, unwrapped) = unwrap_gateway_call(GATEWAY_TOOL_NAME, Some(&input)); + assert_eq!(name, "mcp__lean-ctx__ctx_shell"); + assert_eq!(unwrapped, Some(json!({"command": "cargo test"}))); + } + + #[test] + fn unwrap_gateway_call_unwraps_ctx_patch_for_mutating_tool_classification() { + let input = json!({ + "action": "execute", + "server": "leanctx", + "tool": "ctx_patch", + "args": {"path": "src/main.rs"}, + }); + let (name, _) = unwrap_gateway_call(GATEWAY_TOOL_NAME, Some(&input)); + assert!(MUTATING_TOOLS.contains(&name.as_str()), "{name}"); + } + + #[test] + fn unwrap_gateway_call_passes_through_non_gateway_tool() { + let input = json!({"command": "cargo test"}); + let (name, unwrapped) = unwrap_gateway_call("Bash", Some(&input)); + assert_eq!(name, "Bash"); + assert_eq!(unwrapped, Some(input)); + } + + #[test] + fn unwrap_gateway_call_passes_through_gateway_search_action() { + let input = json!({"action": "search", "query": "ctx_shell"}); + let (name, unwrapped) = unwrap_gateway_call(GATEWAY_TOOL_NAME, Some(&input)); + assert_eq!(name, GATEWAY_TOOL_NAME); + assert_eq!(unwrapped, Some(input)); + } + + #[test] + fn unwrap_gateway_call_passes_through_non_leanctx_server() { + let input = json!({"action": "execute", "server": "other", "tool": "foo", "args": {}}); + let (name, _) = unwrap_gateway_call(GATEWAY_TOOL_NAME, Some(&input)); + assert_eq!(name, GATEWAY_TOOL_NAME); + } + + #[test] + fn unwrap_gateway_call_handles_missing_args() { + let input = json!({"action": "execute", "server": "leanctx", "tool": "ctx_shell"}); + let (name, unwrapped) = unwrap_gateway_call(GATEWAY_TOOL_NAME, Some(&input)); + assert_eq!(name, "mcp__lean-ctx__ctx_shell"); + assert_eq!(unwrapped, Some(json!({}))); + } + #[test] fn completion_gate_blocks_item_done_without_verification() { let input = json!({ "id": "abc", "action": "done" });