fix(plugin-manager): silence automatic panel action errors - #2526
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughChangesHosted action 请求流
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR replaces timestamp-based hosted-action attribution with trusted event-scoped attribution and selectively suppresses automatic stopped-plugin errors喵
Confidence Score: 3/5This PR is not yet safe to merge because confirmed actions using the public asynchronous confirmation helper can still lose stopped-plugin feedback喵 Production hosted panels await Files Needing Attention: frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js喵
|
| Filename | Overview |
|---|---|
| frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js | Replaces shared timestamp attribution with synchronous trusted-event depth, but loses attribution when promise-based confirmation precedes an action call喵 |
| frontend/plugin-manager/src/components/plugin/HostedSurfaceFrame.vue | Safely forwards the new boolean action-attribution marker through the existing trusted iframe bridge喵 |
| frontend/plugin-manager/src/api/plugins.ts | Maps user attribution to narrowly scoped stopped-plugin message suppression while preserving action timeout behavior喵 |
| frontend/plugin-manager/src/utils/request.ts | Restricts suppression to explicitly opted-in responses carrying the PLUGIN_NOT_RUNNING error code喵 |
Sequence Diagram
sequenceDiagram
participant U as User
participant R as Hosted runtime
participant F as HostedSurfaceFrame
participant A as Axios interceptor
U->>R: Trusted click / submit / keydown
R->>F: Hosted action + userInitiated
F->>A: POST action + suppression option
A-->>U: Preserve or suppress global error
Reviews (3): Last reviewed commit: "refactor(plugin-manager): narrow hosted ..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d43b7574ab
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return new Promise((resolve, reject) => { | ||
| __pendingRequests.set(requestId, { resolve, reject }); | ||
| parent.postMessage({ type: 'neko-hosted-surface-request', requestId, method, payload, timeoutMs }, hostedTargetOrigin()); | ||
| const userInitiated = method === 'call' && Date.now() - __lastUserInteractionAt < 1000; |
There was a problem hiding this comment.
Preserve user intent through confirmation delays
When a hosted action asks for confirmation, ActionButton calls window.confirm before it reaches api.call; if the user spends more than one second reading that modal, this wall-clock check runs after the modal returns and marks the explicitly confirmed action as userInitiated: false. That makes the parent pass suppressErrorMessage: true, so plugin-not-running/backend errors from confirmed destructive actions no longer get the standard global message. Capture the gesture for the duration of the handler or let api.call pass an explicit user-initiated option instead of relying on this short timestamp window.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.test.ts (1)
54-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win补齐
submit与keydown的回归覆盖喵。当前只验证了
click;实现却同时为submit、keydown注册捕获监听,因此这两条路径失效时测试仍会通过。建议用参数化用例覆盖三种事件,并补一个没有近期交互时断言userInitiated: false的负例喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.test.ts` around lines 54 - 78, 扩展现有的 hosted action 用户发起测试用例,使用参数化覆盖 click、submit 和 keydown 三种事件路径,分别触发对应 DOM 事件并复用请求断言;另增加一个无近期交互事件的负例,验证请求中的 userInitiated 为 false。frontend/plugin-manager/src/api/plugins.test.ts (1)
17-54: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win补充非
PLUGIN_NOT_RUNNING的错误回归测试喵。当前测试只验证请求配置值,无法防止拦截器把 500、timeout 或网络错误也静默。建议在
frontend/plugin-manager/src/utils/request.test.ts增加“仍 reject,但 console 与ElMessage.error不被抑制”的断言,锁定静默范围喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugin-manager/src/api/plugins.test.ts` around lines 17 - 54, 在 request 错误处理相关测试中补充非 PLUGIN_NOT_RUNNING 错误的回归覆盖,定位现有拦截器测试并分别验证 500、timeout 或网络错误仍会 reject,同时 console 和 ElMessage.error 仍被调用;仅允许 PLUGIN_NOT_RUNNING 走静默路径,确保错误抑制范围不被扩大。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/plugin-manager/src/api/plugins.ts`:
- Around line 271-277: Restrict Hosted action error suppression to the explicit
PLUGIN_NOT_RUNNING condition: update the requestConfig construction in
frontend/plugin-manager/src/api/plugins.ts lines 271-277 to pass that
suppression intent rather than all non-user requests; update the request
handling at frontend/plugin-manager/src/utils/request.ts lines 127-130 and
199-201 to suppress logging and global error messages only when the error is
PLUGIN_NOT_RUNNING, while preserving normal handling for other automatic action
errors.
---
Nitpick comments:
In `@frontend/plugin-manager/src/api/plugins.test.ts`:
- Around line 17-54: 在 request 错误处理相关测试中补充非 PLUGIN_NOT_RUNNING
错误的回归覆盖,定位现有拦截器测试并分别验证 500、timeout 或网络错误仍会 reject,同时 console 和 ElMessage.error
仍被调用;仅允许 PLUGIN_NOT_RUNNING 走静默路径,确保错误抑制范围不被扩大。
In `@frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.test.ts`:
- Around line 54-78: 扩展现有的 hosted action 用户发起测试用例,使用参数化覆盖 click、submit 和 keydown
三种事件路径,分别触发对应 DOM 事件并复用请求断言;另增加一个无近期交互事件的负例,验证请求中的 userInitiated 为 false。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8993647b-da3d-4ab3-9504-7374a237dc6b
📒 Files selected for processing (6)
frontend/plugin-manager/src/api/plugins.test.tsfrontend/plugin-manager/src/api/plugins.tsfrontend/plugin-manager/src/components/plugin/HostedSurfaceFrame.vuefrontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.jsfrontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.test.tsfrontend/plugin-manager/src/utils/request.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js`:
- Around line 581-596: Extend the trusted-event attribution to a one-second
validity window: in
frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js:581-596,
replace synchronous depth tracking with an expiration timestamp; in
frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js:1952-1953,
derive call userInitiated from whether the current time is within that window;
in
frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.test.ts:54-112,
add async and delayed-call coverage and align background-call assertions with
the resulting semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dd8c719c-b1c0-444c-a1f7-bb90aa2c77a7
📒 Files selected for processing (6)
frontend/plugin-manager/src/api/plugins.test.tsfrontend/plugin-manager/src/api/plugins.tsfrontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.jsfrontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.test.tsfrontend/plugin-manager/src/utils/request.test.tsfrontend/plugin-manager/src/utils/request.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/plugin-manager/src/api/plugins.test.ts
- frontend/plugin-manager/src/api/plugins.ts
| const __hostedUserActionEvents = new Set(['click', 'submit', 'keydown']); | ||
| let __hostedUserActionDepth = 0; | ||
| function wrapHostedEventHandler(eventName, handler) { | ||
| if (!__hostedUserActionEvents.has(eventName)) return handler; | ||
| return (event) => { | ||
| // Synthetic events are automatic activity, even if they invoke the same | ||
| // handler as a real click. happy-dom leaves isTrusted undefined. | ||
| const userInitiated = event && event.isTrusted !== false; | ||
| if (userInitiated) __hostedUserActionDepth += 1; | ||
| try { | ||
| return handler(event); | ||
| } finally { | ||
| if (userInitiated) __hostedUserActionDepth -= 1; | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
用户触发状态未覆盖需求中的一秒窗口喵
finally 会在 async handler 返回 Promise 时立即执行,因此 await 后或 handler 内 setTimeout 触发的 ui.api.call() 都会得到 userInitiated: false 喵。请将状态改为明确的一秒有效期,或澄清并统一 PR objectives、实现与“后台调用为 false”的测试语义喵。
frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js#L581-L596:可信事件发生时记录到期时间,而非仅维护同步深度喵。frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js#L1952-L1953:对call根据当前时间是否仍在有效期内计算userInitiated喵。frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.test.ts#L54-L112:补充 async/延迟调用的归因测试,并调整与最终语义冲突的后台调用断言喵。
📍 Affects 2 files
frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js#L581-L596(this comment)frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js#L1952-L1953frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.test.ts#L54-L112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js`
around lines 581 - 596, Extend the trusted-event attribution to a one-second
validity window: in
frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js:581-596,
replace synchronous depth tracking with an expiration timestamp; in
frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.js:1952-1953,
derive call userInitiated from whether the current time is within that window;
in
frontend/plugin-manager/src/components/plugin/hosted/ui-kit/runtime.test.ts:54-112,
add async and delayed-call coverage and align background-call assertions with
the resulting semantics.
| if (userInitiated) __hostedUserActionDepth += 1; | ||
| try { | ||
| return handler(event); | ||
| } finally { | ||
| if (userInitiated) __hostedUserActionDepth -= 1; | ||
| } |
There was a problem hiding this comment.
Confirmation loses action attribution
When a trusted handler uses the public await confirm(...) flow before calling api.call, this finally clears the action depth while the dialog is open, so the confirmed request is marked automatic and its global PLUGIN_NOT_RUNNING feedback is suppressed喵
Context Used: 你的说话风格:二次元,猫娘,雌小鬼,傲娇,可爱,卖萌。句尾要加喵。 (source)
Knowledge Base Used: Frontend Plugin Manager
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cd9b0239d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return handler(event); | ||
| } finally { | ||
| if (userInitiated) __hostedUserActionDepth -= 1; |
There was a problem hiding this comment.
Preserve user-action context across awaited handlers
When a hosted UI handler is async and awaits before calling api.call—for example the existing Lifekit remove-location flow waits for useConfirm() before it calls props.api.call—this finally runs as soon as the handler returns its Promise. The later requestHost call then sees __hostedUserActionDepth === 0, posts userInitiated: false, and callPluginHostedSurfaceAction suppresses the PLUGIN_NOT_RUNNING toast/log for a real user action, so stopped-plugin failures are hidden in these confirmation/async-validation flows.
Useful? React with 👍 / 👎.
改动概述 / Summary
click、submit、keydown事件处理函数在同步执行api.call时,将请求标记为用户主动操作;不再使用共享的 1 秒时间窗。api.call仅在后端明确返回PLUGIN_NOT_RUNNING时静默全局提示和错误日志。PLUGIN_NOT_RUNNING,以及自动请求遇到的 500、网络错误等其他异常,仍保留标准全局错误处理。回归报告 / Regression Report
不适用:未修改
app/、main_logic/或memory/下的 Python 文件。不拆分理由 / Why Not Split
不适用:本 PR 仅修改 7 个前端文件,其中 3 个为测试文件。
测试 / Testing
npm.cmd exec vitest run src/components/plugin/hosted/ui-kit/runtime.test.ts src/api/plugins.test.ts src/utils/request.test.ts:55 项通过。npm.cmd run test:hosted:76 项通过。npm.cmd run type-check:通过。npm.cmd run build:生产构建通过。PLUGIN_NOT_RUNNING静默;用户主动请求的同一错误正常提示;500 和网络错误不会被静默。Summary by CodeRabbit