Conversation
Agents were passing folder names and UUIDs to get, which hit the API 21-char validator. Fail locally with catalog vs folder guidance instead. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Haya Stern seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 070b4c82d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .await?; | ||
| } | ||
| DashboardsCmd::Delete { dashboard_id } => { | ||
| commands::dashboards::validate_dashboard_id(&dashboard_id)?; |
There was a problem hiding this comment.
When the credentialed E2E workflow runs cargo test --locked --test e2e -- --ignored --test-threads=1 (.github/workflows/e2e.yml:34-38), dashboards_delete_nonexistent still passes the 18-character nonexistent-id-000 and asserts that stderr contains Deleting dashboard (tests/e2e/dashboards/mod.rs:48-64). This validation now returns before that status line is emitted, so every E2E run will deterministically fail; use a nonexistent 21-character ID in that test or update it to assert the new local rejection.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Local id validation now rejects shorter strings before the Deleting status line, which would fail this ignored test in CI. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Context
Agents inventorying dashboards often pass a folder name (
BMLL,AWS) or a folder UUID tocx dashboards get. That hits GetDashboard, which returns400 Bad Request: dashboard_id is not 21 characters length. Catalog list already works; the mistake is using the wrong identifier. This PR fails those calls locally with a pointer to catalogidvs folder UUID/--folder.Linked Issues
N/A — prompted by a PoC inventory failure (folder names/UUIDs passed to get), not a tracked ticket.
Design
validate_dashboard_idlives insrc/commands/dashboards/mod.rsand requires a non-empty 21-character id (unicode scalar count).run_get,run_delete, andrun_check(by-id) call it before HTTP. Delete confirmation inmain.rsruns after this check so a folder name never prompts. Help text on get/delete/check and the cx-dashboards / cx-search-dashboard skills document the same rule: catalog itemidfor get; folder UUIDs only for--folder/--parent-id; inventory a folder by filtering catalog JSON.Key Decisions
catalog --folderin this PR (list-then-filter is enough for inventory).--yesprompt so a bad id is never treated as a destructive write.Changes
cx dashboards get|delete|check <id>reject ids that are not 21 characters, with an error that names catalog vs folders list vs filter-by-folder.cx-dashboards0.2.1,cx-search-dashboard0.1.1, deploy reference) tell agents not togeta folder id or name.Testing
cargo fmtcargo clippy --locked -- -D warningscargo test --locked --lib --test dashboard_commands --test dashboards --test console_urlsUnit tests cover accept nanoid / reject BMLL,
catalog, UUID, blank. Integration tests cover get without HTTP for a folder UUID, plus existing get/delete/check paths with 21-char ids.Risks & Rollout
CLI-only, no API or schema change. Ids that are not 21 characters never reached a successful GetDashboard before; those calls now fail earlier with a clearer message. Rollback is revert. Skills version bumps apply on next
cx skillsinstall.Out of Scope / Follow-ups
No
cx dashboards catalog --folderfilter flag. No change to dashboards-api validation. Folder get/list commands are unchanged.Made with Cursor