Skip to content

test(service): expect the env-indirect Bun path in Windows preflight wrappers - #6852

Merged
lidge-jun merged 1 commit into
devfrom
codex/preflight-tests-env-indirect-paths
Oct 10, 2026
Merged

lidge-jun merged 1 commit into
devfrom
codex/preflight-tests-env-indirect-paths

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

Three #6812 preflight tests fail on a normal Windows development machine and pass in CI:

  • Windows shim selected-runtime preflight > fresh install admits once before the first move and freezes all wrapper rendering
  • Windows shim selected-runtime preflight > automatic refresh bakes the frozen admitted path/source after selection changes
  • service selected-runtime admission > scheduler composed install never admits again after stop

They assert that the generated .cmd contains the literal process.execPath. Both generators deliberately write a path under the user profile as a %LOCALAPPDATA% / %USERPROFILE% token (windowsEnvIndirectBatchValue in src/lib/win-paths.ts, used by src/codex/shim-templates.ts and src/service/windows-taskxml.ts) so non-ASCII profile names survive the OEM code page. CI's Bun lives in the tool cache, outside the profile, so the literal and the rendered form coincide there; with Bun under %USERPROFILE%\.bun or a checkout under %LOCALAPPDATA% they differ, e.g. set "OCX_BUN=%LOCALAPPDATA%\Temp\…\node_modules\bun\bin\bun.exe".

The assertions now expect the complete set "OCX_BUN=…" line rendered the way the generators render it (same env-token prefix and the same %/^/" suffix escaping). On macOS, Linux and CI the expected line still contains the literal path; the frozen-runtime and unprobed.exe checks are unchanged. Test-only change. (The CodeRabbit summary below overstates the coverage: the tests use process.execPath, and no fixture contains %, ^ or "; the escaping is mirrored so such a path would also match, but it is not exercised.)

Verification

  • Windows 11 (mini, pinned Bun 1.4.2 under %LOCALAPPDATA%, current dev clone, isolated OPENCODEX_HOME/CODEX_HOME): bun test tests/codex-integration/codex-shim-runtime-preflight.test.ts tests/service/service-runtime-preflight.test.ts → before 28 pass / 3 fail (the three tests above); with this PR 31 pass / 0 fail, twice.
  • macOS: the same two files → 31 pass / 0 fail; bun run typecheck clean.
  • Not run: the full bun run test suite — the change is confined to three assertions in two test files. Ordinary PR CI skips the Windows shards, so the mini run is the Windows evidence.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (Test-only change; none needed.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (No product code changes.)

Summary by CodeRabbit

  • Tests
    • Expanded Windows integration coverage to verify that runtime paths containing special batch characters are correctly represented in command wrappers and scheduled scripts.
    • Added checks for both fresh installations and automatic runtime refreshes. These updates strengthen validation of Windows runtime handling without changing user-facing functionality.

…wrappers

The Windows shim and service-script generators write a Bun that lives under the user profile as a %LOCALAPPDATA%/%USERPROFILE% token (src/lib/win-paths.ts), but three preflight assertions expected the literal process.execPath. They passed on CI, whose Bun sits in the tool cache, and failed on any Windows machine with Bun in the default profile location. The assertions now expect the complete `set "OCX_BUN=..."` line rendered the way the generators render it.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 9, 2026 21:59
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T22:00:39.636742Z f53bf8d PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Oct 9, 2026
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: b55c0b23-b880-4ba1-a976-edfde158d0f8

📥 Commits

Reviewing files that changed from the base of the PR and between 006cec9 and f53bf8d.


📒 Files selected for processing (2)
  • tests/codex-integration/codex-shim-runtime-preflight.test.ts
  • tests/service/service-runtime-preflight.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.



📝 Walkthrough

Walkthrough

The Windows runtime preflight tests now expect batch-escaped runtime paths in shim wrappers and scheduler scripts. They use windowsEnvIndirectBatchValue to build those expected values.

Changes

Windows runtime preflight tests

Layer / File(s) Summary
Batch-escaped runtime expectations
tests/codex-integration/codex-shim-runtime-preflight.test.ts, tests/service/service-runtime-preflight.test.ts
The shim integration tests use an encoded bunLine() expectation for fresh installs (line 89) and automatic refresh (line 114). The service test checks for an escaped OCX_BUN assignment (line 123). Both test files define bunLine() using windowsEnvIndirectBatchValue (shim test lines 10–15; service test lines 15–21).

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other


Merge Risk: ⚪ Minimal · up to f53bf

The Windows preflight assertions now match the shim and scheduler output for profile-based, batch-escaped runtime paths. No actionable merge-blocking risk remains.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: updating Windows preflight wrapper tests to expect the environment-indirect Bun path.


✨ Finishing Touches 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun
lidge-jun merged commit f2b143b into dev Oct 10, 2026
74 of 77 checks passed
@lidge-jun
lidge-jun deleted the codex/preflight-tests-env-indirect-paths branch October 10, 2026 06:01
lidge-jun added a commit that referenced this pull request Oct 10, 2026
…#6855)

* docs(devlog): record the cross-OS probe of the desktop-owned CLI path

Plan, per-host execution notes and outcomes for probing #6802, #6807, #6809, #6812, #6816 and #6818 on Ubuntu 24.04 and Windows 11. Follow-ups: #6851, #6852, #6853, #6854.

* docs(devlog): tighten evidence citations and the F1 hypothesis

* docs(devlog): qualify unretained evidence in the probe outcome

* docs(devlog): record exact-head CI for the probe follow-ups

* docs(devlog): close the cross-OS probe unit and fold review fixes

* docs(devlog): align the P2 escape note and guard the probe cleanup paths
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant