Skip to content

fix(objectql): a cleared optional multi-value field stores null, so a second record cleared on a unique one is no longer refused 409 - #29837

Merged
objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/project-thread-4rru8k
Nov 3, 2026
Merged

objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/project-thread-4rru8k

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Warren · project thread
Fixes #29343

Clause-②: yes (widening)
Metadata-protocol: none

维护者速读

  • 改了什么:可选的多值字段(multiselect、tags、checkboxes,及 multiple: true 的 lookup / user / select / file / image)写入 [] 或 '' 时,写入口统一存成 null;级联 set_null 删光最后一个成员时也读回 null。
  • 为什么改:[] 被存成 JSON 文本 '[]',唯一约束下第二条清空的记录被拒 409;按裁决 A(记录 6591577985)执行,spec 半边已由 docs(spec): a cleared optional multi-value field is stored as null, in FieldSchema's multiple and required docs #29763 落地。
  • 风险与回滚:读形状变化,原来读回 [] 的可选多值字段改读 null;$empty 两者都算空,只按 Array.isArray 判断的读者要处理 null。回滚即 revert 本 PR。
  • 先看:packages/core/src/utils/blank-storage-form.ts(新 reach 'emptyList')、packages/objectql/src/validation/record-validator.ts 的 normalizeBlankTypedRow、packages/objectql/src/engine-cascade-delete-multivalue-probe.test.ts(钉子从 [] 改为 null)。
  • 两分钟验证:pnpm --filter @objectstack/rest exec vitest run src/rest-data-multi-value-blank.test.ts,预期 15/15 通过(main 上 12 条红)。
  • 需要你做的:无,席位落地。

What changes

  • Write door (cherry-picked from the built branch claude/project-thread-tfknlu, eb81c1f6 → ce827aa2, 55aa2656 → f147106e): blankAsNullColumns in @objectstack/core gains the reach 'emptyList' for every optional column on the $empty table's multi_value row, and blankStorageForm stores [], or exactly '', there as null. The objectql door's row walk (normalizeBlankTypedRow) now hands an empty list to the rule. Required columns, whitespace and non-empty lists are untouched; the $empty table is unchanged.
  • Pins moved to null (f0afc329), the cascade set_null repair that empties an optional set now reads back null through the door:
    • packages/objectql/src/engine-cascade-delete-multivalue-probe.test.ts (docblock, the "LAST member" pin, the "every dependent row" pin);
    • packages/objectql/src/engine-cascade-delete-user-field.test.ts (multi-valued set_null);
    • packages/runtime/src/cascade-delete-multivalue-lookup-real-driver.integration.test.ts (real SQLite driver, read back from the database);
    • packages/services/service-automation/src/builtin/delete-record-predicate-walk-batched.integration.test.ts (batched predicate walk).
  • Prose: the "[], never null" sentence is replaced for optional columns in packages/core/src/utils/required-emptiness.ts, the two packages/objectql/src/engine.ts blocks (set_null doc and the member-removal comment) and the record-validator.test.ts docblock. The required half (an empty list fails required) is kept everywhere.
  • Docs: six hand-written pages now say an emptied optional multi-value field is stored as null, that a row stored earlier can still hold [], and that is_empty matches both: content/docs/api/data-api.mdx, content/docs/data-modeling/field-types.mdx, content/docs/data-modeling/fields.mdx, content/docs/data-modeling/validation-rules.mdx, content/docs/deployment/troubleshooting.mdx, content/docs/protocol/objectql/types.mdx. No release notes, no CHANGELOG, no content/docs/references/.
  • Readonly columns (b702c0f1, round-2 ruling A, record 6598733139): the 'emptyList' arm never reaches a readonly: true column. A readonly column is platform-stamped and never cleared by a writer, so sys_activity.changed_fields keeps [] for "stamped, nothing changed" (ADR-0139 E6). BlankAsNullFieldDef gains readonly?: boolean. The 'blank' and 'empty' arms are unchanged. One unit pin covers it, with controls.
  • Changeset .changeset/29343-multi-value-blank-null.md (@objectstack/objectql and @objectstack/core, patch): names ruling A, the Clause-② widening and the read change ([] → null for optional multi-value, including the cascade repair).

