Fix Archive Session for metadata-only cache hits

[franksong2702]

Thinking Path

Archive Session fails only after the target session has already been touched by a metadata-only path. The archive endpoint loads the session with get_session(sid), then mutates archived and calls save(). When the in-memory LRU already contains a _loaded_metadata_only=True instance, that cached stub is returned before a full disk load happens. Session.save() correctly refuses metadata-only instances under the #1558 guard because their messages=[] payload would risk overwriting the full transcript. So the bug is not the guard; it is that /api/session/archive mutates a cached metadata-only stub instead of upgrading it first.

What Changed

Fixes #2243.

Changed 3 files: 59 insertions, 1 deletion.

• api/routes.py: reloads a cached metadata-only session with Session.load(sid) before mutating archived, then refreshes the session cache with the full instance. The existing CLI/imported-session fallback stays unchanged.
• tests/test_metadata_save_wipe_1558.py: adds a regression test that places a metadata-only stub in SESSIONS, calls /api/session/archive, and asserts archive succeeds while the on-disk message history remains intact.
• CHANGELOG.md: documents the user-visible Archive Session fix.

Why It Matters

Archiving is a common sidebar action. The pre-fix behavior looked like a dead click or Archive failed, even though the session existed and the requested mutation was valid. This restores the expected Archive/Restore behavior without weakening the data-loss guard added for #1558.

Verification

Local regression shape:

1. Create a real session JSON with messages on disk.
2. Load a metadata-only session stub.
3. Put that stub into the session cache to reproduce the bad cache hit.
4. Call POST /api/session/archive through the route handler.
5. Verify the response is successful, archived=True persists, the cached object is now a full session, and all messages remain on disk.

Commands run:

/Users/xuefusong/hermes-webui/.venv_test/bin/python -m pytest -q tests/test_metadata_save_wipe_1558.py tests/test_issue2057_worktree_lifecycle.py::test_archive_worktree_session_reports_retained_worktree_without_cleanup
/Users/xuefusong/hermes-webui/.venv_test/bin/python -m py_compile api/routes.py api/models.py
git diff --check

Result: 13 passed plus py_compile and git diff --check passed.

Risks / Follow-ups

Low risk. This does not weaken the #1558 metadata-only save guard. Session.save() still refuses metadata-only instances; this route simply reloads the full persisted session before performing a valid archive mutation. If the full reload fails, the endpoint falls through to the existing not-found/CLI fallback behavior instead of saving the stub.

This PR intentionally does not change broader metadata-only cache semantics or session pin/move behavior.

Model Used

AI-assisted with OpenAI Codex, GPT-5. Human-directed scope and verification. No external code generation tools beyond local shell, tests, and GitHub CLI.

[nesquena-hermes]

Read this end-to-end on the PR worktree against origin/master — the diff is small (3 files, +59/-1) and the fix lands at the right layer.

What the change does

/api/session/archive previously called get_session(sid) which, in api/models.py:885-888, returns whatever instance is already pinned in the in-memory LRU regardless of how it was originally loaded:

with LOCK:
    if sid in SESSIONS:
        SESSIONS.move_to_end(sid)  # LRU: mark as recently used
        return SESSIONS[sid]

load_metadata_only() does not pin into SESSIONS (see get_session() at api/models.py:889-892 — metadata-only loads return the stub without populating the LRU), but the cache can be hot from a previous metadata_only=False load, and a stub can end up there indirectly through other code paths that hand stubs back to the cache (api/streaming.py:2910 builds one; api/routes.py:179, 975, 3226, 3278, 3517, 4461 all hand stubs to other helpers). The #1558 save guard at api/models.py:410-414 then rejects the eventual s.save() because _loaded_metadata_only=True, leaving the UI with Archive failed.

This PR's fix at api/routes.py:5114-5123:

if getattr(s, "_loaded_metadata_only", False):
    s = Session.load(sid)
    if s is None:
        raise KeyError(sid)
    with LOCK:
        SESSIONS[sid] = s

is exactly the same shape as the existing _clear_stale_stream_state() upgrade at api/routes.py:889-892:

if getattr(session, "_loaded_metadata_only", False):
    try:
        from api.models import get_session as _get_session
        session = _get_session(session.session_id, metadata_only=False)

so the pattern is already blessed for this kind of mutation route.

One small inconsistency worth noting

_clear_stale_stream_state() uses get_session(sid, metadata_only=False) to upgrade, which goes through get_session()'s LRU eviction-cap logic (api/models.py:894-900). The PR uses Session.load(sid) + a direct SESSIONS[sid] = s assignment, bypassing move_to_end and the SESSIONS_MAX eviction loop. Functionally fine — move_to_end is best-effort and SESSIONS_MAX will get enforced on the next eviction-aware path — but if you wanted to keep the two upgrade sites identical (which matters for #1558 follow-ups), reusing get_session(sid, metadata_only=False) here would be a one-line change. Not a blocker; just a consistency note.

Test coverage

tests/test_metadata_save_wipe_1558.py::test_archive_route_reloads_metadata_only_cached_session reproduces the precise bad state (stub forced into SESSIONS, route invoked through handle_post) and asserts both that the call returns 200 and that the cache entry is upgraded (_loaded_metadata_only is False, len(cached.messages) == 12). The disk-side assertion via Session.load(sid) confirms the persisted JSON is intact post-archive. Good shape — this would have caught the bug if it had existed when the test was written.

Other archive-route mutations that have the same shape

The same pattern exists at:

• /api/session/pin (api/routes.py:5091-5103) — s.pinned = ...; s.save()
• /api/session/rename (api/routes.py:4282-4294) — s.title = ...; s.save()
• /api/personality/set (api/routes.py:4313-4337) — s.personality = ...; s.save()

All three call get_session(sid) then mutate + save() without the metadata-only upgrade. They are likely just as exposed as archive was. Worth filing a follow-up issue (or expanding this PR's scope, if you'd rather land it in one go) so we don't get a Pin failed / Rename failed report next.

Verdict

LGTM as a focused fix for #2243. Minor consistency suggestion above is optional. The regression test pins the contract; the #1558 save-guard stays intact; CHANGELOG is correctly placed under Unreleased → Fixed.

— maintainer

[franksong2702]

Opened follow-up #2249 for the three adjacent metadata-only mutation routes you called out here: /api/session/pin, /api/session/rename, and /api/personality/set.

I kept this PR focused on Archive Session and tracked the adjacent-route work separately in #2248 so the review boundaries stay clear.