fix(recovery): preserve worktree metadata + workspace + message_count on state.db sidecar rebuild
PR #2053 added worktree-backed session creation. PR #2041 (shipped in v0.51.42) added state.db sidecar reconciliation that rebuilds a missing <sid>.json sidecar from the canonical state.db row when the JSON file is gone (failed save, manual rm, restore-from-backup with mismatched dirs). The two interact silently. `_state_db_row_to_sidecar()` was hard-coding `'workspace': ''` and never propagating the four worktree_* fields from the row to the rebuilt sidecar dict. So a worktree-backed session that loses its sidecar and gets rebuilt from state.db: - loses `worktree_path` → matches the empty-session sidebar filter at `api/models.py:1067/1107` (which spares worktree-backed empty sessions via `not s.get('worktree_path')`) → session disappears from the sidebar even though the worktree directory still exists on disk. - loses `workspace` → downstream tools (terminal panels, file pickers that use `s.workspace`) operate on empty string instead of the original worktree path. - always reports `message_count == 0` → contributes to the empty-session filter even for sessions that have messages in `state.db.messages`. Fix: 1. `_read_state_db_missing_sidecar_rows()` SELECT now includes `workspace, worktree_path, worktree_branch, worktree_repo_root, worktree_created_at, message_count` (each gated by `_sql_optional_col()` so older state.db schemas without those columns continue to work — recovery degrades gracefully rather than 500ing). 2. `_state_db_row_to_sidecar()` propagates each field. workspace comes from the row if it's a string, otherwise '' (matching pre-fix behavior for non-worktree sessions). message_count comes from the row if it's an int, otherwise falls back to `len(messages)` so the rebuilt sidecar always has a coherent count. 3 new regression tests in tests/test_state_db_worktree_recovery.py exercise: - worktree session with messages → all four worktree_* fields preserved. - non-worktree session → worktree_* fields all None (no spurious propagation), workspace=''. - empty worktree session (the worst case) → confirms the rebuilt sidecar does NOT match the empty-session-exempt filter, so it stays visible in the sidebar. Caught by Opus advisor during stage-337 review (the cross-PR interaction between #2053 and the previously-shipped #2041 wasn't exercised by either PR's individual test suite).
This commit is contained in:
nesquena-hermes
@@ -193,11 +193,18 @@ def _read_state_db_missing_sidecar_rows(session_dir: Path, state_db_path: Path |
|
||||
started_expr = _sql_optional_col('started_at', session_cols, '0')
|
||||
parent_expr = _sql_optional_col('parent_session_id', session_cols)
|
||||
msg_count_expr = _sql_optional_col('message_count', session_cols, '0')
|
||||
workspace_expr = _sql_optional_col('workspace', session_cols)
|
||||
worktree_path_expr = _sql_optional_col('worktree_path', session_cols)
|
||||
worktree_branch_expr = _sql_optional_col('worktree_branch', session_cols)
|
||||
worktree_repo_root_expr = _sql_optional_col('worktree_repo_root', session_cols)
|
||||
worktree_created_at_expr = _sql_optional_col('worktree_created_at', session_cols)
|
||||
rows = []
|
||||
for row in conn.execute(
|
||||
f"""
|
||||
SELECT id, source, {title_expr}, {model_expr}, {started_expr},
|
||||
{parent_expr}, {msg_count_expr}
|
||||
{parent_expr}, {msg_count_expr}, {workspace_expr},
|
||||
{worktree_path_expr}, {worktree_branch_expr},
|
||||
{worktree_repo_root_expr}, {worktree_created_at_expr}
|
||||
FROM sessions
|
||||
WHERE source = 'webui'
|
||||
ORDER BY COALESCE(started_at, 0) DESC
|
||||
@@ -250,10 +257,16 @@ def _state_db_row_to_sidecar(row: dict) -> dict:
|
||||
started_at = row.get('started_at') or 0
|
||||
messages = row.get('messages') if isinstance(row.get('messages'), list) else []
|
||||
last_ts = messages[-1].get('timestamp') if messages and isinstance(messages[-1], dict) else started_at
|
||||
workspace_value = row.get('workspace') or ''
|
||||
return {
|
||||
'session_id': row.get('id'),
|
||||
'title': row.get('title') or 'Recovered WebUI Session',
|
||||
'workspace': '',
|
||||
'workspace': workspace_value if isinstance(workspace_value, str) else '',
|
||||
'message_count': row.get('message_count') if isinstance(row.get('message_count'), int) else len(messages),
|
||||
'worktree_path': row.get('worktree_path') or None,
|
||||
'worktree_branch': row.get('worktree_branch') or None,
|
||||
'worktree_repo_root': row.get('worktree_repo_root') or None,
|
||||
'worktree_created_at': row.get('worktree_created_at') or None,
|
||||
'model': row.get('model') or 'unknown',
|
||||
'model_provider': None,
|
||||
'created_at': started_at,
|
||||
|
||||
128
tests/test_state_db_worktree_recovery.py
Normal file
128
tests/test_state_db_worktree_recovery.py
Normal file
@@ -0,0 +1,128 @@
|
||||
"""Regression for state.db × worktree-backed session recovery.
|
||||
|
||||
PR #2053 added worktree-backed session creation. PR #2041 added state.db
|
||||
sidecar reconciliation. When a worktree-backed session's JSON sidecar is
|
||||
lost (failed save, manual rm, restore-from-backup) and state.db is the only
|
||||
source of truth, the recovery path must rebuild a sidecar that preserves
|
||||
the worktree_* fields. Without that, the sidebar exempt-empty filter at
|
||||
api/models.py:1067/1107 (which spares worktree-backed empty sessions) sees
|
||||
no worktree_path on the rebuilt session and silently filters it out — the
|
||||
session vanishes from the sidebar even though the worktree directory still
|
||||
exists on disk.
|
||||
|
||||
Caught by Opus advisor on stage-337 review.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from api.session_recovery import _state_db_row_to_sidecar
|
||||
|
||||
|
||||
def test_state_db_recovery_preserves_worktree_metadata():
|
||||
"""Recovered sidecar must keep worktree_path / worktree_branch / repo_root."""
|
||||
row = {
|
||||
"id": "abc123",
|
||||
"source": "webui",
|
||||
"title": "My worktree session",
|
||||
"model": "anthropic/claude-3-opus",
|
||||
"started_at": 1700000000,
|
||||
"parent_session_id": None,
|
||||
"message_count": 3,
|
||||
"messages": [
|
||||
{"role": "user", "content": "hello", "timestamp": 1700000001},
|
||||
{"role": "assistant", "content": "hi", "timestamp": 1700000002},
|
||||
{"role": "user", "content": "more", "timestamp": 1700000003},
|
||||
],
|
||||
"workspace": "/home/user/proj/.worktrees/hermes-1234",
|
||||
"worktree_path": "/home/user/proj/.worktrees/hermes-1234",
|
||||
"worktree_branch": "hermes/abc123",
|
||||
"worktree_repo_root": "/home/user/proj",
|
||||
"worktree_created_at": 1700000000,
|
||||
}
|
||||
|
||||
sidecar = _state_db_row_to_sidecar(row)
|
||||
|
||||
assert sidecar["session_id"] == "abc123"
|
||||
assert sidecar["title"] == "My worktree session"
|
||||
# The four worktree_* fields must survive the rebuild — without them the
|
||||
# sidebar filter at api/models.py:1067 hides the session.
|
||||
assert sidecar["worktree_path"] == "/home/user/proj/.worktrees/hermes-1234"
|
||||
assert sidecar["worktree_branch"] == "hermes/abc123"
|
||||
assert sidecar["worktree_repo_root"] == "/home/user/proj"
|
||||
assert sidecar["worktree_created_at"] == 1700000000
|
||||
# Workspace must round-trip from the row so terminal panels / file pickers
|
||||
# operate on the correct path, not on empty string.
|
||||
assert sidecar["workspace"] == "/home/user/proj/.worktrees/hermes-1234"
|
||||
# message_count must come from the row so the sidebar exempt-empty filter
|
||||
# accepts message-bearing sessions (was hard-coded 0 pre-fix).
|
||||
assert sidecar["message_count"] == 3
|
||||
|
||||
|
||||
def test_state_db_recovery_non_worktree_session_unaffected():
|
||||
"""A normal (non-worktree) session recovers exactly as before — None worktree fields."""
|
||||
row = {
|
||||
"id": "xyz789",
|
||||
"source": "webui",
|
||||
"title": "Normal chat",
|
||||
"model": "openai/gpt-4",
|
||||
"started_at": 1700000000,
|
||||
"parent_session_id": None,
|
||||
"message_count": 1,
|
||||
"messages": [{"role": "user", "content": "hello"}],
|
||||
# No workspace, no worktree_* fields on the row.
|
||||
}
|
||||
|
||||
sidecar = _state_db_row_to_sidecar(row)
|
||||
|
||||
assert sidecar["worktree_path"] is None
|
||||
assert sidecar["worktree_branch"] is None
|
||||
assert sidecar["worktree_repo_root"] is None
|
||||
assert sidecar["worktree_created_at"] is None
|
||||
assert sidecar["workspace"] == ""
|
||||
assert sidecar["message_count"] == 1
|
||||
|
||||
|
||||
def test_state_db_recovery_zero_message_worktree_session_visible_in_sidebar():
|
||||
"""An empty worktree-backed session recovered from state.db must NOT be
|
||||
silently filtered from the sidebar by the empty-session-exempt rule.
|
||||
|
||||
Pre-fix: the recovery rebuilt a sidecar with no worktree_path → matched the
|
||||
empty-session filter → session disappeared from the sidebar even though
|
||||
the worktree directory still existed on disk. Now that worktree_path is
|
||||
propagated, the exemption clause at api/models.py:1070 fires.
|
||||
"""
|
||||
row = {
|
||||
"id": "empty-worktree-abc",
|
||||
"source": "webui",
|
||||
"title": "Untitled", # default before any user message
|
||||
"model": "anthropic/claude-3-opus",
|
||||
"started_at": 1700000000,
|
||||
"parent_session_id": None,
|
||||
"message_count": 0,
|
||||
"messages": [],
|
||||
"workspace": "/home/user/proj/.worktrees/hermes-empty",
|
||||
"worktree_path": "/home/user/proj/.worktrees/hermes-empty",
|
||||
"worktree_branch": "hermes/empty",
|
||||
"worktree_repo_root": "/home/user/proj",
|
||||
"worktree_created_at": 1700000000,
|
||||
}
|
||||
|
||||
sidecar = _state_db_row_to_sidecar(row)
|
||||
|
||||
# The compact() shape used in sidebar filtering is roughly the sidecar dict
|
||||
# with selected keys. The filter at api/models.py:1067 checks:
|
||||
# title == 'Untitled' and message_count == 0 and not active_stream_id
|
||||
# and not has_pending_user_message and not worktree_path
|
||||
# Pre-fix all 5 clauses matched → exempted FROM the result (i.e., hidden).
|
||||
# Post-fix the worktree_path clause is truthy, so the session SHOULD render.
|
||||
is_hidden_by_empty_filter = (
|
||||
sidecar.get("title", "Untitled") == "Untitled"
|
||||
and sidecar.get("message_count", 0) == 0
|
||||
and not sidecar.get("active_stream_id")
|
||||
and not sidecar.get("pending_user_message")
|
||||
and not sidecar.get("worktree_path")
|
||||
)
|
||||
assert not is_hidden_by_empty_filter, (
|
||||
"Worktree session was hidden by the empty-session exempt filter; "
|
||||
"worktree_path must be propagated through state.db recovery so the "
|
||||
"exempt clause in api/models.py:1070 does NOT match for this session."
|
||||
)
|
||||
Reference in New Issue
Block a user