fix: normalize Codex Context7 naming#970
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR canonicalizes the Context7 MCP TOML section to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/codex/merge-mcp-config.js">
<violation number="1" location="scripts/codex/merge-mcp-config.js:109">
P2: With the new alias mapping, `--update-mcp` leaves legacy `context7-mcp` entries intact when a canonical `context7` entry already exists, causing duplicate server definitions after update.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR normalizes the Codex MCP section name for Context7 from the inconsistent Key changes:
Confidence Score: 5/5Safe to merge — changes are a targeted naming normalization with no logic regressions. All three migration scenarios (only legacy, only canonical, both simultaneously) are handled correctly in the No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Run merge-mcp-config.js --update-mcp] --> B{Check existing config}
B --> C{context7 canonical exists?}
C -->|No| D{context7-mcp legacy exists?}
C -->|Yes| E{context7-mcp legacy also exists?}
D -->|No| F[ADD new context7 section]
D -->|Yes| G[REMOVE context7-mcp + REMOVE context7 if present, ADD context7]
E -->|No| H[REMOVE context7, ADD context7]
E -->|Yes| I[REMOVE context7, REMOVE context7-mcp, ADD context7]
F --> J[Write config.toml]
G --> J
H --> J
I --> J
Reviews (3): Last reviewed commit: "fix: clean up legacy Context7 aliases on..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Addressed the review feedback in a follow-up commit. Changes:
Verification:
|
|
Follow-up fixes are pushed and the bot feedback is addressed. Happy to make any further adjustments if you want the scope tightened. |
…check The global sanity check (check-codex-global-state.sh) hard-fails when persistent_instructions is missing from ~/.codex/config.toml, but neither the baseline .codex/config.toml nor the sync script ever define this field. This causes a clean install to report a failing sanity check even though the sync otherwise succeeds (affaan-m#967). - Add persistent_instructions to the baseline .codex/config.toml so that users who cp the config get a working default. - Downgrade the sanity check from fail to warn, since persistent_instructions is additive and optional — users who rely solely on AGENTS.md should not be blocked. Fixes affaan-m#967 (persistent_instructions part; context7 naming addressed by affaan-m#970) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
affaan-m
force-pushed
the
codex/context7-consistency
branch
from
59636b6 to
432788d
Compare
2 participants
Summary
This PR fixes a small Codex inconsistency around Context7 naming.
ECC's reference Codex config used
[mcp_servers.context7], while the merge and sanity-check scripts treated[mcp_servers.context7-mcp]as canonical. This patch makescontext7the canonical Codex section name everywhere, while keeping@upstash/context7-mcpas the launcher package.Changes
[mcp_servers.context7][mcp_servers.context7-mcp]entries as aliases during updatescontext7Verification
git diff --checkbash -n scripts/codex/check-codex-global-state.shnodewas not available in my local environment, so I did not run the merge script itself.Summary by cubic
Normalize Codex Context7 naming so
[mcp_servers.context7]is canonical across code and docs, while keeping the launcher package@upstash/context7-mcp. On update, legacy[mcp_servers.context7-mcp]entries are migrated to the canonical section and removed.Bug Fixes
[mcp_servers.context7]; treat[mcp_servers.context7-mcp]as a legacy alias.merge-mcp-config.js,--update-mcpreplaces the canonical block and cleans up any legacy alias.scripts/codex/check-codex-global-state.sh, docs, and examples to expectcontext7and warn on legacy.Migration
--update-mcpconverts and removes[mcp_servers.context7-mcp].Written for commit 432788d. Summary will update on new commits.
Summary by CodeRabbit
Documentation
Chores