From 968a08c6d0eee2835f87b034d7dde37660a591cc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 28 Sep 2026 16:07:44 -0700 Subject: [PATCH] perf(nodes): reuse warm workers so node session turns start as fast as local ones (#160265) * perf(nodes): reuse warm workers so node turns start as fast as local ones Retain settled workers for two minutes with at most two idle children per node. Negotiate node-worker-idle-retention-v1 across Gateway, node, and bundle; fresh turn admission and credentials remain mandatory. Protect background work and preserve durable process cleanup and reconciliation. Expose reclaimable idle capacity to placement admission and the session picker. Consolidate lifecycle helpers under the supervisor and use the shared capacity parser. Document the bounded process-count and memory tradeoff; include lifecycle, negotiation, and picker regression coverage. fix(nodes): retry failed idle worker cleanup Rearm the supervisor idle timer after a transient physical cleanup failure while retaining the durable slot and retirement fence. Cover automatic expiry and idle-limit eviction alongside explicit cleanup with a fake clock. Fold the cancellation forwarding wrapper into its owner and remove the immediately repeated admission abort check. Preserve the existing authority, settlement, and shutdown guards. fix(nodes): synchronize idle capacity protocol models Regenerate the Swift worker-slot model for the optional reclaimableIdle field. The schema and named capability contract remain unchanged. Inline the single-caller prepared workspace custody wrapper in the supervisor and move its four cases to the public launch entry point. Preserve exact acquisition, abort, shutdown, and release ordering while keeping the complete branch production delta negative. chore(nodes): prune stale worker assertion allowance Remove the exact one-entry assertion baseline for worker-command.runtime.ts after the unsafe assertion was eliminated. Keep the scanner and all unrelated allowances unchanged. fix(nodes): keep supervisor dependencies on protocol owners Import protocol parsers and types directly from their canonical leaf, including the QA fixture, and remove the redundant supervisor-control re-exports. This breaks the dependency cycle without duplicating the wire contract. Keep the runner cancellation mock and shutdown callback asynchronous with the real Promise contract, including teardown restoration. fix(nodes): complete asynchronous cleanup contracts Join cancellation cleanup in callers and tests while preserving parallel shutdown settlement and expected failure identity. Exclude absent cleanup owners before aggregation and keep stale-connection rejection unchanged. Use immutable worker input, type-only journal imports, stable sorted idle projections, and receiver-bound fault injection. Remove redundant descriptor bookkeeping and finish the test lint cleanup without changing limits or suppressions. * test(nodes): disambiguate inventory fixtures --- .../OpenClawProtocol/GatewayModels.swift | 11 +- config/assertion-safety-baseline.txt | 1 - .../cloud-workers/session-lifecycle.md | 11 + docs/nodes/node-host.md | 2 +- docs/nodes/session-hosting.md | 44 +- .../src/schema/environments.test.ts | 9 + .../src/schema/environments.ts | 3 +- .../src/schema/worker-admission.ts | 2 + .../src/server-capabilities.ts | 1 + src/gateway/node-registry.test.ts | 31 +- src/gateway/node-runner-inventory-runtime.ts | 3 +- .../sessions.dispatch.device.test.ts | 31 +- .../server/ws-connection/connect-hello.ts | 1 + .../device-placement-eligibility.ts | 6 +- .../device-placement-selector.ts | 5 +- .../node-launch-adapter.test.ts | 36 + .../node-launch-adapter.ts | 37 +- src/infra/node-runner-inventory.test.ts | 60 +- src/infra/node-runner-inventory.ts | 7 +- src/node-host/connection.test.ts | 148 ++- src/node-host/connection.ts | 53 +- src/node-host/node-worker-capacity.ts | 86 +- src/node-host/node-worker-journal-worker.ts | 4 + .../node-worker-launch-observation.test.ts | 27 +- .../node-worker-launch-observation.ts | 14 +- src/node-host/node-worker-launch-store.ts | 60 +- src/node-host/node-worker-launch-transport.ts | 26 +- src/node-host/node-worker-launch.ts | 280 ------ ...e-worker-prepared-workspace-launch.test.ts | 87 -- src/node-host/node-worker-supervisor-close.ts | 58 -- .../node-worker-supervisor-commands.ts | 205 ++-- .../node-worker-supervisor-contract.ts | 42 +- .../node-worker-supervisor-ownership.ts | 52 +- .../node-worker-supervisor-recovery.ts | 44 - .../node-worker-supervisor.admission.test.ts | 6 +- ...-worker-supervisor.fixture.test-support.ts | 14 +- .../node-worker-supervisor.idle.test.ts | 755 ++++++++++++++ .../node-worker-supervisor.lifetime.test.ts | 9 +- ...ode-worker-supervisor.mock.test-support.ts | 126 +++ .../node-worker-supervisor.recovery.test.ts | 14 +- ...rker-supervisor.settlement.test-support.ts | 147 +-- .../node-worker-supervisor.settlement.test.ts | 11 +- .../node-worker-supervisor.test-support.ts | 10 +- src/node-host/node-worker-supervisor.test.ts | 33 +- src/node-host/node-worker-supervisor.ts | 930 +++++++++++++++--- src/node-host/node-worker-turn-lifecycle.ts | 367 ------- src/node-host/node-worker-turn-store.ts | 18 +- src/node-host/runner.shutdown.test.ts | 4 +- src/node-host/runner.test-support.ts | 2 +- src/node-host/runtime.computer.test.ts | 6 +- src/node-host/runtime.test-support.ts | 3 + src/node-host/runtime.test.ts | 99 +- src/node-host/runtime.ts | 100 +- src/node-host/runtime.update-pause.test.ts | 38 +- src/node-host/runtime.worker-hosting.test.ts | 1 + .../runtime.worker-supervisor.test.ts | 2 +- src/node-host/worker-runtime.test.ts | 3 +- src/shared/node-list-parse.ts | 18 +- src/worker/github-binding.runtime.test.ts | 46 +- src/worker/github-binding.runtime.ts | 28 +- src/worker/node-supervisor-protocol.test.ts | 21 + src/worker/node-supervisor-protocol.ts | 11 +- src/worker/worker-command.runtime.test.ts | 170 +++- src/worker/worker-command.runtime.ts | 158 +-- src/worker/worker-process-protocol.test.ts | 25 +- src/worker/worker-process-protocol.ts | 58 +- .../worker-runtime-background-exec.suite.ts | 6 +- src/worker/worker.runtime.test.ts | 50 +- src/worker/worker.runtime.ts | 2 +- .../paired-node-worker-wire-fixture.ts | 2 +- ui/src/pages/new-session/device-placement.ts | 5 +- ui/src/pages/new-session/discovery.ts | 23 +- ui/src/pages/new-session/where-chip.test.ts | 9 + 73 files changed, 3086 insertions(+), 1731 deletions(-) delete mode 100644 src/node-host/node-worker-launch.ts delete mode 100644 src/node-host/node-worker-prepared-workspace-launch.test.ts delete mode 100644 src/node-host/node-worker-supervisor-close.ts create mode 100644 src/node-host/node-worker-supervisor.idle.test.ts create mode 100644 src/node-host/node-worker-supervisor.mock.test-support.ts diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index 43b1e6618ec5..06abe22c0427 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -25456,13 +25456,22 @@ public struct WorkerEnvironmentMetadata: Codable, Sendable { public struct WorkerSlotSummary: Codable, Sendable { public let total: Int public let available: Int + public let reclaimableidle: Int? public init( total: Int, - available: Int) + available: Int, + reclaimableidle: Int? = nil) { self.total = total self.available = available + self.reclaimableidle = reclaimableidle + } + + private enum CodingKeys: String, CodingKey { + case total + case available + case reclaimableidle = "reclaimableIdle" } } diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index a539572c88ef..a1e51441c6a8 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -3075,7 +3075,6 @@ src/wizard/setup.ts 1 src/worker/inference-stream.runtime.ts 4 src/worker/launch-descriptor.ts 5 src/worker/node-workspace-protocol.ts 1 -src/worker/worker-command.runtime.ts 1 src/worker/worker-connection-admission.ts 2 src/worker/worker-connection-frames.ts 4 src/worker/worker-process.ts 1 diff --git a/docs/gateway/cloud-workers/session-lifecycle.md b/docs/gateway/cloud-workers/session-lifecycle.md index c2ce00486cea..858220c34527 100644 --- a/docs/gateway/cloud-workers/session-lifecycle.md +++ b/docs/gateway/cloud-workers/session-lifecycle.md @@ -40,6 +40,17 @@ Disconnected workers have no cleanup deadline. Nodes also reclaim copies when th Completed cloud turns preserve eligible, size-bounded workspace files before the turn claim is released. Repository-only sessions accept a cumulative immutable checkpoint in the Gateway's bare artifact repository. Gateway-source sessions apply those changes to their managed worktree. Worker-turn uses its terminal worker event to create the durable pending-result fence. Remote-exec waits for workspace quiescence and enters the same reconciliation flow after the local Codex attempt. Before applying the result, the Gateway stages complete authenticated base/current manifests plus each changed resulting blob as a Git ref under `refs/openclaw/worker-results/`; deletions are represented by the manifests and need no blob. This keeps the cloud delta recoverable even if the Gateway stops during the apply without duplicating unchanged baseline content. Workspace results use Git file semantics: regular files, executable bits, symlinks, additions, changes, and deletions are retained, while empty directories and other directory modes are not. Gateway-source changes remain in the managed worktree for normal review and commit; repository-only changes remain on the node and in the accepted checkpoint. +OpenClaw worker-turn sessions may keep a settled worker process idle for up to +two minutes, with at most two idle workers per node. Follow-up turns reuse the +loaded runtime with fresh turn authority; placement activation does not start a +worker. Idle workers inherit the existing background-retention reconciliation +contract: the process stays alive in both process and container mode, and the +capture/verify/renew/verify manifest fences detect concurrent workspace changes. +Turn connections and temporary profiles are disposed before idle readiness. +Idle workers are evictable for capacity, updates, and disconnect cleanup; +background commands are not. See [node session hosting](/nodes/session-hosting) +for compatibility and memory costs. + Workspace quiescence retries slow process probes within one 30-second budget. The recovery watchdog keeps unfinished processes across at most four passes, with up to seven seconds of backoff between them, so recovery has a total probe and backoff budget of 127 seconds. Slow probes cannot repeatedly resume the same workers and starve the rest. Each probe starts with a two-second allowance and gets more time after a timeout. Exhaustion retains the unfinished PID/start references and reason in the lease for the Gateway's next recovery attempt; check host load and `ps` availability, then retry workspace recovery. Failed reconciliation retains the recoverable workspace result and reports the reason through the normal recovery flow. Result staging and rollback preserve exact supported filenames and file bytes, independently of Git attributes and checkout encodings. diff --git a/docs/nodes/node-host.md b/docs/nodes/node-host.md index b25a7c2625b5..3705911b0a06 100644 --- a/docs/nodes/node-host.md +++ b/docs/nodes/node-host.md @@ -146,7 +146,7 @@ openclaw node restart Node shutdown waits for plugin availability watchers and active computer executions to finish cleanup, and reports failures from those cleanup operations. If a command -reports `Node plugin cleanup failed`, reconnect the node to retry disconnect cleanup +reports `Node disconnect cleanup failed`, reconnect the node to retry disconnect cleanup before sending another command. ### Automatic node updates diff --git a/docs/nodes/session-hosting.md b/docs/nodes/session-hosting.md index 88ca8387b9bf..7b179fac052e 100644 --- a/docs/nodes/session-hosting.md +++ b/docs/nodes/session-hosting.md @@ -113,8 +113,44 @@ This setting enables supervised session turns on the paired device, including Gateway-owned workspace transfer and result reconciliation. By default, each node has one worker slot per available CPU core. Configure the slot count with `nodeHost.workerRuns.capacity`. Launches beyond capacity wait up to 10 seconds -for a durable slot; while all slots are occupied, the node remains available -for status and cancellation but is not selected for a new session turn. +for a durable slot. A slot occupied only by an idle worker can be reclaimed for +new work; active turns and background commands keep their slots. When no free +or reclaimable slot remains, the node stays available for status and cancellation +but is not selected for a new session turn. + +After a turn settles, OpenClaw can retain its worker process for up to two +minutes so an immediate follow-up avoids loading the runtime again. The timer +starts after the worker confirms that turn cleanup is complete. Each node keeps +at most two idle workers, bounded by its configured capacity; it retires the +least recently idle worker first when space is needed. Idle workers still use +memory: expect several hundred MiB per retained worker and its supervision +processes even for a small session, with larger heaps possible after substantial work. +Workers are started by turns, never just by activating a placement. + +Idle reuse requires support from the Gateway, node, and installed worker bundle +through the `node-worker-idle-retention-v1` capability. Older combinations keep +their existing background-command retention behavior. Every reused turn still +gets fresh credentials, admission, history, execution policy, and a turn +profile. Entering idle closes the turn connection, joins its write-capable +cleanup, and removes the finished turn's temporary profile. Background commands +remain protected from idle eviction and timeouts; their existing environment +and credential lifetime applies until they finish. + +Idle retention uses the same workspace-reconciliation contract as background +command retention, in both process and container mode. Neither contract suspends +the worker process. Capture, verification, renewal, and final verification of +the workspace manifest detect concurrent changes; a conflict or failed fence +uses the existing reconciliation and recovery flow. Retaining a settled runtime +does not grant it authority for another turn. + +Disconnect, node shutdown, update pause, or placement teardown retires idle +workers through normal process-tree or container cleanup. Reconnect waits for +that cleanup before publishing fresh capacity. If idle cleanup fails, the running +supervisor keeps its slot reserved and retries after two minutes. Stop, Move, and reclaim retain +their exact placement ownership checks; idle workers are never restored after +a crash or restart. Chat **Stop** cancels active work and does not flush an idle +worker when no turn is running. Use placement Stop or reclaim to release it +immediately. There is no separate idle-retention setting. Stopping an active hosted turn records the accepted cancellation even if the worker encounters an error while stopping. Worker diagnostics retain the shutdown @@ -138,7 +174,7 @@ process tree is gone. The picker derives every device row from `environments.list`. Every selected runtime requires an available, connected paired session host. OpenClaw worker turns additionally require captured exec-policy support and valid exact worker -slots with at least one free slot. Codex paired-device execution launches its +slots with at least one free or reclaimable idle slot. Codex paired-device execution launches its exec-server directly, so it does not consume or require a worker slot. Its required command must appear in the node's effective `invocableCommands`, not merely its declared capabilities. A declared command is usable only when @@ -158,7 +194,7 @@ remote session. Choose **Auto** to let the Gateway select an eligible paired, connected session host. For OpenClaw worker turns, it first prefers hosts with less admitted work relative to their worker capacity. It then compares free -worker slots after accounting for dispatches still starting, and breaks +and reclaimable idle worker slots after accounting for dispatches still starting, and breaks remaining ties by device ID. A session's placement alone does not reserve a worker slot. Runtimes that do not consume worker slots choose the eligible host with the lowest device ID instead. diff --git a/packages/gateway-protocol/src/schema/environments.test.ts b/packages/gateway-protocol/src/schema/environments.test.ts index a045b749544d..ab26072a9a1c 100644 --- a/packages/gateway-protocol/src/schema/environments.test.ts +++ b/packages/gateway-protocol/src/schema/environments.test.ts @@ -343,6 +343,15 @@ describe("worker environment protocol schemas", () => { it("accepts only bounded closed worker slot summaries", () => { const slots = { total: 2, available: 1 }; expect(Value.Check(WorkerSlotSummarySchema, slots)).toBe(true); + expect( + Value.Check(WorkerSlotSummarySchema, { total: 1, available: 0, reclaimableIdle: 1 }), + ).toBe(true); + expect( + Value.Check(WorkerSlotSummarySchema, { total: 1, available: 1, reclaimableIdle: 1 }), + ).toBe(false); + expect( + Value.Check(WorkerSlotSummarySchema, { total: 4, available: 0, reclaimableIdle: 3 }), + ).toBe(false); expect( Value.Check(EnvironmentSummarySchema, { id: "node:build-mac", diff --git a/packages/gateway-protocol/src/schema/environments.ts b/packages/gateway-protocol/src/schema/environments.ts index d2fa8437c6d2..a677c00897bf 100644 --- a/packages/gateway-protocol/src/schema/environments.ts +++ b/packages/gateway-protocol/src/schema/environments.ts @@ -85,8 +85,9 @@ export const WorkerSlotSummarySchema = Type.Refine( closedObject({ total: Type.Integer({ minimum: 1, maximum: 1_024 }), available: Type.Integer({ minimum: 0, maximum: 1_024 }), + reclaimableIdle: Type.Optional(Type.Integer({ minimum: 0, maximum: 2 })), }), - (slots) => slots.available <= slots.total, + (slots) => slots.available + (slots.reclaimableIdle ?? 0) <= slots.total, (slots) => `available worker slots ${slots.available} exceed total ${slots.total}`, ); diff --git a/packages/gateway-protocol/src/schema/worker-admission.ts b/packages/gateway-protocol/src/schema/worker-admission.ts index 1fe082a04cb6..068830077132 100644 --- a/packages/gateway-protocol/src/schema/worker-admission.ts +++ b/packages/gateway-protocol/src/schema/worker-admission.ts @@ -51,6 +51,7 @@ export const WORKER_LAUNCH_V2_PROTOCOL_FEATURE = "worker-launch-v2"; export const WORKER_EXECUTION_CONTEXT_PROTOCOL_FEATURE = "worker-execution-context-v2"; export const WORKER_EXECUTION_AUTHORITY_PROTOCOL_FEATURE = "worker-execution-authority-v1"; export const WORKER_LINEAGE_START_PROTOCOL_FEATURE = "worker-lineage-start-v1"; +export const NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE = "node-worker-idle-retention-v1"; export const WORKER_SESSION_TOOLS_PROTOCOL_FEATURE = "worker-session-tools-v1"; export const WORKER_PORTAL_PROTOCOL_FEATURE = "worker-portal-v1"; export const WORKER_PRESENCE_PROTOCOL_FEATURE = "worker-presence-v1"; @@ -65,6 +66,7 @@ export const WORKER_PROTOCOL_FEATURES = [ WORKER_EXECUTION_CONTEXT_PROTOCOL_FEATURE, WORKER_EXECUTION_AUTHORITY_PROTOCOL_FEATURE, WORKER_LINEAGE_START_PROTOCOL_FEATURE, + NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE, WORKER_SESSION_TOOLS_PROTOCOL_FEATURE, WORKER_PORTAL_PROTOCOL_FEATURE, WORKER_PRESENCE_PROTOCOL_FEATURE, diff --git a/packages/gateway-protocol/src/server-capabilities.ts b/packages/gateway-protocol/src/server-capabilities.ts index cdd06817eb9e..b45de04a3be6 100644 --- a/packages/gateway-protocol/src/server-capabilities.ts +++ b/packages/gateway-protocol/src/server-capabilities.ts @@ -9,6 +9,7 @@ export const GATEWAY_SERVER_CAPS = { NODE_WORKER_BUNDLE_STATUS: "node-worker-bundle-status-v1", NODE_WORKER_CAPTURED_EXEC_POLICY: "node-worker-captured-exec-policy", NODE_WORKER_ENVIRONMENT_SESSION: "node-worker-environment-session-v1", + NODE_WORKER_IDLE_RETENTION: "node-worker-idle-retention-v1", NODE_WORKER_LAUNCH_TOOL_NAMES: "node-worker-launch-tool-names-v1", NODE_WORKER_PORTAL_STREAM: "node-worker-portal-stream-v1", NODE_WORKER_STATUS_WAIT: "node-worker-status-wait-v1", diff --git a/src/gateway/node-registry.test.ts b/src/gateway/node-registry.test.ts index acdb3eb9b4a8..60e8cd400254 100644 --- a/src/gateway/node-registry.test.ts +++ b/src/gateway/node-registry.test.ts @@ -23,7 +23,10 @@ import { NODE_WORKER_SUPERVISOR_STATUS_COMMAND, NODE_WORKER_WORKSPACE_EXEC_COMMAND, } from "../infra/node-commands.js"; -import { NODE_WORKER_SUPERVISOR_PROTOCOL_FEATURE } from "../infra/node-runner-inventory.js"; +import { + NODE_WORKER_SUPERVISOR_PROTOCOL_FEATURE, + type NodeWorkerHostDeclaration, +} from "../infra/node-runner-inventory.js"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { resetLogger, setLoggerOverride } from "../logging/logger.js"; import { createDiagnosticLogRecordCapture } from "../logging/test-helpers/diagnostic-log-capture.js"; @@ -580,13 +583,7 @@ describe("gateway/node-registry", () => { }), { pairingIdentity: "identity-a", pairingGeneration: "generation-a" }, ); - const publish = (workerHost: { - enabled: true; - capacity: { total: number; available: number }; - bundleRetention?: 1; - bundleStatus?: 1; - statusWait?: 1; - }) => + const publish = (workerHost: NodeWorkerHostDeclaration) => updateNodeRunnerInventory({ registry: nodeRegistry, nodeId: "node-1", @@ -636,6 +633,24 @@ describe("gateway/node-registry", () => { if (!proof) { throw new Error("expected current runner proof"); } + expect(nodeWorkerSupervisorTransport.isCurrent(proof, true)).toBe(false); + const idleHost = { + ...retained, + idleRetention: true as const, + capacity: { total: 2, available: 0, reclaimableIdle: 1 }, + }; + expect(publish(idleHost)).toEqual({ changed: true }); + expect(nodeWorkerSupervisorTransport.isCurrent(proof, true)).toBe(true); + expect( + collectNodeCatalogRuntimeState(nodeRegistry, [ + { nodeId: "node-1", connId: "conn-1", pairingGeneration: "generation-a" }, + ]).workerSlotsByNodeId.get("node-1"), + ).toEqual(idleHost.capacity); + expect( + publish({ ...idleHost, capacity: { ...idleHost.capacity, reclaimableIdle: 0 } }), + ).toEqual({ changed: true }); + expect(nodeWorkerSupervisorTransport.isCurrent(proof, true)).toBe(false); + runnerStateChanged.mockClear(); expect( nodeWorkerSupervisorTransport.acceptBundleStatus?.(proof, { bundleHash: "a".repeat(64), diff --git a/src/gateway/node-runner-inventory-runtime.ts b/src/gateway/node-runner-inventory-runtime.ts index e91006067f0a..891bf976f699 100644 --- a/src/gateway/node-runner-inventory-runtime.ts +++ b/src/gateway/node-runner-inventory-runtime.ts @@ -13,6 +13,7 @@ import { type NodeWorkerCapacitySnapshot, } from "../infra/node-runner-inventory.js"; import { createDeferredCore } from "../shared/deferred.js"; +import { availableWorkerSlots } from "../shared/node-list-parse.js"; import type { NodeWorkerBundleStatus } from "../shared/node-list-types.js"; type NodeWorkerHostClientId = @@ -301,7 +302,7 @@ export function isNodeWorkerSupervisorProofCurrent( current.clientId === proof.clientId && current.clientMode === proof.clientMode && current.protocolFeature === proof.protocolFeature && - (!requirements.launchEligibility || current.workerHost.capacity.available > 0) && + (!requirements.launchEligibility || availableWorkerSlots(current.workerHost.capacity) > 0) && (!requirements.environmentSession || current.workerHost.environmentSession === NODE_WORKER_ENVIRONMENT_SESSION_VERSION) && (!requirements.statusWait || diff --git a/src/gateway/server-methods/sessions.dispatch.device.test.ts b/src/gateway/server-methods/sessions.dispatch.device.test.ts index 75030a45de17..311abcf85dff 100644 --- a/src/gateway/server-methods/sessions.dispatch.device.test.ts +++ b/src/gateway/server-methods/sessions.dispatch.device.test.ts @@ -95,7 +95,7 @@ function pairedNode(deviceId: string): PairedDevice { }; } -function connectedNode(deviceId: string, available: number) { +function connectedNode(deviceId: string, available: number): NodeWorkerSupervisorNodeProof { return { nodeId: deviceId, connId: `conn-${deviceId}`, @@ -221,6 +221,35 @@ describe("sessions.dispatch device targets", () => { vi.restoreAllMocks(); }); + it("dispatches to a capacity-one host whose occupied slot is reclaimable idle", async () => { + useDeviceSession(); + const node = connectedNode("idle-host", 0); + node.workerHost.capacity = { total: 1, available: 0, reclaimableIdle: 1 }; + node.workerHost.idleRetention = true; + vi.spyOn(environmentMethods, "listGatewayEnvironments").mockResolvedValue( + deviceEnvironments([node]), + ); + const dispatch = vi.fn().mockResolvedValue(activeDevicePlacement(node.nodeId)); + const context = makeDispatchTestContext({ + nodeRegistry: { get: () => node } as never, + workerPlacementDispatchService: { dispatch }, + workerSessionPlacementService: { getMany: () => new Map() }, + }); + bindDeviceWorkerAvailability(context.workerEnvironmentService!, async () => ({ + available: true, + node, + })); + const respond = await invokeSessionDispatch(context, { autoDevice: true }); + expect(dispatch).toHaveBeenCalledWith( + expect.objectContaining({ deviceId: node.nodeId }), + expect.any(Function), + undefined, + undefined, + ); + expect(respond).toHaveBeenCalledWith(true, expect.objectContaining({ ok: true }), undefined); + expect(node.workerHost.capacity.available).toBe(0); + }); + it("dispatches to the highest-capacity eligible host and identifies it in the response", async () => { useDeviceSession(); const nodes = [connectedNode("smaller", 1), connectedNode("largest", 4)]; diff --git a/src/gateway/server/ws-connection/connect-hello.ts b/src/gateway/server/ws-connection/connect-hello.ts index bfe1b090377c..a91e8e5cde62 100644 --- a/src/gateway/server/ws-connection/connect-hello.ts +++ b/src/gateway/server/ws-connection/connect-hello.ts @@ -181,6 +181,7 @@ export async function sendGatewayHello( GATEWAY_SERVER_CAPS.NODE_WORKER_BUNDLE_STATUS, GATEWAY_SERVER_CAPS.NODE_WORKER_CAPTURED_EXEC_POLICY, GATEWAY_SERVER_CAPS.NODE_WORKER_ENVIRONMENT_SESSION, + GATEWAY_SERVER_CAPS.NODE_WORKER_IDLE_RETENTION, GATEWAY_SERVER_CAPS.NODE_WORKER_LAUNCH_TOOL_NAMES, GATEWAY_SERVER_CAPS.NODE_WORKER_PORTAL_STREAM, GATEWAY_SERVER_CAPS.NODE_WORKER_STATUS_WAIT, diff --git a/src/gateway/worker-environments/device-placement-eligibility.ts b/src/gateway/worker-environments/device-placement-eligibility.ts index fe72521e72f0..430e4ed4a0cb 100644 --- a/src/gateway/worker-environments/device-placement-eligibility.ts +++ b/src/gateway/worker-environments/device-placement-eligibility.ts @@ -4,6 +4,7 @@ import { resolveNodeWorkerExecutionIssue, type NodeRunnerInventoryIssue, } from "../../infra/node-runner-inventory.js"; +import { availableWorkerSlots } from "../../shared/node-list-parse.js"; import { resolveNodeCommandAllowlist, resolveRequiredNodeCommandAuthority, @@ -138,7 +139,8 @@ export async function resolveDevicePlacementEligibility(params: { error: `paired-device command ${requiredNodeCommand.command} is not enabled or approved for ${deviceId}; enable it in gateway.nodes.commands.allow and approve the command on the node`, }; } - if (requirement.consumesWorkerSlot && node.workerHost.capacity.available <= 0) { + const availableSlots = availableWorkerSlots(node.workerHost.capacity); + if (requirement.consumesWorkerSlot && availableSlots <= 0) { return { ok: false, error: deviceUnavailableText(deviceId, { @@ -147,5 +149,5 @@ export async function resolveDevicePlacementEligibility(params: { }), }; } - return { ok: true, availableSlots: node.workerHost.capacity.available, node }; + return { ok: true, availableSlots, node }; } diff --git a/src/gateway/worker-environments/device-placement-selector.ts b/src/gateway/worker-environments/device-placement-selector.ts index 1d10ad84e691..559c85ccccfc 100644 --- a/src/gateway/worker-environments/device-placement-selector.ts +++ b/src/gateway/worker-environments/device-placement-selector.ts @@ -1,6 +1,7 @@ import type { EnvironmentSummary } from "../../../packages/gateway-protocol/src/index.js"; import type { DevicePlacementRequirement } from "../../agents/harness/types.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { availableWorkerSlots } from "../../shared/node-list-parse.js"; import type { NodeRegistry } from "../node-registry.js"; import { resolveDevicePlacementEligibility } from "./device-placement-eligibility.js"; import { deviceUnavailableText } from "./device-provider.js"; @@ -84,7 +85,9 @@ export async function selectDevicePlacementCandidates(params: { 0, eligibility.availableSlots - (params.getPendingDispatchCount?.(deviceId) ?? 0), ) - : (node.workerSlots?.available ?? 0), + : node.workerSlots + ? availableWorkerSlots(node.workerSlots) + : 0, eligibility, }; }), diff --git a/src/gateway/worker-environments/node-launch-adapter.test.ts b/src/gateway/worker-environments/node-launch-adapter.test.ts index 0c51fe529e27..f2b0e03340be 100644 --- a/src/gateway/worker-environments/node-launch-adapter.test.ts +++ b/src/gateway/worker-environments/node-launch-adapter.test.ts @@ -4,6 +4,7 @@ import { GATEWAY_CLIENT_MODES, } from "../../../packages/gateway-protocol/src/client-info.js"; import { + NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE, WORKER_PROTOCOL_FEATURES, WORKER_RPC_SET_VERSION, } from "../../../packages/gateway-protocol/src/schema/worker-admission.js"; @@ -181,6 +182,41 @@ describe("node worker launch adapter", () => { resetGatewayWorkAdmission(); } }); + it.each([ + [false, false], + [false, true], + [true, false], + [true, true], + ])( + "negotiates idle retention only for node=%s and bundle=%s", + async (nodeSupports, bundleSupports) => { + const input = launchInput(); + if (!bundleSupports) { + input.descriptor.admission.handshake.protocolFeatures = + input.descriptor.admission.handshake.protocolFeatures.filter( + (feature) => feature !== NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE, + ); + } + const node = nodeProof(); + if (nodeSupports) { + node.workerHost.idleRetention = true; + } + const expected = { + ...input, + ...(nodeSupports && bundleSupports ? { idleRetention: true as const } : {}), + }; + const invoke = vi.fn(async (request) => { + expect(request.params).toEqual(expected); + return wire(receipt(expected, "completed")); + }); + const adapter = createNodeWorkerLaunchAdapter({ + getTransport: () => transportWith(invoke, async () => [node]), + }); + expect(await adapter.launch(launchRequest(input))).toEqual(receipt(expected, "completed")); + expect(input).not.toHaveProperty("idleRetention"); + expect(invoke).toHaveBeenCalledOnce(); + }, + ); it("cancels an in-flight status wait through the existing terminal cancellation receipt", async () => { const input = launchInput(); diff --git a/src/gateway/worker-environments/node-launch-adapter.ts b/src/gateway/worker-environments/node-launch-adapter.ts index f8c481347981..1d6fbc5d8053 100644 --- a/src/gateway/worker-environments/node-launch-adapter.ts +++ b/src/gateway/worker-environments/node-launch-adapter.ts @@ -1,5 +1,6 @@ import { createHash, randomUUID } from "node:crypto"; import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion"; +import { NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE } from "../../../packages/gateway-protocol/src/schema/worker-admission.js"; import { isAgentRunRestartAbortReason } from "../../agents/run-termination.js"; import { computeBackoff, sleepWithAbort } from "../../infra/backoff.js"; import { @@ -134,10 +135,15 @@ function rearmNodeWorkerLaunchInput( } export function measureNodeWorkerLaunchBytes(nodeId: string, input: NodeWorkerLaunchInput): number { + const measured = input.descriptor.admission.handshake.protocolFeatures.includes( + NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE, + ) + ? { ...input, idleRetention: true as const } + : input; // Re-arms replace a UUID with a SHA-256 hex turn ID. Measure both without changing // the original plan; the registry bounds timeouts and always generates UUID request IDs. return Math.max( - ...[input, rearmNodeWorkerLaunchInput(input, 1)].flatMap((attempt) => [ + ...[measured, rearmNodeWorkerLaunchInput(measured, 1)].flatMap((attempt) => [ Buffer.byteLength( serializeNodeEvent( "node.invoke.request", @@ -152,7 +158,7 @@ export function measureNodeWorkerLaunchBytes(nodeId: string, input: NodeWorkerLa ), "utf8", ), - measureWorkerProcessTurnBytes(attempt.descriptor), + measureWorkerProcessTurnBytes(attempt.descriptor, attempt.idleRetention), ]), ); } @@ -289,6 +295,8 @@ export function createNodeWorkerLaunchAdapter(options: NodeWorkerLaunchAdapterOp isAuthorized: () => boolean; deadline: OperationDeadline; onDispatchReady?: () => void; + idleRetention?: true; + prepareLaunch?: (node: NodeWorkerSupervisorNodeProof) => void; }): Promise<{ receipt: NodeWorkerSupervisorReceipt | null; statusWait: boolean }> => { if (!params.isAuthorized()) { throw new NodeWorkerLaunchTransportError( @@ -324,6 +332,13 @@ export function createNodeWorkerLaunchAdapter(options: NodeWorkerLaunchAdapterOp deviceId: params.deviceId, signal, }); + params.prepareLaunch?.(node); + if (!params.prepareLaunch && params.idleRetention && node.workerHost.idleRetention !== true) { + throw new NodeWorkerLaunchTransportError( + "PRIVATE_DIALECT_UNAVAILABLE", + "node worker idle retention is unavailable", + ); + } if ( params.command === NODE_WORKER_SUPERVISOR_LAUNCH_COMMAND && (node.workerHost.environmentSession !== NODE_WORKER_ENVIRONMENT_SESSION_VERSION || @@ -502,6 +517,24 @@ export function createNodeWorkerLaunchAdapter(options: NodeWorkerLaunchAdapterOp ? NODE_WORKER_SUPERVISOR_STATUS_COMMAND : NODE_WORKER_SUPERVISOR_LAUNCH_COMMAND, payload: pollStatus ? { launchId: input.launchId } : input, + ...(!pollStatus && input.idleRetention ? { idleRetention: true as const } : {}), + ...(!pollStatus && !mayHaveLaunched + ? { + prepareLaunch: (node: NodeWorkerSupervisorNodeProof) => { + if ( + node.workerHost.idleRetention === true && + input.descriptor.admission.handshake.protocolFeatures.includes( + NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE, + ) + ) { + input.idleRetention = true; + } else { + delete input.idleRetention; + } + expected = expectedIdentity(input); + }, + } + : {}), isAuthorized: () => { if (!dispatchReady) { // Publish expiry through the signal; a guard throw can orphan the invoke promise. diff --git a/src/infra/node-runner-inventory.test.ts b/src/infra/node-runner-inventory.test.ts index 168640a78518..b9b70682d103 100644 --- a/src/infra/node-runner-inventory.test.ts +++ b/src/infra/node-runner-inventory.test.ts @@ -1,5 +1,10 @@ -import { expect, it } from "vitest"; -import { parseNodeRunnerInventoryDeclaration } from "./node-runner-inventory.js"; +import { describe, expect, it } from "vitest"; +import { availableWorkerSlots } from "../shared/node-list-parse.js"; +import { + NODE_WORKER_SUPERVISOR_PROTOCOL_FEATURE, + parseNodeRunnerInventoryDeclaration, + type NodeWorkerCapacitySnapshot, +} from "./node-runner-inventory.js"; const capacity = { total: 2, available: 1 }; const workerHost = { @@ -15,7 +20,7 @@ const workerHost = { capturedExecPolicy: true, }; const declaration = (host: unknown) => ({ - protocolFeatures: ["node-worker-supervisor-v6"], + protocolFeatures: [NODE_WORKER_SUPERVISOR_PROTOCOL_FEATURE], workerHost: host, }); @@ -61,3 +66,52 @@ it("keeps retired dialect markers observational and empty declarations valid", ( protocolFeatures, }); }); + +describe("idle worker capacity negotiation", () => { + const idleDeclaration = (slots: unknown, idleRetention?: unknown) => ({ + protocolFeatures: [NODE_WORKER_SUPERVISOR_PROTOCOL_FEATURE], + workerHost: { + enabled: true, + capacity: slots, + statusWait: 1, + ...(idleRetention === undefined ? {} : { idleRetention }), + }, + }); + + it.each<[NodeWorkerCapacitySnapshot, true | undefined]>([ + [{ total: 1, available: 1 }, undefined], + [{ total: 1, available: 0, reclaimableIdle: 1 }, true], + [{ total: 4, available: 1, reclaimableIdle: 2 }, true], + ])("preserves exact negotiated inventory shape %j", (slots, idleRetention) => { + const input = idleDeclaration(slots, idleRetention); + expect(parseNodeRunnerInventoryDeclaration(input)).toEqual(input); + expect(availableWorkerSlots(slots)).toBe(slots.available + (slots.reclaimableIdle ?? 0)); + }); + + it.each([ + [{ total: 1, available: 0, reclaimableIdle: 1 }, undefined], + [{ total: 1, available: 1, reclaimableIdle: 1 }, true], + [{ total: 4, available: 0, reclaimableIdle: 3 }, true], + [{ total: 1, available: 0, reclaimableIdle: -1 }, true], + [{ total: 1, available: 0, reclaimableIdle: 0.5 }, true], + [{ total: 1, available: 0, reclaimableIdle: 0 }, false], + [{ total: 1, available: 0, busy: 1 }, true], + ])("rejects unnegotiated or invalid reclaimable capacity %j", (slots, idleRetention) => { + expect(parseNodeRunnerInventoryDeclaration(idleDeclaration(slots, idleRetention))).toBeNull(); + }); + + it.each([ + { enabled: false, capacity: { total: 1, available: 1 } }, + { enabled: true }, + { enabled: true, capacity: { total: 1, available: 1 }, bundleStatus: 1 }, + { enabled: true, capacity: { total: 1, available: 1 }, preparedWorkspace: 2 }, + Object.create({ enabled: true, capacity: { total: 1, available: 1 } }), + ])("preserves closed host declarations and capability dependencies %j", (host) => { + expect( + parseNodeRunnerInventoryDeclaration({ + protocolFeatures: [NODE_WORKER_SUPERVISOR_PROTOCOL_FEATURE], + workerHost: host, + }), + ).toBeNull(); + }); +}); diff --git a/src/infra/node-runner-inventory.ts b/src/infra/node-runner-inventory.ts index 724bb0d0a75c..3397d51130c0 100644 --- a/src/infra/node-runner-inventory.ts +++ b/src/infra/node-runner-inventory.ts @@ -77,7 +77,12 @@ const WorkerHost = z.union([ preparedWorkspace: z.literal(NODE_WORKER_PREPARED_WORKSPACE_VERSION).optional(), capturedExecPolicy: z.literal(true).optional(), launchToolNames: LaunchToolNames.optional(), - }).refine((host) => host.bundleStatus === undefined || host.bundleRetention !== undefined), + idleRetention: z.literal(true).optional(), + }).refine( + (host) => + (host.bundleStatus === undefined || host.bundleRetention !== undefined) && + (host.capacity.reclaimableIdle === undefined || host.idleRetention === true), + ), ]); export type NodeWorkerCapacitySnapshot = Readonly>; export type NodeWorkerHostDeclaration = z.infer; diff --git a/src/node-host/connection.test.ts b/src/node-host/connection.test.ts index 052a585d6f99..0300e9093d41 100644 --- a/src/node-host/connection.test.ts +++ b/src/node-host/connection.test.ts @@ -136,13 +136,157 @@ it.each([ ).toBeNull(); }); +it("negotiates reclaimable idle capacity without changing old Gateway inventory", async () => { + const { connection, request, start } = startConnectionFixture(true); + try { + start.mock.calls[0]![0].onRunnerCapacityChanged?.({ + total: 2, + available: 0, + reclaimableIdle: 2, + }); + for (const supported of [false, true, false]) { + connection.connect({ + ...gateway, + capabilities: supported ? [GATEWAY_SERVER_CAPS.NODE_WORKER_IDLE_RETENTION] : [], + }); + const declaration = request.mock.calls.findLast( + ([method]) => method === NODE_RUNNER_INVENTORY_UPDATE_METHOD, + )?.[1]; + expect(declaration).toEqual({ + protocolFeatures: ["node-worker-supervisor-v6"], + workerHost: { + enabled: true, + capacity: { total: 2, available: 0, ...(supported ? { reclaimableIdle: 2 } : {}) }, + ...(supported ? { idleRetention: true } : {}), + bundlePrewarm: 1, + }, + }); + expect(parseNodeRunnerInventoryDeclaration(declaration)).toEqual(declaration); + } + } finally { + await connection.close(); + } +}); + +it.each([false, true])( + "joins idle retirement before fresh reconnect capacity, failure=%s", + async (fails) => { + const { connection, request, start, runtime } = startConnectionFixture(true); + const retirement = createDeferred(); + runtime.cancelAll.mockReturnValueOnce(retirement.promise); + if (fails) { + runtime.cancelAll.mockRejectedValueOnce(new Error("idle retirement retry failed")); + } + const replacement = { request: vi.fn().mockResolvedValue({}) }; + try { + start.mock.calls[0]![0].onRunnerCapacityChanged?.({ + total: 1, + available: 0, + reclaimableIdle: 1, + }); + connection.connect(gateway); + connection.disconnect(); + const callsBefore = request.mock.calls.length; + connection.connect(gateway); + connection.connect(gateway, replacement); + start.mock.calls[0]![0].onRunnerCapacityChanged?.({ + total: 1, + available: 1, + reclaimableIdle: 0, + }); + expect(request).toHaveBeenCalledTimes(callsBefore); + expect(replacement.request).not.toHaveBeenCalled(); + if (fails) { + retirement.reject(new Error("idle retirement failed")); + } else { + retirement.resolve(); + } + await vi.advanceTimersByTimeAsync(0); + expect(runtime.cancelAll).toHaveBeenCalledTimes(fails ? 2 : 1); + expect(request).toHaveBeenCalledTimes(callsBefore); + if (fails) { + expect(replacement.request).not.toHaveBeenCalled(); + } else { + expect(replacement.request).toHaveBeenCalledWith( + NODE_RUNNER_INVENTORY_UPDATE_METHOD, + expect.objectContaining({ + workerHost: expect.objectContaining({ capacity: { total: 1, available: 1 } }), + }), + ); + } + } finally { + retirement.resolve(); + await connection.close(); + } + }, +); + +it.each([false, true])( + "retries rejected disconnect cleanup on reconnect, repeated failure=%s", + async (fails) => { + const { connection, request, start, runtime } = startConnectionFixture(true); + const retirement = createDeferred(); + const retry = createDeferred(); + runtime.cancelAll.mockReturnValueOnce(retirement.promise).mockReturnValueOnce(retry.promise); + const replacement = { request: vi.fn().mockResolvedValue({}) }; + const retired = { request: vi.fn().mockResolvedValue({}) }; + try { + start.mock.calls[0]![0].onRunnerCapacityChanged?.({ + total: 1, + available: 0, + reclaimableIdle: 1, + }); + connection.connect(gateway); + connection.disconnect(); + const rejected = expect(retirement.promise).rejects.toThrow("idle retirement failed"); + retirement.reject(new Error("idle retirement failed")); + await rejected; + const callsBefore = request.mock.calls.length; + connection.connect(gateway, retired); + connection.connect(gateway, replacement); + await vi.advanceTimersByTimeAsync(0); + expect(runtime.cancelAll).toHaveBeenCalledTimes(2); + expect(retired.request).not.toHaveBeenCalled(); + expect(replacement.request).not.toHaveBeenCalled(); + start.mock.calls[0]![0].onRunnerCapacityChanged?.({ + total: 1, + available: 1, + reclaimableIdle: 0, + }); + if (fails) { + retry.reject(new Error("idle retirement retry failed")); + } else { + retry.resolve(); + } + await vi.advanceTimersByTimeAsync(0); + expect(runtime.cancelAll).toHaveBeenCalledTimes(2); + expect(request).toHaveBeenCalledTimes(callsBefore); + expect(retired.request).not.toHaveBeenCalled(); + if (fails) { + expect(replacement.request).not.toHaveBeenCalled(); + } else { + expect(replacement.request).toHaveBeenCalledWith( + NODE_RUNNER_INVENTORY_UPDATE_METHOD, + expect.objectContaining({ + workerHost: expect.objectContaining({ capacity: { total: 1, available: 1 } }), + }), + ); + } + } finally { + retirement.resolve(); + retry.resolve(); + await connection.close(); + } + }, +); + function startConnectionFixture(workerHostingEnabled = false, preparedWorkspacesEnabled = false) { const request = vi.fn().mockResolvedValue({ ok: true, handled: false }); const runtime = { invoke: vi.fn(), handleInput: vi.fn(), cancel: vi.fn(), - cancelAll: vi.fn(), + cancelAll: vi.fn<() => Promise>(async () => undefined), tryPauseForUpdate: vi.fn(async () => true), resumeAfterUpdate: vi.fn(), updateGatewayConnection: vi.fn(), @@ -165,7 +309,7 @@ function startConnectionFixture(workerHostingEnabled = false, preparedWorkspaces writeStderrLine, }); const publications = () => request.mock.calls.filter(([method]) => method === "node.event"); - return { connection, request, publications, writeStderrLine, start, prepared }; + return { connection, request, publications, writeStderrLine, start, prepared, runtime }; } beforeEach(() => { diff --git a/src/node-host/connection.ts b/src/node-host/connection.ts index 653da747d980..ce491a8e79ab 100644 --- a/src/node-host/connection.ts +++ b/src/node-host/connection.ts @@ -94,6 +94,7 @@ export function startNodeHostConnection({ let connectedGatewayProtocol = 0; let gatewayCapabilities: ReadonlySet = new Set(); let hostStatsTimer: NodeJS.Timeout | undefined; + let disconnectCleanup: Promise | undefined; const optionalPublicationStates = new Map< NodeOptionalPublicationMethod, NodeOptionalPublicationState @@ -322,7 +323,16 @@ export function startNodeHostConnection({ workerHost: hostingCapacity ? { enabled: true, - capacity: hostingCapacity, + capacity: { + total: hostingCapacity.total, + available: hostingCapacity.available, + ...(gatewayCapabilities.has(GATEWAY_SERVER_CAPS.NODE_WORKER_IDLE_RETENTION) + ? { reclaimableIdle: hostingCapacity.reclaimableIdle ?? 0 } + : {}), + }, + ...(gatewayCapabilities.has(GATEWAY_SERVER_CAPS.NODE_WORKER_IDLE_RETENTION) + ? { idleRetention: true } + : {}), ...(prepared.preparedWorkspacesEnabled ? { preparedWorkspace: NODE_WORKER_PREPARED_WORKSPACE_VERSION } : {}), @@ -368,7 +378,7 @@ export function startNodeHostConnection({ const disconnect = () => { retireGatewayConnection(); runtime.updateGatewayConnection(); - runtime.cancelAll(); + disconnectCleanup = runtime.cancelAll(); }; const runtime = prepared.start({ client, @@ -403,20 +413,33 @@ export function startNodeHostConnection({ }, connect(connection: NodeHostGatewayConnection, connectionClient: NodeHostClient = client) { retireGatewayConnection(); - publicationClient = connectionClient; - runtime.updateGatewayConnection({ - url: connection.url, - ...(connection.tlsFingerprint ? { tlsFingerprint: connection.tlsFingerprint } : {}), - ...(connection.cloudflareAccess ? { cloudflareAccess: connection.cloudflareAccess } : {}), - }); - gatewayHelloReceived = true; - if (!prepared.restrictedSurface) { - startHostStatsPublication(); + const generation = gatewayConnectionGeneration; + const publish = () => { + if (generation !== gatewayConnectionGeneration) { + return; + } + publicationClient = connectionClient; + runtime.updateGatewayConnection(connection); + gatewayHelloReceived = true; + if (!prepared.restrictedSurface) { + startHostStatsPublication(); + } + connectedGatewayProtocol = connection.protocol; + gatewayCapabilities = new Set(connection.capabilities); + publishRunnerInventory(); + publishInventory(); + }; + if (disconnectCleanup) { + disconnectCleanup = disconnectCleanup.catch((error: unknown) => { + if (generation !== gatewayConnectionGeneration) { + throw error; + } + return runtime.cancelAll(); + }); + void disconnectCleanup.then(publish, () => {}); + } else { + publish(); } - connectedGatewayProtocol = connection.protocol; - gatewayCapabilities = new Set(connection.capabilities); - publishRunnerInventory(); - publishInventory(); }, disconnect, close() { diff --git a/src/node-host/node-worker-capacity.ts b/src/node-host/node-worker-capacity.ts index c0f4f6a46178..458b716f39e4 100644 --- a/src/node-host/node-worker-capacity.ts +++ b/src/node-host/node-worker-capacity.ts @@ -1,4 +1,6 @@ +import { addAbortListener } from "node:events"; import os from "node:os"; +import { toErrorObject } from "../infra/errors.js"; import { NODE_WORKER_CAPACITY_EXHAUSTED_ERROR_CODE } from "../infra/node-commands.js"; import type { NodeWorkerCapacitySnapshot } from "../infra/node-runner-inventory.js"; import { NODE_WORKER_CAPACITY_MAX } from "../shared/node-list-parse.js"; @@ -47,6 +49,7 @@ export class NodeWorkerCapacity { private readonly closeAbort = new AbortController(); private publishedCapacity: NodeWorkerCapacitySnapshot; private initialized = false; + private reclaimableIdle?: number; private updates: Promise = Promise.resolve(); @@ -111,6 +114,7 @@ export class NodeWorkerCapacity { claim: NodeWorkerLaunchClaim, supervisor: NodeWorkerProcessIdentity, signal?: AbortSignal, + reclaimIdle?: () => Promise, ): Promise> { const deadlineMs = Date.now() + this.waitMs; const assertCurrent = () => { @@ -132,6 +136,9 @@ export class NodeWorkerCapacity { if (result.action !== "at-capacity") { return result; } + if (reclaimIdle && (await this.wait(deadlineMs, signal, reclaimIdle))) { + continue; + } await this.wait(deadlineMs, signal); } } @@ -167,6 +174,14 @@ export class NodeWorkerCapacity { this.wake(); } + setReclaimableIdle(count: number): void { + this.reclaimableIdle = count; + this.publishCount(this.capacity - this.publishedCapacity.available); + if (count > 0) { + this.wake(); + } + } + private update(operation: () => Promise): Promise { const pending = this.updates.then(operation); this.updates = pending.then( @@ -178,10 +193,22 @@ export class NodeWorkerCapacity { private publishCount(nonterminalCount: number, force = false): void { const available = Math.max(0, this.capacity - nonterminalCount); - if (!force && this.publishedCapacity.available === available) { + const reclaimableIdle = + this.reclaimableIdle === undefined + ? undefined + : Math.min(this.reclaimableIdle, this.capacity - available); + if ( + !force && + this.publishedCapacity.available === available && + this.publishedCapacity.reclaimableIdle === reclaimableIdle + ) { return; } - this.publishedCapacity = Object.freeze({ total: this.capacity, available }); + this.publishedCapacity = Object.freeze({ + total: this.capacity, + available, + ...(reclaimableIdle === undefined ? {} : { reclaimableIdle }), + }); this.onCapacityChanged?.(this.publishedCapacity); } @@ -211,7 +238,11 @@ export class NodeWorkerCapacity { } } - private async wait(deadlineMs: number, signal?: AbortSignal): Promise { + private async wait( + deadlineMs: number, + signal?: AbortSignal, + reclaimIdle?: () => Promise, + ): Promise { const remainingMs = deadlineMs - Date.now(); if (remainingMs <= 0) { throw new NodeWorkerCapacityExhaustedError(this.waitMs); @@ -222,25 +253,44 @@ export class NodeWorkerCapacity { if (this.closeAbort.signal.aborted) { throw new Error("node worker supervisor is closed"); } - await new Promise((resolve, reject) => { - const finish = (operation: () => void) => { + const waiting = signal + ? AbortSignal.any([signal, this.closeAbort.signal]) + : this.closeAbort.signal; + return new Promise((resolve, reject) => { + const wake = (complete = () => resolve(false)) => { clearTimeout(pollTimer); this.waiters.delete(wake); - signal?.removeEventListener("abort", onAbort); - this.closeAbort.signal.removeEventListener("abort", onClose); - operation(); + listener[Symbol.dispose](); + if (waiting.aborted) { + reject( + this.closeAbort.signal.aborted + ? new Error("node worker supervisor is closed") + : capacityAbortReason(waiting), + ); + } else if (reclaimIdle && Date.now() >= deadlineMs) { + reject(new NodeWorkerCapacityExhaustedError(this.waitMs)); + } else { + complete(); + } }; - const wake = () => finish(resolve); - const onAbort = () => - finish(() => - reject(signal ? capacityAbortReason(signal) : new Error("node worker admission aborted")), - ); - const onClose = () => finish(() => reject(new Error("node worker supervisor is closed"))); - const pollTimer = setTimeout(wake, Math.min(CAPACITY_POLL_MS, remainingMs)); + const pollTimer = setTimeout( + () => wake(), + reclaimIdle ? remainingMs : Math.min(CAPACITY_POLL_MS, remainingMs), + ); pollTimer.unref?.(); - this.waiters.add(wake); - signal?.addEventListener("abort", onAbort, { once: true }); - this.closeAbort.signal.addEventListener("abort", onClose, { once: true }); + const listener = addAbortListener(waiting, () => wake()); + if (reclaimIdle) { + // Bound admission's observation; the supervisor retains physical cleanup custody. + void Promise.resolve() + .then(() => (waiting.aborted ? false : reclaimIdle())) + .then( + (reclaimed) => wake(() => resolve(reclaimed)), + (error: unknown) => + wake(() => reject(toErrorObject(error, "node worker idle reclamation failed"))), + ); + } else { + this.waiters.add(wake); + } }); } } diff --git a/src/node-host/node-worker-journal-worker.ts b/src/node-host/node-worker-journal-worker.ts index 6adb522b8b67..e37b90c31988 100644 --- a/src/node-host/node-worker-journal-worker.ts +++ b/src/node-host/node-worker-journal-worker.ts @@ -26,6 +26,10 @@ export class NodeWorkerJournalWorker { constructor(private readonly options: { env?: NodeJS.ProcessEnv; path?: string }) {} + operation(type: Key) { + return (...input: OpenClawStateWorkerOperations[Key]["input"]) => this.execute({ type, input }); + } + execute( command: { type: Key; diff --git a/src/node-host/node-worker-launch-observation.test.ts b/src/node-host/node-worker-launch-observation.test.ts index 1071a3be82ff..58c8ab4eb185 100644 --- a/src/node-host/node-worker-launch-observation.test.ts +++ b/src/node-host/node-worker-launch-observation.test.ts @@ -4,7 +4,10 @@ import { describe, expect, it, vi } from "vitest"; import { createDeferred } from "../../test/helpers/promise.js"; import { createAwaitedDecodedOutput, onDecodedOutput } from "../process/decoded-output.js"; import type { ProcessExtinctionResult } from "../process/supervisor/types.js"; -import type { WorkerProcessResult } from "../worker/worker-process-protocol.js"; +import type { + WorkerProcessMessage, + WorkerProcessResult, +} from "../worker/worker-process-protocol.js"; import { observeNodeWorkerChild, type NodeWorkerTerminalOutcome, @@ -38,7 +41,7 @@ function observationHarness( cleanupContainer?: () => Promise; expectedKind?: "confirmed" | "deferred"; consumeError?: Error; - onResult?: (frame: WorkerProcessResult) => Promise; + onResult?: (frame: WorkerProcessMessage) => Promise; } = {}, ) { const stdout = new PassThrough(); @@ -81,7 +84,7 @@ function observationHarness( kill, dispose, } satisfies NodeWorkerChildAdapter; - const frames: WorkerProcessResult[] = []; + const frames: WorkerProcessMessage[] = []; const completion = observeNodeWorkerChild( { adapter, @@ -137,6 +140,24 @@ function observationHarness( } describe("node worker output framing", () => { + it("delivers idle readiness without replacing the completed turn result", async () => { + const harness = observationHarness(); + const retained = { ...resultFrame("first"), retainWorker: true, retention: "background" }; + const idle = { type: "idle-ready", turnId: "first" }; + try { + await harness.releaseJournal(); + harness.stdout.write(`${JSON.stringify(retained)}\n${JSON.stringify(idle)}\n`); + expect(await harness.close()).toEqual({ + state: "completed", + resultJson: JSON.stringify(retained.result), + }); + expect(harness.frames).toEqual([retained, idle]); + expect(harness.kill).not.toHaveBeenCalled(); + } finally { + await harness.close(); + } + }); + it("requests stop after consumer failure and joins separately completed child output", async () => { const failure = new Error("synthetic stdout consumer failed"); const harness = observationHarness({ consumeError: failure }); diff --git a/src/node-host/node-worker-launch-observation.ts b/src/node-host/node-worker-launch-observation.ts index 79bed0db2bec..cec49cee71b2 100644 --- a/src/node-host/node-worker-launch-observation.ts +++ b/src/node-host/node-worker-launch-observation.ts @@ -5,8 +5,8 @@ import { finalizeCapturedOutput, } from "../process/exec-output.js"; import { - parseWorkerProcessResult, - type WorkerProcessResult, + parseWorkerProcessMessage, + type WorkerProcessMessage, } from "../worker/worker-process-protocol.js"; import type { NodeWorkerTerminalState } from "./node-worker-launch-store.js"; import type { NodeWorkerChildAdapter } from "./node-worker-launch-transport.js"; @@ -40,7 +40,7 @@ type NodeWorkerChildCompletion = Readonly<{ /** Report cleanup facts without releasing supervisor ownership or capacity. */ export async function observeNodeWorkerChild( active: NodeWorkerChildObservation, - onResult: (frame: WorkerProcessResult) => Promise, + onResult: (frame: WorkerProcessMessage) => Promise, currentTurnId: () => string | undefined, cleanupContainer?: () => Promise, ): Promise { @@ -79,7 +79,7 @@ export async function observeNodeWorkerChild( /** Turn results settle independently; the supervisor retains physical cleanup ownership. */ async function observeNodeWorkerChildOutput( active: NodeWorkerChildObservation, - onResult: (frame: WorkerProcessResult) => Promise, + onResult: (frame: WorkerProcessMessage) => Promise, currentTurnId: () => string | undefined, ): Promise { let stdout = ""; @@ -120,7 +120,7 @@ async function observeNodeWorkerChildOutput( if (outputError || observationEnded) { break; } - const frame = parseWorkerProcessResult( + const frame = parseWorkerProcessMessage( JSON.parse(parseNodeWorkerOutputJson(line.toString("utf8"), active.scrubber.scrub)), ); if (!frame) { @@ -130,7 +130,9 @@ async function observeNodeWorkerChildOutput( if (outputError || observationEnded) { break; } - lastResult = JSON.stringify(frame.result); + if (frame.type === "result") { + lastResult = JSON.stringify(frame.result); + } } } } catch (error) { diff --git a/src/node-host/node-worker-launch-store.ts b/src/node-host/node-worker-launch-store.ts index 290573118349..c2ca30d5e07d 100644 --- a/src/node-host/node-worker-launch-store.ts +++ b/src/node-host/node-worker-launch-store.ts @@ -22,7 +22,23 @@ export type { /** Durable launch operations on the canonical shared-state worker. */ export class NodeWorkerLaunchStore { - constructor(private readonly worker: NodeWorkerJournalWorker) {} + readonly listNonterminal; + readonly get; + readonly nonterminalCount; + readonly pruneExpiredTerminal; + readonly getMatching; + readonly cleanupBinding; + readonly finishCancelled; + + constructor(private readonly worker: NodeWorkerJournalWorker) { + this.get = worker.operation("nodeWorker.launch.get"); + this.listNonterminal = worker.operation("nodeWorker.launch.listNonterminal"); + this.nonterminalCount = worker.operation("nodeWorker.launch.nonterminalCount"); + this.pruneExpiredTerminal = worker.operation("nodeWorker.launch.pruneExpiredTerminal"); + this.getMatching = worker.operation("nodeWorker.launch.getMatching"); + this.cleanupBinding = worker.operation("nodeWorker.launch.cleanupBinding"); + this.finishCancelled = worker.operation("nodeWorker.launch.finishCancelled"); + } claim( claim: NodeWorkerLaunchClaim, @@ -64,48 +80,6 @@ export class NodeWorkerLaunchStore { }, authority); } - listNonterminal( - ...params: Parameters - ): Promise> { - return this.worker.execute({ type: "nodeWorker.launch.listNonterminal", input: params }); - } - - nonterminalCount( - ...params: Parameters - ): Promise> { - return this.worker.execute({ type: "nodeWorker.launch.nonterminalCount", input: params }); - } - - pruneExpiredTerminal( - ...params: Parameters - ): Promise> { - return this.worker.execute({ type: "nodeWorker.launch.pruneExpiredTerminal", input: params }); - } - - get( - ...params: Parameters - ): Promise> { - return this.worker.execute({ type: "nodeWorker.launch.get", input: params }); - } - - getMatching( - ...params: Parameters - ): Promise> { - return this.worker.execute({ type: "nodeWorker.launch.getMatching", input: params }); - } - - cleanupBinding( - ...params: Parameters - ): Promise> { - return this.worker.execute({ type: "nodeWorker.launch.cleanupBinding", input: params }); - } - - finishCancelled( - ...params: Parameters - ): Promise> { - return this.worker.execute({ type: "nodeWorker.launch.finishCancelled", input: params }); - } - finish( params: Parameters[0], authority?: NodeWorkerJournalAuthority, diff --git a/src/node-host/node-worker-launch-transport.ts b/src/node-host/node-worker-launch-transport.ts index 2eaf5618d947..aefb05379fc1 100644 --- a/src/node-host/node-worker-launch-transport.ts +++ b/src/node-host/node-worker-launch-transport.ts @@ -7,9 +7,11 @@ import { import { supportsNodeWorkerProcessOwner } from "../process/supervisor/service-child-protocol.js"; import { createServiceChildRelayAdapter } from "../process/supervisor/service-child-relay-host.js"; import type { WorkerLaunchDescriptor } from "../worker/launch-descriptor.js"; -import { parseNodeWorkerConnectionFailureMessage } from "../worker/node-supervisor-protocol.js"; import { - buildWorkerProcessTurn, + parseNodeWorkerConnectionFailureMessage, + type NodeWorkerLaunchInput, +} from "../worker/node-supervisor-protocol.js"; +import { serializeWorkerProcessInput, type WorkerProcessInput, } from "../worker/worker-process-protocol.js"; @@ -31,7 +33,6 @@ import { type NodeWorkerCredentialScrubber, } from "./node-worker-output.js"; import type { NodeWorkerProcessIdentity } from "./node-worker-process-identity.js"; -import type { NodeWorkerLaunchInput } from "./node-worker-supervisor-contract.js"; export type NodeWorkerChildAdapter = AwaitedStdoutChildAdapter & { confirmExtinction?: () => boolean; @@ -177,25 +178,6 @@ export async function prepareNodeWorkerLaunchTransport( } } -/** Both transports admit turns only after the physical owner has been journaled. */ -export async function startNodeWorkerLaunchTransport(params: { - adapter: NodeWorkerChildAdapter; - descriptor: WorkerLaunchDescriptor; - container?: NodeWorkerContainerIdentity; - isCurrent: () => boolean; -}): Promise { - if (!params.isCurrent()) { - throw new Error("node worker admission closed before startup"); - } - if (!params.container) { - await params.adapter.openStartGate?.(); - } - if (!params.isCurrent()) { - throw new Error("node worker admission closed before descriptor dispatch"); - } - await sendNodeWorkerInput(params.adapter, buildWorkerProcessTurn(params.descriptor)); -} - export async function sendNodeWorkerInput( adapter: NodeWorkerChildAdapter, message: WorkerProcessInput, diff --git a/src/node-host/node-worker-launch.ts b/src/node-host/node-worker-launch.ts deleted file mode 100644 index 46132d0738e4..000000000000 --- a/src/node-host/node-worker-launch.ts +++ /dev/null @@ -1,280 +0,0 @@ -import { registerSecretValueForRedaction } from "../logging/secret-redaction-registry.js"; -import { createDeferredCore } from "../shared/deferred.js"; -import type { WorkerLaunchDescriptor } from "../worker/launch-descriptor.js"; -import type { NodeWorkerCapacity } from "./node-worker-capacity.js"; -import type { NodeWorkerContainerEngine } from "./node-worker-container-engine.js"; -import type { NodeWorkerContainerLifecycle } from "./node-worker-container-lifecycle.js"; -import type { NodeWorkerCleanupMode } from "./node-worker-launch-receipt.js"; -import type { - NodeWorkerLaunchClaim, - NodeWorkerLaunchReceipt, - NodeWorkerLaunchStore, - NodeWorkerContainerIdentity, -} from "./node-worker-launch-store.js"; -import { - prepareNodeWorkerLaunchTransport, - startNodeWorkerLaunchTransport, - type NodeWorkerChildAdapter, -} from "./node-worker-launch-transport.js"; -import { - createNodeWorkerCredentialScrubber, - sanitizeNodeWorkerDiagnostic, -} from "./node-worker-output.js"; -import { - requireNodeWorkerProcessIdentity, - type NodeWorkerProcessIdentity, -} from "./node-worker-process-identity.js"; -import type { NodeWorkerLaunchInput } from "./node-worker-supervisor-contract.js"; -import { - createNodeWorkerActiveTurn, - nodeWorkerEnvironmentBinding, - type NodeWorkerActiveOwnership, - type NodeWorkerRunningChild, - type NodeWorkerStopState, -} from "./node-worker-supervisor-ownership.js"; -import { nodeWorkerDescriptorSecrets } from "./node-worker-turn-lifecycle.js"; -import type { NodeWorkerTurnStore } from "./node-worker-turn-store.js"; - -export const NODE_WORKER_STOP_GRACE_MS = 1_000; - -type NodeWorkerLaunchContext = { - bundleRoot: string; - workerEnv: NodeJS.ProcessEnv; - engineEnv: NodeJS.ProcessEnv; - store: NodeWorkerLaunchStore; - turns: NodeWorkerTurnStore; - capacity: NodeWorkerCapacity; - containerEngine?: NodeWorkerContainerEngine; - containerImage?: string; - containerLifecycle?: NodeWorkerContainerLifecycle; - active: Map; - isClosed: () => boolean; - observeChild: (active: NodeWorkerRunningChild) => Promise; - stopChild: (active: NodeWorkerRunningChild, state?: NodeWorkerStopState) => Promise; -}; - -/** Starts one physical owner behind the durable journal gate, independent of turn reuse. */ -export async function startNodeWorkerChild( - context: NodeWorkerLaunchContext, - params: { - input: NodeWorkerLaunchInput; - descriptor: WorkerLaunchDescriptor; - planHash: string; - supervisor: NodeWorkerProcessIdentity; - claim: NodeWorkerLaunchClaim; - signal?: AbortSignal; - }, -): Promise { - const sensitiveValues = nodeWorkerDescriptorSecrets(params.descriptor); - const scrubber = createNodeWorkerCredentialScrubber(sensitiveValues); - // Turn cancellation can beat the child's admission retry deadline. Retain the - // producer's latest cause so the durable terminal receipt does not become generic. - const connectionFailure: { errorText?: string } = {}; - for (const value of sensitiveValues) { - registerSecretValueForRedaction(value); - } - const finishFailed = (errorText: string) => - context.capacity.finish({ - launchId: params.input.launchId, - planHash: params.planHash, - supervisor: params.supervisor, - worker: null, - state: "failed", - errorText, - }); - let adapter: NodeWorkerChildAdapter; - let container: NodeWorkerContainerIdentity | undefined; - let cleanupMode: NodeWorkerCleanupMode | null; - try { - const prepared = await prepareNodeWorkerLaunchTransport({ - bundleRoot: context.bundleRoot, - workerEnv: context.workerEnv, - engineEnv: context.engineEnv, - input: params.input, - descriptor: params.descriptor, - planHash: params.planHash, - supervisor: params.supervisor, - connectionFailure, - scrubber, - store: context.store, - containerEngine: context.containerEngine, - containerLifecycle: context.containerLifecycle, - containerImage: context.containerImage, - }); - if (prepared.kind === "terminal") { - return prepared.receipt; - } - adapter = prepared.adapter; - container = prepared.container; - cleanupMode = prepared.cleanupMode; - } catch (error) { - return finishFailed( - sanitizeNodeWorkerDiagnostic(error, "node worker spawn failed", scrubber.scrub), - ); - } - if (!adapter.pid) { - if (container) { - await requireNodeWorkerContainerLifecycle(context.containerLifecycle).remove( - container, - params.input, - ); - } - adapter.kill("SIGKILL"); - adapter.dispose(); - return finishFailed("node worker spawn did not return a process id"); - } - let worker: NodeWorkerProcessIdentity; - try { - worker = requireNodeWorkerProcessIdentity(adapter.pid); - } catch (error) { - if (container) { - await requireNodeWorkerContainerLifecycle(context.containerLifecycle).remove( - container, - params.input, - ); - } - adapter.kill("SIGKILL"); - await adapter.wait().catch(() => undefined); - adapter.dispose(); - return finishFailed( - sanitizeNodeWorkerDiagnostic( - error, - "node worker process identity unavailable", - scrubber.scrub, - ), - ); - } - const { promise: journalReady, resolve: releaseJournal } = createDeferredCore(); - const active = { - state: "running", - binding: nodeWorkerEnvironmentBinding(params.input), - turn: createNodeWorkerActiveTurn(params.claim), - retiring: false, - adapter, - journalReady, - gatewayNamespace: params.input.gatewayNamespace, - launchId: params.input.launchId, - planHash: params.planHash, - scrubber, - connectionFailure, - supervisor: params.supervisor, - worker, - ...(container ? { container } : {}), - } as NodeWorkerRunningChild; // SAFETY: done is assigned synchronously below; observation waits on journalReady before publishing state. - active.done = context.observeChild(active); - context.active.set(active.launchId, active); - void active.done.catch(() => undefined); - let running: NodeWorkerLaunchReceipt; - try { - running = await context.store.markRunning({ - launchId: active.launchId, - planHash: active.planHash, - supervisor: params.supervisor, - worker, - cleanupMode, - ...(container ? { container } : {}), - }); - } catch (error) { - releaseJournal(); - if (container) { - await context.stopChild(active, "interrupted"); - context.active.delete(active.launchId); - await finishFailed( - sanitizeNodeWorkerDiagnostic( - error, - "node worker container identity could not be persisted", - scrubber.scrub, - ), - ); - } else { - await context.stopChild(active, "interrupted").catch(() => undefined); - } - throw error; - } - releaseJournal(); - if (running.state === "cancelled" || running.state === "interrupted") { - await context.stopChild(active, running.state); - return (await context.store.get(active.launchId)) ?? running; - } - if (running.state !== "running") { - if (container) { - await context.stopChild(active, "interrupted"); - } else { - adapter.closeStartGate?.(); - } - return running; - } - if (context.isClosed() || params.signal?.aborted || active.turn?.cancelled) { - await context.stopChild(active, context.isClosed() ? "interrupted" : "cancelled"); - return (await context.store.get(active.launchId)) ?? running; - } - try { - await startNodeWorkerLaunchTransport({ - adapter, - descriptor: params.descriptor, - container, - isCurrent: () => - context.active.get(active.launchId) === active && - !context.isClosed() && - !params.signal?.aborted && - active.turn?.cancelled === false, - }); - } catch { - // Only cancellation and shutdown override the child's observed exit. - const stopState = context.isClosed() - ? "interrupted" - : params.signal?.aborted || active.turn?.cancelled - ? "cancelled" - : undefined; - await context.stopChild(active, stopState); - return (await context.store.get(active.launchId)) ?? running; - } - return (await context.turns.get(params.input.launchId)) ?? running; -} - -export function requireNodeWorkerContainerLifecycle( - lifecycle?: NodeWorkerContainerLifecycle, -): NodeWorkerContainerLifecycle { - if (!lifecycle) { - throw new Error("node worker container isolation has no available engine"); - } - return lifecycle; -} - -export async function cleanupNodeWorkerChildContainer( - active: NodeWorkerRunningChild, - lifecycle?: NodeWorkerContainerLifecycle, -): Promise { - if (!active.container) { - return; - } - const cleanup = (active.containerCleanup ??= requireNodeWorkerContainerLifecycle(lifecycle) - .remove(active.container, active) - .finally(() => { - if (active.containerCleanup === cleanup) { - active.containerCleanup = undefined; - } - })); - await cleanup; -} - -export async function stopNodeWorkerChild( - active: NodeWorkerRunningChild, - state: NodeWorkerStopState | undefined, - lifecycle?: NodeWorkerContainerLifecycle, -): Promise { - active.stopState ??= state; - if (active.container) { - // The attach client owns no workload; fence the container and prove its - // removal before its launch can become terminal or release capacity. - await cleanupNodeWorkerChildContainer(active, lifecycle); - } - active.adapter.kill("SIGTERM"); - const forceKill = setTimeout(() => active.adapter.kill("SIGKILL"), NODE_WORKER_STOP_GRACE_MS); - forceKill.unref?.(); - try { - await active.done; - } finally { - clearTimeout(forceKill); - } -} diff --git a/src/node-host/node-worker-prepared-workspace-launch.test.ts b/src/node-host/node-worker-prepared-workspace-launch.test.ts deleted file mode 100644 index 473a516e1102..000000000000 --- a/src/node-host/node-worker-prepared-workspace-launch.test.ts +++ /dev/null @@ -1,87 +0,0 @@ -import { describe, expect, it, vi } from "vitest"; -import { createDeferredCore } from "../shared/deferred.js"; -import type { NodeWorkerLaunchReceipt } from "./node-worker-launch-store.js"; -import { launchWithNodeWorkerPreparedWorkspace } from "./node-worker-supervisor-ownership.js"; - -describe("prepared workspace launch custody", () => { - it.each(["completed", "failed", "closed", "aborted"] as const)( - "settles the acquired lease when launch is %s", - async (outcome) => { - const request = { - workspaceDir: "/synthetic/workspace", - environmentId: "environment", - sessionId: "session", - sessionKey: "agent:main:session", - ownerEpoch: 1, - }; - const receipt: NodeWorkerLaunchReceipt = { - launchId: "launch", - planHash: "plan", - gatewayNamespace: "gateway", - environmentId: request.environmentId, - sessionId: request.sessionId, - ownerEpoch: request.ownerEpoch, - placementGeneration: 1, - runId: "run", - state: "running", - supervisor: { pid: 1, startTime: 1 }, - worker: null, - workerCleanupMode: null, - workerLineageSettled: false, - resultJson: null, - errorText: null, - completedAtMs: null, - createdAtMs: 1, - updatedAtMs: 1, - }; - const release = vi.fn(); - const lease = { workspaceDir: request.workspaceDir, homeDir: "/synthetic/home", release }; - const acquired = createDeferredCore(); - const workspace = { acquirePreparedWorkspace: vi.fn(() => acquired.promise) }; - const controller = new AbortController(); - const failure = new Error("synthetic launch failure"); - let current = true; - const started = createDeferredCore(); - const launched = createDeferredCore(); - const launch = vi.fn((homeDir?: string) => { - expect(homeDir).toBe(lease.homeDir); - expect(release).not.toHaveBeenCalled(); - started.resolve(); - return launched.promise; - }); - const pending = launchWithNodeWorkerPreparedWorkspace({ - workspace, - request, - signal: controller.signal, - isCurrent: () => current, - launch, - }); - expect(workspace.acquirePreparedWorkspace).toHaveBeenCalledExactlyOnceWith(request); - expect(launch).not.toHaveBeenCalled(); - if (outcome === "closed") { - current = false; - } else if (outcome === "aborted") { - controller.abort(failure); - } - acquired.resolve(lease); - if (outcome === "completed" || outcome === "failed") { - await started.promise; - expect(release).not.toHaveBeenCalled(); - if (outcome === "failed") { - launched.reject(failure); - } else { - launched.resolve(receipt); - } - } - if (outcome === "completed") { - await expect(pending).resolves.toBe(receipt); - } else if (outcome === "closed") { - await expect(pending).rejects.toThrow("node worker environment is stopping"); - } else { - await expect(pending).rejects.toBe(failure); - } - expect(launch).toHaveBeenCalledTimes(outcome === "completed" || outcome === "failed" ? 1 : 0); - expect(release).toHaveBeenCalledOnce(); - }, - ); -}); diff --git a/src/node-host/node-worker-supervisor-close.ts b/src/node-host/node-worker-supervisor-close.ts deleted file mode 100644 index 68630516dbbd..000000000000 --- a/src/node-host/node-worker-supervisor-close.ts +++ /dev/null @@ -1,58 +0,0 @@ -import type { NodeWorkerJournalWorker } from "./node-worker-journal-worker.js"; -import type { NodeWorkerLaunchReceipt } from "./node-worker-launch-store.js"; -import type { - NodeWorkerActiveOwnership, - NodeWorkerObservedTerminal, - NodeWorkerPendingAdmission, - NodeWorkerRunningChild, -} from "./node-worker-supervisor-ownership.js"; -import type { NodeWorkerRecovery } from "./node-worker-supervisor-recovery.js"; -import type { NodeWorkerWorkspaceRuntime } from "./node-worker-workspace.js"; - -/** Join accepted work and physical cleanup before sealing the supervisor's journal. */ -export async function settleNodeWorkerSupervisorClose(context: { - workspace: NodeWorkerWorkspaceRuntime; - initialization?: Promise; - admissions: ReadonlyMap; - starting: ReadonlyMap>; - recoveries: ReadonlyMap; - retentions: ReadonlySet>; - active: ReadonlyMap; - journal: NodeWorkerJournalWorker; - stopChild(active: NodeWorkerRunningChild): Promise; - reconcileTerminal(active: NodeWorkerObservedTerminal): Promise; -}): Promise { - const errors: unknown[] = []; - await context.workspace.processes.close().catch((error: unknown) => errors.push(error)); - await context.initialization?.catch((error: unknown) => errors.push(error)); - await Promise.allSettled([...context.admissions.values()].map((admission) => admission.done)); - await Promise.allSettled(context.starting.values()); - await Promise.allSettled(context.retentions); - const stopped = await Promise.allSettled([ - ...[...context.recoveries.values()].map((recovery) => recovery.done), - ...[...context.active.values()] - .filter((active): active is NodeWorkerRunningChild => active.state === "running") - .map((active) => context.stopChild(active)), - ]); - errors.push( - ...stopped.flatMap((result) => (result.status === "rejected" ? [result.reason] : [])), - ); - for (const active of context.active.values()) { - if (active.state !== "observed") { - continue; - } - try { - await context.reconcileTerminal(active); - } catch (error) { - errors.push(error); - } - } - await context.journal - .drain({ close: errors.length === 0 }) - .catch((error: unknown) => errors.push(error)); - if (errors.length > 0) { - throw errors.length === 1 - ? errors[0] - : new AggregateError(errors, "node worker terminal reconciliation failed"); - } -} diff --git a/src/node-host/node-worker-supervisor-commands.ts b/src/node-host/node-worker-supervisor-commands.ts index fe1c0114e9fe..341ee8e18e73 100644 --- a/src/node-host/node-worker-supervisor-commands.ts +++ b/src/node-host/node-worker-supervisor-commands.ts @@ -4,12 +4,11 @@ import { boundedWorkerErrorWithCode } from "../gateway/worker-environments/worke import { NODE_WORKER_BUNDLE_INSTALL_COMMAND, NODE_WORKER_CAPACITY_EXHAUSTED_ERROR_CODE, - NODE_WORKER_DESKTOP_COMPUTER_COMMAND, NODE_WORKER_DESKTOP_LAUNCH_COMMAND, NODE_WORKER_DESKTOP_STREAM_COMMAND, NODE_WORKER_ENVIRONMENT_STOP_COMMAND, NODE_WORKER_PORTAL_STREAM_COMMAND, - NODE_WORKER_PRIVATE_COMMANDS, + NODE_WORKER_SUPERVISOR_CANCEL_COMMAND, NODE_WORKER_SUPERVISOR_LAUNCH_COMMAND, NODE_WORKER_SUPERVISOR_STATUS_COMMAND, NODE_WORKER_WORKSPACE_EXEC_COMMAND, @@ -22,6 +21,13 @@ import { parseNodeWorkerBundleInstallInput, type NodeWorkerBundleInstallResult, } from "../worker/node-bundle-install-protocol.js"; +import { + parseNodeWorkerCancelInput, + parseNodeWorkerEnvironmentStopInput, + parseNodeWorkerLaunchInput, + parseNodeWorkerLookupInput, + type NodeWorkerSupervisorReceipt, +} from "../worker/node-supervisor-protocol.js"; import { parseNodeWorkerPreparedWorkspaceInput, type NodeWorkerPreparedWorkspaceResult, @@ -47,13 +53,8 @@ import { invokeNodeWorkerDesktopStream } from "./desktop-stream-command.js"; import type { NodeWorkerBundleInstallerControl } from "./node-worker-bundle-installer.js"; import { NodeWorkerCapacityExhaustedError } from "./node-worker-capacity.js"; import { - parseNodeWorkerCancelInput, - parseNodeWorkerEnvironmentStopInput, - parseNodeWorkerLaunchInput, - parseNodeWorkerLookupInput, projectNodeWorkerSupervisorReceipt, type NodeWorkerSupervisorControl, - type NodeWorkerSupervisorReceipt, } from "./node-worker-supervisor-contract.js"; import type { NodeWorkerWorkspaceRuntime } from "./node-worker-workspace.js"; import { invokeNodeWorkerPortalStream } from "./portal-stream-command.js"; @@ -141,40 +142,24 @@ export async function invokeNodeWorkerSupervisorCommand(params: { gatewayCloudflareAccess?: CloudflareAccessCredentials; signal?: AbortSignal; }): Promise { - if ( - params.command === NODE_WORKER_DESKTOP_COMPUTER_COMMAND || - !NODE_WORKER_PRIVATE_COMMANDS.some((command) => command === params.command) - ) { - return { handled: false }; - } - const runtime = - params.command === NODE_WORKER_BUNDLE_INSTALL_COMMAND - ? params.bundleInstaller - : params.command === NODE_WORKER_WORKSPACE_EXEC_COMMAND || - params.command === NODE_WORKER_WORKSPACE_PREPARE_COMMAND - ? params.workspace - : params.supervisor; - if (!runtime) { - return { - handled: true, - ok: false, - code: "UNAVAILABLE", - message: "node worker runtime unavailable", - }; - } - try { - let payload: NodeWorkerSupervisorCommandPayload; - if (params.command === NODE_WORKER_WORKSPACE_PREPARE_COMMAND) { - payload = await params.workspace!.prepare( - parseNodeWorkerPreparedWorkspaceInput(params.paramsJSON), - params.signal, - ); - } else if (params.command === NODE_WORKER_BUNDLE_INSTALL_COMMAND) { + const { supervisor, bundleInstaller, workspace, paramsJSON, signal } = params; + const receipt = (value: Awaited>) => + value ? projectNodeWorkerSupervisorReceipt(value) : null; + const commands: Record< + string, + () => Promise | undefined + > = { + [NODE_WORKER_WORKSPACE_PREPARE_COMMAND]: () => + workspace?.prepare(parseNodeWorkerPreparedWorkspaceInput(paramsJSON), signal), + [NODE_WORKER_BUNDLE_INSTALL_COMMAND]: () => { + if (!bundleInstaller) { + return undefined; + } if (!params.gatewayUrl) { throw new Error("node worker gateway connection unavailable"); } - payload = await params.bundleInstaller!.ensure({ - input: parseNodeWorkerBundleInstallInput(params.paramsJSON), + return bundleInstaller.ensure({ + input: parseNodeWorkerBundleInstallInput(paramsJSON), gatewayUrl: params.gatewayUrl, ...(params.gatewayTlsFingerprint ? { gatewayTlsFingerprint: params.gatewayTlsFingerprint } @@ -182,12 +167,13 @@ export async function invokeNodeWorkerSupervisorCommand(params: { ...(params.gatewayCloudflareAccess ? { gatewayCloudflareAccess: params.gatewayCloudflareAccess } : {}), - signal: params.signal, + signal, }); - } else if (params.command === NODE_WORKER_WORKSPACE_EXEC_COMMAND) { - payload = await params.workspace!.exec( - parseNodeWorkerWorkspaceExecInput(params.paramsJSON), - params.signal, + }, + [NODE_WORKER_WORKSPACE_EXEC_COMMAND]: () => + workspace?.exec( + parseNodeWorkerWorkspaceExecInput(paramsJSON), + signal, params.gatewayUrl ? { url: params.gatewayUrl, @@ -199,16 +185,19 @@ export async function invokeNodeWorkerSupervisorCommand(params: { : {}), } : undefined, - ); - } else if (params.command === NODE_WORKER_WORKSPACE_RETAIN_COMMAND) { - const input = parseNodeWorkerWorkspaceRetainInput(params.paramsJSON); - const workspace = await params.supervisor!.retainWorkspaces(input, params.signal); + ), + [NODE_WORKER_WORKSPACE_RETAIN_COMMAND]: async () => { + if (!supervisor) { + return undefined; + } + const input = parseNodeWorkerWorkspaceRetainInput(paramsJSON); + const retained = await supervisor.retainWorkspaces(input, signal); let bundles: { deleted: number; hasMore: boolean; generation: number } | undefined; - if (workspace.applied && input.bundleHashes) { - if (!params.bundleInstaller?.retain) { + if (retained.applied && input.bundleHashes) { + if (!bundleInstaller?.retain) { throw new Error("node worker bundle retention unavailable"); } - bundles = await params.bundleInstaller.retain({ + bundles = await bundleInstaller.retain({ gatewayNamespace: input.gatewayNamespace, bundleHashes: input.bundleHashes, ...(input.acknowledgedBundleGeneration !== undefined @@ -216,71 +205,73 @@ export async function invokeNodeWorkerSupervisorCommand(params: { : {}), }); } - const hasMore = workspace.hasMore || bundles?.hasMore === true; - const inspectBundle = params.bundleInstaller?.inspect?.bind(params.bundleInstaller); - if (workspace.applied && input.bundleStatusHash && !hasMore && !inspectBundle) { + const hasMore = retained.hasMore || bundles?.hasMore === true; + const inspectBundle = bundleInstaller?.inspect?.bind(bundleInstaller); + if (retained.applied && input.bundleStatusHash && !hasMore && !inspectBundle) { throw new Error("node worker bundle status unavailable"); } const bundleStatus = - workspace.applied && input.bundleStatusHash && !hasMore && inspectBundle + retained.applied && input.bundleStatusHash && !hasMore && inspectBundle ? await inspectBundle({ gatewayNamespace: input.gatewayNamespace, bundleHash: input.bundleStatusHash, }) : undefined; - payload = - bundles || bundleStatus - ? { - ...workspace, - ...(bundles - ? { - bundleDeleted: bundles.deleted, - bundleGeneration: bundles.generation, - hasMore, - } - : {}), - ...(bundleStatus ? { bundleStatus } : {}), - } - : workspace; - } else if ( - params.command === NODE_WORKER_DESKTOP_STREAM_COMMAND || - params.command === NODE_WORKER_PORTAL_STREAM_COMMAND - ) { - const stream = - params.command === NODE_WORKER_DESKTOP_STREAM_COMMAND - ? invokeNodeWorkerDesktopStream - : invokeNodeWorkerPortalStream; - await stream(params); - payload = null; - } else if (params.command === NODE_WORKER_DESKTOP_LAUNCH_COMMAND) { - payload = await invokeNodeWorkerDesktopLaunch({ - paramsJSON: params.paramsJSON, - signal: params.signal, - }); - } else if (params.command === NODE_WORKER_ENVIRONMENT_STOP_COMMAND) { - await params.supervisor!.stopEnvironment( - parseNodeWorkerEnvironmentStopInput(params.paramsJSON), - ); - payload = null; - } else if (params.command === NODE_WORKER_SUPERVISOR_STATUS_COMMAND) { - const input = parseNodeWorkerLookupInput(params.paramsJSON); - const receipt = await params.supervisor!.status( - input.launchId, - ...(input.waitMs === undefined ? [] : [{ waitMs: input.waitMs, signal: params.signal }]), - ); - payload = receipt ? projectNodeWorkerSupervisorReceipt(receipt) : null; - } else { - const receipt = - params.command === NODE_WORKER_SUPERVISOR_LAUNCH_COMMAND - ? await params.supervisor!.launch( - parseNodeWorkerLaunchInput(params.paramsJSON), - resolveWorkerConnectionEndpoint(params), - params.signal, - ) - : await params.supervisor!.cancel(parseNodeWorkerCancelInput(params.paramsJSON)); - payload = receipt ? projectNodeWorkerSupervisorReceipt(receipt) : null; - } - return { handled: true, ok: true, payload }; + return bundles || bundleStatus + ? { + ...retained, + ...(bundles + ? { bundleDeleted: bundles.deleted, bundleGeneration: bundles.generation, hasMore } + : {}), + ...(bundleStatus ? { bundleStatus } : {}), + } + : retained; + }, + [NODE_WORKER_DESKTOP_STREAM_COMMAND]: () => + supervisor && invokeNodeWorkerDesktopStream(params).then(() => null), + [NODE_WORKER_PORTAL_STREAM_COMMAND]: () => + supervisor && invokeNodeWorkerPortalStream(params).then(() => null), + [NODE_WORKER_DESKTOP_LAUNCH_COMMAND]: () => + supervisor && invokeNodeWorkerDesktopLaunch({ paramsJSON, signal }), + [NODE_WORKER_ENVIRONMENT_STOP_COMMAND]: () => + supervisor?.stopEnvironment(parseNodeWorkerEnvironmentStopInput(paramsJSON)).then(() => null), + [NODE_WORKER_SUPERVISOR_LAUNCH_COMMAND]: () => + supervisor + ?.launch( + parseNodeWorkerLaunchInput(paramsJSON), + resolveWorkerConnectionEndpoint(params), + signal, + ) + .then(receipt), + [NODE_WORKER_SUPERVISOR_STATUS_COMMAND]: () => { + if (!supervisor) { + return undefined; + } + const input = parseNodeWorkerLookupInput(paramsJSON); + return supervisor + .status( + input.launchId, + ...(input.waitMs === undefined ? [] : [{ waitMs: input.waitMs, signal }]), + ) + .then(receipt); + }, + [NODE_WORKER_SUPERVISOR_CANCEL_COMMAND]: () => + supervisor?.cancel(parseNodeWorkerCancelInput(paramsJSON)).then(receipt), + }; + const invoke = Object.hasOwn(commands, params.command) ? commands[params.command] : undefined; + if (!invoke) { + return { handled: false }; + } + try { + const payload = await invoke(); + return payload === undefined + ? { + handled: true, + ok: false, + code: "UNAVAILABLE", + message: "node worker runtime unavailable", + } + : { handled: true, ok: true, payload }; } catch (error) { const invalid = error instanceof Error && error.message.startsWith("INVALID_REQUEST:"); const bundleInstallFailure = error instanceof NodeWorkerBundleInstallError; diff --git a/src/node-host/node-worker-supervisor-contract.ts b/src/node-host/node-worker-supervisor-contract.ts index 0a74ab381b68..659a9306ec1e 100644 --- a/src/node-host/node-worker-supervisor-contract.ts +++ b/src/node-host/node-worker-supervisor-contract.ts @@ -1,46 +1,14 @@ import { parseNodeWorkerSupervisorReceipt, - type NodeWorkerEnvironmentStopInput, - type NodeWorkerLaunchInput, - type NodeWorkerSupervisorIdentity, type NodeWorkerSupervisorReceipt, } from "../worker/node-supervisor-protocol.js"; -import type { - NodeWorkerWorkspaceRetainInput, - NodeWorkerWorkspaceRetainResult, -} from "../worker/node-workspace-retain-protocol.js"; -import type { WorkerConnectionEndpoint } from "../worker/worker-connection-endpoint.js"; import type { NodeWorkerLaunchReceipt } from "./node-worker-launch-store.js"; +import type { createNodeWorkerSupervisor } from "./node-worker-supervisor.js"; -export { - parseNodeWorkerCancelInput, - parseNodeWorkerEnvironmentStopInput, - parseNodeWorkerLaunchInput, - parseNodeWorkerLookupInput, -} from "../worker/node-supervisor-protocol.js"; -export type { - NodeWorkerLaunchInput, - NodeWorkerSupervisorIdentity, - NodeWorkerSupervisorReceipt, -} from "../worker/node-supervisor-protocol.js"; - -export type NodeWorkerSupervisorControl = { - launch( - input: NodeWorkerLaunchInput, - connectionEndpoint: WorkerConnectionEndpoint, - signal?: AbortSignal, - ): Promise; - status( - launchId: string, - options?: { waitMs: number; signal?: AbortSignal }, - ): Promise; - retainWorkspaces( - input: NodeWorkerWorkspaceRetainInput, - signal?: AbortSignal, - ): Promise; - cancel(expected: NodeWorkerSupervisorIdentity): Promise; - stopEnvironment(input: NodeWorkerEnvironmentStopInput): Promise; -}; +export type NodeWorkerSupervisorControl = Pick< + ReturnType, + "launch" | "status" | "retainWorkspaces" | "cancel" | "stopEnvironment" +>; export function projectNodeWorkerSupervisorReceipt( receipt: NodeWorkerLaunchReceipt, diff --git a/src/node-host/node-worker-supervisor-ownership.ts b/src/node-host/node-worker-supervisor-ownership.ts index 6de8c962b2da..60977b4e9882 100644 --- a/src/node-host/node-worker-supervisor-ownership.ts +++ b/src/node-host/node-worker-supervisor-ownership.ts @@ -1,5 +1,10 @@ import type { NodeWorkerCapacitySnapshot } from "../infra/node-runner-inventory.js"; import { createDeferredCore } from "../shared/deferred.js"; +import type { + NodeWorkerEnvironmentStopInput, + NodeWorkerLaunchInput, + NodeWorkerSupervisorIdentity, +} from "../worker/node-supervisor-protocol.js"; import type { NodeWorkerContainerEngine } from "./node-worker-container-engine.js"; import type { NodeWorkerTerminalOutcome } from "./node-worker-launch-observation.js"; import type { @@ -11,10 +16,6 @@ import type { import type { NodeWorkerChildAdapter } from "./node-worker-launch-transport.js"; import type { NodeWorkerCredentialScrubber } from "./node-worker-output.js"; import type { NodeWorkerProcessIdentity } from "./node-worker-process-identity.js"; -import type { - NodeWorkerLaunchInput, - NodeWorkerSupervisorIdentity, -} from "./node-worker-supervisor-contract.js"; import type { NodeWorkerWorkspaceRuntime } from "./node-worker-workspace.js"; export type NodeWorkerStopState = Extract; @@ -23,8 +24,6 @@ export type NodeWorkerEnvironmentBinding = ReturnType, - expected: Pick< - NodeWorkerEnvironmentBinding, - "gatewayNamespace" | "environmentId" | "sessionId" | "ownerEpoch" - >, + binding: NodeWorkerEnvironmentStopInput, + expected: NodeWorkerEnvironmentStopInput, ): boolean { return ( binding.gatewayNamespace === expected.gatewayNamespace && @@ -80,26 +73,6 @@ type NodeWorkerActiveTurn = { settling?: Promise; }; -/** Retain prepared workspace custody until the admitted launch settles. */ -export async function launchWithNodeWorkerPreparedWorkspace(params: { - workspace: Pick; - request: Parameters[0]; - signal: AbortSignal; - isCurrent: () => boolean; - launch: (homeDir?: string) => Promise; -}): Promise { - const workspace = await params.workspace.acquirePreparedWorkspace(params.request); - try { - params.signal.throwIfAborted(); - if (!params.isCurrent()) { - throw new Error("node worker environment is stopping"); - } - return await params.launch(workspace?.homeDir); - } finally { - workspace?.release(); - } -} - export function createNodeWorkerActiveTurn(claim: NodeWorkerLaunchClaim): NodeWorkerActiveTurn { const { promise, resolve } = createDeferredCore(); return { claim, done: promise, settle: resolve, cancelled: false }; @@ -124,11 +97,22 @@ export type NodeWorkerRunningChild = NodeWorkerActiveBase & { connectionFailure: { errorText?: string }; turn?: NodeWorkerActiveTurn; retiring: boolean; + idleGeneration?: number; + retention?: + | { reason: "background"; turnId: string } + | { reason: "idle"; turnId: string; since: number; timer: NodeJS.Timeout }; stopState?: NodeWorkerStopState; containerCleanup?: Promise; deferredOutcome?: NodeWorkerTerminalOutcome; }; +export function clearNodeWorkerRetention(active: NodeWorkerRunningChild): void { + if (active.retention?.reason === "idle") { + clearTimeout(active.retention.timer); + } + active.retention = undefined; +} + export type NodeWorkerObservedTerminal = NodeWorkerActiveBase & { state: "observed"; outcome: NodeWorkerTerminalOutcome; diff --git a/src/node-host/node-worker-supervisor-recovery.ts b/src/node-host/node-worker-supervisor-recovery.ts index 83fdd7cbdf59..7899255be834 100644 --- a/src/node-host/node-worker-supervisor-recovery.ts +++ b/src/node-host/node-worker-supervisor-recovery.ts @@ -6,8 +6,6 @@ import type { NodeWorkerLaunchReceipt, NodeWorkerLaunchStore } from "./node-work import { inspectNodeWorkerProcessIdentity } from "./node-worker-process-identity.js"; import { nodeWorkerReceiptMatchesOwner, - type NodeWorkerActiveOwnership, - type NodeWorkerObservedTerminal, type NodeWorkerStopState, } from "./node-worker-supervisor-ownership.js"; import { @@ -16,8 +14,6 @@ import { signalOwnedNodeWorkerTree, waitForOwnedNodeWorkerTreeDeath, } from "./node-worker-tree-control.js"; -import { reconcileNodeWorkerTurnCancellation } from "./node-worker-turn-lifecycle.js"; -import type { NodeWorkerTurnStore } from "./node-worker-turn-store.js"; const STOP_GRACE_MS = 1_000; const FORCE_STOP_WAIT_MS = 4_000; @@ -279,43 +275,3 @@ async function recoverNodeWorkerLaunch(params: { } } } - -/** Persist the observed owner outcome before releasing its physical slot. */ -export function reconcileNodeWorkerTerminal( - context: { - active: Map; - turns: NodeWorkerTurnStore; - capacity: NodeWorkerCapacity; - }, - active: NodeWorkerObservedTerminal, -): Promise { - if (active.reconciliation) { - return active.reconciliation; - } - const operation = (async () => { - await reconcileNodeWorkerTurnCancellation(active, context.turns); - const receipt = await context.capacity.finish({ - launchId: active.launchId, - planHash: active.planHash, - supervisor: active.supervisor, - worker: active.worker, - ...active.outcome, - }); - if (receipt.state === "pending" || receipt.state === "running") { - throw new Error(`node worker launch ${active.launchId} terminal state was not persisted`); - } - active.turn?.settle(); - active.turn = undefined; - if (context.active.get(active.launchId) === active) { - context.active.delete(active.launchId); - } - return receipt; - })(); - const pending = operation.finally(() => { - if (active.reconciliation === pending) { - active.reconciliation = undefined; - } - }); - active.reconciliation = pending; - return pending; -} diff --git a/src/node-host/node-worker-supervisor.admission.test.ts b/src/node-host/node-worker-supervisor.admission.test.ts index 0afe05db6b75..4dbfebf8f757 100644 --- a/src/node-host/node-worker-supervisor.admission.test.ts +++ b/src/node-host/node-worker-supervisor.admission.test.ts @@ -9,10 +9,8 @@ import type { NodeWorkerSupervisorTransport } from "../gateway/node-registry-pri import { createNodeWorkerLaunchAdapter } from "../gateway/worker-environments/node-launch-adapter.js"; import { NODE_WORKER_SUPERVISOR_PROTOCOL_FEATURE } from "../infra/node-runner-inventory.js"; import { useStateDatabaseTempDirs } from "../test-utils/state-database-temp-dirs.js"; -import { - parseNodeWorkerLaunchInput, - projectNodeWorkerSupervisorReceipt, -} from "./node-worker-supervisor-contract.js"; +import { parseNodeWorkerLaunchInput } from "../worker/node-supervisor-protocol.js"; +import { projectNodeWorkerSupervisorReceipt } from "./node-worker-supervisor-contract.js"; import { createNodeWorkerSupervisor } from "./node-worker-supervisor.js"; import { TEST_WORKER_ENDPOINT, diff --git a/src/node-host/node-worker-supervisor.fixture.test-support.ts b/src/node-host/node-worker-supervisor.fixture.test-support.ts index 15a0e0aaa9f0..30e715682d37 100644 --- a/src/node-host/node-worker-supervisor.fixture.test-support.ts +++ b/src/node-host/node-worker-supervisor.fixture.test-support.ts @@ -21,6 +21,7 @@ import { const supervisorUrl = resolveRuntimeWorkerUrl(workerBackgroundExecEntrypoints.supervisor); const turnsUrl = resolveRuntimeWorkerUrl(workerBackgroundExecEntrypoints.turnStore); +const journalUrl = resolveRuntimeWorkerUrl(workerBackgroundExecEntrypoints.journalWorker); function writeSupervisorOwnerScript(root: string, waitForCompletedTurn: boolean): string { const scriptPath = path.join(root, "supervisor-owner.mjs"); @@ -29,7 +30,7 @@ function writeSupervisorOwnerScript(root: string, waitForCompletedTurn: boolean) ` import fs from "node:fs"; import { createNodeWorkerSupervisor } from ${JSON.stringify(supervisorUrl.href)}; - import { NodeWorkerTurnStore } from ${JSON.stringify(turnsUrl.href)}; + import { NodeWorkerJournalWorker } from ${JSON.stringify(journalUrl.href)}; const [bundleRoot, stateDir, inputPath] = process.argv.slice(2); const supervisor = createNodeWorkerSupervisor({ bundleRoot, @@ -44,11 +45,11 @@ function writeSupervisorOwnerScript(root: string, waitForCompletedTurn: boolean) const completed = Promise.withResolvers(); void completed.promise.catch(() => undefined); if (${waitForCompletedTurn}) { - const finish = NodeWorkerTurnStore.prototype.finish; - NodeWorkerTurnStore.prototype.finish = function (params) { - const finishing = finish.call(this, params); - if (params.expected.launchId === input.launchId) { - NodeWorkerTurnStore.prototype.finish = finish; + const execute = NodeWorkerJournalWorker.prototype.execute; + NodeWorkerJournalWorker.prototype.execute = function (command, authority) { + const finishing = execute.call(this, command, authority); + if (command.type === "nodeWorker.turn.finish" && command.input[0].expected.launchId === input.launchId) { + NodeWorkerJournalWorker.prototype.execute = execute; void finishing.then(completed.resolve, completed.reject); } return finishing; @@ -73,7 +74,6 @@ export function spawnPendingSupervisorOwner({ claim: NodeWorkerLaunchClaim; }): ChildProcess { const storeUrl = resolveRuntimeWorkerUrl(workerBackgroundExecEntrypoints.launchStore); - const journalUrl = resolveRuntimeWorkerUrl(workerBackgroundExecEntrypoints.journalWorker); const identityUrl = resolveRuntimeWorkerUrl(workerBackgroundExecEntrypoints.processIdentity); const claimPath = path.join(root, "claim.json"); const scriptPath = path.join(root, "pending-owner.mjs"); diff --git a/src/node-host/node-worker-supervisor.idle.test.ts b/src/node-host/node-worker-supervisor.idle.test.ts new file mode 100644 index 000000000000..12725038c1ec --- /dev/null +++ b/src/node-host/node-worker-supervisor.idle.test.ts @@ -0,0 +1,755 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE } from "../../packages/gateway-protocol/src/schema/worker-admission.js"; +import { createDeferred } from "../../test/helpers/promise.js"; +import type { NodeWorkerCapacitySnapshot } from "../infra/node-runner-inventory.js"; +import { nodeWorkerTurnMatchesIdentity } from "../worker/node-supervisor-protocol.js"; +import type { WorkerProcessMessage } from "../worker/worker-process-protocol.js"; +import type { NodeWorkerLaunchReceipt } from "./node-worker-launch-store.js"; +import type { NodeWorkerChildAdapter } from "./node-worker-launch-transport.js"; +import { createNodeWorkerSupervisor, mocks } from "./node-worker-supervisor.mock.test-support.js"; +import { + TEST_WORKER_ENDPOINT, + testNodeWorkerEnvironmentIdentity, + testNodeWorkerLaunchIdentity, + testWorkerLaunchInput, +} from "./node-worker-supervisor.test-support.js"; +import type { NodeWorkerTurnReceipt } from "./node-worker-turn-store.js"; + +beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(0); +}); +afterEach(() => { + vi.useRealTimers(); + vi.resetAllMocks(); +}); + +function input(turnId: string, environmentId = "environment-1") { + const value = testWorkerLaunchInput("/synthetic/workspace", turnId); + value.idleRetention = true; + value.descriptor.admission.environmentId = environmentId; + return value; +} + +function fixture(capacity = 1, container = false) { + const launches = new Map(); + const turns = new Map(); + const snapshots: NodeWorkerCapacitySnapshot[] = []; + const children = new Map>(); + const nonterminal = () => + [...launches.values()].filter((row) => row.state === "pending" || row.state === "running"); + const requireLaunch = (id: string) => { + const row = launches.get(id); + if (!row) { + throw new Error(`Missing synthetic launch ${id}`); + } + return row; + }; + const readTurn = (id: string) => { + const row = turns.get(id); + if (!row) { + return undefined; + } + const owner = requireLaunch(row.ownerLaunchId); + return { + ...row, + worker: owner.worker, + supervisor: owner.supervisor, + ...(owner.container ? { container: owner.container } : {}), + }; + }; + mocks.inspectIdentity.mockReturnValue("live"); + mocks.launchPrune.mockResolvedValue(0); + mocks.launchList.mockImplementation(async () => nonterminal()); + mocks.launchCount.mockImplementation(async () => nonterminal().length); + mocks.launchGet.mockImplementation(async (id) => launches.get(id)); + mocks.launchMatching.mockImplementation(async (expected) => { + const row = launches.get(expected.launchId); + return row && nodeWorkerTurnMatchesIdentity(row, expected) ? row : undefined; + }); + mocks.launchClaim.mockImplementation(async (claim, supervisor, limit, _now, authority) => { + authority?.assertCurrent(); + const count = nonterminal().length; + if (count >= limit) { + return { action: "at-capacity", nonterminalCount: count }; + } + const receipt: NodeWorkerLaunchReceipt = { + ...claim, + supervisor, + worker: null, + workerCleanupMode: null, + workerLineageSettled: false, + state: "pending", + resultJson: null, + errorText: null, + completedAtMs: null, + createdAtMs: Date.now(), + updatedAtMs: Date.now(), + }; + launches.set(claim.launchId, receipt); + return { action: "start", receipt, nonterminalCount: count + 1 }; + }); + mocks.launchRunning.mockImplementation(async (params) => { + const receipt = { + ...requireLaunch(params.launchId), + worker: params.worker, + state: "running" as const, + ...(params.container ? { container: params.container } : {}), + }; + launches.set(params.launchId, receipt); + return receipt; + }); + mocks.launchFinish.mockImplementation(async (params) => { + const receipt = { ...requireLaunch(params.launchId), state: params.state }; + launches.set(params.launchId, receipt); + for (const turn of turns.values()) { + if (turn.ownerLaunchId === params.launchId && turn.state === "running") { + turn.state = params.state === "completed" ? "interrupted" : params.state; + } + } + return receipt; + }); + mocks.turnClaim.mockImplementation(async ({ claim, ownerLaunchId }, authority) => { + authority?.assertCurrent(); + const receipt: NodeWorkerTurnReceipt = { + ...requireLaunch(ownerLaunchId), + ...claim, + ownerLaunchId, + state: "running", + }; + turns.set(claim.launchId, receipt); + return { action: "start", receipt }; + }); + mocks.turnGet.mockImplementation(async (id) => readTurn(id)); + mocks.turnMatching.mockImplementation(async (expected) => { + const row = readTurn(expected.launchId); + return row && nodeWorkerTurnMatchesIdentity(row, expected) ? row : undefined; + }); + mocks.turnFinish.mockImplementation(async (params) => { + const row = readTurn(params.expected.launchId); + if (!row) { + throw new Error("Missing synthetic turn"); + } + const receipt = { ...row, state: params.state, resultJson: params.resultJson ?? null }; + turns.set(row.launchId, receipt); + return receipt; + }); + mocks.drain.mockResolvedValue(undefined); + mocks.acquirePreparedWorkspace.mockResolvedValue(undefined); + mocks.send.mockResolvedValue(undefined); + mocks.remove.mockResolvedValue(undefined); + function child() { + const exited = createDeferred(); + const killed = createDeferred(); + let cleanup: Promise = Promise.resolve(); + let onMessage: ((frame: WorkerProcessMessage) => Promise) | undefined; + const adapter: NodeWorkerChildAdapter = { + pid: 200 + children.size, + supportsRawOutput: true, + onStdout: () => {}, + onStderr: () => {}, + onExit: () => {}, + onError: () => {}, + consumeStdout: async () => {}, + wait: async () => { + await exited.promise; + return { code: 0, signal: null }; + }, + kill: vi.fn(() => { + killed.resolve(); + exited.resolve(); + }), + dispose: vi.fn(), + }; + return { + adapter, + killed, + holdCleanup(promise: Promise) { + cleanup = promise; + }, + observe: ( + active: Parameters[0], + receive: NonNullable, + ): ReturnType => { + onMessage = receive; + return exited.promise.then(async () => { + await cleanup; + return { + kind: "confirmed" as const, + outcome: { state: active.stopState ?? "completed" }, + }; + }); + }, + async emit(frame: WorkerProcessMessage) { + if (!onMessage) { + throw new Error("Child observation has not started"); + } + await onMessage(frame); + }, + complete(turnId: string, retention?: "idle" | "background") { + return this.emit({ + type: "result", + turnId, + result: { status: "completed", transcriptLeafId: "leaf-1", transcriptNextSeq: 2 }, + retainWorker: true, + ...(retention ? { retention } : {}), + }); + }, + }; + } + mocks.prepare.mockImplementation(async ({ input: launchInput }) => { + const owner = child(); + children.set(launchInput.launchId, owner); + return { + kind: "started", + adapter: owner.adapter, + cleanupMode: null, + ...(container + ? { + container: { + engine: "docker" as const, + containerId: String(children.size).padStart(64, "0"), + engineTarget: "b".repeat(64), + }, + } + : {}), + }; + }); + mocks.observe.mockImplementation((active, receive) => { + const owner = [...children.values()].find((value) => value.adapter === active.adapter); + if (!owner) { + throw new Error("Missing synthetic child"); + } + return owner.observe(active, receive); + }); + const supervisor = createNodeWorkerSupervisor({ + bundleRoot: "/synthetic/bundles", + env: { OPENCLAW_STATE_DIR: "/synthetic/state" }, + capacity, + onCapacityChanged: (snapshot) => snapshots.push(snapshot), + ...(container + ? { + containerEngine: { + id: "docker" as const, + command: "synthetic-container", + target: "b".repeat(64), + }, + } + : {}), + }); + return { + supervisor, + snapshots, + launches, + children, + async launch(value: ReturnType) { + const receipt = await supervisor.launch(value, TEST_WORKER_ENDPOINT); + const owner = children.get(value.launchId); + if (!owner) { + throw new Error("Missing newly admitted child"); + } + return { ...owner, receipt }; + }, + }; +} + +describe("node worker idle retention", () => { + it.each(["completed", "failed", "closed", "aborted"] as const)( + "releases prepared workspace custody when public launch is %s", + async (outcome) => { + const f = fixture(); + const lease = { + workspaceDir: "/synthetic/workspace", + homeDir: "/synthetic/home", + release: vi.fn(), + }; + const acquired = createDeferred(); + const started = createDeferred(); + const startup = createDeferred(); + const controller = new AbortController(); + const failure = new Error("Synthetic admission failure"); + let closing: Promise | undefined; + mocks.acquirePreparedWorkspace.mockReturnValueOnce(acquired.promise); + mocks.send.mockImplementationOnce(async () => { + started.resolve(); + await startup.promise; + }); + if (outcome === "failed") { + mocks.launchClaim.mockRejectedValueOnce(failure); + } + const pending = f.supervisor.launch( + input("prepared"), + TEST_WORKER_ENDPOINT, + controller.signal, + ); + const settled = pending.then( + (value) => ({ value }), + (error: unknown) => ({ error }), + ); + try { + expect(mocks.prepare).not.toHaveBeenCalled(); + if (outcome === "closed") { + closing = f.supervisor.close(); + } + if (outcome === "aborted") { + controller.abort(failure); + } + acquired.resolve(lease); + if (outcome === "completed") { + await started.promise; + expect(lease.release).not.toHaveBeenCalled(); + expect(mocks.prepare).toHaveBeenCalledWith( + expect.objectContaining({ + workerEnv: expect.objectContaining({ HOME: lease.homeDir }), + }), + ); + startup.resolve(); + expect(await settled).toMatchObject({ + value: { launchId: "prepared", state: "running" }, + }); + } else if (outcome === "closed") { + expect(await settled).toMatchObject({ + error: { message: "node worker supervisor is closed" }, + }); + } else { + expect(await settled).toEqual({ error: failure }); + } + await closing; + expect(lease.release).toHaveBeenCalledOnce(); + expect(mocks.prepare).toHaveBeenCalledTimes(outcome === "completed" ? 1 : 0); + } finally { + acquired.resolve(lease); + startup.resolve(); + await pending.catch(() => undefined); + await f.supervisor.close(); + } + }, + ); + + it("reuses the physical child with fresh turn authority and clears its old expiry", async () => { + const f = fixture(); + try { + const first = input("first"); + const owner = await f.launch(first); + await owner.complete(first.launchId, "idle"); + expect(f.snapshots.at(-1)).toEqual({ total: 1, available: 0, reclaimableIdle: 1 }); + expect(await f.supervisor.hasActiveWork()).toBe(false); + await vi.advanceTimersByTimeAsync(119_999); + const next = input("second"); + next.descriptor.admission.credential = "fresh-turn-credential"; + next.descriptor.assignment.agentRuntimeIdentityToken = "fresh-runtime-authority"; + next.descriptor.assignment.prompt = "fresh prompt"; + expect(await f.supervisor.launch(next, TEST_WORKER_ENDPOINT)).toMatchObject({ + worker: owner.receipt.worker, + }); + expect(mocks.prepare).toHaveBeenCalledTimes(1); + expect(mocks.send).toHaveBeenCalledWith(owner.adapter, { + type: "turn", + turnId: next.launchId, + idleRetention: true, + descriptor: { ...next.descriptor, connectionEndpoint: TEST_WORKER_ENDPOINT }, + }); + expect(f.snapshots.at(-1)).toEqual({ total: 1, available: 0, reclaimableIdle: 0 }); + expect(await f.supervisor.hasActiveWork()).toBe(true); + await vi.advanceTimersByTimeAsync(1); + expect(owner.adapter.kill).not.toHaveBeenCalled(); + await owner.complete(next.launchId, "idle"); + await vi.advanceTimersByTimeAsync(120_000); + expect(owner.adapter.dispose).toHaveBeenCalledOnce(); + expect(f.snapshots.at(-1)).toEqual({ total: 1, available: 1, reclaimableIdle: 0 }); + } finally { + await f.supervisor.close(); + } + }); + + it("publishes idle only after settlement and starts TTL at background idle-ready", async () => { + const f = fixture(); + const release = createDeferred(); + try { + const value = input("background"); + const owner = await f.launch(value); + const finish = mocks.turnFinish.getMockImplementation()!; + const entered = createDeferred(); + mocks.turnFinish.mockImplementationOnce(async (params) => { + entered.resolve(); + await release.promise; + return finish(params); + }); + const completing = owner.complete(value.launchId, "background"); + await entered.promise; + expect(await f.supervisor.hasActiveWork()).toBe(true); + expect(f.snapshots.at(-1)?.reclaimableIdle ?? 0).toBe(0); + release.resolve(); + await completing; + await vi.advanceTimersByTimeAsync(240_000); + expect(owner.adapter.kill).not.toHaveBeenCalled(); + expect(await f.supervisor.hasActiveWork()).toBe(true); + await owner.emit({ type: "idle-ready", turnId: "stale-turn" }); + expect(f.snapshots.at(-1)?.reclaimableIdle ?? 0).toBe(0); + await owner.emit({ type: "idle-ready", turnId: value.launchId }); + expect(await f.supervisor.hasActiveWork()).toBe(false); + await vi.advanceTimersByTimeAsync(119_999); + expect(owner.adapter.kill).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1); + expect(owner.adapter.dispose).toHaveBeenCalledOnce(); + } finally { + release.resolve(); + await f.supervisor.close(); + } + }); + + it("joins idle cleanup before capacity-one admission, including an expiry race", async () => { + const f = fixture(); + const cleanup = createDeferred(); + try { + const old = input("old", "old-environment"); + const owner = await f.launch(old); + await owner.complete(old.launchId, "idle"); + await vi.advanceTimersByTimeAsync(119_999); + owner.holdCleanup(cleanup.promise); + const replacement = f.supervisor.launch( + input("replacement", "new-environment"), + TEST_WORKER_ENDPOINT, + ); + await owner.killed.promise; + expect(mocks.prepare).toHaveBeenCalledTimes(1); + expect(f.snapshots.at(-1)).toEqual({ total: 1, available: 0, reclaimableIdle: 0 }); + await vi.advanceTimersByTimeAsync(9_999); + expect(owner.adapter.kill).toHaveBeenCalledTimes(2); + expect(mocks.prepare).toHaveBeenCalledTimes(1); + cleanup.resolve(); + expect(await replacement).toMatchObject({ launchId: "replacement", state: "running" }); + expect(owner.adapter.dispose).toHaveBeenCalledOnce(); + expect(mocks.prepare).toHaveBeenCalledTimes(2); + } finally { + cleanup.resolve(); + await f.supervisor.close(); + } + }); + + it.each(["deadline", "abort", "close"] as const)( + "bounds admission by %s while retaining stalled idle cleanup", + async (boundary) => { + const f = fixture(); + const cleanup = createDeferred(); + const controller = new AbortController(); + const failure = new Error("Caller cancelled capacity admission"); + const outcomes: unknown[] = []; + let admitted: Promise | undefined; + let closing: Promise | undefined; + try { + const owner = await f.launch(input("old", "old-environment")); + await owner.complete("old", "idle"); + owner.holdCleanup(cleanup.promise); + admitted = f.supervisor + .launch(input("replacement", "new-environment"), TEST_WORKER_ENDPOINT, controller.signal) + .then( + (value) => { + outcomes.push({ value }); + }, + (error: unknown) => { + outcomes.push({ error }); + }, + ); + await owner.killed.promise; + if (boundary === "deadline") { + await vi.advanceTimersByTimeAsync(9_999); + expect(outcomes).toEqual([]); + await vi.advanceTimersByTimeAsync(1); + } else if (boundary === "abort") { + controller.abort(failure); + } else { + closing = f.supervisor.close(); + } + await vi.advanceTimersByTimeAsync(0); + expect(outcomes).toEqual([ + { + error: + boundary === "abort" + ? failure + : expect.objectContaining({ + message: + boundary === "deadline" + ? "node worker capacity remained full for 10000 ms" + : "node worker supervisor is closed", + }), + }, + ]); + expect(mocks.prepare).toHaveBeenCalledTimes(1); + expect(f.launches.get("old")?.state).toBe("running"); + expect(f.launches.has("replacement")).toBe(false); + expect(f.snapshots.at(-1)).toEqual({ total: 1, available: 0, reclaimableIdle: 0 }); + let closed = false; + closing = (closing ?? f.supervisor.close()).then(() => { + closed = true; + }); + await vi.advanceTimersByTimeAsync(0); + expect(closed).toBe(false); + cleanup.resolve(); + await closing; + expect(f.snapshots.at(-1)).toEqual({ total: 1, available: 1, reclaimableIdle: 0 }); + expect(mocks.prepare).toHaveBeenCalledTimes(1); + expect(vi.getTimerCount()).toBe(0); + } finally { + cleanup.resolve(); + await admitted; + await (closing ?? f.supervisor.close()); + } + }, + ); + + it.each([10_000, 10_001])( + "rejects idle reclamation completed at %i ms before its timeout callback runs", + async (elapsedMs) => { + const f = fixture(); + const cleanup = createDeferred(); + try { + const owner = await f.launch(input("old", "old-environment")); + await owner.complete("old", "idle"); + owner.holdCleanup(cleanup.promise); + const outcome = f.supervisor + .launch(input("replacement", "new-environment"), TEST_WORKER_ENDPOINT) + .then( + (value) => ({ value }), + (error: unknown) => ({ error }), + ); + await owner.killed.promise; + vi.setSystemTime(elapsedMs); + cleanup.resolve(); + expect(await outcome).toEqual({ + error: expect.objectContaining({ + name: "NodeWorkerCapacityExhaustedError", + message: "node worker capacity remained full for 10000 ms", + }), + }); + expect(owner.adapter.kill).toHaveBeenCalledOnce(); + expect(owner.adapter.dispose).toHaveBeenCalledOnce(); + expect(f.launches.get("old")?.state).toBe("interrupted"); + expect(f.launches.has("replacement")).toBe(false); + expect(mocks.prepare).toHaveBeenCalledTimes(1); + expect(f.snapshots.at(-1)).toEqual({ total: 1, available: 1, reclaimableIdle: 0 }); + expect(vi.getTimerCount()).toBe(0); + } finally { + cleanup.resolve(); + await f.supervisor.close(); + } + }, + ); + + it("rejects reuse when idle expiry revokes the owner during turn admission", async () => { + const f = fixture(); + const release = createDeferred(); + try { + const owner = await f.launch(input("first")); + await owner.complete("first", "idle"); + mocks.send.mockClear(); + await vi.advanceTimersByTimeAsync(119_999); + const claim = mocks.turnClaim.getMockImplementation()!; + const entered = createDeferred(); + mocks.turnClaim.mockImplementationOnce(async (params, authority) => { + entered.resolve(); + await release.promise; + return claim(params, authority); + }); + const rejection = expect( + f.supervisor.launch(input("expired-reuse"), TEST_WORKER_ENDPOINT), + ).rejects.toThrow("lost its physical owner before admission"); + await entered.promise; + await vi.advanceTimersByTimeAsync(1); + expect(owner.adapter.dispose).toHaveBeenCalledOnce(); + release.resolve(); + await rejection; + expect(mocks.send).not.toHaveBeenCalled(); + expect(await f.supervisor.status("expired-reuse")).toBeUndefined(); + expect(f.snapshots.at(-1)).toEqual({ total: 1, available: 1, reclaimableIdle: 0 }); + } finally { + release.resolve(); + await f.supervisor.close(); + } + }); + + it("retains at most two idle children and reclaims the least recently used, never background", async () => { + const f = fixture(4); + try { + const background = await f.launch(input("background", "background-env")); + await background.complete("background", "background"); + const a = await f.launch(input("a", "a-env")); + await a.complete("a", "idle"); + await vi.advanceTimersByTimeAsync(1); + const b = await f.launch(input("b", "b-env")); + await b.complete("b", "idle"); + await vi.advanceTimersByTimeAsync(1); + await f.supervisor.launch(input("a-next", "a-env"), TEST_WORKER_ENDPOINT); + await a.complete("a-next", "idle"); + await vi.advanceTimersByTimeAsync(1); + const c = await f.launch(input("c", "c-env")); + await c.complete("c", "idle"); + await vi.advanceTimersByTimeAsync(0); + expect(b.adapter.dispose).toHaveBeenCalledOnce(); + expect(a.adapter.kill).not.toHaveBeenCalled(); + expect(c.adapter.kill).not.toHaveBeenCalled(); + expect(background.adapter.kill).not.toHaveBeenCalled(); + expect(f.snapshots.at(-1)).toEqual({ total: 4, available: 1, reclaimableIdle: 2 }); + } finally { + await f.supervisor.close(); + } + }); + + it.each(["stop", "disconnect", "close"] as const)( + "%s retires idle through physical cleanup and preserves the completed turn", + async (operation) => { + const f = fixture(); + try { + const value = input("completed"); + const owner = await f.launch(value); + await owner.complete(value.launchId, "idle"); + const identity = testNodeWorkerEnvironmentIdentity(value); + await f.supervisor.stopEnvironment({ ...identity, ownerEpoch: identity.ownerEpoch - 1 }); + expect(owner.adapter.kill).not.toHaveBeenCalled(); + if (operation === "stop") { + await f.supervisor.stopEnvironment(identity); + } else if (operation === "disconnect") { + await f.supervisor.retireIdle(); + } else { + await f.supervisor.close(); + } + expect(owner.adapter.dispose).toHaveBeenCalledOnce(); + expect(await f.supervisor.status(value.launchId)).toMatchObject({ state: "completed" }); + expect(await f.supervisor.hasActiveWork()).toBe(false); + } finally { + await f.supervisor.close(); + } + }, + ); + + it("disconnect invalidates pending idle acknowledgments without evicting background work", async () => { + const f = fixture(2); + try { + const current = await f.launch(input("current", "current-env")); + const background = await f.launch(input("background", "background-env")); + await background.complete("background", "background"); + await f.supervisor.retireIdle(); + expect(background.adapter.kill).not.toHaveBeenCalled(); + expect(current.adapter.kill).not.toHaveBeenCalled(); + await current.complete("current", "idle"); + await background.emit({ type: "idle-ready", turnId: "background" }); + await vi.advanceTimersByTimeAsync(0); + expect(current.adapter.dispose).toHaveBeenCalledOnce(); + expect(background.adapter.dispose).toHaveBeenCalledOnce(); + } finally { + await f.supervisor.close(); + } + }); + + it.each(["explicit", "expiry", "limit"] as const)( + "retries failed %s idle cleanup without freeing its slot or evicting background work", + async (trigger) => { + const capacity = trigger === "limit" ? 4 : 2; + const f = fixture(capacity, true); + try { + const background = await f.launch(input("background", "background-env")); + await background.complete("background", "background"); + const idle = await f.launch(input("idle", "idle-env")); + await idle.complete("idle", "idle"); + mocks.remove.mockRejectedValueOnce(new Error("Synthetic container removal failure")); + + if (trigger === "explicit") { + await expect(f.supervisor.retireIdle()).rejects.toThrow( + "node worker idle cleanup failed", + ); + } else if (trigger === "expiry") { + await vi.advanceTimersByTimeAsync(120_000); + } else { + for (const turnId of ["second-idle", "third-idle"]) { + const child = await f.launch(input(turnId, turnId)); + await child.complete(turnId, "idle"); + } + await vi.advanceTimersByTimeAsync(0); + } + expect(f.snapshots.at(-1)).toEqual({ + total: capacity, + available: 0, + reclaimableIdle: capacity - 2, + }); + expect(f.launches.get("idle")).toMatchObject({ + state: "running", + container: idle.receipt.container, + }); + expect(idle.adapter.kill).not.toHaveBeenCalled(); + expect(background.adapter.kill).not.toHaveBeenCalled(); + expect(mocks.remove).toHaveBeenCalledExactlyOnceWith( + idle.receipt.container, + expect.objectContaining({ launchId: "idle" }), + ); + + if (trigger === "explicit") { + await f.supervisor.retireIdle(); + } else { + await vi.advanceTimersByTimeAsync(120_000); + } + expect( + mocks.remove.mock.calls.filter(([, owner]) => owner.launchId === "idle"), + ).toHaveLength(2); + expect(idle.adapter.dispose).toHaveBeenCalledOnce(); + expect(background.adapter.kill).not.toHaveBeenCalled(); + expect(f.snapshots.at(-1)).toEqual({ + total: capacity, + available: capacity - 1, + reclaimableIdle: 0, + }); + expect(await f.supervisor.status("idle")).toMatchObject({ state: "completed" }); + } finally { + await f.supervisor.close(); + } + }, + ); + + it.each(["old-gateway", "old-bundle"] as const)( + "keeps %s retention background-only and preserves the old managed turn shape", + async (compatibility) => { + const f = fixture(); + try { + const value = input("legacy"); + if (compatibility === "old-gateway") { + delete value.idleRetention; + } else { + value.descriptor.admission.handshake.protocolFeatures = + value.descriptor.admission.handshake.protocolFeatures.filter( + (feature) => feature !== NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE, + ); + } + const owner = await f.launch(value); + expect(mocks.send).toHaveBeenCalledWith(owner.adapter, { + type: "turn", + turnId: value.launchId, + descriptor: { ...value.descriptor, connectionEndpoint: TEST_WORKER_ENDPOINT }, + }); + await owner.complete(value.launchId); + await vi.advanceTimersByTimeAsync(240_000); + expect(owner.adapter.kill).not.toHaveBeenCalled(); + expect(await f.supervisor.hasActiveWork()).toBe(true); + expect(f.snapshots.every((snapshot) => !Object.hasOwn(snapshot, "reclaimableIdle"))).toBe( + true, + ); + const next = { + ...value, + launchId: "legacy-next", + descriptor: structuredClone(value.descriptor), + }; + next.descriptor.assignment.turnId = next.launchId; + await f.supervisor.launch(next, TEST_WORKER_ENDPOINT); + expect(mocks.send).toHaveBeenCalledWith(owner.adapter, { + type: "turn", + turnId: next.launchId, + descriptor: { ...next.descriptor, connectionEndpoint: TEST_WORKER_ENDPOINT }, + }); + expect( + await f.supervisor.cancel({ ...testNodeWorkerLaunchIdentity(value), ownerEpoch: 0 }), + ).toBeUndefined(); + expect(owner.adapter.kill).not.toHaveBeenCalled(); + } finally { + await f.supervisor.close(); + } + }, + ); +}); diff --git a/src/node-host/node-worker-supervisor.lifetime.test.ts b/src/node-host/node-worker-supervisor.lifetime.test.ts index 8f51cdc205fb..c5386b251b7f 100644 --- a/src/node-host/node-worker-supervisor.lifetime.test.ts +++ b/src/node-host/node-worker-supervisor.lifetime.test.ts @@ -656,11 +656,16 @@ describe("node worker environment lifetime", () => { expect((await store.get(first.launchId))?.state).toBe("running"); expect(inspectNodeWorkerProcessIdentity(owner.worker!)).toBe("live"); - const readOwner = vi.spyOn(NodeWorkerLaunchStore.prototype, "get"); + const readOwner = vi.spyOn(NodeWorkerJournalWorker.prototype, "execute"); admission = supervisor.launch(next, TEST_WORKER_ENDPOINT).catch((error: unknown) => { admissionError = error; }); - await vi.waitFor(() => expect(readOwner).toHaveBeenCalledWith(first.launchId)); + await vi.waitFor(() => + expect(readOwner).toHaveBeenCalledWith({ + type: "nodeWorker.launch.get", + input: [first.launchId], + }), + ); readOwner.mockRestore(); if (cleanupError) { vi.spyOn( diff --git a/src/node-host/node-worker-supervisor.mock.test-support.ts b/src/node-host/node-worker-supervisor.mock.test-support.ts new file mode 100644 index 000000000000..a37868a333b6 --- /dev/null +++ b/src/node-host/node-worker-supervisor.mock.test-support.ts @@ -0,0 +1,126 @@ +import { vi } from "vitest"; +import type { NodeWorkerJournalWorker as JournalWorker } from "./node-worker-journal-worker.js"; +import type { NodeWorkerLaunchStore as LaunchStore } from "./node-worker-launch-store.js"; +import type { NodeWorkerTurnStore } from "./node-worker-turn-store.js"; + +const mocks = vi.hoisted(() => ({ + launchClaim: vi.fn(), + launchGet: vi.fn(), + launchMatching: vi.fn(), + launchList: vi.fn(), + launchCount: vi.fn(), + launchPrune: vi.fn(), + launchRunning: vi.fn(), + launchFinish: vi.fn(), + launchCancelled: vi.fn(), + turnClaim: vi.fn(), + turnGet: vi.fn(), + turnMatching: vi.fn(), + turnFinish: vi.fn(), + drain: vi.fn(async () => {}), + acquirePreparedWorkspace: + vi.fn< + typeof import("./node-worker-workspace.js").NodeWorkerWorkspaceRuntime.prototype.acquirePreparedWorkspace + >(), + retain: + vi.fn< + typeof import("./node-worker-workspace.js").NodeWorkerWorkspaceRuntime.prototype.applyRetainSnapshot + >(), + inspectIdentity: + vi.fn(), + inspectTree: vi.fn(), + remove: + vi.fn< + typeof import("./node-worker-container-lifecycle.js").NodeWorkerContainerLifecycle.prototype.remove + >(), + observe: vi.fn(), + prepare: + vi.fn(), + send: vi.fn(), +})); + +vi.mock("./node-worker-journal-worker.js", () => ({ + NodeWorkerJournalWorker: class { + drain = mocks.drain; + }, +})); +vi.mock("./node-worker-launch-store.js", () => ({ + NodeWorkerLaunchStore: class { + claim = mocks.launchClaim; + get = mocks.launchGet; + getMatching = mocks.launchMatching; + listNonterminal = mocks.launchList; + nonterminalCount = mocks.launchCount; + pruneExpiredTerminal = mocks.launchPrune; + markRunning = mocks.launchRunning; + finish = mocks.launchFinish; + finishCancelled = mocks.launchCancelled; + }, +})); +vi.mock("./node-worker-turn-store.js", () => ({ + NodeWorkerTurnStore: class { + claim = mocks.turnClaim; + get = mocks.turnGet; + getMatching = mocks.turnMatching; + finish = mocks.turnFinish; + }, +})); +vi.mock("./node-worker-container-lifecycle.js", () => ({ + NodeWorkerContainerLifecycle: class { + initialize = async () => {}; + inspect = async () => "live"; + remove = mocks.remove; + }, +})); +vi.mock("./node-worker-workspace.js", () => ({ + NodeWorkerWorkspaceRuntime: class { + acquirePreparedWorkspace = mocks.acquirePreparedWorkspace; + applyRetainSnapshot = mocks.retain; + processes = { + hasActiveWork: () => false, + stopEnvironment: async () => {}, + close: async () => {}, + }; + }, +})); +vi.mock("./node-worker-process-identity.js", () => ({ + requireNodeWorkerProcessIdentity: (pid: number) => ({ pid, startTime: 1 }), + inspectNodeWorkerProcessIdentity: mocks.inspectIdentity, +})); +vi.mock("./node-worker-tree-control.js", () => { + const unexpected = () => { + throw new Error("Process-tree control is outside this pure fixture"); + }; + return { + inspectOwnedNodeWorkerTree: mocks.inspectTree, + signalOwnedNodeWorkerTree: unexpected, + signalOwnedNodeWorkerAnchor: unexpected, + stopOwnedNodeWorkerTree: unexpected, + waitForOwnedNodeWorkerTreeDeath: unexpected, + }; +}); +vi.mock("./node-worker-launch-observation.js", () => ({ + observeNodeWorkerChild: mocks.observe, +})); +vi.mock("./node-worker-launch-transport.js", () => ({ + prepareNodeWorkerLaunchTransport: mocks.prepare, + sendNodeWorkerInput: mocks.send, +})); + +// Re-exports evaluate dependencies before Vitest installs these mocks. +const { NodeWorkerCapacity } = await import("./node-worker-capacity.js"); +const { NodeWorkerContainerLifecycle } = await import("./node-worker-container-lifecycle.js"); +const { NodeWorkerJournalWorker } = await import("./node-worker-journal-worker.js"); +const { NodeWorkerLaunchStore } = await import("./node-worker-launch-store.js"); +const { createNodeWorkerLaunchRecovery } = await import("./node-worker-supervisor-recovery.js"); +const { createNodeWorkerSupervisor } = await import("./node-worker-supervisor.js"); + +export { + NodeWorkerCapacity, + NodeWorkerContainerLifecycle, + NodeWorkerJournalWorker, + NodeWorkerLaunchStore, + createNodeWorkerLaunchRecovery, + createNodeWorkerSupervisor, + mocks, +}; diff --git a/src/node-host/node-worker-supervisor.recovery.test.ts b/src/node-host/node-worker-supervisor.recovery.test.ts index ced1ce443070..dcbe801761c3 100644 --- a/src/node-host/node-worker-supervisor.recovery.test.ts +++ b/src/node-host/node-worker-supervisor.recovery.test.ts @@ -587,10 +587,12 @@ describe("node worker supervisor recovery", () => { onCapacityChanged: (capacity) => capacitySnapshots.push(capacity), }); const reconciliation = vi - .spyOn(NodeWorkerLaunchStore.prototype, "listNonterminal") - .mockImplementationOnce(async () => { - throw new Error("temporary launch journal failure"); - }); + .spyOn(NodeWorkerJournalWorker.prototype, "execute") + .mockRejectedValueOnce(new Error("temporary launch journal failure")); + const attempts = () => + reconciliation.mock.calls.filter( + ([command]) => command.type === "nodeWorker.launch.listNonterminal", + ).length; try { const first = supervisor.initialize(); @@ -599,14 +601,14 @@ describe("node worker supervisor recovery", () => { expect(concurrent).toBe(first); await expect(first).rejects.toThrow("temporary launch journal failure"); await expect(supervisor.initialize()).resolves.toBeUndefined(); - expect(reconciliation).toHaveBeenCalledTimes(2); + expect(attempts()).toBe(2); expect(capacitySnapshots).toEqual([ { total: 2, available: 0 }, { total: 2, available: 0 }, { total: 2, available: 2 }, ]); await expect(supervisor.initialize()).resolves.toBeUndefined(); - expect(reconciliation).toHaveBeenCalledTimes(2); + expect(attempts()).toBe(2); } finally { reconciliation.mockRestore(); await supervisor.close().catch(() => undefined); diff --git a/src/node-host/node-worker-supervisor.settlement.test-support.ts b/src/node-host/node-worker-supervisor.settlement.test-support.ts index de973c6234e9..a789c3eeb00e 100644 --- a/src/node-host/node-worker-supervisor.settlement.test-support.ts +++ b/src/node-host/node-worker-supervisor.settlement.test-support.ts @@ -3,125 +3,23 @@ import { WORKER_PUBLIC_INGRESS_PATH } from "../../packages/gateway-protocol/src/ import { createDeferred } from "../../test/helpers/promise.js"; import { toErrorObject } from "../infra/errors.js"; import { nodeWorkerTurnMatchesIdentity } from "../worker/node-supervisor-protocol.js"; -import { NodeWorkerCapacity } from "./node-worker-capacity.js"; -import { NodeWorkerContainerLifecycle } from "./node-worker-container-lifecycle.js"; -import { NodeWorkerJournalWorker } from "./node-worker-journal-worker.js"; -import { NodeWorkerLaunchStore, type NodeWorkerLaunchReceipt } from "./node-worker-launch-store.js"; +import type { NodeWorkerLaunchReceipt } from "./node-worker-launch-store.js"; import type { NodeWorkerChildAdapter } from "./node-worker-launch-transport.js"; import type { NodeWorkerRunningChild } from "./node-worker-supervisor-ownership.js"; -import { createNodeWorkerLaunchRecovery } from "./node-worker-supervisor-recovery.js"; -import { createNodeWorkerSupervisor } from "./node-worker-supervisor.js"; +import { + NodeWorkerCapacity, + NodeWorkerContainerLifecycle, + NodeWorkerJournalWorker, + NodeWorkerLaunchStore, + createNodeWorkerLaunchRecovery, + createNodeWorkerSupervisor, + mocks, +} from "./node-worker-supervisor.mock.test-support.js"; import { testNodeWorkerLaunchIdentity, testWorkerLaunchInput, } from "./node-worker-supervisor.test-support.js"; -import type { NodeWorkerTurnReceipt, NodeWorkerTurnStore } from "./node-worker-turn-store.js"; - -const mocks = vi.hoisted(() => ({ - launchClaim: vi.fn(), - launchGet: vi.fn(), - launchMatching: vi.fn(), - launchList: vi.fn(), - launchCount: vi.fn(), - launchPrune: vi.fn(), - launchRunning: vi.fn(), - launchFinish: vi.fn(), - launchCancelled: vi.fn(), - turnClaim: vi.fn(), - turnGet: vi.fn(), - turnMatching: vi.fn(), - turnFinish: vi.fn(), - drain: vi.fn(async () => {}), - retain: - vi.fn< - typeof import("./node-worker-workspace.js").NodeWorkerWorkspaceRuntime.prototype.applyRetainSnapshot - >(), - inspectIdentity: - vi.fn(), - inspectTree: vi.fn(), - remove: - vi.fn< - typeof import("./node-worker-container-lifecycle.js").NodeWorkerContainerLifecycle.prototype.remove - >(), - observe: vi.fn(), - prepare: - vi.fn(), - start: vi.fn(), - send: vi.fn(), -})); - -export function settlementMocks() { - return mocks; -} - -vi.mock("./node-worker-journal-worker.js", () => ({ - NodeWorkerJournalWorker: class { - drain = mocks.drain; - }, -})); -vi.mock("./node-worker-launch-store.js", () => ({ - NodeWorkerLaunchStore: class { - claim = mocks.launchClaim; - get = mocks.launchGet; - getMatching = mocks.launchMatching; - listNonterminal = mocks.launchList; - nonterminalCount = mocks.launchCount; - pruneExpiredTerminal = mocks.launchPrune; - markRunning = mocks.launchRunning; - finish = mocks.launchFinish; - finishCancelled = mocks.launchCancelled; - }, -})); -vi.mock("./node-worker-turn-store.js", () => ({ - NodeWorkerTurnStore: class { - claim = mocks.turnClaim; - get = mocks.turnGet; - getMatching = mocks.turnMatching; - finish = mocks.turnFinish; - }, -})); -vi.mock("./node-worker-container-lifecycle.js", () => ({ - NodeWorkerContainerLifecycle: class { - initialize = async () => {}; - inspect = async () => "live"; - remove = mocks.remove; - }, -})); -vi.mock("./node-worker-workspace.js", () => ({ - NodeWorkerWorkspaceRuntime: class { - acquirePreparedWorkspace = () => undefined; - applyRetainSnapshot = mocks.retain; - processes = { - hasActiveWork: () => false, - stopEnvironment: async () => {}, - close: async () => {}, - }; - }, -})); -vi.mock("./node-worker-process-identity.js", () => ({ - requireNodeWorkerProcessIdentity: () => ({ pid: 101, startTime: 1 }), - inspectNodeWorkerProcessIdentity: mocks.inspectIdentity, -})); -vi.mock("./node-worker-tree-control.js", () => { - const unexpected = () => { - throw new Error("Process-tree control is outside this pure fixture"); - }; - return { - inspectOwnedNodeWorkerTree: mocks.inspectTree, - signalOwnedNodeWorkerTree: unexpected, - signalOwnedNodeWorkerAnchor: unexpected, - stopOwnedNodeWorkerTree: unexpected, - waitForOwnedNodeWorkerTreeDeath: unexpected, - }; -}); -vi.mock("./node-worker-launch-observation.js", () => ({ - observeNodeWorkerChild: mocks.observe, -})); -vi.mock("./node-worker-launch-transport.js", () => ({ - prepareNodeWorkerLaunchTransport: mocks.prepare, - startNodeWorkerLaunchTransport: mocks.start, - sendNodeWorkerInput: mocks.send, -})); +import type { NodeWorkerTurnReceipt } from "./node-worker-turn-store.js"; export async function fixture( unknownOutcome = false, @@ -291,8 +189,9 @@ export async function fixture( cleanupMode: null, container: { engine: "docker", containerId: "a".repeat(64), engineTarget: "b".repeat(64) }, }); - mocks.start.mockResolvedValue(undefined); - mocks.send.mockRejectedValue(new Error("Synthetic child input is closed")); + mocks.send + .mockRejectedValue(new Error("Synthetic child input is closed")) + .mockResolvedValueOnce(undefined); const { observeNodeWorkerChild } = await vi.importActual< typeof import("./node-worker-launch-observation.js") >("./node-worker-launch-observation.js"); @@ -319,10 +218,15 @@ export async function fixture( containerEngine: { id: "docker", command: "synthetic-container", target: "b".repeat(64) }, onCapacityChanged: (snapshot) => snapshots.push(snapshot.available), }); - const launching = supervisor.launch(input, { - kind: "websocket", - url: `wss://gateway.example.invalid${WORKER_PUBLIC_INGRESS_PATH}`, - }); + const launching = supervisor + .launch(input, { + kind: "websocket", + url: `wss://gateway.example.invalid${WORKER_PUBLIC_INGRESS_PATH}`, + }) + .then((receipt) => { + mocks.send.mockClear(); + return receipt; + }); if (!admission) { await launching; } @@ -392,7 +296,10 @@ export function recoveryFixture(container = false) { mocks.launchGet.mockImplementation(async () => receipt); mocks.launchMatching.mockImplementation(async () => receipt); mocks.launchCount.mockImplementation(async () => (receipt.state === "running" ? 1 : 0)); - const finish: NodeWorkerLaunchStore["finish"] = async (params, authority) => { + const finish: InstanceType["finish"] = async ( + params, + authority, + ) => { authority?.assertCurrent(); receipt = { ...receipt, state: params.state }; return receipt; diff --git a/src/node-host/node-worker-supervisor.settlement.test.ts b/src/node-host/node-worker-supervisor.settlement.test.ts index b342c2ebeeb1..9c542b1c8a53 100644 --- a/src/node-host/node-worker-supervisor.settlement.test.ts +++ b/src/node-host/node-worker-supervisor.settlement.test.ts @@ -3,21 +3,14 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { createDeferred } from "../../test/helpers/promise.js"; import { SqliteWorkerError } from "../infra/sqlite-worker-contract.js"; import type { NodeWorkerLaunchReceipt } from "./node-worker-launch-store.js"; -import { - fixture, - recoveryFixture, - settlementMocks, -} from "./node-worker-supervisor.settlement.test-support.js"; +import { createNodeWorkerSupervisor, mocks } from "./node-worker-supervisor.mock.test-support.js"; +import { fixture, recoveryFixture } from "./node-worker-supervisor.settlement.test-support.js"; import { TEST_WORKER_ENDPOINT, testNodeWorkerLaunchIdentity, testWorkerLaunchInput, } from "./node-worker-supervisor.test-support.js"; -// Load after the fixture registers its journal and process mocks. -const { createNodeWorkerSupervisor } = await import("./node-worker-supervisor.js"); -const mocks = settlementMocks(); - afterEach(() => vi.resetAllMocks()); it("joins accepted workspace retention before sealing journals on close", async () => { diff --git a/src/node-host/node-worker-supervisor.test-support.ts b/src/node-host/node-worker-supervisor.test-support.ts index 402c423466a8..a067dfba89ba 100644 --- a/src/node-host/node-worker-supervisor.test-support.ts +++ b/src/node-host/node-worker-supervisor.test-support.ts @@ -5,12 +5,12 @@ import { WORKER_RPC_SET_VERSION, } from "../../packages/gateway-protocol/src/schema/worker-admission.js"; import type { WorkerLaunchPlan } from "../worker/launch-descriptor.js"; -import { nodeWorkerPlanHash } from "../worker/node-supervisor-protocol.js"; +import { + nodeWorkerPlanHash, + type NodeWorkerLaunchInput, + type NodeWorkerSupervisorIdentity, +} from "../worker/node-supervisor-protocol.js"; import type { WorkerConnectionEndpoint } from "../worker/worker-connection-endpoint.js"; -import type { - NodeWorkerLaunchInput, - NodeWorkerSupervisorIdentity, -} from "./node-worker-supervisor-contract.js"; const TEST_BUNDLE_HASH = "a".repeat(64); export const TEST_WORKER_CREDENTIAL = 'node worker/"credential\\secret?'; diff --git a/src/node-host/node-worker-supervisor.test.ts b/src/node-host/node-worker-supervisor.test.ts index 17938b821cf8..64dfe2d20650 100644 --- a/src/node-host/node-worker-supervisor.test.ts +++ b/src/node-host/node-worker-supervisor.test.ts @@ -803,6 +803,9 @@ describe("node worker supervisor", () => { const input = launchInput(workspaceDir, "cancel-rejected-terminal", "tree-cancel-reject"); const retryStarted = createDeferred(); const releaseRetry = createDeferred(); + const journalCalls = retryJournal + ? vi.spyOn(NodeWorkerJournalWorker.prototype, "execute") + : undefined; let cancellation: ReturnType | undefined; try { const running = await supervisor.launch(input, TEST_WORKER_ENDPOINT); @@ -814,17 +817,31 @@ describe("node worker supervisor", () => { Number(fs.readFileSync(grandchildPath, "utf8")), ); - if (retryJournal) { - const finish = vi.spyOn(NodeWorkerTurnStore.prototype, "finish"); - finish - .mockImplementationOnce(async () => { - throw new Error("injected cancellation journal failure"); - }) - .mockImplementation(async function (this: NodeWorkerTurnStore, params) { + if (journalCalls) { + const claim = journalCalls.mock.calls.findIndex( + ([command]) => command.type === "nodeWorker.turn.claim", + ); + const journal = journalCalls.mock.contexts[claim]; + journalCalls.mockRestore(); + if (!(journal instanceof NodeWorkerJournalWorker)) { + throw new Error("Missing admitted worker journal"); + } + const execute = journal.execute.bind(journal); + let firstFinish = true; + const finish = vi + .spyOn(journal, "execute") + .mockImplementation(async (command, authority) => { + if (command.type !== "nodeWorker.turn.finish") { + return execute(command, authority); + } + if (firstFinish) { + firstFinish = false; + throw new Error("injected cancellation journal failure"); + } retryStarted.resolve(); await releaseRetry.promise; finish.mockRestore(); - return this.finish(params); + return journal.execute(command, authority); }); } cancellation = supervisor.cancel(testNodeWorkerLaunchIdentity(input)); diff --git a/src/node-host/node-worker-supervisor.ts b/src/node-host/node-worker-supervisor.ts index f29828b6a5e1..be8f055edf5f 100644 --- a/src/node-host/node-worker-supervisor.ts +++ b/src/node-host/node-worker-supervisor.ts @@ -1,11 +1,18 @@ +import { addAbortListener } from "node:events"; import path from "node:path"; +import { NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE } from "../../packages/gateway-protocol/src/schema/worker-admission.js"; import { resolveStateDir } from "../config/paths.js"; +import { racePromiseWithAbortSignal } from "../infra/abort-signal.js"; +import { withTimeout } from "../infra/fs-safe.js"; +import { registerSecretValueForRedaction } from "../logging/secret-redaction-registry.js"; +import { createDeferredCore } from "../shared/deferred.js"; import { completeWorkerLaunchDescriptor, type WorkerLaunchDescriptor, } from "../worker/launch-descriptor.js"; import { nodeWorkerPlanHash, + nodeWorkerTurnMatchesIdentity, validateNodeWorkerLaunchInput, type NodeWorkerEnvironmentStopInput, type NodeWorkerLaunchInput, @@ -16,6 +23,10 @@ import type { NodeWorkerWorkspaceRetainResult, } from "../worker/node-workspace-retain-protocol.js"; import type { WorkerConnectionEndpoint } from "../worker/worker-connection-endpoint.js"; +import { + buildWorkerProcessTurn, + type WorkerProcessMessage, +} from "../worker/worker-process-protocol.js"; import { NodeWorkerCapacity } from "./node-worker-capacity.js"; import type { NodeWorkerContainerEngine } from "./node-worker-container-engine.js"; import { NodeWorkerContainerLifecycle } from "./node-worker-container-lifecycle.js"; @@ -26,23 +37,30 @@ import { observeNodeWorkerChild, type NodeWorkerTerminalOutcome, } from "./node-worker-launch-observation.js"; -import { NodeWorkerLaunchStore, type NodeWorkerLaunchReceipt } from "./node-worker-launch-store.js"; +import type { NodeWorkerCleanupMode } from "./node-worker-launch-receipt.js"; import { - cleanupNodeWorkerChildContainer, - NODE_WORKER_STOP_GRACE_MS, - requireNodeWorkerContainerLifecycle, - startNodeWorkerChild, - stopNodeWorkerChild, -} from "./node-worker-launch.js"; + NodeWorkerLaunchStore, + type NodeWorkerLaunchReceipt, + type NodeWorkerContainerIdentity, +} from "./node-worker-launch-store.js"; +import { + prepareNodeWorkerLaunchTransport, + sendNodeWorkerInput, + type NodeWorkerChildAdapter, +} from "./node-worker-launch-transport.js"; +import { + createNodeWorkerCredentialScrubber, + sanitizeNodeWorkerDiagnostic, +} from "./node-worker-output.js"; import { inspectNodeWorkerProcessIdentity, requireNodeWorkerProcessIdentity, type NodeWorkerProcessIdentity, } from "./node-worker-process-identity.js"; -import { settleNodeWorkerSupervisorClose } from "./node-worker-supervisor-close.js"; import { + clearNodeWorkerRetention, createNodeWorkerObservedTerminal, - launchWithNodeWorkerPreparedWorkspace, + createNodeWorkerActiveTurn, nodeWorkerEnvironmentBinding, nodeWorkerEnvironmentKey, nodeWorkerEnvironmentMatches, @@ -56,19 +74,14 @@ import { } from "./node-worker-supervisor-ownership.js"; import { createNodeWorkerLaunchRecovery, - reconcileNodeWorkerTerminal, type NodeWorkerRecovery, } from "./node-worker-supervisor-recovery.js"; import { stopOwnedNodeWorkerTree } from "./node-worker-tree-control.js"; -import { - createNodeWorkerTurnControl, - settleNodeWorkerTurn, - startNodeWorkerTurn, - waitForNodeWorkerRetirement, -} from "./node-worker-turn-lifecycle.js"; +import { nodeWorkerDescriptorSecrets } from "./node-worker-turn-lifecycle.js"; import { NodeWorkerTurnStore, type NodeWorkerTurnReceipt } from "./node-worker-turn-store.js"; import { NodeWorkerWorkspaceRuntime } from "./node-worker-workspace.js"; +const NODE_WORKER_STOP_GRACE_MS = 1_000; const FORCE_STOP_WAIT_MS = 4_000; /** Owns worker process groups, lifetime gates, and the durable node-host launch journal. */ @@ -77,7 +90,6 @@ class NodeWorkerSupervisor { private readonly starting = new Map>(); private readonly recoveries = new Map(); private readonly recoverRunning: ReturnType; - private readonly turnControl: ReturnType; private readonly bundleRoot: string; private readonly journal: NodeWorkerJournalWorker; private readonly store: NodeWorkerLaunchStore; @@ -97,6 +109,7 @@ class NodeWorkerSupervisor { private closed = false; private closeCompleted = false; private closePromise?: Promise; + private idleGeneration = 0; constructor(options: NodeWorkerSupervisorOptions = {}) { const env = options.env ?? process.env; @@ -124,18 +137,6 @@ class NodeWorkerSupervisor { recoveries: this.recoveries, isRecoveryActive: () => !this.closed, }); - this.turnControl = createNodeWorkerTurnControl({ - admissions: this.admissions, - active: this.active, - turns: this.turns, - launches: this.store, - stopTimeoutMs: NODE_WORKER_STOP_GRACE_MS + FORCE_STOP_WAIT_MS, - isClosed: () => this.closeCompleted, - initialize: () => this.initialize(), - readStatus: (launchId) => this.readStatus(launchId), - cancelOwner: (identity) => this.cancelOwner(identity), - stopChild: (active, state) => this.stopChild(active, state), - }); } initialize(): Promise { @@ -159,22 +160,68 @@ class NodeWorkerSupervisor { } async hasActiveWork(): Promise { - // Retained workers can own background commands after their turn completes; - // durable claims also cover work owned by another live supervisor. const hasLocalWork = () => !this.capacity.isInitialized() || this.admissions.size > 0 || this.starting.size > 0 || this.recoveries.size > 0 || this.retentions.size > 0 || - this.active.size > 0 || + this.active.size > this.idleChildren().length || this.stoppingEnvironments.size > 0 || this.workspace.processes.hasActiveWork(); if (hasLocalWork()) { return true; } const count = await this.store.nonterminalCount(); - return count > 0 || hasLocalWork(); + return count > this.idleChildren().length || hasLocalWork(); + } + + private idleChildren(): NodeWorkerRunningChild[] { + return [...this.active.values()] + .filter( + (owner): owner is NodeWorkerRunningChild => + owner.state === "running" && + !owner.turn && + !owner.retiring && + !owner.stopState && + owner.retention?.reason === "idle", + ) + .toSorted( + (a, b) => + (a.retention?.reason === "idle" ? a.retention.since : 0) - + (b.retention?.reason === "idle" ? b.retention.since : 0), + ); + } + + private publishIdle(): void { + this.capacity.setReclaimableIdle(this.idleChildren().length); + } + + private async reclaimIdle(): Promise { + const oldest = this.idleChildren()[0]; + if (!oldest) { + return false; + } + await this.stopChild(oldest, "interrupted"); + return !this.active.has(oldest.launchId); + } + + async retireIdle(): Promise { + this.idleGeneration++; + await Promise.allSettled([...this.admissions.values()].map((admission) => admission.done)); + const results = await Promise.allSettled( + [...this.active.values()].flatMap((owner) => + owner.state === "running" && owner.retention?.reason === "idle" + ? [this.stopChild(owner, "interrupted")] + : [], + ), + ); + const errors = results.flatMap((result) => + result.status === "rejected" ? [result.reason] : [], + ); + if (errors.length) { + throw new AggregateError(errors, "node worker idle cleanup failed"); + } } async launch( @@ -204,25 +251,45 @@ class NodeWorkerSupervisor { } const admission = this.admissions.get(key); if (admission) { - if (admission.launchId !== input.launchId || admission.planHash !== claimInput.planHash) { + const { launchId, planHash } = admission.identity; + if (launchId !== input.launchId || planHash !== claimInput.planHash) { throw new Error("node worker environment already has a turn being admitted"); } return await admission.done; } const abort = new AbortController(); + const idleGeneration = + input.idleRetention && + descriptor.admission.handshake.protocolFeatures.includes( + NODE_WORKER_IDLE_RETENTION_PROTOCOL_FEATURE, + ) + ? this.idleGeneration + : undefined; const admissionSignal = signal ? AbortSignal.any([signal, abort.signal]) : abort.signal; - const done = launchWithNodeWorkerPreparedWorkspace({ - workspace: this.workspace, - request: { ...binding, sessionKey: input.sessionKey }, - signal: admissionSignal, - isCurrent: () => !this.closed && !this.stoppingEnvironments.has(key), - launch: (homeDir) => - this.launchAdmitted(input, descriptor, claimInput, admissionSignal, homeDir), - }); + const done = (async () => { + const workspace = await this.workspace.acquirePreparedWorkspace({ + ...binding, + sessionKey: input.sessionKey, + }); + try { + admissionSignal.throwIfAborted(); + if (this.closed || this.stoppingEnvironments.has(key)) { + throw new Error("node worker environment is stopping"); + } + return await this.launchAdmitted( + input, + descriptor, + claimInput, + admissionSignal, + workspace?.homeDir, + idleGeneration, + ); + } finally { + workspace?.release(); + } + })(); const pending = { binding, - launchId: input.launchId, - planHash: claimInput.planHash, identity: claimInput, abort, signal: admissionSignal, @@ -244,6 +311,7 @@ class NodeWorkerSupervisor { claimInput: NodeWorkerLaunchClaim, signal: AbortSignal, homeDir?: string, + idleGeneration?: number, ): Promise { await this.initialize(); const supervisor = (this.supervisorIdentity ??= requireNodeWorkerProcessIdentity(process.pid)); @@ -272,7 +340,17 @@ class NodeWorkerSupervisor { continue; } await this.statusOwner(owner.launchId); - await waitForNodeWorkerRetirement(owner, signal); + signal.throwIfAborted(); + if (owner.retiring) { + // Shutdown must abort admission before stopping its retiring physical owner. + const aborted = createDeferredCore(); + const listener = addAbortListener(signal, () => aborted.resolve()); + try { + await Promise.race([owner.done, aborted.promise]); + } finally { + listener[Symbol.dispose](); + } + } signal.throwIfAborted(); if (this.active.get(owner.launchId) !== owner) { continue; @@ -298,18 +376,11 @@ class NodeWorkerSupervisor { signal.throwIfAborted(); continue; } - return await startNodeWorkerTurn({ - active: owner, - descriptor, - claim: claimInput, - signal, - store: this.turns, - cancel: (expected) => this.turnControl.cancelTurn(expected), - stopChild: (active, state) => this.stopChild(active, state), - isCurrent: () => this.active.get(owner.launchId) === owner && !this.closed, - }); + return await this.startTurn(owner, descriptor, claimInput, signal, idleGeneration); } - const claim = await this.capacity.claim(claimInput, supervisor, signal); + const claim = await this.capacity.claim(claimInput, supervisor, signal, () => + this.reclaimIdle(), + ); if (claim.action === "recover") { await this.recoverRunning(claim.receipt); } @@ -337,35 +408,20 @@ class NodeWorkerSupervisor { } let cancellation: Promise | undefined; const cancelClaimed = () => { - cancellation ??= Promise.resolve().then(() => this.turnControl.cancelTurn(claimInput)); + cancellation ??= Promise.resolve().then(() => this.cancelTurn(claimInput)); void cancellation.catch(() => undefined); }; signal?.addEventListener("abort", cancelClaimed, { once: true }); - const startup = startNodeWorkerChild( - { - bundleRoot: this.bundleRoot, - workerEnv: homeDir ? snapshotNodeWorkerEnv(this.workerEnv, homeDir) : this.workerEnv, - engineEnv: this.engineEnv, - store: this.store, - turns: this.turns, - capacity: this.capacity, - containerEngine: this.containerEngine, - containerImage: this.containerImage, - containerLifecycle: this.containerLifecycle, - active: this.active, - isClosed: () => this.closed, - observeChild: (active) => this.observeChild(active), - stopChild: (active, state) => this.stopChild(active, state), - }, - { - input, - descriptor, - planHash: claimInput.planHash, - supervisor, - signal, - claim: claimInput, - }, - ); + const startup = this.startChild({ + workerEnv: homeDir ? snapshotNodeWorkerEnv(this.workerEnv, homeDir) : this.workerEnv, + input, + descriptor, + planHash: claimInput.planHash, + supervisor, + signal, + claim: claimInput, + idleGeneration, + }); this.starting.set(input.launchId, startup); if (signal?.aborted) { cancelClaimed(); @@ -381,8 +437,298 @@ class NodeWorkerSupervisor { } } - status(launchId: string, options?: { waitMs: number; signal?: AbortSignal }) { - return this.turnControl.status(launchId, options); + /** Starts one physical owner behind the durable journal gate, independent of turn reuse. */ + private async startChild(params: { + workerEnv: NodeJS.ProcessEnv; + input: NodeWorkerLaunchInput; + descriptor: WorkerLaunchDescriptor; + planHash: string; + supervisor: NodeWorkerProcessIdentity; + claim: NodeWorkerLaunchClaim; + signal?: AbortSignal; + idleGeneration?: number; + }): Promise { + const sensitiveValues = nodeWorkerDescriptorSecrets(params.descriptor); + const scrubber = createNodeWorkerCredentialScrubber(sensitiveValues); + // Turn cancellation can beat the child's admission retry deadline. Retain the + // producer's latest cause so the durable terminal receipt does not become generic. + const connectionFailure: { errorText?: string } = {}; + for (const value of sensitiveValues) { + registerSecretValueForRedaction(value); + } + const finishFailed = (errorText: string) => + this.capacity.finish({ + launchId: params.input.launchId, + planHash: params.planHash, + supervisor: params.supervisor, + worker: null, + state: "failed", + errorText, + }); + let adapter: NodeWorkerChildAdapter; + let container: NodeWorkerContainerIdentity | undefined; + let cleanupMode: NodeWorkerCleanupMode | null; + try { + const prepared = await prepareNodeWorkerLaunchTransport({ + bundleRoot: this.bundleRoot, + workerEnv: params.workerEnv, + engineEnv: this.engineEnv, + input: params.input, + descriptor: params.descriptor, + planHash: params.planHash, + supervisor: params.supervisor, + connectionFailure, + scrubber, + store: this.store, + containerEngine: this.containerEngine, + containerLifecycle: this.containerLifecycle, + containerImage: this.containerImage, + }); + if (prepared.kind === "terminal") { + return prepared.receipt; + } + adapter = prepared.adapter; + container = prepared.container; + cleanupMode = prepared.cleanupMode; + } catch (error) { + return finishFailed( + sanitizeNodeWorkerDiagnostic(error, "node worker spawn failed", scrubber.scrub), + ); + } + if (!adapter.pid) { + if (container) { + await this.requireContainerLifecycle().remove(container, params.input); + } + adapter.kill("SIGKILL"); + adapter.dispose(); + return finishFailed("node worker spawn did not return a process id"); + } + let worker: NodeWorkerProcessIdentity; + try { + worker = requireNodeWorkerProcessIdentity(adapter.pid); + } catch (error) { + if (container) { + await this.requireContainerLifecycle().remove(container, params.input); + } + adapter.kill("SIGKILL"); + await adapter.wait().catch(() => undefined); + adapter.dispose(); + return finishFailed( + sanitizeNodeWorkerDiagnostic( + error, + "node worker process identity unavailable", + scrubber.scrub, + ), + ); + } + const { promise: journalReady, resolve: releaseJournal } = createDeferredCore(); + const active = { + state: "running", + binding: nodeWorkerEnvironmentBinding(params.input), + turn: createNodeWorkerActiveTurn(params.claim), + retiring: false, + idleGeneration: params.idleGeneration, + adapter, + journalReady, + gatewayNamespace: params.input.gatewayNamespace, + launchId: params.input.launchId, + planHash: params.planHash, + scrubber, + connectionFailure, + supervisor: params.supervisor, + worker, + ...(container ? { container } : {}), + } as NodeWorkerRunningChild; // SAFETY: done is assigned synchronously below; observation waits on journalReady before publishing state. + active.done = this.observeChild(active); + this.active.set(active.launchId, active); + void active.done.catch(() => undefined); + let running: NodeWorkerLaunchReceipt; + try { + running = await this.store.markRunning({ + launchId: active.launchId, + planHash: active.planHash, + supervisor: params.supervisor, + worker, + cleanupMode, + ...(container ? { container } : {}), + }); + } catch (error) { + releaseJournal(); + if (container) { + await this.stopChild(active, "interrupted"); + this.active.delete(active.launchId); + await finishFailed( + sanitizeNodeWorkerDiagnostic( + error, + "node worker container identity could not be persisted", + scrubber.scrub, + ), + ); + } else { + await this.stopChild(active, "interrupted").catch(() => undefined); + } + throw error; + } + releaseJournal(); + if (running.state === "cancelled" || running.state === "interrupted") { + await this.stopChild(active, running.state); + return (await this.store.get(active.launchId)) ?? running; + } + if (running.state !== "running") { + if (container) { + await this.stopChild(active, "interrupted"); + } else { + adapter.closeStartGate?.(); + } + return running; + } + if (this.closed || params.signal?.aborted || active.turn?.cancelled) { + await this.stopChild(active, this.closed ? "interrupted" : "cancelled"); + return (await this.store.get(active.launchId)) ?? running; + } + try { + const isCurrent = () => + this.active.get(active.launchId) === active && + !this.closed && + !params.signal?.aborted && + active.turn?.cancelled === false; + if (!isCurrent()) { + throw new Error("node worker admission closed before startup"); + } + if (!container) { + await adapter.openStartGate?.(); + } + if (!isCurrent()) { + throw new Error("node worker admission closed before descriptor dispatch"); + } + await sendNodeWorkerInput( + adapter, + buildWorkerProcessTurn(params.descriptor, params.idleGeneration !== undefined), + ); + } catch { + // Only cancellation and shutdown override the child's observed exit. + const stopState = this.closed + ? "interrupted" + : params.signal?.aborted || active.turn?.cancelled + ? "cancelled" + : undefined; + await this.stopChild(active, stopState); + return (await this.store.get(active.launchId)) ?? running; + } + return (await this.turns.get(params.input.launchId)) ?? running; + } + + private async startTurn( + active: NodeWorkerRunningChild, + descriptor: WorkerLaunchDescriptor, + claim: NodeWorkerLaunchClaim, + signal: AbortSignal, + idleGeneration?: number, + ): Promise { + const isCurrent = () => this.active.get(active.launchId) === active && !this.closed; + const assertCurrent = () => { + signal.throwIfAborted(); + if (!isCurrent() || active.stopState || active.retiring || active.turn) { + throw new Error("node worker turn lost its physical owner before admission"); + } + }; + assertCurrent(); + const admitted = await this.turns.claim( + { + claim, + ownerLaunchId: active.launchId, + supervisor: active.supervisor, + worker: active.worker, + }, + { assertCurrent }, + ); + if (admitted.action === "replay") { + return admitted.receipt; + } + clearNodeWorkerRetention(active); + active.turn = createNodeWorkerActiveTurn(claim); + const negotiated = active.idleGeneration !== undefined || idleGeneration !== undefined; + active.idleGeneration = idleGeneration; + if (negotiated) { + this.publishIdle(); + } + if (signal.aborted || !isCurrent() || active.stopState || active.retiring) { + await this.stopChild(active, signal.aborted ? "cancelled" : "interrupted"); + return (await this.turns.get(claim.launchId)) ?? admitted.receipt; + } + const secrets = nodeWorkerDescriptorSecrets(descriptor); + for (const value of secrets) { + registerSecretValueForRedaction(value); + } + // The IPC diagnostic handler shares this object, so rotate its contents rather than its owner. + Object.assign(active.scrubber, createNodeWorkerCredentialScrubber(secrets)); + active.connectionFailure.errorText = undefined; + const onAbort = () => { + void this.cancelTurn(claim).catch(() => undefined); + }; + signal.addEventListener("abort", onAbort, { once: true }); + try { + await sendNodeWorkerInput( + active.adapter, + buildWorkerProcessTurn(descriptor, active.idleGeneration !== undefined), + ); + if (signal.aborted) { + await this.cancelTurn(claim); + } + } catch { + await this.stopChild(active, "interrupted"); + } finally { + signal.removeEventListener("abort", onAbort); + } + return (await this.turns.get(claim.launchId)) ?? admitted.receipt; + } + + async status( + launchId: string, + options?: { waitMs: number; signal?: AbortSignal }, + ): Promise { + options?.signal?.throwIfAborted(); + let current = await this.readStatus(launchId); + if (!options || !current || (current.state !== "pending" && current.state !== "running")) { + return current; + } + const elapsed = createDeferredCore(); + const timer = setTimeout(() => elapsed.resolve(false), options.waitMs); + timer.unref(); + try { + while (current && (current.state === "pending" || current.state === "running")) { + const turn: NodeWorkerActiveOwnership["turn"] = this.active.get( + current.ownerLaunchId, + )?.turn; + const admission = this.admissions.get(nodeWorkerEnvironmentKey(current)); + // A journaled turn can precede its live owner. Follow admission into settlement. + const done: Promise | undefined = + turn?.claim.launchId === launchId + ? turn.done + : admission?.identity.launchId === launchId && + admission.identity.planHash === current.planHash + ? admission.done.catch(() => undefined) + : undefined; + // Completion can publish between the journal read and capturing the live owner. + current = await this.readStatus(launchId); + if (!current || (current.state !== "pending" && current.state !== "running")) { + break; + } + const notified: boolean = await racePromiseWithAbortSignal( + done ? Promise.race([done.then(() => true), elapsed.promise]) : elapsed.promise, + options.signal, + ); + options.signal?.throwIfAborted(); + // Settlement follows persistence; a timed-out observation also reconciles recovery. + current = await (notified ? this.turns.get(launchId) : this.readStatus(launchId)); + if (!notified) { + break; + } + } + return current; + } finally { + clearTimeout(timer); + } } private async readStatus(launchId: string): Promise { @@ -420,7 +766,7 @@ class NodeWorkerSupervisor { return this.store.get(launchId); } if (active.container) { - const lifecycle = requireNodeWorkerContainerLifecycle(this.containerLifecycle); + const lifecycle = this.requireContainerLifecycle(); const inspection = await lifecycle.inspect(active.container, active); if (inspection === "unknown") { return this.store.get(launchId); @@ -439,25 +785,25 @@ class NodeWorkerSupervisor { await this.stopChild(active, "interrupted"); } } else { - await cleanupNodeWorkerChildContainer(active, this.containerLifecycle); + await this.cleanupChildContainer(active); await active.done; await this.reconcileDeferredOutcome(active); } - const observed = this.active.get(launchId); - return observed?.state === "observed" - ? this.reconcileActiveTerminal(observed) - : this.store.get(launchId); - } - const workerState = inspectNodeWorkerProcessIdentity(active.worker); - if (workerState === "dead" || workerState === "reused") { - await stopOwnedNodeWorkerTree(active.worker, NODE_WORKER_STOP_GRACE_MS, FORCE_STOP_WAIT_MS); - await active.done; - const observed = this.active.get(launchId); - if (observed?.state === "observed") { - return this.reconcileActiveTerminal(observed); + } else { + const workerState = inspectNodeWorkerProcessIdentity(active.worker); + if (workerState === "dead" || workerState === "reused") { + await stopOwnedNodeWorkerTree( + active.worker, + NODE_WORKER_STOP_GRACE_MS, + FORCE_STOP_WAIT_MS, + ); + await active.done; } } - return this.store.get(launchId); + const observed = this.active.get(launchId); + return observed?.state === "observed" + ? this.reconcileActiveTerminal(observed) + : this.store.get(launchId); } const receipt = await this.store.get(launchId); return receipt?.state === "running" ? await this.recoverRunning(receipt) : receipt; @@ -486,8 +832,28 @@ class NodeWorkerSupervisor { } } - cancel(expected: NodeWorkerSupervisorIdentity): Promise { - return this.turnControl.cancel(expected); + /** External cancellation joins admission; startup invokes only the turn primitive. */ + async cancel( + expected: NodeWorkerSupervisorIdentity, + ): Promise { + const admission = [...this.admissions.values()].find((pending) => + nodeWorkerTurnMatchesIdentity(pending.identity, expected), + ); + const cancellation = this.cancelTurn(expected); + if (!admission) { + return cancellation; + } + const [cancelled, admitted] = await Promise.allSettled([cancellation, admission.done]); + if (cancelled.status === "rejected") { + throw cancelled.reason; + } + if ( + admitted.status === "rejected" && + (!admission.signal.aborted || admitted.reason !== admission.signal.reason) + ) { + throw admitted.reason; + } + return this.turns.getMatching(expected); } async stopEnvironment(expected: NodeWorkerEnvironmentStopInput): Promise { @@ -512,8 +878,8 @@ class NodeWorkerSupervisor { return; } if ( - matchingAdmission?.launchId === owner.launchId && - matchingAdmission.planHash === owner.planHash + matchingAdmission?.identity.launchId === owner.launchId && + matchingAdmission.identity.planHash === owner.planHash ) { await matchingAdmission.done.catch(() => undefined); } @@ -571,6 +937,98 @@ class NodeWorkerSupervisor { } } + private async cancelTurn( + expected: NodeWorkerSupervisorIdentity, + ): Promise { + if (this.closeCompleted) { + return this.turns.getMatching(expected); + } + const afterSettlement = async (settling: Promise) => { + try { + await settling; + } catch { + return await this.cancelTurn(expected); + } + return this.turns.getMatching(expected); + }; + let settling: Promise | undefined; + let matched: + | { + owner: NodeWorkerRunningChild; + turn: NonNullable; + } + | undefined; + for (const admission of this.admissions.values()) { + if (nodeWorkerTurnMatchesIdentity(admission.identity, expected)) { + admission.abort.abort(new Error("node worker turn cancelled")); + } + } + for (const owner of this.active.values()) { + if ( + owner.state === "running" && + owner.turn && + nodeWorkerTurnMatchesIdentity(owner.turn.claim, expected) + ) { + matched = { owner, turn: owner.turn }; + if (owner.turn.settling) { + settling = owner.turn.settling; + } else { + // The start gate must close before journal admission can yield. + owner.turn.cancelled = true; + } + } + } + if (settling) { + return await afterSettlement(settling); + } + await this.initialize(); + const receipt = await this.turns.getMatching(expected); + if (!receipt || (receipt.state !== "pending" && receipt.state !== "running")) { + return receipt ? await this.status(receipt.launchId) : undefined; + } + if (matched?.turn.settling) { + return await afterSettlement(matched.turn.settling); + } + if ( + matched && + (this.active.get(matched.owner.launchId) !== matched.owner || + matched.owner.turn !== matched.turn) + ) { + return this.status(expected.launchId); + } + const active = this.active.get(receipt.ownerLaunchId); + if (active?.state !== "running" || active.turn?.claim.launchId !== expected.launchId) { + const owner = await this.store.get(receipt.ownerLaunchId); + if (owner) { + await this.cancelOwner(owner); + } + return this.turns.getMatching(expected); + } + const turn = active.turn; + if (turn.settling) { + return await afterSettlement(turn.settling); + } + turn.cancelled = true; + try { + // A worker that stopped reading can block the write as well as the reply. + await withTimeout( + sendNodeWorkerInput(active.adapter, { type: "cancel", turnId: expected.launchId }).then( + () => turn.done, + ), + NODE_WORKER_STOP_GRACE_MS + FORCE_STOP_WAIT_MS, + { message: "node worker turn cancellation did not settle" }, + ); + } catch { + if (this.active.get(active.launchId) === active && active.turn === turn) { + await this.stopChild(active, "cancelled"); + } + } + if (this.active.get(active.launchId)?.state === "observed") { + return this.status(expected.launchId); + } + return this.turns.getMatching(expected); + } + private async cancelOwner( expected: NodeWorkerSupervisorIdentity, awaitCleanup = false, @@ -597,24 +1055,22 @@ class NodeWorkerSupervisor { return this.store.getMatching(expected); } const startup = this.starting.get(expected.launchId); - if (startup && receipt.state === "pending" && receipt.supervisor.pid === process.pid) { - if (this.containerEngine) { + if (startup && receipt.supervisor.pid === process.pid) { + if (receipt.container || (receipt.state === "pending" && this.containerEngine)) { // Startup may already own a container while its create/start client is // in flight; retain the durable slot until normal cancellation fences it. await startup; return await this.cancelOwner(expected, awaitCleanup); } - const cancelled = await this.capacity.finishCancelled({ - expected, - supervisor: receipt.supervisor, - worker: null, - }); - await startup; - return (await this.store.getMatching(expected)) ?? cancelled; - } - if (startup && receipt.container && receipt.supervisor.pid === process.pid) { - await startup; - return await this.cancelOwner(expected, awaitCleanup); + if (receipt.state === "pending") { + const cancelled = await this.capacity.finishCancelled({ + expected, + supervisor: receipt.supervisor, + worker: null, + }); + await startup; + return (await this.store.getMatching(expected)) ?? cancelled; + } } return await this.recoverRunning(receipt, true, "cancelled", awaitCleanup); } @@ -628,18 +1084,7 @@ class NodeWorkerSupervisor { for (const admission of this.admissions.values()) { admission.abort.abort(new Error("node worker supervisor is closed")); } - const operation = settleNodeWorkerSupervisorClose({ - workspace: this.workspace, - initialization: this.initializationPromise, - admissions: this.admissions, - starting: this.starting, - recoveries: this.recoveries, - retentions: this.retentions, - active: this.active, - journal: this.journal, - stopChild: (active) => this.stopChild(active, "interrupted"), - reconcileTerminal: (active) => this.reconcileActiveTerminal(active), - }).then(() => { + const operation = this.settleClose().then(() => { this.closeCompleted = true; }); const closePromise = operation.finally(() => { @@ -650,23 +1095,98 @@ class NodeWorkerSupervisor { return (this.closePromise = closePromise); } + /** Join accepted work and physical cleanup before sealing the journal. */ + private async settleClose(): Promise { + const initialization = this.initializationPromise; + const errors: unknown[] = []; + await this.workspace.processes.close().catch((error: unknown) => errors.push(error)); + await initialization?.catch((error: unknown) => errors.push(error)); + await Promise.allSettled([...this.admissions.values()].map((admission) => admission.done)); + await Promise.allSettled(this.starting.values()); + await Promise.allSettled(this.retentions); + const stopped = await Promise.allSettled([ + ...[...this.recoveries.values()].map((recovery) => recovery.done), + ...[...this.active.values()] + .filter((active): active is NodeWorkerRunningChild => active.state === "running") + .map((active) => this.stopChild(active, "interrupted")), + ]); + errors.push( + ...stopped.flatMap((result) => (result.status === "rejected" ? [result.reason] : [])), + ); + for (const active of this.active.values()) { + if (active.state !== "observed") { + continue; + } + try { + await this.reconcileActiveTerminal(active); + } catch (error) { + errors.push(error); + } + } + await this.journal + .drain({ close: errors.length === 0 }) + .catch((error: unknown) => errors.push(error)); + if (errors.length > 0) { + throw errors.length === 1 + ? errors[0] + : new AggregateError(errors, "node worker terminal reconciliation failed"); + } + } + private reconcileActiveTerminal( active: NodeWorkerObservedTerminal, ): Promise { - return reconcileNodeWorkerTerminal( - { active: this.active, turns: this.turns, capacity: this.capacity }, - active, - ); + if (active.reconciliation) { + return active.reconciliation; + } + const operation = (async () => { + if (active.cancelledTurn) { + // Gateway authority may close before worker finishing. The physical failure + // remains separate, and neither journal can settle before process cleanup. + const turn = await this.turns.finish({ + expected: active.cancelledTurn, + ownerLaunchId: active.launchId, + supervisor: active.supervisor, + worker: active.worker, + state: "cancelled", + errorText: active.outcome.errorText ?? "node worker turn cancelled", + }); + if (!turn || turn.state === "pending" || turn.state === "running") { + throw new Error("node worker cancellation lost its physical owner"); + } + } + const receipt = await this.capacity.finish({ + launchId: active.launchId, + planHash: active.planHash, + supervisor: active.supervisor, + worker: active.worker, + ...active.outcome, + }); + if (receipt.state === "pending" || receipt.state === "running") { + throw new Error(`node worker launch ${active.launchId} terminal state was not persisted`); + } + active.turn?.settle(); + active.turn = undefined; + if (this.active.get(active.launchId) === active) { + this.active.delete(active.launchId); + } + return receipt; + })(); + const pending = operation.finally(() => { + if (active.reconciliation === pending) { + active.reconciliation = undefined; + } + }); + active.reconciliation = pending; + return pending; } private async observeChild(active: NodeWorkerRunningChild): Promise { const observation = await observeNodeWorkerChild( active, - (frame) => settleNodeWorkerTurn(active, frame, this.turns), + (frame) => this.settleTurn(active, frame), () => active.turn?.claim.launchId, - active.container - ? () => cleanupNodeWorkerChildContainer(active, this.containerLifecycle) - : undefined, + active.container ? () => this.cleanupChildContainer(active) : undefined, ); if (observation.kind === "deferred") { active.deferredOutcome = observation.outcome; @@ -676,6 +1196,85 @@ class NodeWorkerSupervisor { await this.observeTerminalOutcome(active, observation.outcome); } + private async settleTurn( + active: NodeWorkerRunningChild, + frame: WorkerProcessMessage, + ): Promise { + if (frame.type === "result") { + if (active.stopState) { + return; + } + const turn = active.turn; + if (!turn || turn.claim.launchId !== frame.turnId || active.retiring) { + throw new Error("node worker returned a result outside its active turn"); + } + // Publish this operation before finish can invoke a reentrant cancellation. + const settling = Promise.resolve() + .then(async () => { + const receipt = await this.turns.finish({ + expected: turn.claim, + ownerLaunchId: active.launchId, + supervisor: active.supervisor, + worker: active.worker, + ...(turn.cancelled + ? ({ + state: "cancelled", + errorText: active.connectionFailure.errorText ?? "node worker turn cancelled", + } as const) + : ({ state: "completed", resultJson: JSON.stringify(frame.result) } as const)), + }); + if (!receipt || receipt.state === "pending" || receipt.state === "running") { + throw new Error("node worker turn completion lost its physical owner"); + } + active.turn = undefined; + active.retiring = !frame.retainWorker; + turn.settle(); + }) + .finally(() => { + if (turn.settling === settling) { + turn.settling = undefined; + } + }); + turn.settling = settling; + await settling; + } else if (active.turn || active.retention?.turnId !== frame.turnId) { + return; + } + if ( + active.stopState || + active.retiring || + active.turn || + this.active.get(active.launchId) !== active + ) { + return; + } + const reason = frame.type === "idle-ready" ? "idle" : frame.retention; + if (!reason) { + return; + } + if (active.idleGeneration === undefined) { + throw new Error("node worker reported unnegotiated retention"); + } + clearNodeWorkerRetention(active); + if (reason === "background") { + active.retention = { reason, turnId: frame.turnId }; + } else { + const timer = setTimeout(() => { + if (active.retention?.reason === "idle" && active.retention.timer === timer) { + void this.stopChild(active, "interrupted").catch(() => undefined); + } + }, 120_000); + timer.unref(); + active.retention = { reason, turnId: frame.turnId, since: Date.now(), timer }; + if (this.closed || active.idleGeneration !== this.idleGeneration) { + void this.stopChild(active, "interrupted").catch(() => undefined); + } else if (this.idleChildren().length > 2) { + void this.reclaimIdle().catch(() => undefined); + } + } + this.publishIdle(); + } + private async observeTerminalOutcome( active: NodeWorkerRunningChild, outcome: NodeWorkerTerminalOutcome, @@ -685,6 +1284,10 @@ class NodeWorkerSupervisor { return; } this.active.set(active.launchId, observed); + clearNodeWorkerRetention(active); + if (active.idleGeneration !== undefined) { + this.publishIdle(); + } try { await this.reconcileActiveTerminal(observed); } catch { @@ -708,11 +1311,64 @@ class NodeWorkerSupervisor { await this.observeTerminalOutcome(active, active.deferredOutcome); } + private requireContainerLifecycle(): NodeWorkerContainerLifecycle { + const lifecycle = this.containerLifecycle; + if (!lifecycle) { + throw new Error("node worker container isolation has no available engine"); + } + return lifecycle; + } + + private async cleanupChildContainer(active: NodeWorkerRunningChild): Promise { + if (!active.container) { + return; + } + const cleanup = (active.containerCleanup ??= this.requireContainerLifecycle() + .remove(active.container, active) + .finally(() => { + if (active.containerCleanup === cleanup) { + active.containerCleanup = undefined; + } + })); + await cleanup; + } + private async stopChild( active: NodeWorkerRunningChild, state?: NodeWorkerStopState, ): Promise { - await stopNodeWorkerChild(active, state, this.containerLifecycle); + const stopping = (async () => { + active.retiring = true; + if (active.retention?.reason === "idle") { + clearTimeout(active.retention.timer); + } + active.stopState ??= state; + if (active.container) { + // The attach client owns no workload; fence the container and prove its + // removal before its launch can become terminal or release capacity. + await this.cleanupChildContainer(active); + } + active.adapter.kill("SIGTERM"); + const forceKill = setTimeout(() => active.adapter.kill("SIGKILL"), NODE_WORKER_STOP_GRACE_MS); + forceKill.unref?.(); + try { + await active.done; + } finally { + clearTimeout(forceKill); + } + })(); + if (active.idleGeneration !== undefined) { + this.publishIdle(); + } + await stopping.catch((error: unknown) => { + if (active.retention?.reason === "idle" && !this.closed) { + clearTimeout(active.retention.timer); + active.retention.timer = setTimeout(() => { + void this.stopChild(active, state).catch(() => undefined); + }, 120_000).unref(); + } + throw error; + }); await this.reconcileDeferredOutcome(active); } } diff --git a/src/node-host/node-worker-turn-lifecycle.ts b/src/node-host/node-worker-turn-lifecycle.ts index 2c2783d1b752..d0199588abfc 100644 --- a/src/node-host/node-worker-turn-lifecycle.ts +++ b/src/node-host/node-worker-turn-lifecycle.ts @@ -1,50 +1,4 @@ -import { addAbortListener } from "node:events"; -import { racePromiseWithAbortSignal } from "../infra/abort-signal.js"; -import { withTimeout } from "../infra/fs-safe.js"; -import { registerSecretValueForRedaction } from "../logging/secret-redaction-registry.js"; -import { createDeferredCore } from "../shared/deferred.js"; import type { WorkerLaunchDescriptor } from "../worker/launch-descriptor.js"; -import { nodeWorkerTurnMatchesIdentity } from "../worker/node-supervisor-protocol.js"; -import { - buildWorkerProcessTurn, - type WorkerProcessResult, -} from "../worker/worker-process-protocol.js"; -import type { - NodeWorkerLaunchClaim, - NodeWorkerLaunchReceipt, - NodeWorkerLaunchStore, -} from "./node-worker-launch-store.js"; -import { sendNodeWorkerInput } from "./node-worker-launch-transport.js"; -import { createNodeWorkerCredentialScrubber } from "./node-worker-output.js"; -import type { NodeWorkerSupervisorIdentity } from "./node-worker-supervisor-contract.js"; -import { - createNodeWorkerActiveTurn, - nodeWorkerEnvironmentKey, - type NodeWorkerActiveOwnership, - type NodeWorkerObservedTerminal, - type NodeWorkerPendingAdmission, - type NodeWorkerRunningChild, - type NodeWorkerStopState, -} from "./node-worker-supervisor-ownership.js"; -import type { NodeWorkerTurnReceipt, NodeWorkerTurnStore } from "./node-worker-turn-store.js"; - -/** Shutdown must be able to abort admission before it stops the retiring physical owner. */ -export async function waitForNodeWorkerRetirement( - active: NodeWorkerRunningChild, - signal: AbortSignal, -): Promise { - signal.throwIfAborted(); - if (!active.retiring) { - return; - } - const aborted = createDeferredCore(); - const listener = addAbortListener(signal, () => aborted.resolve()); - try { - await Promise.race([active.done, aborted.promise]); - } finally { - listener[Symbol.dispose](); - } -} export function nodeWorkerDescriptorSecrets(descriptor: WorkerLaunchDescriptor): string[] { const endpoint = descriptor.connectionEndpoint; @@ -55,324 +9,3 @@ export function nodeWorkerDescriptorSecrets(descriptor: WorkerLaunchDescriptor): ...(descriptor.assignment.github ? [descriptor.assignment.github.token] : []), ]; } - -/** Persist completion before releasing the turn; the physical launch still owns cleanup. */ -export async function settleNodeWorkerTurn( - active: NodeWorkerRunningChild, - frame: WorkerProcessResult, - store: NodeWorkerTurnStore, -): Promise { - if (active.stopState) { - return; - } - const turn = active.turn; - if (!turn || turn.claim.launchId !== frame.turnId || active.retiring) { - throw new Error("node worker returned a result outside its active turn"); - } - // Publish this operation before finish can invoke a reentrant cancellation. - const settling = Promise.resolve() - .then(async () => { - const receipt = await store.finish({ - expected: turn.claim, - ownerLaunchId: active.launchId, - supervisor: active.supervisor, - worker: active.worker, - ...(turn.cancelled - ? ({ - state: "cancelled", - errorText: active.connectionFailure.errorText ?? "node worker turn cancelled", - } as const) - : ({ state: "completed", resultJson: JSON.stringify(frame.result) } as const)), - }); - if (!receipt || receipt.state === "pending" || receipt.state === "running") { - throw new Error("node worker turn completion lost its physical owner"); - } - active.turn = undefined; - active.retiring = !frame.retainWorker; - turn.settle(); - }) - .finally(() => { - if (turn.settling === settling) { - turn.settling = undefined; - } - }); - turn.settling = settling; - await settling; -} - -/** Preserve accepted cancellation when a worker exits without a turn result frame. */ -export async function reconcileNodeWorkerTurnCancellation( - active: NodeWorkerObservedTerminal, - store: NodeWorkerTurnStore, -): Promise { - if (!active.cancelledTurn) { - return; - } - // Gateway authority may close before worker finishing. The physical failure - // remains separate, and neither journal can settle before process cleanup. - const turn = await store.finish({ - expected: active.cancelledTurn, - ownerLaunchId: active.launchId, - supervisor: active.supervisor, - worker: active.worker, - state: "cancelled", - errorText: active.outcome.errorText ?? "node worker turn cancelled", - }); - if (!turn || turn.state === "pending" || turn.state === "running") { - throw new Error("node worker cancellation lost its physical owner"); - } -} - -export async function startNodeWorkerTurn({ - active, - descriptor, - claim, - signal, - store, - cancel, - stopChild, - isCurrent, -}: { - active: NodeWorkerRunningChild; - descriptor: WorkerLaunchDescriptor; - claim: NodeWorkerLaunchClaim; - signal: AbortSignal; - store: NodeWorkerTurnStore; - cancel: (expected: NodeWorkerSupervisorIdentity) => Promise; - stopChild: (active: NodeWorkerRunningChild, state: NodeWorkerStopState) => Promise; - isCurrent: () => boolean; -}): Promise { - signal.throwIfAborted(); - const assertCurrent = () => { - signal.throwIfAborted(); - if (!isCurrent() || active.stopState || active.retiring || active.turn) { - throw new Error("node worker turn lost its physical owner before admission"); - } - }; - assertCurrent(); - const admitted = await store.claim( - { - claim, - ownerLaunchId: active.launchId, - supervisor: active.supervisor, - worker: active.worker, - }, - { assertCurrent }, - ); - if (admitted.action === "replay") { - return admitted.receipt; - } - active.turn = createNodeWorkerActiveTurn(claim); - if (signal.aborted || !isCurrent() || active.stopState || active.retiring) { - await stopChild(active, signal.aborted ? "cancelled" : "interrupted"); - return (await store.get(claim.launchId)) ?? admitted.receipt; - } - const secrets = nodeWorkerDescriptorSecrets(descriptor); - for (const value of secrets) { - registerSecretValueForRedaction(value); - } - // The IPC diagnostic handler shares this object, so rotate its contents rather than its owner. - Object.assign(active.scrubber, createNodeWorkerCredentialScrubber(secrets)); - active.connectionFailure.errorText = undefined; - const onAbort = () => { - void cancel(claim).catch(() => undefined); - }; - signal.addEventListener("abort", onAbort, { once: true }); - try { - await sendNodeWorkerInput(active.adapter, buildWorkerProcessTurn(descriptor)); - if (signal.aborted) { - await cancel(claim); - } - } catch { - await stopChild(active, "interrupted"); - } finally { - signal.removeEventListener("abort", onAbort); - } - return (await store.get(claim.launchId)) ?? admitted.receipt; -} - -type NodeWorkerTurnControlContext = { - admissions: ReadonlyMap; - active: ReadonlyMap; - turns: Pick; - launches: Pick; - stopTimeoutMs: number; - isClosed(): boolean; - initialize(): Promise; - readStatus(launchId: string): Promise; - cancelOwner(expected: NodeWorkerSupervisorIdentity): Promise; - stopChild(active: NodeWorkerRunningChild, state: NodeWorkerStopState): Promise; -}; - -async function observeNodeWorkerTurnStatus( - context: NodeWorkerTurnControlContext, - launchId: string, - options?: { waitMs: number; signal?: AbortSignal }, -): Promise { - options?.signal?.throwIfAborted(); - let current = await context.readStatus(launchId); - if (!options || !current || (current.state !== "pending" && current.state !== "running")) { - return current; - } - const elapsed = createDeferredCore(); - const timer = setTimeout(() => elapsed.resolve(false), options.waitMs); - timer.unref(); - try { - while (current && (current.state === "pending" || current.state === "running")) { - const turn: NodeWorkerActiveOwnership["turn"] = context.active.get( - current.ownerLaunchId, - )?.turn; - const admission = context.admissions.get(nodeWorkerEnvironmentKey(current)); - // A journaled turn can precede its live owner. Follow admission into settlement. - const done: Promise | undefined = - turn?.claim.launchId === launchId - ? turn.done - : admission?.launchId === launchId && admission.planHash === current.planHash - ? admission.done.catch(() => undefined) - : undefined; - // Completion can publish between the journal read and capturing the live owner. - current = await context.readStatus(launchId); - if (!current || (current.state !== "pending" && current.state !== "running")) { - break; - } - const notified: boolean = await racePromiseWithAbortSignal( - done ? Promise.race([done.then(() => true), elapsed.promise]) : elapsed.promise, - options.signal, - ); - options.signal?.throwIfAborted(); - // Settlement follows persistence; a timed-out observation also reconciles recovery. - current = await (notified ? context.turns.get(launchId) : context.readStatus(launchId)); - if (!notified) { - break; - } - } - return current; - } finally { - clearTimeout(timer); - } -} - -/** Shares the supervisor's live turn owners between status observation and cancellation. */ -export function createNodeWorkerTurnControl(context: NodeWorkerTurnControlContext) { - const cancelTurn = (expected: NodeWorkerSupervisorIdentity) => - context.isClosed() - ? context.turns.getMatching(expected) - : cancelNodeWorkerTurn(context, expected); - return { - status: (launchId: string, options?: { waitMs: number; signal?: AbortSignal }) => - observeNodeWorkerTurnStatus(context, launchId, options), - // Startup already awaits cancellation and must not join its own admission. - cancelTurn, - cancel: async (expected: NodeWorkerSupervisorIdentity) => { - const admission = [...context.admissions.values()].find((pending) => - nodeWorkerTurnMatchesIdentity(pending.identity, expected), - ); - const cancellation = cancelTurn(expected); - if (!admission) { - return cancellation; - } - const [cancelled, admitted] = await Promise.allSettled([cancellation, admission.done]); - if (cancelled.status === "rejected") { - throw cancelled.reason; - } - if ( - admitted.status === "rejected" && - (!admission.signal.aborted || admitted.reason !== admission.signal.reason) - ) { - throw admitted.reason; - } - return context.turns.getMatching(expected); - }, - }; -} - -/** Cancel one logical turn; physical cleanup remains with its supervisor owner. */ -async function cancelNodeWorkerTurn( - context: NodeWorkerTurnControlContext, - expected: NodeWorkerSupervisorIdentity, -): Promise { - const afterSettlement = async (settling: Promise) => { - try { - await settling; - } catch { - return await cancelNodeWorkerTurn(context, expected); - } - return context.turns.getMatching(expected); - }; - let settling: Promise | undefined; - let matched: - | { - owner: NodeWorkerRunningChild; - turn: NonNullable; - } - | undefined; - for (const admission of context.admissions.values()) { - if (nodeWorkerTurnMatchesIdentity(admission.identity, expected)) { - admission.abort.abort(new Error("node worker turn cancelled")); - } - } - for (const owner of context.active.values()) { - if ( - owner.state === "running" && - owner.turn && - nodeWorkerTurnMatchesIdentity(owner.turn.claim, expected) - ) { - matched = { owner, turn: owner.turn }; - if (owner.turn.settling) { - settling = owner.turn.settling; - } else { - // The start gate must close before journal admission can yield. - owner.turn.cancelled = true; - } - } - } - if (settling) { - return await afterSettlement(settling); - } - await context.initialize(); - const receipt = await context.turns.getMatching(expected); - if (!receipt || (receipt.state !== "pending" && receipt.state !== "running")) { - return receipt ? await context.readStatus(receipt.launchId) : undefined; - } - if (matched?.turn.settling) { - return await afterSettlement(matched.turn.settling); - } - if ( - matched && - (context.active.get(matched.owner.launchId) !== matched.owner || - matched.owner.turn !== matched.turn) - ) { - return context.readStatus(expected.launchId); - } - const active = context.active.get(receipt.ownerLaunchId); - if (active?.state !== "running" || active.turn?.claim.launchId !== expected.launchId) { - const owner = await context.launches.get(receipt.ownerLaunchId); - if (owner) { - await context.cancelOwner(owner); - } - return context.turns.getMatching(expected); - } - const turn = active.turn; - if (turn.settling) { - return await afterSettlement(turn.settling); - } - turn.cancelled = true; - try { - // A worker that stopped reading can block the write as well as the reply. - await withTimeout( - sendNodeWorkerInput(active.adapter, { type: "cancel", turnId: expected.launchId }).then( - () => turn.done, - ), - context.stopTimeoutMs, - { message: "node worker turn cancellation did not settle" }, - ); - } catch { - if (context.active.get(active.launchId) === active && active.turn === turn) { - await context.stopChild(active, "cancelled"); - } - } - if (context.active.get(active.launchId)?.state === "observed") { - return context.readStatus(expected.launchId); - } - return context.turns.getMatching(expected); -} diff --git a/src/node-host/node-worker-turn-store.ts b/src/node-host/node-worker-turn-store.ts index 1497878959d0..eade35ccc97d 100644 --- a/src/node-host/node-worker-turn-store.ts +++ b/src/node-host/node-worker-turn-store.ts @@ -11,7 +11,13 @@ export type { NodeWorkerTurnReceipt } from "./node-worker-journal.types.js"; /** Immutable turn outcomes attached to a separately supervised physical worker. */ export class NodeWorkerTurnStore { - constructor(private readonly worker: NodeWorkerJournalWorker) {} + readonly get; + readonly finish; + + constructor(private readonly worker: NodeWorkerJournalWorker) { + this.get = worker.operation("nodeWorker.turn.get"); + this.finish = worker.operation("nodeWorker.turn.finish"); + } claim( params: Parameters[0], @@ -20,10 +26,6 @@ export class NodeWorkerTurnStore { return this.worker.execute({ type: "nodeWorker.turn.claim", input: [params] }, authority); } - get(turnId: string): Promise { - return this.worker.execute({ type: "nodeWorker.turn.get", input: [turnId] }); - } - async getMatching( expected: NodeWorkerSupervisorIdentity, ): Promise { @@ -31,10 +33,4 @@ export class NodeWorkerTurnStore { const receipt = await this.get(identity.launchId); return receipt && nodeWorkerTurnMatchesIdentity(receipt, identity) ? receipt : undefined; } - - finish( - params: Parameters[0], - ): Promise { - return this.worker.execute({ type: "nodeWorker.turn.finish", input: [params] }); - } } diff --git a/src/node-host/runner.shutdown.test.ts b/src/node-host/runner.shutdown.test.ts index eeed941de87a..d0cdaf707528 100644 --- a/src/node-host/runner.shutdown.test.ts +++ b/src/node-host/runner.shutdown.test.ts @@ -15,7 +15,7 @@ describe("node runner shutdown", () => { }); afterEach(() => { mocks.activeRuntime.close.mockReset().mockResolvedValue(undefined); - mocks.activeRuntime.cancelAll.mockReset(); + mocks.activeRuntime.cancelAll.mockReset().mockResolvedValue(undefined); vi.restoreAllMocks(); }); @@ -44,7 +44,7 @@ describe("node runner shutdown", () => { mocks.activeRuntime.close.mockImplementationOnce(async () => { runtimeClosed = true; }); - mocks.activeRuntime.cancelAll.mockImplementation(() => { + mocks.activeRuntime.cancelAll.mockImplementation(async () => { cleanupRestartedAfterClose ||= runtimeClosed; }); running = runNodeHost({ gatewayHost: "127.0.0.1", gatewayPort: address.port }); diff --git a/src/node-host/runner.test-support.ts b/src/node-host/runner.test-support.ts index ff54e9a2f6bb..66a7d6c3f65f 100644 --- a/src/node-host/runner.test-support.ts +++ b/src/node-host/runner.test-support.ts @@ -65,7 +65,7 @@ const mocks = vi.hoisted(() => ({ invoke: vi.fn(async () => {}), handleInput: vi.fn(), cancel: vi.fn(), - cancelAll: vi.fn(), + cancelAll: vi.fn(async () => {}), tryPauseForUpdate: vi.fn(async () => true), resumeAfterUpdate: vi.fn(), updateGatewayConnection: vi.fn(), diff --git a/src/node-host/runtime.computer.test.ts b/src/node-host/runtime.computer.test.ts index 06fae8cbc29d..424d4b79cbc6 100644 --- a/src/node-host/runtime.computer.test.ts +++ b/src/node-host/runtime.computer.test.ts @@ -222,7 +222,7 @@ describe("private worker computer runtime", () => { gate.resolve(); const host = await starting; expect(await host.invoke({ operation: "capabilities" })).toMatchObject({ ok: true }); - host.runtime.cancelAll(); + await host.runtime.cancelAll(); await host.invoke({ operation: "capabilities" }); expect(prepare).toHaveBeenCalledOnce(); } finally { @@ -292,8 +292,8 @@ describe("private worker computer runtime", () => { providerGeneration: descriptor.provider.generation, params: { executionId: otherExecutionId }, }); - host.runtime.cancelAll(); - await vi.waitFor(() => expect(host.close).toHaveBeenLastCalledWith("gateway-disconnect")); + await host.runtime.cancelAll(); + expect(host.close).toHaveBeenLastCalledWith("gateway-disconnect"); await host.runtime.close(); expect(host.close.mock.calls).toEqual([["completion"], ["gateway-disconnect"]]); } finally { diff --git a/src/node-host/runtime.test-support.ts b/src/node-host/runtime.test-support.ts index a822104dbd51..d40e0c4c91c8 100644 --- a/src/node-host/runtime.test-support.ts +++ b/src/node-host/runtime.test-support.ts @@ -7,6 +7,7 @@ const mocks = vi.hoisted(() => { return { closeMcp, closeWorkerSupervisor: vi.fn(async () => undefined), + retireIdleWorkers: vi.fn<() => Promise>(async () => undefined), workerHasActiveWork: vi.fn(async () => false), pluginHasActiveWork: vi.fn(() => false), initializeWorkerSupervisor: vi.fn(async () => undefined), @@ -48,6 +49,7 @@ vi.mock("./node-worker-supervisor.js", () => ({ createNodeWorkerSupervisor: vi.fn(() => ({ initialize: mocks.initializeWorkerSupervisor, hasActiveWork: mocks.workerHasActiveWork, + retireIdle: mocks.retireIdleWorkers, close: mocks.closeWorkerSupervisor, })), })); @@ -94,6 +96,7 @@ beforeEach(() => { mocks.closeWorkerSupervisor.mockResolvedValue(undefined); mocks.initializeWorkerSupervisor.mockResolvedValue(undefined); mocks.workerHasActiveWork.mockResolvedValue(false); + mocks.retireIdleWorkers.mockResolvedValue(undefined); mocks.pluginHasActiveWork.mockReturnValue(false); mocks.disconnectPlugins.mockResolvedValue(undefined); }); diff --git a/src/node-host/runtime.test.ts b/src/node-host/runtime.test.ts index 466f96f7f42e..644792cb5d98 100644 --- a/src/node-host/runtime.test.ts +++ b/src/node-host/runtime.test.ts @@ -26,7 +26,7 @@ type SkillBinsFixture = { response: SkillBinsResponse; invoke: (id: string) => Promise; expire: () => void; - disconnect: () => void; + disconnect: () => Promise; }; async function withSkillBinsRuntime(run: (fixture: SkillBinsFixture) => Promise) { @@ -68,7 +68,7 @@ async function withSkillBinsRuntime(run: (fixture: SkillBinsFixture) => Promise< disconnect: () => runtime.cancelAll(), }); } finally { - runtime.cancelAll(); + await runtime.cancelAll(); for (const request of requests) { request.resolve({ bins: [] }); } @@ -147,7 +147,7 @@ describe("node-host skill-bin cache", () => { await withSkillBinsRuntime(async (fixture) => { const old = fixture.invoke("old"); await vi.waitFor(() => expect(fixture.requests).toHaveLength(1)); - fixture.disconnect(); + await fixture.disconnect(); const replacement = fixture.invoke("replacement"); await vi.waitFor(() => expect(fixture.requests).toHaveLength(2)); expectDefined(fixture.requests[0], "retired connection refresh").resolve(fixture.response); @@ -167,7 +167,7 @@ describe("node-host invocation cancellation", () => { it("does not admit a queued invocation after its connection is retired", async () => { const runtime = await startRuntime(); const pending = runtime.invoke({ ...frame, command: "system.run" }); - runtime.cancelAll(); + await runtime.cancelAll(); await pending; expect(mocks.handleInvoke).not.toHaveBeenCalled(); await runtime.close(); @@ -213,7 +213,7 @@ describe("node-host invocation cancellation", () => { expect(second.signal).toBeDefined(); }); - runtime.cancelAll(); + await runtime.cancelAll(); expect(first.signal?.aborted).toBe(true); expect(second.signal?.aborted).toBe(true); @@ -331,16 +331,19 @@ describe("node-host invocation cancellation", () => { } }); - it("reports failed disconnect cleanup and retries it on explicit close", async () => { - const failure = new Error("plugin close failed"); - mocks.disconnectPlugins.mockRejectedValueOnce(failure); - const runtime = await startRuntime(); - await expect(runtime.close()).rejects.toBe(failure); - await expect(runtime.close()).resolves.toBeUndefined(); - expect(mocks.disconnectPlugins).toHaveBeenCalledTimes(2); - expect(mocks.closeMcp).toHaveBeenCalledOnce(); - expect(mocks.closeWorkerSupervisor).toHaveBeenCalledOnce(); - }); + it.each(["disconnectPlugins", "retireIdleWorkers"] as const)( + "retries failed %s cleanup on explicit close", + async (owner) => { + const failure = new Error("disconnect cleanup failed"); + mocks[owner].mockRejectedValueOnce(failure); + const runtime = await startRuntime(); + await expect(runtime.close()).rejects.toBe(failure); + await expect(runtime.close()).resolves.toBeUndefined(); + expect(mocks.disconnectPlugins).toHaveBeenCalledTimes(2); + expect(mocks.closeMcp).toHaveBeenCalledOnce(); + expect(mocks.closeWorkerSupervisor).toHaveBeenCalledOnce(); + }, + ); it("joins disconnect cleanup when an abort listener reenters close", async () => { const held = holdInvoke(); @@ -360,6 +363,7 @@ describe("node-host invocation cancellation", () => { const invoking = runtime.invoke({ ...frame, command: "system.run" }); let closing: Promise | undefined; let observed: Promise | undefined; + let disconnecting: Promise | undefined; let closed = false; try { await vi.waitFor(() => expect(held.signal).toBeDefined()); @@ -373,7 +377,7 @@ describe("node-host invocation cancellation", () => { }, { once: true }, ); - runtime.cancelAll(); + disconnecting = runtime.cancelAll(); await entered.promise; await new Promise((resolve) => { setImmediate(resolve); @@ -388,6 +392,7 @@ describe("node-host invocation cancellation", () => { expect(closed).toBe(false); } } + await disconnecting; await closing; expect(mocks.closeWorkerSupervisor).toHaveBeenCalledOnce(); expect(mocks.closeMcp).toHaveBeenCalledOnce(); @@ -397,38 +402,42 @@ describe("node-host invocation cancellation", () => { cleanup.resolve(); } held.release(); - await Promise.allSettled([invoking, closing, observed]); + await Promise.allSettled([invoking, closing, observed, disconnecting]); } }); - it("reports unavailable after failed disconnect and resumes after explicit reconnect cleanup", async () => { - const failure = new Error("plugin disconnect failed"); - mocks.disconnectPlugins.mockRejectedValueOnce(failure); - const request = vi.fn(async () => ({})); - const runtime = await startRuntime(createNodeHostClient(request)); - try { - runtime.cancelAll(); - await runtime.invoke(frame); - expect(mocks.handleInvoke).not.toHaveBeenCalled(); - expect(request).toHaveBeenCalledWith( - "node.invoke.result", - expect.objectContaining({ - id: frame.id, - ok: false, - error: { - code: "UNAVAILABLE", - message: "Node plugin cleanup failed. Reconnect the node to retry cleanup.", - }, - }), - ); - runtime.cancelAll(); - await runtime.invoke({ ...frame, id: "after-reconnect" }); - expect(mocks.handleInvoke).toHaveBeenCalledOnce(); - expect(mocks.disconnectPlugins).toHaveBeenCalledTimes(2); - } finally { - await runtime.close(); - } - }); + it.each(["disconnectPlugins", "retireIdleWorkers"] as const)( + "keeps invoke admission closed after failed %s cleanup until reconnect", + async (owner) => { + const failure = new Error("disconnect cleanup failed"); + mocks[owner].mockRejectedValueOnce(failure); + const request = vi.fn(async () => ({})); + const runtime = await startRuntime(createNodeHostClient(request)); + try { + const disconnecting = runtime.cancelAll().catch((error: unknown) => error); + await runtime.invoke(frame); + expect(await disconnecting).toBe(failure); + expect(mocks.handleInvoke).not.toHaveBeenCalled(); + expect(request).toHaveBeenCalledWith( + "node.invoke.result", + expect.objectContaining({ + id: frame.id, + ok: false, + error: { + code: "UNAVAILABLE", + message: "Node disconnect cleanup failed. Reconnect the node to retry cleanup.", + }, + }), + ); + await runtime.cancelAll(); + await runtime.invoke({ ...frame, id: "after-reconnect" }); + expect(mocks.handleInvoke).toHaveBeenCalledOnce(); + expect(mocks.disconnectPlugins).toHaveBeenCalledTimes(2); + } finally { + await runtime.close(); + } + }, + ); it("aggregates independent supervisor and MCP close failures in owner order", async () => { const supervisorError = new Error("supervisor close failed"); diff --git a/src/node-host/runtime.ts b/src/node-host/runtime.ts index fbd44368c797..57e53240ad91 100644 --- a/src/node-host/runtime.ts +++ b/src/node-host/runtime.ts @@ -77,7 +77,7 @@ type ActiveNodeHostRuntime = { invoke(frame: NodeInvokeRequestPayload): Promise; handleInput(invokeId: string, seq: number, payloadJSON: string): void; cancel(invokeId: string): void; - cancelAll(): void; + cancelAll(): Promise; tryPauseForUpdate(): Promise; resumeAfterUpdate(): void; updateGatewayConnection(connection?: { @@ -106,6 +106,19 @@ function ensureNodePathEnv(): string { return DEFAULT_NODE_PATH; } +async function settleNodeHostCleanup(owners: Array | undefined>): Promise { + const results = await Promise.allSettled(owners.filter((owner) => owner !== undefined)); + const errors = [ + ...new Set(results.flatMap((result) => (result.status === "rejected" ? [result.reason] : []))), + ]; + if (errors.length === 1) { + throw errors[0]; + } + if (errors.length > 1) { + throw new AggregateError(errors, "node-host runtime cleanup failed"); + } +} + export async function prepareNodeHostRuntime(params?: { config?: OpenClawConfig; env?: NodeJS.ProcessEnv; @@ -320,9 +333,9 @@ export async function prepareNodeHostRuntime(params?: { } let skillBins = new SkillBinsCache(client, pathEnv); const activeInvokes = new Map(); - let pluginDisconnectCleanup: Promise = Promise.resolve(); - let pendingPluginDisconnectCleanups = 0; - let pluginDisconnectCleanupFailed = false; + let disconnectCleanup: Promise = Promise.resolve(); + let pendingDisconnectCleanups = 0; + let disconnectCleanupFailed = false; const pluginCommandContext: OpenClawPluginNodeHostCommandContext = { sendNodeEvent: async (event, payload) => await client.request("node.event", buildNodeEventParams(event, payload)), @@ -401,11 +414,17 @@ export async function prepareNodeHostRuntime(params?: { closing || !mcpStartupComplete || inFlightInvokes > 0 || - pendingPluginDisconnectCleanups > 0 || - pluginDisconnectCleanupFailed || + pendingDisconnectCleanups > 0 || + disconnectCleanupFailed || hasRegisteredNodeHostCommandActiveWork() || workerCleanupIncomplete, - hasWorkerActiveWork: () => workerSupervisor?.hasActiveWork(), + hasWorkerActiveWork: async () => { + if (await workerSupervisor?.hasActiveWork()) { + return true; + } + await workerSupervisor?.retireIdle(); + return false; + }, }); return { async invoke(frame) { @@ -422,12 +441,12 @@ export async function prepareNodeHostRuntime(params?: { try { const generation = connectionGeneration; try { - await pluginDisconnectCleanup; + await disconnectCleanup; } catch { if (!closing && generation === connectionGeneration) { await createNodeInvokeResponder(client, frame).error( "UNAVAILABLE", - "Node plugin cleanup failed. Reconnect the node to retry cleanup.", + "Node disconnect cleanup failed. Reconnect the node to retry cleanup.", ); } return; @@ -587,32 +606,34 @@ export async function prepareNodeHostRuntime(params?: { // Retired refreshes may still finish; their cache must never serve the next connection. skillBins = new SkillBinsCache(client, pathEnv); // Close can reenter from an abort listener and must see this cleanup barrier. - pendingPluginDisconnectCleanups += 1; - const cleanup = pluginDisconnectCleanup - .catch(() => {}) - .then(async () => await notifyRegisteredNodeHostCommandDisconnect()) - .finally(() => { - pendingPluginDisconnectCleanups -= 1; - }); - pluginDisconnectCleanup = cleanup; + pendingDisconnectCleanups += 1; + const idleCleanup = workerSupervisor?.retireIdle(); + const cleanup = settleNodeHostCleanup([ + disconnectCleanup.catch(() => {}).then(notifyRegisteredNodeHostCommandDisconnect), + idleCleanup, + ]).finally(() => { + pendingDisconnectCleanups -= 1; + }); + disconnectCleanup = cleanup; // Logging observes the failure; invocation and shutdown retain the rejected result. void cleanup.then( () => { - if (pluginDisconnectCleanup === cleanup) { - pluginDisconnectCleanupFailed = false; + if (disconnectCleanup === cleanup) { + disconnectCleanupFailed = false; } }, (error: unknown) => { - if (pluginDisconnectCleanup === cleanup) { - pluginDisconnectCleanupFailed = true; + if (disconnectCleanup === cleanup) { + disconnectCleanupFailed = true; } - logDebug(`node-host: plugin disconnect cleanup failed: ${String(error)}`); + logDebug(`node-host: disconnect cleanup failed: ${String(error)}`); }, ); for (const active of activeInvokes.values()) { active.controller.abort(); } activeInvokes.clear(); + return cleanup; }, tryPauseForUpdate: updatePause.tryPauseForUpdate, resumeAfterUpdate: updatePause.resumeAfterUpdate, @@ -629,19 +650,18 @@ export async function prepareNodeHostRuntime(params?: { const completion = createDeferredCore(); closePromise = completion.promise; const closeOwners = async () => { - if (!wasClosing) { - if (initializationRetry) { - clearTimeout(initializationRetry); - initializationRetry = undefined; - } - this.cancelAll(); - } else if (pluginDisconnectCleanupFailed) { - this.cancelAll(); + if (!wasClosing && initializationRetry) { + clearTimeout(initializationRetry); + initializationRetry = undefined; + } + if (!wasClosing || disconnectCleanupFailed) { + // cancelAll publishes the cleanup barrier joined below. + void this.cancelAll(); } const watcherClose = stopAvailabilityWatch(); // Startup observes this signal before either independent owner is joined. mcpAbort.abort(); - const disconnectClose = pluginDisconnectCleanup; + const disconnectClose = disconnectCleanup; supervisorClose ??= Promise.resolve() .then(() => workerSupervisor?.close()) .catch((error: unknown) => { @@ -651,23 +671,7 @@ export async function prepareNodeHostRuntime(params?: { }); // MCP close is terminal: another call after failure can return an empty success. mcpClose ??= startup.then((resolved) => resolved?.close()); - const results = await Promise.allSettled([ - watcherClose, - disconnectClose, - supervisorClose, - mcpClose, - ]); - const errors = [ - ...new Set( - results.flatMap((result) => (result.status === "rejected" ? [result.reason] : [])), - ), - ]; - if (errors.length === 1) { - throw errors[0]; - } - if (errors.length > 1) { - throw new AggregateError(errors, "node-host runtime close failed"); - } + await settleNodeHostCleanup([watcherClose, disconnectClose, supervisorClose, mcpClose]); }; void closeOwners().then(completion.resolve, (error: unknown) => { closePromise = undefined; diff --git a/src/node-host/runtime.update-pause.test.ts b/src/node-host/runtime.update-pause.test.ts index c67630bd471d..ed65afcb471b 100644 --- a/src/node-host/runtime.update-pause.test.ts +++ b/src/node-host/runtime.update-pause.test.ts @@ -12,6 +12,29 @@ import { } from "./runtime.test-support.js"; describe("node-host update pause", () => { + it("retires idle workers before update admission while holding new invokes", async () => { + const retirement = createDeferred(); + const entered = createDeferred(); + mocks.retireIdleWorkers.mockImplementationOnce(async () => { + entered.resolve(); + await retirement.promise; + }); + const runtime = await startRuntime(); + const pausing = runtime.tryPauseForUpdate(); + try { + await entered.promise; + await runtime.invoke(frame); + expect(mocks.handleInvoke).not.toHaveBeenCalled(); + retirement.resolve(); + expect(await pausing).toBe(true); + expect(mocks.retireIdleWorkers).toHaveBeenCalledOnce(); + } finally { + retirement.resolve(); + await pausing; + await runtime.close(); + } + }); + it.each(["idle", "busy", "error", "plugin", "disconnect", "close"] as const)( "holds invoke admission through a delayed worker idle read ending in %s", async (outcome) => { @@ -25,6 +48,7 @@ describe("node-host update pause", () => { outcome === "error" ? expect(pausing).rejects.toThrow("journal unavailable") : expect(pausing).resolves.toBe(outcome === "idle"); + let disconnecting: Promise | undefined; try { await runtime.invoke(frame); expect(mocks.handleInvoke).not.toHaveBeenCalled(); @@ -38,7 +62,7 @@ describe("node-host update pause", () => { mocks.pluginHasActiveWork.mockReturnValue(true); } else if (outcome === "disconnect") { mocks.disconnectPlugins.mockImplementationOnce(async () => await cleanup.promise); - runtime.cancelAll(); + disconnecting = runtime.cancelAll(); } else if (outcome === "close") { await runtime.close(); } @@ -49,6 +73,7 @@ describe("node-host update pause", () => { } await result; cleanup.resolve(); + await disconnecting; if (outcome === "idle") { await runtime.invoke({ ...frame, id: "paused" }); expect(mocks.handleInvoke).not.toHaveBeenCalled(); @@ -59,7 +84,7 @@ describe("node-host update pause", () => { } finally { idle.resolve(false); cleanup.resolve(); - await Promise.allSettled([pausing]); + await Promise.allSettled([pausing, disconnecting]); await runtime.close(); } }, @@ -169,7 +194,7 @@ describe("node-host update pause", () => { try { expect(await runtime.tryPauseForUpdate()).toBe(false); await vi.waitFor(() => expect(held.signal).toBeDefined()); - runtime.cancelAll(); + await runtime.cancelAll(); expect(held.signal?.aborted).toBe(true); expect(await runtime.tryPauseForUpdate()).toBe(false); @@ -233,13 +258,16 @@ describe("node-host update pause", () => { mocks.disconnectPlugins.mockImplementationOnce(async () => await cleanup.promise); const runtime = await startRuntime(); try { - runtime.cancelAll(); + const disconnecting = expect(runtime.cancelAll()).rejects.toThrow( + "plugin process tree did not terminate", + ); expect(await runtime.tryPauseForUpdate()).toBe(false); cleanup.reject(new Error("plugin process tree did not terminate")); + await disconnecting; await runtime.invoke(frame); expect(await runtime.tryPauseForUpdate()).toBe(false); - runtime.cancelAll(); + await runtime.cancelAll(); await runtime.invoke(frame); expect(await runtime.tryPauseForUpdate()).toBe(true); } finally { diff --git a/src/node-host/runtime.worker-hosting.test.ts b/src/node-host/runtime.worker-hosting.test.ts index 152d49e1a7f2..53a5e2692d14 100644 --- a/src/node-host/runtime.worker-hosting.test.ts +++ b/src/node-host/runtime.worker-hosting.test.ts @@ -32,6 +32,7 @@ vi.mock("./node-worker-container-engine.js", () => ({ vi.mock("./node-worker-supervisor.js", () => ({ createNodeWorkerSupervisor: vi.fn(() => ({ initialize: mocks.initializeWorkerSupervisor, + retireIdle: vi.fn(async () => undefined), close: mocks.closeWorkerSupervisor, })), })); diff --git a/src/node-host/runtime.worker-supervisor.test.ts b/src/node-host/runtime.worker-supervisor.test.ts index 094bab00f523..cf40e5780ff0 100644 --- a/src/node-host/runtime.worker-supervisor.test.ts +++ b/src/node-host/runtime.worker-supervisor.test.ts @@ -126,7 +126,7 @@ describe("node-host runtime worker supervisor lifetime", () => { expect((await store.get(input.launchId))?.state).toBe("running"); runtime.cancel("invoke-launch"); - runtime.cancelAll(); + await runtime.cancelAll(); expect((await store.get(input.launchId))?.state).toBe("running"); launchResponseHeld.resolve(); await launching; diff --git a/src/node-host/worker-runtime.test.ts b/src/node-host/worker-runtime.test.ts index 90bf4dd0fa54..e2a148b11de4 100644 --- a/src/node-host/worker-runtime.test.ts +++ b/src/node-host/worker-runtime.test.ts @@ -31,7 +31,7 @@ const fixture = vi.hoisted(() => ({ invoke: vi.fn(), handleInput: vi.fn(), cancel: vi.fn(), - cancelAll: vi.fn(), + cancelAll: vi.fn(async () => undefined), updateGatewayConnection: vi.fn(), close: vi.fn(), }, @@ -625,6 +625,7 @@ it("publishes host stats through the native bridge only while connected", async connection: { url: "wss://gateway.example.test", protocol: 4, capabilities: [] }, }), ); + await vi.advanceTimersByTimeAsync(0); expect(publications()).toHaveLength(1); expect(publications()[0]).toMatchObject({ type: "node-event", diff --git a/src/shared/node-list-parse.ts b/src/shared/node-list-parse.ts index d6cfb5345662..1e1e86d73237 100644 --- a/src/shared/node-list-parse.ts +++ b/src/shared/node-list-parse.ts @@ -4,6 +4,10 @@ import type { NodeListNode, PairedNode, PairingList, PendingRequest } from "./no export const NODE_WORKER_CAPACITY_MAX = 1_024; +export function availableWorkerSlots(capacity: NonNullable): number { + return capacity.available + (capacity.reclaimableIdle ?? 0); +} + export function parseWorkerSlotSummary( value: unknown, ): NonNullable | null { @@ -11,9 +15,8 @@ export function parseWorkerSlotSummary( return null; } const keys = Object.keys(value); - const total = value.total; - const available = value.available; - return keys.length === 2 && + const { total, available, reclaimableIdle } = value; + return keys.every((key) => key === "total" || key === "available" || key === "reclaimableIdle") && keys.includes("total") && keys.includes("available") && typeof total === "number" && @@ -23,8 +26,13 @@ export function parseWorkerSlotSummary( total >= 1 && total <= NODE_WORKER_CAPACITY_MAX && available >= 0 && - available <= total - ? { total, available } + available <= total && + (reclaimableIdle === undefined || + (typeof reclaimableIdle === "number" && + Number.isSafeInteger(reclaimableIdle) && + reclaimableIdle >= 0 && + reclaimableIdle <= Math.min(2, total - available))) + ? { total, available, ...(reclaimableIdle === undefined ? {} : { reclaimableIdle }) } : null; } diff --git a/src/worker/github-binding.runtime.test.ts b/src/worker/github-binding.runtime.test.ts index 6922066a5a86..31292976f345 100644 --- a/src/worker/github-binding.runtime.test.ts +++ b/src/worker/github-binding.runtime.test.ts @@ -4,7 +4,10 @@ import path from "node:path"; import { pathToFileURL } from "node:url"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import * as exec from "../process/exec.js"; -import { prepareWorkerGitHubEnvironment } from "./github-binding.runtime.js"; +import { + disposeWorkerGitHubEnvironment, + prepareWorkerGitHubEnvironment, +} from "./github-binding.runtime.js"; const { warn, inspectPathPermissions } = vi.hoisted(() => ({ warn: vi.fn(), @@ -78,7 +81,7 @@ describe("prepareWorkerGitHubEnvironment", () => { prepareWorkerGitHubEnvironment({ binding, stateDir: path.join(root, "state"), - runId: "turn", + turnId: "turn", cwd, }); @@ -177,7 +180,7 @@ describe("prepareWorkerGitHubEnvironment", () => { await prepareWorkerGitHubEnvironment({ binding, stateDir: path.join(root, "state"), - runId: "turn", + turnId: "turn", cwd, signal: controller.signal, }); @@ -204,7 +207,7 @@ describe("prepareWorkerGitHubEnvironment", () => { await prepareWorkerGitHubEnvironment({ binding, stateDir: path.join(root, "state"), - runId: "turn", + turnId: "turn", cwd, signal: controller.signal, }); @@ -248,7 +251,7 @@ describe("prepareWorkerGitHubEnvironment", () => { await prepareWorkerGitHubEnvironment({ binding: withoutRemote, stateDir: path.join(root, "state"), - runId: "turn", + turnId: "turn", cwd, }); @@ -317,6 +320,39 @@ describe("prepareWorkerGitHubEnvironment", () => { expect(JSON.stringify(prepared)).not.toContain(binding.token); expect(process.env.GH_TOKEN).toBe("inherited-synthetic-token"); expect(process.env.GITHUB_TOKEN).toBe("inherited-synthetic-token"); + const profileDir = prepared?.localIdentityEnv?.GH_CONFIG_DIR; + if (!profileDir) { + throw new Error("Expected a turn-owned GitHub profile"); + } + await disposeWorkerGitHubEnvironment(path.join(root, "state"), "turn"); + await expect(fs.access(profileDir)).rejects.toMatchObject({ code: "ENOENT" }); + }); + + it("does not remove a newer profile when the previous turn finishes cleanup", async () => { + const previous = await prepare(); + const current = await prepareWorkerGitHubEnvironment({ + binding, + stateDir: path.join(root, "state"), + turnId: "next-turn", + cwd, + }); + const previousProfile = previous?.localIdentityEnv.GH_CONFIG_DIR; + const currentProfile = current?.localIdentityEnv.GH_CONFIG_DIR; + expect(previousProfile).toBeTruthy(); + expect(currentProfile).toBeTruthy(); + expect(currentProfile).not.toBe(previousProfile); + if (!previousProfile || !currentProfile) { + throw new Error("Expected both turns' GitHub profiles"); + } + expect(await fs.readFile(path.join(previousProfile, "hosts.yml"), "utf8")).toContain( + binding.token, + ); + await disposeWorkerGitHubEnvironment(path.join(root, "state"), "turn"); + expect(await fs.readFile(path.join(currentProfile, "hosts.yml"), "utf8")).toContain( + binding.token, + ); + await disposeWorkerGitHubEnvironment(path.join(root, "state"), "next-turn"); + await expect(fs.access(currentProfile)).rejects.toMatchObject({ code: "ENOENT" }); }); it("warns and continues without changing local files when origin cannot be fetched", async () => { diff --git a/src/worker/github-binding.runtime.ts b/src/worker/github-binding.runtime.ts index b58d81b8c9a3..3603dcad1373 100644 --- a/src/worker/github-binding.runtime.ts +++ b/src/worker/github-binding.runtime.ts @@ -1,8 +1,8 @@ -import fs from "node:fs/promises"; import path from "node:path"; import { inspectPathPermissions } from "@openclaw/fs-safe/permissions"; import { managedGitHubIdentityEnvironment, + removeManagedGitHubProfile, writeManagedGitHubProfileFiles, type PreparedGitHubToolEnvironment, } from "../agents/github-tool-identity.js"; @@ -14,6 +14,12 @@ import type { WorkerGitHubLaunchBinding } from "./launch-descriptor.js"; const log = createSubsystemLogger("worker/github"); +export function disposeWorkerGitHubEnvironment(stateDir: string, turnId: string) { + return removeManagedGitHubProfile( + path.join(stateDir, "github-profiles", sha256HexPrefixCore(turnId, 16)), + ); +} + async function bindWorkerGitHubCheckout( cwd: string, binding: WorkerGitHubLaunchBinding, @@ -52,11 +58,8 @@ async function bindWorkerGitHubCheckout( await requireGit(["update-ref", branch, "HEAD"]); await requireGit(["symbolic-ref", "HEAD", branch]); } - // Reconciliation returns files, not commits; origin holds this session's own pushed history. - // A fast-forward only adds session commits while preserving reconciled working-tree bytes. - // Leave divergence for the agent to resolve. Only the verified GitHub origin the Gateway - // named may receive the token-bound fetch; a binding without one keeps its checkout as is. - // A fenced turn has lost its authority: never start the credentialed fetch for it. + // Reconciliation returns files, not commits. Fetch only the admitted origin; + // fast-forward session commits without overwriting reconciled working bytes. if (!binding.remoteUrl || signal?.aborted) { return; } @@ -103,19 +106,16 @@ async function bindWorkerGitHubCheckout( export async function prepareWorkerGitHubEnvironment(params: { binding: WorkerGitHubLaunchBinding; stateDir: string; - runId: string; + turnId: string; cwd: string; signal?: AbortSignal; }): Promise { - const { binding, stateDir, runId, cwd, signal } = params; + const { binding, stateDir, turnId, cwd, signal } = params; registerSecretValueForRedaction(binding.token); - const profilesRoot = path.join(stateDir, "github-profiles"); - const profileDir = path.join(profilesRoot, sha256HexPrefixCore(runId, 16)); + const profileDir = path.join(stateDir, "github-profiles", sha256HexPrefixCore(turnId, 16)); try { - // Retained workers reuse state across turns, but each turn owns one profile path. - // Remove earlier profiles first so an inherited path cannot expose a later credential; - // an earlier process keeps only the token in its own environment. - await fs.rm(profilesRoot, { recursive: true, force: true }); + // Each turn owns its path; retained commands keep their existing credentials. + await removeManagedGitHubProfile(profileDir); await writeManagedGitHubProfileFiles(profileDir, binding); } catch (error) { const message = error instanceof Error ? error.message : String(error); diff --git a/src/worker/node-supervisor-protocol.test.ts b/src/worker/node-supervisor-protocol.test.ts index 42d1f135dc95..dcef2a53660b 100644 --- a/src/worker/node-supervisor-protocol.test.ts +++ b/src/worker/node-supervisor-protocol.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { testWorkerDescriptor } from "../node-host/node-worker-supervisor.test-support.js"; import { NODE_WORKER_CONNECTION_FAILURE_MESSAGE_TYPE, + nodeWorkerPlanHash, parseNodeWorkerConnectionFailureMessage, parseNodeWorkerEnvironmentStopInput, parseNodeWorkerLaunchInput, @@ -44,6 +45,26 @@ describe("node worker status wait request", () => { }); describe("node worker supervisor launch request", () => { + it("keeps the old launch shape exact and binds negotiated idle retention into its plan hash", () => { + const descriptor = testWorkerDescriptor("/tmp/worker", "success", "turn-1"); + const input = { + environmentSession: 1, + launchId: "turn-1", + gatewayNamespace: "gateway-1", + expectedBundleHash: descriptor.admission.handshake.bundleHash, + placementGeneration: 4, + descriptor, + }; + const legacy = parseNodeWorkerLaunchInput(JSON.stringify(input)); + expect(legacy).toEqual(input); + const retained = parseNodeWorkerLaunchInput(JSON.stringify({ ...input, idleRetention: true })); + expect(retained).toEqual({ ...input, idleRetention: true }); + expect(nodeWorkerPlanHash(retained)).not.toBe(nodeWorkerPlanHash(legacy)); + expect(() => + parseNodeWorkerLaunchInput(JSON.stringify({ ...input, idleRetention: false })), + ).toThrow("INVALID_REQUEST"); + }); + it.each([undefined, 2])( "rejects a Gateway without the negotiated environment lifetime marker %s", (environmentSession) => { diff --git a/src/worker/node-supervisor-protocol.ts b/src/worker/node-supervisor-protocol.ts index de4357cbe996..27bfbd5ffc0e 100644 --- a/src/worker/node-supervisor-protocol.ts +++ b/src/worker/node-supervisor-protocol.ts @@ -51,6 +51,9 @@ const LaunchInput = workerProtocolObject({ error: "INVALID_REQUEST: node worker environment lifetime support required", }), sessionKey: identifier("sessionKey", 1_024).optional(), + idleRetention: z + .literal(true, { error: "INVALID_REQUEST: idleRetention must be true" }) + .optional(), launchId: IdentityShape.launchId, gatewayNamespace: WorkerGatewayNamespace, expectedBundleHash: z.custom(isPlanHash, { @@ -174,7 +177,12 @@ export function parseNodeWorkerEnvironmentStopInput( export function nodeWorkerPlanHash( input: Pick< NodeWorkerLaunchInput, - "descriptor" | "expectedBundleHash" | "gatewayNamespace" | "placementGeneration" | "sessionKey" + | "descriptor" + | "expectedBundleHash" + | "gatewayNamespace" + | "placementGeneration" + | "sessionKey" + | "idleRetention" >, ): string { return createHash("sha256") @@ -185,6 +193,7 @@ export function nodeWorkerPlanHash( gatewayNamespace: input.gatewayNamespace, placementGeneration: input.placementGeneration, ...(input.sessionKey === undefined ? {} : { sessionKey: input.sessionKey }), + ...(input.idleRetention ? { idleRetention: true } : {}), }), ) .digest("hex"); diff --git a/src/worker/worker-command.runtime.test.ts b/src/worker/worker-command.runtime.test.ts index 20cc16427285..c553dee98067 100644 --- a/src/worker/worker-command.runtime.test.ts +++ b/src/worker/worker-command.runtime.test.ts @@ -16,17 +16,28 @@ import type { WorkerLaunchDescriptor } from "./launch-descriptor.js"; import { runWorkerCommand } from "./worker-command.runtime.js"; import { buildWorkerProcessTurn, - parseWorkerProcessResult, + parseWorkerProcessMessage, serializeWorkerProcessInput, + type WorkerProcessMessage, type WorkerProcessResult, } from "./worker-process-protocol.js"; import { runWorkerProcess } from "./worker-process.js"; import { createWorkerRuntimeEnvironment, runWorkerDescriptor } from "./worker.runtime.js"; -const managedRuntime = vi.hoisted(() => ({ backgroundCount: 0, close: vi.fn() })); +const managedRuntime = vi.hoisted(() => ({ + backgroundCount: 0, + close: vi.fn(), + waitForExecScope: vi.fn(), + disposeProfile: vi.fn(), +})); vi.mock("../agents/bash-process-registry.js", () => ({ getActiveBackgroundExecSessionCount: () => managedRuntime.backgroundCount, + waitForExecScope: managedRuntime.waitForExecScope, +})); + +vi.mock("./github-binding.runtime.js", () => ({ + disposeWorkerGitHubEnvironment: managedRuntime.disposeProfile, })); vi.mock("./worker.runtime.js", () => ({ @@ -145,8 +156,8 @@ function managedHarness() { const output = new PassThrough(); const results: WorkerProcessResult[] = []; output.on("data", (chunk: Buffer) => { - const result = parseWorkerProcessResult(JSON.parse(chunk.toString("utf8"))); - if (result) { + const result = parseWorkerProcessMessage(JSON.parse(chunk.toString("utf8"))); + if (result?.type === "result") { results.push(result); } }); @@ -162,10 +173,21 @@ function managedHarness() { input, output, results, + nextMessage: () => + new Promise((resolve, reject) => { + output.once("data", (chunk: Buffer) => { + const message = parseWorkerProcessMessage(JSON.parse(chunk.toString("utf8"))); + if (message) { + resolve(message); + } else { + reject(new Error("Invalid worker message")); + } + }); + }), launch, send, - turn: (value: WorkerLaunchDescriptor = launch) => - send({ type: "turn", turnId: value.assignment.turnId, descriptor: value }), + turn: (value: WorkerLaunchDescriptor = launch, idleRetention = false) => + send(buildWorkerProcessTurn(value, idleRetention)), }; } @@ -187,6 +209,8 @@ describe("worker command lifetime gate", () => { transcriptNextSeq: 1, }); managedRuntime.backgroundCount = 0; + managedRuntime.waitForExecScope.mockReset().mockResolvedValue(undefined); + managedRuntime.disposeProfile.mockReset().mockResolvedValue(undefined); managedRuntime.close.mockReset(); managedRuntime.close.mockResolvedValue(undefined); vi.mocked(createWorkerRuntimeEnvironment).mockReset(); @@ -373,7 +397,12 @@ describe("worker command lifetime gate", () => { lifetime.open(); harness.turn(); await vi.waitFor(() => expect(harness.results).toHaveLength(1)); - expect(harness.results[0]).toMatchObject({ turnId: "turn-1", retainWorker: true }); + expect(harness.results[0]).toEqual({ + type: "result", + turnId: "turn-1", + retainWorker: true, + result: { status: "completed", transcriptLeafId: "first-leaf", transcriptNextSeq: 2 }, + }); expect(lifetime.dispose).not.toHaveBeenCalled(); const next = structuredClone(harness.launch); @@ -400,6 +429,133 @@ describe("worker command lifetime gate", () => { expect(lifetime.dispose).toHaveBeenCalledOnce(); }); + it.each([false, true])( + "joins finalizers and profile disposal before idle readiness (cancelled: %s)", + async (cancelled) => { + const harness = managedHarness(); + harness.launch.assignment.github = { + token: "synthetic-turn-token", + login: "worker-fixture", + branch: "session-fixture", + gitAuthor: { name: "Fixture", email: "fixture@openclaw.invalid" }, + }; + const joining = createDeferred(); + const settled = createDeferred(); + const disposing = createDeferred(); + const disposed = createDeferred(); + managedRuntime.waitForExecScope.mockImplementationOnce(() => { + joining.resolve(); + return settled.promise; + }); + managedRuntime.disposeProfile.mockImplementationOnce(() => { + disposing.resolve(); + return disposed.promise; + }); + const running = runWorkerCommand({ ...harness, managed: true }); + const first = harness.nextMessage(); + harness.turn(harness.launch, true); + await joining.promise; + expect(harness.results).toEqual([]); + expect(managedRuntime.disposeProfile).not.toHaveBeenCalled(); + settled.resolve(); + await disposing.promise; + expect(harness.results).toEqual([]); + if (cancelled) { + harness.send({ type: "cancel", turnId: "turn-1" }); + } + disposed.resolve(); + expect(await first).toMatchObject( + cancelled + ? { retainWorker: false, turnId: "turn-1" } + : { retainWorker: true, retention: "idle", turnId: "turn-1" }, + ); + expect(managedRuntime.disposeProfile).toHaveBeenCalledWith( + "/tmp/openclaw-managed-worker-state", + "turn-1", + ); + if (cancelled) { + await running; + expect(managedRuntime.close).toHaveBeenCalledOnce(); + return; + } + + const next = structuredClone(harness.launch); + next.assignment.turnId = "turn-2"; + next.assignment.runId = "run-2"; + next.assignment.operationalRunInstance = { instanceId: "instance-run-2", runId: "run-2" }; + next.admission.credential = "synthetic-replacement-credential"; + const second = Promise.race([harness.nextMessage(), running]); + harness.turn(next, true); + expect(await second).toMatchObject({ retention: "idle", turnId: "turn-2" }); + expect(vi.mocked(runWorkerDescriptor).mock.lastCall?.[0]).toEqual(next); + expect(createWorkerRuntimeEnvironment).toHaveBeenCalledOnce(); + harness.input.end(); + await running; + expect(managedRuntime.close).toHaveBeenCalledOnce(); + }, + ); + + it.each([false, true])( + "settles background retention without stale idle readiness (new turn: %s)", + async (newTurn) => { + const harness = managedHarness(); + harness.launch.assignment.github = { + token: "synthetic-background-token", + login: "fixture", + branch: "fixture", + }; + const settled = createDeferred(); + const disposed = createDeferred(); + const secondStarted = createDeferred(); + const secondSettled = createDeferred(); + managedRuntime.disposeProfile.mockImplementationOnce(async () => disposed.resolve()); + managedRuntime.backgroundCount = 1; + managedRuntime.waitForExecScope.mockReturnValueOnce(settled.promise); + const running = runWorkerCommand({ ...harness, managed: true }); + const first = harness.nextMessage(); + harness.turn(harness.launch, true); + expect(await first).toMatchObject({ retention: "background", turnId: "turn-1" }); + const nextMessage = harness.nextMessage(); + if (newTurn) { + vi.mocked(runWorkerDescriptor).mockImplementationOnce(async () => { + secondStarted.resolve(); + await secondSettled.promise; + return { status: "completed", transcriptLeafId: null, transcriptNextSeq: 1 }; + }); + const next = structuredClone(harness.launch); + next.assignment.turnId = "turn-2"; + delete next.assignment.github; + harness.turn(next, true); + await secondStarted.promise; + } + managedRuntime.backgroundCount = 0; + settled.resolve(); + await disposed.promise; + expect(managedRuntime.disposeProfile).toHaveBeenCalledExactlyOnceWith( + "/tmp/openclaw-managed-worker-state", + "turn-1", + ); + if (newTurn) { + expect(harness.results).toHaveLength(1); + } + secondSettled.resolve(); + expect(await nextMessage).toMatchObject( + newTurn + ? { type: "result", turnId: "turn-2", retention: "idle" } + : { type: "idle-ready", turnId: "turn-1" }, + ); + expect(managedRuntime.disposeProfile).toHaveBeenCalledTimes(newTurn ? 2 : 1); + if (newTurn) { + expect(managedRuntime.disposeProfile).toHaveBeenLastCalledWith( + "/tmp/openclaw-managed-worker-state", + "turn-2", + ); + } + harness.input.end(); + await running; + }, + ); + it.each(["owner", "output"] as const)( "closes state when %s ends during a pending result write", async (ending) => { diff --git a/src/worker/worker-command.runtime.ts b/src/worker/worker-command.runtime.ts index eed3ebdecc8d..a478d1e65116 100644 --- a/src/worker/worker-command.runtime.ts +++ b/src/worker/worker-command.runtime.ts @@ -1,12 +1,20 @@ import { realpath } from "node:fs/promises"; import type { Readable, Writable } from "node:stream"; +import { readByteStreamWithLimit } from "@openclaw/media-core/read-byte-stream-with-limit"; import { WORKER_PROTOCOL_MAX_INFERENCE_PAYLOAD_BYTES } from "../../packages/gateway-protocol/src/schema/worker-inference.js"; -import { getActiveBackgroundExecSessionCount } from "../agents/bash-process-registry.js"; +import { + getActiveBackgroundExecSessionCount, + waitForExecScope, +} from "../agents/bash-process-registry.js"; import { toErrorObject } from "../infra/errors.js"; import { createBoundedLineFramer } from "../process/bounded-line-framer.js"; import type { WorkerBrowserRuntime } from "./browser-runtime.js"; import { parseWorkerLaunchDescriptor, type WorkerLaunchDescriptor } from "./launch-descriptor.js"; -import { parseWorkerProcessRequest, type WorkerProcessResult } from "./worker-process-protocol.js"; +import { + parseWorkerProcessRequest, + type WorkerProcessMessage, + type WorkerProcessResult, +} from "./worker-process-protocol.js"; import { createWorkerRuntimeEnvironment, runWorkerDescriptor } from "./worker.runtime.js"; type RunWorkerCommandOptions = { @@ -25,6 +33,13 @@ export type WorkerCommandLifetime = { terminateOwnedTree: () => void; }; +function workerInputBytes(raw: unknown, label: string): Buffer { + if (typeof raw === "string" || raw instanceof Uint8Array) { + return Buffer.from(raw); + } + throw new Error(`${label} input must be bytes`); +} + async function runManagedWorkerCommand( options: RunWorkerCommandOptions, signal: AbortSignal, @@ -34,6 +49,8 @@ async function runManagedWorkerCommand( let lastTurnId: string | undefined; let active: { turnId: string; controller: AbortController } | undefined; let running: Promise | undefined; + let idleCleanup: Promise | undefined; + let draining: Promise | undefined; let closed = false; const framer = createBoundedLineFramer( WORKER_PROTOCOL_MAX_INFERENCE_PAYLOAD_BYTES, @@ -59,6 +76,24 @@ async function runManagedWorkerCommand( resolve(); } }; + const write = (response: WorkerProcessMessage) => + new Promise((resolveWrite, rejectWrite) => { + const onClose = () => rejectWrite(new Error("managed worker result output closed")); + // A failed write reports through both the callback and the stream. The callback owns + // the result; retain one listener to consume the matching runtime error event. + const onError = () => {}; + options.output.once("close", onClose); + options.output.once("error", onError); + options.output.write(`${JSON.stringify(response)}\n`, (error) => { + options.output.off("close", onClose); + if (error) { + rejectWrite(error); + } else { + options.output.off("error", onError); + resolveWrite(); + } + }); + }); const onLine = (line: Buffer) => { let value: unknown; try { @@ -81,8 +116,7 @@ async function runManagedWorkerCommand( lastTurnId = request.turnId; const current = { turnId: request.turnId, controller: new AbortController() }; active = current; - running = (async () => { - const descriptor = request.descriptor; + running = (async (descriptor: WorkerLaunchDescriptor, idleRetention?: true) => { const workspaceDir = await realpath(descriptor.assignment.workspaceDir); const workerContainmentRoot = await realpath( descriptor.assignment.workerContainmentRoot ?? workspaceDir, @@ -104,6 +138,7 @@ async function runManagedWorkerCommand( return; } environment ??= await createWorkerRuntimeEnvironment(descriptor.admission.sessionId); + await idleCleanup; if (closed) { return; } @@ -113,12 +148,7 @@ async function runManagedWorkerCommand( assignment: descriptor.assignment.permissionMode === undefined ? { ...descriptor.assignment, workspaceDir } - : { - ...descriptor.assignment, - workspaceDir, - permissionMode: descriptor.assignment.permissionMode, - workerContainmentRoot, - }, + : { ...descriptor.assignment, workspaceDir, workerContainmentRoot }, }, { environmentStateDir: environment.stateDir, @@ -132,55 +162,65 @@ async function runManagedWorkerCommand( if (closed) { return; } - const retainWorker = - (result.status === "completed" || result.status === "failed") && - getActiveBackgroundExecSessionCount() > 0; - const response: WorkerProcessResult = { + const stateDir = environment.stateDir; + const scopeKey = `worker:${descriptor.admission.sessionId}`; + const disposeProfile = async () => { + const { disposeWorkerGitHubEnvironment } = await import("./github-binding.runtime.js"); + await disposeWorkerGitHubEnvironment(stateDir, descriptor.assignment.turnId); + }; + const canRetain = result.status === "completed" || result.status === "failed"; + let retention: WorkerProcessResult["retention"] = + canRetain && getActiveBackgroundExecSessionCount() > 0 + ? "background" + : canRetain && idleRetention && !current.controller.signal.aborted + ? "idle" + : undefined; + if (retention === "idle") { + await waitForExecScope(scopeKey); + await disposeProfile(); + if (current.controller.signal.aborted) { + retention = undefined; + } + } + if (closed) { + return; + } + const retainWorker = retention !== undefined; + if (retainWorker) { + active = undefined; + } + await write({ type: "result", turnId: current.turnId, result, retainWorker, - }; - if (retainWorker) { - active = undefined; - } - await new Promise((resolveWrite, rejectWrite) => { - const onClose = () => rejectWrite(new Error("managed worker result output closed")); - // A failed write reports through both the callback and the stream. The callback owns - // the result; retain one listener to consume the matching runtime error event. - const onError = () => {}; - options.output.once("close", onClose); - options.output.once("error", onError); - options.output.write(`${JSON.stringify(response)}\n`, (error) => { - options.output.off("close", onClose); - if (error) { - rejectWrite(error); - } else { - options.output.off("error", onError); - resolveWrite(); - } - }); + ...(idleRetention && retention ? { retention } : {}), }); + if (retention === "background") { + const isCurrent = () => !closed && !active && lastTurnId === current.turnId; + draining = Promise.all([draining, waitForExecScope(scopeKey)]) + .then(async () => { + // Remove this turn's profile even if a newer turn is already running. + await (idleCleanup = disposeProfile()); + if (idleRetention && isCurrent()) { + await write({ type: "idle-ready", turnId: current.turnId }); + } + }) + .catch(finish); + } + if (!retainWorker) { active = undefined; finish(); } - })().catch(finish); + })(request.descriptor, request.idleRetention).catch(finish); }; const onData = (raw: unknown) => { if (closed) { return; } try { - const chunk = - typeof raw === "string" - ? Buffer.from(raw) - : raw instanceof Uint8Array - ? Buffer.from(raw) - : undefined; - if (!chunk) { - throw new Error("managed worker input must be bytes"); - } + const chunk = workerInputBytes(raw, "managed worker"); for (const line of framer.push(chunk)) { onLine(line); if (closed) { @@ -218,6 +258,7 @@ async function runManagedWorkerCommand( try { await running; await environment?.close(); + await draining; } finally { removeListeners(); } @@ -225,30 +266,19 @@ async function runManagedWorkerCommand( } async function readLaunchDescriptor(input: Readable): Promise { - const chunks: Buffer[] = []; - let byteLength = 0; - for await (const rawChunk of input as AsyncIterable) { - const chunk = - typeof rawChunk === "string" - ? Buffer.from(rawChunk) - : rawChunk instanceof Uint8Array - ? Buffer.from(rawChunk) - : undefined; - if (!chunk) { - throw new Error("worker launch descriptor input must be bytes"); - } - byteLength += chunk.byteLength; - if (byteLength > WORKER_PROTOCOL_MAX_INFERENCE_PAYLOAD_BYTES) { - throw new Error("worker launch descriptor exceeds the protocol payload limit"); - } - chunks.push(chunk); - } - if (byteLength === 0) { + const bytes = await readByteStreamWithLimit( + input.map((chunk: unknown) => workerInputBytes(chunk, "worker launch descriptor")), + { + maxBytes: WORKER_PROTOCOL_MAX_INFERENCE_PAYLOAD_BYTES, + onOverflow: () => new Error("worker launch descriptor exceeds the protocol payload limit"), + }, + ); + if (bytes.length === 0) { throw new Error("worker launch descriptor is required on stdin"); } let decoded: unknown; try { - decoded = JSON.parse(Buffer.concat(chunks).toString("utf8")) as unknown; + decoded = JSON.parse(bytes.toString("utf8")); } catch (error) { throw new Error("worker launch descriptor is not valid JSON", { cause: error }); } diff --git a/src/worker/worker-process-protocol.test.ts b/src/worker/worker-process-protocol.test.ts index d7b0205494a5..7fa49198a024 100644 --- a/src/worker/worker-process-protocol.test.ts +++ b/src/worker/worker-process-protocol.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { + parseWorkerProcessMessage, parseWorkerProcessRequest, - parseWorkerProcessResult, parseWorkerRuntimeResult, } from "./worker-process-protocol.js"; @@ -21,11 +21,28 @@ describe("worker process protocol", () => { ])("only retains workers after started $status results", (result) => { expect(parseWorkerRuntimeResult(result)).toStrictEqual(result); const frame = { type: "result", turnId: "turn-1", result, retainWorker: false }; - expect(parseWorkerProcessResult(frame)).toStrictEqual(frame); + expect(parseWorkerProcessMessage(frame)).toStrictEqual(frame); const retained = { ...frame, retainWorker: true }; - expect(parseWorkerProcessResult(retained)).toStrictEqual( + expect(parseWorkerProcessMessage(retained)).toStrictEqual( result.status === "completed" || result.status === "failed" ? retained : null, ); + for (const retention of ["background", "idle"]) { + const negotiated = { ...retained, retention }; + expect(parseWorkerProcessMessage(negotiated)).toStrictEqual( + result.status === "completed" || result.status === "failed" ? negotiated : null, + ); + expect(parseWorkerProcessMessage({ ...frame, retention })).toBeNull(); + } + }); + + it("accepts only exact idle readiness for a bounded turn identity", () => { + const ready = { type: "idle-ready", turnId: "turn-1" }; + expect(parseWorkerProcessMessage(ready)).toEqual(ready); + expect(parseWorkerProcessMessage({ ...ready, retainWorker: true })).toBeNull(); + expect(parseWorkerProcessMessage({ ...ready, turnId: "" })).toBeNull(); + expect( + parseWorkerProcessMessage(inheritedRecord({ type: "idle-ready" }, { turnId: "turn-1" })), + ).toBeNull(); }); it("rejects request discriminators inherited alongside the wrong own keys", () => { @@ -83,6 +100,6 @@ describe("worker process protocol", () => { }, ); - expect(parseWorkerProcessResult(result)).toBeNull(); + expect(parseWorkerProcessMessage(result)).toBeNull(); }); }); diff --git a/src/worker/worker-process-protocol.ts b/src/worker/worker-process-protocol.ts index 5da24d4e08fd..1d4bb53c2744 100644 --- a/src/worker/worker-process-protocol.ts +++ b/src/worker/worker-process-protocol.ts @@ -13,19 +13,30 @@ import { WORKER_CONNECTION_ENDPOINT_MAX_JSON_BYTES } from "./worker-connection-e /** Private JSONL protocol between one node supervisor and its environment-owned worker. */ export type WorkerProcessInput = - | { type: "turn"; turnId: string; descriptor: WorkerLaunchDescriptor } + | { type: "turn"; turnId: string; descriptor: WorkerLaunchDescriptor; idleRetention?: true } | { type: "cancel"; turnId: string }; -export function buildWorkerProcessTurn(descriptor: T) { - return { type: "turn" as const, turnId: descriptor.assignment.turnId, descriptor }; +export function buildWorkerProcessTurn( + descriptor: T, + idleRetention = false, +) { + return { + type: "turn" as const, + turnId: descriptor.assignment.turnId, + descriptor, + ...(idleRetention ? { idleRetention: true as const } : {}), + }; } -export function measureWorkerProcessTurnBytes(plan: WorkerLaunchPlan): number { +export function measureWorkerProcessTurnBytes( + plan: WorkerLaunchPlan, + idleRetention = false, +): number { // The node supplies the endpoint privately. Replace only its JSON null placeholder // with the parser-owned bound; the managed envelope is the sender's exact shape. return ( Buffer.byteLength( - JSON.stringify(buildWorkerProcessTurn({ ...plan, connectionEndpoint: null })), + JSON.stringify(buildWorkerProcessTurn({ ...plan, connectionEndpoint: null }, idleRetention)), ) - "null".length + WORKER_CONNECTION_ENDPOINT_MAX_JSON_BYTES @@ -57,22 +68,33 @@ const RuntimeResultSchema = z.union([ ...TranscriptResultFields, }), ]); +const TurnIdSchema = z + .string() + .refine( + (value) => Boolean(value.trim()) && value.length <= WORKER_PROTOCOL_MAX_IDENTIFIER_LENGTH, + ); const ProcessResultSchema = workerProtocolObject({ type: z.literal("result"), - turnId: z - .string() - .refine( - (value) => Boolean(value.trim()) && value.length <= WORKER_PROTOCOL_MAX_IDENTIFIER_LENGTH, - ), + turnId: TurnIdSchema, result: RuntimeResultSchema, retainWorker: z.boolean(), + retention: z.enum(["background", "idle"]).optional(), }).refine( - ({ result, retainWorker }) => - !retainWorker || result.status === "completed" || result.status === "failed", + ({ result, retainWorker, retention }) => + (retention === undefined || retainWorker) && + (!retainWorker || result.status === "completed" || result.status === "failed"), ); +const ProcessMessageSchema = z.union([ + ProcessResultSchema, + workerProtocolObject({ + type: z.literal("idle-ready"), + turnId: TurnIdSchema, + }), +]); export type WorkerRuntimeResult = z.infer; export type WorkerProcessResult = z.infer; +export type WorkerProcessMessage = z.infer; export function parseWorkerProcessRequest(value: unknown): WorkerProcessInput { if ( @@ -86,12 +108,16 @@ export function parseWorkerProcessRequest(value: unknown): WorkerProcessInput { if (value.type === "cancel" && hasExactOwnKeys(value, ["type", "turnId"])) { return { type: "cancel", turnId: value.turnId }; } - if (value.type === "turn" && hasExactOwnKeys(value, ["type", "turnId", "descriptor"])) { + if ( + value.type === "turn" && + hasExactOwnKeys(value, ["type", "turnId", "descriptor"], ["idleRetention"]) && + (value.idleRetention === undefined || value.idleRetention === true) + ) { const descriptor = parseWorkerLaunchDescriptor(value.descriptor); if (descriptor.assignment.turnId !== value.turnId) { throw new Error("managed worker request disagrees with its assigned turn"); } - return { type: "turn", turnId: value.turnId, descriptor }; + return buildWorkerProcessTurn(descriptor, value.idleRetention === true); } throw new Error("invalid managed worker request"); } @@ -101,7 +127,7 @@ export function parseWorkerRuntimeResult(value: unknown): WorkerRuntimeResult | return parsed.success ? parsed.data : null; } -export function parseWorkerProcessResult(value: unknown): WorkerProcessResult | null { - const parsed = ProcessResultSchema.safeParse(value); +export function parseWorkerProcessMessage(value: unknown): WorkerProcessMessage | null { + const parsed = ProcessMessageSchema.safeParse(value); return parsed.success ? parsed.data : null; } diff --git a/src/worker/worker-runtime-background-exec.suite.ts b/src/worker/worker-runtime-background-exec.suite.ts index f513a3deeb6b..197e150ec89b 100644 --- a/src/worker/worker-runtime-background-exec.suite.ts +++ b/src/worker/worker-runtime-background-exec.suite.ts @@ -31,7 +31,7 @@ import { getProcessSupervisor } from "../process/supervisor/index.js"; import type { WorkerLaunchDescriptor } from "./launch-descriptor.js"; import type { NodeWorkerLaunchInput } from "./node-supervisor-protocol.js"; import { runWorkerCommand } from "./worker-command.runtime.js"; -import { parseWorkerProcessResult, type WorkerProcessResult } from "./worker-process-protocol.js"; +import { parseWorkerProcessMessage, type WorkerProcessResult } from "./worker-process-protocol.js"; import { workerBackgroundExecEntrypoints } from "./worker-runtime-background-exec-entrypoints.test-support.js"; const workerProcessUrl = resolveRuntimeWorkerUrl(workerBackgroundExecEntrypoints.worker); @@ -374,8 +374,8 @@ export function registerWorkerBackgroundExecLifecycleTests({ const output = new PassThrough(); const result = createDeferred(); output.on("data", (chunk: Buffer) => { - const parsed = parseWorkerProcessResult(JSON.parse(chunk.toString("utf8"))); - if (parsed) { + const parsed = parseWorkerProcessMessage(JSON.parse(chunk.toString("utf8"))); + if (parsed?.type === "result") { result.resolve(parsed); } }); diff --git a/src/worker/worker.runtime.test.ts b/src/worker/worker.runtime.test.ts index fa4ee3456aca..382111ab133e 100644 --- a/src/worker/worker.runtime.test.ts +++ b/src/worker/worker.runtime.test.ts @@ -75,7 +75,11 @@ import { WorkerConnectionStoppedError, } from "./worker-connection-contract.js"; import { createWorkerConnection, type WorkerConnectionState } from "./worker-connection.js"; -import { parseWorkerProcessResult, type WorkerProcessResult } from "./worker-process-protocol.js"; +import { + buildWorkerProcessTurn, + parseWorkerProcessMessage, + type WorkerProcessResult, +} from "./worker-process-protocol.js"; import { WorkerInferenceProxyClient } from "./worker-rpc-inference-client.js"; import { WorkerLiveEventClient } from "./worker-rpc-live-event-client.js"; import { WorkerTranscriptCommitClient } from "./worker-rpc-transcript-client.js"; @@ -1854,8 +1858,8 @@ describe("worker runtime", () => { const output = new PassThrough(); const results: WorkerProcessResult[] = []; output.on("data", (chunk: Buffer) => { - const result = parseWorkerProcessResult(JSON.parse(chunk.toString("utf8"))); - if (result) { + const result = parseWorkerProcessMessage(JSON.parse(chunk.toString("utf8"))); + if (result?.type === "result") { results.push(result); } }); @@ -2112,7 +2116,7 @@ describe("worker runtime", () => { const profileDir = path.join( environment.stateDir, "github-profiles", - createHash("sha256").update(launch.assignment.runId).digest("hex").slice(0, 16), + createHash("sha256").update(launch.assignment.turnId).digest("hex").slice(0, 16), ); const output = toolResult?.content @@ -2171,10 +2175,13 @@ describe("worker runtime", () => { const input = new PassThrough(); const output = new PassThrough(); const results: WorkerProcessResult[] = []; + const idleReady = createDeferred(); output.on("data", (chunk: Buffer) => { - const result = parseWorkerProcessResult(JSON.parse(chunk.toString("utf8"))); - if (result) { - results.push(result); + const message = parseWorkerProcessMessage(JSON.parse(chunk.toString("utf8"))); + if (message?.type === "result") { + results.push(message); + } else if (message?.type === "idle-ready") { + idleReady.resolve(message.turnId); } }); const command = runWorkerCommand({ managed: true, input, output }); @@ -2183,9 +2190,7 @@ describe("worker runtime", () => { const scopeKey = `worker:${SESSION_ID}`; const supervisor = getProcessSupervisor(); try { - input.write( - `${JSON.stringify({ type: "turn", turnId: launch.assignment.turnId, descriptor: launch })}\n`, - ); + input.write(`${JSON.stringify(buildWorkerProcessTurn(launch, true))}\n`); await waitForFast(() => expect(results).toHaveLength(1), { timeout: 30_000 }); expect(results[0]).toMatchObject({ turnId: launch.assignment.turnId, @@ -2217,9 +2222,7 @@ describe("worker runtime", () => { token: "worker-turn-b-token", branch: "openclaw/session-fixture", }; - input.write( - `${JSON.stringify({ type: "turn", turnId: next.assignment.turnId, descriptor: next })}\n`, - ); + input.write(`${JSON.stringify(buildWorkerProcessTurn(next, true))}\n`); await waitForFast(() => expect(results).toHaveLength(2), { timeout: 30_000 }); expect(results[1]).toMatchObject({ turnId: next.assignment.turnId, @@ -2235,24 +2238,25 @@ describe("worker runtime", () => { }); expect(settled).not.toHaveBeenCalled(); - await writeFile(path.join(workspaceDir, "retained-marker"), "read"); - const retainedRead = await waitForFast(() => - readFile(path.join(workspaceDir, "retained-read.txt"), "utf8"), - ); - expect(retainedRead).not.toContain(launch.assignment.github.token); - expect(retainedRead).not.toContain(next.assignment.github.token); - expect(retainedRead).toMatch(/No such file|ENOENT/u); - expect(retainedRead).toMatch(/exit=[1-9]\d*/u); - await expect(stat(previousProfileDir)).rejects.toMatchObject({ code: "ENOENT" }); const nextProfileDir = path.join( stateDir, "github-profiles", - createHash("sha256").update(next.assignment.runId).digest("hex").slice(0, 16), + createHash("sha256").update(next.assignment.turnId).digest("hex").slice(0, 16), ); const hosts = await readFile(path.join(nextProfileDir, "hosts.yml"), "utf8"); expect(hosts).toContain("worker-b"); expect(hosts).not.toContain("worker-a"); expect(hosts).not.toContain(launch.assignment.github.token); + await writeFile(path.join(workspaceDir, "retained-marker"), "read"); + const retainedRead = await waitForFast(() => + readFile(path.join(workspaceDir, "retained-read.txt"), "utf8"), + ); + expect(retainedRead).toContain(launch.assignment.github.token); + expect(retainedRead).not.toContain(next.assignment.github.token); + expect(retainedRead).toMatch(/exit=0/u); + expect(await idleReady.promise).toBe(next.assignment.turnId); + await expect(stat(previousProfileDir)).rejects.toMatchObject({ code: "ENOENT" }); + await expect(stat(nextProfileDir)).rejects.toMatchObject({ code: "ENOENT" }); } finally { input.end(); try { diff --git a/src/worker/worker.runtime.ts b/src/worker/worker.runtime.ts index 665bc90afc1f..f5bd421f9d33 100644 --- a/src/worker/worker.runtime.ts +++ b/src/worker/worker.runtime.ts @@ -237,7 +237,7 @@ export async function runWorkerDescriptor( prepareWorkerGitHubEnvironment({ binding: descriptor.assignment.github!, stateDir, - runId: descriptor.assignment.runId, + turnId: descriptor.assignment.turnId, cwd: workspaceDir, signal: abortController.signal, }), diff --git a/test/e2e/qa-lab/runtime/paired-node-worker-wire-fixture.ts b/test/e2e/qa-lab/runtime/paired-node-worker-wire-fixture.ts index 4a2aae6d8827..244b8b4215ec 100644 --- a/test/e2e/qa-lab/runtime/paired-node-worker-wire-fixture.ts +++ b/test/e2e/qa-lab/runtime/paired-node-worker-wire-fixture.ts @@ -321,7 +321,7 @@ export async function createPairedNodeWorkerHost( import("../../../../src/infra/device-identity.js"), import("../../../../src/node-host/invoke.js"), import("../../../../src/node-host/node-worker-bundle-installer.js"), - import("../../../../src/node-host/node-worker-supervisor-contract.js"), + import("../../../../src/worker/node-supervisor-protocol.js"), import("../../../../src/node-host/node-worker-supervisor.js"), import("../../../../src/node-host/node-worker-workspace.js"), ]); diff --git a/ui/src/pages/new-session/device-placement.ts b/ui/src/pages/new-session/device-placement.ts index c0d7034ace10..de31dc9d9795 100644 --- a/ui/src/pages/new-session/device-placement.ts +++ b/ui/src/pages/new-session/device-placement.ts @@ -1,3 +1,4 @@ +import { availableWorkerSlots } from "../../../../src/shared/node-list-parse.js"; import { t } from "../../i18n/index.ts"; import { registerNewSessionSetupEnglish } from "../../i18n/locales/en-new-session-setup.ts"; import type { DraftEnvironment } from "./discovery.ts"; @@ -67,7 +68,9 @@ function unavailableReason( if (!environment.workerSlots) { return t("newSession.deviceCapacityUnavailable"); } - return environment.workerSlots.available === 0 ? t("newSession.deviceNoSlots") : undefined; + return availableWorkerSlots(environment.workerSlots) === 0 + ? t("newSession.deviceNoSlots") + : undefined; } /** One projection owns device presentation, restore eligibility, and submit eligibility. */ diff --git a/ui/src/pages/new-session/discovery.ts b/ui/src/pages/new-session/discovery.ts index 7893ab30314b..67808045b5f9 100644 --- a/ui/src/pages/new-session/discovery.ts +++ b/ui/src/pages/new-session/discovery.ts @@ -13,6 +13,7 @@ import type { WorkerOperatingSystem, WorkerSlotSummary, } from "../../../../packages/gateway-protocol/src/schema/environments.ts"; +import { parseWorkerSlotSummary } from "../../../../src/shared/node-list-parse.js"; export type DraftBranches = { repoRoot: string; @@ -247,26 +248,6 @@ function isEnvironmentStatus(value: unknown): value is EnvironmentStatus { return typeof value === "string" && ENVIRONMENT_STATUSES.has(value); } -function isSafeInteger(value: unknown): value is number { - return typeof value === "number" && Number.isSafeInteger(value); -} - -function readWorkerSlots(value: unknown): WorkerSlotSummary | undefined { - if ( - !isRecord(value) || - Object.keys(value).some((key) => key !== "total" && key !== "available") || - !isSafeInteger(value.total) || - !isSafeInteger(value.available) - ) { - return undefined; - } - const total = value.total; - const available = value.available; - return total >= 1 && total <= 1_024 && available >= 0 && available <= total - ? { total, available } - : undefined; -} - function readRequiredNodeCommand(value: unknown): RequiredNodeCommand | undefined { if (!isRecord(value) || Object.keys(value).some((key) => key !== "command" && key !== "state")) { return undefined; @@ -335,7 +316,7 @@ export function readDraftEnvironments(value: unknown): DraftEnvironment[] { const lastSeenAtMs = normalizeTimestamp(environment.lastSeenAtMs); const lastSeenReason = normalizeOptionalString(environment.lastSeenReason); const issues = readRuntimeTargetIssues(environment.issues); - const workerSlots = readWorkerSlots(environment.workerSlots); + const workerSlots = parseWorkerSlotSummary(environment.workerSlots); return [ { id, diff --git a/ui/src/pages/new-session/where-chip.test.ts b/ui/src/pages/new-session/where-chip.test.ts index f276d27f4a98..c6db14dcd358 100644 --- a/ui/src/pages/new-session/where-chip.test.ts +++ b/ui/src/pages/new-session/where-chip.test.ts @@ -895,6 +895,15 @@ describe("Where chip", () => { reason: "No worker slots are available. Wait for a slot or pick another device.", label: "Slot utilization unavailable", }, + { + name: "admits worker execution by reclaiming the sole idle worker slot", + devicePlacement: { requiredNodeCommands: [], consumesWorkerSlot: true }, + workerSlots: { total: 1, available: 0, reclaimableIdle: 1 }, + invocableCommands: [], + commandState: undefined, + disabled: false, + label: "1 of 1 session slots in use", + }, { name: "disables a declared remote command that the Gateway has not enabled", devicePlacement: {