Skip to content

✂️ refactor: use artifacts and callbacks to pass UI resources#9472

Closed
samuelpath wants to merge 1 commit intodanny-avila:mainfrom
mcp-ui-dev:refactor-mcp-ui
Closed

✂️ refactor: use artifacts and callbacks to pass UI resources#9472
samuelpath wants to merge 1 commit intodanny-avila:mainfrom
mcp-ui-dev:refactor-mcp-ui

Conversation

@samuelpath
Copy link
Contributor

@samuelpath samuelpath commented Sep 5, 2025

Tip

A few tests were broken but had nothing to do with this PR. After a few rebases on main, all tests are now green ✅.

Summary

I recently shipped the 2 followings PRs to enable a basic integration with MCP-UI:

The approach I took to pass the UI resources received from the MCP Server tool response to the front-end was to use a content item of type text and passing the UI resources array in a new metadata property.

This worked but the issue is that these content items of type text are passed as such the LLMs. We have been lucky so far since all the LLM providers we've tested silently ignored this unknown metadata property. But there is no guarantee that all providers do, nor that existing providers could enforce stricter payload checks.

In order to work around this, we proposed the following PR in the Agents package: danny-avila/agents#16. It would remove the metadata property just before the call to the LLM would be made. However, this was rightly rejected by @danny-avila who explained that there was a better way, using callbacks.

This PR now uses @danny-avila's recommended approach with the callbacks.

Here's the idea:

  • Create a new artifact type for the UI resources Tools.ui_resources
  • Add the UI Resources as artifacts and not in the LLM content
  • The callback is called after the LLM is being called, in which we can parse the artifacts as LLM content attachment
  • We pass the attachments from ToolCall to ToolCallInfo
  • We parse the attachments of type UI Resources in ToolCallInfo to render them

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

See the 2 PRs mentioned above. The behaviour tested is the same.

Test Configuration:

Checklist

Please delete any irrelevant options.

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes
  • Any changes dependent on mine have been merged and published in downstream modules.

@samuelpath samuelpath force-pushed the refactor-mcp-ui branch 2 times, most recently from deff11b to 792260d Compare September 8, 2025 15:06
@samuelpath samuelpath force-pushed the refactor-mcp-ui branch 3 times, most recently from 742d88c to c0cecf0 Compare September 8, 2025 17:57
@samuelpath samuelpath marked this pull request as ready for review September 8, 2025 18:18
resource: (item) => {
if (item.resource.uri.startsWith('ui://')) {
uiResources.push(item.resource as t.UIResource);
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that the UI Resources are passed as artifacts and are thus not sent to the LLM, we can still pass the parsed UI Resources to the LLM as a regular resource. This will be useful in the next integration phase.

);
}

// TODO: a lot of duplicated code in createToolEndCallback
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I first introduce a test file for this file before introducing any refactoring of this class. I have some ideas to make it way simpler with way less duplications, but I first want to introduce the non-regression unit tests.

@danny-avila
Copy link
Owner

I redid the PR with my changes here since I couldn't push up to your fork: #9581

There were some minor typing and import issues that were easier for me to address to get this merged in sooner.

Thanks for your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants