Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changeset/ai-agents-envelope-declaration.md
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 8 additions & 6 deletions packages/client/src/ai-agents-envelope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
*
Expand Down Expand Up @@ -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']);
Expand Down
10 changes: 10 additions & 0 deletions packages/client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AiAgentSummary[]> => {
const route = this.getRoute('ai');
Expand Down
6 changes: 3 additions & 3 deletions packages/runtime/src/domain-handler-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 4 additions & 3 deletions packages/runtime/src/domains/ai.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
3 changes: 2 additions & 1 deletion packages/runtime/src/http-dispatcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
119 changes: 119 additions & 0 deletions packages/spec/src/api/ai-agents-envelope.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
6 changes: 5 additions & 1 deletion packages/spec/src/api/envelope-violations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 17 additions & 1 deletion packages/spec/src/api/protocol.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 —
Expand Down
Loading