fix(gate,review,ops): add glob exclusions, tier the public-comment vocabulary, and make inert config queryable (#9554, #9555, #9433) - #9556
Merged
Conversation
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 | 90513c7 | Commit Preview URL Branch Preview URL |
Jul 28 2026, 09:41 AM |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9556 +/- ##
==========================================
- Coverage 89.62% 89.62% -0.01%
==========================================
Files 867 868 +1
Lines 110801 110861 +60
Branches 26346 26355 +9
==========================================
+ Hits 99307 99355 +48
- Misses 10229 10237 +8
- Partials 1265 1269 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Bundle ReportChanges will increase total bundle size by 210 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: loopover-uiAssets Changed:
|
loopover-orb
Bot
added
gittensor:bug
Owner
Author
|
Pushed additional coverage for the two engine files Codecov flagged. Diagnosis. Codecov ran on my current head ( The likely cause is shard attribution: Fix. The exclusion behaviour is now exercised from both import paths rather than one:
Not redundant duplication: each path is its own real file and its own coverage identity, so whichever shard picks up either suite, the lines stay attributed. 237 tests green across the three suites; typecheck clean; both engine files now report zero uncovered changed lines locally. |
…d attribution cannot miss it
…-untested reject arms Brings packages/loopover-engine/src/review/screenshot-table-gate.ts and src/signals/change-guardrail.ts (and both re-export shims) to 100% statement, branch and function coverage. Three branches had no test on any identity: - evaluateScreenshotTableGate's PRESENCE-mode freshness checkpoint. Matrix mode's identical correlation was pinned by #8866's tests, but presence mode runs it through a separate return site and never saw a headSha in a test. Presence mode is the DEFAULT shape, so the miss meant one table pasted on push #1 could hold the gate green for every later push -- the exact regression the checkpoint exists to stop. Six tests: first-push checkpoint, stale-on-new-head (asserting no checkpoint is re-issued, which would launder the staleness away after one extra push), re-affirmation with fresh URLs, custom message on the stale path, no-headSha degradation, and same-head replay. - guardrailPathMatches' empty-path skip, a deliberate divergence from matchesAny's fail-safe: the boolean form matches an empty path under an over-complex glob, the structured form must not, because its output is rendered verbatim into public review text and audit metadata. - extractTableRows' non-table reject arm, which is what stops ordinary PR prose (and a shell pipe inside backticks) from parsing as table rows. Mirrored across both import identities so a sharded, flag-merged coverage upload cannot report a branch as uncovered on one copy.
…s that transitively need it validate-code failed on this PR with 'Cannot find module @loopover/contract/tools' plus five downstream implicit-any errors in src/mcp/server.ts -- none of which this PR touches. #9530 added @loopover/contract, and src/mcp/server.ts imports @loopover/contract/tools. Three turbo typecheck tasks pull that file into their program without any build edge to the package: - @loopover/ui#typecheck: apps/loopover-ui/tsconfig.json includes $TURBO_ROOT$/worker-configuration.d.ts, which imports './src/index' -- so the ENTIRE Worker is in the UI's typecheck program (src/index.ts -> src/api/routes.ts -> src/mcp/server.ts -> @loopover/contract/tools). Confirmed with tsc --explainFiles, not inferred. apps/loopover-ui has no package.json dependency on contract, so ^build never builds it. - //#typecheck: a root task, where a root package.json dependency creates no build edge. - @loopover/ui-miner#typecheck: reaches packages/loopover-miner/lib/**, which imports the package, and miner-ui has no dependency on it either. Each already carries an explicit @loopover/engine#build edge for precisely this reason -- turbo.json's own comment there describes the same scheduling race, observed intermittently in validate-code, that bit here. Cache-dependent, which is why it looked like flakiness: turbo caches these tasks, so the failure only appears on a cache MISS. Other open PRs are green on cache hits. Verified both directions from a clean contract build state (dist/ and .tsbuildinfo both removed, --force): - with the edges: 4 tasks successful, contract built first, typecheck passes - without them: the identical six errors CI reported
JSONbored
force-pushed
the
fix/glob-exclusions-and-inert-config
branch
from
July 28, 2026 09:38
693a6cb to
90513c7
Compare
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.
Summary
Three defects that share one shape: a safety mechanism that is correct in aggregate but too blunt at the edges, silently destroying good content or hiding its own inertness.
Closes #9554
Closes #9555
Closes #9433
#9554 — the path matcher cannot exclude, so a gate auto-closed PRs for a JSON file
matchesAnyis positive-only: no!negation, and{a,b}braces compile to literals. Soapps/loopover-ui/public/**could not carve out the generatedopenapi.jsoninside it — andCLAUDE.mdrequires contributors to regenerate that file on any API change. Following the documented contribution steps tripped a gate demanding a Desktop/Tablet/Mobile × Dark before/after screenshot matrix of a JSON spec, and withaction: closebeing one-shot, 5 contributor PRs were auto-closed with no recovery path.matchesAnyWithExclusionsis additive —matchesAnyis unchanged, because it is also the hard-guardrail matcher where a maintainer's literal path starting with!must stay guarded, not be silently reinterpreted.The load-bearing subtlety is that the two halves need opposite fail directions for an over-complex (ReDoS-capped) glob. An unsafe include fails toward matching — safe, more paths in scope, and
matchesAny's existing deliberate behaviour. An unsafe exclude must fail toward excluding nothing: failing it toward "matches everything" would silently widen the exclusion and shrink gate coverage, the opposite of what a safety gate is for. My first implementation reusedmatchesAnyfor both halves and got this backwards; the security test caught it.Sibling repos, verified on the server (parent issue's follow-up 1).
metagraphedandawesome-claudeboth carried the identical over-broad glob — so this was never loopover-specific and both were live-exposed withaction: close. Narrowed both to the actually-rendered UI, matching the fix already proven on loopover, with a comment recording why and backups taken first. No restart needed:private-config.tsreads these fresh per lookup.#9555 — ordinary English still drops narrative sentences
#9445 fixed the catastrophic half of #9432 (per-sentence filtering instead of all-or-nothing). The residual:
FORBIDDEN_PUBLIC_COMMENT_WORDSis matched with a plain.includes()and contains ordinary English —reward,ranking,cohort,farming,reviewability. Only barescorehad a per-repo escape hatch, so on every repo a sentence like "this improves reviewability and updates the ranking comparator" was still dropped. No longer a missing summary — a quietly incomplete one, which is harder to notice.Split into
ALWAYS_FORBIDDEN_PUBLIC_COMMENT_WORDS(never exemptible) andAMBIGUOUS_PUBLIC_COMMENT_WORDS(exemptible via the existing allowlist — no new config surface).Why the split is safe: the risk is a leaked private value, and a value is never a bare noun — it is qualified (
reward estimate 12 TAO,raw trust score 0.82,score preview). Every qualified form stays in the always-forbidden tier, enforced for every repo. A barerewardwith no number leaks nothing. This generalizes the judgmentallowBareScoreTermalready made forscorerather than inventing a new one.#9433 — a report, not more boot warnings
The confirmed instance (an unset score-terms allowlist silently replacing every narrative containing "score") sat live for weeks with a green suite, because tests verify a mechanism when enabled, never that an operator set the variable.
I did not follow the issue's obvious path of adding one boot warning per var, because verifying first showed it would be actively harmful.
LOOPOVER_PUBLIC_STATS_REPOSis unset on the ORB self-host box and that is correct — the public-stats surface is served by the Cloudflare Worker, wherewrangler.jsoncsets it to four repos (confirmed live:byProjectreturns 3 entries). An unconditional warning would fire on every self-host deployment forever for a non-problem — the same alert fatigue that let a genuinely broken backup alert be ignored for 8 days.What is actually missing is answerability: no way to ask "which config-gated behaviours are inert on this box?" without reading long-scrolled boot logs.
loopover_inert_configanswers exactly that, on demand, with zero steady-state noise and cardinality bounded by construction (the key set is a fixed list in code). Every entry is classifieddeployment-specificprecisely so nothing here is auto-escalated to a warning.Scope is deliberately narrow: only vars whose unset state changes output correctness. The ~100
Default OFFconvergence flags are excluded — listing them would bury the few that matter.Validation
npx tsc --noEmit,selfhost:env-reference:check(169 vars),dead-source-files:check,actionlint,git diff --check— all cleansrc/server.ts, which is explicitly Codecov-ignored as boot wiring covered by the Docker boot smoke test)Regressions: the exclusion carves a generated file out of a matching directory glob, in both places that read
whenPaths; an allowlisted repo keeps a sentence using each ambiguous term; the report names the confirmed #9433 instance.Invariants: with no exclusions, behaviour is identical to
matchesAny; an all-exclude list is not an implicit wildcard include; a bare!stays a literal include. SECURITY, asserted exhaustively over the real arrays so a term in the wrong tier fails immediately: every always-forbidden phrase still throws with the exemption on; a qualified private value stays blocked even though its bare noun is exempt; an exclude glob is bound by the same ReDoS cap and fails toward excluding nothing. The two tiers are disjoint and together exactly equal the combined list. Gauge samples and entries stay in lockstep, so three surfaces read one answer rather than three drifting lists.Closes #9560