fix(sdk-swift): align broker client with v7, rename RelayCast → AgentRelayClient#989
Merged
Conversation
Drop the legacy hello/hello_ack WebSocket handshake and stop sending
actions over /v1/ws. RelayCast now subscribes to /ws as a read-only
event stream and uses the broker HTTP API (POST /api/spawn, DELETE
/api/spawned/{name}, POST /api/send) for spawn, release, and message
operations.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSwift SDK realigns with broker v7: client renamed to AgentRelayClient, WS handshake removed, events consumed from /ws, actions moved to HTTP via new RelayHTTP, event decoding simplified to bare BrokerEvent, README/CHANGELOG/tests updated, and CI workflows detect sdk-swift-only changes. ChangesSwift SDK v7 Broker Protocol Alignment
Sequence DiagramsequenceDiagram
participant Client as AgentRelayClient
participant Core as RelayCore
participant Transport as RelayTransport (WS)
participant HTTP as RelayHTTP
participant Broker as Broker (v7)
Client->>Core: init(transport, http)
Client->>Core: ensureConnected()
Core->>Transport: connect() (WebSocket upgrade to /ws)
Transport-->>Core: upgrade success
Core->>Core: start frame router
Core-->>Client: emit .connected
Client->>HTTP: POST /api/spawn (spawnAgent)
HTTP->>Broker: POST /api/spawn
Broker-->>HTTP: 200/4xx
HTTP-->>Client: response
Transport->>Core: incoming WebSocket frame
Core->>Client: BrokerEvent (decoded bare)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: daed3414d5
ℹ️ 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".
Comment on lines
134
to
138
| func registerOrRotate(name: String) async throws -> AgentRegistration { | ||
| try await ensureConnected() | ||
| return AgentRegistration(agentName: name, token: name) { agentName, token in | ||
| // v7 brokers do not have a register/rotate endpoint — agents are | ||
| // identified by name and authenticated via the broker API key. | ||
| AgentRegistration(agentName: name, token: name) { agentName, token in | ||
| AgentClient(core: self, agentName: agentName, token: token) |
There was a problem hiding this comment.
registerOrRotate no longer opens the WebSocket connection and now just returns a local AgentRegistration. In flows that rely on registerOrRotate as the startup step, this leaves /ws disconnected, so brokerEvents/connectionState streams stay silent until some separate call (like channel.subscribe()) happens. This is a behavior regression from the previous implementation and breaks callers that only register then listen.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| func releaseAgent(name: String, reason: String? = nil) async throws { | ||
| try await ensureConnected() | ||
| try await send(.releaseAgent(ReleaseAgentPayload(name: name, reason: reason))) | ||
| let escaped = name.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? name |
There was a problem hiding this comment.
Encoding with .urlPathAllowed does not escape /, so names containing slashes (for example team/worker) generate DELETE /api/spawned/team/worker instead of /api/spawned/team%2Fworker. That can target the wrong route/agent or 404; the HTTP path should use segment-safe encoding equivalent to encodeURIComponent for parity with other SDKs.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 5 files
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="packages/sdk-swift/Sources/AgentRelaySDK/RelayCast.swift">
<violation number="1" location="packages/sdk-swift/Sources/AgentRelaySDK/RelayCast.swift:124">
P2: Percent-encode the agent name as a path segment before interpolating it into `/api/spawned/{name}`; `/` must be escaped here.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
| func releaseAgent(name: String, reason: String? = nil) async throws { | ||
| try await ensureConnected() | ||
| try await send(.releaseAgent(ReleaseAgentPayload(name: name, reason: reason))) | ||
| let escaped = name.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? name |
There was a problem hiding this comment.
P2: Percent-encode the agent name as a path segment before interpolating it into /api/spawned/{name}; / must be escaped here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk-swift/Sources/AgentRelaySDK/RelayCast.swift, line 124:
<comment>Percent-encode the agent name as a path segment before interpolating it into `/api/spawned/{name}`; `/` must be escaped here.</comment>
<file context>
@@ -120,41 +108,40 @@ actor RelayCore {
func releaseAgent(name: String, reason: String? = nil) async throws {
- try await ensureConnected()
- try await send(.releaseAgent(ReleaseAgentPayload(name: name, reason: reason)))
+ let escaped = name.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? name
+ let body: Data?
+ if let reason {
</file context>
Suggested change
| let escaped = name.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) ?? name | |
| let escaped = name.addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed)?.replacingOccurrences(of: "/", with: "%2F") ?? name |
`RelayCast` conflated this broker client with the unrelated Relaycast cloud service (api.relaycast.dev). The class actually targets a local or remote agent-relay-broker over its /ws and /api/* endpoints, so rename it to `AgentRelayClient` (matching the Node SDK). `RelayCast` stays around as a `@available(*, deprecated, renamed:)` typealias so existing Swift consumers keep compiling.
willwashburn
changed the title
…-njSkj # Conflicts: # CHANGELOG.md
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk-swift/Sources/AgentRelaySDK/RelayTransport.swift (1)
101-118:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove legacy
tokenquery parameter if present inbaseURL.The inline comment states the
tokenquery parameter is gone in v7, butURLComponentspreserves any query parameters from the originalbaseURL. IfbaseURLcontains?token=..., it will be included in the WebSocket request. The v7 broker might reject this or it could be a security concern. Explicitly remove thetokenquery parameter to align with the documented v7 contract.🧹 Proposed fix to strip token parameter
var components = URLComponents(url: baseURL, resolvingAgainstBaseURL: false) if components?.scheme == "http" { components?.scheme = "ws" } if components?.scheme == "https" { components?.scheme = "wss" } + +// Remove legacy token query parameter (v7 uses X-API-Key header only) +components?.queryItems = components?.queryItems?.filter { $0.name != "token" } +if components?.queryItems?.isEmpty == true { + components?.queryItems = nil +} + let existingPath = components?.path ?? ""🤖 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/sdk-swift/Sources/AgentRelaySDK/RelayTransport.swift` around lines 101 - 118, The websocketRequest() currently builds URLComponents from baseURL but leaves any query parameters (e.g., token) intact; update websocketRequest to explicitly remove any "token" query parameter from components (e.g., filter out components?.queryItems where name == "token") before creating the URLRequest so the resulting request url (used to set request = URLRequest(url: components?.url ?? baseURL)) does not include the legacy token query string while still setting the Authorization/X-API-Key headers with authToken.
🤖 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 `@packages/sdk-swift/Sources/AgentRelaySDK/AgentRelayClient.swift`:
- Around line 131-132: The current percent-encoding uses
CharacterSet.urlPathAllowed which leaves "/" unescaped so
"/api/spawned/\(escaped)" can split into multiple segments; update the encoding
used for the local variable escaped (used to build "/api/spawned/\(escaped)") to
percent-encode the name as a single path component by creating an allowed
CharacterSet derived from CharacterSet.urlPathAllowed with the "/" character
removed (so "/" is percent-encoded) and call
addingPercentEncoding(withAllowedCharacters:) with that set; keep the fallback
to the raw name if encoding fails and then use escaped when constructing the
request path.
In `@packages/sdk-swift/Sources/AgentRelaySDK/RelayTransport.swift`:
- Around line 107-112: The path-handling in RelayTransport.swift currently
preserves "/v1/ws" (variables: components, existingPath, components?.path), so
update the conditional logic to actively rewrite any legacy "/v1/ws" suffix to
"/ws"; specifically, if existingPath endsWith("/v1/ws") set components?.path =
existingPath.replacingOccurrences(of: "/v1/ws", with: "/ws"), otherwise apply
the existing rules (set "/ws" when empty or add "/ws" respecting trailing
slash). Ensure you only mutate components?.path and preserve the rest of the
path when performing the "/v1/ws" -> "/ws" replacement.
---
Outside diff comments:
In `@packages/sdk-swift/Sources/AgentRelaySDK/RelayTransport.swift`:
- Around line 101-118: The websocketRequest() currently builds URLComponents
from baseURL but leaves any query parameters (e.g., token) intact; update
websocketRequest to explicitly remove any "token" query parameter from
components (e.g., filter out components?.queryItems where name == "token")
before creating the URLRequest so the resulting request url (used to set request
= URLRequest(url: components?.url ?? baseURL)) does not include the legacy token
query string while still setting the Authorization/X-API-Key headers with
authToken.
🪄 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: bf6d8492-b27e-4e5e-8b75-85e1a5dff5fc
📒 Files selected for processing (6)
CHANGELOG.mdpackages/sdk-swift/README.mdpackages/sdk-swift/Sources/AgentRelaySDK/AgentRelayClient.swiftpackages/sdk-swift/Sources/AgentRelaySDK/RelayHTTP.swiftpackages/sdk-swift/Sources/AgentRelaySDK/RelayTransport.swiftpackages/sdk-swift/Tests/AgentRelaySDKTests/AgentRelaySDKTests.swift
The rename to AgentRelayClient is a hard break; no back-compat shim.
Comments that describe how things used to work (handshake gone, paths removed, envelopes replaced) just rot; the code already shows what it does.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk-swift/Sources/AgentRelaySDK/RelayHTTP.swift (1)
72-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize websocket-style base paths before building REST URLs.
Line 83 currently appends REST paths onto whatever base path is present. If
baseURLis websocket-shaped (e.g.,.../ws), calls resolve to.../ws/api/sendinstead of/api/send. BecauseAgentRelayClientinjects the same base URL into both transport and HTTP clients, this can break spawn/send/release flows.Suggested patch
private func url(for path: String) -> URL? { guard var components = URLComponents(url: baseURL, resolvingAgainstBaseURL: false) else { return nil } if components.scheme == "ws" { components.scheme = "http" } if components.scheme == "wss" { components.scheme = "https" } - let trimmedBase = (components.path).hasSuffix("/") - ? String(components.path.dropLast()) - : components.path + // REST requests should not inherit ws route/query fragments. + components.query = nil + components.fragment = nil + + var trimmedBase = components.path.hasSuffix("/") + ? String(components.path.dropLast()) + : components.path + if trimmedBase.hasSuffix("/ws") { + trimmedBase.removeLast(3) // "/ws" + } let normalizedPath = path.hasPrefix("/") ? path : "/" + path components.path = trimmedBase + normalizedPath return components.url }🤖 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/sdk-swift/Sources/AgentRelaySDK/RelayHTTP.swift` around lines 72 - 84, The url(for:) helper is appending REST paths onto websocket-style base paths (e.g., a baseURL ending in "/ws"), producing incorrect endpoints like ".../ws/api/send"; update url(for:) to detect websocket-style path segments (such as "/ws" or "/wss" or other known websocket endpoints) and strip that terminal websocket segment from components.path before concatenating normalizedPath so REST calls use the service root; modify the logic in url(for:) that computes trimmedBase (and reference baseURL and AgentRelayClient usage) to remove trailing websocket-specific segments prior to building components.path.
🤖 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.
Outside diff comments:
In `@packages/sdk-swift/Sources/AgentRelaySDK/RelayHTTP.swift`:
- Around line 72-84: The url(for:) helper is appending REST paths onto
websocket-style base paths (e.g., a baseURL ending in "/ws"), producing
incorrect endpoints like ".../ws/api/send"; update url(for:) to detect
websocket-style path segments (such as "/ws" or "/wss" or other known websocket
endpoints) and strip that terminal websocket segment from components.path before
concatenating normalizedPath so REST calls use the service root; modify the
logic in url(for:) that computes trimmedBase (and reference baseURL and
AgentRelayClient usage) to remove trailing websocket-specific segments prior to
building components.path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6970332b-0f19-4eff-950c-6ff105b5620a
📒 Files selected for processing (4)
packages/sdk-swift/Sources/AgentRelaySDK/AgentRelayClient.swiftpackages/sdk-swift/Sources/AgentRelaySDK/RelayHTTP.swiftpackages/sdk-swift/Sources/AgentRelaySDK/RelayTransport.swiftpackages/sdk-swift/Tests/AgentRelaySDKTests/AgentRelaySDKTests.swift
💤 Files with no reviewable changes (2)
- packages/sdk-swift/Sources/AgentRelaySDK/RelayTransport.swift
- packages/sdk-swift/Sources/AgentRelaySDK/AgentRelayClient.swift
Extends the existing web-only skip pattern in test.yml, rust-ci.yml, package-validation.yml, and node-compat.yml with a parallel sdk_swift_only flag. Also negates packages/sdk-swift/** in e2e-tests.yml and relay-cleanroom-hardening.yml's packages/** path filter. Swift-only PRs currently have no Swift CI to run; tracked separately for a path-area refactor that adds it.
Addresses PR review feedback: - RelayHTTP.url(for:) now strips a trailing /ws or /v1/ws from the base path so callers that pass a ws-shaped baseURL still hit /api/* instead of /ws/api/*. - RelayTransport.websocketRequest() normalizes trailing slashes before checking the /ws suffix (so /ws/ no longer becomes /ws/ws), rewrites legacy /v1/ws to /ws, and strips a legacy ?token= query. - releaseAgent uses a path-segment-safe CharacterSet that excludes /, so agent names containing slashes don't split the request path. - registerOrRotate calls ensureConnected() again, restoring the pre-PR behavior of opening the WS as part of registration. Extracted the URL normalization into static helpers and added unit tests covering each edge case.
There was a problem hiding this comment.
1 issue found across 10 files (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="packages/sdk-swift/Sources/AgentRelaySDK/RelayCast.swift">
<violation number="1" location="packages/sdk-swift/Sources/AgentRelaySDK/RelayCast.swift:124">
P2: Percent-encode the agent name as a path segment before interpolating it into `/api/spawned/{name}`; `/` must be escaped here.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Builds and runs swift test in packages/sdk-swift whenever any file in that directory changes. macOS runners are billed at 10x but the job is small and only fires for sdk-swift PRs.
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
2 participants
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.
Fixes #988.
Summary
The Swift SDK's broker client was still implementing the v6 protocol — connecting to
/v1/wsand exchanging ahello/hello_ackhandshake before sending actions over the WebSocket. The v7 broker's/wsendpoint is a read-only event broadcast (handle_dashboard_ws), so every Swift connection timed out waiting forhello_ack.This PR realigns the Swift broker client with the v7 contract used by the Node SDK, and renames it to stop conflating it with the Relaycast cloud service.
v7 protocol fix
RelayTransport.swift— default WS path is now/ws(not/v1/ws); the unused?token=query parameter is gone. Auth stays onX-API-Key/Authorizationheaders.RelayHTTP.swift(new) — minimal actor-based HTTP client withX-API-Keyauth and broker error decoding.AgentRelayClient.swift— drops thehello/hello_ackhandshake (WS upgrade is the "connected" signal) and routes actions through the broker REST API:spawnAgent→POST /api/spawnreleaseAgent→DELETE /api/spawned/{name}post, agentdm/post→POST /api/sendBrokerEvent({kind: "...", ...}) instead of the legacy{type, payload}envelope, and still flow intobrokerEvents,inboundMessages, andChannel.events.Breaking:
RelayCast→AgentRelayClientThe class was named after the Relaycast cloud service (
api.relaycast.dev, talked to by@relaycast/sdkin the Node ecosystem), but it has always targeted a local or remoteagent-relay-brokerover its/wsand/api/*endpoints. RenamingRelayCast→AgentRelayClientmatches the Node SDK'sAgentRelayClientand removes the confusion.No back-compat typealias — Swift consumers must update to the new name. Migration:
Test plan
cd packages/sdk-swift && swift buildcd packages/sdk-swift && swift testagent-relay up --no-dashboard --foreground --state-dir ./.agent-relayAgentRelayClient(...).channel("test").subscribe()returns without timing outchannel.post("hello")succeeds and is observed onchannel.eventsspawnAgent/releaseAgentround-trip through the broker