Skip to content

Make installer exit 0 mean the host integration actually works - #624

Merged
MongLong0214 merged 8 commits into
mainfrom
fix-595-installer-truth
Aug 13, 2026
Merged

Make installer exit 0 mean the host integration actually works#624
MongLong0214 merged 8 commits into
mainfrom
fix-595-installer-truth

Conversation

@MongLong0214

Copy link
Copy Markdown
Owner

Summary

Closes #595.

Both installers decided a host's MCP registration was fine because the config merely mentioned commitlore, or because a command string existed. A registration whose command cannot run — a directory, a non-executable file, the wrong args — was skipped as "already configured" and the installer still exited 0. The user was told the install succeeded when the integration could not work.

install exit 0 now means every requested host is either live-verified healthy, or was explicitly preserved as a healthy custom CommitLore registration.

What changed

  • One shared TypeScript command, installer-hosts, invoked by both install.sh and install.ps1. The shell and PowerShell paths no longer make their own judgements — they call it and act on its result, so the two cannot drift apart.
  • Ownership is exact: the wrapper path and args exactly mcp. No substring, no "the word appears", no "the directory exists".
  • A custom registration is verified live by speaking MCP to it — initialize, then serverInfo, then a minimum tool set. Healthy custom registrations are preserved and reported distinctly from ones we own; dead, foreign or unverifiable ones are not ok.
  • Config writes are atomic: parse → write temp → re-parse the temp → rename. An interruption before the rename leaves the original untouched, and there is a fault-injection seam that proves it rather than asserting it.
  • Any requested host that fails makes the installer exit non-zero in the default mode.

Test plan

Non-zero, each asserted:

  • command does not exist
  • command is a directory
  • our wrapper with the wrong args
  • malformed / unparseable config
  • host CLI that fails when invoked
  • config write interrupted midway — original file intact

Zero, reported distinctly:

  • a healthy registration we own

  • a healthy custom registration, preserved rather than overwritten

  • Negative control: restoring the presence-only check makes the dead-command case pass with exit 0 where it must exit 1

  • 150 cases across installer-hosts, install-script, install-ps1 and doctor

  • npx tsc --noEmit clean

  • dist/ rebuilt on linux/amd64

Unverified

Windows runtime behaviour. pwsh/powershell is not available on the machine this was developed on, so install.ps1 was not executed. Its delegation to the shared command is covered structurally — same command, same JSON summary, same exit meaning — but the actual PowerShell run is unverified here and is left to the install-ps1 job on this pull request to confirm.

Presence-only checks could report a working integration after the configured command had become unusable. The shared live probe makes the installer exit status match host health.

Ruled-out: presence-only registration checks | they allow dead commands to report installation success
Blast: system
Undo: easy
Certainty: firm
Record-Id: r-install595
Provenance: authored
Verified: npx tsc --noEmit; npx vitest run test/installer-hosts.test.ts; npx vitest run test/install-script.test.ts test/install-ps1.test.ts test/doctor.test.ts
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

CommitLore — record lint

Trailers: clean — 8 commits in origin/main..f9162fcfec57945ef5c492a3eb6e48c4ae60235f
Active constraints: not read — commitlore: git log --follow accepts exactly one pathspec, so renames are not followed for 12 paths; query one path at a time to follow its rename chain (12 changed paths)

Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR.

MongLong0214 and others added 7 commits August 13, 2026 23:13
The installer test fixture now gives the installer a self-contained Git working
directory. This keeps runtime smoke validation from resolving Docker's linked
worktree metadata on the macOS host, and the test now names and asserts the
non-zero unhealthy-MCP result it actually exercises.

Ruled-out: changing installer runtime verification | Linux evidence showed the smoke-test failure came from the linked-worktree test cwd
Blast: module
Undo: easy
Certainty: firm
Record-Id: r-linuxcwd595
Follows: r-install595
Provenance: authored
Verified: Linux/amd64 Node 24 Docker focused installer test; Linux/amd64 Node 24 Docker installer-hosts and doctor suites
The bundle was rebuilt on linux/amd64 so the committed bytes match what CI
builds; a macOS build of the same source differs because esbuild resolves a
platform-specific binary.

Provenance: authored
Record-Id: r-distmerge595
Verified: docker linux/amd64 npm ci and npm run build; 274 files in dist
install.sh redirected the host command's stdout to a file and let its stderr
go wherever it liked, then exited with its status. When that command failed
without printing to stdout, the user saw the summary header, an empty line,
and a non-zero exit -- told the install did not succeed and never told what
was wrong or with which host.

