Repository navigation
fix(matrix): keep the whole thread root for a new thread session - #137687
Draft
HarshitRuwali wants to merge 86 commits into
Draft
HarshitRuwali wants to merge 86 commits into
HarshitRuwali wants to merge 86 commits into
Conversation
alt-glitch
added
type/bug
This was referenced Oct 13, 2026
Matrix changes lacked a live exchange test, so mocked adapter tests could miss failures between Synapse, gateway setup, and model-visible context. Start disposable Synapse and a checkout-built Linux gateway through Testcontainers. Drive events with a separate matrix-nio account and a fake model under an isolated HERMES_HOME. Provision a Linux nio client with its own encrypted device store so later encrypted cases can also run from macOS hosts. Run the suite in a dedicated CI job through scripts/run_tests.sh. The plain-room test confirms both the reply and the inbound model context.
Synapse changes the owner of /data to UID 991 when it generates its configuration. On a Linux runner, the host test process cannot append to the resulting homeserver.yaml. Store Synapse data in a Docker volume and edit the configuration from a one-shot container. The test runner can kill a timed-out file worker before pytest tears down its fixtures. Enable Testcontainers' Ryuk reaper, pin its image by digest, and start it before creating the volume. Label the Synapse volume with the session ID; Testcontainers labels its network and containers. Ryuk can then remove these resources when the worker exits.
Parallel Matrix live tests built different checkouts under the same hermes-matrix-live:checkout tag. A second build could replace the tag between the first build and its container start, so a green test might exercise the wrong code. Give each fixture invocation a UUID image tag and remove that exact tag after its containers stop. Concurrent branches can now run the live suite without changing which checkout an existing fixture starts.
A file timeout kills the pytest worker and its build process without running fixture teardown. The unique image tag from the live test can then remain on the Docker host indefinitely. Start Ryuk before building and label the checkout image with the same Testcontainers session ID as its containers, network and volume. Ryuk can remove the image after the worker dies. A SIGKILL probe confirmed that the pinned Ryuk image removes a labelled Docker image.
Matrix live tests built the production image for each checkout. That build prepares SQLite, Chromium, Node and the UI even though the tests start only a gateway. The separate Linux observer also built a second Python environment at test setup time to obtain matrix-nio crypto. Build a small Debian image with the PM-pinned Python and the locked Matrix and test dependencies before copying the checkout source. The gateway and observer use the same prepared interpreter in separate containers, while the gateway still runs against an isolated home and the host fake model. Keep gateway logs in timeout failures so startup problems remain diagnosable. The plain-room test passed with a 1.545-second test body and 26.8 seconds for the complete test file. On this host, an uncached image build took 128.1 seconds and a cached rebuild took 4.67 seconds.
The live plain-room test requested a one-second Synapse poll and allowed a minute before reporting a missing reply. Matrix-nio can retry a sync request beyond that loop deadline, so a stalled exchange could take even longer to fail. Request a 250 ms server wait and bound each sync by the remaining 15-second deadline. Successful replies still arrive through the real Matrix event stream. The separate-client Synapse test passed.
The approval client sends cards, seed reactions and requester controls in quick succession. Synapse's default account event limit exhausts its burst and rejects the controls before the short client deadline. Configure the disposable server for 100 events per second with a burst of 100. The limit remains finite, and tests can exercise protocol events without waiting for the production rate limiter to refill. Client waits, retry rules and assertions are unchanged.
The Matrix live fixtures skipped with "require a running Docker daemon" on macOS installations whose daemon is reachable only through a Docker CLI context, such as Docker Desktop or Colima without the /var/run/docker.sock symlink. The fixture replaced DOCKER_CONFIG with an empty configuration to keep registry credentials away from the tests. That also discarded currentContext and the context metadata, so the Docker SDK, Testcontainers and the docker CLI all fell back to /var/run/docker.sock. scripts/run_tests.sh removed DOCKER_HOST, DOCKER_CONTEXT and DOCKER_CONFIG from the test environment, so a user could not select the socket explicitly either. The fixture now resolves the endpoint the same way the docker CLI does (DOCKER_HOST, then DOCKER_CONTEXT, then currentContext) before it isolates the configuration. It exports the result as DOCKER_HOST and removes DOCKER_CONTEXT, so the SDK probe, Testcontainers and the buildx build all connect to that endpoint. The isolated configuration keeps the CLI plugin directories, because Docker Desktop and Homebrew install buildx outside the default search path. It still contains no registry credentials or credential helpers. On macOS the daemon runs in a Linux VM and resolves bind-mount sources there. Testcontainers mounts the client-side socket path into Ryuk by default, which fails when that path is a host socket such as a context's. The fixture therefore mounts /var/run/docker.sock, the socket inside the VM, into Ryuk on macOS. The runner forwards DOCKER_HOST, DOCKER_CONTEXT and DOCKER_CONFIG. It still drops DOCKER_AUTH_CONFIG, DOCKER_CERT_PATH and DOCKER_TLS_VERIFY. With a scratch context whose socket forwards to OrbStack and counts connections, the plain exchange passed with 70 API connections through that socket. The previous fixture passed the same invocation with no connections through it, because it used /var/run/docker.sock instead. A context pointing at a missing socket now skips with a message that includes the socket path.
The last dependency step of the test Dockerfile imports olm, mautrix.crypto and nio, so an image without Matrix crypto support already fails to build, and the fixture reports the build log. The fixture then started a container from the finished image only to run the same imports again. That check could not fail, and it added a container start to every test module.
The gateway_image fixture built the Linux gateway image inside the pytest worker. run_tests_parallel.py kills a worker when the per-file timeout expires, so the cold build, the Synapse start-up and the exchange all had to fit in the 300-second per-file timeout that the workflow sets. The 45-minute job limit and the build's own 1800-second timeout could never take effect. A cold build of test_plain_exchange.py on OrbStack took 242 seconds of that budget. Move the build command to tests/integration/matrix_live/image_build.py. The fixture still uses it, so a local run builds a fresh image and removes it afterwards as before. The workflow runs the same module in a step before the tests and passes the tag in HERMES_TEST_MATRIX_IMAGE. When that variable is set, the fixture uses the image, checks that the daemon has it, and leaves it in place for the next test file. run_tests.sh forwards the variable, in the same way that it forwards HERMES_TEST_IMAGE for tests/docker. The build no longer has its own timeout: in the fixture the per-file timeout limits it, and in CI the job limit does. Drop the Buildx setup step. The fixture replaces DOCKER_CONFIG, where Buildx records the builder that the step creates, so every build used the daemon's default builder anyway. The pull_request path filter listed the production Dockerfile, which the live tests no longer use, and omitted most of the code that runs in the gateway. Filter on the code that the image copies and runs (the root modules, agent, cron, gateway, hermes_cli, hermes_platform, pm and tools), .dockerignore, the test runner and its setup action. With a prebuilt image, test_plain_exchange.py passed in 70 seconds.
The gateway container ran as UID 10000 and wrote into a bind-mounted
home that the fixture made world-writable. With rootful Docker on
Linux, the directories that the gateway creates, such as logs/, belong
to UID 10000 with mode 0755, so the runner's user cannot delete the
files in them. run_tests_parallel.py ignores errors when it removes its
temporary root, so each test left a tree under
/var/tmp/hermes-pytest-<uid>/ that only root could remove. macOS was
not affected, because OrbStack and Docker Desktop map bind-mount
ownership to the host user.
Run the gateway as the host UID and GID and keep the home's default
mode. The image has no passwd entry for that UID, so set HOME to the
home that the hermes user had. At teardown the fixture removes the home
itself, so a file that the runner cannot delete fails the test instead
of accumulating on the host. Windows has no host UID; Docker Desktop
does not enforce ownership on bind mounts there, so the gateway keeps
running as UID 10000.
On OrbStack's Linux kernel, a home written by UID 10000 could not be
removed by UID 501 ("Permission denied" on logs/gateway.log), and a
home written as 501:20 could. The plain exchange passes as the host
user on macOS.
The fake model server for the live gateway listened on 0.0.0.0, so any machine on the developer's network could send it requests while a test ran. A stray request would consume the scripted reply or appear in main_requests(), and the macOS firewall can ask whether to accept incoming connections. The gateway container reaches the fake as host.docker.internal. With a daemon on this host, as on Linux, the test network's bridge gateway is an address of this host, so the fake now listens on that address and host.docker.internal resolves to it. A daemon in a VM, such as Docker Desktop or OrbStack, does not expose that address on the host, and its host-gateway forwards to the host's loopback interface, so the fake listens on 127.0.0.1 there. The fixture chooses by trying to bind the gateway address. In OrbStack's Linux VM, a server bound to a network's gateway address answered a container on that network through the host.docker.internal mapping. On macOS the bind fails with EADDRNOTAVAIL, and the plain exchange passes over 127.0.0.1.
The gateway fixture yielded as soon as the gateway logged "Matrix: joined <room>". The Matrix adapter joins the invited room shortly after it connects, while the gateway is still in startup restore. During startup restore the gateway queues inbound messages and replays them when restore finishes, so a test's first message could reach the gateway through that replay. A scratch live test that sent its message as soon as the fixture yielded was queued in two of three runs. The replay can reach the adapter while the adapter's original dispatch of the same event still holds the session guard: for Matrix, the guard stays held during the typing-stop request after the handler has queued the event. The adapter then handles the replay as a second message while the session is busy, and the bot's first reply is the "Interrupting current task" notice. One native run of the oversized-media test failed this way. The gateway logs "Press Ctrl+C to stop" after startup restore has finished. The fixture now waits for that line as well as for the join, so a test's first message is dispatched directly. With this check the same scratch test was dispatched directly in three of three runs. When a test that uses the fixture fails, a report hook adds the last 200 lines of the gateway log to the failure report. The fixture deletes the gateway home at teardown, so the log of a failed run was lost before. LiveGateway now includes the home so that the hook can find the log.
Two helpers check whether a reply's HTML body starts with an <mx-reply> fallback. _has_reply_fallback decides whether the plain-text quote is stripped from the message, and extract_mx_reply_quote returns the quote as reply_to_text. Their checks disagreed. _has_reply_fallback required format "org.matrix.custom.html" and a bare "<mx-reply>" tag. extract_mx_reply_quote ignored format and accepted any body that started with "<mx-reply". When the tag had an attribute, as in <mx-reply data-x="1">, or the event had no format field, and the plain-text quote had no pill, the message kept the quote and reply_to_text repeated it. The model then saw the quote twice. Add starts_with_mx_reply(content) to reply_context.py and call it from both helpers. It requires the HTML format and matches an <mx-reply> start tag with or without attributes, because the spec defines the fallback as a formatted_body that "begins with an <mx-reply> start tag". The match ignores case because the quote extractor's HTML parser lower-cases tag names, so the check accepts every tag that the extractor reads as <mx-reply>. extract_mx_reply_quote now takes the event content instead of the formatted body because it needs the format field.
Busy-text debounce updates the reply anchor to the latest inbound message but did not record the earlier message IDs. A new Matrix thread session therefore fetched an earlier trigger as thread history and included the same text again in the current user message. Record each previous anchor and incoming batch's merged IDs before the anchor changes. Thread-history preparation can then exclude every event already represented in the current turn. Input without a new anchor keeps its existing anchor and still contributes its merged IDs. Extend the real model-preparation invariant to both ingress and busy batching. The busy case repeated the first trigger before this change and now excludes it. Independent full-event expectations also reproduce six missing-ID cases and pass after the fix. The pending-merge contract checks unknown, unauthorized and authorized quoted authors together with media and merged IDs.
The room-context prerequisite adds channel_state as the final positional MessageEvent constructor argument. Threading inserted two fields before that argument, so an existing positional room-state dictionary became the reply-author authorization value and the event lost its room state. Append the new threading fields after channel_state. Existing positional construction keeps its original meaning, and the new authorization field defaults to None. A direct constructor probe reproduces the old misassignment and passes after reordering; all 330 tests in the seven adjacent event, thread and gateway files pass.
Resolved reply parents, cached edits and thread history removed every leading Markdown quote block. Modern Matrix clients send replies without a fallback, so an ordinary user-authored quotation disappeared from the context before the model received it. Move the adapter's existing fallback detection and splitting helpers into the shared reply-context module. All context paths now strip only a recognized legacy sender pill or a formatted mx-reply fallback. Edits use the replacement content to identify its formatting correctly. One parameterized behavior contract checks all three context paths against user quotations and both legacy fallback formats. The three user-quotation cases fail on the original head and pass after the fix.
A busy-debounced turn can begin with the thread root and finish with a later reply. The event includes both IDs, but thread backfill excluded only relation chunks and always appended the separately fetched root. The model received the root text twice: once in history and once in the current message. Check the excluded event IDs before resolving the root, just as for thread replies. Extend the real intake, debounce and model-preparation contract to batches that begin with their thread root. The busy case reproduces the duplicate on the original head and passes after the fix; ordinary thread roots still appear once in earlier history.
The shared splitter permits a caller to transform the reply text, so joining the transformed halves does not reproduce the original body. Describe the actual constraint: the quote block includes its separator and remains unchanged while the caller transforms the reply text. The merged event IDs also include earlier busy-debounced input. That path uses the latest message as its reply anchor, so the field comment must not promise that message_id always remains the first input's ID.
Threaded outbound sends always point their fallback reply at the thread root, even after the adapter receives or sends a newer event. Explicit replies are also labelled as fallback replies. The resulting relations lose the natural message chain in clients that do not render threads. Exercise an accepted inbound event, a split text response, an explicit reply, and encrypted and plain media sends. Both tests fail on the current adapter because every fallback target remains the root.
Thread relations previously labelled every outbound message as a fallback, including messages with an explicit reply target. That misrepresented genuine replies and could alter how clients render them. Set is_falling_back to false for an explicit reply and keep its target, even when the target is outside the current thread. For a plain thread send, use the supplied fallback target or the thread root and mark it as a fallback.
Matrix sends previously pointed every thread fallback at the root. A client without thread rendering therefore lost the order of later messages, especially when a response was split or media followed text. Track the latest event per room and thread after an inbound message or a successful outbound send. Use that event for the next fallback, with the root as the first anchor. Explicit reply targets still take precedence and remain genuine replies. The tracker is bounded and belongs to the Matrix plugin, so the gateway core gains no platform-specific state. The focused tests cover split text, explicit replies, encrypted media, plain media and multiple images. The Matrix client-server specification recommends the latest known message for this fallback target.
A separate Matrix client now sends two messages in the same thread and checks the relation on each gateway reply. Each reply targets the current inbound event and uses genuine reply semantics, including after an earlier bot reply in that thread.
The gateway anchors every Matrix final response on the triggering event, so send() passed that event as reply_to for each chunk of a split response. Every chunk became a genuine thread reply with is_falling_back set to false, and Element rendered a quote of the user's message above each chunk. Streamed responses had the same problem: the stream consumer anchors each new chunk on the previous chunk, and later messages in the turn on the triggering event again. In auto-thread rooms the synthetic thread root is the triggering event itself, so the first reply in every new thread quoted its own root. The tracker now records, for each room and thread, whether Hermes sent the latest event and which event Hermes last replied to. A send anchored on the thread root, on Hermes's own latest event, or on an event that already has a reply in the thread is a continuation. It sets is_falling_back to true and points at the latest known event, so only the first message of a response remains a genuine reply. The decision stays in the Matrix plugin, and the platform-neutral chunk anchoring in the stream consumer is unchanged. Unit tests cover a split final response with reply_to, the buffered and live streamed paths, and a reply anchored on the root. A live Synapse test limits the gateway to 500-character messages and checks that only the first chunk of a threaded answer quotes the question. Before this change the second chunk also quoted it.
Outbound thread relations accepted an explicit fallback anchor under two metadata keys, matrix_thread_fallback_event_id and thread_fallback_event_id. Nothing sets the unprefixed key, so remove it. The prefixed key remains because Matrix thread creation sets it to anchor the first reply of a new thread on the new root.
A threaded response quotes its triggering event once, and the later messages of that response are thread fallbacks. The tracker recognised a continuation only when reply_to was the event that Hermes had most recently replied to, or Hermes's own event while it was still the latest event in the thread. Any other genuine reply or inbound post replaced those values, so the rest of the response quoted its trigger again. That happens on ordinary gateway paths. A busy acknowledgement for a follow-up is a genuine reply to the follow-up, and the running turn then sends its next segment anchored on the original question. With thread_sessions_per_user, two sessions can answer in one thread at the same time. The stream consumer anchors each head chunk on the previous chunk, and another user can post between the two sends. Keep, for each thread, a bounded list of the recent events that Hermes has given a genuine reply to and of the events that Hermes has sent. A send whose reply_to is in either list is a continuation. The fallback target is still the latest known event in the thread.
An admitted mention can resume a room discussion after messages were ignored by the mention gate. A new thread session also needs the earlier messages from that thread before the model answers its first turn. Fetch the earlier events through the live Matrix client, bounded by the triggering event and the configured limit. Add the context through the adapter's turn-context hook after admission. Keep room catch-up within its prior timeline boundary, skip unrelated thread events, and retain quoted text as external input. Consolidate the initial implementation and its dependent migrations to the current turn-context hook. The original implementation used the removed room-state and string-returning history interfaces, so those commits could not run individually on the updated parent. Co-authored-by: Jiya (Hermes Agent) <agent@hermes-agent.com> Co-authored-by: Iain Lane <iain@orangesquash.org.uk> Co-authored-by: Cursor <cursoragent@cursor.com>
The user guide said that every message in a thread where the bot already takes part starts a turn, so those threads have no catch-up. With `thread_require_mention: true`, those threads drop messages that do not mention the bot, and an admitted mention includes them as catch-up like any other mention-gated thread. Qualify that sentence, and describe the first turn of a new thread session, which includes the thread root and the earlier thread messages without stopping at the bot's own replies.
The Matrix adapter reads two catch-up limits from config.yaml: matrix.thread_backfill_limit and matrix.room_backfill_limit, each 20 by default. Only the thread limit had an entry in DEFAULT_CONFIG. The dashboard builds its config form from DEFAULT_CONFIG, and GET /api/config fills unset keys from the same defaults. The Matrix section of the dashboard therefore offered the thread limit with its default of 20 but had no field for the room limit, so a user could change the room limit only by editing config.yaml. Config loading and `hermes config set` were unaffected: loading keeps user keys that have no default, and `config set` validates only the first segment of a matrix.* path. Add room_backfill_limit beside thread_backfill_limit with the default that the adapter applies. The dashboard now shows it as a number field set to 20. No _config_version bump is needed because a new key reaches existing configs through the ordinary deep merge.
Mention catch-up scans back from an admitted mention and stops at the bot's last message in the room or thread. The gateway also posts status notices there that do not answer a turn: restart and online notices, background-process and update notifications, heartbeats and tool progress. A notice posted after the bot's last reply ended the scan early, so the messages that the gate dropped before the notice never reached the agent. The gateway already marks these sends with metadata["non_conversational"], but only for Discord. Discord keeps the IDs of marked messages in a sidecar file because it cannot annotate a message. A Matrix event can include a namespaced content key, so the mark can stay with the event on the homeserver. It remains after a gateway restart, and in an encrypted room it is part of the encrypted payload. Mark status sends for Matrix as well. The adapter writes com.nousresearch.hermes.non_conversational into the content of each marked message, and the room and thread history readers skip events that include it. A notice therefore neither ends the scan nor appears in catch-up or in the history for a new thread session, which is how Discord treats its status messages. The readers check the original event content, so a notice that the gateway edits later keeps its mark.
fetch_mention_context said that the transcript already contains the event where catch-up stops. That is not true on the first room turn after `/new`: the new session's transcript is empty, but the scan still stops at the bot's reply to `/new`. It is also not true when each mention starts its own automatic thread, because the previous mention belongs to another thread's session. The boundary itself is the intended one. Mention catch-up recovers the messages that the gate dropped since the previous turn in the room or thread. The messages before that turn belong to the conversation that the reset discarded, and scanning past it would bring that conversation into the new session. Stopping at the start of the new session would be wrong too. A reset by the session policy changes only the session store, and the new session starts with the mention that triggers catch-up, so catch-up would lose every message that the gate dropped since the bot's last reply. A new thread session is different: its first turn receives the whole thread, because the thread is the conversation that the user continues. Describe the boundary as the previous turn, which can belong to an earlier session, in the docstring, the PreviousTurnCheck comment and the user guide. Add a test that pins the room boundary on the first turn after `/new`.
The gateway marks lifecycle and status sends with metadata["non_conversational"] on the platforms whose adapters read the mark, Discord and Matrix, so those notices do not bound a history scan. The shutdown path missed the mark. The restart announcement to active chats, the home-channel broadcast and the cron-interrupted notice all go through _send_notice_logged, which added only the interim marker. On Matrix, a room that received "Hermes is restarting" after the bot's last reply therefore ended mention catch-up at that notice, and the messages that the gate dropped before it never reached the agent. The online notice that follows a restart was already marked, so the scan passed over only half of a restart. Discord had the same gap: its fallback patterns recognise the restarted and online notices but not the shutdown notice. Mark the metadata in _send_notice_logged, which all three sends use.
_non_conversational_metadata decided by platform name whether to mark a
status send, with the set {"discord", "matrix"}. Both adapters are
plugins, and core does not list plugin platforms. Every further
platform that wanted the mark would have had to edit core.
Add reads_non_conversational_mark to PlatformEntry. Discord and Matrix
set it when they register, and the helper reads it from the registry
entry for the platform of the send. The lookup stays keyed on the
platform, not on the adapter object, because a relay transport
delivers Discord sends when no native adapter is connected, and those
sends are marked today. Discord and Matrix sends get the mark as
before, and other platforms keep their metadata unchanged.
Plugin discovery fills the registry, and the gateway runs discovery
before it creates adapters. Tests that expect the mark now run
discovery, as other tests that read registry entries do.
The user guide and the fetch_mention_context docstring said that after `/new` catch-up starts after the bot's reply, so the conversation that the reset discarded stays out of the new session. That is true in the main timeline only. In a thread, `/new` resets the thread's session, and the next mention is the first turn of a new thread session. That turn receives the thread root and the earlier thread messages, including the conversation before `/new`, because the thread is the conversation that the user continues. Say that the rule applies to the main timeline, and that the first message after `/new` in a thread gets the thread history.
Thread catch-up now requires the triggering event and reads history from its boundary token. The earlier quote-preservation fixture returned an unbounded relations response without the required boundary, so the adapter correctly returned no entries before testing the message text. Provide the context and messages tokens and a genuine thread relation. Keep all authored-quote and reply-fallback assertions unchanged. The rebased six-file gate passes 281 tests without retries.
Backfill decrypts an event only after checking its encrypted event type. The general SDK Event factory dispatches to EncryptedEvent for this input, but its dynamically assigned deserialize attribute is absent from the static Event type. Use the concrete decoder and SDK JSON input type, as the published read layer already does. This preserves the runtime decoder and lets the type checker verify the known event kind. The real-SDK reply and thread contracts cover available and missing keys; the six-file gate passes 281 tests without retries.
Later mentions fetch earlier room or thread messages for the next turn. A batched message can already contain some of those events, but only the first-thread-turn path passed their IDs to the history reader. Later mention catch-up therefore repeated their text above the new message. Pass merged event IDs through both mention catch-up paths. The room reader now excludes those IDs before checking the previous-turn boundary, as the thread reader already does. This keeps the current batch separate from earlier discussion and prevents an already batched mention from ending the history scan prematurely. The real room and thread entry paths reproduce duplicate text before the fix. The repaired six-file gate passes 283 tests without retries.
The Matrix SDK retries rate-limit responses and connection timeouts indefinitely by default. A failed request during account or room setup can therefore stop the whole native module until the outer runner kills it, without reporting the setup error. Disable those retries and bound ordinary fixture requests to fifteen seconds. Apply the same configuration to registration, restored accounts and the encrypted observer. The fixtures now expose the SDK error so the failing operation can be diagnosed directly. A real loopback HTTP server reproduces the repeated rate-limit requests through registration and restored accounts. Both paths now make one request and return the expected error.
The first turn of a new thread session gets the thread root, but _format_history_context cut every entry, the root included, to 1,200 characters. A bot answer whose options come after that point lost them, so "go with option A" in a thread on it still had no context. The root is now marked (MatrixEventContext.is_thread_root) and kept whole, as the reply path already does; other entries keep the cap. fetch_thread_entries also returned [] when the thread page request failed, before the root lookup. A new thread session now still gets the root, which is a single event. Catch-up keeps returning [], because it cannot tell whether a failed page held a previous turn. Refs NousResearch#137686
main enabled UP006, UP037, UP041, PLW1510 and RUF024 after NousResearch#125648 and NousResearch#125697 were written, so their lines failed `ruff check .` once rebased onto main. Mechanical fixes only: builtin generics, unquoted annotations (evaluated lazily on Python 3.14), TimeoutError for asyncio.TimeoutError (the same class), an explicit check=False where the caller already reads the return code, and a dict comprehension for dict.fromkeys with a mutable value. No behaviour change.
HarshitRuwali
force-pushed
the
fix/matrix-thread-root-full-text
branch
from
October 13, 2026 09:01
5dae0e9 to
17390b6
Compare
alt-glitch
added
area/install-update
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
The same change is open against #125697's branch as iainlane#11, ready to merge there. This PR is the
main-targeting layer of the stack and stays a draft until #125697 lands. Like the rest of the stack, it targetsmainand includes its prerequisites. Those are #125648 and #125697 rebased onto currentmain; the one conflict, the environment allowlist inscripts/run_tests.sh, keeps the variables from both sides. The change itself isba02c81131. The top commit,17390b6bd9, applies ruff rules thatmainenabled after the stack was written (UP006, UP037, UP041, PLW1510, RUF024) to the stack's lines, soruff check .passes again. Those fixes are mechanical and change no behaviour.What does this PR do?
Closes two gaps in #125697's history for the first turn of a new thread session. Both came up on a real deployment (#137686): in an E2EE DM, the bot answered in the main timeline with a 2,671-character message whose options began at character 1,912, and the user replied in a thread on it, "go with option A".
_format_history_contextpassesmax_chars=1200for every entry, the root included, so the options never reached the model and the thread still had no usable context. The root is now marked (MatrixEventContext.is_thread_root) and kept whole; every other entry keeps the cap. This matches the reply path, which keeps the replied-to text whole because "a preview here silently loses later list items" (gateway/run_inbound.py:1603), and Slack's thread context, which is untruncated (plugins/platforms/slack/adapter.py:5958).fetch_thread_entriesreturned[]when the/contextor/relationsrequest failed, before the root lookup. A new thread session now still gets the root, which is a single event. Catch-up keeps returning[], because it includes the root only when the scan found no previous turn, which a failed page cannot show.Size: the root is still one event. A bot root is at most
max_message_length(15,000 bytes by default); another sender's root is bounded by the Matrix event size limit. Newline neutralization and the[unverified]tag still apply to it.Related Issue
Refs #137686. With #125697 this covers that report's reproduction; the smaller points listed there (an edited root read in its first version after a restart, no notice to the model when the root cannot be decrypted, and the main-timeline exchange before the root) are not addressed here.
Type of Change
Changes Made
plugins/platforms/matrix/reply_context.py:MatrixEventContext.is_thread_root, defaultFalse, last field, so positional construction is unchanged.plugins/platforms/matrix/thread_context.py: the root entry is appended asdataclasses.replace(root, is_thread_root=True), so the cached entry is not changed. On a paging failure a new thread session falls through to the root lookup; catch-up still returns[].plugins/platforms/matrix/adapter.py:_format_history_contextusesmax_chars=0for the root. It is a same-line edit, so the file does not grow.tests/gateway/test_matrix_thread_root_context.py(new): five behaviour tests.[unverified]tag.tests/gateway/test_matrix.py: four equality assertions now expect the root withis_thread_root=True, edited in place.website/docs/user-guide/messaging/matrix.md: one sentence. The root is included in full even when earlier messages cannot be fetched, and earlier messages over 1,200 characters are shortened.How to Test
scripts/run_tests.sh tests/gateway/test_matrix_thread_root_context.py tests/gateway/test_matrix.py.[Earlier messages in this thread]ends in...without the options. With this PR, the whole answer is there.What I ran after rebasing onto
mainat0438ca2da5(Linux container, Python 3.14, mautrix 0.21.1):py_compileon every changed Python file: passes.bash -n scripts/run_tests.sh: passes.ruff check .over the whole repo: passes.tests/gateway/test_matrix*.pyfile, run one file per process:tests/gateway/test_matrix.py: 239 passed.tests/agent/test_context_compressor.py: 305 passed.tests/gateway/test_session.py: 67 passed.tests/scripts/test_run_tests_shell.py: 2 passed.tests/ci/test_classify_changes.pyandtests/ci/test_required_results.py, which covermain's side of therun_tests.shconflict: 159 and 90 passed.tests/gateway/test_matrix_deps_rebind.py(2 failed). It fails the same way on unmodifiedmainin my lighter environment.HEAD~2), this PR's two commits have 0 findings.main, the whole stack has 27 blocking findings, all from fix(matrix): preserve genuine thread replies and fallback chains #125648 and feat(matrix): backfill recent room and thread context #125697 (file length, broad catches and so on). That matches the 26 they had before the rebase, so this PR adds none.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass. Not the whole suite: I ran the Matrix-related files listed above.python scripts/checkpasses. Ruff passes on the whole repo, and this PR's own commits have no code-health findings. The stack below it has 27 pre-existing code-health findings againstmain; see above.Documentation & Housekeeping
docs/, docstrings)cli-config.yaml.exampleif I added/changed config keys: N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows: N/ANot changed here, but worth offering: the broad catch in
fetch_thread_entriesnow continues instead of returning, so addingexc_info=Trueto itslogger.debugcall would keep the traceback. It is #125697's line, so I left it for you.