feat(channel): relay permission prompts through Matrix #115

Merged

jlxq0 merged 3 commits from permission-relay into main 2026-08-25 11:03:24 +00:00

Conversation 0 Commits 3 Files changed 8 +1473 -84

jlxq0 commented 2026-08-25 07:11:44 +00:00

Owner

Copy link

Refs #109. Does not close it — see below.

/channel declares claude/channel/permission alongside claude/channel, so
a session on it can have its tool-approval dialogs answered from Matrix. Claude
Code forwards each prompt as notifications/claude/channel/permission_request;
we render it into a Matrix message carrying the five-letter request id verbatim
and send notifications/claude/channel/permission back with the verdict. Both
dialogs stay live and the first answer wins.

Claude wants to run Bash: Run shell command
{"command": "cargo test --all-features"}

Reply "yes qmzkd" or "no qmzkd"

The three things the docs' worked example does not answer

The docs' example is a stdio server serving one user at a terminal. We are a
remote streamable-HTTP server serving many identities and many sessions.

Which room the prompt goes to. A permission request arrives on an MCP peer
that carries no room at all. permission_room in the account's own
app.matrix_mcp.channel account data is the destination when set — an explicit
statement beats an inferred one, and it cannot be moved by traffic. Otherwise
it is the room an allowlisted sender last wrote in, recorded only after the
allowlist gate passes: recording it before would let a stranger who DMs the bot
redirect every later approval prompt into a room he controls. Neither known is
a warn! and a drop that says the prompt could not be relayed. A guessed room
is an approval handed to whoever is standing in it.

Where the verdict goes. To the session that issued the request, via a
(mxid, request_id) -> session_key map with a 15-minute TTL and a 256-entry
cap. Broadcasting would also work, since a client drops a verdict for an id it
did not issue, but targeting is what produces the log line that makes issue
acceptance item 3 observable at our own layer. The MXID is part of the key
rather than a stored field: five letters from a 25-letter alphabet is ~9.8M
ids, and a bare-id key would let a collision across identities silently evict
the other identity's pending request.

No feedback loop. Confirmed rather than assumed. The prompt is sent by the
client built from the authenticated identity, so ev.sender == mxid in
push_message, and that check sits above both the verdict branch and the
allowlist gate (src/channel.rs, push_message).

The ordering, which is the security requirement

Anyone who can get a verdict through the inbound path can approve tool use in
the session, so the allowlist gate has to run before the verdict branch. That
ordering now lives in a signature rather than in statement order:

enum Inbound { Verdict { request_id: String, behavior: Behavior }, Chat, Ignored }
fn classify_inbound(sender_allowed: bool, text: &str) -> Inbound

classify_inbound(false, "yes abcde") is Ignored, whatever the text says, and
a unit test asks that question directly instead of it being a property of where
a return happens to sit. The regex is the docs' pattern verbatim,
^\s*(y|yes|n|no)\s+([a-km-z]{5})\s*$ with (?i), and the captured id is
lowercased. A verdict is answered exactly once and never also forwarded as chat.

Second round: four findings from review

alan reviewed the diff and Codex confirmed the first trace independently. All
four were confirmed against the code before being fixed, in a6e9d74.

1. A verdict was replayed into a fresh context as chat. handled_as_verdict
consumes the message and returns without pushing, so nothing calls mark_read
and the receipt watermark never moves past it. replay_room filtered only on
sender and carried_of, and a verdict is m.text with a body — so the next
attach got <matrix:message>no qmzkd</matrix:message> with no referent, which
is exactly what the live path exists to refuse, defeated by the other path. The
two paths disagreeing about one event is the #107 bug in another costume.
Replay classifies too now. Not fixed by advancing the receipt: that would skip
the unacknowledged messages behind it.

There was a trap inside this fix. ReadEvent::untrusted_body holds the
sandbox-wrapped body, and the verdict pattern is anchored, so a check written
against that field matches nothing and silently does nothing while looking
correct. is_replayed_verdict reads content.body through raw_body, and
the_replay_verdict_check_reads_the_unwrapped_body exists to tell the two
apart — pointing raw_body at untrusted_body turns it red.

