Skip to content

Describe the surface once, not in every field - #173

Merged
aterga merged 13 commits into
mainfrom
docs/describe-not-direct-metadata
Sep 1, 2026
Merged

Describe the surface once, not in every field#173
aterga merged 13 commits into
mainfrom
docs/describe-not-direct-metadata

Conversation

@sea-snake

@sea-snake sea-snake commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Breaking surface change. Three tools are removed (resolve_app, discover_app_canisters, icp_find_app_by_name) and one is added (candid_syntax_guide). open_app already did both halves of the first two and returns a strict superset of their output; the third was an unserved offline read of the same registry. Consumer docs are updated in this PR. An earlier revision of this description said "nothing was deleted" — that was true when written and is not now.

Summary

The model-readable metadata taught the same three concepts in every tool: what textual Candid is, what a derivation origin is, and how a session is authorized. Every description then re-derived them, qualified them, and restated what the schemas already said. A client paid ~11.5k tokens of prose before its first call.

This rewrites the metadata as one system rather than shortening it sentence by sentence. The server instructions now teach those three concepts once, under CANISTERS, APPS AND IDENTITY and SESSIONS, and all 26 descriptions are written assuming a reader who has them. Each states what the tool does, when to use it, which sibling owns the adjacent case, and the one prerequisite that gates success.

Nothing was deleted, only re-homed: argument semantics to the argument's own schema, the meaning of a returned value to the output schema, diagnosis and repair to the result text, and internal thresholds, metadata keys and legacy parameter aliases to source comments, which are never sent to a model.

Surface Before After
Tool descriptions (26) 19,823 7,629
Server instructions 4,262 1,982
Doc comments across the schema modules 104,769 94,944

canister_update_call, 1,259 chars to 382:

Change a canister's state by calling one of its update methods. Use this when the user asks for something a canister must record; read-only requests go through canister_query. Method names and argument types come from the canister's Candid interface, which get_canister_candid returns. Runs anonymously unless given a derivation origin, which acts as the user's account at that app.

For calibration: Anthropic's own worked example of a good tool description is 400 characters.

Accuracy defects fixed

Four review passes (compliance, code-truth, naive-reader, redundancy) ran over the result. Each finding below is a claim the code contradicts:

  • args typed-encoding was asserted unconditionally. encode_args falls back to to_bytes() whenever no interface is available, where a bare 42 needs its own : type annotation. The old wording made a model emit args the canister rejects.
  • "does not answer Candid data queries" was wrong twice over: this server refuses the call before the network, not the canister, and it refuses every method query on an OQL canister, not only data queries.
  • open_app asserted the no-IC-evidence refusal unconditionally. It fires only where the derivation origin had to be assumed; resolve_app kept that qualifier and open_app had lost it.
  • get_app_principal claimed "without calling any canister" while deriving a delegation against the Internet Identity canister.
  • The update guard was understated as "asset-moving calls". It also refuses neuron management and every update call to a listed financial service, so a refusal of store on a wallet frontend was unpredictable from the metadata.
  • "publishes an API doc" promised what a declaration check cannot establish.
  • read_only named a consequence that is not on the served surface, and read as proof of write access when the level is merely unknown.
  • The anonymous-principal rule was stated globally but does not hold for the OQL reads, which reject a call with no origin.
  • reserved_cycles_limit was the one argument with no unit while all five siblings had one, so its value had to be invented.
  • open_world_hint was false on the two skills tools, which fetch skills.internetcomputer.org. Only the bundled skill:// resources retrieve nothing.

The scan gap that let this accumulate

model_readable_metadata_respects_marketplace_policy scanned IcTools::all_tools() — the 11 served tools. The 13 deferred descriptions ship in the crate and were never scanned, which is how they kept ALWAYS call this FIRST, NEVER guess a domain and WEB SEARCH the app's official URL. The scan now covers every tool defined in the crate.

It immediately paid for itself: on its first run it caught a schema field announcing an instruction to web-search the app's URL, which had walked past the blocklist on the hyphen alone. web-search is now listed.

The bare token base64 comes off the blocklist. The banned shape is an instruction to decode, and icp_install_code legitimately accepts wasm_base64; keeping the word contraband would force that parameter to lie about its own format. The smuggling patterns (b64decode, rot13, decode and, decode the following) stay, and the gate's own sample is caught by decode and.

The schema-scan liveness sentinel now compares on collapsed whitespace. A doc comment carries the line breaks it was wrapped at, so the raw-text match failed on a rewrap rather than on a broken scan — a check that reported a problem it was not testing for.

Two narrowings in this change's own drafts (Use this for anything that...) were caught by the gate and fixed rather than worked around.

A second accuracy pass over all 26

Every description was then re-verified claim by claim against its handler, after review found that fixing the instance of a defect had left four other copies of it standing.

  • instead / data query, in five places. "declares an OQL surface instead of Candid queries" was wrong three ways: has_oql checks for two ordinary Candid methods (schema, execute), so nothing is replaced; update calls on the same canister still go through Candid, since oql_query_redirect is never applied to them; and the refusal is this connector's read-path rule, not a limit of the canister. The "data query" narrowing alongside it hid that every Candid method query is refused there, not only data queries.
  • open_app claimed names are matched against the registry and an unknown name is refused. classify_app_query matches a name or a bare host (so a wrong-TLD guess repairs to the canonical app), and an unknown dotted host is resolved as a URL rather than refused; only an unmatched bare word is.
  • icp_cycles_balance said "the user's principal". cycles_balance goes through management_agent, the identity derived at this connector's own origin, and the user has a different principal at every app, so that named nothing specific.
  • get_canister_api_doc asserted the content of a reply the canister controls.
  • icp_oql_guide read as though the guide were itself a read path.

"Requires an authenticated session" is gone from twelve descriptions

On the hosted server the whole MCP router sits behind require_token (src/lib.rs:305-308), so a tools/call cannot reach a handler without a validated bearer token and bearer_session_resolver always returns Some. The sentence was a constant, and a constant implies a distinction that does not exist. The real variable is anonymous vs acting as the user, which is the derivation origin, and that half of those sentences stays.

Where a session genuinely varies, the fact stays: the runtime error that names it, the SESSIONS paragraph, the local binary's SIGNING IN paragraph, and icp_canister_status' access level, which is a real variable rather than a constant.

Compliance

Checked against the Software Directory Policy, the pre-submission checklist, and the OpenAI Apps SDK and plugin guidelines. The checklist reduces to one line — "Describe what the tool does. Do not tell Claude how to behave" — and both directories still expect a description to say when a tool applies and what is unsafe to pass, so the line drawn here is scope, not imperative mood: Use this when X; Y goes through the sibling tool is the endorsed form.

Unchanged by design: the read/write split, the three test-pinned no-constructed-domain clauses (OpenAI explicitly requires not depending on the model guessing identifiers), the financial policy staying server-wide, and the 11-tool served surface.

Open questions for review

  1. Four runtime error strings still tell the model to WEB SEARCH the app's URL (unverified_app_url_error, app_url_error_with_guidance, discover_app_canisters' empty result, and open_app's unknown-name refusal), with a test pinning that wording. These are repair guidance after a wrong guess, so they are genuinely useful — but the policy's no-external-errands rule is written about the software, not only about descriptions. Left as-is pending a call.
  2. Should canister_update_call's description state the pre-flight refusal? Decided: no. The financial policy stays server-wide and the description is unchanged. Considered and rejected deliberately, so the trade-off is on the record: MCP's schema calls instructions a hint the client MAY use, and some clients drop it, so a client reading only tools/list does not learn the guard exists. financial_policy_is_a_server_instruction_not_a_description continues to pin the current arrangement.
  3. icp_start_canister / icp_stop_canister carry destructiveHint: false. Reversible run-state changes, so this matches OpenAI's definition ("irreversible or difficult to reverse"), but Anthropic's phrasing asks for one of the two hints to be set, and stopping a production canister is disruptive. Judgement call, currently pinned by a test.
  4. Encoding "exactly one of method / oql" and the required-in-practice derivation_origin as oneOf / required in the schemas would let the schema carry constraints prose currently carries. That is a behaviour change — derivation_origin is deliberately optional so omitting it yields a useful error rather than a schema failure — so it belongs in its own PR.
  5. The two skills tools live-fetch with a 15-minute cache and no fallback to the compiled bundle. If they are ever served, pointing them at BUNDLED_SKILLS would remove the network dependency and make openWorldHint: false correct again.

Test plan

  • cargo test --workspace green: 149 + 21 + 52 + 7 + 10 + 1 passing, 0 failed.
  • cargo clippy --workspace --all-targets: no new warnings (the 6 pre-existing map_or / redundant-closure suggestions are in untouched code).

🤖 Generated with Claude Code

Review

All 18 threads addressed and resolved. Twelve descriptions taken from review, most verbatim; four comments declined as implementation detail a client cannot act on (the name-based nature of the OQL and api-doc detection, and the manual .did fallback).

Two defects the review caught that were mine: the server instructions still said canisters "expose OQL instead" — the sixth copy of a claim fixed five times elsewhere — and omitted said the least authoritative "went first", which reads as a claim about list order and inverts it. Both fixed and the surface swept for siblings.

Two safeguards were removed deliberately, each because the thing it guarded against is gone. canister_update_call no longer restates the discoverability gate: a caller can neither check nor act on it before calling, app_url is documented on its own argument, and a refusal names the manifest path and links the guide, so it is stated once in the server instructions instead. open_app no longer carries the no-constructed-domain warning: that existed because resolve_app and discover_app_canisters accepted URLs only, so a caller holding a name had to invent one. Both tools are gone and open_app takes a name. The refusals themselves are untouched in both cases.

sea-snake and others added 2 commits August 31, 2026 17:15
The model-readable metadata taught the same three concepts in every tool:
what textual Candid is, what a derivation origin is, and how a session is
authorized. Each description then re-derived them, qualified them, and
restated what the schemas already said, so a client paid ~11.5k tokens
before its first call.

The server instructions now teach those three concepts once, and every
description is written assuming them. Each states what the tool does, when
to use it, which sibling owns the adjacent case, and the one prerequisite
that gates success — nothing else.

Facts were relocated rather than dropped: argument semantics to the
argument's own schema, the meaning of a returned value to the output
schema, diagnosis and repair to the result text, and internal thresholds,
metadata keys and legacy parameter aliases to source comments, which are
not sent to a model at all.

Two narrowings the policy gate caught in this change's own text
("use this for anything...") are fixed rather than worked around, and the
schema-scan liveness sentinel now compares on collapsed whitespace so a
rewrap cannot masquerade as a broken scan.

Metadata: 46,006 -> 23,655 chars (~11.5k -> ~5.9k tokens).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Accuracy defects, each traced to the code that contradicts the claim:

- `args` claimed unconditional typed encoding; `encode_args` falls back to
  `to_bytes()` whenever no interface is available, where a bare `42` needs its
  own annotation. A model trusting the old wording emits an unencodable arg.
- "does not answer Candid data queries" was wrong twice: this server refuses
  the call, not the canister, and it refuses every `method` query on an OQL
  canister, not only data queries.
- `open_app` asserted the no-IC-evidence refusal unconditionally; it fires only
  where the derivation origin had to be assumed. `resolve_app` kept the
  qualifier, `open_app` had dropped it.
- `get_app_principal` said "without calling any canister" while deriving a
  delegation against the Internet Identity canister.
- The instructions understated the update guard as "asset-moving calls": it also
  refuses neuron management and every update call to a listed financial service.
- "publishes an API doc" promised what a declaration check cannot establish.
- `read_only` named a consequence that is not on the served surface, and read
  as proof of write access when the level is merely unknown.
- The anonymous-principal rule was stated globally but does not hold for the
  OQL reads, which reject a call with no origin.

The policy scan now covers every tool DEFINED here, not the 11 served. That gap
is why a hyphenated `web-search` errand sat in a schema field and why the
deferred half kept its `ALWAYS call this FIRST` era wording; the widened scan
caught the former on its first run.

Two gate corrections it forced. `web-search` joins the blocklist, since the
hyphen alone walked past it. The bare token `base64` leaves it: the banned shape
is an instruction to decode, and `icp_install_code` legitimately takes
`wasm_base64`, so the word cannot be contraband without making that parameter
lie about its format.

