fix: pre-landing review fixes (GST-980 carry reads the stamp file line by line at the branch's positions, and keeps its mode)

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

bin/gstack-ship-receipt

@@ -37,7 +37,7 @@ import * as path from 'node:path';
 import { EXIT, exitTableHelp, type ShipReceipt } from '../lib/headless-artifacts';
 import { resultLine, resultLineWithDocs } from '../lib/result-codes';
 import { versionAtRef } from '../lib/restamp';
-import { branchSides, type BranchSide } from '../lib/stamp-only';
+import { branchSides, stampOnlyLine, type BranchSide } from '../lib/stamp-only';
 import { runScriptSync } from '../lib/script-invocation';
 import { readPregateFile, verifyPregate } from '../lib/pregate/index';
 import { isUnavailable, loadPolicyFromBase, matchesAny, policyUnavailableLine, resolveBaseBranch, runningGstackVersion, stampFilesInTree, validatePolicy, type ShipPolicy } from '../lib/ship-policy';
@@ -496,7 +496,7 @@ function productRules(repoRoot: string, base: string, outputs: readonly string[]
  * change, such as a CHANGELOG fragment next to the base's changelog.d/README.md
  * (GST-343, #3606), into the product patch-id at one head only.
  */
-function productSpec(repoRoot: string, rules: ProductRules, from: string, to: string, branch?: { touched: ReadonlySet<string>; sides: ReadonlyArray<readonly [string, string]>; own: (rel: string) => BranchSide | undefined }): { changed: Array<{ path: string; class: ReturnType<typeof classifyCarryPath> }>; product: string[]; tests: string[]; demoted: string[] } | undefined {
+function productSpec(repoRoot: string, rules: ProductRules, from: string, to: string, branch?: { touched: ReadonlySet<string>; sides: ReadonlyArray<readonly [string, string]>; own: (rel: string) => BranchSide | undefined }): { changed: Array<{ path: string; class: ReturnType<typeof classifyCarryPath> }>; product: string[]; tests: string[]; demoted: string[]; ownRows: string[] } | undefined {
   const changedRaw = git(['diff', '--name-only', from, to], repoRoot);
   if (changedRaw === undefined) return undefined;
   const blobAt = (ref: string, p: string): string | null => { const r = spawnSync('git', ['show', `${ref}:${p}`], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 16 * 1024 * 1024 }); return r.status === 0 ? r.stdout : null; };
@@ -514,7 +514,19 @@ function productSpec(repoRoot: string, rules: ProductRules, from: string, to: st
   const demoted = !branch ? changed.filter(c => c.class === 'product' && matchesAny(rules.metadata, c.path)).map(c => c.path)
     : [...branch.touched].filter(p => stamped(p) && (changed.some(c => c.path === p && c.class === 'product') || branch.sides.some(([a, b]) => blobAt(a, p) !== blobAt(b, p) && classify(p, a, b) === 'product')));
   const metadataGlobs = rules.metadata.filter(g => !demoted.some(p => matchesAny([g], p)));
-  return { changed, product: ['.', ...excludeGlobs(CARRY_TEST_GLOBS), ...excludeGlobs(metadataGlobs)], tests: [...CARRY_TEST_GLOBS.map(g => `:(glob)${g}`), ...excludeGlobs(metadataGlobs)], demoted };
+  // A demoted path whose from-to change is, line for line, a stamp on the branch's own rows (named by their position in
+  // `from`), a regular file with the reviewed mode at both heads (GST-980): never the whole-text version swap.
+  const ownRows = demoted.filter(p => {
+    const side = branch?.own(p);
+    const mode = (ref: string) => /^(100644|100755) blob /.exec(git(['ls-tree', ref, '--', p], repoRoot) ?? '')?.[1];
+    const before = blobAt(from, p), after = blobAt(to, p), oldVersion = version(from), newVersion = version(to);
+    if (!side || before === null || after === null || !oldVersion || !newVersion || !mode(from) || mode(from) !== mode(to)) return false;
+    const was = before.split('\n');
+    const now = after.split('\n');
+    const lines = was.map((_, i) => i).filter(i => was[i] !== now[i]);
+    return was.length === now.length && lines.length > 0 && lines.every(i => side.at.has(i + 1) && stampOnlyLine(was[i]!, now[i]!, oldVersion, newVersion, rules.templates, side, i + 1));
+  });
+  return { changed, product: ['.', ...excludeGlobs(CARRY_TEST_GLOBS), ...excludeGlobs(metadataGlobs)], tests: [...CARRY_TEST_GLOBS.map(g => `:(glob)${g}`), ...excludeGlobs(metadataGlobs)], demoted, ownRows };
 }
 
 /** A commit's product patch-id against its merge base with the base ('' for no product diff), as write records it and a rewritten-history carry recomputes it. */
@@ -662,7 +674,7 @@ function cmdCarry(rest: string[], json: boolean): number {
       for (const p of [...touched].filter(p => movedHead.has(p) || movedBase.has(p)).sort()) {
         if (changedProduct.length >= CARRY_NAMED_PATHS) break;
         const cls = changed.find(c => c.path === p)?.class ?? classifyCarryPath(p, rules.extra);
-        if (cls === 'metadata' && spec.demoted.includes(p) && !movedBase.has(p)) stampOnly.push(p);
+        if (cls === 'metadata' && spec.ownRows.includes(p) && !movedBase.has(p)) stampOnly.push(p);
         if (cls !== 'product') continue;
         const verdict = comparePath(repoRoot, sides, p);
         if (verdict === 'changed') changedProduct.push(p);

changelog.d/gst-979-980-stamp-positions.md

@@ -4,11 +4,11 @@
 - **`lib/stamp-only.ts`:** `BranchSide` carries the positions of the branch's lines, and `stampOnlyLine` takes the removed line's position. Only a line at a branch-added position gets the fork-version bound. When the version does not move, only such a line may change.
 - **`lib/restamp.ts`:** `parseUnifiedDiff` records each removed line's position on the old side, which the tree receipt, the train checks and finish's `stampedFiles` pass along.
 - **`lib/ship-receipt.ts`, `lib/ship-bin-common.ts`:** `stampOnlyText` and `stampedFiles` accept a kept VERSION claim for the branch's own rows and nothing else.
-- **`bin/gstack-ship-receipt` carry:** a stamp file the branch changed as product, whose only change since the reviewed head is a stamp on its own rows and which the base did not touch, is left out of the product comparison. The carry records `a stamp on the branch's own rows (<file>)` as the reason. A product edit anywhere else still refuses.
+- **`bin/gstack-ship-receipt` carry:** a stamp file the branch changed as product, whose only change since the reviewed head is a stamp on its own rows and which the base did not touch, is left out of the product comparison. The carry records `a stamp on the branch's own rows (<file>)` as the reason. It reads that file line by line. Only lines at the branch's own positions may move, and the file must keep its reviewed mode, so a version edit on a base line of the same file or a mode change still refuses, as does a product edit anywhere else.
 
 ### Tests
 - `test/tree-receipt-stamp-sensitive.test.ts`: the bound holds only at the branch line's position, and a kept VERSION refuses a base line moved down.
-- `test/restamp-mentions.test.ts`: a hand move of a base row whose text the branch duplicates reads `TREE: changed`, and the carry follows the fork-version row stamp but still refuses a product edit beside it.
+- `test/restamp-mentions.test.ts`: a hand move of a base row whose text the branch duplicates reads `TREE: changed`, and the carry follows the fork-version row stamp but still refuses a product edit beside it, a hand version edit on a base line of the same file, and a mode change.
 - `test/gstack-ship-receipt-docs.test.ts`, `test/ship-rereview-triggers.test.ts`: the carry classifier and finish accept the kept-claim row move and refuse a base row moved the same way.
 
 On main, all five new tests fail.

docs/ship-policy.md

@@ -1529,7 +1529,7 @@ Stages, each journaled in `.gstack/tmp/restamp-journal.json`:
 1. **version**: `gstack-version-bump write --version <X>` in the staging worktree (VERSION, the manifest, npm lockfiles); skipped when `restamp_command` owns the version. The manifest gets the npm-valid three-part form, except when the policy lists it in `stamp_paths` or `stamp_files` and its copy at the base holds four parts (gbrain's convention): then it keeps the full version (`--manifest-form full`) and the stage detail says so.
 2. **release_tool**: the policy command, under the contract below.
 3. **changelog**: the PR's `[Unreleased]` or old-version section moves to the top under `<X>`, above its predecessor's entry. A duplicate heading the PR introduces (`CHANGELOG_DUPLICATE_HEADING`: the headings are counted against the base's CHANGELOG at `origin/<base>` and at the fork point, so a version the base already heads twice passes as long as the PR adds no further copy) and a top entry that is not `<X>` (`CHANGELOG_TOP_MISMATCH`) are errors; the section being re-headed itself may exist only once.
-4. **stamp_paths**: exact `(was X)`, `since: X` and `vX` rewrites, plus every `stamp_patterns` template, within `stamp_paths` and `stamp_files`. A quote around X in the first two forms is part of the form and is kept as written: `since: 'X'`, `since: "X"`, `(was 'X')` and `(was "X")` move like the bare ones (the shape of a quoted source row such as gbrain's `{ since: '0.60.468.0', … }`), an opened quote must close right after X (`since: '1.2.3-beta'` and `since: '1.2.3"` are not the form, in the writer and in the tree receipt alike), and `vX` already accepts a quote before the `v`. Which X moves is decided by provenance. The branch's own tentative stamp (its VERSION when it moved past base) and `--was <X>`, when given, are each rewritten on every line of every stamp file. Beyond that, only lines the branch itself added are read, by their position in the file (`git diff --unified=0` from the fork point), so a base line with the same text, or a CRLF checkout whose diff is LF-normalized, is told apart: a stamp-form mention there of a version newer than the version at the branch's fork point is a tentative stamp and moves (this is what finds a quoted row after a lost slot and a merge of the base, since the base's release is newer than the fork point), and so does one of the fork-point version itself (a row the branch wrote at the version it forked from instead of a next version is its own stale stamp; a branch-added mention older than the fork point stays), and when the branch has no stamp of its own because its VERSION equals the base's and its fork point is the base itself (a branch rebased onto the base after it reached X, or one never stamped) a mention of the base version on a line it added is its stale stamp and moves too, while the same version on a line the base already carries is the base's release and stays. The stage detail names both counts per file (`VERSION equals base X, so X moved only on lines this branch added and stayed on the base's own lines (<file>: N moved, M kept)`), so the stamp commit shows which lines moved and why. A line the base added since the fork point is the base's even when that diff shows it (a merge of the base on the branch, committed or still staged, brings every line the base added since the fork into the diff from the fork point, main's own release keys included): it is left out, the detail says `N line(s) the base added since the fork point are the base's, not this branch's, and stay (<file>: N)`, and a base diff git cannot run refuses the stage (`POLICY_SOURCE_UNAVAILABLE`, `cannot diff <since>..<base> for the stamp_paths stage`) rather than reading as an empty set that would move the base's lines again. The rule reads line ownership, not meaning: a branch-added line inside `stamp_paths` that means the base's own release in a stamp form (`Requires vX or later` written on a branch forked at X) moves with it, which the `--dry-run` diff and the per-file counts show before anything is written; write such a reference outside a stamp form, or keep that file out of `stamp_paths`. `--was X` is the opposite tool: it rewrites X everywhere, the base's own lines included, and re-heads the CHANGELOG section `## [X]`. Only the repo's own version width counts, so a 3-digit dependency pin in a 4-digit repo is never a candidate. The checks that accept a stamp commit read the same bound as the writer (`stampOnlyLine` with the branch's side in `lib/stamp-only.ts`): the tree receipt behind `stamp`, `verify` and the gate-ahead replay, the merge train's stamp and verify checks (each reading the slot's predecessor, the gated commit's first parent, as the base), the /ship receipt carry, and finish's route each take a moved version on a line the branch added at or above the fork-point version, and on any other line at or above the gated head's VERSION. A branch line is named by its position in the gated file, so a base line whose text duplicates a line the branch added keeps the gated head's bound (GST-979). When the stamp keeps the VERSION (a slot that keeps its claim), only the branch's own lines may move, so a base line moved by hand is refused. The carry follows a stamp on the branch's own rows in a stamp file the branch also changed as product when the base did not touch that file and the rest of the product diff is unchanged (`a stamp on the branch's own rows` in the carry reason, GST-980).
+4. **stamp_paths**: exact `(was X)`, `since: X` and `vX` rewrites, plus every `stamp_patterns` template, within `stamp_paths` and `stamp_files`. A quote around X in the first two forms is part of the form and is kept as written: `since: 'X'`, `since: "X"`, `(was 'X')` and `(was "X")` move like the bare ones (the shape of a quoted source row such as gbrain's `{ since: '0.60.468.0', … }`), an opened quote must close right after X (`since: '1.2.3-beta'` and `since: '1.2.3"` are not the form, in the writer and in the tree receipt alike), and `vX` already accepts a quote before the `v`. Which X moves is decided by provenance. The branch's own tentative stamp (its VERSION when it moved past base) and `--was <X>`, when given, are each rewritten on every line of every stamp file. Beyond that, only lines the branch itself added are read, by their position in the file (`git diff --unified=0` from the fork point), so a base line with the same text, or a CRLF checkout whose diff is LF-normalized, is told apart: a stamp-form mention there of a version newer than the version at the branch's fork point is a tentative stamp and moves (this is what finds a quoted row after a lost slot and a merge of the base, since the base's release is newer than the fork point), and so does one of the fork-point version itself (a row the branch wrote at the version it forked from instead of a next version is its own stale stamp; a branch-added mention older than the fork point stays), and when the branch has no stamp of its own because its VERSION equals the base's and its fork point is the base itself (a branch rebased onto the base after it reached X, or one never stamped) a mention of the base version on a line it added is its stale stamp and moves too, while the same version on a line the base already carries is the base's release and stays. The stage detail names both counts per file (`VERSION equals base X, so X moved only on lines this branch added and stayed on the base's own lines (<file>: N moved, M kept)`), so the stamp commit shows which lines moved and why. A line the base added since the fork point is the base's even when that diff shows it (a merge of the base on the branch, committed or still staged, brings every line the base added since the fork into the diff from the fork point, main's own release keys included): it is left out, the detail says `N line(s) the base added since the fork point are the base's, not this branch's, and stay (<file>: N)`, and a base diff git cannot run refuses the stage (`POLICY_SOURCE_UNAVAILABLE`, `cannot diff <since>..<base> for the stamp_paths stage`) rather than reading as an empty set that would move the base's lines again. The rule reads line ownership, not meaning: a branch-added line inside `stamp_paths` that means the base's own release in a stamp form (`Requires vX or later` written on a branch forked at X) moves with it, which the `--dry-run` diff and the per-file counts show before anything is written; write such a reference outside a stamp form, or keep that file out of `stamp_paths`. `--was X` is the opposite tool: it rewrites X everywhere, the base's own lines included, and re-heads the CHANGELOG section `## [X]`. Only the repo's own version width counts, so a 3-digit dependency pin in a 4-digit repo is never a candidate. The checks that accept a stamp commit read the same bound as the writer (`stampOnlyLine` with the branch's side in `lib/stamp-only.ts`): the tree receipt behind `stamp`, `verify` and the gate-ahead replay, the merge train's stamp and verify checks (each reading the slot's predecessor, the gated commit's first parent, as the base), the /ship receipt carry, and finish's route each take a moved version on a line the branch added at or above the fork-point version, and on any other line at or above the gated head's VERSION. A branch line is named by its position in the gated file, so a base line whose text duplicates a line the branch added keeps the gated head's bound (GST-979). When the stamp keeps the VERSION (a slot that keeps its claim), only the branch's own lines may move, so a base line moved by hand is refused. The carry follows a stamp on the branch's own rows in a stamp file the branch also changed as product when every changed line of that file sits at a branch-added position, the file keeps its mode, the base did not touch it and the rest of the product diff is unchanged (`a stamp on the branch's own rows` in the carry reason, GST-980).
 5. **mirror**: tier 2; declared and validated, reported as not run.
 6. **apply**: every changed file is written to the branch only after all stages validated, each against the fingerprinted preimage; a file edited meanwhile is `RESTAMP_CONFLICT` and the applied writes roll back (rollback refuses to overwrite a file whose bytes differ from the recorded postimage).
 

test/restamp-mentions.test.ts

@@ -383,4 +383,52 @@ describe('a branch-added row left at the fork version after the base moved (GST-
     expect(changed.status).toBe(EXIT.refused);
     expect(changed.err).toContain('RECEIPT_CARRY_PRODUCT_CHANGED');
   });
+
+  /** The GST-980 carry state: a receipt written at the claim 1.2.4.0 with the branch's own row, then the stamp to 1.2.5.0, uncommitted. */
+  function carryAfterStamp(prefix: string, baseRows: string[], mine: string, stamped: string[]): { repo: FixtureRepo; body: string } {
+    const repo = makeFixtureRepo({ prefix, policy: policy(true) }); repos.push(repo);
+    write(repo, NOTICE, notice(baseRows));
+    fs.rmSync(path.join(repo.work, 'package-lock.json'));
+    repo.git('add', '-A'); repo.git('commit', '-qm', 'notice rows, no lockfile'); repo.git('push', '-q', 'origin', 'main');
+    makePr(repo, 'pr-carry');
+    expect(restamp(repo, ['--version', '1.2.4.0']).status).toBe(EXIT.ok);
+    repo.git('commit', '-qam', 'stamp 1.2.4.0');
+    write(repo, NOTICE, notice([...baseRows, '', mine]));
+    repo.git('commit', '-qam', 'mine: a row of its own');
+    const body = path.join(repo.root, 'body.md');
+    fs.writeFileSync(body, '## Summary\n');
+    const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '3', '--base', 'main', '--session-kind', 'x', '--body', body]);
+    expect(w.status, w.err).toBe(EXIT.ok);
+    expect(restamp(repo, ['--version', '1.2.5.0']).status).toBe(EXIT.ok);
+    expect(readFile(repo, NOTICE)).toBe(notice(stamped));
+    return { repo, body };
+  }
+
+  // Value: protects=the carry leaves out only a stamp on the branch's own rows, never a version edit on a base line of
+  //   the same file (GST-980 review); fails_when=the carry trusts the whole-text version swap, so a hand move of a base
+  //   line's version mention rides along with the row stamp; seam=bin/gstack-ship-receipt productSpec ownRows
+  test('the carry refuses a hand version edit on a base line of a stamp file whose own row it follows (GST-980)', () => {
+    const min = "export const MIN = '1.2.4.0';";
+    const { repo, body } = carryAfterStamp('restamp-carry-base-line-', [row('1.2.3.0', 'base_row'), min], row('1.2.4.0', 'mine'),
+      [row('1.2.3.0', 'base_row'), min, '', row('1.2.5.0', 'mine')]);
+    write(repo, NOTICE, notice([row('1.2.3.0', 'base_row'), min.replace('1.2.4.0', '1.2.5.0'), '', row('1.2.5.0', 'mine')]));
+    repo.git('commit', '-qam', 'stamp 1.2.5.0, and move the base pin by hand');
+    const carried = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']);
+    expect(carried.status, carried.out).toBe(EXIT.refused);
+    expect(carried.err).toContain('RECEIPT_CARRY_PRODUCT_CHANGED');
+  });
+
+  // Value: protects=a mode change rides on no stamp: the carry leaves out a stamp file only when its mode is the
+  //   reviewed one (GST-980 review); fails_when=the left-out stamp file drops its mode line from both patch-ids;
+  //   seam=bin/gstack-ship-receipt productSpec ownRows
+  test('the carry refuses a mode change on a stamp file whose own row it follows (GST-980)', () => {
+    const { repo, body } = carryAfterStamp('restamp-carry-mode-', [row('1.2.3.0', 'base_row')], row('1.2.3.0', 'mine'),
+      [row('1.2.3.0', 'base_row'), '', row('1.2.5.0', 'mine')]);
+    fs.chmodSync(path.join(repo.work, NOTICE), 0o755);
+    repo.git('commit', '-qam', 'stamp 1.2.5.0, and make the file executable');
+    expect(repo.git('ls-tree', 'HEAD', '--', NOTICE)).toStartWith('100755 ');
+    const carried = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']);
+    expect(carried.status, carried.out).toBe(EXIT.refused);
+    expect(carried.err).toContain('RECEIPT_CARRY_PRODUCT_CHANGED');
+  });
 });