Add mcpTool and important to defineAction - #3454
Conversation
Two configurations lived in plugin-level name lists that an action could not see: `mcp.connectorCatalog` decided which actions external agents get served, and `initialToolNames` decided which schemas ride the first model request. Both drift the moment an action is renamed, and neither is visible where the action is written. - `mcpTool: false` is a hard veto on every MCP tier, filtered from `actions` rather than the advertised listing so the `--full-catalog` tier — where `actions` IS the callable surface — cannot leave it callable by name. It also drops the action from both direct-A2A projections. - `mcpTool: true` is connector-catalog membership declared on the action, and activates the connector tier on its own, so a template can retire its `connectorCatalog` array. It feeds the A2A local-capability set too, so both surfaces keep serving the same curated list. - `important: true` puts an action in the first-request tool list and narrows the derived default from "every app action" to the marked ones — otherwise the annotation would be decorative and the array would stay. `important: false` drops an action from the derived set only; a name a template still lists explicitly is never silently deleted. Both name lists keep working unchanged, so an app migrates one action at a time.
|
Here's a visual recap of what changed: Open the full interactive recap |
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🔴
Review Details
Code Review Summary
PR #3454 adds action-owned mcpTool and important metadata, preserves the declarations through action discovery, derives first-request tools from important, and merges action declarations with the existing MCP connector catalog. The core approach is sound: omitted values remain distinguishable from explicit booleans, the full-catalog callable set is filtered rather than merely hiding schemas, and the focused tests cover the main connector and A2A paths. This is a standard-risk change because it modifies shared action exposure and agent-surface policy.
Key Findings
- 🔴 HIGH: An action can combine
agentTool: falsewithmcpTool: trueand still be added to the MCP connector catalog. This violates the documented “mcpTool never widens agentTool” contract and can expose an action that should be hidden from all agent surfaces. - 🟡 MEDIUM: Public A2A skill discovery does not apply the new
mcpTool: falseveto, so a supposedly external-hidden action can remain advertised in the public agent card.
The implementation has good backward-compatibility intent for existing name-based configuration and includes useful regression coverage for full-catalog callability.
🧪 Browser testing: Skipped — PR only modifies backend/config/docs, no UI impact
| rawQueryAllowed && | ||
| !denied.has(name) && | ||
| entry.agentTool !== false && | ||
| entry.mcpTool !== false && |
There was a problem hiding this comment.
🟡 Apply the mcpTool veto to public A2A skills
mcpTool: false is enforced for authenticated direct and delegated A2A actions, but filterPublicAgentActions() still includes every anonymously exposed publicAgent action. An action with publicAgent.expose: true and mcpTool: false remains advertised in the public A2A skills card, exposing its name, description, and schema despite the hard-veto contract; apply the veto to the public projection and add a regression test.
Additional Info
Found independently by 3 of 3 parallel code-review agents.
`mcpTool` defaulted to a flat `true`, so an `agentTool: false` action was hidden from the agent but still described as externally exposed. One flag should stay one decision: an undeclared `mcpTool` now inherits `agentTool`, and declaring it overrides that inheritance in both directions — including `agentTool: false` with `mcpTool: true`, an MCP-only action that outside agents get and the app's own agent never sees. `isActionExposedToExternalAgents` in `action.ts` is the single resolver; no surface re-derives it from the two raw fields. Making MCP-only reachable took three gates that all tested `agentTool` alone: - `filterAgentTools` drops these actions before the MCP and A2A mounts, which is right for every in-app surface. The plugin now re-merges exactly the MCP-only set into those two mounts through `filterMcpOnlyActions`, and nowhere else: an `ask_app` run is the app's own agent, which `agentTool: false` already answered. - `executeAgentToolCall` and `searchToolRegistry` are backstops behind a surface-scoped registry, so they now refuse only what no surface may run. Testing `agentTool` alone made them reject the MCP-only actions their own caller had just handed them — advertised over MCP, then "unknown tool" on call, and invisible to `tool-search` on a trimmed catalog. The boundary filter also closes a smaller gap: the standalone stdio server builds its surface from `autoDiscoverActions` and was serving `agentTool: false` actions to external clients.
There was a problem hiding this comment.
Builder reviewed your changes and found 1 potential issue 🟡
Review Details
Incremental Code Review Summary
The latest commit adds the intended MCP-only model: actions with agentTool: false, mcpTool: true are re-merged into external MCP/A2A registries while remaining absent from the app’s own agent surface. It also centralizes the external-exposure inheritance rule and updates the tests and documentation to describe mcpTool inheriting agentTool. The previously reported MCP connector widening issue is fixed and its review thread was resolved. The public A2A-card issue remains open and was intentionally not reposted.
New Finding
- 🟡 MEDIUM: The secondary dev/test filesystem discovery path reconstructs action entries without retaining
mcpToolorimportant. As a result, actions loaded through that path cannot become MCP-only and theirimportantannotation does not affect first-request tool selection.
The main architecture and compatibility behavior otherwise look sound, including explicit support for MCP-only actions and preserving the existing name-based configuration. This remains a standard-risk shared-core exposure change.
🧪 Browser testing: Skipped — PR only modifies backend/config/docs, no UI impact
| const mcpOnlyActions = filterFrameworkToolGroups( | ||
| filterMcpOnlyActions({ |
There was a problem hiding this comment.
🟡 Preserve MCP metadata in the dev discovery registry
The secondary filesystem discovery path that feeds discoveredActionsAll reconstructs imported entries with tool, run, http, and agentTool, but drops mcpTool and important. An action declared { agentTool: false, mcpTool: true } is therefore omitted from the new mcpOnlyActions merge instead of being available externally, and important: true is ignored for initial-tool selection. Carry both flags through the direct-import and shell-wrapper fallback paths, with an integration test for a filesystem-discovered action.
Additional Info
Found independently by 2 of 3 parallel incremental code-review agents.
3 participants
Two configurations lived in plugin-level name lists that an action could not
see:
mcp.connectorCatalogdecided which actions external agents get served,and
initialToolNamesdecided which schemas ride the first model request.Both drift the moment an action is renamed, and neither is visible where the
action is written.
mcpTool: falseis a hard veto on every MCP tier, filtered fromactionsrather than the advertised listing so the
--full-catalogtier — whereactionsIS the callable surface — cannot leave it callable by name. Italso drops the action from both direct-A2A projections.
mcpTool: trueis connector-catalog membership declared on the action, andactivates the connector tier on its own, so a template can retire its
connectorCatalogarray. It feeds the A2A local-capability set too, soboth surfaces keep serving the same curated list.
important: trueputs an action in the first-request tool list and narrowsthe derived default from "every app action" to the marked ones — otherwise
the annotation would be decorative and the array would stay.
important: falsedrops an action from the derived set only; a name a template stilllists explicitly is never silently deleted.
Both name lists keep working unchanged, so an app migrates one action at a
time.