Skip to content

feat(opencode): per-tool execution timeout with abort + session recovery#36869

Open
FahadBinHussain wants to merge 1 commit into
anomalyco:devfrom
FahadBinHussain:tool-execution-timeout
Open

feat(opencode): per-tool execution timeout with abort + session recovery#36869
FahadBinHussain wants to merge 1 commit into
anomalyco:devfrom
FahadBinHussain:tool-execution-timeout

Conversation

@FahadBinHussain

@FahadBinHussain FahadBinHussain commented Jul 14, 2026

Copy link
Copy Markdown

Issue for this PR

Related: #20096, #34888, #20216, #34022, #34960

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Tools (built-in and MCP) can hang the agent loop indefinitely when they wedge on a long-running or unresponsive process. This adds a configurable per-tool execution timeout that aborts the call and synthesizes a tool-result so the agent can continue instead of staying stuck on a status=running part forever.

Config surface (all optional, in milliseconds, 0 disables):

field scope default
experimental.tool_timeout global default 600000 (10 min)
experimental.task_timeout task tool only falls back to tool_timeout
agent.tool_timeout per-agent override highest precedence

Precedence: agent.tool_timeout > experimental.task_timeout (task only) > experimental.tool_timeout > DEFAULT_TOOL_TIMEOUT_MS

On timeout the runner fires an AbortController composed with the user's existing abort signal via AbortSignal.any, so both Esc interrupts and timeouts surface through the same ctx.abort path. The catch block synthesizes a tool-result with metadata.timeout=true and a message guiding the agent to retry differently. The underlying process may still be alive — the abort is a best-effort cleanup nudge, not a hard kill.

Related PRs: #20103 covers the same feature area with the same config keys. That PR is from a different contributor and has been open for a while; this one is an independent implementation with a different approach (centralized ToolTimeout.resolve module, composeSignals helper, per-agent override surfaced on Agent.Info). Happy to collaborate or defer if maintainers prefer consolidating.

How did you verify your code works?

  • test/session/tool-timeout.test.ts — 10 tests covering: default fallback, global override, per-agent override, 0 disable, task tool precedence, task_timeout fallback, non-task tools ignoring task_timeout, agent override beating task_timeout, and ToolTimeoutError message shape.
  • bun typecheck in packages/opencode — clean.
  • bun test test/session/retry.test.ts test/tool/task.test.ts — 49 pass, 0 fail.
  • bun test test/session/llm.test.ts failures are pre-existing (mock LLM server route mismatches causing 5s timeouts), confirmed by running the same suite on clean dev.

Screenshots / recordings

N/A.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Tools (built-in and MCP) can now hang the agent loop indefinitely when
they wedge on a long-running or unresponsive process. This adds a
configurable per-tool execution timeout that aborts the call and
synthesizes a tool-result so the agent can continue instead of
staying stuck on a status=running part forever.

Config surface (all optional, in ms, 0 disables):
  - experimental.tool_timeout  (global default, default 600000 / 10 min)
  - experimental.task_timeout  (overrides tool_timeout for the task tool)
  - agent.tool_timeout          (per-agent override, highest precedence)

Precedence: agent.tool_timeout > task_timeout (task only) > tool_timeout
> DEFAULT_TOOL_TIMEOUT_MS.

On timeout the runner fires an AbortController that is composed with
the user's existing abort signal via AbortSignal.any, so both Esc
interrupts and timeouts surface through the same ctx.abort path. The
catch block synthesizes a tool-result with metadata.timeout=true and
a message guiding the agent to retry differently.

See anomalyco#20096, anomalyco#34888, anomalyco#20216, anomalyco#34022, anomalyco#34960.
@github-actions github-actions Bot added the needs:compliance This means the issue will auto-close after 2 hours. label Jul 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Potential Duplicate/Related PRs Found:

  1. PR fix(opencode): add configurable timeout to Task tool #36755: fix(opencode): add configurable timeout to Task tool

    • Directly related: adds timeout configuration specifically for the Task tool, which appears to be a predecessor or overlapping work on the same feature set.
  2. PR feat(tool): configurable timeout protection for tool and task execution #20103: feat(tool): configurable timeout protection for tool and task execution

    • Directly related: earlier attempt at implementing configurable timeout protection for tools and task execution - same core feature area.
  3. PR fix(shell): bound bash-tool hangs via scope teardown instead of multiple timeouts #35245: fix(shell): bound bash-tool hangs via scope teardown instead of multiple timeouts

    • Related: addresses hanging bash tools through scope teardown, an alternative approach to the timeout problem this PR solves.
  4. PR fix: surface task_id in interrupted tool error text for LLM resume #35222: fix: surface task_id in interrupted tool error text for LLM resume

    • Related: handles tool interruption and session recovery messaging, overlaps with the session recovery aspect of this PR.

The most likely duplicates are #36755 and #20103, as they directly address the same feature (configurable tool execution timeouts). You should verify if #36755 is still open and whether this PR supersedes it or builds on top of it.

@github-actions github-actions Bot removed the needs:compliance This means the issue will auto-close after 2 hours. label Jul 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

@zxkjack123

zxkjack123 commented Jul 16, 2026

Copy link
Copy Markdown

Nice work on this PR — the composeSignals approach and per-agent timeout configuration are exactly the right design for this problem.

I took a close look at the code. A couple of observations:

clearTimeout in finally — clean ✅

I initially flagged potential timer leak in tools.ts, but after pulling the branch I can confirm both tool-wrapping blocks correctly guard clearTimeout(timer) inside try...finally. This is clean.

Type safety on config values

tool-timeout.ts resolves experimental?.tool_timeout through cfg.experimental which is Record<string, unknown>. This works at runtime but a misconfigured value like tool_timeout: "60000" (string instead of number) would silently pass through the ?? fallback to DEFAULT without warning. Adding a typeof guard here would catch this early — tiny change, high signal.

Config key name suggestion

experimental.tool_timeout and experimental.task_timeout conflict with the existing experimental.tool_timeout_ms (milliseconds) and experimental.tool_timeout_sec (seconds) patterns used elsewhere in the codebase. Consider experimental.tool_timeout_ms + experimental.task_timeout_ms for consistency.


Overall this is solid. Would love to see this land — we've been tracking production issues with sub-agent hangs (#11865) and this directly addresses them.

@zxkjack123

Copy link
Copy Markdown

I pulled the branch and did a deeper review. The finally { clearTimeout } guard is already in both call sites — my earlier timer-leak concern was unfounded, sorry about that.

One real improvement I found: the AbortController + setTimeout + composeSignals + clearTimeout pattern is duplicated identically at two call sites (~lines 106-116/159-161 and ~427-439/540-542). I extracted it into ToolTimeout.createTimeoutController() which also makes the timer lifecycle testable in isolation.

Submitted as a stacked PR: FahadBinHussain#2

Changes:

  • tool-timeout.ts: add createTimeoutController() with { signal, dispose }
  • tools.ts: two call sites simplified from ~15 lines to 3 lines each
  • tool-timeout.test.ts: 5 new integration tests (timeout firing, signal composition, dispose cleanup, idle state)

All 15 tool-timeout tests pass, typecheck clean. The existing ~30 lines of config schema + resolve logic are untouched — this is purely a refactor of the execution plumbing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants