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

Merged

jlxq0 merged 10 commits from agents-tag-cleanup into main 2026-08-26 15:55:08 +00:00

Conversation 1 Commits 10 Files changed 4 +2427 -41

jlxq0 commented 2026-08-26 09:20:56 +00:00

Owner

Copy link

Two commits. The fix is the second and the reason for the PR; the first is the pitfall from this morning's tag.

fix(channel): a correction now arrives as a correction — closes #125

An m.replace is an ordinary m.room.message, so a corrected message reached a session as a second, near-identical instruction carrying the * -prefixed fallback body with nothing marking it a replacement. Nothing absent, nothing malformed, no error anywhere, and a reader that acts twice on something its sender sent once. On replay both the original and its correction were carried, in order, as two separate messages.

The * fallback exists for clients that cannot render an edit and a session is not one, so m.new_content goes out instead and both paths mark the event with replaces="$id". Replay additionally sends only the newest version — the third option from #125's table, which pays neither of the other two costs: a session that never saw the original has nothing stale, and one that did can match the id and say which of its instructions was withdrawn.

Two judgements lifted out of functions a test cannot reach, for the reason replay_deliverable was lifted out of replay_room in v0.10.5. live_delivery was four lines inside push_message, which needs a live Room and Peer; replay_body_source was inside replay_body, whose only other ingredient is a Room. A judgement reachable only through a homeserver handle is one no test pins.

Verified by removal, one mutation at a time, each restored before the next, and each naming which test caught it:

Mutation	Reddens
keep the superseded original	3 tests
keep older edits	2 tests
> for >= on the timestamp tie	the tie test — and it would deliver the version he corrected
deliver the * fallback	an_edit_delivers_its_new_content_and_not_the_asterisk_fallback
classify by the outer msgtype	an_edit_is_classified_by_what_it_replaces_the_message_with
replaces_of always None	6 tests
live path delivers content.body	the live test
live path emits no replaces	the live test

The control for the controls is the_edit_filter_does_not_drop_unedited_messages: a supersession pass returning nothing satisfies every assertion about what must not be replayed, so an assertion that only checked for absence would pass against a filter that had destroyed the channel.

Decided rather than inherited: an edit whose original sits outside the replay window is delivered, marked, since the receipt already covers the original and dropping the edit would lose the correction entirely. A malformed edit with no usable m.new_content falls back to its own body rather than being dropped. Ties on origin_server_ts go to the later position in the batch, which arrives in timeline order — event id is not a tiebreak, being opaque, and sorting by it would pick a winner for a reason unrelated to which edit came second.

CHANNEL_INSTRUCTIONS now says what the attribute means, with a test that the two cannot drift apart silently. An attribute nobody was told about leaves a correction reading as a repeat exactly as before.

docs(agents): the tag annotation loses any line starting with #

v0.10.5's annotation shipped without #113 trap. And the live_peers(mxid) == 0 early return dropped verdicts for, so the release note breaks mid-clause and two of the four fixes it describes are unnamed. git tag -a defaults to --cleanup=strip, which removes commentary lines; #107 survived in the same annotation only because it sits mid-line. Measured with a control on a throwaway tag at the same sha: identical message, line kept under --cleanup=verbatim, gone under the default. The entry recommends reflowing over passing the flag, because a paragraph where no line starts with # survives the next person writing the tag command without it. The tag itself was left alone — matrix-mcp:v0.10.5 was already published at sha256:8c25c57e…, and force-pushing a ref an artefact points at is the worse trade.

It also carries the allowlist rule extended to reactions, ahead of #124: an m.reaction from anyone in the room reaches /relations, so once an emoji means "approve this tool call", approval is available to whoever is standing in the room.

Gates

fmt, clippy -D warnings, 252 passed; 0 failed, cargo audit clean, cargo deny check bans licenses sources ok. Toolchain 1.98.0.

Third commit: three ways the first fix dropped a message

Codex was asked one question about 646cb84 — is there any input for which it drops a message the old code delivered, other than an original a newer edit supersedes. It found three, and the suite was green through all of them.

An edit whose m.new_content is present but the wrong shape vanished. The classifier was selected by a shape check (msgtype and body both present), which {"msgtype": 7} satisfies; classification then failed and nothing was delivered, where the old code classified the outer content and delivered it. Now the classifier is asked rather than guessed at — .and_then(classify_content).or_else(|| classify_content(outer)) — which cannot disagree with the classifier because it is the classifier. Covered for a non-string msgtype, a non-string body, and an unsupported msgtype.

An edit naming itself, and two edits naming each other, vanished. Every id involved is both a winner and a superseded target, and the superseded check ran first, so the correction disappeared entirely: worse than the defect 646cb84 fixed. The winner check now runs first and keeps exactly one version of each. Malformed relations are the ordinary case for this, not an exotic one.

A replacement with no event_id was dropped once any other edit made the winner set non-empty, because a missing id read as "" and "" is nobody's winner. Invisible to a single-event test, which the early return covers. This pass now leaves such events alone; replay_room skips them for its own reasons and that is where the decision belongs.

Three more mutation controls, each reddening only its own test: the shape check restored, the two membership checks reordered, the missing id read as "".

Two AGENTS.md entries, both generalising past this file: a shape check that guesses what a parser accepts turns deliverable input into dropped input, and when two membership checks can both match one event their order decides whether it vanishes.

Gates re-run after: fmt, clippy -D warnings, 256 passed; 0 failed, audit clean, deny ok.

Fourth commit: the mirror question, and one more

Alan's suggestion, and it paid. The first review asked what this drops that the old code delivered. The mirror is what it now delivers that the old code correctly withheld, or delivers twice.

One found. carried_of consulted m.new_content on any event carrying the key, without first asking whether the event is a replacement. So an m.location — which the channel does not carry — with an m.text in m.new_content was classified as a message and went out carrying the outer body. Replay only: the live path reads a typed Relation and has no such key to be fooled by. So it is also the two paths disagreeing about one event, the #107 shape for the third time in this file.

The key is now read only when replaces_of says the event is a replacement, with the control being the same event with the relation added, which must still classify from the new content. Mutation: drop the relation check and the decoy test alone reddens.

No duplicate delivery found, and take(budget) after supersession changes which message occupies a slot without exceeding the budget.

Third AGENTS.md entry, the general rule rather than the instance: a field that only means something under a relation must be read only when that relation is present, and when adding a field to one path, ask what the other path reads instead of it.

Why this is not merged on green

Five gates were green when the first fix was pushed, and four defects have arrived since — three from one narrow question to the other engine, one from its mirror. Green is what makes it reviewable, not what makes it ready. 256 passed was also green through every one of the three drops.

Worth aiming a third question somewhere I have not: the live path has one test and its parent push_message still needs a live Room and Peer, so everything after live_delivery returns is unexercised.

Gates after the fourth commit: fmt, clippy -D warnings, 257 passed; 0 failed, audit clean, deny ok. Clippy caught a redundant_clone in the new test on the first run, which is why the whole set is re-run after every change rather than only the suite.

Fifth commit: #124's reply half — 775e073

Julian asked for this directly and it has cost him three times. He replied to a gate-chase message with "Who is this and what is this for?", the session guessed, answered a question he had not asked, and he had to screenshot his own client. Later he sent "Yes it is wrong. Fix it!" and the receiving session had to ask which thing, because there was no way to tell.

Both paths now emit in_reply_to with a truncated quotation beside it, plus in_reply_to_sender, in_reply_to_unresolved, in_reply_to_suspicious and in_reply_to_untrusted_sender. The instructions say what each means, with the same drift test as the edit work.

The relation the replay path already held is necessary and not sufficient. ReadEvent::in_reply_to is populated for a thread fallback as well, where m.in_reply_to points at the latest message in the thread for clients without threading. Reading that field alone would quote the wrong message every time somebody posted in a thread, and attribute a choice to the sender they did not make. Both paths read the relation and refuse when is_falling_back is set.

The quotation is escaped by the same functions as the primary body, and the test compares quote_excerpt's output against what content_sandbox::evaluate produces for the same input, so the two cannot drift. A hand-written expected string would agree with whichever implementation it was copied from. Suspicion runs on the whole body and the cut happens after, so a marker past the cut still raises the flag; the control is the same body with no marker in it.

Five mutations: thread fallback treated as a reply on each path, suspicion measured on the cut excerpt, the quotation left to attr_escape alone, an unresolvable reference omitting its attribute.

Two more from a cross-engine review, folded into the same commit

My sequencing error: these deserved their own commit and 775e073's message does not describe them. The code and the tests are there; the account is here.

A quotation could carry a blocked sender's prose. The quotable batch is built before the allowlist filter, so an allowlisted person replying to a blocked sender pulled that sender's words in, escaped and attributed but silently. Dropping it is wrong, because the ordinary case is a reply to our own message and refusing would make the case the feature exists for unresolvable. The excerpt travels and says who wrote it: in_reply_to_untrusted_sender="true" for anyone neither on the allowlist nor this identity.

A reply target fetched by id was never checked against the id asked for. The requested id stayed as the attribute while the sender and body came from whatever the homeserver returned, so a nonconforming response could attribute words to a message that never contained them — which is the whole point of quoting. The response is now compared to the request. The same class inside a batch is two events claiming one id, where the HashMap kept one and answered for the other; neither is quotable now.

Two more mutations: colliding ids keeping the last writer, and the untrusted marker dropped from the attributes.

Three AGENTS.md entries, all general: a quotation is escaped by the same functions as the body it quotes and the test compares against the body's own output; a lookup keyed on something the sender controls needs a collision policy, and so does a response fetched by id; and trust follows the route content came in by rather than the content.

Gates after: fmt, clippy -D warnings, 266 passed; 0 failed, audit clean, deny ok. Clippy caught a redundant_clone, a redundant_pub_crate and a hundred-line function on the way, none of which the suite would have.

Running count: nineteen self-mutations, zero defects. Three narrow questions to the other engine, six defects, every one of them surviving five green gates.

Sixth commit: read_reactions — 888cdaf

Unblocks mantis-32's permission gate. read_thread's shape with RelationType::Annotation, so the SDK path, the joined-room guard, the chunk decoding and the sandboxing are the ones already in use.

There is no bundled-aggregation route, and it changed the design in the right direction. The spec states m.annotation relationships are not aggregated by the server and are not in m.relations, so there is no count to read. /relations returns the annotation events themselves, carrying each one's sender and timestamp, which is what makes "only this person, and only after I asked" checkable at all. A count would have been easier to read and useless.

The key travels byte for byte: no NFC pass, no folding of variation selectors, no stripping of skin-tone modifiers, and no normalisation crate in the tree, so a consumer quoting the emoji back quotes the one that was tapped. Matching is the consumer's job and it can only strip modifiers while it still holds the original.

Redaction is deliberately undocumented, because it is unmeasured. The spec says a redacted child breaks the relation and m.relates_to lives in content, which redaction strips, so it should not come back. That is an argument rather than a measurement, and a contract asserted from the spec is one nobody checked. The live fixture runs next.

One mutation stayed green, and that was the finding

Deleting the rel_type == "m.annotation" check broke no test. Every entry in the control set was also refused by the missing key, so the guard was load-bearing and nothing measured it. m.relates_to is sender-controlled and accepts arbitrary fields, so {"rel_type": "m.thread", "event_id": "$t", "key": "👍"} is one message to write and would have been read as a thumbs-up — an approval, once an emoji means one. The fixture that pins a guard is the one that satisfies every other check and fails only this one.

Three more from a cross-engine review

A reaction on a different event was returned as one on this event. The response is fetched by event id and was checked against it nowhere, and the returned entry did not carry its target either, so a caller could not detect it. Now compared, and the target is in the result.

A single page could hide a present reaction permanently. /relations returns newest first, so a reaction older than one page is invisible to a single request and stays invisible however often the caller polls, since every poll starts at the same newest page. Pagination is followed to exhaustion or the cap, and hitting the cap sets truncated rather than looking like the end of the list.

Undecodable entries collapsed into "no reactions yet", contradicting this tool's own documented contract: only request failures were errors, while per-entry failures became empty success. A chunk holding only an undecryptable reaction returned reactions: []. Now counted as unreadable, and an empty list beside a non-zero unreadable is a third answer that a caller must treat as "cannot tell".

Three mutation controls for those. Two more AGENTS.md entries: a per-entry decoding failure that drops the entry turns "cannot tell" into "none", and a single page is not the answer to "is it there".

Gates: fmt, clippy -D warnings, 274 passed; 0 failed, audit clean, deny ok.

Running count: twenty-two self-mutations, one defect — and that one was a test pinning nothing rather than wrong code. Four narrow questions to the other engine, nine defects, every one past five green gates.

Two commits. The fix is the second and the reason for the PR; the first is the pitfall from this morning's tag. ## `fix(channel)`: a correction now arrives as a correction — closes #125 An `m.replace` is an ordinary `m.room.message`, so a corrected message reached a session as a second, near-identical instruction carrying the `* `-prefixed fallback body with nothing marking it a replacement. Nothing absent, nothing malformed, no error anywhere, and a reader that acts twice on something its sender sent once. On replay both the original and its correction were carried, in order, as two separate messages. The `* ` fallback exists for clients that cannot render an edit and a session is not one, so `m.new_content` goes out instead and both paths mark the event with `replaces="$id"`. Replay additionally sends only the newest version — the third option from #125's table, which pays neither of the other two costs: a session that never saw the original has nothing stale, and one that did can match the id and say which of its instructions was withdrawn. **Two judgements lifted out of functions a test cannot reach**, for the reason `replay_deliverable` was lifted out of `replay_room` in `v0.10.5`. `live_delivery` was four lines inside `push_message`, which needs a live `Room` and `Peer`; `replay_body_source` was inside `replay_body`, whose only other ingredient is a `Room`. A judgement reachable only through a homeserver handle is one no test pins. **Verified by removal, one mutation at a time, each restored before the next**, and each naming which test caught it: | Mutation | Reddens | |---|---| | keep the superseded original | 3 tests | | keep older edits | 2 tests | | `>` for `>=` on the timestamp tie | the tie test — and it would deliver the version he corrected | | deliver the `* ` fallback | `an_edit_delivers_its_new_content_and_not_the_asterisk_fallback` | | classify by the outer msgtype | `an_edit_is_classified_by_what_it_replaces_the_message_with` | | `replaces_of` always `None` | 6 tests | | live path delivers `content.body` | the live test | | live path emits no `replaces` | the live test | The control for the controls is `the_edit_filter_does_not_drop_unedited_messages`: a supersession pass returning nothing satisfies every assertion about what must not be replayed, so an assertion that only checked for absence would pass against a filter that had destroyed the channel. Decided rather than inherited: an edit whose original sits outside the replay window is delivered, marked, since the receipt already covers the original and dropping the edit would lose the correction entirely. A malformed edit with no usable `m.new_content` falls back to its own body rather than being dropped. Ties on `origin_server_ts` go to the later position in the batch, which arrives in timeline order — event id is not a tiebreak, being opaque, and sorting by it would pick a winner for a reason unrelated to which edit came second. `CHANNEL_INSTRUCTIONS` now says what the attribute means, with a test that the two cannot drift apart silently. An attribute nobody was told about leaves a correction reading as a repeat exactly as before. ## `docs(agents)`: the tag annotation loses any line starting with `#` `v0.10.5`'s annotation shipped without `#113 trap. And the live_peers(mxid) == 0 early return dropped verdicts for`, so the release note breaks mid-clause and two of the four fixes it describes are unnamed. `git tag -a` defaults to `--cleanup=strip`, which removes commentary lines; `#107` survived in the same annotation only because it sits mid-line. Measured with a control on a throwaway tag at the same sha: identical message, line kept under `--cleanup=verbatim`, gone under the default. The entry recommends reflowing over passing the flag, because a paragraph where no line starts with `#` survives the next person writing the tag command without it. The tag itself was left alone — `matrix-mcp:v0.10.5` was already published at `sha256:8c25c57e…`, and force-pushing a ref an artefact points at is the worse trade. It also carries the allowlist rule extended to reactions, ahead of #124: an `m.reaction` from anyone in the room reaches `/relations`, so once an emoji means "approve this tool call", approval is available to whoever is standing in the room. ## Gates fmt, clippy `-D warnings`, `252 passed; 0 failed`, `cargo audit` clean, `cargo deny check bans licenses sources` ok. Toolchain 1.98.0. --- ## Third commit: three ways the first fix dropped a message Codex was asked one question about `646cb84` — is there any input for which it drops a message the old code delivered, other than an original a newer edit supersedes. It found three, and **the suite was green through all of them**. **An edit whose `m.new_content` is present but the wrong shape vanished.** The classifier was selected by a shape check (`msgtype` and `body` both *present*), which `{"msgtype": 7}` satisfies; classification then failed and nothing was delivered, where the old code classified the outer content and delivered it. Now the classifier is asked rather than guessed at — `.and_then(classify_content).or_else(|| classify_content(outer))` — which cannot disagree with the classifier because it is the classifier. Covered for a non-string `msgtype`, a non-string `body`, and an unsupported `msgtype`. **An edit naming itself, and two edits naming each other, vanished.** Every id involved is both a winner and a superseded target, and the superseded check ran first, so the correction disappeared entirely: worse than the defect `646cb84` fixed. The winner check now runs first and keeps exactly one version of each. Malformed relations are the ordinary case for this, not an exotic one. **A replacement with no `event_id` was dropped** once any other edit made the winner set non-empty, because a missing id read as `""` and `""` is nobody's winner. Invisible to a single-event test, which the early return covers. This pass now leaves such events alone; `replay_room` skips them for its own reasons and that is where the decision belongs. Three more mutation controls, each reddening only its own test: the shape check restored, the two membership checks reordered, the missing id read as `""`. Two `AGENTS.md` entries, both generalising past this file: a shape check that guesses what a parser accepts turns deliverable input into dropped input, and when two membership checks can both match one event their order decides whether it vanishes. Gates re-run after: fmt, clippy `-D warnings`, `256 passed; 0 failed`, audit clean, deny ok. --- ## Fourth commit: the mirror question, and one more Alan's suggestion, and it paid. The first review asked what this drops that the old code delivered. The mirror is what it now delivers that the old code correctly withheld, or delivers twice. **One found.** `carried_of` consulted `m.new_content` on any event carrying the key, without first asking whether the event is a replacement. So an `m.location` — which the channel does not carry — with an `m.text` in `m.new_content` was classified as a message and went out carrying the *outer* body. Replay only: the live path reads a typed `Relation` and has no such key to be fooled by. So it is also the two paths disagreeing about one event, the #107 shape for the third time in this file. The key is now read only when `replaces_of` says the event is a replacement, with the control being the same event with the relation added, which must still classify from the new content. Mutation: drop the relation check and the decoy test alone reddens. No duplicate delivery found, and `take(budget)` after supersession changes which message occupies a slot without exceeding the budget. Third `AGENTS.md` entry, the general rule rather than the instance: a field that only means something under a relation must be read only when that relation is present, and when adding a field to one path, ask what the other path reads instead of it. ## Why this is not merged on green Five gates were green when the first fix was pushed, and four defects have arrived since — three from one narrow question to the other engine, one from its mirror. Green is what makes it *reviewable*, not what makes it ready. `256 passed` was also green through every one of the three drops. Worth aiming a third question somewhere I have not: the live path has one test and its parent `push_message` still needs a live `Room` and `Peer`, so everything after `live_delivery` returns is unexercised. Gates after the fourth commit: fmt, clippy `-D warnings`, `257 passed; 0 failed`, audit clean, deny ok. Clippy caught a `redundant_clone` in the new test on the first run, which is why the whole set is re-run after every change rather than only the suite. --- ## Fifth commit: `#124`'s reply half — `775e073` Julian asked for this directly and it has cost him three times. He replied to a gate-chase message with *"Who is this and what is this for?"*, the session guessed, answered a question he had not asked, and he had to screenshot his own client. Later he sent *"Yes it is wrong. Fix it!"* and the receiving session had to ask which thing, because there was no way to tell. Both paths now emit `in_reply_to` with a truncated quotation beside it, plus `in_reply_to_sender`, `in_reply_to_unresolved`, `in_reply_to_suspicious` and `in_reply_to_untrusted_sender`. The instructions say what each means, with the same drift test as the edit work. **The relation the replay path already held is necessary and not sufficient.** `ReadEvent::in_reply_to` is populated for a **thread fallback** as well, where `m.in_reply_to` points at the latest message in the thread for clients without threading. Reading that field alone would quote the wrong message every time somebody posted in a thread, and attribute a choice to the sender they did not make. Both paths read the relation and refuse when `is_falling_back` is set. **The quotation is escaped by the same functions as the primary body**, and the test compares `quote_excerpt`'s output against what `content_sandbox::evaluate` produces for the same input, so the two cannot drift. A hand-written expected string would agree with whichever implementation it was copied from. Suspicion runs on the whole body and the cut happens after, so a marker past the cut still raises the flag; the control is the same body with no marker in it. Five mutations: thread fallback treated as a reply on each path, suspicion measured on the cut excerpt, the quotation left to `attr_escape` alone, an unresolvable reference omitting its attribute. ### Two more from a cross-engine review, folded into the same commit My sequencing error: these deserved their own commit and `775e073`'s message does not describe them. The code and the tests are there; the account is here. **A quotation could carry a blocked sender's prose.** The quotable batch is built before the allowlist filter, so an allowlisted person replying to a blocked sender pulled that sender's words in, escaped and attributed but silently. Dropping it is wrong, because the ordinary case is a reply to *our own* message and refusing would make the case the feature exists for unresolvable. The excerpt travels and says who wrote it: `in_reply_to_untrusted_sender="true"` for anyone neither on the allowlist nor this identity. **A reply target fetched by id was never checked against the id asked for.** The requested id stayed as the attribute while the sender and body came from whatever the homeserver returned, so a nonconforming response could attribute words to a message that never contained them — which is the whole point of quoting. The response is now compared to the request. The same class inside a batch is two events claiming one id, where the `HashMap` kept one and answered for the other; neither is quotable now. Two more mutations: colliding ids keeping the last writer, and the untrusted marker dropped from the attributes. Three `AGENTS.md` entries, all general: a quotation is escaped by the same functions as the body it quotes and the test compares against the body's own output; a lookup keyed on something the sender controls needs a collision policy, and so does a response fetched by id; and trust follows the route content came in by rather than the content. Gates after: fmt, clippy `-D warnings`, `266 passed; 0 failed`, audit clean, deny ok. Clippy caught a `redundant_clone`, a `redundant_pub_crate` and a hundred-line function on the way, none of which the suite would have. **Running count: nineteen self-mutations, zero defects. Three narrow questions to the other engine, six defects, every one of them surviving five green gates.** --- ## Sixth commit: `read_reactions` — `888cdaf` Unblocks `mantis-32`'s permission gate. `read_thread`'s shape with `RelationType::Annotation`, so the SDK path, the joined-room guard, the chunk decoding and the sandboxing are the ones already in use. **There is no bundled-aggregation route, and it changed the design in the right direction.** The spec states `m.annotation` relationships are not aggregated by the server and are not in `m.relations`, so there is no count to read. `/relations` returns the annotation events themselves, carrying each one's sender and timestamp, which is what makes "only this person, and only after I asked" checkable at all. A count would have been easier to read and useless. **The key travels byte for byte**: no NFC pass, no folding of variation selectors, no stripping of skin-tone modifiers, and no normalisation crate in the tree, so a consumer quoting the emoji back quotes the one that was tapped. Matching is the consumer's job and it can only strip modifiers while it still holds the original. **Redaction is deliberately undocumented, because it is unmeasured.** The spec says a redacted child breaks the relation and `m.relates_to` lives in `content`, which redaction strips, so it should not come back. That is an argument rather than a measurement, and a contract asserted from the spec is one nobody checked. The live fixture runs next. ### One mutation stayed green, and that was the finding Deleting the `rel_type == "m.annotation"` check broke no test. Every entry in the control set was **also** refused by the missing `key`, so the guard was load-bearing and nothing measured it. `m.relates_to` is sender-controlled and accepts arbitrary fields, so `{"rel_type": "m.thread", "event_id": "$t", "key": "👍"}` is one message to write and would have been read as a thumbs-up — an approval, once an emoji means one. **The fixture that pins a guard is the one that satisfies every other check and fails only this one.** ### Three more from a cross-engine review **A reaction on a different event was returned as one on this event.** The response is fetched *by* event id and was checked against it nowhere, and the returned entry did not carry its target either, so a caller could not detect it. Now compared, and the target is in the result. **A single page could hide a present reaction permanently.** `/relations` returns newest first, so a reaction older than one page is invisible to a single request and stays invisible however often the caller polls, since every poll starts at the same newest page. Pagination is followed to exhaustion or the cap, and hitting the cap sets `truncated` rather than looking like the end of the list. **Undecodable entries collapsed into "no reactions yet"**, contradicting this tool's own documented contract: only request failures were errors, while per-entry failures became empty success. A chunk holding only an undecryptable reaction returned `reactions: []`. Now counted as `unreadable`, and an empty list beside a non-zero `unreadable` is a third answer that a caller must treat as "cannot tell". Three mutation controls for those. Two more `AGENTS.md` entries: a per-entry decoding failure that drops the entry turns "cannot tell" into "none", and a single page is not the answer to "is it there". Gates: fmt, clippy `-D warnings`, `274 passed; 0 failed`, audit clean, deny ok. **Running count: twenty-two self-mutations, one defect — and that one was a test pinning nothing rather than wrong code. Four narrow questions to the other engine, nine defects, every one past five green gates.**

