Fix #1622: classifySelfProvenance calls a project-local root global-candidate, so the automatic lane probes and reports degraded forever

[philcunliffe]

Feature or issue

classifySelfProvenance classified a copy of hypaware inside a project's node_modules as global-candidate, because its only node_modules test was "is there such a segment at all" and a global root (<prefix>/lib/node_modules/hypaware) has one too. Executed against that shape, the doomed npm install -g never actually ran: applySelfUpdate compares the root against npm's prefix and refuses with not_global_install. What the misread cost was everything around that refusal, every TTL tick and forever: a daily registry probe that LLP 0309 #global-install-only exists to prevent, an npm config get prefix subprocess to ask what the path already answers, a sticky apply_failed: not_global_install that renders as self-update: degraded (...) on every hyp status for a machine with nothing wrong with it, and describeSelfUpdate's staleDaemon comparison reading identity.version as "the version installed on this machine" from a process that is not that install.

Solution

• classifySelfProvenance returns a fourth verdict, project-local, decided by the existing isEphemeralBinPath (a manifest beside the outermost node_modules, which a project carries and <prefix>/lib does not) rather than new path logic. Every gate in the updater is already written against !== 'global-candidate', so the automatic lane now skips before it probes and the status comparison stays off; hyp update reaches the refusal already written for a checkout and an npx cache, naming the third shape, instead of the generic "install failed".
• The change only ever refuses more: no lane installs where it previously did not. A genuine npm install -g root, an _npx run, and a HypAware dev clone are all unchanged, asserted in the same test and re-checked against this machine's real npm prefix.
• hyp daemon install's resolveDurableBinPath (lane 1 of the issue) is deliberately not touched. Widening it to the same predicate makes a daemon install from a project that happens to depend on hypaware spend an unrequested global install with network and global-prefix writes, and the alternatives (refuse with the repair named, or point at an already-installed durable hyp via findInstalledHypawareBin, or warn and record the ephemeral path as the bake sites do) trade consent against version skew in a way LLP 0017 #install-global-package-then-service-manager settles only for npx. Two core lanes still ask the durable-bin question _npx-only after #1619: hyp daemon install and classifySelfProvenance #1622 stays open for that lane.

Code: +30 / -4 lines

Refs #1622

[philcunliffe]

Review round 1 (head 575f4b4d) - approve with one fix landed

Reviewed in an isolated worktree at 575f4b4d. Verdict: sound. One actionable finding (LOW, user-facing string) was fixed and pushed as 5edac31b. Two further observations are recorded as accepted residuals with the evidence for accepting them. npm test: 6538 pass, 3 fail, all three the known pre-existing test/core/hyparquet-floor-pin.test.js failures that reproduce on origin/master. npm run typecheck clean.

1. Direction of change: verified, zero shapes moved toward installing

Not inherited from the author's report. I loaded classifySelfProvenance from origin/master and from this head side by side in one process and ran 16 install shapes through both:

    npm global posix <prefix>/lib/node_modules/hypaware   old=global-candidate  new=global-candidate
    npm global win32 <prefix>/node_modules/hypaware       old=global-candidate  new=global-candidate
    nvm global                                            old=global-candidate  new=global-candidate
 *  project dependency                                    old=global-candidate  new=project-local
 *  transitive dependency                                 old=global-candidate  new=project-local
    npx (npm_config_cache set)                            old=npx               new=npx
    npx (no npm_config_cache)                             old=npx               new=npx
    dev clone (bare)                                      old=checkout          new=checkout
    dev clone w/ its own node_modules                     old=checkout          new=checkout
    clone inside a project's node_modules                 old=checkout          new=checkout
 *  pnpm global root                                      old=global-candidate  new=project-local
 *  yarn global root                                      old=global-candidate  new=project-local
    loose dir, no .git no node_modules                    old=checkout          new=checkout
    node_modules_old decoy segment                        old=checkout          new=checkout
    node_modules with no manifest beside it               old=global-candidate  new=global-candidate
    manifest is a DIRECTORY (unreadable)                  old=global-candidate  new=global-candidate

shapes: 16   changed: 4   toward-installing regressions: 0

Structurally this is guaranteed, not merely observed: the new line is inserted immediately before return 'global-candidate', so it can only ever convert a global-candidate into something else. No shape can acquire global-candidate that did not have it. The claim holds.

A genuine global install still updates, exercised rather than inferred: the three npm global layouts above (posix <prefix>/lib/node_modules, win32 <prefix>/node_modules, and an nvm versioned prefix) all still return global-candidate, and the branch's own test drives a global-candidate root through runSelfUpdatePass to an applied npm install -g. The two shapes where isEphemeralBinPath could have misfired against a real global root, a node_modules with no manifest beside it and one whose package.json is a directory, both stay global-candidate: the helper's fail direction is "durable", so its worst misfire really is today's behaviour and never a new refusal.

