Repository navigation
fix(cli): stop typegen stdin feed hanging on an early-exit tool - #6948
Merged
Merged
Conversation
Under CPU contention, writing the tool's stdin could lose its scheduling race against the pipe's own EPIPE error, leaving the write stuck waiting on a drain event that a reader which already exited will never send. Race the stdin feed against the child's own exit, which is always observable, so the wait is bounded without depending on wall-clock timing.
Contributor
There was a problem hiding this comment.
🤖 AI Review
Both independent reviews were available. No production defect was confirmed. Claude’s minor concern about test scheduling remains uncertain; dependencies are not installed, so runtime verification was unavailable.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | apps/cli/src/commands/gen/types/types.typegen-host.ts:70 |
test-coverage |
claude | Racing stdin consumption against the fake spawner’s immediately successful exitCode may interrupt consumption before the existing stdin assertion’s expected document is recorded, making the test depend on fiber scheduling. |
Stats
Claude findings: 1 · Codex findings: 0 · Confirmed: 0 · Refuted: 0 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
The fake tool now exits only after it has read its stdin, like a real tool, so the stdin assertion no longer depends on fiber scheduling now that the feed races the exit. A new case has the tool exit while stdin stays blocked; it hangs without the exit race.
avallete
approved these changes
Oct 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
types.typegen-host.integration.test.ts› "survives a tool that exits before reading a large stdin" intermittently hit its 5s timeout when the machine was busy.The typegen host writes the request to the tool's stdin and waits for that write to finish. When the write hits backpressure, it waits for a
drainthat never comes once the reader has exited. The only way out is the pipe'serror(EPIPE) event. Under load, that event can be delivered before the listener for it is attached, so the feed hangs even though the tool's exit code, stdout and stderr have all resolved.The stdin feed is now raced against the child's exit, which is always observable once the tool is gone, so the wait no longer depends on winning that scheduling race. Under the same CPU load, the test failed 2 times in 50 before the change and 0 times in 50 after.
The stack's
CommandRunnerstdin path already races its feed against process exit, and it stays settled when several MB of stdin are fed to a psql that exits immediately.