Skip to content

fix(runtime): the /ai/agents degraded fallback answers in the declared envelope (#4053) - #4124

Merged
os-zhuang merged 6 commits into
mainfrom
claude/envelope-drift-route-modules-v2zoky
Jul 30, 2026
Merged

fix(runtime): the /ai/agents degraded fallback answers in the declared envelope (#4053)#4124
os-zhuang merged 6 commits into
mainfrom
claude/envelope-drift-route-modules-v2zoky

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

The framework half of #4053, and the guard's last ratchet retires — 0 ratcheted on both surfaces.

✓ Route modules      — 8 audited: 7 conformant, 0 ratcheted, 1 exempt
✓ Dispatcher domains — 16 audited: 11 helper-only, 5 declared, 0 ratcheted

The finding: this did not need a three-repo batch

#4053 opened by saying the conversion "must land with cloud and the SDK together or ai.agents.list() silently returns []". Checking the SDK properly showed that is true of one of the two possible conversions, not both:

private async unwrapResponse<T>(res: Response): Promise<T> {
  const body = await res.json();
  if (body && typeof body.success === 'boolean' && 'data' in body) return body.data as T;
  return body as T;
}
conversion what unwrapResponse returns list() does body?.agents
data: { agents } (this PR) { agents: [...] } ✅ works
data: [...] (flattened) [...] undefined[]

So with the relocation variant the SDK needs no change, and each surface converts independently. Cloud's service-ai still answers unenveloped and keeps working unchanged; objectui already reads all four shapes (objectui#2992).

Why relocation is the right reading, not just the convenient one

#3983 set the precedent that data carries the payload directly, and following it here would have looked consistent. The distinction:

  • AiAgentsResponseSchema is a declared payload schema.
  • share-links' { links } was an ad-hoc wrapper with no schema behind it.

So this is the #3843 relocation — a declared payload moving under data unchanged, exactly as SettingsNamespacePayload did — rather than a reshape. The convenient answer and the correct one coincide, which is worth stating explicitly so the next person doesn't read it as precedent-breaking.

Why the wrong choice would have been invisible

useAiSurfaceEnabled gates the entire AI surface on agents.length > 0, deliberately: the route is access-filtered per caller, so it is the only signal that is both edition- and user-aware (ADR-0068). An empty catalog is therefore a correct answer:

what the user sees
seat-less user (correct) AI surface hidden
CE deployment, no service-ai (correct) AI surface hidden
flattened conversion (bug) AI surface hidden

No error, no 403, no failed request, no log. That is why the road not taken is pinned as a test rather than left as a comment: { success: true, data: [ASK, BUILD] } asserts [], so the cost is recorded.

Changes

  • runtime/src/domains/ai.ts — the fallback goes through deps.success
  • scripts/check-route-envelope.mjsai.ts drops to handBuilt: 1 (the remaining one is the AI-service passthrough) and loses its ratchet
  • packages/client/src/ai-agents-envelope.test.ts — 5 new tests over both live shapes
  • the dispatcher's own fallback test now asserts the envelope and that no top-level agents survives

Verification

gate result
@objectstack/runtime 914 passed
@objectstack/client 205 passed (5 new)
@objectstack/rest · @objectstack/spec 505 · 6987
check:route-envelope + self-test 0 ratcheted, both surfaces
all ten TypeScript Type Check gates clean
six check:* guards clean

Stale-dist note again, third time this session and the same shape: check:api-surface first reported - TemporalCase, - TemporalRow … "5 breaking" — none of them mine, and I added no exports in this PR. Restarting the branch onto a newer main leaves packages/spec/dist behind it; rebuilding spec clears it. Recording it because the message names other people's exports as breaking changes, which reads as something far worse than it is.

What remains on #4053

Cloud's service-ai (routes/agent-routes.ts:352{ status: 200, body: { agents } }). Genuinely optional now rather than blocking: it works as-is against both consumers, and converting it is a one-line change whenever that repo is being touched anyway. I've left #4053 open with that reduced to its actual scope.


Generated by Claude Code

…d envelope (#4053)

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Jul 30, 2026 11:43am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Jul 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime.

18 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via packages/runtime)
  • content/docs/api/index.mdx (via @objectstack/runtime)
  • content/docs/api/wire-format.mdx (via @objectstack/runtime)
  • content/docs/automation/hook-bodies.mdx (via @objectstack/runtime)
  • content/docs/concepts/north-star.mdx (via packages/runtime)
  • content/docs/data-modeling/drivers.mdx (via @objectstack/runtime)
  • content/docs/deployment/index.mdx (via @objectstack/runtime)
  • content/docs/deployment/production-readiness.mdx (via @objectstack/runtime)
  • content/docs/deployment/single-project-mode.mdx (via @objectstack/runtime)
  • content/docs/deployment/vercel.mdx (via @objectstack/runtime)
  • content/docs/getting-started/your-first-project.mdx (via @objectstack/runtime)
  • content/docs/permissions/authentication.mdx (via @objectstack/runtime)
  • content/docs/permissions/authorization.mdx (via packages/runtime)
  • content/docs/plugins/packages.mdx (via @objectstack/runtime)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/runtime)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/runtime)
  • content/docs/protocol/kernel/lifecycle.mdx (via @objectstack/runtime)
  • content/docs/releases/implementation-status.mdx (via @objectstack/runtime)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

