Skip to content

Fix: governance-event WebSocket broadcasts never reached anyone (two independent bugs) - #5

Merged
vdmo merged 1 commit into
mainfrom
fix/ws-broadcast-never-reached-anyone
Sep 30, 2026
Merged

vdmo merged 1 commit into
mainfrom
fix/ws-broadcast-never-reached-anyone

Conversation

@vdmo

@vdmo vdmo commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Summary

Found this while preparing to build a TiXL plugin/bridge on top of the governance broadcast path — before writing a single line of bridge code, following the same discipline that caught the Resolume bridge's wire-format bugs (#3).

Two independent, silent bugs meant broadcast_websocket — the mechanism process_message uses to push every governance event out in real time — has never actually delivered anything to anyone since it shipped:

  1. The WebSocket server never accepted connections. TransportLayer's WS listener (port 8889) binds via start_transport, but nothing ever called accept_websocket() in a loop. ws_clients was permanently empty, so every broadcast iterated zero clients and silently did nothing. Same bug shape as the earlier UDP receive-loop fix (Real Only-Lang rule engine + signed governance receipts (was: hardcoded rules, no signing, didn't build standalone) #1): a bound socket is not a served one.
  2. Independent of (1): a DENY discarded its own broadcast. RuleEngine::evaluate built the events Vec correctly, but on a DENY returned Err(...) directly, discarding the already-built events before process_message ever got to broadcast them. The DENY was still correctly signed and persisted (GET /governance/export always had it), but the single most important event type was never sent live.

Fix

  • PCHIConductor::run_ws_accept_loop, spawned from main.rs alongside the UDP loop, actually accepts and registers WS clients.
  • RuleEngine::evaluate now returns (events, Option<deny_reason>) instead of using Err for a deny. process_message broadcasts every event first, then converts deny_reason into the Err it returns to its own caller — the "a denial is a real error" contract is unchanged from the caller's point of view.

Test plan

  • New test: connects a real tokio-tungstenite WebSocket client to a running conductor, triggers a real DENY, asserts the client receives it. Confirmed it fails against the pre-fix code (reproduces bug 1) and passes now.
  • Updated two existing rules.rs tests for evaluate()'s new return shape.
  • cargo test --workspace: 30/30 pass. cargo clippy --workspace --all-targets -- -D warnings: clean.
  • Live, outside the test suite: ran the real binary + a real Python websocket-client + a real triggering UDP packet. Client received {'type': 'governance_event', 'gate_state': 'DENY', ...}. Before this fix, it would have received nothing, ever.

🤖 Generated with Claude Code

…DENY events were dropped from broadcast even when they did

Found while preparing to build a TiXL bridge on top of the governance
broadcast path, before writing a single line of bridge code — exactly
the discipline that caught the Resolume bridge's bugs.

Two independent bugs, both silent:

1. TransportLayer's WebSocket server (port 8889) binds via
   start_transport, but nothing ever called accept_websocket() in a
   loop. ws_clients was permanently empty, so broadcast_websocket
   (called from process_message for every fired rule since the
   real-governance-signed-receipts change) iterated zero clients and
   did nothing — every governance-event broadcast has gone nowhere
   since that feature shipped. Same bug shape as the earlier UDP
   receive-loop fix: a bound socket is not a served one.
   PCHIConductor::run_ws_accept_loop closes this, spawned from main.rs
   alongside the UDP loop.

2. Independent of (1): RuleEngine::evaluate built the events Vec
   correctly but, on a DENY, returned Err(ConductorError::RuleError(..))
   directly — discarding the already-built events before process_message
   ever got to broadcast them. The DENY itself was still correctly
   signed and persisted (that path doesn't touch events), so
   GET /governance/export always had it — but nothing was ever
   broadcast live for the single most important event type. evaluate()
   now returns (events, Option<deny_reason>) instead of using Err for
   this; process_message broadcasts every event first, then converts
   deny_reason into the Err it returns to its own caller, preserving
   the existing "a denial is a real error to the caller" contract.

Added a real regression test: connects an actual WebSocket client to a
running conductor, triggers a real DENY, and asserts the client
receives it — not a mock, an actual client on an actual socket. It
failed against the pre-fix code (correctly reproducing bug 1); passes
now. Updated two existing rules.rs tests for evaluate()'s new return
shape.

Verified live, not just in the test suite: ran the real binary, a real
Python websocket-client, and a real triggering UDP packet —
`RECEIVED: governance_event {'gate_state': 'DENY', ...}` — before this
fix, that client would have received nothing, forever.

30/30 tests pass; clippy clean.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
@vdmo
vdmo merged commit 0d640cc into main Sep 30, 2026
1 check passed
vdmo added a commit that referenced this pull request Sep 30, 2026
Path A of the PCHI/TiXL integration: an OSC bridge connecting TiXL
(https://github.com/tixl3d/tixl, 5.1k-star open-source real-time motion
graphics) to the PCHI Conductor, following the same shape as the
existing Resolume bridge — but this time verified against TiXL's real
operator source (Operators/Io/Symbols/lib/io/osc/{OscOutput,OscInput}.cs)
before writing a line of bridge code, rather than guessing at wire
compatibility. TiXL's OSC operators are built on Rug.Osc, a real
spec-compliant OSC library, so python-osc interoperates correctly with
no translation guesswork — unlike the first version of the Resolume
bridge, which shipped with three independent wire-format bugs (#3)
because none of that was verified against real source.

Two legs: TiXL's OscOutput -> raw JSON-over-UDP -> the Conductor
(control updates, same wire-format lesson as the Resolume fix applied
from day one: correct type/payloadType field names, no OSC-wrapping,
complete required pir_invariants on every message including heartbeats);
the Conductor's governance-event WebSocket -> OSC -> TiXL's OscInput
(live verdicts as a numeric gate_state code, since OscInput's outputs
are float-only — confirmed from source, no string channel).

Governance telemetry depends on #5 (the WS accept-loop fix) — without
it the conductor's WS never accepts a connection and this leg silently
receives nothing. Control updates work standalone regardless; verified
against a build of plain `main` (without #5) to confirm exactly that.

Verified live: a real conductor, this bridge, and real OSC traffic
constructed to exactly match TiXL's actual OscOutput/OscInput wire
shape (confirmed from source) — both legs work correctly, including
governance telemetry (tested against a build with #5's fix). Not
verified against real TiXL itself, which is a Windows/DirectX GUI
application — README states this plainly.

Co-authored-by: vdmo <[email protected]>
Co-authored-by: Claude Sonnet 5 <[email protected]>
vdmo added a commit that referenced this pull request Sep 30, 2026
…rect, no OSC (#8)

Path B of the PCHI/TiXL integration (Path A: tools/tixl-pchi-bridge,
#6). TiXL has no separate plugin DLL system — per its own docs, "TiXL
*is* the SDK": operators are C# source the running editor compiles and
hot-reloads. These two files are real operators that speak PCHI's wire
protocol directly (raw JSON-over-UDP out, WebSocket in), eliminating
both the OSC round-trip and the external bridge process Path A needs,
for TiXL specifically.

- PCHISendControlParameter: TargetId/Parameter/Value/SendTrigger inputs,
  sends a real control_parameter PCHIMessage straight to the Conductor's
  UDP port. BuildWireJson() is the one place Rust's #[serde(rename)]
  wire names (type, payloadType) get applied — the exact field-name
  lesson from the Resolume/TiXL OSC bridges (#3), applied natively
  instead of relearned.
- PCHIGovernanceReceive: owns its own ClientWebSocket (reconnecting,
  background Task), exposes GateState (float code, same ALLOW/ESCALATE/
  DENY=1.0/0.5/0.0 encoding the OSC bridge uses so both paths are
  interchangeable to an artist), ReceiptId, Tool, IsConnected as live
  graph values. Requires #5 (the WS accept-loop fix) on the Conductor
  side.

## Verification — stronger than anything else in this integration, and why

TiXL's Editor needs Windows (DirectX 11, WinForms) and can't run here,
but Core.csproj — the library operators actually compile against —
builds on Linux with -p:EnableWindowsTargeting=true (a standard
cross-compile flag, not a workaround). Cloned tixl3d/tixl, installed
.NET 10 SDK (TixlNetFrameworkVersion = net10.0-windows) locally, and
compiled these two files against the real Core.dll: 0 warnings, 0
errors, on the first attempt and confirmed again on a clean rebuild
using the exact command documented in this commit's README. That's the
real compiler checking real API usage — Instance<T>, Slot<T>,
InputSlot<T>, EvaluationContext, IStatusProvider — not written blind.

Not verified: running inside a live TiXL editor. Turning this .cs file
into an actual graph node needs TiXL's GUI (duplicate an existing
operator to scaffold the accompanying .t3/.t3ui files TiXL's editor
owns, then replace the generated body with this code) — documented
plainly in the README as the one step this environment cannot perform
or verify.

verify-build/Pchi.csproj is included so the compile check is
reproducible by anyone, not just asserted.

Co-authored-by: vdmo <[email protected]>
Co-authored-by: Claude Sonnet 5 <[email protected]>
vdmo added a commit that referenced this pull request Sep 30, 2026
Third PCHI integration this session, alongside Resolume (#3) and TiXL
(#6, #8). cables.gl (https://github.com/cables-gl/cables, 562-star
open-source browser-based node/patch tool) runs in a browser sandbox,
which can't open the raw UDP socket the Conductor's control-update path
requires — so unlike TiXL's native operators, this needs a relay: HTTP
in (cables can always fetch()) -> raw JSON-over-UDP out to the
Conductor, and the Conductor's real governance-event WebSocket ->
relayed straight through to a WebSocket server cables can connect to
directly.

The governance-telemetry leg needed zero new protocol work: confirmed
from a real official example (cables-gl/websocket-example) that cables
already ships a working WebSocket-receive op against a plain `ws`
server — this relay's cables-facing WS server is deliberately that same
shape, not something invented for this integration.

Requires #5 (WS accept-loop fix) on the Conductor for governance
telemetry; control updates work regardless.

## A real mistake found and fixed along the way

Building this surfaced that the Heartbeat-exclusion fix (originally
meant to land in #5) had never actually made it into main — I'd pushed
it as a second commit onto that PR's branch and reported it as included,
but the merged PR only ever contained the first commit. Live-testing
this relay reproduced the exact duplicate-DENY bug the fix was supposed
to have eliminated. Re-landed as #9, cherry-picked cleanly, re-verified
live against this relay specifically (22s, 4+ heartbeat cycles, 1
receipt instead of 5) before writing this commit.

## Verification

Live, both directions, against a real Conductor:
- Outbound: a real POST (with an Origin header, and an explicit OPTIONS
  preflight check with Access-Control-Request-* headers — not just a
  bare curl a browser's CORS enforcement wouldn't accept) reaches the
  Conductor and is applied.
- Inbound: a real DENY is broadcast by the Conductor, relayed, and
  arrives at a real external WS client with the documented enriched
  shape (flattened gate_state/receipt_id/tool plus the full raw event).

Not tested against real cables.gl itself — the WebSocket-receive shape
is confirmed from a real official example; the HTTP-request op and
message parsing on the cables side are not independently confirmed the
way TiXL's operators were confirmed against real compiled source.
README states this plainly.

Co-authored-by: vdmo <[email protected]>
Co-authored-by: Claude Sonnet 5 <[email protected]>
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