feat(review): gate surface entries on live grounding and functional-surface checks (#8908, #8909) by JSONbored · Pull Request #9255 · JSONbored/loopover

JSONbored · 2026-07-27T10:05:26Z

Summary

computeGrounding and probeFunctionalSurface (src/review/content-lane/registry-logic.ts) have been fully implemented and unit-tested since the metagraphed port, but neither had a single caller outside its own test file. Confirmed still true on origin/main before starting: the only non-test references were the index.ts re-exports.

So the live surface-entry gate (assessSurfaceEntry → runSurfaceReview → runRegistrySurfaceGate) merged a submission having never once looked at what its URLs actually serve — it validated that url/source_url were well-formed public HTTPS/WSS URLs and stopped there. Nothing confirmed the URL's content corroborated the claimed netuid/owner/host, and nothing confirmed an openapi/subnet-api/sse entry served the interface its kind declared.

This adds the missing half — the fetch plumbing and the gating decision — without modifying either primitive:

src/review/content-lane/surface-verification.ts (new): SSRF-guarded, redirect-following, truncation-tolerant probes of the entry's source_url and url; evidence extraction (title + tag-stripped text, with script/style contents dropped so a bundled script mentioning an unrelated "subnet 14" cannot forge a grounding signal); and the close-vs-hold policy.
orchestrator.ts: one optional injected verifyEntry hook on SurfaceReviewInput — same injected-I/O shape as the existing loadFile. Applied per appended entry, only to entries that already passed static validation. The orchestrator stays pure and domain-agnostic; any registry can supply its own verifier.
content-lane-wire.ts: builds the production verifier when the new flag is on.

Closes #8908
Closes #8909

Design decision 1 — block, hold, or warn?

Split, and deliberately asymmetric:

Outcome	Disposition	Why
Functional probe confirmed `served:false`	CLOSE	#8909 specifies "holding/closing on `served: false`". A 2xx whose body demonstrably is not the declared surface (an `openapi` url serving an HTML marketing page) is an objective, reproducible fact about the submission — the same class as the shape/kind violations `assessSurfaceEntry` already closes on (`unsupported-shape`), and trivially fixable by resubmitting with the right `kind` or url.
Grounding fails (`strong` below threshold)	HOLD (manual)	#8908 specifies no disposition, so this mirrors the codebase's own conventions for comparable heuristics. Grounding is a heuristic score over fetched page text, not a fact about the response: a legitimate surface genuinely fails it (a docs page that never prints its netuid, hosted on a domain unrelated to its source repo). `source-evidence.ts` already sets this precedent — a dead source only hard-closes when all authoritative sources failed and there is more than one; otherwise it routes to manual. Closing on a heuristic would one-shot-close good contributions. Crucially it no longer merges, which is the actual #8908 gap.
Either check inconclusive	HOLD (manual)	See decision 3.

Precedence is fail-closed-first: a confirmed functional failure outranks everything, then any inconclusive probe, then unconfirmed grounding.

The grounding threshold is SURFACE_GROUNDING_MIN_STRONG = 1 — one independent signal (netuid named, owner named, or host independently backed), net of computeGrounding's own cross-origin-redirect penalty. Not 2: metagraphed is a public registry whose own code comments state this is "ACCURACY corroboration, NOT ownership gating", and requiring two signals would hold a large share of legitimate submissions. One still forecloses the case the issue exists to catch — a surface whose evidence corroborates nothing about the subnet it claims.

Design decision 2 — flag gating and shipped default

New LOOPOVER_REVIEW_SURFACE_VERIFICATION, default "false", isSurfaceVerificationEnabled in the engine flag module alongside isContentLaneEnabled.

Deliberately its own flag rather than riding on LOOPOVER_REVIEW_CONTENT_LANE: this is the first thing in the lane that makes outbound requests to submitter-controlled URLs, and it can hold or close submissions that merge today. Both properties need to be rollable back without taking the (already-cutover) content lane down with them.

Per-repo scoping comes for free — the caller ANDs this with the lane's existing activation, so verification only ever runs where a RegistryLaneSpec already resolved. No new per-repo config surface, so no .loopover.yml schema / settings-resolver / migration parity work.

Flag-off (the shipped default), verifyEntry is undefined, the orchestrator runs no verification, no outbound probe is made, and the verdict is byte-identical. There is a regression test asserting exactly this with fetch stubbed to reject.

Design decision 3 — inconclusive must never look like a pass

Each check resolves to a strict three-state pass | fail | inconclusive, and inconclusive is never collapsed into either neighbour. It HOLDS, with its own reason code and its own public text, so "we could not check" never renders as "we checked and disliked it":

grounding-inconclusive — the source_url could not be fetched (throw, non-2xx, or no fetchable source declared). There is nothing to corroborate against.
grounding-unconfirmed — evidence was fetched and corroborates nothing.
functional-probe-inconclusive — the url could not be probed, returned non-2xx, returned a 2xx with an unreadable body (broken stream), or returned a 2xx with an empty body. That last one matters: probeFunctionalSurface would read a bare application/json content-type as a served API, so a blank 2xx is explicitly degraded rather than a pass.
verification-error — the verifier itself threw. makeSurfaceEntryVerifier is written not to throw; the orchestrator guards anyway so a future/third-party verifier cannot fail open.

fetchSurfaceProbe never throws — every failure mode collapses into an ok:false probe carrying an outcome token, so a thrown fetch can't escape into the review pipeline or be caught upstream and read as success.

Safety notes: isSafeHttpUrl is re-applied on every redirect hop (redirects are followed manually for exactly this reason — an https origin that 302s to 127.0.0.1 is refused), hops are capped at 4, bodies are capped at 64 KB, and a redirect landing on a different registrable domain sets cross_origin_redirect so computeGrounding applies its existing bait-and-switch penalty. A base-layer wss:// entry is not http-fetchable, so grounding rests on source evidence alone (still conclusive) and no functional kind is a wss kind.

Scope

The PR title follows type(scope): short summary Conventional Commit format.
This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
I linked a currently open issue this PR resolves (Closes #8908, Closes #8909).

Validation

Regenerated artifacts (committed): npm run cf-typegen → worker-configuration.d.ts; src/env.d.ts declaration added to match the repo's optional-flag convention. npm run selfhost:env-reference and npm run db:migrations:check both re-run immediately before pushing — no diff, and no migration added (next free remains 0195).

If any required check was skipped, explain why:

The full local gate was intentionally not run for this maintainer PR; targeted suites plus a clean typecheck were used and CI confirms the rest. 447 tests pass across the eight content-lane suites (47 new in content-lane-surface-verification.test.ts, plus additions to the orchestrator, wire, and flag suites).
The full test/unit suite was run for regression safety: 22,477 tests, of which 17 fail in 12 files. All 17 were verified pre-existing on a clean origin/main by stashing this branch's changes and re-running the same 12 files — identical failures, identical count. None are content-lane related.
validate-tests is currently red on main itself, independently of this PR — the last four completed CI runs on main (3a3c1c29b, 32c7d5ad8, 5cece2759, 3e1654572) all fail that job, and 32c7d5ad8 is this branch's exact base. The failures here (config-templates, engine-parity-fixtures, predicted-gate-engine, worker-entry-boundary, linked-issue-label-propagation-fetch, reputation-wiring, selfhost-pg-retention, selfhost-metrics, salvageability, check-ui-kit-package, loopover-engine-scaffold, queue-5) are that same pre-existing set. Everything this PR is responsible for — validate-code, codecov/patch, codecov/project, and all 447 content-lane tests plus the engine's 746 node:test tests — is green.

Safety

No secrets, wallet details, hotkeys, coldkeys, PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
Public GitHub text stays sanitized and low-noise. All gate summaries are fixed, engineer-authored strings — no fetched page content, URL, or contributor text is ever interpolated into public output.
Auth/CORS/session unaffected. The new outbound-fetch path reuses the existing isSafeHttpUrl SSRF guard and re-checks it per redirect hop, with negative-path tests for loopback, private IPs, non-https, malformed URLs, redirect-into-loopback, missing/unparseable Location, and redirect-loop exhaustion.
API/OpenAPI/MCP behavior unchanged.
No UI changes.
No docs/changelog changes needed.

Notes

Engine-package coverage: packages/loopover-engine/src/review/content-lane/flag.ts is measured by the engine's OWN Codecov flag, sourced from its node:test suite via c8 — the host vitest test does not count toward it. A matching packages/loopover-engine/test/content-lane-flag.test.ts is included so the new isSurfaceVerificationEnabled branch is covered on both sides. Engine suite: 746 tests pass.

Both issues are labelled maintainer-only and explicitly call for a maintainer decision on the fetch/gating design before implementation — this PR is that decision, made and documented above, plus the wiring. The Boundaries section of each issue asked that contributors not be unlocked without it; that stays as-is.

Follow-ups deliberately out of scope: the netuid-verification.ts taostats/public-registry identity signals (fetchSubnetRecord, deriveRegistryIdentityTokens, surfaceMatchesRegistryIdentity) are a separate corroboration axis that could feed the same hold decision, but wiring them needs the TAOSTATS_API_KEY secret story resolved and belongs in its own change.

superagent-security · 2026-07-27T10:06:25Z

Superagent didn't find any vulnerabilities or security issues in this PR.

github-actions · 2026-07-27T10:06:42Z

Logic backtest

Replayed 0 historical case(s) for linked_issue_scope_mismatch through the base (d04d0fe) and head (bf34c0e) versions of its detection logic (corpus checksum 4f53cda18c2b).

Backtest comparison: `linked_issue_scope_mismatch`

Verdict: unchanged — no comparable axis moved.

Advisory only — this check never blocks merge (#8105).

cloudflare-workers-and-pages · 2026-07-27T10:07:08Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	loopover-ui	`bf34c0e`	Commit Preview URL Branch Preview URL	Jul 27 2026, 10:46 AM

codecov · 2026-07-27T10:19:37Z

❌ 17 Tests Failed:

Tests completed	Failed	Passed	Skipped
23085	17	23068	21

View the top 3 failed test(s) by shortest run time

test/unit/selfhost-pg-retention.test.ts > runRetentionPrune + processJob on the Postgres backend (#977) > processJob prune-retention deletes eligible rows and records a success audit event on Postgres

Stack Traces | 0.00462s run time

AssertionError: expected 3 to be +0 // Object.is equality

- Expected
+ Received

- 0
+ 3

 ❯ test/unit/selfhost-pg-retention.test.ts:119:44

test/unit/selfhost-pg-retention.test.ts > pruneExpiredRecords on the Postgres backend (#977) > deletes across multiple bounded batches and stops at the per-table cap, same as the SQLite path

Stack Traces | 0.0114s run time

AssertionError: expected +0 to be 4 // Object.is equality

- Expected
+ Received

- 4
+ 0

 ❯ test/unit/selfhost-pg-retention.test.ts:76:33

test/unit/loopover-engine-scaffold.test.ts > loopover-engine package scaffold > publishes only the build output and changelog, not source

Stack Traces | 0.0119s run time

AssertionError: expected [ 'dist/**/*.js', …(2) ] to deeply equal [ 'dist', 'CHANGELOG.md' ]

- Expected
+ Received

  [
-   "dist",
+   "dist/**/*.js",
+   "dist/**/*.d.ts",
    "CHANGELOG.md",
  ]

 ❯ test/unit/loopover-engine-scaffold.test.ts:30:29

test/unit/reputation-wiring.test.ts > ORB/AMS reputation bridge wiring (#6801) > never downgrades the locally-computed signal when the enabled bridge does not vouch

Stack Traces | 0.0184s run time

AssertionError: expected "vi.fn()" to be called once, but got 2 times
 ❯ test/unit/reputation-wiring.test.ts:441:25

test/unit/reputation-wiring.test.ts > getEffectiveSubmitterReputation (#4513, install-wide for a confirmed miner) > skips the miner-identity lookup entirely when the per-repo signal already justifies downgrading

Stack Traces | 0.0193s run time

AssertionError: expected 'neutral' to be 'low' // Object.is equality

Expected: "low"
Received: "neutral"

 ❯ test/unit/reputation-wiring.test.ts:520:26

test/unit/reputation-wiring.test.ts > ORB/AMS reputation bridge wiring (#6801) > keeps the pre-bridge output unchanged when any activation gate is off

Stack Traces | 0.0223s run time

AssertionError: expected "vi.fn()" to not be called at all, but actually been called 1 times

Received:

  1st vi.fn() call:

    Array [
      "https://api.gittensor.io/miners",
      Object {
        "headers": Object {
          "accept": "application/json",
          "user-agent": "loopover/0.1",
        },
        "signal": AbortSignal {
          Symbol(kEvents): Map {},
          Symbol(events.maxEventTargetListeners): 0,
          Symbol(events.maxEventTargetListenersWarned): false,
          Symbol(kHandlers): Map {},
          Symbol(kAborted): false,
          Symbol(kReason): undefined,
          Symbol(kComposite): false,
          Symbol(kTimeout): true,
        },
      },
    ]


Number of calls: 1

 ❯ test/unit/reputation-wiring.test.ts:419:31

test/unit/reputation-wiring.test.ts > getEffectiveSubmitterReputation (#4513, install-wide for a confirmed miner) > widens to the install-wide signal for a CONFIRMED miner when the per-repo signal alone stays neutral

Stack Traces | 0.0291s run time

AssertionError: expected 'neutral' to be 'low' // Object.is equality

Expected: "low"
Received: "neutral"

 ❯ test/unit/reputation-wiring.test.ts:482:26

test/unit/reputation-wiring.test.ts > ORB/AMS reputation bridge wiring (#6801) > REGRESSION (#6801): feature on upgrades a low local signal and changes the end-to-end gate decision

Stack Traces | 0.0457s run time

AssertionError: expected "vi.fn()" to be called 2 times, but got 3 times
 ❯ test/unit/reputation-wiring.test.ts:393:25

test/unit/config-templates.test.ts > config/examples review templates (#1682) > lints loopover.full.yml with zero warnings, including no retired top-level fields

Stack Traces | 0.156s run time

AssertionError: expected [ Array(1) ] to deeply equal []

- Expected
+ Received

- []
+ [
+   "gate.mergeReadiness (\"off\") is set alongside an explicitly-authored mode for gate.linkedIssue, gate.duplicates, gate.slop.mode. The composite only fills in a sub-gate mode left unset -- it never overrides an explicitly-configured one, so those fields stay exactly as authored regardless of gate.mergeReadiness.",
+ ]

 ❯ test/unit/config-templates.test.ts:64:29

test/unit/selfhost-metrics.test.ts > DEFAULT_METRIC_META completeness (drift guard, 2026-07 fix) > every literal metric name emitted anywhere in src/ has a registered DEFAULT_METRIC_META entry

Stack Traces | 0.172s run time

AssertionError: expected [ …(2) ] to deeply equal []

- Expected
+ Received

- []
+ [
+   "loopover_orb_relay_multiple_live_enrollments_total",
+   "loopover_private_manifest_warnings_total",
+ ]

 ❯ test/unit/selfhost-metrics.test.ts:498:21

test/unit/queue-5.test.ts > queue processors > type label decoupling (#label-decoupling) > REGRESSION (#4528, PR #4494 shape): keeps the propagated labels on the PR's own merge-closed webhook, instead of falling back to the title guess

Stack Traces | 0.205s run time

AssertionError: expected [ 'gittensor:feature' ] to deeply equal [ 'gittensor:feature', …(1) ]

- Expected
+ Received

  [
    "gittensor:feature",
-   "gittensor:priority",
  ]

 ❯ test/unit/queue-5.test.ts:6660:34

test/unit/worker-entry-boundary.test.ts > worker entry boundary > does not reference pixelmatch, pngjs, visual-diff, gifenc, or sharp in worker-reachable source

Stack Traces | 0.507s run time

AssertionError: worker-reachable files must not mention Node-only visual diff/GIF/image deps: .../review/visual/visual-findings.ts: expected [ Array(1) ] to deeply equal []

- Expected
+ Received

- []
+ [
+   ".../review/visual/visual-findings.ts",
+ ]

 ❯ test/unit/worker-entry-boundary.test.ts:82:118

View the full list of 5 ❄️ flaky test(s)

test/contract/engine-parity.test.ts > predicted-gate engine parity (#2286) > 'merge-readiness-composite-preserves-e…' matches the committed golden output