Commit Graph

45218 Commits

Author SHA1 Message Date
funky-xamarin
44a0fe0359 fix(desktop): respect RTL profile rail edges and wheel direction 2026-09-27 12:48:54 -05:00
funky-xamarin
24bbdab4c7 fix(desktop): reveal clipped profile rail edges and pin actions 2026-09-27 12:48:54 -05:00
Adolan
a88dce6e2d fix(desktop): label the messaging platform enable switch 2026-09-27 12:48:49 -05:00
Cursor Agent
c3b24b74ba test(desktop): pin backslash math and the CJK-variable tradeoff
A span whose body is a backslash command is real math, and a CJK
variable inside one equation must still escape while the next span stays.

Co-authored-by: Yun. <fangyun1008@gmail.com>
2026-09-27 12:48:18 -05:00
Cursor Agent
f77bf679cb fix(desktop): keep inline math spans when CJK prose follows them
escapeCjkProseDollars treated a real closing dollar as the next opener,
so CJK text between two formulas escaped the first equation's closer.

Co-authored-by: Yun. <fangyun1008@gmail.com>
2026-09-27 12:48:18 -05:00
chelsealong
b9e2ddde05 fix(desktop): read inline preview color-scheme from the same source as its tokens
InlineHtmlFrame derived colorScheme from useIsDark(), which reads the
.dark class through React state that only updates a render after
applyTheme() has already mutated the DOM (use-theme-epoch.ts's own
comment documents this ordering: "a child's effect runs before the
provider's applyTheme"). collectThemeBridge() reads the CSS token
values live via getComputedStyle() on every render, so a render caught
between the DOM mutation and the epoch-triggered re-render built a
frame with fresh dark tokens but a stale light color-scheme -- a
transparent iframe with color-scheme:light still paints its canvas
white, so the widget showed near-white text on a white canvas (#123048).

Move the color-scheme read into collectThemeBridge() itself, off
document.documentElement.dataset.hermesMode (the same attribute
applyTheme() sets the token values from, and the pattern
lib/selection-copy-colors.ts's renderedMode() already uses for the
same reason), so the scheme and the tokens it decorates always come
from one synchronous read.
2026-09-27 12:48:11 -05:00
kshitijk4poor
6f7a7991bb test(email): pin both spf clause orders and name the auth-results test for what it covers 2026-09-27 20:49:08 +05:30
kshitijk4poor
3f4533b9de fix(email): read spf/dkim verdicts from their own Authentication-Results clause
The r3 fold scoped only the dmarc verdict to its clause. SPF and DKIM
still came from a whole-string, last-match-wins regex scan, so the same
quoted/comment smuggle closed for dmarc still authenticated a spoofed
From (GHSA-rxqh-5572-8m77), e.g.
  spf=fail smtp.mailfrom="x spf=pass smtp.mailfrom=example.com "@evil.test
  spf=fail (spf=pass) smtp.mailfrom=a@example.com
  spf=fail smtp.mailfrom=a.spf=pass@example.com
  dkim=pass header.d=evil.test header.i="x header.d=example.com y"@evil.test

Every verdict now comes from the leading method=result token of its
own clause (from _ar_clauses, comments dropped), and its domains only
from that clause. Properties are read by a token scanner that consumes
quoted-strings and other key=value tokens whole, so quoted contents are
never read as properties while a quoted value still is
(header.from="example.com"). SPF fails closed on more than one spf
clause; DKIM accepts any single dkim=pass clause whose own header.d
aligns (multi-signature mail is normal), never mixing clauses. The
whole-string methods/props (methods["dmarc"] was dead) are gone.

Also: a stray ')' at depth 0 is now unbalanced (it split header.from
out of the dmarc clause); a From with more than 64 '(' takes the silent
empty-sender drop instead of a parseaddr RecursionError logged as an
error; the cap test asserts the cap directly instead of wall-clock
timing; the empty-From drop assertion moves next to the other
_extract_email_address rejects; and the untested >1-dmarc, unbalanced
and backslash-escape rules get reject strings.
2026-09-27 20:49:08 +05:30
kshitijk4poor
517a97cd31 fix(email): cap From length, quote/comment-aware dmarc clauses, keep commented names
Round-3 review of the #124322 salvage:

- stdlib parseaddr is pure Python and superlinear on hostile input: a
  100KB `From: <a@a@...` held the GIL ~1s per message (main ~0), and any
  remote sender reaches it before auth. Refuse values over _MAX_FROM_LEN
  (2048; RFC 5322 lines cap at 998) right after unfolding so they take the
  existing empty-sender drop. The timing assertion now times
  _extract_email_address itself instead of a private regex.

- The dmarc clause split ignored quoted-strings and stripped comments one
  level only, so an attacker-controlled SPF-passing envelope sender like
  smtp.mailfrom="x;dmarc=pass header.from=example.com x"@evil.test planted
  a fake dmarc=pass ahead of the real dmarc=fail and authenticated
  admin@example.com. Split clauses with a quote- and nested-comment-aware
  scanner (comments dropped, quoted values kept so header.from="x" is still
  read and unquoted), and fail closed on an unbalanced value or when more
  than one clause starts with dmarc=. Tests pin the smuggled clause,
  reason="a;b", the nested comment, a quoted aligned header.from, and that
  every header.from in the dmarc clause must align (all->any goes red).

- `Doe, John (CEO) <j@x>` was dropped because the fallback refused any
  paren. Strip (comments) from the display part first; `attacker@evil.test
  (c) <victim>` stays rejected by the '@' rule.

- Results without '@' (`John` -> `john`, `a\"b <v@x>` -> `a\`) were
  dispatched as sender ids; return "" so they take the drop path.
2026-09-27 20:49:08 +05:30
kshitijk4poor
b821f8637b fix(email): linear, mailbox-safe From fallback; one-clause dmarc verdict
The r1 malformed-From fallback regex ([^<>\s]+@[^<>\s]+) backtracked
catastrophically on `From: <a@a@a@...`: 32KB held the GIL ~23s, and any
remote sender reaches it before auth. Capture <([^<>\s]+)> instead and
check the '@' in Python (same accepted set, 100KB in ~3ms).

The fallback also mapped multi-mailbox / group / comment-prefixed From
values (`attacker@evil.test, <victim@x>`, `Grp: a@evil; <victim@x>`,
`attacker@evil.test (c) <victim@x>`) to the bracketed victim, which a
dmarc=pass without header.from then authenticated. Only fall back when the
display part has no ; : ( ) and no '@' unless it is exactly the bracketed
address; `Doe, John <j@x>` and `j@x <j@x>` still resolve. The rule now lives
only in the docstring (the old inline comment was wrong).

dmarc: strip (comments) before splitting clauses, and take the verdict and
header.from from the same first clause that starts with dmarc=, so
`dmarc=pass (p=none; sp=none) header.from=evil.test`, a later dmarc=pass
clause after dmarc=fail, and duplicate misaligned header.from are rejected,
while `arc=pass (dmarc=fail ...); dmarc=pass header.from=<ours>` passes.
Property clean-up is shared via _auth_props.

The empty-sender drop now runs right after address parsing and gets an
assertion (it was untested). Tests extend existing ones (no new tests).
2026-09-27 20:49:08 +05:30
kshitijk4poor
2d44f5512b fix(email): keep malformed-but-real From values, drop empty senders, scope dmarc header.from
Strict parseaddr returns '' for common RFC-invalid From values that the
old regex resolved: an unquoted address as display name
(john@example.com <john@example.com>), an unquoted comma (Doe, John
<j@example.com>), or a@x.test <b@y.test>. Allowlisted senders using such
clients were silently dropped, and under open access every one of them
shared an empty chat_id. Fall back to the bracketed address only for the
unambiguous shape (no quotes, exactly one <...> pair), so the quoted
display-name spoof still resolves to the attacker. _parse_fetched_message
now drops messages whose From yields no address instead of dispatching
an empty identity.

The dmarc header.from alignment check read header.from from props merged
across all clauses, so a later dkim clause's header.from could override
the dmarc clause's value. Read it from the dmarc clause only, and pin the
misaligned dmarc=pass rejection in the existing dmarc test (previously
no test covered it). Also drop a redundant _domain_of ternary, trim the
_extract_email_address docstring and a duplicate test assertion.
2026-09-27 20:49:08 +05:30
kshitijk4poor
54b0f5db25 fix(email): only trust dmarc=pass when header.from matches the parsed From
_verify_sender_authentication accepted any dmarc=pass verdict, even one
issued for a different domain than the From we parsed. That is what let
the quoted-display-name spoof ride on the attacker's own truthful DMARC
pass. When the trusted Authentication-Results names header.from, require
it to align with the From domain; otherwise fall through to the aligned
SPF/DKIM checks. Defense in depth for the #124322 parser fix.
2026-09-27 20:49:08 +05:30
kshitijk4poor
4556de4128 test(email): keep two From-parsing invariants from the #124322 salvage
The salvaged PR added seven overlapping tests. Keep the two invariants:
the quoted-display-name spoof resolves to the real addr-spec (red on
base), and a folded display name plus the ordinary forms (Name <addr>,
bare, mixed case) still resolve to the same mailbox. The E2E
parse->authenticate->dispatch tests only re-assert the parser result.
2026-09-27 20:49:08 +05:30
KeelTrace
d0c588972e fix(email): parse the From mailbox with the stdlib, not a first angle-bracket match
_extract_email_address took the FIRST <...> pair, so
From: "Victim <victim@example.com>" <attacker@evil.test> resolved to the
victim. The attacker's own domain passes DMARC truthfully, so the
allowlist (EMAIL_ALLOWED_USERS), pairing and session identity were all
evaluated against an address the sender does not control.

Use email.utils.parseaddr, which keeps the quoted text as the display
name and returns the real addr-spec. RFC 5322 folding is unfolded first
so a folded quoted display name is not mistaken for the mailbox.

Salvages #124322.
2026-09-27 20:49:08 +05:30
kshitijk4poor
230f89b47b fix(sessions): share the write-guard filter and trim repeated walks
prune_sessions hand-rolled the same "guarded by a live lease/lock"
comprehension as the new _guarded_ids helper, so the two could drift on
the next guard change. Move _guarded_ids next to _write_guards_reject in
the maintenance mixin and have prune call it.

delete_session walked the delegate tree up to three times in one write
transaction; compute the target ids once for both the guard check and the
expected-ids fence. delete_sessions ran a per-root guard walk for every
selected id; do one batched check over all roots and their children first
and only attribute per root when something is actually guarded.

The CLI export --delete message repeated "session 'X'" because the
exception text already names the session.
2026-09-27 20:49:04 +05:30
kshitijk4poor
e78e7ccdeb fix(web): translate the skipped-active toast and show one toast
selectedSessionsSkippedActive is required in Translations, but only en.ts
had it. Every locale typed `: Translations` failed tsc -b with TS2741, which
breaks scripts/build/web.mjs. Add a translation next to
selectedSessionsDeleted in each of them (ar.ts goes through defineLocale and
falls back to English).

useToast holds one toast and only supports success|error, so the skipped
toast replaced the success toast in the same tick and the deleted count was
lost. Show a single toast instead: success with the count when nothing was
skipped, otherwise one error toast that carries both counts. Also drop the
duplicated comment.
2026-09-27 20:49:04 +05:30
kshitijk4poor
d1849547e9 fix(tui): import SessionActiveWriteGuardError inside session.delete
session.delete's body is rebound onto tui_gateway/server.py's globals by
bind_module, so the module-level import in methods_session.py was never
visible to it. Any exception in the try block then raised NameError while
evaluating the except clause: a live-turn delete crashed instead of
returning 4023, and a plain DB error that used to map to 5036 crashed too.
Import it in the handler body, the same way the module's other rebound
handlers pull in their dependencies.

Also update the test stubs whose delete_session signature predates the
exclude_active_write_guards kwarg, and the bulk-delete endpoint test that
now receives skipped_active: [].
2026-09-27 20:49:04 +05:30
kshitijk4poor
173770144f fix(sessions): tell the user when a delete is refused for a live turn
The browse picker swallowed SessionActiveWriteGuardError into a generic
"Delete failed.", and the dashboard bulk delete only reported the deleted
count, silently keeping rows a live turn owns. Surface both: the picker
flashes that the session is active, and SessionsPage shows a toast with the
skipped_active count (new en key; other locales fall back to English via
defineLocale). Also move the api_server import into its sorted slot.
2026-09-27 20:49:04 +05:30
kshitijk4poor
1a6a9b66b9 fix(sessions): guard delegate children that a delete would cascade
delete_session/delete_sessions cascade-delete delegate children, but the
write-guard check only looked at the root. A guarded delegate child could be
removed out from under its live turn, and in bulk delete an active id that
was also another selected root's delegate child was reported in
skipped_active while the cascade deleted it anyway.

Check {root, *delegate children} via a small _guarded_ids helper: single
delete refuses, bulk delete skips the root, so the cascade never touches a
guarded row. Ports the delegate-protection idea from #124496.

Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
2026-09-27 20:49:04 +05:30
kshitijk4poor
adacbcc5fc fix(sessions): let guarded delete remove idle compression-ended rows
The new entry-side guard in delete_session/delete_sessions called
_write_guards_reject without allow_closed_compression_parent=True, so
_check_transcript_write_guards raised CompressionSessionClosedError for any
row with end_reason='compression'. That type is not caught by
_write_guards_reject, so every user-facing delete of a compressed parent
500'd and a bulk delete containing one rolled back the whole batch.

Pass the flag at both sites, matching prune (hermes_state_maintenance.py).
The lease is keyed on the lineage root, so a live turn on the tip still
blocks deleting its ancestor. Test 1 gains a compression-ended case (red on
the pre-fold file).
2026-09-27 20:49:04 +05:30
kshitijk4poor
3402644764 test(web): expect skipped_active in bulk-delete response
The endpoint now reports rows refused for a live turn; update the
event-loop test stub expectation to the new response shape.
2026-09-27 20:49:04 +05:30
kshitijk4poor
907e3188da fix(sessions): report bulk-delete rows skipped for a live turn
delete_sessions(exclude_active_write_guards=True) dropped guarded rows
silently: the web bulk-delete endpoint returned only a count and the
dashboard removed every selected row optimistically, so refused rows
reappeared on the next reload with no explanation.

The store now appends refused ids to an optional skipped_ids list inside
the same write transaction, the endpoint returns them as skipped_active,
and SessionsPage keeps those rows listed. Also hoists the
SessionActiveWriteGuardError imports to module top (hermes_state_errors
is stdlib-only) and drops the assertion-less lineage comment in the test.
2026-09-27 20:49:04 +05:30
shali10
40523600b0 fix(sessions): refuse to delete a session row a live turn still owns (#123583)
Refactor entry-side deletion refusal to execute in-transaction via
`_write_guards_reject(conn, sid)` (#123583), per maintainer review:

- Underlying `delete_session` and `delete_sessions` now accept an opt-in
  kwarg `exclude_active_write_guards=True` running inside `_do` write
  transaction, eliminating the race condition where a turn acquires the lease
  between check and delete.
- Raises `SessionActiveWriteGuardError` when refusing single delete, leaving
  the row untouched; `delete_sessions` atomically skips active rows.
- Checks both active turn leases and compression locks via the existing
  reclaim-aware `_write_guards_reject` helper.
- Covers all user-facing delete sinks:
  * Web `DELETE /api/sessions/{id}` -> 409 Conflict
  * Web `POST /api/sessions/bulk-delete` -> skips active rows
  * Web / CLI `prune` -> passes `exclude_active_write_guards=True` so lineage
    parents of active conversations are not pruned
  * API Server `DELETE /api/sessions/{id}` -> 409 session_active_turn
  * CLI `hermes sessions delete` & `export --delete-after-verified` -> exits 1
  * CLI browse picker -> refuses active delete
  * TUI Gateway `session.delete` -> 4023 error
- Conforms to rubric with 2 targeted invariant tests in
  `tests/hermes_state/test_delete_session_write_guards.py`.
- Updates user guide and web dashboard docs for 409 / exit 1.

(cherry picked from commit 2c037a7a79dc211b49bacc72e3140951ccf900cf)
2026-09-27 20:49:04 +05:30
kshitijk4poor
a7f146fd15 chore: map shali10 for salvage of #123725 2026-09-27 20:49:04 +05:30
kshitijk4poor
eb4c8efe60 test(simplex): fail loudly if the warned-set name drifts 2026-09-27 20:47:41 +05:30
kshitijk4poor
f9771a445d fix(simplex): warn about name allowlist entries once per process, as authz reads them
Both reconnect paths (gateway/run_adapters.py watcher and multiplex
secondary) build a FRESH SimplexAdapter before connect(is_reconnect=True),
so the per-instance _allowlist_warned flag never suppressed anything: a
daemon-down cold boot re-logged the warning on every backoff retry. Gating
on `not is_reconnect` would instead lose the warning when the first connect
fails. Dedup at module level keyed on (hermes_home_key(), frozenset(names)),
still checked before the connectivity probe. No shared warn-once helper
exists (plugin_compat.warn_once is compat-specific).

Read the value with platform_gate_env (the reader authz uses; differs from
get_scoped_secret when a scope is installed with multiplex off) and decode
JSON list literals with decode_json_list_literal like _coerce_allow_set, so
'["4","9"]' written by `hermes config set` no longer warns that valid IDs
are ignored.

The caplog test now builds two fresh adapters (first connect fails, second
succeeds) and asserts exactly one warning naming only 'alice'; it fails with
2 warnings against the pre-fold adapter.
2026-09-27 20:47:41 +05:30
kshitijk4poor
d00fb8b20b test(simplex): parametrize the contactId-vs-display-name authz test
The two SimpleX allowlist tests differed only in the allowlist value and
expected verdict; one parametrized test keeps both invariants and holds
the stack at two tests after the connect()-warning test was added.
2026-09-27 20:47:41 +05:30
kshitijk4poor
3a47e65ef3 fix(simplex): read allowlist name-warning profile-scoped, before the probe
The name-entry warning in connect() read SIMPLEX_ALLOWED_USERS via raw
os.getenv, while authz reads it profile-scoped. Under multiplexing a
secondary profile would warn about (or stay silent on) the default
profile's list rather than the one actually enforced. Use the module's
_get_scoped_secret + _parse_comma_list like __init__ does.

It also only fired on a successful non-reconnect connect: if the daemon
was down at cold boot the first connect() failed and every retry came in
with is_reconnect=True, so the warning never appeared. Evaluate it before
the connectivity probe, once per adapter via an instance flag.

Test: two connects (first fails) -> exactly one warning naming only
'alice' for scoped '4, alice' while os.environ holds 'bob'. Red on the
pre-fold adapter (0 warnings) and on a raw-os.getenv variant (names bob).
2026-09-27 20:47:41 +05:30
kshitijk4poor
1ec84a2dae fix(simplex): document contactId-only allowlist and warn on name entries
After #44729 SIMPLEX_ALLOWED_USERS matches only the numeric contactId, but
the docs still told operators display names work, and existing name
entries would silently stop matching. Update the docs and log a one-time
warning at first connect listing non-numeric entries that are now ignored.
2026-09-27 20:47:41 +05:30
kshitijk4poor
07c5310295 test(simplex): trim salvaged allowlist tests to the two invariants
Drop test_simplex_allowlist_rejects_colliding_display_name: it passes on
the unfixed base (the allowlist held the contactId, not the colliding
name), so it never guarded #44729. Drop the setup-prompt string check as a
change-detector. Keep rejects_display_name_only (red on base) and
accepts_numeric_contact_id (contactId path still works).
2026-09-27 20:47:41 +05:30
liuhao1024
4670467534 fix(security): remove mutable display-name from SimpleX allowlist check
The SimpleX sender allowlist (SIMPLEX_ALLOWED_USERS) previously matched
against both the stable numeric contactId (user_id) and the mutable
display name (user_name). Since any SimpleX contact can change their
localDisplayName / profile.displayName to match another user's, this
allowed an unauthorized contact to bypass the allowlist by setting a
colliding display name.

Remove the user_name check so that SIMPLEX_ALLOWED_USERS only matches
on the immutable contactId. Operators must use numeric contact IDs in
the allowlist.

Fixes #44729

(cherry picked from commit b4aa29da1567d45920f79aabdb36b44c5f87bde5)
2026-09-27 20:47:41 +05:30
kshitijk4poor
47a738da3b refactor(files): share the 409 mapping with the preview read; encode base64 after release
Co-authored-by: Benjamin PERRY <benjaminperry6@yahoo.fr>
2026-09-27 20:45:56 +05:30
kshitijk4poor
fc53717f72 fix(files): one live-DB refusal source; hold the lock through /api/files/read
Gate r2 Low cleanups (house rule: no aliases/shims):
- Drop the is_live_database_file alias; its point-in-time caveat now lives on
  has_live_connection.
- _refuse_live_database reuses offline_file_access's message (via _serve_offline),
  so a download 409 on state.db-shm names the main database like the read path;
  the verb is "serve" so it fits read/download/stream.
- /api/files/read reads whole files in-process, so _read_base64_file now holds
  offline_file_access through close (409 on a live DB; OSError stays 500). Only
  the streamed FileResponse routes keep the point-in-time check.
- That check takes the global _live_lock, which other threads hold across
  whole-file reads, so fs_download and the managed stream routes run it via
  asyncio.to_thread instead of stalling the event loop.
- _managed_readable_file docstring no longer claims a size cap;
  _read_file_reference returns (early, text) instead of a str|Expansion union
  sniffed with isinstance.

Co-authored-by: Benjamin PERRY <benjaminperry6@yahoo.fr>
2026-09-27 20:45:56 +05:30
kshitijk4poor
48196b0ceb fix(files): refuse live-DB downloads and share one sidecar-aware liveness check
FileResponse opens and closes the file in the dashboard process, so
downloading a live state.db (or its -shm/-wal) via /api/fs/download or
the managed-file read/download/media routes still cancelled the
connection's POSIX locks. Both now return 409 via is_live_database_file;
the registry lock is not held across the streamed response.

The main-or-WAL-sidecar rule now lives in one _live_main_key helper used
by offline_file_access, has_live_connection and read_header_bytes_preopen,
and the sidecar refusal names the main database the connection is open on.

@file previews hold _live_lock only for the raw read; token counting and
formatting run after release. The Linux lock test gains requires_wal
(Hermes uses DELETE mode on WAL-reset-vulnerable SQLite), covers the
download refusal, and drops an ambiguous conditional assert.

Co-authored-by: Benjamin PERRY <benjaminperry6@yahoo.fr>
2026-09-27 20:45:56 +05:30
kshitijk4poor
4901f1a6b1 test(files): gate live-DB lock test with platforms("linux")
The PR predates the removal of the linux_only marker; the collection hook
now rejects it outright, so the whole module errored at collection.
The test reads /proc/locks and is genuinely Linux-only.
2026-09-27 20:45:56 +05:30
kshitijk4poor
7955732e25 test(files): trim live-DB preview lock matrix to four cases
Keep one case per guarded route (@file, @folder, desktop fs_read_text)
plus one WAL-sidecar refusal (-shm); the remaining alias/wal variants
exercise the same offline_file_access keying path and only add runtime.
2026-09-27 20:45:56 +05:30
Benjamin PERRY
cd2bfda63f test(files): prove cross-process SQLite writes survive preview
Co-Authored-By: Hermes Agent / OpenAI Codex / gpt-6-sol <noreply@agents.invalid>
(cherry picked from commit 915447ca9d4de7e9d4f9f30c52fd2f1814619569)
2026-09-27 20:45:56 +05:30
Benjamin PERRY
af595ff8a6 fix(files): preserve SQLite locks during previews
Co-Authored-By: Hermes Agent / OpenAI Codex / gpt-6-sol <noreply@agents.invalid>
(cherry picked from commit 60d879c125177cd4cacdbc840f16c2e048b4199c)
2026-09-27 20:45:56 +05:30
kshitijk4poor
fb7eda7416 refactor(persistence): drop a dead digest pop and scope the transcript-write docstring 2026-09-27 20:45:44 +05:30
kshitijk4poor
b1bb9031e3 test(compression): expect the rotation handoff digest on compressed dicts
The rotation handoff now stamps each child row's stored-row digest, so
the exact-dict comparison must ignore DB_ROW_SNAPSHOT. Assert every
compressed dict carries one: this pins the non-flush restamp, which
otherwise only probes covered.
2026-09-27 20:45:44 +05:30
kshitijk4poor
f337631f43 fix(persistence): restore caller row state when any transcript insert rolls back
_insert_message_rows stamps _row_id and the stored-row digest onto the
caller's dicts inside the write transaction. Only append_messages_batch
restored that state on rollback. archive_and_compact, replace_messages
and the rotation handoff left the rolled-back id + digest on the dicts;
SQLite reuses the id, so a later flush found a digest mismatch on the
foreign row, adopted it and silently dropped the user's message.

Move the capture/restore into _execute_transcript_write, used by every
caller that inserts caller-owned dicts: each attempt starts from the
caller's state and a final failure restores it before re-raising.
(Rewind replacement and import insert dicts built inside the txn.)

Also: bind _message_row_params directly on insert instead of the
serialized-dict round-trip, import the public DB_ROW_SNAPSHOT /
CANONICAL_ROW names, set adopt=False once, and reuse target_row
instead of re-SELECTing when nothing was written.
2026-09-27 20:45:44 +05:30
kshitijk4poor
1a95a75b1a fix(persistence): adopt content only on legacy rows and stamp every inserted row
A legacy (no-digest) dict over a non-blank assistant row adopted the whole
decoded DB row: tool_calls / reasoning* / codex_* were overwritten with the
stored JSON (which still holds the escaped lone surrogate the sanitizer just
fixed, re-injecting it into the provider payload) and live-only fields were
popped. Resumed dicts (_rows_to_conversation stamps _row_id without a
digest) and compaction clones hit this path. Adopt content only, as before
this stack, via a content-only canonical handled like the metadata-only one.

_insert_message_rows dropped a clone's parent digest but only the flush
path restamped it, so clones made by archive_and_compact / replace /
rotation handoff / import reached the legacy path and the first live edit
after a clone was not persisted. Stamp the stored-row digest inside
_insert_message_rows (one batched SELECT, cold paths only; the flush path
statement count is unchanged) and drop the duplicate call in
append_messages_batch.

Define the _db_row_snapshot / _canonical_row keys once in
agent/message_metadata.py and import them everywhere instead of repeating
the literals.
2026-09-27 20:45:44 +05:30
kshitijk4poor
6e8aa00626 fix(persistence): version only owned columns so metadata writes keep row ownership
The row digest hashed every repair column, so a same-process metadata write
(reaction, display-kind stamp, api_content / codex reasoning backfill,
platform message id) made our own row look like a foreign winner. The
re-flush then adopted the stale DB row: a later live edit (the non-ASCII
strip recovery) was reverted, and unsanitized tool_calls/reasoning were
copied back onto the live dict.

The digest now covers only the owned (non-metadata) columns: it means "the
row is still what we last committed". Match -> write the live owned values
and hand over only presentation metadata the live dict lacks; mismatch ->
genuine other writer, adopt as before. The r3 "stored content equals the
durable form of live" special case is subsumed and removed.

Also:
- _insert_message_rows drops a carried digest when it assigns a new row id
  (compaction/replace/import clones carried the parent's version).
- append_messages_batch restores each message's _row_id / digest /
  timestamp and pops the adopted row at the top of every _execute_write
  attempt, so a rolled-back attempt cannot resolve to a foreign row.
- message_id is no longer synced onto live (int -> str flip, spurious
  platform_message_id).
- The JSONL divert strips both bookkeeping keys via one frozenset.
2026-09-27 20:45:44 +05:30
kshitijk4poor
43b1dd7c5f test(persistence): pin metadata-only adoption and stored-row digests
Extend the kept active-row test: a reaction between flushes must not replace
the live multimodal user content with its text projection (red on the previous
tip at the image assertion), and the reaction metadata is synced. The user row
carries an int message_id so the stored-row (TEXT affinity) digest path is
exercised.
2026-09-27 20:45:44 +05:30
kshitijk4poor
ca7e4c1636 fix(persistence): keep live content on metadata-only row changes
Same-process writers (set_message_reaction, display-kind stamping, api_content
backfills, codex reasoning update) change stored columns after a flush without
refreshing the live row digest. The next sanitize + re-flush treated that as a
concurrent winner and copied the lossy durable projection over live multimodal
content, dropping image parts and shifting the prompt-cache prefix. On adoption
we now keep live content when the stored content is just the durable form of
it and sync metadata only; a real concurrent content winner is still adopted.

The legacy blank-assistant path (dict with _row_id but no digest, blank DB row)
again only fills the row from live content, as on main, instead of running the
full canonical sync that wiped live reasoning_content/finish_reason/tool_calls.
Adoption on the legacy path is limited to a non-blank row.

Also: transcript_row_snapshot returns str (the partial-row branch had no
caller), serialization only runs on the digest-match branch that reads it,
stamping reuses hermes_state_common._id_chunks, and _MESSAGE_WRITE_COLUMNS is a
plain top-level import (hermes_state_messages imports this module lazily, so
there is no cycle).
2026-09-27 20:45:44 +05:30
kshitijk4poor
1a43a4ef48 fix(persistence): strip the adopted canonical row from provider payloads
Gateway/TUI/CLI callers pass their live dicts straight to
append_messages_batch, so a concurrent-winner adoption leaves the
decoded durable row (_canonical_row) on a dict that may later be sent
to the model. Treat it as persistence-only like _row_id and the digest
so the outbound builder and token estimator both drop it.
2026-09-27 20:45:44 +05:30
kshitijk4poor
4f6ab19304 fix(persistence): keep live multimodal content and hash stored rows
The row-addressed repair stamped the decoded durable row on every
resolved message, so after our own rewrite the sync copied the lossy
durable projection (image parts -> "text\n[screenshot]") back onto the
live dict: multimodal user/tool messages lost their images and the
prompt-cache prefix changed. Adopt the DB row only when another writer
won (digest mismatch) or on the legacy assistant path, as BASE did.

The insert-time digest hashed Python bind values, but SQLite affinity
rewrites them on storage (int message_id -> TEXT, float token_count ->
INTEGER), so live and DB digests never matched and in-place edits were
silently dropped. Hash the stored rows instead, only on the
append_messages_batch flush path that reads the digest (one SELECT per
batch), incrementally (type tag + length prefix) instead of via JSON.
Also skip the no-op UPDATE, fix the _write_columns comment/spacing and
drop the duplicate top-level Optional import (F811).
2026-09-27 20:45:44 +05:30
kshitijk4poor
4854225903 fix(persistence): version transcript rows by digest, not a row copy
The CAS row snapshot was a full copy of each message's durable payload
riding on the live dict. The rough token estimator priced it (about 2x
estimates -> premature compaction) and it doubled transcript memory.

Replace it with a 16-byte blake2b digest of the repair columns. The
compare now runs in Python against the target row already read inside
the BEGIN IMMEDIATE transaction, followed by a plain UPDATE. Also:
- add _db_row_snapshot to PERSISTENCE_ONLY_MESSAGE_FIELDS so the
  estimator and the outbound request builder both drop it
- derive _REPAIR_COLUMNS/_SYNC_FIELDS from _MESSAGE_WRITE_COLUMNS
- use hermes_state_common._placeholders
- drop the dead resume-path stamp (the SELECT has no token_count, so it
  was always None) and the dead tool name assignment in
  _decoded_repair_row
- keep the digest out of divert JSONL

The kept active-row test now pins estimate stability across a flush and
the survival of a concurrent writer's row. It goes red on the old
prod files and red when the digest compare is removed.
2026-09-27 20:45:44 +05:30
kshitijk4poor
18ee4578b7 test(persistence): trim sanitized-row dedupe tests to two invariants
Keep one test per invariant: active user/tool rows whose _db_persisted marker
was popped by the outbound sanitizer keep their _row_id and are not re-inserted
(the #123462 desktop/serve path), and archived user/tool rows are repaired in
place instead of appended. Both fail on b4410b4bad; the other four PR tests
covered edge branches and are dropped per the <=2 invariant-test budget.
2026-09-27 20:45:44 +05:30
kshitijk4poor
53e49c49dd fix(persistence): import Optional where transcript_repair uses it
transcript_row_snapshot annotates Optional, which was only reachable via the
PLUGIN-COMPAT re-export block at the bottom of the module. Internal code must
not depend on that revert-scheduled block, so import it with the other typing names.
2026-09-27 20:45:44 +05:30