[nwparker]

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​214	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​212
Prod	6	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​122	$\color{#cf222e}{\Huge{\mathbf{−}}}$​21	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​101

The ticket's premise was wrong

STA-4663 says legacy daemons reject cancelCreateOrAttach as an unknown request type, so the client must version-gate it. That is not true, and the previous revision of this PR (a v35 gate) has been thrown away.

git show 181741d769d:src/main/daemon/types.ts | grep 'PROTOCOL_VERSION = '   # 11
git show 181741d769d:src/main/daemon/daemon-server.ts | grep -A2 "case 'cancelCreateOrAttach'"

cancelCreateOrAttach has been a handled case in daemon-server.ts since 181741d, when PROTOCOL_VERSION was 11. v11-v34 daemons accept it and answer ok: true; they never throw "Unknown request type". Gating the request at v35 would have been a functional downgrade: an aborted spawn against a v11-v34 daemon would stop being cancelled at all, letting the daemon finish its preflight and publish an orphan PTY. Legacy adapters have respawnFn = null, so nothing would clean that up.

The real defect (found while checking the premise)

client.ts wires the cancel RPC's failure path straight to a full connection teardown:

onCreateCancellationFailure: () => this.handleDisconnect(generation),
settleCreateCancellation: (sessionId, requestId) =>
  this.request('cancelCreateOrAttach', { sessionId, requestId }, NOTIFY_SETTLEMENT_TIMEOUT_MS)

In daemon-client-rpc-request.ts that callback ran on any rejection of the inner cancel request:

.catch(() => { if (!settled) opts.onCreateCancellationFailure() })

The inner cancel's own type is not createOrAttach, so its createSessionId is null and its timeout path takes rejectAndDrop. So a 5s cancel timeout - or an application-level ok: false reply - called handleDisconnect, which does pendingRequests.rejectAll('Connection lost') and destroys both sockets. One aborted spawn killed every sibling session on that daemon.

The blocked-event-loop case is exactly the unreachable-UNC-share scenario #14848 was written for, and it is also what a v1-v10 daemon produces (those versions really do throw "Unknown request type", and PREVIOUS_DAEMON_PROTOCOL_VERSIONS still lists them).

The fix

Escalation now requires proof that the wire is gone, carried by a dedicated error class rather than by "any rejection":

• DaemonConnectionLostError extends DaemonProtocolError (daemon-errors.ts) - thrown only where the transport itself failed: DaemonPendingRequests.rejectAll, DaemonClient.request's not-connected guard, and a control-socket write that throws. It extends DaemonProtocolError and keeps the exact same messages, so every existing instanceof check and isDaemonGoneError's message match are unaffected.
• daemon-client-rpc-request.ts - onCreateCancellationFailure fires only for DaemonConnectionLostError. Everything else (daemon error reply, cancel timeout, anything unrecognized) falls through to the existing bounded unmatchedCancelGraceMs wait and then rejectAndDrops only its own request. The default direction is deliberately fail-safe: an unclassified error leaves siblings alone.
• The control-socket write is now wrapped. Previously a synchronous throw escaped the promise executor and left the pending entry and its 30s timer in the map forever.

No version gate. A v1-v10 daemon's "Unknown request type" reply is now just an application-level cancel failure and is handled correctly by the same bounded fallback.

Follow-up: the wedged-daemon respawn signal, restored

Narrowing the escalation initially took a real recovery path with it, and readiness review caught it. Before this PR, a daemon wedged with its socket still open (blocked event loop) was recovered like this:

createOrAttach times out (30s) -> cancelCreateOrAttach times out (5s)
  -> onCreateCancellationFailure -> handleDisconnect -> rejectAll('Connection lost')
  -> the spawn rejects with 'Connection lost' -> isDaemonGoneError -> withDaemonRetry respawns

The teardown was collateral damage, but the 'Connection lost' message was the only respawn signal the client can produce while the socket is still open. writeNotifyWithSettlement only proves local drain, armDaemonSocketCloseHandlers only fires on a real close, and there is no client-side ping. Without it, every subsequent spawn times out at 30s with a message isDaemonGoneError does not match, forever, with zero retry.

So the signal is now carried without the teardown:

• DaemonRequestTimeoutError extends DaemonProtocolError (daemon-errors.ts) - our own deadline, as opposed to something the daemon answered. Same messages, so nothing that matches on message or instanceof DaemonProtocolError changes.
• wedgedDaemonError (daemon-client-rpc-request.ts) - when the request and the cancel sent to clean it up both hit that deadline, the request alone rejects with DAEMON_UNAVAILABLE_RECONNECT_MESSAGE (cause keeps the original timeout for diagnosis). isDaemonGoneError matches it, so withDaemonRetry respawns; doRespawn calls client.disconnect() itself, so the still-connected client is torn down there rather than out from under its siblings.
• That message was a bare literal in three places; isDaemonGoneError now imports the shared constant, so the producer and the matcher cannot drift.
• Aborts are excluded (client_disconnected is not a DaemonRequestTimeoutError): the caller asked to stop, not to retry. So is a cancel the daemon answered with an error - a v1-v10 "Unknown request type" arrives promptly and proves the daemon is alive.

Cost: recovery is now 30s + 5s + 5s instead of 30s + 5s, because the wedged case still goes through the bounded grace window on the chance a late response lands. Sibling sessions no longer pay anything at all.

daemon-protocol-version.ts is untouched.

Tests

daemon-client-rpc-request.test.ts

• cancel RPC times out -> no onCreateCancellationFailure, sibling pending request untouched, own request rejects after the bounded grace window
• daemon answers the cancel with Unknown request type: cancelCreateOrAttach -> same
• cancel rejects with DaemonConnectionLostError -> teardown still fires (renamed from "cannot be confirmed")
• control-socket write throws -> pending entry dropped, not leaked
• request times out and its cancel times out -> rejects with the wedged-daemon message, asserted against isDaemonGoneError itself, with onCreateCancellationFailure never called and the sibling request untouched

client.test.ts (end-to-end over a real unix socket)

• mock daemon replies ok: false to cancelCreateOrAttach -> client stays connected, no disconnected event, sibling listSessions not rejected
• a destroyed socket rejects in-flight requests as DaemonConnectionLostError, pinning the throw-site contract the discrimination depends on

Every new test was verified to fail with the source change reverted (measured on macOS 15 / darwin arm64).

Blast radius

DaemonPendingRequests.rejectAll and DaemonClient.request's not-connected guard now produce a DaemonProtocolError subclass. Messages (Connection lost, Not connected) are byte-identical, so isDaemonGoneError's equality checks and the daemon respawn path behave exactly as before. Nothing keys off error.name for these.

requestDaemonRpc's own timeout now produces a DaemonProtocolError subclass as well. Messages are byte-identical and nothing in the tree keys off error.name or the stringified error, so this is invisible to every existing consumer.

Behavior change: when a cancel fails non-transport-wise, the original createOrAttach now rejects after the 5s grace window instead of immediately. It still rejects with the same client_disconnected / timeout error, so callers are unchanged - it just no longer takes the connection down with it. The one exception is the both-timed-out case, which now rejects with Daemon temporarily unavailable; reconnect; that message reaches the user only on adapters with no respawnFn (remote/SSH), where the previous message was an equally non-actionable 30s timeout.

No wire change: no new request type, no new stream opcode, no change to what either side publishes. Behavior is identical on local, WSL, SSH and relay hosts, and on both git worktrees and folder workspaces.

Scoped out

• No REQUEST_SCOPED_SPAWN_CANCELLATION_DAEMON_PROTOCOL_VERSION, no protocol bump, no change to daemon-protocol-version.ts.
• The cancelAfterMs field added to the createOrAttach payload is still sent to every daemon version. It is an additive optional field that older daemons ignore, which is wire-compatible in both directions.
• Whether handleDisconnect is the right recovery for a genuinely undeliverable cancel is unchanged from fix(daemon): validate spawn cwd asynchronously so one dead share cannot freeze every terminal #14848; this PR only narrows when it is reached.
• No client-side liveness probe (a short ping before escalating). The both-timed-out condition already requires 35s of silence across two independent RPCs, so a probe would add a third timeout and a new wire call for no extra evidence.
• A daemon that answers some RPCs but wedges only on createOrAttach still has no detector. That is unchanged by this PR in either direction, and would need per-session health tracking rather than a request-local signal.

[coderabbitai]

📝 Walkthrough

Walkthrough

The daemon client adds distinct errors for connection loss and request timeouts. Pending requests use the connection-loss error when sockets disconnect or writes fail. Cancellation handling distinguishes transport failures from daemon refusals and timeouts, waits for late responses, and cleans up pending state. The daemon availability message is exported and reused by the PTY adapter. Tests cover sibling-request retention and transport failure cases.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	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 summarizes the main fix: failed spawn cancellation no longer tears down the shared daemon connection.
Description check	✅ Passed	The description clearly explains the defect, fix, compatibility impact, recovery behavior, testing, and scope, but it does not follow the repository template headings.

✨ Finishing Touches 💡 1

📝 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

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/main/daemon/daemon-errors.ts (1)

24-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Shorten transport-error comments.

These comments repeat control-flow details that the identifiers and nearby code already express. Keep one concise sentence for the non-obvious reason.

• src/main/daemon/daemon-errors.ts#L24-L30: Keep the transport-loss distinction only.
• src/main/daemon/daemon-client-pending-requests.ts#L47-L50: Keep the reason that rejectAll uses DaemonConnectionLostError.
• src/main/daemon/daemon-client-rpc-request.ts#L109-L114: Keep the reason that daemon cancellation errors do not disconnect siblings.

As per coding guidelines: “Comments must be concise, non-obvious, and brief—prefer one line.”

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e3b301b-e015-4dd9-9013-a7f700b0021f

📥 Commits

Reviewing files that changed from the base of the PR and between e0b3273184b8c5701c2e89fc29b2c0cac93656d7 and b2d1f99.

📒 Files selected for processing (7)

• src/main/daemon/client.test.ts
• src/main/daemon/client.ts
• src/main/daemon/daemon-client-pending-requests.ts
• src/main/daemon/daemon-client-rpc-request.test.ts
• src/main/daemon/daemon-client-rpc-request.ts
• src/main/daemon/daemon-errors.ts
• src/main/daemon/types.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 1 remains after this review.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Gate request-scoped cancellation by daemon protocol version.

cancelCreate invokes settleCreateCancellation for every createOrAttach request. The request payload also always adds cancelAfterMs. Protocol versions 33 and 34 therefore still receive unsupported cancellation behavior and wait for the grace period instead of rejecting locally.

• src/main/daemon/daemon-client-rpc-request.ts#L103-L119: Make settleCreateCancellation optional. Call it only when the client protocol version is at least 35. Gate cancelAfterMs with the same capability.
• src/main/daemon/client.test.ts#L413-L451: Configure protocol version 34. Assert that an aborted request sends no cancelCreateOrAttach RPC and rejects locally.

📍 Affects 2 files

• src/main/daemon/daemon-client-rpc-request.ts#L103-L119 (this comment)
• src/main/daemon/client.test.ts#L413-L451

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
file="src/main/daemon/daemon-client-rpc-request.ts"
wc -l "$file"
ast-grep outline "$file" --lang typescript
sed -n '1,230p' "$file"
printf '\n--- related symbols and call sites ---\n'
rg -n "rejectAndDrop|handleDisconnect|DaemonConnectionLostError|socket\.write|daemon-client-rpc-request" src test tests 2>/dev/null || true

Repository: stablyai/orca

Length of output: 17155

---

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- daemon client request path ---'
sed -n '1,360p' src/main/daemon/client.ts
printf '%s\n' '--- pending request implementation ---'
cat -n src/main/daemon/daemon-client-pending-requests.ts
printf '%s\n' '--- request tests ---'
cat -n src/main/daemon/daemon-client-rpc-request.test.ts
printf '%s\n' '--- client tests around write/disconnect ---'
sed -n '250,330p' src/main/daemon/client.test.ts
sed -n '430,490p' src/main/daemon/client.test.ts
printf '%s\n' '--- socket lifecycle handlers ---'
rg -n -C 8 "on\\(['\"](error|close|end)|handleDisconnect\\(|requestDaemonRpc\\(" src/main/daemon/client.ts src/main/daemon/*.ts

Repository: stablyai/orca

Length of output: 50369

---

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- all write-throw tests and related disconnect assertions ---'
rg -n -C 12 "write throws|write.*throw|onCreateCancellationFailure|rejectAll\\(|disconnected.*Called|isConnected\\(\\).*false" \
  src/main/daemon/*test.ts src/main/daemon/*.ts
printf '%s\n' '--- socket write contract references in repository ---'
rg -n -C 5 "write\\([^)]*\\).*callback|writeNotifyWithSettlement|onUndeliverable|ERR_STREAM_WRITE_AFTER_END|ERR_STREAM_DESTROYED" \
  src/main/daemon src/main/runtime
printf '%s\n' '--- focused implementation slices ---'
sed -n '20,75p' src/main/daemon/daemon-client-socket-connect.ts
sed -n '35,65p' src/main/daemon/daemon-client-pending-requests.ts
sed -n '205,235p' src/main/daemon/client.ts
sed -n '320,355p' src/main/daemon/client.ts
printf '%s\n' '--- focused write-failure test slice ---'
sed -n '215,250p' src/main/daemon/daemon-client-rpc-request.test.ts

Repository: stablyai/orca

Length of output: 50369

---

🏁 Script executed:

#!/bin/bash
set -eu
node - <<'JS'
const net = require('node:net')

async function probe(label, prepare) {
  const socket = new net.Socket()
  const events = []
  socket.on('error', (error) => events.push(`error:${error.code ?? error.message}`))
  socket.on('close', () => events.push('close'))
  prepare(socket)

  let result
  try {
    result = socket.write('rpc\n')
  } catch (error) {
    result = `throw:${error.code ?? error.message}`
  }
  await new Promise((resolve) => setImmediate(resolve))
  console.log(JSON.stringify({ label, result, events }))
  socket.destroy()
}

;(async () => {
  await probe('destroy-before-write', (socket) => socket.destroy())
  await probe('end-before-write', (socket) => socket.end())
})()
JS

Repository: stablyai/orca

Length of output: 316

---

Disconnect the client when an initial RPC write throws.

This catch rejects only the current request. Invoke the client’s disconnect handler so handleDisconnect rejects all sibling requests and clears the shared connection.

[coderabbitai]

🧹 Nitpick comments (1)

src/main/daemon/daemon-client-rpc-request.ts (1)

36-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reduce added explanatory comments.

Keep comments to one concise, non-obvious statement where possible.

• src/main/daemon/daemon-client-rpc-request.ts#L36-L43: Reduce the rationale to one short statement.
• src/main/daemon/daemon-errors.ts#L39-L43: Reduce the timeout rationale to one short statement.
• src/main/daemon/daemon-errors.ts#L51-L54: Reduce the reconnect-message rationale to one short statement.
• src/main/daemon/daemon-client-rpc-request.test.ts#L229-L232: Reduce the test rationale to one short statement.

As per coding guidelines: “Comments must be concise, non-obvious, and brief—prefer one line; do not explain obvious behavior or walk through code.”

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 54c84fad-801b-4093-87bc-bcc8a5abbad6

📥 Commits

Reviewing files that changed from the base of the PR and between b2d1f99 and e0e16b1.

📒 Files selected for processing (5)

• src/main/daemon/daemon-client-rpc-request.test.ts
• src/main/daemon/daemon-client-rpc-request.ts
• src/main/daemon/daemon-errors.ts
• src/main/daemon/daemon-pty-adapter.ts
• src/main/daemon/types.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 0 remain after this review.