[eric] state: a wrong-shaped API answer changes nothing instead of throwing inside immer (ENG-277)

frontend/src/shared/focusGuestForKeys.test.ts

@@ -8,11 +8,22 @@ import { test } from 'node:test';
 import assert from 'node:assert/strict';
 import { focusGuestForKeys, guestKeyTakeoverCount, resetGuestKeyTakeoverCount } from './focusGuestForKeys.ts';
 
-function fakeWebview(): { focus: () => void; focused: number } {
-  const wv = { focused: 0, focus(): void { wv.focused += 1; } };
-  return wv as { focus: () => void; focused: number };
+function fakeWebview(): { focus: (o?: FocusOptions) => void; focused: number; lastOpts?: FocusOptions } {
+  const wv = {
+    focused: 0,
+    lastOpts: undefined as FocusOptions | undefined,
+    focus(o?: FocusOptions): void { wv.focused += 1; wv.lastOpts = o; },
+  };
+  return wv;
 }
 
+// A docked browser parks off-canvas, and focus() is allowed to scroll its target into view.
+test('the guest is focused without scrolling it into view', () => {
+  const wv = fakeWebview();
+  withActive(userInput, () => focusGuestForKeys(wv as never));
+  assert.equal(wv.lastOpts?.preventScroll, true, 'a bare focus() can drag a parked card into view');
+});
+
 function withActive(el: unknown, fn: () => void): void {
   const doc = globalThis.document as unknown as { activeElement: unknown };
   const prev = doc.activeElement;

frontend/src/shared/focusGuestForKeys.ts

@@ -33,7 +33,10 @@ export function focusGuestForKeys(wv: BrowserWebview): void {
   const before = typeof document !== 'undefined' ? document.activeElement : null;
   if (before !== (wv as unknown as Element) && p_isUserTextSurface(before)) p_takeovers += 1;
   try {
-    wv.focus();
+    // A docked browser is parked far off-canvas, and a bare focus() is allowed to scroll its target
+    // into view. Measured harmless at left:-100000 (negative clamps to 0), but not for a card parked
+    // the other way, and refusing the scroll costs nothing: keys still land in a parked guest.
+    wv.focus({ preventScroll: true });
   } catch {
     // The card unmounted mid-command. The keystroke will miss, which beats it hitting the user.
   }

frontend/src/shared/state/agentsSlice.ts

@@ -1215,6 +1215,10 @@ const agentsSlice = createSlice({
       })
       .addCase(fetchSessions.fulfilled, (state, action) => {
         state.loading = false;
+        // A wrong-shaped answer must change nothing. Coercing it to [] is precisely the board wipe
+        // (ENG-271); throwing here dies inside immer's produce, which is the "payload is not
+        // iterable" crash. Doing nothing is the only option that is safe in both directions.
+        if (!Array.isArray(action.payload)) return;
         const fetchedIds = new Set(action.payload.map((s) => s.id));
         const activeStatuses = new Set(['running', 'waiting_approval']);
 
@@ -1430,6 +1434,7 @@ const agentsSlice = createSlice({
         state.trackedNotificationIds = state.trackedNotificationIds.filter((id) => id !== sessionId);
       })
       .addCase(fetchHistory.fulfilled, (state, action) => {
+        if (!Array.isArray(action.payload)) return;
         const history: Record<string, HistorySession> = {};
         for (const s of action.payload) {
           history[s.id] = s;

frontend/src/shared/state/badPayloadIsHarmless.test.ts

@@ -0,0 +1,74 @@
+// Run: node --test (via frontend/scripts/run-tests.mjs)
+//
+// ENG-277. A failed request used to reach these reducers as an undefined payload and they iterated
+// it, which threw INSIDE immer's produce: "t.payload is not iterable" / "Cannot read properties of
+// undefined (reading 'map')", live in a real console.
+//
+// The fix is deliberately "change nothing", not "treat it as empty". Empty is the dangerous reading:
+// it is exactly how one bad answer wiped a whole dashboard layout (ENG-271). So each test asserts
+// BOTH that it does not throw AND that the good data already in the store survived.
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import agents, { fetchSessions, fetchHistory } from './agentsSlice.ts';
+import dashboards, { fetchDashboards } from './dashboardsSlice.ts';
+
+const BAD: unknown[] = [undefined, null, {}, 'nope', 42];
+
+function agentsWithOneSession(): any {
+  const s = agents(undefined, { type: '@@init' }) as any;
+  return agents(s, {
+    type: fetchSessions.fulfilled.type,
+    payload: [{ id: 'keep-me', name: 'Keep me', status: 'completed', messages: [] }],
+    meta: { arg: { dashboardId: 'dash-1' } },
+  }) as any;
+}
+
+test('a session list that is not a list changes nothing and does not throw', () => {
+  const before = agentsWithOneSession();
+  assert.ok(before.sessions['keep-me'], 'fixture never armed');
+  for (const bad of BAD) {
+    const after = agents(before, {
+      type: fetchSessions.fulfilled.type, payload: bad, meta: { arg: { dashboardId: 'dash-1' } },
+    }) as any;
+    assert.ok(after.sessions['keep-me'], `payload ${JSON.stringify(bad)} lost a real session`);
+  }
+});
+
+test('a bad history payload keeps the history it already had', () => {
+  let s = agents(undefined, { type: '@@init' }) as any;
+  s = agents(s, { type: fetchHistory.fulfilled.type, payload: [{ id: 'h1', name: 'Old chat' }] }) as any;
+  assert.ok(s.history.h1, 'fixture never armed');
+  for (const bad of BAD) {
+    s = agents(s, { type: fetchHistory.fulfilled.type, payload: bad }) as any;
+    assert.ok(s.history.h1, `payload ${JSON.stringify(bad)} wiped history`);
+  }
+});
+
+test('a bad dashboard list does not read as "you have no dashboards"', () => {
+  let s = dashboards(undefined, { type: '@@init' }) as any;
+  s = dashboards(s, { type: fetchDashboards.fulfilled.type, payload: [{ id: 'd1', name: 'Board' }] }) as any;
+  assert.ok(s.items.d1, 'fixture never armed');
+  for (const bad of BAD) {
+    s = dashboards(s, { type: fetchDashboards.fulfilled.type, payload: bad }) as any;
+    assert.ok(s.items.d1, `payload ${JSON.stringify(bad)} emptied the dashboard list`);
+  }
+});
+
+// The negative half: a genuinely good answer must still be applied, or "it changes nothing" would
+// pass on a reducer that had been broken into doing nothing at all.
+test('a real payload still applies', () => {
+  const s = agents(agentsWithOneSession(), {
+    type: fetchSessions.fulfilled.type,
+    payload: [{ id: 'keep-me', name: 'Keep me', status: 'completed', messages: [] },
+      { id: 'new-one', name: 'New', status: 'running', messages: [] }],
+    meta: { arg: { dashboardId: 'dash-1' } },
+  }) as any;
+  assert.ok(s.sessions['new-one'], 'a valid payload stopped being applied');
+});
+
+test('a real dashboard list still replaces the old one', () => {
+  let s = dashboards(undefined, { type: '@@init' }) as any;
+  s = dashboards(s, { type: fetchDashboards.fulfilled.type, payload: [{ id: 'd1', name: 'Board' }] }) as any;
+  s = dashboards(s, { type: fetchDashboards.fulfilled.type, payload: [{ id: 'd2', name: 'Other' }] }) as any;
+  assert.ok(s.items.d2 && !s.items.d1, 'a valid dashboard list no longer replaces');
+});

frontend/src/shared/state/dashboardsSlice.ts

@@ -116,6 +116,9 @@ const dashboardsSlice = createSlice({
       })
       .addCase(fetchDashboards.fulfilled, (state, action) => {
         state.loading = false;
+        // This replaces the whole list, so a wrong-shaped answer must not be read as "you have no
+        // dashboards" any more than it should throw inside immer. Keep what we had.
+        if (!Array.isArray(action.payload)) return;
         const items: Record<string, Dashboard> = {};
         for (const d of action.payload) {
           items[d.id] = d;

frontend/src/shared/state/workflowsSlice.ts

@@ -641,6 +641,7 @@ const slice = createSlice({
         delete state.runControlPending[action.meta.arg.runId];
       })
       .addCase(fetchRuns.fulfilled, (state, action) => {
+        if (!action.payload || !Array.isArray(action.payload.runs)) return;
         state.runs[action.payload.id] = action.payload.runs;
         for (const r of action.payload.runs) {
           const pending = state.runControlPending[r.id];