diff --git a/.agents/skills/proof/reference.md b/.agents/skills/proof/reference.md index 7ebeb271..f1842a44 100644 --- a/.agents/skills/proof/reference.md +++ b/.agents/skills/proof/reference.md @@ -142,7 +142,11 @@ flatbread proof cache prune `--since`/`--until` bound `created_at` (gte/lte, ISO strings). - `--relations` values: `derives_from`, `supersedes`, `superseded_by`, `invalidates`, `invalidated_by`, `rejected_by`, `mitigated_by`, - `resolved_by`, `evidence`, `cites` (one hop, explicit only). + `resolved_by`, `evidence`, `cites` (one hop, explicit only). Missing targets + never produce a partial success. For `derives_from`, `invalidates`, + `invalidated_by`, `resolved_by`, and `evidence`, the CLI returns + `PROOF_DANGLING_RELATION`. Flatbread's core reference check rejects missing + targets for the other relations. - `--resolve head`: follow `superseded_by` to the current tip; ancestors render as checkpoint lines (max 5, then a count). - `blocking-decisions` membership (frozen): Decision in the effort with @@ -157,7 +161,8 @@ flatbread proof cache prune generation, or fail. `--timeout-ms ` bounds the wait (default 3000). - Errors (stderr JSON, exit 1): `PROOF_GENERATION_WAIT_TIMEOUT`, `PROOF_INVALID_CURSOR` (cursor reused across a different query or - generation). + generation), and `PROOF_DANGLING_RELATION` (a stored relation target is + missing). ## Configuration surface diff --git a/CHANGELOG.md b/CHANGELOG.md index 6470ecc8..705d05ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,26 @@ ## Unreleased - The DAG runner is now `@flatbread/oven` (`pnpm exec oven`); the memory package is now `@flatbread/proof` with the `flatbread proof` CLI. +- `@flatbread/source-filesystem` reads a content directory that does not exist + as an empty collection instead of throwing `ENOENT`. Git cannot store an + empty directory, and a Proof write creates only the directory it writes, so + a sparse graph was unreadable until every collection directory existed. + Permission and other I/O faults still fail the read. + A failing read now also reports through the normal error path: `fetch` no + longer builds its results in an `async` promise executor, which used to + leave the caller waiting forever while the failure escaped as an unhandled + rejection and a raw stack trace. +- **Breaking for writes:** a Proof create mutation now rejects a + `derives_from` id that no record answers to, with + `Unknown artifact `. This matches the documented contract for forward + edges and the existing behavior of `cites`, `supersedes`, and `invalidates`. + Create the target record first, then link to it. +- `flatbread proof relations` fails with `PROOF_DANGLING_RELATION` (stderr + JSON, exit 1) when a record stores a `derives_from` id that no record answers + to. It used to drop the edge and report `page.returned: 0`, which presented + incomplete provenance as complete. The error names the record, the relation, + and the missing id. Records written before this release keep any dangling + edge until you repair the file. Notes for the Flatbread release train. Some packages also keep their own changelog; this file covers the repository as a whole. diff --git a/packages/flatbread/src/cli/proof.test.ts b/packages/flatbread/src/cli/proof.test.ts index 9b297d82..e7795792 100644 --- a/packages/flatbread/src/cli/proof.test.ts +++ b/packages/flatbread/src/cli/proof.test.ts @@ -15,7 +15,11 @@ import { join, relative } from 'node:path'; import { tmpdir } from 'node:os'; import { proofContent } from '@flatbread/proof'; import { proofContent as publicProofContent } from '../index.js'; -import { ProofValidationError } from '@flatbread/proof'; +import { + ProofDanglingRelationError, + ProofValidationError, + serializeDocument, +} from '@flatbread/proof'; import { handleEffortBlockingDecisions, handleEffortBootstrap, @@ -653,6 +657,177 @@ export default { } ); +test.serial( + 'a strict read of a sparse Proof succeeds without pre-created collection directories', + async (t) => { + const cwd = await createTempProject('flatbread-effort-sparse-', t); + await writeFile( + join(cwd, 'flatbread.config.js'), + `import { source } from '@flatbread/source-filesystem'; +import { transformer } from '@flatbread/transformer-markdown'; +import { proofContent } from '@flatbread/proof'; +export default { + source: source(), + transformer: transformer(), + content: proofContent('.flatbread-proof'), +};` + ); + const effort = await handleEffortWrite( + JSON.stringify({ + type: 'CreateEffort', + title: 'Critical path', + body: 'Exercise storage and consistency.', + }), + { cwd } + ); + const effortId = effort.artifacts[0].id; + const issue = await handleEffortWrite( + JSON.stringify({ + type: 'WriteIssue', + effort: effortId, + title: 'Need a decision', + body: 'A blocker for the strict-read path.', + kind: 'blocker', + }), + { cwd } + ); + + // Writes create only the directories they touch, and Git cannot store an + // empty directory, so a fresh clone always reads a sparse graph. + t.is( + await lstat(join(cwd, '.flatbread-proof', 'findings')).catch(() => null), + null + ); + + const result = await runCli( + cwd, + 'proof', + 'records', + effortId, + '--kinds', + 'issue', + '--strict-min-generation', + issue.generation, + '--timeout-ms', + '3000' + ); + t.is(result.code, 0); + t.is(result.stderr, ''); + t.is(JSON.parse(result.stdout).page.returned, 1); + } +); + +test.serial( + 'a dangling derives_from is rejected on write and reported on read', + async (t) => { + const cwd = await createTempProject('flatbread-effort-dangling-', t); + // Create every collection directory so this case turns only on relation + // integrity, not on how a sparse graph reads. + for (const directory of [ + 'efforts', + 'issues', + 'findings', + 'decisions', + 'constraints', + 'risks', + 'citations', + 'blobs', + ]) + await mkdir(join(cwd, '.flatbread-proof', directory), { + recursive: true, + }); + await writeFile( + join(cwd, 'flatbread.config.js'), + `import { source } from '@flatbread/source-filesystem'; +import { transformer } from '@flatbread/transformer-markdown'; +import { proofContent } from '@flatbread/proof'; +export default { + source: source(), + transformer: transformer(), + content: proofContent('.flatbread-proof'), +};` + ); + const effort = await handleEffortWrite( + JSON.stringify({ type: 'CreateEffort', title: 'Provenance', body: '' }), + { cwd } + ); + const effortId = effort.artifacts[0].id; + const missingId = 'fnd-does-not-exist--0000000000000000'; + + await t.throwsAsync( + () => + handleEffortWrite( + JSON.stringify({ + type: 'WriteDecision', + effort: effortId, + title: 'Dangling derives-from', + body: 'This must have failed closed.', + derives_from: [missingId], + }), + { cwd } + ), + { + instanceOf: ProofValidationError, + message: new RegExp(`Unknown artifact ${missingId}`), + } + ); + // The rejected write leaves the durable generation where the Effort left it. + t.is( + JSON.parse( + await readFile( + join(cwd, '.flatbread-proof', '.journal', 'generation.json'), + 'utf8' + ) + ).generation, + Number(effort.generation) + ); + + // Legacy or hand-edited data can still hold a dangling edge. Recall must say + // so rather than return provenance that dropped the edge in silence. + const decisionId = 'dec-legacy--0000000000000000'; + await writeFile( + join(cwd, '.flatbread-proof', 'decisions', `${decisionId}.md`), + serializeDocument('Written before its evidence existed.', { + id: decisionId, + effort: effortId, + title: 'Legacy decision', + created_at: '2025-01-01T00:00:00.000Z', + state: 'proposed', + derives_from: [missingId], + }) + ); + const error = await t.throwsAsync( + () => + handleEffortRelations(effortId, decisionId, { + cwd, + relations: ['derives_from'], + }), + { instanceOf: ProofDanglingRelationError } + ); + t.deepEqual(error?.shape, { + error: { + code: 'PROOF_DANGLING_RELATION', + message: `Record ${decisionId} stores relation targets that do not exist: derives_from -> ${missingId}`, + from_id: decisionId, + edges: [{ relation: 'derives_from', to_id: missingId }], + }, + }); + const result = await runCli( + cwd, + 'proof', + 'relations', + effortId, + decisionId, + '--relations', + 'derives_from' + ); + t.is(result.code, 1); + t.is(result.stdout, ''); + t.deepEqual(JSON.parse(result.stderr), error?.shape); + t.false(result.stderr.includes(' at ')); + } +); + test.serial( 'bootstrap detects a ready custom root and verify returns action-required JSON state', async (t) => { diff --git a/packages/flatbread/src/proof/read.ts b/packages/flatbread/src/proof/read.ts index f19646f7..9d68edc0 100644 --- a/packages/flatbread/src/proof/read.ts +++ b/packages/flatbread/src/proof/read.ts @@ -2,6 +2,7 @@ import { FlatbreadProvider, type LoadedFlatbreadConfig } from '@flatbread/core'; import { canonicalizeReadQuery, ProofConsistencyError, + ProofDanglingRelationError, ProofInvalidCursorError, ProofReadValidationError, READ_RELATIONS, @@ -663,17 +664,26 @@ export async function relations( if (!source || !sourceInEffort) throw new Error(`Record ${fromId} does not exist in effort ${effortId}`); const selected = new Map(); + const dangling: { relation: ReadRelation; to_id: string }[] = []; for (const relation of relationNames) { for (const targetId of source.relations[relation] ?? []) { const targetCollection = collectionForId(targetId); - if (!targetCollection) continue; - const target = await projection.one(targetCollection, targetId); - // Only return targets that belong to this Effort. The writer should - // prevent foreign links, but this keeps hand-authored files contained. - if (target?.frontmatter.effort === effortId) + const target = targetCollection + ? await projection.one(targetCollection, targetId) + : undefined; + // A stored id that no record answers to means the graph is corrupt. Report + // it instead of returning provenance that dropped the edge in silence. + if (!target) { + dangling.push({ relation, to_id: targetId }); + continue; + } + // Keep the existing effort-scoped read behavior for legacy or + // hand-authored cross-effort links. + if (target.frontmatter.effort === effortId) selected.set(target.id, target); } } + if (dangling.length) throw new ProofDanglingRelationError(fromId, dangling); const records = sortRecords([...selected.values()]); const edges = records.flatMap((record) => relationNames diff --git a/packages/proof/skills/proof/reference.md b/packages/proof/skills/proof/reference.md index 7ebeb271..f1842a44 100644 --- a/packages/proof/skills/proof/reference.md +++ b/packages/proof/skills/proof/reference.md @@ -142,7 +142,11 @@ flatbread proof cache prune `--since`/`--until` bound `created_at` (gte/lte, ISO strings). - `--relations` values: `derives_from`, `supersedes`, `superseded_by`, `invalidates`, `invalidated_by`, `rejected_by`, `mitigated_by`, - `resolved_by`, `evidence`, `cites` (one hop, explicit only). + `resolved_by`, `evidence`, `cites` (one hop, explicit only). Missing targets + never produce a partial success. For `derives_from`, `invalidates`, + `invalidated_by`, `resolved_by`, and `evidence`, the CLI returns + `PROOF_DANGLING_RELATION`. Flatbread's core reference check rejects missing + targets for the other relations. - `--resolve head`: follow `superseded_by` to the current tip; ancestors render as checkpoint lines (max 5, then a count). - `blocking-decisions` membership (frozen): Decision in the effort with @@ -157,7 +161,8 @@ flatbread proof cache prune generation, or fail. `--timeout-ms ` bounds the wait (default 3000). - Errors (stderr JSON, exit 1): `PROOF_GENERATION_WAIT_TIMEOUT`, `PROOF_INVALID_CURSOR` (cursor reused across a different query or - generation). + generation), and `PROOF_DANGLING_RELATION` (a stored relation target is + missing). ## Configuration surface diff --git a/packages/proof/src/__tests__/planner.test.ts b/packages/proof/src/__tests__/planner.test.ts index 92f7370d..b78c7201 100644 --- a/packages/proof/src/__tests__/planner.test.ts +++ b/packages/proof/src/__tests__/planner.test.ts @@ -768,3 +768,54 @@ test('29 same-effort cites and blob happy path', (t) => { citId, ]); }); +test('derives_from must target an existing record', (t) => { + const missing = 'fnd-does-not-exist--0000000000000000'; + t.throws( + () => + planMutation( + { + type: 'WriteDecision', + id: ids.decision, + effort: E, + title: 'D', + body: '', + derives_from: [missing], + }, + snap(), + '/root', + now + ), + { message: new RegExp(`Unknown artifact ${missing}`) } + ); +}); +test('derives_from accepts an existing record', (t) => { + const issue = record(ids.issue, 'issue', { + id: ids.issue, + effort: E, + title: 'I', + kind: 'blocker', + created_at: '2025-01-01T00:00:00.000Z', + status: 'open', + }); + const w = planMutation( + { + type: 'WriteDecision', + id: ids.decision, + effort: E, + title: 'D', + body: '', + derives_from: [ids.issue], + }, + snap([issue]), + '/root', + now + ); + one(t, w, ids.decision, `decisions/${ids.decision}.md`, 'create', { + id: ids.decision, + effort: E, + title: 'D', + derives_from: [ids.issue], + created_at: now.toISOString(), + state: 'proposed', + }); +}); diff --git a/packages/proof/src/index.ts b/packages/proof/src/index.ts index d1cd3583..eaf39c5e 100644 --- a/packages/proof/src/index.ts +++ b/packages/proof/src/index.ts @@ -16,6 +16,7 @@ export * from './digest.js'; export { READ_RELATIONS, ProofConsistencyError, + ProofDanglingRelationError, ProofInvalidCursorError, ProofReadValidationError, canonicalizeReadQuery, diff --git a/packages/proof/src/planner.ts b/packages/proof/src/planner.ts index d37b9f24..aea65701 100644 --- a/packages/proof/src/planner.ts +++ b/packages/proof/src/planner.ts @@ -70,6 +70,19 @@ function assertCites( } } +/** + * `derives_from` is the one forward edge with no reverse projection, so nothing + * else in the create path ever looks its targets up. Resolve each one here: + * `get` throws `Unknown artifact ` for a missing target, which keeps a + * record that points at nothing from ever reaching the journal. + */ +function assertDerivesFrom( + get: (id: string) => NonNullable>, + derivesFrom: string[] | undefined +): void { + for (const targetId of derivesFrom ?? []) get(targetId); +} + export function planMutation( input: ProofMutation, snapshot: ProofSnapshot, @@ -161,6 +174,7 @@ export function planMutation( created_at?: string; blob?: string; cites?: string[]; + derives_from?: string[]; }; if (kind === 'blob' || kind === 'citation') assertNoCitationBlobEdges(input.type, raw); @@ -181,7 +195,10 @@ export function planMutation( `Citation.blob ${raw.blob} belongs to a different effort` ); } - if (EPISTEMIC_CREATE.has(kind)) assertCites(get, raw.effort, raw.cites); + if (EPISTEMIC_CREATE.has(kind)) { + assertCites(get, raw.effort, raw.cites); + assertDerivesFrom(get, raw.derives_from); + } const fm: Record = { ...raw, id, diff --git a/packages/proof/src/read.ts b/packages/proof/src/read.ts index 703cbb4b..c08a4443 100644 --- a/packages/proof/src/read.ts +++ b/packages/proof/src/read.ts @@ -90,6 +90,38 @@ export class ProofInvalidCursorError extends Error { } } +/** + * Raised when a record stores a relation id that no record answers to. Relation + * recall fails closed instead of handing back provenance that quietly lost an + * edge, and the message names the record, the relation, and the missing id so a + * reader can repair the file. + */ +export class ProofDanglingRelationError extends Error { + readonly shape: { + error: { + code: 'PROOF_DANGLING_RELATION'; + message: string; + from_id: string; + edges: { relation: string; to_id: string }[]; + }; + }; + constructor(fromId: string, edges: { relation: string; to_id: string }[]) { + const message = `Record ${fromId} stores relation targets that do not exist: ${edges + .map((edge) => `${edge.relation} -> ${edge.to_id}`) + .join(', ')}`; + super(message); + this.name = 'ProofDanglingRelationError'; + this.shape = { + error: { + code: 'PROOF_DANGLING_RELATION', + message, + from_id: fromId, + edges, + }, + }; + } +} + export class ProofConsistencyError extends Error { readonly shape: ConsistencyErrorShape; constructor(shape: ConsistencyErrorShape) { diff --git a/packages/source-filesystem/src/index.ts b/packages/source-filesystem/src/index.ts index 965bad2b..133360fa 100644 --- a/packages/source-filesystem/src/index.ts +++ b/packages/source-filesystem/src/index.ts @@ -44,23 +44,22 @@ async function getAllNodes( allContentTypes: Record[], config: InitializedSourceFilesystemConfig ): Promise> { + /** + * Build each entry with a plain `async` callback. An `async` executor passed + * to `new Promise` loses a rejection: the outer promise never settles, so a + * caller waits forever and the failure escapes as an unhandled rejection + * instead of an error it can report. + */ const nodeEntries = await Promise.all( allContentTypes.map( - async (contentType): Promise> => - new Promise(async (res) => - res([ - contentType.collection, - await getNodesFromDirectory(contentType.path, config), - ]) - ) + async (contentType): Promise => [ + contentType.collection, + await getNodesFromDirectory(contentType.path, config), + ] ) ); - const nodes = Object.fromEntries( - nodeEntries as Iterable - ); - - return nodes; + return Object.fromEntries(nodeEntries); } async function getNodesFromPaths( diff --git a/packages/source-filesystem/src/utils/gatherFileNodes.ts b/packages/source-filesystem/src/utils/gatherFileNodes.ts index 8f880636..c9883e3d 100644 --- a/packages/source-filesystem/src/utils/gatherFileNodes.ts +++ b/packages/source-filesystem/src/utils/gatherFileNodes.ts @@ -5,7 +5,19 @@ import type { FileNode, GatherFileNodesOptions } from '../types'; type Segment = { name: string; remove: number } | null; async function readDir(path: string): Promise { - const files = (await readdir(path, { withFileTypes: true })) as FileNode[]; + let files: FileNode[]; + try { + files = (await readdir(path, { withFileTypes: true })) as FileNode[]; + } catch (error) { + /** + * A content directory that nothing has written yet is an empty collection, + * not a failure. Git cannot store an empty directory, so a fresh clone of a + * sparse project has none of them. Permission and I/O faults still throw. + */ + const code = (error as NodeJS.ErrnoException).code; + if (code === 'ENOENT') return []; + throw error; + } return files.map((file) => { file.path = join(path, file.name); diff --git a/packages/source-filesystem/src/utils/tests/fetch.test.ts b/packages/source-filesystem/src/utils/tests/fetch.test.ts new file mode 100644 index 00000000..215e1552 --- /dev/null +++ b/packages/source-filesystem/src/utils/tests/fetch.test.ts @@ -0,0 +1,48 @@ +import test from 'ava'; +import type { LoadedFlatbreadConfig } from '@flatbread/core'; +import source from '../../index'; + +const MISSING_DIRECTORY = + 'packages/source-filesystem/src/utils/tests/fixtures/never-written'; +const CAPTURE_PATTERN = + 'packages/source-filesystem/src/utils/tests/fixtures/captures/[category]/[slug].md'; +const CONTENT_FILE = + 'packages/source-filesystem/src/utils/tests/fixtures/captures/news/hello.md'; + +function initializedSource(content: { path: string; collection: string }[]) { + const plugin = source(); + plugin.initialize?.({ + content, + loaded: { extensions: ['.md'] }, + } as unknown as LoadedFlatbreadConfig); + return plugin; +} + +test('fetch returns an empty collection for a directory nothing has written', async (t) => { + const content = [{ path: MISSING_DIRECTORY, collection: 'NeverWritten' }]; + const plugin = initializedSource(content); + + t.deepEqual(await plugin.fetch(content), { NeverWritten: [] }); +}); + +test('fetch still reads the collections that exist beside a missing one', async (t) => { + const content = [ + { path: MISSING_DIRECTORY, collection: 'NeverWritten' }, + { path: CAPTURE_PATTERN, collection: 'CaptureDoc' }, + ]; + const plugin = initializedSource(content); + + const nodes = await plugin.fetch(content); + + t.deepEqual(nodes.NeverWritten, []); + t.is(nodes.CaptureDoc.length, 2); +}); + +test('fetch rejects when a configured directory path is a file', async (t) => { + const content = [{ path: CONTENT_FILE, collection: 'NotADirectory' }]; + const plugin = initializedSource(content); + + const error = await t.throwsAsync(plugin.fetch(content)); + + t.is((error as NodeJS.ErrnoException | undefined)?.code, 'ENOTDIR'); +}); diff --git a/packages/source-filesystem/src/utils/tests/gatherFileNodes.test.ts b/packages/source-filesystem/src/utils/tests/gatherFileNodes.test.ts index e9b2699c..aa0018f3 100644 --- a/packages/source-filesystem/src/utils/tests/gatherFileNodes.test.ts +++ b/packages/source-filesystem/src/utils/tests/gatherFileNodes.test.ts @@ -71,3 +71,13 @@ test('triple level', async (t) => { const result = await gatherFileNodes('./[random]/[name]/[title].md', opts); t.snapshot(result); }); + +// Uses the real directory reader, not the mock: a content directory nobody has +// written yet must read as an empty collection instead of throwing ENOENT. +test('a missing directory reads as an empty collection', async (t) => { + const missing = + 'packages/source-filesystem/src/utils/tests/fixtures/never-written'; + + t.deepEqual(await gatherFileNodes(missing), []); + t.deepEqual(await gatherFileNodes(`${missing}/[category]/[slug].md`), []); +});