Skip to content

Commit aacda8d

Browse files
committed
fix(rest): consume the parsed api sub-config instead of discarding it
`RestServer.normalizeConfig` ran `RestApiConfigSchema` over `config.api` and threw the parsed output away, rebuilding the block from a `??` chain over the raw cast. That chain restated the schema's eleven top-level `z.default(...)`s as eleven literals in `packages/rest`, with nothing pinning that the two stayed equal — a `packages/spec` default change would silently fail to propagate. #11637 made the parse validate-only for two measured reasons; both have since expired (#11983 gave `enableSearch` a declared seat, #12450 withdrew the `projectResolution` omit). Re-measured here: the 14 keys the method reads and the 14 the schema declares after `.omit({ requireAuth: true })` are the same 14 in both directions, so a consumed parse cannot strip anything the runtime honours. `requireAuth` stays omitted and stays warn-and-ignore in the plugin, which reads it off the RAW config. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
1 parent 791a0cb commit aacda8d

1 file changed

Lines changed: 73 additions & 29 deletions

File tree

‎packages/rest/src/rest-server.ts‎

Lines changed: 73 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,15 @@ import { refuseRepeatedQueryParams, assertFilterParamSuppliedOnce } from './quer
8080
// ignored filter is the one wrong answer a caller cannot detect.
8181
import { refuseUnknownQueryParams } from './query-allowlist.js';
8282
import type { DirectMountedRoute, MountedRouteSource } from './direct-mount.js';
83-
import { RestServerConfig, RestApiConfig } from '@objectstack/spec/api';
83+
import { RestServerConfig, RestApiConfigParsed } from '@objectstack/spec/api';
8484
// [#11683] The catalog's own floor for "a required `code` and no more specific
8585
// one" — see its use in `registerSharingEndpoints`, where the nested ADR-0112
8686
// envelope declares `code` REQUIRED while the flat classification it re-dresses
8787
// legitimately carries none.
8888
import { standardErrorCodeForHttpStatus } from '@objectstack/spec/api';
8989
// [#11637] The DECLARED contract for `config.api`, imported as a VALUE rather
9090
// than a type. Both hops into this package were casts, so this schema had
91-
// never run on any deployment path — see `assertDeclaredApiConfig` below.
91+
// never run on any deployment path — see `parseDeclaredApiConfig` below.
9292
import {
9393
RestApiConfigSchema,
9494
CrudEndpointsConfigSchema,
@@ -743,8 +743,13 @@ type NormalizedRestServerConfig = {
743743
enableSearch: boolean;
744744
enableProjectScoping: boolean;
745745
projectResolution: 'required' | 'optional' | 'auto';
746-
documentation: RestApiConfig['documentation'];
747-
responseFormat: RestApiConfig['responseFormat'];
746+
// [#14366] The PARSED shape, not the authored one: this block is
747+
// built from `RestApiConfigSchema`'s output, so a `documentation` or
748+
// `responseFormat` the caller wrote arrives with its OWN declared
749+
// inner defaults applied (`documentation.enabled`, `.title`;
750+
// `responseFormat.envelope`, `.includeMetadata`, `.includePagination`).
751+
documentation: RestApiConfigParsed['documentation'];
752+
responseFormat: RestApiConfigParsed['responseFormat'];
748753
};
749754
crud: {
750755
operations: {
@@ -799,7 +804,7 @@ type NormalizedRestServerConfig = {
799804
*
800805
* `api` is the one entry with a subtraction: its retired `requireAuth`
801806
* tombstone is `.omit()`ed because this seam does not own that key's posture
802-
* (see {@link RestServer.assertDeclaredApiConfig}). The four siblings carry no
807+
* (see {@link RestServer.parseDeclaredApiConfig}). The four siblings carry no
803808
* tombstone of their own and are taken whole. ⛔ `RestServerConfigSchema` — the
804809
* whole-config schema — is deliberately NOT in this table: its `openApi31`
805810
* tombstone (#4579) is a `retiredKey()` whose parse REFUSES the key, while
@@ -824,6 +829,13 @@ function buildDeclaredSubConfigSchemas() {
824829
}
825830
type DeclaredSubConfigSchemas = ReturnType<typeof buildDeclaredSubConfigSchemas>;
826831
type DeclaredSubConfigName = keyof DeclaredSubConfigSchemas;
832+
/**
833+
* [#14366] The parsed `api` sub-object, which `normalizeConfig` now BUILDS
834+
* FROM. Taken off the table's own entry rather than off `RestApiConfigParsed`,
835+
* so it is the post-`.omit()` shape: the retired `requireAuth` tombstone is
836+
* absent here exactly as it is absent from the schema this seam runs.
837+
*/
838+
type DeclaredApiConfigParsed = z.output<DeclaredSubConfigSchemas['api']>;
827839
let declaredSubConfigSchemasCache: DeclaredSubConfigSchemas | undefined;
828840
function declaredSubConfigSchemas(): DeclaredSubConfigSchemas {
829841
return (declaredSubConfigSchemasCache ??= buildDeclaredSubConfigSchemas());
@@ -3616,8 +3628,9 @@ export class RestServer {
36163628
* walked straight past it and mounted the whole API at `/api//`, and
36173629
* `'v1/beta'` spliced an extra path segment into every route.
36183630
*
3619-
* VALIDATION ONLY — the parsed output is deliberately discarded and the
3620-
* normalization below keeps reading the raw input. Two measured reasons:
3631+
* [#14366] The parsed output is CONSUMED — `normalizeConfig` builds the
3632+
* `api` block from what this returns. It was VALIDATE-ONLY from #11637
3633+
* until then, for two measured reasons that have both since expired:
36213634
*
36223635
* - `enableSearch` USED to be the silent-strip trap here: it was read
36233636
* below through `as any` and declared nowhere in `packages/spec`, so
@@ -3626,9 +3639,27 @@ export class RestServer {
36263639
* it off (the ADR-0104 class `shared/retired-key.ts` exists to
36273640
* prevent). #11983 gave it a declared seat
36283641
* (`RestApiConfigSchema.enableSearch`, default `true`), so the parse
3629-
* now preserves it — but the discard stays, for the omitted keys:
3630-
*
3631-
* - the retired `api.requireAuth` key is `.omit()`ed rather than enforced.
3642+
* preserves it.
3643+
*
3644+
* - `api.projectResolution` was `.omit()`ed until #12450 withdrew it.
3645+
*
3646+
* ⇒ Re-measured at #14366 on the landed tree, because the discard is
3647+
* only safe to remove if the key diff is EMPTY: the 14 keys
3648+
* `normalizeConfig` reads and the 14 `RestApiConfigSchema` declares
3649+
* after the `.omit()` are the same 14, in both directions. So the
3650+
* non-strict parse cannot strip anything the runtime honours, and the
3651+
* schema's `.default()`s ARE the defaults — one source, not the two
3652+
* that a `??` chain here duplicated key for key.
3653+
*
3654+
* The one measured behaviour delta is bounded and named: a
3655+
* `documentation` or `responseFormat` object the caller WRITES now
3656+
* arrives carrying its own declared inner defaults, where the `??`
3657+
* chain copied the authored object through untouched. Both keys have
3658+
* zero read sites outside this block (the #14369 census), so nothing
3659+
* observes it today — but it is a real change to this structure's
3660+
* contents and belongs in the record rather than in a reader's surprise.
3661+
*
3662+
* - the retired `api.requireAuth` key is STILL `.omit()`ed rather than enforced.
36323663
* #3963 retired it with a deliberate warn-and-ignore posture
36333664
* (`rest-api-plugin.ts`: "is IGNORED"), chosen in a world where nothing
36343665
* parsed this config; converting that into a boot failure is that
@@ -3676,8 +3707,8 @@ export class RestServer {
36763707
* schema's defaults key for key today, and folding it onto the parse is a
36773708
* separate, separately-measured change — not a rider on the siblings.
36783709
*/
3679-
private assertDeclaredApiConfig(api: unknown): void {
3680-
parseDeclaredSubConfig('api', declaredSubConfigSchemas().api, api, (issues) => (
3710+
private parseDeclaredApiConfig(api: unknown): DeclaredApiConfigParsed {
3711+
return parseDeclaredSubConfig('api', declaredSubConfigSchemas().api, api, (issues) => (
36813712
// The `version` rationale is appended only when `version` is what
36823713
// failed. Measured during #11637's own ablation: a
36833714
// `projectResolution` refusal printed the whole "an empty version
@@ -3697,12 +3728,20 @@ export class RestServer {
36973728
* Normalize configuration with defaults
36983729
*/
36993730
private normalizeConfig(config: RestServerConfig): NormalizedRestServerConfig {
3700-
// [#11637] `api`: parse BEFORE the cast, not instead of it — the cast
3701-
// is what makes the `api` block below type-check, and it is only sound
3702-
// once the declared contract has actually been run. Validate-only; see
3703-
// `assertDeclaredApiConfig` for why its parsed output is discarded.
3704-
this.assertDeclaredApiConfig(config.api);
3705-
const api = (config.api ?? {}) as Partial<RestApiConfig>;
3731+
// [#11637 / #14366] `api`: parsed AND consumed. #11637 ran the declared
3732+
// contract here but discarded its output, leaving the block below to be
3733+
// built from a cast over the raw input through a `??` chain that
3734+
// duplicated `RestApiConfigSchema`'s defaults key for key — ELEVEN
3735+
// literals in `packages/rest` restating the eleven top-level
3736+
// `z.default(...)`s in `packages/spec`, with nothing pinning that the
3737+
// two stayed equal. (Eleven, measured on both sides at #14366; the
3738+
// filing card said twelve, having counted the `config.api ?? {}` that
3739+
// guards the whole object rather than a per-key default.)
3740+
// #14366 folded the chain onto the parse after re-measuring the key
3741+
// diff empty in both directions (see `parseDeclaredApiConfig`), so the
3742+
// schema is now the single source of these defaults. The cast is gone
3743+
// with it: the parsed output is already typed.
3744+
const api = this.parseDeclaredApiConfig(config.api);
37063745
// [#11984] The four siblings: parsed AND consumed. Each used to be
37073746
// `(config.<sub> ?? {}) as Partial<...>`, so `batch.maxBatchSize: 0`
37083747
// was the live batch cap (`0` is not nullish) and
@@ -3719,19 +3758,24 @@ export class RestServer {
37193758
const routes = parseDeclaredSubConfig('routes', schemas.routes, config.routes);
37203759

37213760
return {
3761+
// Keys listed rather than spread: `NormalizedRestServerConfig`
3762+
// declares `documentation` / `responseFormat` as REQUIRED (possibly
3763+
// `undefined`) while the schema declares them `.optional()`, so a
3764+
// spread would not satisfy this type — and listing them is also
3765+
// what makes the empty key diff readable at the seam it protects.
37223766
api: {
3723-
version: api.version ?? 'v1',
3724-
basePath: api.basePath ?? '/api',
3767+
version: api.version,
3768+
basePath: api.basePath,
37253769
apiPath: api.apiPath,
3726-
enableCrud: api.enableCrud ?? true,
3727-
enableMetadata: api.enableMetadata ?? true,
3728-
enableUi: api.enableUi ?? true,
3729-
enableBatch: api.enableBatch ?? true,
3730-
enableDiscovery: api.enableDiscovery ?? true,
3731-
enableOpenApi: api.enableOpenApi ?? true,
3732-
enableSearch: api.enableSearch ?? true,
3733-
enableProjectScoping: api.enableProjectScoping ?? false,
3734-
projectResolution: api.projectResolution ?? 'auto',
3770+
enableCrud: api.enableCrud,
3771+
enableMetadata: api.enableMetadata,
3772+
enableUi: api.enableUi,
3773+
enableBatch: api.enableBatch,
3774+
enableDiscovery: api.enableDiscovery,
3775+
enableOpenApi: api.enableOpenApi,
3776+
enableSearch: api.enableSearch,
3777+
enableProjectScoping: api.enableProjectScoping,
3778+
projectResolution: api.projectResolution,
37353779
documentation: api.documentation,
37363780
responseFormat: api.responseFormat,
37373781
},

0 commit comments

Comments
 (0)