Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scripts/test-layout/layout.json
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,7 @@
"privacy-mask-account.test.ts": "lib",
"privacy-scan-meta-key.test.ts": "ci-workflows",
"probe-lease.test.ts": "routing",
"probe-lease-dispatch-wiring.test.ts": "routing",
"process-control-graceful.test.ts": "lib",
"process-control.test.ts": "lib",
"process-state.test.ts": "service",
Expand Down
137 changes: 130 additions & 7 deletions src/codex/auth-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ import {
pickAlternateCodexAccount,
resolveCodexAccountForThreadDetailed,
type CodexAffinityDecision,
type CodexThreadResolution,
type TransientProbeGrant,
} from "./routing";
// The half-open TRANSIENT-HOLD lease (#4701). Not the quota-cooldown probe lease imported from
// ./routing above -- different module, different domain, and a request never holds both.
import { releaseTransientProbe } from "../routing/probe-lease";
import {
codexConversationIdentity,
recordCodexThreadLineage,
Expand Down Expand Up @@ -202,6 +207,12 @@ export type CodexAuthContext =
affinityDecision?: CodexAffinityDecision;
/** Scope that owns `probeLeaseId`, when it is a scoped recovery probe. */
probeQuotaScope?: CodexQuotaScope;
/**
* Set when this request is the ONE dispatch admitted to test an account held under a
* transient 5xx hold (#4701). Echo it into the upstream outcome so the trial is settled
* by the request that ran it, and release it on any path that never reaches upstream.
*/
transientProbe?: TransientProbeGrant;
}
| {
// Main Codex account participating in rotation: token injected from ~/.codex/auth.json
Expand All @@ -222,6 +233,8 @@ export type CodexAuthContext =
probeLeaseId?: string;
quotaScope?: CodexQuotaScope;
probeQuotaScope?: CodexQuotaScope;
/** See `pool.transientProbe`. */
transientProbe?: TransientProbeGrant;
};

/** Probe lease carried by this context, when it holds one. */
Expand All @@ -234,11 +247,24 @@ export function codexProbeQuotaScope(ctx: CodexAuthContext | undefined): CodexQu
return ctx?.kind === "pool" || ctx?.kind === "main-pool" ? ctx.probeQuotaScope : undefined;
}

/** The transient-hold recovery probe carried by this context, when it holds one (#4701). */
export function codexTransientProbeGrant(ctx: CodexAuthContext | undefined): TransientProbeGrant | undefined {
return ctx?.kind === "pool" || ctx?.kind === "main-pool" ? ctx.transientProbe : undefined;
}

/**
* Hand back a probe lease for a request that will not reach upstream. Safe to
* call with a context that holds no lease.
*
* BOTH leases, deliberately. A context can carry the quota-cooldown probe or the transient-hold
* probe, and every one of the ~30 call sites that already hands back the first is a path where
* the second would leak too. Releasing them together is what makes those sites correct for the
* new lease without re-deriving the discard set by hand -- the failure mode being avoided is a
* held account nobody may probe because the request that held the trial went away quietly.
*/
export function releaseCodexAuthContextProbeLease(ctx: CodexAuthContext | undefined): void {
const transientProbe = codexTransientProbeGrant(ctx);
if (transientProbe) releaseTransientProbe(transientProbe.lease);
const leaseId = codexProbeLeaseId(ctx);
if (!ctx || ctx.kind === "main" || !leaseId) return;
if (ctx.probeQuotaScope) releaseCodexQuotaScopeProbeLease(ctx.accountId!, ctx.probeQuotaScope, leaseId);
Expand Down Expand Up @@ -424,6 +450,44 @@ export class CodexReserveHelperUnsupportedError extends CodexReserveUnavailableE
}
}

/**
* Every account bound to this conversation is held after upstream failures, the recovery
* budget for this window is spent, and there is no detour left -- so this request is refused
* BEFORE any upstream I/O (#4701).
*
* This is not a quota cooldown, and the message below says so. It subclasses
* {@link CodexAccountCooldownError} for one reason: the deadline-carrying refusal has exactly
* one representation in this codebase, and roughly a dozen transports already map it to a 429
* with `Retry-After` and treat it as an expected terminal answer rather than a credential
* fault. Introducing a parallel type would mean either re-deriving that handling in every one
* of them or silently falling through to a 500 in the ones that were missed.
*
* What must NOT be inherited is the quota wording -- "cooling down", `ocx account
* clear-cooldown` -- because none of it describes a 5xx hold and following it would do
* nothing. {@link cooldownErrorMessage} therefore returns this class's own message verbatim,
* the same escape hatch {@link CodexMainAccountHardLockError} and
* {@link CodexReserveUnavailableError} already use.
*
* `cooldownUntil` carries the limiter's own change point, which is strictly in the future:
* either the moment the held account may next be probed or the moment the recovery window
* moves, whichever is later. A refusal that answered `now` would busy-loop the caller into
* the same load it just declined.
*/
export class CodexRecoveryWithheldError extends CodexAccountCooldownError {
/** The sibling still remembered for this thread, when one exists but is itself unusable. */
readonly detourAccountId?: string;

constructor(accountId: string, retryAt: number, detourAccountId?: string) {
super(accountId, retryAt);
this.name = "CodexRecoveryWithheldError";
this.detourAccountId = detourAccountId;
this.message = `Codex account (${cooldownAccountLabel(accountId)}) is held after repeated upstream`
+ ` failures and the pool's recovery budget for this window is spent, so nothing was sent`
+ ` upstream. Retry after ${new Date(retryAt).toISOString()}.`
+ " This clears on its own as the account recovers; no cooldown to lift and no account to switch.";
}
}

