fix(codex): add persistent_instructions to baseline and relax sanity check#974
Conversation
…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>
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a top-level Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Greptile SummaryThis PR makes two small, complementary fixes for
Confidence Score: 5/5Safe to merge — changes are additive and only relax a non-critical sanity check. Both changes are small, targeted, and well-tested by the author. The TOML field is additive, the shell logic is straightforward, and no existing behavior is broken. All findings are P2 or below. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[check-codex-global-state.sh] --> B{CONFIG_FILE exists?}
B -- No --> Z[skip config checks]
B -- Yes --> C[check_config_pattern: multi_agent enabled]
C --> D[check_config_absent: collab flag absent]
D --> E{persistent_instructions set?}
E -- Yes --> F["[OK] persistent_instructions is configured"]
E -- No --> G["[WARN] not set — recommended but optional"]
F --> H[check profiles.strict / profiles.yolo]
G --> H
H --> I[check MCP server sections]
Reviews (2): Last reviewed commit: "fix(codex): allow leading whitespace in ..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/codex/check-codex-global-state.sh`:
- Around line 94-97: The rg pattern used to detect persistent_instructions
currently anchors at line start and will miss indented TOML entries; update the
check in the if that runs rg against "$CONFIG_FILE" to allow leading whitespace
(e.g. use a regex like '^[[:space:]]*persistent_instructions\s*=' or
'^\s*persistent_instructions\s*=') so indented lines are detected and the
warn/ok branch is correct; keep the rest of the surrounding logic (the ok/warn
messages and use of CONFIG_FILE) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5fbb3f3-73d8-46c2-b825-a023cfe6485c
📒 Files selected for processing (2)
.codex/config.tomlscripts/codex/check-codex-global-state.sh
The rg pattern anchored at line start (^persistent_instructions) would miss indented TOML entries. Use ^\s* prefix to match both top-level and indented configurations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
2 participants
Summary
persistent_instructionsto the baseline.codex/config.tomlso thatcp .codex/config.toml ~/.codex/config.tomlproduces a valid global configpersistent_instructionssanity check from hard[FAIL]to[WARN], since this field is additive and optional — users who rely solely on AGENTS.md should not be blockedPartially fixes #967 (the
persistent_instructionspart; context7 naming is addressed by #970).Type
Changes
.codex/config.tomlAdded
persistent_instructionswith a sensible default and a comment explaining its additive nature (unlikemodel_instructions_filewhich replaces AGENTS.md):scripts/codex/check-codex-global-state.shReplaced the hard
check_config_patterncall (which usesfail()) with an inline check that useswarn():Testing
Syntax validation
End-to-end sanity check comparison
Before (original code) — persistent_instructions shows [FAIL]
After (fixed code) — persistent_instructions shows [WARN] instead of [FAIL]
Key difference: Line 7 changed from
[FAIL] persistent_instructions is configuredto[WARN] persistent_instructions is not set (recommended but optional). The other failures are unrelated to this PR (incomplete local ECC setup).For a fully synced environment, users who
cp .codex/config.toml ~/.codex/config.tomlwill now get[OK] persistent_instructions is configuredbecause the baseline includes the field. Users who have a custom global config withoutpersistent_instructionswill see a warning instead of a blocking failure.Checklist
fix(codex):)bash -n)tomllib)Summary by cubic
Make
persistent_instructionspart of the baseline config and downgrade its sanity check from FAIL to WARN. The check now detects indented entries so valid configs aren’t missed.persistent_instructionsto.codex/config.toml(additive; does not replace AGENTS.md).scripts/codex/check-codex-global-state.shto warn whenpersistent_instructionsis missing instead of failing.persistent_instructions.Written for commit 4b4f077. Summary will update on new commits.
Summary by CodeRabbit