mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-08-17 18:25:42 +02:00
[eric] agents: a garbled dashboard file can no longer wipe the board; the sessions list fails loud instead of answering empty, and an unscoped fetch loses its delete authority
(cherry picked from commit 0b0a5d78ca)
This commit is contained in:
@@ -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]:
|
||||
|
||||
@@ -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")
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user