fix(rest): parse api.projectResolution at the construction seam, and retire the green case that defended its omit (#12450)

.changeset/rest-project-resolution-parsed.md

@@ -0,0 +1,30 @@
+---
+'@objectstack/rest': minor
+---
+
+**BREAKING (accept-set tightening)**: `RestServer` now parses `api.projectResolution`
+against `RestApiConfigSchema` at construction, instead of `.omit()`ing it out of that
+parse.
+
+The exemption existed because `@objectstack/runtime`'s standalone stack shipped
+`projectResolution: 'none'` — a value the declared enum has never contained — and
+`os serve` forwarded it straight in. Because `RestApiConfigSchema` is a non-strict
+object, omitting the key meant the undeclared value arrived, was silently stripped as
+an unknown key, and took `'auto'`'s branch by fallthrough, while the discovery handler
+copied it verbatim into `discovery.scoping.resolution` — publishing a payload the
+platform's own `DiscoverySchema` rejects, on every boot. #11999 (PR #12444) settled the
+disagreement by migrating the producer onto the declared `'auto'`; this change withdraws
+the exemption it justified.
+
+**What changes for a caller:** `api.projectResolution` must now be one of the three
+values the schema has always declared — `'required'`, `'optional'` or `'auto'`. Anything
+else, `'none'` included, is refused at construction with a message naming the key. A
+census of every `projectResolution` value in this repo found exactly those three plus
+the retired one, and the retired one now survives only inside assertions that it is no
+longer emitted — so no in-repo boot path is affected.
+
+If a config of yours is refused, correct the value at its producer. Do not re-add the key
+to the `.omit()`: a strategy outside the enum is wrong where it is written, not where it
+is read.
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable is removed or renamed: `RestApiConfigSchema` is untouched, its enum has carried the same three members throughout, and no spec key, export or config field changes spelling. There is also no old declared spelling for the ledger to name — a conversion entry rewrites a previously-LEGAL authorable value into its new one, and `'none'` was never declared; it was accepted only because this seam skipped the parse, so `objectstack migrate meta` has nothing to rewrite. And no conversion could decide it: `'none'` read as "no scoping at all", which is the `enableProjectScoping` switch rather than the strategy, so the producer's own migration to `'auto'` (#11999) rests on an analysis that holds only when scoping is OFF — every reader short-circuits on that flag first. A third-party config that wrote `'none'` WITH scoping enabled states an intent no mechanical rewrite can pick between, exactly as with the `api.version` refusals this seam already carries. Nor is the ledger the only channel that reaches an upgrader here, which is the D7 discriminator: unlike a runtime interface with no metadata surface, this value now meets a loud schema rejection at construction naming the key and the declared rule. -->

packages/rest/src/rest-config-parse-not-cast.test.ts

@@ -30,11 +30,31 @@
  *
  * ⚠️ §C is also where the first round's census miss is pinned. That census was
  * scoped to this package; the risk surface is EVERY package that constructs a
- * REST server, and `packages/cli`'s `os serve` ships
+ * REST server, and `packages/cli`'s `os serve` shipped
  * `projectResolution: 'none'` — a value the declared enum does not contain and
- * `@objectstack/runtime` declares as a literal type. Five `packages/cli` e2e
+ * `@objectstack/runtime` declared as a literal type. Five `packages/cli` e2e
  * boots went red in CI. The lesson, written where the next author will hit it:
  * the search radius belongs where the CONSUMERS live, not where the change does.
+ *
+ * [#12450] That miss is what bought `projectResolution` an `.omit()` from the
+ * declared parse — and this file then carried a GREEN §C case defending the
+ * exemption: "KEEPS `projectResolution: \"none\"` — the value this platform
+ * actually ships". #11999 migrated the producer off that value (PR #12444) and
+ * the case did not notice: it called `construct()` with a hand-written literal,
+ * so its premise could die while it stayed green. ⛔ THE LESSON, and the reason
+ * the retired value is now a REFUSAL in §A rather than a reworded guard here:
+ * **a test that cannot fail when its premise dies is not protected by the
+ * suite — it is hidden by it. The passing status is what stops anyone looking.**
+ *
+ * ⚠️ The other half of that premise is NOT measurable from this package, and
+ * saying so here is part of the fix. "No boot path emits the retired value any
+ * more" is a claim about the PRODUCER, and the producer (`@objectstack/runtime`)
+ * depends on this package — so the coupling cannot be imported in this
+ * direction without a cycle, and it lives at the producer instead:
+ * `packages/runtime/src/standalone-stack.test.ts` drives the REAL emitted `api`
+ * block through a REAL `RestServer` construction. THAT is the case that goes red
+ * if the platform ever emits the retired value again; the two below only pin
+ * what this seam does with a value once it arrives.
  */
 
 import { describe, it, expect, vi } from 'vitest';
@@ -94,6 +114,31 @@ describe('[#11637] §A RestServer construction runs RestApiConfigSchema', () =>
  expect(() => construct({ enableCrud: 'yes' as never })).toThrow(/api\.enableCrud/);
  });
 
+ it('[#12450] refuses the retired `projectResolution: "none"` — the parse finally reaches this key', () => {
+ // `.omit()`ed from the declared parse until #12450, so this seam
+ // ACCEPTED a value `RestApiConfigSchema` has never declared. Measured on
+ // the pre-change tree: `construct({ projectResolution: 'none' })`
+ // returned a server and `getApiBasePath()` answered `/api/v1`.
+ // Declared-but-never-executed is the defect this whole file is named
+ // after (#11637); #11999 removed the reason for the exemption by
+ // migrating `@objectstack/runtime` onto the declared `'auto'`.
+ let message = '';
+ try {
+ construct({ projectResolution: 'none' as never });
+ } catch (err: any) {
+ message = String(err?.message ?? err);
+ }
+ // POSITIVE CONTROL for the negative assertion below, and not a substring
+ // of it: an empty `message` (nothing thrown) would satisfy any
+ // `not.toContain` vacuously, so the refusal has to be proven present
+ // before its shape can be measured.
+ expect(message, 'the retired value must be refused, and the refusal must name the key').toContain('api.projectResolution');
+ expect(
+ message,
+ 'a projectResolution refusal must not diagnose `version`, a key this config never wrote',
+ ).not.toContain('/api//');
+ });
+
  it('appends the version rationale ONLY when `version` is what failed', () => {
  // Caught by this change's own ablation: a `projectResolution` refusal
  // printed the whole "an empty version mounts the entire API at /api//"
@@ -205,20 +250,19 @@ describe('[#11637] §C regression guards — the narrowing is exactly the declar
  expect((rest as any).config.api.enableSearch).toBe(false);
  });
 
- it('KEEPS `projectResolution: "none"` — the value this platform actually ships', () => {
- // ⛔ The case CI caught and this file did not. `RestApiConfigSchema`
- // declares `z.enum(['required', 'optional', 'auto'])`, but the value
- // `os serve` forwards is `'none'`: `@objectstack/runtime`'s
- // `StandaloneStackResult.api` DECLARES the literal type
- // `{ enableProjectScoping: false; projectResolution: 'none' }`, and
- // `serve.ts` passes it through (`?? 'auto'` does not fire — `'none'` is
- // not nullish). Parsing this key would turn every `os serve` boot into
- // a crash; five packages/cli e2e boots did exactly that before the key
- // was `.omit()`ed. Which spelling is right is a contract question about
- // project-scoping semantics, filed as #11999 — NOT this seam's to
- // settle by refusing the value the platform ships.
- expect(() => construct({ enableProjectScoping: false, projectResolution: 'none' })).not.toThrow();
- expect(construct({ projectResolution: 'none' as never }).getApiBasePath()).toBe('/api/v1');
+ it('[#12450] accepts every `projectResolution` the declared enum contains — the narrowing is the retired value ONLY', () => {
+ // The BOUND on the change, and the half that makes "exactly one value
+ // starts being rejected" a measurement rather than a claim. Census over
+ // the tree at the time of #12450: four spellings appear anywhere as a
+ // value for this key — `'required'`, `'optional'`, `'auto'` and the
+ // retired `'none'` — so these three ARE the population that must keep
+ // constructing. Read back off the normalized config rather than off a
+ // mount path: a strategy that parsed and was then dropped in
+ // normalization would still answer `/api/v1`.
+ for (const projectResolution of ['required', 'optional', 'auto'] as const) {
+ const rest = construct({ enableProjectScoping: true, projectResolution });
+ expect((rest as any).config.api.projectResolution, projectResolution).toBe(projectResolution);
+ }
  });
 
  it('KEEPS the retired `api.requireAuth` warn-and-ignore posture (#3963)', () => {

packages/rest/src/rest-server.ts

@@ -713,7 +713,7 @@ type NormalizedRestServerConfig = {
 };
 
 /**
- * The declared `api` contract, minus the retired keys whose posture this seam
+ * The declared `api` contract, minus the ONE retired key whose posture this seam
  * does not own (see {@link RestServer.assertDeclaredApiConfig}).
  *
  * Built on first use, not at module load: `RestApiConfigSchema` is a
@@ -723,7 +723,7 @@ type NormalizedRestServerConfig = {
  * `RestServer` is constructed per boot (and per test).
  */
 function buildDeclaredApiConfigSchema() {
- return RestApiConfigSchema.omit({ requireAuth: true, projectResolution: true });
+ return RestApiConfigSchema.omit({ requireAuth: true });
 }
 let declaredApiConfigSchemaCache: ReturnType<typeof buildDeclaredApiConfigSchema> | undefined;
 
@@ -2989,23 +2989,33 @@ export class RestServer {
  * tombstone ages out of `packages/spec` this line fails `tsc` — the
  * drift cannot go silent.
  *
- * - `api.projectResolution` is `.omit()`ed for a DIFFERENT reason, and it
- * is the one CI caught: the declared enum is
+ * - `api.projectResolution` USED to be `.omit()`ed here too, for a
+ * DIFFERENT reason, and it was the one CI caught: the declared enum is
  * `z.enum(['required', 'optional', 'auto'])`, and the value this
- * platform actually ships is `'none'` — produced by
- * `@objectstack/runtime`'s standalone stack, whose
- * `StandaloneStackResult.api` DECLARES the literal type
- * `{ enableProjectScoping: false; projectResolution: 'none' }`, and
- * forwarded by `os serve` straight into this config
+ * platform shipped was `'none'` — produced by `@objectstack/runtime`'s
+ * standalone stack and forwarded by `os serve` straight into this config
  * (`apiConfig.projectResolution ?? 'auto'` — `'none'` is not nullish, so
- * it passes through). Three packages disagree about this key's
- * vocabulary, and they have disagreed silently for exactly as long as
- * nothing ran the schema. Parsing it here does not settle that
- * disagreement, it just turns every `os serve` boot into a crash.
- * ⛔ Which spelling is right — teach the enum `'none'`, or migrate the
- * runtime onto `'auto'` — is a contract question about project-scoping
- * semantics that this seam cannot answer and this card does not own
- * (`packages/spec` is `domain:spec`'s surface). Filed as #11999.
+ * it passed through). Three packages disagreed about this key's
+ * vocabulary, silently, for exactly as long as nothing ran the schema.
+ * Parsing it THEN would not have settled the disagreement, only turned
+ * every `os serve` boot into a crash — so it was filed as #11999 rather
+ * than decided here.
+ *
+ * [#11999 / PR #12444] settled it at the producer: the runtime migrated
+ * onto the declared `'auto'`. `'none'` read as "no scoping at all" and
+ * got `'auto'`'s behaviour by fallthrough anyway, because every reader
+ * that acts on the key is gated on `enableProjectScoping` first — but
+ * the discovery handler below copies the value into
+ * `discovery.scoping.resolution` UNCONDITIONALLY, and `DiscoverySchema`
+ * declares that field as the same three-member enum, so shipping
+ * `'none'` published a payload the platform's own schema rejects.
+ *
+ * [#12450] withdrew the exemption: the key is parsed here now, so an
+ * undeclared strategy is refused at construction instead of being
+ * stripped as an unknown key (this `z.object()` is non-strict) and
+ * taking `'auto'`'s branch in silence. ⛔ Do not re-add it to the
+ * `.omit()` to make some config boot — a strategy outside the enum is
+ * wrong where it is WRITTEN, not where it is read.
  *
  * The sibling sub-objects (`crud`, `metadata`, `batch`, `routes`) are still
  * cast, not parsed, and carry declared constraints of their own

packages/runtime/src/standalone-stack.test.ts

@@ -25,6 +25,15 @@ import { createDefaultHostConfig, resolveDefaultArtifactPath } from './default-h
 // the value `createStandaloneStack` actually returns, never against a copy of
 // it. `@objectstack/spec` is a plain `dependencies` entry of this package.
 import { RestApiConfigSchema } from '@objectstack/spec/api';
+// [#12450] The CONSUMER seam itself, imported at MODULE LOAD (the #10126 rule
+// below): `@objectstack/rest` is a plain `dependencies` entry of this package,
+// and this package's vitest config aliases it to REST's SOURCE — so the case
+// that drives it measures the seam as it stands in this checkout, not as it
+// stands in some build artifact. The dependency only runs this way:
+// `@objectstack/runtime` depends on `@objectstack/rest`, so the producer→consumer
+// coupling cannot be written from inside `packages/rest` without a cycle, and
+// this file is where it has to live.
+import { RestServer } from '@objectstack/rest';
 // The REAL resolution, imported — not reproduced. `@objectstack/plugin-security`
 // is a plain `dependencies` entry of this package (and another test in this same
 // package, src/domains/share-links-enforcement-context.test.ts, already imports
@@ -155,6 +164,34 @@ describe('createStandaloneStack — surfaces app RBAC from the artifact (ADR-005
  expect(issue?.code).toBe('invalid_value');
  });
 
+ it('[#12450] the emitted `api` block also survives the REAL RestServer construction', () => {
+ // The case above proves the emitted value is DECLARED. It cannot prove that
+ // the seam which CONSUMES it runs that declaration — and until #12450 that
+ // seam did not: `RestServer` `.omit()`ed `projectResolution` out of its own
+ // parse, so the undeclared strategy constructed a server happily for as long
+ // as this factory shipped it. A schema pin is not an execution pin, and that
+ // gap is exactly #11637's defect class.
+ //
+ // ⭐ THIS is the case that goes RED if this factory ever emits an undeclared
+ // strategy again — measured against `result.api`, the REAL boot output, never
+ // a restatement of it. Its sibling in
+ // `packages/rest/src/rest-config-parse-not-cast.test.ts` pins what the seam
+ // does with such a value once it arrives; only this one can see what the
+ // platform actually hands it.
+ const httpServer = {
+ get: () => {}, post: () => {}, put: () => {}, delete: () => {}, patch: () => {},
+ use: () => {}, listen: () => {}, close: () => {},
+ } as any;
+ const protocol = {
+ getMetaItems: async ({ type }: { type: string }) => ({ type, items: [] }),
+ } as any;
+ const rest = new RestServer(httpServer, protocol, { api: { ...result.api } } as any);
+ // Read back off the normalized config: a strategy that parsed and was then
+ // dropped in normalization would still answer a correct mount path.
+ expect((rest as any).config.api.projectResolution).toBe('auto');
+ expect((rest as any).config.api.enableProjectScoping).toBe(false);
+ });
+
  it('the surfaced config feeds the REAL appSecurityPluginOptions → the app profile', () => {
  // Reproduce serve.ts's merge: `config = { ...originalConfig, ...standaloneStack }`,
  // then `new SecurityPlugin(appSecurityPluginOptions(config))`.