From 4aa9e67a83d9b271abc4d347c5f2104263bb22f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 10:29:37 +0000 Subject: [PATCH 1/3] fix(runtime): the /ai/agents degraded fallback answers in the declared envelope (#4053) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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, silently. `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, as SettingsNamespacePayload did — not a reshape. That decides the blast radius. `unwrapResponse` returns `body.data` when a body has a boolean `success` AND a `data` key, so `data: { agents }` keeps `client.ai.agents.list()` reading `.agents` off it, while `data: [...]` would make `.agents` undefined and the method answer `[]`. 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. Broken and legitimate look identical — 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 — which is the finding, not a shortcut around it. Five tests in @objectstack/client pin both shapes, including the road not taken: the flattened body asserts [], so the cost of choosing it is recorded rather than rediscovered. The dispatcher's own fallback test now asserts the envelope and that no `agents` key survives at the top level. runtime 914, client 205, rest 505, spec 6987. All ten typecheck-job gates and the six check scripts pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z --- .changeset/ai-agents-fallback-envelope.md | 47 +++++++++++ .../client/src/ai-agents-envelope.test.ts | 83 +++++++++++++++++++ .../src/domain-handler-registry.test.ts | 10 ++- packages/runtime/src/domains/ai.ts | 13 ++- scripts/check-route-envelope.mjs | 8 +- 5 files changed, 155 insertions(+), 6 deletions(-) create mode 100644 .changeset/ai-agents-fallback-envelope.md create mode 100644 packages/client/src/ai-agents-envelope.test.ts 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 0798c50983..1edc7f3c4a 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -534,7 +534,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 674061b5fa..ea926b30a3 100644 --- a/packages/runtime/src/domains/ai.ts +++ b/packages/runtime/src/domains/ai.ts @@ -43,8 +43,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/scripts/check-route-envelope.mjs b/scripts/check-route-envelope.mjs index 711c28d211..9ae450a9c2 100644 --- a/scripts/check-route-envelope.mjs +++ b/scripts/check-route-envelope.mjs @@ -229,11 +229,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)", }, }; From 2bb7c4df3c853b0ae333571decdbe79997d8a4de Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 10:52:18 +0000 Subject: [PATCH 2/3] fix(runtime): re-point #4058's new /ai/agents assertion at the enveloped body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Semantic conflict, not a textual one. #4058 step 2 (#4086) landed on main while this branch was open and added `http-dispatcher.test.ts`'s "a stub slot 404s per route and keeps the /ai/agents empty list", which asserts the body is exactly `{ agents: [] }` — the shape #4053 enveloped one commit earlier. The two merge cleanly and the test fails, which is why CI caught it and the merge did not. The courtesy that test pins is unchanged: a stub-occupied AI slot still answers an empty list rather than a fault. The list just travels under `data` now, and the assertion says so — plus that no `agents` key survives at the top level, so a revert to the bare body fails here too. Also swept the repo for any other reader of the old shape. Three hits, all correct: the two negative assertions above, and `client/src/index.ts`'s `body?.agents` — which is exactly what the relocation variant keeps working. Merged main in rather than rebasing, per the branch's history so far. Verified against the full suite the way Test Core runs it — `turbo run test --filter='!@objectstack/dogfood'`: 129/129 packages, runtime 924. Two worktree-staleness traps on the way, both reading as code failures: - `plugin-auth` could not resolve `hono` — the merge moved the lockfile and I had not reinstalled. `pnpm install --frozen-lockfile` fixed it. - `runtime` failed under turbo but passed standalone — the gitignored `.objectstack/data/memory-driver.json` accumulating rows again. Neither is CI-visible; CI installs clean and checks out fresh. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z --- packages/runtime/src/http-dispatcher.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 75bb234dff..a2d6aef383 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -2523,7 +2523,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 From 15fbf81ac0a182b4f2bac75bbf48040f6d72acbc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 11:12:40 +0000 Subject: [PATCH 3/3] fix(tooling): drop the wildcard ledger's entry for the mount #4112 removed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NOT part of this branch's work — `main` is red on it and every open PR fails the same check, so this unblocks rather than waits. `origin/main` @ 857a6cf9a fails `check:wildcard-fallthrough` on its own, verified in a detached worktree with no branch involved: ✗ packages/adapters/hono/src/index.ts:all `${prefix}/storage/*` DECLARED but not found by the scan. Semantic conflict between two commits that both landed, not a bug in the guard. #4112 retired that mount outright — the adapter now carries a comment where `app.all(prefix + '/storage/*')` was, because the handler spoke a storage contract that does not exist and the wildcard claimed the whole `/storage` subtree. #4116's MOUNTS ledger (#4122) was written against a tree that still had it and landed after the removal, so the entry was stale on arrival. Git merged both cleanly because they touch different lines; the guard is what noticed, which is it working on day one. Dropping the entry is the whole fix — there is nothing left to ratchet when the mount it names does not exist. Its `${prefix}/auth/*` sibling is untouched and still correctly ratcheted to #4117. After: 6 yielding / 1 ratcheted / 5 exempt (12 namespace-claiming mounts). Flagged on #4122 as well, so whoever owns it can take it back if they would rather land it themselves. Also enumerated the ESLint job properly this time — it runs ELEVEN checks, not the six I had been running (`doc-authoring`, `authz-resolver`, `release-notes` and `node-version` were the ones I was missing, plus `pnpm lint` itself). All eleven pass, and `pnpm lint` is clean once `.cache/` — the objectui console build artifact an earlier `.objectui-sha` bump left in this worktree — is removed. Those 61 "errors" were entirely that directory; CI checks out fresh and never had them. Full suite: 129/129 packages. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z --- scripts/check-wildcard-fallthrough.mjs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/check-wildcard-fallthrough.mjs b/scripts/check-wildcard-fallthrough.mjs index 0a37024dfd..cd98866447 100644 --- a/scripts/check-wildcard-fallthrough.mjs +++ b/scripts/check-wildcard-fallthrough.mjs @@ -155,9 +155,12 @@ const MOUNTS = { 'packages/adapters/hono/src/index.ts:all `${prefix}/auth/*`': { ratchet: '#4117 — terminal, same shape as #4088; adapter has no in-repo consumer', }, - 'packages/adapters/hono/src/index.ts:all `${prefix}/storage/*`': { - ratchet: '#4117 — terminal, same shape as #4088; adapter has no in-repo consumer', - }, + // The `${prefix}/storage/*` sibling of the above is gone: #4112 retired that + // mount outright (the handler spoke a storage contract that does not exist, + // and the wildcard claimed the whole `/storage` subtree). #4116's ledger was + // written against a tree that still had it and landed after the removal, so + // this entry was stale on arrival and main went red on it. Nothing to ratchet + // — the mount it named does not exist. }; /** HTTP-verb registrars plus `use`; anything that can claim a path pattern. */