Why

Ruling A on this card (record 6591577985): an optional multi-value column written [] or '' stores null, unique-covered or not; required still means a non-empty list; the $empty table is unchanged. The spec half (FieldSchema's multiple and required doc blocks) landed in #29763 for #29652. Stored '[]' / '""' rows are cleaned by #29653, not here.

Tests

All readings below are at head 8e4ba024 (the three commits plus a merge of origin/main b62463f1), except the red-first reading, which is at origin/main 97c2982c. Head 60dc73ff adds one comment-only commit: the blankSingleValueColumns docblock in packages/cli/src/commands/migrate/blank-single-values.ts (the contract review's should-fix, 6597445147). At 60dc73ff, the cli unit test for the command passes 46/46, its integration test passes 6/6, and the cli typecheck exits 0. Head b702c0f1 (after merging origin/main 55dca466 as b784ff6a) adds the readonly carve-out. At b702c0f1 the following pass:

  • cli activity-changed-fields.integration 4/4 and blank-single-values 6/6;
  • plugin-audit activity-changed-fields-backfill.integration 5/5;
  • REST 34/34;
  • core 2937/2937 and its typecheck;
  • objectql 10777/10777;
  • service-automation 4/4;
  • the runtime real-driver suite (7, 1 skipped).

With the carve-out removed, the new pin and exactly the CI reds go red (cli cases 2–4, plugin-audit case 2). They pass again once it is restored.

  • Red first, at origin/main 97c2982c with its own build: rest-data-multi-value-blank.test.ts 12 failed / 3 passed (15). Every per-type unique case (multiselect, multi-value lookup, tags, unique-index member) answered 409 on the second [] or ''.
  • Green after the change: that file plus rest-data-single-value-blank and rest-data-optional-unique-blank 34/34; objectql targeted files 191/191; runtime real-driver file 7 passed, 1 skipped (the Postgres cell, needs OS_TEST_POSTGRES_URL); service-automation batched walk 4/4.
  • Whole packages: @objectstack/objectql test 545 files, 10767/10767 (before and after merging main); @objectstack/core test 132 files, 2936/2936; metadata-protocol seed-loader files 26, 257/257; cli blank-single-values.integration.test.ts 6/6 (integration project). Typecheck exit 0 for core, objectql, runtime, service-automation.
  • Ablation, one-shot: blankStorageForm's 'emptyList' arm replaced to return the value unchanged (scripts/ablation-replace.mjs), core rebuilt, ablation-dist-preflight.mjs exit 0. Result: objectql 5 failed / 38 passed (both probe pins, the user-field pin, two door unit pins); REST 12 failed / 3 passed. Restored: blob equals HEAD, git diff HEAD empty, rebuilt, preflight --absent exit 0, the same files 43/43 and 15/15. A first attempt carried its marker in a comment, which the build strips; its preflight answered exit 1, so that run is void and was repeated with a code marker.

Acceptance notes

  • Read change, stated in the changeset: an optional multi-value field cleared from now on reads null, not [], including the set_null cascade repair that removes the last member. $empty: true matches both. A reader that branches on Array.isArray alone has to read null as empty too.
  • packages/cli/src/commands/migrate/blank-single-values.ts (docblock near line 109) still says the blankAsNullColumns map holds three arms; it now holds four. That file is outside this claim's surface; the behaviour is pinned (blank-single-values.integration.test.ts keeps its column set). Noted for the seat.
  • test:repo (vitest repo project) was not run locally; it belongs to CI (Test Core).

Verification

  • node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands at 8e4ba024: 101 families derived. All 101 run locally; every one exit 0. At 60dc73ff, the derivation is the same 101 families. The 46 families on the changed path re-ran, all exit 0, and --ran reports 101 run with 0 NOT-MEASURED. At b702c0f1: 101 derived, 101 run, all exit 0, and --ran exits 0.
  • Seven first answered exit 3 (PREREQUISITE NOT MET, built output missing: check:skill-examples, check:dts-closure (exit 1, read while a sibling build rewrote driver-sql/dist), check:dual-build-cjs-loads, check:i18n, check:i18n-coverage, check:i18n-walk-parity) or could not reach a pinned fixture commit on the shallow clone (check-plugin-teardown-shape.mjs --self-test). After building the client SDK and the packages closure, and fetching that commit, each re-ran exit 0.
  • dispatch-gates --ran: 101 derived, 101 run, 0 NOT-MEASURED, 0 UNRUN, exit 0.
  • CI runs the full farm (pnpm lint, test:repo, dogfood is not owed: no ! in the title and no (narrowing) arm).

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (No packages/spec change.)
  • 2. Every name I added uses the casing for its kind ('emptyList').
  • 3. No formatter ran; pnpm lint belongs to CI.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. The changeset carries both declaration lines, as does this body.
  • 6. packages/spec not touched.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JQwxbWuRcoCQaZSTv51YHt


Generated by Claude Code

An optional column on the $empty table's multi_value row (multiselect,
checkboxes, tags, or a multi-capable type with multiple: true) written []
or '' now stores null. The core rule gains an 'emptyList' reach in
blankAsNullColumns, and the door's row walk hands an empty list to it.
Required columns, whitespace and non-empty lists are untouched.

Claude-Session: https://claude.ai/code/session_01JQwxbZfihWgHXyWG4Mrd8N
Co-authored-by: Claude <noreply@anthropic.com>
…nal multi-value field stores null

The set_null cascade repair that removes the last member of an optional
multi-value reference now reads back null through the write door, so the
objectql, runtime and service-automation pins move from [] to null, and the
engine, required-emptiness and record-validator prose and six hand-written
docs pages stop saying "[], never null". The required half (an empty list
fails required) is unchanged. The changeset names the ruling, the widening
and the read change.

Claude-Session: https://claude.ai/code/session_01JQwxbWuRcoCQaZSTv51YHt
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/cli, @objectstack/core, @objectstack/objectql, touching 10 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/core/src/utils/required-emptiness.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via os migrate blank-single-values (command, read off packages/cli/src/commands/migrate/blank-single-values.ts))
What this run could not see
  • 1 changed file(s) yielded no anchor (packages/core/src/utils/required-emptiness.ts) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 79 pages)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 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; 103 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 — 70 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 55dca466701606e691c1c96d14ba68dd32b6b06e → packageMentionDocs.

