fix(spec): composeStacks says why a top-level connector beside a package body's same-named entry is ambiguous

.changeset/28006-composed-connector-ambiguity-reason.md

@@ -0,0 +1,16 @@
+---
+'@objectstack/spec': patch
+---
+
+fix(spec): `composeStacks` says why a top-level connector beside a package body's same-named entry is ambiguous (#28006)
+
+Clause-②: no (message text only: every composition `composeStacks` refused it still refuses, under the same `STACK_CROSS_REFERENCE_INVALID` envelope, and every one it accepted it still accepts)
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, no export or type changes, and no verdict moves: only the wording of one refusal changes. There is nothing for `objectstack migrate meta` to rewrite. -->
+
+A datasource whose `external.connector` names a connector that a package body and the composed artifact's top level both declare, one able to lend and the other not, was already refused as ambiguous. The refusal gave the reason that holds between two package bodies ("the runtime keeps one entry per package and lends from whichever registers first"), which is not what happens here. The boot does not register a top-level entry whose name a package body declares, so the body's entry lends, and a dev reload replaces it with the artifact's top-level entries of the name, the first of which lends. The finding now says that, and opens with "which the composed artifact declares both in a package body and at its top level".
+
+- **Where it applies.** A `last` composition with a nested multi-package input, where another input's top level declares the same name as one of its bodies. A `preserve` composition with an input that carries `packages` beside its own top-level collections, where another package declares the same name.
+- **Top-level entries only.** Two inputs' top-level entries that disagree, beside package bodies that do not declare the name, get a sentence of their own: no door registers either as a package body's entry, and a dev reload lends from the first top-level entry of the name.
+- **Unchanged.** Two package bodies that disagree keep the first-registered wording. The remedy is the same: rename or remove the entry that cannot lend.

packages/spec/src/stack-datasource-connector.test.ts

@@ -31,6 +31,14 @@
  *   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 top-level entry beside a package body's same-named entry (a nested
+ *   input's body under `last`, or another package's body under `preserve`
+ *   beside an input that carries `packages` and its own top level) is refused
+ *   as ambiguous when the two disagree, for the reason the runtime gives: the
+ *   boot keeps the body's entry, a dev reload replaces it with the
+ *   artifact's top-level entries of the name, the first of which lends; and
+ *   two such inputs' top-level entries, beside bodies silent on the name,
+ *   are refused with a sentence of their own;
  * - a nested multi-package input (a `manifest: 'preserve'` artifact) is read
  *   through its package bodies, where its connectors and datasources live,
  *   and an entry it carries in both halves is read once;
@@ -162,6 +170,10 @@ describe('composeStacks judges external.connector against the connectors of ever
       return err as Envelope;
     }
   };
+  const BESIDE_BODY = 'which the composed artifact declares both in a package body and at its top level, and they disagree';
+  const BOOT_OR_RELOAD =
+    "The boot does not register a top-level entry whose name a package body declares, so the body's entry " +
+    "lends, and a dev reload replaces it with the artifact's top-level entries of the name, the first of which lends";
   const expectComposedFinding = (refused: Envelope | undefined, fragment: string): void => {
     expectOneFinding(refused, fragment);
     expect(refused?.issues).toHaveLength(1);
@@ -275,17 +287,21 @@ describe('composeStacks judges external.connector against the connectors of ever
       ]);
     });
 
-    it("treats a package body and another input's top level as two slots, and names the body", () => {
+    it("refuses as ambiguous a top-level entry beside a package body's same-named entry, 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' })]);
-      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",
+      expectComposedFinding(refused, BESIDE_BODY);
+      const finding = String(refused?.issues?.[0]);
+      expect(finding).toContain(
+        "package 'com.acme.orders' of 'com.acme.other' (stack #0) declares an instance that can lend, " +
+          "and 'com.acme.sap' (stack #1) declares an entry that cannot.",
       );
+      expect(finding).toContain(BOOT_OR_RELOAD);
+      expect(finding).not.toContain('one entry per package');
     });
 
     it('reads an entry the nested input carries in both halves once', () => {
@@ -306,6 +322,81 @@ describe('composeStacks judges external.connector against the connectors of ever
     });
   });
 
+  describe('a `preserve` input that carries `packages` beside its own top-level collections', () => {
+    // Hand-assembled: `preservePackageEntries` passes its bodies through and
+    // assembles its own top level into none, so that top level boots through
+    // the residual rule (under the composed artifact's `manifest.id`, the last
+    // input's), and the collector judges it in its own `package:${id}` slot.
+    const mixed = (connector: Record<string, unknown>, id = 'com.acme.bundle', inner = 'com.acme.inner') =>
+      defineStack({
+        manifest: pluginManifest(id),
+        connectors: [connector],
+        packages: [{ manifest: pluginManifest(inner) }],
+      } as never);
+    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([
+      ['mixed input first', false],
+      ['mixed input last', true],
+    ])("refuses as ambiguous its top-level entry beside another package's body — %s", (_order, reversed) => {
+      const stacks = reversed ? [strictPair(), mixed(descriptor)] : [mixed(descriptor), strictPair()];
+      const refused = composeRefusal(stacks, { manifest: 'preserve' });
+      expectComposedFinding(refused, BESIDE_BODY);
+      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.bundle' (stack #");
+      expect(finding).toContain(BOOT_OR_RELOAD);
+    });
+
+    it('CONTROL: its top-level instance composes beside a body whose same-named instance can lend too, in either order', () => {
+      const other = instance({ auth: { type: 'bearer', credentialRef: 'sap_token_2' } });
+      const credentialRefs = (artifact: Record<string, unknown>): unknown[] =>
+        (artifact.connectors as { auth: { credentialRef: unknown } }[]).map((c) => c.auth.credentialRef);
+      const artifact = composeStacks([mixed(other), strictPair()] as never, { manifest: 'preserve' }) as Record<string, unknown>;
+      // No body owns the mixed input's top level, so the artifact keeps the
+      // whole flattened list, in input order, beside bodies of the inner
+      // package and the plain input only, and the residual rule reads it
+      // under the last input's manifest id. A dev reload lends from the first
+      // entry of that list, which is why the order is pinned.
+      expect((artifact.manifest as Record<string, unknown>).id).toBe('com.acme.orders');
+      expect(
+        (artifact.packages as { manifest: Record<string, unknown> }[]).map((pkg) => pkg.manifest.id),
+      ).toEqual(['com.acme.inner', 'com.acme.orders']);
+      expect(credentialRefs(artifact)).toEqual(['sap_token_2', 'sap_token']);
+      const mixedLast = composeStacks([strictPair(), mixed(other)] as never, { manifest: 'preserve' }) as Record<
+        string,
+        unknown
+      >;
+      expect(credentialRefs(mixedLast)).toEqual(['sap_token', 'sap_token_2']);
+    });
+
+    it.each([
+      ['descriptor first', false],
+      ['instance first', true],
+    ])('refuses two such inputs whose top-level entries disagree, beside bodies silent on the name — %s', (_order, reversed) => {
+      const withDescriptor = mixed(descriptor, 'com.acme.bundle', 'com.acme.inner');
+      const withInstance = mixed(instance(), 'com.acme.kit', 'com.acme.toolkit');
+      const pair = reversed ? [withInstance, withDescriptor] : [withDescriptor, withInstance];
+      const refused = composeRefusal([...pair, looseBorrower()], { manifest: 'preserve' });
+      expectComposedFinding(
+        refused,
+        'which more than one input declares at the top level of a composed artifact that carries package bodies, and they disagree',
+      );
+      const finding = String(refused?.issues?.[0]);
+      expect(finding).toContain("'com.acme.kit' (stack #");
+      expect(finding).toContain("declares an instance that can lend, and 'com.acme.bundle' (stack #");
+      expect(finding).toContain(
+        "No door registers either as a package body's entry, and a dev reload lends from the first top-level entry of the name",
+      );
+    });
+  });
+
   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');

packages/spec/src/stack.zod.ts

@@ -4514,11 +4514,35 @@ interface ProvidedCollectionEntry {
   /** 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
+   * The registry slot the entry is judged 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.
+   * For a package body's own entry ({@link ProvidedCollectionEntry.inBody})
+   * that is the slot it boots into. For a top-level entry that boots beside
+   * package bodies it is a stand-in, one per input: see `inBody`.
    */
   slot: string;
+  /**
+   * Whether the entry boots as a package body's own item: an entry of a
+   * `packages[]` body, or, under `preserve`, the top level of an input that
+   * `preservePackageEntries` folds into a body of its own manifest id.
+   *
+   * `false` for every other top-level entry. When the artifact carries
+   * package bodies, such an entry boots through the residual rule
+   * (`unclaimedTopLevel`, `@objectstack/metadata`): it is not registered at
+   * all when a body declares its name, and is registered under the composed
+   * artifact's `manifest.id` (the last input's) when none does; a dev reload
+   * then replaces the booted entries of the name with the artifact's
+   * top-level list (`mergeDeclaredConnectorSources`,
+   * `@objectstack/service-automation`). Its `slot` is therefore not the slot
+   * it boots into: `'artifact'` under `last` and for an input with no
+   * manifest id, and under `preserve` the input's own `package:${id}` for an
+   * input that carries `packages` beside its own collections. Keeping such a
+   * top level in a slot apart from the bodies is what refuses a disagreement
+   * with a body's entry as ambiguous, which is the verdict the boot-or-reload
+   * split asks for, so the stand-in stays.
+   */
+  inBody: boolean;
 }
 
 /**
@@ -4532,9 +4556,13 @@ interface ProvidedCollectionEntry {
  * 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.
+ * body registers under its own id, and so does the top level of an input that
+ * `preserve` folds into a package body. Any other top-level entry takes the
+ * input's own `package:${id}` under `preserve` when the input has a manifest
+ * id, and the `'artifact'` slot otherwise. When no input carries package
+ * bodies, `'artifact'` is the composed artifact's one package. Beside package
+ * bodies both are stand-ins, because such an entry boots through the residual
+ * rule and a dev reload replaces it (see {@link ProvidedCollectionEntry.inBody}).
  */
 function providedCollectionEntries(
   stack: ObjectStackDefinition,
@@ -4545,25 +4573,26 @@ function providedCollectionEntries(
   const entries: ProvidedCollectionEntry[] = [];
   const seen = new Set<unknown>();
   const input = stackLabel(stack, index);
-  const collect = (list: unknown, declaredIn: string, slot: string): void => {
+  const collect = (list: unknown, declaredIn: string, slot: string, inBody: boolean): void => {
     if (!Array.isArray(list)) return;
     for (const entry of list) {
       if (seen.has(entry)) continue;
       seen.add(entry);
-      entries.push({ entry, declaredIn, slot });
+      entries.push({ entry, declaredIn, slot, inBody });
     }
   };
   const source = stack as Record<string, unknown>;
   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;
-      if (typeof id === 'string') collect(pkg.manifest[key], `package '${id}' of ${input}`, `package:${id}`);
-      else collect(pkg.manifest[key], input, `input:${index}`);
+      if (typeof id === 'string') collect(pkg.manifest[key], `package '${id}' of ${input}`, `package:${id}`, true);
+      else collect(pkg.manifest[key], input, `input:${index}`, true);
     }
   }
   const ownId = stack.manifest?.id ?? stack.manifest?.name;
-  collect(source[key], input, preserve && typeof ownId === 'string' ? `package:${ownId}` : 'artifact');
+  const ownSlot = preserve && typeof ownId === 'string' ? `package:${ownId}` : 'artifact';
+  collect(source[key], input, ownSlot, ownSlot !== 'artifact' && !Array.isArray(source.packages));
   return entries;
 }
 
@@ -4592,12 +4621,23 @@ function providedCollectionEntries(
  * 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.
+ * DIFFERENT slots the kept entries are refused as ambiguous, in either
+ * declaration order, when they disagree on whether they can lend. The build
+ * cannot know which one the datasource gets, and one name meaning one grant
+ * is what D7 asks for. Which mechanism makes it unknowable depends on where
+ * the entries sit, and the finding names it:
+ *
+ * - two package bodies: the registry keeps one entry per package, and the
+ *   lender answers whichever registers first;
+ * - a package body and a top-level entry beside it: the boot does not
+ *   register the top-level entry, so the body's lends, and a dev reload
+ *   replaces the body's entry with the artifact's top-level entries of the
+ *   name, the first of which lends ({@link ProvidedCollectionEntry.inBody});
+ * - top-level entries of two inputs, beside bodies that do not declare the
+ *   name: no door registers either as a package body's entry, and a dev
+ *   reload lends from the first top-level entry of the name.
+ *
+ * When the kept entries agree, the verdict is the same whichever one lends.
  */
 function collectComposedDatasourceConnectorErrors(
   stacks: readonly ObjectStackDefinition[],
@@ -4626,13 +4666,28 @@ function collectComposedDatasourceConnectorErrors(
     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(', ');
+      const disagree =
+        `and they disagree: ${sites(lending)} declares an instance that can lend, and ` +
+        `${sites(notLending)} declares an entry that cannot.`;
+      const inBodies = kept.filter((provided) => provided.inBody).length;
+      const where =
+        inBodies === kept.length
+          ? `which more than one package of the composed artifact declares, ${disagree} The runtime ` +
+            `keeps one entry per package and lends from whichever registers first, so the datasource ` +
+            `could boot with no credential.`
+          : inBodies > 0
+            ? `which the composed artifact declares both in a package body and at its top level, ` +
+              `${disagree} The boot does not register a top-level entry whose name a package body ` +
+              `declares, so the body's entry lends, and a dev reload replaces it with the artifact's ` +
+              `top-level entries of the name, the first of which lends, so the entry the datasource ` +
+              `borrows depends on whether the artifact was booted or reloaded and on the order of its inputs.`
+            : `which more than one input declares at the top level of a composed artifact that ` +
+              `carries package bodies, ${disagree} No door registers either as a package body's ` +
+              `entry, and a dev reload lends from the first top-level entry of the name, so the build ` +
+              `cannot tell which one lends, and the datasource could boot with no credential.`;
       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.`,
+        `${where} Rename or remove the entry that cannot lend, so that one connector name means one instance.`,
       );
       continue;
     }