fix: inherit records, not every trailer paragraph, on squash-preserve

src/core/squash.ts

@@ -69,6 +69,7 @@ import {
   CERTAINTY_VALUES,
   SINGLE_VALUED,
   UNDO_VALUES,
+  isCommitLoreKey,
   type Trailer,
 } from './types.js';
 
@@ -291,6 +292,36 @@ const recordIdOf = (record: CollectedRecord): string | undefined =>
 const contentSet = (trailers: readonly Trailer[]): Set<string> =>
   new Set(trailers.map((trailer) => `${trailer.key}${NUL}${trailer.value}`));
 
+/**
+ * One commit's blocks reduced to the records among them: the trailers whose
+ * keys this protocol defines, and no block left without one.
+ *
+ * git's grammar decides what a trailer block is, and a message's last
+ * paragraph qualifies whatever its keys mean -- so a paragraph holding only
+ * `Claude-Session:` is a trailer block. It is not a record, and inheriting it
+ * wrote a key SPEC §3 does not define into the message the squash was about to
+ * commit, which `commitlore validate` then refused, blocking the merge
+ * `squash-preserve` had been run to protect (#1153).
+ *
+ * `types.ts isCommitLoreKey` already answers this question for the index,
+ * where the same paragraphs were served to agents as recorded decisions
+ * (#335). Here it is asked per trailer rather than per block, because what
+ * this module emits is a commit message that has to validate: a paragraph
+ * mixing `Limit:` with a foreign key is a real record, and dropping the whole
+ * block to be rid of the foreign key would lose it. `Signed-off-by:` goes the
+ * same way as any other key outside the vocabulary -- it is an attestation
+ * about the commit that carried it, and inheriting it would assert a sign-off
+ * on a commit nobody signed.
+ *
+ * Applied where both channels become candidate records, before they are
+ * matched (`mergeCommitBlocks`), so a message cannot reintroduce through its
+ * mirror what the mirror dropped, or the other way round.
+ */
+const recordsAmong = (blocks: readonly Trailer[][]): Trailer[][] =>
+  blocks
+    .map((block) => block.filter((trailer) => isCommitLoreKey(trailer.key)))
+    .filter((block) => block.length > 0);
+
 /**
  * Matches one commit's message blocks against its mirrored note blocks
  * (SPEC §2.4), so a source commit that itself carries several record blocks —
@@ -433,7 +464,7 @@ export const collectRange = (range: string, opts: SquashOptions = {}): Collected
     const noteBlocks =
       cachedNote ?? (mirrored.has(sha) ? readRecordBlocks(sha, opts) : []);
     if (cachedNote === undefined) opts.cache?.notes.set(sha, noteBlocks);
-    const blocks = mergeCommitBlocks(messageBlocks, noteBlocks);
+    const blocks = mergeCommitBlocks(recordsAmong(messageBlocks), recordsAmong(noteBlocks));
 
     for (const trailers of blocks) {
       if (trailers.length === 0) continue;

src/core/stale.ts

@@ -19,6 +19,7 @@
 import {
   RECORD_ID_RE,
   SINGLE_VALUED,
+  isCommitLoreKey,
   parseProvenance,
   type Lifecycle,
   type Record,
@@ -471,6 +472,36 @@ const payloadSignatureWithoutProvenance = (record: StaleRecord): string =>
     .sort()
     .join('\u0001');
 
+/**
+ * `payloadSignatureWithoutProvenance` restricted to the keys this protocol
+ * defines (`types.ts isCommitLoreKey`).
+ *
+ * What an inherited copy carries is the record, not whatever else the origin's
+ * trailer paragraph happened to hold: `squash-preserve` drops the foreign keys
+ * so the message it composes passes `validate` at all (#1153), and a
+ * `Signed-off-by:` on the origin is an attestation about that commit which
+ * must not be copied onto another. Compared in full, every faithful copy of
+ * such a record would read as divergent and be reported as a `duplicate-id`
+ * -- the same false refusal #1148 removed for the provenance stamp, one key
+ * class over.
+ *
+ * Only the inherited-copy comparison uses this. Weakening it costs the case
+ * where a copy carries a *different* value for a foreign key than its origin
+ * did, which stops being a collision; the record's own content still has to
+ * match exactly.
+ */
+const recordPayloadSignature = (record: StaleRecord): string =>
+  record.trailers
+    .filter(
+      (trailer) =>
+        trailer.key !== RECORD_ID_KEY &&
+        trailer.key !== PROVENANCE_KEY &&
+        isCommitLoreKey(trailer.key),
+    )
+    .map((trailer) => `${trailer.key}\u0000${trailer.value}`)
+    .sort()
+    .join('\u0001');
+
 /**
  * Whether `message` is a fold of several records and `note` is one of them
  * (#1116).
@@ -564,7 +595,7 @@ const collisionRivals = (group: StaleRecord[]): StaleRecord[] => group.flatMap((
   if (isOwnCommitMirror(record, group)) return [];
   const origin = inheritedOrigin(record, group);
   if (origin === undefined) return [record];
-  if (payloadSignatureWithoutProvenance(record) === payloadSignatureWithoutProvenance(origin)) return [];
+  if (recordPayloadSignature(record) === recordPayloadSignature(origin)) return [];
   return [{
     ...record,
     trailers: [

test/squash.test.ts

@@ -710,4 +710,111 @@ describe('squash-preserve', () => {
     expect(outcome.stderr).toContain('nothing to preserve');
     expect(readFileSync(draft, 'utf8')).toBe(GITHUB_DRAFT);
   });
+
+  /**
+   * bug-issue-1153: the paragraph a non-CommitLore trailer forms is a trailer
+   * block to git and not a record to this protocol, and inheriting it put a key
+   * SPEC §3 does not define into the message the squash was about to commit —
+   * which `commitlore validate` then refused, blocking the merge it was run to
+   * protect.
+   */
+  it('inherits nothing from a paragraph that carries no record key (bug-issue-1153)', () => {
+    const repo = initRepo('squash-nonrecord-paragraph');
+    const base = commitFile(repo, 'seed.txt', 'seed\n', 'seed\n');
+    const merge = commitFile(
+      repo,
+      'merge.txt',
+      'merge\n',
+      'Merge pull request #1051 from feature\n\nClaude-Session: https://claude.ai/code/session_01ABC\n',
+    );
+    const recorded = commitFile(
+      repo,
+      'queue.ts',
+      'export const workers = 3;\n',
+      'cap the pool\n\nLimit: the vendor caps us at three concurrent workers\nRecord-Id: r-nonrec1153\n',
+    );
+    const draft = draftFile(repo, 'squash the branch\n');
+
+    const outcome = runSquashPreserve({ range: `${base}..HEAD`, messageFile: draft, cwd: repo });
+    expect(outcome.code).toBe(0);
+
+    const composed = readFileSync(draft, 'utf8');
+    // The specific diagnosis, not merely "validate passed": the unfixed build
+    // failed with exactly this violation, and a draft can fail validate for
+    // reasons that have nothing to do with this issue.
+    const result = runValidate({ messageFile: draft, cwd: repo });
+    expect(result.violations).not.toContainEqual(
+      expect.objectContaining({ rule: 'unknown-key', key: 'Claude-Session' }),
+    );
+    expect(result.code).toBe(0);
+    expect(result.violations).toEqual([]);
+    expect(composed).not.toContain('Claude-Session');
+    expect(composed).not.toContain(`inherited ${merge}`);
+    expect(composed).not.toContain(merge);
+
+    // Control: the commit that did record something is still inherited, so the
+    // fix is a filter on non-records and not a filter on everything.
+    const blocks = parseRecordBlocks(composed);
+    expect(value(blockById(blocks, 'r-nonrec1153'), 'Limit')).toBe(
+      'the vendor caps us at three concurrent workers',
+    );
+    expect(value(blockById(blocks, 'r-nonrec1153'), 'Provenance')).toBe(`inherited ${recorded}`);
+    expect(collectRange(`${base}..HEAD`, { cwd: repo }).map((entry) => entry.sha)).toEqual([
+      recorded,
+    ]);
+
+    // The notes mirror carries what the message does, so the same non-record
+    // paragraph cannot come back through the other channel on the next squash.
+    git(repo, ['commit', '--quiet', '-F', draft, '--allow-empty']);
+    expect(runSquashPreserve({ range: `${base}..HEAD`, target: 'HEAD', cwd: repo }).code).toBe(0);
+    for (const block of readRecordBlocks(head(repo), { cwd: repo })) {
+      expect(block.map((trailer) => trailer.key)).not.toContain('Claude-Session');
+    }
+  });
+
+  /**
+   * The same defect through the door the first case does not cover: a record
+   * and a foreign trailer in one paragraph. Dropping the whole block would lose
+   * a real record, so the filter is per trailer — an inherited block carries
+   * only keys this protocol defines, which is what makes the composed message
+   * validate by construction rather than by luck.
+   */
+  it('inherits only protocol keys from a block that mixes them (bug-issue-1153)', () => {
+    const repo = initRepo('squash-mixed-paragraph');
+    const base = commitFile(repo, 'seed.txt', 'seed\n', 'seed\n');
+    const mixed = commitFile(
+      repo,
+      'queue.ts',
+      'export const retries = 2;\n',
+      'retry on 429\n\n' +
+        'Warn: do not raise the retry ceiling without re-reading the vendor quota\n' +
+        'Record-Id: r-mixed1153\n' +
+        'X-Deploy-Window: tuesdays\n' +
+        'Signed-off-by: A Committer <committer@example.invalid>\n' +
+        'Claude-Session: https://claude.ai/code/session_01ABC\n',
+    );
+    const draft = draftFile(repo, 'squash the mixed branch\n');
+
+    expect(runSquashPreserve({ range: `${base}..HEAD`, messageFile: draft, cwd: repo }).code).toBe(
+      0,
+    );
+
+    const composed = readFileSync(draft, 'utf8');
+    const block = blockById(parseRecordBlocks(composed), 'r-mixed1153');
+    expect(value(block, 'Warn')).toBe(
+      'do not raise the retry ceiling without re-reading the vendor quota',
+    );
+    expect(value(block, 'Provenance')).toBe(`inherited ${mixed}`);
+    // An `X-` extension is SPEC §3 vocabulary and stays; a sign-off is an
+    // attestation about another commit and is not inheritable onto this one.
+    expect(value(block, 'X-Deploy-Window')).toBe('tuesdays');
+    expect(block.map((trailer) => trailer.key)).not.toContain('Claude-Session');
+    expect(block.map((trailer) => trailer.key)).not.toContain('Signed-off-by');
+
+    const result = runValidate({ messageFile: draft, cwd: repo });
+    expect(result.violations).not.toContainEqual(
+      expect.objectContaining({ rule: 'unknown-key', key: 'Claude-Session' }),
+    );
+    expect(result.code).toBe(0);
+  });
 });