fix: 工具执行期间不再误判 idle 超时,长工具不被误杀

[lishuceo]

背景

线上反复出现 Query idle timeout after 600s with no activity (total elapsed: 701s),在跑批 / 长编译 / 克隆大仓等场景中断 agent。

根因

executor.ts 的滑动窗口 idle 计时器每收到一条 SDK 消息(或 canUseTool)就重置,本意是捕捉「模型/API 停止产出」的真·卡死。但单条工具调用执行期间不会有任何 SDK 消息到达:

• assistant 消息(含 tool_use)到达 → 重置计时器
• canUseTool 批准 → 重置计时器
• 工具执行(如最长可达 600s 的前台 Bash)…… 全程无消息
• idle 计时器(有工具活动时 2×300=600s)在工具仍在跑时触发 → abort

即:idle 计时器在「工具正常执行中」误判为「卡死」。

改动

利用已有的 pendingToolCalls Map(tool_use 入、tool_result 出)作为「工具是否在执行中」的信号:

• idle 计时器触发时,若仍有工具在执行 且 未超过工具硬顶(CLAUDE_TOOL_TIMEOUT,默认 900s,高于 SDK 单条 Bash 上限 600s),延展计时而非 abort;
• 超过硬顶仍未返回 → 判定工具挂死,以独立错误信息 abort(兜底等待 stdin 的命令等真·挂死);
• 模型空闲(无工具在执行)路径行为完全不变,仍按原 idle 窗口 abort。

判定逻辑抽成纯函数 shouldExtendIdleTimer,便于单测。

新增配置

• config.claude.toolTimeoutSeconds(env CLAUDE_TOOL_TIMEOUT,默认 900)
• ExecuteInput.toolTimeoutSeconds 覆盖项

测试

• shouldExtendIdleTimer 纯函数单测:in-flight / 空闲 / 超硬顶 / 并发工具
• executor 集成测试(假计时器):
  • 工具执行中跨越 idle 窗口 → 不被 abort(延展)
  • 超过工具硬顶 → abort,错误为 tool execution timeout
  • 无工具在执行 → 仍在 idle 窗口 abort,错误为 idle timeout(两类错误信息可区分)
• config 解析 CLAUDE_TOOL_TIMEOUT

npm run typecheck 通过;相关测试全绿。全量回归仅 memory/quality.test.ts 一条 embedding 检索质量断言失败,经 git stash 验证在改动前的干净树上同样失败,与本 PR 无关。

关于自重启

本改动落在服务源码,合并后部署目录需 git pull + npm install + build + 重启才生效。本 PR 不自动重启。

🤖 Generated with Claude Code

[github-actions]

Claude finished @lishuceo's task in 4m 17s —— View job

---

Code Review — idle timeout 误杀长工具修复

Verdict: ✅ Approved — clean, well-scoped fix with solid test coverage.

