fix(msi): install into the root dig-updater probes, closing a non-convergent update loop

[MichaelTaylor3d]

What changed, and why it matters more than a path tidy

dig-updater runs this package on every Windows update (msiexec /i <pkg> /qn /norestart;
dig-node's Windows InstallMethod is WindowsMsi, and the live manifest serves
dig-node-0.99.4-windows-x64.msi today). After running it, the beacon reads the installed version
from <install-root>\dig-node.exe — the canonical protected root %ProgramFiles%\DIG�in.

The package installed to %ProgramFiles%\DIG Network\dig-node\ instead. So the install never
touched the file the updater probes. The probed version never changed, every beacon cycle re-ran
the same install, and PATH kept serving the stale copy: Windows auto-update was non-convergent,
permanently. The package cannot simply be retired — doing so would break Windows auto-update and
redden the signed feed for every channel.

The same mismatch is what made dig-installer fail its own audit (it verifies the service image and
the PATH resolution of dig-node.exe against that root), so every install failed a safety check
against a directory its own payload had just created.

dig-updater's own tests already assert the layout this PR moves to, so dig-updater needs no change
and is the acceptance witness for this one.

The PATH shadowing is cross-hive and therefore unconditional: dig-installer writes the shared root
to the user hive (HKCU\Environment\Path), this package wrote its own root to the machine
hive, and a fresh session composes machine PATH before user PATH.

Four changes in packaging/windows/dig-node.wxs (UpgradeCode byte-identical — it IS the migration
mechanism; changing it would leave two products both owning net.dignetwork.dig-node):

1. INSTALLFOLDER = %ProgramFiles%\DIG\bin (the canonical root — canonical skill / SYSTEM.md
   install-root section; dig-node's own security.rs already names this root).
2. The machine-PATH component is removed, not re-pointed. One entry needs one owner: an MSI
   Environment row is deleted on uninstall, so a row naming the shared root would take that root
   off PATH for digstore, dig-updater and everything else living there. The service is registered
   with an absolute image path and needs no PATH; the user-facing CLI is dign, which dig-installer
   places.
3. RemoveExistingProducts pinned explicitly to afterInstallValidate, with the reasoning in
   the file. The late schedule (afterInstallExecute) is the plausible-but-broken choice: the old
   product's ServiceControl Remove="uninstall" matches the service by name and would delete the
   service the new product just registered.
4. A name-scopedRemoveFile clears a foreign dig-node.exe from the shared root before
   installing. dig-installer drops a raw binary there (it does not run this package — the old
   header comment claiming otherwise is corrected), and Windows Installer keeps such a file rather
   than overwrite it, so the package completes over a binary it did not install — which is exactly
   the stale version dig-updater probes next. Scoped by name because the shared root also holds
   digstore, dig-dns, dig-updater and dig-app. REINSTALLMODE=amus is deliberately NOT used: it
   turns a repair into a silent downgrade.

What the migration does to a box that already has the old layout

All three things the old package owned — binary, machine-PATH row, service registration — move in
one transaction:

RemoveExistingProducts 1401 → StopServices 1900 → DeleteServices 2000 → RemoveFiles 3500
→ InstallFiles 4000 → InstallServices 5800 → StartServices 5900

(sequence numbers read out of the compiled MSI, not the source). net.dignetwork.dig-node is
absent only inside the transaction: an interruption rolls back to the old product with its service
intact, and a completed run ends with the service registered against the new image. There is no
reachable resting state with a registered product and no service — which is exactly the state a
hand-repair produced on the user's machine (deleting the directory left a registered ghost, and
repairing that removed the running service).

Blast radius checked

gitnexus is disabled in the loop (§2.0 override), so this was done with ripgrep + a direct read across
dig-node and dig-installer:

• packaging/windows/dig-node.wxs — the only declaration of the install location. No Rust code in
  this repo derives a path from it; the service resolves its own binary via current_exe().
• SPEC.md — two statements of the old path (§9.1 gate prose, §9.7 package list). Both updated, plus
  a new normative paragraph on upgrade sequencing and the PATH-ownership rule.
