feat(mcp): generate every tool, CLI, client, and docs surface from the contract (#9521) - #9590
Merged
Conversation
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 00:16:58 UTC
Review summary Nits — 8 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
Visual preview
Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy. Scroll preview
A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
loopover-ui | a14b7d7 | Jul 28 2026, 10:07 PM |
JSONbored
force-pushed
the
feat/generated-surfaces-9521
branch
from
July 28, 2026 20:32
f49bdce to
8aae70f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9590 +/- ##
==========================================
+ Coverage 89.85% 89.95% +0.10%
==========================================
Files 880 884 +4
Lines 111175 111247 +72
Branches 26473 26438 -35
==========================================
+ Hits 99891 100074 +183
+ Misses 9992 9843 -149
- Partials 1292 1330 +38
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Bundle ReportBundle size has no change ✅ |
JSONbored
force-pushed
the
feat/generated-surfaces-9521
branch
from
July 28, 2026 21:54
d75f6fe to
dbf8624
Compare
…ct registry First #9521 batch. One generator (scripts/gen-mcp-tool-reference.ts, the gen-command-reference pattern) now emits, between markers, with --check in test:ci: - the stdio README's tool table (which previously documented nothing and told users to run 'loopover-mcp tools' instead); - the miner README's tool section, replacing a hand-kept 12-bullet prose list; - a typed apps/loopover-ui/src/lib/mcp-tool-reference.ts module for the docs site; - the stdio README's telemetry property table, generated from LEGACY_MCP_TELEMETRY_PROPERTY_KEYS. That constant is new: the legacy mcp_tool_call event's four-property allowlist existed three times (src/mcp/telemetry.ts, packages/loopover-mcp/lib/ telemetry.ts, README prose) with nothing holding them together. Both modules now build the event through one shared builder whose SIGNATURE is the allowlist -- there is nowhere in it to put a fifth field. check-docs-drift.ts gains the MCP surface: every loopover_* token in apps/loopover-ui/content/docs must exist in the registry. Non-tool uses of the prefix (a Postgres db name, a secrets filename, Prometheus metric prefixes) sit in an allowlist with per-entry reasons and an anti-rot check; metric-suffix and glob forms are excluded by convention, with a registry-side guard so no future tool can take a metric-shaped name and slip out of the scan. 12 real mentions verified; the very first run caught that the scan WOULD have flagged nothing -- every one of the 32 hand-typed names the issue counted had already drifted out of the docs or into non-tool uses. Stale @loopover/mcp/3.0.0 samples in the stdio README become version-agnostic.
…he CLI The CLI surface was kept in FOUR hand-maintained copies: a flat command-to-subcommands table, a 34-branch if-chain in runCli, a hand-written help string, and the README's command block. They had already drifted -- printHelp silently omitted commands the dispatch chain accepted. CLI_COMMAND_SPEC now carries usage lines alongside subcommands and is exported, so printHelp, all four shell-completion builders, dispatch, and the README all derive from it. The if-chain becomes a Record keyed by CliCommand, which makes tsc reject a command without a handler or a handler without a command; the README block is generated between markers by gen-mcp-tool-reference.ts with --check enforcing it in CI. The handler Record is a hoisted function rather than a const: the entrypoint awaits runCli during module evaluation, so a const declared below it is still in its temporal dead zone at call time. The old if-chain was accidentally safe because it called hoisted function declarations. The parity test imported the spec directly instead of regex-scraping the built bundle, which could pass against a stale dist and could not see a type error at all. The handler side is still read from source, since each run*Cli parses its subcommands inline.
…om the spec parseOptions hand-listed its repeatable and boolean flags as two literal Sets sitting inside the function, so adding a repeatable flag to a command only accumulated if someone also remembered to edit them. CLI_FLAG_SPEC now declares the kind once and parseOptions derives both sets from it. printCacheHelp, printAgentHelp, and printProfileHelp each carried their own copy of the usage lines the spec already held, and they had drifted: `agent start` appeared in the printer but not in the spec, so top-level help silently omitted it. All three now render from the spec's usage lines plus a note field, and the spec carries the full per-subcommand usage that the printers used to own. Their output is unchanged.
…tract Both copies said outright that they were hand-synced and why no shared home existed. Both had already drifted. github-token-resolution.ts re-implemented loopover-mcp's config path, profile selection, session lookup, and API-URL precedence, because @loopover/miner and @loopover/mcp are separately-installable CLIs. The API-URL precedence had diverged: loopover-mcp stopped at a legacy profile apiUrl and returned the default, so a stale profile value masked a valid top-level override, while the miner kept looking. The miner's behavior is the correct one and is now what both get. tenant-credential-resolution.ts duplicated the broker exchange and its ORB_BROKER_URL safety validation, because a relative import into root src/ resolves outside the package's rootDir and fails tsc with TS6059. Its local-host list still accepted a bare "::1" that broker-client.ts had dropped once a WHATWG URL was confirmed to always bracket IPv6. Harmless there, but this function decides where a bootstrap credential may be sent. @loopover/contract is the home both sides can reach, since both already depend on it. The new modules keep it Workers-safe: local-config.ts is pure policy with the filesystem left to its callers, and orb-broker.ts uses only URL, fetch, and AbortSignal. The package's tsconfig moves from "types": [] to the Workers types, which is the same rule stated more precisely -- node builtins still cannot compile there, but the globals the Worker really has are now available. Both new modules are covered at 100% line and branch, and the three branches the miner's new profile fallbacks introduced are covered in its own suite.
…rker serves (#9282) PublicStats was a hand-authored TypeScript interface kept in sync with the backend by whoever last touched both files, and it had drifted in five ways at once: it was missing fleetAccuracy.basis and rulePrecision.rules[].confirmed outright, typed decidedCount as optional where the wire says nullable, typed accuracyTrend's counts as non-null where the wire says nullable, and marked rulePrecision optional where the wire says required. None of that produced a single compiler or CI signal. This lands option 1 from that issue's recorded decision. The shape moves to @loopover/contract/public-api, which both the Worker and the UI already reach, and the UI takes z.infer of the same object. src/openapi/schemas.ts keeps only the OpenAPI decoration, re-wrapping through the local z.object because extendZodWithOpenApi attaches .openapi at construction time and the contract package must not depend on zod-to-openapi. The generated openapi.json is byte-identical. The two component fixtures grow the fields the wire always carries. The one test that genuinely needs a payload without rulePrecision -- an older deployed Worker, which the current schema no longer describes -- now strips it explicitly instead of relying on a type that quietly permitted it. check-ui-derived-types.ts is what stops this recurring: for every type the shared module exports, no file under apps/loopover-ui/src may declare its own type or interface of that name, so migrating another response shape extends the check to it with no edit. It also asserts the pilot still imports the shared module, since regressing to a hand-authored shape would leave the shared names unused rather than duplicated. The generated tool-reference module joins openapi.json in .prettierignore for the reason already recorded there: prettier rewrites the emitted literal, and its drift check then fails against a file nobody edited.
…ndary
The stdio CLI read every API response as `payload: any` and picked fields out by
optional-chaining guesswork, so a renamed Worker field degraded silently at runtime.
gen-contract-api-schemas.ts generates @loopover/contract/api-schemas from
src/openapi/schemas.ts, which stays canonical and untouched: moving the schemas was
tried first and rejected because zod-to-openapi's .openapi() exists only on schemas
constructed after extendZodWithOpenApi runs, and both post-hoc naming routes (.openapi()
clones; .meta({id}) propagates through .nullable()/.extend() differently) changed the
published document. Generation with the names stripped keeps the document byte-identical
by construction, with --check in test:ci like the other generators.
The generator also emits CLI_RESPONSE_SCHEMAS, path -> response schema, derived from a
scan of the CLI's own literal api* call sites joined against the published document --
nothing hand-listed. The scanner requires the closing delimiter: the first draft
collected a template path's truncated prefix, which for a documented base path would
have validated the wrong endpoint's schema. apiGet/apiPost are overloaded so a literal
documented path returns the schema's inferred type, and apiFetch safeParses every
documented response under #9519's recorded posture: report and pass the payload through
untouched by default (returning zod's parse would strip fields the document
under-describes), throw under LOOPOVER_VALIDATE_RESPONSES for CI and self-host. The
warning dedupes per path so a polling loop cannot flood stderr, and lives in a hoisted
accessor because the entrypoint awaits runCli before module consts this far down
initialize.
Typing the responses immediately surfaced real drift: /v1/local/branch-analysis returns
predictedGate and dataQuality, which the document omitted entirely while two predict-gate
tools read them; both are now declared (as unknown until #9531 types the verdict). The
skipped-pr-audit call site moves its query out of the path template so the scanner keeps
it validated.
`payload: any` is extinct in the package and pinned by a grep gate test, along with the
table matching the scan exactly and the unvalidated remainder as a shrink-only list. The
posture tests drive the real compiled bin against a local server in both modes.
Adding four modules to @loopover/contract produced a GREEN `@loopover/contract:build` in CI followed five seconds later, on the same runner, by `Cannot find module '@loopover/contract/local-config'` from the miner's build. `tsc -p` with `incremental` (inherited from the root tsconfig) decides what to emit from .tsbuildinfo alone -- it never checks whether the outputs that stamp describes are still on disk. turbo caches .tsbuildinfo alongside dist/, so a run where the two come back out of lockstep, or a --force run over a leftover stamp, makes tsc declare itself up to date and emit nothing. Reproduced exactly by deleting dist/ with the stamp left in place. All three packages that cache .tsbuildinfo as a turbo output have the same exposure, so each clears the stamp before compiling; any new file in any of them could have hit this. The UI preview workflow separately never built the contract at all -- it builds the engine and then runs ui:openapi, which now reaches src/openapi/schemas.ts's re-export of the public-API schemas. It builds both packages now. package-lock.json was out of sync with two real changes: the contract's new @cloudflare/workers-types devDependency and the UI's new @loopover/contract dependency.
…was silently dropping The module was never committed. `local-config.*` is a common global-gitignore pattern, so every commit silently omitted the file while the local tree built fine — CI then failed on a module that did not exist in the repo. The clean-clone reproduction is what found it: dist/ had orb-broker and public-api but no local-config, because the source was not there. Renamed to cli-config, which no common pattern matches, with a note not to rename it back. Also hoists the contract build above the drift checks, which import the tool registry.
JSONbored
force-pushed
the
feat/generated-surfaces-9521
branch
from
July 28, 2026 22:04
dbf8624 to
a14b7d7
Compare
…r does not fail it The packaging-convention test asserted build:tsc was exactly `tsc -p tsconfig.json`, which the incremental-emit fix breaks by prefixing the stamp clear. Matched as a suffix so the guard still pins the compile itself.
…ead fallbacks codecov/patch was 86.76% with 27 lines missing, all in packages/loopover-mcp/bin. The cause is structural: nearly every CLI test spawns the real compiled bin, which is right for testing the process boundary but reports no coverage back to vitest — the work happens in another process. So the dispatch table, the derived help printers, and the response renderers were exercised without being measured. These drive the same paths in-process through the exported runCli and the in-memory MCP transport, the pattern mcp-cli-basics and mcp-cli-list-notifications-tool already use. The response-validation suite pins both halves of #9519's recorded posture: the default warns on stderr and returns the payload untouched, and LOOPOVER_VALIDATE_RESPONSES opts into throwing. Driving it needed fixture responses that genuinely violate a schema, so the harness gains three narrow options — a body that fails validation, one that is valid JSON of the wrong top-level type, and one that is not JSON at all on a 200. Picking a violating path took care: several validated responses declare every field optional, so `{}` satisfies them and the test would have passed while proving nothing. Three fallbacks turned out to be unreachable rather than untested, so they are gone instead of covered: printableUsage's missing-note arm (its parameter is now narrowed to the commands that declare one), `path.split("?")[0] ?? path` (split always yields an element), and the repeated `watch.labels ?? []` inside its own length check. The mismatch reporter's `issues[0]` is likewise always present on a failed parse; only its path can be empty, which is what "(root)" is for.
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
The hand-edited list this branch added is replaced by the generated one now that #9590 has landed -- which is what the issue asked for: the tool tables pick new tools up with no hand-edits. 163 registry entries across the three servers.
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
The hand-edited list this branch added is replaced by the generated one now that #9590 has landed -- which is what the issue asked for: the tool tables pick new tools up with no hand-edits. 163 registry entries across the three servers.
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…dflare Workers build The Workers build for loopover-ui has been failing on every PR since #9521 (merged as #9590) made src/openapi/schemas.ts import @loopover/contract/public-api: Cannot find module '.../node_modules/@loopover/contract/dist/public-api.js' imported from /opt/buildhome/repo/src/openapi/schemas.ts ui:build builds ui-kit and engine, then runs ui:openapi -- but never builds the contract package, so the import resolves to a dist/ that does not exist. CI did not catch it because the GitHub workflow has its own separate "Build contract package" step (ci.yml:361) before the drift checks; the Cloudflare build runs npm run build:cloudflare -> ui:build directly and gets no such step. The two paths had silently diverged. Add @loopover/contract to the same turbo invocation that already builds the engine, so the one script both paths share produces everything ui:openapi imports. Reproduced locally by deleting packages/loopover-contract/dist and running ui:openapi (identical ERR_MODULE_NOT_FOUND), then confirmed the fixed chain builds the package and writes the spec with no drift.
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…doctor, tenant health (#9523) (#9607) * feat(mcp): add the AMS management tool family behind the governor gate The miner MCP exposed 11 read-only tools while its entire mutating ops surface was CLI-only. This adds 10 tools: a dedicated doctor (split out of status so status stays cheap), a structured metrics snapshot, and eight mutations. Every mutation dispatches through the miner's existing governor-gated chat-action chokepoint -- the same boundary the dashboard's own actions use. The MCP layer never touches a store: it dispatches an action NAME, and the registry structurally refuses any handler not produced by governorGatedHandler(), whose brand is a private symbol a raw function cannot forge. That is what makes "an MCP caller cannot reach a write path the dashboard could not" a property of the code rather than of review discipline, and the structural test asserts it from both ends. Three claims I had to correct against the real code rather than ship as written. The migrate CLI has no dry-run, because applying a migration IS opening the store -- so the tool has no apply flag either, instead of advertising a safety mode that does not exist. The deny-hook store keys proposals by (repo, id), so the decide tool takes the repo rather than scanning every repo's proposals to resolve an id. And purgeRepoAcrossStores is extracted from runPurge so the tool runs the CLI's own purge over the CLI's own target list, rather than a second implementation free to miss a store. collectMinerPredictionMetrics is likewise extracted in the engine: the Prometheus text renderer now formats those families, so the scrape and the JSON snapshot share one aggregation and cannot disagree about what a counter means. validate:mcp earned its keep twice here. It caught the two AMS tenant tools declared but never registered, and it caught loopover_miner_run_migrations/_purge_repo returning fields their output schemas did not declare -- the .shape re-wrap that drops looseObject's catchall, the same -32602 class this epic already fixed once. Both outputs now declare every field their handlers return. AMS tenant create/list/destroy are deliberately absent: #9522's loopover_tenant_* tools are product-parameterized and already serve product "ams", because the control plane's routes are. Only health and wake -- the pair with no ORB counterpart -- are added. The catalog's recorded exclusions (calibration floors, raw run-state set, the one-way kill switch) stay CLI-only, pinned by a test so reversing that decision has to be deliberate. * test(mcp): cover the miner ops actions and the full dispatch path The store operations and the registered handlers were only reachable through the MCP tools, so nothing measured them directly. These drive each action end to end -- action name through the governor gate to the store call -- plus the validator guard clauses for null, primitive, and array params, and the fail-closed paths (flag disabled, unknown action, malformed params never reaching a store). registerMinerOpsChatActions gains the same evaluateGate seam the dashboard's own registerPortfolioQueueChatActions already exposes, so a test can drive the dispatch path without the real chokepoint; production passes none and gets the real one. paramsOf loses its `?? {}`: every action that calls it has a validator requiring an object, and dispatchChatAction runs that validator first, so the fallback had no reachable case. * chore(docs): let #9521's generator own the miner tool table The hand-edited list this branch added is replaced by the generated one now that #9590 has landed -- which is what the issue asked for: the tool tables pick new tools up with no hand-edits. 163 registry entries across the three servers. * test(mcp): cover the AMS tools' registration and result-shaping layers codecov/patch was short because the new tool REGISTRATIONS were never driven in-process -- the miner bin's 169 changed lines had 54 uncovered statements, and the AMS tenant handlers had none of their own tests at all. These drive every new tool over the in-memory transport against injected seams: the doctor mapping from status.js's {name, ok, detail} onto the contract's pass/warn/fail, the metrics snapshot's shared aggregation, and each mutating tool's dispatch and result shaping -- including that a governor refusal comes back as a structured blocked result rather than a thrown error, and that a purge with confirm absent or false is rejected by the schema BEFORE any dispatch happens. The AMS tenant pair gets its not-configured, healthy, and throttled paths, the last of which is the one that must not be audited as a cycle: the schedule guard refusing a too-soon wake is the guard working, not work that ran. Writing the metrics test surfaced a fixture trap worth naming: toPredictionRecords reads the LEDGER row shape (conclusion/targetId/ts), so a fixture shaped like the downstream record yields conclusion: undefined and fails output validation. The test now uses the ledger's own shape. * test(engine): cover the prediction-metrics split in the engine's OWN suite codecov reported this file at 35.84% under the `engine` flag while the root vitest suite showed it fully covered. Both were right: @loopover/engine uploads coverage from its own node:test suite (packages/loopover-engine/test/**), and that suite had no test for this file at all — the vitest one lands under `backend` and does not lift the engine flag. So the aggregation and the renderer get tests where the engine actually measures itself, including the property the split exists for: every sample `collectMinerPredictionMetrics` reports appears verbatim in what `renderMinerPredictionMetrics` emits, so the JSON snapshot and the Prometheus scrape cannot report different numbers. Also pins the escaping, which is a correctness rule rather than cosmetics: a conclusion is DATA, so a quote, backslash, or newline inside one must not forge a second series.
JSONbored
added a commit
that referenced
this pull request
Jul 29, 2026
…#9569) (#9608) * feat(proof): public per-repo proof summary, endpoint and README badge (#9569) The shareable, unauthenticated twin of the in-app trust panel. One composition serves both, so the public page and #9193's panel cannot disagree about a figure -- which is the property the page exists to demonstrate. THE PRIVACY BOUNDARY IS STRUCTURAL. Every field is built by NAMING it, never by filtering a wider object. A blocklist has to anticipate every field a future upstream type might grow and silently leaks the one it did not; an allowlisted shape cannot leak a field nobody wrote down. Tested by feeding hostile records carrying hotkey/wallet/reward/trust-score/private- rank and asserting none of it reaches the serialized page -- while the named fields do, so the test proves allowlisting rather than an empty object. NEVER A BARE SCALAR. Any accuracy figure carries its coverage and a Wilson interval; below a 20-decision floor there is no rate at all, only an explicit insufficient_data state that still publishes the count. A perfect record over 19 decisions must not render as 100%. Wilson rather than Wald because a gate metric lives near p->1, exactly where Wald claims impossible certainty. HONEST BOUNDARY STATES. An empty ledger is `empty`, not `verified` -- different claims. A failed read is `unavailable`, not `broken`, which would accuse the operator of tampering. A FAILED anchor attempt is not an anchor: the public attempt log is where failures are legible, and presenting one here would claim corroboration that does not exist. The verification-contract boundary statement travels IN the payload, so a screenshot or embed cannot shed it the way a footer caption can. The badge reports the LEDGER's state rather than an accuracy percentage: a badge is a one-glance claim, and an accuracy number without the interval that makes it honest does not fit in one. Disabled and errored both render a neutral SVG -- a broken image in a README is worse than an honest "unavailable". DECISION (requirement 6), recorded beside the code that implements it: the page is opt-OUT per repo, default ON once the operator's fleet-wide flag (default OFF) is on. Every figure is already publicly fetchable through the ledger-verify / anchors / decision-record endpoints, so gating a page over it would add friction without privacy. The per-repo switch still exists because a page is a different artifact from an API -- discoverable, linkable, and it markets a repo's numbers whether or not the maintainer wants that. A repo can opt out but cannot opt IN when the operator has not, which keeps the fleet switch a real switch. Found and fixed while testing: `DB.prepare()` throws SYNCHRONOUSLY on a driver-level failure, so the `.catch()` chain never ran and a D1 outage would have 503'd the whole public page instead of degrading. Each section is now a real try/catch, which is the difference between the fail-safe-per-section contract being documented and being true. Backend half of #9569; the /proof/:owner/:repo UI route renders this payload and lands separately. * fix(proof): actually wire the per-repo opt-out the routes only claimed to honor (#9569) Review caught the real defect: both handlers called isProofPageEnabledForRepo(c.env) with no second argument, so the ProofPageRepoOverride documented at length in proof-summary.ts and in the PR body was never loaded or passed. Every repo was effectively opt-out-less once the fleet flag was on -- a gate that is described, typed, and unit-tested as a pure function, but never reachable from the surface it governs. That is the registered-but-unreachable class, and the long comment made it worse rather than better by making it look done. - Adds a real `publicProof:` focus-manifest block (engine parser + toJson + loader snapshot), mirroring `publicStats:`/`ops:`. Precedence is deliberately the opposite of those two: read from the TARGET repo's manifest rather than the operator's self-repo, because the thing being opted out of is that repo's own page. - loadProofPageRepoOverride resolves it, degrading a failed manifest load to "no override" -- a broken manifest never takes a page DOWN, which is the failure direction worth accepting here and is now stated in the doc comment rather than left implicit. - Both routes load the override BEFORE anything else, so a repo that turned its page off does not have its decision records queried to build a summary that will be discarded. - Documents the block in .loopover.yml.example, including the precedence and the opt-out default. Tests that would have caught it: a repo opting out in its manifest now gets 404 from BOTH routes with the fleet flag on, while a different repo in the same fleet still serves 200 (the opt-out is per repo, not a kill switch); explicit opt-in and no-block-at-all both serve; and the resolver is covered across absent/explicit/failing loads. * fix(build): build @loopover/contract in ui:build, unbreaking the Cloudflare Workers build The Workers build for loopover-ui has been failing on every PR since #9521 (merged as #9590) made src/openapi/schemas.ts import @loopover/contract/public-api: Cannot find module '.../node_modules/@loopover/contract/dist/public-api.js' imported from /opt/buildhome/repo/src/openapi/schemas.ts ui:build builds ui-kit and engine, then runs ui:openapi -- but never builds the contract package, so the import resolves to a dist/ that does not exist. CI did not catch it because the GitHub workflow has its own separate "Build contract package" step (ci.yml:361) before the drift checks; the Cloudflare build runs npm run build:cloudflare -> ui:build directly and gets no such step. The two paths had silently diverged. Add @loopover/contract to the same turbo invocation that already builds the engine, so the one script both paths share produces everything ui:openapi imports. Reproduced locally by deleting packages/loopover-contract/dist and running ui:openapi (identical ERR_MODULE_NOT_FOUND), then confirmed the fixed chain builds the package and writes the spec with no drift. * fix(manifest): register publicProof as a known top-level field, and sync the example template Two failures from the #9569 manifest block, both mine. 1. The unknown-top-level-field validator never learned about `publicProof`, so every manifest carrying it warned "Manifest contains unknown top-level field: publicProof." That was invisible on the first pass and appeared on every LATER one, because the first pass parses a manifest with no such key while later passes reload the persisted snapshot -- which my loader change now serializes the field into. The warning lands in the published review comment, so an unchanged PR got a fresh comment PATCH on every regate sweep: exactly the #3379 churn that test exists to prevent, reintroduced by a field the writer knew about and the reader did not. Found by instrumenting the test's PATCH interception to diff the two comment bodies rather than guessing at the cause; the added line named itself. 2. config/examples/loopover.full.yml must mirror .loopover.yml.example from "WHERE IT LIVES" onward, and I documented the block in only one of the two. Verified against origin/main first to confirm both were regressions from this branch rather than pre-existing. * test(proof): close the patch-coverage gaps, and fix a second sync-throw the gap exposed Codecov flagged 8 uncovered changed lines across focus-manifest.ts and routes.ts. I had measured coverage on proof-summary.ts and proof-badge.ts only, and never on the two files the manifest block and the routes actually touched -- so the gap was in my own verification, not just the tests. Closing it turned up a real defect rather than only missing assertions: loadProofPageRepoOverride used `.catch()` on the injected manifest loader, so a loader throwing SYNCHRONOUSLY (a driver-level failure before it ever returns a promise) skipped the handler entirely and would have escaped to the route -- 503ing a public page over a manifest read that is supposed to be optional. That is the same defect this file already had in loadProofSummary's section reads, which I fixed there and then reintroduced here. Now a real try/catch, with a regression test using a synchronously-throwing loader. Coverage: - parsePublicProofConfig / publicProofConfigToJson: explicit on/off, a present-but-empty block (present-but-false, which the resolver keys on), absence, three non-mapping shapes warning rather than throwing, and a snapshot round-trip. - A regression test asserting publicProof is a KNOWN top-level field, so the writer/reader split behind the #3379 regate churn cannot return. - The two route 503 arms are unreachable today (every inner read is individually fail-safe), so they are excluded with the house v8 pragma and a note on why they are kept: a future unguarded read should degrade to 503 rather than 500 on an unauthenticated public route. The badge arm uses ignore start/stop -- `next 2` miscounts across a multi-line comment and left the return uncovered. All three changed files now report zero uncovered changed lines. * refactor(proof): one shared resolver for both surfaces, and delete the unreachable arms Replaces the coverage pragmas with the fix they were papering over. The gate, the read and the outcome now live in ONE resolver (resolveProofPage) that both handlers render. That is not tidiness: the gate previously lived inline in both route bodies and exactly one of them was wired to the per-repo opt-out, which is the defect review caught. A shared resolver makes "the page and the badge agree about whether this repo is published" true by construction instead of by two call sites remembering the same thing. With that in place the two 503 arms were provably unreachable, because loadProofSummary is TOTAL -- every read is wrapped per section, so a failing ledger/anchor/record read degrades to that section's honest neutral state and the page still composes. Rather than excluding dead branches from coverage, the outcome is gone from the type: ProofPageResult is `ok | disabled`. A test asserts the totality directly -- every dependency failing at once, including a DB binding that throws on property access, still resolves to a rendered page in its neutral states. Same treatment for buildProofAccuracy's `!interval` guard: wilsonInterval returns null exactly when there are no trials, which IS the nothing-decided case, so one reachable guard covers both reasons a rate is unpublishable instead of a dead branch behind a pragma. Net: no `v8 ignore` pragmas anywhere in the #9569 code, and zero uncovered changed lines or branches across proof-summary.ts, routes.ts and focus-manifest.ts. --------- Co-authored-by: loopover-orb[bot] <296761690+loopover-orb[bot]@users.noreply.github.com>
This was referenced Jul 29, 2026
This was referenced Jul 29, 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.
Closes #9521. Executes and closes #9282 (its recorded option 1, per that issue's decision record).
What changed
Every hand-maintained MCP/CLI/docs surface this issue inventoried is now generated from, derived from, or validated against one source of truth, with a
--checkdrift gate intest:cifor each generated artifact.Generated reference surfaces (
scripts/gen-mcp-tool-reference.ts,mcp:tool-reference:check): the stdio README tool table, the miner README tool table, the typedapps/loopover-ui/src/lib/mcp-tool-reference.tsdocs module, the stdio README CLI command block, and the README telemetry-property table all emit from the@loopover/contractregistry between markers.check-docs-drift.tsgains an MCP surface: everyloopover_*token underapps/loopover-ui/content/docsmust exist in the registry.One CLI source of truth:
CLI_COMMAND_SPECis now a typed{subcommands, usage, note}table that drivesrunClidispatch (aRecord<CliCommand, handler>tsc exhaustiveness-checks — replacing a 34-branch if-chain),printHelp, the three sub-command help printers, all four shell-completion builders, and the generated README block.CLI_FLAG_SPECreplacesparseOptions' two hand-listed repeatable/booleanSetliterals. The regex-scraping test now imports the spec directly. Real drift this surfaced:agent startexisted in the hand-written help printer but not the spec, so top-level help silently omitted it.Typed validated API client:
scripts/gen-contract-api-schemas.tsgenerates@loopover/contract/api-schemasfromsrc/openapi/schemas.ts(which stays canonical — moving the schemas was tried and rejected because both post-hoc component-naming routes changed the published document; generation keepsopenapi.jsonbyte-identical by construction) plusCLI_RESPONSE_SCHEMAS, a path→schema table derived from scanning the CLI's literalapi*call sites against the published document — nothing hand-listed.apiGet/apiPostare overloaded so a documented literal path returns the schema's inferred type, andapiFetchsafeParses every documented response under #9519's recorded posture: warn on stderr (deduped per path) and pass the payload through untouched by default, throw underLOOPOVER_VALIDATE_RESPONSES(CI/self-host).payload: anyis extinct, pinned by a grep-gate test; the unvalidated remainder is pinned as a shrink-only list for #9531. Drift this surfaced:/v1/local/branch-analysisreturnspredictedGate/dataQuality, which the document omitted while two predict-gate tools read them — now declared.Miner dedup into
@loopover/contract(recorded choice: the contract package, the one home both separately-installable CLIs already depend on):local-config(config path/profile/session/API-URL policy, pure with fs injected so the package stays Workers-safe) andorb-broker(theORB_BROKER_URLvalidation + stored-secret exchange, also imported bysrc/orb/broker-client.ts). Both hand-copies had already drifted: the two CLIs disagreed on API-URL precedence past a legacy value (the miner's fall-through is correct and now shared), and the miner's local-host list kept a bare::1that #8334 removed. The contract tsconfig moves from"types": []to the Workers types — the same node-builtins ban stated precisely, with the globals the Worker really has.#9282 executed:
PublicStats/PublicRulePrecisionmove to@loopover/contract/public-api; the UI infers viaz.infer,src/openapi/schemas.tskeeps only the OpenAPI decoration (document byte-identical). The hand-typed interface had drifted five ways with zero signal (missingfleetAccuracy.basisandrules[].confirmed, wrong nullability ondecidedCountandaccuracyTrendcounts,rulePrecisionwrongly optional).check-ui-derived-types.ts(ui-derived-types:check, intest:ci) fails any hand-authored type/interface shadowing a shared name and grows automatically as shapes migrate.Telemetry allowlist single-sourced from
@loopover/contract(buildLegacyToolCallProperties, whose signature is the allowlist) consumed by both telemetry modules and the generated README section; stale3.0.0README samples are now version-agnostic.Validation
Full typecheck clean (root + all packages). All MCP suites (149 files), miner token/credential suites, contract suites (100% line+branch on the new modules), UI typecheck/tests/build,
ui:openapi:check,mcp:tool-reference:check,contract:api-schemas:checkall green after rebasing onto main. The response-validation posture is tested both against the real compiled bin (subprocess, both modes) and in-process for coverage.