From 617c1f451df9f1edad619e708d4147306545ac0a Mon Sep 17 00:00:00 2001 From: Jaret Bottoms <25535214+jbbottoms@users.noreply.github.com> Date: Sun, 12 Jul 2026 10:30:54 -0500 Subject: [PATCH] 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. --- static/sessions.js | 44 +++- .../test_firefox_sidebar_scroll_stability.py | 4 +- ...est_issue5960_cron_unread_profile_scope.py | 246 ++++++++++++++++++ ...t_issue856_background_completion_unread.py | 4 +- tests/test_session_sidebar_resilience.py | 2 +- 5 files changed, 290 insertions(+), 10 deletions(-) diff --git a/static/sessions.js b/static/sessions.js index b9c6d274..d43910a6 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1172,7 +1172,21 @@ function _markPollingCompletionUnreadTransitions(sessions) { const meta = (typeof _cronCompletionUnreadMetaForSession === 'function') ? _cronCompletionUnreadMetaForSession(s) : null; - _markSessionCompletionUnread(sid, s.message_count, meta); + // 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; - _markPollingCompletionUnreadTransitions(_allSessions); + // 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 diff --git a/tests/test_firefox_sidebar_scroll_stability.py b/tests/test_firefox_sidebar_scroll_stability.py index 281b01dc..d99e3ba4 100644 --- a/tests/test_firefox_sidebar_scroll_stability.py +++ b/tests/test_firefox_sidebar_scroll_stability.py @@ -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" ) diff --git a/tests/test_issue5960_cron_unread_profile_scope.py b/tests/test_issue5960_cron_unread_profile_scope.py index 8d638cca..35d1514e 100644 --- a/tests/test_issue5960_cron_unread_profile_scope.py +++ b/tests/test_issue5960_cron_unread_profile_scope.py @@ -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" diff --git a/tests/test_issue856_background_completion_unread.py b/tests/test_issue856_background_completion_unread.py index 9844e6ad..656dddea 100644 --- a/tests/test_issue856_background_completion_unread.py +++ b/tests/test_issue856_background_completion_unread.py @@ -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 " diff --git a/tests/test_session_sidebar_resilience.py b/tests/test_session_sidebar_resilience.py index 7c4caa62..2233d27d 100644 --- a/tests/test_session_sidebar_resilience.py +++ b/tests/test_session_sidebar_resilience.py @@ -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():