[OrcaWin]

	Files	Added	Deleted	Net
Test	5	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​458	0	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​458
Prod	4	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​332	0	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​332

ELI5

This is the first slice of #16741's T2 track (SSH core). It adds a per-connection work ledger that tracks in-flight operations and channels, so a later reset can drain cleanly, plus a transport close ledger. Nothing uses them yet, so behavior doesn't change.

What Changed

• ssh-connection-work-ledger.ts: tracks operations and channel lifetimes, and blocks new work once a reset starts.
  • Work nested under an operation it already admitted, and one exactly identified control channel, can still run.
  • A failure or unverifiable opening after that point fails the drain, and the drain stays failed.
• Transport close ledger and close observation.
• Adapted from feat(orcad): consolidate Bun runtime and SSH/Remote migration #16741:
  • Dropped assertProfileLifetimeAdmission, because it depends on the native addon dropped in T0 and overlaps the SQLite authority (Persist profile state in SQLite with a durable background writer #22612).
  • Replaced the broad object resource type with a named type, and the cast with a type guard.
• Porting notes are in each commit.

Why

Design D9: these are the T2 foundations that SshConnection wiring and T3 reset build on.

Linked Issue

Part of the Node runtime plan (Phase 3). Stacked on #24185, with #24186 merged in.

Visual Proof

N/A. Nothing uses this code yet.

Testing

• The ported ledger, channel-lifetime and forward-socket-admission tests pass.

• Not wired yet:

  • the relay grace-lifecycle drain, which needs host-side handling of a closed admission;
  • SshConnection integration;
  • profile-lifetime admission, which needs a decision on the native addon versus SQLite authority.

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Adds SSH connection and transport lifecycle tracking.

The PR appears safe to merge as an unwired foundation for later SSH reset integration.

Summary

The PR adds per-connection SSH work and channel-lifetime tracking, plus transport-close tracking and observation. The new ledgers are not yet wired into SshConnection; this change adds their implementations and tests without changing the active connection flow.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Admit SSH work] --> B[Track operation or channel]
  B --> C[Fence new work for reset]
  C --> D[Drain admitted work]
  D --> E[Observe transport close]

Loading

_{Reviews (3) · Last reviewed commit: "feat(ssh): add SSH transport close ledge..."}

[greptile-apps]

Comments Outside Diff

These findings could not be posted inline.

•  Required listener can be missing src/main/runtime/runtime-rpc/runtime-rpc-lifecycle.ts:78 ▶

  When requirePinnedWsPort is true, a device-registry or encryption-key initialization failure takes this branch and startup still succeeds with only the Unix socket. The managed SSH tunnel then has no WebSocket listener at its required port. Required-port startup should fail when pairing identity initialization prevents that listener from starting.

[pullfrog]

ℹ️ No critical issues — one API-consistency note inline, a couple of defer-able observations below.

Reviewed changes

