fix(hooks): avoid false admission timeouts during runtime reload (#128975)

This commit is contained in:
Peter Steinberger
2026-08-24 22:12:54 -07:00
committed by GitHub
parent ebc70fa1b3
commit e9017714c2
6 changed files with 67 additions and 22 deletions
+3 -3
View File
@@ -634,12 +634,12 @@ Query-string tokens are rejected.
- Supplying both a concrete `channel` and `to` enables direct announce delivery.
- Set `accountId` with `channel` and `to` to select a configured, enabled account on multi-account channels. Unknown, disabled, or invalid account IDs return `400` and schedule no run.
The HTTP response waits only for runner admission, not for the agent turn to finish. A `200` may take up to 15 seconds and means the run entered its agent runner. Pre-run failures return `{ ok: false, error, runId }` with:
The HTTP response waits only for canonical session/global placement admission, not for the agent turn to finish. A `200` may take up to 15 seconds and means the execution path acquired that admission; the run may still be preparing its model runtime. Pre-admission failures return `{ ok: false, error, runId }` with:
- `400` when delivery coordinates or account selection are invalid; correct the request before retrying.
- `409` when the target session changed or otherwise rejects new work; retry after resolving the session conflict.
- `502` when Gateway or cron preparation fails before runner entry.
- `503` when runner admission does not complete within 15 seconds. Timed-out queued work is canceled and does not start later.
- `502` when Gateway or cron preparation fails before placement admission.
- `503` when placement admission does not occur within 15 seconds. Timed-out queued work is canceled and does not start later.
</Accordion>
<Accordion title="Mapped hooks (POST /hooks/<name>)">
+2 -2
View File
@@ -1114,8 +1114,8 @@ Validation and safety notes:
- Direct announce delivery requires both a concrete `channel` and `to`; supplying only one fails before the run is scheduled.
- `accountId` selects a configured, enabled account for direct announce delivery and requires both `channel` and `to`; invalid selections return `400` before a run starts.
- Omit both delivery fields for completion-only hooks, or set `deliver: false` to ignore supplied destination data.
- The request waits up to 15 seconds for runner admission, not run completion. `200` means the agent runner was entered.
- Pre-run failures return `{ ok: false, error, runId }`: `400` for invalid delivery coordinates or account selection, `409` for session admission conflicts, `502` for other preparation failures, and `503` when the 15-second admission deadline expires. Timed-out queued work is canceled and will not start later.
- The request waits up to 15 seconds for canonical session/global placement admission, not run completion. `200` means the execution path acquired that admission; the run may still be preparing its model runtime.
- Pre-admission failures return `{ ok: false, error, runId }`: `400` for invalid delivery coordinates or account selection, `409` for session admission conflicts, `502` for other preparation failures, and `503` when placement admission does not occur within 15 seconds. Timed-out queued work is canceled and will not start later.
- `POST /hooks/<name>` → resolved via `hooks.mappings`
- Template-rendered mapping `sessionKey` values are treated as externally supplied and also require `hooks.allowRequestSessionKey=true`.
- Mapped `agent` actions use the same admission wait and `200`/`400`/`409`/`502`/`503` outcomes.
@@ -207,7 +207,8 @@ describe("queued embedded run context liveness", () => {
const registeredAt = 1_000;
const admissionAt = registeredAt + CONTEXT_TTL_MS + 1;
const clock = vi.spyOn(Date, "now").mockReturnValue(registeredAt);
const { controller, params } = createRunController();
const onLaneWait = vi.fn();
const { controller, params } = createRunController({ onLaneWait });
registerAgentRunContext(params.runId, {
lifecycleGeneration: params.lifecycleGeneration,
registeredAt,
@@ -235,6 +236,7 @@ describe("queued embedded run context liveness", () => {
try {
await placementEntered.promise;
expect(onLaneWait).not.toHaveBeenCalledWith(expect.objectContaining({ waiting: false }));
expect(readAgentRunIndexVersion()).toBe(versionBeforeQueue);
clock.mockReturnValue(admissionAt);
expect(sweepStaleRunContexts()).toBe(0);
@@ -242,6 +244,11 @@ describe("queued embedded run context liveness", () => {
placementAdmitted.resolve();
await remoteStarted.promise;
expect(onLaneWait).toHaveBeenCalledExactlyOnceWith({
waitMs: 0,
queuedAhead: 0,
waiting: false,
});
expect(getAgentRunContext(params.runId)?.lastActiveAt).toBe(admissionAt);
expect(readAgentRunIndexVersion()).toBe(versionBeforeQueue + 1);
expect(localTurn).not.toHaveBeenCalled();
@@ -126,7 +126,6 @@ export function createEmbeddedRunLaneController<TParams extends LaneParams>(opti
};
const taskWithCurrentLifecycle = async () => {
let params = options.getParams();
params.onLaneWait?.({ waitMs: 0, queuedAhead: 0, waiting: false });
params.replyOperation?.markGlobalLaneWaitEnded();
throwIfAborted();
let lifecycleGeneration = options.getLifecycleGeneration();
@@ -187,6 +186,8 @@ export function createEmbeddedRunLaneController<TParams extends LaneParams>(opti
lifecycleGeneration,
lastActiveAt: Date.now(),
});
// Queue dequeue can still block on writer or placement admission.
params.onLaneWait?.({ waitMs: 0, queuedAhead: 0, waiting: false });
},
),
);
@@ -204,10 +205,6 @@ export function createEmbeddedRunLaneController<TParams extends LaneParams>(opti
};
const enqueueSession = <T>(task: () => Promise<T>, opts?: CommandQueueEnqueueOptions) => {
const sessionOpts: CommandQueueEnqueueOptions = { ...opts, priority: sessionQueuePriority };
const taskWithLaneAdmission = () => {
options.getParams().onLaneWait?.({ waitMs: 0, queuedAhead: 0, waiting: false });
return task();
};
const params = options.getParams();
// Session admission, deferred maintenance, and global admission share one queue owner.
releaseQueuedRunContext = retainQueuedAgentRunContext(
@@ -225,14 +222,10 @@ export function createEmbeddedRunLaneController<TParams extends LaneParams>(opti
let queuedRun: Promise<T>;
try {
if (params.enqueue) {
queuedRun = params.enqueue(taskWithLaneAdmission, withRunLaneWait(sessionOpts));
queuedRun = params.enqueue(task, withRunLaneWait(sessionOpts));
} else {
noteLaneWaitIfBusy(options.sessionLane);
queuedRun = enqueueCommandInLane(
options.sessionLane,
taskWithLaneAdmission,
withRunLaneWait(sessionOpts),
);
queuedRun = enqueueCommandInLane(options.sessionLane, task, withRunLaneWait(sessionOpts));
}
} catch (error) {
releaseQueuedContext("abandoned");
@@ -233,6 +233,47 @@ describe("gateway hook admission", () => {
});
});
test("admits an HTTP hook after placement while runtime preparation remains blocked", async () => {
testState.hooksConfig = { enabled: true, token: HOOK_TOKEN };
await withGatewayServer(async ({ port }) => {
const placementAdmissionPublished = createDeferred();
const runtimePreparation = createDeferred();
let runnerEntered = false;
let response: Response | undefined;
cronIsolatedRun.mockClear();
cronIsolatedRun.mockImplementationOnce(async (params: unknown) => {
const callbacks = params as {
onExecutionStarted?: () => void;
onLaneWait?: (info: { waiting: boolean }) => void;
};
callbacks.onLaneWait?.({ waiting: false });
placementAdmissionPublished.resolve();
await runtimePreparation.promise;
runnerEntered = true;
callbacks.onExecutionStarted?.();
return { status: "ok", summary: "done" };
});
const responsePromise = postHook(
port,
"/hooks/agent",
{ message: "Dispatch" },
"placement-admission-before-runtime",
).then((result) => {
response = result;
return result;
});
try {
await placementAdmissionPublished.promise;
await expect.poll(() => response?.status, { timeout: 1_000, interval: 10 }).toBe(200);
expect(runnerEntered).toBe(false);
} finally {
runtimePreparation.resolve();
await responsePromise;
}
});
});
test("shares one pending persistent dispatch without losing its session target", async () => {
testState.hooksConfig = {
enabled: true,
+9 -5
View File
@@ -422,6 +422,10 @@ export function createGatewayHooksRequestHandler(params: {
});
const admissionTimeoutError = new Error(HOOK_AGENT_START_ADMISSION_TIMEOUT_ERROR);
const startupAbortController = new AbortController();
const settleSuccessfulAdmission = () => {
startupAbortController.signal.throwIfAborted();
settleAdmission({ ok: true, runId });
};
admissionTimer = setTimeout(() => {
admissionTimedOut = true;
startupAbortController.abort(admissionTimeoutError);
@@ -493,12 +497,12 @@ export function createGatewayHooksRequestHandler(params: {
},
},
abortSignal: startupAbortController.signal,
onExecutionStarted: () => {
// Existing runner-entry callbacks are the final owner-boundary fence:
// a deadline that wins this race prevents the runner call itself.
startupAbortController.signal.throwIfAborted();
settleAdmission({ ok: true, runId });
onLaneWait: (info) => {
if (info?.waiting === false) {
settleSuccessfulAdmission();
}
},
onExecutionStarted: settleSuccessfulAdmission,
});
const result = await runWithScheduledGatewayContext({
...(scheduledGatewayContextResolver