Skip to content

mcp: advertise the registry's projection on all three servers (#9655) - #9812

Merged
JSONbored merged 1 commit into
mainfrom
mcp/advertise-contract-metadata-9655
Jul 29, 2026
Merged

mcp: advertise the registry's projection on all three servers (#9655)#9812
JSONbored merged 1 commit into
mainfrom
mcp/advertise-contract-metadata-9655

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Closes #9655. Also closes #9656 and #9657 — all three are the same registration path, and fixing one without the others would leave the check that proves it green for the wrong reason.

The defect

@loopover/contract gives every tool a title, a description and an annotations posture, and projectToolDefinitions applies { readOnlyHint: true, destructiveHint: false } so the projection every consumer reads carries a complete, defaulted pair. None of the three servers advertised that projection, and nothing could see it: diffToolSets compares name sets, and checkAdvertisedShape only asks whether a description is non-empty.

Server What it advertised
Remote (src/mcp/server.ts) A description literal per call site — 35 had drifted from the contract's. No tool title, no annotations anywhere in the file.
Stdio contract.annotations raw — a Partial stating only what differs from the default. Five tools shipped readOnlyHint with no destructiveHint; ~95 shipped no annotations at all.
Miner Neither, on all 21 registrations.

The drifted descriptions matter because the two texts reach different audiences: the remote's tools/list served the inline literal, while listToolDefinitions() — which feeds the OpenAI/Anthropic spec builders and the .well-known catalogs — served the contract's. The posture matters because an MCP client that gates a confirmation prompt on destructiveHint got nothing from the server that performs the writes.

The miner's 21 -32602s

Every miner registration passed SomeSchema.shape. The SDK re-wraps a raw shape in a plain z.object that discards the catchall — and every miner output is a looseObject:

as the object : {}        ← additionalProperties
via .shape    : false

So all 21 were advertised and enforced as additionalProperties: false, and any field a payload carried beyond the modelled set came back as a -32602 the caller could do nothing about. This is exactly the class #9762 fixed on the other two servers — but its guard's SERVER_ROOTS omitted packages/loopover-miner/bin, so it reported zero offenders while the server it never looked at had 21. A guard that can quietly watch nothing is worse than no guard, so it now proves its own roots cover every file that registers a tool (verified failing when a root is removed).

What changed

  • One registration helper per server, each filling every advertised field from the projection: the remote's register wrapper resolves getToolDefinition(name) and throws on a name with no entry; registerStdioTool uses the projected annotations; registerMinerTool replaces 21 hand-written config objects.
  • The contract's projection gained a singular, total form (projectToolDefinition), so a server that already resolved a contract applies the defaults without a second lookup that can fail.
  • ToolContract is generic in its input/output schemas, so a server registering from an entry gets that tool's real argument type in its handler rather than the erased z.ZodObject.
  • checkAdvertisedMetadata (scripts/lib/validate-mcp/invariants.ts) compares advertised title/description/posture against the projection, and runs for all three surfaces in validateSurface.

#9656 — the remote's descriptions and categories

166 advertised literals deleted from the call sites. MCP_TOOL_CATEGORIES was already derived from TOOL_CONTRACTS and MCP_TOOL_CATEGORY_IDS already aliases the contract's TOOL_CATEGORIES, so the category half needed nothing further; _meta.category now reads the typed projection instead of an index-signature lookup that yielded undefined for an unlisted name with no compile error.

#9657 — the fifth admin tool

loopover_admin_rotate_secret was the one tool still registering from schemas declared in src/mcp/server.ts while its four siblings read the contract. Those local declarations are deleted and the registration reads AdminRotateSecretInput/AdminRotateSecretOutput.

The reason it survived is fixed too: the admin tools register only under LOOPOVER_MCP_ADMIN_ENABLED, which no validate-mcp case set — so the admin category was never diffed, compiled, smoke-called or output-validated, and the validator's own "nothing is registered without a contract entry" assertion was structurally unable to see the one tool that violated it. A second remote surface with the flag set now runs the full pass: 139 tools, 99 output-validated, up from 134.

No posture changed

