diff --git a/bin/gstack-version-bump b/bin/gstack-version-bump index 298fab17d..d1aae05d0 100755 --- a/bin/gstack-version-bump +++ b/bin/gstack-version-bump @@ -21,18 +21,22 @@ // the caller must handle), exit 2 on bad args / unresolvable base. // // write --version [--version-path

] -// Validates the 4-digit pattern, writes VERSION + package.json.version. -// Use for the FRESH bump (or an approved queue rebump). Exit 3 on a -// half-write (VERSION written, package.json failed) so the caller knows -// drift exists; the next classify() will report DRIFT_STALE_PKG. +// Validates the 4-digit pattern, writes VERSION + package.json.version + +// package-lock.json.version (when a lockfile is present). Use for the FRESH +// bump (or an approved queue rebump). Exit 2 on a lockfile we can't parse — +// preflighted, so nothing is written. Exit 3 on a half-write (an earlier +// file written, a later one failed) so the caller knows drift exists; the +// next classify() will report DRIFT_STALE_PKG. // // repair [--version-path

] -// DRIFT_STALE_PKG path: sync package.json.version to the current VERSION -// file. No bump. Validates the VERSION pattern first. +// DRIFT_STALE_PKG path: sync package.json.version (+ the lockfile, when +// present) to the current VERSION file. No bump. Validates the VERSION +// pattern first. // -// Contract: classify NEVER writes. write/repair mutate VERSION + package.json -// only. No git mutation, no network. Mirrors gstack-next-version's reader/writer -// split so /ship composes them. +// Contract: classify NEVER writes. write/repair mutate VERSION + package.json + +// package-lock.json only. Parse failures are preflighted before the first write, +// so a file we can't read never leaves a partial bump behind. No git mutation, no +// network. Mirrors gstack-next-version's reader/writer split so /ship composes them. import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { execFileSync } from "node:child_process"; @@ -101,6 +105,77 @@ function writePkgVersion(cwd: string, version: string): void { writeFileSync(pkgPath, JSON.stringify(parsed, null, 2) + "\n"); } +/** A prepared package-lock.json write, or null when there's no lockfile to sync. */ +type LockPlan = { path: string; content: string } | null; + +/** + * Preflight the package-lock.json sync: read, parse, and prepare the updated + * content in memory WITHOUT writing anything. Returns null when the lockfile is + * absent (yarn/pnpm/bun lockfile, or no JS at all) — a silent skip, not an error. + * Exits 2 on an unreadable/malformed/non-object lockfile. + * + * Split from the write so the deterministic failure — a lockfile we can't parse — + * is caught BEFORE VERSION or package.json are touched. Previously this ran after + * both were written, so a malformed lockfile exited 2 having already bumped them, + * leaving the three files inconsistent. + * + * lockfileVersion 3 stores the version in two places: top-level `version` and + * `packages[""].version`. Both must move together or `npm ci` warns. + * + * The state classifier intentionally does NOT inspect the lockfile: a stale + * lockfile root version doesn't break `npm ci` or anything runtime, so lockfile-only + * drift is opportunistically corrected on FRESH/repair but is not a halt condition. + * That non-blocking stance applies to a STALE lockfile, not to a malformed one we + * were asked to write. + */ +function planLockVersion(cwd: string, version: string): LockPlan { + const lockPath = join(cwd, "package-lock.json"); + if (!existsSync(lockPath)) return null; + let raw: string; + try { + raw = readFileSync(lockPath, "utf-8"); + } catch { + fail("package-lock.json exists but could not be read. Fix the file before re-running /ship.", 2); + } + let parsed: Record; + try { + parsed = JSON.parse(raw) as Record; + } catch { + fail("package-lock.json is not valid JSON. Fix the file before re-running /ship.", 2); + } + // JSON.parse happily returns a number/string/array/null. Assigning .version to a + // primitive throws under ESM strict mode, so reject a non-object shape up front. + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + fail("package-lock.json is not a JSON object. Fix the file before re-running /ship.", 2); + } + parsed.version = version; + // packages[""] is best-effort: absent on lockfileVersion 1, and a corrupt lockfile + // can put anything there. A truthy primitive would throw on property assignment + // under ESM strict mode, so check the shape rather than just truthiness — skipping + // an unusable nested root matches how we already skip an absent one. + const packages = parsed.packages; + if (packages && typeof packages === "object" && !Array.isArray(packages)) { + const root = (packages as Record)[""]; + if (root && typeof root === "object" && !Array.isArray(root)) { + (root as Record).version = version; + } + } + return { path: lockPath, content: JSON.stringify(parsed, null, 2) + "\n" }; +} + +/** + * Commit a plan prepared by planLockVersion. Returns true if a lockfile was + * written, false when there was none. Throws on I/O failure; the caller maps that + * to exit 3 (a genuine half-write we report rather than hide — permissions can + * change between preflight and write, and closing that TOCTOU window fully would + * need temp-file + rename, which is out of scope for this fix). + */ +function commitLockVersion(plan: LockPlan): boolean { + if (!plan) return false; + writeFileSync(plan.path, plan.content); + return true; +} + function baseVersion(cwd: string, base: string, versionRel: string): string { // Verify the base ref resolves, mirroring the Step 12 guard. try { @@ -160,6 +235,9 @@ function cmdWrite(args: string[], cwd: string): void { fail(`NEW_VERSION (${version}) does not match MAJOR.MINOR.PATCH.MICRO. Aborting.`, 2); } const versionPath = resolveVersionPath(cwd, argVal(args, "--version-path")); + // Preflight the lockfile before touching anything: a malformed lockfile must + // fail with VERSION and package.json untouched, not half-bumped. + const lockPlan = planLockVersion(cwd, version!); writeFileSync(versionPath, version + "\n"); if (existsSync(join(cwd, "package.json"))) { try { @@ -172,7 +250,18 @@ function cmdWrite(args: string[], cwd: string): void { ); } } - process.stdout.write(JSON.stringify({ wrote: version, packageJson: existsSync(join(cwd, "package.json")) }) + "\n"); + let lockfile = false; + try { + lockfile = commitLockVersion(lockPlan); + } catch { + fail( + "failed to update package-lock.json. VERSION and package.json were written but the lockfile is now stale.", + 3, + ); + } + process.stdout.write( + JSON.stringify({ wrote: version, packageJson: existsSync(join(cwd, "package.json")), packageLock: lockfile }) + "\n", + ); } function cmdRepair(args: string[], cwd: string): void { @@ -188,12 +277,20 @@ function cmdRepair(args: string[], cwd: string): void { if (!existsSync(join(cwd, "package.json"))) { fail("repair: no package.json to sync.", 2); } + // Same preflight as write: parse the lockfile before mutating package.json. + const lockPlan = planLockVersion(cwd, current); try { writePkgVersion(cwd, current); } catch { fail("drift repair failed — could not update package.json.", 3); } - process.stdout.write(JSON.stringify({ repaired: current }) + "\n"); + let lockfile = false; + try { + lockfile = commitLockVersion(lockPlan); + } catch { + fail("drift repair failed — could not update package-lock.json.", 3); + } + process.stdout.write(JSON.stringify({ repaired: current, packageLock: lockfile }) + "\n"); } // Exported for unit tests (pure logic, no I/O). diff --git a/test/gstack-version-bump.test.ts b/test/gstack-version-bump.test.ts index ffcecd1a7..741d7deda 100644 --- a/test/gstack-version-bump.test.ts +++ b/test/gstack-version-bump.test.ts @@ -54,7 +54,7 @@ describe('write (FRESH bump)', () => { fs.writeFileSync(path.join(dir, 'VERSION'), '1.0.0.0\n'); fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0.0', scripts: { t: 'y' } }, null, 2) + '\n'); const out = execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: dir }).toString(); - expect(JSON.parse(out)).toEqual({ wrote: '1.1.0.0', packageJson: true }); + expect(JSON.parse(out)).toEqual({ wrote: '1.1.0.0', packageJson: true, packageLock: false }); expect(fs.readFileSync(path.join(dir, 'VERSION'), 'utf-8').trim()).toBe('1.1.0.0'); const pkg = JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf-8')); expect(pkg.version).toBe('1.1.0.0'); @@ -72,7 +72,7 @@ describe('write (FRESH bump)', () => { const d2 = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-noPkg-')); fs.writeFileSync(path.join(d2, 'VERSION'), '0.1.0.0\n'); const out = execFileSync('bun', [BIN, 'write', '--version', '0.2.0.0'], { cwd: d2 }).toString(); - expect(JSON.parse(out)).toEqual({ wrote: '0.2.0.0', packageJson: false }); + expect(JSON.parse(out)).toEqual({ wrote: '0.2.0.0', packageJson: false, packageLock: false }); expect(fs.readFileSync(path.join(d2, 'VERSION'), 'utf-8').trim()).toBe('0.2.0.0'); fs.rmSync(d2, { recursive: true, force: true }); }); @@ -86,7 +86,7 @@ describe('repair (DRIFT_STALE_PKG)', () => { fs.writeFileSync(path.join(dir, 'VERSION'), '2.0.0.0\n'); fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'x', version: '1.9.0.0' }, null, 2) + '\n'); const out = execFileSync('bun', [BIN, 'repair'], { cwd: dir }).toString(); - expect(JSON.parse(out)).toEqual({ repaired: '2.0.0.0' }); + expect(JSON.parse(out)).toEqual({ repaired: '2.0.0.0', packageLock: false }); expect(JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf-8')).version).toBe('2.0.0.0'); expect(fs.readFileSync(path.join(dir, 'VERSION'), 'utf-8').trim()).toBe('2.0.0.0'); // unchanged }); @@ -100,6 +100,143 @@ describe('repair (DRIFT_STALE_PKG)', () => { }); }); +describe('package-lock.json sync', () => { + // lockfileVersion 3 stores the version in two places: top-level `version` + // and packages[""].version. Both must move together or `npm ci` warns. + const mkLock = (dir: string, version: string) => + fs.writeFileSync( + path.join(dir, 'package-lock.json'), + JSON.stringify({ name: 'x', version, lockfileVersion: 3, packages: { '': { name: 'x', version } } }, null, 2) + '\n', + ); + + test('write syncs lockfile root + packages[""] version, reports packageLock: true', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-write-')); + fs.writeFileSync(path.join(d, 'VERSION'), '1.0.0.0\n'); + fs.writeFileSync(path.join(d, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0.0' }, null, 2) + '\n'); + mkLock(d, '1.0.0.0'); + const out = execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: d }).toString(); + expect(JSON.parse(out)).toEqual({ wrote: '1.1.0.0', packageJson: true, packageLock: true }); + const lock = JSON.parse(fs.readFileSync(path.join(d, 'package-lock.json'), 'utf-8')); + expect(lock.version).toBe('1.1.0.0'); + expect(lock.packages[''].version).toBe('1.1.0.0'); + fs.rmSync(d, { recursive: true, force: true }); + }); + + test('repair syncs lockfile up to VERSION, reports packageLock: true', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-repair-')); + fs.writeFileSync(path.join(d, 'VERSION'), '2.0.0.0\n'); + fs.writeFileSync(path.join(d, 'package.json'), JSON.stringify({ name: 'x', version: '1.9.0.0' }, null, 2) + '\n'); + mkLock(d, '1.9.0.0'); + const out = execFileSync('bun', [BIN, 'repair'], { cwd: d }).toString(); + expect(JSON.parse(out)).toEqual({ repaired: '2.0.0.0', packageLock: true }); + const lock = JSON.parse(fs.readFileSync(path.join(d, 'package-lock.json'), 'utf-8')); + expect(lock.version).toBe('2.0.0.0'); + expect(lock.packages[''].version).toBe('2.0.0.0'); + fs.rmSync(d, { recursive: true, force: true }); + }); + + test('malformed package-lock.json fails write with exit 2', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-bad-')); + fs.writeFileSync(path.join(d, 'VERSION'), '1.0.0.0\n'); + fs.writeFileSync(path.join(d, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0.0' }, null, 2) + '\n'); + fs.writeFileSync(path.join(d, 'package-lock.json'), '{ not valid json'); + let code = 0; + try { execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: d, stdio: 'pipe' }); } + catch (e: any) { code = e.status; } + expect(code).toBe(2); + fs.rmSync(d, { recursive: true, force: true }); + }); + + // The lockfile is parsed in a preflight, before VERSION or package.json are + // touched. Without it, a malformed lockfile exits 2 having already bumped both — + // the caller is left with three files disagreeing and no state that classify() + // can name. These two lock the "nothing written" half of that guarantee. + test('malformed package-lock.json leaves VERSION + package.json untouched on write', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-atomic-write-')); + fs.writeFileSync(path.join(d, 'VERSION'), '1.0.0.0\n'); + fs.writeFileSync(path.join(d, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0.0' }, null, 2) + '\n'); + fs.writeFileSync(path.join(d, 'package-lock.json'), '{ not valid json'); + let code = 0; + try { execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: d, stdio: 'pipe' }); } + catch (e: any) { code = e.status; } + expect(code).toBe(2); + expect(fs.readFileSync(path.join(d, 'VERSION'), 'utf-8').trim()).toBe('1.0.0.0'); + expect(JSON.parse(fs.readFileSync(path.join(d, 'package.json'), 'utf-8')).version).toBe('1.0.0.0'); + expect(fs.readFileSync(path.join(d, 'package-lock.json'), 'utf-8')).toBe('{ not valid json'); + fs.rmSync(d, { recursive: true, force: true }); + }); + + test('malformed package-lock.json leaves package.json untouched on repair', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-atomic-repair-')); + fs.writeFileSync(path.join(d, 'VERSION'), '2.0.0.0\n'); + fs.writeFileSync(path.join(d, 'package.json'), JSON.stringify({ name: 'x', version: '1.9.0.0' }, null, 2) + '\n'); + fs.writeFileSync(path.join(d, 'package-lock.json'), '{ not valid json'); + let code = 0; + try { execFileSync('bun', [BIN, 'repair'], { cwd: d, stdio: 'pipe' }); } + catch (e: any) { code = e.status; } + expect(code).toBe(2); + expect(JSON.parse(fs.readFileSync(path.join(d, 'package.json'), 'utf-8')).version).toBe('1.9.0.0'); + fs.rmSync(d, { recursive: true, force: true }); + }); + + test('non-object package-lock.json fails write with exit 2, nothing written', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-nonobj-')); + fs.writeFileSync(path.join(d, 'VERSION'), '1.0.0.0\n'); + fs.writeFileSync(path.join(d, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0.0' }, null, 2) + '\n'); + // Valid JSON, but not an object — assigning .version to it would throw. + fs.writeFileSync(path.join(d, 'package-lock.json'), '[]'); + let code = 0; + try { execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: d, stdio: 'pipe' }); } + catch (e: any) { code = e.status; } + expect(code).toBe(2); + expect(fs.readFileSync(path.join(d, 'VERSION'), 'utf-8').trim()).toBe('1.0.0.0'); + fs.rmSync(d, { recursive: true, force: true }); + }); + + // A corrupt lockfile can put anything at packages[""]. A truthy primitive there + // throws on property assignment under strict mode; the sync must degrade to + // "update what I can" rather than crash out with a stack trace. + test('unusable packages[""] degrades to a top-level-only sync, no crash', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-badpkgs-')); + fs.writeFileSync(path.join(d, 'VERSION'), '1.0.0.0\n'); + fs.writeFileSync(path.join(d, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0.0' }, null, 2) + '\n'); + fs.writeFileSync( + path.join(d, 'package-lock.json'), + JSON.stringify({ name: 'x', version: '1.0.0.0', lockfileVersion: 3, packages: { '': 5 } }, null, 2) + '\n', + ); + const out = execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: d }).toString(); + expect(JSON.parse(out)).toEqual({ wrote: '1.1.0.0', packageJson: true, packageLock: true }); + const lock = JSON.parse(fs.readFileSync(path.join(d, 'package-lock.json'), 'utf-8')); + expect(lock.version).toBe('1.1.0.0'); + expect(lock.packages['']).toBe(5); // left alone, not crashed on + fs.rmSync(d, { recursive: true, force: true }); + }); + + // The yarn/pnpm/bun (and no-JS) path: absence is a silent skip, never an error + // and never a created file. We only sync a lockfile that already exists; we never + // bring one into being. + test('write silently skips when no package-lock.json exists', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-absent-write-')); + fs.writeFileSync(path.join(d, 'VERSION'), '1.0.0.0\n'); + fs.writeFileSync(path.join(d, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0.0' }, null, 2) + '\n'); + const out = execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: d }).toString(); + expect(JSON.parse(out)).toEqual({ wrote: '1.1.0.0', packageJson: true, packageLock: false }); + expect(fs.existsSync(path.join(d, 'package-lock.json'))).toBe(false); + expect(fs.readFileSync(path.join(d, 'VERSION'), 'utf-8').trim()).toBe('1.1.0.0'); + fs.rmSync(d, { recursive: true, force: true }); + }); + + test('repair silently skips when no package-lock.json exists', () => { + const d = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-absent-repair-')); + fs.writeFileSync(path.join(d, 'VERSION'), '2.0.0.0\n'); + fs.writeFileSync(path.join(d, 'package.json'), JSON.stringify({ name: 'x', version: '1.9.0.0' }, null, 2) + '\n'); + const out = execFileSync('bun', [BIN, 'repair'], { cwd: d }).toString(); + expect(JSON.parse(out)).toEqual({ repaired: '2.0.0.0', packageLock: false }); + expect(fs.existsSync(path.join(d, 'package-lock.json'))).toBe(false); + fs.rmSync(d, { recursive: true, force: true }); + }); +}); + describe('classify (idempotency over a real git base)', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-classify-')); afterAll(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* noop */ } });