Make the setup say what it is doing, extract faster, and stop crushing the prerequisites page - #183
Merged
Merged
Conversation
3 tasks
…twice over An upgrade sat for minutes on "Updating NodePilot" with a motionless bar, then finished quickly. Nothing was wrong; the wizard simply had no vocabulary for the stretch that costs the most. Measured on the 1.2.1 artifact: 2867 files totalling 114 MB, of which 2646 are under 64 KB and 1573 under 8 KB - `knowledge/` alone contributes 2304 files for 25 MB. An update walks that tree six times over (expand to staging, hash against the signed manifest, back up the old install, wipe the install path, copy staging in, hash again), so roughly 17,000 individual file operations, nearly all on tiny files. One full hashing pass measures 2.7 s on a warm NVMe, which rules hashing out as the cost: what makes it minutes is a real-time scanner inspecting every one of those creations. Three changes, none of them to what the update actually does: The updater now announces `Extracting artifact` before it expands and verifies, matching the installer's existing heading, and the progress table gains the matching entry at 10% - ahead of the backup at 20%. The first phase used to be the backup, so everything before it ran silently. The bidirectional drift guard already pins table and Write-Step calls to each other, and Test-SetupAdapter now asserts specifically that extraction is announced and ordered ahead of the backup, so a rename or a reorder cannot quietly restore the silence. The AV hand-off list gains `%TEMP%\nodepilot-artifact-*`. It documented Program Files, ProgramData and the backup folders, but not the staging directory where installer and updater create those ~2900 files first - so the one place that dominates the runtime was the one place not covered. Recorded as recommended rather than mandatory, with the residual risk stated plainly: the directory already carries a restrictive DACL applied atomically at creation, and its contents are verified file by file against the signed manifest immediately afterwards. `Import-NodePilotPkcsTypes` now asks the edition instead of attempting and catching. Add-Type raises a TERMINATING error under the setup's Stop preference and Start-Transcript records it before the catch swallows it, so every setup log carried a red "assembly could not be found" directly above the line confirming the signature had verified. Harmless, and exactly the kind of thing that makes an operator abort a healthy install.
Measured on the real 1.2.1 artifact (2867 files, 114 MB, 2646 of them
under 64 KB), twice, warm:
Expand-Archive 25.8 s / 24.7 s
ExtractToDirectory 2.2 s / 2.1 s ~12x
Expand-Archive pays per-entry pipeline overhead, which dominates a tree of
mostly tiny files. On a host whose real-time scanner inspects every
creation the absolute saving is proportionally larger, and this runs on
both the install and the update path.
Equivalence was verified rather than assumed: both extractors produce the
same 2867 files across the same 377 directories, with zero differences in
relative path, length or SHA-256.
Zip-slip is not given up in the trade. .NET refuses an entry whose
resolved path leaves the destination, confirmed against a crafted archive
carrying '../escaped.txt' - nothing was written outside, same as
Expand-Archive. Test-ArtifactSecurity now builds that archive and asserts
the rejection, because a later swap to a hand-rolled extraction loop is
exactly how this property would be lost quietly. Assert-NodePilotExtracted-
Files remains the second line regardless: rooted and dot-dot manifest paths
rejected, exact file count required, every file hashed against the signed
manifest.
Paths are resolved to absolute first - ExtractToDirectory resolves a
relative path against the process working directory, which is neither the
caller's location nor the staging parent. Import-NodePilotZipTypes loads
the assembly by edition for the same reason as the Pkcs helper: a
try/catch would write a red terminating error into every setup transcript.
…sured
The comment quoted 2.2 s against 25.8 s and the commit before it called
that ~12x. That number is real but it is a workstation number, and a
Server 2025 VM does not reproduce it:
Win 11 workstation (PS 5.1.22621) 24.3-24.8 s -> 2.0-2.7 s ~9-12x
Server 2025 VM, 4 cores (26100) 7.3- 7.7 s -> 4.6-5.2 s ~1.6x
Same artifact, three and two runs. Notably the workstation is the FASTER
machine on the new path and the slower one on the old, so the gap is in
what Expand-Archive costs per entry there rather than in raw I/O - which
is exactly why a single multiplier does not travel.
The change stays: faster on both, identical output, zip-slip still
rejected. But a server operator reading "12x" and measuring 1.6x would be
right to distrust the rest of the file, and the lab number is the one they
will get.
With all ten checks reporting, the page had nothing left for the field that explains them. Each row is "Title: Detail", so at the default width six of ten wrap to two lines; LayoutReadiness stacks ~238 px of rows into a ~309 px surface and the remediation box gets what remains - about 30 px, one clipped line. That box is where a failed database check has to show a CREATE LOGIN / CREATE USER block, so one line is not a cosmetic problem. Scrolling was the other option and is disproportionate here: Inno's Pascal Script exposes no TScrollBox, the rows are individual controls on the page surface, and a real scroll container would mean rebuilding the list as a TRichEditViewer - losing the per-row click targets and the auto-fix checkboxes. That is a different feature, not a layout fix. So the window grows instead, in both directions and for different reasons. Width does most of the work: +25% puts most rows back on one line, which shortens the stack before height is even considered. Height then buys the explanation real room: ~309 -> ~471 px of surface leaves ~210 px, about twelve lines. At ~560 px including its frame the window still fits the 768 px console of a server VM. An earlier note in [Setup] claimed "every page fits at 100%" and reverted a previous attempt on that basis. That claim is what the ten-check page disproves, so it is replaced with the measurement rather than deleted - otherwise the next reader reverts this for the same stated reason. WizardResizable stays off: those controls are positioned once, at construction, and carry no anchors, so a window dragged open at runtime would grow around a certificate picker that stays put. A fixed larger START size is a different thing and is safe - everything that must grow is already sized from SurfaceWidth. The remediation floor moves 13 px -> 34 px in the same pass. At one line the control reads as a broken edit field rather than an explanation. The larger window means the floor is not reached in practice; it is there for high-DPI scaling and for the day an eleventh check arrives. Rows still win and the box still gives - the other precedence would draw the explanation over the last checks, and a check nobody can see is worse than an explanation that has to scroll.
Sev7eNup
force-pushed
the
fix/update-progress-and-av-staging
branch
from
August 11, 2026 15:02
56b9480 to
7f34808
Compare
Sev7eNup
changed the title
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
1 participant
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.
Four changes to the server setup, all from one thread: an upgrade on a domain-joined client sat for minutes on "Updating NodePilot" with a motionless bar, and chasing that turned up three more things worth fixing.
1. The silent stretch now has a phase
The first update phase was the backup, so signature verification, extraction and the first hash pass ran with nothing to display. The updater now announces
Extracting artifactbefore it expands and verifies — the same heading the installer already uses — with the matching table entry at 10 %, ahead of the backup at 20 %.Where the time actually goes (measured on the 1.2.1 artifact): 2867 files, 114 MB, of which 2646 are under 64 KB and 1573 under 8 KB;
knowledge/alone contributes 2304 files. An update walks that tree six times — expand to staging, hash against the signed manifest, back up, wipe, copy in, hash again — roughly 17 000 file operations. One full hashing pass measures 2.7 s on a warm NVMe, which rules hashing out as the cause; what turns it into minutes is a real-time scanner inspecting every creation. Repo growth is ruled out too: 1.1.0 → 1.2.1 added only +64 files (+3 %).2. The AV hand-off list was missing the place that dominates the runtime
docs/av-exclusions.mddocumented Program Files, ProgramData and the backup folders — but not%TEMP%\nodepilot-artifact-*, where installer and updater create those ~2900 files first. Recorded as recommended, with the residual risk stated: the directory already carries a restrictive DACL applied atomically at creation, and its contents are verified file by file against the signed manifest immediately afterwards.3. Faster extraction — with the honest numbers
Expand-Archive→[IO.Compression.ZipFile]::ExtractToDirectory. Faster everywhere measured, but the margin does not travel, so no single multiplier is quoted:The workstation is the faster machine on the new path and the slower one on the old, so the gap is in what
Expand-Archivecosts per entry there, not in raw I/O. The lab number is the one a server operator will get — an earlier commit message in this branch called it "~12×" and the follow-up commit corrects that.Equivalence verified, not assumed: both produce the same 2867 files across the same 377 directories, zero differences in relative path, length or SHA-256. Zip-slip is not given up — .NET refuses an entry resolving outside the destination, confirmed against a crafted archive carrying
../escaped.txt, andTest-ArtifactSecuritynow builds that archive and asserts the rejection.4. Setup-log noise removed
Import-NodePilotPkcsTypesasks the edition instead of try/catch.Add-Typeraises a terminating error under the setup's Stop preference andStart-Transcriptrecords it before the catch swallows it — so every setup log carried a red "Die Assembly System.Security.Cryptography.Pkcs konnte nicht gefunden werden" directly above the line confirming the signature had verified.5. Prerequisites page —⚠️ not visually verified
With all ten checks reporting, the remediation box got ~30 px: one clipped line, in the field that has to show a
CREATE LOGINblock when the database check fails.WizardSizePercent=125,145— width puts most rows back on one line, height gives the explanation ~210 px. The floor moves 13 px → 34 px so it can never render as a broken sliver.This one lands unverified visually. Contract suites pass and ISCC compiles, but on the Inno-owned Installing page the extra height leaves a large empty area, and the prerequisites page itself has not been seen with the new size. Height is two numbers on one line and easy to tune down (145 → 120) once it has been looked at.
Scrolling was the alternative and is disproportionate: Inno's Pascal Script exposes no
TScrollBox, the rows are individual controls, and a real scroll container would mean rebuilding the list asTRichEditViewer— losing the per-row click targets and the auto-fix checkboxes.Verification
Three deployment suites green on Windows PowerShell 5.1 (templates, setup adapter 211 assertions, artifact security), ASCII contract on the
.issholds.End-to-end on the lab server (CM1, Server 2025, gMSA, SQL Server 2022): uninstall → full wipe incl. database drop → install published 1.2.1 → upgrade to this branch → uninstall with
/PURGEDATA=1→ fresh install with unattended bootstrap admin. All exit 0, healthlive/ready/database200 throughout, bootstrap admin login verified against the API. The new phase appears in the update log exactly where the silence was, and the log carries 0Add-Typeterminating errors.