mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
refactor(cli): dispatch web as a reserved token, drop parse machinery
Simplify the Commander adapter now that behavior can change: dispatch a leading `web` token to its own parser instead of a subcommand of the root program, and read opts()/processedArgs after parse() instead of action closures with a mutable holder. This removes enablePositionalOptions(), the parent-option leak guard, both action closures, and the --resume/--prompt argParser threading. Behavior changes: `dsh -p x web` is a headless prompt (extra positional dropped), `dsh web -p x` fails loud (web has no -p), and a repeated --resume is natural last-wins. The two real fail-loud invariants stay as post-parse checks: an empty --resume= id (agent-loop treats '' as no-resume) and an empty -p task. Trims args.spec.ts to the routing/fail-loud/help behavior that matters; the tui-agent keyless PTY smoke still covers bin.ts dispatch end to end. Net ~114 fewer lines across adapter and tests.
This commit is contained in:
@@ -5,7 +5,8 @@
|
||||
* already-parsed values instead of re-reading argv. Output is suppressed and
|
||||
* `exitOverride` is set so Commander never writes or exits on its own — every
|
||||
* outcome (including `--help`/`--version` and parse errors) is returned to the
|
||||
* caller as data.
|
||||
* caller as data. The `web` subcommand is a reserved first token dispatched to
|
||||
* its own parser, so root flags and `web` flags never share a grammar.
|
||||
* @module @deepseek-ai/dsh/args
|
||||
*/
|
||||
|
||||
@@ -57,23 +58,7 @@ export type DshInvocation =
|
||||
| InfoInvocation
|
||||
| ErrorInvocation
|
||||
|
||||
/** Raw Commander option bag for the root command before it is narrowed to a mode. */
|
||||
interface RootOptions {
|
||||
prompt?: string
|
||||
resume?: string
|
||||
}
|
||||
|
||||
/** Commander option bag for the `web` subcommand after `--port` coercion. */
|
||||
interface WebOptions {
|
||||
host: string
|
||||
port: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Coerce `--port` to an integer in 0–65535; a bad value throws
|
||||
* {@link InvalidArgumentError}, which Commander reports as a parse error the
|
||||
* adapter returns as an {@link ErrorInvocation}.
|
||||
*/
|
||||
/** Coerce `--port` to an integer in 0–65535; a bad value fails loud as a parse error. */
|
||||
function parsePort(raw: string): number {
|
||||
const port = Number(raw)
|
||||
if (!Number.isInteger(port) || port < 0 || port > 65535) {
|
||||
@@ -82,102 +67,90 @@ function parsePort(raw: string): number {
|
||||
return port
|
||||
}
|
||||
|
||||
/** Reject an empty `--prompt` task; an empty headless prompt has nothing to run. */
|
||||
function parsePrompt(raw: string): string {
|
||||
if (raw === '') throw new InvalidArgumentError("option '-p, --prompt <task>' must not be empty")
|
||||
return raw
|
||||
/**
|
||||
* A configured `Command` under `exitOverride` with output captured into `sink`,
|
||||
* so `--help`, `--version`, and parse errors surface as thrown `CommanderError`s
|
||||
* (see {@link settle}) rather than writing to a stream or exiting.
|
||||
*/
|
||||
function program(name: string, version: string, sink: string[]): Command {
|
||||
return new Command()
|
||||
.name(name)
|
||||
.version(version, '-V, --version', 'output the version number')
|
||||
.exitOverride()
|
||||
.configureOutput({
|
||||
writeOut: chunk => void sink.push(chunk),
|
||||
writeErr: chunk => void sink.push(chunk),
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a `--resume` value: reject an empty id and a repeated flag. Both are
|
||||
* mistypes that must fail loud, never silently start a fresh session or keep
|
||||
* only the last id. `previous` is the value from an earlier `--resume` on the
|
||||
* same invocation (Commander threads it in), so a second occurrence is caught.
|
||||
* Run `command.parse` and map its thrown `CommanderError` to an info/error
|
||||
* invocation, or `undefined` when the parse succeeded (the caller then reads the
|
||||
* parsed options).
|
||||
*/
|
||||
function parseResume(raw: string, previous: string | undefined): string {
|
||||
if (previous !== undefined) throw new InvalidArgumentError("option '--resume <id>' may be given only once")
|
||||
if (raw === '') throw new InvalidArgumentError("option '--resume <id>' must not be empty")
|
||||
return raw
|
||||
function settle(command: Command, argv: readonly string[], sink: string[]): InfoInvocation | ErrorInvocation | undefined {
|
||||
try {
|
||||
command.parse(argv, { from: 'user' })
|
||||
return undefined
|
||||
} catch (error) {
|
||||
/* v8 ignore next -- Commander only throws CommanderError from parse under exitOverride */
|
||||
if (!(error instanceof CommanderError)) throw error
|
||||
if (error.code === 'commander.helpDisplayed') return { mode: 'help', text: sink.join('') }
|
||||
if (error.code === 'commander.version') return { mode: 'version', text: sink.join('') }
|
||||
return { mode: 'error', message: error.message }
|
||||
}
|
||||
}
|
||||
|
||||
/** Parse `dsh web` arguments (everything after the `web` token). */
|
||||
function parseWeb(argv: readonly string[], version: string): DshInvocation {
|
||||
const sink: string[] = []
|
||||
const web = program('dsh web', version, sink)
|
||||
.description('serve the browser UI')
|
||||
.addOption(new Option('--host <host>', 'bind host').choices([LOOPBACK_HOST, ALL_INTERFACES_HOST]).default(LOOPBACK_HOST))
|
||||
.addOption(new Option('--port <port>', 'listen port').default(DEFAULT_WEB_PORT).argParser(parsePort))
|
||||
const settled = settle(web, argv, sink)
|
||||
if (settled !== undefined) return settled
|
||||
const { host, port } = web.opts<{ host: string; port: number }>()
|
||||
return { mode: 'web', host, port }
|
||||
}
|
||||
|
||||
/** Parse the default (TUI / headless) arguments: `[config]`, `-p/--prompt`, `--resume`. */
|
||||
function parseRoot(argv: readonly string[], version: string): DshInvocation {
|
||||
const sink: string[] = []
|
||||
const root = program('dsh', version, sink)
|
||||
.description('dsh: interactive TUI, headless task, and browser UI')
|
||||
.argument('[config]', 'config to boot instead of the shipped default (TUI mode)')
|
||||
.option('-p, --prompt <task>', 'run one headless turn for this task, print the result, and exit')
|
||||
.option('--resume <id>', 'resume the persisted session with this id (TUI mode)')
|
||||
const settled = settle(root, argv, sink)
|
||||
if (settled !== undefined) return settled
|
||||
const { prompt, resume } = root.opts<{ prompt?: string; resume?: string }>()
|
||||
const config = root.processedArgs[0] as string | undefined
|
||||
|
||||
if (prompt !== undefined) {
|
||||
// A headless prompt owns the invocation; an empty task has nothing to run.
|
||||
if (prompt === '') return { mode: 'error', message: "error: option '-p, --prompt <task>' must not be empty" }
|
||||
return { mode: 'headless', prompt }
|
||||
}
|
||||
// An empty `--resume=` id would silently start a fresh session downstream
|
||||
// (agent-loop treats '' as no-resume), so a mistyped resume must fail loud.
|
||||
if (resume === '') return { mode: 'error', message: "error: option '--resume <id>' must not be empty" }
|
||||
return {
|
||||
mode: 'tui',
|
||||
...config !== undefined ? { config } : {},
|
||||
...resume !== undefined ? { resume } : {},
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the raw argv into a single {@link DshInvocation}. Never writes to a
|
||||
* stream and never exits; `--help`/`--version` and every parse error come back
|
||||
* as data for `bin.ts` to act on.
|
||||
* as data for `bin.ts` to act on. A leading `web` token dispatches to the web
|
||||
* parser; everything else is the default TUI/headless grammar.
|
||||
* @param argv - the arguments after the node binary and script (`process.argv.slice(2)`).
|
||||
* @param version - the version string `--version` prints; read from this app's package.json.
|
||||
* @returns the resolved invocation, discriminated by `mode`.
|
||||
*/
|
||||
export function parseDshArgs(argv: readonly string[], version: string): DshInvocation {
|
||||
let resolved: DshInvocation | undefined
|
||||
const output: string[] = []
|
||||
|
||||
const program = new Command()
|
||||
.name('dsh')
|
||||
.description('dsh: interactive TUI, headless task, and browser UI')
|
||||
.version(version, '-V, --version', 'output the version number')
|
||||
.exitOverride()
|
||||
.configureOutput({
|
||||
writeOut: chunk => void output.push(chunk),
|
||||
writeErr: chunk => void output.push(chunk),
|
||||
})
|
||||
|
||||
// Positional options keep `dsh -p x web` from routing to the `web`
|
||||
// subcommand: a token after a root option is a positional, not a command.
|
||||
program
|
||||
.enablePositionalOptions()
|
||||
.argument('[config]', 'config to boot instead of the shipped default (TUI mode)')
|
||||
.addOption(new Option('-p, --prompt <task>', 'run one headless turn for this task, print the result, and exit').argParser(parsePrompt))
|
||||
.addOption(new Option('--resume <id>', 'resume the persisted session with this id (TUI mode)').argParser(parseResume))
|
||||
.action((config: string | undefined, options: RootOptions) => {
|
||||
if (options.prompt !== undefined) {
|
||||
// A headless prompt owns the invocation; a config positional is meaningless there.
|
||||
if (config !== undefined) {
|
||||
throw new InvalidArgumentError(`error: --prompt takes no config argument (got '${config}')`)
|
||||
}
|
||||
resolved = { mode: 'headless', prompt: options.prompt }
|
||||
return
|
||||
}
|
||||
resolved = {
|
||||
mode: 'tui',
|
||||
...config !== undefined ? { config } : {},
|
||||
...options.resume !== undefined ? { resume: options.resume } : {},
|
||||
}
|
||||
})
|
||||
|
||||
program
|
||||
.command('web')
|
||||
.description('serve the browser UI')
|
||||
.addOption(
|
||||
new Option('--host <host>', 'bind host')
|
||||
.choices([LOOPBACK_HOST, ALL_INTERFACES_HOST])
|
||||
.default(LOOPBACK_HOST),
|
||||
)
|
||||
.addOption(
|
||||
new Option('--port <port>', 'listen port').default(DEFAULT_WEB_PORT).argParser(parsePort),
|
||||
)
|
||||
.action((options: WebOptions, command: Command) => {
|
||||
// Root options placed before `web` (`dsh -p x web`) leak onto the parent;
|
||||
// reject them so a misplaced flag fails loud instead of silently serving.
|
||||
const leaked = command.parent?.opts<RootOptions>()
|
||||
if (leaked?.prompt !== undefined || leaked?.resume !== undefined) {
|
||||
throw new InvalidArgumentError('error: web takes no --prompt or --resume; place web first')
|
||||
}
|
||||
resolved = { mode: 'web', host: options.host, port: options.port }
|
||||
})
|
||||
|
||||
try {
|
||||
program.parse(argv, { from: 'user' })
|
||||
} catch (error) {
|
||||
/* v8 ignore next -- Commander only throws CommanderError from parse under exitOverride */
|
||||
if (!(error instanceof CommanderError)) throw error
|
||||
if (error.code === 'commander.helpDisplayed') return { mode: 'help', text: output.join('') }
|
||||
if (error.code === 'commander.version') return { mode: 'version', text: output.join('') }
|
||||
// Every other CommanderError is a parse failure; its message is the diagnostic.
|
||||
return { mode: 'error', message: error.message }
|
||||
}
|
||||
|
||||
/* v8 ignore next -- one action always resolves the invocation or parse throws above */
|
||||
if (resolved === undefined) throw new Error('dsh: argument parsing did not resolve a mode')
|
||||
return resolved
|
||||
return argv[0] === 'web' ? parseWeb(argv.slice(1), version) : parseRoot(argv, version)
|
||||
}
|
||||
|
||||
@@ -1,120 +1,33 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { ALL_INTERFACES_HOST, LOOPBACK_HOST, parseDshArgs } from '../src/args.ts'
|
||||
|
||||
const VERSION = '1.2.3'
|
||||
const parse = (argv: string[]) => parseDshArgs(argv, VERSION)
|
||||
const parse = (argv: string[]) => parseDshArgs(argv, '1.2.3')
|
||||
|
||||
/** Assert argv resolves to an error invocation whose message contains `needle`. */
|
||||
function expectError(argv: string[], needle: string): void {
|
||||
const result = parse(argv)
|
||||
expect(result.mode).toBe('error')
|
||||
if (result.mode !== 'error') throw new Error('expected error mode')
|
||||
expect(result.message).toContain(needle)
|
||||
}
|
||||
|
||||
describe('parseDshArgs — TUI (default mode)', () => {
|
||||
it('defaults to the TUI with no config and no resume when given no arguments', () => {
|
||||
describe('parseDshArgs', () => {
|
||||
it('routes each mode by its shape: default TUI, -p headless, web subcommand', () => {
|
||||
expect(parse([])).toEqual({ mode: 'tui' })
|
||||
})
|
||||
|
||||
it('carries a positional config into the TUI mode', () => {
|
||||
expect(parse(['custom.yml'])).toEqual({ mode: 'tui', config: 'custom.yml' })
|
||||
})
|
||||
|
||||
it('parses --resume in the space and inline forms, independent of a config positional', () => {
|
||||
expect(parse(['--resume', 'sess-1'])).toEqual({ mode: 'tui', resume: 'sess-1' })
|
||||
expect(parse(['--resume=sess-2'])).toEqual({ mode: 'tui', resume: 'sess-2' })
|
||||
expect(parse(['--resume', 'sess-3', 'app.yml'])).toEqual({ mode: 'tui', config: 'app.yml', resume: 'sess-3' })
|
||||
expect(parse(['app.yml', '--resume', 'sess-4'])).toEqual({ mode: 'tui', config: 'app.yml', resume: 'sess-4' })
|
||||
})
|
||||
|
||||
it('fails loud on a valueless or empty --resume rather than silently starting fresh', () => {
|
||||
expectError(['--resume'], '--resume')
|
||||
expectError(['--resume='], 'must not be empty')
|
||||
})
|
||||
|
||||
it('rejects a repeated --resume instead of silently keeping the last id', () => {
|
||||
expectError(['--resume', 'a', '--resume', 'b'], 'may be given only once')
|
||||
expectError(['--resume=a', '--resume=b'], 'may be given only once')
|
||||
})
|
||||
})
|
||||
|
||||
describe('parseDshArgs — headless', () => {
|
||||
it('routes -p / --prompt to the headless mode with the task text', () => {
|
||||
expect(parse(['--resume', 'sess', 'app.yml'])).toEqual({ mode: 'tui', config: 'app.yml', resume: 'sess' })
|
||||
expect(parse(['-p', 'do the thing'])).toEqual({ mode: 'headless', prompt: 'do the thing' })
|
||||
expect(parse(['--prompt', 'do the thing'])).toEqual({ mode: 'headless', prompt: 'do the thing' })
|
||||
})
|
||||
|
||||
it('routes to headless regardless of the prompt flag position', () => {
|
||||
// Positional-independent: the old `argv.includes('-p')` dispatch could not
|
||||
// tell a real prompt flag from one buried after other tokens.
|
||||
expect(parse(['-p', 'task'])).toEqual({ mode: 'headless', prompt: 'task' })
|
||||
})
|
||||
|
||||
it('rejects an empty prompt and a stray config positional', () => {
|
||||
expectError(['-p', ''], 'must not be empty')
|
||||
expectError(['-p', 'task', 'app.yml'], 'takes no config')
|
||||
})
|
||||
})
|
||||
|
||||
describe('parseDshArgs — web', () => {
|
||||
it('defaults the web mode to loopback and port 3080', () => {
|
||||
expect(parse(['web'])).toEqual({ mode: 'web', host: LOOPBACK_HOST, port: 3080 })
|
||||
})
|
||||
|
||||
it('accepts an explicit loopback or all-interfaces host and a valid port', () => {
|
||||
expect(parse(['web', '--host', ALL_INTERFACES_HOST, '--port', '8080']))
|
||||
.toEqual({ mode: 'web', host: ALL_INTERFACES_HOST, port: 8080 })
|
||||
expect(parse(['web', '--port', '0'])).toEqual({ mode: 'web', host: LOOPBACK_HOST, port: 0 })
|
||||
})
|
||||
|
||||
it('rejects a non-integer or out-of-range port with a --port diagnostic', () => {
|
||||
expectError(['web', '--port', 'abc'], '--port')
|
||||
expectError(['web', '--port', '70000'], '--port')
|
||||
expectError(['web', '--port', '-1'], '--port')
|
||||
it('fails loud instead of silently starting fresh or serving on bad input', () => {
|
||||
// An empty resume/prompt would otherwise be swallowed (agent-loop treats an
|
||||
// empty resume id as no-resume); a bad host/port must not reach the listener.
|
||||
expect(parse(['--resume=']).mode).toBe('error')
|
||||
expect(parse(['-p', '']).mode).toBe('error')
|
||||
expect(parse(['web', '--host', '10.0.0.1']).mode).toBe('error')
|
||||
expect(parse(['web', '--port', 'abc']).mode).toBe('error')
|
||||
expect(parse(['--bogus']).mode).toBe('error')
|
||||
})
|
||||
|
||||
it('rejects a host outside the allowed choices with a --host diagnostic', () => {
|
||||
expectError(['web', '--host', '10.0.0.1'], '--host')
|
||||
})
|
||||
|
||||
it('rejects an unexpected positional after web', () => {
|
||||
expectError(['web', 'extra'], 'too many arguments')
|
||||
})
|
||||
|
||||
it('fails loud when a root flag is placed before web instead of serving with it dropped', () => {
|
||||
// `dsh web -p x` and `dsh -p x web` both misrouted or dropped the flag under
|
||||
// the old `argv[0]==='web'` / `argv.includes('-p')` dispatch.
|
||||
expectError(['web', '-p', 'x'], "unknown option '-p'")
|
||||
expectError(['web', '--resume', 'y'], "unknown option '--resume'")
|
||||
expectError(['-p', 'x', 'web'], 'web takes no')
|
||||
expectError(['--resume', 'y', 'web'], 'web takes no')
|
||||
})
|
||||
|
||||
it('renders web usage for web --help', () => {
|
||||
const help = parse(['web', '--help'])
|
||||
expect(help.mode).toBe('help')
|
||||
if (help.mode !== 'help') throw new Error('expected help mode')
|
||||
expect(help.text).toContain('Usage: dsh web')
|
||||
})
|
||||
})
|
||||
|
||||
describe('parseDshArgs — help, version, and errors', () => {
|
||||
it('returns the rendered usage for --help / -h', () => {
|
||||
it('surfaces --help and --version as printable data, not a process exit', () => {
|
||||
const help = parse(['--help'])
|
||||
expect(help.mode).toBe('help')
|
||||
if (help.mode !== 'help') throw new Error('expected help mode')
|
||||
expect(help.text).toContain('Usage: dsh')
|
||||
expect(help.text).toContain('web')
|
||||
expect(parse(['-h']).mode).toBe('help')
|
||||
})
|
||||
|
||||
it('returns the version string for --version / -V', () => {
|
||||
expect(parse(['--version'])).toEqual({ mode: 'version', text: `${VERSION}\n` })
|
||||
expect(parse(['-V'])).toEqual({ mode: 'version', text: `${VERSION}\n` })
|
||||
})
|
||||
|
||||
it('reports an unknown option as an error invocation', () => {
|
||||
expectError(['--nope'], "unknown option '--nope'")
|
||||
expect(help).toMatchObject({ mode: 'help' })
|
||||
if (help.mode === 'help') expect(help.text).toContain('Usage: dsh')
|
||||
expect(parse(['--version'])).toEqual({ mode: 'version', text: '1.2.3\n' })
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user