From 99cf34996c0ac0865322fe14e702a7ef2ece138d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 00:59:55 +0000 Subject: [PATCH 1/2] fix(metadata-fs): attaching a FileSystemRepository no longer creates its root (#7000) --- .changeset/gentle-pears-attach.md | 17 ++ packages/metadata-fs/README.md | 17 +- packages/metadata-fs/src/repository.ts | 54 +++++- .../test/no-root-on-attach.test.ts | 177 ++++++++++++++++++ .../plugin-no-metadata-root-on-boot.test.ts | 143 ++++++++++++++ 5 files changed, 400 insertions(+), 8 deletions(-) create mode 100644 .changeset/gentle-pears-attach.md create mode 100644 packages/metadata-fs/test/no-root-on-attach.test.ts create mode 100644 packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts diff --git a/.changeset/gentle-pears-attach.md b/.changeset/gentle-pears-attach.md new file mode 100644 index 0000000000..d4fab773e6 --- /dev/null +++ b/.changeset/gentle-pears-attach.md @@ -0,0 +1,17 @@ +--- +"@objectstack/metadata-fs": patch +--- + +`FileSystemRepository` no longer creates its root directory when it is attached — only when it first writes. + +`start()` used to `mkdir` both `` and `/.objectstack/.log` unconditionally, so merely attaching a repository was a write. Because `MetadataPlugin` attaches one at `/.objectstack/metadata` during every boot, a command that never writes metadata still brought a directory skeleton into existence. The loudest case is `os migrate plan`, a declared dry run: on a project that had never been started it left + +``` +.objectstack/metadata/.objectstack/.log +``` + +behind, which also destroyed the one signal — does `.objectstack/` exist? — by which the next command can tell a fresh project from a started one. This is the filesystem half of the same property the database half already covers: a dry run leaves nothing behind. + +Attaching and reading a repository whose root does not exist is now explicitly supported and answers as an empty repository (`get`, `getByHash`, `list`, `history`, `watch`). The root, the type directories and the JSONL change log all appear on the first `put` / `delete`, and nothing about the boot's read-only character changes: no metadata is written that was not written before. + +One behavioural note for direct users of the package: when the root is absent at `start()`, the chokidar watcher is armed by the first write instead, because chokidar cannot watch a path that does not yet exist. A root brought into existence by a third party while the process runs — with this repository never writing — is therefore not picked up until the next `start()`. diff --git a/packages/metadata-fs/README.md b/packages/metadata-fs/README.md index 3e4b1ce8a6..9bcbaf5854 100644 --- a/packages/metadata-fs/README.md +++ b/packages/metadata-fs/README.md @@ -35,7 +35,7 @@ const repo = new FileSystemRepository({ root: './metadata', org: 'system', }); -await repo.start(); // scan + open watcher +await repo.start(); // scan + open watcher — creates nothing on disk const view = await repo.get({ org: 'system', @@ -47,4 +47,19 @@ for await (const evt of repo.watch({})) { } ``` +## Root creation is a write, not an attach + +`start()` never creates ``. Attaching a repository whose root does not +exist is legal: reads answer as if the repository were empty, and the root — +together with `/.objectstack/.log/` — appears on the **first write** +(`put` / `delete`). This is what keeps a read-only boot, such as the dry run +`os migrate plan` performs on a project that has never been started, from +leaving a directory skeleton behind (#7000, the filesystem half of #6743). + +One consequence worth knowing: when the root is absent at `start()`, the +chokidar watcher is armed by the first write instead, because chokidar cannot +watch a path that does not exist yet. A root brought into existence by a third +party while the process runs, without this repository ever writing, is +therefore not picked up until the next `start()`. + See ADR-0008 (incl. §0 amendment) and the `metadata-branch-removal` changeset. diff --git a/packages/metadata-fs/src/repository.ts b/packages/metadata-fs/src/repository.ts index afa0324c6c..8361757bb6 100644 --- a/packages/metadata-fs/src/repository.ts +++ b/packages/metadata-fs/src/repository.ts @@ -16,6 +16,9 @@ * log is a denormalised history index. * - chokidar-driven external edits are translated into MetadataEvents * by hashing the new content and comparing to the last-known hash. + * - The root directory is created **on the first write, not on attach** + * (#7000). Attaching and reading a repository whose root does not exist + * is legal and answers "empty"; see `start()` / `ensureRoot()`. */ import fs from 'node:fs/promises'; @@ -46,7 +49,6 @@ import { itemPath, parseItemPath, typeDir, - logDir, logFile, } from './layout.js'; import { JsonlLog } from './jsonl-log.js'; @@ -108,21 +110,55 @@ export class FileSystemRepository implements MetadataRepository { // ── Lifecycle ─────────────────────────────────────────────────────── + /** + * Attach the repository. **Creates nothing on disk** (#7000). + * + * Attaching is not a write. `start()` used to `mkdir` both the root and + * `/.objectstack/.log` unconditionally, which meant every read-only + * boot that merely attaches a repository left a skeleton behind — most + * visibly `os migrate plan`, a declared dry run, on a project that has + * never been started. That is the same property #6743 ruled on for + * `.objectstack/data/`: a dry run leaves nothing behind, and the existence + * of `.objectstack/` has to stay a usable "this project has been started" + * signal. + * + * Every read path below already treats a missing root as an empty + * repository (`scanHeads` swallows ENOENT, `JsonlLog` guards on + * `existsSync`, `get` guards on `existsSync`), so the root is materialized + * by `ensureRoot()` on the first write instead. + */ async start(): Promise { if (this.started) return; this.started = true; - await fs.mkdir(this.layout.root, { recursive: true }); - await fs.mkdir(logDir(this.layout), { recursive: true }); - // 1) Scan body files to build the head index. + // 1) Scan body files to build the head index. No-op on a missing root. await this.scanHeads(); - // 2) Hydrate nextSeq from the existing log. + // 2) Hydrate nextSeq from the existing log. No-op on a missing log. const highest = await this.log.highestSeq(); this.nextSeq = highest + 1; - // 3) Start the watcher (unless disabled). - if (!this.disableWatch) this.startWatcher(); + // 3) Start the watcher (unless disabled). chokidar cannot watch a path + // that does not exist yet: measured on chokidar 5 with `usePolling`, + // a root created AFTER `watch()` produces no events at all, ever. So + // when the root is absent the watcher is armed later, by the + // `ensureRoot()` call that brings the root into existence — otherwise + // dropping the `mkdir` above would silently kill external-edit + // detection for the whole life of the process. + if (!this.disableWatch && existsSync(this.layout.root)) this.startWatcher(); + } + + /** + * Bring the repository root into existence. Called by every write path + * immediately before it touches the disk — `start()` deliberately does not + * create it (#7000), so this is the single seam where the root appears. + * + * It is also where a watcher that `start()` could not arm (missing root) + * gets armed, so "external edits are detected" survives the change. + */ + private async ensureRoot(): Promise { + await fs.mkdir(this.layout.root, { recursive: true }); + if (this.started && !this.disableWatch && !this.watcher) this.startWatcher(); } async close(): Promise { @@ -258,6 +294,8 @@ export class FileSystemRepository implements MetadataRepository { const seq = this.nextSeq++; const ts = this.now().toISOString(); const file = itemPath(this.layout, ref.type, ref.name); + // First write of the process materializes the root (#7000). + await this.ensureRoot(); await fs.mkdir(typeDir(this.layout, ref.type), { recursive: true }); this.selfWrites.add(file); try { @@ -309,6 +347,8 @@ export class FileSystemRepository implements MetadataRepository { throw new ConflictError(ref, opts.parentVersion, currentHead); } const file = itemPath(this.layout, ref.type, ref.name); + // A delete appends a tombstone to the change log, so it is a write too. + await this.ensureRoot(); this.selfWrites.add(file); try { if (existsSync(file)) await fs.unlink(file); diff --git a/packages/metadata-fs/test/no-root-on-attach.test.ts b/packages/metadata-fs/test/no-root-on-attach.test.ts new file mode 100644 index 0000000000..cf05e3e3e4 --- /dev/null +++ b/packages/metadata-fs/test/no-root-on-attach.test.ts @@ -0,0 +1,177 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7000 — attaching a `FileSystemRepository` must leave nothing on disk. + * + * This is the filesystem half of the property #6743 ruled on: a dry run + * leaves nothing behind. #6743 / PR #6997 covered `.objectstack/data/` (the + * sqlite file the driver brought into existence at open); the half left over + * is the metadata repository, whose `start()` used to `mkdir` both the root + * and `/.objectstack/.log` unconditionally. `os migrate plan` on a + * project that has never been started therefore left a + * `.objectstack/metadata/.objectstack/.log` skeleton behind, and the presence + * of `.objectstack/` stopped being a usable "this project has been started" + * signal. + * + * The pin is written the same way #6743's is: assert the ABSENCE of the side + * effect on a fresh fixture, and carry a control that proves the fixture is + * genuine — here, that the very same repository is attachable, readable and + * writable, so the absence is not the uninteresting kind produced by a boot + * that failed early. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs/promises'; +import { existsSync, readdirSync } from 'node:fs'; +import path from 'node:path'; +import os from 'node:os'; +import type { MetaRef, MetadataEvent, MetadataItemHeader } from '@objectstack/metadata-core'; +import { FileSystemRepository } from '../src/index.js'; + +const ref = (name: string): MetaRef => ({ org: 'system', type: 'view', name }); +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +describe('FileSystemRepository — attaching creates nothing on disk (#7000)', () => { + /** Stands in for the project directory: nothing below it exists yet. */ + let base: string; + /** Stands in for `/.objectstack/metadata` — deliberately absent. */ + let root: string; + let repo: FileSystemRepository | undefined; + + beforeEach(async () => { + base = await fs.mkdtemp(path.join(os.tmpdir(), 'objectstack-fs7000-')); + root = path.join(base, '.objectstack', 'metadata'); + }); + + afterEach(async () => { + if (repo) await repo.close().catch(() => undefined); + repo = undefined; + await fs.rm(base, { recursive: true, force: true }); + }); + + /** + * Every path anywhere below the project directory. Naming only the two + * paths the old `mkdir`s created would miss a third one added later, so the + * pin sweeps the whole fixture instead. + */ + function residueOnDisk(): string[] { + const found: string[] = []; + const walk = (dir: string, depth: number): void => { + if (depth > 5) return; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + found.push(path.relative(base, full)); + if (entry.isDirectory()) walk(full, depth + 1); + } + }; + walk(base, 0); + return found.sort(); + } + + it('start() on a never-started project creates neither the root nor .objectstack/.log', async () => { + expect(existsSync(root)).toBe(false); + + repo = new FileSystemRepository({ root, org: 'system', disableWatch: true }); + await repo.start(); + + expect(existsSync(root)).toBe(false); + expect(existsSync(path.join(root, '.objectstack', '.log'))).toBe(false); + expect(residueOnDisk()).toEqual([]); + + // Detach is not a write either. + await repo.close(); + expect(residueOnDisk()).toEqual([]); + }); + + it('an absent root is attachable and reads as an empty repository', async () => { + repo = new FileSystemRepository({ root, org: 'system', disableWatch: true }); + await repo.start(); + + expect(await repo.get(ref('case_grid'))).toBeNull(); + expect(await repo.getByHash(ref('case_grid'), 'not-a-real-hash')).toBeNull(); + + const listed: MetadataItemHeader[] = []; + for await (const header of repo.list({ org: 'system' })) listed.push(header); + expect(listed).toEqual([]); + + const past: MetadataEvent[] = []; + for await (const evt of repo.history(ref('case_grid'))) past.push(evt); + expect(past).toEqual([]); + + // No read may be paid for with a directory. + expect(residueOnDisk()).toEqual([]); + }); + + it('the FIRST write materializes the root, the body file and the change log', async () => { + repo = new FileSystemRepository({ root, org: 'system', disableWatch: true }); + await repo.start(); + expect(existsSync(root)).toBe(false); + + await repo.put(ref('case_grid'), { label: 'Cases' }, { parentVersion: null, actor: 'tester' }); + + expect(existsSync(root)).toBe(true); + expect(existsSync(path.join(root, 'view', 'case_grid.json'))).toBe(true); + expect(existsSync(path.join(root, '.objectstack', '.log', 'main.jsonl'))).toBe(true); + + // The deferred root is a real repository, not a half-built one: a fresh + // attach over the same path reads the item and its log back. + await repo.close(); + repo = new FileSystemRepository({ root, org: 'system', disableWatch: true }); + await repo.start(); + const item = await repo.get(ref('case_grid')); + expect(item?.body).toEqual({ label: 'Cases' }); + expect(item?.seq).toBe(1); + + // And seq numbering continues from the log the deferred root wrote. + const second = await repo.put( + ref('case_grid'), + { label: 'Cases (renamed)' }, + { parentVersion: item!.hash, actor: 'tester' }, + ); + expect(second.seq).toBe(2); + }); + + it('arms the watcher on the first write, so external edits are still detected', async () => { + // The one property that dropping the `mkdir` could have silently killed: + // chokidar cannot watch a path that does not exist yet, so a watcher armed + // by `start()` against an absent root would never fire again. `ensureRoot` + // arms it at the moment the root appears — this is that guard. + // + // The root here is deliberately NOT under a dot-directory. The watcher's + // own `ignored: [/(^|[\\/])\../]` matches ANY dot segment of the path, + // including segments of the watched root itself, so a root such as the + // plugin's `/.objectstack/metadata` is ignored wholesale and + // chokidar watches nothing at all. That is pre-existing on `origin/main` + // and out of scope here (filed separately); this case pins the arming, so + // it uses a root the watcher can actually see. + const plainRoot = path.join(base, 'project', 'metadata'); + repo = new FileSystemRepository({ root: plainRoot, org: 'system' }); // watcher ENABLED + await repo.start(); + expect(existsSync(plainRoot)).toBe(false); + + await repo.put(ref('externally_edited'), { label: 'original' }, { parentVersion: null, actor: 'tester' }); + expect(existsSync(plainRoot)).toBe(true); + + const iter = repo.watch({ org: 'system' }, 999)[Symbol.asyncIterator](); + const collected: MetadataEvent[] = []; + const collector = (async () => { + const r = await iter.next(); + if (!r.done) collected.push(r.value as MetadataEvent); + })(); + + // Past the 200ms self-write suppression window of the put above. + await sleep(400); + await fs.writeFile( + path.join(plainRoot, 'view', 'externally_edited.json'), + JSON.stringify({ label: 'externally edited' }, null, 2), + ); + + await Promise.race([collector, sleep(8000)]); + await iter.return?.(undefined); + + expect(collected).toHaveLength(1); + expect(collected[0]!.op).toBe('update'); + expect(collected[0]!.source).toBe('fs'); + expect(collected[0]!.actor).toBe('fs'); + }, 20_000); +}); diff --git a/packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts b/packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts new file mode 100644 index 0000000000..9cfbc77633 --- /dev/null +++ b/packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts @@ -0,0 +1,143 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7000 — booting `MetadataPlugin` on a project that has never been started + * must not bring `.objectstack/metadata/` into existence. + * + * This is the command-facing half of the same pin. `os migrate plan` is a + * declared dry run: #3917 made its boot stop writing DDL and seed rows, and + * #6743 / PR #6997 stopped the sqlite driver from creating + * `.objectstack/data/`. What was left was this plugin — it attaches a + * `FileSystemRepository` at `/.objectstack/metadata` during `start()`, + * and the repository's `start()` used to `mkdir` its root unconditionally, so + * the dry run left behind: + * + * .objectstack/metadata/.objectstack/.log + * + * The CLI command is only the trigger — it performs no `mkdir` of its own — + * so the pin sits at the attach seam, where the fix does. Everything that + * boots this plugin without writing metadata is covered by the same change, + * not just `migrate plan`. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, existsSync, readdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { MetadataPlugin } from './plugin'; + +vi.mock('@objectstack/core', async (orig) => { + const real = (await orig()) as any; + return { + ...real, + createLogger: () => ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }), + }; +}); + +function createMockPluginContext() { + return { + registerService: vi.fn(), + replaceService: vi.fn(), + getService: vi.fn().mockReturnValue(null), + getServices: vi.fn().mockReturnValue(new Map()), + hook: vi.fn(), + trigger: vi.fn(), + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + getKernel: vi.fn(), + } as any; +} + +describe('MetadataPlugin boot leaves no .objectstack/metadata behind (#7000)', () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'os-meta7000-')); + }); + + afterEach(() => { + try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('a read-only boot creates nothing under the project directory', async () => { + expect(existsSync(join(dir, '.objectstack'))).toBe(false); + + const plugin = new MetadataPlugin({ + rootDir: dir, + watch: false, + config: { bootstrap: 'lazy' }, + }); + const ctx = createMockPluginContext(); + await plugin.init(ctx); + await plugin.start(ctx); + + try { + // Control: the attach really happened. Without this the pin would + // pass for the uninteresting reason that no repository was ever + // created — the boot log line `[MetadataPlugin] FileSystemRepository + // attached` names exactly this object. + expect((plugin as any).repository).toBeDefined(); + + expect(existsSync(join(dir, '.objectstack'))).toBe(false); + expect(existsSync(join(dir, '.objectstack', 'metadata'))).toBe(false); + expect(readdirSync(dir)).toEqual([]); + } finally { + await plugin.stop(ctx); + } + + // Shutting the plugin down is not a write either. + expect(readdirSync(dir)).toEqual([]); + }); + + it('the same boot with the watcher enabled also creates nothing', async () => { + const plugin = new MetadataPlugin({ + rootDir: dir, + watch: true, + config: { bootstrap: 'lazy' }, + }); + const ctx = createMockPluginContext(); + await plugin.init(ctx); + await plugin.start(ctx); + + try { + expect((plugin as any).repository).toBeDefined(); + expect(readdirSync(dir)).toEqual([]); + } finally { + await plugin.stop(ctx); + } + }, 20_000); + + it('the attached repository is still usable — a write materializes the root', async () => { + // The other half of the control: absence must come from "attach does + // not write", not from a repository that cannot write at all. + const plugin = new MetadataPlugin({ + rootDir: dir, + watch: false, + config: { bootstrap: 'lazy' }, + }); + const ctx = createMockPluginContext(); + await plugin.init(ctx); + await plugin.start(ctx); + + const repo = (plugin as any).repository as { + put(ref: unknown, spec: unknown, opts: unknown): Promise<{ version: string }>; + }; + expect(repo).toBeDefined(); + + try { + await repo.put( + { org: 'system', type: 'view', name: 'case_grid' }, + { label: 'Cases' }, + { parentVersion: null, actor: 'tester' }, + ); + expect(existsSync(join(dir, '.objectstack', 'metadata', 'view', 'case_grid.json'))).toBe(true); + expect(existsSync(join(dir, '.objectstack', 'metadata', '.objectstack', '.log', 'main.jsonl'))).toBe(true); + } finally { + await plugin.stop(ctx); + } + }); +}); From 38b11183278c12977cf8c5f5e5853e27c5c1ea27 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 01:12:20 +0000 Subject: [PATCH 2/2] test(metadata): use an explicit .js extension so the pin adds no type-check debt --- packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts b/packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts index 9cfbc77633..ff75a1bb28 100644 --- a/packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts +++ b/packages/metadata/src/plugin-no-metadata-root-on-boot.test.ts @@ -24,7 +24,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { mkdtempSync, rmSync, existsSync, readdirSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { MetadataPlugin } from './plugin'; +import { MetadataPlugin } from './plugin.js'; vi.mock('@objectstack/core', async (orig) => { const real = (await orig()) as any;