From 8893380b6aced19ed4559fdf26fb89eb9c60cf92 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:01:44 -0700 Subject: [PATCH 1/3] wip(review): merge-train settings plumbing checkpoint (schema+types+migration, executor wiring pending) --- migrations/0119_merge_train_mode.sql | 6 ++++++ src/db/schema.ts | 3 +++ src/types.ts | 8 ++++++++ 3 files changed, 17 insertions(+) create mode 100644 migrations/0119_merge_train_mode.sql diff --git a/migrations/0119_merge_train_mode.sql b/migrations/0119_merge_train_mode.sql new file mode 100644 index 0000000000..3164b4d9f7 --- /dev/null +++ b/migrations/0119_merge_train_mode.sql @@ -0,0 +1,6 @@ +-- Merge-train FIFO gate (#selfhost-merge-train). Off by default, same "opt-in, no surprise behavior +-- change" shape as review_evasion_protection: a PR merges the instant its own gate clears today, with +-- zero awareness of an older sibling PR still open in the same repo -- proven live to cause out-of-order +-- merges and the conflicts that follow. "audit" logs what the gate would hold without holding anything; +-- "enforce" actually defers a merge behind a still-viable older sibling. +ALTER TABLE repository_settings ADD COLUMN merge_train_mode TEXT NOT NULL DEFAULT 'off'; diff --git a/src/db/schema.ts b/src/db/schema.ts index cfd2adda9c..9f8b37a961 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -157,6 +157,9 @@ export const repositorySettings = sqliteTable("repository_settings", { reviewEvasionProtection: text("review_evasion_protection").notNull().default("off"), reviewEvasionLabel: text("review_evasion_label").notNull().default("review-evasion"), reviewEvasionComment: integer("review_evasion_comment", { mode: "boolean" }).notNull().default(true), + // Merge-train FIFO gate (#selfhost-merge-train): off by default, same "opt-in, no surprise behavior change" + // shape as reviewEvasionProtection above. + mergeTrainMode: text("merge_train_mode").notNull().default("off"), // Config-driven before/after screenshot-table gate (#2006): off by default. whenLabels/whenPaths are JSON // string arrays (mirrors contributorBlacklistJson's shape); screenshotTableGateMessage is nullable ("no // override" is a `.gittensory.yml`-only concept -- null here means "use the built-in default message"). diff --git a/src/types.ts b/src/types.ts index 2f4111dae1..24b6e0fa6e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1009,6 +1009,14 @@ export type RepositorySettings = { * re-closes as the App -- a close the contributor cannot themselves reopen (#one-shot-reopen) -- applies * the configured label/comment, and records a `review_evasion` moderation strike. */ reviewEvasionProtection?: "off" | "close" | undefined; + /** Merge-train FIFO 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 to cause + * out-of-order merges and the conflicts that follow. `"off"` (the default) is unchanged behavior. + * `"audit"` logs what the gate WOULD hold, without actually holding anything -- the safe way to validate + * the fix before enabling it for real. `"enforce"` actually defers a merge behind a still-viable older + * sibling, bounded by a staleness cap (see {@link "../review/merge-train"}) so one stuck old PR can never + * block newer ones forever. */ + mergeTrainMode?: "off" | "audit" | "enforce" | undefined; /** Review-evasion protection: label applied alongside the enforcement close, gated on `close` autonomy * like every other anti-abuse label (#label-scoping), mirroring {@link blacklistLabel}'s shape. `undefined` * ⇒ the `"review-evasion"` default; explicit `null` ⇒ close without any label. */ From 096ac76ef9fcda6d6f98335a660b18f034450e84 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:25:37 -0700 Subject: [PATCH 2/3] feat(review): merge-train FIFO gate for out-of-order merges A PR currently merges the instant its OWN gate clears, with zero awareness of an older sibling PR still open in the same repo -- a live production query proved this causes hundreds of out-of-order merges per repo (505/708/1364 across the three gated repos), and the conflicts that follow. Adds a new settings.mergeTrainMode ("off" | "audit" | "enforce", default "off") gate: "audit" logs what would be held without holding anything; "enforce" actually defers a merge behind a still-viable older sibling, bounded by a 24h staleness cap so one stuck PR can never wedge the repo's newer ones forever. A git-conflicted sibling never blocks -- it isn't about to merge, it's stuck. The decision itself (src/review/merge-train.ts) is a pure function reusing data already fetched on every merge pass (listOtherOpenPullRequests, already ordered ascending by PR number) -- no new query, no new persisted state beyond the one settings column. Wired into executeAgentMaintenanceActions right before the actual merge mutation, threaded through the two call sites that can plan a merge action. --- .gittensory.yml.example | 8 +++ apps/gittensory-ui/public/openapi.json | 8 +++ config/examples/gittensory.full.yml | 8 +++ src/db/repositories.ts | 6 ++ src/openapi/schemas.ts | 1 + src/queue/processors.ts | 2 + src/review/merge-train.ts | 62 +++++++++++++++++ src/selfhost/metrics.ts | 2 + src/services/agent-action-executor.ts | 40 +++++++++++ src/services/agent-approval-queue.ts | 2 + test/unit/agent-action-executor.test.ts | 81 +++++++++++++++++++++- test/unit/merge-train.test.ts | 91 +++++++++++++++++++++++++ 12 files changed, 310 insertions(+), 1 deletion(-) create mode 100644 src/review/merge-train.ts create mode 100644 test/unit/merge-train.test.ts diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 9aa5b9e435..08c4ffa618 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -779,6 +779,14 @@ settings: # # Default: review-evasion. # reviewEvasionComment: true # Post the public explanation comment before the enforcement close. Default: true. + # Merge-train FIFO 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 to cause out-of-order + # merges and the conflicts that follow. "audit" logs what the gate WOULD hold, without actually holding + # anything -- validate the fix before enabling it for real. "enforce" actually defers a merge behind a + # still-viable older sibling, bounded by a 24h staleness cap so one stuck old PR can never block newer ones + # forever. Off by default. + # mergeTrainMode: off # off | audit | enforce. Default: off. + # Linked-issue HARD-RULE auto-close (#linked-issue-hard-rules): a DETERMINISTIC verdict about the # linked issue itself (not an AI verdict), gated per-rule to block|off. Fires regardless of # `guardrailHit`; still respects the `close` autonomy class and the owner/automation exemption. Off diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index 1baedc354e..34d2a92fc7 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -9293,6 +9293,14 @@ }, "publicQualityMetrics": { "type": "boolean" + }, + "mergeTrainMode": { + "type": "string", + "enum": [ + "off", + "audit", + "enforce" + ] } }, "required": [ diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index 8497fd698c..a5b2fd8e9a 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -792,6 +792,14 @@ settings: # # Default: review-evasion. # reviewEvasionComment: true # Post the public explanation comment before the enforcement close. Default: true. + # Merge-train FIFO 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 to cause out-of-order + # merges and the conflicts that follow. "audit" logs what the gate WOULD hold, without actually holding + # anything -- validate the fix before enabling it for real. "enforce" actually defers a merge behind a + # still-viable older sibling, bounded by a 24h staleness cap so one stuck old PR can never block newer ones + # forever. Off by default. + # mergeTrainMode: off # off | audit | enforce. Default: off. + # Linked-issue HARD-RULE auto-close (#linked-issue-hard-rules): a DETERMINISTIC verdict about the # linked issue itself (not an AI verdict), gated per-rule to block|off. Fires regardless of # `guardrailHit`; still respects the `close` autonomy class and the owner/automation exemption. Off diff --git a/src/db/repositories.ts b/src/db/repositories.ts index cfc7131b83..93623eb0c5 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -645,6 +645,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise reviewEvasionProtection: normalizeReviewEvasionProtection(row.reviewEvasionProtection), reviewEvasionLabel: row.reviewEvasionLabel, reviewEvasionComment: row.reviewEvasionComment, + mergeTrainMode: normalizeMergeTrainMode(row.mergeTrainMode), screenshotTableGate: parseScreenshotTableGateRow(row), createdAt: row.createdAt, updatedAt: row.updatedAt, @@ -765,6 +766,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial { + const siblingCreatedMs = sibling.createdAt ? Date.parse(sibling.createdAt) : Number.NaN; + // Both sides need a real, distinct createdAt to compare by date -- if either is missing (or they tie + // exactly), fall back to the lower PR number as "older" (matches the duplicate-winner election's own + // tie-break precedent elsewhere in this codebase; PR numbers are assigned sequentially at creation, so + // this fallback is a safe, always-available proxy for open order). + if (Number.isFinite(siblingCreatedMs) && Number.isFinite(thisCreatedMs) && siblingCreatedMs !== thisCreatedMs) { + return siblingCreatedMs < thisCreatedMs; + } + return sibling.number < thisPrNumber; + }; + const viable = siblings + .filter((sibling) => sibling.number !== thisPrNumber) + .filter((sibling) => sibling.mergeableState !== "dirty") + .filter((sibling) => isOlder(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 + return nowMs - siblingCreatedMs < MERGE_TRAIN_MAX_WAIT_MS; + }) + .sort((a, b) => a.number - b.number); + const blocker = viable[0]; + return blocker ? { wait: true, blockingPr: blocker.number } : { wait: false }; +} diff --git a/src/selfhost/metrics.ts b/src/selfhost/metrics.ts index 2c46fbbebb..2a1551b460 100644 --- a/src/selfhost/metrics.ts +++ b/src/selfhost/metrics.ts @@ -127,6 +127,7 @@ const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [ ["gittensory_gate_decisions_total", { help: "Gate decisions by conclusion.", type: "counter" }], ["gittensory_precision_breaker_downgrades_total", { help: "Would-merge/would-close actions downgraded to a human hold by an accuracy circuit-breaker, by breaker direction.", type: "counter" }], ["gittensory_agent_disposition_total", { help: "Final agent disposition per PR pass (merge/close/hold), by repo, action class, blocker-code class, and autonomy level.", type: "counter" }], + ["gittensory_merge_train_deferred_total", { help: "Merge-train FIFO gate deferrals (an older still-viable sibling held a merge), by repo and mode (audit/enforce).", type: "counter" }], ["gittensory_reviews_published_total", { help: "Published review comments.", type: "counter" }], ["gittensory_github_branch_protection_permission_denied_total", { help: "GitHub branch-protection reads denied by permissions.", type: "counter" }], ["gittensory_github_pr_files_fetch_total", { help: "GitHub pull-request file fetch attempts.", type: "counter" }], @@ -154,6 +155,7 @@ const PRIVATE_REPO_LABEL_METRICS = new Set([ const ALWAYS_REDACT_REPO_LABEL_METRICS = new Set([ "gittensory_agent_disposition_total", "gittensory_queue_backlog_by_repo", + "gittensory_merge_train_deferred_total", ]); const redactedRepoLabels = new Map(); diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index f1330080d7..533981d4c5 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -6,6 +6,7 @@ import { getGlobalModerationConfig, insertNotificationDeliveryIfAbsent, isGlobalAgentFrozen, + listOtherOpenPullRequests, markPullRequestApproved, markPullRequestMergeBlocked, recordAuditEvent, @@ -35,6 +36,7 @@ import { type ModerationRuleType, } from "../settings/moderation-rules"; import { incr } from "../selfhost/metrics"; +import { shouldWaitForOlderSiblings } from "../review/merge-train"; import { captureError } from "../selfhost/sentry"; // The agent actor name on every audit record — the App acts on the maintainer's behalf per their configured @@ -179,6 +181,15 @@ export type AgentActionExecutionContext = { // custom label name is honored instead of only ever checking the literal default. `null` explicitly disables // the manual-review label (and this guard with it); absent/undefined uses the default AGENT_LABEL_NEEDS_REVIEW. manualReviewLabel?: string | null | undefined; + // Merge-train FIFO gate (#selfhost-merge-train), resolved by the CALLER (same "the executor has no settings + // access" shape as the fields above): "off" (default, unchanged behavior) | "audit" (log what would be held, + // never actually hold) | "enforce" (actually defer a merge behind a still-viable older sibling). Absent/ + // undefined behaves exactly like "off". + mergeTrainMode?: "off" | "audit" | "enforce" | undefined; + // This PR's own creation time, resolved by the CALLER (already has the PR record in scope) — the merge-train + // 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; }; export type ModerationContextSettings = { @@ -450,6 +461,35 @@ 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". + 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()); + if (decision.wait) { + incr("gittensory_merge_train_deferred_total", { repo: ctx.repoFullName, mode: ctx.mergeTrainMode }); + if (ctx.mergeTrainMode === "enforce") { + await audit("denied", `merge train: waiting for older mergeable sibling #${decision.blockingPr} — action not executed`); + continue; + } + // "audit" mode: record a SEPARATE, informational audit-trail entry (never through the shared `audit` + // closure above, which pushes into the SAME outcomes[] this function returns -- calling it here too + // would silently double the returned outcome count for this one action). The merge itself proceeds + // unaffected below. + await recordAuditEvent(env, { + eventType: "agent.action.merge_train_would_wait", + actor: "gittensory", + targetKey, + outcome: "denied", + detail: `merge train (audit mode): would wait for older mergeable sibling #${decision.blockingPr}`, + metadata: { repoFullName: ctx.repoFullName, pullNumber: ctx.pullNumber, blockingPr: decision.blockingPr }, + }).catch(() => undefined); + } + } // 9) live — perform the real mutation, recording success or the error. try { const detailOverride = await performAction(env, ctx, action); diff --git a/src/services/agent-approval-queue.ts b/src/services/agent-approval-queue.ts index e87b5a8f16..e3111f6881 100644 --- a/src/services/agent-approval-queue.ts +++ b/src/services/agent-approval-queue.ts @@ -407,6 +407,8 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de agentPaused: settings.agentPaused, agentDryRun: settings.agentDryRun, installationPermissions: installation ? installation.permissions : null, + mergeTrainMode: settings.mergeTrainMode, + pullRequestCreatedAt: pr?.createdAt, // 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. diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index 9ec557efff..0b45adf862 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("../../src/github/pr-actions", () => ({ createPullRequestReview: vi.fn(async () => ({ id: 1 })), @@ -1852,3 +1852,82 @@ describe("pendingClosureLabelApplied (#1136 Pass-2 trigger)", () => { expect(pendingClosureLabelApplied([approve2, labelAdd], [out("completed", "approve")])).toBe(false); }); }); + +describe("executeAgentMaintenanceActions merge-train gate (#selfhost-merge-train)", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(fetchPullRequestFreshness).mockImplementation(async (_env, args) => ({ + status: "current", + liveHeadSha: args.expectedHeadSha ?? null, + liveState: "open", + liveLabels: [], + })); + // Pin "now" alongside the fixtures' fixed 2026-07-0x createdAt values -- the gate's staleness cap + // (MERGE_TRAIN_MAX_WAIT_MS, 24h) compares against the REAL Date.now() in production code, so leaving the + // system clock unpinned would make these fixed dates drift stale (or not) purely based on whatever day + // the test suite happens to run on. + vi.setSystemTime(new Date("2026-07-05T12:00:00.000Z")); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("mergeTrainMode absent/\"off\" (default) merges immediately, ignoring an older open sibling", async () => { + const env = createTestEnv({}); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 3, title: "Older sibling", state: "open", user: { login: "c" }, head: { sha: "sha3" }, labels: [], body: "", created_at: "2026-07-05T08:00:00.000Z" }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "This PR", state: "open", user: { login: "c" }, head: { sha: "sha7" }, labels: [], body: "", created_at: "2026-07-05T10:00:00.000Z" }); + + const outcomes = await executeAgentMaintenanceActions(env, ctx({ pullRequestCreatedAt: "2026-07-05T10:00:00.000Z" }), [merge]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(mergePullRequest).toHaveBeenCalled(); + }); + + it("mergeTrainMode: \"enforce\" holds the merge behind a still-open OLDER sibling", async () => { + const env = createTestEnv({}); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 3, title: "Older sibling", state: "open", user: { login: "c" }, head: { sha: "sha3" }, labels: [], body: "", created_at: "2026-07-05T08:00:00.000Z" }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "This PR", state: "open", user: { login: "c" }, head: { sha: "sha7" }, labels: [], body: "", created_at: "2026-07-05T10:00:00.000Z" }); + + const outcomes = await executeAgentMaintenanceActions(env, ctx({ mergeTrainMode: "enforce", pullRequestCreatedAt: "2026-07-05T10:00:00.000Z" }), [merge]); + expect(outcomes[0]).toMatchObject({ actionClass: "merge", outcome: "denied" }); + expect(outcomes[0]?.detail).toContain("merge train"); + expect(outcomes[0]?.detail).toContain("#3"); + expect(mergePullRequest).not.toHaveBeenCalled(); + }); + + it("mergeTrainMode: \"enforce\" merges normally when no older sibling exists", async () => { + const env = createTestEnv({}); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 9, title: "Newer sibling", state: "open", user: { login: "c" }, head: { sha: "sha9" }, labels: [], body: "", created_at: "2026-07-05T11:00:00.000Z" }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "This PR", state: "open", user: { login: "c" }, head: { sha: "sha7" }, labels: [], body: "", created_at: "2026-07-05T10:00:00.000Z" }); + + const outcomes = await executeAgentMaintenanceActions(env, ctx({ mergeTrainMode: "enforce", pullRequestCreatedAt: "2026-07-05T10:00:00.000Z" }), [merge]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(mergePullRequest).toHaveBeenCalled(); + }); + + it("mergeTrainMode: \"audit\" records the would-hold decision but still executes the merge", async () => { + const env = createTestEnv({}); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 3, title: "Older sibling", state: "open", user: { login: "c" }, head: { sha: "sha3" }, labels: [], body: "", created_at: "2026-07-05T08:00:00.000Z" }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "This PR", state: "open", user: { login: "c" }, head: { sha: "sha7" }, labels: [], body: "", created_at: "2026-07-05T10:00:00.000Z" }); + + const outcomes = await executeAgentMaintenanceActions(env, ctx({ mergeTrainMode: "audit", pullRequestCreatedAt: "2026-07-05T10:00:00.000Z" }), [merge]); + // Exactly ONE outcome for the one planned action -- the audit-mode signal must not double it. + expect(outcomes).toHaveLength(1); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(mergePullRequest).toHaveBeenCalled(); + const wouldWaitRow = await env.DB.prepare("select outcome, detail from audit_events where event_type = ? order by created_at desc limit 1") + .bind("agent.action.merge_train_would_wait") + .first<{ outcome: string; detail: string }>(); + expect(wouldWaitRow?.detail).toContain("#3"); + }); + + it("a git-conflicted (\"dirty\") older sibling never blocks an enforce-mode merge", async () => { + const env = createTestEnv({}); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 3, title: "Conflicted older sibling", state: "open", user: { login: "c" }, head: { sha: "sha3" }, labels: [], body: "", created_at: "2026-07-05T08:00:00.000Z", mergeable_state: "dirty" }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "This PR", state: "open", user: { login: "c" }, head: { sha: "sha7" }, labels: [], body: "", created_at: "2026-07-05T10:00:00.000Z" }); + + const outcomes = await executeAgentMaintenanceActions(env, ctx({ mergeTrainMode: "enforce", pullRequestCreatedAt: "2026-07-05T10:00:00.000Z" }), [merge]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(mergePullRequest).toHaveBeenCalled(); + }); +}); diff --git a/test/unit/merge-train.test.ts b/test/unit/merge-train.test.ts new file mode 100644 index 0000000000..87a15e4555 --- /dev/null +++ b/test/unit/merge-train.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it } from "vitest"; +import { MERGE_TRAIN_MAX_WAIT_MS, shouldWaitForOlderSiblings, type MergeTrainSibling } from "../../src/review/merge-train"; + +const NOW = Date.parse("2026-07-07T12:00:00.000Z"); +const sibling = (number: number, createdAt: string | null | undefined, mergeableState?: string | null): MergeTrainSibling => ({ + number, + createdAt, + mergeableState, +}); + +describe("shouldWaitForOlderSiblings (#selfhost-merge-train)", () => { + it("waits for a genuinely older, viable sibling (by createdAt)", () => { + const siblings = [sibling(105, "2026-07-07T10:00:00.000Z")]; + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", siblings, NOW)).toEqual({ wait: true, blockingPr: 105 }); + }); + + it("does not wait for a NEWER sibling (by createdAt)", () => { + const siblings = [sibling(115, "2026-07-07T11:30:00.000Z")]; + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", siblings, NOW)).toEqual({ wait: false }); + }); + + it("does not wait when there are no other open siblings", () => { + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", [], NOW)).toEqual({ wait: false }); + }); + + it("never counts itself as its own blocking sibling", () => { + const siblings = [sibling(110, "2026-07-07T09:00:00.000Z")]; + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", siblings, NOW)).toEqual({ wait: false }); + }); + + it("a git-conflicted older sibling never blocks — it is stuck, not about to merge", () => { + const siblings = [sibling(105, "2026-07-07T10:00:00.000Z", "dirty")]; + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", siblings, NOW)).toEqual({ wait: false }); + }); + + it("a non-dirty mergeableState (clean/unknown/unstable) still blocks", () => { + const siblings = [sibling(105, "2026-07-07T10:00:00.000Z", "unstable")]; + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", siblings, NOW)).toEqual({ wait: true, blockingPr: 105 }); + }); + + it("the OLDEST of several viable older siblings is the blocker", () => { + const siblings = [sibling(107, "2026-07-07T10:30:00.000Z"), sibling(105, "2026-07-07T10:00:00.000Z"), sibling(108, "2026-07-07T10:45:00.000Z")]; + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", siblings, NOW)).toEqual({ wait: true, blockingPr: 105 }); + }); + + it("staleness cap: an older sibling past MERGE_TRAIN_MAX_WAIT_MS no longer blocks", () => { + const staleCreatedAt = new Date(NOW - MERGE_TRAIN_MAX_WAIT_MS - 1000).toISOString(); + const siblings = [sibling(105, staleCreatedAt)]; + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", siblings, NOW)).toEqual({ wait: false }); + }); + + it("staleness cap: an older sibling just under the cap still blocks", () => { + // thisPrCreatedAt is pinned at NOW (not the fixed 11:00:00 literal used elsewhere) so a sibling whose AGE + // is just under the cap is unambiguously older than this PR too, decoupling "is it stale" from "is it older". + const freshCreatedAt = new Date(NOW - MERGE_TRAIN_MAX_WAIT_MS + 1000).toISOString(); + const siblings = [sibling(105, freshCreatedAt)]; + expect(shouldWaitForOlderSiblings(110, new Date(NOW).toISOString(), siblings, NOW)).toEqual({ wait: true, blockingPr: 105 }); + }); + + it("missing createdAt on the sibling falls back to PR-number tiebreak (lower number = older)", () => { + const siblings = [sibling(105, null)]; + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", siblings, NOW)).toEqual({ wait: true, blockingPr: 105 }); + }); + + it("missing createdAt on the sibling + higher sibling number ⇒ does not block", () => { + const siblings = [sibling(115, undefined)]; + expect(shouldWaitForOlderSiblings(110, "2026-07-07T11:00:00.000Z", siblings, NOW)).toEqual({ wait: false }); + }); + + it("missing createdAt on THIS pr but sibling has one still falls back to PR-number tiebreak", () => { + const siblings = [sibling(115, "2026-07-07T09:00:00.000Z")]; + expect(shouldWaitForOlderSiblings(110, null, siblings, NOW)).toEqual({ wait: false }); + }); + + it("missing createdAt on both sides falls back to PR-number tiebreak", () => { + const siblings = [sibling(105, undefined)]; + expect(shouldWaitForOlderSiblings(110, undefined, siblings, NOW)).toEqual({ wait: true, blockingPr: 105 }); + }); + + it("an exact createdAt tie falls back to PR-number tiebreak", () => { + const tie = "2026-07-07T11:55:00.000Z"; // recent, well clear of the staleness boundary tested separately above + const siblings = [sibling(105, tie)]; + expect(shouldWaitForOlderSiblings(110, tie, siblings, NOW)).toEqual({ wait: true, blockingPr: 105 }); + }); + + it("an exact createdAt tie with a LOWER-numbered current PR does not block", () => { + const tie = "2026-07-07T11:55:00.000Z"; // recent, well clear of the staleness boundary tested separately above + const siblings = [sibling(115, tie)]; + expect(shouldWaitForOlderSiblings(110, tie, siblings, NOW)).toEqual({ wait: false }); + }); +}); From 81cc01e654083f5b0bc626922b5cc25aad032f51 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:52:30 -0700 Subject: [PATCH 3/3] fix(queue): renumber merge-train migration 0119 -> 0121 (collision) 0119 was claimed by another merged PR (ai_slop_cache), and 0120 by #4032, while this one was in flight. Renumbered to the next free slot on top of the current branch tip. --- .../{0119_merge_train_mode.sql => 0121_merge_train_mode.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename migrations/{0119_merge_train_mode.sql => 0121_merge_train_mode.sql} (100%) diff --git a/migrations/0119_merge_train_mode.sql b/migrations/0121_merge_train_mode.sql similarity index 100% rename from migrations/0119_merge_train_mode.sql rename to migrations/0121_merge_train_mode.sql