Normalize legacy Codex session runtime state#90321
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 4, 2026, 9:17 PM ET / 01:17 UTC. Summary PR surface: Source +39, Tests +69. Total +108 across 2 files. Reproducibility: yes. source inspection shows current main can persist runtime provider/model metadata without legacy Codex canonicalization, and the PR discussion supplies live Windows/Telegram failure-shape proof. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused write-boundary guard with the companion migration work so existing stale state is repaired and future session writes stay on canonical Do we have a high-confidence way to reproduce the issue? Yes: source inspection shows current main can persist runtime provider/model metadata without legacy Codex canonicalization, and the PR discussion supplies live Windows/Telegram failure-shape proof. Is this the best way to solve the issue? Yes: normalizing at the shared session helper and merge/write boundary is the durable fix; status-only display normalization or doctor-only repair would still allow runtime writes to recreate the legacy route. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 4dd00347fc95. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +39, Tests +69. Total +108 across 2 files. View PR surface stats
Acceptance criteria:
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
|
|
Addressed the ClawSweeper runtime-write-path feedback in What changed:
Focused proof from the patched branch: Also reran @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review after adding the runtime write-boundary fix/proof to the PR body. |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
Follow-up |
57b938c to
acc7051
Compare
|
Rebased onto latest |
|
🦞👀 Command router queued. I will update this comment with the next step. |
acc7051 to
60de233
Compare
|
Rebased again onto latest |
|
🦞👀 Command router queued. I will update this comment with the next step. |
|
The remaining |
|
Additional live Windows 6.1 proof for why this write boundary matters:
This is why the PR intentionally covers the merge/persist writer path ( Related maintainer/community signal I used to scope this PR: the current problem is not another broad migration rewrite; it is narrow prevention of legacy @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
60de233 to
03f051a
Compare
|
@ooiuuii this fix is valid but currently we're avoiding most runtime shims like this (there are quite a few but we're hoping to move away from that) and only officially supporting updates through |
Summary
setSessionRuntimeModel()from writingmodelProvider: "openai-codex"ormodel: "openai-codex/*"back into session stateopenai-codex + openai-codex/gpt-*andopenai + openai-codex/gpt-*inputsContext
This is PR3 from the Codex/OpenAI migration follow-up. PR #90317 covers config migration; PR #90319 covers repairing already-persisted session-store routes. This PR covers the remaining forward-looking guard: preventing runtime completion/session persistence from reintroducing legacy
openai-codexroute state after repair.Related to #90036.
Verification
pnpm tsx .tmp-check-codex-pr3.mtsbehavior check:PR3_BEHAVIOR_OKgit diff --checkNote: direct targeted Vitest startup stayed in repeated Rolldown
PLUGIN_TIMINGSoutput locally, so I used a focused behavior check for this helper and left the regression in the Vitest suite for CI.Real behavior proof / runtime write-boundary proof
Addressed ClawSweeper's runtime write-path concern in
a9812bba9f.Focused proof from the patched branch:
Added regression coverage for both merge boundaries and
updateSessionStore(...)write boundaries, so legacy Codex routes are normalized before session state can be persisted again.CI follow-up
57b938c5d1fixes the failing store-write-boundary test to useupdateSessionStoreEntry(...), the entry writer that exercises merge + persist behavior.Focused behavior proof after the test fix:
Local targeted Vitest stalled with no output in this environment again, so CI is the authoritative rerun for the full runtime-config shard.
Real behavior proof (structured for gate)
openai-codexprovider/model state at runtime/session-store write boundaries.2026.6.1Telegram/dashboard failure mode plus this PR branch on macOS source checkout.openai/gpt-5.5plus Codex runtime, noopenai-codex/*persisted), then ran the patched store write-boundary regression withnode scripts/run-vitest.mjs src/config/sessions/sessions.test.ts --testNamePattern="canonicalizes legacy Codex runtime fields at store write boundaries".modelProvider/modeltoopenai/gpt-5.5before persistence, preventing the Windows 6.1Unknown model: openai-codex/gpt-5.5route from being written back again.