Skip to content

Commit a1b66ef

Browse files
os-zhuangclaude
andauthored
fix(metadata-fs): attaching a FileSystemRepository no longer creates its root (#7000) (#7152)
* fix(metadata-fs): attaching a FileSystemRepository no longer creates its root (#7000) * test(metadata): use an explicit .js extension so the pin adds no type-check debt --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6146b67 commit a1b66ef

5 files changed

Lines changed: 400 additions & 8 deletions

File tree

.changeset/gentle-pears-attach.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/metadata-fs": patch
3+
---
4+
5+
`FileSystemRepository` no longer creates its root directory when it is attached — only when it first writes.
6+
7+
`start()` used to `mkdir` both `<root>` and `<root>/.objectstack/.log` unconditionally, so merely attaching a repository was a write. Because `MetadataPlugin` attaches one at `<project>/.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
8+
9+
```
10+
.objectstack/metadata/.objectstack/.log
11+
```
12+
13+
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.
14+
15+
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.
16+
17+
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()`.

packages/metadata-fs/README.md

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const repo = new FileSystemRepository({
3535
root: './metadata',
3636
org: 'system',
3737
});
38-
await repo.start(); // scan + open watcher
38+
await repo.start(); // scan + open watcher — creates nothing on disk
3939

4040
const view = await repo.get({
4141
org: 'system',
@@ -47,4 +47,19 @@ for await (const evt of repo.watch({})) {
4747
}
4848
```
4949

50+
## Root creation is a write, not an attach
51+
52+
`start()` never creates `<root>`. Attaching a repository whose root does not
53+
exist is legal: reads answer as if the repository were empty, and the root —
54+
together with `<root>/.objectstack/.log/` — appears on the **first write**
55+
(`put` / `delete`). This is what keeps a read-only boot, such as the dry run
56+
`os migrate plan` performs on a project that has never been started, from
57+
leaving a directory skeleton behind (#7000, the filesystem half of #6743).
58+
59+
One consequence worth knowing: when the root is absent at `start()`, the
60+
chokidar watcher is armed by the first write instead, because chokidar cannot
61+
watch a path that does not exist yet. A root brought into existence by a third
62+
party while the process runs, without this repository ever writing, is
63+
therefore not picked up until the next `start()`.
64+
5065
See ADR-0008 (incl. §0 amendment) and the `metadata-branch-removal` changeset.

packages/metadata-fs/src/repository.ts

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
* log is a denormalised history index.
1717
* - chokidar-driven external edits are translated into MetadataEvents
1818
* by hashing the new content and comparing to the last-known hash.
19+
* - The root directory is created **on the first write, not on attach**
20+
* (#7000). Attaching and reading a repository whose root does not exist
21+
* is legal and answers "empty"; see `start()` / `ensureRoot()`.
1922
*/
2023

2124
import fs from 'node:fs/promises';
@@ -46,7 +49,6 @@ import {
4649
itemPath,
4750
parseItemPath,
4851
typeDir,
49-
logDir,
5052
logFile,
5153
} from './layout.js';
5254
import { JsonlLog } from './jsonl-log.js';
@@ -108,21 +110,55 @@ export class FileSystemRepository implements MetadataRepository {
108110

109111
// ── Lifecycle ───────────────────────────────────────────────────────
110112

113+
/**
114+
* Attach the repository. **Creates nothing on disk** (#7000).
115+
*
116+
* Attaching is not a write. `start()` used to `mkdir` both the root and
117+
* `<root>/.objectstack/.log` unconditionally, which meant every read-only
118+
* boot that merely attaches a repository left a skeleton behind — most
119+
* visibly `os migrate plan`, a declared dry run, on a project that has
120+
* never been started. That is the same property #6743 ruled on for
121+
* `.objectstack/data/`: a dry run leaves nothing behind, and the existence
122+
* of `.objectstack/` has to stay a usable "this project has been started"
123+
* signal.
124+
*
125+
* Every read path below already treats a missing root as an empty
126+
* repository (`scanHeads` swallows ENOENT, `JsonlLog` guards on
127+
* `existsSync`, `get` guards on `existsSync`), so the root is materialized
128+
* by `ensureRoot()` on the first write instead.
129+
*/
111130
async start(): Promise<void> {
112131
if (this.started) return;
113132
this.started = true;
114-
await fs.mkdir(this.layout.root, { recursive: true });
115-
await fs.mkdir(logDir(this.layout), { recursive: true });
116133

117-
// 1) Scan body files to build the head index.
134+
// 1) Scan body files to build the head index. No-op on a missing root.
118135
await this.scanHeads();
119136

120-
// 2) Hydrate nextSeq from the existing log.
137+
// 2) Hydrate nextSeq from the existing log. No-op on a missing log.
121138
const highest = await this.log.highestSeq();
122139
this.nextSeq = highest + 1;
123140

124-
// 3) Start the watcher (unless disabled).
125-
if (!this.disableWatch) this.startWatcher();
141+
// 3) Start the watcher (unless disabled). chokidar cannot watch a path
142+
// that does not exist yet: measured on chokidar 5 with `usePolling`,
143+
// a root created AFTER `watch()` produces no events at all, ever. So
144+
// when the root is absent the watcher is armed later, by the
145+
// `ensureRoot()` call that brings the root into existence — otherwise
146+
// dropping the `mkdir` above would silently kill external-edit
147+
// detection for the whole life of the process.
148+
if (!this.disableWatch && existsSync(this.layout.root)) this.startWatcher();
149+
}
150+
151+
/**
152+
* Bring the repository root into existence. Called by every write path
153+
* immediately before it touches the disk — `start()` deliberately does not
154+
* create it (#7000), so this is the single seam where the root appears.
155+
*
156+
* It is also where a watcher that `start()` could not arm (missing root)
157+
* gets armed, so "external edits are detected" survives the change.
158+
*/
159+
private async ensureRoot(): Promise<void> {
160+
await fs.mkdir(this.layout.root, { recursive: true });
161+
if (this.started && !this.disableWatch && !this.watcher) this.startWatcher();
126162
}
127163

128164
async close(): Promise<void> {
@@ -258,6 +294,8 @@ export class FileSystemRepository implements MetadataRepository {
258294
const seq = this.nextSeq++;
259295
const ts = this.now().toISOString();
260296
const file = itemPath(this.layout, ref.type, ref.name);
297+
// First write of the process materializes the root (#7000).
298+
await this.ensureRoot();
261299
await fs.mkdir(typeDir(this.layout, ref.type), { recursive: true });
262300
this.selfWrites.add(file);
263301
try {
@@ -309,6 +347,8 @@ export class FileSystemRepository implements MetadataRepository {
309347
throw new ConflictError(ref, opts.parentVersion, currentHead);
310348
}
311349
const file = itemPath(this.layout, ref.type, ref.name);
350+
// A delete appends a tombstone to the change log, so it is a write too.
351+
await this.ensureRoot();
312352
this.selfWrites.add(file);
313353
try {
314354
if (existsSync(file)) await fs.unlink(file);
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #7000 — attaching a `FileSystemRepository` must leave nothing on disk.
5+
*
6+
* This is the filesystem half of the property #6743 ruled on: a dry run
7+
* leaves nothing behind. #6743 / PR #6997 covered `.objectstack/data/` (the
8+
* sqlite file the driver brought into existence at open); the half left over
9+
* is the metadata repository, whose `start()` used to `mkdir` both the root
10+
* and `<root>/.objectstack/.log` unconditionally. `os migrate plan` on a
11+
* project that has never been started therefore left a
12+
* `.objectstack/metadata/.objectstack/.log` skeleton behind, and the presence
13+
* of `.objectstack/` stopped being a usable "this project has been started"
14+
* signal.
15+
*
16+
* The pin is written the same way #6743's is: assert the ABSENCE of the side
17+
* effect on a fresh fixture, and carry a control that proves the fixture is
18+
* genuine — here, that the very same repository is attachable, readable and
19+
* writable, so the absence is not the uninteresting kind produced by a boot
20+
* that failed early.
21+
*/
22+
23+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
24+
import fs from 'node:fs/promises';
25+
import { existsSync, readdirSync } from 'node:fs';
26+
import path from 'node:path';
27+
import os from 'node:os';
28+
import type { MetaRef, MetadataEvent, MetadataItemHeader } from '@objectstack/metadata-core';
29+
import { FileSystemRepository } from '../src/index.js';
30+
31+
const ref = (name: string): MetaRef => ({ org: 'system', type: 'view', name });
32+
const sleep = (ms: number) => new Promise<void>((r) => setTimeout(r, ms));
33+
34+
describe('FileSystemRepository — attaching creates nothing on disk (#7000)', () => {
35+
/** Stands in for the project directory: nothing below it exists yet. */
36+
let base: string;
37+
/** Stands in for `<project>/.objectstack/metadata` — deliberately absent. */
38+
let root: string;
39+
let repo: FileSystemRepository | undefined;
40+
41+
beforeEach(async () => {
42+
base = await fs.mkdtemp(path.join(os.tmpdir(), 'objectstack-fs7000-'));
43+
root = path.join(base, '.objectstack', 'metadata');
44+
});
45+
46+
afterEach(async () => {
47+
if (repo) await repo.close().catch(() => undefined);
48+
repo = undefined;
49+
await fs.rm(base, { recursive: true, force: true });
50+
});
51+
52+
/**
53+
* Every path anywhere below the project directory. Naming only the two
54+
* paths the old `mkdir`s created would miss a third one added later, so the
55+
* pin sweeps the whole fixture instead.
56+
*/
57+
function residueOnDisk(): string[] {
58+
const found: string[] = [];
59+
const walk = (dir: string, depth: number): void => {
60+
if (depth > 5) return;
61+
for (const entry of readdirSync(dir, { withFileTypes: true })) {
62+
const full = path.join(dir, entry.name);
63+
found.push(path.relative(base, full));
64+
if (entry.isDirectory()) walk(full, depth + 1);
65+
}
66+
};
67+
walk(base, 0);
68+
return found.sort();
69+
}
70+
71+
it('start() on a never-started project creates neither the root nor .objectstack/.log', async () => {
72+
expect(existsSync(root)).toBe(false);
73+
74+
repo = new FileSystemRepository({ root, org: 'system', disableWatch: true });
75+
await repo.start();
76+
77+
expect(existsSync(root)).toBe(false);
78+
expect(existsSync(path.join(root, '.objectstack', '.log'))).toBe(false);
79+
expect(residueOnDisk()).toEqual([]);
80+
81+
// Detach is not a write either.
82+
await repo.close();
83+
expect(residueOnDisk()).toEqual([]);
84+
});
85+
86+
it('an absent root is attachable and reads as an empty repository', async () => {
87+
repo = new FileSystemRepository({ root, org: 'system', disableWatch: true });
88+
await repo.start();
89+
90+
expect(await repo.get(ref('case_grid'))).toBeNull();
91+
expect(await repo.getByHash(ref('case_grid'), 'not-a-real-hash')).toBeNull();
92+
93+
const listed: MetadataItemHeader[] = [];
94+
for await (const header of repo.list({ org: 'system' })) listed.push(header);
95+
expect(listed).toEqual([]);
96+
97+
const past: MetadataEvent[] = [];
98+
for await (const evt of repo.history(ref('case_grid'))) past.push(evt);
99+
expect(past).toEqual([]);
100+
101+
// No read may be paid for with a directory.
102+
expect(residueOnDisk()).toEqual([]);
103+
});
104+
105+
it('the FIRST write materializes the root, the body file and the change log', async () => {
106+
repo = new FileSystemRepository({ root, org: 'system', disableWatch: true });
107+
await repo.start();
108+
expect(existsSync(root)).toBe(false);
109+
110+
await repo.put(ref('case_grid'), { label: 'Cases' }, { parentVersion: null, actor: 'tester' });
111+
112+
expect(existsSync(root)).toBe(true);
113+
expect(existsSync(path.join(root, 'view', 'case_grid.json'))).toBe(true);
114+
expect(existsSync(path.join(root, '.objectstack', '.log', 'main.jsonl'))).toBe(true);
115+
116+
// The deferred root is a real repository, not a half-built one: a fresh
117+
// attach over the same path reads the item and its log back.
118+
await repo.close();
119+
repo = new FileSystemRepository({ root, org: 'system', disableWatch: true });
120+
await repo.start();
121+
const item = await repo.get(ref('case_grid'));
122+
expect(item?.body).toEqual({ label: 'Cases' });
123+
expect(item?.seq).toBe(1);
124+
125+
// And seq numbering continues from the log the deferred root wrote.
126+
const second = await repo.put(
127+
ref('case_grid'),
128+
{ label: 'Cases (renamed)' },
129+
{ parentVersion: item!.hash, actor: 'tester' },
130+
);
131+
expect(second.seq).toBe(2);
132+
});
133+
134+
it('arms the watcher on the first write, so external edits are still detected', async () => {
135+
// The one property that dropping the `mkdir` could have silently killed:
136+
// chokidar cannot watch a path that does not exist yet, so a watcher armed
137+
// by `start()` against an absent root would never fire again. `ensureRoot`
138+
// arms it at the moment the root appears — this is that guard.
139+
//
140+
// The root here is deliberately NOT under a dot-directory. The watcher's
141+
// own `ignored: [/(^|[\\/])\../]` matches ANY dot segment of the path,
142+
// including segments of the watched root itself, so a root such as the
143+
// plugin's `<project>/.objectstack/metadata` is ignored wholesale and
144+
// chokidar watches nothing at all. That is pre-existing on `origin/main`
145+
// and out of scope here (filed separately); this case pins the arming, so
146+
// it uses a root the watcher can actually see.
147+
const plainRoot = path.join(base, 'project', 'metadata');
148+
repo = new FileSystemRepository({ root: plainRoot, org: 'system' }); // watcher ENABLED
149+
await repo.start();
150+
expect(existsSync(plainRoot)).toBe(false);
151+
152+
await repo.put(ref('externally_edited'), { label: 'original' }, { parentVersion: null, actor: 'tester' });
153+
expect(existsSync(plainRoot)).toBe(true);
154+
155+
const iter = repo.watch({ org: 'system' }, 999)[Symbol.asyncIterator]();
156+
const collected: MetadataEvent[] = [];
157+
const collector = (async () => {
158+
const r = await iter.next();
159+
if (!r.done) collected.push(r.value as MetadataEvent);
160+
})();
161+
162+
// Past the 200ms self-write suppression window of the put above.
163+
await sleep(400);
164+
await fs.writeFile(
165+
path.join(plainRoot, 'view', 'externally_edited.json'),
166+
JSON.stringify({ label: 'externally edited' }, null, 2),
167+
);
168+
169+
await Promise.race([collector, sleep(8000)]);
170+
await iter.return?.(undefined);
171+
172+
expect(collected).toHaveLength(1);
173+
expect(collected[0]!.op).toBe('update');
174+
expect(collected[0]!.source).toBe('fs');
175+
expect(collected[0]!.actor).toBe('fs');
176+
}, 20_000);
177+
});

0 commit comments

Comments
 (0)