feat(ams): migrate the AMS miner MCP server to @loopover/contract - #9542
Merged
Conversation
Contributor
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-28 08:38:57 UTC
Review summary Nits — 6 non-blocking
CI checks failing
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
🟩 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.
|
5 tasks
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9542 +/- ##
==========================================
- Coverage 89.59% 88.70% -0.89%
==========================================
Files 858 859 +1
Lines 110456 110512 +56
Branches 26300 26304 +4
==========================================
- Hits 98961 98029 -932
- Misses 10231 11513 +1282
+ Partials 1264 970 -294
Flags with carried forward coverage won't be shown. Click here to find out more.
|
JSONbored
force-pushed
the
feat/loopover-contract-package-9517
branch
from
July 28, 2026 07:56
d9cea33 to
6192bad
Compare
24 tasks
JSONbored
force-pushed
the
feat/miner-mcp-contract-9536
branch
from
July 28, 2026 08:01
5aed607 to
b885089
Compare
All 11 loopover_miner_* tools now register from the shared contract instead of
declaring their own zod shapes -- the last of LoopOver's three MCP servers to
gain a source of truth, and the only one with zero output schemas before this
(every handler returned unschematized text). Every tool now emits
structuredContent alongside its text block.
Two real defects surfaced during the migration, not before it:
- The output schema for loopover_miner_get_audit_feed initially wrapped
collectEventLedgerAuditFeed's return value in a second { events: ... } layer,
double-nesting a payload that already has that shape ({ repoFullName?, events }).
The MCP SDK's own structured-output validation caught it immediately --
"expected array, received object" -- confirming the schema is doing its job
as a runtime contract, not just documentation.
- list_claims, list_plans, and governor_decisions previously returned BARE ARRAYS
as their text payload (JSON.stringify(ledger.listClaims(...)), etc.), but an MCP
structuredContent value must be a JSON object per spec. Wrapping structuredContent
in { claims }/{ plans }/{ decisions } while leaving the text block as the
original array (via a small structured/text split in the result helper) is what
keeps every existing text-parsing consumer working unchanged -- confirmed by
running the real handler before committing to the shape, not by inspecting the
aggregator's declared TS return type alone.
Also: a unified error envelope across all 11 tools. Before, only
loopover_miner_get_audit_feed caught a store failure and returned isError: true;
the other ten threw, so the same failure surfaced as a clean result from one tool
and a raw protocol error from the rest.
test/unit/miner-mcp-scaffold.test.ts's hardcoded 11-name array and
miner-mcp-tool-docs-parity.test.ts's regex-scrape of the COMPILED dist bundle
(a pattern my own earlier investigation flagged as fragile -- a reformatted
registration call would silently break the guard, not the code) are both
replaced by a direct read of listToolDefinitions({ locality: ["miner"] }): one
list every server and every test derives from, no build step, no regex.
Full unit suite: 23041 tests, 1208 files, all green. build:miner (135 files,
check-syntax), test:miner-pack, and test:miner-deployment-docs-audit all pass
unchanged.
Closes #9536
…ct.input accessor Registering with minerXTool.input.shape/output.shape compiled fine standalone but broke once each handler declared an explicit z.infer parameter type: ToolContract.input is typed as the general z.ZodObject class (the type every contract entry shares), so z.infer<typeof minerXTool.input> collapses to a generic/incorrect shape instead of the tool's real fields -- and passing that same widened .shape to registerTool's inputSchema, while the handler declares the SPECIFIC narrower type, is a real contravariant mismatch the MCP SDK's overload correctly rejected (planId missing in type ShapeOutput<...>). Every registration and handler now references the tool's own named MinerXInput/MinerXOutput export from @loopover/contract/tools instead of the ToolContract.input/.output accessor -- the schema passed to registerTool and the type the handler destructures against are now the same concrete object. build:miner and the full repo typecheck are clean; 106 tests green across miner-mcp-scaffold/contract/audit-feed/manage-status/calibration-report/ governor-decisions/tool-docs-parity and contract-registry. Refs #9536
JSONbored
force-pushed
the
feat/miner-mcp-contract-9536
branch
from
July 28, 2026 08:21
b885089 to
b77e154
Compare
JSONbored
added a commit
that referenced
this pull request
Jul 28, 2026
…ontract Completes #9518. All seven remote-server categories -- admin, maintainer, review, branch, discovery, utility and agent -- now take their input and output schemas from the contract package instead of ~120 declarations local to src/mcp/server.ts. The remote server is the second of the three to be migrated to completion, after the AMS miner (#9542). A handful of inputs deliberately stay server-side, each for a stated reason: schemas carrying a .transform() or .default() (a caller must not be able to assert its own branch eligibility into its own score), one input shared verbatim with a REST route, and three bounded by constants a zod-only leaf package cannot import. Their outputs migrate either way. Two fixes fall out of the move: - The changed-file and validation-entry schemas behind loopover_run_local_scorer and loopover_preflight_local_diff, and both nested schemas behind loopover_suggest_boundary_tests, lost their .strict() in an earlier batch of this migration. That strictness is the no-upload boundary: without it a caller that smuggles a patch/content field gets a silently-stripped call instead of a rejected one, and believes the upload succeeded. Restored, with regression tests on both surfaces. - PLAN_STEP_STATUSES said 'in_progress' where both real surfaces -- the remote plan DAG and the miner's plan store -- say 'running'. Nothing consumed it yet, so nothing broke, but the first consumer would have rejected every running step the store has ever persisted. Corrected, aliased from the miner contract so there is one vocabulary, and pinned by a test. The REST<->MCP parity guards now read the contract rather than server-local declarations that no longer exist. Tool descriptions are relocated verbatim.
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
Migrates the AMS miner MCP server's all 11 tools to
@loopover/contract— the last of LoopOver's three MCP servers to gain a shared source of truth, and the only one with zero output schemas before this. Every handler returned unschematized text; now every tool emitsstructuredContentvalidated against a real schema.Stacked on #9530 (base branch here is
feat/loopover-contract-package-9517, notmain) — this diff is additive on top of that PR and should merge after it. Only the commitfeat(ams): migrate the AMS miner MCP server...is this issue's actual change; everything before it is #9530/#9528's history.Defects the migration surfaced, not assumed going in
loopover_miner_get_audit_feed's output schema initially double-wrapped the payload.collectEventLedgerAuditFeedalready returns{ repoFullName?, events }; my first pass at the handler wrapped that again in{ events: ... }. The MCP SDK's own structured-output validation caught it immediately —"expected array, received object"— proof the schema is a real runtime contract, not documentation. Fixed by returning the aggregator's result directly.structuredContentvalue cannot be (must be a JSON object per spec).list_claims,list_plans, andgovernor_decisionsall didJSON.stringify(store.listX(filter))— a raw array. Verified this against the real handlers, not just the aggregators' declared TS types. Fixed with a smallstructured/textsplit in the result helper:structuredContentgets the object wrapper the schema requires ({ claims: [...] }), while the text block stays the original bare array — so every current text-parsing consumer keeps working unchanged.Unified error envelope
Before this, only
loopover_miner_get_audit_feedcaught a store failure and returnedisError: true; the other ten threw, so the identical failure (a missing or corrupted local SQLite file) surfaced as a clean structured result from one tool and a raw protocol error from the rest. All 11 now route through onewithMinerToolErrorHandlingwrapper with a closed error-code set (store_unavailable|unknown_error).Two hand-maintained test surfaces replaced
test/unit/miner-mcp-scaffold.test.tshardcoded the 11 tool names in an array — adding a tool needed a manual edit here too. Now asserts againstlistToolDefinitions({ locality: ["miner"] })directly.test/unit/miner-mcp-tool-docs-parity.test.tsregex-scrapedserver.registerTool("loopover_miner_...")calls out of the compiled dist bundle — a pattern my own earlier investigation for this epic flagged as fragile (a reformatted registration call silently breaks the guard, not the code; it also required a freshnpm run build:minerto see any real change). Now reads the registry directly: no build step, no regex, same list every server registers from.Closes #9536
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally (scoped — see note)npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateGreen: full unit suite (23041 tests, 1208 files),
build:miner(135 files,check-syntaxat full count),test:miner-pack,test:miner-deployment-docs-audit, all 254 tests across every miner-mcp-* / contract-registry test file explicitly re-run, andtypecheck.If any required check was skipped, explain why:
Safety
structuredContentis new on every tool; every text block is byte-identical to before, verified explicitly for the three array-returning tools.)UI Evidencesection below. (n/a — no visible change)Notes
Part of #9515. Blocked by #9517 (stacked on its branch). All three MCP servers now register from
@loopover/contractfor at least their initial tool sets — the miner server completely, the remote/stdio servers for the #9517 pilot batch, with the rest landing in #9518/#9537.