`open_world_hint` is now true on the two skills tools, which fetch
skills.internetcomputer.org; only the bundled `skill://` resources retrieve
nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Consolidates model-facing metadata to reduce repetition while preserving tool semantics.

Changes:

  • Centralizes Candid, identity, session, and financial-policy guidance.
  • Shortens descriptions and schema documentation across 26 tools.
  • Expands marketplace-policy scanning to deferred tools.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/imcp2-local/src/server.rs Streamlines authentication metadata.
crates/imcp2-core/src/tools.rs Reworks tool descriptions, instructions, and policy tests.
crates/imcp2-core/src/management.rs Documents the reserved-cycles unit.
crates/imcp2-core/src/identities.rs Condenses identity schema documentation.
crates/imcp2-core/src/discover.rs Condenses discovery schema documentation.
crates/imcp2-core/src/calls.rs Refines call argument and output documentation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/calls.rs Outdated
Comment thread crates/imcp2-core/src/calls.rs
Comment thread crates/imcp2-core/src/discover.rs Outdated
sea-snake and others added 6 commits August 31, 2026 17:39
… refusing

"declares an OQL surface instead of Candid queries" was wrong three ways.
`has_oql` checks that the interface declares two ordinary Candid methods,
`schema` and `execute`, so nothing is replaced; update calls on the same
canister still go through Candid, since `oql_query_redirect` is never applied to
them; and the refusal is this connector's read-path rule, not a limit of the
canister.

The same wording, and the same "data query" narrowing that hid the fact that
EVERY Candid `method` query is refused on such a canister, appeared in five
places. All five now say what the flag means and who acts on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Verified every description against its handler. Four said something the code
does not do:

- open_app claimed names are matched against the registry and an unknown name
  is refused. `classify_app_query` matches a name OR a bare host, so a
  wrong-TLD guess repairs to the canonical app, and an unknown DOTTED host is
  resolved as a URL rather than refused; only an unmatched bare word is.
- icp_cycles_balance said "the user's principal". `cycles_balance` goes through
  `management_agent`, the identity derived at this connector's own origin, and
  the user has a different principal at every app, so that named nothing
  specific.
- get_canister_api_doc asserted the content of a reply the canister controls; a
  canister can declare the method and return anything.
- icp_oql_guide read as though the guide were itself a read path.

"Requires an authenticated session" is gone from twelve descriptions. On the
hosted server the whole MCP router sits behind `require_token`, so a tool call
cannot reach a handler without a validated session and the sentence is a
constant. Where a session genuinely varies the fact stays: the runtime error
that names it, the SESSIONS paragraph, the local binary's SIGNING IN paragraph,
and icp_canister_status' access level, which is a real variable rather than a
constant. The derivation-origin half of those sentences also stays, since that
is the caller's choice.

Descriptions: 19,823 -> 7,629 chars across all 26.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drove the surface with an agent that had only the server's own metadata, over
five requests. Three defects it hit are fixed here; a fourth it reported was an
artefact of the harness (output schemas were not shown to it) and is not real.

- With a URL in hand, open_app, resolve_app and discover_app_canisters all
  applied and nothing said which to prefer; the client called it the weakest
  separation on the surface and picked the narrower tool on a hunch. Each of the
  two narrow tools now says what it alone gives you.
- The session's access level is reported by get_app_principal, but no
  description said so, so a client picking tools by description could not find
  it and concluded the level was unreachable.

It also refused the token-transfer request correctly, and correctly predicted
that a benign rename on an NNS canister would be refused by the canister-scoped
guard — both read straight off the instructions, which is the accuracy fix in
the previous commit doing its job.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every `candid://`, `oql://` and `skill://` mention in model-readable text sat in
the server instructions, which the MCP schema calls a hint the client MAY use.
A client that drops that field got one inline example of textual Candid and no
route to the syntax reference this server already serves, on the two tools whose
arguments are nothing but textual Candid.

