fix(windows): preserve scheduled-task elevation launcher arguments

[ulrich-zogo]

Summary

• Keep -FilePath, -ArgumentList, and -Verb RunAs in one Start-Process statement for elevated scheduled-task registration.
• Add a focused regression test that fails when the outer PowerShell launcher is split before its elevation arguments.
• Preserve trusted System32 PowerShell resolution, immutable encoded XML transport, UAC cancellation, and elevated exit-code protocol behavior.

Closes #1843

Verification

• Final HEAD: b58ac6a7a75fff452aecfe3bfd760d19b4711af3.
• Current dev: 02da6cc9099db02873d6a38894af9b73d11f9f18.
• bun test tests/windows-elevation-spawn.test.ts — 52 pass, 0 fail, 175 assertions on final HEAD with Bun 1.3.14 on Windows 11.
• bun test --isolate tests/provider-workspace-data.test.ts — 48 pass, 0 fail, 180 assertions after the clean CI-equivalent GUI bootstrap.
• Clean local readiness gate — bun run prepush passed with exit code 0 on exact final HEAD under Ubuntu 24.04 / WSL2 with Bun 1.3.14, after installing both root and GUI frozen dependencies and building gui/dist, matching the repository CI bootstrap. Full result: 12,714 pass, 15 skip, 0 fail, 159,070 assertions.
• bun run typecheck — passed on final HEAD.
• bun run privacy:scan — passed on final HEAD.
• git diff --check upstream/dev...HEAD — passed on final HEAD.
• Windows PowerShell 5.1 exit-protocol check — child exit code 7 and missing-ExitCode protocol code 13 both propagated correctly on final HEAD.
• Windows 11 non-elevated service-install smoke — not run: an existing user-owned opencodex-proxy task is installed and running on port 10100, so the fresh-install path could not be exercised without deleting the user's task.
• UAC cancellation smoke — not run for the same existing-task safety reason.

No documentation change was required because this restores the already documented Windows elevation behavior.

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.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0eb174f9-04d2-457c-8009-66dbbc976eae

📥 Commits

Reviewing files that changed from the base of the PR and between ba456bd and 704d2f0.

📒 Files selected for processing (2)

• src/lib/windows-elevation.ts
• tests/windows-elevation-spawn.test.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The scheduled-task elevation launcher now assembles PowerShell fragments without splitting Start-Process parameters into separate statements. A Windows regression test verifies parameter ordering and registration success.

Changes

Windows elevation launcher

Layer / File(s)	Summary
Launcher assembly and regression coverage src/lib/windows-elevation.ts, tests/windows-elevation-spawn.test.ts	The scheduled-task launcher concatenates command fragments with inline separators. The regression test verifies one correctly ordered Start-Process statement containing -FilePath, -ArgumentList, -Verb RunAs, and -Wait.

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

Merge Risk: ⚪ Minimal · up to 704d2

The change is localized and no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

• lidge-jun/opencodex#408: Modifies the same Windows elevation implementation and spawn tests.
• lidge-jun/opencodex#1626: Modifies Windows elevation command construction and its spawn tests.

Suggested labels: intake: hygiene-blocked

Suggested reviewers: wibias, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR fixes Start-Process argument splitting, but src/lib/windows-elevation.ts still lacks separators in exit-code fragments, risking incorrect elevated exit behavior required by issue #1843.	Add statement separators to the exit-code validation and final exit fragments, then test the complete generated PowerShell launcher.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The changes are limited to the Windows elevation launcher and its regression test, which directly support issue #1843.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving Windows scheduled-task elevation launcher arguments.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

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

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu @Wibias

[ulrich-zogo]

@coderabbitai review

[ulrich-zogo]

The branch is based on the current dev head and the deterministic repository checks are green. Cross-platform CI and React Doctor are still awaiting fork-workflow approval and currently contain zero jobs. Could a maintainer approve those workflow runs?

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[ulrich-zogo]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[ulrich-zogo]

CodeRabbit's linked-issue warning about the adjacent }exit statement boundary is not actionable. Windows PowerShell 5.1 executed that exact boundary with exit code 7 for a non-null child ExitCode and protocol code 13 when ExitCode was null. This also matches the existing runWindowsElevated() launcher shape. No code change is required; bun test tests/windows-elevation-spawn.test.ts remains green with 52/52 tests on commit f10161df3.

[Ingwannu]

Maintainer review of exact head 6e68c0ebb is positive on scope and security boundary.

The diff keeps Start-Process, -FilePath, -ArgumentList, -Verb RunAs, and -Wait in one statement while preserving the trusted System32 PowerShell path, immutable encoded XML transport, UAC cancellation mapping, and elevated exit-code protocol. I reran the focused suite (52/52), typecheck, privacy scan, and diff check successfully.

This remains a draft with only the lightweight checks currently available. Please mark it ready and complete the exact-head cross-platform CI/readiness gate before merge. No separate Go-native counterpart exists for this TypeScript Windows installer path.

[Wibias]

Reviewed exact head b58ac6a7a75fff452aecfe3bfd760d19b4711af3.

Approve. I found no actionable P0-P2 code, security, compatibility, architecture, or test-coverage issue in this change.

The fix is correctly scoped to the malformed outer PowerShell launcher: Start-Process, -FilePath, -ArgumentList, -Verb RunAs, and -Wait now remain one statement, matching the established runWindowsElevated() construction. The trusted System32 PowerShell resolution, encoded immutable XML transport, UAC cancellation handling, and elevated exit-code protocol remain intact.

The focused regression is strong enough to catch the original failure by asserting the first statement terminator occurs only after -Wait and rejecting the previously split launcher shapes. The existing elevation suite also covers XML transport, cancellation, exit-code propagation, launch failure, signal termination, and timeout behavior.

I also reviewed the earlier CodeRabbit concern about the adjacent }exit boundary. It is not actionable: the same launcher shape already exists in runWindowsElevated(), and the exact boundary was verified under Windows PowerShell 5.1 for both normal child exit-code propagation and the missing-ExitCode protocol path.

No docs/config/dependency/database change is required for this bug fix.

Cross-platform CI for this exact head is still running at review time, so this approval is for the code itself; merge should still wait for the required exact-head CI to finish green.

[Wibias]

Thanks @ulrich-zogo — this is a really useful fix. It restores the fresh Windows service-install path from a non-elevated shell so the UAC prompt and scheduled-task registration actually run, while keeping the trusted System32 PowerShell resolution, encoded XML transport, cancellation handling, and exit-code protocol intact. The focused regression test also protects this exact launcher-shape bug from coming back. Merged — appreciate the tight scope and solid verification.