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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .loopover.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,17 @@ gate:
- build
- test

# Third-party check-runs that must never gate CI pass/fail or count as pending, but whose non-pass
# conclusions should route an otherwise-green PR to manual review instead of merging through a flag the
# operator installed an app to raise (#4372). Each entry matches a check-run by exact name AND trusted app
# slug (same trust model as gate.cla.checkRunName/checkRunAppSlug). A completed advisory check with
# success/neutral/skipped is settled and ignored; failure/action_required/etc. holds for manual review.
# List of { name, appSlug } mappings, or omit. Default: not configured (byte-identical to today's behavior).
# Config-as-code only — no DB column or dashboard toggle.
# advisoryCheckRuns:
# - name: Example trust scan
# appSlug: example-trust-app

# Promote a confident AI-judgment-only finding (one the reviewer itself placed under "Blockers", never
# a "Nit") into a real, deterministic gate blocker instead of leaving it advisory (#3907). Only matters
# for repos already running the registry content lane (see contentLane below) — content/registry repos
Expand Down
11 changes: 11 additions & 0 deletions config/examples/loopover.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,17 @@ gate:
- build
- test

# Third-party check-runs that must never gate CI pass/fail or count as pending, but whose non-pass
# conclusions should route an otherwise-green PR to manual review instead of merging through a flag the
# operator installed an app to raise (#4372). Each entry matches a check-run by exact name AND trusted app
# slug (same trust model as gate.cla.checkRunName/checkRunAppSlug). A completed advisory check with
# success/neutral/skipped is settled and ignored; failure/action_required/etc. holds for manual review.
# List of { name, appSlug } mappings, or omit. Default: not configured (byte-identical to today's behavior).
# Config-as-code only — no DB column or dashboard toggle.
# advisoryCheckRuns:
# - name: Example trust scan
# appSlug: example-trust-app

# Promote a confident AI-judgment-only finding (one the reviewer itself placed under "Blockers", never
# a "Nit") into a real, deterministic gate blocker instead of leaving it advisory (#3907). Only matters
# for repos already running the registry content lane (see contentLane below) — content/registry repos
Expand Down
40 changes: 40 additions & 0 deletions packages/loopover-engine/src/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ export type FocusManifestIssueDiscoveryPolicy = "encouraged" | "neutral" | "disc
* axis entirely: whether/how the "LoopOver Orb Review Agent" check-RUN publishes, independent of gate
* evaluation itself (which always runs regardless of `checkMode`/`enabled`) — see {@link ReviewCheckMode}.
*/
export type AdvisoryCheckRunSpec = {
name: string;
appSlug: string;
};

export type FocusManifestGateConfig = {
present: boolean;
/** `gate.enabled` (legacy): a boolean shorthand for `checkMode` below -- `true` maps to `"required"`,
Expand Down Expand Up @@ -186,6 +191,10 @@ export type FocusManifestGateConfig = {
* (unset) ⇒ no generic fallback configured — the live-CI aggregate keeps today's fold-all behavior
* when branch protection is also unreadable. See {@link RepositorySettings.expectedCiContexts}. */
expectedCiContexts: ReadonlyArray<string> | null;
/** `gate.advisoryCheckRuns` (#4372): third-party check-runs that must never gate CI pass/fail or count as
* "still running"; a completed non-pass routes to manual-review instead. null (unset) ⇒ byte-identical to
* today's behavior for repos that do not configure it. */
advisoryCheckRuns: ReadonlyArray<AdvisoryCheckRunSpec> | null;
/** `gate.aiJudgmentBlockers` (#3907): "gate" | "advisory", null (unset) ⇒ "advisory" (byte-identical to
* today everywhere that doesn't opt in). Config-as-code only, YML-only (no DB column, no dashboard
* toggle) — mirrors `contentLane`'s own YML-only shape, since this only has an effect for repos already
Expand Down Expand Up @@ -997,6 +1006,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = {
claCheckRunName: null,
claCheckRunAppSlug: null,
expectedCiContexts: null,
advisoryCheckRuns: null,
aiJudgmentBlockersMode: null,
copycatMode: null,
copycatMinScore: null,
Expand Down Expand Up @@ -1281,6 +1291,31 @@ function normalizeOptionalReviewers(
return out.length > 0 ? out : null;
}

function normalizeAdvisoryCheckRunsList(
value: JsonValue | undefined,
field: string,
warnings: string[],
): ReadonlyArray<AdvisoryCheckRunSpec> | null {
if (value === undefined || value === null) return null;
if (!Array.isArray(value)) {
warnings.push(`Manifest field "${field}" must be a list; ignoring it.`);
return null;
}
const out: AdvisoryCheckRunSpec[] = [];
for (const entry of value) {
if (typeof entry !== "object" || entry === null || Array.isArray(entry)) {
warnings.push(`Manifest field "${field}" has a non-mapping entry; dropping it.`);
continue;
}
const record = entry as Record<string, JsonValue>;
const name = parsePublicSafeText(record.name, `${field}[].name`, warnings);
const appSlug = parsePublicSafeText(record.appSlug, `${field}[].appSlug`, warnings);
if (name === null || appSlug === null) continue;
out.push({ name, appSlug });
}
return out.length > 0 ? out : null;
}

/**
* Parse the optional `gate:` mapping. Every field stays `null` when unset so the resolver can layer
* this OVER DB settings without clobbering. A nested `readiness: { mode, minScore }` block is accepted.
Expand Down Expand Up @@ -1364,6 +1399,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
claCheckRunName: parsePublicSafeText(claRecord?.checkRunName, "gate.cla.checkRunName", warnings),
claCheckRunAppSlug: parsePublicSafeText(claRecord?.checkRunAppSlug, "gate.cla.checkRunAppSlug", warnings),
expectedCiContexts: normalizeOptionalStringList(record.expectedCiContexts, "gate.expectedCiContexts", warnings),
advisoryCheckRuns: normalizeAdvisoryCheckRunsList(record.advisoryCheckRuns, "gate.advisoryCheckRuns", warnings),
aiJudgmentBlockersMode: normalizeOptionalEnum(record.aiJudgmentBlockers, "gate.aiJudgmentBlockers", ["gate", "advisory"] as const, warnings),
copycatMode: normalizeOptionalEnum(copycatRecord?.mode, "gate.copycat.mode", ["off", "warn", "label", "block"] as const, warnings),
copycatMinScore: normalizeOptionalScore(copycatRecord?.minScore, "gate.copycat.minScore", warnings),
Expand Down Expand Up @@ -1424,6 +1460,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
gate.claCheckRunName !== null ||
gate.claCheckRunAppSlug !== null ||
gate.expectedCiContexts !== null ||
gate.advisoryCheckRuns !== null ||
gate.aiJudgmentBlockersMode !== null ||
gate.copycatMode !== null ||
gate.copycatMinScore !== null;
Expand Down Expand Up @@ -1503,6 +1540,9 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue {
out.cla = cla;
}
if (gate.expectedCiContexts !== null) out.expectedCiContexts = gate.expectedCiContexts as JsonValue;
if (gate.advisoryCheckRuns !== null) {
out.advisoryCheckRuns = gate.advisoryCheckRuns.map((entry) => ({ name: entry.name, appSlug: entry.appSlug })) as JsonValue;
}
if (gate.aiJudgmentBlockersMode !== null) out.aiJudgmentBlockers = gate.aiJudgmentBlockersMode;
if (gate.copycatMode !== null || gate.copycatMinScore !== null) {
const copycat: Record<string, JsonValue> = {};
Expand Down
1 change: 1 addition & 0 deletions packages/loopover-engine/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,7 @@ export {
REVIEW_PROFILES,
type AutoReviewConfig,
type CommentVerbosity,
type AdvisoryCheckRunSpec,
type ConvergedFeatureKey,
type ExperimentalPluginKey,
type FocusManifest,
Expand Down
51 changes: 51 additions & 0 deletions src/github/advisory-check-runs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import type { AdvisoryCheckRunSpec } from "@loopover/engine";

/** True when `run` matches a configured `gate.advisoryCheckRuns` entry (name + app slug, case-insensitive). */
export function matchesConfiguredAdvisoryCheckRun(
run: { name: string; app?: { slug?: string | null } | null },
specs: ReadonlyArray<AdvisoryCheckRunSpec> | null | undefined,
): boolean {
if (!specs || specs.length === 0) return false;
const runName = run.name.trim().toLowerCase();
const runSlug = (run.app?.slug ?? "").trim().toLowerCase();
if (!runName || !runSlug) return false;
return specs.some(
(spec) => spec.name.trim().toLowerCase() === runName && spec.appSlug.trim().toLowerCase() === runSlug,
);
}

const ADVISORY_PASSING_CONCLUSIONS = new Set(["success", "neutral", "skipped"]);

/** A completed advisory check-run with one of these conclusions is settled and needs no hold. */
export function isAdvisoryCheckRunSettledPass(conclusion: string): boolean {
return ADVISORY_PASSING_CONCLUSIONS.has(conclusion.trim().toLowerCase());
}

/** Stable cache-key fragment for a repo's configured advisory check-run list (#4372). */
export function advisoryCheckRunsKeyPart(specs: ReadonlyArray<AdvisoryCheckRunSpec> | null | undefined): string {
if (!specs || specs.length === 0) return "";
return JSON.stringify(
[...specs]
.map((spec) => ({ name: spec.name, appSlug: spec.appSlug }))
.sort((left, right) => `${left.appSlug}/${left.name}`.localeCompare(`${right.appSlug}/${right.name}`)),
);
}

export function resolveAdvisoryCheckHold(
advisoryHoldDetails: ReadonlyArray<{ name: string; summary?: string; appSlug?: string }> | undefined,
advisoryCheckRuns: ReadonlyArray<AdvisoryCheckRunSpec> | null | undefined,
): { checkNames: readonly string[]; reason: string; comment: string } | undefined {
if (!advisoryCheckRuns?.length || !advisoryHoldDetails?.length) return undefined;
const checkNames = advisoryHoldDetails.map((detail) => detail.name);
const lines = advisoryHoldDetails.map((detail) => {
const app = detail.appSlug?.trim();
const summary = detail.summary?.trim();
const prefix = app ? `${detail.name} (${app})` : detail.name;
return summary ? `- ${prefix}: ${summary}` : `- ${prefix}`;
});
return {
checkNames,
reason: `advisory check-run hold (${checkNames.join(", ")})`,
comment: `LoopOver: a configured advisory check-run needs maintainer action before this PR can merge:\n${lines.join("\n")}`,
};
}
41 changes: 32 additions & 9 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ import {
type GitHubRateLimitAdmissionKey,
} from "./client";
import { fetchCachedGitHubGraphQl } from "./graphql-cache";
import { isAdvisoryCheckRunSettledPass, matchesConfiguredAdvisoryCheckRun } from "./advisory-check-runs.js";
import type { AdvisoryCheckRunSpec } from "@loopover/engine";
import { incr } from "../selfhost/metrics";
import { fetchBrokeredInstallationToken, isOrbBrokerMode } from "../orb/broker-client";
type GitHubLabelPayload = {
Expand Down Expand Up @@ -2572,6 +2574,9 @@ export type LiveCiAggregate = {
failingDetails: Array<{ name: string; summary?: string; detailsUrl?: string }>;
// Historical compatibility: non-required red checks are now folded into failingDetails so this stays empty.
nonRequiredFailingDetails: Array<{ name: string; summary?: string; detailsUrl?: string }>;
/** Configured advisory check-runs (#4372) that completed with a non-pass conclusion — routed to manual-review,
* never folded into ciState/hasPending. Empty when the repo did not opt in via `gate.advisoryCheckRuns`. */
advisoryHoldDetails?: Array<{ name: string; summary?: string; detailsUrl?: string; appSlug?: string }>;
// Informational-only (#2137): set when the aggregate resolved to "passed" with no branch-protection required
// contexts configured (`enforceRequiredOnly` false) — meaning a workflow that never triggers on this commit at
// all (e.g. path-filtered out, or a broken YAML trigger) is indistinguishable from one that doesn't exist, and
Expand Down Expand Up @@ -2799,9 +2804,10 @@ async function reduceLiveCiAggregate(
checkRunsIncomplete: boolean;
statusIncomplete: boolean;
fetchSuites: () => Promise<ReadonlyArray<LiveCiSuite> | null>;
advisoryCheckRuns?: ReadonlyArray<AdvisoryCheckRunSpec> | null | undefined;
},
): Promise<LiveCiAggregate> {
const { checkRuns, statuses, requiredContexts, checkRunsIncomplete, statusIncomplete, fetchSuites } = inputs;
const { checkRuns, statuses, requiredContexts, checkRunsIncomplete, statusIncomplete, fetchSuites, advisoryCheckRuns } = inputs;
const enforceRequiredOnly = requiredContexts != null && requiredContexts.size > 0;
const isRequired = (name: string): boolean => !enforceRequiredOnly || requiredContexts!.has(name);
// Deliberately the OPPOSITE unknown-case default from isRequired() above, and used ONLY for a third-party
Expand All @@ -2819,6 +2825,7 @@ async function reduceLiveCiAggregate(
const isConfirmedRequired = (name: string): boolean => enforceRequiredOnly && requiredContexts!.has(name);
const failingDetails: LiveCiAggregate["failingDetails"] = [];
const nonRequiredFailingDetails: LiveCiAggregate["nonRequiredFailingDetails"] = [];
const advisoryHoldDetails: LiveCiAggregate["advisoryHoldDetails"] = [];
let total = 0;
let anyPending = false;
let anyVisiblePending = false;
Expand All @@ -2836,10 +2843,22 @@ async function reduceLiveCiAggregate(
seenContextNames.add(run.name); // mark BEFORE bot-check skip: a bot-owned required context is "seen"
const appSlug = (run.app?.slug ?? "").toLowerCase();
if (appSlug === "github-actions") sawFirstPartyCheckRun = true;
if (isOwnGitHubAppCheckRun(env, run)) continue; // never wait on the bot's own Gate/Context check-runs
total += 1;
const conclusion = (run.conclusion ?? "").toLowerCase();
const status = (run.status ?? "").toLowerCase();
if (isOwnGitHubAppCheckRun(env, run)) continue; // never wait on the bot's own Gate/Context check-runs
if (matchesConfiguredAdvisoryCheckRun(run, advisoryCheckRuns)) {
if (status === "completed" && conclusion && !isAdvisoryCheckRunSettledPass(conclusion)) {
const summary = checkRunSummary(run);
advisoryHoldDetails.push({
name: run.name,
...(summary ? { summary } : {}),
...(run.details_url ? { detailsUrl: run.details_url } : {}),
...(appSlug ? { appSlug } : {}),
});
}
continue;
}
total += 1;
// A THIRD-PARTY app's OWN action_required verdict on an already-COMPLETED check-run (for example, a
// security/check tool asking for human review) is a settled, terminal adverse result -- but ONLY when that
// check is actually a REQUIRED context. A non-required third-party check must never hard-fail/auto-close the
Expand Down Expand Up @@ -2947,7 +2966,7 @@ async function reduceLiveCiAggregate(
// A partial/paginated read can't tell "never appears" from "appears on a page we didn't fetch" -- only a
// COMPLETE read's absence is a confident signal worth a short surfacing cap (#selfhost-ci-deferral-staleness).
const hasMissingRequiredContext = anyMissingRequiredContext && !checkRunsIncomplete && !statusIncomplete;
return { ciState, hasPending, hasVisiblePending: anyRequiredVisiblePending, hasMissingRequiredContext, failingDetails, nonRequiredFailingDetails, ciCompletenessWarning };
return { ciState, hasPending, hasVisiblePending: anyRequiredVisiblePending, hasMissingRequiredContext, failingDetails, nonRequiredFailingDetails, advisoryHoldDetails, ciCompletenessWarning };
}

/**
Expand All @@ -2968,8 +2987,9 @@ export async function fetchLiveCiAggregate(
// Completed red checks/statuses still fail the aggregate even when they are not branch-protection-required.
requiredContexts?: ReadonlySet<string> | null,
admissionKey?: GitHubRateLimitAdmissionKey,
advisoryCheckRuns?: ReadonlyArray<AdvisoryCheckRunSpec> | null,
): Promise<LiveCiAggregate> {
if (!headSha) return { ciState: "unverified", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null };
if (!headSha) return { ciState: "unverified", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null };
// Check-runs + classic statuses are accumulated across pages here; the single classification lives in
// reduceLiveCiAggregate so the REST and GraphQL paths reach byte-identical verdicts (#1941).
const checkRuns: LiveCiCheckRun[] = [];
Expand Down Expand Up @@ -3014,6 +3034,7 @@ export async function fetchLiveCiAggregate(
requiredContexts,
checkRunsIncomplete,
statusIncomplete,
advisoryCheckRuns,
// Lazily read the check-SUITES backstop only when the reducer finds the cheaper sources fully settled; a fetch
// error returns null so the reducer fails closed exactly as the inline path did.
fetchSuites: async () => {
Expand Down Expand Up @@ -3051,6 +3072,7 @@ export async function fetchLiveCiAggregateViaGraphQl(
token: string | undefined,
requiredContexts?: ReadonlySet<string> | null,
admissionKey?: GitHubRateLimitAdmissionKey,
advisoryCheckRuns?: ReadonlyArray<AdvisoryCheckRunSpec> | null,
): Promise<LiveCiAggregate | null> {
if (!headSha || !token) return null;
const [owner, name] = repoFullName.split("/");
Expand Down Expand Up @@ -3132,6 +3154,7 @@ export async function fetchLiveCiAggregateViaGraphQl(
requiredContexts,
checkRunsIncomplete: false,
statusIncomplete: false,
advisoryCheckRuns,
fetchSuites: async () => suites, // already fetched in the same query — never a second round-trip
});
}
Expand All @@ -3149,14 +3172,13 @@ export async function fetchLiveCiAggregatePreferGraphQl(
token: string | undefined,
requiredContexts?: ReadonlySet<string> | null,
admissionKey?: GitHubRateLimitAdmissionKey,
advisoryCheckRuns?: ReadonlyArray<AdvisoryCheckRunSpec> | null,
): Promise<LiveCiAggregate> {
if (isStatusRollupGraphQlEnabled(env)) {
// fetchLiveCiAggregateViaGraphQl handles all its own errors and returns null on any uncertainty (it never
// rejects), so a null result — not a throw — is the fall-back-to-REST signal.
const rollup = await fetchLiveCiAggregateViaGraphQl(env, repoFullName, headSha, token, requiredContexts, admissionKey);
const rollup = await fetchLiveCiAggregateViaGraphQl(env, repoFullName, headSha, token, requiredContexts, admissionKey, advisoryCheckRuns);
if (rollup) return rollup;
}
return fetchLiveCiAggregate(env, repoFullName, headSha, token, requiredContexts, admissionKey);
return fetchLiveCiAggregate(env, repoFullName, headSha, token, requiredContexts, admissionKey, advisoryCheckRuns);
}

/**
Expand Down Expand Up @@ -3628,6 +3650,7 @@ export function deserializeCachedCiAggregate(
hasMissingRequiredContext: cached.ciHasMissingRequiredContext ?? false,
failingDetails,
nonRequiredFailingDetails,
advisoryHoldDetails: [],
ciCompletenessWarning: cached.ciCompletenessWarning ?? null,
};
} catch {
Expand Down
Loading
Loading