Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,8 @@ tool call

**Tool wall-clock budget vs. permission prompts.** Each tool `run()` is wrapped by an outer execution watchdog (`src/tui/tool-execution-watchdog.ts`). The watchdog arms only when Settings set `tools.timeoutMs` / `tools.maxTimeoutMs`, or when `run_shell` passes a positive timeout (requested plus slack, so this layer cannot beat shell-guard). The `task` tool is always exempt, regardless of Settings — a sub-agent run is bounded by its own limits (maxTurns, no-progress, thrash, opt-in deadlineMs), so the generic per-tool budget never aborts a healthy long-running worker; parent cancel, maxTurns, and eval `--agent-timeout-ms` still bound the run. By default (`tools.waitForApproval`, Settings → Tools, **On**), an armed budget freezes while the operator is deciding on a permission prompt, so a late approve still runs the tool and the agent waits for the decision instead of timing out under the modal. When **Off**, the budget keeps ticking during the prompt; if it expires first the tool is skipped and the permission modal is dismissed via the budget AbortSignal (auto-deny with a timeout message). The TUI permission queue (`src/tui/gate-wire.ts`, backed by `src/permission/queue.ts`) attaches that signal so ghost prompts cannot outlive an already-aborted tool.

`mcp__*` tool calls are the exception to "arms only when Settings set it": they arm unconditionally with a 5-minute default (`DEFAULT_MCP_TOOL_TIMEOUT_MS`), overridable via `mcp.timeoutMs` and still capped by `tools.maxTimeoutMs` (CL-6895). Nothing else bounds an MCP call — the stall watchdog treats an in-flight tool as activity by design, so a wedged MCP server previously hung a tool call, and the turn, forever. On expiry the call returns a normal tool-error result ("MCP tool `<name>` timed out after `<n>`s — the server may be wedged; retry or continue without it"); the turn is never aborted. The MCP client itself (`src/mcp/client.ts`, wrapping `@modelcontextprotocol/sdk`) multiplexes concurrent requests over one connection by JSON-RPC message id with no serial queue or mutex in our code or in the vendored SDK's `Protocol.request()` — so concurrent calls to the same server are not expected to deadlock each other. Live forensics for CL-6895 showed multi-minute MCP calls that eventually completed successfully, consistent with a slow server response rather than a client-side deadlock.

Approval scopes offered: Allow Once (persist nothing), Allow Always for a file or its directory (file tools), or a command shape (shell). There is intentionally no "all files" rung. Project-scoped Allow Always grants are confined to the session that minted them: they match the session root and its registered git worktrees (`cwdMatchesGrant` in `src/permission/authz-grants.ts` via `createWorktreeRootsProvider`), not bare process-cwd equality — so a grant at the repo root still covers a sub-agent running in a sibling worktree of the same project.

A queued gate's display-dependent timers (auto-deny timeout, tool-budget pause ceiling) arm when the request is actually shown to the operator, not when it is received — a request sitting behind others in the queue does not burn its timeout invisibly. `ask_operator` has the same abort/timeout safety net as the permission gate, so a queued operator question behind a stuck overlay cannot hang a run. Both live in `src/tui/gate-wire.ts`'s `onPermission`/`onOperator`.
Expand Down
11 changes: 11 additions & 0 deletions docs/IMPLEMENTATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,16 @@ Provider and model configuration lives in JSON settings files. The global file h
- `timeoutMs` / `maxTimeoutMs` — outer execution watchdog around each tool `run()`. Unset leaves the watchdog unarmed; set these to arm it. `maxTimeoutMs` clamps non-shell tools when set and does not cap a longer requested `run_shell`. The `task` tool is always exempt: a dispatched sub-agent is bounded by its own limits (maxTurns, no-progress, thrash, opt-in `deadlineMs`), not the generic per-tool budget.
- `waitForApproval` (default **true** when unset) — freeze that budget while a permission prompt is open so a late approve still runs the tool. **Settings → Tools** toggles this live for the next tool call and persists it here. When **false**, the budget keeps ticking during the prompt; on expiry the tool is skipped and the modal is auto-dismissed. The freeze is bounded: after **30 minutes** with the prompt still unanswered the budget resumes ticking on its own, so a prompt that never becomes visible (overlay open, UI gone) cannot hang a tool run indefinitely.

Optional `mcp` block bounds MCP tool calls (`mcp__*` names) specifically — unlike `tools.*`, this arms **unconditionally** even with no settings at all, defaulting to **5 minutes**, since a wedged MCP server otherwise hangs a call forever with nothing to bound it (CL-6895):

```json
"mcp": {
"timeoutMs": 300000
}
```

On expiry the call returns a normal tool-error result ("MCP tool `<name>` timed out after `<n>`s — the server may be wedged; retry or continue without it") that the model can react to; the turn itself is never aborted. `tools.maxTimeoutMs`, if set, still caps `mcp.timeoutMs`.

Optional `subagentMaxTurns` (integer **1–100**, default **30**) sets the default inference-turn budget for dispatched workers (not the parent chat session limit). Per-dispatch `task(maxTurns)` and agent profile `maxTurns` override this default; values above **100** are rejected on `task` and clamped for profiles. Always applies — the primary session is always orchestrator-capable (CL-5814).

Optional `sessionMode` is **deprecated**. Legacy values (`single` | `orchestrator`) may still appear on disk and load without error; resolve always returns **orchestrator**. There is no first-run mode picker and no Settings row. Both the interactive TUI (`runTUI`) and the non-TUI product path (`runExec` / `corbits exec`) are orchestrator-only. Exec bootstrap is otherwise a forked copy of the TUI path (shared stack, intentional deltas documented under Architecture → Exec Runner).
Expand All @@ -253,6 +263,7 @@ All `tools.*` keys live in the global settings file only — there is no per-rep
| `tools.timeoutMs` | unset (watchdog unarmed) | Outer wall-clock budget per tool `run()` when set |
| `tools.maxTimeoutMs` | unset | Cap on the outer budget when set; does not cap a longer requested `run_shell` |
| `tools.waitForApproval` | `true` | Freeze the budget while a permission prompt is open (freeze capped at 30 min); `false` keeps the clock ticking and auto-dismisses the prompt on expiry |
| `mcp.timeoutMs` | **300000** (5 min) — armed even when unset | Outer wall-clock budget for `mcp__*` tool calls specifically; capped by `tools.maxTimeoutMs` when set |

The `waitForApproval` default is resolved once at the watchdog boundary (`resolveWaitForApproval`); toggling **Settings → Tools** updates the live config for the next tool call and persists the value here.

Expand Down
32 changes: 22 additions & 10 deletions src/config/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ export type Settings = {
// the budget keeps ticking during the prompt; if it expires first the tool is
// skipped and the prompt is dismissed.
tools?: { timeoutMs?: number; maxTimeoutMs?: number; waitForApproval?: boolean };
// Wall-clock budget for MCP tool calls specifically (mcp__* names). Unlike
// the generic `tools` budget, this one is armed by default (see
// DEFAULT_MCP_TOOL_TIMEOUT_MS) since a wedged MCP server otherwise hangs a
// tool call forever with nothing to bound it.
mcp?: { timeoutMs?: number };
// Anonymous PostHog telemetry. Global only — never written to per-repo
// local settings. `enabled` defaults to true (opt-out); `installationId`
// is a random UUID generated once on first use; `noticeShown` stamps that
Expand Down Expand Up @@ -244,20 +249,23 @@ export function shellTimeoutFromSettings(
};
}

// Maps the settings tools block to the shape the tool-execution watchdog expects.
// Returns undefined when nothing is configured so callers can skip the override.
// Maps the settings tools/mcp blocks to the shape the tool-execution watchdog
// expects. Returns undefined only when nothing at all is configured so callers
// can skip the override; mcp.timeoutMs alone (with no tools.* set) still
// produces a config, since MCP timeouts are armed unconditionally.
export function toolWatchdogFromSettings(
settings?: Settings | null,
): { defaultMs?: number; maxMs?: number; waitForApproval?: boolean } | undefined {
): { defaultMs?: number; maxMs?: number; waitForApproval?: boolean; mcpTimeoutMs?: number } | undefined {
const tools = settings?.tools;
if (tools === undefined) return undefined;
const hasTimeout = tools.timeoutMs !== undefined || tools.maxTimeoutMs !== undefined;
const hasWait = tools.waitForApproval !== undefined;
if (!hasTimeout && !hasWait) return undefined;
const mcpTimeoutMs = settings?.mcp?.timeoutMs;
const hasTimeout = tools?.timeoutMs !== undefined || tools?.maxTimeoutMs !== undefined;
const hasWait = tools?.waitForApproval !== undefined;
if (!hasTimeout && !hasWait && mcpTimeoutMs === undefined) return undefined;
return {
...(tools.timeoutMs !== undefined ? { defaultMs: tools.timeoutMs } : {}),
...(tools.maxTimeoutMs !== undefined ? { maxMs: tools.maxTimeoutMs } : {}),
...(tools.waitForApproval !== undefined ? { waitForApproval: tools.waitForApproval } : {}),
...(tools?.timeoutMs !== undefined ? { defaultMs: tools.timeoutMs } : {}),
...(tools?.maxTimeoutMs !== undefined ? { maxMs: tools.maxTimeoutMs } : {}),
...(tools?.waitForApproval !== undefined ? { waitForApproval: tools.waitForApproval } : {}),
...(mcpTimeoutMs !== undefined ? { mcpTimeoutMs } : {}),
};
}

Expand Down Expand Up @@ -464,6 +472,9 @@ const SettingsSchema = type({
"maxTimeoutMs?": "number",
"waitForApproval?": "boolean",
}),
"mcp?": type({
"timeoutMs?": "number",
}),
"telemetry?": type({
"enabled?": "boolean",
"installationId?": "string",
Expand Down Expand Up @@ -771,6 +782,7 @@ export async function loadSettings(path: string): Promise<Settings | null> {
: undefined,
shell: s.shell as Settings["shell"] | undefined,
tools: s.tools as Settings["tools"] | undefined,
mcp: s.mcp as Settings["mcp"] | undefined,
telemetry: s.telemetry as Settings["telemetry"] | undefined,
otel: s.otel as Settings["otel"] | undefined,
recentModels: s.recentModels as Settings["recentModels"] | undefined,
Expand Down
8 changes: 8 additions & 0 deletions src/plugins/tool-time-budget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ export function formatToolExecutionTimeoutMessage(
return `${trimmed}\n\n${notice}`;
}

export function formatMcpToolTimeoutMessage(toolName: string, timeoutMs: number): string {
const seconds = Math.round(timeoutMs / 1000);
return (
`MCP tool ${toolName} timed out after ${seconds}s — the server may be wedged; ` +
`retry or continue without it.`
);
}

export function formatReadFileTimeoutMessage(
path: string,
partialResult?: string,
Expand Down
16 changes: 16 additions & 0 deletions src/settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,22 @@ describe("loaders", () => {
});
expect(toolWatchdogFromSettings({ providers: {} })).toBeUndefined();
});

test("toolWatchdogFromSettings maps mcp.timeoutMs alone (no tools.* set)", () => {
expect(toolWatchdogFromSettings({ providers: {}, mcp: { timeoutMs: 45_000 } })).toEqual({
mcpTimeoutMs: 45_000,
});
});

test("toolWatchdogFromSettings merges mcp.timeoutMs alongside tools.*", () => {
expect(
toolWatchdogFromSettings({
providers: {},
tools: { timeoutMs: 120_000, maxTimeoutMs: 600_000 },
mcp: { timeoutMs: 45_000 },
}),
).toEqual({ defaultMs: 120_000, maxMs: 600_000, mcpTimeoutMs: 45_000 });
});
});

describe("persistSkipPermissionsDefault", () => {
Expand Down
105 changes: 105 additions & 0 deletions src/tui/tool-execution-watchdog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test";
import type { AgentTool } from "@intx/agent";
import { createDynamicToolRunner } from "./dynamic-tool-runner.js";
import {
DEFAULT_MCP_TOOL_TIMEOUT_MS,
MAX_TOOL_EXECUTION_TIMEOUT_MS,
RUN_SHELL_WATCHDOG_SLACK_MS,
getToolApprovalBudget,
Expand Down Expand Up @@ -122,6 +123,44 @@ describe("tool execution watchdog", () => {
expect(resolveToolExecutionTimeoutMs({ defaultMs: 9_999_999, maxMs: 100 }, call)).toBe(100);
});

test("mcp tool calls are bounded by default even with no config (CL-6895)", () => {
const call = { id: "1", name: "mcp__linear__get_issue", arguments: {} };
expect(resolveToolExecutionTimeoutMs(undefined, call)).toBe(DEFAULT_MCP_TOOL_TIMEOUT_MS);
expect(resolveToolExecutionTimeoutMs({}, call)).toBe(DEFAULT_MCP_TOOL_TIMEOUT_MS);
});

test("mcp.timeoutMs overrides the mcp default", () => {
const call = { id: "1", name: "mcp__linear__get_issue", arguments: {} };
expect(resolveToolExecutionTimeoutMs({ mcpTimeoutMs: 45_000 }, call)).toBe(45_000);
});

test("tools.defaultMs alone (no mcpTimeoutMs) does not affect the mcp default", () => {
const call = { id: "1", name: "mcp__linear__get_issue", arguments: {} };
expect(resolveToolExecutionTimeoutMs({ defaultMs: 5_000 }, call)).toBe(
DEFAULT_MCP_TOOL_TIMEOUT_MS,
);
});

test("tools.maxTimeoutMs still caps a longer mcp.timeoutMs override", () => {
const call = { id: "1", name: "mcp__linear__get_issue", arguments: {} };
expect(
resolveToolExecutionTimeoutMs({ mcpTimeoutMs: 9_999_999, maxMs: 100 }, call),
).toBe(100);
});

test("non-positive or non-finite mcp.timeoutMs falls back to the default instead of a 1ms timeout", () => {
const call = { id: "1", name: "mcp__linear__get_issue", arguments: {} };
expect(resolveToolExecutionTimeoutMs({ mcpTimeoutMs: 0 }, call)).toBe(
DEFAULT_MCP_TOOL_TIMEOUT_MS,
);
expect(resolveToolExecutionTimeoutMs({ mcpTimeoutMs: -5 }, call)).toBe(
DEFAULT_MCP_TOOL_TIMEOUT_MS,
);
expect(resolveToolExecutionTimeoutMs({ mcpTimeoutMs: NaN }, call)).toBe(
DEFAULT_MCP_TOOL_TIMEOUT_MS,
);
});

test("withTimeout dispose clears timer without leaving hung state", async () => {
const parent = new AbortController();
const budget = withTimeout(parent.signal, 50);
Expand Down Expand Up @@ -158,6 +197,72 @@ describe("tool execution watchdog", () => {
expect(result.content).toBe(formatToolExecutionTimeoutMessage("slow", 30));
});

test(
"mcp tool whose promise never resolves times out with a model-reactable error, turn continues",
async () => {
const runner = createDynamicToolRunner(
[
stringTool(
"mcp__linear__get_issue",
() => new Promise<string>(() => {}), // never resolves — wedged server
),
],
{ mcpTimeoutMs: 30 },
);
const result = await runner.run(
{ id: "1", name: "mcp__linear__get_issue", arguments: {} },
new AbortController().signal,
);
expect(result.isError).toBe(true);
expect(result.content).toContain("mcp__linear__get_issue timed out after 0s");
expect(result.content).toContain("the server may be wedged");
},
10_000,
);

test(
"concurrent mcp tool calls each time out independently",
async () => {
const runner = createDynamicToolRunner(
[
stringTool("mcp__linear__get_issue", () => new Promise<string>(() => {})),
stringTool("mcp__linear__list_issues", async () => "ok"),
],
{ mcpTimeoutMs: 30 },
);
const signal = new AbortController().signal;
const [hung1, hung2, fast] = await Promise.all([
runner.run({ id: "1", name: "mcp__linear__get_issue", arguments: {} }, signal),
runner.run({ id: "2", name: "mcp__linear__get_issue", arguments: {} }, signal),
runner.run({ id: "3", name: "mcp__linear__list_issues", arguments: {} }, signal),
]);
expect(hung1.isError).toBe(true);
expect(hung1.content).toContain("mcp__linear__get_issue timed out");
expect(hung2.isError).toBe(true);
expect(hung2.content).toContain("mcp__linear__get_issue timed out");
expect(fast.content).toBe("ok");
expect(fast.isError).toBeUndefined();
},
10_000,
);

test(
"mcp.timeoutMs: 0 does not instantly time out an mcp tool call (falls back to the default)",
async () => {
const runner = createDynamicToolRunner(
[stringTool("mcp__linear__get_issue", async () => "ok")],
{ mcpTimeoutMs: 0 },
);
const result = await runner.run(
{ id: "1", name: "mcp__linear__get_issue", arguments: {} },
new AbortController().signal,
);
expect(result.isError).toBeUndefined();
expect(result.content).toBe("ok");
},
10_000,
);

test("parent cancel prefers execute salvage body over synthetic aborted", async () => {
const parent = new AbortController();
const salvage = {
Expand Down
Loading
Loading