Skip to content

examples(mcp-app): convention-first routes own the status server, resource, and App (#726) - #735

Merged
ScriptedAlchemy merged 3 commits into
mainfrom
feat/726-mcp-app-conventional
Sep 7, 2026
Merged

examples(mcp-app): convention-first routes own the status server, resource, and App (#726)#735
ScriptedAlchemy merged 3 commits into
mainfrom
feat/726-mcp-app-conventional

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Closes #726.

examples/mcp-app is convention-first. The handwritten src/mcp/status.ts (McpServer, registerTool, registerResource, the @modelcontextprotocol/server dependency), the config-declared mcp.servers.status.apps block, src/compiler-status-contract.ts, and the App's hand-typed StatusPanelRouteContracts / AppRegister augmentation are gone. Route modules own registration and the browser client's types:

Route Module
tool:status/show-status src/mcp/status/tools/show-status.tsx_meta.ui.resourceUri: appResourceUri('status')
resource:status/readiness-policy src/mcp/status/resources/readiness-policy.tsxpolicy://mcp-app-example/readiness
app:status/status src/mcp/status/apps/status.ts + status.html (moved from views/)

src/service-status.ts holds the schemas and the two records; src/readiness-policy.ts holds the policy text and URI (browser-safe, no zod in the App bundle). The App calls tool:status/show-status typed from the generated .agent-bundle/routes.d.ts.

The two advertised controls are real routes, not deleted: the App's Refresh status re-calls show-status for the service on screen (there is no refresh-status tool — the old view called one the handwritten server never registered), and Read policy reads the readiness-policy resource that now exists. The App is built for every target the status server ships to (AB4828), not portable only.

Deletion totals: 23 files, +282/−277 → net example source −136 lines (src/mcp/status.ts −57, compiler-status-contract.ts −37, config −14, App view −49 of hand-typed contract/bridge code).

Tests

  • examples/mcp-app/tests/browser-app/status-panel.browser.test.ts — real compiled App over the product bridge; refresh now waits for the opening result and asserts the show-status re-call.
  • packages/workbench/tests/web-command.e2e.test.ts — real web command, real generated server, source directory deleted before launch; asserts the policy text and “Status refreshed.” from the real routes.
  • packages/workbench/tests/examples-real.e2e.test.ts — the App leaf now exists, so the Workbench App workspace is driven for the first time: select payments-api, Call tool and preview, assert #service/#status inside the sandboxed App frame.
  • packages/agent-bundle/tests/examples-contract.test.ts — manifest route graph [app, resource, tool], listing carries _meta.ui, invoke returns the degraded record.
  • packages/agent-bundle/tests/workbench-surface.test.ts — three-route catalog and Tools/Resources/Apps subgroups.

Docs

examples/mcp-app/README.md, website/docs/{en,zh}/examples/mcp-app.mdx, website/docs/{en,zh}/guide/authoring/mcp.mdx (the example no longer hand-declares an AppRegister map), docs/entry-conventions.md.

No changeset: only examples/* (private), tests, and docs change — skip-changeset.

Local gate (branch contains origin/main @ a2eb7f4, i.e. after #729 and #734)

pnpm install --frozen-lockfile --offline && pnpm build && pnpm typecheck && pnpm lint   # ok
pnpm test:unit                                                     # 4468 passed, 0 failed
cd examples/mcp-app && pnpm check && pnpm test:browser-app          # validate/build/typecheck ok; 7 passed
pnpm exec rstest --config rstest.integration.config.ts (whole pool) # 1170 passed, 0 failed
pnpm test:packed                                                   # 41 passed
pnpm docs:site:build                                               # parity ok, 0 broken links

Deslop

Deslop: Claude, 3 edits (duplicate currentService assignment, consent comment, EOF blank line).

Self-review

Reviewer: GPT-5.6 Sol Max (generalPurpose; change-risk-reviewer had no shell/TraceDecay in this session). Two passes.

# Finding Disposition
1 Medium — currentService only set on success, so Refresh was a no-op after a failed opening call (Codex P2 raised the same) Fixed: set from onToolInput; browser-app error test now retries via Refresh and asserts the show-status call + recovery (7793244).
2 Low — records documented as immutable but plain objects Fixed: claim removed from comment, README, en/zh docs.
3 Low — en step 4 described the old immediate preview; zh and the e2e describe select service + Call tool and preview Fixed: en mirrors zh.
Stale references to deleted files/tools, eval/Skill references, non-public imports, zod dep, App element ids, en/zh parity No findings.

Pass 2: all resolved, no new risks.

@ScriptedAlchemy ScriptedAlchemy added the skip-changeset PR changes a publishable package but ships no observable change; changeset not required label Sep 7, 2026
@changeset-bot

changeset-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 7793244

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T05:58:24.958595Z 113c246 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ScriptedAlchemy
ScriptedAlchemy force-pushed the feat/726-mcp-app-conventional branch from 113c246 to 81e0b43 Compare September 7, 2026 05:56

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

4. Select the `status` App under **Application → MCP → status → Apps**. Its live preview occupies
the route workspace. **Advanced → Artifact** shows the composite plugin root, where
`mcp-apps/status.html` appears whichever target is selected.

P2 Badge Document the required tool call before previewing

After this example becomes a conventional App route, selecting the App does not immediately render a live preview: AppRouteWorkspace initially shows “Call the bound tool to render the App with its result,” and the user must choose a service and click Call tool and preview. This English walkthrough therefore prevents readers from reproducing the populated state; update it to describe the invocation step, as the corresponding Chinese walkthrough does.

AGENTS.md reference: AGENTS.md:L139-L142

ℹ️ 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".

Comment thread examples/mcp-app/src/mcp/status/apps/status.ts
@ScriptedAlchemy
ScriptedAlchemy force-pushed the feat/726-mcp-app-conventional branch from 7990a50 to 7793244 Compare September 7, 2026 06:20

@ScriptedAlchemy ScriptedAlchemy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current-head example review — 77932441aacc9f92f241e83cf13495894a7d8f7a

No additional concrete code blocker found in the inspected migration. Complete/record the actual final-head gate and self-review before merge. Reviewed the changed-file inventory, conventional tool/resource/App modules, shared domain records, client typing, package/lockfile changes, browser and source-deleted web tests, and the updated walkthrough. Repository/browser tests were not run by this reviewer.

#726's requested ownership change is real

The custom MCP server factory/registration and manual browser contract augmentation are deleted. The framework now registers the real show-status operation, readiness-policy resource and bound App; AppRouteInput/AppRouteResult derive from generated routes, not a second handwritten map. The browser keeps createAppClient rather than recreating transport. Moving the real read-policy URI out of ui:// is appropriate for the new non-App resource; the old URI was not a working registered operation.

The tests now distinguish mock bridge coverage from an actual generated-server journey: web-command.e2e.test.ts deletes source before launch, reads the real policy, and waits for the real refresh response rather than treating unavailable as successful example functionality. Preserve that level of proof.

Previous review findings are fixed in this head

  • onToolInput stores currentService, allowing a failed opening call to be retried without relying on a successful result first.
  • The English walkthrough explicitly says choose a service and Call tool and preview, matching the conventional App workspace.

Reply to those existing threads with this evidence; do not add the same fixes again.

Gate / integration details

The PR body still has _pending_ local gate and self-review sections at the time checked. Those are missing evidence, not reproduced failing tests. Record the accepted local build/typecheck/lint/unit gate plus the changed browser, examples-contract, workbench-surface, examples-real and web-command tests, and the English/Chinese docsite on the final head. This review does not add a requirement to wait for hosted CI instead of the repository's local-gate policy.

#734 also edits examples-contract/workbench-surface/examples-real expectations. Whichever lands second must retain both changes and rerun the shared tests rather than resolve route counts mechanically. Keep dedicated custom-factory interoperability fixtures: a convention-first primary example is not removal of the supported low-level escape hatch.

No changeset is demanded solely for private examples/tests/docs. No approval, code change, merge, host execution, or publication was performed.

@ScriptedAlchemy
ScriptedAlchemy merged commit 4036e89 into main Sep 7, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR changes a publishable package but ships no observable change; changeset not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P2] Make the MCP App example convention-first; generate its contracts and implement the controls it advertises

1 participant