fix(resource-panel): show files and plugins when opening via button click#13079
fix(resource-panel): show files and plugins when opening via button click#13079
Conversation
…lick When plugins/skills were installed, clicking the ResourceButton showed only plugins but no files. The manager's useEffect was overwriting the button's file list with its own empty file list + plugins. Fix: pass plugins directly to ResourceButton so it builds a complete list, and tighten the manager's early-return guard to only update when it has actually loaded files itself (via @ trigger). Co-Authored-By: Claude Opus 4.6 <[email protected]>
| const agentId = session?.agentId | ||
|
|
||
| // Fetch installed plugins (agents and skills) for the button | ||
| const { plugins, loading: pluginsLoading } = useInstalledPlugins(agentId) |
There was a problem hiding this comment.
Nit: useInstalledPlugins(agentId) is now called here and also in ResourceQuickPanelManager (line 24), resulting in two separate API calls to window.api.claudeCodePlugin.listInstalled(agentId) for the same agentId. This works correctly but is a minor inefficiency. A future improvement could lift the plugin state to a shared context or pass it down through the tool framework to avoid the duplicate fetch.
| useEffect(() => { | ||
| if (role !== 'manager') return | ||
| if (!hasAttemptedLoadRef.current && fileList.length === 0 && !isLoading && plugins.length === 0) { | ||
| if (!hasAttemptedLoadRef.current && fileList.length === 0 && !isLoading) { |
There was a problem hiding this comment.
Observation: Correct fix. Removing plugins.length === 0 ensures the manager only updates the QuickPanel list when hasAttemptedLoadRef.current is true (set by loadFiles() inside openQuickPanel). When the button opens the panel, the manager's hasAttemptedLoadRef is still false, so it correctly skips the overwrite.
There was a problem hiding this comment.
Note
This review was translated by Claude.
Code review approved, the fix precisely resolved the architecture issues.
Minor suggestion (non-blocking):
When calling useInstalledPlugins in resourceTool.tsx, consider adding error logging to improve observability:
const { plugins, loading: pluginsLoading, error: pluginsError } = useInstalledPlugins(agentId)
if (pluginsError) {
logger.warn('Failed to load plugins for ResourceButton', { error: pluginsError })
}However, this is a minor improvement, and the current implementation is acceptable. Approval does not depend on this change.
Original Content
代码审查通过,修复方案精准解决了架构问题。
轻微建议 (非阻塞):
resourceTool.tsx 中调用 useInstalledPlugins 时可考虑添加错误日志以提高可观测性:
const { plugins, loading: pluginsLoading, error: pluginsError } = useInstalledPlugins(agentId)
if (pluginsError) {
logger.warn('Failed to load plugins for ResourceButton', { error: pluginsError })
}不过这是一个很小的改进,当前实现已可接受,Approval 不依赖此更改。
What this PR does
Before this PR:
When plugins/skills were installed, clicking the ResourceButton (folder icon) in the input bar showed only plugins but no files. The
@trigger worked correctly.After this PR:
Clicking the ResourceButton correctly shows both files and plugins (agents/skills), consistent with the
@trigger behavior.Why we need it and why it was done in this way
The root cause was an architectural issue between two instances of
useResourcePanel:'button') passedplugins: [], so it built a list with only files.'manager') had auseEffectthat detected the panel opening (sameMentionModelssymbol) and overwrote the list with its owncategorizedItems— which contained plugins but an empty file list.The fix has two parts:
pluginsandpluginsLoadingtoResourceButtonso it builds a complete list independently.plugins.length === 0condition) so the manager only updates the QuickPanel list when it has actually loaded files itself (i.e., was opened via@trigger).The following tradeoffs were made:
useInstalledPluginsis now called in bothresourceTool.tsx(for the button) andResourceQuickPanelManager(for the manager). This is a minor duplication but avoids changing the genericquickPanelManagerinterface.The following alternatives were considered:
Lifting the plugin state to a shared parent was considered but would require changing the tool framework's
quickPanelManagerinterface, which is overkill for this fix.Breaking changes
None.
Special notes for your reviewer
The key insight is that both the button and the manager share the same
QuickPanelReservedSymbol.MentionModelssymbol. When the button opens the panel, the manager'suseEffectfires and overwrites the list. The fix ensures each instance is self-sufficient and the manager only updates when it initiated the panel opening.Checklist
Release note