fix(upgrade): stop npm installs self-shadowing on upgrade; verify the resolved version after every upgrade (#1238, #1071) (#1245)
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
edb9f2f14c
commit
63eb488ed4
+147
-1
@@ -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<UpgradeDeps> & { 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
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user