fix(review): scope merge-train FIFO to overlapping siblings, fix dead persistence

src/db/repositories.ts

@@ -566,6 +566,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise
       reviewEvasionProtection: "close", // #4011: default-ON -- see normalizeReviewEvasionProtection's doc comment
       reviewEvasionLabel: DEFAULT_REVIEW_EVASION_LABEL,
       reviewEvasionComment: true,
+      mergeTrainMode: "off",
       screenshotTableGate: { ...DEFAULT_SCREENSHOT_TABLE_GATE, whenLabels: [], whenPaths: [] },
     };
   }
@@ -842,6 +843,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
       reviewEvasionProtection: resolved.reviewEvasionProtection,
       reviewEvasionLabel: resolved.reviewEvasionLabel,
       reviewEvasionComment: resolved.reviewEvasionComment,
+      mergeTrainMode: resolved.mergeTrainMode,
       screenshotTableGateEnabled: resolved.screenshotTableGate.enabled,
       screenshotTableGateWhenLabelsJson: jsonString(resolved.screenshotTableGate.whenLabels),
       screenshotTableGateWhenPathsJson: jsonString(resolved.screenshotTableGate.whenPaths),
@@ -925,6 +927,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
         reviewEvasionProtection: resolved.reviewEvasionProtection,
         reviewEvasionLabel: resolved.reviewEvasionLabel,
         reviewEvasionComment: resolved.reviewEvasionComment,
+        mergeTrainMode: resolved.mergeTrainMode,
         screenshotTableGateEnabled: resolved.screenshotTableGate.enabled,
         screenshotTableGateWhenLabelsJson: jsonString(resolved.screenshotTableGate.whenLabels),
         screenshotTableGateWhenPathsJson: jsonString(resolved.screenshotTableGate.whenPaths),

src/queue/processors.ts

