diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 8a5e3d7cc..29020a533 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,8 @@ ### 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 endpoint lock commit and final handoff, giving the first real client a complete idle window to attach (Refs #1656). + ### Removed ## [2026.9.15] - 2026-09-15 @@ -105,8 +107,10 @@ - Fixed the goal monitor parking on the ask-user idle-timeout setting instead of the earliest pending question deadline, so a shorter request no longer waits for a longer one; typing in an answer now extends that park without adding continuation prompts ([#1645](https://github.com/code-yeongyu/senpi/issues/1645)). + - 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 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..8bedc3b52 100644 --- a/packages/coding-agent/docs/rpc.md +++ b/packages/coding-agent/docs/rpc.md @@ -172,6 +172,11 @@ 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. For a newly spawned host, `ensureHost()` +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 f1b85146e..a2c8ee9d7 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` 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 + +- 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, outer endpoint-lock cleanup, and probe socket lifetime in `packages/coding-agent/src/modes/rpc/host-ensure.ts`. + ## 2026-09-14 - Keep bundled workers out of supervisor entry dispatch ### What changed @@ -54,6 +74,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-ensure.ts b/packages/coding-agent/src/modes/rpc/host-ensure.ts index 89d64a6b6..55387602f 100644 --- a/packages/coding-agent/src/modes/rpc/host-ensure.ts +++ b/packages/coding-agent/src/modes/rpc/host-ensure.ts @@ -66,6 +66,11 @@ export interface EnsureHostOptions { * to stall the real process-identity probe. */ readonly beforePidFileWrite?: () => 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; }; @@ -77,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[]; @@ -137,11 +152,22 @@ export async function ensureHost(options: EnsureHostOptions): Promise Promise) => releaseLock()); + try { + await releaseOwnershipLock(release, () => 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(); + } } } @@ -151,13 +177,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); + 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; @@ -177,7 +203,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)); @@ -273,7 +299,12 @@ 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)) { + 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- // wise propagate instead of the readiness message and skip cleanupState below, @@ -405,7 +436,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 readinessLease?: ReadinessLease; +}; + +type ProtocolProbeResult = { + readonly protocol?: ProtocolInfo; + readonly readinessLease?: ReadinessLease; +}; async function pollProtocolInfo( socket: string, @@ -418,6 +458,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)) { @@ -427,44 +468,63 @@ 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) => { 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)) { + retained = true; + resolveProbe({ + protocol: value, + readinessLease: { + isRetained: () => retained, + release: () => { + retained = false; + socket.destroy(); + }, + }, + }); + return; + } socket.destroy(); - resolveProbe(value); + resolveProbe({ protocol: value }); }; const timeout = setTimeout(() => finish(), timeoutMs); socket.once("connect", () => { @@ -477,7 +537,10 @@ async function probeProtocolInfo(socketPath: string, timeoutMs: number): Promise 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/src/modes/rpc/host-lifecycle.ts b/packages/coding-agent/src/modes/rpc/host-lifecycle.ts index f1480582b..5987c10df 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, isBundledNode } 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..21543b1fc 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,7 +45,16 @@ import { sendSocketHandshake, socketSecretPath, } from "../src/modes/rpc/socket-transport.ts"; -import { hermeticProviderEnv, MOCK_MODEL, MOCK_PROVIDER, writeRpcModelsJson } from "./helpers/rpc-hermetic.ts"; +import { registerIdleProxyTests } from "./helpers/lifecycle-idle-proxy.ts"; +import { + hermeticProviderEnv, + MOCK_API_KEY, + MOCK_MODEL, + MOCK_PROVIDER, + writeRpcModelsJson, +} from "./helpers/rpc-hermetic.ts"; + +registerIdleProxyTests(); const roots: string[] = []; const peers: JsonlPeer[] = []; @@ -56,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(); @@ -116,49 +128,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 () => { @@ -217,39 +186,65 @@ 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("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, { policy: { idleExitMs: 600 } }); + const readinessAcrossLockRelease: boolean[] = []; + + // When: ensureHost starts a transient host and completes its ownership handoff. + const ensured = await ensureLifecycleHost(qa, { + policy: { idleExitMs: 600 }, + 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); - 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"); @@ -678,6 +673,7 @@ async function ensureLifecycleHost( hostArgs?: string[]; env?: Record; spawn?: { command: string; args: string[] }; + releaseOwnershipLock?: (releaseLock: () => Promise, isReadinessRetained: () => boolean) => Promise; } = {}, ) { const hostArgs = options.hostArgs ?? []; @@ -697,6 +693,7 @@ async function ensureLifecycleHost( ...(options.env ?? {}), }, hostArgs, + releaseOwnershipLock: options.releaseOwnershipLock, spawn: options.spawn ? { command: process.execPath, @@ -722,11 +719,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 }; @@ -985,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 {