export type CodexAuthPolicyConfig = Readonly<Pick<OcxConfig,
"codexMainAccountHardLock" | "codexDesktopAuthless" | "runtimeRole" | "pausedCodexAccountIds"
>>;
Expand Down Expand Up @@ -634,7 +698,12 @@ export function cooldownAccountLabel(accountId: string): string {
* injected `openai_base_url` in config.toml.
*/
export function cooldownErrorMessage(err: CodexAccountCooldownError, accountSelector?: string): string {
if (err instanceof CodexMainAccountHardLockError || err instanceof CodexReserveUnavailableError) return err.message;
if (err instanceof CodexMainAccountHardLockError
|| err instanceof CodexReserveUnavailableError
// A transient-hold refusal is not a quota cooldown. Its own wording is the only accurate
// one, and the quota recovery advice below would send the operator after a cooldown that
// does not exist (#4701).
|| err instanceof CodexRecoveryWithheldError) return err.message;
const until = new Date(err.cooldownUntil).toISOString();
const scopeLabels: Record<CodexQuotaScope, string> = {
shared: "shared native quota", reserve: "Reserve quota",
Expand Down Expand Up @@ -870,6 +939,15 @@ export async function resolveCodexAuthContext(
// Why this request is on this account, carried to the request log so a move reads as an event
// instead of something inferred from account labels across lines (#4546).
let affinityDecision: CodexAffinityDecision | undefined;
// The half-open trial this request was granted, if it is the one allowed to test a held
// account. Declared out here because the release paths below and the returned context are on
// opposite sides of several throws (#4701).
let transientProbe: TransientProbeGrant | undefined;
const releaseTransientProbeGrant = (): void => {
if (!transientProbe) return;
releaseTransientProbe(transientProbe.lease);
transientProbe = undefined;
};
// Retained startup recovery makes the physical main identity ineligible. Routing
// can still preserve service by selecting a healthy configured pool account. A
// request-owned bearer likewise cannot inspect or reconcile file-main state.
Expand Down Expand Up @@ -917,7 +995,11 @@ export async function resolveCodexAuthContext(
// and may still route to non-main pool accounts without touching switch state.
if (reserve && !nativeMainReadsForbidden && !selectionAdmission) throw new CodexMainProfileDrainingError();
if (!nativeMainReadsForbidden) reconcileMainCodexAccountRuntimeState();
const resolution = fixedAccountId !== undefined
// Annotated, not inferred. The two literals below carry neither `affinity` nor
// `transientProbe`, so an inferred union makes `"k" in resolution` widen those reads to
// `unknown` and a discriminant narrowing fail outright. Contextually typing every branch to
// the resolver's own union is what lets the reads below stay total.
const resolution: CodexThreadResolution = fixedAccountId !== undefined
? { status: "selected" as const, accountId: fixedAccountId }
: options.excludeAccountId
? (() => {
Expand All @@ -942,8 +1024,17 @@ export async function resolveCodexAuthContext(
lineage,
);
if (resolution.status === "expired") throw new CodexThreadAffinityExpiredError(resolution.accountId);
// THE REFUSAL. Every candidate is held, the recovery budget is spent, and no detour is
// left -- so this request must not reach upstream at all. Returning the held account here
// is what #4701 is about: under a provider-wide 503 that is every bound request piling
// onto an account already known to be failing. Thrown before any credential is read, so
// nothing is sent and nothing is spent.
if (resolution.status === "withheld") {
throw new CodexRecoveryWithheldError(resolution.accountId, resolution.retryAt, resolution.detourAccountId);
}
const selected = resolution.status === "selected" ? resolution.accountId : null;
affinityDecision = "affinity" in resolution ? resolution.affinity : undefined;
affinityDecision = resolution.affinity;
transientProbe = resolution.status === "selected" ? resolution.transientProbe : undefined;
if (!selected) {
// A retry that excluded a failed Pool account may still use the validated caller-owned
// main credential. Treating every exclusion as if main itself had failed strands a healthy
Expand Down Expand Up @@ -1025,12 +1116,25 @@ export async function resolveCodexAuthContext(
throw new CodexPoolAuthenticationError("Selected Codex account is unavailable");
}
}
} catch (cause) {
// Selection granted a trial and then a later policy check refused the account. The trial
// never runs, so hand it back instead of leaving the held account unprobeable until its
// deadline lapses (#4701).
releaseTransientProbeGrant();
throw cause;
} finally {
selectionAdmission?.release();
}
// Legacy selectors may retain an unusable account for actionable errors. A
// deferred credential must never become request auth through that fallback.
assertCodexAccountValidationReady(accountId);
try {
assertCodexAccountValidationReady(accountId);
} catch (cause) {
// Nothing will reach upstream, so give the trial back instead of leaving the held account
// unprobeable until the lease deadline lapses (#4701).
releaseTransientProbeGrant();
throw cause;
}
// Lazy prime: if the selected account has no quota yet, the pool is likely
// unprimed (dashboard never opened, or startup prime was blocked). Kick a
// best-effort prime so the NEXT routing decision has real scores. This never
Expand All @@ -1049,6 +1153,13 @@ export async function resolveCodexAuthContext(
// a literal Retry-After reads very differently to a user than a reset-derived guess.
const cooldown = getCodexQuotaHealthSnapshot(accountId, quotaScope);
const cooldownUntil = cooldown?.cooldownUntil;
// A transient-hold trial and a quota cooldown cannot both describe this account:
// `isTransientOnlyAffinityBlock` refuses to recognise a transient hold on an account carrying
// quota health, so the cooldown branch below is unreachable while a trial is held. That is
// also why no request pays two recovery permits for one send. The release is defensive --
// should that invariant ever move, the trial is handed back rather than stranded behind a
// refusal that belongs to the other domain.
if (cooldownUntil && transientProbe) releaseTransientProbeGrant();
// A cooled-down account never sends traffic, so upstream recovery can never be
// observed and the cooldown outlives the real limit. Admit one probe per
// interval; its outcome decides whether the cooldown ends (#433).
Expand Down Expand Up @@ -1089,6 +1200,7 @@ export async function resolveCodexAuthContext(
if (token) mainQuotaWriter = observeSelectedMainCredential(token, mainQuotaWriter);
assertMainAccountPolicy(policy);
} catch (cause) {
releaseTransientProbeGrant();
if (probeLeaseId && probeQuotaScope) releaseCodexQuotaScopeProbeLease(accountId, probeQuotaScope, probeLeaseId);
else if (probeLeaseId) releaseCodexQuotaProbeLease(accountId, probeLeaseId);
if (cause instanceof CodexMainAccountHardLockError) throw cause;
Expand All @@ -1099,15 +1211,23 @@ export async function resolveCodexAuthContext(
}
if (!token) {
// Nothing will reach upstream, so give the probe back instead of burning it.
releaseTransientProbeGrant();
if (probeLeaseId && probeQuotaScope) releaseCodexQuotaScopeProbeLease(accountId, probeQuotaScope, probeLeaseId);
else if (probeLeaseId) releaseCodexQuotaProbeLease(accountId, probeLeaseId);
throw new CodexPoolAuthenticationError(
fixedAccountId !== undefined ? "Selected Codex account is unavailable" : undefined,
);
}
const reserveAuthorization = reserve
? await authorizeReserveCredential(token, mainQuotaWriter, policy, options.signal, undefined, writerGeneration)
: undefined;
let reserveAuthorization: MainReserveAuthorization | undefined;
try {
reserveAuthorization = reserve
? await authorizeReserveCredential(token, mainQuotaWriter, policy, options.signal, undefined, writerGeneration)
: undefined;
} catch (cause) {
// A Reserve refusal ends the request here, so the trial it was holding never runs.
releaseTransientProbeGrant();
throw cause;
}
return {
kind: "main-pool",
accountId,
Expand All @@ -1121,6 +1241,7 @@ export async function resolveCodexAuthContext(
...(quotaScope ? { quotaScope } : {}),
...(probeLeaseId ? { probeLeaseId } : {}),
...(probeQuotaScope ? { probeQuotaScope } : {}),
...(transientProbe ? { transientProbe } : {}),
};
}

Expand All @@ -1141,8 +1262,10 @@ export async function resolveCodexAuthContext(
...(probeLeaseId ? { probeLeaseId } : {}),
...(probeQuotaScope ? { probeQuotaScope } : {}),
...(affinityDecision ? { affinityDecision } : {}),
...(transientProbe ? { transientProbe } : {}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Propagate probe grants through sidecar outcomes

Ensure every consumer of this returned context echoes transientProbe into its outcome metadata. The shared recorder in src/providers/openai-sidecar.ts:228-239, used by search, images, live, and audio sidecars, forwards the quota probe fields but omits the new transient grant; when one of those requests is admitted as the half-open trial, its terminal response never settles the lease, so subsequent requests remain withheld until the 30-second lease expiry instead of observing the result and following the 15-second probe pacing.

Useful? React with 👍 / 👎.

};
} catch (cause) {
releaseTransientProbeGrant();
if (probeLeaseId && probeQuotaScope) releaseCodexQuotaScopeProbeLease(accountId, probeQuotaScope, probeLeaseId);
else if (probeLeaseId) releaseCodexQuotaProbeLease(accountId, probeLeaseId);
if (!options.signal?.aborted && shouldMarkAccountNeedsReauthForCodexAuthFailure(cause)) {
Expand Down
Loading
Loading