Which tree this was computed on

This run read content/docs from aa46f8ffe1b34c419f525a6d9b1857ea0db4b763 — the merge of head b702c0f1cee285eed74ff9e93e23a53c3babe785 into base 55dca466701606e691c1c96d14ba68dd32b6b06e, 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 aa46f8ffe1b34c419f525a6d9b1857ea0db4b763 && git checkout aa46f8ffe1b34c419f525a6d9b1857ea0db4b763
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 55dca466701606e691c1c96d14ba68dd32b6b06e b702c0f1cee285eed74ff9e93e23a53c3babe785 && git checkout -B drift-repro 55dca466701606e691c1c96d14ba68dd32b6b06e && git merge --no-ff b702c0f1cee285eed74ff9e93e23a53c3babe785

node scripts/docs-audit/affected-docs.mjs --json 55dca466701606e691c1c96d14ba68dd32b6b06e

⚠️ 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 55dca466701606e691c1c96d14ba68dd32b6b06e → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-warren · session_01JQwxbYyp6wSRcywGFK2L4A

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 8e4ba0244a1d4133eba059bf2ace13c8738e2dc8
Local-runs: none
Scope: packages/core/src/utils/blank-storage-form.ts, packages/objectql/src/validation/record-validator.ts, packages/objectql/src/engine.ts, packages/core/src/utils/required-emptiness.ts, .changeset/29343-multi-value-blank-null.md + packages/metadata-protocol/src/seed-loader.ts, packages/cli/src/commands/migrate/blank-single-values.ts

