FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(windows): repair install.ps1, honest dry-run status, deterministic daemon_ipc by DeusData · Pull Request #1320 · DeusData/codebase-memory-mcp · GitHub

fix(windows): repair install.ps1, honest dry-run status, deterministic daemon_ipc - #1320

Merged
DeusData merged 3 commits into
mainfrom
fix/windows-install-contract-and-ipc-determinism
Jul 28, 2026
Merged

fix(windows): repair install.ps1, honest dry-run status, deterministic daemon_ipc#1320
DeusData merged 3 commits into
mainfrom
fix/windows-install-contract-and-ipc-determinism

Conversation

Copy link
Copy Markdown
Owner

Unblocks the 0.9.1-rc.1 release. Three independent fixes, one of them
user-facing and serious.

install.ps1 could not run at all

install.ps1 failed to PARSE and has been in that state on main since
61a432c. Windows PowerShell 5.1 decodes a BOM-less .ps1 as ANSI, so a UTF-8
em-dash arrives as cp1252 bytes ending in a double quote. One of the three in
this file sat inside a string literal, where that quote closed the string early
and took the enclosing blocks with it.

It shipped because nothing ever executed the script: the smoke covers it in
Phase 13, but every Windows smoke run since the launcher removal aborted at an
earlier phase, and the VirusTotal check only scanned the bytes. Since
install.ps1 is now the sole Windows install AND update path, this was a total
loss of both.

Fixed as ASCII rather than with a BOM, because the documented install pipes the
file straight into the parser (irm ... | iex). The bundle contract now rejects
any non-ASCII codepoint and names it.

install --dry-run reported success on a failed plan check

The Windows guard that an invalid PATH seam must fail closed was passing
vacuously. The seam still refused and still printed PATH configuration failed,
but the process exited 0, because Windows now compiles the shared
cbm_cmd_install, which since 30b5f12 downgraded any failed dry-run plan check
to CLI_OK. Keeping the dry-run summary was right; reporting success was not.
Only the status changes -- a real install already failed non-zero, so
fail-closed behaviour was never at risk, just its observability.

The smoke also still encoded the launcher-era portable-vs-managed split, which
does not survive one-binary-per-platform: 6b/6c expected refusals that no
longer apply (and 6c contradicted 14a), 14a used cmp, which is absent
from the Windows MSYS shell, and 14f seeded the MCP entry at the retired
binary that the old in-process update used to rewrite.

daemon_ipc was decided by a coin flip

Two Windows tests polled for the startup thread HOLDING the startup lock -- a
state with no lower bound, since acquire -> handoff -> release completes in
microseconds when the handoff does not block. This blocked release run
30374135923. Widening the budget was tried and rejected; a transient window is
never a fixed test. The production path now exposes a gate that fires with the
lock held and before the handoff, matching the existing POSIX publication hook,
so the interleaving is pinned by construction and inert unless a test installs
it.

Also fixes an unrelated use-after-free it exposed: ipc_forever_wait_server
closed and NULLed the connection from the server thread while the test was about
to interrupt the pointer it had already loaded. It crashed roughly one Windows
run in five.

Verification

Full 3-OS local, on the final tree:

Leg Result
macOS 320 suite tests, daemon_ipc 46 passed x5 under ASan/UBSan, full smoke exit 0
Linux (arm64) 6606 passed, 0 failed, 4 skipped, 116 suites
Windows (real VM) daemon_ipc 5/5 consecutive, smoke ALL PASSED (Phases 1-17)

install.ps1 verified executing end to end on Windows (13g: binary placed,
13h: binary runs). lint-ci clean. The new ASCII guard verified in both
directions -- passes clean, fails with U+2014 when an em-dash is reintroduced.

DeusData added 3 commits July 28, 2026 19:42
install.ps1 did not run. It failed to PARSE, dying with a cascade of syntax
errors before its first statement, and it has been in that state on main since
61a432c.

Windows PowerShell 5.1 decodes a BOM-less .ps1 as ANSI, not UTF-8. The three
em-dashes in this file are 3 UTF-8 bytes each, and cp1252 renders them as a
sequence ENDING IN A DOUBLE QUOTE. Two sat in comments and were harmless; the
third sat inside a string literal:

    "error: could not retire the existing $BinName - close all running"

where the injected quote closed the string early and left the rest of the line
as garbage expression, taking the enclosing if/foreach blocks with it.

This shipped broken because nothing executed the script. The smoke covers it in
Phase 13, but every Windows smoke run since the launcher removal aborted at an
earlier phase, and the VirusTotal check only ever scanned the bytes. Since
install.ps1 is now the sole Windows install AND update path, a parse error there
is a total loss of both.

Fixed as ASCII rather than by adding a BOM: the documented install pipes the
file straight into the parser (irm ... | iex), where a BOM would become part of
the string and break that path instead.

The bundle contract now rejects any non-ASCII codepoint in install.ps1 and names
it, so this cannot return silently.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…atus

The Windows smoke guards that an invalid PATH seam must fail closed rather than
silently falling back to the live HKCU\Environment\Path. That guard went green
in a way that meant nothing: the seam still refused, and install still printed
"PATH configuration failed", but the process exited 0.

Windows release builds used to compile a separate managed-install path. With one
binary per platform they compile the shared cbm_cmd_install, which since 30b5f12
downgraded ANY failed dry-run plan check to CLI_OK. That commit was fixing a real
complaint -- the dry-run summary was being skipped entirely on Windows -- but it
bundled the exit status into the fix. Keeping the summary is right; reporting
success is not. --dry-run exists to answer "would this install work?", so a
caller that only sees the exit code could not tell a refused PATH probe from a
clean plan.

The summary and the triage note still print; only the status changes. A real
install already failed non-zero here, so the fail-closed behaviour itself was
never at risk -- just its observability.

The smoke also still encoded the launcher-era portable-vs-managed split, which
does not survive one-binary-per-platform:

  * 6b expected uninstall to be REFUSED. That refusal existed because a portable
    extracted bundle was a different artifact from the launcher-managed install
    it would have torn down. The extracted binary now IS the installed one.
  * 6c expected update to be REFUSED, contradicting the contract Phase 14a
    asserts against a real update: Windows hands off to install.ps1, exits 0 and
    prints the command. 6c now pins that same handoff.
  * 14a compared binaries with cmp, which is absent from the Windows MSYS shell,
    so it reported "different" for two copies of one file. Hashes instead, the
    way Phase 12 already verifies the release archive.
  * 14f seeded the MCP entry at the RETIRED binary. The old launcher-managed
    update rewrote that entry in-process; the handoff does not, so uninstall
    correctly declined to remove an entry owned by a different installation and
    the phase demanded the one thing it must never do. It now seeds the
    installed binary, which is what install.ps1 leaves a real user holding.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…ling for it

Two Windows tests needed the startup thread to be observed HOLDING the startup
lock, and looked for it by polling the lock for BUSY up to 200 times. That state
has no lower bound: whenever the rendezvous handoff does not block, acquire ->
handoff -> release completes in microseconds, so the window can close between
two polls and startup_observed asserts on a coin flip. This blocked release run
30374135923 on test-windows (windows-2025, CLANG64, x86_64).

Budget-tuning the poll was tried and rejected -- a wider budget only lowers the
failure rate, and a transient window is never a fixed test. Instead the
production path exposes a gate that fires once the startup lock is held and
before the handoff runs, matching the hook pattern already used for POSIX
publication. The test parks the startup thread there and releases it when it is
ready, so the interleaving is pinned by construction. The acquired flag is
monotonic, so the observation cannot be missed; the remaining bound only catches
a thread that never started at all. The gate is inert unless a test installs it.

Also fixes an unrelated use-after-free this exposed. ipc_forever_wait_server
closed the connection and NULLed it from the server thread, while the test was
about to call cbm_daemon_ipc_connection_interrupt on the pointer it had already
loaded -- and receive_frame can return on its own before that call lands. It
crashed roughly one Windows run in five, independent of the change above.
Ownership now stays with the test, which closes after joining, so interrupt can
only ever see a live connection.

Verified on real Windows: 5 consecutive daemon_ipc runs, no failure and no
crash. macOS 46 passed x5 under ASan/UBSan; Linux 6606 passed, 0 failed.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData merged commit 3ad4274 into main Jul 28, 2026
11 checks passed
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL