fix(cli): stop workspace create from orphaning its key, self-heal workspace active#1261
fix(cli): stop workspace create from orphaning its key, self-heal workspace active#1261khaliqgant wants to merge 6 commits into
Conversation
…ive resolve `agent-relay workspace create` minted a workspace directly against Relaycast (AgentRelay.createWorkspace) and stored only that key locally. `workspace active`/`agentworkforce login`/`deploy` resolve through Cloud's Postgres `relay_workspaces` row, which the direct-Relaycast create path never inserts — so every key from `workspace create` permanently 404s with "Workspace not found" on its very first resolve, even though the key itself is valid in Relaycast. Root-caused via live reproduction in #1260. - workspace create now goes through @agent-relay/cloud's unified `/api/v1/workspaces` create endpoint (same path relayfile setup already uses), which persists the missing Postgres row and returns a relaycastApiKey that actually resolves. Renamed --base-url to --api-url on the command to match `workspace active`'s existing flag. - resolveActiveWorkspace now self-heals: it opportunistically caches the resolved cloudWorkspaceId per local workspace alias, and if a key later stops resolving (rotated server-side, or an existing pre-fix orphan) it re-mints a working key for that SAME workspace via the session-authenticated `/{workspaceId}/join` endpoint and retries once — no new workspace is ever created, and any failure falls through to the original resolve error so the failure mode is unchanged when self-heal isn't possible (e.g. a key that never successfully resolved has no cached id to heal from). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request updates the workspace creation flow to go through Cloud's unified endpoint, ensuring that the returned relaycastApiKey is stored alongside a cached cloudWorkspaceId. It also introduces a self-healing mechanism in resolveActiveWorkspace that attempts to re-mint a key via a new /join endpoint if the primary key fails to resolve. The review feedback correctly points out that resolveWorkspaceDescriptor can throw errors (such as 401/403) that would propagate and bypass the self-heal block entirely, and suggests wrapping the primary resolution in a try-catch block to properly intercept these failures.
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.
| 'No active Agent Relay workspace found. Run `agent-relay workspace set_key <name> <key>` or `agent-relay workspace join <name> <key>`.' | ||
| ); | ||
| } | ||
| const { name, entry } = active; |
There was a problem hiding this comment.
If the server returns a non-404/405 error (such as 401 Unauthorized or 403 Forbidden due to an invalid or rotated key), resolveWorkspaceDescriptor will throw an error immediately instead of returning { lastUnsupported }. Since this call is not wrapped in a try-catch block, the error will propagate and completely bypass the self-heal block below.\n\nWrapping the primary resolution in a try-catch block ensures that any resolution failure (including 401/403 or network errors) can be intercepted and opportunistically self-healed if a cloudWorkspaceId is cached.
let primary: { descriptor: ActiveWorkspaceDescriptor } | { lastUnsupported: Error | null };\n try {\n primary = await resolveWorkspaceDescriptor(entry.key, options);\n } catch (err) {\n primary = { lastUnsupported: err instanceof Error ? err : new Error(String(err)) };\n }|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 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 |
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughWorkspace creation now uses Cloud-issued workspace IDs and relaycast API keys. The local store caches workspace IDs, while active-workspace resolution can rejoin stale workspaces and retry with refreshed credentials. CLI output and validation reflect the new Cloud response contract. ChangesWorkspace lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant CloudWorkspaceAPI
participant WorkspaceStore
CLI->>CloudWorkspaceAPI: createWorkspace(name, apiUrl)
CloudWorkspaceAPI-->>CLI: workspaceId and relaycastApiKey
CLI->>WorkspaceStore: setWorkspaceKey(name, relaycastApiKey, workspaceId)
CLI-->>CLI: print workspaceId and workspaceKey
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d979c359ca
ℹ️ 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".
| const { response } = await authorizedApiFetch(auth, endpoint, { | ||
| method: 'POST', | ||
| body: JSON.stringify(body), |
There was a problem hiding this comment.
Pass non-interactive options to POST auth retries
When workspace active runs with interactive: false and self-heal reaches joinWorkspace, this helper only passes interactive to ensureAuthenticated; if the join POST gets a 401 and token refresh fails, authorizedApiFetch defaults back to interactive browser login because no fourth argument is supplied. In headless/CI use this can open or wait for browser login instead of falling through to the original resolve error, so pass the same { interactive, refreshTimeoutMs } options here as tryGetJson does.
Useful? React with 👍 / 👎.
| const store = readWorkspaceStore(env); | ||
| store.workspaces[workspaceName] = { key }; | ||
| const existing = store.workspaces[workspaceName]; | ||
| const resolvedCloudWorkspaceId = cloudWorkspaceId ?? existing?.cloudWorkspaceId; |
There was a problem hiding this comment.
Clear cached workspace ids on manual key replacement
For an alias that already cached rw_old, a later agent-relay workspace set_key ops <new-key> or workspace join ops <new-key> preserves that old id here even though the new key may belong to another workspace. If the new key is stale, mistyped, or temporarily 404s, resolveActiveWorkspace will self-heal by joining rw_old and overwrite the user's replacement key, causing subsequent commands to target the previous workspace instead of failing or resolving the new one; only preserve the cache when the caller supplies the matching cloudWorkspaceId.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/cli/src/cli/commands/workspace.test.ts (1)
110-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the assertion if caching
workspaceIdon creation.If you accept the suggestion to cache the
workspaceIdimmediately during creation inworkspace.ts, ensure you update this test assertion to expect the fourth argument.♻️ Proposed fix
- expect(vi.mocked(setWorkspaceKey)).toHaveBeenCalledWith('new-ws', 'rk_live_new'); + expect(vi.mocked(setWorkspaceKey)).toHaveBeenCalledWith('new-ws', 'rk_live_new', undefined, 'rw_new');🤖 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/workspace.test.ts` at line 110, Update the setWorkspaceKey assertion in the workspace creation test to include the fourth workspaceId argument when workspace.ts caches it immediately during creation; preserve the existing expected workspace and key values.packages/cli/src/cli/commands/workspace.ts (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCache the
cloudWorkspaceIdimmediately upon creation.Since
workspace.workspaceIdis available from the creation response, consider passing it tosetWorkspaceKeyright away. This allows self-healing to work even if the workspace key gets rotated or orphaned before the user's first successful resolution.♻️ Proposed fix
- setWorkspaceKey(name, workspace.relaycastApiKey); + setWorkspaceKey(name, workspace.relaycastApiKey, undefined, workspace.workspaceId);🤖 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/workspace.ts` at line 69, Update the workspace creation flow around setWorkspaceKey to pass the newly created workspace.workspaceId immediately along with workspace.relaycastApiKey, ensuring cloudWorkspaceId is cached before any later resolution or key rotation.
🤖 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.
Nitpick comments:
In `@packages/cli/src/cli/commands/workspace.test.ts`:
- Line 110: Update the setWorkspaceKey assertion in the workspace creation test
to include the fourth workspaceId argument when workspace.ts caches it
immediately during creation; preserve the existing expected workspace and key
values.
In `@packages/cli/src/cli/commands/workspace.ts`:
- Line 69: Update the workspace creation flow around setWorkspaceKey to pass the
newly created workspace.workspaceId immediately along with
workspace.relaycastApiKey, ensuring cloudWorkspaceId is cached before any later
resolution or key rotation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 85199d8a-b6b0-4ae5-ae47-2f4d722d4698
📒 Files selected for processing (9)
packages/cli/src/cli/commands/workspace.test.tspackages/cli/src/cli/commands/workspace.tspackages/cli/src/cli/lib/sdk-command.test.tspackages/cli/src/cli/lib/sdk-command.tspackages/cloud/src/types.tspackages/cloud/src/workspace-store.test.tspackages/cloud/src/workspace-store.tspackages/cloud/src/workspaces.test.tspackages/cloud/src/workspaces.ts
Fixes 3 real issues found by cubic-dev-ai and gemini-code-assist on #1261: - resolveActiveWorkspace now catches a thrown non-404/405 resolve error (e.g. 401/403 from a rotated/invalid key) and folds it into the same `{ lastUnsupported }` shape as an exhausted-candidates failure, so self-heal still gets a chance whenever a cloudWorkspaceId is cached. Previously such an error propagated straight past the self-heal block (gemini-code-assist + cubic-dev-ai both flagged this independently). - setWorkspaceKey no longer carries a previously-cached cloudWorkspaceId forward on a plain key update — only an explicitly-supplied id is stored. Carrying it forward meant a user repointing an alias at a different workspace via `workspace set_key`/`workspace join` could have self-heal silently rejoin the OLD workspace if the new key transiently failed to resolve (cubic-dev-ai). - tryPostJson now forwards interactive/refreshTimeoutMs into authorizedApiFetch (matching tryGetJson's existing pattern), so a 401 mid-request during self-heal's join call can't trigger an unwanted interactive browser re-auth in a non-interactive context (cubic-dev-ai). - `workspace create` now caches cloudWorkspaceId immediately from the create response instead of waiting for the first successful resolve (coderabbitai nitpick, same underlying issue as the setWorkspaceKey fix). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Addressed the review feedback in the latest commit (26be7b3): cubic-dev-ai's 4 issues — all fixed:
gemini-code-assist flagged the same #4 independently — same fix covers it. coderabbitai nitpicks — both addressed (same underlying fix as #1/#2 above); updated the corresponding test assertion. New regression test added for the 401-bypasses-self-heal case, plus the |
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ansport/auth failures cubic-dev-ai (PR #1261 review, comment on workspaces.ts:456): the prior catch around resolveWorkspaceDescriptor's primary resolve call was too broad — it treated ANY thrown error (a network/transport failure, an ensureAuthenticated/session error, a malformed-response schema error from normalizeActiveWorkspaceDescriptor) as "this key doesn't resolve" and proceeded into self-heal, which mints a new key via /join and overwrites the locally-stored one. A transient 500, timeout, or auth blip could therefore churn a perfectly valid key. Adds a WorkspaceResolveHttpError, thrown only for a genuine non-2xx HTTP response from the resolve endpoint itself (401/403/etc., matching the 401-bypasses-self-heal fix from the prior commit). resolveActiveWorkspace's catch now narrows to only that type via instanceof, rethrowing everything else unchanged so self-heal can't fire on a transport/auth/schema failure. Regression test added: a network-level fetch failure now rejects with the original error, never calls /join, and never touches the stored key. Updated the existing 401 self-heal test to use 403 instead — a 401 makes authorizedApiFetch attempt a token refresh + retry first, which is a separate concern from what that test is verifying. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Fixes #1260 —
agent-relay workspace create(used byagentworkforce login --workspace <key>) minted a workspace directly against Relaycast and never created the corresponding Cloud Postgres row, soworkspace active/agentworkforce login/deployimmediately 404 with "Workspace not found" on that same key, forever. Root-caused with a live reproduction in #1260.What changed
workspace createnow goes through Cloud's unified/api/v1/workspacesendpoint (@agent-relay/cloud.createWorkspace, the same pathrelayfile setupalready uses) instead of the direct-RelaycastAgentRelay.createWorkspace. That endpoint persists the missing Postgres row, so the returned key actually resolves. Renamed the command's--base-urlflag to--api-urlto matchworkspace active's existing flag (it now points at Cloud, not the Relaycast gateway). Fails loudly instead of silently storing an unusable key if the response is missingrelaycastApiKey.resolveActiveWorkspaceself-heals. It opportunistically caches the resolvedcloudWorkspaceIdalongside each local workspace alias. If a key later stops resolving (rotated server-side, or a pre-fix orphan created before this PR), it re-mints a working key for that same workspace via the session-authenticatedPOST /api/v1/workspaces/{id}/joinendpoint and retries once — no new workspace is ever created. Best-effort: any failure (no session, no longer a member, no cached id to heal from) falls through to the original resolve error, so the failure mode is unchanged for cases self-heal can't help with.Why this shape
/joinendpoint already exists and is exactly the right primitive — session-authenticated, returns the canonicalrelaycastApiKeyfor a workspace the caller already belongs to. No new server-side (cloud repo) code was needed.cloudWorkspaceIdfrom a prior successful resolve, so it can't paper over a workspace the CLI has never proven the caller has access to.Test plan
packages/cloud/src/workspace-store.test.ts— caching/preservingcloudWorkspaceIdacross key updatespackages/cloud/src/workspaces.test.ts— cachescloudWorkspaceIdon success; self-heals viajoinWorkspaceon a stale key; falls through to the original error with no cached id; falls through when self-heal itself fails;joinWorkspaceunit testspackages/cli/src/cli/commands/workspace.test.ts—createstores the Cloud-issued key and prints it; fails loudly when the key is missingpackages/cli/src/cli/lib/sdk-command.test.ts(new) — assertswithSdkDefaults().createWorkspacecalls@agent-relay/cloud, never the direct-Relaycast SDKnpx vitest run packages/cloud packages/cli— 776 passed, 9 pre-existing unrelated failures (confirmed present onmainbefore this branch too), 0 regressionsnpx tsc --noEmitclean on bothpackages/cloudandpackages/clifor every file this PR touches🤖 Generated with Claude Code