diff --git a/docs/superpowers/specs/2026-08-13-dt-skill-batch-commands-codex-install-design.md b/docs/superpowers/specs/2026-08-13-dt-skill-batch-commands-codex-install-design.md new file mode 100644 index 0000000..15b61f5 --- /dev/null +++ b/docs/superpowers/specs/2026-08-13-dt-skill-batch-commands-codex-install-design.md @@ -0,0 +1,121 @@ +# dt-skill 批量命令与 Codex 安装设计 + +## 目标 + +扩展 `dt-skill` 的上传、更新和安装能力: + +- `upload` 一次接受多个 skill 路径 +- `upload` 接受容器目录时,将一级 skill 子目录作为独立 skill 上传(保留现有 package 打包行为) +- `update` 一次接受多个已安装 skill 的 slug 或路径 +- Codex 全局安装时在 `~/.codex/skills` 下提供可直接使用的 skill 入口 + +## 非目标 + +- 不新增 `upload-many` 或 `update-many` 命令 +- 不递归扫描二级及更深层目录 +- 不改变单路径容器目录的 package 打包行为(保留 `/api/skills/import-file`) +- 不改变无参数 `update` 表示更新全部已跟踪 skill 的行为 +- 不改变 registry API、skill 内容格式或 lockfile schema + +## 命令契约 + +### 批量上传 + +命令接受一个或多个路径: + +```bash +npx dt-skill upload omp-skill zentao-api aaa/bbb-skill # 多路径:逐个单 skill +npx dt-skill upload skills # 单路径容器目录:package 打包(现状) +npx dt-skill upload skills local/one-skill # 多路径:容器目录也展开逐个 +``` + +每个输入路径独立展开: + +1. 路径自身包含 `SKILL.md` 或 `skill.md` 时,将其作为一个 skill +2. 否则只检查该目录的一级子目录,将包含 `SKILL.md` 或 `skill.md` 的子目录分别作为 skill +3. 忽略不符合 skill 结构的一级子目录 +4. 按最终 slug 去重(`sanitizeSlug` 后),并按输入顺序、目录内 slug 顺序执行 +5. 没有解析出任何 skill 时返回明确错误 + +执行模式按输入数量区分: + +- **恰好一个路径且为容器目录**(路径自身不含 `SKILL.md`,一级子目录含):走现有 `cmdPublishBatch`,ZIP 打包 + `/api/skills/import-file` 创建 package。交互式勾选**默认全部选中**(`--all` 跳过勾选直接全量)。这是对现状的唯一改动。 +- **其他情况**(单个 skill 路径,或多个路径):每个展开出的 skill 逐个走 `/api/v1/skills` 单 skill 发布链路,保留内容指纹、覆盖确认、分类、贡献者和嵌套文件路径等既有行为。 + +批量(逐个)执行时,一个 skill 失败不阻断后续 skill。命令结束时输出成功、未变化和失败汇总;只要存在失败,进程退出码为非零。`--category`、`--owner`、`--version`、`--changelog`、`--clawscan-note`、`--tags` 和 `--yes` 可共享给全部 skill。展开结果超过一个 skill 时,拒绝 `--slug`、`--name`、`--fork-of`、`--description` 和 `--migrate-owner`,避免把单 skill 身份或描述错误复用到多个 skill。 + +### 批量更新 + +命令接受零个、一个或多个 slug 或路径: + +```bash +npx dt-skill update +npx dt-skill update omp-skill +npx dt-skill update omp-skill zentao-api aaa/bbb-skill +``` + +每个参数按以下规则解析为 slug: + +- 含 `/` 或 `\` 的参数视为路径,slug 取 `sanitizeSlug(basename(resolve(arg)))`,与 publish 的 slug 推导一致 +- 否则视为 slug,走 `normalizeSkillSlugOrFail` + +- 无参数:保持现状,更新所选范围内全部已跟踪且未 pinned 的 skill +- 有参数:解析后按 slug 规范化并去重,只处理明确给出的 skill +- 多个参数共用一次 Project、Global 或 Both 范围选择 +- `--all` 与任意显式参数互斥 +- `--version` 只允许恰好一个显式参数 +- 未安装、远端不存在、恶意内容、被 pinned 或下载失败都记录为该 skill 的失败或跳过,不阻断其他 skill +- 最终沿用统一 update summary;存在失败时返回非零退出码 + +### Codex 全局安装 + +现状存在分类 bug:`codex` 在 `agents/definitions.ts` 里 `skillsDir: '.agents/skills'`,导致 `isUniversalAgent` 将其判为 universal,`linkOrCopyToAgent` 对 universal 直接 skip,`~/.codex/skills` 软链接从不创建。交互式 agent 选择里 codex 被归入锁定区「Universal (.agents/skills)」,看不到 `~/.codex`。 + +目标行为:以下命令在 `~/.codex/skills/` 生成 Codex 可使用的入口: + +```bash +npx dt-skill install omp-skill --global --agent codex +``` + +`~/.agents/skills/` 继续作为规范安装目录并持有 origin/lockfile 对应内容。Codex 全局入口是指向规范目录的软链接,使后续 `update` 原子替换规范目录后无需复制即可读取最新内容。 + +实现上修正 universal 判定或 codex 定义,使全局场景下 codex 走 symlink 逻辑指向 `~/.codex/skills`。注意 `cursor`、`gemini-cli`、`github-copilot`、`opencode` 等也是 `skillsDir: '.agents/skills'` + 独立 `globalSkillsDir` 的组合,存在同样的分类问题;若采用「全局且 `globalSkillsDir` 存在且不等于 canonical 目录时走 symlink」的判定修正,这些 agent 会一并获得正确的全局 symlink,属于预期修复,需在测试中覆盖并记录。 + +卸载全局 skill 时,同时清理 `~/.codex/skills/` 入口。项目范围安装仍沿用 `.agents/skills`,不额外创建项目内 `.codex/skills`。 + +## 代码结构 + +- `src/cli.ts`:把 `publish/upload` 改为可变路径参数,把 `update` 改为可选可变 slug/路径参数,并完成单 skill 参数约束 +- `src/cli/commands/publish.ts`:增加多路径展开、去重、逐项发布;单 skill 发布逻辑保持独立可复用;保留 `cmdPublishBatch`(package 打包)并改为默认全选 +- `src/cli/scanSkills.ts`:继续负责直接 skill 与一级 skill 子目录识别,不增加递归扫描 +- `src/cli/commands/update.ts`:把单个可选 slug 改为显式参数集合(slug 或路径),并在一个 scope 中批量处理 +- `src/cli/agents/definitions.ts` 与 `src/cli/installer.ts`:修正 codex 的 universal 判定,使全局安装走 symlink 到 `~/.codex/skills` +- `src/cli/commands/skills.ts`:卸载时按同一规则清理 Codex 全局入口 +- `README.md`:补充三个新命令示例和路径说明 + +## 错误处理 + +- 不存在或不是目录的上传输入明确报告原始路径 +- 多路径展开后出现重复 slug 时只执行一次 +- 批量上传和批量更新都继续处理后续项,并在结尾集中报告失败 +- 单 skill 调用保留原有抛错和交互行为,减少兼容性变化 +- 批量(逐个)上传中,非交互且未提供 `--category` 时,首次发布的 skill 会按单 skill 规则逐个报错,由失败汇总兜住;交互下沿用覆盖确认 +- Codex 入口创建失败时,该 skill 安装失败,不写入成功状态 + +## 测试与验证 + +采用测试驱动实现,至少覆盖: + +- Commander 将多个 upload 路径和多个 update 参数解析为数组 +- 直接 skill、一级容器目录和混合输入的展开、排序与去重(按 slug) +- 单路径容器目录仍走 package 导入 API(import-file),交互勾选默认全选 +- 多路径展开出的 skill 分别调用单 skill API,而不是 package 导入 API +- 批量上传单项失败后继续,并设置失败退出状态 +- update 的 slug 与路径参数解析为同一 slug(`aaa/bbb-skill` → `bbb-skill`),去重后只处理一次 +- 批量更新只处理指定参数、共用一次 scope、保持其他项继续执行 +- `--all` 与参数、`--version` 与多个参数的参数冲突 +- Codex 全局入口解析为 `~/.codex/skills`,项目范围仍使用 `.agents/skills` +- 安装和卸载分别创建、清理 Codex 全局入口 +- codex 判定修正后,其他同定义 agent(cursor 等)的全局 symlink 行为符合预期 + +最终运行 `dt-skill` 源码测试、TypeScript 类型检查、构建产物测试和 `git diff --check`。实现与本地验证完成后,启动独立 agent 只读审核需求覆盖、最终 diff 和验证证据;确认审核问题已处理后再交付。 diff --git a/dt-skill/README.md b/dt-skill/README.md index 65cc609..31c2f4e 100644 --- a/dt-skill/README.md +++ b/dt-skill/README.md @@ -51,6 +51,39 @@ Interaction guide: Selected child skills are installed into individual folders under your skills directory (`/`). +## Install location + +The canonical install dir is `/.agents/skills` (`base` = home for `--global`, project root otherwise). When a target agent declares its own global skills dir, the CLI symlinks there instead: + +```bash +dt-skill install my-skill --agent codex --global # → ~/.codex/skills/my-skill +dt-skill install my-skill --agent cursor --global # → ~/.cursor/skills/my-skill +``` + +Codex, Cursor, Gemini CLI, GitHub Copilot, and OpenCode all get their dedicated +global dirs rather than the shared `.agents/skills`. + +## Batch upload & update + +`upload`/`publish` and `update` accept multiple positional arguments. Each +argument may be a skill slug, a skill folder path, or (for upload) a directory +whose first-level subfolders are each uploaded as a skill. Slugs are deduplicated. + +```bash +# Upload three skills individually (slugs resolved from folder names) +dt-skill upload omp-skill zentao-api aaa/bbb-skill + +# Upload every first-level skill folder under ./skills +dt-skill upload ./skills + +# Update several installed skills (slug or path both work) +dt-skill update omp-skill zentao-api aaa/bbb-skill +``` + +For upload, `--slug/--name/--fork-of/--description/--migrate-owner` require a +single skill path; batch mode publishes each skill independently via +`/api/v1/skills` (not the `/api/skills/import-file` package route). + ## Examples ```bash @@ -59,9 +92,12 @@ dt-skill install my-skill-pack dt-skill pin bear-notes --reason "scanner-flagged while awaiting moderation" dt-skill update --all dt-skill update --all --no-input --force +dt-skill update omp-skill zentao-api aaa/bbb-skill # batch update (slugs or paths) dt-skill unpin bear-notes dt-skill skill publish ./my-skill-pack --slug my-skill-pack --name "My Skill Pack" --version 1.2.0 --changelog "Fixes + docs" dt-skill skill publish ./org-skill --owner openclaw --version 1.2.0 --changelog "Org publish" +dt-skill upload omp-skill zentao-api aaa/bbb-skill # batch upload multiple skills +dt-skill upload ./skills # upload first-level subfolders as skills dt-skill package explore --family skill dt-skill package explore --family code-plugin dt-skill package inspect @openclaw/example-plugin diff --git a/dt-skill/src/cli.ts b/dt-skill/src/cli.ts index 2c24f1b..84ea6d7 100644 --- a/dt-skill/src/cli.ts +++ b/dt-skill/src/cli.ts @@ -214,18 +214,18 @@ registerCommand(program, ['update']) .description( 'Update installed skills to registry content (by hash). Bare update = all tracked skills.' ) - .argument('[slug]', 'Skill slug (omit to update all)') + .argument('[slugs...]', 'Skill slugs or paths (omit to update all)') .option('--all', 'Update all installed skills (same as bare update)') .option('--version ', 'Update to specific version (single slug only, legacy)') .option('--force', 'Overwrite when local files do not match registry content') .option('-g, --global', 'Update global skills only (~/.agents)') .option('-p, --project', 'Update project skills only') - .action(async (slug, options) => { + .action(async (slugs, options) => { const opts = await resolveGlobalOpts(); const programOpts = program.opts<{ global?: boolean; yes?: boolean }>(); await cmdUpdate( opts, - slug, + slugs, { all: options.all, version: options.version, @@ -315,7 +315,7 @@ registerCommand(program, ['publish']) 'Publish a skill folder to the registry (push remote). Re-publish same slug overwrites by content hash.' ) .alias('upload') - .argument('', 'Skill folder path') + .argument('', 'Skill folder path(s)') .option('--slug ', 'Skill slug') .option('--name ', 'Display name') .option('--owner ', 'Publish under an org/user publisher handle') @@ -335,9 +335,9 @@ registerCommand(program, ['publish']) 'Optional market card summary (create defaults from SKILL.md; re-publish keeps card unless set)' ) .option('--yes', 'Skip overwrite confirmation') - .action(async (folder, options) => { + .action(async (folders, options) => { const opts = await resolveGlobalOpts(); - await cmdPublish(opts, folder, options); + await cmdPublish(opts, folders, options); }); registerCommand(program, ['delete']) @@ -387,7 +387,7 @@ registerCommand(program, ['unhide']) const skill = registerCommandGroup(program, ['skill']).description('Manage published skills'); registerCommand(skill, ['skill', 'publish']) .description('Publish a skill from folder (same as publish/upload)') - .argument('', 'Skill folder path') + .argument('', 'Skill folder path(s)') .option('--slug ', 'Skill slug') .option('--name ', 'Display name') .option('--owner ', 'Publish under an org/user publisher handle') @@ -403,9 +403,9 @@ registerCommand(skill, ['skill', 'publish']) 'Optional market card summary (create defaults from SKILL.md; re-publish keeps card unless set)' ) .option('--yes', 'Skip overwrite confirmation') - .action(async (folder, options) => { + .action(async (folders, options) => { const opts = await resolveGlobalOpts(); - await cmdPublish(opts, folder, options); + await cmdPublish(opts, folders, options); }); registerCommand(program, ['star']) diff --git a/dt-skill/src/cli/agents/definitions.ts b/dt-skill/src/cli/agents/definitions.ts index e4d6baf..2bba29b 100644 --- a/dt-skill/src/cli/agents/definitions.ts +++ b/dt-skill/src/cli/agents/definitions.ts @@ -74,7 +74,7 @@ export const AGENT_DEFINITIONS = { name: 'amp', displayName: 'Amp', skillsDir: '.agents/skills', - globalSkillsDir: join(configHome, 'agents/skills'), + globalSkillsDir: join(home, '.agents', 'skills'), detectInstalled: () => existsSync(join(configHome, 'amp')), }, antigravity: { @@ -455,7 +455,7 @@ export const AGENT_DEFINITIONS = { name: 'replit', displayName: 'Replit', skillsDir: '.agents/skills', - globalSkillsDir: join(configHome, 'agents/skills'), + globalSkillsDir: join(home, '.agents', 'skills'), showInUniversalList: false, detectInstalled: () => existsSync(join(process.cwd(), '.replit')), }, @@ -588,7 +588,7 @@ export const AGENT_DEFINITIONS = { name: 'universal', displayName: 'Universal', skillsDir: '.agents/skills', - globalSkillsDir: join(configHome, 'agents/skills'), + globalSkillsDir: join(home, '.agents', 'skills'), showInUniversalList: false, detectInstalled: () => false, }, diff --git a/dt-skill/src/cli/commands/publish.test.ts b/dt-skill/src/cli/commands/publish.test.ts index 9378b26..3a63b28 100644 --- a/dt-skill/src/cli/commands/publish.test.ts +++ b/dt-skill/src/cli/commands/publish.test.ts @@ -900,4 +900,108 @@ describe('cmdPublish', () => { } }); }); + + describe('cmdPublish multiple paths', () => { + it('publishes each path as an individual skill via /api/v1/skills (not import-file)', async () => { + const workdir = await makeTmpWorkdir(); + try { + await mkdir(join(workdir, 'omp-skill'), { recursive: true }); + await mkdir(join(workdir, 'zentao-api'), { recursive: true }); + await writeFile(join(workdir, 'omp-skill', 'SKILL.md'), '# OMP\n', 'utf8'); + await writeFile(join(workdir, 'zentao-api', 'SKILL.md'), '# Zentao\n', 'utf8'); + + httpMocks.apiRequest.mockRejectedValue(new Error('not found')); + httpMocks.apiRequestForm.mockResolvedValue({ + ok: true, + skillId: '1', + versionId: 'v0.0.0', + fingerprint: 'abc', + unchanged: false, + }); + + await cmdPublish(makeOpts(workdir), ['omp-skill', 'zentao-api'], { + category: '通用', + yes: true, + }); + + const v1Calls = httpMocks.apiRequestForm.mock.calls.filter((call: any[]) => { + const req = call[1] as { path?: string } | undefined; + return req?.path === '/api/v1/skills'; + }); + expect(v1Calls).toHaveLength(2); + + const importCalls = httpMocks.apiRequestForm.mock.calls.filter((call: any[]) => { + const req = call[1] as { path?: string } | undefined; + return req?.path === '/api/skills/import-file'; + }); + expect(importCalls).toHaveLength(0); + + const slugs = v1Calls.map((call: any[]) => { + const form = (call[1] as { form?: FormData }).form as FormData; + const payload = JSON.parse(String(form.get('payload'))); + return payload.slug; + }); + expect(slugs.sort()).toEqual(['omp-skill', 'zentao-api']); + } finally { + await rm(workdir, { recursive: true, force: true }); + } + }); + + it('dedups repeated slugs across paths', async () => { + const workdir = await makeTmpWorkdir(); + try { + await mkdir(join(workdir, 'omp-skill'), { recursive: true }); + await mkdir(join(workdir, 'nested', 'omp-skill'), { recursive: true }); + await writeFile(join(workdir, 'omp-skill', 'SKILL.md'), '# OMP\n', 'utf8'); + await writeFile( + join(workdir, 'nested', 'omp-skill', 'SKILL.md'), + '# OMP2\n', + 'utf8' + ); + + httpMocks.apiRequest.mockRejectedValue(new Error('not found')); + httpMocks.apiRequestForm.mockResolvedValue({ + ok: true, + skillId: '1', + versionId: 'v0.0.0', + fingerprint: 'abc', + unchanged: false, + }); + + await cmdPublish(makeOpts(workdir), ['omp-skill', 'nested/omp-skill'], { + category: '通用', + yes: true, + }); + + const v1Calls = httpMocks.apiRequestForm.mock.calls.filter((call: any[]) => { + const req = call[1] as { path?: string } | undefined; + return req?.path === '/api/v1/skills'; + }); + expect(v1Calls).toHaveLength(1); // dedup by slug + } finally { + await rm(workdir, { recursive: true, force: true }); + } + }); + + it('fails fast on invalid --category before publishing any skill', async () => { + const workdir = await makeTmpWorkdir(); + try { + await mkdir(join(workdir, 'skill-a'), { recursive: true }); + await mkdir(join(workdir, 'skill-b'), { recursive: true }); + await writeFile(join(workdir, 'skill-a', 'SKILL.md'), '# A\n', 'utf8'); + await writeFile(join(workdir, 'skill-b', 'SKILL.md'), '# B\n', 'utf8'); + + await expect( + cmdPublish(makeOpts(workdir), ['skill-a', 'skill-b'], { + category: 'not-a-category', + yes: true, + }) + ).rejects.toThrow(/--category must be one of/); + + expect(httpMocks.apiRequestForm).not.toHaveBeenCalled(); + } finally { + await rm(workdir, { recursive: true, force: true }); + } + }); + }); }); diff --git a/dt-skill/src/cli/commands/publish.ts b/dt-skill/src/cli/commands/publish.ts index 2fe5385..fb7abc9 100644 --- a/dt-skill/src/cli/commands/publish.ts +++ b/dt-skill/src/cli/commands/publish.ts @@ -49,29 +49,43 @@ function resolveContributorForPublish(): string | null { } } +export type PublishOptions = { + slug?: string; + name?: string; + owner?: string; + version?: string; + changelog?: string; + tags?: string; + forkOf?: string; + clawscanNote?: string; + migrateOwner?: boolean; + all?: boolean; + category?: string; + /** + * Optional market card summary. + * Omit: create uses SKILL.md; re-publish keeps existing card (empty → SKILL.md backfill). + */ + description?: string; + yes?: boolean; +}; + export async function cmdPublish( opts: GlobalOpts, - folderArg: string, - options: { - slug?: string; - name?: string; - owner?: string; - version?: string; - changelog?: string; - tags?: string; - forkOf?: string; - clawscanNote?: string; - migrateOwner?: boolean; - all?: boolean; - category?: string; - /** - * Optional market card summary. - * Omit: create uses SKILL.md; re-publish keeps existing card (empty → SKILL.md backfill). - */ - description?: string; - yes?: boolean; - } + folderArg: string | string[], + options: PublishOptions ) { + const folders = (Array.isArray(folderArg) ? folderArg : [folderArg]).filter(Boolean); + if (folders.length === 0) fail('Path required'); + + if (folders.length === 1) { + await cmdPublishOne(opts, folders[0] as string, options); + return; + } + + await cmdPublishMany(opts, folders, options); +} + +async function cmdPublishOne(opts: GlobalOpts, folderArg: string, options: PublishOptions) { // Resolve folder path against the project base (parent of the canonical // .agents workdir), falling back to cwd so relative paths work from anywhere. const folder = folderArg ? await resolveFolderPath(dirname(opts.workdir), folderArg) : null; @@ -92,9 +106,88 @@ export async function cmdPublish( } } + await publishSingleSkill(opts, folder, options); +} + +async function cmdPublishMany(opts: GlobalOpts, folderArgs: string[], options: PublishOptions) { + if ( + options.slug || + options.name || + options.forkOf || + options.description || + options.migrateOwner + ) { + fail('--slug/--name/--fork-of/--description/--migrate-owner require a single skill path'); + } + + const expanded: Array<{ folder: string; slug: string }> = []; + const seen = new Set(); + for (const f of folderArgs) { + const folder = await resolveFolderPath(dirname(opts.workdir), f); + const folderStat = await stat(folder).catch(() => null); + if (!folderStat || !folderStat.isDirectory()) { + fail(`Path must be a folder: ${f}`); + } + if (await looksLikePluginFolder(folder)) { + fail('This folder looks like a code plugin, not a skill. Use a folder with SKILL.md.'); + } + const found = await findSkillFolders(folder); + for (const sf of found) { + if (seen.has(sf.slug)) continue; + seen.add(sf.slug); + expanded.push({ folder: sf.folder, slug: sf.slug }); + } + } + if (expanded.length === 0) { + fail('No skills found in the given paths'); + } + + // Validate shared options once before looping, so a bad --category or + // --version fails fast instead of repeating the same error per skill. + const version = options.version?.trim() || DEFAULT_PUBLISH_VERSION; + if (!semver.valid(version)) fail('--version must be valid semver when provided'); + const category = options.category?.trim() || ''; + if (category && !SKILL_CATEGORY_SET.has(category)) { + fail(`--category must be one of: ${SKILL_CATEGORY_OPTIONS.join(', ')}`); + } + + const contributor = resolveContributorForPublish(); + + let published = 0; + let unchanged = 0; + const failed: Array<{ slug: string; error: string }> = []; + for (const sf of expanded) { + try { + const outcome = await publishSingleSkill(opts, sf.folder, options, contributor); + if (outcome === 'unchanged') unchanged += 1; + else published += 1; + } catch (error) { + failed.push({ slug: sf.slug, error: formatError(error) }); + } + } + + console.log(''); + console.log( + `Upload summary: ${published} published, ${unchanged} up to date, ${failed.length} failed` + ); + for (const f of failed) { + console.log(` failed ${f.slug}: ${f.error}`); + } + if (failed.length > 0) { + fail(`Failed to upload ${failed.length} skill(s)`); + } +} + +async function publishSingleSkill( + opts: GlobalOpts, + folder: string, + options: PublishOptions, + contributorOverride?: string | null +): Promise<'published' | 'unchanged'> { // Single skill mode (existing logic) const registry = await getRegistry(opts, { cache: true }); - const contributor = resolveContributorForPublish(); + const contributor = + contributorOverride !== undefined ? contributorOverride : resolveContributorForPublish(); // Requested identity from folder / --slug. May be rewritten to remote slug if registry // already has this skill under another primary key (installKey alias / legacy upload-*). @@ -202,7 +295,7 @@ export async function cmdPublish( ); if (!ok) { console.log('Publish cancelled'); - return; + return 'unchanged'; } spinner.start(`Publishing ${slug}`); } @@ -255,12 +348,14 @@ export async function cmdPublish( result.fingerprint ? ` (${result.fingerprint.slice(0, 12)}…)` : '' }` ); + return 'unchanged'; } else { spinner.succeed( `OK. Published ${slug}${ result.fingerprint ? ` hash=${result.fingerprint.slice(0, 12)}…` : '' } (${result.versionId})` ); + return 'published'; } } catch (error) { spinner.fail(formatError(error)); @@ -359,6 +454,7 @@ export async function cmdPublishBatch( const selected = await searchMultiselect({ message: `Select skills to publish (${discoveredSkills.length} found):`, items, + initialSelected: discoveredSkills.map((s) => s.slug), required: true, }); diff --git a/dt-skill/src/cli/commands/skills.test.ts b/dt-skill/src/cli/commands/skills.test.ts index efd5dd7..dc3d32b 100644 --- a/dt-skill/src/cli/commands/skills.test.ts +++ b/dt-skill/src/cli/commands/skills.test.ts @@ -506,6 +506,77 @@ describe('cmdUpdate', () => { expect(writeLockfile).toHaveBeenCalledTimes(1); }); + it('parses a path argument into its folder basename slug', async () => { + mockApiRequest.mockResolvedValue({ + latestVersion: { version: '2.0.0' }, + moderation: null, + }); + mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3])); + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { 'bbb-skill': { version: '1.0.0', installedAt: 123 } }, + }); + vi.mocked(writeLockfile).mockResolvedValue(); + vi.mocked(readSkillOrigin).mockResolvedValue(null); + vi.mocked(writeSkillOrigin).mockResolvedValue(); + vi.mocked(extractZipToDir).mockResolvedValue(); + vi.mocked(listTextFiles).mockResolvedValue([]); + vi.mocked(hashSkillFiles).mockReturnValue({ fingerprint: 'hash', files: [] }); + vi.mocked(stat).mockRejectedValue(new Error('missing')); + vi.mocked(rm).mockResolvedValue(); + + await cmdUpdate(makeOpts(), ['aaa/bbb-skill'], { force: true, ...projectYes }, false); + + expect(mockApiRequest).toHaveBeenCalledTimes(1); + const [, args] = mockApiRequest.mock.calls[0] ?? []; + expect(args?.path).toBe(`${ApiRoutes.skills}/${encodeURIComponent('bbb-skill')}`); + }); + + it('updates multiple slugs from a mix of slugs and paths', async () => { + mockApiRequest.mockResolvedValue({ + latestVersion: { version: '2.0.0' }, + moderation: null, + }); + mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3])); + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { + 'plain-slug': { version: '1.0.0', installedAt: 1 }, + 'path-slug': { version: '1.0.0', installedAt: 2 }, + }, + }); + vi.mocked(writeLockfile).mockResolvedValue(); + vi.mocked(readSkillOrigin).mockResolvedValue(null); + vi.mocked(writeSkillOrigin).mockResolvedValue(); + vi.mocked(extractZipToDir).mockResolvedValue(); + vi.mocked(listTextFiles).mockResolvedValue([]); + vi.mocked(hashSkillFiles).mockReturnValue({ fingerprint: 'hash', files: [] }); + vi.mocked(stat).mockRejectedValue(new Error('missing')); + vi.mocked(rm).mockResolvedValue(); + + await cmdUpdate( + makeOpts(), + ['plain-slug', 'some/dir/path-slug'], + { force: true, ...projectYes }, + false + ); + + expect(mockApiRequest).toHaveBeenCalledTimes(2); + const paths = mockApiRequest.mock.calls.map((call) => { + const [, args] = call as unknown as [unknown, { path?: string } | undefined, unknown]; + return args?.path; + }); + expect(paths).toContain(`${ApiRoutes.skills}/${encodeURIComponent('plain-slug')}`); + expect(paths).toContain(`${ApiRoutes.skills}/${encodeURIComponent('path-slug')}`); + }); + + it('rejects path arguments containing .. segments', async () => { + await expect( + cmdUpdate(makeOpts(), ['foo/../bar'], { force: true, ...projectYes }, false) + ).rejects.toThrow(/Invalid path slug/); + expect(mockApiRequest).not.toHaveBeenCalled(); + }); + it('reports up to date when local fingerprint matches skill.fingerprint', async () => { const sharedFp = 'same-content-fp-abc'; mockApiRequest.mockResolvedValue({ diff --git a/dt-skill/src/cli/commands/skills.ts b/dt-skill/src/cli/commands/skills.ts index 79b199b..ac32806 100644 --- a/dt-skill/src/cli/commands/skills.ts +++ b/dt-skill/src/cli/commands/skills.ts @@ -1,6 +1,6 @@ import { lstat, mkdir, rm } from 'node:fs/promises'; import { homedir } from 'node:os'; -import { dirname, join } from 'node:path'; +import { dirname, join, resolve } from 'node:path'; import { apiRequest, downloadZip, registryUrl } from '../../http.js'; import { @@ -28,6 +28,7 @@ import { getUniversalAgents, } from '../agents.js'; import { + getAgentSkillsDir, getCanonicalPath, getCanonicalSkillsDir, getCanonicalWorkdir, @@ -793,13 +794,12 @@ export async function cmdUninstall( /** Remove per-agent symlink/copy entries for a skill whose canonical dir was just removed. */ async function removeAgentLinks(slug: string, global: boolean, base: string) { const agentTypes = Object.keys(AGENT_DEFINITIONS) as AgentType[]; + const canonicalSkillsDir = getCanonicalSkillsDir(global, base); await Promise.all( agentTypes.map(async (agent) => { - const config = AGENT_DEFINITIONS[agent]; - if (config.skillsDir === '.agents/skills') return; // universal — lives in canonical - const agentDir = global ? config.globalSkillsDir ?? null : join(base, config.skillsDir); - if (!agentDir) return; - const linkPath = join(agentDir, slug); + const agentBase = getAgentSkillsDir(agent, global, base); + if (resolve(agentBase) === resolve(canonicalSkillsDir)) return; // lives in canonical + const linkPath = join(agentBase, slug); try { await lstat(linkPath); } catch { diff --git a/dt-skill/src/cli/commands/update.ts b/dt-skill/src/cli/commands/update.ts index c461c76..48e1cc3 100644 --- a/dt-skill/src/cli/commands/update.ts +++ b/dt-skill/src/cli/commands/update.ts @@ -1,6 +1,6 @@ import { readdir, rm } from 'node:fs/promises'; import { homedir } from 'node:os'; -import { join, resolve } from 'node:path'; +import { basename, join, resolve } from 'node:path'; import semver from 'semver'; import { apiRequest, downloadZip } from '../../http.js'; @@ -18,6 +18,7 @@ import { import { hashSkillFiles, listTextFiles, readSkillOrigin, writeSkillOrigin } from '../../skills.js'; import { getRegistry } from '../registry.js'; import { decideSkillSync, remoteCurrentFromDetail } from '../skillSync.js'; +import { sanitizeSlug } from '../slug.js'; import type { GlobalOpts } from '../types.js'; import { createSpinner, @@ -134,26 +135,49 @@ export async function resolveUpdateScope( return picked; } +function resolveUpdateSlugs(arg: string | string[] | undefined): string[] { + const raw = Array.isArray(arg) ? arg : arg ? [arg] : []; + const out: string[] = []; + const seen = new Set(); + for (const item of raw) { + const trimmed = item.trim(); + if (!trimmed) continue; + let slug: string; + if (trimmed.includes('/') || trimmed.includes('\\')) { + if (trimmed.split(/[\\/]/).includes('..')) fail(`Invalid path slug: ${trimmed}`); + slug = sanitizeSlug(basename(resolve(trimmed))); + if (!slug) fail(`Invalid path slug: ${trimmed}`); + } else { + slug = normalizeSkillSlugOrFail(trimmed); + } + if (!seen.has(slug)) { + seen.add(slug); + out.push(slug); + } + } + return out; +} + /** * Update installed skills by content hash. * Scope selection matches vercel; remote identity is skill.fingerprint. */ export async function cmdUpdate( opts: GlobalOpts, - slugArg: string | undefined, + slugArg: string | string[] | undefined, options: UpdateScopeOptions, inputAllowed: boolean ) { - const slug = slugArg ? normalizeSkillSlugOrFail(slugArg) : undefined; - if (slug && options.all) fail('Use either or --all'); - if (options.version && !slug) fail('--version requires a single '); + const slugs = resolveUpdateSlugs(slugArg); + if (slugs.length > 0 && options.all) fail('Use either or --all'); + if (options.version && slugs.length !== 1) fail('--version requires a single '); if (options.version && !semver.valid(options.version)) fail('--version must be valid semver'); const roots = getUpdateScopeRoots(opts); const scope = await resolveUpdateScope(options, { inputAllowed, projectWorkdir: roots.project.workdir, - hasSkillNames: Boolean(slug), + hasSkillNames: slugs.length > 0, }); if (scope === null) return; @@ -168,6 +192,7 @@ export async function cmdUpdate( const registry = await getRegistry(opts, { cache: true }); const allowPrompt = isInteractive() && inputAllowed && !options.yes; + const handled = new Set(); for (const scopeLabel of scopesToRun) { const { workdir, dir } = roots[scopeLabel]; const result = await updateSkillsInOneScope({ @@ -175,12 +200,16 @@ export async function cmdUpdate( installDir: dir, scopeLabel, multiScope: scopesToRun.length > 1, - slug, + slugs, options, registry, allowPrompt, }); const tag = (s: string) => (scopesToRun.length > 1 ? `${s} (${scopeLabel})` : s); + for (const s of result.updated) handled.add(s); + for (const s of result.alreadyCurrent) handled.add(s); + for (const s of result.skippedPinned) handled.add(s); + for (const f of result.failed) handled.add(f.slug); updated.push(...result.updated.map(tag)); alreadyCurrent.push(...result.alreadyCurrent.map(tag)); skippedPinned.push(...result.skippedPinned.map(tag)); @@ -189,18 +218,14 @@ export async function cmdUpdate( } } - if ( - slug && - updated.length === 0 && - alreadyCurrent.length === 0 && - skippedPinned.length === 0 && - failed.length === 0 - ) { - failed.push({ slug, error: 'not found', scope: scope === 'both' ? 'both' : scope }); + for (const slug of slugs) { + if (!handled.has(slug)) { + failed.push({ slug, error: 'not found', scope: scope === 'both' ? 'both' : scope }); + } } if ( - !slug && + slugs.length === 0 && updated.length === 0 && alreadyCurrent.length === 0 && skippedPinned.length === 0 && @@ -210,7 +235,12 @@ export async function cmdUpdate( return; } - if (skippedPinned.length > 0 && updated.length === 0 && alreadyCurrent.length === 0 && !slug) { + if ( + skippedPinned.length > 0 && + updated.length === 0 && + alreadyCurrent.length === 0 && + slugs.length === 0 + ) { console.log(`Skipped ${skippedPinned.length} pinned skill(s): ${skippedPinned.join(', ')}`); } @@ -237,7 +267,7 @@ async function updateSkillsInOneScope(args: { installDir: string; scopeLabel: string; multiScope: boolean; - slug: string | undefined; + slugs: string[]; options: UpdateScopeOptions; registry: string; allowPrompt: boolean; @@ -250,7 +280,7 @@ async function updateSkillsInOneScope(args: { const { installWorkdir, installDir, - slug, + slugs, options, registry, allowPrompt, @@ -260,26 +290,31 @@ async function updateSkillsInOneScope(args: { const lock = await readLockfile(installWorkdir); - if (slug && isPinnedSkillEntry(lock.skills[slug])) { - fail(`skill "${slug}" is pinned; run \`dt-skill unpin ${slug}\` first`); + if (slugs.length === 1) { + const only = slugs[0]; + if (isPinnedSkillEntry(lock.skills[only])) { + fail(`skill "${only}" is pinned; run \`dt-skill unpin ${only}\` first`); + } } - if (slug) { - const onDisk = await fileExists(join(installDir, slug)); - if (!lock.skills[slug] && !onDisk) { - return { updated: [], alreadyCurrent: [], skippedPinned: [], failed: [] }; + const existingSlugs: string[] = []; + if (slugs.length > 0) { + for (const s of slugs) { + const onDisk = await fileExists(join(installDir, s)); + if (lock.skills[s] || onDisk) { + existingSlugs.push(s); + } } } - const requestedSlugs = slug ? [slug] : Object.keys(lock.skills).filter(isSafeSkillSlug); - const skippedPinned = slug - ? [] - : requestedSlugs.filter((entry) => isPinnedSkillEntry(lock.skills[entry])); - const slugs = slug - ? requestedSlugs - : requestedSlugs.filter((entry) => !isPinnedSkillEntry(lock.skills[entry])); + const requestedSlugs = + slugs.length > 0 ? existingSlugs : Object.keys(lock.skills).filter(isSafeSkillSlug); + const skippedPinned = requestedSlugs.filter((entry) => isPinnedSkillEntry(lock.skills[entry])); + const slugsToProcess = requestedSlugs.filter( + (entry) => !isPinnedSkillEntry(lock.skills[entry]) + ); - if (slugs.length === 0) { + if (slugsToProcess.length === 0) { return { updated: [], alreadyCurrent: [], skippedPinned, failed: [] }; } @@ -289,7 +324,7 @@ async function updateSkillsInOneScope(args: { let lockDirty = false; const checkLabel = multiScope ? `Checking ${scopeLabel}` : 'Checking'; - for (const entry of slugs) { + for (const entry of slugsToProcess) { const spinner = createSpinner(`${checkLabel} ${entry}`); try { const target = join(installDir, entry); diff --git a/dt-skill/src/cli/installer.test.ts b/dt-skill/src/cli/installer.test.ts new file mode 100644 index 0000000..4418248 --- /dev/null +++ b/dt-skill/src/cli/installer.test.ts @@ -0,0 +1,75 @@ +/* @vitest-environment node */ +import { mkdir, mkdtemp, readlink, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { afterAll, afterEach, describe, expect, it } from 'vitest'; + +// codexHome is computed at module load from CODEX_HOME; pin it to a temp path +// so linkOrCopyToAgent writes symlinks under /tmp instead of the real ~/.codex. +const originalCodexHome = process.env.CODEX_HOME; +const CODEX_HOME = join(tmpdir(), 'dt-skill-codex-test-home'); +process.env.CODEX_HOME = CODEX_HOME; + +const { getAgentSkillsDir, getCanonicalSkillsDir, linkOrCopyToAgent } = await import( + './installer.js' +); + +afterEach(async () => { + await rm(CODEX_HOME, { recursive: true, force: true }).catch(() => {}); +}); + +afterAll(() => { + if (originalCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = originalCodexHome; +}); + +describe('getAgentSkillsDir (global codex)', () => { + it('resolves codex to ~/.codex/skills, not the canonical .agents/skills', () => { + expect(getAgentSkillsDir('codex', true, '/work')).toBe(join(CODEX_HOME, 'skills')); + expect(getAgentSkillsDir('codex', true, '/work')).not.toBe( + getCanonicalSkillsDir(true, '/work') + ); + }); + + it('still maps project-scoped codex to the canonical dir (universal)', () => { + const cwd = '/work'; + expect(getAgentSkillsDir('codex', false, cwd)).toBe(getCanonicalSkillsDir(false, cwd)); + }); + + it('keeps amp/replit/universal on the canonical global dir (pre-fix behavior)', () => { + for (const agent of ['amp', 'replit', 'universal'] as const) { + expect(getAgentSkillsDir(agent, true, '/work')).toBe( + getCanonicalSkillsDir(true, '/work') + ); + } + }); +}); + +describe('linkOrCopyToAgent (global codex symlink)', () => { + it('creates a symlink for codex instead of skipping as universal', async () => { + const workdir = await mkdtemp(join(tmpdir(), 'dt-skill-codex-wd-')); + try { + const canonicalDir = join(workdir, '.agents', 'skills', 'demo'); + await mkdir(canonicalDir, { recursive: true }); + await writeFile(join(canonicalDir, 'SKILL.md'), '# demo\n', 'utf8'); + + const result = await linkOrCopyToAgent('demo', canonicalDir, 'codex', { + global: true, + cwd: workdir, + mode: 'symlink', + }); + + const agentDir = join(CODEX_HOME, 'skills', 'demo'); + expect(result.success).toBe(true); + expect(result.skipped).toBeFalsy(); + expect(result.mode).toBe('symlink'); + expect(result.path).toBe(agentDir); + + // The symlink resolves back to the canonical skill dir. + const linkTarget = await readlink(agentDir); + expect(resolve(dirname(agentDir), linkTarget)).toBe(resolve(canonicalDir)); + } finally { + await rm(workdir, { recursive: true, force: true }).catch(() => {}); + } + }); +}); diff --git a/dt-skill/src/cli/installer.ts b/dt-skill/src/cli/installer.ts index 45d4328..497c608 100644 --- a/dt-skill/src/cli/installer.ts +++ b/dt-skill/src/cli/installer.ts @@ -73,13 +73,18 @@ export function getAgentSkillsDir( global: boolean, cwd: string = process.cwd() ): string { - if (isUniversalAgent(agentType)) { - return getCanonicalSkillsDir(global, cwd); - } const agent = AGENT_DEFINITIONS[agentType]; if (global) { - // Callers filter out agents without globalSkillsDir before reaching here. - return agent.globalSkillsDir ?? join(homedir(), agent.skillsDir); + if (agent.globalSkillsDir !== undefined) { + return agent.globalSkillsDir; + } + if (isUniversalAgent(agentType)) { + return getCanonicalSkillsDir(true, cwd); + } + return join(homedir(), agent.skillsDir); + } + if (isUniversalAgent(agentType)) { + return getCanonicalSkillsDir(false, cwd); } return join(cwd, agent.skillsDir); } @@ -171,8 +176,10 @@ export async function linkOrCopyToAgent( }; } - // Universal agents already live in the canonical dir. - if (isUniversalAgent(agentType)) { + // If the agent's install dir IS the canonical dir, no extra entry is needed. + // Universal agents share .agents/skills; non-universal agents whose global dir + // differs from canonical (e.g. codex -> ~/.codex/skills) still need a symlink. + if (resolve(agentBase) === resolve(getCanonicalSkillsDir(global, cwd))) { return { success: true, path: canonicalDir,