Skip to content

fix(cli): lint sets process.exitCode instead of process.exit() (flush stdout) [P1]#1936

Merged
miguel-heygen merged 1 commit into
mainfrom
fix/lint-json-nonTTY-flush
Jul 7, 2026
Merged

fix(cli): lint sets process.exitCode instead of process.exit() (flush stdout) [P1]#1936
miguel-heygen merged 1 commit into
mainfrom
fix/lint-json-nonTTY-flush

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Root cause

hyperframes lint --json wrote the JSON payload with console.log() and then immediately called process.exit(). process.exit() terminates the process before Node flushes an asynchronously-buffered stdout — which is exactly what a non-TTY (piped) stdout is. So hyperframes lint --json | tee, > out.json, or any CI/agent capture silently lost the entire JSON payload on Windows (reported on 0.7.31, non-TTY).

The same console.log-then-process.exit pattern was on all four exit sites (both --json branches plus the human-readable and thrown-error paths), so any of them could truncate its output.

Fix

Set process.exitCode and return, letting run() unwind so Node drains stdout before exiting with the code. This is exactly the pattern the other commands (publish / transcribe / upgrade / play / present) already use — lint was the outlier still calling process.exit() right after writing output. No logic change; only the exit mechanism.

Test plan

New lint.test.ts drives the command's run() with mocked lintProject/resolveProject and a process.exit spy that throws if called. Four cases — --json with errors, --json clean, --json on a thrown error, and the human-readable errors path — each asserts process.exit is never called and the correct process.exitCode is set. These fail against the pre-fix code (the spy throws on the first process.exit).

  • bunx vitest run packages/cli/src/commands/lint.test.ts — 4/4 passing.
  • CLI tsc --noEmit clean; full bun run build clean; oxlint/oxfmt clean.

Context: this was the highest-confidence fix from a ~17.5h backlog of feedback that accumulated during a PostHog outage; several other precise reports in that batch (render viewport clipping to ~996px on macOS, webm-alpha producing opaque output on Windows, bundled SFX library shipping no mp3s, non-ASCII Windows username breaking ffmpeg paths) are logged for follow-up.

… stdout)

`hyperframes lint --json` wrote the JSON payload with console.log() and
then immediately called process.exit(). process.exit() terminates the
process before Node flushes an asynchronously-buffered stdout, which is
what a non-TTY (piped) stdout is — so `hyperframes lint --json | tee`,
`> out.json`, or any agent/CI capture silently lost the entire payload
on Windows (reported on 0.7.31 non-TTY). The same console.log-then-exit
pattern was on all four exit sites (both --json branches and the
human-readable + thrown-error paths), so any of them could truncate.

Fix: set process.exitCode and return, letting run() unwind so Node
drains stdout before exiting with the code. This is exactly the pattern
the other commands (publish/transcribe/upgrade/play/present) already
use; lint was the outlier still calling process.exit() after writing.

Test: new lint.test.ts drives the command's run() with mocked
lintProject/resolveProject and a process.exit spy that throws if
called. Covers the --json-with-errors, --json-clean, --json-thrown, and
human-readable paths — each asserts process.exit is never called and
the correct exitCode is set. Fails against the pre-fix code (the spy
throws on the first process.exit).
@miguel-heygen miguel-heygen changed the title fix(cli): lint sets process.exitCode instead of process.exit() (flush stdout) [P1] fix(cli): lint sets process.exitCode instead of process.exit() (flush stdout) Jul 7, 2026
@miguel-heygen miguel-heygen marked this pull request as ready for review July 7, 2026 19:04

@miga-heygen miga-heygen 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.

Review: LGTM ✅

Clean, focused fix for a real P1 data-loss bug. The process.exitCode + return pattern is idiomatic Node and matches how the other HyperFrames CLI commands already handle exits.

Verified

  • Root cause correct: process.exit() terminates before Node flushes asynchronously-buffered stdout on non-TTY (piped) output. Replacing with process.exitCode = N; return; lets the event loop drain naturally.
  • All four exit paths covered: --json with errors, --json clean, --json on thrown error, and human-readable with errors — all switched from process.exit() to process.exitCode.
  • No hang risk: The lint command doesn't open persistent handles (timers, connections), so the process exits naturally after the event loop drains.
  • Tests well-designed: The process.exit spy that throws if called is a smart regression guard — any reintroduction of process.exit() immediately fails the test.

Suggestions (out of scope, non-blocking)

  1. validate has the same issuevalidate.ts:615,619 uses the identical console.log(JSON.stringify(...)); process.exit(exitCode) pattern with --json. Worth a follow-up to apply the same fix there.

  2. resolveProject() also calls process.exit(1)project.ts:65 on InvalidProjectError. If lint is invoked with a bad directory and --json, stdout truncation can still happen before lint.run() starts. Separate concern.

  3. Test gap: Human-readable clean path (0 errors, 0 warnings) isn't tested. That path does a plain return (unchanged by this PR), so no regression risk — just noting for completeness.

Ship it.

— Miga

@miguel-heygen miguel-heygen changed the title [P1] fix(cli): lint sets process.exitCode instead of process.exit() (flush stdout) fix(cli): lint sets process.exitCode instead of process.exit() (flush stdout) [P1] Jul 7, 2026
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

@miga thanks for the review. On the out-of-scope note: you're right that validate --json has the same process.exit()-before-flush pattern — tracking it as a follow-up so this PR stays focused on lint.

@aszala-hg aszala-hg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified independently: all four exit sites in packages/cli/src/commands/lint.ts are converted to process.exitCode + return, with no process.exit() remaining anywhere in the file at ee5a90c, and no fall-through past any exit path. The new lint.test.ts genuinely pins the regression — a process.exit spy that throws if called, plus exitCode assertions across all four paths. No hang risk: lintProject is pure fs + linkedom parsing (no servers/watchers/timers) and withMeta is a synchronous cache read. All required checks green at head ee5a90c (the earlier "Tests on windows-latest" failure was an unrelated engine-package videoFrameExtractor afterAll rmSync flake — same workflow had passed at this exact SHA; re-run passed).

One line on Miga's out-of-scope note, independently confirmed: validate.ts still has the same console.log-then-process.exit() pattern (lines 572/576) and deserves the same treatment in a follow-up.

Approving per Abhay's go-ahead.

— Abhai

@miguel-heygen miguel-heygen merged commit 4834de3 into main Jul 7, 2026
89 of 91 checks passed
@miguel-heygen miguel-heygen deleted the fix/lint-json-nonTTY-flush branch July 7, 2026 21:10
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