Yichen Jiang
8ddc53f7a0
$DSH_HOME/config.yaml was an implicit composition layer: if the file existed, every launch applied an arbitrary Loader patch graph over the shipped tree, kept live by a dedicated HMR watcher. Three costs came from the implicitness, not the capability. A patch replaces its target row's whole config, so a file written months ago pins that row to the field set it knew and every default the shipped tree later adds silently stops applying. It competed with the typed settings namespaces llm-deepseek and llm-pi-ai already register, so which one wins was a function of layer order rather than meaning. And the explicit escape hatch it was supposedly redundant with did not exist on every surface: dsh -p, dsh meta, and dsh upgrade all rejected --config, so for them the implicit file was the only composition route at all. Complete the explicit layer first: --config and --config-replace now work on every booting surface. A headless --config-replace tree must still mount a webserver row, because that surface reaches its own agent over the same HTTP gateway the browser uses; AppCLIEntry names that contract in the failure instead of reporting a bare missing service. Then delete the implicit one. PERSONAL_CONFIG_FILENAME, loadPersonalPatches, watchPersonalPatches, and the config-only HMR row mounted for it are gone; a file left at that path is inert, and --dump-config no longer reads the Harness home. --config therefore stops *replacing* the personal overlay and simply *is* the user overlay. No migration: a user who wants the old behavior names the same file (dsh --config ~/.dsh/config.yaml), which a shell alias makes permanent.
135 lines
8.1 KiB
TypeScript
135 lines
8.1 KiB
TypeScript
import { afterEach, describe, expect, it, vi } from 'vitest'
|
|
import { parseDshArgs } from '../src/args.ts'
|
|
|
|
const parse = (argv: string[], experimentalEnv = false) => parseDshArgs(argv, '1.2.3', experimentalEnv)
|
|
|
|
/**
|
|
* `parseDshArgs` calls `process.exit` for `--help`/`--version`/errors and lets
|
|
* Commander print to the real streams; capture the exit code and mute output.
|
|
*/
|
|
function exitCode(argv: string[], experimentalEnv = false): number {
|
|
const exit = vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('exit') })
|
|
vi.spyOn(process.stdout, 'write').mockReturnValue(true)
|
|
vi.spyOn(process.stderr, 'write').mockReturnValue(true)
|
|
try {
|
|
parse(argv, experimentalEnv)
|
|
throw new Error(`expected ${JSON.stringify(argv)} to exit`)
|
|
} catch {
|
|
return exit.mock.calls.at(-1)?.[0] as number
|
|
} finally {
|
|
vi.restoreAllMocks()
|
|
}
|
|
}
|
|
|
|
afterEach(() => { vi.restoreAllMocks() })
|
|
|
|
describe('parseDshArgs', () => {
|
|
it('routes each mode by its shape: default TUI, -p headless, experimental and web subcommands', () => {
|
|
expect(parse([])).toEqual({ mode: 'tui' })
|
|
expect(parse(['--config', 'custom.yml'])).toEqual({ mode: 'tui', config: 'custom.yml' })
|
|
expect(parse(['--config-replace', 'tree.yml'])).toEqual({ mode: 'tui', configReplace: 'tree.yml' })
|
|
expect(parse(['--resume', 'sess', '--config', 'app.yml'])).toEqual({ mode: 'tui', config: 'app.yml', resume: 'sess' })
|
|
expect(parse(['-p', 'do the thing'])).toEqual({ mode: 'headless', prompt: 'do the thing' })
|
|
// Every booting surface takes the composition flags: with the personal
|
|
// overlay gone, naming a tree is the only way to compose one, so a
|
|
// surface that could not name one would have no composition path at all.
|
|
expect(parse(['-p', 'task', '--config', 'c.yml']))
|
|
.toEqual({ mode: 'headless', prompt: 'task', config: 'c.yml' })
|
|
expect(parse(['-p', 'task', '--config-replace', 'tree.yml']))
|
|
.toEqual({ mode: 'headless', prompt: 'task', configReplace: 'tree.yml' })
|
|
expect(parse(['meta', '--experimental', '--config', 'c.yml']))
|
|
.toEqual({ mode: 'meta', config: 'c.yml' })
|
|
expect(parse(['upgrade', '--experimental', '--config-replace', 'tree.yml']))
|
|
.toEqual({ mode: 'upgrade', configReplace: 'tree.yml' })
|
|
// Experimental subcommands run under the per-invocation flag or the env opt-in.
|
|
expect(parse(['meta', '--experimental'])).toEqual({ mode: 'meta' })
|
|
expect(parse(['meta'], true)).toEqual({ mode: 'meta' })
|
|
// Bare `web` carries no host/port: the shipped Web overlay owns the default.
|
|
expect(parse(['web'])).toEqual({ mode: 'web', dev: false })
|
|
expect(parse(['web', '--config', 'web.yml'])).toEqual({ mode: 'web', dev: false, config: 'web.yml' })
|
|
// Host/port are unvalidated pass-throughs (the webserver schema gates them
|
|
// at boot); the adapter only coerces the port string to a number.
|
|
expect(parse(['web', '--host', '0.0.0.0', '--port', '8080', '--dev', '--workspace-root', '/w']))
|
|
.toEqual({ mode: 'web', host: '0.0.0.0', port: 8080, dev: true, workspaceRoot: '/w' })
|
|
// Guided fresh-session entries carry nothing: bare mode discriminant only.
|
|
expect(parse(['upgrade', '--experimental'])).toEqual({ mode: 'upgrade' })
|
|
expect(parse(['upgrade'], true)).toEqual({ mode: 'upgrade' })
|
|
// --trusted-host is variadic and repeatable; authorities pass through unvalidated.
|
|
expect(parse(['web', '--trusted-host', 'harness.internal:3080', 'lab.internal', '--trusted-host', '10.0.0.9']))
|
|
.toEqual({ mode: 'web', dev: false, trustedHosts: ['harness.internal:3080', 'lab.internal', '10.0.0.9'] })
|
|
})
|
|
|
|
it('routes the dump flags per surface: composed with the user layer, or shipped only', () => {
|
|
expect(parse(['--dump-config'])).toEqual({ mode: 'dump-config', surface: 'tui', defaultOnly: false })
|
|
expect(parse(['--dump-config', '--config', 'c.yml']))
|
|
.toEqual({ mode: 'dump-config', surface: 'tui', defaultOnly: false, config: 'c.yml' })
|
|
expect(parse(['--dump-default-config'])).toEqual({ mode: 'dump-config', surface: 'tui', defaultOnly: true })
|
|
expect(parse(['web', '--dump-config'])).toEqual({ mode: 'dump-config', surface: 'web', defaultOnly: false })
|
|
expect(parse(['web', '--dump-config', '--config', 'w.yml']))
|
|
.toEqual({ mode: 'dump-config', surface: 'web', defaultOnly: false, config: 'w.yml' })
|
|
expect(parse(['web', '--dump-default-config'])).toEqual({ mode: 'dump-config', surface: 'web', defaultOnly: true })
|
|
// The two dump flags contradict each other; boot-only flags alongside a
|
|
// dump would be silently ignored; the shipped tree takes no user overlay.
|
|
expect(exitCode(['--dump-config', '--dump-default-config'])).toBe(1)
|
|
expect(exitCode(['--dump-default-config', '--config', 'c.yml'])).toBe(1)
|
|
expect(exitCode(['--dump-config', '--resume', 's'])).toBe(1)
|
|
expect(exitCode(['--dump-config', '-p', 'task'])).toBe(1)
|
|
expect(exitCode(['--dump-config', '--config-replace', 'tree.yml'])).toBe(1)
|
|
expect(exitCode(['web', '--dump-config', '--dump-default-config'])).toBe(1)
|
|
expect(exitCode(['web', '--dump-default-config', '--config', 'w.yml'])).toBe(1)
|
|
// A leaked dump flag on a subcommand that has none is a mistyped invocation.
|
|
expect(exitCode(['meta', '--experimental', '--dump-config'])).toBe(1)
|
|
expect(exitCode(['upgrade', '--experimental', '--dump-config'])).toBe(1)
|
|
})
|
|
|
|
it('exits nonzero instead of silently starting fresh or dropping inputs', () => {
|
|
// Empty resume/prompt would be swallowed downstream; --prompt mixed with
|
|
// TUI inputs must not lose them. (Bad host/port are gated by the webserver
|
|
// schema at boot, not here.)
|
|
expect(exitCode(['--resume='])).toBe(1)
|
|
expect(exitCode(['-p', ''])).toBe(1)
|
|
expect(exitCode(['--config', 'c.yml', '--config-replace', 'tree.yml'])).toBe(1)
|
|
expect(exitCode(['-p', 'x', '--config', 'c.yml', '--config-replace', 'tree.yml'])).toBe(1)
|
|
expect(exitCode(['-p', 'x', '--resume', 's'])).toBe(1)
|
|
expect(exitCode(['--bogus'])).toBe(1)
|
|
expect(exitCode(['bogus-positional'])).toBe(1)
|
|
// A default-surface flag on either side of `web` leaks into program.opts()
|
|
// but the web subcommand shares none of them: reject rather than serve.
|
|
expect(exitCode(['web', '-p', 'task'])).toBe(1)
|
|
expect(exitCode(['web', '--resume', 's'])).toBe(1)
|
|
expect(exitCode(['--config', 'c.yml', 'web'])).toBe(1)
|
|
expect(exitCode(['--config-replace', 'tree.yml', 'web'])).toBe(1)
|
|
// Same rule for each subcommand that shares no option with the default
|
|
// surface, so a leaked flag is a typo, not something to ignore.
|
|
// `meta` always starts fresh, so the session options are rejected; the
|
|
// composition flags are its own and only their combination is rejected.
|
|
expect(exitCode(['meta', '--experimental', '--resume', 's'])).toBe(1)
|
|
expect(exitCode(['meta', '--experimental', '-p', 'task'])).toBe(1)
|
|
expect(exitCode(['meta', '--experimental', '--config', 'c.yml', '--config-replace', 't.yml'])).toBe(1)
|
|
// `upgrade` always mints a fresh session, so `--resume` and a leaked
|
|
// parent flag are mistyped invocations; its own composition flags are not.
|
|
expect(exitCode(['upgrade', '--experimental', '--resume', 's'])).toBe(1)
|
|
expect(exitCode(['-p', 'task', 'upgrade', '--experimental'])).toBe(1)
|
|
// The pre-release command names have no compatibility aliases.
|
|
expect(exitCode(['experimental-meta'])).toBe(1)
|
|
expect(exitCode(['experimental-upgrade'])).toBe(1)
|
|
})
|
|
|
|
it('gates experimental subcommands behind --experimental or the env opt-in', () => {
|
|
// Bare `meta`/`upgrade` without either opt-in must fail loud, not run.
|
|
expect(exitCode(['meta'])).toBe(1)
|
|
expect(exitCode(['upgrade'])).toBe(1)
|
|
// A leaked default-surface flag stays a typo even when the gate is passed
|
|
// by the environment alone.
|
|
expect(exitCode(['meta', '--resume', 's'], true)).toBe(1)
|
|
// The flag and the env opt-in may coexist.
|
|
expect(parse(['meta', '--experimental'], true)).toEqual({ mode: 'meta' })
|
|
expect(parse(['upgrade', '--experimental'], true)).toEqual({ mode: 'upgrade' })
|
|
})
|
|
|
|
it('exits 0 for --help (disclosing web) and --version', () => {
|
|
expect(exitCode(['--help'])).toBe(0)
|
|
expect(exitCode(['--version'])).toBe(0)
|
|
})
|
|
})
|