Skip to content

release #5562: refresh sidebar recency after missed session events - #5591

Merged
nesquena-hermes merged 2 commits into
masterfrom
release/stage-5562
Jul 4, 2026
Merged

nesquena-hermes merged 2 commits into
masterfrom
release/stage-5562

Conversation

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Release: #5562 — refresh sidebar recency after missed session events

Ships @rodboev's sidebar reliability fix for #5551, rebuilt on current master from the live PR head and fully gated.

The bug (#5551)

A conversation reactivated from another device (or another tab) while your current tab was backgrounded/unfocused was not bumped to the top / into "Today" until you manually refreshed. refreshSessionList() early-returns while document.hidden unless force:true, and the sidebar SSE closes on blur — but the in-flight/pending refresh coalescing only preserved the refresh reason string and dropped the opts, so a coalesced sessions_changed refresh lost its force:true and got skipped on a hidden tab.

The fix (static/sessions.js)

  • Pending-refresh state changed from a bare reason string to {reason, opts}, with _mergeSessionListRefreshOptions() OR-merging force / refreshActive across coalesced requests so a forced refresh's flag survives.
  • _refreshSessionListAfterSidebarResume() — a dedicated resume path that clears the onopen catch-up flag and forces a refresh when the sidebar regains focus.
  • The in-flight guard stores the merged request; the finally-block re-schedules with the preserved opts.

Gate (green)

Attribution: original author @rodboev (Co-authored-by trailer preserved).

Closes #5562
Closes #5551

nesquena-hermes and others added 2 commits July 4, 2026 22:34
A reactivated older conversation wasn't bumped to the top / into Today until a
manual refresh when the viewing tab was backgrounded/unfocused: refreshSessionList
early-returns on document.hidden unless force=true, and the sidebar SSE closes on
blur — but the in-flight/pending coalescing only preserved the refresh REASON
string, dropping the opts (force/refreshActive). So a coalesced 'sessions_changed'
refresh lost its force flag and got skipped on a hidden tab. Now the pending
request preserves merged opts (force/refreshActive OR-merged), and a dedicated
resume-refresh path forces a catch-up when the sidebar regains focus, so
cross-device recency re-sorts land without a manual refresh.

Co-authored-by: rodboev <rodboev@users.noreply.github.com>
@nesquena-hermes
nesquena-hermes merged commit b504fe3 into master Jul 4, 2026
18 checks passed
@nesquena-hermes
nesquena-hermes deleted the release/stage-5562 branch July 4, 2026 22:39
@greptile-apps

greptile-apps Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a coalescing bug where a pending refreshSessionList call lost its force:true option when it was queued behind an in-flight refresh, causing a tab that went hidden mid-refresh to silently skip a required sidebar re-sort from another device. The fix upgrades the pending-refresh state from a bare reason string to a {reason, opts} object and introduces _mergeSessionListRefreshOptions() to OR-merge boolean flags across coalesced requests.

  • _mergeSessionListRefreshOptions(prev, next) correctly handles the override case (next.force=false with prev.force=true) by spreading first and then explicitly OR-setting the protected flags — force and refreshActive now survive any number of coalescing steps in both the _scheduleSessionEventsRefresh debounce window and the refreshSessionList in-flight queue.
  • _refreshSessionListAfterSidebarResume(reason) centralises the focus/visible/reconnect refresh paths, clears _sessionEventsNeedsRefreshOnOpen synchronously (preventing a double-refresh when the SSE reconnects right after focus), and forces the list render unconditionally.
  • _closeSessionEventsSSE() now sets _sessionEventsNeedsRefreshOnOpen = true on every close (not just on SSE error), ensuring the onopen catch-up path fires even when the close was triggered by a blur/hidden transition rather than a network error.

Confidence Score: 4/5

Safe to merge — the change is narrowly scoped to the pending-refresh coalescing path, the runtime behaviour change (force:true on SSE events) is intentional and bounded to the blur-to-close transition window, and 31 frontend/poll/SSE tests plus three new Node-executed integration tests all pass.

The fix is logically correct and well-tested. The only note is that two slightly different _function_body helper implementations now exist in the test suite — one regex-based (older file) and one brace-depth counting (new file) — where the brace-depth version is more correct for nested braces and is used to extract code that runs in Node.js. No functional defects in production code were found.

tests/test_webui_external_refresh_frontend.py — the new _function_body helper is more robust than the one in test_issue3916_external_refresh_poll.py; consider consolidating to avoid divergence.

Important Files Changed

Filename Overview
static/sessions.js Core fix: pending refresh state upgraded from bare reason string to {reason, opts} with OR-merge of force/refreshActive flags; _closeSessionEventsSSE now sets NeedsRefreshOnOpen; sessions_changed events now always pass force:true to bypass document.hidden guard
tests/test_webui_external_refresh_frontend.py Three new Node.js-executed integration tests verifying opts coalescing, force survival, and timer merging; introduces a brace-depth _function_body helper that differs from the regex-based one already in test_issue3916_external_refresh_poll.py
tests/test_issue3916_external_refresh_poll.py Updated assertions to match new _scheduleSessionEventsRefresh signature and sessions_changed call site; structurally unchanged
tests/test_issue4151_pwa_focus_sse.py One new assertion verifying the focus hook now calls _refreshSessionListAfterSidebarResume instead of refreshSessionList directly
CHANGELOG.md New changelog entry describing the sidebar recency fix; part of the release process

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SSE sessions_changed event] -->|force:true, refreshActive:true| B[_scheduleSessionEventsRefresh]
    B --> C{_sessionEventsRefreshPendingRequest\nOR-merge force/refreshActive}
    C -->|timer already running| D[Merge opts into pending request]
    C -->|no timer| E[Set pending request\nStart 300ms timer]
    E --> F[Timer fires]
    D --> F
    F --> G[refreshSessionList\nrequest.reason, request.opts]
    G --> H{_sessionListRefreshInFlight?}
    H -->|yes| I[Coalesce: OR-merge opts\ninto _sessionListRefreshPendingRequest]
    H -->|no| J{force:true OR\ndocument.hidden==false?}
    J -->|no force AND hidden| K[Early return - skipped]
    J -->|force OR visible| L[renderSessionList\nrefreshActiveSession if refreshActive]
    L --> M[finally: check pending]
    M -->|pending exists| N[_scheduleSessionEventsRefresh\nwith preserved opts]
    M -->|no pending| O[Done]

    P[Tab blur/hidden] --> Q[_closeSessionEventsSSE\nsets NeedsRefreshOnOpen=true]
    R[Tab focus] --> S[_refreshSessionListAfterSidebarResume\nclears NeedsRefreshOnOpen\nforce:true refresh]
    T[SSE onopen] --> U{NeedsRefreshOnOpen?}
    U -->|true| V[_refreshSessionListAfterSidebarResume\nreconnect]
    U -->|false| W[No-op - focus already handled]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[SSE sessions_changed event] -->|force:true, refreshActive:true| B[_scheduleSessionEventsRefresh]
    B --> C{_sessionEventsRefreshPendingRequest\nOR-merge force/refreshActive}
    C -->|timer already running| D[Merge opts into pending request]
    C -->|no timer| E[Set pending request\nStart 300ms timer]
    E --> F[Timer fires]
    D --> F
    F --> G[refreshSessionList\nrequest.reason, request.opts]
    G --> H{_sessionListRefreshInFlight?}
    H -->|yes| I[Coalesce: OR-merge opts\ninto _sessionListRefreshPendingRequest]
    H -->|no| J{force:true OR\ndocument.hidden==false?}
    J -->|no force AND hidden| K[Early return - skipped]
    J -->|force OR visible| L[renderSessionList\nrefreshActiveSession if refreshActive]
    L --> M[finally: check pending]
    M -->|pending exists| N[_scheduleSessionEventsRefresh\nwith preserved opts]
    M -->|no pending| O[Done]

    P[Tab blur/hidden] --> Q[_closeSessionEventsSSE\nsets NeedsRefreshOnOpen=true]
    R[Tab focus] --> S[_refreshSessionListAfterSidebarResume\nclears NeedsRefreshOnOpen\nforce:true refresh]
    T[SSE onopen] --> U{NeedsRefreshOnOpen?}
    U -->|true| V[_refreshSessionListAfterSidebarResume\nreconnect]
    U -->|false| W[No-op - focus already handled]
Loading

Reviews (1): Last reviewed commit: "release #5562: refresh sidebar recency a..." | Re-trigger Greptile

Comment on lines 7 to +28
@@ -7,6 +11,33 @@
PANELS_JS = Path("static/panels.js").read_text(encoding="utf-8")


def _function_body(src: str, name: str) -> str:
# Extracted functions intentionally avoid braces in strings so this stays source-light.
m = re.search(rf"(?:async\s+)?function\s+{re.escape(name)}\b", src)
assert m, f"{name} function not found"
sig_end = src.find(")", m.end())
assert sig_end != -1, f"{name} function signature not terminated"
brace_start = src.find("{", sig_end)
assert brace_start != -1, f"{name} function body not found"
depth = 0
for idx in range(brace_start, len(src)):
ch = src[idx]
if ch == "{":
depth += 1
elif ch == "}":
depth -= 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Two different _function_body implementations now coexist in the test suite. The version in test_issue3916_external_refresh_poll.py uses a simple re.DOTALL | re.MULTILINE regex (.*?^}}), which can truncate at the first top-level } on its own line inside a nested block. The new version here uses brace-depth counting and is more robust for nested braces. Consider replacing the regex-based version in the other file with the brace-depth one (or sharing via conftest.py) so both files use the same, more correct helper — especially since the brace-depth version is now used to extract code that is actually executed in Node.js, where an incorrect extraction would silently produce wrong test results.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(session-list): reactivated older conversation not bumped to top / into Today until manual refresh

1 participant