Prefer channel_name for pubsub broadcasts by willwashburn · Pull Request #11 · AgentWorkforce/relaycast

willwashburn · 2026-02-10T00:39:38Z

Use the human-readable channel name (data.channel_name) when broadcasting pub/sub channel events instead of the snowflake channel_id, falling back to channel_id if name is absent. Add a unit test to verify channel_name is preferred. Wire up workspace subscription and client message handling on WS connect (subscribeToWorkspace + handleClientMessage). Add end-to-end production WebSocket test scripts (test-prod-ws.mjs and test-prod-ws.sh) and allow local bash commands in .claude/settings.local.json for running those checks.

Copilot

Pull request overview

Updates WebSocket pub/sub fanout to prefer broadcasting on a human-readable channel_name (when available) instead of channel_id, and adds supporting connection wiring plus production E2E test scripts.

Changes:

Prefer event.data.channel_name (fallback to channel_id) when broadcasting Redis pub/sub events to channel subscribers.
On WS connect, subscribe the server to the workspace’s Redis channel and handle client subscribe/unsubscribe messages.
Add production WebSocket E2E test scripts (bash + Node) and extend local Claude settings to allow running them.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`packages/server/src/ws/pubsub.ts`	Changes broadcast routing to use `channel_name` when available.
`packages/server/src/ws/__tests__/pubsub.test.ts`	Adds unit test asserting `channel_name` is preferred over `channel_id`.
`packages/server/src/ws/server.ts`	Wires workspace subscription and client message handling on WS connect.
`test-prod-ws.sh`	Adds bash-based production WS E2E test script.
`test-prod-ws.mjs`	Adds Node-based production WS E2E test script.
`.claude/settings.local.json`	Expands allowed local bash commands for running checks/scripts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-10T00:46:59Z

+    const channelName = typeof event.data?.channel_name === 'string'
+      ? event.data.channel_name
+      : event.channel_id;


channelName will use event.data.channel_name for any string value, including '' or whitespace. In that case if (channelName) will be falsy and the event will be routed to broadcastToWorkspace rather than falling back to event.channel_id. Consider trimming and only using channel_name when it’s a non-empty string; otherwise fall back to event.channel_id.

Suggested change

const channelName = typeof event.data?.channel_name === 'string'

? event.data.channel_name

: event.channel_id;

const rawChannelName = typeof event.data?.channel_name === 'string'

? event.data.channel_name.trim()

: undefined;

const channelName = rawChannelName || event.channel_id;

Copilot · 2026-02-10T00:46:59Z

      "WebFetch(domain:developers.cloudflare.com)",
-      "WebFetch(domain:raw.githubusercontent.com)"
+      "WebFetch(domain:raw.githubusercontent.com)",
+      "Bash(chmod:*)",


Bash(chmod:*) is a very broad permission (effectively any chmod on any path), which increases the blast radius of accidental/malicious commands. If the intent is only to make the new script executable, consider narrowing this to the specific command/path needed (e.g., chmod +x test-prod-ws.sh) rather than a wildcard.

Suggested change

"Bash(chmod:*)",

"Bash(chmod +x test-prod-ws.sh)",

Copilot · 2026-02-10T00:46:59Z

+    // Clients subscribe by channel name, so broadcast using channel_name
+    // (included in event data) rather than the snowflake channel_id.
+    const channelName = typeof event.data?.channel_name === 'string'
+      ? event.data.channel_name
+      : event.channel_id;
+    if (channelName) {
+      broadcastToChannel(event.workspace_id, channelName, event);


Given the comment that clients subscribe by channel name, falling back to broadcasting on event.channel_id when data.channel_name is absent means some channel-scoped event types won’t reach subscribers. Several publishers currently emit events with channel_id but without data.channel_name (e.g. reaction.added, thread.reply, webhook.received). To avoid silent drops, either ensure all channel-scoped events always include data.channel_name, or broadcast to both data.channel_name and channel_id when both are present (backward compatibility).

Suggested change

// Clients subscribe by channel name, so broadcast using channel_name

// (included in event data) rather than the snowflake channel_id.

const channelName = typeof event.data?.channel_name === 'string'

? event.data.channel_name

: event.channel_id;

if (channelName) {

broadcastToChannel(event.workspace_id, channelName, event);

// Clients subscribe by channel name, so prefer broadcasting using channel_name

// (included in event data) but also broadcast to channel_id when present for

// backward compatibility with any ID-based subscribers.

const channelName =

typeof event.data?.channel_name === 'string' ? (event.data.channel_name as string) : undefined;

const channelId =

typeof event.channel_id === 'string' ? (event.channel_id as string) : undefined;

if (channelName || channelId) {

if (channelName) {

broadcastToChannel(event.workspace_id, channelName, event);

}

if (channelId && channelId !== channelName) {

broadcastToChannel(event.workspace_id, channelId, event);

}

Copilot · 2026-02-10T00:47:00Z

+JOIN_RESP=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/members" \
+  -H "Authorization: Bearer $LISTENER_TOKEN" \
+  -H "Content-Type: application/json")
+echo "   Response: $JOIN_RESP"
+echo ""
+
+# Also join with sender
+JOIN_RESP2=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/members" \


Same issue as above: this uses POST /v1/channels/$CHANNEL/members for the sender, but joining is done via POST /v1/channels/:name/join with an agent token. Update this request to use the /join route so the sender is actually a channel member.

Suggested change

JOIN_RESP=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/members" \

-H "Authorization: Bearer $LISTENER_TOKEN" \

-H "Content-Type: application/json")

echo " Response: $JOIN_RESP"

echo ""

# Also join with sender

JOIN_RESP2=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/members" \

JOIN_RESP=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/join" \

-H "Authorization: Bearer $LISTENER_TOKEN" \

-H "Content-Type: application/json")

echo " Response: $JOIN_RESP"

echo ""

# Also join with sender

JOIN_RESP2=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/join" \

Copilot · 2026-02-10T00:47:00Z

+JOIN_RESP=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/members" \
+  -H "Authorization: Bearer $LISTENER_TOKEN" \
+  -H "Content-Type: application/json")
+echo "   Response: $JOIN_RESP"
+echo ""
+
+# Also join with sender
+JOIN_RESP2=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/members" \


This call uses POST /v1/channels/$CHANNEL/members to join the channel, but the server’s join endpoint is POST /v1/channels/:name/join (agent token required). As written, the listener may never become a member, which can prevent message delivery in this E2E test.

Suggested change

JOIN_RESP=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/members" \

-H "Authorization: Bearer $LISTENER_TOKEN" \

-H "Content-Type: application/json")

echo " Response: $JOIN_RESP"

echo ""

# Also join with sender

JOIN_RESP2=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/members" \

JOIN_RESP=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/join" \

-H "Authorization: Bearer $LISTENER_TOKEN" \

-H "Content-Type: application/json")

echo " Response: $JOIN_RESP"

echo ""

# Also join with sender

JOIN_RESP2=$(curl -s -X POST "$BASE/v1/channels/$CHANNEL/join" \

Copilot · 2026-02-10T00:47:00Z

+  echo "Install: npm install -g wscat"
+  exit 1
+fi
+


This script relies on jq to parse API responses (e.g., extracting tokens), but it only checks for wscat/websocat. With set -euo pipefail, missing jq will cause a hard-to-understand failure later. Add an explicit command -v jq dependency check alongside the existing WebSocket tool check.

Suggested change

# Check for jq (used to parse API responses)

if ! command -v jq &> /dev/null; then

echo "Need jq to parse API responses."

echo "Install: see https://stedolan.github.io/jq/download/"

exit 1

fi

Copilot · 2026-02-10T00:47:01Z

+        subscribeToWorkspace(auth.workspaceId).catch(() => {});
+
+        ws.on('message', (raw) => {
+          handleClientMessage(client, raw.toString());
+        });


subscribeToWorkspace(auth.workspaceId).catch(() => {}) silently swallows subscription errors. If Redis subscription fails, clients will connect successfully but never receive events, and it will be very hard to diagnose. Consider at least logging the error (with workspaceId context) and/or notifying the client before proceeding.

Copilot · 2026-02-10T00:47:01Z

        clients.set(client.id, client);
        indexAddClient(client);
+        subscribeToWorkspace(auth.workspaceId).catch(() => {});
+
+        ws.on('message', (raw) => {


subscribeToWorkspace(auth.workspaceId) is added on connect, but there’s no corresponding unsubscribeFromWorkspace when the last client for a workspace disconnects. Over time this can grow subscribedWorkspaces without bound and keep Redis subscriptions active for inactive workspaces. Consider tracking a per-workspace connection count (or checking workspaceIndex after disconnect) and unsubscribing when it reaches zero.

Add channel_name throughout the stack: include channel.name in reaction and thread payloads, include channel_name in inbound webhook events, and pass channel_name into reaction event publishing (while stripping internal ids from client responses). Improve pubsub handling to trim and fall back to channel_id when channel_name is empty, add a test for whitespace channel_name, and make WS server subscribe/unsubscribe more robust (log subscribe errors and unsubscribe when last client leaves). Also update .claude local settings and remove test-prod-ws.sh.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-10T10:25:53Z

+    const timeout = setTimeout(() => {
+      sock.close();
+      reject(new Error("Timed out waiting for message.created event (15s)"));
+    }, 15000);
+
+    const events = [];
+    const sock = new WebSocket(wsUrl);


The timeout callback calls sock.close(), but sock is declared/initialized later. If the WebSocket constructor throws synchronously (bad URL, missing TLS support, etc.), the timeout will later fire and throw when referencing sock. Declare sock before creating the timeout (e.g., let sock: WebSocket | undefined) and guard the close call.

Suggested change

const timeout = setTimeout(() => {

sock.close();

reject(new Error("Timed out waiting for message.created event (15s)"));

}, 15000);

const events = [];

const sock = new WebSocket(wsUrl);

let sock;

const timeout = setTimeout(() => {

if (sock) {

sock.close();

}

reject(new Error("Timed out waiting for message.created event (15s)"));

}, 15000);

const events = [];

sock = new WebSocket(wsUrl);

Copilot · 2026-02-10T10:25:54Z

+      "Bash(npx turbo test:*)",
+      "Bash(gh api:*)"


This adds very broad Bash tool allow-list entries (e.g. Bash(gh api:*)), which can enable arbitrary GitHub API operations from the tool runner. Consider narrowing these patterns to the minimal required commands/endpoints (or keeping this configuration out of version control) to reduce the risk of unintended destructive operations.

Suggested change

"Bash(npx turbo test:*)",

"Bash(gh api:*)"

"Bash(npx turbo test:*)"

Copilot · 2026-02-10T10:25:54Z

+        ws.on('message', (raw) => {
+          handleClientMessage(client, raw.toString());
+        });


ws.on('message', ...) is registered twice in this upgrade handler (there is another identical listener later in the same callback). This will cause each client message to be processed twice (e.g., duplicate subscriptions and duplicate acknowledgements). Remove the duplicate listener so handleClientMessage runs exactly once per incoming message.

Copilot · 2026-02-10T10:25:54Z

+        const removeClient = () => {
+          const wid = client.workspaceId;
          indexRemoveClient(client);
          clients.delete(client.id);
-        });
+          // Unsubscribe from Redis when no more clients in this workspace
+          if (!workspaceIndex.has(wid)) {
+            unsubscribeFromWorkspace(wid).catch(() => {});
+          }


Cleanup/unsubscribe logic is now centralized in removeClient(), but the ping-timeout branch later in this file still removes clients directly (via indexRemoveClient/clients.delete) and won't trigger unsubscribeFromWorkspace when the last client in a workspace is reaped. Consider reusing removeClient() (or extracting shared cleanup) for all disconnect paths so Redis workspace subscriptions don't leak after idle clients are terminated.

PR #11 added message handler and subscribeToWorkspace at the top of handleUpgrade but the originals at the bottom were not removed, causing handleClientMessage to fire twice per message (breaking unsubscribe). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

willwashburn requested a review from Copilot February 10, 2026 00:39

Copilot started reviewing on behalf of willwashburn February 10, 2026 00:40 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

willwashburn added 3 commits February 9, 2026 20:00

Merge branch 'main' into enable-workspace-registration-in-mcp

611a9ed

Merge branch 'main' into enable-workspace-registration-in-mcp

6074641

willwashburn requested a review from Copilot February 10, 2026 10:21

Copilot started reviewing on behalf of willwashburn February 10, 2026 10:21 View session

willwashburn merged commit dc6a806 into main Feb 10, 2026
7 checks passed

willwashburn deleted the enable-workspace-registration-in-mcp branch February 10, 2026 10:25

Copilot AI reviewed Feb 10, 2026

View reviewed changes

-    const channelName = typeof event.data?.channel_name === 'string'
-      ? event.data.channel_name
-      : event.channel_id;
+    const rawChannelName = typeof event.data?.channel_name === 'string'
+      ? event.data.channel_name.trim()
+      : undefined;
+    const channelName = rawChannelName || event.channel_id;

+# Check for jq (used to parse API responses)
+if ! command -v jq &> /dev/null; then
+  echo "Need jq to parse API responses."
+  echo "Install: see https://stedolan.github.io/jq/download/"
+  exit 1
+fi

	"Bash(npx turbo test:*)",
	"Bash(gh api:*)"
	"Bash(npx turbo test:*)"

Conversation

willwashburn commented Feb 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants