diff --git a/.agents/skills/effort-modeling/SKILL.md b/.agents/skills/effort-modeling/SKILL.md index 669faf88..fad4dba7 100644 --- a/.agents/skills/effort-modeling/SKILL.md +++ b/.agents/skills/effort-modeling/SKILL.md @@ -58,7 +58,7 @@ outside sources. ### External evidence (URL or longform) -When a Finding, Decision, Constraint, or Risk rests on an external source, +When a Finding, Issue, Decision, Constraint, or Risk rests on an external source, create a Citation instead of copying the source into the record body: 1. **`WriteBlob`** — only when you need to save large or non-text content @@ -66,8 +66,8 @@ create a Citation instead of copying the source into the record body: 2. **`WriteCitation`** — body is often the URL string; pass optional `blob` and `role` when needed. 3. **Create the record** — pass `cites: [""]` when creating the - Finding, Decision, Constraint, or Risk. You cannot add a citation later, - so create the Citation first. + Finding, Issue, Decision, Constraint, or Risk. You cannot add a citation + later, so create the Citation first. Record a Decision when it is hard to reverse, surprising without context, and the result of a real trade-off. Create it as proposed while the user is still diff --git a/.flatbread-efforts/findings/fnd-citation-and-blob-records-now-match-the-current--p6c37v29mx4jjfr2.md b/.flatbread-efforts/findings/fnd-citation-and-blob-records-now-match-the-current--p6c37v29mx4jjfr2.md index 17ea3b62..dafa4499 100644 --- a/.flatbread-efforts/findings/fnd-citation-and-blob-records-now-match-the-current--p6c37v29mx4jjfr2.md +++ b/.flatbread-efforts/findings/fnd-citation-and-blob-records-now-match-the-current--p6c37v29mx4jjfr2.md @@ -10,4 +10,4 @@ invalidates: - fnd-skill-and-hard-constraint-still-teach-13-mutatio--gvg2btns0q7rp0eq --- -The skills and related Decision, Issue, and Constraint now describe eight record types, fifteen mutations, and the Citation-to-optional-Blob path. Direct Blob citations are not supported. Cross-Effort `cites` and `Citation.blob` links are rejected. `CreateEffort` rejects `cites` when callers use the CLI or mutation schema; planner and writer enforcement remains a follow-up so programmatic callers cannot silently drop the field. This Finding replaces the prior audits and retrospective where their earlier claims no longer match the current contract. +The skills and related Decision, Issue, and Constraint now describe eight record types, fifteen mutations, and the Citation-to-optional-Blob path. Direct Blob citations are not supported. Cross-Effort `cites` and `Citation.blob` links are rejected. `CreateEffort` rejects `cites` through the CLI, mutation schema, planner, and writer, so callers cannot silently attach citations while creating an Effort. This Finding replaces the prior audits and retrospective where their earlier claims no longer match the current contract. diff --git a/packages/effort-graph/skills/effort-modeling/SKILL.md b/packages/effort-graph/skills/effort-modeling/SKILL.md index 669faf88..fad4dba7 100644 --- a/packages/effort-graph/skills/effort-modeling/SKILL.md +++ b/packages/effort-graph/skills/effort-modeling/SKILL.md @@ -58,7 +58,7 @@ outside sources. ### External evidence (URL or longform) -When a Finding, Decision, Constraint, or Risk rests on an external source, +When a Finding, Issue, Decision, Constraint, or Risk rests on an external source, create a Citation instead of copying the source into the record body: 1. **`WriteBlob`** — only when you need to save large or non-text content @@ -66,8 +66,8 @@ create a Citation instead of copying the source into the record body: 2. **`WriteCitation`** — body is often the URL string; pass optional `blob` and `role` when needed. 3. **Create the record** — pass `cites: [""]` when creating the - Finding, Decision, Constraint, or Risk. You cannot add a citation later, - so create the Citation first. + Finding, Issue, Decision, Constraint, or Risk. You cannot add a citation + later, so create the Citation first. Record a Decision when it is hard to reverse, surprising without context, and the result of a real trade-off. Create it as proposed while the user is still diff --git a/packages/effort-graph/src/__tests__/planner.test.ts b/packages/effort-graph/src/__tests__/planner.test.ts index 3db763dd..9e699168 100644 --- a/packages/effort-graph/src/__tests__/planner.test.ts +++ b/packages/effort-graph/src/__tests__/planner.test.ts @@ -526,6 +526,44 @@ test('23 WriteCitation with optional blob', (t) => { ); t.is(parseDocument(w[0].afterBytes, 'citation').frontmatter.blob, blobId); }); +test('WriteCitation rejects cites before planning a write', (t) => { + t.throws( + () => + planMutation( + { + type: 'WriteCitation', + id: 'cit-paper--0123456789abcdef', + effort: E, + title: 'Paper', + body: 'https://example.com/paper', + cites: ['cit-other--0123456789abcdef'], + } as unknown as import('../schemas.js').EffortGraphMutation, + snap(), + '/root', + now + ), + { message: /WriteCitation does not accept cites/ } + ); +}); +test('WriteBlob rejects cites before planning a write', (t) => { + t.throws( + () => + planMutation( + { + type: 'WriteBlob', + id: 'blb-payload--0123456789abcdef', + effort: E, + title: 'Payload', + body: '# longform research\n', + cites: ['cit-paper--0123456789abcdef'], + } as unknown as import('../schemas.js').EffortGraphMutation, + snap(), + '/root', + now + ), + { message: /WriteBlob does not accept cites/ } + ); +}); test('24 WriteFinding cites Citation', (t) => { const citId = 'cit-paper--0123456789abcdef'; const citation = record( diff --git a/packages/effort-graph/src/__tests__/schemas.test.ts b/packages/effort-graph/src/__tests__/schemas.test.ts index 0e6b7d92..1f3a94e5 100644 --- a/packages/effort-graph/src/__tests__/schemas.test.ts +++ b/packages/effort-graph/src/__tests__/schemas.test.ts @@ -152,7 +152,7 @@ test('rejects malformed ids', (t) => { ); }); -test('WriteCitation allows URL body without blob; cites accept Citation ids', (t) => { +test('WriteCitation allows URL body without blob; records cite Citation ids', (t) => { t.notThrows(() => EffortGraphMutationSchema.parse({ type: 'WriteCitation', @@ -193,6 +193,26 @@ test('CreateEffort rejects cites', (t) => { ); }); +test('WriteCitation and WriteBlob reject relation keys', (t) => { + for (const type of ['WriteCitation', 'WriteBlob'] as const) { + const result = EffortGraphMutationSchema.safeParse({ + ...validMutations[type], + cites: [`cit-paper--${suffix}`], + }); + t.false(result.success, type); + if (!result.success) + t.true( + result.error.issues.some( + (issue) => + issue.code === 'unrecognized_keys' && + issue.keys.includes('cites') && + issue.path.length === 0 + ), + type + ); + } +}); + test('frontmatter schemas passthrough unknown keys', (t) => { const parsed = EffortFrontmatterSchema.parse({ id: eff, diff --git a/packages/effort-graph/src/__tests__/writer.test.ts b/packages/effort-graph/src/__tests__/writer.test.ts index eada7e50..63a7870b 100644 --- a/packages/effort-graph/src/__tests__/writer.test.ts +++ b/packages/effort-graph/src/__tests__/writer.test.ts @@ -42,6 +42,39 @@ test('CreateEffort rejects cites through the writer', async (t) => { ); }); +test('WriteCitation and WriteBlob reject cites through the writer', async (t) => { + const { writer } = await makeWriter(); + const effort = soleId( + await writer.mutate({ type: 'CreateEffort', title: 'E', body: '' }) + ); + await t.throwsAsync( + writer.mutate({ + type: 'WriteCitation', + effort, + title: 'Paper', + body: 'https://example.com/paper', + cites: ['cit-other--0123456789abcdef'], + } as unknown as EffortGraphMutation), + { + instanceOf: EffortGraphValidationError, + message: /WriteCitation does not accept cites/, + } + ); + await t.throwsAsync( + writer.mutate({ + type: 'WriteBlob', + effort, + title: 'Payload', + body: '# longform research\n', + cites: ['cit-paper--0123456789abcdef'], + } as unknown as EffortGraphMutation), + { + instanceOf: EffortGraphValidationError, + message: /WriteBlob does not accept cites/, + } + ); +}); + test('WriteDecision with supersedes materializes superseded_by on the target file', async (t) => { const { root, writer } = await makeWriter(); const effort = soleId( diff --git a/packages/effort-graph/src/planner.ts b/packages/effort-graph/src/planner.ts index 6e2de8eb..0027abc8 100644 --- a/packages/effort-graph/src/planner.ts +++ b/packages/effort-graph/src/planner.ts @@ -32,6 +32,26 @@ const EPISTEMIC_CREATE = new Set([ 'risk', ]); +const CITATION_BLOB_FORBIDDEN_KEYS = [ + 'cites', + 'derives_from', + 'supersedes', + 'invalidates', +] as const; + +function assertNoCitationBlobEdges( + type: string, + input: Record +): void { + const present = CITATION_BLOB_FORBIDDEN_KEYS.filter((key) => key in input); + if (present.length) + throw new EffortGraphValidationError( + `${type} does not accept ${present.join( + ', ' + )}; relation fields are only valid on Issue, Finding, Decision, Constraint, or Risk records.` + ); +} + function assertCites( get: ( id: string @@ -140,7 +160,17 @@ export function planMutation( } if (input.type in kinds) { const kind = kinds[input.type]; - const raw = input as any; + const raw = input as Record & { + id?: string; + title: string; + body: string; + effort: string; + created_at?: string; + blob?: string; + cites?: string[]; + }; + if (kind === 'blob' || kind === 'citation') + assertNoCitationBlobEdges(input.type, raw); const id = raw.id ?? generateArtifactId(kind, raw.title, randomBytes); if (!validateArtifactId(id, kind) || snapshot.hasId(id)) throw new EffortGraphValidationError('Invalid or duplicate id'); diff --git a/packages/effort-graph/src/schemas.ts b/packages/effort-graph/src/schemas.ts index 69d8f49c..6062cc19 100644 --- a/packages/effort-graph/src/schemas.ts +++ b/packages/effort-graph/src/schemas.ts @@ -58,20 +58,24 @@ export const WriteRiskSchema = z.object({ likelihood: z.enum(['low', 'medium', 'high']), severity: z.enum(['low', 'medium', 'high']), }); -export const WriteCitationSchema = z.object({ - type: z.literal('WriteCitation'), - ...common, - effort: id, - /** Optional longform/payload target; body alone (e.g. a URL) is valid. */ - blob: id.optional(), - role: z.string().min(1).optional(), -}); -export const WriteBlobSchema = z.object({ - type: z.literal('WriteBlob'), - ...common, - effort: id, - kind: z.string().min(1).optional(), -}); +export const WriteCitationSchema = z + .object({ + type: z.literal('WriteCitation'), + ...common, + effort: id, + /** Optional longform/payload target; body alone (e.g. a URL) is valid. */ + blob: id.optional(), + role: z.string().min(1).optional(), + }) + .strict(); +export const WriteBlobSchema = z + .object({ + type: z.literal('WriteBlob'), + ...common, + effort: id, + kind: z.string().min(1).optional(), + }) + .strict(); export const SupersedeSchema = z.object({ type: z.literal('Supersede'), supersederId: id, diff --git a/packages/flatbread/src/cli/effort.test.ts b/packages/flatbread/src/cli/effort.test.ts index 9296627c..8a222af0 100644 --- a/packages/flatbread/src/cli/effort.test.ts +++ b/packages/flatbread/src/cli/effort.test.ts @@ -473,6 +473,20 @@ export default { { cwd } ); const effortId = effort.artifacts[0].id; + await t.throwsAsync( + () => + handleEffortWrite( + JSON.stringify({ + type: 'WriteCitation', + effort: effortId, + title: 'Invalid source', + body: 'https://example.com/invalid', + cites: ['cit-paper--0123456789abcdef'], + }), + { cwd } + ), + { message: /Unrecognized key\(s\) in object: 'cites'/ } + ); await t.throwsAsync( () => handleEffortWrite( @@ -586,6 +600,18 @@ export default { ), } ); + await t.throwsAsync( + () => + handleEffortRelations(effortId, foreignCitation.artifacts[0].id, { + cwd, + relations: ['cites'], + }), + { + message: new RegExp( + `Record ${foreignCitation.artifacts[0].id} does not exist in effort ${effortId}` + ), + } + ); const browse = await handleEffortRecords(effortId, { cwd }); const browseDigest = await readFile(browse.artifact_path, 'utf8'); diff --git a/packages/flatbread/src/effort/read.ts b/packages/flatbread/src/effort/read.ts index f5bd28d9..720dba8e 100644 --- a/packages/flatbread/src/effort/read.ts +++ b/packages/flatbread/src/effort/read.ts @@ -668,6 +668,8 @@ export async function relations( 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) selected.set(target.id, target); } diff --git a/packages/flatbread/src/graphql/liveServerEffortGraph.test.ts b/packages/flatbread/src/graphql/liveServerEffortGraph.test.ts index f971588e..c71db353 100644 --- a/packages/flatbread/src/graphql/liveServerEffortGraph.test.ts +++ b/packages/flatbread/src/graphql/liveServerEffortGraph.test.ts @@ -7,6 +7,7 @@ import markdownTransformer from '@flatbread/transformer-markdown'; import { initializeConfig } from '@flatbread/core'; import { effortGraphContent } from '@flatbread/effort-graph'; import type { ConfigResult, LoadedFlatbreadConfig } from '@flatbread/core'; +import type { EffortGraphMutation } from '@flatbread/effort-graph'; import { startGraphqlServer } from './liveServer.js'; async function makeDir() { @@ -101,12 +102,27 @@ test.serial( port: 0, }); t.teardown(() => server.close()); + await t.throwsAsync( + server.effortGraph!.writer.mutate({ + type: 'CreateEffort', + title: 'Invalid', + body: '', + cites: ['cit-paper--0123456789abcdef'], + } as unknown as EffortGraphMutation), + { message: /CreateEffort does not accept cites/ } + ); const effort = await server.effortGraph!.writer.mutate({ type: 'CreateEffort', title: 'GraphQL cite chain', body: '', }); const effortId = effort.artifacts[0].id; + const otherEffort = await server.effortGraph!.writer.mutate({ + type: 'CreateEffort', + title: 'Other chain', + body: '', + }); + const otherEffortId = otherEffort.artifacts[0].id; const blob = await server.effortGraph!.writer.mutate({ type: 'WriteBlob', effort: effortId, @@ -133,6 +149,64 @@ test.serial( { title: 'Paper', blob: { id: blobId, title: 'Payload' } }, ]); t.deepEqual(response.data?.allBlobs, [{ id: blobId, title: 'Payload' }]); + const foreignBlob = await server.effortGraph!.writer.mutate({ + type: 'WriteBlob', + effort: otherEffortId, + title: 'Foreign payload', + body: '', + }); + await t.throwsAsync( + server.effortGraph!.writer.mutate({ + type: 'WriteCitation', + effort: effortId, + title: 'Bad source', + body: 'https://example.com/bad-source', + blob: foreignBlob.artifacts[0].id, + }), + { + message: new RegExp( + `Citation\\.blob ${foreignBlob.artifacts[0].id} belongs to a different effort` + ), + } + ); + const foreignCitation = await server.effortGraph!.writer.mutate({ + type: 'WriteCitation', + effort: otherEffortId, + title: 'Foreign paper', + body: 'https://example.com/foreign-paper', + }); + await t.throwsAsync( + server.effortGraph!.writer.mutate({ + type: 'WriteFinding', + effort: effortId, + title: 'Bad cite', + body: '', + kind: 'measurement', + cites: [foreignCitation.artifacts[0].id], + }), + { + message: new RegExp( + `cites target ${foreignCitation.artifacts[0].id} belongs to a different effort` + ), + } + ); + const finding = await server.effortGraph!.writer.mutate({ + type: 'WriteFinding', + effort: effortId, + title: 'Measured', + body: 'short', + kind: 'measurement', + cites: [citation.artifacts[0].id], + }); + await server.effortGraph!.waitForCommittedGeneration(finding.generation); + const findingResponse = await query( + server.port, + `{ allFindings { title cites { id } } }` + ); + t.deepEqual(findingResponse.errors, undefined); + t.deepEqual(findingResponse.data?.allFindings, [ + { title: 'Measured', cites: [{ id: citation.artifacts[0].id }] }, + ]); } );