mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-18 02:14:43 +00:00
fix(providers): whitelist effort env vars in filterCLIEnv + wire per-agent --effort (#886)
filterCLIEnv stripped all CLAUDE_* env vars except OAUTH_TOKEN, preventing CLAUDE_CODE_EFFORT_LEVEL and CLAUDE_CODE_DISABLE_ADAPTIVE_THINKING from reaching the subprocess. Switch to an explicit allowlist map that preserves auth + behavioral tuning vars. Add effort parameter to buildArgs and plumb OptThinkingLevel from req.Options through Chat/ChatStream so per-agent reasoning level maps to --effort flag. When an explicit flag is set, strip the env var to avoid precedence ambiguity. Validate effort with isAlphaOnly to reject malformed input.
This commit is contained in:
1 parent
339b13e8f0
commit
96cc940f2b
2 files changed
+73
-10
No files matched your search
@@ -45,7 +45,8 @@ func (p *ClaudeCLIProvider) Chat(ctx context.Context, req ChatRequest) (*ChatRes
|
||||
if len(images) > 0 {
|
||||
outputFmt = "stream-json"
|
||||
}
|
||||
args := p.buildArgs(model, workDir, mcpPath, cliSessionID, outputFmt, len(images) > 0, disableTools)
|
||||
effortLevel := extractStringOpt(req.Options, OptThinkingLevel)
|
||||
args := p.buildArgs(model, workDir, mcpPath, cliSessionID, outputFmt, len(images) > 0, disableTools, effortLevel)
|
||||
|
||||
var stdin *bytes.Reader
|
||||
if len(images) > 0 {
|
||||
@@ -57,6 +58,10 @@ func (p *ClaudeCLIProvider) Chat(ctx context.Context, req ChatRequest) (*ChatRes
|
||||
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
|
||||
}
|
||||
@@ -102,7 +107,8 @@ func (p *ClaudeCLIProvider) ChatStream(ctx context.Context, req ChatRequest, onC
|
||||
disableTools := extractBoolOpt(req.Options, OptDisableTools)
|
||||
bc := bridgeContextFromOpts(req.Options)
|
||||
mcpPath := p.resolveMCPConfigPath(ctx, sessionKey, bc)
|
||||
args := p.buildArgs(model, workDir, mcpPath, cliSessionID, "stream-json", len(images) > 0, disableTools)
|
||||
effortLevel := extractStringOpt(req.Options, OptThinkingLevel)
|
||||
args := p.buildArgs(model, workDir, mcpPath, cliSessionID, "stream-json", len(images) > 0, disableTools, effortLevel)
|
||||
|
||||
var stdin *bytes.Reader
|
||||
if len(images) > 0 {
|
||||
@@ -115,6 +121,9 @@ func (p *ClaudeCLIProvider) ChatStream(ctx context.Context, req ChatRequest, onC
|
||||
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
|
||||
}
|
||||
@@ -150,6 +159,7 @@ func (p *ClaudeCLIProvider) ChatStream(ctx context.Context, req ChatRequest, onC
|
||||
|
||||
var finalResp ChatResponse
|
||||
var contentBuf strings.Builder
|
||||
var streamErrMsg string // error message from stream-json result event
|
||||
|
||||
for scanner.Scan() {
|
||||
if ctx.Err() != nil {
|
||||
@@ -192,8 +202,14 @@ func (p *ClaudeCLIProvider) ChatStream(ctx context.Context, req ChatRequest, onC
|
||||
finalResp.Content = contentBuf.String()
|
||||
}
|
||||
finalResp.FinishReason = "stop"
|
||||
if ev.Subtype == "error" {
|
||||
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{
|
||||
@@ -223,7 +239,16 @@ func (p *ClaudeCLIProvider) ChatStream(ctx context.Context, req ChatRequest, onC
|
||||
if finalResp.Content != "" {
|
||||
return &finalResp, nil
|
||||
}
|
||||
return nil, fmt.Errorf("claude-cli: %w (stderr: %s)", err, stderrBuf.String())
|
||||
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())
|
||||
|
||||
@@ -29,7 +29,8 @@ func validateCLIModel(model string) error {
|
||||
|
||||
// buildArgs constructs CLI arguments.
|
||||
// mcpConfigPath is the resolved per-session MCP config file (may differ per call).
|
||||
func (p *ClaudeCLIProvider) buildArgs(model, workDir, mcpConfigPath string, cliSessionID uuid.UUID, outputFormat string, hasImages, disableTools bool) []string {
|
||||
// effort is the reasoning effort level (low/medium/high); empty or "off" omits the flag.
|
||||
func (p *ClaudeCLIProvider) buildArgs(model, workDir, mcpConfigPath string, cliSessionID uuid.UUID, outputFormat string, hasImages, disableTools bool, effort string) []string {
|
||||
args := []string{
|
||||
"-p",
|
||||
"--output-format", outputFormat,
|
||||
@@ -38,6 +39,15 @@ func (p *ClaudeCLIProvider) buildArgs(model, workDir, mcpConfigPath string, cliS
|
||||
"--verbose",
|
||||
}
|
||||
|
||||
if effort != "" && effort != "off" {
|
||||
effort = strings.ToLower(strings.TrimSpace(effort))
|
||||
if isAlphaOnly(effort) {
|
||||
args = append(args, "--effort", effort)
|
||||
} else {
|
||||
slog.Warn("claude-cli: invalid effort value, skipping --effort flag", "effort", effort)
|
||||
}
|
||||
}
|
||||
|
||||
if mcpConfigPath != "" {
|
||||
args = append(args, "--mcp-config", mcpConfigPath)
|
||||
}
|
||||
@@ -284,21 +294,49 @@ func ResetCLISession(baseWorkDir, sessionKey string) {
|
||||
}
|
||||
}
|
||||
|
||||
// filterCLIEnv removes CLAUDE* env vars to prevent nested session conflicts,
|
||||
// but preserves CLAUDE_CODE_OAUTH_TOKEN for authentication.
|
||||
// filterCLIEnv removes CLAUDE* env vars to prevent nested session conflicts.
|
||||
// Behavioral tuning vars are whitelisted so they can be set system-wide
|
||||
// and picked up by claude-cli subprocesses.
|
||||
func filterCLIEnv(environ []string) []string {
|
||||
allowed := map[string]bool{
|
||||
"CLAUDE_CODE_OAUTH_TOKEN": true, // auth
|
||||
"CLAUDE_CODE_EFFORT_LEVEL": true, // behavioral tuning
|
||||
"CLAUDE_CODE_DISABLE_ADAPTIVE_THINKING": true, // behavioral tuning
|
||||
}
|
||||
var filtered []string
|
||||
for _, e := range environ {
|
||||
key := e
|
||||
if before, _, ok := strings.Cut(e, "="); ok {
|
||||
key = before
|
||||
}
|
||||
// Filter out variables that could cause nested CLI conflicts,
|
||||
// but preserve auth token needed by the subprocess.
|
||||
if strings.HasPrefix(key, "CLAUDE") && key != "CLAUDE_CODE_OAUTH_TOKEN" {
|
||||
if strings.HasPrefix(key, "CLAUDE") && !allowed[key] {
|
||||
continue
|
||||
}
|
||||
filtered = append(filtered, e)
|
||||
}
|
||||
return filtered
|
||||
}
|
||||
|
||||
// isAlphaOnly returns true if s is non-empty and contains only a-z characters.
|
||||
func isAlphaOnly(s string) bool {
|
||||
if len(s) == 0 {
|
||||
return false
|
||||
}
|
||||
for _, c := range s {
|
||||
if c < 'a' || c > 'z' {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// removeEnvKey returns a copy of environ with all entries for the given key removed.
|
||||
func removeEnvKey(environ []string, key string) []string {
|
||||
result := make([]string, 0, len(environ))
|
||||
for _, e := range environ {
|
||||
if k, _, _ := strings.Cut(e, "="); k != key {
|
||||
result = append(result, e)
|
||||
}
|
||||
}
|
||||
return result
|
||||
}
|
||||
Reference in new issue
Block a user