Skip to content

fix(doctor): normalize stale OpenAI Codex session refs#90662

Closed
ooiuuii wants to merge 1 commit into
openclaw:mainfrom
ooiuuii:agent/xiaozhua/codex-session-doctor-normalize
Closed

fix(doctor): normalize stale OpenAI Codex session refs#90662
ooiuuii wants to merge 1 commit into
openclaw:mainfrom
ooiuuii:agent/xiaozhua/codex-session-doctor-normalize

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR tightens the doctor repair path for stale Codex session store state after the OpenAI/Codex provider unification.

  • If a persisted session already has canonical modelProvider: "openai" / providerOverride: "openai" but still stores a legacy openai-codex/... model value, doctor now rewrites the model value to the unscoped OpenAI model id (gpt-*).
  • Existing behavior for providerless legacy refs still canonicalizes to openai/... so the provider can be inferred later.
  • Existing behavior for legacy modelProvider: "openai-codex" still rewrites the provider to openai and keeps the model unscoped.
  • Stale Codex runtime pins continue to be cleared as part of session route repair.

This is intentionally doctor/update repair only. It does not reintroduce the closed runtime write-boundary shim from #90321.

Linked context

Related #90319
Related #90321

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: a stale persisted session shape with canonical modelProvider: "openai" but legacy model: "openai-codex/gpt-*" was repaired to model: "openai/gpt-*", leaving a provider-prefixed model under an already canonical provider. Session resolution treats modelProvider + model as a pair, so the durable repaired state should be modelProvider: "openai", model: "gpt-*".
  • Real environment tested: macOS local OpenClaw source checkout on this PR branch (agent/xiaozhua/codex-session-doctor-normalize, commit 0f24e0f9570) using the real repairCodexSessionStoreRoutes doctor session repair function.
  • Exact steps or command run after this patch:
node --import tsx - <<'EOF'
import { repairCodexSessionStoreRoutes } from './src/commands/doctor/shared/codex-route-warnings.ts';
const store = {
  staleOpenAIProvider: {
    sessionId: 's1',
    updatedAt: 1,
    modelProvider: 'openai',
    model: 'openai-codex/gpt-5.5',
    providerOverride: 'openai',
    modelOverride: 'openai-codex/gpt-5.4',
    agentHarnessId: 'codex',
    agentRuntimeOverride: 'codex',
  },
};
const result = repairCodexSessionStoreRoutes({ store, now: 123 });
console.log(JSON.stringify({ result, store }, null, 2));
EOF
  • Evidence after fix:
{
  "result": {
    "changed": true,
    "sessionKeys": [
      "staleOpenAIProvider"
    ]
  },
  "store": {
    "staleOpenAIProvider": {
      "sessionId": "s1",
      "updatedAt": 123,
      "modelProvider": "openai",
      "model": "gpt-5.5",
      "providerOverride": "openai",
      "modelOverride": "gpt-5.4"
    }
  }
}
  • Observed result after fix: doctor rewrites canonical OpenAI session-provider pairs to unscoped OpenAI model ids and clears stale Codex runtime pins.
  • What was not tested: I did not run this against a production session store. The proof uses the real doctor repair function plus focused and full tests for the relevant doctor warning file.
  • Before evidence: before this patch, the same modelProvider: "openai", model: "openai-codex/gpt-5.5" shape repaired to model: "openai/gpt-5.5", leaving a provider-prefixed model under canonical modelProvider: "openai".

Tests and validation

Commands run:

node scripts/run-vitest.mjs src/commands/doctor/shared/codex-route-warnings.test.ts --testNamePattern="repairs legacy Codex model refs under canonical OpenAI session providers|repairs persisted session route refs|repairs Telegram direct session routes"
node scripts/run-vitest.mjs src/commands/doctor/shared/codex-route-warnings.test.ts
pnpm exec oxlint src/commands/doctor/shared/codex-route-warnings.ts src/commands/doctor/shared/codex-route-warnings.test.ts
git diff --check

Results:

[test] passed 1 Vitest shard in 26.02s
[test] passed 1 Vitest shard in 35.06s
Found 0 warnings and 0 errors.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes. Running doctor/update on stale session stores produces cleaner canonical OpenAI session model state.

Did config, environment, or migration behavior change? (Yes/No)

Yes. Doctor session-store repair now normalizes this additional stale session shape.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No.

What is the highest-risk area?

Accidentally changing providerless legacy model refs that need a provider prefix for later inference.

How is that risk mitigated?

The new unscoped rewrite applies only when the persisted session provider is already canonical openai; providerless legacy refs continue to use the existing openai/... canonical form. Existing full codex-route-warnings coverage still passes.

Current review state

What is the next action?

Wait for CI and ClawSweeper review.

What is still waiting on author, maintainer, CI, or external proof?

CI/ClawSweeper are pending.

Which bot or reviewer comments were addressed?

