mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(cli): resolve findings from a full command-surface audit (#6788)
* fix(cli): resolve findings from a full command-surface audit Exercised all 147 commands against a live deployment. Fixes the defects that surfaced, plus the docs and generator drift they exposed. Transport - Stop following redirects. A bare domain that 301s to www silently converted POST to GET and dropped the body, so reads worked while every write failed with a misleading validation error and login returned 405. Both the client and the device flow now explain the redirect and name the endpoint to configure, rather than carrying credentials off-origin. - Report a non-JSON response as one instead of printing the HTML page. - Name the personal-API-key remedy on a workspace-key refusal, reading the machine-readable code the API actually sends. - Drop union-branch noise from validation errors that contradicted itself. - Show paging progress on stderr for multi-page fetches. Output - Clamp record values for table only. text is the format built for pipes, and it was truncating signed URLs and tool source mid-value. - Infer timestamp, duration, bytes and boolean formatting for API-owned keys so undeclared commands stop printing raw ISO and float ms. Skips user-defined table cells and leaves json/yaml on the raw payload. - Render a declared-but-absent field as an em dash; billing credits were vanishing silently. Paths, naming and validation - Percent-encode folder paths per segment and decode them for display, so a folder reads and types as the name shown in the app. - Reject a malformed endpoint where it is set and where it resolves, instead of crashing with a URL parse trace. - Request the detail level logs list's own columns need; its workflow column could never populate. - Rename three commands that described themselves wrongly and align two flags with their siblings. Old spellings still work: hidden, warned on stderr, and kept out of help and docs. - Verify whoami against the API, separating a bad key from an unreachable endpoint, and report the workspace by name. - Correct the --yes help text, which advertised skipping a prompt that does not exist. Docs - Teach the docs generator that a flag required by the runtime is required, and that hidden commands are not documented. * fix(cli): clear the paging progress line when a page fails Progress is written without a trailing newline so it can be overwritten in place, and both paging loops cleaned it up only on success. A page that threw part-way through left `fetched 1200…` on the line the error was then printed onto, so the two ran together. * fix(cli): name a working API root when an endpoint redirects The suggested endpoint was the redirect target's origin, which drops a path prefix. A self-hosted deployment reached at https://host/sim was told to set https://www.host — not an API root, so following the advice replaced one broken endpoint with another. Derive it by stripping the request's own path from the target instead, so a prefix survives, and say nothing about --set-endpoint when the target resolves to the endpoint already configured: a trailing-slash or path normalization redirect keeps the origin, and naming the value the caller already has explains nothing. The login poll shared both faults and now shares the helper.
This commit is contained in:
@@ -79,8 +79,28 @@ function titleFor(name: string): string {
|
||||
.join(' ')
|
||||
}
|
||||
|
||||
/**
|
||||
* Commander records a hidden command on a private field and offers no getter,
|
||||
* so this narrows structurally rather than widening the command to `any`.
|
||||
*/
|
||||
function isHiddenCommand(command: Command): boolean {
|
||||
return (command as Command & { _hidden?: boolean })._hidden === true
|
||||
}
|
||||
|
||||
/** Every option a reader should be taught, in declaration order. */
|
||||
function documentedOptions(command: Command): Command['options'] {
|
||||
return command.options.filter((option) => !option.hidden)
|
||||
}
|
||||
|
||||
/**
|
||||
* Hidden entries are excluded for the same reason `--help` omits them: they are
|
||||
* spellings the CLI has retired and keeps working only so an existing script
|
||||
* does not break. Documenting one would teach the name being retired.
|
||||
*/
|
||||
function subcommands(command: Command): Command[] {
|
||||
return command.commands.filter((child) => child.name() !== HELP_COMMAND)
|
||||
return command.commands.filter(
|
||||
(child) => child.name() !== HELP_COMMAND && !isHiddenCommand(child)
|
||||
)
|
||||
}
|
||||
|
||||
/** Depth-first walk yielding every leaf command, in the order commander lists them. */
|
||||
@@ -144,17 +164,35 @@ function usageLine(entry: DocumentedCommand): string {
|
||||
const name = argument.variadic ? `${argument.name()}...` : argument.name()
|
||||
parts.push(argument.required ? `<${name}>` : `[${name}]`)
|
||||
}
|
||||
if (entry.command.options.length > 0) parts.push('[options]')
|
||||
if (documentedOptions(entry.command).length > 0) parts.push('[options]')
|
||||
return parts.join(' ')
|
||||
}
|
||||
|
||||
const REQUIRED_SUFFIX = /\s*\(required\)\s*$/i
|
||||
|
||||
/**
|
||||
* Commander help already spells required-ness inside the description of a
|
||||
* derived flag. The table states it in its own column, so the trailing marker
|
||||
* would read as "Yes | Workflow ID (required)".
|
||||
*/
|
||||
function stripRequiredSuffix(description: string): string {
|
||||
return description.replace(/\s*\(required\)\s*$/i, '')
|
||||
return description.replace(REQUIRED_SUFFIX, '')
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the flag must be supplied for the command to run.
|
||||
*
|
||||
* `option.mandatory` alone under-reports it. A destructive command's `--yes` is
|
||||
* enforced by the runtime rather than by Commander, deliberately: making it
|
||||
* mandatory would replace the refusal that names the consequence ("This deletes
|
||||
* the knowledge base and every document in it. Re-run with --yes to confirm.")
|
||||
* with Commander's bare "required option '--yes' not specified". The flag is
|
||||
* still required, and the description says so — which is the same marker
|
||||
* {@link stripRequiredSuffix} removes, so reading it here keeps the column and
|
||||
* the prose from contradicting each other.
|
||||
*/
|
||||
function isRequiredOption(option: Command['options'][number]): boolean {
|
||||
return option.mandatory || REQUIRED_SUFFIX.test(option.description || '')
|
||||
}
|
||||
|
||||
/** Help text is written without terminal punctuation; appended clauses need it. */
|
||||
@@ -208,12 +246,12 @@ function renderArguments(entry: DocumentedCommand): string[] {
|
||||
}
|
||||
|
||||
function renderOptions(entry: DocumentedCommand): string[] {
|
||||
const options = entry.command.options
|
||||
const options = documentedOptions(entry.command)
|
||||
if (options.length === 0) return []
|
||||
|
||||
const rows = options.map(
|
||||
(option) =>
|
||||
`| ${code(option.flags)} | ${option.mandatory ? 'Yes' : 'No'} | ${describeOption(option)} |`
|
||||
`| ${code(option.flags)} | ${isRequiredOption(option) ? 'Yes' : 'No'} | ${describeOption(option)} |`
|
||||
)
|
||||
|
||||
return [
|
||||
@@ -393,7 +431,9 @@ function renderReferencePage(
|
||||
'',
|
||||
'| Option | Description |',
|
||||
'| --- | --- |',
|
||||
...program.options.map((option) => `| ${code(option.flags)} | ${describeOption(option)} |`),
|
||||
...documentedOptions(program).map(
|
||||
(option) => `| ${code(option.flags)} | ${describeOption(option)} |`
|
||||
),
|
||||
'',
|
||||
]
|
||||
|
||||
@@ -467,7 +507,9 @@ function renderIndexPage(
|
||||
'',
|
||||
'| Option | Description |',
|
||||
'| --- | --- |',
|
||||
...program.options.map((option) => `| ${code(option.flags)} | ${describeOption(option)} |`),
|
||||
...documentedOptions(program).map(
|
||||
(option) => `| ${code(option.flags)} | ${describeOption(option)} |`
|
||||
),
|
||||
'',
|
||||
'## Command groups',
|
||||
'',
|
||||
|
||||
Reference in New Issue
Block a user