fix(cli)!: report the named export the config default export already declares, instead of dropping it silently (#18419)

.changeset/18419-config-shadowed-named-export-reported.md

@@ -0,0 +1,20 @@
+---
+"@objectstack/cli": minor
+---
+
+fix(cli)!: a named export the config's default export already declares is reported instead of silently dropped (#18419)
+
+<!-- adr-0087: not-required (no-migration-prescription) no metadata key is retired, renamed or given a new meaning here, no stored `sys_metadata` document changes, and the only authored construct whose treatment moves is a NAMED EXPORT of `objectstack.config.ts` that was already inert — it was skipped by the merge and reached no artifact. `os migrate meta` has nothing it could rewrite, so there is no ledger entry for this to be missing. -->
+
+`objectstack.config.ts` is loaded as a module: `loadConfig()` takes the default export as the base and merges every named export onto it as a top-level stack key. A named export whose name the default export **already carries** loses — the default's value wins — and until now it lost in complete silence. `os build` exited 0, the artifact carried the default's value, and nothing was written at any level:
+
+```ts
+export default defineStack({ manifest, objects: [Task] });
+export const objects = [Task, Invoice];   // Invoice never reached the artifact
+```
+
+The loader now says so on stderr, names every shadowed key, and states the rule and the remedy. It is an **advisory, not a refusal** — the stack that comes out is valid, it is merely missing what the shadowed export carried — which is the disposition this package already gives the same failure class (`#3786`'s undeclared authoring keys are "advisory, never fatal"; `#4095`'s orphaned runtime members are "reported rather than dropped"). It goes to stderr rather than stdout because `loadConfig()` is handed no `--json` flag and twelve commands call it, so a `--json` run's stdout stays a single parseable document. `LoadedConfig.shadowedNamedExports` carries the same names structurally.
+
+**BREAKING** in the accept-set sense, landing in the launch window as `minor` (the lockstep convention: `major` is refused by `check-changeset-no-major`, and breaking-ness is carried by this banner plus the ADR-0087 disposition above): the collision test now reads **own keys only**. `key in merged` walked the prototype chain, so every `Object.prototype` member — `toString`, `valueOf`, `constructor`, `hasOwnProperty`, `propertyIsEnumerable`, `toLocaleString`, `isPrototypeOf` — was treated as a key the default export "already carries" when the default carries no such key at all. Such an export was skipped by the merge and therefore never reached the strict parse that refuses an undeclared stack key by name, so `export const toString = …` beside a valid stack built green while `export const collectPackageDirs = …` was refused. That hole is closed: those names now merge like any other and are refused by name, the same sentence every other undeclared helper export has always got.
+
+Nobody's metadata or stored data changes. A config affected by the narrowing was already shipping that export's value nowhere; what changes is that the build now says so instead of exiting 0. Move the helper into a sibling module and import it, which is what the config-authoring docs have always prescribed for a helper exported beside the stack.

content/docs/deployment/cli.mdx

@@ -2063,10 +2063,21 @@ recognisable rather than as patterns to use:
 |:---|:---|
 | not a declared stack key | the build **fails**, naming the key (above) |
 | a declared stack key the default export does **not** carry | merged in and **accepted** — the `onEnable` / `functions` path |
-| a key the default export **already** carries | dropped **silently**; the build exits 0 and the exported value is never read |
+| a key the default export **already** carries | the default's value wins and the exported one is dropped — the build still exits 0, and the drop is **reported on stderr** |
 
 The last row is why every stack key belongs inside `defineStack()`: a second
-copy beside it is not a second declaration.
+copy beside it is not a second declaration, it is a value nothing reads.
+
+```console
+$ printf '\nexport const objects = [myExtraObject];\n' >> objectstack.config.ts
+$ os build
+  ⚠ `objects` is a named export that was DROPPED — the default-exported stack already declares that key
+```
+
+The advisory goes to **stderr**, so it reaches a `--json` run's operator without
+putting anything but the envelope on stdout. It does not fail the build: the
+stack it produces is valid, it is simply missing what the shadowed export
+carried.
 
 ### Config File Auto-Detection
 

content/docs/getting-started/your-first-project.mdx

@@ -154,10 +154,12 @@ them a pattern to rely on:
 - a named export whose name **is** a declared stack key that the default export
   does not carry is merged in and accepted — that is the `onEnable` /
   `functions` path;
-- a named export whose name the default export **already carries** is dropped
-  **silently**, and the build still succeeds. A second
-  `export const objects = [...]` beside `defineStack({ objects })` is therefore
-  a value nothing reads. Write every stack key inside `defineStack()`.
+- a named export whose name the default export **already carries** loses to the
+  default, and the build still succeeds — but the loader now **says so on
+  stderr**, naming the key. A second `export const objects = [...]` beside
+  `defineStack({ objects })` is still a value nothing reads; it is no longer a
+  value nothing reads *and nothing mentions*. Write every stack key inside
+  `defineStack()`.
 
 </Callout>
 

packages/cli/src/utils/config-shadowed-named-export.test.ts

@@ -0,0 +1,236 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * A named export the default-exported stack already declares is DROPPED — and
+ * the drop is now reported instead of silent (#18419).
+ *
+ * ── The finding ──────────────────────────────────────────────────────────
+ *
+ * `loadConfig()` merges every named export of `objectstack.config.ts` onto the
+ * default export as a top-level stack key. A name the default already carries
+ * was skipped by `if (key === 'default' || key in merged) continue`, so:
+ *
+ *     export default defineStack({ manifest, objects: [] });
+ *     export const objects = [oneRow];          // <- never reaches anything
+ *
+ * built to exit 0, wrote an artifact whose `objects` is the default's `[]`, and
+ * logged NOTHING at any level. Authored content vanished on the success path.
+ *
+ * ── The positive control this file carries ───────────────────────────────
+ *
+ * The same channel is LOUD for a named export the stack schema does not
+ * declare: `export const ProbeNamedExport = [1,2,3]` is merged in, refused by
+ * the strict parse and named in the message (#18171, pinned next door in
+ * `config-named-export-rule.test.ts`). So silence was specific to this one arm
+ * rather than a property of the loader — which is why the first two pins below
+ * assert the UNCHANGED rows. A pin that only proved "a warning appears" could
+ * not tell a repaired loader from one that warns about everything.
+ *
+ * ── The second arm, measured while sweeping for the first ────────────────
+ *
+ * `key in merged` walks the PROTOTYPE chain, so `Object.prototype`'s members
+ * answered true for a default export that carries no such key at all. An
+ * `export const toString = …` was therefore skipped by the collision arm and
+ * never reached the strict parse that would have refused it by name — the loud
+ * refusal above, silently turned off by the spelling of the key. `loadConfig`
+ * now tests own keys only, so that row rejoins the control.
+ *
+ * ── Disposition: advisory, not refusal ───────────────────────────────────
+ *
+ * Read off this package's own repairs of this class — #3786's undeclared
+ * authoring keys ("Advisory, never fatal") and #4095's orphaned runtime members
+ * ("reported rather than dropped") — and recorded in full on
+ * {@link shadowedNamedExportWarning}. The stack that comes out is valid; it is
+ * merely missing what the shadowed export carried, so the run continues and the
+ * author is told. The accept set therefore moves for the prototype-chain row
+ * only, and in the narrowing direction.
+ */
+
+import { describe, it, expect, afterAll, vi } from 'vitest';
+import fs from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { ObjectStackDefinitionSchema } from '@objectstack/spec';
+
+import { loadConfig, shadowedNamedExportWarning } from './config.js';
+
+const HERE = path.dirname(fileURLToPath(import.meta.url));
+/** `packages/cli/tmp` — throwaway projects, the placement sibling suites use. */
+const TMP_ROOT = path.resolve(HERE, '..', '..', 'tmp');
+
+/** The card's own probe export — a name the stack schema does not declare. */
+const PROBE = 'ProbeNamedExport';
+
+const MANIFEST = `{
+    id: 'com.example.probe',
+    namespace: 'probe',
+    version: '0.1.0',
+    type: 'app',
+    name: 'Probe',
+    engines: { protocol: '^17' },
+  }`;
+
+const roots: string[] = [];
+afterAll(() => {
+  for (const dir of roots) fs.rmSync(dir, { recursive: true, force: true });
+});
+
+function writeConfig(tag: string, body: string): string {
+  fs.mkdirSync(TMP_ROOT, { recursive: true });
+  const dir = fs.mkdtempSync(path.join(TMP_ROOT, `shadowed-${tag}-`));
+  roots.push(dir);
+  const file = path.join(dir, 'objectstack.config.ts');
+  fs.writeFileSync(file, body);
+  return file;
+}
+
+/** Load `body`, capturing whatever the loader wrote to each stream. */
+async function loadCapturing(tag: string, body: string) {
+  const err: string[] = [];
+  const out: string[] = [];
+  const errSpy = vi.spyOn(console, 'error').mockImplementation((...a) => { err.push(a.join(' ')); });
+  const outSpy = vi.spyOn(console, 'log').mockImplementation((...a) => { out.push(a.join(' ')); });
+  try {
+    const loaded = await loadConfig(writeConfig(tag, body));
+    return { ...loaded, stderr: err.join('\n'), stdout: out.join('\n') };
+  } finally {
+    errSpy.mockRestore();
+    outSpy.mockRestore();
+  }
+}
+
+describe('#18419 — a named export the default already declares is dropped, and said so', () => {
+  it('CONTROL: an undeclared named export is still refused by name, and is not a "drop"', async () => {
+    const loaded = await loadCapturing('control', `import { defineStack } from '@objectstack/spec';
+
+export default defineStack({
+  manifest: ${MANIFEST},
+});
+
+export const ${PROBE} = [1, 2, 3];
+`);
+
+    // Merged, therefore reaches the parse, therefore refused — the property
+    // #18171 pinned and this change must not spend.
+    expect(loaded.namedExports).toEqual([PROBE]);
+    expect(loaded.shadowedNamedExports).toEqual([]);
+    const result = ObjectStackDefinitionSchema.safeParse(loaded.config);
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    const unrecognized = result.error.issues.filter((i) => i.code === 'unrecognized_keys');
+    expect((unrecognized[0] as unknown as { keys: string[] }).keys).toEqual([PROBE]);
+
+    // Nothing was dropped, so nothing is announced. "Reported" has to be
+    // distinguishable from "always reported".
+    expect(loaded.stderr).not.toContain('DROPPED');
+  }, 60_000);
+
+  it('CONTROL: a declared key the default does NOT carry is still merged, silently and legally', async () => {
+    const loaded = await loadCapturing('accepted', `import { defineStack } from '@objectstack/spec';
+
+export default defineStack({
+  manifest: ${MANIFEST},
+});
+
+export const objects = [];
+`);
+
+    expect(loaded.namedExports).toEqual(['objects']);
+    expect(loaded.shadowedNamedExports).toEqual([]);
+    expect(ObjectStackDefinitionSchema.safeParse(loaded.config).success).toBe(true);
+    expect(loaded.stderr).toBe('');
+  }, 60_000);
+
+  it('the collision is RECORDED and ANNOUNCED — the silence this card is about', async () => {
+    const loaded = await loadCapturing('collision', `import { defineStack } from '@objectstack/spec';
+
+export default defineStack({
+  manifest: ${MANIFEST},
+  objects: [],
+});
+
+export const objects = [{ name: 'probe_row', label: 'Probe Row', fields: { name: { type: 'text', label: 'Name' } } }];
+`);
+
+    // The drop itself is unchanged — the default's value still wins…
+    expect(loaded.config.objects).toEqual([]);
+    expect(loaded.namedExports).toEqual([]);
+    // …and it is no longer invisible.
+    expect(loaded.shadowedNamedExports).toEqual(['objects']);
+    expect(loaded.stderr).toContain('objects');
+    expect(loaded.stderr).toContain('DROPPED');
+    // The rule and the remedy, not just the fact.
+    expect(loaded.stderr).toContain('loaded as a MODULE');
+    expect(loaded.stderr).toContain('defineStack');
+
+    // Advisory, never fatal: the stack that comes out is still valid.
+    expect(ObjectStackDefinitionSchema.safeParse(loaded.config).success).toBe(true);
+  }, 60_000);
+
+  it('…including on `functions` — the runtime member an app really does author', async () => {
+    const loaded = await loadCapturing('functions', `import { defineStack } from '@objectstack/spec';
+
+export default defineStack({
+  manifest: ${MANIFEST},
+  functions: { fromDefault: () => 'default' },
+});
+
+export const functions = { fromNamedExport: () => 'named' };
+`);
+
+    expect(loaded.shadowedNamedExports).toEqual(['functions']);
+    expect(Object.keys(loaded.config.functions)).toEqual(['fromDefault']);
+    // The handler that vanished is named, because that is the one the author
+    // has to go looking for.
+    expect(loaded.stderr).toContain('functions');
+  }, 60_000);
+
+  it('a name that is only on Object.prototype is NOT a collision — it rejoins the control', async () => {
+    const loaded = await loadCapturing('proto', `import { defineStack } from '@objectstack/spec';
+
+export default defineStack({
+  manifest: ${MANIFEST},
+});
+
+export const toString = [1, 2, 3];
+`);
+
+    // The default export carries no `toString` of its own, so nothing shadows
+    // this — it is an undeclared stack key like any other, and goes the loud way.
+    expect(loaded.shadowedNamedExports).toEqual([]);
+    expect(loaded.namedExports).toEqual(['toString']);
+    const result = ObjectStackDefinitionSchema.safeParse(loaded.config);
+    expect(result.success).toBe(false);
+    if (result.success) return;
+    const unrecognized = result.error.issues.filter((i) => i.code === 'unrecognized_keys');
+    expect((unrecognized[0] as unknown as { keys: string[] }).keys).toContain('toString');
+  }, 60_000);
+
+  it('the advisory goes to stderr only — a --json run keeps stdout parseable', async () => {
+    const loaded = await loadCapturing('streams', `import { defineStack } from '@objectstack/spec';
+
+export default defineStack({
+  manifest: ${MANIFEST},
+  objects: [],
+});
+
+export const objects = [{ name: 'probe_row', label: 'Probe Row', fields: { name: { type: 'text', label: 'Name' } } }];
+`);
+
+    expect(loaded.stderr).toContain('DROPPED');
+    // `loadConfig` is handed no `--json` flag, so the one channel it must never
+    // write to is the one the machine reads.
+    expect(loaded.stdout).toBe('');
+  }, 60_000);
+
+  it('the warning names every key, and says what to do instead', () => {
+    const one = shadowedNamedExportWarning(['objects']).join('\n');
+    expect(one).toContain('`objects`');
+    expect(one).toContain('is a named export that was DROPPED');
+    expect(one).toContain('move the value inside defineStack');
+
+    const many = shadowedNamedExportWarning(['objects', 'functions']).join('\n');
+    expect(many).toContain('`objects`, `functions`');
+    expect(many).toContain('are named exports that were DROPPED');
+  });
+});