feat(gate): gate.ignoredCheckRuns — exclude a third-party check from CI resolution entirely - #9813
Merged
Conversation
…CI entirely
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.
Contributor
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | cfe35e5 | Commit Preview URL Branch Preview URL |
Jul 29 2026, 11:01 AM |
❌ 6 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Bundle ReportChanges will increase total bundle size by 674 bytes (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
…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.
This was referenced Jul 29, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…ty scan (#9818) * chore(superagent): disable contributor-trust scoring, keep the security scan The contributor-trust check has been posting `action_required` on established contributors' PRs — shin-core scored 10/100 "dangerous" on PR #9816 — and was a major source of manual-review volume. Two independent reasons to turn it off rather than tune it further: 1. The trustedAuthors exemption does not work. shin-core is listed in it and was still flagged. An exemption that does not exempt is worse than none: it looks handled while every affected PR still stalls. 2. Its verdict has no consumer. LoopOver now lists this check under gate.ignoredCheckRuns (#9813) on all three gate repos, so a non-passing result no longer gates, pends, or holds anything. Leaving it on produces a permanently red check on contributor PRs that nothing acts on — noise that makes good contributors think they have failed something. prScan stays enabled and still gates CI normally: that is the protection worth having, and it is a different check from the same app. trustedAuthors is kept (inert) so the history survives if this is ever revisited. * chore(superagent): drop the trustedAuthors allowlist too It provably does not work (a listed author was still flagged action_required) and is inert with contributorTrust disabled. Keeping it would only suggest the repo has a working exemption mechanism that it does not.
This was referenced Jul 29, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…s parity test requires (#9822) The second way #9813 left main red. `.loopover.yml.example` documents the new `gate.ignoredCheckRuns` field; `config/examples/loopover.full.yml` does not -- and `test/unit/config-templates.test.ts` asserts the two are byte-identical from the WHERE IT LIVES marker onward, precisely so a field cannot be documented in one place and missing from the other. The full example now carries the same block. Nothing else in the two bodies differed.
This was referenced Jul 29, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…n change (#9829) main's CI has been red since #9813 (gate.ignoredCheckRuns) merged -- the last four ci.yml runs on main all failed, and every branch cut from or merged with main inherits it. #9804 went red on these tests with no changes of its own anywhere near them. #9813's implementation is correct; only the expectations were left behind: * fetchLiveCiAggregate / fetchLiveCiAggregatePreferGraphQl gained an eighth parameter (ignoredCheckRuns) while four toHaveBeenCalledWith assertions still passed seven; * the durable cache key gained an `|ign:` segment, while queue.test.ts's durable-cache regression still expected `|adv:` alone; * the whole ignoredCheckRuns block (19 lines) was added to .loopover.yml.example but not to config/examples/loopover.full.yml, which config-templates.test.ts requires byte-identical from `# WHERE IT LIVES` onward. No production behaviour changes. This matters beyond a red build: the gate auto-CLOSES a contributor PR on red CI, so a contributor who opened a PR after #9813 landed would be closed for a defect that is not theirs and that they cannot fix -- the same harm #9798 and #9801 describe, except this one does not self-correct. Closes #9828
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…either gate.ignoredCheckRuns (#9813) excluded the check from LoopOver's own CI aggregate, but mergeable_state is GITHUB's computation and stays "unstable" while the check exists at all -- and derivePrDisposition holds unconditionally on unstable. So the ignore was half-effective: the check no longer failed the gate, and the PR was held anyway. Observed live on #9816 immediately after the config flip, reason "mergeable_state is unstable -- non-required check(s) not passing: Contributor trust". Dismiss an unstable state ONLY when the ignore list fully explains it: at least one ignored run concluded non-passing, our aggregate found no non-required failure, and ciState is not failed. Any other unstable cause still holds, and the flag never rescues a PR held for a different reason. Also make the un-itemized unstable message actionable. It used to say only "a non-required check or status is not passing" -- no check name, no next step. GitHub never says why, and the aggregate can legitimately fail to itemize it (a commit status rather than a check-run, an unreadable app page, a run that appeared after CI was read). Name that ambiguity and point at the Checks tab.
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…either gate.ignoredCheckRuns (#9813) excluded the check from LoopOver's own CI aggregate, but mergeable_state is GITHUB's computation and stays "unstable" while the check exists at all -- and derivePrDisposition holds unconditionally on unstable. So the ignore was half-effective: the check no longer failed the gate, and the PR was held anyway. Observed live on #9816 immediately after the config flip, reason "mergeable_state is unstable -- non-required check(s) not passing: Contributor trust". Dismiss an unstable state ONLY when the ignore list fully explains it: at least one ignored run concluded non-passing, our aggregate found no non-required failure, and ciState is not failed. Any other unstable cause still holds, and the flag never rescues a PR held for a different reason. Also make the un-itemized unstable message actionable. It used to say only "a non-required check or status is not passing" -- no check name, no next step. GitHub never says why, and the aggregate can legitimately fail to itemize it (a commit status rather than a check-run, an unreadable app page, a run that appeared after CI was read). Name that ambiguity and point at the Checks tab.
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…nvocation Review blocker on #9821, and correct: resolveReviewKnobs was computed, logged, and then DROPPED. Only selfConsistencyRuns was consumed, so "choose provider, model, effort" was unimplemented at the one place it takes effect -- and the doc comment claiming the fields "ride through to the provider invocation" was false. Apply them where review.ai_model's overrides already land: claudeModel/codexModel/ollamaModel/openaiModel/openaiCompatibleModel/ anthropicModel <- reviewKnobs.model ?? review.ai_model ?? env claudeEffort/codexEffort <- reviewKnobs.effort ?? ... ?? env reviewKnobs wins over review.ai_model because it is strictly more specific: it is the only layer that can differ PER PR, and an escalation existing at all means it should win for the PR that triggered it. Unset ⇒ `??` falls straight through, so a repo using only review.ai_model is untouched. Provider also now governs the BYOK key. The providerKey gate runs before the knobs are resolved and only knew settings.aiReviewProvider, so an ESCALATED provider would have kept using a stored key belonging to a different one. Same rule, re-applied with the resolved value: a mismatch drops the key. Also re-syncs config/examples/loopover.full.yml with .loopover.yml.example -- caught by running the FULL suite this time, which is the same break class that took main down after #9813.
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…either (#9820) * fix(gate): an ignored check must not hold the PR via mergeable_state either gate.ignoredCheckRuns (#9813) excluded the check from LoopOver's own CI aggregate, but mergeable_state is GITHUB's computation and stays "unstable" while the check exists at all -- and derivePrDisposition holds unconditionally on unstable. So the ignore was half-effective: the check no longer failed the gate, and the PR was held anyway. Observed live on #9816 immediately after the config flip, reason "mergeable_state is unstable -- non-required check(s) not passing: Contributor trust". Dismiss an unstable state ONLY when the ignore list fully explains it: at least one ignored run concluded non-passing, our aggregate found no non-required failure, and ciState is not failed. Any other unstable cause still holds, and the flag never rescues a PR held for a different reason. Also make the un-itemized unstable message actionable. It used to say only "a non-required check or status is not passing" -- no check name, no next step. GitHub never says why, and the aggregate can legitimately fail to itemize it (a commit status rather than a check-run, an unreadable app page, a run that appeared after CI was read). Name that ambiguity and point at the Checks tab. * fix(gate): an ignored check must not hold the PR via mergeable_state either gate.ignoredCheckRuns (#9813) excluded the check from LoopOver's own CI aggregate, but mergeable_state is GITHUB's computation and stays "unstable" while the check exists at all -- and derivePrDisposition holds unconditionally on unstable. So the ignore was half-effective: the check no longer failed the gate, and the PR was held anyway. Observed live on #9816 immediately after the config flip, reason "mergeable_state is unstable -- non-required check(s) not passing: Contributor trust". Dismiss an unstable state ONLY when the ignore list fully explains it: at least one ignored run concluded non-passing, our aggregate found no non-required failure, and ciState is not failed. Any other unstable cause still holds, and the flag never rescues a PR held for a different reason. Also make the un-itemized unstable message actionable. It used to say only "a non-required check or status is not passing" -- no check name, no next step. GitHub never says why, and the aggregate can legitimately fail to itemize it (a commit status rather than a check-run, an unreadable app page, a run that appeared after CI was read). Name that ambiguity and point at the Checks tab.
This was referenced Jul 29, 2026
Closed
loopover-orb Bot
pushed a commit
that referenced
this pull request
Jul 29, 2026
…9837) contract:api-schemas:check fails on main: #9813 added ignoredCheckRuns to RepositorySettingsSchema in src/ without committing the regenerated contract copy. Fourth piece of that PR's fallout, after the three #9829 fixed. @loopover/contract is what MCP and miner consume, and api-schemas.ts is what an external client validates a settings write against -- so a client built on the published contract silently strips ignoredCheckRuns: accepted by the server, absent from the schema, configuration that appears to save and then does nothing. Generated output, not hand-edited: `npm run contract:api-schemas`. Closes #9836
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…ard the coverage exposed Codecov flagged 39 uncovered lines on #9821, and closing them found a REAL bug: gateConfigToJson's outer aiReview guard was never taught the two new fields, so a manifest whose aiReview block set ONLY effort/selfConsistencyRuns serialized to NOTHING -- the setting silently vanished on the next snapshot reload. The new round-trip assertion failed on exactly that before touching coverage at all. (An earlier edit had aimed at this guard and hit the presence expression instead; the round-trip test is what caught the difference.) Coverage added: - engine parse: full/partial/invalid/non-mapping guardrailEscalation, invalid effort + runs values warn and stay null, presence flips on EACH of the six fields alone (the #9813 presence-gap class, per field), serialize round-trips full AND partial blocks - resolution: every field lands on effective settings; absent leaves the stored value untouched - END-TO-END escalation: env self-consistency OFF, manifest hardGuardrailGlobs + guardrailEscalation.selfConsistencyRuns=3, a guarded changed file -- the pipeline delivers 1 primary + 2 rotated-exemplar extra calls, proving the resolved knobs reach the invocation (the #9821 review blocker, now pinned by an integration test, not just the precedence mirror) - INERT invariant: same escalation block, no guarded file -- zero selfConsistency usage rows (counted via the usage marker, not raw AI.run calls, matching the flag-off baseline's discipline)
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…— per repo, and escalated on guarded paths (#9821) * feat(gate): choose provider, model, effort and self-consistency runs — per repo, and escalated on guarded paths Effort DID already exist (CLAUDE_AI_EFFORT -> `claude --effort`, CODEX_AI_EFFORT -> model_reasoning_effort) but only as a GLOBAL env var, while provider and model were already per-repo manifest fields. Self-consistency runs had the same asymmetry. So the two knobs that most directly control review depth were the two a repo could not set. Close that, and use it where it matters most. Adds: gate.aiReview.effort / .selfConsistencyRuns — per-repo parity with the env vars gate.guardrailEscalation.{provider,model,effort,selfConsistencyRuns} — used INSTEAD when a PR touches a hardGuardrailGlobs path Precedence resolves per field independently: escalation > per-repo > global. An escalation block that sets only `effort` still inherits the repo's model and the global provider -- the common ask is "same model, think harder", not "swap the model out". WHY: a guardrail hit used to change exactly one thing -- it suppressed auto-merge and queued a human. It bought no extra analysis at all: a PR touching .github/workflows/** got the same single-pass, same-model, same-effort review as a README typo. Measured on the production ORB that was 74 distinct PRs held in 14 days, on repos whose guardrail lists had already been narrowed twice. A guarded path can now buy MORE SCRUTINY instead of maintainer toil; manual review stays the fallback when the escalated review is not clean. Every layer unset ⇒ null throughout ⇒ byte-identical to today for every existing repo and self-hoster. An escalation is logged (ai_review_guardrail_escalated) and describeReviewEscalation renders the one-line "why" for the panel/decision record. * fix(gate): actually apply the resolved effort/model/provider at the invocation Review blocker on #9821, and correct: resolveReviewKnobs was computed, logged, and then DROPPED. Only selfConsistencyRuns was consumed, so "choose provider, model, effort" was unimplemented at the one place it takes effect -- and the doc comment claiming the fields "ride through to the provider invocation" was false. Apply them where review.ai_model's overrides already land: claudeModel/codexModel/ollamaModel/openaiModel/openaiCompatibleModel/ anthropicModel <- reviewKnobs.model ?? review.ai_model ?? env claudeEffort/codexEffort <- reviewKnobs.effort ?? ... ?? env reviewKnobs wins over review.ai_model because it is strictly more specific: it is the only layer that can differ PER PR, and an escalation existing at all means it should win for the PR that triggered it. Unset ⇒ `??` falls straight through, so a repo using only review.ai_model is untouched. Provider also now governs the BYOK key. The providerKey gate runs before the knobs are resolved and only knew settings.aiReviewProvider, so an ESCALATED provider would have kept using a stored key belonging to a different one. Same rule, re-applied with the resolved value: a mismatch drops the key. Also re-syncs config/examples/loopover.full.yml with .loopover.yml.example -- caught by running the FULL suite this time, which is the same break class that took main down after #9813. * test(gate): cover every new manifest branch, and fix the serialize guard the coverage exposed Codecov flagged 39 uncovered lines on #9821, and closing them found a REAL bug: gateConfigToJson's outer aiReview guard was never taught the two new fields, so a manifest whose aiReview block set ONLY effort/selfConsistencyRuns serialized to NOTHING -- the setting silently vanished on the next snapshot reload. The new round-trip assertion failed on exactly that before touching coverage at all. (An earlier edit had aimed at this guard and hit the presence expression instead; the round-trip test is what caught the difference.) Coverage added: - engine parse: full/partial/invalid/non-mapping guardrailEscalation, invalid effort + runs values warn and stay null, presence flips on EACH of the six fields alone (the #9813 presence-gap class, per field), serialize round-trips full AND partial blocks - resolution: every field lands on effective settings; absent leaves the stored value untouched - END-TO-END escalation: env self-consistency OFF, manifest hardGuardrailGlobs + guardrailEscalation.selfConsistencyRuns=3, a guarded changed file -- the pipeline delivers 1 primary + 2 rotated-exemplar extra calls, proving the resolved knobs reach the invocation (the #9821 review blocker, now pinned by an integration test, not just the precedence mirror) - INERT invariant: same escalation block, no guarded file -- zero selfConsistency usage rows (counted via the usage marker, not raw AI.run calls, matching the flag-off baseline's discipline) * fix(gate): consume the resolved model/effort in ai-review.ts itself, where the reviewer looked Re-review blocker: the diff still showed only selfConsistencyRuns consumed in src/services/ai-review.ts. The orchestration DID pre-apply model/effort to the claude*/codex*/HTTP-model fields, so behavior was correct -- but this module, which owns the doc comment claiming the fields "ride through to the provider invocation", never read them, and any OTHER caller passing reviewKnobs would have been silently ignored. Fold reviewKnobs.model/effort into the AiRunCorrelation, which IS what becomes the provider's per-call options at this module's dispatch. Same ?? precedence as the orchestration, so applying it in both places is idempotent, and this module is now self-sufficient. Correct the doc comment to state exactly what is consumed WHERE, including that `provider` is deliberately NOT consumed here -- it governs BYOK key selection at the orchestration, before this runs, and rides along for logging/replay only. Test captures the options the provider actually receives (claudeEffort/ claudeModel/codexEffort/anthropicModel) for a guarded-path PR with an escalation block, rather than asserting on the precedence mirror. * test(engine): cover the new manifest branches in the ENGINE's own suite Codecov kept reporting focus-manifest.ts at ~65% despite the root vitest suite covering every added line. The reports disagree because they measure differently: the root v8 run instruments only the 11 lines it actually executes from that source, while the engine's own c8 run uses --all over dist/ and instruments all 66 -- and the engine's node:test suite never exercised the new branches. Codecov merges both, so 21 lines read as uncovered no matter how thorough the vitest tests were. Add the engine-side tests: parse, per-field presence, serialize round-trip (full AND partial -- the case whose guard was genuinely broken), invalid-value rejection, non-mapping escalation, and absent-means-null including that the serializer emits no escalation block at all. Engine c8 now reports 0 of 66 added lines uncovered, down from 21.
This was referenced Jul 29, 2026
This was referenced Jul 29, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 30, 2026
…stry, write down the coverage rule (#9949) * chore(guards): pin the focus-manifest field registry, document the coverage asymmetry Refs #9860 (items 4 and 5). ITEM 4. FOCUS_MANIFEST_TOP_LEVEL_FIELDS is a hand-kept list of 28 keys whose doc comment claims it is every top-level key parseFocusManifest actually reads. Nothing checked that claim, and it is load-bearing both ways: a key the parser reads but the list omits is reported to operators as an unknown field, while a key in the list the parser never reads is worse -- the validator blesses it, the runtime ignores it, and the operator's setting silently does nothing while every surface says it is fine. #9813 and #9821 each shipped a bug from missing one of this registry's touchpoints. The relation is now computed from the parser's own source rather than trusted. Currently in sync at 28/28, so this pins today's state rather than fixing a live bug. Source-scanned deliberately: the failure is a key the parser reads and the list forgot, which produces no behavioural difference to assert on. Includes an anti-vacuity check, because a refactor renaming the local would otherwise empty both sets and make the assertions pass while checking nothing. Not guarded: the parse/serialize pairing. Four parsers (AutoReview, MaxFindings, SelfHostAiModel, Visual) have no *ConfigToJson of their own because they are nested under the review config and serialized through it, so a naive 1:1 check would fire four false positives -- and a checker that cries wolf gets muted. ITEM 5. The engine coverage asymmetry is now a contributor-facing rule rather than folklore. packages/loopover-engine/src/** is credited by two uploads whose hits Codecov unions, and the two runs disagree about which lines exist (c8 --all instruments files no test imports; v8 does not), so a genuinely 100%-tested engine change can land at ~65% on codecov/patch. The rule: change engine source, add the test to packages/loopover-engine/test/** as well. Also corrects AGENTS.md, which stated that only src/** is measured -- untrue since the engine flag, and misleading in exactly the direction that costs a PR. And records the issue's own closing note: ui:typecheck is in test:ci but not in the root typecheck script, so a locally-clean type change can still break apps/** (#9815). * chore(mcp): derive the help banner's --login command list instead of typing it Refs #9860 (item 1). The LOOPOVER_LOGIN line in `loopover-mcp --help` named its commands as hand-typed prose, and it had already drifted: it omitted contributor-profile, explain-review-risk and watch, all three of which accept --login and resolve it through the same resolveLogin fallback. A user reading --help was told those commands would not pick up their configured login. They do. Derived from CLI_COMMAND_SPEC now -- every command declaring --login in its usage resolves it that way, and resolveLogin is the single helper that does it, so the usage table is the fact to read rather than a second list to keep in step. Also drops a no-op .replace() on the usage line beside it. Items 1 and 2 of #9860 are otherwise already closed by the MCP contract epic: STDIO_TOOL_DESCRIPTORS derives from STDIO_TOOL_NAMES, printHelp's command list derives from CLI_COMMAND_SPEC, the README block is generated by mcp:tool-reference, and the stdio bin now declares no zod shapes of its own -- all 119 z. uses are z.infer over contract-imported schemas. This line was the one hand-maintained list left in that surface.
JSONbored
added a commit
that referenced
this pull request
Jul 31, 2026
FOCUS_MANIFEST_TOP_LEVEL_FIELDS' own header says "Every top-level key parseFocusManifest below actually reads." That is a statement about code twenty lines further down, and nothing enforced it -- the same shape as the turbo.json inputs comment #9860 calls out as the tell: a snapshot written as a guarantee. The list is the single source of truth for unknown-field detection (#9065), so drift is silent in both directions and asymmetrically bad: DECLARED BUT NEVER READ -- the parser accepts the field, warns about nothing, and ignores it. An operator sets it in .loopover.yml, sees no error, and the setting does nothing. This is the worse direction: it looks configured. READ BUT NOT DECLARED -- the parser consumes the field while warning "unknown top-level field" about it, and config-lint's recognizedFieldsFor leaves it out of the recognized count. A working field reported as a mistake. Adding a field means editing two places twenty lines apart, and #9813 and #9821 each shipped a bug from missing one touchpoint of a multi-touchpoint field. Computed, not restated: the declared names come out of the array literal and the used names out of the function body, so this file holds no second list and cannot itself go stale. It is a regex over source rather than the TS AST, which is a real tradeoff. The access pattern is uniform and mechanical, and every extractor THROWS when the shape it depends on is missing -- a renamed constant, a moved signature, or a refactor to destructuring all fail loudly instead of matching nothing and reporting "no drift". A guard that passes vacuously is worse than none, because its presence reads as coverage. Holds today at 28 declared and 28 read, empty both ways, so this lands green and guards forward. Mutation-tested: declaring an unread field, reading an undeclared one, and removing a still-read declaration each fail with the field named. Closes #9979
loopover-orb Bot
pushed a commit
that referenced
this pull request
Jul 31, 2026
…ds (#9980) FOCUS_MANIFEST_TOP_LEVEL_FIELDS' own header says "Every top-level key parseFocusManifest below actually reads." That is a statement about code twenty lines further down, and nothing enforced it -- the same shape as the turbo.json inputs comment #9860 calls out as the tell: a snapshot written as a guarantee. The list is the single source of truth for unknown-field detection (#9065), so drift is silent in both directions and asymmetrically bad: DECLARED BUT NEVER READ -- the parser accepts the field, warns about nothing, and ignores it. An operator sets it in .loopover.yml, sees no error, and the setting does nothing. This is the worse direction: it looks configured. READ BUT NOT DECLARED -- the parser consumes the field while warning "unknown top-level field" about it, and config-lint's recognizedFieldsFor leaves it out of the recognized count. A working field reported as a mistake. Adding a field means editing two places twenty lines apart, and #9813 and #9821 each shipped a bug from missing one touchpoint of a multi-touchpoint field. Computed, not restated: the declared names come out of the array literal and the used names out of the function body, so this file holds no second list and cannot itself go stale. It is a regex over source rather than the TS AST, which is a real tradeoff. The access pattern is uniform and mechanical, and every extractor THROWS when the shape it depends on is missing -- a renamed constant, a moved signature, or a refactor to destructuring all fail loudly instead of matching nothing and reporting "no drift". A guard that passes vacuously is worse than none, because its presence reads as coverage. Holds today at 28 declared and 28 read, empty both ways, so this lands green and guards forward. Mutation-tested: declaring an unread field, reading an undeclared one, and removing a still-read declaration each fail with the field named. Closes #9979
This was referenced Jul 31, 2026
loopover-orb Bot
pushed a commit
that referenced
this pull request
Jul 31, 2026
…ust its cache key (#10163) gate.ignoredCheckRuns (#9813) is supposed to exclude a maintainer-declared check-run from CI resolution entirely, and the reducer + fetchLiveCiAggregatePreferGraphQl honour it. But ci-resolution.ts folded it into the durable cache key at all three hand-offs while dropping it from the arg object passed downstream, so cachedFetchLiveCiAggregate's ignoredCheckRuns was always undefined -- TypeScript can't catch it because the property is optional at every hop. The consequence on the live maintenance path: ciAggregate's ignoredCheckDetails is always [], ignoredCheckNonPassing is always empty, and unstableExplainedByIgnoredChecks is always false, so the #9810 follow-up that stops an ignored check from holding a PR at mergeable_state 'unstable' can never fire in production, and the planner and executor can disagree about the same PR's CI state. Forward args.ignoredCheckRuns at all three call sites (fetchLiveCiAggregateWithRequiredContexts, cachedLiveCiAggregate, refreshLiveCiAggregate), mirroring how advisoryCheckRuns is already threaded through the same hops. Signatures stay optional so positional callers are byte-identical; the requiredContextsKey composition and advisoryCheckRuns threading are unchanged, and nothing outside ci-resolution.ts is touched. Closes #10018
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
gate.advisoryCheckRunsstops 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 your repo, that trades a stall for a queue of human decisions.Live on the ORB: a vendor app publishes both a real security scan and a heuristic "Contributor trust" score. The trust score fails for known-good contributors, so every one of their otherwise-flawless PRs becomes a manual review. This previously auto-closed those PRs (#4414); the fix made it hold instead — better, but it still means the maintainer touches every PR, and contributors are left wondering whether the bar is being applied fairly.
Manual-review volume is the actual complaint: 277 hold decision-records across 75 contributor PRs in 14 days, and this class is a large share of it.
The feature
A matched run is treated as if it did not exist — never gates, never counts as "still running", never holds. Its conclusion is surfaced as
ignoredCheckDetails(informational only) so the exclusion is visible rather than the check silently vanishing.Design points worth review:
{ name, appSlug }both required; a name-only match is untrusted, so a contributor-controlled same-name run from another app can't get itself ignored.advisoryCheckRuns— otherwise a stale entry keeps gating on a check you just ignored.Tests
6 behavioral cases in
backfill-2.test.ts: the motivating regression (failing ignored check ⇒passed, no hold — the decisive difference from advisory), sibling-still-gates, pending-doesn't-stall, ignore-beats-advisory, spoof resistance, and unset-is-unchanged. Plus manifest parse/serialize/resolve coverage (810 pass) and the executor's call-shape assertions updated to pin the new argument rather than loosened.tsc,docs:drift-check,manifest:drift-check,engine-parity:drift-check,ui:openapi:check,contract:api-schemas:check,ui-derived-types:check,dead-source-files:checkall green..loopover.yml.exampledocuments the field and when to reach for it overadvisoryCheckRuns.After this merges
Server-side config change on all three repos: move
Contributor trustfromadvisoryCheckRunstoignoredCheckRuns, keeping the security scan gating. Tracked alongside the broader manual-review reduction work in #9808.