diff --git a/src/agents/embedded-agent-runner/run/lane-controller.group-wait.test.ts b/src/agents/embedded-agent-runner/run/lane-controller.group-wait.test.ts new file mode 100644 index 000000000000..d5119f09dfc0 --- /dev/null +++ b/src/agents/embedded-agent-runner/run/lane-controller.group-wait.test.ts @@ -0,0 +1,153 @@ +/** + * Group-blocked waits must be visible to the cron setup watchdog. + * + * Round-4 review (fiducian-spencer-001) asked for the 8-cron / hook-holding- + * reserve / 8th-cron-waiting regression asserting no false setup timeout. The + * chain spans three files: + * + * lane-controller.noteLaneWaitIfBusy -- emits onLaneWait({waiting:true}) + * -> timer-job-runner.noteLaneState -- maps it to the watchdog + * -> agent-watchdog.noteLaneWait() -- sets waitingForLane, clears timeout + * -> agent-watchdog:98 -- suppresses the setup timeout + * + * The watchdog end is already covered by agent-watchdog.test.ts. The link the + * capacity-group change introduced is the FIRST one, and it is the one that can + * silently fail: a group-blocked lane looks idle to a lane-local view, so the + * predicate returns false, no wait is ever reported, and a healthy run queued + * behind group capacity takes a false setup timeout. + * + * These tests drive the real predicate with real snapshots from a real group. + */ +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { + enqueueCommandInLane, + getCommandLaneSnapshot, + publishLaneConfiguration, + resetAllLanes, + setCommandLaneConcurrency, +} from "../../../process/command-queue.js"; +import { shouldNoteLaneWait } from "./lane-runtime.js"; + +const CRON = "cron-nested"; +const HOOK = "hook-dispatch"; +const GROUP = "cron-hooks"; + +type LaneGroupSpec = NonNullable[0]["groups"]>[string]; + +function setCommandLaneGroup(group: string, spec: LaneGroupSpec): void { + publishLaneConfiguration({ groups: { [group]: spec } }); +} + +function clearCommandLaneGroup(group: string): void { + publishLaneConfiguration({ clearGroups: [group] }); +} + +function gate() { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; +} + +async function settle(): Promise { + for (let i = 0; i < 5; i++) { + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + } +} + +beforeEach(() => { + resetAllLanes(); + clearCommandLaneGroup(GROUP); + setCommandLaneConcurrency(CRON, 8); + setCommandLaneConcurrency(HOOK, 8); + setCommandLaneGroup(GROUP, { + budget: 8, + members: [CRON, HOOK], + reservations: { [HOOK]: 1 }, + }); +}); + +afterEach(() => { + clearCommandLaneGroup(GROUP); + resetAllLanes(); +}); + +describe("group-blocked lane waits are reported", () => { + test("8th cron run blocked by the hook's reserve reports a wait", async () => { + // 7 cron active; the 8th slot is the hook's hard reservation. + const gates = Array.from({ length: 7 }, () => gate()); + const runs = gates.map((g) => enqueueCommandInLane(CRON, async () => await g.promise)); + await settle(); + + const snapshot = getCommandLaneSnapshot(CRON); + // This is the state that defeats a lane-local predicate: under its own + // maxConcurrent, nothing queued, yet unable to start. + expect(snapshot.activeCount).toBe(7); + expect(snapshot.maxConcurrent).toBe(8); + expect(snapshot.queuedCount).toBe(0); + expect(snapshot.queuedCount > 0 || snapshot.activeCount >= snapshot.maxConcurrent).toBe(false); + + // ...and the predicate must still report the wait, or the watchdog never + // suppresses its setup timeout and the run fails spuriously. + expect(shouldNoteLaneWait(snapshot)).toBe(true); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + test("a hook blocked by group budget reports a wait", async () => { + // Fill the group entirely, including the hook's own reserved slot. + const cronGates = Array.from({ length: 7 }, () => gate()); + const cronRuns = cronGates.map((g) => enqueueCommandInLane(CRON, async () => await g.promise)); + const hookGate = gate(); + const hookRun = enqueueCommandInLane(HOOK, async () => await hookGate.promise); + await settle(); + + // A second hook cannot start because the group budget is full. + expect(shouldNoteLaneWait(getCommandLaneSnapshot(HOOK))).toBe(true); + + hookGate.release(); + await hookRun; + for (const g of cronGates) { + g.release(); + } + await Promise.all(cronRuns); + }); + + test("no wait is reported when the lane can start immediately", async () => { + // The negative control. Without it, a predicate hardcoded to `true` would + // pass both tests above. + expect(shouldNoteLaneWait(getCommandLaneSnapshot(CRON))).toBe(false); + expect(shouldNoteLaneWait(getCommandLaneSnapshot(HOOK))).toBe(false); + + const g = gate(); + const run = enqueueCommandInLane(CRON, async () => await g.promise); + await settle(); + // One active out of eight: still admits, still no wait. + expect(shouldNoteLaneWait(getCommandLaneSnapshot(CRON))).toBe(false); + + g.release(); + await run; + }); + + test("waits are still reported for ordinary lane-local saturation", async () => { + // The pre-existing behaviour must survive the predicate change. + clearCommandLaneGroup(GROUP); + setCommandLaneConcurrency("ungrouped", 1); + const g = gate(); + const run = enqueueCommandInLane("ungrouped", async () => await g.promise); + await settle(); + + const snapshot = getCommandLaneSnapshot("ungrouped"); + expect(snapshot.activeCount).toBe(1); + expect(shouldNoteLaneWait(snapshot)).toBe(true); + + g.release(); + await run; + }); +}); diff --git a/src/agents/embedded-agent-runner/run/lane-controller.ts b/src/agents/embedded-agent-runner/run/lane-controller.ts index f589c8fd3a71..97364de0c39e 100644 --- a/src/agents/embedded-agent-runner/run/lane-controller.ts +++ b/src/agents/embedded-agent-runner/run/lane-controller.ts @@ -13,6 +13,7 @@ import { EMBEDDED_RUN_LANE_TIMEOUT_GRACE_MS, resolveEmbeddedRunLaneTimeoutMs, resolveEmbeddedRunSessionQueuePriority, + shouldNoteLaneWait, withEmbeddedRunLaneTimeout, } from "./lane-runtime.js"; import type { RunEmbeddedAgentParams } from "./params.js"; @@ -90,7 +91,7 @@ export function createEmbeddedRunLaneController(opti return; } const snapshot = getCommandLaneSnapshot(lane); - if (snapshot.queuedCount > 0 || snapshot.activeCount >= snapshot.maxConcurrent) { + if (shouldNoteLaneWait(snapshot)) { params.onLaneWait({ waitMs: 0, queuedAhead: snapshot.queuedCount + snapshot.activeCount, diff --git a/src/agents/embedded-agent-runner/run/lane-runtime.ts b/src/agents/embedded-agent-runner/run/lane-runtime.ts index 40d2326b1052..5df6bcf8379d 100644 --- a/src/agents/embedded-agent-runner/run/lane-runtime.ts +++ b/src/agents/embedded-agent-runner/run/lane-runtime.ts @@ -2,6 +2,7 @@ import { addTimerTimeoutGraceMs, MAX_TIMER_TIMEOUT_MS, } from "@openclaw/normalization-core/number-coercion"; +import type { CommandLaneSnapshot } from "../../../process/command-queue.js"; import type { CommandQueueEnqueueOptions } from "../../../process/command-queue.types.js"; import { isMainSessionRestartRecoveryInputProvenance } from "../../../sessions/input-provenance.js"; import { DEFAULT_AGENT_TIMEOUT_MS } from "../../timeout.js"; @@ -10,6 +11,14 @@ import type { RunEmbeddedAgentParams } from "./params.js"; export const EMBEDDED_RUN_LANE_TIMEOUT_GRACE_MS = 30_000; export const EMBEDDED_RUN_LANE_HEARTBEAT_MS = EMBEDDED_RUN_LANE_TIMEOUT_GRACE_MS / 2; +export function shouldNoteLaneWait(snapshot: CommandLaneSnapshot): boolean { + return ( + snapshot.queuedCount > 0 || + snapshot.activeCount >= snapshot.maxConcurrent || + snapshot.blockedBy != null + ); +} + export async function withEmbeddedRunLaneProgressHeartbeat( noteLaneTaskProgress: () => void, fn: () => Promise, diff --git a/src/agents/session-suspension.test.ts b/src/agents/session-suspension.test.ts index 6fe900c6e08a..224b156815a3 100644 --- a/src/agents/session-suspension.test.ts +++ b/src/agents/session-suspension.test.ts @@ -88,6 +88,77 @@ describe("session suspension", () => { ); }); + it("auto-resumes hook dispatch to the shared cron concurrency width", async () => { + vi.useFakeTimers(); + + await suspendLane(100, {} as OpenClawConfig, CommandLane.HookDispatch); + + expect(commandQueueMocks.setCommandLaneConcurrency).toHaveBeenCalledWith( + CommandLane.HookDispatch, + 0, + ); + + await vi.advanceTimersByTimeAsync(100); + + expect(commandQueueMocks.setCommandLaneConcurrency).toHaveBeenLastCalledWith( + CommandLane.HookDispatch, + DEFAULT_CRON_MAX_CONCURRENT_RUNS, + ); + }); + + it("retargets a suspended hook lane when hooks are disabled before its TTL", async () => { + vi.useFakeTimers(); + const { getSuspendedLaneIdsForGatewayPublication, setGatewayLaneResumeConcurrencies } = + await import("./session-suspension.js"); + + await suspendLane(100, {} as OpenClawConfig, CommandLane.HookDispatch); + + setGatewayLaneResumeConcurrencies({ [CommandLane.HookDispatch]: 0 }); + expect(getSuspendedLaneIdsForGatewayPublication()).toEqual(new Set([CommandLane.HookDispatch])); + + commandQueueMocks.setCommandLaneConcurrency.mockClear(); + await vi.advanceTimersByTimeAsync(100); + + expect(commandQueueMocks.setCommandLaneConcurrency).toHaveBeenCalledExactlyOnceWith( + CommandLane.HookDispatch, + 0, + ); + }); + + it("uses hooks-off concurrency when a pending suspension write finishes late", async () => { + vi.useFakeTimers(); + const { setGatewayLaneResumeConcurrencies } = await import("./session-suspension.js"); + let resolvePatch: (() => void) | undefined; + sessionAccessorMocks.patchSessionEntry.mockImplementationOnce(async (_scope, update) => { + await new Promise((resolve) => { + resolvePatch = resolve; + }); + return update({}); + }); + + const suspension = suspendLane(100, {} as OpenClawConfig, CommandLane.HookDispatch); + await vi.waitFor(() => { + expect(resolvePatch).toBeTypeOf("function"); + }); + + setGatewayLaneResumeConcurrencies({ [CommandLane.HookDispatch]: 0 }); + resolvePatch?.(); + await suspension; + + expect(commandQueueMocks.setCommandLaneConcurrency).toHaveBeenLastCalledWith( + CommandLane.HookDispatch, + 0, + ); + commandQueueMocks.setCommandLaneConcurrency.mockClear(); + + await vi.advanceTimersByTimeAsync(100); + + expect(commandQueueMocks.setCommandLaneConcurrency).toHaveBeenCalledExactlyOnceWith( + CommandLane.HookDispatch, + 0, + ); + }); + it("clamps oversized suspension TTLs for timers and persisted resume time", async () => { // Persisted expectedResumeBy must match the clamped timer, not MAX_SAFE_INTEGER. vi.useFakeTimers(); diff --git a/src/agents/session-suspension.ts b/src/agents/session-suspension.ts index 53fa5eda96e5..cbfcbccf31c3 100644 --- a/src/agents/session-suspension.ts +++ b/src/agents/session-suspension.ts @@ -40,6 +40,7 @@ type ClearedLaneResume = { type SessionSuspensionRuntimeState = { laneResumeTimers: Map; clearedLaneResumes: Map; + gatewayLaneResumeConcurrencies: Map; pendingSuspensionWrites: Map< string, { @@ -66,6 +67,7 @@ function getSessionSuspensionState(): SessionSuspensionRuntimeState { () => ({ laneResumeTimers: new Map(), clearedLaneResumes: new Map(), + gatewayLaneResumeConcurrencies: new Map(), pendingSuspensionWrites: new Map< string, { @@ -83,6 +85,9 @@ function getSessionSuspensionState(): SessionSuspensionRuntimeState { if (!state.clearedLaneResumes) { state.clearedLaneResumes = new Map(); } + if (!state.gatewayLaneResumeConcurrencies) { + state.gatewayLaneResumeConcurrencies = new Map(); + } if (!state.pendingSuspensionWrites) { state.pendingSuspensionWrites = new Map< string, @@ -129,6 +134,7 @@ function resolveLaneResumeConcurrency(cfg: OpenClawConfig | undefined, laneId: s return resolveSubagentMaxConcurrent(cfg); case "cron": case "cron-nested": + case "hook-dispatch": return resolveCronMaxConcurrentRuns(); default: return DEFAULT_CUSTOM_LANE_RESUME_CONCURRENCY; @@ -144,6 +150,7 @@ function isGatewayManagedLane(laneId: string): boolean { lane === CommandLane.Subagent || lane === CommandLane.Cron || lane === CommandLane.CronNested || + lane === CommandLane.HookDispatch || lane === CommandLane.Nested ); } @@ -188,21 +195,31 @@ function scheduleLaneAutoResume( if (existing) { clearTimeout(existing.timer); } + const canonicalResumeConcurrency = isGatewayManagedLane(laneId) + ? (state.gatewayLaneResumeConcurrencies.get(laneId) ?? resumeConcurrency) + : resumeConcurrency; + const entry = { + timer: undefined as unknown as ReturnType, + resumeConcurrency: canonicalResumeConcurrency, + resumeAtMs: nowMs + delayMs, + }; const timer = setTimeout(() => { - if (state.laneResumeTimers.get(laneId)?.timer === timer) { - state.laneResumeTimers.delete(laneId); + if (state.laneResumeTimers.get(laneId) !== entry) { + return; } - setCommandLaneConcurrency(laneId, resumeConcurrency); + state.laneResumeTimers.delete(laneId); + setCommandLaneConcurrency(laneId, entry.resumeConcurrency); log.info("auto-resumed lane after suspension TTL", { laneId, delayMs, - resumeConcurrency, + resumeConcurrency: entry.resumeConcurrency, }); }, delayMs); + entry.timer = timer; if (typeof timer.unref === "function") { timer.unref(); } - state.laneResumeTimers.set(laneId, { timer, resumeConcurrency, resumeAtMs: nowMs + delayMs }); + state.laneResumeTimers.set(laneId, entry); } export function clearSessionSuspensionTimers(): number { @@ -222,38 +239,57 @@ export function clearSessionSuspensionTimers(): number { return cleared; } -export function enableSessionSuspensionTimersForGatewayStart( - resolveResumeConcurrency: (laneId: string, savedResumeConcurrency: number) => number = ( - _laneId, - savedResumeConcurrency, - ) => savedResumeConcurrency, -): Set { +export function enableSessionSuspensionTimersForGatewayStart(): Set { const state = getSessionSuspensionState(); state.cleanupGeneration += 1; state.cleanupActive = false; const suspendedLaneIds = new Set(); const nowMs = Date.now(); for (const [laneId, cleared] of state.clearedLaneResumes) { - const resumeConcurrency = resolveResumeConcurrency(laneId, cleared.resumeConcurrency); const remainingMs = resolveTimerTimeoutMs(cleared.resumeAtMs - nowMs, 0, 0); if (remainingMs > 0) { setCommandLaneConcurrency(laneId, 0); - scheduleLaneAutoResume(laneId, remainingMs, resumeConcurrency, { nowMs }); + scheduleLaneAutoResume(laneId, remainingMs, cleared.resumeConcurrency, { nowMs }); suspendedLaneIds.add(laneId); continue; } if (isGatewayManagedLane(laneId)) { continue; } - setCommandLaneConcurrency(laneId, resumeConcurrency); + setCommandLaneConcurrency(laneId, cleared.resumeConcurrency); } state.clearedLaneResumes.clear(); return suspendedLaneIds; } -export function getCleanupSuspendedLaneIdsForGatewayPublication(): Set { +export function setGatewayLaneResumeConcurrencies( + concurrencies: Readonly>, +): void { + // Gateway publication owns the desired post-suspension widths. Record them + // even when no timer exists yet so an asynchronous suspension write that + // finishes after a config reload cannot schedule a stale resume target. const state = getSessionSuspensionState(); - return state.cleanupActive ? new Set(state.clearedLaneResumes.keys()) : new Set(); + for (const [laneId, rawConcurrency] of Object.entries(concurrencies)) { + if (!isGatewayManagedLane(laneId)) { + continue; + } + const resumeConcurrency = Math.max(0, Math.floor(rawConcurrency)); + state.gatewayLaneResumeConcurrencies.set(laneId, resumeConcurrency); + const activeTimer = state.laneResumeTimers.get(laneId); + if (activeTimer) { + activeTimer.resumeConcurrency = resumeConcurrency; + } + const clearedResume = state.clearedLaneResumes.get(laneId); + if (clearedResume) { + clearedResume.resumeConcurrency = resumeConcurrency; + } + } +} + +export function getSuspendedLaneIdsForGatewayPublication(): Set { + const state = getSessionSuspensionState(); + const suspended = state.cleanupActive ? state.clearedLaneResumes : state.laneResumeTimers; + return new Set(suspended.keys()); } export async function suspendSession(params: SessionSuspensionParams) { @@ -420,6 +456,7 @@ function resetSessionSuspensionStateForTest(): void { } state.laneResumeTimers.clear(); state.clearedLaneResumes.clear(); + state.gatewayLaneResumeConcurrencies.clear(); state.pendingSuspensionWrites.clear(); state.suspensionWriteChain = Promise.resolve(); state.cleanupGeneration = 0; diff --git a/src/gateway/server-lanes.hook-group.test.ts b/src/gateway/server-lanes.hook-group.test.ts new file mode 100644 index 000000000000..3f86449f5a86 --- /dev/null +++ b/src/gateway/server-lanes.hook-group.test.ts @@ -0,0 +1,417 @@ +/** + * The cron+hook capacity group is opt-in on `hooks.enabled`. + * + * The reservation is a real cost: it withholds a slot from cron inner work even + * while the hook lane is idle. That price buys the guarantee that hooks cannot + * be starved by a saturated cron budget — so it is only paid by deployments + * that actually run hooks. With hooks disabled no group is installed and + * `cron-nested` keeps the entire cron budget, unchanged from before this + * feature existed. + */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { DEFAULT_CRON_MAX_CONCURRENT_RUNS } from "../config/cron-limits.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { enqueueCommandInLane, getCommandLaneSnapshot } from "../process/command-queue.js"; +import { resetCommandQueueStateForTest } from "../process/command-queue.test-support.js"; +import { CommandLane } from "../process/lanes.js"; +import { applyGatewayLaneConcurrency, resolveGatewayLaneConcurrency } from "./server-lanes.js"; + +function publish(config: OpenClawConfig): void { + applyGatewayLaneConcurrency(resolveGatewayLaneConcurrency(config)); +} + +const HOOKS_ON = { + hooks: { enabled: true, token: "t" }, +} as unknown as OpenClawConfig; +const HOOKS_OFF = {} as OpenClawConfig; + +function gate() { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; +} + +async function settle(): Promise { + for (let i = 0; i < 5; i++) { + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + } +} + +describe("cron+hook capacity group", () => { + afterEach(async () => { + if (vi.isFakeTimers()) { + await vi.runOnlyPendingTimersAsync(); + vi.clearAllTimers(); + } + vi.useRealTimers(); + const { resetSessionSuspensionStateForTest } = + await import("../agents/session-suspension.test-support.js"); + resetSessionSuspensionStateForTest(); + resetCommandQueueStateForTest(); + }); + + it("installs no group when hooks are disabled, leaving cron at full budget", async () => { + publish(HOOKS_OFF); + + const snapshot = getCommandLaneSnapshot(CommandLane.CronNested); + expect(snapshot.group).toBeUndefined(); + expect(snapshot.maxConcurrent).toBe(DEFAULT_CRON_MAX_CONCURRENT_RUNS); + + const gates = Array.from({ length: DEFAULT_CRON_MAX_CONCURRENT_RUNS }, () => gate()); + const runs = gates.map((g) => + enqueueCommandInLane(CommandLane.CronNested, async () => await g.promise, { + warnAfterMs: 10_000, + }), + ); + await settle(); + + // The whole budget, not budget-minus-a-reservation. + expect(getCommandLaneSnapshot(CommandLane.CronNested).activeCount).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS, + ); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + it("installs the group when hooks are enabled and reserves a slot for them", async () => { + publish(HOOKS_ON); + + const snapshot = getCommandLaneSnapshot(CommandLane.CronNested); + expect(snapshot.group).toBe("cron-hooks"); + expect(snapshot.groupBudget).toBe(DEFAULT_CRON_MAX_CONCURRENT_RUNS); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch)).toMatchObject({ + maxConcurrent: DEFAULT_CRON_MAX_CONCURRENT_RUNS, + reservedForLane: 1, + }); + + const gates = Array.from({ length: DEFAULT_CRON_MAX_CONCURRENT_RUNS }, () => gate()); + const runs = gates.map((g) => + enqueueCommandInLane(CommandLane.CronNested, async () => await g.promise, { + warnAfterMs: 10_000, + }), + ); + await settle(); + + // One short of the budget: the hook's reserved slot is withheld even though + // the hook lane is idle. This is the cost the opt-in exists to avoid paying + // on deployments that do not use hooks. + expect(getCommandLaneSnapshot(CommandLane.CronNested).activeCount).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS - 1, + ); + + // And a hook starts immediately despite cron holding everything else. + const hookGate = gate(); + const hookRun = enqueueCommandInLane( + CommandLane.HookDispatch, + async () => await hookGate.promise, + { warnAfterMs: 10_000 }, + ); + await settle(); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).activeCount).toBe(1); + + // Aggregate is exactly the pre-existing cron cap — no slot added outside it. + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).groupActive).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS, + ); + + hookGate.release(); + await hookRun; + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + it("admits hook bursts up to the shared budget and queues the ninth", async () => { + publish(HOOKS_ON); + + const gates = Array.from({ length: DEFAULT_CRON_MAX_CONCURRENT_RUNS + 1 }, () => gate()); + const runs = gates.map((g) => + enqueueCommandInLane(CommandLane.HookDispatch, async () => await g.promise, { + warnAfterMs: 10_000, + }), + ); + await settle(); + + expect(getCommandLaneSnapshot(CommandLane.HookDispatch)).toMatchObject({ + activeCount: DEFAULT_CRON_MAX_CONCURRENT_RUNS, + queuedCount: 1, + groupActive: DEFAULT_CRON_MAX_CONCURRENT_RUNS, + groupBudget: DEFAULT_CRON_MAX_CONCURRENT_RUNS, + }); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + it("admits seven cron plus one hook, then gives freed capacity to a second hook", async () => { + publish(HOOKS_ON); + + const cronGates = Array.from({ length: DEFAULT_CRON_MAX_CONCURRENT_RUNS - 1 }, () => gate()); + const cronRuns = cronGates.map((g) => + enqueueCommandInLane(CommandLane.CronNested, async () => await g.promise, { + warnAfterMs: 10_000, + }), + ); + const firstHookGate = gate(); + const firstHook = enqueueCommandInLane( + CommandLane.HookDispatch, + async () => await firstHookGate.promise, + { warnAfterMs: 10_000 }, + ); + await settle(); + + expect(getCommandLaneSnapshot(CommandLane.CronNested).activeCount).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS - 1, + ); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).activeCount).toBe(1); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).groupActive).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS, + ); + + const secondHookGate = gate(); + const secondHook = enqueueCommandInLane( + CommandLane.HookDispatch, + async () => await secondHookGate.promise, + { warnAfterMs: 10_000 }, + ); + await settle(); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).queuedCount).toBe(1); + + cronGates[0]?.release(); + await cronRuns[0]; + await settle(); + + expect(getCommandLaneSnapshot(CommandLane.CronNested).activeCount).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS - 2, + ); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).activeCount).toBe(2); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).groupActive).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS, + ); + + firstHookGate.release(); + secondHookGate.release(); + for (const g of cronGates.slice(1)) { + g.release(); + } + await Promise.all([...cronRuns.slice(1), firstHook, secondHook]); + }); + + it("hooks-off immediately drains cron work released by the teardown", async () => { + // Teardown must WAKE the lanes it frees, not merely delete membership. + // Asserting only `group === undefined` on an idle lane would pass even if + // clearGroups forgot to add its former members to the commit-drain set, + // leaving released work stuck until some unrelated enqueue pokes the lane. + publish(HOOKS_ON); + + const gates = Array.from({ length: DEFAULT_CRON_MAX_CONCURRENT_RUNS }, () => gate()); + const runs = gates.map((g) => + enqueueCommandInLane(CommandLane.CronNested, async () => await g.promise, { + warnAfterMs: 10_000, + }), + ); + await settle(); + + // One short of the budget, with the last entry queued behind the hook's + // reservation rather than running. + expect(getCommandLaneSnapshot(CommandLane.CronNested).activeCount).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS - 1, + ); + expect(getCommandLaneSnapshot(CommandLane.CronNested).queuedCount).toBe(1); + expect(getCommandLaneSnapshot(CommandLane.CronNested).blockedBy).toBe("sibling-reservation"); + + // Turning hooks off returns the reserved slot to cron. The queued entry + // must start on the publish itself. + publish(HOOKS_OFF); + await settle(); + + expect(getCommandLaneSnapshot(CommandLane.CronNested).group).toBeUndefined(); + expect(getCommandLaneSnapshot(CommandLane.CronNested).activeCount).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS, + ); + expect(getCommandLaneSnapshot(CommandLane.CronNested).queuedCount).toBe(0); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + it("keeps in-flight hooks inside the aggregate budget while disabling hooks", async () => { + publish(HOOKS_ON); + + const hookGate = gate(); + const hookRun = enqueueCommandInLane( + CommandLane.HookDispatch, + async () => await hookGate.promise, + { warnAfterMs: 10_000 }, + ); + const cronGates = Array.from({ length: DEFAULT_CRON_MAX_CONCURRENT_RUNS }, () => gate()); + const cronRuns = cronGates.map((g) => + enqueueCommandInLane(CommandLane.CronNested, async () => await g.promise, { + warnAfterMs: 10_000, + }), + ); + await settle(); + + expect(getCommandLaneSnapshot(CommandLane.CronNested).activeCount).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS - 1, + ); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).groupActive).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS, + ); + + publish(HOOKS_OFF); + await settle(); + + // The lane closes before the group reservation is removed. The running hook + // remains grouped, so cron cannot expand beyond the original aggregate cap. + expect(getCommandLaneSnapshot(CommandLane.HookDispatch)).toMatchObject({ + maxConcurrent: 0, + group: "cron-hooks", + reservedForLane: 0, + activeCount: 1, + }); + expect(getCommandLaneSnapshot(CommandLane.CronNested)).toMatchObject({ + activeCount: DEFAULT_CRON_MAX_CONCURRENT_RUNS - 1, + queuedCount: 1, + groupActive: DEFAULT_CRON_MAX_CONCURRENT_RUNS, + }); + + let lateHookStarted = false; + const lateHook = enqueueCommandInLane(CommandLane.HookDispatch, async () => { + lateHookStarted = true; + }); + await settle(); + expect(lateHookStarted).toBe(false); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).queuedCount).toBe(1); + + hookGate.release(); + await hookRun; + await settle(); + + // Hook completion hands its slot to cron, not to work queued on the closed + // hook lane, and aggregate activity remains bounded by the same group. + expect(getCommandLaneSnapshot(CommandLane.CronNested)).toMatchObject({ + activeCount: DEFAULT_CRON_MAX_CONCURRENT_RUNS, + queuedCount: 0, + groupActive: DEFAULT_CRON_MAX_CONCURRENT_RUNS, + }); + expect(lateHookStarted).toBe(false); + + for (const g of cronGates) { + g.release(); + } + await Promise.all(cronRuns); + + publish(HOOKS_ON); + await lateHook; + expect(lateHookStarted).toBe(true); + }); + + it("clears the group on hooks-off even when the grouped lane is suspended", async () => { + // The teardown path publishes only lanes that are NOT suspended. If every + // grouped member is suspended, the lane map is empty — and a guard that + // skips publication on an empty map would skip the group teardown with it. + // The stale group survives, and its members resume still paying a + // reservation for a hook lane that no longer receives work. + publish(HOOKS_ON); + expect(getCommandLaneSnapshot(CommandLane.CronNested).group).toBe("cron-hooks"); + + const { seedClearedLaneResumeForTest } = + await import("../agents/session-suspension.test-support.js"); + seedClearedLaneResumeForTest(CommandLane.CronNested, { + resumeConcurrency: DEFAULT_CRON_MAX_CONCURRENT_RUNS, + resumeAtMs: Date.now() + 60_000, + }); + + // gatewayStart consults the cleared-resume map for the suspended set. + applyGatewayLaneConcurrency(resolveGatewayLaneConcurrency(HOOKS_OFF), { gatewayStart: true }); + + expect(getCommandLaneSnapshot(CommandLane.CronNested).group).toBeUndefined(); + }); + + it("reinstalls the group before suspended lanes resume after hooks are re-enabled", async () => { + vi.useFakeTimers(); + vi.setSystemTime(1_000); + publish(HOOKS_ON); + + const { seedClearedLaneResumeForTest } = + await import("../agents/session-suspension.test-support.js"); + for (const lane of [CommandLane.CronNested, CommandLane.HookDispatch]) { + seedClearedLaneResumeForTest(lane, { + resumeConcurrency: DEFAULT_CRON_MAX_CONCURRENT_RUNS, + resumeAtMs: 1_100, + }); + } + + applyGatewayLaneConcurrency(resolveGatewayLaneConcurrency(HOOKS_OFF), { gatewayStart: true }); + expect(getCommandLaneSnapshot(CommandLane.CronNested).group).toBeUndefined(); + + // Both lanes remain at zero while their timers are active. Re-enabling + // hooks must still restore membership now; the per-lane resume setters + // deliberately cannot infer or install a missing capacity group later. + publish(HOOKS_ON); + expect(getCommandLaneSnapshot(CommandLane.CronNested)).toMatchObject({ + maxConcurrent: 0, + group: "cron-hooks", + }); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch)).toMatchObject({ + maxConcurrent: 0, + group: "cron-hooks", + reservedForLane: 1, + }); + + const cronGates = Array.from({ length: DEFAULT_CRON_MAX_CONCURRENT_RUNS }, () => gate()); + const hookGates = Array.from({ length: DEFAULT_CRON_MAX_CONCURRENT_RUNS }, () => gate()); + const cronRuns = cronGates.map((g) => + enqueueCommandInLane(CommandLane.CronNested, async () => await g.promise, { + warnAfterMs: 10_000, + }), + ); + const hookRuns = hookGates.map((g) => + enqueueCommandInLane(CommandLane.HookDispatch, async () => await g.promise, { + warnAfterMs: 10_000, + }), + ); + + await vi.advanceTimersByTimeAsync(100); + + expect(getCommandLaneSnapshot(CommandLane.CronNested).groupActive).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS, + ); + expect(getCommandLaneSnapshot(CommandLane.HookDispatch).groupActive).toBe( + DEFAULT_CRON_MAX_CONCURRENT_RUNS, + ); + expect( + getCommandLaneSnapshot(CommandLane.CronNested).activeCount + + getCommandLaneSnapshot(CommandLane.HookDispatch).activeCount, + ).toBe(DEFAULT_CRON_MAX_CONCURRENT_RUNS); + + for (const g of [...cronGates, ...hookGates]) { + g.release(); + } + await Promise.all([...cronRuns, ...hookRuns]); + }); + + it("removes the group when hooks are turned off by a config reload", async () => { + publish(HOOKS_ON); + expect(getCommandLaneSnapshot(CommandLane.CronNested).group).toBe("cron-hooks"); + + publish(HOOKS_OFF); + // Membership must actually be torn down, or cron keeps paying a reservation + // for a lane that no longer receives work. + expect(getCommandLaneSnapshot(CommandLane.CronNested).group).toBeUndefined(); + expect(getCommandLaneSnapshot(CommandLane.CronNested).blockedBy).toBeNull(); + }); +}); diff --git a/src/gateway/server-lanes.ts b/src/gateway/server-lanes.ts index eb909a57b8c0..aefab012a70c 100644 --- a/src/gateway/server-lanes.ts +++ b/src/gateway/server-lanes.ts @@ -1,24 +1,47 @@ import { enableSessionSuspensionTimersForGatewayStart, - getCleanupSuspendedLaneIdsForGatewayPublication, + getSuspendedLaneIdsForGatewayPublication, + setGatewayLaneResumeConcurrencies, } from "../agents/session-suspension.js"; // Gateway command-lane concurrency applier. // Pushes config-derived agent/cron limits into the process command queue. import { resolveAgentMaxConcurrent, resolveSubagentMaxConcurrent } from "../config/agent-limits.js"; import { resolveCronMaxConcurrentRuns } from "../config/cron-limits.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; -import { setCommandLaneConcurrency } from "../process/command-queue.js"; +import { + getCommandLaneSnapshot, + publishLaneConfiguration, + setCommandLaneConcurrency, +} from "../process/command-queue.js"; import { CommandLane } from "../process/lanes.js"; type GatewayLaneConcurrency = { cron: number; + /** + * Width of the hook lane, or 0 when hooks are disabled. + * + * Zero is meaningful: a steady-state hooks-off publication creates no group, + * so a deployment that does not use hooks keeps the full cron budget and sees + * no behaviour change from this feature. + */ + hookDispatch: number; main: number; subagent: number; }; +/** Capacity held inside the cron budget so hook dispatch cannot be starved. */ +const HOOK_DISPATCH_LANE_RESERVATION = 1; + +/** Group bounding cron inner work and hook dispatch to one shared budget. */ +const CRON_HOOK_LANE_GROUP = "cron-hooks"; + export function resolveGatewayLaneConcurrency(cfg: OpenClawConfig): GatewayLaneConcurrency { + const cron = resolveCronMaxConcurrentRuns(); return { - cron: resolveCronMaxConcurrentRuns(), + cron, + // The reservation guarantees one slot, but hooks may use every free slot + // inside the shared budget. A one-wide lane would serialize unrelated hooks. + hookDispatch: cfg.hooks?.enabled === true ? cron : 0, main: resolveAgentMaxConcurrent(cfg), subagent: resolveSubagentMaxConcurrent(cfg), }; @@ -28,34 +51,70 @@ export function applyGatewayLaneConcurrency( concurrency: GatewayLaneConcurrency, opts: { gatewayStart?: boolean } = {}, ): void { + setGatewayLaneResumeConcurrencies({ + [CommandLane.Cron]: concurrency.cron, + [CommandLane.CronNested]: concurrency.cron, + [CommandLane.HookDispatch]: concurrency.hookDispatch, + [CommandLane.Main]: concurrency.main, + [CommandLane.Nested]: 1, + [CommandLane.Subagent]: concurrency.subagent, + }); // Lane ids are open strings (plugins mint their own); narrow once so the // gateway-managed cases compare within the enum. const suspendedLaneIds: ReadonlySet = opts.gatewayStart - ? enableSessionSuspensionTimersForGatewayStart((laneId, savedResumeConcurrency) => { - switch (laneId as CommandLane) { - case CommandLane.Cron: - case CommandLane.CronNested: - return concurrency.cron; - case CommandLane.Main: - return concurrency.main; - case CommandLane.Nested: - return 1; - case CommandLane.Subagent: - return concurrency.subagent; - default: - return savedResumeConcurrency; - } - }) - : getCleanupSuspendedLaneIdsForGatewayPublication(); + ? enableSessionSuspensionTimersForGatewayStart() + : getSuspendedLaneIdsForGatewayPublication(); // Resolution is deliberately separate: this commit-edge applier only updates // live queue state and cannot reject a config midway through publication. if (!suspendedLaneIds.has(CommandLane.Cron)) { setCommandLaneConcurrency(CommandLane.Cron, concurrency.cron); } - // Cron isolated agent turns remap inner LLM work to this lane. + // `cron-nested` (cron inner agent work) and `hook-dispatch` (external hook + // agent runs) are published as ONE transaction together with the group that + // bounds them. Applying them with the per-lane setter would drain each lane + // the moment it went positive — before the group existed — so both could + // dispatch up to their individual maxima and exceed the shared budget. That + // is precisely the additive-capacity behaviour openclaw#98813 was held for. + const hooksEnabled = concurrency.hookDispatch > 0; + const hookSnapshot = getCommandLaneSnapshot(CommandLane.HookDispatch); + // Closing hooks must not detach already-running hook work from the shared + // budget while cron immediately expands back to its full width. Retain the + // group without a reservation until a later publication sees no active hook. + const retainInFlightHookBudget = !hooksEnabled && hookSnapshot.activeCount > 0; + const grouped: Record = {}; if (!suspendedLaneIds.has(CommandLane.CronNested)) { - setCommandLaneConcurrency(CommandLane.CronNested, concurrency.cron); + grouped[CommandLane.CronNested] = concurrency.cron; } + if (!suspendedLaneIds.has(CommandLane.HookDispatch)) { + grouped[CommandLane.HookDispatch] = concurrency.hookDispatch; + } + // Publish even when `grouped` is empty. Both lanes can be suspended during + // config reload, but the group still needs its reservation updated or its + // membership cleared before their independent resume timers reopen them. + publishLaneConfiguration({ + lanes: grouped, + // Opt-in. A clean hooks-off publication installs no group and + // `cron-nested` keeps the entire cron budget. During an enabled-to-disabled + // transition, a zero-reservation group may remain while in-flight hooks + // finish so aggregate work stays bounded without withholding idle capacity. + groups: + hooksEnabled || retainInFlightHookBudget + ? { + // Budget equals the existing cron cap, so the hook lane costs + // nothing in AGGREGATE concurrency; it reserves one slot inside + // that cap rather than adding one outside it. Cron inner work + // trades one slot for the guarantee that hooks cannot be starved. + [CRON_HOOK_LANE_GROUP]: { + budget: concurrency.cron, + members: [CommandLane.CronNested, CommandLane.HookDispatch], + reservations: hooksEnabled + ? { [CommandLane.HookDispatch]: HOOK_DISPATCH_LANE_RESERVATION } + : undefined, + }, + } + : undefined, + clearGroups: hooksEnabled || retainInFlightHookBudget ? undefined : [CRON_HOOK_LANE_GROUP], + }); if (!suspendedLaneIds.has(CommandLane.Main)) { setCommandLaneConcurrency(CommandLane.Main, concurrency.main); } diff --git a/src/gateway/server.hooks-lane.test.ts b/src/gateway/server.hooks-lane.test.ts new file mode 100644 index 000000000000..359b4bf53ca4 --- /dev/null +++ b/src/gateway/server.hooks-lane.test.ts @@ -0,0 +1,99 @@ +/** + * Guards the command lane that external hook agent-runs dispatch into. + * + * Hook runs used to pass `lane: "cron"`, which `resolveCronAgentLane` remaps to + * `cron-nested` — the same lane cron's own inner agent work uses. A saturated + * cron budget therefore starved every hook. Hook runs now dispatch into + * `hook-dispatch` so they are schedulable in their own right; aggregate capacity + * is bounded by the lane group that owns both lanes, not by adding a slot + * outside the cron budget. + * + * This assertion is the only thing standing between that fix and a silent + * regression: nothing else in the suite reads the dispatched lane, so reverting + * the call site to `"cron"` leaves every other test green. + */ +import { afterEach, describe, expect, test, vi } from "vitest"; +import { resolveCronAgentLane } from "../agents/lanes.js"; +import { resolveMainSessionKeyFromConfig } from "../config/sessions.js"; +import { drainSystemEvents } from "../infra/system-events.js"; +import { CommandLane } from "../process/lanes.js"; +import { + cronIsolatedRun, + installGatewayTestHooks, + testState, + withGatewayServer, +} from "./test-helpers.js"; + +installGatewayTestHooks({ scope: "suite" }); + +await import("./server.js"); + +const HOOK_TOKEN = "hook-secret"; + +afterEach(() => { + drainSystemEvents(resolveMainSessionKeyFromConfig()); + vi.restoreAllMocks(); +}); + +async function postHook( + port: number, + path: string, + body: Record, + idempotencyKey: string, +): Promise { + return await fetch(`http://127.0.0.1:${port}${path}`, { + method: "POST", + headers: { + Authorization: `Bearer ${HOOK_TOKEN}`, + "Content-Type": "application/json", + "Idempotency-Key": idempotencyKey, + }, + body: JSON.stringify(body), + }); +} + +async function waitForCronIsolatedRuns(count: number): Promise { + await expect + .poll(() => cronIsolatedRun.mock.calls.length, { timeout: 2_000, interval: 10 }) + .toBe(count); +} + +describe("gateway hook dispatch lane", () => { + test("dispatches hook agent runs into the hook lane, not the cron lane", async () => { + testState.hooksConfig = { enabled: true, token: HOOK_TOKEN }; + await withGatewayServer(async ({ port }) => { + cronIsolatedRun.mockClear(); + cronIsolatedRun.mockImplementation(async (params: unknown) => { + (params as { onExecutionStarted?: () => void }).onExecutionStarted?.(); + return { status: "ok", summary: "done" }; + }); + + const response = await postHook( + port, + "/hooks/agent", + { message: "Dispatch" }, + "hook-lane-idem", + ); + expect(response.status).toBe(200); + await waitForCronIsolatedRuns(1); + + const [dispatched] = cronIsolatedRun.mock.calls[0] as [{ lane?: string }]; + expect(dispatched.lane).toBe(CommandLane.HookDispatch); + // The regression this guards: `"cron"` is the value that used to be passed. + expect(dispatched.lane).not.toBe(CommandLane.Cron); + }); + }); + + test("the dispatched lane survives cron lane resolution unremapped", () => { + // `resolveCronAgentLane` collapses empty and `cron` onto `cron-nested` and + // passes every other lane through. The hook lane must land in the second + // case, or the group reserves capacity for a lane nothing ever uses. + expect(resolveCronAgentLane(CommandLane.HookDispatch)).toBe(CommandLane.HookDispatch); + + // Control: prove the assertion above can fail. These are the inputs that DO + // get remapped, so a resolver that passed everything through unchanged + // would break here rather than passing vacuously. + expect(resolveCronAgentLane(CommandLane.Cron)).toBe(CommandLane.CronNested); + expect(resolveCronAgentLane(undefined)).toBe(CommandLane.CronNested); + }); +}); diff --git a/src/gateway/server/hooks.ts b/src/gateway/server/hooks.ts index 1373027a5ebb..5d1408ccbe7e 100644 --- a/src/gateway/server/hooks.ts +++ b/src/gateway/server/hooks.ts @@ -30,6 +30,7 @@ import { validateExplicitMessageAccountSelection } from "../../infra/outbound/me import { enqueueSystemEvent } from "../../infra/system-events.js"; import type { createSubsystemLogger } from "../../logging/subsystem.js"; import { runWithGatewayIndependentRootWorkContinuation } from "../../process/gateway-work-admission.js"; +import { CommandLane } from "../../process/lanes.js"; import { toAgentStoreSessionKey } from "../../routing/session-key.js"; import type { HookAgentDispatchPayload, HooksConfigResolved } from "../hooks.js"; import { @@ -416,7 +417,11 @@ export function createGatewayHooksRequestHandler(params: { // Isolated runs derive their lifecycle key from random jobId (or an // already-stable cron: key), so accepted agentId closes reload drift. agentId, - lane: "cron", + // Hook agent runs get their own lane rather than sharing + // `cron-nested` with cron inner work, so a saturated cron budget + // cannot starve them. Aggregate capacity stays bounded by the lane + // group that owns both lanes. + lane: CommandLane.HookDispatch, abortSignal: startupAbortController.signal, onExecutionStarted: () => { // Existing runner-entry callbacks are the final owner-boundary fence: diff --git a/src/infra/heartbeat-runner-execution.ts b/src/infra/heartbeat-runner-execution.ts index b6bdaf228ffb..32a712a4e56c 100644 --- a/src/infra/heartbeat-runner-execution.ts +++ b/src/infra/heartbeat-runner-execution.ts @@ -213,8 +213,13 @@ export async function resolveHeartbeatWakeStage(opts: HeartbeatRunOptions) { owningCronLaneTaskMarker?.lane === CommandLane.Cron && isCommandLaneTaskMarkerCurrent(owningCronLaneTaskMarker); const cronLaneDepth = getSize(CommandLane.Cron); + // HookDispatch is included so moving hook agent runs off `cron-nested` onto + // their own lane does not silently stop them from suppressing heartbeats. + // They are still active agent work; only the lane they occupy changed. const cronLaneBusy = - cronLaneDepth > (ownsCronLaneTask ? 1 : 0) || getSize(CommandLane.CronNested) > 0; + cronLaneDepth > (ownsCronLaneTask ? 1 : 0) || + getSize(CommandLane.CronNested) > 0 || + getSize(CommandLane.HookDispatch) > 0; if (cronBusy || cronLaneBusy) { emitHeartbeatEvent({ status: "skipped", diff --git a/src/process/command-queue.capacity-groups.test.ts b/src/process/command-queue.capacity-groups.test.ts new file mode 100644 index 000000000000..c70215d795b5 --- /dev/null +++ b/src/process/command-queue.capacity-groups.test.ts @@ -0,0 +1,369 @@ +/** + * Capacity groups: a shared hard budget across lanes, with non-borrowable + * per-member reservations. + * + * The invariant under test is the one the upstream maintainer asked for on + * openclaw#98813: giving hook dispatch its own lane must NOT add a concurrent + * slot outside the existing cron budget. A group whose budget equals that cap + * is what makes the separate lane safe. + */ +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { + enqueueCommandInLane, + getCommandLaneSnapshot, + publishLaneConfiguration, + resetAllLanes, + resetCommandLane, + setCommandLaneConcurrency, +} from "./command-queue.js"; + +const CRON = "cron-nested"; +const HOOK = "hook-dispatch"; +const GROUP = "cron-hooks"; + +type LaneGroupSpec = NonNullable[0]["groups"]>[string]; + +function setCommandLaneGroup(group: string, spec: LaneGroupSpec): void { + publishLaneConfiguration({ groups: { [group]: spec } }); +} + +function clearCommandLaneGroup(group: string): void { + publishLaneConfiguration({ clearGroups: [group] }); +} + +/** A task that blocks until released, so occupancy is controllable. */ +function gate() { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; +} + +async function settle(): Promise { + for (let i = 0; i < 5; i++) { + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + } +} + +beforeEach(() => { + resetAllLanes(); + clearCommandLaneGroup(GROUP); + setCommandLaneConcurrency(CRON, 8); + setCommandLaneConcurrency(HOOK, 8); +}); + +afterEach(() => { + clearCommandLaneGroup(GROUP); + resetAllLanes(); +}); + +describe("command lane capacity groups", () => { + test("a reserved lane starts under sibling saturation", async () => { + setCommandLaneGroup(GROUP, { + budget: 8, + members: [CRON, HOOK], + reservations: { [HOOK]: 1 }, + }); + + // Fill the group to its budget minus the hook's reservation. + const gates = Array.from({ length: 7 }, () => gate()); + const cronRuns = gates.map((g) => enqueueCommandInLane(CRON, async () => await g.promise)); + await settle(); + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(7); + + // The 8th slot is the hook's hard reservation: cron must not take it. + const extra = gate(); + const blockedCron = enqueueCommandInLane(CRON, async () => await extra.promise); + await settle(); + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(7); + expect(getCommandLaneSnapshot(CRON).blockedBy).toBe("sibling-reservation"); + + // And the hook starts immediately despite the group being otherwise full. + const hookGate = gate(); + const hookRun = enqueueCommandInLane(HOOK, async () => await hookGate.promise); + await settle(); + expect(getCommandLaneSnapshot(HOOK).activeCount).toBe(1); + expect(getCommandLaneSnapshot(HOOK).groupActive).toBe(8); + + hookGate.release(); + await hookRun; + for (const g of gates) { + g.release(); + } + extra.release(); + await Promise.all([...cronRuns, blockedCron]); + }); + + test("total active never exceeds the group budget", async () => { + setCommandLaneGroup(GROUP, { + budget: 8, + members: [CRON, HOOK], + reservations: { [HOOK]: 1 }, + }); + + const gates = Array.from({ length: 20 }, () => gate()); + const runs = gates.map((g, i) => + enqueueCommandInLane(i % 2 === 0 ? CRON : HOOK, async () => await g.promise), + ); + await settle(); + + const cron = getCommandLaneSnapshot(CRON); + const hook = getCommandLaneSnapshot(HOOK); + expect(cron.activeCount + hook.activeCount).toBeLessThanOrEqual(8); + // Not vacuous: the group must actually be saturated, not merely under cap. + expect(cron.activeCount + hook.activeCount).toBe(8); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + test("a member may use the full group budget beyond its reservation", async () => { + setCommandLaneGroup(GROUP, { + budget: 8, + members: [CRON, HOOK], + reservations: { [HOOK]: 1 }, + }); + + const gates = Array.from({ length: 9 }, () => gate()); + const runs = gates.map((g) => enqueueCommandInLane(HOOK, async () => await g.promise)); + await settle(); + + expect(getCommandLaneSnapshot(HOOK)).toMatchObject({ + activeCount: 8, + queuedCount: 1, + maxConcurrent: 8, + groupActive: 8, + groupBudget: 8, + reservedForLane: 1, + blockedBy: "lane", + }); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + test("capacity freed by one member wakes a queued sibling", async () => { + setCommandLaneGroup(GROUP, { budget: 2, members: [CRON, HOOK] }); + + const a = gate(); + const b = gate(); + const first = enqueueCommandInLane(CRON, async () => await a.promise); + const second = enqueueCommandInLane(CRON, async () => await b.promise); + await settle(); + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(2); + + // Budget is full, so the hook cannot start. + const hookGate = gate(); + const hookRun = enqueueCommandInLane(HOOK, async () => await hookGate.promise); + await settle(); + expect(getCommandLaneSnapshot(HOOK).activeCount).toBe(0); + expect(getCommandLaneSnapshot(HOOK).blockedBy).toBe("group-budget"); + + // Releasing a cron task must wake the hook, which lives on a DIFFERENT + // lane — a lane-local pump would leave it queued behind free capacity. + a.release(); + await first; + await settle(); + expect(getCommandLaneSnapshot(HOOK).activeCount).toBe(1); + + hookGate.release(); + b.release(); + await Promise.all([second, hookRun]); + }); + + test("a failing task releases group capacity like a successful one", async () => { + setCommandLaneGroup(GROUP, { budget: 1, members: [CRON, HOOK] }); + + const boom = gate(); + const failing = enqueueCommandInLane(CRON, async () => { + await boom.promise; + throw new Error("task blew up"); + }); + await settle(); + + const hookGate = gate(); + const hookRun = enqueueCommandInLane(HOOK, async () => await hookGate.promise); + await settle(); + expect(getCommandLaneSnapshot(HOOK).activeCount).toBe(0); + + boom.release(); + await expect(failing).rejects.toThrow("task blew up"); + await settle(); + expect(getCommandLaneSnapshot(HOOK).activeCount).toBe(1); + + hookGate.release(); + await hookRun; + }); + + test("a timed-out task releases group capacity to a queued sibling", async () => { + setCommandLaneGroup(GROUP, { budget: 1, members: [CRON, HOOK] }); + + const timedOut = enqueueCommandInLane(CRON, async () => new Promise(() => {}), { + taskTimeoutMs: 10, + }); + const hookGate = gate(); + const hookRun = enqueueCommandInLane(HOOK, async () => await hookGate.promise); + + await expect(timedOut).rejects.toMatchObject({ name: "CommandLaneTaskTimeoutError" }); + await settle(); + expect(getCommandLaneSnapshot(HOOK).activeCount).toBe(1); + + hookGate.release(); + await hookRun; + }); + + test("resetting a member releases group capacity to a queued sibling", async () => { + setCommandLaneGroup(GROUP, { budget: 1, members: [CRON, HOOK] }); + + const cronGate = gate(); + const cronRun = enqueueCommandInLane(CRON, async () => await cronGate.promise); + await settle(); + + const hookGate = gate(); + const hookRun = enqueueCommandInLane(HOOK, async () => await hookGate.promise); + await settle(); + expect(getCommandLaneSnapshot(HOOK).activeCount).toBe(0); + + expect(resetCommandLane(CRON)).toBe(1); + await settle(); + expect(getCommandLaneSnapshot(HOOK).activeCount).toBe(1); + + cronGate.release(); + hookGate.release(); + await Promise.all([cronRun, hookRun]); + }); + + test("an idle sibling's reservation is withheld, not borrowed", async () => { + setCommandLaneGroup(GROUP, { + budget: 4, + members: [CRON, HOOK], + reservations: { [HOOK]: 1 }, + }); + + const gates = Array.from({ length: 6 }, () => gate()); + const runs = gates.map((g) => enqueueCommandInLane(CRON, async () => await g.promise)); + await settle(); + + // 3, not 4: the hook is idle but its reserved slot is genuinely held back. + // A borrowable reservation would show 4 here and starve the hook. + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(3); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + test("blockedBy reports hypothetical immediate admission with an EMPTY queue", async () => { + // The non-vacuity condition for the whole wait-visibility fix. + // + // `noteLaneWaitIfBusy` runs BEFORE enqueue, so it sees queuedCount === 0. If + // blockedBy were only populated for an already-queued head entry, the + // pre-enqueue snapshot would read "not blocked", no onLaneWait(waiting:true) + // would fire, and agent-watchdog's setup-timeout suppression would never + // engage — producing a false setup timeout for a run that is merely waiting + // on group capacity. blockedBy must answer "could this lane start work right + // now?", independent of whether anything is queued. + setCommandLaneGroup(GROUP, { + budget: 8, + members: [CRON, HOOK], + reservations: { [HOOK]: 1 }, + }); + + const gates = Array.from({ length: 7 }, () => gate()); + const runs = gates.map((g) => enqueueCommandInLane(CRON, async () => await g.promise)); + await settle(); + + const snapshot = getCommandLaneSnapshot(CRON); + // Nothing queued, and the lane is under its own maxConcurrent of 8... + expect(snapshot.queuedCount).toBe(0); + expect(snapshot.activeCount).toBeLessThan(snapshot.maxConcurrent); + // ...yet it genuinely cannot start: the last slot is the hook's reserve. + expect(snapshot.blockedBy).toBe("sibling-reservation"); + + // A lane with room reports null, so the assertion above is discriminating + // rather than always-truthy. + expect(getCommandLaneSnapshot(HOOK).blockedBy).toBeNull(); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + test("an unmaterialized lane still reports its group block state", async () => { + // A member lane may not exist yet (never enqueued) or may have been retired + // while idle. `noteLaneWaitIfBusy` can snapshot it in exactly that state, so + // the not-found path must consult the group rather than return a bare + // default that reads as "free". + setCommandLaneGroup(GROUP, { budget: 1, members: [CRON, HOOK] }); + const busy = gate(); + const run = enqueueCommandInLane(CRON, async () => await busy.promise); + await settle(); + + const snapshot = getCommandLaneSnapshot(HOOK); + expect(snapshot.activeCount).toBe(0); + expect(snapshot.blockedBy).toBe("group-budget"); + expect(snapshot.groupBudget).toBe(1); + + busy.release(); + await run; + }); + + test("lanes outside any group are unconstrained by it", async () => { + setCommandLaneGroup(GROUP, { budget: 1, members: [CRON, HOOK] }); + setCommandLaneConcurrency("unpooled", 4); + + const gates = Array.from({ length: 4 }, () => gate()); + const runs = gates.map((g) => enqueueCommandInLane("unpooled", async () => await g.promise)); + await settle(); + expect(getCommandLaneSnapshot("unpooled").activeCount).toBe(4); + expect(getCommandLaneSnapshot("unpooled").blockedBy).toBe("lane"); + expect(getCommandLaneSnapshot("unpooled").group).toBeUndefined(); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + test("rejects reservations that exceed the budget", () => { + expect(() => + setCommandLaneGroup(GROUP, { + budget: 2, + members: [CRON, HOOK], + reservations: { [CRON]: 2, [HOOK]: 1 }, + }), + ).toThrow(/reserves 3 slots but its budget is 2/); + }); + + test("rejects lanes that can be synchronously awaited", () => { + // `cron` awaits `cron-nested`; grouping them turns a wait into a deadlock. + expect(() => setCommandLaneGroup(GROUP, { budget: 2, members: ["cron", HOOK] })).toThrow( + /cannot join a capacity group/, + ); + expect(() => setCommandLaneGroup(GROUP, { budget: 2, members: ["session:abc", HOOK] })).toThrow( + /cannot join a capacity group/, + ); + expect(() => setCommandLaneGroup(GROUP, { budget: 2, members: ["main", HOOK] })).toThrow( + /cannot join a capacity group/, + ); + }); + + test("rejects a reservation for a non-member lane", () => { + expect(() => + setCommandLaneGroup(GROUP, { + budget: 2, + members: [CRON], + reservations: { [HOOK]: 1 }, + }), + ).toThrow(/reserves for non-member lane/); + }); +}); diff --git a/src/process/command-queue.capacity-groups.ts b/src/process/command-queue.capacity-groups.ts new file mode 100644 index 000000000000..a2d16968cf15 --- /dev/null +++ b/src/process/command-queue.capacity-groups.ts @@ -0,0 +1,246 @@ +// Capacity groups: a shared, hard aggregate budget across several command +// lanes, with per-member reservations. Split out of command-queue.ts to keep +// that file within its size budget; the queue supplies its own `drainLane` so +// this module never has to import back into it. +import { getQueueState, normalizeLane } from "./command-queue.state.js"; +import { CommandLane } from "./lanes.js"; + +/** Drains a single lane. Supplied by command-queue.ts to avoid a cycle. */ +type DrainLaneFn = (lane: string) => void; + +/** Why a lane cannot admit, from the narrowest cause outward. */ +export type CommandLaneBlockReason = "lane" | "group-budget" | "sibling-reservation" | null; + +/** Declares a group's shared budget and its members' hard reservations. */ +export type CommandLaneGroupSpec = { + /** Hard aggregate cap across all members. */ + budget: number; + members: readonly string[]; + /** + * Slots a member may always claim, non-borrowable by siblings. + * + * Not validated against the member's own `maxConcurrent`, because lane widths + * and group definitions are published together and the width may not be + * applied yet at validation time. A reservation larger than the lane's width + * is therefore accepted but partly unusable: the excess is withheld from + * siblings while its owner cannot claim it. + */ + reservations?: Readonly>; +}; + +export type LaneGroupState = { + group: string; + budget: number; + members: Set; + reservations: Map; +}; + +/** + * Lanes that must never join a group, because a group member can be made to + * wait for a sibling and these lanes can be synchronously awaited by other + * lanes — which would turn a wait into a deadlock. + * + * Known wait edges at this base: outer `cron` -> `cron-nested` + * (`server-cron.ts` passes lane "cron"; `agents/lanes.ts` remaps inner work), + * and `session:` -> global lane (embedded-agent-runner run + compaction). + */ +const GROUP_INELIGIBLE_LANES: ReadonlySet = new Set([ + CommandLane.Cron, + CommandLane.Main, + CommandLane.Subagent, + CommandLane.Nested, +]); + +const GROUP_INELIGIBLE_PREFIXES = ["session:", "nested:", "context-engine-turn-maintenance:"]; + +function assertGroupEligibleLane(lane: string): void { + if (GROUP_INELIGIBLE_LANES.has(lane)) { + throw new Error( + `command lane "${lane}" cannot join a capacity group: it can be synchronously awaited by another lane`, + ); + } + for (const prefix of GROUP_INELIGIBLE_PREFIXES) { + if (lane.startsWith(prefix)) { + throw new Error( + `command lane "${lane}" cannot join a capacity group: "${prefix}*" lanes can be synchronously awaited`, + ); + } + } +} + +/** Group registry, keyed by group id and by member lane name. */ +export function getGroupRegistry(): { + groups: Map; + groupByLane: Map; +} { + const state = getQueueState() as unknown as { + laneGroups?: Map; + laneGroupByLane?: Map; + }; + // Migration: an older singleton (pre-upgrade, inherited via globalThis after + // a SIGUSR1 in-process restart) has neither field. Active counts are derived, + // so a late-initialized registry cannot desynchronize from lane state. + if (!state.laneGroups) { + state.laneGroups = new Map(); + } + if (!state.laneGroupByLane) { + state.laneGroupByLane = new Map(); + } + return { groups: state.laneGroups, groupByLane: state.laneGroupByLane }; +} + +export function getLaneGroup(lane: string): LaneGroupState | undefined { + const { groups, groupByLane } = getGroupRegistry(); + const groupId = groupByLane.get(lane); + return groupId ? groups.get(groupId) : undefined; +} + +/** + * Active task count for a group member WITHOUT creating the lane. Creating it + * here would resurrect lanes that `retireIdleScopedCommandLane` just removed. + */ +export function getMemberActiveCount(lane: string): number { + return getQueueState().lanes.get(lane)?.activeTaskIds.size ?? 0; +} + +/** + * Why `lane` cannot admit another task, or null if it can. + * + * Group capacity is always DERIVED from members' `activeTaskIds`, never tracked + * in a separate counter. That is what makes timeout, abort, clear, reset and + * stale-generation completion release capacity for free: they all remove the + * task id, so the next admission decision simply sees a smaller number. The + * only remaining obligation is that those paths re-drain the group. + */ +export function resolveLaneBlockReason(lane: string): CommandLaneBlockReason { + const state = getQueueState().lanes.get(lane); + if (state && state.activeTaskIds.size >= state.maxConcurrent) { + return "lane"; + } + const group = getLaneGroup(lane); + if (!group) { + return null; + } + let groupActive = 0; + let siblingReserveHeld = 0; + for (const member of group.members) { + const active = getMemberActiveCount(member); + groupActive += active; + if (member !== lane) { + // Unused portion of a sibling's reservation. Held back even while that + // sibling is idle — a hard reservation that siblings can borrow is not a + // reservation at all. + siblingReserveHeld += Math.max(0, (group.reservations.get(member) ?? 0) - active); + } + } + if (groupActive >= group.budget) { + return "group-budget"; + } + // Own reservation still unfilled: admit regardless of what siblings hold. + if (getMemberActiveCount(lane) < (group.reservations.get(lane) ?? 0)) { + return null; + } + // Otherwise this task would be borrowing unreserved capacity, which must not + // eat into what siblings are guaranteed. + return groupActive + siblingReserveHeld < group.budget ? null : "sibling-reservation"; +} + +export function canAdmitInGroup(lane: string): boolean { + const reason = resolveLaneBlockReason(lane); + return reason === null || reason === "lane"; +} + +/** + * Define or replace a capacity group. + * + * Membership is held here, keyed by lane name, and deliberately NOT inside + * `LaneState`: `setCommandLaneConcurrency` must not be able to detach a lane + * from its group, or session suspend/resume would silently restore a member to + * ungoverned concurrency. + */ +export function validateCommandLaneGroupSpec( + group: string, + spec: CommandLaneGroupSpec, +): LaneGroupState { + const members = spec.members.map((member) => normalizeLane(member)); + for (const member of members) { + assertGroupEligibleLane(member); + } + const reservations = new Map(); + let reservedTotal = 0; + for (const [rawLane, count] of Object.entries(spec.reservations ?? {})) { + const member = normalizeLane(rawLane); + if (!members.includes(member)) { + throw new Error(`command lane group "${group}" reserves for non-member lane "${member}"`); + } + const reserved = Math.max(0, Math.floor(count)); + reservations.set(member, reserved); + reservedTotal += reserved; + } + const budget = Math.max(0, Math.floor(spec.budget)); + if (reservedTotal > budget) { + // Silent starvation otherwise: reservations that cannot all be honoured + // would permanently withhold capacity no member is able to claim. + throw new Error( + `command lane group "${group}" reserves ${reservedTotal} slots but its budget is ${budget}`, + ); + } + return { group, budget, members: new Set(members), reservations }; +} + +/** Install a validated group, detaching its members from any previous owner. */ +export function installCommandLaneGroup(next: LaneGroupState): void { + const { groups, groupByLane } = getGroupRegistry(); + const previous = groups.get(next.group); + if (previous) { + for (const member of previous.members) { + groupByLane.delete(member); + } + } + for (const member of next.members) { + // A lane may belong to at most one group. Without this, the old owner's + // `members` would still contain the lane and would keep counting its active + // tasks toward a budget it no longer participates in. + const owner = groupByLane.get(member); + if (owner && owner !== next.group) { + groups.get(owner)?.members.delete(member); + } + } + groups.set(next.group, next); + for (const member of next.members) { + groupByLane.set(member, next.group); + } +} + +/** + * Drain the given member lanes. + * + * Never creates a lane: `drainLane` calls `getLaneState`, which would resurrect + * a scoped lane that `retireIdleScopedCommandLane` had just removed. A lane + * whose width is 0 admits nothing when pumped, so no width check is needed + * here — it would only skip a call that is already a no-op. + */ +function drainMembers(lanes: Iterable, drainLane: DrainLaneFn): void { + for (const lane of lanes) { + const state = getQueueState().lanes.get(lane); + if (state && state.queue.length > 0 && !state.draining) { + drainLane(lane); + } + } +} + +/** + * Re-drain every OTHER member of `lane`'s group. Capacity that a completion + * frees belongs to the group, not to the lane that freed it, so a lane-local + * pump would leave siblings queued behind capacity that is already available. + */ +export function drainGroupSiblings(lane: string, drainLane: DrainLaneFn): void { + const group = getLaneGroup(lane); + if (!group) { + return; + } + drainMembers( + [...group.members].filter((member) => member !== lane), + drainLane, + ); +} diff --git a/src/process/command-queue.publish-transaction.test.ts b/src/process/command-queue.publish-transaction.test.ts new file mode 100644 index 000000000000..ea11c6a6e18f --- /dev/null +++ b/src/process/command-queue.publish-transaction.test.ts @@ -0,0 +1,299 @@ +/** + * Atomic lane-configuration publication. + * + * Round-4 review (fiducian-spencer-001) asked specifically for a regression + * that "would fail if any member drains during publication before the group is + * installed, not just a post-state assertion". A post-state check is too weak: + * work admitted above budget during the publication window can complete before + * the assertion runs, leaving final counts looking correct. + * + * These tests therefore observe PEAK concurrency across the window, using tasks + * that park so nothing can retire before it is counted. + */ +import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { + clearCommandLane, + enqueueCommandInLane, + getCommandLaneSnapshot, + publishLaneConfiguration, + resetAllLanes, + setCommandLaneConcurrency, +} from "./command-queue.js"; + +const CRON = "cron-nested"; +const HOOK = "hook-dispatch"; +const GROUP = "cron-hooks"; + +type LaneGroupSpec = NonNullable[0]["groups"]>[string]; + +function setCommandLaneGroup(group: string, spec: LaneGroupSpec): void { + publishLaneConfiguration({ groups: { [group]: spec } }); +} + +function clearCommandLaneGroup(group: string): void { + publishLaneConfiguration({ clearGroups: [group] }); +} + +function gate() { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; +} + +async function settle(): Promise { + for (let i = 0; i < 5; i++) { + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + } +} + +beforeEach(() => { + resetAllLanes(); + clearCommandLaneGroup(GROUP); +}); + +afterEach(() => { + clearCommandLaneGroup(GROUP); + resetAllLanes(); +}); + +describe("publishLaneConfiguration", () => { + test("no member dispatches above budget DURING publication", async () => { + // Both lanes start closed with work already queued, so the only thing that + // can release them is publication itself. If publication widened a lane and + // drained it before installing the group — what the sequential per-lane + // setter does — the two lanes would admit up to 8 + 4 = 12 tasks. + setCommandLaneConcurrency(CRON, 0); + setCommandLaneConcurrency(HOOK, 0); + + let active = 0; + let peak = 0; + const gates: Array<{ release: () => void }> = []; + const runs: Array> = []; + const park = (lane: string) => { + const g = gate(); + gates.push(g); + runs.push( + enqueueCommandInLane(lane, async () => { + active += 1; + // Peak is sampled on entry, before anything can retire, so work + // admitted inside the publication window cannot escape the count. + peak = Math.max(peak, active); + await g.promise; + active -= 1; + }), + ); + }; + for (let i = 0; i < 12; i++) { + park(CRON); + } + for (let i = 0; i < 6; i++) { + park(HOOK); + } + await settle(); + expect(active).toBe(0); // nothing may run before publication + + publishLaneConfiguration({ + lanes: { [CRON]: 8, [HOOK]: 4 }, + groups: { + [GROUP]: { + budget: 8, + members: [CRON, HOOK], + reservations: { [HOOK]: 1 }, + }, + }, + }); + await settle(); + + // The assertion the review asked for: peak, not final state. + expect(peak).toBeLessThanOrEqual(8); + // And not vacuous — publication must actually have dispatched to the cap. + expect(peak).toBe(8); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + test("a rejected configuration does not leave lanes widened and dispatching", async () => { + setCommandLaneConcurrency(CRON, 0); + const gates = Array.from({ length: 4 }, () => gate()); + const runs = gates.map((g) => enqueueCommandInLane(CRON, async () => await g.promise)); + await settle(); + + // sum(reservations) > budget is rejected. Validation must happen before any + // drain, or the lane is left open at width 8 governed by no group at all. + expect(() => + publishLaneConfiguration({ + lanes: { [CRON]: 8 }, + groups: { + [GROUP]: { + budget: 2, + members: [CRON, HOOK], + reservations: { [CRON]: 2, [HOOK]: 1 }, + }, + }, + }), + ).toThrow(/reserves 3 slots but its budget is 2/); + await settle(); + + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(0); + + for (const g of gates) { + g.release(); + } + // The lane never opened, so this work is still queued. resetAllLanes + // PRESERVES queued entries by design, so it would never settle these — + // clearCommandLane rejects them instead. + clearCommandLane(CRON); + await Promise.allSettled(runs); + }); + + test("a rejected configuration does not leave lane maxima mutated", async () => { + // Stronger than asserting activeCount === 0 after the throw: that only + // proves no commit-time drain ran, not that the lane was left alone. If + // phase 1 widens a lane and group validation then throws, the lane sits at + // the new width governed by NO group, and the next unrelated drain trigger + // dispatches the preserved queue ungoverned. + setCommandLaneConcurrency(CRON, 0); + const gates = Array.from({ length: 4 }, () => gate()); + const runs = gates.map((g) => enqueueCommandInLane(CRON, async () => await g.promise)); + await settle(); + expect(getCommandLaneSnapshot(CRON).maxConcurrent).toBe(0); + + expect(() => + publishLaneConfiguration({ + lanes: { [CRON]: 8 }, + groups: { + [GROUP]: { + budget: 2, + members: [CRON, HOOK], + reservations: { [CRON]: 2, [HOOK]: 1 }, + }, + }, + }), + ).toThrow(/reserves 3 slots but its budget is 2/); + await settle(); + + // The lane must be exactly as it was before the rejected publish. + expect(getCommandLaneSnapshot(CRON).maxConcurrent).toBe(0); + expect(getCommandLaneSnapshot(CRON).group).toBeUndefined(); + + // And a later drain trigger must not dispatch the queue that was preserved + // across the failed publish. + const extra = gate(); + const extraRun = enqueueCommandInLane(CRON, async () => await extra.promise); + await settle(); + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(0); + + for (const g of gates) { + g.release(); + } + extra.release(); + clearCommandLane(CRON); + await Promise.allSettled([...runs, extraRun]); + }); + + test("a rejected replacement does not tear down the existing group first", async () => { + // costaff round-5: combining clearGroups with an invalid replacement is the + // worst case — the old group could be removed before the new one throws, + // leaving BOTH lane width and group membership partially committed. Phase 0 + // validation has to run before the clear, not just before the install. + publishLaneConfiguration({ + lanes: { [CRON]: 8, [HOOK]: 1 }, + groups: { + [GROUP]: { budget: 8, members: [CRON, HOOK], reservations: { [HOOK]: 1 } }, + }, + }); + expect(getCommandLaneSnapshot(CRON).group).toBe(GROUP); + + expect(() => + publishLaneConfiguration({ + lanes: { [CRON]: 99 }, + clearGroups: [GROUP], + groups: { + "replacement-group": { + budget: 1, + members: [CRON, HOOK], + reservations: { [CRON]: 1, [HOOK]: 1 }, + }, + }, + }), + ).toThrow(/reserves 2 slots but its budget is 1/); + + // Everything must be exactly as before: group intact, width untouched. + expect(getCommandLaneSnapshot(CRON).group).toBe(GROUP); + expect(getCommandLaneSnapshot(CRON).groupBudget).toBe(8); + expect(getCommandLaneSnapshot(CRON).maxConcurrent).toBe(8); + expect(getCommandLaneSnapshot(HOOK).reservedForLane).toBe(1); + }); + + test("publication wakes members when a replacement frees capacity", async () => { + // costaff round-5: the exported primitive's "replace" semantics were not + // self-waking. publishLaneConfiguration drains at commit, but a direct + // A publication that widens a budget or drops a reservation would + // leave queued members stuck until some unrelated enqueue poked the lane. + setCommandLaneConcurrency(CRON, 8); + setCommandLaneConcurrency(HOOK, 1); + setCommandLaneGroup(GROUP, { budget: 2, members: [CRON, HOOK] }); + + const gates = Array.from({ length: 5 }, () => gate()); + const runs = gates.map((g) => enqueueCommandInLane(CRON, async () => await g.promise)); + await settle(); + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(2); + expect(getCommandLaneSnapshot(CRON).queuedCount).toBe(3); + + // Widen the budget via the bare primitive — no publication involved. + setCommandLaneGroup(GROUP, { budget: 5, members: [CRON, HOOK] }); + await settle(); + + // The queued work must start on the replacement itself. + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(5); + expect(getCommandLaneSnapshot(CRON).queuedCount).toBe(0); + + for (const g of gates) { + g.release(); + } + await Promise.all(runs); + }); + + test("republishing a narrower budget does not admit beyond the new cap", async () => { + publishLaneConfiguration({ + lanes: { [CRON]: 8, [HOOK]: 1 }, + groups: { + [GROUP]: { budget: 8, members: [CRON, HOOK], reservations: { [HOOK]: 1 } }, + }, + }); + + const gates = Array.from({ length: 3 }, () => gate()); + const runs = gates.map((g) => enqueueCommandInLane(CRON, async () => await g.promise)); + await settle(); + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(3); + + // Narrowing mid-flight cannot evict running work, but it must not admit + // more: the group is already over its new budget. + publishLaneConfiguration({ + lanes: { [CRON]: 8, [HOOK]: 1 }, + groups: { + [GROUP]: { budget: 2, members: [CRON, HOOK], reservations: { [HOOK]: 1 } }, + }, + }); + const extra = gate(); + const blocked = enqueueCommandInLane(CRON, async () => await extra.promise); + await settle(); + + expect(getCommandLaneSnapshot(CRON).activeCount).toBe(3); + expect(getCommandLaneSnapshot(CRON).blockedBy).toBe("group-budget"); + + for (const g of gates) { + g.release(); + } + extra.release(); + clearCommandLane(CRON); + await Promise.allSettled([...runs, blocked]); + }); +}); diff --git a/src/process/command-queue.state.ts b/src/process/command-queue.state.ts new file mode 100644 index 000000000000..8b0d1665d459 --- /dev/null +++ b/src/process/command-queue.state.ts @@ -0,0 +1,106 @@ +// Shared command-queue runtime state, split out of command-queue.ts so the +// capacity-group policy can read lane state without importing the queue itself. +import { resolveGlobalSingleton } from "../shared/global-singleton.js"; +import { CommandLane } from "./lanes.js"; + +export type CommandLaneTaskMarker = Readonly<{ + lane: string; + taskId: number; + generation: number; +}>; + +export type QueueEntry = { + task: (marker: CommandLaneTaskMarker) => Promise; + resolve: (value: unknown) => void; + reject: (reason?: unknown) => void; + enqueuedAt: number; + sequence: number; + priority: number; + warnAfterMs: number; + queuedAheadAtEnqueue: number; + activeAheadAtEnqueue: number; + taskTimeoutMs?: number; + taskTimeoutProgressAtMs?: () => number | undefined; + taskTimeoutAbortSignal?: AbortSignal; + taskTimeoutAbortGraceMs?: number; + taskTimeoutReleaseSignal?: AbortSignal; + onWait?: (waitMs: number, queuedAhead: number) => void; +}; + +export type LaneState = { + lane: string; + queue: QueueEntry[]; + activeTaskIds: Set; + maxConcurrent: number; + draining: boolean; + generation: number; +}; + +export type ActiveTaskWaiter = { + activeTaskIds: Set; + resolve: (value: { drained: boolean }) => void; + timeout?: ReturnType; +}; + +/** + * Keep queue runtime state on globalThis so every bundled entry/chunk shares + * the same lanes, counters, and draining flag in production builds. + */ +const COMMAND_QUEUE_STATE_KEY = Symbol.for("openclaw.commandQueueState"); + +export function getQueueState() { + const state = resolveGlobalSingleton(COMMAND_QUEUE_STATE_KEY, () => ({ + lanes: new Map(), + activeTaskWaiters: new Set(), + nextTaskId: 1, + nextQueueSequence: 1, + })); + // Schema migration: the singleton may have been created by an older code + // version (e.g. v2026.4.2) that did not include `activeTaskWaiters`. After + // a SIGUSR1 in-process restart the new code inherits the stale object via + // `resolveGlobalSingleton` because the Symbol key already exists on + // globalThis. Patch the missing field so all downstream consumers see a + // valid Set instead of `undefined`. + if (!state.activeTaskWaiters) { + state.activeTaskWaiters = new Set(); + } + if (!state.nextQueueSequence) { + state.nextQueueSequence = 1; + } + let maxQueueSequence = state.nextQueueSequence - 1; + for (const lane of state.lanes.values()) { + for (const [index, entry] of ( + lane.queue as Array< + QueueEntry & { + activeAheadAtEnqueue?: number; + priority?: number; + queuedAheadAtEnqueue?: number; + sequence?: number; + } + > + ).entries()) { + if (typeof entry.priority !== "number") { + entry.priority = 0; + } + if (typeof entry.sequence !== "number") { + entry.sequence = state.nextQueueSequence++; + } else { + maxQueueSequence = Math.max(maxQueueSequence, entry.sequence); + } + if (typeof entry.queuedAheadAtEnqueue !== "number") { + entry.queuedAheadAtEnqueue = index; + } + if (typeof entry.activeAheadAtEnqueue !== "number") { + entry.activeAheadAtEnqueue = lane.activeTaskIds.size; + } + } + } + if (state.nextQueueSequence <= maxQueueSequence) { + state.nextQueueSequence = maxQueueSequence + 1; + } + return state; +} + +export function normalizeLane(lane: string): string { + return lane.trim() || CommandLane.Main; +} diff --git a/src/process/command-queue.ts b/src/process/command-queue.ts index eb124f96e217..aa345a1b639a 100644 --- a/src/process/command-queue.ts +++ b/src/process/command-queue.ts @@ -4,7 +4,6 @@ import { logLaneDequeue, logLaneEnqueue, } from "../logging/diagnostic-runtime.js"; -import { resolveGlobalSingleton } from "../shared/global-singleton.js"; import { clampPositiveTimerTimeoutMs } from "../shared/number-coercion.js"; import type { CommandQueueEnqueueOptions } from "./command-queue.types.js"; import { @@ -15,7 +14,29 @@ import { resetGatewayWorkAdmission, } from "./gateway-work-admission.js"; export { GatewayDrainingError } from "./gateway-work-admission.js"; +import { + canAdmitInGroup, + type CommandLaneBlockReason, + type CommandLaneGroupSpec, + drainGroupSiblings, + getGroupRegistry, + getLaneGroup, + getMemberActiveCount, + installCommandLaneGroup, + type LaneGroupState, + resolveLaneBlockReason, + validateCommandLaneGroupSpec, +} from "./command-queue.capacity-groups.js"; +import { + type ActiveTaskWaiter, + type CommandLaneTaskMarker, + getQueueState, + type LaneState, + normalizeLane, + type QueueEntry, +} from "./command-queue.state.js"; import { CommandLane } from "./lanes.js"; +export type { CommandLaneTaskMarker } from "./command-queue.state.js"; /** * Dedicated error type thrown when a queued command is rejected because * its lane was cleared. Callers that fire-and-forget enqueued tasks can @@ -76,39 +97,6 @@ export function isCommandLaneTaskTimeoutError(err: unknown, lane?: string): bool // low-risk parallelism (e.g. cron jobs) without interleaving stdin / logs for // the main auto-reply workflow. -export type CommandLaneTaskMarker = Readonly<{ - lane: string; - taskId: number; - generation: number; -}>; - -type QueueEntry = { - task: (marker: CommandLaneTaskMarker) => Promise; - resolve: (value: unknown) => void; - reject: (reason?: unknown) => void; - enqueuedAt: number; - sequence: number; - priority: number; - warnAfterMs: number; - queuedAheadAtEnqueue: number; - activeAheadAtEnqueue: number; - taskTimeoutMs?: number; - taskTimeoutProgressAtMs?: () => number | undefined; - taskTimeoutAbortSignal?: AbortSignal; - taskTimeoutAbortGraceMs?: number; - taskTimeoutReleaseSignal?: AbortSignal; - onWait?: (waitMs: number, queuedAhead: number) => void; -}; - -type LaneState = { - lane: string; - queue: QueueEntry[]; - activeTaskIds: Set; - maxConcurrent: number; - draining: boolean; - generation: number; -}; - export type CommandLaneSnapshot = { lane: string; queuedCount: number; @@ -116,12 +104,20 @@ export type CommandLaneSnapshot = { maxConcurrent: number; draining: boolean; generation: number; -}; - -type ActiveTaskWaiter = { - activeTaskIds: Set; - resolve: (value: { drained: boolean }) => void; - timeout?: ReturnType; + /** Group this lane belongs to, if any. */ + group?: string; + /** Sum of active tasks across every member of the group. Always derived. */ + groupActive?: number; + /** Hard aggregate cap shared by the group's members. */ + groupBudget?: number; + /** Slots within the budget this lane may always claim. */ + reservedForLane?: number; + /** + * Why this lane cannot start more work right now, or null if it can. + * `lane` is the lane's own maxConcurrent; the other two are group-imposed and + * are invisible to a lane-local view — see `noteLaneWaitIfBusy`. + */ + blockedBy?: CommandLaneBlockReason; }; function isExpectedNonErrorLaneFailure(err: unknown): boolean { @@ -138,82 +134,32 @@ function isQuietProbeLane(lane: string): boolean { ); } -/** - * Keep queue runtime state on globalThis so every bundled entry/chunk shares - * the same lanes, counters, and draining flag in production builds. - */ -const COMMAND_QUEUE_STATE_KEY = Symbol.for("openclaw.commandQueueState"); - -function getQueueState() { - const state = resolveGlobalSingleton(COMMAND_QUEUE_STATE_KEY, () => ({ - lanes: new Map(), - activeTaskWaiters: new Set(), - nextTaskId: 1, - nextQueueSequence: 1, - })); - // Schema migration: the singleton may have been created by an older code - // version (e.g. v2026.4.2) that did not include `activeTaskWaiters`. After - // a SIGUSR1 in-process restart the new code inherits the stale object via - // `resolveGlobalSingleton` because the Symbol key already exists on - // globalThis. Patch the missing field so all downstream consumers see a - // valid Set instead of `undefined`. - if (!state.activeTaskWaiters) { - state.activeTaskWaiters = new Set(); - } - if (!state.nextQueueSequence) { - state.nextQueueSequence = 1; - } - let maxQueueSequence = state.nextQueueSequence - 1; - for (const lane of state.lanes.values()) { - for (const [index, entry] of ( - lane.queue as Array< - QueueEntry & { - activeAheadAtEnqueue?: number; - priority?: number; - queuedAheadAtEnqueue?: number; - sequence?: number; - } - > - ).entries()) { - if (typeof entry.priority !== "number") { - entry.priority = 0; - } - if (typeof entry.sequence !== "number") { - entry.sequence = state.nextQueueSequence++; - } else { - maxQueueSequence = Math.max(maxQueueSequence, entry.sequence); - } - if (typeof entry.queuedAheadAtEnqueue !== "number") { - entry.queuedAheadAtEnqueue = index; - } - if (typeof entry.activeAheadAtEnqueue !== "number") { - entry.activeAheadAtEnqueue = lane.activeTaskIds.size; - } - } - } - if (state.nextQueueSequence <= maxQueueSequence) { - state.nextQueueSequence = maxQueueSequence + 1; - } - return state; -} - -function normalizeLane(lane: string): string { - return lane.trim() || CommandLane.Main; -} - function getLaneDepth(state: LaneState): number { return state.queue.length + state.activeTaskIds.size; } function createCommandLaneSnapshot(state: LaneState): CommandLaneSnapshot { - return { + const snapshot: CommandLaneSnapshot = { lane: state.lane, queuedCount: state.queue.length, activeCount: state.activeTaskIds.size, maxConcurrent: state.maxConcurrent, draining: state.draining, generation: state.generation, + blockedBy: resolveLaneBlockReason(state.lane), }; + const group = getLaneGroup(state.lane); + if (group) { + let groupActive = 0; + for (const member of group.members) { + groupActive += getMemberActiveCount(member); + } + snapshot.group = group.group; + snapshot.groupActive = groupActive; + snapshot.groupBudget = group.budget; + snapshot.reservedForLane = group.reservations.get(state.lane) ?? 0; + } + return snapshot; } function getLaneState(lane: string): LaneState { @@ -471,7 +417,11 @@ function drainLane(lane: string) { const pump = () => { try { - while (state.activeTaskIds.size < state.maxConcurrent && state.queue.length > 0) { + while ( + state.activeTaskIds.size < state.maxConcurrent && + state.queue.length > 0 && + canAdmitInGroup(lane) + ) { const entry = state.queue.shift() as QueueEntry; const waitedMs = Date.now() - entry.enqueuedAt; if (waitedMs >= entry.warnAfterMs) { @@ -504,6 +454,8 @@ function drainLane(lane: string) { `lane task done: lane=${lane} durationMs=${Date.now() - startTime} active=${state.activeTaskIds.size} queued=${state.queue.length}`, ); pump(); + // Freed capacity belongs to the group, not to this lane. + drainGroupSiblings(lane, drainLane); } entry.resolve(result); } catch (err) { @@ -521,6 +473,9 @@ function drainLane(lane: string) { if (completedCurrentGeneration) { notifyActiveTaskWaiters(); pump(); + // A failed task releases group capacity exactly like a successful + // one; siblings must be woken on both paths. + drainGroupSiblings(lane, drainLane); } entry.reject(err); } @@ -547,6 +502,70 @@ export function isGatewayDraining(): boolean { return isGatewayWorkAdmissionClosed(); } +/** + * Apply lane concurrencies and group definitions as ONE transaction. + * + * `setCommandLaneConcurrency` drains the instant a lane goes positive, and + * gateway publication is sequential — so applying lanes one at a time can widen + * a member and let it dispatch BEFORE its group exists, admitting work above + * the budget the group was meant to enforce. Suppressing drains until every + * lane max and every group definition is installed closes that window; a single + * commit-time drain pass then dispatches under the final configuration. + * + * Callers must route grouped lanes through here rather than the per-lane + * setter, which cannot know about a group that does not exist yet. + */ +export function publishLaneConfiguration(config: { + lanes?: Readonly>; + groups?: Readonly>; + /** Groups to remove as part of the same transaction. */ + clearGroups?: readonly string[]; +}): void { + // Phase 0 — validate EVERYTHING before mutating anything. Validating inside + // the install loop would leave already-widened lanes behind on a throw: + // governed by no group, and dispatching their preserved queue on the next + // unrelated drain trigger. Rejection must be a no-op, not a partial apply. + const validated: LaneGroupState[] = []; + for (const [group, spec] of Object.entries(config.groups ?? {})) { + validated.push(validateCommandLaneGroupSpec(group, spec)); + } + + const touched = new Set(); + // Phase 1 — install state with dispatch suppressed. Nothing may start here. + for (const [rawLane, maxConcurrent] of Object.entries(config.lanes ?? {})) { + const lane = normalizeLane(rawLane); + const state = getLaneState(lane); + const minConcurrent = isQuietProbeLane(lane) ? 1 : 0; + state.maxConcurrent = Math.max(minConcurrent, Math.floor(maxConcurrent)); + touched.add(lane); + } + for (const group of config.clearGroups ?? []) { + const { groups, groupByLane } = getGroupRegistry(); + const existing = groups.get(group); + if (existing) { + for (const member of existing.members) { + groupByLane.delete(member); + touched.add(member); + } + groups.delete(group); + } + } + for (const next of validated) { + installCommandLaneGroup(next); + for (const member of next.members) { + touched.add(member); + } + } + // Phase 2 — commit. Group membership and budgets are now final, so every + // admission decision in this pass sees the configuration the caller intended. + for (const lane of touched) { + const state = getQueueState().lanes.get(lane); + if (state && state.maxConcurrent > 0 && state.queue.length > 0 && !state.draining) { + drainLane(lane); + } + } +} + export function setCommandLaneConcurrency(lane: string, maxConcurrent: number) { const cleaned = normalizeLane(lane); const state = getLaneState(cleaned); @@ -606,14 +625,30 @@ export function getCommandLaneSnapshot(lane: string = CommandLane.Main): Command const resolved = normalizeLane(lane); const state = getQueueState().lanes.get(resolved); if (!state) { - return { + // The lane may not exist yet (first enqueue) or may have been retired while + // idle, but it can still be a configured group member — and a caller asking + // "can this lane start work?" needs the group answer, not a bare default. + const group = getLaneGroup(resolved); + const empty: CommandLaneSnapshot = { lane: resolved, queuedCount: 0, activeCount: 0, maxConcurrent: 1, draining: false, generation: 0, + blockedBy: resolveLaneBlockReason(resolved), }; + if (group) { + let groupActive = 0; + for (const member of group.members) { + groupActive += getMemberActiveCount(member); + } + empty.group = group.group; + empty.groupActive = groupActive; + empty.groupBudget = group.budget; + empty.reservedForLane = group.reservations.get(resolved) ?? 0; + } + return empty; } return createCommandLaneSnapshot(state); } @@ -676,6 +711,8 @@ export function resetCommandLane(lane: string = CommandLane.Main): number { if (state.queue.length > 0) { drainLane(cleaned); } + // Clearing activeTaskIds released group capacity; siblings may now admit. + drainGroupSiblings(cleaned, drainLane); notifyActiveTaskWaiters(); return released; } diff --git a/src/process/lanes.ts b/src/process/lanes.ts index 8ff40807c3de..af8abef50830 100644 --- a/src/process/lanes.ts +++ b/src/process/lanes.ts @@ -4,6 +4,12 @@ export const enum CommandLane { SystemAgent = "system-agent", Cron = "cron", CronNested = "cron-nested", + /** + * External hook agent-run dispatch. Distinct from `cron-nested` so hook work + * is schedulable in its own right; capacity is bounded by the shared lane + * group rather than by adding a slot outside the cron budget. + */ + HookDispatch = "hook-dispatch", SkillWorkshopReview = "skill-workshop-review", Subagent = "subagent", Nested = "nested", diff --git a/test/gateway-hook-concurrency.e2e.test.ts b/test/gateway-hook-concurrency.e2e.test.ts new file mode 100644 index 000000000000..b96ebfdcb06c --- /dev/null +++ b/test/gateway-hook-concurrency.e2e.test.ts @@ -0,0 +1,354 @@ +// E2E: hook dispatch uses every free slot inside the shared cron budget. +import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../src/config/types.openclaw.js"; +import { + createOpenClawTestInstance, + type OpenClawTestInstance, +} from "./helpers/openclaw-test-instance.js"; + +const TEST_TIMEOUT_MS = 180_000; +const MODEL_REF = "hook-concurrency/hook-concurrency"; +const SHARED_BUDGET = 8; + +type Deferred = { + promise: Promise; + resolve: () => void; +}; + +type HookResponse = { + body: string; + status: number; +}; + +type HeldModelServer = { + active: () => number; + close: () => Promise; + hold: () => void; + peak: () => number; + releaseAll: () => void; + requestCount: () => number; + url: string; +}; + +const instances: OpenClawTestInstance[] = []; +const modelServers: HeldModelServer[] = []; + +afterEach(async () => { + await Promise.allSettled(instances.splice(0).map((instance) => instance.cleanup())); + await Promise.allSettled(modelServers.splice(0).map((server) => server.close())); +}); + +describe("Gateway hook concurrency", () => { + it( + "admits eight hooks, times out the queued ninth, then admits after release", + { timeout: TEST_TIMEOUT_MS }, + async () => { + const modelServer = await startHeldModelServer(); + modelServers.push(modelServer); + const instance = await createOpenClawTestInstance({ + name: "gateway-hook-concurrency", + config: createTestConfig(modelServer.url), + env: { OPENCLAW_SKIP_PROVIDERS: undefined }, + }); + instances.push(instance); + await instance.startGateway(); + + // Pay the one-time plugin, session, and model-runtime preparation cost + // before measuring steady-state lane admission. + await warmGatewayHook(instance, modelServer); + modelServer.hold(); + + const responses: Array = Array.from({ + length: SHARED_BUDGET + 1, + }); + const requests = responses.map((_, index) => + postHook(instance, index).then((response) => { + responses[index] = response; + return response; + }), + ); + await vi.waitFor( + () => + expect(responses.filter((response) => response?.status === 200)).toHaveLength( + SHARED_BUDGET, + ), + { interval: 20, timeout: 30_000 }, + ); + await vi.waitFor(() => expect(responses.every(Boolean)).toBe(true), { + interval: 20, + timeout: 30_000, + }); + + expect(responses.filter((response) => response?.status === 200)).toHaveLength(SHARED_BUDGET); + const timedOut = responses.find((response) => response?.status === 503); + expect(timedOut?.status).toBe(503); + expect(JSON.parse(timedOut?.body ?? "{}")).toMatchObject({ + ok: false, + error: "hook agent run did not start before admission timeout", + runId: expect.any(String), + }); + expect(modelServer.active(), instance.logs()).toBeGreaterThan(0); + expect(modelServer.peak(), instance.logs()).toBeLessThanOrEqual(SHARED_BUDGET); + expect(modelServer.requestCount(), instance.logs()).toBeLessThanOrEqual(SHARED_BUDGET + 1); + + // Completing the admitted work frees shared capacity. A fresh request + // must then cross the same real Gateway admission fence. + const requestCountBeforeRelease = modelServer.requestCount(); + modelServer.releaseAll(); + await expect(Promise.all(requests)).resolves.toHaveLength(SHARED_BUDGET + 1); + const afterRelease = await postHook(instance, SHARED_BUDGET + 1); + expect(afterRelease.status, afterRelease.body).toBe(200); + await vi.waitFor( + () => expect(modelServer.requestCount()).toBeGreaterThan(requestCountBeforeRelease), + { interval: 20, timeout: 30_000 }, + ); + await vi.waitFor(() => expect(modelServer.active()).toBe(0), { + interval: 20, + timeout: 30_000, + }); + }, + ); +}); + +function createTestConfig(baseUrl: string): OpenClawConfig { + return { + plugins: { slots: { memory: "none" } }, + hooks: { + enabled: true, + allowRequestSessionKey: true, + allowedSessionKeyPrefixes: ["hook:"], + }, + agents: { + defaults: { + heartbeat: { every: "0m" }, + model: { primary: MODEL_REF }, + models: { [MODEL_REF]: { agentRuntime: { id: "openclaw" } } }, + skipBootstrap: true, + skills: [], + }, + }, + tools: { profile: "minimal" }, + models: { + mode: "replace", + providers: { + "hook-concurrency": { + baseUrl: `${baseUrl}/v1`, + apiKey: "test-token-placeholder", + api: "openai-responses", + request: { allowPrivateNetwork: true }, + models: [ + { + id: "hook-concurrency", + name: "hook-concurrency", + api: "openai-responses", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 128_000, + maxTokens: 4_096, + }, + ], + }, + }, + }, + }; +} + +async function warmGatewayHook( + instance: OpenClawTestInstance, + modelServer: HeldModelServer, +): Promise { + for (let attempt = 0; attempt < 3; attempt += 1) { + const requestCountBefore = modelServer.requestCount(); + const response = await postHook(instance, -(attempt + 1)); + if (response.status === 200) { + await vi.waitFor( + () => expect(modelServer.requestCount()).toBeGreaterThan(requestCountBefore), + { interval: 20, timeout: 30_000 }, + ); + await vi.waitFor(() => expect(modelServer.active()).toBe(0), { + interval: 20, + timeout: 30_000, + }); + return; + } + expect(response.status, response.body).toBe(503); + expect(JSON.parse(response.body)).toMatchObject({ + ok: false, + error: "hook agent run did not start before admission timeout", + }); + } + throw new Error("Gateway hook warmup did not reach the model after three attempts"); +} + +async function postHook(instance: OpenClawTestInstance, index: number): Promise { + const response = await fetch(`http://127.0.0.1:${instance.port}/hooks/agent`, { + method: "POST", + headers: { + Authorization: `Bearer ${instance.hookToken}`, + "Content-Type": "application/json", + "Idempotency-Key": `hook-concurrency-${index}`, + }, + body: JSON.stringify({ + message: `hook concurrency request ${index}`, + name: `Hook concurrency ${index}`, + sessionKey: `hook:concurrency:${index}`, + sessionMode: "persistent", + deliver: false, + }), + }); + return { + body: await response.text(), + status: response.status, + }; +} + +function createDeferred(): Deferred { + let resolve!: () => void; + const promise = new Promise((settle) => { + resolve = settle; + }); + return { promise, resolve }; +} + +async function startHeldModelServer(): Promise { + const releases: Deferred[] = []; + let holdRequests = false; + let active = 0; + let peak = 0; + let requestCount = 0; + const server = createServer((request, response) => { + void handleModelRequest(request, response).catch((error: unknown) => { + if (response.destroyed) { + return; + } + response.writeHead(500, { "content-type": "application/json" }); + response.end(JSON.stringify({ error: { message: String(error) } })); + }); + }); + + async function handleModelRequest( + request: IncomingMessage, + response: ServerResponse, + ): Promise { + const url = new URL(request.url ?? "/", "http://127.0.0.1"); + if (request.method === "GET" && url.pathname === "/v1/models") { + response.writeHead(200, { "content-type": "application/json" }); + response.end(JSON.stringify({ data: [{ id: "hook-concurrency", object: "model" }] })); + return; + } + if (request.method !== "POST" || url.pathname !== "/v1/responses") { + response.writeHead(404).end(); + return; + } + + await drainRequest(request); + const index = requestCount; + requestCount += 1; + const release = createDeferred(); + releases[index] = release; + if (!holdRequests) { + release.resolve(); + } + active += 1; + peak = Math.max(peak, active); + try { + await release.promise; + if (!response.destroyed) { + writeModelResponse(response, index); + } + } finally { + active -= 1; + } + } + + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("hook concurrency model server did not bind"); + } + + const releaseAll = () => { + holdRequests = false; + for (const release of releases) { + release?.resolve(); + } + }; + return { + active: () => active, + hold: () => { + holdRequests = true; + }, + peak: () => peak, + releaseAll, + requestCount: () => requestCount, + url: `http://127.0.0.1:${address.port}`, + close: async () => { + releaseAll(); + server.closeAllConnections(); + await new Promise((resolve) => { + server.close(() => resolve()); + }); + }, + }; +} + +async function drainRequest(request: IncomingMessage): Promise { + for await (const chunk of request) { + void chunk; + } +} + +function writeModelResponse(response: ServerResponse, sequence: number): void { + const text = `hook concurrency response ${sequence}`; + const message = { + type: "message", + id: `hook-concurrency-message-${sequence}`, + role: "assistant", + status: "completed", + content: [{ type: "output_text", text, annotations: [] }], + }; + const events = [ + { + type: "response.output_item.added", + output_index: 0, + item: { ...message, status: "in_progress", content: [] }, + }, + { + type: "response.output_text.delta", + item_id: message.id, + output_index: 0, + content_index: 0, + delta: text, + }, + { + type: "response.output_text.done", + item_id: message.id, + output_index: 0, + content_index: 0, + text, + }, + { type: "response.output_item.done", output_index: 0, item: message }, + { + type: "response.completed", + response: { + id: `hook-concurrency-response-${sequence}`, + status: "completed", + output: [message], + usage: { input_tokens: 1, output_tokens: 1, total_tokens: 2 }, + }, + }, + ]; + response.writeHead(200, { + "content-type": "text/event-stream", + "cache-control": "no-store", + connection: "keep-alive", + }); + response.end( + `${events.map((event) => `data: ${JSON.stringify(event)}\n\n`).join("")}data: [DONE]\n\n`, + ); +}