fix(agent): avoid global trace scans#1946
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (24)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (20)
📝 WalkthroughWalkthroughThe PR revises agent extension policies to use skills and MCP servers instead of per-agent plugin IDs, adds ACP-specific Plugins Hub availability behavior, centralizes plugin and remote state, removes trace aggregation from runtime history reads, and updates related contracts, UI, localization, and tests. ChangesAgent extension and runtime behavior
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant AgentStore
participant PluginsHubPage
participant PluginCatalogStore
participant PluginClient
AgentStore->>PluginsHubPage: selected agent changes
PluginsHubPage->>PluginsHubPage: determine ACP availability
PluginsHubPage->>PluginCatalogStore: use shared catalog state
PluginCatalogStore->>PluginClient: load plugins and remote status
PluginClient-->>PluginCatalogStore: catalog and status snapshot
PluginCatalogStore-->>PluginsHubPage: current plugin and remote state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/renderer/components/PluginsCatalogPage.test.ts (1)
188-201: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the
console.warncall, not just its suppression.The spy is created and restored but never asserted against, so this test doesn't verify that a warning is actually logged on IPC failure — only that the catalog is retained. Compare with the equivalent test in
WindowSideBar.test.ts(lines 1893-1896), which assertswarnwas called with a specific message.✏️ Suggested addition
expect(wrapper.findAll('article')).toHaveLength(2) + expect(warn).toHaveBeenCalled() warn.mockRestore()🤖 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 `@test/renderer/components/PluginsCatalogPage.test.ts` around lines 188 - 201, Update the test case “keeps the current remote catalog when the IPC refresh fails” to assert that the console.warn spy is called with the expected IPC refresh failure message after loadCatalog rejects. Keep the existing catalog-retention assertions and cleanup unchanged.
🤖 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 `@src/renderer/src/pages/plugins/OfficialPluginDetailPage.vue`:
- Around line 185-188: Update the runtime details row in
OfficialPluginDetailPage so its <dt> uses a dedicated translation label for the
runtime state/status instead of settings.plugins.runtime. Keep the section
header unchanged and add or reuse the appropriate settings.plugins state/status
translation key.
---
Nitpick comments:
In `@test/renderer/components/PluginsCatalogPage.test.ts`:
- Around line 188-201: Update the test case “keeps the current remote catalog
when the IPC refresh fails” to assert that the console.warn spy is called with
the expected IPC refresh failure message after loadCatalog rejects. Keep the
existing catalog-retention assertions and cleanup unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6f6d9fb6-b2d6-4894-8873-b2c59684fc04
📒 Files selected for processing (76)
docs/architecture/agent-scoped-extensions/plan.mddocs/architecture/agent-scoped-extensions/spec.mddocs/architecture/agent-scoped-extensions/tasks.mddocs/architecture/plugin-availability-boundary/spec.mddocs/features/plugins-hub/plan.mddocs/features/plugins-hub/spec.mddocs/features/plugins-hub/tasks.mddocs/issues/runtime-history-global-trace-aggregation/spec.mdsrc/main/presenter/agentRepository/index.tssrc/main/presenter/agentRuntimePresenter/dispatch.tssrc/main/presenter/agentRuntimePresenter/index.tssrc/main/presenter/agentRuntimePresenter/types.tssrc/main/presenter/mcpPresenter/index.tssrc/main/presenter/mcpPresenter/toolManager.tssrc/main/presenter/remoteControlPresenter/index.tssrc/main/presenter/skillPresenter/skillTools.tssrc/main/presenter/sqlitePresenter/tables/deepchatMessages.tssrc/main/presenter/toolPresenter/agentTools/agentToolManager.tssrc/main/presenter/toolPresenter/index.tssrc/renderer/settings/components/CronJobsSettings.vuesrc/renderer/settings/components/RemoteSettings.vuesrc/renderer/src/components/WindowSideBar.vuesrc/renderer/src/components/trace/TraceDialog.vuesrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/de-DE/settings.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/es-ES/settings.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/id-ID/settings.jsonsrc/renderer/src/i18n/it-IT/settings.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/ms-MY/settings.jsonsrc/renderer/src/i18n/pl-PL/settings.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/tr-TR/settings.jsonsrc/renderer/src/i18n/vi-VN/settings.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/renderer/src/pages/plugins/AgentExtensionPolicyPanel.vuesrc/renderer/src/pages/plugins/OfficialPluginDetailPage.vuesrc/renderer/src/pages/plugins/PluginsCatalogPage.vuesrc/renderer/src/pages/plugins/PluginsHubPage.vuesrc/renderer/src/stores/pluginCatalog.tssrc/shared/contracts/domainSchemas.tssrc/shared/types/agent-interface.d.tssrc/shared/types/presenters/core.presenter.d.tssrc/shared/types/presenters/remote-control.presenter.d.tssrc/shared/types/presenters/tool.presenter.d.tssrc/types/i18n.d.tstest/e2e/specs/11-remote-control-readonly-route.smoke.spec.tstest/main/performance/memory/messageHistoryTraceScale.perf.tstest/main/presenter/agentRepository.test.tstest/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.tstest/main/presenter/agentRuntimePresenter/messageStore.test.tstest/main/presenter/mcpPresenter.test.tstest/main/presenter/mcpPresenter/toolManager.test.tstest/main/presenter/remoteControlPresenter/remoteControlPresenter.test.tstest/main/presenter/skillPresenter/skillTools.test.tstest/main/presenter/sqlitePresenter/deepchatMessagesTable.test.tstest/main/presenter/toolPresenter/toolPresenter.test.tstest/main/routes/dispatcher.test.tstest/renderer/api/clients.test.tstest/renderer/components/AgentExtensionPolicyPanel.test.tstest/renderer/components/OfficialPluginDetailPage.test.tstest/renderer/components/PluginPageWrappers.test.tstest/renderer/components/PluginsCatalogPage.test.tstest/renderer/components/PluginsHubPage.test.tstest/renderer/components/RemoteSettings.test.tstest/renderer/components/WindowSideBar.test.tstest/renderer/components/trace/TraceDialog.test.tstest/renderer/stores/pluginCatalogStore.test.ts
💤 Files with no reviewable changes (13)
- docs/features/plugins-hub/plan.md
- docs/architecture/agent-scoped-extensions/tasks.md
- src/main/presenter/agentRuntimePresenter/types.ts
- src/shared/types/presenters/tool.presenter.d.ts
- docs/features/plugins-hub/tasks.md
- docs/architecture/agent-scoped-extensions/plan.md
- src/renderer/src/pages/plugins/AgentExtensionPolicyPanel.vue
- test/renderer/components/AgentExtensionPolicyPanel.test.ts
- test/renderer/components/PluginPageWrappers.test.ts
- src/shared/types/agent-interface.d.ts
- src/shared/contracts/domainSchemas.ts
- src/main/presenter/agentRepository/index.ts
- src/shared/types/presenters/core.presenter.d.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Runtime history and Trace diagnostics
Plugins, Remote channels, and agent availability
RemoteControlPresenterthe only static remote channel catalog authority and derivesupportsCronDeliveryfromREMOTE_DELIVERY_CHANNELSsupportsNotificationsmetadata and the three renderer-owned fallback catalogsenabledPluginIdscontract and generic plugin-scope panel while retaining DeepChat agent-scoped Skills and normal MCP policiesRoot cause
Runtime history
DeepChatMessagesTable.getBySession()joined a subquery that grouped the entiredeepchat_message_tracestable before filtering messages to the requested session. Runtime context, predicates, compaction, and Tape paths therefore paid for UI/debug metadata whose cost grew with global trace retention.Plugins and Remote channels
The remote channel descriptor had one main-process definition and three renderer fallback copies. They had already drifted, and the unused
supportsNotificationsflag no longer described Cron delivery capability. On IPC failure, the fallbacks also converted a contract failure into a fake complete catalog.enabledPluginIdsexposed a per-agent plugin policy even though plugin installation, process lifecycle, and status remained global. It filtered only some plugin-owned MCP and Skill contributions, so the UI promised a scope boundary the runtime did not implement. ACP agents use an external runtime, so DeepChat plugin controls cannot affect them.Impact
Runtime history now reads only
deepchat_messagesthrough the existing session index. UI history continues to expose accurate trace counts per paged message, while full trace payloads remain loaded only by explicit diagnostics. No schema migration, new index, preload contract, or IPC change is required.Remote descriptors now come from main only. Renderer state is a cache of successful main-process data, not a second static catalog. Failed refreshes keep the current snapshot, and optimistic plugin/remote state changes roll back on failure. Cron delivery options use the main-derived capability flag, so Telegram, Feishu, Discord, and Weixin remain eligible while QQBot is excluded.
RemoteControlPresenter.listRemoteChannels()supportsCronDelivery, derived fromREMOTE_DELIVERY_CHANNELSUI structure
Before:
After:
The Trace dialog now uses the existing accessible Select primitive. Its compact summary is fixed to two rows:
Integrity warnings remain on a separate line when present.
Validation
Runtime-history validation
pnpm run formatpnpm run i18npnpm run lintpnpm run typecheck:nodepnpm run typecheck:webPlugin/Remote validation
pnpm run formatpnpm run i18npnpm run lintpnpm run typecheckgit diff --checkObserved warmed median
getBySession()latency for a fixed 1,000-message session:Closes #1945
Summary by CodeRabbit
New Features
Bug Fixes
Documentation