Reduce program UI to display-only output - #493
Conversation
zackbart
left a comment
There was a problem hiding this comment.
REQUEST CHANGES
Blocking: keep explicit model-only visibility on every non-app tool.
src/meta-tools.ts now omits _meta.ui.visibility from skills, search_tools, call_tool, call_destructive_tool, authorize_connector, and get_result, and the new server test asserts that omission. In MCP Apps, omitted visibility defaults to ["model", "app"]: https://apps.extensions.modelcontextprotocol.io/api/interfaces/app.McpUiToolMeta.html#visibility. This therefore makes all six tools app-callable. In particular, it does not revoke call_tool from a cached v2 read-capable shell, and it newly exposes the destructive, authorization, discovery, and result tools to Apps. That contradicts the display-only boundary and the project design record, which already explains why visibility must be explicit.
The v3 URI correctly keeps new results from selecting cached v2 shell bytes, but URI versioning cannot replace the server-side tool visibility restriction. Please retain _meta: { ui: { visibility: ["model"] } } on these six tools, removing only "app" from call_tool, and change the wire test to assert the explicit model-only metadata.
I otherwise confirmed that the read manifest/bridge are gone outside the intentionally superseded historical record, the v3 URI is used consistently and v2 is rejected, the one-string connecta.ui break is covered on both executors, local UI delivery remains success-only and sandboxed, and the seven-tool surface is unchanged.
Checks run on commit 19d058d: focused Node + Workers + QuickJS/server/UI suites, 241 passed and 40 skipped; browser suite, 15 passed; npm run check, 114 files passed with 2,721 tests passed and 40 skipped.
GitHub does not permit the PR author account to submit a formal request-changes review, so this is posted as a comment review with the requested verdict.
|
Visibility finding fixed in fdf34c6. All six non-execute tools again send exact The server test now asserts the complete wire Checks after the fix:
Ready for re-review. |
zackbart
left a comment
There was a problem hiding this comment.
APPROVE
The prior Apps visibility blocker is resolved at fdf34c6.
The wire contract now gives each of the six non-execute_code tools exact _meta.ui.visibility: ["model"]. call_tool has lost only its former "app" access. execute_code is the sole tool with resourceUri: "ui://connecta/program-ui/v3", paired with the same model-only visibility. The end-to-end tools/list test asserts the complete _meta object for all seven tools, so omission, extra metadata, or a resource URI on another tool will fail.
The normative code-mode guide and design record now state why the metadata cannot be omitted: MCP Apps defaults omission to model plus app visibility. This also completes the cached-shell story. The v3 URI keeps new results off cached v2 bytes, while explicit model-only metadata revokes the old shell route at the current catalog boundary.
Verification on the fixed head:
- Focused
server.test.tson Node and Workers: 107 passed, 3 skipped. - Browser suite: 15 passed.
npm run check: 114 files passed, 2,721 tests passed, 40 skipped.- Detached review worktree remained clean.
GitHub does not permit the PR author account to submit a formal approval, so this is posted as a comment review with the requested verdict.
Summary
connecta.ui(html)as a one-string, display-only output channelethos.mdand retain the former decision as a clearly superseded historical recordCompatibility
This is a breaking guest API reduction. Programs passing a second argument to
connecta.uinow receiveinvalid_args. The static Apps resource moves fromui://connecta/program-ui/v2toui://connecta/program-ui/v3so hosts do not reuse the cached read-capable shell. The seven MCP tools and one-string UI result shape are unchanged.Version, upgrade guide, and changelog changes are intentionally left to the aggregate 0.20 release PR.
Verification
npm run check: 114 files passed, 2,721 tests passed, 40 skipped; build and both deployment-shape typechecks passednpm run test:browser: 15 passedgit diff --checkCloses #484
Part of #489