diff --git a/CHANGELOG.md b/CHANGELOG.md index c7e96228..5df5a536 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **Manual `/compress` now sticks across reloads and restarts instead of springing back to the old context size.** A manual compression shrinks the model-facing context but the old transcript could return through two paths: (1) append-only `state.db` reconciliation re-appending pre-compression rows, and (2) startup `.bak` recovery treating the intentional shrink as data loss. `/compress` now persists a truncation watermark/boundary (so reconciliation stops replaying pre-compression rows), refreshes the post-compression token metric, and drops the now-stale backup. Crucially, the `.bak` safeguard is only suppressed for the *pre-compression* backup whose restore would undo the shrink (detected by content — its model-facing context is still the larger uncompressed one) — a backup written *after* the compression that captures genuine post-compression data loss is **still recovered**, so manual compression can never permanently disable crash-recovery for that session. Thanks @hyl-ailab. (#4986, fixes #4836) + - **Typed composer input is no longer wiped when a saved session auto-opens on boot.** On a hard refresh, boot-restore auto-opens the last session from localStorage — but that load can finish *after* you've already started typing into the fresh composer, and the saved-session draft restore would clobber what you typed. The boot-restore path now passes `preserveActiveInput` through to the existing draft-restore guard, so your in-progress text wins over the saved draft; a blank composer still restores the saved draft as before. Thanks @rodboev. (#5069, fixes #5060) - **The Hermes Dashboard now has a show/hide chip in Settings → Appearance.** The Appearance → "Sidebar tabs" grid (where you toggle which tabs appear in the sidebar/rail) gains a "Hermes Dashboard" chip alongside the others, bidirectionally synced with the existing dashboard-mode dropdown (never/always/auto): toggling the chip off sets the dropdown to "never," toggling it on restores the last non-never mode, and changing the dropdown updates the chip. The chip is a proxy for the dashboard config — it never mutates the sidebar `hidden_tabs`/`tab_order` state — and the sync is guarded against a stale config GET clobbering a newer save, with a clean rollback if the save fails. Thanks @rodboev. (#4936, fixes #2724) diff --git a/api/routes.py b/api/routes.py index 60043668..f5db7d1c 100644 --- a/api/routes.py +++ b/api/routes.py @@ -19138,7 +19138,12 @@ def _handle_session_compress(handler, body): if _sanitize_messages_for_api(s.messages) != original_messages: return bad(handler, "Session was modified during compression; please retry.", 409) - s.context_messages = copy.deepcopy(compressed) + from api.session_ops import _truncation_watermark_for + from api.streaming import _stamp_missing_message_timestamps + + compressed_copy = copy.deepcopy(compressed) + _stamp_missing_message_timestamps(compressed_copy) + s.context_messages = compressed_copy s.active_stream_id = None s.pending_user_message = None s.pending_attachments = [] @@ -19153,7 +19158,19 @@ def _handle_session_compress(handler, body): s.compression_anchor_summary = _compact_summary_text( summary_text or _compression_summary_from_messages(compressed) or "" ) + # Persist an intentional-shrink boundary so append-only state.db + # reconciliation does not replay pre-compression rows (#4836). + compress_watermark = _truncation_watermark_for(compressed_copy) + s.truncation_watermark = compress_watermark + s.truncation_boundary = compress_watermark + s.compression_anchor_mode = "manual" + s.last_prompt_tokens = new_tokens s.save() + # Drop stale backups that would undo an intentional manual compress. + try: + s.path.with_suffix(".json.bak").unlink(missing_ok=True) + except OSError: + pass session_payload = redact_session_data( s.compact() | { diff --git a/api/session_recovery.py b/api/session_recovery.py index 336da319..6dce748a 100644 --- a/api/session_recovery.py +++ b/api/session_recovery.py @@ -65,6 +65,106 @@ def _msg_count(p: Path) -> int: return len(msgs) if isinstance(msgs, list) else -1 +def _session_records_intentional_compress_shrink(session_path: Path) -> bool: + """Return True when the live sidecar records an intentional context shrink. + + Manual ``/compress`` keeps the visible transcript but replaces the + model-facing ``context_messages`` with a smaller compacted prefix. That + operation must not be treated as accidental data loss by the #1558 + ``.bak`` safeguard or startup recovery (#4836). + + NOTE: this only reports *whether* the live session was intentionally + compressed. It must NOT, on its own, decide to skip ``.bak`` recovery — + a session can be manually compressed and *then* later suffer a genuine + #1558 ``messages``-array loss. The caller pairs this with a freshness + check (the backup must predate the compression) so a real post-compress + loss is still recovered. See ``inspect_session_recovery_status``. + """ + try: + data = json.loads(session_path.read_text(encoding='utf-8')) + except (OSError, json.JSONDecodeError, ValueError): + return False + if not isinstance(data, dict): + return False + + context_messages = data.get('context_messages') + messages = data.get('messages') + has_shorter_context = ( + isinstance(context_messages, list) + and isinstance(messages, list) + and len(context_messages) < len(messages) + ) + anchor_summary = str(data.get('compression_anchor_summary') or '').strip() + anchor_key = data.get('compression_anchor_message_key') + mode = str(data.get('compression_anchor_mode') or '').strip().lower() + watermark = data.get('truncation_watermark') + + if mode == 'manual': + return True + if has_shorter_context and anchor_summary and anchor_key is not None: + return True + if has_shorter_context and watermark is not None: + return True + return False + + +def _backup_predates_intentional_shrink(session_path: Path, bak_path: Path) -> bool: + """True when the ``.bak`` captured the PRE-compression transcript. + + The intentional-compress guard (#4836) must only suppress recovery for a + backup whose restore would *undo* the user's deliberate shrink — i.e. a + backup taken before ``/compress`` ran, still carrying the large + UNCOMPRESSED ``context_messages``. A backup written *after* the + compression (a later save that lost data) carries the ALREADY-COMPRESSED + context and MUST still be recoverable. + + Time can't tell these apart (the ``.bak`` and main file are written in the + same ``save()`` milliseconds apart), so we use the same content-semantic + signal reconciliation uses: the compaction marker. A post-compression + backup carries the ``[context compaction…]`` marker in its + ``context_messages`` (it persists across saves); a pre-compression backup + does not. We suppress recovery ONLY when the backup is genuinely + pre-compression — its context lacks the compaction marker AND is larger + than the live compressed context (the shrink hasn't been applied to it). + If the backup already carries the marker, it post-dates the compression → + a real loss → recover. The length clause is a secondary guard for the rare + compression that emits no marker; whenever in doubt we fail OPEN (return + False, allow recovery) — for a data-loss safeguard, recovering real data + is always the safer error. Fail OPEN on any read/parse error too. + """ + try: + live = json.loads(session_path.read_text(encoding='utf-8')) + bak = json.loads(bak_path.read_text(encoding='utf-8')) + except (OSError, json.JSONDecodeError, ValueError): + return False + if not isinstance(live, dict) or not isinstance(bak, dict): + return False + + bak_ctx = bak.get('context_messages') + bak_ctx = bak_ctx if isinstance(bak_ctx, list) else [] + + # If the backup's context already carries the compaction marker, the backup + # post-dates the compression (the marker persists across saves) → it is a + # recoverable post-compression snapshot, never a shrink-undoing one. + try: + from api.models import _context_messages_include_compression_marker + if _context_messages_include_compression_marker(bak_ctx): + return False + except Exception: + # If the marker check is unavailable, fall through to the length guard. + logger.debug("compaction-marker check unavailable in recovery", exc_info=True) + + live_ctx = live.get('context_messages') + live_ctx_len = len(live_ctx) if isinstance(live_ctx, list) else 0 + bak_ctx_len = len(bak_ctx) + + # Secondary guard (unmarked compression): a pre-compression backup's context + # is larger than the live compressed context (the shrink hasn't been applied + # to it). A backup whose context is already <= the live compressed context + # post-dates the compression and represents recoverable post-compress data. + return bak_ctx_len > live_ctx_len + + def inspect_session_recovery_status(session_path: Path) -> dict: """Return a status dict describing whether recovery is recommended. @@ -86,6 +186,17 @@ def inspect_session_recovery_status(session_path: Path) -> dict: } bak_count = _msg_count(bak_path) if bak_count > live_count: + if ( + _session_records_intentional_compress_shrink(session_path) + and _backup_predates_intentional_shrink(session_path, bak_path) + ): + return { + "session_id": session_path.stem, + "live_messages": live_count, + "bak_messages": bak_count, + "recommend": "no_action", + "intentional_compress_shrink": True, + } return { "session_id": session_path.stem, "live_messages": live_count, diff --git a/tests/test_issue4836_manual_compression_recovery.py b/tests/test_issue4836_manual_compression_recovery.py new file mode 100644 index 00000000..bf994ca9 --- /dev/null +++ b/tests/test_issue4836_manual_compression_recovery.py @@ -0,0 +1,334 @@ +"""Regression tests for #4836 — manual /compress undone by reconciliation/recovery.""" +from __future__ import annotations + +import contextlib +import io +import json +import sys +import time +import types + +import api.models as models +from api.models import Session, reconciled_state_db_messages_for_session +from api.routes import _handle_session_compress, get_session +from api.session_recovery import inspect_session_recovery_status, recover_all_sessions_on_startup +from tests._pytest_port import BASE + + +class _FakeHandler: + def __init__(self): + self.wfile = io.BytesIO() + self.status = None + self.sent_headers = {} + + def send_response(self, status): + self.status = status + + def send_header(self, key, value): + self.sent_headers[key] = value + + def end_headers(self): + pass + + def payload(self): + return json.loads(self.wfile.getvalue().decode("utf-8")) + + +class _FakeCompressor: + def compress(self, messages, current_tokens=None, focus_topic=None): + if len(messages) >= 2: + return [messages[0], messages[-1]] + return list(messages) + + +class _FakeAgent: + last_instance = None + + def __init__(self, **kwargs): + self.kwargs = kwargs + self.context_compressor = _FakeCompressor() + _FakeAgent.last_instance = self + + +def _install_fake_compression_runtime(monkeypatch, agent_cls): + fake_run_agent = types.ModuleType("run_agent") + fake_run_agent.AIAgent = agent_cls + monkeypatch.setitem(sys.modules, "run_agent", fake_run_agent) + + import api.config as _cfg + + fake_runtime_provider = types.ModuleType("hermes_cli.runtime_provider") + fake_runtime_provider.resolve_runtime_provider = lambda requested=None: { + "api_key": "fake-key", + "provider": requested or "openai", + "base_url": "https://api.openai.com/v1", + } + fake_hermes_cli = types.ModuleType("hermes_cli") + fake_hermes_cli.__path__ = [] + fake_hermes_cli.runtime_provider = fake_runtime_provider + monkeypatch.setitem(sys.modules, "hermes_cli", fake_hermes_cli) + monkeypatch.setitem(sys.modules, "hermes_cli.runtime_provider", fake_runtime_provider) + import hermes_cli.runtime_provider as _rtp + + monkeypatch.setattr( + _cfg, + "resolve_model_provider", + lambda model: ("openai/gpt-5.4-mini", "openai", "https://api.openai.com/v1"), + ) + monkeypatch.setattr( + _cfg, + "_get_session_agent_lock", + lambda sid: contextlib.nullcontext(), + ) + monkeypatch.setattr( + _rtp, + "resolve_runtime_provider", + lambda requested=None: { + "api_key": "fake-key", + "provider": requested or "openai", + "base_url": "https://api.openai.com/v1", + }, + ) + + +def _msg(role, content, ts): + return {"role": role, "content": content, "timestamp": ts, "_ts": ts} + + +def test_manual_compress_persists_truncation_boundary(monkeypatch, cleanup_test_sessions, tmp_path): + session_dir = tmp_path / "sessions" + session_dir.mkdir(parents=True) + monkeypatch.setattr(models, "SESSION_DIR", session_dir) + monkeypatch.setattr(models, "SESSION_INDEX_FILE", session_dir / "_index.json") + models.SESSIONS.clear() + + original_messages = [ + _msg("user", "one", 1.0), + _msg("assistant", "two", 2.0), + _msg("user", "three", 3.0), + _msg("assistant", "four", 4.0), + ] + sid = f"issue4836_{time.time_ns()}" + cleanup_test_sessions.append(sid) + session = Session( + session_id=sid, + title="Untitled", + workspace=str(tmp_path), + model="openai/gpt-5.4-mini", + messages=original_messages, + ) + session.save(touch_updated_at=False) + + _install_fake_compression_runtime(monkeypatch, _FakeAgent) + handler = _FakeHandler() + _handle_session_compress(handler, {"session_id": sid}) + + assert handler.status == 200 + loaded = Session.load(sid) + assert loaded.compression_anchor_mode == "manual" + assert loaded.truncation_watermark is not None + assert loaded.truncation_boundary == loaded.truncation_watermark + assert loaded.last_prompt_tokens is not None + assert len(loaded.context_messages) == 2 + assert loaded.messages == original_messages + + +def test_manual_compress_blocks_state_db_replay(monkeypatch, cleanup_test_sessions, tmp_path): + session_dir = tmp_path / "sessions" + session_dir.mkdir(parents=True) + monkeypatch.setattr(models, "SESSION_DIR", session_dir) + monkeypatch.setattr(models, "SESSION_INDEX_FILE", session_dir / "_index.json") + models.SESSIONS.clear() + + sidecar = [ + _msg("user", "one", 1.0), + _msg("assistant", "two", 2.0), + _msg("user", "three", 3.0), + _msg("assistant", "four", 4.0), + ] + state_db = [ + _msg("user", "one", 1.0), + _msg("assistant", "two", 2.0), + _msg("user", "three", 3.0), + _msg("assistant", "four", 4.0), + _msg("assistant", "", 4.1), # reasoning-only row that used to replay + ] + session = Session( + session_id=f"issue4836_reconcile_{time.time_ns()}", + title="Untitled", + workspace=str(tmp_path), + model="openai/gpt-5.4-mini", + messages=sidecar, + context_messages=sidecar, + ) + session.save(touch_updated_at=False) + cleanup_test_sessions.append(session.session_id) + + _install_fake_compression_runtime(monkeypatch, _FakeAgent) + handler = _FakeHandler() + _handle_session_compress(handler, {"session_id": session.session_id}) + assert handler.status == 200 + + loaded = Session.load(session.session_id) + merged = reconciled_state_db_messages_for_session( + loaded, + prefer_context=True, + state_messages=state_db, + ) + assert len(merged) == len(loaded.context_messages) + + +def test_startup_recovery_skips_intentional_manual_compress(monkeypatch, tmp_path): + session_dir = tmp_path / "sessions" + session_dir.mkdir(parents=True) + sid = "issue4836_recovery" + live_path = session_dir / f"{sid}.json" + bak_path = session_dir / f"{sid}.json.bak" + + live = { + "session_id": sid, + "title": "Untitled", + "workspace": str(tmp_path), + "model": "openai/gpt-5.4-mini", + "messages": [ + _msg("user", "one", 1.0), + _msg("assistant", "two", 2.0), + ], + "context_messages": [ + _msg("user", "one", 1.0), + ], + "compression_anchor_summary": "Compressed: 2 -> 1 messages", + "compression_anchor_message_key": {"role": "assistant", "ts": 2.0, "text": "two", "attachments": 0}, + "compression_anchor_mode": "manual", + "truncation_watermark": 1.0, + "truncation_boundary": 1.0, + "message_count": 2, + } + bak = dict(live) + bak["messages"] = live["messages"] + [ + _msg("user", "three", 3.0), + _msg("assistant", "four", 4.0), + ] * 150 + # The .bak is the PRE-compression backup: its model-facing context_messages + # is still the large UNCOMPRESSED context (the shrink hasn't been applied to + # it). Restoring it would undo the intentional shrink → no_action. + bak["context_messages"] = bak["messages"] + live_path.write_text(json.dumps(live), encoding="utf-8") + bak_path.write_text(json.dumps(bak), encoding="utf-8") + + status = inspect_session_recovery_status(live_path) + assert status["recommend"] == "no_action" + assert status.get("intentional_compress_shrink") is True + + result = recover_all_sessions_on_startup(session_dir) + assert result["restored"] == 0 + restored = json.loads(live_path.read_text(encoding="utf-8")) + assert len(restored["messages"]) == 2 + + +def test_startup_recovery_still_fires_for_post_compress_real_loss(monkeypatch, tmp_path): + """A manually-compressed session that LATER loses its messages array must + still be recovered from a backup written AFTER the compression (#4836 + must not permanently disable #1558 crash-recovery for compressed sessions). + """ + session_dir = tmp_path / "sessions" + session_dir.mkdir(parents=True) + sid = "issue4836_post_compress_loss" + live_path = session_dir / f"{sid}.json" + bak_path = session_dir / f"{sid}.json.bak" + + # Live sidecar: manually compressed AND then truncated by a genuine loss + # (messages array shrank to 1 entry). + live = { + "session_id": sid, + "title": "Untitled", + "workspace": str(tmp_path), + "model": "openai/gpt-5.4-mini", + "messages": [_msg("user", "one", 1.0)], + "context_messages": [_msg("user", "one", 1.0)], + "compression_anchor_summary": "Compressed earlier", + "compression_anchor_message_key": {"role": "assistant", "ts": 2.0, "text": "two", "attachments": 0}, + "compression_anchor_mode": "manual", + "truncation_watermark": 1.0, + "truncation_boundary": 1.0, + "message_count": 1, + } + # Backup: a healthy post-compression snapshot (more messages, written AFTER + # the compress). Its context_messages is the SAME already-compressed context + # as live (the compression persists across saves), so it post-dates the + # compression and must be restored when the live file later loses data. + bak = dict(live) + bak["messages"] = [ + _msg("user", "one", 1.0), + _msg("assistant", "two", 2.0), + _msg("user", "three", 3.0), + _msg("assistant", "four", 4.0), + ] + bak["context_messages"] = [_msg("user", "one", 1.0)] # same compressed context as live + live_path.write_text(json.dumps(live), encoding="utf-8") + bak_path.write_text(json.dumps(bak), encoding="utf-8") + + status = inspect_session_recovery_status(live_path) + assert status["recommend"] == "restore", ( + "post-compression real loss must still be recoverable even though the " + "session was manually compressed" + ) + + result = recover_all_sessions_on_startup(session_dir) + assert result["restored"] == 1 + restored = json.loads(live_path.read_text(encoding="utf-8")) + assert len(restored["messages"]) == 4 + assert len(restored["context_messages"]) == 1 + + +def test_startup_recovery_fires_when_loss_shrinks_both_messages_and_context(monkeypatch, tmp_path): + """A post-compression loss that clobbers BOTH messages AND context_messages + below the healthy backup's context must STILL be recovered. + + Opus gate edge: a pure length heuristic (bak_ctx_len > live_ctx_len) would + misread the healthy backup's larger context as 'pre-compression' and wrongly + suppress recovery — permanent silent data loss. The compaction-marker + discriminator fixes this: the healthy post-compression backup carries the + `[context compaction…]` marker, so it is recognized as post-compression and + recovered regardless of relative context lengths. + """ + session_dir = tmp_path / "sessions" + session_dir.mkdir(parents=True) + sid = "issue4836_both_shrunk" + live_path = session_dir / f"{sid}.json" + bak_path = session_dir / f"{sid}.json.bak" + + marker = {"role": "user", "content": "[context compaction] summary of earlier turns", "timestamp": 1.0} + # Live: a genuine loss clobbered messages (130->2) AND context (50->1 marker only). + live = { + "session_id": sid, + "title": "Untitled", + "workspace": str(tmp_path), + "model": "openai/gpt-5.4-mini", + "messages": [_msg("user", "one", 1.0), _msg("assistant", "two", 2.0)], + "context_messages": [marker], + "compression_anchor_summary": "Compressed earlier", + "compression_anchor_message_key": {"role": "assistant", "ts": 2.0, "text": "two", "attachments": 0}, + "compression_anchor_mode": "manual", + "truncation_watermark": 1.0, + "truncation_boundary": 1.0, + "message_count": 2, + } + # Healthy post-compression backup: MORE messages AND a LARGER compressed + # context (still carries the compaction marker) — must be recovered. + bak = dict(live) + bak["messages"] = [_msg("user", f"m{i}", float(i)) for i in range(130)] + bak["context_messages"] = [marker] + [_msg("assistant", f"c{i}", float(i)) for i in range(49)] + live_path.write_text(json.dumps(live), encoding="utf-8") + bak_path.write_text(json.dumps(bak), encoding="utf-8") + + status = inspect_session_recovery_status(live_path) + assert status["recommend"] == "restore", ( + "a loss shrinking both messages and context must still recover from a " + "marked (post-compression) backup — the marker, not length, decides" + ) + + result = recover_all_sessions_on_startup(session_dir) + assert result["restored"] == 1 + restored = json.loads(live_path.read_text(encoding="utf-8")) + assert len(restored["messages"]) == 130 diff --git a/tests/test_sprint46.py b/tests/test_sprint46.py index 7fd1c25e..272f8668 100644 --- a/tests/test_sprint46.py +++ b/tests/test_sprint46.py @@ -172,14 +172,24 @@ def test_session_compress_roundtrip(monkeypatch, cleanup_test_sessions): loaded = get_session(sid) persisted = Session.load(sid) assert loaded.messages == original_messages - assert loaded.context_messages == compressed_messages + assert [m.get("role") for m in loaded.context_messages] == [m.get("role") for m in compressed_messages] + assert [m.get("content") for m in loaded.context_messages] == [m.get("content") for m in compressed_messages] assert loaded.tool_calls == settled_tool_calls assert persisted.messages == original_messages - assert persisted.context_messages == compressed_messages + assert [m.get("role") for m in persisted.context_messages] == [m.get("role") for m in compressed_messages] + assert [m.get("content") for m in persisted.context_messages] == [m.get("content") for m in compressed_messages] assert persisted.tool_calls == settled_tool_calls assert loaded.compression_anchor_summary == payload["session"]["compression_anchor_summary"] assert loaded.compression_anchor_visible_idx == payload["session"]["compression_anchor_visible_idx"] assert loaded.compression_anchor_message_key == payload["session"]["compression_anchor_message_key"] + assert loaded.compression_anchor_mode == "manual" + assert loaded.truncation_watermark is not None + assert loaded.truncation_boundary == loaded.truncation_watermark + assert loaded.last_prompt_tokens is not None + assert persisted.compression_anchor_mode == "manual" + assert persisted.truncation_watermark == loaded.truncation_watermark + assert persisted.truncation_boundary == loaded.truncation_boundary + assert not (SESSION_DIR / f"{sid}.json.bak").exists() assert persisted.compression_anchor_visible_idx == 3 assert persisted.compression_anchor_message_key == payload["session"]["compression_anchor_message_key"] assert _FakeAgent.last_instance is not None @@ -269,10 +279,10 @@ def test_session_compress_start_is_async_and_reuses_running_job(monkeypatch, cle assert done_payload["session"]["tool_calls"] == settled_tool_calls persisted = Session.load(sid) assert persisted.tool_calls == settled_tool_calls - assert persisted.context_messages == [ - {"role": "user", "content": "one"}, - {"role": "assistant", "content": "four"}, - ] + # /compress now stamps missing message timestamps, so compare role/content + # rather than full-dict equality (mirrors test_session_compress_roundtrip). + assert [m.get("role") for m in persisted.context_messages] == ["user", "assistant"] + assert [m.get("content") for m in persisted.context_messages] == ["one", "four"] def test_session_compress_status_reports_worker_error_without_raw_paths(monkeypatch, cleanup_test_sessions):