diff --git a/static/panels.js b/static/panels.js index 8928c77d8..240d66543 100644 --- a/static/panels.js +++ b/static/panels.js @@ -3159,7 +3159,7 @@ function loadTodos() { if (!todos.length) { if (typeof _todosLastRenderedHash !== 'undefined' && _todosLastRenderedHash === '__empty__') return; - panel.innerHTML = `
${esc(t('todos_no_active'))}
`; + panel.innerHTML = renderTodoEmptyState(); if (typeof _todosLastRenderedHash !== 'undefined') _todosLastRenderedHash = '__empty__'; return; } @@ -3170,18 +3170,9 @@ function loadTodos() { _todosLastRenderedHash = hash; } - const statusIcon = {pending:li('square',14), in_progress:li('loader',14), completed:li('check',14), cancelled:li('x',14)}; - const statusColor = {pending:'var(--muted)', in_progress:'var(--blue)', completed:'rgba(100,200,100,.8)', cancelled:'rgba(200,100,100,.5)'}; // Single innerHTML join is the cheapest correct way to materialize // ~10–50 leaf nodes. All user-controlled content goes through esc(). - panel.innerHTML = todos.map(td => ` -
- ${statusIcon[td.status]||li('square',14)} -
-
${esc(td.content)}
-
${esc(td.id)} · ${esc(td.status)}
-
-
`).join(''); + panel.innerHTML = renderTodoRows(todos, {metadata:true}); } // Legacy fallback: reverse-scan settled tool messages for the most diff --git a/static/ui.js b/static/ui.js index b3bb0fe2d..7060c2bf5 100644 --- a/static/ui.js +++ b/static/ui.js @@ -7215,7 +7215,7 @@ function clearInflightState(sid){ // `S.session = ...` settle point so cross-session navigation never // leaves a stale list visible. // -// The hash is keyed on (id, content, status); the render itself uses +// The hash is keyed on (id, content/text, status); the render itself uses // `esc()` for any user-controlled string, so XSS surface is the same as // any other innerHTML path in this file. let _todosLastRenderedHash=null; @@ -7232,11 +7232,73 @@ function _todosHash(items){ let h=items.length+'|'; for(let i=0;i${esc(t('todos_no_active'))}`; +} + +function renderTodoRow(todo,options={}){ + const td=todo||{}; + const status=todoStatusKey(td.status); + const visual=todoStatusVisual(status); + const showMetadata=!(options&&options.metadata===false); + const isCompleted=status==='completed'; + const contentColor=isCompleted?'var(--muted)':'var(--text)'; + const completedStyle=isCompleted?'text-decoration:line-through;opacity:.5':''; + const metadata=showMetadata + ? `
${esc(td.id)} · ${esc(status)}
` + : ''; + return ` +
+ ${renderTodoStatusIcon(status,14)} +
+
${esc(todoContent(td))}
+ ${metadata} +
+
`; +} + +function renderTodoRows(todos,options={}){ + const items=Array.isArray(todos)?todos:[]; + return items.map(td=>renderTodoRow(td,options)).join(''); +} + function _todosPanelIsActive(){ if(typeof document==='undefined') return false; const panel=document.getElementById('panelTodos'); diff --git a/static/workspace.js b/static/workspace.js index 0ddda0d71..09ca4eae1 100644 --- a/static/workspace.js +++ b/static/workspace.js @@ -378,30 +378,10 @@ function _loadWorkspacePanelTodos(){ } }catch(e){ todos = []; } if(!todos.length){ - if(_workspaceTodosLastRenderedHash === '__empty__') return; - panel.innerHTML = '
No active tasks
'; - _workspaceTodosLastRenderedHash = '__empty__'; + panel.innerHTML = renderTodoEmptyState({centered:true}); return; } - const hash = _workspaceTodosHash(todos); - if(hash === _workspaceTodosLastRenderedHash) return; - _workspaceTodosLastRenderedHash = hash; - const statusIcon = (s) => { - if(s === 'completed') return ''; - if(s === 'in_progress') return ''; - if(s === 'cancelled') return ''; - // pending - return ''; - }; - const items = todos.map(t => { - const s = t.status || 'pending'; - const isDone = s === 'completed' || s === 'cancelled'; - return `
`+ - `${statusIcon(s)}`+ - `${_escHtml(t.content||t.text||'')}`+ - `
`; - }).join(''); - panel.innerHTML = `
${items}
`; + panel.innerHTML = renderTodoRows(todos, {metadata:true}); } function _escHtml(s){ diff --git a/tests/test_todo_panel_cold_load_static.py b/tests/test_todo_panel_cold_load_static.py index 51b6b869d..c7e90a41b 100644 --- a/tests/test_todo_panel_cold_load_static.py +++ b/tests/test_todo_panel_cold_load_static.py @@ -1,5 +1,9 @@ +import shutil +import subprocess from pathlib import Path +import pytest + REPO_ROOT = Path(__file__).parent.parent @@ -39,7 +43,7 @@ def test_legacy_todos_fallback_still_uses_raw_session_messages(): def test_workspace_todos_tab_prefers_live_sse_snapshot_before_cold_load_sidecar(): src = (REPO_ROOT / "static" / "workspace.js").read_text(encoding="utf-8") start = src.find("function _loadWorkspacePanelTodos()") - end = src.find("function _escHtml", start) + end = src.find("const ARTIFACT_IGNORE_RE", start) assert start != -1 assert end != -1 @@ -54,17 +58,107 @@ def test_workspace_todos_tab_prefers_live_sse_snapshot_before_cold_load_sidecar( ) -def test_workspace_todos_render_uses_separate_hash_cache(): +def test_todo_panels_delegate_rendering_to_shared_helpers(): + ui = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") + panels = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8") + workspace = (REPO_ROOT / "static" / "workspace.js").read_text(encoding="utf-8") + + assert "const TODO_STATUS_RENDERING=Object.freeze({" in ui + assert "function renderTodoStatusIcon(status,size=14)" in ui + assert "function renderTodoRow(todo,options={})" in ui + assert "function renderTodoRows(todos,options={})" in ui + assert "function renderTodoEmptyState(options={})" in ui + + left_start = panels.find("function loadTodos()") + left_end = panels.find("function _legacyTodosFromMessages()", left_start) + workspace_start = workspace.find("function _loadWorkspacePanelTodos()") + workspace_end = workspace.find("const ARTIFACT_IGNORE_RE", workspace_start) + + assert left_start != -1 and left_end != -1 + assert workspace_start != -1 and workspace_end != -1 + left_block = panels[left_start:left_end] + workspace_block = workspace[workspace_start:workspace_end] + + assert "renderTodoEmptyState()" in left_block + assert "renderTodoRows(todos, {metadata:true})" in left_block + assert "renderTodoEmptyState({centered:true})" in workspace_block + assert "renderTodoRows(todos, {metadata:true})" in workspace_block + + +def test_workspace_todos_no_longer_defines_local_icon_or_empty_text_mapping(): src = (REPO_ROOT / "static" / "workspace.js").read_text(encoding="utf-8") start = src.find("function _loadWorkspacePanelTodos()") - end = src.find("function _escHtml", start) + end = src.find("const ARTIFACT_IGNORE_RE", start) - assert "let _workspaceTodosLastRenderedHash = null;" in src - assert "function _resetWorkspaceTodosRenderCache()" in src - assert start != -1 - assert end != -1 + assert start != -1 and end != -1 helper = src[start:end] - assert "_workspaceTodosLastRenderedHash === '__empty__'" in helper - assert "_workspaceTodosHash(todos)" in helper - assert "if(hash === _workspaceTodosLastRenderedHash) return;" in helper + assert "No active tasks" not in helper + assert "const statusIcon" not in helper + assert "`; +}} +function esc(value) {{ + return String(value ?? '').replace(/[&<>"']/g, c => ({{'&':'&','<':'<','>':'>','"':'"',"'":'''}}[c])); +}} +function t(key) {{ return key === 'todos_no_active' ? 'No active task list in this session.' : key; }} +const api = new Function('li', 'esc', 't', helper + '; return {{TODO_STATUS_RENDERING, todoStatusKey, renderTodoRow, renderTodoRows, renderTodoEmptyState}};')(li, esc, t); +function assert(cond, msg) {{ if(!cond) throw new Error(msg); }} +const expected = {{ + pending: ['square', 'var(--muted)'], + in_progress: ['loader', 'var(--blue)'], + completed: ['check', 'rgba(100,200,100,.8)'], + cancelled: ['x', 'rgba(200,100,100,.5)'], +}}; +for (const [status, [icon, color]] of Object.entries(expected)) {{ + assert(api.TODO_STATUS_RENDERING[status].icon === icon, status + ' icon mismatch'); + assert(api.TODO_STATUS_RENDERING[status].color === color, status + ' color mismatch'); + const row = api.renderTodoRow({{id: status + '-id', content: status + ' content', status}}, {{metadata: true}}); + assert(row.includes(`data-icon="${{icon}}"`), status + ' row icon mismatch'); + assert(row.includes(color), status + ' row color mismatch'); + assert(row.includes(`${{status}}-id · ${{status}}`), status + ' metadata mismatch'); +}} +assert(api.todoStatusKey('unknown') === 'pending', 'unknown statuses fall back to pending'); +assert(api.renderTodoRows([{{id:'a', content:'A', status:'pending'}}, {{id:'b', text:'B', status:'completed'}}], {{metadata:true}}).includes('b · completed'), 'shared rows include metadata'); +assert(api.renderTodoEmptyState({{centered:true}}).includes('No active task list in this session.'), 'empty state uses i18n text'); +""" + script_path = tmp_path / "shared_todo_renderer_test.js" + script_path.write_text(script, encoding="utf-8") + result = subprocess.run( + ["node", str(script_path)], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0, result.stderr or result.stdout