Repository navigation
Fix: heartbeat-exclusion fix never actually made it into main (process mistake, re-landing it) - #9
Merged
Merged
Conversation
…ng unchanged violations forever Found live while testing the TiXL bridge (built on top of this branch's WS-broadcast fix): process_message unconditionally re-evaluated all rules after every message, including Heartbeat, which carries no scene- state change. A real, still-unaddressed violation (tentacle_system. total_curl left over threshold) got re-signed into a brand-new receipt on every subsequent heartbeat — every 5s by default in the bundled bridges — for as long as the condition sat unaddressed. Reproduced: one real DENY produced 5 identical signed receipts (same receipt_id, since nothing about the content changed) over 20 idle seconds. Not fixed by de-duplicating receipts after the fact — fixed at the source: Heartbeat now returns early from process_message before equilibrium-status update or rule evaluation runs, since a message that changes nothing has nothing new to evaluate. Added a regression test: a real violation produces exactly one receipt, then 5 heartbeats against the same unchanged state must not add any more. 31/31 tests pass; clippy clean. Verified live, not just in the test suite: re-ran the exact scenario that surfaced this (real conductor, real TiXL-bridge heartbeat traffic, a real lingering DENY) for 22 seconds spanning 4 heartbeat cycles — 1 receipt, not 5. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 of 4 tasks
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 <vdmo@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
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.
What happened
The heartbeat-exclusion fix (excluding `Heartbeat` from rule evaluation, requested and confirmed working in the conversation that produced #5) was committed and pushed to the `fix/ws-broadcast-never-reached-anyone` branch as a second commit, after #5 was already open. I reported at the time that #5 "now includes both fixes." That was wrong, and I didn't catch it until testing the cables.gl relay just now and seeing the exact same duplicate-DENY behavior the fix was supposed to have eliminated.
Checked via the GitHub API: PR #5, as merged, contains exactly one commit (`d86621e`) — the original WS-broadcast fix. The second commit (`8fa6445`, the heartbeat fix) was pushed to the branch but never became part of the merged PR. I don't have a confirmed root cause for why the squash-merge didn't pick it up (possibly a stale view at merge time — I checked `mergeable`/`mergeStateStatus` before merging but never explicitly re-diffed the PR's commit list against what I'd just pushed). What matters is: the fix has not been live since I said it was, until this PR.
The fix
Cherry-picked `8fa6445` verbatim onto current `main` (clean cherry-pick, no conflicts). Same commit, same content, same tests — see that commit's message for the original description.
Re-verification
🤖 Generated with Claude Code