Skip to content

Commit e5a4d26

Browse files
authored
feat(plugin-hono-server)!: delete the CRUD/discovery surface — the plugin is a transport adapter (#4073) (#4296)
Completes the retirement. HonoServerPlugin now owns the socket, the middleware and the three current-user endpoints, and nothing else. The data and discovery APIs have one owner each — @objectstack/rest and the runtime dispatcher (ADR-0076 D11). Net −678 lines. Deleted: raw C+R `/api/v1/data/:object` (+ `/:id`), this plugin's third `/api/v1/discovery` + `/.well-known/objectstack` payload, and the `registerStandardEndpoints` flag itself — gone, not defaulted off, with a test pinning that passing the removed option resurrects nothing. Unaffected: `/auth/me/*` + `/me/apps` (sole supply, unconditional since #4144), and every composed host — `os serve`, `objectstack dev`, cloud's objectos — which mount an owner that already answered these routes byte-identically either way (#4260). A host with NO owner keeps the boot warn, reworded to name both remedies now that opting in is not one of them. Tests decided on what each was actually pinning: `hono-discovery` and `hono-anonymous-deny` deleted (wholly about the retired routes); `server-timing-e2e` KEPT and repointed at `/auth/me/permissions` (#3361's value is "perf middleware + a handler that resolves the principal", not the route); `client.hono` KEPT and repointed at a REST-composed host (it is the only coverage of `client.connect()` and the discovery handshake); the default-pin suite rewritten as `hono-transport-only`. Three gates caught bookkeeping the deletion owed, each fixed on its own terms: - the #2567 anonymous-deny ledger flagged STALE covers. The row went with its surface, and its id came out of HIGH_RISK — but the ratchet's own negative test used that row as its FIXTURE, so it was repointed at a live row rather than left to pass vacuously and disarm the check that just caught this. - the docs drift check surfaced `authorization.mdx`, which documented the deleted surface twice as part of a SECURITY invariant. Both now read as history, matching how the same sentence already handles the retired GraphQL endpoint: the surface LEFT the matrix; the invariant is unchanged and simply has one fewer door to hold shut. - a merge conflict with #4282's CORS work: both intents kept. AGENTS.md gains the four rules this thread paid for — one route one owner, explicit composition over default magic, absence must be loud, machine-readable surfaces must not lie — plus the verification note that "who serves this path" is a question about the composed, PROVISIONED runtime. #4073 was answered wrongly three times, once per shortcut around that. Verified: plugin-hono-server 135/135, dogfood 430 passed, client 12/12, runtime 19/19, http-conformance 46/46, full build 71/71, CI 16/16. (Temporal Conformance timed out once on a live-service step touching no package in this diff; it passed on re-run.)
1 parent d4f4f36 commit e5a4d26

23 files changed

Lines changed: 351 additions & 1021 deletions
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/plugin-hono-server": major
3+
---
4+
5+
feat(plugin-hono-server)!: delete the CRUD/discovery convenience surface and the `registerStandardEndpoints` flag — the plugin is a transport adapter (#4073)
6+
7+
Completes the retirement. `HonoServerPlugin` now owns the socket, the middleware
8+
and the three current-user endpoints, and nothing else. The data and discovery
9+
APIs have one owner each: `@objectstack/rest` and the runtime dispatcher
10+
(ADR-0076 D11).
11+
12+
**Removed**
13+
14+
- `POST/GET /api/v1/data/:object` and `GET /api/v1/data/:object/:id` — the raw
15+
C+R surface that delegated straight to ObjectQL.
16+
- `GET /api/v1/discovery` and `GET /.well-known/objectstack` — this plugin's
17+
third discovery payload, which predated `DiscoverySchema` and could not
18+
satisfy it (no `services`, the ADR-0076 D12 source of truth).
19+
- The `registerStandardEndpoints` option. It is gone, not defaulted off: passing
20+
it is now a type error, and passing it via `as never` mounts nothing.
21+
22+
**Unaffected**
23+
24+
- `/auth/me/permissions`, `/auth/me/localization` and `/me/apps` — this plugin
25+
is the platform's only supply and they register unconditionally (#4144).
26+
- Every composed host: `os serve`, `objectstack dev`, cloud's objectos and every
27+
documented composition mount REST and/or the dispatcher, which already served
28+
these routes and answered byte-identically with the flag on or off (#4260).
29+
30+
**Migration** — only a host that mounts `HonoServerPlugin` with neither owner is
31+
affected. It now has no data or discovery API, and the boot warns once naming
32+
both remedies. Mount `createRestApiPlugin` from `@objectstack/rest` for full
33+
CRUD behind the gate stack, or `createDispatcherPlugin` from
34+
`@objectstack/runtime`. There is no flag to opt back in.
35+
36+
**Why** — the surface was duplicate and lesser supply (C+R only, a subset of the
37+
gates, a non-conforming discovery payload), and it charged rent: #2567, #3298
38+
and #4018 each had to re-implement a platform invariant on it after the fact,
39+
because a second implementation of a route is a second place every future
40+
invariant must be remembered.

AGENTS.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,47 @@ export class MyPlugin implements Plugin {
365365

366366
---
367367

368+
## Route & surface ownership
369+
370+
Four rules, each paid for by a real bug. They matter more than usual here because
371+
this repo is largely written by agents, and every one of them is a trap that
372+
reads as reasonable code.
373+
374+
**1. One route, one owner.** Never add a second implementation of a path that
375+
another package already serves, however convenient. A shadowed duplicate is code
376+
that `grep` finds and the runtime never runs — the exact input that makes an
377+
agent (or a human) reason confidently from dead code. It also silently forks
378+
every future invariant: the retired hono `/data` surface had to re-learn the
379+
anonymous-deny gate (#2567), honest batch capability reporting (#3298) and
380+
discovery accuracy (#4018), each after the fact, each because someone fixed the
381+
real owner and never knew about the copy. Retired in #4073.
382+
383+
**2. Explicit composition over default magic.** A capability that appears
384+
because of a default nobody wrote down is invisible at every call site — and
385+
call sites are the primary evidence an agent reasons from. #4073's own first
386+
analysis checked *who passes the option* and missed *who relies on the default*;
387+
the correction is in that issue's opening paragraph. If a host should get a
388+
surface, it should mount it.
389+
390+
**3. Absence must be loud.** A composition that legitimately serves nothing
391+
should say so once at boot, naming the remedy — never leave a bare 404 to be
392+
diagnosed. The same rule applies to tooling: a verifier that silently degrades
393+
(reusing a stale build, skipping a check it could not run) is worse than no
394+
verifier, because it reports success. Prefer failing to falling back.
395+
396+
**4. Machine-readable surfaces must not lie.** `/discovery` and friends are read
397+
by SDKs, codegen and AI clients. Advertise only what is actually mounted, and
398+
mount everything advertised (ADR-0076 D12) — a wrong answer here propagates into
399+
everything built on top of it.
400+
401+
**Verifying any of this:** "who serves this path" is a question about the
402+
composed, *provisioned* runtime — not about which plugin declares it, not about
403+
registration order, and not about a minimal harness that merely boots. #4073 was
404+
answered wrongly three times, once per each of those shortcuts. Boot the real
405+
composition with its real services, or do not claim an answer.
406+
407+
---
408+
368409
## Post-Task Checklist
369410

370411
1. `pnpm test` — verify nothing broke.

content/docs/permissions/authorization.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ site — the file you read when behavior surprises you.
4848

4949
| # | Gate | What it decides | Enforcement site | Failure direction |
5050
|---|---|---|---|---|
51-
| 1 | **Anonymous deny** | No identity → HTTP 401. **Uniform across every HTTP surface that reaches object data** (#2567): REST `/data`, the metadata endpoints (`/meta`), and the raw-hono standard `/data` routes (the dispatcher GraphQL endpoint left the matrix when the GraphQL surface was removed in v17 — `/graphql` now 404s) — one shared decision, so a caller denied on `/data` can't read the same rows through a sibling door. **Default-on** (ADR-0056 D2): serving the whole data plane publicly requires an explicit `api.requireAuth: false` opt-out, which logs a boot warning. Narrower public surfaces do **not** need it — each derives its own authorization from a declaration rather than from the deployment posture: control plane (`/auth`, `/health`, `/discovery`) is allow-listed; public form submission carries a `publicFormGrant` (ADR-0056 Option A); share-links validate their token then read as SYSTEM; and an anonymous **GET** of the book/doc read surface is admitted so `book.audience: 'public'` works under the secure default, with the ADR-0046 §6.7 audience gate — `'public'` only, fail-closed — doing the authorizing (#3963). | `packages/core/src/security/anonymous-deny.ts` `shouldDenyAnonymous` — called by `rest-server.ts` `enforceAuth`, the dispatcher `handleMetadata`/`handleAI`, and `plugin-hono-server` `denyAnonymous` (default in `packages/spec/src/api/rest-server.zod.ts`); a source-enumerating ratchet in `authz-conformance.test.ts` fails CI if a new surface ships ungated | fail-closed |
51+
| 1 | **Anonymous deny** | No identity → HTTP 401. **Uniform across every HTTP surface that reaches object data** (#2567): REST `/data` and the metadata endpoints (`/meta`) — the raw-hono standard `/data` routes left the matrix when that duplicate surface was deleted in v17 (#4073), and the dispatcher GraphQL endpoint left when the GraphQL surface was removed (`/graphql` now 404s) — one shared decision, so a caller denied on `/data` can't read the same rows through a sibling door. **Default-on** (ADR-0056 D2): serving the whole data plane publicly requires an explicit `api.requireAuth: false` opt-out, which logs a boot warning. Narrower public surfaces do **not** need it — each derives its own authorization from a declaration rather than from the deployment posture: control plane (`/auth`, `/health`, `/discovery`) is allow-listed; public form submission carries a `publicFormGrant` (ADR-0056 Option A); share-links validate their token then read as SYSTEM; and an anonymous **GET** of the book/doc read surface is admitted so `book.audience: 'public'` works under the secure default, with the ADR-0046 §6.7 audience gate — `'public'` only, fail-closed — doing the authorizing (#3963). | `packages/core/src/security/anonymous-deny.ts` `shouldDenyAnonymous` — called by `rest-server.ts` `enforceAuth` and the dispatcher `handleMetadata`/`handleAI` (default in `packages/spec/src/api/rest-server.zod.ts`); a source-enumerating ratchet in `authz-conformance.test.ts` fails CI if a new surface ships ungated | fail-closed |
5252
| 2 | **Public-form grant** | An anonymous form submission carries a declaration-derived `publicFormGrant` authorizing ONLY create + read-back on the form's declared target object — never anything else (ADR-0056 Option A). No guest-portal configuration needed (anonymous principals hold the `guest` position). | `packages/plugins/plugin-security/src/security-plugin.ts` (ObjectQL middleware) | scope-limited allow |
5353
| 3 | **Object CRUD** | `allowRead/Create/Edit/Delete` (+ the destructive lifecycle class `allowTransfer/Restore/Purge`, gated ahead of the M2 operations — #1883) resolved across the caller's permission sets. | `packages/plugins/plugin-security/src/permission-evaluator.ts` `checkObjectPermission` | fail-closed 403 |
5454
| 4 | **OWD / sharing** | Org-wide default (`private` / `public_read` / `public_read_write` / `controlled_by_parent`; **unset or unknown ⇒ `private`, fail-closed** — ADR-0090 D1) plus the external dial (`externalSharingModel`, ADR-0090 D11), manual record shares, criteria sharing rules (owner-type rules were removed from the authoring surface in v17 rather than left declared-but-skipped — [Sharing Rules](/docs/permissions/sharing-rules#recipient-types)), business-unit hierarchy widening (ADR-0057 D5: scope-depth hierarchy lives on `sys_business_unit`, not positions). | `packages/plugins/plugin-sharing/src/sharing-service.ts` + `sharing-rule-service.ts` | fail-closed to owner-only |

packages/client/src/client.batch-transaction.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ describe('data.batchTransaction (live Hono, #1604)', () => {
5959
port: 0,
6060
// Skip hardcoded hono CRUD routes so createRestApiPlugin owns
6161
// route registration (including the root /batch route).
62-
registerStandardEndpoints: false,
6362
}),
6463
);
6564

packages/client/src/client.environment-scoping.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ describe('Project-scoped REST routing (live Hono)', () => {
4444
port: 0,
4545
// IMPORTANT: skip hardcoded hono CRUD routes so createRestApiPlugin
4646
// owns /data and /meta registration end-to-end.
47-
registerStandardEndpoints: false,
4847
});
4948
kernel.use(honoPlugin);
5049

packages/client/src/client.hono.test.ts

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { LiteKernel } from '@objectstack/core';
33
import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql';
44
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
55
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
6+
import { createRestApiPlugin } from '@objectstack/runtime';
67
import { ObjectStackClient } from './index';
78

89
describe('ObjectStackClient (with Hono Server)', () => {
@@ -25,19 +26,22 @@ describe('ObjectStackClient (with Hono Server)', () => {
2526
},
2627
} as any);
2728

28-
// 2. Setup Hono Plugin
29-
// This suite exercises the CLIENT's data operations over the raw-hono
30-
// standard endpoints; it wires no auth service. Opt out of the
31-
// secure-by-default anonymous-deny posture (#2567) so the anonymous
32-
// data CRUD under test stays reachable — the same explicit
33-
// `requireAuth: false` a deployment that intentionally serves data
34-
// publicly would set. The gate itself is proven in
35-
// plugin-hono-server/hono-anonymous-deny.test.ts.
36-
const honoPlugin = new HonoServerPlugin({
37-
port: 0,
38-
registerStandardEndpoints: true,
39-
});
29+
// 2. Setup Hono Plugin — transport only.
30+
//
31+
// This suite used to run against the raw-hono standard endpoints. That
32+
// surface is gone (#4073): the plugin serves the socket and the
33+
// current-user endpoints, and the data + discovery APIs belong to their
34+
// owner. What is under test here is the CLIENT — `connect()`, the
35+
// discovery handshake, and CRUD over HTTP — so it now runs against the
36+
// real owner, which is also what every deployment actually serves.
37+
const honoPlugin = new HonoServerPlugin({ port: 0 });
4038
kernel.use(honoPlugin);
39+
40+
// `requireAuth: false` keeps the anonymous CRUD under test reachable —
41+
// the same explicit opt-out a deployment intentionally serving public
42+
// data would set (#2567/#3963); the gate itself is proven in
43+
// @objectstack/rest's own suites.
44+
kernel.use(createRestApiPlugin({ api: { api: { requireAuth: false } as any } }));
4145

4246
// --- BROKER SHIM START ---
4347
// HttpDispatcher requires a broker to function. We inject a simple shim.
@@ -152,19 +156,14 @@ describe('ObjectStackClient (with Hono Server)', () => {
152156
// Client should have populated discovery info
153157
expect(client['discoveryInfo']).toBeDefined();
154158

155-
// The standalone hono surface advertises what it actually mounts
156-
// (#4018): /data CRUD and the /auth/me/* helpers are registered here,
157-
// so both are advertised and both answer.
159+
// Discovery is REST's, computed from its registry (#4018 D12: declared
160+
// === enforced). Every route it advertises must actually answer.
158161
const endpoints = client['discoveryInfo']!.routes;
159162
expect(endpoints.data).toContain('/api/v1/data');
160-
expect(endpoints.auth).toContain('/api/v1/auth');
161-
162-
// `metadata` is NOT advertised on this boot, and that is the point of
163-
// #4018: no plugin here mounts /api/v1/meta (it ships with
164-
// @objectstack/rest / the dispatcher), so the old hardcoded table was
165-
// promising a route that 404s. Proof the omission is honest:
166-
expect(endpoints.metadata).toBeUndefined();
167-
expect((await fetch(`${baseUrl}/api/v1/meta/objects`)).status).toBe(404);
163+
expect(endpoints.metadata).toContain('/api/v1/meta');
164+
165+
// Enforced, not just declared — the pairing #4018 exists to hold.
166+
expect((await fetch(`${baseUrl}/api/v1/meta/objects`)).status).not.toBe(404);
168167
});
169168

170169
it('should create and retrieve data via hono', async () => {

packages/plugins/plugin-hono-server/src/current-user-endpoints.ts

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@
1313
* `/auth/me/localization` for regional defaults, and `core`'s auth gate
1414
* allow-lists `/me/apps` + `/me/localization` as endpoints a gated user MUST
1515
* still reach to bootstrap the remediation UI. #4079 lifted them out from under
16-
* `registerStandardEndpoints` (which covers only DUPLICATE supply — raw `/data`
17-
* CRUD and a discovery the dispatcher/REST own) so the flag could not take the
18-
* console down with it.
16+
* `registerStandardEndpoints`, which also gated a DUPLICATE `/data` CRUD +
17+
* discovery surface, so that flag could not take the console down with it. That
18+
* surface and the flag have since been deleted (#4073); these three are all the
19+
* plugin serves beyond the socket.
1920
*
2021
* That split fixed the flag but left the supply still welded to
2122
* {@link HonoServerPlugin}: a host that stands up a bare {@link HonoHttpServer}
@@ -405,13 +406,14 @@ export function annotateEffectiveApiOperations(
405406
}
406407

407408
/**
408-
* Build the session → `ExecutionContext` resolver the current-user endpoints
409-
* and the plugin's standalone `/data` CRUD surface — both need.
409+
* Build the session → `ExecutionContext` resolver the current-user endpoints
410+
* need.
410411
*
411-
* Extracted from `registerDiscoveryAndCrudEndpoints` when the current-user
412-
* endpoints stopped being gated on `registerStandardEndpoints` (#4073): they
413-
* resolve the same principal the `/data` routes do, and one resolver is the
414-
* only way the two groups can agree on who the caller is.
412+
* Extracted from `registerDiscoveryAndCrudEndpoints` when these endpoints
413+
* stopped being gated on `registerStandardEndpoints` (#4073), so the two groups
414+
* would agree on who the caller is. That surface has since been deleted and
415+
* these endpoints are the only remaining caller — kept as a named export
416+
* because the serverless host path (cloud#924) composes it directly.
415417
*/
416418
export function makeExecutionContextResolver(ctx: CurrentUserEndpointsContext) {
417419
const getObjectQL = () => ctx.getService<IDataEngine>('objectql');
@@ -600,21 +602,19 @@ export function makeExecutionContextResolver(ctx: CurrentUserEndpointsContext) {
600602
* Register the current-user endpoints — `/auth/me/permissions`,
601603
* `/auth/me/localization` and `/me/apps` — on `rawApp`.
602604
*
603-
* When {@link HonoServerPlugin} drives this, it is UNCONDITIONAL, unlike the
604-
* plugin's CRUD + discovery block (#4073). Those two groups used to ride on one
605-
* `registerStandardEndpoints` flag, which conflated two opposite things. The flag
606-
* covers DUPLICATE supply — raw `/data` CRUD that `@objectstack/rest` also serves
607-
* (and, being registered first, really serves), plus a discovery that the
608-
* dispatcher/REST own (#4018). These three are the opposite: nothing else in the
609-
* platform mounts them. `packages/rest` and `packages/runtime` register no
610-
* `/me/*` route at all, the console reads `/auth/me/permissions` for its whole
611-
* permission layer and `/auth/me/localization` for regional defaults, and
605+
* When {@link HonoServerPlugin} drives this, it is UNCONDITIONAL — and these are
606+
* now the only routes it mounts beyond the socket itself.
607+
*
608+
* They used to ride on the `registerStandardEndpoints` flag alongside a raw
609+
* `/data` CRUD + discovery surface, one flag over two opposite things (#4073).
610+
* That surface was DUPLICATE supply and has been deleted; these three are the
611+
* opposite and could not be. Nothing else in the platform mounts them:
612+
* `packages/rest` and `packages/runtime` register no `/me/*` route at all, the
613+
* console reads `/auth/me/permissions` for its whole permission layer and
614+
* `/auth/me/localization` for regional defaults, and
612615
* `core/security/auth-gate.ts` allow-lists `/me/apps` + `/me/localization` as
613-
* endpoints a gated user MUST still reach to bootstrap the remediation UI.
614-
* `os serve` gets them only because `registerStandardEndpoints` defaults to true
615-
* (`cli/src/commands/serve.ts` passes just `{ port }`), so turning that flag off
616-
* — or retiring the convenience surface it names — would have taken the console
617-
* down with it.
616+
* endpoints a gated user MUST still reach to bootstrap the remediation UI. The
617+
* split (#4144) had to land before the deletion for exactly that reason.
618618
*
619619
* IDEMPOTENT: returns `false` and registers nothing when all three paths are
620620
* already served. That is what lets a host call this eagerly on its own raw app

0 commit comments

Comments
 (0)