From cb91ad7d9ed155a8f8a8801cc8070eb30a765e45 Mon Sep 17 00:00:00 2001 From: Bohan-J Date: Wed, 29 Jul 2026 15:43:05 +0800 Subject: [PATCH] fix(agent): deny CodeBuddy's interactive plan-mode tools in headless runs (MUL-5383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeBuddy exempts AskUserQuestion and ExitPlanMode from permission-mode finalization, so `--permission-mode bypassPermissions` never auto-approves them. Once the model entered plan mode the session mode is Plan, not BypassPermissions, so ExitPlanMode also missed the daemon's bypass fast-path and went to the SDK permission bridge — which waits with no timeout for a confirmation the headless runtime cannot render. The task sat in-flight until the 2h tool watchdog, and users killed it by hand. Deny EnterPlanMode/ExitPlanMode alongside the AskUserQuestion we already deny. Each tool is passed as its own argv value because CodeBuddy matches disallowedTools entries exactly and does not split on commas. Also send `allowed: true` on control_response: CodeBuddy's SdkPermissionClient reads `allowed`, not Claude Code's `behavior`, so the daemon's "auto-approve" was being read as a denial. Fixes #6012 Co-authored-by: multica-agent --- server/pkg/agent/codebuddy.go | 22 ++++++- server/pkg/agent/codebuddy_test.go | 93 +++++++++++++++++++++++++++++- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/server/pkg/agent/codebuddy.go b/server/pkg/agent/codebuddy.go index c2528b4c61..906a8a9719 100644 --- a/server/pkg/agent/codebuddy.go +++ b/server/pkg/agent/codebuddy.go @@ -44,7 +44,22 @@ func buildCodebuddyArgs(opts ExecOptions, logger *slog.Logger) []string { "--verbose", "--strict-mcp-config", "--permission-mode", "bypassPermissions", - "--disallowedTools", "AskUserQuestion", + // CodeBuddy's interactive tools have no UI to render in under the + // daemon's headless stream-json transport. AskUserQuestion and + // ExitPlanMode are both exempted from CodeBuddy's permission-mode + // finalization, so --permission-mode bypassPermissions does NOT + // auto-approve them — they always reach the permission bridge and + // stall the turn waiting for a confirmation nobody can give + // (GitHub #6012). EnterPlanMode is denied alongside them: leaving it + // enabled would let the model enter a plan mode it then has no tool + // to leave. Plan-shaped work still happens — the plan is written as + // ordinary assistant output instead of behind an approval gate. + // + // Pass one value per tool: --disallowedTools is variadic and + // CodeBuddy compares each entry against the tool name exactly + // (PermissionUtils.matchPermissionRules), so a comma-joined string + // would match nothing despite what the CLI's own help text claims. + "--disallowedTools", "AskUserQuestion", "EnterPlanMode", "ExitPlanMode", } if opts.Model != "" { args = append(args, "--model", opts.Model) @@ -404,6 +419,11 @@ func (b *codebuddyBackend) handleControlRequest(msg codebuddySDKMessage, stdin i "subtype": "success", "request_id": msg.RequestID, "response": map[string]any{ + // CodeBuddy's SdkPermissionClient reads `allowed` and treats a + // missing key as a denial; `behavior` is Claude Code's spelling, + // which the fork still honours on its other permission paths. + // Send both so an approval is never read as a silent reject. + "allowed": true, "behavior": "allow", "updatedInput": inputMap, }, diff --git a/server/pkg/agent/codebuddy_test.go b/server/pkg/agent/codebuddy_test.go index a8aeac93ae..2f0e72ba74 100644 --- a/server/pkg/agent/codebuddy_test.go +++ b/server/pkg/agent/codebuddy_test.go @@ -1,6 +1,7 @@ package agent import ( + "bytes" "context" "encoding/json" "log/slog" @@ -27,7 +28,7 @@ func TestBuildCodebuddyArgs_Basic(t *testing.T) { "--verbose", "--strict-mcp-config", "--permission-mode", "bypassPermissions", - "--disallowedTools", "AskUserQuestion", + "--disallowedTools", "AskUserQuestion", "EnterPlanMode", "ExitPlanMode", "--model", "claude-sonnet-4-20250514", "--max-turns", "25", "--append-system-prompt", "You are an agent.", @@ -43,6 +44,37 @@ func TestBuildCodebuddyArgs_Basic(t *testing.T) { } } +func TestBuildCodebuddyArgs_DisallowsInteractiveTools(t *testing.T) { + t.Parallel() + + // The daemon runs CodeBuddy headless, so every tool that waits on a human + // confirmation stalls the turn instead of ending it (GitHub #6012). + // CodeBuddy matches each --disallowedTools entry against the tool name + // exactly, so each tool must arrive as its own argv value. + args := buildCodebuddyArgs(ExecOptions{}, slog.Default()) + + idx := -1 + for i, a := range args { + if a == "--disallowedTools" { + idx = i + break + } + } + if idx == -1 { + t.Fatalf("expected --disallowedTools in args: %v", args) + } + + for offset, want := range []string{"AskUserQuestion", "EnterPlanMode", "ExitPlanMode"} { + got := "" + if idx+1+offset < len(args) { + got = args[idx+1+offset] + } + if got != want { + t.Fatalf("disallowed tool %d = %q, want %q\nfull args: %v", offset, got, want, args) + } + } +} + func TestBuildCodebuddyArgs_InjectsEffort(t *testing.T) { t.Parallel() @@ -474,3 +506,62 @@ func TestCodebuddyHandleUserToolResult(t *testing.T) { t.Fatal("expected message on channel") } } + +func TestCodebuddyHandleControlRequestApprovesInCodebuddyShape(t *testing.T) { + t.Parallel() + + b := &codebuddyBackend{cfg: Config{Logger: slog.Default()}} + + var written bytes.Buffer + + msg := codebuddySDKMessage{ + Type: "control_request", + RequestID: "perm_1730000000000_1", + Request: mustMarshal(t, codebuddyControlRequestPayload{ + Subtype: "can_use_tool", + ToolName: "Bash", + Input: mustMarshal(t, map[string]any{"command": "ls"}), + }), + } + + b.handleControlRequest(msg, &written) + + var resp map[string]any + if err := json.Unmarshal(bytes.TrimSpace(written.Bytes()), &resp); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + + if resp["type"] != "control_response" { + t.Fatalf("expected type control_response, got %v", resp["type"]) + } + respInner, ok := resp["response"].(map[string]any) + if !ok { + t.Fatalf("expected response object, got %v", resp["response"]) + } + if respInner["subtype"] != "success" { + t.Fatalf("expected subtype success, got %v", respInner["subtype"]) + } + if respInner["request_id"] != "perm_1730000000000_1" { + t.Fatalf("expected the request_id to be echoed back, got %v", respInner["request_id"]) + } + + innerResp, ok := respInner["response"].(map[string]any) + if !ok { + t.Fatalf("expected inner response object, got %v", respInner["response"]) + } + // CodeBuddy reads `allowed`; a missing key is read as a denial, which + // leaves the CLI waiting on a confirmation the daemon can never deliver. + if innerResp["allowed"] != true { + t.Fatalf("expected allowed=true, got %v", innerResp["allowed"]) + } + if innerResp["behavior"] != "allow" { + t.Fatalf("expected behavior allow, got %v", innerResp["behavior"]) + } + updatedInput, ok := innerResp["updatedInput"].(map[string]any) + if !ok { + t.Fatalf("expected updatedInput object, got %v", innerResp["updatedInput"]) + } + if updatedInput["command"] != "ls" { + t.Fatalf("expected the original tool input to be preserved, got %v", updatedInput["command"]) + } +}