From 50ef3f0bd1217fdad3ebbc06d37b36f499d36c5a Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 13 Sep 2026 20:04:25 +0900 Subject: [PATCH 1/6] fix(rpc): record connection edges before Windows named-pipe idle exit Authenticated readiness probes can open and close between supervisor ticks. Record attachment and first detachment immediately so the idle window measures continuous inactivity rather than sampled connection counts. Extract the proxy for deterministic real-socket coverage of reconnect, exact expiry, and rejected authentication. Verification: forced-ordering RED and mutation reproduce connect ENOENT; macOS lifecycle 33/33 on three runs; related RPC suites 40/40; static checks, build, RPC CLI QA and tool-loop QA pass. Broader mock-loop text-recovery QA has the same nine failures on base. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai Plan: .omo/plans/omo-dependency-diet.md --- packages/coding-agent/CHANGELOG.md | 2 + packages/coding-agent/docs/rpc.md | 3 + .../coding-agent/src/modes/rpc/changes.md | 20 +++ .../src/modes/rpc/host-client-proxy.ts | 47 +++++ .../src/modes/rpc/host-lifecycle.ts | 42 ++--- .../test/helpers/lifecycle-idle-proxy.ts | 162 ++++++++++++++++++ .../test/rpc-host-lifecycle.test.ts | 47 +---- 7 files changed, 247 insertions(+), 76 deletions(-) create mode 100644 packages/coding-agent/src/modes/rpc/host-client-proxy.ts create mode 100644 packages/coding-agent/test/helpers/lifecycle-idle-proxy.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 40cb5d54f..286a02e07 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -24,6 +24,8 @@ - Fixed shared RPC hosts expiring an old idle window after a short readiness connection, which could remove the Windows named pipe before the client attached (part of #1290). - Fixed missing Bedrock, Cursor, and Devin implementations in relocated standalone binaries by registering bundled modules in both the launcher and shared-session workers ([#1656](https://github.com/code-yeongyu/senpi/issues/1656)). +- Fixed the shared RPC host missing readiness connections between idle checks, which could close its Windows named pipe before the next client attached. Authenticated connection edges now reset the idle window immediately (Refs #1656). + - Fixed the ask-user question dialog carrying a committed own-answer into the next question: after answering a question with typed text, the next question's editor no longer shows the previous answer's text and pressing Enter again no longer submits it as the next question's own answer. - Fixed the remaining focus traps in the ask-user question dialog: committing an own answer now lands on the next question's option list instead of leaving the editor open; Up/Down, Tab/Shift+Tab and Backspace-on-empty leave the own-answer editor (Left/Right move its cursor); the Submit tab's review rows are navigable (Up from the comment highlights the last answer, Enter on a row jumps back to that question, Left/Right move the comment cursor once it has text); Backspace on the option list clears the answer instead of opening the editor; Esc inside the own-answer editor of an async question returns to the options instead of collapsing it; and re-expanding an async question restores its draft answers and comment. diff --git a/packages/coding-agent/docs/rpc.md b/packages/coding-agent/docs/rpc.md index 4aeb1fb54..bcfcee9c8 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -172,6 +172,9 @@ Environment overrides beat the file, and invalid values fall through to the next (`transient`|`persistent`) and `SENPI_RPC_HOST_IDLE_EXIT_MS` (positive integer milliseconds). The host exits only after the window elapses with NO attached client connections and NO active turns — continuously. +Authenticated connection and disconnection events update the idle clock immediately, including readiness probes that +fit entirely between timer ticks. Rejected authentication does not reset it. A readiness response is not a lease: +a caller that waits a full idle window before attaching may need to call `ensureHost()` again. Any connection or agent turn resets the window, so a busy host never exits, and the exit itself is clean: the RPC host receives SIGTERM first, flushes pending output, removes its socket, and the supervisor then removes `host.pid` and `settings.json` (the stderr log stays for diagnostics). After an idle exit, the next `ensureHost()` transparently diff --git a/packages/coding-agent/src/modes/rpc/changes.md b/packages/coding-agent/src/modes/rpc/changes.md index 7448fc31b..069aa0dc1 100644 --- a/packages/coding-agent/src/modes/rpc/changes.md +++ b/packages/coding-agent/src/modes/rpc/changes.md @@ -36,6 +36,26 @@ - `packages/coding-agent/src/modes/rpc/session-worker.ts` startup imports and initialization before `parentPort` message subscription. +## 2026-09-13 - Preserve short readiness activity between supervisor idle ticks (#1656) + +### What changed + +- `packages/coding-agent/src/modes/rpc/host-lifecycle.ts` delegates the authenticated public proxy to `packages/coding-agent/src/modes/rpc/host-client-proxy.ts` and updates its idle decider on connection edges, not only timer ticks. +- The extracted proxy reports successful attachment and the first detachment; rejected authentication does not count as activity. Idle policy values and the readiness protocol are unchanged. +- The lifecycle suite registers real-socket, controlled-clock regressions covering reconnect before expiry, exact idle expiry, and rejected authentication. + +### Why + +- A readiness connection could open and close between ticks. The next tick then treated time containing that connection as continuously idle, removed the Windows named pipe, and left the attaching client with `connect ENOENT`. + +### Why an extension could not handle it + +- The detached lifecycle supervisor owns the public listener and idle clock before any session extension runs. + +### Expected merge conflict zones + +- The public proxy construction in `packages/coding-agent/src/modes/rpc/host-lifecycle.ts`, its extracted implementation in `packages/coding-agent/src/modes/rpc/host-client-proxy.ts`, and the lifecycle test registration. + ## 2026-09-12 - Queued RPC input carries its source to extension `input` handlers ### What changed diff --git a/packages/coding-agent/src/modes/rpc/host-client-proxy.ts b/packages/coding-agent/src/modes/rpc/host-client-proxy.ts new file mode 100644 index 000000000..b54e66ce6 --- /dev/null +++ b/packages/coding-agent/src/modes/rpc/host-client-proxy.ts @@ -0,0 +1,47 @@ +import { createConnection, createServer, type Server, type Socket } from "node:net"; +import { authenticateSocket, resolveSocketTransportAddress, sendSocketHandshake } from "./socket-transport.ts"; + +interface HostProxyEndpoints { + readonly publicSecret: Uint8Array | undefined; + readonly internalSocket: string; + readonly internalSecret: Uint8Array | undefined; +} + +interface HostProxyActivity { + readonly clients: Set; + readonly isShuttingDown: () => boolean; + readonly onActivity: () => void; +} + +/** Authenticated public connections proxied to the private RPC host. */ +export function createHostClientProxy(endpoints: HostProxyEndpoints, activity: HostProxyActivity): Server { + return createServer((client) => { + const accept = (): void => { + if (activity.isShuttingDown()) { + client.destroy(); + return; + } + const internal = createConnection( + resolveSocketTransportAddress(endpoints.internalSocket, process.platform, endpoints.internalSecret), + ); + if (endpoints.internalSecret) sendSocketHandshake(internal, endpoints.internalSecret); + activity.clients.add(client); + // A readiness probe can connect and disconnect entirely between ticks. + // Record both edges so the idle window measures continuous inactivity. + activity.onActivity(); + const detach = (): void => { + if (activity.clients.delete(client)) activity.onActivity(); + internal.destroy(); + client.destroy(); + }; + client.pipe(internal); + internal.pipe(client); + client.once("close", detach); + client.once("error", detach); + internal.once("close", detach); + internal.once("error", detach); + }; + if (endpoints.publicSecret) authenticateSocket(client, endpoints.publicSecret, accept); + else accept(); + }); +} diff --git a/packages/coding-agent/src/modes/rpc/host-lifecycle.ts b/packages/coding-agent/src/modes/rpc/host-lifecycle.ts index 7a5161b24..6a21136c8 100644 --- a/packages/coding-agent/src/modes/rpc/host-lifecycle.ts +++ b/packages/coding-agent/src/modes/rpc/host-lifecycle.ts @@ -41,12 +41,13 @@ import { type ChildProcess, spawn } from "node:child_process"; import { randomUUID } from "node:crypto"; import { realpathSync, writeSync } from "node:fs"; import { access, chmod, mkdir, readFile, rm, unlink, writeFile } from "node:fs/promises"; -import { createConnection, createServer, type Server, type Socket } from "node:net"; +import { createConnection, type Server, type Socket } from "node:net"; import { tmpdir } from "node:os"; import { dirname, extname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { getAgentDir, isBunBinary } from "../../config.ts"; import { processIsLive, readProcessStartTime } from "../app-server/daemon/process.ts"; +import { createHostClientProxy } from "./host-client-proxy.ts"; import { createHostDaemonPaths } from "./host-ensure.ts"; import { HOST_CLEANUP_PATHS_ENV, @@ -65,7 +66,6 @@ import { writeSocketIdentityFile, } from "./socket-ownership.ts"; import { - authenticateSocket, createSocketSecret, ensureSocketSecret, resolveSocketTransportAddress, @@ -437,36 +437,14 @@ export async function runHostSupervisor(launch: SupervisorLaunch): Promise void shutdown(reason, exitCode); }); - const server = createServer((client) => { - const accept = (): void => { - if (shuttingDown) { - client.destroy(); - return; - } - const internal = createConnection( - resolveSocketTransportAddress(internalSocket, process.platform, internalSecret), - ); - if (internalSecret) sendSocketHandshake(internal, internalSecret); - clientSockets.add(client); - // A readiness exchange can begin and end between ticks. Record the - // attachment now, before a later tick can reuse the preceding idle window. - decider.update(currentActivity()); - const detach = (): void => { - clientSockets.delete(client); - decider.update(currentActivity()); - internal.destroy(); - client.destroy(); - }; - client.pipe(internal); - internal.pipe(client); - client.once("close", detach); - client.once("error", detach); - internal.once("close", detach); - internal.once("error", detach); - }; - if (publicSecret) authenticateSocket(client, publicSecret, accept); - else accept(); - }); + const server = createHostClientProxy( + { publicSecret, internalSocket, internalSecret }, + { + clients: clientSockets, + isShuttingDown: () => shuttingDown, + onActivity: () => decider.update(currentActivity()), + }, + ); server.once("error", (cause) => { if (!shuttingDown) void shutdown(`public socket listener failed: ${errorMessage(cause)}`, 1); }); diff --git a/packages/coding-agent/test/helpers/lifecycle-idle-proxy.ts b/packages/coding-agent/test/helpers/lifecycle-idle-proxy.ts new file mode 100644 index 000000000..23a0b0996 --- /dev/null +++ b/packages/coding-agent/test/helpers/lifecycle-idle-proxy.ts @@ -0,0 +1,162 @@ +import { randomBytes } from "node:crypto"; +import { once } from "node:events"; +import { mkdtempDisposable } from "node:fs/promises"; +import { createConnection, createServer, type Socket } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { createHostClientProxy } from "../../src/modes/rpc/host-client-proxy.ts"; +import { IdleExitDecider } from "../../src/modes/rpc/host-lifecycle.ts"; +import { resolveSocketTransportAddress, sendSocketHandshake } from "../../src/modes/rpc/socket-transport.ts"; + +/** #1656: registered by the lifecycle entry so the Windows job executes the regression. */ +export function registerIdleProxyTests(): void { + describe("idle exit decision core", () => { + function fakeClock(start: number): { now: () => number; advance: (ms: number) => void } { + let current = start; + return { now: () => current, advance: (ms: number) => (current += ms) }; + } + + it("exits only after the window elapsed with continuous idle", () => { + const clock = fakeClock(1_000); + const decider = new IdleExitDecider(600, clock.now); + expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); + clock.advance(599); + expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); + clock.advance(1); + expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("exit"); + }); + + it("activity resets the window; a connection or turn holds the host open", () => { + const clock = fakeClock(0); + const decider = new IdleExitDecider(600, clock.now); + decider.update({ connections: 0, activeTurns: 0 }); + clock.advance(500); + expect(decider.update({ connections: 1, activeTurns: 0 })).toBe("active"); + clock.advance(60_000); + expect(decider.update({ connections: 1, activeTurns: 0 })).toBe("active"); + expect(decider.update({ connections: 0, activeTurns: 2 })).toBe("active"); + clock.advance(60_000); + expect(decider.update({ connections: 0, activeTurns: 2 })).toBe("active"); + expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); + clock.advance(599); + expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); + clock.advance(1); + expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("exit"); + }); + + it("an infinite window (persistent cold start) never exits", () => { + const clock = fakeClock(0); + const decider = new IdleExitDecider(Number.POSITIVE_INFINITY, clock.now); + decider.update({ connections: 0, activeTurns: 0 }); + clock.advance(Number.MAX_SAFE_INTEGER); + expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); + }); + }); + + describe("supervisor activity between idle ticks", () => { + it.each([ + { + name: "accepts the next client when a readiness connection fits entirely between ticks", + authorized: true, + tickAt: 800, + serves: true, + }, + { + name: "exits when the full idle window elapses after the readiness connection detaches", + authorized: true, + tickAt: 1_599, + serves: false, + }, + { + name: "does not reset the idle window when authentication is rejected", + authorized: false, + tickAt: 800, + serves: false, + }, + ])( + "$name", + async ({ authorized, tickAt, serves }) => { + // Given: a real authenticated proxy, with only the idle clock controlled. + await using root = await mkdtempDisposable(join(tmpdir(), "hlp-")); + const internalSocket = join(root.path, "i.sock"); + const publicSocket = resolveSocketTransportAddress(join(root.path, "p.sock"), process.platform); + const secret = randomBytes(32); + let now = 0; + const decider = new IdleExitDecider(800, () => now); + const clients = new Set(); + const sockets = new Set(); + const currentActivity = () => ({ connections: clients.size, activeTurns: 0 }); + const activity = { + clients, + isShuttingDown: () => false, + onActivity: () => decider.update(currentActivity()), + }; + await using upstream = createServer((socket) => { + sockets.add(socket); + socket.pipe(socket); + }); + await using proxy = createHostClientProxy( + { publicSecret: secret, internalSocket, internalSecret: undefined }, + activity, + ); + const upstreamListening = once(upstream, "listening"); + upstream.listen(resolveSocketTransportAddress(internalSocket, process.platform)); + await upstreamListening; + const proxyListening = once(proxy, "listening"); + proxy.listen(publicSocket); + await proxyListening; + try { + decider.update(currentActivity()); + now = 790; + const accepted = new Promise((resolve) => proxy.once("connection", resolve)); + const probe = createConnection(publicSocket); + sockets.add(probe); + await once(probe, "connect", { signal: AbortSignal.timeout(5_000) }); + const detached = once(await accepted, "close", { signal: AbortSignal.timeout(5_000) }); + if (authorized) { + const reply = once(probe, "data", { signal: AbortSignal.timeout(5_000) }); + sendSocketHandshake(probe, secret); + probe.write("readiness"); + await reply; + now = 799; + probe.destroy(); + } else { + sendSocketHandshake( + probe, + secret.map((byte) => byte ^ 0xff), + ); + } + await detached; + + // When: the next supervisor tick runs after that entire connection. + now = tickAt; + if (decider.update(currentActivity()) === "exit") { + const closed = once(proxy, "close"); + proxy.close(); + await closed; + } + + // Then: connectability reflects continuous idle time, not sampled activity. + const client = createConnection(publicSocket); + sockets.add(client); + const connected = once(client, "connect", { signal: AbortSignal.timeout(5_000) }); + if (!serves) { + await expect(connected).rejects.toMatchObject({ code: "ENOENT" }); + return; + } + await connected; + const response = once(client, "data", { signal: AbortSignal.timeout(5_000) }); + sendSocketHandshake(client, secret); + client.write("next-request"); + const [data] = await response; + expect(data).toEqual(Buffer.from("next-request")); + } finally { + for (const socket of sockets) socket.destroy(); + for (const socket of clients) socket.destroy(); + } + }, + 15_000, + ); + }); +} diff --git a/packages/coding-agent/test/rpc-host-lifecycle.test.ts b/packages/coding-agent/test/rpc-host-lifecycle.test.ts index 1fdef178d..4533cea44 100644 --- a/packages/coding-agent/test/rpc-host-lifecycle.test.ts +++ b/packages/coding-agent/test/rpc-host-lifecycle.test.ts @@ -25,7 +25,6 @@ import { findInternalSupervisorArgs, HOST_COLD_START_ENV, HOST_IDLE_EXIT_MS_ENV, - IdleExitDecider, INTERNAL_SUPERVISOR_FLAG, resolveHostChildLaunch, resolveHostPolicy, @@ -46,8 +45,11 @@ import { sendSocketHandshake, socketSecretPath, } from "../src/modes/rpc/socket-transport.ts"; +import { registerIdleProxyTests } from "./helpers/lifecycle-idle-proxy.ts"; import { hermeticProviderEnv, MOCK_MODEL, MOCK_PROVIDER, writeRpcModelsJson } from "./helpers/rpc-hermetic.ts"; +registerIdleProxyTests(); + const roots: string[] = []; const peers: JsonlPeer[] = []; const models: HeldAnthropicModel[] = []; @@ -116,49 +118,6 @@ describe("host lifecycle policy resolution", () => { }); }); -describe("idle exit decision core", () => { - function fakeClock(start: number): { now: () => number; advance: (ms: number) => void } { - let current = start; - return { now: () => current, advance: (ms: number) => (current += ms) }; - } - - it("exits only after the window elapsed with continuous idle", () => { - const clock = fakeClock(1_000); - const decider = new IdleExitDecider(600, clock.now); - expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); - clock.advance(599); - expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); - clock.advance(1); - expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("exit"); - }); - - it("activity resets the window; a connection or turn holds the host open", () => { - const clock = fakeClock(0); - const decider = new IdleExitDecider(600, clock.now); - decider.update({ connections: 0, activeTurns: 0 }); - clock.advance(500); - expect(decider.update({ connections: 1, activeTurns: 0 })).toBe("active"); - clock.advance(60_000); - expect(decider.update({ connections: 1, activeTurns: 0 })).toBe("active"); - expect(decider.update({ connections: 0, activeTurns: 2 })).toBe("active"); - clock.advance(60_000); - expect(decider.update({ connections: 0, activeTurns: 2 })).toBe("active"); - expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); - clock.advance(599); - expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); - clock.advance(1); - expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("exit"); - }); - - it("an infinite window (persistent cold start) never exits", () => { - const clock = fakeClock(0); - const decider = new IdleExitDecider(Number.POSITIVE_INFINITY, clock.now); - decider.update({ connections: 0, activeTurns: 0 }); - clock.advance(Number.MAX_SAFE_INTEGER); - expect(decider.update({ connections: 0, activeTurns: 0 })).toBe("idle"); - }); -}); - describe("ensureHost-spawned host lifecycle", () => { // #1290: an ensure readiness connection can fit entirely between idle ticks. it("resets the idle window for a connection entirely between timer ticks", async () => { From 605413e5f563dab9b36b76863e08bb073151b718 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 14 Sep 2026 13:20:35 +0900 Subject: [PATCH 2/6] test(rpc): reproduce post-readiness idle expiry Force post-readiness ensure work past the host idle window so the lifecycle contract fails deterministically before the production fix.\n\nPlan: .omo/plans/omo-dependency-diet.md --- packages/coding-agent/src/modes/rpc/host-ensure.ts | 7 ++++++- packages/coding-agent/test/rpc-host-lifecycle.test.ts | 9 +++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/src/modes/rpc/host-ensure.ts b/packages/coding-agent/src/modes/rpc/host-ensure.ts index 89d64a6b6..ba2cbdc1c 100644 --- a/packages/coding-agent/src/modes/rpc/host-ensure.ts +++ b/packages/coding-agent/src/modes/rpc/host-ensure.ts @@ -66,6 +66,8 @@ export interface EnsureHostOptions { * to stall the real process-identity probe. */ readonly beforePidFileWrite?: () => Promise; + /** Runs after a successful readiness probe but before ensureHost returns. */ + readonly afterReadiness?: () => Promise; /** Overrides the process-identity probe so a test can force its failure. */ readonly readProcessStartTime?: (pid: number) => Promise; }; @@ -273,7 +275,10 @@ async function startHost( } const readinessTimeoutMs = testOptions?.readinessTimeoutMs ?? DEFAULT_READINESS_TIMEOUT_MS; const result = await pollProtocolInfo(socket, readinessTimeoutMs, childExit); - if (isCompatible(result.protocol)) return { pid: pidFile.pid, socket, reused: false }; + if (isCompatible(result.protocol)) { + await testOptions?.afterReadiness?.(); + return { pid: pidFile.pid, socket, reused: false }; + } // Teardown runs for the diagnostic's sake, so it must never replace it: a stop // failure here (unreadable identity, a host that outlives SIGKILL) would other- // wise propagate instead of the readiness message and skip cleanupState below, diff --git a/packages/coding-agent/test/rpc-host-lifecycle.test.ts b/packages/coding-agent/test/rpc-host-lifecycle.test.ts index 4533cea44..884d64b91 100644 --- a/packages/coding-agent/test/rpc-host-lifecycle.test.ts +++ b/packages/coding-agent/test/rpc-host-lifecycle.test.ts @@ -176,9 +176,12 @@ describe("ensureHost-spawned host lifecycle", () => { expect(listInternalSocketDirs().filter((dir) => !internalBefore.includes(dir))).toEqual([]); }, 45_000); - it("does not exit while a client is attached, then exits after it detaches", async () => { + it("returns an attachable host when post-readiness work outlasts the idle window", async () => { const qa = scratch("conn"); - await ensureLifecycleHost(qa, { policy: { idleExitMs: 600 } }); + await ensureLifecycleHost(qa, { + policy: { idleExitMs: 600 }, + afterReadiness: () => delay(800), + }); const entry = currentManaged(); const peer = await JsonlPeer.connect(qa.socket); await delay(2_000); @@ -637,6 +640,7 @@ async function ensureLifecycleHost( hostArgs?: string[]; env?: Record; spawn?: { command: string; args: string[] }; + afterReadiness?: () => Promise; } = {}, ) { const hostArgs = options.hostArgs ?? []; @@ -656,6 +660,7 @@ async function ensureLifecycleHost( ...(options.env ?? {}), }, hostArgs, + afterReadiness: options.afterReadiness, spawn: options.spawn ? { command: process.execPath, From f1b18e33ec262bda0e1a8e95d5c347ea3a8f5dcc Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 14 Sep 2026 13:40:04 +0900 Subject: [PATCH 3/6] fix(rpc): retain readiness through host handoff Keep the successful authenticated readiness connection attached until ensureHost is ready to return, so post-probe filesystem work cannot consume the transient host's idle window before the real client attaches.\n\nPlan: .omo/plans/omo-dependency-diet.md --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/docs/rpc.md | 6 +- .../coding-agent/src/modes/rpc/changes.md | 20 +++++++ .../coding-agent/src/modes/rpc/host-ensure.ts | 57 +++++++++++++------ .../test/rpc-host-lifecycle.test.ts | 7 +-- 5 files changed, 68 insertions(+), 23 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 286a02e07..266590d81 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -22,6 +22,7 @@ ### Fixed +- Fixed newly spawned shared RPC hosts consuming their entire idle window in post-readiness filesystem work before `ensureHost()` returned. The authenticated readiness connection now stays attached through the handoff, giving the first real client a complete idle window to attach (Refs #1656). - Fixed shared RPC hosts expiring an old idle window after a short readiness connection, which could remove the Windows named pipe before the client attached (part of #1290). - Fixed missing Bedrock, Cursor, and Devin implementations in relocated standalone binaries by registering bundled modules in both the launcher and shared-session workers ([#1656](https://github.com/code-yeongyu/senpi/issues/1656)). - Fixed the shared RPC host missing readiness connections between idle checks, which could close its Windows named pipe before the next client attached. Authenticated connection edges now reset the idle window immediately (Refs #1656). diff --git a/packages/coding-agent/docs/rpc.md b/packages/coding-agent/docs/rpc.md index bcfcee9c8..07aae38d6 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -173,8 +173,10 @@ Environment overrides beat the file, and invalid values fall through to the next The host exits only after the window elapses with NO attached client connections and NO active turns — continuously. Authenticated connection and disconnection events update the idle clock immediately, including readiness probes that -fit entirely between timer ticks. Rejected authentication does not reset it. A readiness response is not a lease: -a caller that waits a full idle window before attaching may need to call `ensureHost()` again. +fit entirely between timer ticks. Rejected authentication does not reset it. For a newly spawned host, `ensureHost()` +keeps its successful authenticated readiness connection attached through the remaining ownership and state work, then +releases it as the call returns. The caller therefore receives one complete idle window in which to attach; slow +post-probe filesystem work cannot consume that window before the handoff. Any connection or agent turn resets the window, so a busy host never exits, and the exit itself is clean: the RPC host receives SIGTERM first, flushes pending output, removes its socket, and the supervisor then removes `host.pid` and `settings.json` (the stderr log stays for diagnostics). After an idle exit, the next `ensureHost()` transparently diff --git a/packages/coding-agent/src/modes/rpc/changes.md b/packages/coding-agent/src/modes/rpc/changes.md index 069aa0dc1..61c064951 100644 --- a/packages/coding-agent/src/modes/rpc/changes.md +++ b/packages/coding-agent/src/modes/rpc/changes.md @@ -1,5 +1,25 @@ # changes +## 2026-09-14 - Retain readiness through the ensure handoff (#1656) + +### What changed + +- `packages/coding-agent/src/modes/rpc/host-ensure.ts` retains the successful authenticated readiness-probe connection through post-probe ownership and state work, releasing it only as `ensureHost()` returns. +- `packages/coding-agent/test/rpc-host-lifecycle.test.ts` forces post-readiness work beyond a complete idle window and verifies that the returned host still accepts and holds a real client. +- `packages/coding-agent/docs/rpc.md` documents the full idle-window handoff guarantee for newly spawned hosts. + +### Why + +- On a loaded Windows runner, lock release and state writes after the readiness probe detached could outlast the transient host's idle window. The supervisor then removed the named pipe before `ensureHost()` returned, so the first real client received `connect ENOENT`. + +### Why an extension could not handle it + +- `ensureHost()` owns the readiness connection and returns before any session extension can observe or influence the client handoff. + +### Expected merge conflict zones + +- LOW: the spawned-host readiness result and probe socket lifetime in `packages/coding-agent/src/modes/rpc/host-ensure.ts`. + ## 2026-09-13 - Reset supervisor idle time at occupancy transitions (#1290) ### What changed diff --git a/packages/coding-agent/src/modes/rpc/host-ensure.ts b/packages/coding-agent/src/modes/rpc/host-ensure.ts index ba2cbdc1c..c122094eb 100644 --- a/packages/coding-agent/src/modes/rpc/host-ensure.ts +++ b/packages/coding-agent/src/modes/rpc/host-ensure.ts @@ -66,7 +66,7 @@ export interface EnsureHostOptions { * to stall the real process-identity probe. */ readonly beforePidFileWrite?: () => Promise; - /** Runs after a successful readiness probe but before ensureHost returns. */ + /** Runs while the successful readiness connection is retained, before ensureHost returns. */ readonly afterReadiness?: () => Promise; /** Overrides the process-identity probe so a test can force its failure. */ readonly readProcessStartTime?: (pid: number) => Promise; @@ -155,7 +155,7 @@ async function ensureHostLocked( testOptions: EnsureHostOptions["_test"], ): Promise { const pidFile = await readPidFile(paths); - const protocol = await probeProtocolInfo(socket, EXISTING_HOST_PROBE_TIMEOUT_MS); + const { protocol } = await probeProtocolInfo(socket, EXISTING_HOST_PROBE_TIMEOUT_MS); if (isCompatible(protocol)) { // A compatible socket is attachable even when another client surface // started it. Only hosts we spawned are eligible for lifecycle management. @@ -274,10 +274,17 @@ async function startHost( await stderr.close(); } const readinessTimeoutMs = testOptions?.readinessTimeoutMs ?? DEFAULT_READINESS_TIMEOUT_MS; + // Keep the authenticated readiness connection attached through every remaining + // ensure step. Releasing it at the return boundary resets the supervisor's idle + // clock there, so slow lock/state I/O cannot consume the caller's attach window. const result = await pollProtocolInfo(socket, readinessTimeoutMs, childExit); if (isCompatible(result.protocol)) { - await testOptions?.afterReadiness?.(); - return { pid: pidFile.pid, socket, reused: false }; + try { + await testOptions?.afterReadiness?.(); + return { pid: pidFile.pid, socket, reused: false }; + } finally { + result.release?.(); + } } // Teardown runs for the diagnostic's sake, so it must never replace it: a stop // failure here (unreadable identity, a host that outlives SIGKILL) would other- @@ -410,7 +417,16 @@ async function waitForGone( type ChildExit = { readonly code: number | null; readonly signal: NodeJS.Signals | null }; -type ProtocolPollResult = { readonly protocol?: ProtocolInfo; readonly exited?: ChildExit }; +type ProtocolPollResult = { + readonly protocol?: ProtocolInfo; + readonly exited?: ChildExit; + readonly release?: () => void; +}; + +type ProtocolProbeResult = { + readonly protocol?: ProtocolInfo; + readonly release?: () => void; +}; async function pollProtocolInfo( socket: string, @@ -423,6 +439,7 @@ async function pollProtocolInfo( const probe = probeProtocolInfo( socket, Math.min(SPAWNED_HOST_PROBE_TIMEOUT_MS, Math.max(1, deadline - Date.now())), + true, ); const raced = childExit ? await Promise.race([probe, childExit]) : await probe; if (isChildExit(raced)) { @@ -432,32 +449,36 @@ async function pollProtocolInfo( // had a chance to deliver an answer. A host that never answers still // resolves through probeProtocolInfo's bounded timeout/close handling. const info = await probe; - if (info) { - lastProtocol = info; - if (isCompatible(info)) return { protocol: info }; + if (info.protocol) { + lastProtocol = info.protocol; + if (isCompatible(info.protocol)) return info; } else { return { protocol: lastProtocol, exited: raced }; } - } else if (raced) { - lastProtocol = raced; - if (isCompatible(raced)) return { protocol: raced }; + } else if (raced.protocol) { + lastProtocol = raced.protocol; + if (isCompatible(raced.protocol)) return raced; } await delay(50); } return { protocol: lastProtocol }; } -function isChildExit(value: ProtocolInfo | ChildExit | undefined): value is ChildExit { - return !!value && "code" in value && "signal" in value; +function isChildExit(value: ProtocolProbeResult | ChildExit): value is ChildExit { + return "code" in value && "signal" in value; } -async function probeProtocolInfo(socketPath: string, timeoutMs: number): Promise { +async function probeProtocolInfo( + socketPath: string, + timeoutMs: number, + retainCompatibleConnection = false, +): Promise { let secret: Buffer | undefined; if (process.platform === "win32") { try { secret = await readSocketSecret(socketSecretPath(socketPath)); } catch { - return undefined; + return {}; } } return new Promise((resolveProbe) => { @@ -468,8 +489,12 @@ async function probeProtocolInfo(socketPath: string, timeoutMs: number): Promise if (settled) return; settled = true; clearTimeout(timeout); + if (retainCompatibleConnection && isCompatible(value)) { + resolveProbe({ protocol: value, release: () => socket.destroy() }); + return; + } socket.destroy(); - resolveProbe(value); + resolveProbe({ protocol: value }); }; const timeout = setTimeout(() => finish(), timeoutMs); socket.once("connect", () => { diff --git a/packages/coding-agent/test/rpc-host-lifecycle.test.ts b/packages/coding-agent/test/rpc-host-lifecycle.test.ts index 884d64b91..b2c9314c6 100644 --- a/packages/coding-agent/test/rpc-host-lifecycle.test.ts +++ b/packages/coding-agent/test/rpc-host-lifecycle.test.ts @@ -180,6 +180,7 @@ describe("ensureHost-spawned host lifecycle", () => { const qa = scratch("conn"); await ensureLifecycleHost(qa, { policy: { idleExitMs: 600 }, + // Given: post-probe ensure work takes longer than a complete idle window. afterReadiness: () => delay(800), }); const entry = currentManaged(); @@ -686,11 +687,7 @@ async function ensureLifecycleHost( } } -/** - * The transient supervisor can idle-exit between answering the handshake and - * ensureHost returning, so the pidfile may already be gone; the returned pid is - * still the identity every later liveness/exit probe needs. - */ +/** The returned pid remains the fallback identity if shutdown removes the pidfile. */ async function recordedPidFile(pidFilePath: string, pid: number): Promise<{ pid: number; processStartTime: string }> { try { return JSON.parse(await readFile(pidFilePath, "utf8")) as { pid: number; processStartTime: string }; From e305c73d121238953a9c613238f2a517fdf8c046 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 14 Sep 2026 15:10:51 +0900 Subject: [PATCH 4/6] test(rpc): expose readiness across lock release Drive the real ownership-safe lock release and record readiness ownership on both sides, while retaining the existing premature inner teardown as the deterministic RED.\n\nPlan: .omo/plans/omo-dependency-diet.md --- .../coding-agent/src/modes/rpc/host-ensure.ts | 68 ++++++++++++++----- .../test/rpc-host-lifecycle.test.ts | 27 +++++--- 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/packages/coding-agent/src/modes/rpc/host-ensure.ts b/packages/coding-agent/src/modes/rpc/host-ensure.ts index c122094eb..3445fdb8e 100644 --- a/packages/coding-agent/src/modes/rpc/host-ensure.ts +++ b/packages/coding-agent/src/modes/rpc/host-ensure.ts @@ -66,8 +66,11 @@ export interface EnsureHostOptions { * to stall the real process-identity probe. */ readonly beforePidFileWrite?: () => Promise; - /** Runs while the successful readiness connection is retained, before ensureHost returns. */ - readonly afterReadiness?: () => Promise; + /** Wraps the real ownership-lock release so tests can observe readiness ownership across it. */ + readonly releaseOwnershipLock?: ( + releaseLock: () => Promise, + isReadinessRetained: () => boolean, + ) => Promise; /** Overrides the process-identity probe so a test can force its failure. */ readonly readProcessStartTime?: (pid: number) => Promise; }; @@ -79,6 +82,16 @@ export interface EnsuredHost { readonly reused: boolean; } +interface ReadinessLease { + readonly release: () => void; + readonly isRetained: () => boolean; +} + +type EnsuredHostLocked = { + readonly host: EnsuredHost; + readonly readinessLease?: ReadinessLease; +}; + type ProtocolInfo = { readonly serverVersion: string; readonly capabilities: readonly string[]; @@ -139,11 +152,19 @@ export async function ensureHost(options: EnsureHostOptions): Promise Promise) => releaseLock()); + try { + await releaseOwnershipLock(release, () => result?.readinessLease?.isRetained() ?? false); + } finally { + result?.readinessLease?.release(); + } } } @@ -153,13 +174,13 @@ async function ensureHostLocked( agentDir: string, policy: HostLifecyclePolicyInput | undefined, testOptions: EnsureHostOptions["_test"], -): Promise { +): Promise { const pidFile = await readPidFile(paths); const { protocol } = await probeProtocolInfo(socket, EXISTING_HOST_PROBE_TIMEOUT_MS); if (isCompatible(protocol)) { // A compatible socket is attachable even when another client surface // started it. Only hosts we spawned are eligible for lifecycle management. - return { pid: pidFile?.pid ?? 0, socket, reused: true }; + return { host: { pid: pidFile?.pid ?? 0, socket, reused: true } }; } const probe = testOptions?.readProcessStartTime ?? readProcessStartTime; const pidMatches = pidFile ? await matchesPidFileOrUnknown(pidFile, probe) : false; @@ -179,7 +200,7 @@ async function startHost( agentDir: string, policy: HostLifecyclePolicyInput | undefined, testOptions: EnsureHostOptions["_test"], -): Promise { +): Promise { // The settings file must exist before the supervisor reads it at boot, so it // records the policy before the spawn instead of beside the pidfile. if (process.platform === "win32") await createSocketSecret(socketSecretPath(socket)); @@ -274,16 +295,15 @@ async function startHost( await stderr.close(); } const readinessTimeoutMs = testOptions?.readinessTimeoutMs ?? DEFAULT_READINESS_TIMEOUT_MS; - // Keep the authenticated readiness connection attached through every remaining - // ensure step. Releasing it at the return boundary resets the supervisor's idle - // clock there, so slow lock/state I/O cannot consume the caller's attach window. const result = await pollProtocolInfo(socket, readinessTimeoutMs, childExit); if (isCompatible(result.protocol)) { try { - await testOptions?.afterReadiness?.(); - return { pid: pidFile.pid, socket, reused: false }; + return { + host: { pid: pidFile.pid, socket, reused: false }, + readinessLease: result.readinessLease, + }; } finally { - result.release?.(); + result.readinessLease?.release(); } } // Teardown runs for the diagnostic's sake, so it must never replace it: a stop @@ -420,12 +440,12 @@ type ChildExit = { readonly code: number | null; readonly signal: NodeJS.Signals type ProtocolPollResult = { readonly protocol?: ProtocolInfo; readonly exited?: ChildExit; - readonly release?: () => void; + readonly readinessLease?: ReadinessLease; }; type ProtocolProbeResult = { readonly protocol?: ProtocolInfo; - readonly release?: () => void; + readonly readinessLease?: ReadinessLease; }; async function pollProtocolInfo( @@ -485,12 +505,23 @@ async function probeProtocolInfo( const socket = createConnection(resolveSocketTransportAddress(socketPath, process.platform, secret)); let buffer = ""; let settled = false; + let retained = false; const finish = (value?: ProtocolInfo): void => { if (settled) return; settled = true; clearTimeout(timeout); if (retainCompatibleConnection && isCompatible(value)) { - resolveProbe({ protocol: value, release: () => socket.destroy() }); + retained = true; + resolveProbe({ + protocol: value, + readinessLease: { + isRetained: () => retained, + release: () => { + retained = false; + socket.destroy(); + }, + }, + }); return; } socket.destroy(); @@ -507,7 +538,10 @@ async function probeProtocolInfo( finish(readProtocolInfo(buffer.slice(0, newline))); }); socket.once("error", () => finish()); - socket.once("close", () => finish()); + socket.once("close", () => { + retained = false; + finish(); + }); // Register the error listener before sending the Windows named-pipe handshake. // When an idle host has already removed its pipe, the handshake write can // surface ENOENT immediately; without the listener this probe escapes instead diff --git a/packages/coding-agent/test/rpc-host-lifecycle.test.ts b/packages/coding-agent/test/rpc-host-lifecycle.test.ts index b2c9314c6..513ac4c87 100644 --- a/packages/coding-agent/test/rpc-host-lifecycle.test.ts +++ b/packages/coding-agent/test/rpc-host-lifecycle.test.ts @@ -176,17 +176,28 @@ describe("ensureHost-spawned host lifecycle", () => { expect(listInternalSocketDirs().filter((dir) => !internalBefore.includes(dir))).toEqual([]); }, 45_000); - it("returns an attachable host when post-readiness work outlasts the idle window", async () => { + it("keeps readiness attached through ownership lock release and returns an attachable host", async () => { + // Given: the real ownership-lock release records readiness ownership on both sides of its COMMIT. const qa = scratch("conn"); - await ensureLifecycleHost(qa, { + const readinessAcrossLockRelease: boolean[] = []; + + // When: ensureHost starts a transient host and completes its ownership handoff. + const ensured = await ensureLifecycleHost(qa, { policy: { idleExitMs: 600 }, - // Given: post-probe ensure work takes longer than a complete idle window. - afterReadiness: () => delay(800), + releaseOwnershipLock: async (releaseLock, isReadinessRetained) => { + readinessAcrossLockRelease.push(isReadinessRetained()); + await releaseLock(); + readinessAcrossLockRelease.push(isReadinessRetained()); + }, }); + + // Then: readiness spans the real lock release, the returned endpoint serves RPC, + // and detaching the only real client still lets the transient host idle-exit. + expect(ensured.reused).toBe(false); + expect(readinessAcrossLockRelease).toEqual([true, true]); const entry = currentManaged(); const peer = await JsonlPeer.connect(qa.socket); - await delay(2_000); - await expectHostAlive(qa, entry.pidFile); + expect(await peer.request({ id: "handoff", type: "get_protocol_info" })).toMatchObject({ success: true }); peer.destroy(); await waitForHostExit(entry); }, 45_000); @@ -641,7 +652,7 @@ async function ensureLifecycleHost( hostArgs?: string[]; env?: Record; spawn?: { command: string; args: string[] }; - afterReadiness?: () => Promise; + releaseOwnershipLock?: (releaseLock: () => Promise, isReadinessRetained: () => boolean) => Promise; } = {}, ) { const hostArgs = options.hostArgs ?? []; @@ -661,7 +672,7 @@ async function ensureLifecycleHost( ...(options.env ?? {}), }, hostArgs, - afterReadiness: options.afterReadiness, + releaseOwnershipLock: options.releaseOwnershipLock, spawn: options.spawn ? { command: process.execPath, From 8a00139a6332897af984cbdb65a97f19614ecefb Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 14 Sep 2026 15:24:50 +0900 Subject: [PATCH 5/6] fix(rpc): retain readiness through lock release Transfer the successful readiness lease to ensureHost so it survives the ownership lock COMMIT and close, then release it in failure-safe final cleanup at the actual return boundary.\n\nPlan: .omo/plans/omo-dependency-diet.md --- packages/coding-agent/CHANGELOG.md | 3 +-- packages/coding-agent/docs/rpc.md | 6 +++--- packages/coding-agent/src/modes/rpc/changes.md | 6 +++--- .../coding-agent/src/modes/rpc/host-ensure.ts | 15 +++++++-------- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9105ac96a..f01bfff28 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,7 +12,7 @@ ### Fixed -- Fixed newly spawned shared RPC hosts consuming their entire idle window in post-readiness filesystem work before `ensureHost()` returned. The authenticated readiness connection now stays attached through the handoff, giving the first real client a complete idle window to attach (Refs #1656). +- Fixed newly spawned shared RPC hosts consuming their entire idle window in post-readiness filesystem work before `ensureHost()` returned. The authenticated readiness connection now stays attached through the endpoint lock commit and final handoff, giving the first real client a complete idle window to attach (Refs #1656). - Fixed Enter on multi-select question options to toggle the highlighted choice without advancing, including option 1; empty own-answer commits preserve selections, and hints direct users to Tab and Submit when done (#8249). @@ -64,7 +64,6 @@ - Fixed shared RPC hosts expiring an old idle window after a short readiness connection, which could remove the Windows named pipe before the client attached (part of #1290). - Fixed missing Bedrock, Cursor, and Devin implementations in relocated standalone binaries by registering bundled modules in both the launcher and shared-session workers ([#1656](https://github.com/code-yeongyu/senpi/issues/1656)). -- Fixed the shared RPC host missing readiness connections between idle checks, which could close its Windows named pipe before the next client attached. Authenticated connection edges now reset the idle window immediately (Refs #1656). - Fixed the ask-user question dialog carrying a committed own-answer into the next question: after answering a question with typed text, the next question's editor no longer shows the previous answer's text and pressing Enter again no longer submits it as the next question's own answer. - Fixed the remaining focus traps in the ask-user question dialog: committing an own answer now lands on the next question's option list instead of leaving the editor open; Up/Down, Tab/Shift+Tab and Backspace-on-empty leave the own-answer editor (Left/Right move its cursor); the Submit tab's review rows are navigable (Up from the comment highlights the last answer, Enter on a row jumps back to that question, Left/Right move the comment cursor once it has text); Backspace on the option list clears the answer instead of opening the editor; Esc inside the own-answer editor of an async question returns to the options instead of collapsing it; and re-expanding an async question restores its draft answers and comment. diff --git a/packages/coding-agent/docs/rpc.md b/packages/coding-agent/docs/rpc.md index 07aae38d6..8bedc3b52 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -174,9 +174,9 @@ Environment overrides beat the file, and invalid values fall through to the next The host exits only after the window elapses with NO attached client connections and NO active turns — continuously. Authenticated connection and disconnection events update the idle clock immediately, including readiness probes that fit entirely between timer ticks. Rejected authentication does not reset it. For a newly spawned host, `ensureHost()` -keeps its successful authenticated readiness connection attached through the remaining ownership and state work, then -releases it as the call returns. The caller therefore receives one complete idle window in which to attach; slow -post-probe filesystem work cannot consume that window before the handoff. +keeps its successful authenticated readiness connection attached through the remaining ownership work, including the +endpoint lock's commit and close, then releases it as the call returns. The caller therefore receives one complete idle +window in which to attach; slow post-probe filesystem work cannot consume that window before the handoff. Any connection or agent turn resets the window, so a busy host never exits, and the exit itself is clean: the RPC host receives SIGTERM first, flushes pending output, removes its socket, and the supervisor then removes `host.pid` and `settings.json` (the stderr log stays for diagnostics). After an idle exit, the next `ensureHost()` transparently diff --git a/packages/coding-agent/src/modes/rpc/changes.md b/packages/coding-agent/src/modes/rpc/changes.md index 61c064951..de829a8d9 100644 --- a/packages/coding-agent/src/modes/rpc/changes.md +++ b/packages/coding-agent/src/modes/rpc/changes.md @@ -4,8 +4,8 @@ ### What changed -- `packages/coding-agent/src/modes/rpc/host-ensure.ts` retains the successful authenticated readiness-probe connection through post-probe ownership and state work, releasing it only as `ensureHost()` returns. -- `packages/coding-agent/test/rpc-host-lifecycle.test.ts` forces post-readiness work beyond a complete idle window and verifies that the returned host still accepts and holds a real client. +- `packages/coding-agent/src/modes/rpc/host-ensure.ts` transfers the successful authenticated readiness-probe connection to the outer ensure scope, retains it through the endpoint lock's real commit and close, and releases it in final cleanup immediately before `ensureHost()` settles. +- `packages/coding-agent/test/rpc-host-lifecycle.test.ts` wraps the real ownership-lock release, records readiness ownership before and after its commit, verifies a protocol exchange after handoff, and proves the transient host still idle-exits after that client detaches without a scheduling delay. - `packages/coding-agent/docs/rpc.md` documents the full idle-window handoff guarantee for newly spawned hosts. ### Why @@ -18,7 +18,7 @@ ### Expected merge conflict zones -- LOW: the spawned-host readiness result and probe socket lifetime in `packages/coding-agent/src/modes/rpc/host-ensure.ts`. +- LOW: the spawned-host readiness result, outer endpoint-lock cleanup, and probe socket lifetime in `packages/coding-agent/src/modes/rpc/host-ensure.ts`. ## 2026-09-13 - Reset supervisor idle time at occupancy transitions (#1290) diff --git a/packages/coding-agent/src/modes/rpc/host-ensure.ts b/packages/coding-agent/src/modes/rpc/host-ensure.ts index 3445fdb8e..55387602f 100644 --- a/packages/coding-agent/src/modes/rpc/host-ensure.ts +++ b/packages/coding-agent/src/modes/rpc/host-ensure.ts @@ -163,6 +163,9 @@ export async function ensureHost(options: EnsureHostOptions): Promise result?.readinessLease?.isRetained() ?? false); } finally { + // The outer ensure scope owns the successful readiness connection. Release + // it only after the endpoint lock has committed and closed, immediately + // before the function settles, including when lock release itself fails. result?.readinessLease?.release(); } } @@ -297,14 +300,10 @@ async function startHost( const readinessTimeoutMs = testOptions?.readinessTimeoutMs ?? DEFAULT_READINESS_TIMEOUT_MS; const result = await pollProtocolInfo(socket, readinessTimeoutMs, childExit); if (isCompatible(result.protocol)) { - try { - return { - host: { pid: pidFile.pid, socket, reused: false }, - readinessLease: result.readinessLease, - }; - } finally { - result.readinessLease?.release(); - } + return { + host: { pid: pidFile.pid, socket, reused: false }, + readinessLease: result.readinessLease, + }; } // Teardown runs for the diagnostic's sake, so it must never replace it: a stop // failure here (unreadable identity, a host that outlives SIGKILL) would other- From f04ecdc1a8a321d72a2322e44d58e57c4127ca8e Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 15 Sep 2026 18:07:00 +0900 Subject: [PATCH 6/6] test(rpc): keep turn-active waiters inside the test budget JsonlPeer.waitFor defaulted to 15s and scanned before arming. The turn-active lifecycle case already declared a 60s test timeout, so the nested clock was timing luck on a loaded Windows runner (CI run 34823696518 attempt 3). Arm the waiter first, mark sibling timeouts handled, and use the test budget for open/prompt/agent_start. Plan: .omo/plans/omo-dependency-diet.md --- .../test/rpc-host-lifecycle.test.ts | 93 +++++++++++++------ 1 file changed, 66 insertions(+), 27 deletions(-) diff --git a/packages/coding-agent/test/rpc-host-lifecycle.test.ts b/packages/coding-agent/test/rpc-host-lifecycle.test.ts index 513ac4c87..21543b1fc 100644 --- a/packages/coding-agent/test/rpc-host-lifecycle.test.ts +++ b/packages/coding-agent/test/rpc-host-lifecycle.test.ts @@ -46,7 +46,13 @@ import { socketSecretPath, } from "../src/modes/rpc/socket-transport.ts"; import { registerIdleProxyTests } from "./helpers/lifecycle-idle-proxy.ts"; -import { hermeticProviderEnv, MOCK_MODEL, MOCK_PROVIDER, writeRpcModelsJson } from "./helpers/rpc-hermetic.ts"; +import { + hermeticProviderEnv, + MOCK_API_KEY, + MOCK_MODEL, + MOCK_PROVIDER, + writeRpcModelsJson, +} from "./helpers/rpc-hermetic.ts"; registerIdleProxyTests(); @@ -58,6 +64,10 @@ const collisionChildFixture = join(import.meta.dirname, "fixtures", "rpc-collisi // Windows supervisor-exit observation is load-dependent; teardown is eventually // consistent within the watchdog fallback bound, so the affected waits allow 30s. const WINDOWS_SUPERVISOR_EXIT_TIMEOUT_MS = 30_000; +// The turn-active case already opts into a 60s test budget. JsonlPeer.request/ +// waitFor default to 15s; that nested clock is what fired on Windows CI +// (run 34823696518 attempt 3) while the test timeout still had headroom. +const TURN_ACTIVE_TEST_MS = 60_000; afterEach(async () => { for (const peer of peers.splice(0)) peer.destroy(); @@ -202,28 +212,39 @@ describe("ensureHost-spawned host lifecycle", () => { await waitForHostExit(entry); }, 45_000); - it("does not exit while a turn is active even with no connections; exits after the turn settles", async () => { - const qa = scratch("turn"); - const model = await HeldAnthropicModel.start(); - models.push(model); - writeRpcModelsJson(qa.agentDir, model.origin); - await ensureLifecycleHost(qa, { - policy: { idleExitMs: 800 }, - hostArgs: ["--provider", MOCK_PROVIDER, "--model", MOCK_MODEL], - }); - const entry = currentManaged(); - const peer = await JsonlPeer.connect(qa.socket); - const opened = await peer.request({ id: "open", type: "open_session", cwd: qa.cwd }); - const sessionId = openedSessionId(opened); - const agentStart = peer.waitFor((value) => value.type === "agent_start" && value.sessionId === sessionId); - await peer.request({ id: "prompt", type: "prompt", sessionId, message: "hold this turn open" }); - await agentStart; - peer.destroy(); - await delay(2_500); - await expectHostAlive(qa, entry.pidFile); - model.release(); - await waitForHostExit(entry, 20_000); - }, 60_000); + it( + "does not exit while a turn is active even with no connections; exits after the turn settles", + async () => { + const qa = scratch("turn"); + const model = await HeldAnthropicModel.start(); + models.push(model); + writeRpcModelsJson(qa.agentDir, model.origin); + await ensureLifecycleHost(qa, { + policy: { idleExitMs: 800 }, + hostArgs: ["--provider", MOCK_PROVIDER, "--model", MOCK_MODEL], + env: { ANTHROPIC_API_KEY: MOCK_API_KEY }, + }); + const entry = currentManaged(); + const peer = await JsonlPeer.connect(qa.socket); + const opened = await peer.request({ id: "open", type: "open_session", cwd: qa.cwd }, TURN_ACTIVE_TEST_MS); + const sessionId = openedSessionId(opened); + const agentStart = peer.waitFor( + (value) => value.type === "agent_start" && value.sessionId === sessionId, + TURN_ACTIVE_TEST_MS, + ); + await peer.request( + { id: "prompt", type: "prompt", sessionId, message: "hold this turn open" }, + TURN_ACTIVE_TEST_MS, + ); + await agentStart; + peer.destroy(); + await delay(2_500); + await expectHostAlive(qa, entry.pidFile); + model.release(); + await waitForHostExit(entry, 20_000); + }, + TURN_ACTIVE_TEST_MS, + ); it("starts a fresh host transparently on the next ensure after an idle exit", async () => { const qa = scratch("ensure"); @@ -957,19 +978,37 @@ class JsonlPeer { } waitFor(predicate: (value: RecordValue) => boolean, timeoutMs = 15_000): Promise { - const existing = this.messages.find(predicate); - if (existing) return Promise.resolve(existing); - return new Promise((resolve, reject) => { + const pending = new Promise((resolve, reject) => { const waiter = { predicate, resolve, timer: setTimeout(() => { this.waiters.delete(waiter); - reject(new Error("Timed out waiting for RPC record")); + reject( + new Error( + `Timed out waiting for RPC record: ${JSON.stringify( + this.messages.map((value) => ({ + type: value.type, + id: value.id, + command: value.command, + })), + )}`, + ), + ); }, timeoutMs), }; this.waiters.add(waiter); + const existing = this.messages.find(predicate); + if (!existing) return; + clearTimeout(waiter.timer); + this.waiters.delete(waiter); + resolve(existing); }); + // Arm-then-await: a sibling waiter can time out while this test is still + // awaiting another request. Mark handled at creation so Node does not blame + // the next test; the rejection still surfaces on the eventual await. + pending.catch(() => {}); + return pending; } destroy(): void {