fix(cli): flush stream-json output - #1919
Conversation
🦋 Changeset detectedLatest commit: 092fac8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
5d32ece to
35beeb1
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35beeb15e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const cleanupPromptRun = async (): Promise<void> => { | ||
| const pending = (cleanupPromise ??= (async () => { | ||
| removeTerminationCleanup?.(); | ||
| await drainPromptOutput(stdout); |
There was a problem hiding this comment.
When a SIGINT/SIGTERM arrives while --output-format stream-json stdout is backpressured and the pipe does not drain within PROMPT_CLEANUP_TIMEOUT_MS, cleanup times out at this first await and restorePromptSessionPermission()/harness.close() never run before installPromptTerminationCleanup() calls process.exit(). For resumed sessions that were forced to auto, that can leave the stored permission mode changed after an interrupted print run; wait for stdout with its own bounded best effort or move it after the required state cleanup. The v2 cleanup has the same ordering.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
| writeResumeHint(session.id, outputFormat, stdout, stderr); | ||
| await drainPromptOutput(stdout); |
There was a problem hiding this comment.
When --output-format stream-json writes enough data to make stdout.write() return false and the downstream pipe stays open but stops reading, this await never resolves. Because main.ts only calls finalizeHeadlessRun() after runPrompt() resolves, the existing bounded stdio drain/force-exit backstop is never reached, so a completed headless command can hang indefinitely instead of exiting after the configured stdio timeout. The v2 runner has the same unbounded await at run-v2-print.ts.
Useful? React with 👍 / 👎.
35beeb1 to
092fac8
Compare
|
Closing this to avoid keeping two implementations in review now that #2170 covers the same buffered-output shutdown path. |
1 participant
Related Issue
Resolves #1897
The stream-JSON writers in headless mode ignored the return value of
stdout.write(). With a slow or piped consumer, the final assistant message or resume hint could still be buffered when signal cleanup calledprocess.exit().Both headless runners now track backpressured writes and wait for
drainbefore ordinary completion. That wait uses the existing bounded stdio timeout, so a consumer that stops reading cannot hang a completed command indefinitely. Signal cleanup restores session permissions and closes runtime resources before giving pending output its bounded flush window.The regression tests cover delayed and permanently stalled drains, ordinary writes, resume hints, signal shutdown, permission restoration, and both headless engines. On current main I ran the full CLI suite (2,384 passing, 5 skipped), package typecheck, type-aware lint, changeset validation, and
git diff --check.This includes a patch changeset for
@moonshot-ai/kimi-code. The stream-JSON interface itself has not changed, so no documentation update is needed.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.