The regression test re-imports api.profiles in isolation by stubbing
flask/yaml/agent in sys.modules and deleting+reloading the real api /
api.profiles modules. Teardown only popped api/api.* (never restoring the
real modules) and left the flask/yaml stubs behind, so the manipulation
LEAKED: subsequent tests re-imported api.config/api.routes against the stub
yaml (safe_load->None) and a half-populated api package, silently breaking
~120 unrelated tests in the full serial suite (e.g. MCP/provider/config
tests whose get_config patch no longer saw real config). Snapshot every
sys.modules key we touch and restore it exactly (real modules back, injected
stubs removed) in a finally block. Full suite now matches master baseline
(11537 passed, only the 2 known cron-isolation artifacts).
- _repair_bare_custom_provider_model matches display-named providers via
_custom_provider_slug_from_name (custom:my-proxy matches 'My Proxy').
- _ordered_custom_provider_model_ids now handles dict keys, list strings,
and list dicts with id/model/name, aligned with api/config.py catalog.
- config_obj=None fallback uses get_config() instead of raw cfg.
- _read_profile_model_config returns profile config dict too, avoiding a
second YAML parse on hot display paths.
- Tests added for slug matching, list-form models, and get_config fallback.
Co-authored-by: b3nw <b3nw@users.noreply.github.com>
The two-tier mtime cache from #4783 fixed the per-request SKILL.md rescan
but left two concurrency holes that only bite at container cold start,
when the frontend fires several profile-data requests at once and the
caches are empty:
1. `_get_profile_skills_stats()` had no lock, so concurrent misses on the
same profile each ran `os.walk(followlinks=True)` + parsed every
SKILL.md simultaneously.
2. `_build_profile_rows_fast()` ran outside `_LIST_PROFILES_CACHE_LOCK`
in `list_profiles_api()`, so every concurrent request rebuilt all rows
(each walking every profile's skill tree) at once.
With ThreadingHTTPServer (one OS thread per request) and Docker overlay2,
this stacked thousands of concurrent stat() calls and stalled workers
57-70s (per the report's thread dumps).
Fix:
- Add a per-profile compute lock (registry guarded by a meta-lock) and
use double-checked locking in `_get_profile_skills_stats()`: concurrent
misses on one profile collapse to a single compute, while independent
profiles still compute in parallel.
- Single-flight the row build in `list_profiles_api()` by holding
`_LIST_PROFILES_CACHE_LOCK` across the build + cache write. Lock order
is strictly list-lock -> per-profile skills-lock, so no deadlock.
The report's third suggestion (debounce the mtime probe) is deliberately
NOT taken: the every-call cheap probe is the #4783 out-of-band
change-detection contract (test_issue4783 asserts it MUST run on every
call). Serializing the misses removes the herd without weakening that
contract, since only the expensive compute is guarded, not the probe.
Adds tests/test_issue5364_skills_stats_thundering_herd.py proving the
herd collapses (single compute / single build under a concurrent burst),
independent profiles still parallelize, and the every-call probe contract
is preserved. All existing #4783 contract tests still pass.
Co-authored-by: claw-io <claw-io@users.noreply.github.com>
_session_message_content_key (the state.db reconciliation key in
api/models.py) normalized whitespace only, while the streaming-side
identity _message_identity strips the workspace prefix for user turns.
WebUI sends the model a workspace-prefixed user_message
([Workspace::v1: /path]\n<text>) while the visible/optimistic bubble and
sidecar row carry the bare <text>. The mismatch made a prefixed state.db
row and a bare sidecar row key differently, so state_db_delta_after_context
failed to align them, treated the state.db copy as new, and appended a
duplicate user turn. The agent-side merge then concatenated the two
adjacent user rows into a permanent composite -- the post-restart
stale-user-prepend bug.
Fix: strip the workspace prefix for role=='user' in the reconciliation
key, reusing the same _strip_workspace_prefix helper the streaming side
uses (lazy import to avoid the api.streaming -> api.models cycle) so the
two dedup layers can't drift again. Assistant/tool keys are unchanged and
prefix-free user messages key identically (idempotent).
Fixes#5339.
Root cause of the silent-crash-after-hours cluster (#4765/#2233/#4633): the
global in-memory SESSIONS LRU evicted with a blind popitem(last=False), which
could drop an actively streaming or not-yet-persisted session (data loss) and
was capped only via an env var. On long-running installs the effective result
was unbounded RAM growth until segfault.
- Add _session_is_evictable(): a session is evictable ONLY when it is not
streaming (no active_stream_id), has no in-flight turn (no pending_user_message
/ pending_started_at), and its full state is proven on disk (sidecar
message_count >= in-memory count; metadata-only stubs and zero-message shells
are trivially safe).
- Add _evict_sessions_over_cap(): replaces all 8 blind popitem loops across
models.py, routes.py, streaming.py. Walks the LRU oldest-first and removes only
provably-safe entries; never acquires LOCK/stream locks itself (caller holds
LOCK) so no lock-ordering deadlock. May briefly exceed the cap rather than ever
evict an active/unsaved session.
- Make the cap configurable via config.yaml webui.sessions_cache_max
(get_sessions_cache_max()); precedence config.yaml -> HERMES_WEBUI_SESSIONS_MAX
(legacy) -> DEFAULT_SESSIONS_CACHE_MAX=300. No new HERMES_* env var. Invalid or
<1 values fall back so a typo can never disable the bound.
- Evicted sessions lazily reload from their JSON sidecar via the existing
get_session() accessor; no call sites changed. _index.json sidebar behavior
unchanged (the sidebar reads the index, not SESSIONS).
- Add tests/test_issue4765_sessions_lru_eviction.py (8 tests): eviction past
cap, active/streaming never evicted, unsaved/stale-tail never evicted, lazy
reload with identical content, and no-data-loss under heavy churn.
- README: document the config.yaml key + safety semantics.
Fixes#4765.
Regression seam behind #5308: a delegated subagent child's sidebar row is built
from a stale sidecar that reports message_count==0, and the state.db count
overlay in _apply_sidebar_state_db_override_metadata was gated on
`state_db_source == 'webui'`. A subagent child (state_db_source=='subagent')
therefore never received its true message count, so the front-end visibility
predicate (_sidebarRowHasVisibleMessages) dropped the row and the subagent
session disappeared entirely (not nested, not orphaned) after #5244+#5306.
Fix: widen the count/last-message overlay to `state_db_source in ('webui','subagent')`,
keeping the same conservative anti-resurrection guard. The source-tag/title
reassignment stays WebUI-only so a subagent child keeps its subagent
classification. Same state.db-blind-metadata root as the #5307 transcript
recovery, fixed server-side rather than by loosening the front-end predicate
(which would fight the #5306 active-parent scoping).
Tests: subagent child gets its count overlaid + classification preserved; a
non-webui/non-subagent foreign source (cron) still gets NO overlay.
Fixes#5308.
server.py exited SILENTLY after 9-16h: no traceback, no shutdown-audit line,
no core dump — the log just stopped mid-request. It runs a ThreadingHTTPServer
with daemon_threads=True, so an unhandled exception in a request/SSE/long-poll
handler thread could terminate work with nothing recorded, and faulthandler was
not enabled so a native crash left nothing at all. The WebUI also configures no
logging handlers, so INFO/ERROR records are dropped by logging's lastResort
filter (WARNING+ only) — meaning even the existing shutdown audit never reached
the log.
Add api/crash_visibility.py (stdlib-only, hooks never raise) and wire
install_crash_visibility() into server.main() before any heavy startup:
* faulthandler.enable(all_threads=True) — native crash dumps a C-level
traceback; SIGUSR1 registered for on-demand hang diagnosis.
* threading.excepthook — logs uncaught daemon/handler-thread exceptions
(thread name, ident, traceback) instead of losing them silently.
* sys.excepthook — logs uncaught main-thread exceptions (KeyboardInterrupt
preserved).
* atexit exit-audit breadcrumb — a clean/unwound exit is recorded; its
absence narrows a silent death to an un-unwound kill (OOM/SIGKILL/abort).
All diagnostics are written directly to the fault stream (stderr, which the
bootstrap redirects into the WebUI log) AND mirrored through logging, so the
line is guaranteed to land regardless of logging config.
No request-handling behavior change, no new deps. Paired memory root-cause: #4765.
Fixes#4633.
The Node-executed gate in test_anchor_fallback_ownership.py
(test_render_messages_keeps_anchor_owned_turn_out_of_legacy_activity_rebuilds)
eval()s the real renderMessages(). Commit 7536f6f1 made renderMessages schedule
its post-render pass via _postProcessWithAnchorSuppression(), but the harness
only stubbed postProcessRenderedMessages() — so the eval threw
'ReferenceError: _postProcessWithAnchorSuppression is not defined' and the test
failed on Linux CI (shard 2). It passed locally only because Windows hit the
unrelated WinError 206 command-line-too-long first, masking the real error.
Add a no-op stub for _postProcessWithAnchorSuppression alongside the existing
postProcessRenderedMessages stub. Verified by dumping the generated node script
to a temp .js file and running 'node file.js' (bypassing the Windows -e length
limit): the eval no longer throws and the test's assertions pass.
Commit 7536f6f1 routed the deferred post-render dispatches through
_postProcessWithAnchorSuppression() (holds overflow-anchor suppression across
the async media/layout settle frame, then calls postProcessRenderedMessages).
Six pre-existing tests string-matched the old
'requestAnimationFrame(()=>postProcessRenderedMessages(inner))' literal and
failed on the rename — behavior is preserved (the wrapper still invokes
postProcessRenderedMessages), so this is a test-fix not a code-fix.
Per the gate-cert recommendation, the tests now assert the BEHAVIOR chain
(post-render is scheduled via _postProcessWithAnchorSuppression, and that
wrapper calls postProcessRenderedMessages) rather than the exact rAF literal, so
a future wrapper rename can't re-orphan them.
Files: test_csv_table_rendering, test_excalidraw_inline_embed,
test_issue483_inline_diff_viewer, test_issue484_json_tree_viewer, test_issue347,
test_pdf_html_preview. Verified: the 6 updated assertions pass locally (the only
local failures are the pre-existing Windows-only WinError 206 command-line-too-long
in Node-harness tests, unrelated, green on Linux CI).
The sync-frame guards (_fixMobileScrollJank / _suppressBrowserOverflowAnchor)
only cover the render frame itself. postProcessRenderedMessages() — syntax
highlight, inline diff/csv/pdf/html/excalidraw, katex/mermaid — is scheduled a
FRAME LATER via requestAnimationFrame(), after those guards have released. Each
of those can change the height of rows ABOVE the viewport; on mobile
(overflow-anchor:auto) the browser's native anchor engine then compensates
scrollTop a SECOND time in that unguarded frame, yanking an unpinned reader to
another turn (the residual mobile 往回大跳).
Wrap all three deferred post-process dispatches (fast-path cache branch, main
render tail, live-tool remount) in _postProcessWithAnchorSuppression(), which
routes through the shared _suppressBrowserOverflowAnchor() and holds suppression
one extra frame so late media/layout reflow is covered too. Desktop rests at
overflow-anchor:none so the wrapper is a verified no-op there.
Reproduced on an isolated debug instance with a cloned 1179-message session:
above-viewport +350px during the async settle window jumped scrollTop +350 on
mobile (auto) and 0 with the wrapper; desktop (none) 0 both ways. static/ui.js
only.
Codex gate on #5346 flagged two brittle static extractors that broke on the
cancelStream(reason) signature change + the new '[stream] cancel requested'
stdout log:
- test_cancel_stream_owner_guard.py: the Node harness parsed ALL of stdout as
JSON; the provenance console.info line (fires during runAll) polluted it.
Parse the LAST non-empty stdout line (the result JSON is always emitted last,
after runAll resolves).
- test_sprint36.py: two extractors did src.find('async function cancelStream()')
(exact, no params). Switched to a signature-tolerant regex and widened the
catch-block window (the provenance log/comments now precede the try/catch).
Both pre-existing tests, updated to match the intentional #5345 change (not the
code bent to fit the test).
/api/clarify/pending always returns HTTP 200 when present (returns
{"pending": null} for an unknown session — it never 404s). The front-end
clarify poller warned "Clarify endpoint unavailable. Please restart
server." on ANY caught error whose message merely contained "404" or
"not found", so an unrelated stale-session 404 ("Session not found", e.g.
an old-profile session polling briefly after a profile switch) or a
transient error produced a misleading missing-endpoint toast that pointed
operators at the wrong layer.
Clarify polling now branches on the structured HTTP status that api()
attaches to the thrown Error (err.status):
- 404 "Session not found" -> handled as a stale-session poll (stop + hide
card silently), no toast;
- restart-server warning fires only on a genuine route-not-found 404 whose
body is NOT session-scoped;
- poll failures are logged with path, status, polling session id, and
current session id for diagnosis.
Interrupt provenance: cancelStream()/cancelSessionStream() now log a
'[stream] cancel requested' line with the trigger reason
(composer-stop / slash-stop / slash-interrupt / busy-interrupt /
sidebar-stop). Passive UI lifecycle events (session switch, tab hide, page
unload) already tear down only the local SSE transport via
closeLiveStream() and never call /api/chat/cancel — only explicit
Stop/interrupt paths interrupt the backend agent/tool run. This is
confirmed by test_clarify_pending_never_404s locking the handler shape.
Supersedes #5343 (which handled only the profile-switch sub-case and kept
the broad message-scrape). Adds tests/test_issue5345_*.py (9 tests).
Co-authored-by: claw-io <claw-io@users.noreply.github.com>
Co-authored-by: ruizanthony <ruizanthony@users.noreply.github.com>
Structural hardening per user request, following the 3rd round of the
same bypass class on PR #5313's reasoning_efforts feature:
Round 1: plain-ordering regression (provider-config short-circuit ran
before the nested-route deny).
Round 2: a provider-qualified hint (@custom:<slug>:vertex/gemini-...)
left a slug fragment that the deny's prefix-match missed.
Both were legitimate, narrowly-targeted fixes, but the pattern —
_nested_route_reasoning_denied() requiring the model string to START
WITH the route prefix — meant every future wrapper/nesting scheme
would need the strip logic to be updated in lockstep, and a missed
case fails OPEN (reasoning re-enabled on a route that must never show
it), which is the wrong failure mode for a security-adjacent guard.
This commit removes that class of bug instead of patching its latest
instance: _nested_route_reasoning_denied() now searches for the
vertex/gemini- or gemini_cli/gemini- pattern ANYWHERE in the string at
a non-alphanumeric boundary, rather than requiring it at position 0.
Correctness no longer depends on _strip_provider_hint_for_reasoning()
having stripped exactly the right prefix first — any number of opaque
wrapper layers (@provider:, a named custom-provider slug, or any
nesting scheme not yet invented) can precede the route and the deny
still fires.
Verified: all historical bypass strings (round 1 and round 2, plus a
hypothetical deeper double-wrapped case) now correctly deny; embedded
substrings that must NOT match (e.g. 'notvertex/gemini-x') correctly
don't, thanks to the boundary lookbehind.
Added test_nested_route_deny_is_boundary_based_not_prefix_based
locking in the structural invariant directly, independent of any
particular wrapper scheme.
Verification:
./scripts/test.sh tests/test_reasoning_effort_model_capabilities.py tests/test_custom_provider_bare_model_reasoning.py
# 54 passed in 1.31s
Full suite: 11257 passed, 64 pre-existing failures (identical file/test
set as the prior commit's baseline — network/credential/profile-isolation/fd-leak
tests, unrelated to reasoning_efforts).
