From e4ff1da4ea9ac2abc91cf784516eaf73f56061e3 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 06:17:34 -0700 Subject: [PATCH 01/10] agents|feat: Refuse kb-add writes into KBs marked readonly in kb.yaml `kb-add` now refuses to write into a knowledge base whose `kb.yaml` entry sets `readonly: true`. Previously the flag was advisory: `kb-add` did not consult it, so a write into a vault marked read-only would succeed. --- .../agents/src/kb-add/__tests__/cli.test.ts | 33 +++++++ packages/agents/src/kb-add/cli.ts | 42 ++++++--- packages/agents/src/kb-add/types.ts | 27 +++--- .../fixtures/discovered-kb/.kb/.gitkeep | 0 .../discovered-kb/.kb/tag-aliases.yaml | 0 .../home-readonly-default/.agents/kb.yaml | 13 +++ .../home-readonly-named/.agents/kb.yaml | 12 +++ .../home-with-default/.agents/kb.yaml | 0 .../malformed-registry/.agents/kb.yaml | 0 .../fixtures/project-kb/.agents/kb.yaml | 0 .../fixtures/project-kb/.kb/.gitkeep | 0 .../__tests__/fixtures/vault-a/.kb/.gitkeep | 0 .../__tests__/fixtures/vault-b/.kb/.gitkeep | 0 .../fixtures/vault-readonly/.kb/.gitkeep | 0 .../__tests__/resolve-writable-kb.test.ts} | 93 +++++++++++++++---- .../resolve-writable-kb.ts} | 46 +++++++-- 16 files changed, 216 insertions(+), 50 deletions(-) rename packages/agents/src/{kb-add => kb-shared}/__tests__/fixtures/discovered-kb/.kb/.gitkeep (100%) rename packages/agents/src/{kb-add => kb-shared}/__tests__/fixtures/discovered-kb/.kb/tag-aliases.yaml (100%) create mode 100644 packages/agents/src/kb-shared/__tests__/fixtures/home-readonly-default/.agents/kb.yaml create mode 100644 packages/agents/src/kb-shared/__tests__/fixtures/home-readonly-named/.agents/kb.yaml rename packages/agents/src/{kb-add => kb-shared}/__tests__/fixtures/home-with-default/.agents/kb.yaml (100%) rename packages/agents/src/{kb-add => kb-shared}/__tests__/fixtures/malformed-registry/.agents/kb.yaml (100%) rename packages/agents/src/{kb-add => kb-shared}/__tests__/fixtures/project-kb/.agents/kb.yaml (100%) rename packages/agents/src/{kb-add => kb-shared}/__tests__/fixtures/project-kb/.kb/.gitkeep (100%) rename packages/agents/src/{kb-add => kb-shared}/__tests__/fixtures/vault-a/.kb/.gitkeep (100%) rename packages/agents/src/{kb-add => kb-shared}/__tests__/fixtures/vault-b/.kb/.gitkeep (100%) create mode 100644 packages/agents/src/kb-shared/__tests__/fixtures/vault-readonly/.kb/.gitkeep rename packages/agents/src/{kb-add/__tests__/resolve-kb.test.ts => kb-shared/__tests__/resolve-writable-kb.test.ts} (50%) rename packages/agents/src/{kb-add/resolve-kb.ts => kb-shared/resolve-writable-kb.ts} (58%) diff --git a/packages/agents/src/kb-add/__tests__/cli.test.ts b/packages/agents/src/kb-add/__tests__/cli.test.ts index 289323e8..78279b59 100644 --- a/packages/agents/src/kb-add/__tests__/cli.test.ts +++ b/packages/agents/src/kb-add/__tests__/cli.test.ts @@ -245,6 +245,39 @@ describe(runAdd, () => { expect(content).toBe('pre-existing\n'); }); + it('returns readonly-kb when the explicit --kb names a readonly registry entry', async () => { + // Stand up an isolated HOME with a `.agents/kb.yaml` declaring the only writable target as readonly. + // runAdd resolves through resolveWritableKb, so the refusal surfaces as a top-level readonly-kb error + // without ever touching disk inside the KB. + const kbPath = await makeKb(); + const homeDir = await mkdtemp(join(tmpdir(), 'kb-add-readonly-')); + await mkdir(join(homeDir, '.agents'), { recursive: true }); + await writeFile( + join(homeDir, '.agents', 'kb.yaml'), + `kbs:\n locked:\n path: ${kbPath}\n readonly: true\n`, + 'utf8', + ); + + const result = await runAdd({ + argv: ['--kb', 'locked', '--type', 'howto', '--title', 'Refused'], + stdin: bodyStream(''), + // startDir avoids the KB so discovery does not produce a writable fallback. + startDir: homeDir, + now: NOW, + home: homeDir, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('readonly-kb'); + expect(result.details?.readonlyKbName).toBe('locked'); + expect(result.details?.readonlyKbPath).toBe(kbPath); + } + // No note should have landed in the readonly KB. + const entries = await readdir(kbPath); + expect(entries.filter((name) => name !== '.kb')).toEqual([]); + }); + it('returns invalid-args when --title is missing', async () => { const kbPath = await makeKb(); diff --git a/packages/agents/src/kb-add/cli.ts b/packages/agents/src/kb-add/cli.ts index 247e3e92..b2559309 100644 --- a/packages/agents/src/kb-add/cli.ts +++ b/packages/agents/src/kb-add/cli.ts @@ -10,8 +10,8 @@ import type { AliasMap, KbRoot } from '@codeassembly/kb-core'; import { loadSchema } from '@codeassembly/kb-core/schema'; import { loadAliases } from '@codeassembly/kb-core/tags'; +import { resolveWritableKb } from '../kb-shared/resolve-writable-kb.ts'; import { prepareNote } from './prepare-note.ts'; -import { resolveKb } from './resolve-kb.ts'; import type { AddResult, ParsedArgs } from './types.ts'; import { writeNote } from './write-note.ts'; @@ -115,24 +115,40 @@ export async function runAdd(input: { return { ok: false, error: 'invalid-args', message: error instanceof Error ? error.message : String(error) }; } - const resolved = await resolveKb({ + const resolved = await resolveWritableKb({ startDir: input.startDir, explicitKb: args.kb, ...(input.home !== undefined && { home: input.home }), }); if (!resolved.ok) { - const failure: AddResult = { - ok: false, - error: 'no-kb-resolvable', - message: - resolved.requestedKb === null - ? 'no .kb/ discovered, no registry default configured, and no --kb supplied' - : `--kb "${resolved.requestedKb}" does not match any registered knowledge base`, - }; - if (resolved.requestedKb !== null) { - failure.details = { requestedKb: resolved.requestedKb }; + switch (resolved.reason) { + case 'no-kb-resolvable': { + const failure: AddResult = { + ok: false, + error: 'no-kb-resolvable', + message: + resolved.requestedKb === null + ? 'no .kb/ discovered, no registry default configured, and no --kb supplied' + : `--kb "${resolved.requestedKb}" does not match any registered knowledge base`, + }; + if (resolved.requestedKb !== null) { + failure.details = { requestedKb: resolved.requestedKb }; + } + return failure; + } + case 'readonly-kb': + return { + ok: false, + error: 'readonly-kb', + message: `knowledge base "${resolved.kbName}" is marked readonly in kb.yaml; writes are refused`, + details: { readonlyKbName: resolved.kbName, readonlyKbPath: resolved.kbPath }, + }; + default: { + // Exhaustiveness check: a new ResolveKbOutcome variant will surface here at compile time. + const _exhaustive: never = resolved; + throw new Error(`unhandled resolveWritableKb failure: ${JSON.stringify(_exhaustive)}`); + } } - return failure; } const kb = resolved.kb; diff --git a/packages/agents/src/kb-add/types.ts b/packages/agents/src/kb-add/types.ts index 75d29dbb..d12b03df 100644 --- a/packages/agents/src/kb-add/types.ts +++ b/packages/agents/src/kb-add/types.ts @@ -6,6 +6,8 @@ import type { Finding, Frontmatter } from '@codeassembly/kb-core'; +import type { ResolvedKb } from '../kb-shared/resolve-writable-kb.ts'; + /** Parsed command-line invocation of the kb-add helper. */ export interface ParsedArgs { /** Optional explicit KB name; when set, wins over discovery and registry-default. */ @@ -22,16 +24,6 @@ export interface ParsedArgs { lastVerified: string | null; } -/** A knowledge base resolved as the write target. */ -export interface ResolvedKb { - /** The KB's display name. `null` for a `.kb/`-discovered KB with no registry entry. */ - name: string | null; - /** Absolute path to the KB's root directory. */ - path: string; - /** Which selection rule fired. */ - source: 'explicit' | 'discovered' | 'registry-default'; -} - /** The prepared note ready to be written: the rendered frontmatter, the body, and the canonicalization audit trail. */ export interface PreparedNote { /** Frontmatter with canonical tags, UTC dates filled in, and the optional `last-verified` field merged into `extra`. */ @@ -69,7 +61,13 @@ export interface AddFailure { } /** Categorical error codes the helper can return without an unexpected throw. */ -export type AddErrorCode = 'no-kb-resolvable' | 'invalid-args' | 'invalid-title' | 'schema-validation' | 'collision'; +export type AddErrorCode = + | 'no-kb-resolvable' + | 'invalid-args' + | 'invalid-title' + | 'schema-validation' + | 'collision' + | 'readonly-kb'; /** Per-error structured details. */ export interface AddErrorDetails { @@ -79,7 +77,14 @@ export interface AddErrorDetails { findings?: Finding[]; /** Name of the explicit KB that did not resolve, set when `error: 'no-kb-resolvable'` after `--kb` was supplied. */ requestedKb?: string; + /** Registry name of the readonly KB that refused the write, set when `error: 'readonly-kb'`. */ + readonlyKbName?: string; + /** Absolute path of the readonly KB that refused the write, set when `error: 'readonly-kb'`. */ + readonlyKbPath?: string; } /** The helper's full stdout payload: a discriminated union on `ok`. */ export type AddResult = AddSuccess | AddFailure; + +// Re-export so existing kb-add consumers don't need to learn the kb-shared path. +export type { ResolvedKb } from '../kb-shared/resolve-writable-kb.ts'; diff --git a/packages/agents/src/kb-add/__tests__/fixtures/discovered-kb/.kb/.gitkeep b/packages/agents/src/kb-shared/__tests__/fixtures/discovered-kb/.kb/.gitkeep similarity index 100% rename from packages/agents/src/kb-add/__tests__/fixtures/discovered-kb/.kb/.gitkeep rename to packages/agents/src/kb-shared/__tests__/fixtures/discovered-kb/.kb/.gitkeep diff --git a/packages/agents/src/kb-add/__tests__/fixtures/discovered-kb/.kb/tag-aliases.yaml b/packages/agents/src/kb-shared/__tests__/fixtures/discovered-kb/.kb/tag-aliases.yaml similarity index 100% rename from packages/agents/src/kb-add/__tests__/fixtures/discovered-kb/.kb/tag-aliases.yaml rename to packages/agents/src/kb-shared/__tests__/fixtures/discovered-kb/.kb/tag-aliases.yaml diff --git a/packages/agents/src/kb-shared/__tests__/fixtures/home-readonly-default/.agents/kb.yaml b/packages/agents/src/kb-shared/__tests__/fixtures/home-readonly-default/.agents/kb.yaml new file mode 100644 index 00000000..b6996d63 --- /dev/null +++ b/packages/agents/src/kb-shared/__tests__/fixtures/home-readonly-default/.agents/kb.yaml @@ -0,0 +1,13 @@ +# User-global registry fixture for readonly enforcement. +# The default-marked entry is also readonly so resolveWritableKb must refuse to fall back to it. +# A second non-default writable entry exists so the registry is non-empty. +# Paths are relative to this file's directory (`home-readonly-default/.claude/`). +kbs: + readonly-default: + path: ../../vault-readonly + description: Fixture readonly default vault + default: true + readonly: true + also-writable: + path: ../../vault-b + description: Non-default writable vault diff --git a/packages/agents/src/kb-shared/__tests__/fixtures/home-readonly-named/.agents/kb.yaml b/packages/agents/src/kb-shared/__tests__/fixtures/home-readonly-named/.agents/kb.yaml new file mode 100644 index 00000000..2ebbabb9 --- /dev/null +++ b/packages/agents/src/kb-shared/__tests__/fixtures/home-readonly-named/.agents/kb.yaml @@ -0,0 +1,12 @@ +# User-global registry fixture that registers vault-readonly by name (without making it the default) +# alongside a writable default. Exercises explicit-flag readonly refusal and discovered-path readonly refusal. +# Paths are relative to this file's directory (`home-readonly-named/.claude/`). +kbs: + writable-default: + path: ../../vault-b + description: Writable default vault + default: true + readonly-named: + path: ../../vault-readonly + description: Readonly vault registered by name + readonly: true diff --git a/packages/agents/src/kb-add/__tests__/fixtures/home-with-default/.agents/kb.yaml b/packages/agents/src/kb-shared/__tests__/fixtures/home-with-default/.agents/kb.yaml similarity index 100% rename from packages/agents/src/kb-add/__tests__/fixtures/home-with-default/.agents/kb.yaml rename to packages/agents/src/kb-shared/__tests__/fixtures/home-with-default/.agents/kb.yaml diff --git a/packages/agents/src/kb-add/__tests__/fixtures/malformed-registry/.agents/kb.yaml b/packages/agents/src/kb-shared/__tests__/fixtures/malformed-registry/.agents/kb.yaml similarity index 100% rename from packages/agents/src/kb-add/__tests__/fixtures/malformed-registry/.agents/kb.yaml rename to packages/agents/src/kb-shared/__tests__/fixtures/malformed-registry/.agents/kb.yaml diff --git a/packages/agents/src/kb-add/__tests__/fixtures/project-kb/.agents/kb.yaml b/packages/agents/src/kb-shared/__tests__/fixtures/project-kb/.agents/kb.yaml similarity index 100% rename from packages/agents/src/kb-add/__tests__/fixtures/project-kb/.agents/kb.yaml rename to packages/agents/src/kb-shared/__tests__/fixtures/project-kb/.agents/kb.yaml diff --git a/packages/agents/src/kb-add/__tests__/fixtures/project-kb/.kb/.gitkeep b/packages/agents/src/kb-shared/__tests__/fixtures/project-kb/.kb/.gitkeep similarity index 100% rename from packages/agents/src/kb-add/__tests__/fixtures/project-kb/.kb/.gitkeep rename to packages/agents/src/kb-shared/__tests__/fixtures/project-kb/.kb/.gitkeep diff --git a/packages/agents/src/kb-add/__tests__/fixtures/vault-a/.kb/.gitkeep b/packages/agents/src/kb-shared/__tests__/fixtures/vault-a/.kb/.gitkeep similarity index 100% rename from packages/agents/src/kb-add/__tests__/fixtures/vault-a/.kb/.gitkeep rename to packages/agents/src/kb-shared/__tests__/fixtures/vault-a/.kb/.gitkeep diff --git a/packages/agents/src/kb-add/__tests__/fixtures/vault-b/.kb/.gitkeep b/packages/agents/src/kb-shared/__tests__/fixtures/vault-b/.kb/.gitkeep similarity index 100% rename from packages/agents/src/kb-add/__tests__/fixtures/vault-b/.kb/.gitkeep rename to packages/agents/src/kb-shared/__tests__/fixtures/vault-b/.kb/.gitkeep diff --git a/packages/agents/src/kb-shared/__tests__/fixtures/vault-readonly/.kb/.gitkeep b/packages/agents/src/kb-shared/__tests__/fixtures/vault-readonly/.kb/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/packages/agents/src/kb-add/__tests__/resolve-kb.test.ts b/packages/agents/src/kb-shared/__tests__/resolve-writable-kb.test.ts similarity index 50% rename from packages/agents/src/kb-add/__tests__/resolve-kb.test.ts rename to packages/agents/src/kb-shared/__tests__/resolve-writable-kb.test.ts index f5cc35c2..70a8a091 100644 --- a/packages/agents/src/kb-add/__tests__/resolve-kb.test.ts +++ b/packages/agents/src/kb-shared/__tests__/resolve-writable-kb.test.ts @@ -2,21 +2,24 @@ import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; -import { resolveKb } from '../resolve-kb.ts'; +import { resolveWritableKb } from '../resolve-writable-kb.ts'; const FIXTURES = join(import.meta.dirname, 'fixtures'); const PROJECT_KB = join(FIXTURES, 'project-kb'); const DISCOVERED_KB = join(FIXTURES, 'discovered-kb'); const VAULT_A = join(FIXTURES, 'vault-a'); const VAULT_B = join(FIXTURES, 'vault-b'); +const VAULT_READONLY = join(FIXTURES, 'vault-readonly'); const HOME_WITH_DEFAULT = join(FIXTURES, 'home-with-default'); const HOME_MALFORMED = join(FIXTURES, 'malformed-registry'); +const HOME_READONLY_DEFAULT = join(FIXTURES, 'home-readonly-default'); +const HOME_READONLY_NAMED = join(FIXTURES, 'home-readonly-named'); // A home directory with no `.agents/kb.yaml`, so the user-global registry resolves empty. const HOME_EMPTY = FIXTURES; -describe(resolveKb, () => { +describe(resolveWritableKb, () => { it('returns the discovered KB when one is found, preferring it over registry-default', async () => { - const result = await resolveKb({ startDir: PROJECT_KB, explicitKb: null, home: HOME_EMPTY }); + const result = await resolveWritableKb({ startDir: PROJECT_KB, explicitKb: null, home: HOME_EMPTY }); expect(result).toEqual({ ok: true, @@ -27,7 +30,7 @@ describe(resolveKb, () => { it('annotates a discovered KB with its registry name when its path matches a registered entry', async () => { // VAULT_A is registered in HOME_WITH_DEFAULT as `named-vault-a`. Discovery from VAULT_A returns VAULT_A, // whose absolute path then matches the registry entry, so `name` is populated rather than null. - const result = await resolveKb({ startDir: VAULT_A, explicitKb: null, home: HOME_WITH_DEFAULT }); + const result = await resolveWritableKb({ startDir: VAULT_A, explicitKb: null, home: HOME_WITH_DEFAULT }); expect(result).toEqual({ ok: true, @@ -38,16 +41,16 @@ describe(resolveKb, () => { it('returns name: null for a discovered KB whose path is not in the registry', async () => { // DISCOVERED_KB is not registered in HOME_WITH_DEFAULT, so the discovered match has no registry entry // and the name falls back to null. - const result = await resolveKb({ startDir: DISCOVERED_KB, explicitKb: null, home: HOME_WITH_DEFAULT }); + const result = await resolveWritableKb({ startDir: DISCOVERED_KB, explicitKb: null, home: HOME_WITH_DEFAULT }); - expect(result.ok).toBe(true); - if (result.ok) { - expect(result.kb).toEqual({ name: null, path: DISCOVERED_KB, source: 'discovered' }); - } + expect(result).toEqual({ + ok: true, + kb: { name: null, path: DISCOVERED_KB, source: 'discovered' }, + }); }); it('returns the registry default when no .kb/ is discovered', async () => { - const result = await resolveKb({ startDir: FIXTURES, explicitKb: null, home: HOME_WITH_DEFAULT }); + const result = await resolveWritableKb({ startDir: FIXTURES, explicitKb: null, home: HOME_WITH_DEFAULT }); expect(result).toEqual({ ok: true, @@ -56,7 +59,7 @@ describe(resolveKb, () => { }); it('returns the explicit KB when --kb names a registered entry, overriding discovery and default', async () => { - const result = await resolveKb({ startDir: PROJECT_KB, explicitKb: 'vault-b', home: HOME_EMPTY }); + const result = await resolveWritableKb({ startDir: PROJECT_KB, explicitKb: 'vault-b', home: HOME_EMPTY }); expect(result).toEqual({ ok: true, @@ -65,19 +68,19 @@ describe(resolveKb, () => { }); it('returns no-kb-resolvable when --kb names an entry that does not exist', async () => { - const result = await resolveKb({ startDir: PROJECT_KB, explicitKb: 'nonexistent', home: HOME_EMPTY }); + const result = await resolveWritableKb({ startDir: PROJECT_KB, explicitKb: 'nonexistent', home: HOME_EMPTY }); expect(result).toEqual({ ok: false, reason: 'no-kb-resolvable', requestedKb: 'nonexistent' }); }); it('returns no-kb-resolvable when no .kb/, no default, and no explicit KB is available', async () => { - const result = await resolveKb({ startDir: '/', explicitKb: null, home: HOME_EMPTY }); + const result = await resolveWritableKb({ startDir: '/', explicitKb: null, home: HOME_EMPTY }); expect(result).toEqual({ ok: false, reason: 'no-kb-resolvable', requestedKb: null }); }); it('uses the discovered KB even when a registry default is also configured', async () => { - const result = await resolveKb({ startDir: DISCOVERED_KB, explicitKb: null, home: HOME_WITH_DEFAULT }); + const result = await resolveWritableKb({ startDir: DISCOVERED_KB, explicitKb: null, home: HOME_WITH_DEFAULT }); expect(result).toEqual({ ok: true, @@ -88,19 +91,75 @@ describe(resolveKb, () => { it('degrades a malformed user-global registry to an empty config rather than throwing', async () => { // HOME_MALFORMED contains a syntactically invalid `.agents/kb.yaml`. The helper must swallow the parse // error and surface a structured result — here, the no-kb-resolvable failure for a startDir with no `.kb/` - // marker — rather than letting the throw escape `resolveKb`. - const result = await resolveKb({ startDir: '/', explicitKb: null, home: HOME_MALFORMED }); + // marker — rather than letting the throw escape `resolveWritableKb`. + const result = await resolveWritableKb({ startDir: '/', explicitKb: null, home: HOME_MALFORMED }); expect(result).toEqual({ ok: false, reason: 'no-kb-resolvable', requestedKb: null }); }); it('degrades a malformed user-global registry while still honoring a discovered KB', async () => { // Even with a malformed registry, discovery should still succeed and the result should not throw. - const result = await resolveKb({ startDir: DISCOVERED_KB, explicitKb: null, home: HOME_MALFORMED }); + const result = await resolveWritableKb({ startDir: DISCOVERED_KB, explicitKb: null, home: HOME_MALFORMED }); expect(result).toEqual({ ok: true, kb: { name: null, path: DISCOVERED_KB, source: 'discovered' }, }); }); + + it('refuses a readonly registry-default with readonly-kb', async () => { + const result = await resolveWritableKb({ startDir: FIXTURES, explicitKb: null, home: HOME_READONLY_DEFAULT }); + + expect(result).toEqual({ + ok: false, + reason: 'readonly-kb', + kbName: 'readonly-default', + kbPath: VAULT_READONLY, + }); + }); + + it('refuses an explicit --kb that names a readonly entry with readonly-kb', async () => { + const result = await resolveWritableKb({ + startDir: FIXTURES, + explicitKb: 'readonly-named', + home: HOME_READONLY_NAMED, + }); + + expect(result).toEqual({ + ok: false, + reason: 'readonly-kb', + kbName: 'readonly-named', + kbPath: VAULT_READONLY, + }); + }); + + it('refuses a discovered KB whose path matches a readonly registry entry with readonly-kb', async () => { + // VAULT_READONLY is registered as `readonly-named` (readonly: true) in HOME_READONLY_NAMED. Discovery from + // VAULT_READONLY returns its own path, which then matches the readonly registry entry. + const result = await resolveWritableKb({ + startDir: VAULT_READONLY, + explicitKb: null, + home: HOME_READONLY_NAMED, + }); + + expect(result).toEqual({ + ok: false, + reason: 'readonly-kb', + kbName: 'readonly-named', + kbPath: VAULT_READONLY, + }); + }); + + it('allows an explicit --kb naming the writable entry when a readonly default also exists', async () => { + const result = await resolveWritableKb({ + startDir: FIXTURES, + explicitKb: 'also-writable', + home: HOME_READONLY_DEFAULT, + }); + + expect(result).toEqual({ + ok: true, + kb: { name: 'also-writable', path: VAULT_B, source: 'explicit' }, + }); + }); }); diff --git a/packages/agents/src/kb-add/resolve-kb.ts b/packages/agents/src/kb-shared/resolve-writable-kb.ts similarity index 58% rename from packages/agents/src/kb-add/resolve-kb.ts rename to packages/agents/src/kb-shared/resolve-writable-kb.ts index dac06ae9..69e58db1 100644 --- a/packages/agents/src/kb-add/resolve-kb.ts +++ b/packages/agents/src/kb-shared/resolve-writable-kb.ts @@ -3,23 +3,42 @@ import process from 'node:process'; import type { KbConfig } from '@codeassembly/kb-core'; import { findKbRoot, loadKbConfig } from '@codeassembly/kb-core/discovery'; -import type { ResolvedKb } from './types.ts'; +/** A knowledge base resolved as the write target. */ +export interface ResolvedKb { + /** The KB's display name. `null` for a `.kb/`-discovered KB with no registry entry. */ + name: string | null; + /** Absolute path to the KB's root directory. */ + path: string; + /** Which selection rule fired. */ + source: 'explicit' | 'discovered' | 'registry-default'; +} -/** The selection outcome: a resolved KB, or a categorical failure the caller turns into a structured error. */ +/** + * The selection outcome: a resolved writable KB, or a categorical failure the caller turns into a structured + * error. + * + * - `no-kb-resolvable`: no `.kb/` discovered, no registry default, and either no `--kb` or a `--kb` that did not + * match any registered entry. + * - `readonly-kb`: the resolved KB is registered with `readonly: true`. Always carries the resolved name and path + * so the caller can surface them in its structured error. + */ export type ResolveKbOutcome = | { ok: true; kb: ResolvedKb } - | { ok: false; reason: 'no-kb-resolvable'; requestedKb: string | null }; + | { ok: false; reason: 'no-kb-resolvable'; requestedKb: string | null } + | { ok: false; reason: 'readonly-kb'; kbName: string; kbPath: string }; /** - * Resolves the single knowledge base to write into. + * Resolves the single knowledge base to write into and refuses read-only KBs. * * Precedence: `--kb ` (explicit) beats `.kb/` (discovered), which beats the registry's default-marked entry. - * Returns a categorical failure when nothing resolves, or when an explicit `--kb` does not match any registered entry. + * After a KB is selected, the matching `kb.yaml` entry's `readonly` flag is consulted: a `true` value refuses the + * write with `'readonly-kb'`. A discovered KB with no registry entry has no metadata to consult and is assumed + * writable. * * `home` overrides the directory the user-global `kb.yaml` is read from; it defaults to the real `$HOME` * and exists so tests can isolate registry resolution from the developer's environment. */ -export async function resolveKb(input: { +export async function resolveWritableKb(input: { startDir: string; explicitKb: string | null; home?: string; @@ -34,12 +53,18 @@ export async function resolveKb(input: { if (match === undefined) { return { ok: false, reason: 'no-kb-resolvable', requestedKb: input.explicitKb }; } + if (match.readonly === true) { + return { ok: false, reason: 'readonly-kb', kbName: match.name, kbPath: match.path }; + } return { ok: true, kb: { name: match.name, path: match.path, source: 'explicit' } }; } const discovered = await findKbRoot({ startDir: input.startDir }); if (discovered !== null) { const registryMatch = config.entries.find((entry) => entry.path === discovered.path); + if (registryMatch?.readonly === true) { + return { ok: false, reason: 'readonly-kb', kbName: registryMatch.name, kbPath: registryMatch.path }; + } return { ok: true, kb: { @@ -52,6 +77,9 @@ export async function resolveKb(input: { const defaultEntry = config.entries.find((entry) => entry.default === true); if (defaultEntry !== undefined) { + if (defaultEntry.readonly === true) { + return { ok: false, reason: 'readonly-kb', kbName: defaultEntry.name, kbPath: defaultEntry.path }; + } return { ok: true, kb: { name: defaultEntry.name, path: defaultEntry.path, source: 'registry-default' }, @@ -67,8 +95,8 @@ export async function resolveKb(input: { * Loads the merged `kb.yaml` registry, degrading a malformed or unreadable registry to an empty config and emitting * a warning to stderr so the operator can see why the registry did not contribute entries. * - * A defective project- or user-level `kb.yaml` would otherwise throw out of `resolveKb` and break the structured - * `AddResult` contract that every other failure path honors. Without the warning, a permission error or YAML + * A defective project- or user-level `kb.yaml` would otherwise throw out of `resolveWritableKb` and break the + * structured result contract that every other failure path honors. Without the warning, a permission error or YAML * defect looked identical to "no config file at all," which made the resulting `no-kb-resolvable` failure hard * to diagnose. */ @@ -80,7 +108,7 @@ async function loadKbConfigSafely(input: { projectDir: string; home?: string }): }); } catch (error) { const message = error instanceof Error ? error.message : String(error); - process.stderr.write(`kb-add: warning: could not load kb.yaml registry: ${message}\n`); + process.stderr.write(`kb-shared: warning: could not load kb.yaml registry: ${message}\n`); return { entries: [], sources: {} }; } } From 8632f7fc1213a680e535ed32c6a843ae4e14e2be Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 06:22:09 -0700 Subject: [PATCH 02/10] agents|feat: Scaffold kb-edit helper with argv parser for five mutually-exclusive ops Add the kb-edit CLI surface: a positional note path plus exactly one operation flag (`--bump-updated`, `--verify`, `--append`, `--retag`, `--supersede-with`). Combining two operation flags, omitting the path, or supplying a value-bearing flag without its value is rejected with `invalid-args`. The orchestration body is a placeholder that returns `invalid-args` for any parsed input. Subsequent commits wire in note loading, single-file operations, and the two-file supersede operation. --- .../agents/src/kb-edit/__tests__/cli.test.ts | 89 +++++++ packages/agents/src/kb-edit/cli.ts | 229 ++++++++++++++++++ packages/agents/src/kb-edit/types.ts | 106 ++++++++ 3 files changed, 424 insertions(+) create mode 100644 packages/agents/src/kb-edit/__tests__/cli.test.ts create mode 100644 packages/agents/src/kb-edit/cli.ts create mode 100644 packages/agents/src/kb-edit/types.ts diff --git a/packages/agents/src/kb-edit/__tests__/cli.test.ts b/packages/agents/src/kb-edit/__tests__/cli.test.ts new file mode 100644 index 00000000..4fe30e2c --- /dev/null +++ b/packages/agents/src/kb-edit/__tests__/cli.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from 'vitest'; + +import { parseArgs } from '../cli.ts'; + +describe(parseArgs, () => { + it('parses --bump-updated with a positional path', () => { + const parsed = parseArgs(['notes/foo.md', '--bump-updated']); + + expect(parsed).toEqual({ operation: 'bump-updated', path: 'notes/foo.md' }); + }); + + it('parses --verify with a positional path', () => { + const parsed = parseArgs(['/abs/path/foo.md', '--verify']); + + expect(parsed).toEqual({ operation: 'verify', path: '/abs/path/foo.md' }); + }); + + it('parses --append with a positional path', () => { + const parsed = parseArgs(['foo.md', '--append']); + + expect(parsed).toEqual({ operation: 'append', path: 'foo.md' }); + }); + + it('parses --retag with a comma-separated list, trimming whitespace and dropping empties', () => { + const parsed = parseArgs(['foo.md', '--retag', 'one, two ,three,,']); + + expect(parsed).toEqual({ operation: 'retag', path: 'foo.md', tags: ['one', 'two', 'three'] }); + }); + + it('parses --retag with an inline = value', () => { + const parsed = parseArgs(['foo.md', '--retag=a,b']); + + expect(parsed).toEqual({ operation: 'retag', path: 'foo.md', tags: ['a', 'b'] }); + }); + + it('parses --supersede-with as a single value', () => { + const parsed = parseArgs(['old.md', '--supersede-with', 'new.md']); + + expect(parsed).toEqual({ operation: 'supersede-with', path: 'old.md', newPath: 'new.md' }); + }); + + it('parses --supersede-with with an inline = value', () => { + const parsed = parseArgs(['old.md', '--supersede-with=new.md']); + + expect(parsed).toEqual({ operation: 'supersede-with', path: 'old.md', newPath: 'new.md' }); + }); + + it('accepts the operation flag before the positional path', () => { + const parsed = parseArgs(['--bump-updated', 'foo.md']); + + expect(parsed).toEqual({ operation: 'bump-updated', path: 'foo.md' }); + }); + + it('throws when no operation flag is supplied', () => { + expect(() => parseArgs(['foo.md'])).toThrow(/one operation flag is required/); + }); + + it('throws when two operation flags are combined', () => { + expect(() => parseArgs(['foo.md', '--bump-updated', '--verify'])).toThrow(/mutually exclusive/); + }); + + it('throws when --retag and --supersede-with are combined', () => { + expect(() => parseArgs(['foo.md', '--retag', 'a', '--supersede-with', 'new.md'])).toThrow(/mutually exclusive/); + }); + + it('throws when the positional path is missing', () => { + expect(() => parseArgs(['--bump-updated'])).toThrow(/missing required /); + }); + + it('throws when an extra positional argument is supplied', () => { + expect(() => parseArgs(['foo.md', 'bar.md', '--bump-updated'])).toThrow(/unexpected extra positional/); + }); + + it('throws on an unknown flag', () => { + expect(() => parseArgs(['foo.md', '--bogus'])).toThrow(/unknown flag/); + }); + + it('throws when --retag has no value', () => { + expect(() => parseArgs(['foo.md', '--retag'])).toThrow(/--retag requires a value/); + }); + + it('throws when --retag is followed by another flag instead of a value', () => { + expect(() => parseArgs(['foo.md', '--retag', '--bump-updated'])).toThrow(/--retag requires a value/); + }); + + it('throws when --supersede-with has no value', () => { + expect(() => parseArgs(['foo.md', '--supersede-with'])).toThrow(/--supersede-with requires a value/); + }); +}); diff --git a/packages/agents/src/kb-edit/cli.ts b/packages/agents/src/kb-edit/cli.ts new file mode 100644 index 00000000..39914bf7 --- /dev/null +++ b/packages/agents/src/kb-edit/cli.ts @@ -0,0 +1,229 @@ +/* eslint n/no-process-exit: off */ +/* eslint unicorn/no-process-exit: off */ +import { realpathSync } from 'node:fs'; +import process from 'node:process'; +import type { Readable } from 'node:stream'; +import { fileURLToPath } from 'node:url'; + +import type { EditResult, OperationName, ParsedArgs } from './types.ts'; + +/** Operation flag → operation name. Order is the documented surface order in SKILL.md. */ +const OPERATION_FLAGS = [ + { flag: '--bump-updated', name: 'bump-updated', takesValue: false }, + { flag: '--verify', name: 'verify', takesValue: false }, + { flag: '--append', name: 'append', takesValue: false }, + { flag: '--retag', name: 'retag', takesValue: true }, + { flag: '--supersede-with', name: 'supersede-with', takesValue: true }, +] as const satisfies readonly { flag: string; name: OperationName; takesValue: boolean }[]; + +/** Executes the helper from `process.argv` and writes the JSON result to stdout. */ +async function main(): Promise { + try { + const result = await runEdit({ + argv: process.argv.slice(2), + stdin: process.stdin, + now: new Date(), + }); + process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); + // The helper's contract is exit 0 with a structured `{ ok: false, ... }` for recoverable failures. + // System failures (unexpected throws) take the catch arm below. + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`kb-edit: ${message}\n`); + process.exit(1); + } +} + +if (isEntryPoint()) { + await main(); +} + +/** + * Parses the helper's argv. + * + * Layout: exactly one positional `` plus exactly one operation flag. `--retag` and `--supersede-with` take an + * inline or following value; `--bump-updated`, `--verify`, and `--append` are boolean. Multiple operation flags, an + * unknown flag, a missing positional, or a missing required value throws with a usage-style message. + * + * @internal - Exported to allow testing. + */ +export function parseArgs(argv: readonly string[]): ParsedArgs { + const scanned = scanArgv(argv); + return composeParsedArgs(scanned); +} + +/** + * Runs the helper end to end. Tasks 3-5 wire in the load/operation/write pipeline; the scaffolding currently + * returns `invalid-args` for any parsed input so the structured contract is exercisable from day one. The `now`, + * `stdin`, and `home` plumbing is in place so later tasks can connect operations without changing this signature. + * + * @internal - Exported to allow testing. + */ +export function runEdit(input: { + argv: readonly string[]; + stdin: Readable; + now: Date; + home?: string; +}): Promise { + let args: ParsedArgs; + try { + args = parseArgs(input.argv); + } catch (error) { + return Promise.resolve({ + ok: false, + error: 'invalid-args', + message: error instanceof Error ? error.message : String(error), + }); + } + + // Suppress unused-variable warnings until Tasks 3-5 wire these in. + void input.stdin; + void input.now; + void input.home; + + return Promise.resolve({ + ok: false, + error: 'invalid-args', + message: `operation "${args.operation}" is not yet implemented`, + }); +} + +// region | Helpers + +/** A captured operation flag: its canonical name plus the value (if any) that followed it. */ +interface SelectedOp { + name: OperationName; + value: string | null; +} + +/** + * Walks `argv` once, separating the positional `` from operation flags. Captures every operation flag seen + * (length checks in `composeParsedArgs` reject zero or more-than-one), and rejects unknown flags, missing values for + * value-bearing flags, and extra positional arguments. Returns the captured shape so the per-op composition can be + * a separate, narrow function. + */ +function scanArgv(argv: readonly string[]): { positional: string | null; selectedOps: SelectedOp[] } { + let positional: string | null = null; + const selectedOps: SelectedOp[] = []; + + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + if (arg === undefined) continue; + + if (arg.startsWith('--')) { + const matched = matchOperationFlag(arg); + if (matched === null) { + throw new Error(`unknown flag: ${arg}`); + } + let value: string | null = matched.inlineValue; + if (matched.takesValue && value === null) { + value = argv[index + 1] ?? null; + index += 1; + } + if (matched.takesValue && (value === null || value === '' || value.startsWith('--'))) { + throw new Error(`${matched.flag} requires a value`); + } + selectedOps.push({ name: matched.name, value }); + continue; + } + + if (positional !== null) { + throw new Error(`unexpected extra positional argument: ${arg}`); + } + positional = arg; + } + + return { positional, selectedOps }; +} + +/** + * Validates the scanned argv shape (one positional, exactly one operation flag) and projects it onto the typed + * `ParsedArgs` union. Per-op value requirements (`--retag`, `--supersede-with`) are checked here too, so the loop + * in `scanArgv` doesn't need to know which op is selected. + */ +function composeParsedArgs(input: { positional: string | null; selectedOps: SelectedOp[] }): ParsedArgs { + const { positional, selectedOps } = input; + + if (positional === null) { + throw new Error('missing required positional argument'); + } + if (selectedOps.length === 0) { + const flags = OPERATION_FLAGS.map(({ flag }) => flag).join(', '); + throw new Error(`one operation flag is required (one of: ${flags})`); + } + if (selectedOps.length > 1) { + const seen = selectedOps.map(({ name }) => `--${name}`).join(', '); + throw new Error(`operation flags are mutually exclusive; got ${seen}`); + } + + const [op] = selectedOps; + if (op === undefined) { + // Unreachable: length checks above guarantee a single entry. + throw new Error('internal error: missing operation after length checks'); + } + + switch (op.name) { + case 'bump-updated': + case 'verify': + case 'append': + return { operation: op.name, path: positional }; + case 'retag': + if (op.value === null) { + throw new Error('--retag requires a value'); + } + return { operation: 'retag', path: positional, tags: parseTagList(op.value) }; + case 'supersede-with': + if (op.value === null) { + throw new Error('--supersede-with requires a value'); + } + return { operation: 'supersede-with', path: positional, newPath: op.value }; + default: { + const _exhaustive: never = op.name; + throw new Error(`unhandled operation: ${String(_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 (broken symlink, permission denied) the + * function emits a warning to stderr and returns `false`, matching the degrade-with-warning pattern kb-add uses. + */ +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(`kb-edit: warning: could not determine entry point: ${message}\n`); + return false; + } +} + +/** Matches an operation flag, returning its name, whether it takes a value, and any inline `=value`. */ +function matchOperationFlag( + arg: string, +): { flag: string; name: OperationName; takesValue: boolean; inlineValue: string | null } | null { + for (const entry of OPERATION_FLAGS) { + if (arg === entry.flag) { + return { ...entry, inlineValue: null }; + } + if (entry.takesValue && arg.startsWith(`${entry.flag}=`)) { + return { ...entry, inlineValue: arg.slice(`${entry.flag}=`.length) }; + } + } + return null; +} + +/** Splits a comma-separated tag string into individual tags, dropping empties and trimming whitespace. */ +function parseTagList(value: string): string[] { + return value + .split(',') + .map((tag) => tag.trim()) + .filter((tag) => tag.length > 0); +} + +// endregion | Helpers diff --git a/packages/agents/src/kb-edit/types.ts b/packages/agents/src/kb-edit/types.ts new file mode 100644 index 00000000..5f257a9b --- /dev/null +++ b/packages/agents/src/kb-edit/types.ts @@ -0,0 +1,106 @@ +// Shapes for the kb-edit helper: parsed CLI input and the JSON result emitted to stdout. +// +// The helper's stdout payload is a discriminated union on `ok`. Recoverable failures (collision-adjacent issues such +// as `note-not-found`, schema validation, readonly KB, supersede-target-missing, and partial-supersede) return +// `{ ok: false, error, details? }`; successes return `{ ok: true, ... }`. System errors (out-of-disk, permission +// denied) are out of band: they print to stderr and exit non-zero. + +import type { Finding, Frontmatter } from '@codeassembly/kb-core'; + +import type { ResolvedKb } from '../kb-shared/resolve-writable-kb.ts'; + +/** Operation names — one per mutually-exclusive op flag. */ +export type OperationName = 'bump-updated' | 'verify' | 'retag' | 'append' | 'supersede-with'; + +/** + * Parsed command-line invocation of the kb-edit helper. + * + * A discriminated union on `operation` so each operation module receives only its own typed inputs. + */ +export type ParsedArgs = + | { operation: 'bump-updated'; path: string } + | { operation: 'verify'; path: string } + | { operation: 'retag'; path: string; tags: string[] } + | { operation: 'append'; path: string } + | { operation: 'supersede-with'; path: string; newPath: string }; + +/** The helper's stdout payload on success for a single-file operation. */ +export interface EditSingleSuccess { + ok: true; + operation: Exclude; + /** Absolute path of the edited note. */ + path: string; + /** The KB the note belongs to. */ + kb: ResolvedKb; + /** The frontmatter that was written, post-canonicalization. */ + frontmatter: Frontmatter; + /** Tag list as the agent supplied it before canonicalization. Set only for `retag`. */ + originalTags?: string[]; + /** Tag list as written to disk, after canonicalization. Set only for `retag`. */ + canonicalTags?: string[]; +} + +/** The helper's stdout payload on a successful `supersede-with`. */ +export interface EditSupersedeSuccess { + ok: true; + operation: 'supersede-with'; + /** Absolute path of the old note (now marked superseded). */ + oldPath: string; + /** Absolute path of the new (superseding) note. */ + newPath: string; + /** The KB both notes belong to. */ + kb: ResolvedKb; + /** Frontmatter written to the old note, including `superseded-by` and the `deprecated` tag. */ + oldFrontmatter: Frontmatter; + /** Frontmatter written to the new note, including `supersedes`. */ + newFrontmatter: Frontmatter; +} + +/** Discriminated success union. */ +export type EditSuccess = EditSingleSuccess | EditSupersedeSuccess; + +/** The helper's stdout payload on a recoverable failure. */ +export interface EditFailure { + ok: false; + /** Categorical error code. */ + error: EditErrorCode; + /** Short human-readable explanation. */ + message: string; + /** Optional per-error details. */ + details?: EditErrorDetails; +} + +/** Categorical error codes the helper can return without an unexpected throw. */ +export type EditErrorCode = + | 'invalid-args' + | 'no-kb-resolvable' + | 'note-not-found' + | 'note-parse' + | 'schema-validation' + | 'readonly-kb' + | 'supersede-target-missing' + | 'partial-supersede'; + +/** Per-error structured details. */ +export interface EditErrorDetails { + /** Path that failed to resolve, set when `error: 'note-not-found'` or `'supersede-target-missing'`. */ + missingPath?: string; + /** YAML parse error message, set when `error: 'note-parse'`. */ + parseError?: string; + /** Findings, set when `error: 'schema-validation'`. */ + findings?: Finding[]; + /** Registry name of the readonly KB that refused the write, set when `error: 'readonly-kb'`. */ + readonlyKbName?: string; + /** Absolute path of the readonly KB that refused the write, set when `error: 'readonly-kb'`. */ + readonlyKbPath?: string; + /** Old-note path, set when `error: 'partial-supersede'`. */ + oldPath?: string; + /** New-note path, set when `error: 'partial-supersede'`. */ + newPath?: string; +} + +/** The helper's full stdout payload: a discriminated union on `ok`. */ +export type EditResult = EditSuccess | EditFailure; + +// Re-export so kb-edit consumers don't need to learn the kb-shared path. +export type { ResolvedKb } from '../kb-shared/resolve-writable-kb.ts'; From ac93b9ef8eb9af8f62b68e8668536c9ce6ee43a3 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 06:24:31 -0700 Subject: [PATCH 03/10] agents|feat: Add kb-edit note IO layer with categorical failure surfaces Add `loadNote` and `writeBackNote` modules for the kb-edit helper. `loadNote` translates filesystem and YAML parse errors into `note-not-found` and `note-parse` outcomes; other I/O errors propagate to the helper's stderr/exit path. `writeBackNote` validates the proposed frontmatter against the destination KB's schema and only then atomically replaces the file via a same-directory temp file plus rename. The schema check is colocated with the write so per-operation code cannot bypass it. Notes with no frontmatter block, or whose frontmatter parses as something other than a YAML map, are refused as unmutatable. --- .../src/kb-edit/__tests__/load-note.test.ts | 93 +++++++++++++++ .../src/kb-edit/__tests__/write-back.test.ts | 111 ++++++++++++++++++ packages/agents/src/kb-edit/load-note.ts | 62 ++++++++++ packages/agents/src/kb-edit/write-back.ts | 74 ++++++++++++ 4 files changed, 340 insertions(+) create mode 100644 packages/agents/src/kb-edit/__tests__/load-note.test.ts create mode 100644 packages/agents/src/kb-edit/__tests__/write-back.test.ts create mode 100644 packages/agents/src/kb-edit/load-note.ts create mode 100644 packages/agents/src/kb-edit/write-back.ts diff --git a/packages/agents/src/kb-edit/__tests__/load-note.test.ts b/packages/agents/src/kb-edit/__tests__/load-note.test.ts new file mode 100644 index 00000000..61dbc8bd --- /dev/null +++ b/packages/agents/src/kb-edit/__tests__/load-note.test.ts @@ -0,0 +1,93 @@ +import { mkdtemp, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { loadNote } from '../load-note.ts'; + +async function makeTempDir(prefix: string): Promise { + return mkdtemp(join(tmpdir(), prefix)); +} + +const VALID_NOTE = `--- +title: Example +type: howto +created: 2026-05-01 +updated: 2026-05-01 +tags: [example] +--- + +Body text. +`; + +describe(loadNote, () => { + it('returns the parsed note when the file exists and frontmatter is valid', async () => { + const dir = await makeTempDir('kb-edit-load-ok-'); + const path = join(dir, 'note.md'); + await writeFile(path, VALID_NOTE, 'utf8'); + + const result = await loadNote({ path }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.note.frontmatter?.title).toBe('Example'); + expect(result.note.frontmatter?.tags).toEqual(['example']); + expect(result.note.body.trim()).toBe('Body text.'); + } + }); + + it('returns note-not-found when the path does not exist', async () => { + const dir = await makeTempDir('kb-edit-load-missing-'); + const path = join(dir, 'absent.md'); + + const result = await loadNote({ path }); + + expect(result).toEqual({ ok: false, reason: 'note-not-found', path }); + }); + + it('returns note-parse when the frontmatter block is malformed YAML', async () => { + const dir = await makeTempDir('kb-edit-load-bad-yaml-'); + const path = join(dir, 'note.md'); + // A frontmatter block with an unterminated flow mapping forces yaml to record a parse error. + await writeFile(path, '---\ntitle: {broken\n---\n\nBody\n', 'utf8'); + + const result = await loadNote({ path }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe('note-parse'); + expect(result.path).toBe(path); + if (result.reason === 'note-parse') { + expect(result.parseError).toMatch(/./); + } + } + }); + + it('returns note-parse when no frontmatter block is present', async () => { + const dir = await makeTempDir('kb-edit-load-no-fm-'); + const path = join(dir, 'note.md'); + await writeFile(path, 'Just body, no frontmatter.\n', 'utf8'); + + const result = await loadNote({ path }); + + expect(result.ok).toBe(false); + if (!result.ok && result.reason === 'note-parse') { + expect(result.parseError).toBe('no frontmatter block found'); + } + }); + + it('returns note-parse when the frontmatter block is not a YAML map', async () => { + const dir = await makeTempDir('kb-edit-load-not-map-'); + const path = join(dir, 'note.md'); + // Frontmatter parses successfully but yields a sequence instead of a map. + await writeFile(path, '---\n- one\n- two\n---\n\nBody\n', 'utf8'); + + const result = await loadNote({ path }); + + expect(result.ok).toBe(false); + if (!result.ok && result.reason === 'note-parse') { + expect(result.parseError).toBe('frontmatter is not a YAML map'); + } + }); +}); diff --git a/packages/agents/src/kb-edit/__tests__/write-back.test.ts b/packages/agents/src/kb-edit/__tests__/write-back.test.ts new file mode 100644 index 00000000..342707d2 --- /dev/null +++ b/packages/agents/src/kb-edit/__tests__/write-back.test.ts @@ -0,0 +1,111 @@ +import { mkdtemp, readFile, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import type { Frontmatter, Schema } from '@codeassembly/kb-core'; +import { defaultSchema } from '@codeassembly/kb-core'; +import { describe, expect, it } from 'vitest'; + +import { writeBackNote } from '../write-back.ts'; + +async function makeTempPath(prefix: string): Promise { + const dir = await mkdtemp(join(tmpdir(), prefix)); + return join(dir, 'note.md'); +} + +const SCHEMA: Schema = defaultSchema; + +function validFrontmatter(overrides: Partial = {}): Frontmatter { + return { + title: 'Example', + type: 'howto', + created: '2026-05-01', + updated: '2026-05-01', + tags: ['example'], + extra: {}, + ...overrides, + }; +} + +describe(writeBackNote, () => { + it('renders frontmatter and body to disk on a valid write', async () => { + const path = await makeTempPath('kb-edit-wb-ok-'); + await writeFile(path, '---\ntitle: Old\n---\n\nOld body\n', 'utf8'); + + const result = await writeBackNote({ + path, + frontmatter: validFrontmatter({ title: 'New' }), + body: 'New body\n', + schema: SCHEMA, + }); + + expect(result.ok).toBe(true); + const written = await readFile(path, 'utf8'); + expect(written).toContain('title: New'); + expect(written).toContain('New body'); + }); + + it('returns the written content on success', async () => { + const path = await makeTempPath('kb-edit-wb-content-'); + await writeFile(path, '---\ntitle: x\n---\n\nbody\n', 'utf8'); + + const result = await writeBackNote({ + path, + frontmatter: validFrontmatter(), + body: 'body\n', + schema: SCHEMA, + }); + + expect(result.ok).toBe(true); + if (result.ok) { + const written = await readFile(path, 'utf8'); + expect(result.content).toBe(written); + } + }); + + it('refuses to write and leaves the original file untouched when validation fails', async () => { + const path = await makeTempPath('kb-edit-wb-bad-type-'); + const before = '---\ntitle: original\n---\n\nuntouched\n'; + await writeFile(path, before, 'utf8'); + + const result = await writeBackNote({ + path, + // `rant` is not in the default schema's types vocabulary. + frontmatter: validFrontmatter({ type: 'rant' }), + body: 'should not land\n', + schema: SCHEMA, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe('schema-validation'); + expect(result.findings.some((f) => f.rule === 'frontmatter.type')).toBe(true); + } + const after = await readFile(path, 'utf8'); + expect(after).toBe(before); + }); + + it('round-trips a structurally identical note when frontmatter is unchanged', async () => { + const path = await makeTempPath('kb-edit-wb-roundtrip-'); + const original = '---\ntitle: x\ntype: howto\ncreated: 2026-05-01\nupdated: 2026-05-01\ntags: [a]\n---\n\nbody\n'; + await writeFile(path, original, 'utf8'); + + const result = await writeBackNote({ + path, + frontmatter: { + title: 'x', + type: 'howto', + created: '2026-05-01', + updated: '2026-05-01', + tags: ['a'], + extra: {}, + }, + body: 'body\n', + schema: SCHEMA, + }); + + expect(result.ok).toBe(true); + const written = await readFile(path, 'utf8'); + expect(written).toBe(original); + }); +}); diff --git a/packages/agents/src/kb-edit/load-note.ts b/packages/agents/src/kb-edit/load-note.ts new file mode 100644 index 00000000..f5c05fb8 --- /dev/null +++ b/packages/agents/src/kb-edit/load-note.ts @@ -0,0 +1,62 @@ +import type { ParsedNote } from '@codeassembly/kb-core'; +import { parseNote } from '@codeassembly/kb-core/frontmatter'; + +/** Successful load: a fully-parsed note with valid frontmatter. */ +export interface LoadSuccess { + ok: true; + note: ParsedNote; +} + +/** Categorical load failures the helper surfaces as structured results. */ +export type LoadFailure = + | { ok: false; reason: 'note-not-found'; path: string } + | { ok: false; reason: 'note-parse'; path: string; parseError: string }; + +/** The outcome of attempting to load a note for editing. */ +export type LoadOutcome = LoadSuccess | LoadFailure; + +/** + * Reads a note from disk and parses its frontmatter, surfacing the two failure modes kb-edit cares about as + * categorical results. + * + * `ENOENT` becomes `note-not-found`; a YAML parse error recorded by kb-core's parser becomes `note-parse`. Other + * I/O errors (permission denied, EIO) re-throw so callers can surface them as system errors via the main process's + * stderr/exit path. A note with no frontmatter block at all also resolves as `note-parse`, since kb-edit cannot + * mutate frontmatter that isn't there. + */ +export async function loadNote(input: { path: string }): Promise { + let parsed: ParsedNote; + try { + parsed = await parseNote({ path: input.path }); + } catch (error) { + if (isEnoent(error)) { + return { ok: false, reason: 'note-not-found', path: input.path }; + } + throw error; + } + + const raw = parsed.frontmatterRaw; + if (raw === null) { + return { ok: false, reason: 'note-parse', path: input.path, parseError: 'no frontmatter block found' }; + } + if (raw.parseError !== undefined) { + return { ok: false, reason: 'note-parse', path: input.path, parseError: raw.parseError }; + } + if (parsed.frontmatter === null) { + // A frontmatter block was present and parsed without YAML errors, but the projection to typed Frontmatter + // collapsed to null — for example, the block parsed as a scalar or sequence rather than a map. Treat as + // unmutatable so callers don't operate on a missing frontmatter object. + return { ok: false, reason: 'note-parse', path: input.path, parseError: 'frontmatter is not a YAML map' }; + } + + return { ok: true, note: parsed }; +} + +// region | Helpers + +/** Returns true when `error` is a Node ENOENT filesystem error. */ +function isEnoent(error: unknown): boolean { + return typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT'; +} + +// endregion | Helpers diff --git a/packages/agents/src/kb-edit/write-back.ts b/packages/agents/src/kb-edit/write-back.ts new file mode 100644 index 00000000..c00c57e1 --- /dev/null +++ b/packages/agents/src/kb-edit/write-back.ts @@ -0,0 +1,74 @@ +import { randomBytes } from 'node:crypto'; +import { rename, unlink, writeFile } from 'node:fs/promises'; + +import type { Finding, Frontmatter, Schema } from '@codeassembly/kb-core'; +import { parseNoteContent, writeFrontmatter } from '@codeassembly/kb-core/frontmatter'; +import { frontmatterRule, runRules } from '@codeassembly/kb-core/rules'; + +/** Successful write-back: the note has been re-rendered and atomically replaced. */ +export interface WriteBackSuccess { + ok: true; + /** The bytes that were written, for callers that want to assert on the final content. */ + content: string; +} + +/** Schema-validation failure: the proposed frontmatter does not pass the destination KB's schema. */ +export interface WriteBackFailure { + ok: false; + reason: 'schema-validation'; + findings: Finding[]; +} + +/** The outcome of an atomic write-back. */ +export type WriteBackOutcome = WriteBackSuccess | WriteBackFailure; + +/** + * Validates the proposed frontmatter against the destination KB's schema and, on pass, atomically rewrites the file + * at `path` with the rendered note. Operations call this rather than touching `writeFile` directly so schema + * enforcement cannot be bypassed. + * + * Validation runs by rendering the frontmatter to a note string, re-parsing it, and feeding the parsed shape through + * `frontmatterRule`. Round-tripping through the parser is the cheapest way to give the rule a real `ParsedNote` + * carrying a `yaml.Document` and the raw text positions it expects. + * + * The write is atomic via a same-directory temp file plus `rename`. On rename failure the temp file is cleaned up + * best-effort and the error re-thrown so a permission or disk error surfaces unambiguously. + */ +export async function writeBackNote(input: { + path: string; + frontmatter: Frontmatter; + body: string; + schema: Schema; +}): Promise { + const content = writeFrontmatter({ frontmatter: input.frontmatter, body: input.body }); + + const parsed = parseNoteContent({ content, path: input.path }); + const findings = runRules({ rules: [frontmatterRule], notes: [parsed], schema: input.schema }); + const errorFindings = findings.filter((finding) => finding.severity === 'error'); + if (errorFindings.length > 0) { + return { ok: false, reason: 'schema-validation', findings: errorFindings }; + } + + await atomicWrite({ targetPath: input.path, content }); + + return { ok: true, content }; +} + +// region | Helpers + +/** + * Atomic write via same-directory temp file plus rename. The temp filename uses a random suffix so concurrent writes + * to nearby paths cannot collide. A failed rename triggers a best-effort temp-file cleanup, then re-throws. + */ +async function atomicWrite(input: { targetPath: string; content: string }): Promise { + const tempPath = `${input.targetPath}.${randomBytes(8).toString('hex')}.tmp`; + await writeFile(tempPath, input.content, 'utf8'); + try { + await rename(tempPath, input.targetPath); + } catch (error) { + await unlink(tempPath).catch(() => {}); + throw error; + } +} + +// endregion | Helpers From 3ac22d5aa4fa3f43187c6f1fe370f618da780303 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 06:32:10 -0700 Subject: [PATCH 04/10] agents|feat: Wire kb-edit single-file operations end to end `kb-edit --bump-updated` sets the note's `updated:` field to today (UTC). `kb-edit --verify` sets `last-verified:` to today, adding the field if absent, and leaves `updated:` alone so re-verification stays distinct from a content edit. `kb-edit --retag ""` replaces the tag list, canonicalizes each tag through the KB's `.kb/tag-aliases.yaml`, deduplicates in first-occurrence order, and bumps `updated:`. The result includes both the pre-canonicalization and post-canonicalization tag lists for audit. `kb-edit --append` reads a stdin body and appends it after the existing body with a separating blank line, then bumps `updated:`. Empty or whitespace-only stdin is refused with `invalid-args`. Every write validates the resulting frontmatter against the destination KB's schema before atomically replacing the file via a same-directory temp file plus rename. A note whose path resolves into a `readonly: true` KB is refused with `readonly-kb` without touching disk. Two small shared helpers (`formatUtcDate`, `dedupeInOrder`) move into `packages/agents/src/kb-shared/note-helpers.ts`; `kb-add` imports them from there rather than carrying private copies. `--supersede-with` is rejected with `invalid-args` until its dedicated commit lands. --- packages/agents/src/kb-add/prepare-note.ts | 20 +- .../agents/src/kb-edit/__tests__/cli.test.ts | 271 +++++++++++++++++- packages/agents/src/kb-edit/cli.ts | 252 +++++++++++++++- .../operations/__tests__/append.test.ts | 90 ++++++ .../operations/__tests__/bump-updated.test.ts | 47 +++ .../operations/__tests__/retag.test.ts | 87 ++++++ .../operations/__tests__/verify.test.ts | 63 ++++ .../agents/src/kb-edit/operations/append.ts | 43 +++ .../src/kb-edit/operations/bump-updated.ts | 18 ++ .../agents/src/kb-edit/operations/retag.ts | 33 +++ .../agents/src/kb-edit/operations/verify.ts | 23 ++ .../kb-shared/__tests__/note-helpers.test.ts | 36 +++ packages/agents/src/kb-shared/note-helpers.ts | 16 ++ 13 files changed, 964 insertions(+), 35 deletions(-) create mode 100644 packages/agents/src/kb-edit/operations/__tests__/append.test.ts create mode 100644 packages/agents/src/kb-edit/operations/__tests__/bump-updated.test.ts create mode 100644 packages/agents/src/kb-edit/operations/__tests__/retag.test.ts create mode 100644 packages/agents/src/kb-edit/operations/__tests__/verify.test.ts create mode 100644 packages/agents/src/kb-edit/operations/append.ts create mode 100644 packages/agents/src/kb-edit/operations/bump-updated.ts create mode 100644 packages/agents/src/kb-edit/operations/retag.ts create mode 100644 packages/agents/src/kb-edit/operations/verify.ts create mode 100644 packages/agents/src/kb-shared/__tests__/note-helpers.test.ts create mode 100644 packages/agents/src/kb-shared/note-helpers.ts diff --git a/packages/agents/src/kb-add/prepare-note.ts b/packages/agents/src/kb-add/prepare-note.ts index ec92efc7..0555499b 100644 --- a/packages/agents/src/kb-add/prepare-note.ts +++ b/packages/agents/src/kb-add/prepare-note.ts @@ -3,6 +3,7 @@ import { parseNoteContent, writeFrontmatter } from '@codeassembly/kb-core/frontm import { frontmatterRule, runRules } from '@codeassembly/kb-core/rules'; import { canonicalize } from '@codeassembly/kb-core/tags'; +import { dedupeInOrder, formatUtcDate } from '../kb-shared/note-helpers.ts'; import type { ParsedArgs, PreparedNote } from './types.ts'; /** Successful preparation: a fully-typed `Frontmatter` plus the canonicalization audit trail. */ @@ -66,25 +67,6 @@ export function prepareNote(input: { args: ParsedArgs; schema: Schema; aliases: // region | Helpers -/** Returns `values` with duplicate entries dropped, preserving first-occurrence order. */ -function dedupeInOrder(values: readonly string[]): string[] { - const seen = new Set(); - const result: string[] = []; - for (const value of values) { - if (seen.has(value)) { - continue; - } - seen.add(value); - result.push(value); - } - return result; -} - -/** Formats a `Date` as a UTC `YYYY-MM-DD` string. */ -function formatUtcDate(date: Date): string { - return date.toISOString().slice(0, 10); -} - /** Renders the frontmatter to a note string, re-parses it, and runs the frontmatter rule against the parsed shape. */ function validate(input: { frontmatter: Frontmatter; schema: Schema }): Finding[] { const rendered = writeFrontmatter({ frontmatter: input.frontmatter, body: '' }); diff --git a/packages/agents/src/kb-edit/__tests__/cli.test.ts b/packages/agents/src/kb-edit/__tests__/cli.test.ts index 4fe30e2c..884c3822 100644 --- a/packages/agents/src/kb-edit/__tests__/cli.test.ts +++ b/packages/agents/src/kb-edit/__tests__/cli.test.ts @@ -1,6 +1,39 @@ +import { mkdir, mkdtemp, 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 } from '../cli.ts'; +import { parseArgs, runEdit } from '../cli.ts'; + +const NOW = new Date('2026-05-24T14:35:00Z'); +const TODAY = '2026-05-24'; + +const SAMPLE_NOTE = `--- +title: Sample +type: howto +created: 2026-05-01 +updated: 2026-05-01 +tags: [sample] +--- + +Original body. +`; + +/** Build a Readable stream that emits the given body and ends. */ +function bodyStream(body: string): Readable { + return Readable.from([Buffer.from(body, 'utf8')]); +} + +/** Stand up a temp KB with a single seed note; return paths. */ +async function makeKbWithNote(): Promise<{ kbPath: string; notePath: string }> { + const kbPath = await mkdtemp(join(tmpdir(), 'kb-edit-cli-')); + await mkdir(join(kbPath, '.kb'), { recursive: true }); + const notePath = join(kbPath, 'Sample.md'); + await writeFile(notePath, SAMPLE_NOTE, 'utf8'); + return { kbPath, notePath }; +} describe(parseArgs, () => { it('parses --bump-updated with a positional path', () => { @@ -87,3 +120,239 @@ describe(parseArgs, () => { expect(() => parseArgs(['foo.md', '--supersede-with'])).toThrow(/--supersede-with requires a value/); }); }); + +describe(runEdit, () => { + it('bumps updated and writes the note', async () => { + const { kbPath, notePath } = await makeKbWithNote(); + + const result = await runEdit({ + argv: [notePath, '--bump-updated'], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(true); + if (result.ok && result.operation === 'bump-updated') { + expect(result.frontmatter.updated).toBe(TODAY); + const written = await readFile(notePath, 'utf8'); + expect(written).toContain(`updated: ${TODAY}`); + expect(written).toContain('Original body.'); + } + }); + + it('sets last-verified without bumping updated', async () => { + const { kbPath, notePath } = await makeKbWithNote(); + + const result = await runEdit({ + argv: [notePath, '--verify'], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(true); + if (result.ok && result.operation === 'verify') { + expect(result.frontmatter.extra['last-verified']).toBe(TODAY); + expect(result.frontmatter.updated).toBe('2026-05-01'); + const written = await readFile(notePath, 'utf8'); + expect(written).toContain(`last-verified: ${TODAY}`); + expect(written).toContain('updated: 2026-05-01'); + } + }); + + it('replaces tags via --retag and surfaces canonicalization audit', async () => { + const { kbPath, notePath } = await makeKbWithNote(); + + const result = await runEdit({ + argv: [notePath, '--retag', 'one,two,three'], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(true); + if (result.ok && result.operation === 'retag') { + expect(result.originalTags).toEqual(['one', 'two', 'three']); + expect(result.canonicalTags).toEqual(['one', 'two', 'three']); + expect(result.frontmatter.tags).toEqual(['one', 'two', 'three']); + expect(result.frontmatter.updated).toBe(TODAY); + } + }); + + it('appends stdin to the body with a separating blank line and bumps updated', async () => { + const { kbPath, notePath } = await makeKbWithNote(); + + const result = await runEdit({ + argv: [notePath, '--append'], + stdin: bodyStream('Appended paragraph.\n'), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(true); + if (result.ok && result.operation === 'append') { + const written = await readFile(notePath, 'utf8'); + expect(written).toContain('Original body.\n\nAppended paragraph.'); + expect(result.frontmatter.updated).toBe(TODAY); + } + }); + + it('returns invalid-args when --append receives empty stdin', async () => { + const { kbPath, notePath } = await makeKbWithNote(); + + const result = await runEdit({ + argv: [notePath, '--append'], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('invalid-args'); + expect(result.message).toMatch(/non-empty stdin/); + } + // Original file untouched. + const written = await readFile(notePath, 'utf8'); + expect(written).toBe(SAMPLE_NOTE); + }); + + it('returns note-not-found when the path does not exist', async () => { + const { kbPath } = await makeKbWithNote(); + const missing = join(kbPath, 'absent.md'); + + const result = await runEdit({ + argv: [missing, '--bump-updated'], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('note-not-found'); + expect(result.details?.missingPath).toBe(missing); + } + }); + + it('returns note-parse when the note has malformed YAML', async () => { + const { kbPath } = await makeKbWithNote(); + const path = join(kbPath, 'broken.md'); + await writeFile(path, '---\ntitle: {broken\n---\n\nBody\n', 'utf8'); + + const result = await runEdit({ + argv: [path, '--bump-updated'], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('note-parse'); + expect(result.details?.parseError).toMatch(/./); + } + }); + + it('returns no-kb-resolvable when the note path is not inside any .kb/', async () => { + const wd = await mkdtemp(join(tmpdir(), 'kb-edit-orphan-')); + const path = join(wd, 'note.md'); + await writeFile(path, SAMPLE_NOTE, 'utf8'); + + const result = await runEdit({ + argv: [path, '--bump-updated'], + stdin: bodyStream(''), + startDir: wd, + now: NOW, + home: wd, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('no-kb-resolvable'); + } + }); + + it('returns readonly-kb when the note resolves into a readonly registry entry', async () => { + const { kbPath, notePath } = await makeKbWithNote(); + const homeDir = await mkdtemp(join(tmpdir(), 'kb-edit-readonly-home-')); + await mkdir(join(homeDir, '.claude'), { recursive: true }); + await writeFile( + join(homeDir, '.claude', 'kb.yaml'), + `kbs:\n locked:\n path: ${kbPath}\n readonly: true\n`, + 'utf8', + ); + + const result = await runEdit({ + argv: [notePath, '--bump-updated'], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: homeDir, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('readonly-kb'); + expect(result.details?.readonlyKbName).toBe('locked'); + expect(result.details?.readonlyKbPath).toBe(kbPath); + } + // Original file untouched. + const written = await readFile(notePath, 'utf8'); + expect(written).toBe(SAMPLE_NOTE); + }); + + it('returns schema-validation when the result fails frontmatter rules', async () => { + // Stand up a fixture note with a `type` outside the default vocabulary. + // Bumping `updated:` re-validates the resulting frontmatter, so this surfaces as schema-validation. + const { kbPath } = await makeKbWithNote(); + const path = join(kbPath, 'bad-type.md'); + await writeFile( + path, + '---\ntitle: x\ntype: rant\ncreated: 2026-05-01\nupdated: 2026-05-01\ntags: [x]\n---\n\nbody\n', + 'utf8', + ); + + const result = await runEdit({ + argv: [path, '--bump-updated'], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('schema-validation'); + expect(result.details?.findings?.some((f) => f.rule === 'frontmatter.type')).toBe(true); + } + }); + + it('returns invalid-args for --supersede-with until Task 5 wires it in', async () => { + const { kbPath, notePath } = await makeKbWithNote(); + const newPath = join(kbPath, 'New.md'); + await writeFile(newPath, SAMPLE_NOTE, 'utf8'); + + const result = await runEdit({ + argv: [notePath, '--supersede-with', newPath], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('invalid-args'); + expect(result.message).toMatch(/not yet implemented/); + } + }); +}); diff --git a/packages/agents/src/kb-edit/cli.ts b/packages/agents/src/kb-edit/cli.ts index 39914bf7..e84ae5e7 100644 --- a/packages/agents/src/kb-edit/cli.ts +++ b/packages/agents/src/kb-edit/cli.ts @@ -1,11 +1,24 @@ /* eslint n/no-process-exit: off */ /* eslint unicorn/no-process-exit: off */ import { realpathSync } from 'node:fs'; +import { dirname, isAbsolute, resolve } from 'node:path'; import process from 'node:process'; import type { Readable } from 'node:stream'; import { fileURLToPath } from 'node:url'; -import type { EditResult, OperationName, ParsedArgs } from './types.ts'; +import type { AliasMap, Frontmatter, KbRoot, ParsedNote, Schema } from '@codeassembly/kb-core'; +import { loadSchema } from '@codeassembly/kb-core/schema'; +import { loadAliases } from '@codeassembly/kb-core/tags'; + +import type { ResolvedKb } from '../kb-shared/resolve-writable-kb.ts'; +import { resolveWritableKb } from '../kb-shared/resolve-writable-kb.ts'; +import { loadNote } from './load-note.ts'; +import { append } from './operations/append.ts'; +import { bumpUpdated } from './operations/bump-updated.ts'; +import { retag } from './operations/retag.ts'; +import { verify } from './operations/verify.ts'; +import type { EditResult, EditSingleSuccess, OperationName, ParsedArgs } from './types.ts'; +import { writeBackNote } from './write-back.ts'; /** Operation flag → operation name. Order is the documented surface order in SKILL.md. */ const OPERATION_FLAGS = [ @@ -22,6 +35,7 @@ async function main(): Promise { const result = await runEdit({ argv: process.argv.slice(2), stdin: process.stdin, + startDir: process.cwd(), now: new Date(), }); process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); @@ -53,15 +67,19 @@ export function parseArgs(argv: readonly string[]): ParsedArgs { } /** - * Runs the helper end to end. Tasks 3-5 wire in the load/operation/write pipeline; the scaffolding currently - * returns `invalid-args` for any parsed input so the structured contract is exercisable from day one. The `now`, - * `stdin`, and `home` plumbing is in place so later tasks can connect operations without changing this signature. + * Runs the helper end to end: parses args, resolves the writable KB that owns the note, loads the note and the + * KB's schema/aliases, dispatches to the operation module, and atomically writes the result back. Recoverable + * failures (no KB, readonly KB, note not found, note parse error, schema validation, op-side rejections) become + * structured `{ ok: false, ... }` results. System failures (out-of-disk, EPERM) propagate to `main`'s try/catch. + * + * `--supersede-with` is rejected with `invalid-args` here pending Task 5. * * @internal - Exported to allow testing. */ -export function runEdit(input: { +export async function runEdit(input: { argv: readonly string[]; stdin: Readable; + startDir: string; now: Date; home?: string; }): Promise { @@ -69,27 +87,231 @@ export function runEdit(input: { try { args = parseArgs(input.argv); } catch (error) { - return Promise.resolve({ + return { ok: false, error: 'invalid-args', message: error instanceof Error ? error.message : String(error), - }); + }; } - // Suppress unused-variable warnings until Tasks 3-5 wire these in. - void input.stdin; - void input.now; - void input.home; + if (args.operation === 'supersede-with') { + return { ok: false, error: 'invalid-args', message: '--supersede-with is not yet implemented' }; + } - return Promise.resolve({ - ok: false, - error: 'invalid-args', - message: `operation "${args.operation}" is not yet implemented`, + const notePath = absoluteNotePath({ path: args.path, startDir: input.startDir }); + + const kbOutcome = await resolveKbForPath({ notePath, ...(input.home !== undefined && { home: input.home }) }); + if (!kbOutcome.ok) { + return kbOutcome.failure; + } + + const loadOutcome = await loadNote({ path: notePath }); + if (!loadOutcome.ok) { + return loadFailureToResult(loadOutcome); + } + + const { schema, aliases } = await loadKbContext({ kb: kbOutcome.kb }); + + const prepared = await prepareOperation({ + args, + note: loadOutcome.note, + aliases, + now: input.now, + stdin: input.stdin, + }); + if (!prepared.ok) { + return prepared.failure; + } + + const writeOutcome = await writeBackNote({ + path: notePath, + frontmatter: prepared.frontmatter, + body: prepared.body, + schema, }); + if (!writeOutcome.ok) { + return { + ok: false, + error: 'schema-validation', + message: `frontmatter did not pass schema validation: ${writeOutcome.findings.map((f) => f.message).join('; ')}`, + details: { findings: writeOutcome.findings }, + }; + } + + const success: EditSingleSuccess = { + ok: true, + operation: args.operation, + path: notePath, + kb: kbOutcome.kb, + frontmatter: prepared.frontmatter, + }; + if (prepared.originalTags !== undefined) { + success.originalTags = prepared.originalTags; + } + if (prepared.canonicalTags !== undefined) { + success.canonicalTags = prepared.canonicalTags; + } + return success; } // region | Helpers +/** Resolves a possibly-relative note path against the caller's start directory. */ +function absoluteNotePath(input: { path: string; startDir: string }): string { + return isAbsolute(input.path) ? input.path : resolve(input.startDir, input.path); +} + +/** + * Resolves the writable KB that owns the note at `notePath`. Walks up from the note's directory, not the + * caller's cwd, so the KB context tracks where the note lives rather than where the helper was invoked. + * Maps resolver failures (`no-kb-resolvable`, `readonly-kb`) onto top-level `EditResult` failures. + */ +async function resolveKbForPath(input: { + notePath: string; + home?: string; +}): Promise<{ ok: true; kb: ResolvedKb } | { ok: false; failure: EditResult }> { + const resolved = await resolveWritableKb({ + startDir: dirname(input.notePath), + explicitKb: null, + ...(input.home !== undefined && { home: input.home }), + }); + if (resolved.ok) { + return { ok: true, kb: resolved.kb }; + } + switch (resolved.reason) { + case 'no-kb-resolvable': + return { + ok: false, + failure: { + ok: false, + error: 'no-kb-resolvable', + message: `no .kb/ discovered for note at ${input.notePath}`, + }, + }; + case 'readonly-kb': + return { + ok: false, + failure: { + ok: false, + error: 'readonly-kb', + message: `knowledge base "${resolved.kbName}" is marked readonly in kb.yaml; writes are refused`, + details: { readonlyKbName: resolved.kbName, readonlyKbPath: resolved.kbPath }, + }, + }; + default: { + const _exhaustive: never = resolved; + throw new Error(`unhandled resolveWritableKb failure: ${JSON.stringify(_exhaustive)}`); + } + } +} + +/** Loads schema and aliases for a resolved KB. Falls back to an empty alias map on a malformed aliases file. */ +async function loadKbContext(input: { kb: ResolvedKb }): Promise<{ schema: Schema; aliases: AliasMap }> { + const kbRoot: KbRoot = { path: input.kb.path, kbDir: `${input.kb.path}/.kb`, via: 'ancestor-walk' }; + const [schema, aliases] = await Promise.all([loadSchema({ kbRoot }), loadAliasesWithWarning({ kbRoot })]); + return { schema, aliases }; +} + +/** + * Loads tag aliases, degrading a malformed or unreadable `tag-aliases.yaml` to an empty map and emitting a warning + * to stderr so the operator can see why canonicalization was skipped. + */ +async function loadAliasesWithWarning(input: { kbRoot: KbRoot }): Promise { + try { + return await loadAliases({ kbRoot: input.kbRoot }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + process.stderr.write(`kb-edit: warning: could not load tag aliases: ${message}\n`); + return new Map(); + } +} + +/** Translates a `loadNote` failure outcome into a top-level `EditResult`. */ +function loadFailureToResult(outcome: Exclude>, { ok: true }>): EditResult { + if (outcome.reason === 'note-not-found') { + return { + ok: false, + error: 'note-not-found', + message: `no file at ${outcome.path}`, + details: { missingPath: outcome.path }, + }; + } + return { + ok: false, + error: 'note-parse', + message: `could not parse frontmatter at ${outcome.path}: ${outcome.parseError}`, + details: { parseError: outcome.parseError }, + }; +} + +/** Shape returned by `prepareOperation`: a mutated frontmatter+body plus optional per-op metadata. */ +interface PreparedOperation { + ok: true; + frontmatter: Frontmatter; + body: string; + originalTags?: string[]; + canonicalTags?: string[]; +} + +/** Dispatches an operation to its module and returns the prepared write payload, or a top-level failure. */ +async function prepareOperation(input: { + args: Exclude; + note: ParsedNote; + aliases: AliasMap; + now: Date; + stdin: Readable; +}): Promise { + const frontmatter = nonNullFrontmatter(input.note); + const body = input.note.body; + + switch (input.args.operation) { + case 'bump-updated': { + const result = bumpUpdated({ frontmatter, body, now: input.now }); + return { ok: true, ...result }; + } + case 'verify': { + const result = verify({ frontmatter, body, now: input.now }); + return { ok: true, ...result }; + } + case 'retag': { + const result = retag({ frontmatter, body, tags: input.args.tags, aliases: input.aliases, now: input.now }); + return { ok: true, ...result }; + } + case 'append': { + const addition = await readAll(input.stdin); + const result = append({ frontmatter, body, addition, now: input.now }); + if (!result.ok) { + return { ok: false, failure: { ok: false, error: 'invalid-args', message: result.message } }; + } + return { ok: true, frontmatter: result.frontmatter, body: result.body }; + } + default: { + const _exhaustive: never = input.args; + throw new Error(`unhandled operation: ${JSON.stringify(_exhaustive)}`); + } + } +} + +/** Pulls the typed frontmatter off a loaded note; loadNote guarantees it is non-null at this point. */ +function nonNullFrontmatter(note: ParsedNote): Frontmatter { + if (note.frontmatter === null) { + throw new Error(`internal error: note at ${note.path} unexpectedly has null frontmatter after loadNote`); + } + return note.frontmatter; +} + +/** Reads a readable stream to completion as a UTF-8 string. */ +async function readAll(stream: Readable): Promise { + const chunks: Buffer[] = []; + for await (const chunk of stream) { + if (!Buffer.isBuffer(chunk)) { + throw new TypeError('readAll: expected Buffer chunks (stream must be in binary mode)'); + } + chunks.push(chunk); + } + return Buffer.concat(chunks).toString('utf8'); +} + /** A captured operation flag: its canonical name plus the value (if any) that followed it. */ interface SelectedOp { name: OperationName; diff --git a/packages/agents/src/kb-edit/operations/__tests__/append.test.ts b/packages/agents/src/kb-edit/operations/__tests__/append.test.ts new file mode 100644 index 00000000..3d9ddd8f --- /dev/null +++ b/packages/agents/src/kb-edit/operations/__tests__/append.test.ts @@ -0,0 +1,90 @@ +import type { Frontmatter } from '@codeassembly/kb-core'; +import { describe, expect, it } from 'vitest'; + +import { append } from '../append.ts'; + +const NOW = new Date('2026-05-24T14:35:00Z'); + +function frontmatter(overrides: Partial = {}): Frontmatter { + return { + title: 'Example', + type: 'howto', + created: '2026-05-01', + updated: '2026-05-01', + tags: ['example'], + extra: {}, + ...overrides, + }; +} + +describe(append, () => { + it('appends the addition after a separating blank line and bumps updated', () => { + const result = append({ + frontmatter: frontmatter(), + body: 'First paragraph.', + addition: 'Second paragraph.', + now: NOW, + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.body).toBe('First paragraph.\n\nSecond paragraph.\n'); + expect(result.frontmatter.updated).toBe('2026-05-24'); + } + }); + + it('trims trailing whitespace from the existing body before appending', () => { + const result = append({ + frontmatter: frontmatter(), + body: 'First paragraph.\n\n\n', + addition: 'Second paragraph.', + now: NOW, + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.body).toBe('First paragraph.\n\nSecond paragraph.\n'); + } + }); + + it('trims trailing whitespace from the addition', () => { + const result = append({ + frontmatter: frontmatter(), + body: 'First.', + addition: 'Second paragraph.\n\n', + now: NOW, + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.body).toBe('First.\n\nSecond paragraph.\n'); + } + }); + + it('handles an empty existing body', () => { + const result = append({ frontmatter: frontmatter(), body: '', addition: 'New content.', now: NOW }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.body).toBe('\n\nNew content.\n'); + } + }); + + it('rejects empty stdin with empty-addition', () => { + const result = append({ frontmatter: frontmatter(), body: 'body', addition: '', now: NOW }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe('empty-addition'); + } + }); + + it('rejects whitespace-only stdin with empty-addition', () => { + const result = append({ frontmatter: frontmatter(), body: 'body', addition: ' \n\n ', now: NOW }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe('empty-addition'); + } + }); +}); diff --git a/packages/agents/src/kb-edit/operations/__tests__/bump-updated.test.ts b/packages/agents/src/kb-edit/operations/__tests__/bump-updated.test.ts new file mode 100644 index 00000000..4073dced --- /dev/null +++ b/packages/agents/src/kb-edit/operations/__tests__/bump-updated.test.ts @@ -0,0 +1,47 @@ +import type { Frontmatter } from '@codeassembly/kb-core'; +import { describe, expect, it } from 'vitest'; + +import { bumpUpdated } from '../bump-updated.ts'; + +const NOW = new Date('2026-05-24T14:35:00Z'); + +function frontmatter(overrides: Partial = {}): Frontmatter { + return { + title: 'Example', + type: 'howto', + created: '2026-05-01', + updated: '2026-05-01', + tags: ['example'], + extra: {}, + ...overrides, + }; +} + +describe(bumpUpdated, () => { + it('sets updated to today (UTC) and leaves other fields untouched', () => { + const result = bumpUpdated({ frontmatter: frontmatter(), body: 'unchanged body', now: NOW }); + + expect(result.frontmatter.updated).toBe('2026-05-24'); + expect(result.frontmatter.title).toBe('Example'); + expect(result.frontmatter.created).toBe('2026-05-01'); + expect(result.frontmatter.tags).toEqual(['example']); + expect(result.body).toBe('unchanged body'); + }); + + it('preserves extra fields including last-verified', () => { + const fm = frontmatter({ extra: { 'last-verified': '2026-05-10', 'applies-to': 'node 24' } }); + + const result = bumpUpdated({ frontmatter: fm, body: 'b', now: NOW }); + + expect(result.frontmatter.extra).toEqual({ 'last-verified': '2026-05-10', 'applies-to': 'node 24' }); + }); + + it('does not mutate the input frontmatter', () => { + const fm = frontmatter(); + const originalUpdated = fm.updated; + + bumpUpdated({ frontmatter: fm, body: 'b', now: NOW }); + + expect(fm.updated).toBe(originalUpdated); + }); +}); diff --git a/packages/agents/src/kb-edit/operations/__tests__/retag.test.ts b/packages/agents/src/kb-edit/operations/__tests__/retag.test.ts new file mode 100644 index 00000000..05437c7e --- /dev/null +++ b/packages/agents/src/kb-edit/operations/__tests__/retag.test.ts @@ -0,0 +1,87 @@ +import type { AliasMap, Frontmatter } from '@codeassembly/kb-core'; +import { describe, expect, it } from 'vitest'; + +import { retag } from '../retag.ts'; + +const NOW = new Date('2026-05-24T14:35:00Z'); + +function frontmatter(overrides: Partial = {}): Frontmatter { + return { + title: 'Example', + type: 'howto', + created: '2026-05-01', + updated: '2026-05-01', + tags: ['old', 'tags'], + extra: {}, + ...overrides, + }; +} + +const NO_ALIASES: AliasMap = new Map(); +const NODE_ALIASES: AliasMap = new Map([ + ['node.js', 'nodejs'], + ['node', 'nodejs'], +]); + +describe(retag, () => { + it('replaces the tag list with the supplied tags and bumps updated', () => { + const result = retag({ + frontmatter: frontmatter(), + body: 'b', + tags: ['new', 'set'], + aliases: NO_ALIASES, + now: NOW, + }); + + expect(result.frontmatter.tags).toEqual(['new', 'set']); + expect(result.frontmatter.updated).toBe('2026-05-24'); + }); + + it('returns originalTags and canonicalTags for audit', () => { + const result = retag({ + frontmatter: frontmatter(), + body: 'b', + tags: ['node.js', 'react'], + aliases: NODE_ALIASES, + now: NOW, + }); + + expect(result.originalTags).toEqual(['node.js', 'react']); + expect(result.canonicalTags).toEqual(['nodejs', 'react']); + expect(result.frontmatter.tags).toEqual(['nodejs', 'react']); + }); + + it('dedupes after canonicalization in first-occurrence order', () => { + // node.js and node both canonicalize to nodejs; the second arrival is dropped. + const result = retag({ + frontmatter: frontmatter(), + body: 'b', + tags: ['node.js', 'react', 'node'], + aliases: NODE_ALIASES, + now: NOW, + }); + + expect(result.canonicalTags).toEqual(['nodejs', 'react']); + }); + + it('accepts an empty list and writes empty tags', () => { + const result = retag({ + frontmatter: frontmatter(), + body: 'b', + tags: [], + aliases: NO_ALIASES, + now: NOW, + }); + + expect(result.frontmatter.tags).toEqual([]); + expect(result.canonicalTags).toEqual([]); + }); + + it('does not mutate the input frontmatter', () => { + const fm = frontmatter(); + + retag({ frontmatter: fm, body: 'b', tags: ['x'], aliases: NO_ALIASES, now: NOW }); + + expect(fm.tags).toEqual(['old', 'tags']); + }); +}); diff --git a/packages/agents/src/kb-edit/operations/__tests__/verify.test.ts b/packages/agents/src/kb-edit/operations/__tests__/verify.test.ts new file mode 100644 index 00000000..b8cef888 --- /dev/null +++ b/packages/agents/src/kb-edit/operations/__tests__/verify.test.ts @@ -0,0 +1,63 @@ +import type { Frontmatter } from '@codeassembly/kb-core'; +import { describe, expect, it } from 'vitest'; + +import { verify } from '../verify.ts'; + +const NOW = new Date('2026-05-24T14:35:00Z'); + +function frontmatter(overrides: Partial = {}): Frontmatter { + return { + title: 'Example', + type: 'howto', + created: '2026-05-01', + updated: '2026-05-01', + tags: ['example'], + extra: {}, + ...overrides, + }; +} + +describe(verify, () => { + it('sets last-verified to today (UTC) and does not bump updated', () => { + const result = verify({ frontmatter: frontmatter(), body: 'body', now: NOW }); + + expect(result.frontmatter.extra['last-verified']).toBe('2026-05-24'); + expect(result.frontmatter.updated).toBe('2026-05-01'); + }); + + it('adds last-verified when the field is absent', () => { + const fm = frontmatter({ extra: {} }); + + const result = verify({ frontmatter: fm, body: 'b', now: NOW }); + + expect(result.frontmatter.extra).toEqual({ 'last-verified': '2026-05-24' }); + }); + + it('overwrites an existing last-verified value', () => { + const fm = frontmatter({ extra: { 'last-verified': '2026-01-15' } }); + + const result = verify({ frontmatter: fm, body: 'b', now: NOW }); + + expect(result.frontmatter.extra['last-verified']).toBe('2026-05-24'); + }); + + it('preserves other extra fields when adding last-verified', () => { + const fm = frontmatter({ extra: { 'applies-to': 'node 24', sources: ['docs.example.com'] } }); + + const result = verify({ frontmatter: fm, body: 'b', now: NOW }); + + expect(result.frontmatter.extra).toEqual({ + 'applies-to': 'node 24', + sources: ['docs.example.com'], + 'last-verified': '2026-05-24', + }); + }); + + it('does not mutate the input frontmatter', () => { + const fm = frontmatter(); + + verify({ frontmatter: fm, body: 'b', now: NOW }); + + expect(fm.extra).toEqual({}); + }); +}); diff --git a/packages/agents/src/kb-edit/operations/append.ts b/packages/agents/src/kb-edit/operations/append.ts new file mode 100644 index 00000000..7751be5c --- /dev/null +++ b/packages/agents/src/kb-edit/operations/append.ts @@ -0,0 +1,43 @@ +import type { Frontmatter } from '@codeassembly/kb-core'; + +import { formatUtcDate } from '../../kb-shared/note-helpers.ts'; + +/** Successful append: a mutated `{ frontmatter, body }` ready for `writeBackNote`. */ +export interface AppendSuccess { + ok: true; + frontmatter: Frontmatter; + body: string; +} + +/** Append rejected because the addition is empty after trimming whitespace. */ +export interface AppendFailure { + ok: false; + reason: 'empty-addition'; + message: string; +} + +/** The outcome of attempting to append. */ +export type AppendOutcome = AppendSuccess | AppendFailure; + +/** + * Appends `addition` to the end of the existing body with a single separating blank line, then bumps `updated:`. + * + * The existing body and the addition both have trailing whitespace trimmed so the inserted blank line is unambiguous + * and the final note ends with a single trailing newline. An addition that is empty or whitespace-only after + * trimming is rejected so the caller surfaces `invalid-args` rather than committing a no-op write. + */ +export function append(input: { frontmatter: Frontmatter; body: string; addition: string; now: Date }): AppendOutcome { + const trimmedAddition = input.addition.replace(/\s+$/, ''); + if (trimmedAddition === '') { + return { ok: false, reason: 'empty-addition', message: '--append requires non-empty stdin' }; + } + + const trimmedBody = input.body.replace(/\s+$/, ''); + const newBody = `${trimmedBody}\n\n${trimmedAddition}\n`; + + return { + ok: true, + frontmatter: { ...input.frontmatter, updated: formatUtcDate(input.now), extra: { ...input.frontmatter.extra } }, + body: newBody, + }; +} diff --git a/packages/agents/src/kb-edit/operations/bump-updated.ts b/packages/agents/src/kb-edit/operations/bump-updated.ts new file mode 100644 index 00000000..56ed6a01 --- /dev/null +++ b/packages/agents/src/kb-edit/operations/bump-updated.ts @@ -0,0 +1,18 @@ +import type { Frontmatter } from '@codeassembly/kb-core'; + +import { formatUtcDate } from '../../kb-shared/note-helpers.ts'; + +/** + * Sets `updated:` to today (UTC). The body is preserved exactly. Running twice on the same UTC day is an + * idempotent no-op at the field level but still produces a write — schema validation runs unconditionally + * so a note that was already invalid surfaces as `schema-validation` rather than rotting silently. + */ +export function bumpUpdated(input: { frontmatter: Frontmatter; body: string; now: Date }): { + frontmatter: Frontmatter; + body: string; +} { + return { + frontmatter: { ...input.frontmatter, updated: formatUtcDate(input.now), extra: { ...input.frontmatter.extra } }, + body: input.body, + }; +} diff --git a/packages/agents/src/kb-edit/operations/retag.ts b/packages/agents/src/kb-edit/operations/retag.ts new file mode 100644 index 00000000..ad62c690 --- /dev/null +++ b/packages/agents/src/kb-edit/operations/retag.ts @@ -0,0 +1,33 @@ +import type { AliasMap, Frontmatter } from '@codeassembly/kb-core'; +import { canonicalize } from '@codeassembly/kb-core/tags'; + +import { dedupeInOrder, formatUtcDate } from '../../kb-shared/note-helpers.ts'; + +/** + * Replaces the tag list, canonicalizing each entry through the supplied alias map, deduplicating in + * first-occurrence order, and bumping `updated:`. An empty list is a valid result. + * + * Canonicalization can collapse distinct inputs onto the same canonical, so dedupe runs after canonicalize. + * The pre-canonicalization list is returned as `originalTags` so the caller can surface an audit trail. + */ +export function retag(input: { + frontmatter: Frontmatter; + body: string; + tags: readonly string[]; + aliases: AliasMap; + now: Date; +}): { frontmatter: Frontmatter; body: string; originalTags: string[]; canonicalTags: string[] } { + const originalTags = [...input.tags]; + const canonicalTags = dedupeInOrder(originalTags.map((tag) => canonicalize(tag, input.aliases))); + return { + frontmatter: { + ...input.frontmatter, + tags: canonicalTags, + updated: formatUtcDate(input.now), + extra: { ...input.frontmatter.extra }, + }, + body: input.body, + originalTags, + canonicalTags, + }; +} diff --git a/packages/agents/src/kb-edit/operations/verify.ts b/packages/agents/src/kb-edit/operations/verify.ts new file mode 100644 index 00000000..df222039 --- /dev/null +++ b/packages/agents/src/kb-edit/operations/verify.ts @@ -0,0 +1,23 @@ +import type { Frontmatter } from '@codeassembly/kb-core'; + +import { formatUtcDate } from '../../kb-shared/note-helpers.ts'; + +/** + * Sets `last-verified:` to today (UTC), inserting the field if it isn't already present. Does **not** bump + * `updated:` — the schema treats `last-verified:` as a re-verification event distinct from a content edit. + * + * The `extra` map preserves key order on round-trip, so the first `--verify` adds `last-verified` at the + * end of the YAML map and subsequent runs update the value in place. + */ +export function verify(input: { frontmatter: Frontmatter; body: string; now: Date }): { + frontmatter: Frontmatter; + body: string; +} { + return { + frontmatter: { + ...input.frontmatter, + extra: { ...input.frontmatter.extra, 'last-verified': formatUtcDate(input.now) }, + }, + body: input.body, + }; +} diff --git a/packages/agents/src/kb-shared/__tests__/note-helpers.test.ts b/packages/agents/src/kb-shared/__tests__/note-helpers.test.ts new file mode 100644 index 00000000..0d0373a6 --- /dev/null +++ b/packages/agents/src/kb-shared/__tests__/note-helpers.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest'; + +import { dedupeInOrder, formatUtcDate } from '../note-helpers.ts'; + +describe(formatUtcDate, () => { + it('formats midnight UTC as YYYY-MM-DD', () => { + expect(formatUtcDate(new Date('2026-05-24T00:00:00Z'))).toBe('2026-05-24'); + }); + + it('formats a mid-day UTC instant as the same calendar date', () => { + expect(formatUtcDate(new Date('2026-05-24T14:35:00Z'))).toBe('2026-05-24'); + }); + + it('uses the UTC date when local timezone would yield a different day', () => { + // 23:30 UTC on May 24 is May 25 in most positive offsets and May 24 elsewhere; the helper anchors to UTC. + expect(formatUtcDate(new Date('2026-05-24T23:30:00Z'))).toBe('2026-05-24'); + }); +}); + +describe(dedupeInOrder, () => { + it('returns an empty array unchanged', () => { + expect(dedupeInOrder([])).toEqual([]); + }); + + it('preserves order and drops later duplicates', () => { + expect(dedupeInOrder(['a', 'b', 'a', 'c', 'b'])).toEqual(['a', 'b', 'c']); + }); + + it('preserves a list with no duplicates exactly', () => { + expect(dedupeInOrder(['one', 'two', 'three'])).toEqual(['one', 'two', 'three']); + }); + + it('treats distinct types as distinct values', () => { + expect(dedupeInOrder([1, 2, 1, 3])).toEqual([1, 2, 3]); + }); +}); diff --git a/packages/agents/src/kb-shared/note-helpers.ts b/packages/agents/src/kb-shared/note-helpers.ts new file mode 100644 index 00000000..35b8ddfd --- /dev/null +++ b/packages/agents/src/kb-shared/note-helpers.ts @@ -0,0 +1,16 @@ +/** Formats a `Date` as a UTC `YYYY-MM-DD` string for note frontmatter date fields. */ +export function formatUtcDate(date: Date): string { + return date.toISOString().slice(0, 10); +} + +/** Returns `values` with duplicate entries dropped, preserving first-occurrence order. */ +export function dedupeInOrder(values: readonly T[]): T[] { + const seen = new Set(); + const result: T[] = []; + for (const value of values) { + if (seen.has(value)) continue; + seen.add(value); + result.push(value); + } + return result; +} From 6494bedce51f4f81e0000723fcd12a719a5c42bb Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 06:37:24 -0700 Subject: [PATCH 05/10] agents|feat: Add kb-edit --supersede-with for atomic two-file deprecation `kb-edit --supersede-with ` marks one note superseded by another in a single invocation. The old note gains a `superseded-by` pointer and the `deprecated` tag (canonicalized through the KB's tag-aliases map, idempotent if already present); the new note gains a `supersedes` pointer back; both notes bump `updated:`. Pointer values are written KB-relative so a vault can be moved without rewriting every chain. The two writes commit with best-effort atomicity: both temp files are staged before either rename, and on the second rename's failure the captured original bytes of the old note are written back to restore consistency. An unrecoverable partial state surfaces as `partial-supersede` with both paths in `details`. Cross-KB supersession is rejected with `invalid-args`; a missing new note is rejected with `supersede-target-missing`. Both notes' resulting frontmatters are validated against the destination KB's schema before either rename runs, so a validation failure leaves the filesystem untouched. --- .../agents/src/kb-edit/__tests__/cli.test.ts | 140 +++++++++++- packages/agents/src/kb-edit/cli.ts | 199 +++++++++++++++++- .../__tests__/supersede-with.test.ts | 148 +++++++++++++ .../src/kb-edit/operations/supersede-with.ts | 67 ++++++ 4 files changed, 544 insertions(+), 10 deletions(-) create mode 100644 packages/agents/src/kb-edit/operations/__tests__/supersede-with.test.ts create mode 100644 packages/agents/src/kb-edit/operations/supersede-with.ts diff --git a/packages/agents/src/kb-edit/__tests__/cli.test.ts b/packages/agents/src/kb-edit/__tests__/cli.test.ts index 884c3822..0b22418c 100644 --- a/packages/agents/src/kb-edit/__tests__/cli.test.ts +++ b/packages/agents/src/kb-edit/__tests__/cli.test.ts @@ -336,23 +336,153 @@ describe(runEdit, () => { } }); - it('returns invalid-args for --supersede-with until Task 5 wires it in', async () => { - const { kbPath, notePath } = await makeKbWithNote(); + it('commits both writes on --supersede-with and surfaces KB-relative pointers', async () => { + const { kbPath, notePath: oldPath } = await makeKbWithNote(); const newPath = join(kbPath, 'New.md'); - await writeFile(newPath, SAMPLE_NOTE, 'utf8'); + await writeFile(newPath, SAMPLE_NOTE.replace('Sample', 'Replacement'), 'utf8'); const result = await runEdit({ - argv: [notePath, '--supersede-with', newPath], + argv: [oldPath, '--supersede-with', newPath], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(true); + if (result.ok && result.operation === 'supersede-with') { + expect(result.oldFrontmatter.extra['superseded-by']).toBe('New.md'); + expect(result.newFrontmatter.extra.supersedes).toBe('Sample.md'); + expect(result.oldFrontmatter.tags).toContain('deprecated'); + expect(result.oldFrontmatter.updated).toBe(TODAY); + expect(result.newFrontmatter.updated).toBe(TODAY); + } + + const oldOnDisk = await readFile(oldPath, 'utf8'); + const newOnDisk = await readFile(newPath, 'utf8'); + expect(oldOnDisk).toContain('superseded-by: New.md'); + expect(newOnDisk).toContain('supersedes: Sample.md'); + expect(oldOnDisk).toContain('deprecated'); + }); + + it('returns supersede-target-missing when the new path does not exist', async () => { + const { kbPath, notePath: oldPath } = await makeKbWithNote(); + const missingNew = join(kbPath, 'NoSuch.md'); + + const result = await runEdit({ + argv: [oldPath, '--supersede-with', missingNew], stdin: bodyStream(''), startDir: kbPath, now: NOW, home: kbPath, }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('supersede-target-missing'); + expect(result.details?.missingPath).toBe(missingNew); + } + // Old note untouched. + const onDisk = await readFile(oldPath, 'utf8'); + expect(onDisk).toBe(SAMPLE_NOTE); + }); + + it('rejects cross-KB supersession with invalid-args', async () => { + const { kbPath: kbA, notePath: oldPath } = await makeKbWithNote(); + const { kbPath: kbB } = await makeKbWithNote(); + const newPath = join(kbB, 'New.md'); + await writeFile(newPath, SAMPLE_NOTE, 'utf8'); + + const result = await runEdit({ + argv: [oldPath, '--supersede-with', newPath], + stdin: bodyStream(''), + startDir: kbA, + now: NOW, + home: kbA, + }); + expect(result.ok).toBe(false); if (!result.ok) { expect(result.error).toBe('invalid-args'); - expect(result.message).toMatch(/not yet implemented/); + expect(result.message).toMatch(/same KB/); + } + expect(await readFile(oldPath, 'utf8')).toBe(SAMPLE_NOTE); + }); + + it('does not commit either write when validation of the resulting frontmatter fails', async () => { + const { kbPath, notePath: oldPath } = await makeKbWithNote(); + const newPath = join(kbPath, 'BadType.md'); + // New note has a type outside the schema vocabulary; supersede-with validates both before either rename. + await writeFile( + newPath, + '---\ntitle: x\ntype: rant\ncreated: 2026-05-01\nupdated: 2026-05-01\ntags: [x]\n---\n\nbody\n', + 'utf8', + ); + + const result = await runEdit({ + argv: [oldPath, '--supersede-with', newPath], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('schema-validation'); + } + // Both files untouched. + const oldAfter = await readFile(oldPath, 'utf8'); + expect(oldAfter).toBe(SAMPLE_NOTE); + }); + + it('canonicalizes the deprecated tag against the KB alias map', async () => { + const { kbPath, notePath: oldPath } = await makeKbWithNote(); + const newPath = join(kbPath, 'New.md'); + await writeFile(newPath, SAMPLE_NOTE.replace('Sample', 'Replacement'), 'utf8'); + // Declare an alias so `deprecated` canonicalizes to `archived` when added to the old note. + await writeFile(join(kbPath, '.kb', 'tag-aliases.yaml'), 'aliases:\n archived: [deprecated]\n', 'utf8'); + + const result = await runEdit({ + argv: [oldPath, '--supersede-with', newPath], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(true); + if (result.ok && result.operation === 'supersede-with') { + expect(result.oldFrontmatter.tags).toContain('archived'); + expect(result.oldFrontmatter.tags).not.toContain('deprecated'); + } + }); + + it('is idempotent on the deprecated tag when it is already present', async () => { + const kbPath = await mkdtemp(join(tmpdir(), 'kb-edit-cli-dep-')); + await mkdir(join(kbPath, '.kb'), { recursive: true }); + const oldPath = join(kbPath, 'Old.md'); + const newPath = join(kbPath, 'New.md'); + // Old note already carries the deprecated tag; supersede-with should not duplicate it. + await writeFile( + oldPath, + '---\ntitle: Old\ntype: howto\ncreated: 2026-05-01\nupdated: 2026-05-01\ntags: [legacy, deprecated]\n---\n\nbody\n', + 'utf8', + ); + await writeFile(newPath, SAMPLE_NOTE.replace('Sample', 'New'), 'utf8'); + + const result = await runEdit({ + argv: [oldPath, '--supersede-with', newPath], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(true); + if (result.ok && result.operation === 'supersede-with') { + const occurrences = result.oldFrontmatter.tags.filter((t) => t === 'deprecated'); + expect(occurrences).toHaveLength(1); } }); }); diff --git a/packages/agents/src/kb-edit/cli.ts b/packages/agents/src/kb-edit/cli.ts index e84ae5e7..6299885f 100644 --- a/packages/agents/src/kb-edit/cli.ts +++ b/packages/agents/src/kb-edit/cli.ts @@ -1,12 +1,16 @@ /* eslint n/no-process-exit: off */ /* eslint unicorn/no-process-exit: off */ +import { randomBytes } from 'node:crypto'; import { realpathSync } from 'node:fs'; +import { rename, unlink, writeFile } from 'node:fs/promises'; import { dirname, isAbsolute, resolve } from 'node:path'; import process from 'node:process'; import type { Readable } from 'node:stream'; import { fileURLToPath } from 'node:url'; -import type { AliasMap, Frontmatter, KbRoot, ParsedNote, Schema } from '@codeassembly/kb-core'; +import type { AliasMap, Finding, Frontmatter, KbRoot, ParsedNote, Schema } from '@codeassembly/kb-core'; +import { parseNoteContent, writeFrontmatter } from '@codeassembly/kb-core/frontmatter'; +import { frontmatterRule, runRules } from '@codeassembly/kb-core/rules'; import { loadSchema } from '@codeassembly/kb-core/schema'; import { loadAliases } from '@codeassembly/kb-core/tags'; @@ -16,8 +20,9 @@ import { loadNote } from './load-note.ts'; import { append } from './operations/append.ts'; import { bumpUpdated } from './operations/bump-updated.ts'; import { retag } from './operations/retag.ts'; +import { prepareSupersedeWith } from './operations/supersede-with.ts'; import { verify } from './operations/verify.ts'; -import type { EditResult, EditSingleSuccess, OperationName, ParsedArgs } from './types.ts'; +import type { EditResult, EditSingleSuccess, EditSupersedeSuccess, OperationName, ParsedArgs } from './types.ts'; import { writeBackNote } from './write-back.ts'; /** Operation flag → operation name. Order is the documented surface order in SKILL.md. */ @@ -72,8 +77,6 @@ export function parseArgs(argv: readonly string[]): ParsedArgs { * failures (no KB, readonly KB, note not found, note parse error, schema validation, op-side rejections) become * structured `{ ok: false, ... }` results. System failures (out-of-disk, EPERM) propagate to `main`'s try/catch. * - * `--supersede-with` is rejected with `invalid-args` here pending Task 5. - * * @internal - Exported to allow testing. */ export async function runEdit(input: { @@ -95,7 +98,12 @@ export async function runEdit(input: { } if (args.operation === 'supersede-with') { - return { ok: false, error: 'invalid-args', message: '--supersede-with is not yet implemented' }; + return runSupersedeWith({ + args, + startDir: input.startDir, + now: input.now, + ...(input.home !== undefined && { home: input.home }), + }); } const notePath = absoluteNotePath({ path: args.path, startDir: input.startDir }); @@ -300,6 +308,187 @@ function nonNullFrontmatter(note: ParsedNote): Frontmatter { return note.frontmatter; } +/** + * Orchestrates `--supersede-with`: resolves and validates both paths into the same KB, prepares the in-memory + * edits, validates both resulting frontmatters against the schema, then commits both writes with best-effort + * atomicity. On the second rename's failure, the captured original bytes of the old note are restored. If + * restoration also fails, the result surfaces as `partial-supersede` with both paths in `details`. + */ +async function runSupersedeWith(input: { + args: Extract; + startDir: string; + now: Date; + home?: string; +}): Promise { + const oldPath = absoluteNotePath({ path: input.args.path, startDir: input.startDir }); + const newPath = absoluteNotePath({ path: input.args.newPath, startDir: input.startDir }); + + const oldKb = await resolveKbForPath({ notePath: oldPath, ...(input.home !== undefined && { home: input.home }) }); + if (!oldKb.ok) { + return oldKb.failure; + } + const newKb = await resolveKbForPath({ notePath: newPath, ...(input.home !== undefined && { home: input.home }) }); + if (!newKb.ok) { + return newKb.failure; + } + if (oldKb.kb.path !== newKb.kb.path) { + return { + ok: false, + error: 'invalid-args', + message: `supersede chain requires both notes in the same KB; old is in ${oldKb.kb.path}, new is in ${newKb.kb.path}`, + }; + } + + const oldLoad = await loadNote({ path: oldPath }); + if (!oldLoad.ok) { + return loadFailureToResult(oldLoad); + } + const newLoad = await loadNote({ path: newPath }); + if (!newLoad.ok) { + if (newLoad.reason === 'note-not-found') { + return { + ok: false, + error: 'supersede-target-missing', + message: `--supersede-with target does not exist: ${newPath}`, + details: { missingPath: newPath }, + }; + } + return loadFailureToResult(newLoad); + } + + const { schema, aliases } = await loadKbContext({ kb: oldKb.kb }); + + const prepared = prepareSupersedeWith({ + oldNote: oldLoad.note, + oldFrontmatter: nonNullFrontmatter(oldLoad.note), + newNote: newLoad.note, + newFrontmatter: nonNullFrontmatter(newLoad.note), + kbPath: oldKb.kb.path, + aliases, + now: input.now, + }); + + const oldRendered = writeFrontmatter({ frontmatter: prepared.old.frontmatter, body: prepared.old.body }); + const newRendered = writeFrontmatter({ frontmatter: prepared.new.frontmatter, body: prepared.new.body }); + + const oldFindings = validateRendered({ content: oldRendered, path: oldPath, schema }); + const newFindings = validateRendered({ content: newRendered, path: newPath, schema }); + const allFindings = [...oldFindings, ...newFindings]; + if (allFindings.length > 0) { + return { + ok: false, + error: 'schema-validation', + message: `frontmatter did not pass schema validation: ${allFindings.map((f) => f.message).join('; ')}`, + details: { findings: allFindings }, + }; + } + + const commitOutcome = await commitSupersede({ + oldPath, + newPath, + oldOriginalContent: oldLoad.note.content, + oldNewContent: oldRendered, + newNewContent: newRendered, + }); + if (!commitOutcome.ok) { + return { + ok: false, + error: 'partial-supersede', + message: `supersede-with: failed to commit and could not roll back; both notes may be in an inconsistent state. Original error: ${commitOutcome.message}`, + details: { oldPath, newPath }, + }; + } + + const success: EditSupersedeSuccess = { + ok: true, + operation: 'supersede-with', + oldPath, + newPath, + kb: oldKb.kb, + oldFrontmatter: prepared.old.frontmatter, + newFrontmatter: prepared.new.frontmatter, + }; + return success; +} + +/** + * Renders a frontmatter+body string and runs the frontmatter rule against it, returning error-severity findings. + * Mirrors the validation inside `writeBackNote` but runs detached from the write so two notes can be validated + * before either rename happens. + */ +function validateRendered(input: { content: string; path: string; schema: Schema }): Finding[] { + const parsed = parseNoteContent({ content: input.content, path: input.path }); + return runRules({ rules: [frontmatterRule], notes: [parsed], schema: input.schema }).filter( + (finding) => finding.severity === 'error', + ); +} + +/** + * Commits two pre-validated note writes with best-effort atomicity. + * + * Sequence: + * 1. Write both temp files (no destination mutated yet). + * 2. Rename temp-old → oldPath. On failure, cleanup both temps and re-throw. + * 3. Rename temp-new → newPath. On failure: write the captured original bytes to a new temp and rename it onto + * oldPath to undo step 2. If the rollback fails, return `{ ok: false }` so the caller can surface + * `partial-supersede`. + * + * System errors during step 1 or step 2's failure-handling propagate (caller's main catch). + */ +async function commitSupersede(input: { + oldPath: string; + newPath: string; + oldOriginalContent: string; + oldNewContent: string; + newNewContent: string; +}): Promise<{ ok: true } | { ok: false; message: string }> { + const oldTmp = `${input.oldPath}.${randomBytes(8).toString('hex')}.tmp`; + const newTmp = `${input.newPath}.${randomBytes(8).toString('hex')}.tmp`; + + await writeFile(oldTmp, input.oldNewContent, 'utf8'); + try { + await writeFile(newTmp, input.newNewContent, 'utf8'); + } catch (error) { + await unlink(oldTmp).catch(() => {}); + throw error; + } + + try { + await rename(oldTmp, input.oldPath); + } catch (error) { + await unlink(oldTmp).catch(() => {}); + await unlink(newTmp).catch(() => {}); + throw error; + } + + try { + await rename(newTmp, input.newPath); + return { ok: true }; + } catch (renameError) { + await unlink(newTmp).catch(() => {}); + const originalMessage = renameError instanceof Error ? renameError.message : String(renameError); + const rollback = await tryRollbackOld({ oldPath: input.oldPath, originalContent: input.oldOriginalContent }); + if (rollback.ok) { + // Rollback succeeded: the world is consistent again. Re-throw so the failure surfaces as a system error. + throw renameError; + } + return { ok: false, message: originalMessage }; + } +} + +/** Restores the captured original bytes to `oldPath` via temp + rename. Returns ok on success. */ +async function tryRollbackOld(input: { oldPath: string; originalContent: string }): Promise<{ ok: boolean }> { + const rollbackTmp = `${input.oldPath}.${randomBytes(8).toString('hex')}.rollback.tmp`; + try { + await writeFile(rollbackTmp, input.originalContent, 'utf8'); + await rename(rollbackTmp, input.oldPath); + return { ok: true }; + } catch { + await unlink(rollbackTmp).catch(() => {}); + return { ok: false }; + } +} + /** Reads a readable stream to completion as a UTF-8 string. */ async function readAll(stream: Readable): Promise { const chunks: Buffer[] = []; diff --git a/packages/agents/src/kb-edit/operations/__tests__/supersede-with.test.ts b/packages/agents/src/kb-edit/operations/__tests__/supersede-with.test.ts new file mode 100644 index 00000000..dfd37425 --- /dev/null +++ b/packages/agents/src/kb-edit/operations/__tests__/supersede-with.test.ts @@ -0,0 +1,148 @@ +import type { AliasMap, Frontmatter, ParsedNote } from '@codeassembly/kb-core'; +import { describe, expect, it } from 'vitest'; + +import { prepareSupersedeWith } from '../supersede-with.ts'; + +const NOW = new Date('2026-05-24T14:35:00Z'); +const KB_PATH = '/tmp/vault'; + +function note(input: { name: string; tags?: string[]; extra?: Record }): ParsedNote { + const tags = input.tags ?? ['example']; + const extra = input.extra ?? {}; + const frontmatter: Frontmatter = { + title: input.name.replace(/\.md$/, ''), + type: 'howto', + created: '2026-05-01', + updated: '2026-05-01', + tags, + extra, + }; + return { + path: `${KB_PATH}/${input.name}`, + content: '', + frontmatter, + frontmatterRaw: { text: '', startLine: 1, endLine: 6 }, + body: '\nBody.\n', + bodyStartLine: 7, + }; +} + +function frontmatterOf(parsed: ParsedNote): Frontmatter { + if (parsed.frontmatter === null) { + throw new Error('test setup: expected non-null frontmatter'); + } + return parsed.frontmatter; +} + +const NO_ALIASES: AliasMap = new Map(); + +describe(prepareSupersedeWith, () => { + it('writes superseded-by on old, supersedes on new, and adds deprecated to old', () => { + const oldNote = note({ name: 'old.md' }); + const newNote = note({ name: 'new.md' }); + + const result = prepareSupersedeWith({ + oldNote, + oldFrontmatter: frontmatterOf(oldNote), + newNote, + newFrontmatter: frontmatterOf(newNote), + kbPath: KB_PATH, + aliases: NO_ALIASES, + now: NOW, + }); + + expect(result.old.frontmatter.extra['superseded-by']).toBe('new.md'); + expect(result.new.frontmatter.extra.supersedes).toBe('old.md'); + expect(result.old.frontmatter.tags).toContain('deprecated'); + }); + + it('bumps updated on both notes', () => { + const oldNote = note({ name: 'old.md' }); + const newNote = note({ name: 'new.md' }); + + const result = prepareSupersedeWith({ + oldNote, + oldFrontmatter: frontmatterOf(oldNote), + newNote, + newFrontmatter: frontmatterOf(newNote), + kbPath: KB_PATH, + aliases: NO_ALIASES, + now: NOW, + }); + + expect(result.old.frontmatter.updated).toBe('2026-05-24'); + expect(result.new.frontmatter.updated).toBe('2026-05-24'); + }); + + it('writes KB-relative pointers when notes are in subfolders', () => { + const oldNote: ParsedNote = { ...note({ name: 'old.md' }), path: `${KB_PATH}/legacy/old.md` }; + const newNote: ParsedNote = { ...note({ name: 'new.md' }), path: `${KB_PATH}/current/new.md` }; + + const result = prepareSupersedeWith({ + oldNote, + oldFrontmatter: frontmatterOf(oldNote), + newNote, + newFrontmatter: frontmatterOf(newNote), + kbPath: KB_PATH, + aliases: NO_ALIASES, + now: NOW, + }); + + expect(result.old.frontmatter.extra['superseded-by']).toBe('current/new.md'); + expect(result.new.frontmatter.extra.supersedes).toBe('legacy/old.md'); + }); + + it('is idempotent when deprecated tag is already present', () => { + const oldNote = note({ name: 'old.md', tags: ['legacy', 'deprecated'] }); + const newNote = note({ name: 'new.md' }); + + const result = prepareSupersedeWith({ + oldNote, + oldFrontmatter: frontmatterOf(oldNote), + newNote, + newFrontmatter: frontmatterOf(newNote), + kbPath: KB_PATH, + aliases: NO_ALIASES, + now: NOW, + }); + + expect(result.old.frontmatter.tags.filter((t) => t === 'deprecated')).toHaveLength(1); + expect(result.old.frontmatter.tags).toEqual(['legacy', 'deprecated']); + }); + + it('canonicalizes deprecated through the alias map before adding it', () => { + const aliases: AliasMap = new Map([['deprecated', 'archived']]); + const oldNote = note({ name: 'old.md', tags: ['legacy'] }); + const newNote = note({ name: 'new.md' }); + + const result = prepareSupersedeWith({ + oldNote, + oldFrontmatter: frontmatterOf(oldNote), + newNote, + newFrontmatter: frontmatterOf(newNote), + kbPath: KB_PATH, + aliases, + now: NOW, + }); + + expect(result.old.frontmatter.tags).toEqual(['legacy', 'archived']); + }); + + it('preserves other extra fields on both notes', () => { + const oldNote = note({ name: 'old.md', extra: { 'applies-to': 'node 22' } }); + const newNote = note({ name: 'new.md', extra: { 'last-verified': '2026-04-15' } }); + + const result = prepareSupersedeWith({ + oldNote, + oldFrontmatter: frontmatterOf(oldNote), + newNote, + newFrontmatter: frontmatterOf(newNote), + kbPath: KB_PATH, + aliases: NO_ALIASES, + now: NOW, + }); + + expect(result.old.frontmatter.extra['applies-to']).toBe('node 22'); + expect(result.new.frontmatter.extra['last-verified']).toBe('2026-04-15'); + }); +}); diff --git a/packages/agents/src/kb-edit/operations/supersede-with.ts b/packages/agents/src/kb-edit/operations/supersede-with.ts new file mode 100644 index 00000000..cc79c99d --- /dev/null +++ b/packages/agents/src/kb-edit/operations/supersede-with.ts @@ -0,0 +1,67 @@ +import { relative } from 'node:path'; + +import type { AliasMap, Frontmatter, ParsedNote } from '@codeassembly/kb-core'; +import { canonicalize } from '@codeassembly/kb-core/tags'; + +import { dedupeInOrder, formatUtcDate } from '../../kb-shared/note-helpers.ts'; + +/** + * Prepares the in-memory edits for `--supersede-with`. Old note: `superseded-by` pointer plus a `deprecated` tag + * (canonicalized through the alias map, idempotent if already present). New note: `supersedes` pointer. Both notes' + * `updated:` are bumped. + * + * Pointers are written KB-relative so a vault can be moved without rewriting every chain. The KB-relative + * computation runs here so the operation owns its pointer convention rather than scattering `relative()` calls + * across the orchestrator. + * + * Returns only the prepared `{ frontmatter, body }` pair per file. The atomic two-file write is the caller's job + * (see `runEdit`), because rollback is tied to the on-disk staging sequence and lives at that layer. + */ +export function prepareSupersedeWith(input: { + oldNote: ParsedNote; + oldFrontmatter: Frontmatter; + newNote: ParsedNote; + newFrontmatter: Frontmatter; + kbPath: string; + aliases: AliasMap; + now: Date; +}): { + old: { frontmatter: Frontmatter; body: string }; + new: { frontmatter: Frontmatter; body: string }; +} { + const today = formatUtcDate(input.now); + const oldRelative = relative(input.kbPath, input.oldNote.path); + const newRelative = relative(input.kbPath, input.newNote.path); + + const oldTagsWithDeprecated = addDeprecatedTag({ + existingTags: input.oldFrontmatter.tags, + aliases: input.aliases, + }); + + const oldPrepared: Frontmatter = { + ...input.oldFrontmatter, + tags: oldTagsWithDeprecated, + updated: today, + extra: { ...input.oldFrontmatter.extra, 'superseded-by': newRelative }, + }; + + const newPrepared: Frontmatter = { + ...input.newFrontmatter, + updated: today, + extra: { ...input.newFrontmatter.extra, supersedes: oldRelative }, + }; + + return { + old: { frontmatter: oldPrepared, body: input.oldNote.body }, + new: { frontmatter: newPrepared, body: input.newNote.body }, + }; +} + +/** + * Adds the `deprecated` tag to a list, canonicalizing through the alias map and deduping in first-occurrence + * order. Already-present (canonical) `deprecated` results in a no-op. + */ +function addDeprecatedTag(input: { existingTags: readonly string[]; aliases: AliasMap }): string[] { + const canonicalDeprecated = canonicalize('deprecated', input.aliases); + return dedupeInOrder([...input.existingTags, canonicalDeprecated]); +} From 7e814480651b32c4f3f7be6432b7dab480810416 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 06:39:17 -0700 Subject: [PATCH 06/10] agents|docs: Document the kb-edit skill Add `SKILL.md` for kb-edit covering its argument surface, the five mutually-exclusive operations (`--bump-updated`, `--verify`, `--append`, `--retag`, `--supersede-with`), how the destination KB is inferred from the note's location, an error-code routing table for the agent to act on, and example invocations including a heredoc pattern for `--append`. Cross-references `kb-add`, `kb-retrieve`, and the planned `kb-curate`. --- .../agents/content/skills/kb-edit/SKILL.md | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 packages/agents/content/skills/kb-edit/SKILL.md diff --git a/packages/agents/content/skills/kb-edit/SKILL.md b/packages/agents/content/skills/kb-edit/SKILL.md new file mode 100644 index 00000000..412e7a67 --- /dev/null +++ b/packages/agents/content/skills/kb-edit/SKILL.md @@ -0,0 +1,114 @@ +--- +name: kb-edit +description: Mutate an existing knowledge-base note via a single mechanical operation — bump updated, mark verified, replace tags, append a section, or supersede with another note +user-invocable: true +--- + +# Edit an existing knowledge-base note + +Apply a single mutation to a note that already exists in a knowledge base. A bundled helper does the mechanical work — it resolves the writable KB the note belongs to, loads the note, runs alias canonicalization where relevant, validates the resulting frontmatter against the destination schema, and writes atomically. You do the judgment work — pick which operation fits the change, supply the new tags or body content, and decide when supersession is the right move. + +The split is deliberate: the helper is narrow and mechanical; the operation choice is wide and judgment-driven. Treat the helper as a guardrail. It refuses to write into a KB marked `readonly: true`, refuses notes that fail schema validation after the edit, and refuses to leave a half-finished supersede chain. + +For new notes, use `kb-add`. For finding notes, use `kb-retrieve`. For periodic vault hygiene (broken wikilinks, stale verifications, tag drift), use `kb-curate` once it ships. + +**Announce at start:** "Using kb-edit to {short description of the change}." + +## Arguments + +| Argument | Description | Required | +| ---------------------- | ------------------------------------------------------------------------------------- | -------- | +| `` | Path to the existing note. Absolute or relative to the current working directory. | Yes | +| `--bump-updated` | Set `updated:` to today (UTC). Body unchanged. | One op | +| `--verify` | Set `last-verified:` to today (UTC). Does **not** bump `updated:`. | One op | +| `--append` | Append the body read from stdin after a separating blank line, and bump `updated:`. | One op | +| `--retag ` | Replace `tags:` with the comma-separated list. Canonicalizes; bumps `updated:`. | One op | +| `--supersede-with

` | Mark `` superseded by `

`. Two-file atomic write; both notes bump `updated:`. | One op | + +A value-bearing flag accepts both `--retag node,react` and `--retag=node,react`. Exactly one operation flag is required per invocation; combining two is rejected with `invalid-args`. The note body for `--append` is read from stdin to EOF; empty or whitespace-only stdin is rejected. + +### KB selection + +The destination knowledge base is inferred by walking up from the note's directory for a `.kb/` folder. There is no `--kb` override: the note's location is the selector. A KB whose `kb.yaml` entry sets `readonly: true` refuses writes with `readonly-kb`. + +## Runtime dependencies + +- **`node` ≥ 24** — the bundled helper inherits the Node version floor of `@codeassembly/kb-core`. + +## Modes + +- **Default mode**: Pick the operation, decide tags/body, present the proposed change to the user, run the helper after confirmation. +- **Auto mode (`--auto`)**: Pick the operation and supply inputs without asking. The agent never prompts in this mode. + +The `--auto` flag is consumed by you, not by the bundled helper; it controls whether you present the proposal for confirmation before invoking the helper. + +## Operations: when to use each + +- **`--bump-updated`** — A non-empirical edit to the note (rewording, restructuring, fact correction) where the body change is made out of band and you want only to refresh `updated:`. Rare on its own; mostly an audit-trail tool. +- **`--verify`** — You reran the note's instructions or re-confirmed its claims and they still hold. Use this for the "I just checked; still good" path. Does not bump `updated:` because nothing about the content changed. +- **`--append`** — Add a section to an existing note. The new content lands after the existing body with a separating blank line. Use for accumulating findings or extending a list. +- **`--retag`** — Replace the tag list wholesale (canonicalized through the KB's `.kb/tag-aliases.yaml`). Use when tags drift, when restructuring categories, or when remediating findings from `kb-curate`. +- **`--supersede-with`** — Mark an old note deprecated and point it at its replacement. Both notes' frontmatter is updated atomically (best-effort): old gains `superseded-by` and the `deprecated` tag, new gains `supersedes`. Use when a note is no longer canonical but should remain discoverable. + +## Process + +### 1. Pick the operation + +Identify which single operation fits the change. If you have several distinct changes in mind (retag and append, say), they become two separate invocations — the helper rejects combined flags. + +### 2. Survey context with kb-retrieve when warranted + +For `--retag` and `--supersede-with`, run `kb-retrieve` for related notes first: a retag is often a vault-wide pattern change worth applying consistently, and a supersession needs the right successor identified. + +### 3. Present the proposal (default mode) + +In default mode, present the note path, the operation, and the operation-specific inputs (new tags, addition body, supersede target). Wait for confirmation or a redirect. In auto mode, skip this step. + +### 4. Invoke the helper + +`--bump-updated`, `--verify`, `--retag`, and `--supersede-with` take no stdin: + +```bash +node "$(dirname "$SKILL_PATH")/kb-edit.mjs" --bump-updated +node "$(dirname "$SKILL_PATH")/kb-edit.mjs" --verify +node "$(dirname "$SKILL_PATH")/kb-edit.mjs" --retag "tag1,tag2" +node "$(dirname "$SKILL_PATH")/kb-edit.mjs" --supersede-with +``` + +`--append` reads the new body from stdin. A heredoc keeps the addition legible without quoting gymnastics: + +```bash +cat <<'EOF' | node "$(dirname "$SKILL_PATH")/kb-edit.mjs" --append +A new section appended to the existing body. The helper adds a separating +blank line and bumps `updated:` to today (UTC). +EOF +``` + +Or, when the skill directory is known: + +```bash +node {platform_home_dir}/skills/kb-edit/kb-edit.mjs Tools/tmux/tmux-insights.md --verify +``` + +### 5. Handle the result + +The helper prints a JSON object to stdout. On success the payload carries `ok: true`, the resolved `kb`, the written `frontmatter`, and (for `--retag`) the `originalTags` / `canonicalTags` audit trail. `--supersede-with` returns `oldFrontmatter` and `newFrontmatter` for both files. + +On failure, `ok: false` plus a categorical `error` code: + +| Code | What it means | What to do | +| -------------------------- | --------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | +| `invalid-args` | Missing/extra flags, unknown flag, empty `--append` stdin, or cross-KB supersede. | Correct the invocation. The message names the specific defect. | +| `no-kb-resolvable` | The note's path is not inside any discoverable `.kb/`. | Confirm the path; the note may live outside a KB or the wrong path was supplied. | +| `note-not-found` | The path does not exist. | Confirm the path. For `--supersede-with`, the _new_ path gets `supersede-target-missing` instead. | +| `note-parse` | The note's frontmatter block is malformed (YAML error, missing block, non-map). | Repair the frontmatter manually; the helper will not rewrite a note it cannot parse. | +| `schema-validation` | The resulting frontmatter does not pass the KB's schema. | Inspect `details.findings`; either fix the source note or declare a schema change. | +| `readonly-kb` | The note resolves into a KB marked `readonly: true` in `kb.yaml`. | Switch to a writable KB or update the registry entry intentionally. | +| `supersede-target-missing` | The `--supersede-with` target path does not exist. | Create the new note (use `kb-add`) before issuing the supersession. | +| `partial-supersede` | A `--supersede-with` write committed one side, the rollback also failed. | Inspect both paths in `details`; resolve the inconsistency manually before retrying. | + +System failures (out-of-disk, permission denied) print to stderr and exit non-zero. They are out of band and never appear as a structured `error` code. + +## Completion + +A mutated note at the reported path (or two notes for `--supersede-with`) with frontmatter valid per the destination KB's schema, plus the canonicalization audit trail for `--retag` so the user can verify which alias tags were rewritten. From a4f84872a55a741892686a0acce7c665bfa6adf2 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 06:42:12 -0700 Subject: [PATCH 07/10] agents|chore: Register kb-edit bundle and end-to-end smoke test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the kb-edit `BundleTarget` so `pnpm run build` produces `content/skills/kb-edit/kb-edit.mjs`, and add a post-build smoke test that runs the bundled helper with `--bump-updated` against a temp-dir fixture KB. The smoke test exercises the load → mutate → write-back pipeline against the built bundle, asserting `ok: true` and a today-shaped `updated:` field — coverage the unit suite cannot give for the bundle's runtime contract. Add the generated bundle path to `.gitignore` alongside the other build-time helper bundles. --- .gitignore | 1 + .../agents/scripts/bundle-skill-helpers.ts | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/.gitignore b/.gitignore index af795a57..54eeefe5 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ docs/plans/ # Bundled skill helpers (generated by the agents build) packages/agents/content/skills/kb-add/kb-add.mjs +packages/agents/content/skills/kb-edit/kb-edit.mjs packages/agents/content/skills/kb-retrieve/kb-retrieve.mjs packages/agents/content/skills/update-jira-ticket/update-jira-ticket.mjs diff --git a/packages/agents/scripts/bundle-skill-helpers.ts b/packages/agents/scripts/bundle-skill-helpers.ts index 84499308..9d5c437e 100644 --- a/packages/agents/scripts/bundle-skill-helpers.ts +++ b/packages/agents/scripts/bundle-skill-helpers.ts @@ -61,6 +61,11 @@ export const targets: BundleTarget[] = [ entry: 'src/kb-add/cli.ts', outFile: 'content/skills/kb-add/kb-add.mjs', }, + { + entry: 'src/kb-edit/cli.ts', + outFile: 'content/skills/kb-edit/kb-edit.mjs', + smokeTest: makeKbEditSmokeTest(), + }, { entry: 'src/kb-retrieve/cli.ts', outFile: 'content/skills/kb-retrieve/kb-retrieve.mjs', @@ -137,6 +142,53 @@ function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null; } +/** + * Stands up a fixture KB with a single seed note and returns a `SmokeTestInvocation` that runs the bundle with + * `--bump-updated` against it. Exercises the load → mutate → write-back pipeline end to end, which is the only + * code path that wires the bundled `parseNote`, schema loader, and atomic write together. `HOME` is overridden to + * the fixture dir so the dev's real `~/.claude/kb.yaml` does not pollute KB resolution. + * + * The fixture is process-lifetime — `mkdtempSync` runs at module load and the OS reclaims short-lived temp + * directories without explicit cleanup. The seed note's `updated:` field is bumped by every invocation; + * `--bump-updated` is idempotent at the field level so re-runs within the same day are no-ops on disk. + */ +function makeKbEditSmokeTest(): SmokeTestInvocation { + const fixtureDir = mkdtempSync(path.join(tmpdir(), 'kb-edit-smoke-')); + mkdirSync(path.join(fixtureDir, '.kb'), { recursive: true }); + const notePath = path.join(fixtureDir, 'Smoke.md'); + writeFileSync( + notePath, + '---\ntitle: Smoke\ntype: howto\ncreated: 2026-05-01\nupdated: 2026-05-01\ntags: [smoke]\n---\n\nSmoke body.\n', + 'utf8', + ); + return { + args: [notePath, '--bump-updated'], + cwd: fixtureDir, + env: { ...process.env, HOME: fixtureDir }, + assertResult: assertKbEditSmokeResult, + }; +} + +/** Assert the kb-edit smoke produced an ok bump-updated result with a today-shaped `updated:` field. */ +function assertKbEditSmokeResult(result: unknown): void { + if (!isRecord(result)) { + throw new TypeError('expected object result from kb-edit'); + } + if (result.ok !== true) { + throw new Error(`expected ok: true, got ${JSON.stringify(result)}`); + } + if (result.operation !== 'bump-updated') { + throw new Error(`expected operation 'bump-updated', got ${JSON.stringify(result.operation)}`); + } + const frontmatter = result.frontmatter; + if (!isRecord(frontmatter)) { + throw new TypeError('expected frontmatter object on kb-edit result'); + } + if (typeof frontmatter.updated !== 'string' || !/^\d{4}-\d{2}-\d{2}$/.test(frontmatter.updated)) { + throw new Error(`expected updated to be YYYY-MM-DD, got ${JSON.stringify(frontmatter.updated)}`); + } +} + /** Assert the parsed smoke-test result reports a composition-code-inline-mark finding. */ function assertCompositionViolationFinding(result: unknown): void { if (!isRecord(result)) { From 574dbc434d189523f80c79caa24aea4c4f5504d4 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 06:44:13 -0700 Subject: [PATCH 08/10] agents|chore: Exclude kb-edit bundle from prettier and eslint Add `content/skills/kb-edit/kb-edit.mjs` to the agents-package `.prettierignore` and `eslint.config.js` ignore lists alongside the other generated bundle outputs, so the build artifact does not surface as a thousand lint findings on the next `nmr check`. --- packages/agents/.prettierignore | 1 + packages/agents/eslint.config.js | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/agents/.prettierignore b/packages/agents/.prettierignore index b85aef76..fb146a5c 100644 --- a/packages/agents/.prettierignore +++ b/packages/agents/.prettierignore @@ -4,6 +4,7 @@ dist/ # Generated esbuild bundles of skill helpers, not authored source. content/skills/derive-session-context/derive-session-context.mjs content/skills/kb-add/kb-add.mjs +content/skills/kb-edit/kb-edit.mjs content/skills/kb-retrieve/kb-retrieve.mjs content/skills/update-jira-ticket/update-jira-ticket.mjs diff --git a/packages/agents/eslint.config.js b/packages/agents/eslint.config.js index 5145c2d3..6ef17086 100644 --- a/packages/agents/eslint.config.js +++ b/packages/agents/eslint.config.js @@ -9,6 +9,7 @@ export default [ 'content/skills/_platforms/**', 'content/skills/derive-session-context/derive-session-context.mjs', 'content/skills/kb-add/kb-add.mjs', + 'content/skills/kb-edit/kb-edit.mjs', 'content/skills/kb-retrieve/kb-retrieve.mjs', 'content/skills/update-jira-ticket/update-jira-ticket.mjs', ]), From 82d4707bf27ac929c0532df488f2906a9c4bea13 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 07:05:49 -0700 Subject: [PATCH 09/10] agents|fix: Track ~/.agents/kb.yaml move in kb-edit readonly test fixture The kb-edit readonly test stands up an isolated `HOME` with a registry entry marked `readonly: true` so the helper's refusal can be exercised end to end. Switch the inline fixture from `.claude/kb.yaml` to `.agents/kb.yaml` to match #672's move of the user-global registry path. --- packages/agents/src/kb-edit/__tests__/cli.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/agents/src/kb-edit/__tests__/cli.test.ts b/packages/agents/src/kb-edit/__tests__/cli.test.ts index 0b22418c..00fcfa84 100644 --- a/packages/agents/src/kb-edit/__tests__/cli.test.ts +++ b/packages/agents/src/kb-edit/__tests__/cli.test.ts @@ -284,9 +284,9 @@ describe(runEdit, () => { it('returns readonly-kb when the note resolves into a readonly registry entry', async () => { const { kbPath, notePath } = await makeKbWithNote(); const homeDir = await mkdtemp(join(tmpdir(), 'kb-edit-readonly-home-')); - await mkdir(join(homeDir, '.claude'), { recursive: true }); + await mkdir(join(homeDir, '.agents'), { recursive: true }); await writeFile( - join(homeDir, '.claude', 'kb.yaml'), + join(homeDir, '.agents', 'kb.yaml'), `kbs:\n locked:\n path: ${kbPath}\n readonly: true\n`, 'utf8', ); From 91ffd6c8522d279fb212b9c34aeb6e801e189ccb Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 09:28:13 -0700 Subject: [PATCH 10/10] agents|feat: Harden kb-edit supersede surface and clarify --retag empty-input handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `kb-edit --supersede-with ` (the same path supplied twice) is now refused with `invalid-args` instead of silently producing a self-referential supersede chain. `kb-edit --retag=` and `kb-edit --retag ""` now explicitly clear all tags, matching the behavior of the long-form `--retag ,,`. The empty value was previously rejected by the argv scanner, leaving operators with no idiomatic way to clear tags. `--supersede-with` still rejects empty values (an empty path resolves to the start directory and would fail downstream with a confusing EISDIR). The `--supersede-with` write-and-rollback machinery moves into its own module (`kb-edit/commit-supersede.ts`) with an injected IO interface, and the validation helper used to validate both notes before either rename moves into `kb-edit/write-back.ts` (`validateFrontmatter`). The same validation now backs both `writeBackNote` and the supersede flow, so a future change to either surface stays in step. The IO seam lets new tests exercise the rollback paths — including `partial-supersede` — without depending on filesystem trickery. --- .../agents/src/kb-edit/__tests__/cli.test.ts | 39 ++++- .../__tests__/commit-supersede.test.ts | 144 ++++++++++++++++++ packages/agents/src/kb-edit/cli.ts | 110 +++---------- .../agents/src/kb-edit/commit-supersede.ts | 115 ++++++++++++++ packages/agents/src/kb-edit/write-back.ts | 17 ++- 5 files changed, 333 insertions(+), 92 deletions(-) create mode 100644 packages/agents/src/kb-edit/__tests__/commit-supersede.test.ts create mode 100644 packages/agents/src/kb-edit/commit-supersede.ts diff --git a/packages/agents/src/kb-edit/__tests__/cli.test.ts b/packages/agents/src/kb-edit/__tests__/cli.test.ts index 00fcfa84..7ba6da0e 100644 --- a/packages/agents/src/kb-edit/__tests__/cli.test.ts +++ b/packages/agents/src/kb-edit/__tests__/cli.test.ts @@ -108,7 +108,7 @@ describe(parseArgs, () => { expect(() => parseArgs(['foo.md', '--bogus'])).toThrow(/unknown flag/); }); - it('throws when --retag has no value', () => { + it('throws when --retag has no value at all (end of argv)', () => { expect(() => parseArgs(['foo.md', '--retag'])).toThrow(/--retag requires a value/); }); @@ -116,9 +116,25 @@ describe(parseArgs, () => { expect(() => parseArgs(['foo.md', '--retag', '--bump-updated'])).toThrow(/--retag requires a value/); }); + it('treats --retag="" as an explicit clear and yields an empty tag list', () => { + const parsed = parseArgs(['foo.md', '--retag=']); + + expect(parsed).toEqual({ operation: 'retag', path: 'foo.md', tags: [] }); + }); + + it('treats --retag "" as an explicit clear and yields an empty tag list', () => { + const parsed = parseArgs(['foo.md', '--retag', '']); + + expect(parsed).toEqual({ operation: 'retag', path: 'foo.md', tags: [] }); + }); + it('throws when --supersede-with has no value', () => { expect(() => parseArgs(['foo.md', '--supersede-with'])).toThrow(/--supersede-with requires a value/); }); + + it('throws when --supersede-with is given an empty value', () => { + expect(() => parseArgs(['foo.md', '--supersede-with='])).toThrow(/--supersede-with requires a value/); + }); }); describe(runEdit, () => { @@ -387,6 +403,27 @@ describe(runEdit, () => { expect(onDisk).toBe(SAMPLE_NOTE); }); + it('rejects self-supersession (same path for old and new) with invalid-args', async () => { + const { kbPath, notePath } = await makeKbWithNote(); + + const result = await runEdit({ + argv: [notePath, '--supersede-with', notePath], + stdin: bodyStream(''), + startDir: kbPath, + now: NOW, + home: kbPath, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toBe('invalid-args'); + expect(result.message).toMatch(/distinct paths/); + } + // Original note untouched: no superseded-by or supersedes pointers, no deprecated tag. + const onDisk = await readFile(notePath, 'utf8'); + expect(onDisk).toBe(SAMPLE_NOTE); + }); + it('rejects cross-KB supersession with invalid-args', async () => { const { kbPath: kbA, notePath: oldPath } = await makeKbWithNote(); const { kbPath: kbB } = await makeKbWithNote(); diff --git a/packages/agents/src/kb-edit/__tests__/commit-supersede.test.ts b/packages/agents/src/kb-edit/__tests__/commit-supersede.test.ts new file mode 100644 index 00000000..d02d6cd3 --- /dev/null +++ b/packages/agents/src/kb-edit/__tests__/commit-supersede.test.ts @@ -0,0 +1,144 @@ +import { describe, expect, it } from 'vitest'; + +import { commitSupersede, type CommitSupersedeIo } from '../commit-supersede.ts'; + +interface Call { + fn: 'writeFile' | 'rename' | 'unlink'; + path: string; + content?: string; +} + +/** + * Builds a mock IO that records every call and lets the test register per-call failure modes: + * + * - `renameFailures`: zero-based indexes of `rename` calls that should reject with the given error. + * - `writeFileFailures`: same shape for `writeFile`. + * + * `unlink` calls always succeed (production code best-effort-unlinks failed temps anyway). + */ +function makeIo( + failures: { + renameFailures?: Map; + writeFileFailures?: Map; + } = {}, +): { io: CommitSupersedeIo; calls: Call[] } { + const calls: Call[] = []; + let renameIndex = 0; + let writeFileIndex = 0; + + // Production code passes string paths and string contents; coerce here so the recorded `Call` is always + // ergonomic to assert against without re-narrowing PathLike on every assertion. + const asString = (value: unknown): string => (typeof value === 'string' ? value : JSON.stringify(value)); + + const io: CommitSupersedeIo = { + writeFile: (filePath, content) => { + calls.push({ fn: 'writeFile', path: asString(filePath), content: asString(content) }); + const failure = failures.writeFileFailures?.get(writeFileIndex); + writeFileIndex += 1; + return failure === undefined ? Promise.resolve() : Promise.reject(failure); + }, + rename: (oldPath, newPath) => { + calls.push({ fn: 'rename', path: `${asString(oldPath)} -> ${asString(newPath)}` }); + const failure = failures.renameFailures?.get(renameIndex); + renameIndex += 1; + return failure === undefined ? Promise.resolve() : Promise.reject(failure); + }, + unlink: (path) => { + calls.push({ fn: 'unlink', path: asString(path) }); + return Promise.resolve(); + }, + }; + + return { io, calls }; +} + +const INPUT = { + oldPath: '/vault/old.md', + newPath: '/vault/new.md', + oldOriginalContent: 'original old\n', + oldNewContent: 'new old (with superseded-by)\n', + newNewContent: 'new new (with supersedes)\n', +} as const; + +describe(commitSupersede, () => { + it('writes both temps, renames both, and returns ok on the happy path', async () => { + const { io, calls } = makeIo(); + + const result = await commitSupersede({ ...INPUT, io }); + + expect(result).toEqual({ ok: true }); + // Sequence: writeFile oldTmp, writeFile newTmp, rename oldTmp->oldPath, rename newTmp->newPath. + const fns = calls.map((c) => c.fn); + expect(fns).toEqual(['writeFile', 'writeFile', 'rename', 'rename']); + }); + + it('cleans up oldTmp and re-throws when the second writeFile (newTmp) fails', async () => { + const error = new Error('disk full'); + const { io, calls } = makeIo({ writeFileFailures: new Map([[1, error]]) }); + + await expect(commitSupersede({ ...INPUT, io })).rejects.toBe(error); + + // After step-2 failure: oldTmp unlinked, no renames attempted. + const fns = calls.map((c) => c.fn); + expect(fns).toEqual(['writeFile', 'writeFile', 'unlink']); + }); + + it('cleans up both temps and re-throws when the first rename (oldTmp -> oldPath) fails', async () => { + const error = new Error('permission denied'); + const { io, calls } = makeIo({ renameFailures: new Map([[0, error]]) }); + + await expect(commitSupersede({ ...INPUT, io })).rejects.toBe(error); + + // After step-3 failure: both temps unlinked, second rename not attempted. + const fns = calls.map((c) => c.fn); + expect(fns).toEqual(['writeFile', 'writeFile', 'rename', 'unlink', 'unlink']); + }); + + it('restores the old note from captured original bytes when the second rename fails but rollback succeeds, then re-throws the rename error', async () => { + // Rename index 0 = oldTmp->oldPath (succeeds), index 1 = newTmp->newPath (fails), + // index 2 = rollbackTmp->oldPath (succeeds). Rollback succeeds → original error re-thrown. + const renameError = new Error('rename to new failed'); + const { io, calls } = makeIo({ renameFailures: new Map([[1, renameError]]) }); + + await expect(commitSupersede({ ...INPUT, io })).rejects.toBe(renameError); + + // Expected sequence: + // writeFile oldTmp, writeFile newTmp, rename oldTmp->oldPath, rename newTmp->newPath (fails), + // unlink newTmp, writeFile rollbackTmp, rename rollbackTmp->oldPath. + const fns = calls.map((c) => c.fn); + expect(fns).toEqual(['writeFile', 'writeFile', 'rename', 'rename', 'unlink', 'writeFile', 'rename']); + // The rollback writeFile must carry the captured original bytes, not the mutated content. + const rollbackWrite = calls[5]; + expect(rollbackWrite?.content).toBe(INPUT.oldOriginalContent); + }); + + it('returns ok: false with the original message when both the second rename and the rollback rename fail', async () => { + // Rename index 1 (newTmp->newPath) fails, rename index 2 (rollbackTmp->oldPath) also fails. + const renameError = new Error('rename to new failed'); + const rollbackError = new Error('rollback rename failed'); + const { io } = makeIo({ + renameFailures: new Map([ + [1, renameError], + [2, rollbackError], + ]), + }); + + const result = await commitSupersede({ ...INPUT, io }); + + expect(result).toEqual({ ok: false, message: 'rename to new failed' }); + }); + + it('returns ok: false when the second rename fails and the rollback writeFile fails', async () => { + // Rename index 1 fails, writeFile index 2 (rollbackTmp) fails. Rollback can't even stage; partial-supersede. + const renameError = new Error('rename to new failed'); + const writeError = new Error('rollback write failed'); + const { io } = makeIo({ + renameFailures: new Map([[1, renameError]]), + writeFileFailures: new Map([[2, writeError]]), + }); + + const result = await commitSupersede({ ...INPUT, io }); + + expect(result).toEqual({ ok: false, message: 'rename to new failed' }); + }); +}); diff --git a/packages/agents/src/kb-edit/cli.ts b/packages/agents/src/kb-edit/cli.ts index 6299885f..c98cee43 100644 --- a/packages/agents/src/kb-edit/cli.ts +++ b/packages/agents/src/kb-edit/cli.ts @@ -1,21 +1,19 @@ /* eslint n/no-process-exit: off */ /* eslint unicorn/no-process-exit: off */ -import { randomBytes } from 'node:crypto'; import { realpathSync } from 'node:fs'; -import { rename, unlink, writeFile } from 'node:fs/promises'; import { dirname, isAbsolute, resolve } from 'node:path'; import process from 'node:process'; import type { Readable } from 'node:stream'; import { fileURLToPath } from 'node:url'; -import type { AliasMap, Finding, Frontmatter, KbRoot, ParsedNote, Schema } from '@codeassembly/kb-core'; -import { parseNoteContent, writeFrontmatter } from '@codeassembly/kb-core/frontmatter'; -import { frontmatterRule, runRules } from '@codeassembly/kb-core/rules'; +import type { AliasMap, Frontmatter, KbRoot, ParsedNote, Schema } from '@codeassembly/kb-core'; +import { writeFrontmatter } from '@codeassembly/kb-core/frontmatter'; import { loadSchema } from '@codeassembly/kb-core/schema'; import { loadAliases } from '@codeassembly/kb-core/tags'; import type { ResolvedKb } from '../kb-shared/resolve-writable-kb.ts'; import { resolveWritableKb } from '../kb-shared/resolve-writable-kb.ts'; +import { commitSupersede } from './commit-supersede.ts'; import { loadNote } from './load-note.ts'; import { append } from './operations/append.ts'; import { bumpUpdated } from './operations/bump-updated.ts'; @@ -23,7 +21,7 @@ import { retag } from './operations/retag.ts'; import { prepareSupersedeWith } from './operations/supersede-with.ts'; import { verify } from './operations/verify.ts'; import type { EditResult, EditSingleSuccess, EditSupersedeSuccess, OperationName, ParsedArgs } from './types.ts'; -import { writeBackNote } from './write-back.ts'; +import { validateFrontmatter, writeBackNote } from './write-back.ts'; /** Operation flag → operation name. Order is the documented surface order in SKILL.md. */ const OPERATION_FLAGS = [ @@ -323,6 +321,14 @@ async function runSupersedeWith(input: { const oldPath = absoluteNotePath({ path: input.args.path, startDir: input.startDir }); const newPath = absoluteNotePath({ path: input.args.newPath, startDir: input.startDir }); + if (oldPath === newPath) { + return { + ok: false, + error: 'invalid-args', + message: `supersede chain requires distinct paths; got the same path for old and new: ${oldPath}`, + }; + } + const oldKb = await resolveKbForPath({ notePath: oldPath, ...(input.home !== undefined && { home: input.home }) }); if (!oldKb.ok) { return oldKb.failure; @@ -371,8 +377,8 @@ async function runSupersedeWith(input: { const oldRendered = writeFrontmatter({ frontmatter: prepared.old.frontmatter, body: prepared.old.body }); const newRendered = writeFrontmatter({ frontmatter: prepared.new.frontmatter, body: prepared.new.body }); - const oldFindings = validateRendered({ content: oldRendered, path: oldPath, schema }); - const newFindings = validateRendered({ content: newRendered, path: newPath, schema }); + const oldFindings = validateFrontmatter({ content: oldRendered, path: oldPath, schema }); + const newFindings = validateFrontmatter({ content: newRendered, path: newPath, schema }); const allFindings = [...oldFindings, ...newFindings]; if (allFindings.length > 0) { return { @@ -411,84 +417,6 @@ async function runSupersedeWith(input: { return success; } -/** - * Renders a frontmatter+body string and runs the frontmatter rule against it, returning error-severity findings. - * Mirrors the validation inside `writeBackNote` but runs detached from the write so two notes can be validated - * before either rename happens. - */ -function validateRendered(input: { content: string; path: string; schema: Schema }): Finding[] { - const parsed = parseNoteContent({ content: input.content, path: input.path }); - return runRules({ rules: [frontmatterRule], notes: [parsed], schema: input.schema }).filter( - (finding) => finding.severity === 'error', - ); -} - -/** - * Commits two pre-validated note writes with best-effort atomicity. - * - * Sequence: - * 1. Write both temp files (no destination mutated yet). - * 2. Rename temp-old → oldPath. On failure, cleanup both temps and re-throw. - * 3. Rename temp-new → newPath. On failure: write the captured original bytes to a new temp and rename it onto - * oldPath to undo step 2. If the rollback fails, return `{ ok: false }` so the caller can surface - * `partial-supersede`. - * - * System errors during step 1 or step 2's failure-handling propagate (caller's main catch). - */ -async function commitSupersede(input: { - oldPath: string; - newPath: string; - oldOriginalContent: string; - oldNewContent: string; - newNewContent: string; -}): Promise<{ ok: true } | { ok: false; message: string }> { - const oldTmp = `${input.oldPath}.${randomBytes(8).toString('hex')}.tmp`; - const newTmp = `${input.newPath}.${randomBytes(8).toString('hex')}.tmp`; - - await writeFile(oldTmp, input.oldNewContent, 'utf8'); - try { - await writeFile(newTmp, input.newNewContent, 'utf8'); - } catch (error) { - await unlink(oldTmp).catch(() => {}); - throw error; - } - - try { - await rename(oldTmp, input.oldPath); - } catch (error) { - await unlink(oldTmp).catch(() => {}); - await unlink(newTmp).catch(() => {}); - throw error; - } - - try { - await rename(newTmp, input.newPath); - return { ok: true }; - } catch (renameError) { - await unlink(newTmp).catch(() => {}); - const originalMessage = renameError instanceof Error ? renameError.message : String(renameError); - const rollback = await tryRollbackOld({ oldPath: input.oldPath, originalContent: input.oldOriginalContent }); - if (rollback.ok) { - // Rollback succeeded: the world is consistent again. Re-throw so the failure surfaces as a system error. - throw renameError; - } - return { ok: false, message: originalMessage }; - } -} - -/** Restores the captured original bytes to `oldPath` via temp + rename. Returns ok on success. */ -async function tryRollbackOld(input: { oldPath: string; originalContent: string }): Promise<{ ok: boolean }> { - const rollbackTmp = `${input.oldPath}.${randomBytes(8).toString('hex')}.rollback.tmp`; - try { - await writeFile(rollbackTmp, input.originalContent, 'utf8'); - await rename(rollbackTmp, input.oldPath); - return { ok: true }; - } catch { - await unlink(rollbackTmp).catch(() => {}); - return { ok: false }; - } -} - /** Reads a readable stream to completion as a UTF-8 string. */ async function readAll(stream: Readable): Promise { const chunks: Buffer[] = []; @@ -528,10 +456,14 @@ function scanArgv(argv: readonly string[]): { positional: string | null; selecte } let value: string | null = matched.inlineValue; if (matched.takesValue && value === null) { + // Inline `--flag=value` already provided the value; otherwise consume the next argv slot. Empty inline + // values (`--flag=` or `--flag ""`) are intentionally allowed at this layer — each op decides whether + // empty is meaningful (`--retag ""` clears tags) or downstream-rejected (`--supersede-with ""` fails + // path resolution). value = argv[index + 1] ?? null; index += 1; } - if (matched.takesValue && (value === null || value === '' || value.startsWith('--'))) { + if (matched.takesValue && (value === null || value.startsWith('--'))) { throw new Error(`${matched.flag} requires a value`); } selectedOps.push({ name: matched.name, value }); @@ -584,7 +516,9 @@ function composeParsedArgs(input: { positional: string | null; selectedOps: Sele } return { operation: 'retag', path: positional, tags: parseTagList(op.value) }; case 'supersede-with': - if (op.value === null) { + // Empty `--supersede-with` value is rejected here so it surfaces as a clear `invalid-args`, not a confusing + // EISDIR downstream when `''` resolves to the start directory. + if (op.value === null || op.value === '') { throw new Error('--supersede-with requires a value'); } return { operation: 'supersede-with', path: positional, newPath: op.value }; diff --git a/packages/agents/src/kb-edit/commit-supersede.ts b/packages/agents/src/kb-edit/commit-supersede.ts new file mode 100644 index 00000000..b0a4b2db --- /dev/null +++ b/packages/agents/src/kb-edit/commit-supersede.ts @@ -0,0 +1,115 @@ +import { randomBytes } from 'node:crypto'; +import { rename, unlink, writeFile } from 'node:fs/promises'; + +/** + * The subset of `node:fs/promises` operations `commitSupersede` performs. Tests inject mocks via this interface + * so the rollback paths (which depend on specific renames failing) can be exercised without filesystem trickery. + * Production callers use `REAL_IO` and never see this seam. + */ +export interface CommitSupersedeIo { + writeFile: typeof writeFile; + rename: typeof rename; + unlink: typeof unlink; +} + +/** Production IO: the real `node:fs/promises` functions. */ +const REAL_IO: CommitSupersedeIo = { writeFile, rename, unlink }; + +/** Successful commit: both renames succeeded; the world is in the post-supersede state. */ +export interface CommitSuccess { + ok: true; +} + +/** + * Commit aborted, then rollback also failed. The old note is in an unknown state (either the new content from + * step 3 or partially-rolled-back content); the new note is in its original state. Surfaces to the caller as + * `partial-supersede`. + */ +export interface CommitFailure { + ok: false; + message: string; +} + +/** Discriminated outcome of `commitSupersede`. */ +export type CommitOutcome = CommitSuccess | CommitFailure; + +/** + * Commits two pre-validated note writes with best-effort atomicity. + * + * Sequence: + * 1. Write both temp files (no destination mutated yet). + * 2. Rename temp-old → oldPath. On failure, cleanup both temps and re-throw. + * 3. Rename temp-new → newPath. On failure: write the captured original bytes to a new temp and rename it onto + * oldPath to undo step 2. If the rollback fails, return `{ ok: false }` so the caller can surface + * `partial-supersede`. + * + * System errors during step 1 or step 2 propagate (caller's main catch). When step 3 fails but rollback + * succeeds, the original rename error re-throws so the failure surfaces as a system error — the contract + * keeps rename failures uniform with other I/O failures, since the recoverable-vs-fatal split would otherwise + * have to widen the structured error vocabulary for a narrow class. + * + * `io` defaults to real `node:fs/promises` operations; tests inject mocks to force specific failure paths. + */ +export async function commitSupersede(input: { + oldPath: string; + newPath: string; + oldOriginalContent: string; + oldNewContent: string; + newNewContent: string; + io?: CommitSupersedeIo; +}): Promise { + const io = input.io ?? REAL_IO; + const oldTmp = `${input.oldPath}.${randomBytes(8).toString('hex')}.tmp`; + const newTmp = `${input.newPath}.${randomBytes(8).toString('hex')}.tmp`; + + await io.writeFile(oldTmp, input.oldNewContent, 'utf8'); + try { + await io.writeFile(newTmp, input.newNewContent, 'utf8'); + } catch (error) { + await io.unlink(oldTmp).catch(() => {}); + throw error; + } + + try { + await io.rename(oldTmp, input.oldPath); + } catch (error) { + await io.unlink(oldTmp).catch(() => {}); + await io.unlink(newTmp).catch(() => {}); + throw error; + } + + try { + await io.rename(newTmp, input.newPath); + return { ok: true }; + } catch (renameError) { + await io.unlink(newTmp).catch(() => {}); + const originalMessage = renameError instanceof Error ? renameError.message : String(renameError); + const rollback = await tryRollbackOld({ + oldPath: input.oldPath, + originalContent: input.oldOriginalContent, + io, + }); + if (rollback.ok) { + // Rollback succeeded: the world is consistent again. Re-throw so the failure surfaces as a system error. + throw renameError; + } + return { ok: false, message: originalMessage }; + } +} + +/** Restores the captured original bytes to `oldPath` via temp + rename. Returns ok on success. */ +async function tryRollbackOld(input: { + oldPath: string; + originalContent: string; + io: CommitSupersedeIo; +}): Promise<{ ok: boolean }> { + const rollbackTmp = `${input.oldPath}.${randomBytes(8).toString('hex')}.rollback.tmp`; + try { + await input.io.writeFile(rollbackTmp, input.originalContent, 'utf8'); + await input.io.rename(rollbackTmp, input.oldPath); + return { ok: true }; + } catch { + await input.io.unlink(rollbackTmp).catch(() => {}); + return { ok: false }; + } +} diff --git a/packages/agents/src/kb-edit/write-back.ts b/packages/agents/src/kb-edit/write-back.ts index c00c57e1..b3787a3b 100644 --- a/packages/agents/src/kb-edit/write-back.ts +++ b/packages/agents/src/kb-edit/write-back.ts @@ -5,6 +5,19 @@ import type { Finding, Frontmatter, Schema } from '@codeassembly/kb-core'; import { parseNoteContent, writeFrontmatter } from '@codeassembly/kb-core/frontmatter'; import { frontmatterRule, runRules } from '@codeassembly/kb-core/rules'; +/** + * Validates a rendered note string against the destination KB's schema, returning error-severity findings. + * Round-trips through `parseNoteContent` so the rule sees a real `ParsedNote` carrying a `yaml.Document` and the + * raw text positions it expects. Used by `writeBackNote` and by the supersede orchestrator (which needs to validate + * both notes before either rename runs). + */ +export function validateFrontmatter(input: { content: string; path: string; schema: Schema }): Finding[] { + const parsed = parseNoteContent({ content: input.content, path: input.path }); + return runRules({ rules: [frontmatterRule], notes: [parsed], schema: input.schema }).filter( + (finding) => finding.severity === 'error', + ); +} + /** Successful write-back: the note has been re-rendered and atomically replaced. */ export interface WriteBackSuccess { ok: true; @@ -42,9 +55,7 @@ export async function writeBackNote(input: { }): Promise { const content = writeFrontmatter({ frontmatter: input.frontmatter, body: input.body }); - const parsed = parseNoteContent({ content, path: input.path }); - const findings = runRules({ rules: [frontmatterRule], notes: [parsed], schema: input.schema }); - const errorFindings = findings.filter((finding) => finding.severity === 'error'); + const errorFindings = validateFrontmatter({ content, path: input.path, schema: input.schema }); if (errorFindings.length > 0) { return { ok: false, reason: 'schema-validation', findings: errorFindings }; }