feat: add typed MCP App client - #601
Conversation
🦋 Changeset detectedLatest commit: e5f3fe4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6ef29c5ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…lient # Conflicts: # docs/diagnostics.md
…lient # Conflicts: # docs/entry-conventions.md # packages/agent-bundle/tests/entries.test.ts # website/docs/en/examples/mcp-app.mdx # website/docs/zh/examples/mcp-app.mdx
…landed after the router change)
…#590) (#610) * docs(website): TypeDoc router 'module' — one API page per entry point (#590) * docs(website): point en API links at module pages (#590) * docs(website): point zh API links at module pages (#590) * docs(website): finish module-routed API config (#590) * docs(website): describe the module-page API reference (#590) * docs(website): anchor rewrite requires a page-unique heading at any depth (#590) * review(#610): anchor rewrite only for dead fragments with exact github-slugger ids; built-link check matches authored API member labels to their headings * review(#610): heading ids from @rspress/shared/github-slugger, the slugger Rspress runs, instead of an inlined approximation * docs(website): architecture page links ArtifactManifest on its module page (#597 landed after the router change) * ci(dependabot): group @rspress/* so core and shared bump together (#610 review) * docs(website): API sidebar and overview list the app module page (#601 landed after the router change) * docs(website): drop comments and a zh clause that restated their code and prose (#610 deslop)
…artifact, one browser relay, /web in dev (#564) (#620) * web-host: extract the shared host core from serve-app (#564, lane 1) Lift the standalone MCP App host out of serve-app/serve-mcp-app.ts into plain-Node modules under src/web-host/ that both `agent-bundle serve-app` and the generated `<plugin> web` command (later lanes) run: - select-app.ts: parseAppSelector (was parseServeAppSelector), appNameOf, openApp over an AppSelectionSource, requireJsonObject. - session.ts: openStdioAppSession / sessionAuthorityFor; the session now carries a `selection` adapter over the SDK client. - page.ts: renderWebHostPage({ script, seed }), WEB_HOST_TOKEN_HEADER (x-agent-bundle-web-host), webHostContentSecurityPolicy; the HTML shell, style, and escaping move here from serve-app-page.ts, the inline relay script is replaced by the caller-supplied built page script, and the seed element is #agent-bundle-web-host-seed. - page-script.ts: readWebHostPageScript() reads dist/web-host/page.js once. - host-server.ts: startWebHost / validPort / validProfile — the loopback HTTP host as plain async acquire/release, closing newest-first. serve-mcp-app.ts keeps its public surface and becomes the framework-side orchestration (validate, artifact, launch env, session, selection, host) inside the existing Effect scope; serve-app-page.ts is deleted and api.ts switches to parseAppSelector. page.ts imports WEB_HOST_SEED_ELEMENT_ID / WebHostPageSeed from ./browser/seed.ts, which Lane 2 owns and provides; no stub is committed. The framework page script (dist/web-host/page.js) is also Lane 2's, so tests/serve-app.test.ts needs Lane 2's build step to run green. Tests: web-host-page.test.ts, web-host-select-app.test.ts (unit); serve-app.test.ts updated for the new seed id and token header. * feat: share browser MCP App relay * feat: add web config and manifest surface * Remove agent-bundle/serve-app-command and advertise the web surface. Generated <plugin> web replaces the spawnServeApp entry, so capability tables, doctor, and the mcp-app example now expose Apps from the composite artifact instead of spawning the framework CLI. * feat(agent-bundle): emit web command in artifact bins * feat(web-host): the `<plugin> web` command runtime — resolveWebLaunch and runWebCommand (#564) `web-host/launch.ts#resolveWebLaunch` turns a manifest `web.apps[]` entry into the StdioLaunch of its MCP server: this Node + the artifact-relative `entry` (refused when it escapes the plugin root or names no file: WebLaunchError `entry-outside-root` / `entry-missing`), cwd = plugin root, env = inherited string values, then the declared static env with `agent-bundle:path:*` tokens expanded (`plugin-data` → `<root>/.agent-bundle/web/<server>`, created when referenced), then the `AGENT_BUNDLE_PLUGIN_ROOT` anchor unless declared. `web-host/command.ts#runWebCommand` is the command: `[<server>/<app>] [--port N] [--open|--no-open] [--tool T] [--input JSON] [--allow <cap>]... [--profile <id>] [--json] [--help]`; coded WebCommandError (`usage`, `app-ambiguous`, `app-not-exposed` → 2; `manifest-missing`, `web-missing`, `manifest-invalid`, `server-exited` → 1); reads the manifest's web section, picks the App, launches → openStdioAppSession → openApp → startWebHost, prints the shared ready line or one stable JSON line, and stays until the signal aborts (0) or the server exits (1 with its stderr tail). The modules it drives are an injectable runtime so the command is unit-tested against fakes. `src/web-host.ts` is the public `agent-bundle/web-host` entry (rslib entry and package export land with Lane 2). Plain Node: no Effect, no compiler modules (AB6005/AB4837). * feat(cli-entry): dispatch the framework-owned `web` command before the authored tree (#564) `RunGeneratedCliOptions.web?: GeneratedCliWebCommand` — when the generated bin passes it, `<plugin> web ...` reaches `web.run(argv after web, { name, signal, writeErr, writeOut })` right after the `--version` check and before the compiled command tree is consulted, and the root `--help` lists `web` among the top-level commands in sorted position. Without the hook nothing changes: `web` is an unknown command as before. * feat(dev): serve exposed web apps * test: retire the serve-app-command tests and fixture for <plugin> web (#564) Delete tests/packed-serve-app-command.test.ts, tests/serve-app-command-spawn.test.ts, tests/serve-app-command.test.ts and fixtures/serve-app-command/ — the spawnServeApp route is replaced by the framework-owned web command. The AB4837 recovery expectation in cli-routes-build.test.ts and route-framework-imports.test.ts now reads the web.apps / <plugin> web sentence. * test: acceptance proofs for the <plugin> web command (#564) - tests/packed-web-command.test.ts (packed pool): fixtures/web-surface consumer built with the installed CLI, source removed (packed-deleted-source), then bin/<plugin>.mjs web --no-open --json as a real process: sorted JSON ready document, human ready line, seed element, CSP, 403 without the token header, SIGINT 130 / SIGTERM 143 with no surviving descendants, --help listing both the authored dashboard command and web, exit 1 without a manifest, exit 2 for an unexposed App or an --allow outside the vocabulary, no surviving agent-bundle or effect import. - packages/workbench/tests/web-command.e2e.test.ts (integration pool): builds examples/mcp-app, runs its bin, opens the page at 1440x900, waits out the binding status, asserts the sandbox-origin proxy frame and the srcdoc App frame, the rendered status panel, the relayed resources/read and tools/call round trips, the hidden consent panel, then SIGINT teardown. - tests/support/bin-process.ts: spawn/await-line/teardown helpers shared by both. - artifact-cli-bin.test.ts: a project with web.apps and no src/cli still emits bin/<plugin>.mjs whose --help lists web and whose manifest carries the web section. - rstest.integration-tests.ts: pool membership for the new files. * docs: document the web surface, <plugin> web, AB4341, and serve-app-command removal * test(web-command): canonical input type in the fake selection (#564 integration) * build(bin): a web-only bin imports no @agent-bundle/runtime; the web command hosts the artifact it ships in (#564 integration) * docs(site): publish agent-bundle/web-host in the API reference (#564) * fix(web): web never displaces an authored executable (AB4341 names the collision); per-page opening id isolates concurrent dev /web pages; manifest keys via hasDataKeys * fix(config): optional generatedCli access in web bin ownership rule * fix(dev): watcher ignores output staging dirs (spurious rebuild after every package build); web-config fixtures on #601 route ids; tool match via mcpRouteProtocolName; architecture prose marks the bridge fork as landed * chore: changeset PR number * chore(doctor): replace stale web-surface plan comment with the lenient-read rationale The comment said to switch to readWebManifest "once web-host/manifest.ts lands (Lane 3)" — that module landed in this same branch, and switching would be wrong anyway: doctor reads the web section leniently by design so it reports what a bundle contains instead of failing on a malformed manifest (the strict parser rejects the minimal manifests doctor accepts). Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com> * chore(564): deslop pass over the web surface diff Sol lane (source, 36 edits): drop comments that restate code, replace type-bypassing casts with narrowed types, reuse core/strict-json and core/errors helpers (parseJsonWithoutDuplicateKeys, isErrno, errorMessage), and make install/doctor read the manifest web section through web-host/manifest.ts instead of its own lenient parser. Grok lane (tests, 66 edits): drop restating comments, import WEB_HOST_SEED_ELEMENT_ID / WEB_HOST_TOKEN_HEADER / WebHostPageSeed instead of redeclaring them, narrow with instanceof instead of casts. Integrator: restore the three relay comments that pre-existed on main, fix manifest.ts narrowing with throw, give the doctor test a valid manifest web section. * fix(564): isolate web-host teardown listeners; web-only bins skip state/providers - web-host/session.ts: each close listener runs isolated (sync throw and rejection), matching McpSessionService's watcher dispatch; lease watchSessionClosed forwards the listener's promise so rejections are caught - entry-shell.ts: a command-less (web-only) bin mounts neither state nor providers, so their modules cannot keep <plugin> web from starting - docs/diagnostics.md: AB4341 documents nine shapes - tests: web-session.test.ts (throwing/rejecting listeners, lease), web-only bin with state+providers equals the plain web-only bin * fix(build): a web-only bin is judged on the web capability row, not cli generatedCliBinEntry gives every generated bin a generatedCli (empty for a web-only plugin), so targetHostsGeneratedBin always consulted cli and a custom adapter without a web row still emitted the web bin. generatedBinCapability decides by command count; test pits opposing cli/web states. --------- Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Summary
agent-bundle/appwithcreateAppClient(): secure opaque-sandbox initialization, exact source/origin pinning, request ids, timeout/abort cancellation, JSON-RPC decoding, structured results, typed opening notifications, rebind, and disposalAppRegistertool contracts from canonical route typegen; carry each canonical route id in private_metaso the shared host bridge rejects calls for a server/tool other than the bound onecreateMcpAppBridgehonor App-owned cancellation, reject duplicate in-flight ids, and recover repeated initialization; thread abort signals through the MCP session@modelcontextprotocol/ext-appsAB4837; add one patch changesetDesign: #594 (comment)
Host boundary pointer: #564 (comment)
API
Generated
.agent-bundle/routes.d.tsaugmentsAppRegister.routeswith tool-only{ input, result }contracts inferred from canonical schema exports.call()returns objectstructuredContentdirectly and rejects malformed, missing, orisErrorresults.request()is the lower-level JSON-RPC escape hatch.#564 can mount the existing shared
createMcpAppBridgehost core and speak this standard MCP Apps wire. This PR does not own<plugin> web, the standalone host page/process, frame relay extraction, sandbox proxy, or host HTTP routes.Tests
serve-app, Workbench real App, and host-embedding journeys exercise the same contractmcp-apps/and shared MCP entry pathsFinal local gates after merging latest
origin/main(including #578):pnpm buildpnpm typecheck∥pnpm lint∥pnpm test:unit∥pnpm test:route-unitpnpm docs:site:buildpnpm lint:releaseAll pass.
cargo-hauler dry-run
Read-only origin:
/fast/projects/agent-plugins/cargo-conductor; disposable proof:/tmp/issue-594-scratch.JsonRpcMessage,ToolCallResult,PendingRequest, request-id allocator, pending map, 15 s timers, wildcardpostMessage, response router, manual initialize/initialized frames, and pending cleanuppnpm typecheckpass against the packed tarball and generatedAppRegisterdist/app.jsgraph: three local files, zero external specifiers, no Node/Zod/Effect/compiler/server-route bytesThis unblocks cargo-hauler #107 §3.
Self-review
Reviewer: Claude Fable 5.1 Thinking Max (
change-risk-reviewer). No Grok model was used for any recorded self-review pass.Actionable findings and disposition:
call()classified conventionalisErrorresults withoutstructuredContentas malformed — fixed by checking the error envelope first and preserving fulldata; regression test added.rebind({ targetOrigin })mutated a live connection before validation — fixed by pre-validating; regression test proves pending work and connection remain live.e5f3fe49b: the typed client carries canonical route id in private_meta, and the bridge validates it against the immutable bound server/tool before consent or dispatch; standard clients without the extension remain compatible.After the fixes and the #578 merge, the reviewer reran on the complete final diff through
e5f3fe49band reported no remaining actionable defect, breaking change, security issue, missed test, or doc/changeset gap.Closes #594