Skip to content

fix(cli): resolve findings from a full command-surface audit - #6788

Merged
waleedlatif1 merged 3 commits into
stagingfrom
fix/cli-audit-findings
Aug 17, 2026
Merged

fix(cli): resolve findings from a full command-surface audit#6788
waleedlatif1 merged 3 commits into
stagingfrom
fix/cli-audit-findings

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Exercised all 147 CLI commands against a live deployment and fixed what broke.
  • Redirects no longer eat write bodies. A bare domain that 301s to www converted POST to GET and dropped the body, so reads worked while every create/update/delete failed with a misleading Workspace ID is required and sim login returned 405. The client and device flow now refuse to follow and name the endpoint to configure — following would also carry the API key and login secret to another origin.
  • --output text no longer truncates. The record path clamped at 160 chars before branching on format, corrupting the format built for pipes; tables exports download returned an unusable signed URL. Clamping is now table-only.
  • Undeclared commands format their output. Timestamp/duration/bytes/bool inference for API-owned keys, so workflows run stops printing durationMs: 9.145596999907866. User-defined table cells are untouched, and json/yaml still render from the raw payload.
  • Folder paths read and type as the name shown in the app. --folder '/Folder 1' works; the wire form still does too. Encoding mirrors the server's canonical segment encoder byte for byte.
  • sim logs list can attribute a run. Its workflow column could never populate because the default detail level omits the data.
  • Malformed endpoints error instead of crashing with a URL parse trace; a non-JSON response is reported as one instead of printing the HTML page; a workspace-key refusal names the personal-key remedy; validation errors drop union noise that contradicted itself; whoami verifies against the API and separates a bad key from an unreachable endpoint.
  • Three commands renamed to describe what they do, two flags aligned with their siblings. Old spellings still work — hidden, warned on stderr, kept out of help and docs.
  • Docs generator: a flag the runtime requires now reads Required: Yes (42 rows said No for --yes), and hidden commands are no longer documented.

Type of Change

  • Bug fix

Testing

Tested manually against a live deployment — every finding reproduced before the fix and re-verified after, including the redirect, path encoding, output formats, renamed spellings, and exit codes. Regression swept the pre-existing surface: both global-flag positions, exit codes, --limit 0 auto-paging, export/import round-trip, upload/get/delete, and the trace renderer.

Suite is 334 passed / 1 skipped (was 242 / 1). Every new assertion was proven red by reverting its source change and green on restore. lint, type-check, build, and all 29 audits in check:audits pass, including check:cli-docs and check:cli-api.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

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.
@vercel

vercel Bot commented Aug 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 17, 2026 10:44pm

Request Review

@cursor

cursor Bot commented Aug 17, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes affect CLI authentication, HTTP handling, and many command surfaces but not server-side auth or data paths; mis-handled redirects or exit codes could break CI scripts until callers adapt to the new whoami semantics.

Overview
This PR tightens the sim CLI after exercising the full command surface against a live deployment: HTTP behavior, output formatting, folder paths, auth diagnostics, and docs now match what users type and what scripts expect.

Redirects and endpoints — The client and login poll use redirect: 'manual' and refuse to follow 301/302/303 (and related statuses), because fetch would turn POSTs into bodyless GETs and could send the API key or login secret to another origin. Failures name a suggested --endpoint (including path prefixes on self-hosted installs). Malformed or non-http(s) endpoints fail with clear errors instead of raw URL parser crashes; HTML or other non-JSON bodies are summarized instead of dumped.

Output — Record clamping applies only in table mode so --output text (e.g. signed download URLs) stays full length. List pagination reports progress on stderr for multi-page fetches. Durations round sub-millisecond floats for display. Folder paths encode on the wire and decode in human-oriented columns.

Commands and flags — Three paths are renamed to match behavior (tables rows count, files restore, workflows deployment status); old paths remain hidden with warnings. Row find uses --query (alias --q). Knowledge document commands consistently use knowledgeBaseId. Destructive commands document -y/--yes as required confirmation. sim logs list defaults to fuller detail so workflow names can populate. sim whoami optionally verifies credentials via a workspace GET, with exit 1 for bad credentials and 2 when the check cannot run (--no-verify skips the call).

Docs — Generated and narrative CLI docs align with the above (whoami, folder wording, restore/count/status commands, exit codes).

Reviewed by Cursor Bugbot for commit ac958ac. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR resolves failures found during a full CLI command-surface audit and preserves compatibility for renamed commands and flags.

  • Prevents redirects from dropping write bodies or forwarding credentials to another origin.
  • Corrects pagination cleanup, output formatting, folder-path encoding, command metadata, and diagnostic handling.
  • Updates generated CLI documentation and expands regression coverage.

Confidence Score: 5/5

The PR appears safe to merge.

The previously reported pagination cleanup defect is fixed in both pagination loops, and no blocking failure remains.

Important Files Changed

Filename Overview
packages/sim-cli/src/http/client.ts Adds redirect refusal, safer response handling, pagination progress reporting, and unconditional progress cleanup.
packages/sim-cli/src/runtime/execute.ts Adds progress reporting and finally-based cleanup to generated-command cursor pagination.
packages/sim-cli/src/commands/auth.ts Adds live whoami verification with distinct credential and connectivity outcomes.
packages/sim-cli/src/output/render.ts Corrects format-specific truncation and improves inferred formatting for API-owned output fields.
packages/sim-cli/src/runtime/build.ts Implements compatible command and option renames while preserving hidden deprecated spellings.
scripts/generate-cli-docs.ts Corrects required-option documentation and excludes hidden compatibility commands.

Reviews (3): Last reviewed commit: "fix(cli): name a working API root when a..." | Re-trigger Greptile

Comment thread packages/sim-cli/src/http/client.ts Outdated
Comment thread packages/sim-cli/src/http/client.ts
Comment thread packages/sim-cli/src/auth/device-flow.ts
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.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

2 issues from previous reviews remain unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 00fca62. Configure here.

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.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit ac958ac. Configure here.

@waleedlatif1
waleedlatif1 merged commit ae21476 into staging Aug 17, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/cli-audit-findings branch August 17, 2026 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant