feat: refactor and normalise props #2
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/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.
ℹ️ 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".
|
Initial pass looks good. |
no preference I guess we can merge, we should deprecate iframeRenderData at some point though? |
|
I think we have to be consistent with naming also of the event, like ui-lifecycle-iframe-render-data |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| } | ||
|
|
||
| if (resource.mimeType === 'text/html+skybridge') { | ||
| const widgetStateKey = `openai-widget-state:${toolName}:`; | ||
| const widgetStateKey = `openai-widget-state:${toolName ?? ''}:`; | ||
| htmlContent = htmlContent.replace( | ||
| /<head([^>]*)>/i, | ||
| `<head$1>\n${apiScript( | ||
| `<head$1>\n${apiScript({ | ||
| widgetStateKey, | ||
| toolInput, | ||
| initialRenderData, | ||
| toolResponseMetadata | ||
| )}\n` | ||
| toolOutput, | ||
| toolResponseMetadata, |
There was a problem hiding this comment.
In processHTMLResource the value passed into the apiScript for toolOutput is computed as mcpContextProps?.toolOutput ?? initialRenderData. Because HTMLResourceRenderer already merges iframeRenderData/metadata with mcpContextProps.toolOutput, any time toolOutput is provided the merged render data is discarded and only the raw MCP tool output is embedded into the skybridge HTML (openaiAPI.toolOutput). This contradicts the docs that say the sandbox payload merges iframeRenderData with toolOutput and means HTML resources that depend on those extra fields won’t see them until a later postMessage. Consider merging the two sources before injection (e.g. spreading initialRenderData together with mcpContextProps?.toolOutput) so the initial HTML payload matches what is sent via postMessage.
Useful? React with 👍 / 👎.
this is what the new interface loos like