fix(spec,cli): config.storage was never a stack key — stop reading it, and report undeclared top-level keys (#4167)

.changeset/stack-storage-not-an-authoring-key.md

@@ -0,0 +1,71 @@
+---
+'@objectstack/spec': minor
+'@objectstack/cli': patch
+---
+
+**`config.storage` is not a stack key, and an undeclared top-level key now says
+so instead of vanishing (#4167).**
+
+`os serve` read `config.storage` and forwarded it to `StorageServicePlugin`.
+It could almost never arrive: `ObjectStackDefinitionSchema` does not declare
+`storage`, and is not `.strict()`, so `defineStack` — which every documented
+authoring path and every compiled artifact goes through — strips the key before
+`serve` runs. The one combination that reached the branch (a bare-object config
+on the config-boot path) then carried the `driver`/`root` spelling the plugin
+does not read either, so it did nothing there too.
+
+The result was one authoring key that worked on a single unreachable-in-practice
+path and disappeared silently everywhere else. A host writing
+`storage: { driver: 's3', … }` believed it had configured S3 and got local disk.
+
+- **`serve` no longer reads it.** `resolveStorageCapabilityArg` takes only the
+  env root; the production warning stops naming `config.storage` and names the
+  two channels that work — `OS_STORAGE_*` and Setup → Settings, the latter being
+  the one with proper credential handling.
+- **The undeclared-key lint now covers the stack's own top-level keys.** New
+  `lintUnknownStackKeys(rawStack, stackSchema)`, wired into `defineStack`,
+  `os validate` and `os compile` beside the existing walker. `storage` gets a
+  prescriptive entry naming both channels and why a stack definition is the
+  wrong home for a credential — it would commit it to git and to any published
+  artifact. An ordinary misspelling still gets the edit-distance suggestion
+  (`datasource` → `datasources`).
+- **`os migrate files-to-references` shares the resolver.** It built
+  `{ driver: 'local', root }` — the same dead keys — so its adapter used
+  `./storage` while the server writes under `.objectstack/data/uploads` since
+  #4096. That command reconciles what records claim against what storage holds,
+  so a disagreeing root reconciled against the wrong tree.
+
+**`onEnable` is exempt, and the exemption has one owner.** `onEnable` is a
+function, so `ObjectStackDefinitionSchema` cannot declare it and
+`dist/objectstack.json` cannot carry it — but it is not lost: `AppPlugin` calls
+it off the authored bundle, and the artifact-boot path grafts it back (#4095).
+"Not declared" and "dropped at load" are different claims, and this is the
+surface where they come apart. New `STACK_RUNTIME_MEMBERS` in `@objectstack/spec`
+names the members the runtime honours off the bundle; the lint treats them as
+declared, and the CLI's `GRAFTABLE_RUNTIME_MEMBERS` is now **derived** from it
+rather than restating it, so the list that decides what gets grafted and the
+list that decides what the lint stays quiet about cannot drift. `onDisable` is
+deliberately not on it — nothing calls it, so a value written there really does
+go nowhere and the lint should say so.
+
+Additive: `lintUnknownAuthoringKeys` keeps its signature. The new pass is a
+separate export rather than a fold into that walker for two reasons. The walker
+iterates metadata COLLECTIONS, so a stack whose only mistake is at the envelope
+level — no objects, no pages, nothing to iterate — walks clean; and the stack
+schema has to be INJECTED, because `stack.zod.ts` imports the lint module and
+importing back would close a cycle. A separate export keeps that requirement
+visible: a call site either asks for the coverage or does not, and its absence
+shows up in a diff. An optional parameter would be the same silent-loss shape
+this rule family exists to report. It follows the walker's posture rule — only a
+schema that STRIPS unknown keys is linted, so if the stack schema ever graduates
+to `.strict()` the parse takes over and this goes quiet.
+
+Verified end to end: authoring `storage:` through `defineStack` warns at load,
+and `os compile` reports it for configs that skip `defineStack`.
+
+Nothing is being taken away that worked. `storage` was never in the schema, is
+not documented anywhere, and has no consumer in `objectstack-ai/cloud` (checked).
+Whether the platform should eventually grow a real in-stack storage declaration
+is a separate question — if so it should follow `datasources`, which solves
+credentials by referencing `sys_secret` rather than inlining them, and that
+deserves an ADR rather than a resurrected undeclared key.

content/docs/deployment/validating-metadata.mdx

@@ -248,6 +248,26 @@ the parse itself rejects loudly, with the schema's own guidance. Because the
 lint reads each schema's real unknown-key posture, it can never disagree with
 the parse.
 
+It also covers the stack's **own top-level keys** — the envelope those
+collections sit in, and the level where the silence is hardest to spot, because
+an undeclared key there reads as configuration that took effect rather than as
+a typo:
+
+```ts
+export default defineStack({
+  storage: { adapter: 's3', s3: { bucket: 'app-files' } },
+  // ↑ not a stack key → dropped at load; the app keeps writing to local disk
+  objects: [...],
+});
+```
+
+```
+stack.storage: 'storage' is not a declared stack key, so its value is dropped at
+  load — the file-storage backend is a deployment concern, not an application
+  declaration. Configure it with the OS_STORAGE_* environment variables, or
+  per-deployment in Setup → Settings → Storage.
+```
+
 This is **advisory** — the stack still loads. Strict rejection is where these
 schemas are headed (ADR-0049 enforce-or-remove), but these are the protocol's
 most-authored surfaces, so the tightening is scheduled on what this check finds
@@ -273,7 +293,7 @@ time, so an author sees them without running the CLI at all.
 | View references — form targets, view-key collisions (#2554) | ✓ | ✓ |
 | Flow authoring anti-patterns (#1874) | ✓ | ✓ |
 | Liveness author-warnings | ✓ | ✓ |
-| Undeclared authoring keys, every metadata collection (#3786) | ✓ | ✓ |
+| Undeclared authoring keys — every metadata collection (#3786) and the stack's own top-level keys (#4167) | ✓ | ✓ |
 | Emits `dist/objectstack.json` | — | ✓ |
 
 So `os validate` is the fast inner-loop check (no artifact); `os build` is what

packages/cli/src/commands/compile.ts

@@ -9,6 +9,7 @@ import {
   ObjectStackDefinitionSchema,
   normalizeStackInput,
   lintUnknownAuthoringKeys,
+  lintUnknownStackKeys,
   formatUnknownAuthoringKey,
   type ConversionNotice,
 } from '@objectstack/spec';
@@ -263,7 +264,10 @@ export default class Compile extends Command {
       //     authored through it; this covers the ones that skip it (a plain
       //     object default-export, `strict: false`) and would otherwise emit an
       //     artifact with the key quietly gone. Advisory, never fatal.
-      const unknownKeyFindings = lintUnknownAuthoringKeys(normalized as Record<string, unknown>);
+      const unknownKeyFindings = [
+        ...lintUnknownStackKeys(normalized as Record<string, unknown>, ObjectStackDefinitionSchema),
+        ...lintUnknownAuthoringKeys(normalized as Record<string, unknown>),
+      ];
       if (unknownKeyFindings.length > 0 && !flags.json) {
         printWarning(`Undeclared authoring keys (${unknownKeyFindings.length}) — dropped at load (#3786)`);
         for (const f of unknownKeyFindings.slice(0, 50)) {

packages/cli/src/commands/migrate/files-to-references.ts

@@ -17,7 +17,7 @@ import {
 import { bootSchemaStack } from '../../utils/schema-migrate.js';
 import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js';
 import { describeOccupancy } from '../../utils/sqlite-occupancy.js';
-import { loadConfig } from '../../utils/config.js';
+import { resolveStorageCapabilityArg } from '../serve.js';
 
 async function confirm(question: string): Promise<boolean> {
   if (!process.stdin.isTTY) return false; // non-interactive → require --yes
@@ -36,8 +36,15 @@ async function confirm(question: string): Promise<boolean> {
  * Settings first: the storage plugin re-resolves its adapter from persisted
  * settings when a settings service is present, which is how an S3-configured
  * deployment's backfill uploads land in S3 rather than on this machine.
- * Storage config mirrors `os serve`'s resolution (config.storage, else the
- * local default) so the CLI materialises bytes exactly where the server would.
+ * Storage config goes through the SAME resolver `os serve` uses
+ * (`resolveStorageCapabilityArg`), so the CLI materialises bytes exactly where
+ * the server would. It did not, and that mattered here more than anywhere: this
+ * command reconciles what records claim against what storage actually holds, so
+ * a root that disagrees with the server's reconciles against the wrong tree.
+ * It built `{ driver: 'local', root }` — keys `StorageServicePluginOptions` does
+ * not declare — so the adapter silently used the plugin's own `./storage`
+ * default while the server (since framework#4096) writes under
+ * `.objectstack/data/uploads`.
  */
 async function buildDataMigrationPlugins(): Promise<unknown[]> {
   const plugins: unknown[] = [];
@@ -48,19 +55,8 @@ async function buildDataMigrationPlugins(): Promise<unknown[]> {
     // optional — without it, constructor/env-driven storage config still applies
   }
   const { StorageServicePlugin } = await import('@objectstack/service-storage');
-  let arg: Record<string, unknown> | undefined;
-  try {
-    const { config } = await loadConfig();
-    const cfgStorage = (config as any)?.storage;
-    if (cfgStorage && (cfgStorage.driver || cfgStorage.adapter)) arg = cfgStorage;
-  } catch {
-    // artifact-only project (no objectstack.config.ts) — use the default below
-  }
-  if (arg === undefined) {
-    const root = process.env.OS_STORAGE_ROOT || '.objectstack/data/uploads';
-    arg = { driver: 'local', root };
-  }
-  plugins.push(new StorageServicePlugin({ ...(arg as any), registerRoutes: false }));
+  const { options } = resolveStorageCapabilityArg(process.env.OS_STORAGE_ROOT);
+  plugins.push(new StorageServicePlugin({ ...options, registerRoutes: false }));
   return plugins;
 }
 

packages/cli/src/commands/serve-storage-capability.test.ts

@@ -16,6 +16,13 @@
  * option SHAPE, because a shape mismatch is exactly the failure a passing type
  * check does not catch when the receiving interface has no index signature and
  * the value is built as a plain object literal.
+ *
+ * framework#4167 then removed the `config.storage` branch this helper also had.
+ * `storage` was never a declared stack key, so `defineStack` — which every
+ * documented authoring path and every compiled artifact goes through — stripped
+ * it long before `serve` ran. Reading it here made one authoring key work on the
+ * single path that skips `defineStack` and vanish on all the others. Authors who
+ * write it are now told, by `lintUnknownAuthoringKeys`.
  */
 
 import { describe, it, expect } from 'vitest';
@@ -24,7 +31,7 @@ import { resolveStorageCapabilityArg } from './serve.js';
 describe('resolveStorageCapabilityArg', () => {
   it('builds options StorageServicePlugin actually reads', () => {
     // `adapter` + `local.rootDir` — NOT `driver` + `root`.
-    expect(resolveStorageCapabilityArg(undefined).options).toEqual({
+    expect(resolveStorageCapabilityArg().options).toEqual({
       adapter: 'local',
       local: { rootDir: '.objectstack/data/uploads' },
     });
@@ -33,49 +40,40 @@ describe('resolveStorageCapabilityArg', () => {
   it('never emits the keys the plugin ignores', () => {
     // The regression guard proper: `{driver, root}` type-checks fine as an
     // argument and does nothing at runtime.
-    const { options } = resolveStorageCapabilityArg(undefined);
+    const { options } = resolveStorageCapabilityArg();
     expect(options).not.toHaveProperty('driver');
     expect(options).not.toHaveProperty('root');
   });
 
   it('honours OS_STORAGE_ROOT, which the old shape discarded', () => {
-    const { options, localRoot } = resolveStorageCapabilityArg(undefined, '/srv/uploads');
+    const { options, localRoot } = resolveStorageCapabilityArg('/srv/uploads');
     expect(options).toEqual({ adapter: 'local', local: { rootDir: '/srv/uploads' } });
     expect(localRoot).toBe('/srv/uploads');
   });
 
   it('ignores a blank or whitespace-only env root', () => {
     for (const blank of ['', '   ']) {
-      expect(resolveStorageCapabilityArg(undefined, blank).options).toEqual({
+      expect(resolveStorageCapabilityArg(blank).options).toEqual({
         adapter: 'local',
         local: { rootDir: '.objectstack/data/uploads' },
       });
     }
   });
 
-  it('reports the local root so only the fallback triggers the production warning', () => {
-    // A host that configured its own backend must not be told it is on local disk.
-    expect(resolveStorageCapabilityArg(undefined).localRoot).toBe('.objectstack/data/uploads');
-    expect(resolveStorageCapabilityArg({ adapter: 's3', s3: { bucket: 'b', region: 'r' } }).localRoot)
-      .toBeUndefined();
+  it('reports the local root the production warning names', () => {
+    // The warning quotes this, so it has to be the directory actually in use —
+    // it previously named a root the adapter was not using.
+    expect(resolveStorageCapabilityArg().localRoot).toBe('.objectstack/data/uploads');
+    expect(resolveStorageCapabilityArg('/srv/x').localRoot).toBe('/srv/x');
   });
 
-  it('forwards a host-configured storage block verbatim', () => {
-    const cfg = { adapter: 's3', s3: { bucket: 'b', region: 'r' } };
-    expect(resolveStorageCapabilityArg(cfg).options).toBe(cfg);
-    // The `driver` dialect is still forwarded untouched — the plugin does not
-    // read it either, but rewriting it here would fossilize the wrong contract
-    // rather than fix it. Tracked separately.
-    const legacy = { driver: 's3', bucket: 'b' };
-    expect(resolveStorageCapabilityArg(legacy).options).toBe(legacy);
+  it('does not read config.storage at all — it was never a stack key (#4167)', () => {
+    // `ObjectStackDefinitionSchema` does not declare `storage`, and is not
+    // `.strict()`, so `defineStack` strips it before serve could see it. Reading
+    // it here made the same authoring key work on the one path that skips
+    // `defineStack` and vanish on every documented one. The signature no longer
+    // accepts it; `lintUnknownAuthoringKeys` now tells the author instead.
+    expect(resolveStorageCapabilityArg.length).toBe(1);
   });
 
-  it('falls back when the block names no backend at all', () => {
-    // `config.storage = { presignedTtl: 60 }` configures no backend, so the
-    // local default still applies rather than being replaced by a partial block.
-    expect(resolveStorageCapabilityArg({ presignedTtl: 60 }).options).toEqual({
-      adapter: 'local',
-      local: { rootDir: '.objectstack/data/uploads' },
-    });
-  });
 });

packages/cli/src/commands/serve.ts

@@ -2184,14 +2184,13 @@ export default class Serve extends Command {
             // In production mode we emit a single loud warning so the
             // operator knows to point storage at S3 / GCS / Azure before
             // shipping (data on a single pod is volatile / non-replicated).
-            const storageArg = resolveStorageCapabilityArg(
-              (config as any).storage,
-              process.env.OS_STORAGE_ROOT,
-            );
+            const storageArg = resolveStorageCapabilityArg(process.env.OS_STORAGE_ROOT);
             arg = storageArg.options;
             if (storageArg.localRoot && !isDev) {
+              // Names only the channels that actually work — `config.storage`
+              // was in this sentence and was never read (framework#4167).
               console.warn(chalk.yellow(
-                `  ⚠ StorageServicePlugin using local driver (${storageArg.localRoot}) — switch to S3/GCS/Azure for production (set config.storage or OS_STORAGE_*).`,
+                `  ⚠ StorageServicePlugin using local driver (${storageArg.localRoot}) — switch to S3/GCS/Azure for production (set OS_STORAGE_* or configure storage in Setup → Settings).`,
               ));
             }
           }
@@ -2665,20 +2664,23 @@ export interface StorageCapabilityArg {
  * default IS `./.objectstack/data/uploads`), which swapped the adapter and
  * warned about stranded files — on every boot of a healthy server.
  *
- * A caller-supplied `config.storage` is still forwarded verbatim, including the
- * `driver`/`root` dialect, which the plugin does not read either. That is the
- * same mismatch one layer up and is tracked separately: correcting it means
- * deciding whether the plugin accepts that dialect or the config schema is
- * wrong, and a lenient alias here would fossilize the wrong contract
- * (AGENTS.md Prime Directive #12).
+ * `config.storage` is deliberately NOT read (framework#4167). It was never a
+ * stack key: `ObjectStackDefinitionSchema` does not declare it, and the schema
+ * is not `.strict()`, so `defineStack` — which every documented authoring path
+ * and every compiled artifact goes through — strips it before `serve` could
+ * ever see it. It arrived here only from a bare-object config on the
+ * config-boot path, i.e. one unreachable-in-practice combination, where it then
+ * ALSO carried the `driver`/`root` spelling the plugin does not read. Honouring
+ * it on that one path meant the same authoring key worked in one place and
+ * vanished in every other, which is worse than not having it.
+ *
+ * The storage backend is a deployment concern with two real channels: the
+ * `OS_STORAGE_*` env vars (below) and the `storage` settings namespace, which
+ * is also the one with proper credential handling. Authors who write `storage:`
+ * anyway now get told so — `lintUnknownStackKeys` reports undeclared top-level
+ * keys, and `STACK_KEY_GUIDANCE` names both channels.
  */
-export function resolveStorageCapabilityArg(
-  cfgStorage: any,
-  envRoot?: string,
-): StorageCapabilityArg {
-  if (cfgStorage && (cfgStorage.driver || cfgStorage.adapter)) {
-    return { options: cfgStorage };
-  }
+export function resolveStorageCapabilityArg(envRoot?: string): StorageCapabilityArg {
   const rootDir = envRoot?.trim() || '.objectstack/data/uploads';
   return { options: { adapter: 'local', local: { rootDir } }, localRoot: rootDir };
 }

packages/cli/src/commands/validate.ts

@@ -10,6 +10,7 @@ import {
   ObjectStackDefinitionSchema,
   normalizeStackInput,
   lintUnknownAuthoringKeys,
+  lintUnknownStackKeys,
   formatUnknownAuthoringKey,
   type ConversionNotice,
 } from '@objectstack/spec';
@@ -97,8 +98,10 @@ export default class Validate extends Command {
       // key the author actually wrote. Computed here rather than down in the
       // warnings section so the `--json` path reports it too — the
       // "computed, then discarded" shape this file already had to fix once.
-      const unknownKeyWarnings = lintUnknownAuthoringKeys(normalized as Record<string, unknown>)
-        .map(formatUnknownAuthoringKey);
+      const unknownKeyWarnings = [
+        ...lintUnknownStackKeys(normalized as Record<string, unknown>, ObjectStackDefinitionSchema),
+        ...lintUnknownAuthoringKeys(normalized as Record<string, unknown>),
+      ].map(formatUnknownAuthoringKey);
       const result = ObjectStackDefinitionSchema.safeParse(normalized);
 
       if (!result.success) {