Skip to content

Commit 7d1014e

Browse files
authored
feat(gate): force a fresh rebase + CI recheck immediately before merge when base has advanced (#2616)
* fix(gate): key the bounded-retry counter by PR, not head SHA A successful forced update_branch produces a new head SHA, which reset a headSha-keyed counter back to attempt 0 on every force — making the MAX_FRESH_REBASE_FORCES cap unreachable via the exact path it exists to bound. Key by repo+PR number only; a 24h TTL still gives an eventual fresh start. * fix(gate): gate forced rebase on an imminent merge, wire settings PATCH Two issues found by the gate's own AI review on PR #2616: 1. The freshness check ran before planAgentMaintenanceActions and only checked mergeableState === "clean" (a git-level textual-conflict signal). A PR with pending CI, a gate blocker, or missing approval could still be git-clean, so it could burn the bounded retry cap on rebases nobody was about to act on -- exhausting it before the PR was ever actually merge-eligible. Moved the check to run after the full plan (gate/CI/blockers/precision breakers) resolves, and gated it on the plan containing a merge action that would execute now (requiresApproval stages for a human, not an immediate merge). 2. maintainerSettingsSchema in src/api/routes.ts omitted requireFreshRebaseWindowMinutes, so a dashboard/API PUT for the new setting was silently stripped by validation and never reached upsertRepositorySettings despite the DB/repository/OpenAPI wiring being complete. Added the field plus a round-trip test.
1 parent 074b888 commit 7d1014e

15 files changed

Lines changed: 468 additions & 3 deletions

.gittensory.yml.example

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,15 @@ gate:
183183
# PR, so it is opt-in rather than a new default.
184184
premergeContentRecheck: false
185185

186+
# Force-rebase-before-merge window in minutes (#2552, anti-race). When the base branch has advanced within
187+
# this many minutes of the actual merge-decision moment, an agent-driven merge forces an update_branch +
188+
# fresh CI recheck cycle before merging, instead of trusting a mergeable_state: clean read that may already
189+
# be stale relative to a sibling commit that just landed on the base. A bounded retry cap prevents a
190+
# fast-moving base from live-locking the PR — after a few forced attempts it falls through to a normal
191+
# merge with an audit note. Positive integer (minutes), or omit/null. Default: null (never force).
192+
# DB-backed (dashboard-settable too); this overrides the stored value.
193+
requireFreshRebaseWindow: 10
194+
186195
# AI maintainer review. Opt-in; the AI capabilities are switched on at the
187196
# deployment level.
188197
aiReview:

apps/gittensory-ui/public/openapi.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8682,6 +8682,12 @@
86828682
},
86838683
"badgeEnabled": {
86848684
"type": "boolean"
8685+
},
8686+
"requireFreshRebaseWindowMinutes": {
8687+
"type": "integer",
8688+
"nullable": true,
8689+
"minimum": 0,
8690+
"exclusiveMinimum": true
86858691
}
86868692
},
86878693
"required": [
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- Force-rebase-before-merge gate (#2552): optional per-repo window (minutes). NULL (the default) means the
2+
-- gate never forces a rebase -- byte-identical behavior for every existing row. When set, an agent-driven
3+
-- merge whose base branch advanced within this window forces an update_branch + fresh CI recheck before
4+
-- merging, instead of trusting a mergeable_state: clean read that may already be stale relative to the base.
5+
ALTER TABLE repository_settings ADD COLUMN require_fresh_rebase_window_minutes INTEGER;

src/api/routes.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,7 @@ const maintainerSettingsSchema = z
678678
badgeEnabled: z.boolean(),
679679
agentPaused: z.boolean(),
680680
agentDryRun: z.boolean(),
681+
requireFreshRebaseWindowMinutes: z.number().int().positive().nullable(),
681682
commandAuthorization: z.object({
682683
default: z.array(z.enum(["maintainer", "collaborator", "pr_author", "confirmed_miner"])).max(4).optional(),
683684
commands: z.record(z.string().trim().min(1).max(64), z.array(z.enum(["maintainer", "collaborator", "pr_author", "confirmed_miner"])).max(4)).optional(),

src/db/repositories.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise
510510
reviewNagCooldownDays: 5,
511511
reviewNagLabel: "review-nag-cooldown",
512512
autoCloseExemptLogins: [],
513+
requireFreshRebaseWindowMinutes: null,
513514
};
514515
}
515516
return {
@@ -562,6 +563,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise
562563
reviewNagCooldownDays: normalizePositiveIntWithDefault(row.reviewNagCooldownDays, 5),
563564
reviewNagLabel: row.reviewNagLabel,
564565
autoCloseExemptLogins: parseAutoCloseExemptLogins(row.autoCloseExemptLoginsJson),
566+
requireFreshRebaseWindowMinutes: normalizeOpenItemCap(row.requireFreshRebaseWindowMinutes),
565567
createdAt: row.createdAt,
566568
updatedAt: row.updatedAt,
567569
};
@@ -646,6 +648,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
646648
reviewNagCooldownDays: normalizePositiveIntWithDefault(settings.reviewNagCooldownDays, 5),
647649
reviewNagLabel: settings.reviewNagLabel ?? "review-nag-cooldown",
648650
autoCloseExemptLogins: normalizeAutoCloseExemptLogins(settings.autoCloseExemptLogins).logins,
651+
requireFreshRebaseWindowMinutes: normalizeOpenItemCap(settings.requireFreshRebaseWindowMinutes),
649652
};
650653
const db = getDb(env.DB);
651654
await db
@@ -700,6 +703,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
700703
reviewNagCooldownDays: resolved.reviewNagCooldownDays,
701704
reviewNagLabel: resolved.reviewNagLabel,
702705
autoCloseExemptLoginsJson: jsonString(resolved.autoCloseExemptLogins),
706+
requireFreshRebaseWindowMinutes: resolved.requireFreshRebaseWindowMinutes,
703707
updatedAt: nowIso(),
704708
})
705709
.onConflictDoUpdate({
@@ -755,6 +759,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
755759
reviewNagCooldownDays: resolved.reviewNagCooldownDays,
756760
reviewNagLabel: resolved.reviewNagLabel,
757761
autoCloseExemptLoginsJson: jsonString(resolved.autoCloseExemptLogins),
762+
requireFreshRebaseWindowMinutes: resolved.requireFreshRebaseWindowMinutes,
758763
updatedAt: nowIso(),
759764
},
760765
});

