Skip to content

Per-connection protocol message control#892

Closed
ask-bonk[bot] wants to merge 1 commit into
mainfrom
opencode/issue881-20260211145936
Closed

Per-connection protocol message control#892
ask-bonk[bot] wants to merge 1 commit into
mainfrom
opencode/issue881-20260211145936

Conversation

@ask-bonk

@ask-bonk ask-bonk Bot commented Feb 11, 2026

Copy link
Copy Markdown
Contributor

Everything is done. Here's a summary of what was implemented and the tradeoffs/edge cases analysis:

Implementation Summary

What was added

  1. shouldSendProtocolMessages(connection, ctx) — overridable hook on Agent (default: true). Called once on connect to determine if protocol text frames should be sent.

  2. isConnectionProtocolEnabled(connection) — public predicate to check protocol status at runtime.

  3. Internal plumbing:

    • _cf_no_protocol flag stored in connection state (survives hibernation)
    • _broadcastProtocol() helper that filters out no-protocol connections before calling broadcast()
    • _setConnectionNoProtocol() private setter
    • Generalized the connection state wrapping to hide all internal keys (_cf_readonly and _cf_no_protocol) rather than just the readonly key

Files changed

  • packages/agents/src/index.ts — core implementation (+174/-43 lines)
  • packages/agents/src/tests/agents/protocol-messages.ts — test agent
  • packages/agents/src/tests/protocol-messages.test.ts — 11 tests across 7 describe blocks
  • packages/agents/src/tests/agents/index.ts, worker.ts, wrangler.jsonc — test infrastructure

Test coverage (11 tests)

  • Protocol messages sent/suppressed on connect
  • isConnectionProtocolEnabled returns correct values
  • RPC works on no-protocol connections (both read and mutating)
  • State broadcasts filter out no-protocol connections
  • Mixed protocol/no-protocol connections coexist
  • Reconnection re-evaluates the hook
  • Internal flag hidden from connection.state

Tradeoffs and Edge Cases

1. Broadcast performance with many no-protocol connections

The _broadcastProtocol() method iterates over all connections via getConnections() to build the exclusion list, then passes it to broadcast(). If you have hundreds of no-protocol connections, this builds a large array on every state change. The alternative would be to override broadcast() itself, but that would break the partyserver contract. For the expected use case (a few binary clients alongside web clients), this is fine.

2. Not dynamically settable (by design)

Per the request, this is evaluated once on connect and stored as a flag. There is no setConnectionProtocolEnabled() method. If you need to change a connection's protocol status, you'd need to disconnect and reconnect. This simplifies the mental model and avoids edge cases around mid-session protocol toggling.

3. RPC still sends JSON text frames

No-protocol connections still receive RPC responses as JSON text frames. This is intentional — the feature suppresses only the automatic protocol messages (identity, state sync, MCP servers), not user-initiated communication. If a binary-only client needs pure binary communication, they should avoid calling RPC methods that return JSON. This could be a point of confusion.

4. Client-initiated state updates still work

A no-protocol connection can still send CF_AGENT_STATE messages to update state, and those updates will be processed and broadcast to other (protocol-enabled) connections. The no-protocol flag only controls outbound protocol messages.

5. Interaction with sendIdentityOnConnect: false

When sendIdentityOnConnect is false at the agent level AND shouldSendProtocolMessages returns false for a connection, the behavior is consistent — no identity is sent either way. The per-connection hook is a strict superset of the static option for the affected connections.

6. Workflow broadcasts are NOT filtered

The _workflow_broadcast() method (used by AgentWorkflow) calls this.broadcast() directly and does NOT go through _broadcastProtocol(). This is intentional — workflow messages are application-level, not protocol-level. If this should also be filtered, it would be a separate decision.

7. observability events still fire

The "Connection established" observability event still fires for no-protocol connections. This is correct — observability should track all connections regardless of protocol status.

8. Connection state wrapping generalization

The wrapping in _ensureConnectionWrapped was refactored from single-key (_cf_readonly) to multi-key (CF_INTERNAL_KEYS array). This is more maintainable but slightly changes the internal mechanics — instead of destructuring a single key, it iterates over the key list. The behavior is identical for existing readonly functionality (verified by all 17 readonly tests passing).

Closes #881

github run

…bled predicate

Add per-connection control over protocol text frames (CF_AGENT_IDENTITY,
CF_AGENT_STATE, CF_AGENT_MCP_SERVERS). Override shouldSendProtocolMessages()
to return false for connections that should not receive these frames.

This enables binary-only clients (e.g. MQTT devices) to connect without
receiving JSON text frames, while still participating in RPC and regular
messaging.

Changes:
- Add shouldSendProtocolMessages(connection, ctx) hook (default: true)
- Add isConnectionProtocolEnabled(connection) predicate helper
- Update onConnect to skip protocol messages when hook returns false
- Update state and MCP broadcasts to exclude no-protocol connections
- Generalize connection state wrapping to handle multiple internal flags
- Add comprehensive test suite (12 tests)

Closes #881
@changeset-bot

changeset-bot Bot commented Feb 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 097da58

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
agents Major
@cloudflare/ai-chat Major
@cloudflare/codemode Major
hono-agents Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Feb 11, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/agents@892

commit: 097da58

@threepointone
threepointone self-requested a review February 11, 2026 15:46
@threepointone

Copy link
Copy Markdown
Contributor

/bonk looks like you missed one, cf_agent_mcp_servers, could you fix your pr to exclude that message as well

@ask-bonk

ask-bonk Bot commented Feb 11, 2026

Copy link
Copy Markdown
Contributor Author

The code is already correct. Looking at the implementation:

  1. On connect (line 963-968 in index.ts): The cf_agent_mcp_servers send is inside the if (sendProtocol) block (line 941), so it's already suppressed for no-protocol connections.

  2. Broadcast (broadcastMcpServers() at line 3688-3695): Already uses _broadcastProtocol() which excludes no-protocol connections.

  3. Tests (lines 161 and 180): The test at line 161 verifies CF_AGENT_MCP_SERVERS IS sent to protocol-enabled connections, and line 180 verifies it is NOT sent to no-protocol connections.

The cf_agent_mcp_servers message is already properly excluded for no-protocol connections in this PR. Both the on-connect path and the broadcast path are guarded. The tests at lines 144-181 explicitly verify this for all three protocol message types (identity, state, and mcp_servers).

Is there a specific scenario where you're seeing cf_agent_mcp_servers leak through to a no-protocol connection, or were you perhaps looking at an earlier version of the diff?

github run

@threepointone
threepointone deleted the opencode/issue881-20260211145936 branch February 15, 2026 16:00
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.

feature request: option to turn off agent protocol messages on the websocket

1 participant