The `oql` argument already pointed at icp_oql_guide, so that dialect survived
without the instructions; the Candid syntax did not. Both `args` docs now name
`candid://textual-syntax`, which is read at the moment the value is written.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scanned the built tools/list JSON, not the source, for leak vocabulary and
fixed every hit in model-readable text. What a caller reads in a reply stays;
how this server obtained it goes.

- discover_app_canisters described its own scraping: "the frontend canister seen
  at the gateway, or an id mined from the site's runtime config or scripts". A
  caller needs the trust level, not the technique.
- `sources` keeps its literal values, since a caller reads them and must know
  how far to trust each, but loses the gloss explaining what each artifact is.
- `label` no longer enumerates where a label came from.
- "canonicalization" is gone from four field docs; the decision it existed for,
  comparing `requested` against `derived_for_origin`, stays.
- "Null when it was not probed" becomes "Null when unknown": what matters is
  that null is not false.
- "the output caps dropped" and two always-constant clauses ("Always false
  here", "then always true") are reworded to state the rule rather than the
  constant or the internal bound.
- The api-doc method names and the OQL `schema`/`execute` pair are dropped from
  output docs; nothing calls them through this surface.

Wire text: 34,103 -> 31,627 chars.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ource

Driving the surface with a client that had no server instructions turned up a
hole this change had itself created: both `args` docs pointed at
`candid://textual-syntax`, and no tool on the surface fetches a `candid://` URI.
Resources are read through a client capability, and a client that drops the
instructions field is unlikely to surface resources to a model either, so the
one thing a caller cannot derive — the value syntax every canister call is
written in — was delegated to a document it had no way to reach.

The server had already solved this for OQL: `icp_oql_guide` is a TOOL, which is
why the OQL dialect survived the same test. `candid_syntax_guide` mirrors it,
returning the same 4.6 KB cheat sheet the `candid://textual-syntax` resource
serves. The 25 KB full type reference stays a resource and is mentioned as
supplementary rather than inlined.

Served surface is 12 tools; the count contracts in three tests move with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sea-snake
sea-snake marked this pull request as ready for review September 1, 2026 08:50
@sea-snake
sea-snake requested review from a team and Copilot September 1, 2026 08:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Comment thread crates/imcp2-core/src/tools.rs Outdated
#166 restricts update calls to canisters an app declares, which changed
behaviour this branch describes, so the conflicts were reconciled on the facts
rather than textually.

- canister_update_call gains `app_url` and the manifest gate. Its description
  and the `app_url` schema now carry both, in this branch's shorter form but
  using the exact phrases #166's own test pins.
- The instructions gain a WRITES section: reading any canister is open, a write
  reaches only a canister its owning app declares, and a refusal does not close
  the app to reads.
- `sources` values changed on main: "ai-connect.html" is gone and
  "ic-architecture" replaces it, with only that one authorizing a write. This
  branch's trust framing would have named a source that no longer exists.
- open_app now says the app URL it returns is what canister_update_call takes.

Two things the merge surfaced, unrelated to the conflict:

`candid://reference` was named as though a model could act on it. It is an MCP
resource, reachable only through a client that surfaces resources, so both
mentions now say so rather than dangling a URI.

candid_syntax_guide described its own content as "the `(...)` form and the
literal for each type", which says nothing. It now names what the document
holds: the literal form per Candid type, and when a value needs an explicit
`: type` annotation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 1, 2026 08:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/tools.rs Outdated
open_app already does both halves in one call, and its output is a strict
superset: OpenAppOutput carries everything ResolveAppOutput did, plus everything
DiscoverOutput did apart from the echoed `domain`. Three tools that all took a
name-or-domain and all returned pieces of the same answer were the weakest
separation on the surface — a client driven against it said so unprompted, and
picked between them on a hunch.

One behaviour goes with them. discover_app_canisters answered "is this domain on
the Internet Computer?" with an empty list, where open_app refuses a URL whose
origin would have to be assumed and shows no Internet-Computer evidence. The
question is still answerable, but as a refusal carrying the reason rather than
an empty result.

Everything that pointed at the two is repointed at open_app: five argument
schemas, the anonymous-read remediation, the OQL data-access note, the
known-app and unknown-name notes, and the discovery-failure retry. Their
argument and output types are deleted rather than left orphaned, and open_app's
trigger clause is widened now that it is the only entry point.

Served surface: 12 tools -> 10. Wire text 31,627 -> 28,245 chars.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 1, 2026 10:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Suppressed comments (1)

crates/imcp2-core/src/calls.rs:136

  • expected: true does not mean the interface declared no documentation method. api_doc_method also returns None when the fetched interface cannot be parsed or exceeds the Candid guard, and the handler sets expected: true in that case (tools.rs:393-405). This schema currently tells clients that absence was established and retry cannot help when detection actually failed; retain that ambiguity in the field documentation.
    /// When `available` is false: true if the interface was read and declared no doc method,
    /// which is the normal case and will not change on a retry.

Comment thread crates/imcp2-core/src/tools.rs
Comment thread crates/imcp2-core/src/tools.rs Outdated
Comment thread crates/imcp2-core/src/discover.rs Outdated
Comment thread crates/imcp2-core/src/discover.rs
sea-snake and others added 2 commits September 1, 2026 13:49
Descriptions taken from review, mostly verbatim:

- canister_update_call loses the discoverability gate. It is a precondition the
  caller can neither check nor act on before calling: `app_url` is documented on
  its own argument, and a refusal names the manifest path and links the guide.
  The three description assertions in #166's gate test go with it; its
  instructions assertions and the app_url schema check stay, so the gate is
  still stated once, server-wide.
- open_app loses the no-constructed-domain warning. That safeguard existed
  because resolve_app and discover_app_canisters accepted URLs only, so a caller
  holding a name had to invent one. Both are gone; open_app takes a name, and
  its first sentence says so. open_app_metadata_forbids_a_constructed_domain is
  removed with the warning, and the `app` schema trimmed to match. The refusal
  itself is untouched.
- get_app_principal, list_app_accounts, get_canister_oql_schema and
  get_canister_api_doc take the reviewer's wording. get_app_principal keeps one
  clause on the query-only session, the only pointer to where the access level
  is reported.
- canister_query takes the suggestion minus one sentence whose two halves were
  each restated by the two that follow it.
- Ordering imperatives ("Use this before ...") are gone from four descriptions;
  "Use this when ..." triggers stay, as the review's own suggestions keep them.

icp_find_app_by_name is deleted. It was a pure offline read of the same
four-app registry open_app already consults, returning a strict subset of what
open_app returns, and it was not served. The find_app_by_name function stays:
open_app builds its unknown-name refusal from it.

Two accuracy fixes:

- The instructions still said canisters "expose OQL instead" — the sixth copy of
  a claim fixed five times elsewhere. They now say a canister declares OQL in
  addition, that this server refuses a Candid `method` query on it, and that
  update calls are unaffected.
- `omitted` said the least authoritative "went first", which reads as a claim
  about list order and inverts it. It now says the list is ordered most
  authoritative first and drops come off the other end.

Declined: four review comments asking for the name-based nature of the OQL and
api-doc detection to be restored. It is an implementation detail no client can
act on, and the actionable consequence is already stated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three tools were removed and one added, so the prose describing them was stale
in twenty places. Removing a tool and leaving its table row is a broken PR, not
a metadata change.

- README: the `resolve_app`, `discover_app_canisters` and (never present)
  `icp_find_app_by_name` rows are gone, `candid_syntax_guide` gains one, and the
  "narrower tools remain" escape hatch in Typical flow goes with the tools it
  pointed at. Fifteen inline `open_app`/`resolve_app` pairs collapse to the tool
  that remains.
- docs/scoping-local-deployment.md: the served-tool list is corrected.
- docs/openai-directory-submission.md: two claims that cited the removed tools
  now cite open_app, which is what enforces them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 1, 2026 11:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Suppressed comments (4)

crates/imcp2-core/src/tools.rs:252

  • This tool returns static/oql-primer.md, which still tells callers to obtain the origin from open_app / resolve_app. Since this PR removes resolve_app from the router, the newly served guide directs clients to a nonexistent tool. Update the bundled primer alongside the tool removal.
        description = "Return the guide to OQL, the JSON query language behind canister_query's `oql` argument: the query object's filters, aggregation, ordering, edge traversal and paging. get_canister_oql_schema supplies one canister's entity and field names.",

crates/imcp2-core/src/calls.rs:136

  • expected=true does not mean the interface declared no doc method: api_doc_method also returns None when fetched interface text cannot be parsed or exceeds its guard. The handler deliberately reports that case as expected, so this schema currently overstates absence and can cause clients to stop investigating a real method.
    /// When `available` is false: true if the interface was read and declared no doc method,
    /// which is the normal case and will not change on a retry.

crates/imcp2-core/src/calls.rs:242

  • A derivation origin may be identical to the app's website origin. This absolute wording can make callers avoid the correct value; describe it as “not necessarily” the website URL instead.
    /// The app's derivation origin, from open_app, to read as the user's
    /// account there. Not the app's website URL. Required for an OQL query; omit on a Candid
    /// query to read anonymously.

crates/imcp2-core/src/tools.rs:2341

  • The count change confirms that this PR removes two served tools (resolve_app and discover_app_canisters) while adding one, reducing the public surface from 11 to 10. That is a breaking API change, but the PR is presented as a metadata-only rewrite, says all 26 descriptions remain, and says nothing was deleted. Either retain compatibility or explicitly scope and version this removal so existing clients are not silently broken.
        assert_eq!(served.len(), 10, "expected 10 served tools, got {}", served.len());

Comment thread crates/imcp2-core/src/tools.rs
Comment thread crates/imcp2-core/src/discover.rs
Comment thread crates/imcp2-core/src/calls.rs Outdated
Comment thread README.md Outdated
Comment thread crates/imcp2-core/src/calls.rs
Comment thread crates/imcp2-core/src/tools.rs
Both found by review on the merged branch, both mine, both the same pattern:
an instance fixed and its siblings missed.

- The `derivation_origin` argument said "Not the app's website URL" on both
  tools that take one. It is not NECESSARILY the website URL: most apps pin no
  custom origin, which is why `app_url_default` exists as a source and why
  resolve_app's own text says the origin is "assumed to equal the app's own
  origin". As written it invited a caller to discard the correct value that
  open_app had just handed it. One word restored.
- README still promised get_app_principal returns "without a call", the claim
  removed from the tool description several commits ago once it was traced to
  `derive_app_delegation`, which makes an update and a query against the
  Internet Identity canister. Swept the README for the other corrections made
  in code; nothing else stale.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 1, 2026 12:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Suppressed comments (7)

Previously missed (2) — in code that hasn't changed since the last review.

docs/openai-directory-submission.md:61

  • This compliance row now claims the open_app description and input schema require an unchanged name or an officially sourced URL, but this PR deliberately removes both clauses and deletes open_app_metadata_forbids_a_constructed_domain. Row 60 also still names that deleted test. Update the checklist to describe the remaining runtime refusal rather than asserting metadata safeguards that no longer exist.
| Required identifiers must not depend on the model guessing them | ✅ `open_app` refuses an unknown bare name, and refuse a URL that would need its own origin assumed as the derivation origin when that origin shows no Internet Computer evidence, rather than resolving a guess (an app that declares its derivation origin is taken at its declaration, so that path is not gated by the evidence probe — but a CROSS-origin declaration is accepted only when the declared origin authorizes this app in its `/.well-known/ii-alternative-origins`, and an unauthorized one is refused outright rather than falling back, so a declaration is not a way around the identity checks); the description and the `app` schema both say to pass the user's name unchanged and to pass only a user-supplied or officially sourced URL. The IC-evidence check is stated for what it is — evidence that a domain is served from the Internet Computer, not that it is the intended app |

crates/imcp2-core/src/tools.rs:1962

  • The centralized model-facing instructions contain the grammatical error “open_app resolve it.” Change this to “open_app resolves it” so the core identity rule reads unambiguously.
const SERVER_INSTRUCTIONS: &str = "Tools for the Internet Computer: read what a canister offers, read and write its data, and act as the user's own identity at an app.\n\nCANISTERS. Each canister publishes a Candid interface declaring its methods and their types. Method arguments and replies are textual Candid, the `(...)` syntax, e.g. `(record { owner = principal \"aaaaa-aa\"; amount = 5 : nat })`; candid_syntax_guide returns that syntax; the full type system is served as an MCP resource, `candid://reference`. Some canisters additionally declare an OQL surface. This server reads those through OQL and refuses a Candid `method` query on them; update calls are unaffected. icp_oql_guide documents the dialect. IC how-to guides are served as `skill://<name>` resources.\n\nAPPS AND IDENTITY. An app is a website backed by canisters. Internet Identity gives the user a different principal at each app, derived from that app's derivation origin, which is not always the app's visible URL; open_app resolve it, and the tools that act as the user take the resolved value. Within one app the user may hold several accounts.\n\nWRITES. State-changing calls reach only apps that publish a service-discoverability manifest: canister_update_call is made to a canister only when the app that owns it declares that canister at /.well-known/ic-architecture (https://docs.internetcomputer.org/guides/frontends/service-discoverability/), which is how its operators opt in. `app_url` names that app and open_app returns it; where a call carries both an `app_url` and a derivation origin, the two must belong to the same app. Reading is not gated that way: every read tool works on any canister, declared or not.\n\nSESSIONS. Calls that act as the user are signed with the Internet Identity credential this connection was authorized with. Internet Identity offers two access levels at that point: \"Questions only\", where reads work and state-changing calls are rejected by the network, and \"Actions & questions\", which permits both.\n\nFINANCIAL TRANSACTIONS ARE NOT SUPPORTED, to protect the user: do not use canister_update_call to move assets. Value-moving methods, neuron management, and every update call to a known wallet, ledger, exchange or staking canister are refused before they reach the network; that guard is a safeguard, not a complete filter, so treat this policy, rather than the absence of a refusal, as the limit. For financial operations (token transfers, spending approvals, payments, trades), the user works outside this connector, in a trusted interface they control.\n\nBuilding and deploying canisters happens in the user's own environment with the icp CLI; no tool here creates, funds or deploys one.";

crates/imcp2-core/src/tools.rs:247

  • The new tool returns CANDID_TEXTUAL_MD, whose opening says that every tool takes and returns textual Candid. That is false for open_app, the identity tools, and the OQL path, and contradicts the newly centralized instructions that scope textual Candid to method arguments/replies. Update the returned document to make the same scope explicit before exposing it as a tool result.
        let output = calls::CandidGuideOutput { content: CANDID_TEXTUAL_MD.to_string() };

crates/imcp2-core/src/tools.rs:252

  • The OQL guide returned here still tells callers to obtain the origin from open_app / resolve_app (static/oql-primer.md:26), but this PR removes resolve_app. Update the bundled guide so invoking this tool cannot direct clients to a nonexistent route.
        description = "Return the guide to OQL, the JSON query language behind canister_query's `oql` argument: the query object's filters, aggregation, ordering, edge traversal and paging. get_canister_oql_schema supplies one canister's entity and field names.",

crates/imcp2-core/src/tools.rs:2658

  • This newly added comment still says there are 11 served tools, while the assertion above now establishes 10. Keeping the scanner commentary synchronized avoids obscuring which surfaces it actually covers.
        // Every tool DEFINED here, not just the 11 served: a deferred description

crates/imcp2-core/src/discover.rs:2368

  • Remove the stray space before the period.
    /// The canisters discovered behind the app, most authoritative first . Empty when the app declares

crates/imcp2-core/src/tools.rs:2341

  • These assertions establish 10 served and 22 core tools; including the two local login tools gives 24 descriptions, not the PR body's 26. Consumer docs also remain on the removed 11/24-tool surface (docs/scoping-local-deployment.md:109-111, docs/openai-directory-submission.md:57,60, and docs/anthropic-directory-submission.md:54-55,202), with the Anthropic doc still naming the deleted safeguard test. Update the surface inventory everywhere as part of this breaking change.
        assert_eq!(served.len(), 10, "expected 10 served tools, got {}", served.len());

Comment thread crates/imcp2-core/src/tools.rs
@aterga
aterga merged commit 8c742c5 into main Sep 1, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants