Skip to content

fix(extension-agent): 修复 Skill 导入与管理页刷新#812

Open
CookSleep wants to merge 1 commit intoChatLunaLab:v1-devfrom
CookSleep:fix/agent-skill-import-refresh
Open

fix(extension-agent): 修复 Skill 导入与管理页刷新#812
CookSleep wants to merge 1 commit intoChatLunaLab:v1-devfrom
CookSleep:fix/agent-skill-import-refresh

Conversation

@CookSleep
Copy link
Copy Markdown
Member

Summary

  • 修复从 GitHub 导入 Skill 时,预览目录与正式导入目录不一致导致无法识别勾选项的问题。
  • 调整 Skill 导入成功提示,去掉“默认启用”的误导性文案。
  • 保留切回 Skills、Sub Agent、Tools 页时的刷新能力,同时用 KeepAlive、按需请求和刷新去重减少重复加载。

Testing

  • yarn fast-build extension-agent

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 3, 2026

Walkthrough

本PR优化了组件生命周期管理与数据加载策略,改进了导入功能的用户反馈消息,增强了刷新操作的并发控制机制,并完善了GitHub导入时的目录检测逻辑。共涉及6个文件的功能性改进。

Changes

Cohort / File(s) Summary
Skills导入消息优化
packages/extension-agent/client/components/skills/skills-import-folder-dialog.vue, skills-import-github-dialog.vue, skills-import-markdown-dialog.vue
调整导入成功提示:当无名称冲突时,移除"默认启用"的描述,仅显示导入的Skill数量。
Agent Shell组件优化
packages/extension-agent/client/components/layout/agent-shell.vue
将Transition包装器替换为KeepAlive块实现标签页缓存;重构刷新并发控制,将单调递增的refreshId替换为共享的refreshTask Promise,确保同时仅运行一个刷新请求。
Sub-Agent页面生命周期重构
packages/extension-agent/client/components/sub-agent/sub-agent-page.vue
拆分初始化加载为多个生命周期钩子;重构数据加载函数(loadExtraData→loadMeta、loadDynamicData→loadAvailability);优化标签页驱动的刷新逻辑,仅在必要时加载特定数据。
GitHub导入目录处理
packages/extension-agent/src/skills/import.ts
改进GitHub zipball解压逻辑:检测是否存在单一顶层目录,若存在则以其为基准进行路径解析,改善对嵌套结构仓库的处理。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • dingyi222666

Poem

🐰 标签页缓存轻飘飘,
刷新并发不再吵,
生命周期新安排,
GitHub导入更爽快,
消息提示更简洁,清爽!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR 标题清晰准确地概括了主要变更:修复 Skill 导入与管理页刷新问题,与文件变更和 PR 目标高度相关。
Description check ✅ Passed PR 描述详细说明了三个关键修复内容,与代码变更完全相关,提供了充分的上下文说明。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +613 to +615
if (listTab.value === 'availability') {
await loadAvailability()
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

注意到在多个函数(如 reloadSubAgents, toggleAgent, saveSelected 等)中都出现了相同的逻辑来按需刷新可用性数据:

if (listTab.value === 'availability') {
    await loadAvailability()
}

为了提高代码的可维护性和减少重复,建议将此逻辑提取到一个独立的辅助函数中。

例如,您可以创建一个名为 refreshAvailabilityIfNeeded 的函数:

async function refreshAvailabilityIfNeeded() {
    if (listTab.value === 'availability') {
        await loadAvailability()
    }
}

然后在所有需要的地方调用 await refreshAvailabilityIfNeeded()

Comment on lines +481 to +489
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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

为了确定 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])
            : root

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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

📥 Commits

Reviewing files that changed from the base of the PR and between 086a1f7 and 39948d6.

📒 Files selected for processing (6)
  • packages/extension-agent/client/components/layout/agent-shell.vue
  • packages/extension-agent/client/components/skills/skills-import-folder-dialog.vue
  • packages/extension-agent/client/components/skills/skills-import-github-dialog.vue
  • packages/extension-agent/client/components/skills/skills-import-markdown-dialog.vue
  • packages/extension-agent/client/components/sub-agent/sub-agent-page.vue
  • packages/extension-agent/src/skills/import.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant