Repository navigation
test: stub stdin ref/unref in cli premature-exit regression test - #2337
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title is concise, scoped, and accurately describes the stdin ref/unref stubs added to the CLI regression test. |
| Description check | |
The description covers the change, rationale, impact, validation results, and notes. It follows the required template and reports focused test results. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Risk Surface Disclosed | |
The diff changes only the fake stdin setup in src/entrypoints/cli.test.ts; it does not change CLI startup code. The test targets interactive startup, and the PR description identifies the change as … |
| No Hidden Policy Change | |
The PR changes only src/entrypoints/cli.test.ts. The diff adds process.stdin.ref and process.stdin.unref stubs in the existing child-process test setup. It does not change product behavior or pr… |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.
Full details: Risk Surface Disclosed
-
Explanation
The diff changes only the fake stdin setup in
src/entrypoints/cli.test.ts; it does not change CLI startup code. The test targets interactive startup, and the PR description identifies the change as test-only with no user-facing impact. The current review reports zero actionable findings, so no blocker is identified.
Full details: No Hidden Policy Change
-
Explanation
The PR changes only
src/entrypoints/cli.test.ts. The diff addsprocess.stdin.refandprocess.stdin.unrefstubs in the existing child-process test setup. It does not change product behavior or product, trust-model, routing-default, telemetry/network, or permission policy. No explicit maintainer alignment is needed.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
— jatmn's PR Review agent
I found no actionable code defects in the three-dot diff. Details below.
Merge readiness
- Draft PR: GitHub reports
mergeable_state: blockedbecause the pull request is still a draft. Mark it ready for review when you want merge-queue / required-review gating to apply beyond CI. - CI (head
04f9c1d908e4e87c467e5b272a2dd792450cf87a): All required PR checks on the head commit completed success (eightunit-testsshards,smoke-and-tests,typecheck,dedicated-tests,launcher-compatibilityfor Node 22 and 24.11.x,launcher-node-floor,pr-intent-scan, CodeQL family).webskipped (no web diff). Combined commit status success; CodeRabbit success. - Node 24 vs local verification: The regression describe block targets Node 24 / #1678, but CI
unit-testsruns Bun shards on Node 22 and the subprocess uses whatevernodeis onPATH(also 22 in that job).launcher-compatibilityexercises Node 24.11.x foropenclaude --versiononly—not this integration test. That matrix gap predates this PR; your Node 22 verification still matches what CI exercises forcli.test.ts. Not a merge blocker for this two-line harness fix. - Scope: Single file, test-only, no dependency on other open PRs.
🟢 Thanks for the contribution. I do not see any actionable issues from my review.
Summary
cli.test.ts) now stubsrefandunrefnext tosetRawMode.Appcallsstdin.ref()before attaching its input listener. Without the stub it throws, Ink unmounts the app and logs[Ink:uncaught] TypeError: stdin.ref is not a function, so the test passed against an unmounted UI instead of the interactive startup it is meant to cover.Impact
Testing
bun run buildok,bun run typecheckok,bun run checkok (0 failures in every shard).bun test src/entrypoints/cli.test.ts5/5 pass, 0 runs log the Ink error (on main: 20/20 pass, 20/20 log it).Notes
Summary by CodeRabbit