diff --git a/package.json b/package.json index 2d54ffa..c1c8419 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,7 @@ "scripts": { "build": "node -e \"require('node:fs').rmSync('dist', { recursive: true, force: true })\" && tsc -p tsconfig.json && tsc -p tsconfig.cjs.json && node scripts/finalize-build.mjs", "typecheck": "tsc -p tsconfig.json --noEmit && tsc -p tsconfig.cjs.json --noEmit", - "test": "node --test tests/feedback-*.test.mjs", + "test": "node --test tests/feedback-*.test.mjs tests/execution-argv.test.mjs", "ci:generate": "hollywood generate 'ci/**/*.ts' --source-root ci --output .", "test:package": "node scripts/verify-staging-package.mjs" }, diff --git a/scripts/verify-staging-package.mjs b/scripts/verify-staging-package.mjs index 697b6bd..b94eca7 100644 --- a/scripts/verify-staging-package.mjs +++ b/scripts/verify-staging-package.mjs @@ -43,7 +43,7 @@ try { [ '--test', ...readdirSync('tests') - .filter((name) => /^feedback-.*\.test\.mjs$/.test(name)) + .filter((name) => /^feedback-.*\.test\.mjs$/.test(name) || name === 'execution-argv.test.mjs') .map((name) => join('tests', name)), ], { diff --git a/src/cli/runtime.ts b/src/cli/runtime.ts index c3a807b..6ef7db1 100644 --- a/src/cli/runtime.ts +++ b/src/cli/runtime.ts @@ -13,6 +13,11 @@ import { type CliAuthDefinition, UsageError, runLogin, runLogout, storedCredenti import { createDiagnostics, diagnosticScope } from '../feedback/diagnostics.js'; +// Custom commands can carry literal input past flag/file coercion. +export class LiteralCliValue { + constructor(readonly value: unknown) {} +} + const LOGIN_COMMAND = 'dedalus login'; type OutputFormat = 'auto' | 'json' | 'jsonl' | 'pretty' | 'raw' | 'toon' | 'yaml'; @@ -1009,6 +1014,7 @@ const coerceValue = ( itemKind?: CliValueKind, label = 'value', ): unknown => { + if (value instanceof LiteralCliValue) return value.value; if (Array.isArray(value)) { const elementKind = kind === 'array' ? (itemKind ?? 'unknown') : kind; return value.map((item) => coerceValue(item, elementKind, undefined, label)); diff --git a/src/custom/README.md b/src/custom/README.md index de39bee..5db143f 100644 --- a/src/custom/README.md +++ b/src/custom/README.md @@ -22,3 +22,27 @@ node --test tests/machine-aliases.test.mjs node --test tests/ssh-paths.test.mjs python3 tests/ssh-picker-pty.py ``` + +# Execution arguments + +Use `dedalus machines exec --machine-id -- [args...]` to +create an execution. `machines executions`, `machines:exec`, and +`machines:executions` accept the same arguments, with optional `create`. +Arguments after `--` are literal: spaces, empty arguments, remote flags, and +`@file` references are sent unchanged. The executable itself cannot be empty. +Do not combine these arguments with `--command`. + +```sh +dedalus machines exec --machine-id 12345678-1234-4234-8234-123456789abc -- printf '%s\n' '@remote-file' +dedalus machines exec create --machine-id 12345678-1234-4234-8234-123456789abc --command '["echo", "hello"]' +``` + +Existing execution management subcommands remain available under every spelling. +This shortcut returns the execution response; use `output` to retrieve output. +The custom machines registration installs the shortcut after generated commands. +Keep the `LiteralCliValue` coercion guard when regenerating the runtime so remote +arguments never become local file reads. + +```sh +node --test tests/execution-argv.test.mjs +``` diff --git a/src/custom/executions.ts b/src/custom/executions.ts new file mode 100644 index 0000000..80f90b4 --- /dev/null +++ b/src/custom/executions.ts @@ -0,0 +1,51 @@ +/** Preserve remote argv without interpreting its flags or local file references. */ +import { Command } from 'commander' +import { LiteralCliValue, usageExitCode } from '../cli/runtime.js' + +export const addExecutionArguments = (program: Command): void => { + const generated = program.commands.find((command) => command.name() === 'machines:executions') + const machines = program.commands.find((command) => command.name() === 'machines') + if (!generated || !machines) return + const create = generated.commands.find((command) => command.name() === 'create') + if (!create) throw new Error('execution create command is missing') + + create.argument('[argv...]', 'Literal remote command and arguments after --') + const parseOptions = create.parseOptions.bind(create) + let literal: string[] = [] + create.parseOptions = (args) => { + const separator = args.indexOf('--') + literal = separator < 0 ? [] : args.slice(separator + 1) + return parseOptions(args) + } + create.hook('preAction', () => { + const argv = create.processedArgs[0] as string[] + if (argv.length === 0) return + if (argv.length !== literal.length) create.error('use -- before a remote command', { exitCode: 2 }) + if (create.getOptionValue('command') !== undefined) { + create.error('use either --command or arguments after --, not both', { exitCode: 2 }) + } + if (argv[0] === '') create.error('the executable after -- must not be empty', { exitCode: 2 }) + create.setOptionValue('command', new LiteralCliValue(argv)) + }) + + // Both resource spellings share the generated API handlers and options. + machines.enablePositionalOptions() + generated.enablePositionalOptions().alias('machines:exec') + const nested = usageExitCode(new Command('executions')) + .enablePositionalOptions() + .alias('exec') + .description('Run commands on a machine or manage executions') + .showHelpAfterError() + for (const command of generated.commands) { + nested.addCommand(command) + } + for (const group of [generated, nested]) { + const parse = group.parseOptions.bind(group) + group.parseOptions = (args) => { + const first = args[0] + const implicit = first?.startsWith('-') && first !== '--help' && first !== '-h' + return parse(implicit ? ['create', ...args] : args) + } + } + machines.addCommand(nested) +} diff --git a/src/custom/machines.ts b/src/custom/machines.ts index 8341b8d..927d09a 100644 --- a/src/custom/machines.ts +++ b/src/custom/machines.ts @@ -7,6 +7,7 @@ import { type CliClientOptionDefinition, type GlobalOptions, sdkClientOptions, writeError, errorExitCode, normalizeFormat, usageExitCode, } from '../cli/runtime.js' +import { addExecutionArguments } from './executions.js' import { pickSSHMachine } from './ssh-picker.js' import { connectMachine, type SSHAPI } from './ssh.js' @@ -39,6 +40,7 @@ export const addMachineAliases = ( clientOptions: readonly CliClientOptionDefinition[], options: AliasOptions = {}, ): Command => { + addExecutionArguments(program) const makeAPI = options.api ?? createMachineAPI const connect = options.connect ?? connectMachine const pick = options.pick ?? pickSSHMachine diff --git a/tests/execution-argv.test.mjs b/tests/execution-argv.test.mjs new file mode 100644 index 0000000..1ea51e2 --- /dev/null +++ b/tests/execution-argv.test.mjs @@ -0,0 +1,76 @@ +import assert from 'node:assert/strict' +import { spawn } from 'node:child_process' +import { resolve as resolvePath } from 'node:path' +import { createServer } from 'node:http' +import { test } from 'node:test' + +const run = (args, baseURL) => new Promise((resolve, reject) => { + const child = spawn(process.execPath, [resolvePath(process.env.FEEDBACK_PACKAGE_ROOT ?? '.', 'dist/esm/bin.js'), '--base-url', baseURL, + '--api-key', 'test', '--max-retries', '0', ...args], { stdio: ['pipe', 'pipe', 'pipe'], env: { PATH: process.env.PATH } }) + let stdout = '', stderr = '' + child.stdout.on('data', (data) => { stdout += data }) + child.stderr.on('data', (data) => { stderr += data }) + child.on('error', reject) + child.on('close', (status) => resolve({ status, stdout, stderr })) + child.stdin.end() +}) + +const withAPI = async (action) => { + const requests = [] + const server = createServer(async (request, response) => { + let source = '' + for await (const chunk of request) source += chunk + requests.push({ method: request.method, url: request.url, body: source ? JSON.parse(source) : undefined }) + response.setHeader('content-type', 'application/json') + response.end(JSON.stringify(request.method === 'GET' ? { items: [] } : { execution_id: 'execution' })) + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + try { await action(`http://127.0.0.1:${server.address().port}`, requests) } + finally { await new Promise((resolve) => server.close(resolve)) } +} + +const resources = [['machines', 'exec'], ['machines', 'executions'], ['machines:exec'], ['machines:executions']] + +test('invariant_execution_argv_reaches_the_wire_literally_for_every_resource_spelling', async () => { + await withAPI(async (baseURL, requests) => { + const argv = ['printf', '@does-not-exist', '\\@literal', '--help', '--command', 'two words', '', 'null', '123', '--'] + for (const resource of resources) { + for (const verb of [[], ['create']]) { + const result = await run([...resource, ...verb, '--machine-id', 'machine', '--cwd', '/tmp', '--', ...argv], baseURL) + assert.equal(result.status, 0, result.stderr) + assert.deepEqual(requests.at(-1), { method: 'POST', url: '/v1/machines/machine/executions', body: { command: argv, cwd: '/tmp' } }) + } + } + }) +}) + +test('invariant_invalid_execution_syntax_never_sends_a_request', async () => { + await withAPI(async (baseURL, requests) => { + for (const suffix of [ + ['--command', '["echo"]', '--', 'other'], + ['--', '', 'argument'], + ['echo'], + ['echo', '--', 'argument'], + ]) { + const result = await run(['machines', 'exec', 'create', '--machine-id', 'machine', ...suffix], baseURL) + assert.equal(result.status, 2, result.stderr) + } + assert.deepEqual(requests, []) + }) +}) + +test('invariant_existing_execution_flags_and_management_commands_remain_available', async () => { + await withAPI(async (baseURL, requests) => { + for (const resource of resources) { + const create = await run([...resource, 'create', '--machine-id', 'machine', '--command', '["echo","hello"]'], baseURL) + assert.equal(create.status, 0, create.stderr) + assert.deepEqual(requests.at(-1).body, { command: ['echo', 'hello'] }) + const list = await run([...resource, 'list', '--machine-id', 'machine'], baseURL) + assert.equal(list.status, 0, list.stderr) + assert.equal(requests.at(-1).method, 'GET') + const help = await run([...resource, '--help'], baseURL) + assert.equal(help.status, 0, help.stderr) + assert.match(help.stdout, /create.*argv/u) + } + }) +})