diff --git a/apps/docs/content/docs/cli/files.mdx b/apps/docs/content/docs/cli/files.mdx index a6fc46dd930..1e7aa1588e2 100644 --- a/apps/docs/content/docs/cli/files.mdx +++ b/apps/docs/content/docs/cli/files.mdx @@ -634,16 +634,26 @@ API request and response schemas: [deleteFile](/api-reference/files/deleteFile). This archives the listed files; restore them with `files restore`. ```bash -sim files batch-delete [options] +sim files batch-delete [file-ids...] [options] ``` +**Arguments** + + + +| Argument | Required | Description | +| --- | --- | --- | +| `file-ids...` | No | File identifiers to delete. Same as --file-ids. | + + + **Options** | Option | Required | Description | | --- | --- | --- | -| `--file-ids ` | Yes | File identifiers to delete. (space-separated, or @path / @- with one value per line; @@value for a literal leading @). | +| `--file-ids ` | No | File identifiers to delete. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). | | `-y, --yes` | Yes | Confirm this operation. | diff --git a/apps/docs/content/docs/cli/reference.mdx b/apps/docs/content/docs/cli/reference.mdx index 9648f138e13..8dae294ffbb 100644 --- a/apps/docs/content/docs/cli/reference.mdx +++ b/apps/docs/content/docs/cli/reference.mdx @@ -972,16 +972,26 @@ Delete several files at once This archives the listed files; restore them with `files restore`. ```bash -sim files batch-delete [options] +sim files batch-delete [file-ids...] [options] ``` +**Arguments** + + + +| Argument | Required | Description | +| --- | --- | --- | +| `file-ids...` | No | File identifiers to delete. Same as --file-ids. | + + + **Options** | Option | Required | Description | | --- | --- | --- | -| `--file-ids ` | Yes | File identifiers to delete. (space-separated, or @path / @- with one value per line; @@value for a literal leading @). | +| `--file-ids ` | No | File identifiers to delete. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). | | `-y, --yes` | Yes | Confirm this operation. | diff --git a/apps/docs/scripts/fixtures/content-baseline.json b/apps/docs/scripts/fixtures/content-baseline.json index a535b99a55a..c990fe65fa0 100644 --- a/apps/docs/scripts/fixtures/content-baseline.json +++ b/apps/docs/scripts/fixtures/content-baseline.json @@ -4422,8 +4422,8 @@ { "lang": "bash", "meta": null, - "sha256": "41e9881b8582523af56643e45a5a1ba351eee353683c169a0dc8fc56b4d3ad66", - "bytes": 32, + "sha256": "c569c5e38502a1c1e8f6078954bc90b559dd80adb577bb4cf7b63ec7bbdb3aa7", + "bytes": 46, "line": 632 }, { @@ -11786,8 +11786,8 @@ { "lang": "bash", "meta": null, - "sha256": "41e9881b8582523af56643e45a5a1ba351eee353683c169a0dc8fc56b4d3ad66", - "bytes": 32, + "sha256": "c569c5e38502a1c1e8f6078954bc90b559dd80adb577bb4cf7b63ec7bbdb3aa7", + "bytes": 46, "line": 970 }, { diff --git a/packages/sim-cli/src/contract/commands.ts b/packages/sim-cli/src/contract/commands.ts index 0223e860639..b0044d2816b 100644 --- a/packages/sim-cli/src/contract/commands.ts +++ b/packages/sim-cli/src/contract/commands.ts @@ -1556,7 +1556,7 @@ export const CLI_CONTRACT: CliContract = { command: 'files batch-delete', describe: 'Delete several files at once', flags: { - fileIds: { list: true }, + fileIds: { list: true, trailing: true }, }, confirm: 'This archives the listed files; restore them with `files restore`.', }, diff --git a/packages/sim-cli/src/contract/types.ts b/packages/sim-cli/src/contract/types.ts index 5328656a9b6..ae81c6f7de4 100644 --- a/packages/sim-cli/src/contract/types.ts +++ b/packages/sim-cli/src/contract/types.ts @@ -69,6 +69,11 @@ export interface FlagSpec { * error, and a literal `#` value can still be passed. */ manifest?: true + /** + * The list's values may also follow the command as arguments, so `files batch-delete ...` + * reads like `rm`. Passing both forms is refused. At most one field per command. + */ + trailing?: true /** Take a JSON string. Implied for object/array/unknown fields. */ json?: boolean /** Read a scalar string verbatim from `@path` / `@-`; `@@` escapes a literal leading `@`. */ diff --git a/packages/sim-cli/src/runtime/build.ts b/packages/sim-cli/src/runtime/build.ts index e3ca431f120..94e3f610efb 100644 --- a/packages/sim-cli/src/runtime/build.ts +++ b/packages/sim-cli/src/runtime/build.ts @@ -14,6 +14,7 @@ import { isProfileWorkspacePath, PROFILE_INJECTED_FIELD, RESERVED_PROGRAM_FLAGS, + trailingListField, } from './request' import type { OperationSpec } from './types' @@ -312,6 +313,22 @@ function configureOperation( ) } + const trailing = trailingListField(spec) + if (trailing) { + const trailingFlags = Object.values(spec.flags ?? {}).filter((flag) => flag.trailing) + const descriptor = operationSpec.query?.[trailing] ?? operationSpec.body?.[trailing] + if (trailingFlags.length > 1) + throw new Error(`${operation} declares more than one trailing list`) + if (!descriptor || !flagSpecFor(operation, trailing).list) { + throw new Error(`${operation}.${trailing} is trailing but not a list request field`) + } + const name = flagNameFor(operation, trailing) + command.argument( + `[${name}...]`, + `${flagSpecFor(operation, trailing).describe ?? descriptor.describe} Same as --${name}.` + ) + } + if (spec.requestFields) { for (const field of spec.requestFields) { if ( diff --git a/packages/sim-cli/src/runtime/execute.ts b/packages/sim-cli/src/runtime/execute.ts index 7cddeaab597..2ffb1d7009d 100644 --- a/packages/sim-cli/src/runtime/execute.ts +++ b/packages/sim-cli/src/runtime/execute.ts @@ -22,6 +22,7 @@ import { flagNameFor, isProfileWorkspacePath, PROFILE_INJECTED_FIELD, + trailingListField, } from './request' import { foldPageEnvelope, renderPage, renderResult } from './result' import type { OperationSpec } from './types' @@ -416,6 +417,17 @@ export async function executeOperation( for (const [index, field] of (commandSpec.positionals ?? []).entries()) { requestFlags[camel(flagNameFor(operation, field))] = invocation[pathPositionalCount + index] } + const trailing = trailingListField(commandSpec) + const trailingValues = trailing + ? invocation[pathPositionalCount + (commandSpec.positionals?.length ?? 0)] + : undefined + if (trailing && Array.isArray(trailingValues) && trailingValues.length > 0) { + const name = flagNameFor(operation, trailing) + if (requestFlags[camel(name)] !== undefined) { + throw new SimApiError(`Pass the ${name} as arguments or with --${name}, not both`, 0) + } + requestFlags[camel(name)] = trailingValues + } foldRenamedFlags(operation, commandSpec, requestFlags) assertCapIsUsable(operation, requestFlags) diff --git a/packages/sim-cli/src/runtime/options.ts b/packages/sim-cli/src/runtime/options.ts index 13b1d443a78..ec44b7e2194 100644 --- a/packages/sim-cli/src/runtime/options.ts +++ b/packages/sim-cli/src/runtime/options.ts @@ -233,7 +233,13 @@ function addFieldOption( : flag.textSource ? ' (@path / @- reads a file or stdin verbatim, including trailing newlines; @@value for a literal leading @)' : '' - }${descriptor.required ? ' (required)' : ''}${literalNull ? literalNullHint(documented, name) : ''}` + }${ + descriptor.required + ? flag.trailing + ? ' (required, or pass the values as arguments)' + : ' (required)' + : '' + }${literalNull ? literalNullHint(documented, name) : ''}` const renamedFrom = flag.renamedFrom ?? [] const option = new Option(`${short}--${name} ${placeholder}`, describe) @@ -243,11 +249,12 @@ function addFieldOption( option.default(undefined, String(descriptor.default)) } // Commander's mandatory check runs before `executeOperation` can fold a - // renamed spelling onto the current one, so a required field that has been - // renamed would reject the very argv this exists to keep working. The - // requirement is not lost: `buildRequest` raises it against the current - // spelling once both have had their chance to supply the value. - if (descriptor.required && renamedFrom.length === 0) option.makeOptionMandatory() + // renamed spelling or trailing arguments onto the flag, so a required field + // supplied either way would be rejected. The requirement is not lost: + // `buildRequest` raises it once every spelling has had its chance. + if (descriptor.required && renamedFrom.length === 0 && !flag.trailing) { + option.makeOptionMandatory() + } command.addOption(option) for (const previous of renamedFrom) { diff --git a/packages/sim-cli/src/runtime/request.ts b/packages/sim-cli/src/runtime/request.ts index 65cf2a31848..dff6e56d2c6 100644 --- a/packages/sim-cli/src/runtime/request.ts +++ b/packages/sim-cli/src/runtime/request.ts @@ -61,6 +61,11 @@ export function flagSpecFor(operation: V2OperationName, field: string): FlagSpec return CLI_CONTRACT[operation]?.flags?.[field] ?? {} } +/** The list field whose values may also follow the command as arguments, if any. */ +export function trailingListField(commandSpec: CommandSpec): string | undefined { + return Object.entries(commandSpec.flags ?? {}).find(([, flag]) => flag.trailing)?.[0] +} + /** * Long and short flags the root program has already claimed. * diff --git a/packages/sim-cli/src/runtime/trailing-list.test.ts b/packages/sim-cli/src/runtime/trailing-list.test.ts new file mode 100644 index 00000000000..f2cff1ce3ef --- /dev/null +++ b/packages/sim-cli/src/runtime/trailing-list.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' +import { runEmbeddedCli } from '../embed' + +const identity = { endpoint: 'https://sim.test', apiKey: 'fixture', workspaceId: 'workspace' } + +/** Runs one embedded invocation, recording each request body it sends. */ +async function run(argv: string[]) { + const bodies: unknown[] = [] + const result = await runEmbeddedCli(argv, { + ...identity, + transport: async (_url, init) => { + bodies.push(JSON.parse(String(init?.body))) + return Response.json({ data: { deletedItems: { files: 2 } } }) + }, + }) + return { ...result, bodies } +} + +describe('files batch-delete ids as trailing arguments', () => { + it('sends ids given as arguments exactly as it sends --file-ids', async () => { + const trailing = await run(['files', 'batch-delete', 'wf_a', 'wf_b', '--yes']) + const flagged = await run(['files', 'batch-delete', '--file-ids', 'wf_a', 'wf_b', '--yes']) + expect(trailing.exitCode).toBe(0) + expect(trailing.bodies).toEqual([{ workspaceId: 'workspace', fileIds: ['wf_a', 'wf_b'] }]) + expect(flagged.bodies).toEqual(trailing.bodies) + }) + + it('refuses both forms at once, and neither, before any request', async () => { + const both = await run(['files', 'batch-delete', 'wf_a', '--file-ids', 'wf_b', '--yes']) + expect(both.exitCode).toBe(1) + expect(both.stderr).toContain('as arguments or with --file-ids, not both') + const neither = await run(['files', 'batch-delete', '--yes']) + expect(neither.exitCode).toBe(1) + expect(neither.stderr).toContain('--file-ids is required') + expect([...both.bodies, ...neither.bodies]).toEqual([]) + }) + + it('keeps the destructive confirmation for ids given as arguments', async () => { + const unconfirmed = await run(['files', 'batch-delete', 'wf_a']) + expect(unconfirmed.exitCode).toBe(1) + expect(unconfirmed.stderr).toContain('Re-run with --yes to confirm') + expect(unconfirmed.bodies).toEqual([]) + }) +})