Skip to content

Fix WebUI reconciliation after Desktop session continuation - #4834

Closed
franksong2702 wants to merge 3 commits into
nesquena:masterfrom
franksong2702:fix/desktop-webui-state-db-reconcile
Closed

franksong2702 wants to merge 3 commits into
nesquena:masterfrom
franksong2702:fix/desktop-webui-state-db-reconcile

Conversation

@franksong2702

@franksong2702 franksong2702 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Reproduced the settled-state mismatch with a regression test that first caches /api/sessions from a WebUI sidecar, then simulates the official Hermes Desktop App appending settled rows to the same Hermes Agent state.db session.
  • Traced /api/sessions through the sidebar cache and all_sessions(include_lineage_metadata=False) path.
  • Found two reconciliation gaps: the sidebar state-db override only carried source/title metadata (not settled message count/time), and the session-list cache stamp ignored state.db entirely when external/CLI sessions were hidden.
  • After CI exposed the shard-2 hang, traced the cache follower path: once /api/sessions started observing state.db stamps, a stamp mismatch could discard the stale payload before a follower request could use it, forcing that follower to wait behind a slow owner rebuild.
  • After CI exposed a neighboring shard-1 race, tightened the wait-stage test so it deterministically waits until the follower is actually blocked before releasing the owner.
  • After live 8787 audit found settled worker sessions readable via /api/session but missing from /api/sessions, traced the index path: it pruned stale rows and overlaid active in-memory sessions, but did not recover persisted sidecar JSON files missing from a non-empty _index.json once the runtime overlay was gone.
  • Kept the scope to settled-state reconciliation and sidebar/cache behavior only; no Desktop App change, no live stream mirroring, no multi-client conflict behavior change, and no Manual compression is undone by state.db reconciliation and .bak startup recovery #4836 compression recovery work.

What Changed

  • Batch-read state.db.messages count and max timestamp in the existing sidebar state-db override helper.
  • Overlay newer WebUI-origin state.db count/time onto stale WebUI sidecar rows when the DB has strictly grown beyond the sidecar.
  • Recover messageful persisted WebUI sidecar sessions that are missing from a non-empty sidebar _index.json, using metadata-only loads, then write those recovered rows back to the index.
  • Make /api/sessions cache stamps observe state.db even when CLI/external sessions are hidden, while preserving the existing active-stream hold-down behavior.
  • Preserve the existing hot-path invariant for sidebar cache followers: if a stale payload exists, a follower can reuse it while the owner performs a slow rebuild, even when the source stamp changed.
  • Harden cache follower tests with try/finally cleanup so failed assertions release the owner builder and join threads instead of orphaning them until CI timeout.
  • Added/updated regressions for:
    • WebUI-origin /api/sessions metadata updating after Desktop-settled state.db append.
    • /api/session?messages=1 returning one reconciled transcript instead of duplicating a sidecar prefix.
    • the next WebUI turn saving a single reconciled history that includes externally appended settled messages.
    • cache followers reusing stale payloads during slow rebuilds after a source-stamp change.
    • a settled messageful sidecar that Session.load() can read still appearing in sidebar rows even when a non-empty _index.json missed it and the in-memory runtime overlay is gone.
  • Updated CHANGELOG.md.

Why It Matters

Users can continue a WebUI-created session from the official Hermes Desktop App and then return to WebUI. After the Desktop turn settles, WebUI now shows the complete final transcript in the sidebar/detail view and uses that settled history as context for the next WebUI message, without duplicating old rows in the sidecar.

It also keeps completed worker sessions discoverable in the sidebar after the active runtime overlay disappears, as long as their sidecar JSON remains readable by session detail.

Fixes #4833.

Verification

  • ./scripts/test.sh tests/test_streaming_session_sidebar.py::test_messageful_session_missing_from_index_is_recovered_in_sidebar -q — 1 passed.
  • ./scripts/test.sh tests/test_session_sidebar_cache.py::test_session_list_cache_follower_reuses_stale_payload_during_slow_rebuild -q — 1 passed.
  • ./scripts/test.sh tests/test_session_sidebar_cache.py::test_session_list_cache_follower_wait_stage_when_rebuild_inflight tests/test_session_sidebar_cache.py::test_session_list_cache_follower_reuses_stale_payload_during_slow_rebuild -q — 2 passed.
  • ./scripts/test.sh tests/test_streaming_session_sidebar.py tests/test_session_sidebar_cache.py tests/test_webui_state_db_reconciliation.py tests/test_webui_state_db_context_reconciliation.py tests/test_session_db_sidecar_reconciliation.py tests/test_issue4766_sidebar_source_pushdown.py tests/test_cli_sessions_cache_fingerprint.py tests/test_issue3930_source_filter_pushdown.py tests/test_issue1494_state_db_fd_leak.py -q — 90 passed.
  • .venv/bin/python -m py_compile api/models.py api/routes.py tests/test_streaming_session_sidebar.py tests/test_session_sidebar_cache.py tests/test_webui_state_db_reconciliation.py tests/test_webui_state_db_context_reconciliation.py — passed.
  • git diff --check — passed.

Risks / Follow-ups

  • /api/sessions now observes state.db for WebUI-only sidebar views. The existing active-stream hold-down remains in place to avoid per-token cache churn during WebUI streaming.
  • Recovering missing sidecar index rows uses metadata-only loads and only triggers for persisted session IDs missing from the non-empty index; the recovered rows then flow through the existing visibility filters.
  • Stale follower fallback is intentionally preserved only as a fallback while an owner rebuild is in-flight; the owner still refreshes and stores the updated payload.
  • This does not mirror Desktop live tokens into WebUI and does not change simultaneous-send conflict semantics.
  • Overlay is limited to WebUI-origin state.db rows and only when state.db has grown beyond the sidecar, to avoid resurrecting restamped/replayed histories.
  • Manual compression recovery remains separate in Manual compression is undone by state.db reconciliation and .bak startup recovery #4836 and is not part of this PR.

Model Used

OpenAI Codex GPT-5.5

@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes WebUI session reconciliation when a Hermes Desktop App continues a WebUI-origin session: the sidebar now shows the correct final message count/timestamp, the detail view returns the full merged transcript without duplicating the sidecar prefix, and persisted sidecar sessions that were dropped from _index.json are recovered on the next sidebar load.

  • Sidebar state-db count/time overlay (api/models.py): _read_state_db_sidebar_overrides now batch-reads messages count and max timestamp per chunk, and _apply_sidebar_state_db_override_metadata overlays those onto WebUI-origin sidecar rows when state.db has grown.
  • Cache stamp fix (api/routes.py): _session_list_cache_source_stamp no longer short-circuits to zero-stamps when show_cli_sessions is False, so WebUI-only sidebar views detect state.db changes; stale-payload followers now receive the cached payload immediately instead of discarding it when the stamp has changed.
  • Missing index recovery (api/models.py): all_sessions() detects persisted session IDs absent from a non-empty _index.json (after the in-memory overlay) and reloads them via metadata-only reads, writing recovered rows back to the index.

Confidence Score: 4/5

Safe to merge; the changes are scoped to the sidebar reconciliation path and cache follower fallback, and are backed by targeted regression tests for each scenario.

The reconciliation logic, stale-follower cache path, and index recovery are each narrow and well-tested. The one area worth a second look is _apply_sidebar_state_db_override_metadata: when state.db has messages but no timestamps (state_last = 0), the overlay fires on count alone, which could theoretically apply a replayed history count — a known trade-off documented in comments but without a test for that edge.

api/models.py — the overlay guard state_last <= 0 or state_last > current_last in _apply_sidebar_state_db_override_metadata is the most nuanced part of the change; all other files are straightforward.

Important Files Changed

