Merge pull request #608 from iflytek/fix/cli-namespace-errors
Some checks are pending
Deploy Docs / build (push) Waiting to run
Deploy Docs / Deploy (push) Blocked by required conditions
Security / Dependency Review (push) Waiting to run
Security / CodeQL (java-kotlin) (push) Waiting to run
Security / CodeQL (javascript-typescript) (push) Waiting to run
Security / CodeQL (python) (push) Waiting to run

fix(cli): normalize namespace coordinates and errors
This commit is contained in:
XiaoSeS 2026-07-29 11:08:15 +08:00 • committed by GitHub
commit bafb9fe3b9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
25 changed files with 1351 additions and 206 deletions

20
cli/CHANGELOG.md Normal file
View file

@ -0,0 +1,20 @@
# Changelog
All notable CLI behavior changes are documented in this file.
## Unreleased
### Fixed
- Resolve `namespace/slug`, `@namespace/slug`, and `namespace--slug`
coordinates against their declared namespace instead of silently falling
back to `global`.
- Reject a namespaced coordinate combined with a conflicting `--namespace`
value; a matching value remains valid.
- Limit local removal with a namespaced coordinate or explicit `--namespace`
to the matching namespace, preventing collateral deletion of same-slug
installations in other namespaces. Bare-slug removal retains its existing
cross-namespace behavior for compatibility.
- Preserve public registry `msg` and `requestId` fields for unsuccessful
responses. HTTP 403 without a public message now reports the neutral
`access denied` fallback instead of assuming the token lacks scope.

View file

@ -126,15 +126,34 @@ Output format: `namespace/slug version summary`
## 📥 Install Skills
The install coordinate accepts a bare slug or any of the equivalent namespace
forms below:
| Coordinate | Resolved namespace | Resolved slug |
|------------|--------------------|---------------|
| `my-skill` | `global` | `my-skill` |
| `team/my-skill` | `team` | `my-skill` |
| `@team/my-skill` | `team` | `my-skill` |
| `team--my-skill` | `team` | `my-skill` |
For a bare slug, `--namespace team` selects a non-global namespace. A
namespaced coordinate may be combined with the same `--namespace` value, but a
conflicting value is rejected instead of silently overriding the coordinate.
```bash
# Install to auto-detected Agent directory
skillhub install pdf-parser
# Equivalent namespaced coordinates
skillhub install team/my-skill
skillhub install @team/my-skill
skillhub install team--my-skill
# Choose install scope explicitly
skillhub install pdf-parser --scope user
skillhub install pdf-parser --scope project --agent codex
# Specify namespace (default: global)
# Specify namespace for a bare slug (default: global)
skillhub install pdf-parser --namespace myspace
# Specify version
@ -239,9 +258,17 @@ skillhub list --json
### Remove Skills
```bash
# Remove all local installation targets
# A bare slug removes matching local installations across namespaces
skillhub remove pdf-parser
# A namespaced coordinate removes only that namespace
skillhub remove myspace/pdf-parser
skillhub remove @myspace/pdf-parser
skillhub remove myspace--pdf-parser
# Equivalent precise local removal with an explicit namespace
skillhub remove pdf-parser --namespace myspace
# Remove only specific Agent's installation
skillhub remove pdf-parser --agent codex
@ -337,9 +364,9 @@ Update mechanism:
| `skillhub logout [--registry <url>] [--json]` | Remove token for specified registry |
| `skillhub whoami [--registry <url>] [--token <token>] [--json]` | Validate current token and display user information |
| `skillhub search <query> [--registry <url>] [--token <token>] [--limit <n>] [--json]` | Search published skills |
| `skillhub install <slug> [--scope <user\|project>] [--namespace <slug>] [--version <v>] [--agent <profile>] [--dir <path>] [--force] [--registry <url>] [--token <token>] [--json]` | Install a skill |
| `skillhub install <coordinate> [--scope <user\|project>] [--namespace <slug>] [--version <v>] [--agent <profile>] [--dir <path>] [--force] [--registry <url>] [--token <token>] [--json]` | Install a skill |
| `skillhub list [--agent <profile>] [--dir <path>] [--registry <url>] [--json]` | List installed skills |
| `skillhub remove <slug> [--agent <profile>] [--all] [--remote] [--hard] [--namespace <slug>] [--registry <url>] [--token <token>] [--json]` | Remove a skill |
| `skillhub remove <coordinate> [--agent <profile>] [--all] [--remote] [--hard] [--namespace <slug>] [--registry <url>] [--token <token>] [--json]` | Remove a skill |
| `skillhub doctor [--json]` | Scan project directory and rebuild local inventory |
| `skillhub publish <path> [--namespace <slug>] [--visibility <v>] [--registry <url>] [--token <token>] [--json]` | Publish a skill |
| `skillhub update [--check] [--json]` | Check or execute CLI self-update |
@ -364,6 +391,11 @@ skillhub whoami
skillhub login --token sk_xxx
```
For structured registry failures, the CLI prints the server's public `msg` and
`requestId`. HTTP 403 without a public message falls back to `access denied`;
it is not automatically described as a missing token scope. Include the
request ID when asking a registry operator to investigate.
### Network Error
```bash

View file

