diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aa88668f2..e5610501b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -228,6 +228,21 @@ only when a separate capability level is intentional, as for Apple Terminal charts. Tests that change `chalk.level` must restore it, and environment-driven color detection tests must import in a fresh process. +### CLI argument parsing + +Keep `commander` and `@commander-js/extra-typings` pinned together at exactly +`15.0.0`; the typings peer requires `commander ~15.0.0`. Regenerate `bun.lock` +with `bun install` after changing both pins. Use their public ESM exports. + +Declare positional arguments explicitly, including variadic arguments for commands +that forward subprocess arguments. Excess arguments are rejected by default. +Short flags must be a single character; use `--debug-to-stderr` in place of the +unsupported `-d2e` spelling. Dual long flags, such as `--yolo` and +`--dangerously-skip-permissions`, are supported. When registering both positive +and negative boolean flags, set an explicit default only if the command needs one; +otherwise the omitted option remains undefined. Keep help and parse-error checks +on the built CLI so command registration is exercised under Node. + ### Dependency pins and overrides Terminal syntax highlighting uses the exact `highlight.js` pin directly, with diff --git a/bun.lock b/bun.lock index 01fd229e0..4159d76ba 100644 --- a/bun.lock +++ b/bun.lock @@ -19,7 +19,7 @@ "@aws-sdk/client-sts": "3.1156.0", "@aws-sdk/credential-provider-node": "3.972.88", "@azure/identity": "4.14.0", - "@commander-js/extra-typings": "12.1.0", + "@commander-js/extra-typings": "15.0.0", "@grpc/grpc-js": "1.14.6", "@grpc/proto-loader": "0.8.1", "@modelcontextprotocol/sdk": "1.32.1", @@ -38,7 +38,7 @@ "chalk": "6.0.1", "chokidar": "6.0.0", "cli-boxes": "4.0.1", - "commander": "12.1.0", + "commander": "15.0.0", "cross-spawn": "7.0.6", "diff": "9.0.0", "duck-duck-scrape": "2.2.7", @@ -231,7 +231,7 @@ "@babel/runtime": ["@babel/runtime@7.29.2", "", {}, "sha512-JiDShH45zKHWyGe4ZNVRrCjBz8Nh9TMmZG1kh4QTK8hCBTWBi8Da+i7s1fJw7/lYpM4ccepSNfqzZ/QvABBi5g=="], - "@commander-js/extra-typings": ["@commander-js/extra-typings@12.1.0", "", { "peerDependencies": { "commander": "~12.1.0" } }, "sha512-wf/lwQvWAA0goIghcb91dQYpkLBcyhOhQNqG/VgWhnKzgt+UOMvra7EX/2fv70arm5RW+PUHoQHHDa6/p77Eqg=="], + "@commander-js/extra-typings": ["@commander-js/extra-typings@15.0.0", "", { "peerDependencies": { "commander": "~15.0.0" } }, "sha512-yeJlba62xqmkgELUsn7356MEnzLLu/fw2x4lofFqGnXh6YysRdEs2BaLeLtg1+KU0AXvMeqQvTTp+3hBEBK+EA=="], "@emnapi/core": ["@emnapi/core@1.11.2", "", { "dependencies": { "@emnapi/wasi-threads": "1.2.2", "tslib": "^2.4.0" } }, "sha512-TC8MkTuZUtcTSiFeuC0ksCh9QIJ5+F21MvZ4Wn4ORfYaFJ/0dsiudv5tVkejgwZlwQ39jL9WWDe2lz8x0WglOA=="], @@ -669,7 +669,7 @@ "combined-stream": ["combined-stream@1.0.8", "", { "dependencies": { "delayed-stream": "~1.0.0" } }, "sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg=="], - "commander": ["commander@12.1.0", "", {}, "sha512-Vw8qHK3bZM9y/P10u3Vib8o/DdkvA2OtPtZvD871QKjy74Wj1WSKFILMPRPSdUSx5RFK1arlJzEtA4PkFgnbuA=="], + "commander": ["commander@15.0.0", "", {}, "sha512-z67u4ZhzCL/Tydu1lJARtEZYWbWaN7oYLHbsuzocr6y4N6WZAagG3RQ4FW61V1/0+jImpj293XfrcYnd1qxtPg=="], "concat-map": ["concat-map@0.0.1", "", {}, "sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg=="], @@ -1275,6 +1275,8 @@ "zod-to-json-schema": ["zod-to-json-schema@3.25.2", "", { "peerDependencies": { "zod": "^3.25.28 || ^4" } }, "sha512-O/PgfnpT1xKSDeQYSCfRI5Gy3hPf91mKVDuYLUHZJMiDFptvP41MSnWofm8dnCm0256ZNfZIM7DSzuSMAFnjHA=="], + "@anthropic-ai/sandbox-runtime/commander": ["commander@12.1.0", "", {}, "sha512-Vw8qHK3bZM9y/P10u3Vib8o/DdkvA2OtPtZvD871QKjy74Wj1WSKFILMPRPSdUSx5RFK1arlJzEtA4PkFgnbuA=="], + "@aws-crypto/crc32/@aws-crypto/util": ["@aws-crypto/util@3.0.0", "", { "dependencies": { "@aws-sdk/types": "^3.222.0", "@aws-sdk/util-utf8-browser": "^3.0.0", "tslib": "^1.11.1" } }, "sha512-2OJlpeJpCR48CC8r+uKVChzs9Iungj9wkZrl8Z041DWEWvyIHILYKCPNzJghKsivj+S3mLo6BVc7mBNzdxA46w=="], "@aws-crypto/crc32/@aws-sdk/types": ["@aws-sdk/types@3.973.8", "", { "dependencies": { "@smithy/types": "^4.14.1", "tslib": "^2.6.2" } }, "sha512-gjlAdtHMbtR9X5iIhVUvbVcy55KnznpC6bkDUWW9z915bi0ckdUr5cjf16Kp6xq0bP5HBD2xzgbL9F9Quv5vUw=="], diff --git a/package.json b/package.json index e815269e6..bf3e75676 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,7 @@ "@aws-sdk/client-sts": "3.1156.0", "@aws-sdk/credential-provider-node": "3.972.88", "@azure/identity": "4.14.0", - "@commander-js/extra-typings": "12.1.0", + "@commander-js/extra-typings": "15.0.0", "@grpc/grpc-js": "1.14.6", "@grpc/proto-loader": "0.8.1", "@modelcontextprotocol/sdk": "1.32.1", @@ -118,7 +118,7 @@ "chalk": "6.0.1", "chokidar": "6.0.0", "cli-boxes": "4.0.1", - "commander": "12.1.0", + "commander": "15.0.0", "cross-spawn": "7.0.6", "diff": "9.0.0", "duck-duck-scrape": "2.2.7", diff --git a/src/entrypoints/commanderCompatibility.test.ts b/src/entrypoints/commanderCompatibility.test.ts new file mode 100644 index 000000000..215ff076d --- /dev/null +++ b/src/entrypoints/commanderCompatibility.test.ts @@ -0,0 +1,73 @@ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test' +import { copyFile, mkdtemp, rm, symlink } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' + +let buildDir: string +let cliPath: string + +function runCli(args: string[]) { + const env: NodeJS.ProcessEnv = { ...process.env, OPENCLAUDE_DISABLE_TELEMETRY: '1' } + delete env.OPENCLAUDE_DISABLE_CLI_ENTRYPOINT_AUTO_RUN + return Bun.spawnSync(['node', cliPath, ...args], { + env, + timeout: 8_000, + }) +} + +describe('built CLI Commander compatibility', () => { + beforeAll(async () => { + const repoRoot = resolve(import.meta.dir, '../..') + buildDir = await mkdtemp(join(tmpdir(), 'openclaude-commander-test-')) + // The build script writes to cwd/dist. Give it the current source and + // dependencies in an isolated workspace so every run tests a fresh bundle. + for (const directory of ['src', 'scripts', 'node_modules']) { + await symlink(join(repoRoot, directory), join(buildDir, directory), 'junction') + } + for (const file of ['package.json', 'tsconfig.json']) { + await copyFile(join(repoRoot, file), join(buildDir, file)) + } + const build = Bun.spawn([process.execPath, 'run', 'scripts/build.ts'], { + cwd: buildDir, + stdout: 'pipe', + stderr: 'pipe', + timeout: 120_000, + }) + const [exitCode, stdout, stderr] = await Promise.all([ + build.exited, + new Response(build.stdout).text(), + new Response(build.stderr).text(), + ]) + if (exitCode !== 0) { + throw new Error(`CLI build failed (${exitCode}):\n${stdout}\n${stderr}`) + } + cliPath = join(buildDir, 'dist/cli.mjs') + }, 130_000) // Leave time beyond the build deadline for output and cleanup. + + afterAll(async () => { + if (buildDir) await rm(buildDir, { recursive: true, force: true }) + }) + + // Allow time for the eight-second subprocess deadline and assertions. + test('registers the stderr debug option without breaking help', () => { + const result = runCli(['--debug-to-stderr', '--help']) + expect(result.exitCode).toBe(0) + expect(result.stdout.toString()).toContain('Usage: openclaude') + expect(result.stdout.toString()).toContain('--yolo, --dangerously-skip-permissions') + expect(result.stderr.toString()).not.toContain('error:') + }, 10_000) + + test('rejects excess prompt arguments before starting a session', () => { + const result = runCli(['first', 'second']) + expect(result.exitCode).toBe(1) + expect(result.stderr.toString()).toContain('too many arguments') + expect(result.stderr.toString()).toContain('Expected 1 argument but got 2') + }, 10_000) + + test('parses a negative numeric value and reports the option validation error', () => { + const result = runCli(['--max-turns', '-1']) + expect(result.exitCode).toBe(1) + expect(result.stderr.toString()).toContain("option '--max-turns ' argument '-1' is invalid") + expect(result.stderr.toString()).not.toContain('unknown option') + }, 10_000) +}) diff --git a/src/main.tsx b/src/main.tsx index 1e1cc2888..e8ceb9b7f 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -897,7 +897,7 @@ async function run(): Promise { // If not provided but flag is present, value will be true // The actual filtering is handled in debug.ts by parsing process.argv return true; - }).addOption(new Option('-d2e, --debug-to-stderr', 'Enable debug mode (to stderr)').argParser(Boolean).hideHelp()).option('--debug-file ', 'Write debug logs to a specific file path (implicitly enables debug mode)', () => true).option('--verbose', 'Override verbose mode setting from config', () => true).option('-p, --print', 'Print response and exit (useful for pipes). Note: The workspace trust dialog is skipped when Claude is run with the -p mode. Only use this flag in directories you trust.', () => true).addOption(new Option('--heartbeat ', 'Emit a headless liveness heartbeat while output is quiet (only works with --print, e.g. 30s, 2m; pre-stream startup heartbeats use stderr)').argParser(value => { + }).addOption(new Option('--debug-to-stderr', 'Enable debug mode (to stderr)').argParser(Boolean).hideHelp()).option('--debug-file ', 'Write debug logs to a specific file path (implicitly enables debug mode)', () => true).option('--verbose', 'Override verbose mode setting from config', () => true).option('-p, --print', 'Print response and exit (useful for pipes). Note: The workspace trust dialog is skipped when Claude is run with the -p mode. Only use this flag in directories you trust.', () => true).addOption(new Option('--heartbeat ', 'Emit a headless liveness heartbeat while output is quiet (only works with --print, e.g. 30s, 2m; pre-stream startup heartbeats use stderr)').argParser(value => { try { return parseHeadlessHeartbeatDuration(value); } catch (error) { diff --git a/src/utils/debug.ts b/src/utils/debug.ts index 8d132adf7..510f9b38c 100644 --- a/src/utils/debug.ts +++ b/src/utils/debug.ts @@ -84,9 +84,7 @@ export const getDebugFilter = memoize((): DebugFilter | null => { }) export const isDebugToStdErr = memoize((): boolean => { - return ( - process.argv.includes('--debug-to-stderr') || process.argv.includes('-d2e') - ) + return process.argv.includes('--debug-to-stderr') }) export const getDebugFilePath = memoize((): string | null => {