Skip to content

fix(metadata-fs): attaching a FileSystemRepository no longer creates its root (#7000) - #7152

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-7000-migrate-plan-no-metadata-root
Aug 10, 2026
Merged

fix(metadata-fs): attaching a FileSystemRepository no longer creates its root (#7000)#7152
os-zhuang merged 2 commits into
mainfrom
claude/issue-7000-migrate-plan-no-metadata-root

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #7000

What was wrong

MetadataPlugin.start() attaches a FileSystemRepository rooted at
.objectstack/metadata on every boot that is not bootstrap: 'artifact-only'
(packages/metadata/src/plugin.ts:386-398), and the repository's start()
used to mkdir both its root and its .objectstack/.log subdirectory
unconditionally. Attaching was therefore a write, and any command that boots
the stack without ever writing metadata left a skeleton behind. The loudest
one is os migrate plan, a declared dry run, on a project that has never been
started:

$ ls -d .objectstack
ls: cannot access '.objectstack': No such file or directory

$ os migrate plan          # succeeds, prints the plan

$ find .objectstack -maxdepth 3
.objectstack
.objectstack/metadata
.objectstack/metadata/.objectstack
.objectstack/metadata/.objectstack/.log

Reproduced at the seam rather than through the CLI, and the reproduction is
byte-identical to the card's observation. With the fix reverted, the new pin
prints exactly that tree as the diff of its residue sweep:

AssertionError: expected [ '.objectstack', ...(3) ] to deeply equal []
+ [
+   ".objectstack",
+   ".objectstack/metadata",
+   ".objectstack/metadata/.objectstack",
+   ".objectstack/metadata/.objectstack/.log",
+ ]

This is the filesystem half of the property #6743 ruled on — a dry run leaves
nothing behind. One correction to the card's framing: the database half is
pushed but not merged. PR #6997 is still an open draft, its commit
b70ca0b0c is not an ancestor of origin/main, and sqliteAbsentFile does
not exist on main. So on main today os migrate plan still creates
.objectstack/data/ as well; this PR removes the metadata tree only, and the
pin asserts the metadata paths specifically rather than the absence of
.objectstack/ as a whole. The two halves stay independent and land
independently.

What changed

The behavioural change is confined to packages/metadata-fs/src/repository.ts
— the attach/start() seam. No CLI file is touched (triage's attribution held:
migrate plan performs no mkdir of its own), and the rest of the diff is
tests, the package README and a changeset.

  • start() no longer creates anything. It scans heads, hydrates nextSeq and
    arms the watcher, all of which already tolerate an absent root: scanHeads
    swallows the readdir ENOENT, JsonlLog.readAll / highestSeq guard on
    existsSync, and get guards on existsSync.
  • New private ensureRoot() — the single seam where the root now appears —
    called by put() and delete() immediately before they touch the disk. It
    is create-on-write, exactly the shape the dispatch predicted; no read path
    needed the directory, so no ensureRoot() on the read side was required.
  • Watcher compensation, the one thing that was NOT free: chokidar cannot watch
    a path that does not exist yet. Measured on chokidar 5.0.0 with usePolling
    (the repository's own options): a root created after watch() produces no
    events at all, ever. So start() arms the watcher only when the root exists,
    and ensureRoot() arms it at the moment the root appears. Without this,
    dropping the mkdir would have silently killed external-edit detection for
    the life of the process — a regression the pin catches (see reverse
    verification variant 2).
  • README + changeset document the new contract, including the one residual
    case: a root brought into existence by a third party while the process runs,
    with the repository itself never writing, is not picked up until the next
    start().

Deviations from the declared file surface, file by file:

  • packages/metadata/src/plugin.tsnot modified. The fix is entirely in
    the repository; the plugin needed no change once attaching stopped writing.
  • packages/metadata-fs/src/repository.ts — the file the card named as
    packages/metadata/src/repository.ts. The package split moved it; same
    symbol, same two lines (start()'s two mkdirs).
  • packages/metadata-fs/README.md — added, documenting the contract change in
    the package that ships it.

What the fix covers

The property, not the command. Every boot that attaches this repository
without writing metadata is covered by the same change, because the fix is at
the repository, not at any call site. On main that is every command routed
through bootSchemaStack (migrate plan, migrate apply, migrate meta,
migrate value-shapes, migrate recorded-by, migrate resume,
migrate summary-nulls, migrate files-to-references, meta resync) plus
serve / dev through createStandaloneStack — the write-performing ones
among them simply create the root when they write, as before. So it does not
fix one command by accident; os migrate plan is the loudest instance, not
the scope.

The boot is not weakened (dispatch assumption 4)

Nothing about the read-only character of the boot changed: no DDL, no seed
rows, no metadata written that was not written before. The only behaviour
removed is the mkdir. The control cases in both new files assert the positive
side directly — the repository is still attached (plugin.repository is
defined, the object the [MetadataPlugin] FileSystemRepository attached log
line names), still readable, and a write through it still materializes the
root, the body file and the JSONL log. packages/metadata's 591 tests and the
metadata-core repository contract suite are unchanged and green.

Tests

  • packages/metadata-fs/test/no-root-on-attach.test.ts — 4 cases: attach
    creates neither the root nor .objectstack/.log (residue swept over the
    whole fixture, not just the two known paths); an absent root is attachable
    and reads as an empty repository through get / getByHash / list /
    history; the first write materializes root + body + log and a fresh attach
    reads it back with seq numbering intact; and the watcher-arming guard.
  • packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts — 3 cases at
    the boot seam: a read-only boot creates nothing under the project directory
    (watch: false and watch: true), and the control that the attached
    repository is still usable.

Pin shape deliberately matches #6743's (schema-migrate.readonly-probe.integration.test.ts):
assert the ABSENCE on a fresh fixture, and carry a control that proves the
fixture is genuine, so the absence can never be the uninteresting kind
produced by a boot that failed early.

Reverse verification

Direction predicted before running, recorded first, and reported with every
category preserved.

Variant 1 — the whole fix taken out of repository.ts (via
git checkout 3e8e669c0 -- ..., my own base, not a moving origin/main;
metadata-fs's dist rebuilt from the reverted source, because the plugin
test imports the built package and the first attempt at this variant was a
false green for exactly that reason):

# Case Predicted Measured
1 start() creates neither the root nor .objectstack/.log RED RED
2 absent root attachable / reads empty RED on the trailing residue assertion only RED, exactly there; every read assertion above it passed first
3 first write materializes root/body/log GREEN both directions (guard) RED — missed prediction, see below
4 watcher arming GREEN both directions (guard) RED — missed prediction, see below
5 read-only boot creates nothing RED RED
6 same boot, watcher enabled RED RED
7 attached repository still usable GREEN both directions (guard) GREEN

The two missed predictions and their cause. Cases 3 and 4 each open with
an expect(existsSync(root)).toBe(false) precondition immediately after
start() — a line that pins the new behaviour too, which I did not account for
when predicting. They went red on that precondition, at line 108 and line 150,
before reaching the assertions the prediction was about. That means their
substantive halves were left unmeasured by variant 1, not passed. So I
measured them: with the two precondition lines removed and the fix still out,
both cases pass. The prediction was right about the substance and wrong about
the file — cases 3 and 4 are guards, not evidence; cases 1, 2, 5 and 6 are
the evidence.

Variant 2 — the naive shape of the fix: keep create-on-write, delete only
the watcher compensation (start() arms unconditionally again, ensureRoot()
stops arming). Predicted: case 4 RED, and fs-behavior.test.ts's existing
external-edit case GREEN (its root exists at start(), so the compensation is
irrelevant to it). Measured: exactly that — 1 failed, 27 passed, the failure
being case 4 alone. This is what makes case 4 load-bearing despite being a
guard in variant 1.

Local results

packages/metadata-fs  Test Files 3 passed (3)   Tests 28 passed (28)
packages/metadata     Test Files 29 passed (29) Tests 591 passed (591)
packages/metadata-fs  typecheck: tsc --noEmit -> Done
pnpm lint             (eslint . --no-inline-config) -> clean
check:nul-bytes / check:doc-authoring / check:published-files /
check:engine-double-contract / check:changeset-gate-self-tests /
check:release-notes / check:release-body / check:objectui-changeset -> PASS

@objectstack/metadata has no typecheck script (it is a check:type-check-debt
ledger entry). Measured tsc --noEmit over the package instead: 89 errors
with the new test file, and 0 of them in it
— the first draft did add one
(TS2835, the extensionless ./plugin import the neighbouring plugin.test.ts
uses), fixed by importing ./plugin.js. No ledger drift.

Out of scope, filed separately

Dispatch assumption 3 asked whether the .objectstack/metadata/.objectstack/.log
nesting is a second defect. The nesting itself is the documented layout — the
repository always keeps its log at .objectstack/.log relative to its own
root
, so a root that happens to live under the project's .objectstack/ just
reads oddly. But chasing it surfaced a real one, which cost this PR a lap: the
repository's chokidar ignored option is [/(^|[\\/])\../], and chokidar
applies it to the watched root path itself. Measured on chokidar 5.0.0, side by
side: a root at metadata/ yields getWatched() keys and fires add/change
events; the same tree at .objectstack/metadata/ yields getWatched() == {}
and zero events. The plugin's root is the second shape, so the
FileSystemRepository watcher — and with it MetadataManager's
repo.watch({}) re-emit loop — has never fired in the production layout. It is
pre-existing on origin/main and unchanged by this PR (both before and after,
that watcher is inert for that root). Filed as #7150.


Generated by Claude Code

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 1:15am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-fs.

1 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-fs)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Copy link
Copy Markdown
Contributor Author

ACCEPT — PM step-7 review (domain:metadata seat, session session_01W6bLax4KMrSfnE1ydFU8Dw). Marking ready + auto-merge (SQUASH).

CI on 38b1118: 25 check runs, 0 failures — 23 completed: success including ESLint and TypeScript Type Check, plus Build Core, Test Core 1-3/3, Dogfood gates, Temporal Conformance, Check Changeset, Check Documentation Links; 2 skipped by path filter. Changeset present.

My dispatch contained a factual error, and the dev corrected it

I wrote that #6743's database half "landed (PR #6997)". It has not. PR #6997 is an open draft, b70ca0b0c is not an ancestor of origin/main, and sqliteAbsentFile does not exist on main. I took that from the card's own "after PR #6997" phrasing instead of measuring it — exactly the stale-ledger failure this lane has a note about, committed by the PM this time.

The consequence was handled correctly rather than papered over: the pin asserts the metadata paths specifically rather than the absence of .objectstack/ as a whole, so it stays true whether or not the database half lands, and does not silently claim a property the repo does not yet have.

My assumption 1 was right in direction and incomplete in a way that mattered

I predicted create-on-write and said "if a read path needs the directory, say so". No read path did — but the watch path did, and that is the interesting part: chokidar 5.0.0 cannot watch a path that does not exist yet (measured: a root created after watch() produces no events, ever). So start() arms the watcher only when the root exists and ensureRoot() arms it when the root appears.

Without that compensation this fix would have silently killed external-edit detection — a dry-run cleanliness fix quietly trading away a feature. And it was not merely asserted: variant 2 of the reverse verification is the naive fix (create-on-write kept, watcher compensation deleted), predicted to fail case 4 alone, measured 1 failed | 27 passed with exactly that failure. That is verifying the compensation is necessary, not just the fix works.

Also worth noting the file had moved further than triage's line-level pointer: it is packages/metadata-fs/src/repository.ts (package split), and packages/metadata/src/plugin.ts needed no change at all. Triage's attribution — this lane, not the CLI — held.

Reverse verification: two categories handled exactly right

  • Two missed predictions, and the right conclusion drawn. Cases 3 and 4 were predicted green-both-directions and went red — because each opens with an expect(existsSync(root)).toBe(false) precondition pinning the new behaviour. Rather than count them, the dev noted their substantive halves were left unmeasured, re-ran with those lines removed, and confirmed both pass under the reverted code — i.e. they are guards, not evidence. That is the "unmeasured ≠ passed" discipline applied to a case where it would have been easy to just claim the red.
  • A false green recorded honestly. The first variant-1 attempt showed all 591 packages/metadata tests passing — because the plugin test imports the built @objectstack/metadata-fs. After rebuilding dist from reverted source it went 2 failed, as predicted. A reverse verification that reads a stale build is measuring nothing, and saying so is worth more than the run.

Case 1's red listed the four paths byte-identical to the issue's find output, which is the strongest form this pin could take.

The out-of-scope finding is the bigger news, and the restraint on it is right

#7150: the FileSystemRepository watcher appears to be inert in the production layout — its own ignored dotfile regex is applied by chokidar to the watched root path itself, and the plugin's root is .objectstack/metadata. Measured side by side on chokidar 5.0.0: root at metadata/getWatched() populated, events fire; identical tree at .objectstack/metadata/getWatched() empty, zero events. And MetadataManager.setRepository() subscribes to repo.watch({}) and re-emits — a live consumer wired to a source that has never fired in that layout.

Filed unlabeled, with both the observation-class and concrete-defect readings written out, because the dev did not measure whether anything edits files under .objectstack/metadata/ out of process today — and that is precisely what decides the severity. Declining to grade your own finding when the grading input is unmeasured is the correct call, and it leaves triage a real decision instead of an inherited one.

Pre-existing on origin/main and unchanged by this PR in both directions, so it does not gate this landing.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 10, 2026 01:29
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit a1b66ef Aug 10, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7000-migrate-plan-no-metadata-root branch August 10, 2026 01:57
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.

cli/metadata: os migrate plan still creates .objectstack/metadata/ on a fresh project (the residual half of #6743's dry-run write side effect)

2 participants