• dig-installer — msi::MSI_PACKAGES keys on UpgradeCode + DisplayName, both unchanged, so
  its supersede/uninstall path keeps working against packages built from this source.
• docs.dig.net — no occurrence of the old path.
• Risk: HIGH — an MSI upgrade that owns a service, on machines that already carry the old layout.
  That is why the on-machine evidence below exists rather than only unit tests.

Evidence

Proven on a real Windows machine that was deliberately put back into the old layout by installing
the released v0.99.4 MSI, then upgraded with a package built from this branch:

	before (released v0.99.4)	after the upgrade
service ImagePath	C:\Program Files\DIG Network\dig-node\dig-node.exe	C:\Program Files\DIG\bin\dig-node.exe, Running
C:\Program Files\DIG Network	present	absent (both levels)
machine PATH	C:\Program Files\DIG Network\dig-node\	no DIG entry
Add/Remove	DIG NETWORK: NODE 0.99.4	DIG NETWORK: NODE 0.99.9, one entry, no orphan
where dig-node.exe, fresh environment block	…\DIG Network\dig-node\dig-node.exethen…\DIG\bin\dig-node.exe	…\DIG\bin\dig-node.exe only

The fresh-environment probe runs through the Task Scheduler. A running shell cannot show this: the
stored machine PATH contains a literal %PATH% self-reference, so expanding it inside a composed
shell splices that shell's own PATH in and inverts the ordering.

dig-installer run end to end afterwards (elevated, --with-dig-node): ✓ DIG is ready.,
including the audit line that used to fail —
✓ dig-node runs from the protected install root ("C:\Program Files\DIG\bin\dig-node.exe" run-service)
— and ✓ dig-node --version resolved + ran as micha.

