fix(spec,cli): config.storage was never a stack key — stop reading it, and report undeclared top-level keys (#4167) - #4189
Merged
Conversation
…, and report undeclared top-level keys (#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 (bare-object config on the config-boot path) then carried the `driver`/`root` spelling the plugin does not read either. So one authoring key worked on a single unreachable-in-practice path and vanished 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, and the production warning names the two channels that work (OS_STORAGE_*, Setup → Settings) instead of advertising the one that does not. - `lintUnknownAuthoringKeys` now covers TOP-LEVEL stack keys, not just object and field keys. `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`), and the rule now runs on a stack with no `objects` at all, which previously exited early. - `os migrate files-to-references` shares the resolver. It built the same dead `{ driver: 'local', root }`, so its adapter used `./storage` while the server writes under `.objectstack/data/uploads` since #4096 — and that command reconciles what records claim against what storage holds, so a disagreeing root reconciled against the wrong tree. `lintUnknownAuthoringKeys(rawStack)` becomes `(rawStack, stackSchema)`. Required rather than optional so a caller that forgets it fails to compile instead of silently losing the check — the exact failure this rule reports. Injected rather than imported because stack.zod.ts imports this module and importing back would close a cycle. Verified end to end: authoring `storage:` through defineStack warns at load ("defineStack: stack.storage: 'storage' is not a declared stack key…"), and `os compile` reports it for configs that skip defineStack. spec 273 files / 7150 tests, cli 89 files / 916 tests, root `pnpm lint` clean. Nothing that worked is being removed: `storage` was never in the schema, is undocumented, and has no consumer in objectstack-ai/cloud (checked directly). Closes #4167 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 112 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…tack keys too (#4167) The one page that documents the #3786 rule described it as object/field only, which #4167 made inaccurate: `lintUnknownAuthoringKeys` now also reports keys `ObjectStackDefinitionSchema` does not declare. - Section title and intro name all three schemas. - The gate table row becomes "Undeclared stack/object/field keys". - Adds the top-level case with both guidance kinds, using `storage` as the worked example — it is the one where the silence is easiest to miss, because the key reads as configuration that took effect, and points at the two channels that do configure storage (OS_STORAGE_* and the Settings UI, which is also where credentials belong — a stack definition is committed to git and compiled into any published artifact). Docs-only; found by acting on the docs-drift advisory rather than waving it through. `pnpm check:doc-authoring` clean. Refs #4167 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
…4167) `check:api-surface` failed on the #4167 commit: exporting `STACK_KEY_GUIDANCE` adds to `@objectstack/spec`'s public surface, and the generated snapshot is the gate's baseline. ./data + STACK_KEY_GUIDANCE (const) @objectstack/spec public API changed: 0 breaking (removed/narrowed), 1 added. Regenerated with the command the gate names. The diff is one line — the export is additive, alongside the FIELD_KEY_GUIDANCE / OBJECT_KEY_GUIDANCE entries it sits with, and nothing was removed or narrowed. Also ran the rest of the gates CI runs, so this does not take another round: doc-authoring, role-word, org-identifier, authz-resolver, route-envelope, error-code-casing, wildcard-fallthrough, release-notes, node-version, plus the spec package's authorable-surface, spec-changes, upgrade-guide, liveness and docs checks — all green. Refs #4167 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
…gs-visibility-12kkwh # Conflicts: # content/docs/deployment/validating-metadata.mdx # packages/spec/src/data/authoring-key-lint.test.ts # packages/spec/src/data/authoring-key-lint.ts
#4178 landed while this was in review and refactored the unknown-authoring-key rule from a two-surface check (object/field) into a walker that derives its coverage from `PLURAL_TO_SINGULAR` and reads each schema's own unknown-key posture. This rebases the #4167 contribution onto that architecture instead of carrying the version it replaced. What changes vs. the pre-merge shape: - `lintUnknownAuthoringKeys` keeps its single-argument signature. The top-level pass is a separate export, `lintUnknownStackKeys(rawStack, stackSchema)`. - It reuses the walker's own `keyPosture`, so it lints only while the stack schema STRIPS unknown keys and goes quiet if that schema is ever made strict — the lint can never become a second, disagreeing voice next to the parse. - `STACK_KEY_GUIDANCE` stays in `data/authoring-key-lint.ts` beside the other two curated tables, held to the same non-rotting discipline. The two passes are complementary, not redundant: 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 reports nothing. A test pins exactly that. Verified end to end after the rebuild: `defineStack` warns at config-load time, and `os validate` / `os build` report `stack.storage` with its prescription plus `stack.datasource` with its edit-distance suggestion, alongside the field-level findings the walker already produced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
github-actions
Bot
added
documentation
The top-level coverage moved to `lintUnknownStackKeys` when this was rebuilt on #4178's walker; the comment still pointed at `lintUnknownAuthoringKeys`, which no longer does that job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
Caught by running the new lint against our own shipped examples before
merging: `examples/app-todo` and `examples/app-showcase` both got
⚠ stack.onEnable: 'onEnable' is not a declared stack key, so its value is
dropped at load.
which is false. `onEnable` is a function, so `ObjectStackDefinitionSchema`
cannot declare it and `dist/objectstack.json` cannot carry it — but `AppPlugin`
reads it straight off the authored bundle and calls it at `start()`
(`app-plugin.ts`), and the artifact-boot path grafts it back (#4095). It is the
documented place to register action handlers. "Not declared" and "dropped at
load" are different claims, and this is the one surface where they come apart.
New `STACK_RUNTIME_MEMBERS` names the authored top-level members the runtime
honours off the bundle; `lintUnknownStackKeys` treats them as declared. The
CLI's `GRAFTABLE_RUNTIME_MEMBERS` is now DERIVED from it instead of restating
it — two hand-written copies could disagree, and the disagreement would be
silent in exactly the direction this lint family exists to catch.
`onDisable` is deliberately excluded from the exemption: it is declared in the
protocol but no kernel, runtime or service calls it, so a value written there
really does go nowhere. A test pins both halves of that distinction.
Re-harvested after the fix: all three shipped examples validate with zero
undeclared-key warnings.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
os-zhuang
marked this pull request as ready for review
July 30, 2026 15:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4167.
What the investigation changed
#4167 framed this as a choice: teach the plugin the
driver/rootdialect, or reject it at authoring. The evidence made it a third thing.storagewas never a stack key at all.ObjectStackDefinitionSchemadeclares 41 top-level keys andstorageis not among them, and the schema is not.strict()— sodefineStackstrips it, and every documented authoring path goes throughdefineStack. The compiled-artifact path goes through the same parse.So
os serve's branch could only ever fire on a bare-object config on the config-boot path — where the value then also carried thedriver/rootspelling the plugin does not read. One authoring key, working on a single unreachable-in-practice path, silently absent on every other. A host writingstorage: { driver: 's3', … }believed it configured S3 and got local disk.That also rules out the alias option: the
driver/rootdialect has no external author. Repo-wide, nothing inexamples/,content/docs/, or any test writes it — only two CLI-internal fallbacks did, andgit log -S 'driver?:' -- storage-service-plugin.tsreturns nothing, so those keys were never valid. There is no dialect to absorb; there was a typo.The change
servestops readingconfig.storage.resolveStorageCapabilityArgtakes only the env root. The production warning stops advertisingconfig.storageand names the two channels that work:OS_STORAGE_*and Setup → Settings — the latter being the one with real credential handling (s3_secret_access_keyis apassword-typed setting).lintUnknownStackKeys(rawStack, stackSchema), wired intodefineStack,os validateandos compile.storagegets a prescriptive entry naming where the setting really lives; an ordinary misspelling still gets the edit-distance suggestion.os migrate files-to-referencesshares the resolver. It built the same dead{ driver: 'local', root }, so its adapter used./storagewhile the server has written under.objectstack/data/uploadssince StorageServicePlugin warns "storage adapter swapped (LocalStorageAdapter → LocalStorageAdapter) … files may be unreachable" on every clean boot #4096. That command reconciles what records claim against what storage actually holds — a disagreeing root reconciles against the wrong tree, on the command that gates ADR-0104 file-reference rollout.Rebuilt on #4178's walker — and why it is a second pass, not a fold
#4178 landed mid-review and refactored the unknown-key rule from a two-surface check (object/field) into a walker that derives its coverage from
PLURAL_TO_SINGULARand reads each schema's own unknown-key posture. This PR was rebased onto that architecture.lintUnknownAuthoringKeyskeeps its signature — the earlier revision changed it, the current one does not.The top-level pass stays a separate export for two reasons:
stack.zod.tsimports 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 reuses the walker's own
keyPosture, so it lints only while the stack schema strips unknown keys, and goes quiet if that schema ever graduates to.strict()(ADR-0049 / #4001) — the lint can never become a second, disagreeing voice next to the parse.onEnableis exempt — a false positive this PR caught on its own examplesBefore merging I pointed the new lint at the apps we ship.
examples/app-todoandexamples/app-showcaseboth reported:That is false.
onEnableis a function, soObjectStackDefinitionSchemacannot declare it anddist/objectstack.jsoncannot carry it — but it is not lost:AppPluginreads it straight off the authored bundle and calls it atstart(), and the artifact-boot path grafts it back (#4095). It is the documented place to register action handlers. Not declared and dropped at load are different claims, and this is the one surface where they come apart.New
STACK_RUNTIME_MEMBERSnames the authored top-level members the runtime honours off the bundle; the lint treats them as declared. The CLI'sGRAFTABLE_RUNTIME_MEMBERSis now derived from it rather than restating it — two hand-written copies could disagree, and the disagreement would be silent in exactly the direction this rule family exists to catch.onDisableis deliberately not exempt: it is declared in the protocol but no kernel, runtime or service calls it, so a value written there really does go nowhere. A test pins both halves of that distinction.Verified end to end
Authoring path (
defineStack):Compile path (config that skips
defineStack), showing both guidance kinds:Re-harvested after the
onEnablefix: all three shipped examples validate with zero undeclared-key warnings — which is also the first real evidence datapoint the #4001 strict-tier programme wanted out of this rule family.@objectstack/spec274 files / 7147 tests;@objectstack/cli89 files / 916 tests; rootpnpm lintclean;check:api-surfaceand all ten repo guard scripts pass locally.Nothing that worked is being removed
defineStackand bycompile;OS_STORAGE_*and the settings namespace);objectstack-ai/cloud— I attached and searched that repo directly rather than assuming.Found in cloud while checking that — filed as cloud#935
The same dead-key mistake is in
service-cloud's per-environment local-disk fallback, where the discarded value is the one implementing tenant isolation:rootis dropped, so every environment shares one./storage. The isolation that comment describes is not in effect. It is the fallback branch andOS_STORAGE_REQUIRE_REMOTE=1avoids it, and I did not establish that two environments can collide on a storage key — so cloud#935 states the missing boundary without claiming a demonstrated leak. Same file gets the s3 path right, which is what makes it a typo rather than a dialect.If storage should become a real stack key later
That is a separate, legitimate question — and if the answer is yes, the shape to copy is
datasources, which solves credentials by referencingsys_secretinstead of inlining them. That deserves an ADR, not a resurrected undeclared key. This PR does not foreclose it.Generated by Claude Code