feat(cli): add trusted Cloud rooms and Fleet workflow#1358
Conversation
|
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds workspace-scoped Cloud room management, integration discovery and delegated credentials, agent identity commands, credential-safe SDK behavior, CLI documentation, and atomic broker port allocation for Fleet scenarios. ChangesCloud room lifecycle
Cloud integrations
Agent identity and registration
Atomic broker ports
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant CLI
participant CloudSession
participant CloudAPI
participant SecureFile
Operator->>CLI: run room or integration command
CLI->>CloudSession: ensure authenticated session
CloudSession-->>CLI: bound API host
CLI->>CloudAPI: authorized scoped request
CloudAPI-->>CLI: normalized response or credential lease
CLI->>SecureFile: write owner-only secret when requested
SecureFile-->>CLI: write result
CLI->>CloudAPI: revoke lease if file writing fails
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![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: 11df645277
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/cli/src/cli/commands/cloud-integration.ts (1)
442-443: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead
--jsonbranch inconnectionsandgrants. Both commands advertise--jsonbut theif/elsearms calljson(deps, payload)identically, so the flag has no effect and no human-readable renderer exists. Either implement a distinct default rendering or drop the--jsonoption and the redundant branch.
packages/cli/src/cli/commands/cloud-integration.ts#L442-L443: collapse to a singlejson(deps, payload)call (and remove--jsonon theconnectionscommand) or add a real non-JSON renderer.packages/cli/src/cli/commands/cloud-integration.ts#L540-L541: apply the same fix for thegrantscommand.🤖 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-integration.ts` around lines 442 - 443, The --json branches in the connections and grants command handlers are redundant because both paths call json(deps, payload). In packages/cli/src/cli/commands/cloud-integration.ts lines 442-443 and 540-541, collapse each conditional to a single json(deps, payload) call and remove the unused --json option from the connections command, unless implementing a distinct human-readable renderer instead.
🤖 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 `@CHANGELOG.md`:
- Around line 8-10: Change the changelog heading from “Unreleased - Minor” to
the standard “Unreleased” format, leaving the existing “### Added” section
unchanged.
In `@packages/cli/src/cli/commands/cloud-integration.ts`:
- Around line 394-402: Update the catalog command around workspaceId and the
request URL so the validated workspace is included in a workspace-scoped route,
using the established `/api/v1/workspaces/{id}/integrations/catalog` pattern and
encoding the ID before interpolation. Preserve the existing dynamic query
behavior and catalog normalization.
---
Nitpick comments:
In `@packages/cli/src/cli/commands/cloud-integration.ts`:
- Around line 442-443: The --json branches in the connections and grants command
handlers are redundant because both paths call json(deps, payload). In
packages/cli/src/cli/commands/cloud-integration.ts lines 442-443 and 540-541,
collapse each conditional to a single json(deps, payload) call and remove the
unused --json option from the connections command, unless implementing a
distinct human-readable renderer instead.
🪄 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: fbd3b00a-2541-4b05-bd2e-61083c15e1cf
📒 Files selected for processing (13)
CHANGELOG.mdpackages/cli/README.mdpackages/cli/src/cli/bootstrap.test.tspackages/cli/src/cli/commands/agent.test.tspackages/cli/src/cli/commands/agent.tspackages/cli/src/cli/commands/cloud-integration.test.tspackages/cli/src/cli/commands/cloud-integration.tspackages/cli/src/cli/commands/cloud-room.test.tspackages/cli/src/cli/commands/cloud-room.tspackages/cli/src/cli/commands/cloud.test.tspackages/cli/src/cli/commands/cloud.tspackages/cli/src/cli/lib/sdk-client.test.tspackages/cli/src/cli/lib/sdk-client.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/cli/src/cli/commands/cloud-room.ts (2)
824-824: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRole terminology mismatch: "observer" vs "viewer".
This help text says "participant or observer credential", but the role vocabulary used everywhere else in this file (e.g., line 551's
--roledescription,RoomRole) is "viewer or participant". This is likely a leftover naming inconsistency that could confuse users about actual role names.✏️ Fix
- .option('--json', 'Output the session, including its scoped participant or observer credential') + .option('--json', 'Output the session, including its scoped participant or viewer credential')🤖 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-room.ts` at line 824, Update the --json option description in the cloud-room command to say “viewer or participant credential” instead of “observer credential,” matching the role terminology used by RoomRole and the --role description elsewhere in the file.
602-646: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winInvitee email echoed to stdout in both delivery branches.
deps.logincludes the invitee's email in the confirmation message for both the email-delivery branch (line 607-608) and the manual-token branch (line 642) — flagged by static analysis as CWE-532. Since CLI stdout is routinely captured by CI logs, shell history, or telemetry, this can persist a third party's email address (invitees are often not the operator) beyond the interactive session, which is a compliance/privacy concern (GDPR/CCPA-style PII retention).Consider omitting the email from the human-readable confirmation, or truncating/masking it (the recipient already knows their own email; the operator already typed it), and reserve the full email for
--jsonoutput only.🔒 Example mitigation
- deps.log(`Sent ${options.role} room invitation to ${email}.`); + deps.log(`Sent ${options.role} room invitation.`);- deps.log(`Created ${options.role} room invitation for ${email}.`); + deps.log(`Created ${options.role} room invitation.`);🤖 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-room.ts` around lines 602 - 646, Remove the invitee email from the human-readable confirmation messages in the email-delivery branch and the manual-token branch of the room invitation flow. Update the deps.log calls around normalizeEmailInviteCreate and the final “Created” message to use role/status wording without echoing email; leave --json behavior unchanged so the full email remains available there.Source: Linters/SAST tools
🧹 Nitpick comments (1)
packages/cli/src/cli/commands/cloud-room.ts (1)
586-586: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the duplicated workspace-ID validators.
requireWorkspaceId()incloud-room.tsandworkspaceId()incloud-integration.tsduplicate the same Cloud workspace UUID /rw_validation. Move these patterns into a shared Cloud workspace validation helper and reuse them at both call sites so the accepted formats cannot drift.🤖 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-room.ts` at line 586, Replace the duplicated validation in requireWorkspaceId and workspaceId with a shared Cloud workspace validation helper. Move the existing UUID and rw_ format rules into that helper, then update both call sites to reuse it while preserving their current behavior and error handling.
🤖 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.
Outside diff comments:
In `@packages/cli/src/cli/commands/cloud-room.ts`:
- Line 824: Update the --json option description in the cloud-room command to
say “viewer or participant credential” instead of “observer credential,”
matching the role terminology used by RoomRole and the --role description
elsewhere in the file.
- Around line 602-646: Remove the invitee email from the human-readable
confirmation messages in the email-delivery branch and the manual-token branch
of the room invitation flow. Update the deps.log calls around
normalizeEmailInviteCreate and the final “Created” message to use role/status
wording without echoing email; leave --json behavior unchanged so the full email
remains available there.
---
Nitpick comments:
In `@packages/cli/src/cli/commands/cloud-room.ts`:
- Line 586: Replace the duplicated validation in requireWorkspaceId and
workspaceId with a shared Cloud workspace validation helper. Move the existing
UUID and rw_ format rules into that helper, then update both call sites to reuse
it while preserving their current behavior and error handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0930c651-7cb4-4300-be02-5d941cea789b
📒 Files selected for processing (4)
packages/cli/src/cli/commands/cloud-integration.test.tspackages/cli/src/cli/commands/cloud-integration.tspackages/cli/src/cli/commands/cloud-room.test.tspackages/cli/src/cli/commands/cloud-room.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/cli/src/cli/commands/cloud-integration.test.ts
- packages/cli/src/cli/commands/cloud-integration.ts
- packages/cli/src/cli/commands/cloud-room.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/e2e/fleet/harness.ts`:
- Around line 125-130: Update getFreeBrokerBasePort so the broker’s returned
base + 1 port is actually reserved through broker startup rather than merely
probed and released. Keep the reservation alive until the broker claims the
port, or move port allocation into the broker startup path, while preserving the
existing valid-port behavior.
🪄 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: 62720d14-bc7f-44f6-b436-74132961e9a0
📒 Files selected for processing (2)
tests/e2e/fleet/fleet-e2e.test.tstests/e2e/fleet/harness.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/core.ts`:
- Around line 153-156: Update the binaryArgs construction in createDefaultRelay
so persist is disabled when apiPort uses the default OS-assigned value of 0,
preserving ephemeral fallback broker behavior; enable persistence only when an
explicit nonzero port or other existing opt-in requires it.
In `@packages/cli/src/cli/lib/broker-lifecycle.ts`:
- Around line 336-345: Update the basePort === 0 startup flow around
candidate.getStatus and the apiPort validation so any rejected status check or
failed validation shuts down candidate before propagating the startup error.
Reuse the existing shutdown cleanup behavior, ensuring broker processes and
locks are removed while preserving successful return of the relay and assigned
API port.
🪄 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: bfce806a-b527-4e60-b048-a17b844bf547
📒 Files selected for processing (17)
CHANGELOG.mdcrates/broker/src/cli/mod.rspackages/cli/src/cli/commands/cloud-integration.test.tspackages/cli/src/cli/commands/cloud-integration.tspackages/cli/src/cli/commands/cloud-room.test.tspackages/cli/src/cli/commands/cloud-room.tspackages/cli/src/cli/commands/core.test.tspackages/cli/src/cli/commands/core.tspackages/cli/src/cli/lib/broker-lifecycle.tspackages/cli/src/cli/lib/sdk-client.test.tspackages/cli/src/cli/lib/sdk-client.tspackages/harness-driver/src/spawn-config.tspackages/sdk/src/__tests__/agent-relay.test.tspackages/sdk/src/agent-relay.tspackages/sdk/src/messaging/relaycast-client.tstests/e2e/fleet/fleet-e2e.test.tstests/e2e/fleet/harness.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/cli/src/cli/lib/sdk-client.ts
- CHANGELOG.md
- packages/cli/src/cli/commands/cloud-room.ts
- packages/cli/src/cli/commands/cloud-integration.ts
Final validation and review-feedback closure — exact head
|
Exact-head final signoff —
|
khaliqgant
changed the title
240d962 to
b95c855
Compare
1 participant
Summary
fleet spawn --session-refperform a real Claude/Codex resume, allow immediate same-name respawn after release, and advertise broker spawn/release capacity before the first remote spawnV1 trust model
Every invitee is a trusted full room participant with ordinary agent-level Relaycast collaboration actions. There is no viewer role, room integration grant/lease service, Cloud chat proxy, or Relaycast source/version change. The workspace owner key, owner-only Relaycast administration, and Agent Relay Cloud organization administration remain owner-only.
Validation
npm run typecheckcargo clippy --lib -- -D warnings6fc373bbeb9e90589fdaceac3087c65cc075dd1ccbbdeb2d526e43a5cdd84425) transferred over Tailscale tofinn-mini019f93bd-019e-7663-b1b0-b320acb71ea9at PID 26879; after CLI release, an immediate same-namefleet spawn --session-refcreated PID 28000 on the same node,/api/spawnedretained that exact thread UUID, the phase-two task did not contain the marker, and the resumed Codex context recalledPR1358_232A_MEMORY_5E8F14git diff --checkRelated