fix(cli): prevent stalled agent streams - #12249
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Reviewed the incremental changes since the last automated review (commit It adds an explicit No production code changed in this increment, and the change is well-scoped to CI reliability on Windows. No new correctness, security, or fork-hygiene issues found in the changed lines. The previously flagged items (global 60s watchdog default discussion in Files Reviewed (1 file changed since last review)
Previous Review Summaries (6 snapshots, latest commit c42e3c5)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit c42e3c5)Status: No Issues Found | Recommendation: Merge Reviewed the incremental changes since the last automated review (commit No new correctness, security, or fork-hygiene issues found in the changed lines. The previously flagged items (global 60s watchdog default discussion, swallowed Files Reviewed (1 file changed since last review)
Previous review (commit 50a311c)Status: No Issues Found | Recommendation: Merge Reviewed the incremental changes since the last automated review (commit No new correctness, security, or fork-hygiene issues found in the changed lines. The previously flagged items (global 60s watchdog default discussion, swallowed Files Reviewed (1 file changed since last review)
Previous review (commit 7e7a3d5)Status: No Issues Found | Recommendation: Merge Reviewed the incremental changes since the last automated review (commit No new correctness, security, or fork-hygiene issues found in the changed lines. The previously flagged items (global 60s watchdog default discussion, swallowed Files Reviewed (1 file changed since last review)
Previous review (commit ca8950a)Status: No Issues Found | Recommendation: Merge Reviewed the incremental changes since the last automated review (commit The new logic is covered by extensive unit tests ( The previous suggestion about the swallowed Files Reviewed (9 files changed since last review)
Previous review (commit 4f89293)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file changed since last review)
Fix these issues in Kilo Cloud Previous review (commit 6d48404)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Reviewed by claude-sonnet-5 · Input: 34 · Output: 8.6K · Cached: 804.2K Review guidance: REVIEW.md from base branch |
|
Goat. I was getting ready to tackle exactly this problem because I've been seeing the issue more frequently with GPT 5.6, but luckily Kilo found this PR. |
|
This change appears to be having a very unwanted side effect. Running a subagent causes the session to be killed after 60 seconds. This is a huge breaking bug and will need to be fixed before this is merged. Here is the information that my agent was able to determine which will hopefully help guide a fix: DiagnosisHigh confidence: commit It changed the default AI SDK stream configuration from no chunk timeout to: timeout: { chunkMs: 60_000 }Failure chain
Relevant paths:
Timing evidenceThe apparent This includes Kilo/Anthropic and OpenAI models. The parallel subagents launched during this investigation were both created at Why
|
| Candidate | Assessment |
|---|---|
| Network/provider failure | Ruled out by exact 60-second lifetime across providers and machines |
| A configured 15-second provider timeout | Ruled out; no matching loaded config and actual lifetime is 60 seconds |
| Manual/session abort | No initiating parent cancellation event; timing repeats exactly |
| Background-job timeout | Foreground background.wait() has no timeout |
| OS process/resource killing | Logs show application-level AbortSignal, not a process signal/OOM |
| Orphan cleanup logic | Downstream consequence only |
| New default chunk timeout | Confirmed by commit and timing evidence |
The existing timeout unit test passes (7 pass, 0 fail) but only verifies configuration output. It does not cover a tool running longer than chunkMs.
marius-kilocode
left a comment
marius-kilocode
left a comment
There was a problem hiding this comment.
@iscekic I think @shssoichiro might be right here.
Blocking: the reviewer is correct. The new default at packages/opencode/src/kilocode/session/llm.ts:10,31 is passed to AI SDK streamText at packages/opencode/src/session/llm.ts:392. In AI SDK 6.0.168, chunkMs:
Starts/reset on normalized stream chunks.
Is merged into the abort signal supplied to local tools.
Remains active while the tool is executing.
Is only cleared after the tool result arrives.
A parent receives a task tool-call chunk and then waits silently while the foreground subagent runs. After exactly 60 seconds, the timer aborts the tool signal. Kilo maps that signal directly to Tool.Context.abort at packages/opencode/src/session/tools.ts:61-64, and TaskTool cancels the child at packages/opencode/src/tool/task.ts:405-438. Cleanup then marks the task interrupted at packages/opencode/src/session/processor.ts:1069-1100, producing the reported orphaned-tool exit.
This affects any local tool taking more than 60 seconds without preliminary output, not only subagents.
High: the default cannot reliably be disabled through documented provider configuration. chunkTimeout only accepts positive integers at packages/core/src/v1/config/provider.ts:118-121. Although KiloLLM.timeout() treats 0 as disabled, provider..options.chunkTimeout: 0 is rejected by the public schema. Users can only raise the timeout, not disable the new default through the documented provider setting.
High: the test only verifies object construction. packages/opencode/test/kilocode/session/llm.test.ts:34 checks that { chunkMs: 60_000 } is returned. It does not exercise a tool or subagent running longer than chunkMs, which is exactly where the regression occurs.
|
Maybe we need a bit more complexity to solve this. Something that observes the idle state? |
|
Hey, this was my overnight bot submitting the PR. I think both of you are right, I'll play with this a bit more today and push an update. |
…hang # Conflicts: # packages/opencode/test/kilocode/session-prompt-permission-refresh.test.ts
|
(bot) Addressed the blocking regression on head |
An async generator's return() cannot preempt an in-flight internal await; when suspended mid-await it only applies once that await settles on its own. For a genuinely stalled stream that await never settles, so interrupting a session mid-stream (e.g. aborting while a local tool call is pending) hung instead of cancelling. Replace the generator with a hand-rolled AsyncIterator whose return() runs immediately and forwards to the source's return() without waiting on any outstanding pull, matching how interruption already behaves for the unwrapped upstream iterator. Fixes CI failures in test/session/processor-effect.test.ts and test/session/prompt.test.ts that hung/timed out on this branch.
|
(bot) Pushed Also replied in the open discussion thread on @marius-kilocode could you take another look and re-review when you have a chance? |
git-bash on Windows CI runners spawns and writes the readiness marker file noticeably slower than the Unix shells this suite otherwise runs under, so tests A and C's 5s file-poll and 30s scenario timeout were too tight there and failed with 'readiness marker never appeared' even though the tool was already running. Double both on win32, matching the existing platform-aware timeout doubling in test/kilocode/background-process.test.ts.
path.join() yields backslash-separated paths on Windows. Embedded inside a double-quoted git-bash string, a literal backslash is an escape character, so the ready/release marker paths could resolve to the wrong file (or nothing) instead of erroring, making 'touch' and the '[ -f ... ]' poll silently miss each other. Normalize to forward slashes before interpolating into the script; git-bash/MSYS accept them natively on every platform this suite runs on. This is the actual root cause of the 'readiness marker never appeared' failures on Windows shards; the previous commit's timeout doubling was only masking symptoms.
The production bash tool runs every command through a login shell (bash -l -c ..., src/shell/shell.ts) so ~/.bashrc/aliases behave like an interactive terminal. Git for Windows' login-shell startup rescans the full Windows PATH and is known to take several seconds on CI hardware, well past the previous 15s/60s Windows margins, before the script's own touch ever runs. Extend waitForFile to 30s and the two affected scenario timeouts to 90s on win32.
…n Windows Root cause, finally isolated: without a config-level shell field, the bash tool defaultShell() falls back to cmd.exe on Windows (see packages/core/src/tool/bash.ts). cmd.exe cannot run bashGate POSIX syntax (touch, test -f, while/done), so touch failed instantly and silently and the readiness marker never appeared - no timeout was ever going to fix that, which is why the previous two commits margin increases did not help. Set shell to bash in tests A and C config so the bash tool resolves real git-bash via src/shell/shell.ts on Windows, and drop the speculative timeout inflation back to the original values plus a small, now-accurate margin for git-bash slower login-shell startup.
|
(bot) Final status on head CI: all checks green, including every previously failing job ( Root causes fixed:
Marius's concern: replied in the discussion thread on Local E2E: verified directly against the PR-branch CLI build (dev-local run against this worktree's backend): a subagent-spawning session and a plain session both completed and streamed to completion without hanging — the exact failure mode this PR fixes — including one run where the CLI paused on a real permission prompt mid-turn and resumed/completed normally afterward. Mobile mirroring of this specific local worktree's CLI sessions showed "Session terminated" for reasons unrelated to this PR (session-ingest's Durable Object reported Outstanding: |
…#12438) PR #12249 was reverted (open PR #12435) because its test imported three modules that v1.17.4 compat (2855ebb) removed/moved: - Reference/RepositoryCache from ../../src/reference/* (now packages/core) - Ripgrep from @opencode-ai/core/filesystem/ripgrep Re-land the production fix by pointing RepositoryCache and Ripgrep at @opencode-ai/core/{repository-cache,ripgrep} and dropping the now-gone Reference.defaultLayer from the layer stack, matching the current full-stack sibling (session-prompt-compaction-safety.test.ts). Typecheck passes; all 3 watchdog tests pass.
This reverts commit cd205d8.
* fix(cli): prevent stalled agent streams * docs(cli): clarify stream timeout scope * test(cli): stabilize global skill permission timing * fix(cli): make stream timeout tool-aware * fix(cli): let the idle watchdog cancel a stalled pull immediately An async generator's return() cannot preempt an in-flight internal await; when suspended mid-await it only applies once that await settles on its own. For a genuinely stalled stream that await never settles, so interrupting a session mid-stream (e.g. aborting while a local tool call is pending) hung instead of cancelling. Replace the generator with a hand-rolled AsyncIterator whose return() runs immediately and forwards to the source's return() without waiting on any outstanding pull, matching how interruption already behaves for the unwrapped upstream iterator. Fixes CI failures in test/session/processor-effect.test.ts and test/session/prompt.test.ts that hung/timed out on this branch. * test(cli): give Windows more time for the watchdog integration bash gate git-bash on Windows CI runners spawns and writes the readiness marker file noticeably slower than the Unix shells this suite otherwise runs under, so tests A and C's 5s file-poll and 30s scenario timeout were too tight there and failed with 'readiness marker never appeared' even though the tool was already running. Double both on win32, matching the existing platform-aware timeout doubling in test/kilocode/background-process.test.ts. * test(cli): use POSIX-style paths in the watchdog bash gate script path.join() yields backslash-separated paths on Windows. Embedded inside a double-quoted git-bash string, a literal backslash is an escape character, so the ready/release marker paths could resolve to the wrong file (or nothing) instead of erroring, making 'touch' and the '[ -f ... ]' poll silently miss each other. Normalize to forward slashes before interpolating into the script; git-bash/MSYS accept them natively on every platform this suite runs on. This is the actual root cause of the 'readiness marker never appeared' failures on Windows shards; the previous commit's timeout doubling was only masking symptoms. * test(cli): extend Windows margins further for the watchdog bash gate The production bash tool runs every command through a login shell (bash -l -c ..., src/shell/shell.ts) so ~/.bashrc/aliases behave like an interactive terminal. Git for Windows' login-shell startup rescans the full Windows PATH and is known to take several seconds on CI hardware, well past the previous 15s/60s Windows margins, before the script's own touch ever runs. Extend waitForFile to 30s and the two affected scenario timeouts to 90s on win32. * test(cli): give the watchdog bash gate an explicit shell so it runs on Windows Root cause, finally isolated: without a config-level shell field, the bash tool defaultShell() falls back to cmd.exe on Windows (see packages/core/src/tool/bash.ts). cmd.exe cannot run bashGate POSIX syntax (touch, test -f, while/done), so touch failed instantly and silently and the readiness marker never appeared - no timeout was ever going to fix that, which is why the previous two commits margin increases did not help. Set shell to bash in tests A and C config so the bash tool resolves real git-bash via src/shell/shell.ts on Windows, and drop the speculative timeout inflation back to the original values plus a small, now-accurate margin for git-bash slower login-shell startup.
…Kilo-Org#12438) PR Kilo-Org#12249 was reverted (open PR Kilo-Org#12435) because its test imported three modules that v1.17.4 compat (7a1394e) removed/moved: - Reference/RepositoryCache from ../../src/reference/* (now packages/core) - Ripgrep from @opencode-ai/core/filesystem/ripgrep Re-land the production fix by pointing RepositoryCache and Ripgrep at @opencode-ai/core/{repository-cache,ripgrep} and dropping the now-gone Reference.defaultLayer from the layer stack, matching the current full-stack sibling (session-prompt-compaction-safety.test.ts). Typecheck passes; all 3 watchdog tests pass.
This reverts commit 182e775.
4 participants
Summary
Replace the default AI SDK chunk timeout with a Kilo-owned, tool-aware model-stream idle watchdog. Stalled root and child streams still return control after 60 seconds, while local commands and foreground subagents may run longer without being aborted. Explicit request, model, agent, and provider overrides continue to win, and
chunkTimeout: falsedisables the watchdog.Context
AI SDK merges
chunkMsinto the abort signal supplied to local tools, so the previous implementation canceled a parent and its foreground child after exactly 60 seconds. The watchdog now observes raw stream events, pauses while non-provider-executed tools are active, resumes after their results, and aborts the underlying provider stream only on genuine inactivity.