2. deliver_verdict retired the request before knowing the send worked. A
missing peer or a failed send left the id gone, so a second no qmzkd from
Matrix read as unissued while the terminal dialog was still open. It now looks
up, sends, and retires only on Ok. Retirement is conditional on a per-entry
serial: no lock is held across the await, so the map may hold a different
prompt at that key by the time delivery succeeds, and removing by key alone
would retire the newer one. Instant cannot serve — two entries created in the
same tick compare equal.

3. client.get_room() returns Left, Invited, Knocked and Banned rooms, so
Some was not "joined" — the trap #113 fixed in download_attachment. Nothing
leaked, because room.send then failed with WrongRoomState, but a pending id
was retained for a prompt nobody saw and the operator was told the wrong thing
twice. prompt_room_is_usable is an allowlist of one rather than != Left, so
a state added by a future matrix-sdk defaults to refused.

4. live_peers(mxid) == 0 returned above the verdict branch. live_peers
counts without evicting, because is_transport_closed false-positives and
evicting on a read once killed the channel for a whole session — so live == 0
does not mean deliver_verdict would fail to find the peer. A verdict was
dropped at the top of push_message for a dialog still open at the terminal.
The early return now stands unless has_pending(mxid): one in-memory map read,
on the path that was about to return anyway, so the no-listener common case
still costs no config fetch.

Controls for the four

Mutation	Test that caught it
replay verdict filter deleted	a_verdict_is_not_replayed_into_a_fresh_context_as_chat
replay filter drops everything, not just verdicts	same
verdict check reads the sandbox-wrapped body	the_replay_verdict_check_reads_the_unwrapped_body
entry consumed before the send	a_verdict_that_reached_nobody_stays_answerable
retirement ignores the serial	an_in_flight_delivery_cannot_retire_a_reissued_prompt
retirement never removes	a_delivered_verdict_is_answerable_exactly_once
room-state check deleted	a_prompt_only_goes_into_a_joined_room
room-state check written as != Left	same
has_pending always false	a_verdict_is_looked_for_even_when_nothing_looks_live
has_pending ignores the identity	same
has_pending ignores the TTL	an_expired_prompt_does_not_keep_the_no_listener_path_awake

Fix 2 falsified an existing test rather than adding to it:
a_verdict_is_answerable_exactly_once asserted the entry was consumed after
deliver_verdict, which is now true only on a path that delivered. Its real
property moved into retire_if_current, which is a function rather than test
scaffolding, and split into the two tests above.

Negative controls

Per AGENTS.md, every claim below was checked by breaking the code it covers and
watching the test go red. Twenty mutations, all red:

Mutation	Test that caught it
classify_inbound ignores the sender gate	a_verdict_from_a_sender_who_is_not_allowlisted_is_ignored
id alphabet allows l	near_misses_fall_through_as_chat_rather_than_as_verdicts
id length not exactly 5	same
whitespace between word and id made optional	same
end anchor dropped, trailing text tolerated	same
(?i) dropped	autocorrect_capitalisation_is_tolerated_and_the_id_is_lowercased
captured id not lowercased	same
prompt omits the reply instruction	the_prompt_carries_the_request_id_verbatim
per-field caps made unlimited	an_oversized_preview_never_eats_the_reply_instruction
clip slices without walking to a char boundary	a_multibyte_preview_is_clipped_on_a_character_boundary
unissued verdict reported as delivered	a_verdict_for_an_id_we_never_issued_is_dropped
pending map keyed on request_id alone	a_verdict_does_not_cross_an_identity_boundary
answered request left pending	a_verdict_is_answerable_exactly_once
cap on outstanding prompts removed	outstanding_prompts_are_bounded
cap applied after the insert, evicting the newest	the_newest_prompt_survives_the_cap
remember_room keeps the first room forever	the_prompt_destination_starts_unknown_and_follows_allowlisted_traffic
permission_room field name wrong on the wire	the_config_round_trips_and_defaults_to_no_permission_room
behavior wire values swapped	the_verdict_wire_values_are_the_two_claude_code_accepts
request notification method name typo	the_relay_method_names_are_the_ones_in_the_contract
m.notice carried live but not on replay	the_live_classifier_agrees_with_the_replay_one
capability declared under the wrong key	channel_mount_declares_the_permission_capability
capability declared with a non-empty value	same
capability also declared on the full mount	the_main_mount_never_declares_the_permission_capability

