fix(chatbot): read the agent catalog in the declared envelope too (objectstack#4053) (#2992)

`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 declared
`{ success: true, data }` envelope. useAgents read only `{ agents }` and a bare
array, so the day either producer converts, the parse misses.

That miss is unusually dangerous here, 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 every user.
Nothing downstream would catch it.

extractAgentList now folds all four shapes to the same list — bare array,
`{ agents }`, `{ success: true, data: [...] }`, `{ 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. It is
exported and pure so the shapes are testable without standing up the hook.

No behaviour change against any server shipping today: the shapes that worked
before parse identically. This only removes the lockstep requirement, so the
server side can convert on its own schedule instead of having to land with a
console release — the same consumer-first ordering used for SharedRecordPage in
objectstack#3983.

Nine tests; reverting to the previous two-shape read fails five of them.


Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.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.

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]);
+  });
+});

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() });