fix(extension-agent): 修复 Skill 导入与管理页刷新#812
fix(extension-agent): 修复 Skill 导入与管理页刷新#812CookSleep wants to merge 1 commit intoChatLunaLab:v1-devfrom
Conversation
Walkthrough本PR优化了组件生命周期管理与数据加载策略,改进了导入功能的用户反馈消息,增强了刷新操作的并发控制机制,并完善了GitHub导入时的目录检测逻辑。共涉及6个文件的功能性改进。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces KeepAlive support for several agent pages, refactors the data refresh mechanism to use promise-based locking, and optimizes sub-agent data loading by fetching availability data only when the relevant tab is active. Additionally, it refines import success messages and improves the directory detection logic for GitHub imports. Feedback suggests extracting repeated availability refresh logic into a helper function to improve maintainability and optimizing the base directory detection in the GitHub import process by using readdir instead of a recursive file search for better performance.
| if (listTab.value === 'availability') { | ||
| await loadAvailability() | ||
| } |
There was a problem hiding this comment.
注意到在多个函数(如 reloadSubAgents, toggleAgent, saveSelected 等)中都出现了相同的逻辑来按需刷新可用性数据:
if (listTab.value === 'availability') {
await loadAvailability()
}为了提高代码的可维护性和减少重复,建议将此逻辑提取到一个独立的辅助函数中。
例如,您可以创建一个名为 refreshAvailabilityIfNeeded 的函数:
async function refreshAvailabilityIfNeeded() {
if (listTab.value === 'availability') {
await loadAvailability()
}
}然后在所有需要的地方调用 await refreshAvailabilityIfNeeded()。
| const files = await collectFilesRecursive(root, { relative: true }) | ||
| const tops = Array.from( | ||
| new Set(files.map((file) => file.replaceAll('\\', '/').split('/')[0])) | ||
| ).filter(Boolean) | ||
| const base = | ||
| tops.length === 1 && | ||
| (await stat(join(root, tops[0])).catch(() => undefined))?.isDirectory() | ||
| ? join(root, tops[0]) | ||
| : root |
There was a problem hiding this comment.
为了确定 base 目录,这里使用了 collectFilesRecursive 来递归地收集所有文件,然后处理文件路径。对于大型仓库,这可能会有性能问题。
一个更高效的方法是只读取 root 目录的顶层条目。如果只有一个条目并且它是一个目录,那么它就是 base 目录。这可以通过 fs.promises.readdir 实现。
建议进行如下修改(同时请记得在文件顶部从 fs/promises 导入 readdir):
const entries = await readdir(root)
const base =
entries.length === 1 &&
(await stat(join(root, entries[0])).catch(() => undefined))?.isDirectory()
? join(root, entries[0])
: rootThere was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/extension-agent/client/components/sub-agent/sub-agent-page.vue (1)
584-607:loadMeta内部设置busy状态可能导致 loading 指示器闪烁
loadMeta在内部独立管理busy.value,当被其他函数(如reloadSubAgents)调用时,其finally块会在外层函数完成前将busy设为false,可能导致 loading 指示器短暂消失。建议将
busy状态管理移至调用方,或让loadMeta/loadAvailability接受一个参数来控制是否管理busy状态。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-agent/client/components/sub-agent/sub-agent-page.vue` around lines 584 - 607, The busy flag is toggled inside loadMeta (and similarly loadAvailability) causing flicker when callers like reloadSubAgents also manage busy; change these helpers to not unconditionally mutate busy.value: either remove busy.value assignments from loadMeta/loadAvailability and let callers (e.g., reloadSubAgents) set busy before/after awaiting them, or add an explicit parameter (e.g., manageBusy = false) to loadMeta and loadAvailability so they only set busy when manageBusy is true; update all callers to pass the appropriate argument or manage busy themselves (ensure finally blocks in callers clear busy.value).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/extension-agent/client/components/sub-agent/sub-agent-page.vue`:
- Around line 584-607: The busy flag is toggled inside loadMeta (and similarly
loadAvailability) causing flicker when callers like reloadSubAgents also manage
busy; change these helpers to not unconditionally mutate busy.value: either
remove busy.value assignments from loadMeta/loadAvailability and let callers
(e.g., reloadSubAgents) set busy before/after awaiting them, or add an explicit
parameter (e.g., manageBusy = false) to loadMeta and loadAvailability so they
only set busy when manageBusy is true; update all callers to pass the
appropriate argument or manage busy themselves (ensure finally blocks in callers
clear busy.value).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3986cb35-24e0-4da2-a0ae-3dbdd3dfb429
📒 Files selected for processing (6)
packages/extension-agent/client/components/layout/agent-shell.vuepackages/extension-agent/client/components/skills/skills-import-folder-dialog.vuepackages/extension-agent/client/components/skills/skills-import-github-dialog.vuepackages/extension-agent/client/components/skills/skills-import-markdown-dialog.vuepackages/extension-agent/client/components/sub-agent/sub-agent-page.vuepackages/extension-agent/src/skills/import.ts
Summary
Testing