None. This follows up on the remaining doctor-session repair gap rather than reviving the closed runtime shim from #90321.

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 5, 2026
@clawsweeper

clawsweeper Bot commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 5, 2026, 9:02 AM ET / 13:02 UTC.

Summary
The PR changes doctor session-store repair so legacy openai-codex/* model refs under an already canonical OpenAI session provider are rewritten to unscoped gpt-* model ids.

PR surface: Source +1, Tests +29. Total +30 across 2 files.

Reproducibility: yes. from source inspection. On current main, a session entry with canonical modelProvider: "openai" but model: "openai-codex/gpt-*" reaches the stale-model branch and is rewritten to openai/gpt-*, leaving a provider-prefixed model under an already separate provider field.

Review metrics: 1 noteworthy metric.

  • Session repair rule: 1 doctor session repair rule changed. The changed rule mutates persisted model and modelOverride fields during doctor repair, so reviewers should notice the upgrade/session-state impact.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] This changes doctor/update behavior for persisted session state: openclaw doctor --fix will rewrite an additional stale OpenAI/Codex model shape, so maintainers should treat it as a small session-state migration despite the focused proof.

Maintainer options:

  1. Land the doctor-only migration (recommended)
    Merge after normal CI if maintainers accept the focused helper proof and regression coverage for this stale persisted session shape.
  2. Ask for affected-store proof
    Request a redacted doctor --fix run on an affected real session store if maintainers want stronger upgrade evidence before changing persisted repair behavior.

Next step before merge

  • No ClawSweeper repair lane is needed because there are no blocking patch findings; maintainers should proceed with normal CI and merge review.

Security
Cleared: The diff only changes doctor session repair logic and its focused test; it does not add dependencies, workflows, install hooks, secrets handling, or new code-execution paths.

Review details

Best possible solution:

Land the narrow doctor-only migration if CI stays green, keeping runtime write-boundary changes out of this PR unless maintainers reopen that separate product decision.

Do we have a high-confidence way to reproduce the issue?

Yes from source inspection. On current main, a session entry with canonical modelProvider: "openai" but model: "openai-codex/gpt-*" reaches the stale-model branch and is rewritten to openai/gpt-*, leaving a provider-prefixed model under an already separate provider field.

Is this the best way to solve the issue?

Yes. The PR fixes the existing doctor migration helper at the repair boundary, preserves providerless legacy behavior, and avoids reviving the broader runtime write-boundary shim from #90321.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 520992a1defd.

Label changes

Label changes:

  • add P2: This is a narrow bug fix for doctor repair of stale OpenAI/Codex session state after provider unification.
  • add merge-risk: 🚨 session-state: The PR intentionally changes how doctor repair rewrites persisted session model fields, which can affect upgraded session state.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from the real doctor repair helper showing the repaired persisted session shape; no contributor action is needed for proof.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from the real doctor repair helper showing the repaired persisted session shape; no contributor action is needed for proof.
  • remove rating: 🌊 off-meta tidepool: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This is a narrow bug fix for doctor repair of stale OpenAI/Codex session state after provider unification.
  • merge-risk: 🚨 session-state: The PR intentionally changes how doctor repair rewrites persisted session model fields, which can affect upgraded session state.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from the real doctor repair helper showing the repaired persisted session shape; no contributor action is needed for proof.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from the real doctor repair helper showing the repaired persisted session shape; no contributor action is needed for proof.
Evidence reviewed

PR surface:

Source +1, Tests +29. Total +30 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
Tests 1 29 0 +29
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 31 1 +30

What I checked:

Likely related people:

  • Peter Steinberger: Current shallow blame attributes the doctor repair helpers, session model-pair helpers, and OpenAI/Codex normalization helpers to the source-bridge commit that seeded this checkout. (role: introduced current helper surface; confidence: medium; commits: 9fd5f9ee7ca0; files: src/commands/doctor/shared/codex-route-warnings.ts, src/config/sessions/types.ts, src/agents/model-selection.ts)
  • ooiuuii: Merged related Codex/OpenAI doctor migration coverage in the same test area shortly before this PR, so they are connected to the current repair thread beyond authoring this branch. (role: recent adjacent contributor; confidence: high; commits: 560b77a4aff7, cfd5f1ad13a7; files: src/commands/doctor/shared/codex-route-warnings.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added the rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. label Jun 5, 2026
@ooiuuii

ooiuuii commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 5, 2026
@RomneyDa

RomneyDa commented Jun 5, 2026

Copy link
Copy Markdown
Member

@ooiuuii I merged some single-test PRs yesterday, but please consolidate the rest of your plan on a single PR, we don't usually merge those.

@RomneyDa RomneyDa closed this Jun 5, 2026
@ooiuuii ooiuuii mentioned this pull request Jun 5, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

commands Command implementations merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ooiuuii @RomneyDa