Allow portable extraction before Windows boot deadline - #97
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: dbalders/TritonAI-Installer/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
dbalders
left a comment
dbalders
left a comment
There was a problem hiding this comment.
TritonAI Review
No actionable findings in the inspected changes; see coverage limits.
Reviewed ee117636159a7130ec30261e019521f400565be3 against d352c504407d51d3a1f2ecfd8967a61494ce4127.
Inspected 1 file(s) in 1 part(s); reused 0 unchanged review part(s).
Changes
-
scripts/verify-windows-packaged-boot.ps1: This PR parameterizes the packaged-boot readiness deadline in scripts/verify-windows-packaged-boot.ps1. Invoke-PackagedBoot gains an optional LaunchTimeoutSeconds parameter (default 45 seconds, previously a hard-coded 35), the portable NSIS wrapper candidate is given 180 seconds to allow unpacking of the bundled runtimes before Electron's own internal readiness timer, and on timeout the script now prints bounded process identity diagnostics (pid, ppid, name of the wrapper and its direct children) before throwing. Marker validation (Test-SmokeMarker), the healthy-window check, process-exit checks, and cleanup behavior are unchanged. The installed-app candidate now runs with the 45-second default. Contract assertions in scripts/test-windows-signing.ts that check for literal substrings of this script (e.g. 'Invoke-PackagedBoot$PortablePath', 'exited with code $ ($Process.ExitCode) before writing its packaged boot readiness marker') still match the edited text.
Coverage limits
- scripts/verify-windows-packaged-boot.ps1: The supplied excerpt of scripts/test-windows-signing.ts is explicitly a selection with gaps omitted, and scripts/verify-windows-unsigned-release.ts (which invokes this PowerShell script via verifyWindowsPackagedBoot and may assert on its contents) was not supplied. All contract assertions visible in the excerpt still match the edited script, including the portable-call substring and the pre-marker-exit error message, but I cannot rule out an unshown assertion that pins the old 35-second deadline or the exact old timeout message. Local test runs reported in the PR description suggest these pass, but a full-file review of those two scripts would confirm.
CI snapshot
Combined commit status: success
- status CodeRabbit: success Review skipped: automatic reviews are disabled
Check runs:
- Macroscope - Correctness Check: status=completed conclusion=skipped app=
- Greptile Review: status=in_progress conclusion=none app=
- windows-latest: status=in_progress conclusion=none app=
- macos-latest: status=in_progress conclusion=none app=
CI is reported separately. This code review does not claim to have run tests.
Comment /triton-review to request a fresh review. Findings are advisory; GitHub branch protection remains authoritative.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
dbalders
left a comment
dbalders
left a comment
There was a problem hiding this comment.
TritonAI Review
No actionable findings in the inspected changes; see coverage limits.
Reviewed fbcb40f3113b99cd02d85bc979d45fd49c0bda01 against d352c504407d51d3a1f2ecfd8967a61494ce4127.
Inspected 1 file(s) in 1 part(s); reused 0 unchanged review part(s).
Changes
scripts/verify-windows-packaged-boot.ps1: This PR changes scripts/verify-windows-packaged-boot.ps1 so that Invoke-PackagedBoot accepts a configurable launch timeout (default 45 seconds) instead of a hard-coded 35-second deadline. The portable NSIS wrapper candidate now gets 180 seconds to unpack the bundled runtimes and write its readiness marker, while the installed-app candidate keeps the default. On marker timeout, the script now prints bounded process identity diagnostics (PID, parent PID, name of the launched process and its direct children) before throwing, and the timeout error message includes the configured limit. Marker validation, the five-second healthy-window check, process-exit checks, and cleanup are unchanged. The in-repo contract assertions in scripts/test-windows-signing.ts (which pin substrings such as 'Invoke-PackagedBoot $PortablePath', 'Invoke-PackagedBoot $InstalledExecutable', and the process-exit error message) all still match the edited script, and the script still parses under the PowerShell parser check exercised on Windows.
Coverage limits
- scripts/verify-windows-packaged-boot.ps1: The Electron-side smoke mode (claimed unchanged 30-second readiness timer, atomicity of the marker write) and any release-contract or runner tests that may pin additional literal strings in verify-windows-packaged-boot.ps1 were not supplied. No finding in this review depends on them, but a reviewer with access should confirm no other test asserts the removed 'AddSeconds(35)' or the old timeout wording.
CI snapshot
Combined commit status: success
- status CodeRabbit: success Review skipped: automatic reviews are disabled
Check runs:
- Greptile Review: status=completed conclusion=success app=
- Macroscope - Correctness Check: status=completed conclusion=skipped app=
- macos-latest: status=completed conclusion=success app=
- windows-latest: status=in_progress conclusion=none app=
CI is reported separately. This code review does not claim to have run tests.
Comment /triton-review to request a fresh review. Findings are advisory; GitHub branch protection remains authoritative.
1 participant
The signed Windows portable executable stayed running but exceeded the boot gate's 35-second total deadline before writing its readiness marker. That deadline includes NSIS unpacking the bundled Harness and runtimes, whereas Electron has a separate 30-second readiness limit.
Allow up to three minutes for the portable wrapper to unpack and reach readiness; installed-app checks use 45 seconds. Preserve Electron's readiness timer, five-second healthy-window requirement, marker validation, and process-exit checks. Print bounded process identity diagnostics on timeout.
Validation: build and focused Windows signing tests pass locally; native CI parses the PowerShell script. A hosted release retry must verify actual portable and installed boot behavior.