feat(skills): 优化远程 Skill 批量安装,单次克隆完成多 skill 安装#695
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10781e80c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| export const installRemoteSkillsBatch = async (payload) => { | ||
| return apiSuperAdminPost(`${BASE_URL}/remote/install-batch`, payload) |
There was a problem hiding this comment.
Use the imported admin POST helper for batch installs
When the batch install UI calls installRemoteSkillsBatch, this module has only imported apiAdminGet, apiAdminPost, apiAdminPut, and apiAdminDelete, so apiSuperAdminPost is an undefined identifier in the browser and the request is never sent. This breaks all remote batch installs from the Skills modal; this should use an imported helper, most likely the existing admin POST helper used by the single-skill install endpoint.
Useful? React with 👍 / 👎.
| normalized_source = _normalize_source(source) | ||
| if not skills: | ||
| raise ValueError("skills 列表不能为空") | ||
| normalized_skills = [_normalize_skill_name(skill) for skill in skills] |
There was a problem hiding this comment.
Preserve per-skill failures for malformed batch entries
Because the Skills modal uses tag input, users can submit a valid skill together with a malformed tag such as bad name; this list comprehension raises before any CLI/install work starts, so the route returns one 400 and installs none of the valid selections. The previous per-skill frontend loop isolated each failure, and the new batch API returns per-skill results, so invalid names should be recorded as failed entries while continuing with the valid normalized skills.
Useful? React with 👍 / 👎.
| async def fake_import_skill_dir(_db, *, source_dir, created_by): | ||
| imported_skills.append(source_dir.name) | ||
| return {"slug": source_dir.name} |
There was a problem hiding this comment.
Return slug attributes from the batch import fakes
In the new batch tests, the monkeypatched import_skill_dir returns a dict, but the production batch implementation reads item.slug before appending a success result. That makes these tests exercise the exception path instead of the intended success path, so once pytest_asyncio is installed the “installs all” assertions fail and the tests will not protect the happy-path batch import behavior.
Useful? React with 👍 / 👎.
| - 合并智能体对话导航:移除 `AgentChatComponent` 内部聊天侧边栏,将新建对话入口和对话历史移动到 `AppLayout` 主侧边栏,并通过共享线程 store 统一管理历史列表、当前线程、重命名、删除、置顶和分页加载。 | ||
| - 新增独立模型配置模块:增加 `model_providers` 表、独立管理接口和”模型配置”页面,支持 provider 基础信息、可配置模型列表端点、远端候选模型、`enabled_models` 的早期配置验证;启动时会补齐内置 provider 模板,`provider_type` 暂统一默认为 `openai`,该模块暂不接入现有运行时模型选择逻辑。远端模型加载默认使用 `/models` 获取 chat/通用模型,provider 声明 `embedding` 能力时使用 `/embeddings/models` 获取 embedding 候选,rerank 模型列表端点按供应商文档显式配置后加载;修复路由请求模型未接收 `embedding_base_url`/`rerank_base_url` 导致前端已填写仍被后端校验拦截的问题。补充手动添加模型能力:`enabled_models[i]` 新增可选 `source: "manual"|"remote"` 字段(默认 `remote`),管理员可通过”+ 手动添加”入口录入远端清单未覆盖的模型(典型:自部署 embedding/rerank),手动模型在前端跳过”远端不存在”的 stale 警告并显示「手动」标签;type 选项受 `provider.capabilities` 约束,后端在 `_normalize_payload` 与 `update_provider_config` 双层一致性校验中拦截越权写入。 | ||
| - 统一前端 Markdown 预览渲染:新增共享 `MarkdownPreview` 组件与 `markdown_preview` 渲染工具,替换 Agent 消息、文件预览、知识库 chunk、任务工具结果、聊天导出等场景中的旧 `md-editor-v3/marked` 预览;支持 KaTeX、任务列表、frontmatter 卡片、Shiki 代码高亮、DOMPurify 清洗和浅层渲染缓存,并抽取 HTML 转义与代码语言归一化工具。Skill 详情页复用 `AgentFilePreview`,统一文件预览、编辑、保存和全屏交互。 | ||
| - 优化远程 Skill 批量安装:`remote_skill_install_service.py` 新增 `install_remote_skills_batch()`,利用 `npx skills add --skill A --skill B --skill C` 原生多 skill 支持,将安装 N 个 skill 的仓库克隆次数从 2N 降至 1;配套新增路由 `POST /remote/install-batch`、前端 `installRemoteSkillsBatch()` API 方法和批处理 UI 逻辑;方案文档见 `docs/vibe/2026-05-14-remote-skill-batch-install-plan.md` |
There was a problem hiding this comment.
Link only to a committed batch-install plan
This roadmap entry points readers to docs/vibe/2026-05-14-remote-skill-batch-install-plan.md, but that file is not present in the commit or working tree, so the new documentation link is broken. Either commit the referenced plan or remove the reference to avoid sending maintainers to a missing design note.
Useful? React with 👍 / 👎.
6a886ef to
3690421
Compare
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3690421da4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try: | ||
| normalized_skills.append(_normalize_skill_name(skill)) | ||
| except ValueError as e: | ||
| results.append({"slug": skill, "success": False, "error": str(e)}) |
There was a problem hiding this comment.
Preserve batch install result order
When a batch mixes valid and invalid names (for example ['valid-skill', 'Bad Name', 'another-valid']), invalid entries are appended to results during validation before any valid skill install results are appended, so the response order becomes invalid-first instead of matching the request order. This contradicts the new test_install_remote_skills_batch_handles_invalid_names expectation and can make API clients that display per-request results by position show failures next to the wrong skill.
Useful? React with 👍 / 👎.
| except Exception as e: | ||
| results.append({"slug": name, "success": False, "error": str(e)}) |
There was a problem hiding this comment.
Roll back the session after import failures
When one import_skill_dir fails during database work (for example a commit/constraint error), the shared AsyncSession is left in a failed transaction state; this catch records the error but then reuses the same session for subsequent skills, so later valid imports can all fail until a rollback is issued. To keep the advertised partial-success behavior reliable, roll back the session before continuing after database-backed import failures.
Useful? React with 👍 / 👎.
| for name in normalized_skills: | ||
| skill_args.extend(["--skill", name]) | ||
|
|
||
| await _run_skills_cli( |
There was a problem hiding this comment.
Keep one missing skill from aborting the batch
If any requested --skill does not match the remote repo, the skills CLI reports No matching skills found for: ... (checked against vercel-labs/skills#452) and exits as a failed install; because the single CLI call is outside the per-skill result handling, _run_skills_cli raises before the code can import already-valid selections or return per-skill failures. This regresses the previous UI flow where a stale or manually mistyped skill did not prevent the other selected skills from installing.
Useful? React with 👍 / 👎.
|
P2 级别的,酌情修复吧 |
6445bc9 to
196cb2a
Compare
新增 install_remote_skills_batch() 函数,利用 npx skills add 原生多 --skill 支持, 将安装 N 个 skill 的仓库克隆次数从 2N 降至 1。 后端: - remote_skill_install_service.py 新增批处理 + _find_skill_dir() - skill_router.py 新增 POST /remote/install-batch 端点 (admin 权限) - 非法 skill 名不阻塞其他合法 skill,结果按请求顺序返回 - CLI 部分失败时仍导入已安装的 skill - import 失败后 rollback session,不影响后续导入 前端: - skill_api.js 新增 installRemoteSkillsBatch() - SkillCardList.vue 改为单次批处理调用 测试: - 4 个单元测试覆盖正常安装/缺失跳过/部分失败/非法名场景 文档: - docs/develop-guides/roadmap.md 更新开发记录 - docs/vibe/2026-05-14-remote-skill-batch-install-plan.md 方案文档(含设计决策)
196cb2a to
44d1b20
Compare
@xerrors 已修复 |
变更描述
简要描述这个 PR 做了什么
新增 install_remote_skills_batch() 函数,利用 npx skills add 原生多 --skill 支持, 将安装 N 个 skill 的仓库克隆次数从 2N 降至 1。
变更类型
测试
相关日志或者截图
说明
(可选)有什么需要特别说明的吗?
💡 提示: 提交前可以运行
make lint和make format检查代码规范