fix(ok): resolve mirror-PR fix references by their origin trailer (PRD-7718) (#3085)

.github/scripts/write-back.mjs

@@ -40,9 +40,12 @@
 import { execFileSync, spawnSync } from 'node:child_process';
 import { pathToFileURL } from 'node:url';
 import {
+  firstContainingStableTag,
   parseFixRef,
+  parseGitOriginRevIds,
   resolvePrivateSha,
   resolveShippedVersion,
+  sortStableTagsAscending,
 } from './resolve-shipped-version.mjs';
 import { composeReply, evaluateFanIn, partitionAttachments } from './write-back-gate.mjs';
 
@@ -157,12 +160,28 @@ export function isFixRepoInRemit({ kind, owner, repo }, { defaultRepo = DEFAULT_
   return reachable.includes(target);
 }
 
+/**
+ * Is this parsed fix reference a pull request on the MIRROR side — the repo
+ * this workflow runs on, when that repo is not also the private origin? The
+ * second half matters: in a test or rehearsal run executing inside the private
+ * monorepo itself, `GITHUB_REPOSITORY` equals the default private repo, and
+ * its pull requests are private references that must keep the trailer-search
+ * path.
+ */
+export function isSelfRepoPr({ kind, owner, repo }, selfRepo, defaultRepo = DEFAULT_PRIVATE_REPO) {
+  if (kind !== 'pr') return false;
+  const self = String(selfRepo ?? '').trim().toLowerCase();
+  if (!self || self === String(defaultRepo ?? '').trim().toLowerCase()) return false;
+  return self === `${owner}/${repo}`.toLowerCase();
+}
+
 export function deriveVersionForFixRefs({
   fixReferences = [],
   stableTags,
   findMirroredCommits,
   contains,
   resolvePrMergeSha,
+  readCommitMessage = () => null,
   defaultRepo = DEFAULT_PRIVATE_REPO,
   selfRepo = process.env.GITHUB_REPOSITORY,
   log = () => {},
@@ -180,12 +199,49 @@ export function deriveVersionForFixRefs({
       );
       return null;
     }
-    const privateSha = resolvePrivateSha(parsed, { resolvePrMergeSha });
-    if (!privateSha) {
+    const refSha = resolvePrivateSha(parsed, { resolvePrMergeSha });
+    if (!refSha) {
       log(`::notice::write-back: ${ref.url} was closed without merging, so it carries no fix commit.`);
       return null;
     }
-    const result = resolveShippedVersion({ privateSha, stableTags, findMirroredCommits, contains });
+    let result;
+    if (isSelfRepoPr(parsed, selfRepo, defaultRepo)) {
+      // A pull request in this repo (the Copybara mirror lands every export
+      // through one, and Linear's linkback attaches it to the ticket) merges a
+      // commit that is already on the mirror side, so its SHA must not be
+      // trailer-searched as if it were a private one: nothing carries
+      // `GitOrigin-RevId: <mirror-sha>`, and treating it that way made the
+      // whole ticket underivable. Recover the origin SHA from the merge
+      // commit's own trailer and resolve THAT, so cherry-picked copies on
+      // point-release tags are found too. A merge commit with no trailer — or
+      // an ambiguous one that embedded someone else's footer — falls back to
+      // containment of the merge SHA itself.
+      // A batched sync PR (several rebased commits landing through one PR,
+      // which only happens when the serialized mirror is catching up) makes
+      // this trailer belong to the LAST sibling in the batch rather than the
+      // reporter's fix. Highest-wins across refs keeps that error one-sided:
+      // the reporter can be told a slightly later version, never an earlier
+      // one that lacks their fix.
+      const revIds = parseGitOriginRevIds(readCommitMessage(refSha) ?? '');
+      if (revIds.length === 1) {
+        result = resolveShippedVersion({ privateSha: revIds[0], stableTags, findMirroredCommits, contains });
+      } else {
+        log(
+          `::notice::write-back: ${ref.url} merges a commit with ${revIds.length === 0 ? 'no' : 'more than one'} ` +
+            'origin trailer; falling back to direct tag containment of the merge commit itself.',
+        );
+        const tag = firstContainingStableTag({
+          sortedStableTags: sortStableTagsAscending(stableTags),
+          sha: refSha,
+          contains,
+        });
+        result = tag
+          ? { shipped: true, version: tag.replace(/^v/, ''), tag }
+          : { shipped: false, reason: 'not-in-any-stable' };
+      }
+    } else {
+      result = resolveShippedVersion({ privateSha: refSha, stableTags, findMirroredCommits, contains });
+    }
     if (!result.shipped) {
       log(`::notice::write-back: ${ref.url} has not reached a stable release yet (${result.reason}).`);
       return null;
@@ -742,6 +798,10 @@ function realStableTags() {
     .filter((t) => STABLE_TAG_RE.test(t));
 }
 
+function realReadCommitMessage(sha) {
+  return runGit(['show', '-s', '--format=%B', sha]);
+}
+
 const REC_SEP = '\x1e';
 const UNIT_SEP = '\x1f';
 
@@ -982,6 +1042,7 @@ async function main() {
       findMirroredCommits: realFindMirroredCommits,
       contains: realContains,
       resolvePrMergeSha: realResolvePrMergeSha,
+      readCommitMessage: realReadCommitMessage,
       log,
     });
 

.github/scripts/write-back.test.mjs

@@ -8,6 +8,7 @@ import {
   findChangesetPath,
   isFixRepoInRemit,
   isRetryableNetworkError,
+  isSelfRepoPr,
   isRetryableStatus,
   LINEAR_RETRY_ATTEMPTS,
   LINEAR_RETRY_CAP_MS,
@@ -159,6 +160,125 @@ describe('version derivation', () => {
     expect(derive({ fixReferences: [] })).toBeNull();
   });
 
+  test('a mirror pull request in this repo resolves through its merge commit trailer', () => {
+    // The Copybara mirror lands every export through a short-lived PR in this
+    // repo, and Linear's linkback attaches it to the ticket. Its merge commit
+    // IS the mirrored copy, so the resolvable identity is the origin SHA in
+    // its own trailer — which also finds the cherry-picked copy a point
+    // release carries when the main-line copy is in no stable yet.
+    const mirrorMain = 'd'.repeat(40);
+    const cherryPick = 'e'.repeat(40);
+    expect(
+      deriveVersionForFixRefs({
+        fixReferences: [{ channel: 'pull-request', url: 'https://github.com/inkeep/open-knowledge/pull/928' }],
+        stableTags: STABLE_TAGS,
+        selfRepo: 'inkeep/open-knowledge',
+        resolvePrMergeSha: () => mirrorMain,
+        readCommitMessage: (sha) => (sha === mirrorMain ? `subject\n\nGitOrigin-RevId: ${PRIVATE_SHA}\n` : null),
+        findMirroredCommits: (sha) =>
+          sha === PRIVATE_SHA
+            ? [
+                { sha: mirrorMain, message: `GitOrigin-RevId: ${PRIVATE_SHA}` },
+                { sha: cherryPick, message: `GitOrigin-RevId: ${PRIVATE_SHA}` },
+              ]
+            : [],
+        contains: containsFrom({ [cherryPick]: ['v0.35.6', 'v0.36.0'] }),
+      }),
+    ).toBe('0.35.6');
+  });
+
+  test('a private fix reference still derives with the mirror PR echo attached beside it', () => {
+    // Regression: the echo used to be trailer-searched as a private SHA, find
+    // nothing, and null the whole ticket — every ticket whose fix PR title
+    // carried an identifier became permanently underivable.
+    const mirrorEcho = 'f'.repeat(40);
+    expect(
+      deriveVersionForFixRefs({
+        fixReferences: [
+          { channel: 'pull-request', url: GH_PULL },
+          { channel: 'pull-request', url: 'https://github.com/inkeep/open-knowledge/pull/928' },
+        ],
+        stableTags: STABLE_TAGS,
+        selfRepo: 'inkeep/open-knowledge',
+        resolvePrMergeSha: ({ repo }) => (repo === 'agents-private' ? PRIVATE_SHA : mirrorEcho),
+        readCommitMessage: (sha) => (sha === mirrorEcho ? `subject\n\nGitOrigin-RevId: ${PRIVATE_SHA}\n` : null),
+        findMirroredCommits: (sha) =>
+          sha === PRIVATE_SHA ? [{ sha: MIRRORED_SHA, message: `GitOrigin-RevId: ${PRIVATE_SHA}` }] : [],
+        contains: containsFrom({ [MIRRORED_SHA]: ['v0.36.0'] }),
+      }),
+    ).toBe('0.36.0');
+  });
+
+  test('a mirror pull request with no trailer falls back to containment of its merge commit', () => {
+    const directSha = 'a1'.repeat(20);
+    expect(
+      deriveVersionForFixRefs({
+        fixReferences: [{ channel: 'pull-request', url: 'https://github.com/inkeep/open-knowledge/pull/500' }],
+        stableTags: STABLE_TAGS,
+        selfRepo: 'inkeep/open-knowledge',
+        resolvePrMergeSha: () => directSha,
+        readCommitMessage: () => 'subject with no trailer\n',
+        findMirroredCommits: () => {
+          throw new Error('a trailerless self-repo merge must not be trailer-searched');
+        },
+        contains: containsFrom({ [directSha]: ['v0.36.0'] }),
+      }),
+    ).toBe('0.36.0');
+  });
+
+  test('a mirror merge commit with more than one origin trailer is ambiguous and falls back to containment', () => {
+    // A message can embed someone else's footer; picking either trailer would
+    // be a guess, so the merge commit's own containment is the answer.
+    const otherSha = '9'.repeat(40);
+    const mergeSha = 'c3'.repeat(20);
+    const logs = [];
+    expect(
+      deriveVersionForFixRefs({
+        fixReferences: [{ channel: 'pull-request', url: 'https://github.com/inkeep/open-knowledge/pull/502' }],
+        stableTags: STABLE_TAGS,
+        selfRepo: 'inkeep/open-knowledge',
+        resolvePrMergeSha: () => mergeSha,
+        readCommitMessage: () => `GitOrigin-RevId: ${PRIVATE_SHA}\nGitOrigin-RevId: ${otherSha}\n`,
+        findMirroredCommits: () => {
+          throw new Error('an ambiguous trailer must not be trailer-searched');
+        },
+        contains: containsFrom({ [mergeSha]: ['v0.35.6'] }),
+        log: (m) => logs.push(m),
+      }),
+    ).toBe('0.35.6');
+    expect(logs.join(' ')).toContain('more than one origin trailer');
+  });
+
+  test('isSelfRepoPr is repo-identity plus a not-the-origin guard', () => {
+    const pr = (owner, repo) => ({ kind: 'pr', owner, repo, number: 1 });
+    expect(isSelfRepoPr(pr('inkeep', 'open-knowledge'), 'inkeep/open-knowledge')).toBe(true);
+    expect(isSelfRepoPr(pr('inkeep', 'Open-Knowledge'), 'inkeep/open-knowledge')).toBe(true);
+    expect(isSelfRepoPr(pr('inkeep', 'agents-private'), 'inkeep/open-knowledge')).toBe(false);
+    expect(isSelfRepoPr({ kind: 'sha', sha: 'a'.repeat(40) }, 'inkeep/open-knowledge')).toBe(false);
+    expect(isSelfRepoPr(pr('inkeep', 'open-knowledge'), undefined)).toBe(false);
+    // A run inside the private monorepo: its own PRs are private references,
+    // not mirror-side ones, even though repo identity matches selfRepo.
+    expect(isSelfRepoPr(pr('inkeep', 'agents-private'), 'inkeep/agents-private')).toBe(false);
+    expect(isSelfRepoPr(pr('inkeep', 'some-fork'), 'inkeep/some-fork', 'inkeep/some-fork')).toBe(false);
+  });
+
+  test('a mirror pull request contained in no stable yet yields no version', () => {
+    const logs = [];
+    expect(
+      deriveVersionForFixRefs({
+        fixReferences: [{ channel: 'pull-request', url: 'https://github.com/inkeep/open-knowledge/pull/501' }],
+        stableTags: STABLE_TAGS,
+        selfRepo: 'inkeep/open-knowledge',
+        resolvePrMergeSha: () => 'b2'.repeat(20),
+        readCommitMessage: () => 'subject with no trailer\n',
+        findMirroredCommits: () => [],
+        contains: () => false,
+        log: (m) => logs.push(m),
+      }),
+    ).toBeNull();
+    expect(logs.join(' ')).toContain('not-in-any-stable');
+  });
+
   test('a fix reference naming a pull request that was closed unmerged is an answer, not a fault', () => {
     // Real shape: PRD-7539 still carries agents-private#2844, which was closed
     // in favour of #2864. A stale attachment is for a human to fix in Linear,
@@ -209,7 +329,9 @@ describe('version derivation', () => {
 
   test('the two repos it CAN read are still attempted, so the 404 bug cannot come back', () => {
     // A blanket 404 suppression would have masked the missing-permission bug on
-    // agents-private. These two must always reach the API.
+    // agents-private. These two must always reach the API. The self-repo PR
+    // resolves through its merge commit's own trailer rather than a trailer
+    // search for its SHA, but it is attempted all the same.
     for (const url of [
       'https://github.com/inkeep/agents-private/pull/2864',
       'https://github.com/inkeep/open-knowledge/pull/12',
@@ -223,6 +345,8 @@ describe('version derivation', () => {
             attempted = true;
             return PRIVATE_SHA;
           },
+          readCommitMessage: (sha) =>
+            sha === PRIVATE_SHA ? `subject\n\nGitOrigin-RevId: ${PRIVATE_SHA}\n` : null,
         }),
       ).toBe('0.36.0');
       expect(attempted, `${url} must be attempted`).toBe(true);