Filename Overview
api/models.py Core reconciliation logic: adds per-chunk messages count/timestamp query to _read_state_db_sidebar_overrides and conditional overlay in _apply_sidebar_state_db_override_metadata; adds missing-index recovery loop in all_sessions(). Logic is sound.
api/routes.py Removes the short-circuit that zeroed the cache stamp when show_cli_sessions=False, and adds a stale-payload return path in _session_list_cache_get so followers can reuse a cached payload while the owner rebuilds after a stamp change.
tests/test_session_sidebar_cache.py Adds autouse fixture for cache isolation, monkeypatches stable stamps in threading tests, hardens follower-wait test with wait_seen Event for determinism, and wraps thread joins with try/finally.
tests/test_streaming_session_sidebar.py New regression test verifies a messageful sidecar missing from a non-empty _index.json is recovered in all_sessions().
tests/test_webui_state_db_reconciliation.py New tests cover sidebar overlay after Desktop append, no-duplication on full session load, and float actual_message_count handling.
tests/test_webui_state_db_context_reconciliation.py Adds saved-context assertions confirming no message duplication in the persisted sidecar after a reconciled turn.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client as Browser/Sidebar
    participant Cache as SessionListCache
    participant Owner as Owner Thread
    participant DB as state.db
    participant Index as _index.json

    Note over Client,Index: Normal path — stamp fresh
    Client->>Cache: "_session_list_cache_get(allow_stale=True)"
    Cache-->>Client: "(payload, is_fresh=True) → cache hit"

    Note over Client,Index: Desktop App settles rows → stamp changes
    Client->>Cache: "_session_list_cache_get(allow_stale=True)"
    Cache-->>Client: "(stale_payload, is_fresh=False)"
    Client->>Owner: claim_rebuild (become owner)
    Owner->>DB: _read_state_db_sidebar_overrides (count + MAX timestamp per chunk)
    DB-->>Owner: state_count, state_last_message_at
    Owner->>Owner: overlay count/time onto WebUI-origin sidecar rows
    Owner->>Cache: _session_list_cache_set(key, fresh_payload)

    Note over Client,Index: Concurrent follower while owner rebuilds
    Client->>Cache: "_session_list_cache_get(allow_stale=True)"
    Cache-->>Client: "(stale_payload, is_fresh=False)"
    Client->>Cache: claim_rebuild → follower, wait(stale_wait_seconds)
    Owner-->>Client: event.set()
    Client->>Cache: "_session_list_cache_get(allow_stale=False)"
    Cache-->>Client: (fresh_payload, True)

    Note over Client,Index: Missing index recovery (all_sessions)
    Owner->>Index: read _index.json
    Owner->>Owner: prune stale rows, overlay in-memory SESSIONS
    Owner->>Owner: diff persisted_ids vs index_map → missing_ids
    loop each missing_id
        Owner->>Owner: Session.load_metadata_only(sid)
        Owner->>Index: add recovered row to index_map + write back
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client as Browser/Sidebar
    participant Cache as SessionListCache
    participant Owner as Owner Thread
    participant DB as state.db
    participant Index as _index.json

    Note over Client,Index: Normal path — stamp fresh
    Client->>Cache: "_session_list_cache_get(allow_stale=True)"
    Cache-->>Client: "(payload, is_fresh=True) → cache hit"

    Note over Client,Index: Desktop App settles rows → stamp changes
    Client->>Cache: "_session_list_cache_get(allow_stale=True)"
    Cache-->>Client: "(stale_payload, is_fresh=False)"
    Client->>Owner: claim_rebuild (become owner)
    Owner->>DB: _read_state_db_sidebar_overrides (count + MAX timestamp per chunk)
    DB-->>Owner: state_count, state_last_message_at
    Owner->>Owner: overlay count/time onto WebUI-origin sidecar rows
    Owner->>Cache: _session_list_cache_set(key, fresh_payload)

    Note over Client,Index: Concurrent follower while owner rebuilds
    Client->>Cache: "_session_list_cache_get(allow_stale=True)"
    Cache-->>Client: "(stale_payload, is_fresh=False)"
    Client->>Cache: claim_rebuild → follower, wait(stale_wait_seconds)
    Owner-->>Client: event.set()
    Client->>Cache: "_session_list_cache_get(allow_stale=False)"
    Cache-->>Client: (fresh_payload, True)

    Note over Client,Index: Missing index recovery (all_sessions)
    Owner->>Index: read _index.json
    Owner->>Owner: prune stale rows, overlay in-memory SESSIONS
    Owner->>Owner: diff persisted_ids vs index_map → missing_ids
    loop each missing_id
        Owner->>Owner: Session.load_metadata_only(sid)
        Owner->>Index: add recovered row to index_map + write back
    end
Loading

Reviews (6): Last reviewed commit: "Fix sidebar reconciliation review issues" | Re-trigger Greptile

Comment thread CHANGELOG.md Outdated
Comment on lines +6 to +8
### Fixed

- **WebUI-created sessions stay in sync after the official Hermes Desktop App continues the same Hermes Agent session.** The conversation sidebar now refreshes WebUI-origin rows from settled `state.db` message counts/timestamps even when external/CLI sessions are hidden, full session loads avoid duplicating the sidecar prefix when merging `state.db`, and the next WebUI turn saves a single reconciled transcript instead of re-writing duplicated history.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 CHANGELOG.md should not be touched by contributor PRs

