mirror of
https://github.com/iflytek/skillhub.git
synced 2026-08-27 11:14:59 +00:00
fix(cli): preflight canonical install targets
Signed-off-by: betterlmy <betterlmy@icloud.com>
This commit is contained in:
parent
8b84201516
commit
f519b08a73
9 changed files with 188 additions and 39 deletions
|
|
@ -160,7 +160,7 @@ The CLI determines the installation location using the following logic:
|
|||
1. If `--dir` is specified: Install to that directory, agent marked as `custom`. `--dir` is mutually exclusive with `--scope` and `--agent`.
|
||||
2. If `--scope user|project` is specified: Limit detection to the chosen scope.
|
||||
- With `--agent <profile>`: Install to that profile's user or project skills directory directly.
|
||||
- Without `--agent`: Detect existing skills directories within the chosen scope only.
|
||||
- Without `--agent`: Detect existing skills directories within the chosen scope only. In interactive user scope, the `generic` target (`<home>/.agents/skills/`) is always also offered and can be selected alone or together with detected targets.
|
||||
- No detected directory in the chosen scope → Fallback to `<home>/.agents/skills/` for `--scope user` or `<cwd>/.agents/skills/` for `--scope project`.
|
||||
3. If `--agent` is specified (no `--scope`): Install to the corresponding Agent's skills directory (existing behaviour, unchanged).
|
||||
4. If none of the above is specified:
|
||||
|
|
@ -191,7 +191,7 @@ Each Agent has both project-level and user-level skills directories. Use `--scop
|
|||
| `kilo` | `<project>/.kilo/skills/` | `~/.kilo/skills/` |
|
||||
| _fallback_ | `<project>/.agents/skills/` | `~/.agents/skills/` |
|
||||
|
||||
For Agents not in the list, use `--dir` to specify the installation path. When `--scope user|project` finds no matching agent directory, the CLI falls back to the `_fallback_` row above.
|
||||
For a custom path or an unsupported Agent directory, use `--dir` to specify the installation path. In interactive user scope, the `generic` target is offered alongside detected Agent targets. When `--scope user|project` finds no matching agent directory, the CLI falls back to the `_fallback_` row above.
|
||||
|
||||
### File Structure After Installation
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { homedir } from 'node:os'
|
||||
import { CliError } from '../shared/errors'
|
||||
import { EXIT } from '../shared/constants'
|
||||
import { pathExists } from '../platform/paths'
|
||||
import { canonicalizeExistingPath, pathExists } from '../platform/paths'
|
||||
import type { AgentCandidate } from './types'
|
||||
import { allProfiles, profileMap } from './detector'
|
||||
|
||||
|
|
@ -66,10 +66,10 @@ async function resolveScopedTargets(
|
|||
} else {
|
||||
candidates = await generateScopedCandidates(scope, options.cwd, scopedHome)
|
||||
}
|
||||
candidates = dedupeByRoot(candidates)
|
||||
candidates = await dedupeByRoot(candidates)
|
||||
|
||||
if (scope === 'user' && agentList.length === 0 && options.interactive && !options.json) {
|
||||
candidates = dedupeByRoot([
|
||||
candidates = await dedupeByRoot([
|
||||
...candidates,
|
||||
{
|
||||
agent: 'generic',
|
||||
|
|
@ -161,13 +161,18 @@ async function resolveExplicitAgents(
|
|||
return results
|
||||
}
|
||||
|
||||
function dedupeByRoot(candidates: AgentCandidate[]): AgentCandidate[] {
|
||||
async function dedupeByRoot(candidates: AgentCandidate[]): Promise<AgentCandidate[]> {
|
||||
const seen = new Set<string>()
|
||||
return candidates.filter(c => {
|
||||
if (seen.has(c.rootDir)) return false
|
||||
seen.add(c.rootDir)
|
||||
return true
|
||||
})
|
||||
const deduped: AgentCandidate[] = []
|
||||
|
||||
for (const candidate of candidates) {
|
||||
const canonicalRootDir = await canonicalizeExistingPath(candidate.rootDir)
|
||||
if (seen.has(canonicalRootDir)) continue
|
||||
seen.add(canonicalRootDir)
|
||||
deduped.push(candidate)
|
||||
}
|
||||
|
||||
return deduped
|
||||
}
|
||||
|
||||
async function selectTargetsInteractively(candidates: AgentCandidate[]): Promise<AgentCandidate[]> {
|
||||
|
|
|
|||
|
|
@ -24,6 +24,15 @@ export async function pathExists(path: string): Promise<boolean> {
|
|||
}
|
||||
}
|
||||
|
||||
export async function canonicalizeExistingPath(path: string): Promise<string> {
|
||||
const { realpath } = await import('node:fs/promises')
|
||||
try {
|
||||
return await realpath(path)
|
||||
} catch {
|
||||
return path
|
||||
}
|
||||
}
|
||||
|
||||
export async function applyCredentialPermissions(path: string): Promise<void> {
|
||||
if (process.platform === 'win32') return
|
||||
const { chmod } = await import('node:fs/promises')
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import { CliError } from '../shared/errors'
|
|||
import { EXIT } from '../shared/constants'
|
||||
import { extractZip } from '../platform/archive'
|
||||
import { readBoundedResponseBody } from '../platform/download'
|
||||
import { pathExists } from '../platform/paths'
|
||||
import { canonicalizeExistingPath, pathExists } from '../platform/paths'
|
||||
import type { AgentCandidate } from '../agents/types'
|
||||
|
||||
export interface InstallOptions {
|
||||
|
|
@ -20,7 +20,40 @@ export interface InstallOptions {
|
|||
home?: string | undefined
|
||||
}
|
||||
|
||||
async function preflightInstallTargets(
|
||||
targets: AgentCandidate[],
|
||||
slug: string,
|
||||
force: boolean
|
||||
): Promise<Array<{ target: AgentCandidate; skillDir: string }>> {
|
||||
const seenSkillDirs = new Set<string>()
|
||||
const preparedTargets: Array<{ target: AgentCandidate; skillDir: string }> = []
|
||||
|
||||
for (const target of targets) {
|
||||
const canonicalRootDir = await canonicalizeExistingPath(target.rootDir)
|
||||
const canonicalSkillDir = join(canonicalRootDir, slug)
|
||||
if (seenSkillDirs.has(canonicalSkillDir)) {
|
||||
throw new CliError(`multiple install targets resolve to ${canonicalSkillDir}`, EXIT.usage, {
|
||||
path: canonicalSkillDir,
|
||||
next: 'select only one target for this directory'
|
||||
})
|
||||
}
|
||||
seenSkillDirs.add(canonicalSkillDir)
|
||||
|
||||
const skillDir = join(target.rootDir, slug)
|
||||
if (await pathExists(skillDir) && !force) {
|
||||
throw new CliError(`skill already installed at ${skillDir}`, EXIT.filesystem, {
|
||||
path: skillDir,
|
||||
next: 'pass --force to overwrite'
|
||||
})
|
||||
}
|
||||
preparedTargets.push({ target, skillDir })
|
||||
}
|
||||
|
||||
return preparedTargets
|
||||
}
|
||||
|
||||
export async function installSkill(options: InstallOptions): Promise<{ installed: Array<{ agent: string; dir: string }> }> {
|
||||
const preparedTargets = await preflightInstallTargets(options.targets, options.slug, options.force)
|
||||
const client = new SkillHubClient(options.registry, options.token)
|
||||
const resolved = await client.resolve(options.namespace, options.slug, options.version)
|
||||
const response = await client.download(options.namespace, options.slug, resolved.version)
|
||||
|
|
@ -29,16 +62,7 @@ export async function installSkill(options: InstallOptions): Promise<{ installed
|
|||
const installed: Array<{ agent: string; dir: string }> = []
|
||||
const store = new InventoryStore(options.home)
|
||||
|
||||
for (const target of options.targets) {
|
||||
const skillDir = join(target.rootDir, options.slug)
|
||||
|
||||
if (await pathExists(skillDir) && !options.force) {
|
||||
throw new CliError(`skill already installed at ${skillDir}`, EXIT.filesystem, {
|
||||
path: skillDir,
|
||||
next: 'pass --force to overwrite'
|
||||
})
|
||||
}
|
||||
|
||||
for (const { target, skillDir } of preparedTargets) {
|
||||
await mkdir(target.rootDir, { recursive: true })
|
||||
const tempDir = await mkdtemp(join(target.rootDir, `.${options.slug}.install-`))
|
||||
let movedIntoPlace = false
|
||||
|
|
|
|||
|
|
@ -1,18 +1,30 @@
|
|||
import { describe, expect, mock, test } from 'bun:test'
|
||||
import { afterEach, describe, expect, mock, test } from 'bun:test'
|
||||
import type { AgentCandidate } from '../../../src/agents/types'
|
||||
|
||||
interface PromptChoice {
|
||||
value: AgentCandidate
|
||||
}
|
||||
|
||||
interface PromptOptions {
|
||||
choices?: PromptChoice[]
|
||||
onRender?: (this: { cursor?: number }) => void
|
||||
format?: (selectedTargets: AgentCandidate[]) => AgentCandidate[]
|
||||
}
|
||||
|
||||
const defaultSelectedTargets = (options: PromptOptions): AgentCandidate[] => options.format?.([]) ?? []
|
||||
let selectPromptTargets = defaultSelectedTargets
|
||||
|
||||
mock.module('prompts', () => ({
|
||||
default: (options: PromptOptions) => {
|
||||
options.onRender?.call({ cursor: 1 })
|
||||
return { selected: options.format?.([]) ?? [] }
|
||||
return { selected: selectPromptTargets(options) }
|
||||
}
|
||||
}))
|
||||
|
||||
afterEach(() => {
|
||||
selectPromptTargets = defaultSelectedTargets
|
||||
})
|
||||
|
||||
const { resolveInstallTargets } = await import('../../../src/agents/resolver')
|
||||
|
||||
describe('resolveInstallTargets interactive prompt', () => {
|
||||
|
|
@ -34,7 +46,21 @@ describe('resolveInstallTargets interactive prompt', () => {
|
|||
expect(targets).toEqual([highlighted])
|
||||
})
|
||||
|
||||
test('offers the generic user target alongside detected agent targets', async () => {
|
||||
test('allows selecting generic alongside detected user targets', async () => {
|
||||
selectPromptTargets = options => options.choices?.map(choice => choice.value) ?? []
|
||||
const codex: AgentCandidate = {
|
||||
agent: 'codex',
|
||||
rootDir: '/home/u/.codex/skills',
|
||||
scope: 'user',
|
||||
source: 'detected'
|
||||
}
|
||||
const generic: AgentCandidate = {
|
||||
agent: 'generic',
|
||||
rootDir: '/home/u/.agents/skills',
|
||||
scope: 'user',
|
||||
source: 'fallback'
|
||||
}
|
||||
|
||||
const targets = await resolveInstallTargets({
|
||||
cwd: '/repo',
|
||||
home: '/home/u',
|
||||
|
|
@ -42,16 +68,9 @@ describe('resolveInstallTargets interactive prompt', () => {
|
|||
scope: 'user',
|
||||
json: false,
|
||||
interactive: true,
|
||||
detected: [
|
||||
{ agent: 'codex', rootDir: '/home/u/.codex/skills', scope: 'user', source: 'detected' }
|
||||
]
|
||||
detected: [codex]
|
||||
})
|
||||
|
||||
expect(targets).toEqual([{
|
||||
agent: 'generic',
|
||||
rootDir: '/home/u/.agents/skills',
|
||||
scope: 'user',
|
||||
source: 'fallback'
|
||||
}])
|
||||
expect(targets).toEqual([codex, generic])
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
import { mkdir, mkdtemp, rm, symlink } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
import { resolveInstallTargets } from '../../../src/agents/resolver'
|
||||
import type { AgentCandidate } from '../../../src/agents/types'
|
||||
|
||||
describe('resolveInstallTargets', () => {
|
||||
test('rejects dir and agent together before filesystem writes', async () => {
|
||||
|
|
@ -216,4 +220,36 @@ describe('resolveInstallTargets', () => {
|
|||
expect(targets[0]!.rootDir).toBe('/home/u/.codex/skills')
|
||||
expect(targets[0]!.scope).toBe('user')
|
||||
})
|
||||
|
||||
test('deduplicates a symlinked detected target and the generic user target', async () => {
|
||||
const home = await mkdtemp(join(tmpdir(), 'skillhub-resolver-home-'))
|
||||
const genericRoot = join(home, '.agents', 'skills')
|
||||
const codexRoot = join(home, '.codex', 'skills')
|
||||
const codex: AgentCandidate = {
|
||||
agent: 'codex',
|
||||
rootDir: codexRoot,
|
||||
scope: 'user',
|
||||
source: 'detected'
|
||||
}
|
||||
|
||||
try {
|
||||
await mkdir(genericRoot, { recursive: true })
|
||||
await mkdir(join(home, '.codex'), { recursive: true })
|
||||
await symlink(genericRoot, codexRoot, process.platform === 'win32' ? 'junction' : 'dir')
|
||||
|
||||
const targets = await resolveInstallTargets({
|
||||
cwd: '/repo',
|
||||
home,
|
||||
agents: [],
|
||||
scope: 'user',
|
||||
json: false,
|
||||
interactive: true,
|
||||
detected: [codex]
|
||||
})
|
||||
|
||||
expect(targets).toEqual([codex])
|
||||
} finally {
|
||||
await rm(home, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { access, mkdir, mkdtemp, readFile, writeFile } from 'node:fs/promises'
|
||||
import { access, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, test } from 'bun:test'
|
||||
|
|
@ -72,6 +72,62 @@ describe('installSkill', () => {
|
|||
})).rejects.toThrow('skill already installed')
|
||||
})
|
||||
|
||||
test('preflights all targets before writing when a later target is occupied', async () => {
|
||||
globalThis.fetch = installFetch({ 'SKILL.md': '# Demo' })
|
||||
const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-'))
|
||||
const firstRoot = await mkdtemp(join(tmpdir(), 'skillhub-install-first-root-'))
|
||||
const secondRoot = await mkdtemp(join(tmpdir(), 'skillhub-install-second-root-'))
|
||||
const firstSkillDir = join(firstRoot, 'demo')
|
||||
const secondSkillDir = join(secondRoot, 'demo')
|
||||
await mkdir(secondSkillDir, { recursive: true })
|
||||
|
||||
await expect(installSkill({
|
||||
registry: 'http://registry.test',
|
||||
namespace: 'global',
|
||||
slug: 'demo',
|
||||
targets: [
|
||||
{ agent: 'codex', rootDir: firstRoot, scope: 'project', source: 'explicit' },
|
||||
{ agent: 'claude-code', rootDir: secondRoot, scope: 'project', source: 'explicit' }
|
||||
],
|
||||
force: false,
|
||||
home
|
||||
})).rejects.toThrow(`skill already installed at ${secondSkillDir}`)
|
||||
|
||||
expect(await exists(firstSkillDir)).toBe(false)
|
||||
expect(await exists(join(home, '.skillhub', 'inventory.json'))).toBe(false)
|
||||
})
|
||||
|
||||
test('rejects canonical target aliases before writing any installation', async () => {
|
||||
globalThis.fetch = installFetch({ 'SKILL.md': '# Demo' })
|
||||
const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-'))
|
||||
const targetParent = await mkdtemp(join(tmpdir(), 'skillhub-install-targets-'))
|
||||
const genericRoot = join(targetParent, 'generic')
|
||||
const codexRoot = join(targetParent, 'codex')
|
||||
const skillDir = join(genericRoot, 'demo')
|
||||
try {
|
||||
await mkdir(genericRoot, { recursive: true })
|
||||
await symlink(genericRoot, codexRoot, process.platform === 'win32' ? 'junction' : 'dir')
|
||||
|
||||
await expect(installSkill({
|
||||
registry: 'http://registry.test',
|
||||
namespace: 'global',
|
||||
slug: 'demo',
|
||||
targets: [
|
||||
{ agent: 'codex', rootDir: codexRoot, scope: 'user', source: 'detected' },
|
||||
{ agent: 'generic', rootDir: genericRoot, scope: 'user', source: 'fallback' }
|
||||
],
|
||||
force: false,
|
||||
home
|
||||
})).rejects.toThrow('multiple install targets resolve to')
|
||||
|
||||
expect(await exists(skillDir)).toBe(false)
|
||||
expect(await exists(join(home, '.skillhub', 'inventory.json'))).toBe(false)
|
||||
} finally {
|
||||
await rm(home, { recursive: true, force: true })
|
||||
await rm(targetParent, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('force replaces the old skill directory instead of overlaying files', async () => {
|
||||
globalThis.fetch = installFetch({ 'SKILL.md': '# New' })
|
||||
const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-'))
|
||||
|
|
|
|||
|
|
@ -157,7 +157,7 @@ The CLI determines the installation location using the following logic:
|
|||
1. If `--dir` is specified: Install to that directory, agent marked as `custom`. `--dir` is mutually exclusive with `--scope` and `--agent`.
|
||||
2. If `--scope user|project` is specified: Limit detection to the chosen scope.
|
||||
- With `--agent <profile>`: Install to that profile's user or project skills directory directly.
|
||||
- Without `--agent`: Detect existing skills directories within the chosen scope only.
|
||||
- Without `--agent`: Detect existing skills directories within the chosen scope only. In interactive user scope, the `generic` target (`<home>/.agents/skills/`) is always also offered and can be selected alone or together with detected targets.
|
||||
- No detected directory in the chosen scope → Fallback to `<home>/.agents/skills/` for `--scope user` or `<cwd>/.agents/skills/` for `--scope project`.
|
||||
3. If `--agent` is specified (no `--scope`): Install to the corresponding Agent's skills directory (existing behaviour, unchanged).
|
||||
4. If none of the above is specified:
|
||||
|
|
@ -188,7 +188,7 @@ Each Agent has both project-level and user-level skills directories. Use `--scop
|
|||
| `kilo` | `<project>/.kilo/skills/` | `~/.kilo/skills/` |
|
||||
| _fallback_ | `<project>/.agents/skills/` | `~/.agents/skills/` |
|
||||
|
||||
For Agents not in the list, use `--dir` to specify the installation path. When `--scope user|project` finds no matching agent directory, the CLI falls back to the `_fallback_` row above.
|
||||
For a custom path or an unsupported Agent directory, use `--dir` to specify the installation path. In interactive user scope, the `generic` target is offered alongside detected Agent targets. When `--scope user|project` finds no matching agent directory, the CLI falls back to the `_fallback_` row above.
|
||||
|
||||
### File Structure After Installation
|
||||
|
||||
|
|
|
|||
|
|
@ -157,7 +157,7 @@ CLI 按以下逻辑确定安装位置:
|
|||
1. 指定 `--dir`:安装到该目录,agent 标记为 `custom`。`--dir` 与 `--scope`、`--agent` 互斥。
|
||||
2. 指定 `--scope user|project`:探测限定在该 scope 内。
|
||||
- 同时指定 `--agent <profile>`:直接安装到该 profile 对应 scope 的 skills 目录。
|
||||
- 未指定 `--agent`:只探测该 scope 下已存在的 skills 目录。
|
||||
- 未指定 `--agent`:只探测该 scope 下已存在的 skills 目录。在交互式 user scope 下,始终额外提供 `generic` 目标(`<home>/.agents/skills/`),可单独选择或与已探测目标同时选择。
|
||||
- 该 scope 下未探测到 → fallback:`--scope user` 回退到 `<home>/.agents/skills/`,`--scope project` 回退到 `<cwd>/.agents/skills/`。
|
||||
3. 指定 `--agent`(无 `--scope`):安装到对应 Agent 的 skills 目录(沿用现有行为,不变)。
|
||||
4. 三者均未指定:
|
||||
|
|
@ -188,7 +188,7 @@ CLI 按以下逻辑确定安装位置:
|
|||
| `kilo` | `<project>/.kilo/skills/` | `~/.kilo/skills/` |
|
||||
| _fallback_ | `<project>/.agents/skills/` | `~/.agents/skills/` |
|
||||
|
||||
对于不在列表中的 Agent,使用 `--dir` 指定安装路径。当 `--scope user|project` 找不到匹配的 agent 目录时,CLI 会回退到上表的 `_fallback_` 行。
|
||||
对于自定义路径或不在列表中的 Agent 目录,使用 `--dir` 显式指定安装路径。交互式 user scope 下会与已探测 Agent 目标一同提供 `generic` 目标;当 `--scope user|project` 找不到匹配的 agent 目录时,CLI 会回退到上表的 `_fallback_` 行。
|
||||
|
||||
### 安装后的文件结构
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue