fix(session): settle stdin closure after child exit

[flyingrobots]

When a Node child closes stdin during input shutdown without emitting finish, closeInput() currently remains pending after process exit. This also strands typed Git protocol shutdown and downstream session retirement. Handle terminal stdin or child-process close, preserve input errors and process exit validation, and remove temporary listeners on every terminal path. Prepare patch release 3.3.1 so git-warp can consume the repair.

Closes #18.

Change kind: bug fix and release metadata. Regression commit 21630bb records the first failing assertion; 93837f8 handles stdin close. The downstream real-Git probe exposed process completion before the stdin close notification; a sixth test was observed red against 93837f8, and 9b60038 repairs that ordering. b4cbd46 prepares 3.3.1. The Node spawn boundary is injectable for controlled stream-event schedules.

Validation: named regression failures rather than runner timeouts; 50 focused lifecycle and real Git protocol tests pass. Final local Docker matrix passes Node 237 tests, Bun 237 tests, and Deno 30 top-level tests / 272 steps. ESLint, touched-JavaScript formatting, and production dependency audit pass. The 3.3.1 npm tarball builds successfully. Commands, oracle, and evidence are in docs/evidence/session-input-close.md.

Hosted CI at b4cbd46 exposed two existing streaming tests that consumed stdout without awaiting process completion, leaving a process/timer/status waiter live in Deno. 590d4ee awaits gitStream.finished and checks code 0; the exact final-head lint and multi-runtime CI now pass. A separate overloaded local run was stopped after widespread watchdog failures; local verification then ran the same full suites serially with one Node worker. Test timeouts and sanitizer checks were preserved.

The runtime dependency audit is clean. The existing development-tool dependency tree still reports 12 audit findings; those dependencies are unchanged by this patch.

Downstream: a git-warp test advances the git-cas idle timer with a fake clock, uses real Git, and holds the writable final callback while allowing real EOF. On Plumbing 3.3.0, git-warp history closure stays pending after child exit; with this repair it completes. The full Node 22 integration lane passes 138 tests across 35 files with the candidate adapter, including all original occurrence assertions. Registry-backed adoption remains pending publication.

Related: git-stunts/git-warp#878. Its historical CI event ordering remains unknown; this is controlled evidence of the reproduced closure mechanism, with the original failure retained.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a5e36212-4192-492c-98b4-873f61d2ebbd

📥 Commits

Reviewing files that changed from the base of the PR and between 93837f8 and 590d4ee.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (6)

• CHANGELOG.md
• docs/evidence/session-input-close.md
• package.json
• src/infrastructure/adapters/node/NodeShellRunner.js
• test/NodeSessionLifecycle.test.js
• test/Streaming.test.js

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details

🔇 Additional comments (7)

CHANGELOG.md (1)

10-10: LGTM!

Also applies to: 15-16

docs/evidence/session-input-close.md (1)

46-49: LGTM!

Also applies to: 51-56, 72-72, 74-79, 84-84

package.json (1)

3-3: LGTM!

src/infrastructure/adapters/node/NodeShellRunner.js (2)

114-114: LGTM!

Also applies to: 133-135

---

164-164: 🎯 Functional Correctness

Do not bind ShellRunner.run.

ShellRunner.run does not access this. ShellRunnerFactory.createPorts() binds NodeShellRunner.run to its adapter before GitPlumbing invokes it.

test/NodeSessionLifecycle.test.js (1)

13-13: LGTM!

Also applies to: 19-19, 38-38, 50-50, 88-88, 93-109, 125-125

test/Streaming.test.js (1)

7-7: LGTM!

Also applies to: 22-23, 33-33, 44-45, 50-50

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes

  • Fixed Node command-session shutdowns that could remain pending when child input closes without a completion event.
  • Preserved reporting of input errors and process exit failures during shutdown.
  • Improved cleanup of input listeners after session closure.

• Tests

  • Added coverage for successful and failed input closure, child exit codes, and Git session shutdown behavior across supported runtimes.

• Documentation

  • Added technical evidence and validation details for the session input closure fix.

Walkthrough

The Node session adapter now settles closeInput() on stdin or child-process closure, preserves input errors, cleans up listeners, and supports injected process spawning. Lifecycle and streaming tests cover shutdown ordering, exit codes, and completion.

Changes

Node session closure

Layer / File(s)	Summary
Input closure implementation src/infrastructure/adapters/node/NodeShellRunner.js	NodeShellRunner accepts an injectable spawn function. closeInput() handles finish, close, child completion, input errors, listener cleanup, and synchronous stdin.end() failures.
Lifecycle regression coverage test/NodeSessionLifecycle.test.js, test/deno_entry.js	Controlled streams test close-only shutdown, successful flush, child completion ordering, input errors, listener cleanup, fast-import exit codes, and Deno registration.
Streaming completion validation test/Streaming.test.js	Streaming tests await gitStream.finished and assert exit code 0.
Fix evidence and release metadata CHANGELOG.md, docs/evidence/session-input-close.md, package.json	The changelog and evidence report describe the contract, reproduction, repair, validation, and release 3.3.1.

Priority: ➖ Normal — Impact reflects medium issue severity.

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 590d4

Node command sessions now complete input shutdown when stdin closes without a finish event, including child-first completion, while retaining input-error and process-exit failure behavior. The added lifecycle and streaming completion coverage leaves no concrete current-head merge risk.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: fixing stdin closure settlement after child-process exit.
Description check	✅ Passed	The description directly explains the session lifecycle bug, the repair, regression coverage, release update, and validation results.
Linked Issues check	✅ Passed	The changes address issue #18 by settling closure on stdin or child close, preserving errors and exit validation, cleaning up listeners, adding deterministic Node and typed-protocol tests, and reporti…
Out of Scope Changes check	✅ Passed	The release metadata, evidence documentation, streaming test completion checks, lifecycle tests, and Deno test entry update support the linked session-lifecycle fix and its validation. No unrelated co…
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (3 skipped: 3 …

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.}

_{Comment @coderabbitai help to get the list of available commands.}

[flyingrobots]

The final head 590d4ee includes the process-completion-first closure case found by the real-Git consumer probe and the missing subprocess completion waits exposed by Deno's sanitizer. Exact-head lint and full Node/Bun/Deno CI are green. The git-warp fake-clock regression is red on Plumbing 3.3.0 and green with the prepared 3.3.1 package; the full Node 22 integration lane passed 138 tests.

The prior review covered 93837f8. The included-review window has elapsed; please review the final changes for the required latest-push approval.

@coderabbitai review

[coderabbitai]

@flyingrobots I will review the latest changes at head 590d4ee.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.