Skip to content

Commit 0db5520

Browse files
claude[bot]claude
andauthored
feat(objectql): retain and expose external.credentialsRef on datasource definitions (#12806)
* feat(objectql): retain and expose external.credentialsRef on datasource definitions `registerDatasourceDef`'s inline parameter type carried only `name`, `schemaMode` and `external.allowWrites`, so a caller passing a fresh object literal with `external.credentialsRef` was refused by excess-property checking (TS2353) — and the engine exposed no reader onto its datasource index at all, its only consumer being the private write gate. Measured before changing anything: nothing stripped the reference at runtime. The writer stores the caller's `external` object whole, by reference, and the manifest install path spreads the def straight through, so the value was already in the index — unreachable to every typed producer and to every consumer. The defect was type-level, and the fix is a widening plus the accessor that was missing. - name the shape as `DatasourceDef` rather than restating it at all three touch points, and widen it with `external.credentialsRef?: string` — the key `@objectstack/spec` already declares (`ExternalDatasourceSettingsSchema`), valid in every `schemaMode` per #8153, so retention rather than invention; - add `ObjectQL.listDatasourceDefs()`, deliberately unfiltered and returning copied `external` blocks, so a `sys_secret` reference sweep can see the handles a datasource declared IN CODE holds — those never reach `sys_metadata`, so today the host has to remember to pass them in; - pin the compile-time half in a `.pin.ts`, since the package's tsconfig excludes `**/*.test.ts` and a `@ts-expect-error` in a test file there would be a phantom check. The write gate is untouched: it reads `schemaMode` + `allowWrites` and the new key is inert to it, which the runtime tests pin in both directions. Part of #12758 * predict: what each ablation must do, committed BEFORE mutating anything A1 — type-level pin. Delete `credentialsRef?: string;` from `DatasourceDef`. PREDICTION: `pnpm --filter @objectstack/objectql typecheck` goes RED with TS2353 on the POSITIVE lines of `datasource-def-credentials-ref.pin.ts`, AND the vitest run stays GREEN. The second half is the point: a runtime test cannot see a compile-time widening, which is why the pin exists at all. A2 — the accessor's retention. Make `listDatasourceDefs` copy only `allowWrites` out of the stored `external` block. PREDICTION: vitest goes RED on the read-back cases (both entry routes, the managed-datasource case, the defensive-copy case) and STAYS GREEN on the two write-gate cases, which do not read the reference. A3 — the write gate is really the write gate. Force `dsAllows = true`. PREDICTION: vitest goes RED on "still refuses a write without the double opt-in" here AND in the pre-existing `external-write-gate.test.ts`, proving this file's gate assertion rides the real Gate 3 and not a local stub. Restore leg for each: `git checkout HEAD -- <absolute path>`, proven by a `git hash-object` match against the HEAD blob plus an empty `git diff`. No rebuild is needed on any leg: the pin and the test both import `./engine` RELATIVELY, so neither resolves through the package `exports` field into `dist/` — the mutation on disk is the code under test. Part of #12758 * chore(changeset): minor for the datasource credentials-reference widening Argues the bump rather than defaulting it: zero runtime change (the case for patch) against three additions to public API (the case for minor). Part of #12758 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 26deb31 commit 0db5520

5 files changed

Lines changed: 425 additions & 7 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
feat(objectql): retain and expose `external.credentialsRef` on datasource definitions (#12758)
6+
7+
`ObjectQL.registerDatasourceDef`'s parameter type carried only `name`,
8+
`schemaMode` and `external.allowWrites`, so a caller passing a fresh object
9+
literal with `external.credentialsRef` was refused by excess-property checking
10+
(`TS2353`) — while the docs (`/docs/data-modeling/external-datasources`)
11+
prescribe exactly that key on a code-declared datasource, and
12+
`@objectstack/spec` has declared it all along on
13+
`ExternalDatasourceSettingsSchema`, valid in every `schemaMode` (#8153). The
14+
engine also exposed **no reader at all** onto its datasource index; its sole
15+
consumer was the private write gate.
16+
17+
Measured before anything was changed: nothing stripped the reference at
18+
runtime. The writer stores the caller's `external` object whole, by reference,
19+
and the package-manifest install path spreads the def straight through — so the
20+
value was already in the index, unreachable to every typed producer and every
21+
consumer. The defect was type-level, and the fix is a widening plus the
22+
accessor that was missing.
23+
24+
- `registerDatasourceDef` now takes the named, exported `DatasourceDef`, whose
25+
`external` block carries `credentialsRef?: string` beside `allowWrites`.
26+
Retention, not invention: the key is the spec's, and every shape that
27+
compiled before still compiles.
28+
- New `ObjectQL.listDatasourceDefs()` answers every definition the engine
29+
holds, from both entry routes. Deliberately unfiltered — `credentialsRef` is
30+
valid on a managed datasource too, so filtering by schema mode would hide
31+
live handles from a `sys_secret` reference sweep, and under-reporting is the
32+
direction that deletes live credentials. Each entry carries a copied
33+
`external` block so a reader cannot reach through it and mutate the write
34+
gate's own input.
35+
36+
Why this matters beyond tidiness: a datasource declared **in code** never
37+
reaches `sys_metadata`, so the cross-producer `sys_secret` reference union
38+
(#12663) cannot see the handle it holds and must be handed the list by its
39+
host. That makes the completeness of the union — the precondition an orphan
40+
sweep's deletion predicate rests on — depend on every caller remembering to
41+
pass a list. This moves the guarantee from process to mechanism. The union is
42+
not rewired here; that is consumer-side work on a shipped contract and is
43+
tracked separately.
44+
45+
The write gate is untouched: it reads `schemaMode` + `allowWrites`, the new key
46+
is inert to it, and both directions of the gate stay pinned.
47+
48+
**Why `minor` and not `patch`.** Zero runtime behaviour changes, which is the
49+
honest case for `patch` — but the bump describes the **contract**, not the
50+
bytes executed, and this release adds public API three ways: a new public
51+
method (`listDatasourceDefs`), a newly exported type (`DatasourceDef`), and a
52+
widened accepted set on an existing public method (calls that were rejected at
53+
compile time now compile). A consumer pinning `~` would receive new API under a
54+
`patch`, which misdescribes the release. Nothing is removed, narrowed or
55+
renamed, so no breaking-change declaration and no ADR-0087 entry arise; `minor`
56+
is the additive-surface bump, not the launch-window breaking convention.
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #12758 — compile-time pin for the shape `registerDatasourceDef` accepts and
5+
* the shape `listDatasourceDefs` answers.
6+
*
7+
* THE DEFECT THIS PINS WAS PURELY TYPE-LEVEL, which is why the pin lives here
8+
* and not only in a `.test.ts`. Measured on the pre-change tree: nothing ever
9+
* stripped `external.credentialsRef` at runtime — `registerDatasourceDef`
10+
* stored the caller's `external` object whole, by reference, and the manifest
11+
* install path spread the def straight through — so the reference was already
12+
* in the engine's index. What did not exist was any way to put it there
13+
* honestly or to read it back:
14+
*
15+
* - a caller passing a FRESH object literal was refused with TS2353
16+
* ("'credentialsRef' does not exist in type '{ allowWrites?: boolean }'"),
17+
* so the only way in was a pre-typed variable or an `as any`; and
18+
* - the engine exposed no accessor onto the index at all — its sole reader
19+
* was the private write gate.
20+
*
21+
* A runtime test therefore cannot cover this card: the runtime never changed.
22+
* The accepted set of a public method did, and only `tsc` can see that.
23+
*
24+
* WHY A `.pin.ts` AND NOT A `*.test.ts`: `packages/objectql/tsconfig.json`
25+
* excludes `**\/*.test.ts`, so a `@ts-expect-error` written in a test file here
26+
* is a phantom check — no tsc program the `typecheck` script runs would ever
27+
* evaluate it, and deleting the directive would leave every gate green. This
28+
* file IS in that program. Same convention, and same reasoning, as
29+
* `register-object-authored-shape.pin.ts`. It carries no executable pin: the
30+
* assertions live in a function nobody calls, and the companion
31+
* `datasource-def-credentials-ref.test.ts` covers the runtime half.
32+
*/
33+
34+
import type { DatasourceDef, ObjectQL } from './engine.js';
35+
36+
/**
37+
* Taken off the METHOD, not off {@link DatasourceDef}, so that re-narrowing the
38+
* method's own signature moves this pin even if the named type survives.
39+
*/
40+
type RegisterArg = Parameters<ObjectQL['registerDatasourceDef']>[0];
41+
type ListedDefs = ReturnType<ObjectQL['listDatasourceDefs']>;
42+
43+
/**
44+
* Never called — every line is a type-level assertion evaluated by
45+
* `tsc --noEmit`. The members are taken as parameters rather than read off a
46+
* live engine so the pin needs no instance.
47+
*/
48+
export function __pinDatasourceDefCarriesCredentialsRef(
49+
register: (def: RegisterArg) => void,
50+
listed: ListedDefs,
51+
): void {
52+
// ── POSITIVE: the calls this card exists for. ────────────────────────────
53+
// FRESH object literals throughout — excess-property checking is the thing
54+
// under test, so a pre-typed variable here would defeat the pin entirely.
55+
register({
56+
name: 'warehouse',
57+
schemaMode: 'external',
58+
external: { allowWrites: true, credentialsRef: 'sys_secret:sec_1' },
59+
});
60+
// `credentialsRef` alone, no federation key: legal on a MANAGED datasource
61+
// per #8153, and the shape the Studio wizard's createDatasource writes.
62+
register({ name: 'warehouse', external: { credentialsRef: 'secret:warehouse/password' } });
63+
64+
// ── The pre-#12758 shapes must keep compiling — this is a WIDENING. ──────
65+
register({ name: 'warehouse' });
66+
register({ name: 'warehouse', schemaMode: 'external', external: { allowWrites: true } });
67+
68+
// ── NEGATIVE: the widening must not admit garbage. ───────────────────────
69+
// @ts-expect-error `name` is required — a definition without one registers nothing
70+
register({ schemaMode: 'external' });
71+
// @ts-expect-error `credentialsRef` is a REFERENCE into the secrets store, so a string
72+
register({ name: 'warehouse', external: { credentialsRef: 12_345 } });
73+
// @ts-expect-error inline credentials are refused everywhere — `password` is not a key here
74+
register({ name: 'warehouse', external: { password: 'hunter2' } });
75+
// @ts-expect-error the widening is scoped to credentialsRef; `validation` has no engine reader
76+
register({ name: 'warehouse', external: { validation: { onMismatch: 'warn' } } });
77+
78+
// ── READ-BACK: the accessor answers definitions, keyed by name. ──────────
79+
const one: DatasourceDef | undefined = listed[0];
80+
const ref: string | undefined = one?.external?.credentialsRef;
81+
const gate: boolean | undefined = one?.external?.allowWrites;
82+
void ref;
83+
void gate;
84+
// @ts-expect-error the accessor answers definitions, not bare datasource names
85+
const notAName: string = listed[0];
86+
void notAName;
87+
}
Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #12758 — runtime half of the datasource-definition credentials-reference
5+
* contract. The compile-time half is in
6+
* `datasource-def-credentials-ref.pin.ts` (it has to be: this file is excluded
7+
* from every tsc program the `typecheck` script runs, so a `@ts-expect-error`
8+
* written here would never be evaluated).
9+
*
10+
* ⛔ NOTHING HERE IS PHRASED AS "the reference is no longer dropped". Measured
11+
* on the pre-change tree, the reference was never dropped: `registerDatasourceDef`
12+
* stored the caller's `external` object whole, by reference, and the manifest
13+
* install path spread the def straight through. A test claiming otherwise would
14+
* pin something that was never true. What IS new — and what this file covers —
15+
* is that the value is now READABLE, through an accessor that did not exist:
16+
* the engine had no reader onto its datasource index at all, only the private
17+
* write gate.
18+
*
19+
* Why it matters: a datasource declared IN CODE never reaches `sys_metadata`,
20+
* so the cross-producer `sys_secret` reference union cannot see the handle it
21+
* holds and has to be handed the list by its host. This accessor is what lets
22+
* the engine answer instead of the caller remembering.
23+
*/
24+
25+
import { describe, expect, it } from 'vitest';
26+
import type { IDataDriver } from '@objectstack/spec/contracts';
27+
import { ExternalWriteForbiddenError } from '@objectstack/spec/shared';
28+
import { ObjectQL } from './engine';
29+
30+
const REF = 'sys_secret:sec_credref_12758';
31+
32+
function makeDriver(name: string): IDataDriver {
33+
const store = new Map<string, Record<string, unknown>>();
34+
return {
35+
name,
36+
version: '1.0.0',
37+
async connect() {},
38+
async disconnect() {},
39+
async find() { return []; },
40+
async findOne() { return null; },
41+
async count() { return 0; },
42+
async create(object: string, data: Record<string, unknown>) {
43+
const id = (data.id as string) ?? String(store.size + 1);
44+
const row = { ...data, id };
45+
store.set(`${object}:${id}`, row);
46+
return row;
47+
},
48+
async update(object: string, id: string, data: Record<string, unknown>) {
49+
const row = { ...(store.get(`${object}:${id}`) ?? {}), ...data, id };
50+
store.set(`${object}:${id}`, row);
51+
return row;
52+
},
53+
async delete(object: string, id: string) { return store.delete(`${object}:${id}`); },
54+
async syncSchema() {},
55+
async dropTable() {},
56+
} as unknown as IDataDriver;
57+
}
58+
59+
/** The one definition, as every route below declares it. */
60+
const DEF = {
61+
name: 'warehouse',
62+
schemaMode: 'external',
63+
external: { allowWrites: true, credentialsRef: REF },
64+
} as const;
65+
66+
describe('datasource definitions retain external.credentialsRef and are readable (#12758)', () => {
67+
describe('entry route 1 — the direct registerDatasourceDef call', () => {
68+
it('lists the definition back with its credentials reference', () => {
69+
const engine = new ObjectQL();
70+
// No cast. If the parameter is ever re-narrowed this line stops compiling
71+
// in the pin file; here it is the runtime read-back that is under test.
72+
engine.registerDatasourceDef({ ...DEF, external: { ...DEF.external } });
73+
74+
const listed = engine.listDatasourceDefs();
75+
expect(listed).toHaveLength(1);
76+
expect(listed[0]).toMatchObject({
77+
name: 'warehouse',
78+
schemaMode: 'external',
79+
external: { allowWrites: true, credentialsRef: REF },
80+
});
81+
});
82+
});
83+
84+
describe('entry route 2 — the package-manifest install path (registerApp)', () => {
85+
// The widest blast radius of the narrowing: a code-declared datasource
86+
// reaches the engine here and nowhere else. Manifests may spell
87+
// `datasources` as an array OR as a name-keyed map, and the two take
88+
// different branches, so both are pinned.
89+
it('retains the reference through the ARRAY spelling', () => {
90+
const engine = new ObjectQL();
91+
engine.registerApp({
92+
id: 'wh_pkg_array',
93+
name: 'Warehouse',
94+
datasources: [{ ...DEF, external: { ...DEF.external } }],
95+
});
96+
97+
expect(engine.listDatasourceDefs()).toEqual([
98+
{ name: 'warehouse', schemaMode: 'external', external: { allowWrites: true, credentialsRef: REF } },
99+
]);
100+
});
101+
102+
it('retains the reference through the NAME-KEYED MAP spelling', () => {
103+
const engine = new ObjectQL();
104+
engine.registerApp({
105+
id: 'wh_pkg_map',
106+
name: 'Warehouse',
107+
datasources: { warehouse: { schemaMode: 'external', external: { allowWrites: true, credentialsRef: REF } } },
108+
});
109+
110+
expect(engine.listDatasourceDefs()).toEqual([
111+
{ name: 'warehouse', schemaMode: 'external', external: { allowWrites: true, credentialsRef: REF } },
112+
]);
113+
});
114+
});
115+
116+
describe('the accessor is unfiltered, which is the whole point of it', () => {
117+
it('lists a MANAGED datasource that carries only a credentials reference (#8153)', () => {
118+
// `credentialsRef` is valid in every schemaMode. A reader that filtered
119+
// by schema mode would hide a live handle from a credentials sweep, and
120+
// under-reporting is the direction that deletes live credentials.
121+
const engine = new ObjectQL();
122+
engine.registerDatasourceDef({ name: 'billing', external: { credentialsRef: 'secret:billing/password' } });
123+
124+
expect(engine.listDatasourceDefs()).toEqual([
125+
{ name: 'billing', external: { credentialsRef: 'secret:billing/password' } },
126+
]);
127+
});
128+
129+
it('lists definitions that carry no reference at all, rather than dropping them', () => {
130+
const engine = new ObjectQL();
131+
engine.registerDatasourceDef({ name: 'plain', schemaMode: 'external', external: { allowWrites: false } });
132+
engine.registerDatasourceDef({ name: 'bare' });
133+
134+
const names = engine.listDatasourceDefs().map((d) => d.name).sort();
135+
expect(names).toEqual(['bare', 'plain']);
136+
});
137+
138+
it('answers an empty list on an engine that was told about no datasources', () => {
139+
// The control for every case above: the accessor reads a real index, and
140+
// an empty answer here is what makes a non-empty one elsewhere a reading.
141+
expect(new ObjectQL().listDatasourceDefs()).toEqual([]);
142+
});
143+
});
144+
145+
describe('the accessor hands out a copy, never the write gate\'s own input', () => {
146+
it('mutating the returned external block does not change what the engine holds', () => {
147+
const engine = new ObjectQL();
148+
engine.registerDatasourceDef({ ...DEF, external: { ...DEF.external } });
149+
150+
const first = engine.listDatasourceDefs()[0];
151+
first.external!.credentialsRef = 'sys_secret:tampered';
152+
first.external!.allowWrites = false;
153+
154+
expect(engine.listDatasourceDefs()[0].external).toEqual({ allowWrites: true, credentialsRef: REF });
155+
});
156+
});
157+
158+
describe('the write gate is unmoved by the widening', () => {
159+
function makeGatedEngine(allowWrites: boolean, objWritable: boolean) {
160+
const engine = new ObjectQL();
161+
engine.registerDriver(makeDriver('default'), true);
162+
engine.registerDriver(makeDriver('warehouse'));
163+
// Carries a credentialsRef in every case — the widened key must be inert
164+
// to Gate 3, which reads schemaMode + allowWrites and nothing else.
165+
engine.registerDatasourceDef({
166+
name: 'warehouse',
167+
schemaMode: 'external',
168+
external: { allowWrites, credentialsRef: REF },
169+
});
170+
engine.registerApp({
171+
id: 'wh_gate_pkg',
172+
name: 'Warehouse',
173+
objects: [{
174+
name: 'wh_order',
175+
datasource: 'warehouse',
176+
external: { remoteName: 'fact_orders', writable: objWritable },
177+
fields: { order_id: { type: 'text' } },
178+
}],
179+
});
180+
return engine;
181+
}
182+
183+
it('still refuses a write without the double opt-in, with the ADR-0112 envelope intact', async () => {
184+
const engine = makeGatedEngine(false, true);
185+
// The envelope, not merely "it threw": a driver throwing a bare Error
186+
// would satisfy `toThrow()` and tell us nothing about the gate.
187+
const err = await engine.insert('wh_order', { order_id: 'o1' }).then(
188+
() => { throw new Error('insert resolved — the write gate did not fire'); },
189+
(e: unknown) => e,
190+
);
191+
expect(err).toBeInstanceOf(ExternalWriteForbiddenError);
192+
expect(err).toMatchObject({
193+
code: (new ExternalWriteForbiddenError()).code,
194+
status: (new ExternalWriteForbiddenError()).status,
195+
});
196+
expect((err as Error).message).toContain("datasource 'warehouse' is external");
197+
});
198+
199+
it('still allows a write when both halves opt in, credentials reference present', async () => {
200+
const engine = makeGatedEngine(true, true);
201+
await expect(engine.insert('wh_order', { order_id: 'o1' })).resolves.toBeDefined();
202+
});
203+
});
204+
});

0 commit comments

Comments
 (0)