diff --git a/.changeset/ai-agents-envelope-declaration.md b/.changeset/ai-agents-envelope-declaration.md new file mode 100644 index 0000000000..42d4a80bc2 --- /dev/null +++ b/.changeset/ai-agents-envelope-declaration.md @@ -0,0 +1,4 @@ +--- +--- + +test(spec,client,runtime): relocate `AiAgentsResponseSchema` onto the `data` payload it now describes, and pin `GET /ai/agents` against its declaration (#4053). Both producers of the route — the framework's degraded fallback (#4124) and cloud's `service-ai` (cloud#929) — converted onto the declared envelope; this closes out the declaration half and corrects four comments that still described the migration as in flight. Comments, a doc-block and one new test file; releases nothing. diff --git a/packages/client/src/ai-agents-envelope.test.ts b/packages/client/src/ai-agents-envelope.test.ts index f1ee0ef76e..f5b7e1d5d9 100644 --- a/packages/client/src/ai-agents-envelope.test.ts +++ b/packages/client/src/ai-agents-envelope.test.ts @@ -4,11 +4,13 @@ * `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. + * no AI service is registered, and cloud's `service-ai`. Both answer in the declared + * envelope now: the framework side converted first (#4124) and cloud followed on its + * own schedule (cloud#929). The unenveloped shape is still read here because an SDK + * outlives the server it is pointed at — a current client talking to a deployment + * from before the conversion is ordinary, not hypothetical. * - * That is only true because the conversion RELOCATES the declared payload under + * That independence was only possible because the conversion RELOCATES the payload under * `data` (`data: { agents }`) rather than flattening it to the bare array * (`data: [...]`). The distinction is the whole reason this file exists: * @@ -46,14 +48,14 @@ 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 () => { + it('reads the UNENVELOPED body — a pre-#4053 server this client still has to talk to', 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 () => { + it('reads the ENVELOPED body with `data: { agents }` — what both producers serve (#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']); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 6cd168540c..29b139e26a 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -3798,6 +3798,16 @@ export class ObjectStackClient { * Agents the CALLER may chat with — the route filters by the caller's * permissions (ADR-0049), so an empty list is a legitimate answer for a * seat-less user rather than an error to retry. + * + * The `.agents` read below survived #4053 unchanged, and that is the + * conclusion rather than an oversight. `AiAgentsResponseSchema` moved + * under the envelope's `data` as a RELOCATION (#3843's precedent), so + * `unwrapResponse` hands back `{ agents }` from an enveloped producer and + * the same object from a pre-conversion one. Had the route flattened to + * `data: [...]` (#3983's precedent) this line would read `undefined` and + * answer `[]` — an empty catalog, which is what the console reads as "this + * caller has no AI", so the regression would have been invisible. + * `ai-agents-envelope.test.ts` pins both readings. */ list: async (): Promise => { const route = this.getRoute('ai'); diff --git a/packages/runtime/src/domain-handler-registry.test.ts b/packages/runtime/src/domain-handler-registry.test.ts index 47120dcd17..c8b996744b 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -698,9 +698,9 @@ describe('HttpDispatcher extracted domains (PR-7: auth/ai)', () => { // #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. + // reads `.agents` off it — which is what let this surface convert on its + // own and cloud's `service-ai`, the route's other producer, follow + // independently instead of in lockstep. Both answer this now (cloud#929). expect(envelopeViolations(result.response?.body), JSON.stringify(result.response?.body)).toEqual([]); expect(result.response?.body?.data?.agents).toEqual([]); expect(result.response?.body?.agents).toBeUndefined(); diff --git a/packages/runtime/src/domains/ai.ts b/packages/runtime/src/domains/ai.ts index d7b2f566d4..1d2f6b15e3 100644 --- a/packages/runtime/src/domains/ai.ts +++ b/packages/runtime/src/domains/ai.ts @@ -57,9 +57,10 @@ export async function handleAIRequest(deps: DomainHandlerDeps, subPath: string, // 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, + // `.agents` off it. It is also what let this side convert first and cloud's + // `service-ai` — the route's OTHER producer — follow independently rather + // than in lockstep; both answer this shape now (cloud#929). 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') { diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 3ba4fec20a..ecd5f2fda5 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -2785,7 +2785,8 @@ describe('HttpDispatcher', () => { // 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. + // `unwrapResponse` returns. Cloud's `service-ai` — the route's other + // producer — answers the same shape (cloud#929). expect(agents.response?.body).toEqual({ success: true, data: { agents: [] }, meta: undefined }); expect(agents.response?.body?.agents).toBeUndefined(); }); diff --git a/packages/spec/src/api/ai-agents-envelope.test.ts b/packages/spec/src/api/ai-agents-envelope.test.ts new file mode 100644 index 0000000000..062710e857 --- /dev/null +++ b/packages/spec/src/api/ai-agents-envelope.test.ts @@ -0,0 +1,119 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `GET /api/v1/ai/agents` — the declaration tied to the wire (#4053). + * + * The route has TWO producers in two repos: the framework dispatcher's degraded + * fallback when no AI service is registered (`runtime/src/domains/ai.ts`, the + * open-source default — `service-ai` is Cloud/EE) and cloud's `service-ai` + * `buildAgentRoutes`. Both now answer in the declared envelope, with + * `AiAgentsResponseSchema` RELOCATED under `data` rather than flattened to the + * bare array (framework#4124, cloud#929). + * + * Each producer pins its own body in its own repo. Neither repo can pin the + * OTHER one, and `scripts/check-route-envelope.mjs` reaches neither — it scans + * route modules that write via `res.json(...)`, while the dispatcher domains + * return `{ status, body }` for a central sender (#4049) and cloud is outside + * the framework guard entirely. What both producers DO share is this package, + * which is where the payload is declared. So the agreement is pinned here, once, + * against the declaration itself. + * + * Why bother, when nothing is broken today: the failure this route has is + * invisible. `useAiSurfaceEnabled` gates the ENTIRE AI surface on + * `agents.length > 0`, deliberately — the route is access-filtered per caller, + * making it the only signal that is both edition- and user-aware (ADR-0068). So + * an empty catalog is a CORRECT answer: + * + * seat-less user → AI surface hidden (correct) + * Community Edition, no service-ai → AI surface hidden (correct) + * a producer flattening `data` → AI surface hidden (the bug) + * + * No error, no 403, no failed request, no log. There is no downstream signal that + * would catch a producer drifting, which is why the shape is asserted rather than + * described. + */ + +import { describe, expect, it } from 'vitest'; +import { envelopeViolations } from './contract.zod'; +import { AiAgentsResponseSchema } from './protocol.zod'; + +const ASK = { + name: 'ask', label: 'Ask', role: 'assistant', + capabilities: { authoring: false, canvas: false, debug: false, resume: false }, +}; +const BUILD = { + name: 'build', label: 'Build', role: 'authoring', + capabilities: { authoring: true, canvas: true, debug: true, resume: true }, +}; + +/** Both halves of the contract: a conformant envelope AND a conformant `data`. */ +function servesTheDeclaredShape(body: unknown): boolean { + if (envelopeViolations(body).length > 0) return false; + return AiAgentsResponseSchema.safeParse((body as { data?: unknown }).data).success; +} + +describe('GET /ai/agents — the body both producers now answer with', () => { + it('the populated catalog: envelope-conformant, and `data` IS the declared payload', () => { + const body = { success: true, data: { agents: [ASK, BUILD] } }; + expect(envelopeViolations(body)).toEqual([]); + expect(AiAgentsResponseSchema.safeParse(body.data).success).toBe(true); + }); + + it('the empty catalog — the framework fallback, and a seat-less caller', () => { + // Two different reasons for the same body, both correct. The fallback emits + // exactly this when no AI service is registered; `service-ai` emits it for a + // caller whose permission set reaches no agent (ADR-0049). + const body = { success: true, data: { agents: [] } }; + expect(envelopeViolations(body)).toEqual([]); + expect(AiAgentsResponseSchema.safeParse(body.data).success).toBe(true); + }); + + it('`meta` beside the payload is fine — `deps.success` builds it', () => { + expect(servesTheDeclaredShape({ success: true, data: { agents: [] }, meta: undefined })).toBe(true); + expect(servesTheDeclaredShape({ success: true, data: { agents: [] }, meta: { timestamp: 't' } })).toBe(true); + }); +}); + +describe('GET /ai/agents — the shapes a producer must NOT drift back into', () => { + it('the FLATTENED conversion — the one that empties the agent list', () => { + // #3983 set the precedent that `data` carries the payload directly, and + // following it here would look consistent. It is the wrong reading: + // `AiAgentsResponseSchema` is a DECLARED payload schema, where share-links' + // `{ links }` was an ad-hoc wrapper with none — so this is the #3843 + // relocation, not a reshape. + // + // Note WHICH check catches it. The envelope is fine; it is the payload that + // no longer matches its declaration. A suite leading with `envelopeViolations` + // alone would pass this body, which is exactly how it would ship. + const body = { success: true, data: [ASK, BUILD] }; + expect(envelopeViolations(body)).toEqual([]); + expect(AiAgentsResponseSchema.safeParse(body.data).success).toBe(false); + expect(servesTheDeclaredShape(body)).toBe(false); + }); + + it('the pre-#4053 bare body — no flag for `unwrapResponse` to key on', () => { + const body = { agents: [ASK] }; + expect(servesTheDeclaredShape(body)).toBe(false); + expect(envelopeViolations(body)).toContain('success is missing, must be a boolean'); + }); + + it('the duplicate-payload shim — `agents` kept alive beside `data`', () => { + // The drift #4038 / #4049 removed from `/share-links`: the payload under + // both spellings, so no consumer ever has to choose and neither dialect + // dies. Here the mirrored key catches it; the payload itself is valid. + const body = { success: true, data: { agents: [ASK] }, agents: [ASK] }; + expect(envelopeViolations(body)).toEqual([ + 'stray top-level key `agents` — the payload belongs under `data`', + ]); + expect(servesTheDeclaredShape(body)).toBe(false); + }); + + it('an agent row without `capabilities` — the field UI affordances branch on', () => { + // cloud#816 / ADR-0057 "B+": hosts render debug drawer, Live Canvas and + // resume-vs-fresh BY CAPABILITY. A row missing it is not a row this route + // returns, so it fails the payload half even inside a clean envelope. + const body = { success: true, data: { agents: [{ name: 'build', label: 'Build', role: 'authoring' }] } }; + expect(envelopeViolations(body)).toEqual([]); + expect(servesTheDeclaredShape(body)).toBe(false); + }); +}); diff --git a/packages/spec/src/api/envelope-violations.test.ts b/packages/spec/src/api/envelope-violations.test.ts index 345820efe9..888c9e761e 100644 --- a/packages/spec/src/api/envelope-violations.test.ts +++ b/packages/spec/src/api/envelope-violations.test.ts @@ -88,7 +88,11 @@ describe('envelopeViolations — what the schema MISSES', () => { }); it('a payload left at the top level beside the flag', () => { - // The shape `GET /ai/agents` still answers in, minus the flag (#4053). + // The shim `GET /ai/agents` would have grown had it kept `agents` alive + // beside `data` through the conversion. It did not — both producers moved + // the payload rather than mirroring it (#4053) — and this is the check that + // would have said so. See `ai-agents-envelope.test.ts` for that route's own + // pins, including the half this predicate deliberately does not cover. const body = { success: true, data: [], agents: [] }; expect(parses(body)).toBe(true); expect(conformant(body)).toBe(false); diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index 4556ec7def..f5b1f70b7b 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -1069,7 +1069,23 @@ export const AiAgentSummarySchema = lazySchema(() => z.object({ })); /** - * `GET /api/v1/ai/agents`. + * `GET /api/v1/ai/agents` — the **`data` payload**, not the whole body. + * + * The body is the platform envelope (`BaseResponseSchema`), and this schema is + * what travels under `data`: `{ success: true, data: { agents: [...] } }`. It + * was declared as the whole body until #4053 converted the route, and moving it + * under `data` was a RELOCATION — the payload is unchanged, exactly as + * `SettingsNamespacePayload` moved in #3843, not a flatten to the bare array the + * way #3983 reshaped `/share-links`. + * + * That distinction is load-bearing, which is why it is recorded on the + * declaration rather than only at the two producers. `unwrapResponse` returns + * `data`, so `client.ai.agents.list()` reads `.agents` off this object; flattening + * to `data: [...]` would make that read `undefined` → an empty catalog, which + * `useAiSurfaceEnabled` turns into "hide the entire AI surface" — the same thing a + * seat-less caller and a Community-Edition deployment correctly see, with no + * error, no 403 and no log to tell them apart. `ai-agents-envelope.test.ts` pins + * both halves. * * ACCESS-AWARE: the route returns only agents the CALLER may chat with * (ADR-0049), so an empty list is a legitimate answer for a seat-less user —