• Per-connection work ledger (src/main/ssh/ssh-connection-work-ledger.ts): tracks operations and channels under an AsyncLocalStorage scope, admits nested work only under an already-admitted non-exempt parent after fenceForReset(), and drains cancellably while retaining the first failure.
• Channel/socket lifetime tracking (src/main/ssh/ssh-connection-channel-lifetime.ts): binds each open→close to a ledger entry; an untrackable open result is marked unverifiable rather than settled.
• Transport close ledger and observation (src/main/ssh/ssh-transport-close-ledger.ts, ssh-transport-close-observation.ts): wait for every allocated client's real close (failed-auth included) and refuse allocations added after the drain snapshot.
• WebSocket lifecycle extraction and strict bind (src/main/runtime/rpc/node-websocket-lifecycle.ts, websocket-transport-limits.ts, ws-transport.ts; from #24186, merged in): handlers and stop() extracted order-equivalently, stop() now also calls closeAllConnections(), and strictPort / requirePinnedWsPort fails closed instead of falling back when a managed tunnel needs one port.
• Tests for all of the above (33 cases), all passing locally.

ℹ️ Nitpicks

• src/main/ssh/ssh-connection-work-ledger.ts:44-48 — markTransportUnverifiable() is a silent no-op before fenceForReset(), unlike beginChannelOpen().markUnverifiable(), which persists its error on the entry even pre-fence. Fine if the reset flow always fences first; otherwise a connection-level failure can vanish before the fence.
• src/main/ssh/ssh-connection-work-ledger.ts:90-132 — beginSession() is the only public method in the port with no test coverage, and it owns non-trivial closed / closeFailed / active state. Close-before-drain, close-with-error mid-flight, and run-after-close would pin it.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

isDrained() counts the exempt control entry as pending, while drain() and assertDrained() deliberately exclude it. After a legitimate drain with a live control channel (ssh-connection-work-ledger.test.ts:216-233), fence.assertDrained() passes but isDrained() still reports false — anything that gates reset completion on isDrained() would never see it finish. Consider excluding this.exempt (or documenting isDrained() as the pre-fence variant) and adding a matching test.

Technical details

# isDrained() disagrees with the fence's drained definition

## Affected sites
- `src/main/ssh/ssh-connection-work-ledger.ts:35-37` — `isDrained()` returns `pending.size === 0 && !failure`
- `src/main/ssh/ssh-connection-work-ledger.ts:169-175` — `assertDrained()` filters `entry !== this.exempt`
- `src/main/ssh/ssh-connection-work-ledger.ts:184` — `drain()` filters the exempt entry too

## Required outcome
After a legitimate drain that leaves the exempt control channel open, `isDrained()` and the fence's `assertDrained()` return the same verdict.

## Suggested approach (optional)
Filter `this.exempt` out of the `isDrained()` count, or rename/document it as the pre-fence variant. Add a test mirroring the exempt-control case at `ssh-connection-work-ledger.test.ts:216-233`.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: aac39639-4b9d-4f26-820e-2d65971d8af7

📥 Commits

Reviewing files that changed from the base of the PR and between dc08ffe and a5540c4.

📒 Files selected for processing (9)

• src/main/ssh/ssh-connection-channel-lifetime.test.ts
• src/main/ssh/ssh-connection-channel-lifetime.ts
• src/main/ssh/ssh-connection-work-ledger.test.ts
• src/main/ssh/ssh-connection-work-ledger.ts
• src/main/ssh/ssh-forward-socket-admission.test.ts
• src/main/ssh/ssh-transport-close-ledger.test.ts
• src/main/ssh/ssh-transport-close-ledger.ts
• src/main/ssh/ssh-transport-close-observation.test.ts
• src/main/ssh/ssh-transport-close-observation.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The changes add a work ledger for SSH connection operations, sessions, and channel openings. Channel tracking links opened sockets to ledger entries and settles them on closure. The changes also add a ledger for tracking transport client closure and an abortable observer for transport resource close events. Tests cover admission, reset draining, channel errors, close events, aborts, and retries.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to a5540

This change adds tracking modules that nothing uses yet, so SSH behavior is unchanged. No concrete merge-blocking problem was found.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a5540

Current exposure appears limited because the new lifecycle code is not connected to production SSH operations. No introduced attack path was established, but reset and recovery guarantees cannot yet be assessed end to end.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The examined accounting state is instance-local: each work ledger owns its pending entries and asynchronous admission scope, and each transport ledger owns its tracked client map. These primitives do not themselves establish tenant-wide or persistent reset authority.

Trust Boundaries and Controls

• observed — Post-fence admission is restricted to an active pending, non-exempt parent. A control-channel exemption requires exactly one pending entry with the same resource identity; stale or ambiguous identities cannot establish that exemption. The injected admission assertion remains a caller-supplied control.

Resilience and Maintainability Implications

• observed — Once fenced, failed or unverifiable non-exempt work latches a ledger failure that prevents successful drain. Cancelling a wait does not reopen admission or prove resource closure; ownership tracking remains separate from the waiting caller's cancellation.

Hardening Proposals

• proposed — Future production integration should make allocation fencing and admission authority explicit, register closure tracking before resources can close, and preserve ownership tracking through cancellation and recovery. These are integration obligations, not observed vulnerabilities in this PR.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: porting the SSH connection work ledger and transport close ledger.
Description check	✅ Passed	The description explains the purpose, scope, implementation, testing, and lack of user-visible behavior changes. It includes the required core sections, but it omits the Review, Notes, and Checklist s…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

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