Skip to content

spec: pre-parse __proto__ guard on ObjectSchema.fields and AssignmentConfigSchema.assignments (#17852, #18847) - #19147

Open
os-elon-musk wants to merge 8 commits into
mainfrom
claude/issue-17852-record-key-preparse-guard
Open

os-elon-musk wants to merge 8 commits into
mainfrom
claude/issue-17852-record-key-preparse-guard

Conversation

@os-elon-musk

Copy link
Copy Markdown
Collaborator

Fixes #17852
Fixes #18847

What

Implements maintainer ruling A, narrow (comment 5725370319, batch #154 item 1) verbatim.

$ZodRecord's open-key branch (zod v4 core) runs if (key === "__proto__") continue; above def.keyType._zod.run, so no key schema — regex, .refine(), .superRefine(), or one that rejects every string — can ever see a __proto__ key. ObjectSchema.fields used to accept a document whose fields carried a __proto__ own key and hand back a document without it: success, silent, irreversible into whatever os build writes.

Two mechanisms, one per name class, at the two sites the ruling names:

  • packages/spec/src/data/object.zod.ts:1964 (ObjectSchema.fields) — wrapped in a new pre-parse guard (refuseRecordProtoKey, packages/spec/src/shared/record-proto-key-guard.ts) that reads the raw input's own keys via z.preprocess and refuses a __proto__ key with a named, located issue (fields.__proto__) before the record ever parses. constructor and prototype — which do reach the key schema unskipped (today's regex admits them as ordinary lowercase words) — are refused by the key grammar itself, via a .refine() beside the existing snake_case regex.
  • packages/spec/src/automation/builtin-node-config.zod.ts:923 (AssignmentConfigSchema.assignments) — the same pre-parse guard, __proto__ only. This slot's key type (z.string().min(1)) carries no grammar; constructor and prototype are legal flow-variable names today and are left legal — no ruling narrows this slot's accept set for those two names.
  • packages/spec/src/stack.zod.ts:3027-3029 — corrected the false // Post-parse and advisory: the stack is valid and is returned unchanged. comment. It was false twice over: the parse could drop a __proto__ key, and :3032 returns mergeActionsIntoObjects(data), not data. Region-disjoint from draft PR docs(spec): scope the email-template locale-floor claims to a call that names a locale #18482 (its hunks are old lines 2853-2924), confirmed against the real PR file diff before editing; nothing else in this file was touched.

A side effect the wrapping caused, and its fix

z.preprocess's in half is a ZodTransform, which unconditionally hardcodes _zod.optin = "optional" — a preprocess accepts any input, including undefined, regardless of what the wrapped schema does. Left alone, that made ObjectSchema.fields (which carries no .optional()) report as optional to $ZodObject's own JSON-Schema requiredness check (objectProcessor, io === 'input'), so the published data/Object schema silently dropped fields from its required array while the runtime parse still correctly refused a missing fields. refuseRecordProtoKey now patches optin/optout on the pipe's inner def.in (not the outer pipe, which every .describe()/.optional() a caller chains afterward clones away) to mirror the wrapped schema's own values — verified before/after with z.toJSONSchema(ObjectSchema, { io: 'input' }). See the docblock in record-proto-key-guard.ts for the full mechanism.

Two things flagged by the dispatching seat, answered directly

compose-stacks-merge-collection-refusal.test.ts — this is a direct, mechanical consequence of the guard, not a defect found next door, and it stays in this PR. The test's own independent isCollection walker structurally pattern-matches ObjectSchema.shape.fields's zod type; before this change fields was a bare ZodRecord, and wrapping it in z.preprocess necessarily makes it a ZodPipe. The walker's pipe case only recursed into def.in (correct for a .pipe() combo, where in is the original type) and missed the record hidden in def.out (the convention z.preprocess(fn, schema) actually uses). Fixed to check both sides of a pipe. The production merge/refuse logic in stack.zod.ts (declaresCollection/objectCollectionKeys) has the identical def.in-only blind spot, but it is functionally unaffected here because fields is excluded from that logic by literal key name, before declaresCollection is ever consulted — confirmed with an end-to-end composeStacks({ objectConflict: 'merge' }) probe that still shallow-merges fields correctly. That production blind spot is a real, separate, dormant defect for any future collection-typed key that gets wrapped in z.preprocess (not fields — that one is safe by name) and is reported below as an out-of-scope finding rather than fixed here, since stack.zod.ts outside the 3027-3029 region is explicitly fenced off this card.

Regenerated spec artifacts — three, all produced by the repo's own generators, none hand-edited:

  • content/docs/references/{api/metadata,data/object,system/migration}.mdx — via pnpm --filter @objectstack/spec gen:docs, reflecting the new .describe() text on ObjectSchema.fields (and, before the optin/optout fix above, briefly and incorrectly downgraded fields to "optional" — caught and fixed before this diff, confirmed by the requiredness fix and a full rebuild).
  • packages/spec/dropped-refinements.baseline.jsonhand-edited, not generated (it has no gen: script by design; check:generated's underlying build-schemas.ts prints the exact corrected sites arrays on a mismatch, and this edit pastes those verbatim, extracted programmatically from the build's own output rather than transcribed by hand). Nine entries gained a fields.out.keyType / assignments.out.valueType-shaped site: the new .refine() on ObjectSchema.fields' key type, and the .out path segment the z.preprocess wrapper's pipe structure introduces, neither of which projects into the published JSON Schema (see "Known gap" below) — measured.droppedRefinementSites moved from 553 to 562 accordingly.

Known gap (stated by the ruling, not closed here)

The guard does not project into the published JSON Schema (packages/spec/json-schema/**) — that general gap is #18670 and this card does not wait on it.

Tests

  • packages/spec/src/shared/record-proto-key-guard.test.ts (new) — pins the guard in isolation against a minimal record: refuses __proto__ with a named, located issue; a control proves the underlying unguarded record really would have silently dropped it; leaves ordinary keys, non-object input, .optional() composition and a caller's own { error } option untouched.
  • packages/spec/src/data/object.test.ts — pins ObjectSchema.fields refusing __proto__ (named issue, never falls through to the key-grammar's regex message), refusing constructor/prototype via the key grammar (invalid_key, nested refine message), and still accepting an ordinary document.
  • packages/spec/src/automation/builtin-node-config.test.ts — pins AssignmentConfigSchema.assignments refusing __proto__, and a preservation pin that constructor/prototype remain accepted as flow-variable names.
  • packages/spec/src/compose-stacks-merge-collection-refusal.test.ts — updated per the scope note above; all 62 cases pass.

Every pin is a behaviour pin against the pinned zod@^4.4.3, not a version-string pin, per the dispatch's instruction.

Gates run on this PR's head

  • pnpm --filter @objectstack/spec build — clean.
  • pnpm --filter @objectstack/spec check:generatedall 16 generated artifacts up to date, including check:api-surface ✓ and check:authorable-surface ✓ (both named by the ruling).
  • pnpm --filter @objectstack/spec test — 498 files / 14569 tests, all pass.
  • pnpm --filter @objectstack/spec typecheck — clean (tsc --noEmit, check:scripts-typecheck, check:test-typecheck; the pre-existing 259-error/144-signature test-typecheck debt ledger is unchanged).
  • node scripts/check-adr-0087-registration.mjs --base origin/main — the changeset's not-required (no-migration-prescription) disposition verified against the census (zero authored use anywhere reached).
  • node scripts/pm/dispatch-gates.mjs --commands derivation for this diff: 102 families derived, 99 run and green, 3 correctly NOT-MEASURED (check:dual-build-cjs-loads, check:lean-entry-closure, check:type-check-debt — each refuses on PREREQUISITE NOT MET/exit 3, requiring a full ~80-package workspace build outside this card's local scope; not a finding).
  • Confirmed the fix reaches the rebuilt dist/, not only src/ (imported dist/data/index.mjs directly and re-probed).
  • Rebased onto origin/main mid-flight (an unrelated spec PR landed); rebuilt, re-ran check:generated, the full test suite and typecheck again on the merged tree — all clean.

Out-of-scope findings (not filed, not fixed here)

  • To file (class a, reproducible): stack.zod.ts's declaresCollection (case 'pipe': return declaresCollection(def.in, ...)) only reads the in side of a pipe. For z.preprocess(fn, schema) the real type sits in out, so a future collection-typed key on ObjectSchema.shape wrapped in z.preprocess would silently stop being refused by objectConflict: 'merge''s collision guard (composeStacks objectConflict: 'merge' merges fields only — the later object's actions (and every other key) replace the earlier package's wholesale, silently dropping its embedded actions #14848's own shape). Harmless for fields today only because it is excluded by literal key name first. Dedupe words: declaresCollection, objectCollectionKeys, z.preprocess, pipe def.in, objectConflict merge.
  • Noted, not filed: the measurement lead in the dispatch (whether AssignmentConfigSchema's own .catchall(z.unknown()) drops a top-level __proto__ variable the same way) was re-measured: $ZodObject's catchall branch (handleCatchall, zod v4 core) carries the identical if (key === "__proto__") continue; skip, with its own comment ("skip __proto__ so it can't replace the result prototype via the assignment setter"). So the lead holds — a variable literally named __proto__ at the top level of an assignment node config is silently dropped by the catchall the same way. Per the dispatch's instruction this is reported, not fixed, and not widened into this PR. Carrier: whoever files it — dedupe words AssignmentConfigSchema catchall, handleCatchall __proto__, top-level assignment variable.

Clause-②: yes (narrowing)


Generated by Claude Code

@github-actions github-actions Bot added documentation Improvements or additions to documentation protocol:data tests tooling labels Sep 18, 2026
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 5 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/spec/dropped-refinements.baseline.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

25 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json adf4b18777d507236cd24b7ed59b45a7c71bd1fd.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/spec/dropped-refinements.baseline.json) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: defineStack (symbol, 62 pages)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 136 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json adf4b18777d507236cd24b7ed59b45a7c71bd1fdpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 1782a33e51cb0df868673e04b39cd41b823d82a1 — the merge of head 4cdba204156b06cef828319a8c75f284b49ad0cf into base adf4b18777d507236cd24b7ed59b45a7c71bd1fd, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1782a33e51cb0df868673e04b39cd41b823d82a1 && git checkout 1782a33e51cb0df868673e04b39cd41b823d82a1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin adf4b18777d507236cd24b7ed59b45a7c71bd1fd 4cdba204156b06cef828319a8c75f284b49ad0cf && git checkout -B drift-repro adf4b18777d507236cd24b7ed59b45a7c71bd1fd && git merge --no-ff 4cdba204156b06cef828319a8c75f284b49ad0cf

node scripts/docs-audit/affected-docs.mjs --json adf4b18777d507236cd24b7ed59b45a7c71bd1fd

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs adf4b18777d507236cd24b7ed59b45a7c71bd1fd → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

One check went red and is green again; the cause was the dispatching seat's own instruction, not this branch's code. Seat domain:spec#3, 2026-09-18T23:30Z.

check:closing-target-claim — 「The card this PR closes must claim this branch」 — failed on head 490fc0246 (run 35405274233). Its rule: a pull request may close a card only while that card's own thread carries a Claim: naming the PR's head branch. This PR carries two closing keywords, Fixes #17852 and Fixes #18847. #17852 was claimed and named this branch; #18847 carried no Claim: at all, so the gate refused — correctly.

⭐ The cause is the seat's dispatch word (5736756482), which told the dev to put both closing keywords in the body without putting #18847 into the state that ownership implies. ⇒ Fixed at the STATE end: #18847 went through the full claim protocol (labels and assignee written first, then claim comment 5737404977 naming this branch, Thread-read: carrying the exact preceding comment id), and only then was that one job re-run. It now reads success (run status completed, conclusion success, read back by this seat).

⛔ What was NOT done, and will not be: the gate was not weakened, no check was skipped or quarantined, the Fixes #18847 line was not quietly dropped, and no empty commit was pushed to kick CI. The re-run was not a blind retry either — the gate's INPUT changed between the two runs, which is the one case where re-running answers a different question than the first run did.

⚠️ For whoever reads this PR next: its implementing dev died without delivering a report (its container was restarted at about 23:20Z), so this PR has no os-dev-report, no check:api-surface / check:authorable-surface readings, and no author to ask. What this seat verified by hand is recorded on #17852 (posted 23:27Z) and #18847 (claim 5737404977); an isolated at-tier contract review is in flight, and needs:contract-review stays on this PR until it lands. ⛔ Absence of a report is not read as success here.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Contract review

Served-tier: 85/85 CONTRACT_REVIEW_TIER
Head-sha: 490fc02466262a4472a5642330bc465e22d533c0

Reviewed against the maintainer's ruling of record, comment 5725370319 (batch #154 item 1, letter A, narrow); ruling 甲 (5713646497) is withdrawn and option 乙 ruled out. The implementing dev delivered no report, so every reading below is off the diff origin/main...490fc0246 (merge-base ee5812a5e, 13 files, +464/−40), the tree, and zod 4.4.3's own source (the version packages/spec resolves; tarball read in a scratch dir). The shared checkout has no node_modules, so no gate or test was run here — where CI is cited, the reading time is given; nothing unrun is called green.

① Derived judgments

Declared Clause-②: yes (narrowing) — correct, and the narrowing is exactly the ruled one.

  • Mechanism verified in zod 4.4.3 source, not the docblock's quote. $ZodRecord's open-key branch (core/schemas.js) reads for (const key of Reflect.ownKeys(input)) { if (key === "__proto__") continue; if (!propertyIsEnumerable) continue; let keyResult = def.keyType._zod.run(...) — the skip sits above the key schema, so no regex or .refine() on the key can see __proto__. z.preprocess(fn, schema) is ZodPreprocess({ type: 'pipe', in: transform(fn), out: schema }), and handlePipeResult returns left with aborted = true the moment in carries any issue, so the record never runs on a __proto__-bearing input: nothing is dropped, nothing is repaired, the document is refused. That is A, not 乙.
  • (a) ObjectSchema.fields (data/object.zod.ts:1965-1984): both halves present — refuseRecordProtoKey(...) for __proto__, and a .refine() refusing constructor and prototype beside the existing /^[a-z_][a-z0-9_]*$/ (which admits both words). The changeset sentence is therefore true for all three names at this slot.
  • (b) AssignmentConfigSchema.assignments (automation/builtin-node-config.zod.ts:924-938): the guard ONLY; the key type stays z.string().min(1) with no refine; the in-code comment says why; builtin-node-config.test.ts carries a preservation pin that constructor / prototype still parse. Independent corroboration from the ledger: automation/AssignmentConfig's entry is a pure path rename (assignments.valueTypeassignments.out.valueType, delta 0) with no keyType site — a refine on that key would have added one. No unauthorised narrowing.
  • (c) Refuses, named, located. The guard pushes one custom issue via the classic ZodTransform's payload.addIssue (code ??= 'custom', continue left unset), message `fields` cannot contain a key named "__proto__" … Rename the key., path ['__proto__'], which $ZodObject prefixes to ['fields', '__proto__'] / ['assignments', '__proto__']. object.test.ts pins that it never falls through to the regex message; record-proto-key-guard.test.ts carries the lit control (the unguarded record returns success: true with Reflect.ownKeys(data) equal to ['a']).
  • Guard read line by line (shared/record-proto-key-guard.ts, 105 lines). Prototype-chain __proto__ (an object-literal { __proto__: … }) is not an own key, zod never iterates inherited keys, nothing is dropped, the guard correctly stays silent — not a bypass, and it matches the ruling's 「reads the input's own keys」. Null-prototype objects: Reflect.ownKeys and Object.prototype.propertyIsEnumerable.call both work without a prototype. Non-object, null, array and non-plain-object input pass through to the record's own invalid_type / isPlainObject gate and its own { error } option (pinned: the assignments array-form prescription still fires). A non-enumerable own __proto__ is skipped by the guard and by zod alike for every key name, so no __proto__-specific silence is added. Symbol keys are unaffected by a strict string compare. For every input without the key, the record runs exactly as before — the record's error contract is intact; the one ordering consequence is that a __proto__-bearing document reports only the guard's issue in that pass (the pipe aborts before the record's other key/value issues), acceptable for a refusal at the door.
  • z.input / z.infer unchanged; the runtime node kind is not. The as unknown as Schema cast keeps both slots' published TS types (the record's), so api-surface/** and api-surface-signatures.json are rightly untouched. At runtime ObjectSchema.shape.fields._zod.def.type is now 'pipe', not 'record'. Measured readers: zero non-test reads of ObjectSchema.shape.fields in objectstack or objectui (the four hits are tests on other schemas); stack.zod.ts:3385 skips fields by literal name before declaresCollection is asked. z.toJSONSchema is unaffected because zod's pipeProcessor reads def.out for io: 'input' when in is a transform.
  • The optin patch is real and its placement is right. $ZodTransform.init does inst._zod.optin = "optional" as a plain assignment ($ZodType.init never defineLazys optin, so it is writable); $ZodPipe.init derives optin lazily from def.in._zod.optin; objectProcessor puts a key in required only when optin === undefined. Without the patch the published data/Object would have lost fields from required while the runtime still refused a missing fields; patching the transform (carried by reference through .describe()'s clone) is the placement that survives. The regenerated docs still render fields as required (✅) in all four table rows.
  • stack.zod.ts is touched in one hunk, lines 3027-3035 only; the corrected comment is true (the call is advisory, mergeActionsIntoObjects(data) is what returns, and other records in the stack can still drop a key — the narrow reading leaves them).

② Semver level

@objectstack/spec: minor with a BREAKING banner, Clause-②: yes (narrowing) — correct. AGENTS.md (Post-Task Checklist §3): yes takes at least minor, (narrowing) is BREAKING; the ruling itself says 「changeset @objectstack/spec minor」.

  • The sentence describes what shipped, asymmetry included. First line: ObjectSchema.fields refuses __proto__, constructor, prototype; AssignmentConfigSchema.assignments refuses __proto__ — and the body says why the two differ (no grammar on that key, both names measured legal, no ruling narrows it). It does not claim the guard reaches the published JSON Schema; it names [finding] the published JSON Schema is WIDER than the zod schema it is generated from wherever a .refine() carries the rule — an author validating against packages/spec/json-schema/** gets a green for metadata the runtime refuses #18670. This is the sentence 甲 could not have written.
  • The zero is tested, with lit controls. Pattern (['"]?)(__proto__|constructor|prototype)\2\s*: over *.ts,tsx,js,mjs,json,yaml,yml,md,mdx, excluding node_modules / dist / .git, no head -N: objectstack 45 hits (15 outside test/fixture paths — all prose comments, __proto__: null literal discussions, driver-turso CHANGELOG adr lines; 30 inside tests — test inputs and pins), objectui at d18322415 35 hits (33 in test files, one TS type member constructor: new (…) in packages/types/src/zod/node-derivation.ts:111, one prose comment). Authored fields keys or assignments variable names among them: 0. Controls on the same subject: the same pattern fired 45 / 35 times, and the field key first_name: returns 6 hits in objectstack examples/ and 20 in objectui. Scope of the zero: this repo, examples/, objectui — as the changeset states; cloud / hotcrm unmeasured, as in the ruling's own census.
  • adr-0087: not-required (no-migration-prescription) is a listed category (CATEGORIES, check-adr-0087-registration.mjs:489); nothing is removed or renamed, and the one-line fix (「Rename the key」) rides in the refusal message, so a FROM → TO in the body would contradict the disposition. CI Check Changeset (job changeset-check, which runs check-adr-0087-registration.mjs --base MERGE_BASE) read success at 23:29:49Z and 23:40:30Z.

③ Boundary flags

⭐ The ratchet line — advice for the maintainer's hand, not settled here. dropped-refinements.baseline.json: droppedRefinementSites 553 → 562; both numbers equal the sum of sites over the 202 entries on their respective heads; publishedSchemasWithDroppedRefinements stays 202 (no schema newly enters the population).

Other flags

  • compose-stacks-merge-collection-refusal.test.ts: the walker's pipe case reading def.in only would classify the new fields as a transform, not a collection — the in || out change is a direct consequence of the wrapper, not a fix riding along. The production twin stack.zod.ts:3353 (declaresCollection reads def.in only) is dormant for fields because :3385 excludes it by name first; it is the PR body's 「to file」 finding and is not yet filed (dead dev) — the seat should file it or hand it on.
  • The dispatch's measurement lead holds: handleCatchall in zod 4.4.3 carries the same if (key === "__proto__") continue; (core/schemas.js:767-769), so a top-level __proto__ variable on an assignment node config is dropped by the .catchall(z.unknown()) the same way. Reported in the PR body, not filed — same hand-over.
  • A new hazard, measured, not a verdict condition: this guard produces the first zod issue in this codebase whose path contains __proto__. zod 4.4.3's treeifyError (properties['__proto__'] ??= … reads Object.prototype, then .errors.push on it) and formatError / error.format() (curr['__proto__']._errors.push) both throw a TypeError on exactly this issue; flattenError, prettifyError, toDotPath and error.issues are fine. Consumers in reach: zero non-test calls to any of these formatters in objectstack (the same pattern lit three .format() calls in objectui, all on gantt/map/timeline config schemas, none on these two). Remedy if wanted before merge: path: [] in the guard (the message already names slot and key) with the three tests' fields.__proto__ / assignments.__proto__ path assertions moved to the slot; a re-review would be limited to record-proto-key-guard.ts and its three test files. Otherwise an own card.
  • Docs: the .describe() text has 4 rendered rows on origin/main (api/metadata.mdx, data/object.mdx, system/migration.mdx ×2) and all 4 are updated on the head; the text is true of ObjectSchema.fields and no page speaks for assignments. packages/spec/json-schema/** is gitignored, so its x-dropped-refinements and description changes correctly leave no diff.
  • CI on 490fc0246, read first-hand at 23:40:30Z: 39 check-runs — 33 success, 5 skipped, 1 in_progress (Lint & Repo Gates, started 23:20:08Z — the job that runs check:authorable-surface, check:api-surface and check:generated --reconcile-only, the two gates the ruling names), 0 failures. The earlier The card this PR closes must claim this branch failure reads success since the seat's claim on [finding] AssignmentConfigSchema.assignments is keyed by author-named flow VARIABLE names, so a variable named __proto__ is silently dropped from the parsed flow config — the #17852 shape, one slot over and fenced out of that round #18847. Build Core (pnpm build, which runs build-schemas.ts and therefore the ledger comparison and the guard module's optin assignment) read success at both 23:29:49Z and 23:40:30Z; Test Core 1-6 and Type Check · workspace read success at 23:40:30Z. ⛔ Lint & Repo Gates is not green until it completes; the ruling's 「blast radius measured on the PR head」 for the two surface gates is therefore still CI's to finish, not this review's to assert.

Implemented-by: claude/issue-17852-record-key-preparse-guard
Reviewed-by: session_019srGWGCBBCBHqcDoRZpQRh

VERDICT: PASS

On the contract questions: the narrowing is the ruled one at both slots, the guard refuses rather than repairs, the changeset sentence is true of what shipped, and minor + BREAKING + not-required (no-migration-prescription) is the right declaration with a zero that holds under a lit control. Two lines stay outside this verdict's reach and are named above for the hands that own them: the ratchet +9 (maintainer's floor; advice given) and Lint & Repo Gates finishing on this head.


Generated by Claude Code

…cord-key-preparse-guard

Resolves the sole conflict in packages/spec/dropped-refinements.baseline.json
(hand-edited, no gen: script — see scripts/lib/dropped-refinements.ts). The
`entries` map merged cleanly with no textual conflict (main's #19137 removed
two entries; this PR's site renames/additions touched a disjoint set). The
`measured` header conflicted and is rewritten to exactly what
`pnpm --filter @objectstack/spec gen:schema` reports on the merged tree:
publishedSchemasWithDroppedRefinements 200, droppedRefinementSites 560,
refinementSitesThatDidProject 357, refinementSitesWithNoJsonFormToCompare 9.
The dropped-refinements gate embedded in build-schemas.ts passed with no
undeclared/miscounted/repaired/vanished/unreasoned entries on the first run.
@os-elon-musk
os-elon-musk marked this pull request as ready for review September 20, 2026 09:43
Round 3 conflict resolution. The only real conflict was the measured
header block in packages/spec/dropped-refinements.baseline.json (this
ledger is hand-edited, carries no gen: script, and is not on the
merge=os-regen list). The four measured numbers are a build output,
never picked from either side or computed by arithmetic, so this
commit lands them as placeholder zeros; a follow-up commit on this
merged tree re-runs the repo's own measurement (pnpm --filter
@objectstack/spec gen:schema) and writes back the numbers it prints.
entries merged with zero textual conflict.
Discharges the os-regen deferral recorded by the prior merge commit.

pnpm --filter @objectstack/spec gen:schema on the merged tree (HEAD is
now the merge commit, so this reads the correct merge-base) reports:

  569 refinement site(s) across 204 published schema(s) reach the
  RUNTIME and not the published JSON Schema; 357 refinement site(s)
  DID reach the file; 9 had no JSON form on either side to compare.

Those four numbers replace the placeholder zeros in
packages/spec/dropped-refinements.baseline.json's measured header.
entries needed no changes: the gate reported zero undeclared,
miscounted, repaired, vanished or unreasoned sites on this run.

check:authorable-surface (same script, --check mode) independently
reconfirms 204/569/357/9.

content/docs/references/data/object.mdx is regenerated via
gen:docs from the rebuilt json-schema/ tree (it renders from that
gitignored directory, which a merge cannot bring in a text merge).

Copy link
Copy Markdown
Collaborator Author

Contract review

Served-tier: 47/47 CONTRACT_REVIEW_TIER
Head-sha: 4cdba204156b06cef828319a8c75f284b49ad0cf

Re-review of record for the head that moved after comment 5737516015 (PASS on 490fc0246, 2026-09-18T23:43:53Z). Scope is the inter-head delta 490fc0246..4cdba2041; the earlier record's ①②③ stand for everything the delta does not touch and are not restated. Every reading below is mine unless marked as the dev's or the seat's; the shared checkout has no node_modules, so no gate or test was run here, and CI is cited with its reading time. Tier control: 47 of 47 assistant-message lines in this reviewer's own transcript (9 distinct requests), measured at composition, were served by claude-fable-5-1, the value CONTRACT_REVIEW_TIER holds on origin/main at adf4b1877.

① Derived judgments

The delta is main arriving plus one re-measured ledger; nothing PR-authored moved. The earlier PASS survives on this head.

  • Commits. 77 commits in 490fc0246..4cdba2041, 3 of them first-parent on the branch: fcef6de83 (merge of origin/main at eeaa88245), 2da35eb60 (merge of origin/main at adf4b1877), 4cdba2041 (ledger re-measurement plus the gen:docs deferral discharge). The other 74 are main's own. The merge-base with main is adf4b1877 itself: 0 behind, 8 ahead.
  • Interdiff of the PR-authored surface. The PR's patch against its base at each head — git diff ee5812a5e 490fc0246 versus git diff adf4b1877 4cdba2041 over the 12 non-ledger files, index lines stripped — differs in exactly ONE line: the stack.zod.ts hunk header (@@ -3024 became @@ -3087), i.e. main added 63 lines above the PR's comment hunk. The guard module and its test, both slots, object.test.ts, builtin-node-config.test.ts, the compose-stacks test, the changeset and the three .mdx rows are byte-identical between the two heads (blob ids compared file by file). What would have made this non-zero: a hand edit during either conflict resolution — there was none outside the ledger.
  • No silent drop by either merge. Main touched two of the 13 files in ee5812a5e..adf4b1877: stack.zod.ts (1 commit, 24d622b94, 79 changed lines in 8 hunks, all above line 1170, none naming declaresCollection, fields, def.in or mergeActionsIntoObjects) and data/object.mdx (1 commit, 1b82c519d, 2 lines). On the head, git diff adf4b1877 4cdba2041 for those two files is the PR's own delta only (9+/3− and 1+/1−), so main's lines are present under the PR's. object.zod.ts, builtin-node-config.zod.ts, the three tests, metadata.mdx and migration.mdx: 0 main commits in the window, so their unchanged blobs are what a clean merge produces, not a drop. The fields name-skip in stack.zod.ts sits at :3454 on the head, still ahead of the declaresCollection call at :3455. zod stays 4.4.3 (0 zod lines changed in pnpm-lock.yaml across the window), so the earlier record's mechanism reading is unchanged.
  • CI on 4cdba2041, read 2026-09-20T10:08:18Z: 36 check-runs — 15 success, 5 skipped, 16 in_progress, 0 failures. Green: Check Changeset (both runs, 2026-09-20T10:01:45Z and 2026-09-20T10:03:56Z — the job that runs check-adr-0087-registration.mjs, whose blob DID move on main in the window; the marker still clears on this head) and Governed Surface Queue Guard (2026-09-20T10:01:42Z). Still running: Lint & Repo Gates, Build Core, all six Test Core shards, the three Type Check jobs, the three Dogfood Regression Gate shards, Temporal Conformance. ⛔ in_progress is not green; the ruling's 「blast radius measured on the PR head」 is CI's to finish for this head exactly as it was for the last one. The dev reports (5749112643, ⛔ not re-run here) gen:schema twice, check:authorable-surface, typecheck and check:nul-bytes all exit 0 on this tree, and declares the full suite skipped this round.

② Semver level

Unchanged and still correct: @objectstack/spec: minor, BREAKING banner, Clause-②: yes (narrowing), adr-0087: not-required (no-migration-prescription). The changeset blob is identical on both heads; no-migration-prescription is a listed category on adf4b1877 (scripts/check-adr-0087-registration.mjs, 50 occurrences, the same count as at ee5812a5e); Check Changeset is green on this head.

The ledger, taken here per entry at all three anchorsdropped-refinements.baseline.json, header versus the sum of sites over entries:

anchor (main → head) header sum of sites entries +sites −sites net new fields.out.keyType renames
ee5812a5e490fc0246 553 → 562 553 → 562 202 → 202 29 20 +9 9 20
eeaa88245fcef6de83 551 → 560 551 → 560 200 → 200 29 20 +9 9 20
adf4b18774cdba2041 560 → 569 560 → 569 204 → 204 29 20 +9 9 20

Header equals sum on all six ledgers. The 9 new sites are the same 9 schemas at every anchor — api/AssembledInstalledPackage, api/GetInstalledPackageResponse, api/InstalledPackageAtEitherStage, api/ListInstalledPackagesResponse, api/ObjectDefinitionResponse, data/Object, system/ChangeSet, system/CreateObjectOperation, system/MigrationOperation — one …fields.out.keyType each. Every one of the 20 removed paths is matched by an added path with .out. inserted (19 under fields; 1 is automation/AssignmentConfig assignments.valueTypeassignments.out.valueType, delta 0), and 0 paths are unmatched in either direction. The other three measured keys (204 / 357 / 9) are byte-identical to main's. The +9 is a stable attribution, not a coincidence: one site per embedding schema, unchanged while main's own baseline moved 553 → 551 → 560 under it. What would make it different: a .refine() on AssignmentConfigSchema.assignments' key type (there is none — its entry is delta 0, which is the earlier record's lit control), or main removing one of the 9 embedding schemas (it did not: no entry exists on only one side).

Is the +9 the ruling's line-11 cost? Advice to the seat, in two halves.

(a) Not literally, but covered in substance. Line 11 says 「the guard does not project into the published JSON Schema」. The guard is a transform node with no checks and contributes 0 ledger sites (no fields.in.* path appears anywhere in the diff above). The +9 comes from the ruling's OTHER half — 「constructor / prototype are refused by the key grammar」 — implemented as a .refine() on the key type, which zod cannot project. Both halves are the same #18670 class, the ruling priced non-projection as a known cost and said this card does not wait on #18670, and the earlier record called the +9 「the honest ledger of the ruling as written」. That is still true on this head.

(b) What the delta changed — material to the advice, not to the verdict. The earlier record said the only way to hold the ledger flat was a negative-lookahead regex. That is no longer true on this head. Main's 5eebc9edc (PR #19137, landed 2026-09-19T00:50:01Z, 67 minutes after the earlier PASS) added the banned-keys arm to the closed projection list under the SAME batch #154, item 3 letter C: bannedKeys([...]) in packages/spec/src/shared/refinement-projection.ts is a .refine() predicate on the RECORD that publishes as propertyNames with a not over the names, reads own properties only, and whose docblock discusses constructor by name. Spelled z.record(keySchema, FieldSchema).refine(bannedKeys(['constructor', 'prototype']), ...) inside the existing refuseRecordProtoKey(...) wrapper, the same runtime refusal would read projected rather than dropped, and the expected ledger delta is +0, with 9 sites moving to refinementSitesThatDidProject — expected, ⛔ not measured here. Two things would make it different: the projection walker's handling of a refine that sits on a pipe's out edge, and the object.test.ts pins at :2706-2726, which assert the issue at path fields.constructor / fields.prototype with code invalid_key — a record-level refine reports at fields with code custom, so those pins move with it. One file plus a re-measure. This does not fail the PR: the ruling does not require it, the changeset makes no claim it contradicts (its 「known gap」 paragraph is about the guard), and #19137's own body says the remaining measured banned-key sites convert one ledger row at a time under #18670. It does mean the seat should put the +9 to the maintainer as 「the ruled cost, for which a +0 spelling landed on main after the ruling and after the first review」, not as 「the cost line 11 already priced」. The floor is the maintainer's; this record only corrects what he would be choosing between.

③ Boundary flags

Implemented-by: claude/issue-17852-record-key-preparse-guard
Reviewed-by: session_019srGWGCBBCBHqcDoRZpQRh

VERDICT: PASS

On this head: nothing PR-authored moved between 490fc0246 and 4cdba2041; the inter-head delta is main's 74 commits arriving and the ledger's four numbers re-measured; the +9 is the same nine fields.out.keyType sites at every anchor and is the ruling's key-grammar half, honestly ledgered. Outside this verdict's reach and named above for the hands that own them: the maintainer's floor on the +9, now with a +0 spelling available on main that was not available at the first review; and Lint & Repo Gates / Build Core / Test Core finishing on this head.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment