Release Q — v0.51.41 — 3-PR contributor batch (session recovery audit + run-lifecycle health + transcript dedup)

[nesquena-hermes]

Release Q — v0.51.41 — 3-PR contributor batch

Theme: Session recovery audit + run-lifecycle health + transcript dedup. Mixed-author, mixed-surface batch.

PRs included

• feat: add read-only session recovery audit #2036 by @ai-ag2026 — Stacked PR including fix: recover orphaned session backups on startup #2035 (orphan .bak startup recovery) + the read-only audit_session_recovery() API and module CLI. fix: recover orphaned session backups on startup #2035 auto-closes on merge.
• Fix session message identity dedup #2038 by @franksong2702 — Transcript dedup by message identity (closes Merged-messages dedup at routes.py:3033 keys on partial tuple — can drop legitimate retries #2027). Prefers id/message_id when present; falls back to legacy key for messages without IDs.
• fix: expose active run lifecycle in health #2039 by @ai-ag2026 — Active-run lifecycle in /health. Tracks WebUI worker runs separately from SSE streams so restart/update guards see worker state.

Verification

• Pytest: 5108 passed, 11 skipped, 1 xfailed, 2 xpassed EXIT 0 on Python 3.11 with HERMES_HOME isolation
• Diff: ~880 lines (incl. CHANGELOG), 11 files (mostly tests)
• node --check clean on all touched JS (none in this batch)
• Python AST clean on all touched .py files
• No merge-conflict markers anywhere

File collisions (stage merge)

• api/routes.py touched by all 3 PRs — verified disjoint hunks (fix: expose active run lifecycle in health #2039 at lines 2529/2609 for /health lifecycle, Fix session message identity dedup #2038 at line 3040 for transcript dedup, feat: add read-only session recovery audit #2036 at line 4147 for DELETE .bak unlink)
• api/session_recovery.py only fix: recover orphaned session backups on startup #2035/feat: add read-only session recovery audit #2036 (stacked)
• api/streaming.py / api/config.py only fix: expose active run lifecycle in health #2039
• CHANGELOG.md conflict on Fix session message identity dedup #2038 (predates v0.51.40 release entry) — resolved by preserving v0.51.40 history and re-adding Fix session message identity dedup #2038 bullet under [Unreleased] before promoting to v0.51.41 release entry

Opus advisor verdict

SHIP — three disjoint, well-scoped PRs. All concerns verified clean against code line-by-line. Two non-blocking nits flagged for follow-up. Full review posted as a separate comment.

Tests

5100 → 5108 (+8 net new across new test files for session-recovery audit, run-lifecycle health, transcript dedup, and orphan-backup recovery).

Self-agent review

PR #2035 received a substantive nesquena-hermes code-review comment with file:line citations confirming the tombstone state.db check is correct (fail-open) and the delete-side .bak unlink is the right complementary guard. Not counted as merge approval — requesting human review here.

Holds untouched

7 PRs with hold label left untouched per explicit non-hold scope: #1418, #1721, #1884, #1924, #1970, #1975, #1997.

Follow-ups

• Test isolation: os.execv in update-banner/restart tests re-executes the pytest suite. Suite still passes (EXIT 0) but wall-time inflates. Maintenance batch fix.
• idle_grace_remaining vs idle_seconds_since_last_run — confirm no external monitoring spec uses the older name (Opus nit).
• api/routes.py:6502 bg-session cleanup unlinks .json only — extend to .bak for consistency (Opus edge-case).

cc @nesquena — requesting independent review per self-built work policy.

[nesquena-hermes]

Opus Advisor review — stage-335 (Release Q)

Independent review by Claude Opus 4.7 (max thinking) on the merged stage diff (commit 4bbed44b, 3 effective PRs, +480 lines). Opus pulled actual files; verified each ask against the code line-by-line.

Verified

PR #2039 — active-run lifecycle (thread-safety concern)

• api/config.py:3691-3727 — ACTIVE_RUNS_LOCK = threading.Lock() declared, all three helpers (register_active_run/update_active_run/unregister_active_run) acquire it before mutating
• api/routes.py:2539 — _run_lifecycle_health() reads under the same lock
• Lock ordering: unregister_active_run runs inside finally while caller holds STREAMS_LOCK (api/streaming.py:3943); _handle_health calls _streams_lock_health() and _run_lifecycle_health() sequentially, never nested. No reverse lock-order path. Safe.
• Minor doc nit: brief promised idle_grace_remaining but actual payload key is idle_seconds_since_last_run (routes.py:2562 / 2658). CHANGELOG copy is fine ("idle grace timing"); flag if any monitoring spec mentions the older key.

PR #2038 — dedup-by-identity (falsy-id concern)

• api/routes.py:3084-3097 — message_identity = msg.get("id") or msg.get("message_id"). Empty-string / None / 0 short-circuits cleanly to the legacy (role, content, timestamp, tool_call_id, tool_name) tuple. Verified by tests/test_session_lineage_full_transcript.py:64 which exercises the distinct-id path. No regression risk.

PR #2036 / #2035 — orphan-backup startup recovery (startup IO concern)

• api/session_recovery.py:125-148 — _state_db_has_session() opens fresh sqlite3.connect(..., mode=ro) per orphan, indexed single-row lookup. Fails open on state.db missing/locked/older-schema (per design). Per-orphan ~1ms connection open is fine at realistic session-dir sizes. Could optimize later with single shared connection — flag as optional follow-up.
• api/routes.py:4195-4196 — DELETE handler unlinks both .json and .json.bak. Matches brief intent.
• Edge case (non-blocking): bg session cleanup at routes.py:6502 unlinks .json only. If a bg session crashed mid-write leaving .bak AND state.db was simultaneously unreadable at next boot, the fail-open path would restore the bg-session orphan as sidebar-visible. Requires two simultaneous failures; pre-existing data path; safe under the new tombstone check in the happy case.

Answers to asks

1. feat: add read-only session recovery audit #2036 startup IO — no blocking concern. Bounded by session-dir size, fail-open, ro connection. Optimization (single shared connection) is optional follow-up.
2. fix: expose active run lifecycle in health #2039 thread-safety — no concern. Lock properly declared and consistently acquired on both write and read paths; no nested-lock inversion.
3. Fix session message identity dedup #2038 falsy-id — no regression. or chain falls through to legacy key cleanly.

Verdict

SHIP → tag v0.51.41.

Three disjoint, well-scoped PRs. All concerns verified clean against the code on stage-335. No blocking issues, no cross-PR interference, regression coverage added for each change. The os.execv test pollution is pre-existing and already on the follow-up list — do not gate the release on it.

[nesquena]

Review — end-to-end ✅ (clean APPROVE, no fix pushed)

What this ships

Release Q (v0.51.41) — 3-PR contributor batch, mixed authors:

• #2035 + #2036 (@ai-ag2026, stacked) — Startup recovery now covers the orphan backup shape: if <sid>.json is gone but <sid>.json.bak survives, recreate the live sidecar. Guarded by a fail-open state.db.sessions tombstone check — explicit deletes do not resurrect. Adds a read-only audit_session_recovery() API + python -m api.session_recovery --audit ... CLI. Delete handler also unlinks <sid>.json.bak so deletes stay deleted.
• #2038 (@franksong2702) — /api/session transcript dedup now prefers id/message_id when present; falls back to the legacy (role, content, timestamp, tool_call_id, tool_name) tuple for messages without IDs. Closes #2027.
• #2039 (@ai-ag2026) — /health exposes worker-run lifecycle (active_runs, runs[], oldest_run_age_seconds, last_run_finished_at, idle_seconds_since_last_run) separate from SSE channel state. Wired through register_active_run / update_active_run / unregister_active_run in streaming.

~649 insertions / 19 deletions across 11 files. CI green on 3.11/3.12/3.13.

Traced against upstream hermes-agent

Pulled a fresh tarball (/tmp/hermes-agent-fresh). The changes are webui-internal and don't write anything new into config.yaml or the session schema. Confirmed:

• Dedup #2038 — Touches only the merge inside handle_get at api/routes.py:3081-3097. CLI sidecar messages and webui sidecar messages are merged with the new dedup key. Adding ("message_id", str(id)) vs ("legacy", ...) namespace tag means a literal id="legacy" cannot collide with the fallback. Verified by harness scenario 8 below.
• state.db tombstone check — Read-only mode=ro URI connection; if state.db is missing / locked / lacks sessions table, returns True (fail-open). Tests #2036 cover all four shapes.
• Active runs — Pure webui-internal registry. Does not feed into agent state, session files, or config.yaml.

End-to-end trace

#2035 + #2036 (api/session_recovery.py, server.py, api/routes.py)

• Boot path: server.py:223-228 now calls recover_all_sessions_on_startup(SESSION_DIR, rebuild_index=True, state_db_path=_active_state_db_path()).
• _orphaned_backup_live_paths() at api/session_recovery.py:148-180 — scans *.json.bak, skips entries that have a live counterpart, malformed payloads (_msg_count < 0), or are missing from state.db (when DB is readable + has sessions table + no row for sid).
• _state_db_has_session() at api/session_recovery.py:124-146 — sqlite3.connect(f"file:{path}?mode=ro", uri=True), indexed single-row lookup, parameterized query. Any exception → fail-open True. ~1 ms per orphan; bounded by session-dir size.
• recover_all_sessions_on_startup() at api/session_recovery.py:282-329 — runs recover_session() for both shrunken-live and orphan-bak paths, optionally rebuilds _index.json via api.models._write_session_index.
• audit_session_recovery() at api/session_recovery.py:206-280 — pure read-only classifier producing {status, summary, items}. Categories: shrunken_live, orphan_backup, orphan_backup_without_state_row, malformed_orphan_backup, index_missing_file, index_missing_entry.
• CLI module entry at api/session_recovery.py:336-349 — argparse, output is json.dumps(report). No mutating modes.
• Delete handler at api/routes.py:4193-4197 — unlinks both <sid>.json and <sid>.json.bak.

#2038 (api/routes.py:3084-3097)

message_identity = msg.get("id") or msg.get("message_id")
if message_identity:
    key = ("message_id", str(message_identity))
else:
    key = ("legacy", role, content, timestamp, tool_call_id, tool_name)

• Namespace prefix prevents collision between literal id="legacy" and the legacy 6-tuple.
• Falsy id (None, "", 0) short-circuits to legacy path — no behavioural regression for ID-less messages.

#2039 (api/config.py, api/streaming.py, api/routes.py)

• Registry declaration at api/config.py:3691-3694 — ACTIVE_RUNS: dict, ACTIVE_RUNS_LOCK = threading.Lock(), LAST_RUN_FINISHED_AT: float | None.
• Helpers at api/config.py:3699-3727 — all three acquire the lock before mutation. update_active_run() is no-op on missing key (does not implicitly create — verified). Empty stream_id rejected.
• Wiring in api/streaming.py:
  • register_active_run(...) at api/streaming.py:2034-2043 (entry, after q is None early-return).
  • update_active_run(stream_id, phase="running", ...) at api/streaming.py:2225 (inside main try, after s = get_session(...)).
  • update_active_run(stream_id, phase="finalizing") at api/streaming.py:3932 (inside finally, before sync).
  • unregister_active_run(stream_id) at api/streaming.py:3943 (inside finally + STREAMS_LOCK).
• Health snapshot at api/routes.py:2532-2570 — acquires _live_config.ACTIVE_RUNS_LOCK, materializes a list copy, releases lock before sort/serialize. Imports api.config as _live_config to dodge the import-alias staleness trap for LAST_RUN_FINISHED_AT.
• _handle_health at api/routes.py:2645-2666 — calls _streams_lock_health() and _run_lifecycle_health() sequentially. No nested locks.

Other audit — things that are correct already

Security

• sqlite3.connect uses mode=ro URI form + parameterized queries. No SQL injection vector. (One minor caveat below.)
• audit_session_recovery() is read-only by design; no Path.write_text / Path.unlink anywhere in its call tree.
• CLI is admin-invoked; argparse consumes --session-dir / --state-db as Path — no shell expansion.
• register_active_run accepts **metadata but _run_lifecycle_health() only consumes well-known fields and serializes via dict(raw or {}). No reflection-based attacks possible.

Thread safety

• ACTIVE_RUNS_LOCK properly held on every read and write. Verified concurrent register/update/unregister across 20 threads — no leaks, no races (harness below).
• Lock ordering: unregister_active_run(stream_id) runs inside the with STREAMS_LOCK: body at api/streaming.py:3935-3943, then internally acquires ACTIVE_RUNS_LOCK. Reader _handle_health acquires STREAMS_LOCK (via _streams_lock_health) first, releases it, then acquires ACTIVE_RUNS_LOCK. Both orderings are STREAMS_LOCK → ACTIVE_RUNS_LOCK. No inversion path. Safe.
• _state_db_has_session() uses a fresh per-call connection — no shared SQLite handle race.

Index rebuild idempotency

• _write_session_index(updates=None) is called only when rebuild_index=True AND at least one restore happened. Falsy _index.json → rebuilt. Test test_recover_all_sessions_on_startup_rebuilds_index_after_orphan_restore exercises a stale index path.

Delete path

• p.with_suffix('.json.bak').unlink(missing_ok=True) — .unlink(missing_ok=True) since the bak may or may not exist. Same try/except block as the .json unlink, same logger handling. Symmetric. Regression test enforces this at tests/test_regressions.py:339-348.

Behavioural harnesses

#2038 dedup (Python) — 8/8 scenarios pass:

Scenario 1 (distinct ids, same content):       2 retained ✓
Scenario 2 (true dup, same id):                1 retained ✓
Scenario 3 (legacy collapse identical):         1 retained ✓
Scenario 4 (legacy ts-diff):                    2 retained ✓
Scenario 5 (id vs no-id same content):          2 retained ✓ (namespace tag)
Scenario 6 (message_id fallback):               1 retained ✓
Scenario 7 (empty-string id → legacy):          1 retained ✓
Scenario 8 (id='legacy' vs legacy-tuple):       2 retained ✓ (namespace tag)

#2039 lifecycle (Python) — register/update/unregister roundtrip, empty-stream-id rejection, no-op update on missing key, 20-thread concurrent stress (no leaks), _run_lifecycle_health snapshot, idle reporting. All confirmed.

#2036 recovery (Python) — 6/6 fail-open paths:

No state.db                       → repairable (fail-open)        ✓
state.db w/o sessions table       → repairable (fail-open)        ✓
state.db missing 'sessions' table → repairable (fail-open)        ✓
state.db missing file path        → fail-open True                 ✓
state.db = None                   → fail-open True                 ✓
state.db has sid                  → repairable; orphan restored    ✓
state.db lacks sid                → unsafe_to_repair               ✓

Edge-case trace

Scenario	Expected	Actual
Orphan .bak, state.db has row	restored	✅ harness
Orphan .bak, state.db lacks row	skipped (tombstoned)	✅ harness
Orphan .bak, state.db missing	fail-open → restored	✅ harness
Orphan .bak, malformed JSON	skipped	✅ _msg_count < 0 guard
_* system files in session dir	skipped	✅ name.startswith('_')
Stale _index.json after restore	rebuilt	✅ test #1558
Dedup: retry with distinct ids	both survive	✅ harness
Dedup: literal id="legacy"	does not collide	✅ namespace tag
update_active_run on missing sid	no-op (no implicit create)	✅ harness
20-thread concurrent register/unregister	no leaks	✅ harness
_handle_health while runs active	reports active_runs > 0	✅ test
_handle_health after all done	reports idle_seconds_since_last_run	✅ test
STREAMS_LOCK ↔ ACTIVE_RUNS_LOCK ordering	one-way only	✅ code review
Delete handler removes .bak	unlinked	✅ regression test
CLI audit on clean dir	status="ok", summary.ok=N	✅ test

Tests

• PR-targeted: 69/69 pass (1 unrelated SSE smoke test correctly skips).
• Full suite (Python 3.14): 4996 passed, 59 skipped, 3 xpassed, 0 failed (ignoring known test_docker_env_readonly_vars.py macOS bash 3.2 baseline failures + test_ctl_script.py, neither touched by this PR).
• PR's CI: 5108 / 11 skipped / 1 xfailed / 2 xpassed in ~160 s on Python 3.11 (per PR body); 3.11 / 3.12 / 3.13 all green.

Minor observations (non-blocking)

1. register_active_run lives ~190 lines before the main try: block. At api/streaming.py:2034 (register) vs api/streaming.py:2223 (try-start). The intervening code is dict mutations, closure definitions, and time.time() — none of which raise in practice — but a future change between these lines that throws would leak an ACTIVE_RUNS entry. Two options for a follow-up: (a) move register_active_run inside the try-block, or (b) wrap the intervening setup in its own try/except that calls unregister_active_run on failure. Not a blocker — current code paths are exception-clean.

2. sqlite3.connect(f"file:{state_db_path}?mode=ro", uri=True) interpolates state_db_path into a URI. If the path ever contained ? or #, the URI parser would mangle. In practice the path comes from _active_state_db_path() (deterministic, admin-controlled) or --state-db CLI (admin). No exploitable shape today. Defensive follow-up: urllib.parse.quote(str(state_db_path), safe='/') before interpolation.

3. Doc nit (already flagged by Opus advisor): brief promised idle_grace_remaining, actual payload key is idle_seconds_since_last_run at api/routes.py:2562. CHANGELOG copy says "idle grace timing" which is fine. If any external monitoring spec references the older name, update it.

4. bg-session cleanup at api/routes.py:6502 unlinks .json only. If a bg session ever leaves a .bak, the orphan-recovery path would later restore it (when state.db is unreadable). Same fix shape as #2036's delete-handler fix; flag as a small follow-up.

5. os.execv test pollution — already on the follow-up list per PR body. Not related to this batch.

Recommendation

Approved. Three disjoint, well-scoped PRs. State.db tombstone check is correctly fail-open. Dedup namespace tag rules out namespace collisions. Active-run lock ordering is one-way only. Behavioural harnesses confirm every claimed invariant. Code, tests, and CHANGELOG align.

✅ Parked at approval — ready for the release agent's merge/tag pipeline.