Repository navigation
fix: repair three regressions from the #2463/#1478 landings - #2614
Merged
Merged
Conversation
Found by the full suite on dev HEAD (36 failures across four files), not by any of the three PRs' own focused runs - each was green in isolation. 1. rebase-provenance: structuredClone on the whole config threw DataCloneError whenever a provider carried a non-cloneable value. A test fixture injects its own fetch, and a function is not cloneable, which took down every provider probe and both CLI parity suites. Only the top-level provenance key is rewritten in that projection, so nothing below the top level needed copying at all - a shallow copy is both correct and what the function actually meant. 2. /api/providers/:name/alias swallowed /api/providers/keys/alias. Model routes dispatch before oauth-account routes, so the new alias route matched name='keys', found no such provider, and 404'd every API-key-pool rename. Guarded rather than reordered: the dispatch order is load-bearing elsewhere. 3. The alias command and its two endpoints were missing from the help banner and the GUI/CLI parity map. Both sweeps exist to catch exactly this, and both did. Each repair verified against the failing test, and the full local set of affected suites is green: provider-api-keys, provider-connection-test, config-user-edits, config-rebase-provenance-writers, alias-management-api, cli-headless-parity, cli-registry.
Contributor
|
✅ Deterministic PR hygiene checks passed. |
![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: 1690787231
ℹ️ 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".
| const modelAliasMatch = url.pathname.match(/^\/api\/providers\/([^/]+)\/model-aliases$/); | ||
| if (modelAliasMatch && req.method === "PUT") { | ||
| const name = decodeURIComponent(modelAliasMatch[1]!); | ||
| if (name === "keys") return null; |
There was a problem hiding this comment.
keys
When a user has a custom provider named keys—which isValidProviderName and the provider creation route currently allow—this guard returns null for PUT /api/providers/keys/model-aliases. No later handler owns that path, so the management API returns 404 and both ocx alias set keys/<model> ... and ocx alias rm keys/<model> fail. Remove this second guard; only the singular /api/providers/keys/alias path collides with the key-pool rename endpoint.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
tarunravi
pushed a commit
to tarunravi/opencodex
that referenced
this pull request
Sep 14, 2026
…landings (lidge-jun#2614) Found by the full suite on dev HEAD (36 failures across four files), not by any of the three PRs' own focused runs - each was green in isolation. 1. rebase-provenance: structuredClone on the whole config threw DataCloneError whenever a provider carried a non-cloneable value. A test fixture injects its own fetch, and a function is not cloneable, which took down every provider probe and both CLI parity suites. Only the top-level provenance key is rewritten in that projection, so nothing below the top level needed copying at all - a shallow copy is both correct and what the function actually meant. 2. /api/providers/:name/alias swallowed /api/providers/keys/alias. Model routes dispatch before oauth-account routes, so the new alias route matched name='keys', found no such provider, and 404'd every API-key-pool rename. Guarded rather than reordered: the dispatch order is load-bearing elsewhere. 3. The alias command and its two endpoints were missing from the help banner and the GUI/CLI parity map. Both sweeps exist to catch exactly this, and both did. Each repair verified against the failing test, and the full local set of affected suites is green: provider-api-keys, provider-connection-test, config-user-edits, config-rebase-provenance-writers, alias-management-api, cli-headless-parity, cli-registry.
agentHits
pushed a commit
to agentHits/opencodex
that referenced
this pull request
Sep 17, 2026
…landings (lidge-jun#2614) Found by the full suite on dev HEAD (36 failures across four files), not by any of the three PRs' own focused runs - each was green in isolation. 1. rebase-provenance: structuredClone on the whole config threw DataCloneError whenever a provider carried a non-cloneable value. A test fixture injects its own fetch, and a function is not cloneable, which took down every provider probe and both CLI parity suites. Only the top-level provenance key is rewritten in that projection, so nothing below the top level needed copying at all - a shallow copy is both correct and what the function actually meant. 2. /api/providers/:name/alias swallowed /api/providers/keys/alias. Model routes dispatch before oauth-account routes, so the new alias route matched name='keys', found no such provider, and 404'd every API-key-pool rename. Guarded rather than reordered: the dispatch order is load-bearing elsewhere. 3. The alias command and its two endpoints were missing from the help banner and the GUI/CLI parity map. Both sweeps exist to catch exactly this, and both did. Each repair verified against the failing test, and the full local set of affected suites is green: provider-api-keys, provider-connection-test, config-user-edits, config-rebase-provenance-writers, alias-management-api, cli-headless-parity, cli-registry.
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
1 participant
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
The full suite on
devHEAD reported 36 failures across four files. Each of the three PRs that caused them (#2610, #2613) was green on its own focused run — these only appear once the landings are combined, which is exactly what a full-suite gate is for.1.
DataCloneErrorin the provenance projection.projectConfigRebaseProvenancedeep-cloned the whole config, and a provider entry may carry a valuestructuredClonerefuses — a test fixture injects its ownfetch, and a function is not cloneable. That took down everyPOST /api/providers/testcase and both CLI parity suites. The projection only rewrites the top-level provenance key, so nothing below the top level ever needed copying: a shallow copy is both correct and what the function actually meant. The deep clone was doing work it never asked for.2. The new alias route swallowed the API-key-pool rename.
/api/providers/:name/aliasmatched/api/providers/keys/aliaswithname = "keys". Model routes dispatch beforeoauth-account-routes, so the alias handler found no provider namedkeysand returned 404 for every key-pool rename. Fixed with an explicit guard rather than by reordering dispatch, since that order is load-bearing for other routes and reordering to fix a pattern collision would trade one silent shadow for another.3.
ocx aliasand its two endpoints were undocumented. Missing from theprintUsagebanner and from the GUI/CLI parity coverage map. Both sweeps exist to catch precisely this, and both did.Verification
Each repair was verified against the specific failing assertion it addresses — the
DataCloneErrorstack, theexpect(rename.status).toBe(200)that received 404, and the two coverage sweeps'Expected [] / Received ["alias"]and["/api/aliases", "/api/default-aliases"].Checklist
dev