jlxq0 added 1 commit 2026-08-26 09:20:56 +00:00

docs(agents): the tag annotation loses any line starting with a hash ... 39a5ea0fbc

v0.10.5's annotation shipped without the line naming the 113 trap and the
live_peers early return, so the release note breaks mid-clause and two of
the four fixes it describes are unnamed in it. git tag -a defaults to
--cleanup=strip, which removes commentary lines, and the paragraph happened
to wrap so that a bare issue reference started one. The 107 reference in the
same annotation survived because it sits mid-line. Nothing in git's output
says a line was dropped and the tag was on the remote before anyone read it
back.

Measured with a control on a throwaway tag at the same sha, the flag being
the whole difference: identical message, line kept under --cleanup=verbatim
and gone under the default. The pitfall recommends the reflow over the flag,
since a paragraph where no line starts with a hash survives the next person
writing the tag command without it.

The second entry carries the allowlist rule forward to reactions, which is
the next inbound surface: an annotation from anyone in the room reaches
/relations, so once an emoji means approve, approval is available to whoever
is standing in the room. Written before the code so it is read before the
code, which is the argument for the file.

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

jlxq0 added 1 commit 2026-08-26 09:39:39 +00:00

fix(channel): deliver a correction as a correction, not as a second message ... 646cb84ad0

An m.replace is an ordinary m.room.message, so a corrected message reached a
session as a second, near-identical instruction carrying the asterisk-prefixed
fallback body, with nothing marking it a replacement. Nothing absent, nothing
malformed, no error anywhere, and a reader that acts twice on something its
sender sent once. On replay both the original and its correction were carried,
in order, as two separate messages.

The fallback exists for clients that cannot render an edit and a session is
not one, so the replacement's own m.new_content goes out instead, and both
paths mark the event with the id it replaces. Replay additionally sends only
the newest version of an edited message: sending both is the defect, and
sending only the latest unmarked costs a session that already saw the original
its only clue about which of its instructions was withdrawn. The mark is what
makes the third option cheaper than either.

Two judgements are lifted out of functions a test cannot reach, for the reason
replay_deliverable was lifted out of replay_room in the last release.
live_delivery is the live path's choice between an edit's new content and its
fallback, previously four lines inside push_message, which needs a live Room
and Peer. replay_body_source is the same choice on the replay path, previously
inside replay_body, whose only other ingredient is a Room. Both are now free
functions with controls.

Verified by removal, one mutation at a time, each restored before the next.
Keeping the superseded original reddens three tests; keeping older edits
reddens two; a greater-than in place of the greater-or-equal on the timestamp
tie reddens the tie test alone, and would have delivered the version he
corrected. Delivering the asterisk fallback, classifying by the outer msgtype,
and making replaces_of always return None each redden their own. On the live
side, delivering content.body rather than new_content, and emitting no
replaces, both redden the live test. The control for the controls is
the_edit_filter_does_not_drop_unedited_messages: a supersession pass that
returned nothing would satisfy every assertion about what must not be
replayed.

