From 0ce032d2370ac4f64f1be5bed5fcbbc7dfd1681d Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 05:22:09 -0700 Subject: [PATCH] =?UTF-8?q?fix(test):=20repair=20main=20=E2=80=94=20#9813?= =?UTF-8?q?=20changed=20three=20contracts=20and=20updated=20only=20some=20?= =?UTF-8?q?callers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #9813 added `ignoredCheckRuns` as an 8th argument to fetchLiveCiAggregate*, added an `|ign:` fragment to the durable CI-cache key, and documented the field in .loopover.yml.example. Three assertions were never updated with it, so main has been red since it merged: - agent-approval-queue.test.ts x3: toHaveBeenCalledWith pinned the 7-arg shape - queue.test.ts: same arity pin, plus the cache key asserted as "|adv:" only - config-templates.test.ts: config/examples/loopover.full.yml must match .loopover.yml.example from the WHERE IT LIVES marker onward, and only the root example got the new block My error, and specifically a verification error: I ran the touched test files rather than the full suite, so three files that assert on contracts I changed were never executed before merge. The full suite is green here (25,209 passed) -- note mcp-cli-* additionally require `npm run build:mcp` first, which CI does and a bare `vitest run` does not. --- config/examples/loopover.full.yml | 19 +++++++++++++++++++ test/unit/agent-approval-queue.test.ts | 6 +++--- test/unit/queue.test.ts | 7 +++++-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/config/examples/loopover.full.yml b/config/examples/loopover.full.yml index 13096355f8..24056fc0c9 100644 --- a/config/examples/loopover.full.yml +++ b/config/examples/loopover.full.yml @@ -290,6 +290,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/test/unit/agent-approval-queue.test.ts b/test/unit/agent-approval-queue.test.ts index f38c30cf2c..82be71643f 100644 --- a/test/unit/agent-approval-queue.test.ts +++ b/test/unit/agent-approval-queue.test.ts @@ -583,7 +583,7 @@ describe("agent approval queue (#779)", () => { expect(result.status).toBe("accepted"); expect(fetchRequiredStatusContexts).toHaveBeenCalledWith(env, "owner/repo", null, expect.any(String), expect.any(String)); - expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "h7", expect.any(String), new Set(["build", "test"]), expect.any(String), undefined); + expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "h7", expect.any(String), new Set(["build", "test"]), expect.any(String), undefined, undefined); }); it("unions branch-protection contexts into the accept-time live CI re-check", async () => { @@ -599,7 +599,7 @@ describe("agent approval queue (#779)", () => { expect(result.status).toBe("accepted"); expect(fetchRequiredStatusContexts).toHaveBeenCalledWith(env, "owner/repo", "main", expect.any(String), expect.any(String)); - expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "h7", expect.any(String), new Set(["branch-required", "build"]), expect.any(String), undefined); + expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "h7", expect.any(String), new Set(["branch-required", "build"]), expect.any(String), undefined, undefined); }); it("falls back to expectedCiContexts when the accept-time branch-protection read fails", async () => { @@ -614,7 +614,7 @@ describe("agent approval queue (#779)", () => { const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); expect(result.status).toBe("accepted"); - expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "h7", expect.any(String), new Set(["build"]), expect.any(String), null); + expect(fetchLiveCiAggregate).toHaveBeenCalledWith(env, "owner/repo", "h7", expect.any(String), new Set(["build"]), expect.any(String), null, null); }); it("accept supersedes a staged merge when live CI has since turned pending, not just failed (#2126)", async () => { diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 54be4cf857..405c2ace0f 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1877,6 +1877,7 @@ describe("queue processors", () => { new Set(["trusted-required-ci"]), "installation:9001", undefined, // #4372: advisoryCheckRuns (unconfigured here) + undefined, // #9810: ignoredCheckRuns (unconfigured here) ); expect(gateChecks).toBeGreaterThan(0); const finalized = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?") @@ -3168,8 +3169,10 @@ describe("queue processors", () => { await processJob(env, { type: "agent-regate-pr", deliveryId: "required-contexts-lookup-recovers", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }); expect(liveCiSpy.mock.calls.length).toBeGreaterThan(liveReadsAfterFailedLookup); expect(await renderMetrics()).toContain('loopover_ci_state_cache_total{field="aggregate",result="miss"} 1'); - // #4372: the durable cache key now folds in the advisory-check-runs fingerprint (empty "|adv:" when unconfigured). - expect(await getPullRequestDetailSyncState(env, "owner/agent-repo", 7)).toMatchObject({ ciState: "passed", ciRequiredContextsKey: `${JSON.stringify(["trusted-required-ci"])}|adv:` }); + // #4372/#9810: the durable cache key folds in BOTH check-run fingerprints — advisory and ignored — + // so a config change to either invalidates the cached aggregate instead of serving a stale verdict. + // Both empty here ("|adv:|ign:") because neither list is configured in this fixture. + expect(await getPullRequestDetailSyncState(env, "owner/agent-repo", 7)).toMatchObject({ ciState: "passed", ciRequiredContextsKey: `${JSON.stringify(["trusted-required-ci"])}|adv:|ign:` }); } finally { liveCiSpy.mockRestore(); }