fix: pre-landing review fixes (the answer gate names every flag that decides the write; Version Gate and validate understand bump never)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

bin/gstack-ship-policy

@@ -18,7 +18,7 @@ import * as path from 'node:path';
 import { EXIT, exitTableHelp } from '../lib/headless-artifacts';
 import { resultLine } from '../lib/result-codes';
 import {
-  POLICY_KEYS, POLICY_KEY_INTRODUCED, SHIP_POLICY_PATH, examplePolicyJson, explainPolicy, isUnavailable, loadPolicyFromBase, loadPolicyFromHead,
+  BUMP_NEVER_KEY_VERSION, POLICY_KEYS, POLICY_KEY_INTRODUCED, SHIP_POLICY_PATH, examplePolicyJson, explainPolicy, isUnavailable, loadPolicyFromBase, loadPolicyFromHead,
   minimumReader, policyUnavailableLine, renderEffective, resolveBaseBranch, type LoadedPolicy,
 } from '../lib/ship-policy';
 
@@ -87,8 +87,8 @@ function main(argv: string[]): number {
     const loaded = load(root, rest, cmd === 'validate' ? 'head' : 'base');
     if (typeof loaded === 'number') return loaded;
     const set = POLICY_KEYS.filter(k => loaded.values[k] && loaded.values[k] !== 'default');
-    const introduced = Object.fromEntries(set.map(k => [k, POLICY_KEY_INTRODUCED[k]]));
-    const minReader = set.length ? minimumReader(introduced) : null;
+    const introduced = Object.fromEntries(set.map(k => [k, k === 'bump' && loaded.policy.bump === 'never' ? BUMP_NEVER_KEY_VERSION : POLICY_KEY_INTRODUCED[k]]));
+    const minReader = set.length ? minimumReader(Object.fromEntries(set.map(k => [k, loaded.policy[k]]))) : null;
     if (json) {
       process.stdout.write(JSON.stringify({ valid: loaded.errors.length === 0, present: loaded.present, source: loaded.source, policy: loaded.policy, values: loaded.values, introduced, min_reader: minReader, errors: loaded.errors }) + '\n');
       return loaded.errors.length ? EXIT.fail : EXIT.ok;

docs/ship-policy.md

@@ -39,7 +39,7 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with
 
 | Key | Values | Default | Meaning |
 |---|---|---|---|
-| `bump` | `patch` \| `ask` \| `never` | `ask` | `patch` skips the MINOR/MAJOR prompt in `/ship` Step 12 and `gstack-restamp --next` bumps PATCH; `ask` keeps the prompt, and in a session no person answers `--bump` stops with `BUMP_ANSWER_REQUIRED` until `--answer "<the owner's words>"`; `never` is for a repo whose unit PRs never change VERSION: Step 12 keeps it with no question, refuses `--bump`, and `gstack-restamp --next` needs an explicit `--bump` or `--version` (the release process bumps). `never` needs `min_gstack_version` at its introducing release and cannot be combined with `queue_mode: stamp-at-merge`, `train` or `changelog_fragments`, which stamp a version at every merge. |
+| `bump` | `patch` \| `ask` \| `never` | `ask` | `patch` skips the MINOR/MAJOR prompt in `/ship` Step 12 and `gstack-restamp --next` bumps PATCH; `ask` keeps the prompt, and in a session no person answers any version Step 12 would write (`--bump` on a fresh branch, `--version <v>` on a bumped one) stops with `BUMP_ANSWER_REQUIRED` until `--answer "<the owner's words>"`; `never` is for a repo whose unit PRs never change VERSION: Step 12 keeps it with no question, refuses `--bump`, and `gstack-restamp --next` needs an explicit `--bump` or `--version` (the release process bumps). `never` needs `min_gstack_version` at its introducing release and cannot be combined with `queue_mode: stamp-at-merge`, `train` or `changelog_fragments`, which stamp a version at every merge. |
 | `queue_mode` | `claim-at-open` \| `stamp-at-merge` | `claim-at-open` | See [stamp-at-merge](#stamp-at-merge). `claim-at-open` is today's queue (`gstack-next-version` claims a slot when the PR opens). |
 | `release_tool` | command or `null` | `null` | The repo's own release command (`bun run release:restamp`). Runs in an isolated staging worktree under the [release-tool contract](#release-tool-contract). |
 | `release_outputs` | globs | `[]` | Paths the release tool may write beyond the version files, lockfiles, CHANGELOG and `stamp_paths`. Anything else is `RESTAMP_OUTPUT_OUTSIDE_ALLOWED`. |
@@ -867,7 +867,7 @@ which carry the workflow consequences the skill prose would otherwise spell out:
 | `ship: gate-ahead: …` | with `.gstack/tmp/gate-ahead.json` present, Step 16 reuses only the lanes `gstack-tree-receipt … --json > .gstack/tmp/tree-receipt.json` marks reusable; stamp-sensitive and not-eligible lanes run live; `--force-gate <reason>` is the coordinator's call |
 | `ship: bump patch: …` | Step 12 never asks for MINOR or MAJOR; PATCH or MICRO by size |
 | `ship: bump never: …` | Step 12 prints `VERSION: <v> kept (policy: bump never; no write)`, asks nothing and refuses `--bump`; a branch that moved VERSION stops until it is put back |
-| `ship: bump ask: …` | Step 12 asks for MINOR and MAJOR; on FRESH, `--keep-current` (or `--version <current>`) keeps VERSION, and in a session no person answers any `--bump` waits for `--answer` |
+| `ship: bump ask: …` | Step 12 asks for MINOR and MAJOR; on FRESH, `--keep-current` (or `--version <current>`) keeps VERSION, and in a session no person answers any version Step 12 would write waits for `--answer` |
 | `ship: preregistration: …` | `gstack-evidence ancestor --from-policy --base <base>` must print only `ANCESTOR: ok` before push |
 | `ship: release_tool: …` | the command runs only inside `gstack-restamp` (staging worktree, outputs confined to `release_outputs`) |
 | `ship: review_scope …` | Step 9.4 item 0 runs `gstack-review-scope classify` on each pass's findings; see [review_scope](#review-scope) |

lib/ship-release.ts

@@ -90,7 +90,8 @@ export function version(argv: readonly string[], cwd: string): Report {
   }
   const atBase = !!explicit && cmpVersion(parseVersion(explicit)!, parseVersion(j.baseVersion)!) === 0;
   const keep = fresh && (has('--keep-current') || atBase);
-  if (keep && asked) return r.stop(`--bump ${asked} moves VERSION and ${has('--keep-current') ? '--keep-current' : `--version ${explicit}`} keeps it: pass one`);
+  const mover = asked ? `--bump ${asked}` : explicit && !atBase ? `--version ${explicit}` : '';
+  if (keep && mover) return r.stop(`${mover} moves VERSION and ${has('--keep-current') ? '--keep-current' : `--version ${explicit}`} keeps it: pass one`);
   if (keep) return r.add(`VERSION: ${j.baseVersion} kept (FRESH, ${has('--keep-current') ? '--keep-current' : `--version ${explicit}`}; no write)`).next(KEPT_NEXT);
   const level = fresh ? asked ?? (policy?.bump === 'patch' ? 'patch' : undefined) : asked ?? derivedLevel(j.baseVersion, j.currentVersion) ?? undefined;
   if (!level) {
@@ -98,9 +99,11 @@ export function version(argv: readonly string[], cwd: string): Report {
     return r.stop(`BUMP_LEVEL needed (FRESH, ${stat || 'no diff'}): MICRO under 50 lines, PATCH 50+ with no feature signal, MINOR/MAJOR ask first (Step 12.2); rerun with --bump <level>, or with --keep-current to leave VERSION at ${j.baseVersion}`);
   }
   const answer = flag('--answer');
-  if ((fresh ? asked : explicit) && policy?.bump !== 'patch' && !answer) {
+  if (explicit && cmpVersion(parseVersion(explicit)!, parseVersion(j.baseVersion)!) <= 0) return r.stop(`${explicit} is not above the base version ${j.baseVersion}`);
+  const rewrites = fresh ? !!asked : !!explicit && cmpVersion(parseVersion(explicit)!, parseVersion(j.currentVersion)!) !== 0;
+  if (rewrites && policy?.bump !== 'patch' && !answer) {
     const kind = bin(cwd, 'gstack-session-kind', [], 30_000).stdout.trim() || 'unknown';
-    const pick = fresh ? `--bump ${asked}` : `--version ${explicit}`;
+    const pick = [asked && `--bump ${asked}`, explicit && `--version ${explicit}`].filter(Boolean).join(' ');
     if (kind !== 'interactive') return r.stop(`BUMP_ANSWER_REQUIRED (${kind} session, bump ask): a version Step 12 writes is the owner's approval, and a brief's "take the recommended option" is not an answer. Ask: ${pick} (recommended), keep ${fresh ? j.baseVersion : j.currentVersion} (--keep-current), or stop; then rerun with ${pick} --answer "<their words>". A repo whose unit PRs never bump sets bump never; one that always takes PATCH sets bump patch`);
   }
   let q = queue(cwd, base, level, j.baseVersion);

lib/version-gate-train.ts

@@ -17,7 +17,7 @@ import { matchesAny } from './ship-policy';
 
 export type TrainHead =
   | { kind: 'off' }
-  | { kind: 'unstamped'; fragments?: number; entry?: 'fragment' | 'unreleased' | 'none-bot' }
+  | { kind: 'unstamped'; fragments?: number; entry?: 'fragment' | 'unreleased' | 'none-bot' | 'kept' }
   | { kind: 'stamped'; expected: string; fragments?: boolean }
   | { kind: 'invalid'; reason: string };
 
@@ -96,11 +96,15 @@ export function classifyTrainHead(o: { repoRoot: string; baseRef: string; head:
   let policy: any = null;
   try { policy = JSON.parse(show(base, '.gstack/ship-policy.json') ?? 'null'); } catch { policy = null; }
   const fragDir: string | null = typeof policy?.changelog_fragments?.dir === 'string' ? policy.changelog_fragments.dir : null;
-  if (!policy || ((typeof policy.train !== 'object' || policy.train === null) && !fragDir)) return { kind: 'off' };
+  if (!policy || ((typeof policy.train !== 'object' || policy.train === null) && !fragDir && policy.bump !== 'never')) return { kind: 'off' };
   const mergeBase = git(o.repoRoot, ['merge-base', base, o.head])?.trim();
   if (!mergeBase) return { kind: 'invalid', reason: `no merge base between ${base} and ${o.head}` };
 
   const npmVersion = (rev: string) => { try { return JSON.parse(show(rev, 'package.json') ?? '{}').version ?? null; } catch { return null; } };
+  if (policy.bump === 'never') {
+    return show(mergeBase, 'VERSION') === show(o.head, 'VERSION') && npmVersion(mergeBase) === npmVersion(o.head) ? { kind: 'unstamped', entry: 'kept' }
+      : { kind: 'invalid', reason: `the base's ship policy says bump never: leave VERSION and package.json's version at the merge base's values` };
+  }
   const changed = (git(o.repoRoot, ['diff', '--name-only', mergeBase, o.head]) ?? '').split('\n').filter(Boolean);
   const releaseOutputs: string[] = Array.isArray(policy.release_outputs) ? policy.release_outputs : [];
   // touched: what marks the head as stamped (the labels only feed the length check below)

scripts/compare-pr-version.ts

@@ -41,6 +41,10 @@ const prVersionEnv = (process.env.PR_VERSION ?? "").trim();
 if (process.env.BASE_REF && process.env.HEAD_SHA) {
   const train = classifyTrainHead({ repoRoot: process.cwd(), baseRef: process.env.BASE_REF, head: process.env.HEAD_SHA, author: process.env.PR_AUTHOR });
   if (train.kind === "unstamped") {
+    if (train.entry === "kept") {
+      console.log(`✓ ${tag} keeps its merge base's VERSION and package.json version, as the base's ship policy (bump never) requires.`);
+      process.exit(0);
+    }
     const entry = train.entry === "fragment" ? `its entry is ${train.fragments} CHANGELOG fragment(s)` : train.entry === "none-bot" ? "it is a bot PR with no entry (the merge step writes one from its title)" : "the CHANGELOG change is confined to [Unreleased]";
     console.log(`✓ ${tag} is unstamped under the base's ship policy: VERSION and package.json match its merge base, any changed release output is what the release tool generates at the merge base's VERSION, and ${entry}. The merge step stamps it.`);
     process.exit(0);

test/gstack-ship-policy.test.ts

@@ -222,6 +222,15 @@ describe('gstack-ship-policy (bin)', () => {
     expect(runBin(repo, 'gstack-ship-policy', ['show-effective', '--policy-from', 'head']).out).toMatch(/^policy: head \(unreviewed\)/);
   });
 
+  // Value: protects=validate reports bump never's own floor as the policy's minimum reader; fails_when=the CLI prints the bump key's 1.91.73.0 and a crew upgrades to a gstack that refuses the policy;
+  //   why_new=GST-380 outside-voice review (the CLI passed introduction versions to minimumReader); seam=bin/gstack-ship-policy; no_claim=-
+  test('validate --json names bump never\'s floor as the minimum reader', () => {
+    const repo = fixture();
+    fs.writeFileSync(path.join(repo.work, '.gstack', 'ship-policy.json'), JSON.stringify({ bump: 'never', min_gstack_version: BUMP_NEVER_KEY_VERSION }) + '\n');
+    const j = JSON.parse(runBin(repo, 'gstack-ship-policy', ['validate', '--json']).out);
+    expect([j.valid, j.introduced.bump, j.min_reader]).toEqual([true, BUMP_NEVER_KEY_VERSION, BUMP_NEVER_KEY_VERSION]);
+  });
+
   test('POLICY_SOURCE_UNAVAILABLE (exit 3) when origin/<base> does not resolve; no fallback to the working tree', () => {
     const repo = fixture();
     const r = runBin(repo, 'gstack-ship-policy', ['show-effective', '--base', 'release']);

test/ship-release.test.ts

@@ -102,8 +102,13 @@ describe('version (Step 12): keeping VERSION and waiting for the bump answer (GS
     untouched(f);
     const ok = f.run(RELEASE, ['version', '--base', 'main', '--bump', 'micro', '--answer', 'micro is fine'], UNATTENDED);
     expect([ok.status, read(f, 'VERSION')]).toEqual([0, '1.0.0.1\n']);
-    const rebump = f.run(RELEASE, ['version', '--base', 'main', '--bump', 'major', '--version', '2.0.0.0'], UNATTENDED);
-    expect([rebump.status, /^STOP: BUMP_ANSWER_REQUIRED/m.test(rebump.stdout), read(f, 'VERSION')]).toEqual([3, true, '1.0.0.1\n']);
+    const rebump = f.run(RELEASE, ['version', '--base', 'main', '--version', '2.0.0.0'], UNATTENDED);
+    expect([rebump.status, rebump.stdout.includes('BUMP_ANSWER_REQUIRED'), rebump.stdout.includes('Ask: --version 2.0.0.0 (recommended), keep 1.0.0.1'), read(f, 'VERSION')]).toEqual([3, true, true, '1.0.0.1\n']);
+    expect(f.run(RELEASE, ['version', '--base', 'main', '--version', '1.0.0.0'], UNATTENDED).stdout).toContain('STOP: 1.0.0.0 is not above the base version 1.0.0.0');
+    const rewritten = f.run(RELEASE, ['version', '--base', 'main', '--version', '2.0.0.0', '--answer', 'go to 2.0.0.0'], UNATTENDED);
+    expect([rewritten.status, read(f, 'VERSION')]).toEqual([0, '2.0.0.0\n']);
+    const both = fx().run(RELEASE, ['version', '--base', 'main', '--bump', 'micro', '--version', '1.0.0.5'], UNATTENDED);
+    expect([both.status, both.stdout.includes('Ask: --bump micro --version 1.0.0.5 (recommended)')]).toEqual([3, true]);
     const patch = fx({ '.gstack/ship-policy.json': '{"bump":"patch"}\n' });
     expect([patch.run(RELEASE, ['version', '--base', 'main'], UNATTENDED).status, read(patch, 'VERSION')]).toEqual([0, '1.0.1.0\n']);
   });
@@ -130,12 +135,14 @@ describe('version (Step 12): keeping VERSION and waiting for the bump answer (GS
 
   // Value: protects=a --bump beside --keep-current or --version <current> is a contradiction the bin names instead of picking one; fails_when=it silently keeps or silently bumps;
   //   why_new=GST-380 review F8 (flag precedence); seam=none; no_claim=-
-  test('--bump with --keep-current or --version <current> stops and writes nothing', () => {
+  test('--bump or --version <other> with --keep-current or --version <current> stops and writes nothing', () => {
     const f = fx();
     for (const args of [['--keep-current'], ['--version', '1.0.0.0']]) {
       const r = f.run(RELEASE, ['version', '--base', 'main', '--bump', 'micro', ...args]);
       expect([r.status, r.stdout.includes('STOP: --bump micro moves VERSION and')]).toEqual([3, true]);
     }
+    const keepAndMove = f.run(RELEASE, ['version', '--base', 'main', '--keep-current', '--version', '1.0.0.5']);
+    expect([keepAndMove.status, keepAndMove.stdout.includes('STOP: --version 1.0.0.5 moves VERSION and --keep-current keeps it')]).toEqual([3, true]);
     untouched(f);
   });
 });

test/version-gate-fragments.test.ts

@@ -7,6 +7,7 @@
 import { afterEach, describe, expect, test } from 'bun:test';
 import * as fs from 'node:fs';
 import * as path from 'node:path';
+import { BUMP_NEVER_KEY_VERSION } from '../lib/ship-policy';
 import { BASE_POLICY, cleanup, makeFixtureRepo, ROOT, type FixtureRepo } from './helpers/restamp-fixture';
 
 const SCRIPT = path.join(ROOT, 'scripts', 'compare-pr-version.ts');
@@ -24,6 +25,22 @@ function run(r: FixtureRepo, head: string, extra: Record<string, string> = {}, c
   return { code: out.exitCode, out: out.stdout.toString() + out.stderr.toString() };
 }
 
+describe('version gate: bump never', () => {
+  // Value: protects=under bump never the Version Gate passes a head that keeps VERSION and fails one that moves it; fails_when=the gate rejects every kept-version PR as "VERSION not bumped", or lets a unit PR bump;
+  //   why_new=GST-380 added bump never (outside-voice review); seam=classifyTrainHead, scripts/compare-pr-version.ts; no_claim=the claim queue
+  test('a kept VERSION passes and a moved VERSION fails', () => {
+    const r = makeFixtureRepo({ policy: { ...BASE_POLICY, queue_mode: 'claim-at-open', release_tool: null, release_outputs: [], bump: 'never', min_gstack_version: BUMP_NEVER_KEY_VERSION }, prefix: 'version-gate-never-' });
+    repos.push(r);
+    r.git('checkout', '-q', '-b', 'k');
+    write(r, 'k.txt', 'k\n');
+    const kept = run(r, commit(r, 'k'));
+    expect([kept.code, kept.out.includes('as the base\'s ship policy (bump never) requires')]).toEqual([0, true]);
+    write(r, 'VERSION', '1.2.4.0\n');
+    const moved = run(r, commit(r, 'bump'));
+    expect([moved.code, moved.out.includes('the base\'s ship policy says bump never')]).toEqual([1, true]);
+  });
+});
+
 describe('version gate: changelog fragments', () => {
   test('a fragment head passes, a dependency or script edit beside it too', () => {
     const r = fixture();