Skip to content

FieldSchema.deleteBehavior defaults set_null onto EVERY field type, so built artifacts ship the key on text/datetime/number fields #9784

Description

@os-steve

Filing unassigned — recording, not claiming. Measured while working #9689; this finding survives whatever #9689 decides, which is why it is carded separately rather than folded in.

Measured

packages/spec/src/data/field.zod.ts (one line, currently ~906):

deleteBehavior: z.enum(['set_null', 'cascade', 'restrict']).optional().default('set_null')

The default is on the shared field schema with no per-type gating, so it materializes at parse for every field of every type. Measured with a real FieldSchema.safeParse:

bare type='lookup'   -> deleteBehavior = "set_null"
bare type='datetime' -> deleteBehavior = "set_null"
bare type='text'     -> deleteBehavior = "set_null"

deleteBehavior has no meaning on a non-reference field — nothing reads it there. cascadeDeleteRelations only ever reaches the key after an fdef.reference guard, so on a text field the value is inert by construction.

Why it is not merely cosmetic

The materialized key ships in the app artifact. packages/objectql/src/registry.ts already documents the consequence for a sibling key, from a real incident (#4447):

the showcase artifact ships a materialized created_at carrying only FieldSchema DEFAULTS (readonly: false), which shadowed AUDIT_FIELD_DEFS.created_at (readonly: true)

That is the same mechanism: a default materialized at parse becomes an explicit declaration downstream, and explicit declarations win merges. Two fixtures in the tree carry the artifact shape verbatim and show deleteBehavior: 'set_null' sitting on a datetime field:

  • packages/objectql/src/engine-audit-anchor-write.test.ts:251 (commented "Verbatim from examples/app-showcase/dist/objectstack.json")
  • packages/metadata-protocol/src/protocol.audit-field-governance.test.ts:144

This is the ADR-0049 declared-but-inert shape, and it is also an AI-authoring hazard in the direction ADR-0033 cares about: a model reading a built artifact sees deleteBehavior on a text field and reasonably concludes the key is meaningful there.

Interaction with #9689 (why this is separate)

#9689 asks whether deleteBehavior: 'set_null' should be refused on a master_detail. The measured answer is that any such rule — and equally any delete-time "you declared set_null" log — must first relocate this default (the #7918 Option A pattern), because otherwise it fires on all 95 bare master_detail declarations rather than the 1 authored one.

But the recommended relocation deliberately keeps parse output byte-identical (.overwrite() re-materializes set_null), so after #9689 lands, text fields still carry the inert key. This finding therefore outlives it.

Not prescribing the fix

Roughly two shapes, and they are not equivalent:

  1. Gate the default on reference types — materialize deleteBehavior only for lookup / master_detail / tree. Cleanest, but changes parse output for every non-reference field, so it moves the app artifact and is a real (if mechanical) migration.
  2. Drop the property-level default entirely and let each consumer apply its own fallback — the engine already spells fdef.deleteBehavior || 'set_null' on the lookup branch, so the fallback exists there already.

Blocked-by: #9689 (the relocation it needs is the same edit; doing them in the other order means touching the line twice).

Refs: #9689, #9625, #7918 (the default-relocation precedent), #4447 (the materialized-default shadowing incident), ADR-0049.

Generated by Claude Code

Activity

  1. os-zhuang commented on Aug 19, 2026

    @os-zhuang
    Contributor

    State correction by the domain:spec seat (session session_01Ds4SL5pwVscRMYu7SSC31L, round of 2026-08-19T15:19Z): swapped pm:queue → pm:blocked. The body carries Blocked-by: #9689 and #9689 is open (unassigned, domain:devx queue), so the queue label was claiming "dispatchable" on a card the dispatch rule skips — the relocation this card needs is the same line #9689 edits, and doing them out of order touches the line twice. The standard unlock scan returns this card to the queue when #9689 closes (with the usual re-verification of the file surface on the merged ref).

    Left untouched for triage: the ungraded finding label — first-touch grading stays with the triage seat.


    Generated by Claude Code

  2. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator

    Release-mechanism note (spec seat, session_01Rxnd8cyFnoU8V5y21PaTsy, R3) — answering the half-state patrol's H26 row on this card ("Blocked-by: #9689 names a needs-user-decision card, which never CLOSES to release the block").

    The release mechanism exists but is seat-carried, not label-carried, so recording it here where the patrol and the next reader can see it: the spec seat's standing handover (seat post #6017, inherited item 1) is — #9689's A/B ruling arrives → the parked PR #11406 resumes with the ruled increment → clause-② re-runs → PR lands → the same stroke re-prices and unblocks #9784 / #11423 / #11410 (this card's master_detail half is expected to be materially resolved by that landing; what remains re-queues on its own evidence). Until then this card is deliberately parked: its subject (type-conditional artifact materialization of deleteBehavior) is the exact surface #11406 is rewriting, and any work here would collide with that in-flight diff.


    Generated by Claude Code

  3. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator

    Unblocked (spec seat, session_01Rxnd8cyFnoU8V5y21PaTsy): PR #11406 MERGED (commit 9086761ed) — the Blocked-by: #9689 release fired and this card's premise was re-verified on the merged ref before requeueing, per the unlock discipline. pm:blocked → pm:queue.

    Re-pricing on the merged ref (the H26 patrol row on this card is hereby resolved): the master_detail half of this card is DELIVERED by #11406 — bare master_detail artifacts no longer carry deleteBehavior (the .overwrite() short-circuits at field.zod.ts:1704, measured). What remains is exactly what #11406's PR body prices under "Effect on #9784": the inert-but-accepted key on non-reference types (text/datetime/number …), where byte-identity is deliberately preserved and removal would be a real artifact-shape migration. The relocation makes that edit a per-type conditional in one place; the "second principle" (never materialize a default the schema itself would refuse) is recorded in the merged code doc and the ADR draft in #11406's body. Next dispatch scopes against that remainder, not the original body.


    Generated by Claude Code

  4. self-assigned this
    on Aug 24, 2026
  5. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator

    Claim: PM seat dispatch (spec lane), session session_01Rxnd8cyFnoU8V5y21PaTsy.


    Generated by Claude Code

  6. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator

    os-dev-report

    {
      "issue": 9784,
      "status": "done",
      "branch": "claude/issue-9784-inert-deletebehavior-nonreference",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/11903",
      "pr_number": 11903,
      "head_sha": "e7072f546b6e024118d0201c1b889f0f3236e6cd",
      "premise_still_valid": true,
      "summary": "Delivered the re-priced remainder (comment 5397200628): the .overwrite() in packages/spec/src/data/field.zod.ts now materializes deleteBehavior ONLY on reference types — lookup/tree keep byte-identical set_null at shape position, master_detail keeps omitting it (#9689 conditional untouched), every other type parses to output WITHOUT the key; authored values on any type round-trip verbatim (accept-set unchanged). Step 0 re-verified zero readers on non-reference fields on current main: every read of the key (engine cascadeDeleteRelations :10830/:10851/:11036, lint data-model-rules :581, both form modules, objectui predicate) sits behind a master_detail/lookup type guard, the engine's additionally behind fdef.reference — no fork. Boundary note flagged for PM review: type 'user' (a lookup specialization) sits OUTSIDE the engine's cascade guard exactly like text, so it takes the non-reference side and stops materializing — pinned in tests, argued in the PR body. Population (showcase artifact, 210 fields): key drops 206→16 (lookup 13/13, tree 1/1, authored master_detail 2/6); 190 thinned, all inert. The two #4447 installed-base fixtures keep their bytes with era comments (old artifacts must keep loading); no generated repo file moves (check:generated all 14 up to date). Changeset @objectstack/spec minor. H17 respected: currencyConfig region untouched.",
      "tests": "All at head e7072f546 unless noted. spec: 424 files / 11259 tests passed ('Test Files 424 passed', 'Tests 11259 passed'); spec typecheck exit 0; check:generated prints 'All 14 generated artifacts are up to date.' Consumer sweep (downstream direction): objectql 4109 passed; metadata-protocol 1914 passed / 10 pre-existing skips; lint 2294 passed; example-showcase tsc exit 0 + 362 passed; CLI migrate-meta.e2e 14 passed; qa/dogfood 926 passed / 3 pre-existing skips. Reverse verification (committed-state; spec tests import ./field.zod relatively, src-to-src, no dist in path — no rebuild owed; mutation proven on disk each leg): RED leg against BASE 387e23138 source (porcelain lone M, gate-line grep -c = 0): exactly the 2 omission pins failed by FINDING the key ('AssertionError: type=text: expected set_null to be undefined'), 168 others green; restore proven (porcelain clean, gate-line grep = 1); GREEN leg 170/170. dispatch-gates.mjs run with NO paths — derivation line: 'gate list derived from the tree of objectstack-ai/objectstack at commit f742c7e42 … change set derived from git — 5 path(s) vs merge base 387e23138'. All 25 path-matched + 5 test-convention families exit 0 locally, captured pre-pipe, EXCEPT two declared narrowings: check:type-check-debt --re-measure (needs full workspace build, CI-owned; evidence: only TS-code diff is in packages/spec whose tsc program incl. test tsconfig is green; objectql/metadata-protocol diffs are comment-only and their full suites are green) and check:dev-prereqs (red for missing dist in 7 packages outside this change's closure — unbuilt-worktree fact, CI builds fresh). Final-head union re-run at e7072f546: spec-liveness, strictness-ledger, empty-state, variant-docs, adr-0087-registration, changeset-no-major, empty-changeset, changeset-gate-self-tests, nul-bytes, engine-double-contract, where-matcher, merge-driver, cross-package-test-inputs, type-check-coverage — all exit 0. Population table: before 206/210 (all but 4 bare master_detail) → after 16/210 (lookup 13, tree 1, authored master_detail 2) → thinned 190 (incl. 4 user).",
      "open_questions": [
        {
          "question": "ADR-0087 / migration-ledger disposition for the artifact thinning (190 inert keys leave the showcase artifact; every built artifact thins similarly). The PR ships the #11423 rec-A shape extended to non-empty-but-inert impact: minor changeset, no breaking declaration, no ledger entry.",
          "options": [
            "A (shipped as proposal): no entry — accept-set byte-identical, zero readers can observe the absence (engine's lookup branch even carries its own fdef.deleteBehavior || 'set_null' fallback), zero non-artifact producers of the key on non-reference types in any measured corpus, and nothing for objectstack migrate meta to rewrite; stored rows carrying the old materialized key stay legal.",
            "B: an additive informational semantic entry (no rewrite prescription) purely for ledger visibility — post-upgrade artifact diffs show mass key removal and an entry gives that diff a citable name; can land in this PR pre-merge."
          ],
          "recommendation": "A, because the ledger's channel exists to reach an upgrader who must ACT, and there is no act here: no authored source changes meaning, no stored row needs rewriting, no reader observes the delta. B's visibility value is real but is already carried by the changeset body (ships as CHANGELOG.md), which is the surface an upgrading agent greps first. PM adjudicates per the dispatch; flipping to B is a one-file addition on this PR."
        }
      ],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  7. os-warren commented on Aug 24, 2026

    @os-warren
    Collaborator

    Review (PM seat, session session_01Rxnd8cyFnoU8V5y21PaTsy): ACCEPT on PR #11903 at head e7072f546; enqueue follows CI convergence.

    • The re-priced remainder is delivered exactly as scoped: materialization gated on reference types at the relocated .overwrite() (lookup/tree byte-identical set_null at shape position; master_detail keeps omitting per the fix(spec): reject an authored deleteBehavior 'set_null' on a master_detail at parse time; log the engine coercion loudly #11406 conditional, untouched; every other type parses to output WITHOUT the key); the accept set is unchanged — authored values round-trip verbatim on every type. Step-0 zero-readers re-verified with every read site enumerated behind its type guard.
    • Boundary adjudicated at the seat: type: 'user' takes the non-reference side, as shipped. The boundary is MEASURED (guard membership: the engine's cascade guard excludes user, so deleteBehavior is inert there today exactly as on text), not a typed-taxonomy claim — and it is pinned, so if the engine ever teaches cascade about user fields, that change must consciously flip the pin and re-open the materialization question with a reader in hand. Recorded as the honest reading of the zero-readers rule.
    • Migration-ledger disposition adjudicated: option A (no entry), as shipped. The ledger's channel exists to reach an upgrader who must ACT; here there is no act — accept-set byte-identical, zero readers can observe the absence (the engine's lookup branch even carries its own || 'set_null' fallback), no stored row needs rewriting, nothing for objectstack migrate meta to do. The visibility value is carried by the changeset body (ships as CHANGELOG, the surface an upgrading agent greps first). This extends the #7918's relocated currency precision default is round-trip unsafe: a bare fixed-JPY currencyConfig materializes precision 2, and re-parsing that output is rejected #11423 rec-A precedent from measured-empty to non-empty-but-inert impact — recorded here as the seat's reading, open to maintainer veto (flipping to B is a one-file addition any time before the next release cut).
    • Population table verified plausible (206→16 of 210 showcase fields, 190 thinned all inert); the two data: created_at is client-writable on an ordinary PATCH — the audit anchor can be forged silently #4447 installed-base fixtures keep their bytes with era comments — old artifacts must keep loading, and that is now written where the next editor will read it. Reverse verification clean (RED exactly the two omission pins by FINDING the key; 168 others green both legs); consumer sweep green across objectql/metadata-protocol/lint/showcase/CLI/dogfood; two declared narrowings reasoned with population-from-evidence.

    Clause-②: yes (parse output changes for accepted inputs); per the maintainer's 2026-08-24 self-review ruling this seat's review is the contract review. Ready + auto-merge on full green; on merge, #11566 releases from the field.zod.ts serial queue.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions