fix(service-datasource): forward ?schema= on the admin remote-tables route (#7955)

.changeset/remote-tables-twins-forward-schema.md

@@ -0,0 +1,37 @@
+---
+"@objectstack/service-datasource": patch
+---
+
+fix(service-datasource): the admin `remote-tables` route honours `?schema=` instead of dropping it (#7955)
+
+`IExternalDatasourceService.listRemoteTables` is reachable through two live
+routes, and only one of them read the query:
+
+- `GET /api/v1/datasources/:name/external/tables` (federation, `packages/rest`)
+ forwards `?schema=` to the service.
+- `GET /api/v1/datasources/:name/remote-tables` (admin, this package) never
+ touched `req.query`, so `?schema=public` came back as the UNFILTERED listing —
+ not the filtered set, and not a refusal either.
+
+Wire-visible change, on the admin spelling only: `?schema=<name>` now narrows the
+listing to that remote schema, exactly as the federation twin already did. A
+request with no `?schema=` is unchanged — it still returns the full listing, so
+every existing caller (none of which can have been passing the parameter
+meaningfully) sees the same bytes as before.
+
+The coercion is copied from the federation route rather than reinvented, down to
+its treatment of a non-string: a repeated `?schema=a&schema=b` reaches the
+handler as an array and both spellings drop it to "no filter". No refusal, no
+warning, no deprecation is added here — whether an unusable query parameter
+should be REFUSED is the global ingress-policy question tracked by #7606, and
+honouring the parameter is correct under either answer it reaches, so the twins
+can move together then.
+
+This finishes on the REQUEST path what #4249 did for the failure path ("one
+operation, one failure contract now, on both paths"). The equivalence is pinned
+across the two packages by
+`packages/rest/src/remote-tables-twin.equivalence.test.ts`, which drives the same
+query at BOTH spellings against one service and compares the answers — a test
+that exercised only the fixed route could not fail if the twins drift apart
+again. `@objectstack/rest` gains two dev-only workspace dependencies so that test
+can mount both registrars; its published surface is unchanged.

packages/rest/package.json

@@ -35,8 +35,10 @@
  "@objectstack/metadata": "workspace:*",
  "@objectstack/metadata-protocol": "workspace:*",
  "@objectstack/objectql": "workspace:*",
+ "@objectstack/plugin-hono-server": "workspace:*",
  "@objectstack/plugin-security": "workspace:*",
  "@objectstack/service-analytics": "workspace:*",
+ "@objectstack/service-datasource": "workspace:*",
  "@types/node": "^26.1.2",
  "typescript": "^6.0.3",
  "vitest": "^4.1.10"

packages/rest/src/remote-tables-twin.equivalence.test.ts

@@ -0,0 +1,218 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * `listRemoteTables` REQUEST-shape equivalence across its two live spellings
+ * (#7955).
+ *
+ * `IExternalDatasourceService.listRemoteTables` is reachable through two
+ * mounted routes, in two packages:
+ *
+ * GET /api/v1/datasources/:name/external/tables ← this package, federation
+ * GET /api/v1/datasources/:name/remote-tables ← @objectstack/service-datasource, admin
+ *
+ * They are not near-duplicates that happen to look alike: both resolve the SAME
+ * `external-datasource` service slot and call the SAME method with the same
+ * datasource name. #4249 already reconciled what happens when that one call
+ * THROWS ("One operation, one failure contract now, on both paths",
+ * `external-datasource-routes.ts`). What was never compared is the other half —
+ * what the two paths do with the REQUEST — and #7955 is the residue: the admin
+ * spelling never read `req.query`, so `?schema=public` came back as the
+ * UNFILTERED listing there and the filtered one here. Not an error, not the
+ * filter: the quietest form of "declared ≠ enforced".
+ *
+ * ## Why this test is here, and why it drives BOTH routes
+ *
+ * A test that exercised only the fixed route could not fail if the twins drift
+ * apart again — it would pin one path's behaviour and say nothing about the
+ * relationship, which IS the invariant. So every case below issues the same
+ * query to both spellings and asserts the two answers are equal, against ONE
+ * service instance mounted on ONE server: the difference between the readings
+ * can then only come from the two handlers.
+ *
+ * It lives in `packages/rest` because that is the side that can reach both
+ * halves without widening anyone's public API: `registerDatasourceAdminRoutes`
+ * is exported from `@objectstack/service-datasource`'s index, while
+ * `registerExternalDatasourceRoutes` is deliberately internal here (it is
+ * composed by `rest-api-plugin.ts`, not published). The dependency is dev-only
+ * and points rest → service-datasource, which is not a cycle: that package
+ * depends on `core`/`spec`/`types` and never on this one. Same reasoning the
+ * client-side ledger guard used when it chose its side (`service-route-ledger-
+ * coverage.test.ts`, "a service→client package edge would be backwards").
+ *
+ * The service under the routes is the REAL `ExternalDatasourceService` over a
+ * fake introspector, not a `vi.fn()` recording its arguments. A mock would only
+ * prove the admin handler now passes an options bag; what the card asks is that
+ * the two routes return the same SET, which takes the real filter.
+ *
+ * Driven through the real `HonoHttpServer` — the adapter `os serve` mounts — so
+ * `?schema=` is parsed by the code that parses it in production. That matters
+ * for the last case: a repeated key reaches a handler as an ARRAY, and only a
+ * real adapter produces one.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { HonoHttpServer } from '@objectstack/plugin-hono-server';
+import {
+ ExternalDatasourceService,
+ registerDatasourceAdminRoutes,
+} from '@objectstack/service-datasource';
+import type { IntrospectedColumn, IntrospectedSchema } from '@objectstack/spec/contracts';
+import { registerExternalDatasourceRoutes } from './external-datasource-routes.js';
+
+const DS = 'demo_ext';
+
+/** One remote column, spelled in full so the fixture needs no cast to be an
+ * `IntrospectedSchema` — `primaryKey` and `nullable` are both required. */
+const col = (name: string, primaryKey = false): IntrospectedColumn => ({
+ name,
+ type: name === 'id' ? 'uuid' : 'text',
+ nullable: !primaryKey,
+ primaryKey,
+});
+
+/** Two remote schemas, so a `?schema=` filter has something to exclude. */
+const REMOTE: IntrospectedSchema = {
+ dialect: 'postgres',
+ introspectedAt: '2026-08-12T00:00:00.000Z',
+ tables: {
+ 'public.customers': {
+ name: 'public.customers',
+ columns: [col('id', true), col('email')],
+ indexes: [],
+ },
+ 'public.orders': {
+ name: 'public.orders',
+ columns: [col('id', true)],
+ indexes: [],
+ },
+ 'analytics.events': {
+ name: 'analytics.events',
+ columns: [col('id', true)],
+ indexes: [],
+ },
+ },
+};
+
+/**
+ * One server, one service, both registrars — the point of the fixture.
+ *
+ * `registerDatasourceAdminRoutes` also mounts the datasource-lifecycle family,
+ * whose services are absent here; those routes answer 503 and are simply never
+ * driven. The two paths under test both resolve `external-datasource`, which is
+ * the one service wired.
+ */
+function mountBoth() {
+ const service = new ExternalDatasourceService({
+ introspect: async () => REMOTE,
+ getDatasource: async (name: string) => ({ name }),
+ getObject: async () => undefined,
+ listObjects: async () => [],
+ });
+ const server = new HonoHttpServer(0);
+ const ctx = {
+ getService: (name: string) => {
+ if (name === 'external-datasource') return service;
+ throw new Error(`no service: ${name}`);
+ },
+ } as any;
+ registerExternalDatasourceRoutes(server, ctx, '/api/v1');
+ registerDatasourceAdminRoutes(server, ctx, '/api/v1');
+ return server.getRawApp();
+}
+
+/** The two wire spellings of the one operation, keyed by how they are named. */
+const SPELLING = {
+ federation: (qs: string) => `/api/v1/datasources/${DS}/external/tables${qs}`,
+ admin: (qs: string) => `/api/v1/datasources/${DS}/remote-tables${qs}`,
+} as const;
+
+interface Reading {
+ status: number;
+ tables: Array<{ schema?: string; name: string }>;
+}
+
+/** Drive one spelling and read back the table set it answers with. */
+async function read(app: any, spelling: keyof typeof SPELLING, qs: string): Promise<Reading> {
+ const res = await app.fetch(new Request(`http://local${SPELLING[spelling](qs)}`));
+ const body = (await res.json()) as { success: boolean; data?: { tables?: Reading['tables'] } };
+ return { status: res.status, tables: body.data?.tables ?? [] };
+}
+
+/** Both spellings, same query — the comparison every case makes. */
+async function readBoth(qs: string): Promise<{ federation: Reading; admin: Reading }> {
+ const app = mountBoth();
+ return {
+ federation: await read(app, 'federation', qs),
+ admin: await read(app, 'admin', qs),
+ };
+}
+
+const qualified = (r: Reading) => r.tables.map((t) => `${t.schema}.${t.name}`).sort();
+
+describe('listRemoteTables twins agree on the request shape (#7955)', () => {
+ it('?schema= returns the SAME filtered set on both spellings', async () => {
+ const { federation, admin } = await readBoth('?schema=public');
+
+ expect(federation.status).toBe(200);
+ expect(admin.status).toBe(200);
+ // The filter really filtered — otherwise "equal" could mean "both unfiltered",
+ // which is precisely the pre-#7955 reading on one of the two paths.
+ expect(qualified(federation)).toEqual(['public.customers', 'public.orders']);
+ expect(qualified(admin)).toEqual(qualified(federation));
+ });
+
+ it('no ?schema= returns the SAME unfiltered set on both spellings', async () => {
+ const { federation, admin } = await readBoth('');
+
+ expect(qualified(federation)).toEqual([
+ 'analytics.events',
+ 'public.customers',
+ 'public.orders',
+ ]);
+ // The absent-parameter arm is not a formality: a fix that filtered
+ // unconditionally would satisfy the case above and break every existing
+ // caller of the admin spelling, which has never passed one.
+ expect(qualified(admin)).toEqual(qualified(federation));
+ });
+
+ it('a ?schema= that matches nothing returns the SAME empty set on both spellings', async () => {
+ const { federation, admin } = await readBoth('?schema=nonexistent');
+
+ expect(federation.tables).toEqual([]);
+ expect(admin.tables).toEqual([]);
+ expect(admin.status).toBe(federation.status);
+ });
+
+ it('a repeated ?schema= degrades to no filter on both spellings, identically', async () => {
+ // The adapter surfaces a repeated key as an array; `typeof … === 'string'`
+ // is what both handlers do with it, so both fall back to the unfiltered
+ // listing rather than filtering by an arbitrary one of the two. Whether such
+ // a request should be REFUSED instead is #7606's global ingress question —
+ // this case pins that the twins answer it the SAME way today, whatever that
+ // card decides tomorrow.
+ const { federation, admin } = await readBoth('?schema=public&schema=analytics');
+
+ expect(qualified(federation)).toEqual([
+ 'analytics.events',
+ 'public.customers',
+ 'public.orders',
+ ]);
+ expect(qualified(admin)).toEqual(qualified(federation));
+ expect(admin.status).toBe(federation.status);
+ });
+
+ it('an empty ?schema= is no filter on both spellings, identically', async () => {
+ // `?schema=` parses to the empty string, which is a string — so the
+ // coercion keeps it and the service's own `opts?.schema &&` guard is what
+ // treats it as "no filter". Both spellings inherit that from the one
+ // service, and this pins that neither route second-guesses it.
+ const { federation, admin } = await readBoth('?schema=');
+
+ expect(qualified(federation)).toEqual([
+ 'analytics.events',
+ 'public.customers',
+ 'public.orders',
+ ]);
+ expect(qualified(admin)).toEqual(qualified(federation));
+ });
+});

packages/rest/vitest.config.ts

@@ -1,10 +1,50 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { defineConfig } from 'vitest/config';
+import path from 'path';
 
 export default defineConfig({
  test: {
  globals: true,
  environment: 'node',
  },
+ resolve: {
+ // Both entries exist for `remote-tables-twin.equivalence.test.ts` (#7955),
+ // the one suite here that imports sibling packages as VALUES: it mounts the
+ // `service-datasource` admin registrar next to this package's federation
+ // registrar to pin that the two `listRemoteTables` spellings answer `?schema=`
+ // identically, and drives both through the real Hono adapter.
+ //
+ // Unaliased, those two specifiers resolve through the workspace link to
+ // `dist/` — a BUILD ARTIFACT — which makes this suite's verdict a function of
+ // build state rather than of the source in the checkout. The loud failure
+ // (missing export) is the mild half; a dist merely BEHIND lets the test run
+ // GREEN against the dependency's old behaviour, and nothing in the output
+ // says so. That is exactly the hazard for a CROSS-PACKAGE equivalence pin:
+ // its whole job is to notice when one of the two twins moves, and a stale
+ // `service-datasource` dist would report the pre-fix admin route as agreeing
+ // with the federation one — the #7955 defect itself, passing.
+ //
+ // Turbo already orders `test` after `^build`, so `turbo run test` was never
+ // the failing path. The paths it does not mediate are: `pnpm test` inside
+ // this package, `vitest run <file>`, an editor runner, or an agent working
+ // in a tree built at an older commit — which are precisely the ways this pin
+ // gets re-run WHILE someone is changing one of the two routes.
+ //
+ // Array form with anchored patterns, deliberately: the object form matches
+ // by PREFIX, so a bare `@objectstack/service-datasource` key with a FILE
+ // replacement would also swallow `@objectstack/service-datasource/contracts`
+ // and resolve it to `…/src/index.ts/contracts` (ENOTDIR) at run time, in a
+ // config that looks right. Same shape as `service-storage`'s config (#7778).
+ alias: [
+ {
+ find: /^@objectstack\/plugin-hono-server$/,
+ replacement: path.resolve(__dirname, '../plugins/plugin-hono-server/src/index.ts'),
+ },
+ {
+ find: /^@objectstack\/service-datasource$/,
+ replacement: path.resolve(__dirname, '../services/service-datasource/src/index.ts'),
+ },
+ ],
+ },
 });

packages/services/service-datasource/src/__tests__/admin-routes.test.ts

@@ -77,7 +77,23 @@ describe('registerDatasourceAdminRoutes (real HonoHttpServer)', () => {
  const res = await app.fetch(json('/api/v1/datasources/demo_ext/remote-tables'));
  expect(res.status).toBe(200);
  expect(await res.json()).toEqual({ success: true, data: { tables: [{ name: 'customers', columnCount: 4 }] } });
- expect(listRemoteTables).toHaveBeenCalledWith('demo_ext');
+ // No `?schema=` ⇒ the options bag carries no filter (#7955). The bag itself
+ // is always passed; `{ schema: undefined }` is what the service reads as
+ // "unfiltered", and it is the same value the federation twin hands it.
+ expect(listRemoteTables).toHaveBeenCalledWith('demo_ext', { schema: undefined });
+ });
+
+ // The forwarding half of #7955 at THIS spelling. The cross-package half —
+ // that the two spellings answer the same SET — is
+ // `packages/rest/src/remote-tables-twin.equivalence.test.ts`, which has to
+ // live where both registrars are reachable; this case is what fails first if
+ // the query stops being read here at all.
+ it('GET /api/v1/datasources/:name/remote-tables forwards ?schema= to the service', async () => {
+ const listRemoteTables = vi.fn().mockResolvedValue([]);
+ const app = mount({ listRemoteTables });
+ const res = await app.fetch(json('/api/v1/datasources/demo_ext/remote-tables?schema=public'));
+ expect(res.status).toBe(200);
+ expect(listRemoteTables).toHaveBeenCalledWith('demo_ext', { schema: 'public' });
  });
 
  it('POST /api/v1/datasources/:name/object-draft generates a draft (400 without table)', async () => {

packages/services/service-datasource/src/admin-routes.ts

@@ -57,7 +57,7 @@ const SERVICE_ERROR_CODE: Record<ServiceName, ErrorCode> = {
  *
  * Served by `external-datasource`:
  *
- * GET /datasources/:name/remote-tables → listRemoteTables
+ * GET /datasources/:name/remote-tables → listRemoteTables (?schema= filters)
  * POST /datasources/:name/test → testConnection (a SAVED datasource)
  * POST /datasources/:name/object-draft → generateObjectDraft
  *
@@ -197,11 +197,28 @@ export function registerDatasourceAdminRoutes(
  // `POST /datasources/:name/object-draft` generates an ObjectStack object
  // definition draft for one table (introspect + type-map, no persistence —
  // the caller creates the object through the normal metadata channel).
+ //
+ // `?schema=` narrows the listing to one remote schema, and is forwarded here
+ // for the same reason #4249 gave the two spellings one FAILURE contract: they
+ // are one operation — `IExternalDatasourceService.listRemoteTables`, resolved
+ // from the same `external-datasource` slot — reached two ways. Until #7955
+ // this handler never read the query, so `?schema=public` came back UNFILTERED:
+ // neither the filtered answer nor a refusal, which is the "declared ≠
+ // enforced" shape (Prime Directive #10) in its quietest form — the twin one
+ // path over honoured the same parameter. The coercion below is copied from
+ // that twin (`packages/rest/src/external-datasource-routes.ts`) deliberately,
+ // down to what it does with a NON-string: a repeated `?schema=a&schema=b`
+ // reaches the handler as an array (the adapter surfaces repeated keys that
+ // way), and both spellings drop it to `undefined` — no filter. Whether an
+ // unusable query parameter should instead be REFUSED is the ingress-policy
+ // question #7606 owns globally; honouring it is correct under either answer,
+ // so this route does not pre-empt it.
  server.get(`${root}/:name/remote-tables`, async (req: any, res: any) => {
  const svc = resolve(res, 'external-datasource', 'listRemoteTables');
  if (!svc) return;
  try {
- const tables = await svc.listRemoteTables(req.params.name);
+ const schema = typeof req.query?.schema === 'string' ? req.query.schema : undefined;
+ const tables = await svc.listRemoteTables(req.params.name, { schema });
  sendOk(res, { tables });
  } catch (err) {
  badRequest(res, 'external-datasource', err);