fix: harden bounded sidebar cache projection (#7489)

Preserve nested request isolation, make the response allowlist single-source,
and strip heavy fields from existing index rows after reconciliation.

Co-authored-by: Martin Dell <martindell@users.noreply.github.com>
This commit is contained in:
nandMartin Dell committed 2026-09-09 23:36:41 +00:00
1 parent 989d7f3668
commit 3e616ad455
6 files changed
+305 -117

No files matched your search

+26 -7
View File
@@ -1191,6 +1191,25 @@ def model_explicit_pick_signature(model, model_provider) -> str:
return f"{_m}\x1f{_p}" return f"{_m}\x1f{_p}"
_SIDEBAR_HEAVY_METADATA_FIELDS = (
'compression_anchor_summary',
'compression_anchor_details',
'context_engine_state',
'compression_recovery',
'gateway_routing_history',
'composer_draft',
'process_wakeup_pause',
'share_token',
)
def _strip_sidebar_heavy_metadata(row: dict) -> dict:
"""Remove detail-only values after sidebar reconciliation is complete."""
for key in _SIDEBAR_HEAVY_METADATA_FIELDS:
row.pop(key, None)
return row
class Session: class Session:
def __init__(self, session_id: str=None, title: str='Untitled', def __init__(self, session_id: str=None, title: str='Untitled',
workspace=str(DEFAULT_WORKSPACE), created_workspace=None, workspace=str(DEFAULT_WORKSPACE), created_workspace=None,
@@ -1815,13 +1834,7 @@ class Session:
) if include_runtime else False, ) if include_runtime else False,
} }
if sidebar_metadata_only: if sidebar_metadata_only:
for key in ( _strip_sidebar_heavy_metadata(compact)
'compression_anchor_summary', 'compression_anchor_details',
'context_engine_state', 'compression_recovery',
'gateway_routing_history', 'composer_draft',
'process_wakeup_pause', 'share_token',
):
compact.pop(key, None)
return compact return compact
@@ -6450,6 +6463,9 @@ def all_sessions(
for s in result: for s in result:
if not s.get('profile'): if not s.get('profile'):
s['profile'] = 'default' s['profile'] = 'default'
if sidebar_metadata_only:
for s in result:
_strip_sidebar_heavy_metadata(s)
return result return result
except Exception: except Exception:
logger.debug("Failed to load session index, falling back to full scan") logger.debug("Failed to load session index, falling back to full scan")
@@ -6507,6 +6523,9 @@ def all_sessions(
for s in result: for s in result:
if not s.get('profile'): if not s.get('profile'):
s['profile'] = 'default' s['profile'] = 'default'
if sidebar_metadata_only:
for s in result:
_strip_sidebar_heavy_metadata(s)
return result return result
+118 -38
View File
@@ -1,5 +1,6 @@
"""Session-list cache helpers extracted from api.routes.""" """Session-list cache helpers extracted from api.routes."""
import copy
import os import os
import re import re
import threading import threading
@@ -43,29 +44,86 @@ _SESSIONS_CACHE_ALL_PROFILES_INVALIDATION_VERSION = 0
_SESSIONS_CACHE_PROFILE_INVALIDATION_VERSION: dict[str, int] = {} _SESSIONS_CACHE_PROFILE_INVALIDATION_VERSION: dict[str, int] = {}
_SIDEBAR_SESSION_RESPONSE_FIELDS = {
"session_id",
"title",
"display_title",
"_state_db_title",
"workspace",
"model",
"model_provider",
"message_count",
"user_message_count",
"created_at",
"updated_at",
"last_message_at",
"pinned",
"archived",
"project_id",
"profile",
"input_tokens",
"output_tokens",
"estimated_cost",
"cache_read_tokens",
"cache_write_tokens",
"cache_hit_percent",
"personality",
"context_length",
"config_context_length",
"window_usage_percent",
"source_tag",
"raw_source",
"session_source",
"source_label",
"is_cli_session",
"is_messaging_session",
"is_streaming",
"cron_running",
"active_stream_id",
"has_pending_user_message",
"pending_started_at",
"default_hidden",
"worktree_path",
"worktree_branch",
"parent_session_id",
"parent_title",
"parent_source",
"relationship_type",
"pre_compression_snapshot",
"_lineage_root_id",
"_lineage_tip_id",
"_compression_segment_count",
"_lineage_collapsed_count",
"_parent_lineage_root_id",
"_parent_lineage_tip_id",
"_cross_surface_child_session",
"match_type",
"match_preview",
# Preserved so the sidebar can suppress rename / action-menu / swipe on
# read-only sessions and render the detailed gateway model label. Only the
# latest bounded routing object is included; routing history stays excluded.
"read_only",
"is_read_only",
"gateway_routing",
}
def _session_list_cache_sidebar_fields() -> set[str]: def _session_list_cache_sidebar_fields() -> set[str]:
"""Return the canonical bounded field set used by the list response.""" """Return the canonical bounded field set used by the list response."""
try: return _SIDEBAR_SESSION_RESPONSE_FIELDS
import api.routes as _routes
fields = getattr(_routes, "_SIDEBAR_SESSION_RESPONSE_FIELDS", None)
if isinstance(fields, set): def _session_list_cache_copy_value(value):
return fields """Copy only mutable values after the transcript-bearing projection."""
except Exception: if isinstance(value, (dict, list, set)):
pass return copy.deepcopy(value)
# The fallback is intentionally small and contains no transcript-bearing return value
# fields. Normal runtime calls resolve the canonical set above after
# api.routes has finished importing.
def _session_list_cache_copy_row(row: dict) -> dict:
return { return {
"session_id", "title", "workspace", "model", "model_provider", key: _session_list_cache_copy_value(value)
"message_count", "user_message_count", "created_at", "updated_at", for key, value in row.items()
"last_message_at", "pinned", "archived", "project_id", "profile",
"source_tag", "session_source", "is_streaming", "active_stream_id",
"has_pending_user_message", "pending_started_at", "default_hidden",
"worktree_path", "worktree_branch", "parent_session_id",
"parent_title", "parent_source", "relationship_type",
"pre_compression_snapshot", "read_only", "is_read_only",
"gateway_routing",
} }
@@ -86,34 +144,40 @@ def _session_list_cache_bounded_payload(payload: dict) -> dict:
projected.append({}) projected.append({})
continue continue
item = {key: row[key] for key in fields if key in row} item = {key: row[key] for key in fields if key in row}
if isinstance(item.get("gateway_routing"), dict): projected.append(_session_list_cache_copy_row(item))
item["gateway_routing"] = dict(item["gateway_routing"])
projected.append(item)
return projected return projected
bounded = { bounded = {
key: value key: _session_list_cache_copy_value(value)
for key, value in payload.items() for key, value in payload.items()
if key not in {"sessions", "sidebar_reference_sessions"} if key not in {"sessions", "sidebar_reference_sessions"}
} }
bounded["sessions"] = project_rows(payload.get("sessions")) if "sessions" in payload:
bounded["sidebar_reference_sessions"] = project_rows( bounded["sessions"] = project_rows(payload.get("sessions"))
payload.get("sidebar_reference_sessions") if "sidebar_reference_sessions" in payload:
) bounded["sidebar_reference_sessions"] = project_rows(
payload.get("sidebar_reference_sessions")
)
return bounded return bounded
def _session_list_cache_copy_payload(payload: dict) -> dict: def _session_list_cache_copy_payload(payload: dict) -> dict:
"""Copy only the already-bounded cache shape for request-local mutation.""" """Copy only the already-bounded cache shape for request-local mutation."""
copied = dict(payload) copied = {
copied["sessions"] = [dict(row) for row in payload.get("sessions", [])] key: _session_list_cache_copy_value(value)
copied["sidebar_reference_sessions"] = [ for key, value in payload.items()
dict(row) for row in payload.get("sidebar_reference_sessions", []) if key not in {"sessions", "sidebar_reference_sessions"}
] }
for rows in (copied["sessions"], copied["sidebar_reference_sessions"]): if "sessions" in payload:
for row in rows: copied["sessions"] = [
if isinstance(row.get("gateway_routing"), dict): _session_list_cache_copy_row(row)
row["gateway_routing"] = dict(row["gateway_routing"]) for row in payload.get("sessions", [])
]
if "sidebar_reference_sessions" in payload:
copied["sidebar_reference_sessions"] = [
_session_list_cache_copy_row(row)
for row in payload.get("sidebar_reference_sessions", [])
]
return copied return copied
@@ -351,16 +415,32 @@ def _session_list_cache_stale_reason(key: tuple) -> str | None:
return None return None
def _session_list_cache_set(key: tuple, payload: dict) -> None: def _session_list_cache_set(
key: tuple,
payload: dict,
*,
expected_invalidation_stamp: tuple[int, int] | None = None,
) -> bool:
if not isinstance(payload, dict): if not isinstance(payload, dict):
return return False
stamp = _session_list_cache_resolved_source_stamp(key) stamp = _session_list_cache_resolved_source_stamp(key)
bounded = _session_list_cache_bounded_payload(payload) bounded = _session_list_cache_bounded_payload(payload)
with _SESSIONS_CACHE_LOCK: with _SESSIONS_CACHE_LOCK:
# Projection intentionally happens outside the lock so large source rows
# cannot block cache hits. Re-check the caller's pre-build generation
# atomically before insertion, otherwise a rename/archive/delete clear
# that lands during projection can be undone by this stale write.
if (
expected_invalidation_stamp is not None
and _session_list_cache_invalidation_stamp(key)
!= expected_invalidation_stamp
):
return False
_SESSIONS_CACHE[key] = (time.monotonic(), stamp, bounded) _SESSIONS_CACHE[key] = (time.monotonic(), stamp, bounded)
_SESSIONS_CACHE.move_to_end(key) _SESSIONS_CACHE.move_to_end(key)
while len(_SESSIONS_CACHE) > _SESSIONS_CACHE_MAX_ENTRIES: while len(_SESSIONS_CACHE) > _SESSIONS_CACHE_MAX_ENTRIES:
_SESSIONS_CACHE.popitem(last=False) _SESSIONS_CACHE.popitem(last=False)
return True
def _session_list_cache_clear(profile: str | None = None) -> None: def _session_list_cache_clear(profile: str | None = None) -> None:
+27 -70
View File
@@ -2771,8 +2771,14 @@ def _get_cached_session_list_payload(
"session list stale-cache background rebuild failed" "session list stale-cache background rebuild failed"
) )
return return
if _session_list_cache_invalidation_stamp(key) == invalidation_stamp: if (
_session_list_cache_set(key, payload) _session_list_cache_invalidation_stamp(key) == invalidation_stamp
and _session_list_cache_set(
key,
payload,
expected_invalidation_stamp=invalidation_stamp,
)
):
return return
rebuild_attempts += 1 rebuild_attempts += 1
if rebuild_attempts >= 3: if rebuild_attempts >= 3:
@@ -2808,8 +2814,14 @@ def _get_cached_session_list_payload(
while True: while True:
invalidation_stamp = _session_list_cache_invalidation_stamp(key) invalidation_stamp = _session_list_cache_invalidation_stamp(key)
payload = builder() payload = builder()
if _session_list_cache_invalidation_stamp(key) == invalidation_stamp: if (
_session_list_cache_set(key, payload) _session_list_cache_invalidation_stamp(key) == invalidation_stamp
and _session_list_cache_set(
key,
payload,
expected_invalidation_stamp=invalidation_stamp,
)
):
if diag is not None: if diag is not None:
try: try:
diag.stage("session_list_cache_stored") diag.stage("session_list_cache_stored")
@@ -2868,7 +2880,11 @@ def _get_cached_session_list_payload(
invalidation_stamp = _session_list_cache_invalidation_stamp(key) invalidation_stamp = _session_list_cache_invalidation_stamp(key)
payload = builder() payload = builder()
if _session_list_cache_invalidation_stamp(key) == invalidation_stamp: if _session_list_cache_invalidation_stamp(key) == invalidation_stamp:
_session_list_cache_set(key, payload) _session_list_cache_set(
key,
payload,
expected_invalidation_stamp=invalidation_stamp,
)
return payload return payload
from api.config import ( from api.config import (
@@ -10680,71 +10696,12 @@ def _session_attention_summary(session_id: str) -> dict | None:
return None return None
_SIDEBAR_SESSION_RESPONSE_FIELDS = { # One canonical allowlist owns both cache projection and final serialization.
"session_id", # Keeping it in the cache module avoids a circular-import fallback that could
"title", # silently truncate otherwise valid sidebar fields.
"display_title", _SIDEBAR_SESSION_RESPONSE_FIELDS = (
"_state_db_title", _route_session_list_cache._SIDEBAR_SESSION_RESPONSE_FIELDS
"workspace", )
"model",
"model_provider",
"message_count",
"user_message_count",
"created_at",
"updated_at",
"last_message_at",
"pinned",
"archived",
"project_id",
"profile",
"input_tokens",
"output_tokens",
"estimated_cost",
"cache_read_tokens",
"cache_write_tokens",
"cache_hit_percent",
"personality",
"context_length",
"config_context_length",
"window_usage_percent",
"source_tag",
"raw_source",
"session_source",
"source_label",
"is_cli_session",
"is_messaging_session",
"is_streaming",
"cron_running",
"active_stream_id",
"has_pending_user_message",
"pending_started_at",
"default_hidden",
"worktree_path",
"worktree_branch",
"parent_session_id",
"parent_title",
"parent_source",
"relationship_type",
"pre_compression_snapshot",
"_lineage_root_id",
"_lineage_tip_id",
"_compression_segment_count",
"_lineage_collapsed_count",
"_parent_lineage_root_id",
"_parent_lineage_tip_id",
"_cross_surface_child_session",
"match_type",
"match_preview",
# Preserved so the sidebar can suppress rename / action-menu / swipe on
# read-only (imported CLI + Claude Code) sessions, and render the detailed
# gateway model label. Dropping these silently regressed both surfaces.
# Only the latest `gateway_routing` is included (the sidebar label reader
# prefers it); the unbounded `gateway_routing_history` is intentionally NOT
# sent in the list payload to avoid per-row bloat.
"read_only",
"is_read_only",
"gateway_routing",
}
def _sidebar_session_response_item(session: dict, *, redact_enabled: bool | None = None) -> dict: def _sidebar_session_response_item(session: dict, *, redact_enabled: bool | None = None) -> dict:
+3 -1
View File
@@ -606,7 +606,9 @@ def test_session_compact_includes_parent():
# the top of compact() which pushed the parent_session_id field beyond a # the top of compact() which pushed the parent_session_id field beyond a
# 1500-char window — widen the scan to 3000 chars to cover the full # 1500-char window — widen the scan to 3000 chars to cover the full
# return-dict body without re-tightening every time compact() grows. # return-dict body without re-tightening every time compact() grows.
compact_def_match = re.search(r"def compact\(self", src) # compact() accepts optional projection flags on separate lines; match the
# method name and first parameter without pinning formatting.
compact_def_match = re.search(r"def compact\(\s*self", src)
assert compact_def_match, "Could not find compact() method" assert compact_def_match, "Could not find compact() method"
snippet = src[compact_def_match.start():compact_def_match.start() + 3000] snippet = src[compact_def_match.start():compact_def_match.start() + 3000]
assert "'parent_session_id'" in snippet, \ assert "'parent_session_id'" in snippet, \
+3 -1
View File
@@ -95,7 +95,9 @@ def test_issue1855_target_routes_are_wired_to_diagnostics():
src = Path("api/routes.py").read_text(encoding="utf-8") src = Path("api/routes.py").read_text(encoding="utf-8")
assert 'RequestDiagnostics.maybe_start("GET", parsed.path' in src assert 'RequestDiagnostics.maybe_start("GET", parsed.path' in src
assert "all_sessions(diag=diag, include_lineage_metadata=False)" in src assert 'kwargs = {"diag": diag, "include_lineage_metadata": False}' in src
assert 'kwargs["sidebar_metadata_only"] = True' in src
assert "return all_sessions(**kwargs)" in src
assert 'RequestDiagnostics.maybe_start("POST", parsed.path' in src assert 'RequestDiagnostics.maybe_start("POST", parsed.path' in src
assert "_handle_chat_start(handler, body, diag=diag)" in src assert "_handle_chat_start(handler, body, diag=diag)" in src
for stage in ( for stage in (
+128
View File
@@ -1,6 +1,11 @@
"""Regression tests for bounded sidebar session-list caching.""" """Regression tests for bounded sidebar session-list caching."""
import json
import threading
from api import route_session_list_cache as cache from api import route_session_list_cache as cache
from api import models
from api import routes
from api.models import Session from api.models import Session
@@ -59,6 +64,89 @@ def test_cache_read_returns_independent_bounded_rows(monkeypatch):
assert second["sessions"][0]["title"] == "One" assert second["sessions"][0]["title"] == "One"
def test_cache_read_isolates_nested_rows_and_settings(monkeypatch):
cache._session_list_cache_clear()
monkeypatch.setattr(cache, "_session_list_cache_resolved_source_stamp", lambda _key: ("stable",))
key = _key()
cache._session_list_cache_set(key, {
"sessions": [{
"session_id": "s1",
"gateway_routing": {
"routing": [{"provider": "first", "model": "model-a"}],
},
}],
"settings": {"show_cli_sessions": False},
})
first, _ = cache._session_list_cache_get(key)
assert first is not None
first["sessions"][0]["gateway_routing"]["routing"][0]["provider"] = "mutated"
first["settings"]["show_cli_sessions"] = True
second, _ = cache._session_list_cache_get(key)
assert second is not None
assert second["sessions"][0]["gateway_routing"]["routing"][0]["provider"] == "first"
assert second["settings"]["show_cli_sessions"] is False
def test_invalidation_during_projection_cannot_reinsert_stale_payload(monkeypatch):
cache._session_list_cache_clear()
key = routes._session_list_cache_key(
active_profile="default",
all_profiles=False,
show_cli_sessions=False,
show_previous_messaging_sessions=False,
show_cron_sessions=False,
)
stale_payload = {"sessions": [{"session_id": "before-clear", "title": "stale"}]}
fresh_payload = {"sessions": [{"session_id": "after-clear", "title": "fresh"}]}
projection_started = threading.Event()
release_projection = threading.Event()
real_project = cache._session_list_cache_bounded_payload
def blocked_project(value):
if value == stale_payload:
projection_started.set()
assert release_projection.wait(5), "projection release timed out"
return real_project(value)
monkeypatch.setattr(cache, "_session_list_cache_bounded_payload", blocked_project)
result = {}
build_count = 0
def builder():
nonlocal build_count
build_count += 1
return stale_payload if build_count == 1 else fresh_payload
def owner():
result["payload"] = routes._get_cached_session_list_payload(
key=key,
builder=builder,
)
thread = threading.Thread(target=owner)
thread.start()
assert projection_started.wait(5), "cache set never reached projection"
cache._session_list_cache_clear()
release_projection.set()
thread.join(5)
assert not thread.is_alive()
assert build_count == 2
assert result["payload"] == fresh_payload
cached, fresh = cache._session_list_cache_get(key)
assert fresh is True
assert cached == fresh_payload
def test_sidebar_field_allowlist_has_one_authority():
fields = getattr(cache, "_SIDEBAR_SESSION_RESPONSE_FIELDS", None)
assert fields is not None
assert routes._SIDEBAR_SESSION_RESPONSE_FIELDS is fields
assert cache._session_list_cache_sidebar_fields() is fields
def test_sidebar_compact_omits_heavy_session_metadata(): def test_sidebar_compact_omits_heavy_session_metadata():
session = Session( session = Session(
session_id="heavy", session_id="heavy",
@@ -86,3 +174,43 @@ def test_sidebar_compact_omits_heavy_session_metadata():
"process_wakeup_pause", "share_token", "process_wakeup_pause", "share_token",
): ):
assert field not in sidebar assert field not in sidebar
def test_sidebar_metadata_only_projects_existing_index_rows(monkeypatch, tmp_path):
heavy_fields = {
"compression_anchor_summary": "s" * 1000000,
"compression_anchor_details": {"detail": "d" * 1000000},
"context_engine_state": {"state": "x" * 1000000},
"compression_recovery": {"recovery": "r" * 1000000},
"gateway_routing_history": [{"provider": "old"}] * 1000,
"composer_draft": {"draft": "c" * 1000000},
"process_wakeup_pause": {"pause": "p" * 1000000},
"share_token": "token-value",
}
index_path = tmp_path / "_index.json"
index_path.write_text(json.dumps([{
"session_id": "indexed",
"title": "Indexed",
"message_count": 1,
"user_message_count": 1,
"created_at": 1.0,
"updated_at": 2.0,
"last_message_at": 2.0,
"profile": "default",
**heavy_fields,
}]))
monkeypatch.setattr(models, "SESSION_DIR", tmp_path)
monkeypatch.setattr(models, "SESSION_INDEX_FILE", index_path)
monkeypatch.setattr(models, "SESSIONS", {})
monkeypatch.setattr(models, "_persisted_session_ids_snapshot", lambda: {"indexed"})
monkeypatch.setattr(models, "_active_stream_ids", lambda: set())
monkeypatch.setattr(models, "_apply_sidebar_state_db_overrides", lambda _rows: None)
rows = models.all_sessions(
include_lineage_metadata=False,
sidebar_metadata_only=True,
)
assert [row["session_id"] for row in rows] == ["indexed"]
for field in heavy_fields:
assert field not in rows[0]