fix(providers): tolerate unknown Claude CLI deny rules (#1381)

internal/providers/claude_cli.go

@@ -51,17 +51,18 @@ const OptAllowedToolNames = "allowed_tool_names"
 // It acts as a thin proxy: CLI manages session history, tool execution, and context.
 // GoClaw only forwards the latest user message and streams back the response.
 type ClaudeCLIProvider struct {
-	name              string         // provider name (default: "claude-cli")
-	cliPath           string         // path to claude binary (default: "claude")
-	defaultModel      string         // default: "sonnet"
-	baseWorkDir       string         // base dir for agent workspaces
-	mcpConfigData     *MCPConfigData // per-session MCP config data
-	permMode          string         // permission mode (default: "bypassPermissions")
-	hooksSettingsPath string         // generated settings.json with security hooks (empty = no hooks)
-	hooksCleanup      func()         // cleanup function for hooks temp files
-	mu                sync.Mutex     // protects workdir creation
-	sessionMu         sync.Map       // key: string, value: *sync.Mutex — per-session lock
-	mcpConfigDirs     sync.Map       // key: string (dir path), value: struct{} — tracks per-session MCP config dirs for cleanup
+	name                       string         // provider name (default: "claude-cli")
+	cliPath                    string         // path to claude binary (default: "claude")
+	defaultModel               string         // default: "sonnet"
+	baseWorkDir                string         // base dir for agent workspaces
+	mcpConfigData              *MCPConfigData // per-session MCP config data
+	permMode                   string         // permission mode (default: "bypassPermissions")
+	hooksSettingsPath          string         // generated settings.json with security hooks (empty = no hooks)
+	hooksCleanup               func()         // cleanup function for hooks temp files
+	mu                         sync.Mutex     // protects workdir creation
+	sessionMu                  sync.Map       // key: string, value: *sync.Mutex — per-session lock
+	mcpConfigDirs              sync.Map       // key: string (dir path), value: struct{} — tracks per-session MCP config dirs for cleanup
+	invalidDisallowedToolNames sync.Map       // key: string, value: struct{} — Claude CLI deny rules rejected by this CLI version
 }
 
 // ClaudeCLIOption configures the provider.

internal/providers/claude_cli_chat.go

@@ -56,27 +56,40 @@ func (p *ClaudeCLIProvider) Chat(ctx context.Context, req ChatRequest) (*ChatRes
 		args = append(args, "--", userMsg)
 	}
 
-	cmd := exec.CommandContext(ctx, p.cliPath, args...)
-	cmd.Dir = workDir
-	cmd.Env = filterCLIEnv(os.Environ())
-	if effortLevel != "" && effortLevel != "off" {
-		// Explicit --effort flag takes precedence; drop env to avoid ambiguity.
-		cmd.Env = removeEnvKey(cmd.Env, "CLAUDE_CODE_EFFORT_LEVEL")
-	}
-	if stdin != nil {
-		cmd.Stdin = stdin
-	}
+	for {
+		cmd := exec.CommandContext(ctx, p.cliPath, args...)
+		cmd.Dir = workDir
+		cmd.Env = filterCLIEnv(os.Environ())
+		if effortLevel != "" && effortLevel != "off" {
+			// Explicit --effort flag takes precedence; drop env to avoid ambiguity.
+			cmd.Env = removeEnvKey(cmd.Env, "CLAUDE_CODE_EFFORT_LEVEL")
+		}
+		if stdin != nil {
+			cmd.Stdin = stdin
+		}
 
-	var stderr bytes.Buffer
-	cmd.Stderr = &stderr
+		var stderr bytes.Buffer
+		cmd.Stderr = &stderr
 
-	slog.Debug("claude-cli exec", "cmd", fmt.Sprintf("%s %s", p.cliPath, strings.Join(args, " ")), "workdir", workDir)
-	output, err := cmd.Output()
-	if err != nil {
-		return nil, fmt.Errorf("claude-cli: %w (stderr: %s)", err, stderr.String())
+		slog.Debug("claude-cli exec", "cmd", fmt.Sprintf("%s %s", p.cliPath, strings.Join(args, " ")), "workdir", workDir)
+		output, err := cmd.Output()
+		if err != nil {
+			if tool, retry := p.noteInvalidDisallowedTool(stderr.String()); retry {
+				slog.Warn("claude-cli: retrying without unsupported --disallowedTools rule", "tool", tool)
+				args = p.buildArgs(model, workDir, mcpPath, cliSessionID, outputFmt, len(images) > 0, disableTools, effortLevel, allowedToolNames)
+				if len(images) == 0 {
+					args = append(args, "--", userMsg)
+				}
+				if len(images) > 0 {
+					stdin = buildStreamJSONInput(userMsg, images)
+				}
+				continue
+			}
+			return nil, fmt.Errorf("claude-cli: %w (stderr: %s)", err, stderr.String())
+		}
+
+		return parseJSONResponse(output)
 	}
-
-	return parseJSONResponse(output)
 }
 
 // ChatStream runs the CLI with stream-json output, calling onChunk for each text delta.
@@ -119,149 +132,161 @@ func (p *ClaudeCLIProvider) ChatStream(ctx context.Context, req ChatRequest, onC
 		args = append(args, "--", userMsg)
 	}
 
-	cmd := exec.CommandContext(ctx, p.cliPath, args...)
-	cmd.WaitDelay = 5 * time.Second // force-close pipes if process lingers after kill
-	cmd.Dir = workDir
-	cmd.Env = filterCLIEnv(os.Environ())
-	if effortLevel != "" && effortLevel != "off" {
-		cmd.Env = removeEnvKey(cmd.Env, "CLAUDE_CODE_EFFORT_LEVEL")
-	}
-	if stdin != nil {
-		cmd.Stdin = stdin
-	}
-
-	var stderrBuf bytes.Buffer
-	cmd.Stderr = &stderrBuf
-
-	stdout, err := cmd.StdoutPipe()
-	if err != nil {
-		return nil, fmt.Errorf("claude-cli stdout pipe: %w", err)
-	}
-
-	fullCmd := fmt.Sprintf("%s %s", p.cliPath, strings.Join(args, " "))
-	slog.Debug("claude-cli stream exec", "cmd", fullCmd, "workdir", workDir)
-	if err := cmd.Start(); err != nil {
-		return nil, fmt.Errorf("claude-cli start: %w", err)
-	}
-
-	// Debug log file: only enabled when GOCLAW_DEBUG=1
-	var debugFile *os.File
-	if os.Getenv("GOCLAW_DEBUG") == "1" {
-		debugLogPath := filepath.Join(workDir, "cli-debug.log")
-		debugFile, _ = os.OpenFile(debugLogPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)
-		if debugFile != nil {
-			fmt.Fprintf(debugFile, "=== CMD: %s\n=== WORKDIR: %s\n=== TIME: %s\n\n", fullCmd, workDir, time.Now().Format(time.RFC3339))
-			defer debugFile.Close()
+	for {
+		cmd := exec.CommandContext(ctx, p.cliPath, args...)
+		cmd.WaitDelay = 5 * time.Second // force-close pipes if process lingers after kill
+		cmd.Dir = workDir
+		cmd.Env = filterCLIEnv(os.Environ())
+		if effortLevel != "" && effortLevel != "off" {
+			cmd.Env = removeEnvKey(cmd.Env, "CLAUDE_CODE_EFFORT_LEVEL")
 		}
-	}
-
-	// Parse stream-json line-by-line
-	scanner := bufio.NewScanner(stdout)
-	scanner.Buffer(make([]byte, 0, StdioScanBufInit), StdioScanBufMax)
-
-	var finalResp ChatResponse
-	var contentBuf strings.Builder
-	var streamErrMsg string // error message from stream-json result event
-
-	for scanner.Scan() {
-		if ctx.Err() != nil {
-			break // context cancelled (abort) → exit immediately
-		}
-		line := scanner.Bytes()
-		if len(line) == 0 {
-			continue
+		if stdin != nil {
+			cmd.Stdin = stdin
 		}
 
-		// Write raw line to debug log
-		if debugFile != nil {
-			fmt.Fprintf(debugFile, "%s\n", line)
+		var stderrBuf bytes.Buffer
+		cmd.Stderr = &stderrBuf
+
+		stdout, err := cmd.StdoutPipe()
+		if err != nil {
+			return nil, fmt.Errorf("claude-cli stdout pipe: %w", err)
 		}
 
-		var ev cliStreamEvent
-		if err := json.Unmarshal(line, &ev); err != nil {
-			slog.Debug("claude-cli: skip malformed stream line", "error", err)
-			continue
+		fullCmd := fmt.Sprintf("%s %s", p.cliPath, strings.Join(args, " "))
+		slog.Debug("claude-cli stream exec", "cmd", fullCmd, "workdir", workDir)
+		if err := cmd.Start(); err != nil {
+			return nil, fmt.Errorf("claude-cli start: %w", err)
 		}
 
-		switch ev.Type {
-		case "assistant":
-			if ev.Message == nil {
+		// Debug log file: only enabled when GOCLAW_DEBUG=1
+		var debugFile *os.File
+		if os.Getenv("GOCLAW_DEBUG") == "1" {
+			debugLogPath := filepath.Join(workDir, "cli-debug.log")
+			debugFile, _ = os.OpenFile(debugLogPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)
+			if debugFile != nil {
+				fmt.Fprintf(debugFile, "=== CMD: %s\n=== WORKDIR: %s\n=== TIME: %s\n\n", fullCmd, workDir, time.Now().Format(time.RFC3339))
+				defer debugFile.Close()
+			}
+		}
+
+		// Parse stream-json line-by-line
+		scanner := bufio.NewScanner(stdout)
+		scanner.Buffer(make([]byte, 0, StdioScanBufInit), StdioScanBufMax)
+
+		var finalResp ChatResponse
+		var contentBuf strings.Builder
+		var streamErrMsg string // error message from stream-json result event
+
+		for scanner.Scan() {
+			if ctx.Err() != nil {
+				break // context cancelled (abort) → exit immediately
+			}
+			line := scanner.Bytes()
+			if len(line) == 0 {
 				continue
 			}
-			text, thinking := extractStreamContent(ev.Message)
-			if text != "" {
-				contentBuf.WriteString(text)
-				onChunk(StreamChunk{Content: text})
-			}
-			if thinking != "" {
-				onChunk(StreamChunk{Thinking: thinking})
+
+			// Write raw line to debug log
+			if debugFile != nil {
+				fmt.Fprintf(debugFile, "%s\n", line)
 			}
 
-		case "result":
-			if ev.Result != "" {
-				finalResp.Content = ev.Result
-			} else {
-				finalResp.Content = contentBuf.String()
+			var ev cliStreamEvent
+			if err := json.Unmarshal(line, &ev); err != nil {
+				slog.Debug("claude-cli: skip malformed stream line", "error", err)
+				continue
 			}
+
+			switch ev.Type {
+			case "assistant":
+				if ev.Message == nil {
+					continue
+				}
+				text, thinking := extractStreamContent(ev.Message)
+				if text != "" {
+					contentBuf.WriteString(text)
+					onChunk(StreamChunk{Content: text})
+				}
+				if thinking != "" {
+					onChunk(StreamChunk{Thinking: thinking})
+				}
+
+			case "result":
+				if ev.Result != "" {
+					finalResp.Content = ev.Result
+				} else {
+					finalResp.Content = contentBuf.String()
+				}
+				finalResp.FinishReason = "stop"
+				if ev.Subtype == "error" || ev.IsError {
+					finalResp.FinishReason = "error"
+					// Prefer ev.Error (result may be empty for usage/rate limit errors).
+					if ev.Error != "" {
+						streamErrMsg = ev.Error
+					} else if ev.Result != "" {
+						streamErrMsg = ev.Result
+					}
+				}
+				if ev.Usage != nil {
+					finalResp.Usage = &Usage{
+						PromptTokens:     ev.Usage.InputTokens,
+						CompletionTokens: ev.Usage.OutputTokens,
+						TotalTokens:      ev.Usage.InputTokens + ev.Usage.OutputTokens,
+					}
+				}
+			}
+		}
+
+		// Context cancelled (abort): best-effort reap (bounded by WaitDelay), then return.
+		if ctx.Err() != nil {
+			_ = cmd.Wait()
+			return nil, ctx.Err()
+		}
+
+		if err := scanner.Err(); err != nil {
+			return nil, fmt.Errorf("claude-cli: stream read error: %w", err)
+		}
+
+		if err := cmd.Wait(); err != nil {
+			if debugFile != nil {
+				fmt.Fprintf(debugFile, "\n=== STDERR:\n%s\n=== EXIT ERROR: %v\n", stderrBuf.String(), err)
+			}
+			// If we got partial content, return it with the error.
+			if finalResp.Content != "" {
+				return &finalResp, nil
+			}
+			stderrStr := strings.TrimSpace(stderrBuf.String())
+			if tool, retry := p.noteInvalidDisallowedTool(stderrStr); retry {
+				slog.Warn("claude-cli: retrying without unsupported --disallowedTools rule", "tool", tool)
+				args = p.buildArgs(model, workDir, mcpPath, cliSessionID, "stream-json", len(images) > 0, disableTools, effortLevel, allowedToolNames)
+				if len(images) == 0 {
+					args = append(args, "--", userMsg)
+				} else {
+					stdin = buildStreamJSONInput(userMsg, images)
+				}
+				continue
+			}
+			if stderrStr == "" && finalResp.FinishReason == "error" {
+				// claude-cli communicates API errors via stream-json stdout, not stderr.
+				if streamErrMsg != "" {
+					stderrStr = "stream: " + streamErrMsg
+				} else {
+					stderrStr = "stream error (no message)"
+				}
+			}
+			return nil, fmt.Errorf("claude-cli: %w (stderr: %s)", err, stderrStr)
+		}
+		if debugFile != nil && stderrBuf.Len() > 0 {
+			fmt.Fprintf(debugFile, "\n=== STDERR:\n%s\n", stderrBuf.String())
+		}
+
+		// Fallback if no "result" event was received
+		if finalResp.Content == "" {
+			finalResp.Content = contentBuf.String()
 			finalResp.FinishReason = "stop"
-			if ev.Subtype == "error" || ev.IsError {
-				finalResp.FinishReason = "error"
-				// Prefer ev.Error (result may be empty for usage/rate limit errors).
-				if ev.Error != "" {
-					streamErrMsg = ev.Error
-				} else if ev.Result != "" {
-					streamErrMsg = ev.Result
-				}
-			}
-			if ev.Usage != nil {
-				finalResp.Usage = &Usage{
-					PromptTokens:     ev.Usage.InputTokens,
-					CompletionTokens: ev.Usage.OutputTokens,
-					TotalTokens:      ev.Usage.InputTokens + ev.Usage.OutputTokens,
-				}
-			}
 		}
-	}
 
-	// Context cancelled (abort): best-effort reap (bounded by WaitDelay), then return.
-	if ctx.Err() != nil {
-		_ = cmd.Wait()
-		return nil, ctx.Err()
+		onChunk(StreamChunk{Done: true})
+		return &finalResp, nil
 	}
-
-	if err := scanner.Err(); err != nil {
-		return nil, fmt.Errorf("claude-cli: stream read error: %w", err)
-	}
-
-	if err := cmd.Wait(); err != nil {
-		if debugFile != nil {
-			fmt.Fprintf(debugFile, "\n=== STDERR:\n%s\n=== EXIT ERROR: %v\n", stderrBuf.String(), err)
-		}
-		// If we got partial content, return it with the error
-		if finalResp.Content != "" {
-			return &finalResp, nil
-		}
-		stderrStr := strings.TrimSpace(stderrBuf.String())
-		if stderrStr == "" && finalResp.FinishReason == "error" {
-			// claude-cli communicates API errors via stream-json stdout, not stderr.
-			if streamErrMsg != "" {
-				stderrStr = "stream: " + streamErrMsg
-			} else {
-				stderrStr = "stream error (no message)"
-			}
-		}
-		return nil, fmt.Errorf("claude-cli: %w (stderr: %s)", err, stderrStr)
-	}
-	if debugFile != nil && stderrBuf.Len() > 0 {
-		fmt.Fprintf(debugFile, "\n=== STDERR:\n%s\n", stderrBuf.String())
-	}
-
-	// Fallback if no "result" event was received
-	if finalResp.Content == "" {
-		finalResp.Content = contentBuf.String()
-		finalResp.FinishReason = "stop"
-	}
-
-	onChunk(StreamChunk{Done: true})
-	return &finalResp, nil
 }

internal/providers/claude_cli_session.go

@@ -8,6 +8,7 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"regexp"
 	"slices"
 	"strings"
 
@@ -41,6 +42,8 @@ var cliNativeToolsAlwaysBlocked = []string{
 	"Glob", "Grep", "TodoWrite", "NotebookEdit",
 }
 
+var unknownDisallowedToolRe = regexp.MustCompile(`Permission deny rule "([^"]+)" matches no known tool`)
+
 // disallowedCLITools computes the --disallowedTools value for the Claude CLI
 // subprocess from the agent's policy-filtered allowed GoClaw tool names.
 // allowedToolNames == nil is treated as "no tools allowed" (fail closed):
@@ -62,6 +65,42 @@ func disallowedCLITools(allowedToolNames []string) []string {
 	return blocked
 }
 
+// filterInvalidDisallowedTools removes deny rules that the local Claude CLI has
+// already rejected as unknown. Claude Code occasionally changes native tool
+// names; retrying without rejected names keeps the provider usable while still
+// denying every rule the installed CLI recognizes.
+func (p *ClaudeCLIProvider) filterInvalidDisallowedTools(tools []string) []string {
+	if len(tools) == 0 {
+		return tools
+	}
+	filtered := tools[:0]
+	for _, tool := range tools {
+		if _, invalid := p.invalidDisallowedToolNames.Load(tool); invalid {
+			continue
+		}
+		filtered = append(filtered, tool)
+	}
+	return filtered
+}
+
+// noteInvalidDisallowedTool records a Claude CLI deny rule rejected by the
+// installed CLI. It returns true only the first time the tool is observed so
+// callers can bound retries.
+func (p *ClaudeCLIProvider) noteInvalidDisallowedTool(stderr string) (string, bool) {
+	match := unknownDisallowedToolRe.FindStringSubmatch(stderr)
+	if len(match) != 2 {
+		return "", false
+	}
+	tool := match[1]
+	if tool == "" {
+		return "", false
+	}
+	if _, loaded := p.invalidDisallowedToolNames.LoadOrStore(tool, struct{}{}); loaded {
+		return tool, false
+	}
+	return tool, true
+}
+
 // validCLIModels lists accepted model aliases for the Claude CLI.
 var validCLIModels = map[string]bool{
 	"sonnet": true, "opus": true, "haiku": true,
@@ -124,11 +163,14 @@ func (p *ClaudeCLIProvider) buildArgs(model, workDir, mcpConfigPath string, cliS
 	// This must never be skipped — omitting it previously let the CLI
 	// subprocess run with its full native toolset unrestricted whenever no MCP
 	// config was resolved.
+	var blockedTools []string
 	if disableTools {
-		args = append(args, "--disallowedTools", strings.Join(disallowedCLITools(nil), ","))
+		blockedTools = disallowedCLITools(nil)
 	} else {
-		args = append(args, "--disallowedTools", strings.Join(disallowedCLITools(allowedToolNames), ","))
+		blockedTools = disallowedCLITools(allowedToolNames)
 	}
+	blockedTools = p.filterInvalidDisallowedTools(blockedTools)
+	args = append(args, "--disallowedTools", strings.Join(blockedTools, ","))
 
 	if p.hooksSettingsPath != "" {
 		args = append(args, "--settings", p.hooksSettingsPath)

internal/providers/claude_cli_session_test.go

@@ -149,3 +149,49 @@ func TestDisallowedCLITools_NoStaleToolNames(t *testing.T) {
 		}
 	}
 }
+
+func TestClaudeCLIRemembersUnknownDisallowedToolRules(t *testing.T) {
+	p := NewClaudeCLIProvider("claude")
+
+	if _, retry := p.noteInvalidDisallowedTool(`Permission deny rule "NotebookRead" matches no known tool — check for typos.`); !retry {
+		t.Fatalf("first unknown tool observation should request retry")
+	}
+	if _, retry := p.noteInvalidDisallowedTool(`Permission deny rule "NotebookRead" matches no known tool — check for typos.`); retry {
+		t.Fatalf("repeated unknown tool observation should not request another retry")
+	}
+
+	filtered := p.filterInvalidDisallowedTools([]string{"Bash", "NotebookRead", "TodoWrite"})
+	want := []string{"Bash", "TodoWrite"}
+	if len(filtered) != len(want) {
+		t.Fatalf("filtered tools = %v, want %v", filtered, want)
+	}
+	for i := range want {
+		if filtered[i] != want[i] {
+			t.Fatalf("filtered tools = %v, want %v", filtered, want)
+		}
+	}
+}
+
+func TestClaudeCLIDisallowedToolsExcludeRemovedReadOnlyTools(t *testing.T) {
+	blocked := disallowedCLITools(nil)
+	for _, removed := range []string{"NotebookRead", "TodoRead"} {
+		for _, got := range blocked {
+			if got == removed {
+				t.Fatalf("disallowedCLITools included removed Claude CLI tool %q in %v", removed, blocked)
+			}
+		}
+	}
+
+	for _, want := range []string{"NotebookEdit", "TodoWrite"} {
+		found := false
+		for _, got := range blocked {
+			if got == want {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Fatalf("disallowedCLITools missing still-supported tool %q in %v", want, blocked)
+		}
+	}
+}