Per the project's process, CHANGELOG.md is maintained exclusively by the release agent via release: vX.Y.Z commits. Individual contributor PRs must not edit this file directly. Please revert this change; the release automation will pick up the entry when the next version ships.

Rule Used: Do not flag missing CHANGELOG.md updates on indivi... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread api/models.py Outdated
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Read the diff at HEAD against origin/master (api/models.py:3226-3367, api/routes.py:1761-1780) plus the new tests/test_webui_state_db_reconciliation.py. The reconciliation logic is sound and I can see it's already staged for release in #4834's release PR. One narrow edge in the overlay guard worth noting for a possible follow-up.

The overlay guard

The count/time overlay in _apply_sidebar_state_db_override_metadata() is gated by:

if state_count > current_count and (state_last <= 0 or state_last > current_last):
    session['message_count'] = state_count
    ...

with current_last = max(last_message_at, updated_at) of the sidecar row. The state_last <= 0 branch correctly handles a messages table without a timestamp column (messages_has_timestamp False → last_message_at NULL), and the strict state_count > current_count + state_last > current_last pair is the right conservative anti-resurrection rule the PR body promises.

The edge

Because current_last folds in the sidecar's updated_at, a metadata-only sidecar write that lands after the Desktop append (e.g. a title rename or source-tag rewrite bumping updated_at) makes current_last > state_last, so the count growth is silently dropped even though messages genuinely grew. The new test uses strictly increasing message timestamps (1002/1003 vs sidecar 1001) so it never exercises a updated_at > state_last case. This is consistent with "favor not-resurrecting," so not a blocker — but a regression that appends Desktop rows then touches sidecar metadata would lock in the conservative tradeoff. If that's intended, a one-line comment on the guard would save a future reader the head-scratch; if not, gating on message-count growth alone (dropping updated_at from current_last) would catch it.

The no-duplicate-prefix merge assertion in test_api_session_full_load_does_not_duplicate_state_db_prefix is the right shape and directly covers the #4833 symptom. Nice work.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks @franksong2702 — the Desktop-continuation reconciliation logic itself is sound (Codex SAFE + Opus SAFE: the new sidebar overlay is read-only, content-free, and structurally can't cause the ghost/message-loss class). But it can't ship yet: it introduces a parallel test-isolation regression that the full sharded suite reproduces.

The failure

tests/test_session_sidebar_cache.py::test_session_list_cache_follower_wait_stage_when_rebuild_inflight fails under the full parallel (xdist) suite on this branch — reproduced twice locally, and CI fails it on two independent shards (test (3.12, 1) + test (3.13, 1)). It passes in isolation and in a small slice, and it passes on clean master under the same full xdist run (10364 passed) — so the branch is the cause, not a pre-existing flake.

Likely cause

The change removes the if not cache_show_cli_sessions: guard in _session_list_cache_source_stamp so the state.db signal is always folded into the cache stamp (the "refresh WebUI rows even when CLI sessions are hidden" fix). That makes the source-stamp read state.db on every cache check, which appears to perturb the timing/global-state the single-flight follower-wait test depends on when other tests run concurrently. The test uses a release.wait() + _StageRecorder single-flight rebuild handshake, so it's sensitive to cross-test cache-state bleed under parallelism.

Fix direction

Make the new always-on state.db stamp read isolation-safe under the test suite, or update the single-flight cache tests to reset/scope the shared _SESSIONS_CACHE state they rely on so the new stamp behavior doesn't leak across parallel workers. Reproduce with the full suite under xdist (not -p no:xdist, and not the cache file alone — it only surfaces with the whole suite running in parallel):

PYTHONPATH=. python -m pytest tests/ -q

Everything else checks out — both gates SAFE on the reconciliation logic, 31 reconciliation tests green, serial full suite green. It's specifically the parallel-isolation interaction with the sidebar-cache single-flight test. Re-gate when the full xdist suite is green. Keeping changes-requested.

@nesquena-hermes nesquena-hermes added the changes-requested Maintainer left detailed feedback requesting changes; PR is waiting on author to address label Jun 24, 2026
@franksong2702
franksong2702 force-pushed the fix/desktop-webui-state-db-reconcile branch 3 times, most recently from 06fec29 to 873ca8e Compare June 25, 2026 01:19
@franksong2702
franksong2702 force-pushed the fix/desktop-webui-state-db-reconcile branch from 873ca8e to cdb1118 Compare June 25, 2026 01:22
@nesquena-hermes nesquena-hermes added size:L Large PR (>10 files or >250 LOC) and removed size:M Medium PR (≤10 files, ≤250 LOC) labels Jun 25, 2026
…tate-db-reconcile

# Conflicts:
#	CHANGELOG.md
@franksong2702

Copy link
Copy Markdown
Contributor Author

Updated this branch against current master and resolved the remaining DIRTY state. The conflict was limited to CHANGELOG.md; the session reconciliation code paths auto-merged cleanly.

Local verification:

  • ./scripts/test.sh tests/test_session_sidebar_cache.py tests/test_streaming_session_sidebar.py tests/test_webui_state_db_context_reconciliation.py tests/test_webui_state_db_reconciliation.py -q
  • 55 passed

GitHub CI is now green on 0d6ffcb. I also ran the full local suite. It is not clean on this machine due to failures that reproduce on clean origin/master as well, so I am relying on the PR-specific local slice plus GitHub CI for the final gate. Ready for re-review.

@franksong2702

Copy link
Copy Markdown
Contributor Author

Addressed the review items on this branch.

What changed:

  • Rebased/updated the PR branch onto current origin/master and removed the contributor CHANGELOG diff from the final PR tree.
  • Replaced the .isdigit() actual_message_count coercion with try/int(), plus a regression for numeric values such as 5.0.
  • Isolated the session-list cache tests by clearing cache + inflight rebuild state before/after each test and pinning the pure single-flight thread tests to a stable source stamp.
  • Added a comment documenting the conservative updated_at/state.db anti-resurrection guard.

Verification:

  • git diff --check
  • ./scripts/test.sh tests/test_session_sidebar_cache.py tests/test_webui_state_db_reconciliation.py::test_sidebar_state_db_overlay_preserves_numeric_actual_count tests/test_webui_state_db_reconciliation.py::test_api_sessions_overlays_webui_state_db_summary_after_desktop_append -q (18 passed)
  • ./scripts/test.sh tests/test_session_sidebar_cache.py tests/test_streaming_session_sidebar.py tests/test_webui_state_db_reconciliation.py tests/test_webui_state_db_context_reconciliation.py -q (56 passed)
  • ./scripts/test.sh tests/ -q completed the full xdist run; the previously reported session-sidebar-cache failure did not reproduce. Local full-suite result was 10470 passed / 106 skipped / 4 failed, with the remaining failures in unchanged provider/TLS/agent-import tests: test_issue1426_openrouter_free_tier_live_fetch.py::test_free_tier_cap_prevents_picker_drowning, test_minimax_provider.py::test_minimax_cn_detected_from_os_environ, test_tls_support.py::TestTLSEndToEnd::test_tls_startup_failure_fallback_to_http, and test_v050259_sessiondb_fd_leak.py::test_session_db_close_is_idempotent.

nesquena-hermes added a commit that referenced this pull request Jun 26, 2026
…fixes #4833)

Release XX (v0.51.668): WebUI<->Desktop session reconciliation (#4834, fixes #4833)
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.668 (Release XX, just deployed) — thanks @franksong2702! Fixes #4833. WebUI-origin sessions now reconcile correctly after the Desktop app continues them (sidebar refreshes from settled state.db count/time even with CLI sessions hidden; metadata-only overlay, strictly-grew, active-stream hold-down preserved). The prior bounce (parallel test-isolation regression) is resolved — the follower-wait test now passes under the full xdist suite. Gate: Codex SAFE + Opus SHIP IT (test-isolation fix confirmed test-only; overlay read-only/no-resurrection), suite 10631. Verified on prod.

yud657 pushed a commit to yud657/hermes-webui that referenced this pull request Jun 26, 2026
Du7chManiac pushed a commit to TheCouchCoder-com/hermes-webui that referenced this pull request Jul 6, 2026
v0.51.668 — Release XX: WebUI<->Desktop session reconciliation (nesquena#4834, fixes nesquena#4833)

# Conflicts:
#	CHANGELOG.md
#	tests/test_webui_state_db_reconciliation.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes-requested Maintainer left detailed feedback requesting changes; PR is waiting on author to address size:L Large PR (>10 files or >250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebUI falls out of sync after Hermes Desktop continues a WebUI-created session

2 participants