Compiled-artifact inspection (WindowsInstaller.Installer over the built MSI): Directory table
INSTALLFOLDER → DIGROOTFOLDER(DIG) → ProgramFiles64Folder; the Environment table is absent;
RemoveFile dig-node.exe INSTALLFOLDER InstallMode=1; Upgrade attributes 513 (max-inclusive —
AllowSameVersionUpgrades from #618 preserved).

The verified artifact vs the final artifact. The on-machine run above used a package built
before the round-2 comment/SPEC edits. Dumping both MSI databases, the ONLY difference is the
version in the Upgrade table (0.99.9 → 0.99.10): Directory, Component, ServiceInstall,
ServiceControl, RemoveFile, the absent Environment table and the whole
InstallExecuteSequence are identical, so the migration evidence carries over. A UAC prompt was
declined when I tried to re-run the verifier against the final build, so that re-run did not happen;
verify-upgrade-migration.ps1 parses clean and its elevation guard fires, but the script has not
been executed end to end as a single unit — its individual steps were all run by hand.

New testscripts/tests/msi-install-root.test.sh (runs on the existing ubuntu scripts CI job,
so this packaging change does get automated cover). Each assertion was proven load-bearing by
reverting only its own fix and confirming only that assertion fails:

reverted fix	failing assertion	others
directory chain → DIG Network\dig-node	location (+ leftover-root check)	pass
re-add the Environment component (one line)	PATH ownership	pass
re-add it with attributes wrapped across lines	PATH ownership	pass — this passed before the round-2 flatten fix
Schedule="afterInstallExecute"	upgrade sequencing	pass
delete the RemoveFile element	foreign-binary removal	pass
widen it to Name="*.exe"	removal + scoping (2 FAILs)	pass
change it to On="uninstall"	runs-on-install	pass

The location check composes the directory chain rather than grepping for DIG, because the
nearest wrong implementations (a different manufacturer folder, a DIG\<component> leaf) contain the
right substring.

What is NOT proven

• A from-scratch dig-installer run that actually executes the FIXED MSI. The installer fetches
  from GitHub releases, so it still runs the old v0.99.4 package; in the green run above it skipped
  the MSI step (already up to date) and validated the layout this branch produced. The full
  fetch→install→ready loop can only be closed once this is released.
• An MSI install in CI.build .msi (windows-x64) DOES run on this PR and passed, so the
  changed WiX source is proven to COMPILE in CI (correcting my initial assumption that dig-node's
  gating jobs are ubuntu-only). Nothing in CI installs or upgrades the MSI, so the migration
  behaviour above rests entirely on the local run, not on CI.
• macOS/Linux packaging is untouched.

Relationship to DIG-Network/dig-installer#62

That PR removes the superseded DIG Network root on the installer side, on the premise that the root
is historical. It was not historical — this package recreated it on every install, which is what
this PR fixes upstream. The two are complementary and neither makes the other unnecessary:

• This PR stops new installs from creating the root.
• feat(session)!: migrate DID resolver to dig-identity v0.4 BLS-G1 (resolve_bls_public_key, 48B) #62 remains required as a one-time migration for machines whose last dig-node install predates
  this release and that never take another MSI upgrade. Its refusal conditions must keep holding.

Note that an MSI upgrade from a pre-fix version already removes the directory, the PATH row and the
old registration by itself (measured above), so #62 should treat an absent old root as the normal
outcome, not as evidence it has nothing to do.

Gate round 2 — what changed since the first review

1. The RemoveFile rationale was false, and this PR was promoting it to normative SPEC. It said
   dig-installer "places its own copy of the binary there before running this package". dig-installer
   never runs the package; it drops a RawBinary. Both that comment and the pre-existing header line
   ("the dig-installer just fetches + runs this .msi") are rewritten against what actually happens:
   dig-updater runs /i, dig-installer drops a raw binary today, and unifying that is a separate
   unit of work. SPEC §9.7 carries the same correction.
2. The RemoveFile MUST had no mechanical guard — deleting the line left all assertions green.
   Now guarded, and proven load-bearing three ways: deleting the element → 1 FAIL; widening it to
   Name="*.exe" → 2 FAILs (the shared root makes a wildcard removal worse than the original bug);
   changing it to On="uninstall" → 1 FAIL. SPEC now states four requirements and the test asserts
   four.
3. The PATH guard was fail-open to a line break. A real machine-PATH <Environment> component
   with its attributes wrapped across two lines passed every assertion. The check now flattens with
   tr ' ' ' ' first; re-running that same wrapped injection FAILs, and the file was restored from
   a copy afterwards.

Stage 2 — not in this PR, and what it needs

dig-installer switching from RawBinary to running this same .msi is a different repo and a
separate unit of work. What it needs from this side: the package now installs to the root
dig-installer already manages, owns no PATH row, and tolerates a foreign binary already present in
that root — so dig-installer can adopt msiexec /i without a layout change here. Until it lands,
two writers keep sharing DIG�in\dig-node.exe, which is why the RemoveFile exists.

Version

0.99.9 → 0.99.10 (patch: a packaging fix, no API or wire change). Re-derived from origin/main
at rebase time — main moved to 0.99.9 during this work.

[MichaelTaylor3d]

VERDICT: CHANGES-REQUIRED

(Recorded as a COMMENT review: GitHub rejects both --approve and --request-changes from the PR author identity with HTTP 422. The three inline threads below are the blocker - they stay unresolved until addressed, which bars merge under required_conversation_resolution.)

CHANGES-REQUIRED - reviewed at ce65bda

(The dispatch quoted fe1f989; the head has moved. All line numbers below resolve against ce65bda.)

mergeable=MERGEABLE, mergeStateStatus=CLEAN, so unlike #189 the check rows are meaningful. build .msi (windows-x64) genuinely ran and passed - your premise correction is confirmed.

The direction is right and the on-machine evidence is the best kind. Three findings, one substantive.

What I verified as correct

• RemoveFile narrow safety (scrutiny point 2): clean.Name="dig-node.exe" is a literal, the directory is INSTALLFOLDER via the component, and On="install" restricts it to the install action. It cannot touch digstore.exe, dig-updater.exe, dig-dns.exe or dig-app, and it runs inside the transaction, so a rollback restores whatever it deleted.
• The upgrade transaction / no-ghost claim (scrutiny point 1): sound as reasoned.RemoveExistingProducts at 1401 is inside the install transaction and is rolled back on failure; StopServices 1900 precedes RemoveFiles 3500 so the image is not locked; ServiceControl Start="install" Wait="yes" plus ServiceInstall Vital="yes" makes a failed start an error rather than a silent skip. I did not execute msiexec - this is MSI semantics plus your machine run, not my own measurement.
• Cross-hive precedence (scrutiny point 3): both halves hold. dig-installer writes HKCU\Environment\Path (src/paths.rs:465-486) and its own src/pathcheck.rs:242-243 states the machine-then-user composition. And an MSI Environment row is deleted on uninstall, so re-pointing really would strip the shared root for digstore and dig-updater.
• UpgradeCode byte-identical contract intact: dig-node.wxs:41 == dig-installer src/msi.rs:57.
• Version 0.99.10 correct against main at 0.99.9. --shortstat == --ignore-cr-at-eol (238/18 both) - no CRLF inflation. No Closes in the body, so nothing auto-closes early. Old-root sweep clean: the three surviving DIG Network\dig-node strings are deliberate historical narration plus one unrelated HKLM registry key.

Findings

1. (gating)dig-node.wxs:117 - the RemoveFile rationale rests on a sequence that does not happen, and the real consequence of the shared root is unanalysed.
2. (gating)scripts/tests/msi-install-root.test.sh:144 - the fourth fix has no test. Proven by mutation.
3. (gating)scripts/tests/msi-install-root.test.sh:96 - the PATH-ownership guard is fail-open to a multi-line <Environment> row. Proven by mutation.

Detail inline.

[MichaelTaylor3d]

Gate round 2 addressed — PR #194 updated, HEAD ede9dc4, still not merged.

Reframed around the real defect. The PR title and body now lead with the non-convergent update
loop rather than PATH shadowing: dig-updater runs this package on every Windows update and then
probes <install-root>\dig-node.exe, which the install never wrote — so the probed version never
changed and every beacon cycle re-ran the same install. SPEC.md §9.7 states both reasons the root
is mandatory (convergence first, the installer's audit second) and records that dig-updater, not
dig-installer, runs the .msi.

The three findings, each fixed and each proven:

1. False RemoveFile rationale, which this PR was promoting to normative SPEC. Rewritten in
   dig-node.wxs (both the component comment and the pre-existing header line 4) and in SPEC:
   dig-updater runs /i, dig-installer drops a RawBinary, unifying them is separate work.
2. RemoveFile had no mechanical guard — confirmed, deleting the line left everything green.
   Now guarded and load-bearing three ways: delete the element → 1 FAIL; widen to Name="*.exe" →
   2 FAILs; change to On="uninstall" → 1 FAIL. The scoping assertion is new: a wildcard removal in
   the SHARED root would delete digstore/dig-dns/dig-updater/dig-app binaries, which is worse than
   the original bug. SPEC now states four requirements and the test asserts four.
3. PATH guard fail-open to a line break — confirmed. Now flattens with tr '\n' ' ' first; the
   same wrapped <Environment> injection that used to pass now FAILs.

REINSTALLMODE=amus is not used, and is now called out as forbidden in both the wxs and SPEC.
UpgradeCode is byte-identical.

New:packaging/windows/verify-upgrade-migration.ps1 — the elevated old-layout→upgrade
acceptance check CI cannot run, ending in a service-RUNNING assertion and a fresh-environment-block
resolution probe. It parses clean and its elevation guard fires, but it has not been executed end
to end as one unit: a UAC prompt was declined on the retry. Its individual steps were all run by hand
earlier, green.

Evidence bridge for the round-2 edits. The on-machine run used a package built before these
comment/SPEC edits. Dumping both MSI databases, the only difference is the version in the Upgrade
table (0.99.9 → 0.99.10) — Directory, Component, ServiceInstall, ServiceControl,
RemoveFile, the absent Environment table and the entire InstallExecuteSequence are identical.

CI status: a GitHub Actions outage, not this change.mergeable=MERGEABLE (checks are real).
build .msi (windows-x64)passes on the current head, as do Clippy, Rustfmt, Release-script
tests, CodeQL/Analyze and the ubuntu + windows install matrices. Four jobs are red purely at Set up job with Failed to resolve action download info. Error: Service Unavailable (3 retries each,
before any repo code runs): Lint commit messages, Test + coverage, build .pkg (macos-universal),
system-scope install/uninstall (macos-14). Re-runs dispatched; the previous head ce65bda had a
FULL green including those jobs, and the diff since then is comments, SPEC prose, one shell test and
one PowerShell script — no Rust.

Next action: re-run the four outage-failed jobs when Actions recovers, then gate → merge →
stable dispatch → pointer bump → close. Do not close on merge alone.

[MichaelTaylor3d]

Round 2 addressed. The reframe is the important part — this PR fixes a non-convergent auto-update loop, not a path nit. Keep that first in the body; it is what makes the change urgent rather than tidy.

Two corrections to your realizations:

Realization 4 — the canonical skill is already fixed. Amended in a9b19e6 on the superproject before your round-2 message. The entry now states the root is %ProgramFiles%\DIG\bin as of MSI 0.99.10, records that pre-0.99.10 packages used DIG Network\<component>\, and captures the cross-hive mechanism: dig-installer writes the USER hive, the MSI wrote the MACHINE hive, and a fresh session composes machine-before-user — which is why merely re-pointing the row would not have fixed the shadow. You were reading a checkout from before that commit.

Realization 2 is a real finding and I have hit it too.gh pr checks reporting cancelled as fail is a false-red, and it belongs alongside the mergeStateStatus trap as a second way that command misleads. The CI watcher on this PR has been told to resolve every non-success row through gh api .../jobs/<id> -q .conclusion before calling anything red.

Realization 1 is the one to internalise.git checkout -- <path> inside a revert loop wiped uncommitted work even with the edits staged — the exact hazard the brief warned about, hit anyway. That is the third destructive-revert incident in this ecosystem today. The safe shape is a file copy taken before the loop, never a git command that cannot distinguish your mutation from your work.

The MSI-database bridge is a genuinely good argument and it belongs in the PR body if it is not there already: the installed-and-verified build and the final build differ in exactly one place, the version in the Upgrade table. Directory, Component, ServiceInstall, ServiceControl, RemoveFile, the absent Environment table and the entire InstallExecuteSequence are identical. That is what carries the machine evidence forward across the round-2 edits without re-running the whole elevated experiment, and a reviewer should not have to reconstruct it.

Guard scoping — the right call. A wildcard removal in the shared root would delete digstore, dig-dns, dig-updater and dig-app binaries, which is worse than the bug being fixed. Asserting the scope, not just the presence, is what makes that guard real.

The unproven items stay stated as unproven: the verifier script has not run end to end as one unit, and a from-scratch install of the fixed MSI needs a release. Do not let green checks imply either.

[MichaelTaylor3d]

All three gating findings verified fixed at this head. Checked each against the tree rather than taking the round-2 report.

1 — the false rationale. The header now states who runs the package accurately: dig-updater runs it on EVERY Windows update (msiexec /i <pkg> /qn /norestart, dig-updater-broker install.rs, because dig-node's Windows InstallMethod is WindowsMsi), and dig-installer today does NOT run it — it drops a raw binary into the install root itself. The previous wording had it backwards, and that mattered: it was the premise I used to tell the gate nobody runs this package, which nearly retired a live Windows update path.

2 — the fail-open PATH guard. Now flattens with tr '\n' ' ' before matching, and the comment records the measurement: a real <Environment Name="PATH" …> split across two lines passed the old line-oriented grep, and fails now. That is the guard proving itself rather than asserting.

3 — the unguarded RemoveFile MUST. Now guarded four ways — presence, that it names dig-node.exe, that it runs on install, and critically that it is scoped: a wildcard or nameless removal in the SHARED root would delete digstore, dig-dns, dig-updater and dig-app binaries, which is worse than the bug being fixed.

Why this PR matters more than its title suggests. It is not path hygiene. The beacon probes <install-root>\dig-node.exe while the MSI installed to DIG Network\dig-node\, so the Windows auto-update never converged — every cycle re-ran the same install, the probed version never moved, and PATH kept serving the stale copy. Forever.

All five required checks green; Attach packages to the release is tags-only and correctly skipped on a PR (not a required context). Merging.