fix: make Codex Desktop worktree setup Windows-safe - #1565
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 12 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdded a Codex Desktop worktree setup command. It validates runtimes and lockfiles, reuses complete matching dependencies, falls back to locked installation, installs hooks, and documents Windows and Cloud setup boundaries. ChangesCodex Desktop worktree setup
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #7739 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![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: e1d928f292
ℹ️ 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".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch codex/fix-windows-worktree-setup at starting commit e1d928f; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:codex/fix-windows-worktree-setup, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/scripts-index.md (1)
1-9: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd
setup-codex-worktree.mjsto the itemized script list.The header states: "Every top-level
.mjs/.ts/.cjsscript is named below; the remaining files are fixtures, SQL, subfolder helpers, and small shared helpers grouped rather than itemised." The newscripts/setup-codex-worktree.mjsis a top-level.mjsscript, but it isn't named in any category in this file, even though the counts on line 3 were bumped to include it. This breaks the document's own stated completeness contract. It fits naturally alongsidecheck-installed-lock-parity.mjsandinstall-git-hooks.mjsin "Runner & guard infrastructure [infra]," since it calls both.📝 Proposed addition to the Runner & guard infrastructure table
| `check-installed-lock-parity.mjs`, `phone-chrome-plan.mjs`, `verify-phone-chrome.mjs`, `playwright-browser-preflight.mjs` | Lock-trust preflight, change-scoped phone contracts, and Playwright browser-binary preflight before build | +| `setup-codex-worktree.mjs` | Windows Codex Desktop worktree bootstrap: validates Node/npm, reuses a byte-identical local install or runs locked `npm ci`, then installs Git hooks |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/scripts-index.md` around lines 1 - 9, Add setup-codex-worktree.mjs to the itemized “Runner & guard infrastructure [infra]” table in docs/scripts-index.md, alongside check-installed-lock-parity.mjs and install-git-hooks.mjs. Preserve the existing counts and table organization while ensuring the top-level script is explicitly listed.
🧹 Nitpick comments (2)
README.md (1)
23-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReference AGENTS.md instead of restating the Cloud-script prohibition.
Lines 26-27 restate the same policy that AGENTS.md's
codex-desktop-worktree-setupsection already states: don't run the Cloud-only Bash setup script on Windows Desktop. Having the same rule phrased separately in two files risks the two drifting apart if the policy changes later.As per coding guidelines, "Treat
AGENTS.mdas the single source of truth for repository rules; orientation documents must not duplicate policy, and conflicts defer toAGENTS.md." README.md is the onboarding/orientation document here, so it should link to the AGENTS.md section rather than restate the rule.♻️ Proposed rewording to reference AGENTS.md
-Codex Desktop worktrees use `npm run setup:codex-worktree`. The local bootstrap -reuses dependencies only from a complete worktree with a byte-identical lockfile, -then validates the installed metadata. It falls back to the locked install above -when no safe local donor exists. Do not configure Windows Desktop worktrees to run -the Cloud-only Bash setup script. +Codex Desktop worktrees use `npm run setup:codex-worktree`. The local bootstrap +reuses dependencies only from a complete worktree with a byte-identical lockfile, +then validates the installed metadata. It falls back to the locked install above +when no safe local donor exists. See `AGENTS.md`'s Codex Desktop worktree setup +section for the Windows-vs-Cloud setup boundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 23 - 27, Update the README guidance around Codex Desktop worktree setup to remove the duplicated Windows Cloud-only Bash prohibition and instead reference the `codex-desktop-worktree-setup` section in `AGENTS.md` as the authoritative policy. Preserve the surrounding setup and fallback instructions.Source: Coding guidelines
tests/setup-codex-worktree.test.ts (1)
6-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider covering
assertRuntime,copyDependencies, andmain().Only
parseWorktreeList,lockDigest,installedMetadataMatches,installationIsComplete,findDependencyDonor, andresolveNpmCliare imported and tested here.assertRuntime,copyDependencies,removePartialInstall, andmainare not exported fromscripts/setup-codex-worktree.mjs, so they cannot be unit-tested directly. These functions encode the runtime-validation and donor-copy guarantees the PR is built around (Node/npm version matching, Windows robocopy vs. POSIXcpSyncexclusions, post-copy verification). Exporting them for direct testing, or adding an integration-level test that mocksspawnSync/filesystem calls, would close this gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/setup-codex-worktree.test.ts` around lines 6 - 13, Expand coverage for the untested assertRuntime, copyDependencies, removePartialInstall, and main flows in setup-codex-worktree.mjs. Either export these symbols for direct unit tests or add integration tests that mock spawnSync and filesystem operations, covering runtime version validation, platform-specific donor copying and exclusions, partial-install cleanup, and post-copy verification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/setup-codex-worktree.test.ts`:
- Around line 81-83: Gate the resolveNpmCli() assertion in the test so it runs
only on Windows, where the npm JavaScript entrypoint expectation applies.
Preserve the existing regex assertion for Windows and skip it on Unix platforms
where resolveNpmCli() may return null.
---
Outside diff comments:
In `@docs/scripts-index.md`:
- Around line 1-9: Add setup-codex-worktree.mjs to the itemized “Runner & guard
infrastructure [infra]” table in docs/scripts-index.md, alongside
check-installed-lock-parity.mjs and install-git-hooks.mjs. Preserve the existing
counts and table organization while ensuring the top-level script is explicitly
listed.
---
Nitpick comments:
In `@README.md`:
- Around line 23-27: Update the README guidance around Codex Desktop worktree
setup to remove the duplicated Windows Cloud-only Bash prohibition and instead
reference the `codex-desktop-worktree-setup` section in `AGENTS.md` as the
authoritative policy. Preserve the surrounding setup and fallback instructions.
In `@tests/setup-codex-worktree.test.ts`:
- Around line 6-13: Expand coverage for the untested assertRuntime,
copyDependencies, removePartialInstall, and main flows in
setup-codex-worktree.mjs. Either export these symbols for direct unit tests or
add integration tests that mock spawnSync and filesystem operations, covering
runtime version validation, platform-specific donor copying and exclusions,
partial-install cleanup, and post-copy verification.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: efd6c771-cb1f-473d-a959-6d8b263ede7c
📒 Files selected for processing (7)
AGENTS.mdREADME.mddocs/codex-cloud.mddocs/scripts-index.mdpackage.jsonscripts/setup-codex-worktree.mjstests/setup-codex-worktree.test.ts
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
🔒 Upgrade RequiredPrivate repository analysis requires Pro or Enterprise. Upgrade: https://ecc.tools/pricing?plan=pro ECC Tools keeps the core app open, and puts private repos, team features, and enterprise controls behind paid tiers. |
1 participant
Summary
node_modulesexistsnpm ci --include=devWhy
The Database Codex environment was configured to invoke the Linux-only Cloud Bash setup from Windows. Codex launched that command through WSL outside the Windows worktree, so new worktrees were left without dependencies and repeatedly failed. The effective local Codex environment selection has also been corrected to use the Windows launcher; this PR makes the durable repository contract available to every future worktree after merge.
Files and areas
scripts/setup-codex-worktree.mjstests/setup-codex-worktree.test.tspackage.jsonAGENTS.md,README.md,docs/codex-cloud.md,docs/scripts-index.mdVerification
node scripts/run-vitest.mjs run tests/setup-codex-worktree.test.ts tests/installed-lock-parity.test.ts --reporter=dot: 2 files passed, 11 tests passednode scripts/setup-codex-worktree.mjs --dry-run: existing dependencies matchpackage-lock.jsonnpm run format: exit 0; whole tree unchangednpm run format:changed: PASSnpm run verify:pr-local -- --files ...: runtime, installed-lock parity, formatting, full lint, and typecheck completed; the repository-wide Vitest phase stopped producing progress and never emitted a verdict, so the task-owned run was terminatedChecks not completed
verify:pr-localis sequential, its later build and RAG fixture steps were not reachedRAG impact: no retrieval behaviour change — worktree setup, tests, and documentation only.
Risk and follow-up
The generated Codex Desktop environment TOML is machine-local and intentionally not committed. This machine's effective Database environment selection now points to the durable Windows launcher, while the tracked setup script and instructions prevent the Linux Cloud command from being reused for future Windows worktrees.
Summary by CodeRabbit
New Features
Documentation