fix(cli): resolve cloud enrollment workspace identifiers#1343
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesCloud enrollment workspace resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CloudEnrollCommand
participant CloudResolver
participant EnrollmentAPI
User->>CloudEnrollCommand: provide workspace identifier
CloudEnrollCommand->>CloudResolver: resolve identifier
CloudResolver-->>CloudEnrollCommand: return canonical cloudWorkspaceId
CloudEnrollCommand->>EnrollmentAPI: mint enrollment token
EnrollmentAPI-->>CloudEnrollCommand: return enrollment credentials
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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.
Code Review
This pull request updates the agent-relay cloud enroll --workspace command to resolve Cloud workspace UUIDs, unified rw_ IDs, and workspace keys before minting enrollment tokens, and handles incompatible Relaycast-only IDs gracefully. Feedback on these changes suggests improving error handling during workspace resolution by explicitly checking for rate-limiting (429 status) and handling cases where the response payload fails to parse as JSON to avoid misleading error messages.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
khaliqgant
left a comment
There was a problem hiding this comment.
Independent review verdict: REQUEST CHANGES (narrow Relay PR).
The pushed secret-selector repair is correct: rk_live_… and the reported numeric ID are rejected before auth/network I/O, resolver errors no longer echo selectors, the accepted rw_ grammar matches Cloud’s authoritative isValidWorkspaceId, and refreshed auth is carried into minting.
Two resolver-stage gaps remain before signoff:
- A resolver
429falls through to the generic error and dropsRetry-After; the existing rate-limit test covers only the later mint request. PreserveRetry-Afterat the new preflight stage and prove mint is not attempted. - A successful resolver response with invalid/non-JSON content is reported as “not linked,” which misdiagnoses a malformed server response. Emit a generic malformed-response error (without the selector) and test it.
Please also add resolver 403/404 regression coverage proving the redacted generic messages and that failures stop before mint. These findings match the two existing Gemini inline comments.
Scope note: even after these Relay changes, this PR does not fix the owner authorization denial. Production still requires the separate Cloud route change and deploy that admits only scoped CLI subjects and then enforces active owner/admin authority on the exact workspace.
GitHub does not allow the authenticated PR author account to submit an actual REQUEST_CHANGES review, so this is posted as a COMMENT review; the verdict above is unchanged.
khaliqgant
left a comment
There was a problem hiding this comment.
Independent re-review verdict on 043cfdcd: SIGNOFF — no remaining findings for this narrowly scoped Relay client change.
Verified line-by-line:
- accepted selectors are limited to non-secret Cloud UUIDs and authoritative lowercase
rw_IDs - unsupported/numeric/
rk_live_…values fail before login or network I/O and are not echoed - resolver 403/404 and generic failures do not trust or echo response payloads
- resolver 429 preserves
Retry-After - malformed descriptors and non-UUID
cloudWorkspaceIdvalues fail before minting - refreshed auth from resolution is used for the mint request
- direct
--tokenenrollment remains unchanged - PR #1342 has no code overlap; only
CHANGELOG.mdmay need a routine rebase resolution
Independent local verification: targeted cloud.test.ts (42/42), changed-file Prettier check, and full TypeScript typecheck all pass. The author reports the full suite at 1,297 passing tests; GitHub Actions for this head are still running as of this review.
Important scope boundary: this signoff does not mean the reported owner enrollment failure is fixed. Production still denies the correct Cloud UUID because the Cloud mint route rejects CLI bearer auth before owner/admin evaluation. The separate Cloud authorization fix, deploy, negative permission tests, and live owner enrollment verification remain mandatory. Do not treat this Relay PR as READY FOR MERGE for the end-to-end issue.
GitHub does not allow the authenticated PR-author account to approve its own PR, so this signoff is recorded as a COMMENT review.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
khaliqgant
left a comment
There was a problem hiding this comment.
Review from cloud-train lead (verified against live repro):
Client-side changes: APPROVE. The identifier-validation + resolver + honest-error-mapping design is right, and the diagnosis matches my live failure exactly — my original denial used the relaycast numeric id (204337648549896192), an unsupported identifier the server mislabeled as a permission failure. The rk_live non-disclosure test is a nice touch, as is refusing to mint on an invalid resolver descriptor.
But this PR alone does not fix enrollment — server-side bug confirmed live. Just now on sf-mini (agent-relay 10.6.5, khaliq's owner account, verified identical cloud whoami on both machines):
$ agent-relay cloud enroll --workspace 50587328-441d-4acb-b8f3-dbe1b3c5de99 --name sf-mini
You do not have permission to enroll nodes in workspace 50587328-441d-4acb-b8f3-dbe1b3c5de99. An organization owner or admin must run this command.
That's the proper cloud workspace UUID and the org owner being denied at /api/v1/fleet/enrollment-tokens — a server-side role-mapping defect (CLI subject → workspace owner/admin), which lives in the agentrelay.com service, not this repo. Please make sure the PR body names that server component + endpoint explicitly as the remaining half, so this doesn't get closed as 'fixed' while enrollment still fails for owners.
Two smaller notes: (1) confirm /api/v1/workspaces/{id}/resolve exists in the deployed cloud API — if it doesn't, the new resolver path 404s and the 'identifier was not found' message would mislead; state its availability in the PR body. (2) UNIFIED_WORKSPACE_ID_PATTERN pins rw_ + exactly 8 [a-z0-9] — fine if that's the format contract, but worth a comment citing where that contract is defined.
Scope
This Relay PR fixes the client-side workspace-ID namespace and diagnostics for session-based fleet enrollment. It does not claim to fix the production owner denial: the authoritative authorization defect is in
AgentWorkforce/cloudand requires a Cloud change plus deploy.What changed
--workspaceas a non-secret Cloud UUID or unifiedrw_ID, then resolve it throughGET /api/v1/workspaces/:workspaceId/resolvebefore mintingPOST /api/v1/fleet/enrollment-tokensrk_live_...secret) in errorsRoot cause and production blocker
There are two independent failures:
204337648549896192is not a supported Cloud workspace UUID or unifiedrw_workspace ID. A raw production request returns404 {"error":"Workspace not found"}fromGET /api/v1/workspaces/204337648549896192/resolve. The current Cloud workspace UUID is50587328-441d-4acb-b8f3-dbe1b3c5de99.packages/web/app/api/v1/fleet/enrollment-tokens/route.tscallsresolveRequestAuth(request)and then immediately rejects!requireSessionAuth(auth). The stored Cloud login issource=token,subjectType=cli, with scopesauth:workspace:follow-user, cli:auth, so every CLI bearer is rejected beforefindWorkspaceorrequireOrgOwnercan evaluate the owner's role./api/v1/auth/whoamireports the current organization role asowner.The Cloud fix must keep the secret-mint boundary strict: admit only an authenticated CLI subject with the required CLI scope(s), bind it to the exact target Cloud workspace, then query an active
owner|adminrole before minting.packages/web/lib/auth/deployment-api-token-access.tsis an existing exact-workspace/active-role DB-join reference;packages/web/app/api/v1/fleet/local-surface/route.tsis the existing deliberate session-or-cli:authroute pattern. The Cloud regression suite must prove owner/admin success and deny member/non-owner, unrelated workspace, non-CLI token, and missing-scope token cases. A production Cloud deploy is required after that fix.Verbatim evidence
Before, released
agent-relay10.6.5:After this Relay change, the unsupported identifier is rejected locally—without being echoed—before auth, resolution, or minting:
The canonical UUID reaches the authoritative mint route and proves the remaining Cloud blocker:
Direct production contract evidence:
Validation
exit:1; unsupported numeric/rk_live_...selectors reached resolution and were echoed before validation/redaction; resolver 429 responses lostRetry-After; malformed resolver descriptors were misreported as unlinked workspacesnpm run buildnpm run typechecknpm test— 96 files passed, 2 skipped; 1,297 tests passed, 18 skippednpm run lint— 0 errors (38 pre-existing warnings)npm run format:checkPR #1342 has no enrollment-source overlap; only the root changelog may need routine rebase conflict resolution.