From 3985783c23b33cf9cd8a9021a1a95afacd82ac67 Mon Sep 17 00:00:00 2001 From: "TaoMu (GTC2080)" <1724815229@qq.com> Date: Thu, 4 Jun 2026 14:39:44 +1000 Subject: [PATCH 1/2] fix: make bash foreground timeout configurable Fixes #2982 --- README.md | 1 + README.zh-CN.md | 1 + docs/SPEC.md | 1 + internal/boot/boot.go | 9 +- internal/cli/acp.go | 3 +- internal/config/config.go | 15 ++- internal/config/render.go | 5 +- internal/config/render_test.go | 4 + internal/config/tools_test.go | 11 +++ internal/skill/builtins.go | 2 +- internal/tool/builtin/bash.go | 45 ++++++--- internal/tool/builtin/bash_timeout_test.go | 106 +++++++++++++++++++++ internal/tool/builtin/confine.go | 9 +- internal/tool/builtin/workspace.go | 12 ++- reasonix.example.toml | 1 + 15 files changed, 194 insertions(+), 31 deletions(-) create mode 100644 internal/config/tools_test.go create mode 100644 internal/tool/builtin/bash_timeout_test.go diff --git a/README.md b/README.md index 93bf1d8f3..fb9845341 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,7 @@ api_key_env = "DEEPSEEK_API_KEY" [tools] enabled = [] # omit/empty = all built-ins +bash_timeout_seconds = 0 # 0 = no foreground timeout; cancellation still stops the process [skills] # paths = ["~/my-skills", "../shared/skills"] # extra custom skill roots diff --git a/README.zh-CN.md b/README.zh-CN.md index 43d506af4..232e3ad81 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -112,6 +112,7 @@ api_key_env = "DEEPSEEK_API_KEY" [tools] enabled = [] # 省略/为空 = 全部内置工具 +bash_timeout_seconds = 0 # 0 = 前台命令不设超时；取消操作仍会停止进程 [skills] # paths = ["~/my-skills", "../shared/skills"] # 额外的自定义技能目录 diff --git a/docs/SPEC.md b/docs/SPEC.md index 5d2a69ff2..be8b639ec 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -383,6 +383,7 @@ api_key_env = "MIMO_API_KEY" [tools] enabled = [] # omit/empty = all built-ins +bash_timeout_seconds = 0 # 0 = no foreground timeout; cancellation still stops the process [skills] # paths = ["~/my-skills", "../shared/skills"] # extra custom skill roots diff --git a/internal/boot/boot.go b/internal/boot/boot.go index 15e9963ad..d5e7016eb 100644 --- a/internal/boot/boot.go +++ b/internal/boot/boot.go @@ -193,7 +193,8 @@ func Build(ctx context.Context, opts Options) (*control.Controller, error) { fmt.Fprintln(stderr, "warning: bash not found on PATH; the shell tool will run commands under Windows PowerShell. Install Git for Windows or WSL to use bash.") } searchSpec := builtin.ResolveSearch(cfg.Tools.Search.Engine, cfg.Tools.Search.RgPath, stderr) - addBuiltins(reg, cfg.Tools.Enabled, cfg.WriteRootsForRoot(root), bashSpec, searchSpec, stderr, root) + bashTimeout := time.Duration(cfg.BashTimeoutSeconds()) * time.Second + addBuiltins(reg, cfg.Tools.Enabled, cfg.WriteRootsForRoot(root), bashSpec, bashTimeout, searchSpec, stderr, root) // Always construct a host, even with no plugins configured, so the controller's // host pointer is stable for the session and `/mcp add` can hot-add into it. pluginHost := plugin.NewHost() @@ -664,12 +665,12 @@ func NewProviderWithProxy(e *config.ProviderEntry, proxy netclient.ProxySpec) (p // instance bound to writeRoots (preserving registry order). // When workDir is non-empty, tools resolve relative paths against it instead of // the process cwd, enabling concurrent multi-project sessions. -func addBuiltins(reg *tool.Registry, enabled, writeRoots []string, bashSpec sandbox.Spec, searchSpec builtin.SearchSpec, stderr io.Writer, workDir string) { +func addBuiltins(reg *tool.Registry, enabled, writeRoots []string, bashSpec sandbox.Spec, bashTimeout time.Duration, searchSpec builtin.SearchSpec, stderr io.Writer, workDir string) { // If a workspace directory is set, use workspace-bound tools that resolve // paths relative to that directory. Otherwise fall back to the process-cwd // compile-time builtins. if workDir != "" { - ws := builtin.Workspace{Dir: workDir, WriteRoots: writeRoots, Bash: bashSpec, Search: searchSpec} + ws := builtin.Workspace{Dir: workDir, WriteRoots: writeRoots, Bash: bashSpec, BashTimeout: bashTimeout, Search: searchSpec} for _, t := range ws.Tools(enabled...) { reg.Add(t) } @@ -692,7 +693,7 @@ func addBuiltins(reg *tool.Registry, enabled, writeRoots []string, bashSpec sand // Replace the unconfined defaults with confined instances (registry order is // preserved on replace): file-writers bound to the workspace, bash to the OS // sandbox. Only replace tools actually enabled/present. - confined := append(builtin.ConfineWriters(writeRoots), builtin.ConfineBash(bashSpec), builtin.ConfineSearch(searchSpec)) + confined := append(builtin.ConfineWriters(writeRoots), builtin.ConfineBash(bashSpec, bashTimeout), builtin.ConfineSearch(searchSpec)) for _, t := range confined { if _, ok := reg.Get(t.Name()); ok { reg.Add(t) diff --git a/internal/cli/acp.go b/internal/cli/acp.go index bf8222427..93bf07f90 100644 --- a/internal/cli/acp.go +++ b/internal/cli/acp.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "os/signal" + "time" "reasonix/internal/acp" "reasonix/internal/agent" @@ -106,7 +107,7 @@ func (f *acpFactory) NewSession(ctx context.Context, p acp.SessionParams) (*cont writeRoots = []string{p.Cwd} } bashSpec := sandbox.Spec{Mode: cfg.BashMode(), WriteRoots: writeRoots, Network: cfg.Sandbox.Network} - ws := builtin.Workspace{Dir: p.Cwd, WriteRoots: writeRoots, Bash: bashSpec, Search: builtin.ResolveSearch(cfg.Tools.Search.Engine, cfg.Tools.Search.RgPath, nil)} + ws := builtin.Workspace{Dir: p.Cwd, WriteRoots: writeRoots, Bash: bashSpec, BashTimeout: time.Duration(cfg.BashTimeoutSeconds()) * time.Second, Search: builtin.ResolveSearch(cfg.Tools.Search.Engine, cfg.Tools.Search.RgPath, nil)} for _, t := range ws.Tools(cfg.Tools.Enabled...) { reg.Add(t) } diff --git a/internal/config/config.go b/internal/config/config.go index 9e7fc44bc..883f883c7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -423,8 +423,19 @@ func (e *ProviderEntry) HasModel(m string) bool { // ToolsConfig selects which built-in tools are enabled. Empty means all of them. type ToolsConfig struct { - Enabled []string `toml:"enabled"` - Search SearchConfig `toml:"search"` + Enabled []string `toml:"enabled"` + BashTimeoutSeconds int `toml:"bash_timeout_seconds"` + Search SearchConfig `toml:"search"` +} + +// BashTimeoutSeconds returns the foreground bash timeout in seconds. Zero means +// no tool-local cap; negative config values are treated as zero so a typo cannot +// produce a nonsensical negative duration. +func (c *Config) BashTimeoutSeconds() int { + if c.Tools.BashTimeoutSeconds < 0 { + return 0 + } + return c.Tools.BashTimeoutSeconds } // SearchConfig tunes the grep tool's engine. Engine is "auto" (default — use diff --git a/internal/config/render.go b/internal/config/render.go index d385e181c..6d3049845 100644 --- a/internal/config/render.go +++ b/internal/config/render.go @@ -161,7 +161,7 @@ func RenderTOML(c *Config) string { b.WriteString("[tools]\n") if len(c.Tools.Enabled) == 0 { - b.WriteString("enabled = [] # empty = all built-in tools\n\n") + b.WriteString("enabled = [] # empty = all built-in tools\n") } else { b.WriteString("enabled = [") for i, t := range c.Tools.Enabled { @@ -170,8 +170,9 @@ func RenderTOML(c *Config) string { } fmt.Fprintf(&b, "%q", t) } - b.WriteString("]\n\n") + b.WriteString("]\n") } + fmt.Fprintf(&b, "bash_timeout_seconds = %d # 0 = no foreground timeout; cancellation still stops the process\n\n", c.BashTimeoutSeconds()) b.WriteString("[codegraph]\n") fmt.Fprintf(&b, "enabled = %v # built-in MCP server; off by default for first-run sessions\n", c.Codegraph.Enabled) diff --git a/internal/config/render_test.go b/internal/config/render_test.go index d40112f8f..d5f3202b8 100644 --- a/internal/config/render_test.go +++ b/internal/config/render_test.go @@ -17,6 +17,7 @@ func TestRenderTOMLRoundTrips(t *testing.T) { orig.Agent.AutoPlanClassifier = "deepseek-flash" orig.Agent.SubagentModel = "mimo-pro" orig.Agent.SubagentModels = map[string]string{"review": "deepseek-pro"} + orig.Tools.BashTimeoutSeconds = 900 orig.Permissions = PermissionsConfig{ Mode: "deny", Deny: []string{"bash(rm -rf*)"}, @@ -106,6 +107,9 @@ func TestRenderTOMLRoundTrips(t *testing.T) { if got.Agent.SubagentModels["review"] != "deepseek-pro" { t.Errorf("subagent_models.review = %q, want deepseek-pro", got.Agent.SubagentModels["review"]) } + if got.Tools.BashTimeoutSeconds != 900 { + t.Errorf("tools.bash_timeout_seconds = %d, want 900", got.Tools.BashTimeoutSeconds) + } if g, _ := got.Provider("mimo-pro"); g == nil || g.BaseURL != "http://localhost:8000/v1" { t.Errorf("mimo-pro base_url not preserved: %+v", g) } diff --git a/internal/config/tools_test.go b/internal/config/tools_test.go new file mode 100644 index 000000000..e5fa4829e --- /dev/null +++ b/internal/config/tools_test.go @@ -0,0 +1,11 @@ +package config + +import "testing" + +func TestBashTimeoutSecondsNormalizesNegative(t *testing.T) { + cfg := Default() + cfg.Tools.BashTimeoutSeconds = -1 + if got := cfg.BashTimeoutSeconds(); got != 0 { + t.Fatalf("BashTimeoutSeconds() = %d, want 0", got) + } +} diff --git a/internal/skill/builtins.go b/internal/skill/builtins.go index 644fd7a91..cd1bf00bf 100644 --- a/internal/skill/builtins.go +++ b/internal/skill/builtins.go @@ -112,7 +112,7 @@ const builtinTestBody = `This skill is INLINED — you run in the parent loop. T How to operate: 1. Detect the test command. Look at the project: go.mod → ` + "`go test ./...`" + `; package.json scripts.test → ` + "`npm test`" + ` (or pnpm/yarn); pyproject.toml/requirements.txt → ` + "`pytest`" + `; Cargo.toml → ` + "`cargo test`" + `. If you can't tell, ASK — don't guess. -2. Run it via bash (timeout ~120s, more for a big suite). Capture stdout + stderr. +2. Run it via bash. Capture stdout + stderr; for intentionally long-running commands, start them in the background and use wait/bash_output. 3. Read the failures: which tests failed, the actual error, the file + line that threw. Locate the exact assertion or stack frame. 4. Fix each distinct failure: - Production bug (test caught a real defect) → fix the production code. diff --git a/internal/tool/builtin/bash.go b/internal/tool/builtin/bash.go index 59f29f95c..a1ab75274 100644 --- a/internal/tool/builtin/bash.go +++ b/internal/tool/builtin/bash.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "os/exec" @@ -16,22 +17,26 @@ import ( ) const ( - bashTimeout = 120 * time.Second bashWaitDelay = 5 * time.Second ) +var errBashTimeout = errors.New("bash foreground timeout") + func init() { tool.RegisterBuiltin(bash{}) } -// bash runs a shell command with a timeout to avoid hangs. sb, when it enforces, -// wraps the command in an OS sandbox; the zero value registered at init runs -// unconfined and is overridden per run by ConfineBash. shell is the resolved -// interpreter (real bash, or PowerShell on a Windows host without bash); the -// zero value resolves lazily. workDir, when non-empty, is the directory the -// command runs in (cmd.Dir); empty uses the process cwd. +// bash runs a shell command. sb, when it enforces, wraps the command in an OS +// sandbox; the zero value registered at init runs unconfined and is overridden +// per run by ConfineBash. shell is the resolved interpreter (real bash, or +// PowerShell on a Windows host without bash); the zero value resolves lazily. +// workDir, when non-empty, is the directory the command runs in (cmd.Dir); +// empty uses the process cwd. timeout optionally caps foreground commands; +// zero or negative means no tool-local cap, while parent context cancellation +// still kills the process tree. type bash struct { sb sandbox.Spec shell sandbox.Shell workDir string + timeout time.Duration } func (bash) Name() string { return "bash" } @@ -65,7 +70,7 @@ func (b bash) resolved() sandbox.Shell { } func (bash) Schema() json.RawMessage { - return json.RawMessage(`{"type":"object","properties":{"command":{"type":"string","description":"Shell command to execute"},"run_in_background":{"type":"boolean","description":"Run detached: returns a job id immediately and keeps running across turns (no timeout). Read new output with bash_output, wait for it with wait, stop it with kill_shell. Use for long-running commands like servers, watchers, or builds you don't need to block on."}},"required":["command"]}`) + return json.RawMessage(`{"type":"object","properties":{"command":{"type":"string","description":"Shell command to execute"},"run_in_background":{"type":"boolean","description":"Run detached: returns a job id immediately and keeps running across turns. Read new output with bash_output, wait for it with wait, stop it with kill_shell. Use for long-running commands like servers, watchers, or builds you don't need to block on."}},"required":["command"]}`) } // ReadOnly is false: bash's effect cannot be inferred from args (rm, curl, @@ -101,7 +106,7 @@ func (b bash) Execute(ctx context.Context, args json.RawMessage) (string, error) return "", fmt.Errorf("background execution is not available in this context") } workDir := b.workDir - // The job runs under the manager's session context (no 120s timeout), so it + // The job runs under the manager's session context (no foreground timeout), so it // survives this turn; its combined output streams to the job buffer. job := jm.Start("bash", commandPreview(p.Command), func(jobCtx context.Context, out io.Writer) (string, error) { cmd := exec.CommandContext(jobCtx, argv[0], argv[1:]...) @@ -115,10 +120,15 @@ func (b bash) Execute(ctx context.Context, args json.RawMessage) (string, error) return fmt.Sprintf("Started background job %q. It keeps running across turns; read new output with bash_output(job_id=%q), wait for it with wait, or stop it with kill_shell(job_id=%q).", job.ID, job.ID, job.ID), nil } - ctx, cancel := context.WithTimeout(ctx, bashTimeout) - defer cancel() + runCtx := ctx + timeout := b.foregroundTimeout() + if timeout > 0 { + var cancel context.CancelFunc + runCtx, cancel = context.WithTimeoutCause(ctx, timeout, errBashTimeout) + defer cancel() + } - cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) + cmd := exec.CommandContext(runCtx, argv[0], argv[1:]...) cmd.Dir = b.workDir // "" lets exec use the process working directory setKillTree(cmd) cmd.WaitDelay = bashWaitDelay @@ -132,8 +142,8 @@ func (b bash) Execute(ctx context.Context, args json.RawMessage) (string, error) err := cmd.Run() out := buf.String() - if ctx.Err() == context.DeadlineExceeded { - return out, fmt.Errorf("command timed out (> %s)", bashTimeout) + if errors.Is(context.Cause(runCtx), errBashTimeout) { + return out, fmt.Errorf("command timed out (> %s)", timeout) } if err != nil { // Non-zero exit: feed output and error back so the model can self-correct. @@ -142,6 +152,13 @@ func (b bash) Execute(ctx context.Context, args json.RawMessage) (string, error) return out, nil } +func (b bash) foregroundTimeout() time.Duration { + if b.timeout <= 0 { + return 0 + } + return b.timeout +} + // progressWriter forwards each chunk the command writes to a tool.ProgressFunc, // so foreground bash output streams to the frontend as it is produced. type progressWriter struct{ emit tool.ProgressFunc } diff --git a/internal/tool/builtin/bash_timeout_test.go b/internal/tool/builtin/bash_timeout_test.go new file mode 100644 index 000000000..db2655db1 --- /dev/null +++ b/internal/tool/builtin/bash_timeout_test.go @@ -0,0 +1,106 @@ +package builtin + +import ( + "context" + "strings" + "testing" + "time" + + "reasonix/internal/sandbox" +) + +func TestBashForegroundTimeoutConfig(t *testing.T) { + sh := sandbox.ResolveShell() + b := bash{shell: sh, timeout: 150 * time.Millisecond} + + start := time.Now() + out, err := b.Execute(context.Background(), argsJSON(t, map[string]any{"command": longSleepCommand(sh)})) + elapsed := time.Since(start) + if err == nil { + t.Fatalf("expected timeout error, got nil (out=%q)", out) + } + if !strings.Contains(err.Error(), "timed out") { + t.Fatalf("error = %v, want timeout", err) + } + if elapsed > 5*time.Second { + t.Fatalf("configured timeout returned too slowly: %v", elapsed) + } +} + +func TestBashZeroTimeoutDoesNotCapForeground(t *testing.T) { + sh := sandbox.ResolveShell() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + start := time.Now() + out, err := (bash{shell: sh}).Execute(ctx, argsJSON(t, map[string]any{"command": oneSecondCommand(sh)})) + elapsed := time.Since(start) + if err != nil { + t.Fatalf("zero-timeout foreground command failed: %v (out=%q)", err, out) + } + if !strings.Contains(out, "done") { + t.Fatalf("output = %q, want done", out) + } + if elapsed < 800*time.Millisecond { + t.Fatalf("command returned too quickly (%v), so the sleep did not run", elapsed) + } +} + +func TestWorkspacePassesBashTimeout(t *testing.T) { + sh := sandbox.ResolveShell() + b := byName(Workspace{Dir: t.TempDir(), BashTimeout: 150 * time.Millisecond}.Tools())["bash"] + + out, err := b.Execute(context.Background(), argsJSON(t, map[string]any{"command": longSleepCommand(sh)})) + if err == nil { + t.Fatalf("expected workspace bash timeout, got nil (out=%q)", out) + } + if !strings.Contains(err.Error(), "timed out") { + t.Fatalf("error = %v, want timeout", err) + } +} + +func longSleepCommand(sh sandbox.Shell) string { + if sh.Kind == sandbox.ShellPowerShell { + return "Start-Sleep -Seconds 2" + } + return "sleep 2" +} + +func oneSecondCommand(sh sandbox.Shell) string { + if sh.Kind == sandbox.ShellPowerShell { + return "Start-Sleep -Seconds 1; Write-Output done" + } + return "sleep 1; printf done" +} + +func BenchmarkBashForegroundTimeoutDisabled(b *testing.B) { + bt := bash{} + ctx := context.Background() + for b.Loop() { + runCtx := ctx + timeout := bt.foregroundTimeout() + if timeout > 0 { + b.Fatal("zero-value bash should not create a timeout context") + } + if runCtx == nil { + b.Fatal("nil context") + } + } +} + +func BenchmarkBashForegroundTimeoutConfigured(b *testing.B) { + bt := bash{timeout: 10 * time.Minute} + ctx := context.Background() + for b.Loop() { + runCtx := ctx + timeout := bt.foregroundTimeout() + if timeout > 0 { + var cancel context.CancelFunc + runCtx, cancel = context.WithTimeoutCause(ctx, timeout, errBashTimeout) + cancel() + } + if runCtx == nil { + b.Fatal("nil context") + } + } +} diff --git a/internal/tool/builtin/confine.go b/internal/tool/builtin/confine.go index c01f68c3c..d988aeb78 100644 --- a/internal/tool/builtin/confine.go +++ b/internal/tool/builtin/confine.go @@ -4,6 +4,7 @@ import ( "fmt" "path/filepath" "strings" + "time" "reasonix/internal/sandbox" "reasonix/internal/tool" @@ -12,8 +13,12 @@ import ( // ConfineBash returns the bash built-in bound to an OS-sandbox spec, overriding // the unconfined instance registered at init. When the spec enforces, bash runs // each command through the sandbox (see package sandbox). -func ConfineBash(spec sandbox.Spec) tool.Tool { - return bash{sb: spec, shell: sandbox.ResolveShell()} +func ConfineBash(spec sandbox.Spec, timeout ...time.Duration) tool.Tool { + b := bash{sb: spec, shell: sandbox.ResolveShell()} + if len(timeout) > 0 { + b.timeout = timeout[0] + } + return b } // ConfineWriters returns the file-writing built-ins (write_file, edit_file, diff --git a/internal/tool/builtin/workspace.go b/internal/tool/builtin/workspace.go index 4e62f64bf..d190b6637 100644 --- a/internal/tool/builtin/workspace.go +++ b/internal/tool/builtin/workspace.go @@ -2,6 +2,7 @@ package builtin import ( "path/filepath" + "time" "reasonix/internal/sandbox" "reasonix/internal/tool" @@ -19,10 +20,11 @@ import ( // root, so writes stay inside the project by default. Bash is the OS-sandbox // spec for the bash tool (as ConfineBash). type Workspace struct { - Dir string - WriteRoots []string - Bash sandbox.Spec - Search SearchSpec + Dir string + WriteRoots []string + Bash sandbox.Spec + BashTimeout time.Duration + Search SearchSpec } // Tools returns the built-in tools bound to the workspace, ready to Add to a @@ -44,7 +46,7 @@ func (w Workspace) Tools(enabled ...string) []tool.Tool { multiEdit{workDir: w.Dir, roots: roots}, deleteRange{workDir: w.Dir, roots: roots}, deleteSymbol{workDir: w.Dir, roots: roots}, - bash{workDir: w.Dir, sb: w.Bash}, + bash{workDir: w.Dir, sb: w.Bash, timeout: w.BashTimeout}, listDir{workDir: w.Dir}, globTool{workDir: w.Dir}, grepTool{workDir: w.Dir, rg: w.Search.RgPath}, diff --git a/reasonix.example.toml b/reasonix.example.toml index 8c8b99417..ad970c950 100644 --- a/reasonix.example.toml +++ b/reasonix.example.toml @@ -83,6 +83,7 @@ effort = "high" [tools] enabled = [] # empty = all built-in tools +bash_timeout_seconds = 0 # 0 = no foreground timeout; cancellation still stops the process # CodeGraph code intelligence (symbol/call-graph search via tree-sitter + SQLite), # a built-in MCP server giving the agent codegraph_* search / context / explore / From 1d36378c3b434b9cbdd95b7197ce163c50e4c80f Mon Sep 17 00:00:00 2001 From: "TaoMu (GTC2080)" <1724815229@qq.com> Date: Thu, 4 Jun 2026 21:18:47 +1000 Subject: [PATCH 2/2] fix: keep bash timeout safety default Keep the historical 120s foreground bash cap when tools.bash_timeout_seconds is omitted, while preserving explicit 0 as the opt-in unlimited setting. --- README.md | 2 +- README.zh-CN.md | 2 +- docs/SPEC.md | 2 +- internal/boot/boot.go | 1 + internal/config/config.go | 17 +++++---- internal/config/render.go | 2 +- internal/config/render_test.go | 8 +++-- internal/config/tools_test.go | 41 ++++++++++++++++++++-- internal/tool/builtin/bash_timeout_test.go | 12 +++---- reasonix.example.toml | 2 +- 10 files changed, 65 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index fb9845341..a974d8bac 100644 --- a/README.md +++ b/README.md @@ -116,7 +116,7 @@ api_key_env = "DEEPSEEK_API_KEY" [tools] enabled = [] # omit/empty = all built-ins -bash_timeout_seconds = 0 # 0 = no foreground timeout; cancellation still stops the process +bash_timeout_seconds = 120 # foreground safety cap; set 0 for no tool-local cap [skills] # paths = ["~/my-skills", "../shared/skills"] # extra custom skill roots diff --git a/README.zh-CN.md b/README.zh-CN.md index 232e3ad81..60f947368 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -112,7 +112,7 @@ api_key_env = "DEEPSEEK_API_KEY" [tools] enabled = [] # 省略/为空 = 全部内置工具 -bash_timeout_seconds = 0 # 0 = 前台命令不设超时；取消操作仍会停止进程 +bash_timeout_seconds = 120 # 前台安全上限；设为 0 表示不设工具层超时 [skills] # paths = ["~/my-skills", "../shared/skills"] # 额外的自定义技能目录 diff --git a/docs/SPEC.md b/docs/SPEC.md index be8b639ec..24a80811a 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -383,7 +383,7 @@ api_key_env = "MIMO_API_KEY" [tools] enabled = [] # omit/empty = all built-ins -bash_timeout_seconds = 0 # 0 = no foreground timeout; cancellation still stops the process +bash_timeout_seconds = 120 # foreground safety cap; set 0 for no tool-local cap [skills] # paths = ["~/my-skills", "../shared/skills"] # extra custom skill roots diff --git a/internal/boot/boot.go b/internal/boot/boot.go index d5e7016eb..c998f255a 100644 --- a/internal/boot/boot.go +++ b/internal/boot/boot.go @@ -17,6 +17,7 @@ import ( "os" "path/filepath" "strings" + "time" "reasonix/internal/agent" "reasonix/internal/codegraph" diff --git a/internal/config/config.go b/internal/config/config.go index 883f883c7..12734d3e7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -424,18 +424,21 @@ func (e *ProviderEntry) HasModel(m string) bool { // ToolsConfig selects which built-in tools are enabled. Empty means all of them. type ToolsConfig struct { Enabled []string `toml:"enabled"` - BashTimeoutSeconds int `toml:"bash_timeout_seconds"` + BashTimeoutSeconds *int `toml:"bash_timeout_seconds"` Search SearchConfig `toml:"search"` } -// BashTimeoutSeconds returns the foreground bash timeout in seconds. Zero means -// no tool-local cap; negative config values are treated as zero so a typo cannot -// produce a nonsensical negative duration. +const defaultBashTimeoutSeconds = 120 + +// BashTimeoutSeconds returns the foreground bash timeout in seconds. An omitted +// config keeps the historical 120s safety cap, explicit 0 disables the +// tool-local cap, and positive values set a custom cap. Negative values fall +// back to the default so a typo cannot silently remove the safety net. func (c *Config) BashTimeoutSeconds() int { - if c.Tools.BashTimeoutSeconds < 0 { - return 0 + if c.Tools.BashTimeoutSeconds == nil || *c.Tools.BashTimeoutSeconds < 0 { + return defaultBashTimeoutSeconds } - return c.Tools.BashTimeoutSeconds + return *c.Tools.BashTimeoutSeconds } // SearchConfig tunes the grep tool's engine. Engine is "auto" (default — use diff --git a/internal/config/render.go b/internal/config/render.go index 6d3049845..9f8b4ceeb 100644 --- a/internal/config/render.go +++ b/internal/config/render.go @@ -172,7 +172,7 @@ func RenderTOML(c *Config) string { } b.WriteString("]\n") } - fmt.Fprintf(&b, "bash_timeout_seconds = %d # 0 = no foreground timeout; cancellation still stops the process\n\n", c.BashTimeoutSeconds()) + fmt.Fprintf(&b, "bash_timeout_seconds = %d # foreground safety cap; set 0 for no tool-local cap\n\n", c.BashTimeoutSeconds()) b.WriteString("[codegraph]\n") fmt.Fprintf(&b, "enabled = %v # built-in MCP server; off by default for first-run sessions\n", c.Codegraph.Enabled) diff --git a/internal/config/render_test.go b/internal/config/render_test.go index d5f3202b8..52a0d7c4c 100644 --- a/internal/config/render_test.go +++ b/internal/config/render_test.go @@ -17,7 +17,7 @@ func TestRenderTOMLRoundTrips(t *testing.T) { orig.Agent.AutoPlanClassifier = "deepseek-flash" orig.Agent.SubagentModel = "mimo-pro" orig.Agent.SubagentModels = map[string]string{"review": "deepseek-pro"} - orig.Tools.BashTimeoutSeconds = 900 + orig.Tools.BashTimeoutSeconds = intPtr(900) orig.Permissions = PermissionsConfig{ Mode: "deny", Deny: []string{"bash(rm -rf*)"}, @@ -107,8 +107,8 @@ func TestRenderTOMLRoundTrips(t *testing.T) { if got.Agent.SubagentModels["review"] != "deepseek-pro" { t.Errorf("subagent_models.review = %q, want deepseek-pro", got.Agent.SubagentModels["review"]) } - if got.Tools.BashTimeoutSeconds != 900 { - t.Errorf("tools.bash_timeout_seconds = %d, want 900", got.Tools.BashTimeoutSeconds) + if got.Tools.BashTimeoutSeconds == nil || *got.Tools.BashTimeoutSeconds != 900 { + t.Errorf("tools.bash_timeout_seconds = %v, want 900", got.Tools.BashTimeoutSeconds) } if g, _ := got.Provider("mimo-pro"); g == nil || g.BaseURL != "http://localhost:8000/v1" { t.Errorf("mimo-pro base_url not preserved: %+v", g) @@ -156,3 +156,5 @@ func TestRenderTOMLRoundTrips(t *testing.T) { } func boolPtr(v bool) *bool { return &v } + +func intPtr(v int) *int { return &v } diff --git a/internal/config/tools_test.go b/internal/config/tools_test.go index e5fa4829e..688a40a40 100644 --- a/internal/config/tools_test.go +++ b/internal/config/tools_test.go @@ -1,11 +1,46 @@ package config -import "testing" +import ( + "testing" -func TestBashTimeoutSecondsNormalizesNegative(t *testing.T) { + "github.com/BurntSushi/toml" +) + +func TestBashTimeoutSecondsDefaultsToSafetyCap(t *testing.T) { + cfg := Default() + if cfg.Tools.BashTimeoutSeconds != nil { + t.Fatalf("default raw bash timeout = %v, want nil", *cfg.Tools.BashTimeoutSeconds) + } + if got := cfg.BashTimeoutSeconds(); got != 120 { + t.Fatalf("BashTimeoutSeconds() = %d, want 120", got) + } +} + +func TestBashTimeoutSecondsAllowsExplicitZero(t *testing.T) { + cfg := Default() + cfg.Tools.BashTimeoutSeconds = intPtr(0) + if got := cfg.BashTimeoutSeconds(); got != 0 { + t.Fatalf("BashTimeoutSeconds() = %d, want 0", got) + } +} + +func TestBashTimeoutSecondsParsesExplicitZero(t *testing.T) { cfg := Default() - cfg.Tools.BashTimeoutSeconds = -1 + if _, err := toml.Decode("[tools]\nbash_timeout_seconds = 0\n", cfg); err != nil { + t.Fatalf("decode explicit zero: %v", err) + } + if cfg.Tools.BashTimeoutSeconds == nil { + t.Fatal("explicit zero decoded as nil") + } if got := cfg.BashTimeoutSeconds(); got != 0 { t.Fatalf("BashTimeoutSeconds() = %d, want 0", got) } } + +func TestBashTimeoutSecondsFallsBackForNegative(t *testing.T) { + cfg := Default() + cfg.Tools.BashTimeoutSeconds = intPtr(-1) + if got := cfg.BashTimeoutSeconds(); got != 120 { + t.Fatalf("BashTimeoutSeconds() = %d, want 120", got) + } +} diff --git a/internal/tool/builtin/bash_timeout_test.go b/internal/tool/builtin/bash_timeout_test.go index db2655db1..4a58f950c 100644 --- a/internal/tool/builtin/bash_timeout_test.go +++ b/internal/tool/builtin/bash_timeout_test.go @@ -27,13 +27,13 @@ func TestBashForegroundTimeoutConfig(t *testing.T) { } } -func TestBashZeroTimeoutDoesNotCapForeground(t *testing.T) { +func TestBashExplicitZeroTimeoutDoesNotCapForeground(t *testing.T) { sh := sandbox.ResolveShell() ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() start := time.Now() - out, err := (bash{shell: sh}).Execute(ctx, argsJSON(t, map[string]any{"command": oneSecondCommand(sh)})) + out, err := (bash{shell: sh, timeout: 0}).Execute(ctx, argsJSON(t, map[string]any{"command": oneSecondCommand(sh)})) elapsed := time.Since(start) if err != nil { t.Fatalf("zero-timeout foreground command failed: %v (out=%q)", err, out) @@ -73,8 +73,8 @@ func oneSecondCommand(sh sandbox.Shell) string { return "sleep 1; printf done" } -func BenchmarkBashForegroundTimeoutDisabled(b *testing.B) { - bt := bash{} +func BenchmarkBashForegroundTimeoutExplicitZero(b *testing.B) { + bt := bash{timeout: 0} ctx := context.Background() for b.Loop() { runCtx := ctx @@ -88,8 +88,8 @@ func BenchmarkBashForegroundTimeoutDisabled(b *testing.B) { } } -func BenchmarkBashForegroundTimeoutConfigured(b *testing.B) { - bt := bash{timeout: 10 * time.Minute} +func BenchmarkBashForegroundTimeoutConfiguredCap(b *testing.B) { + bt := bash{timeout: 120 * time.Second} ctx := context.Background() for b.Loop() { runCtx := ctx diff --git a/reasonix.example.toml b/reasonix.example.toml index ad970c950..0aa6a103b 100644 --- a/reasonix.example.toml +++ b/reasonix.example.toml @@ -83,7 +83,7 @@ effort = "high" [tools] enabled = [] # empty = all built-in tools -bash_timeout_seconds = 0 # 0 = no foreground timeout; cancellation still stops the process +bash_timeout_seconds = 120 # foreground safety cap; set 0 for no tool-local cap # CodeGraph code intelligence (symbol/call-graph search via tree-sitter + SQLite), # a built-in MCP server giving the agent codegraph_* search / context / explore /