refactor(transport): put a MessageTransport port in front of Matrix (CHOO-1436)

[amaudruz]

Why

Replacing the Matrix homeserver with a Postgres-backed message store means touching every module that imports matrix-nio. There were eight, and two of them — bridges/agent/protocol/service.py and bridges/collaboration/bridge_core.py — reached past ClientBase to the raw nio client at 28 sites between them.

This is step 2 of the agreed plan: put a port in front of Matrix so every later step is "write another implementation" rather than surgery on 5,500 lines.

Behaviour is unchanged. That is the point of the step, and the thing to check in review.

What you get

git grep "^from nio\|^import nio" core/switch_core/ returns exactly one file: switch_core/transport/matrix.py. No module reaches for a raw client — the nio_client escape hatch is deleted, not deprecated.

A test enforces both by parsing imports (tests/switch_core/transport/test_no_nio_outside_transport.py), plus a test that the guard can actually detect a violation — a check that always passes is worse than none.

The port

• transport/types.py — neutral vocabulary. SendResult, UploadResult, DownloadResult, HistoryPage, and the inbound DTOs RoomRef / InboundEvent / InboundMessage / InboundMedia / InboundMembership / InboundCustomEvent. Nothing here names Matrix.
• transport/port.py — MessageTransport, a runtime-checkable Protocol. Every method corresponds to something a caller already did with a nio client.
• transport/matrix.py — the implementation. Content dicts, m.relates_to, mxc URIs, pagination tokens, the sync loop, and the timestamp_to_event seek all live here.

What changed at the callers

• ClientBase names no implementation. It holds a MessageTransport built by a factory the caller injects, so the choice of transport is the factory's business.
• Credentials are opaque. access_token / device_id are gone; ClientBase holds a session_state dict it persists and hands back without reading. A client still holding a Matrix access token would still be a Matrix client.
• Inbound events are DTOs. The transport converts at the edge, so all 29 on_* hooks and five subclasses take (RoomRef, Inbound*).
• service.py and bridge_core.py are closed. History, single-event fetch, media download, the timestamp seek and all 16 com.switch.* sends go through the port.

Two design calls worth arguing about

The port raises; ClientBase still returns None. send_message used to log-and-return-None and callers depend on that, so ClientBase catches TransportError and preserves it exactly. The contract is right and behaviour is unchanged; the try/except pairs go when callers are ready.

InboundMedia extends InboundMessage. mypy forced this and it was the right correction — a media event is a message that carries a file, exactly as nio's own RoomMessageMedia and RoomMessageText both extend RoomMessage. Code that only cares about sender, body or thread now treats them alike, which is what the calling code already assumed.

The bug this nearly shipped — worth reading

The mechanical part of this migration was done with a codemod, and it half-converted read_context. Four reads still used nio attribute names on objects that are now DTOs. Because they were written defensively — getattr(event, "server_timestamp", None), hasattr(event, "source") — nothing raised. It degraded silently:

• sender names fell back to the raw mxid; attachment metadata came out {"mimetype": "", "size": 0}
• every timestamp was None, so since/before filtering never fired, oldest_timestamp was always null and truncated was wrong
• _thread_root_id never matched, so every message became its own thread root — threading in read_context was entirely gone

The tell was inside a single function: one line already used the new timestamp field while another still used server_timestamp.

This is the failure mode the repo's "fail loud" rule exists to prevent. Those defensive spellings were written when the events really were nio objects, outlived their reason, and turned a rename into wrong data instead of a crash. They are removed along with the bug.

It was caught because the tests were treated as the specification: the fixtures were updated to the new vocabulary, but no assertion was weakened to make a failure go away. Ten tests kept failing and were right to.

Testing

• 2204 passed, 0 failed. ruff and mypy clean across 196 source files.
• FakeTransport (tests/switch_core/transport/fake.py) is the shared double replacing every ad-hoc fake nio client.
• 16 new tests for the transport, including the send_media wire-format cases (caption vs filename, multi-file group marker) that would otherwise have been lost when that serialisation moved behind the port.
• The only test file importing nio is the one testing the Matrix implementation.

Not verified

The integration suite against the real Matrix stack has not been run. For a change whose entire claim is behaviour preservation, that is the real safety net and it should be green before this merges.

Deliberately out of scope

• matrix_user_id remains as the client's address. Renaming it to a neutral identity is a repo-wide codemod that belongs with the schema change in step 5.
• The lifecycle is still Matrix-shaped: register_user provisioning and the next_batch resume cursor stay until the transport is actually replaced.

🤖 Generated with Claude Code