feat(chat): add plugin UI slots and generic page overlays#241
feat(chat): add plugin UI slots and generic page overlays#241RachelForster wants to merge 12 commits into
Conversation
Reviewer's Guide by Ameath小爱整理了一条清晰的查看路径,我们按重点往下看。 调用链为 文件级变更
Tips and commands
Original review guide in EnglishThe flow is |
There was a problem hiding this comment.
小爱先从这一处开始核对,细节我们慢慢看。
小爱已完成全部变更审查,共发布 2 条行级建议(高风险 0,中风险 1)。
Prompt for AI Agents
请逐项处理以下审查问题;修改前先核实上下文,修改后运行相关测试。
1. 为 chat-output 页面动作提供导航 / Supply navigation for chat-output page actions
frontend/src/shared/plugin/PluginSlot.tsx:126 (RIGHT)
<issue_to_address>FrontendChatUIContribution 可以在 chat-output 中声明 open-plugin-page,bridge 也会将其标为可执行;但 DialogLayer 以 <PluginSlot slot="chat-output" /> 渲染该插槽时没有传入 onOpenPluginPage。此分支会直接返回,而输出卡仍根据 actionable 渲染可点击按钮,所以点击不会打开页面。请传入同一导航回调,或拒绝该动作与此插槽的组合。
FrontendChatUIContribution can declare open-plugin-page in chat-output, and the bridge marks it actionable; however, DialogLayer renders that slot without onOpenPluginPage. This branch returns while the output card still renders an enabled action button, so clicking it cannot open the page. Pass the same navigation callback or reject this action/slot combination.</issue_to_address>
2. 不要让回退页吞掉请求页 / Do not let a fallback page consume the requested page
frontend/src/features/plugin-manager/PluginDetailPanel.tsx:230 (RIGHT)
<issue_to_address>冷缓存时会先用 fallbackPluginUiPages 的合成键选择第一项(例如 settings:settings-0)。如果真实详情随后包含一个合法的同名页面,同时本次动作请求另一个 pageId(如 phone),当前键仍在 pageKeys 中,条件不再进入,initialPageId 永远不会被应用,用户会落在错误的页签。应等真实详情页可用后再消费初始目标,或单独记录尚未应用的初始选择。
On a cold cache, the fallback page list first selects a synthetic key such as settings:settings-0. If the real detail later contains that valid page plus a different requested pageId (for example phone), the active key remains in pageKeys, so the initial target is never applied and the user lands on the wrong tab. Apply the initial target after real details are available or track an unapplied initial selection separately.</issue_to_address>
| if (!contribution.actionable || running) { | ||
| return; | ||
| } | ||
| if (contribution.actionType === "open-plugin-page") { |
There was a problem hiding this comment.
[MEDIUM · correctness] 为 chat-output 页面动作提供导航 / Supply navigation for chat-output page actions
FrontendChatUIContribution 可以在 chat-output 中声明 open-plugin-page,bridge 也会将其标为可执行;但 DialogLayer 以 <PluginSlot slot="chat-output" /> 渲染该插槽时没有传入 onOpenPluginPage。此分支会直接返回,而输出卡仍根据 actionable 渲染可点击按钮,所以点击不会打开页面。请传入同一导航回调,或拒绝该动作与此插槽的组合。
FrontendChatUIContribution can declare open-plugin-page in chat-output, and the bridge marks it actionable; however, DialogLayer renders that slot without onOpenPluginPage. This branch returns while the output card still renders an enabled action button, so clicking it cannot open the page. Pass the same navigation callback or reject this action/slot combination.
置信度 99% · 可直接回复讨论 · 👍/👎 会影响后续审查
| const preferredPage = detailPages.find( | ||
| (page) => page.id === initialPageId || pluginUiPageKey(page) === initialPageId, | ||
| ); | ||
| setActiveDetailPageId(preferredPage ? pluginUiPageKey(preferredPage) : pageKeys[0]); |
There was a problem hiding this comment.
[LOW · correctness] 不要让回退页吞掉请求页 / Do not let a fallback page consume the requested page
冷缓存时会先用 fallbackPluginUiPages 的合成键选择第一项(例如 settings:settings-0)。如果真实详情随后包含一个合法的同名页面,同时本次动作请求另一个 pageId(如 phone),当前键仍在 pageKeys 中,条件不再进入,initialPageId 永远不会被应用,用户会落在错误的页签。应等真实详情页可用后再消费初始目标,或单独记录尚未应用的初始选择。
Original in English
On a cold cache, the fallback page list first selects a synthetic key such as settings:settings-0. If the real detail later contains that valid page plus a different requested pageId (for example phone), the active key remains in pageKeys, so the initial target is never applied and the user lands on the wrong tab. Apply the initial target after real details are available or track an unapplied initial selection separately.
置信度 91% · 可直接回复讨论 · 👍/👎 会影响后续审查
76023d8 to
c96fec6
Compare
c96fec6 to
87be1e8
Compare
|
@Ame4th review |
Extends the chat-UI plugin slots with an "overlay" page mode: a chat-top-toolbar contribution can pop its plugin page as a draggable floating window over the chat stage, instead of navigating to the plugin settings route. - FrontendChatUIContribution action gains an optional "mode": "overlay" (default "navigate"), serialized as pageMode. - PluginSlot threads pageMode into the open-plugin-page target. - ChatStagePage opens overlay-mode pages in a new PluginPageOverlay instead of navigating away. - PluginPageOverlay: a plugin-neutral floating window hosting the plugin page iframe; drag by the bottom bar or, for a cooperating page, from any blank area (a postMessage drag protocol); tap the bar to collapse.
There was a problem hiding this comment.
小爱把需要留意的地方放在对应位置,方便逐项确认。
小爱已完成全部变更审查,共发布 4 条行级建议(高风险 0,中风险 2)。
合并前关注点(风险 1 · 建议测试 0)
风险
- PR 元数据没有关联 issue;基线
CONTRIBUTING.md要求每个 PR 在描述中链接既有 issue。
Prompt for AI Agents
请逐项处理以下审查问题;修改前先核实上下文,修改后运行相关测试。
1. chat-output 页面动作会静默无效 / chat-output page actions are no-ops
frontend/src/shared/plugin/PluginSlot.tsx:163 (RIGHT)
<issue_to_address>chat-output 分支未像其他槽位使用 canRun。StageLayers.tsx 仍以没有 onOpenPluginPage 的方式挂载该槽位,所以合法的 open-plugin-page 贡献仍显示可点击按钮;run() 发现没有回调后直接返回。请让 DialogLayer 传递同一导航处理器,或在没有处理器时禁用该按钮。
Original in English
The chat-output branch does not apply the canRun guard used by the other slots. StageLayers.tsx still mounts this slot without onOpenPluginPage, so a valid open-plugin-page contribution renders a clickable button, but run() finds no handler and returns immediately. Pass the same navigation handler through DialogLayer, or disable the button when no handler is available.
2. 返回 Chat 时丢失未发送输入 / Preserve unsent Chat input
frontend/src/features/chat-stage/ChatStagePage.tsx:457 (RIGHT)
<issue_to_address>这里保存的只是旧的 location.state,不包含 reducer 中的 inputDraft 或 inputAttachments。实际 AppRoutes 切换会卸载 ChatStagePage;从插件详情返回后会以空状态重挂载,而 hydrateFromSnapshot 只保留新 state 的附件,桥接快照也不保存手工输入。用户在点击插件入口前输入但未发送的文本或已暂存附件因此消失。请显式保存并恢复这些本地字段,或在离开前提示用户。
Original in English
This saves only the prior location.state, not the reducer's inputDraft or inputAttachments. The real AppRoutes switch unmounts ChatStagePage; returning from plugin details mounts it with empty state. hydrateFromSnapshot preserves attachments only from that new state, and the bridge snapshot does not retain manually typed input. Text typed but not sent, and staged attachments, are therefore lost when a user opens a plugin entry. Preserve and restore these local fields explicitly, or warn before leaving.
3. 截断后的标识无法回查 / Truncated identifiers cannot be resolved
frontend_bridge_core/plugin_ui.py:379 (RIGHT)
<issue_to_address>register_frontend_chat_ui 只要求非空 ID,但此 payload 将 page_id、contribution_id 和 plugin_id 截短。回调端按完整字符串匹配,故 129 字符的贡献或插件 ID 会被前端发送为前缀并得到 not-found;长 page ID 也只会选择默认页。要么原样传递标识,要么在 SDK 注册时用相同上限拒绝它们。
Original in English
register_frontend_chat_ui requires only non-empty IDs, but this payload truncates page_id, contribution_id, and plugin_id. The callback endpoint compares full strings, so a 129-character contribution or plugin ID is sent by the frontend as a prefix and cannot be found; a long page ID similarly falls back to the default page. Either preserve identifiers verbatim or reject them at SDK registration with the same limits.
4. 冷缓存可覆盖请求的插件页面 / Cold cache can override the requested plugin page
frontend/src/features/plugin-manager/PluginDetailPanel.tsx:230 (RIGHT)
<issue_to_address>首次进入时 fallbackPluginUiPages 提供合成 key,例如 settings-0。SDK 没有限制真实 page ID,插件可以同时声明真实的 settings-0 和目标 phone。这里先把 fallback key 设为活动页;查询完成后真实列表仍包含 settings-0,于是不会重新应用 initialPageId,最终停在错误页面。应在权威详情数据到达后优先解析请求的页面。
Original in English
On a cold query cache, fallbackPluginUiPages supplies synthetic keys such as settings-0. The SDK does not reserve real page IDs, so a plugin can legitimately declare a real settings-0 page and a requested phone page. This first selects the fallback key; when the query completes, the real list still contains settings-0, so initialPageId is never applied and the user remains on the wrong page. Resolve the requested page again once authoritative detail data arrives.
| {contribution.description ? <small>{contribution.description}</small> : null} | ||
| </span> | ||
| {contribution.actionable ? ( | ||
| <Button loading={running} onClick={() => void run()} variant={contribution.variant}> |
There was a problem hiding this comment.
[MEDIUM · bug] chat-output 页面动作会静默无效 / chat-output page actions are no-ops
chat-output 分支未像其他槽位使用 canRun。StageLayers.tsx 仍以没有 onOpenPluginPage 的方式挂载该槽位,所以合法的 open-plugin-page 贡献仍显示可点击按钮;run() 发现没有回调后直接返回。请让 DialogLayer 传递同一导航处理器,或在没有处理器时禁用该按钮。
Original in English
The chat-output branch does not apply the canRun guard used by the other slots. StageLayers.tsx still mounts this slot without onOpenPluginPage, so a valid open-plugin-page contribution renders a clickable button, but run() finds no handler and returns immediately. Pass the same navigation handler through DialogLayer, or disable the button when no handler is available.
置信度 99% · 可直接回复讨论 · 👍/👎 会影响后续审查
| hash: location.hash, | ||
| pathname: location.pathname, | ||
| search: location.search, | ||
| state: location.state, |
There was a problem hiding this comment.
[MEDIUM · correctness] 返回 Chat 时丢失未发送输入 / Preserve unsent Chat input
这里保存的只是旧的 location.state,不包含 reducer 中的 inputDraft 或 inputAttachments。实际 AppRoutes 切换会卸载 ChatStagePage;从插件详情返回后会以空状态重挂载,而 hydrateFromSnapshot 只保留新 state 的附件,桥接快照也不保存手工输入。用户在点击插件入口前输入但未发送的文本或已暂存附件因此消失。请显式保存并恢复这些本地字段,或在离开前提示用户。
Original in English
This saves only the prior location.state, not the reducer's inputDraft or inputAttachments. The real AppRoutes switch unmounts ChatStagePage; returning from plugin details mounts it with empty state. hydrateFromSnapshot preserves attachments only from that new state, and the bridge snapshot does not retain manually typed input. Text typed but not sent, and staged attachments, are therefore lost when a user opens a plugin entry. Preserve and restore these local fields explicitly, or warn before leaving.
置信度 97% · 可直接回复讨论 · 👍/👎 会影响后续审查
| "actionable": action_type != "none", | ||
| "description": str(getattr(contribution, "description", "") or "").strip()[:500], | ||
| "icon": icon if icon in allowed_icons else "puzzle", | ||
| "id": contribution_id[:128], |
There was a problem hiding this comment.
[LOW · correctness] 截断后的标识无法回查 / Truncated identifiers cannot be resolved
register_frontend_chat_ui 只要求非空 ID,但此 payload 将 page_id、contribution_id 和 plugin_id 截短。回调端按完整字符串匹配,故 129 字符的贡献或插件 ID 会被前端发送为前缀并得到 not-found;长 page ID 也只会选择默认页。要么原样传递标识,要么在 SDK 注册时用相同上限拒绝它们。
Original in English
register_frontend_chat_ui requires only non-empty IDs, but this payload truncates page_id, contribution_id, and plugin_id. The callback endpoint compares full strings, so a 129-character contribution or plugin ID is sent by the frontend as a prefix and cannot be found; a long page ID similarly falls back to the default page. Either preserve identifiers verbatim or reject them at SDK registration with the same limits.
置信度 98% · 可直接回复讨论 · 👍/👎 会影响后续审查
| const preferredPage = detailPages.find( | ||
| (page) => page.id === initialPageId || pluginUiPageKey(page) === initialPageId, | ||
| ); | ||
| setActiveDetailPageId(preferredPage ? pluginUiPageKey(preferredPage) : pageKeys[0]); |
There was a problem hiding this comment.
[LOW · bug] 冷缓存可覆盖请求的插件页面 / Cold cache can override the requested plugin page
首次进入时 fallbackPluginUiPages 提供合成 key,例如 settings-0。SDK 没有限制真实 page ID,插件可以同时声明真实的 settings-0 和目标 phone。这里先把 fallback key 设为活动页;查询完成后真实列表仍包含 settings-0,于是不会重新应用 initialPageId,最终停在错误页面。应在权威详情数据到达后优先解析请求的页面。
Original in English
On a cold query cache, fallbackPluginUiPages supplies synthetic keys such as settings-0. The SDK does not reserve real page IDs, so a plugin can legitimately declare a real settings-0 page and a requested phone page. This first selects the fallback key; when the query completes, the real list still contains settings-0, so initialPageId is never applied and the user remains on the wrong page. Resolve the requested page again once authoritative detail data arrives.
置信度 95% · 可直接回复讨论 · 👍/👎 会影响后续审查
Summary
chat-top-toolbarsloticon-onlypresentationopen-plugin-pageaction that can only target a registered page owned by the same pluginnavigate(default) and a responsive, draggable, host-ownedoverlayregister.frontend_ui()Scope
This PR provides plugin-neutral UI extension points. It does not include incoming-call events, call state, accept/reject/hang-up behavior, a phone-specific protocol, or a persistent phone iframe from #271.
Why
Plugins could add dialog actions and output cards, but they could not expose a compact entry in the actual top Chat toolbar or open richer plugin UI without leaving the active Chat context. The generic overlay mode lets any supported Chat slot—or the plugin backend itself—present a registered page over the stage without adding feature-specific events to the host.
This is sufficient for #271-style experiences: the plugin owns its domain event and business state, then asks the host to present a generic page with a JSON payload. Accept/reject or other business actions continue through the plugin's authenticated backend rather than a phone-specific host protocol.
Safety and interaction
overlayowned by the same plugin and declared in that plugin's frontend page contributionsVITE_SHINSEKAI_API_BASE, while preserving bridge authentication tokensValidation
Summary by Ameath
小爱先把这次核对的重点收拢一下,方便快速查看。
该 PR 为 React Chat 增加宿主渲染的插件槽位、顶栏图标入口与同插件页面导航;审阅发现页面往返会丢失未发送的本地输入,
chat-output的页面动作无法导航,且初始页选择和协议标识截断会使有效声明落到错误或无效目标。New Features
FrontendChatUIContribution、chat-top-toolbar、smartphone图标和 callback/页面动作的桥接协议。Enhancements
ChatStagePage的插件页路由,并在插件详情页支持初始页面选择与返回 Chat。Tests
Original summary in English
This PR adds host-rendered plugin slots, top-toolbar icon entries, and same-plugin page navigation to React Chat; the review found that a page round trip loses unsent local input,
chat-outputpage actions cannot navigate, and initial-page selection plus protocol identifier truncation can send valid declarations to an incorrect or invalid target.New Features
FrontendChatUIContribution,chat-top-toolbar, thesmartphoneicon, and bridge support for callback/page actions.Enhancements
ChatStagePageplugin-page routing, and supports an initial page plus return-to-Chat in plugin details.Tests