[WRONG BRANCH] fix(service): accept legacy WSL ownership state - #262
[WRONG BRANCH] fix(service): accept legacy WSL ownership state#262luvs01 wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Comment |
|
✅ Deterministic PR hygiene checks passed. |
![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: 114ece2949
ℹ️ 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".
| const env = deps.env ?? process.env; | ||
| if (env.CODEX_HOME?.trim() || !isWslRuntime(deps)) return false; | ||
| const legacyDefault = join((deps.homedir ?? homedir)(), ".codex"); | ||
| return serviceHomeMatches(recordedHome, legacyDefault); |
There was a problem hiding this comment.
When a WSL service was intentionally installed with CODEX_HOME=/home/$USER/.codex, but that home has no config.toml—for example, because Codex integration was disabled—a later shell with CODEX_HOME unset can discover the sole Windows home. This fallback cannot distinguish that explicit install record from legacy implicit metadata and returns true, so repair silently rewrites the unit and state for the Windows home, while stop/uninstall restore the wrong home. Only accept the compatibility case when the installed launcher/state proves the original home was implicit.
Useful? React with 👍 / 👎.
| const expected = normalizePathForCompare(state.codexHome); | ||
| const actual = normalizePathForCompare(actualCodexHome); | ||
| if (expected !== actual) { | ||
| if (!serviceCodexHomeMatchesInstall(state.codexHome)) { |
There was a problem hiding this comment.
In WSL, home discovery depends on live config.toml files and candidate directories, but assertServiceEnvironmentMatchesInstall() resolves actualCodexHome and then this call resolves it again inside serviceCodexHomeMatchesInstall(). If candidates change between those reads, the check can reject a matching record while reporting identical installed/current paths, or accept based on a different snapshot. Pass the already-resolved actualCodexHome into the compatibility comparison so ownership and diagnostics use the same result.
Useful? React with 👍 / 👎.
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
1 participant
Motivation
~/.codex) from being treated as foreign after runtime WSL home discovery changed to a Windows Desktop path.CODEX_HOMEauthoritative.Description
serviceCodexHomeMatchesInstall(recordedHome, deps)which accepts an exact legacy~/.codexrecord when running under WSL withCODEX_HOMEunset and otherwise enforces strict path equality.assertServiceEnvironmentMatchesInstall()to use the compatibility-aware comparator forcodexHomewhile preserving the existingOPENCODEX_HOMEcomparison logic.tests/codex-home-wsl.test.tsthat verifies WSL Windows-home discovery and the legacy-Linux fallback acceptance and non-acceptance cases.structure/02_config-and-codex-home.mdand leave all other foreign ownership records unmigrated.Testing
./node_modules/.bin/bun test tests/codex-home-wsl.test.tswhich passed (6/6)../node_modules/.bin/bun run typecheckand./node_modules/.bin/bun run privacy:scanwhich succeeded../node_modules/.bin/bun test tests/service.test.ts --timeout 15000which showed 112 passing service tests and one unrelated pre-existing assertion failure in this checkout.git diff --checkand committed the change asfix(service): accept legacy WSL ownership state.Codex Task