claude added 3 commits July 30, 2026 10:37
…ped body

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z
claude added 2 commits July 30, 2026 11:12
…moved

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` @ 857a6cf 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z
…-route-modules-v2zoky

# Conflicts:
#	scripts/check-wildcard-fallthrough.mjs

Copy link
Copy Markdown
Contributor Author

Conflict resolved — my wildcard-ledger fix is withdrawn in favour of #4133's, which supersedes it.

While this PR was open, 554ff923f ("the auth wildcard yields paths the auth service does not own, and unbreak the #4116 ledger", #4117/#4133) landed and fixed the same file. We had both removed the stale ${prefix}/storage/* entry, so the two collided on that line.

Theirs is strictly better and I took it wholesale (git checkout --theirs, now byte-identical to main):

So scripts/check-wildcard-fallthrough.mjs no longer appears in this PR's diff at all. What remains is only the six files this PR is actually about:

.changeset/ai-agents-fallback-envelope.md
packages/client/src/ai-agents-envelope.test.ts
packages/runtime/src/domain-handler-registry.test.ts
packages/runtime/src/domains/ai.ts
packages/runtime/src/http-dispatcher.test.ts
scripts/check-route-envelope.mjs

Worth noting the mechanism, since it is the third such collision on this branch: main moved seven commits during the merge, and the only textual conflict was the one place two people fixed the same defect. The two earlier ones (#4058's /ai/agents assertion, #4112's deletion of domains/storage.ts from my own guard table) merged cleanly and would have shipped broken — CI caught one, and #4112's author correctly updated my table for the other.

Re-verified after the merge

Applied all three staleness lessons up front this time rather than rediscovering them: pnpm install --frozen-lockfile, full rebuild, and cleared packages/runtime/.objectstack + .cache/.

full suite (turbo run test, Test Core's command) 129/129 packages
ESLint job — all 11 checks clean
TypeScript Type Check job — all 10 gates clean

Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review July 30, 2026 12:38
@os-zhuang
os-zhuang merged commit 6fa1827 into main Jul 30, 2026
17 checks passed
@os-zhuang
os-zhuang deleted the claude/envelope-drift-route-modules-v2zoky branch July 30, 2026 12:38
os-zhuang added a commit that referenced this pull request Jul 31, 2026
… now — relocate the declaration and pin it (#4053) (#4238)

Closes out #4053. Both producers converted (#4124, cloud#929); the SDK
needed no change because the conversion RELOCATED the declared payload
under `data` rather than flattening it.

- spec: `AiAgentsResponseSchema` now describes the `data` payload, not the
  whole body — the #3843 relocation, recorded on the declaration itself.
- spec: new `ai-agents-envelope.test.ts` pins both halves of the contract
  (envelope conformance AND `data` matching the declared payload), with the
  flatten, the bare body, the mirrored `agents` key and a capabilities-less
  row as negatives. It lives here because neither producer's repo can pin
  the other's and `check-route-envelope.mjs` reaches neither.
- runtime/client: four comments that still described cloud as unenveloped
  and the migration as in flight.

Empty changeset — releases nothing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants