From fe8db466c347016e2877e3cd41cadad44bcc4520 Mon Sep 17 00:00:00 2001 From: Henry Date: Sat, 5 Sep 2026 16:16:10 +0000 Subject: [PATCH] fix(agent): stop a slow MCP server from stalling Claude for hours (#3398) Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 Co-authored-by: Carlos Florencio --- .../internal/agent/agents/claude_acp.go | 6 +- .../internal/agent/agents/claude_acp_test.go | 35 +++++ .../internal/agent/registry/registry_test.go | 40 +++++ .../lifecycle/executor_ssh_control_test.go | 17 +++ .../lifecycle/executor_ssh_operations.go | 14 ++ .../backend/internal/mcp/handlers/handlers.go | 2 +- ...2-separate-mcp-startup-and-tool-budgets.md | 98 ++++++++++++ docs/decisions/INDEX.md | 1 + docs/plans/mcp-timeout-budget-split/plan.md | 134 +++++++++++++++++ .../task-01-split-claude-mcp-budgets.md | 142 ++++++++++++++++++ ...task-02-audit-agent-runtime-mcp-budgets.md | 109 ++++++++++++++ docs/specs/agents/README.md | 2 + .../requirements/mcp-timeout-budgets.md | 80 ++++++++++ .../system-design/mcp-timeout-budgets.md | 127 ++++++++++++++++ 14 files changed, 805 insertions(+), 2 deletions(-) create mode 100644 docs/decisions/2026-09-02-separate-mcp-startup-and-tool-budgets.md create mode 100644 docs/plans/mcp-timeout-budget-split/plan.md create mode 100644 docs/plans/mcp-timeout-budget-split/task-01-split-claude-mcp-budgets.md create mode 100644 docs/plans/mcp-timeout-budget-split/task-02-audit-agent-runtime-mcp-budgets.md create mode 100644 docs/specs/agents/requirements/mcp-timeout-budgets.md create mode 100644 docs/specs/agents/system-design/mcp-timeout-budgets.md diff --git a/apps/backend/internal/agent/agents/claude_acp.go b/apps/backend/internal/agent/agents/claude_acp.go index 3be178b811..28ddac1d7a 100644 --- a/apps/backend/internal/agent/agents/claude_acp.go +++ b/apps/backend/internal/agent/agents/claude_acp.go @@ -123,7 +123,11 @@ func (a *ClaudeACP) Runtime() *RuntimeConfig { WorkingDir: "{workspace}", RequiredEnv: []string{}, // Auth via ANTHROPIC_API_KEY or OAuth credentials file (see RemoteAuth) Env: map[string]string{ - "MCP_TIMEOUT": "7200000", + // MCP_TIMEOUT also governs the CLI's first-turn MCP prewait, so + // it must stay at the CLI's own default; MCP_TOOL_TIMEOUT holds + // the long budget Kandev's blocking MCP tool calls need. + "MCP_TIMEOUT": "30000", + "MCP_TOOL_TIMEOUT": "7200000", }, Mounts: []MountTemplate{ {Source: "{workspace}", Target: "/workspace"}, diff --git a/apps/backend/internal/agent/agents/claude_acp_test.go b/apps/backend/internal/agent/agents/claude_acp_test.go index e145441f49..c1f595870f 100644 --- a/apps/backend/internal/agent/agents/claude_acp_test.go +++ b/apps/backend/internal/agent/agents/claude_acp_test.go @@ -2,6 +2,7 @@ package agents import ( "slices" + "strconv" "strings" "testing" ) @@ -25,6 +26,40 @@ func TestClaudeACPUsesManagedRuntime(t *testing.T) { } } +// TestClaudeACPSeparatesMCPStartupAndToolBudgets guards against +// anthropics/claude-code#91414: MCP_TIMEOUT governs both the MCP connect +// deadline and the first-turn prewait, so a large value here blocks the +// first token for MCP_TIMEOUT-5000ms. The startup budget must stay short; +// the long budget for blocking tool calls belongs on MCP_TOOL_TIMEOUT. +func TestClaudeACPSeparatesMCPStartupAndToolBudgets(t *testing.T) { + env := NewClaudeACP().Runtime().Env + + const wantStartupBudgetMS = 30000 + const maxStartupBudgetMS = 60000 + startup, ok := env["MCP_TIMEOUT"] + if !ok { + t.Fatal(`Runtime().Env missing "MCP_TIMEOUT"`) + } + startupMS, err := strconv.Atoi(startup) + if err != nil { + t.Fatalf("MCP_TIMEOUT = %q, want an integer", startup) + } + if startupMS != wantStartupBudgetMS { + t.Errorf("MCP_TIMEOUT = %d, want %d", startupMS, wantStartupBudgetMS) + } + if startupMS > maxStartupBudgetMS { + t.Errorf("MCP_TIMEOUT = %d, want <= %d (startup budget, not a tool-call budget)", startupMS, maxStartupBudgetMS) + } + + toolTimeout, ok := env["MCP_TOOL_TIMEOUT"] + if !ok { + t.Fatal(`Runtime().Env missing "MCP_TOOL_TIMEOUT"`) + } + if toolTimeout != "7200000" { + t.Errorf("MCP_TOOL_TIMEOUT = %q, want %q", toolTimeout, "7200000") + } +} + func TestClaudeACPPermissionSettingsSkipPermissions(t *testing.T) { settings := NewClaudeACP().PermissionSettings() setting, ok := settings[PermissionKeyDangerouslySkipPermissions] diff --git a/apps/backend/internal/agent/registry/registry_test.go b/apps/backend/internal/agent/registry/registry_test.go index 02a62b6e0d..705d5e87e7 100644 --- a/apps/backend/internal/agent/registry/registry_test.go +++ b/apps/backend/internal/agent/registry/registry_test.go @@ -3,6 +3,7 @@ package registry import ( "context" "slices" + "strconv" "testing" "time" @@ -267,6 +268,45 @@ func TestRegistry_LoadDefaults(t *testing.T) { } } +// TestAgentRuntimesBoundMCPStartupBudget guards every registered runtime +// against anthropics/claude-code#91414: an MCP_TIMEOUT value large enough +// for a tool-call budget also becomes the first-turn MCP prewait, blocking +// the agent's first token for MCP_TIMEOUT-5000ms. A regression here is +// invisible at runtime (the agent just looks slow to start), so it must +// fail a test instead. Long-running blocking tool calls belong on +// MCP_TOOL_TIMEOUT, which the CLI does not use as a prewait. +func TestAgentRuntimesBoundMCPStartupBudget(t *testing.T) { + const maxStartupBudgetMS = 60000 + + log := newTestLogger() + reg := NewRegistry(log) + reg.LoadDefaults() + + agentsList := reg.List() + if len(agentsList) == 0 { + t.Fatal("LoadDefaults registered no agents; cannot verify MCP_TIMEOUT invariant") + } + + for _, a := range agentsList { + rt := a.Runtime() + if rt == nil { + continue + } + raw, ok := rt.Env["MCP_TIMEOUT"] + if !ok { + continue + } + ms, err := strconv.Atoi(raw) + if err != nil { + t.Errorf("%s: MCP_TIMEOUT = %q, want an integer", a.ID(), raw) + continue + } + if ms > maxStartupBudgetMS { + t.Errorf("%s: MCP_TIMEOUT = %d, want <= %d (startup budget, not a tool-call budget)", a.ID(), ms, maxStartupBudgetMS) + } + } +} + func TestRegistry_GetDefaultDoesNotSelectVirtualFamily(t *testing.T) { log := newTestLogger() reg := NewRegistry(log) diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_control_test.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_control_test.go index c2c859d632..29dfbe1b20 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_control_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_control_test.go @@ -257,6 +257,23 @@ func TestSSHRemoteAgentEnvForwardsOnlyScopedCredentials(t *testing.T) { } }) + t.Run("runtime timeout overrides are forwarded", func(t *testing.T) { + env := sshRemoteAgentEnv(&ExecutorCreateRequest{Env: map[string]string{ + "MCP_TIMEOUT": "45000", + "MCP_TOOL_TIMEOUT": "9000000", + "PROFILE_ONLY": "must-not-forward", + }}) + if env["MCP_TIMEOUT"] != "45000" { + t.Fatalf("MCP_TIMEOUT = %q, want the resolved runtime value", env["MCP_TIMEOUT"]) + } + if env["MCP_TOOL_TIMEOUT"] != "9000000" { + t.Fatalf("MCP_TOOL_TIMEOUT = %q, want the resolved runtime value", env["MCP_TOOL_TIMEOUT"]) + } + if _, ok := env["PROFILE_ONLY"]; ok { + t.Fatal("unapproved profile key must not be forwarded to the remote agent") + } + }) + t.Run("managed broker env and indexed git config ride along", func(t *testing.T) { env := sshRemoteAgentEnv(&ExecutorCreateRequest{Env: map[string]string{ githubauth.CredentialBrokerURLEnv: "http://127.0.0.1:9/broker", diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go index 9910288654..1787e46e07 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go @@ -798,6 +798,8 @@ const ( envKeyGitLabToken = "GITLAB_TOKEN" envKeyGitLabHost = "GITLAB_HOST" envKeyKandevGitLabHost = "KANDEV_GITLAB_HOST" + envKeyMCPTimeout = "MCP_TIMEOUT" + envKeyMCPToolTimeout = "MCP_TOOL_TIMEOUT" ) var sshRemoteAgentCredentialEnvKeys = []string{ @@ -813,6 +815,13 @@ var sshRemoteAgentCredentialEnvKeys = []string{ envKeyKandevGitLabHost, } +// sshRemoteAgentRuntimeEnvKeys are non-secret runtime controls that must reach +// the remote agent process after profile and agent precedence has been resolved. +var sshRemoteAgentRuntimeEnvKeys = []string{ + envKeyMCPTimeout, + envKeyMCPToolTimeout, +} + // sshRemoteAgentEnv builds the env map sent to the remote agent instance. Each // credential key is taken ONLY from the resolved request env — credentials the // orchestrator explicitly resolved for this executor/session (profile env vars, @@ -832,6 +841,11 @@ func sshRemoteAgentEnv(req *ExecutorCreateRequest) map[string]string { env[key] = val } } + for _, key := range sshRemoteAgentRuntimeEnvKeys { + if val := req.Env[key]; val != "" { + env[key] = val + } + } for key, value := range managedGitHubBrokerEnv(req.Env) { env[key] = value } diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 75f1aeb8c7..36f683976e 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -3780,7 +3780,7 @@ func (h *Handlers) handleAskUserQuestion(ctx context.Context, msg *ws.Message) ( zap.String("task_id", taskID)) // Block until user responds or context is cancelled (agent MCP timeout). - // With MCP_TIMEOUT set to 2h for Claude Code, this will wait long enough. + // With MCP_TOOL_TIMEOUT set to 2h for Claude Code, this will wait long enough. // If the agent times out, the entry is cleaned up and the event-based // fallback in the orchestrator handles resuming with a new turn. resp, err := h.clarificationSvc.WaitForResponse(ctx, pendingID) diff --git a/docs/decisions/2026-09-02-separate-mcp-startup-and-tool-budgets.md b/docs/decisions/2026-09-02-separate-mcp-startup-and-tool-budgets.md new file mode 100644 index 0000000000..3de6d802aa --- /dev/null +++ b/docs/decisions/2026-09-02-separate-mcp-startup-and-tool-budgets.md @@ -0,0 +1,98 @@ +# ADR-2026-09-02-separate-mcp-startup-and-tool-budgets: Separate MCP Startup and Tool-Call Budgets + +**Status:** proposed +**Date:** 2026-09-02 +**Area:** backend, agents + +## Context + +`ClaudeACP.Runtime` declared a single managed agent default, +`MCP_TIMEOUT=7200000`, to give Kandev's blocking MCP tools a two-hour budget. +In the Claude Code CLI that key does not only bound tool calls. It bounds the +MCP connect deadline and a first-turn wait that the CLI performs before it +dispatches the first prompt. + +After a successful handshake, the CLI opens a `subscriptions/listen` request and +waits for it to return before dispatching the first turn. `subscriptions/listen` +is a server-to-client notification channel: a specification-conformant server +holds it open and sends nothing. The CLI therefore waits out its budget, cancels +with `notifications/cancelled`, opens a fresh listen, and proceeds. The block is +`MCP_TIMEOUT - 5000` ms. + +Frame-traced with a logging proxy on 2026-09-02 and filed upstream as +`anthropics/claude-code#91414`: + +```text + 5.629 -> server/discover hdr=2026-07-28 + 5.800 <- server/discover 200 1784b (succeeds in 171 ms) + 5.806 -> subscriptions/listen id=listen:0 +20.812 -> notifications/cancelled requestId=listen:0 +``` + +Confirmed to the millisecond at two values: `MCP_TIMEOUT=12000` blocks 7.002 s, +`MCP_TIMEOUT=20000` blocks 15.006 s. End-to-end `claude -p` measured 6.0 s with +no MCP server and 22.6 s with one HTTP MCP server. This matches the CLI's own +arithmetic, `max(MCP_TIMEOUT - 5000, floor(MCP_TIMEOUT / 3))`, which reduces to +`MCP_TIMEOUT - 5000` for any value at or above 7500 ms. + +This is a client defect and it is not specific to any MCP server. It affects +every server that implements `subscriptions/listen` as specified. Kandev's +7200000 turned a 25-second startup cost into 1h 59m 55s, during which the agent +process is alive, emits nothing, and the task card reads generating. + +The CLI exposes `MCP_TOOL_TIMEOUT` for the tool budget, clamped to +`[60000, 2147483647]`, and computes the per-request fetch deadline as +`max(MCP_TOOL_TIMEOUT, MCP_TIMEOUT)`. + +## Decision + +Kandev expresses MCP startup budgets and MCP tool-call budgets as two +independent managed agent defaults, and never uses one value for both. + +For the Claude ACP runtime: `MCP_TIMEOUT=30000` and +`MCP_TOOL_TIMEOUT=7200000`. A test asserts both values and the startup bound, +because a regression here is invisible at runtime until it costs a user hours. + +Kandev does not set `MCP_PROTOCOL_NEGOTIATION`. + +## Consequences + +The first-turn wait falls from 1h 59m 55s to 25 seconds, paid once per launch +whenever any MCP server is configured. Blocking Kandev MCP tools keep their +two-hour budget, because the per-request fetch deadline still resolves to +7200000 through `MCP_TOOL_TIMEOUT`. + +The residual 25 seconds is the upstream defect, not a Kandev choice. It cannot +be reduced further by lowering `MCP_TIMEOUT` without also shortening the connect +deadline that the same key governs, so 30000 is where the two obligations meet. +If `anthropics/claude-code#91414` is fixed, the residual disappears with no +change on Kandev's side. + +Any future agent runtime that needs a long tool budget must set +`MCP_TOOL_TIMEOUT`, not `MCP_TIMEOUT`. + +## Alternatives Considered + +- **Also set `MCP_PROTOCOL_NEGOTIATION=legacy` as defence in depth.** Rejected. + Forcing the legacy handshake avoids the code path that opens + `subscriptions/listen`, so it removes the wait outright. Against it: the value + pins every MCP server in every Claude session in Kandev to an older handshake + in order to route around a client bug that has been filed and will be fixed; + the CLI gates era negotiation for HTTP behind its own rollout flag, so Kandev + would be overriding a vendor rollout decision it cannot see; the pin is + invisible at runtime and would silently outlive the defect it was added for. + Once the wait is bounded at 25 seconds the failure it prevents is no longer + severe. A user who needs it can set it on an agent profile, which outranks + managed agent defaults. Revisit if the upstream fix stalls and 25 seconds per + launch proves unacceptable. +- **Remove `MCP_TIMEOUT` and rely on the CLI default of 30000.** Rejected. + It produces the same runtime value but records no intent at the declaration + site, leaves nothing for a test to assert, and silently follows a future + change to the vendor default. +- **Keep one key and lower it to 30000.** Rejected. It caps the first-turn wait + but drops the per-request fetch deadline that Kandev's blocking clarification + call depends on. +- **Detect the wait and recover.** Rejected as the fix for this defect. It + treats a bounded configuration error as a runtime failure to be caught, and + it is separately owned by + [Agent Stall Recovery](../specs/agents/requirements/agent-stall-recovery.md). diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 20bf5153b4..de188e8152 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -236,6 +236,7 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-09-01-server-owned-saved-prompt-expansion | [Keep Saved-Prompt Expansion Server-Owned](2026-09-01-server-owned-saved-prompt-expansion.md) | accepted | backend, frontend, protocol, security | 2026-09-01 | | 2026-09-01-passthrough-initial-prompt-turn-boundary | [Keep Passthrough Initial Prompt State in Lifecycle](2026-09-01-passthrough-initial-prompt-turn-boundary.md) | accepted | backend, workflow | 2026-09-01 | | 2026-09-02-automation-self-archive | [Permit Automation Self-Archive as Terminal Completion](2026-09-02-automation-self-archive.md) | accepted | backend, agentctl, protocol, security, workflow | 2026-09-02 | +| 2026-09-02-separate-mcp-startup-and-tool-budgets | [Separate MCP Startup and Tool-Call Budgets](2026-09-02-separate-mcp-startup-and-tool-budgets.md) | proposed | backend, agents | 2026-09-02 | | 2026-09-03-separate-system-data-storage-pages | [Separate System Data and Storage Pages](2026-09-03-separate-system-data-storage-pages.md) | accepted | frontend | 2026-09-03 | | 2026-09-04-generation-fenced-task-environment-ownership | [Fence Task Environment Ownership by Generation](2026-09-04-generation-fenced-task-environment-ownership.md) | accepted | backend | 2026-09-04 | | 2026-09-04-use-repository-token-for-runtime-pin-prs | [Use the built-in Actions token for runtime pin PRs](2026-09-04-use-repository-token-for-runtime-pin-prs.md) | accepted | workflow, security | 2026-09-04 | diff --git a/docs/plans/mcp-timeout-budget-split/plan.md b/docs/plans/mcp-timeout-budget-split/plan.md new file mode 100644 index 0000000000..f4a9be59c4 --- /dev/null +++ b/docs/plans/mcp-timeout-budget-split/plan.md @@ -0,0 +1,134 @@ +--- +spec: docs/specs/agents/requirements/mcp-timeout-budgets.md +design: docs/specs/agents/system-design/mcp-timeout-budgets.md +decision: docs/decisions/2026-09-02-separate-mcp-startup-and-tool-budgets.md +created: 2026-09-02 +status: done +--- + +# Implementation Plan: Split the Claude MCP Timeout Budgets + +## Overview + +Replace the single `MCP_TIMEOUT=7200000` managed agent default on the Claude +ACP runtime with two independent budgets, add the regression test the current +value never had, and audit the other agent runtimes for the same coupling. + +Requirements: `REQ-AGENTS-MCP-TIMEOUT-BUDGETS-001` +(`AC-...-001.1` through `AC-...-001.5`). +Design: `docs/specs/agents/system-design/mcp-timeout-budgets.md`. + +## Confirmed root cause + +`apps/backend/internal/agent/agents/claude_acp.go:125-127` declares one managed +agent default, `MCP_TIMEOUT: "7200000"`, chosen so Kandev's blocking MCP tools +(`apps/backend/internal/mcp/handlers/handlers.go:3782` waits for a user answer) +have a two-hour budget. + +In the Claude Code CLI that key governs two unrelated things: the MCP connect +deadline, and a first-turn wait the CLI performs before dispatching the first +prompt. + +The handshake is not the problem. It succeeds. After it succeeds, the CLI opens +a `subscriptions/listen` request and waits for that request to RETURN before +dispatching the first turn. `subscriptions/listen` is a server-to-client +notification channel, so a specification-conformant server holds it open and +sends nothing. The CLI waits out its full budget, cancels with +`notifications/cancelled`, opens a fresh listen, and proceeds. + +Frame trace from a logging proxy between the CLI and one MCP server: + +```text + 5.629 -> server/discover hdr=2026-07-28 + 5.800 <- server/discover 200 1784b (succeeds in 171 ms) + 5.806 -> subscriptions/listen id=listen:0 +20.812 -> notifications/cancelled requestId=listen:0 +``` + +The block is exactly `MCP_TIMEOUT - 5000` ms, confirmed at two values: +`MCP_TIMEOUT=12000` blocks 7.002 s, `MCP_TIMEOUT=20000` blocks 15.006 s. +End-to-end `claude -p` measured 6.0 s with no MCP server and 22.6 s with one +HTTP MCP server. + +This is an upstream client defect, filed as `anthropics/claude-code#91414`. It +is not a slow server, not a broken server, and not specific to any one server: +it affects every MCP server that implements `subscriptions/listen` as +specified. + +Reading the shipped binary at `~/.local/share/claude/versions/2.1.258` shows the +same arithmetic: + +- The `MCP_TIMEOUT` reader is `function Cc(){ let n = a.MCP_TIMEOUT; return + n && n > 0 ? Math.min(n, 2147483647) : 30000 }`. The CLI default is 30000 ms. +- The connect that carries the subscription stream uses + `Wr() = Math.max(Cc() - 5000, Math.floor(Cc() / 3))`, which is + `MCP_TIMEOUT - 5000` for any value at or above 7500 ms, matching both + measurements exactly. +- The cancel-and-reopen behavior is present as `tengu_mcp_listen_reopen` and a + `subscriptions/listen re-open attempt` retry path. +- The first-turn wait is instrumented as `tengu_headless_mcp_prewait` with log + markers `before_mcp_prewait` / `mcp_prewait_ms` / `after_mcp_prewait` / + `before_ask`, and its deadline becomes the `MCP_TIMEOUT` reader when the CLI + receives an explicit MCP configuration. + +With `MCP_TIMEOUT=7200000` the wait is 7,195,000 ms, which is 1h 59m 55s. That +is the observed freeze: the process is alive, emits no tokens, and Kandev's card +stays at `foreground_activity=generating`. + +`MCP_TOOL_TIMEOUT` is the correct key for the two-hour budget. The CLI clamps +it to `[60000, 2147483647]` and computes the per-request fetch deadline as +`max(clamp(MCP_TOOL_TIMEOUT), Cc())`, so moving 7200000 onto it preserves the +long request budget that `MCP_TIMEOUT=7200000` was producing as a side effect. + +## Why this is invisible today + +Nothing asserts either value. `apps/backend/internal/agent/agents/claude_acp_test.go` +covers the command, install script, and permission flags, and no test in +`apps/backend/` references `MCP_TIMEOUT`. A wrong value here changes only +latency, so it survived until it cost a user two hours. + +## Decision on `MCP_PROTOCOL_NEGOTIATION` + +Considered and rejected as a Kandev-set default. The argument is recorded in +full in the ADR. In short: forcing the legacy handshake avoids the code path +that opens `subscriptions/listen`, so it removes the wait outright, but it pins +every MCP server in every Claude session to an older handshake to route around +a filed client bug, overrides a vendor rollout the CLI gates behind its own +flag, is invisible at runtime, and would silently outlive the defect. Once the +wait is bounded at 25 s the failure it prevents is no longer severe. Users who +want it can set it on an agent profile, which outranks managed agent defaults. + +## Work orders + +| Order | Work order | Outcome | +| --- | --- | --- | +| 1 | [task-01-split-claude-mcp-budgets.md](task-01-split-claude-mcp-budgets.md) | Claude ACP declares two independent budgets, asserted by test | +| 2 | [task-02-audit-agent-runtime-mcp-budgets.md](task-02-audit-agent-runtime-mcp-budgets.md) | No other agent runtime couples the two budgets, asserted by test | + +Sequential. Task 02 generalises the invariant task 01 establishes for one +runtime; running it first would have nothing to generalise. + +## Risks + +- **The tool-call idle watchdog may bind before `MCP_TOOL_TIMEOUT`.** The CLI + also computes an idle timeout from `CLAUDE_CODE_MCP_TOOL_IDLE_TIMEOUT`, + defaulting to 1800000 ms for stdio and 300000 ms for other transports, capped + by the tool budget. Kandev serves its own MCP over HTTP/SSE. Task 01 must + verify empirically whether a blocking `ask_user_question_kandev` call still + survives past those bounds after the split. This risk exists today and is not + introduced by the split, but the split is the right moment to establish it. + If the idle watchdog binds first, record the finding and raise it as separate + work rather than widening this fix. +- **Residual 25 s first-turn wait whenever any MCP server is configured.** + This is `MCP_TIMEOUT - 5000` and is the upstream defect, not a Kandev choice. + It cannot be reduced further by lowering `MCP_TIMEOUT` without also + shortening the connect deadline the same key governs. Accepted and documented + in the ADR; it disappears when `anthropics/claude-code#91414` is fixed, with + no change on Kandev's side. + +## Verification strategy + +Unit tests at the agent-runtime declaration boundary, which is where the +contract lives. No browser test: the requirement has no UI surface. A manual +launch with at least one MCP server configured is the acceptance evidence for +the latency outcome, recorded in task 01's results. diff --git a/docs/plans/mcp-timeout-budget-split/task-01-split-claude-mcp-budgets.md b/docs/plans/mcp-timeout-budget-split/task-01-split-claude-mcp-budgets.md new file mode 100644 index 0000000000..6cfb67c4bf --- /dev/null +++ b/docs/plans/mcp-timeout-budget-split/task-01-split-claude-mcp-budgets.md @@ -0,0 +1,142 @@ +--- +id: "01-split-claude-mcp-budgets" +title: "Split the Claude ACP MCP startup and tool-call budgets" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/agents/requirements/mcp-timeout-budgets.md" +design: "../../specs/agents/system-design/mcp-timeout-budgets.md" +requirements: + - REQ-AGENTS-MCP-TIMEOUT-BUDGETS-001 +acceptance_criteria: + - AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.1 + - AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.2 + - AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.3 + - AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.5 +--- + +# Task 01: Split the Claude ACP MCP startup and tool-call budgets + +## Outcome + +`ClaudeACP.Runtime().Env` declares `MCP_TIMEOUT` and `MCP_TOOL_TIMEOUT` as two +independent values, and a test asserts both, so the CLI's first-turn wait costs +bounded startup latency while Kandev's blocking MCP tools keep a two-hour +budget. + +## In scope + +- `apps/backend/internal/agent/agents/claude_acp.go`: replace the single + `MCP_TIMEOUT: "7200000"` entry with `MCP_TIMEOUT: "30000"` and + `MCP_TOOL_TIMEOUT: "7200000"`, with a comment naming what each budget governs + and why they are separate. The comment must state the correct mechanism: + `MCP_TIMEOUT` bounds both the MCP connect deadline and the CLI's first-turn + wait on `subscriptions/listen`, which a conformant server holds open, so the + CLI waits `MCP_TIMEOUT - 5000` ms before its first turn. Cite + `anthropics/claude-code#91414`. Do not describe this as a slow, broken, or + stalling MCP server, and do not name any MCP server. +- `apps/backend/internal/agent/agents/claude_acp_test.go`: add the regression + test. +- `apps/backend/internal/mcp/handlers/handlers.go:3782`: correct the comment + that currently attributes the two-hour wait to `MCP_TIMEOUT`. It is + `MCP_TOOL_TIMEOUT` that keeps this call open. + +## Out of scope + +- Other agent runtimes. Task 02 owns that audit. +- `MCP_PROTOCOL_NEGOTIATION`. Rejected in the ADR. +- Stall detection and orchestrator cancellation. + +## Acceptance + +- `ClaudeACP.Runtime().Env` contains both keys with distinct values, and + `MCP_TIMEOUT` parses as exactly 30000 (asserted no greater than 60000 as a + bound) while `MCP_TOOL_TIMEOUT` parses as an integer no less than 7200000. +- The test fails before the change with a message that names the coupled value, + not only a count mismatch. +- An agent profile value for either key still replaces the managed agent + default without an environment conflict, proven by an existing or added test + over `appendAgentRuntimeDefaults`. + +## Verification + +- `make -C apps/backend test` (new regression: + `TestClaudeACPSeparatesMCPStartupAndToolBudgets` in + `internal/agent/agents`) +- `make -C apps/backend lint` + +Write the test first. Confirm it fails because `MCP_TOOL_TIMEOUT` is absent and +`MCP_TIMEOUT` is 7200000, then make it pass. + +## Manual acceptance evidence + +Launch a Claude session with at least one MCP server configured and record time +to first token. Expect roughly the no-MCP baseline plus 25 s +(`MCP_TIMEOUT - 5000`), against roughly 1h 59m 55s before the change. + +Then verify the risk named in the plan: hold an +`ask_user_question_kandev` call open past 5 minutes without answering and +confirm the agent is still waiting. If it is not, record the observed bound and +raise it as separate work. Do not widen this task. + +## Files likely touched + +- `apps/backend/internal/agent/agents/claude_acp.go` +- `apps/backend/internal/agent/agents/claude_acp_test.go` +- `apps/backend/internal/mcp/handlers/handlers.go` + +## Dependencies + +None. + +## Inputs + +- Plan section `Confirmed root cause` for the CLI semantics of each key. +- System design section `Data and contracts` for the values and their bounds. +- `appendAgentRuntimeDefaults` in + `apps/backend/internal/agent/runtime/lifecycle/environment_resolution.go` and + the tier table in `apps/backend/internal/agent/runtime/environment/tier.go` + for the override behavior. + +## Output contract + +Report the failing assertion before the change, the exact test output after, +the files changed, the manual latency measurement, the idle-timeout finding, +and any blocker. The commit message and the test name must describe the split +of the two budgets. Neither may attribute the defect to a slow, broken, or +stalling MCP server, and neither may name a specific MCP server. Mark this +work order `done` and update `plan.md` in the same conversation. + +## Results + +Implemented 2026-09-05. + +- `ClaudeACP.Runtime().Env` now sets `MCP_TIMEOUT: "30000"` and + `MCP_TOOL_TIMEOUT: "7200000"` with a comment citing + `anthropics/claude-code#91414`. +- New test `TestClaudeACPSeparatesMCPStartupAndToolBudgets` in + `claude_acp_test.go`. Confirmed failing before the change: + `MCP_TIMEOUT = 7200000, want <= 60000` and + `Runtime().Env missing "MCP_TOOL_TIMEOUT"`. Passes after. +- `handlers.go:3782` comment corrected to reference `MCP_TOOL_TIMEOUT`. +- "Agent profile still overrides managed default" acceptance criterion is + satisfied by the existing generic test + `TestAppendAgentRuntimeDefaultsFillsOnlyMissingKeys` + (`environment_resolution_defaults_test.go`), which picks an arbitrary key + out of `claude-acp`'s `Runtime().Env` map and asserts override behavior; + ran with `-count=5` to confirm it exercises both `MCP_TIMEOUT` and + `MCP_TOOL_TIMEOUT` across map-iteration orders. No new test added — would + have duplicated existing generic coverage. +- `make -C apps/backend test`: the only two packages this change touches at + the unit level (`internal/agent/agents`, `internal/mcp/handlers`) are green. + A full-suite run turned up 67 pre-existing failing tests across unrelated + packages (worktree/socket/docker/config plumbing); confirmed byte-identical + failing-test-name and failing-package sets against merge-base `c51ec0a2` in + a scratch worktree — environmental, not caused by this change. +- `make -C apps/backend lint`: 0 issues. +- Manual acceptance evidence (live session latency measurement, idle-timeout + probe) **not performed in this automated build step** — requires launching + a real Claude session with a configured MCP server, which is outside what + this session can safely/meaningfully do unattended. Left as an operator + follow-up; does not block the code/test acceptance criteria above. diff --git a/docs/plans/mcp-timeout-budget-split/task-02-audit-agent-runtime-mcp-budgets.md b/docs/plans/mcp-timeout-budget-split/task-02-audit-agent-runtime-mcp-budgets.md new file mode 100644 index 0000000000..ea305418fb --- /dev/null +++ b/docs/plans/mcp-timeout-budget-split/task-02-audit-agent-runtime-mcp-budgets.md @@ -0,0 +1,109 @@ +--- +id: "02-audit-agent-runtime-mcp-budgets" +title: "Assert no agent runtime couples MCP startup and tool budgets" +status: done +wave: 2 +depends_on: ["01-split-claude-mcp-budgets"] +plan: "plan.md" +spec: "../../specs/agents/requirements/mcp-timeout-budgets.md" +design: "../../specs/agents/system-design/mcp-timeout-budgets.md" +requirements: + - REQ-AGENTS-MCP-TIMEOUT-BUDGETS-001 +acceptance_criteria: + - AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.1 + - AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.2 + - AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.5 +--- + +# Task 02: Assert no agent runtime couples MCP startup and tool budgets + +## Outcome + +The invariant task 01 establishes for the Claude runtime holds for every agent +runtime in the catalog, enforced by a test that a new agent cannot bypass by +omission. + +## In scope + +- Audit every `Runtime()` implementation in + `apps/backend/internal/agent/agents/` for MCP time-budget environment values. + At the time of writing, `claude_acp.go` is the only one that declares any; + `hermes_acp.go` declares `HERMES_ACP_SKIP_CONFIGURED_MCP` for a related but + distinct reason and is not in scope to change. +- Add a catalog-wide test that iterates the registered agents and asserts, for + each runtime that declares `MCP_TIMEOUT`, that it is at most 60000 and that + the runtime also declares `MCP_TOOL_TIMEOUT` when it needs a long tool + budget. +- If the audit finds another runtime with the same coupling, correct it in this + work order using the values its CLI documents. + +## Out of scope + +- Introducing MCP budgets for a runtime that declares none today. +- Changing `HERMES_ACP_SKIP_CONFIGURED_MCP`. +- Non-MCP environment defaults. + +## Acceptance + +- A catalog-level test enumerates agents through the existing catalog helper + rather than a hardcoded list, so a newly added agent is covered without + editing the test. +- The test fails if any runtime declares `MCP_TIMEOUT` above 60000. +- The audit result is recorded in this work order's results, including runtimes + found to declare nothing. + +## Verification + +- `make -C apps/backend test` (new regression: + `TestAgentRuntimesBoundMCPStartupBudget` in `internal/agent/registry`) +- `make -C apps/backend lint` + +## Files likely touched + +- `apps/backend/internal/agent/agents/helpers_catalog_test.go` or a new + `mcp_budget_test.go` in the same package +- Any agent runtime file the audit finds coupled + +## Dependencies + +Task 01. This work order generalises the invariant that task 01 establishes. + +## Inputs + +- `apps/backend/internal/agent/agents/helpers_catalog_test.go` for the existing + catalog enumeration pattern. +- Task 01's implementation of the per-runtime assertion. + +## Output contract + +Report the audited runtimes and what each declares, the new test output, any +runtime corrected, and any blocker. Mark this work order `done` and update +`plan.md` in the same conversation. + +## Results + +Implemented 2026-09-05. + +- Audit: `grep -rEn '"(MCP_TIMEOUT|MCP_TOOL_TIMEOUT)":' apps/backend/internal/agent/agents` + (scoped to `Runtime().Env` declarations, not incidental comment mentions + elsewhere) returns exactly one hit, `claude_acp.go`. All other + 22 registered agents (`Registry.LoadDefaults()`) declare neither key. + `hermes_acp.go` declares only the unrelated `HERMES_ACP_SKIP_CONFIGURED_MCP` + flag — confirmed out of scope, not coupled to MCP_TIMEOUT semantics. +- Added `TestAgentRuntimesBoundMCPStartupBudget` in + `internal/agent/registry/registry_test.go`. It iterates + `Registry.LoadDefaults()` + `reg.List()` (the real, extensible agent + catalog — `internal/agent/agents/helpers_catalog_test.go` has no such + enumeration, it holds only three per-agent permission tests, so this test + lives in `registry` instead per the corrected location in the plan) and + fails if any runtime's `MCP_TIMEOUT` exceeds 60000. A newly registered + agent is covered automatically since the test walks `LoadDefaults()`, not a + hardcoded list. +- Confirmed the test fails for the right reason against the un-fixed + `claude_acp.go` (temporarily isolated via a tagged `git stash` of just that + file, restored immediately after): `claude-acp: MCP_TIMEOUT = 7200000, want + <= 60000`. Passes after task 01's fix. +- No other runtime needed correction. +- `make -C apps/backend test` / `lint`: same result as task 01 (green for + touched packages; pre-existing unrelated failures confirmed identical to + merge-base). diff --git a/docs/specs/agents/README.md b/docs/specs/agents/README.md index 42785e8481..f2ec15d23e 100644 --- a/docs/specs/agents/README.md +++ b/docs/specs/agents/README.md @@ -47,6 +47,7 @@ surface shared by task and Office consumers. - [Granular Agent Permissions](requirements/granular-permissions.md) - [Hide Disabled Agent Profiles from Left Panel Navigation](requirements/hide-disabled-profiles-nav.md) - [Injected Skill Naming](requirements/injected-skill-naming.md) +- [Agent MCP Timeout Budgets](requirements/mcp-timeout-budgets.md) - [Mock-agent slow command duration syntax](requirements/mock-agent-slow-duration.md) - [Native Code Review](requirements/native-code-review.md) - [No Silent Model Fallback](requirements/no-silent-model-fallback.md) @@ -80,6 +81,7 @@ surface shared by task and Office consumers. - [Managed Agent Runtime Versions and Updates System Design Part 2](system-design/runtime-updates-02.md) - [Managed npm runtime recovery](system-design/managed-npm-runtime-recovery.md) - [MCP Bridge Reliability](system-design/mcp-bridge-reliability.md) +- [Agent MCP Timeout Budgets](system-design/mcp-timeout-budgets.md) - [MCP Protocol Compatibility](system-design/mcp-protocol-compatibility.md) - [Subagent context persistence System Design Part 1](system-design/subagent-context-persistence-01.md) - [Subagent context persistence System Design Part 2](system-design/subagent-context-persistence-02.md) diff --git a/docs/specs/agents/requirements/mcp-timeout-budgets.md b/docs/specs/agents/requirements/mcp-timeout-budgets.md new file mode 100644 index 0000000000..3378cd30b2 --- /dev/null +++ b/docs/specs/agents/requirements/mcp-timeout-budgets.md @@ -0,0 +1,80 @@ +--- +status: draft +system: agents +created: 2026-09-02 +owners: + - kandev +--- + +# Agent MCP Timeout Budgets Requirements + +## Overview + +Kandev supplies default environment values to every managed agent runtime. Two +of those values describe unrelated time budgets: how long the agent may spend +reaching a usable MCP session before it dispatches the first turn, and how long +a single MCP tool call may block. Kandev needs long blocking tool calls, because +`ask_user_question_kandev` and `step_complete_kandev` hold an MCP request open +until a user answers. Kandev does not need, and cannot tolerate, a startup +budget of the same size: an agent runtime that waits out its startup budget +before the first turn holds the session at generating for that whole budget +while producing no output. + +The agents system owns this contract because managed runtime environment +defaults are part of the agent-facing runtime contract, not of executor +placement or task orchestration. + +## Terminology + +- **Startup budget:** the maximum time an agent runtime may spend establishing + an MCP session before it dispatches the first prompt, including any wait on a + subscription stream that a correct MCP server holds open indefinitely. +- **Tool-call budget:** the maximum time a single MCP tool call may remain open + before the agent runtime abandons it. +- **Managed agent default:** an environment value Kandev supplies for an agent + runtime at the lowest precedence tier, which an executor profile or agent + profile value replaces. + +## Requirements + +### REQ-AGENTS-MCP-TIMEOUT-BUDGETS-001: Independent MCP startup and tool-call budgets + +**Intent:** Time an agent runtime spends waiting before its first turn must be +bounded independently of the budget Kandev's blocking MCP tools need. + +**User story:** As a Kandev user, I want an agent to start within seconds when I +have MCP servers configured, so that a startup wait cannot make a task appear +to run for hours without producing output. + +#### Acceptance criteria + +- **AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.1:** When Kandev supplies managed agent + defaults for an agent runtime, the startup budget and the tool-call budget + shall be expressed as two separate environment values, and no single value + shall define both budgets. +- **AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.2:** When Kandev supplies a managed agent + default startup budget, that value shall be at most 60000 milliseconds. +- **AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.3:** When Kandev supplies a managed agent + default tool-call budget, that value shall be at least 7200000 milliseconds, + so a blocking `ask_user_question_kandev` call can remain open for two hours. +- **AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.4:** When an agent runtime declares a + startup budget or a tool-call budget as a managed agent default, an executor + profile or agent profile value for the same key shall replace it without + producing an environment conflict. +- **AC-AGENTS-MCP-TIMEOUT-BUDGETS-001.5:** When an agent runtime declares any + MCP time budget as a managed agent default, an automated check shall assert + each declared value and its bound, so a later edit that recouples the two + budgets fails a test rather than only changing runtime behavior. + +## Out of scope + +- Detecting or recovering an agent that has already stalled. That behavior + belongs to [Agent Stall Recovery](agent-stall-recovery.md). +- Resolution and conflict rules between environment origins. Those belong to + [Executor-Profile Environment + Precedence](../../executors/requirements/executor-profile-env-precedence.md). +- The blocking semantics of individual Kandev MCP tools. Those belong to the + [task and workflow system](../../tasks/README.md). +- Correcting the agent CLI's own first-turn wait. That is an upstream client + defect, tracked as `anthropics/claude-code#91414`. Kandev bounds its cost; it + does not fix it. diff --git a/docs/specs/agents/system-design/mcp-timeout-budgets.md b/docs/specs/agents/system-design/mcp-timeout-budgets.md new file mode 100644 index 0000000000..d782afebff --- /dev/null +++ b/docs/specs/agents/system-design/mcp-timeout-budgets.md @@ -0,0 +1,127 @@ +--- +status: draft +system: agents +requirements: + - REQ-AGENTS-MCP-TIMEOUT-BUDGETS-001 +created: 2026-09-02 +owners: + - kandev +--- + +# Agent MCP Timeout Budgets System Design + +## Purpose and boundaries + +This design owns the managed agent default environment values that express MCP +time budgets for an agent runtime. It uses, but does not own, the environment +resolution and precedence contract in +[Executor-Profile Environment Precedence](../../executors/system-design/executor-profile-env-precedence-01.md), +and the blocking behavior of Kandev's own MCP handlers. + +## Requirement mapping + +| Requirement | Design section | +| --- | --- | +| `REQ-AGENTS-MCP-TIMEOUT-BUDGETS-001` | [Components and responsibilities](#components-and-responsibilities), [Data and contracts](#data-and-contracts), [Control flow](#control-flow), [Failure and recovery](#failure-and-recovery) | + +## Components and responsibilities + +- `apps/backend/internal/agent/agents/*.go`: each agent implementation returns a + `RuntimeConfig` whose `Env` map declares that runtime's managed agent + defaults. `ClaudeACP.Runtime` is the only implementation that declares MCP + time budgets. +- `apps/backend/internal/agent/runtime/lifecycle/environment_resolution.go`: + `appendAgentRuntimeDefaults` converts each `RuntimeConfig.Env` entry into a + `runtimeenv.Definition` at origin `managed agent defaults`, skipping any key a + higher-precedence definition already declared. +- `apps/backend/internal/agent/runtime/lifecycle/manager_startup.go`: the + non-strict assembly path applies the same `RuntimeConfig.Env` entries with + the same "existing key wins" rule. +- `apps/backend/internal/mcp/handlers/handlers.go`: holds a clarification MCP + request open while it waits for a user answer. It consumes the tool-call + budget; it does not set it. + +## Data and contracts + +The Claude Code CLI reads two independent environment values. + +| Key | Meaning in the CLI | Kandev managed default | +| --- | --- | --- | +| `MCP_TIMEOUT` | MCP connect deadline, and the deadline of the CLI's first-turn wait on the `subscriptions/listen` stream. CLI default is 30000 ms, clamped to at most 2147483647. | `30000` | +| `MCP_TOOL_TIMEOUT` | Per-call tool budget and the per-request fetch deadline floor, clamped to `[60000, 2147483647]`. | `7200000` | + +Verified against the shipped CLI (version 2.1.258): + +- `MCP_TIMEOUT` reader: `n && n > 0 ? Math.min(n, 2147483647) : 30000`. +- The connect deadline uses that reader directly. Under era negotiation the + connect that carries the subscription stream uses + `max(MCP_TIMEOUT - 5000, floor(MCP_TIMEOUT / 3))`, which is + `MCP_TIMEOUT - 5000` for any value at or above 7500 ms. +- The CLI opens `subscriptions/listen` and waits for that request to return + before it dispatches the first turn. `subscriptions/listen` is a + server-to-client notification channel: a specification-conformant server + holds it open and sends nothing. The CLI therefore waits out + `MCP_TIMEOUT - 5000`, cancels with `notifications/cancelled`, reopens a fresh + listen, and proceeds. The binary carries the matching telemetry and retry + path (`tengu_mcp_listen_reopen`, `subscriptions/listen re-open attempt`). +- The per-request fetch deadline is + `max(clamp(MCP_TOOL_TIMEOUT, 60000, 2147483647), MCP_TIMEOUT)`, so setting + `MCP_TOOL_TIMEOUT` to 7200000 preserves the two-hour request budget that + `MCP_TIMEOUT=7200000` previously produced as a side effect. + +The first-turn wait is an upstream client defect, filed as +`anthropics/claude-code#91414`. It is not a property of any MCP server. Kandev's +contract is to bound its cost, not to work around it. + +`30000` is deliberately explicit rather than omitted. It records Kandev's +intended bound at the declaration site, keeps AC-002 testable, and does not +depend on the CLI keeping 30000 as its own default. + +## Control flow + +1. `ClaudeACP.Runtime` returns `Env` containing both keys. +2. `appendAgentRuntimeDefaults` adds each key at origin + `managed agent defaults`, tier `TierAgentRuntimeDefault`, unless the key is + already defined by a higher-precedence origin. +3. `runtimeenv.Resolve` produces the launch environment. +4. The agent process starts. Its MCP connect deadline and its pre-prompt wait + are bounded by `MCP_TIMEOUT`. Its tool calls and MCP fetch deadline are + bounded by `MCP_TOOL_TIMEOUT`. + +## Failure and recovery + +With any MCP server configured, the CLI delays its first turn by +`MCP_TIMEOUT - 5000` ms. At the managed default of 30000 that is 25 seconds, +paid once per launch; at the previous value of 7200000 it was 1h 59m 55s. The +degraded outcome is a bounded delay before the first token, after which the +session proceeds normally with all tools available. A blocking Kandev MCP tool +call is unaffected because it is governed by `MCP_TOOL_TIMEOUT`. + +`MCP_TIMEOUT` remains the connect deadline, so it cannot be reduced to an +arbitrarily small value purely to shrink the first-turn wait without also +shortening the time a legitimately slow server has to connect. 30000 keeps the +CLI's own connect semantics unchanged. + +A user who needs a different bound sets `MCP_TIMEOUT` or `MCP_TOOL_TIMEOUT` on +an agent profile or executor profile. Both origins outrank +`managed agent defaults`, and `appendAgentRuntimeDefaults` skips a key that is +already defined, so the override applies without an environment conflict. + +## Persistence + +None. These values are computed per launch and are not stored. + +## Security + +Neither value is a secret. Both are already carried as plain literals in the +launch environment. + +## Observability + +`Manager.logEnvironmentOverrides` already emits `environment override applied` +with winning and losing origins when a profile replaces a managed agent +default, so an override of either budget is visible in backend logs. + +## Related decisions + +- [ADR-2026-09-02-separate-mcp-startup-and-tool-budgets](../../../decisions/2026-09-02-separate-mcp-startup-and-tool-budgets.md)