Conversation
The audit drove the first version of these tests red on the parent branch: all three passed with the production change reverted, because they only re-asserted what resetCodexRoutingForManualSelection and the deliberately exempt failover promote already did. Three defects fall out of that. Deletion reaches routing through clearCodexUpstreamHealthForAccount, which did not revoke the preference, so a preference could outlive its account and suppress every later write. The generation sweep had the same hole. The model-detour promote wrote over the operator's selection while preemption next to it yielded. The independent-scope preference entries were written and consumed but never read by any guard. Co-authored-by: Heisenberg <audit@localhost>
Guarding it failed 8 cases in codex-routing.test.ts. A model detour runs because the operator account cannot serve the model at all, and under a rotating strategy the promote moves only the process-local cursor, never the persisted selection.
reconcileCodexRoutingHealth had no test at all, so the preference sweep added for the delete-path blocker was verified by reading rather than by running. Both halves are covered now: an account the generation no longer lists loses its preference, and one that is still listed keeps it. Red control: removing the four sweep lines makes the first case read undefined.
…ards clearCodexUpstreamHealth dropped every health map but kept lastReconciledGeneration and liveHealthAccountIds, so after a full reset recordCodexUpstreamOutcome would still discard a writer whose generation predated the watermark for any account missing from the equally stale live set. Found because the watermark leaked between test files: the new generation-sweep coverage failed two unrelated cases in codex-routing.test.ts when both files ran in one process.
feat(providers): pick a warm API key before the first attempt
refactor(oauth): move the pool rotation kernel out of the Codex namespace
…-followup docs(structure): close the review findings on the SSOT gate
…it-boundary codex-ws: settle post-send failures as honest gateway statuses and replace the fixed prelude deadline with liveness
…on-wins fix(codex): let an operator account selection outrank the pool cursor
…rnel round-robin and fill-first now actually select an account for a generic OAuth provider, on both the initial-preference and the 429 path. Both quota guards are skipped for them deliberately: hasHeadroomEvidence refuses every provider with no quota data, which is exactly where round-robin is the point, and the healthy-active early return fires before autoSwitchThreshold can be read. quota, and the flag off, keep the pre-kernel path unchanged. The live round-robin pick commits at admission rather than at proposal, matching commitAnthropicSelectionRouting: peek never creates the pool state and notePoolRotationSuccess no-ops without it, so a peek-only path would never turn the ring.
The inert contract was published in the English reference and in seven translated locales, all of which said a generic strategy and threshold never steer selection. That is now true only with pool.kernel off, so each page describes both states rather than the old one.
The deadline now starts before credential selection, so a client that leaves during that wait must be reported as a hangup with nothing dispatched and the admitted turn released.
…dex key Security review found the relay unreachable in its only supported configuration: the default bind is loopback, admission there never reads a token, and the injector only rewrites the base URL for loopback hosts, so every history call returned 403 even with a valid key. The relay now asks the identity question separately from admission - a context request presenting a real opencodex key owns its sessions, one presenting none is still refused - and a regression exercises the real resolver instead of a hand-built admission.
…wnership Security re-audit found the previous fix one-sided: the relay could name a loopback caller but the model turn still recorded ownership from admission alone, so a correctly named caller found no entry and got 409. Both paths now use resolveContextPrincipal. The guides and structure doc now state what actually works - a remote bind with a configured key, or a client that sends x-opencodex-api-key - because the built-in loopback injection cannot carry that header, and whether a loopback bind should identify callers is left as a maintainer decision rather than a relaxed refusal.
…ental opt-in The opt-in only guarded the injected base URL, so the ten endpoints and the ownership registry stayed live for any caller able to reach the data plane, including one posting the paths directly. contextRelayActivated reads Codex own config and now guards the relay handler and the recording path as well; absent, unreadable or malformed config is not an opt-in, and the decision is cached against config identity so turning it off applies without a restart. Regressions assert 404 on both prefixes, no owner recorded and zero upstream calls while it is off.
Resolve the Codex home inside the try rather than in a default parameter, since those evaluate before the body and this gate now runs during model turns, where an unreadable CODEX_HOME would abort a turn upstream already served. Gate the route match as well so a disabled feature takes no turn lease. Pin cache invalidation without the reset seam and the unreadable-home refusal, and restore the Korean disable paragraph.
Add the lidge-jun#3433 transport matrix with actual Chat handler and captured canonical Responses request. Preserve key and original header spellings; no identity synthesis. Synthetic fixtures do not establish Hermes emission or cache-hit improvement. Local suites NOT RUN; hosted CI pending.
…cache-hermes test(chat): verify native outbound conversation identity (lidge-jun#3433)
The fallback-port fixture records a port from allocateFreePort, which reports the port it has already released. On a four-shard runner every other test binding an ephemeral port is a candidate to take it, and when that happens status finds a listener on the recorded port, reports the record as live, and the assertion fails against something the test never set up. It failed exactly that way on the preview promotion run while the same commit passed on dev. Confirm the recorded port refuses immediately before and immediately after the status probe, and re-allocate when something took it in between. The assertion is unchanged and no weaker: a run only counts when the endpoint demonstrably refused across the whole probe, and exhausting the attempts fails with that reason rather than silently passing.
…relay feat(codex): relay experimental context history under a least-privilege owner
…53.0 chore(release): open dev at 2.53.0 before releasing 2.52.0
…port-flake test(cli): stop judging a stale record on a port another test can take
Carry lidge-jun#4124 with unchanged pin/cap/mapping policy and regression coverage. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
…models-effort-caps fix(chat): enforce effort ceilings without model pins
…he tool-catalog nudge (lidge-jun#4415) * refactor(devin): retire the ACP adapter and give the shared adapter the tool-catalog nudge Both Devin provider rows already stream Cognition's Connect-RPC api-server on the `devin` adapter. They differ only in where the credential came from: a browser sign-in through RegisterUser, or the `devin-session-token` the installed CLI already wrote to its own credentials.toml. A second adapter registered under the id `devin-cli` still spawned `devin acp` and drove the child over Agent Client Protocol on stdio. Nothing routed to it: `routedProviderConfig` pins the adapter from the registry for any registry id, so only a custom-named row such as `"devin-acp"` could select it. It is removed rather than kept, because the premise that justified it was false. The CLI's credential is the ordinary cloud token, so importing it does everything the child did without a placeholder `buildRequest`, a disabled `parseStream`, an identity-only `baseUrl` no request may connect to, and a subprocess running in the operator's own tree. `projectDevinCliAuthMode` used to warn and change nothing when a saved row still named that adapter, reasoning that routing already pinned the transport. That held only for the registry id. With the adapter gone a custom-named row has nothing pinning it and would throw `Unknown adapter: devin-cli` on every request, so the migration now rewrites every row naming the retired id whatever the row is called, and repoints a row still carrying the identity-only `cli.devin.ai` host at the api-server in the same pass. The nudge is the other half. Every non-OpenAI adapter that advertises a client tool catalog injects `buildNonOpenAIToolCatalogNudgeForTools` into its system prompt; Devin advertises a real catalog on proto field #10 and was the only one without the paragraph. It goes into `mapOcxMessagesToDevin`, which covers both provider rows at once. The wire-name callback is `tool => tool.name` rather than the default namespaced form, because `mapOcxToolsToDevin` writes the bare name — a nudge listing names the model is never offered is worse than none. The ACP wire could never have carried it: `session/prompt` takes prompt text only, with `capabilities: {}` and `mcpServers: []`. * fix(devin): share the api-server default and drop the out-of-scope GUI comment The migration wrote its own copy of the Cognition host, so a later change to the shared default would have left it writing the old address. It now imports DEVIN_DEFAULT_API_SERVER. The GUI comment rewrite went back to its dev state. It was unrelated cleanup by the PR checklist's own standard, and because the screenshot gate is path-based it was asking a comment-only diff to produce a screenshot of nothing. It can go in on its own.
…ion (lidge-jun#4411) * fix(codex): refresh the catalog when paginated history refuses injection An explicit `ocx sync` is also the refresh path for side profiles that consume the OpenCodex catalog without injection. Since paginated Codex rollouts began refusing external writes, that refusal was reaching `syncModelsToCodex` through the validate-only preflight and failing the whole sync, so the model catalog went stale on a home whose history simply requires its native writer. The preflight refusal now carries a structured `historyPreflightFailureReason` instead of only display text. When an explicit sync sees `history_paginated_requires_native_writer`, it keeps the injector's refusal intact, publishes through the existing catalog owner, and returns `catalog-only` with config and conversation files untouched. Unattended sync and every other config or integrity refusal keep their hard failure. `refreshOutcome` distinguishes a validated catalog commit from a refused refresh, so `refreshCodexModelCatalog` no longer rewrites the models cache after a refusal and `ocx sync` exits non-zero when a catalog-only refresh did not complete. * docs(devlog): record the 2.43/2.46 releases and the open closeout units Carries the planning and delivery records that were still sitting untracked in the working tree: the 2.43 and 2.46 release units, the per-work-phase execution and delivery records for the 260905 open-work closeout, the provider runtime stack unit, the 249 bulk closeout, and the beginner PDF plan. The 006 dispositions and 060 ledger updates record two maintainer decisions made during that campaign: `bun run test:changed` was removed from the local verifier set because its import-graph selection reaches most of the suite on the touched layers, and from wp4 onward the campaign accepted the final `dev` tip CI run as batch evidence instead of per-PR exact-head runs. * test(codex): cover the catalog-only path for a paginated-history refusal Three cases the change has to keep apart: an explicit sync refreshes the catalog and reports `catalog-only` when the injector refuses on `history_paginated_requires_native_writer`; a refused refresh under the same condition stays unsuccessful and writes no cache; and an unattended sync keeps the hard failure it always had. * fix(devlog): drop a maintainer email address from the wp4 reverify record `privacy:scan` flags any address outside its allowlist, and the carried closeout record quoted the `-c user.email` value used to re-author two layers. The address was never the point of the note, so it is replaced with a description of the flag pair. This is what failed `test 3/4`, `gates`, and `macos 2/2` on the previous head. * docs(structure): record the catalog-only exception and declare refreshOutcome Two residuals a delegated review found on this PR. `structure/config.md` owns the sync preflight contract and still said every deterministic refusal leaves the catalog and cache untouched, which this change makes untrue for one reason code. And `refreshOutcome` reached callers through a spread without ever being declared on `CodexSyncResult`.
Both work phases landed on dev (lidge-jun#4411 as 2d3c05f, lidge-jun#4415 as 213065e), so the unit moves to _fin with an outcome record. It records what the removal actually corrected: the ACP adapter was written on the untested assumption that OpenCodex could not hold the installed CLI's credential, then kept on a second assumption that leaving it registered was harmless because routing pinned the registry id away from it. That was true for the registry id and false for the custom-named row the documentation itself recommended, which is the row that would have broken on removal. It also corrects two beliefs this unit started with: devin-cli has not run over ACP since the account-login unit, and its token usage is reported, not missing. What is genuinely absent is account quota, which Cognition exposes only through Enterprise-scoped endpoints.
…nd cache defects (lidge-jun#4418) * docs(devlog): plan Devin hardening and the cached-token companion Locks the roadmap for the devin-cli token transition, the cloud-direct transport and usage decode, and the cached-token display companion, before any implementation lands. * docs(devlog): fold the roadmap audit findings into the Devin hardening plan An independent review of the first draft failed it on three counts. Field 7 needs its own uint64 decoder and must suppress field 28 within a message, because both arrive together and the adapter last-writes usage. The input-versus-cache mapping is derived from the frame rather than assuming Cognition is exclusive, since guessing wrong inflates input and bills cache at the uncached rate. And missing_ui_screenshot is path-based on gui/src, so the display phase carries a real screenshot instead of avoiding the word. * fix(devin-cli): harden the imported CLI session against path, read, and cache defects Five defects on the credential-import path. An empty APPDATA or XDG_DATA_HOME resolved to a cwd-relative credentials path, because `??` treats an empty string as a set value; a file planted beside the proxy would have imported as the operator's own CLI session. Every read failure collapsed into `undefined`, so a permission error on an existing file was reported as 'not signed in' and sent the operator to `devin auth login`, which does not fix it. The outcome is now missing, unreadable, incomplete or ok, each with its own message, and the parse is bounded at 64 KiB. A bare JWT in Metadata.api_key went out without the `devin-session-token$` prefix and came back as an opaque permission_denied, which reads as a revoked account rather than a malformed credential. Only a three-segment JWT is reshaped, so a Codeium UUID, an sk-ws key and a cog_ key pass through untouched. Logout cleared the shared user-JWT and catalog cache only for provider `devin`, and account deletion never cleared it, so a CLI-imported key's JWT outlived its own logout. Both ids now clear on both paths. redactSecretString recognised neither a Devin session token nor a bare JWT, and a Connect trailer can quote the request that carried one.
…un#4421) A cached request's total is mostly cache. A 58,000-token prompt that is 57,000 cache read and 1,000 fresh was printed as a bare 58,000 on every surface except the logs table row, so it read as a different, smaller request than the row directly beside it. formatTokensWithCache renders the total with its cached companion — 5.8만 c5.7만, 58K c57K — reusing the marker the logs.tokens.cacheRead label already documents as "cache read (c)", so no new i18n key is needed. A provider that reports no cache is untouched, and a turn served entirely from cache still shows the marker, since that is the row worth seeing. No backend change: /api/logs forwards the whole usage object and /api/usage already emits cache on summary, models and providers. The loss was purely client-side, in the row types and the aggregators, so the fix widens UsageModel, UsageProvider, UsageSummary30d, the CLI CostRow, and summarizeFilteredLogs to keep the fields that were already arriving. The log detail panel is deliberately left alone: it already has separate cache read and cache write cells, and stacking the companion onto its total would duplicate them.
…es (lidge-jun#4419) * fix(devin): read usage from ModelUsageStats and classify cloud failures A cached Devin turn reported a bare token total with no cached subset, so its log row looked like a smaller request than it was. The decoder was reading GetChatMessageResponse field 28. Field 28 is response_dimension_groups, the rows the IDE renders; field 7 is ModelUsageStats, the per-turn accounting. The old path worked by accident: ResponseDimension.uid is that message's field 5, which the entry walker treats as a metric id, so cache numbers appeared only when the service happened to render cache rows. Field 7 carries cache read and cache write unconditionally. Both fields arrive in the same message and the adapter keeps the last usage event, so decoding both is not enough: field 7 now suppresses field 28 within a message and is yielded last, and it needs its own uint64 decoder because the field-28 walker reads a fixed32 float out of a sub-message. Whether Cognition's input_tokens already includes cache is unsettled, and guessing inclusive is the expensive error: normalizeCostTokens only rejects read + write > input, so an inflated input passes validation and bills cached tokens at the uncached rate. The mapping is therefore derived from the frame. Both branches agree on the 58k-prompt case that prompted this. Two further classification defects. CloudChatError carried no HTTP status, so inferHttpStatusFromAdapterMessage turned an upstream 429 into a 502 and core's failover never rotated or backed off. And a cancelled turn said "Devin turn was aborted.", which isClientClosedMessage does not recognise, so a client hanging up was logged as an upstream failure; it now emits the phrase the classifier knows, with status 499. Usage frames are merged per field instead of replaced, because the counters are cumulative and a later partial frame used to zero an earlier count. * fix(devin): report field-7 usage ahead of finish and derive the merged total Review follow-ups on the usage decode. The authoritative ModelUsageStats event was yielded after the rest of the frame, so a frame that also carried finish reported usage behind the turn's end. It is now yielded first, which makes the order independent of where the service places the field. mergeDevinUsage took the max of two totals alongside the per-field maxima, which can leave totalTokens different from input + output; the cost and log paths read that total. The total is now derived from the merged counts. Regression coverage for what the change is actually for: field 7 suppressing the display rows within one frame and landing before finish, the display rows still decoding when no field 7 is present, a partial frame not zeroing an earlier count, and an HTTP status becoming a structured classification. A Connect trailer still carries no HTTP status, so a cap delivered that way keeps the older message-inference path. That is noted at the throw site as a follow-up rather than silently left open.
…idge-jun#4423) * fix(devin): map Connect trailer codes onto the status core acts on Only the HTTP status line carried a status, so a cap or an expired credential delivered as a Connect EOS trailer fell through to inferHttpStatusFromAdapterMessage and became a generic 502 — not an auth prompt, not a backoff, and nothing core's failover acts on. connectTrailerHttpStatus maps the Connect codes Cognition actually sends, and treats permission_denied carrying "your limit will reset" or "reached overall message rate limit" as the quota refusal it is rather than an authorization failure. It reads the raw trailer message, not the enriched text, so the tool-blocklist wrapper cannot trip the quota regex. An unrecognised code returns undefined and keeps the older inference path. Review follow-up in the same change: unimplemented maps to 501, and the blanket "5xx is retryable" rule was putting retryable: true on the SSE failure a client reads for a call the service will never implement. * docs(devlog): record the trailer-status mapping as closed 020 listed the Connect trailer mapping as the deferred half of the cloud-direct work. It landed, along with the 501 retryability fix the review caught, so the doc records the outcome and the two accepted residuals.
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
3 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.
Temporary sync PR to merge upstream dev 92e21bc into feat/usage-ledger-retention-v2. Do not merge upstream PR lidge-jun#4042 itself.