Uh oh!
There was an error while loading. Please reload this page.
fix(setup): stop --skip-mount prompting for a mount dir; name the unbound-workspace failure - #461
Conversation
…ailure
Two production failures in `relayfile setup` 0.10.53.
1. `--skip-mount` still prompted "Local mount directory [./relayfile-mount]: "
whenever --local-dir was omitted. --skip-mount is the headless contract —
setup finishes without starting the mount process — so a run with --no-open
has no terminal to answer, and the read blocked forever. Under --skip-mount,
take the value the prompt's default would have produced instead of asking.
The directory is still used (mirror layout, and the printed
`relayfile mount <id> <dir>` hint), so nothing else changes.
2. `resolveCloudWorkspaceForRelayfile` collapsed two different Cloud states
into one message: "Cloud did not return cloudWorkspaceId and
relayfileWorkspaceId; workspace provisioning did not complete". For a
CLI-created workspace, provisioning DID complete — Cloud returns
relayfileWorkspaceId rw_* and cloudWorkspaceId: null, because nothing binds
a CLI-created relay workspace to a Cloud workspace. Split the checks so the
error names the actual state: the workspace is not bound to a Cloud
workspace, and that binding is Cloud's to establish.
Setup deliberately still fails on the unbound case rather than tolerating a
null cloudWorkspaceId. Continuing does not produce a usable workspace: the next
call, POST /api/v1/workspaces/{rw_*}/relayfile/delegated-token, authorizes
through the same app-workspace binding (hasWorkspaceAccess) and answers
404 workspace_not_found. Tolerating null would only relocate the failure to a
step that cannot explain it. The fix that makes `relayfile setup` work is
cloud-side.
Tests: all three new cases were confirmed to fail against the unfixed code —
the --skip-mount case reproduces the prompt and the stdin read.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kk8xnrAeA3QfS4A9n6dAd3
Session-Id: 0b0827b8-9619-4a88-9943-f5d30aa95869Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 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 |
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
The new tests called testing.T.Chdir, which landed in Go 1.24. go.mod targets go 1.22, CI pins GO_VERSION 1.22 and Contract builds with go-version-file: go.mod, so both checks failed to compile: cmd/relayfile-cli/setup_headless_test.go:35:4: t.Chdir undefined FAIL github.com/agentworkforce/relayfile/cmd/relayfile-cli [build failed] It passed locally because the author's toolchain was newer than the module's target — a green local run could not have caught this. Replaced with a chdirForTest helper doing the pre-1.24 dance (os.Getwd, os.Chdir, restore in t.Cleanup), rather than raising go.mod's Go version, which would change the module's minimum for every consumer to buy one test convenience. Verified with GOTOOLCHAIN=go1.22.12 go vet ./cmd/relayfile-cli/ — CI's exact toolchain, and vet compiles test files, which is where this broke. Behaviour verified on go1.26 (tests pass). Running a 1.22-built binary on this macOS is not possible (dyld: missing LC_UUID load command), so the run itself is left to CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVwyjaMnm1PTRC6m7oXHJV Session-Id: 13b5d5ea-39b6-42f3-86df-6d7bf3570e4b
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cmd/relayfile-cli/setup_headless_test.go">
<violation number="1" location="cmd/relayfile-cli/setup_headless_test.go:46">
P3: In the cleanup registered by chdirForTest, t.Fatalf calls FailNow (runtime.Goexit), which aborts the test goroutine from inside a cleanup. That stops the remaining cleanup functions (LIFO) and marks the test failed in a way that the testing framework does not reliably attribute to a normal cleanup failure. Use t.Errorf in the cleanup so the restore failure is reported without aborting the cleanup chain.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
| t.Cleanup(func() { | ||
| if err := os.Chdir(previous); err != nil { | ||
| t.Fatalf("restore working directory: %v", err) |
There was a problem hiding this comment.
P3: In the cleanup registered by chdirForTest, t.Fatalf calls FailNow (runtime.Goexit), which aborts the test goroutine from inside a cleanup. That stops the remaining cleanup functions (LIFO) and marks the test failed in a way that the testing framework does not reliably attribute to a normal cleanup failure. Use t.Errorf in the cleanup so the restore failure is reported without aborting the cleanup chain.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/relayfile-cli/setup_headless_test.go, line 46:
<comment>In the cleanup registered by chdirForTest, t.Fatalf calls FailNow (runtime.Goexit), which aborts the test goroutine from inside a cleanup. That stops the remaining cleanup functions (LIFO) and marks the test failed in a way that the testing framework does not reliably attribute to a normal cleanup failure. Use t.Errorf in the cleanup so the restore failure is reported without aborting the cleanup chain.</comment>
<file context>
@@ -28,11 +28,31 @@ func (r *refusingStdin) Read([]byte) (int, error) {
+ }
+ t.Cleanup(func() {
+ if err := os.Chdir(previous); err != nil {
+ t.Fatalf("restore working directory: %v", err)
+ }
+ })
</file context>
Uh oh!
There was an error while loading. Please reload this page.
Two verified production failures in
relayfile setup0.10.53.1.
--skip-mountstill prompted and blocked on stdinrunSetupWithOptionspromptedLocal mount directory [./relayfile-mount]:whenever--local-dirwas omitted, regardless of--skip-mount.--skip-mountis the headless contract — setup finishes without starting the mount process — so a run with--no-openhad no terminal to answer and the read blocked forever.Under
--skip-mount, setup now takes the value the prompt's default would have produced instead of asking. The directory is still used (the mirror layout is created there, and the printedrelayfile mount <id> <dir>hint names it), so nothing else about the run changes. Interactive setup is untouched.2.
setupcannot create a usable workspace — the root cause is cloud-sideresolveCloudWorkspaceForRelayfilecollapsed two different Cloud states into one message:For a CLI-created workspace that message is wrong. Provisioning did complete. What actually comes back is
relayfileWorkspaceId: "rw_…"andcloudWorkspaceId: null, because nothing binds a CLI-created relay workspace to a Cloud workspace:POST /api/v1/workspaceswithoutcreateAppWorkspace: truecallsregistry.create(...), which writes only the relay-workspace row. It never setsworkspaces.relayWorkspaceIdon the app workspace.GET /api/v1/workspaces/{id}/resolvereads that binding. On therw_*branch it returnscloudWorkspaceId: binding.appWorkspaceId, which isnullfor an unbound workspace. Only the app-workspace-UUID branch provisions (resolveOrProvisionRelayWorkspace).createAppWorkspace: true: that branch requiresrequireSessionAuth, and the CLI authenticates with acli:authbearer (source: "token"), so it would 403.This PR splits the two checks so the error names the real state instead of blaming provisioning.
Why this PR does not just tolerate a null
cloudWorkspaceIdTolerating it would not produce a usable workspace — it would only move the failure one step later, to a message that cannot explain itself. The next call setup makes is
POST /api/v1/workspaces/{rw_*}/relayfile/delegated-token, whosehasWorkspaceAccessgate is:which returns
404 workspace_not_found. So the CLI fails fast at the step that can name the cause.The fix that makes
relayfile setupwork is cloud-side:POST /api/v1/workspaces(thecli:auth, non-createAppWorkspacepath) must bind the new relay workspace to the caller's app workspace, orresolve'srw_*branch must provision that binding the way the UUID branch already does. That change belongs inAgentWorkforce/cloudand is out of scope here.Worth noting for whoever picks that up: the cloud tests that cover this path all mock
resolveAppWorkspaceByRelayWorkspaceIdto return a non-nullappWorkspaceId(workspaces/create-resolve.integration.test.ts,[workspaceId]/resolve/route.test.tsincl. "resolves a direct rw_ workspace id without app provisioning"). No test exercises the unbound case, which is why this shipped.Tests
New file
cmd/relayfile-cli/setup_headless_test.go:TestSetupSkipMountDoesNotPromptForLocalMountDirectory— headless--skip-mount --no-openrun with a stdin reader that errors on any read. Asserts setup succeeds, never reads stdin, never prints the prompt, prepares./relayfile-mount, and issues exactly create → resolve → delegated-token.TestSetupNamesUnboundCloudWorkspaceOnResolve— replays the production resolve payload (cloudWorkspaceId: null,relayfileWorkspaceId: "rw_1234abcd"). Asserts the error names the unbound binding and that setup stops before the delegated-token call.TestResolveCloudWorkspaceForRelayfileSeparatesUnprovisionedFromUnbound— the two states now produce distinct errors.All three were confirmed to fail against the unfixed code (the
--skip-mountcase reproducesLocal mount directory [./relayfile-mount]:and the blocking read).go build ./...,go vet ./cmd/relayfile-cli/,gofmt -l,go test ./...(full repo,-count=1oncmd/relayfile-cli, 126s) andscripts/check-contract-surface.shall pass. No HTTP surface changed, so the OpenAPI spec is untouched.Not run: no live run against production Cloud. The cloud-side mechanism above is established by reading
AgentWorkforce/cloud@origin/main, not by executing it.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kk8xnrAeA3QfS4A9n6dAd3