feat(registry): carry a server's own initialize instructions on the overview - #14
feat(registry): carry a server's own initialize instructions on the overview#14yh928 wants to merge 1 commit into
Conversation
…verview The MCP handshake already returns `instructions` — free-form guidance the server wants the client to have — and `McpInitializeResult` already parses it. Both transports cache the result. Nothing then read it, so it was discarded at the point where it is most useful. `ConnectedServerOverview` is what a host renders a connected server from, and for a **hand-added** server it has almost nothing to render: there is no registry entry, so `description` is `None`, and the line degrades to a name and a tool count. The server's own instructions are the only thing that can say what it is for. `Connection` now captures the handshake's instructions at connect time and the overview carries them. Captured once rather than re-read per overview: the value cannot change without a reconnect, which rebuilds the record anyway. The read happens after `list_tools`, which cannot have succeeded without a completed handshake, so it hits the cache and does not dial again. A transport error yields `None` for the same reason a silent server does — the caller is describing a server, and a missing description is not worth failing over. The field is `#[serde(default)]`, so an overview written before it existed still decodes, as `None`. Untrusted remote text, and documented as such on the field: it sits on exactly the same footing as `description`, which this crate already warns callers to sanitize before putting in an LLM's context. 678 + 150 tests pass; fmt clean.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: Warning Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice. Comment |
How this change flows3 changed behaviours across 9 relationships. 5 surrounding behaviours are shown (60 graph nodes walked). 42 further behaviours left out to keep the diagram readable. flowchart LR
n0["ActiveClient<br/>changed"]:::changed
n1["Connection<br/>changed"]:::changed
n2["Connections<br/>changed"]:::changed
n3["connect_inner"]:::impacted
n4["dial_remote"]:::impacted
n5["all_status"]:::impacted
n6["dial_stdio"]:::impacted
n7["connect"]:::impacted
n1 -->|uses| n0
n2 -->|uses| n1
n3 -->|uses| n1
n3 -->|calls| n4
n3 -->|calls| n6
n4 -->|uses| n0
n5 -->|uses| n1
n6 -->|uses| n0
n7 -->|calls| n3
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
|
Correction to my own PR description. I wrote that Re-verified on stable 1.98.1:
Nothing about the change moved; only my claim about the repo was wrong, and I would rather correct it than leave a false note about your lint hygiene sitting in a PR description. Worth mentioning why I re-checked at all: on a sibling PR (tinyhumansai/tinyagents#144) CI caught a clippy finding my 1.96.1 could not see — |
1 participant
Summary
The MCP handshake returns
instructions— free-form guidance the server wants the client to have.McpInitializeResultalready parses it and both transports already cache it. Nothing read it, so it was discarded exactly where it is most useful.This carries it onto
ConnectedServerOverview.Problem
ConnectedServerOverviewis what a host renders a connected server from. For a hand-added server there is no registry entry, sodescriptionisNoneand the line a host can produce degrades to a name plus "N tools available" — which says nothing about what the server does.The server's own
initializeinstructions are the only thing that can answer that, and the protocol already delivers them.Change
ConnectedServerOverview::instructions: Option<String>,#[serde(default)]so an overview written before the field existed still decodes (asNone).Connectioncaptures the handshake's instructions at connect time;connected_overviewcopies them out.Three details worth stating, since each was a choice:
Captured once, not read per overview. The value cannot change without a reconnect, and a reconnect rebuilds the whole record anyway.
connected_overviewis on a prompt-rendering path, so making it await a transport per server would be the wrong trade.The read cannot dial again. It runs after
list_tools, which cannot have succeeded without a completed handshake, soinitialize()returns the cached result.A transport error yields
None, the same as a server that sent nothing. The caller is describing a server; a missing description is not worth failing the listing over.Trust
Documented on the field as untrusted remote text needing sanitization before it reaches an LLM's context — the same warning
descriptionalready carries, and the same onetransport/mod.rsgives for this value. This PR only carries it; it does not decide how a host renders it.Tests
678 + 150 pass. The round-trip test covers the new field, and a new test pins that a payload written before it existed still decodes rather than failing the whole overview and taking the server listing with it. fmt clean.
cargo clippyfails on this toolchain for an unrelated pre-existing reason —#[allow(clippy::unused_async_trait_impl)]attinymcp/src/tinybus_module/service.rs:122is an unknown lint here. Untouched by this PR (3 files, +55 lines).Related
Unblocks tinyhumansai/openhuman#5321, which wants to fall back to a server's instructions in the orchestrator prompt when the registry has no description for it. That change lives in openhuman; this is the one field it needs from here.