From 7349a9434f61ca18f3dcfaeda8d80b45a1bbb3dc Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sun, 5 Jul 2026 04:03:22 +0000 Subject: [PATCH] fix(#5501): improve session-list retry feedback The sidebar's load-error Retry button behaved like a raw idle browser button while the next slow fetch was pending, so it looked broken/unresponsive. Retry now paints an immediate pending state (label 'Retrying...', disabled, aria-busy), keeps the retry state local to the error block via _renderSessionListLoadErrorNote(), and lets the existing success/failure paths replace the error object when the request settles (restoring 'Retry' on failure). Also adds proper styling. Contributor stage; gate-pass. Co-authored-by: rodboev --- static/sessions.js | 69 ++- static/style.css | 7 + ...issue5501_session_list_retry_affordance.py | 556 ++++++++++++++++++ 3 files changed, 615 insertions(+), 17 deletions(-) create mode 100644 tests/test_issue5501_session_list_retry_affordance.py diff --git a/static/sessions.js b/static/sessions.js index be37ce64..91a8db6b 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -4673,6 +4673,56 @@ function _showSessionListLoadError(error){ }; } +function _renderSessionListLoadErrorNote(){ + if(!_sessionListLoadError) return null; + const note=document.createElement('div'); + note.className='session-list-error session-empty-note'; + const title=document.createElement('div'); + title.textContent=_sessionListLoadError.message||'Could not load conversations.'; + note.appendChild(title); + if(_sessionListLoadError.detail){ + const detail=document.createElement('div'); + detail.className='session-list-error-detail'; + detail.textContent=_sessionListLoadError.detail; + note.appendChild(detail); + } + const retry=document.createElement('button'); + retry.type='button'; + retry.className='session-list-error-retry'; + const retrying=Boolean(_sessionListLoadError.retrying); + const setPending=()=>{ + retry.textContent='Retrying...'; + retry.disabled=true; + retry.setAttribute('aria-busy','true'); + retry.onclick=null; + }; + const bindRetry=()=>{ + retry.onclick=(e)=>{ + e.stopPropagation(); + if(!_sessionListLoadError||_sessionListLoadError.retrying) return; + setPending(); + _sessionListLoadError={..._sessionListLoadError,retrying:true}; + renderSessionListFromCache(); + void renderSessionList({deferWhileInteracting:false}).finally(()=>{ + if(!retry.parentNode||(_sessionListLoadError&&_sessionListLoadError.retrying)) return; + retry.textContent='Retry'; + retry.disabled=false; + retry.removeAttribute('aria-busy'); + bindRetry(); + }); + }; + }; + if(retrying){ + setPending(); + }else{ + retry.textContent='Retry'; + retry.disabled=false; + bindRetry(); + } + note.appendChild(retry); + return note; +} + async function _runRenderSessionListRefresh(opts, _gen){ const deferWhileInteracting=Boolean(opts&&opts.deferWhileInteracting); if(!deferWhileInteracting) _pendingSessionListPayload=null; @@ -6709,23 +6759,8 @@ function renderSessionListFromCache(){ if(_sessionSelectMode&&_selectedSessions.size>0){batchBar.style.display='flex';_renderBatchActionBar();} else{batchBar.style.display='none';} if(_sessionListLoadError){ - const note=document.createElement('div'); - note.className='session-list-error session-empty-note'; - const title=document.createElement('div'); - title.textContent=_sessionListLoadError.message||'Could not load conversations.'; - note.appendChild(title); - if(_sessionListLoadError.detail){ - const detail=document.createElement('div'); - detail.className='session-list-error-detail'; - detail.textContent=_sessionListLoadError.detail; - note.appendChild(detail); - } - const retry=document.createElement('button'); - retry.type='button'; - retry.textContent='Retry'; - retry.onclick=(e)=>{e.stopPropagation();_sessionListLoadError=null;void renderSessionList({deferWhileInteracting:false});}; - note.appendChild(retry); - list.appendChild(note); + const note=_renderSessionListLoadErrorNote(); + if(note) list.appendChild(note); } if(window._showCliSessions || cliSessionCount>0){ const sourceTabs=document.createElement('div'); diff --git a/static/style.css b/static/style.css index c04e8f2a..83b333a6 100644 --- a/static/style.css +++ b/static/style.css @@ -5525,6 +5525,13 @@ main.main > #mainPlugin{display:none;} .session-source-tab:hover{background:rgba(255,255,255,.08);color:var(--text);} .session-source-tab.active{background:var(--accent-bg);color:var(--accent-text);border-color:var(--accent-bg);} .session-empty-note{padding:20px 14px;color:var(--muted);font-size:12px;text-align:center;opacity:.7;} +.session-list-error{margin:4px 10px 8px;padding:10px 12px;border:1px solid var(--border2);border-radius:8px;background:var(--surface);color:var(--text);display:flex;flex-direction:column;align-items:flex-start;gap:4px;text-align:left;opacity:1;} +.session-list-error>div:first-child{font-size:12px;font-weight:600;line-height:1.35;word-break:break-word;} +.session-list-error-detail{font-size:11px;line-height:1.35;color:var(--muted);word-break:break-word;} +.session-list-error-retry{margin-top:4px;align-self:flex-start;display:inline-flex;align-items:center;justify-content:center;min-width:0;padding:5px 10px;border:1px solid var(--border);border-radius:8px;background:var(--input-bg);color:var(--text);font-size:11px;font-weight:600;line-height:1.2;cursor:pointer;transition:background .15s,border-color .15s,color .15s,opacity .15s;} +.session-list-error-retry:hover:not(:disabled):not([aria-busy="true"]){background:var(--hover-bg);border-color:var(--accent-bg-strong);color:var(--text);} +.session-list-error-retry:focus-visible{outline:2px solid var(--focus-ring);outline-offset:2px;} +.session-list-error-retry:disabled,.session-list-error-retry[aria-busy="true"]{background:var(--surface);border-color:var(--border2);color:var(--muted);cursor:wait;opacity:1;} .project-bar{display:flex;gap:4px;padding:4px 10px 8px;flex-wrap:wrap;align-items:center;flex-shrink:0;} .project-chip{font-size:10px;font-weight:600;padding:3px 8px;border-radius:12px;cursor:pointer;border:1px solid var(--border2);background:var(--input-bg);color:var(--muted);transition:all .15s;white-space:nowrap;display:inline-flex;align-items:center;gap:4px;-webkit-user-select:none;user-select:none;-webkit-touch-callout:none;touch-action:manipulation;} .project-chip:hover{background:rgba(255,255,255,.08);color:var(--text);} diff --git a/tests/test_issue5501_session_list_retry_affordance.py b/tests/test_issue5501_session_list_retry_affordance.py new file mode 100644 index 00000000..6d279375 --- /dev/null +++ b/tests/test_issue5501_session_list_retry_affordance.py @@ -0,0 +1,556 @@ +"""Regression coverage for issue #5501: session-list retry affordance.""" + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +SESSIONS_JS = ROOT / "static" / "sessions.js" +STYLE_CSS = ROOT / "static" / "style.css" +NODE = shutil.which("node") + + +def _extract_function(source_text, function_name): + marker = f"function {function_name}(" + start = source_text.index(marker) + brace_start = source_text.index("{", start) + depth = 0 + for index in range(brace_start, len(source_text)): + char = source_text[index] + if char == "{": + depth += 1 + elif char == "}": + depth -= 1 + if depth == 0: + return source_text[start : index + 1] + raise AssertionError(f"Could not extract {function_name}") + + +def _run_node(script): + proc = subprocess.run([NODE, "-e", script], capture_output=True, text=True, check=True) + return json.loads(proc.stdout) + + +def _build_node_script(extra_js): + src = SESSIONS_JS.read_text(encoding="utf-8") + show_error_fn = _extract_function(src, "_showSessionListLoadError") + retry_note_fn = _extract_function(src, "_renderSessionListLoadErrorNote") + return f""" +class FakeClassList {{ + constructor(node) {{ + this.node = node; + this.names = new Set(); + }} + _sync() {{ + this.node.className = Array.from(this.names).join(' '); + }} + add(...names) {{ + for (const name of names) {{ + if (name) this.names.add(name); + }} + this._sync(); + }} + remove(...names) {{ + for (const name of names) {{ + this.names.delete(name); + }} + this._sync(); + }} + contains(name) {{ + return this.names.has(name); + }} + toggle(name, force) {{ + const shouldAdd = force === undefined ? !this.names.has(name) : !!force; + if (shouldAdd) this.names.add(name); + else this.names.delete(name); + this._sync(); + return this.names.has(name); + }} +}} +class FakeElement {{ + constructor(tag) {{ + this.tagName = tag; + this.children = []; + this.parentNode = null; + this.attributes = {{}}; + this.dataset = {{}}; + this.style = {{}}; + this.className = ''; + this.classList = new FakeClassList(this); + this.disabled = false; + this.textContent = ''; + this.onclick = null; + this.id = ''; + this.title = ''; + this._innerHTML = ''; + }} + appendChild(child) {{ + this.children.push(child); + child.parentNode = this; + return child; + }} + setAttribute(name, value) {{ + this.attributes[name] = String(value); + if (name === 'class') {{ + this.className = String(value); + this.classList.names = new Set(String(value).split(/\\s+/).filter(Boolean)); + }} + if (name.startsWith('data-')) {{ + const key = name.slice(5).replace(/-([a-z])/g, (_, c) => c.toUpperCase()); + this.dataset[key] = String(value); + }} + }} + getAttribute(name) {{ + return Object.prototype.hasOwnProperty.call(this.attributes, name) ? this.attributes[name] : null; + }} + removeAttribute(name) {{ + delete this.attributes[name]; + if (name === 'class') {{ + this.className = ''; + this.classList.names = new Set(); + }} + if (name.startsWith('data-')) {{ + const key = name.slice(5).replace(/-([a-z])/g, (_, c) => c.toUpperCase()); + delete this.dataset[key]; + }} + }} + get innerHTML() {{ + return this._innerHTML; + }} + set innerHTML(value) {{ + this._innerHTML = String(value); + this.children = []; + }} +}} +function deferred() {{ + let resolve; + let reject; + const promise = new Promise((res, rej) => {{ + resolve = res; + reject = rej; + }}); + return {{ promise, resolve, reject }}; +}} +function findButton(node) {{ + return (node.children || []).find((child) => child.tagName === 'button') || null; +}} +const list = new FakeElement('div'); +list.id = 'sessionList'; +console.warn = () => {{}}; +global.window = {{ _showCliSessions: false }}; +global.document = {{ createElement: (tag) => new FakeElement(tag) }}; +global.$ = (id) => id === 'sessionList' ? list : null; +global._allSessions = []; +global._sessionListLoadError = null; +global._sessionListFromCacheCalls = []; +global.renderSessionListCalls = []; +global.renderSessionListFromCache = () => {{ + const phase = global._sessionListLoadError && global._sessionListLoadError.retrying ? 'retrying' : 'idle'; + global._sessionListFromCacheCalls.push(phase); + list.innerHTML = ''; + const note = _renderSessionListLoadErrorNote(); + if (note) {{ + list.appendChild(note); + return; + }} + for (const session of global._allSessions) {{ + const row = new FakeElement('div'); + row.className = 'session-item'; + row.textContent = session.session_id; + row.dataset.sid = session.session_id; + list.appendChild(row); + }} +}}; +const retryFlow = deferred(); +global.renderSessionList = (opts) => {{ + global.renderSessionListCalls.push(opts); + return retryFlow.promise.then( + () => {{ + global._sessionListLoadError = null; + global._allSessions = [{{ session_id: 'row-1' }}]; + global.renderSessionListFromCache(); + }}, + (error) => {{ + _showSessionListLoadError(error); + global._allSessions = []; + global.renderSessionListFromCache(); + throw error; + }}, + ).catch(() => {{}}); +}}; +{show_error_fn} +{retry_note_fn} +{extra_js} +""" + + +def _render_script(extra_js): + return _build_node_script(extra_js) + + +def _load_source(): + return SESSIONS_JS.read_text(encoding="utf-8"), STYLE_CSS.read_text(encoding="utf-8") + + +def test_retry_button_pending_state_paints_before_settlement(): + if NODE is None: + pytest.skip("node not on PATH") + + script = _render_script( + """ +_showSessionListLoadError(new Error('backend busy')); +global.renderSessionListFromCache(); +const note = list.children[0]; +const retry = findButton(note); +const before = { + noteClass: note.className, + retryClass: retry.className, + text: retry.textContent, + disabled: retry.disabled, + ariaBusy: retry.getAttribute('aria-busy'), + phases: [...global._sessionListFromCacheCalls], + renderCalls: [...global.renderSessionListCalls], + detail: note.children[1] ? note.children[1].textContent : null, +}; +retry.onclick({ stopPropagation() {} }); +const pendingNote = list.children[0]; +const pendingRetry = findButton(pendingNote); +console.log(JSON.stringify({ + before, + pending: { + noteClass: pendingNote.className, + retryClass: pendingRetry.className, + text: pendingRetry.textContent, + disabled: pendingRetry.disabled, + ariaBusy: pendingRetry.getAttribute('aria-busy'), + phases: [...global._sessionListFromCacheCalls], + renderCalls: [...global.renderSessionListCalls], + }, +})); +""" + ) + body = _run_node(script) + + assert body["before"] == { + "noteClass": "session-list-error session-empty-note", + "retryClass": "session-list-error-retry", + "text": "Retry", + "disabled": False, + "ariaBusy": None, + "phases": ["idle"], + "renderCalls": [], + "detail": "backend busy", + } + assert body["pending"] == { + "noteClass": "session-list-error session-empty-note", + "retryClass": "session-list-error-retry", + "text": "Retrying...", + "disabled": True, + "ariaBusy": "true", + "phases": ["idle", "retrying"], + "renderCalls": [{"deferWhileInteracting": False}], + } + + +def test_retry_button_mutates_immediately_when_full_repaint_is_blocked(): + if NODE is None: + pytest.skip("node not on PATH") + + script = _render_script( + """ +_showSessionListLoadError(new Error('backend busy')); +global.renderSessionListFromCache(); +const note = list.children[0]; +const retry = findButton(note); +global.renderSessionListFromCache = () => { + global._sessionListFromCacheCalls.push('blocked'); +}; +retry.onclick({ stopPropagation() {} }); +console.log(JSON.stringify({ + sameButton: retry === findButton(list.children[0]), + text: retry.textContent, + disabled: retry.disabled, + ariaBusy: retry.getAttribute('aria-busy'), + onclick: retry.onclick === null, + phases: [...global._sessionListFromCacheCalls], + renderCalls: [...global.renderSessionListCalls], +})); +""" + ) + body = _run_node(script) + + assert body == { + "sameButton": True, + "text": "Retrying...", + "disabled": True, + "ariaBusy": "true", + "onclick": True, + "phases": ["idle", "blocked"], + "renderCalls": [{"deferWhileInteracting": False}], + } + + +def test_retry_button_success_clears_pending_state_when_settlement_repaint_is_blocked(): + if NODE is None: + pytest.skip("node not on PATH") + + script = _render_script( + """ +_showSessionListLoadError(new Error('backend busy')); +global.renderSessionListFromCache(); +const note = list.children[0]; +const retry = findButton(note); +global.renderSessionListFromCache = () => { + global._sessionListFromCacheCalls.push('blocked'); +}; +retry.onclick({ stopPropagation() {} }); +const pending = { + text: retry.textContent, + disabled: retry.disabled, + ariaBusy: retry.getAttribute('aria-busy'), + onclick: retry.onclick === null, +}; +retryFlow.resolve(); +(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + console.log(JSON.stringify({ + pending, + settled: { + text: retry.textContent, + disabled: retry.disabled, + ariaBusy: retry.getAttribute('aria-busy'), + onclick: typeof retry.onclick, + }, + phases: [...global._sessionListFromCacheCalls], + renderCalls: [...global.renderSessionListCalls], + error: global._sessionListLoadError, + })); +})().catch((error) => { + console.error(error); + process.exit(1); +}); +""" + ) + body = _run_node(script) + + assert body["pending"] == { + "text": "Retrying...", + "disabled": True, + "ariaBusy": "true", + "onclick": True, + } + assert body["settled"] == { + "text": "Retry", + "disabled": False, + "ariaBusy": None, + "onclick": "function", + } + assert body["phases"] == ["idle", "blocked", "blocked"] + assert body["renderCalls"] == [{"deferWhileInteracting": False}] + assert body["error"] is None + + +def test_retry_button_failure_restores_click_handler_when_settlement_repaint_is_blocked(): + if NODE is None: + pytest.skip("node not on PATH") + + script = _render_script( + """ +_showSessionListLoadError(new Error('backend busy')); +global.renderSessionListFromCache(); +const note = list.children[0]; +const retry = findButton(note); +global.renderSessionListFromCache = () => { + global._sessionListFromCacheCalls.push('blocked'); +}; +retry.onclick({ stopPropagation() {} }); +const pending = { + text: retry.textContent, + disabled: retry.disabled, + ariaBusy: retry.getAttribute('aria-busy'), + onclick: retry.onclick === null, +}; +retryFlow.reject(new Error('boom')); +(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + console.log(JSON.stringify({ + pending, + settled: { + text: retry.textContent, + disabled: retry.disabled, + ariaBusy: retry.getAttribute('aria-busy'), + onclick: typeof retry.onclick, + }, + phases: [...global._sessionListFromCacheCalls], + renderCalls: [...global.renderSessionListCalls], + error: global._sessionListLoadError, + })); +})().catch((error) => { + console.error(error); + process.exit(1); +}); +""" + ) + body = _run_node(script) + + assert body["pending"] == { + "text": "Retrying...", + "disabled": True, + "ariaBusy": "true", + "onclick": True, + } + assert body["settled"] == { + "text": "Retry", + "disabled": False, + "ariaBusy": None, + "onclick": "function", + } + assert body["phases"] == ["idle", "blocked", "blocked"] + assert body["renderCalls"] == [{"deferWhileInteracting": False}] + assert body["error"] == { + "message": "Could not load conversations.", + "detail": "boom", + } + + +def test_retry_button_success_repaints_rows_after_settlement(): + if NODE is None: + pytest.skip("node not on PATH") + + script = _render_script( + """ +_showSessionListLoadError(new Error('backend busy')); +global.renderSessionListFromCache(); +const note = list.children[0]; +const retry = findButton(note); +retry.onclick({ stopPropagation() {} }); +const pending = { + text: findButton(list.children[0]).textContent, + disabled: findButton(list.children[0]).disabled, + ariaBusy: findButton(list.children[0]).getAttribute('aria-busy'), +}; +retryFlow.resolve(); +(async () => { + await Promise.resolve(); + const rows = list.children.map((child) => ({ + className: child.className, + text: child.textContent, + })); + console.log(JSON.stringify({ + pending, + phases: [...global._sessionListFromCacheCalls], + renderCalls: [...global.renderSessionListCalls], + error: global._sessionListLoadError, + rows, + })); +})().catch((error) => { + console.error(error); + process.exit(1); +}); +""" + ) + body = _run_node(script) + + assert body["pending"] == { + "text": "Retrying...", + "disabled": True, + "ariaBusy": "true", + } + assert body["phases"] == ["idle", "retrying", "idle"] + assert body["renderCalls"] == [{"deferWhileInteracting": False}] + assert body["error"] is None + assert body["rows"] == [{"className": "session-item", "text": "row-1"}] + + +def test_retry_button_failure_restores_idle_retry_state(): + if NODE is None: + pytest.skip("node not on PATH") + + script = _render_script( + """ +_showSessionListLoadError(new Error('backend busy')); +global.renderSessionListFromCache(); +const note = list.children[0]; +const retry = findButton(note); +retry.onclick({ stopPropagation() {} }); +const pending = { + text: findButton(list.children[0]).textContent, + disabled: findButton(list.children[0]).disabled, + ariaBusy: findButton(list.children[0]).getAttribute('aria-busy'), +}; +retryFlow.reject(new Error('boom')); +(async () => { + await Promise.resolve(); + const failureNote = list.children[0]; + const retryAfterFailure = findButton(failureNote); + console.log(JSON.stringify({ + pending, + phases: [...global._sessionListFromCacheCalls], + renderCalls: [...global.renderSessionListCalls], + error: global._sessionListLoadError, + failureNote: { + title: failureNote.children[0] ? failureNote.children[0].textContent : null, + detail: failureNote.children[1] ? failureNote.children[1].textContent : null, + }, + retryAfterFailure: { + text: retryAfterFailure.textContent, + disabled: retryAfterFailure.disabled, + ariaBusy: retryAfterFailure.getAttribute('aria-busy'), + }, + rows: list.children.map((child) => ({ + className: child.className, + text: child.textContent, + })), + })); +})().catch((error) => { + console.error(error); + process.exit(1); +}); +""" + ) + body = _run_node(script) + + assert body["pending"] == { + "text": "Retrying...", + "disabled": True, + "ariaBusy": "true", + } + assert body["phases"] == ["idle", "retrying", "idle"] + assert body["renderCalls"] == [{"deferWhileInteracting": False}] + assert body["error"] == { + "message": "Could not load conversations.", + "detail": "boom", + } + assert body["failureNote"] == { + "title": "Could not load conversations.", + "detail": "boom", + } + assert body["retryAfterFailure"] == { + "text": "Retry", + "disabled": False, + "ariaBusy": None, + } + assert body["rows"][0]["className"] == "session-list-error session-empty-note" + + +def test_session_list_error_css_and_retry_options_are_preserved(): + src, css = _load_source() + + assert ".session-list-error{" in css + assert ".session-list-error-detail{" in css + assert ".session-list-error-retry{" in css + assert ".session-list-error-retry:focus-visible{" in css + assert ".session-list-error-retry:disabled,.session-list-error-retry[aria-busy=\"true\"]" in css + assert "opacity:1" in css + assert "const sessionRequestOpts={" in src + assert "retries:1," in src + assert "retryStatuses:[502,503,504]," in src + boot_gate = src.index("if(!_sessionListHasLoadedOnce){") + assert src.index("retries:1,") < boot_gate + assert src.index("retryStatuses:[502,503,504],") < boot_gate + assert "sessionRequestOpts.timeoutMs=_SESSION_LIST_BOOT_TIMEOUT_MS;" in src + assert "sessionRequestOpts.retryTimeouts=true;" in src