Conversation
…o-builtins
Both workers previously gated on a single boolean metadata flag
(mcp.expose / a2a.expose) with --expose-all as the only escape hatch.
Past ~20 functions the resulting tool list becomes noise: engine
internals sit next to agent-facing functions as if equivalent, and
Claude Desktop / MCP Inspector burn context just reading it. This PR
tightens the surface:
Hard floor — ALWAYS_HIDDEN_PREFIXES (engine::, state::, stream::,
iii., mcp:: or a2a::). These NEVER appear in tools/list or agent-card
skills, even under --expose-all. Matches the agent worker's existing
DEFAULT_EXCLUDED_PREFIXES carve-out.
Tier filter — optional mcp.tier / a2a.tier string on the function
metadata plus a --tier <name> CLI flag. One worker can tag functions
as 'user' / 'agent' / 'ops' and three server instances behind three
different auth proxies show three different tool lists.
no-builtins — new --no-builtins flag on iii-mcp hides the 6 built-in
worker-management tools (iii_worker_register, iii_worker_stop,
iii_trigger_register, iii_trigger_unregister, iii_trigger_void,
iii_trigger_enqueue). Defaults: off for stdio (common Claude Desktop
path needs them), on for --no-stdio HTTP (over HTTP worker
management rejects itself as 'requires stdio' anyway — pure noise).
Internals:
- mcp/handler.rs: new ExposureConfig carrying { expose_all, no_builtins,
tier } threaded through McpHandler and dispatch_http. is_function_exposed
centralizes the three gates (hard floor, metadata flag, tier match) and
is reused in tools/list + tools/call + HTTP tools/call.
- a2a/handler.rs: same ExposureConfig { expose_all, tier } threaded
through register/build_agent_card/handle_a2a_request/handle_send.
is_exposed helper for in-hand FunctionInfo, is_function_exposed async
helper for looking up a function_id before dispatch.
- mcp/main.rs + a2a/main.rs: add --tier CLI, mcp adds --no-builtins.
- docs: add mcp/README.md and a2a/README.md documenting the opt-in
pattern with Rust/Node/Python registration snippets and a worked
example of the tier flag across three audiences.
- initialize_result() instructions string updated to describe the new
gate.
Verification:
- cargo fmt --all -- --check ok on both
- cargo clippy --all-targets --all-features -- -D warnings ok on both
- cargo test --all-features 0/0 + 0/0 (integration tests live elsewhere)
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a centralized ExposureConfig (expose_all, optional tier, plus always-hidden prefixes) and applies tier-aware, hard-floor exposure filtering across A2A and MCP; updates CLI flags, handler registration signatures, and documentation; preserves fail-closed listing and refines error messages. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AgentCard as "Agent Card /.well-known/agent-card.json"
participant A2A as "A2A /a2a (Handler)"
participant Registry
participant TaskStore
Client->>AgentCard: GET discover base_url / capabilities
Client->>A2A: POST message/send (function_id in structured data or text)
A2A->>Registry: resolve function_id
Registry-->>A2A: function metadata (including a2a.expose, a2a.tier, namespace)
A2A->>A2A: apply ExposureConfig (expose_all, tier) + ALWAYS_HIDDEN_PREFIXES
alt exposed
A2A->>TaskStore: persist task (a2a:tasks) / check idempotent terminal result
A2A->>Registry: invoke function / stream result
TaskStore-->>A2A: store result
A2A-->>Client: return result
else not exposed
A2A-->>Client: JSON-RPC error (not-exposed / denied)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
a2a/src/handler.rs (1)
434-452:⚠️ Potential issue | 🟡 MinorMisleading rejection message: doesn't reflect hard-floor or tier gates.
The error text blames only
a2a.exposemetadata, butis_function_exposedrejects for three distinct reasons: hard-floor namespace, missinga2a.expose(when--expose-allis off), anda2a.tiermismatch (when--tieris set). An operator invokingengine::fooor a function with the wrong tier will be told to checka2a.expose, which is the wrong fix.Consider a message that doesn't pin a specific reason, or have
is_function_exposedreturn a discriminated reason so the surface matches reality:🛠️ Proposed fix
parts: vec![text_part(format!( - "Function '{}' is not exposed via a2a.expose metadata", - function_id + "Function '{}' is not exposed (check hard-floor namespaces, a2a.expose, and --tier)", + function_id ))],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@a2a/src/handler.rs` around lines 434 - 452, The rejection text incorrectly blames only a2a.expose even though is_function_exposed can fail for hard-floor namespace, missing expose when --expose-all is off, or a2a.tier mismatch; update the code so the rejection message accurately reflects the real cause by either (A) changing is_function_exposed to return a discriminated enum (e.g. ExposeRejection::HardFloor | ::NotExposed | ::TierMismatch) and use that enum in the handler to set TaskStatus/Message with a specific, correct message, or (B) if you prefer a quicker change, replace the current hardcoded error text around function_id with a neutral message such as "Function is not allowed to be invoked by current namespace/exposure/tier settings" so the user isn't misled; refer to is_function_exposed, TaskStatus, TaskState, Message and function_id when implementing the fix.mcp/src/handler.rs (1)
352-378:⚠️ Potential issue | 🟠 MajorFilter the functions resource with the same exposure rules.
iii://functionsstill returns rawlist_functions()output, so MCP clients can enumerate hard-hidden or tier-filtered functions even whentools/listhides them. Applyis_function_exposedhere too, or remove this resource from unprivileged MCP surfaces.Suggested fix
"iii://functions" => { - let v = self + let functions = self .iii .list_functions() .await .map_err(|e| format!("{}", e))?; + let v: Vec<_> = functions + .into_iter() + .filter(|f| is_function_exposed(f, &self.exposure)) + .collect(); ( serde_json::to_string_pretty(&v).unwrap_or_else(|_| "[]".into()), "application/json",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp/src/handler.rs` around lines 352 - 378, resources_read currently returns the raw output of iii.list_functions() for "iii://functions", exposing hidden/tier-filtered entries; update the "iii://functions" branch in resources_read to filter the listed functions using the same is_function_exposed predicate used by tools/list (call is_function_exposed for each function and only include exposed ones) before serializing, or alternatively remove "iii://functions" from resources_list to avoid exposing the resource to unprivileged MCP surfaces; reference resources_read, resources_list, is_function_exposed, and iii.list_functions() when applying the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@a2a/README.md`:
- Around line 65-73: The fenced CLI block in README.md is missing a language tag
(MD040); update the opening fence for the CLI snippet that begins with ``` to
use a language identifier (e.g., change the opening fence to ```text or
```console) so the block showing the flags (--engine-url, --expose-all, --tier,
--base-url, --debug) validates and renders predictably.
In `@a2a/src/handler.rs`:
- Line 28: ALWAYS_HIDDEN_PREFIXES currently includes "iii." but not "iii::", so
functions using the SDK double-colon notation (e.g., iii::durable::publish,
iii::config) will slip past the hidden-prefix check; update the constant
ALWAYS_HIDDEN_PREFIXES in handler.rs to include "iii::" (add the string "iii::"
to the slice) so both notations are covered by the prefix-matching logic.
In `@mcp/README.md`:
- Around line 87-90: The README incorrectly documents an unsupported override
(--no-builtins=false); update the text in the README section that mentions "Flip
with `--no-builtins=false`" to remove that override and instead note that the
CLI only provides the negative flag `--no-builtins` and that mcp/src/main.rs
forces HTTP-only mode to hide built-ins, so there is no runtime override; ensure
the sentence clarifies that built-ins remain hidden over HTTP and delete or
replace the false-override example.
In `@mcp/src/handler.rs`:
- Around line 322-333: The current code skips the exposure check when
self.iii.list_functions().await returns Err, allowing calls to proceed to
self.iii.trigger(...) even if exposure cannot be verified; change the logic to
fail closed: if list_functions() returns Err, treat the function as not exposed
and return the same tool_error response instead of continuing. Specifically,
adjust the block that calls self.iii.list_functions().await and the identical
block used around lines 603-617 so that failures from list_functions() set
exposed = false (or immediately return the tool_error), using the existing
helpers is_function_exposed, self.exposure, and function_id to decide and bail
out rather than falling through to iii.trigger.
- Around line 225-229: The current code only hides built-ins from discovery by
using self.exposure.no_builtins when building the tools list (via
builtin_tools()), but it does not block direct execution of built-in tools;
update the tool invocation paths (the HTTP/tools/list and tools/call handlers
and any call-resolution logic in the functions covering the ranges around
241-311, 520-524, and 544-591) to consult self.exposure.no_builtins and refuse
or filter calls to built-in tool names (e.g., iii_trigger_void,
iii_trigger_enqueue) when that flag is set; specifically, add a guard in the
function that resolves and dispatches a tool call (the handler that currently
executes tools by name) to check self.exposure.no_builtins and either return an
error/403 or skip resolving builtin_tools() entries before executing.
In `@mcp/src/main.rs`:
- Around line 70-75: The HTTP handler is currently registered with builtins
visible by default because http_no_builtins is derived from args.no_stdio; add
an explicit opt-in flag (e.g. args.http_builtins) and change the logic so
builtins are hidden by default for HTTP: declare the new boolean flag in your
args parsing (name it http_builtins), compute http_no_builtins as
!args.http_builtins || args.no_builtins || args.no_builtins, or more clearly
http_no_builtins = (!args.http_builtins) || args.no_builtins || args.no_builtins
(ensure you only include the real relevant existing flags once), then pass that
into ExposureConfig::new(...) and call handler::register_http(&iii,
http_exposure) as before; this makes HTTP builtins opt-in via --http-builtins
while preserving existing --no-builtin/--no-stdio behavior.
---
Outside diff comments:
In `@a2a/src/handler.rs`:
- Around line 434-452: The rejection text incorrectly blames only a2a.expose
even though is_function_exposed can fail for hard-floor namespace, missing
expose when --expose-all is off, or a2a.tier mismatch; update the code so the
rejection message accurately reflects the real cause by either (A) changing
is_function_exposed to return a discriminated enum (e.g.
ExposeRejection::HardFloor | ::NotExposed | ::TierMismatch) and use that enum in
the handler to set TaskStatus/Message with a specific, correct message, or (B)
if you prefer a quicker change, replace the current hardcoded error text around
function_id with a neutral message such as "Function is not allowed to be
invoked by current namespace/exposure/tier settings" so the user isn't misled;
refer to is_function_exposed, TaskStatus, TaskState, Message and function_id
when implementing the fix.
In `@mcp/src/handler.rs`:
- Around line 352-378: resources_read currently returns the raw output of
iii.list_functions() for "iii://functions", exposing hidden/tier-filtered
entries; update the "iii://functions" branch in resources_read to filter the
listed functions using the same is_function_exposed predicate used by tools/list
(call is_function_exposed for each function and only include exposed ones)
before serializing, or alternatively remove "iii://functions" from
resources_list to avoid exposing the resource to unprivileged MCP surfaces;
reference resources_read, resources_list, is_function_exposed, and
iii.list_functions() when applying the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ea5179c-f380-41c1-b065-f641f50faa9a
📒 Files selected for processing (8)
a2a/README.mda2a/src/handler.rsa2a/src/main.rsmcp/README.mdmcp/src/handler.rsmcp/src/main.rsmcp/src/transport.rsmcp/src/worker_manager.rs
…r message Local integration test caught two issues: F1 (major): Under --expose-all, each protocol worker's ALWAYS_HIDDEN_PREFIXES only blocked its own namespace. mcp --expose-all surfaced a2a::agent_card + a2a::jsonrpc as MCP tools; a2a --expose-all surfaced mcp::handler as an A2A skill. Both are stateless RPC dispatchers — routing an MCP call through mcp::handler recurses into ourselves, and double-enveloping an A2A request inside MCP (or vice versa) is not a meaningful tool surface. Fix: expand both hard-floor lists to include the sibling protocol namespace. F2 (minor): A2A hard-floor rejections reused the metadata-miss message, so 'function is in engine namespace' and 'function forgot the a2a.expose flag' looked identical to callers. MCP already distinguishes them. Mirror the distinct phrasing on A2A so operators can tell a policy miss from a protocol-level block. Verification (live engine + workers, local run): - default MCP tools/list: 3 opt-in demo tools, nothing else - default A2A agent card: 3 opt-in demo skills, nothing else - --expose-all MCP: no a2a::*, no mcp::handler - --expose-all A2A: no mcp::handler, no a2a::* - --tier user: only the user-tier demo tool - tools/call on hidden function: rejected, metadata-miss message - tools/call on state:: prefix: rejected, hard-floor message - message/send on state:: prefix: rejected, distinct hard-floor message Full test matrix in the PR description.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
a2a/src/handler.rs (1)
28-38:⚠️ Potential issue | 🟠 MajorHard floor still misses the
iii::SDK notation.
ALWAYS_HIDDEN_PREFIXEScoversiii.(dot form) but notiii::(double-colon form). The engine registers SDK-internal callbacks using theiii::notation (e.g.iii::durable::publish,iii::config— seeintrospect/src/functions/diagram.rs), so under--expose-allthese would bypass the hard floor and become skills on the agent card. Theiii.entry alone doesn’t catch them becausestarts_with("iii.")doesn't matchiii::….🛡️ Proposed fix
pub const ALWAYS_HIDDEN_PREFIXES: &[&str] = &[ "engine::", "state::", "stream::", "iii.", + "iii::", // Sibling protocol worker entry point. Calling `mcp::handler` via A2A // message/send double-envelopes an MCP request inside A2A — not a // meaningful skill. Hide symmetrically with mcp's a2a:: carve-out. "mcp::", "a2a::", ];#!/bin/bash # Confirm whether any registered functions use the `iii::` notation that would # slip past the current `iii.` prefix. rg -nP --type=rust '"iii::[A-Za-z0-9_:]+"' -C1 rg -nP --type=rust '\biii::[a-z_]+::[a-z_]+\b' -C1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@a2a/src/handler.rs` around lines 28 - 38, ALWAYS_HIDDEN_PREFIXES currently contains "iii." but misses the SDK double-colon notation "iii::", so SDK-internal callbacks like iii::durable::publish and iii::config can slip past the hard floor; update the ALWAYS_HIDDEN_PREFIXES array in handler.rs to include the "iii::" entry (in addition to "iii.") so starts_with checks will hide both dot and double-colon forms and preserve the hard floor behavior for functions registered with iii:: prefixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@a2a/src/handler.rs`:
- Around line 28-38: ALWAYS_HIDDEN_PREFIXES currently contains "iii." but misses
the SDK double-colon notation "iii::", so SDK-internal callbacks like
iii::durable::publish and iii::config can slip past the hard floor; update the
ALWAYS_HIDDEN_PREFIXES array in handler.rs to include the "iii::" entry (in
addition to "iii.") so starts_with checks will hide both dot and double-colon
forms and preserve the hard floor behavior for functions registered with iii::
prefixes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 821df327-b3a3-40b4-8720-8d12715e1fdc
📒 Files selected for processing (2)
a2a/src/handler.rsmcp/src/handler.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- mcp/src/handler.rs
Handler hardcoded protocolVersion: '2025-11-25' — a future date that no
real MCP client recognizes. MCP Inspector rejected it with:
Failed to connect to MCP server: Server's protocol version is not
supported: 2025-11-25
Real spec revisions are stamped dates from spec.modelcontextprotocol.io.
Move to '2025-06-18' (current published spec at the time of this commit).
Bump when moving to a newer one.
Caught by live Inspector smoke test over Streamable HTTP. Post-fix:
Inspector lists tools, calls exposed tools, and gets the expected
rejection on hidden tools on both stdio and HTTP transports.
…licy, builtin invocation gate, resource filter F1 (hard floor gap). Both workers' ALWAYS_HIDDEN_PREFIXES listed 'iii.' but missed 'iii::'. The SDK uses both notations — callback-style 'iii.on_functions_available.<uuid>' AND namespace-style 'iii::durable::publish', 'iii::config'. Only the dot form was blocked; double-colon SDK internals slipped past the hard floor under --expose-all. Add both to the slice on both workers. F2 (builtin invocation gate). --no-builtins hid the 6 management tools from tools/list but tools_call still dispatched them by name. A client that knew the name could call iii_worker_register on a server that claimed it had no such tool, bypassing policy. Add BUILTIN_TOOL_NAMES + is_builtin_tool helper; gate tools/call (stdio AND HTTP dispatch) on cfg.no_builtins && is_builtin_tool(name). F3 (fail-open on list_functions error). Both stdio tools_call and HTTP dispatch_http used 'if let Ok(fns)' — when list_functions errored, the block was skipped and the call fell through to iii.trigger. An engine outage hid the policy gate. Flip to fail-closed: list_functions error returns an explicit tool_error 'exposure could not be verified; denying call'. F4 (HTTP builtins default). Previous main.rs computed http_no_builtins = args.no_builtins || args.no_stdio, which meant stdio-default mode (the common case) registered HTTP WITH builtins. Flip to hide by default: http_no_builtins = args.no_builtins || !args.http_builtins. Add --http-builtins opt-in flag for deploys that want builtins over HTTP. --no-builtins always wins. F5 (resource surface bypass). resources_read 'iii://functions' handed out the raw list_functions() output. A client could skip tools/list and read the resource to enumerate hidden functions — bypassing mcp.expose, tier, and hard floor. Apply is_function_exposed filter inside resources_read so the resource surface matches the tool surface. Doc fixes: - mcp/README: correct the bogus --no-builtins=false example; document --http-builtins; expand hard-floor table to show iii.* AND iii::* plus a2a::*. - a2a/README: add 'text' language tag to the CLI fenced block (MD040); expand hard-floor table to show iii.* AND iii::* plus mcp::*. Verification (live engine + workers): - HTTP default: 3 opt-in demo tools, no builtins - HTTP --http-builtins: 3 demo + 6 builtins - --no-builtins + tools/call iii_trigger_void: rejected with 'disabled on this server' message - stdio resources/read iii://functions: demo::exposed present, demo::hidden + state::bogus_demo + engine::* + iii.* + iii::* all absent - cargo fmt + clippy -D warnings clean on both workers
rohitg00
added a commit
that referenced
this pull request
Apr 23, 2026
Conflict resolved by taking main's richer mcp/src/handler.rs, which carries the opt-in exposure work merged via #37. Clippy + cargo test pass across mcp, a2a, agent, shell, coding. CR findings addressed on chore/fix-merged-workers: - agent/src/main.rs: replace `let _ = state_set(...)` with match/log. The discard swallowed engine errors, so a failed state write showed up in logs as "tool cache refreshed" (identical to success). - shell/src/functions/exec.rs + exec_bg.rs: strict args validation. The previous filter_map silently dropped non-string entries, so a caller sending `{"args": ["--count", 5]}` would have 5 removed and the shell would run with partial arguments. Now returns `IIIError::Handler("'args[i]' must be a string (got ...)")`. - coding/src/functions/execute.rs: `.kill_on_drop(true)` on all four tokio Commands (rustc compile, rust bin run, ts runtime, python3). Without it, timed-out children kept running in the background and accumulated as zombies over time. - coding/src/templates.rs: `normalize_trigger_config` helper strips a leading slash from `api_path` before emitting trigger config. The engine prepends `/`, so a user-supplied `"/myworker/greet"` became `//myworker/greet` on the wire and returned 404 at invoke time. Applied at all six trigger-serialization sites (rust/ts/python worker templates + the three single-trigger generators). - coding/src/templates.rs: migrate the TypeScript template's `iii.registerFunction({id, ...}, handler)` object-first form to the iii-sdk 0.11 positional signature `(id, handler, {description, request_format, response_format})`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Running iii-mcp over Streamable HTTP works cleanly — inspector hits `POST /mcp`, pulls the 6 builtins + every registered worker function as tools. But at scale the tool list turns into noise:
Same problem applies to the A2A agent card.
Fix
Three layers of gate, both workers:
MCP also gets `--no-builtins`
The 6 management tools (`iii_worker_`, `iii_trigger_`) are useful over stdio (Claude Desktop can spawn workers) but pure noise over HTTP (they reject with "requires stdio"). Default: on for stdio, off for HTTP.
Metadata schema (worker authors)
```rust
metadata: Some(json!({
"mcp.expose": true, // opt-in gate
"mcp.tier": "agent", // optional — free-form string
"a2a.expose": true, // same pattern for A2A
"a2a.tier": "partner",
}))
```
CLI
```
iii-mcp [--no-stdio] [--expose-all] [--no-builtins] [--tier ]
iii-a2a [--expose-all] [--tier ] [--base-url ]
```
Multi-audience example
```
Claude Desktop user config → iii-mcp --tier user --no-builtins
Agent orchestrator client → iii-mcp --no-stdio --tier agent
Ops dashboard → iii-mcp --no-stdio --tier ops
```
Same engine, three tool lists.
Test plan
Summary by CodeRabbit
New Features
Documentation
Bug Fixes