fix(cron): drop stale pre-switch session-list completion marks

Greptile #5975 P1: capture _cronPollGeneration before /api/sessions await and
re-check in _applySessionListPayload before completion-unread marking so a
delayed profile-A response cannot remount cron dots after switch to B.
Also skip inactive-profile cron marks in single-profile mode. Tests cover the
race, positive path, and collateral call-shape contracts.
This commit is contained in:
Jaret Bottoms authored and nesquena-hermes committed 2026-07-12 19:20:53 +00:00
1 parent b17e76d171
commit 617c1f451d
5 files changed
+288 -8

No files matched your search

+36 -4
View File
@@ -1172,7 +1172,21 @@ function _markPollingCompletionUnreadTransitions(sessions) {
const meta = (typeof _cronCompletionUnreadMetaForSession === 'function')
? _cronCompletionUnreadMetaForSession(s)
: null;
// Defense: never re-create a cron unread for a non-active profile while
// the sidebar is single-profile (stale pre-switch payloads).
const allProfilesOn = (typeof _showAllProfiles !== 'undefined' && !!_showAllProfiles);
if (
meta
&& meta.source === 'cron'
&& meta.profile
&& !allProfilesOn
&& typeof _cronMarkerProfileMatchesActive === 'function'
&& !_cronMarkerProfileMatchesActive(meta.profile, (typeof S !== 'undefined' && S && S.activeProfile) || 'default')
) {
// Skip mark for inactive-profile cron row.
} else {
_markSessionCompletionUnread(sid, s.message_count, meta);
}
} else {
// Sync viewed count so we don't flag stale unread on tab switch (#3020)
_setSessionViewedCount(sid, messageCount);
@@ -5039,7 +5053,11 @@ function _schedulePendingSessionListApply(){
const payload=_pendingSessionListPayload;
_pendingSessionListPayload=null;
if(payload.gen!==_renderSessionListGen) return;
_applySessionListPayload(payload.sessData,payload.projData);
// Profile switch may have bumped unread gen after the list gen check
// window; still drop completion-marking for the stale pre-switch payload.
_applySessionListPayload(payload.sessData,payload.projData,{
unreadGen:payload.unreadGen,
});
}, Math.max(120, SESSION_LIST_INTERACTION_IDLE_MS));
}
@@ -5109,10 +5127,11 @@ function _sessionListRenderSignature(){
]);
}catch(_){ return null; }
}
function _applySessionListPayload(sessData, projData){
function _applySessionListPayload(sessData, projData, opts){
// Server's other_profile_count tells us how many sessions exist outside the
// active profile so the "Show N from other profiles" toggle can render
// without a second round-trip. Stashed on the module for renderSessionListFromCache.
const applyOpts = (opts && typeof opts === 'object') ? opts : {};
_otherProfileCount = sessData.other_profile_count || 0;
_archivedWebuiCount = Number(sessData.archived_webui_count ?? sessData.archived_count ?? 0);
_archivedCliCount = Number(sessData.archived_cli_count ?? 0);
@@ -5176,7 +5195,16 @@ function _applySessionListPayload(sessData, projData){
const _hadSessionListLoadError = !!_sessionListLoadError;
_sessionListLoadError = null;
_sessionListHasLoadedOnce = true;
// Greptile #5975 P1: a /api/sessions request started under profile A can
// finish after a switch to B already cleared A's cron markers. The list gen
// check can already have passed (TOCTOU) or a deferred apply can land later.
// Re-validate the profile-switch unread generation (shared with cron poll
// reset via _cronPollGeneration) immediately before marking completions.
const expectedUnreadGen = applyOpts.unreadGen;
const currentUnreadGen = (typeof _cronPollGeneration === 'number') ? _cronPollGeneration : 0;
if (typeof expectedUnreadGen !== 'number' || expectedUnreadGen === currentUnreadGen) {
_markPollingCompletionUnreadTransitions(_allSessions);
}
const isStreaming = _allSessions.some(s => _isSessionEffectivelyStreaming(s));
if (isStreaming) {
startStreamingPoll();
@@ -5315,6 +5343,10 @@ function _renderSessionListLoadErrorNote(){
async function _runRenderSessionListRefresh(opts, _gen){
const deferWhileInteracting=Boolean(opts&&opts.deferWhileInteracting);
if(!deferWhileInteracting) _pendingSessionListPayload=null;
// Capture profile-switch unread generation BEFORE the await so a switch
// mid-flight (which increments _cronPollGeneration) invalidates completion
// marking for this response even if list gen checks already passed.
const unreadGen = (typeof _cronPollGeneration === 'number') ? _cronPollGeneration : 0;
try{
if(!($('sessionSearch').value||'').trim()) _contentSearchResults = [];
const sessionListQS = _sessionListQueryString();
@@ -5343,11 +5375,11 @@ async function _runRenderSessionListRefresh(opts, _gen){
// renderSessionList(), so that render's payload is the first allowed to paint.
if (_profileSwitchListEmbargo) return;
if(deferWhileInteracting&&_isSessionListUserInteracting()){
_pendingSessionListPayload={gen:_gen,sessData,projData};
_pendingSessionListPayload={gen:_gen,sessData,projData,unreadGen};
_schedulePendingSessionListApply();
return;
}
_applySessionListPayload(sessData,projData);
_applySessionListPayload(sessData,projData,{unreadGen});
}catch(e){
if (_gen !== _renderSessionListGen) return;
// #4671: same embargo guard as the success path — a mid-switch /api/sessions that
@@ -33,9 +33,9 @@ def test_polling_payloads_are_deferred_while_user_scrolls_sidebar():
assert "async function _runRenderSessionListRefresh(opts, _gen)" in refresh_block
assert "const deferWhileInteracting=Boolean(opts&&opts.deferWhileInteracting);" in refresh_block
assert "if(deferWhileInteracting&&_isSessionListUserInteracting())" in refresh_block
assert "_pendingSessionListPayload={gen:_gen,sessData,projData};" in refresh_block
assert "_pendingSessionListPayload={gen:_gen,sessData,projData,unreadGen};" in refresh_block
assert "_schedulePendingSessionListApply();" in refresh_block
assert "_applySessionListPayload(sessData,projData);" in refresh_block
assert "_applySessionListPayload(sessData,projData,{unreadGen});" in refresh_block
assert "_markPollingCompletionUnreadTransitions(_allSessions);" in apply_block, (
"deferring sidebar refreshes must preserve background-completion unread semantics"
)
@@ -301,9 +301,12 @@ def test_session_list_path_tags_cron_markers_with_source_and_profile():
is_cron = _extract_function(SESSIONS_JS, "_isCronSessionForUnread")
meta_fn = _extract_function(SESSIONS_JS, "_cronCompletionUnreadMetaForSession")
source_key = _extract_function(SESSIONS_JS, "_sourceKeyForSession")
match_fn = _extract_function(SESSIONS_JS, "_cronMarkerProfileMatchesActive")
profile_match = _extract_function(SESSIONS_JS, "_profileMatchesActiveProfile")
script = f"""
const markCalls=[];
global.S={{activeProfile:'profile-a',activeProfileIsDefault:false}};
let _showAllProfiles=false;
global._allSessions=[];
global._sessionListSnapshotById=new Map();
global._sessionStreamingById=new Map();
@@ -323,6 +326,8 @@ function _markSessionCompletionUnread(sid, count, meta){{
{source_key}
{is_cron}
{meta_fn}
{profile_match}
{match_fn}
{mark_poll}
const sessions=[
{{
@@ -478,3 +483,244 @@ process.stdout.write(JSON.stringify({{
assert state["otherCleared"] is True
assert "root-cron" in state["persisted"]
assert "other-cron" not in state["persisted"]
@pytest.mark.skipif(NODE is None, reason="node not on PATH")
def test_stale_pre_switch_session_list_does_not_recreate_cron_markers():
"""Greptile #5975 P1: delayed /api/sessions after profile switch must not remount cron dots."""
# Re-read sources so this test always sees the latest shipped functions.
sessions_js = (ROOT / "static" / "sessions.js").read_text(encoding="utf-8")
panels_js = (ROOT / "static" / "panels.js").read_text(encoding="utf-8")
apply_fn = _extract_function(sessions_js, "_applySessionListPayload")
mark_poll = _extract_function(sessions_js, "_markPollingCompletionUnreadTransitions")
helpers = "\n".join(
[
_extract_function(sessions_js, "_isCronSessionForUnread"),
_extract_function(sessions_js, "_sourceKeyForSession"),
_extract_function(sessions_js, "_cronCompletionUnreadMetaForSession"),
_extract_function(sessions_js, "_resolveCronCompletionMarkerOrigin"),
_extract_function(sessions_js, "_cronMarkerProfileMatchesActive"),
_extract_function(sessions_js, "_profileMatchesActiveProfile"),
_extract_function(sessions_js, "_getSessionCompletionUnread"),
_extract_function(sessions_js, "_saveSessionCompletionUnread"),
_extract_function(sessions_js, "_markSessionCompletionUnread"),
_extract_function(sessions_js, "_hasSessionCompletionUnread"),
_extract_function(sessions_js, "_hasUnreadForSession"),
_extract_function(sessions_js, "_clearCronSessionCompletionUnreadForInactiveProfiles"),
_extract_function(panels_js, "_resetCronUnreadForProfileSwitch"),
]
)
script = f"""
const store={{'hermes-session-completion-unread':JSON.stringify({{}})}};
global.localStorage={{
getItem:(key)=>Object.prototype.hasOwnProperty.call(store,key)?store[key]:null,
setItem:(key,value)=>{{ store[key]=String(value); }},
removeItem:(key)=>{{ delete store[key]; }},
}};
let _sessionCompletionUnread=null;
let _sessionViewedCounts={{}};
const SESSION_COMPLETION_UNREAD_KEY='hermes-session-completion-unread';
let _cronPollGeneration=0;
let _cronPollSince=10;
let _cronUnreadCount=0;
const _cronNewJobIds=new Set(['old-job']);
let _allSessions=[];
let _allSessionsScope=null;
let _sidebarReferenceSessions=[];
let _otherProfileCount=0;
let _archivedWebuiCount=0;
let _archivedCliCount=0;
let _serverWebuiSessionCount=null;
let _serverCliSessionCount=null;
let _serverTimeDelta=0;
let _serverTz=null;
let _sessionListLoadError=null;
let _sessionListHasLoadedOnce=false;
let _sessionListFirstRenderAnimated=true;
let _showAllProfiles=false;
const _optimisticallyRemovedSessionIds=new Set();
const _sessionStreamingById=new Map([['old-cron', true]]);
const _sessionListSnapshotById=new Map([['old-cron', {{message_count:1, last_message_at:10}}]]);
const _sessionListSourceById=new Map();
global.S={{activeProfile:'profile-a',activeProfileIsDefault:false}};
global.renderSessionListFromCache=()=>{{}};
global.updateCronBadge=()=>{{ _cronUnreadCount=_cronNewJobIds.size; }};
function _getSessionViewedCounts(){{ return _sessionViewedCounts; }}
function _setSessionViewedCount(){{}}
function _getSessionObservedStreaming(){{ return {{}}; }}
function _isSessionEffectivelyStreaming(){{ return false; }}
function _hasPendingUserMessageSignal(){{ return false; }}
function _isSessionActivelyViewedForList(){{ return false; }}
function _rememberSessionListSource(){{}}
function _rememberObservedStreamingSession(){{}}
function _forgetObservedStreamingSession(){{}}
function _reconcileActiveSessionIdleStateFromList(){{}}
function _mergeOptimisticFirstTurnSessions(s){{ return s; }}
function _recordSessionProfileCount(){{}}
function _syncSessionAttentionSoundState(){{}}
function _pruneLineageReportCacheToVisibleSessions(){{}}
function _requestedSessionSidebarSource(){{ return 'webui'; }}
function _sessionListExcludeHiddenEnabled(){{ return false; }}
function startStreamingPoll(){{}}
function stopStreamingPoll(){{}}
function ensureSessionTimeRefreshPoll(){{}}
function ensureActiveSessionExternalRefreshPoll(){{}}
function animateNextSessionListRefresh(){{}}
function ensureSessionEventsSSE(){{}}
function _activeSessionIdForSidebar(){{ return null; }}
function _sessionListRenderSignature(){{ return 'sig'; }}
function _purgeStaleInflightEntries(){{}}
let _sessionListSkeletonActive=false;
let _sessionListRefreshAnimationPending=false;
let _lastSessionListRenderSig=null;
let _renamingSid=null;
let _sessionActionMenu=null;
let _allProjects=[];
{helpers}
{mark_poll}
{apply_fn}
// Snapshot generation under profile A (request starts).
const unreadGenAtStart=_cronPollGeneration;
// Profile switch to B: bumps unread gen + clears markers.
global.S.activeProfile='profile-b';
_resetCronUnreadForProfileSwitch();
// Delayed pre-switch /api/sessions payload (profile A cron still streaming→done).
const sessData={{
sessions:[{{
session_id:'old-cron',
message_count:2,
last_message_at:20,
source_tag:'cron',
profile:'profile-a',
is_streaming:false,
}}],
active_profile:'profile-a',
other_profile_count:0,
}};
_applySessionListPayload(sessData, {{projects:[]}}, {{unreadGen:unreadGenAtStart}});
const persisted=JSON.parse(store['hermes-session-completion-unread']||'{{}}');
process.stdout.write(JSON.stringify({{
generation:_cronPollGeneration,
badgeJobs:Array.from(_cronNewJobIds),
oldCronMarked:Object.prototype.hasOwnProperty.call(persisted,'old-cron'),
persisted,
}}));
"""
result = subprocess.run(
[NODE, "-e", script], check=True, capture_output=True, text=True, timeout=30
)
state = json.loads(result.stdout)
assert state["generation"] == 1
assert state["badgeJobs"] == []
assert state["oldCronMarked"] is False
assert "old-cron" not in state["persisted"]
@pytest.mark.skipif(NODE is None, reason="node not on PATH")
def test_fresh_session_list_still_marks_when_unread_gen_matches():
"""Sanity: matching unreadGen still allows completion marks after switch."""
sessions_js = (ROOT / "static" / "sessions.js").read_text(encoding="utf-8")
apply_fn = _extract_function(sessions_js, "_applySessionListPayload")
mark_poll = _extract_function(sessions_js, "_markPollingCompletionUnreadTransitions")
helpers = "\n".join(
[
_extract_function(sessions_js, "_isCronSessionForUnread"),
_extract_function(sessions_js, "_sourceKeyForSession"),
_extract_function(sessions_js, "_cronCompletionUnreadMetaForSession"),
_extract_function(sessions_js, "_cronMarkerProfileMatchesActive"),
_extract_function(sessions_js, "_profileMatchesActiveProfile"),
_extract_function(sessions_js, "_getSessionCompletionUnread"),
_extract_function(sessions_js, "_saveSessionCompletionUnread"),
_extract_function(sessions_js, "_markSessionCompletionUnread"),
_extract_function(sessions_js, "_hasSessionCompletionUnread"),
_extract_function(sessions_js, "_hasUnreadForSession"),
]
)
script = f"""
const store={{'hermes-session-completion-unread':JSON.stringify({{}})}};
global.localStorage={{
getItem:(key)=>Object.prototype.hasOwnProperty.call(store,key)?store[key]:null,
setItem:(key,value)=>{{ store[key]=String(value); }},
removeItem:(key)=>{{ delete store[key]; }},
}};
let _sessionCompletionUnread=null;
let _sessionViewedCounts={{}};
const SESSION_COMPLETION_UNREAD_KEY='hermes-session-completion-unread';
let _cronPollGeneration=3;
let _allSessions=[];
let _allSessionsScope=null;
let _sidebarReferenceSessions=[];
let _otherProfileCount=0;
let _archivedWebuiCount=0;
let _archivedCliCount=0;
let _serverWebuiSessionCount=null;
let _serverCliSessionCount=null;
let _serverTimeDelta=0;
let _serverTz=null;
let _sessionListLoadError=null;
let _sessionListHasLoadedOnce=false;
let _sessionListFirstRenderAnimated=true;
let _showAllProfiles=false;
const _optimisticallyRemovedSessionIds=new Set();
const _sessionStreamingById=new Map([['new-cron', true]]);
const _sessionListSnapshotById=new Map([['new-cron', {{message_count:1, last_message_at:10}}]]);
const _sessionListSourceById=new Map();
global.S={{activeProfile:'profile-b',activeProfileIsDefault:false}};
global.renderSessionListFromCache=()=>{{}};
function _getSessionViewedCounts(){{ return _sessionViewedCounts; }}
function _setSessionViewedCount(){{}}
function _getSessionObservedStreaming(){{ return {{}}; }}
function _isSessionEffectivelyStreaming(){{ return false; }}
function _hasPendingUserMessageSignal(){{ return false; }}
function _isSessionActivelyViewedForList(){{ return false; }}
function _rememberSessionListSource(){{}}
function _rememberObservedStreamingSession(){{}}
function _forgetObservedStreamingSession(){{}}
function _reconcileActiveSessionIdleStateFromList(){{}}
function _mergeOptimisticFirstTurnSessions(s){{ return s; }}
function _recordSessionProfileCount(){{}}
function _syncSessionAttentionSoundState(){{}}
function _pruneLineageReportCacheToVisibleSessions(){{}}
function _requestedSessionSidebarSource(){{ return 'webui'; }}
function _sessionListExcludeHiddenEnabled(){{ return false; }}
function startStreamingPoll(){{}}
function stopStreamingPoll(){{}}
function ensureSessionTimeRefreshPoll(){{}}
function ensureActiveSessionExternalRefreshPoll(){{}}
function animateNextSessionListRefresh(){{}}
function ensureSessionEventsSSE(){{}}
function _sessionListRenderSignature(){{ return 'sig'; }}
function _purgeStaleInflightEntries(){{}}
let _sessionListSkeletonActive=false;
let _sessionListRefreshAnimationPending=false;
let _lastSessionListRenderSig=null;
let _renamingSid=null;
let _sessionActionMenu=null;
let _allProjects=[];
{helpers}
{mark_poll}
{apply_fn}
_applySessionListPayload({{
sessions:[{{
session_id:'new-cron',
message_count:2,
last_message_at:20,
source_tag:'cron',
profile:'profile-b',
is_streaming:false,
}}],
active_profile:'profile-b',
}}, {{projects:[]}}, {{unreadGen:3}});
const persisted=JSON.parse(store['hermes-session-completion-unread']||'{{}}');
process.stdout.write(JSON.stringify({{
marked:Object.prototype.hasOwnProperty.call(persisted,'new-cron'),
meta:persisted['new-cron']||null,
}}));
"""
result = subprocess.run(
[NODE, "-e", script], check=True, capture_output=True, text=True, timeout=30
)
state = json.loads(result.stdout)
assert state["marked"] is True
assert state["meta"]["source"] == "cron"
assert state["meta"]["profile"] == "profile-b"
@@ -163,7 +163,9 @@ def test_polling_transition_marks_completion_unread_without_sse_done():
assert "_markSessionCompletionUnread(sid, s.message_count" in transition_block
assert "_sessionStreamingById.set(sid, isStreaming);" in transition_block
assert "const _streamingPollMs = 30000;" in SESSIONS_JS
assert "_applySessionListPayload(sessData,projData);" in refresh_block
# Greptile #5975: apply carries unreadGen so stale pre-switch lists skip mark.
assert "_applySessionListPayload(sessData,projData,{unreadGen});" in refresh_block
assert "unreadGen" in refresh_block
assert "_markPollingCompletionUnreadTransitions(_allSessions);" in apply_block
assert "_allSessions.some(s => _isSessionEffectivelyStreaming(s))" in apply_block, (
"the streaming poll fallback must stay active for the same server-confirmed "
+1 -1
View File
@@ -53,7 +53,7 @@ def test_sessions_and_projects_load_independently_so_projects_failure_cannot_bla
assert "return await api('/api/projects' + projectQS,{timeoutToast:false});" in helper
assert "console.warn('renderProjectsList',projectError);" in helper
assert "const projData = await projectPromise;" in helper
assert "_applySessionListPayload(sessData,projData)" in block
assert "_applySessionListPayload(sessData,projData,{unreadGen})" in block
def test_sessions_api_always_retries_transient_upstream_statuses_and_boot_keeps_longer_timeout():