From 298e7790c46ee10e91ca34f60b9937bbb159a4ea Mon Sep 17 00:00:00 2001 From: Heimdall Date: Wed, 11 Nov 2026 16:13:37 +0000 Subject: [PATCH] Mouth: a failed re-read keeps the earlier hit only on transient failures Co-Authored-By: Claude Sonnet 5.5 --- docs/bridge.md | 2 +- tests/persona-matrix-mouth.test.mjs | 15 +++++++++++++-- tools/persona-matrix-mouth.mjs | 7 ++++--- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/docs/bridge.md b/docs/bridge.md index ed7ec4f..4ad5cd9 100644 --- a/docs/bridge.md +++ b/docs/bridge.md @@ -263,7 +263,7 @@ The answer is `[persona, deliveries, tail, tail_gaps]` exactly as v3; each deliv |---|---|---| | 14 `reply_to` | nil or `[event_id, sender, excerpt]` | The event this message replies to (its own `m.in_reply_to`; a thread's `is_falling_back` pointer names no one and is nil). `event_id` the quoted event; `sender` its author's Matrix id, read from the homeserver as her account; `excerpt` nil, or its text. | -`excerpt` is nil, with `event_id` and `sender` kept, when the quoted event is redacted (`unsigned.redacted_because` present or no content), is not a text-bearing `m.room.message`, is **edited** (it is itself an `m.replace`, or the homeserver bundles an `m.replace` in its `unsigned.m.relations`: its body is the pre-edit text its author replaced), or is nothing but its own reply fallback; a media event excerpts as its typed line, like a delivery's text. Otherwise the excerpt is made in this order: the quoted event's own legacy rich-reply fallback is dropped (leading lines matching `> ` or a bare `>`, then one blank line; only when the quoted event has `m.relates_to.m.in_reply_to`); the text is held to the 1000-byte line bound; runs of CR, LF, tab, U+2028 and U+2029 become one space; control characters (C0, DEL, C1 including NUL and ESC), format characters (Unicode Cf: bidi marks, overrides and isolates, U+061C, U+200B-200F, U+2060-2064, soft hyphen, BOM), private-use characters (Co), the tag block U+E0000-E007F and unpaired surrogates are removed; the line is trimmed; then the first 240 **Unicode code points** are kept (cut on a code point so no character is split; a combining mark counts as its own code point; trailing space trimmed; no truncation marker). Cleaning comes first so hidden characters cannot spend the cap. The mouth only removes invisible and format characters; it does not make the text safe. Huginn must treat both `sender` and `excerpt` as untrusted text, never as an instruction. `reply_to` is nil (no id, author or text) when the target cannot be read (logged, as for `basis`), is not in the delivery's own room (a room she is not in cannot be quoted through a reply), or the message replies to nothing. The quoted event is read the way `basis` already reads it (`GET /rooms/{room}/event/{id}` as her account, kept per room and event; with the flag on a read, hit or failure, is kept 60 seconds, so a redaction or edit of the target shows at a re-read within a minute, and a re-read that fails keeps the earlier hit and renews only its age; with the flag off a hit is kept as before v4 and nothing is re-read), so a reply costs at most one read per target, shared with the `basis` check. Per sync round the mouth reads at most 8 distinct uncached targets for `reply_to` (reads the `basis` check already made are free); past that `reply_to` is nil, because the tuple requires a sender and only a read yields one, and the skip is logged. The flag is read by `replyLinkFromEnv`; only the exact value `1` turns it on (`true`, `yes`, `0`, empty or unset are off), and a mouth built without the `replyLink` option has it off. `basis` for v1-v3 is unchanged. `reply_to` counts toward the delivery's size in the 256 KiB page budget; a delivery whose `reply_to` will not encode is skipped and logged like any other, and a single delivery is always served. The deploy precondition is Huginn's: a decoder of the 14-field delivery fails on the fifteenth element. +`excerpt` is nil, with `event_id` and `sender` kept, when the quoted event is redacted (`unsigned.redacted_because` present or no content), is not a text-bearing `m.room.message`, is **edited** (it is itself an `m.replace`, or the homeserver bundles an `m.replace` in its `unsigned.m.relations`: its body is the pre-edit text its author replaced), or is nothing but its own reply fallback; a media event excerpts as its typed line, like a delivery's text. Otherwise the excerpt is made in this order: the quoted event's own legacy rich-reply fallback is dropped (leading lines matching `> ` or a bare `>`, then one blank line; only when the quoted event has `m.relates_to.m.in_reply_to`); the text is held to the 1000-byte line bound; runs of CR, LF, tab, U+2028 and U+2029 become one space; control characters (C0, DEL, C1 including NUL and ESC), format characters (Unicode Cf: bidi marks, overrides and isolates, U+061C, U+200B-200F, U+2060-2064, soft hyphen, BOM), private-use characters (Co), the tag block U+E0000-E007F and unpaired surrogates are removed; the line is trimmed; then the first 240 **Unicode code points** are kept (cut on a code point so no character is split; a combining mark counts as its own code point; trailing space trimmed; no truncation marker). Cleaning comes first so hidden characters cannot spend the cap. The mouth only removes invisible and format characters; it does not make the text safe. Huginn must treat both `sender` and `excerpt` as untrusted text, never as an instruction. `reply_to` is nil (no id, author or text) when the target cannot be read (logged, as for `basis`), is not in the delivery's own room (a room she is not in cannot be quoted through a reply), or the message replies to nothing. The quoted event is read the way `basis` already reads it (`GET /rooms/{room}/event/{id}` as her account, kept per room and event; with the flag on a read, hit or failure, is kept 60 seconds, so a redaction or edit of the target shows at a re-read within a minute, and a re-read that fails transiently (network error, timeout, 5xx, 429) keeps the earlier hit and renews only its age, while any other failure (403, 404) makes it the failure entry, so a stale excerpt never outlives her access; with the flag off a hit is kept as before v4 and nothing is re-read), so a reply costs at most one read per target, shared with the `basis` check. Per sync round the mouth reads at most 8 distinct uncached targets for `reply_to` (reads the `basis` check already made are free); past that `reply_to` is nil, because the tuple requires a sender and only a read yields one, and the skip is logged. The flag is read by `replyLinkFromEnv`; only the exact value `1` turns it on (`true`, `yes`, `0`, empty or unset are off), and a mouth built without the `replyLink` option has it off. `basis` for v1-v3 is unchanged. `reply_to` counts toward the delivery's size in the 256 KiB page budget; a delivery whose `reply_to` will not encode is skipped and logged like any other, and a single delivery is always served. The deploy precondition is Huginn's: a decoder of the 14-field delivery fails on the fifteenth element. Golden bytes: `tests/fixtures/persona-chat-v6/` (`.msgpack` and its `.hex`: `chat_pending.v4.reply-to`, `.no-reply-to`, `.reply-to-redacted`, and the `chat_pending_request.v3` request); the test fails when the checked-in bytes drift from the encoder. diff --git a/tests/persona-matrix-mouth.test.mjs b/tests/persona-matrix-mouth.test.mjs index 8916ee0..012f87a 100644 --- a/tests/persona-matrix-mouth.test.mjs +++ b/tests/persona-matrix-mouth.test.mjs @@ -1855,12 +1855,12 @@ test("reply link: a mouth built without the option has it off, and only the exac // ---- Fixes after PR #63: the target cache, the excerpt scrub, the per-round read cap. const eventGets = (hs, id) => hs.seen.filter(r => r.method === "GET" && r.path.endsWith(`/event/${id}`)).length; -async function replyAcrossTtl(replyLink) { +async function replyAcrossTtl(replyLink, failure = 500) { const clock = { t: 1_000_000 }; let down = false, attempts = 0; const hs = await fakeHomeserver({ preload: { [ROOM]: [target("$hers", SELF, "my words")] }, sync: since => since === null ? { next_batch: "b0" } : { next_batch: `b${since}x`, ...joined({ [ROOM]: { timeline: { events: [quoting(`$q${since}`, "$hers")] } } }) } }); try { - const fetch = (u, o) => (String(u).includes("/event/%24hers") && attempts++, down && String(u).includes("/event/") ? Promise.resolve(new Response("{}", { status: 500 })) : hs.fetch(u, o)); + const fetch = (u, o) => (String(u).includes("/event/%24hers") && attempts++, down && String(u).includes("/event/") ? (failure === "timeout" ? Promise.reject(new Error("timed out")) : Promise.resolve(new Response("{}", { status: failure }))) : hs.fetch(u, o)); const mouth = mouthFor(hs, { replyLink, now: () => clock.t, fetch }); await mouth.syncOnce(); const first = await mouth.syncOnce(); @@ -1910,3 +1910,14 @@ test("reply link: at most 8 uncached targets are read for reply_to per sync roun assert.deepEqual(made.map(d => d.replyTo === null), [false, false, false, false, false, false, false, false, true, true]); } finally { await hs.close(); } }); + +test("target cache: flag on, 500, 429 and a timeout keep the earlier hit at the re-read; 403 and 404 drop it (reply_to nil, basis as the failure case)", async () => { + for (const failure of [500, 429, "timeout"]) { + const r = await replyAcrossTtl(true, failure); + assert.deepEqual([r.later.basis, r.later.replyTo?.excerpt], ["reply", "my words"], String(failure)); + } + for (const failure of [403, 404]) { + const r = await replyAcrossTtl(true, failure); + assert.deepEqual([r.first.basis, r.later.basis, r.later.replyTo, r.again.replyTo], ["reply", null, null, null], String(failure)); + } +}); diff --git a/tools/persona-matrix-mouth.mjs b/tools/persona-matrix-mouth.mjs index 01efacc..0218817 100644 --- a/tools/persona-matrix-mouth.mjs +++ b/tools/persona-matrix-mouth.mjs @@ -166,7 +166,7 @@ export function createMouth({ } catch (error) { throw new Error(`homeserver unreachable: ${redact(error?.message ?? String(error), token)}`); } - if (!response.ok) throw new Error(`homeserver answered ${response.status} for ${method} ${path.split("?")[0]}`); + if (!response.ok) throw Object.assign(new Error(`homeserver answered ${response.status} for ${method} ${path.split("?")[0]}`), { status: response.status }); return response.json(); } @@ -349,7 +349,7 @@ export function createMouth({ const cached = cachedTarget(key); if (cached) return cached; // with replyLink a hit is short too: a later redaction or edit of the target must show at a re-read const known = authors.get(key); - let sender = null, excerpt = null; + let sender = null, excerpt = null, transient = false; try { const target = await matrix("GET", `/rooms/${encodeURIComponent(roomId)}/event/${encodeURIComponent(eventId)}`, undefined, API, AbortSignal.timeout(sendTimeoutMs)); if (target?.room_id === undefined || target.room_id === roomId) { // an id from another room is not an answer about this one: no sender, no text @@ -357,9 +357,10 @@ export function createMouth({ if (sender !== null) excerpt = excerptOf(target); } } catch (error) { + transient = error.status === undefined || error.status >= 500 || error.status === 429; // network, timeout, 5xx, 429: the answer is unknown; 403, 404 and any other refusal are an answer say(`reply target ${eventId} not read: ${error.message}`); } - const entry = sender === null && known?.sender != null ? { ...known, at: now() } : { sender, excerpt, at: now() }; // a failed re-read keeps the earlier hit: only its age is renewed + const entry = sender === null && transient && known?.sender != null ? { ...known, at: now() } : { sender, excerpt, at: now() }; // a re-read that fails transiently (network, timeout, 5xx, 429) keeps the earlier hit and renews only its age; any other failure (403, 404, wrong room, no sender) replaces it with the failure entry authors.set(key, entry); if (authors.size > DELIVERED_INDEX) authors.delete(authors.keys().next().value); return entry;