[lidge-jun]

Summary

• Make Windows ownership-removal fixtures exercise the unelevated owner path they assert. Elevated Windows uninstall deliberately refuses any junction or other reparse point, so ambient administrator rights made two unrelated junction assertions return refused instead of their expected partial or removed result.
• Add a real Windows junction security regression with an elevated no-junction control: the junction must be refused before ACL evaluation or any owned/external state changes. The ACL runner is stubbed only for this controlled test. See Microsoft's reparse point documentation for the underlying Windows link behavior.

Verification

• Windows before: bun test tests/config/config-ownership-uninstall.test.ts produced 8 pass, 3 existing platform skips, and 14 fail on an elevated host; the two junction cases matched CI's refused failures. The case-insensitive-path fixture also received refused in the security file. Windows bisect identified fdad3daeab0e09c579c15e1178bf7355a1617603 (fix(config): remove owned paths relative to the config root #8141), with matching cases passing at 6f915d865f and failing at 0a30c55130.
• A temporary probe forced unelevated execution: both junction tests passed with all 8 assertions, and the full uninstall file passed 22 tests with the same 3 platform skips. Forcing elevated restored both failures. The host's real token probe reports elevated; a no-junction control was still refused after one successful invocation of the real ACL subprocess. The specific ACL entry responsible was not identified.
• Windows after: the two focused files passed 30 tests, with 31 existing platform skips and 156 assertions; the strengthened junction case then passed with 21 assertions. It checks refusal and ACL ordering, no new ownership lock or directory entries, and unchanged owner, manifest, config, junction target and external target.
• bun run typecheck — pass; bun run test:changed — 30 pass, 31 existing platform skips, 0 fail, 160 assertions; bun run privacy:scan — pass.
• Full local suite not run because this Windows host is shared by another lane; focused tests and test:changed ran locally, and broader coverage is left to CI. Linux/macOS local runs and hosted Windows proof have not yet run.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

• Tests
  • Expanded Windows test coverage for configuration removal, including behavior when the configuration tree contains a junction and when removal runs without elevation.
  • Added checks that junctions, ownership records, manifests, and external targets remain intact when elevated removal is refused.

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-06T05:26:03.249669Z	2d241b4	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.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The tests add Windows coverage for elevated uninstall refusal when an owned config directory contains a junction. They also set the uninstall elevation probe to the unelevated state for tests that exercise that removal path.

Changes: Windows config uninstall tests

Layer / File(s)	Summary
Junction refusal and test isolation tests/config/config-ownership-removal.test.ts, tests/config/config-ownership-uninstall.test.ts	A Windows-only test verifies that elevated removal refuses a config tree containing a junction without reading ACLs or changing the config directory, ownership files, manifest, junction, or external target. The uninstall suite sets and resets the elevation probe for the unelevated path. The case-insensitive-volume test also sets the probe to unelevated before removal.

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

Change: Other

Possibly related PRs

• lidge-jun/opencodex#8141: Adds the elevated Windows junction refusal behavior and the test probe used by this PR.

Suggested labels: chore

Merge Risk: Minimal · up to 2d241

No actionable merge-blocking risk is identified in these test-only changes. Normal platform checks should complete before merging.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title accurately describes the isolation of Windows uninstall privilege scenarios in the test changes.
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• 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

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: lidge-jun/opencodex
• Branch: codex/fix-windows-uninstall-junction
• File: .coderabbit.yaml
• Commit: 2d241b4
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 21e75f1d-1c82-4420-9a51-d6c7709555e3

Commits

Reviewing files that changed from the base of the PR and between 650cbc6 and 2d241b4.

Files selected for processing: 2

• tests/config/config-ownership-removal.test.ts
• tests/config/config-ownership-uninstall.test.ts

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

[github-actions]

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

Maintainer integration into dev under MAINTAINERS.md (lidge-jun, admin).

• Checked head: 2d241b4; all 26 PR checks on this head passed; no unresolved review threads.
• Windows proof: branch lane=all run https://github.com/lidge-jun/opencodex/actions/runs/42168265793 on this exact head — shard 5/11 SUCCESS, which runs tests/config/config-ownership-uninstall.test.ts with no failures (it failed on dev in the same shard). The run as a whole was cancelled after other shards finished.
• Review: the junction uninstall tests now exercise the unelevated path explicitly and assert the elevated refusal separately, preserving the fix(config): remove owned paths relative to the config root #8141 elevated Windows no-link security policy and owned config/ownership metadata.