fix(lint)!: os validate and os build resolve a dashboard's dataset through the reader's declared closure, as the runtime does

.changeset/32967-dataset-reader-closure-authoring.md

@@ -0,0 +1,36 @@
+---
+'@objectstack/lint': minor
+'@objectstack/core': patch
+'@objectstack/cli': patch
+'@objectstack/metadata-protocol': patch
+'@objectstack/spec': patch
+---
+
+Clause-②: no (narrowing) — `os validate`, `os build` and `os lint` refuse (`widget-dataset-unknown`, error) a dashboard widget in a package of a multi-package artifact whose `dataset` only a package outside that package's declared dependency closure holds, or two packages inside it hold, where they accepted it. The runtime already refused both reads (`409 RESOURCE_CONFLICT`) since 18.4.0; no runtime answer changes.
+Metadata-protocol: narrowed
+Skills-docs: none — `skills/**` teaches neither cross-package dataset resolution nor `widget-dataset-unknown`.
+
+<!-- adr-0087: registered dashboard-dataset-outside-reader-closure-refused -->
+
+fix(lint)!: `os validate` and `os build` resolve a dashboard's dataset through the reading package's declared dependency closure, as the runtime does
+
+**BREAKING** accept-set narrowing in the authoring doors (`os validate`, `os build`, `os lint`; `@objectstack/lint`'s `widget-dataset-unknown`), for a package of a multi-package artifact (`packages[]`, the `composeStacks(stacks, { manifest: 'preserve' })` shape).
+
+- **Before.** The per-package pass resolved a dashboard widget's `dataset` against every package of the artifact, so a widget bound to a dataset another package holds was accepted whether or not the reading package declares it.
+- **Now.** The name resolves in the reading package, then to the one holder in its declared dependency closure (`manifest.dependencies`, transitively, plus `sys`). A name held only outside that closure, or by two packages inside it, is refused with `widget-dataset-unknown` (error), naming the dataset, the holders and the open fix. When the reading package declares a dependency the artifact does not carry, an outside-closure name is reported (`widget-dataset-unknown`, warning, suppressible with `suppressWarnings`) instead, since that package may hold it once installed; two holders inside the closure are refused even then.
+- **Unchanged.** A stack without `packages[]`, the union run over the whole artifact, a package's own dataset, a dataset of a declared dependency, and a name no package holds (refused as before). Every runtime answer.
+- **Fix.** Declare the holding package in the reading package's `dependencies`; where the holder already depends on the reader (a module of the app), move the dashboard into the holder, or move the dataset into the reader. Upgrade-guide entry `dashboard-dataset-outside-reader-closure-refused`.
+
+Since 18.2.0 (#25514) the per-package authoring pass resolved a widget's `dataset` against every package of the artifact. The runtime does not: since 18.4.0 a package's dashboard resolves a dataset name in its own package, then in the packages its manifest declares in `dependencies` (transitively) plus `sys`, and refuses the rest (ADR-0150 D3, the maintainer's ruling on #32929). So `os validate` passed a dashboard that answered `409 RESOURCE_CONFLICT` once installed. HotCRM is the first-party app built this way: its app package's CRM Overview read `product_metrics`, which its revenue module holds. Repository examples affected: 0. Other first-party apps outside this repository: not measured.
+
+The authoring pass now asks the same question. Who moves: an app whose package dashboard binds a dataset that another package of the same artifact holds, without declaring that package. The finding names the dataset, the holders and the open fix, read off the artifact:
+
+- the holder does not depend on the reading package: declare it in the reader's `dependencies`, or move the dataset into the reader;
+- the holder already depends on the reading package (a module of the app): declaring it back is a dependency cycle the kernel refuses at boot, so move the dashboard into the holder, or move the dataset into the reader;
+- two holders inside the closure: nothing picks one; keep one, or depend on one.
+
+A report's (or a joined block's) `runtimeFilter` reads the same index: on the per-package pass its keys are no longer judged against a dataset the report cannot resolve. No verdict is lost: `chart-dataset-unknown` already refuses that report there, and the union run still judges the filter.
+
+The refusal sentences that name a fix for `outside-closure` now name the open one too: the core sentence (`409` at every package-scoped by-name door) says "move the TYPE into P" alone when only the environment holds the item, and asks for a dependency on ONE package when two or more hold it; `os migrate stored-references` (the report and `bind`), the CLI's picklist-reference hint and the publish probe's `widget_not_probed` fix name the move beside the dependency. Which item a read answers, its status, code and `details.candidates` do not change.
+
+The remedy (correcting the 18.4.0 note of #32310, which named only the dependency and the `?package=` qualifier): declare the holder in the reading package's `dependencies`; where the holder already depends on the reader, move the reading item into the holder, or move the read item into the reader.

packages/cli/src/commands/migrate/stored-references/index.ts

@@ -27,7 +27,7 @@ export function describeCandidate(c: ReferenceCandidate): string {
 const REASONS = {
   ambiguous: 'more than one holder, none picked — bind one after review',
   'environment-only': 'only an environment-authored item holds it, which a package never reaches (ADR-0150 D3 (a)) — not bindable',
-  'outside-closure': 'only packages outside the writer\'s declared dependencies hold it (ADR-0150 D3 (c)) — declare the dependency; not bindable',
+  'outside-closure': 'only packages outside the writer\'s declared dependencies hold it (ADR-0150 D3 (c)) — declare the dependency, or move the item where that dependency would be a cycle; not bindable',
 } as const;
 
 /**

packages/cli/src/utils/picklist-references.test.ts

@@ -226,6 +226,16 @@ describe('judgePicklistReferences — a field naming a sibling package\'s list (
     expect(refusals[0].hint).toContain('`dependencies` of package \'com.example.orders\'');
   });
 
+  it('[#32967] a holder that already depends on the declaring package: the hint names the move, never the cycle', () => {
+    const { refusals } = judgePicklistReferences({
+      packages: [core({ dependencies: { 'com.example.orders': '^1.0.0' } }), orders()],
+    });
+    expect(refusals).toHaveLength(1);
+    expect(refusals[0].hint).toContain('dependency cycle');
+    expect(refusals[0].hint).toContain('declare the list in \'com.example.orders\' itself');
+    expect(refusals[0].hint).not.toContain('Declare the dependency');
+  });
+
   it('names every holder when two siblings hold the list and the declaring package depends on neither', () => {
     const { refusals } = judgePicklistReferences({
       packages: [core(), { manifest: { ...manifest('com.example.crm'), picklists: [industry] } }, orders()],

packages/cli/src/utils/picklist-references.ts

@@ -390,9 +390,20 @@ function refusedReferenceHint(refusal: PackageScopedRefusal, bodies: readonly Ju
   const holders = refusal.candidates.flatMap((c) => (c.kind === 'package' ? [c.packageId] : []));
   const caller = refusal.callerPackageId;
   if (refusal.reason === 'outside-closure' && caller !== null) {
-    const declare = holders.length === 1
-      ? `add \`'${holders[0]}': '${rangeOf(holders[0], bodies)}'\` to the \`dependencies\` of package '${caller}'`
-      : `add the one package whose list the field means (${holders.map((id) => `'${id}'`).join(' or ')}) ` +
+    // [#32967] A holder that already depends on the caller (a module of the
+    // app whose field names it) cannot be declared back: the kernel refuses
+    // the cycle at boot. Read off the artifact, so the hint names the open fix.
+    const registry = artifactRegistry(bodies);
+    const declarable = holders.filter((id) => !packageResolutionClosure(registry, id).has(caller));
+    if (holders.length > 0 && declarable.length === 0) {
+      return `${holders.length === 1 ? `'${holders[0]}' already depends` : `${holders.map((id) => `'${id}'`).join(' and ')} already depend`} ` +
+        `on '${caller}', so declaring a holder back would be a dependency cycle: declare the list in '${caller}' itself, ` +
+        `or move the field into the package that holds the list. ` +
+        'Until then the installed field serves no options and its values are refused.';
+    }
+    const declare = declarable.length === 1
+      ? `add \`'${declarable[0]}': '${rangeOf(declarable[0], bodies)}'\` to the \`dependencies\` of package '${caller}'`
+      : `add the one package whose list the field means (${declarable.map((id) => `'${id}'`).join(' or ')}) ` +
         `to the \`dependencies\` of package '${caller}'`;
     return `Declare the dependency: ${declare}. Or declare the list in '${caller}' itself. ` +
       'Until then the installed field serves no options and its values are refused.';

packages/cli/src/utils/stored-package-references.ts

@@ -666,7 +666,8 @@ export async function bindStoredPackageReference(
   }
   if (reason === 'outside-closure') {
     return refuse('outside-closure', `${answer.message} Binding it would resolve past the package's declared dependencies, `
-      + 'which the rule never does (ADR-0150 D3 (c)): declare the dependency instead. Nothing written.', candidates);
+      + 'which the rule never does (ADR-0150 D3 (c)): declare the dependency instead, or, where the holder already depends '
+      + 'on the package (a cycle), make the move the sentence names. Nothing written.', candidates);
   }
   const chosen = candidates.find((c) => c.holder === 'package' && c.packageId === request.packageId);
   if (!chosen) {

packages/core/src/package-scoped-resolution.test.ts

@@ -88,6 +88,23 @@ describe('[#32929] the outside-closure sentence names both fixes', () => {
     );
   });
 
+  it('[#32967] an environment-only holder: no package to depend on, so the sentence names only the move into the caller', () => {
+    const answer = resolvePackageScopedHolders({ type: 'dataset', name: 'n', callerPackageId: P }, packageScopedHolderSource([[null, 'env']]), closure([]));
+    expect(answer).toMatchObject({ status: 'refused', reason: 'outside-closure', candidates: [{ kind: 'environment' }] });
+    const message = (answer as { message: string }).message;
+    expect(message).toContain(`move the dataset into "${P}"`);
+    expect(message).not.toContain('Declare a dependency');
+    expect(message).not.toContain('that package');
+  });
+
+  it('[#32967] two package holders: the dependency asked for is ONE of them, never every one', () => {
+    const answer = resolvePackageScopedHolders({ type: 'dataset', name: 'n', callerPackageId: P }, packageScopedHolderSource([[A, 'a'], [B, 'b']]), closure([]));
+    expect(answer).toMatchObject({ status: 'refused', reason: 'outside-closure' });
+    const message = (answer as { message: string }).message;
+    expect(message).toContain('the one package whose dataset is meant');
+    expect(message).not.toContain('the package that holds it');
+  });
+
   it('names the type in its own words', () => {
     const answer = resolvePackageScopedHolders({ type: 'email_template', name: 'n', callerPackageId: P }, packageScopedHolderSource([[A, 'a']]), closure([]));
     expect((answer as { message: string }).message).toContain(`or move the email template into "${P}"`);

packages/core/src/package-scoped-resolution.ts

@@ -565,15 +565,29 @@ export function packageScopedRefusalMessage(refusal: Omit<PackageScopedRefusal,
     case 'ambiguous-in-environment':
       return `${subject} is held by ${refusal.candidates.length} ${counted} in this environment and the caller names ` +
         `no package: ${named}. Nothing picks one of them: address it by its package.`;
-    case 'outside-closure':
+    case 'outside-closure': {
       // [#32929] Two fixes, because the first is not always open: when the
       // holder already depends on the caller (a module of the app that reads
       // it), declaring the holder back is a dependency cycle the kernel
       // refuses at boot. The sentence is static; the answer does not move.
-      return `${subject} is held outside the declared dependency closure of package "${refusal.callerPackageId}": ` +
-        `${named}. Declare a dependency on the package that holds it (ADR-0141). If that package already depends on ` +
-        `"${refusal.callerPackageId}", the dependency would be a cycle: move what reads it into that package, or move the ` +
-        `${typeLabel(refusal.type)} into "${refusal.callerPackageId}" (ADR-0150 D3).`;
+      const caller = refusal.callerPackageId;
+      const moveIn = `move the ${typeLabel(refusal.type)} into "${caller}"`;
+      const packages = refusal.candidates.filter((c) => c.kind === 'package').length;
+      const head = `${subject} is held outside the declared dependency closure of package "${caller}": ${named}.`;
+      // [#32967] Only the environment holds it (step 3's environment
+      // candidate): no package holds it, so there is nothing to depend on.
+      if (packages === 0) {
+        return `${head} No package holds it, so no dependency reaches it: ${moveIn} (ADR-0150 D3).`;
+      }
+      // [#32967] Two or more package holders: depending on every one would
+      // only turn this refusal into `ambiguous-in-closure`.
+      const declare = packages === 1
+        ? 'Declare a dependency on the package that holds it (ADR-0141).'
+        : `Declare a dependency on the one package whose ${typeLabel(refusal.type)} is meant; depending on more than one ` +
+          'leaves the name ambiguous (ADR-0141).';
+      return `${head} ${declare} If that package already depends on "${caller}", the dependency would be a cycle: ` +
+        `move what reads it into that package, or ${moveIn} (ADR-0150 D3).`;
+    }
     case 'not-held-by-qualifier':
       return `${subject} is not held by ${refusal.callerPackageId === null ? 'the environment' : `package "${refusal.callerPackageId}"`}, ` +
         `which the address names exactly (ADR-0150 D5): it is held by ${named}. Address it by its holder.`;