Repository navigation
feat(migrate,metadata-protocol): os migrate meta --stored rewrites sys_metadata rows in place (#4327) - #4464
Merged
Conversation
…s_metadata rows in place (#4327) #4317 closed the correctness gap from the read side: every stored-row rehydration seam replays the full ADR-0087 conversion chain, retired entries included, so a row written under any past protocol is served canonical forever. The rows themselves stayed legacy — the chain re-lowers them on every load and each logs a conversion notice per process. Until now the only things that rewrote such a row were a Studio re-save and duplicatePackage. `os migrate meta --stored` walks sys_metadata (active + draft, all orgs), replays the same applyConversionsToStoredItem pass, and re-saves each changed body through saveMetaItem — so a rewritten row gets a sys_metadata_history entry, a fresh checksum and the mutation projectors, exactly like an author's save. The history row's source is `migrate-stored`, distinguishing an upgrade from an edit. parentVersion is the row's own checksum, so a concurrent writer produces a 409 the report names rather than a clobber. Preview is the default and --apply the only writing mode, matching its two siblings and #3617's "a dry run changes nothing"; an apply run refuses to start while another process holds the SQLite database. Nothing gates on this having run (#3855) and no sys_migration flag is recorded — a flag would advertise enforcement that does not exist. What a run buys is hygiene plus an assertable verdict: nothing left to do exits 0, work remaining exits 1. Three carve-outs are reported rather than counted as done: flow rows (their seam is AutomationEngine.registerFlow, which holds the executor registry the node-type conflict guard needs), types with no repository write path (agent), and rows that still fail the current schema after conversion. An empty scan says it attests nothing rather than reading as a pass. Also: protocol.migrateStoredMetadata() returns the same structured report an admin route would render, and saveMetaItem takes an optional `source` for its history/audit rows — server-stated, never request-derived. Closes #4327 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WoZPKPDqJ7WB7z84xk9y3f
…sh line (#4454) The addendum said flows were "tracked separately" without saying where. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WoZPKPDqJ7WB7z84xk9y3f
…tion-stored-rewrite-jxlz2k # Conflicts: # packages/metadata-protocol/src/protocol.ts
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 23 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
The v17 entry for #3903 described the read-path guarantee and stopped there, and the upgrade checklist listed the two per-deployment migrations without this one. Both now point at `os migrate meta --stored`, marked optional — it opens no gate, unlike its two neighbours in that list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WoZPKPDqJ7WB7z84xk9y3f
…tion-stored-rewrite-jxlz2k
…tion-stored-rewrite-jxlz2k
This was referenced Aug 1, 2026
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Oct 7, 2026
…on a read-only boot (objectstack-ai#21389) Fixes objectstack-ai#21349 Clause-②: yes (narrowing) ## What changed Without `--apply`, `os migrate meta --stored` and `os migrate audit-metadata-bodies` now boot through the read-only boot `os migrate plan` already uses: `bootSchemaStack({ deferSchemaDdl: true, readOnlyProbe: true })`. Schema DDL is held back, the artifact's inline seed loader is suppressed (`deferSchemaDdl` implies `skipSeedData`), and a SQLite file that does not exist is opened as an empty in-memory stand-in instead of being created. `--apply` keeps the plain boot, unchanged. The code change is one spread per command (`meta.ts`, `audit-metadata-bodies.ts`). There is no new mechanism, no table skip list and no driver change. ## Measured before the fix (the card's steps, on `examples/app-crm`) Setup: `os build`, then `os dev -d file:base.db` on `examples/app-crm`. The seed load ran (28 rows) and the first admin was seeded. The server was stopped and the file copied. The copy has 82 tables and 150 rows. Each command ran on its own copy; the row hashes were read on a separate read-only connection. | run | crm_account | crm_activity | crm_contact | crm_lead | crm_opportunity | schema | |:--|:--|:--|:--|:--|:--|:--| | `meta --stored`, BASE `3937ad2f32` | 3 of 3 rows changed | 5 of 5 | 3 of 3 | 5 of 5 | 12 of 12 | unchanged | | `audit-metadata-bodies`, BASE | 3 of 3 | 5 of 5 | 3 of 3 | 5 of 5 | 12 of 12 | unchanged | | `meta --stored`, this PR | identical | identical | identical | identical | identical | identical | | `audit-metadata-bodies`, this PR | identical | identical | identical | identical | identical | identical | The changed columns were `updated_at` (bumped) and `organization_id` (stamped with the admin's organization on seeded rows that had none). The log line was `[Seeder] Seed loading complete {"inserted":0,"updated":28,...}`. After the fix it is `[Seeder] skipSeedData — inline seed suppressed`. The report each preview prints is the same as before, apart from one paged-read notice the seed loader itself caused. ### The write paths found, and what closes each 1. **The app's inline seed** (`AppPlugin.start`): an upsert of every seeded row. In the pin below it also puts an operator's edit to a seeded row back to the seed's value (`status: 'won'` back to `'open'`). This is closed by `skipSeedData`. 2. **Boot schema sync**: on a database behind the app's schema, the plain boot adds missing columns and creates missing tables. Measured with the plain boot `--apply` takes, which is the boot both previews took before this PR: a dropped `crm_lead.phone` column was added back, and `sys_audit_log` was created with five named indexes. This is closed by `deferSchemaDdl`. On the same two edge databases, the fixed previews left the schema and every row identical. 3. **Creating the target**: a preview at a missing SQLite path created the file. This is closed by `readOnlyProbe`. These are not write paths on this boot: the platform repair migrations (already off: `runPlatformMigrations: false`), a host `onEnable` (these commands compose no host config, and a compiled artifact cannot carry one), and the lifecycle sweep (its first run comes after the one-shot command has exited). ## Pins `packages/cli/src/commands/migrate/preview-read-only.integration.test.ts` runs the real commands through oclif: parse, occupancy probe, boot, walk and shutdown. Each case runs against a database that a plain boot seeded and an operator then edited, and that carries one legacy flow row and one cleartext audit copy of a datasource body. Per dialect cell: - `meta --stored` without `--apply`: the schema and every row are byte-identical, and the report still names the one pending row. - `meta --stored --apply --yes`: the legacy row is rewritten (the control). - `audit-metadata-bodies` without `--apply`: byte-identical, and the report still counts the one copy to rewrite. - `audit-metadata-bodies --apply --yes`: the copy is redacted (the control). - SQLite only: either preview at a missing file creates no file, `-wal`, `-shm` or `-journal`. Cells: SQLite always runs. Live PostgreSQL runs with `OS_TEST_POSTGRES_URL`, in a database derived from the file's path (`os_lv_` + slug + 12 hex characters of sha256; `check-live-db-isolation` PASS with this file in its 43 scanned files). Without the URL the cell is a named skip, and a failure under `OS_EXPECT_LIVE_DIALECT_MATRIX=1`. ## Ablations The fix was committed first (`88e37aa343`). Each leg ran through `scripts/ablation-replace.mjs` in WRAP mode: the anchor hit once, the mutation was confirmed on disk (anchor 1 to 0, marker 0 to 1, blob changed), and the restore was proven (blob == HEAD, `git diff HEAD` empty). The pin imports the commands by relative path, so it resolves to `src`, not `dist`. No rebuild was needed between legs. Runs used the live PostgreSQL cell. | leg | mutation | red | green | |:--|:--|:--|:--| | L1 | `meta.ts`: plain boot (spread removed) | SQLite preview, SQLite missing file, PG preview (3) | 7 | | L2 | `audit-metadata-bodies.ts`: plain boot | SQLite preview, SQLite missing file, PG preview (3) | 7 | | L3 | `meta.ts`: `deferSchemaDdl` only | SQLite missing file (1) | 9 | | L4 | `meta.ts`: `readOnlyProbe` only | SQLite preview, PG preview (2) | 8 | | L5 | `audit-metadata-bodies.ts`: `deferSchemaDdl` only | SQLite missing file (1) | 9 | | L6 | `audit-metadata-bodies.ts`: `readOnlyProbe` only | SQLite preview, PG preview (2) | 8 | The L1 diff shows the operator's edit reverted on both dialects: `"status": "won"` became `"open"` with a new `updated_at`. ## The 17.5.0 comparison This did not reproduce on `examples/app-crm`. The 17.5.0 CLI (tag `@objectstack/cli@17.5.0`, built in a comparison worktree) ran on a database it had seeded itself, and `meta --stored` rewrote the same 28 rows. That was measured twice, and both runs logged `"updated":28`. `meta --stored` has booted without the read-only options since it landed in `83cf2d3082` (objectstack-ai#4464). `audit-metadata-bodies` has done the same since it landed in `336e191441`, which is in 17.6.0 and not in 17.5.0 (`git merge-base --is-ancestor` exit 1; control leg, the root commit, exit 0; full clone). Why the HotCRM 17.5.0 run left its hashes identical cannot be measured from this repository. ## One edge behaves differently A preview pointed at a database that lacks the table it reads now fails and exits 1. Before, it created the table and reported nothing to examine. Measured: `meta --stored` on a missing file, or on a database without `sys_metadata`, exits 1 with the driver's refusal for `sys_metadata`, and no file is created. `audit-metadata-bodies` on a database without the audit tables exits 1 with `could not read sys_audit_log — its rows were NOT examined`. Before the fix, the same runs created the file or the tables and answered `No stored metadata to examine` or `Nothing to rewrite` with exit 0. The changeset states this. ## Verification at `a67e290cc7` (this branch merged with `origin/main` `11905a4f8b`) - `pnpm --filter @objectstack/cli build && pnpm --filter @objectstack/cli typecheck`: exit 0, and `check:test-typecheck` OK. The new test is under `src`, which `tsconfig.json` includes. - `pnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2`: 246 files, 3489 tests passed. - Integration: the new file plus `meta.stored-flow-resolution`, `schema-migrate.deferred-ddl` and `platform-migrations-arming` gave 4 files and 23 tests passed, with the live PostgreSQL cell (local PostgreSQL 16 on a private port). Without the URL: 6 passed and 1 named skip. - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands`: 63 families, each run one by one with its exit code recorded before any pipe. `--ran` reported 63 run, 0 NOT-MEASURED, 0 UNRUN. `check:dual-build-cjs-loads` and `check:i18n-coverage` first answered exit 3 (PREREQUISITE NOT MET: nine packages outside the CLI closure had no `dist`). After a turbo build of those nine (42 of 42 cache hits), both exited 0. - Lint, as a proven narrowing: eslint with the repo config and `--no-inline-config` over the 3 changed TS files (`--format json`) reported 3 files linted, 0 errors and 0 warnings, and no file was reported as ignored. The config enables no type-aware linting (`eslint.config.mjs` says so: no `parserOptions.project`, no typed rules), so this diff cannot move the verdict on any untouched file. The full `pnpm lint` is left to CI. ## Acceptance notes 1. **The same defect in seven sibling commands, not changed here because they are outside this claim's surface.** Measured at `a67e290cc7` on the same app-crm database: `os migrate value-shapes`, `summary-nulls`, `recorded-by`, `resume`, `files-to-references`, `os secret orphans` and `os storage orphans` each ran the seed loader and rewrote the same 28 rows. All seven are dry-run by default or report-only. `os migrate account-issuer` and `multi-value-columns` already boot read-only and left the database identical. This is reported to the seat as one family. 2. `--apply` on these two commands still runs the boot's seed loader and schema sync, as before. The direction keeps `--apply` unchanged. 3. Two comments now say more than is true: the `runPlatformMigrations` block in `schema-migrate.ts` and the non-deferred case in `platform-migrations-arming.integration.test.ts` list `os migrate meta` among the boots without `deferSchemaDdl`. After this PR that is true only with `--apply`. They are not edited here because both files are outside this claim's surface. 4. The live PostgreSQL cell has no CI leg. No step supplies `OS_TEST_POSTGRES_URL` to `@objectstack/cli`, so in CI the cell is a named skip. Wiring it takes one build step and one run step in the Temporal Conformance job of `.github/workflows/ci.yml`, which is also outside this claim's surface. 5. Docs: `content/docs/deployment/cli.mdx` already says the `meta --stored` preview "writes nothing", and this PR makes that true, so the docs are not edited. `cli.mdx` has no entry for `audit-metadata-bodies`. --- _Generated by [Claude Code](https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4327.
Why
#4317 answered #3903 from the read side: every stored-row rehydration seam replays the full ADR-0087 conversion chain, retired entries included, so a row written under any past protocol is served canonical forever. What it deliberately did not do is make the rows themselves canonical — a pre-17 row keeps its legacy bytes, the chain re-lowers it on every load, and each affected row logs one conversion notice per process. Until now the only things that ever rewrote such a row were a Studio re-save and
duplicatePackage.What
protocol.migrateStoredMetadata()walkssys_metadata—active+draft, every org — replays the sameapplyConversionsToStoredItempass, and re-saves each changed body throughsaveMetaItem. So a rewritten row gets everything an author's save gets: the schema gate, asys_metadata_historyrow, a fresh checksum, the mutation projectors and the watch event Studio's HMR consumes. Three deliberate arguments:parentVersion: row.checksum— a true optimistic lock. A concurrent writer that moved the row gets a 409, reported asfailed, never a clobber.force: true— the destructive diff compares the stored body (whichgetMetaItemalready serves converted) against the same conversion. Empty by construction, and there is no author here for a prompt to reach.source: 'migrate-stored'— a later history diff distinguishes an upgrade from somebody's edit.os migrate meta --storedis the operator surface. Archived/deprecated rows are never read (they are a record of what was), andsys_metadata_historyis appended to, never rewritten — converting a version body would break the checksum↔body pairing the contract depends on.Two deliberate deviations from the issue text
Preview is the default;
--applyis the only writing mode. The issue sketched "optionally--dry-run", i.e. writing by default. Both siblings (os migrate value-shapes,os migrate files-to-references) and #3617's "a dry run changes nothing" establish the opposite, and the reason applies with more force here because what moves is metadata: every affected row's checksum and one history entry per row. An apply run also refuses to start while another process holds the SQLite database, for the same reason the file migration does (--forceoverrides).No
sys_migrationflag row. The issue is explicit that this is not load-bearing, and #3855's conclusion stands — an operator-run migration cannot be relied upon, so the read path remains the guarantee. A flag row would advertise a gate that does not exist. The "explicit, verifiable my data is on protocol N" the issue asks for is delivered by the re-run instead: nothing left to do exits0, work remaining exits1, so the claim becomes a CI check rather than a belief.What it declines, and names
Reported, never counted as done:
flowrowsreservedNodeTypes). Flows canonicalize atregisterFlow. Filed as #4454agent)saveMetaItemroutes those down the legacy raw-engine branch: no history row, and a forcedstate: 'active'that would promote a draft. A historyless half-write is worse than leaving the row to the read pathAn empty scan says it attests nothing rather than reading as a pass — same lesson as the value-shape scan: "nothing to convert" and "nothing was looked at" are different claims, and a run from the wrong project root produces the second.
No speculative API was added for the flow case: a
canonicalizeFlowhook nothing can pass would advertise a capability the runtime does not deliver (PD #10). #4454 sketches the automation-engine entry point it needs.One bug fixed on the way
oclif's
dependsOncannot express "stored-only": a boolean withdefault: falseand anenv-backed string both read as provided, sodependsOn: ['stored']on--database-urlwould make a merely-exportedOS_DATABASE_URLbreakos migrate meta --from N. The guard reads raw argv instead (storedOnlyFlagsIn), which also closes adeclared ≠ enforcedgap:--applyon the authored-source chain is now refused rather than silently ignored.Verification
protocol.stored-migration.test.ts(preview writes nothing; apply rewrites in place; history row + fresh checksum +migrate-storedsource; re-run is a no-op; drafts stay drafts; all orgs; archived untouched; type filter; each carve-out; unparseable body; schema refusal; optimistic-lock conflict does not clobber), 8 inmeta.stored-flags.test.ts(the argv guard incl. the env-var trap, and the flag declarations).metadata-protocol·cli·objectql·rest·metadata·runtime·specall green after mergingmain(which carried three commits touching these packages; one import conflict inprotocol.ts, resolved keeping both sides).pnpm lint,tsc --noEmit, and thedoc-authoring/type-check-coverage/published-files/changeset-fixedgates pass.conditionalRequiredrow intoexamples/app-todo's SQLite → preview exits 1 and names the conversion →--applyrewrites → DB shows the same row id with a canonical body, a new checksum, and asys_metadata_historyrow{source: 'migrate-stored', previous_checksum: <the legacy checksum>}→ re-run reports canonical and exits 0.Docs
content/docs/deployment/cli.mdxgains anos migrate meta --storedsection (including the occupancy-gate row and the "hygiene, not a gate" framing), and ADR-0087 gains a 2026-08-01 addendum recording the decision — why no flag, why the write gate is not bypassed, why history stays verbatim, and what the pass does not cover.🤖 Generated with Claude Code
https://claude.ai/code/session_01WoZPKPDqJ7WB7z84xk9y3f
Generated by Claude Code