Refresh workspace token in relayfile login when one is registered#137
Merged
Conversation
Plain `relayfile login` only wrote ~/.relayfile/cloud-credentials.json, leaving the data-plane JWT in ~/.relayfile/credentials.json stale. Users hit `http 401 unauthorized: Token has expired` on every workspace operation after the cloud sign-in succeeded, and the only documented recovery was `--api-key` (requires a key the user doesn't have) or `relayfile setup` (re-triggers OAuth). After ensureCloudCredentials succeeds, resolve the active workspace and re-mint the data-plane JWT via joinWorkspaceViaCloud + persistJoinedWorkspace so both credential files are fresh in one step. - Add --workspace flag to target a specific workspace (errors if the refresh fails, matching the explicit-intent contract). - Add --skip-workspace-refresh to retain the cloud-only behavior. - No-op silently when no workspace is registered (the cold-start path the existing TestLoginDefaultsToCloudBrowserFlow exercises). - Soft-warn when the workspace exists but the join call fails so the cloud login still counts as successful. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 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)
📝 WalkthroughWalkthroughAdds optional workspace token refresh to Cloud browser login. New flags ChangesWorkspace Token Refresh on Cloud Login
Sequence DiagramsequenceDiagram
participant CLI as relayfile login (CLI)
participant CloudAPI as Cloud API
participant LocalStorage as Local Storage (credentials.json)
CLI->>CloudAPI: Browser login flow (OIDC)
CloudAPI-->>CLI: Cloud credentials acquired
alt skip-workspace-refresh not set
CLI->>CLI: Resolve workspace (flag or active)
CLI->>CloudAPI: POST /api/v1/workspaces/<id>/join
CloudAPI-->>CLI: Refreshed workspace JWT & server URL
CLI->>LocalStorage: Write refreshed workspace credentials
LocalStorage-->>CLI: Persist success
CLI-->>CLI: Print "Refreshed workspace token for <name> (<id>)"
else skip-workspace-refresh set
CLI-->>CLI: Skip workspace refresh
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@cmd/relayfile-cli/main.go`:
- Around line 1290-1295: When persistJoinedWorkspace(record, joined,
creds.APIURL, record.LocalDir) returns an error, change the logic so that if the
command was invoked with an explicit --workspace value (check the variable that
represents the flag/option used elsewhere in main.go, e.g., workspaceFlag or
opts.Workspace), propagate and return that error instead of printing a warning;
otherwise keep the current warning behavior. Update the branch that currently
prints "warning: workspace token refresh succeeded but persisting it failed: %v"
to conditionally return the error when --workspace was provided, and only print
the warning when no explicit workspace flag was used.
🪄 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: 974bf42e-a20f-448b-b843-549ba8104ce8
📒 Files selected for processing (2)
cmd/relayfile-cli/main.gocmd/relayfile-cli/main_test.go
5 tasks
Address CodeRabbit P1 on relayfile#137: a persist failure during workspace
token refresh was always downgraded to a warning, even when --workspace
was passed explicitly. That left credentials.json stale while the command
reported success — inconsistent with the join/resolve branches above,
which already return the error when --workspace is explicit.
Mirror that contract for the persist branch:
- Explicit --workspace: return fmt.Errorf("refresh workspace token:
persist refreshed credentials: %w", perr)
- Implicit default workspace: keep the warning so the cloud login itself
still counts as successful
Two new tests cover both branches by pre-creating credentials.json as a
non-empty directory so writeFileAtomically succeeds at the temp-write
stage but fails at the final os.Rename. That keeps cloud-credentials.json
writable (it lives at a different path) so the test only exercises the
persist-failure branch, not the cloud-credentials write branch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 `@cmd/relayfile-cli/main.go`:
- Around line 1238-1250: The code currently lets --skip-workspace-refresh
silently override --workspace; after flag parsing validate that the two flags
are not used together by checking the parsed pointers workspaceFlag and
skipWorkspace (e.g., if *workspaceFlag != "" && *skipWorkspace) and return a
user-facing error or exit non-zero with a clear message stating that --workspace
and --skip-workspace-refresh are mutually exclusive. Add this check immediately
after the fs.Parse(...) succeeds (before any workspace/token refresh logic) so
the command fails fast on contradictory input.
🪄 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: cd529b8d-7241-4082-b653-6af073b57bfc
📒 Files selected for processing (2)
cmd/relayfile-cli/main.gocmd/relayfile-cli/main_test.go
Address CodeRabbit minor on relayfile#137: the two flags are contradictory — --workspace says "refresh the JWT for this workspace", --skip-workspace-refresh says "don't touch the workspace JWT at all". Currently --skip-workspace-refresh silently wins, so relayfile login --workspace demo --skip-workspace-refresh succeeds while leaving credentials.json unchanged, which is the exact silent-success-with-stale-creds shape the rest of this PR is trying to eliminate. Fail fast with a clear error immediately after fs.Parse. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
relayfile loginonly wrote~/.relayfile/cloud-credentials.jsonand left the data-plane JWT in~/.relayfile/credentials.jsonstale. Every workspace op (status,tree,read, mount auth) returnedhttp 401 unauthorized: Token has expiredafter the cloud sign-in claimed success.ensureCloudCredentialssucceeds, resolve the active workspace and re-mint the data-plane JWT viajoinWorkspaceViaCloud+persistJoinedWorkspaceso both credential files are fresh in one step.--workspace NAME(errors if refresh fails) and--skip-workspace-refresh(retain cloud-only behavior). No-ops silently when no workspace is registered, so the cold-startTestLoginDefaultsToCloudBrowserFlowpath is unchanged.Background
Hit while verifying the v2 file-native adapter rollout against
rw_517d60b6. After the data-plane JWT expired mid-session, the only documented recoveries wererelayfile login --api-key(needs an API key the user doesn't have) orrelayfile setup(re-triggers OAuth and mount). TherefreshMountAuthloop insiderelayfile mountalready has the right shape — this PR lifts the same join+persist into the user-facinglogincommand.Test plan
go test ./cmd/relayfile-cli/— full suite green (4.5s).go vet ./cmd/relayfile-cli/clean.TestLoginRefreshesWorkspaceTokenForDefaultWorkspace— asserts/api/v1/workspaces/<id>/joinfires andcredentials.jsonis rewritten with the refreshed token + server URL.TestLoginSkipsWorkspaceRefreshWhenFlagSet— asserts--skip-workspace-refreshsuppresses the join call and leavescredentials.jsonabsent.TestLoginDefaultsToCloudBrowserFlow(cold-start, no workspace) still passes — silent no-op path verified.relayfile loginon a host with an expiredcredentials.json; verifyrelayfile statussucceeds without a follow-up command.🤖 Generated with Claude Code