diff --git a/apps/docs/content/docs/en/cli/credentials.mdx b/apps/docs/content/docs/en/cli/credentials.mdx index 3cfdf26ec5b..de77e662048 100644 --- a/apps/docs/content/docs/en/cli/credentials.mdx +++ b/apps/docs/content/docs/en/cli/credentials.mdx @@ -21,7 +21,7 @@ sim credentials delete [options] | Argument | Required | Description | | --- | --- | --- | -| `credentialId` | Yes | Credential to update or disconnect. | +| `credentialId` | Yes | Credential to disconnect. | @@ -84,7 +84,7 @@ sim credentials update [options] | Argument | Required | Description | | --- | --- | --- | -| `credentialId` | Yes | Credential to update or disconnect. | +| `credentialId` | Yes | Credential to update. | diff --git a/apps/docs/content/docs/en/cli/files.mdx b/apps/docs/content/docs/en/cli/files.mdx index d4e9847ba85..f910224e817 100644 --- a/apps/docs/content/docs/en/cli/files.mdx +++ b/apps/docs/content/docs/en/cli/files.mdx @@ -256,7 +256,8 @@ sim files list [options] | Option | Required | Description | | --- | --- | --- | | `--folder ` | No | Folder path as shown in the app; the leading / is optional. | -| `--recursive` | No | Whether the folder filter includes files in subfolders. Defaults to true when a search is set, false otherwise, so listing a folder shows that folder while searching one looks through everything in it. Ignored when no folder filter is set, which already spans the workspace. The listed spellings are the whole accepted vocabulary and are case-sensitive; any other value is rejected. | +| `--recursive` | No | Whether the folder filter includes files in subfolders. Defaults to true when a search is set, false otherwise, so listing a folder shows that folder while searching one looks through everything in it. Ignored when no folder filter is set, which already spans the workspace. | +| `--no-recursive` | No | Send --recursive as false. | | `--scope ` | No | Which lifecycle set to list: `active` (default) for live files, `archived` for files a `DELETE` soft-deleted. `folderPath` resolves against active folders only, so pairing it with `scope=archived` returns an empty page when the containing folder was archived too. Accepted values: `active`, `archived`. | | `--search ` | No | Case-insensitive substring match against the file name. | | `--sort-by ` | No | Field used to sort the result. Sorting by `name` is case-sensitive and follows the storage collation, so do not rely on a case-insensitive order. Accepted values: `name`, `size`, `uploadedAt`, `updatedAt`. | diff --git a/apps/docs/content/docs/en/cli/reference.mdx b/apps/docs/content/docs/en/cli/reference.mdx index 2b5d368a73d..a2db7d27cbf 100644 --- a/apps/docs/content/docs/en/cli/reference.mdx +++ b/apps/docs/content/docs/en/cli/reference.mdx @@ -379,7 +379,7 @@ sim credentials delete [options] | Argument | Required | Description | | --- | --- | --- | -| `credentialId` | Yes | Credential to update or disconnect. | +| `credentialId` | Yes | Credential to disconnect. | @@ -448,7 +448,7 @@ sim credentials update [options] | Argument | Required | Description | | --- | --- | --- | -| `credentialId` | Yes | Credential to update or disconnect. | +| `credentialId` | Yes | Credential to update. | @@ -950,7 +950,8 @@ sim files list [options] | Option | Required | Description | | --- | --- | --- | | `--folder ` | No | Folder path as shown in the app; the leading / is optional. | -| `--recursive` | No | Whether the folder filter includes files in subfolders. Defaults to true when a search is set, false otherwise, so listing a folder shows that folder while searching one looks through everything in it. Ignored when no folder filter is set, which already spans the workspace. The listed spellings are the whole accepted vocabulary and are case-sensitive; any other value is rejected. | +| `--recursive` | No | Whether the folder filter includes files in subfolders. Defaults to true when a search is set, false otherwise, so listing a folder shows that folder while searching one looks through everything in it. Ignored when no folder filter is set, which already spans the workspace. | +| `--no-recursive` | No | Send --recursive as false. | | `--scope ` | No | Which lifecycle set to list: `active` (default) for live files, `archived` for files a `DELETE` soft-deleted. `folderPath` resolves against active folders only, so pairing it with `scope=archived` returns an empty page when the containing folder was archived too. Accepted values: `active`, `archived`. | | `--search ` | No | Case-insensitive substring match against the file name. | | `--sort-by ` | No | Field used to sort the result. Sorting by `name` is case-sensitive and follows the storage collation, so do not rely on a case-insensitive order. Accepted values: `name`, `size`, `uploadedAt`, `updatedAt`. | diff --git a/apps/docs/openapi-v2-resources.json b/apps/docs/openapi-v2-resources.json index 8b34648aa02..6a33679caf7 100644 --- a/apps/docs/openapi-v2-resources.json +++ b/apps/docs/openapi-v2-resources.json @@ -2411,12 +2411,12 @@ "name": "credentialId", "in": "path", "required": true, - "description": "Credential to update or disconnect.", + "description": "Credential to disconnect.", "schema": { "type": "string", "minLength": 1, "maxLength": 255, - "description": "Credential to update or disconnect." + "description": "Credential to disconnect." } }, { @@ -2487,12 +2487,12 @@ "name": "credentialId", "in": "path", "required": true, - "description": "Credential to update or disconnect.", + "description": "Credential to update.", "schema": { "type": "string", "minLength": 1, "maxLength": 255, - "description": "Credential to update or disconnect." + "description": "Credential to update." } }, { diff --git a/apps/sim/lib/api/contracts/v2/credentials.ts b/apps/sim/lib/api/contracts/v2/credentials.ts index a4f0091a8cf..8c2bf8bc3d9 100644 --- a/apps/sim/lib/api/contracts/v2/credentials.ts +++ b/apps/sim/lib/api/contracts/v2/credentials.ts @@ -467,9 +467,24 @@ export const v2CreateServiceAccountCredentialContract = defineRouteContract({ }, }) -export const v2CredentialParamsSchema = z +/** + * The credential a path addresses, named for what the route does to it. + * + * `PATCH` and `DELETE` sit on the same path but are not the same operation, and + * the OpenAPI document already publishes them as two components + * (`UpdateCredentialParams`, `DeleteCredentialParams`). One shared `describe()` + * forced both to read "update or disconnect", so the disconnect reference + * offered an update the route cannot perform. + */ +export const v2UpdateCredentialParamsSchema = z + .object({ + credentialId: nonEmptyIdSchema.max(255).describe('Credential to update.'), + }) + .strict() + +export const v2DeleteCredentialParamsSchema = z .object({ - credentialId: nonEmptyIdSchema.max(255).describe('Credential to update or disconnect.'), + credentialId: nonEmptyIdSchema.max(255).describe('Credential to disconnect.'), }) .strict() @@ -618,7 +633,7 @@ export type V2UpdateCredentialBody = z.input` twin, because the - * string union behind it has no agreed false spelling to send. That also - * makes it the way to withhold the negation from a field the API declares as - * `z.literal(true)`, where a sent `false` is a request the route rejects. + * A toggle declared here carries no generated `--no-` twin by default, + * because sending false is usually either meaningless — the server already + * defaults the field to false — or rejected outright, as on a field the API + * declares as `z.literal(true)`. {@link negatable} asks for the twin back on + * the one kind of field where false is a real request. */ boolean?: true + /** + * Give a {@link boolean} toggle its `--no-` twin after all. + * + * Withholding the twin is right for a one-way switch: most string-backed + * toggles sit on a field the server already defaults to false, so a negation + * would only restate the default, and on a `z.literal(true)` field it would + * send a request the route rejects. `files list --recursive` is neither — the + * API turns it on by itself as soon as a search is set, so without a spelling + * for false there is no way to search one folder without descending into it. + * Declared per flag rather than derived from the union's false spellings, + * which every one of these toggles publishes whether or not sending one means + * anything. + */ + negatable?: true /** * This field carries a folder path, so percent-encode each of its segments. * diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index 4c6127802c5..79500c574c8 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -10104,7 +10104,7 @@ export const V2_OPERATIONS = { method: 'DELETE', path: '/api/v2/credentials/[credentialId]', pathParams: ['credentialId'] as const, - pathParamDocs: { credentialId: 'Credential to update or disconnect.' }, + pathParamDocs: { credentialId: 'Credential to disconnect.' }, responseMode: 'json', summary: 'Disconnect Credential', query: { @@ -13411,7 +13411,7 @@ export const V2_OPERATIONS = { method: 'PATCH', path: '/api/v2/credentials/[credentialId]', pathParams: ['credentialId'] as const, - pathParamDocs: { credentialId: 'Credential to update or disconnect.' }, + pathParamDocs: { credentialId: 'Credential to update.' }, responseMode: 'json', summary: 'Update Credential', query: { diff --git a/packages/sim-cli/src/runtime/options.test.ts b/packages/sim-cli/src/runtime/options.test.ts index 94ba5bbcb4e..a4492463a2e 100644 --- a/packages/sim-cli/src/runtime/options.test.ts +++ b/packages/sim-cli/src/runtime/options.test.ts @@ -87,3 +87,97 @@ describe('a body field the contract documents as cleared by null', () => { expect(updateHelp().match(/sends the word/g)).toHaveLength(1) }) }) + +const LIST_FILES: OperationSpec = { + method: 'GET', + path: '/api/v2/files', + pathParams: [], + query: { + recursive: { + kind: 'boolean', + describe: + 'Whether the folder filter includes files in subfolders. The listed spellings are the whole accepted vocabulary and are case-sensitive; any other value is rejected.', + }, + folder: { kind: 'string', describe: 'Folder path as shown in the app.' }, + }, +} + +function listFilesHelp(): string { + const command = new Command('list') + addOperationOptions(command, 'listFiles', {}, LIST_FILES) + return command.helpInformation() +} + +describe('a boolean query param that documents its wire spellings', () => { + /** + * The API accepts twelve spellings for a boolean query param and says so. The + * CLI renders the field as a bare `--recursive` with a `--no-recursive` twin, + * neither of which takes a value, so the sentence pointed at a list the help + * never prints — in `--help` and in the generated reference alike. + */ + it('drops the vocabulary sentence a bare flag cannot honour', () => { + const help = listFilesHelp() + + expect(help).toMatch(/--recursive\s+Whether the folder filter includes files in subfolders\./) + expect(help).not.toMatch(/listed spellings/) + expect(help).not.toMatch(/case-sensitive/) + }) + + /** + * Stripping the clause must not eat the sentence that carries the meaning, and + * must not reach a flag that does take a value — those still publish their + * vocabulary, because there the reader can act on it. + */ + it('keeps the rest of the prose, and leaves valued flags untouched', () => { + const help = listFilesHelp() + + expect(help).toMatch(/--folder \s+Folder path as shown in the app\./) + expect(help).toMatch(/includes files in subfolders\./) + }) +}) + +const LIST_FILES_NEGATABLE: OperationSpec = { + method: 'GET', + path: '/api/v2/files', + pathParams: [], + query: { + recursive: { + kind: 'enum', + values: ['true', 'false'] as const, + describe: 'Whether the folder filter includes files in subfolders.', + }, + }, +} + +function negatableOpts(argv: string[]): Record { + const command = new Command('list').exitOverride() + addOperationOptions( + command, + 'listFiles', + { flags: { recursive: { boolean: true, negatable: true } } }, + LIST_FILES_NEGATABLE + ) + command.parse(argv, { from: 'user' }) + return command.opts() +} + +describe('a string-backed toggle the API defaults to true', () => { + /** + * `--recursive` alone had no way to say false, so a folder search always + * descended. The twin restores it. + */ + it('offers both spellings, and each sends what it says', () => { + expect(negatableOpts(['--recursive']).recursive).toBe(true) + expect(negatableOpts(['--no-recursive']).recursive).toBe(false) + }) + + /** + * Commander gives a lone `--no-x` an implicit `true` default, which would + * make every unqualified list send `recursive=true` and override the API's + * own conditional default. Declaring the positive flag first suppresses it — + * an ordering this asserts rather than trusts. + */ + it('leaves the field absent when neither spelling is given', () => { + expect(negatableOpts([])).not.toHaveProperty('recursive') + }) +}) diff --git a/packages/sim-cli/src/runtime/options.ts b/packages/sim-cli/src/runtime/options.ts index beb04102fb4..d1da7024017 100644 --- a/packages/sim-cli/src/runtime/options.ts +++ b/packages/sim-cli/src/runtime/options.ts @@ -51,6 +51,24 @@ function literalNullHint(documented: string, name: string): string { return /\bnull\b/i.test(documented) ? ` (--${name} null sends the word, not JSON null)` : '' } +/** + * The wire's boolean vocabulary, which a bare flag cannot offer. + * + * A boolean query param documents the spellings an HTTP caller may send + * (`true`, `1`, `yes`, `on`, …) and closes by saying the listed ones are the + * whole accepted set. That is correct for the API and publishes unchanged in + * the OpenAPI specs, but this CLI renders those fields as a bare `--flag` and + * its `--no-flag` twin, neither of which takes a value — so the sentence points + * at a list the reader is never shown, in `--help` and in the generated + * reference alike. Dropping it here keeps the terminal honest without weakening + * the prose REST callers actually need. + */ +const WIRE_VOCABULARY_SENTENCE = /\s*The listed spellings[^.]*\.\s*/g + +function withoutWireVocabulary(documented: string): string { + return documented.replace(WIRE_VOCABULARY_SENTENCE, ' ').trim() +} + function addFieldOption( command: Command, operation: V2OperationName, @@ -78,21 +96,24 @@ function addFieldOption( const documented = describeField(flag, descriptor, name, field) if (descriptor.kind === 'boolean' || flag.boolean) { + const booleanDoc = withoutWireVocabulary(documented) if (descriptor.required) { command.addOption( - new Option(`${short}--${name} `, `${documented} (required)`) + new Option(`${short}--${name} `, `${booleanDoc} (required)`) .choices(['true', 'false']) .makeOptionMandatory() ) return } - command.option(`${short}--${name}`, documented) + command.option(`${short}--${name}`, booleanDoc) // The twin exists to send an explicit `false`. Restating the positive // flag's prose here inverts its meaning ("Return only deployed workflows" // on the flag that stops doing exactly that), so it names its counterpart // instead and lets the reader look up one description, not two. - if (!flag.boolean) command.option(`--no-${name}`, `Send --${name} as false`) + if (!flag.boolean || flag.negatable) { + command.option(`--no-${name}`, `Send --${name} as false`) + } return } diff --git a/packages/sim-cli/src/runtime/request.test.ts b/packages/sim-cli/src/runtime/request.test.ts index 85cabb1d0a5..f7a071f3a1d 100644 --- a/packages/sim-cli/src/runtime/request.test.ts +++ b/packages/sim-cli/src/runtime/request.test.ts @@ -9,6 +9,27 @@ import { buildRequest, coerce, type FieldSpec } from './request' const WORKSPACE = 'ws_local' describe('buildRequest', () => { + /** + * `recursive` is the one string-backed toggle the API turns on by itself — + * it defaults to true as soon as a search is set. Its `--no-` twin has to + * reach the wire as an explicit false, or searching a single folder without + * descending into it is unsayable from the terminal. + */ + it('sends an explicit false for a negated string-backed toggle', () => { + const built = buildRequest( + 'listFiles', + [], + { folderPath: '/Reports', search: 'q3', recursive: false }, + WORKSPACE + ) + expect(built.query.recursive).toBe(false) + }) + + it('sends true when the same toggle is set positively', () => { + const built = buildRequest('listFiles', [], { recursive: true }, WORKSPACE) + expect(built.query.recursive).toBe(true) + }) + it('substitutes path params from positional args and injects the workspace', () => { expect(buildRequest('upsertTableRow', ['tbl_1'], { data: '{"a":1}' }, WORKSPACE)).toEqual({ path: '/api/v2/tables/tbl_1/rows/upsert',