src/db/schema.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ export const repositorySettings = sqliteTable("repository_settings", {
9999
reviewNagLabel: text("review_nag_label").notNull().default("review-nag-cooldown"),
100100
// Shared repo-scoped exemption list (#2463): a JSON array of GitHub logins.
101101
autoCloseExemptLoginsJson: text("auto_close_exempt_logins_json").notNull().default("[]"),
102+
// Force-rebase-before-merge window in minutes (#2552): null = never force (default). Enforcement lands in
103+
// runAgentMaintenancePlanAndExecute, not here.
104+
requireFreshRebaseWindowMinutes: integer("require_fresh_rebase_window_minutes"),
102105
createdAt: text("created_at").notNull().$defaultFn(() => nowIso()),
103106
updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()),
104107
});

src/github/backfill.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2637,6 +2637,32 @@ export async function fetchLivePullRequestMergeState(
26372637
return result?.data.mergeable_state ?? undefined;
26382638
}
26392639

2640+
/**
2641+
* The base branch's LIVE tip-commit timestamp via REST `GET /commits/{ref}`, for the force-fresh-rebase gate
2642+
* (#2552): `mergeable_state` only detects git-level TEXTUAL conflicts, so a base that advanced with a new,
2643+
* non-conflicting sibling commit (e.g. a second PR's distinct-but-colliding migration file) still reads
2644+
* `clean`, letting a merge proceed on a decision that is stale relative to what just landed on the base.
2645+
* Comparing this timestamp against "now" lets the caller force a fresh rebase + CI recheck instead of
2646+
* trusting a `clean` read that predates the base's latest commit. Best-effort: a fetch error returns
2647+
* undefined (caller fails open — no forced rebase, same as today).
2648+
*/
2649+
export async function fetchLiveBaseBranchAdvancedAt(
2650+
env: Env,
2651+
repoFullName: string,
2652+
baseRef: string,
2653+
token: string | undefined,
2654+
admissionKey?: GitHubRateLimitAdmissionKey,
2655+
): Promise<string | undefined> {
2656+
const result = await githubJsonWithHeaders<{ commit?: { committer?: { date?: string | null } | null } | null }>(
2657+
env,
2658+
repoFullName,
2659+
`/commits/${encodeURIComponent(baseRef)}`,
2660+
token,
2661+
githubRateLimitOptions(admissionKey),
2662+
).catch(() => undefined);
2663+
return result?.data.commit?.committer?.date ?? undefined;
2664+
}
2665+
26402666
/** The PR's LIVE state ("open" / "closed") via REST `GET /pulls/{n}`. The stored open-PR cache lags GitHub, so a
26412667
* sibling closed/merged on GitHub can still read `open` locally; the duplicate-winner election (#dup-winner /
26422668
* audit #15) confirms a lower sibling's live state before treating this PR as a cluster loser. Best-effort:

src/openapi/schemas.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,7 @@ export const RepositorySettingsSchema = z
598598
sizeGateMode: z.enum(["off", "advisory", "block"]).optional(),
599599
gateDryRun: z.boolean().optional(),
600600
premergeContentRecheck: z.boolean().optional(),
601+
requireFreshRebaseWindowMinutes: z.number().int().positive().nullable().optional(),
601602
mergeReadinessGateMode: z.enum(["off", "advisory", "block"]),
602603
manifestPolicyGateMode: z.enum(["off", "advisory", "block"]),
603604
selfAuthoredLinkedIssueGateMode: z.enum(["off", "advisory", "block"]),

src/queue/processors.ts

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ import {
7878
enqueueRepositoryOpenDataBackfill,
7979
fetchAndStorePullRequestFilesForReview,
8080
fetchLinkedIssueFacts,
81+
fetchLiveBaseBranchAdvancedAt,
8182
fetchLiveCiAggregatePreferGraphQl,
8283
type LiveCiAggregate,
8384
fetchLiveIssueState,
@@ -2069,6 +2070,39 @@ async function runAgentMaintenancePlanAndExecute(
20692070
);
20702071
if (breakerOnPlan.length === 0) return;
20712072

2073+
// #2552 (gate review finding, round 2): force a fresh rebase + CI recheck when the base has advanced within
2074+
// the configured window, immediately before what would otherwise be an agent-driven merge — mergeable_state
2075+
// only detects git-level TEXTUAL conflicts, so a base that advanced with a new, non-conflicting sibling
2076+
// commit (e.g. a second PR's distinct-but-colliding migration file) still reads `clean`, on a decision that
2077+
// predates the base's latest commit. Deliberately placed AFTER the full plan (gate/CI/blockers/breakers) is
2078+
// resolved, not on the raw mergeableState alone: the original placement ran this unconditionally whenever
2079+
// mergeableState was clean, so a PR sitting on red CI or a gate blocker (still git-clean) could burn the
2080+
// bounded retry cap on rebases nobody was about to act on, exhausting it before the PR was ever actually
2081+
// merge-eligible. Only fires when the resolved plan contains a merge THAT WOULD EXECUTE NOW (requiresApproval
2082+
// stages for a human, not an immediate merge). A forced rebase's resulting `synchronize` webhook re-triggers
2083+
// a fresh evaluation on the new head, so this pass stops here rather than executing against stale inputs.
2084+
const requireFreshRebaseWindowMinutes = settings.requireFreshRebaseWindowMinutes;
2085+
const planHasImminentMerge = breakerOnPlan.some((action) => action.actionClass === "merge" && !action.requiresApproval);
2086+
if (
2087+
typeof requireFreshRebaseWindowMinutes === "number" &&
2088+
baseRef &&
2089+
planHasImminentMerge &&
2090+
(liveMergeState ?? pr.mergeableState) === "clean" &&
2091+
(await maybeForceFreshRebase(env, {
2092+
installationId,
2093+
repoFullName,
2094+
pr,
2095+
settings,
2096+
windowMinutes: requireFreshRebaseWindowMinutes,
2097+
baseRef,
2098+
token,
2099+
admissionKey,
2100+
deliveryId,
2101+
}))
2102+
) {
2103+
return;
2104+
}
2105+
20722106
const installation = await getInstallation(env, installationId);
20732107
/* v8 ignore next -- an installed-App PR webhook always carries an installation record; the null is defensive. */
20742108
const installationPermissions = installation?.permissions ?? null;
@@ -2545,6 +2579,115 @@ async function ciPendingDeferStuck(
25452579
}
25462580
}
25472581

