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
28 changes: 28 additions & 0 deletions .changeset/standard-endpoints-parity-correction.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
"@objectstack/runtime": patch
---

test(runtime): correct the #4073 evidence — the `registerStandardEndpoints` flip IS a no-op for a composed host

#4192 added a test concluding that turning the flag off makes `/api/v1/data/:object`
a 404, and blocked the #4073 retirement on it. That conclusion was wrong.

It mounted `createRestApiPlugin({})` against a STUB `objectql` service. REST
generates CRUD from the object registry, so it needs a real engine — driver plus
registered objects — and its own `api.api` config. Under-provisioned it serves
nothing, which says nothing about REST.

Provisioned the way `client.environment-scoping.test.ts` does it (that suite runs
`registerStandardEndpoints: false` and asserts `GET /api/v1/data/task` → 200 from
REST), `/data/:object`, `/discovery` and `/.well-known/objectstack` all return
byte-identical responses with the flag on and off.

The test now asserts that parity directly rather than a status code, because a
status was what misled it: `/data/task` answers 404 `OBJECT_NOT_FOUND` here — the
engine's answer, i.e. a route that WORKS — where a routing miss would be
`{"error":"Not found"}`. A separate assertion pins that the compared routes are
live, so parity cannot be satisfied by two identical misses.

No production code changes. The default is untouched: flipping it is still a real
change for a BARE host mounting neither REST nor the dispatcher, and that is an
API decision, not one this test makes.
186 changes: 108 additions & 78 deletions packages/runtime/src/standard-endpoints-precedence.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,71 +4,90 @@
* #4073 — is `registerStandardEndpoints` really all DUPLICATE supply?
*
* The retirement plan (flip the default to `false` → observe a release → delete
* `registerDiscoveryAndCrudEndpoints`) rests on that premise: every path the
* flag mounts is also served, more completely, by `@objectstack/rest` or the
* runtime dispatcher, so flipping the default changes nothing a caller can see.
* `registerDiscoveryAndCrudEndpoints`) rests on that premise. This file is the
* evidence, and it CORRECTS the first version of itself, which got the `/data`
* half wrong.
*
* The premise is not uniform across the surface — the two halves reach it by
* different mechanisms, and only one of them holds:
* That version mounted `createRestApiPlugin({})` against a STUB `objectql`
* service, saw `/api/v1/data/:object` 404 with the flag off, and concluded the
* flip removes the route. The 404 was real; the conclusion was not. REST
* generates its CRUD from the object registry, so it needs a real engine — a
* driver and at least one registered object — plus its own `api.api` config.
* Under-provision it and it serves nothing, which says nothing about REST and
* everything about the harness.
*
* - **`/discovery` + `/.well-known/objectstack` — ceded explicitly (#4018).**
* `registerDiscoveryEndpoints` checks `kernel.hasPlugin(rest|dispatcher)` and
* declines to register at all. That is order-independent, so it holds however
* Hono resolves precedence. Verified below against the REAL dispatcher.
* `packages/client/src/client.environment-scoping.test.ts` had the answer the
* whole time: it boots `registerStandardEndpoints: false` and asserts
* `GET /api/v1/data/task` → 200, served by REST, with a comment saying that is
* the point ("skip hardcoded hono CRUD routes so createRestApiPlugin owns /data
* ... end-to-end").
*
* - **`/data/:object` — no such check.** Its shadowing is asserted purely on
* "REST registers first and wins". Booted for real, that does not hold: with
* the flag off the path is a 404, with REST mounted or not. The flag's raw
* surface is the ONLY thing answering `/api/v1/data/:object` in every
* composition this harness can boot.
* Reproducing that provisioning, the answer is unambiguous: `/data/:object`,
* `/discovery` and `/.well-known/objectstack` return BYTE-IDENTICAL responses
* with the flag on and off. The surface is duplicate supply on both halves, and
* the flip is a no-op for a host that mounts REST or the dispatcher — which is
* every composed deployment, `os serve` included.
*
* So the flip is NOT the no-op the plan describes, and this file exists to keep
* it from being made on the assumption. If you are here because you are about to
* flip the default: first make `rest=true, flag=OFF` serve `/data/:object`, then
* update these expectations in the same commit.
* What that does NOT license: flipping the default is still a real change for a
* BARE host that mounts neither, which is the composition the flag was written
* for. That is a deliberate API decision, not something this file decides.
*
* Caveat, stated so the next reader does not over-read this: REST is mounted
* here with a minimal service set (`objectql` + `auth`). A fully provisioned
* `os serve` also has metadata/protocol services, and REST may mount `/data`
* only once those resolve. What is proven is narrower than "REST never serves
* `/data`" — it is "nothing in these compositions serves it once the flag is
* off", which is enough to block a default flip that assumes otherwise.
* The lesson #4073 has now hit three times: "who serves this path" is a question
* about the composed, PROVISIONED runtime. Not about which plugin declares it
* (the issue's first correction), not about registration order (the second), and
* not about a minimal harness that merely boots (this one).
*/

import { describe, it, expect, afterEach } from 'vitest';
import { LiteKernel } from '@objectstack/core';
import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql';
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
import { createRestApiPlugin } from '@objectstack/rest';
import { createDispatcherPlugin } from './dispatcher-plugin.js';

function stubServices() {
function authStub() {
return {
name: 'com.objectstack.test.standard-endpoints-precedence',
version: '1.0.0',
init: async (ctx: any) => {
ctx.registerService('objectql', {
async find() { return [{ id: 'r1' }]; },
async findOne() { return { id: 'r1' }; },
async insert(_o: string, d: any) { return d; },
});
metadata: { name: 'test-auth', version: '1.0.0' },
async init(ctx: any) {
ctx.registerService('auth', {
async getSession() { return { user: { id: 'u1' }, session: {} }; },
api: { getSession: async () => ({ user: { id: 'test-user' } }) },
});
},
};
} as any;
}

async function boot(registerStandardEndpoints: boolean, withRest: boolean) {
/**
* `serve.ts` order — Hono (line 1173) long before REST (1872) and the dispatcher
* (1883) — with REST provisioned the way a real deployment provisions it.
*/
async function boot(registerStandardEndpoints: boolean) {
const kernel = new LiteKernel();
kernel.use(stubServices());
// `serve.ts` order, real plugins: HonoServerPlugin (line 1173) long before
// REST (line 1872) and the dispatcher (line 1883).
kernel.use(new ObjectQLPlugin());
kernel.use(authStub());
kernel.use(new HonoServerPlugin({ port: 0, registerStandardEndpoints, cors: false }));
if (withRest) {
const { createRestApiPlugin } = await import('@objectstack/rest');
kernel.use(createRestApiPlugin({} as any));
}
kernel.use(createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false, requireAuth: false }));
kernel.use(
createRestApiPlugin({
// Routing probe with no auth stack mounted — same opt-out the client
// suites use (ADR-0056 D2).
api: { api: { requireAuth: false } as any },
}),
);
kernel.use(createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }));
await kernel.bootstrap();

// After bootstrap, mirroring the client suites: the engine service only
// exists once plugins have initialised, and REST's `/data/:object` is a
// generic route rather than one emitted per object, so registering the
// object afterwards is enough to make the read resolve.
const ql = kernel.getService<ObjectQL>('objectql');
ql.registerDriver(new SqliteWasmDriver({ filename: ':memory:' }) as never, true);
ql.registerObject({
name: 'task',
label: 'Task',
fields: { name: { type: 'text', label: 'Name' } },
} as never);

const server = kernel.getService<any>('http.server');
return { kernel, baseUrl: `http://127.0.0.1:${server.getPort()}` };
}
Expand All @@ -79,45 +98,56 @@ afterEach(async () => {
kernel = undefined;
});

describe('#4073 — registerStandardEndpoints is not uniformly duplicate supply', () => {
for (const withRest of [true, false]) {
const label = withRest ? 'REST + dispatcher' : 'dispatcher only';
/** Status + body for a path, so two flag positions can be compared exactly. */
async function probe(baseUrl: string, path: string) {
const res = await fetch(`${baseUrl}${path}`);
return { status: res.status, body: (await res.text()).slice(0, 400) };
}

it(`${label}: with the flag ON, /data/:object is served and gated`, async () => {
const booted = await boot(true, withRest);
kernel = booted.kernel;
const res = await fetch(`${booted.baseUrl}/api/v1/data/account`);
// 401 — the anonymous-deny gate (#2567) fired, which proves the route
// is MOUNTED. A 404 would mean nothing is serving it.
expect(res.status, 'flag ON must mount /data/:object').toBe(401);
}, 30_000);
describe('#4073 — registerStandardEndpoints against a PROVISIONED REST', () => {
/**
* The retirement question stated exactly: does turning the flag off change
* what a caller sees? Compare the two positions rather than asserting a
* status, because a status alone is what misled the earlier version of this
* file — `/data/task` answers 404 `OBJECT_NOT_FOUND` here (the object is
* registered after bootstrap, so the handler reaches the engine and the
* ENGINE says no), which is a route that WORKS. A routing 404 would be
* `{"error":"Not found"}`. Same-response-in-both-positions is the property
* the flip actually needs, and it does not depend on that distinction.
*/
for (const path of ['/api/v1/data/task?top=5', '/api/v1/discovery', '/.well-known/objectstack']) {
it(`${path} answers identically with the flag ON and OFF`, async () => {
const on = await boot(true);
kernel = on.kernel;
const withFlag = await probe(on.baseUrl, path);
await on.kernel.shutdown();

const off = await boot(false);
kernel = off.kernel;
const withoutFlag = await probe(off.baseUrl, path);

it(`${label}: with the flag OFF, /data/:object 404s — nothing else picks it up`, async () => {
const booted = await boot(false, withRest);
kernel = booted.kernel;
const res = await fetch(`${booted.baseUrl}/api/v1/data/account`);
expect(
res.status,
'if this is no longer 404, another plugin now serves /data — re-read #4073, the flip may be safe',
).toBe(404);
}, 30_000);
withoutFlag,
`${path} differs between flag positions — the #4073 flip is NOT a no-op for this path`,
).toEqual(withFlag);
}, 60_000);
}

/**
* The half of the surface that IS safe to retire: discovery cedes by an
* explicit `hasPlugin` check, so the dispatcher's computed payload answers
* whether the flag is on or off.
* ...and the routes are live, not uniformly absent — otherwise the parity
* above would be satisfied by two identical 404s.
*/
for (const flag of [true, false]) {
it(`discovery is the dispatcher's either way — flag ${flag ? 'ON' : 'OFF'} (#4018 cede)`, async () => {
const booted = await boot(flag, false);
kernel = booted.kernel;
const res = await fetch(`${booted.baseUrl}/api/v1/discovery`);
expect(res.status).toBe(200);
const body = await res.json();
// `data.routes` is the dispatcher's shape; the flag's own payload is
// a flat `{ version, apiName, routes, capabilities }`.
expect(body?.data?.routes, 'the dispatcher must own discovery').toBeTruthy();
}, 30_000);
}
it('the compared routes are actually mounted', async () => {
const booted = await boot(false);
kernel = booted.kernel;

const data = await probe(booted.baseUrl, '/api/v1/data/task?top=5');
// Reached the engine: OBJECT_NOT_FOUND is the engine's answer, not Hono's
// unmatched-route 404.
expect(data.body, '/data/:object must reach the engine').toContain('OBJECT_NOT_FOUND');

const discovery = await probe(booted.baseUrl, '/api/v1/discovery');
expect(discovery.status, '/discovery must be served').toBe(200);
expect(discovery.body).toContain('"routes"');
}, 30_000);
});
Loading