Skip to content

Commit 7710185

Browse files
committed
fix stale update jobs and combo quota fallback
1 parent d482086 commit 7710185

8 files changed

Lines changed: 384 additions & 36 deletions

File tree

docs-site/src/content/docs/guides/web-dashboard.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ The GUI is a thin client over the proxy's JSON management API. Useful endpoints
117117
| `POST /api/startup-action` | Install the background service or Codex launcher shim through fixed, allowlisted actions. |
118118
| `GET` / `POST /api/windows-tray` | Read or change the Windows tray installation and visible-process state. POST accepts `install`, `start`, `stop`, or `uninstall`. |
119119
| `POST /api/sync` | Rebuild the shared model catalog and stale the Codex model cache. |
120-
| `GET /api/update/check` · `POST /api/update/run` · `GET /api/update/status` | Check, run, and monitor self-update jobs. |
120+
| `GET /api/update/check` · `POST /api/update/run` · `GET /api/update/status` | Check, run, and monitor self-update jobs. Worker PIDs are persisted so a crashed job recovers automatically; legacy no-PID jobs recover after ten minutes. |
121121
| `GET` / `PUT /api/sidecar-settings` | Read or set search/vision sidecar model settings. |
122122
| `GET` / `PUT /api/injection-model` | Read or set the shared sub-agent model/effort selection and the independent guidance/native-default switches. |
123123
| `GET` / `PUT /api/v2` | Read or set the surface mode, Codex feature flag, and v2 thread limit. |

docs-site/src/content/docs/reference/cli.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,3 +530,8 @@ Two dispatch targets are intentionally omitted from normal help: `__refresh-vers
530530
refreshes the update-notification cache in a detached process, and
531531
`__gui-update-worker <job-id> [latest|preview] [restart]` runs a dashboard update job. They are
532532
implementation details, not stable user-facing commands.
533+
534+
The dashboard persists the detached update worker PID and automatically recovers an active job if
535+
that worker is no longer alive. Active records written by older releases without a PID are treated
536+
as stale after ten minutes. A live worker remains protected from concurrent updates even if the
537+
update takes longer than that window.

src/server/responses/core.ts

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ import {
7272
type CodexAuthContext,
7373
} from "../../codex/auth-context";
7474
import {
75+
computeQuotaCooldown,
7576
formatCodexProviderForLog,
7677
previewCodexAccountForRequest,
7778
recordCodexUpstreamOutcome,
@@ -251,6 +252,7 @@ interface CodexPoolAccountRetryArgs {
251252
options: {
252253
abortSignal?: AbortSignal;
253254
onCodexAuthContextResolved?: (ctx: CodexAuthContext) => void;
255+
deferCodexResetDerivedCooldown?: boolean;
254256
};
255257
firstAuthCtx: Extract<CodexAuthContext, { kind: "pool" | "main-pool" }>;
256258
firstResponse: Response;
@@ -276,6 +278,31 @@ type CodexPoolAccountRetryResult =
276278
authCtx: Extract<CodexAuthContext, { kind: "pool" | "main-pool" }>;
277279
};
278280

281+
function codexQuotaOutcomeMeta(response: Response): {
282+
retryAfter: string | null;
283+
resetAt: string[];
284+
} {
285+
return {
286+
retryAfter: response.headers.get("retry-after"),
287+
resetAt: [
288+
response.headers.get("x-codex-primary-reset-at"),
289+
response.headers.get("x-codex-secondary-reset-at"),
290+
response.headers.get("x-codex-tertiary-reset-at"),
291+
].filter((value): value is string => !!value),
292+
};
293+
}
294+
295+
/**
296+
* A reset timestamp describes a quota window, not an explicit instruction to
297+
* stop using the whole account. A combo may therefore try a later model in the
298+
* same request, while Retry-After and headerless quota failures remain blocking.
299+
*/
300+
function shouldDeferCodexResetDerivedCooldown(response: Response, enabled?: boolean): boolean {
301+
return enabled === true
302+
&& (response.status === 429 || response.status === 402)
303+
&& computeQuotaCooldown(codexQuotaOutcomeMeta(response)).source === "reset-derived";
304+
}
305+
279306
/**
280307
* One bounded alternate-account retry for Codex pool auth. Used for allow-listed
281308
* model-400 and for pre-stream 429/402 quota failures (#584).
@@ -306,23 +333,20 @@ async function retryCodexPoolOnAlternateAccount(
306333
return { kind: "no-alternate" };
307334
}
308335

309-
const retryAfterRaw = firstResponse.headers.get("retry-after");
336+
const quotaMeta = codexQuotaOutcomeMeta(firstResponse);
310337
if (outcomeStatus === 429 || outcomeStatus === 402) {
311338
const { applyAccountQuotaFromUpstreamHeaders } = await import("../../codex/auth-api");
312339
applyAccountQuotaFromUpstreamHeaders(firstAuthCtx.accountId, firstResponse.headers);
313340
}
314-
recordCodexUpstreamOutcome(config, firstAuthCtx.accountId, outcomeStatus, {
315-
retryAfter: retryAfterRaw,
316-
resetAt: [
317-
firstResponse.headers.get("x-codex-primary-reset-at"),
318-
firstResponse.headers.get("x-codex-secondary-reset-at"),
319-
firstResponse.headers.get("x-codex-tertiary-reset-at"),
320-
].filter(Boolean),
321-
threadId: req.headers.get("x-codex-parent-thread-id"),
322-
probeLeaseId: codexProbeLeaseId(firstAuthCtx),
323-
// Retry already advanced the RR ring via excludeAccountId — reuse for promotion.
324-
...(retryAuthCtx.accountId ? { promoteAccountId: retryAuthCtx.accountId } : {}),
325-
});
341+
if (!shouldDeferCodexResetDerivedCooldown(firstResponse, options.deferCodexResetDerivedCooldown)) {
342+
recordCodexUpstreamOutcome(config, firstAuthCtx.accountId, outcomeStatus, {
343+
...quotaMeta,
344+
threadId: req.headers.get("x-codex-parent-thread-id"),
345+
probeLeaseId: codexProbeLeaseId(firstAuthCtx),
346+
// Retry already advanced the RR ring via excludeAccountId — reuse for promotion.
347+
...(retryAuthCtx.accountId ? { promoteAccountId: retryAuthCtx.accountId } : {}),
348+
});
349+
}
326350

327351
const retryHeaders = headersForCodexAuthContext(req.headers, retryAuthCtx);
328352
const retryProvider = applyCodexAuthContextToProvider(
@@ -469,6 +493,8 @@ export interface HandleResponsesOptions {
469493
promptCacheKeyIsSharedCohort?: boolean;
470494
/** Internal recursion guard; callers outside this module must not set it. */
471495
comboAttempt?: boolean;
496+
/** Internal combo handoff: allow a later same-provider model after a reset-derived 429/402. */
497+
deferCodexResetDerivedCooldown?: boolean;
472498
/** 030-owned handoff when a child consumed the original failure under bounds. */
473499
onConsumedComboFailure?: (failure: ConsumedComboFailure) => void;
474500
}
@@ -902,9 +928,17 @@ export async function handleComboResponses(
902928
const callbackGate = createChildPassthroughCallbackGate(options);
903929
let response: Response;
904930
try {
931+
const currentTargetProvider = pick.target.provider;
932+
const deferCodexResetDerivedCooldown = combo.strategy === "failover"
933+
&& combo.targets.slice(pick.targetIndex + 1).some(target =>
934+
target.provider === currentTargetProvider
935+
&& payloadEligible(target)
936+
&& !isComboTargetInCooldown(comboId, target),
937+
);
905938
response = await handleResponses(childRequest, config, childLog, {
906939
...options,
907940
comboAttempt: true,
941+
deferCodexResetDerivedCooldown,
908942
// Attempt-relative TTFT is recorded HERE (not via childLog.firstOutputMs — a later
909943
// Object.assign(logCtx, childLog) would overwrite the request-relative value).
910944
onFirstOutput: () => {
@@ -1488,7 +1522,7 @@ export async function handleResponses(
14881522
if (usesCodexForwardPoolAuth(authCtx, route.provider)) {
14891523
// primary was the 5h window; it now carries weekly data for GPT plans.
14901524
// Prefer primary when present, fall back to secondary for compatibility.
1491-
const retryAfterRaw = upstreamResponse.headers.get("retry-after");
1525+
const quotaMeta = codexQuotaOutcomeMeta(upstreamResponse);
14921526
const { applyAccountQuotaFromUpstreamHeaders } = await import("../../codex/auth-api");
14931527
applyAccountQuotaFromUpstreamHeaders(authCtx.accountId, upstreamResponse.headers);
14941528
if (terminalBodyWillRecord) {
@@ -1512,16 +1546,14 @@ export async function handleResponses(
15121546
}
15131547
options.onNativePassthroughTerminal?.(status);
15141548
});
1515-
} else {
1549+
} else if (!shouldDeferCodexResetDerivedCooldown(
1550+
upstreamResponse,
1551+
options.deferCodexResetDerivedCooldown,
1552+
)) {
15161553
recordCodexUpstreamOutcome(config, authCtx.accountId, upstreamResponse.status, {
1517-
retryAfter: retryAfterRaw,
1518-
resetAt: [
1519-
upstreamResponse.headers.get("x-codex-primary-reset-at"),
1520-
upstreamResponse.headers.get("x-codex-secondary-reset-at"),
1521-
upstreamResponse.headers.get("x-codex-tertiary-reset-at"),
1522-
].filter(Boolean),
1523-
threadId: req.headers.get("x-codex-parent-thread-id"),
1524-
probeLeaseId: codexProbeLeaseId(authCtx),
1554+
...quotaMeta,
1555+
threadId: req.headers.get("x-codex-parent-thread-id"),
1556+
probeLeaseId: codexProbeLeaseId(authCtx),
15251557
});
15261558
}
15271559
}

src/update/job.ts

Lines changed: 100 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { existsSync, mkdirSync, readFileSync } from "node:fs";
33
import { dirname, join } from "node:path";
44
import { fileURLToPath } from "node:url";
55
import { atomicWriteFile, getConfigDir, loadConfig, readPid, readRuntimePort } from "../config";
6-
import { killProxy } from "../lib/process-control";
6+
import { isProcessAlive, killProxy } from "../lib/process-control";
77
import { reclaimListenPort } from "../server/port-reclaim";
88
import { isOpencodexHealthz, probeHostname, proxyIdentityAt, type HealthzIdentity } from "../server/proxy-liveness";
99
import { isServiceInstalled } from "../service";
@@ -28,6 +28,8 @@ const UPDATE_TIMEOUT_MS = 180_000;
2828
const RESTART_TIMEOUT_MS = 60_000;
2929
const RESTART_HEALTH_TIMEOUT_MS = 15_000;
3030
const RESTART_STABILITY_WINDOW_MS = 15_000;
31+
/** Legacy active records did not persist a worker PID, so age is their only safe recovery signal. */
32+
export const UPDATE_JOB_LEGACY_STALE_MS = 10 * 60_000;
3133
/** How long update restart waits for the captured port to become bindable after stop. */
3234
export const RESTART_PORT_RECLAIM_MS = 30_000;
3335

@@ -77,6 +79,19 @@ export interface UpdateCheckDeps {
7779
latestVersion: (tag: Channel) => string | null;
7880
}
7981

82+
interface UpdateWorkerProcess {
83+
pid?: number;
84+
unref(): void;
85+
once(event: "error", listener: (error: Error) => void): unknown;
86+
}
87+
88+
export interface StartUpdateJobDeps {
89+
checkForUpdateFn: (channel: Channel) => UpdateCheckResult;
90+
spawnWorkerFn: (jobId: string, channel: Channel, restart: boolean) => UpdateWorkerProcess;
91+
isProcessAliveFn: (pid: number) => boolean;
92+
nowMs: () => number;
93+
}
94+
8095
const defaultCheckDeps: UpdateCheckDeps = {
8196
currentVersion,
8297
detectInstall,
@@ -225,19 +240,77 @@ function newJobId(): string {
225240
return `${Date.now()}-${Math.random().toString(36).slice(2, 10)}`;
226241
}
227242

228-
export function startUpdateJob(channel: Channel, restart: boolean): UpdateJobState {
243+
/**
244+
* [Decision Log]
245+
* - Purpose: recover dashboard updates after a detached worker dies without unlocking concurrent live updates.
246+
* - Existing constraints: legacy records have no PID, while a healthy update may legitimately run for minutes.
247+
* - Alternatives considered: clear every active record by age, or require operators to delete the file manually.
248+
* - Chosen approach: trust PID liveness first and use a conservative age limit only for legacy no-PID records.
249+
* - Why: age-only recovery can start two installers, while never recovering leaves the dashboard permanently blocked.
250+
* - Impact: live PID records remain locked regardless of age; dead PIDs recover immediately; legacy records recover after ten minutes.
251+
*/
252+
export function staleActiveUpdateJobReason(
253+
job: Pick<UpdateJobState, "status" | "pid" | "updatedAt">,
254+
now = Date.now(),
255+
isAlive: (pid: number) => boolean = isProcessAlive,
256+
): string | null {
257+
if (job.status !== "running" && job.status !== "restarting") return null;
258+
if (typeof job.pid === "number" && Number.isSafeInteger(job.pid) && job.pid > 0) {
259+
return isAlive(job.pid) ? null : `update worker PID ${job.pid} is no longer running`;
260+
}
261+
const updatedAt = Date.parse(job.updatedAt);
262+
if (Number.isFinite(updatedAt) && now - updatedAt >= UPDATE_JOB_LEGACY_STALE_MS) {
263+
return "legacy active update record has no worker PID and exceeded the stale window";
264+
}
265+
return null;
266+
}
267+
268+
const defaultStartUpdateJobDeps: StartUpdateJobDeps = {
269+
checkForUpdateFn: channel => checkForUpdate(channel),
270+
spawnWorkerFn: (jobId, channel, restart) => spawn(
271+
process.execPath,
272+
[process.argv[1], "__gui-update-worker", jobId, channel, restart ? "restart" : "no-restart"],
273+
{
274+
detached: true,
275+
stdio: "ignore",
276+
windowsHide: true,
277+
env: { ...process.env, OCX_SERVICE: "1" },
278+
},
279+
),
280+
isProcessAliveFn: isProcessAlive,
281+
nowMs: Date.now,
282+
};
283+
284+
export function startUpdateJob(
285+
channel: Channel,
286+
restart: boolean,
287+
deps: Partial<StartUpdateJobDeps> = {},
288+
): UpdateJobState {
289+
const resolvedDeps = { ...defaultStartUpdateJobDeps, ...deps };
229290
const running = readUpdateJob();
230291
if (running?.status === "running" || running?.status === "restarting") {
231-
throw new UpdateJobError("An update job is already running", 409, "update_already_running");
292+
const staleReason = staleActiveUpdateJobReason(
293+
running,
294+
resolvedDeps.nowMs(),
295+
resolvedDeps.isProcessAliveFn,
296+
);
297+
if (!staleReason) {
298+
throw new UpdateJobError("An update job is already running", 409, "update_already_running");
299+
}
300+
updateJob(
301+
running,
302+
{ status: "failed", error: `Recovered stale update job: ${staleReason}.`, exitCode: null },
303+
`Recovered stale update job: ${staleReason}.`,
304+
);
232305
}
233306

234-
const check = checkForUpdate(channel);
307+
const check = resolvedDeps.checkForUpdateFn(channel);
235308
if (!check.canUpdate) {
236309
throw new UpdateJobError(check.reason ?? "No update is available", 409, check.reason ?? "update_unavailable");
237310
}
238311

239312
const id = newJobId();
240-
const now = new Date().toISOString();
313+
const now = new Date(resolvedDeps.nowMs()).toISOString();
241314
const job: UpdateJobState = {
242315
id,
243316
status: "running",
@@ -254,14 +327,30 @@ export function startUpdateJob(channel: Channel, restart: boolean): UpdateJobSta
254327
};
255328
writeJob(job);
256329

257-
const child = spawn(process.execPath, [process.argv[1], "__gui-update-worker", id, channel, restart ? "restart" : "no-restart"], {
258-
detached: true,
259-
stdio: "ignore",
260-
windowsHide: true,
261-
env: { ...process.env, OCX_SERVICE: "1" },
330+
let child: UpdateWorkerProcess;
331+
try {
332+
child = resolvedDeps.spawnWorkerFn(id, channel, restart);
333+
} catch (error) {
334+
const message = error instanceof Error ? error.message : String(error);
335+
updateJob(job, { status: "failed", error: `Could not start update worker: ${message}` }, "Update worker failed to start.");
336+
throw new UpdateJobError("Could not start update worker", 500, "update_worker_start_failed");
337+
}
338+
if (typeof child.pid !== "number" || !Number.isSafeInteger(child.pid) || child.pid <= 0) {
339+
updateJob(job, { status: "failed", error: "Could not start update worker: no worker PID was returned." }, "Update worker failed to start.");
340+
throw new UpdateJobError("Could not start update worker", 500, "update_worker_start_failed");
341+
}
342+
const startedJob = updateJob(job, { pid: child.pid }, `Update worker started as PID ${child.pid}.`);
343+
child.once("error", error => {
344+
const current = readUpdateJob(id);
345+
if (!current || current.pid !== child.pid || (current.status !== "running" && current.status !== "restarting")) return;
346+
updateJob(
347+
current,
348+
{ status: "failed", error: `Update worker failed to start: ${error.message}` },
349+
"Update worker emitted a startup error.",
350+
);
262351
});
263352
child.unref();
264-
return { ...job, pid: child.pid };
353+
return startedJob;
265354
}
266355

267356
function runLoggedCommand(job: UpdateJobState, bin: string, args: string[], timeout: number): { status: number | null; signal: NodeJS.Signals | null } {

structure/04_transports-and-sidecars.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,26 @@ retried. Guarded paths: the ChatGPT passthrough and generic adapter fetch in
387387
fallback. Adapters with their own `fetchResponse` (kiro, cursor, google) keep their own retry
388388
policies; kiro imports the shared abort/sleep helpers from this module.
389389

390+
## Same-provider combo quota fallback
391+
392+
For a failover combo with multiple models on the same Codex-login OpenAI provider, a pre-stream
393+
429/402 carrying only `x-codex-*-reset-at` may advance to the later model on the same account. The
394+
failed physical combo target still enters its normal target cooldown. An explicit `Retry-After`
395+
remains an account-wide instruction and blocks the later target; a quota response with neither an
396+
explicit retry delay nor a usable reset timestamp keeps the conservative default account cooldown.
397+
This exception is request-scoped and is not applied to direct requests, round-robin combos, or a
398+
combo whose remaining eligible targets use other providers.
399+
400+
```text
401+
[Decision Log]
402+
- 목적과 의도: Let an ordered combo recover when one model-specific Codex quota window is exhausted but another model on the same account remains usable.
403+
- 기존 구현 및 제약 조건: Account health is shared across models, and recording a reset-derived 429 before combo advancement rejected the later model locally.
404+
- 검토한 주요 대안: Make every quota cooldown model-scoped; ignore all combo 429 cooldowns; or defer only reset-derived cooldown recording for an eligible later same-provider failover target.
405+
- 선택한 방식: Use the narrow request-scoped deferral while retaining target cooldown and all explicit Retry-After/default account cooldown behavior.
406+
- 다른 대안 대신 이 방식을 선택한 이유: Reset timestamps identify quota windows rather than a literal account-wide retry instruction, but widening the exception would risk hot retries and provider abuse.
407+
- 장점, 단점 및 영향: Same-account model fallback works without weakening explicit upstream backoff; the account health map intentionally does not remember that one deferred reset-derived failure, while the combo target map does.
408+
```
409+
390410
## Sidecars
391411

392412
Web search and vision sidecars only run when the mode-aware `openai` forward ChatGPT authority

0 commit comments

Comments
 (0)