Skip to content

fix(metadata-protocol): removing a field that stores nothing is not a destructive change - #24479

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-24411-unstored-field-removal
Oct 19, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-24411-unstored-field-removal

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24411

Clause-②: yes (widening) (the save door newly accepts, without force, a removal it refused; owes the seat's TIER_DEFAULT review of record before the queue)

Dispatched by the domain:engine seat 1 PM (session session_01JtSQQ3EWsbNPzyYMug1tXy, claim comment 6259474942). Dev session: session_01RaFonyZMJU1FzgAT78bA47.

What changed

detectDestructiveObjectChanges in packages/metadata-protocol/src/protocol.ts (the object save's Phase 3a-destructive diff) now reports a finding only when the field holds stored data. "Stored" is the spec's storage predicate isVirtualSearchField (SEARCH_VIRTUAL_TYPES, exactly formula today), read from the field's own declaration. It is the predicate the served sortability projection (packages/spec/src/api/sortability.zod.ts) already reads, and protocol.ts already imported it. No name list or type list is kept in the detector.

  • field_removed: skipped when the removed field is unstored.
  • field_type_change: judged by what the stored side loses. An unstored previous field (a formula) held no values, so retyping it into any kind is not destructive. Retyping a stored field into a formula is still refused, because the stored side loses its column.
  • field_required_no_default: unchanged. It is not a storage-loss finding (see Acceptance notes).
  • No change to the force mechanism, to the message of a true removal, or to any error code.

Measured (M, booted composition)

Booted with bootStack (@objectstack/verify, sqlite-wasm) on a fixture app. Object dfu_invoice was saved at runtime through PUT /api/v1/meta/object/dfu_invoice with name/notes (text), amount (number), doubled (formula) and reference (autonumber), plus one record. Pin: packages/qa/dogfood/test/meta-object-unstored-field-removal.dogfood.test.ts.

case before (main fa26308bb0) after
remove doubled (formula), no force 409 DESTRUCTIVE_CHANGE, "Field 'doubled' removed — existing data in this column will become inaccessible. — re-submit with ?force=true to proceed." 200. GET /data/dfu_invoice/:id reads back name: 'INV-1', notes: 'keep me', amount: 21, reference: '0001' unchanged
remove notes (stored text), no force 409 DESTRUCTIVE_CHANGE, issues [{code: 'field_removed', field: 'notes', message: "Field 'notes' removed — existing data in this column will become inaccessible."}] identical 409, same issue and message
remove reference (autonumber: engine-computed but stored) 409 DESTRUCTIVE_CHANGE 409 DESTRUCTIVE_CHANGE (unchanged)

The predicate names only one unstored kind (formula). summary and autonumber are stored columns: the sortability docblock records them as "deliberately NOT members". So the "one more kind" reading is autonumber, measured as a stored control that still answers 409. This confirms the axis is storage, not COMPUTED_VALUE_TYPES.

Unit pins (protocol.destructive-gate-reachable-types.test.ts §4, reusing that file's existing engine double, so no new fake engine):

  • stored-text removal control: exact issue list, nothing written;
  • formula removal resolves and persists;
  • autonumber removal refused;
  • formula + text removed together reports notes alone;
  • formula → number resolves;
  • text → formula refused with field_type_change.

Before the fix, the three unstored cases were red and the stored controls green (Tests 3 failed | 13 passed (16)).

Ablation (one-shot, not kept)

  • Mutation: node scripts/ablation-replace.mjs --delete removed the line if (!isStored(prevFields[fname])) continue; (anchor x1 → x0, blob 767873d0 → fb38f325).
  • Rebuild and preflight: @objectstack/metadata-protocol rebuilt; ablation-dist-preflight --absent reported the marker absent from all 24 built files.
  • Result, red: unit 2 failed | 14 passed (formula removal, formula+text). Dogfood 1 failed | 2 passed: the formula removal answered the old 409.
  • Restore: blob back to 767873d0, equal to HEAD, and git diff HEAD empty. After a rebuild, the preflight reported the marker present in 2 built files and the tree clean. Unit 16/16 and dogfood 3/3 green.

Verification

  • pnpm --filter @objectstack/metadata-protocol test: Test Files 246 passed | 3 skipped, Tests 20661 passed | 19 skipped (exit 0, pre-merge head fb06fda4c).
  • pnpm --filter @objectstack/metadata-protocol typecheck: exit 0. --listFiles includes the edited test file.
  • pnpm --filter @objectstack/dogfood typecheck: exit 0. --listFiles includes the new pin.
  • Gates, first pass: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (no paths) derived 71 commands on fb06fda4c. All 71 were run, each exit captured before any pipe:
    • 70 exited 0.
    • pnpm check:dual-build-cjs-loads first exited 3 (PREREQUISITE NOT MET: 8 unbuilt packages). Those were built, and the re-run exited 0 ("111 published require entry point(s) across 69 package(s) load").
    • --ran reconciliation, with codes recorded: "71 derived, 71 run, 0 NOT-MEASURED, 0 UNRUN … a DERIVED zero".
  • The changeset-only follow-up commit re-ran check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check:nul-bytes, check:issue-citations and check:doc-authoring: all exit 0.
  • origin/main (ec951e29e) was merged cleanly before this PR opened. On the merge head 3174dbc00, every step exited 0:
    • pnpm install --frozen-lockfile;
    • pnpm --filter '@objectstack/dogfood^...' build;
    • pnpm --filter @objectstack/spec check:generated;
    • both packages' typechecks;
    • both pins.
  • The gate union was re-derived on 3174dbc00 (the same 71 commands) and re-run. All 71 exited 0, check:dual-build-cjs-loads included, and --ran reconciled "71 derived, 71 run, 0 NOT-MEASURED, 0 UNRUN".

Pin sweep

git grep for field_removed, DESTRUCTIVE_CHANGE and "will become inaccessible" across tests, docs, ADRs and the checklist found no pin that asserts a formula or unstored field removal is destructive. Every existing field_removed pin removes a text field, and those still hold. No hand-written doc states the old behaviour. ADR-0027's table names field_removed generically and still holds.

Acceptance notes

  • field_required_no_default on a formula is left as is. Its claim is about write validation of rows, not stored data becoming unreachable, and nothing here measured whether required-validation reaches a formula. That is outside this card.
  • The column planner (driver-sql fieldHasColumn, driver-turso) still spells type === 'formula' locally instead of calling the spec predicate. It agrees with the predicate today. This is noted as observed drift risk only, not filed (no carrier).
  • The claim's pin "a type change between two unstored kinds is not destructive" cannot be built today, because the predicate names one unstored kind. It is pinned as formula → stored (not destructive) and stored → formula (destructive). Once the predicate gains a second kind, the same isStored(prevField) check covers it with no edit.

Generated by Claude Code

…eld that stores nothing

A formula is computed on read and has no column, so removing it, or retyping
it into a stored kind, loses no data. The diff now judges each finding by the
spec's storage predicate (isVirtualSearchField), read from the field's own
declaration. A stored field's removal or retype keeps today's 409 and message.

Claude-Session: https://claude.ai/code/session_01RaFonyZMJU1FzgAT78bA47
Co-authored-by: Claude <noreply@anthropic.com>
…/meta/object/:name

Claude-Session: https://claude.ai/code/session_01RaFonyZMJU1FzgAT78bA47
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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 — 11 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 31ce45c071bbcb6e92a730ce19ebf30dcfa361cb → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 31ce45c071bbcb6e92a730ce19ebf30dcfa361cb

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 3174dbc003142448e408461f3b11410f57c95da1
Local-runs: none

Inputs: card #24411 (triage 6257436308, claim 6259474942, dispatch 6259559169), the dev's os-dev-report 6261095328 as claims, PR #24479's body and the net diff against its base ec951e29e4 (4 files, +261 / −1), and the head's check-runs (41 completed, none failed). Read with them: packages/spec/src/data/search-fields.ts (isVirtualSearchField, SEARCH_VIRTUAL_TYPES) and its readers.

① Derived judgments

  1. The accept-set change — RIGHT. detectDestructiveObjectChanges (packages/metadata-protocol/src/protocol.ts, the object save's destructive gate) now skips a removed field whose previous declaration is unstored, and reports field_type_change only when the previous field is stored. "Stored" is !isVirtualSearchField(field), judged from the field's own declaration.
  2. The predicate is the spec's one storage judgment — RIGHT. isVirtualSearchField reads SEARCH_VIRTUAL_TYPES (packages/spec/src/data/search-fields.ts). Its docblock says the set "mirrors the drivers' own storage rule", and search-fields.test.ts pins it to exactly ['formula'], so a second virtual type has to widen it deliberately. The same predicate is read by sortability.zod.ts, the search and sort lints, the filter doors and two ADR-0087 entries. protocol.ts already imported it (:165). No type list or name list is kept at the gate.
  3. What still answers 409 — RIGHT, and pinned:
    • a stored text removal answers the same field_removed finding and message, live and in-process;
    • an autonumber removal is engine-computed but stored, so it still answers 409;
    • a formula and a stored field removed together report the stored one alone;
    • a stored field retyped into a formula still answers 409 (field_type_change).
      A field whose type is unreadable is not virtual, so it stays "stored" and keeps the finding. That is the conservative direction.
  4. Unstored → stored retype — RIGHT, by the dispatch's rule ("judged by what the stored side loses"). A formula held no values, so the new column starts empty and nothing is lost. A 409 there would be the false finding that trains force=true.
  5. Unchanged — RIGHT by the diff: the force mechanism, every message, every error code, and field_required_no_default.
  6. Measured — the dev measured main fa26308bb0 on a booted composition: removing a formula through PUT /api/v1/meta/object/:name answered 409 DESTRUCTIVE_CHANGE; after the change it answers 200 without force, and the stored fields read back unchanged. The ablation of the one isStored skip turned the unit and booted formula pins red, and the restore was checked. The booted pin is packages/qa/dogfood/test/meta-object-unstored-field-removal.dogfood.test.ts, 3/3.

② Semver level

@objectstack/metadata-protocol minor. The changeset and the PR body carry the claim's line verbatim: Clause-②: yes (widening). The save door newly accepts, without force, a removal it refused. Nothing it accepted is refused, so the arm is right. No spec key, export or stored shape moves, so no ADR-0087 entry is owed. RIGHT.

③ Boundary flags

  1. The booted pin in packages/qa/dogfood: allowed by the dispatch and declared to domain:cli by the seat with this record.
  2. The unit pins as §4 of the existing protocol.destructive-gate-reachable-types.test.ts, reusing its pinned engine double: accepted (os-dev prefers an existing double).
  3. The claim's "type change between two unstored kinds" pin cannot be built while the predicate names one unstored kind. It is replaced by formula → number (not destructive) and text → formula (still 409): accepted. Those are the two directions the rule actually decides.
  4. The detached primary checkout in the dev's own container, with no edits there: accepted. Noted for the seat's next prompt.
  5. 71 gates, not the dispatch's 50: the no-path derivation reads the whole change set. All 71 ran with exit 0 on the final head. Accepted.
  6. The trailers: the model-free pair AGENTS.md prescribes. Accepted.
  7. Open question 1 (field_required_no_default for a formula): A, left as it is. That finding is about validating existing rows, not stored data becoming unreachable, so the storage rule does not reach it by its own terms. Answered.
  8. Open question 2 (unstored → stored retype): A, judged in ①.4. Answered.
  9. Out-of-scope finding: driver-sql's fieldHasColumn (schema-drift.ts) and driver-turso's remote transport each spell type === 'formula' locally instead of calling the spec predicate. They agree today and no wrong answer is measured, so it is below the filing gate. It is noted for the next touch of either driver, which should call isVirtualSearchField.

Implemented-by: claude/issue-24411-unstored-field-removal
Reviewed-by: session_01JtSQQ3EWsbNPzyYMug1tXy

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · domain:engine seat 1 · session_01JtSQQ3EWsbNPzyYMug1tXy · 2026-10-19T07:03Z · ACCEPT

Head: 3174dbc003 (the stored check, the unit and booted pins, the changeset, a merge of main). Card #24411 (p1, target:v18, bug), claim 6259474942, dispatch 6259559169. Dev report: 6261095328.

Item Reading
Shape Draft PR → main. The body opens Fixes #24411. 4 files, +261 / −1. Not governed (check-governed-merges.mjs --pr 24479: no path hits the register).
Scope detectDestructiveObjectChanges in metadata-protocol/src/protocol.ts: field_removed and field_type_change are reported only for a stored previous field, judged by the spec's isVirtualSearchField. One minor changeset. The unit pins extend the existing destructive-gate test; the booted pin is in packages/qa/dogfood.
Seat's own read SEARCH_VIRTUAL_TYPES is pinned to ['formula'] and mirrors the drivers' storage rule. The sortability projection, the search and sort lints and the filter doors read the same predicate, and protocol.ts already imported it. An unreadable type stays "stored", so the gate fails toward the warning.
Measured Booted, on main: a formula removal answered 409 DESTRUCTIVE_CHANGE. After: 200 without force, with the stored fields unchanged. A text and an autonumber removal answer the same 409, before and after.
Ablation Deleting the one isStored skip turned the unit and booted formula pins red. The restore read back as the HEAD blob.
Review TIER_DEFAULT record of record 6261617754 on this head: PASS, in-seat.
Declaration Clause-②: yes (widening) on the claim, the PR body and the changeset.
Cross-lane The dogfood pin is declared to domain:cli (6261627636, 6261632524).
Gates Dev, on 3174dbc003: 71 derived, 71 run, 71 exit 0. Typecheck exit 0 for metadata-protocol and the dogfood package.
CI At this read on 3174dbc003: 41 runs, all completed, none failed.

Disposition:

  • open_questions: both accepted (record ③.7, ③.8).
  • deviations: all accepted (record ③.1–③.6).
  • out_of_scope_findings: the two drivers' local type === 'formula' spellings agree with the predicate today. Noted for the next touch, not filed.

Landing: ordinary queue (pr_ready + automerge_enable through the relay). Fixes #24411 then closes the card.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 19, 2026 07:04
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 19, 2026 07:04
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 33a45cc Oct 19, 2026
42 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24411-unstored-field-removal branch October 19, 2026 07:38
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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Removing a formula field answers 409 DESTRUCTIVE_CHANGE ("existing data in this column will become inaccessible") although a formula stores nothing

2 participants