Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,15 @@ gate:
# PR, so it is opt-in rather than a new default.
premergeContentRecheck: false

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

# AI maintainer review. Opt-in; the AI capabilities are switched on at the
# deployment level.
aiReview:
Expand Down
6 changes: 6 additions & 0 deletions apps/gittensory-ui/public/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -8682,6 +8682,12 @@
},
"badgeEnabled": {
"type": "boolean"
},
"requireFreshRebaseWindowMinutes": {
"type": "integer",
"nullable": true,
"minimum": 0,
"exclusiveMinimum": true
}
},
"required": [
Expand Down
5 changes: 5 additions & 0 deletions migrations/0093_gate_require_fresh_rebase_window.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Force-rebase-before-merge gate (#2552): optional per-repo window (minutes). NULL (the default) means the
-- gate never forces a rebase -- byte-identical behavior for every existing row. When set, an agent-driven
-- merge whose base branch advanced within this window forces an update_branch + fresh CI recheck before
-- merging, instead of trusting a mergeable_state: clean read that may already be stale relative to the base.
ALTER TABLE repository_settings ADD COLUMN require_fresh_rebase_window_minutes INTEGER;
1 change: 1 addition & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,7 @@ const maintainerSettingsSchema = z
badgeEnabled: z.boolean(),
agentPaused: z.boolean(),
agentDryRun: z.boolean(),
requireFreshRebaseWindowMinutes: z.number().int().positive().nullable(),
commandAuthorization: z.object({
default: z.array(z.enum(["maintainer", "collaborator", "pr_author", "confirmed_miner"])).max(4).optional(),
commands: z.record(z.string().trim().min(1).max(64), z.array(z.enum(["maintainer", "collaborator", "pr_author", "confirmed_miner"])).max(4)).optional(),
Expand Down
5 changes: 5 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise
reviewNagCooldownDays: 5,
reviewNagLabel: "review-nag-cooldown",
autoCloseExemptLogins: [],
requireFreshRebaseWindowMinutes: null,
};
}
return {
Expand Down Expand Up @@ -562,6 +563,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise
reviewNagCooldownDays: normalizePositiveIntWithDefault(row.reviewNagCooldownDays, 5),
reviewNagLabel: row.reviewNagLabel,
autoCloseExemptLogins: parseAutoCloseExemptLogins(row.autoCloseExemptLoginsJson),
requireFreshRebaseWindowMinutes: normalizeOpenItemCap(row.requireFreshRebaseWindowMinutes),
createdAt: row.createdAt,
updatedAt: row.updatedAt,
};
Expand Down Expand Up @@ -646,6 +648,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
reviewNagCooldownDays: normalizePositiveIntWithDefault(settings.reviewNagCooldownDays, 5),
reviewNagLabel: settings.reviewNagLabel ?? "review-nag-cooldown",
autoCloseExemptLogins: normalizeAutoCloseExemptLogins(settings.autoCloseExemptLogins).logins,
requireFreshRebaseWindowMinutes: normalizeOpenItemCap(settings.requireFreshRebaseWindowMinutes),
};
const db = getDb(env.DB);
await db
Expand Down Expand Up @@ -700,6 +703,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
reviewNagCooldownDays: resolved.reviewNagCooldownDays,
reviewNagLabel: resolved.reviewNagLabel,
autoCloseExemptLoginsJson: jsonString(resolved.autoCloseExemptLogins),
requireFreshRebaseWindowMinutes: resolved.requireFreshRebaseWindowMinutes,
updatedAt: nowIso(),
})
.onConflictDoUpdate({
Expand Down Expand Up @@ -755,6 +759,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial<Repos
reviewNagCooldownDays: resolved.reviewNagCooldownDays,
reviewNagLabel: resolved.reviewNagLabel,
autoCloseExemptLoginsJson: jsonString(resolved.autoCloseExemptLogins),
requireFreshRebaseWindowMinutes: resolved.requireFreshRebaseWindowMinutes,
updatedAt: nowIso(),
},
});
Expand Down
3 changes: 3 additions & 0 deletions src/db/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ export const repositorySettings = sqliteTable("repository_settings", {
reviewNagLabel: text("review_nag_label").notNull().default("review-nag-cooldown"),
// Shared repo-scoped exemption list (#2463): a JSON array of GitHub logins.
autoCloseExemptLoginsJson: text("auto_close_exempt_logins_json").notNull().default("[]"),
// Force-rebase-before-merge window in minutes (#2552): null = never force (default). Enforcement lands in
// runAgentMaintenancePlanAndExecute, not here.
requireFreshRebaseWindowMinutes: integer("require_fresh_rebase_window_minutes"),
createdAt: text("created_at").notNull().$defaultFn(() => nowIso()),
updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()),
});
Expand Down
26 changes: 26 additions & 0 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2637,6 +2637,32 @@ export async function fetchLivePullRequestMergeState(
return result?.data.mergeable_state ?? undefined;
}

/**
* The base branch's LIVE tip-commit timestamp via REST `GET /commits/{ref}`, for the force-fresh-rebase gate
* (#2552): `mergeable_state` only detects git-level TEXTUAL conflicts, so a base that advanced with a new,
* non-conflicting sibling commit (e.g. a second PR's distinct-but-colliding migration file) still reads
* `clean`, letting a merge proceed on a decision that is stale relative to what just landed on the base.
* Comparing this timestamp against "now" lets the caller force a fresh rebase + CI recheck instead of
* trusting a `clean` read that predates the base's latest commit. Best-effort: a fetch error returns
* undefined (caller fails open — no forced rebase, same as today).
*/
export async function fetchLiveBaseBranchAdvancedAt(
env: Env,
repoFullName: string,
baseRef: string,
token: string | undefined,
admissionKey?: GitHubRateLimitAdmissionKey,
): Promise<string | undefined> {
const result = await githubJsonWithHeaders<{ commit?: { committer?: { date?: string | null } | null } | null }>(
env,
repoFullName,
`/commits/${encodeURIComponent(baseRef)}`,
token,
githubRateLimitOptions(admissionKey),
).catch(() => undefined);
return result?.data.commit?.committer?.date ?? undefined;
}

/** The PR's LIVE state ("open" / "closed") via REST `GET /pulls/{n}`. The stored open-PR cache lags GitHub, so a
* sibling closed/merged on GitHub can still read `open` locally; the duplicate-winner election (#dup-winner /
* audit #15) confirms a lower sibling's live state before treating this PR as a cluster loser. Best-effort:
Expand Down
1 change: 1 addition & 0 deletions src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ export const RepositorySettingsSchema = z
sizeGateMode: z.enum(["off", "advisory", "block"]).optional(),
gateDryRun: z.boolean().optional(),
premergeContentRecheck: z.boolean().optional(),
requireFreshRebaseWindowMinutes: z.number().int().positive().nullable().optional(),
mergeReadinessGateMode: z.enum(["off", "advisory", "block"]),
manifestPolicyGateMode: z.enum(["off", "advisory", "block"]),
selfAuthoredLinkedIssueGateMode: z.enum(["off", "advisory", "block"]),
Expand Down
143 changes: 143 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ import {
enqueueRepositoryOpenDataBackfill,
fetchAndStorePullRequestFilesForReview,
fetchLinkedIssueFacts,
fetchLiveBaseBranchAdvancedAt,
fetchLiveCiAggregatePreferGraphQl,
type LiveCiAggregate,
fetchLiveIssueState,
Expand Down Expand Up @@ -2069,6 +2070,39 @@ async function runAgentMaintenancePlanAndExecute(
);
if (breakerOnPlan.length === 0) return;

// #2552 (gate review finding, round 2): force a fresh rebase + CI recheck when the base has advanced within
// the configured window, immediately before what would otherwise be an agent-driven merge — mergeable_state
// only detects git-level TEXTUAL conflicts, so a base that advanced with a new, non-conflicting sibling
// commit (e.g. a second PR's distinct-but-colliding migration file) still reads `clean`, on a decision that
// predates the base's latest commit. Deliberately placed AFTER the full plan (gate/CI/blockers/breakers) is
// resolved, not on the raw mergeableState alone: the original placement ran this unconditionally whenever
// mergeableState was clean, so a PR sitting on red CI or a gate blocker (still git-clean) could burn the
// bounded retry cap on rebases nobody was about to act on, exhausting it before the PR was ever actually
// merge-eligible. Only fires when the resolved plan contains a merge THAT WOULD EXECUTE NOW (requiresApproval
// stages for a human, not an immediate merge). A forced rebase's resulting `synchronize` webhook re-triggers
// a fresh evaluation on the new head, so this pass stops here rather than executing against stale inputs.
const requireFreshRebaseWindowMinutes = settings.requireFreshRebaseWindowMinutes;
const planHasImminentMerge = breakerOnPlan.some((action) => action.actionClass === "merge" && !action.requiresApproval);
if (
typeof requireFreshRebaseWindowMinutes === "number" &&
baseRef &&
planHasImminentMerge &&
(liveMergeState ?? pr.mergeableState) === "clean" &&
(await maybeForceFreshRebase(env, {
installationId,
repoFullName,
pr,
settings,
windowMinutes: requireFreshRebaseWindowMinutes,
baseRef,
token,
admissionKey,
deliveryId,
}))
) {
return;
}

const installation = await getInstallation(env, installationId);
/* v8 ignore next -- an installed-App PR webhook always carries an installation record; the null is defensive. */
const installationPermissions = installation?.permissions ?? null;
Expand Down Expand Up @@ -2545,6 +2579,115 @@ async function ciPendingDeferStuck(
}
}

// #2552: bounded-retry cap for the force-fresh-rebase gate — without this, a fast-moving base could keep the
// freshness window perpetually "hot" and never let the PR clear to a real merge. Past the cap, the gate falls
// through to a normal merge decision (with an audit trail) rather than holding the PR hostage to base
// velocity. Deliberately keyed by PR NUMBER ONLY, NOT head SHA (gate review finding on the first version of
// this PR): a SUCCESSFUL forced update_branch itself produces a NEW head SHA, so a headSha-keyed counter would
// mint a fresh key — and reset to attempt 0 — on every single successful force, making the cap unreachable via
// the exact path it exists to bound. A 24h TTL on the stored counter still gives an eventual fresh start.
const MAX_FRESH_REBASE_FORCES = 3;
function freshRebaseForceCountKey(repoFullName: string, prNumber: number): string {
return `fresh-rebase-forced:${repoFullName.toLowerCase()}#${prNumber}`;
}

/**
* #2552: when the repo has opted into `gate.requireFreshRebaseWindow` and the base branch's live tip commit
* landed within that window of NOW, force an `update_branch` (merges base into head, re-triggering CI on the
* rebased result — the SAME action class/write-permission/dry-run/kill-switch stack `prReadyForReview`'s
* BEHIND-branch path already uses, not a new one) immediately before what would otherwise be a merge, instead
* of trusting a `mergeable_state: clean` read that predates the base's latest commit. Returns true when it
* forced the rebase (the caller stops this pass — the resulting `synchronize` webhook re-triggers a fresh
* evaluation on the new head); false when the freshness check doesn't apply, the cap was already reached, or
* the forced action itself couldn't complete (not authorized / dry-run / transient failure) — in every false
* case the caller falls through to the normal merge decision, so this gate fails open to today's behavior.
*/
async function maybeForceFreshRebase(
env: Env,
args: {
installationId: number;
repoFullName: string;
pr: PullRequestRecord;
settings: RepositorySettings;
// Narrowed by the caller (typeof settings.requireFreshRebaseWindowMinutes === "number") -- re-deriving and
// re-checking the same nullable field here would just be an unreachable duplicate of that guard.
windowMinutes: number;
baseRef: string;
token: string | undefined;
admissionKey: GitHubRateLimitAdmissionKey | undefined;
deliveryId: string;
},
): Promise<boolean> {
const { installationId, repoFullName, pr, settings, windowMinutes, baseRef, token, admissionKey, deliveryId } = args;
/* v8 ignore next -- structurally unreachable: the caller only invokes this after confirming
* (liveMergeState ?? pr.mergeableState) === "clean", which GitHub can never compute for a PR with no
* head commit; the null check is belt-and-suspenders against the field's optional TS type. */
if (!pr.headSha) return false;
const advancedAt = await fetchLiveBaseBranchAdvancedAt(env, repoFullName, baseRef, token, admissionKey);
if (!advancedAt) return false; // fail-open: unreadable base commit -> no forced rebase
const advancedAtMs = Date.parse(advancedAt);
if (!Number.isFinite(advancedAtMs) || Date.now() - advancedAtMs >= windowMinutes * 60_000) return false;

const countKey = freshRebaseForceCountKey(repoFullName, pr.number);
const storedCount = Number(await getTransientKey(env, countKey));
const attempt = Number.isFinite(storedCount) && storedCount > 0 ? storedCount : 0;
if (attempt >= MAX_FRESH_REBASE_FORCES) {
await recordAuditEvent(env, {
eventType: "agent.action.fresh_rebase_window_cap_exceeded",
actor: "gittensory",
targetKey: `${repoFullName}#${pr.number}`,
outcome: "completed",
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`,
metadata: { deliveryId, repoFullName, headSha: pr.headSha, windowMinutes },
}).catch(
/* v8 ignore next -- fail-safe: an audit write failure never blocks the caller's fallthrough */
() => undefined,
);
return false;
}

const autonomyLevel = resolveAutonomy(settings.autonomy, "update_branch");
const installation = await getInstallation(env, installationId);
const [outcome] = await executeAgentMaintenanceActions(
env,
{
installationId,
repoFullName,
pullNumber: pr.number,
headSha: pr.headSha,
autonomy: settings.autonomy,
agentPaused: settings.agentPaused,
agentDryRun: settings.agentDryRun,
/* 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). */
installationPermissions: installation?.permissions ?? null,
authorLogin: pr.authorLogin,
},
[
{
actionClass: "update_branch",
requiresApproval: autonomyRequiresApproval(autonomyLevel),
reason: `base branch advanced within the ${windowMinutes}m freshness window; forcing a fresh rebase + CI recheck before merge`,
expectedHeadSha: pr.headSha,
},
],
);
if (outcome?.outcome !== "completed") return false;
const nextAttempt = attempt + 1;
await putTransientKey(env, countKey, String(nextAttempt), 24 * 3600);
await recordAuditEvent(env, {
eventType: "agent.action.forced_rebase_freshness",
actor: "gittensory",
targetKey: `${repoFullName}#${pr.number}`,
outcome: "completed",
detail: `forced update_branch (attempt ${nextAttempt}/${MAX_FRESH_REBASE_FORCES}) — base advanced within the ${windowMinutes}m freshness window`,
metadata: { deliveryId, repoFullName, headSha: pr.headSha, windowMinutes, attempt: nextAttempt },
}).catch(
/* v8 ignore next -- fail-safe: an audit write failure never blocks the caller */
() => undefined,
);
return true;
}

// One CI run fires MANY check_run (one per job) + check_suite completions. Re-reviewing on every one storms the
// PR with duplicate reviews (and races the request_changes/approve dedup). reviewbot's CI_COALESCE_WINDOW parity:
// re-review a given PR at most once per this window. The re-review always re-fetches the LIVE CI, so the window
Expand Down
Loading
Loading