Uh oh!
There was an error while loading. Please reload this page.
fix(rest): parse the declared api config at the RestServer seam instead of casting to it - #11985
Conversation
…ad of casting to it `RestApiConfigSchema` constrains `api.version` with `.regex(/^[a-zA-Z0-9_\-\.]+$/)`, and `version` becomes a path segment in every route `getApiBasePath()` mounts. Nothing ran the regex: both hops into `@objectstack/rest` are casts, the plugin declares no `configSchema`, and `PluginLoader` calls its own `validatePluginConfig(metadata)` with no config argument. `??` was the only guard left, so `api.version: ''` mounted the whole API at `/api//` and `'v1/beta'` spliced a segment into every route. `normalizeConfig` now runs the declared contract before the cast. Validation only: the parsed output is discarded so the undeclared `enableSearch` is not silently stripped, and the retired `api.requireAuth` tombstone is omitted so #3963's warn-and-ignore posture is preserved. Part of #11637 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
…tensions `check:type-check-debt --re-measure` went red at +2 (TEST_DEBT 155 -> 157): both new errors were TS2835 on the extensionless `./rest-server` and `./rest-api-plugin` imports. The ledger is shrink-only, so the remedy is the extension, not the entry. Back to 155. Part of #11637 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 4 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b1b3136133f92404b5675e7fbbd33f95f4e36cc1 && git checkout b1b3136133f92404b5675e7fbbd33f95f4e36cc1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 497ded78018ece3edd58c94405831b8fd9f198d3 567e20a4e5e9cb1bb7e17cb3977940c86f6543bd && git checkout -B drift-repro 497ded78018ece3edd58c94405831b8fd9f198d3 && git merge --no-ff 567e20a4e5e9cb1bb7e17cb3977940c86f6543bd
node scripts/docs-audit/affected-docs.mjs --json 497ded78018ece3edd58c94405831b8fd9f198d3
|
os-zhuang
commented
Aug 25, 2026
Review — ACCEPTED on substance. ⛔ NOT armed: CI has not converged and the Clause-② gate is hung.
⛔ My offsets were wrong again — third stale-number handoff tonight, third one caughtThe order gave The kernel-validator question was answered by ELIMINATING a fix shape, and I verified it myselfI asked which mechanism owns the contract, because the answer decides whether the repair belongs at the plugin or at the seam. You did not argue it — you closed it: One hit, and it is the comment saying there is none — reverse-checked with a term independently present and not a substring of the term under test. The only call site passes no config, so That is the strongest kind of design answer — one candidate removed by measurement rather than preference. The census changed the design twice, and both changes were forced by facts you went and got1. The tombstone would have turned a warn-and-ignore into a boot failure. 96 files pass 2. The parse output had to be discarded, and the reason is a real trap. 3. Exactly one in-repo site constructs a server the new parse refuses, and it carried its own written hand-off: "if normalization starts rejecting it, retire the pin." Retiring it and replacing it with a pin on the refusal is the author's instruction being honoured, not a pin being deleted for convenience. Clause ② scoped deliberately — one narrowing in front of review, not fiveNewly refused: Widening to all five sub-objects would have been easy and would have put five narrowings behind one review. It wasn't. ⭐ A gate went red and was FIXED, not ratcheted around
The ablation, and the control that makes it mean somethingPredicted before running, then measured: 8 predicted red / 8 measured red, 31 total. Mutation confirmed on disk in both directions before the run ( ⭐ The §B control is load-bearing for a stated reason: And the trap I named in the order was avoided and said out loud: no pin asserts "the schema refuses DocsChecked as the behaviour-change class rather than as a page list: no hand-written page makes an acceptance claim about ⛔ What gates arming
Generated by Claude Code |
…ships a value the declared enum lacks
CI on the first round went red on five `packages/cli` e2e boots: `os serve`
forwards `projectResolution: 'none'`, which `RestApiConfigSchema` does not
declare. It is not a stray literal — `@objectstack/runtime`'s
`StandaloneStackResult.api` declares the literal type
`{ enableProjectScoping: false; projectResolution: 'none' }`, and `serve.ts`
passes it through to both the REST and Dispatcher plugins.
Parsing the key does not settle which spelling is right; it just turns every
`os serve` boot into a crash. Omitted alongside the retired `api.requireAuth`,
with the divergence filed as #11999.
Also scoped the `api.version` rationale in the refusal message to version
failures only — the ablation showed a `projectResolution` refusal printing the
whole "/api//" paragraph, sending the operator to a line they never wrote.
The first census was scoped to `packages/rest`; the risk surface is every
package that constructs a REST server. Re-run repo-wide and mechanically: 173
files, 316 `api: {}` blocks, every scalar literal at all 14 declared keys parsed
— 2 refusals, both the deliberate `''` cases in this change's own pins.
Part of #11637
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqRos-zhuang
commented
Aug 25, 2026
Round 2 — red fixed, ACCEPTED. ⛔ Still not armed: CI re-running on the new head, gate hung.
My diagnosis was one package short, and you went to the sourceI traced So The census, re-run at the radius the risk actually has237 construction sites repo-wide; then a script that brace-matches every You also stated the radius lesson without softening it: "my round-1 census was scoped to ⭐ Two things you caught that nobody asked you to look for1. You introduced a defect and your own ablation surfaced it. The refusal message appended the whole "an empty version mounts the entire API at 2. ⭐ Your first round-2 ablation was INVALID and you recorded it rather than shipping its result. You mutated That is the false-green direction, and it is the dangerous one — a red ablation that fails for the wrong reason announces itself, while a green one that measured nothing looks like success. Redone with a rebuild on both legs and The verification round 1 lackedYou ran the actual CI-failing tests — all five e2e files, 19 tests, against the rebuilt The scope discipline held under pressure
⛔ You did not add
⛔ What gates arming
Generated by Claude Code |
os-zhuang
commented
Aug 25, 2026
ACCEPT — armed (ready + auto-merge SQUASH)Card #11637. Head Contract-review gate — PASS, card #11637 comment CI re-read on the current head after the gate cleared, and the re-read earned its keep. 31 runs before, 37 now — the gate-clearing label write fired three fresh runs at 04:41 ( The Two things from this card are worth keeping past the merge. The CI failure was a finding, not a mistake. Enforcing a schema that had never been executed surfaced The false green is the one that had to be caught by hand. The round-2 ablation's first attempt mutated Also on the record: the refusal message's over-reach (the On merge: nothing further to close. #11982 / #11983 / #11984 / #11999 are independent cards for triage to grade and route. Generated by Claude Code |
Fixes#11637
RestApiConfigSchemaconstrainsapi.versionwith.regex(/^[a-zA-Z0-9_\-\.]+$/), which refuses''. The REST server never ran it: both hops into@objectstack/restare casts and the plugin declares noconfigSchema.RestServer.normalizeConfignow runs the declared contract before the cast, soapi.version: ''and'v1/beta'are refused at construction instead of mounting the whole API at/api//.1. The seam, re-derived at
origin/mainEvery offset in the card and its comments predates PR #11933 (
ce744bcdf), which landed in this exact file. Re-derived atorigin/main@7899f5745, which contains #11933:Measured vs. the dispatch order — the mechanism holds, three of the four offsets moved:
7899f5745rest-server.ts:2926—version: api.version ?? 'v1':2932rest-server.ts:3018—api.apiPath ?? …:3024rest-server.ts:3667:3673rest-server.zod.ts:48—RestApiConfigSchema:48✔ (regex on:52)rest-api-plugin.ts:68/:77—api?: RestServerConfig:104(interface opens:95)rest-api-plugin.ts:388—new RestServer(…, config.api as any, …):415premise_still_valid: true. The card's current framing is correct in every load-bearing part, and the two withdrawn PM comments were right to be withdrawn.2. The two cast hops, and what each casts from
RestServerConfig | undefined→any. It erases no type — the parameter it feeds is alreadyRestServerConfig— so what it actually buys is silence: excess-property and shape checking at the one call site where a deployment's config enters the package. It is also how{ api: { requireAuth: false } }keeps compiling against a keyz.inputtypes asnever.RestApiConfig | undefined→Partial<RestApiConfig>, whereRestApiConfig = z.input<typeof RestApiConfigSchema>.z.inputalready makes every.default()ed key optional, so the cast adds almost nothing type-wise. What it does is assert "this value satisfies the schema" without asking the schema — the entire defect in one expression.Between them there was no parse, and
??substitutesnull/undefinedonly, so''walked pastversion: api.version ?? 'v1'.3. Why the kernel validator does not see it — answered with code, not preference
Three independent reasons, each measured:
createRestApiPlugindeclares noconfigSchema.git grep -c configSchema origin/main -- packages/rest/src→ 1 hit, and it is the comment inrest-server.tssaying there is none. (Reverse-checked withoptionalDependencies, 1 hit inrest-api-plugin.ts, a term independently known present and not a substring of the term under test.)PluginLoader.loadPlugin(packages/core/src/plugin-loader.ts:157) callsthis.validatePluginConfig(metadata)with no config argument, and:406returns early whenconfig === undefined, logging "config validation postponed". There is no other caller.kernel.use(plugin)handsloadPluginthePluginobject;createRestApiPlugin(config)captures its config in a closure, andPluginMetadatahas no field for it.⇒ "Declare
configSchemaon the plugin" was structurally unavailable, not merely a second-best option: it would have validated nothing. That is what settled the shape in favour of the seam. Filed as #11982 (the mechanism has zero live consumers repo-wide — no kernelPlugindeclaresconfigSchema).4. In-repo config census — what would stop booting
Triage asked for this explicitly, and it changed the design twice.
packages/rest/src; fivepackages/clie2e boots went red becauseos serveships aprojectResolutionvalue the declared enum does not contain. The radius that matters is every package that constructs a REST server, not the package the change lives in. Redone mechanically and repo-wide:Both refusals are the deliberate
''cases in this change's own pin file. Three gaps closed by hand, since a literal census cannot see them:documentation/responseFormatliteralspackages/spec/src/api/rest-server.test.ts— the schema's own tests, which never construct a serveros serve(serve.ts:3139), traced to@objectstack/runtime'sStandaloneStackResult.api, a declared literal type{ enableProjectScoping: false; projectResolution: 'none' }api.requireAuthfixturesOne in-repo pin had to be retired:
rest-openapi-route.test.ts's falsy-version case, which carried its own written hand-off — "this pin describes the empty-version mount — if normalization starts rejecting it, retire the pin". Replaced here with a pin on the refusal.What the census forced
RestApiConfigSchema.parse({ version: 'v1', enableSearch: false })returns an object with noenableSearchproperty. The schema is not.strict(), and a non-strictz.object()strips what it does not declare, so consuming the parsed value would silently turn search back on for a deployment that turned it off — the ADR-0104 silent-strip class. Normalization keeps reading the raw input. Filed asapi.enableSearchandmetadata.maskObjectFieldsare live REST config keys that no schema inpackages/specdeclares — read throughas any, and stripped by any parse of their own config object #11983.requireAuthtombstone is.omit()ed, not enforced.把 public 从"全局开关的副产品"升级为声明式能力,然后删掉 api.requireAuth 开关 #3963 retired that key with a deliberate warn-and-ignore posture (rest-api-plugin.ts: "is IGNORED"), chosen when nothing parsed this config; converting it into a boot failure is that decision's to make, and 96 fixtures still pass the key..omit()is typed against the shape, sotscfails here the day the tombstone ages out — the drift cannot go silent.projectResolutionis.omit()ed too — this is the CI fix. The declared enum isz.enum(['required', 'optional', 'auto']); the value the platform ships is'none', declared as a literal type by@objectstack/runtimeand forwarded byos serveinto both the REST and Dispatcher plugins (serve.ts:3139and:3160;?? 'auto'never fires because'none'is not nullish). Three packages disagree about this key's vocabulary and have done so silently for exactly as long as nothing ran the schema. Parsing it here does not settle that — it just turns everyos serveboot into a crash. Filed asprojectResolution: 'none'is shipped by@objectstack/runtimeand forwarded byos serve, butRestApiConfigSchemadeclares onlyrequired|optional|auto— accepted only because the schema was never executed #11999; closing that is what lets the omit come out.crud/metadata/batch/routescarry unenforced constraints of their own (batch.maxBatchSize: z.number().int().min(1).max(1000), three enums). Same defect class, filed asRestServer.normalizeConfigstill castscrud/metadata/batch/routesinstead of parsing them —batch.maxBatchSizebounds and three declared enums never execute #11984 — one narrowing in front of contract review, not five.5. Clause ② — yes, and it is a NARROWING, against the shape actually built
Enforcing the declared regex means configs that boot today start being refused loudly.
Newly refused, at
new RestServer(...)andcreateRestApiPlugin().start():api.version: ''— the reported case.api.versioncarrying any character outside[a-zA-Z0-9_-.]—'v1/beta','v1 beta','v1%2F'.api.enableCrud: 'yes',api.basePath: 42, a malformeddocumentation/responseFormat).Deliberately not refused:
api.projectResolution(#11999 — the platform ships'none');api.requireAuth(tombstone omitted, above); undeclared keys such asapi.enableSearch(validation-only, nothing stripped);api.basePath: ''(a barez.string()— the narrowing follows the contract, it does not extend it); the four sibling sub-objects.The changeset names every one of these.
needs:contract-reviewis hung on this PR and is not cleared here — the review chain records its verdict on the card.6. Anti-vacuity: predicted vs. measured ablation
Method: fix committed first (
0b44a2093), thengit checkout 0b44a2093~1 -- packages/rest/src/rest-server.tswith atrap … EXIT INT TERMrestore, pins untouched. Mutation confirmed on disk in both directions before the run — an editing tool's exit code is not evidence:No rebuild leg is needed for the
packages/restpins and none is claimed: they import the mutated file relatively (import { RestServer } from './rest-server.js'— resolved tosrc/by vitest, never throughdist/), and@objectstack/spec(which is resolved through itsexports/dist) is untouched by the mutation.packages/clie2e tests spawn a child process that resolves@objectstack/restthroughdist/index.cjs, so mutatingsrc/alone left them green — a false green that would have certified the fix as unverified. Redone with a rebuild on both legs andscripts/ablation-dist-preflight.mjsproving the marker reached the artifact:'','v1/beta', whitespace, wrong type, message content + scoping, no doubled slash (7)createRestApiPlugin({api:{api:{version:''}}}).start()rejects (1)/api/v1, allowed spellings,basePath: '',apiPath,enableSearchkept,requireAuthkept (6)projectResolutionablated,@objectstack/restREBUILT, the 5 CI-failingpackages/clie2e bootsapi.projectResolution: Invalid option+rollback complete✔rest-openapi-route.test.ts— retired pin's replacement (1)rest-openapi-route.test.ts— remaining 16createRestApiPlugin.start()returns quietly whenhttp.serverorprotocolis missing, so without it a rejection could be attributed to a thin ctx rather than to the config.⛔ No pin asserts "the schema refuses
''" — that is green before the change. Every §A/§B assertion drives real construction.7. Checks — each gate's own printed verdict line
Gate family re-derived from the actual change set (
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, 4 paths, 19 path-matched + 6 convention-triggered families), and re-derived again after the change set moved. Union re-run at final commit567e20a4e:One gate went red and was fixed, not ratcheted around:
check:type-check-debt --re-measurereported "@objectstack/rest: TEST_DEBT records 155 raw tsc error(s),tsc --noEmitnow reports 157 (+2)". Both wereTS2835on the new pin file's extensionless./rest-server/./rest-api-pluginimports. Remedy taken was the author's one — explicit.jsextensions — never raising the entry. Back to 155.Docs-drift class
This is a behaviour change, so a page describing what the server accepts for
api.versioncould be genuinely falsified. Checked that class specifically: no hand-written page makes an acceptance claim.content/docs/references/api/rest-server.mdxis AUTO-GENERATED fromRestApiConfigSchema, which this PR does not touch, so it is unchanged and still accurate;content/docs/api/index.mdx:143andcontent/docs/protocol/kernel/http-protocol.mdx:85-88describe what discovery'sversionfield means (the serving artifact's version since #11292), not what config is accepted; and nocreateRestApiPluginexample incontent/docs/**passes anapi.versionat all.docs-drift-check.yml's own two gates are green above.content/docs/releases/**untouched.8. Scope
packages/specis untouched — read-only on this card. Where the measurement said the real repair belongs there, it was filed rather than edited: #11982, #11983, #11984, #11999. In particular ⛔'none'was not added to the enum and the CLI was not changed to stop using it — the first isdomain:spec's call, the second is project-scoping semantics. Declared surface only:packages/rest/src/rest-server.ts+ its pins + one changeset.⛔ Draft, not armed: not marked ready, no auto-merge.
needs:contract-reviewhangs on the PR and is not cleared here.Generated by Claude Code
Generated by Claude Code