From ce8f2ee6d3dbef7d34a165d68a6502efa04c1075 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 03:46:35 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat(gate):=20gate.ignoredCheckRuns=20?= =?UTF-8?q?=E2=80=94=20exclude=20a=20third-party=20check=20from=20CI=20ent?= =?UTF-8?q?irely?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit advisoryCheckRuns stops a non-passing third-party check from GATING, but still routes it to a manual-review hold. For a check whose verdict carries no signal for a repo, that trades a stall for a queue of human decisions: on the ORB, a vendor's heuristic contributor-trust score fails for known-good contributors, so every one of their otherwise-flawless PRs became a manual review -- automation replaced by maintainer time, and contributors left wondering whether the bar is applied fairly. Add gate.ignoredCheckRuns: same spoof-resistant { name, appSlug } matching, but a matched run is treated as if it did not exist -- never gates, never counts as pending, never holds. Its conclusion is surfaced in the aggregate as ignoredCheckDetails (informational only) so the exclusion stays visible instead of the check silently vanishing. Sibling checks from the SAME app are unaffected: a vendor's real security scan keeps gating while its trust score is ignored. If a check is named in both lists ignore wins, being the stronger and more explicit intent. Both live-fact memo keys and the durable CI-state cache key now include the ignore list, so changing it invalidates cached aggregates the way changing advisoryCheckRuns already does. Trailing-optional throughout: unset means byte-identical behavior for every existing repo and self-hoster. --- .loopover.yml.example | 19 +++++ apps/loopover-ui/public/openapi.json | 19 +++++ .../loopover-engine/src/focus-manifest.ts | 13 +++ .../src/types/manifest-deps-types.ts | 3 + .../src/types/predicted-gate-types.ts | 1 + src/github/backfill.ts | 36 ++++++-- src/openapi/schemas.ts | 1 + src/queue/ci-resolution.ts | 26 ++++-- src/queue/processors.ts | 2 + src/services/agent-action-executor.ts | 4 +- src/services/agent-approval-queue.ts | 3 +- src/signals/focus-manifest.ts | 1 + src/types.ts | 5 ++ test/unit/agent-action-executor.test.ts | 22 ++--- test/unit/agent-approval-queue.test.ts | 10 +-- test/unit/backfill-2.test.ts | 83 ++++++++++++++++++- test/unit/ci-resolution.test.ts | 4 +- test/unit/focus-manifest.test.ts | 5 +- test/unit/mcp-automation-state.test.ts | 2 +- test/unit/pr-detail-durable-cache.test.ts | 4 +- test/unit/queue-2.test.ts | 6 +- test/unit/queue-4.test.ts | 26 +++--- test/unit/queue-5.test.ts | 2 +- test/unit/queue.test.ts | 44 +++++----- test/unit/routes-agent-approval.test.ts | 2 +- 25 files changed, 266 insertions(+), 77 deletions(-) diff --git a/.loopover.yml.example b/.loopover.yml.example index f0aa1a82d1..a32890c11e 100644 --- a/.loopover.yml.example +++ b/.loopover.yml.example @@ -276,6 +276,25 @@ gate: - name: Contributor trust appSlug: example-security-app + # Check-runs to IGNORE ENTIRELY (#9810) — the stronger sibling of advisoryCheckRuns above. Same + # spoof-resistant { name, appSlug } matching, but a matched run is treated as if it did not exist: it never + # gates CI, never counts as "still running", and — unlike advisory — never routes the PR to a manual-review + # hold either. Its conclusion is surfaced informationally only. + # + # Use this when a check's verdict carries no signal for YOUR repo while OTHER checks from the same app stay + # meaningful. The motivating case: a vendor app publishes both a real security scan AND a heuristic + # contributor-trust score. The scan is worth gating on; the trust score fails for perfectly good + # contributors, and listing it under advisoryCheckRuns still converts every one of their otherwise-clean PRs + # into a manual review — automation replaced by a queue of human decisions, and contributors left wondering + # whether they are being judged fairly. Ignoring the trust check keeps the scan's protection and drops the + # noise. If BOTH lists name the same check, ignore wins (it is the stronger, more explicit intent). + # + # List of { name, appSlug }, or omit. Default: not configured (byte-identical behavior for every repo that + # doesn't opt in). Config-as-code only — no DB column or dashboard toggle. + ignoredCheckRuns: + - name: Contributor trust + appSlug: example-security-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 diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 50b8e25760..424fb6588f 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -10127,6 +10127,25 @@ "updatedAt": { "type": "string", "nullable": true + }, + "ignoredCheckRuns": { + "type": "array", + "nullable": true, + "items": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "appSlug": { + "type": "string" + } + }, + "required": [ + "name", + "appSlug" + ] + } } }, "required": [ diff --git a/packages/loopover-engine/src/focus-manifest.ts b/packages/loopover-engine/src/focus-manifest.ts index 7986668d9d..8d016d8f8c 100644 --- a/packages/loopover-engine/src/focus-manifest.ts +++ b/packages/loopover-engine/src/focus-manifest.ts @@ -232,6 +232,13 @@ export type FocusManifestGateConfig = { * config-only — no vendor name is ever hardcoded in behavior. null/empty (unset) ⇒ byte-identical to today. * See {@link RepositorySettings.advisoryCheckRuns}. */ advisoryCheckRuns: ReadonlyArray<{ name: string; appSlug: string }> | null; + /** `gate.ignoredCheckRuns` (#9810): third-party check-runs to EXCLUDE from CI resolution entirely -- never + * gate, never pend, never hold. Same spoof-resistant `{ name, appSlug }` matching as advisoryCheckRuns. + * For a check whose verdict a maintainer has decided carries no signal for this repo (e.g. a vendor's + * contributor-trust score) while OTHER checks from the same app stay meaningful. Advisory still routes a + * failure to a manual-review hold; ignored does not -- the run is treated as if it did not exist, and is + * surfaced informationally in the aggregate for panel transparency. */ + ignoredCheckRuns: ReadonlyArray<{ name: string; appSlug: string }> | 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 @@ -1358,6 +1365,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = { claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, + ignoredCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null, @@ -1806,6 +1814,7 @@ const GATE_TOP_LEVEL_KEYS = new Set([ "cla", "expectedCiContexts", "advisoryCheckRuns", + "ignoredCheckRuns", "aiJudgmentBlockers", "copycat", ]); @@ -1900,6 +1909,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu claCheckRunAppSlug: parsePublicSafeText(claRecord?.checkRunAppSlug, "gate.cla.checkRunAppSlug", warnings), expectedCiContexts: normalizeOptionalStringList(record.expectedCiContexts, "gate.expectedCiContexts", warnings), advisoryCheckRuns: normalizeOptionalAdvisoryCheckRuns(record.advisoryCheckRuns, "gate.advisoryCheckRuns", warnings), + ignoredCheckRuns: normalizeOptionalAdvisoryCheckRuns(record.ignoredCheckRuns, "gate.ignoredCheckRuns", 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), @@ -2059,6 +2069,9 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue { if (gate.advisoryCheckRuns !== null) { out.advisoryCheckRuns = gate.advisoryCheckRuns.map((c) => ({ name: c.name, appSlug: c.appSlug })) as JsonValue; } + if (gate.ignoredCheckRuns !== null) { + out.ignoredCheckRuns = gate.ignoredCheckRuns.map((c) => ({ name: c.name, appSlug: c.appSlug })) as JsonValue; + } if (gate.aiJudgmentBlockersMode !== null) out.aiJudgmentBlockers = gate.aiJudgmentBlockersMode; if (gate.copycatMode !== null || gate.copycatMinScore !== null) { const copycat: Record = {}; diff --git a/packages/loopover-engine/src/types/manifest-deps-types.ts b/packages/loopover-engine/src/types/manifest-deps-types.ts index 2a38ec0c72..0c270443c3 100644 --- a/packages/loopover-engine/src/types/manifest-deps-types.ts +++ b/packages/loopover-engine/src/types/manifest-deps-types.ts @@ -245,6 +245,9 @@ export type RepositorySettings = { * counts as "still running"); a non-passing conclusion routes the PR to the manual-review hold instead of * being swallowed. Config-as-code only — no DB column; set via `.loopover.yml gate.advisoryCheckRuns`. */ advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; + /** `gate.ignoredCheckRuns` (#9810): check-runs excluded from CI resolution entirely -- never gate, never + * pend, never hold. Config-as-code only; same `{ name, appSlug }` anti-spoof shape as advisoryCheckRuns. */ + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; /** Dry-run disposition (#gate-dryrun). When true, the gate renders the would-be merge/close/manual verdict (every * advisory sub-gate promoted to block) WITHOUT enforcing — the posted check stays non-blocking. Lets advisory mode * preview exactly what it would do before the maintainer flips to real enforcement. Default off. */ diff --git a/packages/loopover-engine/src/types/predicted-gate-types.ts b/packages/loopover-engine/src/types/predicted-gate-types.ts index d3da3975c5..88b51fb367 100644 --- a/packages/loopover-engine/src/types/predicted-gate-types.ts +++ b/packages/loopover-engine/src/types/predicted-gate-types.ts @@ -318,6 +318,7 @@ export type FocusManifestGateConfig = { claCheckRunAppSlug: string | null; expectedCiContexts: ReadonlyArray | null; advisoryCheckRuns: ReadonlyArray<{ name: string; appSlug: string }> | null; + ignoredCheckRuns: ReadonlyArray<{ name: string; appSlug: string }> | null; }; export type PreMergeCheck = { diff --git a/src/github/backfill.ts b/src/github/backfill.ts index 00c6b9d28a..00b32510e8 100644 --- a/src/github/backfill.ts +++ b/src/github/backfill.ts @@ -2719,6 +2719,11 @@ export type LiveCiAggregate = { // surfaced here so the disposition planner can route the PR to a manual-review hold instead of silently // swallowing a signal a maintainer installed a whole app to raise. Empty for every repo that doesn't opt in. advisoryHoldDetails: Array<{ name: string; appSlug: string; conclusion: string }>; + // #9810: a maintainer-declared `gate.ignoredCheckRuns` check-run that was seen and excluded ENTIRELY -- never + // gates, never pends, never holds, whatever its conclusion. Informational only, surfaced so the panel/audit + // can say "this check was ignored by repo policy" instead of the check silently vanishing. Empty for every + // repo that doesn't opt in. + ignoredCheckDetails: Array<{ name: string; appSlug: string; conclusion: 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 @@ -2992,12 +2997,13 @@ async function reduceLiveCiAggregate( statuses: ReadonlyArray; requiredContexts: ReadonlySet | null | undefined; advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; checkRunsIncomplete: boolean; statusIncomplete: boolean; fetchSuites: () => Promise | null>; }, ): Promise { - const { checkRuns, statuses, requiredContexts, advisoryCheckRuns, checkRunsIncomplete, statusIncomplete, fetchSuites } = inputs; + const { checkRuns, statuses, requiredContexts, advisoryCheckRuns, ignoredCheckRuns, checkRunsIncomplete, statusIncomplete, fetchSuites } = 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 @@ -3016,6 +3022,7 @@ async function reduceLiveCiAggregate( const failingDetails: LiveCiAggregate["failingDetails"] = []; const nonRequiredFailingDetails: LiveCiAggregate["nonRequiredFailingDetails"] = []; const advisoryHoldDetails: LiveCiAggregate["advisoryHoldDetails"] = []; + const ignoredCheckDetails: LiveCiAggregate["ignoredCheckDetails"] = []; let total = 0; let anyPending = false; let anyVisiblePending = false; @@ -3040,6 +3047,14 @@ async function reduceLiveCiAggregate( // with a non-passing conclusion, it is recorded in advisoryHoldDetails so the disposition planner can route // the PR to a manual-review hold. An advisory check still in progress is simply ignored (it may yet pass); // nothing about it holds the gate either way. + // #9810: an ignored check-run is excluded from EVERYTHING -- ciState, pending, holds. Recorded + // informationally so the exclusion is visible rather than the check silently vanishing. Checked BEFORE + // the advisory list: if a check appears in both, ignore is the stronger, later-declared maintainer intent. + const ignoredMatch = matchAdvisoryCheckRun(run, ignoredCheckRuns); + if (ignoredMatch) { + ignoredCheckDetails.push({ name: ignoredMatch.name, appSlug: ignoredMatch.appSlug, conclusion: (run.conclusion ?? run.status ?? "").toLowerCase() }); + continue; + } const advisoryMatch = matchAdvisoryCheckRun(run, advisoryCheckRuns); if (advisoryMatch) { const advisoryConclusion = (run.conclusion ?? "").toLowerCase(); @@ -3159,7 +3174,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, advisoryHoldDetails, ciCompletenessWarning }; + return { ciState, hasPending, hasVisiblePending: anyRequiredVisiblePending, hasMissingRequiredContext, failingDetails, nonRequiredFailingDetails, advisoryHoldDetails, ignoredCheckDetails, ciCompletenessWarning }; } /** @@ -3183,8 +3198,11 @@ export async function fetchLiveCiAggregate( // #4372: maintainer-declared advisory check-runs (trailing/optional — every existing positional caller passing // requiredContexts+admissionKey stays byte-identical). Excluded from the aggregate; non-passing ⇒ advisoryHoldDetails. advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null, + // #9810: maintainer-declared IGNORED check-runs — excluded from the aggregate entirely, and (unlike advisory) + // never routed to a hold. Same trailing-optional discipline: existing positional callers are unaffected. + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null, ): Promise { - if (!headSha) return { ciState: "unverified", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }; + if (!headSha) return { ciState: "unverified", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], 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[] = []; @@ -3239,6 +3257,7 @@ export async function fetchLiveCiAggregate( statuses, requiredContexts, advisoryCheckRuns, + ignoredCheckRuns, checkRunsIncomplete, statusIncomplete, // Lazily read the check-SUITES backstop only when the reducer finds the cheaper sources fully settled; a fetch @@ -3305,6 +3324,7 @@ export async function fetchLiveCiAggregateViaGraphQl( requiredContexts?: ReadonlySet | null, admissionKey?: GitHubRateLimitAdmissionKey, advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null, + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null, ): Promise { if (!headSha || !token) return null; const parsed = parseBackfillRepoFullName(repoFullName); @@ -3390,6 +3410,7 @@ export async function fetchLiveCiAggregateViaGraphQl( statuses, requiredContexts, advisoryCheckRuns, + ignoredCheckRuns, checkRunsIncomplete: false, statusIncomplete: false, fetchSuites: async () => suites, // already fetched in the same query — never a second round-trip @@ -3410,14 +3431,15 @@ export async function fetchLiveCiAggregatePreferGraphQl( requiredContexts?: ReadonlySet | null, admissionKey?: GitHubRateLimitAdmissionKey, advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null, + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null, ): Promise { 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, advisoryCheckRuns); + const rollup = await fetchLiveCiAggregateViaGraphQl(env, repoFullName, headSha, token, requiredContexts, admissionKey, advisoryCheckRuns, ignoredCheckRuns); if (rollup) return rollup; } - return fetchLiveCiAggregate(env, repoFullName, headSha, token, requiredContexts, admissionKey, advisoryCheckRuns); + return fetchLiveCiAggregate(env, repoFullName, headSha, token, requiredContexts, admissionKey, advisoryCheckRuns, ignoredCheckRuns); } /** @@ -3958,6 +3980,10 @@ export function deserializeCachedCiAggregate( // it is already baked into the cached `ciState`. Pinned end-to-end by pr-detail-durable-cache.test.ts (the // deserialize-[] + invalidation halves) and backfill-2.test.ts (the fresh read re-deriving the hold). advisoryHoldDetails: [], + // #9810: same treatment, and safer still -- ignoredCheckDetails drives NOTHING (informational surfacing + // only), so an empty reconstruction on a cache hit cannot change any disposition. The exclusion's real + // effect is already baked into the cached `ciState`. + ignoredCheckDetails: [], ciCompletenessWarning: cached.ciCompletenessWarning ?? null, }; } catch { diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index 5857d51177..3a38185ec7 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -745,6 +745,7 @@ export const RepositorySettingsSchema = z // the compile-time parity assertion in test/unit/openapi-settings-schema-parity.test.ts. expectedCiContexts: z.array(z.string()).readonly().nullable().optional(), advisoryCheckRuns: z.array(z.object({ name: z.string(), appSlug: z.string() })).readonly().nullable().optional(), + ignoredCheckRuns: z.array(z.object({ name: z.string(), appSlug: z.string() })).readonly().nullable().optional(), copycatGateMode: z.enum(["off", "warn", "label", "block"]).optional(), copycatGateMinScore: z.number().nullable().optional(), gateDryRun: z.boolean().optional(), diff --git a/src/queue/ci-resolution.ts b/src/queue/ci-resolution.ts index 4eaf038374..9c5e2476ae 100644 --- a/src/queue/ci-resolution.ts +++ b/src/queue/ci-resolution.ts @@ -76,6 +76,14 @@ function advisoryCheckRunsKeyPart(advisoryCheckRuns: ReadonlyArray<{ name: strin return JSON.stringify(advisoryCheckRuns.map((c) => `${c.name}\0${c.appSlug}`).sort()); } +// #9810: the ignored list changes the aggregate exactly the way the advisory list does (which runs are excluded +// from ciState/hasPending), so it must be part of the cache key too -- otherwise a stale entry from before the +// change keeps gating on a check the maintainer just ignored. +function ignoredCheckRunsKeyPart(ignoredCheckRuns: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined): string { + if (!ignoredCheckRuns || ignoredCheckRuns.length === 0) return ""; + return JSON.stringify(ignoredCheckRuns.map((c) => `${c.name}\0${c.appSlug}`).sort()); +} + // RC2 + #selfhost-ci-verification: the EFFECTIVE required-status-check contexts for this repo/baseRef, merging // live branch-protection required contexts with the maintainer-configured settings.expectedCiContexts fallback // (mergeRequiredCiContexts — branch protection stays authoritative when readable; expectedCiContexts is the @@ -176,6 +184,7 @@ async function cachedFetchLiveCiAggregate( requiredContexts: ReadonlySet | null | undefined; requiredContextsKey: string; advisoryCheckRuns: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; forceRefresh: boolean; // False when the caller's own required-context lookup FAILED (not merely resolved to "none configured") -- // that fail-open aggregate must never be persisted under the normal key, or a transient lookup error would @@ -194,7 +203,7 @@ async function cachedFetchLiveCiAggregate( } } incr(CI_STATE_CACHE_METRIC, { field: "aggregate", result: args.forceRefresh ? "forced" : "miss" }); - const live = await fetchLiveCiAggregatePreferGraphQl(env, args.repoFullName, args.headSha, args.token, args.requiredContexts, args.admissionKey, args.advisoryCheckRuns); + const live = await fetchLiveCiAggregatePreferGraphQl(env, args.repoFullName, args.headSha, args.token, args.requiredContexts, args.admissionKey, args.advisoryCheckRuns, args.ignoredCheckRuns); if (args.requiredContextsResolved) { await writeThroughCiStateCache(env, args.repoFullName, args.prNumber, cached, args.headSha, args.requiredContextsKey, live); } @@ -212,6 +221,7 @@ function fetchLiveCiAggregateWithRequiredContexts( token: string | undefined; expectedCiContexts: ReadonlyArray | null | undefined; advisoryCheckRuns: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; forceRefresh: boolean; admissionKey?: GitHubRateLimitAdmissionKey | undefined; }, @@ -233,7 +243,7 @@ function fetchLiveCiAggregateWithRequiredContexts( // #4372: the advisory-check-runs config changes the aggregate but is NOT part of the resolved required // contexts, so fold its fingerprint into the durable cache key alongside them — else a config change // would keep serving a stale aggregate computed against the old advisory list. - requiredContextsKey: `${resolvedRequiredContextsKeyPart(requiredContexts)}|adv:${advisoryCheckRunsKeyPart(args.advisoryCheckRuns)}`, + requiredContextsKey: `${resolvedRequiredContextsKeyPart(requiredContexts)}|adv:${advisoryCheckRunsKeyPart(args.advisoryCheckRuns)}|ign:${ignoredCheckRunsKeyPart(args.ignoredCheckRuns)}`, advisoryCheckRuns: args.advisoryCheckRuns, forceRefresh: args.forceRefresh, requiredContextsResolved: resolved, @@ -253,10 +263,11 @@ export function cachedLiveCiAggregate( token: string | undefined; expectedCiContexts: ReadonlyArray | null | undefined; advisoryCheckRuns: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; admissionKey?: GitHubRateLimitAdmissionKey | undefined; }, ): Promise { - const key = liveFactKey(args.repoFullName, args.headSha, args.baseRef, liveFactTokenPart(args.token), `${expectedCiContextsKeyPart(args.expectedCiContexts)}|adv:${advisoryCheckRunsKeyPart(args.advisoryCheckRuns)}`); + const key = liveFactKey(args.repoFullName, args.headSha, args.baseRef, liveFactTokenPart(args.token), `${expectedCiContextsKeyPart(args.expectedCiContexts)}|adv:${advisoryCheckRunsKeyPart(args.advisoryCheckRuns)}|ign:${ignoredCheckRunsKeyPart(args.ignoredCheckRuns)}`); const cached = args.facts.ciAggregates.get(key); if (cached) return cached; const next = evictLiveFactOnReject( @@ -290,10 +301,11 @@ export function refreshLiveCiAggregate( token: string | undefined; expectedCiContexts: ReadonlyArray | null | undefined; advisoryCheckRuns: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; admissionKey?: GitHubRateLimitAdmissionKey | undefined; }, ): Promise { - const key = liveFactKey(args.repoFullName, args.headSha, args.baseRef, liveFactTokenPart(args.token), `${expectedCiContextsKeyPart(args.expectedCiContexts)}|adv:${advisoryCheckRunsKeyPart(args.advisoryCheckRuns)}`); + const key = liveFactKey(args.repoFullName, args.headSha, args.baseRef, liveFactTokenPart(args.token), `${expectedCiContextsKeyPart(args.expectedCiContexts)}|adv:${advisoryCheckRunsKeyPart(args.advisoryCheckRuns)}|ign:${ignoredCheckRunsKeyPart(args.ignoredCheckRuns)}`); const next = evictLiveFactOnReject( args.facts.ciAggregates, key, @@ -439,9 +451,11 @@ export function reuseOrRefreshLiveCiAggregate( // #4372: trailing/optional so existing positional callers stay byte-identical (advisoryCheckRuns undefined ⇒ // exclusion off, today's behavior). Folded into the memo key alongside expectedCiContexts, like the entry points. advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null, + // #9810: same trailing-optional discipline -- undefined ⇒ no ignore list ⇒ today's behavior exactly. + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null, ): Promise { - const key = liveFactKey(repoFullName, headSha, baseRef, liveFactTokenPart(token), `${expectedCiContextsKeyPart(expectedCiContexts)}|adv:${advisoryCheckRunsKeyPart(advisoryCheckRuns)}`); + const key = liveFactKey(repoFullName, headSha, baseRef, liveFactTokenPart(token), `${expectedCiContextsKeyPart(expectedCiContexts)}|adv:${advisoryCheckRunsKeyPart(advisoryCheckRuns)}|ign:${ignoredCheckRunsKeyPart(ignoredCheckRuns)}`); const cached = facts.forcedCiAggregateKeys.has(key) ? facts.ciAggregates.get(key) : undefined; if (cached) return cached; - return refreshLiveCiAggregate(env, { repoFullName, facts, prNumber, headSha, baseRef, token, expectedCiContexts, advisoryCheckRuns, admissionKey }); + return refreshLiveCiAggregate(env, { repoFullName, facts, prNumber, headSha, baseRef, token, expectedCiContexts, advisoryCheckRuns, ignoredCheckRuns, admissionKey }); } diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 4b6b9d7450..9b99ea7c68 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -3272,6 +3272,7 @@ async function runAgentMaintenancePlanAndExecute( settings.expectedCiContexts, admissionKey, settings.advisoryCheckRuns, + settings.ignoredCheckRuns, ); // #2137: informational-only nudge for the operator — never affects the disposition below (ciState is // unchanged). recordAuditEvent is a DB write with its own internal failure handling; a failure here must @@ -4011,6 +4012,7 @@ async function runAgentMaintenancePlanAndExecute( // plan was evaluated against, or the two can disagree on ciState. requiredCiContexts: requiredContexts, advisoryCheckRuns: settings.advisoryCheckRuns, // #4372: same exclusion the plan used, for step-8 re-verify + ignoredCheckRuns: settings.ignoredCheckRuns, // #9810: ditto for the ignore list // #3472 split-brain: the executor's own live manual-review hold guard (immediately before approve/merge) // must check the SAME configured label the planner itself resolves labels.manualReview from. manualReviewLabel: settings.manualReviewLabel, diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index b230c83af9..01f43f27f4 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -232,6 +232,8 @@ export type AgentActionExecutionContext = { // exclusion the planning pass used — otherwise the executor could see a maintainer-declared advisory check as // failing/pending and block a merge the planner already cleared. Absent ⇒ exclusion off, unchanged from before. advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; + // #9810: the ignore list, resolved by the CALLER exactly like advisoryCheckRuns above. + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; // settings.manualReviewLabel (#3472 split-brain), resolved by the CALLER (same "the executor has no settings // access" shape as requiredCiContexts above): the approve/merge live label guard (step 7b below) needs the // SAME configured label name the planner itself resolves labels.manualReview from (agent-actions.ts), so a @@ -589,7 +591,7 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE const admissionKey = githubRateLimitAdmissionKeyForToken(env, ciToken, ctx.installationId); const [liveCi, liveMergeableState, liveThreadBlockers, liveWinnerState] = await Promise.all([ requiresLiveCiRecheck - ? fetchLiveCiAggregate(env, ctx.repoFullName, expectedHeadSha, ciToken, ctx.requiredCiContexts ?? null, admissionKey, ctx.advisoryCheckRuns ?? null) + ? fetchLiveCiAggregate(env, ctx.repoFullName, expectedHeadSha, ciToken, ctx.requiredCiContexts ?? null, admissionKey, ctx.advisoryCheckRuns ?? null, ctx.ignoredCheckRuns ?? null) : Promise.resolve(undefined), requiresLiveMergeableRecheck || requiresLiveApproveMergeableRecheck ? fetchLivePullRequestMergeState(env, ctx.repoFullName, ctx.pullNumber, ciToken, admissionKey) diff --git a/src/services/agent-approval-queue.ts b/src/services/agent-approval-queue.ts index 89b4fc1176..521d46d5f3 100644 --- a/src/services/agent-approval-queue.ts +++ b/src/services/agent-approval-queue.ts @@ -278,7 +278,7 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de shouldRecheckLiveDisposition ? fetchRequiredStatusContexts(env, pending.repoFullName, pr!.baseRef, token, admissionKey) .then((branchProtectionContexts) => mergeRequiredCiContexts(branchProtectionContexts, settings.expectedCiContexts)) - .then((requiredContexts) => fetchLiveCiAggregate(env, pending.repoFullName, pr!.headSha, token, requiredContexts, admissionKey, settings.advisoryCheckRuns)) + .then((requiredContexts) => fetchLiveCiAggregate(env, pending.repoFullName, pr!.headSha, token, requiredContexts, admissionKey, settings.advisoryCheckRuns, settings.ignoredCheckRuns)) : Promise.resolve(undefined), shouldRecheckLiveDisposition ? fetchLivePullRequestMergeState(env, pending.repoFullName, pending.pullNumber, token, admissionKey) : Promise.resolve(undefined), shouldRecheckLiveDisposition ? fetchLivePullRequestReviewDecision(env, pending.repoFullName, pending.pullNumber, token, admissionKey) : Promise.resolve(undefined), @@ -540,6 +540,7 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de // (above) evaluated against. Re-fetch so branch-protection changes remain authoritative at accept time. requiredCiContexts: executionRequiredContexts, advisoryCheckRuns: settings.advisoryCheckRuns, // #4372: same exclusion the plan used, for step-8 re-verify + ignoredCheckRuns: settings.ignoredCheckRuns, // #9810: ditto for the ignore list // #3472 split-brain: a staged approve/merge can sit queued long enough for a SIBLING pass to publish a // manual-review hold on this same PR/head before the maintainer accepts — the executor's own live guard // (step 7b of executeAgentMaintenanceActions) needs the configured label to check for. diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 4026164e76..833e52a048 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -538,6 +538,7 @@ function applyGateConfigOverrides(effective: RepositorySettings, gate: FocusMani if (gate.claCheckRunAppSlug !== null) effective.claCheckRunAppSlug = gate.claCheckRunAppSlug; if (gate.expectedCiContexts !== null) effective.expectedCiContexts = gate.expectedCiContexts; if (gate.advisoryCheckRuns !== null) effective.advisoryCheckRuns = gate.advisoryCheckRuns; + if (gate.ignoredCheckRuns !== null) effective.ignoredCheckRuns = gate.ignoredCheckRuns; if (gate.copycatMode !== null) effective.copycatGateMode = gate.copycatMode; if (gate.copycatMinScore !== null) effective.copycatGateMinScore = gate.copycatMinScore; } diff --git a/src/types.ts b/src/types.ts index 82e4ef4531..5331a2846e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1031,6 +1031,11 @@ export type RepositorySettings = { * `null`/absent/empty ⇒ byte-identical to today for every repo that doesn't opt in. Config-as-code only — * no DB column; set via `.loopover.yml gate.advisoryCheckRuns`. */ advisoryCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; + /** `gate.ignoredCheckRuns` (#9810): third-party check-runs EXCLUDED from CI resolution entirely -- never + * gate, never pend, never hold. Spoof-resistant `{ name, appSlug }` match, same as advisoryCheckRuns. For a + * check whose verdict carries no signal for this repo (e.g. a vendor contributor-trust score) while other + * checks from the same app stay meaningful. Config-as-code only; set via `.loopover.yml gate.ignoredCheckRuns`. */ + ignoredCheckRuns?: ReadonlyArray<{ name: string; appSlug: string }> | null | undefined; /** Dry-run disposition (#gate-dryrun). When true, the gate renders the would-be merge/close/manual verdict (every * advisory sub-gate promoted to block) WITHOUT enforcing — the posted check stays non-blocking. Lets advisory mode * preview exactly what it would do before the maintainer flips to real enforcement. Default off. diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index c48ea87038..3944865e9c 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -51,7 +51,7 @@ vi.mock("../../src/github/app", async (importOriginal) => ({ // named winning sibling is still open, i.e. the duplicate justification still holds) for the same reason. vi.mock("../../src/github/backfill", async (importOriginal) => ({ ...(await importOriginal()), - fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null })), + fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null })), fetchLivePullRequestMergeState: vi.fn(async () => "clean" as const), fetchLiveReviewThreadBlockers: vi.fn(async () => [{ title: "still unresolved", scannerFinding: false }]), fetchLivePullRequestState: vi.fn(async () => "open" as const), @@ -579,7 +579,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { it("LIVE heuristic close is denied when live CI has since turned green (#2128)", async () => { const env = createTestEnv({}); const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic", closeRequiresCiState: "failed" }; - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const outcomes = await executeAgentMaintenanceActions(env, ctx(), [heuristicClose]); expect(outcomes[0]?.outcome).toBe("denied"); expect(outcomes[0]?.detail).toContain("CI state changed since planning (now: passed)"); @@ -589,7 +589,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { it("LIVE heuristic close proceeds when live CI is still failing (#2128)", async () => { const env = createTestEnv({}); const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic", closeRequiresCiState: "failed" }; - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const outcomes = await executeAgentMaintenanceActions(env, ctx(), [heuristicClose]); expect(outcomes[0]?.outcome).toBe("completed"); expect(closePullRequest).toHaveBeenCalledWith(env, 123, "owner/repo", 7); @@ -605,7 +605,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { const replayed = pendingActionToPlanned({ actionClass: "close", params: persisted, reason: heuristicClose.reason }); expect(replayed.closeKind).toBe("heuristic"); expect(replayed.closeRequiresCiState).toBe("failed"); - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const outcomes = await executeAgentMaintenanceActions(env, ctx(), [replayed]); expect(outcomes[0]?.outcome).toBe("denied"); expect(outcomes[0]?.detail).toContain("CI state changed since planning (now: passed)"); @@ -643,7 +643,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { // could execute after CI recovers. const env = createTestEnv({}); const legacyHeuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic" }; - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const outcomes = await executeAgentMaintenanceActions(env, ctx(), [legacyHeuristicClose]); expect(outcomes[0]?.outcome).toBe("denied"); expect(outcomes[0]?.detail).toContain("CI state changed since planning (now: passed)"); @@ -653,7 +653,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { it("a LEGACY heuristic close (closeRequiresCiState absent) still proceeds when live CI is genuinely still failing, matching the old pre-#2478 behavior", async () => { const env = createTestEnv({}); const legacyHeuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic" }; - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const outcomes = await executeAgentMaintenanceActions(env, ctx(), [legacyHeuristicClose]); expect(outcomes[0]?.outcome).toBe("completed"); expect(closePullRequest).toHaveBeenCalledWith(env, 123, "owner/repo", 7); @@ -927,7 +927,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { it("LIVE merge is denied when live CI has since turned failing (#2128)", async () => { const env = createTestEnv({}); - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const outcomes = await executeAgentMaintenanceActions(env, ctx({ installationId: 127 }), [merge]); expect(outcomes[0]?.outcome).toBe("denied"); expect(outcomes[0]?.detail).toContain("live CI is no longer passing (now: failed)"); @@ -936,7 +936,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { it("REGRESSION (#2364): LIVE merge is denied when live CI has since become pending, not just failed", async () => { const env = createTestEnv({}); - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "pending", hasPending: true, hasVisiblePending: true, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "pending", hasPending: true, hasVisiblePending: true, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]); expect(outcomes[0]?.outcome).toBe("denied"); expect(outcomes[0]?.detail).toContain("live CI is no longer passing (now: pending)"); @@ -945,7 +945,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { it("REGRESSION (#2364): LIVE merge is denied when live CI has since become unverified (unreadable), not just failed", async () => { const env = createTestEnv({}); - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "unverified", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "unverified", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]); expect(outcomes[0]?.outcome).toBe("denied"); expect(outcomes[0]?.detail).toContain("live CI is no longer passing (now: unverified)"); @@ -959,14 +959,14 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { const env = createTestEnv({}); const outcomes = await executeAgentMaintenanceActions(env, ctx({ requiredCiContexts: new Set(["build", "test"]) }), [merge]); expect(outcomes[0]?.outcome).toBe("completed"); - expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "sha7", expect.any(String), new Set(["build", "test"]), expect.any(String), null); + expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "sha7", expect.any(String), new Set(["build", "test"]), expect.any(String), null, null); }); it("passes null (fold-all) requiredContexts when ctx.requiredCiContexts is unset — unchanged pre-existing behavior", async () => { const env = createTestEnv({}); const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]); expect(outcomes[0]?.outcome).toBe("completed"); - expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "sha7", expect.any(String), null, expect.any(String), null); + expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "sha7", expect.any(String), null, expect.any(String), null, null); }); it("the live CI re-check fails open on a token-mint error — it is defense-in-depth, not the primary gate (#2128)", async () => { diff --git a/test/unit/agent-approval-queue.test.ts b/test/unit/agent-approval-queue.test.ts index a3ebe7eda3..f38c30cf2c 100644 --- a/test/unit/agent-approval-queue.test.ts +++ b/test/unit/agent-approval-queue.test.ts @@ -37,7 +37,7 @@ vi.mock("../../src/github/app", async (importOriginal) => ({ // override these to exercise the staleness-supersede / staleness-denial paths. vi.mock("../../src/github/backfill", async (importOriginal) => ({ ...(await importOriginal()), - fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null })), + fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null })), fetchRequiredStatusContexts: vi.fn(async () => null), fetchLivePullRequestMergeState: vi.fn(async () => "clean"), fetchLivePullRequestReviewDecision: vi.fn(async () => undefined), @@ -552,7 +552,7 @@ describe("agent approval queue (#779)", () => { await seedInstallation(env); await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "x" }); const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" }); - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); // Also exercise a best-effort-failed mergeable/review read (undefined) alongside the CI failure — the // audit metadata's nullish fallback must not throw, and ciState alone is still sufficient to deny. vi.mocked(fetchLivePullRequestMergeState).mockResolvedValueOnce(undefined); @@ -625,7 +625,7 @@ describe("agent approval queue (#779)", () => { const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" }); // A FULFILLED "pending" read is a genuine non-passing signal — distinct from a REJECTED read (fail-open, // covered by the "ITSELF rejects" test below), which must NOT supersede. - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "pending", hasPending: true, hasVisiblePending: true, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "pending", hasPending: true, hasVisiblePending: true, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); expect(result.status).toBe("rejected"); @@ -818,7 +818,7 @@ describe("agent approval queue (#779)", () => { await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "x" }); const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" }); vi.mocked(fetchLivePullRequestMergeState).mockRejectedValueOnce(new Error("GitHub API transient 502")); - vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); expect(result.status).toBe("rejected"); @@ -953,7 +953,7 @@ describe("agent approval queue (#779)", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); const { action } = await createPendingAgentActionIfAbsent(env, { diff --git a/test/unit/backfill-2.test.ts b/test/unit/backfill-2.test.ts index be5fb72baa..faf03ffaa1 100644 --- a/test/unit/backfill-2.test.ts +++ b/test/unit/backfill-2.test.ts @@ -177,7 +177,7 @@ describe("GitHub backfill", () => { const aggregate = await fetchLiveCiAggregate(env, "JSONbored/gittensory", null, "public-token", null); - expect(aggregate).toEqual({ ciState: "unverified", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null }); + expect(aggregate).toEqual({ ciState: "unverified", hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null }); expect(fetchSpy).not.toHaveBeenCalled(); }); @@ -607,6 +607,87 @@ describe("GitHub backfill", () => { expect(aggregate.advisoryHoldDetails).toEqual([]); }); + it("#9810: an IGNORED failing check is excluded entirely — no gate failure AND no hold (unlike advisory)", async () => { + // The motivating case: a vendor app posts both a real security scan and a heuristic contributor-trust + // score. The trust score fails for perfectly good contributors; under advisoryCheckRuns that still + // converted every one of their clean PRs into a manual review. Ignored means "as if it did not exist". + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + stubChecks([ + { name: "validate", status: "completed", conclusion: "success", app: { slug: "github-actions" } }, + { name: "Contributor trust", status: "completed", conclusion: "action_required", app: { slug: "example-security-app" } }, + ]); + const aggregate = await fetchLiveCiAggregate(env, "acme/widget", "sha-ign-1", "public-token", new Set(["validate", "Contributor trust"]), undefined, null, [ + { name: "Contributor trust", appSlug: "example-security-app" }, + ]); + expect(aggregate.ciState).toBe("passed"); + expect(aggregate.advisoryHoldDetails).toEqual([]); // the decisive difference from advisory + expect(aggregate.ignoredCheckDetails).toEqual([{ name: "Contributor trust", appSlug: "example-security-app", conclusion: "action_required" }]); + }); + + it("#9810: a SIBLING check from the same app still gates — ignoring one check never disarms the others", async () => { + // Keeping the security scan's protection while dropping the trust check's noise is the whole point. + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + stubChecks([ + { name: "validate", status: "completed", conclusion: "success", app: { slug: "github-actions" } }, + { name: "Contributor trust", status: "completed", conclusion: "failure", app: { slug: "example-security-app" } }, + { name: "Security scan", status: "completed", conclusion: "failure", app: { slug: "example-security-app" } }, + ]); + const aggregate = await fetchLiveCiAggregate(env, "acme/widget", "sha-ign-2", "public-token", new Set(["validate", "Security scan"]), undefined, null, [ + { name: "Contributor trust", appSlug: "example-security-app" }, + ]); + expect(aggregate.ciState).toBe("failed"); + expect(aggregate.failingDetails.map((d) => d.name)).toEqual(["Security scan"]); + }); + + it("#9810: an ignored check still PENDING does not hold the gate as 'still running'", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + stubChecks([ + { name: "validate", status: "completed", conclusion: "success", app: { slug: "github-actions" } }, + { name: "Contributor trust", status: "in_progress", conclusion: null, app: { slug: "example-security-app" } }, + ]); + const aggregate = await fetchLiveCiAggregate(env, "acme/widget", "sha-ign-3", "public-token", new Set(["validate"]), undefined, null, [ + { name: "Contributor trust", appSlug: "example-security-app" }, + ]); + expect(aggregate.ciState).toBe("passed"); + expect(aggregate.hasPending).toBe(false); + }); + + it("#9810: IGNORE wins over ADVISORY when a check is listed in both (the stronger, more explicit intent)", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + stubChecks([ + { name: "validate", status: "completed", conclusion: "success", app: { slug: "github-actions" } }, + { name: "Contributor trust", status: "completed", conclusion: "failure", app: { slug: "example-security-app" } }, + ]); + const entry = [{ name: "Contributor trust", appSlug: "example-security-app" }]; + const aggregate = await fetchLiveCiAggregate(env, "acme/widget", "sha-ign-4", "public-token", new Set(["validate"]), undefined, entry, entry); + expect(aggregate.advisoryHoldDetails).toEqual([]); // NOT held + expect(aggregate.ignoredCheckDetails).toHaveLength(1); + }); + + it("#9810: a name-only match (no producing app slug) is NOT ignored — spoof resistance matches the advisory path", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + stubChecks([ + { name: "validate", status: "completed", conclusion: "success", app: { slug: "github-actions" } }, + { name: "Contributor trust", status: "completed", conclusion: "failure" }, // no app ⇒ untrusted + ]); + const aggregate = await fetchLiveCiAggregate(env, "acme/widget", "sha-ign-5", "public-token", new Set(["validate", "Contributor trust"]), undefined, null, [ + { name: "Contributor trust", appSlug: "example-security-app" }, + ]); + expect(aggregate.ciState).toBe("failed"); + expect(aggregate.ignoredCheckDetails).toEqual([]); + }); + + it("#9810: with NO ignoredCheckRuns configured, behavior is byte-identical to today", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); + stubChecks([ + { name: "validate", status: "completed", conclusion: "success", app: { slug: "github-actions" } }, + { name: "Contributor trust", status: "completed", conclusion: "failure", app: { slug: "example-security-app" } }, + ]); + const aggregate = await fetchLiveCiAggregate(env, "acme/widget", "sha-ign-6", "public-token", new Set(["validate", "Contributor trust"])); + expect(aggregate.ciState).toBe("failed"); + expect(aggregate.ignoredCheckDetails).toEqual([]); + }); + it("#4372: a run whose NAME matches but carries NO producing app slug is NOT treated as advisory (a name-only match is spoofable)", async () => { const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" }); stubChecks([ diff --git a/test/unit/ci-resolution.test.ts b/test/unit/ci-resolution.test.ts index 9dece4218f..a48e90f17d 100644 --- a/test/unit/ci-resolution.test.ts +++ b/test/unit/ci-resolution.test.ts @@ -83,7 +83,7 @@ describe("cachedLiveCiAggregate request-scoped memoization (#4498)", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); const facts = emptyFacts(); @@ -116,7 +116,7 @@ describe("cachedLiveCiAggregate request-scoped memoization (#4498)", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); const facts = emptyFacts(); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 03bbe5f32a..0777ba7c16 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -341,6 +341,7 @@ describe(".loopover.yml.example field-exhaustiveness (#1670)", () => { claCheckRunAppSlug: "checkRunAppSlug:", expectedCiContexts: "expectedCiContexts:", advisoryCheckRuns: "advisoryCheckRuns:", + ignoredCheckRuns: "ignoredCheckRuns:", aiJudgmentBlockersMode: "aiJudgmentBlockers:", copycatMode: "copycat:", copycatMinScore: "copycat:", @@ -998,7 +999,7 @@ describe("compileFocusManifestPolicy", () => { issueDiscoveryPolicy: "neutral", maintainerNotes: [], publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], - gate: { present: false, enabled: null, checkMode: null, pack: null, closeAuditHoldoutPct: 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, aiReviewSalvageabilityMinScore: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, contentLaneDeliverable: null, backtestRegression: 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 }, + gate: { present: false, enabled: null, checkMode: null, pack: null, closeAuditHoldoutPct: 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, aiReviewSalvageabilityMinScore: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, contentLaneDeliverable: null, backtestRegression: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, staleBaseAheadByThreshold: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, ignoredCheckRuns: 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, activeReviewReconciliation: 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 }, @@ -1192,7 +1193,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, closeAuditHoldoutPct: 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, aiReviewSalvageabilityMinScore: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, contentLaneDeliverable: null, backtestRegression: 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 }); + expect(m.gate).toEqual({ present: true, enabled: null, checkMode: null, pack: null, closeAuditHoldoutPct: 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, aiReviewSalvageabilityMinScore: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, contentLaneDeliverable: null, backtestRegression: null, manifestPolicy: null, dryRun: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, staleBaseAheadByThreshold: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, ignoredCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }); }); it("parses gate.mergeReadiness, round-trips it, and warns on a bad value (#822)", () => { diff --git a/test/unit/mcp-automation-state.test.ts b/test/unit/mcp-automation-state.test.ts index 35144f500e..f73bea98eb 100644 --- a/test/unit/mcp-automation-state.test.ts +++ b/test/unit/mcp-automation-state.test.ts @@ -42,7 +42,7 @@ vi.mock("../../src/github/pr-freshness", async (importOriginal) => { // dedicated staleness-supersede test coverage lives in agent-approval-queue.test.ts, not this MCP-surface file. vi.mock("../../src/github/backfill", async (importOriginal) => ({ ...(await importOriginal()), - fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null })), + fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null })), fetchLivePullRequestMergeState: vi.fn(async () => "clean"), fetchLivePullRequestReviewDecision: vi.fn(async () => undefined), })); diff --git a/test/unit/pr-detail-durable-cache.test.ts b/test/unit/pr-detail-durable-cache.test.ts index 0bcf42c7ce..b97ac11613 100644 --- a/test/unit/pr-detail-durable-cache.test.ts +++ b/test/unit/pr-detail-durable-cache.test.ts @@ -555,7 +555,7 @@ describe("durable CI-state cache (#selfhost-ci-verification)", () => { hasMissingRequiredContext: false, failingDetails: [{ name: "ci/build", summary: "failed", detailsUrl: "https://ci.example.test/1" }], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }; @@ -695,7 +695,7 @@ describe("durable CI-state cache (#selfhost-ci-verification)", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); }); diff --git a/test/unit/queue-2.test.ts b/test/unit/queue-2.test.ts index 1ef9edfe3f..fbe0a7ec43 100644 --- a/test/unit/queue-2.test.ts +++ b/test/unit/queue-2.test.ts @@ -474,7 +474,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); @@ -570,7 +570,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); @@ -2260,7 +2260,7 @@ describe("queue processors", () => { hasMissingRequiredContext: true, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { diff --git a/test/unit/queue-4.test.ts b/test/unit/queue-4.test.ts index 033facaa19..cf26c294ee 100644 --- a/test/unit/queue-4.test.ts +++ b/test/unit/queue-4.test.ts @@ -3169,7 +3169,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -3350,7 +3350,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -3525,7 +3525,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -3679,7 +3679,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -3792,7 +3792,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -3935,7 +3935,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -4114,7 +4114,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -4637,7 +4637,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -4790,7 +4790,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -4959,7 +4959,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -5137,7 +5137,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -5467,7 +5467,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -5638,7 +5638,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { diff --git a/test/unit/queue-5.test.ts b/test/unit/queue-5.test.ts index eaa979f4b4..1e63149a29 100644 --- a/test/unit/queue-5.test.ts +++ b/test/unit/queue-5.test.ts @@ -6125,7 +6125,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); const posted = { count: 0, body: "" }; diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 013b2b4c47..54be4cf857 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1796,7 +1796,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -1848,7 +1848,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -1910,7 +1910,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -1988,7 +1988,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let liveHeadSha = "a7"; @@ -2066,7 +2066,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -2137,7 +2137,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -2193,7 +2193,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -2257,7 +2257,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -2312,7 +2312,7 @@ describe("queue processors", () => { hasMissingRequiredContext: true, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -2381,7 +2381,7 @@ describe("queue processors", () => { hasMissingRequiredContext: true, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -2447,7 +2447,7 @@ describe("queue processors", () => { hasMissingRequiredContext: true, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -2510,7 +2510,7 @@ describe("queue processors", () => { hasMissingRequiredContext: true, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let gateChecks = 0; @@ -2559,7 +2559,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: "CI resolved to passed with no branch-protection required checks configured — cannot verify every expected workflow ran.", }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -2603,7 +2603,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -2710,7 +2710,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); @@ -2934,7 +2934,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -2972,7 +2972,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -3020,7 +3020,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -3080,7 +3080,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -3130,7 +3130,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); let branchProtectionReadable = false; @@ -3192,7 +3192,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -3243,7 +3243,7 @@ describe("queue processors", () => { hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], - advisoryHoldDetails: [], + advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null, }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { diff --git a/test/unit/routes-agent-approval.test.ts b/test/unit/routes-agent-approval.test.ts index 6a66ecaa3e..89ae61cc94 100644 --- a/test/unit/routes-agent-approval.test.ts +++ b/test/unit/routes-agent-approval.test.ts @@ -37,7 +37,7 @@ vi.mock("../../src/github/backfill", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ciCompletenessWarning: null })), + fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, hasMissingRequiredContext: false, failingDetails: [], nonRequiredFailingDetails: [], advisoryHoldDetails: [], ignoredCheckDetails: [], ciCompletenessWarning: null })), }; }); From cfe35e5383e7b0e14577b78eff49e35abee9a71c Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 03:58:35 -0700 Subject: [PATCH 2/2] fix(gate): register ignoredCheckRuns in gate-presence, and stop type-only files failing codecov/patch Two things Codecov's patch report surfaced. REAL BUG: gate.present did not consider ignoredCheckRuns, so a manifest whose gate block set ONLY that field parsed as present:false -- the block would be treated as absent and the ignore silently dropped. Found by adding the parse/round-trip/resolve test the field was missing (the serialize branch was the 3 uncovered engine lines); the test failed on the presence assertion before it ever reached serialization. Exactly the class 99% patch coverage exists for. ARTIFACT: the other 4 flagged lines are in three engine `types/*.ts` modules that contain ZERO runtime declarations. v8 instruments nothing in a pure type file, so it reports 0% and any added line counts as uncovered -- the same artifact src/env.d.ts is already ignored for in codecov.yml. Ignore the three, and guard the claim: a new test fails if any ignored path ever gains a runtime declaration, and fails if a listed path stops being listed, so the ignore can never quietly start hiding real code. Mutation-tested in both directions. --- codecov.yml | 8 ++++ .../loopover-engine/src/focus-manifest.ts | 1 + test/unit/codecov-ignore-type-only.test.ts | 44 +++++++++++++++++++ test/unit/focus-manifest.test.ts | 34 ++++++++++++++ 4 files changed, 87 insertions(+) create mode 100644 test/unit/codecov-ignore-type-only.test.ts diff --git a/codecov.yml b/codecov.yml index 1aa928209a..db59ca5976 100644 --- a/codecov.yml +++ b/codecov.yml @@ -71,6 +71,14 @@ comment: # matches the local report. ignore: - "src/env.d.ts" + # Type-ONLY declaration modules (#9810): zero runtime statements, so v8 instruments nothing and the whole + # file reports 0% — the same artifact src/env.d.ts is ignored for above. Adding a single field to one of + # these otherwise fails codecov/patch on lines that can never execute. Guarded, not trusted: the + # "codecov ignore list stays honest" test in test/unit/codecov-ignore-type-only.test.ts fails if any path + # listed here ever gains a runtime declaration, so an ignore can never quietly start hiding real code. + - "packages/loopover-engine/src/types/manifest-deps-types.ts" + - "packages/loopover-engine/src/types/predicted-gate-types.ts" + - "packages/loopover-engine/src/types/reward-risk-types.ts" - "apps/**" - "test/**" - "scripts/**" diff --git a/packages/loopover-engine/src/focus-manifest.ts b/packages/loopover-engine/src/focus-manifest.ts index 8d016d8f8c..f01546976b 100644 --- a/packages/loopover-engine/src/focus-manifest.ts +++ b/packages/loopover-engine/src/focus-manifest.ts @@ -1970,6 +1970,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu gate.claCheckRunAppSlug !== null || gate.expectedCiContexts !== null || gate.advisoryCheckRuns !== null || + gate.ignoredCheckRuns !== null || gate.aiJudgmentBlockersMode !== null || gate.copycatMode !== null || gate.copycatMinScore !== null; diff --git a/test/unit/codecov-ignore-type-only.test.ts b/test/unit/codecov-ignore-type-only.test.ts new file mode 100644 index 0000000000..8023d68d9d --- /dev/null +++ b/test/unit/codecov-ignore-type-only.test.ts @@ -0,0 +1,44 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +// #9810: codecov.yml ignores three engine `types/*.ts` modules because they are pure type declarations — +// zero runtime statements, so v8 instruments nothing and the file reports 0%, exactly the artifact +// `src/env.d.ts` is already ignored for. Adding one field to such a file otherwise fails codecov/patch on +// lines that can never execute. +// +// That reasoning holds ONLY while the files stay declaration-only. An ignore entry that quietly starts +// covering real runtime code is precisely the silent-rot failure this repo keeps finding (the +// validate-no-hand-written-js stale-path lesson), so the claim is enforced rather than trusted. + +const TYPE_ONLY_IGNORED_PATHS = [ + "packages/loopover-engine/src/types/manifest-deps-types.ts", + "packages/loopover-engine/src/types/predicted-gate-types.ts", + "packages/loopover-engine/src/types/reward-risk-types.ts", +]; + +/** A top-level runtime declaration — anything v8 could actually execute. Type/interface declarations and + * `export type { … }` re-exports compile away entirely and are deliberately not matched. */ +const RUNTIME_DECLARATION = /^\s*(?:export\s+)?(?:default\s+)?(?:const|let|var|function|class|enum)\s/m; + +describe("codecov ignore list stays honest (#9810)", () => { + it.each(TYPE_ONLY_IGNORED_PATHS)("%s is still declaration-only", (path) => { + const source = readFileSync(path, "utf8"); + const offending = source + .split("\n") + .map((line, index) => ({ line, number: index + 1 })) + .filter((entry) => RUNTIME_DECLARATION.test(entry.line)); + // Named in the failure so the fix is obvious: either drop the codecov ignore and cover the new code, or + // move that code to a module that is measured. + expect({ path, runtimeDeclarations: offending.map((entry) => `${entry.number}: ${entry.line.trim().slice(0, 60)}`) }).toEqual({ + path, + runtimeDeclarations: [], + }); + }); + + it("INVARIANT: every path this test claims to guard is actually listed in codecov.yml", () => { + // Without this, renaming a file in codecov.yml but not here would leave the guard watching nothing — + // green, and guarding air. + const codecov = readFileSync("codecov.yml", "utf8"); + for (const path of TYPE_ONLY_IGNORED_PATHS) expect({ path, listed: codecov.includes(`"${path}"`) }).toEqual({ path, listed: true }); + }); +}); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 0777ba7c16..acc35e2941 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -1630,6 +1630,40 @@ describe("parseFocusManifest gate config", () => { expect(over.warnings.some((w) => /gate\.aiReview\.reviewers" is capped/.test(w))).toBe(true); }); + it("parses gate.ignoredCheckRuns, makes the gate present, round-trips + resolves it, and drops spoofable entries (#9810)", () => { + const m = parseFocusManifest({ gate: { ignoredCheckRuns: [{ name: "Contributor trust", appSlug: "example-security-app" }] } }); + expect(m.gate.present).toBe(true); + expect(m.gate.ignoredCheckRuns).toEqual([{ name: "Contributor trust", appSlug: "example-security-app" }]); + // Round-trip through gateConfigToJson: the serialize branch must emit the field, or a manifest that + // ignores a check silently loses that decision on the next reload. + expect(parseFocusManifest({ gate: gateConfigToJson(m.gate) }).gate).toEqual(m.gate); + const eff = resolveEffectiveSettings({ ignoredCheckRuns: undefined } as unknown as RepositorySettings, m); + expect(eff.ignoredCheckRuns).toEqual([{ name: "Contributor trust", appSlug: "example-security-app" }]); + // Absent ⇒ null ⇒ the DB/default value is left untouched (same contract as every other manifest field). + const noFlag = parseFocusManifest({ gate: { claMode: "advisory" } }); + expect(noFlag.gate.ignoredCheckRuns).toBeNull(); + expect(resolveEffectiveSettings({ ignoredCheckRuns: [{ name: "Existing", appSlug: "existing-app" }] } as unknown as RepositorySettings, noFlag).ignoredCheckRuns).toEqual([ + { name: "Existing", appSlug: "existing-app" }, + ]); + // Non-array ⇒ warns, stays null. + expect(parseFocusManifest({ gate: { ignoredCheckRuns: "Contributor trust" } }).warnings.some((w) => /gate\.ignoredCheckRuns/.test(w))).toBe(true); + expect(parseFocusManifest({ gate: { ignoredCheckRuns: "Contributor trust" } }).gate.ignoredCheckRuns).toBeNull(); + // A name-only entry is spoofable and dropped — same anti-spoof contract as advisoryCheckRuns. + const mixed = parseFocusManifest({ gate: { ignoredCheckRuns: [{ name: "Good", appSlug: "good-app" }, { name: "NoSlug" }] } }); + expect(mixed.gate.ignoredCheckRuns).toEqual([{ name: "Good", appSlug: "good-app" }]); + expect(mixed.warnings.some((w) => /gate\.ignoredCheckRuns\[1\]/.test(w))).toBe(true); + // Both lists coexist independently: ignoring one check must not disturb the advisory list. + const both = parseFocusManifest({ + gate: { + advisoryCheckRuns: [{ name: "Security scan", appSlug: "example-security-app" }], + ignoredCheckRuns: [{ name: "Contributor trust", appSlug: "example-security-app" }], + }, + }); + expect(both.gate.advisoryCheckRuns).toEqual([{ name: "Security scan", appSlug: "example-security-app" }]); + expect(both.gate.ignoredCheckRuns).toEqual([{ name: "Contributor trust", appSlug: "example-security-app" }]); + expect(parseFocusManifest({ gate: gateConfigToJson(both.gate) }).gate).toEqual(both.gate); + }); + it("parses gate.advisoryCheckRuns, makes the gate present, round-trips + resolves it, caps entries, and drops entries missing name/appSlug (#4372)", () => { const m = parseFocusManifest({ gate: { advisoryCheckRuns: [{ name: "Third-Party Scan", appSlug: "example-scanner" }, { name: "Trust Check", appSlug: "example-trust" }] } }); expect(m.gate.present).toBe(true);