fix(cli): seven defects found by live-testing the CLI against staging - #7176
Merged
Conversation
`tools get github_comment` answered NOT_FOUND while `github_comment_v2` worked, though the toolId help promises an unversioned name resolves to the newest version. A superseded tool stays in the registry, so `resolveToolId` short-circuits on the exact hit and returns it unchanged; the visibility gate then refuses it because no visible block exposes a v1 tool. 204 base names were unresolvable this way. Resolution now walks the visible set newest-first, the way blocks already do. `resolveToolId` is untouched — execution depends on an exact id returning that exact id, and none of the 5182 visible ids change under the new path.
The archived folder listing built its path map from the archived rows alone, so a folder whose parent is still active came back as its own name. Deleting `a/sub` and restoring `a/sub` therefore disagreed — restore only matched the truncated `sub` — and the path and parentPath fields were wrong. The extra read is taken only for the archived scope; active and all keep their single query, which a test now pins. Restoring by path also stopped guessing. Archiving, recreating and archiving again leaves two archived folders with the same canonical path, and the resolver took the first match, silently restoring the wrong one. It now refuses and names the folder-id form.
A parentPath naming no folder returned 404 on the workflow, table and knowledge folder lists, and an empty page on files. The rule the codebase already publishes is the empty page: V2_FOLDER_FILTER_MISS is appended to the folderPath filter on six list surfaces, and resolveFolderPathFilter documents why a list must not become an existence oracle — a 404 claims the collection is missing and breaks a walk when a folder is deleted mid-pagination. Both TSDocs asserted the sibling folder lists already behaved that way. They did not; that premise is corrected here too. Mutations keep every 404. The miss short-circuits before the row query, because an unfiltered parent id lists the whole workspace.
`workflows activate create` switched which version production serves with no confirmation, while `rollback` refused without --yes. They are the same application operation under two transitions, so gating one and not the other was an accident of naming. The destructive-operation classification in the client tests listed activate as non-destructive, which is what kept its sweep from noticing. Moved, so two independent tests now hold the gate.
Refusing a root global printed a command to save it — without --profile, so following it verbatim wrote the default profile and left the named one untouched. The neighbouring suggestions in this file already carry the flag. Resolution matches resolveProfile, so SIM_PROFILE is covered too, and the profile name is redacted like the value beside it.
`tables rows batch-delete` exited 0 when none of the named rows existed, while the table equivalent exited 1 on the same shape. Only the id-list selection is checked: a filter answers without a requested count, so the guard self-excludes and an idempotent sweep still exits 0 on its second run.
Short ids draw from a 64-character alphabet containing one dash, so 1 in 64 open with one and commander reads it as an unknown option. It reaches `audit-logs get` and the custom-tool commands, and the escape was documented nowhere. The hint is appended only for a lone dash followed by two or more characters carrying an uppercase letter or digit — a shape no flag on this surface has — so a misspelt flag keeps commander's own suggestion.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Greptile SummaryThis PR fixes seven live-tested CLI and API defects while aligning generated documentation and contract projections.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/sim-cli/src/commands/configure.ts | The selected profile now follows CLI-over-environment precedence and is safely quoted for POSIX pasteable commands. |
| packages/sim-cli/src/runtime/build.ts | Unknown-option handling now provides the documented -- escape for ID-shaped positional arguments while deliberately preserving Commander diagnostics for ordinary flag typos. |
| apps/sim/lib/catalog/application/tool-scope.ts | Adds visible-set version resolution while preserving exact visible tool IDs. |
| apps/sim/lib/workspace-files/application/workspace-file-folders.ts | Archived path restoration now rejects ambiguous canonical paths instead of selecting an arbitrary folder. |
| apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts | Archived folder paths are built with active and archived ancestors so canonical paths are not truncated. |
| apps/sim/lib/workflows/application/workflow-folders.ts | Missing parent-path filters now short-circuit to an empty folder page. |
| apps/sim/lib/table/application/folders.ts | Table folder listing adopts the shared empty-page behavior for unmatched parent paths. |
| apps/sim/lib/knowledge/application/folders.ts | Knowledge folder listing adopts the shared empty-page behavior for unmatched parent paths. |
Reviews (3): Last reviewed commit: "fix(cli): quote a profile name a pasted ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Collaborator
Author
The suggestion configure prints is meant to be pasted, and it interpolated the profile name bare. Profile-name validation is creation-only by design — the validator says so, because a hand-written `[profile my stack]` has to keep resolving — so a name carrying whitespace, or a `;` that would end the pasted command and start another, reaches this message unchecked. Names that already satisfy the creation rule stay bare; the rest are single quoted, embedded quotes included. Redaction runs first, so a control character becomes a space and is then quoted rather than splitting the command.
waleedlatif1
changed the title
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Seven defects found by exercising every CLI domain against staging, each independently re-verified before it was fixed. Sixteen reported issues were investigated; eight did not survive verification and are not here.
tools get <unversioned>returned NOT_FOUND for 204 of 244 versioned families. A superseded tool stays registered, soresolveToolIdshort-circuits on the exact hit and the visibility gate then refuses it. Resolution now walks the visible set newest-first, as blocks already do.resolveToolIdis untouched — execution depends on an exact id returning that exact id, and none of the 5182 visible ids change.folders delete a/subandfolders restore a/subaddressed different folders and thepath/parentPathfields were wrong. Restoring by path also silently picked the first of several archived folders sharing a canonical path — it now refuses and names the folder-id form.workflows activate createswitched production with no confirmation whilerollback— the same application operation under a different transition — refused without--yes. The destructive-operation classification in the client tests listed activate as non-destructive, which is what kept its sweep from noticing.configure's refusal printed a command without--profile, so following it verbatim wrote the default profile.folders list --parent <missing>404'd on workflows, tables and knowledge and returned an empty page on files. The empty page is the rule the API already publishes on six list surfaces; two TSDocs asserted the folder lists already followed it, and they did not.tables rows batch-deleteexited 0 when nothing matched, while the table equivalent exited 1 on the same shape.-could not be passed at all (1 in 64 short ids), and the--escape was documented nowhere.Type of Change
Public API behavior change
GET /api/v2/{workflows,tables,knowledge}/folderswith aparentPathnaming no folder now returns 200 with an empty page instead of 404. Folder mutations keep their 404. Restore-by-path on an ambiguous archived path now returns a conflict instead of restoring an arbitrary match.Testing
Every fix carries a test that was mutation-verified — the fix reverted, the test confirmed red, the fix restored.
bun run lint,bun run check:audits(36/36), both type-checks, 782 CLI tests and 8363apps/simtests.Checklist