fix: tolerate Windows NSIS line endings by Astro-Han · Pull Request #509 · Astro-Han/pawwork

Astro-Han · 2026-05-09T13:54:48Z

Summary

Allow the Windows NSIS shortcut customization test to match both LF and CRLF line endings.

Why

The post-merge windows-advisory workflow started failing after #503 because Git for Windows checked out packages/desktop-electron/resources/installer.nsh with CRLF line endings, while the test asserted a literal LF-only substring. The installer script content was correct; the test was too line-ending-specific for the Windows runner.

Related Issue

No dedicated issue. This fixes the repeated post-merge windows-advisory failure introduced by #503.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

Confirm the regex preserves the original structural assertion while accepting Windows CRLF checkout behavior.

Risk Notes

Low. This is a test-only change and does not change installer runtime behavior.

How To Verify

Focused desktop test: 9 passed, 0 failed via bun --cwd packages/desktop-electron test electron-builder-nsis-shortcut.test.ts
Line-ending proof: ok for both LF and CRLF via a Bun regex smoke check
Diff check: git diff --check passed with no whitespace errors
Final diff: 1 file changed, 1 insertion, 1 deletion

Screenshots or Recordings

Not applicable. No visible UI change.

Checklist

Summary by CodeRabbit

Release Notes

Tests
- Enhanced Windows installer test robustness to handle varying line ending formats across different systems.

coderabbitai · 2026-05-09T13:55:03Z

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 35 minutes and 10 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ec4366a7-d001-4ce1-acde-952b7e94036d

📥 Commits

Reviewing files that changed from the base of the PR and between 9469ce1 and 45311c6.

📒 Files selected for processing (1)

packages/desktop-electron/electron-builder-nsis-shortcut.test.ts

📝 Walkthrough

Walkthrough

A single test assertion in the Windows NSIS installer shortcut customization test is updated to use newline-tolerant regex matching (\r?\n) instead of a hardcoded string with Unix line endings. This makes the test pass regardless of the line ending style in the generated installer.nsh file.

Changes

NSIS Shortcut Test Robustness

Layer / File(s)	Summary
Line Ending Resilience `packages/desktop-electron/electron-builder-nsis-shortcut.test.ts`	The assertion for `!ifndef BUILD_UNINSTALLER` and `Var AddDesktopShortcutCheckbox` declaration changes from `toContain()` with hardcoded `\n` to `toMatch()` with regex `\r?\n`, tolerating both Windows (`\r\n`) and Unix (`\n`) line endings.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Astro-Han/pawwork#503: Added or modified the NSIS desktop-shortcut test and installer content that this change now makes line-ending resilient.

Poem

🐰 Line endings dance between systems two,
Now regex plays both old and new,
\r?\n bends either way,
No more brittle tests will fray!
A tiny fix, yet smooth to stay. 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: tolerate Windows NSIS line endings' directly and concisely describes the main change: modifying the test to handle both LF and CRLF line endings on Windows.
Description check	✅ Passed	The pull request description comprehensively follows the template with all required sections completed: Summary, Why, Related Issue, Human Review Status, Review Focus, Risk Notes, How To Verify, Screenshots/Recordings, and a fully checked Checklist.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch codex/fix-windows-nsis-line-endings

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 and usage tips.}

gemini-code-assist

Code Review

This pull request updates a test in electron-builder-nsis-shortcut.test.ts to use a regular expression for matching script content, which improves compatibility with different line endings. The review feedback suggests further enhancing the regex by using \s+ to make the test more resilient to variations in indentation and whitespace.

Root cause: - The post-merge `windows-advisory` workflow was still red after PR #509, but the remaining failure was no longer the NSIS desktop installer test. - The failing job was `unit-windows-opencode-session`, and the failure happened inside `packages/opencode/test/session/export.test.ts` before the export assertion ran. - Windows reported `EBUSY: resource busy or locked` while the test attempted to delete a git-backed temp project directory with `fs.rm(...)` in order to simulate a missing original session instruction directory. - The same `EBUSY` signature appeared in consecutive dev advisory runs, so it was actionable CI instability rather than a one-off signal to ignore. What changed: - Added a small test-local helper, `removeSessionProjectDirectory`, that keeps the existing deletion behavior but adds retry budget for transient Windows file-handle release delays. - Replaced the two direct `fs.rm(sessionProject.path, ...)` calls in the export tests that intentionally remove the original session project directory. - Kept the export/runtime implementation and the existing assertions unchanged. Scope boundary: - Test-only change in `packages/opencode/test/session/export.test.ts`. - No product export behavior, installer code, workflow configuration, dependencies, or generated files changed. - This is separate from the PR #509 NSIS line-ending fix; #509 fixed `unit-windows-desktop`, while this PR addresses the remaining opencode session export advisory failure. Verification: - `git diff --check` completed with no whitespace errors. - `bun --cwd packages/opencode test test/session/export.test.ts` passed locally with 32 pass / 0 fail. - PR #511 status rollup completed successfully: `ci`, `desktop-smoke`, `codeql`, `e2e-artifacts`, `commit-lint`, PR title lint, dependency review, CodeRabbit, and unit result checks were all green. Follow-up: - The direct original failure surface is the `windows-advisory` workflow on dev. That workflow is not part of the normal PR path, so the final confirmation is the next dev advisory run after this squash merge, or a manual dispatch of `windows-advisory`.

fix: tolerate Windows NSIS line endings

9469ce1

Astro-Han added bug Something isn't working ci Continuous integration / GitHub Actions windows Windows-specific P2 Medium priority labels May 9, 2026

gemini-code-assist Bot reviewed May 9, 2026

View reviewed changes

Comment thread packages/desktop-electron/electron-builder-nsis-shortcut.test.ts Outdated

Astro-Han added 2 commits May 9, 2026 22:09

fix: relax NSIS installer test whitespace

b3f1224

fix: preserve NSIS declaration line boundary

45311c6

Astro-Han merged commit ec5c6e2 into dev May 9, 2026
20 checks passed

Astro-Han deleted the codex/fix-windows-nsis-line-endings branch May 9, 2026 14:26

Astro-Han mentioned this pull request May 9, 2026

test(opencode): retry missing session directory cleanup #511

Merged

11 tasks

coderabbitai Bot mentioned this pull request May 14, 2026

test(app): normalize perf workflow line endings #623

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: tolerate Windows NSIS line endings#509

fix: tolerate Windows NSIS line endings#509
Astro-Han merged 3 commits into
devfrom
codex/fix-windows-nsis-line-endings

Astro-Han commented May 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 9, 2026 •

edited

Loading

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Astro-Han commented May 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Related Issue

Human Review Status

Review Focus

Risk Notes

How To Verify

Screenshots or Recordings

Checklist

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Astro-Han commented May 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 9, 2026 •

edited

Loading