An edit whose original sits outside the replay window is delivered rather than
dropped, since the receipt already covers the original and dropping the edit
would lose the correction entirely. A malformed edit with no usable
m.new_content falls back to its own body rather than being dropped. Ties on
origin_server_ts go to the later position in the batch, which arrives in
timeline order; event id is not a tiebreak, being opaque.

CHANNEL_INSTRUCTIONS now say what the attribute means, with a test that the
two cannot drift apart silently: an attribute nobody was told about leaves a
correction reading as a repeat exactly as before.

Closes #125.

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

jlxq0 changed title from docs(agents): the tag annotation loses any line starting with a hash to fix(channel): deliver a correction as a correction, not as a second message 2026-08-26 09:40:03 +00:00

jlxq0 added 1 commit 2026-08-26 09:44:10 +00:00

fix(channel): three ways the edit fix dropped a message, found in review ... 0a7e4aad93

Codex was asked one question about the previous commit: is there any input for
which it drops a message the old code delivered, other than an original a
newer edit supersedes. Three, all of which the suite was green through.

An edit whose m.new_content is present but the wrong shape was dropped
entirely. The classifier was chosen by a shape check -- both msgtype and body
present -- which a msgtype of 7 satisfies, after which classification failed
and nothing was delivered. The old code classified the outer content and
delivered it. Now the classifier is asked rather than guessed at, falling back
to the outer content on None, which cannot disagree with the classifier
because it is the classifier. A non-string body and an unsupported msgtype are
the same shape and the test covers all three.

An edit naming itself, and two edits naming each other, vanished. Every id
involved is both a winner and a superseded target, and the superseded check
ran first, so the correction disappeared entirely -- worse than the defect the
previous commit fixed. The winner check now runs first, which keeps exactly
one version of each. Malformed relations are the ordinary case for this.

A replacement with no event_id was dropped once any other edit made the winner
set non-empty, because a missing id was read as the empty string and the empty
string is nobody's winner. It was invisible to a single-event test, which the
early return covers. This pass now leaves such events alone; replay_room skips
them for its own reasons and that is where the decision belongs.

Each fix has a mutation control: the shape check restored, the two membership
checks reordered, and the missing id read as the empty string. All three
redden their own tests and nothing else.

Two entries in AGENTS.md, both generalising past this file: a shape check that
guesses what a parser accepts turns deliverable input into dropped input, and
when two membership checks can both match one event their order decides
whether it vanishes.

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

jlxq0 added 1 commit 2026-08-26 09:48:36 +00:00

fix(channel): m.new_content on a non-replacement is a decoy ... e7d7b7fb72

The mirror of the question that found the last three: does anything now get
delivered that the old code correctly withheld. One does.

carried_of consulted m.new_content on any event carrying the key, without
first asking whether the event is a replacement. So an m.location, which the
channel does not carry, with an m.text in m.new_content, classified as a
message and went out carrying the outer body. Replay only -- the live path
reads a typed Relation and has no such key to be fooled by -- so it is also
the two paths disagreeing about one event, which is the #107 shape for the
third time in this file.

The key is now read only when replaces_of says the event is a replacement.
The control is the same event with the relation added, which must still be
classified from the new content.

Mutation: drop the relation check and the decoy test alone reddens.

The AGENTS.md entry is the general rule rather than the instance: a field that
only means something under a relation must be read only when that relation is
present, and when adding a field to one path, ask what the other path reads
instead of it.

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

jlxq0 commented 2026-08-26 10:06:23 +00:00

Author

Owner

Copy link

CI on e7d7b7f: cargo success, docker skipped. Development is paused for the evening; this is the run finishing and nothing after it.

17387  cargo   success   09:48:37Z → 09:50:02Z   1m25s
17388  docker  skipped   09:50:05Z → 09:50:06Z   1s

The docker tick on this PR is a skip, and its green carries no information about the image. .forgejo/workflows/ci.yml:76 reads if: github.event_name != 'pull_request', so the job cannot run here; Forgejo still reports CI / docker (pull_request) as success. Every one of this branch's four pushes shows the same pair, and so does #123 before it. A build of this change has therefore not happened anywhere and will not until it reaches main.

Two things about reading it, since the shape is easy to misread:

