The per-token hot path mirrors partial text, reasoning, and live tool
calls into STREAM_PARTIAL_TEXT / STREAM_REASONING_TEXT /
STREAM_LIVE_TOOL_CALLS without holding STREAMS_LOCK, while snapshot
readers (_snapshot_and_append_partial_on_error, cancel_stream) read them
under STREAMS_LOCK. This is safe under the CPython GIL for two reasons —
NOT because `+=` is one bytecode (it is not: `d[k] += s` compiles to
load / add / STORE_SUBSCR):
1. Single writer — only the streaming thread ever writes a given
stream_id's buffers, so there is no writer/writer race to tear.
2. Reader/writer atomicity — `+=` builds a complete new immutable str,
then the final STORE_SUBSCR binding it into the dict is a single
atomic bytecode, so a concurrent reader's dict.get returns either
the old or new *complete* string (strings are immutable), never a
half-built one. list.append / single-key dict writes are atomic too.
So a reader sees a complete-but-possibly-stale value, never a torn one,
and the snapshot is a best-effort partial reconciled by later journal/SSE
events.
Chose documentation over locking: pulling the writes under STREAMS_LOCK
would add per-token contention against readers copying large buffers and
entangle the documented LOCK -> STREAMS_LOCK ordering (deadlock risk) for
only a recoverable staleness window. Make the assumption explicit at the
write sites and the reader instead. No behaviour change.
Rollback: revert this commit (comments only).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three fixes for the CI failures (lint + test shard 1):
1. B009 lint fix (api/routes.py:6281): Replace getattr(session, 'profile')
with session.profile — the guard on line 6275 already ensures it exists.
2. B905 lint fix (api/routes.py:13843): Add strict=True to zip() of
_draft_stages slices. Both sides are always the same length (shifted
by one), so strict=True is correct.
3. _emit_slow format-arg fix (api/request_diagnostics.py:186-195):
Keep the prefix in the format string and pass the JSON payload as a
separate positional argument. The test at
test_issue1855_request_diagnostics.py:33 does
json.loads(caplog.records[0].args[0]) and the old code embedded the
prefix in the payload string, breaking the JSON parse. Now
self.logger.warning("%s %s", prefix, payload) — wait, actually
self.logger.warning(f"{prefix} %s", payload) — so args[0] is the
raw JSON string, same as the pre-_emit_slow code.
_probe_remote_gateway held _remote_probe_lock only around the cache
read and cache write; the network probe itself (_http_probe per path,
each up to ~2s) ran unlocked. On a cold cache a dashboard that fans out
to N panels fired N concurrent probe sets at the (possibly dead)
gateway, each blocking up to ~6s (#5455, #2476).
Wrap the existing lock in a Condition and add an in-flight set: the
first 'leader' thread marks the base_url in-flight and runs the probe
(via extracted _run_remote_probe), while latecomers wait on the
Condition for the leader's cached result instead of probing themselves.
The leader clears the marker and notify_all()s in a finally block, so
waiters are woken even if the probe raises (no deadlock); a bounded wait
budget lets a waiter fall back to probing itself if a leader hangs.
_reset_remote_probe_cache_for_tests also clears the in-flight set.
Expire the cache from the probe's COMPLETION time, not the leader's
entry time: walking every path of a hung gateway takes
len(paths) * timeout (~6s), which exceeds the 5s TTL, so
`entry_time + TTL` would store an already-expired cache line. Waiters
woken right after would then miss the cache and each re-probe the dead
gateway — collapsing single-flight and regressing latency to worse-than-
serial. A regression test drives a walk longer than the TTL and asserts
still exactly one probe set (the shipped happy-path test kept the walk
well under the TTL and so never exercised this).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
append_run_event() without an explicit seq called _next_seq(path), which
re-read and re-parsed the entire journal file on every append — O(n) per
append, O(n^2) over a run. Direct callers therefore paid a full file
reparse per event; only RunJournalWriter avoided it via its own in-memory
counter.
Introduce a module-level _SEQ_CACHE keyed by run-journal file path. The
per-path _lock_for(path) serializes same-path reserve->append so seqs
stay monotonic and file order matches; both RunJournalWriter and the free
append_run_event now draw from this one cache (_reserve_next_seq), so
seqs stay gapless even when both write the same path — the writer's
private counter is removed to keep a single source of truth. An explicit
caller-supplied seq advances the cache (_note_assigned_seq) so it can
never be re-issued. delete_run_journal() now also evicts cache entries
for the removed session so a run re-created at the same path restarts
numbering at 1.
Guard every structural _SEQ_CACHE access (reserve/note/evict) with a
dedicated _SEQ_CACHE_LOCK. The eviction sweep in delete_run_journal
iterates the whole dict; the per-path locks the append path holds do NOT
serialize against it, so a concurrent append on another session inserted
a fresh key mid-iteration and raised "dictionary changed size during
iteration" (the caller swallows it, but the eviction then aborts and
leaks stale seqs, so a re-created run resumed a stale number instead of
restarting at 1). A stress test races deletes against cross-session
appends and asserts no such error.
Rollback: revert this commit; _next_seq(path) re-read behaviour returns.
No on-disk format change, so existing journals stay compatible.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When cached session has no anchor_activity_scenes and the prefix reader
returns None (e.g. sidecar too large for prefix), the empty-scenes path
did not mark _scene_check_inconclusive = True. This meant an inactive
session with matching message counts would return False (cache fresh)
even though disk may have gained its first scene record.
Fix: the empty-scenes else-branch now sets _scene_check_inconclusive
when disk_meta_quick is None, matching the existing behavior in the
cached-scenes branch (line 3173). The code then falls through to the
full metadata comparison instead of serving a stale cache.
discussion_r3548650345
Two Greptile P1 comments from review of b5c0447a (tier3c):
1. Inconclusive Prefix Stays Fresh (discussion_r3548477859):
When _persisted_session_meta_prefix() returns None and cached has scene
records, the fast-path skipped scene comparison entirely and returned
False (cache fresh) for inactive equal-count sessions. Added
_scene_check_inconclusive flag that forces fall-through to the full
Session.load_metadata_only() comparison when the cheap prefix read
fails, preventing stale anchor_activity_scenes.
2. Prefix Check Misses Edits (discussion_r3548477915):
The 128-char content-prefix backstop in _read_profile_config_cached
only detected edits within the first 128 bytes of config.yaml. Replaced
with a full-content comparison that reads the entire file (~1-10KB)
and compares it to a copy stored in the cache entry. This detects
edits anywhere in the file at a fraction of the cost of yaml.safe_load().
Updated cache type hint from tuple[float, str|None, dict] to
tuple[float, str, dict].
Tests: 108 passed (41 reconciliation + 39 anchor-scene + 28 profile-config)
The fast-path scene comparison in _cached_session_lags_disk had two bugs
flagged by nesquena-hermes review:
1. Direction: used symmetric != instead of directional subset check.
When the cache had scene keys that disk lacked (cache-ahead), the old
code force-reloaded from disk, silently dropping un-persisted scene
data. Now mirrors master: reload only when disk is strictly ahead
(disk_keys.issubset(cached_keys)). The 'not disk_scenes' early-return
is also removed — empty disk scenes means disk is not ahead, so keep
the cache.
2. Key filtering mismatch: used raw set(disk_scenes.keys()) instead of
_anchor_scene_record_keys() helper which filters to truthy keys with
dict values. Now filters disk keys the same way as master, and uses
_anchor_scene_record_keys(cached) for the cached session object.
Also updated the tier3b content-prefix docstring to clarify it is a
best-effort backstop limited to the first 128 chars, not a full
integrity check.
Greptile P1 (discussion_r3548069113): the inode+mtime+size cache key
misses in-place rewrites on coarse filesystems. When a file is rewritten
in-place (same inode, same mtime tick, same byte size), the cache returns
the old parsed config for up to 60s.
Fix: on cache hit, read the first 128 characters of the file and compare
against the stored prefix from when the entry was populated. If they differ,
the content has changed — fall through to re-parse. Cost on hot path: one
128-char read + string comparison (negligible vs YAML parse).
_drain_loop read process_registry.completion_queue directly under a bare
`except Exception: continue`. A registry missing that attribute raised
AttributeError that was swallowed and retried with no backoff — a
100%-CPU tight loop with no log line. The loop now reads the queue via
getattr(..., None) and backs off on the stop event when it is absent
(mirroring streaming.py), catches queue.Empty explicitly for the normal
idle path, and logs a warning + backs off (_DRAIN_STOP.wait(1.0)) on any
other queue error so a persistent fault can neither spin the CPU nor stay
invisible.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The passkey-login success handler wrote its 200 JSON body without a
Content-Length header, unlike the password-login and logout handlers. Under
HTTP/1.1 keep-alive that response is unframed, so the browser's fetch().json()
waits for connection close and appears to hang until it times out.
Encode the body once and send Content-Length before end_headers(), mirroring
the /api/auth/login block. Header order is unchanged so set_auth_cookie's
Set-Cookie still precedes end_headers().
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The streaming worker called meter().begin_session(stream_id) but never a
paired end_session. A turn that produced zero output tokens (pre-flight
cancel, or a setup exception before the first token) left a _SessionMeter
in GlobalMeter._sessions forever: get_stats() only prunes sessions with
first_token_ts > 0, so a zero-token session is never reclaimed, and each
leaked entry inflates the `active` count sent over the SSE metering event.
begin_session() and the metering ticker's .start() are now registered as
the first statements inside the worker's outer `try`, and paired with an
idempotent meter().end_session(stream_id, 0) plus a deterministic
_metering_stop.set() in that try's outer `finally` (the same block that
pops STREAMS/CANCEL_FLAGS) — so every exit path tears the session down.
Deferring .start() until after put() is defined also closes a latent
start-before-put ordering window in the ticker closure.
The metering payload is unchanged; end_session only pops the session.
Verification: tests/test_metering_session_lifecycle.py (leak reproduction,
end_session reclaim, idempotency, begin->cancel-before-token->empty) plus a
plain-assert run on python3.11 (conftest gates pytest to 3.11-3.13; system
python is 3.14). py_compile clean.
WARNING streaming-contract: adjusts the streaming worker's begin/end
teardown ordering. Needs RFC/contract review against docs/rfcs/
(session-sse-contract-v1.md) before merge — draft PR.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three unaddressed Greptile review comments fixed:
P1 - Empty scenes skip check (api/models.py):
When the cached session had no anchor_activity_scenes, the scene-comparison
branch was skipped entirely by the 'if cached_scenes:' guard. An inactive
session would return False (cache fresh) even when disk had gained its first
scene record. Add an else clause that checks disk for scene records when the
cache has none.
P2 - Draft slow log sink (api/routes.py):
The /api/session/draft handler's slow-path log (>200ms) used logger.warning,
which is silently dropped in the WebUI process (root logger has no handler).
Route through handler._safe_webui_print() instead, the same fix applied to
the /api/session slow log in tier2c.
P2 - Config cache same-timestamp stale (api/routes.py):
The profile config.yaml cache key (profile_name, mtime, size) could collide
on coarse-mtime filesystems (FAT, some network FS) when a config edit within
the same timestamp tick happened to produce the same byte count. Add st_ino
(inode) to the cache key — most editors use atomic-rename writes which assign
a new inode, so the key changes even when mtime+size are identical.
Greptile review caught that /api/session handler calls _diag.maybe_start()
at entry and _diag.finish() only on the success path. Three exit paths
leaked the watchdog entry:
1. early-return when sid is missing (400)
2. early-return when session is not visible to active profile (409)
3. early-return when session not found (404)
4. the existing 'except KeyError:' branch at the bottom of the handler
Without _diag.finish() the watchdog's _watchdog_pending dict held the request
until the 5s slow-request timeout fired _on_timeout and emitted a spurious
'Slow WebUI request still running' log. The pending dict also stayed
unbounded by in-flight count.
Fix: every early-return path now calls _diag.finish() before returning;
the except KeyError branch calls it once at entry so all paths through
it are covered. finish() is idempotent (no-ops if already called).
Greptile review caught that the Tier 1a fast-path returns 'cache fresh' when
disk_count == cached_count AND the session is inactive, but api/routes.py
saves the sidecar after editing only anchor_activity_scenes (no message
change). That path leaves the cached scene records stale until the cache is
otherwise invalidated.
Fix: when the cached session has scene records, also compare them against
the disk's scenes via the same metadata-prefix read that _persisted_message_count
already does. anchor_activity_scenes is in METADATA_FIELDS and serializes
before messages, so it lands in the prefix.
New _persisted_session_meta_prefix() helper exposes the parsed prefix dict;
_cached_session_lags_disk reads it and compares scene key sets + max
updated_at against the cached session.
Cost: one extra dict parse per cache hit on sessions that have scene
records, no cost on sessions without. Inactive sessions still take the
fast-path exit when scenes match.
Codex gate CORE: /api/updates/apply + /force ignored the request-body channel
and re-read the saved setting, so a channel switch whose debounced autosave
hadn't landed could apply the OLD channel. The banner now sends the channel the
CHECK reported for each target; the endpoints validate it against the enum and
thread it into apply_update/apply_force_update (None → saved-setting fallback,
preserving prior behavior). Agent stays channel-neutral server-side.
Tier 2c-followup added print_fn=handler._safe_webui_print to the
/api/session handler so its [SLOW] line and structured
'RequestDiagnostics' log actually land in the systemd journal (the
WebUI process boots with no root-logger handler, so logger.warning
calls are silently dropped — the per-request ms line has always
worked because it bypasses logging and writes via _safe_webui_print).
There are 4 OTHER call sites of RequestDiagnostics.maybe_start() in
api/routes.py (for /api/models, /api/sessions, /api/profiles, and the
generic POST dispatch). All four were wired with the same broken
logger.warning path, so their structured slow logs have been silent
in production as well.
This commit:
- passes print_fn=getattr(handler, '_safe_webui_print', None) to
the same 4 call sites
- keeps the existing stage() and finish() call shape intact
- the existing /api/sessions handler already has stage() markers
(load_settings, profile_lookups, response_write) so the [SLOW]
log on sidebar-list will now show real per-stage breakdown
No behavior change on the happy path. No public API change. The
allowlist still gates which paths get a diag at all; this only
fixes the sink for the structured log on the 4 already-instrumented
endpoints.
Tier 2c added per-stage instrumentation on /api/session and added
'GET /api/session' to the RequestDiagnostics allowlist, but the
structured log emitted by RequestDiagnostics.finish() and the
pre-existing [SLOW] line both call logger.warning() — which the
WebUI process silently drops because the root logger has no handler
configured at boot.
The per-request 'ms' line has worked all along because it goes
through handler._safe_webui_print(), which writes directly to the
systemd journal socket.
This commit:
- adds a print_fn= parameter to RequestDiagnostics.__init__ and
RequestDiagnostics.maybe_start. When provided, finish() and
_on_timeout() route through it instead of self.logger.warning.
- routes the [SLOW] line in routes.py through handler._safe_webui_print
instead of logger.warning.
- passes print_fn=handler._safe_webui_print from the /api/session
handler to RequestDiagnostics.maybe_start.
After this fix, every slow /api/session request (and the existing
[SLOW] line) actually lands in the journal — the very data Tier 2c
was supposed to produce. Without this, the allowlist + handler
instrumentation would be a no-op in production despite the code
being present.
Measured: the same 2.97s /api/session call (sid 295592fd560e,
m=1 ml=30) which produced a per-request ms line but no [SLOW] line
before this fix, now produces both lines with full per-stage breakdown.
No public API change. The new print_fn parameter is optional and
defaults to None, preserving the existing logger.warning behavior
for any caller that hasn't been updated.
_read_profile_model_config() fires on every chat-open that hits the
resolve_model=1 path (one of the 3 calls in the chat-open waterfall).
For sessions under a named profile (non-default), the function
performs:
- 1× os.path.isfile() call
- 1× open() of config.yaml
- 1× yaml.safe_load() of the full file
On a Celeron N3350 with eMMC the open+safe_load alone is single-digit
ms per call, repeated on every chat-open. The /api/session 24h log
showed ?messages=0&resolve_model=1 at p50 1514ms with this YAML
parse as one of the dominant costs.
This wraps the parse in a per-process cache:
- key = (profile_name, mtime, size)
- value = parsed dict + monotonic timestamp
- TTL 60s as a backstop for filesystem mtime resolution issues
- cap 32 entries (bounded; profiles are bounded in practice)
mtime+size auto-invalidates on edit. The TTL only fires when mtime
resolution is too coarse to detect a same-second write; in normal
operation, every config edit lands as a different mtime and the next
chat-open rebuilds the cache entry.
Threading: a single Lock guards the dict. yaml.safe_load is the slow
step, not the lock, so contention is bounded. The lock is held for
microseconds on hit and milliseconds on miss (parse).
Field-semantics check: the function's signature is unchanged
((provider, default, config_dict) tuple) and the only call sites
(_resolve_effective_session_model_for_display and its sibling)
treat it the same as before. The cache stores the parsed dict;
the function still does the per-call (provider, default) extraction
inline so the tuple's content is identical to the pre-patch path.
No public API change. No behavior change observable other than
faster chat-open for sessions under named profiles.
The /api/session handler already has _t0.._t6 local timers plus an
env-var-gated [SLOW] WARNING that logs the total wall time and the
_tN-to-_tN+1 deltas, but only when HERMES_DEBUG_SLOW is set or total
>= 2s. In production we cannot rely on the env var, and the 2s
threshold means the typical chat-open (sub-2s on Tier 1) never logs
at all — we only see the worst cases, never the marginal ones that
would tell us where the next bottleneck is.
This wires the handler into the existing RequestDiagnostics framework:
- add ('GET', '/api/session') to the allowlist
- call RequestDiagnostics.maybe_start() at the _t0 boundary
- call stage() at each of _t1.._t6
- call finish() before the response returns
The watchdog fires _on_timeout (with thread stack snapshot) when the
request crosses the per-path slow threshold, and finish() emits a
single structured log line on the same threshold crossing. The
existing env-var-gated [SLOW] log stays as a no-diag fallback.
Greptile P1 trap avoidance: previous allowlist additions for similar
endpoints were flagged as dead code because the handler was never
updated to call maybe_start/finish. Both halves are present in this
commit so the watchdog entry actually registers.
No behavior change observable on the happy path (diag is None for
non-allowlisted paths; the existing _tN log is the fallback for both
None-diag and slow cases). No public API change. The /api/sessions
and /api/profiles handlers already use this pattern.
Adds monotonic-clock stage markers around the existing
/api/session/draft hot path so we can decompose wall time by phase
(get_session, lock acquire, save, json emit). Logs a single WARNING
line only when total > 200ms — keeps the per-keystroke poll cheap
while still catching the slow cases that justify further work.
This is the source of the slow-path data needed to plan Tier 2:
without stage breakdowns on /api/session/draft, every subsequent
patch on this endpoint is a guess.
The existing _t0.._t6 pattern on /api/session (gated on
HERMES_DEBUG_SLOW) is intentionally NOT extended here — the env-var
gate requires a manual restart and is not useful for routine
production diagnosis. The new stage markers log via the standard
[webui] WARNING channel and require no env var.
No semantic change. No public API change. Logging only fires when
the request exceeds 200ms, so it is invisible on the happy path.
Prior to this change _cached_session_lags_disk() called
Session.load_metadata_only(sid) on every LRU cache hit, parsing the
full sidecar JSON (~15-20ms even for a 1.3MB sidecar on Celeron+ eMMC).
For draft auto-saves hitting get_session() on every keystroke debounce
(~400ms cadence while typing), that 15-20ms multiplied out to ~75% of
the request's wall time on the Chromebook.
Reorder the check: read only the JSON metadata prefix
(_persisted_message_count, already defined for the LRU-eviction path)
to compare message counts. For INACTIVE sessions
(no active_stream_id, no pending_user_message) with matching counts,
the cache is provably at parity with disk — anchor-scene records cannot
have advanced without the message count advancing too, and the full
Session.load_metadata_only() is skipped. For ACTIVE sessions, or when
counts differ, fall through to the original anchor-scene comparison
path so correctness is preserved.
Measured cost reduction on the request hot path: 15-20ms -> <1ms on
the typical cache-hit case (inactive session, count matches). The
slow path is unchanged for cases that genuinely need the full reload.
No public API change. No behavior change observable on user-facing
flows. The LRU eviction safety invariant (#4765) is preserved —
_session_is_evictable() still calls _persisted_message_count under
the same conditions, and Tier 1a adds NO new eviction gate.
`_lock_for` caches one threading.Lock per (dir, file, pid) in the module-global
`_WRITER_LOCKS`, but nothing evicted those entries: `delete_run_journal` rmtree'd
the on-disk `_run_journal/{sid}/` directory yet left the cached lock objects
behind, so a long-lived gateway leaked one entry per deleted run forever.
Delete now evicts every cached lock whose parent directory matches the removed
session (pid-independent) under `_WRITER_LOCKS_GUARD`, leaving unrelated
sessions' locks intact.
Refs #4633, #2097.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
start_drain_thread and start_session_channel_reaper checked is_alive()
and then created + started the daemon thread without holding a lock, a
check-then-act race: two concurrent callers could both see "not alive"
and each spawn a thread. The loser's thread was never stored in the
module global and ran forever, un-joinable by the matching stop_*. Both
check-then-start sequences now run under a dedicated
_THREAD_LIFECYCLE_LOCK (kept separate from the purpose-bound
SESSION_CHANNELS_LOCK / _EMIT_COALESCE_LOCK), so exactly one thread is
ever created.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BG_TASK_COMPLETE_EVENTS_SEEN gained a session_id -> set[process_id] entry the
first time a bg task completed for a session and was never deleted anywhere, so
it grew unbounded for the server lifetime.
The entry is created in _process_one for EVERY completion, whether or not any
SSE channel/tab exists, so pruning it only when the SessionChannel is reaped
would miss the dominant headless case (task fires, tab closed or never opened —
no channel to collect). Instead the reaper now sweeps the map by DELIVERY: once
a completion is drained (its session_id removed from PENDING_BG_TASK_COMPLETIONS)
the short _move_to_finished dedup window is closed and the entry is swept, every
tick, regardless of any channel. The registry's per-process_id
_completion_consumed gate remains the primary idempotency backstop, so sweeping a
delivered session's set can never resurrect an already-delivered completion.
Session deletion also prunes the entry (new forget_bg_task_completion_dedup),
covering a session deleted while a completion is still pending (which the
delivery-gated sweep deliberately retains).
Refs #4633
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The session-listing path already opens the live agent state.db read-only
(file:...?mode=ro) so a write-capable handle doesn't add checkpoint/lock
surface while the agent streams into the same WAL DB (#5455). Three pure-read
projections were missed and still opened a read-WRITE connection:
- read_session_lineage_report
- read_session_lineage_metadata
- the gateway-watcher fingerprint projection (a 5s poll)
Route all three through a shared open_state_db_readonly() helper that mirrors
the listing path: read-only file: URI with a writable fallback that warns. The
self-heal write path (missing idx_messages_session) is intentionally left
writable. No behavior change beyond open mode; reads perform zero writes.
Refs #5455
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MUST-FIX: /api/updates/check now accepts an explicit 'channel' in the POST body;
the Settings dropdown passes the just-picked value to checkUpdatesNow(channel) so
the immediate re-check can't race the debounced autosave PUT and answer for the
previous channel (verified live: body channel wins over stale saved setting).
SHOULD-FIX:
- copy: 'soaked' -> 'soak-tested'; append 'Applies to WebUI updates only.' to the
channel helper text (all 15 locales, native translations)
- mobile: .settings-version-badge gets max-width/overflow/ellipsis in the <=768px
query so the '· Experimental' suffix can't overflow a 320px viewport
(#5 badge-suffix i18n key deferred — non-blocking fast-follow per Fable, matches
the existing English-only version badge on master.)
Codex round-2 gate: apply_clear_lock re-entered _apply_update_inner(target)
without the channel, so an experimental WebUI lock-recovery retry silently fell
back to stable. Now passes _read_update_channel() through. + regression test.
Codex gate CORE fix: check_for_updates passed the user's WebUI channel into the
Agent repo resolution, so on 'experimental' the Agent ignored its v* tags and
fell back to origin/master. The Agent is a separate project (plain v* tags,
tracks master past tags) and must ALWAYS use DEFAULT_UPDATE_CHANNEL. Forced in
all 3 sites: check_for_updates agent leg + _apply_update_inner + apply_force_update
(target=='agent' -> channel=DEFAULT_UPDATE_CHANNEL). Added a real-git regression
proving Agent release/apply resolution is identical under both WebUI channels.
Add an update_channel setting (stable|experimental) that selects which git-tag
stream the self-updater tracks on the single linear master line:
stable -> 'v*' promoted, soaked releases (default; unchanged glob)
experimental -> 'exp-v*' every release batch (opt-in testers)
A channel is only a tag glob — no branches, no divergence — so every ff-only
guarantee (#2653/#2846/#3140) is preserved. Channel governs the WebUI repo only;
the Agent repo keeps its historical branch fall-through.
Correctness (advisor-reviewed, Codex + Fable):
- describe uses --match <glob> so a commit tagged both v* and exp-v* resolves
to the channel-correct tag
- stable never falls through to origin/master when HEAD contains the latest
stable tag (the firehose-suppression that makes channels work)
- apply_force_update refuses to reset --hard onto an ancestor ref (rewind guard)
- update cache + in-progress guard keyed by (channel, include_agent)
- channel display badge is a SEPARATE field; WEBUI_VERSION stays channel-neutral
(asset cache-busting / SW cache / stale-client skew all do exact-string equality)
Settings UX: channel dropdown with risk-forward copy + the ff-only asymmetry
helper ('switching back to Stable keeps your current version until Stable catches
up'), channel chip on the version badge, i18n keys, re-check on switch.
Tests: new tests/test_update_channels.py (12 real-git-fixture tests) + existing
update suite updated for the channel-aware signatures.
Strict providers (DeepSeek v4, newer OpenAI) reject an assistant message
carrying tool_calls: [] with HTTP 400. _sanitize_messages_for_api already
strips orphaned tool results, but not an assistant message that literally
stores an empty tool_calls list.
Drop the empty key on ALL THREE sanitized-dict sites that must agree:
_sanitize_messages_for_api, _api_safe_message_positions, and the
_safe_projection inside _restore_reasoning_metadata (so a tool_calls: [] row
projects identically on both sides of the metadata-carry-forward alignment and
doesn't lose its reasoning/id/timestamp every turn — Fable gate follow-up,
folded in).
Added mutation-checked regression tests: empty tool_calls dropped + populated
chain preserved (both sanitizer paths), and metadata carry-forward survives a
tool_calls: [] row.
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
A valid session:// deep link to a session owned by a DIFFERENT Hermes
profile used to look identical to a deleted session (404 -> frontend
self-heals away). Now GET /api/session returns a structured 409
session_profile_mismatch envelope (error/code/session_id/profile ONLY,
no transcript) ONLY when the owning profile is KNOWN, and loadSession()
catches it, switches to the owning profile, and retries once. Truly
missing/deleted or legacy None-profile sessions keep the 404 self-heal.
Gate fixes applied (Codex + Fable):
- Codex CORE: added a post-await stale-load guard after
_switchProfileForSessionLoad so a navigation during the switch can't
hijack the UI back to the old session.
- Fable Finding 1: only emit 409 when _session_profile is truthy;
a None-profile (missing/legacy) session under a non-default active
profile now keeps 404 instead of a useless profile=null 409 (which
skipped self-heal + spun the SSE reconnect against a dead sid). Both
detail branches. + 2 regression tests.
- Fable Finding 2: _switchProfileForSessionLoad now clears the sidebar
skeleton + re-renders from cache on switch-POST failure (mirrors the
#4671 canonical-switch catch), then rethrows, so a failed switch can't
strand the sidebar on the skeleton.
Reconciled tests/test_issue1611_session_profile_filtering.py (4 tests) to
the 409 contract while preserving the no-leak boundary assertion.
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
Add creationflags to _run_git() and _config_names_for_scope() so git
child processes don't accumulate hidden cmd.exe/conhost windows on Windows.
Uses a local module-level _windows_hide_flags() helper (mirrors the
api/updates.py pattern: CREATE_NO_WINDOW on win32, 0 elsewhere) rather than
importing from the OPTIONAL hermes_cli package — a standalone/agent-less
WebUI (and the repo's own CI runners, which don't pip-install hermes-agent)
must keep full workspace-git functionality. windows_hide_flags() is a safe
no-op (0) on non-Windows. Scoped to workspace git commands.
Adds mutation-checked tests asserting the kwarg is wired on both sites.
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
Per Greptile's review on #5717: the previous
`if result.get('messages') is not None` guard created an asymmetric
condition where the silent-failure classifier could miss the injected
fallback when the agent result had `messages: None` (which never
actually happens in practice since `finalize_turn` always returns a
list, but the asymmetry read as code smell). Drop the guard; the
writeback is now unconditional on `isinstance(result, dict)`.
Per the maintainer's review on #5717: there may be a
`finalize_turn` invariant on the agent branch this WebUI ships
against that already appends the fallback as an assistant row, in
which case the live bug is gone and this PR becomes a regression
guard. Reframe the test docstrings as dual-purpose pins of the user-
visible closure-text contract so they read correctly under both
interpretations.
Co-authored-by: b3nw <b3nw@users.noreply.github.com>
The WebUI already surfaces the model-generated summary when the agent's
tool-iteration budget is exhausted. The remaining gap: when the agent
hits the limit but its summary call yields no usable assistant content
(common with reasoning-only responses), the user sees a bare
`tool_limit_reached` error instead of any closure text.
`hermes-agent.handle_max_iterations()` always returns a non-empty
`final_response` (either the summary or a graceful fallback), and
`AIAgent` exposes that string on the result dict. The WebUI, by
contrast, only reads `result['messages']`, so an empty summary
left no final assistant answer and routed to the error path.
Inject `result['final_response']` as a final assistant turn when
`max_iterations_reached` fires and no usable answer exists in
`messages`. The existing `_mark_latest_assistant_tool_limit_status`
flow then attaches the status card and the user sees the closure
text instead of the bare error. Hermes-agent parity preserved (it's
literally the same string); the bare-error path stays for the case
where the agent died entirely (no fallback either).
Tests cover: fallback-injection surfaces closure text + status card,
no double-injection when a real summary already exists, the existing
'died entirely' apperror path is unchanged, and three unit cases for
the helper's skip conditions.
Co-authored-by: b3nw <b3nw@users.noreply.github.com>
Add a complete Czech (cs) locale to hermes-webui: full key parity with the
English reference (1642/1642 keys), real Czech translations for all string and
function-valued keys, Slavic plural helpers for tool/worklog summaries, the cs
login-screen locale in api/routes.py, and a dedicated tests/test_czech_locale.py
parity+placeholder+diacritics guard mirroring the other per-locale tests. All
15 locale-count assertions bumped 14->15.
Co-authored-by: ostravajih <ostravajih@users.noreply.github.com>
Address round-2 gate cert (issuecomment-4896321817) on sha:dcec232d:
1. BRICK (Codex strace-verified): v2's fcntl-flock holder check on
.git/index.lock was wrong. Codex straced git 2.43.0 and proved
that git uses O_CREAT|O_EXCL + rename(2), NOT advisory locking,
so an fcntl-flock probe returns False for a lock file a live git
process is holding open. The 'fail-closed' claim was wrong:
auto-delete could still race and corrupt the index.
2. CORE (Codex): if .git/index.lock vanished between _is_lock_held
and os.remove, the FileNotFoundError surfaced as a 'remove-failed'
diagnostic instead of 'already gone' and prevented the apply retry.
Fix: stop deleting lock files from the server entirely. The only thing
that removes a lock is the user, on the host, via the manual command
surfaced in the response. Once the lock is gone, the user re-clicks
Update Now and the normal non-destructive apply path runs.
Implementation:
api/updates.py:
- Removed _is_lock_held (fcntl-flock probe) -- was provably unsafe.
- Removed _try_remove_lock (os.remove path) -- was provably unsafe.
- Removed _GIT_LOCK_FILES_REMOVABLE (no longer enumerable by name).
- Added _inventory_locks(path) -> dict: pure inventory of well-known
+ other lock files under .git/. Never touches anything.
- Rewrote apply_clear_lock to inventory-only + manual-instruction.
When .git/index.lock is absent, runs the normal non-destructive
apply path; when present, returns ok=False with the exact 'rm -f'
command + an explanation of why the server cannot do this safely
(O_CREAT|O_EXCL cannot be detected with advisory probes).
CORE-1 race evaporates: there is no os.remove to race against.
api/routes.py:
- Updated the /api/updates/clear_lock comment to reflect v2.2.
static/ui.js:
- applyClearUpdateLock: when res.lock_held, call new
_renderLockManualInstruction(target, res) which surfaces the exact
'rm -f <path>' command in a copyable code block plus an
'I've removed the lock -- retry update' button that POSTs the
same endpoint again; the second call takes the success branch
and re-runs the normal apply path. Also lists any other lock
files present so the operator can investigate.
tests/test_updates.py:
- Removed all 4 tests for the deleted v2 helpers (_is_lock_held
Returns*, _try_remove_lock*).
- Added test_v2_probe_helpers_removed: regression guard that
fails loud if either helper is ever reintroduced.
- Rewrote test_apply_clear_lock_removes_unheld_lock_and_runs_normal_update
and test_apply_clear_lock_refuses_when_lock_held for the v2.2
contract. The 'refuses' test monkeypatches os.remove to record
any deletion attempt and asserts the call list is empty -- a
strict runtime guard against future regressions to auto-delete.
- Added 7 new tests: test_inventory_locks_* (4), test_apply_clear
lock_with_no_lock_runs_normal_update, test_apply_clear_lock_with
lock_present_returns_manual_instruction, test_apply_clear_lock
listing_includes_other_locks, test_apply_clear_lock_rejects
unknown_target, test_apply_clear_lock_rejects_not_git_repo.
Verification:
./scripts/test.sh tests/test_updates.py -> 73 passed
./scripts/test.sh tests/test_update_banner_fixes.py -> 102 passed
./scripts/test.sh tests/test_api_timeout.py -> 8 passed
python3 scripts/ruff_lint.py --diff upstream/master -> 0 new violations
Full ./scripts/test.sh -> 12137 passed, 18 failed (all 18 pre-existing
flakes in unrelated subsystems; verified not v2.2 regressions)
Greptile rerun and the maintainer review both raised the same concern:
state.db and the sidecar self.messages can diverge by hundreds of
messages during recovery / mid-flight writes / pending_user_message.
On the test corpus:
0db167553ac7 db=11 sidecar=12
295592fd560e db=813 sidecar=889
7478cae31f01 db=2730 sidecar=2405
The previous patch's SQL path was reading state.db. The pre-patch
inline walk was reading self.messages (the sidecar). Sidebar stale-row
detection (_looks_like_stale_zero_message_row,
_row_may_need_sidecar_metadata_refresh) consumes this field as if the
sidecar were the source of truth, so swapping the source silently flips
the field's semantics.
Drop the SQL path entirely. The helper now does the same in-memory
walk the pre-patch code did, extracted into a named method for
visibility. The inline role check (dict.get('role') with default
empty string) is 1.7x faster than calling _message_role() per
iteration on the test corpus (2.57ms -> 1.49ms across 8 sidecars,
2405 msgs total).
Bench (2405-msg sidecar): 0.76ms per call, vs ~1.5s for the pre-patch
inline walk on the same sidecar (the original perf hotspot). The
1.5s figure included JSON parse, dict construction, and the SQLite
overhead of the now-removed SQL path; the bare walk is the
0.76ms shown here.
Correctness: 27/27 webui sidecars produce identical counts vs the
pre-patch inline walk. No behavior change for any caller.
Addresses Greptile findings (P1 silent-zero, P2 docstring, P2 dead
allowlist) by removing the SQL path entirely rather than patching
around its divergent-source issue.
Greptile flagged that _compute_user_message_count_lazy silently returns 0
when the SQLite query fails (DB locked, missing file, schema drift).
The pre-patch compact() walked self.messages in Python and returned the
correct count without ever touching the DB, so the silent zero
fallback is a regression that breaks _looks_like_stale_zero_message_row
under contention.
Fix: helper now takes an optional messages argument. On SQLite failure
it falls back to the same in-memory walk the original code used, so
callers see the same number they'd have seen before for any session
where Session.load() populated self.messages.
Also drop the dead ('GET', '/api/session') allowlist entry flagged by
Greptile — the /api/session handler already does its own per-stage
logging via the _t0.._t6 block (with auto-log on slow requests), and
adding RequestDiagnostics on top would just duplicate the slow-request
journal entries without adding information.
Address the RED gate cert on PR #5688 (2 BRICK + 1 CORE) and the
incoming Greptile review (1 P1 + 3 P2):
- BRICK-1 (race-safety): drop the mtime heuristic and any age-based
lock removal. Add /api/updates/clear_lock with a fail-closed
fcntl.flock holder probe (refuses if any process still holds the
file, on POSIX). On non-POSIX, fails closed. Touches only
.git/index.lock (the only well-known short-lived lock git creates).
- BRICK-2 (destructive path coupling): remove lock_conflict from the
force-button condition. Add a separate btnClearUpdateLock that calls
/api/updates/clear_lock -- never apply_force_update. The recovery
path never runs checkout/clean/reset --hard.
- CORE-1 (unconditional pre-cleanup): apply_force_update no longer
iterates .git/**/*.lock. Lock cleanup is exclusively the clear_lock
endpoint's job.
- Greptile P1: when a pull-lock error fires after a stash was pushed,
run git stash pop (with apply+drop fallback) so the user's local
modifications aren't stranded in the stash silently. v2 tests
cover the stashed and the clean cases.
- Greptile P2 (broad 'lock file' substring): tightened _GIT_LOCK_SIGNATURES
to specific git error strings ('index.lock': file exists, '.lock':
file exists, 'another git process seems to be running',
'unable to create .git/index.lock') so unrelated ref-transaction
lock-loss messages no longer trigger a lock-conflict response.
- Greptile P2 (redundant git_dir.exists / magic number): both gone as a
side effect of removing the apply_force_update cleanup loop entirely.
Refactor: rebased onto current upstream/master so PR is no longer
'behind' 130 commits.
Tests:
- 14 new v2 tests covering holder probe (true/false/NOGIL probe),
_try_remove_lock (refuse-on-held/success-on-unheld),
apply_clear_lock (success/refused), signature set parameter table
including false-positive class, apply_force_update no-touch
contract, pull-lock stash restore, pull-lock no-stash-when-clean.
- 2 v1 tests removed (they encoded the unsafe behavior).
- 2 v1 parametrize rows dropped (their positive cases relied on the
broad 'lock file' substring).
Closes#5687
- Detect lock file errors (lock_conflict flag) on fetch/status/pull
failures in _apply_update_inner (#1)
- Expose Force Update button on frontend when lock_conflict is
received (#3)
- Proactively clean stale lock files (>30s old) in apply_force_update
using rglob discovery instead of a hardcoded path list (#2)
- Add 10 tests: _is_git_lock_error parametrized unit tests,
_apply_update_inner lock path coverage, and apply_force_update
stale/recent lock behavior
Session.compact() called _last_message_timestamp(self.messages) which
reversed-walked all 2,730 messages on every /api/session response.
The messages array is chronologically ordered, so the last non-tool
message sits at the very end. Scan only the last 8 messages; fall
back to a full scan only if no timestamp is found in the window
(unusual sessions with >8 trailing tool rows).
Reverts to identical behavior when the window hits, since the most
recent user/assistant message's timestamp is what compact() emits.
Bench (single-shot, live Chromebook eMMC):
before: get_session=1107ms compact=1725ms total=2880ms
after: get_session= 648ms compact= 288ms total=1313ms
(-80% on compact, -54% on total)
Revert: git checkout master && systemctl --user restart hermes-webui
Adds auto-logging when total /api/session latency exceeds 2s, so we don't
need HERMES_DEBUG_SLOW env var to diagnose regressions in production.
Env var still forces logging on every request for development.
Baseline data on the Chromebook (Celeron N3350, eMMC storage):
- long_session_idle (2,445 msgs) p50=5.8s, p95=9.1s, max=9.7s, 2/10 timeouts
- stage breakdown for a 2.8s request:
get_session=1041ms, compact=1717ms, redact=31ms, json_write=8ms
- compact stage iterates all messages for user_message_count and runs
recursive redact_session_data + json.dumps -- biggest single cost
Ref: perf/session-load-latency
The slow-request journal showed /api/profiles, /api/models, and /api/session
fire on every session click but had no per-stage timing. Add:
- RequestDiagnostics coverage for GET /api/profiles, /api/models, /api/session
- Stage markers inside /api/profiles (list / active lookup / isolated check)
- Stage markers inside /api/models (freshness routing, serialize)
- Per-stage stage-log inside get_available_models_for_session_visit that
emits a [SLOW] line when total wall time crosses HERMES_DEBUG_SLOW ms
(default 500ms) so we can pinpoint which sub-step is blocking
No behavior change. Existing _t0.._t6 timing on /api/session and the
2.5s/10s TTL on /api/sessions are unchanged. Reversible: revert commit.
Relaxes the /api/session/branch read-only gate so a canonical-cron session can be forked into a new WebUI-owned session (source never saved). Gate hardening folded in: the read-only branch gate now covers BOTH the synthesized/missing-sidecar path AND the persisted/loaded path (Codex found a stored read-only non-cron session could be branched + saved); relaxation keys on the server-authoritative resolved source (source_tag/raw_source/source == 'cron'), never the id prefix. Full gate: Codex adversarial SAFE, 84 branch/claim tests + suite 12175/0, browser clean.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
get_state_db_session_messages() returned archived (active=0) rows that in-place
compaction had marked inactive. WebUI reconciliation feeds this reader straight
into the next model-facing context (reconciled_state_db_messages_for_session,
prefer_context=True), so those archived rows got pulled back in — resurrecting
pre-compaction history and making every later turn re-trigger compression. Now,
WHEN the messages table exposes an `active` column, inactive rows are excluded by
default (`AND (active IS NULL OR active != 0)` — NULL/legacy rows preserved), with
an `include_inactive=True` escape hatch for explicit recovery/audit callers.
Schemas without the column are unaffected (guarded by `'active' in available`).
Contributor stage; ungated on arrival. Co-authored-by: ai-ag2026 <ai-ag2026@users.noreply.github.com>
_webui_sidecar_lineage_messages_for_display stitches a session's pre-compression
snapshot parents for full-transcript display. A cleared fork child could resurrect
its ORIGINAL (non-fork) parent's transcript when that parent was later compressed.
Now, when the root session is itself a fork (session_source=='fork'), the stitch
walk allows fork-sourced snapshot parents but stops at the first non-fork
ancestor — so a compressed fork continuation still shows its own pre-compression
turns, while an ordinary fork child stays isolated from the original parent.
Additive guard (one extra break condition); non-fork lineage display unchanged.
Contributor stage; gate-pass. Co-authored-by: rodboev <rodboev@users.noreply.github.com>
OpenCode Go's /v1/models endpoint returns models from the full public catalog that
are NOT enabled on the Go tier; selecting any such probe-only model 404s
('model not found') when the chat request is sent. get_available_models now skips
the live probe for opencode-go (bare branch) so the next `if not raw_models` falls
through to the curated static _PROVIDER_MODELS['opencode-go'] list — the actual
Go-tier models. Narrow, provider-scoped; no other provider affected.
Contributor stage; gate-pass. Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
The initial fix added ~16 lines to server.py, tripping test_server_py_under_750_lines
(server.py must stay a thin dispatcher; logic belongs in api/). Moved the whole
preflight-response into apply_cors_preflight_headers(handler) in api/routes.py;
server.py's do_OPTIONS is now a 4-line dispatcher. server.py 764 -> 746 lines.
Same security behavior (same-origin/allowlist echo, Vary: Origin, no wildcard,
header-less denial); test rewritten to the header-emitting API + added Vary and
no-header-on-denial assertions and a routes.py wildcard regression guard.
do_OPTIONS answered every CORS preflight with Access-Control-Allow-Origin: *,
advertising broader cross-origin access than a real request is granted — on a
password-less deployment that lets any site read authenticated responses. It now
echoes the request Origin only when _check_same_origin_browser_request approves
it (the exact same-origin / HERMES_WEBUI_ALLOWED_ORIGINS policy the CSRF gate
enforces for real requests), adds Vary: Origin, and never emits *. A disallowed
origin gets a 200 with no CORS headers (browser treats as preflight denial).
Default deployments (no allowlist) are unaffected.
Contributor stage; gate-pass. Co-authored-by: mo7al876any <mo7al876any@users.noreply.github.com>
Codex gate CORE: the exact-empty sentinel only suppressed restore for a bit-empty
cleared sidecar. If the user cleared, then sent ONE post-clear message, and the
stale pre-clear .json.bak still existed (unlink failed), recovery saw
bak_count>live_count and RESTORED the pre-clear transcript on top of the new
message. Added _live_supersedes_backup_by_clear_generation: suppress restore when
the live sidecar carries a clear_generation the backup lacks AND has a non-empty
transcript AND still shows the clear boundary reset (watermark==boundary==0.0).
Scoped to non-empty live so empty clear-shaped sidecars keep their existing
recovery semantics (active/pending/partial variants still restore). Fails open on
unreadable/partial reads. +2 regression tests (post-clear-message not restored;
post-clear-after-real-compaction still restores).
Closes the crash-window resurrection corner in the #5532 clear-conversation
data-loss fix. /api/session/clear writes a .json.bak before truncating, and
because the cleared sidecar's truncation_watermark is 0.0, startup recovery saw
bak_count > live_count and could restore the backup — resurrecting the cleared
transcript after a crash/restart in the clear window.
Fix: stamp a unique per-clear `clear_generation` UUID on the sidecar when
clearing a session that had messages (persisted via Session model). Recovery's
_session_records_clear_sentinel() returns no_action when the live sidecar carries
a clear_generation the backup doesn't share AND the live sidecar matches the
exact cleared-empty shape (messages/context empty, watermark/boundary 0.0,
pending fields cleared). Same-generation backups stay recoverable; unreadable or
partial matches FAIL OPEN (normal recovery), so a real crash-loss is never
suppressed.
Contributor stage; gate-pass. Co-authored-by: rodboev <rodboev@users.noreply.github.com>
The streaming settlement path could show a misleading "No response from provider"
error card AFTER a completed answer, because a stale terminal/partial result flag
still forced the generic no_response error path even when the merged current turn
already had a final assistant answer. Now the generic no_response terminal error
is suppressed ONLY when the already-merged current turn has a completed assistant
reply (scoped to soft `partial` results per the issue-thread recommendation),
letting normal `done` settlement continue. Real silent failures, partial
failures, auth/quota errors, cancellation, replayed rows, and terminal
tool/compression cases all keep their existing error behavior.
Self-built stage of contributor PR; gate-pass. Co-authored-by: rodboev <rodboev@users.noreply.github.com>
'All 0 credential(s) exhausted for <provider>' did not match _is_quota_error_text
so it fell through to a generic 'Error' with no hint (the exception path's else
branch discarded the hint entirely). Adds a distinct credential_pool_empty
classification (ordered before the quota check) + an explicit exception-path
branch + an actionable pool-specific hint, so the user sees why the turn failed
and what to do. Does not over-match (doesn't swallow real quota errors) and
doesn't block partial-harvest; downstream tolerates the new type.
NOTE: this is the CLASSIFICATION half of #3929 only — the data-loss half
(reasoning/partial-work restore on reload-reconcile) is a scoped follow-up that
needs the reporter's server logs.
Self-built (nesquena-hermes); nesquena independent review APPROVED.
An expired-auth 401 on the login page fed the login redirect its own address:
each of the three redirect guards (_safeNextPath in login.js,
_safe_login_redirect_path in routes.py, the api() 401 redirect in workspace.js,
_safe_login_inner_next in auth.py) validated against open-redirect but none
rejected a `next` pointing back at the login page, so every auth bounce wrapped +
re-encoded the previous login URL one level deeper — growing the URL exponentially
until the browser's length limit broke the tab. All guards now collapse/reject a
`next` whose decoded leading PATH resolves to the login route (bounded
percent-decoding up to 8 levels, fail-closed at the cap), while preserving the
existing open-redirect protections AND legitimate non-login paths that merely
carry their own `next=` query key. Length cap as belt-and-suspenders.
Self-built (nesquena-hermes); nesquena independent review APPROVED. Gate (Codex)
found + fixed an over-broad nested-next reject (regressed next-carrying paths) and
a deep-encode decode off-by-one; both root-fixed, login guards verified clean.
profile_env_for_background_worker mirrored the profile home into process-global
os.environ["HERMES_HOME"] and yielded the worker body OUTSIDE the setup lock, so a
concurrent cross-profile worker (e.g. background title generation on another
profile) could clobber os.environ mid-body — the agent config reader then resolved
the WRONG profile's config (intermittent turn-init failures citing an unused
provider; b3nw's v0.51.849 repro). Now installs hermes-agent v0.18.0's
context-local home override (set_hermes_home_override, a ContextVar that
get_hermes_home() consults before os.environ) for the resolved non-default profile
home — set before yield, reset in finally — so a config read in the worker body
resolves THIS profile's home from task-local state, immune to a concurrent
os.environ clobber. No worker serialization, no os.environ mutation, no detection
heuristic (both earlier WebUI-only mitigations were rejected as regression-unsafe).
Graceful degradation: the override module is resolved lazily+optionally
(_resolve_hermes_home_override, mirroring _resolve_secret_scope_module). On agents
< v0.18.0 the symbol is absent -> returns None -> pre-existing os.environ-mirror
behavior unchanged (no regression, no new failure mode).
Self-built (nesquena-hermes); nesquena independent review APPROVED.
Codex round-3 gate finding: _dedupe_replayed_context_messages deep-copies the
stale-user repaired boundary row (streaming.py:4501) into the context array. When
the mint ran AFTER dedupe, that deep-copied context row was no longer the shared
result dict, so it stayed id-less while the display copy got the minted id —
re-opening the cross-array id divergence for the stale-repair path. Fix: move
_assign_stable_message_ids() to run immediately after _restore_reasoning_metadata
and BEFORE _dedupe_replayed_context_messages at all four commit sites (streaming
main / in-band self-heal / except-path self-heal + routes runs/MoA), so the shared
result rows are stamped before any deep-copy — both arrays inherit the id.
Codex/Opus gate finding: in eager session-save mode the current user turn is
checkpointed into s.messages before the agent runs (no id yet); after
_assign_stable_message_ids stamps the returned result row, the display merge
keeps the durable eager checkpoint and skips the stamped result — leaving the
display row id-less while its context twin carries the minted id, silently
defeating id-based fork/truncate alignment for eager-mode users. The merge now
copies the minted id onto the kept checkpoint when it lacks one (guarded so it
can't overwrite or duplicate). Opus verified cross-array id pairing holds across
the real 3-turn transform pipeline.
WebUI keeps two parallel arrays per session: messages (display transcript) and
context_messages (what's sent to the model). In large/compacted sessions they
diverge. Forking / in-place truncation copies a prefix of both and relies on
truncate_context_for_display_keep() to translate a display keep-index into the
matching context index — an aligner that prefers a stable per-row `id`. But the
model-context rows carried neither id nor timestamp (a 989-session live scan
found 0 with any id), so alignment fell back to fragile content-signature
matching that goes ambiguous on repeated tool calls / empty assistant turns.
This closes the DATA gap (the alignment logic itself already prefers `id` and
handles the compacted case via the matcher, shipped in #5563): mint a monotonic,
session-unique integer `id` on the per-turn result rows AFTER the context restore
and BEFORE both arrays are built, so the display and model-context copies of a
logical row share the same id.
- api/streaming.py: new _assign_stable_message_ids(); _restore_reasoning_metadata
carries `id` forward across turns (as it already does timestamp); wired into all
three streaming commit sites (main turn, retry, self-heal).
- api/routes.py: same mint on the runs/MoA _handle_chat_sync commit path.
- api/gateway_chat.py: mints ids on the two new rows of a gateway turn.
- The `id` is stripped before the provider API call (not in _API_SAFE_MSG_KEYS),
same treatment as timestamp — nothing new reaches the provider.
session_ops.py alignment was already delivered by #5563 (id-preferring matcher +
compacted-case fall-through), so no change is needed there now; tests updated to
assert the post-#5563 behavior (id-bearing = exact cut; id-less = errs toward
under-keeping, never the old raw-index mis-cut).
Co-authored-by: b3nw <b3nw@users.noreply.github.com>
Move the 1-5s commit_session_memory() extraction off the POST /api/session/new
request thread (was blocking + New Chat) into a fire-and-forget daemon thread,
guarded by the existing per-session in-flight serialization.
Data-loss prevention on managed stop:
- server.py installs a SIGTERM handler that requests an orderly stop so
serve_forever()'s finally block runs drain_all_on_shutdown() (the default
SIGTERM disposition terminated without unwinding, losing in-flight commits).
- session_lifecycle: bounded self-unregistering background-commit registry;
a _draining flag so no NEW worker starts once the drain begins (the inline
generation-drain commits any late arrival instead); a 30s overall drain
deadline so a stuck worker cannot hang the stop path forever.
Co-authored-by: luperrypf <luperrypf@users.noreply.github.com>
Fork ("Fork from here") on a large/compacted session could cut context off a
real turn boundary — slicing mid-turn or leaving a dangling tool call, feeding
a malformed context to the model. Align the fork cut to a resolved turn boundary
via a signature matcher handling both compacted and non-compacted display
coordinate spaces; errs toward under-keeping with send-time sanitization backstop.
Co-authored-by: b3nw <b3nw@users.noreply.github.com>
Rebuilt the combined #5556+#5553 fix on v0.51.859 (now has the flake fix + the
#5542 RFC de-brittle, so no anchor-test collateral). Nathan's call: rodboev's
#5556 primary + fold self-built #5553 improvements, credit both.
Clear handler now:
- routes through shared truncate_session_at_keep(s,0) (single source of truth,
sets watermark=_truncation_watermark_for([])==0.0, the #2914 sentinel that
blocks state.db append-merge replay)
- detaches compression lineage ONLY when the parent is a pre_compression_snapshot
(Codex-caught: preserve genuine fork parent links for nesting + "Forked from")
- rodboev's persisted-clear read-back verification + stale .bak removal
RFC conflict resolved in favor of the de-brittled (symbol-anchor) master version.
Ships both PRs' test files (state_db_replay + clear_truncation_watermark incl the
fork-preservation regression). Opus (on the amended tree): both resurrection paths
closed, detach lineage-safe, verification sound — SHIP. 3 non-blocking follow-ups
filed (#5570 .bak crash-window, #5571 fork-stitch corner, #5572 messaging clear).
Co-authored-by: rodboev <rodboev@users.noreply.github.com>
