fix(sidebar): reject cached fallback rows from the wrong hidden-filter mode (#4775)
This commit is contained in:
1 parent
9296604667
commit
bd72c785b5
4 files changed
+45
-33
No files matched your search
+9
-2
@@ -1637,10 +1637,14 @@ function _requestedSessionSidebarSource() {
|
||||
return window._showCliSessions ? _sessionSourceFilter : 'webui';
|
||||
}
|
||||
|
||||
function _sessionListExcludeHiddenEnabled() {
|
||||
return _activeProject===null || _activeProject===NO_PROJECT_FILTER;
|
||||
}
|
||||
|
||||
function _sessionListQueryString() {
|
||||
const qs = new URLSearchParams();
|
||||
qs.set('sidebar_source', _requestedSessionSidebarSource());
|
||||
if(_activeProject===null || _activeProject===NO_PROJECT_FILTER) qs.set('exclude_hidden','1');
|
||||
if(_sessionListExcludeHiddenEnabled()) qs.set('exclude_hidden','1');
|
||||
if(_showAllProfiles) qs.set('all_profiles','1');
|
||||
if(_showArchived) qs.set('include_archived','1');
|
||||
return `?${qs.toString()}`;
|
||||
@@ -3882,6 +3886,7 @@ function _applySessionListPayload(sessData, projData){
|
||||
: (S.activeProfile || 'default'),
|
||||
allProfiles: !!_showAllProfiles,
|
||||
sidebarSource: _requestedSessionSidebarSource(),
|
||||
excludeHidden: _sessionListExcludeHiddenEnabled(),
|
||||
};
|
||||
_syncSessionAttentionSoundState(_allSessions);
|
||||
_pruneLineageReportCacheToVisibleSessions(_allSessions);
|
||||
@@ -3987,11 +3992,13 @@ async function _runRenderSessionListRefresh(opts, _gen){
|
||||
profile: S.activeProfile || 'default',
|
||||
allProfiles: !!_showAllProfiles,
|
||||
sidebarSource: _requestedSessionSidebarSource(),
|
||||
excludeHidden: _sessionListExcludeHiddenEnabled(),
|
||||
};
|
||||
const _scopeMatches = _allSessionsScope
|
||||
&& _allSessionsScope.profile === _curScope.profile
|
||||
&& _allSessionsScope.allProfiles === _curScope.allProfiles
|
||||
&& _allSessionsScope.sidebarSource === _curScope.sidebarSource;
|
||||
&& _allSessionsScope.sidebarSource === _curScope.sidebarSource
|
||||
&& _allSessionsScope.excludeHidden === _curScope.excludeHidden;
|
||||
// #4671: the /api/sessions fetch failed — clear the skeleton flag so this error
|
||||
// render (matched cache, or empty rows for a mismatched scope) replaces the
|
||||
// up-front profile-switch skeleton instead of stranding it.
|
||||
|
||||
@@ -1,16 +1,5 @@
|
||||
"""Regression tests for #4167 review items — sidebar payload allowlist must
|
||||
preserve read-only + gateway-routing fields, and the failed-refresh path must
|
||||
not re-render a prior profile's sessions after a profile switch.
|
||||
"""Regression tests for #4167 sidebar payload and scope guards."""
|
||||
|
||||
Item 1/2: _SIDEBAR_SESSION_RESPONSE_FIELDS must include read_only / is_read_only
|
||||
(so the sidebar suppresses rename / action-menu / swipe on read-only sessions)
|
||||
and gateway_routing (so the model label renders). Dropping these silently
|
||||
regressed both surfaces.
|
||||
|
||||
Item 3: renderSessionList()'s catch path must scope-tag the cache and clear it
|
||||
when the requested profile scope differs, instead of unconditionally
|
||||
re-rendering the cached (prior-profile) rows.
|
||||
"""
|
||||
import re
|
||||
import sys
|
||||
from pathlib import Path
|
||||
@@ -35,7 +24,6 @@ def test_sidebar_response_item_preserves_read_only_flag():
|
||||
|
||||
fn = getattr(routes, "_sidebar_session_response_item", None)
|
||||
if fn is None:
|
||||
# Helper name differs across versions — fall back to allowlist assertion.
|
||||
assert "read_only" in routes._SIDEBAR_SESSION_RESPONSE_FIELDS
|
||||
return
|
||||
row = fn({
|
||||
@@ -51,12 +39,14 @@ def test_sidebar_response_item_preserves_read_only_flag():
|
||||
|
||||
|
||||
def test_failed_refresh_clears_cache_on_profile_scope_change():
|
||||
"""The catch path in renderSessionList must clear stale rows when the
|
||||
requested scope differs from the cached scope (no cross-profile leak)."""
|
||||
"""The catch path must reject cached rows from a mismatched sidebar scope."""
|
||||
src = (_ROOT / "static" / "sessions.js").read_text(encoding="utf-8")
|
||||
# The cache must be scope-tagged.
|
||||
assert "_allSessionsScope" in src, "cache is not scope-tagged"
|
||||
# The catch path must compare current scope to the cached scope and clear
|
||||
# _allSessions when they differ rather than always rendering from cache.
|
||||
assert re.search(r"_scopeMatches", src), "catch path does not gate fallback on scope match"
|
||||
assert re.search(r"_allSessions\s*=\s*\[\]", src), "catch path does not clear stale rows on scope mismatch"
|
||||
assert "excludeHidden: _sessionListExcludeHiddenEnabled()" in src, (
|
||||
"scope tagging must include the hidden-filter query mode"
|
||||
)
|
||||
assert "_allSessionsScope.excludeHidden === _curScope.excludeHidden" in src, (
|
||||
"catch path must reject cached rows fetched under the wrong hidden-filter mode"
|
||||
)
|
||||
@@ -250,6 +250,7 @@ def test_frontend_sends_sidebar_source_param():
|
||||
def test_session_list_query_string_respects_sidebar_source_and_flags():
|
||||
src = SESSIONS_JS.read_text(encoding="utf-8")
|
||||
requested_source_fn = _extract_function(src, "_requestedSessionSidebarSource")
|
||||
exclude_hidden_fn = _extract_function(src, "_sessionListExcludeHiddenEnabled")
|
||||
query_fn = _extract_function(src, "_sessionListQueryString")
|
||||
script = f"""
|
||||
global.window = {{ _showCliSessions: true }};
|
||||
@@ -258,6 +259,7 @@ global._sessionSourceFilter = 'cli';
|
||||
global._showAllProfiles = true;
|
||||
global._showArchived = false;
|
||||
{requested_source_fn}
|
||||
{exclude_hidden_fn}
|
||||
{query_fn}
|
||||
const first = _sessionListQueryString();
|
||||
window._showCliSessions = false;
|
||||
@@ -325,6 +327,7 @@ console.log(JSON.stringify({{
|
||||
def test_apply_payload_and_tab_count_helpers_cover_old_and_new_payloads():
|
||||
src = SESSIONS_JS.read_text(encoding="utf-8")
|
||||
requested_source_fn = _extract_function(src, "_requestedSessionSidebarSource")
|
||||
exclude_hidden_fn = _extract_function(src, "_sessionListExcludeHiddenEnabled")
|
||||
apply_fn = _extract_function(src, "_applySessionListPayload")
|
||||
count_fn = _extract_function(src, "_sessionSourceTabCount")
|
||||
clear_fn = _extract_function(src, "_clearSessionSourceTabCounts")
|
||||
@@ -345,6 +348,8 @@ global._sessionListLoadError = null;
|
||||
global._sessionListHasLoadedOnce = false;
|
||||
global._sessionListFirstRenderAnimated = true;
|
||||
global._sessionListSkeletonActive = true;
|
||||
global._activeProject = null;
|
||||
global.NO_PROJECT_FILTER = '__none__';
|
||||
global._showAllProfiles = false;
|
||||
global._sessionSourceFilter = 'webui';
|
||||
global.S = {{ activeProfile: 'default' }};
|
||||
@@ -358,13 +363,14 @@ global.startStreamingPoll = () => {{}};
|
||||
global.stopStreamingPoll = () => {{}};
|
||||
global.ensureSessionTimeRefreshPoll = () => {{}};
|
||||
global.ensureActiveSessionExternalRefreshPoll = () => {{}};
|
||||
global.ensureSessionEventsSSE = () => {{}};
|
||||
global.animateNextSessionListRefresh = () => {{}};
|
||||
global.renderSessionListFromCache = () => {{}};
|
||||
{clear_fn}
|
||||
{count_fn}
|
||||
{requested_source_fn}
|
||||
{apply_fn}
|
||||
global.ensureSessionEventsSSE = () => {{}};
|
||||
global.animateNextSessionListRefresh = () => {{}};
|
||||
global.renderSessionListFromCache = () => {{}};
|
||||
{clear_fn}
|
||||
{count_fn}
|
||||
{requested_source_fn}
|
||||
{exclude_hidden_fn}
|
||||
{apply_fn}
|
||||
const sessions = [{{ session_id: 'webui-1' }}];
|
||||
_applySessionListPayload({{ sessions, other_profile_count: 0, archived_count: 0, active_profile: 'default' }}, {{ projects: [] }});
|
||||
const oldPayload = {{
|
||||
@@ -520,6 +526,7 @@ def test_scope_mismatch_error_path_respects_sidebar_source():
|
||||
purge_fn = _extract_function(src, "_purgeStaleInflightEntries")
|
||||
clear_fn = _extract_function(src, "_clearSessionSourceTabCounts")
|
||||
requested_source_fn = _extract_function(src, "_requestedSessionSidebarSource")
|
||||
exclude_hidden_fn = _extract_function(src, "_sessionListExcludeHiddenEnabled")
|
||||
query_fn = _extract_function(src, "_sessionListQueryString")
|
||||
refresh_fn = _extract_function(src, "_runRenderSessionListRefresh").replace(
|
||||
"function _runRenderSessionListRefresh",
|
||||
@@ -537,6 +544,8 @@ global._profileSwitchListEmbargo = false;
|
||||
global._pendingSessionListPayload = null;
|
||||
global._allProjects = [];
|
||||
global._contentSearchResults = ['stale'];
|
||||
global._activeProject = null;
|
||||
global.NO_PROJECT_FILTER = '__none__';
|
||||
global.S = {{ activeProfile: 'default' }};
|
||||
global.$ = () => ({{ value: '' }});
|
||||
global._isSessionListUserInteracting = () => false;
|
||||
@@ -558,12 +567,13 @@ global.renderSessionListFromCache = () => {{
|
||||
}});
|
||||
}};
|
||||
global.api = () => Promise.reject(new Error('boom'));
|
||||
global.clearInflightState = sid => cleared.push(sid);
|
||||
{purge_fn}
|
||||
{clear_fn}
|
||||
{requested_source_fn}
|
||||
{query_fn}
|
||||
{refresh_fn}
|
||||
global.clearInflightState = sid => cleared.push(sid);
|
||||
{purge_fn}
|
||||
{clear_fn}
|
||||
{requested_source_fn}
|
||||
{exclude_hidden_fn}
|
||||
{query_fn}
|
||||
{refresh_fn}
|
||||
async function runCase(requestedSource, cachedSource) {{
|
||||
global._sessionSourceFilter = requestedSource;
|
||||
global._allSessions = [{{ session_id: cachedSource + '-1' }}];
|
||||
@@ -571,6 +581,7 @@ async function runCase(requestedSource, cachedSource) {{
|
||||
profile: 'default',
|
||||
allProfiles: false,
|
||||
sidebarSource: cachedSource,
|
||||
excludeHidden: true,
|
||||
}};
|
||||
global._sessionListSourceById = new Map([['webui-live', 'webui']]);
|
||||
global.INFLIGHT = {{ 'webui-live': {{ lastAssistantText: 'working' }} }};
|
||||
@@ -609,6 +620,7 @@ async function runCase(requestedSource, cachedSource) {{
|
||||
"profile": "default",
|
||||
"allProfiles": False,
|
||||
"sidebarSource": "cli",
|
||||
"excludeHidden": True,
|
||||
}
|
||||
assert body["mismatch"]["webui"] is None
|
||||
assert body["mismatch"]["cli"] is None
|
||||
@@ -622,6 +634,7 @@ async function runCase(requestedSource, cachedSource) {{
|
||||
"profile": "default",
|
||||
"allProfiles": False,
|
||||
"sidebarSource": "webui",
|
||||
"excludeHidden": True,
|
||||
}
|
||||
assert body["match"]["webui"] == 11
|
||||
assert body["match"]["cli"] == 5
|
||||
|
||||
@@ -245,6 +245,7 @@ def test_omit_exclude_hidden_still_returns_default_hidden_rows(monkeypatch):
|
||||
def test_default_and_unassigned_queries_send_exclude_hidden(monkeypatch):
|
||||
src = SESSIONS_JS.read_text(encoding="utf-8")
|
||||
requested_source_fn = _extract_function(src, "_requestedSessionSidebarSource")
|
||||
exclude_hidden_fn = _extract_function(src, "_sessionListExcludeHiddenEnabled")
|
||||
project_filter_fn = _extract_function(src, "_setActiveProjectFilter")
|
||||
query_fn = _extract_function(src, "_sessionListQueryString")
|
||||
script = f"""
|
||||
@@ -257,6 +258,7 @@ global.NO_PROJECT_FILTER = '__none__';
|
||||
global.renderSessionListFromCache = () => {{}};
|
||||
global.renderSessionList = () => Promise.resolve();
|
||||
{requested_source_fn}
|
||||
{exclude_hidden_fn}
|
||||
{project_filter_fn}
|
||||
{query_fn}
|
||||
const default_query = _sessionListQueryString();
|
||||
|
||||
Reference in new issue
Block a user