From ae76af78712f7baf36667bd6c9f73c59148dcb8d Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 13:57:41 -0700 Subject: [PATCH 01/11] agents|internal: Assemble a lede decision episode from a ticket's artifacts Adds the resolution step behind lede-decision capture: given a ticket's artifact directory, it produces the lede an agent published, the lede that reached the merge commit, whether the two differ once whitespace is normalized, the change's work type, tier, and scope, and a content fingerprint of the doctrine that governed the agent's text. Fingerprinting the doctrine by content rather than recording a version is what lets records group by doctrine generation with nothing written at install time; the mapping back to a commit stays recoverable by re-hashing the file's history. Each lede accepts an override file, so a pull request merged outside the merge flow can still be resolved from text the caller supplies. Every missing input reports a distinct reason rather than throwing, so a caller can report and continue. --- .../__tests__/resolve-episode.test.ts | 259 +++++++++++++ .../capture-lede-decision/resolve-episode.ts | 352 ++++++++++++++++++ .../agents/src/capture-lede-decision/types.ts | 101 +++++ 3 files changed, 712 insertions(+) create mode 100644 packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts create mode 100644 packages/agents/src/capture-lede-decision/resolve-episode.ts create mode 100644 packages/agents/src/capture-lede-decision/types.ts diff --git a/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts new file mode 100644 index 00000000..a2c1b354 --- /dev/null +++ b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts @@ -0,0 +1,259 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { extractSection, resolveEpisode } from '../resolve-episode.ts'; +import type { LedeEpisode, ResolveEpisodeOutcome } from '../types.ts'; + +const AGENT_LEDE = 'Rulebooks can now address a file by linking to it.'; +const MERGED_LEDE = 'Rulebooks can now address a file by linking to it: a Markdown link reaches each harness.'; + +describe(resolveEpisode, () => { + it('resolves both ledes, the doctrine fingerprint, and the change identity', async () => { + const fixture = await createFixture({}); + + const episode = expectEpisode(await resolveEpisode(fixture.input)); + + expect(episode.agentLede).toBe(AGENT_LEDE); + expect(episode.mergedLede).toBe(MERGED_LEDE); + expect(episode.doctrineHash).toMatch(/^sha256:[\da-f]{64}$/); + expect(episode.identity).toMatchObject({ type: 'feat', tier: 'public', scope: 'agents', pr: '1124' }); + }); + + it('reports the ledes as differing when the merged text was rewritten', async () => { + const fixture = await createFixture({}); + + expect(expectEpisode(await resolveEpisode(fixture.input)).differ).toBe(true); + }); + + it('reports the ledes as identical when they differ only by whitespace', async () => { + const fixture = await createFixture({ mergedLede: `${AGENT_LEDE.replace(' ', '\n ')}\n` }); + + expect(expectEpisode(await resolveEpisode(fixture.input)).differ).toBe(false); + }); + + it('reads the newest artifact of each kind', async () => { + const fixture = await createFixture({}); + await writeArtifact(fixture.artifactDir, '20260731-090000Z_later_merge.md', section('Body', 'A later lede.')); + + expect(expectEpisode(await resolveEpisode(fixture.input)).mergedLede).toBe('A later lede.'); + }); + + it('finds an artifact nested in a run subdirectory', async () => { + const fixture = await createFixture({}); + const runDir = join(fixture.artifactDir, '20260731-100000Z-run'); + await mkdir(runDir, { recursive: true }); + await writeArtifact(runDir, '20260731-100000Z_run_merge.md', section('Body', 'A lede from a run.')); + + expect(expectEpisode(await resolveEpisode(fixture.input)).mergedLede).toBe('A lede from a run.'); + }); + + it('derives the tier from a work type declared as an alias', async () => { + const fixture = await createFixture({ type: 'feature' }); + + expect(expectEpisode(await resolveEpisode(fixture.input)).identity.tier).toBe('public'); + }); + + it('falls back to the change summary for a type and scope the caller did not pass', async () => { + const fixture = await createFixture({}); + const { type: _type, scope: _scope, ...withoutIdentity } = fixture.input; + + const episode = expectEpisode(await resolveEpisode(withoutIdentity)); + + expect(episode.identity).toMatchObject({ type: 'fix', scope: 'kb' }); + }); + + it('reads a lede from an override file rather than its artifact', async () => { + const fixture = await createFixture({}); + const overrideFile = join(fixture.root, 'override.md'); + await writeFile(overrideFile, ' A lede fetched from the forge.\n', 'utf8'); + + const episode = expectEpisode(await resolveEpisode({ ...fixture.input, mergedLedeFile: overrideFile })); + + expect(episode.mergedLede).toBe('A lede fetched from the forge.'); + }); + + it('omits the agents version when the install manifest is unreadable', async () => { + const fixture = await createFixture({}); + + const episode = expectEpisode( + await resolveEpisode({ ...fixture.input, manifestPath: join(fixture.root, 'absent.json') }), + ); + + expect(episode.agentsVersion).toBeUndefined(); + }); + + it('reads the agents version from the install manifest', async () => { + const fixture = await createFixture({}); + const manifestPath = join(fixture.root, 'manifest.json'); + await writeFile(manifestPath, JSON.stringify({ shared: { version: '1.2.3' } }), 'utf8'); + + expect(expectEpisode(await resolveEpisode({ ...fixture.input, manifestPath })).agentsVersion).toBe('1.2.3'); + }); + + it('reports a missing artifact directory', async () => { + const fixture = await createFixture({}); + + const outcome = await resolveEpisode({ ...fixture.input, artifactDir: join(fixture.root, 'absent') }); + + expect(expectFailure(outcome)).toBe('no-artifact-dir'); + }); + + it('reports an absent pull-request artifact separately from an absent merge artifact', async () => { + const fixture = await createFixture({ omit: 'pull-request' }); + + expect(expectFailure(await resolveEpisode(fixture.input))).toBe('no-agent-lede'); + }); + + it('reports a merge artifact carrying no body section', async () => { + const fixture = await createFixture({ mergedLede: '' }); + + expect(expectFailure(await resolveEpisode(fixture.input))).toBe('no-merged-lede'); + }); + + it('reports an unreadable doctrine file', async () => { + const fixture = await createFixture({}); + await rm(join(fixture.dataDir, 'lede-voice.md')); + + expect(expectFailure(await resolveEpisode(fixture.input))).toBe('no-doctrine'); + }); + + it('reports a work type the taxonomy does not declare', async () => { + const fixture = await createFixture({ type: 'invented' }); + + expect(expectFailure(await resolveEpisode(fixture.input))).toBe('unresolved-identity'); + }); +}); + +describe(extractSection, () => { + it('captures everything up to the next second-level heading', () => { + const text = '## What\n\nThe lede.\n\n## Why\n\nThe motivation.\n'; + + expect(extractSection({ text, heading: 'What' })).toBe('The lede.'); + }); + + it('captures a nested third-level heading rather than stopping at it', () => { + const text = '## Body\n\nLead.\n\n### Detail\n\nMore.\n\n## Next\n'; + + expect(extractSection({ text, heading: 'Body' })).toBe('Lead.\n\n### Detail\n\nMore.'); + }); + + it('captures the final section when no heading follows it', () => { + const text = '# Title\n\n## Body\n\nThe lede.\n'; + + expect(extractSection({ text, heading: 'Body' })).toBe('The lede.'); + }); + + it('matches the heading without regard to case', () => { + expect(extractSection({ text: '## WHAT\n\nThe lede.\n', heading: 'What' })).toBe('The lede.'); + }); + + it('yields null for a heading the document does not carry', () => { + expect(extractSection({ text: '## Why\n\nThe motivation.\n', heading: 'What' })).toBeNull(); + }); + + it('yields null for a heading whose section holds no text', () => { + expect(extractSection({ text: '## What\n\n## Why\n\nThe motivation.\n', heading: 'What' })).toBeNull(); + }); +}); + +// region | Helpers + +/** One temporary fixture tree: the artifact directory, the `_data` directory, and a ready-made resolver input. */ +interface Fixture { + root: string; + artifactDir: string; + dataDir: string; + input: Parameters[0]; +} + +/** + * Builds a temporary ticket directory carrying a pull-request, merge, and change-summary artifact, plus a `_data` + * directory holding a doctrine file and a minimal work-type taxonomy. The change summary declares a type and scope + * that differ from the resolver input's, so a test can tell a flag from its fallback. + */ +async function createFixture(overrides: { + type?: string; + mergedLede?: string; + omit?: 'pull-request' | 'merge'; +}): Promise { + const root = await mkdtemp(join(tmpdir(), 'lede-decision-')); + const artifactDir = join(root, 'tickets', '1107'); + const dataDir = join(root, '_data'); + await mkdir(artifactDir, { recursive: true }); + await mkdir(dataDir, { recursive: true }); + + if (overrides.omit !== 'pull-request') { + const body = `## Body\n\n${section('What', AGENT_LEDE)}\n## Why\n\nThe motivation.\n`; + await writeArtifact(artifactDir, '20260730-174300Z_fixture_pull-request.md', body); + } + if (overrides.omit !== 'merge') { + await writeArtifact( + artifactDir, + '20260730-175638Z_fixture_merge.md', + section('Body', overrides.mergedLede ?? MERGED_LEDE), + ); + } + await writeArtifact( + artifactDir, + '20260730-174234Z_fixture_change-summary.md', + `---\ntype: fix\nscope: kb\nticket_id: '1107'\n---\n\n# Title\n`, + ); + + await writeFile(join(dataDir, 'lede-voice.md'), '# Lede voice\n\nDoctrine text.\n', 'utf8'); + await writeFile( + join(dataDir, 'work-types.json'), + JSON.stringify({ + types: [ + { key: 'feat', tier: 'public', aliases: ['feature'] }, + { key: 'fix', tier: 'public', aliases: [] }, + ], + }), + 'utf8', + ); + + return { + root, + artifactDir, + dataDir, + input: { + artifactDir, + dataDir, + pr: '1124', + mergeCommit: '35aa58d7', + type: overrides.type ?? 'feat', + scope: 'agents', + manifestPath: join(root, 'manifest.json'), + }, + }; +} + +/** Narrows a resolver outcome to its success arm, failing the test with the reported reason when it is not one. */ +function expectEpisode(outcome: ResolveEpisodeOutcome): LedeEpisode { + if (outcome.ok) { + return outcome.episode; + } + throw new Error(`expected a resolved episode, got ${outcome.error}: ${outcome.message}`); +} + +/** Narrows a resolver outcome to its failure arm and yields the error code, failing the test when it succeeded. */ +function expectFailure(outcome: ResolveEpisodeOutcome): string { + if (outcome.ok) { + throw new Error('expected the resolver to fail, but it resolved an episode'); + } + return outcome.error; +} + +/** Renders a second-level Markdown section with its heading. */ +function section(heading: string, body: string): string { + return `## ${heading}\n\n${body}\n`; +} + +/** Writes one artifact file into a directory. */ +async function writeArtifact(directory: string, filename: string, content: string): Promise { + await writeFile(join(directory, filename), content, 'utf8'); +} + +// endregion | Helpers diff --git a/packages/agents/src/capture-lede-decision/resolve-episode.ts b/packages/agents/src/capture-lede-decision/resolve-episode.ts new file mode 100644 index 00000000..584d7bc6 --- /dev/null +++ b/packages/agents/src/capture-lede-decision/resolve-episode.ts @@ -0,0 +1,352 @@ +import { createHash } from 'node:crypto'; +import { readdir, readFile, stat } from 'node:fs/promises'; +import path from 'node:path'; +import process from 'node:process'; + +import { readNoteContent } from '@codeassembly/kb/note-io'; + +import { extractString } from '../kb-shared/note-helpers.ts'; +import { isEnoent, isRecord } from '../lib/type-guards.ts'; +import type { EpisodeIdentity, ResolveEpisodeOutcome } from './types.ts'; + +/** Artifact filename suffix holding the lede the agent published, and the heading that lede sits under. */ +const AGENT_LEDE_SOURCE = { suffix: '_pull-request', heading: 'What' } as const; + +/** Artifact filename suffix holding the lede that merged, and the heading it sits under. */ +const MERGED_LEDE_SOURCE = { suffix: '_merge', heading: 'Body' } as const; + +/** + * Assembles a lede decision episode from a ticket's artifact directory: the lede the agent published, the lede that + * merged, the change's identity, and a fingerprint of the doctrine that governed the agent's text. + * + * Each lede accepts an override file, so a pull request merged outside the merge flow — which writes no `_merge.md` — + * can still be recorded from text the caller fetched. Absent an override, each is read from the newest artifact of its + * kind, searched recursively because an orchestrated run nests its artifacts in a run subdirectory. Artifact filenames + * open with a `YYYYMMDDD-HHMMSSZ` stamp, so the lexicographically greatest basename is the newest. + * + * The doctrine is fingerprinted by content rather than recorded as a version, which is what lets records group by + * doctrine generation with nothing written at install time: the mapping back to a commit stays recoverable afterwards + * by re-hashing the file's own history. + */ +export async function resolveEpisode(input: { + artifactDir: string; + /** Directory holding `lede-voice.md` and `work-types.json`; the `_data` sibling of the installed helper. */ + dataDir: string; + pr: string; + mergeCommit: string; + type?: string; + scope?: string; + ticket?: string; + agentLedeFile?: string; + mergedLedeFile?: string; + /** Install manifest supplying the agents-package version; defaults to the user-global manifest. */ + manifestPath?: string; +}): Promise { + if (!(await isDirectory(input.artifactDir))) { + return { ok: false, error: 'no-artifact-dir', message: `artifact directory not found: ${input.artifactDir}` }; + } + + const agentLede = await readLede({ + artifactDir: input.artifactDir, + source: AGENT_LEDE_SOURCE, + ...(input.agentLedeFile !== undefined && { overrideFile: input.agentLedeFile }), + }); + if (agentLede === null) { + return { + ok: false, + error: 'no-agent-lede', + message: `no "## ${AGENT_LEDE_SOURCE.heading}" section in a ${AGENT_LEDE_SOURCE.suffix}.md artifact under ${input.artifactDir}`, + }; + } + + const mergedLede = await readLede({ + artifactDir: input.artifactDir, + source: MERGED_LEDE_SOURCE, + ...(input.mergedLedeFile !== undefined && { overrideFile: input.mergedLedeFile }), + }); + if (mergedLede === null) { + return { + ok: false, + error: 'no-merged-lede', + message: `no "## ${MERGED_LEDE_SOURCE.heading}" section in a ${MERGED_LEDE_SOURCE.suffix}.md artifact under ${input.artifactDir}`, + }; + } + + const doctrinePath = path.join(input.dataDir, 'lede-voice.md'); + const doctrineHash = await hashFile(doctrinePath); + if (doctrineHash === null) { + return { ok: false, error: 'no-doctrine', message: `doctrine file not readable: ${doctrinePath}` }; + } + + const identity = await resolveIdentity(input); + if (!identity.ok) { + return identity; + } + + const agentsVersion = await readAgentsVersion(input.manifestPath); + + return { + ok: true, + episode: { + agentLede, + mergedLede, + differ: normalizeLede(agentLede) !== normalizeLede(mergedLede), + identity: identity.identity, + doctrineHash, + ...(agentsVersion !== null && { agentsVersion }), + }, + }; +} + +/** + * Reads a named `## ` section from Markdown: everything between the heading and the next `## ` heading or the end of + * the document, trimmed. Yields `null` when the heading is absent or its section is empty. + * + * Matching is line-based and case-insensitive on the heading text. The artifacts this reads carry no frontmatter, so + * their heading structure is the only handle on their content. + */ +export function extractSection(input: { text: string; heading: string }): string | null { + const lines = input.text.split('\n'); + const target = input.heading.trim().toLowerCase(); + let start = -1; + + for (const [index, line] of lines.entries()) { + if (start === -1) { + if (line.startsWith('## ') && line.slice(3).trim().toLowerCase() === target) { + start = index + 1; + } + continue; + } + if (line.startsWith('## ')) { + return joinSection(lines.slice(start, index)); + } + } + + return start === -1 ? null : joinSection(lines.slice(start)); +} + +// region | Helpers + +/** + * Locates the newest artifact whose basename ends with `{suffix}.md`, searching recursively so a run subdirectory's + * artifact competes with the ticket root's. Yields `null` when none exists. + */ +async function findNewestArtifact(input: { artifactDir: string; suffix: string }): Promise { + let entries: string[]; + try { + entries = await readdir(input.artifactDir, { recursive: true }); + } catch (error) { + if (isEnoent(error)) { + return null; + } + throw error; + } + + let newest: { basename: string; relativePath: string } | null = null; + for (const relativePath of entries) { + const basename = path.basename(relativePath); + if (!basename.endsWith(`${input.suffix}.md`)) { + continue; + } + if (newest === null || basename > newest.basename) { + newest = { basename, relativePath }; + } + } + + return newest === null ? null : path.join(input.artifactDir, newest.relativePath); +} + +/** Computes a `sha256:`-prefixed digest of a file's bytes; `null` when the file cannot be read. */ +async function hashFile(filePath: string): Promise { + const content = await readFileSafely(filePath); + return content === null ? null : `sha256:${createHash('sha256').update(content).digest('hex')}`; +} + +/** Reports whether a path exists and is a directory. */ +async function isDirectory(dirPath: string): Promise { + try { + return (await stat(dirPath)).isDirectory(); + } catch (error) { + if (isEnoent(error)) { + return false; + } + throw error; + } +} + +/** Joins section lines and trims them, yielding `null` for a section that holds no text. */ +function joinSection(lines: readonly string[]): string | null { + const section = lines.join('\n').trim(); + return section.length > 0 ? section : null; +} + +/** Collapses runs of whitespace so two ledes differing only by reflow compare equal. */ +function normalizeLede(value: string): string { + return value.replaceAll(/\s+/gu, ' ').trim(); +} + +/** Reads the installed agents-package version from the install manifest; `null` when it is absent or unreadable. */ +async function readAgentsVersion(manifestPath: string | undefined): Promise { + const resolved = manifestPath ?? path.join(process.env.HOME ?? '', '.codeassembly', 'agents-manifest.json'); + const content = await readFileSafely(resolved); + if (content === null) { + return null; + } + + let parsed: unknown; + try { + parsed = JSON.parse(content); + } catch { + return null; + } + if (!isRecord(parsed) || !isRecord(parsed.shared)) { + return null; + } + return typeof parsed.shared.version === 'string' ? parsed.shared.version : null; +} + +/** + * Reads `type`, `scope`, and `ticket_id` from the newest change-summary artifact's frontmatter; each may be `null`. + * The read is field-blind rather than routed through the knowledge base's record parser: a change summary is an + * artifact, not a knowledge-base record, and imposing that schema on it would reject the whole block over fields an + * artifact never carries. + */ +async function readChangeSummaryFields( + artifactDir: string, +): Promise<{ type: string | null; scope: string | null; ticket: string | null }> { + const absent = { type: null, scope: null, ticket: null }; + + const artifactPath = await findNewestArtifact({ artifactDir, suffix: '_change-summary' }); + if (artifactPath === null) { + return absent; + } + const content = await readFileSafely(artifactPath); + if (content === null) { + return absent; + } + + const { fields } = readNoteContent(content); + return { + type: extractString(fields, 'type'), + scope: extractString(fields, 'scope'), + ticket: extractString(fields, 'ticket_id'), + }; +} + +/** Reads a file as UTF-8, yielding `null` when it does not exist. */ +async function readFileSafely(filePath: string): Promise { + try { + return await readFile(filePath, 'utf8'); + } catch (error) { + if (isEnoent(error)) { + return null; + } + throw error; + } +} + +/** + * Reads one lede: the override file's whole contents when given, otherwise the named section of the newest artifact of + * its kind. Yields `null` when neither yields text. + */ +async function readLede(input: { + artifactDir: string; + source: { suffix: string; heading: string }; + overrideFile?: string; +}): Promise { + if (input.overrideFile !== undefined) { + const override = await readFileSafely(input.overrideFile); + return override === null ? null : joinSection([override]); + } + + const artifactPath = await findNewestArtifact({ artifactDir: input.artifactDir, suffix: input.source.suffix }); + if (artifactPath === null) { + return null; + } + const text = await readFileSafely(artifactPath); + return text === null ? null : extractSection({ text, heading: input.source.heading }); +} + +/** + * Resolves the change's identity, preferring the caller's flags and falling back to the newest change-summary + * artifact's frontmatter, which is the only artifact in the chain that carries typed fields. The tier derives from the + * work type through the installed taxonomy rather than being passed in, so it always reflects the taxonomy in force. + */ +async function resolveIdentity(input: { + artifactDir: string; + dataDir: string; + pr: string; + mergeCommit: string; + type?: string; + scope?: string; + ticket?: string; +}): Promise<{ ok: true; identity: EpisodeIdentity } | { ok: false; error: 'unresolved-identity'; message: string }> { + const fallback = await readChangeSummaryFields(input.artifactDir); + + const type = input.type ?? fallback.type; + if (type === null || type === undefined) { + return { ok: false, error: 'unresolved-identity', message: 'work type could not be resolved; pass --type' }; + } + + const scope = input.scope ?? fallback.scope; + if (scope === null || scope === undefined) { + return { ok: false, error: 'unresolved-identity', message: 'scope could not be resolved; pass --scope' }; + } + + const tier = await resolveTier({ dataDir: input.dataDir, type }); + if (tier === null) { + return { + ok: false, + error: 'unresolved-identity', + message: `work type "${type}" is not declared in work-types.json, so its tier cannot be resolved`, + }; + } + + const ticket = input.ticket ?? fallback.ticket; + + return { + ok: true, + identity: { + type, + tier, + scope, + pr: input.pr, + mergeCommit: input.mergeCommit, + ...(ticket !== null && ticket !== undefined && { ticket }), + }, + }; +} + +/** + * Looks up a work type's tier in the installed taxonomy, matching the canonical key or any declared alias; `null` when + * the taxonomy is unreadable or declares no such type. + */ +async function resolveTier(input: { dataDir: string; type: string }): Promise { + const content = await readFileSafely(path.join(input.dataDir, 'work-types.json')); + if (content === null) { + return null; + } + + let parsed: unknown; + try { + parsed = JSON.parse(content); + } catch { + return null; + } + if (!isRecord(parsed) || !Array.isArray(parsed.types)) { + return null; + } + + for (const entry of parsed.types) { + if (!isRecord(entry) || typeof entry.tier !== 'string') { + continue; + } + const aliases = Array.isArray(entry.aliases) ? entry.aliases : []; + if (entry.key === input.type || aliases.includes(input.type)) { + return entry.tier; + } + } + return null; +} + +// endregion | Helpers diff --git a/packages/agents/src/capture-lede-decision/types.ts b/packages/agents/src/capture-lede-decision/types.ts new file mode 100644 index 00000000..529e35ea --- /dev/null +++ b/packages/agents/src/capture-lede-decision/types.ts @@ -0,0 +1,101 @@ +// Shapes for the capture-lede-decision helper: the verdict an author records, the resolved decision episode, and the +// JSON payloads emitted to stdout. +// +// The stdout payload is a discriminated union on `ok`, mirroring `capture-event`. Recoverable failures (an unresolvable +// artifact, no reachable store, schema validation, invalid args) return `{ ok: false, error, message }`; system errors +// (out-of-disk, permission denied) print to stderr and exit non-zero. + +/** The verdicts an author may record, in the order the skill presents them. */ +export const LEDE_VERDICTS = ['accepted', 'revised'] as const; + +/** An author's decision about a lede: the agent's text shipped as written, or it was rewritten before merge. */ +export type LedeVerdict = (typeof LEDE_VERDICTS)[number]; + +// A widened-element set for membership tests: the `as const` tuple's literal element type rejects a `string` argument +// to `.includes`, and type assertions are banned, so the set's `.has(string)` is the assertion-free lookup. +const VERDICT_SET: ReadonlySet = new Set(LEDE_VERDICTS); + +/** Reports whether a value is one of the declared {@link LEDE_VERDICTS}. */ +export function isLedeVerdict(value: unknown): value is LedeVerdict { + return typeof value === 'string' && VERDICT_SET.has(value); +} + +/** The change a decision describes, resolved from caller flags with change-summary frontmatter as the fallback. */ +export interface EpisodeIdentity { + type: string; + tier: string; + scope: string; + pr: string; + mergeCommit: string; + /** Ticket the change served; absent for a branch that carried none. */ + ticket?: string; +} + +/** A resolved decision episode: both ledes, whether they differ, the change's identity, and the doctrine in force. */ +export interface LedeEpisode { + /** The `## What` the agent published to the pull request. */ + agentLede: string; + /** The lede that reached the merge commit. */ + mergedLede: string; + /** Whether the ledes differ once whitespace is normalized, so a reflow alone does not read as a revision. */ + differ: boolean; + identity: EpisodeIdentity; + /** `sha256:`-prefixed digest of the doctrine file that governed the agent's lede. */ + doctrineHash: string; + /** Installed agents-package version; absent when the install manifest is unreadable. */ + agentsVersion?: string; +} + +/** The outcome of resolving an episode: the episode, or the categorical reason it could not be assembled. */ +export type ResolveEpisodeOutcome = + { ok: true; episode: LedeEpisode } | { ok: false; error: EpisodeErrorCode; message: string }; + +/** + * Categorical reasons an episode cannot be resolved, each naming a distinct missing input. `unresolved-identity` covers + * every field of {@link EpisodeIdentity} under one code, with the message naming the field that failed: the caller's + * recourse is the same in each case — supply the flag — so splitting it per field would buy the caller nothing. + */ +export type EpisodeErrorCode = + 'no-artifact-dir' | 'no-agent-lede' | 'no-merged-lede' | 'no-doctrine' | 'unresolved-identity'; + +/** The stdout payload for `--inspect`: the resolved episode, with nothing written. */ +export interface InspectSuccess { + ok: true; + mode: 'inspect'; + episode: LedeEpisode; +} + +/** The stdout payload for a recorded decision. */ +export interface CommitSuccess { + ok: true; + mode: 'commit'; + verdict: LedeVerdict; + /** The generated ULID, which is also the record's filename stem. */ + id: string; + capturedAt: string; + path: string; + /** Registry name of the store the record was written to. */ + store: string; +} + +/** The stdout payload on a recoverable failure. */ +export interface DecisionFailure { + ok: false; + error: DecisionErrorCode; + message: string; + /** Validation errors, set when `error: 'schema-validation'`. */ + errors?: string[]; +} + +/** Every categorical error code the helper can return without an unexpected throw. */ +export type DecisionErrorCode = + | EpisodeErrorCode + | 'invalid-args' + | 'missing-store' + | 'store-not-registered' + | 'readonly-store' + | 'no-default-store' + | 'schema-validation'; + +/** The helper's full stdout payload: a discriminated union on `ok`. */ +export type DecisionResult = InspectSuccess | CommitSuccess | DecisionFailure; From 2338c6317a7fcf45aa37b0218c7b5c35d2727139 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 13:59:47 -0700 Subject: [PATCH 02/11] agents|refactor: Share one refusal message across helpers that require a store The message naming the registered knowledge stores, emitted when a capture is refused for want of a named destination, now has a single definition beside the equivalent already shared by the discovery-based helpers. Two verbatim copies stood behind it, and a third was about to join them. --- packages/agents/src/capture-event/cli.ts | 24 +---------------- .../src/kb-shared/format-missing-store.ts | 26 +++++++++++++++++++ packages/agents/src/kb-update-events/cli.ts | 24 +---------------- 3 files changed, 28 insertions(+), 46 deletions(-) create mode 100644 packages/agents/src/kb-shared/format-missing-store.ts diff --git a/packages/agents/src/capture-event/cli.ts b/packages/agents/src/capture-event/cli.ts index 785d8068..8e6a000d 100644 --- a/packages/agents/src/capture-event/cli.ts +++ b/packages/agents/src/capture-event/cli.ts @@ -17,6 +17,7 @@ import { } from '@codeassembly/kb/records'; import { ulid } from 'ulid'; +import { formatMissingStoreMessage } from '../kb-shared/format-missing-store.ts'; import { formatUtcTimestamp, isSafeEventId } from '../kb-shared/note-helpers.ts'; import { resolveCaptureTarget } from '../kb-shared/resolve-capture-target.ts'; import { parseTagList } from '../kb-shared/tag-helpers.ts'; @@ -302,29 +303,6 @@ function amendRecord(existing: KbEvent, args: ParsedArgs, body: string): KbEvent }; } -/** - * Builds the agent-facing error message for an omitted `--store`, naming the registered stores and, when configured, - * the registry default reachable as `--store @default`. - */ -function formatMissingStoreMessage(resolved: { - registeredStores: string[]; - defaultName?: string; - registryError?: string; -}): string { - if (resolved.registryError !== undefined) { - return `--store is required, but the kb.yaml registry could not be loaded: ${resolved.registryError}`; - } - if (resolved.registeredStores.length === 0) { - return '--store is required, but no stores are registered in kb.yaml'; - } - const stores = resolved.registeredStores.join(', '); - const defaultHint = - resolved.defaultName !== undefined - ? `the registry default is "${resolved.defaultName}", reachable as --store @default` - : 'no default_kb is configured'; - return `--store is required. Registered stores: ${stores}. Pass --store to choose one; ${defaultHint}.`; -} - /** * Returns true when this module is the process entry point. Both sides are resolved through `realpathSync`, so a * symlinked invocation path still matches. On a `realpathSync` failure the function emits a warning and returns diff --git a/packages/agents/src/kb-shared/format-missing-store.ts b/packages/agents/src/kb-shared/format-missing-store.ts new file mode 100644 index 00000000..5fb77975 --- /dev/null +++ b/packages/agents/src/kb-shared/format-missing-store.ts @@ -0,0 +1,26 @@ +/** + * Builds the agent-facing error message for an omitted `--store`, naming the registered stores and, when configured, + * the registry default reachable as `--store @default`. Shared by the helpers that refuse a capture with no named + * destination, so their refusals stay parallel and each surfaces the registered names the resolver already computed. + * + * The `--kb` family has its own wording in `formatMissingDestinationMessage`: those tools discover a `.kb/` by walking + * the working directory, so their refusal has to explain that the walk found nothing as well as that no flag was given. + */ +export function formatMissingStoreMessage(resolved: { + registeredStores: string[]; + defaultName?: string; + registryError?: string; +}): string { + if (resolved.registryError !== undefined) { + return `--store is required, but the kb.yaml registry could not be loaded: ${resolved.registryError}`; + } + if (resolved.registeredStores.length === 0) { + return '--store is required, but no stores are registered in kb.yaml'; + } + const stores = resolved.registeredStores.join(', '); + const defaultHint = + resolved.defaultName !== undefined + ? `the registry default is "${resolved.defaultName}", reachable as --store @default` + : 'no default_kb is configured'; + return `--store is required. Registered stores: ${stores}. Pass --store to choose one; ${defaultHint}.`; +} diff --git a/packages/agents/src/kb-update-events/cli.ts b/packages/agents/src/kb-update-events/cli.ts index f852653f..4f56adb4 100644 --- a/packages/agents/src/kb-update-events/cli.ts +++ b/packages/agents/src/kb-update-events/cli.ts @@ -10,6 +10,7 @@ import { type ReadNote, readNote, writeNote } from '@codeassembly/kb/note-io'; import { EVENT_IMPACT_LEVELS, isEventImpact, type KbEvent, parseEvent, renderEvent } from '@codeassembly/kb/records'; import { loadAliases } from '@codeassembly/kb/tags'; +import { formatMissingStoreMessage } from '../kb-shared/format-missing-store.ts'; import { isSafeEventId, splitCommaList } from '../kb-shared/note-helpers.ts'; import { resolveCaptureTarget, type ResolveCaptureTargetOutcome } from '../kb-shared/resolve-capture-target.ts'; import { parseTagList } from '../kb-shared/tag-helpers.ts'; @@ -212,29 +213,6 @@ async function editOne(input: { return { ok: true, id, path }; } -/** - * Builds the agent-facing error message for an omitted `--store`, naming the registered stores and, when configured, - * the registry default reachable as `--store @default`. - */ -function formatMissingStoreMessage(resolved: { - registeredStores: string[]; - defaultName?: string; - registryError?: string; -}): string { - if (resolved.registryError !== undefined) { - return `--store is required, but the kb.yaml registry could not be loaded: ${resolved.registryError}`; - } - if (resolved.registeredStores.length === 0) { - return '--store is required, but no stores are registered in kb.yaml'; - } - const stores = resolved.registeredStores.join(', '); - const defaultHint = - resolved.defaultName !== undefined - ? `the registry default is "${resolved.defaultName}", reachable as --store @default` - : 'no default_kb is configured'; - return `--store is required. Registered stores: ${stores}. Pass --store to choose one; ${defaultHint}.`; -} - /** Returns true when this module is the process entry point, resolving both sides through `realpathSync` so a symlinked invocation still matches. */ function isEntryPoint(): boolean { const entry = process.argv[1]; From aa689deb78f6d02dffbc569805a3c1cc6973367f Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 14:09:02 -0700 Subject: [PATCH 03/11] agents|internal: Record a lede decision as a knowledge-base event Adds the command behind lede-decision capture. Inspecting reports the resolved episode and writes nothing, so a caller can show both ledes before asking the author to decide; recording writes one event carrying the verdict, both ledes, an optional comment, and the change's identity and doctrine fingerprint. A decision joins the existing event substrate rather than introducing a record type of its own, so it recalls beside every other captured event. Its tags carry the group, the verdict, and the work type under a namespace, which keeps a work type from colliding with the topical tags an event already uses. The body records the merged lede whenever the two texts differ, independently of the verdict: the verdict holds what the author says they did, and the body holds what happened. Every failure the command can foresee comes back as a structured result rather than a throw, so a caller running this after an irreversible merge can report one line and carry on. --- .../__tests__/cli.test.ts | 254 +++++++++++++++ .../__tests__/prepare-decision.test.ts | 148 +++++++++ .../__tests__/resolve-episode.test.ts | 179 ++++------ .../agents/src/capture-lede-decision/cli.ts | 306 ++++++++++++++++++ .../capture-lede-decision/prepare-decision.ts | 103 ++++++ .../capture-lede-decision/resolve-episode.ts | 6 +- .../test-utils/create-lede-fixture.ts | 82 +++++ 7 files changed, 952 insertions(+), 126 deletions(-) create mode 100644 packages/agents/src/capture-lede-decision/__tests__/cli.test.ts create mode 100644 packages/agents/src/capture-lede-decision/__tests__/prepare-decision.test.ts create mode 100644 packages/agents/src/capture-lede-decision/cli.ts create mode 100644 packages/agents/src/capture-lede-decision/prepare-decision.ts create mode 100644 packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts diff --git a/packages/agents/src/capture-lede-decision/__tests__/cli.test.ts b/packages/agents/src/capture-lede-decision/__tests__/cli.test.ts new file mode 100644 index 00000000..1e9319c5 --- /dev/null +++ b/packages/agents/src/capture-lede-decision/__tests__/cli.test.ts @@ -0,0 +1,254 @@ +import { mkdir, mkdtemp, readdir, readFile, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { Readable } from 'node:stream'; + +import { describe, expect, it } from 'vitest'; + +import { parseArgs, runDecision } from '../cli.ts'; +import { createLedeFixture, type LedeFixture } from '../test-utils/create-lede-fixture.ts'; +import type { DecisionResult } from '../types.ts'; + +const NOW = new Date('2026-07-30T20:41:17.000Z'); +const STORE_NAME = 'codeassembly'; + +describe(parseArgs, () => { + it('parses every value-bearing flag in long form', () => { + const parsed = parseArgs([ + '--verdict', + 'revised', + '--artifact-dir', + '/tickets/1107', + '--pr', + '1124', + '--merge-commit', + '35aa58d7', + '--data-dir', + '/skills/_data', + '--store', + STORE_NAME, + '--type', + 'feat', + '--scope', + 'agents', + '--ticket', + '1107', + '--agent-lede-file', + '/tmp/agent.md', + '--merged-lede-file', + '/tmp/merged.md', + '--manifest', + '/tmp/manifest.json', + '--harness', + 'claude', + ]); + + expect(parsed).toStrictEqual({ + mode: 'commit', + verdict: 'revised', + artifactDir: '/tickets/1107', + pr: '1124', + mergeCommit: '35aa58d7', + dataDir: '/skills/_data', + store: STORE_NAME, + type: 'feat', + scope: 'agents', + ticket: '1107', + agentLedeFile: '/tmp/agent.md', + mergedLedeFile: '/tmp/merged.md', + manifest: '/tmp/manifest.json', + harness: 'claude', + }); + }); + + it('selects inspect mode and leaves the verdict unset', () => { + const parsed = parseArgs(['--inspect', ...requiredFlags()]); + + expect(parsed.mode).toBe('inspect'); + expect(parsed.verdict).toBeNull(); + }); + + it.each([['artifact-dir'], ['pr'], ['merge-commit']])('requires --%s', (name) => { + const argv = withoutFlag(['--inspect', ...requiredFlags()], name); + + expect(() => parseArgs(argv)).toThrow(`--${name} is required`); + }); + + it('refuses an invocation that selects both modes', () => { + expect(() => parseArgs(['--inspect', '--verdict', 'revised', ...requiredFlags()])).toThrow('mutually exclusive'); + }); + + it('refuses an invocation that selects neither mode', () => { + expect(() => parseArgs(requiredFlags())).toThrow('one of --inspect or --verdict'); + }); + + it('refuses a verdict outside the declared set', () => { + expect(() => parseArgs(['--verdict', 'maybe', ...requiredFlags()])).toThrow('--verdict must be one of'); + }); +}); + +describe(runDecision, () => { + it('reports the resolved episode in inspect mode', async () => { + const fixture = await createLedeFixture(); + + const result = await runDecision(runInput({ argv: ['--inspect', ...flagsFor(fixture)], fixture })); + + expect(result).toMatchObject({ ok: true, mode: 'inspect' }); + expect(expectInspect(result).differ).toBe(true); + }); + + it('needs no store in inspect mode, so resolving one cannot block a report', async () => { + const fixture = await createLedeFixture(); + + const result = await runDecision(runInput({ argv: ['--inspect', ...flagsFor(fixture)], fixture, home: undefined })); + + expect(result.ok).toBe(true); + }); + + it('writes one event record when a verdict is recorded', async () => { + const fixture = await createLedeFixture(); + const store = await makeStore(); + + const result = await runDecision( + runInput({ + argv: ['--verdict', 'revised', '--store', STORE_NAME, ...flagsFor(fixture)], + fixture, + home: store.home, + comment: 'Cut the setup clause.', + }), + ); + + const written = expectCommit(result); + expect(written.store).toBe(STORE_NAME); + const content = await readFile(written.path, 'utf8'); + expect(content).toMatch(/^tags: \[lede-decision, type:feat, revised]$/m); + expect(content).toContain('## Comment\n\nCut the setup clause.'); + }); + + it('writes nothing in inspect mode', async () => { + const fixture = await createLedeFixture(); + const store = await makeStore(); + + await runDecision( + runInput({ argv: ['--inspect', '--store', STORE_NAME, ...flagsFor(fixture)], fixture, home: store.home }), + ); + + await expect(readdir(join(store.storePath, 'content', 'events'))).rejects.toThrow(); + }); + + it('reports a resolution failure by its own code rather than a generic error', async () => { + const fixture = await createLedeFixture(); + const argv = ['--inspect', ...flagsFor({ ...fixture, artifactDir: join(fixture.root, 'absent') })]; + + expect(expectFailure(await runDecision(runInput({ argv, fixture })))).toBe('no-artifact-dir'); + }); + + it('refuses to record a decision with no named store', async () => { + const fixture = await createLedeFixture(); + const store = await makeStore(); + const argv = ['--verdict', 'accepted', ...flagsFor(fixture)]; + + const result = await runDecision(runInput({ argv, fixture, home: store.home })); + + expect(expectFailure(result)).toBe('missing-store'); + }); + + it('reports an invalid invocation without touching the artifacts', async () => { + const fixture = await createLedeFixture(); + + expect(expectFailure(await runDecision(runInput({ argv: ['--inspect'], fixture })))).toBe('invalid-args'); + }); +}); + +// region | Helpers + +/** Narrows a result to a recorded decision, failing the test with the reported reason when it is not one. */ +function expectCommit(result: DecisionResult): Extract { + if (result.ok && result.mode === 'commit') { + return result; + } + throw new Error(`expected a recorded decision, got ${JSON.stringify(result)}`); +} + +/** Narrows a result to its failure arm and yields the error code, failing the test when it succeeded. */ +function expectFailure(result: DecisionResult): string { + if (result.ok) { + throw new Error(`expected a failure, got ${JSON.stringify(result)}`); + } + return result.error; +} + +/** Narrows a result to an inspect report, failing the test when it is not one. */ +function expectInspect(result: DecisionResult): Extract['episode'] { + if (result.ok && result.mode === 'inspect') { + return result.episode; + } + throw new Error(`expected an inspect report, got ${JSON.stringify(result)}`); +} + +/** The flags a merge caller supplies, pointing at a fixture tree. */ +function flagsFor(fixture: Pick): string[] { + return [ + '--artifact-dir', + fixture.artifactDir, + '--data-dir', + fixture.dataDir, + '--manifest', + fixture.manifestPath, + '--pr', + '1124', + '--merge-commit', + '35aa58d7', + '--type', + 'feat', + '--scope', + 'agents', + ]; +} + +/** Stands up a temp event store plus an isolated home registering it, so registry resolution never reads the real one. */ +async function makeStore(): Promise<{ storePath: string; home: string }> { + const storePath = await mkdtemp(join(tmpdir(), 'lede-decision-store-')); + await mkdir(join(storePath, '.kb'), { recursive: true }); + + const home = await mkdtemp(join(tmpdir(), 'lede-decision-home-')); + await mkdir(join(home, '.agents'), { recursive: true }); + await writeFile( + join(home, '.agents', 'kb.yaml'), + `default_kb: ${STORE_NAME}\nkbs:\n ${STORE_NAME}:\n path: ${storePath}\n`, + 'utf8', + ); + + return { storePath, home }; +} + +/** The three flags every invocation must carry, used to build otherwise-minimal argv in parser tests. */ +function requiredFlags(): string[] { + return ['--artifact-dir', '/tickets/1107', '--pr', '1124', '--merge-commit', '35aa58d7']; +} + +/** Builds runner input over a fixture, defaulting the environment so no test reads the developer's own. */ +function runInput(input: { + argv: string[]; + fixture: LedeFixture; + home?: string | undefined; + comment?: string; +}): Parameters[0] { + return { + argv: input.argv, + stdin: Readable.from([Buffer.from(input.comment ?? '', 'utf8')]), + cwd: input.fixture.root, + env: {}, + now: NOW, + defaultDataDir: input.fixture.dataDir, + ...(input.home !== undefined && { home: input.home }), + }; +} + +/** Removes a value-bearing flag and its value from an argv list, yielding the list unchanged when it is absent. */ +function withoutFlag(argv: readonly string[], name: string): string[] { + const index = argv.indexOf(`--${name}`); + return index === -1 ? [...argv] : [...argv.slice(0, index), ...argv.slice(index + 2)]; +} + +// endregion | Helpers diff --git a/packages/agents/src/capture-lede-decision/__tests__/prepare-decision.test.ts b/packages/agents/src/capture-lede-decision/__tests__/prepare-decision.test.ts new file mode 100644 index 00000000..3a1ef581 --- /dev/null +++ b/packages/agents/src/capture-lede-decision/__tests__/prepare-decision.test.ts @@ -0,0 +1,148 @@ +import { describe, expect, it } from 'vitest'; + +import { prepareDecision, type PrepareDecisionOutcome } from '../prepare-decision.ts'; +import type { LedeEpisode } from '../types.ts'; + +const ID = '01HZZZZZZZZZZZZZZZZZZZZZZZZ'; +const CAPTURED_AT = '2026-07-30T20:41:17Z'; + +const AGENT_LEDE = 'Rulebooks can now address a file by linking to it.'; +const MERGED_LEDE = 'Rulebooks can now address a file by linking to it: a Markdown link reaches each harness.'; + +describe(prepareDecision, () => { + it('writes recordType: event, so a decision joins the existing event substrate', () => { + const content = expectContent(prepareDecision(decisionFor({}))); + + expect(content).toMatch(/^recordType: event$/m); + }); + + it('carries the group, the namespaced work type, and the verdict as tags', () => { + const content = expectContent(prepareDecision(decisionFor({ verdict: 'revised' }))); + + expect(content).toMatch(/^tags: \[lede-decision, type:feat, revised]$/m); + }); + + it('carries the change identity and the doctrine fingerprint in frontmatter', () => { + const content = expectContent(prepareDecision(decisionFor({}))); + + expect(content).toMatch(/^type: feat$/m); + expect(content).toMatch(/^tier: public$/m); + expect(content).toMatch(/^scope: agents$/m); + expect(content).toMatch(/^pr: '1124'$/m); + expect(content).toMatch(/^merge-commit: 35aa58d7$/m); + expect(content).toMatch(/^doctrine-hash: sha256:abc$/m); + }); + + it('summarizes the decision so recall names the verdict and the change', () => { + const content = expectContent(prepareDecision(decisionFor({ verdict: 'revised' }))); + + expect(content).toMatch(/^summary: 'Lede revised for agents #1124'$/m); + }); + + it('records only the agent lede when the two texts match', () => { + const content = expectContent(prepareDecision(decisionFor({ differ: false }))); + + expect(content).toContain('## Agent lede'); + expect(content).not.toContain('## Merged lede'); + }); + + it('records the merged lede whenever the two texts differ', () => { + const content = expectContent(prepareDecision(decisionFor({ differ: true }))); + + expect(content).toContain(`## Merged lede\n\n${MERGED_LEDE}`); + }); + + it('records the merged lede on a differing pair even when the author called it accepted', () => { + const content = expectContent(prepareDecision(decisionFor({ differ: true, verdict: 'accepted' }))); + + expect(content).toContain('## Merged lede'); + expect(content).toMatch(/^tags: \[lede-decision, type:feat, accepted]$/m); + }); + + it('omits the comment section when no comment was given', () => { + const content = expectContent(prepareDecision(decisionFor({ comment: ' \n' }))); + + expect(content).not.toContain('## Comment'); + }); + + it('records a comment when the author explained the decision', () => { + const content = expectContent(prepareDecision(decisionFor({ comment: 'Cut the setup clause.\n' }))); + + expect(content).toContain('## Comment\n\nCut the setup clause.'); + }); + + it('omits every optional field the episode and context did not carry', () => { + const content = expectContent(prepareDecision(decisionFor({}))); + + expect(content).not.toMatch(/^ticket:/m); + expect(content).not.toMatch(/^agents-version:/m); + expect(content).not.toMatch(/^repo:/m); + expect(content).not.toMatch(/^harness:/m); + expect(content).not.toMatch(/^session:/m); + }); + + it('carries the optional fields the episode and context did supply', () => { + const content = expectContent( + prepareDecision({ + ...decisionFor({}), + episode: { ...episodeFor({}), agentsVersion: '1.2.3', identity: { ...IDENTITY, ticket: '1107' } }, + context: { cwd: '/tmp/work', session: 'session-abc', repo: 'owner/name' }, + harness: 'claude', + }), + ); + + expect(content).toMatch(/^ticket: '1107'$/m); + expect(content).toMatch(/^agents-version: 1.2.3$/m); + expect(content).toMatch(/^repo: owner\/name$/m); + expect(content).toMatch(/^harness: claude$/m); + expect(content).toMatch(/^session: session-abc$/m); + }); +}); + +// region | Helpers + +const IDENTITY = { + type: 'feat', + tier: 'public', + scope: 'agents', + pr: '1124', + mergeCommit: '35aa58d7', +} as const; + +/** Builds a decision input over a minimal episode, applying the overrides a test cares about. */ +function decisionFor(overrides: { + verdict?: 'accepted' | 'revised'; + differ?: boolean; + comment?: string; +}): Parameters[0] { + return { + episode: episodeFor(overrides), + verdict: overrides.verdict ?? 'revised', + comment: overrides.comment ?? '', + context: { cwd: '/tmp/work' }, + harness: null, + id: ID, + capturedAt: CAPTURED_AT, + }; +} + +/** Builds a minimal resolved episode carrying no optional field unless a test supplies one. */ +function episodeFor(overrides: { differ?: boolean }): LedeEpisode { + return { + agentLede: AGENT_LEDE, + mergedLede: MERGED_LEDE, + differ: overrides.differ ?? true, + identity: { ...IDENTITY }, + doctrineHash: 'sha256:abc', + }; +} + +/** Narrows a prepare outcome to its rendered content, failing the test with the validation errors when it did not. */ +function expectContent(outcome: PrepareDecisionOutcome): string { + if (outcome.ok) { + return outcome.prepared.content; + } + throw new Error(`expected a prepared decision, got errors: ${outcome.errors.join('; ')}`); +} + +// endregion | Helpers diff --git a/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts index a2c1b354..82a7710a 100644 --- a/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts +++ b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts @@ -1,129 +1,128 @@ -import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; +import { mkdir, rm, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; import { extractSection, resolveEpisode } from '../resolve-episode.ts'; +import { + createLedeFixture, + FIXTURE_AGENT_LEDE, + FIXTURE_MERGED_LEDE, + type LedeFixture, + renderSection, + writeArtifact, +} from '../test-utils/create-lede-fixture.ts'; import type { LedeEpisode, ResolveEpisodeOutcome } from '../types.ts'; -const AGENT_LEDE = 'Rulebooks can now address a file by linking to it.'; -const MERGED_LEDE = 'Rulebooks can now address a file by linking to it: a Markdown link reaches each harness.'; - describe(resolveEpisode, () => { it('resolves both ledes, the doctrine fingerprint, and the change identity', async () => { - const fixture = await createFixture({}); + const fixture = await createLedeFixture(); - const episode = expectEpisode(await resolveEpisode(fixture.input)); + const episode = expectEpisode(await resolveEpisode(inputFor(fixture))); - expect(episode.agentLede).toBe(AGENT_LEDE); - expect(episode.mergedLede).toBe(MERGED_LEDE); + expect(episode.agentLede).toBe(FIXTURE_AGENT_LEDE); + expect(episode.mergedLede).toBe(FIXTURE_MERGED_LEDE); expect(episode.doctrineHash).toMatch(/^sha256:[\da-f]{64}$/); expect(episode.identity).toMatchObject({ type: 'feat', tier: 'public', scope: 'agents', pr: '1124' }); }); it('reports the ledes as differing when the merged text was rewritten', async () => { - const fixture = await createFixture({}); + const fixture = await createLedeFixture(); - expect(expectEpisode(await resolveEpisode(fixture.input)).differ).toBe(true); + expect(expectEpisode(await resolveEpisode(inputFor(fixture))).differ).toBe(true); }); it('reports the ledes as identical when they differ only by whitespace', async () => { - const fixture = await createFixture({ mergedLede: `${AGENT_LEDE.replace(' ', '\n ')}\n` }); + const fixture = await createLedeFixture({ mergedLede: `${FIXTURE_AGENT_LEDE.replace(' ', '\n ')}\n` }); - expect(expectEpisode(await resolveEpisode(fixture.input)).differ).toBe(false); + expect(expectEpisode(await resolveEpisode(inputFor(fixture))).differ).toBe(false); }); it('reads the newest artifact of each kind', async () => { - const fixture = await createFixture({}); - await writeArtifact(fixture.artifactDir, '20260731-090000Z_later_merge.md', section('Body', 'A later lede.')); + const fixture = await createLedeFixture(); + await writeArtifact(fixture.artifactDir, '20260731-090000Z_later_merge.md', renderSection('Body', 'A later lede.')); - expect(expectEpisode(await resolveEpisode(fixture.input)).mergedLede).toBe('A later lede.'); + expect(expectEpisode(await resolveEpisode(inputFor(fixture))).mergedLede).toBe('A later lede.'); }); it('finds an artifact nested in a run subdirectory', async () => { - const fixture = await createFixture({}); + const fixture = await createLedeFixture(); const runDir = join(fixture.artifactDir, '20260731-100000Z-run'); await mkdir(runDir, { recursive: true }); - await writeArtifact(runDir, '20260731-100000Z_run_merge.md', section('Body', 'A lede from a run.')); + await writeArtifact(runDir, '20260731-100000Z_run_merge.md', renderSection('Body', 'A lede from a run.')); - expect(expectEpisode(await resolveEpisode(fixture.input)).mergedLede).toBe('A lede from a run.'); + expect(expectEpisode(await resolveEpisode(inputFor(fixture))).mergedLede).toBe('A lede from a run.'); }); it('derives the tier from a work type declared as an alias', async () => { - const fixture = await createFixture({ type: 'feature' }); + const fixture = await createLedeFixture(); - expect(expectEpisode(await resolveEpisode(fixture.input)).identity.tier).toBe('public'); + expect(expectEpisode(await resolveEpisode(inputFor(fixture, { type: 'feature' }))).identity.tier).toBe('public'); }); it('falls back to the change summary for a type and scope the caller did not pass', async () => { - const fixture = await createFixture({}); - const { type: _type, scope: _scope, ...withoutIdentity } = fixture.input; + const fixture = await createLedeFixture(); + const { type: _type, scope: _scope, ...withoutIdentity } = inputFor(fixture); const episode = expectEpisode(await resolveEpisode(withoutIdentity)); - expect(episode.identity).toMatchObject({ type: 'fix', scope: 'kb' }); + expect(episode.identity).toMatchObject({ type: 'fix', scope: 'kb', ticket: '1107' }); }); it('reads a lede from an override file rather than its artifact', async () => { - const fixture = await createFixture({}); + const fixture = await createLedeFixture(); const overrideFile = join(fixture.root, 'override.md'); await writeFile(overrideFile, ' A lede fetched from the forge.\n', 'utf8'); - const episode = expectEpisode(await resolveEpisode({ ...fixture.input, mergedLedeFile: overrideFile })); + const episode = expectEpisode(await resolveEpisode({ ...inputFor(fixture), mergedLedeFile: overrideFile })); expect(episode.mergedLede).toBe('A lede fetched from the forge.'); }); it('omits the agents version when the install manifest is unreadable', async () => { - const fixture = await createFixture({}); - - const episode = expectEpisode( - await resolveEpisode({ ...fixture.input, manifestPath: join(fixture.root, 'absent.json') }), - ); + const fixture = await createLedeFixture(); - expect(episode.agentsVersion).toBeUndefined(); + expect(expectEpisode(await resolveEpisode(inputFor(fixture))).agentsVersion).toBeUndefined(); }); it('reads the agents version from the install manifest', async () => { - const fixture = await createFixture({}); - const manifestPath = join(fixture.root, 'manifest.json'); - await writeFile(manifestPath, JSON.stringify({ shared: { version: '1.2.3' } }), 'utf8'); + const fixture = await createLedeFixture(); + await writeFile(fixture.manifestPath, JSON.stringify({ shared: { version: '1.2.3' } }), 'utf8'); - expect(expectEpisode(await resolveEpisode({ ...fixture.input, manifestPath })).agentsVersion).toBe('1.2.3'); + expect(expectEpisode(await resolveEpisode(inputFor(fixture))).agentsVersion).toBe('1.2.3'); }); it('reports a missing artifact directory', async () => { - const fixture = await createFixture({}); + const fixture = await createLedeFixture(); - const outcome = await resolveEpisode({ ...fixture.input, artifactDir: join(fixture.root, 'absent') }); + const outcome = await resolveEpisode({ ...inputFor(fixture), artifactDir: join(fixture.root, 'absent') }); expect(expectFailure(outcome)).toBe('no-artifact-dir'); }); it('reports an absent pull-request artifact separately from an absent merge artifact', async () => { - const fixture = await createFixture({ omit: 'pull-request' }); + const fixture = await createLedeFixture({ omit: 'pull-request' }); - expect(expectFailure(await resolveEpisode(fixture.input))).toBe('no-agent-lede'); + expect(expectFailure(await resolveEpisode(inputFor(fixture)))).toBe('no-agent-lede'); }); it('reports a merge artifact carrying no body section', async () => { - const fixture = await createFixture({ mergedLede: '' }); + const fixture = await createLedeFixture({ mergedLede: '' }); - expect(expectFailure(await resolveEpisode(fixture.input))).toBe('no-merged-lede'); + expect(expectFailure(await resolveEpisode(inputFor(fixture)))).toBe('no-merged-lede'); }); it('reports an unreadable doctrine file', async () => { - const fixture = await createFixture({}); + const fixture = await createLedeFixture(); await rm(join(fixture.dataDir, 'lede-voice.md')); - expect(expectFailure(await resolveEpisode(fixture.input))).toBe('no-doctrine'); + expect(expectFailure(await resolveEpisode(inputFor(fixture)))).toBe('no-doctrine'); }); it('reports a work type the taxonomy does not declare', async () => { - const fixture = await createFixture({ type: 'invented' }); + const fixture = await createLedeFixture(); - expect(expectFailure(await resolveEpisode(fixture.input))).toBe('unresolved-identity'); + expect(expectFailure(await resolveEpisode(inputFor(fixture, { type: 'invented' })))).toBe('unresolved-identity'); }); }); @@ -161,75 +160,6 @@ describe(extractSection, () => { // region | Helpers -/** One temporary fixture tree: the artifact directory, the `_data` directory, and a ready-made resolver input. */ -interface Fixture { - root: string; - artifactDir: string; - dataDir: string; - input: Parameters[0]; -} - -/** - * Builds a temporary ticket directory carrying a pull-request, merge, and change-summary artifact, plus a `_data` - * directory holding a doctrine file and a minimal work-type taxonomy. The change summary declares a type and scope - * that differ from the resolver input's, so a test can tell a flag from its fallback. - */ -async function createFixture(overrides: { - type?: string; - mergedLede?: string; - omit?: 'pull-request' | 'merge'; -}): Promise { - const root = await mkdtemp(join(tmpdir(), 'lede-decision-')); - const artifactDir = join(root, 'tickets', '1107'); - const dataDir = join(root, '_data'); - await mkdir(artifactDir, { recursive: true }); - await mkdir(dataDir, { recursive: true }); - - if (overrides.omit !== 'pull-request') { - const body = `## Body\n\n${section('What', AGENT_LEDE)}\n## Why\n\nThe motivation.\n`; - await writeArtifact(artifactDir, '20260730-174300Z_fixture_pull-request.md', body); - } - if (overrides.omit !== 'merge') { - await writeArtifact( - artifactDir, - '20260730-175638Z_fixture_merge.md', - section('Body', overrides.mergedLede ?? MERGED_LEDE), - ); - } - await writeArtifact( - artifactDir, - '20260730-174234Z_fixture_change-summary.md', - `---\ntype: fix\nscope: kb\nticket_id: '1107'\n---\n\n# Title\n`, - ); - - await writeFile(join(dataDir, 'lede-voice.md'), '# Lede voice\n\nDoctrine text.\n', 'utf8'); - await writeFile( - join(dataDir, 'work-types.json'), - JSON.stringify({ - types: [ - { key: 'feat', tier: 'public', aliases: ['feature'] }, - { key: 'fix', tier: 'public', aliases: [] }, - ], - }), - 'utf8', - ); - - return { - root, - artifactDir, - dataDir, - input: { - artifactDir, - dataDir, - pr: '1124', - mergeCommit: '35aa58d7', - type: overrides.type ?? 'feat', - scope: 'agents', - manifestPath: join(root, 'manifest.json'), - }, - }; -} - /** Narrows a resolver outcome to its success arm, failing the test with the reported reason when it is not one. */ function expectEpisode(outcome: ResolveEpisodeOutcome): LedeEpisode { if (outcome.ok) { @@ -246,14 +176,17 @@ function expectFailure(outcome: ResolveEpisodeOutcome): string { return outcome.error; } -/** Renders a second-level Markdown section with its heading. */ -function section(heading: string, body: string): string { - return `## ${heading}\n\n${body}\n`; -} - -/** Writes one artifact file into a directory. */ -async function writeArtifact(directory: string, filename: string, content: string): Promise { - await writeFile(join(directory, filename), content, 'utf8'); +/** Builds resolver input over a fixture, supplying the flags a merge caller would pass. */ +function inputFor(fixture: LedeFixture, overrides: { type?: string } = {}): Parameters[0] { + return { + artifactDir: fixture.artifactDir, + dataDir: fixture.dataDir, + pr: '1124', + mergeCommit: '35aa58d7', + type: overrides.type ?? 'feat', + scope: 'agents', + manifestPath: fixture.manifestPath, + }; } // endregion | Helpers diff --git a/packages/agents/src/capture-lede-decision/cli.ts b/packages/agents/src/capture-lede-decision/cli.ts new file mode 100644 index 00000000..e4196985 --- /dev/null +++ b/packages/agents/src/capture-lede-decision/cli.ts @@ -0,0 +1,306 @@ +/* eslint n/no-process-exit: off */ +/* eslint unicorn/no-process-exit: off */ +import { realpathSync } from 'node:fs'; +import path from 'node:path'; +import process from 'node:process'; +import type { Readable } from 'node:stream'; +import { fileURLToPath } from 'node:url'; + +import { ulid } from 'ulid'; + +import { writeEvent } from '../capture-event/write-event.ts'; +import { formatMissingStoreMessage } from '../kb-shared/format-missing-store.ts'; +import { formatUtcTimestamp } from '../kb-shared/note-helpers.ts'; +import { resolveCaptureTarget } from '../kb-shared/resolve-capture-target.ts'; +import { type FlagSpec, scanFlags, valueFlagMap } from '../lib/parse-flags.ts'; +import { readAll } from '../lib/stream-helpers.ts'; +import { resolveRepo } from '../shared/resolve-repo.ts'; +import { resolveSession } from '../shared/resolve-session.ts'; +import { prepareDecision } from './prepare-decision.ts'; +import { resolveEpisode } from './resolve-episode.ts'; +import { type DecisionResult, isLedeVerdict, LEDE_VERDICTS, type LedeVerdict } from './types.ts'; + +/** The flags this helper accepts; the comment comes from stdin, so it has no flag of its own. */ +const FLAGS: readonly FlagSpec[] = [ + { name: 'agent-lede-file', takesValue: true }, + { name: 'artifact-dir', takesValue: true }, + { name: 'data-dir', takesValue: true }, + { name: 'harness', takesValue: true }, + { name: 'inspect', takesValue: false }, + { name: 'manifest', takesValue: true }, + { name: 'merge-commit', takesValue: true }, + { name: 'merged-lede-file', takesValue: true }, + { name: 'pr', takesValue: true }, + { name: 'scope', takesValue: true }, + { name: 'store', takesValue: true }, + { name: 'ticket', takesValue: true }, + { name: 'type', takesValue: true }, + { name: 'verdict', takesValue: true }, +]; + +/** Parsed command-line invocation of the capture-lede-decision helper. */ +export interface ParsedArgs { + /** `inspect` resolves and reports the episode; `commit` records the author's verdict. */ + mode: 'inspect' | 'commit'; + /** The author's decision; `null` in inspect mode. */ + verdict: LedeVerdict | null; + artifactDir: string; + pr: string; + mergeCommit: string; + /** Directory holding `lede-voice.md` and `work-types.json`; `null` falls back to the helper's own `_data` sibling. */ + dataDir: string | null; + store: string | null; + type: string | null; + scope: string | null; + ticket: string | null; + agentLedeFile: string | null; + mergedLedeFile: string | null; + manifest: string | null; + harness: string | null; +} + +/** Executes the helper from `process.argv` and writes the JSON result to stdout. */ +async function main(): Promise { + try { + const result = await runDecision({ + argv: process.argv.slice(2), + stdin: process.stdin, + cwd: process.cwd(), + env: process.env, + now: new Date(), + defaultDataDir: path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '_data'), + }); + process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`capture-lede-decision: ${message}\n`); + process.exit(1); + } +} + +if (isEntryPoint()) { + await main(); +} + +/** + * Runs the helper end to end. In inspect mode it resolves the decision episode and reports it, writing nothing and + * reading no stdin, so a caller can present both ledes before asking the author to decide. In commit mode it resolves + * the same episode, reads the comment from stdin, and writes one event record to the named store. + * + * Every resolution failure and every store-resolution failure becomes a structured `{ ok: false, ... }` result, so a + * caller invoking this after an irreversible merge can report one line and continue. System failures (out-of-disk, + * permission denied) propagate to the caller's try/catch. + * + * @internal - Exported to allow testing. + */ +export async function runDecision(input: { + argv: readonly string[]; + stdin: Readable; + cwd: string; + env: NodeJS.ProcessEnv; + now: Date; + defaultDataDir: string; + home?: string; +}): Promise { + let args: ParsedArgs; + try { + args = parseArgs(input.argv); + } catch (error) { + return { ok: false, error: 'invalid-args', message: error instanceof Error ? error.message : String(error) }; + } + + const resolved = await resolveEpisode({ + artifactDir: args.artifactDir, + dataDir: args.dataDir ?? input.defaultDataDir, + pr: args.pr, + mergeCommit: args.mergeCommit, + ...(args.type !== null && { type: args.type }), + ...(args.scope !== null && { scope: args.scope }), + ...(args.ticket !== null && { ticket: args.ticket }), + ...(args.agentLedeFile !== null && { agentLedeFile: args.agentLedeFile }), + ...(args.mergedLedeFile !== null && { mergedLedeFile: args.mergedLedeFile }), + ...(args.manifest !== null && { manifestPath: args.manifest }), + }); + if (!resolved.ok) { + return { ok: false, error: resolved.error, message: resolved.message }; + } + + if (args.mode === 'inspect') { + return { ok: true, mode: 'inspect', episode: resolved.episode }; + } + + const verdict = args.verdict; + if (verdict === null) { + return { ok: false, error: 'invalid-args', message: '--verdict is required to record a decision' }; + } + + const target = await resolveCaptureTarget({ + explicitName: args.store, + ...(input.home !== undefined && { home: input.home }), + }); + if (!target.ok) { + return describeStoreFailure(target); + } + + const comment = await readAll(input.stdin); + const session = resolveSession(input.env); + const repo = await resolveRepo(input.cwd); + + const prep = prepareDecision({ + episode: resolved.episode, + verdict, + comment, + context: { + cwd: input.cwd, + ...(session !== undefined && { session }), + ...(repo !== undefined && { repo }), + }, + harness: args.harness, + id: ulid(), + capturedAt: formatUtcTimestamp(input.now), + }); + if (!prep.ok) { + return { + ok: false, + error: 'schema-validation', + message: `decision did not pass validation: ${prep.errors.join('; ')}`, + errors: prep.errors, + }; + } + + const recordPath = await writeEvent({ + storePath: target.store.path, + id: prep.prepared.id, + content: prep.prepared.content, + }); + + return { + ok: true, + mode: 'commit', + verdict, + id: prep.prepared.id, + capturedAt: prep.prepared.capturedAt, + path: recordPath, + store: target.store.name, + }; +} + +/** + * Parses the helper's argv. `--inspect` and `--verdict` select the mode and are mutually exclusive; exactly one must + * appear. `--artifact-dir`, `--pr`, and `--merge-commit` are always required, because a decision that cannot name the + * change it describes is not worth recording. Every other flag is optional: the work type, scope, and ticket fall back + * to the change-summary artifact, and the two lede overrides fall back to their artifacts. + * + * @internal - Exported to allow testing. + */ +export function parseArgs(argv: readonly string[]): ParsedArgs { + const { positionals, flags } = scanFlags(argv, FLAGS); + if (positionals[0] !== undefined) { + throw new Error(`unexpected argument: ${positionals[0]}`); + } + const raw = valueFlagMap(flags); + for (const [name, value] of Object.entries(raw)) { + if (value === '') { + throw new Error(`--${name} requires a value`); + } + } + + const inspect = flags.some((flag) => flag.name === 'inspect'); + const rawVerdict = raw.verdict; + if (inspect && rawVerdict !== undefined) { + throw new Error('--inspect and --verdict are mutually exclusive'); + } + if (!inspect && rawVerdict === undefined) { + throw new Error(`one of --inspect or --verdict <${LEDE_VERDICTS.join('|')}> is required`); + } + if (rawVerdict !== undefined && !isLedeVerdict(rawVerdict)) { + throw new Error(`--verdict must be one of ${LEDE_VERDICTS.join(', ')}`); + } + + return { + mode: inspect ? 'inspect' : 'commit', + verdict: isLedeVerdict(rawVerdict) ? rawVerdict : null, + artifactDir: requireFlag(raw, 'artifact-dir'), + pr: requireFlag(raw, 'pr'), + mergeCommit: requireFlag(raw, 'merge-commit'), + dataDir: raw['data-dir'] ?? null, + store: raw.store ?? null, + type: raw.type ?? null, + scope: raw.scope ?? null, + ticket: raw.ticket ?? null, + agentLedeFile: raw['agent-lede-file'] ?? null, + mergedLedeFile: raw['merged-lede-file'] ?? null, + manifest: raw.manifest ?? null, + harness: raw.harness ?? null, + }; +} + +// region | Helpers + +/** Maps a store-resolution failure onto the helper's structured result, preserving the resolver's categorical reason. */ +function describeStoreFailure( + resolved: Extract>, { ok: false }>, +): DecisionResult { + switch (resolved.reason) { + case 'missing-store': + return { ok: false, error: 'missing-store', message: formatMissingStoreMessage(resolved) }; + case 'not-registered': + return { + ok: false, + error: 'store-not-registered', + message: + resolved.registryError !== undefined + ? `could not load kb.yaml registry: ${resolved.registryError}` + : `event store "${resolved.requestedName}" is not registered in kb.yaml`, + }; + case 'readonly-store': + return { + ok: false, + error: 'readonly-store', + message: `event store "${resolved.name}" is marked readonly in kb.yaml; decisions are refused`, + }; + case 'no-default': + return { + ok: false, + error: 'no-default-store', + message: + resolved.registryError !== undefined + ? `could not resolve the default event store: ${resolved.registryError}` + : '--store @default was given but no default_kb is configured in kb.yaml', + }; + default: { + const _exhaustive: never = resolved; + throw new Error(`unhandled resolveCaptureTarget failure: ${JSON.stringify(_exhaustive)}`); + } + } +} + +/** + * Returns true when this module is the process entry point. Both sides are resolved through `realpathSync`, so a + * symlinked invocation path still matches. On a `realpathSync` failure the function emits a warning and returns + * `false`, matching the degrade-with-warning pattern the sibling helpers use. + */ +function isEntryPoint(): boolean { + const entry = process.argv[1]; + if (entry === undefined) { + return false; + } + try { + return realpathSync(fileURLToPath(import.meta.url)) === realpathSync(entry); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`capture-lede-decision: warning: could not determine entry point: ${message}\n`); + return false; + } +} + +/** Reads a required value-bearing flag, throwing a usage-style message when it is absent. */ +function requireFlag(raw: Record, name: string): string { + const value = raw[name]; + if (value === undefined) { + throw new Error(`--${name} is required`); + } + return value; +} + +// endregion | Helpers diff --git a/packages/agents/src/capture-lede-decision/prepare-decision.ts b/packages/agents/src/capture-lede-decision/prepare-decision.ts new file mode 100644 index 00000000..ca960522 --- /dev/null +++ b/packages/agents/src/capture-lede-decision/prepare-decision.ts @@ -0,0 +1,103 @@ +import { readNoteContent, renderNote } from '@codeassembly/kb/note-io'; +import { type KbEvent, parseEvent, renderEvent } from '@codeassembly/kb/records'; + +import type { LedeEpisode, LedeVerdict } from './types.ts'; + +/** A prepared decision ready to write: its ULID-keyed filename stem and the full rendered note content. */ +export interface PreparedDecision { + /** The decision's ULID, also the filename stem. */ + id: string; + /** ISO-8601 capture timestamp. */ + capturedAt: string; + /** The full note content (frontmatter fence plus body) to write. */ + content: string; +} + +/** The outcome of preparing a decision for write: the rendered note, or the errors that blocked it. */ +export type PrepareDecisionOutcome = { ok: true; prepared: PreparedDecision } | { ok: false; errors: string[] }; + +/** + * Composes a lede decision as a knowledge-base `event`, renders it through the record module's `renderEvent`, and + * validates the serialized note by re-parsing it. A decision is a third consumer of the event substrate rather than a + * record type of its own, so it carries the same typed spine every captured event does and rides its change identity, + * doctrine fingerprint, and provenance in `extra`. + * + * Tags carry the group (`lede-decision`), the work type under a `type:` namespace, and the verdict. The namespace is + * what keeps a work type from colliding with the topical tags an event already uses — a bare `fix` already means a + * solved-problem episode. + * + * The body carries the merged lede whenever the two texts differ, which is a fact about the change rather than a + * restatement of the verdict: the verdict records what the author says they did, and the body records what happened. + */ +export function prepareDecision(input: { + episode: LedeEpisode; + verdict: LedeVerdict; + comment: string; + context: { cwd: string; session?: string; repo?: string }; + harness: string | null; + id: string; + capturedAt: string; +}): PrepareDecisionOutcome { + const { episode, verdict, context, harness, id, capturedAt } = input; + const { identity } = episode; + + const extra: Record = { + type: identity.type, + tier: identity.tier, + scope: identity.scope, + pr: identity.pr, + 'merge-commit': identity.mergeCommit, + ...(identity.ticket !== undefined && { ticket: identity.ticket }), + 'doctrine-hash': episode.doctrineHash, + ...(episode.agentsVersion !== undefined && { 'agents-version': episode.agentsVersion }), + ...(context.repo !== undefined && { repo: context.repo }), + ...(harness !== null && { harness }), + }; + + const record: KbEvent = { + recordType: 'event', + id, + capturedAt, + ...(context.session !== undefined && { session: context.session }), + cwd: context.cwd, + summary: `Lede ${verdict} for ${identity.scope} #${identity.pr}`, + tags: ['lede-decision', `type:${identity.type}`, verdict], + addressedBy: [], + extra, + body: composeBody({ episode, comment: input.comment }), + }; + + const rendered = renderEvent(record); + const content = renderNote(rendered.fields, rendered.body); + + const errors = validate(content); + if (errors.length > 0) { + return { ok: false, errors }; + } + + return { ok: true, prepared: { id, capturedAt, content } }; +} + +// region | Helpers + +/** Renders the decision body: the agent's lede, the merged lede when it differs, and the comment when one was given. */ +function composeBody(input: { episode: LedeEpisode; comment: string }): string { + const sections = [`## Agent lede\n\n${input.episode.agentLede}`]; + if (input.episode.differ) { + sections.push(`## Merged lede\n\n${input.episode.mergedLede}`); + } + const comment = input.comment.trim(); + if (comment.length > 0) { + sections.push(`## Comment\n\n${comment}`); + } + return `${sections.join('\n\n')}\n`; +} + +/** Re-parses the rendered note and validates it as an `event` record, returning any validation errors. */ +function validate(content: string): string[] { + const { fields, body } = readNoteContent(content); + const result = parseEvent(fields, body); + return result.ok ? [] : result.errors; +} + +// endregion | Helpers diff --git a/packages/agents/src/capture-lede-decision/resolve-episode.ts b/packages/agents/src/capture-lede-decision/resolve-episode.ts index 584d7bc6..6dfd8a51 100644 --- a/packages/agents/src/capture-lede-decision/resolve-episode.ts +++ b/packages/agents/src/capture-lede-decision/resolve-episode.ts @@ -284,12 +284,12 @@ async function resolveIdentity(input: { const fallback = await readChangeSummaryFields(input.artifactDir); const type = input.type ?? fallback.type; - if (type === null || type === undefined) { + if (type === null) { return { ok: false, error: 'unresolved-identity', message: 'work type could not be resolved; pass --type' }; } const scope = input.scope ?? fallback.scope; - if (scope === null || scope === undefined) { + if (scope === null) { return { ok: false, error: 'unresolved-identity', message: 'scope could not be resolved; pass --scope' }; } @@ -312,7 +312,7 @@ async function resolveIdentity(input: { scope, pr: input.pr, mergeCommit: input.mergeCommit, - ...(ticket !== null && ticket !== undefined && { ticket }), + ...(ticket !== null && { ticket }), }, }; } diff --git a/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts b/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts new file mode 100644 index 00000000..c7f37d01 --- /dev/null +++ b/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts @@ -0,0 +1,82 @@ +import { mkdir, mkdtemp, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +/** The lede the fixture's pull-request artifact carries. */ +export const FIXTURE_AGENT_LEDE = 'Rulebooks can now address a file by linking to it.'; + +/** The lede the fixture's merge artifact carries, a revision of {@link FIXTURE_AGENT_LEDE}. */ +export const FIXTURE_MERGED_LEDE = + 'Rulebooks can now address a file by linking to it: a Markdown link reaches each harness.'; + +/** A temporary fixture tree: the ticket's artifact directory, the `_data` directory, and an install-manifest path. */ +export interface LedeFixture { + /** Temporary root holding everything the fixture created. */ + root: string; + artifactDir: string; + dataDir: string; + /** Path the fixture would write an install manifest to; absent unless a test writes one. */ + manifestPath: string; +} + +/** + * Builds a temporary ticket directory carrying a pull-request, merge, and change-summary artifact, plus a `_data` + * directory holding a doctrine file and a minimal work-type taxonomy. + * + * The change summary declares a work type and scope that differ from what a caller would normally pass as flags, so a + * test can tell a supplied flag from its artifact fallback. + */ +export async function createLedeFixture( + overrides: { + type?: string; + mergedLede?: string; + omit?: 'pull-request' | 'merge'; + } = {}, +): Promise { + const root = await mkdtemp(join(tmpdir(), 'lede-decision-')); + const artifactDir = join(root, 'tickets', '1107'); + const dataDir = join(root, '_data'); + await mkdir(artifactDir, { recursive: true }); + await mkdir(dataDir, { recursive: true }); + + if (overrides.omit !== 'pull-request') { + const body = `## Body\n\n${renderSection('What', FIXTURE_AGENT_LEDE)}\n## Why\n\nThe motivation.\n`; + await writeArtifact(artifactDir, '20260730-174300Z_fixture_pull-request.md', body); + } + if (overrides.omit !== 'merge') { + await writeArtifact( + artifactDir, + '20260730-175638Z_fixture_merge.md', + renderSection('Body', overrides.mergedLede ?? FIXTURE_MERGED_LEDE), + ); + } + await writeArtifact( + artifactDir, + '20260730-174234Z_fixture_change-summary.md', + "---\ntype: fix\nscope: kb\nticket_id: '1107'\n---\n\n# Title\n", + ); + + await writeFile(join(dataDir, 'lede-voice.md'), '# Lede voice\n\nDoctrine text.\n', 'utf8'); + await writeFile( + join(dataDir, 'work-types.json'), + JSON.stringify({ + types: [ + { key: 'feat', tier: 'public', aliases: ['feature'] }, + { key: 'fix', tier: 'public', aliases: [] }, + ], + }), + 'utf8', + ); + + return { root, artifactDir, dataDir, manifestPath: join(root, 'manifest.json') }; +} + +/** Renders a second-level Markdown section with its heading. */ +export function renderSection(heading: string, body: string): string { + return `## ${heading}\n\n${body}\n`; +} + +/** Writes one artifact file into a directory. */ +export async function writeArtifact(directory: string, filename: string, content: string): Promise { + await writeFile(join(directory, filename), content, 'utf8'); +} From 0c63e22243585f7ca88b7783fc677162e7c2e6d2 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 14:11:58 -0700 Subject: [PATCH 04/11] agents|internal: Read a ticket id the change summary wrote as a number Episode resolution now recovers the ticket id from a change summary whose frontmatter carries it unquoted, which is how a wholly numeric id is written. A string-only read dropped it, so a decision resolved without explicit flags reached the record with no ticket. A prefixed key such as `MAC-42` arrives as a string and is unaffected. --- .../__tests__/resolve-episode.test.ts | 12 ++++++++++++ .../src/capture-lede-decision/resolve-episode.ts | 14 +++++++++++++- .../test-utils/create-lede-fixture.ts | 4 +++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts index 82a7710a..25fa455a 100644 --- a/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts +++ b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts @@ -69,6 +69,18 @@ describe(resolveEpisode, () => { expect(episode.identity).toMatchObject({ type: 'fix', scope: 'kb', ticket: '1107' }); }); + it('reads a wholly numeric ticket id, which the change summary writes unquoted', async () => { + const fixture = await createLedeFixture({ ticketId: '1107' }); + + expect(expectEpisode(await resolveEpisode(inputFor(fixture))).identity.ticket).toBe('1107'); + }); + + it('reads a prefixed ticket key, which the change summary writes as a string', async () => { + const fixture = await createLedeFixture({ ticketId: 'MAC-42' }); + + expect(expectEpisode(await resolveEpisode(inputFor(fixture))).identity.ticket).toBe('MAC-42'); + }); + it('reads a lede from an override file rather than its artifact', async () => { const fixture = await createLedeFixture(); const overrideFile = join(fixture.root, 'override.md'); diff --git a/packages/agents/src/capture-lede-decision/resolve-episode.ts b/packages/agents/src/capture-lede-decision/resolve-episode.ts index 6dfd8a51..4617318f 100644 --- a/packages/agents/src/capture-lede-decision/resolve-episode.ts +++ b/packages/agents/src/capture-lede-decision/resolve-episode.ts @@ -229,10 +229,22 @@ async function readChangeSummaryFields( return { type: extractString(fields, 'type'), scope: extractString(fields, 'scope'), - ticket: extractString(fields, 'ticket_id'), + ticket: readIdentifier(fields, 'ticket_id'), }; } +/** + * Reads an identifier field that YAML may have typed as a number: a wholly numeric ticket id is written unquoted, so a + * string-only read would silently drop it, while a prefixed key such as `MAC-42` arrives as a string. + */ +function readIdentifier(fields: Record, key: string): string | null { + const value = fields[key]; + if (typeof value === 'number' && Number.isFinite(value)) { + return String(value); + } + return extractString(fields, key); +} + /** Reads a file as UTF-8, yielding `null` when it does not exist. */ async function readFileSafely(filePath: string): Promise { try { diff --git a/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts b/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts index c7f37d01..801a4a3c 100644 --- a/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts +++ b/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts @@ -31,6 +31,8 @@ export async function createLedeFixture( type?: string; mergedLede?: string; omit?: 'pull-request' | 'merge'; + /** Ticket id as the change summary spells it; a wholly numeric id is written unquoted, as the real artifact does. */ + ticketId?: string; } = {}, ): Promise { const root = await mkdtemp(join(tmpdir(), 'lede-decision-')); @@ -53,7 +55,7 @@ export async function createLedeFixture( await writeArtifact( artifactDir, '20260730-174234Z_fixture_change-summary.md', - "---\ntype: fix\nscope: kb\nticket_id: '1107'\n---\n\n# Title\n", + `---\ntype: fix\nscope: kb\nticket_id: ${overrides.ticketId ?? '1107'}\n---\n\n# Title\n`, ); await writeFile(join(dataDir, 'lede-voice.md'), '# Lede voice\n\nDoctrine text.\n', 'utf8'); From 04d586626172fa46343163aaa76e18fa080b7be8 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 14:11:59 -0700 Subject: [PATCH 05/11] agents|feat: Add a skill for recording what the author decided about a lede Adds `capture-lede-decision`, which records whether the lede an agent wrote for a pull request shipped as written or was rewritten before merge. It reads both ledes from the ticket's own artifacts, so neither has to be supplied, and it writes one knowledge-base event carrying the verdict, both texts, an optional free-text comment, and a fingerprint of the lede doctrine in force at the time. The corpus it builds holds positive signals only: a record exists because someone looked at a lede and decided, and declining to decide writes nothing. The absence of a record therefore means nothing was evaluated, never that a lede was accepted, which keeps a week spent shipping under time pressure from manufacturing approvals nobody gave. Recall the corpus as a group by its `lede-decision` tag, or one work type at a time. Because it holds only changes someone chose to evaluate, it reads what good looks like and what typically fails, and must not seed a comparison's fixture draw. --- .../skills/capture-lede-decision/SKILL.md | 128 ++++++++++++++++++ .../agents/scripts/bundle-skill-helpers.ts | 4 + 2 files changed, 132 insertions(+) create mode 100644 packages/agents/content/skills/capture-lede-decision/SKILL.md diff --git a/packages/agents/content/skills/capture-lede-decision/SKILL.md b/packages/agents/content/skills/capture-lede-decision/SKILL.md new file mode 100644 index 00000000..77da646d --- /dev/null +++ b/packages/agents/content/skills/capture-lede-decision/SKILL.md @@ -0,0 +1,128 @@ +--- +name: capture-lede-decision +description: Record the author's decision about a merged pull request's lede — accepted as the agent wrote it, or revised — into the lede-decision corpus. Use after a merge, or to record a pull request merged outside the merge flow. +user-invocable: true +--- + +# Capture a lede decision + +Record what the author decided about a lede: that the agent's `## What` shipped as written, or that it was rewritten before merge. A bundled helper does the mechanical work — it reads the lede the agent published and the lede that merged from the ticket's own artifacts, fingerprints the doctrine that governed the first, and writes one event record. You present the pair and relay the author's decision. + +**Announce at start:** "Using capture-lede-decision to record the lede decision for #{pr}." + +## The corpus stores positive signals only + +A record exists because the author looked at the lede and decided. There are exactly two decisions, and no third: + +- **`accepted`** — the author read the agent's lede and shipped it as written. +- **`revised`** — the author rewrote it before merge. + +Declining to decide writes nothing. **The absence of a record carries no meaning, and in particular is not an acceptance**: a merge nobody evaluated is indistinguishable from a merge this skill never ran on. Never infer a verdict, and never record one the author did not give — a lede that shipped unchanged under time pressure is not an accepted lede, and recording it as one is the single failure that would make the corpus useless. + +For the same reason, the corpus is outcome-selected: it holds only changes someone chose to evaluate. It is the right population for reading what good looks like and what typically fails, and the wrong one for measuring whether guidance helps. A comparison's fixture draw must never read it. + +## Arguments + +| Argument | Description | Required | +| -------------------- | --------------------------------------------------------------------------------------- | -------- | +| `--artifact-dir` | The ticket's artifact directory, holding the pull-request and merge artifacts. | Yes | +| `--pr` | The pull-request number. | Yes | +| `--merge-commit` | The merge commit's SHA. | Yes | +| `--inspect` | Resolve and report the episode without writing. Mutually exclusive with `--verdict`. | Mode | +| `--verdict` | The author's decision: `accepted` or `revised`. Mutually exclusive with `--inspect`. | Mode | +| `--store` | Registry name of the event store, or `@default` for the `default_kb`. Needed to record. | Yes | +| `--type` | Work type. Falls back to the change summary's frontmatter. | No | +| `--scope` | Package or surface scope. Falls back to the change summary's frontmatter. | No | +| `--ticket` | Ticket id. Falls back to the change summary's frontmatter. | No | +| `--merged-lede-file` | File holding the merged lede, for a pull request that wrote no merge artifact. | No | +| `--agent-lede-file` | File holding the agent's lede, for a pull request that wrote no pull-request artifact. | No | +| `--harness` | The agent platform (`claude`, `rovodev`); install-injected — keep as-is. | Injected | + +Exactly one of `--inspect` and `--verdict` must appear. The author's comment is read from stdin to EOF; an empty comment is allowed and records no comment section. + +## Runtime dependencies + +- **`node` ≥ 24** — the bundled helper inherits the Node version floor of `@codeassembly/kb`. + +## Process + +### 1. Inspect the episode + +```bash +node {harness_home_dir}/skills/capture-lede-decision/capture-lede-decision.mjs \ + --inspect \ + --artifact-dir \ + --pr \ + --merge-commit \ + [--type ] [--scope ] [--ticket ] +``` + +The helper prints a JSON object to stdout: `ok: true` with `episode` on success, or `ok: false` with `error` and `message`. Inspecting writes nothing and needs no store, so it can never block or alter a merge that already happened. + +On `ok: false`, report the `message` on one line and stop. The merge has already succeeded — do not present this as a merge failure, and do not retry. + +### 2. Present the pair and ask + +Read `episode.differ`. Present the ledes and ask, following [option format](#option-format): + +When `differ` is `true`, show the agent's lede and the merged lede, then ask: + +1. ■■□ Record it as a revision (add a comment to explain what was wrong, if you want) +2. ■□□ Skip — this was a content change, or not a decision worth recording + +When `differ` is `false`, show the single lede and ask: + +1. ■■□ Record it as accepted — you read it and shipped it as written +2. ■□□ Skip — you did not evaluate it + +Ask once. A skip is a complete answer, not a prompt to re-ask or to persuade: the corpus is better off one record smaller than holding a decision the author did not make. + +### 3. Record the decision + +On a skip, write nothing and say nothing further. + +On a decision, pipe the author's comment (empty when they gave none) to the helper: + +```bash +cat <<'EOF' | node {harness_home_dir}/skills/capture-lede-decision/capture-lede-decision.mjs \ + --verdict \ + --store \ + --harness {harness_id} \ + --artifact-dir \ + --pr \ + --merge-commit \ + [--type ] [--scope ] [--ticket ] + +EOF +``` + +Relay the comment verbatim. It is free text on purpose: naming which doctrine rule the fix invoked is the refinement pass's job, and a rule list offered at capture time would presuppose which rules matter, which is the question the corpus exists to answer. + +Report the written `path` on success. + +## The record + +One event per decision, in the named store: + +- **Tags** — `lede-decision`, `type:{work type}`, and the verdict. Recall the corpus as a group with `kb-retrieve-events --tag lede-decision`, and by work type with `--tag type:feat`. +- **Frontmatter** — the work type, tier, and scope; the pull-request number, merge commit, and ticket; `doctrine-hash`, a digest of the lede doctrine in force when the agent wrote; and `agents-version` when the install manifest supplies one. +- **Body** — `## Agent lede`, then `## Merged lede` whenever the two texts differ, then `## Comment` when one was given. + +`doctrine-hash` is what groups records by doctrine generation. Nothing is recorded at install time to make that work: the mapping from a digest back to the commit that introduced it stays recoverable by re-hashing the doctrine file's own history. + +## Handling failures + +Route by the `error` code: + +- `no-artifact-dir`, `no-agent-lede`, `no-merged-lede` — the ticket's artifacts do not carry both ledes. Report and stop; supply `--agent-lede-file` or `--merged-lede-file` only when the text is genuinely in hand. +- `no-doctrine` — the installed doctrine file is unreadable. Report it as an install problem. +- `unresolved-identity` — the work type, tier, or scope could not be resolved. The message names which; pass the corresponding flag. +- `invalid-args` — surface the message and propose a corrected invocation. +- `missing-store`, `store-not-registered`, `readonly-store`, `no-default-store` — the destination could not be resolved; the message lists the registered stores. +- `schema-validation` — surface the `errors`. + +## Completion + +Either one written record at the reported path, or nothing at all. There is no third outcome, and no record is ever written without the author's decision. + + diff --git a/packages/agents/scripts/bundle-skill-helpers.ts b/packages/agents/scripts/bundle-skill-helpers.ts index d53fda11..e86a218f 100644 --- a/packages/agents/scripts/bundle-skill-helpers.ts +++ b/packages/agents/scripts/bundle-skill-helpers.ts @@ -64,6 +64,10 @@ export const targets: BundleTarget[] = [ entry: 'src/capture-event/cli.ts', outFile: 'content/skills/capture-event/capture-event.mjs', }, + { + entry: 'src/capture-lede-decision/cli.ts', + outFile: 'content/skills/capture-lede-decision/capture-lede-decision.mjs', + }, { entry: 'src/kb-update-events/cli.ts', outFile: 'content/skills/kb-update-events/kb-update-events.mjs', From 878a7791d39a1350cfccdbd768341c28218d8460 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 14:13:18 -0700 Subject: [PATCH 06/11] agents|feat: Ask what the author decided about the lede after a merge Merging a pull request now ends by asking whether the lede that shipped was the one the agent wrote or a rewrite, and records the answer. The question comes after the merge, so declining costs a data point and nothing else, and a failure to record can never read as a failure to merge. --- .../agents/content/skills/merge-pr/SKILL.md | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/agents/content/skills/merge-pr/SKILL.md b/packages/agents/content/skills/merge-pr/SKILL.md index 74f676f3..afb98b32 100644 --- a/packages/agents/content/skills/merge-pr/SKILL.md +++ b/packages/agents/content/skills/merge-pr/SKILL.md @@ -187,7 +187,25 @@ Pass the following inputs to the selected delegate per the delegate interface: The orchestrator never passes ambiguous-status dimensions or `prompt` sentinels to the delegate — all values are concrete by this point. -After the delegate returns, emit `skill.completed` (payload `{"outcome":"merged"}`, or `{"outcome":"stopped: "}` when the delegate stopped or failed) per [Lifecycle events](#lifecycle-events). +If the delegate stopped or failed, emit `skill.completed` (payload `{"outcome":"stopped: "}`) per [Lifecycle events](#lifecycle-events) and stop. Otherwise capture the merge commit SHA from the delegate's completion report and continue. + +### 10. Record the lede decision + +The merge has already happened, so this step can only add a record. Declining costs a data point and nothing else, and nothing here can undo or re-run the merge — never present a failure at this step as a merge failure. + +Invoke `{skill:capture-lede-decision}` with: + +| Input | Value | +| ------------------- | ------------------------------------------------------------------------------------------------------------------------- | +| `--artifact-dir` | `{artifact_base_dir}/projects/{project_slug}/tickets/{ticket_id}/` | +| `--pr` | Resolved PR number | +| `--merge-commit` | The merge commit SHA from the delegate's completion report | +| `--type`, `--scope` | The values resolved in step 3, as settled at the approval gate | +| `--store` | `codeassembly` — the project's agent-guidance KB. Pass a different store only when the user directs the record elsewhere. | + +That skill owns the prompt and the record: it asks once, writes one event on a decision, and writes nothing on a skip. Do not ask again, and do not infer a verdict from whether the ledes differ — a lede that shipped unchanged under time pressure is not an accepted lede. + +Then emit `skill.completed` (payload `{"outcome":"merged"}`) per [Lifecycle events](#lifecycle-events). ## Important From 8277fd02c603dd07b9c1c6d4b32dc55eb3578bc8 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 14:19:58 -0700 Subject: [PATCH 07/11] agents|refactor: Split mode resolution out of the decision command's parser Deciding which mode an invocation selects, and validating that it names exactly one, now sits in its own function rather than inline among the field reads. Resolving the directory the doctrine ships in likewise reads as one named step. --- .../__tests__/cli.test.ts | 8 +++- .../__tests__/resolve-episode.test.ts | 39 ++++++++++------ .../agents/src/capture-lede-decision/cli.ts | 45 +++++++++++++------ 3 files changed, 63 insertions(+), 29 deletions(-) diff --git a/packages/agents/src/capture-lede-decision/__tests__/cli.test.ts b/packages/agents/src/capture-lede-decision/__tests__/cli.test.ts index 1e9319c5..710d41c2 100644 --- a/packages/agents/src/capture-lede-decision/__tests__/cli.test.ts +++ b/packages/agents/src/capture-lede-decision/__tests__/cli.test.ts @@ -140,7 +140,9 @@ describe(runDecision, () => { const fixture = await createLedeFixture(); const argv = ['--inspect', ...flagsFor({ ...fixture, artifactDir: join(fixture.root, 'absent') })]; - expect(expectFailure(await runDecision(runInput({ argv, fixture })))).toBe('no-artifact-dir'); + const result = await runDecision(runInput({ argv, fixture })); + + expect(expectFailure(result)).toBe('no-artifact-dir'); }); it('refuses to record a decision with no named store', async () => { @@ -156,7 +158,9 @@ describe(runDecision, () => { it('reports an invalid invocation without touching the artifacts', async () => { const fixture = await createLedeFixture(); - expect(expectFailure(await runDecision(runInput({ argv: ['--inspect'], fixture })))).toBe('invalid-args'); + const result = await runDecision(runInput({ argv: ['--inspect'], fixture })); + + expect(expectFailure(result)).toBe('invalid-args'); }); }); diff --git a/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts index 25fa455a..7b057a7f 100644 --- a/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts +++ b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts @@ -29,20 +29,20 @@ describe(resolveEpisode, () => { it('reports the ledes as differing when the merged text was rewritten', async () => { const fixture = await createLedeFixture(); - expect(expectEpisode(await resolveEpisode(inputFor(fixture))).differ).toBe(true); + expect((await resolveFor(fixture)).differ).toBe(true); }); it('reports the ledes as identical when they differ only by whitespace', async () => { const fixture = await createLedeFixture({ mergedLede: `${FIXTURE_AGENT_LEDE.replace(' ', '\n ')}\n` }); - expect(expectEpisode(await resolveEpisode(inputFor(fixture))).differ).toBe(false); + expect((await resolveFor(fixture)).differ).toBe(false); }); it('reads the newest artifact of each kind', async () => { const fixture = await createLedeFixture(); await writeArtifact(fixture.artifactDir, '20260731-090000Z_later_merge.md', renderSection('Body', 'A later lede.')); - expect(expectEpisode(await resolveEpisode(inputFor(fixture))).mergedLede).toBe('A later lede.'); + expect((await resolveFor(fixture)).mergedLede).toBe('A later lede.'); }); it('finds an artifact nested in a run subdirectory', async () => { @@ -51,13 +51,13 @@ describe(resolveEpisode, () => { await mkdir(runDir, { recursive: true }); await writeArtifact(runDir, '20260731-100000Z_run_merge.md', renderSection('Body', 'A lede from a run.')); - expect(expectEpisode(await resolveEpisode(inputFor(fixture))).mergedLede).toBe('A lede from a run.'); + expect((await resolveFor(fixture)).mergedLede).toBe('A lede from a run.'); }); it('derives the tier from a work type declared as an alias', async () => { const fixture = await createLedeFixture(); - expect(expectEpisode(await resolveEpisode(inputFor(fixture, { type: 'feature' }))).identity.tier).toBe('public'); + expect((await resolveFor(fixture, { type: 'feature' })).identity.tier).toBe('public'); }); it('falls back to the change summary for a type and scope the caller did not pass', async () => { @@ -72,13 +72,13 @@ describe(resolveEpisode, () => { it('reads a wholly numeric ticket id, which the change summary writes unquoted', async () => { const fixture = await createLedeFixture({ ticketId: '1107' }); - expect(expectEpisode(await resolveEpisode(inputFor(fixture))).identity.ticket).toBe('1107'); + expect((await resolveFor(fixture)).identity.ticket).toBe('1107'); }); it('reads a prefixed ticket key, which the change summary writes as a string', async () => { const fixture = await createLedeFixture({ ticketId: 'MAC-42' }); - expect(expectEpisode(await resolveEpisode(inputFor(fixture))).identity.ticket).toBe('MAC-42'); + expect((await resolveFor(fixture)).identity.ticket).toBe('MAC-42'); }); it('reads a lede from an override file rather than its artifact', async () => { @@ -94,14 +94,14 @@ describe(resolveEpisode, () => { it('omits the agents version when the install manifest is unreadable', async () => { const fixture = await createLedeFixture(); - expect(expectEpisode(await resolveEpisode(inputFor(fixture))).agentsVersion).toBeUndefined(); + expect((await resolveFor(fixture)).agentsVersion).toBeUndefined(); }); it('reads the agents version from the install manifest', async () => { const fixture = await createLedeFixture(); await writeFile(fixture.manifestPath, JSON.stringify({ shared: { version: '1.2.3' } }), 'utf8'); - expect(expectEpisode(await resolveEpisode(inputFor(fixture))).agentsVersion).toBe('1.2.3'); + expect((await resolveFor(fixture)).agentsVersion).toBe('1.2.3'); }); it('reports a missing artifact directory', async () => { @@ -115,26 +115,34 @@ describe(resolveEpisode, () => { it('reports an absent pull-request artifact separately from an absent merge artifact', async () => { const fixture = await createLedeFixture({ omit: 'pull-request' }); - expect(expectFailure(await resolveEpisode(inputFor(fixture)))).toBe('no-agent-lede'); + const outcome = await resolveEpisode(inputFor(fixture)); + + expect(expectFailure(outcome)).toBe('no-agent-lede'); }); it('reports a merge artifact carrying no body section', async () => { const fixture = await createLedeFixture({ mergedLede: '' }); - expect(expectFailure(await resolveEpisode(inputFor(fixture)))).toBe('no-merged-lede'); + const outcome = await resolveEpisode(inputFor(fixture)); + + expect(expectFailure(outcome)).toBe('no-merged-lede'); }); it('reports an unreadable doctrine file', async () => { const fixture = await createLedeFixture(); await rm(join(fixture.dataDir, 'lede-voice.md')); - expect(expectFailure(await resolveEpisode(inputFor(fixture)))).toBe('no-doctrine'); + const outcome = await resolveEpisode(inputFor(fixture)); + + expect(expectFailure(outcome)).toBe('no-doctrine'); }); it('reports a work type the taxonomy does not declare', async () => { const fixture = await createLedeFixture(); - expect(expectFailure(await resolveEpisode(inputFor(fixture, { type: 'invented' })))).toBe('unresolved-identity'); + const outcome = await resolveEpisode(inputFor(fixture, { type: 'invented' })); + + expect(expectFailure(outcome)).toBe('unresolved-identity'); }); }); @@ -201,4 +209,9 @@ function inputFor(fixture: LedeFixture, overrides: { type?: string } = {}): Para }; } +/** Resolves an episode over a fixture and narrows it to the success arm, so an assertion reads as one call. */ +async function resolveFor(fixture: LedeFixture, overrides: { type?: string } = {}): Promise { + return expectEpisode(await resolveEpisode(inputFor(fixture, overrides))); +} + // endregion | Helpers diff --git a/packages/agents/src/capture-lede-decision/cli.ts b/packages/agents/src/capture-lede-decision/cli.ts index e4196985..aaa1f755 100644 --- a/packages/agents/src/capture-lede-decision/cli.ts +++ b/packages/agents/src/capture-lede-decision/cli.ts @@ -68,7 +68,7 @@ async function main(): Promise { cwd: process.cwd(), env: process.env, now: new Date(), - defaultDataDir: path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '_data'), + defaultDataDir: resolveDefaultDataDir(), }); process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); } catch (error) { @@ -205,21 +205,11 @@ export function parseArgs(argv: readonly string[]): ParsedArgs { } } - const inspect = flags.some((flag) => flag.name === 'inspect'); - const rawVerdict = raw.verdict; - if (inspect && rawVerdict !== undefined) { - throw new Error('--inspect and --verdict are mutually exclusive'); - } - if (!inspect && rawVerdict === undefined) { - throw new Error(`one of --inspect or --verdict <${LEDE_VERDICTS.join('|')}> is required`); - } - if (rawVerdict !== undefined && !isLedeVerdict(rawVerdict)) { - throw new Error(`--verdict must be one of ${LEDE_VERDICTS.join(', ')}`); - } + const { mode, verdict } = resolveMode({ inspect: flags.some((flag) => flag.name === 'inspect'), raw }); return { - mode: inspect ? 'inspect' : 'commit', - verdict: isLedeVerdict(rawVerdict) ? rawVerdict : null, + mode, + verdict, artifactDir: requireFlag(raw, 'artifact-dir'), pr: requireFlag(raw, 'pr'), mergeCommit: requireFlag(raw, 'merge-commit'), @@ -294,6 +284,33 @@ function isEntryPoint(): boolean { } } +/** Resolves the `_data` directory shipped beside the installed helper, holding the doctrine and the work-type taxonomy. */ +function resolveDefaultDataDir(): string { + const helperDir = path.dirname(fileURLToPath(import.meta.url)); + return path.resolve(helperDir, '..', '_data'); +} + +/** + * Resolves which mode the invocation selects, rejecting an argv that names both or neither. Exactly one of `--inspect` + * and `--verdict` must appear, so the mode and the verdict are decided together rather than validated separately. + */ +function resolveMode(input: { inspect: boolean; raw: Record }): { + mode: 'inspect' | 'commit'; + verdict: LedeVerdict | null; +} { + const rawVerdict = input.raw.verdict; + if (rawVerdict !== undefined && input.inspect) { + throw new Error('--inspect and --verdict are mutually exclusive'); + } + if (rawVerdict === undefined && !input.inspect) { + throw new Error(`one of --inspect or --verdict <${LEDE_VERDICTS.join('|')}> is required`); + } + if (rawVerdict !== undefined && !isLedeVerdict(rawVerdict)) { + throw new Error(`--verdict must be one of ${LEDE_VERDICTS.join(', ')}`); + } + return input.inspect ? { mode: 'inspect', verdict: null } : { mode: 'commit', verdict: rawVerdict ?? null }; +} + /** Reads a required value-bearing flag, throwing a usage-style message when it is absent. */ function requireFlag(raw: Record, name: string): string { const value = raw[name]; From 8d09bb4936a1a702de382b949ae0616b32be3868 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 14:21:56 -0700 Subject: [PATCH 08/11] agents|docs: Give the route for recording a pull request merged elsewhere The capture skill now spells out how to supply the merged lede for a pull request that never went through the merge flow, and so wrote no merge artifact for the command to read. --- .../content/skills/capture-lede-decision/SKILL.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/agents/content/skills/capture-lede-decision/SKILL.md b/packages/agents/content/skills/capture-lede-decision/SKILL.md index 77da646d..a37ea7d5 100644 --- a/packages/agents/content/skills/capture-lede-decision/SKILL.md +++ b/packages/agents/content/skills/capture-lede-decision/SKILL.md @@ -100,6 +100,16 @@ Relay the comment verbatim. It is free text on purpose: naming which doctrine ru Report the written `path` on success. +### Recording a pull request merged outside the merge flow + +Such a pull request wrote no merge artifact, so the merged lede has to be supplied. Fetch the body, take its `## What` section, write that to a file, and pass the file: + +```bash +gh pr view --json body --jq '.body' > "$TMPDIR/merged-body.md" +``` + +Then hand the `## What` section's text to `--merged-lede-file` and continue from step 2. Everything else resolves from the ticket's artifacts as usual. + ## The record One event per decision, in the named store: From 1728a0837b555ed5b2c0a70260137d572e5d23bb Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 19:02:04 -0700 Subject: [PATCH 09/11] agents|fix: Capture a lede decision only from a real merge and lede A pull request merged outside the merge flow records its actual lede. The documented route supplies a command that extracts the `## What` section into the file `--merged-lede-file` reads, so the whole pull-request body no longer lands in the corpus as the lede. `merge-pr` asks about a lede decision only after a merge that produced a commit. A Bitbucket merge, which the delegate leaves to the user, is skipped rather than reported as a capture failure. --- .../content/skills/capture-lede-decision/SKILL.md | 11 ++++++++--- packages/agents/content/skills/merge-pr/SKILL.md | 6 ++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/agents/content/skills/capture-lede-decision/SKILL.md b/packages/agents/content/skills/capture-lede-decision/SKILL.md index a37ea7d5..a248ee4a 100644 --- a/packages/agents/content/skills/capture-lede-decision/SKILL.md +++ b/packages/agents/content/skills/capture-lede-decision/SKILL.md @@ -102,13 +102,18 @@ Report the written `path` on success. ### Recording a pull request merged outside the merge flow -Such a pull request wrote no merge artifact, so the merged lede has to be supplied. Fetch the body, take its `## What` section, write that to a file, and pass the file: +Such a pull request wrote no merge artifact, so the merged lede has to be supplied. A lede file is read whole and recorded as the lede, with none of the heading extraction the artifact path applies: a file holding the entire pull-request body records the entire body as the lede. Extract the `## What` section as the file is written: ```bash -gh pr view --json body --jq '.body' > "$TMPDIR/merged-body.md" +gh pr view --json body --jq '.body' \ + | awk '{ sub(/\r$/, "") } + tolower($0) ~ /^## what[[:space:]]*$/ { capturing = 1; next } + /^## / { capturing = 0 } + capturing' \ + > "$TMPDIR/merged-lede.md" ``` -Then hand the `## What` section's text to `--merged-lede-file` and continue from step 2. Everything else resolves from the ticket's artifacts as usual. +Pass that file to `--merged-lede-file` and continue from step 2. Everything else resolves from the ticket's artifacts as usual. ## The record diff --git a/packages/agents/content/skills/merge-pr/SKILL.md b/packages/agents/content/skills/merge-pr/SKILL.md index afb98b32..0e722fd2 100644 --- a/packages/agents/content/skills/merge-pr/SKILL.md +++ b/packages/agents/content/skills/merge-pr/SKILL.md @@ -187,11 +187,13 @@ Pass the following inputs to the selected delegate per the delegate interface: The orchestrator never passes ambiguous-status dimensions or `prompt` sentinels to the delegate — all values are concrete by this point. -If the delegate stopped or failed, emit `skill.completed` (payload `{"outcome":"stopped: "}`) per [Lifecycle events](#lifecycle-events) and stop. Otherwise capture the merge commit SHA from the delegate's completion report and continue. +If the delegate stopped or failed, emit `skill.completed` (payload `{"outcome":"stopped: "}`) per [Lifecycle events](#lifecycle-events) and stop. Otherwise capture the merge commit SHA from the delegate's completion report, if it carries one, and continue. ### 10. Record the lede decision -The merge has already happened, so this step can only add a record. Declining costs a data point and nothing else, and nothing here can undo or re-run the merge — never present a failure at this step as a merge failure. +Skip this step when the delegate's completion report carries no merge commit SHA: nothing merged, so there is no shipped lede to decide about. The Bitbucket delegate is the standing case, since it prints the resolved values and exits successfully without merging. Emit `skill.completed` (payload `{"outcome":"not merged"}`) per [Lifecycle events](#lifecycle-events) and stop. + +Otherwise the merge has already happened, so this step can only add a record. Declining costs a data point and nothing else, and nothing here can undo or re-run the merge — never present a failure at this step as a merge failure. Invoke `{skill:capture-lede-decision}` with: From 25cf96ae112c569f5ec3ba1f04d93676e7a541bc Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 19:02:09 -0700 Subject: [PATCH 10/11] agents|internal: Resolve the install manifest against the injected home A caller that supplies a home directory has it honored for every lookup the lede-decision capture makes, including the install manifest that names the recorded agents version. An unset `HOME` resolves to the real home rather than to the filesystem root. --- .../__tests__/resolve-episode.test.ts | 16 ++++++++++++++++ packages/agents/src/capture-lede-decision/cli.ts | 1 + .../src/capture-lede-decision/resolve-episode.ts | 13 +++++++++---- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts index 7b057a7f..1245a733 100644 --- a/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts +++ b/packages/agents/src/capture-lede-decision/__tests__/resolve-episode.test.ts @@ -104,6 +104,22 @@ describe(resolveEpisode, () => { expect((await resolveFor(fixture)).agentsVersion).toBe('1.2.3'); }); + it('resolves the default manifest path against the caller-supplied home', async () => { + const fixture = await createLedeFixture(); + const home = join(fixture.root, 'home'); + await mkdir(join(home, '.codeassembly'), { recursive: true }); + await writeFile( + join(home, '.codeassembly', 'agents-manifest.json'), + JSON.stringify({ shared: { version: '4.5.6' } }), + 'utf8', + ); + const { manifestPath: _manifestPath, ...withoutManifest } = inputFor(fixture); + + const episode = expectEpisode(await resolveEpisode({ ...withoutManifest, home })); + + expect(episode.agentsVersion).toBe('4.5.6'); + }); + it('reports a missing artifact directory', async () => { const fixture = await createLedeFixture(); diff --git a/packages/agents/src/capture-lede-decision/cli.ts b/packages/agents/src/capture-lede-decision/cli.ts index aaa1f755..5aeb6594 100644 --- a/packages/agents/src/capture-lede-decision/cli.ts +++ b/packages/agents/src/capture-lede-decision/cli.ts @@ -120,6 +120,7 @@ export async function runDecision(input: { ...(args.agentLedeFile !== null && { agentLedeFile: args.agentLedeFile }), ...(args.mergedLedeFile !== null && { mergedLedeFile: args.mergedLedeFile }), ...(args.manifest !== null && { manifestPath: args.manifest }), + ...(input.home !== undefined && { home: input.home }), }); if (!resolved.ok) { return { ok: false, error: resolved.error, message: resolved.message }; diff --git a/packages/agents/src/capture-lede-decision/resolve-episode.ts b/packages/agents/src/capture-lede-decision/resolve-episode.ts index 4617318f..5f8b63f8 100644 --- a/packages/agents/src/capture-lede-decision/resolve-episode.ts +++ b/packages/agents/src/capture-lede-decision/resolve-episode.ts @@ -1,7 +1,7 @@ import { createHash } from 'node:crypto'; import { readdir, readFile, stat } from 'node:fs/promises'; +import { homedir } from 'node:os'; import path from 'node:path'; -import process from 'node:process'; import { readNoteContent } from '@codeassembly/kb/note-io'; @@ -41,6 +41,8 @@ export async function resolveEpisode(input: { mergedLedeFile?: string; /** Install manifest supplying the agents-package version; defaults to the user-global manifest. */ manifestPath?: string; + /** Home directory the user-global manifest path resolves against; defaults to the real home. */ + home?: string; }): Promise { if (!(await isDirectory(input.artifactDir))) { return { ok: false, error: 'no-artifact-dir', message: `artifact directory not found: ${input.artifactDir}` }; @@ -83,7 +85,7 @@ export async function resolveEpisode(input: { return identity; } - const agentsVersion = await readAgentsVersion(input.manifestPath); + const agentsVersion = await readAgentsVersion({ manifestPath: input.manifestPath, home: input.home }); return { ok: true, @@ -186,8 +188,11 @@ function normalizeLede(value: string): string { } /** Reads the installed agents-package version from the install manifest; `null` when it is absent or unreadable. */ -async function readAgentsVersion(manifestPath: string | undefined): Promise { - const resolved = manifestPath ?? path.join(process.env.HOME ?? '', '.codeassembly', 'agents-manifest.json'); +async function readAgentsVersion(input: { + manifestPath: string | undefined; + home: string | undefined; +}): Promise { + const resolved = input.manifestPath ?? path.join(input.home ?? homedir(), '.codeassembly', 'agents-manifest.json'); const content = await readFileSafely(resolved); if (content === null) { return null; From f4e08f55b7bf7daaa9ba02237ae2703154612c2e Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 30 Jul 2026 19:02:14 -0700 Subject: [PATCH 11/11] agents|tests: Remove an unread type override from the lede fixture The lede fixture accepts only the overrides it applies. A test that asks it for a work type fails to compile rather than silently receiving the fixture's own. --- .../src/capture-lede-decision/test-utils/create-lede-fixture.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts b/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts index 801a4a3c..4a8b5b50 100644 --- a/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts +++ b/packages/agents/src/capture-lede-decision/test-utils/create-lede-fixture.ts @@ -28,7 +28,6 @@ export interface LedeFixture { */ export async function createLedeFixture( overrides: { - type?: string; mergedLede?: string; omit?: 'pull-request' | 'merge'; /** Ticket id as the change summary spells it; a wholly numeric id is written unquoted, as the real artifact does. */