Three of these were green on the first pass and the tests were fixed, not the
mutations:

• a_multibyte_preview_is_clipped_on_a_character_boundary used
  "ü".repeat(MAX), which puts a character boundary at every even offset — and
  both caps are even, so a clip that never walked to a boundary passed. The
  fixture now carries a one-byte ASCII prefix to shift every boundary off the
  cap. This is now an AGENTS.md pitfall: the same trap applies to cap_body
  and cap_wrapped.
• a_verdict_does_not_cross_an_identity_boundary needed the mutation applied
  at both the insert and the lookup; with only one changed the lookup missed
  for the wrong reason.
• the_newest_prompt_survives_the_cap is guarded by a conjunction
  (prune-before-insert and evict-oldest); flipping either half alone leaves
  it true. The control applies both.

What is not covered by a test, and why

Every decision in this PR is a pure function with a mutation-proven test.
No call site in push_message, replay_room or on_custom_notification
is reachable from a unit test — each needs a live Room, Client or Peer.
I measured that rather than assuming it: deleting the call to
is_replayed_verdict, retire_if_current, has_pending or
prompt_room_is_usable leaves the suite green.

So the split is: what to decide is tested here, and that the decision is
consulted is issue acceptance items 1-4 on the deployed server. The one
exception is deliver_verdict's look-up-don't-consume ordering, which is
exercised end to end by a_verdict_that_reached_nobody_stays_answerable —
hoisting the removal back above the send turns it red.

Not closing the issue

Refs #109, deliberately not Closes. The four acceptance observations in
#109 are live ones: a real gated call answered no <id> from Matrix and seen
to be rejected, the same with yes <id>, a well-formed unissued id seen to be
dropped, and a non-allowlisted sender's yes <id> seen to do nothing. They
need the deployed server and a human at a phone, so they happen after Clark's
release, not in this PR. A keyword that auto-closed on merge would leave the
reopen as the step that gets forgotten.

Scope

CHANNEL_TOOLS and CHANNEL_INSTRUCTIONS are untouched (#111 is changing
both). No release tag.

carried_of_live is the one refactor beyond relay: clippy::too_many_lines
fired on push_message once the verdict branch went in, and splitting the
five-armed msgtype match out mirrors carried_of on the replay path. A test
asserts the two classifiers agree.

Gates

cargo fmt --all --check, cargo clippy --all-targets --all-features --locked -- -D warnings, cargo test --all-features --locked (225 passed),
cargo audit, cargo deny check bans licenses sources — all green on stable
1.98.0 with RUSTFLAGS=-Dwarnings and OPUS_STATIC=1.

