diff --git a/.changeset/agent-catalog-envelope-tolerance.md b/.changeset/agent-catalog-envelope-tolerance.md new file mode 100644 index 0000000000..b4b1f8ffea --- /dev/null +++ b/.changeset/agent-catalog-envelope-tolerance.md @@ -0,0 +1,33 @@ +--- +'@object-ui/plugin-chatbot': patch +--- + +**Read the agent catalog in the declared envelope too, before the server converts.** + +`GET /api/v1/ai/agents` is served by two producers — the framework dispatcher's +degraded fallback when no AI service is registered, and cloud's `service-ai` — and +it is one of the last SDK-addressable routes still answering outside the platform's +declared `{ success: true, data }` envelope (objectstack#4053). `useAgents` read +only `{ agents }` and a bare array, so the day either producer converts, the parse +would miss. + +That miss is unusually dangerous on this particular route, which is why it is worth +getting ahead of rather than fixing after. The catalog is not just data: +`useAiSurfaceEnabled` gates the **entire AI surface** on `agents.length > 0`, +because the route is access-filtered per caller and is therefore the only signal +that is both edition- and user-aware (ADR-0068). An empty list is the correct +answer for a seat-less user or a Community-Edition deployment with no `service-ai` +— so a parse miss and the legitimate hidden state are **indistinguishable**: no +error, no 403, no log, just the FAB, the top-bar link and the designer's "Ask AI" +quietly gone for everyone. + +`extractAgentList` now folds all four shapes to the same list — a bare array, +`{ agents }`, `{ success: true, data: [...] }`, and `{ success: true, data: +{ agents } }` — detecting the envelope the way `ObjectStackClient.unwrapResponse` +does (a **boolean** `success`), so the two readers cannot disagree about what +counts as one. Nine tests cover it; reverting to the previous two-shape read fails +five of them. + +No behaviour change against any server shipping today: the shapes that worked +before still parse identically. This only removes the lockstep requirement, so the +server side can convert on its own schedule. diff --git a/packages/plugin-chatbot/src/__tests__/agentListShapes.test.ts b/packages/plugin-chatbot/src/__tests__/agentListShapes.test.ts new file mode 100644 index 0000000000..8bba4d9910 --- /dev/null +++ b/packages/plugin-chatbot/src/__tests__/agentListShapes.test.ts @@ -0,0 +1,96 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `GET /api/v1/ai/agents` shape tolerance (objectstack#4053). + * + * Two producers serve this route — the framework dispatcher's degraded fallback + * when no AI service is registered, and cloud's `service-ai` — and it is + * mid-migration onto the platform's declared `{ success: true, data }` envelope. + * + * Why this file exists rather than a comment: an unrecognised shape here does not + * throw, warn, or log. It yields an empty list, and `useAiSurfaceEnabled` turns an + * empty list into "hide the entire AI surface". That is ALSO the correct behaviour + * for a seat-less user (ADR-0068) or a Community-Edition deployment with no + * `service-ai` — so a parse miss looks exactly like the legitimate hidden state. + * There is no downstream signal that would catch it: no failing request, no 403, + * no visible difference. The tests are the only thing standing between an envelope + * conversion on the server and the AI UI quietly vanishing for every user. + * + * Teaching the reader all three shapes BEFORE any producer converts is what lets + * the server side move on its own schedule instead of landing in lockstep with a + * console release. + */ + +import { describe, expect, it } from 'vitest'; +import { extractAgentList } from '../useAgents'; + +const ASK = { name: 'ask', label: 'Ask' }; +const BUILD = { name: 'build', label: 'Build' }; + +describe('extractAgentList — every shape this route answers in', () => { + it('reads `{ agents }` — what both producers send today', () => { + expect(extractAgentList({ agents: [ASK, BUILD] })).toEqual([ASK, BUILD]); + }); + + it('reads a bare array', () => { + expect(extractAgentList([ASK, BUILD])).toEqual([ASK, BUILD]); + }); + + it('reads the declared envelope with `data` as the array', () => { + // The shape objectstack#3983 set the precedent for: `data` carries the + // payload directly. This is the variant that silently emptied the list. + expect(extractAgentList({ success: true, data: [ASK, BUILD] })).toEqual([ASK, BUILD]); + }); + + it('reads the declared envelope with `data: { agents }`', () => { + // The other plausible conversion — relocating the existing payload under + // `data` rather than flattening it. Both must read the same. + expect(extractAgentList({ success: true, data: { agents: [ASK, BUILD] } })).toEqual([ASK, BUILD]); + }); + + it('agrees across all four shapes — the point of one extractor', () => { + const shapes: unknown[] = [ + { agents: [ASK] }, + [ASK], + { success: true, data: [ASK] }, + { success: true, data: { agents: [ASK] } }, + ]; + for (const s of shapes) expect(extractAgentList(s)).toEqual([ASK]); + }); +}); + +describe('extractAgentList — empty is a real answer, not a fallback', () => { + it('an empty catalog stays empty in every shape', () => { + // A seat-less user gets this legitimately (ADR-0068), so it must not be + // conflated with a miss — but it must not throw either. + expect(extractAgentList({ agents: [] })).toEqual([]); + expect(extractAgentList([])).toEqual([]); + expect(extractAgentList({ success: true, data: [] })).toEqual([]); + expect(extractAgentList({ success: true, data: { agents: [] } })).toEqual([]); + }); + + it('never throws on a shape it does not recognise', () => { + // Callers gate the whole AI surface on this; a throw would surface as a + // load error rather than a hidden surface, but neither is worth a crash. + for (const junk of [null, undefined, 0, 'nope', {}, { data: null }, { agents: 'no' }]) { + expect(extractAgentList(junk)).toEqual([]); + } + }); +}); + +describe('extractAgentList — envelope detection matches unwrapResponse', () => { + it('keys on a BOOLEAN `success`, as the SDK does', () => { + // `ObjectStackClient.unwrapResponse` treats a body as an envelope iff + // `typeof body.success === 'boolean'`. Diverging from that rule is how two + // readers of one route end up disagreeing about which shape they got. + expect(extractAgentList({ success: true, data: [ASK] })).toEqual([ASK]); + expect(extractAgentList({ success: false, data: [ASK] })).toEqual([ASK]); + }); + + it('does NOT treat a non-boolean `success` as an envelope', () => { + // A payload that happens to carry a truthy `success` field is data, not an + // envelope — unwrapping it would hide the real agents. + expect(extractAgentList({ success: 'yes', agents: [ASK] })).toEqual([ASK]); + expect(extractAgentList({ success: 1, agents: [ASK] })).toEqual([ASK]); + }); +}); diff --git a/packages/plugin-chatbot/src/useAgents.ts b/packages/plugin-chatbot/src/useAgents.ts index 9583dade41..bebbe12dc1 100644 --- a/packages/plugin-chatbot/src/useAgents.ts +++ b/packages/plugin-chatbot/src/useAgents.ts @@ -149,6 +149,43 @@ export function resolveDefaultAgentName( return agents[0].name; } +/** + * Pull the agent list out of whatever `GET /api/v1/ai/agents` answered. + * + * This route is served by more than one producer and is mid-migration onto the + * platform's declared `{ success: true, data }` envelope + * (objectstack#4053), so three shapes have to read the same: + * + * [ … ] a bare array + * { agents: [ … ] } today's shape, both producers + * { success: true, data: … } the declared envelope, whose `data` + * may be the array or `{ agents }` + * + * The envelope is detected the way `ObjectStackClient.unwrapResponse` detects + * it — a **boolean** `success` — so the two agree on what counts as one. + * + * Reading the envelope BEFORE any producer emits it is deliberate, and it is the + * whole point of doing this ahead of the conversion. An unrecognised shape here + * does not throw or warn: it yields an empty list, and `useAiSurfaceEnabled` + * turns an empty list into "hide the entire AI surface". That is also the + * CORRECT behaviour for a seat-less user or a Community-Edition deployment with + * no `service-ai` — so a parse miss is indistinguishable from the legitimate + * hidden state, with no error, no 403 and no log to notice it by. Teaching the + * consumer first means the producer can convert on its own schedule instead of + * having to land in lockstep with this file. + */ +export function extractAgentList(payload: unknown): RawAgent[] { + const isEnvelope = + !!payload && typeof payload === 'object' && !Array.isArray(payload) && + typeof (payload as { success?: unknown }).success === 'boolean'; + + const body = isEnvelope ? (payload as { data?: unknown }).data : payload; + + if (Array.isArray(body)) return body as RawAgent[]; + const agents = (body as { agents?: unknown } | null | undefined)?.agents; + return Array.isArray(agents) ? (agents as RawAgent[]) : []; +} + function normalize(raw: RawAgent[]): AgentDescriptor[] { return raw .filter((a) => typeof a?.name === 'string' && a.name.length > 0) @@ -196,9 +233,8 @@ function fetchAgentsCached( }) .then(async (res) => { if (!res.ok) throw new Error(`Failed to load agents (${res.status})`); - const payload = (await res.json()) as { agents?: RawAgent[] } | RawAgent[]; - const list = Array.isArray(payload) ? payload : payload?.agents ?? []; - return normalize(list); + // Bare array, `{ agents }`, or the declared envelope — see `extractAgentList`. + return normalize(extractAgentList(await res.json())); }) .then((normalized) => { agentsCache.set(apiBase, { data: normalized, timestamp: Date.now() });