From f9f0e84add06cfc058b9e1dafe1c37f81dfc4da0 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 17 Aug 2026 10:09:48 -0700 Subject: [PATCH] fix(version-bump): missing or empty VERSION no longer repairs a fabricated 0.0.0.0 into package.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit repair now fails with exit 2 when the VERSION file is absent or empty instead of folding to DEFAULT ("0.0.0.0") — which passed VERSION_RE and regressed package.json below where it started. classify gains an additive versionFileExists field so /ship can tell a real 0.0.0.0 from a fabricated one. Re-derived from PR #2612 under the generated-file screening rule. Fixes #2600 (repair half; the path-configurability half landed in v1.67 via #2531). Contributed by @Lockyer228 Co-Authored-By: Claude Fable 5 --- bin/gstack-version-bump | 28 +++++++ test/gstack-version-bump.test.ts | 128 +++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) diff --git a/bin/gstack-version-bump b/bin/gstack-version-bump index cf756800e..6717272bc 100755 --- a/bin/gstack-version-bump +++ b/bin/gstack-version-bump @@ -313,6 +313,11 @@ function cmdClassify(args: string[], cwd: string): void { // the version itself. const expectedPkg = jsonSource ? current : npmVersion(current); const state = classifyState(current, baseV, pkg.exists, pkg.version, expectedPkg); + // Surface version-file absence so callers (and /ship) can tell "version is + // genuinely 0.0.0.0" from "we made up 0.0.0.0 because the file is missing" + // (#2600). Without this, the DRIFT_STALE_PKG dispatch on a missing VERSION + // would feed repair a fabricated version that passes the shape check. + const versionFileExists = existsSync(versionPath); process.stdout.write( JSON.stringify({ state, @@ -322,6 +327,7 @@ function cmdClassify(args: string[], cwd: string): void { pkgExists: pkg.exists, pkgPath: pkg.exists ? relative(cwd, pkgPath) : null, expectedPkgVersion: pkg.exists ? expectedPkg : null, + versionFileExists, }) + "\n", ); // DRIFT_UNEXPECTED is a real, decidable state — the caller stops on it, but the @@ -442,7 +448,29 @@ function cmdRepair(args: string[], cwd: string): void { ); return; } + // Guard: if the VERSION file does not exist, readVersionFile folds that into + // DEFAULT ("0.0.0.0") — a structurally valid but fabricated version. The + // shape check below (VERSION_RE) would pass it, and we would write 0.0.0 + // into package.json, regressing it below where it started (#2600). + if (!existsSync(versionPath)) { + fail( + `VERSION file not found at ${versionRel}. ` + + "Cannot repair package.json without a real version to sync. " + + "Pass --version-path or set .gstack/version-path if the file lives elsewhere.", + 2, + ); + } const current = readVersionFile(versionPath, versionRel); + // Guard against readVersionFile folding "file exists but is empty / unparsable" + // into DEFAULT ("0.0.0.0") — same data-corruption pathway as file-missing (#2600). + // A fabricated version must never propagate into package.json. + if (current === DEFAULT) { + fail( + `VERSION file at ${versionRel} is empty or contains no parsable version. ` + + "Cannot repair package.json with a fabricated version.", + 2, + ); + } if (!VERSION_RE.test(current)) { fail( `VERSION file contents (${current}) do not match MAJOR.MINOR.PATCH[.MICRO]. ` + diff --git a/test/gstack-version-bump.test.ts b/test/gstack-version-bump.test.ts index f7ea38828..a35afd3c2 100644 --- a/test/gstack-version-bump.test.ts +++ b/test/gstack-version-bump.test.ts @@ -577,3 +577,131 @@ describe('path containment: pins and flags cannot escape the repo', () => { expect(JSON.parse(fs.readFileSync(path.join(dir, 'frontend', 'package.json'), 'utf-8')).version).toBe('1.1.0'); }); }); + +describe('#2600: repair must not write fabricated 0.0.0.0 when VERSION is missing', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-2600-')); + afterAll(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* noop */ } }); + + test('repair fails with exit 2 when VERSION file does not exist', () => { + // Set up: package.json exists with version 0.1.0.0, but no VERSION file + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'x', version: '0.1.0.0' }, null, 2) + '\n'); + // VERSION file deliberately absent + expect(fs.existsSync(path.join(dir, 'VERSION'))).toBe(false); + + let code = 0; + let stderr = ''; + try { + execFileSync('bun', [BIN, 'repair'], { cwd: dir, stdio: 'pipe' }); + } catch (e: any) { + code = e.status; + stderr = (e.stderr || '').toString(); + } + + // Should fail, not succeed + expect(code).toBe(2); + expect(stderr).toContain('VERSION file not found'); + // package.json must NOT be modified + expect(JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf-8')).version).toBe('0.1.0.0'); + }); + + test('repair works normally when VERSION file exists', () => { + // Set up: both VERSION and package.json exist, with drift + 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' }, null, 2) + '\n'); + + const out = execFileSync('bun', [BIN, 'repair'], { cwd: dir }).toString(); + const result = JSON.parse(out); + + expect(result.repaired).toBe('2.0.0.0'); + expect(result.packageJsonVersion).toBe('2.0.0'); + expect(JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf-8')).version).toBe('2.0.0'); + }); + + test('repair refuses to propagate a fabricated version when VERSION file is empty (#2600)', () => { + // VERSION exists but is empty — readVersionFile folds this into DEFAULT ("0.0.0.0"). + // Without the `current === DEFAULT` guard, this would write 0.0.0 into package.json. + fs.writeFileSync(path.join(dir, 'VERSION'), ''); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'x', version: '0.5.0' }, null, 2) + '\n'); + + let code = 0; + let stderr = ''; + try { + execFileSync('bun', [BIN, 'repair'], { cwd: dir, stdio: 'pipe' }); + } catch (e: any) { + code = e.status; + stderr = (e.stderr || '').toString(); + } + + expect(code).toBe(2); + expect(stderr).toContain('empty or contains no parsable version'); + // package.json must NOT be modified + expect(JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf-8')).version).toBe('0.5.0'); + }); + + test('repair reproduces the exact issue scenario: VERSION in root, package.json in app/ (#2600)', () => { + // The exact layout from the issue: VERSION at repo root, package.json in app/ + // Running repair from app/ cwd with no VERSION there used to write 0.0.0.0 into app/package.json. + const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-2600-exact-')); + afterAll(() => { try { fs.rmSync(rootDir, { recursive: true, force: true }); } catch { /* noop */ } }); + + fs.mkdirSync(path.join(rootDir, 'app'), { recursive: true }); + fs.writeFileSync(path.join(rootDir, 'VERSION'), '0.2.0.0\n'); + fs.writeFileSync(path.join(rootDir, 'app', 'package.json'), JSON.stringify({ name: 'x', version: '0.1.0.0' }, null, 2) + '\n'); + + // Run from app/ — no VERSION in cwd, readVersionFile would fold to 0.0.0.0 + let code = 0; + let stderr = ''; + try { + execFileSync('bun', [BIN, 'repair'], { cwd: path.join(rootDir, 'app'), stdio: 'pipe' }); + } catch (e: any) { + code = e.status; + stderr = (e.stderr || '').toString(); + } + + expect(code).toBe(2); + expect(stderr).toContain('VERSION file not found'); + // app/package.json must NOT be modified + expect(JSON.parse(fs.readFileSync(path.join(rootDir, 'app', 'package.json'), 'utf-8')).version).toBe('0.1.0.0'); + }); +}); + +describe('#2600: classify must surface versionFileExists=false when VERSION is missing', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vbump-2600-classify-')); + afterAll(() => { try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* noop */ } }); + + // Set up a minimal git repo so classify can resolve base + const git = (...a: string[]) => execFileSync('git', a, { cwd: dir, stdio: 'pipe' }); + git('init', '-q', '-b', 'main'); + git('config', 'user.email', 't@t'); git('config', 'user.name', 't'); + // Commit with no VERSION file + fs.writeFileSync(path.join(dir, 'README.md'), 'test\n'); + git('add', '-A'); git('commit', '-q', '-m', 'base'); + const head = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: dir }).toString().trim(); + fs.mkdirSync(path.join(dir, '.git', 'refs', 'remotes', 'origin'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.git', 'refs', 'remotes', 'origin', 'main'), head + '\n'); + + test('classify reports versionFileExists=false when VERSION is absent', () => { + // No package.json: pkgExists=false, pkgAgrees=true, current===base → FRESH. + // (A package.json with a non-zero version would cause DRIFT_UNEXPECTED.) + + const out = execFileSync('bun', [BIN, 'classify', '--base', 'main'], { cwd: dir }).toString(); + const result = JSON.parse(out); + + expect(result.versionFileExists).toBe(false); + expect(result.currentVersion).toBe('0.0.0.0'); // fabricated default + expect(result.state).toBe('FRESH'); // base also reads 0.0.0.0, no pkg drift + }); + + test('classify reports versionFileExists=true when VERSION is present', () => { + // Now create VERSION AND sync package.json so pkgAgrees=true → ALREADY_BUMPED. + fs.writeFileSync(path.join(dir, 'VERSION'), '0.2.0.0\n'); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'x', version: '0.2.0.0' }, null, 2) + '\n'); + + const out = execFileSync('bun', [BIN, 'classify', '--base', 'main'], { cwd: dir }).toString(); + const result = JSON.parse(out); + + expect(result.versionFileExists).toBe(true); + expect(result.currentVersion).toBe('0.2.0.0'); + expect(result.state).toBe('ALREADY_BUMPED'); // base is 0.0.0.0, current is 0.2.0.0, pkg in sync + }); +});