Nothing that previously self-updated correctly has stopped. The four changed shapes were all already refused one guard later: applySelfUpdate (self_update.js:608) realpath-compares the root against <npm prefix>/lib/node_modules/<name> and returns not_global_install before spending an install. I re-derived the issue's two disproved claims independently and confirm both: the doomed npm install -g never ran (observed subprocess calls per automatic tick on the branch: []; on master, npm config get prefix and nothing else), and hyp update exited 1 with install of 1.1.0 failed (not_global_install), not a false success.

2. Classification correctness: correct, with one documented false positive

Every shape in the matrix lands where it should, including the two the brief called out: a HypAware dev clone with its own node_modules is still checkout (the .git test fires first, and the clone root carries no node_modules segment anyway), and a transitive node_modules/tool/node_modules/hypaware is project-local on the outermost tree, which is the rule isEphemeralBinPath already documents.

The one imprecision is inherent to the reused predicate: pnpm's global/<n> and yarn's config/yarn/global write a manifest beside their global root, so a global install under either reads project-local. This is not new information, it is the finding issue #1625 raised against this same predicate and settled two commits ago. See finding 3 and residual B.

3. FINDING (LOW, fixed) - src/core/commands/update.js:154 reintroduced the exact wording #1625 removed

const how = ... : "a project's node_modules". Given the pnpm/yarn false positive above, hyp update told a pnpm- or yarn-global user "this install runs from a project's node_modules", pointing them at a checkout and an npm ci they do not have. That is verbatim the claim commit 0bfcb698 (issue #1625) took out of describeEphemeralBinPath one commit earlier on this same branch's history, on this same predicate, and test/core/global-install.test.js:184 asserts doesNotMatch(/inside a project's node_modules/) for those trees. #1625's acceptance condition was explicit: name the tree "correctly (or neutrally, without asserting a project node_modules)".

Fixed in 5edac31b: the arm now reads 'a node_modules tree' and carries a comment naming #1625 so it is not written back. The repair the message points at (npm install -g <name>) is unchanged and is correct for a project dependency and a pnpm/yarn global root alike. The same honesty pass went into classifySelfProvenance's JSDoc and the SelfInstallProvenance doc comment in types.d.ts, both of which asserted "some project's node_modules", and the pnpm-shaped global root is now asserted in the test so a later change to that verdict is a decision someone makes on purpose rather than a rediscovery.

4. Residual A (accepted) - the stale apply_failed on an already-affected machine

The author flagged this and declined to fix it. I uphold that, on stronger evidence than the author gave. The reason it is safe is not just that clearing it would mean adding a state write to a lane that writes nothing (true, and it would put a disk write on the pre-boot lane every tick, and it would contradict LLP 0309 #cli-surface's explicit reasoning that the text line derives only from the shared state file because the process rendering status is not necessarily the install that wrote it). The stronger reason is that the stale line names its own cure and the cure works. Executed against the branch:

seeded            line: self-update: degraded (apply_failed: not_global_install); run 'hyp update' or 'npm install -g hypaware@latest'
auto pass:        {"action":"skipped","reason":"project-local"}  subprocesses: []
after auto        line: self-update: degraded (apply_failed: not_global_install); ...   <- the wart
forced (hyp update): {"action":"checked","reason":"project-local","latest":"1.1.0"}  subprocesses: []
state.error after forced: undefined
after hyp update  line: self-update: 1.1.0 available (running 1.0.0)

hyp update takes the force path, probes, and the successful-probe write at self_update.js:936 sets error: undefined, error_since: undefined before the provenance gate at 981. So one run of the command the degraded line itself prints clears the state permanently, with no network install and no subprocess. And the line it leaves behind is self-healing in turn: available is re-derived against identity.version on every render, so once the project bumps its dependency past that version the line goes null on its own. A permanently red line that prints a one-command cure which actually works is not the symptom this PR is fixing.

5. Residual B (accepted, not filed as an issue) - pnpm/yarn global roots lose their status line

Worth stating plainly because it is the largest real cost of the change. For a pnpm- or yarn-global install, master probed daily, was refused by applySelfUpdate, and left self-update: degraded (apply_failed: not_global_install); run 'hyp update' or 'npm install -g hypaware@latest' on hyp status. That line was wrong about the cause but right about the repair. On this branch the pass returns before the probe, so available is never written, staleDaemon is gated off at self_update.js:1276, and describeSelfUpdate returns line: null.

I am accepting this rather than filing a follow-up, for three reasons:

• It takes nothing an apply would have given them. Those roots were already refused by the prefix comparison; no install is lost, only a notice.
• It is the settled shape of the surface, not a new hole. checkout and npx have returned line: null from the text line since LLP 0309, for the same reason. hyp update still probes from anywhere and still names the shape, which is the surface #cli-surface reserves for the manual lane, and hyp status --json carries provenance.
• Separating a pnpm/yarn global root from a project tree needs a heuristic neither predicate has (both trees carry a manifest, a lockfile and a node_modules, with nothing on the tree to tell them apart), and Ephemeral-bin warning's else arm misdescribes pnpm/yarn global roots as a project's node_modules #1625 ruled that new heuristic out of scope for this predicate on the record. Filing it again would re-litigate a settled call. The consequence is now written into the JSDoc and pinned by an assertion instead.

If a maintainer disagrees, the escalation is a new heuristic recognizing pnpm/yarn global root shapes, benefiting describeEphemeralBinPath and this verdict together. It is not this PR's size.

6. Pre-boot import cost: verified, LLP 0309 #unstick-from-the-front holds

src/core/cli/global_install.js imports only Node builtins (node:child_process, node:fs, node:fs/promises, node:path, node:process, node:url) and no repo module, so the pre-boot lane's import graph does not widen by a single application file. Its module-level work is two pure path computations (fileURLToPath(new URL(...)) and one path.join) plus a class declaration: no I/O at load, confirmed by loading it in isolation. Of its builtins, only node:fs/promises and node:process are new to this lane; self_update.js already imported node:child_process, node:fs, node:path and node:url, and already imports three ../daemon/* modules, so the lane was never builtin-only to begin with. The constraint is met.

7. Test quality: they bite

Both new tests were confirmed non-vacuous by stashing the source. With the single line if (isEphemeralBinPath(root, env)) return 'project-local' removed from classifySelfProvenance and nothing else changed:

not ok 1 - classifySelfProvenance separates a project dependency from the global install
not ok 2 - the automatic lane refuses a project-local install instead of installing beside it
# pass 0  # fail 2

and with it restored, both pass. They are not self-agreeing: the second drives the real runSelfUpdatePass and asserts the absence of work (probe.calledCount() === 0, calls empty, state.error undefined, line null), which is the only shape that can catch this class of bug, and then re-drives the same root with force to prove the manual lane is still reachable and still names the shape. The staleDaemon assertion at the end pins the third symptom independently. Coverage gap, pre-existing and not introduced here: no test anywhere in test/ exercises runUpdate, so the checkout/npx/project-local message strings are unasserted in all three arms. Not fixed, since there is no existing harness to extend and the smallest change rule applies.

8. Conventions

No semicolons, no em dashes (U+2014) anywhere in the diff including my own commit, no NUL bytes, no @typedef, no inline import('...') types, no TypeScript types outside .d.ts, root-anchored .js type-import specifiers unchanged, no new runtime dependencies (package.json and package-lock.json untouched). No new @ref annotations were added, correctly: the existing @ref LLP 0309#global-install-only [implements] on classifySelfProvenance still describes exactly what that function does, and the new verdict is that guard doing its job, not a new decision. Read LLP 0309 and its Extended-by LLP 0365 before ruling; nothing in either is contradicted, and #global-install-only's "a skipped install does not probe the registry either" is more nearly true after this change than before. No LLP is owed: this narrows a predicate to match a decision already on the record, it does not change one.

9. CPU and memory pass

No concern; the change is a net reduction.

• Added per call: one path.resolve, one bounded upward walk over path segments (O(path depth), ~10 to 20 iterations of path.dirname, short-lived strings only, no accumulator), and at most one statSync. classifySelfProvenance already did an fs.existsSync on the same root, so worst-case syscalls on this path go from 1 to 2.
• Call frequency: once per update pass (daily, or hourly after an unhealthy boot) and once per hyp status render. Not a hot path, not per-record, not per-row.
• Net effect on the affected population is strongly negative work: a daily fetch to the npm registry, an npm config get prefix subprocess spawn, an apply-lock take and release, and a state-file write are all removed and replaced by one statSync. Measured subprocess calls per automatic tick: master 1, branch 0. Network calls: master 1, branch 0.
• Memory: nothing retained, nothing accumulated, no growth with uptime or data volume. The added import puts one more small builtin-only module in the pre-boot process; String.prototype.split(path.sep) in the classifier is unchanged from master.
• Long-running processes: the daemon calls this once per pass and holds no reference to anything it produces.

Scope

Confirmed the PR touches lane 2 only. resolveDurableBinPath in src/core/daemon/install.js is untouched, and the body's reasoning for leaving it (LLP 0017 #install-global-package-then-service-manager scopes its rule to npx, and nothing in the corpus settles consent for spending an unrequested global install) is sound. The body ends Refs #1622, not Fixes; gh pr view 1634 --json closingIssuesReferences returns [], so #1622 stays open for the lane-1 consent decision. I made no edit to the PR body.