From c27a2b20d0002f8d150edbd72a934ca0a02bff54 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 16 Jun 2026 15:55:13 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20fix:=20Codex=20final=20?= =?UTF-8?q?round=20=E2=80=94=20paused-job=20TTL=20+=20pendingAction=20in?= =?UTF-8?q?=20resume=20contract?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of three findings on e7d9cf21b6 (third deferred to Slice B): - K2 paused-job TTL: a paused (requires_action) job no longer inherits the 20-minute running TTL — it uses a dedicated requires_action backstop (default 24h, configurable) so a no-expiry approval (the buildPendingAction default), which the API treats as live, isn't evicted by Redis mid-window. A longer pendingAction.expiresAt still extends beyond the backstop. - K3 resume contract: pendingAction is now carried on the typed ResumeState (data-provider) and populated by getResumeState for a live paused job, so a reloading / cross-replica client can rebuild the prompt from resumeState (the contract useResumeOnLoad actually reads), not just a loose status field. Deferred (Slice B): K1 — emit a terminal SSE event on expiry so already- subscribed clients close. Requires the manager/eventTransport layer (the store-level lifecycle and cleanup loops have no transport access) and has no live subscriber until the Slice B subscribe/resume path exists; tracked there. tsc + lint clean; policy + type-contract specs pass. --- .../api/src/stream/GenerationJobManager.ts | 7 +++++ .../stream/implementations/RedisJobStore.ts | 27 ++++++++++++++----- packages/data-provider/src/types/agents.ts | 6 +++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/api/src/stream/GenerationJobManager.ts b/packages/api/src/stream/GenerationJobManager.ts index e145d7ed17..05422c462b 100644 --- a/packages/api/src/stream/GenerationJobManager.ts +++ b/packages/api/src/stream/GenerationJobManager.ts @@ -20,6 +20,7 @@ import { import { InMemoryEventTransport } from './implementations/InMemoryEventTransport'; import { InMemoryJobStore } from './implementations/InMemoryJobStore'; import { filterPersistableAbortContent } from './abortContent'; +import { isPendingActionStale } from './interfaces/IJobStore'; import { ApprovalLifecycle } from './ApprovalLifecycle'; /** Error surfaced to any client still attached when a stale/hung job is reaped. */ @@ -1478,6 +1479,12 @@ class GenerationJobManagerClass { replayEvents, collectedUsage, contextUsage, + // Carry the live pending approval in the resume contract so a reloading / + // cross-replica client can rebuild the prompt from resumeState. + pendingAction: + jobData.status === 'requires_action' && !isPendingActionStale(jobData) + ? jobData.pendingAction + : undefined, }; } diff --git a/packages/api/src/stream/implementations/RedisJobStore.ts b/packages/api/src/stream/implementations/RedisJobStore.ts index b82f3ab393..8c12203fe6 100644 --- a/packages/api/src/stream/implementations/RedisJobStore.ts +++ b/packages/api/src/stream/implementations/RedisJobStore.ts @@ -83,6 +83,14 @@ const DEFAULT_TTL = { runStepsAfterComplete: 0, /** Safety-net TTL for per-user job tracking sets (24 hours). Refreshed on each createJob. */ userJobsSet: 86400, + /** + * Backstop TTL for a job paused for human review (24 hours). A paused job is + * NOT a hung generation, so it must not inherit the 20-minute running TTL — + * an approval with no explicit `expiresAt` is "live" per the API contract and + * would otherwise be evicted mid-window. A pendingAction with a longer + * `expiresAt` extends beyond this (see pauseTtlSeconds). + */ + requiresAction: 86400, }; /** @@ -117,6 +125,8 @@ export interface RedisJobStoreOptions { runStepsAfterCompleteTtl?: number; /** TTL for per-user job tracking sets in seconds (default: 86400 = 24 hours). 0 = no TTL. */ userJobsSetTtl?: number; + /** Backstop TTL for a paused (requires_action) job in seconds (default: 86400 = 24 hours). */ + requiresActionTtl?: number; } export class RedisJobStore implements IJobStore { @@ -152,6 +162,7 @@ export class RedisJobStore implements IJobStore { chunksAfterComplete: options?.chunksAfterCompleteTtl ?? DEFAULT_TTL.chunksAfterComplete, runStepsAfterComplete: options?.runStepsAfterCompleteTtl ?? DEFAULT_TTL.runStepsAfterComplete, userJobsSet: options?.userJobsSetTtl ?? DEFAULT_TTL.userJobsSet, + requiresAction: options?.requiresActionTtl ?? DEFAULT_TTL.requiresAction, }; // Detect cluster mode using ioredis's isCluster property this.isCluster = (redis as Cluster).isCluster === true; @@ -351,19 +362,21 @@ export class RedisJobStore implements IJobStore { } /** - * Live-key TTL (seconds) for a paused job. Defaults to the running TTL but - * extends to cover a pendingAction whose `expiresAt` is farther out, plus a - * grace margin so a decision arriving right at the deadline can still resume. - * Without this, a long approval window (e.g. 1h) on the default 20m stream - * TTL would let Redis evict the paused job mid-window. + * Live-key TTL (seconds) for a paused job. A paused job isn't a hung + * generation, so it uses the longer requires_action backstop rather than the + * running TTL — otherwise a no-expiry approval (the buildPendingAction + * default), which the API treats as "live", would be evicted after the 20m + * running window. A pendingAction with an `expiresAt` farther out than the + * backstop extends to cover it, plus a grace margin so a decision arriving + * right at the deadline can still resume. */ private pauseTtlSeconds(pendingAction?: Agents.PendingAction): number { const exp = pendingAction?.expiresAt; if (exp == null) { - return this.ttl.running; + return this.ttl.requiresAction; } const secondsUntilExpiry = Math.ceil((exp - Date.now()) / 1000) + 60; - return Math.max(this.ttl.running, secondsUntilExpiry); + return Math.max(this.ttl.requiresAction, secondsUntilExpiry); } /** The membership set a status belongs to; terminal statuses have none. */ diff --git a/packages/data-provider/src/types/agents.ts b/packages/data-provider/src/types/agents.ts index 5943cce96b..6318dd651f 100644 --- a/packages/data-provider/src/types/agents.ts +++ b/packages/data-provider/src/types/agents.ts @@ -246,6 +246,12 @@ export namespace Agents { collectedUsage?: TTokenUsageEvent[]; /** Latest context window snapshot; restores the usage gauge on resume */ contextUsage?: TContextUsageEvent; + /** + * Live pending approval when the run is paused for human review. Carried in + * the resume contract (not just /chat/status) so a reloading or + * cross-replica client can rebuild and render the prompt from `resumeState`. + */ + pendingAction?: PendingAction; } /** * Represents a run step delta i.e. any changed fields on a run step during