Skip to content

feat: relayfile platform workflows#6

Open
khaliqgant wants to merge 16 commits intomainfrom
feat/auto-workflows
Open

feat: relayfile platform workflows#6
khaliqgant wants to merge 16 commits intomainfrom
feat/auto-workflows

Conversation

@khaliqgant
Copy link
Member

@khaliqgant khaliqgant commented Mar 25, 2026

5 workflows building out the relayfile platform: bulk/export, CI, cloud server, developer experience, landing page.


Open with Devin

@khaliqgant khaliqgant marked this pull request as ready for review March 25, 2026 18:25
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 7 additional findings in Devin Review.

Open in Devin Review

"interval": true,
"interval-jitter": true,
"timeout": true,
"websocket": true,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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"]

  1. normalizeFlagArgs sees my-workspace → positional
  2. Sees --websocket → flag, takesValue=true, consumes next arg ./src into flagArgs
  3. Result: ["--websocket", "./src", "my-workspace"]
  4. Go's flag.Bool sets websocket=true, treats ./src and my-workspace as positionals
  5. Arg(0)="./src" (used as workspace), Arg(1)="my-workspace" (used as local dir) — swapped!

Compare with --once which is correctly mapped as false.

Suggested change
"websocket": true,
"websocket": false,
Open in Devin Review

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>
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 9 additional findings in Devin Review.

Open in Devin Review

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant