fix(doctor): consolidate legacy Codex migration cleanup#90451
Open
ooiuuii wants to merge 5 commits into
Open
Conversation
Contributor
|
Codex review: needs maintainer review before merge. Reviewed June 7, 2026, 11:25 AM ET / 15:25 UTC. Summary PR surface: Source +251, Tests +463. Total +714 across 4 files. Reproducibility: yes. at source level. Current main resolves OpenAI/Codex context config through canonical Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the doctor/update repair after maintainer review confirms the model-scoped migration policy, keeping the current regression coverage and avoiding runtime Do we have a high-confidence way to reproduce the issue? Yes, at source level. Current main resolves OpenAI/Codex context config through canonical Is this the best way to solve the issue? Yes. Doctor/update is the right layer under the repository policy, and current head scopes legacy provider metadata to model rows or a manual-warning path instead of adding runtime fallback readers. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 66b91d78feb3. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +251, Tests +463. Total +714 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
7add989 to
3021712
Compare
3021712 to
ffb1873
Compare
ffb1873 to
02363e8
Compare
Contributor
Author
|
Addressed the ClawSweeper recommendation in latest head What changed:
Validation:
|
02363e8 to
5ce5bf2
Compare
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Contributor
Author
|
Addressed the latest ClawSweeper provider-level budget precedence finding in head What changed:
Validation:
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Contributor
Author
|
Addressed the latest ClawSweeper provider-default precedence finding in head What changed:
Validation:
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
871e9df to
9652497
Compare
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Contributor
Author
|
@clawsweeper re-review Current-head fix for the P1 mixed provider-budget finding is pushed at What changed:
Focused current-head checks passed:
|
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Contributor
Author
|
@clawsweeper re-review Current head Validation on this head:
The PR body proof was refreshed with current-head migration output for unrelated canonical OpenAI models, matching-plus-disjoint Codex models, and unmapped provider-budget manual fallback. |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
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
This PR consolidates legacy Codex/OpenAI doctor migration cleanup after the OpenAI/Codex provider unification.
models.providers.openaiand legacymodels.providers.openai-codexexist, doctor now copies missing positive context metadata from the legacy provider before removing it.contextTokens,contextWindow, andmaxTokensat provider/model scope.openaivalues remain authoritative: existing canonical metadata is not overwritten, explicit canonical provider-level context budgets are not overridden by stale legacy model rows, andcontextTokens/contextWindoware treated as one context-budget group so legacy data cannot reverse runtime precedence.gpt-5.5,openai/gpt-5.5, andopenai-codex/gpt-5.5; scoped canonical OpenAI rows are normalized back to runtime-readable unscoped ids before metadata is copied.openai-codex/*model refs when the persisted session provider is already canonicalopenai, so stored model/modelOverride fields become runtime-readable unscoped ids such asgpt-5.5.openai.Why now: 2026.5.28-era installs could store Codex context overrides under
models.providers.openai-codex.models[]. After 2026.6.1 unification, configs that already had canonicalmodels.providers.openaiwould remove the legacy provider without carrying over that context metadata, causing unified OpenAI/Codex routes to fall back to lower default context budgets.Out of scope: runtime fallback/shims, changing Codex defaults, changing OAuth/auth routing, or restoring closed runtime write-boundary behavior. This PR only repairs persisted config/session state during doctor/update.
Linked context
Closes #90448
Related #77858
Related #77861
Consolidates the useful doctor-side session-store edge case from closed PR #90662.
Real behavior proof (required for external PRs)
models.providers.openai-codexduring OpenAI/Codex route unification. It must preserve legacy CodexcontextTokens/contextWindow/maxTokenswithout turning those Codex-only provider defaults into broad canonicalmodels.providers.openaiprovider defaults for unrelated OpenAI models. It must also repair stale persisted sessions withmodelProvider: "openai"butmodel/modelOverridestill scoped asopenai-codex/gpt-*.agent/xiaozhua/codex-context-override-migration, current PR head2226e281030) using the realLEGACY_CONFIG_MIGRATIONSdoctor migration pipeline. The original regression shape comes from official OpenClaw upgrades across the 2026.5.28 → 2026.6.1 OpenAI/Codex unification.{ "unrelatedCanonicalModel": { "providers": { "openai": { "api": "openai-chatgpt-responses", "models": [ { "id": "text-embedding-3-small", "name": "OpenAI embeddings" }, { "id": "gpt-5.5", "name": "Legacy Codex GPT-5.5", "baseUrl": "https://chatgpt.com/backend-api/codex", "api": "openai-chatgpt-responses", "contextWindow": 1050000, "contextTokens": 1050000, "maxTokens": 16384 } ] } }, "changes": [ "Moved models.providers.openai-codex.api \"openai-codex-responses\" → \"openai-chatgpt-responses\".", "Merged 1 model(s) from models.providers.openai-codex into models.providers.openai: gpt-5.5." ] }, "matchingPlusDisjoint": { "providers": { "openai": { "api": "openai-chatgpt-responses", "baseUrl": "https://api.openai.com/v1", "models": [ { "id": "gpt-5.5", "contextTokens": 1050000, "contextWindow": 1050000, "maxTokens": 16384 }, { "id": "gpt-5.6", "name": "Legacy Codex GPT-5.6", "baseUrl": "https://chatgpt.com/backend-api/codex", "api": "openai-chatgpt-responses", "contextWindow": 1050000, "contextTokens": 1050000, "maxTokens": 16384 } ] } }, "changes": [ "Moved models.providers.openai-codex.api \"openai-codex-responses\" → \"openai-chatgpt-responses\".", "Normalized models.providers.openai.models[0].id to gpt-5.5 and copied contextTokens, contextWindow, maxTokens metadata.", "Merged 1 model(s) from models.providers.openai-codex into models.providers.openai: gpt-5.6." ] }, "unmappedProviderBudget": { "providers": { "openai": { "api": "openai-chatgpt-responses", "models": [ { "id": "text-embedding-3-small", "name": "OpenAI embeddings" } ] }, "openai-codex": { "contextTokens": 1050000, "contextWindow": 1050000, "maxTokens": 16384 } }, "changes": [ "Skipped removing models.providers.openai-codex because provider-level context metadata cannot be mapped safely to a canonical OpenAI model row." ] } }models.providers.openai. In the unrelated canonical model scenario, the existingtext-embedding-3-smallrow remains budget-free while the merged Codexgpt-5.5row receives the Codex base URL and context budget. In the matching-plus-disjoint scenario, the matchinggpt-5.5row and mergedgpt-5.6row both preserve Codex budgets. In the unmapped scenario, doctor keepsmodels.providers.openai-codexand emits a manual-migration change instead of silently broadening the budget to every OpenAI model. Existing canonical provider/model context budgets remain authoritative and are not overwritten. The session-store repair also preservesmodelProvider: "openai"while rewriting staleopenai-codex/gpt-*model/modelOverride values to unscopedgpt-*and clearing stale Codex runtime pins.models.providers.openai, making Codex-only budgets fallback defaults for unrelated OpenAI models; in the earlier mixed path, provider-level budgets could also be skipped before deleting the legacy provider.Tests and validation
Commands run after the current-head P1 fixes:
Results:
Earlier validation retained by this PR:
Regression coverage added:
openai-codex.models[].contextTokens/contextWindow/maxTokensby creating canonicalopenai.models[]metadata when canonical OpenAI exists but has no matching model row.openai-codex.contextTokens/contextWindow/maxTokenson matching canonical OpenAI model rows when another disjoint legacy model must also be merged.openai/gpt-5.5to runtime-readablegpt-5.5before copying missing legacy metadata.openaiprovider/model config.openaivalues are not overwritten by stale legacy Codex values.contextTokensandcontextWindowremain authoritative over stale legacy Codex context-budget metadata, including the reversecontextWindowversus legacycontextTokenscase, matching canonical model rows with provider-level budgets, and canonical model-level budgets over legacy provider defaults.modelProvider: "openai"+model/modelOverride: "openai-codex/gpt-*"to provider-separatedgpt-*ids while clearing stale Codex runtime pins.Risk checklist
Did user-visible behavior change? (
Yes/No)Yes. After running doctor/update migration, legacy Codex context metadata can continue to affect unified OpenAI/Codex context budgets via canonical
models.providers.openai, and stale session model refs under canonical OpenAI providers are normalized to runtime-readable unscoped ids.Did config, environment, or migration behavior change? (
Yes/No)Yes. Doctor migration now preserves missing context metadata before removing
models.providers.openai-codexwhen canonicalopenaialready exists, and session-store doctor repair handles one additional stale OpenAI/Codex model shape.Did security, auth, secrets, network, or tool execution behavior change? (
Yes/No)No. This does not change auth, runtime provider routing, network behavior, or tool execution.
What is the highest-risk area?
Copying stale legacy context metadata into canonical
openaiwhere explicit canonical values should remain authoritative; additionally, normalizing only stale Codex-scoped session model refs without disturbing valid canonical provider/model pairs.How is that risk mitigated?
The migration copies only positive numeric metadata, only for missing target fields, and tests cover canonical-wins behavior at provider and model scope. Session repair tests cover preserving canonical
modelProvider: "openai", rewriting only staleopenai-codex/*model refs to unscoped ids, and clearing stale Codex runtime pins.Current review state
What is the next action?
Wait for CI and ClawSweeper re-review on current head
2226e281030.What is still waiting on author, maintainer, CI, or external proof?
CI/ClawSweeper are pending after the latest provider-level budget scoping repair.
Which bot or reviewer comments were addressed?
Addressed ClawSweeper's P1 provider-level budget scoping finding, the mixed matching-plus-disjoint provider-budget finding, the earlier scoped canonical-row finding, and consolidated the useful doctor-side edge case from closed #90662 after maintainer feedback to consolidate remaining small migration PRs into a single PR.