That is the defect this issue is about, inverted. A silent success and a
silent failure are the same design error: an exit code is not a diagnosis.

CI showed the shape exactly. The host command exited non-zero having written
neither stdout nor stderr, and the installer had nothing to say about it.
Whatever the command manages to report is now surfaced, and reporting nothing
at all is itself reported rather than swallowed.

Limit: this surfaces what the host command said; it does not diagnose a command that says nothing, and that case is now named as unknown rather than guessed at
Ruled-out: letting stderr flow straight to the terminal | it interleaves with the wrapper's own output and is lost entirely when a caller captures only stdout, which is how this went unnoticed
Blast: local
Undo: easy
Certainty: firm
Provenance: authored
Record-Id: r-hostsay595
Verified: 55 cases across install-script and installer-hosts on a real Linux git repository in a linux/amd64 container; sh -n install.sh clean
Unverified: which failure CI is actually hitting -- that is what this change exists to reveal, and the next run reports it rather than swallowing it
The probe writes an MCP initialize to the registered command's stdin. A
command that is not an MCP server usually proves it by exiting at once, and
that write then lands on a closed pipe. Node delivers EPIPE as an error event
on the stream, and nothing was listening, so an unhandled error took the whole
inspector down.

The installer then exited non-zero having reported nothing about any host --
strictly worse than the unhealthy verdict it was one step away from giving,
and the same silent-failure shape this issue exists to remove.

CI showed it directly:

  Error: write EPIPE
      at probeMcp (dist/commitlore.mjs:23970)
  Unhandled error event

Whether the write or the child exit wins is a race, so the crash appears under
load and not on a quiet machine.

Limit: the negative control could not be reproduced outside CI -- with the handler removed the suite still passes locally and in a linux container, because the probe reaches its five-second timeout instead of losing the race
Ruled-out: swallowing the write failure silently | the probe would then wait out its full timeout for a command already known to be gone, and report a timeout rather than the closed input that actually happened
Blast: local
Undo: easy
Certainty: tentative
Provenance: authored
Record-Id: r-epipe595
Verified: 12 cases in installer-hosts including a fixture that closes its own stdin and stays alive; tsc clean; dist rebuilt on linux/amd64 at 274 files
Unverified: that this is the CI failure's whole cause -- the stack trace names this write and this handler covers it, but the crash has not been reproduced outside CI, so the next run is the evidence
@MongLong0214
MongLong0214 merged commit 0d60c75 into main Aug 13, 2026
12 checks passed
@MongLong0214
MongLong0214 deleted the fix-595-installer-truth branch August 13, 2026 18:29
MongLong0214 added a commit that referenced this pull request Aug 17, 2026
install.ps1 delegates to `installer-hosts` at :817 and exits on its status. The 351 lines after that exit have been unreachable since #624, and every has_/wire_ pair in install.sh is defined and never called. Both files kept describing host support they had stopped performing.

That is not free. #689 was caused by reading install.sh as the list of supported hosts, its body records two more readers reaching the same wrong conclusion the same day, and today I read install.ps1:881 as live behaviour and filed #712 on it before checking control flow. Three readers in three days, once past the repository boundary.

Deleting unreachable code cannot change what an install does, which removes the Windows machine from this step: the verification recorded on the issue assumed the blocks participate in wiring, and they cannot.

Four guards were asserting against the deleted copies and now target the enumeration that runs. #689's coverage guard was the one that mattered: it compared install.sh's pairs to the CLI, and with the shell gone a list read out of the module under test would shrink with it. It now calls inspectAndApplyHosts with an empty PATH and home and requires every known host in notDetected -- a branch that returns without pushing is missing code, invisible to any text search.

Limit: install.ps1 line coverage is unchanged -- what ran before still runs
Ruled-out: keep the blocks until a Windows machine confirms wiring | they sit after an unconditional exit, so no run can distinguish their presence from their absence
Blast: system
Undo: easy
Certainty: firm
Record-Id: r-dead691
Provenance: authored
Verified: dropping the windsurf row from the enumeration fails the rewritten guard with 'windsurf appears in neither hosts nor notDetected'; 142 cases pass across the seven installer suites; npx tsc --noEmit clean; sh -n install.sh; npm run build:canonical then artifact:verify -> de9222c7187eb818ab20cfea63fa3e918e56f7eca1dc9fff654cae4bf39c0daf (dist unchanged, no src in this change); test/init.test.ts fails 6 cases on clean main here and in this branch alike -- a local environment difference, not this change
CommitLore-Version: 2.0.0
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.

Both installers still skip a host whose registration cannot run, and exit 0

1 participant