diff --git a/.changeset/ai-agents-fallback-envelope.md b/.changeset/ai-agents-fallback-envelope.md new file mode 100644 index 0000000000..39ca9345ac --- /dev/null +++ b/.changeset/ai-agents-fallback-envelope.md @@ -0,0 +1,47 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): the `/ai/agents` degraded fallback answers in the declared envelope (#4053) + +`GET /ai/agents` was the last unenveloped SDK-addressable route. The framework's +degraded fallback — what an open-source runtime with no `service-ai` answers — +now returns `{ success: true, data: { agents: [] } }` via `deps.success`, and the +route-envelope guard's last ratchet retires with it: **0 ratcheted on both +surfaces.** + +## Why `data: { agents }` and not `data: []` + +#3983 set the precedent that `data` carries the payload directly, and following it +here would have looked consistent. It would also have been wrong, and silently so. + +`AiAgentsResponseSchema` is a **declared** payload schema; share-links' `{ links }` +was an ad-hoc wrapper with none. So this is the #3843 relocation — the declared +payload moves under `data` unchanged, the way `SettingsNamespacePayload` did — +rather than a reshape. + +That distinction decides the blast radius. `unwrapResponse` returns `body.data` +when a body has a boolean `success` **and** a `data` key, so: + +| conversion | `client.ai.agents.list()` | +|---|---| +| `data: { agents }` (this one) | reads `.agents` off it — **works** | +| `data: [...]` (flattened) | `.agents` is `undefined` → **`[]`** | + +An empty list is not a visible failure on this route. `useAiSurfaceEnabled` gates +the entire AI surface on `agents.length > 0`, and an empty catalog is the *correct* +answer for a seat-less user (ADR-0068) or a Community-Edition deployment. The +broken state and the legitimate one are indistinguishable — no error, no 403, no +log. + +## Consequence: no lockstep + +Because the SDK reads both shapes identically, **each surface converts on its own +schedule**. Cloud's `service-ai` still answers unenveloped and keeps working +unchanged; objectui already reads all four shapes (objectui#2992). The +"three repos in one batch" framing #4053 opened with does not apply to this +variant. + +Five tests in `@objectstack/client` pin it, including the road not taken: the +flattened body asserts `[]`, so the cost of choosing it is recorded rather than +rediscovered. diff --git a/packages/client/src/ai-agents-envelope.test.ts b/packages/client/src/ai-agents-envelope.test.ts new file mode 100644 index 0000000000..f1ee0ef76e --- /dev/null +++ b/packages/client/src/ai-agents-envelope.test.ts @@ -0,0 +1,83 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `client.ai.agents.list()` against both shapes of `GET /api/v1/ai/agents` (#4053). + * + * This route has two producers — the framework dispatcher's degraded fallback when + * no AI service is registered, and cloud's `service-ai` — and it is mid-migration + * onto the declared envelope. The framework side converted first; cloud's has not + * yet, so both shapes are live in the fleet and the SDK has to read either. + * + * That is only true because the conversion RELOCATES the declared payload under + * `data` (`data: { agents }`) rather than flattening it to the bare array + * (`data: [...]`). The distinction is the whole reason this file exists: + * + * • `unwrapResponse` returns `body.data` when a body has a boolean `success` + * AND a `data` key, so `list()` reads `.agents` off `{ agents }` either way. + * • Flattening would make `unwrapResponse` return the array, `.agents` read + * `undefined`, and `list()` answer `[]`. + * + * An empty list is not a visible failure here. `useAiSurfaceEnabled` gates the + * ENTIRE AI surface on `agents.length > 0`, and an empty catalog is the CORRECT + * answer for a seat-less user (ADR-0068) or a Community-Edition deployment with no + * `service-ai`. So the broken state and the legitimate one look identical — no + * error, no 403, no log — which is why the shape is pinned here rather than left + * to be noticed. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackClient } from './index'; + +function clientAnswering(body: unknown) { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => body, + headers: new Headers(), + }); + return { + client: new ObjectStackClient({ baseUrl: 'http://localhost:3000', fetch: fetchMock }), + fetchMock, + }; +} + +const ASK = { name: 'ask', label: 'Ask', role: 'assistant' }; +const BUILD = { name: 'build', label: 'Build', role: 'assistant' }; + +describe('client.ai.agents.list — both producers', () => { + it('reads the UNENVELOPED body (cloud service-ai, pre-#4053)', async () => { + const { client, fetchMock } = clientAnswering({ agents: [ASK, BUILD] }); + const agents = await client.ai.agents.list(); + expect(String(fetchMock.mock.calls[0][0])).toBe('http://localhost:3000/api/v1/ai/agents'); + expect(agents.map((a: any) => a.name)).toEqual(['ask', 'build']); + }); + + it('reads the ENVELOPED body with `data: { agents }` (the framework fallback, #4053)', async () => { + const { client } = clientAnswering({ success: true, data: { agents: [ASK, BUILD] } }); + const agents = await client.ai.agents.list(); + expect(agents.map((a: any) => a.name)).toEqual(['ask', 'build']); + }); + + it('answers identically either way — which is what lets the surfaces convert independently', async () => { + const bare = await clientAnswering({ agents: [ASK] }).client.ai.agents.list(); + const enveloped = await clientAnswering({ success: true, data: { agents: [ASK] } }).client.ai.agents.list(); + expect(enveloped).toEqual(bare); + }); + + it('an empty catalog stays empty in both shapes — a seat-less caller answers this legitimately', async () => { + expect(await clientAnswering({ agents: [] }).client.ai.agents.list()).toEqual([]); + expect(await clientAnswering({ success: true, data: { agents: [] } }).client.ai.agents.list()).toEqual([]); + }); + + it('the FLATTENED variant is why `data` keeps the `{ agents }` wrapper', async () => { + // Pinning the road not taken. #3983 set the precedent that `data` carries + // the payload directly, and following it here would look consistent — but + // `AiAgentsResponseSchema` is a DECLARED payload schema (share-links' + // `{ links }` was an ad-hoc wrapper with none), so the #3843 relocation + // reading applies instead. This asserts the cost of getting it wrong: + // a silently empty agent list, not a parse error. + const { client } = clientAnswering({ success: true, data: [ASK, BUILD] }); + expect(await client.ai.agents.list()).toEqual([]); + }); +}); diff --git a/packages/runtime/src/domain-handler-registry.test.ts b/packages/runtime/src/domain-handler-registry.test.ts index 4eb1f025a2..69d1ead4c2 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -550,7 +550,15 @@ describe('HttpDispatcher extracted domains (PR-7: auth/ai)', () => { it('/ai/agents returns an empty list (not 404) when no AI service is configured', async () => { const result = await makeDispatcher().dispatch('GET', '/ai/agents', undefined, {}, {} as any); expect(result.response?.status).toBe(200); - expect(result.response?.body?.agents).toEqual([]); + // #4053: in the declared envelope now, with `AiAgentsResponseSchema`'s + // `{ agents }` RELOCATED under `data` rather than flattened to the bare + // array. `unwrapResponse` returns `data`, so `client.ai.agents.list()` + // reads `.agents` off it and keeps working against cloud's still- + // unenveloped `service-ai` too — which is what lets the two surfaces + // convert independently instead of in lockstep. + expect(envelopeViolations(result.response?.body), JSON.stringify(result.response?.body)).toEqual([]); + expect(result.response?.body?.data?.agents).toEqual([]); + expect(result.response?.body?.agents).toBeUndefined(); }); it('/ai routes 404 (service missing) for non-agents paths', async () => { diff --git a/packages/runtime/src/domains/ai.ts b/packages/runtime/src/domains/ai.ts index e6989b0cb0..49d50e65aa 100644 --- a/packages/runtime/src/domains/ai.ts +++ b/packages/runtime/src/domains/ai.ts @@ -50,8 +50,19 @@ export async function handleAIRequest(deps: DomainHandlerDeps, subPath: string, // service-ai is a Cloud/Enterprise package) into console error-log // spam on every page. An empty list conveys the same information // without looking like a fault. Every other /ai/* route still 404s. + // + // The body is the declared envelope (#4053). `data` carries + // `AiAgentsResponseSchema`'s `{ agents }` — a RELOCATION of the declared + // payload under `data`, the way `SettingsNamespacePayload` moved in #3843, + // not a flatten to the bare array. That distinction is load-bearing here: + // `unwrapResponse` returns `data`, so `client.ai.agents.list()` reads + // `.agents` off it and keeps working, and it keeps working against cloud's + // `service-ai` too while that surface still answers unenveloped. Flattening + // to `data: []` would have made `.agents` `undefined` — an empty catalog, + // which `useAiSurfaceEnabled` turns into "hide the entire AI surface", and + // which is indistinguishable from the legitimate seat-less/CE state. if (method === 'GET' && subPath === '/ai/agents') { - return { handled: true, response: { status: 200, body: { agents: [] } } }; + return { handled: true, response: deps.success({ agents: [] }) }; } // [#3842] Was a hand-rolled envelope with the status in `code`. It has // no header or shape of its own, so it is simply the shared exit now. diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 48518416a1..7613180333 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -2603,7 +2603,14 @@ describe('HttpDispatcher', () => { const agents = await dispatcher.handleAI('/ai/agents', 'GET', undefined, {}, { request: {} }); expect(agents.handled).toBe(true); expect(agents.response?.status).toBe(200); - expect(agents.response?.body).toEqual({ agents: [] }); + // #4053 enveloped this body while #4058 was in flight. The courtesy + // this test pins is unchanged — an empty list rather than a fault — + // it just travels under `data` now. `AiAgentsResponseSchema`'s + // `{ agents }` is RELOCATED, not flattened to the bare array, so + // `client.ai.agents.list()` still reads `.agents` off what + // `unwrapResponse` returns. + expect(agents.response?.body).toEqual({ success: true, data: { agents: [] }, meta: undefined }); + expect(agents.response?.body?.agents).toBeUndefined(); }); // Discovery must say exactly what the domains do — one predicate feeds diff --git a/scripts/check-route-envelope.mjs b/scripts/check-route-envelope.mjs index 2e6a373fa6..34d8421054 100644 --- a/scripts/check-route-envelope.mjs +++ b/scripts/check-route-envelope.mjs @@ -232,11 +232,11 @@ const DISPATCHER_DOMAINS = { note: "bridges better-auth, whose client parses ITS shapes (`{ user, session }`, `{ session: null, user: null }`, `{ success: true }`) — BaseResponseSchema does not govern them", }, - // Kind 4 — real drift, tracked. + // Kind 2 — the `{ agents: [] }` fallback moved onto `deps.success` in #4053; + // what remains is the passthrough of the AI service's own result. 'ai.ts': { - handBuilt: 2, - ratchet: '#4053', - note: 'one passthrough of the AI service result (kind 2); one is the unenveloped `{ agents: [] }` fallback for GET /ai/agents — an SDK-addressable route whose conversion must land with cloud and the SDK together or `ai.agents.list()` silently returns []', + handBuilt: 1, + note: "passthrough of the AI service result (status and body are the service's, streaming included)", }, };