diff --git a/.changeset/tidy-emus-relate.md b/.changeset/tidy-emus-relate.md new file mode 100644 index 00000000000..bc68e0ee4aa --- /dev/null +++ b/.changeset/tidy-emus-relate.md @@ -0,0 +1,52 @@ +--- +'@objectstack/service-datasource': patch +--- + +`os datasource introspect` now generates the authorised `*.object.ts` shape + +The Object draft rendered by `generateObjectDraft` (and served by +`POST /api/v1/datasources/:name/external/tables/:remote/draft`) used the +annotated-object-literal form. The director-seat ruling of 2026-09-12 (decision +batch #122 item 1) makes `ObjectSchema.create({ … })` the one authorised shape +for a `*.object.ts`, and the draft is destined for a committed `*.object.ts` — +the command's own `--out objects/wh_order.object.ts` example says so. Drafts +generated before this release were therefore written in the shape the platform +refuses. + +FROM → TO, for a draft you already committed — one mechanical rewrite: + +```ts +// FROM +import type { ServiceObject } from '@objectstack/spec/data'; + +const wh_order: ServiceObject = { + name: 'wh_order', + // … +}; + +export default wh_order; + +// TO +import { ObjectSchema } from '@objectstack/spec/data'; + +export const wh_order = ObjectSchema.create({ + name: 'wh_order', + // … +}); +``` + +Two things change beyond the wrapper. The spec import is now a **value** +import, because the factory runs when the file is evaluated — an `import type` +would be elided and the module would throw on its own first line. And the +export is **named only**: the `export default` is gone, matching the barrel the +scaffolder writes (`export { X } from './x.object.js'`). A barrel that imported +the draft as a default (`import X from './x.object.js'`) becomes +`import { X } from './x.object.js'`. + +Regenerating the draft is the other route: re-run +`os datasource introspect --table --out `. + +The two authored comment blocks the draft carries — the remote-primary-key note +and the ADR-0028 unprefixed-name TODO — are unchanged. + +Clause-②: no diff --git a/packages/services/service-datasource/src/__tests__/external-object-draft-authorised-shape.test.ts b/packages/services/service-datasource/src/__tests__/external-object-draft-authorised-shape.test.ts new file mode 100644 index 00000000000..6e42799bd9f --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/external-object-draft-authorised-shape.test.ts @@ -0,0 +1,205 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The generated draft is emitted in the ONE authorised `*.object.ts` shape. + * + * Director-seat ruling, decision batch #122 item 1, maintainer 「同意」 + * 2026-09-12, verbatim: + * + * > `ObjectSchema.create({ … })` is the one authorised shape for a + * > `.object.ts` … The factory parses the object against `ObjectSchema` when + * > the file is evaluated, so an error surfaces where it was written; the typed + * > literal defers everything to a build the author may never run. + * + * `renderObjectSource` wrote the other one — `const X: ServiceObject = { … }` + * behind an `import type`, closed by `export default X`. Its own docblock calls + * the output a `*.object.ts`, and `os datasource introspect --out + * objects/x.object.ts` tells the author to commit it under that suffix, so the + * bytes really do land where the ruling governs. + * + * ## Why this file is not one `toContain` line + * + * The failure mode a shape assertion invites is a generator that emits the + * right-LOOKING call around a definition the factory refuses — the defect moved + * one layer down, with every string assertion still green. Both halves are + * therefore pinned separately, and they measure different things: + * + * 1. **the form** — the value import, the named export bound to + * `ObjectSchema.create(`, and the ABSENCE of the refused annotated-literal + * form. A shape pin that only asserts the new spelling cannot say the old + * one left. + * 2. **the round-trip** — the emitted module body is EVALUATED with the real + * `ObjectSchema` from `@objectstack/spec/data`, which is the same factory + * call the committed file makes on the author's machine. Nothing is + * re-spelled here: if `create()` would throw in the author's file, it throws + * in this test. + * + * ## Why the evaluation instrument carries its own negative control + * + * An evaluation harness that silently stopped running the factory — a transform + * that no longer matched, an import that resolved to a stub — would report + * every draft as valid forever. The last block feeds it a source with one + * unknown top-level key spliced in and requires it to throw. A round-trip that + * cannot fail is not a measurement. + */ + +import { describe, it, expect } from 'vitest'; +import type { IntrospectedSchema } from '@objectstack/spec/contracts'; +import { ObjectSchema } from '@objectstack/spec/data'; +import { + ExternalDatasourceService, + type DatasourceLike, +} from '../external-datasource-service.js'; + +function remoteSchema(): IntrospectedSchema { + return { + dialect: 'postgres', + introspectedAt: '2026-09-23T00:00:00.000Z', + tables: { + 'mart.customers': { + name: 'mart.customers', + indexes: [], + columns: [ + { name: 'id', type: 'text', nullable: false, primaryKey: true }, + { name: 'name', type: 'varchar(255)', nullable: true, primaryKey: false }, + { name: 'signed_up_at', type: 'timestamptz', nullable: true, primaryKey: false }, + ], + }, + }, + }; +} + +function serviceWith(namespace?: string): ExternalDatasourceService { + return new ExternalDatasourceService({ + introspect: async () => remoteSchema(), + getDatasource: async (name): Promise => ({ name, schemaMode: 'external' }), + getObject: async () => undefined, + listObjects: async () => [], + getNamespace: () => namespace, + }); +} + +const draftFor = (namespace?: string) => + serviceWith(namespace).generateObjectDraft('warehouse', 'customers'); + +/** + * Evaluate the emitted module body and return what its single export is bound + * to — i.e. run the author's own `ObjectSchema.create(…)` call. + * + * The transform is asserted rather than assumed: a silently non-matching + * `replace` would hand `new Function` a body with no `return` in it, which + * evaluates to `undefined` and throws nothing at all. + */ +function evaluateEmittedModule(source: string): unknown { + const body = source + .split('\n') + .filter((line) => !line.startsWith('import ')) + .join('\n') + .replace(/^export const [A-Za-z_$][\w$]* = /m, 'return '); + expect(body, 'the emitted module has no single named export to evaluate').toContain( + 'return ObjectSchema.create(', + ); + return new Function('ObjectSchema', body)(ObjectSchema) as unknown; +} + +describe('the emitted draft carries the authorised `ObjectSchema.create` shape', () => { + it('imports the factory as a VALUE and binds a single named export to its call', async () => { + const draft = await draftFor('wh'); + + expect(draft.source).toContain("import { ObjectSchema } from '@objectstack/spec/data';"); + expect(draft.source).toContain('export const wh_customers = ObjectSchema.create({'); + // The call is closed as a call, not as a bare object literal. + expect(draft.source.trimEnd().endsWith('});')).toBe(true); + }); + + it('carries the shape on the no-namespace path too, TODO block and all', async () => { + const draft = await draftFor(undefined); + + // The TODO block renders ABOVE the import; the shape must survive it. + expect(draft.source).toContain('TODO(namespace)'); + expect(draft.source).toContain("import { ObjectSchema } from '@objectstack/spec/data';"); + expect(draft.source).toContain('export const customers = ObjectSchema.create({'); + }); + + it('emits a value import — an `import type` would be elided and the file would throw', async () => { + const draft = await draftFor('wh'); + expect(draft.source).not.toContain('import type'); + }); +}); + +describe('the refused annotated-literal form is absent, by name', () => { + it.each([ + ['the `ServiceObject` type annotation', ': ServiceObject = {'], + ['the type-only spec import', "import type { ServiceObject } from '@objectstack/spec/data';"], + ['the unexported `const` binding', 'const wh_customers: ServiceObject'], + ['the default export that closed it', 'export default'], + ])('%s is gone', async (_label, refused) => { + const draft = await draftFor('wh'); + expect(draft.source).not.toContain(refused); + }); + + it('names no `ServiceObject` type at all outside the preserved remote-key note', async () => { + const draft = await draftFor('wh'); + const mentions = draft.source + .split('\n') + .filter((l) => l.includes('ServiceObject')) + .map((l) => l.trim()); + + // The one survivor is the remote-primary-key tombstone, which explains a + // SPEC type rather than describing this file's shape. + expect(mentions).toEqual([ + "// Preserved as a COMMENT because 'ServiceObject' has no authorable key for a", + ]); + }); +}); + +describe('round-trip — the factory call inside the emitted file accepts the draft', () => { + it('evaluates without throwing and yields the definition the draft reports', async () => { + const draft = await draftFor('wh'); + const evaluated = evaluateEmittedModule(draft.source) as Record; + + expect(evaluated.name).toBe('wh_customers'); + expect(evaluated.label).toBe('Customers'); + expect(evaluated.datasource).toBe('warehouse'); + expect(evaluated.sharingModel).toBe('private'); + expect(Object.keys(evaluated.fields as Record)).toEqual([ + 'id', + 'name', + 'signed_up_at', + ]); + // `writable` is NOT emitted by the renderer — it is the schema default, + // applied because the evaluated file really parsed. Spelled out rather than + // loosened to `toMatchObject`: this key is the cheapest standing evidence + // that the factory ran instead of an object literal being handed back. + expect(evaluated.external).toEqual({ + remoteSchema: 'mart', + remoteName: 'customers', + writable: false, + }); + }); + + it('round-trips the no-namespace draft too — the TODO comment is inert to the factory', async () => { + const draft = await draftFor(undefined); + const evaluated = evaluateEmittedModule(draft.source) as Record; + expect(evaluated.name).toBe('customers'); + }); + + it('agrees with the structured definition the same draft carries', async () => { + const draft = await draftFor('wh'); + const evaluated = evaluateEmittedModule(draft.source); + // `create()` parses, so the comparison is against the parsed definition — + // the rendered file and `draft.definition` must describe one object. + expect(evaluated).toEqual(ObjectSchema.parse(draft.definition)); + }); + + it('NEGATIVE CONTROL — the evaluation really runs the factory', async () => { + const draft = await draftFor('wh'); + const poisoned = draft.source.replace( + " name: 'wh_customers',", + " name: 'wh_customers',\n workflows: [],", + ); + expect(poisoned).not.toBe(draft.source); + + expect(() => evaluateEmittedModule(poisoned)).toThrow(/workflows/); + }); +}); diff --git a/packages/services/service-datasource/src/__tests__/external-object-draft-os-build.test.ts b/packages/services/service-datasource/src/__tests__/external-object-draft-os-build.test.ts index 474335418cc..6eeecb3f68b 100644 --- a/packages/services/service-datasource/src/__tests__/external-object-draft-os-build.test.ts +++ b/packages/services/service-datasource/src/__tests__/external-object-draft-os-build.test.ts @@ -135,7 +135,7 @@ describe('defect 1 — the generated object name carries the package namespace p // …and the rendered file agrees with the structured definition. expect(draft.definition.name).toBe('wh_customers'); expect(draft.source).toContain("name: 'wh_customers'"); - expect(draft.source).toContain('const wh_customers: ServiceObject = {'); + expect(draft.source).toContain('export const wh_customers = ObjectSchema.create({'); }); it('does NOT double-prefix a remote table that already carries the namespace', async () => { @@ -227,7 +227,14 @@ describe('an absent or blank namespace must not trade one invalid draft for anot expect(draft.name).toBe('customers'); expect(draft.name.startsWith('_')).toBe(false); - expect(draft.source).not.toContain('_customers:'); + // Spelled against the binding the authorised shape emits. The old spelling + // (`'_customers:'`) read the annotated literal's `const _customers:` and + // went vacuous the moment the type annotation left the file — it matches no + // substring of `export const _customers = ObjectSchema.create({`. A bare + // `not.toContain('_customers')` cannot replace it either: the no-namespace + // TODO block legitimately renders `'_customers'`. + expect(draft.source).toContain('export const customers = ObjectSchema.create({'); + expect(draft.source).not.toContain('export const _customers'); expect(ObjectSchema.safeParse(draft.definition).success).toBe(true); }); diff --git a/packages/services/service-datasource/src/external-datasource-service.ts b/packages/services/service-datasource/src/external-datasource-service.ts index f8547d8e673..5922f8abfbe 100644 --- a/packages/services/service-datasource/src/external-datasource-service.ts +++ b/packages/services/service-datasource/src/external-datasource-service.ts @@ -247,13 +247,19 @@ const GENERATED_SHARING_MODEL = 'private'; * into the generated source — and the one place the reason is written down. * * `fields..primaryKey` is **not a key of the spec field schema**. Emitting - * it produced a `*.object.ts` the platform's own toolchain refused on both - * instruments it is annotated for: `tsc --noEmit` against `ServiceObject` + * it produced a `*.object.ts` the platform's own toolchain refused on both of + * its instruments: `tsc --noEmit` against `ServiceObject` * (`TS2353 … 'primaryKey' does not exist in type`) and * `ObjectSchema.safeParse` (`unrecognized_keys` at `["fields",""]`). So the * generator had a pinned path that produced a draft neither the compiler nor * the validator would take (#11000). * + * Both instruments still judge it, and the second one moved CLOSER: the + * emitted file is now the authorised `ObjectSchema.create({ … })` shape (see + * {@link renderObjectSource}), so the validator's verdict arrives when the + * committed module is evaluated rather than only when somebody runs a build. + * Re-emitting the key would therefore throw in the author's own file. + * * Maintainer ruling, 2026-08-22 live session (「同意所有」, item 8) — **D**: * * > `generateObjectDraft`/`renderObjectSource` stop emitting @@ -856,8 +862,26 @@ export class ExternalDatasourceService implements IExternalDatasourceService { /** * Render a reviewable `*.object.ts` source string for an object draft. * - * The output is annotated `ServiceObject`, which makes `tsc` over this string - * a complete acceptance instrument for the draft's shape — use it that way. + * The output is the ONE authorised `*.object.ts` shape: a single named export + * bound to `ObjectSchema.create({ … })` — director-seat ruling, decision batch + * #122 item 1, maintainer 「同意」 2026-09-12. Its reasoning is what the two + * halves of this renderer have to preserve: + * + * > The factory parses the object against `ObjectSchema` when the file is + * > evaluated, so an error surfaces where it was written; the typed literal + * > defers everything to a build the author may never run. + * + * ⇒ `tsc` over this string is NO LONGER the whole acceptance instrument, and a + * renderer that emitted the factory CALL around a definition the factory + * refuses would have moved the defect rather than fixed it. Both instruments + * are owed a pin: the emitted call's shape, and an evaluation of the emitted + * module body through the real `ObjectSchema.create` + * (`external-object-draft-authorised-shape.test.ts` holds both). + * + * The export is NAMED, and there is deliberately no `export default` beside + * it: the scaffolded barrel re-exports object modules by name + * (`export { X } from './x.object.js'`), and a second export form in a + * generated file is exactly the parallel shape the ruling closed. * * `namespace` is passed in rather than re-derived from `definition.name`, * because the two absent cases are NOT the same file: a name that is already @@ -926,9 +950,12 @@ function renderObjectSource( return [ `// Generated by \`os datasource introspect\` (ADR-0015). Review before committing.`, ...namespaceTodo, - `import type { ServiceObject } from '@objectstack/spec/data';`, + // A VALUE import, not a type-only one: the factory runs when the committed + // file is evaluated. `import type` here would be elided at compile time and + // the emitted module would throw on its own first line. + `import { ObjectSchema } from '@objectstack/spec/data';`, ``, - `const ${definition.name as string}: ServiceObject = {`, + `export const ${definition.name as string} = ObjectSchema.create({`, ` name: '${definition.name as string}',`, ` label: '${definition.label as string}',`, ` datasource: '${definition.datasource as string}',`, @@ -943,9 +970,7 @@ function renderObjectSource( ` // draft that omitted it could not compile. '${GENERATED_SHARING_MODEL}' is the rule's own`, ` // recommended default: owner + explicit shares. Widen it deliberately.`, ` sharingModel: '${definition.sharingModel as string}',`, - `};`, - ``, - `export default ${definition.name as string};`, + `});`, ``, ].join('\n'); }