Requirement 4 of #9655, held exactly. Notably loopover_delete_branch is not advertised destructive: the remote tool builds a local-execution spec and touches nothing, and the hints describe the tool call rather than the command a caller may later run with their own credentials. The test says so, so a future reader does not "fix" it.

Validation

  • npx vitest run --changed=origin/main: 2528 passed, 171 files. test/contract/: 108 passed. tsc --noEmit clean.
  • checkAdvertisedMetadata unit-tested for: agreement, a rewritten title, a rewritten description, each hint disagreeing, a tool advertising no annotations at all, and staying quiet about a tool the server never registered (that is diffToolSets' finding).
  • Posture asserted end-to-end on all three servers — over the real stdio transport for the CLI, and against additionalProperties for the miner's loose outputs.
  • docs:drift-check, manifest:drift-check, command-reference:check, ui:openapi:check, contract:api-schemas:check all clean.

@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-29 12:34:57 UTC

13 files · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Manual Review

Review summary
This PR centralizes MCP tool advertising (title/description/annotations) through the contract's `projectToolDefinition`/`getToolDefinition` on all three servers, replacing per-call-site literals that had drifted (35 descriptions on remote, missing annotations on stdio, nothing on miner). It also fixes a real defect in the miner server: registrations previously passed `.shape` which the SDK re-wraps into a strict `z.object`, discarding the catchall that miner's `looseObject` outputs need — the new `registerMinerTool` helper passes the schema objects directly. The remote server's `register` wrapper correctly spreads `advertised` fields after `config`, and throws if a tool has no contract entry, closing the drift permanently rather than just this once.

Nits — 6 non-blocking
  • packages/loopover-mcp/bin/loopover-mcp.ts imports `getToolContract`/`projectToolDefinition` per the diff snippet shown but the full file wasn't reviewable in this pass to confirm stdio's own registration wrapper mirrors the remote/miner pattern exactly — worth double-checking descriptions aren't still hardcoded there.
  • src/mcp/server.ts:2432 and similar admin-tool registrations now leave a trailing comma after removing `annotations: contractAnnotations(...)` (e.g. `{ inputSchema: AdminGetStatusInput, outputSchema: AdminGetStatusOutput, }`) — harmless but inconsistent with the rest of the file's style.
  • The external brief flags several new numeric literals (`9655`, `-32602`, `21`) in comments only, not code — not actionable as real magic numbers.
  • Confirm test/unit/mcp-register-tool-shape-guard.test.ts (referenced in the diff context) actually asserts the miner's `.shape`-discards-catchall defect can't regress, since that's the highest-value regression test in this PR.
  • Consider extracting the repeated `{ inputSchema: X, outputSchema: Y, }` trailing-comma pattern via a lint/format pass across src/mcp/server.ts for consistency.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

CI checks failing

  • codecov/patch — 96.66% of diff hit (target 99.00%)

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9655, #9656
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (2 linked issues).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 14 registered-repo PR(s), 13 merged, 357 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 14 PR(s), 357 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Addressed
The diff introduces a shared registration helper (register wrapper in server.ts, registerMinerTool in miner) that pulls title/description/annotations from the contract's projection via getToolDefinition/projectToolDefinition, removing all per-call-site description literals and adding annotations/title advertising across all three servers, matching the required 'single registration helper' pattern

Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: not available
  • Official Gittensor activity: 14 PR(s), 357 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record
  • action: hold · clause: success
  • config: 45d5d176f9276f8a7a4b1f6af26b0a0786018c21274bb9bc1e018e3b0211781c · pack: oss-anti-slop · ci: failed
  • record: 1e469b76a1dcdca8008dd113b38a38f9e5ad7bd7d104c5fdc08568ddad65cbde (schema v5, head be0e20e)

🟩 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.

  • Re-run LoopOver review

@JSONbored JSONbored self-assigned this Jul 29, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@loopover-orb loopover-orb Bot added gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context labels Jul 29, 2026
@JSONbored
JSONbored force-pushed the mcp/advertise-contract-metadata-9655 branch from a1779ba to 5577551 Compare July 29, 2026 11:01
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.59%. Comparing base (9d7f567) to head (be0e20e).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
packages/loopover-contract/src/tools/index.ts 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #9812       +/-   ##
===========================================
+ Coverage   79.03%   91.59%   +12.56%     
===========================================
  Files         280      915      +635     
  Lines       58366   112692    +54326     
  Branches     6697    27078    +20381     
===========================================
+ Hits        46128   103220    +57092     
+ Misses      11955     8144     -3811     
- Partials      283     1328     +1045     
Flag Coverage Δ
backend 95.58% <96.66%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-contract/src/tool-definition.ts 100.00% <100.00%> (ø)
packages/loopover-mcp/bin/loopover-mcp.ts 67.42% <ø> (ø)
packages/loopover-miner/bin/loopover-miner-mcp.ts 97.29% <100.00%> (ø)
src/mcp/server.ts 97.35% <100.00%> (ø)
packages/loopover-contract/src/tools/index.ts 87.50% <50.00%> (ø)

... and 763 files with indirect coverage changes

One contract entry produced three differently-advertised tools, and nothing could see it:
`diffToolSets` compares name sets, and `checkAdvertisedShape` only asks whether a description is
non-empty.

- The REMOTE server wrote a description literal at every `register(...)` call site -- 35 had drifted
  from the contract's, which is the text `listToolDefinitions()` serves to the agent-spec builders
  and the `.well-known` catalogs, so one tool was described two different ways depending on which
  LoopOver surface you asked. It advertised no tool `title` and no `annotations` at all, so a client
  that gates confirmation on `destructiveHint` saw nothing for the server that performs the writes.
  The `register` wrapper now fills all three from `getToolDefinition(name)`, spread after the call
  site's config so a stray literal cannot win, and throws on a name with no contract entry.

- The STDIO server passed `contract.annotations` raw. That field is a `Partial` stating only what
  differs from the default posture, so the five tools declaring one field advertised it without a
  `destructiveHint`, and the ~95 declaring none advertised no annotations -- both disagreeing with
  what the registry publishes for the same tool.

- The MINER server passed neither, and passed every schema as `SomeSchema.shape`. The SDK re-wraps a
  raw shape in a plain `z.object` that DISCARDS the catchall, and every miner output is a
  `looseObject`: all 21 were advertised and enforced as `additionalProperties: false`, so any field a
  payload carried beyond the modelled set came back as a -32602 the caller could do nothing about.
  #9762 fixed this class elsewhere and added a guard, but its roots omitted
  `packages/loopover-miner/bin` -- the guard reported none while the server it did not look at had 21.
  A `registerMinerTool` helper replaces all of them, and the guard now proves its own roots cover
  every file that registers a tool.

Also closes #9656 and #9657, which are the same registration path: the remote's descriptions come
from the contract now (its `MCP_TOOL_CATEGORIES` was already derived), and
`loopover_admin_rotate_secret` -- the one tool still registering from schemas declared in
`src/mcp/server.ts` -- reads its four siblings' contract entry instead. The reason it survived is
fixed too: every case booted a server without `LOOPOVER_MCP_ADMIN_ENABLED`, so the admin category was
never diffed, compiled, smoke-called or output-validated, and the validator's own "nothing is
registered without a contract entry" assertion was structurally unable to see the tool that violated
it. A second remote surface with the flag set now runs the full pass over all five.

`ToolContract` becomes generic in its input/output schemas so a server registering from an entry gets
that tool's real argument type in its handler rather than the erased `z.ZodObject`.

New invariant `checkAdvertisedMetadata` compares advertised title/description/posture against the
projection, and runs for all three surfaces. No tool's declared posture changed.
@JSONbored
JSONbored force-pushed the mcp/advertise-contract-metadata-9655 branch from b365e8c to be0e20e Compare July 29, 2026 12:18
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 29, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  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 be0e20e Commit Preview URL

Branch Preview URL
Jul 29 2026, 12:20 PM

@JSONbored
JSONbored merged commit 4b1f5e2 into main Jul 29, 2026
8 of 9 checks passed
@JSONbored
JSONbored deleted the mcp/advertise-contract-metadata-9655 branch July 29, 2026 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

1 participant