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.
Daniel Harcek