From cc6dcb01e34b02da8ecb06773e1fd20c50cd0b48 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 17:00:03 +0000 Subject: [PATCH 1/2] =?UTF-8?q?test(spec,client,runtime):=20both=20produce?= =?UTF-8?q?rs=20of=20/ai/agents=20are=20enveloped=20now=20=E2=80=94=20relo?= =?UTF-8?q?cate=20the=20declaration=20and=20pin=20it=20(#4053)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4053 asked for three steps plus one relocation. The three landed: the framework's degraded fallback (#4124), cloud's `service-ai` (cloud#929), and the SDK read — which needed no change, and that is the finding rather than an omission. What was left is the relocation the issue named, and a set of records that all still say the conversion is half done. ## The declaration follows the wire `AiAgentsResponseSchema` was declared as the whole `GET /api/v1/ai/agents` body. Since the conversion it is what travels under `data`, so it now says so — the same move `SettingsNamespacePayload` made in #3843, and the reason this was a RELOCATION rather than #3983's flatten is recorded on the declaration instead of only at the two producers that implement it. ## One pin, in the package both producers share `packages/spec/src/api/ai-agents-envelope.test.ts` asserts BOTH halves of the contract — `envelopeViolations` clean AND `data` parsing as the declared payload — with the drifts pinned as negatives: the flattened conversion, the pre-#4053 bare body, an `agents` key mirrored beside `data`, and a row with no `capabilities`. Note which check catches the flatten. The envelope is *fine*; it is the payload that stops matching its declaration. A suite leading with `envelopeViolations` alone passes that body, which is exactly how it would ship. This lives in spec because nothing else can hold it. Each producer pins its own body in its own repo and neither can pin the other's; `scripts/check-route-envelope.mjs` reaches neither — 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. ## Records that had gone stale Four comments still described cloud as unenveloped and the migration as in flight. On this route that is not cosmetic: the whole reason #4053 exists is that a wrong belief about the shape produces no error, no 403 and no log — just a hidden AI surface, which is also what a seat-less caller and a Community Edition deployment correctly see. A reader who believes the servers are bare is one step from "clean this up". Verified: spec 7174 passed (277 files, 7 new), client 205, runtime 956; `check:route-envelope` 0 ratcheted on both surfaces, self-test included; `check:role-word`, `check:nul-bytes` clean; spec and client tsc clean. No behavior change, so no changeset. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01BZaiYNM5igzGk6JiibjShb --- .../client/src/ai-agents-envelope.test.ts | 14 ++- packages/client/src/index.ts | 10 ++ .../src/domain-handler-registry.test.ts | 6 +- packages/runtime/src/domains/ai.ts | 7 +- packages/runtime/src/http-dispatcher.test.ts | 3 +- .../spec/src/api/ai-agents-envelope.test.ts | 119 ++++++++++++++++++ .../spec/src/api/envelope-violations.test.ts | 6 +- packages/spec/src/api/protocol.zod.ts | 18 ++- 8 files changed, 168 insertions(+), 15 deletions(-) create mode 100644 packages/spec/src/api/ai-agents-envelope.test.ts 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 a4136063fe..240171914c 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -694,9 +694,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 49d50e65aa..2daa72fc1d 100644 --- a/packages/runtime/src/domains/ai.ts +++ b/packages/runtime/src/domains/ai.ts @@ -56,9 +56,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 8fa6cf2c4b..dac8d0e5da 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -2770,7 +2770,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 — From 83126e2b30d567b899134fc427ddd83d27ca7596 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 17:02:37 +0000 Subject: [PATCH 2/2] =?UTF-8?q?chore(changeset):=20empty=20changeset=20?= =?UTF-8?q?=E2=80=94=20this=20PR=20releases=20nothing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Check Changeset gate takes an empty-frontmatter changeset as the sanctioned "releases nothing" declaration, on par with the skip-changeset label. The diff is comments, a doc-block relocation and one new test file. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01BZaiYNM5igzGk6JiibjShb --- .changeset/ai-agents-envelope-declaration.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changeset/ai-agents-envelope-declaration.md 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.