From b10850058e88a6aef1cd40771add7a717f39ef9c Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Fri, 10 Jul 2026 08:52:14 -0500 Subject: [PATCH] fix(upgrade): stop npm installs self-shadowing on upgrade; verify the resolved version after every upgrade (#1238, #1071) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes to make `codegraph upgrade` trustworthy in the terminal it ran in: 1. detectInstallMethod checked the bundle layout before the node_modules path check, but the npm thin-installer's per-platform package IS a complete bundle inside node_modules — so every npm install misdetected as a standalone bundle, and upgrade curled install.sh into ~/.codegraph: a second install that never wins the PATH race against npm's shim, leaving `codegraph -v` permanently on the old version. Path-based checks (_npx, node_modules) now win over layout sniffing, so npm installs upgrade through npm again, in place. 2. After a successful swap, runUpgrade now probes the PATH-resolved `codegraph --version` and reports the real outcome: a green confirmation that this terminal already serves the new version, a loud shadow warning naming the fix (`which -a codegraph`) on mismatch, or the old soft new-terminal hint only when the probe is inconclusive. Replaces the unconditional "open a new terminal if the version looks unchanged" hedge. Skipped for npm-local installs, whose binary PATH never serves. Companion to #1239: the misdetection also broke its post-upgrade `install --refresh` for npm users (the spawn resolved the stale shim). Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 2 + __tests__/upgrade.test.ts | 148 +++++++++++++++++++++++++++++++++++++- src/bin/codegraph.ts | 1 + src/upgrade/index.ts | 108 ++++++++++++++++++++++++---- 4 files changed, 246 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eef0b6151..522e43a0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- `codegraph upgrade` on an npm install now upgrades through npm again instead of quietly creating a second copy that never wins the PATH race — previously `codegraph --version` kept reporting the old version forever, no matter how many times you upgraded. (#1238) +- After every upgrade, CodeGraph now checks that the `codegraph` command your terminal resolves actually serves the freshly installed version — confirming you don't need a new terminal, or telling you exactly which stale install is shadowing the new one. (#1071) - The safety watchdog no longer kills a healthy index on severely degraded storage. It used to judge liveness purely by the event loop, so one long database write on a struggling disk looked identical to a hung process and could get a valid, in-progress index terminated. During `codegraph index`/`codegraph init` the watchdog now also checks whether the index files on disk are advancing before it acts: slow-but-progressing work is left alone (bounded by a hard cap), while a genuinely hung process is still killed exactly as fast as before. (#1231) ## [1.4.0] - 2026-07-10 diff --git a/__tests__/upgrade.test.ts b/__tests__/upgrade.test.ts index ceef23280..66a4ee6c0 100644 --- a/__tests__/upgrade.test.ts +++ b/__tests__/upgrade.test.ts @@ -13,6 +13,7 @@ import { parseLatestTagFromLocation, reindexAdvisory, runUpgrade, + verifyResolvedVersion, buildWindowsUpgradeScript, NPM_PACKAGE, type InstallMethod, @@ -89,6 +90,41 @@ describe('detectInstallMethod', () => { expect(m).toEqual({ kind: 'npx' }); }); + // The npm thin-installer's per-platform package IS a complete bundle + // (vendored node + bin/ launcher) sitting inside node_modules. The layout + // sniff must not win over the node_modules path check, or `upgrade` curls + // install.sh into ~/.codegraph — a second install that loses the PATH race + // to npm's shim, so `codegraph -v` stays on the old version forever. + it('detects the npm thin-installer platform package as npm, not bundle', () => { + const root = '/usr/local/lib/node_modules/@colbymchenry/codegraph/node_modules/@colbymchenry/codegraph-linux-x64'; + const filename = `${root}/lib/dist/bin/codegraph.js`; + const present = new Set([`${root}/node`, `${root}/bin/codegraph`]); + const m = detectInstallMethod({ + filename, + platform: 'linux', + cwd: '/home/u/project', + exists: bundleExists(present), + }); + expect(m).toEqual({ kind: 'npm', scope: 'global' }); + }); + + it('detects a project-local thin-installer platform package as npm local', () => { + const cwd = '/home/u/project'; + const root = `${cwd}/node_modules/@colbymchenry/codegraph/node_modules/@colbymchenry/codegraph-darwin-arm64`; + const filename = `${root}/lib/dist/bin/codegraph.js`; + const present = new Set([`${root}/node`, `${root}/bin/codegraph`]); + const m = detectInstallMethod({ filename, platform: 'darwin', cwd, exists: bundleExists(present) }); + expect(m).toEqual({ kind: 'npm', scope: 'local' }); + }); + + it('still detects an npx run when the cached platform package has the bundle layout', () => { + const root = '/home/u/.npm/_npx/abc123/node_modules/@colbymchenry/codegraph/node_modules/@colbymchenry/codegraph-linux-x64'; + const filename = `${root}/lib/dist/bin/codegraph.js`; + const present = new Set([`${root}/node`, `${root}/bin/codegraph`]); + const m = detectInstallMethod({ filename, platform: 'linux', cwd: '/home/u', exists: bundleExists(present) }); + expect(m).toEqual({ kind: 'npx' }); + }); + it('detects a source checkout via sibling package.json + .git', () => { const repo = '/home/u/dev/codegraph'; const filename = `${repo}/dist/bin/codegraph.js`; @@ -191,6 +227,7 @@ describe('version helpers', () => { interface Calls { runs: Array<{ cmd: string; args: string[]; env?: NodeJS.ProcessEnv }>; + captures: Array<{ cmd: string; args: string[] }>; logs: string[]; errors: string[]; } @@ -199,7 +236,7 @@ function makeDeps( overrides: Partial & { method: InstallMethod; currentVersion: string }, runExit = 0 ): { deps: UpgradeDeps; calls: Calls } { - const calls: Calls = { runs: [], logs: [], errors: [] }; + const calls: Calls = { runs: [], captures: [], logs: [], errors: [] }; const deps: UpgradeDeps = { currentVersion: overrides.currentVersion, method: overrides.method, @@ -208,6 +245,12 @@ function makeDeps( calls.runs.push({ cmd, args, env }); return runExit; }, + // Default probe: spawn fails → 'inconclusive'. Tests that exercise the + // post-upgrade version check override this. + capture: (cmd, args) => { + calls.captures.push({ cmd, args }); + return overrides.capture ? overrides.capture(cmd, args) : null; + }, hasCommand: overrides.hasCommand ?? ((c) => c === 'curl'), log: (m) => calls.logs.push(m), warn: (m) => calls.logs.push(m), @@ -365,6 +408,109 @@ describe('runUpgrade', () => { }); }); +// --------------------------------------------------------------------------- +// Post-upgrade version probe — does the PATH-resolved `codegraph` serve the +// version we just installed, in THIS terminal? +// --------------------------------------------------------------------------- + +describe('post-upgrade version probe', () => { + const npmGlobal = { method: { kind: 'npm', scope: 'global' } as InstallMethod, currentVersion: '0.9.8' }; + + it('match: confirms the same terminal already serves the new version', async () => { + const { deps, calls } = makeDeps({ + ...npmGlobal, + hasCommand: (c) => c === 'codegraph', + capture: () => ({ code: 0, stdout: '0.9.9\n' }), + }); + const code = await runUpgrade({}, deps); + expect(code).toBe(0); + expect(calls.captures).toEqual([{ cmd: 'codegraph', args: ['--version'] }]); + const out = calls.logs.join('\n'); + expect(out).toMatch(/now reports v0\.9\.9/); + expect(out).not.toMatch(/Open a new terminal/); + }); + + it('mismatch: warns that a shadowing install is still serving the old version', async () => { + const { deps, calls } = makeDeps({ + ...npmGlobal, + hasCommand: (c) => c === 'codegraph', + capture: () => ({ code: 0, stdout: '0.9.8\n' }), + }); + const code = await runUpgrade({}, deps); + expect(code).toBe(0); // the upgrade itself succeeded — warn, don't fail + const out = calls.logs.join('\n'); + expect(out).toMatch(/still reports an older version/); + expect(out).toMatch(/shadowing/); + expect(out).toMatch(/which -a codegraph/); + }); + + it('inconclusive: falls back to the soft new-terminal hint when codegraph is not on PATH', async () => { + const { deps, calls } = makeDeps(npmGlobal); // hasCommand resolves only curl + const code = await runUpgrade({}, deps); + expect(code).toBe(0); + expect(calls.captures).toHaveLength(0); + expect(calls.logs.join('\n')).toMatch(/Open a new terminal/); + }); + + it('inconclusive: a failing or unparsable probe never warns about shadowing', async () => { + const { deps, calls } = makeDeps({ + ...npmGlobal, + hasCommand: (c) => c === 'codegraph', + capture: () => ({ code: 0, stdout: 'something went wrong\n' }), + }); + const code = await runUpgrade({}, deps); + expect(code).toBe(0); + const out = calls.logs.join('\n'); + expect(out).not.toMatch(/shadowing/); + expect(out).toMatch(/Open a new terminal/); + }); + + it('parses the last non-empty line, so a runtime warning above the version is harmless', () => { + const { deps } = makeDeps({ + ...npmGlobal, + hasCommand: (c) => c === 'codegraph', + capture: () => ({ code: 0, stdout: '(node:1) ExperimentalWarning: blah\nv0.9.9\n\n' }), + }); + expect(verifyResolvedVersion('v0.9.9', deps)).toBe('match'); + }); + + it('routes the probe through cmd.exe on Windows (.cmd launcher)', async () => { + const { deps, calls } = makeDeps({ + ...npmGlobal, + platform: 'win32', + hasCommand: (c) => c === 'codegraph' || c === 'npm.cmd', + capture: () => ({ code: 0, stdout: '0.9.9\r\n' }), + }); + const code = await runUpgrade({}, deps); + expect(code).toBe(0); + expect(calls.captures).toEqual([{ cmd: 'cmd.exe', args: ['/d', '/s', '/c', 'codegraph --version'] }]); + expect(calls.logs.join('\n')).toMatch(/now reports v0\.9\.9/); + }); + + it('skips the probe for npm-local installs — PATH serves a different copy', async () => { + const { deps, calls } = makeDeps({ + method: { kind: 'npm', scope: 'local' }, + currentVersion: '0.9.8', + hasCommand: (c) => c === 'codegraph', + capture: () => ({ code: 0, stdout: '0.9.7\n' }), + }); + const code = await runUpgrade({}, deps); + expect(code).toBe(0); + expect(calls.captures).toHaveLength(0); + expect(calls.logs.join('\n')).not.toMatch(/shadowing/); + }); + + it('does not probe after a failed upgrade', async () => { + const { deps, calls } = makeDeps( + { ...npmGlobal, hasCommand: (c) => c === 'codegraph', capture: () => ({ code: 0, stdout: '0.9.9\n' }) }, + 1 + ); + const code = await runUpgrade({}, deps); + expect(code).toBe(1); + expect(calls.captures).toHaveLength(0); + }); +}); + // --------------------------------------------------------------------------- // Re-index staleness — real index, real metadata stamp // --------------------------------------------------------------------------- diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 826305c73..75950cc43 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -2351,6 +2351,7 @@ program method, resolveLatest: () => up.resolveLatestVersion(), run: up.defaultRun, + capture: up.defaultCapture, hasCommand: up.hasCommand, log: (m: string) => console.log(m), warn: (m: string) => warn(m), diff --git a/src/upgrade/index.ts b/src/upgrade/index.ts index e98f5fff8..9fc97df3d 100644 --- a/src/upgrade/index.ts +++ b/src/upgrade/index.ts @@ -99,19 +99,20 @@ export function detectInstallMethod(input: DetectInput): InstallMethod { const P = isWin ? path.win32 : path.posix; const binDir = P.dirname(input.filename); // <…>/bin - // Bundle: /lib/dist/bin/codegraph.js → is up 3 from bin/. - // A bundle has a vendored node + a launcher script as siblings of lib/. - const bundleRoot = P.resolve(binDir, '..', '..', '..'); - const vendoredNode = P.join(bundleRoot, isWin ? 'node.exe' : 'node'); - const launcher = P.join(bundleRoot, 'bin', isWin ? 'codegraph.cmd' : 'codegraph'); - if (exists(vendoredNode) && exists(launcher)) { - const os = isWin ? 'windows' : 'unix'; - return { kind: 'bundle', os, bundleRoot, installDir: deriveInstallDir(bundleRoot, os, exists) }; - } - const norm = toPosix(input.filename); + // Path-based checks come FIRST. The npm thin-installer's per-platform + // package (@colbymchenry/codegraph--) is itself a complete + // bundle — vendored node + bin/ launcher — living inside node_modules, so + // the layout sniff below would misread every npm install as a standalone + // bundle. `upgrade` would then curl install.sh into ~/.codegraph: a SECOND + // install that never wins the PATH race against npm's shim, leaving + // `codegraph -v` permanently on the old version (the #1071 shadow, + // self-inflicted). A path under node_modules is authoritative about HOW the + // user installed, whatever the artifact inside looks like. + // npx cache: <…>/_npx//node_modules/@colbymchenry/codegraph/… + // (checked before npm — the npx cache path also contains /node_modules/). if (norm.includes('/_npx/')) { return { kind: 'npx' }; } @@ -122,6 +123,16 @@ export function detectInstallMethod(input: DetectInput): InstallMethod { return { kind: 'npm', scope: underCwd ? 'local' : 'global' }; } + // Bundle: /lib/dist/bin/codegraph.js → is up 3 from bin/. + // A bundle has a vendored node + a launcher script as siblings of lib/. + const bundleRoot = P.resolve(binDir, '..', '..', '..'); + const vendoredNode = P.join(bundleRoot, isWin ? 'node.exe' : 'node'); + const launcher = P.join(bundleRoot, 'bin', isWin ? 'codegraph.cmd' : 'codegraph'); + if (exists(vendoredNode) && exists(launcher)) { + const os = isWin ? 'windows' : 'unix'; + return { kind: 'bundle', os, bundleRoot, installDir: deriveInstallDir(bundleRoot, os, exists) }; + } + // Source checkout: running /dist/bin/codegraph.js with a sibling .git. const repoRoot = P.resolve(binDir, '..', '..'); if (exists(P.join(repoRoot, 'package.json')) && exists(P.join(repoRoot, '.git'))) { @@ -278,6 +289,8 @@ export interface UpgradeDeps { resolveLatest: (pin?: string) => Promise; /** Run a command inheriting stdio; returns its exit code (-1 = spawn failed). */ run: (cmd: string, args: string[], env?: NodeJS.ProcessEnv) => number; + /** Run a command capturing stdout (nothing reaches the terminal); null = spawn failed. */ + capture: (cmd: string, args: string[]) => { code: number; stdout: string } | null; hasCommand: (cmd: string) => boolean; log: (msg: string) => void; warn: (msg: string) => void; @@ -376,6 +389,11 @@ export async function runUpgrade(opts: UpgradeOptions, deps: UpgradeDeps): Promi // an existing Claude config, and skipped entirely by the kill-switch. Never // fatal to the upgrade. if (code === 0) { + try { + reportResolvedVersion(latest, deps); + } catch { + /* an inconclusive probe must not fail the upgrade */ + } try { await selfHealPromptHook(deps); } catch { @@ -385,6 +403,59 @@ export async function runUpgrade(opts: UpgradeOptions, deps: UpgradeDeps): Promi return code; } +type VersionProbe = 'match' | 'mismatch' | 'inconclusive'; + +/** + * Prove the upgrade actually took: spawn the `codegraph` this terminal's PATH + * resolves and compare its reported version to the target. Catches the silent + * failure mode where ANOTHER install shadows the one we just upgraded (issue + * #1071 — e.g. a stale `npm i -g` copy earlier on PATH than the bundle + * launcher): the upgrade "succeeds" but `codegraph -v` — in this terminal and + * every future one — keeps serving the old version. Exported for unit tests. + */ +export function verifyResolvedVersion(latest: string, deps: UpgradeDeps): VersionProbe { + if (!deps.hasCommand('codegraph')) return 'inconclusive'; + // Windows installs expose codegraph through a .cmd launcher; Node can't + // spawn .cmd files without a shell, so route through cmd.exe there. + const probe = deps.platform === 'win32' + ? deps.capture('cmd.exe', ['/d', '/s', '/c', 'codegraph --version']) + : deps.capture('codegraph', ['--version']); + if (!probe || probe.code !== 0) return 'inconclusive'; + // `codegraph --version` prints the bare version; take the last non-empty + // line so a stray runtime warning above it can't spoil the parse. + const reported = probe.stdout.trim().split(/\r?\n/).pop()?.trim() ?? ''; + if (!parseSemver(reported)) return 'inconclusive'; + return compareVersions(reported, latest) === 0 ? 'match' : 'mismatch'; +} + +/** + * Log the outcome of the post-upgrade version probe. On a match the user + * knows the current terminal is already serving the new version; on a + * mismatch they get told exactly which stale install is hijacking their PATH + * instead of discovering it via a mysteriously unchanged `codegraph -v`. + * Inconclusive probes fall back to the old soft hint — never a scare on + * setups we can't inspect (no `codegraph` on PATH yet, exotic wrappers). + */ +function reportResolvedVersion(latest: string, deps: UpgradeDeps): void { + const { method } = deps; + // A project-local npm install isn't served by PATH's `codegraph` (that + // would be some other install) — a probe could only false-alarm. + if (method.kind === 'npm' && method.scope === 'local') return; + switch (verifyResolvedVersion(latest, deps)) { + case 'match': + deps.log(c.green(`✓ \`codegraph\` on your PATH now reports ${latest} — this terminal is already using it.`)); + break; + case 'mismatch': + deps.warn(`Installed ${latest}, but the \`codegraph\` this terminal resolves still reports an older version.`); + deps.log(c.dim('Another CodeGraph install earlier on your PATH is shadowing the one just upgraded.')); + deps.log(c.dim('Find every copy with `which -a codegraph` (Windows: `where codegraph`) and remove or upgrade the stale one.')); + break; + case 'inconclusive': + deps.log(c.dim('Open a new terminal if `codegraph --version` looks unchanged (PATH cache).')); + break; + } +} + /** * Wire the Claude `UserPromptSubmit` front-load hook on upgrade for an * already-configured global Claude install. No-op when Claude isn't configured, @@ -429,7 +500,9 @@ function upgradeUnixBundle( return 1; } deps.log(''); - deps.log(c.green('✓ Upgrade complete.') + c.dim(' Open a new terminal if the version looks unchanged (PATH cache).')); + // No "open a new terminal" hedge here — after the swap, runUpgrade probes + // the PATH-resolved `codegraph --version` and reports the real outcome. + deps.log(c.green('✓ Upgrade complete.')); deps.log(reindexAdvisory()); return 0; } @@ -484,7 +557,9 @@ function upgradeWindowsBundle( return 1; } deps.log(''); - deps.log(c.green('✓ Upgrade complete.') + c.dim(' Open a new terminal to be safe (PATH/version cache).')); + // The running node.exe was renamed aside, so the version probe in + // runUpgrade already exercises the NEW binary — no terminal hedge needed. + deps.log(c.green('✓ Upgrade complete.')); deps.log(reindexAdvisory()); return 0; } @@ -550,3 +625,12 @@ export function defaultRun(cmd: string, args: string[], env?: NodeJS.ProcessEnv) if (r.error) return -1; return r.status ?? -1; } + +export function defaultCapture(cmd: string, args: string[]): { code: number; stdout: string } | null { + // stdio is piped (the default with `encoding`), so nothing the probed + // command prints reaches the user's terminal. The timeout keeps a wedged + // probe from hanging the upgrade's last step. + const r = spawnSync(cmd, args, { encoding: 'utf-8', windowsHide: true, timeout: 30_000 }); + if (r.error) return null; + return { code: r.status ?? -1, stdout: r.stdout ?? '' }; +}