@@ -3167,6 +3167,8 @@ async function runAgentMaintenancePlanAndExecute(
       authorLogin: pr.authorLogin,
       mergeTrainMode: settings.mergeTrainMode,
       pullRequestCreatedAt: pr.createdAt,
+      pullRequestLinkedIssues: pr.linkedIssues,
+      pullRequestChangedFiles: pr.changedFiles,
       // CI-run cancellation on a contributor_cap close (#2462): the repo's own explicit setting always wins;
       // null/undefined (unset) falls back to the install-wide CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT env var.
       contributorCapCancelCi: settings.contributorCapCancelCi ?? env.CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT === "true",

src/review/merge-train.ts

@@ -1,40 +1,94 @@
 // FIFO merge-train gate (#selfhost-merge-train). Without this, a PR merges the instant its OWN gate clears,
 // with zero awareness of an older sibling PR still open in the same repo -- proven live via a production D1
 // query to cause hundreds of out-of-order merges per repo, and the conflicts that follow. This module is the
-// pure decision only: a still-viable older sibling (not conflicted, not past the staleness cap) holds the
-// newer PR's merge until the older one either merges, closes, or goes stale. Mirrors this codebase's existing
-// "advisory, fail-open, defense-in-depth" lock philosophy (see claimTransientLock's own doc comment) rather
-// than a hard, unbypassable serialization -- the staleness cap is the deliberate escape hatch so one stuck old
-// PR can never block the repo's newer PRs forever.
+// pure decision only: a still-viable, OVERLAPPING older sibling (not conflicted, not past the staleness cap)
+// holds the newer PR's merge until the older one either merges, closes, or goes stale. Mirrors this codebase's
+// existing "advisory, fail-open, defense-in-depth" lock philosophy (see claimTransientLock's own doc comment)
+// rather than a hard, unbypassable serialization -- the staleness cap is the deliberate escape hatch so one
+// stuck old PR can never block the repo's newer PRs forever.
+//
+// Overlap-scoped by design (#selfhost-merge-train-overlap), not blanket FIFO: the actual problem this gate
+// exists to prevent is a merge CONFLICT (or duplicated issue-closing effort) from two PRs touching the same
+// area merging out of order -- not "any newer PR must wait for any older PR, related or not." A newer PR whose
+// files/linked-issues share nothing with an older sibling creates no conflict risk by merging first, so it
+// never waits, REGARDLESS of that sibling's review state (this is also what keeps a PR stuck in manual review
+// from silently wedging the ENTIRE queue -- it can only ever hold up a PR that actually overlaps it). An
+// overlapping older sibling DOES still count as a blocker even while held for manual review: letting the
+// newer, overlapping PR merge first doesn't remove the conflict risk, it just defers it to whenever the older
+// PR resumes (a normal rebase-on-conflict then, versus every newer related PR queueing behind it now) -- an
+// acceptable, bounded tradeoff given the 24h staleness cap already prevents an abandoned PR from blocking
+// forever.
 
 /** The subset of a sibling PR's fields this gate actually needs -- kept minimal and independent of
  *  `PullRequestRecord`'s full shape so this module has zero import surface beyond plain data. */
 export type MergeTrainSibling = {
   number: number;
   createdAt?: string | null | undefined;
   mergeableState?: string | null | undefined;
+  /** Issue numbers this PR closes, for overlap detection -- always populated on a real PullRequestRecord
+   *  (empty array, never undefined, when the PR closes no issue). */
+  linkedIssues?: readonly number[] | undefined;
+  /** Changed file paths, when the caller has resolved them (e.g. from the `pull_request_files` cache).
+   *  Absent/undefined degrades to issue-only overlap detection for this sibling, never to "no overlap
+   *  possible" -- a sibling with unresolved files can still overlap via a shared linked issue. */
+  changedFiles?: readonly string[] | undefined;
 };
 
-/** How long an older sibling can hold up newer ones before it's excluded from blocking (24 hours, matching
- *  `REGATE_REPAIR_ATTEMPT_LOOKBACK_MS`'s own "genuinely stuck, not just mid-review" cutoff in
+/** How long an older, OVERLAPPING sibling can hold up a newer one before it's excluded from blocking (24
+ *  hours, matching `REGATE_REPAIR_ATTEMPT_LOOKBACK_MS`'s own "genuinely stuck, not just mid-review" cutoff in
  *  src/queue/processors.ts). A normal review cycle (CI, AI review, human review) can easily run for hours; a
- *  genuinely stuck PR (its author vanished, review never completes) must not wedge every newer PR in the repo
+ *  genuinely stuck PR (its author vanished, review never completes) must not wedge a related newer PR
  *  indefinitely, so this is the escape hatch, not a tight SLA. */
 export const MERGE_TRAIN_MAX_WAIT_MS = 24 * 60 * 60 * 1000;
 
 export type MergeTrainDecision = { wait: true; blockingPr: number } | { wait: false };
 
-/** True when an older, still-viable sibling exists and `thisPrNumber` should wait its turn. A sibling never
- *  blocks when it is: the same PR, not older (by createdAt, falling back to PR number when createdAt is
- *  missing on either side -- mirrors the duplicate-winner election's own createdAt-then-number precedent),
- *  git-conflicted (`mergeableState === "dirty"` -- it isn't "about to merge," it's stuck), or past the
- *  staleness cap. Deterministic and total: same inputs always produce the same decision. */
-export function shouldWaitForOlderSiblings(
-  thisPrNumber: number,
-  thisPrCreatedAt: string | null | undefined,
-  siblings: readonly MergeTrainSibling[],
-  nowMs: number,
-): MergeTrainDecision {
+/** Low-priority path buckets (lockfiles, generated/build output, dist/ artifacts) that overlapping alone
+ *  never counts as real conflict risk -- ported from `review-grounding.ts`'s `diffFilePriority` classification
+ *  (bucket 4, "least useful to review") so this module stays dependency-free rather than importing a whole
+ *  review-pipeline module for one number. Kept as a small, explicit suffix/name list rather than a generic
+ *  "some overlap" check: a shared `package-lock.json` or `dist/bundle.js` touch is routine noise, not the
+ *  same-area conflict risk this gate exists to catch. */
+const LOW_SIGNAL_FILENAME_RE = /(?:^|\/)(?:package-lock\.json|yarn\.lock|pnpm-lock\.yaml|Cargo\.lock)$/i;
+const LOW_SIGNAL_DIR_RE = /(?:^|\/)(?:dist|build|coverage|node_modules)\//i;
+
+function isMeaningfulPath(path: string): boolean {
+  return !LOW_SIGNAL_FILENAME_RE.test(path) && !LOW_SIGNAL_DIR_RE.test(path);
+}
+
+/** True when `thisPr` and `sibling` overlap enough to carry real conflict/duplicate-effort risk: a shared
+ *  linked issue, OR a shared meaningful (non-lockfile, non-generated) changed file path. A sibling with no
+ *  resolved `changedFiles` can still match via linked issues -- it is never treated as "definitely no overlap"
+ *  purely for missing file data. */
+function overlaps(thisPrLinkedIssues: readonly number[], thisPrChangedFiles: readonly string[] | undefined, sibling: MergeTrainSibling): boolean {
+  const siblingIssues = sibling.linkedIssues ?? [];
+  if (thisPrLinkedIssues.some((issue) => siblingIssues.includes(issue))) return true;
+  if (!thisPrChangedFiles || !sibling.changedFiles) return false;
+  const siblingFiles = new Set(sibling.changedFiles);
+  return thisPrChangedFiles.some((path) => siblingFiles.has(path) && isMeaningfulPath(path));
+}
+
+export type ShouldWaitForOlderSiblingsInput = {
+  thisPrNumber: number;
+  thisPrCreatedAt: string | null | undefined;
+  /** This PR's own linked issues (always available -- `PullRequestRecord.linkedIssues` is never optional). */
+  thisPrLinkedIssues: readonly number[];
+  /** This PR's own changed file paths, when the caller has resolved them. Absent degrades overlap detection
+   *  to linked-issue-only for every sibling (never fails closed into "nothing can overlap"). */
+  thisPrChangedFiles?: readonly string[] | undefined;
+  siblings: readonly MergeTrainSibling[];
+  nowMs: number;
+};
+
+/** True when an OVERLAPPING, older, still-viable sibling exists and `thisPrNumber` should wait its turn. A
+ *  sibling never blocks when it is: the same PR, not older (by createdAt, falling back to PR number when
+ *  createdAt is missing on either side -- mirrors the duplicate-winner election's own createdAt-then-number
+ *  precedent), git-conflicted (`mergeableState === "dirty"` -- it isn't "about to merge," it's stuck), past
+ *  the staleness cap, or simply UNRELATED (shares no linked issue and no meaningful changed file with this PR
+ *  -- see the module header for why overlap-scoping, not blanket FIFO, is the actual fix here). Deterministic
+ *  and total: same inputs always produce the same decision. */
+export function shouldWaitForOlderSiblings(input: ShouldWaitForOlderSiblingsInput): MergeTrainDecision {
+  const { thisPrNumber, thisPrCreatedAt, thisPrLinkedIssues, thisPrChangedFiles, siblings, nowMs } = input;
   const thisCreatedMs = thisPrCreatedAt ? Date.parse(thisPrCreatedAt) : Number.NaN;
   const isOlder = (sibling: MergeTrainSibling): boolean => {
     const siblingCreatedMs = sibling.createdAt ? Date.parse(sibling.createdAt) : Number.NaN;
@@ -51,6 +105,7 @@ export function shouldWaitForOlderSiblings(
     .filter((sibling) => sibling.number !== thisPrNumber)
     .filter((sibling) => sibling.mergeableState !== "dirty")
     .filter((sibling) => isOlder(sibling))
+    .filter((sibling) => overlaps(thisPrLinkedIssues, thisPrChangedFiles, sibling))
     .filter((sibling) => {
       const siblingCreatedMs = sibling.createdAt ? Date.parse(sibling.createdAt) : Number.NaN;
       if (!Number.isFinite(siblingCreatedMs)) return true; // unknown age -- fail open toward still blocking

src/services/agent-action-executor.ts

@@ -7,6 +7,7 @@ import {
   insertNotificationDeliveryIfAbsent,
   isGlobalAgentFrozen,
   listOtherOpenPullRequests,
+  listRepoPullRequestFilePaths,
   markPullRequestApproved,
   markPullRequestMergeBlocked,
   recordAuditEvent,
@@ -190,6 +191,16 @@ export type AgentActionExecutionContext = {
   // gate below compares this against open siblings fetched fresh, since siblings are only ever fetched lazily
   // when the gate is actually enabled (see step 8b), not threaded through every caller unconditionally.
   pullRequestCreatedAt?: string | null | undefined;
+  // This PR's own linked issues (#selfhost-merge-train-overlap), resolved by the CALLER (already has the PR
+  // record in scope): the merge-train gate only holds a merge behind an OVERLAPPING older sibling (shared
+  // linked issue or shared meaningful changed file), never a blanket "any older PR" wait -- see
+  // merge-train.ts's module header for why. Absent/undefined behaves like an empty list (issue-overlap never
+  // matches; file-overlap can still apply via pullRequestChangedFiles below).
+  pullRequestLinkedIssues?: readonly number[] | undefined;
+  // This PR's own changed file paths, when the caller has them resolved (e.g. a webhook path with the
+  // `pull_request_files` cache already populated). Absent/undefined degrades the merge-train overlap check to
+  // linked-issue-only for this PR, never to "no overlap possible".
+  pullRequestChangedFiles?: readonly string[] | undefined;
 };
 
 export type ModerationContextSettings = {
@@ -461,15 +472,39 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE
         continue;
       }
     }
-    // 8b) merge-train FIFO gate (#selfhost-merge-train): a still-viable OLDER open sibling in this repo holds
-    // this merge until it merges, closes, or goes stale (see merge-train.ts's staleness cap). Siblings are
-    // fetched fresh here, lazily, ONLY when the gate is actually enabled for this repo — not threaded through
-    // every caller unconditionally, since the vast majority of merges never need this check. "audit" mode logs
-    // the decision but never actually holds anything, so it's safe to enable everywhere to validate the fix
-    // before switching a repo to "enforce".
+    // 8b) merge-train FIFO gate (#selfhost-merge-train): a still-viable, OVERLAPPING older open sibling in this
+    // repo holds this merge until it merges, closes, or goes stale (see merge-train.ts's staleness cap and its
+    // module header for why overlap-scoping, not blanket FIFO, is the actual fix -- an unrelated older sibling,
+    // even one stuck in manual review, never blocks). Siblings + their changed-file paths are fetched fresh
+    // here, lazily, ONLY when the gate is actually enabled for this repo — not threaded through every caller
+    // unconditionally, since the vast majority of merges never need this check. "audit" mode logs the decision
+    // but never actually holds anything, so it's safe to enable everywhere to validate the fix before switching
+    // a repo to "enforce".
     if (action.actionClass === "merge" && ctx.mergeTrainMode && ctx.mergeTrainMode !== "off") {
       const siblings = await listOtherOpenPullRequests(env, ctx.repoFullName, ctx.pullNumber);
-      const decision = shouldWaitForOlderSiblings(ctx.pullNumber, ctx.pullRequestCreatedAt, siblings, Date.now());
+      const filePaths = await listRepoPullRequestFilePaths(env, ctx.repoFullName, {
+        pullNumbers: [ctx.pullNumber, ...siblings.map((sibling) => sibling.number)],
+      });
+      const pathsByPullNumber = new Map<number, string[]>();
+      for (const row of filePaths) {
+        const paths = pathsByPullNumber.get(row.pullNumber) ?? [];
+        paths.push(row.path);
+        pathsByPullNumber.set(row.pullNumber, paths);
+      }
+      const decision = shouldWaitForOlderSiblings({
+        thisPrNumber: ctx.pullNumber,
+        thisPrCreatedAt: ctx.pullRequestCreatedAt,
+        thisPrLinkedIssues: ctx.pullRequestLinkedIssues ?? [],
+        thisPrChangedFiles: pathsByPullNumber.get(ctx.pullNumber) ?? ctx.pullRequestChangedFiles,
+        siblings: siblings.map((sibling) => ({
+          number: sibling.number,
+          createdAt: sibling.createdAt,
+          mergeableState: sibling.mergeableState,
+          linkedIssues: sibling.linkedIssues,
+          changedFiles: pathsByPullNumber.get(sibling.number),
+        })),
+        nowMs: Date.now(),
+      });
       if (decision.wait) {
         incr("gittensory_merge_train_deferred_total", { repo: ctx.repoFullName, mode: ctx.mergeTrainMode });
         if (ctx.mergeTrainMode === "enforce") {

src/services/agent-approval-queue.ts

@@ -409,6 +409,8 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de
       installationPermissions: installation ? installation.permissions : null,
       mergeTrainMode: settings.mergeTrainMode,
       pullRequestCreatedAt: pr?.createdAt,
+      pullRequestLinkedIssues: pr?.linkedIssues,
+      pullRequestChangedFiles: pr?.changedFiles,
       // CI-run cancellation on a contributor_cap close (#2462): a contributor_cap close CAN be staged for
       // approval (close autonomy = auto_with_approval), so the accept-replay path needs this resolved the
       // same way the live webhook path does (src/queue/processors.ts) for the cancel hook to fire here too.