Merged
fix(npm): honor update --help and global flags, recognize pnpm installs #574
aflyingtoAdd on Sep 24
fix(npm): honor update --help and global flags, recognize pnpm installs #574
Merged
aflyingtoAdd on Sep 24
Total: 8 files changed+266-18
@@ -70,7 +70,7 @@ gitcode config set update.mode off
70GC_NO_UPDATE_CHECK=1 gitcode version70GC_NO_UPDATE_CHECK=1 gitcode version
71```71```
72 72 
73-`CI=true` and `--no-interactive` disable background checks. The updater touches only the coordinate you installed (for example `atomgit-cli`); it never invokes pip, Homebrew, apt, dnf, or rpm, and it never rewrites PATH. If another `gitcode` is earlier on PATH, npm's non-failing install check and `doctor install` report the exact candidates and remediation choices.73+`CI=true` and `--no-interactive` disable background checks. The updater touches only the coordinate you installed (for example `atomgit-cli`); it never invokes pip, Homebrew, apt, dnf, or rpm, and it never rewrites PATH. If another `gitcode` is earlier on PATH, npm's non-failing install check and `doctor install` report the exact candidates and remediation choices. Installs managed by pnpm are detected as such and get pnpm's own upgrade command (`pnpm add -g <coordinate>@latest`); the updater never installs a parallel npm copy for a pnpm-managed install.
74 74 
75## Troubleshooting75## Troubleshooting
76 76 
@@ -109,8 +109,32 @@ function npmInvocation(execPath = process.execPath, env = process.env, platform
109 return { command, prefix: [], metadataPath: command, shell: platform === "win32" };109 return { command, prefix: [], metadataPath: command, shell: platform === "win32" };
110}110}
111 111 
112+function isPnpmEnvironment(env, packageRoot, isWin) {
113+ if (/^pnpm\//.test(String((env || {}).npm_config_user_agent || ""))) return true;
114+ // Fallback for direct invocation outside a pnpm shim: pnpm's global layout.
115+ // Windows paths keep backslashes through normalizePath, so unify them.
116+ const normalized = normalizePath(packageRoot, isWin).replace(/\\/g, "/");
117+ return /\/pnpm\/global\//.test(normalized);
118+}
119+ 
112function discoverGlobalInstall(packageRoot, options = {}) {120function discoverGlobalInstall(packageRoot, options = {}) {
113- const invoke = options.npm || npmInvocation(options.execPath, options.env, options.platform);121+ const env = options.env || process.env;
122+ const isWin = (options.platform || process.platform) === "win32";
123+ if (isPnpmEnvironment(env, packageRoot, isWin)) {
124+ // pnpm manages its own global layout; the npm-based discovery below
125+ // (root -g comparison) would misclassify it, and an npm-channel update
126+ // would install a parallel npm copy.
127+ const layoutGlobal = /\/pnpm\/global\//.test(normalizePath(packageRoot, isWin).replace(/\\/g, "/"));
128+ return {
129+ schema: 1,
130+ distribution: "pnpm",
131+ global: layoutGlobal || String(env.npm_config_global) === "true",
132+ version: options.version || "",
133+ prefix: "",
134+ npm: "",
135+ };
136+ }
137+ const invoke = options.npm || npmInvocation(options.execPath, env, options.platform);
114 const runner = options.runner || spawnSync;138 const runner = options.runner || spawnSync;
115 const run = (args) =>139 const run = (args) =>
116 runner(invoke.command, [...invoke.prefix, ...args], {140 runner(invoke.command, [...invoke.prefix, ...args], {
@@ -122,7 +146,6 @@ function discoverGlobalInstall(packageRoot, options = {}) {
122 const root = run(["root", "-g"]);146 const root = run(["root", "-g"]);
123 const prefix = run(["prefix", "-g"]);147 const prefix = run(["prefix", "-g"]);
124 if (root.status !== 0 || prefix.status !== 0) return null;148 if (root.status !== 0 || prefix.status !== 0) return null;
125- const isWin = options.platform === "win32";
126 const pathAPI = isWin ? path.win32 : path.posix;149 const pathAPI = isWin ? path.win32 : path.posix;
127 const expectedRoot = normalizePath(root.stdout.trim(), isWin);150 const expectedRoot = normalizePath(root.stdout.trim(), isWin);
128 const actualRoot = normalizePath(pathAPI.resolve(packageRoot, "..", ".."), isWin);151 const actualRoot = normalizePath(pathAPI.resolve(packageRoot, "..", ".."), isWin);
@@ -163,6 +186,7 @@ module.exports = {
163 discoverGlobalInstall,186 discoverGlobalInstall,
164 ensureInstallMetadata,187 ensureInstallMetadata,
165 expectedGlobalBin,188 expectedGlobalBin,
189+ isPnpmEnvironment,
166 normalizePath,190 normalizePath,
167 npmInvocation,191 npmInvocation,
168 pathConflict,192 pathConflict,
@@ -4,7 +4,7 @@
4 4 
5const path = require("path");5const path = require("path");
6const pkg = require("../package.json");6const pkg = require("../package.json");
7-const { pathConflict, writeInstallMetadata } = require("./install-metadata");7+const { isPnpmEnvironment, pathConflict, writeInstallMetadata } = require("./install-metadata");
8 8 
9function isGlobalInstall(env = process.env) {9function isGlobalInstall(env = process.env) {
10 return env.npm_config_global === "true" || env.npm_config_global === true;10 return env.npm_config_global === "true" || env.npm_config_global === true;
@@ -16,19 +16,30 @@ function globalInstallReport(env = process.env, isWin = process.platform === "wi
16 return { prefix, ...pathConflict(prefix, env, isWin) };16 return { prefix, ...pathConflict(prefix, env, isWin) };
17}17}
18 18 
19-function runPostinstall(env = process.env, stderr = process.stderr) {19+function runPostinstall(env = process.env, stderr = process.stderr, packageRoot = path.resolve(__dirname, "..")) {
20 if (!isGlobalInstall(env)) return;20 if (!isGlobalInstall(env)) return;
21+ // pnpm global installs must never be recorded as npm installs: the npm
22+ // updater would then run through pnpm with npm-only flags and fail.
23+ if (isPnpmEnvironment(env, packageRoot, process.platform === "win32")) {
24+ writeInstallMetadata(packageRoot, {
25+ schema: 1,
26+ distribution: "pnpm",
27+ global: true,
28+ version: pkg.version,
29+ prefix: "",
30+ npm: "",
31+ });
32+ } else {
33+ writeInstallMetadata(packageRoot, {
34+ distribution: "npm",
35+ global: true,
36+ version: pkg.version,
37+ prefix: env.npm_config_prefix || "",
38+ npm: env.npm_execpath || "",
39+ });
40+ }
21 41 
22- const packageRoot = path.resolve(__dirname, "..");
23 const report = globalInstallReport(env);42 const report = globalInstallReport(env);
24- writeInstallMetadata(packageRoot, {
25- distribution: "npm",
26- global: true,
27- version: pkg.version,
28- prefix: report ? report.prefix : env.npm_config_prefix || "",
29- npm: env.npm_execpath || "",
30- });
31- 
32 if (!report) return;43 if (!report) return;
33 if (report.shadowed) {44 if (report.shadowed) {
34 stderr.write(45 stderr.write(
@@ -4,13 +4,34 @@
4 4 
5const { runUpdate } = require("./update");5const { runUpdate } = require("./update");
6 6 
7+const UPDATE_HELP = `Check for or apply an npm-channel update.
8+ 
9+Usage: gc update [flags]
10+ 
11+Flags:
12+ --check Check for an update without installing it
13+ --json Print a stable JSON object to stdout
14+ --background Internal: run as the detached background updater
15+ --no-update-check Accepted for CLI consistency (ignored here)
16+ --no-interactive Accepted for CLI consistency (ignored here)
17+ -h, --help Show this help
18+ 
19+This wrapper command manages global npm installs (atomgit-cli,
20+@atomgit-cli/cli, @gitcode-cli/cli). npm-bootstrap installations are
21+handled by the gc binary itself. Other channels (pip, Homebrew, deb, rpm)
22+stay user-controlled and are never invoked implicitly.
23+`;
24+ 
7function parseArgs(args) {25function parseArgs(args) {
8- const options = { background: false, checkOnly: false, json: false };26+ const options = { background: false, checkOnly: false, json: false, help: false };
9 for (const arg of args) {27 for (const arg of args) {
10 if (arg === "--background") options.background = true;28 if (arg === "--background") options.background = true;
11 else if (arg === "--check") options.checkOnly = true;29 else if (arg === "--check") options.checkOnly = true;
12 else if (arg === "--json") options.json = true;30 else if (arg === "--json") options.json = true;
13- else throw new Error(`unknown update argument: ${arg}`);31+ else if (arg === "--help" || arg === "-h") options.help = true;
32+ else if (arg === "--no-update-check" || arg === "--no-interactive") {
33+ // Global flags: accepted for CLI consistency, no effect here.
34+ } else throw new Error(`unknown update argument: ${arg}`);
14 }35 }
15 return options;36 return options;
16}37}
@@ -19,6 +40,10 @@ function main(args = process.argv.slice(2)) {
19 let options;40 let options;
20 try {41 try {
21 options = parseArgs(args);42 options = parseArgs(args);
43+ if (options.help) {
44+ if (!options.background) process.stdout.write(UPDATE_HELP);
45+ return 0;
46+ }
22 const result = runUpdate(options);47 const result = runUpdate(options);
23 if (!options.background) {48 if (!options.background) {
24 process.stdout.write(options.json ? `${JSON.stringify(result)}\n` : `${result.message}\n`);49 process.stdout.write(options.json ? `${JSON.stringify(result)}\n` : `${result.message}\n`);
@@ -38,4 +63,4 @@ function main(args = process.argv.slice(2)) {
38 63 
39if (require.main === module) process.exitCode = main();64if (require.main === module) process.exitCode = main();
40 65 
41-module.exports = { main, parseArgs };66+module.exports = { UPDATE_HELP, main, parseArgs };
@@ -249,6 +249,20 @@ function performUpdate(options = {}) {
249 const packageRoot = options.packageRoot || path.resolve(__dirname, "..");249 const packageRoot = options.packageRoot || path.resolve(__dirname, "..");
250 const metadata = options.metadata || readInstallMetadata(packageRoot);250 const metadata = options.metadata || readInstallMetadata(packageRoot);
251 if (!metadata || !metadata.global || metadata.distribution !== "npm" || !metadata.prefix) {251 if (!metadata || !metadata.global || metadata.distribution !== "npm" || !metadata.prefix) {
252+ if (metadata && metadata.distribution === "pnpm") {
253+ if (metadata.global) {
254+ throw new Error(`this installation is managed by pnpm; update it with "pnpm add -g ${pkg.name}@latest"`);
255+ }
256+ throw new Error(
257+ 'this pnpm-managed dependency is not a global install; update it with "pnpm update" in the owning project'
258+ );
259+ }
260+ if (metadata && metadata.distribution === "npm-local") {
261+ throw new Error(
262+ `automatic update requires a global npm install; run "npm install -g ${pkg.name}" to switch, ` +
263+ "or update through the channel that owns this installation"
264+ );
265+ }
252 throw new Error("automatic update is available only for a global npm installation");266 throw new Error("automatic update is available only for a global npm installation");
253 }267 }
254 const latest = checkLatest(metadata);268 const latest = checkLatest(metadata);
@@ -6,7 +6,7 @@ const fs = require("fs");
6const os = require("os");6const os = require("os");
7const path = require("path");7const path = require("path");
8const { commandCandidates, discoverGlobalInstall, normalizePath, pathConflict } = require("../lib/install-metadata");8const { commandCandidates, discoverGlobalInstall, normalizePath, pathConflict } = require("../lib/install-metadata");
9-const { isGlobalInstall } = require("../lib/postinstall");9+const { isGlobalInstall, runPostinstall } = require("../lib/postinstall");
10 10 
11test("normalizes Windows paths case-insensitively and trims separators", () => {11test("normalizes Windows paths case-insensitively and trims separators", () => {
12 assert.strictEqual(normalizePath("C:\\Users\\WPF\\npm\\", true), normalizePath("c:\\users\\wpf\\npm", true));12 assert.strictEqual(normalizePath("C:\\Users\\WPF\\npm\\", true), normalizePath("c:\\users\\wpf\\npm", true));
@@ -48,6 +48,7 @@ test("runtime discovery distinguishes global and project-local npm packages", ()
48 runner,48 runner,
49 platform: "linux",49 platform: "linux",
50 version: "1.2.3",50 version: "1.2.3",
51+ env: {},
51 });52 });
52 assert.strictEqual(global.global, true);53 assert.strictEqual(global.global, true);
53 assert.strictEqual(global.distribution, "npm");54 assert.strictEqual(global.distribution, "npm");
@@ -58,7 +59,89 @@ test("runtime discovery distinguishes global and project-local npm packages", ()
58 npm: { command: "node", prefix: ["npm-cli.js"], metadataPath: "npm-cli.js" },59 npm: { command: "node", prefix: ["npm-cli.js"], metadataPath: "npm-cli.js" },
59 runner,60 runner,
60 platform: "linux",61 platform: "linux",
62+ env: {},
61 });63 });
62 assert.strictEqual(local.global, false);64 assert.strictEqual(local.global, false);
63 assert.strictEqual(local.distribution, "npm-local");65 assert.strictEqual(local.distribution, "npm-local");
64});66});
67+ 
68+test("runtime discovery recognizes pnpm installs without invoking npm", () => {
69+ const runner = () => {
70+ throw new Error("npm must not be invoked for a pnpm install");
71+ };
72+ const metadata = discoverGlobalInstall("/home/u/.local/share/pnpm/global/5/node_modules/@gitcode-cli/cli", {
73+ npm: { command: "node", prefix: ["npm-cli.js"], metadataPath: "npm-cli.js" },
74+ runner,
75+ platform: "linux",
76+ version: "1.2.3",
77+ env: { npm_config_user_agent: "pnpm/9.12.0 npm/? node/v20.0.0 linux x64" },
78+ });
79+ assert.strictEqual(metadata.distribution, "pnpm");
80+ assert.strictEqual(metadata.global, true);
81+ assert.strictEqual(metadata.version, "1.2.3");
82+ 
83+ const byLayout = discoverGlobalInstall("/home/u/.local/share/pnpm/global/5/node_modules/@gitcode-cli/cli", {
84+ npm: { command: "node", prefix: ["npm-cli.js"], metadataPath: "npm-cli.js" },
85+ runner,
86+ platform: "linux",
87+ env: {},
88+ });
89+ assert.strictEqual(byLayout.distribution, "pnpm");
90+});
91+ 
92+test("postinstall records pnpm global installs as pnpm, never as npm", () => {
93+ const root = fs.mkdtempSync(path.join(os.tmpdir(), "gc-postinstall-pnpm-"));
94+ const packageRoot = path.join(root, "lib", "node_modules", "@gitcode-cli", "cli");
95+ fs.mkdirSync(packageRoot, { recursive: true });
96+ runPostinstall(
97+ { npm_config_global: "true", npm_config_user_agent: "pnpm/9.15.9 npm/? node/v20", npm_config_prefix: "/prefix" },
98+ { write: () => {} },
99+ packageRoot
100+ );
101+ const metadata = JSON.parse(fs.readFileSync(path.join(packageRoot, ".gitcode-install.json"), "utf8"));
102+ assert.strictEqual(metadata.distribution, "pnpm");
103+ assert.strictEqual(metadata.global, true);
104+ assert.strictEqual(metadata.prefix, "");
105+});
106+ 
107+test("postinstall keeps recording npm global installs as npm", () => {
108+ const root = fs.mkdtempSync(path.join(os.tmpdir(), "gc-postinstall-npm-"));
109+ const packageRoot = path.join(root, "lib", "node_modules", "@gitcode-cli", "cli");
110+ fs.mkdirSync(packageRoot, { recursive: true });
111+ runPostinstall(
112+ { npm_config_global: "true", npm_config_user_agent: "npm/10.0.0 node/v20", npm_config_prefix: "/prefix", npm_execpath: "/usr/bin/npm" },
113+ { write: () => {} },
114+ packageRoot
115+ );
116+ const metadata = JSON.parse(fs.readFileSync(path.join(packageRoot, ".gitcode-install.json"), "utf8"));
117+ assert.strictEqual(metadata.distribution, "npm");
118+ assert.strictEqual(metadata.prefix, "/prefix");
119+});
120+ 
121+test("pnpm layout detection also matches Windows global paths", () => {
122+ const runner = () => {
123+ throw new Error("npm must not be invoked for a pnpm install");
124+ };
125+ const metadata = discoverGlobalInstall("C:\\Users\\u\\AppData\\Local\\pnpm\\global\\5\\node_modules\\@gitcode-cli\\cli", {
126+ npm: { command: "node", prefix: ["npm-cli.js"], metadataPath: "npm-cli.js" },
127+ runner,
128+ platform: "win32",
129+ env: {},
130+ });
131+ assert.strictEqual(metadata.distribution, "pnpm");
132+ assert.strictEqual(metadata.global, true);
133+});
134+ 
135+test("a project-level pnpm dependency is pnpm but not global", () => {
136+ const runner = () => {
137+ throw new Error("npm must not be invoked for a pnpm install");
138+ };
139+ const metadata = discoverGlobalInstall("/workspace/node_modules/@gitcode-cli/cli", {
140+ npm: { command: "node", prefix: ["npm-cli.js"], metadataPath: "npm-cli.js" },
141+ runner,
142+ platform: "linux",
143+ env: { npm_config_user_agent: "pnpm/9.15.9 npm/? node/v20" },
144+ });
145+ assert.strictEqual(metadata.distribution, "pnpm");
146+ assert.strictEqual(metadata.global, false);
147+});
@@ -0,0 +1,90 @@
1+"use strict";
2+ 
3+// Tests for lib/update-helper.js (the `gc update` wrapper entry) and the
4+// performUpdate refusal guidance for non-npm-global channels.
5+ 
6+const test = require("node:test");
7+const assert = require("node:assert");
8+const fs = require("fs");
9+const os = require("os");
10+const path = require("path");
11+const { UPDATE_HELP, main, parseArgs } = require("../lib/update-helper");
12+const { performUpdate } = require("../lib/update");
13+ 
14+test("update parseArgs accepts help and CLI-consistency flags", () => {
15+ assert.deepStrictEqual(parseArgs([]), { background: false, checkOnly: false, json: false, help: false });
16+ for (const flag of ["--help", "-h"]) {
17+ assert.strictEqual(parseArgs([flag]).help, true, flag);
18+ }
19+ for (const flag of ["--no-update-check", "--no-interactive"]) {
20+ const options = parseArgs([flag]);
21+ assert.strictEqual(options.background, false, flag);
22+ assert.strictEqual(options.checkOnly, false, flag);
23+ assert.strictEqual(options.help, false, flag);
24+ }
25+ assert.strictEqual(parseArgs(["--check", "--json", "--background"]).checkOnly, true);
26+ assert.throws(() => parseArgs(["--force"]), /unknown update argument: --force/);
27+});
28+ 
29+test("gc update --help prints the wrapper help and exits zero", () => {
30+ const chunks = [];
31+ const original = process.stdout.write;
32+ process.stdout.write = (chunk) => {
33+ chunks.push(String(chunk));
34+ return true;
35+ };
36+ try {
37+ assert.strictEqual(main(["--help"]), 0);
38+ } finally {
39+ process.stdout.write = original;
40+ }
41+ const output = chunks.join("");
42+ assert.ok(output.includes("Usage: gc update"), output);
43+ assert.ok(output.includes("--check"), output);
44+ assert.ok(output.includes("--no-interactive"), output);
45+ assert.strictEqual(output, UPDATE_HELP);
46+});
47+ 
48+test("gc update --help --background stays silent for the detached path", () => {
49+ const chunks = [];
50+ const original = process.stdout.write;
51+ process.stdout.write = (chunk) => {
52+ chunks.push(String(chunk));
53+ return true;
54+ };
55+ try {
56+ assert.strictEqual(main(["--help", "--background"]), 0);
57+ } finally {
58+ process.stdout.write = original;
59+ }
60+ assert.strictEqual(chunks.length, 0);
61+});
62+ 
63+test("performUpdate names pnpm with its own upgrade command", () => {
64+ assert.throws(
65+ () => performUpdate({ metadata: { distribution: "pnpm", global: true } }),
66+ (error) => /managed by pnpm/.test(error.message) && /pnpm add -g /.test(error.message)
67+ );
68+});
69+ 
70+test("performUpdate points npm-local installs at the global switch", () => {
71+ assert.throws(
72+ () => performUpdate({ metadata: { distribution: "npm-local", global: false } }),
73+ (error) => /requires a global npm install/.test(error.message) && /npm install -g /.test(error.message)
74+ );
75+});
76+ 
77+test("performUpdate keeps the generic refusal when no metadata exists", () => {
78+ const root = fs.mkdtempSync(path.join(os.tmpdir(), "gc-no-metadata-"));
79+ assert.throws(
80+ () => performUpdate({ packageRoot: root }),
81+ /automatic update is available only for a global npm installation/
82+ );
83+});
84+ 
85+test("performUpdate distinguishes project-level pnpm dependencies from global installs", () => {
86+ assert.throws(
87+ () => performUpdate({ metadata: { distribution: "pnpm", global: false } }),
88+ (error) => /pnpm-managed dependency/.test(error.message) && /pnpm update/.test(error.message)
89+ );
90+});
@@ -207,6 +207,7 @@ test("update helper parser accepts check/json/background only", () => {
207 background: false,207 background: false,
208 checkOnly: true,208 checkOnly: true,
209 json: true,209 json: true,
210+ help: false,
210 });211 });
211 assert.throws(() => parseArgs(["--channel", "next"]), /unknown update argument/);212 assert.throws(() => parseArgs(["--channel", "next"]), /unknown update argument/);
212});213});