This review is owed for the Clause-②: yes (widening) line alone, under claim amendment 6591192037. It is a declaration face: no packages/spec/** path and no governed path is in the diff. The seat reviewed it at its own tier.

① Derived judgments

  • The rule. blankAsNullColumns sends every optional, non-lifecycle column whose expandEmptyOperator arm is multi_value to the new 'emptyList' reach, unique or not. That is ruling A (6591577985) word for word.
  • The door. normalizeBlankTypedRow now admits an empty array as a candidate blank. It nulls the value only where blankStorageForm says null, so [] on a column outside the reach (a JSON field, a required list) passes through untouched.
  • The seed replay (seed-loader.ts isNoOpReplay) already compares blankStorageForm(value, reach) with no string gate. A seed [] compares as null, which matches the changeset's "rewritten to null once on the next replay".
  • os migrate blank-single-values takes only reach === 'empty', so the new arm does not widen it. The changeset's "does not touch these columns" holds.
  • Pins and prose moved to null, as spec record 6595089655 named them (via 6595097212):
    • the objectql cascade probe, now asserting the key is present and null;
    • the runtime real-driver and service-automation pins;
    • required-emptiness.ts, the two engine.ts docblocks and the six hand-written docs pages.

② Semver level

Clause-②: yes (widening): a second cleared write that used to be refused 409 now lands, and no write that used to land is refused. Metadata-protocol: none: an author can write nothing new. @objectstack/core patch and @objectstack/objectql patch. Under the changeset rule, a non-protocol change ships patch. BlankAsNullReach gains a union member, which the changeset's Export bullet names.

③ Boundary flags

④ Design judgment

Should-fix (prose, outside the claimed surface): the docblock of blankSingleValueColumns in packages/cli/src/commands/migrate/blank-single-values.ts (L106-118) is now wrong twice.

  • It says "That map holds three arms"; it now holds four.
  • It lists "multiple: true columns (the multi_value row)" as out "by the map's own rule". The map now INCLUDES them, as 'emptyList'. They are out because this run takes only reach === 'empty'.

The seat amends the claim to add that docblock, and the dev fixes it in a patch round. There is no must-fix.

Implemented-by: claude/project-thread-4rru8k
Reviewed-by: session_01JQwxbYyp6wSRcywGFK2L4A

VERDICT: PASS

…ulti-value columns are out of its run

Prose only. The blankAsNullColumns map now carries the 'emptyList' arm for
multi_value-row columns; this run leaves them out because it takes only
reach 'empty', not by the map's own rule.

Claude-Session: https://claude.ai/code/session_01JQwxbWuRcoCQaZSTv51YHt
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-warren · session_01JQwxbYyp6wSRcywGFK2L4A

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 60dc73ff71c5f729ca9ced300ae37340249de3a9
Local-runs: none
Scope: packages/cli/src/commands/migrate/blank-single-values.ts (delta 8e4ba024..60dc73ff, +6/−4, fast-forward) + packages/core/src/utils/blank-storage-form.ts

This is a delta review by the same at-tier reviewer as record 6597445147, answering its one should-fix. The moved path is a code file under packages/cli/src, not a prose face, so this hop gets a delta record, not a Regen-provenance: pointer.

① Derived judgments

  • Should-fix (cli docblock): landed correctly. The blankSingleValueColumns docblock now says the map "holds four arms".
  • Its Out paragraph now splits the two reasons correctly:
    • required and lifecycle columns are out by the map's own rule, as in blankAsNullColumns, which continues on both before any arm.
    • multiple: true columns, held on the 'emptyList' arm, and typed columns, on the 'blank' arm, are out because the run takes only reach === 'empty'. That is the guard on the loop line, which this hop leaves unchanged.
  • The delta changes only comment lines in this file.

② Semver level

Unchanged: Clause-②: yes (widening), Metadata-protocol: none, @objectstack/core and @objectstack/objectql patch. The comment-only hop in @objectstack/cli ships nothing new.

③ Boundary flags

None new. The dev reran the cli unit and integration tests for the command (46/46, 6/6) and the cli typecheck (exit 0). The 101 derived gate families are all accounted for at 60dc73ff, with 0 NOT-MEASURED.

④ Design judgment

No change from record 6597445147. There is no must-fix, and the one should-fix landed, so this review stops here.

Implemented-by: claude/project-thread-4rru8k
Reviewed-by: session_01JQwxbYyp6wSRcywGFK2L4A

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-warren · session_01JQwxbYyp6wSRcywGFK2L4A

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 60dc73ff71c5f729ca9ced300ae37340249de3a9
Local-runs: none
Scope: packages/core/src/utils/blank-storage-form.ts, packages/objectql/src/validation/record-validator.ts + packages/plugins/plugin-audit/src/objects/sys-activity.object.ts, packages/plugins/plugin-audit/src/activity-changed-fields.ts, packages/plugins/plugin-audit/src/activity-field-redaction.ts, packages/plugins/plugin-audit/src/audit-writers.ts, packages/cli/src/commands/migrate/activity-changed-fields.integration.test.ts

VERDICT: FAIL

This record supersedes this reviewer's PASS records 6597445147 and 6597824563 on this PR. CI on this head (Test Core (4/8), job 125777038850) shows a consumer that the first review did not read. The PR is draft with no auto-merge armed, so there is nothing to disarm.

① Derived judgments

  • M1 (must-fix): the 'emptyList' arm erases the ADR-0139 E6 "stamped, empty" state of sys_activity.changed_fields.
    • changed_fields is an optional tags column (sys-activity.object.ts, required: false, readonly: true). Its own description says: "empty for every other row … NULL means the row is not stamped yet".
    • The writer stamps [] on every non-update activity: planActivityStamp returns { changed_fields: [], … } (activity-changed-fields.ts), which audit-writers.ts writes on insert.
    • The read rule keeps a row only when changed_fields is not null. An unstamped row "is withheld" from every non-system reader (activity-field-redaction.ts, ADR-0139 D4).
    • With this PR, the door stores that [] as null. Every create, delete and note activity written from now on therefore reads as unstamped, and is withheld from every non-system reader.
    • The backfill os migrate activity-changed-fields reads only NULL rows. It would re-plan those rows forever: CI activity-changed-fields.integration.test.ts cases 2–4 expect changed_fields: [] and receive null.
  • Ruling A (6591577985) reads "a cleared optional multi-value field … is stored as null". For changed_fields, [] is not a clear: it is a platform-written value that means "judged, nothing changed". So the ruling's reach as built collides with a second, already-ruled meaning of NULL.

② Semver level

Not reached: the head fails.

③ Boundary flags

  • This is an access-visibility regression: activity rows that non-system readers see today would be withheld. It is not a security leak; it fails closed.
  • Other optional multi-value columns where [] and NULL mean different things have not been censused. The decision analysis should count them.

④ Design judgment

This needs a ruling, not a dev fix, because each route changes either ruling A's reach or ADR-0139 E6. The routes the seat sees:

  • (a) the 'emptyList' arm skips readonly: true columns: platform-stamped, never cleared by a writer;
  • (b) sys_activity moves its "unstamped" marker off NULL (an ADR-0139 amendment, a migration);
  • (c) narrow ruling A back to unique-covered columns.

The seat puts this to the maintainer through a decision thread and holds the PR at this head. ⛔ No push until the ruling is recorded on #29343.

Implemented-by: claude/project-thread-4rru8k
Reviewed-by: session_01JQwxbYyp6wSRcywGFK2L4A

VERDICT: FAIL

…amped [] is stored as written

Ruling record 6598733139: a readonly multi-value column is platform-stamped
and never cleared by a writer, so its [] is a stamped value, not a cleared
field. sys_activity.changed_fields keeps [] (stamped, nothing changed) apart
from null (unstamped, ADR-0139 E6). The 'blank' and 'empty' arms are
unchanged. One unit pin and one changeset sentence.

Claude-Session: https://claude.ai/code/session_01JQwxbWuRcoCQaZSTv51YHt
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-warren · session_01JQwxbYyp6wSRcywGFK2L4A

Contract review

Served-tier: TIER_DEFAULT
Head-sha: b702c0f1cee285eed74ff9e93e23a53c3babe785
Local-runs: none
Scope: packages/core/src/utils/blank-storage-form.ts, packages/core/src/utils/blank-storage-form.test.ts, .changeset/29343-multi-value-blank-null.md (delta 60dc73ff..b702c0f1: merge of origin/main 55dca466 as b784ff6a, then b702c0f1, +28/−1) + packages/plugins/plugin-audit/src/activity-field-redaction.ts, packages/plugins/plugin-audit/src/activity-changed-fields.ts

VERDICT: PASS

This is a delta review by the same at-tier reviewer as FAIL record 6597870397, answering its M1 under ruling record 6598733139 (round 2, letter A, maintainer 08:17Z). It supersedes that FAIL for this PR.

① Derived judgments

  • M1 resolved, as ruled. BlankAsNullFieldDef gains readonly?: boolean. In blankAsNullColumns, the multi_value branch now sets 'emptyList' only when def.readonly !== true, and continues either way. A readonly multi-value column therefore gets no reach. blankStorageForm(v, undefined) returns the value as written, so sys_activity.changed_fields keeps the [] its writer stamps.
  • The other arms are unchanged. The 'blank' and 'empty' arms are untouched, as the ruling bounds them. The unit pin's control stage_ro (a readonly select) still maps to 'empty', and the plain tags control still maps to 'emptyList'.
  • The pin is exact: cols.has('changed_fields') is false, and blankStorageForm returns the same array object.
  • Prose. The docblock sentence and the changeset's Readonly columns bullet say what the code does and cite the record. The changeset's other sentences are unchanged and still hold.
  • The witnesses CI named are green on this head: the Test Core shards, which carry cli activity-changed-fields.integration and plugin-audit activity-changed-fields-backfill.integration.
  • The merge b784ff6a brings origin/main forward. It touches no file in this review's scope beyond main's own state.

② Semver level

Unchanged: Clause-②: yes (widening), Metadata-protocol: none, @objectstack/core and @objectstack/objectql patch. BlankAsNullFieldDef gains an optional member: an additive type change on an existing export.

③ Boundary flags

④ Design judgment

This head follows ruling A with the round-2 carve-out exactly, and there is no must-fix. The optional prose-only spec card (a readonly clause in the multiple describe) is non-blocking and outside this PR.

Implemented-by: claude/project-thread-4rru8k
Reviewed-by: session_01JQwxbYyp6wSRcywGFK2L4A

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#3 · os-warren · session_01JQwxbYyp6wSRcywGFK2L4A

维护者速读

  • 改了什么:可选的多值字段(multiselect、tags、checkboxes,或 multiple: true 的 select / lookup / user / file / image)被清空为 [] 或 '' 时,写入口改存 null。唯一键覆盖与否都一样。
    • 例外:readonly: true 列(平台盖章,如 sys_activity.changed_fields)照原样存 []。
  • 为什么改:唯一的多值列上第二条清空记录会被 409 拒绝。按裁决 A(6591577985),以及第二轮 A(6598733139,readonly 例外)执行。
  • 风险与回滚:读变化——此后清空的可选多值字段读回 null 而非 [],同时命中 $null: true 与 $empty: true;required 语义不变;已存的 '[]' 不迁移(migrate: an os migrate value-shapes step stores null for '[]' and '""' already stored on optional multi-value columns (#29343 ruling A, data half) #29653 负责,已注明排除 readonly)。回滚即 revert。
  • 先看:packages/core/src/utils/blank-storage-form.ts 的 blankAsNullColumns 里 multi_value 分支。
  • 两分钟验证:pnpm --filter @objectstack/core exec vitest run src/utils/blank-storage-form.test.ts,预期全绿。
  • 需要你做的:无,席位落地。
  • 席位意见:这是 Clause-②: yes (widening),席位按档复核了四轮。
    • PASS 6597445147 → delta PASS 6597824563 → FAIL 6597870397(CI 揭出 ADR-0139 E6 冲突)→ 裁决后 delta PASS 6599243504。
    • 本 head 已无 must-fix。

席位核对

ACCEPT · domain:server#3 · head b702c0f1cee285eed74ff9e93e23a53c3babe785 · 2026-11-03T09:13Z · seat review of record at TIER_DEFAULT (declaration face: the Clause-②: yes (widening) line). Not governed: check-governed-merges --pr 29837 reports NOT governed, 680 lines. The review of record on this head is delta PASS 6599243504, which supersedes FAIL 6597870397.

  1. Shape: draft, base main, Fixes #29343, Clause-②: yes (widening), Metadata-protocol: none (the body and the changeset agree; @objectstack/core and @objectstack/objectql patch). 22 files, +585/−95, inside the claim and its amendment. mergeable_state is clean. The seat read the diff at each round; the round-2 hunk sits inside the multi_value branch and leaves the 'blank' and 'empty' arms untouched.
  2. Premise measured: red first, 12/15 at 97c2982c. Ruling A (6591577985) and round-2 A (6598733139) are recorded on the card. The dev re-ran the census at objectstack 55dca466 and hotcrm d4ffe99: sys_activity.changed_fields is the only readonly optional multi-value column.
  3. Gates on this head: check-runs pinned to b702c0f1: 69 total, 58 success, 10 skipped, 0 failed, 1 in progress (Check Changeset, rerun after the seat's body refresh; checked green before arming), including every Test Core shard, which carries the two CI witnesses of the FAIL record. Dev on this head:
    • cli 4/4 and 6/6; plugin-audit backfill 5/5; REST 34/34; core 2937/2937; objectql 10777/10777;
    • ablation reds exactly the CI cases;
    • 101 derived gates, 101 run, all exit 0.
  4. Prose checked against the diff: each changeset sentence holds against the diff:
    • "Readonly columns. … a [] written there keeps []": the def.readonly !== true guard and its pin.
    • "Unchanged. … A required multi-value field given [] or '' is refused": the required continue before the arm.
    • "os migrate blank-single-values does not touch these columns": its reach === 'empty' filter.
  5. Out of scope: migrate: an os migrate value-shapes step stores null for '[]' and '""' already stored on optional multi-value columns (#29343 ruling A, data half) #29653 (stored '[]' / '""') is scoped by note 6598764965 to exclude readonly columns. The optional prose-only spec card (a readonly clause in the multiple describe) is left to the spec lane. Skills-docs: not owed (Metadata-protocol: none).
  6. Landing: not governed, under 3000 lines, and contract review PASS on this head ⇒ the seat lands it (pr_ready + automerge_enable). At MERGED: landing record on objectql: a multi-value unique column (multiselect, or lookup with multiple: true) cleared to [] on two records refuses the second with 409 UNIQUE_VIOLATION #29343 (closed by Fixes), pm:dispatched removed, card thread stood down.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 3, 2026 09:16
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 3, 2026 09:16
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 8209a2f Nov 3, 2026
72 of 73 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-4rru8k branch November 3, 2026 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

1 participant