From 2256b8f410a5f9eeb2be71eef873704adaafc058 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 2 Jul 2026 06:55:00 -0700 Subject: [PATCH 1/2] fix(gate): key the bounded-retry counter by PR, not head SHA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .gittensory.yml.example | 9 + apps/gittensory-ui/public/openapi.json | 6 + .../0093_gate_require_fresh_rebase_window.sql | 5 + src/db/repositories.ts | 5 + src/db/schema.ts | 3 + src/github/backfill.ts | 26 +++ src/openapi/schemas.ts | 1 + src/queue/processors.ts | 138 +++++++++++++ src/signals/focus-manifest.ts | 14 +- src/types.ts | 7 + test/unit/data-spine.test.ts | 10 + test/unit/focus-manifest.test.ts | 40 +++- test/unit/queue.test.ts | 192 ++++++++++++++++++ 13 files changed, 453 insertions(+), 3 deletions(-) create mode 100644 migrations/0093_gate_require_fresh_rebase_window.sql diff --git a/.gittensory.yml.example b/.gittensory.yml.example index da2c2b01b3..9906ef4255 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -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: diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index d56ea4c3ef..197086e44f 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -8682,6 +8682,12 @@ }, "badgeEnabled": { "type": "boolean" + }, + "requireFreshRebaseWindowMinutes": { + "type": "integer", + "nullable": true, + "minimum": 0, + "exclusiveMinimum": true } }, "required": [ diff --git a/migrations/0093_gate_require_fresh_rebase_window.sql b/migrations/0093_gate_require_fresh_rebase_window.sql new file mode 100644 index 0000000000..2e38d6ec18 --- /dev/null +++ b/migrations/0093_gate_require_fresh_rebase_window.sql @@ -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; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index ef4dd93a82..a5fee158ad 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -510,6 +510,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise reviewNagCooldownDays: 5, reviewNagLabel: "review-nag-cooldown", autoCloseExemptLogins: [], + requireFreshRebaseWindowMinutes: null, }; } return { @@ -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, }; @@ -646,6 +648,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial nowIso()), updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()), }); diff --git a/src/github/backfill.ts b/src/github/backfill.ts index c2e533b054..df40c6f30c 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -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 { + 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: diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index 877fccd559..d50d6f3ca7 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -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"]), diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 65cf515d63..dc4de40da6 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -78,6 +78,7 @@ import { enqueueRepositoryOpenDataBackfill, fetchAndStorePullRequestFilesForReview, fetchLinkedIssueFacts, + fetchLiveBaseBranchAdvancedAt, fetchLiveCiAggregatePreferGraphQl, type LiveCiAggregate, fetchLiveIssueState, @@ -1998,6 +1999,34 @@ async function runAgentMaintenancePlanAndExecute( } } + // #2552: 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. Only pays the extra live GitHub read when the repo opted in AND the PR is + // otherwise merge-mechanically-ready (mergeableClean) — nothing is gained checking a PR that isn't clean + // yet. A forced rebase's resulting `synchronize` webhook re-triggers a fresh evaluation on the new head, + // so this pass stops here rather than falling through to planAgentMaintenanceActions with stale inputs. + const requireFreshRebaseWindowMinutes = settings.requireFreshRebaseWindowMinutes; + if ( + typeof requireFreshRebaseWindowMinutes === "number" && + baseRef && + (liveMergeState ?? pr.mergeableState) === "clean" && + (await maybeForceFreshRebase(env, { + installationId, + repoFullName, + pr, + settings, + windowMinutes: requireFreshRebaseWindowMinutes, + baseRef, + token, + admissionKey, + deliveryId, + })) + ) { + return; + } + const planned = planAgentMaintenanceActions({ conclusion: gate.conclusion, blockerTitles: gate.blockers.map((blocker) => blocker.title), @@ -2545,6 +2574,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 { + 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 diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 0b57e9be5b..2c7da0579b 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -53,6 +53,13 @@ export type FocusManifestGateConfig = { * costs one extra, uncached GitHub Trees-API call for any PR that touches migrations/**, so it is opt-in * rather than a new default. */ premergeContentRecheck: boolean | null; + /** `gate.requireFreshRebaseWindow` (#2552, anti-race): minutes. When the base branch has advanced within + * this window of the actual merge-decision moment, an agent-driven merge forces an `update_branch` + + * fresh CI recheck cycle before merging, instead of trusting a `mergeableState: clean` read that may + * already be stale relative to a sibling commit that just landed on the base. null (unset) ⇒ never force + * (byte-identical to today) — a discrete positive-minutes count, not a score, so it is neither clamped + * nor rounded; an invalid value (fractional, non-positive, non-finite) is dropped with a warning. */ + requireFreshRebaseWindowMinutes: number | null; }; // The converged per-PR review features a self-host operator toggles PER-REPO under `features:` in the private @@ -302,6 +309,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = { dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, + requireFreshRebaseWindowMinutes: null, }; const EMPTY_FEATURES_CONFIG: FocusManifestFeaturesConfig = { @@ -521,6 +529,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu dryRun: normalizeOptionalBoolean(record.dryRun, "gate.dryRun", warnings), firstTimeContributorGrace: normalizeOptionalBoolean(record.firstTimeContributorGrace, "gate.firstTimeContributorGrace", warnings), premergeContentRecheck: normalizeOptionalBoolean(record.premergeContentRecheck, "gate.premergeContentRecheck", warnings), + requireFreshRebaseWindowMinutes: normalizeOptionalPositiveInteger(record.requireFreshRebaseWindow, "gate.requireFreshRebaseWindow", warnings), }; // #2266: the flag is parsed, clamped, and threaded end-to-end, but the gate evaluator never reads it — a // maintainer who sets it to true believing it softens a blocker for newcomers gets no such effect. Surface @@ -551,7 +560,8 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu gate.selfAuthoredLinkedIssue !== null || gate.dryRun !== null || gate.firstTimeContributorGrace !== null || - gate.premergeContentRecheck !== null; + gate.premergeContentRecheck !== null || + gate.requireFreshRebaseWindowMinutes !== null; return gate; } @@ -596,6 +606,7 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue { if (gate.dryRun !== null) out.dryRun = gate.dryRun; if (gate.firstTimeContributorGrace !== null) out.firstTimeContributorGrace = gate.firstTimeContributorGrace; if (gate.premergeContentRecheck !== null) out.premergeContentRecheck = gate.premergeContentRecheck; + if (gate.requireFreshRebaseWindowMinutes !== null) out.requireFreshRebaseWindow = gate.requireFreshRebaseWindowMinutes; return out; } @@ -1208,6 +1219,7 @@ export function resolveEffectiveSettings( if (gate.dryRun !== null) effective.gateDryRun = gate.dryRun; if (gate.firstTimeContributorGrace !== null) effective.firstTimeContributorGrace = gate.firstTimeContributorGrace; if (gate.premergeContentRecheck !== null) effective.premergeContentRecheck = gate.premergeContentRecheck; + if (gate.requireFreshRebaseWindowMinutes !== null) effective.requireFreshRebaseWindowMinutes = gate.requireFreshRebaseWindowMinutes; // The dashboard "Require linked issue" toggle must not silently diverge from gate blocking: when the // boolean is on but linkedIssueGateMode is still off, treat it as a block requirement (#797). if (effective.requireLinkedIssue && effective.linkedIssueGateMode === "off") { diff --git a/src/types.ts b/src/types.ts index 21a2007a58..b8b6e57b70 100644 --- a/src/types.ts +++ b/src/types.ts @@ -659,6 +659,13 @@ export type RepositorySettings = { * on top of the standing owner/admin/automation-bot exemption. Always populated by the DB layer (default * `[]`); optional so existing settings fixtures/callers need not be touched. */ autoCloseExemptLogins?: string[] | undefined; + /** Force-rebase-before-merge window in minutes (#2552, anti-race). When a 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 first, rather than trusting a `mergeableState: clean` read that may already be + * stale relative to a sibling commit that just landed on the base. `null`/undefined (default) = never + * force -- a `mergeable_state: clean` read is trusted exactly as it is today. Layered like every other + * settings field (`.gittensory.yml` `gate.requireFreshRebaseWindow` > DB > `null`). */ + requireFreshRebaseWindowMinutes?: number | null | undefined; /** Agent-layer autonomy dial (#773): per-action-class level. Always populated by the DB layer (default * `{}` = deny-by-default = "observe" for every class); optional so existing settings fixtures/callers * need not be touched. The single source the action layer (#778) reads via `resolveAutonomy`. */ diff --git a/test/unit/data-spine.test.ts b/test/unit/data-spine.test.ts index c9d5955f62..20c40c9e71 100644 --- a/test/unit/data-spine.test.ts +++ b/test/unit/data-spine.test.ts @@ -324,6 +324,16 @@ describe("data spine repositories", () => { expect((await getRepositorySettings(env, "owner/caprepo")).contributorCapLabel).toBe("spam-cap"); await upsertRepositorySettings(env, { repoFullName: "owner/caprepo", contributorCapLabel: "renamed-cap" }); expect((await getRepositorySettings(env, "owner/caprepo")).contributorCapLabel).toBe("renamed-cap"); // update persists + // #2552 force-rebase-before-merge window: no row and no override both default to null (never force). + expect((await getRepositorySettings(env, "missing/repo")).requireFreshRebaseWindowMinutes).toBeNull(); + expect((await getRepositorySettings(env, "owner/defaultpack")).requireFreshRebaseWindowMinutes).toBeNull(); + // Round-trips on insert and persists on update; a fractional/non-positive value drops to null. + await upsertRepositorySettings(env, { repoFullName: "owner/rebasewindowrepo", requireFreshRebaseWindowMinutes: 15 }); + expect((await getRepositorySettings(env, "owner/rebasewindowrepo")).requireFreshRebaseWindowMinutes).toBe(15); + await upsertRepositorySettings(env, { repoFullName: "owner/rebasewindowrepo", requireFreshRebaseWindowMinutes: 30 }); + expect((await getRepositorySettings(env, "owner/rebasewindowrepo")).requireFreshRebaseWindowMinutes).toBe(30); // update persists + await upsertRepositorySettings(env, { repoFullName: "owner/rebasewindowrepo", requireFreshRebaseWindowMinutes: 2.5 as never }); + expect((await getRepositorySettings(env, "owner/rebasewindowrepo")).requireFreshRebaseWindowMinutes).toBeNull(); // #2463 review-nag cooldown + shared exemption list: no row and no override both default to off/3/5/the // default label/empty exemption list. expect(await getRepositorySettings(env, "missing/repo")).toMatchObject({ diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index c885e3b959..a0a50ef9b8 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -180,6 +180,8 @@ describe("parseFocusManifestContent", () => { expect(manifest.gate.dryRun).toBe(false); expect(manifest.gate.selfAuthoredLinkedIssue).toBe("advisory"); expect(manifest.gate.aiReviewCloseConfidence).toBeNull(); + // #2552: requireFreshRebaseWindow also round-trips through the real parser. + expect(manifest.gate.requireFreshRebaseWindowMinutes).toBe(10); }); }); @@ -492,7 +494,7 @@ describe("compileFocusManifestPolicy", () => { issueDiscoveryPolicy: "neutral", maintainerNotes: [], publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], - gate: { present: false, enabled: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null }, + gate: { present: false, enabled: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null }, settings: {}, review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null }, @@ -800,7 +802,7 @@ describe("parseFocusManifest gate config", () => { // the block→advisory deprecation-downgrade behavior itself is covered separately below. const m = parseFocusManifest({ gate: { linkedIssue: "block", duplicates: "advisory", readiness: { mode: "advisory", minScore: 70 } } }); expect(m.present).toBe(true); - expect(m.gate).toEqual({ present: true, enabled: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null }); + expect(m.gate).toEqual({ present: true, enabled: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null }); }); it("parses gate.mergeReadiness + gate.firstTimeContributorGrace, round-trips them, and warns on bad values (#822)", () => { @@ -1832,3 +1834,37 @@ describe("gate.premergeContentRecheck live migration-collision recheck config (# expect(m.warnings.some((w) => /gate\.premergeContentRecheck/i.test(w))).toBe(true); }); }); + +describe("gate.requireFreshRebaseWindow force-rebase-before-merge config (#2552)", () => { + it("parses gate.requireFreshRebaseWindow, sets present, round-trips, and resolves into effective settings", () => { + const m = parseFocusManifest({ gate: { requireFreshRebaseWindow: 10 } }); + expect(m.gate.requireFreshRebaseWindowMinutes).toBe(10); + expect(m.gate.present).toBe(true); + expect(gateConfigToJson(m.gate)).toMatchObject({ requireFreshRebaseWindow: 10 }); + const eff = resolveEffectiveSettings({} as unknown as RepositorySettings, m); + expect(eff.requireFreshRebaseWindowMinutes).toBe(10); + }); + + it("defaults to unset/undefined when omitted — byte-identical to today", () => { + const m = parseFocusManifest({}); + expect(m.gate.requireFreshRebaseWindowMinutes).toBeNull(); + const eff = resolveEffectiveSettings({} as unknown as RepositorySettings, m); + expect(eff.requireFreshRebaseWindowMinutes).toBeUndefined(); + }); + + it("warns and drops a fractional/non-positive value rather than silently coercing it", () => { + const fractional = parseFocusManifest({ gate: { requireFreshRebaseWindow: 2.5 } }); + expect(fractional.gate.requireFreshRebaseWindowMinutes).toBeNull(); + expect(fractional.warnings.some((w) => /gate\.requireFreshRebaseWindow/i.test(w))).toBe(true); + + const nonPositive = parseFocusManifest({ gate: { requireFreshRebaseWindow: 0 } }); + expect(nonPositive.gate.requireFreshRebaseWindowMinutes).toBeNull(); + expect(nonPositive.warnings.some((w) => /gate\.requireFreshRebaseWindow/i.test(w))).toBe(true); + }); + + it("lets the DB value pass through when the manifest doesn't override it", () => { + const db = { requireFreshRebaseWindowMinutes: 15 } as unknown as RepositorySettings; + const eff = resolveEffectiveSettings(db, parseFocusManifest(null)); + expect(eff.requireFreshRebaseWindowMinutes).toBe(15); + }); +}); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 9368d78cba..3f4447b2a9 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -6124,6 +6124,198 @@ describe("queue processors", () => { }); }); + describe("force-fresh-rebase-before-merge gate (#2552)", () => { + // Full merge-eligible stub set (clean + green + approved), reused across scenarios — mirrors the #2550 + // migration-recheck fixture above. `baseAdvancedAt` stubs the NEW /commits/{baseRef} freshness read; + // `null` simulates an unreadable base commit (404). + function stubFreshRebaseFetch(prNumber: number, opts: { baseAdvancedAt: string | null; mergeableState?: string; headSha?: string }, seen: { merged: boolean; updateBranchCalls: number; baseCommitCalls: number }) { + const headSha = opts.headSha ?? "sha1"; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = (init?.method ?? "GET").toUpperCase(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/check-runs/") && method === "PATCH") return Response.json({ id: 901 }); + if (url.includes(`/pulls/${prNumber}/update-branch`) && method === "PUT") { + seen.updateBranchCalls += 1; + return Response.json({ message: "Updating pull request branch." }, { status: 202 }); + } + if (url.endsWith("/commits/main")) { + seen.baseCommitCalls += 1; + if (opts.baseAdvancedAt === null) return new Response("not found", { status: 404 }); + return Response.json({ commit: { committer: { date: opts.baseAdvancedAt } } }); + } + if (/\/pulls\/\d+(?:\?|$)/.test(url) && method === "GET" && !url.includes(`/pulls/${prNumber}/`)) { + return Response.json({ number: prNumber, state: "open", user: { login: "contributor" }, head: { sha: headSha }, base: { ref: "main", sha: "base" }, mergeable_state: opts.mergeableState ?? "clean", labels: [] }); + } + if (url.includes(`/pulls/${prNumber}/files`)) return Response.json([{ filename: "src/index.ts", status: "modified", additions: 5, deletions: 1, changes: 6, patch: "@@\n+export const x = 1;" }]); + if (url.includes(`/commits/${headSha}/check-runs`)) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes(`/commits/${headSha}/status`)) return Response.json({ state: "success", statuses: [{ context: "ci/build", state: "success", description: "ok" }] }); + if (url.includes(`/commits/${headSha}/check-suites`)) return Response.json({ check_suites: [] }); + if (url.includes("/branches/")) return Response.json({ contexts: [] }); + if (url === "https://api.github.com/graphql") return Response.json({ data: { repository: { pullRequest: { reviewDecision: "APPROVED" } } } }); + if (url.includes(`/pulls/${prNumber}/merge`) && method === "PUT") { + seen.merged = true; + return Response.json({ merged: true, sha: "merged-sha1" }); + } + if (url.includes(`/issues/${prNumber}/labels`) && method === "GET") return Response.json([]); + if (url.includes(`/issues/${prNumber}/labels`) && method === "POST") return Response.json([]); + if (url.endsWith("/labels") && method === "POST") return Response.json({ name: "x" }, { status: 201 }); + if (url.includes(`/issues/${prNumber}/comments`)) return Response.json([]); + if (url.includes("/check-runs") && method === "POST") return Response.json({ id: 901 }, { status: 201 }); + return Response.json({}); + }); + } + + async function seedFreshRebaseRepo(env: Env, prNumber: number, opts: { requireFreshRebaseWindowMinutes?: number | null; autonomy?: Record } = {}) { + await upsertInstallation(env, { + installation: { id: 123, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { pull_requests: "write", issues: "write" }, events: [] }, + }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "owner/repo", + autonomy: opts.autonomy ?? { merge: "auto", update_branch: "auto", label: "auto" }, + autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, + aiReviewMode: "off", + gatePack: "oss-anti-slop", + gateCheckMode: "enabled", + checkRunMode: "off", + commentMode: "off", + publicSurface: "off", + ...(opts.requireFreshRebaseWindowMinutes !== undefined ? { requireFreshRebaseWindowMinutes: opts.requireFreshRebaseWindowMinutes } : {}), + }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: prNumber, title: "Fresh rebase PR", state: "open", user: { login: "contributor" }, head: { sha: "sha1" }, base: { ref: "main" }, labels: [], body: "" }); + } + + it("merges normally when the base has not advanced recently", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedFreshRebaseRepo(env, 90, { requireFreshRebaseWindowMinutes: 10 }); + const seen = { merged: false, updateBranchCalls: 0, baseCommitCalls: 0 }; + stubFreshRebaseFetch(90, { baseAdvancedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, seen); // 1h ago, outside a 10m window + + await processJob(env, { type: "agent-regate-pr", deliveryId: "fresh-rebase-old-base", repoFullName: "owner/repo", prNumber: 90, installationId: 123 }); + + expect(seen.baseCommitCalls).toBe(1); + expect(seen.updateBranchCalls).toBe(0); + expect(seen.merged).toBe(true); + const merge = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.action.merge").first<{ outcome: string }>(); + expect(merge?.outcome).toBe("completed"); + }); + + it("forces update_branch instead of merging when the base advanced within the freshness window", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedFreshRebaseRepo(env, 91, { requireFreshRebaseWindowMinutes: 10 }); + const seen = { merged: false, updateBranchCalls: 0, baseCommitCalls: 0 }; + stubFreshRebaseFetch(91, { baseAdvancedAt: new Date(Date.now() - 60_000).toISOString() }, seen); // 1 minute ago, within a 10m window + + await processJob(env, { type: "agent-regate-pr", deliveryId: "fresh-rebase-forced", repoFullName: "owner/repo", prNumber: 91, installationId: 123 }); + + expect(seen.updateBranchCalls).toBe(1); + expect(seen.merged).toBe(false); + const ub = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.action.update_branch").first<{ outcome: string }>(); + expect(ub?.outcome).toBe("completed"); + const forced = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.action.forced_rebase_freshness").first<{ outcome: string }>(); + expect(forced?.outcome).toBe("completed"); + const merge = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?").bind("agent.action.merge").first<{ n: number }>(); + expect(merge?.n).toBe(0); + }); + + it("never fetches the base commit or forces a rebase when the setting is unset (off by default)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedFreshRebaseRepo(env, 92); // requireFreshRebaseWindowMinutes left unset + const seen = { merged: false, updateBranchCalls: 0, baseCommitCalls: 0 }; + stubFreshRebaseFetch(92, { baseAdvancedAt: new Date().toISOString() }, seen); // "now" — would force if the setting were on + + await processJob(env, { type: "agent-regate-pr", deliveryId: "fresh-rebase-off", repoFullName: "owner/repo", prNumber: 92, installationId: 123 }); + + expect(seen.baseCommitCalls).toBe(0); + expect(seen.updateBranchCalls).toBe(0); + expect(seen.merged).toBe(true); + }); + + it("falls through to a normal merge once the bounded-retry cap is reached, with a cap-exceeded audit event", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedFreshRebaseRepo(env, 93, { requireFreshRebaseWindowMinutes: 10 }); + // Seed the bounded-retry counter at the cap (3) for this exact head SHA, matching what 3 prior forced + // attempts would have left behind. + await env.SELFHOST_TRANSIENT_CACHE?.set("fresh-rebase-forced:owner/repo#93", "3", 24 * 3600); + const seen = { merged: false, updateBranchCalls: 0, baseCommitCalls: 0 }; + stubFreshRebaseFetch(93, { baseAdvancedAt: new Date(Date.now() - 60_000).toISOString() }, seen); // still within window + + await processJob(env, { type: "agent-regate-pr", deliveryId: "fresh-rebase-capped", repoFullName: "owner/repo", prNumber: 93, installationId: 123 }); + + expect(seen.updateBranchCalls).toBe(0); // capped — never forces a 4th attempt + expect(seen.merged).toBe(true); // falls through to a normal merge instead + const capped = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.action.fresh_rebase_window_cap_exceeded").first<{ outcome: string }>(); + expect(capped?.outcome).toBe("completed"); + }); + + it("fails open (merges normally) when the base commit is unreadable", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedFreshRebaseRepo(env, 94, { requireFreshRebaseWindowMinutes: 10 }); + const seen = { merged: false, updateBranchCalls: 0, baseCommitCalls: 0 }; + stubFreshRebaseFetch(94, { baseAdvancedAt: null }, seen); // 404 on the base commit fetch + + await processJob(env, { type: "agent-regate-pr", deliveryId: "fresh-rebase-unreadable", repoFullName: "owner/repo", prNumber: 94, installationId: 123 }); + + expect(seen.baseCommitCalls).toBe(1); + expect(seen.updateBranchCalls).toBe(0); + expect(seen.merged).toBe(true); + }); + + it("falls through to a normal merge when the forced update_branch action itself is not authorized", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + // update_branch is deliberately absent from autonomy (resolves to the deny-by-default "observe" level), + // while merge stays "auto" — proving the freshness gate fails open independently of the eventual merge + // action's own authorization. + await seedFreshRebaseRepo(env, 95, { requireFreshRebaseWindowMinutes: 10, autonomy: { merge: "auto", label: "auto" } }); + const seen = { merged: false, updateBranchCalls: 0, baseCommitCalls: 0 }; + stubFreshRebaseFetch(95, { baseAdvancedAt: new Date(Date.now() - 60_000).toISOString() }, seen); // within window + + await processJob(env, { type: "agent-regate-pr", deliveryId: "fresh-rebase-not-authorized", repoFullName: "owner/repo", prNumber: 95, installationId: 123 }); + + expect(seen.baseCommitCalls).toBe(1); + expect(seen.updateBranchCalls).toBe(0); // denied by autonomy before any GitHub mutation is attempted + expect(seen.merged).toBe(true); // falls through to the normal merge decision + const denied = await env.DB.prepare("select outcome from audit_events where event_type = ? order by created_at desc limit 1").bind("agent.action.update_branch").first<{ outcome: string }>(); + expect(denied?.outcome).toBe("denied"); + }); + + it("REGRESSION (gate finding): the bounded-retry counter accumulates across successful forces even though each one changes the head SHA", async () => { + // A successful update_branch itself produces a NEW head SHA (the merge-base-into-head commit). The + // counter must NOT reset just because ITS OWN action changed the head -- otherwise the cap could never + // be reached via the exact path it exists to bound, and a fast-moving base would force a rebase on + // EVERY pass forever. Simulates 3 rounds, each with a genuinely different head SHA (mirroring the + // synchronize webhook a real update_branch triggers), then a 4th round proving the cap holds. + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedFreshRebaseRepo(env, 96, { requireFreshRebaseWindowMinutes: 10 }); + const shas = ["sha-r1", "sha-r2", "sha-r3", "sha-r4"]; + + for (const [round, sha] of shas.entries()) { + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 96, title: "Fresh rebase PR", state: "open", user: { login: "contributor" }, head: { sha }, base: { ref: "main" }, labels: [], body: "" }); + const seen = { merged: false, updateBranchCalls: 0, baseCommitCalls: 0 }; + stubFreshRebaseFetch(96, { baseAdvancedAt: new Date(Date.now() - 60_000).toISOString(), headSha: sha }, seen); // always within window + + await processJob(env, { type: "agent-regate-pr", deliveryId: `fresh-rebase-multi-round-${round}`, repoFullName: "owner/repo", prNumber: 96, installationId: 123 }); + + if (round < 3) { + // Rounds 0-2 (attempts 1-3): still under/at the cap -- forces update_branch, never merges. + expect(seen.updateBranchCalls).toBe(1); + expect(seen.merged).toBe(false); + } else { + // Round 3 (the 4th evaluation): the cap (3) was already reached by round 2 -- falls through to merge. + expect(seen.updateBranchCalls).toBe(0); + expect(seen.merged).toBe(true); + } + } + + const forcedCount = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?").bind("agent.action.forced_rebase_freshness").first<{ n: number }>(); + expect(forcedCount?.n).toBe(3); // exactly 3 successful forces, not 4 + const capped = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.action.fresh_rebase_window_cap_exceeded").first<{ outcome: string }>(); + expect(capped?.outcome).toBe("completed"); + }); + }); + it("contributor open-PR cap (#2270): a contributor's 3rd open PR (over a cap of 2) is labeled + closed deterministically with no merit merge", async () => { let aiCalls = 0; const env = createTestEnv({ From ba41ec668c057c970dfeb22dbbbc82926d7dbdea Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 2 Jul 2026 10:00:41 -0700 Subject: [PATCH 2/2] 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. --- src/api/routes.ts | 1 + src/queue/processors.ts | 61 +++++++++++++++++--------------- test/unit/queue.test.ts | 2 +- test/unit/routes-ai-byok.test.ts | 9 +++++ 4 files changed, 44 insertions(+), 29 deletions(-) diff --git a/src/api/routes.ts b/src/api/routes.ts index 6a82c37190..eb9cb34214 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -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(), diff --git a/src/queue/processors.ts b/src/queue/processors.ts index dc4de40da6..ffbb345d69 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -1999,34 +1999,6 @@ async function runAgentMaintenancePlanAndExecute( } } - // #2552: 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. Only pays the extra live GitHub read when the repo opted in AND the PR is - // otherwise merge-mechanically-ready (mergeableClean) — nothing is gained checking a PR that isn't clean - // yet. A forced rebase's resulting `synchronize` webhook re-triggers a fresh evaluation on the new head, - // so this pass stops here rather than falling through to planAgentMaintenanceActions with stale inputs. - const requireFreshRebaseWindowMinutes = settings.requireFreshRebaseWindowMinutes; - if ( - typeof requireFreshRebaseWindowMinutes === "number" && - baseRef && - (liveMergeState ?? pr.mergeableState) === "clean" && - (await maybeForceFreshRebase(env, { - installationId, - repoFullName, - pr, - settings, - windowMinutes: requireFreshRebaseWindowMinutes, - baseRef, - token, - admissionKey, - deliveryId, - })) - ) { - return; - } - const planned = planAgentMaintenanceActions({ conclusion: gate.conclusion, blockerTitles: gate.blockers.map((blocker) => blocker.title), @@ -2098,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; diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 3f4447b2a9..c5ccf7b9d2 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -6236,7 +6236,7 @@ describe("queue processors", () => { it("falls through to a normal merge once the bounded-retry cap is reached, with a cap-exceeded audit event", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await seedFreshRebaseRepo(env, 93, { requireFreshRebaseWindowMinutes: 10 }); - // Seed the bounded-retry counter at the cap (3) for this exact head SHA, matching what 3 prior forced + // Seed the bounded-retry counter at the cap (3) for this repo+PR, matching what 3 prior forced // attempts would have left behind. await env.SELFHOST_TRANSIENT_CACHE?.set("fresh-rebase-forced:owner/repo#93", "3", 24 * 3600); const seen = { merged: false, updateBranchCalls: 0, baseCommitCalls: 0 }; diff --git a/test/unit/routes-ai-byok.test.ts b/test/unit/routes-ai-byok.test.ts index bd144f8325..bc5615c744 100644 --- a/test/unit/routes-ai-byok.test.ts +++ b/test/unit/routes-ai-byok.test.ts @@ -105,6 +105,15 @@ describe("maintainer AI-review config route", () => { expect(settings.gateCheckMode).toBe("enabled"); }); + it("round-trips requireFreshRebaseWindowMinutes through the maintainer settings PUT route (#2552 gate finding)", async () => { + const app = createApp(); + const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET }); + const res = await app.request(`/v1/repos/${REPO}/settings`, { method: "PUT", headers: apiHeaders(env), body: JSON.stringify({ requireFreshRebaseWindowMinutes: 15 }) }, env); + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ requireFreshRebaseWindowMinutes: 15 }); + expect((await getRepositorySettings(env, REPO)).requireFreshRebaseWindowMinutes).toBe(15); + }); + it("lets the internal full settings route persist closeOwnerAuthors", async () => { const app = createApp(); const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET });