diff --git a/backend/apps/agents/manager/session/SessionLifecycle.py b/backend/apps/agents/manager/session/SessionLifecycle.py index ca23310d..12b998ad 100644 --- a/backend/apps/agents/manager/session/SessionLifecycle.py +++ b/backend/apps/agents/manager/session/SessionLifecycle.py @@ -256,15 +256,23 @@ class SessionLifecycle(AgentManagerProtocol): @typechecked def p_dashboard_card_ids(self, dashboard_id: str) -> Set[str]: """Session ids the dashboard's layout currently has agent cards for. - Read straight off disk (no dashboards-module import, avoids a cycle).""" - try: - import os - import backend.config.paths as config_paths - from backend.config.json_store import read_json_or_none - d = read_json_or_none(os.path.join(config_paths.DASHBOARDS_DIR, f"{dashboard_id}.json")) or {} - return set((d.get("layout", {}).get("cards") or {}).keys()) - except Exception: + Read straight off disk (no dashboards-module import, avoids a cycle). + + Missing file = a real empty board. Anything ELSE fails loud on purpose: answering "no + cards" off a garbled or half-written file makes list_sessions answer "no sessions", and + the renderer treats that as authority, strips its store, deletes every card, and the + debounced layout save then persists the wipe. A transient read error must surface as an + error, never as an empty board (the ENG-271 wipe chain, reproduced live). + """ + import os + import json + import backend.config.paths as config_paths + path = os.path.join(config_paths.DASHBOARDS_DIR, f"{dashboard_id}.json") + if not os.path.exists(path): return set() + with open(path, encoding="utf-8") as f: + d = json.load(f) + return set((d.get("layout", {}).get("cards") or {}).keys()) @typechecked def get_session(self, session_id: str) -> Optional[AgentSession]: diff --git a/backend/tests/test_sessions_list_never_wipes_on_read_error.py b/backend/tests/test_sessions_list_never_wipes_on_read_error.py new file mode 100644 index 00000000..525fac08 --- /dev/null +++ b/backend/tests/test_sessions_list_never_wipes_on_read_error.py @@ -0,0 +1,50 @@ +"""A transient read error must surface as an error, never as an empty board. + +The wipe chain, reproduced live on 2026-08-12 (it destroyed a real dev dashboard): the sessions +list answering [] is treated as AUTHORITY by the renderer, which strips its store, deletes every +card, and the debounced layout save persists the wipe. Card-less sessions are never promoted from +disk again, so one wrong [] is permanent. p_dashboard_card_ids used to swallow every exception into +an empty set, which turned a garbled or half-written dashboard file (a crash mid-save is exactly +the world ENG-244/246 live in) into that wrong []. Missing file stays a real empty board. +""" + +import json +import os +from typing import Any +import pytest +import backend.config.paths as config_paths +from backend.apps.agents.agent_manager import agent_manager + + +@pytest.fixture() +def dash_dir(tmp_path: Any, monkeypatch: pytest.MonkeyPatch) -> str: + d = tmp_path / "dashboards" + d.mkdir() + monkeypatch.setattr(config_paths, "DASHBOARDS_DIR", str(d)) + return str(d) + + +def test_a_missing_dashboard_file_is_a_real_empty_board(dash_dir: str) -> None: + assert agent_manager.p_dashboard_card_ids("no-such-dashboard") == set() + + +def test_a_readable_dashboard_yields_its_card_ids(dash_dir: str) -> None: + with open(os.path.join(dash_dir, "d1.json"), "w", encoding="utf-8") as f: + json.dump({"layout": {"cards": {"s1": {"session_id": "s1"}, "s2": {"session_id": "s2"}}}}, f) + assert agent_manager.p_dashboard_card_ids("d1") == {"s1", "s2"} + + +def test_a_garbled_dashboard_file_fails_loud_instead_of_answering_empty(dash_dir: str) -> None: + """The whole point: corrupt must NOT read as empty, because empty is a delete instruction.""" + with open(os.path.join(dash_dir, "d2.json"), "w", encoding="utf-8") as f: + f.write('{"layout": {"cards": {') + with pytest.raises(Exception): + agent_manager.p_dashboard_card_ids("d2") + + +def test_get_all_sessions_propagates_the_read_error(dash_dir: str) -> None: + """And the caller must not quietly catch it back into a [] either.""" + with open(os.path.join(dash_dir, "d3.json"), "w", encoding="utf-8") as f: + f.write("not json at all") + with pytest.raises(Exception): + agent_manager.get_all_sessions(dashboard_id="d3") diff --git a/frontend/src/shared/state/agentsSlice.ts b/frontend/src/shared/state/agentsSlice.ts index 47c7085d..c355dfb4 100644 --- a/frontend/src/shared/state/agentsSlice.ts +++ b/frontend/src/shared/state/agentsSlice.ts @@ -194,11 +194,16 @@ const initialState: AgentsState = { export const fetchSessions = createAsyncThunk( 'agents/fetchSessions', - async ({ dashboardId }: { dashboardId?: string } = {}) => { + // dashboardId is REQUIRED: the fulfilled reducer treats the response as authority and strips + // dead sessions, and an unscoped answer must never carry that power (see the reducer). + async ({ dashboardId }: { dashboardId: string }) => { const params = new URLSearchParams(); - if (dashboardId) params.set('dashboard_id', dashboardId); - const qs = params.toString(); - const res = await fetch(`${AGENTS_API}/sessions${qs ? `?${qs}` : ''}`); + params.set('dashboard_id', dashboardId); + const res = await fetch(`${AGENTS_API}/sessions?${params.toString()}`); + // Same rule the layout fetch learned the hard way: a non-2xx body silently parsing to "no + // sessions" is how a healthy board gets wiped. An error must land in .rejected (which strips + // nothing), not masquerade as an empty fulfilled. + if (!res.ok) throw new Error(`sessions fetch failed: ${res.status}`); const data = await res.json(); return data.sessions as AgentSession[]; }, @@ -1199,13 +1204,19 @@ const agentsSlice = createSlice({ // Strip sessions the server no longer has, but a dashboard's list is only authoritative for ITS OWN sessions: hopping dashboards must not eat the finished chat you were just reading (it looked like wiped history). const fetchedDashboardId = action.meta.arg?.dashboardId; - for (const [id, existing] of Object.entries(state.sessions)) { - if (fetchedIds.has(id)) continue; - if (fetchedDashboardId && existing.dashboard_id !== fetchedDashboardId) continue; - if (existing.status === 'draft') continue; - if (state.trackedNotificationIds.includes(id)) continue; - if (activeStatuses.has(existing.status)) continue; - delete state.sessions[id]; + // No scope = no authority to delete ANYTHING. An unscoped answer is memory-only on the + // backend (empty right after a respawn), and one such payload stripping globally is the + // start of the wipe chain: store emptied -> reconcile deletes every card -> debounced save + // persists it, permanently, because card-less sessions are never promoted again (ENG-271). + if (fetchedDashboardId) { + for (const [id, existing] of Object.entries(state.sessions)) { + if (fetchedIds.has(id)) continue; + if (existing.dashboard_id !== fetchedDashboardId) continue; + if (existing.status === 'draft') continue; + if (state.trackedNotificationIds.includes(id)) continue; + if (activeStatuses.has(existing.status)) continue; + delete state.sessions[id]; + } } // Merge fetched sessions, preserving local-only fields diff --git a/frontend/src/shared/state/fetchSessionsStrip.test.ts b/frontend/src/shared/state/fetchSessionsStrip.test.ts new file mode 100644 index 00000000..8b3235ba --- /dev/null +++ b/frontend/src/shared/state/fetchSessionsStrip.test.ts @@ -0,0 +1,57 @@ +// Run: node --test (via scripts/run-tests.mjs) +// +// The wipe chain, reproduced live 2026-08-12: one fetchSessions.fulfilled whose payload was empty +// and whose meta carried no dashboardId stripped EVERY completed session from the store; the +// reconcile effect then deleted every card, and the debounced layout save persisted the wipe. +// Card-less sessions are never promoted from disk again, so it was permanent (ENG-271). These pin +// the reducer's strip authority: scoped fetches prune their own dashboard, unscoped prune nothing. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import reducer, { fetchSessions } from './agentsSlice.ts'; + +function seeded(): any { + const mk = (id: string, dash: string, status = 'completed') => ({ + id, name: id, status, mode: 'agent', provider: 'anthropic', model: 'm', + dashboard_id: dash, messages: [], pending_approvals: [], tool_group_meta: {}, + }); + const empty = reducer(undefined, { type: '@@init' }) as any; + return { + ...empty, + sessions: { + a1: mk('a1', 'dashA'), a2: mk('a2', 'dashA'), + b1: mk('b1', 'dashB'), + run: mk('run', 'dashA', 'running'), + }, + }; +} + +function fulfilled(payload: any[], arg: any): any { + return { type: fetchSessions.fulfilled.type, payload, meta: { arg, requestId: 't' } }; +} + +test('a scoped empty answer prunes only its own dashboard', () => { + const next = reducer(seeded(), fulfilled([], { dashboardId: 'dashA' })) as any; + assert.deepEqual(Object.keys(next.sessions).sort(), ['b1', 'run'], + 'dashA completed sessions go; dashB and the running one stay'); +}); + +test('an UNSCOPED empty answer deletes nothing at all', () => { + const next = reducer(seeded(), fulfilled([], {})) as any; + assert.deepEqual(Object.keys(next.sessions).sort(), ['a1', 'a2', 'b1', 'run'], + 'no scope means no authority to strip; this exact dispatch wiped a real board'); +}); + +test('an unscoped answer still merges what it carries', () => { + const incoming = { id: 'new1', name: 'new1', status: 'completed', mode: 'agent', + provider: 'anthropic', model: 'm', dashboard_id: 'dashC', messages: [], + pending_approvals: [], tool_group_meta: {} }; + const next = reducer(seeded(), fulfilled([incoming], {})) as any; + assert.ok(next.sessions.new1, 'merge still works without scope'); + assert.equal(Object.keys(next.sessions).length, 5, 'and nothing was deleted'); +}); + +test('a rejected fetch strips nothing', () => { + const next = reducer(seeded(), { type: fetchSessions.rejected.type, error: { message: '500' }, + meta: { arg: { dashboardId: 'dashA' }, requestId: 't' } }) as any; + assert.equal(Object.keys(next.sessions).length, 4); +});