• Read CLAUDE.md & PR intent
• Reviewed real PR diff (gh pr diff 264 — the two-dot diff vs origin/main is polluted by feat: 拦截 /compact 与 /clear，让上下文压缩真正生效 #263's /compact having landed after this branch forked)
• Traced pendingToolCalls / toolWaitStartedAt lifecycle in executor.ts
• Checked interaction with hardTimeoutSeconds and canUseTool

---

Assessment

The root-cause analysis is correct: the sliding idle window only resets on SDK messages, and a single in-flight tool (foreground Bash up to 600s, big-repo clone, compile) emits nothing while running — so the idle timer fired mid-execution and aborted a healthy agent. Reusing the existing pendingToolCalls Map as the "tool in-flight" signal is the right, low-risk approach.

The implementation holds up under the cases I traced:

• Lifecycle correctness — toolWaitStartedAt is set on the null→non-empty transition (executor.ts:1129) and cleared when pendingToolCalls drains to 0 (executor.ts:1211). Since tool_results always precede the next assistant tool_use in the stream, the hard cap is measured per in-flight batch and resets cleanly between sequential tools — no false accumulation across a multi-tool turn.
• Hard timeout still wins — the independent hardTimer (executor.ts:677-686) aborts regardless of extension, so routing/short tasks with hardTimeoutSeconds are not kept alive by the new extension path. Error-message precedence (hard → toolStuck → idle) at executor.ts:1239 is consistent.
• Model-idle path unchanged — when inFlightToolCount === 0, shouldExtendIdleTimer returns false and the original idle abort fires with the original message. The regression test (aborts at the idle window when no tool is in-flight) locks this in, and the two error strings are distinguishable.
• Extraction to a pure function — shouldExtendIdleTimer is a good seam; the unit tests cover in-flight / idle / over-cap / concurrent.
• TDZ-safe — inFlightToolCount = () => pendingToolCalls.size is a closure over a later-declared const, but it's only invoked inside the async timer callback, never during the synchronous initial resetIdleTimer().

Minor, non-blocking observations (confidence < 75 — no action required)

1. Hard-cap overshoot ≤ ~1s — nextMs = Math.max(1000, Math.min(baseIdleTimeout(), toolTimeoutMs - toolElapsed)) (executor.ts:659). When the remaining time to the cap is under 1s, the floor of 1000ms makes the abort land up to ~1s past toolTimeoutMs. Harmless given the cap is 900s, and the comment already acknowledges it's approximate.
2. Leaked pending entry edge case — if a tool_use were ever recorded without a matching tool_result draining it (not something this PR introduces), toolWaitStartedAt would stay set and a genuinely-idle model would be classified as toolStuck, extending the abort window from idleTimeout×2 out to the 900s hard cap and mislabeling the error. The denial/normal paths all emit a matching tool_result, so this is theoretical — noting only for awareness.

Both are negligible; nothing here blocks merge. The fix matches CLAUDE.md conventions (ESM, pure-function-for-testability, feat+test pairing) and the test additions are appropriate.

Note: I could not execute npm run typecheck / vitest in this review environment (command execution is sandboxed/denied here). The verdict is based on static analysis; the PR description reports typecheck + targeted tests green, which is consistent with what I read.

[github-actions]

Claude finished @lishuceo's task in 2m 36s —— View job

---

Code Review — idle timeout 误杀长工具修复

Verdict: ✅ Approved — correct root-cause fix, well-scoped, properly tested. No new issues found on this synchronize.

• Read CLAUDE.md & PR intent
• Check/resolve previous inline review comments (none from claude[bot] on this PR)
• Review src/claude/executor.ts changes (idle extension logic)
• Review src/config.ts + .env.example (new config)
• Review test additions
• Post verdict

---

Assessment

The diagnosis holds: the sliding idle window only resets on SDK messages, and a single in-flight tool (foreground Bash up to 600s, big-repo clone, compile) emits nothing while running — so the idle timer fired mid-execution and killed a healthy agent. Reusing the existing pendingToolCalls Map as the in-flight signal is the right, minimal-surface fix.

Traced the lifecycle end-to-end and it's sound:

• In-flight cycle bookkeeping — toolWaitStartedAt is set on the null→non-empty transition (executor.ts:1156) and cleared when pendingToolCalls drains to 0 (executor.ts:1238). Because the model can't emit a new tool_use batch before the prior batch's tool_results return, pending always drains to 0 between turns, so the hard cap is measured per in-flight batch and resets cleanly — no cross-turn accumulation. Parallel tools in one batch correctly measure from the first tool's start.
• Extension vs. abort — shouldExtendIdleTimer returns true only while inFlightToolCount > 0 && elapsed < toolTimeoutMs; the timer re-arms with min(baseIdleTimeout(), toolTimeoutMs - elapsed) (executor.ts:686) so the 900s cap lands precisely. Past the cap → toolStuck = true and abort. Clean.
• Model-idle path unchanged — inFlightToolCount() === 0 ⇒ no extension, original idle abort fires with the original message. The two error strings (idle timeout vs tool execution timeout) are distinguishable, and the regression test locks both in.
• Hard timeout still wins — the independent hardTimer (executor.ts:704) aborts regardless of extension, and error-message precedence (hard → toolStuck → idle, executor.ts:1265) is consistent — short/routing tasks with hardTimeoutSeconds are not kept alive by the new path.
• TDZ-safe — inFlightToolCount = () => pendingToolCalls.size closes over a later-declared const but is only invoked inside the async timer callback, never during the synchronous initial resetIdleTimer().

Tests are appropriate: pure-function unit cases (in-flight / idle / over-cap / concurrent), fake-timer integration covering both the extend-then-cap-abort path and the model-idle abort path, plus the CLAUDE_TOOL_TIMEOUT config parse. Matches CLAUDE.md conventions (ESM, pure-function-for-testability, feat+test pairing).

Minor, non-blocking (no action required)

1. Leaked-pending edge case (confidence ~60) — if a tool_use were ever recorded without a draining tool_result, toolWaitStartedAt would stay set and a genuinely-idle model would be classified as toolStuck, stretching the abort window out to the 900s cap and mislabeling the error. Not introduced by this PR — every normal path (including canUseTool denials) emits a matching tool_result — so this is theoretical. Noting for awareness only.
2. Hard-cap overshoot ≤ ~1s (confidence ~55) — the Math.max(1000, …) floor at executor.ts:686 can push the final abort up to ~1s past toolTimeoutMs. Negligible against a 900s cap; the comment already calls it approximate.

Nothing here blocks merge.

Note: npm run typecheck / vitest could not be executed here (Bash is sandboxed/denied in this review environment, same as the prior run). Verdict is from static analysis; the PR reports typecheck + targeted tests green, consistent with what I read.