fix(ship-receipt): review fixes: a pin needs exactly one naming map key; stamp-file sides compare object ids (GST-1210)

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

bin/gstack-ship-receipt

@@ -538,7 +538,7 @@ function productSpec(repoRoot: string, rules: ProductRules, from: string, to: st
   // A stamp file the branch changed on neither side beyond a stamp against that side's base (back at the base's content
   // after a changelog-fragment conversion, or only its version moved) holds none of the branch's product: whatever else
   // differs between the two heads came from the base (GST-1210).
-  const ownStampOnly = (p: string) => !!branch?.touched.has(p) && stamped(p) && branch.sides.every(([a, b]) => blobAt(a, p) === blobAt(b, p) || classify(p, a, b) === 'metadata');
+  const ownStampOnly = (p: string) => !!branch?.touched.has(p) && stamped(p) && branch.sides.every(([a, b]) => (git(['rev-parse', '--verify', '--quiet', `${a}:${p}`], repoRoot) ?? 'absent') === (git(['rev-parse', '--verify', '--quiet', `${b}:${p}`], repoRoot) ?? 'absent') || classify(p, a, b) === 'metadata');
   const changed = changedRaw.split('\n').filter(Boolean).map(p => ({ path: p, class: ownStampOnly(p) ? 'metadata' as const : classify(p, from, to) }));
   const excludeGlobs = (globs: readonly string[]) => globs.map(g => `:(exclude,glob)${g}`);
   const demoted = !branch ? changed.filter(c => c.class === 'product' && matchesAny(rules.metadata, c.path)).map(c => c.path)

lib/ship-patch.ts

@@ -125,7 +125,8 @@ export function comparePath(repoRoot: string, s: PatchSides, rel: string): PathV
  * branch added (absent at both bases, a regular file at both heads) named `…/<key>.json`, whose whole text at each head is
  * byte for byte the value of entry `<key>` in one JSON map line of a base file `baseMoved` lists: the old value at the
  * reviewed base, the new one at the current base. The branch then holds the base's own edit of that entry and nothing of
- * its own. Null for anything else, so a pin that differs from the base's entry stays a product change.
+ * its own. Exactly one entry of that map may name the file. Null for anything else, so a pin that differs from the base's
+ * entry stays a product change.
  */
 export function portedEntry(repoRoot: string, s: PatchSides, rel: string, baseMoved: Iterable<string>): string | null {
   if (!rel.endsWith('.json')) return null;
@@ -138,14 +139,14 @@ export function portedEntry(repoRoot: string, s: PatchSides, rel: string, baseMo
   const entries = (text: string | null) => new Set((text ?? '').split('\n').map(l => l.trim().replace(/,$/, '')));
   for (const file of baseMoved) {
     if (file === rel || !file.endsWith('.json')) continue;
-    const now = entries(blob(s.currentBase, file));
-    for (const line of now) {
+    const named = [...entries(blob(s.currentBase, file))].flatMap(line => {
       const m = /^("(?:[^"\\]|\\.)*"): (.*)$/.exec(line);
-      if (!m || m[2] !== after) continue;
       let key: unknown;
-      try { key = JSON.parse(m[1]!); } catch { continue; }
-      if (typeof key === 'string' && key && rel.endsWith(`/${key}.json`) && entries(blob(s.reviewedBase, file)).has(`${m[1]}: ${before}`)) return file;
-    }
+      try { key = m ? JSON.parse(m[1]!) : null; } catch { return []; }
+      return typeof key === 'string' && key && rel.endsWith(`/${key}.json`) ? [{ quoted: m![1]!, value: m![2]! }] : [];
+    });
+    // One entry of the map names the pin file; two (`retry.js` and `lib/retry.js` for `…/lib/retry.js.json`) is no port.
+    if (named.length === 1 && named[0]!.value === after && entries(blob(s.reviewedBase, file)).has(`${named[0]!.quoted}: ${before}`)) return file;
   }
   return null;
 }

test/ship-receipt-carry-ported-pin.test.ts

@@ -18,13 +18,13 @@ const carry = (repo: FixtureRepo, body: string) => runBin(repo, 'gstack-ship-rec
 const RETRY = '{"blob":"aaa","since":6,"nonBehavioral":"first pin"}';
 const UTIL = '{"blob":"bbb","since":6}';
 const RETRY_MAIN = '{"blob":"ccc","since":6,"nonBehavioral":"main repinned retry"}';
-const manifest = (retry: string) => `{\n  "version": 6,\n  "files": {\n    "lib/retry.js": ${retry},\n    "lib/util.js": ${UTIL}\n  }\n}\n`;
+const manifest = (retry: string, short?: string) => `{\n  "version": 6,\n  "files": {\n    "lib/retry.js": ${retry},\n    "lib/util.js": ${UTIL}${short ? `,\n    "retry.js": ${short}` : ''}\n  }\n}\n`;
 const PIN = 'scripts/harness-pins/lib/retry.js.json';
 
 /** main holds the pin map; feature splits it into one file per entry beside a product change, /ship receipt there; main repins retry; feature merges main and writes `ports`. */
-function ported(repo: FixtureRepo, ports: Record<string, string>): { reviewed: string; body: string } {
+function ported(repo: FixtureRepo, ports: Record<string, string>, maps: [string, string] = [manifest(RETRY), manifest(RETRY_MAIN)]): { reviewed: string; body: string } {
   repo.git('checkout', '-q', 'main');
-  commit(repo, { 'scripts/harness-version.json': manifest(RETRY) }, 'main: harness pins');
+  commit(repo, { 'scripts/harness-version.json': maps[0] }, 'main: harness pins');
   repo.git('push', '-q', 'origin', 'main');
   repo.git('checkout', '-q', 'feature');
   repo.git('reset', '-q', '--hard', 'origin/main');
@@ -32,7 +32,7 @@ function ported(repo: FixtureRepo, ports: Record<string, string>): { reviewed: s
   const body = path.join(repo.root, 'body.md');
   expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '3903', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]).status).toBe(EXIT.ok);
   repo.git('checkout', '-q', 'main');
-  commit(repo, { 'scripts/harness-version.json': manifest(RETRY_MAIN), 'docs/README.md': '# docs after the repin\n' }, 'main: repin retry');
+  commit(repo, { 'scripts/harness-version.json': maps[1], 'docs/README.md': '# docs after the repin\n' }, 'main: repin retry');
   repo.git('push', '-q', 'origin', 'main');
   repo.git('checkout', '-q', 'feature');
   repo.git('merge', '-q', '--no-edit', 'origin/main');
@@ -75,4 +75,14 @@ describe('carry across a ported harness-pin entry (GST-1210)', () => {
     expect(out.status).toBe(EXIT.refused);
     expect(out.err).toContain('scripts/harness-pins/lib/util.js.json');
   }, 90_000);
+
+  // Value: protects=fail closed when a shorter key's repin lands in a longer key's pin file (`retry.js` for
+  // `…/lib/retry.js.json`); fails_when=the key is matched as a bare path suffix; seam=none; no_claim=gh pr edit
+  test('a repin of a key that is only a suffix of the pin path still refuses', () => {
+    const repo = makeFixtureRepo(); repos.push(repo);
+    const { body } = ported(repo, { [PIN]: `${RETRY_MAIN}\n` }, [manifest(RETRY, RETRY), manifest(RETRY, RETRY_MAIN)]);
+    const out = carry(repo, body);
+    expect(out.status).toBe(EXIT.refused);
+    expect(out.err).toContain(PIN);
+  }, 90_000);
 });