The query side reads identity through transcriptRowIds, which takes
sourceRowId from every part, while the index side still filtered on
`part.type === 'text'`. Only text parts are ever stamped, so the filter
did nothing but make the two sides use different rules; drop it.
Replace the comment that restated the rowId-vs-last-part ternary with
the reason for it: a fold with a rowId is addressed by its first source
row (hydration's key); without one, its last part row is the final
reply the settled live bubble holds, so pending rows land before it.
No behaviour change.
identicalInterimSiblingIndex now returns -1 for a failed completion, so
the stream path and the fallback settle path share one failure gate
(previously the fallback path settled a failed completion onto the
sealed interim). collapseDuplicateFinalOntoIdenticalInterim drops its
own hasFailure option and the dead guards a non-negative interim index
already implies, and shares the merge-and-drop tail with the tool-interim
collapse via a private helper. The no-fallback branch's
`identicalInterimIndex >= 0` arm was unreachable (any such index is a
visible assistant past lastUserIndex, which makes fallbackIndex >= 0) and
is removed.
partRowAnchor reuses transcriptRowIds instead of re-deriving the row
list; the anchor choice (message row first, else the last part row) is
unchanged.
The extra `existingPartRowIndices.get(message.rowId)` clause matched a
fetched fold as soon as its FIRST row was held as a part of another
bubble, short-circuiting partRowAnchor's every-row-held rule. Any unheld
row in that fold (e.g. the final reply) was then dropped and never
rendered. For single-row messages partRowAnchor already covers the
part-row match, so the clause only added the bug.
Add an it.each row for that shape and make the table assert explicit
merged ids instead of branching on a `kept` flag.
Keep the headline spec (red without the collapse) and the message.start
occurrence-bound spec (red without the guard); the distinct-segment and
tool-row boundary specs already pass on main, so they pinned nothing new.
The part-row anchor dropped a fetched fold as soon as ANY of its text rows
was already held, so a fold carrying a tool call and narration the store
did not hold lost both (Enough1122's review on #124472). Anchor through
the part rows only when all of the fold's source rows are held; otherwise
append it as before. Also drops the 'ponytail:' note this replaces and
folds the case-B spec into the existing test as a parametrized pair.
Refs #123801
Co-authored-by: finn763 <165816600+finn763@users.noreply.github.com>
Backfill merge matched only message-level row ids. A hydrated fold is
addressed by its first source row while the final reply rides inside as
a text part sourceRowId, so an overlapping older page appended the fold
next to the settled live bubble and the reply rendered twice. Closes#123801
(cherry picked from commit 50021e8c0a668797d54db0874c6cc4662d702369)
message.start starts a new occurrence while keeping the prior messages (a
chained or prompt-less turn) and resets interimBoundaryPending. Scanning back
only to the prompt row let interim('X') -> message.start -> delta('X') ->
complete('X') match the PREVIOUS turn's interim, settle it and delete this
turn's live bubble.
Gate the scan on the same flag collapseDuplicateFinalAfterToolInterim already
requires — it is exactly 'an interim was sealed since the last message.start' —
and add the same-text restart regression (fails without the guard). Copilot
review on #126023.
(cherry picked from commit 45ced2713efa4cd92acae3783205cdf94c6f5e72)
A reply the client sealed as an interim before it had the text on screen
(the interim materialized its own bubble because a superseded attempt's
frames had cleared the live stream) is painted twice when the turn then
streams the same text and completes: the sealed interim above, the settled
bubble below, while the store holds one row (#123801).
An interim whose text IS the final text is this turn's reply, not a second
segment, so completion now settles onto it and drops the live twin. Interims
carrying tool rows are never collapsed, and a distinct segment keeps its own
bubble.
Reproduced deterministically on main; the regression test fails without the
fix and the 222-file use-message-stream / chat-messages / chat suite passes
with it.
(cherry picked from commit f2980a9c9868a41e4ef3bf216c161acbc260e61b)
publish_runtime_status enqueues writes on a worker thread, so reading the
status file right after the reconnect raced the writer on CI (KeyError 'slack').
Startup only queues an adapterless platform when it can heal on its own (an
unregistered plugin with a bot credential), but the reconnect watcher's
`if not adapter:` branch kept every queued platform retrying forever. A
platform that became builtin-probe-failing or registered-but-None after being
queued (e.g. queued from a runtime fatal or startup connect failure) would
re-warn at the backoff cap indefinitely (#5196 fleet nodes).
Move the predicate into GatewayAdapterLifecycleMixin._adapter_may_heal and use
it at both sites: the watcher now marks such a platform fatal/needs_attention
with the non-retrying message and drops it from the queue. The late imports
of _BUILTIN_ADAPTERS / platform_registry / _platform_has_bot_credential keep
the same seams tests patch.
Tests: fold the builtin-missing-creds "fatal, not queued" case into the
missing-adapter test, run the heal test straight-line, and pin the watcher
dropping a registered plugin that still returns None. Still 2 tests.
The previous commit queued every enabled platform whose adapter came back
None. Some of those cases cannot heal without a config change: a builtin
whose requirements probe fails on missing creds (BlueBubbles, QQBot), a
Signal config that fails validation, a registered plugin that returns None,
or an empty bot credential. The watcher then re-warned about each of them
forever at the 5-minute backoff cap. That is the #5196 fleet case, where one
shared config.yaml runs on nodes holding only some of the credentials.
Now startup queues a platform only when it is not a builtin, no plugin has
registered it, and it has a bot credential. Every other adapterless platform
is still flagged (adapter_unavailable, needs_attention) but is marked fatal,
without the "Retrying" claim, and is not queued.
Also:
- One shared _adapter_unavailable_message() so the reconnect watcher keeps
the actionable plugin/deps/credentials hint instead of overwriting it with
a short message after the first retry.
- The "queued for retry" log counts startup_retryable_errors, the list it
names, rather than _failed_platforms (which includes adapterless entries).
- The all-adapterless warning now mentions background retry.
- Tests share one _runner() helper. The heal test has a builtin-with-missing-
creds row that must not be queued.
An enabled platform whose plugin failed to load (no adapter) was only
flagged at startup and never retried, so a transient plugin-load failure
left it down until a manual restart (#126356). Queue it in
_failed_platforms as retrying/adapter_unavailable (still needs_attention)
and keep it queued with backoff while _create_adapter returns None, so the
existing reconnect watcher connects it once the plugin registers. It is no
longer parked as fatal, so the gateway no longer stays degraded after heal.
Fixes#126356
Co-authored-by: Zheqing Zeng <26277217+zengzheqing@users.noreply.github.com>
prune_never_active_keyed_sessions now returns (deleted, entries_dropped,
skipped) from its own guarded-row list. `sessions prune --never-active`
printed candidates-minus-deleted, which lumped rows that vanished
concurrently in with live-turn skips and needed an "or already gone"
hedge; it now reports only rows spared by a live turn or compression lock.
The docstring claimed a failed sweep rolls back whole; the row sweep is one
transaction but routing entries are dropped in a second write after it
commits, so say that.
The kept guard test now also covers a candidate that vanishes between
listing and the sweep: its routing entry must still be dropped (the r1
per-row loop left it behind, so the gateway would resume a nonexistent id).
Its contract is a boolean "deleted or not" (callers like the CLI /new and
/clear hygiene path treat False as "keep it"), and every other non-candidate
(messages, title, children) returns False. Raising SessionActiveWriteGuardError
here forced every caller to grow a try/except for what is just another reason
the row is not empty-and-idle.
The per-row delete_session loop ran N write transactions and N routing scans,
and a row that vanished between list and delete (concurrent delete) returned
False and kept its routing entry alive, pointing the gateway at a nonexistent
id. delete_sessions already has the exact delete_session semantics (delegate
cascade incl. guard scope over delegate children, branch orphaning, messages,
system-prompt GC, transcript files) in ONE transaction and reports guarded rows
via skipped_ids. Routing entries are now dropped once, after the commit, for
every candidate except the guarded ones; an aborted sweep rolls back whole, so
no route is stranded and the mid-loop abort test is obsolete.
`hermes sessions prune --never-active` now tells the user how many candidates
were skipped (mid-turn or already gone) instead of silently reporting less.
Keep three tests: guarded row survives with its routing entry, a mid-loop
abort strands no live row, and delete_session_if_empty refuses under a
lease. The compression-lock case exercises the same _guarded_ids path as
the turn-lease case, so it adds no coverage. Also correct the abort
test's docstring (it described the ordering backwards) and drop an
unused import.
The never-active keyed prune deletes the row first and drops its routing
entry only once the delete succeeded (row-then-entry), but the docstring
said entry-then-row. hermes_state_errors imports only stdlib, so the
function-local import has no cycle to dodge; hoist it to module top like
hermes_state_sessions already does.
The write-guard refusal (#123583) covers delete_session/delete_sessions/
prune_sessions, but two deletion paths still mutate a row a live turn
could own:
- prune_never_active_keyed_sessions called delete_session without
exclude_active_write_guards and dropped the routing entries up front.
The never-active predicate reads committed state, so a keyed row whose
first turn lease is already held — its messages not yet flushed — looks
empty and is deleted mid-turn; its gateway routing entry died with it.
Now the prune skips guarded rows (guard errors are per-row, not fatal),
and routing entries are removed only for rows actually deleted.
- delete_session_if_empty had no guard at all: its emptiness predicate
(no messages/title/children) shares the same committed-state window.
The in-transaction _guarded_ids check refuses like the guarded deletes;
the CLI's broad except already treats any refusal as keep-the-row.
Tests: two new prune invariants (guarded row survives with its routing
entry; compression lock protects too) and an empty-delete refusal case in
the write-guard suite. hermes_state: 1386 passed; 4 pre-existing macOS
mount/barrier/snapshot failures reproduce on pristine main.
(cherry picked from commit aa579f0cf9a7bc991b675a6d45e1b320fa208234)
Why: sessions.max_export_messages: 0 is the documented escape hatch for big
stores; resolving the limit before the bare export's search_sessions scan
means a disabled guard costs nothing. A limit-0 id on the existing bare
guard test counts the scans so a regression back to scan-then-check fails.
Why: the user-facing refusal lived in a separate export_too_large_message()
helper while str(exc) still said "active messages" — a stale count wording
(the guard counts every stored row) that any new caller using str(exc)
would surface. Moving the text into the exception makes str(exc) the one
refusal and drops the facade-level helper; CLI and console now use str(exc).
Also name the session in the resume refusal's CLI escape hatch:
`hermes sessions export --format md --session-id <id>` — without
--session-id the md export writes every session, not the one too big to
resume.
`hermes sessions export` and the console's `sessions export` each
re-implemented the per-session max_export_messages loop and carried a
near-identical refusal string, so a wording or limit-resolution fix had to
land twice. SessionDB.assert_exports_safe resolves the limit once and
asserts each id; export_too_large_message is the single refusal text.
The CLI now resolves the limit before the bare-export search_sessions
scan, so `max_export_messages: 0` skips that scan entirely instead of
listing every session only to check nothing. The round-trip test now
asserts the refusal names the config key, and the export-guard docs say
the cap counts every stored message, not just active ones.
The resume refusal told users to run plain `hermes sessions export`, but
that JSON/JSONL backup is now capped per session by
sessions.max_export_messages and counts archived rows too — exactly the
huge sessions that hit the resume guard get refused again. Recommend the
two paths that do not materialize the backup in memory: the dashboard
Sessions page's streaming Export action and `--format md` (display
transcript, which skips the export guard).
The backup now loads every stored row, archived generations included,
but `_cmd_export` skipped the per-session `assert_export_safe` guard
that the console export applies. A heavily compacted session with a
small live tail could therefore load an unbounded archive into memory.
All three selection paths (`--session-id`, filters, bare export) now
run the `sessions.max_export_messages` guard over the stored row count
before loading anything, and print the refused session id. Transcript
views keep their display projection and are not guarded, as before.
website/docs/user-guide/sessions.md no longer says JSON/JSONL exports
are live-only. It now says they carry every stored row, including
compaction-archived and rewound or edited-away messages, with their
flags. It also says an import restores those rows as archived, and
that the guard applies to the backup.
(cherry picked from commit 36cdc89e342b7bda4e362294c1bd0ba5d44fed6d)
For --format jsonl/json (the documented "backups, machine round-trip"
format), _collect_sessions called export_all / export_session with
include_compacted=False and the default include_inactive=False — the
live-only projection. In-place compaction (on by default) archives
earlier turns as active=0, compacted=1, so a backup of any compacted
session held only the summary plus the live tail, and restoring it via
import_sessions (or the dashboard import) lost every archived turn.
Commit 85deb4553a (#122679) fixed exactly this, but only in
console_engine's export; the primary CLI sibling was missed.
The backup path now uses the transfer projection (include_inactive=True:
every row with its active/compacted flags, which import_sessions
restores as archived). Transcript views (md/--only) keep the display
projection.
(cherry picked from commit 9ecb273f17e098bdd497a228ed951c0802aa8fd8)
All three ownership-lost call sites in _classify_delivery_outcome computed
ladder_rung=is_retry_run(job) themselves and passed job["id"]; the helper is
the only place that needs the flag, so pass the job and derive it inside.
That removes the duplicated expression (and a mis-indented continuation
line at the third call site) and drops the now-unused function-level
is_retry_run import in _classify_delivery_outcome. The conditional
ladder_rung splat is kept so mark_job_run fakes without the kwarg still
see the same call shape as before. No behaviour change.
test_ladder_reruns_do_not_spend_extra_repeat_budget only exercised the
finish and crash tails, so the dead-worker recovery fix in
scheduler_worker_failure and the ownership-lost fix had no test that fails
without them. Add "worker" (record_unknown_worker_outcome on an unknown
execution) and "ownership_lost" (transport-cancelled rung recorded as
interrupted) rows; each asserts repeat.completed stays at 1. Both rows fail
against the pre-fix source.
_record_fire_ownership_lost is the last terminal write on the claimed
snapshot that still counted a ladder re-run toward repeat. When a re-run of
an unreachable-model occurrence is cancelled at the transport (dashboard
drain) while we still own the claim, it records the interruption through
mark_job_run, which bumped repeat.completed a second time for an occurrence
that had already counted -- the same bug the previous commits fixed for the
finish, crash, dispatch-failure and dead-worker tails.
The helper now takes ladder_rung and forwards it (conditional kwarg, same
idiom as the sibling paths), and its three callers in _run_one_job_body pass
is_retry_run(job). The shutdown/mark_interrupted path carries no snapshot
and is left as is.
The flag was count_run=True, and mark_job_run passed
count_run=not ladder_rung, so the same idea had two names with opposite
polarity. Use ladder_rung on _advance_after_run too. The docstring now
says when the bump is skipped, and the inline comment lists both reasons a
run is not counted again (a ladder re-run and a pre-claimed finite
one-shot). No behaviour change.
The previous commit stopped a re-run from the unreachable-model ladder from
counting toward repeat, but only on the normal _finish_completed_run path.
The other places that record a failed fire on the same claimed snapshot
still counted it: the crash/interrupt tail of _run_one_job_body, the
pre-handoff dispatch failure in run_one_job, and the recovered unknown
worker outcome in scheduler_worker_failure. A re-run that crashed or failed
to dispatch therefore still bumped repeat.completed a second time for an
occurrence that had already counted.
All three now pass ladder_rung=True when is_retry_run(job). The
worker-failure job is the same run_one_job snapshot, which carries
_scheduled_instant (stamped by the due scan and copied by
_process_due_job), so is_retry_run applies there too. Manual runs carry no
_scheduled_instant and still count.
The kept test gains a "crash" row: a re-run whose run_job raises must leave
repeat.completed at 1.
A recurring job with a finite repeat count (for example repeat=3, every
24h) lost one repeat slot for each automatic re-run after a fire that
could not reach the model. The failed occurrence counted, and then each
5/15/30-minute re-run of that occurrence counted again, so a half-hour
outage finished a repeat=3 job with zero model calls. With
cron.retry_unreachable off, the same outage costs only the one failed
occurrence.
Cause: cron/jobs.py:_advance_after_run bumps repeat.completed on every
mark_job_run, and the re-runs parked by
cron/unreachable_retry.py:plan_retry go through it like scheduled
occurrences. cron/unreachable_retry.py:will_retry also treated a re-run
on the last slot as the job's final run.
Fix: cron/scheduler.py:_finish_completed_run passes ladder_rung=True
when the dispatched instant is the parked re-run instant (new
is_retry_run, which compares the dispatch snapshot's _scheduled_instant
with the ladder state). mark_job_run then calls _advance_after_run with
count_run=False, and will_retry leaves that run out of its final-run
check the same way. The first failed occurrence still counts, and manual
runs still count.
(cherry picked from commit 27038f7064bb611811efee8c527dddecd4fcb57b)
The test previously asserted only wall-clock elapsed < 2s, which passes
whether or not the drain is skipped because the fake stream ends
immediately. Record any read past the terminal frame and assert there
were none, so re-enabling the drain without a shutdown-capable socket
fails deterministically. Also inline the single-use _post_terminal_socket
helper at its call site (keeping the exception guard, since the drain
sits outside the inner try; _socket_from_response already handles a
None response), and drop `x and` / bare-truthiness preambles that the
following set-equality asserts already imply.
Fakes without .response never reach the post-terminal drain (no
interruptible socket => drain skipped), so the StopIteration-time waits
were unreachable and the trailing raw_closed.wait was redundant with the
close-thread assertion. Rename the test to say what it actually pins.
_post_terminal_socket duplicated the response -> network_stream/stream ->
connection-candidate walk from _shutdown_stale_attempt_socket. Hoist it into
agent_runtime_helpers._socket_from_response (next to _socket_from_candidate)
and use it from both sites; each caller keeps its own error handling. Drop
the dead event_stream argument: ManagedLlmStream has no .response, only the
raw SDK stream does.
Fold watchdog.start() into the drain try so cancel/join lives once in the
finally (join only when a thread was actually started), replacing the
duplicate start-failure branch and its suppress-wrapped join.
Close guard: relay_llm.stream is ManagedLlmStream, whose close() always
exists and owns the provider stream, so the raw stream only needs closing
when construction failed (event_stream is None). Behaviour-equivalent to the
callable(close) probe.
_close_event_stream's new bool return has no caller that reads it, so
restore the original None-returning shape (and the original comment).
The suppress(Exception) blocks around logger.debug/warning calls in the
post-terminal drain guard against nothing real: logging already swallows
handler errors, and wrapping them hides genuine bugs in the arguments.
Fixes#127390.
Co-authored-by: Eva <239388517+100yenadmin@users.noreply.github.com>
The standalone payload builder imported markdown inside the per-chunk loop
under suppress(ImportError), which read as though the import could fail per
chunk. Try it once before the loop and branch on the result.
_standalone_send seeded `result = None` and returned it after the loop,
suggesting a possible None return. truncate_message always yields at least one
chunk, so the loop always runs; return the failing result directly and the
last result otherwise. No behaviour change.
An edit carries m.mentions twice: once inside m.new_content and once at the
top level. When a budget-sized edit names many distinct Matrix IDs, that
fixed part alone could pass the content budget. Shortening the fallback body
then had no room to work with, and the edit still went out at about 54 KB,
which an encrypted room would reject with M_TOO_LARGE.
When the edit is still over budget with an empty fallback, it now drops the
top-level mentions copy. That copy only drives the edit's own notifications,
and m.new_content keeps the full list.
The event-size tests move to tests/plugins/platforms/matrix/test_event_size.py,
mirroring the plugin source tree, and gain a mention-heavy text. The
tests/gateway/test_matrix_message_length.py file is back to main apart from the
env-override value that the new ceiling requires.
(cherry picked from commit 4b06bcb7dbffb732bef6e877cb66dea2fbff8c73)
The homeserver rejects any event whose canonical JSON is over 65,536 bytes
(M_TOO_LARGE), but Matrix chunked outbound text on characters, 16,000 by
default with a 65,535 ceiling. Each message carries its text twice (body plus
formatted_body HTML), and an edit carries it four times (the "* " fallback
plus m.new_content).
On the default config this caused three failures:
- A formatted chunk of CJK text came to about 95 KB and was rejected, so
_send_with_retry fell back to a plain 3,500-char truncation.
- A streamed edit near the limit came to about 80 KB even for English, so
edits failed mid-stream.
- The standalone sender (cron without a running gateway) posted each
16,000-char chunk as one event, and hit the same failure on non-Latin text.
The fix measures Matrix text in UTF-8 bytes, the unit of the cap:
- The adapter's message_len_fn now returns UTF-8 bytes, so send() and the
stream consumer's split and progress-edit budgets count the same unit.
- A chunk gets a third of a 45,000-byte content budget, which leaves room for
the event envelope and E2EE's 4/3 base64 inflation. That makes 15,000 the
default and the ceiling for max_message_length, so English chunks stay
about the same size.
- _build_text_message_content drops the HTML when escape-heavy text would
push the content past the budget.
- An edit that would still be too large keeps its full m.new_content and
shortens only the top-level fallback, which edit-aware clients never
render.
- The standalone sender re-chunks on UTF-8 bytes and applies the same HTML
guard.
(cherry picked from commit cab00c94175955a68ecd4d38c5748d63148ca6be)
_is_linked_worktree_checkout spawned git twice (git-dir, then
common-dir) while _defer_shared_worktree_cleanup inlined its own
one-spawn copy of the same test. Move the single
`rev-parse --git-dir --git-common-dir` call into the helper so both
callers share one definition and one subprocess, and drop the now
unused _git_dir wrapper.
All three worktree cleanup sites (task cleanup, parent cleanup, gc
backstop) wrote the same two-call guard: check the path is a linked
worktree, then check whether another live task still uses it. Fold both
into _defer_shared_worktree_cleanup so each site makes one call.
The linked-worktree check now asks rev-parse for --git-dir and
--git-common-dir in one spawn instead of two. _is_linked_worktree_checkout
stays for its other caller in worktree creation.
The cross-board test patched Path.iterdir before anything ran, so the
real sibling scan and its no-id-exclusion query were never exercised and
a swallowed crash looked the same as fail-closed. It now first proves a
live task on another board keeps the dir under a different id and under
the same id, then blocks board listing and checks the "unknown" reason.
The gc test also re-runs cleanup after the dir is removed and checks no
new deferred event is written, pinning the is_dir-before-scan ordering.
The shared-use guard ran before the managed-scratch / linked-worktree
checks, so an unmanaged scratch dir (a board default_workdir) or a
non-worktree path that is never removed still scanned every board DB,
logged "Deferring workspace cleanup" and wrote a deferred event for
nothing. Completion, deferred parent cleanup and the gc backstop now run
containment first and only defer inside the branch that would delete.
A sibling board DB that cannot be listed or read still fails closed, but
it was reported as a live sharer, sending the operator after the wrong
cause. Log the failing DB path and error and record reason "unknown" in
the event payload ("shared" for a real sharer). The event write now only
swallows sqlite3.Error.
Reuse kanban_db_connect._main_db_file and _kb.boards_root() instead of
hand-rolled copies, and share one terminal-status SQL fragment.
Trim the salvaged tests to two: gc keeps a scratch dir a live same-board
task still uses (and records the deferred event), and cleanup refuses
the delete when sibling boards cannot be listed while a named board's
live task uses the dir (the fail-closed path #126617 missed). The plain
same-board cleanup, cross-board and same-id cases go through the same
_workspace_in_use_by_other scan.
Every connection reaching _conn_uses_path uses sqlite3.Row (the board
connect and the read-only sibling connect both set it), so the
_row_path try/except fallback to row[0] was dead; index by name.
Run the scratch guard only when the directory still exists: an
already-removed dir otherwise opened every board's kanban.db and logged
a spurious "deferring" warning plus a workspace_cleanup_deferred_shared
event for nothing. Tmux and parent cleanup now run on the normal path.
gc, completion/archive cleanup and deferred parent cleanup rmtree'd a
task's workspace_path as soon as that task went terminal, even when
another non-terminal task (on this board or a sibling board) pointed at
the same directory, destroying its in-progress work.
Before removing, compare the resolved path against every live task on
this board and on every other board's kanban.db; if any still uses it,
skip the delete, log, and record a workspace_cleanup_deferred_shared
event. When the sibling-board set cannot be read, refuse the delete.
Salvaged from #126513 (squash of its commits).
Fixes#126508.
