Skip to content

Fix #384: install the daemon against a durable global bin from every enrollment path#385

Merged
platypii merged 2 commits into
masterfrom
fix/issue-384
Jul 24, 2026
Merged

Fix #384: install the daemon against a durable global bin from every enrollment path#385
platypii merged 2 commits into
masterfrom
fix/issue-384

Conversation

@philcunliffe

Copy link
Copy Markdown
Contributor

Root cause

ensureDurableBinForNpx (the only code that turns an _npx invocation into a durable global bin) had exactly one call site: the walkthrough picker finale (src/core/cli/walkthrough.js), gated behind !skipInstall && !finale.binPath. Every other install path resolved binPath from process.argv[1] (the _npx bin) and skipped the upgrade:

  • hyp daemon install (runDaemonInstall in src/core/commands/daemon.js)
  • the enrolling remote login / hyp join lane, which finishes by calling runDaemonInstall([], ctx) via enrollCentralSink

So enrolling through any non-walkthrough path pinned launchd/systemd to the ephemeral ~/.npm/_npx/<hash>/... bin. When npx exits there is no hyp on PATH: the host is recorded but has no control surface (hyp status / hyp policy set / hyp detach / hyp daemon uninstall all impossible).

Fix (shared choke point)

Move the isNpxBinPath(...) -> ensureDurableBinForNpx(...) upgrade into installDaemon() (src/core/daemon/install.js) via a resolveDurableBinPath helper, run inside the existing daemon.install span before the platform unit is written. Every non-dry-run install funnels through installDaemon, so the walkthrough, hyp daemon install, and the join/enroll lane all inherit the invariant: a daemon is never installed against an _npx bin.

  • The upgrade result rides back on the install plan (plan.globalInstall) so the walkthrough still reports summary.globalInstall and span attributes.
  • The now-redundant one-off in the walkthrough finale is removed, so the upgrade runs exactly once (idempotent).

Escape hatches preserved

  • Explicit --bin: callers set binExplicit: true (walkthrough uses finale.binPath !== undefined, runDaemonInstall uses parsed.binPath !== undefined); installDaemon keeps that bin verbatim.
  • Dry-run: renders through planDaemonInstall / renderDaemonInstall, which never reach installDaemon, so the upgrade never runs.

Regression test

test/core/daemon-install-durable-bin.test.js drives installDaemon (platform darwin, fake launchctl, injected npm runner seam so no real global install happens) with an _npx-style binPath and asserts the written plist references a durable global bin, not the _npx path. It also asserts explicit --bin and dry-run bypass. Confirmed failing on origin/master (3 of 5 subtests fail) and passing with the fix.

Test result

Full npm test: 2629 pass, 7 fail. The 7 failures are the pre-existing test/core/leave-command.test.js failures present on plain master, unrelated to this change.

Fixes #384

neutral-loop and others added 2 commits July 23, 2026 23:30
…enrollment path

ensureDurableBinForNpx had a single call site (the walkthrough picker
finale), so `hyp daemon install` and the enrolling `remote login`/`join`
lane installed launchd/systemd against the ephemeral `~/.npm/_npx/<hash>`
bin. When npx exits that bin is gone, leaving the host recorded but with
no `hyp` control surface (status/policy/detach/uninstall all impossible).

Move the isNpxBinPath -> ensureDurableBinForNpx upgrade into installDaemon,
the single choke point every non-dry-run install funnels through, so the
walkthrough, `hyp daemon install`, and join/enroll all inherit the
invariant "a daemon is never installed against an _npx bin." The upgrade
result rides back on the install plan (globalInstall) so the walkthrough
still reports it. Escape hatches preserved: an explicit --bin sets
binExplicit and is kept verbatim; dry-run renders through planDaemonInstall
and never reaches the upgrade. The now-redundant walkthrough one-off is
removed so the upgrade runs exactly once.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

neutral review — PR #385 (head cc9e842)

Verdict: CLEAN. No actionable findings. The invariant "a daemon is never installed against an _npx bin" holds across every install path, both escape hatches survive, and the regression test genuinely exercises the fix. Nothing fixed/pushed; head unchanged.

