feat(cli): make Cloud workspace IDs discoverable from the CLI - #1413
Conversation
`cloud enroll --workspace` accepted only a Cloud workspace UUID or unified `rw_` ID, and no command printed either one: `cloud whoami` and `workspace list` showed names only, and `~/.agentworkforce/relay/workspaces.json` holds names plus keys. Enrolling from a stored login dead-ended unless you fetched the UUID out of the web dashboard. - Add `agent-relay cloud workspaces`, listing `GET /api/v1/workspaces` as `id slug name` (or `--json`). - Accept a workspace name or slug in `cloud enroll --workspace`, matched case-insensitively against that same listing. ID beats slug beats name so an exact identifier is never shadowed by someone else's display name; an ambiguous name is refused with the candidate IDs. - Print organization and workspace IDs in `cloud whoami`. A value carrying a live-credential prefix is still rejected before any network call, and neither the not-found nor the credential error echoes the value back — a mistyped secret must not reach the terminal. Cloud-provided names are sanitized before display, and `sanitizeForTerminal` now also strips bidirectional overrides so a workspace name cannot reorder its own line. Closes #1372 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WVG4uJ3erC9sq6rUW7551
📝 WalkthroughWalkthroughThe CLI adds Cloud workspace listing, selector resolution by name, slug, UUID, or unified ID, expanded ChangesCloud workspace flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CloudCLI
participant WorkspaceListAPI
participant EnrollmentTokenAPI
CloudCLI->>WorkspaceListAPI: Retrieve authenticated workspaces
WorkspaceListAPI-->>CloudCLI: Return workspace IDs, slugs, and names
CloudCLI->>CloudCLI: Resolve the workspace selector
CloudCLI->>EnrollmentTokenAPI: Mint an enrollment token
EnrollmentTokenAPI-->>CloudCLI: Return the token or an error
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4f5aa9f75
ℹ️ 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".
| throw new Error('A workspace name or ID is required for session-based enrollment.'); | ||
| } | ||
| if (!CLOUD_WORKSPACE_UUID_PATTERN.test(workspaceId) && !UNIFIED_WORKSPACE_ID_PATTERN.test(workspaceId)) { | ||
| if (looksLikeCredential(workspaceId)) { |
There was a problem hiding this comment.
Allow workspace names containing credential-like prefixes
looksLikeCredential treats any selector changed by the deliberately broad redactor as a credential, so a legitimate listed name such as br_team or Platform at_live_dev is rejected before the workspace listing is queried and can never be used with the newly supported name-based enrollment. Restrict this guard to the complete syntax of actual credentials, or attempt the local workspace match before rejecting an unmatched credential-like selector.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, and fixed in b07e8d2 by taking your second option — match against the listing first, and let the credential check pick only the error message for a selector that matched nothing.
Tightening the guard to "complete credential syntax" was the tempting fix, but it re-couples this file to a token grammar that lives in packages/cloud/src/redact.ts, and any prefix that grammar adds later would silently start eating workspace names again. Matching first makes the guard's breadth harmless by construction: a name that is really in your listing resolves no matter what it starts with.
The security property is unchanged. Matching is local against the already-fetched listing, so the selector is never put in a URL or a request body, and neither the credential message nor the not-found message echoes it. Test coverage now asserts both halves: a workspace named br_team enrolls, and a pasted rk_live_… / ocl_node_enr_… is refused with the value absent from stdout, stderr, and every authorizedApiFetch argument.
The one thing given up is that a pasted secret now costs one GET /api/v1/workspaces before it's rejected, instead of failing offline. That request carries the session's own credentials and not the selector, so nothing leaks — it just isn't free anymore.
Generated by Claude Code
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli/src/cli/commands/cloud.test.ts (1)
43-48: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winImport the real
redactCredentialValuesinstead of duplicating its regex.This mock reimplements the credential-prefix regex from
packages/cloud/src/redact.tsby hand.looksLikeCredentialincloud.tsdepends on this function's exact matching behavior to reject credential-like--workspaceselectors before any network call. If the real regex inpackages/cloud/src/redact.tschanges (a prefix added, removed, or its matching logic adjusted), this hardcoded mock won't reflect it, and the credential-rejection tests (lines 1030-1041) would keep passing against stale behavior while production behavior silently diverges.Use
vi.importActual('@agent-relay/cloud')to pull the realredactCredentialValuesinto the mock instead of reimplementing it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/cli/commands/cloud.test.ts` around lines 43 - 48, Update the cloud module mock in the test to obtain redactCredentialValues from vi.importActual("`@agent-relay/cloud`") instead of duplicating the credential regex locally. Remove the hand-rolled implementation while preserving the mock’s other behavior, so looksLikeCredential tests use the production redaction logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/cli/commands/cloud.ts`:
- Around line 481-508: Sanitize workspace IDs before terminal output in both
formatWorkspaceLabel and renderWorkspaceList. Apply sanitizeForTerminalLine to
entry.id and workspace.id, and use the sanitized workspace ID consistently for
width calculation, padding, and rendering.
---
Nitpick comments:
In `@packages/cli/src/cli/commands/cloud.test.ts`:
- Around line 43-48: Update the cloud module mock in the test to obtain
redactCredentialValues from vi.importActual("`@agent-relay/cloud`") instead of
duplicating the credential regex locally. Remove the hand-rolled implementation
while preserving the mock’s other behavior, so looksLikeCredential tests use the
production redaction logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 64561214-86d9-4985-80b1-c313a5698bbd
📒 Files selected for processing (8)
CHANGELOG.mdpackages/cli/README.mdpackages/cli/src/cli/bootstrap.test.tspackages/cli/src/cli/commands/cloud.test.tspackages/cli/src/cli/commands/cloud.tspackages/cli/src/cli/lib/formatting.test.tspackages/cli/src/cli/lib/formatting.tspackages/cli/src/cli/telemetry/events.ts
…dential `looksLikeCredential` gated the lookup, and it keys off the redactor's deliberately broad prefixes — so a workspace legitimately named `br_team` (or `Platform at_live_dev`) was rejected before the listing was ever queried and could never be selected by name. Match against the listing first. The credential check now only picks which error an *unmatched* selector gets, so a real name always resolves. The security property is unchanged: matching is local, so the selector never reaches an outbound request, and neither error echoes it. Also sanitize workspace IDs before display — they are Cloud-provided text like names and slugs — and measure column widths from the sanitized form so padding stays correct. The test mock now takes the real `redactCredentialValues` via `importOriginal` instead of a hand-copied regex that could drift from production. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WVG4uJ3erC9sq6rUW7551
Summary
Closes #1372.
agent-relay cloud enroll --workspaceaccepted only a Cloud workspace UUID or a unifiedrw_ID, and nothing in the CLI printed either one —cloud whoamiandworkspace listshow names only, and~/.agentworkforce/relay/workspaces.jsonholds names plus keys. Enrolling from a stored login dead-ended unless you fetched the UUID out of the web dashboard.This ships all three fixes the issue asked for:
agent-relay cloud workspaces(new) listsGET /api/v1/workspacesasid slug name, or--jsonfor scripts. This endpoint already existed and returns exactly the workspaces the stored login can use; the CLI just never called it.cloud enroll --workspacetakes a name or slug. Matching happens locally against that same listing, case-insensitively, with ID beating slug beating name so an exact identifier is never shadowed by someone else's display name. An ambiguous name is refused with the candidate IDs rather than picking one.cloud whoamiprints IDs, renderingOrganization: Acme (org-1)/Workspace: Chief HQ (<uuid>)instead of names alone.UUIDs and
rw_IDs still take the old fast path — no extra round trip, and the existing resolve/mint call sequence is unchanged for them.On not echoing the selector
The prior code rejected anything that wasn't a UUID or
rw_ID, which also covered a pastedrk_live_…workspace key. Now that arbitrary strings are accepted, that protection is preserved differently:redactCredentialValues, the single source of truth for what a credential looks like) gets the credential-specific message. That check only chooses the message — it deliberately does not gate the lookup, because those prefixes are broad enough (br_,at_live_, …) that gating would make a workspace legitimately namedbr_teampermanently unselectable. Thanks to @chatgpt-codex-connector for catching that; the first revision had it backwards.Cloud-provided names, slugs, and IDs are sanitized before display, and
sanitizeForTerminalnow also strips bidirectional overrides (U+202A–202E,U+2066–2069) so a workspace name can't reorder the line it's printed on to impersonate another one. That helper had no callers outside its own test before this change.Note on
rw_IDs in the listingThe issue suggested
cloud workspacesshowrw_IDs. The list endpoint returns Cloud workspace UUIDs, and turning each into anrw_ID means a/resolvecall per workspace — which callsresolveOrProvisionRelayWorkspaceand can provision a Relay workspace as a side effect (cf. #1378). Listing shouldn't mint anything, so the command prints the Cloud UUID, which is what--workspaceaccepts.Test Plan
Added to
packages/cli/src/cli/commands/cloud.test.ts:cloud whoamirenders org/workspace IDs, and still prints(none)for a login with no workspace selected.cloud workspaceslists IDs, strips terminal control sequences from a Cloud-provided ID and name, emits valid--json, reports an empty listing, and tells a logged-out user to runcloud login.cloud enroll --workspaceresolves a name case-insensitively; prefers an ID/slug match over another workspace's name; resolves a listed name that looks credential-like (br_team); refuses an ambiguous name with the candidate IDs; and reports an unmatched selector without echoing it.rk_live_…andocl_node_enr_…, asserting the value appears in neither stdout, stderr, nor anyauthorizedApiFetchargument.Plus a bidi-stripping case in
formatting.test.tsand the new command in thebootstrap.test.tscommand inventory. The test mock takes the realredactCredentialValuesviaimportOriginalrather than re-deriving its regex, so the prefix set can't drift out from under these tests.Gates run locally on
b07e8d2:npx vitest run— 1680 passed, 1 failed. The single failure ispackages/harnesses/src/ai-sdk/local-host-sandbox.test.ts, which fails identically onmainin this container (annvmline leaks into the captured shell stdout); unrelated to this diff. CI is green on it.npm run typecheck— clean.npm run lint— 76 warnings, 0 errors, byte-identical tomain(no new warnings).npx prettier --check— clean on every touched file.Not manually exercised against live Cloud: this container has no Cloud login, so the HTTP paths are covered by mocked-fetch tests only.
🤖 Generated with Claude Code
https://claude.ai/code/session_012WVG4uJ3erC9sq6rUW7551