feat(plugins): add main window plugins hub#1818
Conversation
|
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 (9)
💤 Files with no reviewable changes (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughIntroduces a ChangesPlugins Hub and Skills Management
Sequence Diagram(s)sequenceDiagram
participant User
participant WindowSideBar
participant PluginsHubPage
participant PluginsCatalogPage
participant OfficialPluginDetailPage
participant PluginClient
participant RemoteControlClient
User->>WindowSideBar: click Plugins command
WindowSideBar->>PluginsHubPage: router.push({ name: 'plugins' })
PluginsHubPage->>PluginsCatalogPage: render index child route
PluginsCatalogPage->>PluginClient: listPlugins()
PluginsCatalogPage->>RemoteControlClient: listChannels() + getChannelStatus() per channel
PluginsCatalogPage-->>User: sorted catalog (enabled first)
User->>PluginsCatalogPage: click item
PluginsCatalogPage->>OfficialPluginDetailPage: navigate /plugins/:pluginId
OfficialPluginDetailPage->>PluginClient: getPlugin(pluginId) [if local plugin]
OfficialPluginDetailPage->>RemoteControlClient: getChannelStatus [if remote:]
User->>OfficialPluginDetailPage: click Enable/Disable
OfficialPluginDetailPage->>PluginClient: enablePlugin / disablePlugin
OfficialPluginDetailPage->>RemoteControlClient: saveChannelSettings [if Feishu]
sequenceDiagram
participant UI
participant SkillSyncClient
participant SkillSyncPresenter
participant SkillPresenter
participant Filesystem
UI->>SkillSyncClient: previewAdoptAgentSkill({ agentId, skillName })
SkillSyncClient->>SkillSyncPresenter: previewAdoptAgentSkill
SkillSyncPresenter->>Filesystem: resolveAdoptionSource + readAdoptableSkill(SKILL.md)
SkillSyncPresenter-->>UI: AdoptAgentSkillPreview (conflict, targetName, warnings)
UI->>SkillSyncClient: executeAdoptAgentSkill(input)
SkillSyncClient->>SkillSyncPresenter: executeAdoptAgentSkill
SkillSyncPresenter->>Filesystem: prepareAdoptionTemp → rename to target → backup agent dir → createDirectoryLink
SkillSyncPresenter->>SkillPresenter: registerAdoptedSkill (writes skills.managementState)
SkillSyncPresenter-->>UI: AdoptAgentSkillResult { success, skillName }
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (27)
src/main/presenter/skillPresenter/index.ts-2409-2413 (1)
2409-2413: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftClean up DeepChat-created agent links before deleting management state.
Deleting the management item also drops
agentLinks, so any symlinks created in external agent directories remain behind but can no longer be identified as DeepChat-owned for repair/removal.🤖 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 `@src/main/presenter/skillPresenter/index.ts` around lines 2409 - 2413, The cleanup path in cleanupUninstalledSkillState currently deletes the skill management item before removing DeepChat-created agent links, which leaves external symlinks orphaned. Update this method so it first identifies and cleans up any DeepChat-owned agent links associated with the skill name, then deletes the management state via deleteSkillManagementItem. Keep the change scoped to the cleanupUninstalledSkillState flow and use the existing isSafeSkillName guard plus the agent-link management helpers already used in SkillPresenter.src/main/presenter/skillPresenter/index.ts-1893-1904 (1)
1893-1904: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRestore the backup if replacement install fails after moving the old skill.
Overwrite/replacement moves the existing skill to backup before
copyDirectory, manifest rewrite, parsing, and state updates. If any later step fails, the old skill is left only in backup and the target can be partially written.🤖 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 `@src/main/presenter/skillPresenter/index.ts` around lines 1893 - 1904, The replacement install flow in skillPresenter’s install path moves the existing skill aside before copyDirectory and subsequent manifest/parsing/state updates, but it does not roll back if a later step fails. Update the replace/overwrite branch around prepareExistingSkillTargetForInstall, copyDirectory, and the later rewrite/parsing/cache updates so any failure restores the backup back into place and leaves the original skill intact, rather than only existing in backup or a partially written target.src/main/presenter/skillSyncPresenter/index.ts-899-919 (1)
899-919: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRollback adoption if management-state registration fails.
After the agent directory is moved and the symlink is created,
registerAdoptedSkillcan still throw. The catch block only removestempPath, leaving a DeepChat skill folder, an agent symlink, and a backup without management state.🤖 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 `@src/main/presenter/skillSyncPresenter/index.ts` around lines 899 - 919, Rollback the adoption flow in skillSyncPresenter when `registerAdoptedSkill` fails, not just when the move/link step fails. Update the adoption logic around `createDirectoryLink`, `pathExists`, and `skillPresenter.registerAdoptedSkill` so that any exception after the agent directory is moved and the symlink is created restores `preview.agentPath` from `backupPath` and removes `preview.targetPath` if it was created. Make sure the cleanup happens for failures thrown by `registerAdoptedSkill` as well, leaving no DeepChat skill folder, symlink, or backup behind without management state.src/main/presenter/skillPresenter/index.ts-1489-1497 (1)
1489-1497: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject empty sync-directory input before resolving it.
path.resolve(input.skillsDirectory.trim())turns an empty string into the process CWD, so saving a blank value can create/use./skillsunexpectedly.🐛 Proposed fix
- const skillsDirectory = path.resolve(input.skillsDirectory.trim()) + const trimmedSkillsDirectory = input.skillsDirectory.trim() + if (!trimmedSkillsDirectory) { + throw new Error('Skills sync directory is required') + } + const skillsDirectory = path.resolve(trimmedSkillsDirectory)🤖 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 `@src/main/presenter/skillPresenter/index.ts` around lines 1489 - 1497, Reject empty sync-directory input before calling path.resolve in the skill sync setup. In the presenter logic that builds skillsDirectory and SkillSyncDirectoryConfig, validate input.skillsDirectory.trim() first and return an error or skip saving when it is blank, so an empty value cannot fall through to the process CWD. Keep the existing path.resolve and fs.mkdirSync behavior only for non-empty, user-provided directories.src/main/presenter/skillSyncPresenter/index.ts-1041-1047 (1)
1041-1047: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove the symlink if link registration fails.
createDirectoryLinksucceeds beforeregisterAgentSkillLink; if state persistence fails, the external agent gets a DeepChat symlink that is not marked as DeepChat-owned and cannot be removed through the new link-management flow.🐛 Proposed fix
try { await this.createDirectoryLink(item.sourcePath, item.targetPath) - await this.skillPresenter.registerAgentSkillLink({ - skillName: item.skillName, - agentId: input.agentId, - agentPath: item.targetPath - }) + try { + await this.skillPresenter.registerAgentSkillLink({ + skillName: item.skillName, + agentId: input.agentId, + agentPath: item.targetPath + }) + } catch (error) { + await fs.promises.rm(item.targetPath, { recursive: true, force: true }).catch(() => undefined) + throw error + } result.linked += 1🤖 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 `@src/main/presenter/skillSyncPresenter/index.ts` around lines 1041 - 1047, The symlink creation in skillSyncPresenter’s flow succeeds before registerAgentSkillLink, so if registration fails the link is left behind without DeepChat ownership metadata. Update the try/catch around createDirectoryLink and skillPresenter.registerAgentSkillLink to clean up by removing the newly created directory link when registration throws, using the existing link-management/remove helper associated with createDirectoryLink so the operation is fully rolled back.src/main/presenter/skillPresenter/index.ts-2149-2149 (1)
2149-2149: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winGuard
git cloneagainst option-like repo URLs.
repoUrlcomes directly from user input insrc/main/presenter/skillPresenter/index.ts:2149, so a value starting with-can still be parsed bygit cloneas another option. Add--before the repository argument.🛡️ Proposed fix
- await execFileAsync('git', ['clone', '--depth', '1', repoUrl, cloneDir], { + await execFileAsync('git', ['clone', '--depth', '1', '--', repoUrl, cloneDir], {🤖 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 `@src/main/presenter/skillPresenter/index.ts` at line 2149, The git clone call in skillPresenter’s repository setup is passing user-controlled repoUrl directly as an argument, so option-like values can be interpreted by git. Update the execFileAsync invocation in the clone flow to insert a `--` separator before repoUrl so it is always treated as the repository target, and keep the rest of the clone arguments unchanged.src/main/presenter/skillPresenter/index.ts-1446-1450 (1)
1446-1450: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
path.dirname()for multi-skill Git source paths.
relativePathis built withpath.join(...), so on Windows it contains\separators and the/SKILL\.md$/regex will not strip the manifest filename. Multi-skill Git installs then pass aSKILL.mdfile path as the source directory.🐛 Proposed fix
const sourceDir = scan.repoFormat === 'single-skill' ? cloneDir - : path.join(cloneDir, item.relativePath.replace(/\/SKILL\.md$/, '')) + : path.join(cloneDir, path.dirname(item.relativePath))🤖 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 `@src/main/presenter/skillPresenter/index.ts` around lines 1446 - 1450, The multi-skill Git source path handling in the skill install flow is stripping the manifest with a POSIX-only regex, so Windows paths built via path.join() keep SKILL.md in the source directory. Update the sourceDir logic in skillPresenter/index.ts to use path.dirname() for the multi-skill case (while keeping single-skill behavior unchanged) so the directory is derived correctly regardless of path separator. Reference the sourceDir calculation that uses scan.repoFormat, cloneDir, and item.relativePath to locate the fix.src/renderer/src/i18n/de-DE/chat.json-364-366 (1)
364-366: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTranslate the new sidebar labels in the German locale.
These three entries are still English, so the updated sidebar will render partly untranslated for
de-DE.🤖 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 `@src/renderer/src/i18n/de-DE/chat.json` around lines 364 - 366, The new sidebar labels in the de-DE locale are still using English strings, so update the translation entries for the chat sidebar labels in the German locale file. Locate the sidebar label keys in the de-DE chat localization object, and replace the values for searchCommand, chatSection, and workspace with proper German translations so the updated sidebar renders fully localized.src/renderer/src/i18n/da-DK/chat.json-348-350 (1)
348-350: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLocalize the new sidebar labels for the Danish locale.
searchCommand,chatSection, andworkspaceare still English here, so the new sidebar section ships with mixed-language UI forda-DK.🤖 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 `@src/renderer/src/i18n/da-DK/chat.json` around lines 348 - 350, The Danish locale strings for the sidebar are still in English, so update the `searchCommand`, `chatSection`, and `workspace` entries in the `da-DK/chat.json` translation file to proper Danish text. Keep the keys unchanged and only replace the values so the new sidebar labels are fully localized for the `chatSection` and related sidebar UI.src/renderer/src/i18n/ru-RU/chat.json-348-350 (1)
348-350: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTranslate the new sidebar labels in
ru-RU.
searchCommand,chatSection, andworkspaceare still English, so the sidebar update is only partially localized for Russian users.🤖 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 `@src/renderer/src/i18n/ru-RU/chat.json` around lines 348 - 350, The new sidebar labels in the ru-RU translation file are still in English, so localize the `searchCommand`, `chatSection`, and `workspace` entries to Russian in the JSON resource used by the sidebar. Update the corresponding keys in the `ru-RU` locale so the translated labels match the rest of the Russian UI.src/renderer/src/i18n/pt-BR/settings.json-1824-1824 (1)
1824-1824: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLocalize the new
pt-BRstrings before shipping this feature.A lot of the added copy is still English (
envWarning, card actions, enable/disable toasts, most of the agents workflow, and severalpluginsHublabels). That leaves the new Plugins/Skills UI mixed-language for Brazilian Portuguese users.Also applies to: 1959-2166, 2750-2759
🤖 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 `@src/renderer/src/i18n/pt-BR/settings.json` at line 1824, The new pt-BR settings copy is still partially in English, leaving the Plugins/Skills UI mixed-language for Brazilian Portuguese users. Translate the remaining strings in the pt-BR localization file, including envWarning, the card action labels, enable/disable toast messages, the agents workflow text, and the pluginsHub labels, and verify the referenced settings sections all use localized text consistently.src/renderer/src/i18n/ru-RU/settings.json-1824-1824 (1)
1824-1824: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winComplete the Russian localization for the new Skills and Plugins Hub flows.
This block still contains a lot of English UI copy (
envWarning, enable/disable messages, agents/adoption/sync dialogs, andpluginsHub.subtitle/manage/pluginNotFound/capabilities/actionResult). The new feature will otherwise ship half-translated inru-RU.Also applies to: 1959-2166, 2750-2759
🤖 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 `@src/renderer/src/i18n/ru-RU/settings.json` at line 1824, The ru-RU settings localization is incomplete for the new Skills and Plugins Hub flows, leaving several UI strings in English. Update the affected entries in settings.json for the Skills/Plugins Hub-related keys such as envWarning, enable/disable messages, agents/adoption/sync dialogs, and pluginsHub.subtitle/manage/pluginNotFound/capabilities/actionResult so they are fully translated into Russian and consistent with the existing ru-RU tone.src/renderer/src/i18n/da-DK/settings.json-1757-1757 (1)
1757-1757: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFinish localizing the new Skills and Plugins Hub copy in
da-DK.A large part of the newly added surface is still English here (
envWarning, enable/disable toasts, tabs/agents flows, andpluginsHub.subtitle/manage/pluginNotFound/capabilities/actionResult). Danish users will get a mixed-language experience across the new feature.Also applies to: 1892-2099, 2750-2759
🤖 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 `@src/renderer/src/i18n/da-DK/settings.json` at line 1757, The new Skills and Plugins Hub copy in settings.json is still partially English, causing mixed-language UI for da-DK users. Localize the remaining strings in the Skills/Plugins Hub section, including envWarning, the enable/disable toast messages, the tabs/agents flow copy, and pluginsHub.subtitle/manage/pluginNotFound/capabilities/actionResult. Use the surrounding translated keys in this file as a guide and keep the terminology consistent across the related settings entries.src/renderer/src/i18n/de-DE/settings.json-1928-1945 (1)
1928-1945: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftComplete the German localization for the new Skills/Plugins copy.
Large parts of the added
de-DEstrings are still English (Enabled,Library,Refresh,Plugin not found., the env warning, most agent dialogs, etc.), so German users will get a mixed-language UI throughout the new flows. Please either translate these entries before merge or intentionally rely on theen-USfallback instead of shipping partial manual translations.Also applies to: 1967-1967, 2077-2180, 2741-2749
🤖 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 `@src/renderer/src/i18n/de-DE/settings.json` around lines 1928 - 1945, The new Skills/Plugins German locale is only partially translated, leaving many added `de-DE` keys in English and causing a mixed-language UI. Update the missing copy in the `settings.json` locale entries for the Skills/Plugins flows, including the `enable`/`disable` blocks and the related agent/dialog strings referenced in this diff, or remove the partial manual translations so the app cleanly falls back to `en-US`. Make sure the translated values are consistent across the newly added settings, plugin, and agent-related messages so the full workflow appears in German.src/renderer/src/i18n/tr-TR/settings.json-1928-1945 (1)
1928-1945: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftFinish the Turkish localization before shipping these flows.
A large share of the new
tr-TRcopy is still English (Enabled,Library,Agents,Refresh,Plugin not found., the env warning, most agent actions/statuses, etc.), which leaves the new Skills/Plugins UI noticeably mixed-language. Please translate these keys now or fall back toen-USfor the unfinished entries.Also applies to: 1967-1967, 2077-2180, 2741-2749
🤖 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 `@src/renderer/src/i18n/tr-TR/settings.json` around lines 1928 - 1945, The Turkish locale in settings.json is still mixed with English for several Skills/Plugins and agent-related labels, so finish translating the remaining tr-TR entries or route them to en-US defaults. Update the affected string groups around the existing keys such as enabled/disabled, library/agents/refresh, plugin error/status messages, and the agent action/status sections so the UI is consistently localized. Use the surrounding translation blocks in this file to locate and complete the unfinished copy.src/renderer/src/stores/ui/spotlight.ts-254-256 (1)
254-256: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep Plugins and Skills reachable from Spotlight.
This blanket
hiddenInSidebarfilter dropssettings-pluginsandsettings-skillsfrom search results, but the new explicit action handling in this store only replaces MCP and Remote. After this change, Spotlight can no longer navigate to the Plugins hub or Skills page.Suggested fix
const buildSettingMatches = (normalizedQuery: string): SpotlightItem[] => SETTINGS_NAVIGATION_ITEMS.filter( - (item) => item.routeName !== 'settings-provider' && !item.hiddenInSidebar + (item) => + item.routeName !== 'settings-provider' && + (!item.hiddenInSidebar || + item.routeName === 'settings-plugins' || + item.routeName === 'settings-skills') )🤖 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 `@src/renderer/src/stores/ui/spotlight.ts` around lines 254 - 256, The Spotlight settings item filter in the store is too broad and removes Plugins and Skills from search. Update the filtering logic around SETTINGS_NAVIGATION_ITEMS so it still excludes the intended sidebar-hidden entries but preserves settings-plugins and settings-skills, since the explicit action handling in this store only replaces MCP and Remote. Use the existing routeName checks in this area to narrow the filter instead of applying hiddenInSidebar universally.src/renderer/src/pages/plugins/OfficialPluginDetailPage.vue-241-248 (1)
241-248: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRefresh the embedded Feishu settings after the page-level toggle.
enablePlugin/disablePluginalso updatefeishu.remoteEnabled, but this embeddedRemoteSettingsinstance never reloads afterwards. The inner Feishu controls will keep reading the oldfeishuSettings.remoteEnabledvalue until the user leaves the page.Suggested fix
<RemoteSettings v-if="isFeishuPlugin" + :key="`feishu:${remoteSettingsVersion}`" channel="feishu" embedded hide-channel-toggle hide-header single-channel />void runPluginAction(async () => { const result = await pluginClient.enablePlugin(currentPlugin.id) if (result.ok && currentPlugin.id === FEISHU_PLUGIN_ID) { await setFeishuRemoteEnabled(true) + remoteSettingsVersion.value += 1 } return result }) } @@ void runPluginAction(async () => { const result = await pluginClient.disablePlugin(currentPlugin.id) if (result.ok && currentPlugin.id === FEISHU_PLUGIN_ID) { await setFeishuRemoteEnabled(false) + remoteSettingsVersion.value += 1 } return result }) }Also applies to: 495-510
🤖 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 `@src/renderer/src/pages/plugins/OfficialPluginDetailPage.vue` around lines 241 - 248, The embedded Feishu RemoteSettings view is not reloaded after the page-level enablePlugin/disablePlugin toggle updates feishu.remoteEnabled, so the inner controls keep stale state. Update OfficialPluginDetailPage to make the embedded RemoteSettings react to the remoteEnabled change by forcing a refresh/remount or passing the changing value as a reactive key/prop, using the existing isFeishuPlugin, enablePlugin, disablePlugin, and feishuSettings references so the embedded settings re-read the latest state immediately.src/renderer/src/components/chat-input/McpIndicator.vue-611-613 (1)
611-613: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the settings fallback for routers without the hub route.
Line 612 assumes
plugins-mcpis always registered. The other new entry points in this PR guardplugins-*redirects withrouter.hasRoute(...), so this button is now the path that breaks when the component is mounted under a router without that route. Keep the oldsettings-mcpfallback here.Proposed fix
+import { createSettingsClient } from '`@api/SettingsClient`' ... const { t } = useI18n() const router = useRouter() +const settingsClient = createSettingsClient() ... const openSettings = async () => { - await router.push({ name: 'plugins-mcp' }) + if (router.hasRoute('plugins-mcp')) { + await router.push({ name: 'plugins-mcp' }) + } else { + await settingsClient.openSettings({ routeName: 'settings-mcp' }) + } panelOpen.value = 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 `@src/renderer/src/components/chat-input/McpIndicator.vue` around lines 611 - 613, The openSettings handler in McpIndicator.vue should not assume the hub route is always registered; it needs the same route fallback used elsewhere in this PR. Update openSettings to check router.hasRoute for plugins-mcp and navigate to settings-mcp when that route is unavailable, while still closing panelOpen afterward. Use the openSettings function and the router.push call as the main place to apply the fallback logic.src/renderer/src/components/chat-input/SkillsIndicator.vue-79-82 (1)
79-82: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuard
plugins-skillsnavigation and keep the legacy fallback.Line 80 has the same missing-route assumption as the MCP indicator. Other updated entry points in this PR check
router.hasRoute(...)before redirecting to plugin pages, but this one dropped the old settings fallback entirely, so the settings button stops working anywhereplugins-skillsis not registered.Proposed fix
+import { createSettingsClient } from '`@api/SettingsClient`' ... const { t } = useI18n() const router = useRouter() +const settingsClient = createSettingsClient() ... -const openSettings = () => { - void router.push({ name: 'plugins-skills' }) +const openSettings = async () => { + if (router.hasRoute('plugins-skills')) { + await router.push({ name: 'plugins-skills' }) + } else { + await settingsClient.openSettings({ routeName: 'settings-skills' }) + } panelOpen.value = 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 `@src/renderer/src/components/chat-input/SkillsIndicator.vue` around lines 79 - 82, The openSettings navigation in SkillsIndicator.vue assumes the plugins-skills route always exists, which removes the legacy settings fallback. Update openSettings to first check router.hasRoute for the plugins-skills route and only push there when present; otherwise preserve the old settings destination used before this change. Keep panelOpen.value reset behavior unchanged.src/renderer/settings/components/skills/SkillDetailDialog.vue-255-289 (1)
255-289: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve unknown frontmatter fields when saving.
Lines 255-265 only hydrate
descriptionandallowedTools, then Lines 268-289 rebuild the YAML from{ name, description, allowedTools }. Saving an unchanged skill will drop every other existing frontmatter key, which corrupts metadata this dialog does not edit.🛠️ Suggested fix
const buildSkillContent = () => { - const frontmatterData: Record<string, unknown> = { - name: props.name, - description: editDescription.value - } + const { frontmatter } = parseSkillContent(props.markdown) + const frontmatterData: Record<string, unknown> = { + ...frontmatter, + name: props.name, + description: editDescription.value + } @@ if (tools.length) { frontmatterData.allowedTools = tools + } else { + delete frontmatterData.allowedTools }🤖 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 `@src/renderer/settings/components/skills/SkillDetailDialog.vue` around lines 255 - 289, Preserve existing frontmatter keys when saving in SkillDetailDialog by carrying through all unknown metadata from parseSkillContent, not just name, description, and allowedTools. Update hydrateEditor/buildSkillContent to parse and store the original frontmatter (or a passthrough object) and merge edited fields back into it before yaml.stringify, so unchanged skill saves do not drop metadata.src/renderer/settings/components/skills/SkillsSettings.vue-316-333 (1)
316-333: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winIgnore stale detail loads before updating dialog state.
Line 318 stores the selected skill before Line 323 awaits
readSkillFile(). If the user opens skill A and then skill B quickly, A’s slower response can still overwriteskillDetailwhileselectedDetailSkillpoints at B.handleDetailSave()then writes the current dialog content back under the wrong skill name.🛠️ Suggested fix
+let detailRequestId = 0 + const openSkillDetail = async (skill: UnifiedSkillItem) => { + const requestId = ++detailRequestId try { - selectedDetailSkill.value = skill + const markdown = await skillClient.readSkillFile(skill.name) + if (requestId !== detailRequestId) return + + selectedDetailSkill.value = skill skillDetail.value = { name: skill.name, description: skill.description, sourcePath: skill.path, - markdown: await skillClient.readSkillFile(skill.name), + markdown, mutable: skill.mutable } detailDialogOpen.value = true🤖 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 `@src/renderer/settings/components/skills/SkillsSettings.vue` around lines 316 - 333, The `openSkillDetail` flow in `SkillsSettings.vue` can overwrite the dialog with stale data when `readSkillFile()` resolves out of order after a newer skill selection. Update `openSkillDetail` to guard against stale loads by capturing the requested skill identity and verifying it is still the current `selectedDetailSkill` before assigning `skillDetail` or opening the dialog. Keep the dialog state consistent with the latest selection so `handleDetailSave()` always saves against the correct skill.src/renderer/settings/components/skills/SkillImportExportTab.vue-238-259 (1)
238-259: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle client-call failures instead of letting them escape.
loadConfig(),saveDirectory(), and the preview/execute flows all await IPC/filesystem calls without a catch. When one rejects, the tab only drops the spinner and Vue gets the error; the user never sees a recoverable failure state or toast.Also applies to: 287-351, 366-371
🤖 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 `@src/renderer/settings/components/skills/SkillImportExportTab.vue` around lines 238 - 259, Handle rejected IPC/filesystem calls in the SkillImportExportTab flows instead of letting them bubble to Vue. Wrap loadConfig(), saveDirectory(), and the preview/execute paths in try/catch around the skillClient and deviceClient awaits, set a user-visible error/recoverable state, and show an error toast on failure while still clearing the loading/saving flags in finally. Use the existing loadConfig, browse, and saveDirectory symbols as entry points, and apply the same pattern to the preview/execute handlers in this component.src/renderer/settings/components/skills/InstallSkillToAgentDialog.vue-190-199 (1)
190-199: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDeduplicate
loadPreview()and ignore stale responses.Line 197 already triggers the watcher on Lines 302-304, but Line 198 calls
loadPreview()again. Because Line 218 assigns the result unconditionally, a slower response from an older agent selection can overwrite the preview for the current one and enable the wrong action.Suggested fix
const loadingAgents = ref(false) const loadingPreview = ref(false) const executing = ref(false) const error = ref<string | null>(null) +let previewRequestId = 0 const loadAgents = async () => { loadingAgents.value = true error.value = null try { agents.value = (await skillSyncClient.scanAgents()).filter( (agent) => agent.supportsLinkManagement && agent.status === 'ready' ) selectedAgentId.value = agents.value[0]?.id ?? null - await loadPreview() } catch (cause) { error.value = cause instanceof Error ? cause.message : String(cause) } finally { loadingAgents.value = false } } const loadPreview = async () => { if (!props.skill || !selectedAgentId.value) { previewItem.value = null return } + const requestId = ++previewRequestId + const agentId = selectedAgentId.value loadingPreview.value = true error.value = null try { const preview = await skillSyncClient.previewLinkDeepChatSkills({ - agentId: selectedAgentId.value, + agentId, skillNames: [props.skill.name] }) - previewItem.value = preview.items[0] ?? null + if (requestId === previewRequestId && selectedAgentId.value === agentId) { + previewItem.value = preview.items[0] ?? null + } } catch (cause) { - error.value = cause instanceof Error ? cause.message : String(cause) - previewItem.value = null + if (requestId === previewRequestId) { + error.value = cause instanceof Error ? cause.message : String(cause) + previewItem.value = null + } } finally { - loadingPreview.value = false + if (requestId === previewRequestId) { + loadingPreview.value = false + } } }Also applies to: 206-225, 292-304
🤖 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 `@src/renderer/settings/components/skills/InstallSkillToAgentDialog.vue` around lines 190 - 199, The preview loading flow in loadAgents/loadPreview is triggering twice and can apply stale results to the current selection. Remove the redundant direct loadPreview() call from loadAgents so the selectedAgentId watcher remains the single trigger, and update loadPreview() to ignore out-of-order responses by checking the currently selected agent before assigning preview state or enabling actions. Make the fix in the loadAgents, loadPreview, and selectedAgentId watcher paths so older agent responses cannot overwrite newer ones.src/renderer/settings/components/skills/SkillImportExportTab.vue-250-255 (1)
250-255: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear preview state after changing the sync directory.
Lines 253-254 switch the configured directory, but the old
exportPreview,importPreview, andselectedImportNamesstay live. The action buttons can still run against the new directory while the UI is showing preview data from the previous one.Suggested fix
const saveDirectory = async () => { saving.value = true try { config.value = await skillClient.setSkillsSyncDirectory(directory.value) directory.value = config.value.skillsDirectory + exportPreview.value = null + importPreview.value = null + selectedImportNames.value = new Set() toast({ title: t('settings.skills.importExport.saved') }) } finally { saving.value = false } }Also applies to: 319-351
🤖 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 `@src/renderer/settings/components/skills/SkillImportExportTab.vue` around lines 250 - 255, The sync-directory update in saveDirectory leaves stale preview state behind, so clear exportPreview, importPreview, and selectedImportNames whenever the configured directory changes. Update the directory-switch flow in SkillImportExportTab.vue to reset these refs right after skillClient.setSkillsSyncDirectory succeeds, and apply the same cleanup in the related import/export action handlers that reuse preview state so the UI and actions stay aligned with the new directory.src/renderer/settings/components/skills/InstallFromGitDialog.vue-13-18 (1)
13-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInvalidate the scan state when
repoUrlchanges.After Line 183 scans one repository, the user can edit the input and still click Install; Line 212 then sends the new
repoUrlwith skill names selected from the oldscanResult. That mixes two different repositories and can install the wrong set or fail unexpectedly.Suggested fix
const repoUrl = ref('https://github.com/op7418/guizang-ppt-skill') const scanResult = ref<GitSkillRepoScanResult | null>(null) const selectedNames = ref<Set<string>>(new Set()) +const scannedRepoUrl = ref<string | null>(null) const strategy = ref<SkillInstallConflictStrategy>('rename') const scanning = ref(false) const installing = ref(false) const error = ref<string | null>(null) const canInstall = computed( () => Boolean(scanResult.value) && + scannedRepoUrl.value === repoUrl.value && selectedNames.value.size > 0 && !scanning.value && !installing.value ) const scan = async () => { error.value = null scanning.value = true try { scanResult.value = await skillClient.scanGitSkillRepo(repoUrl.value) + scannedRepoUrl.value = repoUrl.value selectedNames.value = new Set( scanResult.value.skills.filter((skill) => skill.valid).map((skill) => skill.name) ) } catch (cause) { error.value = cause instanceof Error ? cause.message : String(cause) scanResult.value = null selectedNames.value = new Set() + scannedRepoUrl.value = null } finally { scanning.value = false } } + +watch(repoUrl, (next, prev) => { + if (next !== prev) { + scanResult.value = null + selectedNames.value = new Set() + scannedRepoUrl.value = null + } +})Also applies to: 171-177, 179-215
🤖 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 `@src/renderer/settings/components/skills/InstallFromGitDialog.vue` around lines 13 - 18, The InstallFromGitDialog component is allowing a stale scanResult to be reused after repoUrl changes, which can mix repositories between scan and install. Update the dialog logic so changing repoUrl invalidates or clears the existing scan state (including selected skill names) and disables Install until scan is run again. Make sure the scan and install flows in InstallFromGitDialog stay tied to the same repository by resetting any cached scanResult when repoUrl updates.src/renderer/settings/components/skills/SkillAgentsTab.vue-173-208 (1)
173-208: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winIgnore stale
getAgentDetail()responses.Rapid agent switches start overlapping detail requests, and Line 197 stores whichever one resolves last. That can render agent A's table while agent B is selected, so follow-up actions operate against mismatched UI state.
Suggested fix
const selectedAgentId = ref<string | null>(null) const selectedAgentDetail = ref<InstalledSkillAgentDetail | null>(null) +let detailRequestId = 0 const loadAgentDetail = async (agentId: string) => { + const requestId = ++detailRequestId detailLoading.value = true error.value = null try { - selectedAgentDetail.value = await skillSyncClient.getAgentDetail(agentId) + const detail = await skillSyncClient.getAgentDetail(agentId) + if (requestId === detailRequestId && selectedAgentId.value === agentId) { + selectedAgentDetail.value = detail + } } catch (cause) { - error.value = cause instanceof Error ? cause.message : String(cause) + if (requestId === detailRequestId) { + error.value = cause instanceof Error ? cause.message : String(cause) + } } finally { - detailLoading.value = false + if (requestId === detailRequestId) { + detailLoading.value = false + } } } const selectAgent = async (agentId: string) => { selectedAgentId.value = agentId + selectedAgentDetail.value = null await loadAgentDetail(agentId) }🤖 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 `@src/renderer/settings/components/skills/SkillAgentsTab.vue` around lines 173 - 208, The detail-loading flow in SkillAgentsTab.vue can apply an out-of-date response from loadAgentDetail after the user switches agents quickly, causing selectedAgentDetail to mismatch selectedAgentId. Update loadAgentDetail and selectAgent to ignore stale getAgentDetail results by tracking the active request/agent id and only assigning selectedAgentDetail when the response still matches the currently selected agent. Also ensure loadAgents does not let an older in-flight detail request overwrite the newer selection state.src/shared/contracts/routes/skills.routes.ts-99-119 (1)
99-119: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject option-like Git repo inputs at the contract boundary. These routes accept any non-empty
repoUrl, but the presenter passes it straight togit clone. A value beginning with-can be parsed as a Git option instead of a repository, so tighten the schema here and pass--togit cloneas defense in depth.🤖 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 `@src/shared/contracts/routes/skills.routes.ts` around lines 99 - 119, Tighten the Git repo contract validation for skills.scanGitRepo and skills.installFromGit so repoUrl cannot start with a dash, since these values are passed through to git clone. Update the zod schema in skills.routes.ts to reject option-like inputs at the boundary, and in the presenter/service that invokes git clone for these routes, pass -- before the repo URL as a defense-in-depth safeguard.
🧹 Nitpick comments (9)
docs/features/deepchat-skills-management/tasks.md (1)
3-9: 📐 Maintainability & Code Quality | 🔵 TrivialHistorical context note: "Install tab" references describe pre-Phase 8 state.
Lines 97 and 129 reference an "Install tab" that was later removed in Phase 8. Consider adding a historical note or updating to "Add Skill menu" for future readers, though this is minor since the document tracks implementation history.
🤖 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 `@docs/features/deepchat-skills-management/tasks.md` around lines 3 - 9, The task notes still refer to the removed “Install tab,” which can confuse future readers. Update the wording in the relevant task descriptions to use the current “Add Skill menu” terminology, or add a short historical note where “Install tab” is mentioned in docs/features/deepchat-skills-management/tasks.md so the Phase 8 rename is clear. Keep the references consistent with the existing DeepChat skills management terms used in this document.docs/features/deepchat-skills-management/plan.md (1)
634-641: 📐 Maintainability & Code Quality | 🔵 TrivialPhase numbering inconsistency with tasks.md.
This section lists 6 delivery steps, but
tasks.mduses Phases 0-8 with UX consolidation as Phase 8. Align the phase numbers or add a mapping note to prevent confusion during implementation tracking.🤖 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 `@docs/features/deepchat-skills-management/plan.md` around lines 634 - 641, The phase list in the plan is inconsistent with tasks.md, which can confuse implementation tracking. Update the numbering in this section to match the Phase 0-8 structure used elsewhere, or add a clear mapping note that links each listed delivery step to its corresponding phase; use the surrounding plan section and the UX consolidation item as the reference points when renumbering or annotating.docs/features/deepchat-skills-management/spec.md (1)
514-515: 📐 Maintainability & Code Quality | 🔵 TrivialClarify how
SkillDetailDialogdistinguishes Library vs. Agent context.The spec states the same dialog is used from both Library and Agents rows. Consider adding a note about whether the dialog receives a context prop (
source: 'library' | 'agent') or infers available actions from the source descriptor shape. This prevents UI bugs where agent-owned skills show "Delete" or Library skills show "Adopt."🤖 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 `@docs/features/deepchat-skills-management/spec.md` around lines 514 - 515, Clarify how SkillDetailDialog determines whether it is being opened from a Library row or an Agents row by explicitly documenting the source signal it uses. Update the spec section around the dialog behavior to state whether the dialog accepts a context prop like source: 'library' | 'agent' or derives the context from the source descriptor shape, and then tie action availability (for example Delete vs Adopt) to that identified symbol so the UI cannot show the wrong actions.src/main/routes/index.ts (1)
3008-3013: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winTrack
skillsListCatalogRoutein the startup scheduler.This route is wrapped in
runTrackedRouteTask, butresolveTrackedRouteTaskonly matchesskillsListMetadataRoute, so the new catalog load bypasses the intended deferred settings workload path.⚙️ Proposed fix
- if (routeName === skillsListMetadataRoute.name && isSettings) { + if ( + (routeName === skillsListMetadataRoute.name || routeName === skillsListCatalogRoute.name) && + isSettings + ) {🤖 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 `@src/main/routes/index.ts` around lines 3008 - 3013, The `skillsListCatalogRoute` handler is currently wrapped in `runTrackedRouteTask` but `resolveTrackedRouteTask` only recognizes `skillsListMetadataRoute`, so the catalog load is not being scheduled through the deferred settings workload path. Update the route tracking logic in `resolveTrackedRouteTask` and the related `runTrackedRouteTask` flow so `skillsListCatalogRoute` is also classified and scheduled consistently with the existing metadata route.test/main/presenter/remoteControlPresenter/remoteBindingStore.test.ts (1)
267-274: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the legacy Telegram migration is persisted.
This only checks the derived
enabledflag. IfRemoteBindingStorestops writing the migrated nestedtelegramconfig back toremoteControl, the test still passes and the migration path stays unguarded. Please assert the savedremoteControlshape or thesetSettingcall too.🤖 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/main/presenter/remoteControlPresenter/remoteBindingStore.test.ts` around lines 267 - 274, The legacy Telegram migration test only verifies the derived enabled flag, so it can miss regressions where RemoteBindingStore does not persist the migrated telegram config back into remoteControl. Update the test around RemoteBindingStore and getTelegramConfig to also assert the saved remoteControl shape or verify the setSetting call includes the nested telegram payload after migration, so the persistence path is covered as well as the derived value.test/renderer/components/PluginsCatalogPage.test.ts (1)
81-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocalize the mocked catalog strings you assert against.
Because this
t()stub returns the key for titles, headings, and action labels, the test still passes if the catalog renders raw i18n keys. Please return distinct translated values for the asserted keys and check those values instead. As per coding guidelines,src/renderer/src/**/*.{vue,ts,tsx}: All user-facing strings must use vue-i18n keys insrc/renderer/src/i18n.🤖 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 81 - 89, The test mock for vue-i18n in PluginsCatalogPage.test.ts is still returning raw keys for several asserted catalog strings, so it won’t catch untranslated UI. Update the useI18n().t stub to return distinct localized values for every title, heading, and action label the test checks, and then assert against those translated values in the PluginsCatalogPage assertions.Source: Coding guidelines
test/renderer/components/OfficialPluginDetailPage.test.ts (1)
116-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReturn concrete translations for the asserted title/action keys.
Right now
t()falls back to the raw key for most strings, so these assertions still pass if the page renders i18n keys verbatim instead of localized text. Stub distinct values for the checked title/button keys and assert those instead. As per coding guidelines,src/renderer/src/**/*.{vue,ts,tsx}: All user-facing strings must use vue-i18n keys insrc/renderer/src/i18n.🤖 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/OfficialPluginDetailPage.test.ts` around lines 116 - 124, The vue-i18n mock in OfficialPluginDetailPage.test.ts only returns localized text for a couple of descriptions and falls back to raw keys for the asserted title/action strings, so the test can still pass when the UI renders untranslated keys. Update the mocked useI18n().t in the test to return concrete distinct translations for the specific title/button keys being asserted, and change the expectations to match those concrete values. Use the checked key names in this test to locate the assertions and ensure the mock covers them explicitly.Source: Coding guidelines
test/renderer/api/clients.test.ts (1)
2022-2100: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the returned values from the new client methods.
These additions mostly verify route names and payloads. For these thin bridge clients, the bug-prone part is response unwrapping; the suite still passes if a method starts returning the raw
{ result }/{ preview }/{ config }envelope instead of the inner value. Please capture and assert representative return payloads for the new skill and agent-link methods as well.Also applies to: 2197-2254
🤖 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/api/clients.test.ts` around lines 2022 - 2100, The new skillSync client tests only verify invoke payloads, so they miss regressions where methods return the raw response envelope instead of the unwrapped value. Update the assertions in clients.test.ts around skillSyncClient methods like getAgentDetail, getAgentSkillDetail, previewAdoptAgentSkill, executeAdoptAgentSkill, previewLinkDeepChatSkills, executeLinkDeepChatSkills, repairAgentSkillLink, removeAgentSkillLink, previewImport, executeImport, previewExport, and executeExport to capture and assert representative returned values from each call. Make sure the checks validate the inner result/preview/config values, not just the bridge.invoke arguments.src/renderer/settings/components/skills/SkillsSettings.vue (1)
293-314: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove the extra catalog subscription.
useSkillsStore()already subscribes toskillClient.onCatalogChanged()insrc/renderer/src/stores/skillsStore.ts, so Lines 308-314 add a second identical listener. While this page is mounted, every catalog event reloads the store twice.♻️ Suggested cleanup
-import { ref, computed, onMounted, onUnmounted } from 'vue' +import { ref, computed, onMounted } from 'vue' @@ -const eventCleanup = ref<(() => void) | null>(null) - onMounted(async () => { const enabled = await configClient.getSkillDraftSuggestionsEnabled() draftSuggestionsEnabled.value = enabled ?? false await skillsStore.loadSkills() - setupEventListeners() -}) - -onUnmounted(() => { - if (eventCleanup.value) { - eventCleanup.value() - } }) - -const setupEventListeners = () => { - const handleSkillEvent = () => { - skillsStore.loadSkills() - } - - eventCleanup.value = skillClient.onCatalogChanged(handleSkillEvent) -}🤖 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 `@src/renderer/settings/components/skills/SkillsSettings.vue` around lines 293 - 314, The SkillsSettings.vue component is registering a duplicate catalog change listener on top of the one already set up by useSkillsStore() in skillsStore.ts. Remove the extra setupEventListeners/onUnmounted subscription logic from SkillsSettings and keep only the initial load in onMounted so catalog updates are handled once by the store’s existing skillClient.onCatalogChanged() subscription.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
1 participant
Summary
/pluginsroutes with top tabs for Plugins, Skills, and MCP/plugins/:pluginIdvirtual ids such asremote:telegramUI Layout
Before:
After:
Validation
pnpm run formatpnpm run i18npnpm run lintpnpm run typecheckpnpm exec vitest run test/main/presenter/remoteControlPresenter/remoteBindingStore.test.ts test/renderer/components/WindowSideBar.test.ts test/renderer/components/RemoteSettings.test.tspnpm exec vitest run test/renderer/components/WindowSideBar.test.ts test/renderer/components/McpIndicator.test.ts test/renderer/components/McpSettings.test.ts test/renderer/components/RemoteSettings.test.ts test/renderer/components/WelcomePage.test.ts test/renderer/lib/storeInitializer.test.tspnpm exec vitest run test/renderer/components/OfficialPluginDetailPage.test.ts test/renderer/components/RemoteSettings.test.tsSummary by CodeRabbit