Skip to content

feat(export): E1 — WsJsonClient extraction + bridge protocol client - #121

Merged
simons-plugins merged 2 commits into
mainfrom
feat/e1-bridge-client
Aug 4, 2026
Merged

feat(export): E1 — WsJsonClient extraction + bridge protocol client#121
simons-plugins merged 2 commits into
mainfrom
feat/e1-bridge-client

Conversation

@simons-plugins

@simons-plugins simons-plugins commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Summary

Milestone E1 (PRD §9): the plugin-side half of the local protocol, per docs/BRIDGE_PROTOCOL.md.

  • ws_json_client.py (new, 359): transport core extracted from matter_client.py — run loop, exponential backoff, message_id↔future correlation, disconnect semantics, repeated-failure diagnostic latch. Handshake stays an overridable hook. Spec-required behaviour change: unmatched error responses are now logged (were silently dropped) — BRIDGE_PROTOCOL §3.4's fire-and-forget set_state depends on it.
  • matter_client.py 311 → 147: matter-server specialisation only. tests/test_matter_client.py (446 lines) passes with zero edits.
  • bridge_protocol.py (new, 506): our wire contract — envelope helpers, normalised dataclasses (BridgeCommand/StatusReport/PairingReport/FabricInfo), §1.1 error codes, the §3.1 intent: replace_all rule.
  • bridge_client.py (new, 310): hello-frame handshake with version skew failing closed (halt + resume, no spurious reconnect), attach-reconcile on every connect via an injected endpoint provider, async methods for the full command set, fire-and-forget set_state, event callbacks (command/fabrics_changed/commissioned/window_closed/drift_detected).
  • Golden frames reconciled (both prior reviews asked): single shared tests/fixtures/bridge_protocol/frames.json consumed by BOTH suites; the 13 exchanges the node doesn't implement yet sit in a pending section the TS suite skip-tracks.

Tests

Python 1022 → 1161; Node 68 → 84 (69 pass + 15 pending-skips). pylint 9.26 → 9.33 (note: the previously-recorded 9.90 was a stale pylint cache artifact; 9.26 is the real main baseline).

Spec calls made (documented in code)

factory_reset result assumed {}; drift entry shape taken from the node's DriftEntry; empty attach never auto-carries replace_all (the §3.1 guard is the point); attach pumped inline during handshake (8s, inside the node's 10s unattached close).

🤖 Generated with Claude Code

https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S

Summary by CodeRabbit

  • New Features

    • Added bridge connectivity with automatic reconnection, handshake validation, endpoint synchronization, status, pairing, commissioning, fabric, reset, and recovery commands.
    • Added shared WebSocket communication supporting both Matter and bridge connections.
    • Added bridge protocol support for endpoint roles, events, errors, status, fabrics, and drift reporting.
  • Documentation

    • Expanded architecture and bridge protocol documentation, including drift details and endpoint role definitions.
  • Bug Fixes

    • Improved connection failure handling, request timeouts, malformed-message isolation, and recovery behavior.
  • Tests

    • Added comprehensive protocol, bridge-client, connection, event, and recovery coverage.
  • Chores

    • Updated plugin version to 2026.7.25.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Added a shared reconnecting WebSocket transport for Matter and bridge peers. Added Python bridge protocol models and BridgeClient operations. Unified Python and TypeScript tests around shared golden frames, with expanded handshake, event, recovery, and error coverage.

Changes

Bridge architecture

