From 5cf1b2ddff513a8b19c613afb63af142f23b340c Mon Sep 17 00:00:00 2001 From: ciregenz Date: Wed, 12 Aug 2026 19:03:16 -0700 Subject: [PATCH] [eric] state: the last two array-spread reducers are guarded at both layers (ENG-277) --- .../state/listThunksRejectOnError.test.ts | 35 ++++++++++++------- frontend/src/shared/state/mcpRegistrySlice.ts | 1 + .../src/shared/state/skillRegistrySlice.ts | 3 ++ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/frontend/src/shared/state/listThunksRejectOnError.test.ts b/frontend/src/shared/state/listThunksRejectOnError.test.ts index 4ca21540..3cb089d8 100644 --- a/frontend/src/shared/state/listThunksRejectOnError.test.ts +++ b/frontend/src/shared/state/listThunksRejectOnError.test.ts @@ -19,19 +19,28 @@ import { fetchSkills } from './skillsSlice.ts'; import { fetchModes } from './modesSlice.ts'; import { fetchOutputs } from './outputsSlice.ts'; import { fetchWorkflows } from './workflowsSlice.ts'; +import { searchRegistry } from './mcpRegistrySlice.ts'; +import { searchSkillRegistry, fetchAllRegistrySkills } from './skillRegistrySlice.ts'; type Thunk = { (arg?: unknown): (d: unknown, g: unknown, e: unknown) => Promise<{ type: string; payload?: unknown }> }; -const LIST_THUNKS: Array<[string, Thunk]> = [ - ['fetchTools', fetchTools as unknown as Thunk], - ['fetchSkills', fetchSkills as unknown as Thunk], - ['fetchModes', fetchModes as unknown as Thunk], - ['fetchOutputs', fetchOutputs as unknown as Thunk], - ['fetchWorkflows', fetchWorkflows as unknown as Thunk], +// Some thunks destructure their argument, so passing undefined throws before the fetch and the +// rejection would be the harness's, not the guard's. Each entry carries the arg its thunk needs. +const LIST_THUNKS: Array<[string, Thunk, unknown]> = [ + ['fetchTools', fetchTools as unknown as Thunk, undefined], + ['fetchSkills', fetchSkills as unknown as Thunk, undefined], + ['fetchModes', fetchModes as unknown as Thunk, undefined], + ['fetchOutputs', fetchOutputs as unknown as Thunk, undefined], + ['fetchWorkflows', fetchWorkflows as unknown as Thunk, undefined], + // The last two hard-throw sites: their reducers array-spread payload.servers / payload.skills, + // so an undefined payload does not degrade, it throws inside immer. + ['searchRegistry', searchRegistry as unknown as Thunk, { q: 'x' }], + ['searchSkillRegistry', searchSkillRegistry as unknown as Thunk, { q: 'x' }], + ['fetchAllRegistrySkills', fetchAllRegistrySkills as unknown as Thunk, undefined], ]; /** Run a thunk's payload creator with fetch stubbed, and report which lifecycle action it ended on. */ -async function runWith(thunk: Thunk, response: unknown): Promise { +async function runWith(thunk: Thunk, response: unknown, arg?: unknown): Promise { const realFetch = globalThis.fetch; (globalThis as { fetch: unknown }).fetch = async () => response; try { @@ -39,8 +48,9 @@ async function runWith(thunk: Thunk, response: unknown): Promise { const getState = () => ({ tools: { loading: false }, skills: { loading: false }, modes: { loading: false }, outputs: { loading: false }, workflows: { loading: false, items: {} }, + mcpRegistry: { loading: false, servers: [] }, skillRegistry: { loading: false, skills: [] }, }); - const action = await thunk(undefined)(() => {}, getState, undefined); + const action = await thunk(arg)(() => {}, getState, undefined); return action?.type ?? 'no-action'; } finally { (globalThis as { fetch: unknown }).fetch = realFetch; @@ -53,9 +63,9 @@ const unauthorized = { json: async () => ({ detail: 'Unauthorized' }), }; -for (const [name, thunk] of LIST_THUNKS) { +for (const [name, thunk, arg] of LIST_THUNKS) { test(`${name} REJECTS on 401 instead of resolving undefined into the reducer`, async () => { - const type = await runWith(thunk, unauthorized); + const type = await runWith(thunk, unauthorized, arg); assert.ok( type.endsWith('/rejected'), `${name} ended on "${type}"; a fulfilled action here hands the reducer an undefined payload`, @@ -71,10 +81,11 @@ test('a healthy 200 still fulfils, for every list thunk', async () => { status: 200, json: async () => ({ tools: [], skills: [], modes: [], builtin_defaults: {}, outputs: [], workflows: [], + servers: [], total: 0, offset: 0, limit: 20, }), }; - for (const [name, thunk] of LIST_THUNKS) { - const type = await runWith(thunk, okBody); + for (const [name, thunk, arg] of LIST_THUNKS) { + const type = await runWith(thunk, okBody, arg); assert.ok(type.endsWith('/fulfilled'), `${name} ended on "${type}" for a good response`); } }); diff --git a/frontend/src/shared/state/mcpRegistrySlice.ts b/frontend/src/shared/state/mcpRegistrySlice.ts index 93e17304..55bc92ca 100644 --- a/frontend/src/shared/state/mcpRegistrySlice.ts +++ b/frontend/src/shared/state/mcpRegistrySlice.ts @@ -50,6 +50,7 @@ export const searchRegistry = createAsyncThunk( async ({ q, limit = 20, offset = 0, sort = 'name', source = '' }: { q: string; limit?: number; offset?: number; sort?: string; source?: string }) => { const params = new URLSearchParams({ q, limit: String(limit), offset: String(offset), sort, source }); const res = await fetch(`${MCP_REGISTRY_API}/search?${params}`); + if (!res.ok) throw new Error(`MCP registry search failed: ${res.status}`); return (await res.json()) as { servers: McpServer[]; total: number; offset: number; limit: number }; } ); diff --git a/frontend/src/shared/state/skillRegistrySlice.ts b/frontend/src/shared/state/skillRegistrySlice.ts index 8bdde524..019ee999 100644 --- a/frontend/src/shared/state/skillRegistrySlice.ts +++ b/frontend/src/shared/state/skillRegistrySlice.ts @@ -45,6 +45,7 @@ export const searchSkillRegistry = createAsyncThunk( async ({ q, limit = 20, offset = 0, sort = 'name', category = '' }: { q: string; limit?: number; offset?: number; sort?: string; category?: string }) => { const params = new URLSearchParams({ q, limit: String(limit), offset: String(offset), sort, category }); const res = await fetch(`${SKILL_REGISTRY_API}/search?${params}`); + if (!res.ok) throw new Error(`Skill registry search failed: ${res.status}`); return (await res.json()) as { skills: RegistrySkill[]; total: number; offset: number; limit: number }; }, ); @@ -59,6 +60,7 @@ export const fetchAllRegistrySkills = createAsyncThunk( async () => { const params = new URLSearchParams({ q: '', limit: '100', offset: '0', sort: 'name', category: '' }); const res = await fetch(`${SKILL_REGISTRY_API}/search?${params}`); + if (!res.ok) throw new Error(`Skill registry fetchAll failed: ${res.status}`); return (await res.json()) as { skills: RegistrySkill[]; total: number; offset: number; limit: number }; }, ); @@ -143,6 +145,7 @@ const skillRegistrySlice = createSlice({ }) .addCase(searchSkillRegistry.fulfilled, (state, action) => { state.loading = false; + if (!action.payload || !Array.isArray(action.payload.skills)) return; if (action.meta.arg.offset && action.meta.arg.offset > 0) { state.skills = [...state.skills, ...action.payload.skills]; } else {