mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-06 05:35:46 +02:00
fix(ship): harden Step 12 against whitespace + invalid REPAIR_VERSION
Claude adversarial subagent surfaced three correctness risks in the Step 12 state machine: - CURRENT_VERSION and BASE_VERSION were not stripped of CR/whitespace on read. A CRLF VERSION file would mismatch the clean package.json version, falsely classify as DRIFT_STALE_PKG, then propagate the carriage return into package.json via the repair path. - REPAIR_VERSION was unvalidated. The bump path validates NEW_VERSION against the 4-digit semver pattern, but the drift-repair path wrote whatever cat VERSION returned directly into package.json. A manually-corrupted VERSION file would silently poison the repair. - Empty-string CURRENT_VERSION (0-byte VERSION, directory-at-VERSION) fell through to "not equal to base" and misclassified as ALREADY_BUMPED. Template fix strips \r/newlines/whitespace on every VERSION read, guards against empty-string results, and applies the same semver regex gate in the repair path that already protects the bump path. Adds two regression tests (trailing-CR idempotency + invalid-semver repair rejection). Total Step 12 coverage: 14 tests, 14/14 pass. Opens two follow-up TODOs flagged but not fixed in this branch: test/template drift risk (the tests still reimplement template bash) and BASE_VERSION silent fallback on git-show failure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -37,7 +37,8 @@ const idempotency = (base: string): { stdout: string; code: number } => {
|
||||
const script = `
|
||||
cd "${dir}" || exit 2
|
||||
BASE_VERSION="${base}"
|
||||
CURRENT_VERSION=$(cat VERSION 2>/dev/null || echo "0.0.0.0")
|
||||
CURRENT_VERSION=$(cat VERSION 2>/dev/null | tr -d '\\r\\n[:space:]' || echo "0.0.0.0")
|
||||
[ -z "$CURRENT_VERSION" ] && CURRENT_VERSION="0.0.0.0"
|
||||
PKG_VERSION=""
|
||||
PKG_EXISTS=0
|
||||
if [ -f package.json ]; then
|
||||
@@ -97,7 +98,10 @@ fi`;
|
||||
const syncRepair = (): { code: number } => {
|
||||
const script = `
|
||||
cd "${dir}" || exit 2
|
||||
REPAIR_VERSION=$(cat VERSION)
|
||||
REPAIR_VERSION=$(cat VERSION | tr -d '\\r\\n[:space:]')
|
||||
if ! printf '%s' "$REPAIR_VERSION" | grep -qE '^[0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+$'; then
|
||||
echo "invalid repair semver" >&2; exit 1
|
||||
fi
|
||||
node -e 'const fs=require("fs"),p=require("./package.json");p.version=process.argv[1];fs.writeFileSync("package.json",JSON.stringify(p,null,2)+"\\n")' "$REPAIR_VERSION"`;
|
||||
try {
|
||||
execSync(script, { shell: "/bin/bash", stdio: "pipe" });
|
||||
@@ -183,6 +187,28 @@ test("bump: no package.json is silent", () => {
|
||||
expect(existsSync(join(dir, "package.json"))).toBe(false);
|
||||
});
|
||||
|
||||
// --- Adversarial review regressions: trailing whitespace + invalid REPAIR_VERSION ---
|
||||
|
||||
test("trailing CR in VERSION does not cause false DRIFT_STALE_PKG", () => {
|
||||
// Before the tr-strip fix, VERSION="0.1.0.0\r" read via cat would mismatch
|
||||
// pkg.version="0.1.0.0" and classify as DRIFT_STALE_PKG, then repair would
|
||||
// write garbage \r into package.json. Now CURRENT_VERSION is stripped.
|
||||
writeFileSync(join(dir, "VERSION"), "0.1.0.0\r\n");
|
||||
writeFileSync(join(dir, "package.json"), pkgJson("0.1.0.0"));
|
||||
expect(idempotency("0.0.0.0")).toEqual({ stdout: "STATE: ALREADY_BUMPED", code: 0 });
|
||||
});
|
||||
|
||||
test("DRIFT REPAIR rejects invalid VERSION semver instead of propagating", () => {
|
||||
// If VERSION is corrupted/manually-edited to something non-semver, the
|
||||
// repair path must refuse rather than writing junk into package.json.
|
||||
writeFileSync(join(dir, "VERSION"), "not-a-semver\n");
|
||||
writeFileSync(join(dir, "package.json"), pkgJson("0.0.0.0"));
|
||||
const r = syncRepair();
|
||||
expect(r.code).toBe(1);
|
||||
// package.json must NOT have been overwritten with the garbage.
|
||||
expect(pkgVersion()).toBe("0.0.0.0");
|
||||
});
|
||||
|
||||
// --- THE critical regression test: drift-repair does NOT double-bump ---
|
||||
|
||||
test("DRIFT REPAIR: sync path syncs pkg to VERSION without re-bumping", () => {
|
||||
|
||||
Reference in New Issue
Block a user