diff --git a/devlog/_plan/260920_meaning_preservation_batch/020_lane_c.md b/devlog/_plan/260920_meaning_preservation_batch/020_lane_c.md new file mode 100644 index 00000000000..a6c9b2edaa5 --- /dev/null +++ b/devlog/_plan/260920_meaning_preservation_batch/020_lane_c.md @@ -0,0 +1,121 @@ +# Lane C — retry stage table (7) and one event model (14) + +Status: OPEN. Branch `codex/260920-lane-c-retry-event-model`, cut from `dev` at +`b9483b3b510f9a8d99d465282517f7bf91678de3`. One branch, ordered commits, one pull request to +`dev`, per [010_phase2.md](010_phase2.md). + +## What this lane fixes first + +Bundles 7 and 14 want the same substrate, so the branch defines it before touching anything that +consumes it. `src/lib/request-failure-model.ts` is now the single statement of three things: + +- **Stage** — how far a failed exchange got, ordered by what the DOWNSTREAM CLIENT observed: + `pre-header`, `headers-only`, `protocol-prelude`, `semantic-output`, `side-effect`, `terminal`. + Ordering by client observation rather than by upstream progress is deliberate: the question the + table answers is whether a resend can duplicate something the caller already saw. +- **Cause** — one closed dictionary, with `rate-limit`, `quota-exhausted`, `policy-refusal` and + `ciphertext-refusal` as four separate members because their remedies are four different actions. + `parameter-rejected` is separate from `policy-refusal` for the same reason: the same content + succeeds once the parameter changes, and `payload-too-large` is separate from `payload-rejected` + because a smaller rebuild succeeds where no repair helps the other. +- **Resend permission** — derived from three small per-member facts, not written out as a + stage-by-cause matrix. A 6×14 matrix is a restatement that has to be re-derived by hand whenever + a member is added, and the cell nobody revisited is how two individually correct branches merge + into a wrong table. That is the class that blocked 2.60.0. + +A stage is how far the observable progression got, not which events happened to arrive. A turn that +settled carrying no output — an empty completion, a 4xx error body — did not reach `terminal`; it +stalled at `protocol-prelude`, because the caller saw no answer. `terminal` means the answer was +delivered, which is why it is both last and refused. Commitment is a named per-stage fact rather +than a rank comparison, so a stage added later cannot default into permission. + +`refused-ambiguous` forbids an AUTOMATIC resend. It does not forbid a narrowly scoped, explicitly +opted-in recovery that a maintainer reasoned about and bounded. That distinction is what separates +a sanctioned single-shot rebuild from a retry loop that fires because a counter had room, and it is +why this table can be honest about the recoveries the proxy already performs. + +Funding follows the disposition rather than the permission, for the same reason. The opt-in reset +replay, the bounded empty-completion rebuild and the transient 5xx ladder are all refused +automatically and all really send, so all three still name the allowance they draw on. Keying +funding on permission would leave exactly those paths unfunded, which is how a per-layer counter +comes back. + +Two classifications were corrected during review after being checked against what the code actually +does rather than against what the recovery kind is called. `transient-5xx` covers a status set that +mixes a 503 the origin declined with a 500 it may already have run, so it classifies as +`upstream-fault` and the table never claims the resend was provably safe. `console-go-upload-retry` +replays a byte-identical body that the gateway accepts seconds later, so nothing about the payload +was wrong and it classifies as `upstream-declined`. + +## No second store + +The durable shapes stay `PersistedUsageAttempt`, `PersistedRequestSpend` and +`PersistedUsageEntry` in `src/usage/log.ts`, joined by `addFinalRequestLog()`. That join is already +the one place a logical request id, its attempts, their physical `sendCount` and the terminal +outcome meet, so this lane derives from it rather than growing a parallel history. The new module +declares no record type and holds no state; both of its imports are types and are erased at +runtime, so it stays a leaf. + +## Restatements removed + +Two live instances of the union-defect class, both found while fixing the substrate: + +- `AttemptRecoveryKind` was written twice — as a union and as the read-back whitelist + `normalizedAttempt` filters against. A member added only to the union compiles, is written to + disk, and is dropped on the next read, so the row loses the field that says why it recovered. + Both vocabularies are now frozen rosters with the types derived from them. +- `recoveryClass()` in `src/server/request-metrics.ts` ended in `default: return "other"`, so a + recovery kind added later compiled cleanly and vanished into an unactionable bucket. It is now + total over the shared cause dictionary; a missing member is a typecheck failure. + +## Ownership + +This lane owns `sendCount`, the request-wide send budget, and the stage and cause vocabulary. +Lanes D and E consume them and do not redefine them. #4793 and every per-model cache view belong to +lane D; this branch edits neither and derives nothing from them. + +## Dispositions + +### Carried + +| Item | Disposition | +| --- | --- | +| #5245 (cmdy) | **Carried, narrowed.** Only an embedded `invalid_request_error` / `invalid_encrypted_content` is admitted through the gateway wrapper. The original reruns the whole opaque classifier on the embedded payload, which would also admit the code-less unverifiable-ciphertext wording, the #4469 caller mismatch and the two xAI decoder strings — identities accepted on evidence about how one specific upstream words its own rejection, which a gateway in between is not. A gateway envelope is now decided ONLY by its embedded payload: the pre-existing anchored-wording checks run on the whole message, and a gateway quotes the upstream's message inside its own, so a relayed caller mismatch would otherwise have satisfied the #4469 identity and gained a resend the strict check exists to withhold. Attribution is in the branch commit. | +| #4191 | **Addressed in part.** The WebSocket failure classifier now has a tested projection onto the shared stage and cause, so its four outcomes are stated in the same words as every other surface and the shared table independently reaches the transport's own no-replay-after-send verdict. The projection is not yet threaded into the durable record, and the SSE fallback the issue also asks for is a transport change; neither is in this branch. | +| #5180 | **Addressed in part.** `rate-limit` and `quota-exhausted` are separate causes with different resend decisions and different metric label values. The shared cooldown and `Retry-After` handling the issue also asks for are routing behaviour and are not in this branch. | + +### Deferred, with reasons + +| Item | Disposition | +| --- | --- | +| #4942 (FredAmartey) | **Deferred to a follow-up on this substrate.** The pre-header ambiguous-reset stage and its default refusal are now expressed in the shared table, which is what the PR's `replaySafe`/`replayResets` pair was duplicating. The PR itself is a 28-file transport change touching provider config, key failover and passthrough dispatch, and `dev` has moved under it around `request-execution-budget.ts` and `physical-send.ts`. Landing that reworked and unrun in a branch whose verification is static review would be a worse trade than deferring it. | +| #4989 | **Deferred to the same follow-up.** Its protocol-prelude state gate (`responseCreated && !outputCommitted && !terminal`) is exactly the `protocol-prelude` row of the shared table and is the correct model. It overlaps #4942 in `src/lib/upstream-retry.ts` and `passthrough-dispatch.ts`, and the two must not each buy an independent replacement send for one logical request, so they belong in one reworked change rather than two. | +| #2366 (chilung-cgu) | **Deferred.** Its `StreamTimeline`, `FailureSide` and seven-stage `FailureStage` are good source material and store nothing in parallel, but they are a second stage vocabulary. Reconciling them with the one landed here is a rewrite of the PR, not a carry, and it is better done once the substrate is on `dev`. | +| #3748 (yansigit) | **Deferred as implemented.** It adds an authoritative SQLite failure ledger beside the usage ledger, which is the parallel store this lane exists to avoid. The derived equivalent is to group recorder terminals by a versioned fingerprint of closed cause plus provider and model class. Its API also accepts a free-text `signature`, and regex redaction cannot prove content was removed. | +| #3983 (yansigit) | **Deferred.** Content-free and durable-store-free, but it emits through a second path independent of request recording. The derived form routes the same structural observations through the recorder and formats the debug ring from them. | +| #5063 (Vocllum) | **Deferred.** Sound retention work on the canonical ledger, and orthogonal to the stage and event model. It also changes GUI surface, which this branch cannot evidence. | +| GUI recovery-kind roster | **Deferred, and it is a real defect.** `gui/src/pages/Logs.tsx` declares its own `AttemptRecoveryKind` with nine of the durable thirteen members, so `key-401`, `oauth-account-429`, `opaque-blob-rejection` and `reasoning-effort-downgrade` have no localized label. Fixing it needs new strings across ten locale catalogs and a screenshot of the changed dialog, which a branch that may not build or run the GUI cannot produce. It should be one follow-up that derives the GUI union from the durable roster instead of restating it. | + +## Verification + +Static source review plus exact-head hosted CI, per the batch execution constraints. + +Checked statically on this branch: + +- every assertion in the two new test files was re-derived by hand from the declared tables, and + the only recovery kinds whose Prometheus class changes are `opaque-blob-rejection` + (`payload` to `ciphertext`) and `console-go-upload-retry` (`payload` to `transient`), both + corrections rather than side effects; +- every `satisfies Record` added here is total over its roster, and every value it + produces is a declared member of the target vocabulary; +- `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json` agree + key-for-key, and both new test files sit in the domain they are registered to; +- no file this branch touches has a `tests/fixtures/file-size-baseline.json` cap, and the new test + cases went into a sibling file rather than into `responses-opaque-blob-recovery.test.ts`, which + sits 148 lines under the new-file threshold; +- `src/server/index.ts` is untouched; it has one line of headroom against its cap. + +NOT RUN on this branch, by instruction: `bun run test`, any individual `bun test` file, +`bun run typecheck`, `bun run build:gui`, `bun install`, `bun run structure:check`, +`bun run privacy:scan`, and any live `ocx` execution. None of these may be recorded as passing. +Hosted CI at the exact head is the only execution evidence for this branch. diff --git a/docs-site/src/content/docs/reference/management-api.md b/docs-site/src/content/docs/reference/management-api.md index 4f47ad8bbc1..6dbf08fd9d8 100644 --- a/docs-site/src/content/docs/reference/management-api.md +++ b/docs-site/src/content/docs/reference/management-api.md @@ -290,6 +290,13 @@ boundary. Histogram buckets are cumulative and end with `le="+Inf"`, equal to th | `opencodex_ttft_missing_total` | `protocol`, `result` | Complementary count for requests without observed TTFT. | | `opencodex_metrics_process_start_time_seconds` | none | Process-local reset boundary. | +The `recovery` label takes one of a fixed set of classes: `transient`, `connection`, `credential`, +`rate_limit`, `quota`, `policy`, `ciphertext`, `payload`, `empty_completion`, `effort_downgrade` and +`other`. The set is closed, so no model, account, user or request identifier can ever appear in a +series. `rate_limit`, `quota`, `policy` and `ciphertext` are separate because the operator response +differs: wait out the limit, move to another account, change the prompt, or drop stale encrypted +state. A rejected opaque reasoning blob counts as `ciphertext` rather than `payload`. + If a scanned row exceeds the existing parser size limit, `GET /api/usage` and `GET /api/keys` keep the readable-row aggregates and add `usageIncomplete: true` with `usageIncompleteReason: "oversized_rows"` at response level. This diagnostic survives cached diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index ef1c68922bb..77550eff193 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1062,6 +1062,7 @@ "openai-provider-option-tooling.test.ts": "adapters/openai", "openai-provider-option.test.ts": "adapters/openai", "openai-responses-passthrough.test.ts": "responses", + "opaque-blob-wrapped-rejection.test.ts": "responses", "opencode-cli.test.ts": "providers", "opencode-management-transport.test.ts": "providers", "opencode-free-provider.test.ts": "providers", @@ -1167,6 +1168,7 @@ "reasoning-replay-robustness.test.ts": "adapters", "reasoning-replay-scope-source.test.ts": "lib", "redact.test.ts": "lib", + "failure-stage-model.test.ts": "lib", "relay-eager.test.ts": "server", "release-helper.test.ts": "ci-workflows", "release-notes.test.ts": "ci-workflows", diff --git a/src/lib/request-failure-model.ts b/src/lib/request-failure-model.ts new file mode 100644 index 00000000000..6066cdf616d --- /dev/null +++ b/src/lib/request-failure-model.ts @@ -0,0 +1,308 @@ +/** + * One vocabulary for how far a failed request got, why it failed, and whether this proxy may + * send it again (roadmap items 7 and 14). + * + * These two items are one module on purpose. Item 7 wants a resend decision per failure stage; + * item 14 wants one cause dictionary spanning logical request, attempt, physical send and + * terminal. Defined apart they typecheck on each branch and contradict each other in the merge, + * which is the class that blocked 2.60.0. + * + * What lives here is the vocabulary and the decision derived from it. What does NOT live here is + * a second record store: the durable shapes stay `PersistedUsageAttempt` and + * `PersistedUsageEntry` in src/usage/log.ts, and every projection below reads those structurally + * rather than growing a parallel history. + * + * MUST stay a leaf. Its only imports are types, erased at runtime, so nothing here can pull the + * usage or budget subsystems into a request path that did not already have them. + */ +import type { SendClass } from "./request-execution-budget"; +import type { AttemptRecoveryKind } from "../usage/log"; + +/** + * How far the exchange got, ordered by how much the DOWNSTREAM CLIENT observed. + * + * The order is by client observation rather than by upstream progress, because the question the + * table answers is whether resending can duplicate something the caller already saw. An upstream + * that completed a turn we never relayed has committed nothing downstream; an upstream that + * emitted one token has. + * + * A stage is how far the OBSERVABLE progression got, not which events happened to arrive. A turn + * that settled carrying no output -- an empty completion, a 4xx error body -- did not reach + * `terminal`; it stalled at `protocol-prelude`, because the caller saw no answer. `terminal` + * means the answer was delivered, which is why it is both last and refused. + */ +export const REQUEST_FAILURE_STAGES = Object.freeze([ + /** No response head exists. Whether the origin began the turn is not known from the stage alone. */ + "pre-header", + /** A status line and headers exist, and no protocol body event has been parsed yet. */ + "headers-only", + /** The protocol body began with control events only -- `response.created`, quota frames. */ + "protocol-prelude", + /** At least one output-bearing event reached the caller. */ + "semantic-output", + /** A tool call or other externally visible effect was emitted. */ + "side-effect", + /** A terminal event settled the turn after its answer reached the caller. */ + "terminal", +] as const); + +export type RequestFailureStage = typeof REQUEST_FAILURE_STAGES[number]; + +/** Position in {@link REQUEST_FAILURE_STAGES}. Derived, so the order is stated exactly once. */ +export function stageRank(stage: RequestFailureStage): number { + return REQUEST_FAILURE_STAGES.indexOf(stage); +} + +/** + * What the caller has irreversibly observed at a stage. + * + * Named separately from the rank so a reader can see WHY a stage refuses rather than inferring it + * from a position, and so the three committed stages stay distinguishable in a record. + */ +export type StageCommitment = "nothing-observed" | "output-observed" | "effect-observed" | "answer-delivered"; + +const STAGE_COMMITMENT = { + "pre-header": "nothing-observed", + "headers-only": "nothing-observed", + "protocol-prelude": "nothing-observed", + "semantic-output": "output-observed", + "side-effect": "effect-observed", + "terminal": "answer-delivered", +} as const satisfies Record; + +export function stageCommitment(stage: RequestFailureStage): StageCommitment { + return STAGE_COMMITMENT[stage]; +} + +/** + * Why the request failed, as one closed dictionary for every layer. + * + * Bounded on purpose: these are wire values a maintainer reads and a metric labels by, never a + * credential, an account identifier, an upstream body or prompt content. The four that #5180 and + * the ciphertext path insist on -- `rate-limit`, `quota-exhausted`, `policy-refusal` and + * `ciphertext-refusal` -- are separate members because they need opposite follow-ups: wait, + * change account, change the prompt, strip the ciphertext. + */ +export const REQUEST_FAILURE_CAUSES = Object.freeze([ + /** The bytes provably never reached the origin: connect refused, DNS failure, TLS handshake. */ + "transport-unsent", + /** The bytes left and the connection died before a head. The origin may be running the turn. */ + "transport-ambiguous", + /** The origin answered that it would not start the turn now: 503, overloaded, backpressure. */ + "upstream-declined", + /** A 429 rate limit. Capacity is momentarily gone; waiting is the remedy. */ + "rate-limit", + /** Plan or credit quota is gone. Waiting out a retry window does not help; the account must change. */ + "quota-exhausted", + /** Credentials were rejected: 401, 403 on identity. */ + "credential-rejected", + /** The origin evaluated the content and refused it. Identical bytes get the identical refusal. */ + "policy-refusal", + /** + * The origin rejected a request PARAMETER rather than the content: an unsupported reasoning + * effort, an unknown field. Distinct from `policy-refusal` because the remedy is opposite -- + * the same content succeeds once the parameter is adjusted. + */ + "parameter-rejected", + /** Opaque replay state was rejected as unverifiable. Only a request without it can succeed. */ + "ciphertext-refusal", + /** + * The payload exceeded a size the origin accepts. A smaller rebuild of the same turn can + * succeed, which is why this is not the same answer as `payload-rejected`. + */ + "payload-too-large", + /** The payload was rejected on its merits: unsupported media, malformed part. No repair helps. */ + "payload-rejected", + /** + * The origin returned a server-side fault. Whether it had already begun the turn is not + * knowable from the status, so this is the honest classification for the mixed 5xx set the + * transient layer retries: 503 really did decline, 500 may not have. + */ + "upstream-fault", + /** The turn settled carrying no usable output. */ + "empty-output", + /** The caller went away. */ + "client-cancelled", + /** This proxy refused before dispatch: send budget, route policy, replay refusal. */ + "local-refusal", +] as const); + +export type RequestFailureCause = typeof REQUEST_FAILURE_CAUSES[number]; + +/** + * What the cause proves about whether the origin ran the turn. + * + * This is the safety axis. `unknown` is the RFC 9110 9.2.2 case and is never upgraded by having + * budget left: a request whose upstream execution state is unknown is not replayable merely + * because a counter allows another send. + */ +export type UpstreamProcessingEvidence = "not-processed" | "declined" | "processed" | "unknown"; + +const CAUSE_EVIDENCE = { + "transport-unsent": "not-processed", + "transport-ambiguous": "unknown", + "upstream-declined": "declined", + "rate-limit": "declined", + "quota-exhausted": "declined", + "credential-rejected": "declined", + "policy-refusal": "processed", + "parameter-rejected": "declined", + "ciphertext-refusal": "declined", + "payload-too-large": "declined", + "payload-rejected": "declined", + "upstream-fault": "unknown", + "empty-output": "processed", + "client-cancelled": "unknown", + "local-refusal": "not-processed", +} as const satisfies Record; + +export function causeEvidence(cause: RequestFailureCause): UpstreamProcessingEvidence { + return CAUSE_EVIDENCE[cause]; +} + +/** + * What a resend would have to change to have any chance. + * + * The usefulness axis, orthogonal to safety. A policy refusal is perfectly safe to repeat and + * completely pointless; an ambiguous reset is the reverse. + */ +export type ResendDisposition = "resend-may-help" | "resend-after-repair" | "resend-is-futile"; + +const CAUSE_DISPOSITION = { + "transport-unsent": "resend-may-help", + "transport-ambiguous": "resend-may-help", + "upstream-declined": "resend-may-help", + "rate-limit": "resend-may-help", + "quota-exhausted": "resend-is-futile", + "credential-rejected": "resend-after-repair", + "policy-refusal": "resend-is-futile", + "parameter-rejected": "resend-after-repair", + "ciphertext-refusal": "resend-after-repair", + "payload-too-large": "resend-after-repair", + "payload-rejected": "resend-is-futile", + "upstream-fault": "resend-may-help", + "empty-output": "resend-may-help", + "client-cancelled": "resend-is-futile", + "local-refusal": "resend-is-futile", +} as const satisfies Record; + +export function causeDisposition(cause: RequestFailureCause): ResendDisposition { + return CAUSE_DISPOSITION[cause]; +} + +/** + * The answer this table exists to give. + * + * Every refusal names WHY it refused, because the three reasons need different operator + * responses and used to arrive as one undifferentiated "no retry". + */ +export type ResendPermission = + /** The same request may be sent again. */ + | "permitted" + /** Only a modified request may be sent: rotated credential, stripped ciphertext. */ + | "permitted-after-repair" + /** Upstream execution state is unknown. No AUTOMATIC resend; see the note below. */ + | "refused-ambiguous" + /** The caller already observed output or an externally visible effect. */ + | "refused-committed" + /** Identical bytes would get the identical answer. */ + | "refused-futile"; + +/** + * Whether this proxy may send the request again, from the stage it failed at and the cause. + * + * Derived from the two per-cause facts above and the per-stage commitment, rather than written + * out as a stage-by-cause matrix. A matrix of that size is a restatement: it would have to be + * re-derived by hand every time a member is added, and the cell nobody revisited is exactly how + * two correct branches merge into a wrong table. + * + * `refused-ambiguous` forbids an AUTOMATIC resend. It does not forbid a narrowly scoped, + * explicitly opted-in recovery that a maintainer reasoned about and bounded -- the reset replay + * behind a default-off provider flag, the single-shot empty-completion rebuild. Those are + * separate recorded decisions with their own acceptance, which is precisely what distinguishes + * them from a retry loop that fires because a counter had room. + */ +export function resendPermission( + stage: RequestFailureStage, + cause: RequestFailureCause, +): ResendPermission { + // Any stage at which the caller observed something refuses, whatever the cause says. Testing + // the commitment rather than listing the committed stages is what keeps a stage added later + // from defaulting into permission. + if (STAGE_COMMITMENT[stage] !== "nothing-observed") return "refused-committed"; + if (CAUSE_DISPOSITION[cause] === "resend-is-futile") return "refused-futile"; + const evidence = CAUSE_EVIDENCE[cause]; + if (evidence === "unknown" || evidence === "processed") return "refused-ambiguous"; + return CAUSE_DISPOSITION[cause] === "resend-after-repair" ? "permitted-after-repair" : "permitted"; +} + +/** True for the two permissions that allow a further send. */ +export function permitsResend(permission: ResendPermission): boolean { + return permission === "permitted" || permission === "permitted-after-repair"; +} + +/** + * Which request-wide send budget class a resend for this cause draws on, or null when no resend + * of any kind makes sense. + * + * Funding is keyed on the DISPOSITION, not on the permission. A cause the table refuses to resend + * automatically may still be resent by a narrowly scoped recovery a maintainer opted into, and + * that send has to be bought from the same budget every other send comes from -- the opt-in reset + * replay and the bounded empty-completion rebuild both draw on the transient allowance. Keying on + * permission instead would leave exactly those paths unfunded, which is how a per-layer counter + * reappears. + * + * Only a futile cause is null. `quota-exhausted` is null rather than `account-failover` because + * moving accounts is a route decision this table does not make. + */ +const CAUSE_SEND_CLASS = { + "transport-unsent": "transient", + "transport-ambiguous": "transient", + "upstream-declined": "transient", + "rate-limit": "transient", + "quota-exhausted": null, + "credential-rejected": "auth-recovery", + "policy-refusal": null, + "parameter-rejected": "repair", + "ciphertext-refusal": "repair", + "payload-too-large": "repair", + "payload-rejected": null, + "upstream-fault": "transient", + "empty-output": "transient", + "client-cancelled": null, + "local-refusal": null, +} as const satisfies Record; + +export function resendSendClass(cause: RequestFailureCause): SendClass | null { + return CAUSE_SEND_CLASS[cause]; +} + +/** + * The cause behind each recovery this proxy already records. + * + * Total over `AttemptRecoveryKind` by construction, so a new recovery kind is a typecheck + * failure here rather than a row that quietly classifies as "other" in three projections. + */ +const RECOVERY_KIND_CAUSE = { + // The retried status set mixes 503, which declined, with 500, which may already have run the + // turn. One kind cannot say both, so it says the weaker thing. + "transient-5xx": "upstream-fault", + "connection-reset": "transport-ambiguous", + "oauth-401": "credential-rejected", + "key-401": "credential-rejected", + "key-429": "rate-limit", + "rate-limit-429": "rate-limit", + "anthropic-oauth-429": "rate-limit", + "oauth-account-429": "rate-limit", + "image-413": "payload-too-large", + // The gateway rejects a body it accepts seconds later and the replay is byte-identical, so + // nothing about the payload was wrong; the origin declined to take it at that moment. + "console-go-upload-retry": "upstream-declined", + "opaque-blob-rejection": "ciphertext-refusal", + "empty-completion": "empty-output", + "reasoning-effort-downgrade": "parameter-rejected", +} as const satisfies Record; + +export function causeForRecoveryKind(kind: AttemptRecoveryKind): RequestFailureCause { + return RECOVERY_KIND_CAUSE[kind]; +} diff --git a/src/server/request-metrics.ts b/src/server/request-metrics.ts index d6126d5d9da..7ba966f5c45 100644 --- a/src/server/request-metrics.ts +++ b/src/server/request-metrics.ts @@ -1,13 +1,26 @@ import type { ResponsesTerminalStatus } from "../bridge"; import type { AttemptRecoveryKind } from "../usage/log"; +import { type RequestFailureCause, causeForRecoveryKind } from "../lib/request-failure-model"; export const REQUEST_METRICS_PROTOCOLS = Object.freeze(["responses", "chat", "messages", "unknown"] as const); export const REQUEST_METRICS_RESULTS = Object.freeze(["completed", "failed", "incomplete", "aborted"] as const); +/** + * Closed recovery classes exported as Prometheus label values. + * + * Bounded by construction: the label can only ever take one of these strings, so no user, model, + * account or request identifier can reach a series name. `quota`, `policy` and `ciphertext` are + * separate members because an operator seeing a spike needs to know which one it is -- waiting + * out a rate limit, changing accounts, changing the prompt and dropping stale ciphertext are + * four different responses, and collapsing them is what made the existing counter unactionable. + */ export const REQUEST_METRICS_RECOVERY_CLASSES = Object.freeze([ "transient", "connection", "credential", "rate_limit", + "quota", + "policy", + "ciphertext", "payload", "empty_completion", "effort_downgrade", @@ -83,23 +96,34 @@ function classifyResult(fact: RequestMetricFinalFact): RequestMetricsResult { return "failed"; } +/** + * Metrics class for each shared failure cause. + * + * Keyed on the cause rather than on the recovery kind so this projection and the durable log + * speak one vocabulary. Total by construction: the previous switch ended in `default: "other"`, + * which meant a recovery kind added later compiled cleanly and then disappeared into an + * unactionable bucket. A missing member is now a typecheck failure. + */ +const CAUSE_METRICS_CLASS = { + "transport-unsent": "connection", + "transport-ambiguous": "connection", + "upstream-declined": "transient", + "rate-limit": "rate_limit", + "quota-exhausted": "quota", + "credential-rejected": "credential", + "policy-refusal": "policy", + "parameter-rejected": "effort_downgrade", + "ciphertext-refusal": "ciphertext", + "payload-too-large": "payload", + "payload-rejected": "payload", + "upstream-fault": "transient", + "empty-output": "empty_completion", + "client-cancelled": "other", + "local-refusal": "other", +} as const satisfies Record; + function recoveryClass(kind: AttemptRecoveryKind): RequestMetricsRecoveryClass { - switch (kind) { - case "transient-5xx": return "transient"; - case "connection-reset": return "connection"; - case "oauth-401": - case "key-401": return "credential"; - case "key-429": - case "rate-limit-429": - case "anthropic-oauth-429": - case "oauth-account-429": return "rate_limit"; - case "image-413": - case "console-go-upload-retry": - case "opaque-blob-rejection": return "payload"; - case "empty-completion": return "empty_completion"; - case "reasoning-effort-downgrade": return "effort_downgrade"; - default: return "other"; - } + return CAUSE_METRICS_CLASS[causeForRecoveryKind(kind)]; } function observeHistogram(cell: HistogramCell, bounds: readonly number[], value: number): void { diff --git a/src/server/responses/codex-ws-wire.ts b/src/server/responses/codex-ws-wire.ts index 35764c83522..8bef362d0c0 100644 --- a/src/server/responses/codex-ws-wire.ts +++ b/src/server/responses/codex-ws-wire.ts @@ -4,6 +4,7 @@ import { UPSTREAM_CLOSED_BEFORE_RESPONSE_CODE, UPSTREAM_NO_RESPONSE_CODE, } from "../../lib/upstream-retry"; +import type { RequestFailureCause, RequestFailureStage } from "../../lib/request-failure-model"; import { readFileSync } from "node:fs"; // If the 101 never arrives (network black hole), give SSE a chance well before // the caller's connect timeout (default 200s) would fire. @@ -210,6 +211,37 @@ export function classifyCodexWsFailure(stage: CodexWsFailureStage): CodexWsFailu return "no-response-event"; } +/** + * The same four outcomes said in the shared stage-and-cause vocabulary (#4191). + * + * A projection, not a second classifier: {@link classifyCodexWsFailure} stays the one place that + * reads the counters, and this only restates its answer in the words the durable log, the metrics + * projection and the HTTP path already use. Without it the WebSocket transport is the one surface + * whose failures cannot be compared with anything else, which is the reported symptom -- every + * such failure reached the user as one of two bare sentences. + * + * It does not relax the transport's own rule. The no-replay-after-send contract in + * `codex-ws-exchange.ts` holds regardless of what this returns, and the stage below is + * deliberately not consulted as a fallback-eligibility signal; it reports where the exchange got + * to, and `resendPermission` happens to agree that everything past `before-send` is refused. + */ +export const CODEX_WS_FAILURE_PROJECTION = { + /** The create frame never left, so the origin provably never saw this turn. */ + "before-send": { stage: "pre-header", cause: "transport-unsent" }, + /** The frame left and the socket said nothing at all. The turn may be running upstream. */ + "no-upstream-frame": { stage: "pre-header", cause: "transport-ambiguous" }, + /** Control frames only: the peer is alive and answered, but no Responses event arrived. */ + "no-response-event": { stage: "protocol-prelude", cause: "transport-ambiguous" }, + /** Events already reached the caller, so a resend would duplicate output they have seen. */ + "after-response-started": { stage: "semantic-output", cause: "transport-ambiguous" }, +} as const satisfies Record; + +export function projectCodexWsFailure( + stage: CodexWsFailureStage, +): { stage: RequestFailureStage; cause: RequestFailureCause } { + return CODEX_WS_FAILURE_PROJECTION[classifyCodexWsFailure(stage)]; +} + /** * Render the stage as a suffix appended to an existing failure message. * diff --git a/src/server/responses/core-opaque-recovery.ts b/src/server/responses/core-opaque-recovery.ts index 03df9766863..13a65b76ea0 100644 --- a/src/server/responses/core-opaque-recovery.ts +++ b/src/server/responses/core-opaque-recovery.ts @@ -116,6 +116,88 @@ export function isReasoningBlobCallerMismatchMessage(message: string): boolean { } +/** + * Longest embedded payload this will parse. The wrapper is a short error envelope; anything + * larger is not the shape being matched, and refusing to walk it keeps an upstream-controlled + * string from deciding how much work the classifier does. + */ +const LITELLM_EMBEDDED_PAYLOAD_LIMIT = 16_384; +const LITELLM_WRAPPER_PREFIX = "litellm.BadRequestError:"; +const LITELLM_WRAPPER_MARKER = "OpenAIException - "; + +/** + * The JSON an OpenAI-compatible gateway embeds in its own error message, or undefined. + * + * Brace-aware rather than a regex because the embedded object legitimately contains braces and + * escaped quotes inside its message, and the gateway appends its own prose after the closing + * brace. Counting depth outside string literals is the only way to find the real end. + */ +function liteLlmEmbeddedErrorPayload(message: string): unknown { + if (!message.startsWith(LITELLM_WRAPPER_PREFIX)) return undefined; + const markerIndex = message.indexOf(LITELLM_WRAPPER_MARKER); + if (markerIndex < 0) return undefined; + const start = message.indexOf("{", markerIndex + LITELLM_WRAPPER_MARKER.length); + if (start < 0) return undefined; + const end = Math.min(message.length, start + LITELLM_EMBEDDED_PAYLOAD_LIMIT); + + let depth = 0; + let inString = false; + let escaped = false; + for (let index = start; index < end; index += 1) { + const character = message[index]!; + if (inString) { + if (escaped) escaped = false; + else if (character === "\\") escaped = true; + else if (character === '"') inString = false; + continue; + } + if (character === '"') { inString = true; continue; } + if (character === "{") depth += 1; + else if (character === "}") { + depth -= 1; + if (depth === 0) { + try { return JSON.parse(message.slice(start, index + 1)) as unknown; } catch { return undefined; } + } + } + } + return undefined; +} + + +/** True for an error message that is a gateway envelope rather than an upstream's own wording. */ +function isLiteLlmEnvelopeMessage(message: string): boolean { + return message.startsWith(LITELLM_WRAPPER_PREFIX) && message.includes(LITELLM_WRAPPER_MARKER); +} + + +/** + * An OpenAI-compatible gateway relaying the one authoritative ciphertext rejection inside its + * own error string. + * + * Deliberately narrower than {@link isSelfIdentifiedOpaqueBlobRejection}. The embedded payload is + * matched against exactly one identity -- `invalid_request_error` carrying + * `invalid_encrypted_content` -- and the generic classifier is NOT re-run against it. Re-running + * it would let every other opaque identity arrive through the wrapper as well: the code-less + * unverifiable-ciphertext wording, the #4469 caller mismatch, and the two xAI decoder strings. + * Each of those was admitted on evidence from a specific upstream about how that upstream words + * its own rejection, and a gateway in between is not that evidence. Only the coded identity is + * unambiguous enough to survive relaying. + */ +export function isLiteLlmWrappedCiphertextRejection(payload: unknown): boolean { + if (!payload || typeof payload !== "object" || Array.isArray(payload)) return false; + const outer = (payload as { error?: unknown }).error; + if (!outer || typeof outer !== "object" || Array.isArray(outer)) return false; + const message = (outer as { message?: unknown }).message; + if (typeof message !== "string") return false; + const embedded = liteLlmEmbeddedErrorPayload(message); + if (!embedded || typeof embedded !== "object" || Array.isArray(embedded)) return false; + const inner = (embedded as { error?: unknown }).error; + if (!inner || typeof inner !== "object" || Array.isArray(inner)) return false; + const { type, code } = inner as { type?: unknown; code?: unknown }; + return type === "invalid_request_error" && code === "invalid_encrypted_content"; +} + + export function isSelfIdentifiedOpaqueBlobRejection(bodyText: string): boolean { if (isEncryptedFunctionOutputRejection(bodyText)) return true; try { @@ -132,6 +214,14 @@ export function isSelfIdentifiedOpaqueBlobRejection(bodyText: string): boolean { if (record.error && typeof record.error === "object" && !Array.isArray(record.error)) { const error = record.error as { type?: unknown; code?: unknown; message?: unknown }; + // A gateway envelope is decided ONLY by its embedded payload, before any wording check + // below runs. Those checks match anchored phrases anywhere in the message, and a gateway + // quotes the upstream's message inside its own -- so without this the relayed text would + // satisfy the caller-mismatch identity and gain a resend the strict wrapper check exists to + // withhold. Returning here rather than falling through is the point. + if (typeof error.message === "string" && isLiteLlmEnvelopeMessage(error.message)) { + return isLiteLlmWrappedCiphertextRejection(payload); + } if (error.type === "invalid_request_error") { if (error.code === "invalid_encrypted_content") return true; if ( diff --git a/src/usage/log.ts b/src/usage/log.ts index 4c1cc58204b..b1bd2193e02 100644 --- a/src/usage/log.ts +++ b/src/usage/log.ts @@ -61,21 +61,30 @@ export function isCodexPoolAccountLogLabel(value: unknown): value is "main" | `p /** * Recovery kinds recorded per attempt in the usage log; the GUI renders localized labels * for these wire values. + * + * The roster is the single statement of this vocabulary and the type is derived from it. It used + * to be written twice -- once as a union here, once as the read-back whitelist below -- and the + * two are not interchangeable: a member added only to the union is accepted by the compiler, + * written to disk, and then silently dropped by `normalizedAttempt`, so the row loses its reason + * on the next read. One declaration cannot drift from itself. */ -export type AttemptRecoveryKind = - | "transient-5xx" - | "connection-reset" - | "oauth-401" - | "key-401" - | "key-429" - | "rate-limit-429" - | "anthropic-oauth-429" - | "oauth-account-429" - | "image-413" - | "console-go-upload-retry" - | "opaque-blob-rejection" - | "empty-completion" - | "reasoning-effort-downgrade"; +export const ATTEMPT_RECOVERY_KIND_ROSTER = Object.freeze([ + "transient-5xx", + "connection-reset", + "oauth-401", + "key-401", + "key-429", + "rate-limit-429", + "anthropic-oauth-429", + "oauth-account-429", + "image-413", + "console-go-upload-retry", + "opaque-blob-rejection", + "empty-completion", + "reasoning-effort-downgrade", +] as const); + +export type AttemptRecoveryKind = typeof ATTEMPT_RECOVERY_KIND_ROSTER[number]; /** * Why a recovery this request was otherwise willing to make did not happen. @@ -92,10 +101,15 @@ export type AttemptRecoveryKind = * * Bounded vocabulary on purpose: it is a wire value a maintainer reads, never a credential, an * account id, an upstream body, prompt content, or exception text. + * + * Declared as a roster for the same reason as {@link ATTEMPT_RECOVERY_KIND_ROSTER}. */ -export type AttemptRecoveryWithheld = - | "retry-send-budget" - | "rotation-send-budget"; +export const ATTEMPT_RECOVERY_WITHHELD_ROSTER = Object.freeze([ + "retry-send-budget", + "rotation-send-budget", +] as const); + +export type AttemptRecoveryWithheld = typeof ATTEMPT_RECOVERY_WITHHELD_ROSTER[number]; /** Request-time upstream credential class, never a credential or account identifier. */ export type UsageCredentialSource = "grok-oauth" | "xai-api-key"; @@ -497,25 +511,8 @@ function normalizeUsageValue(usage: OcxUsage | undefined): OcxUsage | undefined }; } -const ATTEMPT_RECOVERY_KINDS = new Set([ - "transient-5xx", - "connection-reset", - "oauth-401", - "key-401", - "key-429", - "rate-limit-429", - "anthropic-oauth-429", - "oauth-account-429", - "image-413", - "console-go-upload-retry", - "opaque-blob-rejection", - "empty-completion", - "reasoning-effort-downgrade", -]); -const ATTEMPT_RECOVERY_WITHHELD = new Set([ - "retry-send-budget", - "rotation-send-budget", -]); +const ATTEMPT_RECOVERY_KINDS: ReadonlySet = new Set(ATTEMPT_RECOVERY_KIND_ROSTER); +const ATTEMPT_RECOVERY_WITHHELD: ReadonlySet = new Set(ATTEMPT_RECOVERY_WITHHELD_ROSTER); const USAGE_STATUSES = new Set([ "reported", "unreported", diff --git a/structure/gui-and-management-api.md b/structure/gui-and-management-api.md index 5ba41a3575d..d179abaa328 100644 --- a/structure/gui-and-management-api.md +++ b/structure/gui-and-management-api.md @@ -620,7 +620,10 @@ log scan, or persistence. Restart creates a fresh owner, resets every counter/hi `opencodex_metrics_process_start_time_seconds`. The label vocabularies are closed: protocol is `responses`, `chat`, `messages`, or `unknown`; result -is `completed`, `failed`, `incomplete`, or `aborted`; recovery is one of eight coarse classes. A +is `completed`, `failed`, `incomplete`, or `aborted`; recovery is one of the coarse classes listed in +`REQUEST_METRICS_RECOVERY_CLASSES`, which is the roster the exporter itself iterates. The count is +deliberately not restated here: it was written as eight, a bounded label value was added, and the +documentation then contradicted the output it describes. A logical request increments once, physical sends sum the finalized attempt counts, and each distinct recovery kind already retained on an attempt contributes once to its coarse class. HTTP 200 never overrides a failed terminal event. Duration observes every valid finalized duration; TTFT observes diff --git a/structure/overview.md b/structure/overview.md index c0627e2dd7f..7178b292b70 100644 --- a/structure/overview.md +++ b/structure/overview.md @@ -130,6 +130,13 @@ still cover the rule, which is a judgement only review makes. there, reported as an unidentified holder otherwise. A configured `port: 0` still asks the OS for a port, and an explicit `--port` still waits for its pin instead of hopping. Enforced by `tests/cli/cli-dispatch.test.ts`. +- **INV-RESEND-01** — One vocabulary in `src/lib/request-failure-model.ts` states how far a failed + request got, why it failed, and whether it may be sent again. Once the caller has observed output + or an externally visible effect, no cause automatically permits a resend, and a cause whose + upstream execution state is unknown is not made replayable by having budget left. A refusal names + which of the three refusals it is. The decision is derived from per-stage and per-cause facts + rather than written out as a stage-by-cause matrix, so a new member cannot leave a stale cell. + Enforced by `tests/lib/failure-stage-model.test.ts`. CI enumerates that domain layout through `scripts/ci/run-bun-test-batches.sh`. Its default general scope and 12-file/120-second process shape leave the dedicated Linux storage-policy and api-usage diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index d8a39f76b8b..41f90938562 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -888,6 +888,7 @@ "openai-provider-option-tooling.test.ts": "adapters/openai", "openai-provider-option.test.ts": "adapters/openai", "openai-responses-passthrough.test.ts": "responses", + "opaque-blob-wrapped-rejection.test.ts": "responses", "opencode-cli.test.ts": "providers", "opencode-management-transport.test.ts": "providers", "opencode-free-provider.test.ts": "providers", @@ -993,6 +994,7 @@ "reasoning-replay-robustness.test.ts": "adapters", "reasoning-replay-scope-source.test.ts": "lib", "redact.test.ts": "lib", + "failure-stage-model.test.ts": "lib", "relay-eager.test.ts": "server", "release-helper.test.ts": "ci-workflows", "release-notes.test.ts": "ci-workflows", diff --git a/tests/lib/failure-stage-model.test.ts b/tests/lib/failure-stage-model.test.ts new file mode 100644 index 00000000000..501c6b9228e --- /dev/null +++ b/tests/lib/failure-stage-model.test.ts @@ -0,0 +1,205 @@ +import { describe, expect, test } from "bun:test"; +import { + REQUEST_FAILURE_CAUSES, + REQUEST_FAILURE_STAGES, + causeDisposition, + causeEvidence, + causeForRecoveryKind, + permitsResend, + resendPermission, + resendSendClass, + stageCommitment, + stageRank, + type RequestFailureCause, + type RequestFailureStage, +} from "../../src/lib/request-failure-model"; +import { ATTEMPT_RECOVERY_KIND_ROSTER } from "../../src/usage/log"; + +/** + * Roadmap items 7 and 14. None of these cases can be satisfied by a request that returned 200: + * every one of them asks what the proxy would do NEXT after a specific failure, which is the + * question a success assertion cannot reach. + * + * They are written over the full cross product rather than over chosen examples on purpose. A + * table this shape is exactly where a merge of two individually correct branches goes wrong -- one + * adds a cause, the other adds a stage, and the cell neither author looked at is the defect. A + * loop over the declared rosters has no cell to miss and cannot go stale when a member is added. + * + * Holds INV-RESEND-01 from structure/overview.md. + */ + +const STAGES: readonly RequestFailureStage[] = REQUEST_FAILURE_STAGES; +const CAUSES: readonly RequestFailureCause[] = REQUEST_FAILURE_CAUSES; + +describe("request failure stage ordering", () => { + test("ranks are the declared positions, unique and gapless", () => { + expect(STAGES.map(stageRank)).toEqual(STAGES.map((_, index) => index)); + expect(new Set(STAGES).size).toBe(STAGES.length); + }); + + test("exactly one stage observes output and exactly one observes an effect", () => { + const byCommitment = STAGES.filter(stage => stageCommitment(stage) === "output-observed"); + const effects = STAGES.filter(stage => stageCommitment(stage) === "effect-observed"); + expect(byCommitment).toEqual(["semantic-output"]); + expect(effects).toEqual(["side-effect"]); + }); +}); + +describe("resend permission", () => { + test("an uncertain resend after output or a side effect is never automatically permitted", () => { + const permitted: string[] = []; + for (const stage of STAGES) { + const commitment = stageCommitment(stage); + if (commitment !== "output-observed" && commitment !== "effect-observed") continue; + for (const cause of CAUSES) { + const permission = resendPermission(stage, cause); + if (permission !== "refused-committed") permitted.push(`${stage}/${cause}=${permission}`); + } + } + expect(permitted).toEqual([]); + }); + + /** + * Stated as "neither output nor an effect" rather than "nothing at all", because a turn can + * settle without ever producing output. A terminal that failed on a rate limit committed nothing + * downstream and is safe to send again; forbidding it would be the same mistake as ranking + * `terminal` above `semantic-output` and calling that commitment. + */ + test("no permitted resend follows observed output or an observed effect", () => { + const leaks: string[] = []; + for (const stage of STAGES) { + const commitment = stageCommitment(stage); + if (commitment !== "output-observed" && commitment !== "effect-observed") continue; + for (const cause of CAUSES) { + if (permitsResend(resendPermission(stage, cause))) leaks.push(`${stage}/${cause}`); + } + } + expect(leaks).toEqual([]); + }); + + /** + * The #4989 boundary. A stream that announced itself and produced nothing may be replaced; the + * first output-bearing event closes that door for every cause at once. + */ + test("the prelude may still be replaced and the two observed stages may not", () => { + expect(resendPermission("protocol-prelude", "upstream-declined")).toBe("permitted"); + for (const stage of ["semantic-output", "side-effect"] as const) { + for (const cause of CAUSES) expect(permitsResend(resendPermission(stage, cause))).toBe(false); + } + }); + + test("an ambiguous transport failure is refused at every stage, budget or not", () => { + for (const stage of STAGES) { + expect(permitsResend(resendPermission(stage, "transport-ambiguous"))).toBe(false); + } + expect(resendPermission("pre-header", "transport-ambiguous")).toBe("refused-ambiguous"); + expect(resendPermission("semantic-output", "transport-ambiguous")).toBe("refused-committed"); + }); + + /** + * Stated as a property rather than as a list of causes, because a list here would be one more + * hand-maintained restatement of the dictionary -- the thing these cases exist to prevent. + */ + test("nothing is repeated at pre-header unless the origin provably did not run it", () => { + expect(resendPermission("pre-header", "transport-unsent")).toBe("permitted"); + const unproven = CAUSES + .filter(cause => permitsResend(resendPermission("pre-header", cause))) + .filter(cause => causeEvidence(cause) !== "not-processed" && causeEvidence(cause) !== "declined"); + expect(unproven).toEqual([]); + }); + + test("a refusal always says which kind of refusal it is", () => { + const refusals = new Set(); + for (const stage of STAGES) { + for (const cause of CAUSES) { + const permission = resendPermission(stage, cause); + if (!permitsResend(permission)) refusals.add(permission); + } + } + expect([...refusals].sort()).toEqual(["refused-ambiguous", "refused-committed", "refused-futile"]); + }); +}); + +describe("the four refusals an operator has to tell apart", () => { + /** + * #5180 reported these arriving as one undifferentiated failure. Waiting, changing account, + * changing the prompt and dropping stale ciphertext are four different responses, so the + * decision each one produces has to differ somewhere a caller can read. + */ + test("rate limit, quota exhaustion, policy refusal and ciphertext refusal never collapse", () => { + const quartet = ["rate-limit", "quota-exhausted", "policy-refusal", "ciphertext-refusal"] as const; + const decisions = quartet.map(cause => JSON.stringify([ + causeEvidence(cause), + causeDisposition(cause), + resendSendClass(cause), + resendPermission("headers-only", cause), + ])); + expect(new Set(decisions).size).toBe(quartet.length); + }); + + test("each of the four keeps the decision its remedy implies", () => { + expect(resendPermission("headers-only", "rate-limit")).toBe("permitted"); + expect(resendPermission("headers-only", "quota-exhausted")).toBe("refused-futile"); + expect(resendPermission("headers-only", "policy-refusal")).toBe("refused-futile"); + expect(resendPermission("headers-only", "ciphertext-refusal")).toBe("permitted-after-repair"); + }); + + test("a rejected parameter is not a refused prompt", () => { + expect(causeDisposition("parameter-rejected")).toBe("resend-after-repair"); + expect(causeDisposition("policy-refusal")).toBe("resend-is-futile"); + }); +}); + +describe("send funding agrees with the permission table", () => { + /** + * Funding follows the disposition, not the permission. A cause the table refuses to resend + * automatically may still be resent by a bounded opt-in recovery, and that send must still be + * bought from the request-wide budget -- an unfunded path is how a per-layer counter returns. + */ + test("a budget class is named for every cause a resend could ever help", () => { + const disagreements: string[] = []; + for (const cause of CAUSES) { + const futile = causeDisposition(cause) === "resend-is-futile"; + if (futile !== (resendSendClass(cause) === null)) disagreements.push(cause); + } + expect(disagreements).toEqual([]); + }); + + test("the bounded opt-in recoveries the proxy already performs are funded", () => { + // #4942's reset replay, the empty-completion rebuild and the transient 5xx ladder are all + // refused automatically and all really send, so all three need an allowance to draw on. + for (const cause of ["transport-ambiguous", "empty-output", "upstream-fault"] as const) { + expect(permitsResend(resendPermission("pre-header", cause))).toBe(false); + expect(resendSendClass(cause)).toBe("transient"); + } + }); +}); + +describe("recovery kinds speak the shared dictionary", () => { + test("every recorded recovery kind resolves to a declared cause", () => { + const unmapped = ATTEMPT_RECOVERY_KIND_ROSTER.filter( + kind => !CAUSES.includes(causeForRecoveryKind(kind)), + ); + expect(unmapped).toEqual([]); + }); + + test("the kinds that drive different recoveries do not share one cause", () => { + expect(causeForRecoveryKind("opaque-blob-rejection")).toBe("ciphertext-refusal"); + expect(causeForRecoveryKind("image-413")).toBe("payload-too-large"); + expect(causeForRecoveryKind("connection-reset")).toBe("transport-ambiguous"); + expect(causeForRecoveryKind("transient-5xx")).toBe("upstream-fault"); + expect(causeForRecoveryKind("reasoning-effort-downgrade")).toBe("parameter-rejected"); + }); + + /** + * Every kind in the roster names a recovery this proxy actually performs, so none of them may + * classify as futile. The first draft put the 413 image rebuild and the gateway upload replay + * under a futile cause, which would have had the shared model assert that recoveries visible in + * the durable log were forbidden and unfunded. + */ + test("no recorded recovery classifies as a resend that cannot help", () => { + const futile = ATTEMPT_RECOVERY_KIND_ROSTER + .filter(kind => causeDisposition(causeForRecoveryKind(kind)) === "resend-is-futile"); + expect(futile).toEqual([]); + }); +}); diff --git a/tests/responses/opaque-blob-wrapped-rejection.test.ts b/tests/responses/opaque-blob-wrapped-rejection.test.ts new file mode 100644 index 00000000000..f8a1bced390 --- /dev/null +++ b/tests/responses/opaque-blob-wrapped-rejection.test.ts @@ -0,0 +1,171 @@ +import { describe, expect, test } from "bun:test"; +import { shouldAttemptOpaqueBlobRecovery } from "../../src/server/responses/core"; +import { isLiteLlmWrappedCiphertextRejection } from "../../src/server/responses/core-opaque-recovery"; +import { causeForRecoveryKind, resendPermission } from "../../src/lib/request-failure-model"; + +/** + * #5245: an OpenAI-compatible gateway relays the ciphertext rejection inside its own error + * string, so the identity the single-shot sanitized rebuild keys on never matched and the turn + * failed outright instead of retrying without the stale blob. + * + * Held in a sibling file rather than appended to responses-opaque-blob-recovery.test.ts, which + * sits 148 lines under the size ratchet's new-file threshold. Two branches can each stay under a + * cap alone and sum over it together, and the remedy for that is a move, never a number. + * + * The negative cases carry the weight here. Recognising the wrapper is easy; recognising ONLY + * the coded identity through it is the part a broad implementation gets wrong, because rerunning + * the whole classifier on the embedded payload silently admits four other identities that were + * each accepted on evidence about how one specific upstream words its own rejection. + */ + +const BLOB = "provider-minted-opaque-state"; + +function outboundWithBlob(): string { + return JSON.stringify({ + model: "model-a", + input: [ + { type: "message", role: "user", content: [{ type: "input_text", text: "before" }] }, + { type: "reasoning", content: [], summary: [], encrypted_content: BLOB, status: "completed" }, + ], + }); +} + +/** An upstream error body as a gateway relays it: the real payload inside a prose message. */ +function wrapped(inner: unknown, trailing = ""): string { + return JSON.stringify({ + error: { + message: `litellm.BadRequestError: OpenAIException - ${JSON.stringify(inner)}${trailing}`, + type: "invalid_request_error", + code: "400", + }, + }); +} + +const CODED_CIPHERTEXT_REJECTION = { + error: { + message: "The encrypted content could not be verified.", + type: "invalid_request_error", + code: "invalid_encrypted_content", + }, +}; + +const base = { + status: 400, + adapterName: "openai-responses", + outboundBody: outboundWithBlob(), + errorBody: wrapped(CODED_CIPHERTEXT_REJECTION), + alreadyAttempted: false, +}; + +describe("a relayed ciphertext rejection", () => { + test("earns the sanitized rebuild the direct rejection already earned", () => { + expect(shouldAttemptOpaqueBlobRecovery(base)).toBe(true); + }); + + test("survives braces and escaped quotes inside the relayed message", () => { + const awkward = { + error: { + message: 'The encrypted content {"id": "a\\"b"} could not be verified.', + type: "invalid_request_error", + code: "invalid_encrypted_content", + }, + }; + expect(shouldAttemptOpaqueBlobRecovery({ ...base, errorBody: wrapped(awkward) })).toBe(true); + }); + + test("survives the prose a gateway appends after the payload", () => { + const errorBody = wrapped(CODED_CIPHERTEXT_REJECTION, " Received Model Group=gpt-5"); + expect(shouldAttemptOpaqueBlobRecovery({ ...base, errorBody })).toBe(true); + }); + + test("is still single-shot and still requires the send to have carried a blob", () => { + expect(shouldAttemptOpaqueBlobRecovery({ ...base, alreadyAttempted: true })).toBe(false); + expect(shouldAttemptOpaqueBlobRecovery({ + ...base, + outboundBody: JSON.stringify({ model: "model-a", input: [{ type: "message", role: "user" }] }), + })).toBe(false); + expect(shouldAttemptOpaqueBlobRecovery({ ...base, status: 500 })).toBe(false); + expect(shouldAttemptOpaqueBlobRecovery({ ...base, adapterName: "openai-chat" })).toBe(false); + }); +}); + +describe("the wrapper admits the coded identity and nothing else", () => { + /** + * Each of these is accepted when the upstream states it DIRECTLY. None may be accepted through + * a relay: the wording evidence belongs to the upstream that produced it, and a gateway in + * between is not that evidence. + */ + const relayedButNotAdmitted: ReadonlyArray = [ + ["an unrelated parameter complaint", { + error: { type: "invalid_request_error", code: "unknown_parameter", message: "Unknown parameter" }, + }], + ["the code-less unverifiable-ciphertext wording", { + error: { + type: "invalid_request_error", + code: null, + message: "The encrypted content 6871-test-ef-0 could not be verified." + + " Reason: Encrypted content could not be decrypted or parsed.", + }, + }], + ["the caller-mismatch wording", { + error: { + type: "invalid_request_error", + code: null, + message: "reasoning `encrypted_content` was not issued to this caller", + }, + }], + ["an xAI compaction-blob decoder error", { + code: "invalid-argument", + error: "Could not decode the compaction blob: invalid payload", + }], + ["a rate limit", { error: { type: "rate_limit_error", code: "rate_limit_exceeded", message: "Slow down" } }], + ["a quota exhaustion", { error: { type: "insufficient_quota", code: "insufficient_quota", message: "No credit" } }], + ["a policy refusal", { error: { type: "invalid_request_error", code: "content_policy_violation", message: "Refused" } }], + ]; + + for (const [label, inner] of relayedButNotAdmitted) { + test(`does not resend on ${label}`, () => { + expect(shouldAttemptOpaqueBlobRecovery({ ...base, errorBody: wrapped(inner) })).toBe(false); + }); + } + + test("ignores a message that only looks like the wrapper", () => { + expect(isLiteLlmWrappedCiphertextRejection(JSON.parse(wrapped(CODED_CIPHERTEXT_REJECTION)))).toBe(true); + expect(isLiteLlmWrappedCiphertextRejection({ + error: { message: `OpenAIException - ${JSON.stringify(CODED_CIPHERTEXT_REJECTION)}` }, + })).toBe(false); + expect(isLiteLlmWrappedCiphertextRejection({ error: { message: "litellm.BadRequestError: no payload" } })).toBe(false); + expect(isLiteLlmWrappedCiphertextRejection({ error: { message: 17 } })).toBe(false); + expect(isLiteLlmWrappedCiphertextRejection(null)).toBe(false); + }); + + test("refuses a relayed payload too large to be this envelope", () => { + const padded = { + error: { + message: "x".repeat(20_000), + type: "invalid_request_error", + code: "invalid_encrypted_content", + }, + }; + expect(isLiteLlmWrappedCiphertextRejection(JSON.parse(wrapped(padded)))).toBe(false); + }); +}); + +describe("the relayed rejection lands on the shared cause", () => { + /** + * The recovery this path takes is recorded as `opaque-blob-rejection`, and the shared table has + * to agree that a ciphertext refusal is repaired rather than repeated or waited out. If these + * ever disagree, the durable log and the metrics projection describe a different decision from + * the one the code made. + */ + test("is a ciphertext refusal, repaired rather than repeated", () => { + expect(causeForRecoveryKind("opaque-blob-rejection")).toBe("ciphertext-refusal"); + expect(resendPermission("headers-only", "ciphertext-refusal")).toBe("permitted-after-repair"); + expect(resendPermission("headers-only", "rate-limit")).toBe("permitted"); + expect(resendPermission("headers-only", "quota-exhausted")).toBe("refused-futile"); + }); + + test("a ciphertext refusal after output reached the caller is not repaired either", () => { + expect(resendPermission("semantic-output", "ciphertext-refusal")).toBe("refused-committed"); + }); +}); diff --git a/tests/responses/ws-failure-stage.test.ts b/tests/responses/ws-failure-stage.test.ts index e84145b04dc..7985d979878 100644 --- a/tests/responses/ws-failure-stage.test.ts +++ b/tests/responses/ws-failure-stage.test.ts @@ -9,10 +9,12 @@ import { closedBeforeTerminalMessage, codexWsFailureDetail, markCodexWsStage, + projectCodexWsFailure, readCodexWsStage, type CodexWsFailureStage, type CodexWsStageRecord, } from "../../src/server/responses/codex-ws-wire"; +import { permitsResend, resendPermission } from "../../src/lib/request-failure-model"; import { codexWsUpstreamFetch, CODEX_WS_RESPONSE_PRELUDE_TIMEOUT_MS, @@ -161,6 +163,41 @@ describe("codex WS failure classification", () => { .toBe("before-send"); }); + /** + * The same four outcomes said in the shared stage-and-cause vocabulary, so a WebSocket failure + * can be compared with an HTTP one instead of being the one surface with private words for it. + * These rows are asserted individually because a projection is a mapping, and a mapping whose + * rows are only checked for totality can be rewritten wholesale without any case objecting. + */ + test("projects each outcome onto the shared stage and cause", () => { + expect(projectCodexWsFailure(stage({ sent: false, elapsedMs: null }))) + .toEqual({ stage: "pre-header", cause: "transport-unsent" }); + expect(projectCodexWsFailure(stage())) + .toEqual({ stage: "pre-header", cause: "transport-ambiguous" }); + expect(projectCodexWsFailure(stage({ upstreamFrames: 3, controlFrames: 3 }))) + .toEqual({ stage: "protocol-prelude", cause: "transport-ambiguous" }); + expect(projectCodexWsFailure(stage({ upstreamFrames: 9, controlFrames: 2, relayedEvents: 7 }))) + .toEqual({ stage: "semantic-output", cause: "transport-ambiguous" }); + }); + + /** + * The shared table has to reach the same verdict the transport already enforces on its own, or + * one of the two is lying about this exchange. A create frame that never left is the only + * outcome the origin provably did not see. + */ + test("only an unsent create frame may be sent again", () => { + const resendable = ([ + stage({ sent: false, elapsedMs: null }), + stage(), + stage({ upstreamFrames: 3, controlFrames: 3 }), + stage({ upstreamFrames: 9, controlFrames: 2, relayedEvents: 7 }), + ]).map(candidate => { + const projected = projectCodexWsFailure(candidate); + return permitsResend(resendPermission(projected.stage, projected.cause)); + }); + expect(resendable).toEqual([true, false, false, false]); + }); + test("renders every field, with n/a for the durations that do not exist yet", () => { expect(codexWsFailureDetail(stage({ upstreamFrames: 2, controlFrames: 2, firstFrameMs: 41 }))).toBe( " [cause=no-response-event request=812B sent=yes frames=2 control=2 relayed=0" diff --git a/tests/server/management-metrics-export.test.ts b/tests/server/management-metrics-export.test.ts index cccc37dc546..9759a2d91b0 100644 --- a/tests/server/management-metrics-export.test.ts +++ b/tests/server/management-metrics-export.test.ts @@ -16,7 +16,14 @@ import { type RequestLogContext, type RequestLogEntry, } from "../../src/server/request-log"; -import { createRequestMetricsOwner } from "../../src/server/request-metrics"; +import { + createRequestMetricsOwner, + REQUEST_DURATION_BUCKETS_SECONDS, + REQUEST_METRICS_PROTOCOLS, + REQUEST_METRICS_RECOVERY_CLASSES, + REQUEST_METRICS_RESULTS, + REQUEST_TTFT_BUCKETS_SECONDS, +} from "../../src/server/request-metrics"; import { startServer } from "../../src/server"; import type { OcxConfig } from "../../src/types"; import type { AttemptRecoveryKind } from "../../src/usage/log"; @@ -339,6 +346,31 @@ describe("request metrics aggregation", () => { expect(sampleValue(output, 'opencodex_recoveries_total{protocol="responses",recovery="rate_limit"}')).toBe(2); }); + test("the four refusals an operator responds to differently get four different classes", () => { + const metrics = createRequestMetricsOwner(123); + addFinalRequestLog("refusal-classes", Date.now() - 1_000, { + model: "m", provider: "p", inboundProtocol: "responses", requestMetricsRecorder: metrics, + attempts: [ + attempt(1, ["opaque-blob-rejection"]), + { ...attempt(1, ["rate-limit-429"]), ordinal: 2 }, + { ...attempt(1, ["reasoning-effort-downgrade"]), ordinal: 3 }, + { ...attempt(1, ["image-413"]), ordinal: 4 }, + ], + } as RequestLogContext, 400, undefined, () => {}); + + const output = metrics.snapshot(); + // A rejected opaque blob is a ciphertext refusal, not a payload problem: the payload was fine + // and the stale encrypted state was not. Counting it as payload alongside an oversize image + // told an operator to look at the wrong thing. + expect(sampleValue(output, 'opencodex_recoveries_total{protocol="responses",recovery="ciphertext"}')).toBe(1); + expect(sampleValue(output, 'opencodex_recoveries_total{protocol="responses",recovery="payload"}')).toBe(1); + expect(sampleValue(output, 'opencodex_recoveries_total{protocol="responses",recovery="rate_limit"}')).toBe(1); + expect(sampleValue(output, 'opencodex_recoveries_total{protocol="responses",recovery="effort_downgrade"}')).toBe(1); + expect(sampleValue(output, 'opencodex_recoveries_total{protocol="responses",recovery="quota"}')).toBe(0); + expect(sampleValue(output, 'opencodex_recoveries_total{protocol="responses",recovery="policy"}')).toBe(0); + expect(sampleValue(output, 'opencodex_recoveries_total{protocol="responses",recovery="other"}')).toBe(0); + }); + test("a failed terminal carried over HTTP 200 is never counted as completed", () => { const metrics = createRequestMetricsOwner(123); metrics.recordFinalRequest({ @@ -405,6 +437,7 @@ describe("request metrics aggregation", () => { toolBody: canaries[8], } as unknown as RequestLogContext; addFinalRequestLog(canaries[0]!, Date.now() - 1, logCtx, 400, undefined, () => {}); + const beforeFanOut = metrics.snapshot().split("\n").filter(line => line && !line.startsWith("#")).length; for (let index = 0; index < 64; index += 1) { addFinalRequestLog(`request-${index}`, Date.now() - 1, { model: `model-${index}`, @@ -417,7 +450,23 @@ describe("request metrics aggregation", () => { const output = metrics.snapshot(); for (const canary of canaries) expect(output).not.toContain(canary); const samples = output.split("\n").filter(line => line && !line.startsWith("#")); - expect(samples).toHaveLength(453); + // The property, stated directly: 64 requests carrying 64 distinct models, providers, keys and + // account labels add no series at all. A dynamic label map would show up here as growth. + expect(samples).toHaveLength(beforeFanOut); + // And the absolute size, derived from the closed vocabularies rather than restated as a + // literal. The literal was correct and went stale the moment a bounded label value was added, + // which is the failure mode this repository keeps hitting in merges. + const perHistogram = (bounds: readonly number[]): number => bounds.length + 1 + 2; + const cells = REQUEST_METRICS_PROTOCOLS.length * REQUEST_METRICS_RESULTS.length; + expect(samples).toHaveLength( + cells + + REQUEST_METRICS_PROTOCOLS.length + + REQUEST_METRICS_PROTOCOLS.length * REQUEST_METRICS_RECOVERY_CLASSES.length + + cells * perHistogram(REQUEST_DURATION_BUCKETS_SECONDS) + + cells * perHistogram(REQUEST_TTFT_BUCKETS_SECONDS) + + cells + + 1, + ); }); test("text exposition has deterministic HELP/TYPE groups and cumulative +Inf buckets", () => {