mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-03 19:20:07 +02:00
fix(agent): deny CodeBuddy's interactive plan-mode tools in headless runs (MUL-5383)
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 <github@multica.ai>
This commit is contained in:
@@ -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,
|
||||
},
|
||||
|
||||
@@ -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"])
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user