mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix: write/repair sync npm lockfiles' version fields (#2567)
npm records the package version twice in its lockfiles — top-level `version` and, in lockfileVersion >= 2, `packages[""].version` (the entry describing the root package itself) — and `npm install` keeps both in step. gstack-version-bump write/repair updated VERSION + package.json but left the lockfile behind, so every /ship bump in an npm repo drifted one field per release until someone ran npm, dirtying the tree on the next `npm install` far from the cause. write and repair now mirror the version into package-lock.json AND npm-shrinkwrap.json (which shares the format and, when present, is what npm actually honors) as a pure JSON edit — no npm spawn, no dependency-tree churn, dependency entries untouched. Per the wave plan's version-tooling end-state spec (decision 11): synced ONLY when the file already exists, never created (gstack itself is bun-only). A failed manifest/lockfile write keeps the existing exit-3 half-write semantics so classify reports DRIFT_STALE_PKG on re-run instead of hiding the drift. Tests: 5 new cases in test/gstack-version-bump.test.ts — both lockfile version fields synced with deps untouched, repair heals a stale lockfile, lockfileVersion 1 (no packages map) doesn't crash, npm-shrinkwrap.json synced without inventing a package-lock.json, malformed lockfile exits 3 loudly (26 pass total in the file). Re-derived from PR #2568 by @ortonom under decision 11. Fixes #2567 Co-authored-by: ortonom <3261546+ortonom@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
ortonom
Claude Fable 5
parent
d7ab20ac06
commit
c33b371f25
+48
-6
@@ -31,8 +31,9 @@
|
||||
// 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.
|
||||
// + npm lockfiles (package-lock.json / npm-shrinkwrap.json, when present)
|
||||
// only. 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";
|
||||
@@ -108,6 +109,38 @@ function writePkgVersion(cwd: string, version: string): void {
|
||||
writeFileSync(pkgPath, JSON.stringify(parsed, null, 2) + "\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* npm records the package version twice in its lockfiles — top-level
|
||||
* `version` and, in lockfileVersion >= 2, `packages[""].version` (the entry
|
||||
* describing the root package itself) — and `npm install` keeps both in
|
||||
* step. Nothing else in a release does, so a lockfile left behind drifts one
|
||||
* field per bump until someone runs npm, dirtying the tree on the next
|
||||
* `npm install` far from the cause (#2567). Pure JSON edit: no npm spawn,
|
||||
* no dependency-tree churn.
|
||||
*
|
||||
* Synced ONLY when the file already exists — never created (gstack itself
|
||||
* is bun-only; decision pinned in the v1.67 fix-wave plan).
|
||||
* npm-shrinkwrap.json shares the format and, when present, is what npm
|
||||
* actually honors, so both names are covered. Returns the names synced.
|
||||
*/
|
||||
const NPM_LOCKFILES = ["package-lock.json", "npm-shrinkwrap.json"];
|
||||
function syncNpmLockfiles(dir: string, version: string): string[] {
|
||||
const synced: string[] = [];
|
||||
for (const name of NPM_LOCKFILES) {
|
||||
const lockPath = join(dir, name);
|
||||
if (!existsSync(lockPath)) continue;
|
||||
const parsed = JSON.parse(readFileSync(lockPath, "utf-8")) as Record<string, unknown>;
|
||||
parsed.version = version;
|
||||
const packages = parsed.packages as Record<string, Record<string, unknown>> | undefined;
|
||||
if (packages && typeof packages[""] === "object" && packages[""] !== null) {
|
||||
packages[""].version = version;
|
||||
}
|
||||
writeFileSync(lockPath, JSON.stringify(parsed, null, 2) + "\n");
|
||||
synced.push(name);
|
||||
}
|
||||
return synced;
|
||||
}
|
||||
|
||||
function baseVersion(cwd: string, base: string, versionRel: string): string {
|
||||
// Verify the base ref resolves, mirroring the Step 12 guard.
|
||||
try {
|
||||
@@ -194,18 +227,26 @@ function cmdWrite(args: string[], cwd: string): void {
|
||||
}
|
||||
|
||||
writeFileSync(versionPath, version + "\n");
|
||||
let lockSynced: string[] = [];
|
||||
if (existsSync(join(cwd, "package.json"))) {
|
||||
try {
|
||||
writePkgVersion(cwd, version!);
|
||||
lockSynced = syncNpmLockfiles(cwd, version!);
|
||||
} catch {
|
||||
fail(
|
||||
"failed to update package.json. VERSION was written but package.json is now stale. " +
|
||||
"Re-run — classify will report DRIFT_STALE_PKG and repair will sync it.",
|
||||
"failed to update package.json/npm lockfiles. VERSION was written but the npm " +
|
||||
"manifests are now stale. Re-run — classify will report DRIFT_STALE_PKG and repair will sync them.",
|
||||
3,
|
||||
);
|
||||
}
|
||||
}
|
||||
process.stdout.write(JSON.stringify({ wrote: version, packageJson: existsSync(join(cwd, "package.json")) }) + "\n");
|
||||
process.stdout.write(
|
||||
JSON.stringify({
|
||||
wrote: version,
|
||||
packageJson: existsSync(join(cwd, "package.json")),
|
||||
packageLock: lockSynced.length > 0,
|
||||
}) + "\n",
|
||||
);
|
||||
}
|
||||
|
||||
function cmdRepair(args: string[], cwd: string): void {
|
||||
@@ -233,8 +274,9 @@ function cmdRepair(args: string[], cwd: string): void {
|
||||
}
|
||||
try {
|
||||
writePkgVersion(cwd, current);
|
||||
syncNpmLockfiles(cwd, current);
|
||||
} catch {
|
||||
fail("drift repair failed — could not update package.json.", 3);
|
||||
fail("drift repair failed — could not update package.json/npm lockfiles.", 3);
|
||||
}
|
||||
process.stdout.write(JSON.stringify({ repaired: current }) + "\n");
|
||||
}
|
||||
|
||||
@@ -62,7 +62,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');
|
||||
@@ -80,7 +80,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 });
|
||||
});
|
||||
@@ -108,6 +108,78 @@ describe('repair (DRIFT_STALE_PKG)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('write/repair sync npm lockfiles (both version fields, #2567)', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-lock-'));
|
||||
afterAll(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* noop */ } });
|
||||
|
||||
const lock = (v: string) => JSON.stringify({
|
||||
name: 'x', version: v, lockfileVersion: 3,
|
||||
packages: { '': { name: 'x', version: v }, 'node_modules/a': { version: '9.9.9' } },
|
||||
}, null, 2) + '\n';
|
||||
|
||||
test('write updates top-level version and packages[""].version, leaves deps alone', () => {
|
||||
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' }, null, 2) + '\n');
|
||||
fs.writeFileSync(path.join(dir, 'package-lock.json'), lock('1.0.0.0'));
|
||||
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, packageLock: true });
|
||||
const l = JSON.parse(fs.readFileSync(path.join(dir, 'package-lock.json'), 'utf-8'));
|
||||
expect(l.version).toBe('1.1.0.0');
|
||||
expect(l.packages[''].version).toBe('1.1.0.0');
|
||||
expect(l.packages['node_modules/a'].version).toBe('9.9.9'); // untouched
|
||||
});
|
||||
|
||||
test('repair heals a stale lockfile alongside package.json', () => {
|
||||
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');
|
||||
fs.writeFileSync(path.join(dir, 'package-lock.json'), lock('1.9.0.0'));
|
||||
execFileSync('bun', [BIN, 'repair'], { cwd: dir });
|
||||
const l = JSON.parse(fs.readFileSync(path.join(dir, 'package-lock.json'), 'utf-8'));
|
||||
expect(l.version).toBe('2.0.0.0');
|
||||
expect(l.packages[''].version).toBe('2.0.0.0');
|
||||
});
|
||||
|
||||
test('lockfileVersion 1 (no packages map) syncs top-level only, no crash', () => {
|
||||
fs.writeFileSync(path.join(dir, 'VERSION'), '3.0.0.0\n');
|
||||
fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'x', version: '2.9.0.0' }, null, 2) + '\n');
|
||||
fs.writeFileSync(path.join(dir, 'package-lock.json'), JSON.stringify({ name: 'x', version: '2.9.0.0', lockfileVersion: 1 }, null, 2) + '\n');
|
||||
execFileSync('bun', [BIN, 'repair'], { cwd: dir });
|
||||
const l = JSON.parse(fs.readFileSync(path.join(dir, 'package-lock.json'), 'utf-8'));
|
||||
expect(l.version).toBe('3.0.0.0');
|
||||
expect(l.packages).toBeUndefined();
|
||||
});
|
||||
|
||||
test('npm-shrinkwrap.json is synced too when present (never created)', () => {
|
||||
const d2 = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-shrink-'));
|
||||
fs.writeFileSync(path.join(d2, 'VERSION'), '1.0.0.0\n');
|
||||
fs.writeFileSync(path.join(d2, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0.0' }, null, 2) + '\n');
|
||||
fs.writeFileSync(path.join(d2, 'npm-shrinkwrap.json'), lock('1.0.0.0').replace('package-lock', 'npm-shrinkwrap'));
|
||||
const out = execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: d2 }).toString();
|
||||
expect(JSON.parse(out).packageLock).toBe(true);
|
||||
const l = JSON.parse(fs.readFileSync(path.join(d2, 'npm-shrinkwrap.json'), 'utf-8'));
|
||||
expect(l.version).toBe('1.1.0.0');
|
||||
expect(l.packages[''].version).toBe('1.1.0.0');
|
||||
// No package-lock.json invented alongside it.
|
||||
expect(fs.existsSync(path.join(d2, 'package-lock.json'))).toBe(false);
|
||||
fs.rmSync(d2, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('malformed lockfile fails the write with exit 3 (half-write is loud, not silent)', () => {
|
||||
const d3 = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-badlock-'));
|
||||
fs.writeFileSync(path.join(d3, 'VERSION'), '1.0.0.0\n');
|
||||
fs.writeFileSync(path.join(d3, 'package.json'), JSON.stringify({ name: 'x', version: '1.0.0.0' }, null, 2) + '\n');
|
||||
fs.writeFileSync(path.join(d3, 'package-lock.json'), '{ not json');
|
||||
let code = 0;
|
||||
try { execFileSync('bun', [BIN, 'write', '--version', '1.1.0.0'], { cwd: d3, stdio: 'pipe' }); }
|
||||
catch (e: any) { code = e.status; }
|
||||
expect(code).toBe(3);
|
||||
// VERSION was written before the failure — exactly the half-write the
|
||||
// exit-3 contract exists to surface.
|
||||
expect(fs.readFileSync(path.join(d3, 'VERSION'), 'utf-8').trim()).toBe('1.1.0.0');
|
||||
fs.rmSync(d3, { 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 */ } });
|
||||
|
||||
Reference in New Issue
Block a user