Conversation
| "interval": true, | ||
| "interval-jitter": true, | ||
| "timeout": true, | ||
| "websocket": true, |
There was a problem hiding this comment.
🔴 Boolean flag websocket incorrectly mapped as value-taking in normalizeFlagArgs, causing positional argument swapping
The normalizeFlagArgs flag map marks "websocket": true, which tells the normalizer that --websocket takes a separate value argument. However, --websocket is a flag.Bool — Go's flag package never consumes the next argument for boolean flags (only --flag=value syntax works). When a user places --websocket between positional arguments (e.g., relayfile mount my-workspace --websocket ./src), the normalizer incorrectly consumes the next positional arg (./src) as the websocket flag's value, producing ["--websocket", "./src", "my-workspace"]. Go's flag parser then sees websocket=true with positionals ./src, my-workspace — silently swapping the workspace and local directory. This causes the CLI to mount the wrong directory against the wrong workspace ID.
Trace of the swapping behavior
Input: ["my-workspace", "--websocket", "./src"]
normalizeFlagArgsseesmy-workspace→ positional- Sees
--websocket→ flag,takesValue=true, consumes next arg./srcinto flagArgs - Result:
["--websocket", "./src", "my-workspace"] - Go's
flag.Boolsetswebsocket=true, treats./srcandmy-workspaceas positionals Arg(0)="./src"(used as workspace),Arg(1)="my-workspace"(used as local dir) — swapped!
Compare with --once which is correctly mapped as false.
| "websocket": true, | |
| "websocket": false, |
Was this helpful? React with 👍 or 👎 to provide feedback.
- CRITICAL: Path traversal prevention in mountsync (normalizeRemotePath + remoteToLocalPath) - HIGH: Fix mutex race window in sync(), restructure lock/unlock pattern - HIGH: Add Authorization header for WebSocket connections - HIGH: Fix script injection in publish-npm.yml (env blocks instead of inline interpolation) - HIGH: Replace direct push to main with PR-based version bump flow - HIGH: Fix release artifact naming mismatch for Homebrew formula - MEDIUM: Add io.LimitReader for bounded response reads - MEDIUM: Update math/rand to math/rand/v2 - MEDIUM: Add vitest/typescript devDependencies and test script to SDK - MEDIUM: Replace hardcoded paths with env vars in workflow files - MEDIUM: Add WebSocket event validation in TypeScript SDK - LOW: Replace fragile hashFiles with explicit directory check in CI Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🔴 WebSocket connection is destroyed after every sync cycle due to context derivation from per-cycle timeout
The connectWebSocket method at internal/mountsync/syncer.go:449 derives readCtx from ctx via context.WithCancel(ctx). However, ctx is the per-sync-cycle timeout context created in the callers (cmd/relayfile-cli/main.go:1147 and cmd/relayfile-mount/main.go:70), which has defer cancel(). When the sync cycle method returns, the timeout context is cancelled, which cascades to readCtx, causing wsjson.Read in the read loop to immediately return context.Canceled. This triggers handleWebSocketDisconnect, setting wsConn = nil. On the next cycle, needsWS is true again and a new connection is dialed, only to be immediately torn down at cycle end. This makes the WebSocket mode functionally broken — it never persists beyond a single sync cycle, defeating the purpose of real-time event streaming.
(Refers to line 449)
Prompt for agents
In internal/mountsync/syncer.go line 449, change `readCtx, cancel := context.WithCancel(ctx)` to derive from a long-lived context instead of the per-cycle timeout context. The WebSocket read loop needs a context that outlives individual sync cycles.
Option 1: Add a persistent context field to the Syncer struct (e.g., `wsRootCtx context.Context`) that is set once during initialization from a long-lived parent context, and derive readCtx from that.
Option 2: Change the `sync()` method signature and the callers in cmd/relayfile-cli/main.go (runMountLoop) and cmd/relayfile-mount/main.go to pass `rootCtx` separately from the per-cycle timeout context. The WebSocket connection should use rootCtx while the sync operations use the timeout context.
The same fix needs to be applied in both cmd/relayfile-mount/main.go:70-71 and cmd/relayfile-cli/main.go:1147-1148 where the callers create the per-cycle timeout context.
Was this helpful? React with 👍 or 👎 to provide feedback.
5 workflows building out the relayfile platform: bulk/export, CI, cloud server, developer experience, landing page.