Repository navigation
fix(codex-cloud): unbreak CI and close open review findings on #3587 - #3588
Conversation
- Treat a blank HEALTH_DEEP_PROBE_SECRET as unset. The offline test wrapper now blanks it, and the strict min(16) rule made every route importing env throw, which failed the unit-coverage and Playwright build jobs. A non-blank value must still be 16+ characters, and an unset secret keeps the deep probe closed. - Shim installer: require Node major 24 (22.x and 20.x no longer pass the fallback check), and skip $HOME/.local/bin in every PATH lookup so a shim can never resolve to itself. - Profile relocation: drop the old separator so reruns do not add blank lines. - Bring the contract checker and tests in line with the shim and profile changes already on this branch. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
BigSimmo has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
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. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
codeant-ai
Bot
added
the
size:M
PR Summary by QodoRestore Codex Cloud CI and make shim setup safe to rerun
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
No findings for this group 1. Working-directory commands disappear
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…lob characters The loop condition matched "$HOME/.local/bin" literally, but the replacement treated $HOME as a pattern, so a home like /tmp/user[1] left the entry in place and the loop never ended. Quote the pattern, and add a regression test that hangs on the old code. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA
|
Code review by qodo was updated up to the latest commit 702b5b9 |
Auto-approval revoked — new findings require attention.
|
|
Qodo Fixer✅ Committed (1) · ☑ Fixed (1) Commits pushed directly to this PR — no separate fix PR opened. Process — 1 fixed
|
…tup contract check The shim PATH cleanup now compares each entry literally (no glob replacement), so the static contract check looks for that form instead of the old replacement text. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA
Code Review ✅ Approved 1 closed / 1 findings🟡 Medium risk · Treating blank HEALTH_DEEP_PROBE_SECRET as unset could leave deep-probe checks unavailable, while shim PATH and Node constraints alter setup behavior. Fixes the Codex Cloud shim installer and blank health-probe secret handling that were breaking CI, with shim PATH lookups no longer resolving to themselves and shell profile reruns no longer adding blank lines. The generated shims' glob-pattern hang, previously flagged, has been addressed. No open issues found. ✅ 1 closed✅ Bug: Generated shims keep the same glob-pattern hang the installer fix removed
VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Approved For more verification details, comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Important Your trial ends in 5 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more. Was this helpful? React with 👍 / 👎 | Gitar |
| clean_path="\${clean_path//:\$HOME\/.local\/bin:/:}" | ||
| clean_path="" | ||
| path_entry_seen=false | ||
| IFS=: read -r -a path_entries <<< "\$PATH" |
There was a problem hiding this comment.
1. Working-directory commands disappear 🐞 Bug ≡ Correctness
The generated shim rebuilds clean_path with IFS=: read -r -a, which drops a trailing empty PATH entry rather than preserving it. When the inherited PATH ends in :, the shim removes the working directory from command lookup for child processes; the installer’s matching fallback_path loop also loses it during fallback lookups.
Agent Prompt
## Issue description
The new PATH-splitting loops discard a trailing empty entry, changing command lookup semantics for a PATH ending in `:`.
## Fix Focus Areas
- scripts/install-codex-cloud-command-shims.sh[34-45]
- scripts/install-codex-cloud-command-shims.sh[81-92]
## Recommended Fix
Filter the shim directory using a literal comparison while preserving every other PATH component, including trailing empty components. Apply the same approach to the installer and generated wrapper, and test a PATH ending in `:`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 14aa2a5 |
…3589) * docs(codex-cloud): retire xhigh prompt marker gate and clarify raw-env local policy repair - Retire the general [xhigh-confirmed] prompt marker and resubmission gate in docs/agents/codex-reasoning-effort.md, AGENTS.md, smart-agent-allocation.md, and retired-rules.json while preserving specialist RAG CLI checks - Clarify in docs/agents/codex-cloud-environment.md, docs/codex-cloud.md, and scripts/check-codex-cloud-raw-env.sh that failed raw-env checks still block app execution and live providers while permitting explicitly requested local policy/doc repair and scrubbed offline checks - Document how tracked repo instructions, Cloud environment settings, and active chat/process state update on separate lifecycles Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(ledger): record review for codex/cloud-policy-clarity Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: Clarify Cloud setting refresh behavior * chore(cloud): remove all remaining xhigh marker references and scrub GH_TOKEN cleanly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(ledger): record review for bdb0cea on codex/cloud-policy-clarity Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(codex-cloud): address review threads on raw-env exit semantics and GH_TOKEN warning * fix(codex-cloud): harden shell profile sourcing, command shims, and offline env scrubbing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(ledger): record review for ce6c8a8 on codex/cloud-policy-clarity Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(codex-cloud): add JSDoc env type annotations to shouldAutoSourceCloudProfile Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(ledger): record review for 51e6077 on codex/cloud-policy-clarity Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(cloud): remove xhighConfirmed from RAG phase receipt schemas and refine GH_TOKEN raw-env warning Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(ledger): record review for 3e0876f on codex/cloud-policy-clarity Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: Relocate existing Cloud profile blocks * fix: 3 findings — Validate Node version before creating shims; Allow PAT - Validate Node version before creating shims - Allow PATH fallback when nvm has no Node - Resolve split Node command installations * fix: Report Cloud profile re-exec failures * fix(codex-cloud): unbreak CI and close open review findings on #3587 (#3588) * fix(codex-cloud): unbreak CI and close open review findings on #3587 - Treat a blank HEALTH_DEEP_PROBE_SECRET as unset. The offline test wrapper now blanks it, and the strict min(16) rule made every route importing env throw, which failed the unit-coverage and Playwright build jobs. A non-blank value must still be 16+ characters, and an unset secret keeps the deep probe closed. - Shim installer: require Node major 24 (22.x and 20.x no longer pass the fallback check), and skip $HOME/.local/bin in every PATH lookup so a shim can never resolve to itself. - Profile relocation: drop the old separator so reruns do not add blank lines. - Bring the contract checker and tests in line with the shim and profile changes already on this branch. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA * fix(codex-cloud): stop shim PATH cleanup hanging on home paths with glob characters The loop condition matched "$HOME/.local/bin" literally, but the replacement treated $HOME as a pattern, so a home like /tmp/user[1] left the entry in place and the loop never ended. Quote the pattern, and add a regression test that hangs on the old code. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA * fix: Remove shim PATH entries using literal comparison * fix(codex-cloud): match the literal-comparison PATH cleanup in the setup contract check The shim PATH cleanup now compares each entry literally (no glob replacement), so the static contract check looks for that form instead of the old replacement text. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com> * test(codex-cloud): policy-only harness now expects the Cloud block in shell profiles #3587 writes the Cloud profile source into .bashrc/.profile/.bash_profile before the policy-only harness stops, so the relocation can be exercised without a real runtime. The harness isolation test still asserted those files were never created, which failed both of its cases in unit coverage partition 2. Assert the block is present instead; nothing outside the redirected HOME changes and no runtime is probed or installed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA * fix: Assert the complete Cloud profile block at the top * fix(codex-cloud): align the policy-only harness test with profile prepending and check the npm fallback The policy-only harness test still asserted that setup leaves .bashrc, .profile and .bash_profile untouched, but this PR now writes the Cloud profile block to the top of each before the harness stops (its own setup test depends on that). Assert the new behaviour: the block is first in every profile, and the runtime-call log still shows no runtime was probed or installed. This is what failed Unit coverage partition (2). Also address the open CodeRabbit finding on the shim installer: when the chosen Node install has no npm or npx and the installer falls back to PATH, require the repository's npm major (package.json engines.npm) instead of accepting any version. Adds a test for both outcomes. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hg9knRJzMaDsgMgtSHmtyh * fix(codex-cloud): quote Node paths in shim installer version probes A supported Node under a directory containing spaces was word-split during the version check, so setup failed before writing the shims. Adds a test with a spaced home directory (fails on the old code, passes now). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
User description
Summary
Follow-up commit for #3587, stacked on its branch (
codex/cloud-policy-clarity). Merge this into that branch to turn #3587's CI green.HEALTH_DEEP_PROBE_SECRET, butsrc/lib/env.tsrequired 16+ characters, so any route importingenvthrew at load. That failed both unit-coverage partitions (93 test files) and the Playwright Next build. A blank value is now treated as unset (same pattern asPROXY_AUTH); a non-blank value under 16 characters is still rejected, and an unset secret keeps the deep probe closed.$HOME/.local/binso a shim cannot resolve to itself.check-codex-cloud-setup.mjsandtests/codex-cloud-setup.test.tsstill described the older shim/profile text; updated to match.Verification (local, offline)
tests/codex-cloud-setup.test.ts+tests/codex-cloud-diagnose.test.ts: 50 passedtests/env.test.ts(2 new cases),tests/work-sync.test.ts,tests/setup-status-route.test.ts: 39 passednode scripts/check-codex-cloud-setup.mjs: PASS;tsc --noEmitand eslint on changed files: cleangenerate-repo-awareness.mjs --check: snapshot already in step (the CodeRabbit snapshot comment is stale)RAG impact: no retrieval behaviour change — Cloud environment scripts and an env-schema parse rule only.
🤖 Generated with Claude Code
https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA
Generated by Claude Code
Summary by cubic
Treats a blank
HEALTH_DEEP_PROBE_SECRETas unset, preventing offline test runs from failing when routes load the environment schema. Non-blank secrets still require at least 16 characters, and an unset secret keeps the deep probe closed.$HOME/.local/binin PATH lookups to prevent shims from resolving to themselves.Written for commit 14aa2a5. Summary will update on new commits.
CodeAnt-AI Description
Fix Codex Cloud setup and blank health-probe secrets
What Changed
Impact
✅ Fewer startup failures with blank probe secrets✅ Codex Cloud commands avoid shim recursion✅ Cleaner shell profiles after repeated setup💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.