Choke-point coverage (invariant verified)

resolveDurableBinPath() in src/core/daemon/install.js:173 runs inside the single installDaemon() operation body (install.js:217-224), and the low-level writers macos.installLaunchAgent / linux.installSystemdUnit have exactly one caller each — install.js:221-222 (grep confirms no other call sites outside install.js). Every install path funnels through it:

  • hyp daemon installrunDaemonInstall (src/core/commands/daemon.js:221) → installDaemon
  • walkthrough finale → installMod.installDaemon (src/core/cli/walkthrough.js:846) ✓
  • join → central.js:158 runDaemonInstallinstallDaemon
  • enrolling login → central.js:246 runDaemonInstallinstallDaemon

No path writes launchd/systemd bypassing installDaemon. planDaemonInstall/renderDaemonInstall (dry-run) never reach resolveDurableBinPath.

Escape hatch #1 — explicit --bin (binExplicit) kept verbatim

Threaded as binExplicit: parsed.binPath !== undefined (daemon.js:194) and finale.binPath !== undefined (walkthrough.js:827). Join passes ['--bin', parsed.binPath] only when the user supplied one (central.js:157), so binExplicit is true iff explicit. resolveDurableBinPath short-circuits on options.binExplicit (install.js:176) and returns binPath untouched. Verified by test 2 ("explicit --bin bypasses the durable upgrade even for an _npx-looking path": no npm run, plist keeps _npx).

Escape hatch #2 — dry-run never triggers a real global install

planDaemonInstall/renderDaemonInstall never call resolveDurableBinPath. Verified by test 3 (renderDaemonInstall leaves the _npx binPath untouched) and test 5 (runDaemonInstall --dry-run --bin ... --platform darwin returns 0, renders the bin, no npm install).

No double upgrade

The walkthrough's one-off ensureDurableBinForNpx call was removed; the upgrade now runs exactly once inside installDaemon. The walkthrough reads the result back off plan.globalInstall for its summary/span. Idempotent single site.

Failure handling (no silent _npx pin)

ensureDurableBinForNpx throws on npm install -g / npm config get prefix failure (global_install.js:48-55, 109-115); the throw propagates through resolveDurableBinPathinstallDaemon's withDaemonOp (logs daemon.install_failed, rethrows) → runDaemonInstall catch (daemon.js:225, returns 1). It fails cleanly and never falls back to writing the _npx bin.

Regression test integrity

Reverting src/core/daemon/install.js + types.d.ts to the pre-fix parent (7dfcce4) makes the new test fail 3/5 (the two core-upgrade cases + the argv case); the fixed tree passes 5/5. The test writes a real launchd plist to a temp home and asserts !content.includes('_npx') and content.includes(GLOBAL_BIN) — it exercises the real install path, not a stub. The CI type change is only a cast on the partial ctx mock argument (/** @type {any} */ (...) then /** @type {CommandRunContext} */ (ctx)); it touches no asserted value, so it does not weaken the assertions (code === 0, out.includes(NPX_BIN)).

Style / LLP

No semicolons or em-dashes (U+2014) in any added line (the em-dashes flagged by grep are all pre-existing prose in types.d.ts). Type imports via top-of-file @import (install.js:42) and import type in types.d.ts; no inline import('...') types in the changed .js. @ref LLP 0025#seed-config-mode resolves to the "## Seed-config mode" heading and matches the established anchor already used in central.js; annotation is honest.

Checks

  • Affected suites: daemon.test.js, daemon-install-durable-bin.test.js, global-install.test.js, join-command.test.js, walkthrough-tui-happy.test.js — 31/31 pass.
  • New regression test — 5/5 pass.
  • npm run typecheck (tsc) — clean.
  • (Pre-existing unrelated test/core/leave-command.test.js failures not in scope.)

@philcunliffe philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Jul 23, 2026
@platypii
platypii merged commit 420cb2a into master Jul 24, 2026
8 checks passed
@platypii
platypii deleted the fix/issue-384 branch July 24, 2026 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

npx enrollment leaves no durable hyp on PATH: durable-bin upgrade has one call site but the daemon installs from several

2 participants