Skip to content

fix(cli): stop typegen stdin feed hanging on an early-exit tool - #6948

Merged
jgoux merged 2 commits into
developfrom
fix/cli-typegen-host-stdin-flake
Oct 2, 2026
Merged

jgoux merged 2 commits into
developfrom
fix/cli-typegen-host-stdin-flake

Conversation

@jgoux

@jgoux jgoux commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

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 drain that never comes once the reader has exited. The only way out is the pipe's error (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 CommandRunner stdin 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.

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.
@jgoux
jgoux requested a review from a team as a code owner October 1, 2026 14:21
@jgoux jgoux self-assigned this Oct 1, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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.

Comment thread apps/cli/src/commands/gen/types/types.typegen-host.ts
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.

@Coly010 Coly010 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jgoux
jgoux added this pull request to the merge queue Oct 2, 2026
Merged via the queue into develop with commit be65dc0 Oct 2, 2026
47 checks passed
@jgoux
jgoux deleted the fix/cli-typegen-host-stdin-flake branch October 2, 2026 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants