fix(cli): normalize namespace coordinates (#606)

Signed-off-by: dongmucat <1127093059@qq.com>

cli/src/commands/install.ts

@@ -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({

cli/src/commands/remove.ts

@@ -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 { 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))

cli/src/index.ts

@@ -232,7 +232,7 @@ cli
 
 cli
   .command('install <slug>', 'Install a skill locally')
-  .option('--namespace <slug>', 'Namespace', { default: 'global' })
+  .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)')

cli/src/shared/skill-name-parser.ts

@@ -1,27 +1,86 @@
+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) {
+    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
+  }
+}
+
+/** 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
   }
 }

cli/test/integration/install-command.test.ts

@@ -324,6 +324,79 @@ describe('install command — P1', () => {
     expect(meta.version).toBe('2.0.0')
   })
 
+  test('@namespace/slug resolves the namespaced registry path', 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')
+    await mkdir(installDir, { recursive: true })
+
+    const result = await runCli(
+      [
+        'install', '@team/my-skill',
+        '--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

cli/test/unit/commands/install-command.test.ts

@@ -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,

cli/test/unit/shared/skill-name-parser.test.ts

@@ -1,90 +1,82 @@
-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--'
+  ])('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'))
   })
 })