Skip to content

Commit 72a302a

Browse files
fix(service-datasource)!: Sync refuses a managed datasource with the existing 400 (ADR-0015 §4.2) (#25167)
Fixes #24837 Clause-②: no (narrowing) Metadata-protocol: none ## What changes ADR-0015 §1 puts a federated object on a datasource whose `schemaMode` is not `managed`, and §4.2 allows `object.external` only there; `managed` is the spec's default. The Sync doors (list remote tables, draft an object, import one, refresh the catalog) refused only the platform's primary datasource (`default`, the route-D ruling on #24772). Every other managed datasource answered, including the runtime datasources Setup creates, which are saved with no `schemaMode` and so are managed (the HotCRM tester's `qa_mem` listed its tables `200`). Now one guard in the federation service, `ExternalDatasourceService.assertFederationSource`, runs first in all four Sync methods and refuses, before anything is introspected: - `default`, by name, with its existing message (unchanged); - any other datasource that is not a federation source: `schemaMode: 'managed'` declared, no `schemaMode` (the default), or no datasource definition at all. Each refusal is the existing `400` with the code that door already serves: `EXTERNAL_DATASOURCE_ERROR`, and `EXTERNAL_IMPORT_ERROR` on the import. The message names ADR-0015's rule and the remedy (sync from a datasource declared `external` or `validate-only`). No status code or error code is minted. The governing text is ADR-0015 §1 and §4.2; retiring the rule instead would be an ADR amendment, which this does not take. Patch round 1 (`658d9d0b5`) adds one test-fixture line in `packages/rest`, declared cross-lane on #22648 (`6332535245`): `remote-tables-twin.equivalence.test.ts` now declares its fixture datasource `schemaMode: 'external'`, because a datasource with no `schemaMode` is managed and both Sync twins now refuse it; no `packages/rest` source line changes. ## Where the refusal lives (measured) The one producer behind every door is `ExternalDatasourceService` in `packages/services/service-datasource/src/external-datasource-service.ts`: | door | handler | producer method | |---|---|---| | `GET /api/v1/datasources/:name/remote-tables` | `service-datasource` `admin-routes.ts` | `listRemoteTables` | | `POST /api/v1/datasources/:name/object-draft` | `service-datasource` `admin-routes.ts` | `generateObjectDraft` | | `GET /api/v1/datasources/:name/external/tables` | `packages/rest` `external-datasource-routes.ts` | `listRemoteTables` | | `POST /api/v1/datasources/:name/external/tables/:remote/draft` | `packages/rest` `external-datasource-routes.ts` | `generateObjectDraft` | | `POST /api/v1/datasources/:name/external/tables/:remote/import` | `packages/rest` `external-datasource-routes.ts` | `importObject` | | `POST /api/v1/datasources/:name/external/refresh-catalog` | `packages/rest` `external-datasource-routes.ts` | `refreshCatalog` | `os datasource list-tables`, `os datasource introspect` and `@objectstack/client` reach those HTTP doors. So no `packages/rest` source line changes. `importObject` now drafts through a private `draftObject` (the old `generateObjectDraft` body, unchanged), so an import is judged once, under its own code. `validateObject` asks the same predicate (`isFederationSource`) for its existing "not federated, nothing to validate" skip: no behaviour change there, and what Sync refuses is now exactly what validation skips. Measured answers on head `b4fb8b8fe` (`demo_ext` declares no `schemaMode`; `lake_ext` declares `external`). The four `packages/rest` rows came from an uncommitted scratch harness built from `remote-tables-twin.equivalence.test.ts`'s fixture, deleted after the run. The final head, `145e52859`, only replaces the guard's code parameter with a door kind, so that `check:dispatcher-error-vocabulary` reads both codes as literals; the codes per door are pinned in `service-datasource`. | door | managed (`demo_ext`) | `external` (`lake_ext`) | |---|---|---| | `GET …/external/tables` | `400 EXTERNAL_DATASOURCE_ERROR` | `200` | | `POST …/external/tables/customers/draft` | `400 EXTERNAL_DATASOURCE_ERROR` | `200` | | `POST …/external/tables/customers/import` | `400 EXTERNAL_IMPORT_ERROR` | `201` | | `POST …/external/refresh-catalog` | `400 EXTERNAL_DATASOURCE_ERROR` | `200` | | `GET …/remote-tables` (admin) | `400 EXTERNAL_DATASOURCE_ERROR` | `200` | | `POST …/object-draft` (admin) | `400 EXTERNAL_DATASOURCE_ERROR` | `200` | ## Readings taken within the ruling (each stated, none new) 1. **The import door keeps `EXTERNAL_IMPORT_ERROR`.** The card says "the existing 400 `EXTERNAL_DATASOURCE_ERROR`" for Sync's routes. The import route's catch serves `400 EXTERNAL_IMPORT_ERROR` for every refusal, whatever the thrown code, and `default`'s refusal already declares that code there. So the import answers its existing code; nothing is minted or moved. 2. **`refresh-catalog` is refused too.** It is the fourth method behind the same producer, and the `default` refusal already covered it. One guard, one fact, so no door stays open. 3. **A name with no datasource definition is refused.** Read with the spec default, an undefined definition has no declared `schemaMode`, so it is managed. It is also how validation already read it ("not federated"), so an import can no longer bind an object the sweep never checks. Its message says "has no datasource definition". ## Pins reversed (not deleted) - `primary-datasource-test-and-sync.test.ts` › "a managed runtime datasource tests and syncs exactly as before" (remote-tables `200` on a managed runtime SQLite datasource) is now "a managed runtime datasource tests exactly as before, and its Sync refuses". The Test half is unchanged. - `primary-datasource-test-and-sync.test.ts` › "Sync on a driver that cannot introspect still refuses with the engine reason": the managed `memory` datasource now gets the managed refusal (asserted), and the engine reason is asserted on a federated `memory` datasource (`stream`). The harness now has the `'metadata'` service the plugin reads, serving the same records the admin list reads. - `external-metadata-read-at-use.test.ts` › "validate and the sweep answer an empty report, the draft a bare name, the catalog an unpersisted snapshot" (the draft, refresh and listing answering with no datasource definition readable) is now "validate and the sweep answer an empty report, and the Sync doors refuse an undeclared datasource". The validate half is unchanged. New: `managed-datasource-sync-refusal.test.ts`, over the real composition (real `ObjectQL`, connection service, driver factory, `ExternalDatasourceServicePlugin` wiring, admin routes on Hono). It covers each door refusing `warehouse` (no `schemaMode`), `ledger` (`managed`) and `qa_mem` (runtime `memory`) with `code` + `status` and nothing saved or introspected; a name with no definition; `lake` (`external`) and `mirror` (`validate-only`) still listing, drafting, importing and refreshing; `default` unchanged; and Setup's list (`schemaMode: 'managed'` for the tester's runtime datasources) agreeing with the refusal row by row. ## Known gap: one `packages/rest` fixture line needs the seat's declaration first (closed) **Closed in patch round 1 (`658d9d0b5`):** the seat declared the edit on #22648 (`6332535245`), the line below is committed, and `Test Core` is green on that head. The paragraph below is the record of the gap as it stood on `145e52859`. `packages/rest/src/remote-tables-twin.equivalence.test.ts` builds its datasource as `{ name }`. That is managed by default, so 5 of its cases now go red (`Test Core`): both twins answer the refusal where the cases expect `200`. It is an under-declared fixture, not a managed-Sync pin: the twin-equivalence cases need a federation source to compare listings. The fix is one line, `getDatasource: async (name: string) => ({ name, schemaMode: 'external' as const })`. Probed uncommitted on `145e52859` through `scripts/ablation-replace.mjs`: with that line the file passes 10 of 10, and the tool restored the file (blob equal to HEAD, `git diff HEAD` empty). This PR does not make the edit: `packages/rest` is `domain:cli`'s, and the dispatch requires a seat declaration before an edit there. ## Tests All on the final head `145e52859`, under the shared verify lock: - `pnpm --filter @objectstack/service-datasource exec vitest run --maxWorkers=2`: `Test Files 54 passed (54)`, `Tests 945 passed (945)`. - `pnpm --filter @objectstack/service-datasource typecheck`: exit 0. `tsc --listFiles` includes all three touched test files. - `pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2 src/remote-tables-twin.equivalence.test.ts src/external-datasource-validate-scope.test.ts src/external-datasource-envelope.conformance.test.ts`: 5 failed, 27 passed. All 5 failures are the twin-fixture cases named above. - The federation dogfood files (`external-import-saves-like-meta`, `external-import-code-datasource-namespace`, `external-import-destructive-remedy`, `external-validate-sees-runtime-save`, `external-validate-start-ordering`) on the booted showcase, whose `showcase_external` declares `schemaMode: 'external'`: 5 files, 20 tests passed. - **Full dogfood battery** (the title carries `!`): the dependency closure was built, then `pnpm --filter @objectstack/dogfood test` ran under the lock with no shard and no path. `VERDICT command-exit 0` · `Test Files 287 passed | 3 skipped (290)` · `Tests 2395 passed | 18 skipped (2413)`. **Ablation (one-off, not kept).** The fix was committed first. Then, through `scripts/ablation-replace.mjs`, the guard's managed line on `145e52859` was replaced with a no-op (anchor hit 1 time, blob `ad6c9f16563f` became `a2e8fe2b78ac`) and the three files were run: 13 failed, 19 passed. Every managed cell, the no-definition cells, the row-by-row list agreement and the three reversed pins turned red. `default`'s refusal and the federated cells stayed green. The tool restored the file (blob back to `ad6c9f16563f`, equal to HEAD; `git diff HEAD` empty). The subject is imported by relative path from `src/`, so no `dist/` leg applies. A first ablation on `b4fb8b8fe` gave the same 13 and 19. **Gates.** The derived list on `145e52859` was `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, 68 commands. All 68 exited 0, each code captured before any pipe. `--ran` reconciliation: 68 derived, 68 run, 0 NOT-MEASURED, 0 UNRUN, exit 0. The no-major level axis was also run against this body as the event payload: `LEVEL AXIS: … clause-② no (narrowing) and metadata-protocol none … a non-protocol change at patch`, exit 0. Every other family is CI's, on this PR's check runs. **Patch round 1, on `658d9d0b5`** (under the verify lock, dependency closures rebuilt): `remote-tables-twin.equivalence.test.ts` 10 passed (10), the 5 cases red on `145e52859` now green; the three `service-datasource` test files 32 passed (32); the 68 derived gates re-run and reconciled (68 derived, 68 run, 0 NOT-MEASURED). The full dogfood battery above stands for this head: the round changes one test-fixture line in `packages/rest` and no runtime code, and CI's `Dogfood Regression Gate` is green on `658d9d0b5`. ## Acceptance notes - **objectui still offers Sync on managed rows.** At objectui `origin/main` `56cf4d8`, `DatasourceResourcePage.tsx` renders the "Sync objects" button on every row, whatever its `schemaMode` (which is already in the list payload). After this PR, the button on a managed row opens the dialog with the refusal message (objectui#13207 draws refusals there). Not changed here; reported to the seat for an objectui card. Carrier: an objectui card the seat files when this PR lands. - **ADR-0015 §4.2's "enforced at metadata-load time" has no enforcing site.** `defineStack()` on the built spec, at `b4fb8b8fe`, accepted an object with `external: { remoteName: 'orders' }` bound to a `schemaMode: 'managed'` datasource. This PR closes the Sync producer of such objects only. Reported to the seat; not fixed here. Carrier: #25168, filed for triage. - No ADR anchor file is added for the guard. `scripts/adr-anchors/` is outside the claim's declared file surface; the ADR id is in the code and in the refusal text. The contract review (`6333095709`) found it not blocking (`check:adr-anchors` is green and no ADR-0015 anchor names this service) and named it a lane follow-up. Carrier: none (no user-visible defect). - Pins for the `packages/rest` doors (draft, import, refresh) belong in `packages/rest`. They were measured once here (table above) and not committed. The contract review (`6333095709`) found it not blocking (each REST catch maps every service throw to that door's fixed code) and named a `packages/rest` pin of the four doors as a `domain:cli` follow-up. Carrier: none (no user-visible defect). --- _Generated by [Claude Code](https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c5915b2 commit 72a302a

6 files changed

Lines changed: 550 additions & 53 deletions
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@objectstack/service-datasource': patch
3+
---
4+
5+
fix(service-datasource)!: Sync (remote tables, object draft, import, catalog refresh) refuses a managed datasource with the existing `400`, because ADR-0015 §4.2 reserves federation for a datasource whose `schemaMode` is not `managed`
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition or stored shape is added, removed, renamed or re-shaped, and no exported declaration is removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What changes is the answer four runtime REST doors give a caller who asks them to list, draft, import or catalog the tables of a datasource the platform owns; the remedy is to sync from a datasource declared `external` or `validate-only`, never a rewrite of a package source or a stored row. Objects an earlier import already bound to a managed datasource are not touched. -->
11+
12+
**BREAKING** (four REST doors' accept set narrows). ADR-0015 §1 puts a federated object on a datasource whose `schemaMode` is not `managed`, and §4.2 allows `object.external` only there; `managed` is the spec's default. The Sync doors refused only the platform's primary datasource (`default`). Every other managed datasource answered, including the runtime datasources an administrator creates in Setup → Datasources, which are saved with no `schemaMode` and so are managed: their tables were listed, drafted and imported as federated objects. The engine's write gate (ADR-0015 §5.3) gates only non-managed datasources, so nothing stood behind such an import. One guard in the federation service now refuses before anything is introspected, and every door that reaches the service inherits it.
13+
14+
- **FROM:** on a datasource other than `default` whose `schemaMode` is `managed`, declared or by default, `GET /api/v1/datasources/:name/remote-tables` and `GET /api/v1/datasources/:name/external/tables` answered `200` with its tables; `POST /api/v1/datasources/:name/object-draft` and `POST /api/v1/datasources/:name/external/tables/:remote/draft` answered `200` with a federated object draft bound to it; `POST /api/v1/datasources/:name/external/tables/:remote/import` answered `201` and saved that object; `POST /api/v1/datasources/:name/external/refresh-catalog` answered `200` and stored an `external_catalog` for it. A name with no datasource definition was introspected whenever a driver was registered under it.
15+
- **TO:** each of those requests answers `400` with the code its door already serves for a refusal, `EXTERNAL_DATASOURCE_ERROR` (the import: `EXTERNAL_IMPORT_ERROR`), and a message naming the rule and the remedy. Nothing is introspected, drafted, saved or stored. A name with no datasource definition gets the same refusal: nothing declares it a federation source.
16+
- **Unchanged:** `default` keeps its own refusal and message. A datasource declared `schemaMode: 'external'` or `'validate-only'` lists, drafts, imports and refreshes as before. Test (`POST /api/v1/datasources/:name/test`) and validation (`POST /api/v1/datasources/:name/external/validate`) answer as before; validation already skipped a managed datasource, and it now asks the same predicate the guard asks. No status code or error code is added.
17+
- **The fix for an integrator:** sync from a datasource the platform does not own. Declare it with `schemaMode: 'external'` (or `'validate-only'`) and its `external` settings; for a runtime datasource, send both in `PATCH /api/v1/datasources/:name`. A managed datasource's tables are the platform's own objects: define objects on it directly instead of importing them.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ const makeQl = () => bindTestSecurityCatalog({
231231
function mountBoth() {
232232
const service = new ExternalDatasourceService({
233233
introspect: async () => REMOTE,
234-
getDatasource: async (name: string) => ({ name }),
234+
getDatasource: async (name: string) => ({ name, schemaMode: 'external' as const }),
235235
getObject: async () => undefined,
236236
listObjects: async () => [],
237237
});

‎packages/services/service-datasource/src/__tests__/external-metadata-read-at-use.test.ts‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@
2929
* The service is asked again at each use, never remembered from the first
3030
* one. A service registered BEFORE `init()` (the `objectstack dev` ordering)
3131
* gets the same answers. With no `'metadata'` service at all, every reader
32-
* keeps its documented fallback.
32+
* keeps its documented fallback. [#24837] For the Sync doors that fallback is
33+
* now a refusal: with no datasource definition to read, nothing declares the
34+
* datasource a federation source (ADR-0015 §4.2), which is also why validate
35+
* skips it.
3336
*
3437
* [#21889] The namespace half (the draft's prefix and the import's name
3538
* check) moved to `external-namespace-reads-engine-registry.test.ts`: the
@@ -277,16 +280,29 @@ describe('control: a metadata service registered before init (the dev ordering)
277280
});
278281

279282
describe('with no metadata service at all, every reader keeps its fallback', () => {
280-
it('validate and the sweep answer an empty report, the draft a bare name, the catalog an unpersisted snapshot', async () => {
283+
// [#24837] REVERSED. This case held the draft, the catalog refresh and the
284+
// remote-table list ANSWERING with no metadata service at all, so with no
285+
// datasource definition to read. The Sync doors now refuse that datasource:
286+
// nothing declares it a federation source, and ADR-0015 §4.2 reserves
287+
// federation for a datasource whose `schemaMode` is not `managed`. It is the
288+
// same reading validate already made of an unreadable definition ("not
289+
// federated", the empty report below), so an import can no longer bind an
290+
// object the sweep would never check. The refusal comes before any
291+
// introspection.
292+
it('validate and the sweep answer an empty report, and the Sync doors refuse an undeclared datasource', async () => {
281293
// [#21842] No engine registry either: that is where validate's objects
282294
// come from, so with neither service the report has nothing to list.
283-
const service = await federation(harness({ registry: false }));
295+
const h = harness({ registry: false });
296+
const service = await federation(h);
284297

285298
expect(await service.validateAll()).toEqual({ ok: true, results: [] });
286-
expect((await service.generateObjectDraft('warehouse', 'customers')).name).toBe('customers');
287-
const catalog = await service.refreshCatalog('warehouse');
288-
expect(catalog.name).toBe('warehouse_catalog');
289-
expect((await service.listRemoteTables('warehouse')).map((t) => t.name).sort()).toEqual(['customers', 'orders']);
299+
const refusal = { status: 400, message: expect.stringContaining("Datasource 'warehouse' has no datasource definition") };
300+
await expect(service.generateObjectDraft('warehouse', 'customers')).rejects.toMatchObject({ code: 'EXTERNAL_DATASOURCE_ERROR', ...refusal });
301+
await expect(service.refreshCatalog('warehouse')).rejects.toMatchObject({ code: 'EXTERNAL_DATASOURCE_ERROR', ...refusal });
302+
await expect(service.listRemoteTables('warehouse')).rejects.toMatchObject({ code: 'EXTERNAL_DATASOURCE_ERROR', ...refusal });
303+
await expect(service.importObject('warehouse', 'customers')).rejects.toMatchObject({ code: 'EXTERNAL_IMPORT_ERROR', ...refusal });
304+
const data = h.services.get('data') as { introspectDatasource: Mock };
305+
expect(data.introspectDatasource).not.toHaveBeenCalled();
290306
});
291307

292308
it('a metadata service with no register leaves the catalog unpersisted, without failing the refresh', async () => {

0 commit comments

Comments
 (0)