Conversation
Replace the hardcoded "c:\\program files" / "c:\\windows" prefix heuristic with a PathResolver::requires_admin method that matches against runtime-resolved FOLDERID_ProgramFilesX64, FOLDERID_ProgramFilesX86, and FOLDERID_Windows. Honors the MVP "no hardcoded paths" rule, catches Program Files (x86) explicitly, and works on non-C: Windows installs. Match logic uses path-component boundaries so "C:\\Program Files Custom" no longer false-positives against "C:\\Program Files". Adds unit tests for the new method via a #[cfg(test)] constructor that lets us seed roots without invoking Win32.
program_files_x86 and windows_dir were stored but never read after construction; only their string forms via admin_roots are used. Compute them as locals in PathResolver::new instead, silencing the dead_code warning without expanding the public surface.
Wine runs the test binary cleanly when targeting x86_64-pc-windows-gnu, so wire that target's runner to wine in .cargo/config.toml. The default windows-msvc build is unaffected. README gains a 'Linux Cross-Build (Optional)' section with the dotnet 10 / mingw-w64 / wine prerequisites and the EnableWindowsTargeting=true recipe needed to cross-build the net8.0-windows C# UI from Linux.
The previous implementation joined std::env::args() with single spaces, which silently broke any argument containing whitespace, double quotes, or trailing backslashes — most importantly manifest paths under 'C:\Program Files' or 'C:\Users\<name with space>'. After elevation the relaunched process saw a different argv than the original. quote_command_line_arg follows the standard MSVCRT / CommandLineToArgvW encoding: quote on space/tab/quote/empty, escape embedded quotes with a preceding backslash, and double any run of backslashes that would otherwise be consumed by a following quote (including the closing one). Tests cover the canonical edge cases plus a round-trip that re-parses each encoded form with a reference argv parser to confirm fidelity.
There was a problem hiding this comment.
Pull request overview
Implements the MVP for covenant-setup: a deterministic Windows installer/uninstaller engine in Rust with a bundled single-file packager, a C# WinForms UI over named-pipe IPC, and Windows VM harnesses/docs to validate real Win32 boundaries.
Changes:
- Added core Win32 capabilities in Rust (path resolution, elevation/relaunch, filesystem + registry mutations, shortcut creation, Restart Manager + MoveFileEx fallback).
- Added a C# WinForms UI sidecar/embedded helper (named-pipe server + JSONL protocol) plus xUnit coverage for protocol/helpers.
- Added Vagrant-based smoke/coverage scripts, CI workflow, and extensive documentation/examples for packaging, install/uninstall, and debugging.
Reviewed changes
Copilot reviewed 31 out of 39 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/self-test/install.toml | Self-test manifest for VM smoke/coverage scenarios. |
| ui/Covenant.Setup.Ui/app.manifest | WinForms app manifest (asInvoker). |
| ui/Covenant.Setup.Ui/Program.cs | WinForms UI + named-pipe server and JSON message handling. |
| ui/Covenant.Setup.Ui/Covenant.Setup.Ui.csproj | C# UI project definition (WinExe, WinForms, internals visible to tests). |
| ui/Covenant.Setup.Ui.Tests/UiMessageJsonTests.cs | Tests for UI message/response JSON contract. |
| ui/Covenant.Setup.Ui.Tests/ProgramTests.cs | Tests for CLI arg parsing (--pipe). |
| ui/Covenant.Setup.Ui.Tests/InstallerUiFormHelperTests.cs | Tests for UI helper functions (errata JSON, mappings, safe summaries). |
| ui/Covenant.Setup.Ui.Tests/Covenant.Setup.Ui.Tests.csproj | C# test project setup and dependencies. |
| src/win.rs | Win32 boundary wrappers (known folders, elevation, registry, shortcuts, Restart Manager, file ops) + unit tests. |
| src/ui.rs | Rust-side UI session management (extract/launch C# UI, pipe protocol, prompts, progress sink). |
| src/sys.rs | Sys trait boundary abstraction and WinSys production implementation. |
| scripts/windows-vm/coverage/uac.ps1 | Guest scenario: UAC/elevation coverage assertions. |
| scripts/windows-vm/coverage/self-test.ps1 | Guest scenario: baseline install/uninstall + journal assertions. |
| scripts/windows-vm/coverage/reboot.ps1 | Guest scenario: locked-file uninstall requiring reboot fallback signals. |
| scripts/windows-vm/coverage/hklm-registry.ps1 | Guest scenario: HKLM registry writes forcing admin path. |
| scripts/windows-vm/coverage/bundled-exec.ps1 | Guest scenario: packaged/bundled installer execution path. |
| scripts/windows-vm/Start-InteractiveSelfInstall.ps1 | Guest interactive scheduled-task harness for install+uninstall verification + diagnostics. |
| scripts/windows-vm/Invoke-InteractiveInstaller.ps1 | Guest wrapper that launches installer/uninstaller and captures diagnostics/trace. |
| scripts/windows-vm/Abort-SmokeDiagnostics.ps1 | Guest abort/cleanup + trace bundle capture. |
| scripts/run-windows-vm-smoke.ps1 | Host orchestrator for interactive VM smoke test (package, upload, run, collect traces). |
| scripts/run-windows-vm-coverage.ps1 | Host orchestrator for multi-scenario VM coverage run. |
| project_mvp.md | MVP requirements/reference document. |
| examples/payload/sample_app.cmd | Example payload for demo installs. |
| examples/payload/post_install.ps1 | Example post-install payload script. |
| examples/install.toml | Example manifest covering directories/files/registry/shortcut/script/purge. |
| examples/README.md | Example usage instructions for packaging/install/uninstall. |
| docs/vagrant-smoke-debugging-notes.md | Deep-dive notes for diagnosing VM smoke failures/hangs. |
| docs/integration-tests-architecture.md | Integration testing architecture and scenario coverage overview. |
| docs/implementation-notes.md | Implementation/review notes capturing decisions and fixes. |
| build.rs | Builds/embeds C# UI helper via dotnet publish + embeds application manifest. |
| Vagrantfile | Windows 11 VM configuration for Hyper-V/WinRM based testing. |
| README.md | Project overview, usage, UI modes, VM harness docs. |
| Cargo.toml | Rust crate dependencies and build dependency configuration. |
| Cargo.lock | Locked dependency graph for reproducible builds. |
| CLAUDE.md | Repository build/run/conventions guidance. |
| .gitignore | Ignore rules for build artifacts/VM artifacts. |
| .github/workflows/ci.yml | Windows CI: build, Rust tests, C# UI tests, quality gate. |
| .cargo/config.toml | Windows MSVC static CRT linking + optional GNU target runner configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
|
@copilot apply changes based on the comments in this thread |
…win.rs Agent-Logs-Url: https://github.com/JMR-dev/covenant-setup/sessions/351376c1-0b93-4176-9cab-35d4cc72957a Co-authored-by: JMR-dev <51939451+JMR-dev@users.noreply.github.com>
Contributor
Applied all changes from the review thread in commit
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implementation of covenant-setup, a deterministic Windows installer engine.