From 65be005c32a183f619741824dc0e46aaa9bce4f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 03:32:33 +0000 Subject: [PATCH 1/4] feat(spec): composeStacks judges a datasource's external.connector against the connectors of every input The composed counterpart of the per-stack check (ADR-0015 amendment D7): an input built with strict: false carried a dangling connector name, a descriptor or an instance with nothing to lend into the artifact with no verdict. Also three doc-only should-fixes in injected-system-column-provenance and the IDataEngine.listDatasourceDefs member doc. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2cLiZUHvCtV2YGRWAd --- packages/spec/src/contracts/data-engine.ts | 4 +- .../data/injected-system-column-provenance.ts | 8 +- .../src/stack-datasource-connector.test.ts | 85 ++++++++++++++++++- packages/spec/src/stack.zod.ts | 37 +++++++- 4 files changed, 128 insertions(+), 6 deletions(-) diff --git a/packages/spec/src/contracts/data-engine.ts b/packages/spec/src/contracts/data-engine.ts index 5798a9c75e5..cca21669b86 100644 --- a/packages/spec/src/contracts/data-engine.ts +++ b/packages/spec/src/contracts/data-engine.ts @@ -496,7 +496,9 @@ export interface IDataEngine { * `schemaMode`, with or without a reference — `credentialsRef` is valid on * a managed datasource too, #8153) and answer FRESH copies: a reader must * not be able to reach through this accessor and mutate the write gate's - * own input. + * own input. Each entry carries the definition's stored `driver` id, and + * omits the key for a definition that never named one (see + * {@link EngineDatasourceDef.driver}). */ listDatasourceDefs?(): EngineDatasourceDef[]; diff --git a/packages/spec/src/data/injected-system-column-provenance.ts b/packages/spec/src/data/injected-system-column-provenance.ts index db49a0f8a02..d2c1b0d1071 100644 --- a/packages/spec/src/data/injected-system-column-provenance.ts +++ b/packages/spec/src/data/injected-system-column-provenance.ts @@ -230,8 +230,9 @@ export function injectedSystemColumnDefs(def: unknown): Record { + const pluginManifest = (id: string) => ({ + id, + namespace: id.split('.').pop(), + version: '1.0.0', + type: 'plugin', + name: id.split('.').pop(), + }); + const owner = (connector: Record = instance()) => + defineStack({ manifest: pluginManifest('com.acme.sap'), connectors: [connector] } as never); + const looseBorrower = (connector = 'sap_s4') => + defineStack({ manifest: pluginManifest('com.acme.orders'), datasources: [sapOrders(connector)] } as never, { + strict: false, + }); + const composeRefusal = (stacks: unknown[], options?: Record): Envelope | undefined => { + try { + composeStacks(stacks as never, options as never); + return undefined; + } catch (err) { + return err as Envelope; + } + }; + const expectComposedFinding = (refused: Envelope | undefined, fragment: string): void => { + expectOneFinding(refused, fragment); + expect(refused?.issues).toHaveLength(1); + expect(refused?.message).toContain('composeStacks artifact cross-reference validation failed (1 issue):'); + }; + + it.each([ + ['owner first, `last`', false, 'last'], + ['borrower first, `last`', true, 'last'], + ['owner first, `preserve`', false, 'preserve'], + ['borrower first, `preserve`', true, 'preserve'], + ])('CONTROL: a datasource borrowing an instance another input declares composes — %s', (_order, reversed, manifestOpt) => { + const stacks = reversed ? [looseBorrower(), owner()] : [owner(), looseBorrower()]; + expect(composeRefusal(stacks, { manifest: manifestOpt })).toBeUndefined(); + }); + + it.each(['last', 'preserve'])('refuses a dangling name a `strict: false` input carries (`%s`)', (manifestOpt) => { + const refused = composeRefusal([owner(instance({ name: 'sap_ariba' })), looseBorrower()], { manifest: manifestOpt }); + expectComposedFinding(refused, 'which is not defined in connectors'); + }); + + it('refuses a descriptor, and an instance with nothing to lend, that another input declares', () => { + const descriptor = { name: 'sap_s4', label: 'SAP S/4HANA', type: 'api' }; + expectComposedFinding(composeRefusal([owner(descriptor), looseBorrower()]), 'which declares no provider'); + expectComposedFinding( + composeRefusal([owner(instance({ auth: { type: 'none' } })), looseBorrower()]), + 'which declares no auth credential', + ); + }); + + it('judges a name two inputs declare against the later entry, the one the runtime keeps', () => { + const strictPair = defineStack({ + manifest: pluginManifest('com.acme.orders'), + datasources: [sapOrders()], + connectors: [instance()], + } as never); + const descriptor = { name: 'sap_s4', label: 'SAP S/4HANA', type: 'api' }; + expectComposedFinding(composeRefusal([strictPair, owner(descriptor)]), 'which declares no provider'); + expect(composeRefusal([owner(descriptor), strictPair])).toBeUndefined(); + }); + + it('CONTROL: the per-stack strict refusal is unchanged — a strict input never carries a dangling name in', () => { + const refused = refusalOf({ manifest: pluginManifest('com.acme.orders'), datasources: [sapOrders()] }); + expectOneFinding(refused, 'which is not defined in connectors'); + expect(refused?.message).not.toContain('composeStacks'); + }); +}); diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index 7beb89b85e6..05bcdb917de 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -4397,7 +4397,9 @@ function collectExternalBindingDatasourceErrors(config: ObjectStackDefinition): * no connection record to share. A disabled instance (`enabled: false`) is * accepted: the switch turns the vendor off for its actions and its records * alike, which is the one-grant behaviour D7 asks for, not a dangling name. - * Runs before the object-count early return: it needs no object. + * Runs before the object-count early return: it needs no object. A + * composition runs it once more over every input's datasources and connectors + * ({@link collectComposedDatasourceConnectorErrors}). */ function collectDatasourceConnectorErrors(config: ObjectStackDefinition): string[] { const errors: string[] = []; @@ -4470,6 +4472,35 @@ function collectComposedExternalBindingDatasourceErrors( return collectExternalBindingDatasourceErrors({ objects: composedObjects, datasources } as ObjectStackDefinition); } +/** + * [ADR-0015 amendment D7] {@link collectDatasourceConnectorErrors} over a + * COMPOSITION: a datasource's `external.connector` against the `connectors` of + * every input. + * + * `defineStack` judges the reference against its own stack only, and an input + * built with `strict: false` never reaches that pass, so a dangling name it + * carries reached the composed artifact with no verdict: the datasource has no + * credential to borrow, and the author learns of the typo when it first + * connects instead of when the artifact is built. Judged here over the `datasources` and the `connectors` of every input, by + * the same collector, so the three findings and their fixes read exactly as + * the per-stack ones do. + * + * What this can newly refuse: a dangling name, a descriptor or an instance + * with nothing to lend, carried by an input built with `strict: false`. A + * cross-input pair resolves (a datasource in one input borrowing an instance + * another declares), because the composed artifact registers both. A + * connector name two inputs declare is judged against the later declaration + * (the collector keeps the last one it meets), which is the entry the runtime + * keeps ({@link SAME_STACK_ENTRY_IDENTITY}'s `connectors` row), so a strict + * input whose own instance a later input replaces with a descriptor is + * refused too: the instance it borrowed from is not the one that boots. + */ +function collectComposedDatasourceConnectorErrors(stacks: readonly ObjectStackDefinition[]): string[] { + const datasources = stacks.flatMap((stack) => (Array.isArray(stack.datasources) ? stack.datasources : [])); + const connectors = stacks.flatMap((stack) => (Array.isArray(stack.connectors) ? stack.connectors : [])); + return collectDatasourceConnectorErrors({ datasources, connectors } as ObjectStackDefinition); +} + /** * Perform strict cross-reference validation on a parsed stack definition. * Returns an array of error messages (empty if valid). @@ -7489,6 +7520,9 @@ function composeBuiltStacks( // [ADR-0015 §4.2 · #25211] …and an `external` object of one input bound // to a datasource another input declares managed: see // {@link collectComposedExternalBindingDatasourceErrors}. + // [ADR-0015 amendment D7] …and a datasource's `external.connector` + // against the connectors of every input: see + // {@link collectComposedDatasourceConnectorErrors}. // [#25234] …and one membership key declared on two objects of the // composed artifact: see {@link collectComposedMembershipKeyErrors}. // [#25514] …and every flow target an input resolved only through @@ -7501,6 +7535,7 @@ function composeBuiltStacks( ...collectComposedDefaultRecordPageErrors(stacks, opts.manifest === 'preserve'), ...collectComposedMembershipKeyErrors(stacks, objects as readonly unknown[] | undefined), ...collectComposedExternalBindingDatasourceErrors(stacks, objects), + ...collectComposedDatasourceConnectorErrors(stacks), ...collectArtifactFlowClaimErrors(stacks), ]; if (artifactCrossRefErrors.length > 0) { From 2a07c4fcd617aa4de5e0a92cfc571e0ca7b05474 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 03:33:05 +0000 Subject: [PATCH 2/4] chore(spec): changeset for the composed external.connector check Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2cLiZUHvCtV2YGRWAd --- .../27803-composed-datasource-connector.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 .changeset/27803-composed-datasource-connector.md diff --git a/.changeset/27803-composed-datasource-connector.md b/.changeset/27803-composed-datasource-connector.md new file mode 100644 index 00000000000..cdd8e336f7b --- /dev/null +++ b/.changeset/27803-composed-datasource-connector.md @@ -0,0 +1,18 @@ +--- +'@objectstack/spec': patch +--- + +fix(spec)!: `composeStacks` refuses a datasource whose `external.connector` names no connector instance the composed inputs declare (#27803) + +Clause-②: yes (narrowing) +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing in `composeStacks` (and so in `os build` on a multi-package artifact). + +- **Before.** `defineStack` judged a datasource's `external.connector` against its own stack's `connectors` only, and an input built with `strict: false` skipped that judgement. Composing such an input carried a dangling name, a catalog descriptor (no `provider`) or an instance with no `auth` into the artifact with no verdict. The datasource then had no credential to borrow, found only when it first connected. +- **Now.** The composition's artifact pass judges every input's datasources against the connectors of every input, with the same three findings as the per-stack check, under the `STACK_CROSS_REFERENCE_INVALID` envelope (422). A datasource in one input that borrows an instance another input declares composes. A connector name two inputs declare is judged against the later entry, which is the one the runtime keeps. +- **Unchanged.** A strict `defineStack` refuses a dangling name in its own stack as before, so an artifact built from strict inputs that each resolve their own references is refused only when a later input replaces the borrowed instance under the same name. + +**The remedy.** Declare the connector instance the datasource borrows (a `connectors` entry with a `provider` and an `auth` credential) in one of the composed packages, or correct the name. If the upstream needs no credential, drop `external.connector` from the datasource. From 489498a45ca928e94ea4dbded9eb58fc1adf03c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 05:18:43 +0000 Subject: [PATCH 3/4] fix(spec): the composed external.connector check reads nested package bodies and names a shadowed instance A multi-package input carries its connectors and datasources only in its package bodies, so reading the top level alone refused an instance the input carries. A connector declared twice whose kept entry cannot lend now names both declarations and prescribes the duplicate's fix. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2cLiZUHvCtV2YGRWAd --- .../27803-composed-datasource-connector.md | 4 +- .../src/stack-datasource-connector.test.ts | 60 +++++++- packages/spec/src/stack.zod.ts | 132 +++++++++++++++--- 3 files changed, 170 insertions(+), 26 deletions(-) diff --git a/.changeset/27803-composed-datasource-connector.md b/.changeset/27803-composed-datasource-connector.md index cdd8e336f7b..704ef9bd7ca 100644 --- a/.changeset/27803-composed-datasource-connector.md +++ b/.changeset/27803-composed-datasource-connector.md @@ -9,10 +9,10 @@ Metadata-protocol: none -**BREAKING** accept-set narrowing in `composeStacks` (and so in `os build` on a multi-package artifact). +**BREAKING** accept-set narrowing in `composeStacks`, and so in every CLI command that loads a project whose config composes two or more packages (`os build`, `os validate`, `os lint`, `os dev`, `os serve`). - **Before.** `defineStack` judged a datasource's `external.connector` against its own stack's `connectors` only, and an input built with `strict: false` skipped that judgement. Composing such an input carried a dangling name, a catalog descriptor (no `provider`) or an instance with no `auth` into the artifact with no verdict. The datasource then had no credential to borrow, found only when it first connected. -- **Now.** The composition's artifact pass judges every input's datasources against the connectors of every input, with the same three findings as the per-stack check, under the `STACK_CROSS_REFERENCE_INVALID` envelope (422). A datasource in one input that borrows an instance another input declares composes. A connector name two inputs declare is judged against the later entry, which is the one the runtime keeps. +- **Now.** The composition's artifact pass judges every input's datasources against the connectors of every input, with the same three findings as the per-stack check, under the `STACK_CROSS_REFERENCE_INVALID` envelope (422). A datasource in one input that borrows an instance another input declares composes. An input that is itself a multi-package artifact is read through its package bodies, where its connectors and datasources live. A connector name declared more than once is judged against the later entry, which is the one the runtime keeps; when an earlier declaration could have lent, the finding names both and asks for the duplicate to be renamed or removed. - **Unchanged.** A strict `defineStack` refuses a dangling name in its own stack as before, so an artifact built from strict inputs that each resolve their own references is refused only when a later input replaces the borrowed instance under the same name. **The remedy.** Declare the connector instance the datasource borrows (a `connectors` entry with a `provider` and an `auth` credential) in one of the composed packages, or correct the name. If the upstream needs no credential, drop `external.connector` from the datasource. diff --git a/packages/spec/src/stack-datasource-connector.test.ts b/packages/spec/src/stack-datasource-connector.test.ts index 13ff6443ef1..3fe5efdd040 100644 --- a/packages/spec/src/stack-datasource-connector.test.ts +++ b/packages/spec/src/stack-datasource-connector.test.ts @@ -24,7 +24,10 @@ * by an input built with `strict: false`, is refused with the same finding * and envelope, under `last` and `preserve`; * - a connector name a later input re-declares is judged against that later - * entry, the one the runtime keeps; + * entry, the one the runtime keeps, and that finding names both + * declarations and prescribes the duplicate's fix; + * - a nested multi-package input (a `manifest: 'preserve'` artifact) is read + * through its package bodies, where its connectors and datasources live; * - CONTROL: a datasource borrowing an instance another input declares * composes, in either order; * - CONTROL: the per-stack strict refusal is unchanged, so a strict input @@ -190,10 +193,63 @@ describe('composeStacks judges external.connector against the connectors of ever connectors: [instance()], } as never); const descriptor = { name: 'sap_s4', label: 'SAP S/4HANA', type: 'api' }; - expectComposedFinding(composeRefusal([strictPair, owner(descriptor)]), 'which declares no provider'); + const refused = composeRefusal([strictPair, owner(descriptor)]); + expectComposedFinding(refused, 'which declares no provider in the entry the composed artifact keeps'); + expect(String(refused?.issues?.[0])).toContain( + "Connector 'sap_s4' is declared more than once: 'com.acme.orders' (stack #0) declares an instance " + + "that can lend, and 'com.acme.sap' (stack #1) declares the later entry, which the runtime keeps. " + + 'Rename or remove the later entry', + ); + expect(String(refused?.issues?.[0])).not.toContain('Give it the provider'); expect(composeRefusal([owner(descriptor), strictPair])).toBeUndefined(); }); + it('keeps the per-stack prescription when no declaration of the name could lend', () => { + const descriptor = { name: 'sap_s4', label: 'SAP S/4HANA', type: 'api' }; + const refused = composeRefusal([owner(descriptor), owner(instance({ auth: { type: 'none' } })), looseBorrower()]); + expectComposedFinding(refused, 'which declares no auth credential — there is nothing to borrow. Declare the'); + }); + + describe('a nested multi-package input, whose collections live only in its package bodies', () => { + const other = () => defineStack({ manifest: pluginManifest('com.acme.other') } as never); + const nested = (...stacks: unknown[]) => composeStacks([...stacks, other()] as never, { manifest: 'preserve' }); + + it('CONTROL: a borrower resolves against an instance a package body of the nested input carries', () => { + const inner = nested(owner()); + expect((inner as Record).connectors).toBeUndefined(); + expect(composeRefusal([inner, looseBorrower()], { manifest: 'preserve' })).toBeUndefined(); + expect(composeRefusal([looseBorrower(), inner])).toBeUndefined(); + }); + + it("judges a datasource a nested input's package body carries", () => { + // Hand-assembled: a composition would refuse the dangling name itself. + const inner = defineStack( + { + manifest: pluginManifest('com.acme.bundle'), + packages: [{ manifest: { ...pluginManifest('com.acme.orders'), datasources: [sapOrders('sap_missing')] } }], + } as never, + { strict: false }, + ); + const refused = composeRefusal([owner(), inner], { manifest: 'preserve' }); + expect({ code: refused?.code, status: refused?.status }).toEqual({ code: 'STACK_CROSS_REFERENCE_INVALID', status: 422 }); + expect(refused?.issues).toEqual([ + expect.stringMatching( + /^Datasource 'sap_orders' borrows the credential of connector 'sap_missing' \(external\.connector\), which is not defined in connectors\./, + ), + ]); + }); + + it('names the package body that declares a shadowed instance', () => { + const strictPair = defineStack({ + manifest: pluginManifest('com.acme.orders'), + datasources: [sapOrders()], + connectors: [instance()], + } as never); + const refused = composeRefusal([nested(strictPair), owner({ name: 'sap_s4', label: 'SAP', type: 'api' })]); + expect(String(refused?.issues?.[0])).toContain("package 'com.acme.orders' of 'com.acme.other' (stack #0)"); + }); + }); + it('CONTROL: the per-stack strict refusal is unchanged — a strict input never carries a dangling name in', () => { const refused = refusalOf({ manifest: pluginManifest('com.acme.orders'), datasources: [sapOrders()] }); expectOneFinding(refused, 'which is not defined in connectors'); diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index 05bcdb917de..3060b1e7e1c 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -4399,9 +4399,15 @@ function collectExternalBindingDatasourceErrors(config: ObjectStackDefinition): * alike, which is the one-grant behaviour D7 asks for, not a dangling name. * Runs before the object-count early return: it needs no object. A * composition runs it once more over every input's datasources and connectors - * ({@link collectComposedDatasourceConnectorErrors}). + * ({@link collectComposedDatasourceConnectorErrors}), passing `shadowedLender`: + * for a name declared more than once whose kept entry cannot lend, it returns + * the fix to prescribe instead (the duplicate), or `undefined` when no earlier + * declaration could have lent either. */ -function collectDatasourceConnectorErrors(config: ObjectStackDefinition): string[] { +function collectDatasourceConnectorErrors( + config: ObjectStackDefinition, + shadowedLender?: (name: string) => string | undefined, +): string[] { const errors: string[] = []; const datasources = Array.isArray(config.datasources) ? config.datasources : []; const borrowing = datasources.filter( @@ -4424,27 +4430,51 @@ function collectDatasourceConnectorErrors(config: ObjectStackDefinition): string ); continue; } - if (typeof entry.provider !== 'string' || entry.provider.length === 0) { + // The one door that sees a name declared twice: the later entry is the one + // the runtime keeps, so an earlier instance that could lend is shadowed, + // and the fix is the duplicate, not the entry's provider or auth. + const shadow = connectorCanLend(entry) ? undefined : shadowedLender?.(name); + if (!connectorHasProvider(entry)) { errors.push( - `${subject}, which declares no provider — a connector entry with no provider is a catalog ` + - `descriptor, never materialized, and holds no credential to lend (ADR-0097). Give it the ` + - `provider that serves this vendor and an auth credential, or name a connector instance ` + - `that has one.`, + shadow !== undefined + ? `${subject}, which declares no provider in the entry the composed artifact keeps — a catalog ` + + `descriptor that holds no credential to lend (ADR-0097). ${shadow}` + : `${subject}, which declares no provider — a connector entry with no provider is a catalog ` + + `descriptor, never materialized, and holds no credential to lend (ADR-0097). Give it the ` + + `provider that serves this vendor and an auth credential, or name a connector instance ` + + `that has one.`, ); continue; } - const auth = entry.auth; - if (!isRecord(auth) || auth.type === 'none') { + if (!connectorHasCredential(entry)) { errors.push( - `${subject}, which declares no auth credential — there is nothing to borrow. Declare the ` + - `instance's auth (e.g. auth: { type: 'bearer', credentialRef }), or, if the upstream ` + - `needs no credential, remove external.connector from the datasource.`, + shadow !== undefined + ? `${subject}, which declares no auth credential in the entry the composed artifact keeps — ` + + `there is nothing to borrow. ${shadow}` + : `${subject}, which declares no auth credential — there is nothing to borrow. Declare the ` + + `instance's auth (e.g. auth: { type: 'bearer', credentialRef }), or, if the upstream ` + + `needs no credential, remove external.connector from the datasource.`, ); } } return errors; } +/** A connector entry with a `provider`: an instance, not a catalog descriptor (ADR-0097). */ +function connectorHasProvider(entry: Record): boolean { + return typeof entry.provider === 'string' && entry.provider.length > 0; +} + +/** A connector entry whose `auth` holds a credential to lend: present, and not `type: 'none'`. */ +function connectorHasCredential(entry: Record): boolean { + return isRecord(entry.auth) && entry.auth.type !== 'none'; +} + +/** A connector entry a datasource's `external.connector` can borrow from. */ +function connectorCanLend(entry: Record): boolean { + return connectorHasProvider(entry) && connectorHasCredential(entry); +} + /** * [ADR-0015 §4.2 · #25211] {@link collectExternalBindingDatasourceErrors} over * a COMPOSITION: an `external` object of one input bound to a datasource @@ -4472,6 +4502,37 @@ function collectComposedExternalBindingDatasourceErrors( return collectExternalBindingDatasourceErrors({ objects: composedObjects, datasources } as ObjectStackDefinition); } +/** + * Every entry of collection `key` that one composition input provides, each + * labelled with where it was declared: the input's own top level, plus each + * `packages[]` body when the input is itself a multi-package artifact, whose + * collections live ONLY in those bodies (#14512) — the + * {@link providedFlowNames} reading, for the same reason: the top level alone + * would refuse an entry the input carries. + */ +function providedCollectionEntries( + stack: ObjectStackDefinition, + index: number, + key: 'connectors' | 'datasources', +): { entry: unknown; declaredIn: string }[] { + const entries: { entry: unknown; declaredIn: string }[] = []; + const input = stackLabel(stack, index); + const collect = (list: unknown, declaredIn: string): void => { + if (!Array.isArray(list)) return; + for (const entry of list) entries.push({ entry, declaredIn }); + }; + const source = stack as Record; + collect(source[key], input); + if (Array.isArray(source.packages)) { + for (const pkg of source.packages) { + if (!isRecord(pkg) || !isRecord(pkg.manifest)) continue; + const id = pkg.manifest.id ?? pkg.manifest.name; + collect(pkg.manifest[key], typeof id === 'string' ? `package '${id}' of ${input}` : input); + } + } + return entries; +} + /** * [ADR-0015 amendment D7] {@link collectDatasourceConnectorErrors} over a * COMPOSITION: a datasource's `external.connector` against the `connectors` of @@ -4481,24 +4542,51 @@ function collectComposedExternalBindingDatasourceErrors( * built with `strict: false` never reaches that pass, so a dangling name it * carries reached the composed artifact with no verdict: the datasource has no * credential to borrow, and the author learns of the typo when it first - * connects instead of when the artifact is built. Judged here over the `datasources` and the `connectors` of every input, by - * the same collector, so the three findings and their fixes read exactly as - * the per-stack ones do. + * connects instead of when the artifact is built. Judged here over the + * `datasources` and the `connectors` of every input, top level and package + * bodies alike ({@link providedCollectionEntries}), by the same collector, so + * the three findings and their fixes read exactly as the per-stack ones do. * * What this can newly refuse: a dangling name, a descriptor or an instance * with nothing to lend, carried by an input built with `strict: false`. A * cross-input pair resolves (a datasource in one input borrowing an instance - * another declares), because the composed artifact registers both. A - * connector name two inputs declare is judged against the later declaration - * (the collector keeps the last one it meets), which is the entry the runtime + * another declares, or one of a nested multi-package input's package bodies + * declares), because the composed artifact registers both. A connector name + * declared more than once is judged against the last declaration (the + * collector keeps the last one it meets), which is the entry the runtime * keeps ({@link SAME_STACK_ENTRY_IDENTITY}'s `connectors` row), so a strict * input whose own instance a later input replaces with a descriptor is - * refused too: the instance it borrowed from is not the one that boots. + * refused too: the instance it borrowed from is not the one that boots. That + * finding names both declarations and prescribes the duplicate's fix, since + * the artifact already carries an instance that could lend. */ function collectComposedDatasourceConnectorErrors(stacks: readonly ObjectStackDefinition[]): string[] { - const datasources = stacks.flatMap((stack) => (Array.isArray(stack.datasources) ? stack.datasources : [])); - const connectors = stacks.flatMap((stack) => (Array.isArray(stack.connectors) ? stack.connectors : [])); - return collectDatasourceConnectorErrors({ datasources, connectors } as ObjectStackDefinition); + const datasources: unknown[] = []; + const connectors: unknown[] = []; + const declarations = new Map; declaredIn: string }[]>(); + for (const [i, stack] of stacks.entries()) { + for (const { entry } of providedCollectionEntries(stack, i, 'datasources')) datasources.push(entry); + for (const { entry, declaredIn } of providedCollectionEntries(stack, i, 'connectors')) { + connectors.push(entry); + if (!isRecord(entry) || typeof entry.name !== 'string') continue; + const named = declarations.get(entry.name) ?? []; + named.push({ entry, declaredIn }); + declarations.set(entry.name, named); + } + } + const shadowedLender = (name: string): string | undefined => { + const named = declarations.get(name) ?? []; + const kept = named[named.length - 1]; + const lender = named.slice(0, -1).reverse().find(({ entry }) => connectorCanLend(entry)); + if (kept === undefined || lender === undefined) return undefined; + return ( + `Connector '${name}' is declared more than once: ${lender.declaredIn} declares an instance that ` + + `can lend, and ${kept.declaredIn} declares the later entry, which the runtime keeps. Rename or ` + + `remove the later entry, or give one of the two its own name and point external.connector at ` + + `the instance.` + ); + }; + return collectDatasourceConnectorErrors({ datasources, connectors } as ObjectStackDefinition, shadowedLender); } /** From 0300a14c57caa9a40ab2064447e91cf2789c5d66 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 05:52:20 +0000 Subject: [PATCH 4/4] fix(spec): judge a duplicated connector name per registration slot in the composed external.connector check The registry keeps one connector entry per package, and the lender answers the first registered, so last-wins holds only within one slot. Two packages that disagree on whether their entry can lend are refused as ambiguous in either order. An entry a nested input carries in both halves is read once. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_017ihL2cLiZUHvCtV2YGRWAd --- .../27803-composed-datasource-connector.md | 4 +- .../src/stack-datasource-connector.test.ts | 70 ++++++- packages/spec/src/stack.zod.ts | 176 +++++++++++++----- 3 files changed, 192 insertions(+), 58 deletions(-) diff --git a/.changeset/27803-composed-datasource-connector.md b/.changeset/27803-composed-datasource-connector.md index 704ef9bd7ca..04ae0a73ee6 100644 --- a/.changeset/27803-composed-datasource-connector.md +++ b/.changeset/27803-composed-datasource-connector.md @@ -12,7 +12,7 @@ Metadata-protocol: none **BREAKING** accept-set narrowing in `composeStacks`, and so in every CLI command that loads a project whose config composes two or more packages (`os build`, `os validate`, `os lint`, `os dev`, `os serve`). - **Before.** `defineStack` judged a datasource's `external.connector` against its own stack's `connectors` only, and an input built with `strict: false` skipped that judgement. Composing such an input carried a dangling name, a catalog descriptor (no `provider`) or an instance with no `auth` into the artifact with no verdict. The datasource then had no credential to borrow, found only when it first connected. -- **Now.** The composition's artifact pass judges every input's datasources against the connectors of every input, with the same three findings as the per-stack check, under the `STACK_CROSS_REFERENCE_INVALID` envelope (422). A datasource in one input that borrows an instance another input declares composes. An input that is itself a multi-package artifact is read through its package bodies, where its connectors and datasources live. A connector name declared more than once is judged against the later entry, which is the one the runtime keeps; when an earlier declaration could have lent, the finding names both and asks for the duplicate to be renamed or removed. -- **Unchanged.** A strict `defineStack` refuses a dangling name in its own stack as before, so an artifact built from strict inputs that each resolve their own references is refused only when a later input replaces the borrowed instance under the same name. +- **Now.** The composition's artifact pass judges every input's datasources against the connectors of every input, with the same three findings as the per-stack check, under the `STACK_CROSS_REFERENCE_INVALID` envelope (422). A datasource in one input that borrows an instance another input declares composes. An input that is itself a multi-package artifact is read through its package bodies, where its connectors and datasources live. A connector name declared twice in the flattened artifact is judged against the later entry, which is the one the registry keeps; when the earlier declaration could have lent, the finding names both and asks for the duplicate to be renamed or removed. A connector name that two packages of the artifact declare is kept twice, and the first one registered lends, so when one of the two can lend and the other cannot, the borrow is refused as ambiguous in either order. +- **Unchanged.** A strict `defineStack` refuses a dangling name in its own stack as before, so an artifact built from strict inputs that each resolve their own references is refused only when another input declares the borrowed connector name with an entry that cannot lend. **The remedy.** Declare the connector instance the datasource borrows (a `connectors` entry with a `provider` and an `auth` credential) in one of the composed packages, or correct the name. If the upstream needs no credential, drop `external.connector` from the datasource. diff --git a/packages/spec/src/stack-datasource-connector.test.ts b/packages/spec/src/stack-datasource-connector.test.ts index 3fe5efdd040..3b192068478 100644 --- a/packages/spec/src/stack-datasource-connector.test.ts +++ b/packages/spec/src/stack-datasource-connector.test.ts @@ -23,11 +23,17 @@ * - a dangling name, a descriptor or an instance with nothing to lend, carried * by an input built with `strict: false`, is refused with the same finding * and envelope, under `last` and `preserve`; - * - a connector name a later input re-declares is judged against that later - * entry, the one the runtime keeps, and that finding names both - * declarations and prescribes the duplicate's fix; + * - a connector name declared twice in one registration slot (the flattened + * artifact's top level) is judged against the later entry, the one the + * registry keeps, and that finding names both declarations and prescribes + * the duplicate's fix; + * - a connector name two PACKAGES declare (two slots: the registry keeps + * both, and the first registered lends) is refused as ambiguous in either + * order when one entry can lend and the other cannot, and accepted when + * both can; * - a nested multi-package input (a `manifest: 'preserve'` artifact) is read - * through its package bodies, where its connectors and datasources live; + * through its package bodies, where its connectors and datasources live, + * and an entry it carries in both halves is read once; * - CONTROL: a datasource borrowing an instance another input declares * composes, in either order; * - CONTROL: the per-stack strict refusal is unchanged, so a strict input @@ -186,7 +192,7 @@ describe('composeStacks judges external.connector against the connectors of ever ); }); - it('judges a name two inputs declare against the later entry, the one the runtime keeps', () => { + it('judges a name two inputs declare in the flattened artifact against the later entry, the one the registry keeps', () => { const strictPair = defineStack({ manifest: pluginManifest('com.acme.orders'), datasources: [sapOrders()], @@ -204,6 +210,36 @@ describe('composeStacks judges external.connector against the connectors of ever expect(composeRefusal([owner(descriptor), strictPair])).toBeUndefined(); }); + describe('a name two packages of a `preserve` artifact declare: the registry keeps both, the first registered lends', () => { + const strictPair = () => + defineStack({ + manifest: pluginManifest('com.acme.orders'), + datasources: [sapOrders()], + connectors: [instance()], + } as never); + const descriptor = { name: 'sap_s4', label: 'SAP S/4HANA', type: 'api' }; + + it.each([ + ['instance first', false], + ['descriptor first', true], + ])('refuses the borrow as ambiguous when one can lend and the other cannot — %s', (_order, reversed) => { + const stacks = reversed ? [owner(descriptor), strictPair()] : [strictPair(), owner(descriptor)]; + const refused = composeRefusal(stacks, { manifest: 'preserve' }); + expectComposedFinding(refused, 'which more than one package of the composed artifact declares, and they disagree'); + const finding = String(refused?.issues?.[0]); + expect(finding).toContain("'com.acme.orders' (stack #"); + expect(finding).toContain("declares an instance that can lend, and 'com.acme.sap' (stack #"); + expect(finding).toContain('lends from whichever registers first'); + expect(finding).not.toContain('which the runtime keeps'); + }); + + it('CONTROL: two packages that each declare an instance that can lend compose, in either order', () => { + const other = owner(instance({ auth: { type: 'bearer', credentialRef: 'sap_token_2' } })); + expect(composeRefusal([strictPair(), other], { manifest: 'preserve' })).toBeUndefined(); + expect(composeRefusal([other, strictPair()], { manifest: 'preserve' })).toBeUndefined(); + }); + }); + it('keeps the per-stack prescription when no declaration of the name could lend', () => { const descriptor = { name: 'sap_s4', label: 'SAP S/4HANA', type: 'api' }; const refused = composeRefusal([owner(descriptor), owner(instance({ auth: { type: 'none' } })), looseBorrower()]); @@ -239,14 +275,34 @@ describe('composeStacks judges external.connector against the connectors of ever ]); }); - it('names the package body that declares a shadowed instance', () => { + it("treats a package body and another input's top level as two slots, and names the body", () => { const strictPair = defineStack({ manifest: pluginManifest('com.acme.orders'), datasources: [sapOrders()], connectors: [instance()], } as never); const refused = composeRefusal([nested(strictPair), owner({ name: 'sap_s4', label: 'SAP', type: 'api' })]); - expect(String(refused?.issues?.[0])).toContain("package 'com.acme.orders' of 'com.acme.other' (stack #0)"); + expectComposedFinding(refused, 'which more than one package of the composed artifact declares, and they disagree'); + expect(String(refused?.issues?.[0])).toContain( + "package 'com.acme.orders' of 'com.acme.other' (stack #0) declares an instance that can lend", + ); + }); + + it('reads an entry the nested input carries in both halves once', () => { + // A nested artifact whose bodies did not reproduce its flattened half + // keeps both, holding the same entry objects in each. + const borrower = sapOrders('sap_missing'); + const both = defineStack( + { + manifest: pluginManifest('com.acme.bundle'), + datasources: [borrower], + packages: [{ manifest: { ...pluginManifest('com.acme.orders'), datasources: [borrower] } }], + } as never, + { strict: false }, + ); + const refused = composeRefusal([owner(), both]); + expect(refused?.issues).toHaveLength(1); + expect(refused?.message).toContain('composeStacks artifact cross-reference validation failed (1 issue):'); }); }); diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index 3060b1e7e1c..67d08be95ab 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -4400,9 +4400,9 @@ function collectExternalBindingDatasourceErrors(config: ObjectStackDefinition): * Runs before the object-count early return: it needs no object. A * composition runs it once more over every input's datasources and connectors * ({@link collectComposedDatasourceConnectorErrors}), passing `shadowedLender`: - * for a name declared more than once whose kept entry cannot lend, it returns - * the fix to prescribe instead (the duplicate), or `undefined` when no earlier - * declaration could have lent either. + * for a name declared more than once in one registration slot whose kept + * entry cannot lend, it returns the fix to prescribe instead (the duplicate), + * or `undefined` when no earlier declaration could have lent either. */ function collectDatasourceConnectorErrors( config: ObjectStackDefinition, @@ -4421,7 +4421,7 @@ function collectDatasourceConnectorErrors( for (const ds of borrowing) { const name = String((ds.external as Record).connector); const entry = connectors.get(name); - const subject = `Datasource '${String(ds.name)}' borrows the credential of connector '${name}' (external.connector)`; + const subject = connectorBorrowSubject(ds.name, name); if (entry === undefined) { errors.push( `${subject}, which is not defined in connectors. Declare the connector instance it borrows ` + @@ -4430,9 +4430,10 @@ function collectDatasourceConnectorErrors( ); continue; } - // The one door that sees a name declared twice: the later entry is the one - // the runtime keeps, so an earlier instance that could lend is shadowed, - // and the fix is the duplicate, not the entry's provider or auth. + // The one door that sees a name declared twice in one registration slot: + // the later entry is the one the runtime keeps, so an earlier instance + // that could lend is shadowed, and the fix is the duplicate, not the + // entry's provider or auth. const shadow = connectorCanLend(entry) ? undefined : shadowedLender?.(name); if (!connectorHasProvider(entry)) { errors.push( @@ -4460,6 +4461,11 @@ function collectDatasourceConnectorErrors( return errors; } +/** The subject every `external.connector` finding opens with. */ +function connectorBorrowSubject(datasource: unknown, connector: string): string { + return `Datasource '${String(datasource)}' borrows the credential of connector '${connector}' (external.connector)`; +} + /** A connector entry with a `provider`: an instance, not a catalog descriptor (ADR-0097). */ function connectorHasProvider(entry: Record): boolean { return typeof entry.provider === 'string' && entry.provider.length > 0; @@ -4502,34 +4508,62 @@ function collectComposedExternalBindingDatasourceErrors( return collectExternalBindingDatasourceErrors({ objects: composedObjects, datasources } as ObjectStackDefinition); } +/** One declaration a composition input provides, and the registration slot it boots into. */ +interface ProvidedCollectionEntry { + entry: unknown; + /** Where it was declared, for a finding: the input, or a package body of it. */ + declaredIn: string; + /** + * The registry slot the entry registers in: the registry keys a + * package-owned item by package id and name, so two entries under one name + * in one slot are one item (the later wins), and in two slots they are two. + */ + slot: string; +} + /** - * Every entry of collection `key` that one composition input provides, each - * labelled with where it was declared: the input's own top level, plus each - * `packages[]` body when the input is itself a multi-package artifact, whose - * collections live ONLY in those bodies (#14512) — the - * {@link providedFlowNames} reading, for the same reason: the top level alone - * would refuse an entry the input carries. + * Every entry of collection `key` that one composition input provides: the + * input's own top level, plus each `packages[]` body when the input is itself + * a multi-package artifact, whose collections live ONLY in those bodies + * (#14512) — the {@link providedFlowNames} reading, for the same reason: the + * top level alone would refuse an entry the input carries. + * + * Each entry object is listed once. A nested artifact whose bodies did not + * reproduce its flattened half keeps both halves, holding the same entry + * objects in each, so the bodies are read first and the top level adds only + * what no body carries. The slot follows how the artifact boots: a package + * body registers under its own id; a top-level entry registers under the + * input's own package when `preserve` folds each input into a package body, + * and under the composed artifact's one package otherwise. */ function providedCollectionEntries( stack: ObjectStackDefinition, index: number, key: 'connectors' | 'datasources', -): { entry: unknown; declaredIn: string }[] { - const entries: { entry: unknown; declaredIn: string }[] = []; + preserve: boolean, +): ProvidedCollectionEntry[] { + const entries: ProvidedCollectionEntry[] = []; + const seen = new Set(); const input = stackLabel(stack, index); - const collect = (list: unknown, declaredIn: string): void => { + const collect = (list: unknown, declaredIn: string, slot: string): void => { if (!Array.isArray(list)) return; - for (const entry of list) entries.push({ entry, declaredIn }); + for (const entry of list) { + if (seen.has(entry)) continue; + seen.add(entry); + entries.push({ entry, declaredIn, slot }); + } }; const source = stack as Record; - collect(source[key], input); if (Array.isArray(source.packages)) { for (const pkg of source.packages) { if (!isRecord(pkg) || !isRecord(pkg.manifest)) continue; const id = pkg.manifest.id ?? pkg.manifest.name; - collect(pkg.manifest[key], typeof id === 'string' ? `package '${id}' of ${input}` : input); + if (typeof id === 'string') collect(pkg.manifest[key], `package '${id}' of ${input}`, `package:${id}`); + else collect(pkg.manifest[key], input, `input:${index}`); } } + const ownId = stack.manifest?.id ?? stack.manifest?.name; + collect(source[key], input, preserve && typeof ownId === 'string' ? `package:${ownId}` : 'artifact'); return entries; } @@ -4551,42 +4585,86 @@ function providedCollectionEntries( * with nothing to lend, carried by an input built with `strict: false`. A * cross-input pair resolves (a datasource in one input borrowing an instance * another declares, or one of a nested multi-package input's package bodies - * declares), because the composed artifact registers both. A connector name - * declared more than once is judged against the last declaration (the - * collector keeps the last one it meets), which is the entry the runtime - * keeps ({@link SAME_STACK_ENTRY_IDENTITY}'s `connectors` row), so a strict - * input whose own instance a later input replaces with a descriptor is - * refused too: the instance it borrowed from is not the one that boots. That - * finding names both declarations and prescribes the duplicate's fix, since - * the artifact already carries an instance that could lend. - */ -function collectComposedDatasourceConnectorErrors(stacks: readonly ObjectStackDefinition[]): string[] { + * declares), because the composed artifact registers both. + * + * A connector name declared more than once is judged by how the artifact + * boots it. Within one registration slot the later declaration is the one the + * registry keeps ({@link SAME_STACK_ENTRY_IDENTITY}'s `connectors` row), so + * the kept entry is judged, and when it cannot lend but an earlier one in the + * slot could, the finding names both and prescribes the duplicate's fix. In + * DIFFERENT slots (two packages of the artifact) the registry keeps one entry + * per package, and the lender answers whichever registers first, so when the + * kept entries disagree on whether they can lend the borrow is refused as + * ambiguous in either declaration order: the build cannot know which one the + * datasource gets, and one name meaning one grant is what D7 asks for. When + * they agree, the verdict is the same whichever one lends. + */ +function collectComposedDatasourceConnectorErrors( + stacks: readonly ObjectStackDefinition[], + preserve: boolean, +): string[] { const datasources: unknown[] = []; - const connectors: unknown[] = []; - const declarations = new Map; declaredIn: string }[]>(); + const declarations = new Map>(); for (const [i, stack] of stacks.entries()) { - for (const { entry } of providedCollectionEntries(stack, i, 'datasources')) datasources.push(entry); - for (const { entry, declaredIn } of providedCollectionEntries(stack, i, 'connectors')) { - connectors.push(entry); + for (const { entry } of providedCollectionEntries(stack, i, 'datasources', preserve)) datasources.push(entry); + for (const provided of providedCollectionEntries(stack, i, 'connectors', preserve)) { + const { entry, slot } = provided; if (!isRecord(entry) || typeof entry.name !== 'string') continue; - const named = declarations.get(entry.name) ?? []; - named.push({ entry, declaredIn }); - declarations.set(entry.name, named); + const bySlot = declarations.get(entry.name) ?? new Map(); + bySlot.set(slot, [...(bySlot.get(slot) ?? []), provided]); + declarations.set(entry.name, bySlot); } } - const shadowedLender = (name: string): string | undefined => { - const named = declarations.get(name) ?? []; - const kept = named[named.length - 1]; - const lender = named.slice(0, -1).reverse().find(({ entry }) => connectorCanLend(entry)); - if (kept === undefined || lender === undefined) return undefined; - return ( + + const lends = (provided: ProvidedCollectionEntry): boolean => connectorCanLend(provided.entry as Record); + const connectors: unknown[] = []; + const shadows = new Map(); + const ambiguities = new Map(); + for (const [name, bySlot] of declarations) { + const kept = [...bySlot.values()].map((inSlot) => inSlot[inSlot.length - 1]); + const lending = kept.filter(lends); + const notLending = kept.filter((provided) => !lends(provided)); + if (lending.length > 0 && notLending.length > 0) { + const sites = (list: ProvidedCollectionEntry[]): string => list.map((p) => p.declaredIn).join(', '); + ambiguities.set( + name, + `which more than one package of the composed artifact declares, and they disagree: ` + + `${sites(lending)} declares an instance that can lend, and ${sites(notLending)} declares an ` + + `entry that cannot. The runtime keeps one entry per package and lends from whichever ` + + `registers first, so the datasource could boot with no credential. Rename or remove the ` + + `entry that cannot lend, so that one connector name means one instance.`, + ); + continue; + } + const representative = kept[kept.length - 1]; + connectors.push(representative.entry); + if (lends(representative)) continue; + const inSlot = bySlot.get(representative.slot) ?? []; + const lender = inSlot.slice(0, -1).reverse().find(lends); + if (lender === undefined) continue; + shadows.set( + name, `Connector '${name}' is declared more than once: ${lender.declaredIn} declares an instance that ` + - `can lend, and ${kept.declaredIn} declares the later entry, which the runtime keeps. Rename or ` + - `remove the later entry, or give one of the two its own name and point external.connector at ` + - `the instance.` + `can lend, and ${representative.declaredIn} declares the later entry, which the runtime keeps. ` + + `Rename or remove the later entry, or give one of the two its own name and point ` + + `external.connector at the instance.`, ); - }; - return collectDatasourceConnectorErrors({ datasources, connectors } as ObjectStackDefinition, shadowedLender); + } + + const errors: string[] = []; + const judged: unknown[] = []; + for (const ds of datasources) { + const connector = isRecord(ds) && isRecord(ds.external) ? ds.external.connector : undefined; + const ambiguity = typeof connector === 'string' ? ambiguities.get(connector) : undefined; + if (ambiguity === undefined) judged.push(ds); + else errors.push(`${connectorBorrowSubject((ds as Record).name, connector as string)}, ${ambiguity}`); + } + errors.push( + ...collectDatasourceConnectorErrors({ datasources: judged, connectors } as ObjectStackDefinition, (name) => + shadows.get(name), + ), + ); + return errors; } /** @@ -7623,7 +7701,7 @@ function composeBuiltStacks( ...collectComposedDefaultRecordPageErrors(stacks, opts.manifest === 'preserve'), ...collectComposedMembershipKeyErrors(stacks, objects as readonly unknown[] | undefined), ...collectComposedExternalBindingDatasourceErrors(stacks, objects), - ...collectComposedDatasourceConnectorErrors(stacks), + ...collectComposedDatasourceConnectorErrors(stacks, opts.manifest === 'preserve'), ...collectArtifactFlowClaimErrors(stacks), ]; if (artifactCrossRefErrors.length > 0) {