Skip to content

Commit bafb9fe

Browse files
authored
Merge pull request #608 from iflytek/fix/cli-namespace-errors
fix(cli): normalize namespace coordinates and errors
2 parents 9f602f8 + 13b3f2d commit bafb9fe

25 files changed

Lines changed: 1344 additions & 199 deletions

‎cli/CHANGELOG.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Changelog
2+
3+
All notable CLI behavior changes are documented in this file.
4+
5+
## Unreleased
6+
7+
### Fixed
8+
9+
- Resolve `namespace/slug`, `@namespace/slug`, and `namespace--slug`
10+
coordinates against their declared namespace instead of silently falling
11+
back to `global`.
12+
- Reject a namespaced coordinate combined with a conflicting `--namespace`
13+
value; a matching value remains valid.
14+
- Limit local removal with a namespaced coordinate or explicit `--namespace`
15+
to the matching namespace, preventing collateral deletion of same-slug
16+
installations in other namespaces. Bare-slug removal retains its existing
17+
cross-namespace behavior for compatibility.
18+
- Preserve public registry `msg` and `requestId` fields for unsuccessful
19+
responses. HTTP 403 without a public message now reports the neutral
20+
`access denied` fallback instead of assuming the token lacks scope.

‎cli/README.md‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,15 +126,34 @@ Output format: `namespace/slug version summary`
126126

127127
## 📥 Install Skills
128128

129+
The install coordinate accepts a bare slug or any of the equivalent namespace
130+
forms below:
131+
132+
| Coordinate | Resolved namespace | Resolved slug |
133+
|------------|--------------------|---------------|
134+
| `my-skill` | `global` | `my-skill` |
135+
| `team/my-skill` | `team` | `my-skill` |
136+
| `@team/my-skill` | `team` | `my-skill` |
137+
| `team--my-skill` | `team` | `my-skill` |
138+
139+
For a bare slug, `--namespace team` selects a non-global namespace. A
140+
namespaced coordinate may be combined with the same `--namespace` value, but a
141+
conflicting value is rejected instead of silently overriding the coordinate.
142+
129143
```bash
130144
# Install to auto-detected Agent directory
131145
skillhub install pdf-parser
132146

147+
# Equivalent namespaced coordinates
148+
skillhub install team/my-skill
149+
skillhub install @team/my-skill
150+
skillhub install team--my-skill
151+
133152
# Choose install scope explicitly
134153
skillhub install pdf-parser --scope user
135154
skillhub install pdf-parser --scope project --agent codex
136155

137-
# Specify namespace (default: global)
156+
# Specify namespace for a bare slug (default: global)
138157
skillhub install pdf-parser --namespace myspace
139158

140159
# Specify version
@@ -239,9 +258,17 @@ skillhub list --json
239258
### Remove Skills
240259

241260
```bash
242-
# Remove all local installation targets
261+
# A bare slug removes matching local installations across namespaces
243262
skillhub remove pdf-parser
244263

264+
# A namespaced coordinate removes only that namespace
265+
skillhub remove myspace/pdf-parser
266+
skillhub remove @myspace/pdf-parser
267+
skillhub remove myspace--pdf-parser
268+
269+
# Equivalent precise local removal with an explicit namespace
270+
skillhub remove pdf-parser --namespace myspace
271+
245272
# Remove only specific Agent's installation
246273
skillhub remove pdf-parser --agent codex
247274

@@ -337,9 +364,9 @@ Update mechanism:
337364
| `skillhub logout [--registry <url>] [--json]` | Remove token for specified registry |
338365
| `skillhub whoami [--registry <url>] [--token <token>] [--json]` | Validate current token and display user information |
339366
| `skillhub search <query> [--registry <url>] [--token <token>] [--limit <n>] [--json]` | Search published skills |
340-
| `skillhub install <slug> [--scope <user\|project>] [--namespace <slug>] [--version <v>] [--agent <profile>] [--dir <path>] [--force] [--registry <url>] [--token <token>] [--json]` | Install a skill |
367+
| `skillhub install <coordinate> [--scope <user\|project>] [--namespace <slug>] [--version <v>] [--agent <profile>] [--dir <path>] [--force] [--registry <url>] [--token <token>] [--json]` | Install a skill |
341368
| `skillhub list [--agent <profile>] [--dir <path>] [--registry <url>] [--json]` | List installed skills |
342-
| `skillhub remove <slug> [--agent <profile>] [--all] [--remote] [--hard] [--namespace <slug>] [--registry <url>] [--token <token>] [--json]` | Remove a skill |
369+
| `skillhub remove <coordinate> [--agent <profile>] [--all] [--remote] [--hard] [--namespace <slug>] [--registry <url>] [--token <token>] [--json]` | Remove a skill |
343370
| `skillhub doctor [--json]` | Scan project directory and rebuild local inventory |
344371
| `skillhub publish <path> [--namespace <slug>] [--visibility <v>] [--registry <url>] [--token <token>] [--json]` | Publish a skill |
345372
| `skillhub update [--check] [--json]` | Check or execute CLI self-update |
@@ -364,6 +391,11 @@ skillhub whoami
364391
skillhub login --token sk_xxx
365392
```
366393

394+
For structured registry failures, the CLI prints the server's public `msg` and
395+
`requestId`. HTTP 403 without a public message falls back to `access denied`;
396+
it is not automatically described as a missing token scope. Include the
397+
request ID when asking a registry operator to investigate.
398+
367399
### Network Error
368400

369401
```bash

‎cli/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
"files": [
2929
"dist",
3030
"README.md",
31+
"CHANGELOG.md",
3132
"LICENSE"
3233
],
3334
"scripts": {

‎cli/src/clients/skillhub-client.ts‎

Lines changed: 53 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,13 @@ export interface DryRunResponse {
5252
resolvedVersion: string | null
5353
}
5454

55-
interface ErrorEnvelope {
56-
msg?: unknown
57-
requestId?: unknown
55+
interface PublicErrorFields {
56+
msg?: string
57+
requestId?: string
5858
}
5959

60+
type ErrorResponseKind = 'json' | 'download'
61+
6062
export class SkillHubClient {
6163
constructor(
6264
readonly registry: string,
@@ -93,17 +95,8 @@ export class SkillHubClient {
9395
} catch {
9496
throw new CliError('registry unreachable', EXIT.network, { registry: this.registry, next: 'check network or pass --registry' })
9597
}
96-
if (response.status === 401) {
97-
throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' })
98-
}
99-
if (response.status === 403) {
100-
throw await this.createAccessDeniedError(response)
101-
}
102-
if (response.status === 404) {
103-
throw new CliError('skill or version not found', EXIT.generic, { registry: this.registry })
104-
}
10598
if (!response.ok) {
106-
throw new CliError(`download failed with status ${response.status}`, EXIT.generic, { registry: this.registry })
99+
throw await this.createResponseError(response, 'download')
107100
}
108101
return response
109102
}
@@ -159,45 +152,65 @@ export class SkillHubClient {
159152
}
160153

161154
private async handleJsonResponse<T>(response: Response): Promise<T> {
162-
if (response.status === 401) {
163-
throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' })
164-
}
165-
if (response.status === 403) {
166-
throw await this.createAccessDeniedError(response)
167-
}
168-
if (response.status === 404) {
169-
throw new CliError('resource not found', EXIT.generic, { registry: this.registry })
170-
}
171-
// 502/503 indicate network-level failures (connection refused, service unavailable)
172-
if (response.status === 502 || response.status === 503) {
173-
throw new CliError(`registry returned ${response.status}`, EXIT.network, { registry: this.registry })
174-
}
175155
if (!response.ok) {
176-
const text = await response.text().catch(() => '')
177-
throw new CliError(`registry returned ${response.status}`, EXIT.generic, { registry: this.registry, detail: text })
156+
throw await this.createResponseError(response, 'json')
178157
}
179158
const body = await response.json()
180159
return body.data as T
181160
}
182161

183-
private async createAccessDeniedError(response: Response): Promise<CliError> {
184-
const error = await this.readErrorEnvelope(response)
185-
return new CliError(error.message ?? 'access denied', EXIT.auth, {
186-
registry: this.registry,
187-
...(error.requestId ? { requestId: error.requestId } : {})
188-
})
162+
private async createResponseError(response: Response, kind: ErrorResponseKind): Promise<CliError> {
163+
const publicFields = await this.readPublicErrorFields(response)
164+
const details: Record<string, unknown> = { registry: this.registry }
165+
if (publicFields.requestId) {
166+
details.requestId = publicFields.requestId
167+
}
168+
169+
let fallback: string
170+
let exitCode: number = EXIT.generic
171+
172+
if (response.status === 401) {
173+
fallback = 'authentication failed'
174+
exitCode = EXIT.auth
175+
details.next = 'run `skillhub login`'
176+
} else if (response.status === 403) {
177+
fallback = 'access denied'
178+
exitCode = EXIT.auth
179+
} else if (response.status === 404) {
180+
fallback = kind === 'download' ? 'skill or version not found' : 'resource not found'
181+
} else if (response.status === 502 || response.status === 503) {
182+
fallback = kind === 'download'
183+
? `download failed with status ${response.status}`
184+
: `registry returned ${response.status}`
185+
exitCode = EXIT.network
186+
} else {
187+
fallback = kind === 'download'
188+
? `download failed with status ${response.status}`
189+
: `registry returned ${response.status}`
190+
}
191+
192+
return new CliError(publicFields.msg ?? fallback, exitCode, details)
189193
}
190194

191-
private async readErrorEnvelope(response: Response): Promise<{ message?: string; requestId?: string }> {
195+
private async readPublicErrorFields(response: Response): Promise<PublicErrorFields> {
196+
let body: unknown
192197
try {
193-
const body = await response.json() as ErrorEnvelope
194-
return {
195-
...(typeof body.msg === 'string' && body.msg.trim() ? { message: body.msg } : {}),
196-
...(typeof body.requestId === 'string' && body.requestId.trim() ? { requestId: body.requestId } : {})
197-
}
198+
body = await response.json()
198199
} catch {
199200
return {}
200201
}
202+
203+
if (typeof body !== 'object' || body === null || Array.isArray(body)) {
204+
return {}
205+
}
206+
207+
const record = body as Record<string, unknown>
208+
const msg = typeof record.msg === 'string' ? record.msg.trim() : ''
209+
const requestId = typeof record.requestId === 'string' ? record.requestId.trim() : ''
210+
return {
211+
...(msg ? { msg } : {}),
212+
...(requestId ? { requestId } : {})
213+
}
201214
}
202215

203216
private headers(): HeadersInit {

‎cli/src/commands/help.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,12 @@ export const commands = {
3333
},
3434
install: {
3535
summary: 'Install a skill locally',
36-
usage: 'skillhub install <slug> [--scope <user|project>] [--namespace <slug>] [--version <v>] [--agent <profile>] [--dir <path>] [--force] [--json]',
36+
usage: 'skillhub install <coordinate> [--scope <user|project>] [--namespace <slug>] [--version <v>] [--agent <profile>] [--dir <path>] [--force] [--json]',
3737
examples: [
3838
'skillhub install pdf-parser',
39+
'skillhub install team/my-skill',
40+
'skillhub install @team/my-skill',
41+
'skillhub install team--my-skill',
3942
'skillhub install pdf-parser --scope user',
4043
'skillhub install pdf-parser --scope project --agent codex'
4144
]
@@ -47,8 +50,13 @@ export const commands = {
4750
},
4851
remove: {
4952
summary: 'Remove local or remote skill',
50-
usage: 'skillhub remove <slug> [--agent <profile>] [--all] [--remote] [--hard] [--namespace <slug>] [--json]',
51-
examples: ['skillhub remove pdf-parser', 'skillhub remove pdf-parser --remote --hard']
53+
usage: 'skillhub remove <coordinate> [--agent <profile>] [--all] [--remote] [--hard] [--namespace <slug>] [--json]',
54+
examples: [
55+
'skillhub remove pdf-parser',
56+
'skillhub remove team/my-skill',
57+
'skillhub remove my-skill --namespace team',
58+
'skillhub remove pdf-parser --remote --hard'
59+
]
5260
},
5361
doctor: {
5462
summary: 'Scan project and merge into local inventory (preserves entries outside scan scope)',

‎cli/src/commands/install.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { installSkill } from '../services/install-service'
55
import { resolveInstallTargets } from '../agents/resolver'
66
import { CliError } from '../shared/errors'
77
import { EXIT } from '../shared/constants'
8-
import { parseSkillName } from '../shared/skill-name-parser'
8+
import { resolveSkillName } from '../shared/skill-name-parser'
99

1010
export interface InstallCommandOptions {
1111
namespace?: string | undefined
@@ -94,9 +94,7 @@ export async function installCommand(
9494
const registry = resolveRegistry(options, process.env, await configStore.read())
9595
const token = resolveToken(options, process.env, await credentialsStore.getToken(registry))
9696

97-
const parsed = parseSkillName(skillNameArg)
98-
const namespace = options.namespace ?? parsed.namespace
99-
const slug = parsed.slug
97+
const { namespace, slug } = resolveSkillName(skillNameArg, options.namespace)
10098

10199
const resolveTargets = deps.resolveInstallTargets ?? resolveInstallTargets
102100
const targets = await resolveTargets({

‎cli/src/commands/remove.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { resolveRegistry, resolveToken } from '../services/registry-service'
55
import { removeLocalSkill } from '../services/remove-service'
66
import { CliError } from '../shared/errors'
77
import { EXIT } from '../shared/constants'
8-
import { parseSkillName } from '../shared/skill-name-parser'
8+
import { hasExplicitNamespace, resolveSkillName } from '../shared/skill-name-parser'
99

1010
export interface RemoveCommandOptions {
1111
agent?: string[] | undefined
@@ -30,9 +30,7 @@ export async function removeCommand(skillNameArg: string, options: RemoveCommand
3030
const credentialsStore = new CredentialsStore()
3131
const registry = resolveRegistry(options, process.env, await configStore.read())
3232

33-
const parsed = parseSkillName(skillNameArg)
34-
const namespace = options.namespace ?? parsed.namespace
35-
const slug = parsed.slug
33+
const { namespace, slug } = resolveSkillName(skillNameArg, options.namespace)
3634

3735
if (options.remote) {
3836
const token = resolveToken(options, process.env, await credentialsStore.getToken(registry))
@@ -62,8 +60,13 @@ export async function removeCommand(skillNameArg: string, options: RemoveCommand
6260
}
6361

6462
// Local remove
63+
const namespaceFilter = options.namespace !== undefined || hasExplicitNamespace(skillNameArg)
64+
? namespace
65+
: undefined
6566
const result = await removeLocalSkill({
66-
registry, slug,
67+
registry,
68+
namespace: namespaceFilter,
69+
slug,
6770
agents: options.agent,
6871
all: options.all
6972
})

‎cli/src/index.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,8 @@ cli
231231
})
232232

233233
cli
234-
.command('install <slug>', 'Install a skill locally')
235-
.option('--namespace <slug>', 'Namespace', { default: 'global' })
234+
.command('install <coordinate>', 'Install a skill locally')
235+
.option('--namespace <slug>', 'Namespace for a bare skill slug')
236236
.option('--version <v>', 'Version')
237237
.option('--scope <scope>', 'Install scope: user or project')
238238
.option('--agent <profile>', 'Agent profile (repeatable)')
@@ -256,17 +256,17 @@ cli
256256
})
257257

258258
cli
259-
.command('remove <slug>', 'Remove local or remote skill')
259+
.command('remove <coordinate>', 'Remove local or remote skill')
260260
.option('--agent <profile>', 'Filter by agent (repeatable)')
261261
.option('--all', 'Remove all targets')
262262
.option('--remote', 'Delete remote skill')
263263
.option('--hard', 'Skip confirmation for remote delete')
264-
.option('--namespace <slug>', 'Namespace for remote delete')
264+
.option('--namespace <slug>', 'Namespace for local or remote delete')
265265
.option('--registry <url>', 'Registry URL')
266266
.option('--token <token>', 'API token')
267267
.option('--json', 'Output JSON')
268-
.action((slug: string, options: RemoveCommandOptions & { agent?: string | string[] }) => {
269-
return runCommand(() => removeCommand(slug, { ...options, agent: toArray(options.agent) }), Boolean(options.json))
268+
.action((coordinate: string, options: RemoveCommandOptions & { agent?: string | string[] }) => {
269+
return runCommand(() => removeCommand(coordinate, { ...options, agent: toArray(options.agent) }), Boolean(options.json))
270270
})
271271

272272
cli

‎cli/src/services/remove-service.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ function isPathUnder(child: string, parent: string): boolean {
1515

1616
export interface RemoveLocalOptions {
1717
registry: string
18+
namespace?: string | undefined
1819
slug: string
1920
agents?: string[] | undefined
2021
all?: boolean | undefined
@@ -29,7 +30,11 @@ export async function removeLocalSkill(options: RemoveLocalOptions): Promise<Rem
2930
const store = new InventoryStore(options.home)
3031
const inventory = await store.read()
3132

32-
const items = inventory.items.filter(i => i.registry === options.registry && i.slug === options.slug)
33+
const items = inventory.items.filter(item =>
34+
item.registry === options.registry &&
35+
item.slug === options.slug &&
36+
(options.namespace === undefined || item.namespace === options.namespace)
37+
)
3338
if (items.length === 0) {
3439
throw new CliError(`skill not found locally: ${options.slug}`, EXIT.generic, {
3540
next: 'run `skillhub list` to see installed skills'

0 commit comments

Comments
 (0)