• The combined status endpoint reports success for a job that never executed. /actions/tasks is where skipped is visible, which is another instance of the rule that the status tick and the task are different questions.
• head_branch on a pull-request task is the PR number (#126), not the branch name. A predicate matching head_branch == "agents-tag-cleanup" finds nothing, always, for every PR — the same silent-empty failure as matching a tag on /actions/runs.

Not merging tonight. The reason is on the PR above and it is not the pause: five gates were green when the first fix was pushed and four defects have arrived since, three from one narrow question to the other engine and one from its mirror. Green is what makes this reviewable rather than ready, and docker never having built it is now a second reason.

Both worktrees are clean, nothing is half-done, and #124 is not started.

**CI on `e7d7b7f`: `cargo` success, `docker` skipped.** Development is paused for the evening; this is the run finishing and nothing after it. 17387 cargo success 09:48:37Z → 09:50:02Z 1m25s 17388 docker skipped 09:50:05Z → 09:50:06Z 1s **The `docker` tick on this PR is a skip, and its green carries no information about the image.** `.forgejo/workflows/ci.yml:76` reads `if: github.event_name != 'pull_request'`, so the job cannot run here; Forgejo still reports `CI / docker (pull_request)` as `success`. Every one of this branch's four pushes shows the same pair, and so does #123 before it. A build of this change has therefore not happened anywhere and will not until it reaches `main`. Two things about reading it, since the shape is easy to misread: - The combined status endpoint reports `success` for a job that never executed. `/actions/tasks` is where `skipped` is visible, which is another instance of the rule that the status tick and the task are different questions. - `head_branch` on a pull-request task is the **PR number** (`#126`), not the branch name. A predicate matching `head_branch == "agents-tag-cleanup"` finds nothing, always, for every PR — the same silent-empty failure as matching a tag on `/actions/runs`. **Not merging tonight.** The reason is on the PR above and it is not the pause: five gates were green when the first fix was pushed and four defects have arrived since, three from one narrow question to the other engine and one from its mirror. Green is what makes this reviewable rather than ready, and `docker` never having built it is now a second reason. Both worktrees are clean, nothing is half-done, and #124 is not started.

jlxq0 added 1 commit 2026-08-26 15:11:38 +00:00

feat(channel): carry what a reply refers to, and quote it ... 775e073f56

Julian replied to a gate-chase message with "Who is this and what is this
for?" and the session guessed, answered a question he had not asked, and he
had to screenshot his own client to show what he meant. Later he sent "Yes it
is wrong. Fix it!" and the receiving session had to ask which thing, because
there was no way to tell. The push carried room, sender and event and nothing
about what the message was answering.

Both paths now emit in_reply_to, and a truncated quotation beside it. The id
alone would be a round trip per message, and a fix that depends on somebody
remembering to take that round trip works when it is not needed.

The replay path already held the relation populated and discarded it, but the
field it held is necessary and not sufficient: ReadEvent::in_reply_to is
populated for a thread fallback too, where m.in_reply_to points at the latest
message in the thread for clients without threading. Quoting that would
attribute a choice to the sender they did not make, so both paths read the
relation and refuse when is_falling_back is set.

The quotation is escaped by the same functions as the primary body rather than
by an escaper written beside it, and the test compares its output against what
content_sandbox::evaluate produces for the same input, so the two cannot
drift. Suspicion runs on the whole body and the cut happens afterwards, so a
marker past the cut still raises the flag; the control is the same body with
no marker in it.

An unresolvable reference emits the id with an explicit marker rather than
omitting the attribute, because a missing attribute reads as "not a reply",
which is the failure being fixed. A reference that resolves to a message with
no prose, an uncaptioned upload, is resolved with no excerpt, which is a third
and different fact.

Replay quotes from the batch it already fetched, built before the watermark
cut because the message being replied to is usually one the session has
already acknowledged, which is exactly the half the cut removes.

Five mutation controls: the thread fallback treated as a reply on each path,
suspicion measured on the cut excerpt, the quotation left to attr_escape
alone, and an unresolvable reference omitting its attribute.

CHANNEL_INSTRUCTIONS say what every new attribute means, with the same test as
the last commit that they cannot drift apart from the code that emits them.

Refs #124.

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

jlxq0 changed title from fix(channel): deliver a correction as a correction, not as a second message to fix(channel): a reply carries what it answers, and a correction arrives as a correction 2026-08-26 15:12:07 +00:00

jlxq0 added 1 commit 2026-08-26 15:28:37 +00:00

feat(mcp): read the reactions on an event ... 888cdaf64f

The write direction has existed since v0.9; the read direction did not, so a
caller could put an emoji on a message and never find out whether anyone
answered. mantis-32's permission gate is blocked on it: Julian wants to
approve a tool call by tapping an emoji on the gate message from his phone
rather than typing an id.

read_reactions is read_thread's shape with RelationType::Annotation, so the
SDK path, the joined-room guard, the chunk decoding and the sandboxing are all
the ones already in use.

There is no bundled-aggregation route and that changed the design in the right
direction. The spec states that m.annotation relationships are not aggregated
by the server and are not in the m.relations property, so there is no count to
read, and /relations returns the annotation events themselves carrying the
sender and the timestamp of each. "Only this person, and only after I asked"
is checkable because of the route we are forced onto; a count would have been
easier to read and useless.

The key travels byte for byte. No NFC pass, no folding of variation selectors,
no stripping of skin-tone modifiers, and no normalisation crate in the tree, so
a consumer quoting the emoji back quotes the one that was tapped. Matching is
the consumer's job and it can only strip modifiers while it still holds the
original.

Nothing is cached and nothing is aggregated locally, so a poll reflects what
the server holds at the moment of the poll. A consumer that waits before
deciding is waiting on something real.

What a redacted annotation does is deliberately not documented, because it is
not measured. The spec says a redacted child breaks the relation and the
server must disassociate it, and m.relates_to lives in content which redaction
strips, so it should not come back. That is an argument rather than a
measurement, and a contract asserted from the spec is one nobody checked. The
fixture runs against a live homeserver next.

One mutation stayed green and that is the finding: deleting the rel_type check
broke no test, because every entry in the control set was also refused by the
missing key. m.relates_to is sender-controlled and takes arbitrary fields, so
a thread relation carrying a key is one message to write and would have been
read as a thumbs-up. The fixture that pins a guard is the one that satisfies
every other check and fails only this one.

Refs #124.

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

jlxq0 added 1 commit 2026-08-26 15:38:17 +00:00

fix(channel): a reply cannot quote what the channel refuses to deliver ... 5d82765ccc

The mirror question on the finished branch, asked of the engine that did not
write it: does anything now deliver or quote something origin/main correctly
withheld. One does.

A quotation is a second route to the same content, and it was escaped like a
body but selected like nothing. quotable_from took raw_body off any event with
an id and a sender, so an allowlisted person replying to a permission verdict
pulled a bare "no qmzkd" into a fresh context, which is the thing replay drops
on purpose and the #107 bug in another costume. The same held for any event
type the channel does not carry, whose content.body the sender chooses, and
for an uncaptioned upload whose body is a filename rather than words.

quotable_body applies the delivery path's own judgement: is_replayed_verdict,
then carried_of, quoting a caption rather than a filename. A withheld event
still resolves and still names its sender, because the session needs to know
its reply answers something and who wrote it; what it does not get is a body
it was never owed. Reporting it as unresolvable would say we could not find
it, which is untrue and sends the session looking.

That is the third time in this file that two paths have disagreed about one
event, and I introduced it with the rule written in a doc comment on the
function above.

One mutation stayed green: making the fetch fallback quote raw_body broke
nothing, because everything above it in resolve_reply is a homeserver round
trip and no test could reach it. That is the judgement being in the wrong
place rather than the test being weak. reply_ref_from is now a free function
over a decoded event, which also pinned the requested-id check and the
undecryptable check, neither of which had a test either.

Six mutation controls across the two: the verdict check, the carried_of
classification, the fetch path's body, the requested-id comparison, the
decryption check, and quoting through the batch.

Two AGENTS.md entries: a second route to the same content needs the first
route's judgement rather than only its escaping, and a mutation that stays
green on a path needing a live Room is telling you where the judgement is
rather than that it is safe.

Refs #124.

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

jlxq0 added 1 commit 2026-08-26 15:41:39 +00:00

fix(channel): quote the correction, not the asterisk fallback ... 7692304fdc

Composition fault between the two features on this branch, found by a review
scoped to the commit that introduced the second. carried_of classifies a
replacement by m.new_content while quotable_body took content.body, so each
was right alone and together they were wrong three ways.

The excerpt for an edited message was the asterisk-prefixed fallback rather
than the text its sender meant.

Suspicion was measured on the fallback, because quote_excerpt runs on whatever
quotable_body returns, so an edit whose correction tripped the injection
heuristic was quoted with no flag.

And the verdict check ran against the fallback too, so an ordinary correction
to a message that had been "no qmzkd" was withheld entirely while the text
actually written was harmless. is_replayed_verdict reads content.body by
design, which is right where the fallback is what would be delivered and wrong
where it is not.

quotable_body now picks its body through replay_body_source, which is the same
choice the delivery path makes, and classifies the verdict against that text.
The control for that last one is an edit whose new text is a verdict, which
stays withheld.

Three mutation controls: the body source forced to AsRead, the verdict check
restored to is_replayed_verdict, and the verdict check removed entirely.

The AGENTS.md entry is the general shape: two correct features compose into a
wrong one when they disagree about which field is the message, so pick the
body once and run every later check against that.

Refs #124, #125.

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

jlxq0 added 1 commit 2026-08-26 15:47:55 +00:00

feat(channel): push reactions, on both paths ... f1f6e23dc6

An m.reaction is not an m.room.message, so the message handler never saw one
and a reaction reached a session as nothing at all. That is not a degraded
delivery: it leaves no trace of having been sent, so nobody can notice it is
missing, which is why the gap survived until somebody went looking for the
read direction.

A second event handler pushes each annotation with the reacting user, the
emoji key and the event it annotates. Behind the same sender allowlist as
message delivery, and for a sharper reason: an emoji on a permission prompt is
an answer to it, so anyone whose reactions reach a session can approve tool use
in it. The key travels as an escaped attribute rather than as a sentence, the
same treatment a filename gets and for the same reason, and the body is empty
so a reaction has the shape of every other channel event.

Replay carries them too. carried_of required a msgtype, which a reaction does
not have, so without this a reaction arriving while nothing was attached would
be lost while the live path delivered it. That is the two paths disagreeing
about one event, which is the fault this file has produced three times, and it
would have been introduced knowingly.

The room is deliberately not remembered on the reaction path. That memory
decides where a permission prompt goes when account data names no room, and a
reaction is a weaker signal of "this is where the operator talks to me" than a
message is.

A relation that is not an m.annotation is not a reaction even when it carries a
key. m.relates_to is sender-controlled and takes arbitrary fields, so a thread
relation with a key on it is one message to write, and read as a reaction it
would be an approval once an emoji means one. Same check as
reactions_from_chunk, same reason.

One mutation stayed green and it was the meta assembly rather than the code:
deleting the reaction attributes from the replay push broke nothing, because
the test asserted the helper that produces them instead of the assembly that
uses them, and the assembly lived inside a function needing a live Room.
replay_meta and live_reaction_meta are free functions now, and the test that
matters compares the two paths' spelling of the same attribute rather than
either alone.

Six mutation controls: the annotation rel_type check, the classifier's call to
it, and the attributes dropped from each of the two pushes.

Refs #124.

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

jlxq0 added 1 commit 2026-08-26 15:53:09 +00:00

fix(channel): three gaps in the reaction push, found in review ... 3409c72927

One narrow question to the engine that did not write it, weighted at the
newest code on the branch: does anything now deliver, quote or count something
origin/main correctly withheld.

The key was unbounded. MAX_PUSH_BYTES and cap_wrapped bound the body and touch
nothing in the meta, and build_params escapes meta values without limiting
them, so a sender-chosen key of sixty kilobytes reached a model's context in
full on both paths, past a cap that only ever looked at a body which is empty
for a reaction. carried_reaction now clips it on a character boundary, in one
place, so the two paths cannot cap differently, and the control is that an
ordinary emoji including a skin-tone modifier passes through unaltered.

push_reaction was missing two of push_message's gates: the RoomState::Joined
check, so a reaction in a room this identity had left would have been
delivered, and the self-sender suppression, so an agent could read its own
emoji back as inbound context. Nothing about a missing guard shows up in a
diff of the new function.

And annotates is the sender's claim about what they reacted to rather than a
verified fact, since verifying it costs a fetch per reaction. The instructions
now say so, so a session that needs to know which message it was reads that
event rather than assuming.

Two AGENTS.md entries: a cap on the body is not a cap on the attributes, and a
new push path must apply every gate the existing one applies because the
omission is invisible in its own diff.

Refs #124.

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

jlxq0 merged commit 92173daab0 into main 2026-08-26 15:55:08 +00:00

jlxq0 deleted branch agents-tag-cleanup 2026-08-26 15:55:09 +00:00

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

Merge pull request 'fix(channel): a reply carries what it answers, and a correction arrives as a correction' (#126) from agents-tag-cleanup into main

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!126

No description provided.