@ -28,6 +28,7 @@
"files": [
"dist",
"README.md",
"CHANGELOG.md",
"LICENSE"
],
"scripts": {

View file

@ -52,11 +52,13 @@ export interface DryRunResponse {
resolvedVersion: string | null
}
interface ErrorEnvelope {
msg?: unknown
requestId?: unknown
interface PublicErrorFields {
msg?: string
requestId?: string
}
type ErrorResponseKind = 'json' | 'download'
export class SkillHubClient {
constructor(
readonly registry: string,
@ -93,17 +95,8 @@ export class SkillHubClient {
} catch {
throw new CliError('registry unreachable', EXIT.network, { registry: this.registry, next: 'check network or pass --registry' })
}
if (response.status === 401) {
throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' })
}
if (response.status === 403) {
throw await this.createAccessDeniedError(response)
}
if (response.status === 404) {
throw new CliError('skill or version not found', EXIT.generic, { registry: this.registry })
}
if (!response.ok) {
throw new CliError(`download failed with status ${response.status}`, EXIT.generic, { registry: this.registry })
throw await this.createResponseError(response, 'download')
}
return response
}
@ -159,45 +152,65 @@ export class SkillHubClient {
}
private async handleJsonResponse<T>(response: Response): Promise<T> {
if (response.status === 401) {
throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' })
}
if (response.status === 403) {
throw await this.createAccessDeniedError(response)
}
if (response.status === 404) {
throw new CliError('resource not found', EXIT.generic, { registry: this.registry })
}
// 502/503 indicate network-level failures (connection refused, service unavailable)
if (response.status === 502 || response.status === 503) {
throw new CliError(`registry returned ${response.status}`, EXIT.network, { registry: this.registry })
}
if (!response.ok) {
const text = await response.text().catch(() => '')
throw new CliError(`registry returned ${response.status}`, EXIT.generic, { registry: this.registry, detail: text })
throw await this.createResponseError(response, 'json')
}
const body = await response.json()
return body.data as T
}
private async createAccessDeniedError(response: Response): Promise<CliError> {
const error = await this.readErrorEnvelope(response)
return new CliError(error.message ?? 'access denied', EXIT.auth, {
registry: this.registry,
...(error.requestId ? { requestId: error.requestId } : {})
})
private async createResponseError(response: Response, kind: ErrorResponseKind): Promise<CliError> {
const publicFields = await this.readPublicErrorFields(response)
const details: Record<string, unknown> = { registry: this.registry }
if (publicFields.requestId) {
details.requestId = publicFields.requestId
}
let fallback: string
let exitCode: number = EXIT.generic
if (response.status === 401) {
fallback = 'authentication failed'
exitCode = EXIT.auth
details.next = 'run `skillhub login`'
} else if (response.status === 403) {
fallback = 'access denied'
exitCode = EXIT.auth
} else if (response.status === 404) {
fallback = kind === 'download' ? 'skill or version not found' : 'resource not found'
} else if (response.status === 502 || response.status === 503) {
fallback = kind === 'download'
? `download failed with status ${response.status}`
: `registry returned ${response.status}`
exitCode = EXIT.network
} else {
fallback = kind === 'download'
? `download failed with status ${response.status}`
: `registry returned ${response.status}`
}
return new CliError(publicFields.msg ?? fallback, exitCode, details)
}
private async readErrorEnvelope(response: Response): Promise<{ message?: string; requestId?: string }> {
private async readPublicErrorFields(response: Response): Promise<PublicErrorFields> {
let body: unknown
try {
const body = await response.json() as ErrorEnvelope
return {
...(typeof body.msg === 'string' && body.msg.trim() ? { message: body.msg } : {}),
...(typeof body.requestId === 'string' && body.requestId.trim() ? { requestId: body.requestId } : {})
}
body = await response.json()
} catch {
return {}
}
if (typeof body !== 'object' || body === null || Array.isArray(body)) {
return {}
}
const record = body as Record<string, unknown>
const msg = typeof record.msg === 'string' ? record.msg.trim() : ''
const requestId = typeof record.requestId === 'string' ? record.requestId.trim() : ''
return {
...(msg ? { msg } : {}),
...(requestId ? { requestId } : {})
}
}
private headers(): HeadersInit {

View file

@ -33,9 +33,12 @@ export const commands = {
},
install: {
summary: 'Install a skill locally',
usage: 'skillhub install <slug> [--scope <user|project>] [--namespace <slug>] [--version <v>] [--agent <profile>] [--dir <path>] [--force] [--json]',
usage: 'skillhub install <coordinate> [--scope <user|project>] [--namespace <slug>] [--version <v>] [--agent <profile>] [--dir <path>] [--force] [--json]',
examples: [
'skillhub install pdf-parser',
'skillhub install team/my-skill',
'skillhub install @team/my-skill',
'skillhub install team--my-skill',
'skillhub install pdf-parser --scope user',
'skillhub install pdf-parser --scope project --agent codex'
]
@ -47,8 +50,13 @@ export const commands = {
},
remove: {
summary: 'Remove local or remote skill',
usage: 'skillhub remove <slug> [--agent <profile>] [--all] [--remote] [--hard] [--namespace <slug>] [--json]',
examples: ['skillhub remove pdf-parser', 'skillhub remove pdf-parser --remote --hard']
usage: 'skillhub remove <coordinate> [--agent <profile>] [--all] [--remote] [--hard] [--namespace <slug>] [--json]',
examples: [
'skillhub remove pdf-parser',
'skillhub remove team/my-skill',
'skillhub remove my-skill --namespace team',
'skillhub remove pdf-parser --remote --hard'
]
},
doctor: {
summary: 'Scan project and merge into local inventory (preserves entries outside scan scope)',

View file

@ -5,7 +5,7 @@ import { installSkill } from '../services/install-service'
import { resolveInstallTargets } from '../agents/resolver'
import { CliError } from '../shared/errors'
import { EXIT } from '../shared/constants'
import { parseSkillName } from '../shared/skill-name-parser'
import { resolveSkillName } from '../shared/skill-name-parser'
export interface InstallCommandOptions {
namespace?: string | undefined
@ -94,9 +94,7 @@ export async function installCommand(
const registry = resolveRegistry(options, process.env, await configStore.read())
const token = resolveToken(options, process.env, await credentialsStore.getToken(registry))
const parsed = parseSkillName(skillNameArg)
const namespace = options.namespace ?? parsed.namespace
const slug = parsed.slug
const { namespace, slug } = resolveSkillName(skillNameArg, options.namespace)
const resolveTargets = deps.resolveInstallTargets ?? resolveInstallTargets
const targets = await resolveTargets({

View file

@ -5,7 +5,7 @@ import { resolveRegistry, resolveToken } from '../services/registry-service'
import { removeLocalSkill } from '../services/remove-service'
import { CliError } from '../shared/errors'
import { EXIT } from '../shared/constants'
import { parseSkillName } from '../shared/skill-name-parser'
import { hasExplicitNamespace, resolveSkillName } from '../shared/skill-name-parser'
export interface RemoveCommandOptions {
agent?: string[] | undefined
@ -30,9 +30,7 @@ export async function removeCommand(skillNameArg: string, options: RemoveCommand
const credentialsStore = new CredentialsStore()
const registry = resolveRegistry(options, process.env, await configStore.read())
const parsed = parseSkillName(skillNameArg)
const namespace = options.namespace ?? parsed.namespace
const slug = parsed.slug
const { namespace, slug } = resolveSkillName(skillNameArg, options.namespace)
if (options.remote) {
const token = resolveToken(options, process.env, await credentialsStore.getToken(registry))
@ -62,8 +60,13 @@ export async function removeCommand(skillNameArg: string, options: RemoveCommand
}
// Local remove
const namespaceFilter = options.namespace !== undefined || hasExplicitNamespace(skillNameArg)
? namespace
: undefined
const result = await removeLocalSkill({
registry, slug,
registry,
namespace: namespaceFilter,
slug,
agents: options.agent,
all: options.all
})

View file

@ -231,8 +231,8 @@ cli
})
cli
.command('install <slug>', 'Install a skill locally')
.option('--namespace <slug>', 'Namespace', { default: 'global' })
.command('install <coordinate>', 'Install a skill locally')
.option('--namespace <slug>', 'Namespace for a bare skill slug')
.option('--version <v>', 'Version')
.option('--scope <scope>', 'Install scope: user or project')
.option('--agent <profile>', 'Agent profile (repeatable)')
@ -256,17 +256,17 @@ cli
})
cli
.command('remove <slug>', 'Remove local or remote skill')
.command('remove <coordinate>', 'Remove local or remote skill')
.option('--agent <profile>', 'Filter by agent (repeatable)')
.option('--all', 'Remove all targets')
.option('--remote', 'Delete remote skill')
.option('--hard', 'Skip confirmation for remote delete')
.option('--namespace <slug>', 'Namespace for remote delete')
.option('--namespace <slug>', 'Namespace for local or remote delete')
.option('--registry <url>', 'Registry URL')
.option('--token <token>', 'API token')
.option('--json', 'Output JSON')
.action((slug: string, options: RemoveCommandOptions & { agent?: string | string[] }) => {
return runCommand(() => removeCommand(slug, { ...options, agent: toArray(options.agent) }), Boolean(options.json))
.action((coordinate: string, options: RemoveCommandOptions & { agent?: string | string[] }) => {
return runCommand(() => removeCommand(coordinate, { ...options, agent: toArray(options.agent) }), Boolean(options.json))
})
cli

View file

@ -15,6 +15,7 @@ function isPathUnder(child: string, parent: string): boolean {
export interface RemoveLocalOptions {
registry: string
namespace?: string | undefined
slug: string
agents?: string[] | undefined
all?: boolean | undefined
@ -29,7 +30,11 @@ export async function removeLocalSkill(options: RemoveLocalOptions): Promise<Rem
const store = new InventoryStore(options.home)
const inventory = await store.read()
const items = inventory.items.filter(i => i.registry === options.registry && i.slug === options.slug)
const items = inventory.items.filter(item =>
item.registry === options.registry &&
item.slug === options.slug &&
(options.namespace === undefined || item.namespace === options.namespace)
)
if (items.length === 0) {
throw new CliError(`skill not found locally: ${options.slug}`, EXIT.generic, {
next: 'run `skillhub list` to see installed skills'

View file

@ -1,27 +1,91 @@
import { EXIT } from './constants'
import { CliError } from './errors'
export interface ParsedSkillName {
namespace: string
slug: string
}
export function parseSkillName(skillName: string, defaultNamespace = 'global'): ParsedSkillName {
const separatorIndex = skillName.indexOf('--')
interface ParsedCoordinate {
namespace?: string
slug: string
}
if (separatorIndex <= 0) {
return {
namespace: defaultNamespace,
slug: separatorIndex === 0 ? skillName.slice(2) : skillName
}
function invalidCoordinate(skillName: string): CliError {
return new CliError(`invalid skill coordinate "${skillName}"`, EXIT.usage)
}
function parseSeparatedCoordinate(
skillName: string,
separatorIndex: number,
separatorLength: number,
namespaceStart = 0
): ParsedCoordinate {
const namespace = skillName.slice(namespaceStart, separatorIndex)
const slug = skillName.slice(separatorIndex + separatorLength)
if (!namespace || !slug || slug.includes('/')) {
throw invalidCoordinate(skillName)
}
if (separatorIndex === skillName.length - 2) {
return {
namespace: defaultNamespace,
slug: skillName.slice(0, -2)
return { namespace, slug }
}
function parseCoordinate(skillName: string): ParsedCoordinate {
if (!skillName) {
throw invalidCoordinate(skillName)
}
const slashIndex = skillName.indexOf('/')
if (skillName.startsWith('@')) {
if (slashIndex < 0) {
throw invalidCoordinate(skillName)
}
return parseSeparatedCoordinate(skillName, slashIndex, 1, 1)
}
const doubleDashIndex = skillName.indexOf('--')
if (slashIndex >= 0 && (doubleDashIndex < 0 || slashIndex < doubleDashIndex)) {
return parseSeparatedCoordinate(skillName, slashIndex, 1)
}
if (doubleDashIndex >= 0) {
return parseSeparatedCoordinate(skillName, doubleDashIndex, 2)
}
return { slug: skillName }
}
export function parseSkillName(skillName: string, defaultNamespace = 'global'): ParsedSkillName {
const parsed = parseCoordinate(skillName)
return {
namespace: parsed.namespace ?? defaultNamespace,
slug: parsed.slug
}
}
/** Return whether a skill coordinate explicitly includes a namespace. */
export function hasExplicitNamespace(skillName: string): boolean {
return parseCoordinate(skillName).namespace !== undefined
}
/** Resolve a skill coordinate and an optional command-line namespace into one registry identity. */
export function resolveSkillName(skillName: string, explicitNamespace?: string): ParsedSkillName {
const parsed = parseCoordinate(skillName)
if (
parsed.namespace !== undefined &&
explicitNamespace !== undefined &&
parsed.namespace !== explicitNamespace
) {
throw new CliError(
`skill coordinate namespace "${parsed.namespace}" conflicts with --namespace "${explicitNamespace}"`,
EXIT.usage
)
}
return {
namespace: skillName.slice(0, separatorIndex),
slug: skillName.slice(separatorIndex + 2)
namespace: parsed.namespace ?? explicitNamespace ?? 'global',
slug: parsed.slug
}
}

View file

@ -23,11 +23,12 @@ export function createFakeRegistry(handlers: Record<string, FakeHandler>) {
* Controls how a specific endpoint behaves when a failure is injected:
* 'auth' => 401 { code: 401, message: 'unauthorized' }
* 'forbidden' => 403 with a standard SkillHub error envelope
* 'forbidden_unstructured' => 403 with a non-JSON response body
* 'not_found' => 404 { code: 404, message: 'not found' }
* 'server_error' => 500 { code: 500, message: 'internal error' }
* 'network' => handler throws, causing fetch() to reject with a TypeError
*/
export type FailureMode = 'auth' | 'forbidden' | 'not_found' | 'server_error' | 'network'
export type FailureMode = 'auth' | 'forbidden' | 'forbidden_unstructured' | 'not_found' | 'server_error' | 'network'
function failureResponse(mode: FailureMode): Response {
switch (mode) {
@ -39,6 +40,11 @@ function failureResponse(mode: FailureMode): Response {
msg: 'API token is missing required scope: skill:publish',
requestId: 'req-test-forbidden'
}, { status: 403 })
case 'forbidden_unstructured':
return new Response('<html>sensitive proxy denial</html>', {
status: 403,
headers: { 'Content-Type': 'text/html' }
})
case 'not_found':
return Response.json({ code: 404, message: 'not found' }, { status: 404 })
case 'server_error':

View file

@ -67,7 +67,7 @@ describe('cli error output', () => {
expect(result.exitCode).toBe(5)
expect(result.stderr).toContain('Error: missing required argument')
expect(result.stderr).toContain('Usage: skillhub install <slug>')
expect(result.stderr).toContain('Usage: skillhub install <coordinate>')
expect(result.stderr).toContain('Run "skillhub help install" for more information.')
})

View file

@ -5,8 +5,26 @@ describe('help command', () => {
test('prints detailed help for install', async () => {
const result = await runCli(['help', 'install'])
expect(result.exitCode).toBe(0)
expect(result.stdout).toContain('Usage: skillhub install <slug>')
expect(result.stdout).toContain('Usage: skillhub install <coordinate>')
expect(result.stdout).toContain('--agent <profile>')
expect(result.stdout).toContain('@team/my-skill')
expect(result.stdout).toContain('team/my-skill')
expect(result.stdout).toContain('team--my-skill')
})
test('prints namespaced local remove contract in command help', async () => {
const result = await runCli(['help', 'remove'])
expect(result.exitCode).toBe(0)
expect(result.stdout).toContain('Usage: skillhub remove <coordinate>')
expect(result.stdout).toContain('skillhub remove team/my-skill')
expect(result.stdout).toContain('skillhub remove my-skill --namespace team')
})
test('prints namespaced local remove contract in --help', async () => {
const result = await runCli(['remove', '--help'])
expect(result.exitCode).toBe(0)
expect(result.stdout).toContain('remove <coordinate>')
expect(result.stdout).toContain('Namespace for local or remote delete')
})
test('prints search help with optional query', async () => {

View file

@ -324,6 +324,83 @@ describe('install command — P1', () => {
expect(meta.version).toBe('2.0.0')
})
test.each([
'team/my-skill',
'@team/my-skill',
'team--my-skill'
])('%s resolves the namespaced registry path', async (coordinate) => {
const env = await createTempHome()
registry = await startFakeRegistry({
token: 'sk_ok',
skills: [{
namespace: 'team',
slug: 'my-skill',
version: '1.0.0',
zipBytes: makeSkillZip()
}]
})
const installDir = join(env.cwd, 'skills-coordinate')
await mkdir(installDir, { recursive: true })
const result = await runCli(
[
'install', coordinate,
'--dir', installDir,
'--registry', registry.url,
'--token', 'sk_ok',
'--json'
],
{ HOME: env.home, USERPROFILE: env.home }
)
expect(result.exitCode).toBe(0)
expect(JSON.parse(result.stdout)).toMatchObject({
ok: true,
namespace: 'team',
slug: 'my-skill'
})
expect(registry.received.resolve).toMatchObject({
namespace: 'team',
slug: 'my-skill'
})
})
test('coordinate conflicting with --namespace fails before registry access', async () => {
const env = await createTempHome()
registry = await startFakeRegistry({
token: 'sk_ok',
skills: [{
namespace: 'team',
slug: 'my-skill',
version: '1.0.0',
zipBytes: makeSkillZip()
}]
})
const installDir = join(env.cwd, 'skills-coordinate-conflict')
await mkdir(installDir, { recursive: true })
const result = await runCli(
[
'install', '@team/my-skill',
'--namespace', 'other',
'--dir', installDir,
'--registry', registry.url,
'--token', 'sk_ok',
'--json'
],
{ HOME: env.home, USERPROFILE: env.home }
)
expect(result.exitCode).toBe(5)
expect(JSON.parse(result.stderr)).toMatchObject({
ok: false,
exitCode: 5
})
expect(registry.received.resolve).toBeNull()
})
// -------------------------------------------------------------------------
// NOTE: multi-target interactive selection (TTY branch) is not tested here
// because Bun.spawn does not support PTY allocation. The interactive path

View file

@ -159,7 +159,7 @@ describe('publish --dry-run', () => {
expect(result.stderr).toContain('authentication')
})
test('--dry-run reports scope error on 403', async () => {
test('--dry-run surfaces the public message and request ID on a structured 403', async () => {
const env = await createTempHome()
registry = await startFakeRegistry({ token: 'sk_ok', failures: { validate: 'forbidden' } })
await login(env, registry.url)
@ -174,4 +174,20 @@ describe('publish --dry-run', () => {
expect(result.stderr).toContain('scope')
expect(result.stderr).toContain('Request ID: req-test-forbidden')
})
test('--dry-run uses a neutral fallback without leaking an unstructured 403 body', async () => {
const env = await createTempHome()
registry = await startFakeRegistry({ token: 'sk_ok', failures: { validate: 'forbidden_unstructured' } })
await login(env, registry.url)
const dir = await makeTempDir(['SKILL.md', '---\nname: test\ndescription: test\n---\n'])
const result = await runCli(['publish', dir, '--dry-run', '--registry', registry.url], {
HOME: env.home,
USERPROFILE: env.home
})
expect(result.exitCode).toBe(2)
expect(result.stderr).toContain('access denied')
expect(result.stderr).not.toContain('sensitive proxy denial')
})
})

View file

@ -1,5 +1,5 @@
import { afterEach, describe, expect, test } from 'bun:test'
import { mkdir, writeFile } from 'node:fs/promises'
import { access, mkdir, writeFile } from 'node:fs/promises'
import { createTempHome } from '../helpers/temp-env'
import { startFakeRegistry } from '../helpers/fake-registry'
import { runCli } from '../helpers/run-cli'
@ -27,6 +27,15 @@ async function createInstallDir(path: string) {
await mkdir(path, { recursive: true })
}
async function pathExists(path: string): Promise<boolean> {
try {
await access(path)
return true
} catch {
return false
}
}
/** Build a minimal inventory item with one target. */
function makeItem(opts: {
registry: string
@ -388,43 +397,131 @@ describe('remove command — local remove (P1)', () => {
expect(survived!.targets.map(t => t.agent)).toEqual(['claude-code'])
})
// -------------------------------------------------------------------------
// P1: --agent + --namespace together filter precisely so a same-slug skill
// in a different namespace is not collateral damage.
// -------------------------------------------------------------------------
test('--agent + --namespace filters precisely; same slug under different namespace is untouched', async () => {
const namespacedRemoveCases: Array<[string, string[]]> = [
['namespace/slug coordinate', ['team/shared-skill']],
['@namespace/slug coordinate', ['@team/shared-skill']],
['namespace--slug coordinate', ['team--shared-skill']],
['--namespace flag', ['shared-skill', '--namespace', 'team']]
]
test.each(namespacedRemoveCases)('%s only removes the targeted same-slug namespace', async (_label, removeArgs) => {
const env = await createTempHome()
registry = await startFakeRegistry({ token: 'sk_ok' })
const rootDir = `${env.home}/agents`
const aDir = `${rootDir}/codex/skills/dup-slug-A`
const bDir = `${rootDir}/codex/skills/dup-slug-B`
await createInstallDir(aDir)
await createInstallDir(bDir)
const globalDir = `${rootDir}/codex/skills/shared-skill`
const teamDir = `${rootDir}/claude-code/skills/shared-skill`
const otherDir = `${rootDir}/cursor/skills/shared-skill`
await createInstallDir(globalDir)
await createInstallDir(teamDir)
await createInstallDir(otherDir)
await seedInventory(env.home, [
{
registry: registry.url, namespace: 'team-a', slug: 'dup-slug-A', version: '1.0.0',
targets: [{ agent: 'codex', rootDir: `${rootDir}/codex`, installDir: aDir, installedAt: '2026-04-20T00:00:00Z' }]
},
{
registry: registry.url, namespace: 'team-b', slug: 'dup-slug-B', version: '1.0.0',
targets: [{ agent: 'codex', rootDir: `${rootDir}/codex`, installDir: bDir, installedAt: '2026-04-20T00:00:00Z' }]
}
makeItem({
registry: registry.url,
namespace: 'global',
slug: 'shared-skill',
agent: 'codex',
rootDir: `${rootDir}/codex`,
installDir: globalDir
}),
makeItem({
registry: registry.url,
namespace: 'team',
slug: 'shared-skill',
agent: 'claude-code',
rootDir: `${rootDir}/claude-code`,
installDir: teamDir
}),
makeItem({
registry: registry.url,
namespace: 'other',
slug: 'shared-skill',
agent: 'cursor',
rootDir: `${rootDir}/cursor`,
installDir: otherDir
})
])
// Remove dup-slug-A only — dup-slug-B should survive even though both
// share the codex agent.
const result = await runCli(
['remove', 'dup-slug-A', '--agent', 'codex', '--registry', registry.url],
['remove', ...removeArgs, '--registry', registry.url, '--json'],
{ HOME: env.home, USERPROFILE: env.home }
)
expect(result.exitCode).toBe(0)
const parsed = JSON.parse(result.stdout)
expect(parsed.removed).toHaveLength(1)
expect(parsed.removed[0]).toMatchObject({ namespace: 'team', agent: 'claude-code' })
expect(await pathExists(globalDir)).toBe(true)
expect(await pathExists(teamDir)).toBe(false)
expect(await pathExists(otherDir)).toBe(true)
const inv = JSON.parse(await Bun.file(`${env.home}/.skillhub/inventory.json`).text()) as {
items: Array<{ slug: string }>
items: Array<{ namespace: string; slug: string; targets: Array<{ installDir: string }> }>
}
const slugs = inv.items.map(i => i.slug).sort()
expect(slugs).toEqual(['dup-slug-B'])
expect(inv.items.map(item => item.namespace).sort()).toEqual(['global', 'other'])
expect(inv.items.every(item => item.slug === 'shared-skill')).toBe(true)
expect(inv.items.map(item => item.targets[0]?.installDir).sort()).toEqual([globalDir, otherDir].sort())
})
test('bare slug retains cross-namespace local removal compatibility', async () => {
const env = await createTempHome()
registry = await startFakeRegistry({ token: 'sk_ok' })
const rootDir = `${env.home}/agents`
const globalDir = `${rootDir}/codex/skills/shared-skill`
const teamDir = `${rootDir}/claude-code/skills/shared-skill`
const otherDir = `${rootDir}/cursor/skills/shared-skill`
await createInstallDir(globalDir)
await createInstallDir(teamDir)
await createInstallDir(otherDir)
await seedInventory(env.home, [
makeItem({
registry: registry.url,
namespace: 'global',
slug: 'shared-skill',
agent: 'codex',
rootDir: `${rootDir}/codex`,
installDir: globalDir
}),
makeItem({
registry: registry.url,
namespace: 'team',
slug: 'shared-skill',
agent: 'claude-code',
rootDir: `${rootDir}/claude-code`,
installDir: teamDir
}),
makeItem({
registry: registry.url,
namespace: 'other',
slug: 'shared-skill',
agent: 'cursor',
rootDir: `${rootDir}/cursor`,
installDir: otherDir
})
])
const result = await runCli(
['remove', 'shared-skill', '--registry', registry.url, '--json'],
{ HOME: env.home, USERPROFILE: env.home }
)
expect(result.exitCode).toBe(0)
const parsed = JSON.parse(result.stdout)
expect(parsed.removed.map((item: { namespace: string }) => item.namespace).sort()).toEqual([
'global',
'other',
'team'
])
expect(await pathExists(globalDir)).toBe(false)
expect(await pathExists(teamDir)).toBe(false)
expect(await pathExists(otherDir)).toBe(false)
const inv = JSON.parse(await Bun.file(`${env.home}/.skillhub/inventory.json`).text()) as {
items: object[]
}
expect(inv.items).toEqual([])
})
})

View file

@ -36,7 +36,7 @@ describe('SkillHubClient', () => {
await err.toHaveProperty('exitCode', EXIT.auth)
})
test('download() throws auth error on 403', async () => {
test('download() preserves the server reason and request ID on 403', async () => {
const fetchImpl = (async () => Response.json({
code: 403,
msg: 'API token is missing required scope: skill:read',
@ -54,6 +54,17 @@ describe('SkillHubClient', () => {
})
})
test('download() uses a neutral access error for an unstructured 403', async () => {
const fetchImpl = (async () => new Response(null, { status: 403 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
await expect(client.download('ns', 'slug')).rejects.toMatchObject({
message: 'access denied',
exitCode: EXIT.auth,
details: { registry: 'http://registry.test' }
})
})
test('download() throws not-found error on 404', async () => {
const fetchImpl = (async () => new Response(null, { status: 404 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
@ -81,6 +92,17 @@ describe('SkillHubClient', () => {
await err.toHaveProperty('exitCode', EXIT.generic)
})
test('download() retains its fallback while classifying 502 as a network error', async () => {
const fetchImpl = (async () => new Response(null, { status: 502 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
await expect(client.download('ns', 'slug')).rejects.toMatchObject({
message: 'download failed with status 502',
exitCode: EXIT.network,
details: { registry: 'http://registry.test' }
})
})
test('download() throws network error on fetch failure', async () => {
const fetchImpl = (async () => { throw new TypeError('fetch failed') }) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
@ -168,6 +190,143 @@ describe('SkillHubClient', () => {
// --- handleJsonResponse() non-2xx classification ---
test('whoami() preserves public fields and ignores unknown fields on a structured 401', async () => {
const fetchImpl = (async () => Response.json({
code: 401,
msg: 'token has been revoked',
requestId: 'req-401',
detail: 'internal token state',
stack: 'internal stack trace'
}, { status: 401 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
const error = await client.whoami().catch((caught: unknown) => caught)
expect(error).toBeInstanceOf(CliError)
expect((error as CliError).message).toBe('token has been revoked')
expect((error as CliError).exitCode).toBe(EXIT.auth)
expect((error as CliError).details).toEqual({
registry: 'http://registry.test',
requestId: 'req-401',
next: 'run `skillhub login`'
})
})
test('search() preserves a public 403 message and request ID', async () => {
const fetchImpl = (async () => Response.json({
code: 403,
msg: 'token has been revoked',
requestId: 'req-403'
}, { status: 403 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
try {
await client.search('test', 20)
throw new Error('expected search to fail')
} catch (error) {
expect(error).toBeInstanceOf(CliError)
expect((error as CliError).message).toBe('token has been revoked')
expect((error as CliError).exitCode).toBe(EXIT.auth)
expect((error as CliError).details).toEqual({
registry: 'http://registry.test',
requestId: 'req-403'
})
}
})
test('search() uses a neutral 403 fallback when msg is absent', async () => {
const fetchImpl = (async () => Response.json({
code: 403,
requestId: 'req-fallback'
}, { status: 403 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
try {
await client.search('test', 20)
throw new Error('expected search to fail')
} catch (error) {
expect(error).toBeInstanceOf(CliError)
expect((error as CliError).message).toBe('access denied')
expect((error as CliError).exitCode).toBe(EXIT.auth)
expect((error as CliError).details).toEqual({
registry: 'http://registry.test',
requestId: 'req-fallback'
})
}
})
test('search() uses a neutral 403 fallback for a non-JSON body', async () => {
const fetchImpl = (async () => new Response('<html>forbidden</html>', {
status: 403,
headers: { 'Content-Type': 'text/html' }
})) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
try {
await client.search('test', 20)
throw new Error('expected search to fail')
} catch (error) {
expect(error).toBeInstanceOf(CliError)
expect((error as CliError).message).toBe('access denied')
expect((error as CliError).exitCode).toBe(EXIT.auth)
expect((error as CliError).details).toEqual({ registry: 'http://registry.test' })
}
})
test('whoami() preserves a structured 404 message and request ID', async () => {
const fetchImpl = (async () => Response.json({
code: 404,
msg: 'namespace not found',
requestId: 'req-404'
}, { status: 404 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
try {
await client.whoami()
throw new Error('expected whoami to fail')
} catch (error) {
expect(error).toBeInstanceOf(CliError)
expect((error as CliError).message).toBe('namespace not found')
expect((error as CliError).exitCode).toBe(EXIT.generic)
expect((error as CliError).details).toEqual({
registry: 'http://registry.test',
requestId: 'req-404'
})
}
})
test('whoami() uses the resource fallback on an unstructured 404', async () => {
const fetchImpl = (async () => new Response(null, { status: 404 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
await expect(client.whoami()).rejects.toMatchObject({
message: 'resource not found',
exitCode: EXIT.generic,
details: { registry: 'http://registry.test' }
})
})
test('download() preserves a structured 403 message and request ID', async () => {
const fetchImpl = (async () => Response.json({
code: 403,
msg: 'namespace access denied',
requestId: 'req-download'
}, { status: 403 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
try {
await client.download('team', 'private-skill')
throw new Error('expected download to fail')
} catch (error) {
expect(error).toBeInstanceOf(CliError)
expect((error as CliError).message).toBe('namespace access denied')
expect((error as CliError).exitCode).toBe(EXIT.auth)
expect((error as CliError).details).toEqual({
registry: 'http://registry.test',
requestId: 'req-download'
})
}
})
test('whoami() surfaces server reason and request ID on 403', async () => {
const fetchImpl = (async () => Response.json({
code: 403,
@ -197,20 +356,63 @@ describe('SkillHubClient', () => {
})
})
test('whoami() throws generic error on 500', async () => {
const fetchImpl = (async () => new Response(null, { status: 500 })) as unknown as typeof fetch
test('whoami() preserves public fields on a structured 500', async () => {
const fetchImpl = (async () => Response.json({
code: 500,
msg: 'registry operation failed',
requestId: 'req-500',
detail: 'internal database error'
}, { status: 500 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
const err = expect(client.whoami()).rejects
await err.toBeInstanceOf(CliError)
await err.toHaveProperty('exitCode', EXIT.generic)
await expect(client.whoami()).rejects.toMatchObject({
message: 'registry operation failed',
exitCode: EXIT.generic,
details: {
registry: 'http://registry.test',
requestId: 'req-500'
}
})
})
test('search() throws network error on 502', async () => {
test('whoami() does not expose a raw non-JSON 500 body', async () => {
const fetchImpl = (async () => new Response('internal stack trace', { status: 500 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
await expect(client.whoami()).rejects.toMatchObject({
message: 'registry returned 500',
exitCode: EXIT.generic,
details: { registry: 'http://registry.test' }
})
})
test('search() preserves public fields and network classification on a structured 502', async () => {
const fetchImpl = (async () => Response.json({
code: 502,
msg: 'registry upstream unavailable',
requestId: 'req-502'
}, { status: 502 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
await expect(client.search('test', 20)).rejects.toMatchObject({
message: 'registry upstream unavailable',
exitCode: EXIT.network,
details: {
registry: 'http://registry.test',
requestId: 'req-502'
}
})
})
test('search() uses the network fallback on an unstructured 502', async () => {
const fetchImpl = (async () => new Response(null, { status: 502 })) as unknown as typeof fetch
const client = new SkillHubClient('http://registry.test', 'token', fetchImpl)
const err = expect(client.search('test', 20)).rejects
await err.toBeInstanceOf(CliError)
await err.toHaveProperty('exitCode', EXIT.network)
await expect(client.search('test', 20)).rejects.toMatchObject({
message: 'registry returned 502',
exitCode: EXIT.network,
details: { registry: 'http://registry.test' }
})
})
// --- deleteRemote() (P1) ---

View file

@ -1,5 +1,6 @@
import { describe, expect, test } from 'bun:test'
import { CliError } from '../../../src/shared/errors'
import { EXIT } from '../../../src/shared/constants'
import {
computeStrictIsTTY,
installCommand,
@ -136,6 +137,63 @@ describe('installCommand dependency injection', () => {
return async () => ({ installed: [{ agent: 'codex', dir: '/home/u/.codex/skills/foo' }] })
}
function fakeResolveInstallTargets(): NonNullable<InstallCommandDeps['resolveInstallTargets']> {
return async () => [{
agent: 'codex',
rootDir: '/home/u/.codex/skills',
scope: 'user',
source: 'explicit'
}] as AgentCandidate[]
}
test('passes a namespaced coordinate to installSkill', async () => {
let received: Parameters<NonNullable<InstallCommandDeps['installSkill']>>[0] | undefined
const deps: InstallCommandDeps = {
isTTY: () => false,
resolveInstallTargets: fakeResolveInstallTargets(),
installSkill: async (options) => {
received = options
return { installed: [{ agent: 'codex', dir: '/home/u/.codex/skills/my-skill' }] }
}
}
await installCommand('@team/my-skill', {
registry: 'http://localhost',
token: 'sk'
}, deps)
expect(received).toMatchObject({
namespace: 'team',
slug: 'my-skill'
})
})
test('rejects a conflicting namespace before installing', async () => {
let installCalls = 0
let error: unknown
const deps: InstallCommandDeps = {
isTTY: () => false,
resolveInstallTargets: fakeResolveInstallTargets(),
installSkill: async () => {
installCalls += 1
return { installed: [] }
}
}
try {
await installCommand('@team/my-skill', {
namespace: 'other',
registry: 'http://localhost',
token: 'sk'
}, deps)
} catch (caught) {
error = caught
}
expect(error).toBeInstanceOf(CliError)
expect((error as CliError).exitCode).toBe(EXIT.usage)
expect(installCalls).toBe(0)
})
test('passes prompted scope and strict isTTY into resolveInstallTargets', async () => {
const calls: { promptScope: number; resolverCalls: ResolveInstallTargetOptions[] } = {
promptScope: 0,

View file

@ -15,7 +15,7 @@ async function exists(path: string): Promise<boolean> {
}
describe('removeLocalSkill', () => {
test('removes all current-registry installs with the same slug across namespaces', async () => {
test('bare slug removes all current-registry installs with the same slug across namespaces', async () => {
const home = await mkdtemp(join(tmpdir(), 'skillhub-remove-home-'))
const root = await mkdtemp(join(tmpdir(), 'skillhub-remove-root-'))
const globalDir = join(root, 'codex', 'demo')
@ -51,6 +51,47 @@ describe('removeLocalSkill', () => {
expect((await store.read()).items).toEqual([])
})
test('namespace filter removes only the matching same-slug install', async () => {
const home = await mkdtemp(join(tmpdir(), 'skillhub-remove-home-'))
const root = await mkdtemp(join(tmpdir(), 'skillhub-remove-root-'))
const globalDir = join(root, 'codex', 'demo')
const teamDir = join(root, 'claude', 'demo')
await mkdir(globalDir, { recursive: true })
await mkdir(teamDir, { recursive: true })
const store = new InventoryStore(home)
await store.write({
items: [
{
registry: 'https://skill.xfyun.cn',
namespace: 'global',
slug: 'demo',
version: '1.0.0',
targets: [{ agent: 'codex', rootDir: join(root, 'codex'), installDir: globalDir, installedAt: '2026-04-20T00:00:00Z' }]
},
{
registry: 'https://skill.xfyun.cn',
namespace: 'team',
slug: 'demo',
version: '1.0.0',
targets: [{ agent: 'claude-code', rootDir: join(root, 'claude'), installDir: teamDir, installedAt: '2026-04-20T00:00:00Z' }]
}
]
})
const result = await removeLocalSkill({
registry: 'https://skill.xfyun.cn',
namespace: 'team',
slug: 'demo',
home
})
expect(result.removed.map(item => item.namespace)).toEqual(['team'])
expect(await exists(globalDir)).toBe(true)
expect(await exists(teamDir)).toBe(false)
expect((await store.read()).items.map(item => item.namespace)).toEqual(['global'])
})
test('throws on path traversal in installDir', async () => {
const home = await mkdtemp(join(tmpdir(), 'skillhub-remove-traversal-'))

View file

@ -26,6 +26,18 @@ describe('renderError', () => {
'Next: check network or pass --registry'
].join('\n'))
})
test('renders a server request ID for human-readable errors', () => {
const error = new CliError('token has been revoked', 2, {
registry: 'https://registry.example.com',
requestId: 'req-403'
})
expect(renderError(error, false)).toBe([
'Error: token has been revoked',
'Context: registry https://registry.example.com',
'Request ID: req-403'
].join('\n'))
})
})
describe('printResult', () => {

View file

@ -1,90 +1,85 @@
import { describe, test, expect } from 'bun:test'
import { parseSkillName } from '../../../src/shared/skill-name-parser'
import { describe, expect, test } from 'bun:test'
import { parseSkillName, resolveSkillName } from '../../../src/shared/skill-name-parser'
import { EXIT } from '../../../src/shared/constants'
import { CliError } from '../../../src/shared/errors'
function expectUsageError(callback: () => unknown): void {
let error: unknown
try {
callback()
} catch (caught) {
error = caught
}
expect(error).toBeInstanceOf(CliError)
expect((error as CliError).exitCode).toBe(EXIT.usage)
}
describe('parseSkillName', () => {
describe('with namespace--slug format', () => {
test('should parse namespace and slug separated by double dash', () => {
const result = parseSkillName('astroclaw--api-gateway')
expect(result).toEqual({
namespace: 'astroclaw',
slug: 'api-gateway'
})
})
test.each([
['my-skill', { namespace: 'global', slug: 'my-skill' }],
['team/my-skill', { namespace: 'team', slug: 'my-skill' }],
['@team/my-skill', { namespace: 'team', slug: 'my-skill' }],
['team--my-skill', { namespace: 'team', slug: 'my-skill' }]
])('parses %s', (skillName, expected) => {
expect(parseSkillName(skillName)).toEqual(expected)
})
test('should handle namespace and slug with single dashes', () => {
const result = parseSkillName('my-org--my-skill-name')
expect(result).toEqual({
namespace: 'my-org',
slug: 'my-skill-name'
})
})
test('should handle multiple double dashes by using first as separator', () => {
const result = parseSkillName('namespace--slug--with--dashes')
expect(result).toEqual({
namespace: 'namespace',
slug: 'slug--with--dashes'
})
test('preserves double dashes after the coordinate separator', () => {
expect(parseSkillName('namespace--slug--with--dashes')).toEqual({
namespace: 'namespace',
slug: 'slug--with--dashes'
})
})
describe('with slug only format', () => {
test('should use default namespace when no separator present', () => {
const result = parseSkillName('api-gateway')
expect(result).toEqual({
namespace: 'global',
slug: 'api-gateway'
})
})
test('should use custom default namespace when provided', () => {
const result = parseSkillName('api-gateway', 'myorg')
expect(result).toEqual({
namespace: 'myorg',
slug: 'api-gateway'
})
})
test('should handle slug with single dashes', () => {
const result = parseSkillName('my-skill-name')
expect(result).toEqual({
namespace: 'global',
slug: 'my-skill-name'
})
test('preserves the custom default namespace for a bare slug', () => {
expect(parseSkillName('api-gateway', 'myorg')).toEqual({
namespace: 'myorg',
slug: 'api-gateway'
})
})
describe('edge cases', () => {
test('should handle separator at start', () => {
const result = parseSkillName('--api-gateway')
expect(result).toEqual({
namespace: 'global',
slug: 'api-gateway'
})
})
test('should handle separator at end', () => {
const result = parseSkillName('astroclaw--')
expect(result).toEqual({
namespace: 'global',
slug: 'astroclaw'
})
})
test('should handle empty string', () => {
const result = parseSkillName('')
expect(result).toEqual({
namespace: 'global',
slug: ''
})
})
test('should handle just separator', () => {
const result = parseSkillName('--')
expect(result).toEqual({
namespace: 'global',
slug: ''
})
})
test.each([
'',
'@team',
'team/',
'/my-skill',
'--my-skill',
'team--',
'team/my-skill/extra',
'@team/my-skill/extra',
'team--my-skill/extra'
])('rejects malformed coordinate %p', (skillName) => {
expectUsageError(() => parseSkillName(skillName))
})
})
describe('resolveSkillName', () => {
test('uses global for a bare slug without an explicit namespace', () => {
expect(resolveSkillName('my-skill')).toEqual({
namespace: 'global',
slug: 'my-skill'
})
})
test('uses an explicit namespace for a bare slug', () => {
expect(resolveSkillName('my-skill', 'team')).toEqual({
namespace: 'team',
slug: 'my-skill'
})
})
test.each([
'team/my-skill',
'@team/my-skill',
'team--my-skill'
])('accepts matching --namespace for %s', (skillName) => {
expect(resolveSkillName(skillName, 'team')).toEqual({
namespace: 'team',
slug: 'my-skill'
})
})
test('rejects a coordinate that conflicts with --namespace', () => {
expectUsageError(() => resolveSkillName('@team/my-skill', 'other'))
})
})

View file

@ -125,15 +125,24 @@ Output format: `namespace/slug version summary`
## Install Skills
Install coordinates accept a bare slug (resolved to `global` by default) and
three equivalent explicit namespace forms. When an explicit coordinate and
`--namespace` are both present, they must match.
```bash
# Install to auto-detected Agent directory
skillhub install pdf-parser
# Equivalent namespace coordinates
skillhub install team/my-skill
skillhub install @team/my-skill
skillhub install team--my-skill
# Choose install scope explicitly
skillhub install pdf-parser --scope user
skillhub install pdf-parser --scope project --agent codex
# Specify namespace (default: global)
# Specify a namespace for a bare slug
skillhub install pdf-parser --namespace myspace
# Specify version
@ -238,9 +247,17 @@ skillhub list --json
### Remove Skills
```bash
# Remove all local installation targets
# A bare slug removes same-named local installations across namespaces
skillhub remove pdf-parser
# An explicit namespaced coordinate removes only that namespace
skillhub remove myspace/pdf-parser
skillhub remove @myspace/pdf-parser
skillhub remove myspace--pdf-parser
# Equivalent precise local removal with an explicit namespace
skillhub remove pdf-parser --namespace myspace
# Remove only specific Agent's installation
skillhub remove pdf-parser --agent codex
@ -472,12 +489,18 @@ Search published skills.
### install
```bash
skillhub install <slug> [options]
skillhub install <coordinate> [options]
```
`<coordinate>` accepts a bare slug (`my-skill`, resolved as `global/my-skill`)
or any of the equivalent explicit namespace forms: `team/my-skill`,
`@team/my-skill`, and `team--my-skill`. Use `--namespace team` to select a
non-global namespace for a bare slug. An explicit coordinate may be combined
with the same `--namespace`; a conflicting value is rejected as a usage error.
Options:
- `--scope <user|project>` — Install scope (omit for interactive prompt in TTY, or fall back to existing detection in non-TTY)
- `--namespace <slug>` — Namespace (default: `global`)
- `--namespace <slug>` — Namespace for a bare slug
- `--version <v>` — Version (default: latest)
- `--agent <profile>` — Agent profile (repeatable)
- `--dir <path>` — Custom installation directory (mutually exclusive with `--scope` and `--agent`)
@ -501,7 +524,7 @@ Options:
### remove
```bash
skillhub remove <slug> [options]
skillhub remove <coordinate> [options]
```
Options:
@ -509,11 +532,16 @@ Options:
- `--all` — Remove all targets
- `--remote` — Remove remote skill
- `--hard` — Skip remote deletion confirmation
- `--namespace <slug>` — Namespace for remote deletion
- `--namespace <slug>` — Namespace for local or remote deletion
- `--registry <url>` — Registry URL
- `--token <token>` — API token
- `--json` — JSON output
An explicit namespaced coordinate (`team/my-skill`, `@team/my-skill`, or
`team--my-skill`) or `--namespace team` removes local installations only from
that namespace. For compatibility, a bare slug removes same-named local
installations across all namespaces in the current registry.
### doctor
```bash

View file

@ -125,15 +125,23 @@ skillhub search pdf --json
## 安装技能
安装坐标支持裸 slug(默认解析到 `global`)和三种等价的显式 namespace
形式。显式坐标与 `--namespace` 同时出现时,两者必须一致。
```bash
# 安装到自动探测的 Agent 目录
skillhub install pdf-parser
# 等价的 namespace 坐标
skillhub install team/my-skill
skillhub install @team/my-skill
skillhub install team--my-skill
# 显式指定安装范围
skillhub install pdf-parser --scope user
skillhub install pdf-parser --scope project --agent codex
# 指定 namespace(默认 global)
# 为裸 slug 指定 namespace
skillhub install pdf-parser --namespace myspace
# 指定版本
@ -238,9 +246,17 @@ skillhub list --json
### 删除技能
```bash
# 删除所有本地安装目标
# 裸 slug 删除所有 namespace 中的同名本地安装
skillhub remove pdf-parser
# 显式 namespace 坐标只删除该 namespace
skillhub remove myspace/pdf-parser
skillhub remove @myspace/pdf-parser
skillhub remove myspace--pdf-parser
# 使用 namespace 参数进行等价的精确本地删除
skillhub remove pdf-parser --namespace myspace
# 只删除指定 Agent 的安装
skillhub remove pdf-parser --agent codex
@ -472,12 +488,17 @@ skillhub search <query> [--registry <url>] [--limit <n>] [--json]
### install
```bash
skillhub install <slug> [options]
skillhub install <coordinate> [options]
```
`<coordinate>` 支持裸 slug(`my-skill`,解析为 `global/my-skill`)以及
`team/my-skill`、`@team/my-skill`、`team--my-skill` 三种等价的显式
namespace 形式。裸 slug 可通过 `--namespace team` 选择非 global namespace;
显式坐标可以同时传入相同的 `--namespace`,但冲突值会作为用法错误被拒绝。
选项:
- `--scope <user|project>` — 安装范围(不传时:TTY 模式下交互式询问,非 TTY 模式沿用现有探测逻辑)
- `--namespace <slug>` — namespace(默认 `global`)
- `--namespace <slug>` — 为裸 slug 指定 namespace
- `--version <v>` — 版本(默认最新版本)
- `--agent <profile>` — Agent 配置(可重复)
- `--dir <path>` — 自定义安装目录(与 `--scope`、`--agent` 互斥)
@ -501,7 +522,7 @@ skillhub list [options]
### remove
```bash
skillhub remove <slug> [options]
skillhub remove <coordinate> [options]
```
选项:
@ -509,11 +530,15 @@ skillhub remove <slug> [options]
- `--all` — 删除所有目标
- `--remote` — 删除远程技能
- `--hard` — 跳过远程删除确认
- `--namespace <slug>` — 远程删除的 namespace
- `--namespace <slug>` — 本地或远程删除的 namespace
- `--registry <url>` — Registry URL
- `--token <token>` — API token
- `--json` — JSON 输出
显式命名空间坐标(`team/my-skill`、`@team/my-skill`、`team--my-skill`)或
`--namespace team` 只删除该 namespace 中的本地安装。为保持兼容,裸 slug
会删除当前 registry 中所有 namespace 下的同名本地安装。
### doctor
```bash

View file

@ -0,0 +1,325 @@
# CLI Namespace Errors Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Make every documented namespace coordinate reach the correct registry path and preserve public server error messages and request IDs without misclassifying all 403 responses as token-scope failures.
**Architecture:** Extend the shared coordinate parser with a resolver that owns explicit namespace conflict handling, then make install/remove consume it while removing the argument parser's early `global` default. Add one response-error converter inside `SkillHubClient` so JSON endpoints and downloads share safe `msg`/`requestId` extraction while retaining status-based exit codes.
**Tech Stack:** TypeScript, Bun test/build, cac, npm package tarballs.
## Completion record
Completed in PR #608 and revalidated after merging `origin/main` on 2026-07-29.
The checklist below reflects the delivered implementation. The maintainer
revalidation did not recreate historical RED states; it reran the current
GREEN gates with the repository-pinned Bun 1.3.13:
- Focused namespace/error/help regression: 142 tests passed.
- Complete CLI regression: 379 tests passed with
`bun test --max-concurrency=1` (peak RSS 180452 KiB).
- Typecheck, lint, and build passed.
- The packed `@astron-team/skillhub@0.1.9` artifact contained `dist/index.js`,
`README.md`, `CHANGELOG.md`, `LICENSE`, and `package.json`.
- Packed Node artifact smoke passed for `version`, `help install`, all three
namespaced coordinate forms, coordinate/`--namespace` conflict handling, and
structured 403 message/request-ID rendering.
- The Chinese and English VitePress documentation build passed.
---
### Task 1: Establish release artifact baseline
**Files:**
- Inspect: `cli/package.json`
- Inspect: npm package `@astron-team/skillhub@0.1.9`
- [x] **Step 1: Read published metadata and download the package**
Run:
```bash
npm view @astron-team/skillhub@0.1.9 version dist.tarball dist.integrity --json
npm pack @astron-team/skillhub@0.1.9 --pack-destination /tmp/skillhub-npm-019-issue-606 --json
```
Expected: version `0.1.9`, a tarball with `dist/index.js`, `README.md`,
`LICENSE`, and `package.json`.
- [x] **Step 2: Confirm the published bundle contains both bug signatures**
Run:
```bash
tar -xOf /tmp/skillhub-npm-019-issue-606/astron-team-skillhub-0.1.9.tgz package/dist/index.js | rg 'indexOf\("--"\)|token may lack required scope'
```
Expected: both patterns are present, proving 0.1.9 includes the double-dash
parser but also the misleading 403 fallback.
### Task 2: Normalize coordinates and reject conflicts
**Files:**
- Modify: `cli/test/unit/shared/skill-name-parser.test.ts`
- Modify: `cli/src/shared/skill-name-parser.ts`
- [x] **Step 1: Replace permissive edge tests with the public coordinate matrix**
Add table-driven assertions for `my-skill`, `team/my-skill`,
`@team/my-skill`, and `team--my-skill`. Add resolver assertions for an explicit
namespace on a bare slug, a matching coordinate namespace, and a conflicting
namespace. Add malformed-input assertions for empty or incomplete coordinates.
- [x] **Step 2: Run the parser test and verify RED**
Run:
```bash
cd cli && bun test test/unit/shared/skill-name-parser.test.ts
```
Expected: failures for slash forms, malformed input, and the missing resolver.
- [x] **Step 3: Implement the minimal parser and resolver**
Keep `ParsedSkillName` unchanged. Add `resolveSkillName(skillName,
explicitNamespace?)` returning `ParsedSkillName`. It calls one internal parser,
applies `global` only to bare slugs, accepts a matching explicit namespace, and
throws `CliError(..., EXIT.usage)` on malformed input or conflict.
- [x] **Step 4: Run the parser test and verify GREEN**
Run:
```bash
cd cli && bun test test/unit/shared/skill-name-parser.test.ts
```
Expected: all parser tests pass with no warnings.
### Task 3: Wire the resolver through real CLI parsing
**Files:**
- Modify: `cli/src/commands/install.ts`
- Modify: `cli/src/commands/remove.ts`
- Modify: `cli/src/index.ts`
- Modify: `cli/test/unit/commands/install-command.test.ts`
- Modify: `cli/test/integration/install-command.test.ts`
- [x] **Step 1: Add failing command and integration tests**
Capture `installSkill` options in the unit test and assert a namespaced
coordinate passes `namespace: 'team'` and `slug: 'my-skill'`. In the integration
test, register a `team/my-skill` fixture and execute:
```text
skillhub install @team/my-skill --dir <temp> --registry <fake> --token sk_ok --json
```
Assert exit 0, JSON namespace `team`, and fake-registry resolve state
`{ namespace: 'team', slug: 'my-skill' }`. Add a conflicting
`--namespace other` case that exits with usage code 5 before registry access.
- [x] **Step 2: Run the focused command tests and verify RED**
Run:
```bash
cd cli && bun test test/unit/commands/install-command.test.ts test/integration/install-command.test.ts
```
Expected: the namespaced integration case resolves `global` or fails, and the
conflict case does not produce the expected usage error.
- [x] **Step 3: Use `resolveSkillName` and remove the cac default**
Change install/remove to call:
```typescript
const { namespace, slug } = resolveSkillName(skillNameArg, options.namespace)
```
Change install's option declaration to:
```typescript
.option('--namespace <slug>', 'Namespace for a bare skill slug')
```
- [x] **Step 4: Run the focused command tests and verify GREEN**
Run the same Bun test command. Expected: all focused command tests pass.
### Task 4: Preserve structured API errors and request IDs
**Files:**
- Modify: `cli/test/unit/clients/skillhub-client.test.ts`
- Modify: `cli/test/unit/shared/output.test.ts`
- Modify: `cli/src/clients/skillhub-client.ts`
- Modify: `cli/src/shared/output.ts`
- [x] **Step 1: Add failing response and output tests**
Add client tests for:
```typescript
Response.json(
{ code: 403, msg: 'token has been revoked', requestId: 'req-403' },
{ status: 403 }
)
```
Assert message `token has been revoked`, auth exit code, and details containing
`requestId: 'req-403'`. Add 403 tests without `msg`, with invalid JSON, and a
404 with structured fields. Add a download 403 structured-response test. Add a
human output assertion for `Request ID: req-403`.
- [x] **Step 2: Run focused tests and verify RED**
Run:
```bash
cd cli && bun test test/unit/clients/skillhub-client.test.ts test/unit/shared/output.test.ts
```
Expected: structured messages/request IDs are discarded and human output omits
the request ID.
- [x] **Step 3: Implement one safe response-error converter**
Inside `SkillHubClient`, add a private method that reads non-success bodies once,
parses only object-shaped JSON, accepts only non-empty string `msg` and
`requestId`, selects status-specific fallback text and exit codes, and returns a
`CliError`. Use it from both `handleJsonResponse` and `download`. Do not add the
old token-scope hint to 403 errors. Update `renderError` with:
```typescript
if (typeof cliError.details.requestId === 'string') {
lines.push(`Request ID: ${cliError.details.requestId}`)
}
```
- [x] **Step 4: Run focused tests and verify GREEN**
Run the same focused Bun test command. Expected: all client/output tests pass.
### Task 5: Document the public contract and release impact
**Files:**
- Modify: `cli/src/commands/help.ts`
- Modify: `cli/README.md`
- Create: `cli/CHANGELOG.md`
- Modify: `cli/package.json`
- Modify: `cli/test/integration/help-command.test.ts`
- [x] **Step 1: Add a failing help assertion**
Assert `skillhub help install` includes `@team/my-skill`,
`team/my-skill`, and `team--my-skill` examples.
- [x] **Step 2: Run the help test and verify RED**
Run:
```bash
cd cli && bun test test/integration/help-command.test.ts
```
Expected: the coordinate examples are absent.
- [x] **Step 3: Update help, README, and release notes**
Use `<coordinate>` in install usage. Document all accepted forms and the
same-namespace/conflict rule. Add an Unreleased changelog entry covering
coordinate normalization and structured 403 messages/request IDs. Include
`CHANGELOG.md` in the npm package `files` list.
- [x] **Step 4: Run the help test and verify GREEN**
Run the same Bun test command. Expected: all help tests pass.
### Task 6: Verify source, build, and packed artifact
**Files:**
- Verify: all files changed by Tasks 2-5
- Produce locally: `cli/dist/index.js`
- Produce locally: npm tarball under `/tmp`
- [x] **Step 1: Run the complete CLI quality gate**
Run:
```bash
cd cli && bun test
cd cli && bun run typecheck
cd cli && bun run lint
cd cli && bun run build
```
Expected: every command exits 0 with no errors or warnings.
- [x] **Step 2: Pack and inspect the candidate artifact**
Run:
```bash
cd cli && npm pack --pack-destination /tmp/skillhub-cli-issue-606 --json
tar -tf /tmp/skillhub-cli-issue-606/astron-team-skillhub-0.1.9.tgz
```
Expected: the package contains the built executable, README, changelog,
license, and package metadata.
- [x] **Step 3: Run packed-bundle smoke checks**
Extract the tarball to a temporary directory and run the built executable's
`version` and `help install` commands. Expected: version reports 0.1.9 and help
shows every coordinate form. Run the relevant unit/integration suites against
source to verify request paths and structured errors.
- [x] **Step 4: Review the diff and commit**
Run:
```bash
git diff --check
git status --short
git diff --stat
```
Expected: only CLI implementation/tests/docs and the two planning documents are
changed; generated `cli/dist/index.js` and tarballs are not committed.
Commit with a conventional message containing the issue ID:
```bash
git commit -m "fix(cli): normalize namespace coordinates and errors (#606)"
```
### Task 7: Review and create the single final PR
**Files:**
- Review: committed diff against `origin/main`
- [x] **Step 1: Run tester and reviewer gates**
The tester must confirm focused and full CLI gates plus package smoke evidence.
The reviewer must inspect coordinate compatibility, error disclosure, test
coverage, docs, commit metadata, and absence of unrelated changes. Resolve all
blocking findings before continuing.
- [x] **Step 2: Push only the assigned branch**
Run:
```bash
git push -u origin fix/cli-namespace-errors
```
Expected: only the assigned branch is created or updated remotely.
- [x] **Step 3: Create one PR linked to the issue**
Create one PR titled `fix(cli): normalize namespace coordinates and errors`
with `Related to #606` in the body, complete test/package evidence, docs and
risk sections, and no close intent unless the project manager requests it.
Do not merge the PR.

View file

@ -0,0 +1,101 @@
# CLI Namespace Coordinates and Structured Errors Design
## Context and approval
GitHub issue #606 reports two coupled CLI 0.1.9 failures: namespaced install
coordinates can silently resolve against `global`, and JSON API responses with
HTTP 403 are always rewritten as a token-scope error. The Multica issue's
technical-analysis comment defines the desired normalization, conflict, error,
documentation, and package-verification behavior. The project manager then
assigned implementation against that design on `fix/cli-namespace-errors`, so
that comment and assignment are the approved design baseline.
## Considered approaches
1. Centralize coordinate normalization and structured response errors in the
existing shared parser and client. This is the selected approach because all
commands receive one interpretation and tests can exercise the public
contract without duplicating parsing or status handling.
2. Patch `install` only. This would be smaller, but `remove --remote` already
consumes the same parser and would retain inconsistent behavior.
3. Change the server or documentation to accept only `--namespace`. This would
preserve the CLI bug and contradict documented coordinate forms.
## Coordinate contract
The CLI accepts these equivalent inputs:
| Input | Namespace | Slug |
|---|---|---|
| `my-skill` | `global` | `my-skill` |
| `team/my-skill` | `team` | `my-skill` |
| `@team/my-skill` | `team` | `my-skill` |
| `team--my-skill` | `team` | `my-skill` |
| `my-skill --namespace team` | `team` | `my-skill` |
The command parser must not inject `global` before coordinate normalization.
`global` is applied only when the input is a bare slug and no explicit
`--namespace` is supplied. If a coordinate and `--namespace` name the same
namespace, the input is accepted. If they differ, the command fails with a
usage error instead of silently choosing either value.
Structurally incomplete coordinates such as an empty string, `@team`,
`team/`, `/my-skill`, `--my-skill`, and `team--` fail with a usage error. The
normalizer does not add new namespace or slug character restrictions; server
validation remains authoritative for those rules.
## Error contract
For unsuccessful JSON API responses, the client reads the body once and only
uses the documented public fields `msg` and `requestId` when they are non-empty
strings. A server `msg` becomes the `CliError` message. A `requestId` is stored
in error details and rendered in both JSON and human-readable CLI output.
Exit classification remains stable:
- 401 and 403 use the authentication exit code.
- 404 and other application failures use the generic exit code.
- 502 and 503 use the network exit code.
When `msg` is absent, invalid, or the body is not JSON, the CLI uses a status-
specific fallback. In particular, the 403 fallback is `access denied` and does
not speculate about token scope. Raw non-JSON bodies and unrecognized fields
are not surfaced, avoiding disclosure of internal response content. Download
responses use the same structured error extraction while retaining their
download-specific fallbacks.
## Components and data flow
- `cli/src/shared/skill-name-parser.ts` parses and resolves coordinates,
including explicit namespace conflict detection.
- `cli/src/commands/install.ts` and `cli/src/commands/remove.ts` consume the
resolved coordinate.
- `cli/src/index.ts` leaves `--namespace` unset unless the caller supplies it.
- `cli/src/clients/skillhub-client.ts` converts unsuccessful responses into
structured `CliError` instances.
- `cli/src/shared/output.ts` renders `requestId` for human users; JSON output
already serializes error details.
- `cli/src/commands/help.ts`, `cli/README.md`, and `cli/CHANGELOG.md` document
supported forms, conflicts, and the 403 behavior change.
## Testing and package verification
Unit tests cover the coordinate matrix, malformed inputs, matching/conflicting
`--namespace`, structured and unstructured 401/403/404/500/502 responses, and
human request-ID rendering. An integration install test executes the real CLI
argument parser against a fake registry so the former `default: 'global'`
override cannot regress.
The release check builds and packs the CLI, inspects the tarball file list, and
runs the packed executable for version/help plus focused coordinate/error smoke
tests. The published npm 0.1.9 package is retained only as a comparison
artifact; no package publication or main-branch merge is part of this work.
## Risks
- Rejecting ambiguous coordinate/flag combinations is an intentional behavior
tightening and is called out in release notes.
- Server `msg` is treated as the public localized message defined by the API
envelope. Raw body content is deliberately not exposed.
- This change does not publish a new npm version; release owners must verify the
future dist-tag after the approved PR is merged and released.