From 4c15eddf884a3c459493fb4ed89a06690106a492 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:08:00 -0700 Subject: [PATCH] fix(review): correct the stale-base grounding fact and close the mergeable_state blind spot it depends on The base-branch staleness fact shipped in #7670 anchored its compare-API read on a PR's own base.sha -- but GitHub keeps that field pointed at the live tip of the target branch as it moves, so comparing it against the current default branch would read ~0 regardless of how stale a PR's actual code is, making the fact effectively dead. Anchors on the PR's HEAD instead, which the compare API resolves via a true git merge-base, independent of that metadata timing. Also adds gate.staleBaseAheadByThreshold: when a repo opts in, the pre-review readiness gate (prReadyForReview) forces an update_branch once the default branch has advanced at least that many commits beyond a PR's head -- the same action the existing BEHIND-base path takes, but triggered by the same compare-API read rather than GitHub's own mergeable_state, which only ever reports "behind" when a repo's branch protection requires branches to be up to date before merging. A repo without that setting can have a PR genuinely dozens of commits behind and never see it auto-rebased before review otherwise. --- .loopover.yml.example | 10 + apps/loopover-ui/public/openapi.json | 6 + config/examples/loopover.full.yml | 10 + ...169_gate_stale_base_ahead_by_threshold.sql | 7 + .../loopover-engine/src/focus-manifest.ts | 17 ++ .../src/types/manifest-deps-types.ts | 7 + .../src/types/predicted-gate-types.ts | 1 + src/api/routes.ts | 1 + src/db/repositories.ts | 5 + src/db/schema.ts | 4 + src/github/backfill.ts | 27 ++- src/openapi/schemas.ts | 1 + src/queue/ai-review-orchestration.ts | 8 +- src/queue/processors.ts | 50 ++++- src/review/grounding-wire.ts | 20 +- src/review/review-grounding.ts | 9 +- src/signals/focus-manifest.ts | 1 + src/types.ts | 7 + test/unit/data-spine.test.ts | 10 + test/unit/focus-manifest.test.ts | 39 +++- test/unit/grounding-wiring.test.ts | 31 ++- test/unit/queue-4.test.ts | 194 +++++++++++++++++- test/unit/routes-ai-byok.test.ts | 9 + 23 files changed, 425 insertions(+), 49 deletions(-) create mode 100644 migrations/0169_gate_stale_base_ahead_by_threshold.sql diff --git a/.loopover.yml.example b/.loopover.yml.example index dfdc9428fe..736ddf271e 100644 --- a/.loopover.yml.example +++ b/.loopover.yml.example @@ -343,6 +343,16 @@ gate: # DB-backed (dashboard-settable too); this overrides the stored value. requireFreshRebaseWindow: 10 + # Stale-base auto-rebase threshold. When the repository's current default branch is at least this many + # commits ahead of a PR's own base commit, the pre-review readiness gate forces an update_branch before + # review runs — independent of GitHub's own mergeable_state "behind" signal, which only fires when the + # repo's branch protection requires branches to be up to date before merging (a repo without that setting + # can have a branch genuinely dozens of commits behind and never see it auto-rebased otherwise). Positive + # integer (commit count), or omit/null. Default: null (never force via this path). Costs one extra GitHub + # compare-API call per non-"behind" readiness check, so it is opt-in rather than a new default. + # DB-backed (dashboard-settable too); this overrides the stored value. + staleBaseAheadByThreshold: 10 + # AI maintainer review. Opt-in; the AI capabilities are switched on at the # deployment level. aiReview: diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index a8107d1810..4a8bfd6ef3 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -9739,6 +9739,12 @@ }, "issuePlanMilestoneReuse": { "type": "boolean" + }, + "staleBaseAheadByThreshold": { + "type": "integer", + "nullable": true, + "minimum": 0, + "exclusiveMinimum": true } }, "required": [ diff --git a/config/examples/loopover.full.yml b/config/examples/loopover.full.yml index 167e3e6fd0..9b17a3da5f 100644 --- a/config/examples/loopover.full.yml +++ b/config/examples/loopover.full.yml @@ -357,6 +357,16 @@ gate: # DB-backed (dashboard-settable too); this overrides the stored value. requireFreshRebaseWindow: 10 + # Stale-base auto-rebase threshold. When the repository's current default branch is at least this many + # commits ahead of a PR's own base commit, the pre-review readiness gate forces an update_branch before + # review runs — independent of GitHub's own mergeable_state "behind" signal, which only fires when the + # repo's branch protection requires branches to be up to date before merging (a repo without that setting + # can have a branch genuinely dozens of commits behind and never see it auto-rebased otherwise). Positive + # integer (commit count), or omit/null. Default: null (never force via this path). Costs one extra GitHub + # compare-API call per non-"behind" readiness check, so it is opt-in rather than a new default. + # DB-backed (dashboard-settable too); this overrides the stored value. + staleBaseAheadByThreshold: 10 + # AI maintainer review. Opt-in; the AI capabilities are switched on at the # deployment level. aiReview: diff --git a/migrations/0169_gate_stale_base_ahead_by_threshold.sql b/migrations/0169_gate_stale_base_ahead_by_threshold.sql new file mode 100644 index 0000000000..7281cbfaf9 --- /dev/null +++ b/migrations/0169_gate_stale_base_ahead_by_threshold.sql @@ -0,0 +1,7 @@ +-- Stale-base auto-rebase threshold (#review-grounding stale-base fact): optional per-repo commit-count +-- threshold. NULL (the default) means this path never forces a rebase -- byte-identical behavior for every +-- existing row. When set, the pre-review readiness gate forces an update_branch whenever the repo's current +-- default branch is at least this many commits ahead of a PR's own base commit, independent of GitHub's own +-- mergeable_state "behind" signal (which only fires when the repo's branch protection requires branches to be +-- up to date before merging). +ALTER TABLE repository_settings ADD COLUMN stale_base_ahead_by_threshold INTEGER; diff --git a/packages/loopover-engine/src/focus-manifest.ts b/packages/loopover-engine/src/focus-manifest.ts index c37cf47674..c4f64b6703 100644 --- a/packages/loopover-engine/src/focus-manifest.ts +++ b/packages/loopover-engine/src/focus-manifest.ts @@ -170,6 +170,19 @@ export type FocusManifestGateConfig = { * (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; + /** `gate.staleBaseAheadByThreshold` (#review-grounding stale-base fact): a commit count. When the repo's + * current default branch is at least this many commits ahead of a PR's own base commit, the pre-review + * readiness gate forces an `update_branch` (same action class as the existing `mergeableState: "behind"` + * path in `prReadyForReview`), independent of whether GitHub itself reports `mergeableState: "behind"` — + * that signal only ever fires when the repo's branch protection has "require branches up to date before + * merging" enabled, so a repo without that setting can have a branch genuinely dozens of commits behind + * and never see it auto-rebased before review otherwise. null (unset) ⇒ never force via this path + * (byte-identical to today) — this costs one extra GitHub compare-API call per non-"behind" readiness + * check, so it is opt-in rather than a new default, mirroring requireFreshRebaseWindowMinutes's own + * opt-in-for-cost rationale directly above. A discrete positive-commit count, not a score, so it is + * neither clamped nor rounded; an invalid value (fractional, non-positive, non-finite) is dropped with a + * warning, same validation as requireFreshRebaseWindowMinutes. */ + staleBaseAheadByThreshold: number | null; /** `gate.claMode` (#2564): off/advisory/block. null (unset) ⇒ off (byte-identical to today) — a repo must * explicitly opt in before any CLA consent check runs. */ claMode: GateRuleMode | null; @@ -1243,6 +1256,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = { dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, + staleBaseAheadByThreshold: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, @@ -1712,6 +1726,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu dryRun: normalizeOptionalBoolean(record.dryRun, "gate.dryRun", warnings), premergeContentRecheck: normalizeOptionalBoolean(record.premergeContentRecheck, "gate.premergeContentRecheck", warnings), requireFreshRebaseWindowMinutes: normalizeOptionalPositiveInteger(record.requireFreshRebaseWindow, "gate.requireFreshRebaseWindow", warnings), + staleBaseAheadByThreshold: normalizeOptionalPositiveInteger(record.staleBaseAheadByThreshold, "gate.staleBaseAheadByThreshold", warnings), claMode: normalizeOptionalGateMode(record.claMode, "gate.claMode", warnings), claConsentPhrase: parsePublicSafeText(claRecord?.consentPhrase, "gate.cla.consentPhrase", warnings), claCheckRunName: parsePublicSafeText(claRecord?.checkRunName, "gate.cla.checkRunName", warnings), @@ -1767,6 +1782,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu gate.dryRun !== null || gate.premergeContentRecheck !== null || gate.requireFreshRebaseWindowMinutes !== null || + gate.staleBaseAheadByThreshold !== null || gate.claMode !== null || gate.claConsentPhrase !== null || gate.claCheckRunName !== null || @@ -1848,6 +1864,7 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue { if (gate.dryRun !== null) out.dryRun = gate.dryRun; if (gate.premergeContentRecheck !== null) out.premergeContentRecheck = gate.premergeContentRecheck; if (gate.requireFreshRebaseWindowMinutes !== null) out.requireFreshRebaseWindow = gate.requireFreshRebaseWindowMinutes; + if (gate.staleBaseAheadByThreshold !== null) out.staleBaseAheadByThreshold = gate.staleBaseAheadByThreshold; if (gate.claMode !== null) out.claMode = gate.claMode; if (gate.claConsentPhrase !== null || gate.claCheckRunName !== null || gate.claCheckRunAppSlug !== null) { const cla: Record = {}; diff --git a/packages/loopover-engine/src/types/manifest-deps-types.ts b/packages/loopover-engine/src/types/manifest-deps-types.ts index aefb142e76..bde465395b 100644 --- a/packages/loopover-engine/src/types/manifest-deps-types.ts +++ b/packages/loopover-engine/src/types/manifest-deps-types.ts @@ -530,6 +530,13 @@ export type RepositorySettings = { * force -- a `mergeable_state: clean` read is trusted exactly as it is today. Layered like every other * settings field (`.loopover.yml` `gate.requireFreshRebaseWindow` > DB > `null`). */ requireFreshRebaseWindowMinutes?: number | null | undefined; + /** Stale-base auto-rebase threshold (#review-grounding stale-base fact): a commit count. When the repo's + * current default branch is at least this many commits ahead of a PR's own base commit, the pre-review + * readiness gate forces an `update_branch` before review, independent of GitHub's own `mergeableState: + * "behind"` signal (which only fires when the repo's branch protection requires branches to be up to date). + * `null`/undefined (default) = never force via this path (byte-identical to today). Layered like every + * other settings field (`.loopover.yml` `gate.staleBaseAheadByThreshold` > DB > `null`). */ + staleBaseAheadByThreshold?: number | null | undefined; /** Account-age throttle (#2561, anti-abuse): an account younger than this many days gets the * {@link newAccountLabel} and a tighter effective contributor cap — friction/visibility, NEVER an * automatic close on account age alone. `null`/undefined (default) = off. Never fires for the repo diff --git a/packages/loopover-engine/src/types/predicted-gate-types.ts b/packages/loopover-engine/src/types/predicted-gate-types.ts index a01d065e21..d3da3975c5 100644 --- a/packages/loopover-engine/src/types/predicted-gate-types.ts +++ b/packages/loopover-engine/src/types/predicted-gate-types.ts @@ -311,6 +311,7 @@ export type FocusManifestGateConfig = { dryRun: boolean | null; premergeContentRecheck: boolean | null; requireFreshRebaseWindowMinutes: number | null; + staleBaseAheadByThreshold: number | null; claMode: GateRuleMode | null; claConsentPhrase: string | null; claCheckRunName: string | null; diff --git a/src/api/routes.ts b/src/api/routes.ts index 25e973876a..6df0b5be06 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -891,6 +891,7 @@ const maintainerSettingsSchema = z agentPaused: z.boolean(), agentDryRun: z.boolean(), requireFreshRebaseWindowMinutes: z.number().int().positive().nullable(), + staleBaseAheadByThreshold: 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/db/repositories.ts b/src/db/repositories.ts index b05d7ed1f6..5b87923f95 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -634,6 +634,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise reviewNagMonitoredMentions: [], autoCloseExemptLogins: [], requireFreshRebaseWindowMinutes: null, + staleBaseAheadByThreshold: null, accountAgeThresholdDays: null, newAccountLabel: "new-account", commandRateLimitPolicy: "off", @@ -735,6 +736,7 @@ export async function getRepositorySettings(env: Env, fullName: string): Promise reviewNagMonitoredMentions: [], autoCloseExemptLogins: [], requireFreshRebaseWindowMinutes: normalizePositiveIntOrNull(row.requireFreshRebaseWindowMinutes), + staleBaseAheadByThreshold: normalizePositiveIntOrNull(row.staleBaseAheadByThreshold), accountAgeThresholdDays: null, newAccountLabel: "new-account", commandRateLimitPolicy: "off", @@ -873,6 +875,7 @@ export async function upsertRepositorySettings(env: Env, settings: Partial( env, repoFullName, - `/compare/${encodeURIComponent(baseSha)}...${encodeURIComponent(defaultBranchRef)}`, + `/compare/${encodeURIComponent(prHeadSha)}...${encodeURIComponent(defaultBranchRef)}`, token, githubRateLimitOptions(admissionKey), ).catch(() => undefined); diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index b43030db8e..3796b8ff3c 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -751,6 +751,7 @@ export const RepositorySettingsSchema = z gateDryRun: z.boolean().optional(), premergeContentRecheck: z.boolean().optional(), requireFreshRebaseWindowMinutes: z.number().int().positive().nullable().optional(), + staleBaseAheadByThreshold: 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/ai-review-orchestration.ts b/src/queue/ai-review-orchestration.ts index a3867e94c5..623482d989 100644 --- a/src/queue/ai-review-orchestration.ts +++ b/src/queue/ai-review-orchestration.ts @@ -547,10 +547,10 @@ export async function runAiReviewForAdvisory( args.pr.number, ), installationId: repo?.installationId ?? null, - // #review-grounding stale-base fact (metagraphed #7305-class incident): both are additive — either - // absent (no baseSha on a rare malformed webhook record, or an unregistered repo with no stored - // defaultBranch) just skips the BASE BRANCH STATUS fact, same as before it existed. - baseSha: args.pr.baseSha, + // #review-grounding stale-base fact (metagraphed #7305-class incident): additive — reuses the + // SAME headSha already passed above (the PR's real current code, not its base.sha, which GitHub + // keeps pointed at the live target-branch tip regardless of staleness). An unregistered repo with + // no stored defaultBranch just skips the BASE BRANCH STATUS fact, same as before it existed. defaultBranchRef: repo?.defaultBranch, }); })() diff --git a/src/queue/processors.ts b/src/queue/processors.ts index d4a853f5bc..7fff7bd795 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -99,6 +99,7 @@ import { import { backfillRepositorySegment, fetchAndStorePullRequestFilesForReview, + fetchBaseAheadBy, fetchLinkedIssueFacts, fetchLiveBaseBranchAdvancedAt, invalidateCiStateCache, @@ -3511,11 +3512,15 @@ async function prReadyForReview( )) ?? env.GITHUB_PUBLIC_TOKEN; if (!token) return true; const admissionKey = githubAdmissionKeyForToken(env, installationId, token); - // 1) rebase if BEHIND base — the synchronize on the new head re-triggers this flow on the merged result. The - // request-local facts may already be seeded from the sweep's resync payload, and the fallback live merge-state - // fetch fails open internally (swallows its own fetch errors → undefined). - const liveMergeState = await cachedLiveMergeState(env, repoFullName, liveFacts, pr.number, token, admissionKey); - if (liveMergeState === "behind") { + // Narrowed to a local const so it stays narrowed to `string` inside the forceUpdateBranch closure below -- + // TS's control-flow narrowing of the `!pr.headSha` guard above does not persist through a property access + // captured by a nested function (only a local const binding does). + const headSha = pr.headSha; + // Shared by both "is this PR behind base" paths below (1a/1b): force an update_branch (merges the current + // base into head, re-triggering CI on the rebased result) and report whether it actually fired. Not + // authorized, staged, dry-run, or failed (conflict/transient) → false, and the caller falls through to + // review without mutating. + const forceUpdateBranch = async (reason: string): Promise => { const autonomyLevel = resolveAutonomy(settings.autonomy, "update_branch"); const installation = await getInstallation(env, installationId); const [outcome] = await executeAgentMaintenanceActions( @@ -3524,7 +3529,7 @@ async function prReadyForReview( installationId, repoFullName, pullNumber: pr.number, - headSha: pr.headSha, + headSha, autonomy: settings.autonomy, agentPaused: settings.agentPaused, agentDryRun: settings.agentDryRun, @@ -3535,15 +3540,40 @@ async function prReadyForReview( { actionClass: "update_branch", requiresApproval: autonomyRequiresApproval(autonomyLevel), - reason: "behind base; update-branch before review", - expectedHeadSha: pr.headSha, + reason, + expectedHeadSha: headSha, }, ], ); - if (outcome?.outcome === "completed") { + return outcome?.outcome === "completed"; + }; + // 1a) rebase if BEHIND base — the synchronize on the new head re-triggers this flow on the merged result. The + // request-local facts may already be seeded from the sweep's resync payload, and the fallback live merge-state + // fetch fails open internally (swallows its own fetch errors → undefined). + const liveMergeState = await cachedLiveMergeState(env, repoFullName, liveFacts, pr.number, token, admissionKey); + if (liveMergeState === "behind") { + if (await forceUpdateBranch("behind base; update-branch before review")) { return false; // the rebase fires a synchronize → fresh review runs on the new head } - // Not authorized, staged, dry-run, or failed (conflict/transient) → fall through and review without mutating. + } else if (typeof settings.staleBaseAheadByThreshold === "number") { + // 1b) #review-grounding stale-base fact companion (metagraphed #7305-class incident): mergeable_state only + // ever reports "behind" when the repo's branch protection requires branches to be up to date before + // merging -- a repo without that setting can have a branch genuinely dozens of commits behind and GitHub + // will never surface it here. A repo that has explicitly opted into a threshold falls back to the SAME + // compare-API read #review-grounding already uses (fetchBaseAheadBy, anchored on the PR's real HEAD, never + // its live-tracking base.sha) and forces the identical update_branch action once the repo's current + // default branch has advanced at least that many commits beyond it. Costs one extra GitHub call per + // non-"behind" readiness check on an opted-in repo, which is exactly why this is opt-in rather than a new + // default (mirrors requireFreshRebaseWindowMinutes's own opt-in-for-cost rationale). + const repo = await getRepository(env, repoFullName); + const defaultBranchRef = repo?.defaultBranch; + const aheadBy = defaultBranchRef ? await fetchBaseAheadBy(env, repoFullName, headSha, defaultBranchRef, token, admissionKey) : undefined; + if (typeof aheadBy === "number" && aheadBy >= settings.staleBaseAheadByThreshold) { + const reason = `default branch is ${aheadBy} commits ahead of this PR's head (threshold ${settings.staleBaseAheadByThreshold}); update-branch before review`; + if (await forceUpdateBranch(reason)) { + return false; // the rebase fires a synchronize → fresh review runs on the new head + } + } } // 2) wait for CI to finish before running the LoopOver review. Required contexts still define which failures // block/close, but hasPending tracks any visible non-bot CI that is not settled yet. diff --git a/src/review/grounding-wire.ts b/src/review/grounding-wire.ts index b2a5ecf481..88c93599b7 100644 --- a/src/review/grounding-wire.ts +++ b/src/review/grounding-wire.ts @@ -273,11 +273,15 @@ export async function buildReviewGroundingText( files: PullRequestFileRecord[]; checks: CheckSummaryRecord[]; installationId: number | null | undefined; - // #review-grounding stale-base fact (metagraphed #7305-class incident): when both are readable, an - // additional BASE BRANCH STATUS fact is folded into the SAME ciGrounding-gated section so an undetailed CI - // failure has a true, deterministic explanation available instead of an unverified guess. Either absent ⇒ - // this fact is simply skipped (byte-identical to before it existed) — it is additive, never required. - baseSha?: string | null | undefined; + // #review-grounding stale-base fact (metagraphed #7305-class incident): when readable, an additional BASE + // BRANCH STATUS fact is folded into the SAME ciGrounding-gated section so an undetailed CI failure has a + // true, deterministic explanation available instead of an unverified guess. Absent ⇒ this fact is simply + // skipped (byte-identical to before it existed) — it is additive, never required. Deliberately NOT the + // PR's own `base.sha` -- that field tracks the LIVE tip of the target branch (GitHub updates it as the + // branch moves), so comparing it against the current default branch would read ~0 regardless of how stale + // the PR's actual code is. `headSha` (the PR's real current code, already a required param above) compared + // against the live default branch is the unambiguous git merge-base computation for "how far behind is + // this PR's actual content," independent of any GitHub-side metadata timing. defaultBranchRef?: string | null | undefined; }, ): Promise { @@ -287,13 +291,13 @@ export async function buildReviewGroundingText( const aggregate = buildCheckAggregate(args.checks); const fetcher = await makeGithubFileFetcher(env, args.repoFullName, args.installationId); const fileContents = await fetchFullFileContents(flags, args.headSha ?? undefined, toGroundingFiles(args.files), fetcher); - const baseSha = args.baseSha; + const headSha = args.headSha; const defaultBranchRef = args.defaultBranchRef; const baseAheadBy = - flags.ciGrounding && baseSha && defaultBranchRef + flags.ciGrounding && headSha && defaultBranchRef ? await (async () => { const { token, admissionKey } = await resolveGroundingToken(env, args.installationId); - return fetchBaseAheadBy(env, args.repoFullName, baseSha, defaultBranchRef, token, admissionKey); + return fetchBaseAheadBy(env, args.repoFullName, headSha, defaultBranchRef, token, admissionKey); })() : undefined; const grounding = buildGrounding(flags, aggregate, fileContents, baseAheadBy); diff --git a/src/review/review-grounding.ts b/src/review/review-grounding.ts index 9f21581e75..f3d87bebb1 100644 --- a/src/review/review-grounding.ts +++ b/src/review/review-grounding.ts @@ -54,10 +54,11 @@ export interface PullRequestFile { export interface ReviewGrounding { checks?: ReviewCiSummary; changedFileContents?: ChangedFileContent[]; - /** How many commits the repo's CURRENT default branch has landed since this PR's own base commit (#review- - * grounding stale-base fact, metagraphed #7305-class incident) — a TRUE, deterministic fact the reviewer can - * cite instead of guessing a content-level cause for an undetailed CI failure. Undefined when unreadable or - * zero (nothing to say); the caller only sets this when it is a positive number worth surfacing. */ + /** How many commits the repo's CURRENT default branch has landed that this PR's HEAD commit doesn't have + * (#review-grounding stale-base fact, metagraphed #7305-class incident) — a TRUE, deterministic fact the + * reviewer can cite instead of guessing a content-level cause for an undetailed CI failure. Undefined when + * unreadable or zero (nothing to say); the caller only sets this when it is a positive number worth + * surfacing. */ baseAheadBy?: number; } diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index ea9b998145..72cc7d4a40 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -523,6 +523,7 @@ function applyGateConfigOverrides(effective: RepositorySettings, gate: FocusMani if (gate.dryRun !== null) effective.gateDryRun = gate.dryRun; if (gate.premergeContentRecheck !== null) effective.premergeContentRecheck = gate.premergeContentRecheck; if (gate.requireFreshRebaseWindowMinutes !== null) effective.requireFreshRebaseWindowMinutes = gate.requireFreshRebaseWindowMinutes; + if (gate.staleBaseAheadByThreshold !== null) effective.staleBaseAheadByThreshold = gate.staleBaseAheadByThreshold; if (gate.claMode !== null) effective.claGateMode = gate.claMode; if (gate.claConsentPhrase !== null) effective.claConsentPhrase = gate.claConsentPhrase; if (gate.claCheckRunName !== null) effective.claCheckRunName = gate.claCheckRunName; diff --git a/src/types.ts b/src/types.ts index 27fe249818..f2ce5a1b82 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1188,6 +1188,13 @@ export type RepositorySettings = { * force -- a `mergeable_state: clean` read is trusted exactly as it is today. Layered like every other * settings field (`.loopover.yml` `gate.requireFreshRebaseWindow` > DB > `null`). */ requireFreshRebaseWindowMinutes?: number | null | undefined; + /** Stale-base auto-rebase threshold (#review-grounding stale-base fact): a commit count. When the repo's + * current default branch is at least this many commits ahead of a PR's own base commit, the pre-review + * readiness gate (`prReadyForReview`) forces an `update_branch`, independent of GitHub's own + * `mergeableState: "behind"` signal (which only fires when the repo's branch protection requires branches + * to be up to date before merging). `null`/undefined (default) = never force via this path. Layered like + * every other settings field (`.loopover.yml` `gate.staleBaseAheadByThreshold` > DB > `null`). */ + staleBaseAheadByThreshold?: number | null | undefined; /** Account-age throttle (#2561, anti-abuse): an account younger than this many days gets the * {@link newAccountLabel} and a tighter effective contributor cap — friction/visibility, NEVER an * automatic close on account age alone. `null`/undefined (default) = off. Never fires for the repo diff --git a/test/unit/data-spine.test.ts b/test/unit/data-spine.test.ts index bec2aec148..826689267b 100644 --- a/test/unit/data-spine.test.ts +++ b/test/unit/data-spine.test.ts @@ -354,6 +354,16 @@ describe("data spine repositories", () => { // contributorOpenPrCap above -- it must NOT inherit that unrelated cap's 100 ceiling. await upsertRepositorySettings(env, { repoFullName: "owner/rebasewindowrepo", requireFreshRebaseWindowMinutes: 500 }); expect((await getRepositorySettings(env, "owner/rebasewindowrepo")).requireFreshRebaseWindowMinutes).toBe(500); + // #review-grounding stale-base fact: no row and no override both default to null (never force via this path). + expect((await getRepositorySettings(env, "missing/repo")).staleBaseAheadByThreshold).toBeNull(); + expect((await getRepositorySettings(env, "owner/defaultpack")).staleBaseAheadByThreshold).toBeNull(); + // Round-trips on insert and persists on update; a fractional/non-positive value drops to null. + await upsertRepositorySettings(env, { repoFullName: "owner/stalebaserepo", staleBaseAheadByThreshold: 5 }); + expect((await getRepositorySettings(env, "owner/stalebaserepo")).staleBaseAheadByThreshold).toBe(5); + await upsertRepositorySettings(env, { repoFullName: "owner/stalebaserepo", staleBaseAheadByThreshold: 20 }); + expect((await getRepositorySettings(env, "owner/stalebaserepo")).staleBaseAheadByThreshold).toBe(20); // update persists + await upsertRepositorySettings(env, { repoFullName: "owner/stalebaserepo", staleBaseAheadByThreshold: 2.5 as never }); + expect((await getRepositorySettings(env, "owner/stalebaserepo")).staleBaseAheadByThreshold).toBeNull(); // #1936/loopover#6445: minimum account-age gate moved off the DB entirely -- config-as-code only via // .loopover.yml's settings: block now. No row and no override both default to null (never enforced); // a caller-supplied DB override is ignored; resolveRepositorySettings honors a manifest override diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 28ea194205..6d68d6e88b 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -287,6 +287,7 @@ describe(".loopover.yml.example field-exhaustiveness (#1670)", () => { dryRun: "dryRun:", premergeContentRecheck: "premergeContentRecheck:", requireFreshRebaseWindowMinutes: "requireFreshRebaseWindow:", + staleBaseAheadByThreshold: "staleBaseAheadByThreshold:", claMode: "claMode:", claConsentPhrase: "consentPhrase:", claCheckRunName: "checkRunName:", @@ -936,7 +937,7 @@ describe("compileFocusManifestPolicy", () => { issueDiscoveryPolicy: "neutral", maintainerNotes: [], publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], - gate: { present: false, enabled: null, checkMode: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, sizeMaxFiles: null, sizeMaxLines: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }, + gate: { present: false, enabled: null, checkMode: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, sizeMaxFiles: null, sizeMaxLines: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, staleBaseAheadByThreshold: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }, settings: {}, review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, sweepWatchdog: null, prReconciliation: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: null }, features: { present: false, rag: null, reputation: null, safety: null, grounding: null, e2eTests: null, screenshots: null, improvementSignal: null, amsReputationBridge: null }, @@ -1101,7 +1102,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, checkMode: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, sizeMaxFiles: null, sizeMaxLines: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }); + expect(m.gate).toEqual({ present: true, enabled: null, checkMode: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, sizeMaxFiles: null, sizeMaxLines: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, staleBaseAheadByThreshold: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }); }); it("parses gate.mergeReadiness, round-trips it, and warns on a bad value (#822)", () => { @@ -5765,6 +5766,40 @@ describe("gate.requireFreshRebaseWindow force-rebase-before-merge config (#2552) }); }); +describe("gate.staleBaseAheadByThreshold stale-base auto-rebase config (#review-grounding stale-base fact)", () => { + it("parses gate.staleBaseAheadByThreshold, sets present, round-trips, and resolves into effective settings", () => { + const m = parseFocusManifest({ gate: { staleBaseAheadByThreshold: 10 } }); + expect(m.gate.staleBaseAheadByThreshold).toBe(10); + expect(m.gate.present).toBe(true); + expect(gateConfigToJson(m.gate)).toMatchObject({ staleBaseAheadByThreshold: 10 }); + const eff = resolveEffectiveSettings({} as unknown as RepositorySettings, m); + expect(eff.staleBaseAheadByThreshold).toBe(10); + }); + + it("defaults to unset/undefined when omitted — byte-identical to today (never forces via this path)", () => { + const m = parseFocusManifest({}); + expect(m.gate.staleBaseAheadByThreshold).toBeNull(); + const eff = resolveEffectiveSettings({} as unknown as RepositorySettings, m); + expect(eff.staleBaseAheadByThreshold).toBeUndefined(); + }); + + it("warns and drops a fractional/non-positive value rather than silently coercing it", () => { + const fractional = parseFocusManifest({ gate: { staleBaseAheadByThreshold: 2.5 } }); + expect(fractional.gate.staleBaseAheadByThreshold).toBeNull(); + expect(fractional.warnings.some((w) => /gate\.staleBaseAheadByThreshold/i.test(w))).toBe(true); + + const nonPositive = parseFocusManifest({ gate: { staleBaseAheadByThreshold: 0 } }); + expect(nonPositive.gate.staleBaseAheadByThreshold).toBeNull(); + expect(nonPositive.warnings.some((w) => /gate\.staleBaseAheadByThreshold/i.test(w))).toBe(true); + }); + + it("lets the DB value pass through when the manifest doesn't override it", () => { + const db = { staleBaseAheadByThreshold: 8 } as unknown as RepositorySettings; + const eff = resolveEffectiveSettings(db, parseFocusManifest(null)); + expect(eff.staleBaseAheadByThreshold).toBe(8); + }); +}); + describe("gate.claMode / gate.cla CLA / license-compatibility gate config (#2564)", () => { it("parses gate.claMode, sets present, round-trips, and resolves into effective settings", () => { const m = parseFocusManifest({ gate: { claMode: "block" } }); diff --git a/test/unit/grounding-wiring.test.ts b/test/unit/grounding-wiring.test.ts index 6e629b4ab0..b7884354f4 100644 --- a/test/unit/grounding-wiring.test.ts +++ b/test/unit/grounding-wiring.test.ts @@ -298,15 +298,18 @@ describe("review-grounding wired into the AI reviewer (flag LOOPOVER_REVIEW_GROU // #review-grounding stale-base fact (metagraphed #7305-class incident): buildReviewGroundingText's OWN // wiring of the compare-API staleness read, on top of review-grounding.ts's already-covered pure logic. + // Anchored on `headSha` (the PR's real current code), NOT a `baseSha` field -- GitHub keeps a PR's own + // `base.sha` pointed at the live target-branch tip as it moves, so comparing THAT against the current + // default branch would read ~0 regardless of how stale the PR's actual code is. describe("buildReviewGroundingText baseAheadBy wiring", () => { const failingNoDetailCheck = check({ name: "test", conclusion: "failure", payload: {} as Record }); - it("folds BASE BRANCH STATUS into the prompt when baseSha/defaultBranchRef resolve a positive ahead_by", async () => { + it("folds BASE BRANCH STATUS into the prompt when headSha/defaultBranchRef resolve a positive ahead_by", async () => { const env = createTestEnv({ LOOPOVER_REVIEW_GROUNDING: "true", GITHUB_PUBLIC_TOKEN: "ghp_test" }); const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { const u = String(url); if (u.includes("/compare/")) { - expect(u).toBe("https://api.github.com/repos/acme/widgets/compare/abc123...main"); + expect(u).toBe("https://api.github.com/repos/acme/widgets/compare/sha7...main"); return Response.json({ ahead_by: 12 }); } return new Response("not found", { status: 404 }); @@ -317,7 +320,6 @@ describe("review-grounding wired into the AI reviewer (flag LOOPOVER_REVIEW_GROU files: [], checks: [failingNoDetailCheck], installationId: null, - baseSha: "abc123", defaultBranchRef: "main", }); expect(out.promptSection).toContain("BASE BRANCH STATUS"); @@ -325,7 +327,7 @@ describe("review-grounding wired into the AI reviewer (flag LOOPOVER_REVIEW_GROU fetchSpy.mockRestore(); }); - it("omits BASE BRANCH STATUS when baseSha/defaultBranchRef are not provided (back-compat) — no compare fetch attempted", async () => { + it("omits BASE BRANCH STATUS when defaultBranchRef is not provided (back-compat) — no compare fetch attempted", async () => { const env = createTestEnv({ LOOPOVER_REVIEW_GROUNDING: "true", GITHUB_PUBLIC_TOKEN: "ghp_test" }); const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("not found", { status: 404 })); const out = await buildReviewGroundingText(env, { @@ -341,6 +343,22 @@ describe("review-grounding wired into the AI reviewer (flag LOOPOVER_REVIEW_GROU fetchSpy.mockRestore(); }); + it("omits BASE BRANCH STATUS when headSha is not provided — no compare fetch attempted", async () => { + const env = createTestEnv({ LOOPOVER_REVIEW_GROUNDING: "true", GITHUB_PUBLIC_TOKEN: "ghp_test" }); + const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("not found", { status: 404 })); + const out = await buildReviewGroundingText(env, { + repoFullName: "acme/widgets", + headSha: null, + files: [], + checks: [failingNoDetailCheck], + installationId: null, + defaultBranchRef: "main", + }); + expect(out.promptSection).not.toContain("BASE BRANCH STATUS"); + expect(fetchSpy).not.toHaveBeenCalled(); + fetchSpy.mockRestore(); + }); + it("fail-safe: a failing compare fetch degrades to no BASE BRANCH STATUS section, CI grounding still present", async () => { const env = createTestEnv({ LOOPOVER_REVIEW_GROUNDING: "true", GITHUB_PUBLIC_TOKEN: "ghp_test" }); const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("server error", { status: 500 })); @@ -350,7 +368,6 @@ describe("review-grounding wired into the AI reviewer (flag LOOPOVER_REVIEW_GROU files: [], checks: [failingNoDetailCheck], installationId: null, - baseSha: "abc123", defaultBranchRef: "main", }); expect(out.promptSection).toContain("CI STATUS"); @@ -375,7 +392,6 @@ describe("review-grounding wired into the AI reviewer (flag LOOPOVER_REVIEW_GROU files: [], checks: [failingNoDetailCheck], installationId: 12345, - baseSha: "abc123", defaultBranchRef: "main", }); expect(sawAuth).toBe("Bearer install-token"); @@ -384,7 +400,7 @@ describe("review-grounding wired into the AI reviewer (flag LOOPOVER_REVIEW_GROU fetchSpy.mockRestore(); }); - it("does not attempt a compare read when the flag is off, even with baseSha/defaultBranchRef set", async () => { + it("does not attempt a compare read when the flag is off, even with headSha/defaultBranchRef set", async () => { const env = createTestEnv({ LOOPOVER_REVIEW_GROUNDING: "false" }); const fetchSpy = vi.spyOn(globalThis, "fetch"); const out = await buildReviewGroundingText(env, { @@ -393,7 +409,6 @@ describe("review-grounding wired into the AI reviewer (flag LOOPOVER_REVIEW_GROU files: [], checks: [failingNoDetailCheck], installationId: null, - baseSha: "abc123", defaultBranchRef: "main", }); expect(out).toEqual({ systemSuffix: "", promptSection: "" }); diff --git a/test/unit/queue-4.test.ts b/test/unit/queue-4.test.ts index 3e56f4b332..7cc93d6382 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -244,12 +244,12 @@ describe("queue processors", () => { vi.restoreAllMocks(); }); - async function seedBehindRepo(env: Env, over: { autonomy?: Record; agentPaused?: boolean; perms?: Record; noInstall?: boolean } = {}) { + async function seedBehindRepo(env: Env, over: { autonomy?: Record; agentPaused?: boolean; perms?: Record; noInstall?: boolean; defaultBranch?: string; staleBaseAheadByThreshold?: number | null } = {}) { await persistRegistrySnapshot( asCloudEnv(env), normalizeRegistryPayload({ "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, { kind: "raw-github", url: "https://example.test" }, "2026-05-23T00:00:00.000Z"), ); - await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" }, ...(over.defaultBranch !== undefined ? { default_branch: over.defaultBranch } : {}) }, 123); if (!over.noInstall) { await upsertInstallation(env, { installation: { @@ -267,6 +267,7 @@ describe("queue processors", () => { autoLabelEnabled: false, autonomy: over.autonomy ?? { merge: "auto", update_branch: "auto" }, agentPaused: over.agentPaused ?? false, + ...(over.staleBaseAheadByThreshold !== undefined ? { staleBaseAheadByThreshold: over.staleBaseAheadByThreshold } : {}), }); await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { reviewCheckMode: "required", commentMode: "off", publicSurface: "off", checkRunMode: "off" } }); } @@ -311,6 +312,32 @@ describe("queue processors", () => { expect(merge?.n).toBe(0); }); + it("auto-maintain (#1092): a BEHIND-base PR is not rebased when update_branch autonomy isn't granted (falls through to review on the stale head)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + // update_branch is absent from the autonomy policy → resolveAutonomy denies-by-default (observe), so the + // executor never issues the write even though the "behind" check itself still fires. + await seedBehindRepo(env, { autonomy: { merge: "auto" } }); + let updateBranchCalls = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/48/update-branch")) { + updateBranchCalls += 1; + return Response.json({}, { status: 202 }); + } + if (/\/pulls\/48(?:\?|$)/.test(url)) return Response.json({ number: 48, state: "open", head: { sha: "behindsha" }, mergeable_state: "behind" }); + // Not rebased (denied) → prReadyForReview falls through to the CI gate on the original head. + if (url.includes("/commits/behindsha/check-runs")) return Response.json({ total_count: 1, check_runs: [{ name: "CI build", status: "in_progress", conclusion: null }] }); + if (url.includes("/commits/behindsha/status")) return Response.json({ state: "pending", statuses: [] }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, behindWebhook()); + + expect(updateBranchCalls).toBe(0); // update_branch autonomy not granted → the executor denies the write; falls through + }); + it("auto-maintain (#1092): a behind PR is not rebased when the installation lacks pull_requests:write (falls through)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await seedBehindRepo(env, { noInstall: true }); @@ -338,6 +365,169 @@ describe("queue processors", () => { expect(updateBranchCalls).toBe(0); // no installation perms → the executor denies the write; the block falls through }); + // #review-grounding stale-base fact companion (metagraphed #7305-class incident): mergeable_state only ever + // reports "behind" when the repo's branch protection requires branches to be up to date before merging -- a + // repo without that setting reads "clean" here even though it is genuinely stale. A repo that opts into + // gate.staleBaseAheadByThreshold gets a compare-API fallback that catches this regardless. + function staleBaseWebhook() { + return { + type: "github-webhook" as const, + deliveryId: "stale-base-update-branch", + eventName: "pull_request" as const, + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 49, title: "Stale base", state: "open", user: { login: "contributor" }, head: { sha: "stalesha" }, labels: [], body: "x" }, + }, + }; + } + + it("auto-maintain (stale-base threshold): a NOT-'behind' PR still forces update-branch via the compare-API fallback once ahead_by meets the configured threshold", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedBehindRepo(env, { defaultBranch: "main", staleBaseAheadByThreshold: 5 }); + let updateBranchCalls = 0; + let compareCalls = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/49/update-branch")) { + updateBranchCalls += 1; + return Response.json({ message: "Updating pull request branch." }, { status: 202 }); + } + if (url.includes("/compare/stalesha...main")) { + compareCalls += 1; + return Response.json({ ahead_by: 10, behind_by: 0 }); + } + if (/\/pulls\/49(?:\?|$)/.test(url)) return Response.json({ number: 49, state: "open", head: { sha: "stalesha" }, mergeable_state: "clean" }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, staleBaseWebhook()); + + expect(compareCalls).toBe(1); + expect(updateBranchCalls).toBe(1); // 10 >= the configured threshold of 5 + 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 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); // deferred for the rebase → no gate verdict published on the stale head + }); + + it("auto-maintain (stale-base threshold): does not force update-branch when ahead_by is below the configured threshold", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedBehindRepo(env, { defaultBranch: "main", staleBaseAheadByThreshold: 5 }); + let updateBranchCalls = 0; + let compareCalls = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/49/update-branch")) { + updateBranchCalls += 1; + return Response.json({}, { status: 202 }); + } + if (url.includes("/compare/stalesha...main")) { + compareCalls += 1; + return Response.json({ ahead_by: 3, behind_by: 0 }); + } + if (/\/pulls\/49(?:\?|$)/.test(url)) return Response.json({ number: 49, state: "open", head: { sha: "stalesha" }, mergeable_state: "clean" }); + // Not rebased → prReadyForReview proceeds to the CI gate on the original head; green CI so it doesn't defer forever. + if (url.includes("/commits/stalesha/check-runs")) return Response.json({ total_count: 1, check_runs: [{ name: "CI build", status: "completed", conclusion: "success" }] }); + if (url.includes("/commits/stalesha/status")) return Response.json({ state: "success", statuses: [] }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, staleBaseWebhook()); + + expect(compareCalls).toBe(1); // the threshold IS configured, so the fallback check still runs + expect(updateBranchCalls).toBe(0); // 3 < the configured threshold of 5 → no forced rebase + }); + + it("auto-maintain (stale-base threshold): never attempts the compare-API fallback when no threshold is configured (zero added cost, byte-identical to before this feature existed)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedBehindRepo(env, { defaultBranch: "main" }); // staleBaseAheadByThreshold left unset (null, the default) + let compareCalls = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/compare/")) { + compareCalls += 1; + return Response.json({ ahead_by: 999, behind_by: 0 }); + } + if (/\/pulls\/49(?:\?|$)/.test(url)) return Response.json({ number: 49, state: "open", head: { sha: "stalesha" }, mergeable_state: "clean" }); + if (url.includes("/commits/stalesha/check-runs")) return Response.json({ total_count: 1, check_runs: [{ name: "CI build", status: "completed", conclusion: "success" }] }); + if (url.includes("/commits/stalesha/status")) return Response.json({ state: "success", statuses: [] }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, staleBaseWebhook()); + + expect(compareCalls).toBe(0); + }); + + it("auto-maintain (stale-base threshold): a threshold configured but no stored default branch skips the compare read entirely (nothing to compare against)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedBehindRepo(env, { staleBaseAheadByThreshold: 5 }); // defaultBranch left unset (no stored repos.default_branch row) + let compareCalls = 0; + let updateBranchCalls = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/49/update-branch")) { + updateBranchCalls += 1; + return Response.json({}, { status: 202 }); + } + if (url.includes("/compare/")) { + compareCalls += 1; + return Response.json({ ahead_by: 999, behind_by: 0 }); + } + if (/\/pulls\/49(?:\?|$)/.test(url)) return Response.json({ number: 49, state: "open", head: { sha: "stalesha" }, mergeable_state: "clean" }); + if (url.includes("/commits/stalesha/check-runs")) return Response.json({ total_count: 1, check_runs: [{ name: "CI build", status: "completed", conclusion: "success" }] }); + if (url.includes("/commits/stalesha/status")) return Response.json({ state: "success", statuses: [] }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, staleBaseWebhook()); + + expect(compareCalls).toBe(0); // no defaultBranchRef to compare against → the fallback short-circuits before any GitHub call + expect(updateBranchCalls).toBe(0); + }); + + it("auto-maintain (stale-base threshold): a met threshold still falls through to review when update_branch autonomy isn't granted", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + // update_branch is absent from the autonomy policy → resolveAutonomy denies-by-default (observe), so the + // executor never issues the write even though the threshold check itself still ran and found it stale. + await seedBehindRepo(env, { defaultBranch: "main", staleBaseAheadByThreshold: 5, autonomy: { merge: "auto" } }); + let compareCalls = 0; + let updateBranchCalls = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/49/update-branch")) { + updateBranchCalls += 1; + return Response.json({}, { status: 202 }); + } + if (url.includes("/compare/")) { + compareCalls += 1; + return Response.json({ ahead_by: 10, behind_by: 0 }); + } + if (/\/pulls\/49(?:\?|$)/.test(url)) return Response.json({ number: 49, state: "open", head: { sha: "stalesha" }, mergeable_state: "clean" }); + // Not rebased (denied) → prReadyForReview falls through to the CI gate on the original head. + if (url.includes("/commits/stalesha/check-runs")) return Response.json({ total_count: 1, check_runs: [{ name: "CI build", status: "in_progress", conclusion: null }] }); + if (url.includes("/commits/stalesha/status")) return Response.json({ state: "pending", statuses: [] }); + return new Response("not found", { status: 404 }); + }); + + await processJob(env, staleBaseWebhook()); + + expect(compareCalls).toBe(1); // the threshold check still ran and found the branch stale (10 >= 5) + expect(updateBranchCalls).toBe(0); // update_branch autonomy not granted → the executor denies the write; falls through + }); + it("recapture-preview (#1158): a clean PR re-review threads previewPollAttempt into the public-surface publish", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await upsertInstallation(env, { action: "created", installation: { id: 9101, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); diff --git a/test/unit/routes-ai-byok.test.ts b/test/unit/routes-ai-byok.test.ts index 1361783e95..a04f5ee26f 100644 --- a/test/unit/routes-ai-byok.test.ts +++ b/test/unit/routes-ai-byok.test.ts @@ -161,6 +161,15 @@ describe("maintainer AI-review config route", () => { expect((await getRepositorySettings(env, REPO)).requireFreshRebaseWindowMinutes).toBe(15); }); + it("round-trips staleBaseAheadByThreshold through the maintainer settings PUT route (#review-grounding stale-base fact)", 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({ staleBaseAheadByThreshold: 10 }) }, env); + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ staleBaseAheadByThreshold: 10 }); + expect((await getRepositorySettings(env, REPO)).staleBaseAheadByThreshold).toBe(10); + }); + it("lets the internal full settings route persist closeOwnerAuthors", async () => { const app = createApp(); const env = createTestEnv({ TOKEN_ENCRYPTION_SECRET: SECRET });