fix(ce-setup): detect the retired Codex tool map - #1577
Merged
Conversation
check-health never read the Codex home instructions file, so an install
still carrying the retired COMPOUND CODEX TOOL MAP block reported a clean
environment. The Bun-era installer wrote that block and the Bun CLI only
strips it on a re-run, so native marketplace installs keep it indefinitely.
Scan ${CODEX_HOME:-$HOME/.codex}/AGENTS.md for the block, report it with
the other diagnostics, and note it in the verdict so the report does not
read as all-clear. ce-setup offers the removal from a skill-local
reference, stated outside the Phase 2 gate because the file is in the
user's Codex home rather than the checkout.
Detection is one condition -- a standalone BEGIN sentinel line followed by
a standalone END sentinel line -- which decides inline prose mentions,
reversed sentinels, and a stray END before a valid pair without
enumerating them.
Replaces #1570. Refs #1559.
Claude-Session: https://claude.ai/code/session_01KDDrb1CmAhvfRNb49kE3UQ
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. |
![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: 6fd89c251a
ℹ️ 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".
…ummary The session-level tool-map remediation was wired into the bundled script's report and the Phase 2 decision, but not into the two other layers it travels through. The Step 2 inline fallback checklist had no Codex-home scan, so on the script-unavailable path the report could never name the map and the Step 3 gate never fired. Mirror the script's condition as a seventh inline check. The Phase 3 summary slots were scoped to repo-local work, so a Codex-home removal had nowhere to land and the run could report "Fixed: none" after editing the user's AGENTS.md. Drop the repo-local qualifier so the slots state what changed and what was declined, rather than which scope it came from. Addresses review feedback on #1577. Claude-Session: https://claude.ai/code/session_01KDDrb1CmAhvfRNb49kE3UQ
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.
Replaces #1570 (closed). Addresses the detection half of #1559; see "What this does not fix" below.
Summary
check-healthnever looked at the Codex home instructions file, so an install still carrying the retired<!-- BEGIN COMPOUND CODEX TOOL MAP -->block reported a clean environment. The Bun-era installer wrote that block;ca474429bstrips it only when someone re-runs the Bun CLI, so anyone who moved to the native marketplace install keeps it indefinitely with nothing surfacing it.The check now scans
${CODEX_HOME:-$HOME/.codex}/AGENTS.md, reports the block alongside the other diagnostics, and adds a line to the verdict so the report does not read as all-clear while it stands.ce-setupoffers the removal from a skill-local reference.Design decisions
Detection is one condition, not a set of cases. The block is present when a standalone BEGIN sentinel line is followed by a standalone END sentinel line. That single condition decides every case #1570 accumulated over its review rounds — inline mentions of both markers in prose, a stray END before the first BEGIN, END/BEGIN/END — in six lines of
awkrather than ~50 lines of branching. Verified against all of them plus CRLF line endings and a missing file.No change to
src/utils/codex-agents.ts. #1570 also tightenedremoveCodexAgentsToolMapBlockto match. The Bun writer (buildCodexAgentsBlock) always emitted sentinels on their own lines, so no file that stripper will ever see is affected. Leaving shipped CLI behavior alone.The remediation is session-level, so it sits outside the Phase 2 gate. Phase 2 is skipped when there is no writable checkout — which is the reported scenario. The offer is stated where that decision is made rather than in the repo-local project-issues list.
Dropped
~/.codex/profiles/*/AGENTS.md. Codex resolves user instructions from$CODEX_HOME/AGENTS.md(falling back to$HOME/.codex); profiles are[profiles.<name>]tables inconfig.tomlselected withcodex -p, not directories with their ownAGENTS.md. That path entered the docs as a hedge in #1118. Removed from the upgrade guide too.Validation
bun run test— 3647 pass, 0 failbun run release:validate— in syncSkill eval
bun run test:skill-eval-cell -- --skill ce-setup --read-only, Claude and Codex, prompted with realcheck-healthoutput. Two scenarios; the pre arm ran against #1570's tree.Both arms offered the removal on both hosts, so the placement change is a correctness-of-representation fix, not a demonstrated routing fix — recording that plainly rather than claiming a behavior win. The writable-checkout row is there because moving the bullet out of the Phase 2 list could have dropped one side or the other; neither host dropped either.
What the eval did discriminate is what reaches the user. The pre arm relayed the accumulated edge-case rules into the user-facing instructions ("a stray END before the first BEGIN doesn't count", "inline mentions of those strings are not a block") and — on both hosts — told the user to also check
~/.codex/profiles/*/AGENTS.md, a directory that does not exist. The post arm gave one condition and one path.What this does not fix
This does not close #1559. The failure that issue reports is
ce-workemittingCode review: skipped (ce-code-review unavailable)with the reason "no callable ce-code-review runner in the current harness" — which is not one of the states that gate enumerates (skills/ce-work/references/shipping-workflow.md:41: dispatch unavailable, unauthenticated, hard-capped, or afailed/degraded/skippedreturn). Detection does not touch that. Leaving #1559 open for it.Credit
The gap, and the
CODEX_HOMEfixture isolation kept here, are from @saurabhkagent-lab's work in #1570.Security Disclosure
The health check reads one additional file,
${CODEX_HOME:-$HOME/.codex}/AGENTS.md, and only when it exists. It is read-only —awkmatches two exact literal lines and the script never writes to it. The path comes from an environment variable already trusted by the surrounding script and is quoted at every use. Removal remains a user-approved action performed by the agent, not by this script. No other security-relevant changes.Agent Disclosure