2582+
// #2552: bounded-retry cap for the force-fresh-rebase gate — without this, a fast-moving base could keep the
2583+
// freshness window perpetually "hot" and never let the PR clear to a real merge. Past the cap, the gate falls
2584+
// through to a normal merge decision (with an audit trail) rather than holding the PR hostage to base
2585+
// velocity. Deliberately keyed by PR NUMBER ONLY, NOT head SHA (gate review finding on the first version of
2586+
// this PR): a SUCCESSFUL forced update_branch itself produces a NEW head SHA, so a headSha-keyed counter would
2587+
// mint a fresh key — and reset to attempt 0 — on every single successful force, making the cap unreachable via
2588+
// the exact path it exists to bound. A 24h TTL on the stored counter still gives an eventual fresh start.
2589+
const MAX_FRESH_REBASE_FORCES = 3;
2590+
function freshRebaseForceCountKey(repoFullName: string, prNumber: number): string {
2591+
return `fresh-rebase-forced:${repoFullName.toLowerCase()}#${prNumber}`;
2592+
}
2593+
2594+
/**
2595+
* #2552: when the repo has opted into `gate.requireFreshRebaseWindow` and the base branch's live tip commit
2596+
* landed within that window of NOW, force an `update_branch` (merges base into head, re-triggering CI on the
2597+
* rebased result — the SAME action class/write-permission/dry-run/kill-switch stack `prReadyForReview`'s
2598+
* BEHIND-branch path already uses, not a new one) immediately before what would otherwise be a merge, instead
2599+
* of trusting a `mergeable_state: clean` read that predates the base's latest commit. Returns true when it
2600+
* forced the rebase (the caller stops this pass — the resulting `synchronize` webhook re-triggers a fresh
2601+
* evaluation on the new head); false when the freshness check doesn't apply, the cap was already reached, or
2602+
* the forced action itself couldn't complete (not authorized / dry-run / transient failure) — in every false
2603+
* case the caller falls through to the normal merge decision, so this gate fails open to today's behavior.
2604+
*/
2605+
async function maybeForceFreshRebase(
2606+
env: Env,
2607+
args: {
2608+
installationId: number;
2609+
repoFullName: string;
2610+
pr: PullRequestRecord;
2611+
settings: RepositorySettings;
2612+
// Narrowed by the caller (typeof settings.requireFreshRebaseWindowMinutes === "number") -- re-deriving and
2613+
// re-checking the same nullable field here would just be an unreachable duplicate of that guard.
2614+
windowMinutes: number;
2615+
baseRef: string;
2616+
token: string | undefined;
2617+
admissionKey: GitHubRateLimitAdmissionKey | undefined;
2618+
deliveryId: string;
2619+
},
2620+
): Promise<boolean> {
2621+
const { installationId, repoFullName, pr, settings, windowMinutes, baseRef, token, admissionKey, deliveryId } = args;
2622+
/* v8 ignore next -- structurally unreachable: the caller only invokes this after confirming
2623+
* (liveMergeState ?? pr.mergeableState) === "clean", which GitHub can never compute for a PR with no
2624+
* head commit; the null check is belt-and-suspenders against the field's optional TS type. */
2625+
if (!pr.headSha) return false;
2626+
const advancedAt = await fetchLiveBaseBranchAdvancedAt(env, repoFullName, baseRef, token, admissionKey);
2627+
if (!advancedAt) return false; // fail-open: unreadable base commit -> no forced rebase
2628+
const advancedAtMs = Date.parse(advancedAt);
2629+
if (!Number.isFinite(advancedAtMs) || Date.now() - advancedAtMs >= windowMinutes * 60_000) return false;
2630+
2631+
const countKey = freshRebaseForceCountKey(repoFullName, pr.number);
2632+
const storedCount = Number(await getTransientKey(env, countKey));
2633+
const attempt = Number.isFinite(storedCount) && storedCount > 0 ? storedCount : 0;
2634+
if (attempt >= MAX_FRESH_REBASE_FORCES) {
2635+
await recordAuditEvent(env, {
2636+
eventType: "agent.action.fresh_rebase_window_cap_exceeded",
2637+
actor: "gittensory",
2638+
targetKey: `${repoFullName}#${pr.number}`,
2639+
outcome: "completed",
2640+
detail: `base advanced within the ${windowMinutes}m freshness window, but the ${MAX_FRESH_REBASE_FORCES}-attempt forced-rebase cap was already reached for this PR — falling through to a normal merge decision`,
2641+
metadata: { deliveryId, repoFullName, headSha: pr.headSha, windowMinutes },
2642+
}).catch(
2643+
/* v8 ignore next -- fail-safe: an audit write failure never blocks the caller's fallthrough */
2644+
() => undefined,
2645+
);
2646+
return false;
2647+
}
2648+
2649+
const autonomyLevel = resolveAutonomy(settings.autonomy, "update_branch");
2650+
const installation = await getInstallation(env, installationId);
2651+
const [outcome] = await executeAgentMaintenanceActions(
2652+
env,
2653+
{
2654+
installationId,
2655+
repoFullName,
2656+
pullNumber: pr.number,
2657+
headSha: pr.headSha,
2658+
autonomy: settings.autonomy,
2659+
agentPaused: settings.agentPaused,
2660+
agentDryRun: settings.agentDryRun,
2661+
/* v8 ignore next -- an installed-App PR webhook always carries an installation record; the null is defensive (mirrors runAgentMaintenancePlanAndExecute's own identical merge-time read). */
2662+
installationPermissions: installation?.permissions ?? null,
2663+
authorLogin: pr.authorLogin,
2664+
},
2665+
[
2666+
{
2667+
actionClass: "update_branch",
2668+
requiresApproval: autonomyRequiresApproval(autonomyLevel),
2669+
reason: `base branch advanced within the ${windowMinutes}m freshness window; forcing a fresh rebase + CI recheck before merge`,
2670+
expectedHeadSha: pr.headSha,
2671+
},
2672+
],
2673+
);
2674+
if (outcome?.outcome !== "completed") return false;
2675+
const nextAttempt = attempt + 1;
2676+
await putTransientKey(env, countKey, String(nextAttempt), 24 * 3600);
2677+
await recordAuditEvent(env, {
2678+
eventType: "agent.action.forced_rebase_freshness",
2679+
actor: "gittensory",
2680+
targetKey: `${repoFullName}#${pr.number}`,
2681+
outcome: "completed",
2682+
detail: `forced update_branch (attempt ${nextAttempt}/${MAX_FRESH_REBASE_FORCES}) — base advanced within the ${windowMinutes}m freshness window`,
2683+
metadata: { deliveryId, repoFullName, headSha: pr.headSha, windowMinutes, attempt: nextAttempt },
2684+
}).catch(
2685+
/* v8 ignore next -- fail-safe: an audit write failure never blocks the caller */
2686+
() => undefined,
2687+
);
2688+
return true;
2689+
}
2690+
25482691
// One CI run fires MANY check_run (one per job) + check_suite completions. Re-reviewing on every one storms the
25492692
// PR with duplicate reviews (and races the request_changes/approve dedup). reviewbot's CI_COALESCE_WINDOW parity:
25502693
// re-review a given PR at most once per this window. The re-review always re-fetches the LIVE CI, so the window

0 commit comments

Comments
 (0)