Skip to content

Fix Windows installer tests rebuilding the prepared runtime - #455

Merged
eval-exec merged 1 commit into
eval-exec:mainfrom
thanosapollo:fix/windows-installer-runtime-order
Oct 3, 2026
Merged

eval-exec merged 1 commit into
eval-exec:mainfrom
thanosapollo:fix/windows-installer-runtime-order

Conversation

@thanosapollo

Copy link
Copy Markdown
Contributor

Problem

The Windows installer workflow runs Cargo-backed launcher tests after cargo xtask fresh-build --release. A subsequent rebuild can replace the prepared editor's fingerprint with its placeholder while leaving the previous portable dump in place. The batch launcher then exits 101 rather than the expected 23.

This failure is present on upstream main as well as #451 and #454; it is not specific to their daemon changes.

Change

  • Compile and list the release windows-tools launcher tests before final runtime preparation.
  • Run the prepared tests using nextest's binaries and Cargo metadata, without another Cargo build.
  • Check that the prepared editor, dump, launcher and cmdproxy hashes remain unchanged after testing.

No loader validation is weakened, and no launcher, GUI or installer test is removed.

Verification

  • Independent correctness and maintainer-fit review: PASS, no blocking findings.
  • Actual nextest 0.9.137 portable compiled-fixture execution: two tests passed with child Cargo/rustc invocations denied and artifact hashes unchanged.
  • Rebuild control reproduced the fingerprint mismatch; an artifact-mutation control failed the checksum guard.

These local checks prove the orchestration mechanism, not native Windows acceptance. The x86_64 and ARM64 hosted jobs must still pass launcher tests, GUI smoke, packaging, and install/upgrade/uninstall. This PR is submitted to obtain that hosted proof; it is not a claim that those gates are already green.

An automated agent prepared and submitted this contribution, with a separate independent agent review.

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: bd75d713-7b7c-4455-9907-e5d1c7e22c7c

📥 Commits

Reviewing files that changed from the base of the PR and between 82b399e and d931183.

📒 Files selected for processing (1)
  • .github/workflows/windows-installer.yml

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


📝 Walkthrough

Walkthrough

The Windows installer workflow now runs the saved launcher test binary and checks that four runtime files retain their post-build hashes.

Changes

Windows launcher test workflow

Layer / File(s) Summary
Run saved launcher test and verify runtime files
.github/workflows/windows-installer.yml
The workflow saves the release-profile test binary and Cargo metadata, records hashes after the fresh build, then runs the saved test binary and checks the runtime-file hashes.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: eval-exec

Merge Risk: ⚪ Minimal · up to d9311

No concrete merge-blocking issue is established. Native Windows acceptance remains to be confirmed by the workflow jobs.

Architecture Summary

Architecture risk: 🔵 Low · up to d9311

The changed surface does not map to a changed system, dependency edge, entrypoint, or external dependency.

Changed systems: None identified.

Architecture concerns
No architecture-level concerns identified.

Review details

Before / after behavior

  • observed — Modified behavior in .github/workflows/windows-installer.yml: Adds a step to list the release-profile Windows launcher test binaries and save Cargo metadata for later test execution.
  • observed — Modified behavior in .github/workflows/windows-installer.yml: After the fresh build, records hashes for neomacs.exe, neomacs.pdump, runneomacs.exe, and cmdproxy.exe.
  • observed — Modified behavior in .github/workflows/windows-installer.yml: Replaces the scoped Cargo test invocation, which could rebuild the test target, with execution using the saved binary and Cargo metadata; afterward, checks the runtime-file hashes against those recorded after the fresh build.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the Windows installer workflow failure, the test orchestration change, and the verification performed. It is directly related to the changeset.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing Windows installer tests from rebuilding the prepared runtime.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@coderabbitai
coderabbitai Bot requested a review from eval-exec October 2, 2026 12:51

@eval-exec eval-exec left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified on main before merging: .github/workflows/windows-installer.yml runs cargo xtask fresh-build --release (line 89) and then a second Cargo build via cargo nextest run ... --features windows-tools (line 95), which is the ordering this PR fixes. The fix pre-lists the launcher test binaries, runs them with --binaries-metadata/--cargo-metadata so no rebuild can replace the prepared fingerprint, and sha256-guards neomacs.exe/pdump/runneomacs/cmdproxy afterwards. Hosted Windows proof (launcher, GUI smoke, packaging, install/upgrade/uninstall) runs on main via the workflow's push trigger.

@eval-exec
eval-exec merged commit 3aa7b9d into eval-exec:main Oct 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants