[WRONG BRANCH] release: promote the verified 2.56.0 product tree to main - #4694
Merged
Merged
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…stry read (#4546) (#4620) Refs #4546. Fixes the product snapshot, both publish SHAs, every gate run id, and the one honest gap: the stable registry endpoint still answers 404 while the publish receipt and the v2.55.0 tag exist. That is registryVerification pending, and a second dispatch against the same version is exactly what the bounded-read path exists to prevent.
…4546) (#4624) * feat(routing): separate auth identity, quota domain and cache domain (#4546) Part of the stacked delivery closing the remaining OCX-4546 cost-guard scope. No call site is rewired; consuming layers land on top of this branch. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(routing): require evidence for a shared cache domain and make declared groups unambiguous (#4546) Review findings on the domain-contract layer: the OpenAI rule inferred cache SHARING from a document that only proves separation; a malformed credentialGroups entry dropped the entire pool object including kernel and cacheAffinity; and a credential claimed by two groups was resolved by array order. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(config): redact credential-group parse issues before warning, and mark the classifier inactive (#4546) Review findings on exact head 898ae81: the degraded-groups warning joined raw Zod issue messages that embed the offending member through JSON.stringify, so a malformed credential carrying secret material could be printed verbatim at config load; and the docs promised active capacity counting and rotation refusal that no routing boundary calls yet. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify.
…dger across restart (#4546) (#4625) * feat(lib): reserve tokens and output before dispatch, and keep the ledger across restart (#4546) Part of the stacked delivery closing the remaining OCX-4546 cost-guard scope. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(lib): refuse a duplicate send id, fail closed on a lost journal write, and bound retention (#4546) Review findings on the reservation ledger: a reused send id authorised a free dispatch, a failed journal append still admitted the request, replay parsed unvalidated JSON, retention was unbounded, an undispatched reservation booked phantom debt, and raw account identifiers reached disk. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify.
… Retry-After in full (#4546) (#4626) * feat(routing): bound recovery with a half-open probe lease and honour Retry-After in full (#4546) Part of the stacked delivery closing the remaining OCX-4546 cost-guard scope. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(routing): honour the caller's Retry-After deadline and bound probe state (#4546) Review findings on the probe-lease layer: fetchWithTransientRetry ignored the documented retryAfterCeilingMs, probeStates retained every account ever probed, and the lease expiry boundary disagreed between liveLease and settleTransientProbe. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof.
Records the execution contract for splitting seven oversized src/ files behind facades and for the file-size ratchet CI gate that keeps them from regrowing. Documents only; no runtime path reads devlog/.
Fails when a new tracked text file lands at 2000+ lines or an already-oversized file grows past its committed cap. Seeds 51 caps and 12 exact generated exemptions. No file is split here.
Pure move. state.ts 2432 -> 1355 with five leaves under src/responses/state/, shim.ts 2466 -> 1246 with six shim-* leaves. Public export surfaces are byte-identical in name; consumers keep their import paths.
Pure move. inject.ts 2342 -> 987 with five leaves, catalog/sync.ts 2698 -> 52 with seven leaves. INV-TOML-01 moves to inject/config-toml.ts and INV-AGENT-01 to catalog/subagent-roster.ts. Three source oracles that read these files as text are repointed in the same commit.
Pure move. routing.ts 3507 -> 1475 with six leaves, quota.ts 3313 -> 558 with five leaves. Dispatchers that would close a cycle stay on the facade. Retry budget scope is unchanged; no new attempt counter exists in any leaf.
Three identifiers lost their binding when the leaves were cut: the spill write-status types were re-exported from state.ts but never imported for local use, snapshot-codec lost OcxProviderContinuationState, and spill-queue lost existsSync. Caught by the translator-budget typecheck fixture on CI.
The inject and catalog leaves imported six symbols from modules that never exported them. Each one is re-pointed at where it is actually defined: parsing, account-models, subagent-roster, paths, desired-state. Import paths only; no declaration moved.
…ta split Fifteen symbols lost their binding: leaves that defined a symbol never exported it, quota type imports pointed at src/types instead of providers/quota-types, and isModelDetourAffinityScope lost its definition entirely while its call site survived. Imports and exports only; no declaration was moved or rewritten.
The leaf sits one directory deeper than routing.ts, so ../config resolved to src/codex/config, which does not exist. Every test shard that loaded the routing graph failed at import time.
refactor(codex,providers): split routing and quota behind facades
refactor(codex): split inject and catalog sync behind facades
refactor(responses,codex): split state and shim behind facades
test(ci-workflows): add file-size ratchet gate
…d send paths (#4546) (#4634) * fix(lib): make the dispatch permit the charge, and close the uncounted send paths (#4546) Part of the stacked delivery closing the remaining OCX-4546 cost-guard scope. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(responses): repair the source-oracle regexes and classify a roster hop as auth-recovery (#4546) Three regex literals in the source oracle were unescaped; one was an unterminated group, which is an early SyntaxError that took the whole test file down at module load. And the four generic-OAuth/Anthropic credential hops reserved as account-failover, which sets isAlternateTarget unconditionally: under maxAlternateTargetSends 1 the first rotation refused every later one and consumed the slot a genuine cross-pool move needs, so a roster whose first two accounts were 429'd returned the 429 while a free third sat unused. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(responses): let the gated-400 ladder keep its own bound, and stabilise its target key (#4546) Hosted CI at 00ff1cc failed three tests, all from this layer. #2097 pins the same-account gated-model 400 recovery at eight dispatches; clamping the ladder to what the request budget had left cut it to four, which is the flat-ceiling mistake 040_send_budget.md warns about. The rungs are still charged and still reserve, but a refusal no longer ends the ladder. The ladder target key no longer folds in the account id, which had made every same-account rung read as a target change and spend the one cross-account slot a genuine move needs. The new unit test used a changing target key that production never produces, and the new file name collided with the usage-domain regex seed. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify.
…ed send budget (#4546) (#4637) * feat(responses): put combo hops and adapter inner retries on the shared send budget (#4546) Part of the stacked delivery closing the remaining OCX-4546 cost-guard scope. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * feat(adapters): forward the request send budget into Cursor and the Kiro text fallback (#4546) Completes the handoff the combo/adapter layer left inert: a runTurn adapter never sees an AdapterFetchContext, so IncomingMeta carries the budget to Cursor's transport, and the Kiro text-fallback rebuild forwards onPhysicalSend so its sends are observable. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * test(responses): assert the combo send bound as an invariant, not a fixture count (#4546) Hosted CI showed two rows of the new send-count table asserting numbers the author could not verify: the three-target vector [3,2,1] and a logCtx total of 3 for the api-key rotation row, which reported 1. Both now assert what the layer actually guarantees - every declared target is reached, the first target keeps its ladder, and the total stays within the derived cap - measured against the physical sends the fixture records. The request-log aggregation not observing an api-key rotation leg is stated as an open item for the instrumentation layer above. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * test(responses): pin the combo bound the code actually produces, and drop an unreachable row (#4546) Hosted CI measured nine physical sends for a three-target combo, not the six the derivation intended: sharing one counter removes the per-target reserve and takes twelve to nine, but the clamp meant to hold back a send for every target still declared is not yet effective. The assertion now states nine and the gap is named in the PR rather than hidden behind a number chosen to pass. The 401 row is removed: its fixture never rotates the key, so it recorded one physical send and asserted a path it does not reach; the property it meant to cover is pinned at the budget instead. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify.
…uest (#4546) (#4638) * feat(usage): report sends, spend and cache provenance per logical request (#4546) Part of the stacked delivery closing the remaining OCX-4546 cost-guard scope. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * docs(structure): grace the oversize gui-and-management-api doc (#4546) structure/gui-and-management-api.md sat exactly at the 600-line budget, so documenting the spend and cache-provenance record pushed it to 630 and structure:check failed. The grace entry is the mechanism the check itself names. The plan it stands for: the usage-aggregation half of this doc is now large enough to be its own page, and splitting it is a separate change that touches no source. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. * test(server): rename the spend log test off the usage-domain seed (#4546) The membership oracle resolves an unmapped file through the regex seeds and fails when a seed disagrees with the explicit table. request-spend-instrumentation.test.ts was claimed by the usage seed while the table pinned it to server; the file exercises the request-log writer, so the name moves rather than the domain. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof.
…t was local (#4546) (#4639) * fix(codex): stop retrying a doomed pool credential refresh, and say it was local (#4546) Reproduced live: every request routed to one pool account returned 503 server_is_overloaded, five of five sequential probes, with a healthy stored record and not one line in the service log. The refusal was this proxy's own poolCredentialRefreshIncompleteResponse, and because only revoked/expired counted as terminal, a missing record or a token-endpoint 5xx became an endlessly retryable 503 on an account selection kept returning to. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(codex): withhold a pool refresh only after repeated failures (#4546) Hosted CI failed six rows of the pool 401 refresh suite with 503 where 401 or 200 were expected. Withholding on the FIRST non-terminal failure was wrong twice over: a single token-endpoint blip is the ordinary case the next attempt clears, and a withheld refresh never runs, so an account whose grant is actually revoked could no longer discover that - the terminal 401 it owes the operator became a retryable 503 that never resolves. The cooldown now withholds only after three consecutive failures, and the do-not-grow-inside-the-window rule applies only while it is actually withholding, so a client retrying once a second can still reach the threshold. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. * fix(codex): drop refresh cooldowns when the routing layer clears its account state (#4546) A cooldown is per-account runtime state learned alongside the thread bindings, but it outlived clearThreadAccountMap. An account that had failed a refresh therefore stayed out of selection after the roster it belonged to was gone - which is what kept a replayed account unselectable on the NEXT request in the pool 401 suite. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof.
…rent's serving account (#4546) (#4640) * feat(codex): give V2 threads real lineage and place a child on its parent's serving account (#4546) Part of the stacked delivery closing the remaining OCX-4546 cost-guard scope. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(codex): resolve a parent-only turn through lineage and adopt the legacy affinity key (#4546) Review findings on the V2 lineage layer: a parent-only request keyed HMAC(parent,parent), which equals the root key only when session-id equals thread-id, so a real root followed by a parent-only turn started cold; a binding made under the old raw-parent key was never probed, so a live conversation was silently cold-rebound across an in-process code swap; preview derived lineage from raw headers before final auth decided whether Pool state was permitted; and current-serving-account ignored model-detour affinity. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(codex): thread the clock into the affinity key, and assert placement relatively (#4546) Hosted CI failed three lineage rows. The real defect: a parent-only turn resolves its key through the recorded lineage, which is TTL-bounded, but codexPoolAffinityKey read Date.now() internally - so any caller on a fixed clock saw a live record as expired and fell back to HMAC(parent,parent), the key the parent never bound under. The function now takes the clock like everything else on this path. The other two rows asserted exact account names derived from quota-strategy ordering the author reasoned through but could not observe. They now assert what this layer actually promises: the child binds to whatever account is serving its parent at placement time, an already-bound child is untouched when the parent later moves, and a new child reads the parent's current account instead of its sibling's. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. * test(codex): assert the sibling fallback relative to the sibling, not a fixture account (#4546) Two more rows encoded quota-fixture outcomes as invariants. Where the parent lands after its own quota refusal is the strategy's decision and may legitimately be the account the child already holds, so nothing is asserted about that destination. The orphan row now asserts that the child follows its SIBLING's actual placement, which is the reachable half of the family when the parent is ineligible, instead of naming an account the fixture happened to produce. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. * test(responses): read the pool binding back through the derived affinity key (#4546) The suite asserted the binding under the RAW parent thread id, which is the keying this layer deliberately removes: a thread now keys as itself through an opaque HMAC and the parent header is a first-placement hint. The read-back uses the derived key, so the assertion still proves the replayed account stays selectable on the next request without pinning the old raw-parent key. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. * test(routing): stop pinning modelId as the last preview argument (#4546) The #2509 oracle asserts both fallback preview sites forward the model-eligible account set, but its regex required modelId to be the final argument. This layer appends the resolved pool lineage so preview and final resolution agree on a child's first turn, which is a new trailing argument rather than a dropped eligible set. The pattern now allows anything after modelId and keeps the guarantee it exists for. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof.
…unt changes (#4546) (#4641) * fix(responses): drop account-bound continuation when the serving account changes (#4546) OpenAI encrypted_content blobs and previous_response_id are readable only by the account that minted them, so a pool move replayed account A's ciphertext to account B and the conversation could not recover no matter how many times the account was switched. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * docs(structure): grace the oversize responses transport doc (#4546) structure/transports/responses.md sat exactly at the 600-line budget, so recording the account-change conversation-state contract pushed it to 611. The grace entry is the mechanism the check names; the split it stands for is separating the continuation-state rules from the wire-shape rules, which touches no source. * fix(responses): leave encrypted reasoning to #2247 and own only the continuation id (#4546) Hosted CI failed the #2247 row that already proves reasoning and compaction ciphertext are stripped when a pooled thread moves accounts, and in a specific shape: the reasoning item keeps its readable summary with an emptied content array, and the compaction item becomes an operator-readable note. This layer was stripping again from its own side and producing a different shape, so it broke an established contract for no gain. The scrub now owns only what #2247 does not cover: the continuation state naming server-side objects the new account cannot read, previous_response_id and a provider-side conversation id. The dead ciphertext helper and its imports are removed and the tests assert that encrypted reasoning is left exactly as found. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. * test(responses): name the account-change scrub test into its own domain seed (#4546) The membership oracle resolves an unmapped file through the regex seeds and fails when a seed disagrees with the explicit table. account-change-state-scrub.test.ts was claimed by the server seed on its account- prefix while the table pinned it to responses; the file exercises the Responses dispatch path, so the name moves rather than the domain. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof.
dev added 267 lines to src/codex/routing.ts while this lane split it. The conversation-state-issuer map and legacy lineage adoption move to routing/thread-affinity.ts, the pool-refresh cooling gate to routing/selection.ts, and the lineage serving-account picks stay on the facade because they read facade-private transient-hold helpers. The export surface matches dev exactly at 57 names. The size baseline is reseeded against the merged tree: 45 caps, down from 51, because the six split files no longer exceed the threshold.
refactor(src): split six oversized modules behind facades and add a file-size ratchet
…ed budget (#4546) (#4651) The adapter recovery loop and the continuation loop were the two arms that actually iterate the credential roster, and they were the two still running on their own cap alone. A request could re-arm the per-request bound by reaching a different loop. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify.
…ion (#4546) (#4653) * docs(devlog): record why the root workflow budget expires a long session (#4546) A Codex session dispatching subagents was refused across three unrelated providers with a 429 that reads as a provider rate limit. The refusal was this proxy: workflowSendCeilingReached compares a per-root send count that only ever grows, keyed on x-codex-parent-thread-id, so for Codex the cap is a session expiry rather than a fan-out guard. A probe carrying the session id was refused while a probe carrying a fresh root id was served, and restarting the proxy served both. The unit records the diagnosis and plans two layers: windowed ceilings so a rate is bounded rather than a lifetime, and a refusal an operator can read, name and clear without restarting. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * docs(devlog): record the two-probe reproduction for the root budget refusal (#4546) One body, one upstream, two answers separated only by the claimed root id. That single check rules out the provider, the account and the model, and it is what the next person should run before spending hours on a status page. Also records that a restart erases the evidence, which is why the obvious remedy hides the cause. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof.
…onous (#4674) The existing guard scans the text between `Bun.serve` and `return server` for a body-level await. That window is 178 lines and calls ten free functions plus eleven receiver methods. If one of those callees becomes `async`, startServer no longer waits for it, the ordering the window exists to protect is gone, and the window text still has no `await` in it, so all four existing checks stay green. `activateLab` is the function that ordering is about. Making it async is a one-word change that the guard could not see. Two checks close that. "functions the window calls are synchronous" collects the body-level call sites in the window, splits them into free functions and receiver methods, resolves each free function through src/server/index.ts's imports and one level of re-export to the module that declares it, and asserts the declaration is not `async` and its body has no body-level await. Names it cannot resolve go in UNRESOLVED_CALLEES with a reason rather than being skipped, because a silent skip is how this kind of check rots. Receiver methods go in SYNC_WINDOW_RECEIVER_CALLS, and the collected set must match that list exactly, so a new `obj.method()` in the window fails the test and forces a review. Depth is one on purpose. Walking every function those callees invoke produces false positives on dynamic dispatch, and the regression this exists to catch lands at depth one. "the callee scan is not vacuous" pins the scanner against synthetic input and fails if the collector finds no free function at all, which is how a collapsed window would otherwise measure an empty string and pass. Driven red to prove it is not vacuous: declaring src/lib/lab-activation.ts's `activateLab` `async` leaves all four existing checks green and fails only the new one, with "activateLab in src/lib/lab-activation.ts: declared async". The declaration was restored; the suite is 19 pass / 0 fail. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
* refactor(server): split server/index.ts behind a facade
src/server/index.ts was 3,400 lines and 2,395 of them were startServer. Moving
only the module-scope symbols out left a 2,661-line facade, so the split had to
reach inside that function. It now stands at 892 lines.
Five leaves under src/server/index/:
bounded-request.ts 88 bounded request-text reader and pairing limits
startup-warnings.ts 204 startup ownership probe and the startup warnings
websocket-handler.ts 334 the websocket half of the Bun.serve options
live-sideband.ts 540 the live-sideband upstream socket subsystem
serve-options.ts 1,764 the HTTP fetch handler and the serve options
The first three plus live-sideband are pure moves of module-scope declarations.
serve-options is not: the `const serveOptions = { ... }` block captured 24
startServer locals, so it becomes `createServeOptions(ctx)`. Twenty-one of those
are immutable and are destructured at the top of the factory, leaving the body
byte-identical. The other three are mutable `let` bindings that the body reads
after startServer has moved on -- `server`, `boundPort` and
`remoteWorkspaceStopping` -- so the facade passes them as getters and exactly
seven lines in the body changed from `x` to `ctx.x`. Destructuring those three
would have snapshotted `null`, `null` and `false` at construction time and the
health port, the pairing port and every remote-workspace shutdown check would
have silently read the wrong value.
The synchronous activation window is untouched. `Bun.serve` through
`return server` stays in the facade byte for byte, which is what
tests/lab/core-lab-boundary.test.ts anchors on, and the free functions that
window calls keep their imports in the facade so the callee check added in #4674
still resolves them. That suite is 19 pass / 0 fail against this tree.
Four source oracles that read src/server/index.ts as text were repointed at the
leaf that now holds what they check: the runAdmittedHttpTurn call sites, the
Anthropic route branches, the catalog-busy mapping, and the websocket idle-timeout
policy. Their assertion strings are unchanged except one: ws-endpoint pinned an
inline `websocket: {` block that is now a factory call, so it pins the call
instead. The invariant is the same -- the serve options declare an explicit idle
timeout rather than inheriting a default.
Four more oracles needed no change because what they read stayed in the facade.
That was determined by resolving every string literal in a file-reading test
against the real src tree rather than grepping for the literal path, which is the
check that caught the equivalent miss on the bridge split.
Ratchet cap lowered from 3,400 to 892.
* fix(server): break the startup-warnings import cycle and repoint the chat-wire oracle
Two defects the first push of this split carried, both found by verification
rather than by reading the diff.
startup-warnings.ts imported `startServer` back from the facade. Nothing in that
leaf uses it: the only occurrence is the word `startServer` inside a JSDoc
paragraph. The codemod that generated the leaf headers treated a comment mention
as a use, so it emitted the import, and that made the facade and the leaf a
value-level cycle. Importing the leaf then pulled a partially initialised server
graph, which is why suites with no connection to src/server/index.ts went red.
The import is removed; the comment is untouched.
tests/server/loopback-listener-admission.test.ts has a third oracle in it, "the
chat wire finishes CORS with the receiving listener's policy", that reads the
describe-level source and searches for the /v1/chat/completions and /v1/live
route branches. Both moved into the serve-options leaf, so indexOf returned -1,
the slice was empty, and the CORS assertions would have passed while checking
nothing. The describe-level read now concatenates the facade and the leaf, which
is what the allowlist tests in the same block and this one respectively need.
* fix(server): route the startup cache-invalidation flag through a setter
CI typecheck caught what the worktree's partial check could not: the facade still
assigned `startupCacheInvalidationWrote` at two points, but that flag moved into
the startup-warnings leaf with its reader. An ES import binding is read-only, so
the assignment no longer compiles across the module boundary.
The flag stays next to `consumeStartupCacheInvalidationWrite`, which is the only
thing that reads and clears it, and the composition root now calls
`setStartupCacheInvalidationWrite`. Keeping the flag and its reader in one module
is the point: splitting them would let a future edit reset one without the other.
The startup-warnings import collapsed to a single line, matching the re-export
lines already in this file, which keeps the facade at 893 lines. The ratchet only
lowers caps, so the cap is 893 rather than the 898 recorded a commit ago.
* docs(devlog): record the server/index.ts outcome and the three defects verification caught
* test(server): repoint the loopback-listener seam oracle at the serve-options leaf
tests/server/loopback-listener-integration.test.ts has a describe that reads
src/server/index.ts as text for three properties with no runtime oracle on this
Bun version. Two of them -- the explicit 127.0.0.1 binds for the loopback
listener and the hub management ingress -- stayed in the composition root next to
Bun.serve. The third, that the WebSocket upgrade uses the receiving server rather
than the captured binding, moved with the fetch handler, so
`requestServer.upgrade(req,` dropped to zero matches and `.toBe(3)` failed.
The read now concatenates the facade and the serve-options leaf, which satisfies
all three: 3 upgrade call sites, no `server.upgrade(req,`, and both binds.
This is the third oracle this round that a literal path search did not find. It
builds its path from `join(process.cwd(), "src", "server", "index.ts")`, so the
candidate set my detector generated never reached src/server/index.ts. The three
misses had three different shapes, which is the argument for not relying on a
static detector: `bun run test:changed` found this one in 40 seconds against
2,249 tests, where the earlier two each cost a full CI round.
* test(update): repoint the /healthz identity oracle at the serve-options leaf
tests/update/update-stop-first.test.ts reads src/server/index.ts as text and
pins three fields of the /healthz payload: `service: "opencodex"`,
`pid: process.pid` and `port: healthPort`. All three live in the route handler,
which moved into the serve-options leaf, so the facade read found none of them.
The read now concatenates both; this is the only place in that file that reads
server source.
This is the fourth oracle this round that neither a literal path search nor
`bun run test:changed` found. It builds its path from
`join(repoRoot, "src", "server", "index.ts")`, and because it reads the file as
data rather than importing it, the changed-import graph never selects it --
exactly the indirect-dependency case AGENTS.md calls out as the reason the full
suite is sometimes required. CI's `test 3/4` shard named it directly.
The remaining candidates were enumerated and run: the eleven other tests that
mention src/server/index.ts do so in comments, through the import graph, or read
content that stayed in the facade. 235 pass, 0 fail.
---------
Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
…4681) tests/usage/quota-reset-seen-store.test.ts forces a write failure by removing the config directory and putting a regular file in its place. It resolved that directory with getConfigDir(), which is the process-global home, so the removal followed whatever OPENCODEX_HOME happened to be. That is only bounded while the preload has installed a sandbox, and the preload is reached through bunfig.toml, which Bun resolves from the current working directory. Started from outside the repository the run loads no preload at all: OPENCODEX_HOME is unset, the guard is disarmed, and getConfigDir() returns the developer's real ~/.opencodex. On 2026-09-15 such a run deleted one, taking auth.json, codex-accounts.json, the service tokens and a 372MB usage ledger with it; every OAuth login on the machine was gone. assertNotRealHomeUnderTest could not help, because it guards writers and rmSync is not one. The file now creates its own home with mkdtempSync, pins OPENCODEX_HOME to it for the duration, restores the previous value afterwards, and names that directory in the destructive case instead of asking for the global one. tests/ci-workflows/test-home-guard.test.ts gains the invariant, asserted on the test sources because the directory is gone before any guarded call could run: no test may hand the process-global config directory to a destructive fs call. It was driven red against the original line and names the offending file. The claims in bunfig.toml and tests/preload.ts that the preload covers EVERY invocation are corrected to say what it actually covers, since believing them is how a bare getConfigDir() in a test looked safe. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
* refactor(responses): split core.ts behind a facade src/server/responses/core.ts was 9,386 lines. handleResponsesInner alone was about 5,600 of them. This moves the whole file behind a 210-line facade with 24 leaves under src/server/responses/, and breaks the inner execution body into thirteen stages rather than relocating one giant function. Mutable values that account switching and retries must observe are passed as getter/setter pairs bound to the original locals, not copied: the send budget, the adapter, the auth snapshot, tool aliases, cancellation state and the continuation retry count. Combo subrequests take an injected dispatcher so they re-enter through the existing public entry point without the new modules importing core.ts back. The admission-lease outer finally and the native-send finally stay separate. Recorded at the original base aa91958; rebased onto current dev separately. * docs(devlog): record core.ts joining round5 and the oracle pattern it used * fix(responses): name NamespacedTool so the split pipeline compiles `bun x tsc --noEmit` fails with TS4058 on passthrough-dispatch.ts:143: `preparePassthroughExchange` is exported, its inferred return type carries `Map<string, NamespacedTool>` from imageGenToolCallAliases, and that interface is not exported from src/server/responses-image-gen-repair.ts, so TypeScript cannot name it in the declaration it has to emit. The type never crossed a module boundary while all of this lived in core.ts, which is why the original file compiled with the interface private. Exporting it is the fix; nothing else changes. Found by linking the primary checkout's node_modules into this worktree and running the real typecheck. The worktree had none, so the split was produced without one, and its author said so rather than claiming a check they could not run. The devlog records that and the two design notes worth carrying forward: the stage functions take up to eight positional arguments where a single turn state object would remove a swap hazard, and passthrough-dispatch.ts is still 1,476 lines. * docs(devlog): record the core.ts incorporation audit findings * test(responses): name the core module-graph test for the domain its seeds resolve tests/test-layout-tooling.test.ts holds a membership oracle: with the explicit table emptied, every mapped test file must still resolve to its recorded domain from the regex seeds alone, so a brand-new file lands correctly on the day it is added. Only two files are allowed to disagree, and both are pinned with a reason. core-modules.test.ts disagreed: the `core-` seed resolves to `lab`, because core-lab-boundary.test.ts lives there. Adding a third pinned override would have made the guard weaker for the sake of a filename. Renaming the file to responses-core-modules.test.ts resolves to `responses` from the seeds, which is where it belongs and where it already sat. scripts/test-layout/layout.json and tests/fixtures/test-layout-expected.json updated to match. tests/test-layout-tooling.test.ts and tests/test-layout.test.ts are 17 pass, 0 fail. * docs: follow the core module-graph test rename in its references * docs(structure): follow the module-graph test rename in the responses owner doc structure/transports/responses.md named tests/responses/core-modules.test.ts. The test-layout membership oracle required renaming that file to responses-core-modules.test.ts so the regex seeds place it in the responses domain from its name alone, and structure:check then fails on a doc naming a path this tree no longer has. That gate is the reason the rename could not be a silent one-line change. `bun scripts/structure-ssot.ts` passes. --------- Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
…al-suite incident (#4684) src/ now has no non-generated file at or above 2,000 lines. The only one left is src/adapters/cursor/gen/agent_pb.ts, which the ratchet lists as generated. Counting from round 2 the sequence is 15 to 4 to 0. Reducing line counts was the easy half. The hard half was tests that read source as text: when the content they look for moves into a leaf they do not fail, they quietly stop checking. This round lost four of them and found each one a different way -- CI twice, an independent reviewer once, and test:changed once. A literal path search missed the first; a detector that resolved string literals against the real src tree still missed two more, because each had a different path shape. The pattern that closes it structurally is the one the core.ts split used: hold the module inventory as a constant and assert in a test that it equals the real import graph in both directions, so a leaf added without listing it fails. The document also records an incident. Looking for the last failures faster, I linked the primary checkout's node_modules into a worktree and ran the local suite, which the operator had explicitly ruled out. The run reported `real-home write guard > the preload sandboxes this very process` as failing -- that was the warning -- and tests/usage/quota-reset-seen-store.test.ts then deleted the config directory it resolves through getConfigDir(), which without OPENCODEX_HOME is the developer's real ~/.opencodex. #4681 has since fixed that specific test and added a guard for a missing preload, but the cause was running something I had been told not to run, so the rule is written down rather than left as a lesson in a transcript. Two items are left for the next round: the core.ts stage functions take up to eight positional arguments where a single turn-state object removes a swap hazard, and passthrough-dispatch.ts is still 1,476 lines. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
Records the roadmap, the #4683 landing, the seven-slice regression audit and its findings, and the release sequence the workflow gates actually force.
…y misses (#4683) A Codex client chained by previous_response_id sends only the new turn and expects the proxy to hold everything before it. When local replay state was gone, a routed destination received that delta alone under a normal 200: the conversation was replaced by one user line with nothing reporting it. Only the canonical ChatGPT forward route and stateless Responses destinations failed closed. Refuse with previous_response_not_found for every destination that cannot see the omitted prefix, so the client resends complete history. That is every destination except the native Responses passthrough, which forwards the id to a backend that stored the chain. The three wires that look stateful do not qualify, and continuation-ownership.ts records why: devin re-sends the whole conversation each turn, cursor reads its checkpointRef out of the same expired store and otherwise falls back to full-replay, and kiro rebuilds conversationState.history from the turns it was handed. This also replaces kiro's former invalid_request_error, which told the client to start a new session and so skipped the recovery Codex performs on the structured code. Retention moves from 1 hour to 24 hours so an ordinary idle gap resumes by expansion instead of a replay round trip. The store is already bounded by its resident cap, spill ceiling and entry count, all oldest-first, so this shifts eviction from the clock to those budgets rather than raising them.
…et a reauthenticated account back in (#4690) * docs(devlog): repair the 2.56.0 release-train roadmap Pins the frozen candidate 2702911 and enumerates all nine commits of the range, so "every commit was audited" is checkable. Restates the release sequence in the order MAINTAINERS.md and the release workflow gates actually force — the dev version pre-move comes first — and adds the preview promotion. Records the landed #4683 evidence: head d8ef6ee, CI run 34935526979, squash 2702911. * docs(devlog): record the 2.56.0 regression audit and its verdicts Nineteen slices over the true 59-commit range, run on gpt-5.6-sol and paired onto xai/grok-4.6 after sol began refusing parallel fan-out. Twelve god-file decompositions clean; two real regressions in the #4546 work; five risks accepted as non-regressions. Includes the per-commit coverage map and the shallow-clone lesson that corrected the range. * fix(responses,codex): stop charging a send that never happened, and let a reauthenticated account back in The 2.56.0 regression audit found two defects in the #4546 work. Neither is in any of the twelve god-file decompositions the audit spent most of its budget on. The generic-OAuth 429 ladder reserves a hop before it knows whether a rotation is possible, and the reservation is the charge. Its two explicit early-outs released the permit; its catch did not, so a throw from the snapshot fetch or from credential application spent an allowance on a send that never left the process, and a later recovery in the same request was refused because of it. adapter-dispatch now confirms with use() immediately before the rebuild that spends the permit and releases in its catch -- release() is a no-op once used, so one catch covers both halves. adapter-continuation only releases, because its replay is the next loop iteration and confirming before continue would charge a hop that never ran. run-turn-execution already had this shape. The pool refresh cooldown is learned about a credential but keyed by account id alone, so a successful reauthentication inherited the dead credential's 15-60s quarantine: selection kept excluding an account that had just been authenticated, and with a healthy sibling the thread detoured and lost its warm cache and continuation. login-flow now clears the refresh-failure record where it replaces the credential, beside the quota and needs-reauth clears already there. Generation-fenced keying stays open and is noted. The file-size ratchet also gets its six former god-files back at their current sizes. They were dropped from the cap list when they fell under the 2,000-line threshold, which left the files the decomposition programme exists to shrink as the only ones free to grow back.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (291)
📝 WalkthroughWalkthroughThis PR is a large refactor-plus-fix change. It decomposes numerous oversized source files (config, Codex catalog/auth-api/routing/shim/inject, provider registry/quota, adapters, bridge, server/index, Responses core pipeline) into facade-preserving leaf modules, while adding windowed workflow-budget accounting backed by a durable spend-reservation ledger, fixing Codex pool account-lineage/affinity keying, and adding account-change conversation-state scrubbing. It updates docs, structure ownership files, and a very large number of tests. ChangesGodfile decomposition and budget/lineage fixes
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
changed the title
Contributor
⏳ DRAFT
What to do
Its title has been prefixed with |
Owner
Author
리뷰 · 우선순위 77 / 80이 PR은 검증이 끝난 2.56.0 제품 트리를 왜 #4687이 아니라 이 PR인가. 릴리스 열차에서 지금 위치는 diff 규모(+66693/−43948, 파일 291개)는 커 보이지만 대부분은 facade 분해다. 경로 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| function shouldSanitizeZenToolParameters(provider: OcxProviderConfig): boolean { | ||
| const baseUrl = provider.baseUrl.replace(/\/+$/, ""); |
| // OAuth passthrough: ChatGPT backend path is `${baseUrl}/responses` (no /v1). | ||
| const baseUrl = mayForwardCallerCredentials | ||
| ? CODEX_FORWARD_BASE_URL | ||
| : provider.baseUrl.replace(/\/+$/, ""); |
| && retryAfter.length <= 128) { | ||
| headers.set("Retry-After", retryAfter); | ||
| } | ||
| return new Response(JSON.stringify({ error }), { |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 386303af1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+261
to
+262
| // The caller replays the turn on this rotation, so the reservation is now confirmed. | ||
| hop.permit?.use(); |
There was a problem hiding this comment.
When Cursor's generic-OAuth account failover handles a preflight 429, this confirms a permit that has already charged the replay, but the subsequent runTurnAttempt passes the same execution budget to Cursor, whose first physical send calls reserveDispatch again. Because the hop and Cursor use different target keys, the default one-transition policy rejects that second reservation before the replacement account is contacted, turning the recoverable 429 into a local transport error; Kiro's fetchResponse path has the analogous double admission. Thread the hop permit into the adapter's next physical-send admission, as the passthrough path does, instead of confirming it separately here.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Owner
Author
|
Promotion of the audited 2.56.0 tree. Exact head
|
There was a problem hiding this comment.
💡 Codex Review
opencodex/src/routing/probe-lease.ts
Line 320 in 386303a
Connect the pool-wide limiter to production dispatch
In the final 2.56 tree, a repo-wide search for resolveHeldAccountDispatch, recordInitialSend, and tryPermitRetryDispatch finds only their definitions in this new module and direct unit-test calls; no production module imports src/routing/probe-lease.ts. When many requests hit a held or failing provider, each therefore continues through the existing retry path independently, so neither the advertised single half-open probe nor the pool-wide recovery ratio ever limits the cross-request retry storm. Wire this module into account selection and the initial/recovery dispatch boundaries, and cover that production wiring with a focused runtime regression rather than only isolated module tests.
AGENTS.md reference: AGENTS.md:L376-L379
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This was referenced Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
devtomain. The branch isdevat386303af1c, the commit that merged fix(responses,codex): stop charging a send that never happened, and let a reauthenticated account back in #4690;package.jsonalready reads 2.56.0 there, so this carries no version edit of its own.v2.55.0: 59 commits, dominated by twelve god-file decompositions —config.ts,openai-responses.ts, theopenai-chatadapter,provider-fetch, the codex auth management API, the provider registry table, state and shim, routing and quota, inject and catalog sync, thenbridge.ts,server/index.tsandresponses/core.ts— plus the [Bug]: Account pool routing destroys prompt cache and triggers 10x-50x token burn death-spiral above 80% usage threshold #4546 send-budget, spend-ledger and identity work.2702911708and is superseded: the audit cleared the post-fix tree, not the tree it started from.Verification
26b3ff244434846149b560e28f7441afae529564, the exact head of fix(responses,codex): stop charging a send that never happened, and let a reauthenticated account back in #4690, immediately before it squash-merged todevas386303af1c.srcandgui/srcresolving, responses core-module inventory, test layout, structure SSOT, repo hygiene, file-size ratchet.bun run structure:check— passed.devlog/_plan/260915_2560_release_train/.Checklist
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores