fix: close four regressions from the 260923 bundle round - #5720
Conversation
…x holds no Codex state #5441 made any local ~/.codex directory the Codex home on WSL, even when config.toml is missing. A WSL user whose ~/.codex exists but holds no Codex state, and who ran against the discovered Windows Codex home, was moved to an empty local home on upgrade: auth and sessions disappeared and a sync wrote a new local config. The local home now wins only when Codex is already using it: config.toml, auth.json, sessions or history.jsonl is present (an unexpected stat error counts as present, so doubt never switches homes). #5441's fresh install keeps its local home once Codex has logged in or run; a bare directory falls back to Windows discovery as before #5441.
… on Windows On Windows, npm -g under a mise-managed Node installs OpenCodex directly into <mise>/installs/node/<version>/node_modules. The mise ownership walk then read Node's own .mise.backend.toml (short = "node", full = "core:node") as contradictory OpenCodex metadata and refused ocx update with metadata_inconsistent, although the install is plain npm. That exact runtime record, with the package directly in the runtime's global node_modules, now falls through to ordinary npm detection. Any other backend or alias under a node tool root, a deeper nested layout, unreadable metadata and every OpenCodex mismatch stay fail-closed. POSIX is unaffected: its lib/node_modules layout never reaches the Node record.
…velope The shared tool-envelope echo filter from #5676 matched as soon as a line started with a marker and dropped that line and everything after it. It is armed on almost every agentic turn (input with tool calls or outputs, or a previous_response_id continuation), so an answer line such as "[Tool Result] shows the build passed." silently truncated the rest of the reply on xAI and Cursor. The Cursor history stripper had been widened to the same prefix rule and removed such prose from replayed history. The envelope OpenCodex replays is a marker alone on its line. A result, error or call marker now counts only when it is the whole line (trailing whitespace and CR allowed), decided when the line completes; the end of the stream follows the same rule, and a bare truncated marker still counts. The "[Tool call:" line keeps its prefix rule because a call echo wraps when its arguments do. The Cursor replay stripper uses the same isWholeLineEchoMarker. Fenced markers, the xAI Responses JSON path and the stored continuation snapshot follow from the shared filter.
… tool call #5548 holds everything after a bare <tool_call><function=...> opening until the stream ends, so a model that writes such a block and then keeps answering without a structured call delivered the rest of its answer only at the end of the turn, and could approach the translator budget. Streaming content now goes through ingestStreaming. Once a closed block is followed by more than 8 KiB of prose with no block open after it, or held text plus queued events would pass 4 MiB (checked before the next delta is retained), everything held is released in order with nothing suppressed. A duplicated block is the tail of the content, so matching blocks followed by their structured call are still removed; past a bound the stream prefers delivery over suppression, the behaviour before #5548. Buffered responses keep the unbounded ingest because their structured calls are already known. openai-chat.ts keeps its line count.
…ueued-event paths A delta that opens a block and already passes 4 MiB is no longer retained, an oversized delta after an open block is delivered after the held text, and queued non-text events count toward the same bound. The fresh WSL home test now models a fresh install (auth.json present, config.toml absent) instead of a mock that reported every path present. Review decisions are recorded in the plan.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (33)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e606cf8682
ℹ️ 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".
|
|
||
| /** Non-whitespace characters after the last closed block, or 0 while a later block is still open. */ | ||
| function proseAfterClosedBlock(text: string): number { | ||
| const closer = text.lastIndexOf(CLOSE_TAG); |
There was a problem hiding this comment.
Require a structurally closed block before releasing it
When a streamed tool argument itself contains the literal </tool_call> and more than 8 KiB follows before the real </function></tool_call> arrives in a later chunk, this substring search treats the still-open block as closed and releases its raw serialized markup. The subsequent structured call can no longer reconcile or suppress it, so users see duplicate protocol text alongside the dispatched tool call. Determine closure using the serialized-call grammar or tracked parser state rather than any occurrence of the closing-tag string.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
#5721) #5720 declared the Codex-state list as a module-level const below defaultCodexHome. The storage workers reach defaultCodexHome during module initialisation through an import cycle, and with a ~/.codex directory present (every Windows CI runner, and most user machines) the call read the const before it was initialised: ReferenceError in the trash-restore and policy-run workers. The list is now local to the function. The CodeBuddy compiled-MCP test also removes its temp directory with the shared retrying helper: Windows keeps the compiled ocx executable locked briefly after the process exits, and a plain rmSync failed with EBUSY.
Summary
Fixes four regressions a post-merge review found in the 260923 bundle round (#5672-#5685). Each changes behaviour an existing user could hit on upgrade without any config change.
~/.codexdirectory had become the Codex home, even with no Codex state, so a WSL user running against the discovered Windows home was moved to an empty local home: auth and sessions disappeared and a sync wrote a new local config. The local home now wins only when Codex is already using it (config.toml,auth.json,sessionsorhistory.jsonlpresent; an unexpected stat error counts as present). An empty local directory falls back to Windows discovery as before fix(codex): keep fresh WSL home before config exists #5441;CODEX_HOMEstill overrides.npm -gunder a mise-managed Node writes into<mise>/installs/node/<ver>/node_modules; the mise ownership walk read Node's own.mise.backend.toml(core:node) as contradictory OpenCodex metadata and refusedocx update. That exact runtime record with the package directly in the runtime's globalnode_modulesnow falls through to npm detection; every other mismatch stays fail-closed. POSIX never reached that record.[Tool call:keeps its prefix rule because call echoes wrap with their arguments. The Cursor history stripper, which fix(xai): bundle lane B — reasoning-model stop/penalty drops, policy 403 as content_filter, tool-result echo filter #5676 had widened to the prefix rule, uses the same predicate.<tool_call>block held the rest of a streamed answer until the turn ended. Streaming now releases everything held, in order and unsuppressed, once a closed block is followed by more than 8 KiB of prose or held text plus queued events would pass 4 MiB. A real duplicate (the tail of the content) is still removed; buffered responses keep their unbounded reconciliation.Plan and review record:
devlog/_plan/260924_regression_risk_fixes/.Verification
tests/codex-integration/codex-home-wsl-local-state.test.ts,tests/update/update-mise-node-runtime.test.ts,tests/adapters/tool-envelope-echo-whole-line.test.ts,tests/adapters/openai/openai-chat-serialized-tool-call-hold-bound.test.ts; the existing fresh-WSL-home test now models a fresh install instead of a mock that reported every path present.~/.codex: 11989 pass / 43 skip / 1 fail; the one failure was this branch's test-layout seed mismatch, fixed in the last commit (27/27 layout tests pass). Focused receipt at head: 1929 pass / 0 fail (--isolate).bun run typecheck(tsc 7, 1833 files) andbun run structure:checkpass.src/adapters/openai-chat.tsstays at 818/822 lines.ci.yml lane=allon the mergeddevtip.Checklist