Severity: Low · Verify confidence: medium
File: apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts:746
What
Commit 455f69b added isClosed()-based reconnect and idempotent-read retry to recover from dropped daemon connections, but the guard only wraps the action call, not ensureProject() which runs first on the same connection.
In callActionForRootUncoalesced the order per attempt is: (1) const project = await this.ensureProject(rootPath) (line 746); (2) entry = await this.connect(); (3) if (entry.client.isClosed()) reconnect (line 752). ensureProject() (line 651-665) calls this.connect() and then entry.client.call("projects.add", ...) with NO isClosed() pre-check and NO drop-retry. connect() returns the cached this.connection if non-null (line 957), and this.connection is only cleared asynchronously by the client onDisconnect callback (line 1141) and the child exit handler (line 1182).
Failure mode: a daemon restart/build-hash recycle closes the socket; before the close event is processed on the event loop, the next IPC action calls ensureProject() -> connect() returns the stale closed client -> projects.add throws "Remote ADE service connection closed." This propagates out before the loop's try/catch (which starts at line 764), so there is no reset, no retry, and the renderer sees the exact raw error the commit set out to eliminate (e.g. "Lane was deleted, but refresh failed: ... connection closed."). The isClosed() guard at line 752 is effectively dead for the first call because ensureProject already used (and threw on) the same stale connection. Tests (localRuntimeConnectionPool.test.ts:1474-1502) only cover the classifier, not the ensureProject drop path.
Trigger
Rebuild desktop so the expected build hash changes (or restart/crash the daemon) while the renderer issues a read action; the first ensureProject->projects.add hits the stale closed connection and surfaces a raw 'Remote ADE service connection closed.' with no retry.
Verification (adversarial)
The structural claim is fully confirmed by reading the code. In callActionForRootUncoalesced (apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts:734), each attempt runs const project = await this.ensureProject(rootPath) at line 746 — OUTSIDE the try block, which only starts at line 764. ensureProject (lines 651-665) calls await this.connect() then entry.client.call("projects.add", ...) at line 656 with NO isClosed() pre-check (the guard exists only at line 752 for the action path) and NO drop-retry. The action call at line 765 IS protected: its "Remote ADE service connection closed." error matches isLocalRuntimeConnectionDropped (line 361, regex /Remote ADE service connection (closed|failed)/i) and is caught/retried/reset at lines 806-824. So the asymmetry the report describes is real: an idempotent read whose ensureProject->projects.add races a daemon drop throws a raw "connection closed" that escapes the retry loop with no reset, exactly the error string the commit set out to eliminate (RuntimeRpcClient.call rejects with this when closedError is set, runtimeRpcClient.ts:75; failConnection sets it, line 204-211). The renderer surfaces err.message raw (LanesPage.tsx), and the path is production runtime-backed (lane.list flows through callActionForRoot at main.ts:4136, registerIpc.ts:4823, runtimeBridge.ts:770), not test-only — so it is genuinely reachable.
However, the reachability is NARROWER than the report implies, which justifies a severity downgrade. The pool's onDisconnect callback (lines 1135-1144) clears BOTH this.connection=null AND projectsByRoot atomically inside failConnection when the socket 'close' event fires. Every state mutation that nulls connection also clears the project cache (verified at lines 945-949, 966-969, 984-987, 1141-1143, 1182-1184). Consequently, in the COMMON post-recycle sequence (close event already delivered before the next action), the next ensureProject cache-misses, connect() sees connection===null, and createConnection() builds a FRESH healthy connection to the new daemon — projects.add succeeds, no bug. The bug only manifests in a true async race: the socket close must be delivered DURING the await this.connect() inside ensureProject (or between connect resolving and the projects.add write) for a not-yet-cached root, so that connect() returns/uses a client whose closedError gets set mid-flight. There is no deterministic "connection non-null + client closed" state outside this race (compatibility-error paths close the client before storing it; reset paths null connection before closing). It is also a single transient failure — the immediately following call reconnects cleanly. So it is a real residual gap / incomplete-fix in the very commit being audited, but it degrades only to the pre-commit behavior in a residual timing window, not a deterministic or repeated failure.
Filed from the 2026-05-29 ADE codebase audit (adversarially verified).
Residual gap in merged commit 455f69b3 (daemon-drop recovery).
Severity: Low · Verify confidence: medium
File:
apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts:746What
Commit 455f69b added isClosed()-based reconnect and idempotent-read retry to recover from dropped daemon connections, but the guard only wraps the action call, not ensureProject() which runs first on the same connection.
In callActionForRootUncoalesced the order per attempt is: (1) const project = await this.ensureProject(rootPath) (line 746); (2) entry = await this.connect(); (3) if (entry.client.isClosed()) reconnect (line 752). ensureProject() (line 651-665) calls this.connect() and then entry.client.call("projects.add", ...) with NO isClosed() pre-check and NO drop-retry. connect() returns the cached this.connection if non-null (line 957), and this.connection is only cleared asynchronously by the client onDisconnect callback (line 1141) and the child exit handler (line 1182).
Failure mode: a daemon restart/build-hash recycle closes the socket; before the close event is processed on the event loop, the next IPC action calls ensureProject() -> connect() returns the stale closed client -> projects.add throws "Remote ADE service connection closed." This propagates out before the loop's try/catch (which starts at line 764), so there is no reset, no retry, and the renderer sees the exact raw error the commit set out to eliminate (e.g. "Lane was deleted, but refresh failed: ... connection closed."). The isClosed() guard at line 752 is effectively dead for the first call because ensureProject already used (and threw on) the same stale connection. Tests (localRuntimeConnectionPool.test.ts:1474-1502) only cover the classifier, not the ensureProject drop path.
Trigger
Rebuild desktop so the expected build hash changes (or restart/crash the daemon) while the renderer issues a read action; the first ensureProject->projects.add hits the stale closed connection and surfaces a raw 'Remote ADE service connection closed.' with no retry.
Verification (adversarial)
The structural claim is fully confirmed by reading the code. In callActionForRootUncoalesced (apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts:734), each attempt runs
const project = await this.ensureProject(rootPath)at line 746 — OUTSIDE the try block, which only starts at line 764. ensureProject (lines 651-665) callsawait this.connect()thenentry.client.call("projects.add", ...)at line 656 with NO isClosed() pre-check (the guard exists only at line 752 for the action path) and NO drop-retry. The action call at line 765 IS protected: its "Remote ADE service connection closed." error matches isLocalRuntimeConnectionDropped (line 361, regex /Remote ADE service connection (closed|failed)/i) and is caught/retried/reset at lines 806-824. So the asymmetry the report describes is real: an idempotent read whose ensureProject->projects.add races a daemon drop throws a raw "connection closed" that escapes the retry loop with no reset, exactly the error string the commit set out to eliminate (RuntimeRpcClient.call rejects with this when closedError is set, runtimeRpcClient.ts:75; failConnection sets it, line 204-211). The renderer surfaces err.message raw (LanesPage.tsx), and the path is production runtime-backed (lane.list flows through callActionForRoot at main.ts:4136, registerIpc.ts:4823, runtimeBridge.ts:770), not test-only — so it is genuinely reachable.However, the reachability is NARROWER than the report implies, which justifies a severity downgrade. The pool's onDisconnect callback (lines 1135-1144) clears BOTH this.connection=null AND projectsByRoot atomically inside failConnection when the socket 'close' event fires. Every state mutation that nulls connection also clears the project cache (verified at lines 945-949, 966-969, 984-987, 1141-1143, 1182-1184). Consequently, in the COMMON post-recycle sequence (close event already delivered before the next action), the next ensureProject cache-misses, connect() sees connection===null, and createConnection() builds a FRESH healthy connection to the new daemon — projects.add succeeds, no bug. The bug only manifests in a true async race: the socket close must be delivered DURING the
await this.connect()inside ensureProject (or between connect resolving and the projects.add write) for a not-yet-cached root, so that connect() returns/uses a client whose closedError gets set mid-flight. There is no deterministic "connection non-null + client closed" state outside this race (compatibility-error paths close the client before storing it; reset paths null connection before closing). It is also a single transient failure — the immediately following call reconnects cleanly. So it is a real residual gap / incomplete-fix in the very commit being audited, but it degrades only to the pre-commit behavior in a residual timing window, not a deterministic or repeated failure.Filed from the 2026-05-29 ADE codebase audit (adversarially verified).
Residual gap in merged commit
455f69b3(daemon-drop recovery).