Repository navigation
fix(codex-cloud): quote Node paths in shim installer version probes - #3589
Conversation
…v 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>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…GH_TOKEN cleanly Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nd GH_TOKEN warning
…ffline env scrubbing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… codex/cloud-policy-clarity Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…loudProfile Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d refine GH_TOKEN raw-env warning Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Validate Node version before creating shims - Allow PATH fallback when nvm has no Node - Resolve split Node command installations
…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>
… 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
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![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
|
|
Warning Review limit reachedYour organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Next included review available in 17 minutes. View limit details
Comment |
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:XS
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
PR Summary by QodoExpect Cloud source in policy-only harness shell profiles
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
No findings for this group No findings for this group No findings for this group Resolved findings 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
|
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
CodeAnt Nitpicks1 code suggestion1. The profiles start empty, and this only checks that the source appears somewhere. A later early return in
|
CodeAnt PR Risk: Low Risk
Assessed commit: |
Qodo Fixer✅ Committed (1) · ☑ Fixed (1) Commits pushed directly to this PR — no separate fix PR opened. Process — 1 fixed
|
…pending 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
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA
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
BigSimmo
changed the title
qodo-code-review
Bot
added
🟠 Medium blast radius
and removed
🟡 Small blast radius
labels
|
Code review by qodo was updated up to the latest commit 94a24f7 |
The base branch was changed.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA
|
Auto-fix isn't included in your plan ↗ |
|
No code changes since the last review — review skipped |
Code Review ✅ Approved🟡 Medium risk · Quoting Node paths changes shim version probing; path-resolution mistakes could reject a supported runtime and leave Codex Cloud shims unavailable. Quotes Node binary paths in the 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 |
Summary
Follow-up to #3587, which has now merged into
main. This PR targetsmainand contains only one small fix.scripts/install-codex-cloud-command-shims.sh: the two Node version probes ($fallback_nodeand$node_bin/node) are now quoted, so a supported Node under a directory containing spaces is no longer word-split and rejected before the shims are written.tests/codex-cloud-setup.test.ts: new test that runs the installer with a home directory containing spaces. It fails on the old code and passes now.Answers one review finding on #3587 that was still open when it merged; the other three were checked and left as they are, with reasons on their threads.
Verification (local, offline)
tests/codex-cloud-setup.test.ts+tests/codex-cloud-policy-harness.test.ts: 53 passednode scripts/check-codex-cloud-setup.mjspassed earlier on the same codeRAG impact: no retrieval behaviour change — setup script and its test only.
🤖 Generated with Claude Code
https://claude.ai/code/session_016NaMorGX7ibPrednN1p5qA