Repository navigation
Conversation
alt-glitch
added
type/bug
Pending withdrawal changes three oversized modules: the shared adapter, turn execution and Matrix adapter. Extract their existing pending merge, post-turn selection and redaction handling into topical siblings first. The Matrix redaction and runner drain methods keep their complete bodies. The pending merge keeps its existing caption helper through a local adapter import. In-tree callers and patch targets use the defining module, with no compatibility re-export. The strict fifteen-file gate passes 631 tests before any withdrawal behaviour changes.
Matrix gateway admission and text-batch receipts share the durable sync boundary. Move their unchanged methods into an intake mixin before extending the same boundary to native content. The adapter retains its configuration, event parsing and transport lifecycle. The four method ASTs are unchanged apart from a defining import for the adapter's command normalisation helper. The existing sync, native-content and reaction admission contracts pass after extraction.
A native emote can wait in the ordinary text batch while Matrix processes later sync responses. Its handler returned no receipt, so the durable sync cursor advanced before the batch entered the gateway. A restart in that window could lose the emote. Ordinary text already deferred the cursor until the same batch completed admission. Use that existing admission method for both text and native emotes. Each buffered event gets the batch's deferred receipt, and immediate emotes report the same consumption result as immediate text. The regression uses typed mautrix sync events and the actual durable checkpoint store. Both text and emotes keep the previous cursor while buffered and advance only after gateway admission.
The restart fixture waited for the model task after admitting its prime input. Durable batch admission commits the sync cursor in a separate checkpoint task, so the fixture sometimes read the old cursor and failed although the input had been admitted correctly. Wait for the existing checkpoint receipt before checking the admitted input and cursor. The assertions still require the same exact message and completed checkpoint.
The extracted intake mixin calls the gateway's cooperative message handler and uses Matrix caches, parked voices and admission receipts. Those dependencies were implicit in the adapter, so the standalone mixin could not type-check its calls or its super dispatch. Make the existing BasePlatformAdapter relationship explicit and declare the Matrix-specific attributes and callback signatures. The adapter's runtime method resolution order is unchanged, including the gateway handler that accepts each durable intake.
iainlane
force-pushed
the
fix/matrix-reaction-feedback-thread
branch
from
October 1, 2026 19:46
1e9af79 to
016267f
Compare
The native rich-content contract expected a redaction placeholder after withdrawing one queued sticker. Pending input is removed before its turn starts, so the shared withdrawal path rebuilds the merged input from the remaining contribution. Placeholders apply to content revalidation after a turn has already started. Assert the complete retained caption, attachment echo and exact PNG for the queued phase. The existing model-request count requires one remaining turn, and the whole input excludes the withdrawn caption and image. Keep the earlier active-turn placeholder contract unchanged. A separate native admission observer confirmed that both stickers were coalesced before the sender redacted the second contribution. The final model input contained only the retained contribution; the obsolete placeholder expectation failed in both plain and encrypted rooms.
The queue composition records the identity of each merged contribution so a pending withdrawal can remove the correct input. The older busy photo, startup and grace controls still expected the pre-withdrawal MessageEvent shape, whose merged message identity list was empty. Keep those whole-event contracts and include the incoming contribution identity in the expected merged event. All sender, reply, permission, attachment and FIFO boundaries remain asserted. The three compatible controls fail on the published composition before this correction.
Input that arrives during startup restoration waits in a separate list. The shutdown writer only captured adapter and overflow queues, so stopping before the startup gate opened lost that deferred input. Include the startup list in the existing profile-scoped snapshot writer. Each session keeps its FIFO tail followed by its deferred inputs. Remove only the inputs whose complete snapshot was written; a failed profile write leaves its inputs available for retry. The invariant uses two owning homes in A/B/A order through the real startup deferral gate. Both homes get their complete ordered records, and a controlled write failure preserves A's inputs while B succeeds.
Pending input can await the busy handler or recursive turn preparation before the next model turn starts. Those paths are inside the oversized adapter and turn execution modules, which makes their shared dispatch ownership boundary difficult to change in a focused patch. Move the unchanged methods into topical mixins before extending pending withdrawal through those waits. Keep the existing facade bindings, logger identities and lazy imports. The method ASTs are unchanged apart from the defining import for the adapter's thread metadata helper.
Pending preparation can overlap cancellation, shutdown and a later input while provider or handler work awaits. The processing and task ownership methods remain inside the oversized base adapter, and the voice interrupt helper remains inside the oversized turn module. Move the unchanged adapter processing lifecycle into a topical mixin and move the unchanged interrupt helper beside pending selection. Existing callers still use the adapter and runner methods through their MRO. Keep logger identities and defining helper imports, and declare the actual host attributes and callback types before extending the ownership model. The eleven method ASTs are unchanged except for the defining adapter helper import. Existing ownership, shutdown, voice and follow-up contracts remain green. The normalized type comparison adds no diagnostic. The current prerequisite reconciles pending input after a cancellation wait times out. Preserve that completion path when extracting processing methods, including the original local imports for their facade helpers. Resolve the captured upstream changes in the original defining scopes when updating this contribution. Keep the extracted implementations and authored behaviour; apply the same current annotation, timeout or UTC spelling in their actual owners where those scopes moved.
The Matrix adapter cached every inbound audio, video and file attachment under a name taken from the event's raw `body`. It never read the `filename` field. Since Matrix 1.10, a client that sends a caption puts the filename in `filename` and the caption in `body`, and some clients send a caption in `body` without any `filename`. A reply can also prefix the body with a quoted reply fallback. The cached document was therefore named after the caption, for example `doc_<id>_please review`, or after several lines of quoted text. Audio took its extension from whatever followed the last dot in the body: a reply whose fallback quoted `@bob:example.org` produced a file named `audio_<id>.org> what did you think?` with the rest of the caption after it. That name has no audio extension when the audio container is not recognized. Add `inbound_media_filename`, which returns the declared `filename`, or the body with any reply fallback removed when the body is itself a filename. A body counts as a filename when it is one line with no directory part and ends in an extension, and either the extension maps to a known MIME type or the body contains no spaces. A client that follows the spec without captions, such as one that sends `Q3 Report.pdf` as the body, keeps its name. A multi-line caption, or one containing a URL, does not become a filename. The helper also reduces a declared filename to its last component for both `/` and `\`, so the name contains no path separator on any host. When neither source gives a filename, the adapter caches the document as `document` or `video` with the extension for the event's MIME type, and audio takes its extension from the MIME type as well. The voice flag from `_classify_inbound_media` was used only to choose between two defaults that both ended in `.ogg`, so it is removed. `cache_document_from_bytes` already strips directory components and checks that the result stays inside the cache directory, so a sender could not write outside it before this change either. Integration: apply the cache-name change in the extracted media sibling. Keep the canonical three-field media classification contract and remove only the obsolete download filename default. Adapt the tests to streamed SDK downloads and the current room identity model. (cherry picked from commit fa87c83)
Root AGENTS.md asks for a facade's siblings to use the name <stem>_<topic>.py. The filename helper belongs to adapter.py, so the module becomes adapter_media.py, as in the Discord adapter. Integration: the media sibling already contains the extracted handler. Move the helper into that module with its function bodies unchanged. (cherry picked from commit 2e0bbf2)
The first version of `inbound_media_filename` rejected any name that `str.isprintable()` refused. That excludes every Unicode space other than U+0020 and every format character. macOS names screen recordings with U+202F before "AM" or "PM", and emoji sequences contain U+200D, so such an attachment was cached as `video.mov` even when the client declared the name in `filename`. A declared name that reduced to nothing, such as `..`, also stopped the helper from using a clean body, and a non-string `filename` was cached under its `str()` form. A body without `filename` needed a MIME type known to `mimetypes` whenever it contained a space. That table differs between Python versions: `config file.yaml` was kept on 3.14 and dropped on 3.11 to 3.13, and `Contract Final.pages` was dropped on all of them. Python 3.14 also treats a trailing `.` as a suffix, so `Done.` became a filename there only. Remove only control characters (category Cc) and the bidirectional embedding, override and isolate controls, which can make a name display with a different extension. Use `filename` only when it is a string and fall back to the body when it reduces to nothing. Accept a body as a filename unless it has more than one line, contains a path separator (which excludes a URL), or lacks an extension of one to ten letters or digits. The rule uses no lookup table, so every supported Python version gives the same answer. Take the generic extension from `media_cache.ext_for_mime`, which the other adapters share. It removes MIME parameters, so `video/mp4; codecs=avc1` now gives `video.mp4` where `mimetypes.guess_extension` returned nothing. Integration: keep the corrected helper in the existing media sibling. Use the shared MIME-extension resolver in the streamed cache path. The original twenty-one filename cases pass against the real cache; the four affected files pass 317 tests with zero retries. (cherry picked from commit 69e68c3)
A queued message leaves its adapter slot before voice transcription and recursive turn preparation finish. Sender deletion could therefore miss that input and allow the deleted text or audio to start the next turn. Overlapping busy handlers also replaced the lane's single reservation. Keep each preparing input in the existing dispatch reservations until steering, redirecting or turn dispatch claims it. Withdrawal rebuilds the remaining contributions in place, including admission copies, so waiting preparers cannot continue with a detached event. Revision checks repeat transcription and inbound preparation after partial withdrawal and check again after transcript echoes and processing hooks. Cancellation restores unclaimed input in FIFO order and releases the captured reservation. A previous task's completion cannot release a new owner. Shutdown snapshots and capacity checks enumerate all accepted reservations, while new intake preparations do not bypass queue limits. The two lifecycle contracts reproduce deletion during STT, transcript echoes, admission rewrites and recursive preparation. They also verify partial voice retention, overlapping busy input and the irreversible claim boundary under cancellation. Automatic durable restoration remains a separate consumer of the pending snapshots. The pending-input protocol resolves the session key before the pause gate. The existing paused-turn test used a bare runner and a source without thread identity, so it failed before checking the paused reply. Construct a real GatewayRunner, MessageEvent and SessionSource while preserving its three original paused-reply assertions. This owner integrates only the pause-fixture hunk from original campaign correction 61048ea. The native locale-warming hunk has separate NousResearch#126281 and NousResearch#128952 union destinations. This unit-fixture correction provides no new native validation. Cancellation before transcript ownership keeps the provisional input for shutdown recovery. Exact durable input ownership excludes active input from pending capacity. Withdrawal closes at the original synchronous dispatch boundary, separately from recovery ownership. Plugin consumption, lease refusal and completed handlers still finish their dispatch claims. Retain linked pending reservations until cancelled processing completes. The completion callback and waiter share reconciliation for each original input, so non-durable input returns to the queue once and durable input is excluded. Preserve the existing synchronous withdrawal boundary and current ownership fixtures when composing the updated prerequisite. Close the withdrawal fixtures and sweep their isolated test-home registry after the original task cleanup. The lease fixture also replaces a constructor store, so closing its replacement alone leaves a registry reference. Preserve all original phase and partial-input assertions.
When the Matrix adapter could not download or cache an inbound attachment for a reason other than its size, such as a network error, an empty download or bytes that the image cache rejects, it gave the agent the attachment's `/_matrix/client/v1/media/download/...` URL instead of a cached file. That endpoint requires the bot's access token, so the agent's attempts to fetch the URL failed, and the model could repeat the URL in its reply. Encrypted media had no URL to fall back to: the event kept its media message type with no attachment, and nothing told the agent that the user had sent one. When the download or cache step produces no file, the adapter now sends a text event with the user's caption followed by a marker such as `[matrix image attachment could not be downloaded: photo.png]`. The marker has the same shape as the `[matrix <kind> attachment too large: <filename>]` marker proposed in NousResearch#125637, and `_inbound_media_marker` builds it so that the oversized-media path can use the same helper. The filename is the declared `filename`, or a body that contains only the transport filename once any reply fallback is removed. The event has no media URL or media type, so downstream tools do not treat the attachment as downloaded. `_mxc_to_http` has no other caller and is removed. Integration: use the shared filename rule and preserve the canonical sticker kind, retained reply context, mention claim and admission result. Oversize failures still take precedence. The current SDK never returns None, so the file failure fixture uses the subsequent source timeout correction at the streamed download boundary. The affected gate passes 319 tests; four original failure cases were reproduced before the fix. (cherry picked from commit cdc5ce6)
The previous commit said that an empty download gets the new marker. That is true only for images. mautrix's `download_media` returns the response body or raises; it never returns `None`. An empty body is `b""`, which the image cache rejects because it has no image signature, so an empty `m.image` gets the marker. The audio and document caches write an empty body as an empty file, so an empty `m.file` or `m.audio` still reaches the agent as an empty cached file with no marker. The failures that do produce the marker are a download that raises, bytes that the image cache rejects, and missing decryption metadata for encrypted media. The file case of the test mocked `download_media` to return `None`, which only a mock does. It now raises `TimeoutError`, as a stalled download does. The test also covers `m.file` and `m.audio`, so it moves out of `TestMatrixImageOnlyMediaNormalization` into a class of its own. Integration: the preceding adapted streamed fixture already uses the real TimeoutError case. Preserve this contribution as the separate test class move. All three original cases pass in the new class, and the previous four-file gate has 319 passing tests. (cherry picked from commit cb2c0db)
A media download failure must remain visible to the model without an HTTP download URL which requires the bot token. The filename and failure repairs also need to preserve the adapter's retained reply context, mention claim and consumed-event result when the input becomes text. Exercise a real homeserver 404, image-cache rejection and encrypted attachment without its required hash through the independent Matrix client and gateway. Compare the complete user-message record before the separate first-message onboarding note, and confirm that no attachment was cached. Keep the existing caption and both oversize cases unchanged. Add one parameterised adapter contract for the retained parent, both admission outcomes, sticker classification and size-limit precedence. The same contract checks that empty audio and documents remain cached, and that missing-URL inputs keep their existing behaviour. The unfixed parent fails the five changed admission cases while six controls pass. The fixed adapter passes all eleven cases. The live parent reproduces the missing failure markers before fixed-image validation.
Intake checks the adapter's pending dispatch reservations before using an input. Unconstrained mock adapters fabricate another mock for that store, so traversing its records follows an endless mock predecessor chain. Initialise the same empty dictionary as a real adapter in those fixtures. The voice fixtures also fabricate an asynchronous channel-prompt resolver, although the adapter API is synchronous. Those three fixtures already emit unawaited-coroutine warnings on the preserved baseline. Provide a synchronous resolver that returns no prompt and keep the existing input and routing assertions.
The canonical context test treated a failed encrypted download and an input without a URL as the same caption-only event. Download failures now include a marker, so the encrypted row rejected the intended new behaviour even though its cache and native-image state were correct. Specify each row's complete expected message. Keep the missing-URL row unchanged because no download was attempted, and retain the assertions that neither path creates an attachment or pending native image. The encrypted row fails on the composed failure repair before this expectation change while the missing-URL control passes. Both rows pass with their distinct expected messages.
The Matrix callback converts the server timestamp to seconds for the startup grace check, but text and media discarded that value when they created a MessageEvent. Downstream consumers therefore saw the adapter's processing time instead of the message's server time. Pass the event time through the shared inbound event builder and convert it to an aware UTC datetime. Events without a server time use the current UTC time. A voice message can also wait for a separate bare mention: keep the voice's server time while it is gated and parked, then use that time when the mention claims it. Otherwise the claimed voice receives the later mention's timestamp. Callback tests exercise text and media with and without server times. The parked-voice test covers both a later claim and concurrent voices; its three claim cases failed before the parked time was preserved. Adapted from a202104 for the shared inbound event builder. Source-commit: a5c20f1 Integration-note: The canonical builder is topical; timestamp is keyword-only after the existing context arguments. Normal text, emote and both successful and marker media paths propagate it. Parked voices use their original server time. Upstream compatibility: keep the current builtin tuple annotation and include the server timestamp in the parked voice record and gate slots.
Adapter tests can confirm timestamp conversion but cannot prove the server time reaches persisted turns through the running gateway. Send text and image events through a separate Matrix client, fetch their origin_server_ts values from Synapse, and compare them with user rows indexed by the corresponding Matrix event IDs. This exercises the live transport, media handling, turn, and transcript paths. Source-commit: edabc0a
A missing gateway write made the live timestamp test poll the database for a full minute before reporting a failure. Persisted rows normally appear as soon as the gateway handles each event. Stop polling after 15 seconds while keeping the 250 ms checks for real text and media events. The separate-client Synapse case passed with a 2.766-second test body. Source-commit: b96eacb
The adapter converted every non-zero origin_server_ts with datetime.fromtimestamp. The sender's homeserver sets that value, and canonical JSON allows integers far beyond year 9999, so a value such as 10**15 ms raised ValueError while the event was being built. By then the event was marked as seen, the read receipt was sent and any media was downloaded, so the message was lost. On the voice-mention path the parked voice had already been claimed, so both the voice and the mention were lost. A representable time far in the future caused a quieter problem. A remote server with a clock a year ahead produced a user turn dated a year ahead. The session's latest message then made it look active until that date, so stale-session archiving skipped it and session lists sorted it first. Convert the server time in one helper. It returns the current UTC time when the value is missing, when datetime cannot convert it, or when it is more than 60 seconds ahead of the current time. The adapter reads the current time through an instance clock so the tests can fix it. The four timestamp cases are now one parameterised callback test in their own class, with rows for a skew within the tolerance, a time a year ahead and an unrepresentable time. The last two rows failed before this change, one with the future date and one with ValueError. Source-commit: 4c262d5 Integration-note: The unchanged source helper is defined in the extracted inbound-events sibling. The UTC clock is an overridable static method there, preserving the injected clock without growing the facade constructor.
The adapter kept a server time up to 60 seconds ahead of its own clock. The sender's homeserver sets origin_server_ts from its own clock, while session persistence stamps the assistant reply with the gateway host's time. When a remote server's clock ran 30 seconds ahead and the agent replied within 10 seconds, the user row was stored about 20 seconds after the reply. Views that order messages by timestamp, such as search results and the neighbouring rows around a search hit, then showed the reply before the question, and with message timestamps enabled the model saw the question stamped after its answer. Any skew inside the tolerance that exceeded the reply latency had this effect. Return the earlier of the server time and the current time instead. A correctly synchronised server stamps an event before the adapter receives it, so the clamp changes that time by at most clock jitter and every past time is kept exactly. A server that is ahead gets the receipt time, which sorts before the reply. Missing and unconvertible values still fall back to the current time, and the tolerance constant is gone. The 30-seconds-ahead test row now expects the current time. It failed before this change with the server time 15:00:30 against a clock of 15:00:00. Source-commit: 1e4c467 Integration-note: The original complete clamp helper is retained in the defining inbound-events sibling.
A notification retry test must include the subscription cursor and the turn admission result. Once collection advances the cursor, checking only a submitted string can miss a batch that was discarded before a turn started. Use real subscriptions and admission for lease refusal, startup failure and Stop. Verify that the buffered batch remains available and starts once on a later poll, without replaying the database event. Also verify that accepted diagnostic entries are removed without deleting an appended message or a regular message with equal text. These invariants fail against the prior integration base: all three refusals discard the batch, and diagnostic startup prematurely removes the selected entries. Canonical-source: 971bc03
The Matrix invite lifecycle and Slack direct-message resolver remain in large adapter facades. Move these topics into typed platform siblings before adapting their pending fixes. Runtime SDK bindings remain deferred through the Matrix facade. Move the configured inbound-media limit reader into its defining sibling and update its consumers and test binding. The existing fallback reader, positive limits and negative-limit behaviour are unchanged by this move. Upstream compatibility: extract the complete current invite and Slack DM target methods with their builtin mapping annotations. Keep the current platform test import while patching the extracted media-limit supplier. Current-layout integration: Keep the extracted Matrix invite implementation and use the current built-in TimeoutError spelling at its defining owner.
Shutdown records preserved public message fields but omitted the original contributions and merge operations. Reconstructing only the final body would therefore prevent a deleted constituent from being removed before replay. Native Matrix snapshots also need fresh dependencies on the current transport cache, rather than objects from the previous process. Record contributions with a closed set of the four existing merge operations. Decode public fields with their actual dataclass types and rebuild Matrix reply, logical excerpt and rich-content snapshots against the current adapter. The consumer must still validate routing, access, remote availability and local files before admission. Keep the aggregate pending UID when withdrawal rebuilds its remaining parts. This identifies the same stored input after re-preparation. Cache voice text only when every original attachment still has the same bytes; a successful clip alone cannot validate a changed untranscribed clip. Already echoed clips retain their own matching attachment receipts. The four merger contracts reproduce UID loss on withdrawal. Changed and missing untranscribed attachments reproduce stale cached voice text. The repaired reconstruction, native snapshot and transit contracts pass 39 tests through the project runner. This does not enable live replay or consume a shutdown record. Repair note: Pending voice can cache formatted text without clip receipts or typed parts. Capture that text when it is not None, including an empty string, so reconstruction reuses the cache when every attachment still matches. Changed and missing attachment controls still discard cached text. The new opaque and empty cache invariant fails on the published contribution for unchanged attachments and passes with this capture case.
Turn preparation resolves the session, lease, transcript and inbound content before the agent executes. Extract that existing phase and its prepared value into a topical sibling so pending input ownership can be updated without adding more logic to the large turn execution module. The two extracted definitions retain their ASTs apart from the actual GatewayRunner receiver annotation. Current gateway failure-owner and inbound preparation contracts pass before the ownership change. Preserve the current input-owner binding when moving turn preparation into its defining mixin. Pending recovery remains provisional until the session store records that owner, while the existing active-turn marker continues to describe the turn lifecycle. Extract the complete current preparation record and method, including the defining YOLO restoration call, prompt pins, input ownership, title and persistence fields. Keep the original receiver annotation and the original recursive input-owner helper.
The initial gateway turn writes its input owner beside the actual user row. Recursive queued turns omitted that metadata, so a durable pending record could not distinguish an executed follow-up from RAM admission. An input without a delivery ID also received a different owner on every attempt, even when its shutdown record already had a stable identity. Resolve input ownership through one helper for both execution paths. Use the stored pending UID when present; ordinary platform inputs keep their existing delivery namespace and keyless live turns retain distinct owners. Partial withdrawal and re-preparation therefore query the same stored input on the canonical transcript continuation. Extend the existing recursive persistence contract to require that owner at the actual agent call. The contract fails without metadata on the base and passes with the repair, alongside the failure-writer and transit controls. This marker proves persistent user input, not RAM admission or exactly-once external effects before the transcript flush succeeds. Bind the pending dispatch to the owner computed by the recursive input helper. The durable-owner query must use that same identity after the helper replaces the inline owner calculation.
Three adapter fixtures left conversations_replies unconstrained. Awaiting that AsyncMock returned another AsyncMock, so the synchronous response .get call created a coroutine that was never awaited. Collection of that coroutine reported warnings in later tests, obscuring the original calls. Return the actual empty Slack response shape in the mentioned command, existing thread and assistant DM fixtures. Their assertions and all other fixture behaviour are unchanged. The full file also reproduces the fault on the published transit baseline.
The text batching extraction moved annotations into a module without the MessageEvent import. Its dynamic chunk-length assignment also retained a type suppression even though the adapter already reads that metadata with getattr. Import the defining event type and write the private chunk metadata with setattr. The module now passes type checking without a suppression; the text batching contracts retain their original behaviour.
Combining forwarded contexts preserves each contribution, but a thread batch also fetched the same shared history for every chunk. A later fetch included the first chunk as history, so the merged model input repeated both the background and the sender's earlier words. Fetch shared history for the first compatible text contribution only. A successful fetch is recorded on that concrete pending event, and the producer checks the pending batch again after the network await. Forwarded content still belongs to its original contribution and is merged normally. An empty first fetch does not prevent a later chunk from fetching history. The existing batch invariant now covers plain and forwarded input. Both cases fail with duplicated history on the composition base and pass with one backfill. Withdrawing the first contribution leaves only the second sender's text and its own forwarded context.
Authored reference expansion now precedes sender and quote formatting. The composed Matrix path also moved snapshot capture after that awaited expansion. An edit during expansion could then be overwritten by the initial remote context fetch, and failed edit recovery could reuse stale quoted text. Capture the typed current context before reference expansion, then expand only authored text before enrichment and rendering. The existing final snapshot refresh still applies edits, redactions and unavailable states. The sixteen reference-boundary controls pass without changing their expected effective-state protection or the cached conversation prefix.
Queued user rows now persist their execution owner beside room state. Add that owner to the existing whole-object room-state assertion so the contract checks both metadata fields. The WeCom eager-reseed invariant counted frames after a 50ms sleep before its existing state wait. A slow transport could therefore fail frame counting before opening the requested stream. Move that existing wait before counting, preserving every assertion and the missing-seed failure. A 200ms transport delay reproduces the original failure and succeeds with this synchronisation; disabling eager seeding still fails the wait.
Two concurrent thread inputs can both finish history fetches before either publishes its text batch. Thread participation awaits after the first pending-history check, so both contributions could still insert growing shared history despite the earlier producer guard. Recheck the compatible batch immediately before the synchronous enqueue. Remove only the redundant shared backfill; the contribution's forwarded context remains. When the first fetch is empty, a later fetch stops before the earliest batched message and records its successful history insertion on the aggregate after merging. Withdrawal therefore preserves the marker only when the contribution with that history remains. A deterministic two-fetch/two-participation barrier reproduces duplicate history on the prior head. The existing invariant also covers an empty first fetch and withdrawal of either contribution. Three cases fail on the prior source; all five focused cases pass with the repair.
Voice references depended on when transcription ran. Fresh preparation expanded the caption before speech existed, while pending preparation expanded an aggregate that also contained generated failure and path notes. The latter could therefore interpret a generated filename as an authored reference. Keep successful clip text separate from generated notes. Both preparation paths expand the caption and raw speech together under the routed profile before sender attribution, backfill, image analysis and reply context. One preprocessing pass preserves the aggregate reference and token limits. Format transcript quotes and failure notes after expansion, and echo the original speech without any attached file content. Invalidate the typed transcript cache when pending media changes. Preserve the existing tuple API and update its concrete event-aware callers so the pending path caches the same typed input as fresh preparation. Two parameterised behaviour contracts reproduce six failures and two controls on the published parent. The repaired contracts cover both paths, profile A/B/A routing, generated and quoted references, root restrictions and the combined caption/speech budget. Adjacent voice and attachment contracts also pass with strict warnings and no retries. Review-follow-up: NousResearch#128679 (comment) Source-commit: 72caa3d Integration-note: Use the existing canonical voice sibling and typed clip API. Preserve Matrix context capture before reference awaits, current-context refresh, pending revision/withdrawal checks and clip-path echo receipts. Persist typed generated/speech parts alongside existing fingerprinted voice receipts, and invalidate them at the existing bind/merge/withdrawal seams. Preserve the current builtin list annotations when adding transcript parts. The complete speech binding, authored text and rendering scopes remain unchanged.
An older pending voice cache may contain only a formatted aggregate. The speech preparation fallback kept that aggregate literal but discarded the reference expansion of the actual event caption. Append the authored caption's context suffix to the opaque aggregate. The cached aggregate and legacy transcript list remain unscanned because they do not distinguish sender speech from generated notes. An opaque-cache row in the existing profile contract fails on the parent and passes after the fix. The full Weixin file and adjacent gateway gate pass 92 tests across 12 files with strict warnings and no retries. Source-commit: 214ba38 Integration-note: Retain actual-caption expansion beside a literal opaque cache. Canonical successful clip records have separate typed provenance; legacy transcript strings remain unscanned.
Reference preprocessing preserves the authored input and appends context, then strips outer whitespace from the final message. The opaque voice fallback sliced that result using the untrimmed caption length, which could truncate an attached-context header for a padded caption. Strip only the opaque fallback's authored preprocessing input before expansion. Keep the cached aggregate literal and leave ordinary non-voice text unchanged when it has no references. Typed speech already uses a normalised joined authored input. A padded-caption variant in the existing opaque-cache contract fails on the parent and passes after the correction. The strict 12-file gate, including the full Weixin file, passes 93 tests without retries. Source-commit: d6806e7 Integration-note: The canonical authored-text combiner already strips its preprocessing input, so the additional opaque-only production guard is redundant here. Preserve the complete padded-caption regression instead.
…very Pending voice recovery reconstructs fingerprinted successful clips and literal generated notes. The reference pipeline must use that provenance after reconstruction instead of rescanning a formatted aggregate. Add a durable-pending variant to the existing profile contract. It writes the actual voice provenance as JSON, restores a fresh event through the canonical codec and prepares it under profiles A, B and A. Successful speech expands, generated notes remain literal, and transcription runs once per clip without a repeated provider call during preparation.
Compaction preserves channel state on the summary or merge row that replaces removed transcript rows. The preparation hook reads the newest row with channel state without filtering by role, so its baseline can come from a summary rather than a user row. Remove the user-only qualification from the adapter hook documentation.
Speech reference tests used empty history, so they did not exercise channel-state acknowledgement while references appeared in stored transcript rows and adapter-generated context. Extend the existing A/B/A contract with empty and stored history for all five speech variants. Check the complete prepared message, unchanged transcript, lookup and adapter calls, returned channel state and single raw authored preprocessing input. Stored references and generated channel notes remain literal while caption and successful speech references expand under the owning profile. The complete file passes 21 cases with strict asynchronous warnings and zero retries. This adds coverage to the existing preparation behaviour; it does not repair a reproduced production defect.
The startup coalescing fixtures compared complete events but omitted the reaction parent's merged-message identities. The direct processor test also cancelled a same-chat waiter before its callback coroutine began, then discarded that caller-owned coroutine without closing it. Strict coroutine warnings exposed the incomplete fixture lifetime. Include the incoming identity in the complete merged-event expectation. Keep references to the direct test callbacks and close them after all processor tasks finish. The predecessor, cancellation and FIFO ordering assertions remain unchanged. Production coroutine ownership is unchanged. The original combined source run reproduced three complete-event mismatches and one unawaited-coroutine failure. All 455 cases in the 22 adjacent source files pass with strict warnings and no retries.
A queued Matrix reaction includes a refreshable context object with its live adapter and retained event cache. Shutdown snapshots copied every constructor field, so serialising this object raised TypeError and left the complete pending input unwritten. Capture public normalised event fields separately from private context objects. Quoted-media dependencies become plain identity records, and the existing inbound snapshot interface supplies its pending state. Matrix records the bounded logical excerpt dependencies and original excerpt without serialising its adapter or cached authority. This data is inspection evidence, not authorisation to replay a turn. Restoration must reconstruct context through the current owning adapter and revalidate effective state before model use. Capturing a pending record leaves the original live snapshot unchanged, so later redactions continue to replace stale quotations. The regression writes a real split-reply reaction through the production shutdown writer. It reproduced the TypeError before the repair, then checks the complete data record and a later redaction on the live input. Close the logical snapshot fixture runner after its original restore and revalidation assertions. Apply the published public fixture correction without replacing the later snapshot contract.
An accepted leftover /steer can arrive while provisional pending input has already filled the queue. The drain ignored FIFO admission failure, so it dropped the correction while keeping the provisional input. The branch that prioritised a steer over a queued event also restored that event directly without releasing its provisional dispatch reservation. Retain an unadmitted steer for the next turn. Check each pending input through the recognised-command guard before combining the remaining text, so a residual command is discarded without losing the correction. Restore deferred FIFO events through the existing dispatch helper, which releases reservations while preserving order. Three assertion failures reproduce the lost correction with ordinary provisional text, the residual /help safety case, and the restored FIFO state on the parent. The FIFO contract releases each returned dispatch before measuring queue depth, as the caller does in normal operation. All eight contracts in the existing file pass after the fix.
The concurrent-update fixture creates three handler coroutines before passing each to the processor. Cancelling the middle update while it waits for its predecessor leaves that handler coroutine unawaited, so the fixture fails under strict RuntimeWarning checks. Keep references to the fixture-created coroutines and close the cancelled handler after every processor task has finished. The existing assertions still require the first and third handlers to complete in order without executing the cancelled handler.
Direct FIFO callers such as /queue, /steer fallback and Telegram grace could bypass the 32-entry bound. Normal busy admission only logged a refusal, while /queue acknowledged input even when no event was queued. Debounced text was also outside the pending count, so a later photo or command could overtake an older accepted burst. Check admission at the shared FIFO boundary and return its result to command and busy handlers. Count buffered bursts as pending input and submit older bursts before accepting a later FIFO event. Refused external input receives a thread-aware queue-full reply; internal wakes remain unaccepted so their existing producers can retry. Earlier accepted events are never replaced to admit the new input. Copy command events without reconstructing their fields. This preserves reply, media and control context together with the existing bot-loop admission flag, so replay cannot acquire permissions or spend admission twice after stripping the command prefix. Parameterised contracts reproduce bypassed capacity, false acknowledgements, missing refusal notices, context loss and overtaking of buffered input. fix(i18n): translate the pending queue refusal The bounded pending queue reports an English-only warning when admission is refused. The other sixteen bundled language catalogues omit that key, so users receive the English fallback even after selecting their language. Add the refusal message to every bundled catalogue. Verify that runtime lookup selects the requested catalogue, that non-English results differ from the English fallback, and that all catalogue keys and placeholders remain in parity. The runtime contract failed for all sixteen translated languages before the entries were added. The complete four-file locale gate now passes all 92 cases. Source-commit: cd37cd3a25ab9c157495df61ed3a8049765e90ab Protected-source-alias: b01a5942e1b077ee7dcd7ad73f3cc0002f926ae1 Maintenance-adaptation: Apply the debounce admission method change in base_text_debounce.py, preserving its typed receiver. Add the original refusal-control branch to the current platform handler without changing the upstream selected-owner loop. Keep the predicate name and defining merge import from the earlier sources in the busy-session module; its other incoming method and declaration bytes remain unchanged. Maintenance-adaptation: Declare the existing pending-refusal collaborator interface and use pytest monkeypatch for the admission fixture overrides and dynamic marker; keep the original scenario values and assertions. Resolve the captured upstream changes in the original defining scopes when updating this contribution. Keep the extracted implementations and authored behaviour; apply the same current annotation, timeout or UTC spelling in their actual owners where those scopes moved.
The notification contracts can use the installed discord.py module when it is already loaded. Their thread helpers still called Thread without arguments, which only worked with the fallback shim. Five existing thread notification cases failed before reaching the normaliser. Allocate the fixture instance without calling its SDK constructor, then populate the fields which the notification normaliser reads. The real Thread class still supplies the adapter's isinstance contract. Keep the existing test bodies and assertions unchanged. All 17 notification cases pass with the installed SDK. The separate raw deletion contracts construct real SDK guild and thread payloads.
Busy interrupts and post-turn drains share the media placeholder builder. The builder is in the oversized runner facade, while attachment transfer already belongs to the inbound media sibling. Move the builder without changing its behaviour and update internal consumers to import its defining module. Classification still uses the existing facade definitions. This isolates the renderer before changing its profile handling.
The fake HTTP API discarded the client_session supplied by the Matrix adapter, so disconnect closed a mock while the real aiohttp session was left open. A reconnect test also replaced disconnect with a stub and left its new connection open after checking the guard. Strict resource warnings expose both leaks. Preserve the supplied session in the fake API. Use a wrapping disconnect spy in the reconnect test and disconnect the replacement client after the existing call assertion. The fake API and reconnect fixture predate this branch, so this focused fixture maintenance is separate from the corrections folded into their campaign contributors. Production code and all original assertions are unchanged. Apply these complete published fake-API and reconnect fixture scopes to the preserved supplier source. The original fixture definitions predate the campaign, and all original assertions remain unchanged.
Discord event normalization and Slack's inbound guard still occupy their large adapter facades. Move those methods into their platform siblings, with deferred facade bindings for runtime SDK and helper resolution. Extract the pending withdrawal traversal behind its existing sender predicate so native deletion notifications can reuse every pending store. The traversal and existing deletion policy remain unchanged. Compose the platform-event and inbound-context extraction with the current prerequisite. Remove the extracted facade implementations and retain their complete runtime contracts in the defining mixins, including current builtin container annotations, explicit receiver types and the Slack facade helper binding.
iainlane
force-pushed
the
fix/matrix-reaction-feedback-thread
branch
2 times, most recently
from
October 10, 2026 00:37
cf58b30 to
0d7667d
Compare
iainlane
added a commit
to iainlane/hermes-agent
that referenced
this pull request
Oct 11, 2026
An expiry notice for a reaction menu appeared in the room timeline even when the menu belonged to a thread. The pending picker kept the request and expiry time but discarded its send metadata, so the feedback sender received no thread root or card fallback target. Keep a copy of the picker metadata and pass the card's notice metadata through the expiry path. The metadata projection uses the existing threaded-control schema from NousResearch#128710. The sender already distinguishes notices from continuation output, so the notice replies to the expired card even after another message appears in the thread. With rich replies disabled, the thread fallback still targets that card. The real adapter regression covers threaded and timeline menus with rich replies enabled and disabled. Both threaded cases fail before the fix; the complete menu test file passes after it. Expiry still removes the control and never starts a selected turn.
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
13 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
Merge each parent before its child. This PR targets
main; its seven own commits are based on the published prerequisite below. The prerequisite's description lists the earlier dependencies.Behaviour
Feedback on a Matrix approval or picker card stays in the card's thread and replies to the targeted card, including when another output message arrives before the reaction. Cards in the room timeline keep ordinary reply relations.
The shared reaction gate previously sent invalid-key, unauthorised and non-requester notices without thread metadata. Model-picker expiry, choice confirmation and failure notices had the same problem. Picker prompts now retain their send metadata. Feedback uses the thread root and the targeted card's fallback event ID, so continuation tracking does not select later output.
The first commit extracts picker state and shared reaction controls into
reaction_controls.py. The extraction preserves the method bodies and logger binding, and updates direct imports and the area guide. The canonical approval lifecycle retains its expiry and terminal notices. Authorisation, approval/model/choice precedence and ordinary reaction follow-ups remain unchanged.Historical validation
The previous complete six-file affected gate passed all 502 tests on its recorded source. The project runner uses four workers, zero retries and strict resource, runtime, unraisable and thread-exception warnings. Complete child output is resource-clean, and the runner source is restored exactly after output capture. Tests use the actual installed Matrix SDKs.
The seven-path static comparison reports 82 existing type findings. Every complete diagnostic, relative span, affected statement and multiplicity matches the original layer or the current prerequisite. No new finding is introduced. Ruff and introduced whitespace checks pass; the same seven files retain their existing formatting differences.
The current adapter retains all prerequisite mixins and adds the extracted reaction-control mixin after the reaction-prompt mixin. The runtime inheritance check uses the installed Mautrix 0.21.1 source. Local merges against both verified upstream revisions,
99a45ecc17cc51c0488ec3b8ccf7eaf483f7203dand1744a19e0df568c647e4f3ff9c37f2a284a282fb, are conflict-free.The earlier native receipt applies to source
f0e03d83b9a9e0b16530d664574938f8fe064bed: all eight plain/encrypted approval cases passed in 153.8 seconds, within the 300-second budget, with zero retries or skips. The independent observer checked requester-only and invalid-key feedback after an intervening notice, including the card reply target, thread root and continuation flag. That is historical evidence; no fresh native run or hosted CI result is claimed for this update.Review scope
The seven own commits preserve their original authors, author and committer dates, complete message prefixes and order. The extraction commit includes a short explanation of the current import and inheritance join. The original FU19 authorship and provenance trailers remain intact.
Published head:
0d7667d19ed1c86350a2e8514c70d3d4bea1bfc8. Review the seven own commits above prerequisitecda5ab71f0ae03c78b1ceb0e0c773e3b1af6e291. Once the prerequisites merge, rebase this layer when needed to remove the already merged changes from the upstream comparison.Current captured-upstream compatibility
This update preserves all seven original contributions above the published #126288 prerequisite. Each complete source snapshot joins automatically against that prerequisite. The author and committer identities, dates, timezones, complete messages and contribution order remain unchanged. The current source merges without conflicts against captured upstream
3637c512fc3211bdccb61b7140d09ac1018508ba.The fresh affected gate passes 502 cases across six complete files with the actual installed Matrix SDKs, four workers, zero retries and all four strict resource and runtime warning categories. Complete child output through interpreter exit contains no database, loop, socket, transport or client resource warnings. The gate authenticates all 9,355 tracked Python, configuration, lock and runner inputs before and after execution.
That runtime validation composes the three published #135482 worker and auxiliary-client lifetime corrections with this branch. Those three corrections are separate prerequisites and are excluded from this branch's seven own commits. The executed runtime source and the pure publication source are recorded separately. This update does not claim a fresh native homeserver run or hosted CI result.
The full seven-path static comparison contains 82 inherited diagnostics, with no new complete diagnostic occurrence compared with either the original layer or the current prerequisite. Ruff passes; the same seven formatter paths remain inherited. The original comparison uses the byte-identical complete source tree from the previously accepted publication. Earlier validation paragraphs retain their historical source qualifications.