Serve only the app/canister tools - #158
Conversation
There was a problem hiding this comment.
Pull request overview
Defers the 16 protocol/meta tools while retaining them for library embedders, reducing the default served surface to 10 canister/app tools.
Changes:
- Routes listing and calls exclusively through
IcCanisterTools. - Keeps skills and OQL guides available as MCP resources.
- Updates tests, guidance, landing content, and submission documentation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/assets/index.html |
Updates landing-page capability groups. |
README.md |
Documents the reduced surface and deferred tools. |
docs/openai-directory-submission.md |
Revises compliance and test guidance. |
docs/anthropic-directory-submission.md |
Revises directory submission guidance. |
crates/imcp2-local/src/server.rs |
Updates merged-surface test expectations. |
crates/imcp2-core/src/tools.rs |
Implements the reduced default composition. |
crates/imcp2-core/src/compliance.rs |
Redirects creation refusals to CLI/resources. |
crates/imcp2-core/src/calls.rs |
Redirects OQL guidance to its resource. |
Suppressed comments (1)
docs/anthropic-directory-submission.md:325
- This “nothing to provision” instruction contradicts lines 203-207, which still tell maintainers to provision a controller identity and cycles balance specifically for the canister-management tools. Remove that obsolete fallback so reviewers are not offered credentials for tools they cannot call.
> 4. Canister-management tools are not part of this version (we anticipate
> they will come in a future version), so there is nothing to provision:
> creating and managing canisters happens outside the connector, with the
> icp CLI.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The default IcTools composition now serves only IcCanisterTools — the 10 app/canister tools. The icp_-prefixed protocol/meta group (dashboard lookups, the skills tools, the OQL guide tool, and canister creation/management) is deferred: every reference in the served text, the README, the landing page, and the submission docs now says we anticipate this will come in a future version. The type, its 16 tools, and their tests stay in the library — IcProtocolTools::new and its pub tool_router let an embedder serve the group today, and re-enabling it in IcTools is one line. What still works without the tools: the OQL dialect guide and the official IC skills remain served as MCP resources (oql://usage, skill://<name>), and served texts point there; name-guess guidance routes through open_app's known-app resolution; the compliance refusal for cycles-ledger creation spends points directly at the user-run icp CLI (install pointer included). The financial posture gets simpler still: no funding or management tools are served at all, and the canister_update_call guard stays. Tests: the served surface is pinned at 10 tools with no icp_ prefix (instructions say the deferred group is coming); the deferred router is pinned at 16 with all contracts (annotations, output schemas, instructions-only descriptions) still guarded; imcp2-local's live MCP round-trip now sees the 12-tool merged surface. Both routers are generated pub (vis = "pub") so the deferred half stays live code for the library. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
Per review-bot findings: the crate-level doc no longer claims IcTools composes both routers; the deferral test asserts a deferred name is not routable through the served router, and imcp2-local's live round-trip now calls icp_get_skill and asserts the method-not-found error (hiding calls, not just listings, is the contract); the submission docs' checklist rows distinguish the 10 served tools from the 16 deferred definitions; reviewer prompts and test cases use only served tools (controllers/status/skill/delete cases replaced); the questions-only notes name canister_update_call; the reviewer-provisioning fallback and the lingering instructions-only phrasing now describe the no-funding-tools-served posture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
8b5a4c2 to
68441a1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Suppressed comments (1)
crates/imcp2-core/src/tools.rs:87
- This promises a one-line re-enable, but the protocol instance is no longer retained by
IcTools; restoring it requires changing the struct/constructor plusall_tools,call_tool, andget_tool. Either retain a dormant protocol field/composition seam that makes the claim true, or document that the router must be composed back into those paths.
/// re-enabling it is one line in the composition here.
Per the author: the OQL dialect guide is part of the canister read flow (guide -> schema -> query), so it moves from IcProtocolTools to IcCanisterTools and stays on the served surface — the one icp_-named tool that does, called out as the explicit exception in the taxonomy and deferral tests. Served surface: 11 tools; deferred: 15. All the guide references that had been rerouted to the oql://usage resource point back at the tool (the resource remains as before), and the counts in the README, landing page, submission docs, and imcp2-local round-trip are updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
Three strings still reachable from the served surface named deferred tools (review findings on #158): - resolve_app's app_url input-schema doc said the URL may come from icp_find_app_by_name; it now points at open_app / web search. - The unknown-skill error behind skill:// resource reads said to call icp_list_skills; it now says to list the skill:// resources. - get_app_principal's Questions-only note listed the management verbs and icp_canister_status; it now names canister_update_call, matching the server instructions. READ_ONLY_MSG in identities.rs keeps its management wording: it is only reachable through the deferred management tools (require_write call sites), where that wording stays accurate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
The IcTools doc claimed re-enabling IcProtocolTools "is one line in the composition", but IcTools no longer retains a protocol instance: wiring it back touches the struct, constructor, all_tools, call_tool, and get_tool. Say so (and note the direct-serving alternative) instead of overpromising. Doc-only; flagged by review on #158. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
crates/imcp2-core/src/tools.rs:90
IcProtocolToolscannot be served “directly”: unlikeIcTools, it does not implementServerHandler; this attribute only exposes a router that an embedder must dispatch from its own handler. The public API documentation should describe wrapping/composing its router rather than promising direct serving.
/// `call_tool`, and `get_tool` here (or serving [`IcProtocolTools`] directly,
/// which stays fully usable as library code).
The imcp2-local deferral regression only asserted is_err(), so a disconnect or an unrelated error would have passed it (review finding on #158). It now expects the exact error the served router emits on a miss — code -32602 (invalid params) with "tool not found" — which is rmcp's shape for ANY unknown tool name, so deferred tools stay indistinguishable from nonexistent ones. The comments (and the IcTools dispatch note) called this "method-not-found"; corrected, since the router never emits -32601 for a miss. Also stop claiming IcProtocolTools can be served "directly": it has no ServerHandler impl, so an embedder dispatches its router from their own handler. The IcTools doc now says that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Suppressed comments (2)
docs/anthropic-directory-submission.md:328
- The submission-day checklist still says to “provision a funded identity” if requested (line 360), contradicting both this instruction that nothing management-related needs provisioning and the new fallback at lines 205-209 (an account at a demo app). Update the checklist to request a demo-app account instead of funding.
> 4. Canister-management tools are not part of this version (we anticipate
> they will come in a future version), so there is nothing to provision:
> creating and managing canisters happens outside the connector, with the
> icp CLI.
crates/imcp2-core/src/tools.rs:2601
- This comment says the composed canister+protocol list is what
tools/listserves, but the implementation and assertion below serve only the canister router. Rewrite it to say the two routers remain non-overlapping and retain all definitions.
// IcCanisterTools — no overlap, nothing dropped, and the composed list is
// what tools/list serves. icp_oql_guide is the one icp_-named exception
Review round on #158 found deferred-group references still written as served behavior in surfaces the earlier sweeps missed: - README's typical-flow tail: managing your own canisters is now described as deferred (future version), with the icp CLI as the current path. - Privacy policy (served asset + draft): the own-origin stable identity exists only for the canister-management tools, which are not part of this version — both copies now say so instead of describing the derivation as live; the identifier-linkage section no longer claims a stable identity is held. - scoping-local-deployment.md: the session-free local tool list drops the skills/lookup tools (deferred; skill documents remain skill:// resources) and gains icp_oql_guide; the security model separates what the served surface wields (per-app update calls) from what the deferred management tools would add if an embedder serves them. - OpenAI submission fallback provisioning matches the Anthropic doc: demo-app account, no controlled canister / cycles balance (the tools that needed them are deferred). - Anthropic checklist row: provision a demo-app account, not funding. - Taxonomy test header no longer claims the composed list is what tools/list serves (the canister half alone is, in this version). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
docs/anthropic-directory-submission.md:50
- This checklist overstates what the unit test enforces:
every_tool_has_correct_read_write_annotationschecks annotation presence plusreadOnlyHint/destructiveHint, but nottitle,idempotentHint, oropenWorldHint. The source link also targets nonexistentsrc/tools.rs; the file is undercrates/imcp2-core. Make the verification scope and link accurate so the directory checklist does not imply untested guarantees.
| Every tool: `title` + `readOnlyHint`/`destructiveHint` (+ `idempotentHint`, `openWorldHint`) | ✅ on all 11 tools, enforced by a unit test ([`src/tools.rs`](../src/tools.rs)) |
crates/imcp2-local/src/server.rs:415
- This 13-tool contract is not reflected in the local binary's own user-facing documentation.
crates/imcp2-local/README.md:6-9,84,96-98andcrates/imcp2-local/src/main.rs:25-29,61-63still advertise canister management andIMCP2_MANAGEMENT_ORIGINas active behavior. The variable now only suppliesIdentities::public_url, which is used bymanagement_identity; no route served byIcToolscalls it. Update those local docs/help strings to mark management as deferred (and the setting as reserved), otherwise users receive an incorrect capability and security model.
// surface (the 11 served core tools + the 2 login tools); tools/call dispatches
// login tools to the wrapper and everything else to IcTools; the login
// lifecycle (signed out → link → pending) runs through the MCP layer —
crates/imcp2-core/src/tools.rs:946
- The runtime guidance is updated here, but the served
get_app_principaloutput schema still describesPrincipalOutput::read_onlyas “canister management is unavailable” (crates/imcp2-core/src/identities.rs:393-394). MCP clients inspecting this schema are therefore still directed toward the deferred surface. Update that field description to say that state-changing calls such ascanister_update_callrequire “Actions & questions.”
but state-changing calls (canister_update_call) are rejected by the network. Ask the \
user to reconnect and choose \"Actions & questions\" on Internet Identity's consent \
screen.)",
Per maintainer review on #158, extended to every user-facing surface: mentioning tooling outside the current scope invites questions about code that isn't deployed. The README (intro, tool-table blockquote, typical-flow tail, name-lookup guidance, skills section, creating & managing canisters, read-only sessions), the local-deployment scoping doc, and the landing page's tool groups now describe today's product only — the deferred-tools blockquote and the "Deferred to a future version" landing group are removed, and no "future version" / "anticipate" phrasing remains in any of them. The library facts stay: IcProtocolTools remains documented in the rustdoc and the scoping doc's architecture note as code an embedder can compose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
Completes the current-scope pass from maintainer review on #158: - README: the "Creating & managing canisters" section is removed (the maintainer asked for it omitted for now); the typical-flow tail points at the icp CLI directly instead of that section. - SERVER_INSTRUCTIONS: the three parentheticals about tools that are not part of this version are gone. The surviving text keeps the operative advice with no forward-looking framing — for a token/service name or an unknown canister id, web search or ask the user; creating, funding, and managing canisters is the user's own icp CLI work. - The deferral test now asserts the instructions do NOT promise a future version, pinning the new contract instead of the old one. No "future version" / "anticipate" phrasing remains in any user- or agent-facing surface. The rustdoc on IcProtocolTools keeps describing the type for embedders, which is library API documentation, not product messaging. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
docs/anthropic-directory-submission.md:50
- This row overstates what the test and definitions guarantee. Of the served tools, only
canister_update_calldeclaresidempotentHint, while the test asserts only annotation presence plus read-only/destructive values; it does not assert title, idempotence, or open-world values. The source link also resolves to nonexistentsrc/tools.rsrather than the core crate file. Make the checklist precise so submitters know which fields still require portal verification.
| Every tool: `title` + `readOnlyHint`/`destructiveHint` (+ `idempotentHint`, `openWorldHint`) | ✅ on all 11 tools, enforced by a unit test ([`src/tools.rs`](../src/tools.rs)) |
README.md:19
- This categorical statement is inaccurate for the README's library audience: the crate still publicly exports
IcProtocolTools, including its funding-guidance and canister-management tools, for embedders to serve. It also omits the advertised “deferred to a future version” distinction. Qualify this as the default connector's current surface and document the retained library API.
no funding or management tools at all: creating, funding, and topping up
canisters is done by the user with the
[`icp` CLI](https://github.com/dfinity/icp-cli) in their own terminal.
Review on #158 caught real collateral from the scope trim: dropping the management-specific sentence from the privacy policy also dropped the cross-session linkage disclosure, which still applies to the per-app identities the current surface derives. Internet Identity derives the same identity for a given application and account, so software holding one could tell that two sessions at that application belong to the same user. Both policy copies now disclose that in terms of today's surface, followed by the three limits that were always there. Also: the annotations row linked ../src/tools.rs, which does not exist (src/auth.rs and src/lib.rs do — tools.rs is in the core crate); the link now points at crates/imcp2-core/src/tools.rs. And the README intro says the connector serves no funding or management tools, scoping the claim to the served surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
docs/anthropic-directory-submission.md:50
- This row overstates what the linked unit test enforces.
every_tool_has_correct_read_write_annotationschecks annotation presence plusreadOnlyHint/destructiveHint, but it does not assert titles,idempotentHint, oropenWorldHint(the OpenAI checklist correctly calls out the latter). Make the enforcement scope explicit so directory preparation does not rely on checks that do not exist.
| Every tool: `title` + `readOnlyHint`/`destructiveHint` (+ `idempotentHint`, `openWorldHint`) | ✅ on all 11 tools, enforced by a unit test ([`crates/imcp2-core/src/tools.rs`](../crates/imcp2-core/src/tools.rs)) |
crates/imcp2-local/src/server.rs:444
- The local binary now exposes 13 tools and no management router, but its public README still says it includes canister management and transfers (
crates/imcp2-local/README.md:6-9,96-98), while the binary docs/help still advertiseIMCP2_MANAGEMENT_ORIGIN(crates/imcp2-local/src/main.rs:25-29,61-63). Users following those surfaces will expect unavailable tools and configure an option that has no effect on the served surface. Update those local-facing descriptions alongside this count change.
13,
"11 served core tools + authenticate + auth_status"
* Refresh submission docs against verified production state A post-merge audit checked both directory docs against the code at main (0a32373) and against live production. Fixes, all doc-only: - Production gap restated with today's facts: /version reports bbf0844 (v0.1.1), which predates the whole compliance chain (#153-#158) and still serves the old 26-tool surface — a release from current main must be deployed before either submission, since the attestations describe main. - The privacy policy, /support, and /terms are LIVE on production (verified HTTP 200), so the "pending the production release" rows and checklist items now say what actually remains: portal entry, plus the next release refreshing the live text to the current draft. - The mcp-review checklist row contradicted blocker 2's resolved-in-code body and wrongly claimed the first-party-API question was asked on 2026-07-31; it now matches the body. - The listing description claimed "identify what a canister is" — a capability of the unserved dashboard-lookup tool; it now leads with the served Candid fetch. - Stale figures and paths: longest served tool name is 23 chars (get_canister_oql_schema), not 30; the dfinity-logo.svg link and the README's connect.css / static/ paths now point into crates/imcp2-core/ where the files live; "ships with this PR" phrasing updated now that the PR is merged and deployed; the stale cycles-funding justification sentence is dropped (no funding tools exist). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk * Distinguish main from the live build; gate OpenAI submission on /version Review findings on #161: the tool-name-length row now gives both values (23 on main, the build to submit; 30 on the live pre-release build), and the OpenAI checklist gains an explicit item requiring production to run a release cut from current main, verified via /version immediately before submitting — the deploy workflow also accepts older tags/SHAs for rollbacks, so a deployed challenge token alone does not prove the compliant build is live. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk * Scope the readiness table's verification claim Review finding on #161: the table header claimed live verification for every row, but the tool-surface rows (11 tools, one write, name lengths, schemas) describe main, not the pre-release build production currently runs. The header now says which rows were verified live (transport/auth, 2026-07-31) and that the tool-surface rows describe main pending the release, with a pointer to re-verify after deploying. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk --------- Co-authored-by: Claude <noreply@anthropic.com>
Per the review rule applied across #158/#161 (describe the current version, not future work): the README's Roadmap section — three unchecked forward-looking items plus a checked history that the feature sections already document — is removed, along with the stale status sentence claiming the live round-trip waits on an II deployment (the II-side connect flow is live). The Anthropic submission doc's parenthetical citing "an open roadmap item in the README" goes with it; its present-tense honesty statement (no per-call confirmation server-side today) stays. Claude-Session: https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk Co-authored-by: Claude <noreply@anthropic.com>
Reduces the served tool surface to the app/canister tools, and drops every mention of tooling outside the current scope from the docs, landing page, policy, and server instructions (maintainer review: describe what we ship today, nothing else). Builds on the
IcCanisterTools/IcProtocolToolssplit from #157 (now merged — this PR targetsmain).What the server serves
IcToolscomposes onlyIcCanisterTools: 11 tools — the app/canister-scoped tools (get_canister_candid,get_canister_oql_schema,get_canister_api_doc,canister_query,canister_update_call,get_app_principal,list_app_accounts,open_app,resolve_app,discover_app_canisters) plusicp_oql_guide, the oneicp_-named tool on the canister side: it is the dialect guide the canister OQL read flow runs on (icp_oql_guide→get_canister_oql_schema→canister_query). 10 of the 11 carry read-only annotations;canister_update_callis the one write. Theskill://resources (the official IC skills) and thecandid:///oql://references stay served.What is no longer served
IcProtocolTools— the 15icp_protocol/meta tools (dashboard name/id lookups, the IC skills as tools, canister creation/management) — is not part of the default composition.tools/callon one of those names returns the router's standard "tool not found" error (-32602invalid params — the same shape any unknown tool name gets, so these are indistinguishable from nonexistent tools). The type remains public, documented, tested library code (pub fn new, pub router) so an embedder can dispatch its router from their ownServerHandler; that is API documentation on the type itself, not product messaging.Changes
crates/imcp2-core/src/tools.rs—IcTools { canister, skills };list_tools/call_tool/get_toolroute to the canister router only;icp_oql_guidelives onIcCanisterTools;SERVER_INSTRUCTIONSdescribes the served surface only, with no forward-looking framing (for an unknown canister id: web search or ask the user; canister creation and management is the user's own icp CLI work). New deferral test (11 served / 15 not routable, and the instructions carry no future-version promise) and taxonomy test with the documentedicp_oql_guideexception; the annotations test still covers all 26 definitions.crates/imcp2-core/src/lib.rs— crate docs describe the composition.crates/imcp2-core/src/calls.rs— OQL guidance points at the servedicp_oql_guide.crates/imcp2-core/src/identities.rs,skills.rs— served-surface guidance (theresolve_appinput schema, the unknown-skill resource error,get_app_principal's Questions-only note) names only served tools.crates/imcp2-local— round-trip test sees the 13-tool merged surface (11 core +authenticate+auth_status) and asserts an unserved call (icp_get_skill) fails with exactly the router's tool-not-found error (-32602), end to end.README.md,src/assets/index.html,docs/scoping-local-deployment.md— tool table, landing groups, local counts (13 local tools) and the security model reflect the 11-tool surface; the "Creating & managing canisters" section and the deferred-tools blockquote/landing group are removed.docs/anthropic-directory-submission.md,docs/openai-directory-submission.md,docs/icp-mcp-privacy-policy-draft.md,src/assets/privacy-policy.html— describe today's product only: no funding or canister-management tools exist, users run those operations with the icp CLI, and the reviewer steps describe the actual Questions-only rejection path (the network rejects the update; the server instructions prime the assistant to recommend reconnecting).No "future version" / "anticipate" phrasing remains in any user- or agent-facing surface.
Verification
cargo testworkspace-green: 149 core + 20 local tests, including the 13-tool round-trip and the unserved-call regression.cargo clippyat the existing 10-warning baseline.🤖 Generated with Claude Code
https://claude.ai/code/session_01LvwAVXfx5kLNc4SqKXkavk