Layer / File(s) Summary
Bridge protocol contracts and golden frames
indigo-matter.indigoPlugin/Contents/Server Plugin/bridge_protocol.py, tests/fixtures/bridge_protocol/frames.json, tests/test_bridge_protocol_frames.py, docs/BRIDGE_PROTOCOL.md
Added protocol domains, normalized models, request builders, frame parsers, shared golden frames, and protocol coverage tests.
Shared WebSocket transport
indigo-matter.indigoPlugin/Contents/Server Plugin/ws_json_client.py, indigo-matter.indigoPlugin/Contents/Server Plugin/matter_client.py, tests/test_ws_json_client.py, tests/fakes.py
Added reconnect, handshake, request correlation, timeout, halt/resume, malformed-frame, and socket lifecycle handling. MatterClient now uses the shared transport.
BridgeClient connection and command flow
indigo-matter.indigoPlugin/Contents/Server Plugin/bridge_client.py, tests/test_bridge_client.py
Added bridge handshake, attach, endpoint and state commands, event dispatch, commissioning and fabric operations, refusal handling, and endpoint-map recovery.
TypeScript bridge-node contract alignment
bridge-node/src/*, bridge-node/test/*, bridge-node/package.json
Typed event and role domains, moved tests to shared fixtures, and expanded E0 and protocol-frame validation.
Architecture and release documentation
CLAUDE.md, docs/HANDOVER.md, indigo-matter.indigoPlugin/Contents/Info.plist
Updated architecture and testing documentation and incremented the plugin version.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR implements E1 bridge transport and protocol work but does not implement the directly linked valve-cluster requirements [#39]. Implement ValveHandler, matterValve mapping, registry integration, device definitions, and valve tests, or link the PR to the correct E1 issue.
Out of Scope Changes check ⚠️ Warning The PR changes WebSocket transport, bridge protocol/client code, fixtures, and tests, none of which relate to the linked valve-cluster objectives [#39]. Remove the bridge E1 changes from this PR or update the linked issue to match the E1 bridge protocol objective.
Docstring Coverage ⚠️ Warning Docstring coverage is 26.93% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the E1 WebSocket transport extraction and bridge protocol client, which are the primary changes.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/e1-bridge-client

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

simons-plugins and others added 2 commits August 4, 2026 20:53
The plugin half of BRIDGE_PROTOCOL.md: a client that speaks to the bridge node,
built on the transport `matter_client.py` already had. Protocol only — no
plugin.py wiring, no allow-list, no launchd (E2/E7).

**`ws_json_client.py` (new, 359 lines)** — the transport pulled out of
`matter_client.py`: run loop with `min(2**attempt, 30)` backoff, message_id →
future correlation with a `wait_for` deadline and a `finally` pop, disconnect
handling (in-flight futures fail with ConnectionError; `on_disconnect` only on a
genuine drop of a live connection), and the once-per-streak repeated-failure
latch. The handshake stays an overridable hook because the two peers' are
genuinely different — server_info + start_listening vs hello + attach — as does
frame classification, so `protocol.py`'s rename firewall stays intact: this
module never names a wire field.

One deliberate behaviour change (§3.4): an error response that no future is
waiting for is now LOGGED at warning, not dropped. `set_state` is
fire-and-forget, and an unnoticed failure there looks exactly like "the
ecosystem shows stale state". Both clients gain it.

`matter_client.py` drops to 147 lines — the URI, the handshake, the convenience
wrappers — with an identical public API. `tests/test_matter_client.py` passes
unchanged, no import edits needed.

**`bridge_protocol.py` (new)** — our envelope: §3 commands, the §1.1 error-code
domain, §4.2 roles, §5 events, and the normalised dataclasses the plugin
consumes (BridgeCommand, StatusReport, PairingReport, FabricInfo, EndpointSpec).
No rename firewall — we author both ends — so what protects us is
`protocolVersion`, per the spec preamble. `build_attach` carries the §3.1
`intent: "replace_all"` rule: never emitted by default, so a stale client cannot
un-export everything.

**`bridge_client.py` (new)** — hello frame, version check, attach, StatusReport.
Skew fails closed (§2): no attach, `on_version_skew`, and the run loop halts
instead of spinning until `resume()`. Attach carries a *fresh* read of the
injected endpoint provider on every connect, which is the reconcile-on-connect
of PRD §5.4. `set_state` does not await its response — it is called from
Indigo's device thread and must never block it — and a push while disconnected
is dropped, not raised, because attach re-delivers everything (§6.2).

**Golden frames reconciled.** `tests/fixtures/bridge_protocol/frames.json` is
now the single shared location §7 asks for: the E0 frames plus the E1 commands
and every §5 event. Commands the node does not implement yet sit in a `pending`
section the TS suite skips (and asserts is well-formed); the Python suite
asserts them in full, since the plugin half ships now. `npm test` copies the
directory in, so both suites break together when a frame moves.

Tests: 1022 → 1161 Python (all pre-existing pass untouched), 68 → 84 Node
(69 pass, 15 skipped pending). pylint 9.26 → 9.33.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
Three reviews converged on the same theme: the bridge client's failure paths
were the least-examined code in the PR, and several of them were wrong in ways
that only show up on a bad day. Fixes, with a regression test each.

Critical
- set_state's send can raise even while `connected` is True (the listen loop
  has not noticed the socket died). It now drops instead of throwing into
  Indigo's device thread. A push while HALTED warns the truth — nothing is
  reconciling — rather than promising "attach will reconcile".
- Terminal attach refusals no longer retry forever. version_mismatch and
  mass_removal_refused halt with a distinguishable `halted_reason`;
  endpoint_map_invalid keeps the socket OPEN un-attached so §1.1's
  get_status/get_pairing/rebuild_endpoint_map — PRD §7's way out — stay
  reachable, and re-attaches once the rebuild succeeds. Everything else
  reconnects on the normal backoff, logging the error_code, not a traceback.
  New `on_attach_refused` callback and an `attached` property distinct from
  transport-level `connected`.
- A peer that opens the socket and says nothing now hits a 5s HELLO_TIMEOUT in
  the shared base instead of waiting forever — the same latent hang existed in
  MatterClient.

High
- A halt latched inside a run-loop iteration wins over a resume() racing it;
  resume() refuses (and warns) while a run loop is live, and documents that it
  does not restart one.
- Frame containment: an undecodable or non-object frame is logged (truncated)
  and dropped, connection kept. Every exit path from a connection attempt now
  closes the socket — the broad-exception path leaked it and turned the 1s
  backoff into a connect/attach hammer.
- The repeated-failure diagnostic fires from the handshake/broad-except path
  too, so handshake failure streaks reach the supervisor.
- Un-awaited sends keep a bounded FIFO note (message_id → "set_state dev N"),
  so an unmatched error names the device instead of a bare message_id.

Medium
- A dict frame that is neither event nor response is logged, not silently
  dropped; event-dispatch failures name the event and payload in both clients;
  asyncio.TimeoutError is caught distinctly (from 3.11 it IS an OSError) so a
  timeout never logs "connection lost: " with an empty reason; upsert_endpoint
  without endpointNumber and a window without pairing codes raise instead of
  defaulting to 0/"".

Fixtures and spec
- The golden `attach` frame is now lawful (empty request set → empty live set);
  the E0 node's deviation is asserted explicitly on the TS side and noted in
  the fixture. `unknown_command` uses a genuinely unknown name.
- New error frames for malformed_args, unknown_role, commissioning_window_failed
  and internal; a fabrics_changed "deleted"; a command for an unexported device;
  rebuild_endpoint_map now demonstrates reallocation instead of echoing attach.
- Coverage is real: ROLE_STATE_KEYS/ROLE_COMMANDS in bridge_protocol drive
  assertions that every §4.2 role, command name and state key, and every §1.1
  error code, is carried by a frame — deleting any of them fails the suite.
- bridge-node: EventFrame.event and EndpointSummary.role are unions now, so the
  `satisfies` mirror catches misspellings; the seven orphan event frames are
  swept by fixtures.test.ts. BRIDGE_PROTOCOL §4.3 documents DriftEntry.

python: 1287 passed (was 1178). bridge-node: 105 tests, 0 fail, 31 skipped
(pending grew 15→31, all frames the node genuinely lacks handlers for).
pylint 9.37/10.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
@simons-plugins
simons-plugins force-pushed the feat/e1-bridge-client branch from 5680908 to f6c81f0 Compare August 4, 2026 20:37

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (5)
indigo-matter.indigoPlugin/Contents/Server Plugin/ws_json_client.py (1)

393-398: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log the reconcile failure with explicit exception info.

_on_reconcile_done is a done-callback, so no exception is being handled at that point. logging.Logger.exception derives the traceback from sys.exc_info(), which is empty here, so a stdlib logger records NoneType: None instead of the real traceback. Pass the exception explicitly and add a message that names the hook.

♻️ Proposed change
-        exc = task.exception()
-        if exc is not None:
-            self.logger.exception(exc)  # on_connect failed — surface, don't swallow
+        exc = task.exception()
+        if exc is not None:
+            # exc_info is explicit: this runs in a done-callback, so sys.exc_info()
+            # is empty and logger.exception() alone would log "NoneType: None".
+            self.logger.error("%s: on_connect reconcile failed: %s", self.PEER, exc,
+                              exc_info=exc)
🤖 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 `@indigo-matter.indigoPlugin/Contents/Server` Plugin/ws_json_client.py around
lines 393 - 398, Update _on_reconcile_done so the non-cancelled task exception
is logged with an explicit error message naming the reconcile hook and the
captured exc value, rather than using logger.exception without exception
context. Preserve the existing cancellation return and exception check.
indigo-matter.indigoPlugin/Contents/Server Plugin/matter_client.py (2)

91-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the sdk_version wire field name into protocol.py.

Line 93 reads the matter-server field "sdk_version" as a literal in this module. The coding guidelines require matter-server wire field names to stay in protocol.py. Add a constant there and use it here.

♻️ Proposed change
-        version = (self.server_info or {}).get("sdk_version", "version unknown")
+        version = (self.server_info or {}).get(protocol.KEY_SDK_VERSION, "version unknown")

Add to protocol.py alongside the other key constants:

KEY_SDK_VERSION = "sdk_version"

As per coding guidelines: "Keep matter-server wire field names confined to protocol.py, the rename firewall."

🤖 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 `@indigo-matter.indigoPlugin/Contents/Server` Plugin/matter_client.py around
lines 91 - 94, Define a KEY_SDK_VERSION constant in protocol.py alongside the
existing protocol key constants, then update the version lookup in the matter
client’s connection logging to use that constant instead of the literal
"sdk_version".

Source: Coding guidelines


88-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Register a send context for the un-awaited start_listening.

start_listening is sent without a pending future, so its response reaches _log_unmatched. If matter-server answers with an error, the warning names only the message_id. _remember_send_context exists for exactly this case. Record the frame's id so the log line names the request.

♻️ Proposed change
-        await self._send_frame(self.proto.build_request(protocol.CMD_START_LISTENING))
+        frame = self.proto.build_request(protocol.CMD_START_LISTENING)
+        self._remember_send_context(self._message_id_of(frame), protocol.CMD_START_LISTENING)
+        await self._send_frame(frame)
🤖 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 `@indigo-matter.indigoPlugin/Contents/Server` Plugin/matter_client.py around
lines 88 - 90, Update the start_listening send path in the surrounding
connection method to register the frame’s message ID with _remember_send_context
before awaiting _send_frame, so any unmatched error response is logged with the
request context. Preserve the existing fire-and-forget behavior and avoid
changing the reconciliation flow.
tests/test_ws_json_client.py (1)

402-414: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a test for the unmatched-error log line.

The class docstring states that an unmatched error must name the request. The only test here checks the size bound of _send_context. The _log_unmatched warning path is the behaviour the shared transport guarantees, and it is untested in this file. Add a scenario that sends a fire-and-forget request, answers it with an error frame, and asserts the warning contains both the recorded context and the error code.

🤖 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 `@tests/test_ws_json_client.py` around lines 402 - 414, Add a test method to
TestUnmatchedContext covering the _log_unmatched warning path: send a
fire-and-forget request, respond with an error frame, and assert the logger
warning includes both the recorded request context and the error code. Reuse the
existing bridge_client, FakeWebSocket, and mock_logger test fixtures and
preserve the existing bounded-context test.
tests/test_bridge_client.py (1)

699-724: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Drop the unused fake binding.

Both tests unpack fake and never use it. Ruff reports RUF059 on lines 702 and 722. Do not name it _fake, because that shadows the module-level helper at line 73.

🧹 Proposed fix
-            fake, client = self._recovering(mock_logger)
+            _unused, client = self._recovering(mock_logger)
             client._on_attach_refused = lambda code, details: refusals.append(code)
-            fake, client = self._recovering(mock_logger)
+            _unused, client = self._recovering(mock_logger)
             task = asyncio.create_task(client.run())
🤖 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 `@tests/test_bridge_client.py` around lines 699 - 724, Remove the unused fake
binding from both test scenarios by unpacking the _recovering result without
assigning its first value, while retaining the client binding and avoiding the
name _fake. Update the assignments in
test_the_connection_is_kept_open_un_attached and
test_the_three_recovery_commands_still_work.

Source: Linters/SAST tools

🤖 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 `@docs/BRIDGE_PROTOCOL.md`:
- Around line 314-316: Update the §4.3 example’s endpointCount value to match
the documented endpoints.length rule, or add entries until the list contains 12
endpoints; keep the example’s endpointCount and endpoints array consistent.

In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/bridge_client.py:
- Around line 383-393: Update the _SEND_ERRORS handler around _send_frame in the
set_state flow to discard the remembered send context for the failed frame
before logging the failure. Use the frame’s message ID with the existing
send-context removal mechanism, ensuring no context remains when the send cannot
produce a response.

In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/bridge_protocol.py:
- Around line 330-358: Update parse_status, parse_fabrics, and parse_command to
validate required fields before indexing or converting values, and raise
BridgeProtocolError(ERR_MALFORMED_ARGS, ...) for missing or malformed required
data instead of allowing KeyError or TypeError to escape. Preserve existing
normalization for valid payloads and ensure malformed status/command responses
follow the attach refusal and failed-event handling paths.

---

Nitpick comments:
In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/matter_client.py:
- Around line 91-94: Define a KEY_SDK_VERSION constant in protocol.py alongside
the existing protocol key constants, then update the version lookup in the
matter client’s connection logging to use that constant instead of the literal
"sdk_version".
- Around line 88-90: Update the start_listening send path in the surrounding
connection method to register the frame’s message ID with _remember_send_context
before awaiting _send_frame, so any unmatched error response is logged with the
request context. Preserve the existing fire-and-forget behavior and avoid
changing the reconciliation flow.

In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/ws_json_client.py:
- Around line 393-398: Update _on_reconcile_done so the non-cancelled task
exception is logged with an explicit error message naming the reconcile hook and
the captured exc value, rather than using logger.exception without exception
context. Preserve the existing cancellation return and exception check.

In `@tests/test_bridge_client.py`:
- Around line 699-724: Remove the unused fake binding from both test scenarios
by unpacking the _recovering result without assigning its first value, while
retaining the client binding and avoiding the name _fake. Update the assignments
in test_the_connection_is_kept_open_un_attached and
test_the_three_recovery_commands_still_work.

In `@tests/test_ws_json_client.py`:
- Around line 402-414: Add a test method to TestUnmatchedContext covering the
_log_unmatched warning path: send a fire-and-forget request, respond with an
error frame, and assert the logger warning includes both the recorded request
context and the error code. Reuse the existing bridge_client, FakeWebSocket, and
mock_logger test fixtures and preserve the existing bounded-context test.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b932579b-e18f-4110-acb4-7d249dffa723

📥 Commits

Reviewing files that changed from the base of the PR and between f763efe and f6c81f0.

📒 Files selected for processing (23)
  • CLAUDE.md
  • bridge-node/package.json
  • bridge-node/src/node.ts
  • bridge-node/src/protocol.ts
  • bridge-node/src/ws-server.ts
  • bridge-node/test/fixture-shapes.ts
  • bridge-node/test/fixtures.test.ts
  • bridge-node/test/fixtures/e0-frames.json
  • bridge-node/test/protocol.test.ts
  • bridge-node/test/stub-bridge.ts
  • docs/BRIDGE_PROTOCOL.md
  • docs/HANDOVER.md
  • indigo-matter.indigoPlugin/Contents/Info.plist
  • indigo-matter.indigoPlugin/Contents/Server Plugin/bridge_client.py
  • indigo-matter.indigoPlugin/Contents/Server Plugin/bridge_protocol.py
  • indigo-matter.indigoPlugin/Contents/Server Plugin/matter_client.py
  • indigo-matter.indigoPlugin/Contents/Server Plugin/ws_json_client.py
  • tests/conftest.py
  • tests/fakes.py
  • tests/fixtures/bridge_protocol/frames.json
  • tests/test_bridge_client.py
  • tests/test_bridge_protocol_frames.py
  • tests/test_ws_json_client.py
💤 Files with no reviewable changes (1)
  • bridge-node/test/fixtures/e0-frames.json

Comment thread docs/BRIDGE_PROTOCOL.md
Comment on lines +314 to +316
`endpoints[].role` is one of the §4.2 enum; `endpointCount` is
`endpoints.length` (it is sent explicitly so a client can log the size without
walking the list).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

The new endpointCount rule contradicts the §4.3 example.

The added text states that endpointCount is endpoints.length. The example above it shows "endpointCount": 12 with one entry in endpoints. Update the example to 1, or extend the example list, so the shape and the rule agree.

🤖 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 `@docs/BRIDGE_PROTOCOL.md` around lines 314 - 316, Update the §4.3 example’s
endpointCount value to match the documented endpoints.length rule, or add
entries until the list contains 12 endpoints; keep the example’s endpointCount
and endpoints array consistent.

Comment on lines +383 to +393
self._remember_send_context(self.proto.message_id_of(frame),
f"set_state dev {indigo_device_id}")
try:
await self._send_frame(frame)
except _SEND_ERRORS as exc:
# The listen loop has not noticed the socket is gone yet, so
# ``connected`` is still True and the check above passed. Dropping is
# still right (attach re-delivers), but it must not surface as an
# exception on Indigo's device thread.
self.logger.debug("bridge node send failed (%s); dropping set_state for %s",
exc, indigo_device_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the send-context bookkeeping in the shared transport.
fd -t f 'ws_json_client.py' --exec ast-grep outline {} --items all
fd -t f 'ws_json_client.py' --exec rg -n -C4 '_send_context|_remember_send_context'

Repository: simons-plugins/indigo-matter

Length of output: 6706


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant state-correlation and send methods, plus tests around missing responses.
printf '\n== outlines ==\n'
for f in indigo-matter.indigoPlugin/Contents/Server\ Plugin/bridge_client.py indigo-matter.indigoPlugin/Contents/Server\ Plugin/ws_json_client.py tests/test_bridge_client.py tests/test_ws_json_client.py; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    ast-grep outline "$f" --match '_handle_frame|_send_context|_remember_send_context|_send_frame|set_state|UnmatchedContext|missing|no' --view expanded || true
  fi
done

printf '\n== targeted source snippets ==\n'
sed -n '330,410p' indigo-matter.indigoPlugin/Contents/Server\ Plugin/bridge_client.py
printf '\n--- ws_json_client relevant ---\n'
sed -n '330,425p' indigo-matter.indigoPlugin/Contents/Server\ Plugin/ws_json_client.py

printf '\n== targeted tests ==\n'
rg -n -C 8 '_send_context|missing response|unmatched|no response|set_state|_send_frame|_ask|_request' indigo-matter.indigoPlugin/Contents/Server\ Plugin tests

Repository: simons-plugins/indigo-matter

Length of output: 50385


Discard the send context when send fails.

When _send_frame raises, no response for that message_id can arrive, and the unmatched-response path never removes the send-context entry. Remove it in the _SEND_ERRORS handler before logging the failure.

🤖 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 `@indigo-matter.indigoPlugin/Contents/Server` Plugin/bridge_client.py around
lines 383 - 393, Update the _SEND_ERRORS handler around _send_frame in the
set_state flow to discard the remembered send context for the failed frame
before logging the failure. Use the frame’s message ID with the existing
send-context removal mechanism, ensuring no context remains when the send cannot
produce a response.

Comment on lines +330 to +358
def parse_fabrics(data: Any) -> list:
"""Normalise a list of §4.3 ``FabricInfo`` objects."""
return [
FabricInfo(
fabric_index=int(item[ARG_FABRIC_INDEX]),
label=str(item.get("label", "")),
vendor_id=int(item.get("vendorId", 0)),
)
for item in (data or [])
]


def parse_status(result: Any) -> StatusReport:
"""Normalise a ``StatusReport`` payload (§4.3)."""
data = result or {}
return StatusReport(
commissioned=bool(data.get("commissioned", False)),
fabrics=parse_fabrics(data.get("fabrics")),
endpoint_count=int(data.get("endpointCount", 0)),
endpoints=[
EndpointSummary(
indigo_device_id=int(item[ARG_INDIGO_DEVICE_ID]),
endpoint_number=int(item["endpointNumber"]),
role=str(item.get("role", "")),
)
for item in (data.get("endpoints") or [])
],
drift=parse_drift(data.get("drift")),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect exception handling around bridge_protocol parsers in the client.
fd -t f 'bridge_client.py' | while IFS= read -r f; do
  rg -n -C 6 'parse_status|parse_command|parse_fabrics|parse_drift|except ' "$f"
done

Repository: simons-plugins/indigo-matter

Length of output: 5891


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- locate files ---'
fd -t f 'bridge_protocol.py|bridge_client.py' .

printf '%s\n' '--- bridge_protocol relevant parser definitions ---'
f=$(fd -t f 'bridge_protocol.py' . | head -n 1)
wc -l "$f"
sed -n '320,425p' "$f"

printf '%s\n' '--- client attach/event parsing calls and handlers ---'
c=$(fd -t f 'bridge_client.py' . | head -n 1)
wc -l "$c"
sed -n '160,190p' "$c"
sed -n '230,280p' "$c"
sed -n '320,345p' "$c"
sed -n '410,445p' "$c"
sed -n '455,472p' "$c"

Repository: simons-plugins/indigo-matter

Length of output: 12611


Fail closed on missing required status/command fields.

parse_status, parse_fabrics, and parse_command raise KeyError or TypeError when required fields such as ARG_INDIGO_DEVICE_ID, endpointNumber, or ARG_FABRIC_INDEX are absent. These exceptions can bypass the attach refusal path and can appear as generic failed events. Convert these missing-field cases to BridgeProtocolError(ERR_MALFORMED_ARGS, ...).

🤖 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 `@indigo-matter.indigoPlugin/Contents/Server` Plugin/bridge_protocol.py around
lines 330 - 358, Update parse_status, parse_fabrics, and parse_command to
validate required fields before indexing or converting values, and raise
BridgeProtocolError(ERR_MALFORMED_ARGS, ...) for missing or malformed required
data instead of allowing KeyError or TypeError to escape. Preserve existing
normalization for valid payloads and ensure malformed status/command responses
follow the attach refusal and failed-event handling paths.

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