fix(cli): reject ambiguous version options

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>

cli/src/index.ts

@@ -33,17 +33,30 @@ function rawStringOption(argv: string[], name: string): string | undefined {
   const end = argv.indexOf('--')
   const args = end === -1 ? argv : argv.slice(0, end)
   let value: string | undefined
+  let occurrences = 0
 
   for (let index = 0; index < args.length; index += 1) {
     const argument = args[index]!
     if (argument === name) {
-      value = args[index + 1]
+      occurrences += 1
+      const candidate = args[index + 1]
+      if (candidate === undefined || candidate.startsWith('-')) {
+        throw new CliError(`option "${name}" value is missing`, EXIT.usage)
+      }
+      value = candidate
       index += 1
     } else if (argument.startsWith(optionWithEquals)) {
+      occurrences += 1
       value = argument.slice(optionWithEquals.length)
+      if (!value) {
+        throw new CliError(`option "${name}" value is missing`, EXIT.usage)
+      }
     }
   }
 
+  if (occurrences > 1) {
+    throw new CliError(`option "${name}" cannot be repeated`, EXIT.usage)
+  }
   return value
 }
 

cli/test/integration/version-option.test.ts

@@ -70,7 +70,61 @@ describe('--version parsing', () => {
     expect(registry.received.resolve?.version).toBe(TIMESTAMP_VERSION)
   })
 
-  test('suite install preserves trailing zeros in a numeric-looking version', async () => {
+  test('install preserves an ordinary text version', async () => {
+    const env = await createTempHome()
+    const registry = await startFakeRegistry({
+      token: 'sk_ok',
+      skills: [{
+        namespace: 'global',
+        slug: 'text-version',
+        version: 'release-a',
+        zipBytes: zipSync({ 'SKILL.md': strToU8('# text version') })
+      }]
+    })
+    stopServer = registry.stop
+    const installDir = join(env.cwd, 'skills-text-version')
+    await mkdir(installDir, { recursive: true })
+
+    const result = await runCli([
+      'install', '@global/text-version',
+      '--version', 'release-a',
+      '--dir', installDir,
+      '--registry', registry.url,
+      '--token', 'sk_ok'
+    ], { HOME: env.home, USERPROFILE: env.home })
+
+    expect(result.exitCode).toBe(0)
+    expect(registry.received.resolve?.version).toBe('release-a')
+  })
+
+  test('ignores --version after the option terminator', async () => {
+    const env = await createTempHome()
+    const registry = await startFakeRegistry({
+      token: 'sk_ok',
+      skills: [{
+        namespace: 'global',
+        slug: 'latest',
+        version: '1.0.0',
+        zipBytes: zipSync({ 'SKILL.md': strToU8('# latest') })
+      }]
+    })
+    stopServer = registry.stop
+    const installDir = join(env.cwd, 'skills-latest')
+    await mkdir(installDir, { recursive: true })
+
+    const result = await runCli([
+      'install', '@global/latest',
+      '--dir', installDir,
+      '--registry', registry.url,
+      '--token', 'sk_ok',
+      '--', '--version', TIMESTAMP_VERSION
+    ], { HOME: env.home, USERPROFILE: env.home })
+
+    expect(result.exitCode).toBe(0)
+    expect(registry.received.resolve?.version).toBeNull()
+  })
+
+  test.each(['separate', 'equals'])('suite install preserves trailing zeros with %s syntax', async syntax => {
     const env = await createTempHome()
     const received = { version: null as string | null }
     const server = Bun.serve({
@@ -93,7 +147,7 @@ describe('--version parsing', () => {
 
     const result = await runCli([
       'suite', 'install', '@global/starter-pack',
-      '--version', TIMESTAMP_VERSION,
+      ...(syntax === 'equals' ? [`--version=${TIMESTAMP_VERSION}`] : ['--version', TIMESTAMP_VERSION]),
       '--dir', installDir,
       '--registry', `http://localhost:${server.port}`,
       '--token', 'sk_ok'
@@ -102,4 +156,34 @@ describe('--version parsing', () => {
     expect(result.exitCode).not.toBe(0)
     expect(received.version).toBe(TIMESTAMP_VERSION)
   })
+
+  test.each([
+    ['a missing value', ['--version']],
+    ['a missing repeated value', ['--version', '1.0.0', '--version']],
+    ['repeated values', ['--version', '1.0.0', '--version', '2.0.0']],
+    ['an empty equals value', ['--version=']]
+  ])('rejects %s before contacting the registry', async (_description, versionArgs) => {
+    const env = await createTempHome()
+    const registry = await startFakeRegistry({
+      token: 'sk_ok',
+      skills: [{
+        namespace: 'global',
+        slug: 'rejected',
+        version: '1.0.0',
+        zipBytes: zipSync({ 'SKILL.md': strToU8('# rejected') })
+      }]
+    })
+    stopServer = registry.stop
+
+    const result = await runCli([
+      'install', '@global/rejected',
+      ...versionArgs,
+      '--json',
+      '--registry', registry.url,
+      '--token', 'sk_ok'
+    ], { HOME: env.home, USERPROFILE: env.home })
+
+    expect(result.exitCode).toBe(5)
+    expect(registry.received.resolves).toBe(0)
+  })
 })