Rebased onto 7525066 (#114 merged), so ci.yml pins 1.98.0 and my local
runs match the runner exactly. The tree cannot lint below 1.98.0 — that is #112,
and AGENTS.md now carries the measured before/after table.

regex is now a direct dependency, named for the verbatim pattern. It was
already in the tree transitively, so the lockfile delta is one line and no new
crate compiles.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SHy74Zo4sLTBcEY4o5REXu

---

Director's re-read, and three items applied in 02ff731

Gates reproduced independently on 1.98.0 before review: fmt clean, clippy 0 errors, 235 tests at that point. The four findings are closed and the untrusted_body trap found inside finding 1 is the best catch here — verified at src/mcp.rs:1085-1091, where the doc says the field is sandbox-wrapped, so an anchored pattern against it matches nothing and the fix would have looked right and done nothing.

The declared coverage gap was verified rather than accepted: stubbing the is_replayed_verdict call in replay_room left all 235 tests green. Honest, not pessimistic.

1. Replay's judgement is now reachable. replay_deliverable is lifted out of replay_room's closure — allowlisted sender, not an already-answered verdict, a carried msgtype, within budget. Inside a closure none of it could be tested; replay_room keeps the homeserver round trip and nothing else.

Control	Before	After
verdict check disabled	green	red
sender gate disabled	green	red
budget ignored	green	red

the_replay_filter_does_not_drop_everything is the control for the control: a filter returning None for everything satisfies "a verdict is not replayed" while destroying the channel.

on_custom_notification and push_message stay unreachable, and that is accepted — they need a live Client, Room and Peer, and inventing seams to reach them buys a test of the seam. Replay was different: the seam was already a closure and the ReadEvent fixtures already existed.

2. prompt_room_is_usable moved to the site its doc describes. It documents "whether a permission prompt may be sent into a room in this state" and its only call site was read_thread, which has nothing to do with permission prompts, while the permission room used a raw != Joined. Behaviourally identical, but the allowlist-of-one argument did not apply where it was written down, and read_thread was not this PR's to touch. Reverted there, applied at the permission site. One predicate across all five RoomState::Joined sites is worth doing separately and should be named for the property, not a caller.

3. A retirement declined by the serial is logged. retire_if_current returning false is the case the serial exists for — a delivery in flight while the id was reissued. Declining is correct; being unable to observe it is not.

Cross-engine review (Codex)

• ABA protection is complete short of a u64 wrap: comparison and removal happen under one write lock, a replacement before it carries a different serial, one after it survives.
• The push_message restructure pushes nothing new. A verdict now reaches the peer when live_peers false-negatives to 0 — which is finding 4's fix, intended. Every non-verdict with live == 0 hits the second return, and remember_room stays below it.

Gates

fmt, clippy -D warnings (0 errors), 240 tests on rustc 1.98.0, rebased onto e15a350.

Still not closing #109

Unchanged: the four acceptance observations need the deployed server and a human answering from a phone. Refs, not Closes.

Refs #109. **Does not close it** — see below. `/channel` declares `claude/channel/permission` alongside `claude/channel`, so a session on it can have its tool-approval dialogs answered from Matrix. Claude Code forwards each prompt as `notifications/claude/channel/permission_request`; we render it into a Matrix message carrying the five-letter request id verbatim and send `notifications/claude/channel/permission` back with the verdict. Both dialogs stay live and the first answer wins. ``` Claude wants to run Bash: Run shell command {"command": "cargo test --all-features"} Reply "yes qmzkd" or "no qmzkd" ``` ## The three things the docs' worked example does not answer The docs' example is a stdio server serving one user at a terminal. We are a remote streamable-HTTP server serving many identities and many sessions. **Which room the prompt goes to.** A permission request arrives on an MCP peer that carries no room at all. `permission_room` in the account's own `app.matrix_mcp.channel` account data is the destination when set — an explicit statement beats an inferred one, and it cannot be moved by traffic. Otherwise it is the room an allowlisted sender last wrote in, recorded **only after** the allowlist gate passes: recording it before would let a stranger who DMs the bot redirect every later approval prompt into a room he controls. Neither known is a `warn!` and a drop that says the prompt could not be relayed. A guessed room is an approval handed to whoever is standing in it. **Where the verdict goes.** To the session that issued the request, via a `(mxid, request_id) -> session_key` map with a 15-minute TTL and a 256-entry cap. Broadcasting would also work, since a client drops a verdict for an id it did not issue, but targeting is what produces the log line that makes issue acceptance item 3 observable at our own layer. The MXID is part of the key rather than a stored field: five letters from a 25-letter alphabet is ~9.8M ids, and a bare-id key would let a collision across identities silently evict the other identity's pending request. **No feedback loop.** Confirmed rather than assumed. The prompt is sent by the client built from the authenticated identity, so `ev.sender == mxid` in `push_message`, and that check sits above both the verdict branch and the allowlist gate (`src/channel.rs`, `push_message`). ## The ordering, which is the security requirement Anyone who can get a verdict through the inbound path can approve tool use in the session, so the allowlist gate has to run before the verdict branch. That ordering now lives in a signature rather than in statement order: ```rust enum Inbound { Verdict { request_id: String, behavior: Behavior }, Chat, Ignored } fn classify_inbound(sender_allowed: bool, text: &str) -> Inbound ``` `classify_inbound(false, "yes abcde")` is `Ignored`, whatever the text says, and a unit test asks that question directly instead of it being a property of where a `return` happens to sit. The regex is the docs' pattern verbatim, `^\s*(y|yes|n|no)\s+([a-km-z]{5})\s*$` with `(?i)`, and the captured id is lowercased. A verdict is answered exactly once and never also forwarded as chat. ## Second round: four findings from review alan reviewed the diff and Codex confirmed the first trace independently. All four were confirmed against the code before being fixed, in `a6e9d74`. **1. A verdict was replayed into a fresh context as chat.** `handled_as_verdict` consumes the message and returns without pushing, so nothing calls `mark_read` and the receipt watermark never moves past it. `replay_room` filtered only on sender and `carried_of`, and a verdict is `m.text` with a body — so the next attach got `<matrix:message>no qmzkd</matrix:message>` with no referent, which is exactly what the live path exists to refuse, defeated by the other path. The two paths disagreeing about one event is the #107 bug in another costume. Replay classifies too now. Not fixed by advancing the receipt: that would skip the unacknowledged messages behind it. There was a trap inside this fix. `ReadEvent::untrusted_body` holds the *sandbox-wrapped* body, and the verdict pattern is anchored, so a check written against that field matches nothing and silently does nothing while looking correct. `is_replayed_verdict` reads `content.body` through `raw_body`, and `the_replay_verdict_check_reads_the_unwrapped_body` exists to tell the two apart — pointing `raw_body` at `untrusted_body` turns it red. **2. `deliver_verdict` retired the request before knowing the send worked.** A missing peer or a failed send left the id gone, so a second `no qmzkd` from Matrix read as unissued while the terminal dialog was still open. It now looks up, sends, and retires only on `Ok`. Retirement is conditional on a per-entry serial: no lock is held across the await, so the map may hold a *different* prompt at that key by the time delivery succeeds, and removing by key alone would retire the newer one. `Instant` cannot serve — two entries created in the same tick compare equal. **3. `client.get_room()` returns Left, Invited, Knocked and Banned rooms**, so `Some` was not "joined" — the trap #113 fixed in `download_attachment`. Nothing leaked, because `room.send` then failed with `WrongRoomState`, but a pending id was retained for a prompt nobody saw and the operator was told the wrong thing twice. `prompt_room_is_usable` is an allowlist of one rather than `!= Left`, so a state added by a future matrix-sdk defaults to refused. **4. `live_peers(mxid) == 0` returned above the verdict branch.** `live_peers` counts without evicting, because `is_transport_closed` false-positives and evicting on a read once killed the channel for a whole session — so `live == 0` does not mean `deliver_verdict` would fail to find the peer. A verdict was dropped at the top of `push_message` for a dialog still open at the terminal. The early return now stands unless `has_pending(mxid)`: one in-memory map read, on the path that was about to return anyway, so the no-listener common case still costs no config fetch. ### Controls for the four | Mutation | Test that caught it | |---|---| | replay verdict filter deleted | `a_verdict_is_not_replayed_into_a_fresh_context_as_chat` | | replay filter drops everything, not just verdicts | same | | verdict check reads the sandbox-wrapped body | `the_replay_verdict_check_reads_the_unwrapped_body` | | entry consumed before the send | `a_verdict_that_reached_nobody_stays_answerable` | | retirement ignores the serial | `an_in_flight_delivery_cannot_retire_a_reissued_prompt` | | retirement never removes | `a_delivered_verdict_is_answerable_exactly_once` | | room-state check deleted | `a_prompt_only_goes_into_a_joined_room` | | room-state check written as `!= Left` | same | | `has_pending` always false | `a_verdict_is_looked_for_even_when_nothing_looks_live` | | `has_pending` ignores the identity | same | | `has_pending` ignores the TTL | `an_expired_prompt_does_not_keep_the_no_listener_path_awake` | Fix 2 falsified an existing test rather than adding to it: `a_verdict_is_answerable_exactly_once` asserted the entry was consumed after `deliver_verdict`, which is now true only on a path that delivered. Its real property moved into `retire_if_current`, which is a function rather than test scaffolding, and split into the two tests above. ## Negative controls Per AGENTS.md, every claim below was checked by breaking the code it covers and watching the test go red. Twenty mutations, all red: | Mutation | Test that caught it | |---|---| | `classify_inbound` ignores the sender gate | `a_verdict_from_a_sender_who_is_not_allowlisted_is_ignored` | | id alphabet allows `l` | `near_misses_fall_through_as_chat_rather_than_as_verdicts` | | id length not exactly 5 | same | | whitespace between word and id made optional | same | | end anchor dropped, trailing text tolerated | same | | `(?i)` dropped | `autocorrect_capitalisation_is_tolerated_and_the_id_is_lowercased` | | captured id not lowercased | same | | prompt omits the reply instruction | `the_prompt_carries_the_request_id_verbatim` | | per-field caps made unlimited | `an_oversized_preview_never_eats_the_reply_instruction` | | `clip` slices without walking to a char boundary | `a_multibyte_preview_is_clipped_on_a_character_boundary` | | unissued verdict reported as delivered | `a_verdict_for_an_id_we_never_issued_is_dropped` | | pending map keyed on `request_id` alone | `a_verdict_does_not_cross_an_identity_boundary` | | answered request left pending | `a_verdict_is_answerable_exactly_once` | | cap on outstanding prompts removed | `outstanding_prompts_are_bounded` | | cap applied after the insert, evicting the newest | `the_newest_prompt_survives_the_cap` | | `remember_room` keeps the first room forever | `the_prompt_destination_starts_unknown_and_follows_allowlisted_traffic` | | `permission_room` field name wrong on the wire | `the_config_round_trips_and_defaults_to_no_permission_room` | | `behavior` wire values swapped | `the_verdict_wire_values_are_the_two_claude_code_accepts` | | request notification method name typo | `the_relay_method_names_are_the_ones_in_the_contract` | | `m.notice` carried live but not on replay | `the_live_classifier_agrees_with_the_replay_one` | | capability declared under the wrong key | `channel_mount_declares_the_permission_capability` | | capability declared with a non-empty value | same | | capability also declared on the full mount | `the_main_mount_never_declares_the_permission_capability` | Three of these were green on the first pass and the tests were fixed, not the mutations: - `a_multibyte_preview_is_clipped_on_a_character_boundary` used `"ü".repeat(MAX)`, which puts a character boundary at every even offset — and both caps are even, so a `clip` that never walked to a boundary passed. The fixture now carries a one-byte ASCII prefix to shift every boundary off the cap. This is now an AGENTS.md pitfall: the same trap applies to `cap_body` and `cap_wrapped`. - `a_verdict_does_not_cross_an_identity_boundary` needed the mutation applied at both the insert and the lookup; with only one changed the lookup missed for the wrong reason. - `the_newest_prompt_survives_the_cap` is guarded by a conjunction (prune-before-insert *and* evict-oldest); flipping either half alone leaves it true. The control applies both. ## What is not covered by a test, and why Every **decision** in this PR is a pure function with a mutation-proven test. No **call site** in `push_message`, `replay_room` or `on_custom_notification` is reachable from a unit test — each needs a live `Room`, `Client` or `Peer`. I measured that rather than assuming it: deleting the call to `is_replayed_verdict`, `retire_if_current`, `has_pending` or `prompt_room_is_usable` leaves the suite green. So the split is: what to decide is tested here, and that the decision is consulted is issue acceptance items 1-4 on the deployed server. The one exception is `deliver_verdict`'s look-up-don't-consume ordering, which *is* exercised end to end by `a_verdict_that_reached_nobody_stays_answerable` — hoisting the removal back above the send turns it red. ## Not closing the issue `Refs #109`, deliberately not `Closes`. The four acceptance observations in #109 are live ones: a real gated call answered `no <id>` from Matrix and seen to be rejected, the same with `yes <id>`, a well-formed unissued id seen to be dropped, and a non-allowlisted sender's `yes <id>` seen to do nothing. They need the deployed server and a human at a phone, so they happen after Clark's release, not in this PR. A keyword that auto-closed on merge would leave the reopen as the step that gets forgotten. ## Scope `CHANNEL_TOOLS` and `CHANNEL_INSTRUCTIONS` are untouched (#111 is changing both). No release tag. `carried_of_live` is the one refactor beyond relay: `clippy::too_many_lines` fired on `push_message` once the verdict branch went in, and splitting the five-armed msgtype match out mirrors `carried_of` on the replay path. A test asserts the two classifiers agree. ## Gates `cargo fmt --all --check`, `cargo clippy --all-targets --all-features --locked -- -D warnings`, `cargo test --all-features --locked` (225 passed), `cargo audit`, `cargo deny check bans licenses sources` — all green on stable **1.98.0** with `RUSTFLAGS=-Dwarnings` and `OPUS_STATIC=1`. Rebased onto `7525066` (#114 merged), so `ci.yml` pins `1.98.0` and my local runs match the runner exactly. The tree cannot lint below 1.98.0 — that is #112, and AGENTS.md now carries the measured before/after table. `regex` is now a direct dependency, named for the verbatim pattern. It was already in the tree transitively, so the lockfile delta is one line and no new crate compiles. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01SHy74Zo4sLTBcEY4o5REXu --- ## Director's re-read, and three items applied in `02ff731` Gates reproduced independently on 1.98.0 before review: fmt clean, clippy 0 errors, 235 tests at that point. The four findings are closed and the `untrusted_body` trap found inside finding 1 is the best catch here — verified at `src/mcp.rs:1085-1091`, where the doc says the field is sandbox-wrapped, so an anchored pattern against it matches nothing and the fix would have looked right and done nothing. The declared coverage gap was verified rather than accepted: stubbing the `is_replayed_verdict` call in `replay_room` left all 235 tests green. Honest, not pessimistic. **1. Replay's judgement is now reachable.** `replay_deliverable` is lifted out of `replay_room`'s closure — allowlisted sender, not an already-answered verdict, a carried msgtype, within budget. Inside a closure none of it could be tested; `replay_room` keeps the homeserver round trip and nothing else. | Control | Before | After | |---|---|---| | verdict check disabled | green | **red** | | sender gate disabled | green | **red** | | budget ignored | green | **red** | `the_replay_filter_does_not_drop_everything` is the control for the control: a filter returning `None` for everything satisfies "a verdict is not replayed" while destroying the channel. `on_custom_notification` and `push_message` stay unreachable, and that is accepted — they need a live `Client`, `Room` and `Peer`, and inventing seams to reach them buys a test of the seam. Replay was different: the seam was already a closure and the `ReadEvent` fixtures already existed. **2. `prompt_room_is_usable` moved to the site its doc describes.** It documents "whether a permission prompt may be sent into a room in this state" and its only call site was `read_thread`, which has nothing to do with permission prompts, while the permission room used a raw `!= Joined`. Behaviourally identical, but the allowlist-of-one argument did not apply where it was written down, and `read_thread` was not this PR's to touch. Reverted there, applied at the permission site. One predicate across all five `RoomState::Joined` sites is worth doing separately and should be named for the property, not a caller. **3. A retirement declined by the serial is logged.** `retire_if_current` returning false is the case the serial exists for — a delivery in flight while the id was reissued. Declining is correct; being unable to observe it is not. ## Cross-engine review (Codex) - **ABA protection is complete** short of a `u64` wrap: comparison and removal happen under one write lock, a replacement before it carries a different serial, one after it survives. - **The `push_message` restructure pushes nothing new.** A verdict now reaches the peer when `live_peers` false-negatives to 0 — which is finding 4's fix, intended. Every non-verdict with `live == 0` hits the second return, and `remember_room` stays below it. ## Gates fmt, clippy `-D warnings` (0 errors), **240 tests** on rustc 1.98.0, rebased onto `e15a350`. ## Still not closing #109 Unchanged: the four acceptance observations need the deployed server and a human answering from a phone. `Refs`, not `Closes`.

jlxq0 added 1 commit 2026-08-25 07:11:44 +00:00

feat(channel): relay permission prompts through Matrix ... 23dc9555ee

A session on `/channel` could only have its tool-approval dialogs answered at
the terminal, so every gated Bash, Write or Edit stalled a fleet driven from a
phone until somebody reached a keyboard.

`/channel` now also declares `claude/channel/permission`. Claude Code forwards
each prompt as `notifications/claude/channel/permission_request`; we render it
into a Matrix message carrying the five-letter request id verbatim — the
terminal dialog never shows that id, so the message is the only place it can be
learned — and a reply of `yes <id>` or `no <id>` goes back as
`notifications/claude/channel/permission`. Both dialogs stay live and the first
answer wins.

The verdict branch sits behind the same sender allowlist as message delivery,
because anyone who can reply through the channel can approve tool use in the
session. The gate is an argument to `classify_inbound` rather than statement
order in the inbound handler, so "a stranger's `yes abcde` is ignored" is a
question a unit test asks directly instead of a property of where a `return`
sits. A verdict is answered once and never also forwarded as chat.

A permission request arrives on a peer carrying no room, so the destination is
`permission_room` from `app.matrix_mcp.channel` account data, and otherwise the
room an allowlisted sender last wrote in — recorded only after that gate
passes, or a stranger who DMs the bot redirects every later prompt into a room
he controls. Neither known is a logged drop: a guessed room hands the approval
to whoever is standing in it. Verdicts are routed to the session that asked,
keyed on (mxid, request id) and expiring after fifteen minutes, so one for an
id nobody issued is visibly dropped rather than broadcast.

`carried_of_live` splits the live msgtype match out of `push_message`, mirroring
`carried_of` on the replay path, with a test that the two agree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SHy74Zo4sLTBcEY4o5REXu

jlxq0 force-pushed permission-relay from 23dc9555ee to a6e9d74c63 2026-08-25 10:05:28 +00:00 Compare

jlxq0 force-pushed permission-relay from a6e9d74c63 to 02ff731d7b 2026-08-25 11:01:31 +00:00 Compare

jlxq0 merged commit 21d74338ad into main 2026-08-25 11:03:24 +00:00

jlxq0 referenced this pull request from a commit 2026-08-25 11:03:26 +00:00

Merge pull request 'feat(channel): relay permission prompts through Matrix' (#115) from permission-relay into main

jlxq0 referenced this pull request 2026-08-26 09:16:34 +00:00

Relay permission prompts through the Matrix channel #109

jlxq0 referenced this pull request 2026-08-26 09:20:56 +00:00

fix(channel): a reply carries what it answers, and a correction arrives as a correction #126

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

blocked

Held on a named, checkable blocker: an issue number, a credential or a measurement. Not "held".

  

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

waiting-on-julian

Blocked on Julian. The ask is in the issue.

  

wontfix

This will not be worked on

No labels

blocked

bug

documentation

duplicate

enhancement

good first issue

help wanted

invalid

question

waiting-on-julian

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jlxq0/matrix-mcp!115

No description provided.