diff --git a/packages/agents/content/skills/capture-event/SKILL.md b/packages/agents/content/skills/capture-event/SKILL.md index 90905522..d88fda8d 100644 --- a/packages/agents/content/skills/capture-event/SKILL.md +++ b/packages/agents/content/skills/capture-event/SKILL.md @@ -6,7 +6,7 @@ user-invocable: true # Capture an event -Append an event record to the shared knowledge substrate, or amend an existing one that has not yet been pushed. A bundled helper does the mechanical work — it resolves the event store by name, auto-fills the record's context (a ULID `id`, the capture timestamp, the working directory, and a best-effort `session` and `repo`), validates the event record's required fields, and writes the record atomically. You supply the `summary` and the event body. +Append an event record to the shared knowledge substrate, or amend an existing one. A bundled helper does the mechanical work — it resolves the event store by name, auto-fills the record's context (a ULID `id`, the capture timestamp, the working directory, and a best-effort `session` and `repo`), validates the event record's required fields, and writes the record atomically. You supply the `summary` and the event body. This is a pure append. Unlike `kb-add`, it runs no survey, no `kb-retrieve` cross-referencing, and no dedup. The point is to capture the event cheaply and move on; recall and triage happen later via `kb-retrieve`. @@ -22,19 +22,18 @@ A **skill-caused mistake** — an error a clearer skill definition would have pr ## Arguments -| Argument | Description | Required | -| ---------------- | --------------------------------------------------------------------------------- | -------- | -| `--summary` | A human-readable one-line summary; becomes the record's label on recall. | Yes | -| `--store` | Registry name of the event store, or `@default` for the `default_kb`. | Yes | -| `--skill` | The skill the event relates to. | No | -| `--model` | The model identifier in play. | No | -| `--harness` | The agent platform (`claude`, `rovodev`); install-injected — keep as-is. | Injected | -| `--tags` | Comma-separated tag list. | No | -| `--impact` | Impact rating: one of `low`, `medium`, `high`, `critical`. Omit to leave unrated. | No | -| `--amend` | Id of an existing event to rewrite in place instead of capturing a new one. | No | -| `--allow-pushed` | With `--amend`, rewrite even an event already pushed to the remote. | No | +| Argument | Description | Required | +| ----------- | --------------------------------------------------------------------------------- | -------- | +| `--summary` | A human-readable one-line summary; becomes the record's label on recall. | Yes | +| `--store` | Registry name of the event store, or `@default` for the `default_kb`. | Yes | +| `--skill` | The skill the event relates to. | No | +| `--model` | The model identifier in play. | No | +| `--harness` | The agent platform (`claude`, `rovodev`); install-injected — keep as-is. | Injected | +| `--tags` | Comma-separated tag list. | No | +| `--impact` | Impact rating: one of `low`, `medium`, `high`, `critical`. Omit to leave unrated. | No | +| `--amend` | Id of an existing event to rewrite in place instead of capturing a new one. | No | -A value-bearing flag accepts both `--summary text` and `--summary=text`; `--allow-pushed` is a boolean flag. The event body is read from stdin to EOF; an empty body is allowed. +A value-bearing flag accepts both `--summary text` and `--summary=text`. The event body is read from stdin to EOF; an empty body is allowed. ### Auto-filled vs agent-supplied @@ -50,9 +49,9 @@ Choose the destination deliberately. When the lesson is specific to a project, p ### Amending an event -An event is editable until it is pushed to the store's remote and immutable after, so `--amend ` is how you correct a capture that is still local — for example, an event a `capture-feedback` pass got wrong. Prefer amending over capturing a near-duplicate. Amend always rewrites `summary` and the body from the invocation. It overwrites `--skill`, `--model`, `--tags`, or `--impact` only when you pass that flag; any you omit keep their existing value, as do the provenance fields (`id`, `captured-at`, `session`, `cwd`, `repo`, `harness`) and any `addressed-by` marks. To clear a curatorial field rather than edit content, use its `kb-update-events` mutator. +`--amend ` rewrites an existing capture in place — for example, an event a `capture-feedback` pass got wrong. Prefer amending over capturing a near-duplicate. Amend always rewrites `summary` and the body from the invocation. It overwrites `--skill`, `--model`, `--tags`, or `--impact` only when you pass that flag; any you omit keep their existing value, as do the provenance fields (`id`, `captured-at`, `session`, `cwd`, `repo`, `harness`) and any `addressed-by` marks. To clear a curatorial field rather than edit content, use its `kb-update-events` mutator. -When the event has already been pushed, the amend is refused. Re-run with `--allow-pushed` to rewrite it deliberately (this rewrites pushed history), or capture a new event instead. +Amend is a plain in-place edit and does not consult push state. To correct an event that may already have been shared, prefer appending a supersession with `kb-update-events --add-addressed-by` over rewriting it, so the correction lands as a new record rather than a change to history. ## Runtime dependencies @@ -99,8 +98,7 @@ On `ok: false`, route by the `error` code: - `schema-validation` — surface the `errors`, then supply the missing field and retry. - `amend-not-found` — `--amend` named an id with no event at it. Confirm the id and store. - `amend-parse` — the event to amend is not a valid event record. Inspect the file. -- `event-pushed` — the event is already pushed. Re-run with `--allow-pushed` to amend it anyway, or capture a new event instead. ## Completion -A written record at the reported path, validated as an event record. A fresh capture never overwrites an existing event; an event stays editable via `--amend` until it is pushed to the remote, and is immutable after. +A written record at the reported path, validated as an event record. A fresh capture never overwrites an existing event; use `--amend` to rewrite one in place. diff --git a/packages/agents/content/skills/capture-feedback/SKILL.md b/packages/agents/content/skills/capture-feedback/SKILL.md index a5ffe658..5c474f54 100644 --- a/packages/agents/content/skills/capture-feedback/SKILL.md +++ b/packages/agents/content/skills/capture-feedback/SKILL.md @@ -59,7 +59,7 @@ Invoke the `{skill:capture-event}` skill to append the record, composing its arg State what was fixed — or that the feedback was behavioral-only — and the captured record's id and path. -If the user then says the record is inaccurate, correct it in place with `capture-event --amend ` rather than capturing a second record — the event is still local and therefore editable. Capture a fresh record only when the correction is a genuinely distinct lesson. +If the user then says the record is inaccurate, correct it in place with `capture-event --amend ` rather than capturing a second record — amend rewrites the record in place. Capture a fresh record only when the correction is a genuinely distinct lesson. ## Completion diff --git a/packages/agents/content/skills/kb-update-events/SKILL.md b/packages/agents/content/skills/kb-update-events/SKILL.md index f9fe92b9..520c12a0 100644 --- a/packages/agents/content/skills/kb-update-events/SKILL.md +++ b/packages/agents/content/skills/kb-update-events/SKILL.md @@ -8,7 +8,7 @@ user-invocable: true Apply one mutation to one or more existing event records in a single invocation. A bundled helper does the mechanical work — it resolves the event store by name, resolves each id to its record, reads it through the type-blind note I/O layer, parses it to a typed `KbEvent`, applies the operation, and writes it back atomically. You supply the store, the operation, and the event ids. -The operation surface is the **curatorial mutable set** only: `addressed-by` (mark an event as addressed by a reference), `tags` (retag), and `impact` (set the impact rating). These are curatorial annotations that stay available regardless of push state, and they stamp no timestamp. Impact is a subjective assessment that may legitimately change, which is why it lives in the mutable set rather than the immutable spine. They are not substantive edits: to change an event's summary or body, use `capture-event --amend`, which works until the event is pushed. For new events, use `capture-event`. For editing assertions, use `kb-edit`. +The operation surface is the **curatorial mutable set** only: `addressed-by` (mark an event as addressed by a reference), `tags` (retag), and `impact` (set the impact rating). These are curatorial annotations that stamp no timestamp. Impact is a subjective assessment that may legitimately change, which is why it lives in the mutable set rather than among the substantive fields. They are not substantive edits: to change an event's summary or body, use `capture-event --amend`. For new events, use `capture-event`. For editing assertions, use `kb-edit`. **Announce at start:** "Using kb-update-events to {mark|retag|rate} {N} event(s)." @@ -73,4 +73,4 @@ On `ok: false`, route by the `error` code: ## Completion -Each named event updated in place and re-validated, written atomically. A mixed batch is partial by design: succeeded events are written; failed ids are reported and left untouched. These `addressed-by`/`tags`/`impact` annotations stay available regardless of push state; substantive content edits go through `capture-event --amend`. +Each named event updated in place and re-validated, written atomically. A mixed batch is partial by design: succeeded events are written; failed ids are reported and left untouched. These `addressed-by`/`tags`/`impact` annotations are curatorial; substantive content edits go through `capture-event --amend`. diff --git a/packages/agents/src/capture-event/__tests__/cli.test.ts b/packages/agents/src/capture-event/__tests__/cli.test.ts index 78522438..e000941a 100644 --- a/packages/agents/src/capture-event/__tests__/cli.test.ts +++ b/packages/agents/src/capture-event/__tests__/cli.test.ts @@ -1,5 +1,5 @@ import { execFile } from 'node:child_process'; -import { appendFile, mkdir, mkdtemp, readdir, readFile, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readdir, readFile, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { Readable } from 'node:stream'; @@ -11,12 +11,6 @@ import { parseArgs, runCapture } from '../cli.ts'; const execFileAsync = promisify(execFile); -/** The upstream branch the fixture store tracks. Deliberately not `main` — see {@link makeGitBackedStore}. */ -const UPSTREAM_BRANCH = 'trunk'; - -/** The remote-tracking ref the fixture publishes to, standing in for what a push would update. */ -const UPSTREAM_REF = `refs/remotes/origin/${UPSTREAM_BRANCH}`; - /** Initialize a throwaway git repo with a single named remote, so `resolveRepo` can derive an `owner/name`. */ async function makeRepoWithRemote(remoteUrl: string, remoteName = 'origin'): Promise { const repo = await mkdtemp(join(tmpdir(), 'capture-cli-repo-')); @@ -62,71 +56,6 @@ async function makeStore(name: string): Promise<{ storePath: string; home: strin return { storePath, home }; } -/** - * Stand up a git-backed event store with a seeded upstream commit, registered under `name`. The upstream is synthesized - * locally — no bare remote, no `git push` — because a push is fixture here, not subject, and the process spawns it costs - * are what make this suite contend with itself under parallel runs. - * - * The store's branch deliberately tracks `origin/trunk` rather than `origin/main`. `isEventPushed` reads the branch's - * *configured* upstream, and if the fixture tracked the default name, an implementation that hardcoded `origin/main` - * would resolve identically and the test could not tell the two apart. Tracking a non-default name is what makes that - * substitution fail. - */ -async function makeGitBackedStore(name: string): Promise<{ storePath: string; home: string }> { - const storePath = await makeStoreDir(); - await execFileAsync('git', ['-C', storePath, 'init', '--quiet', '-b', 'main']); - await writeGitTrackingConfig(storePath); - await execFileAsync('git', ['-C', storePath, 'add', '-A']); - await execFileAsync('git', ['-C', storePath, 'commit', '--quiet', '-m', 'seed']); - await publish(storePath); - - const home = await mkdtemp(join(tmpdir(), 'capture-cli-githome-')); - await mkdir(join(home, '.agents'), { recursive: true }); - await writeFile( - join(home, '.agents', 'kb.yaml'), - `default_kb: ${name}\nkbs:\n ${name}:\n path: ${storePath}\n`, - 'utf8', - ); - - return { storePath, home }; -} - -/** Stage and commit the store's working tree, then publish it to the synthesized upstream. */ -async function commitAndPublish(storePath: string): Promise { - await execFileAsync('git', ['-C', storePath, 'add', '-A']); - await execFileAsync('git', ['-C', storePath, 'commit', '--quiet', '-m', 'capture']); - await publish(storePath); -} - -/** Advance the store's upstream ref to its current `HEAD`, standing in for what a `git push` would do. */ -async function publish(storePath: string): Promise { - await execFileAsync('git', ['-C', storePath, 'update-ref', UPSTREAM_REF, 'HEAD']); -} - -/** - * Write the tracking configuration that makes `@{upstream}` resolve, appended to `.git/config` in one write rather than - * set through a `git config` subprocess apiece. All three parts are load-bearing: without the remote's fetch refspec, - * git cannot map the branch onto a remote-tracking ref and `@{upstream}` fails outright with "upstream branch not stored - * as a remote-tracking branch". The remote is never dialed — its URL exists only to satisfy the config's shape. - */ -async function writeGitTrackingConfig(storePath: string): Promise { - const config = [ - '[user]', - '\temail = test@example.com', - '\tname = Test', - '[commit]', - '\tgpgsign = false', - '[remote "origin"]', - `\turl = ${storePath}`, - '\tfetch = +refs/heads/*:refs/remotes/origin/*', - '[branch "main"]', - '\tremote = origin', - `\tmerge = refs/heads/${UPSTREAM_BRANCH}`, - '', - ].join('\n'); - await appendFile(join(storePath, '.git', 'config'), config, 'utf8'); -} - describe(parseArgs, () => { it('parses every value-bearing flag in long form', () => { const parsed = parseArgs([ @@ -155,20 +84,17 @@ describe(parseArgs, () => { tags: ['one', 'two', 'three'], impact: 'high', amend: null, - allowPushed: false, }); }); - it('parses --amend and the --allow-pushed boolean flag', () => { - const parsed = parseArgs(['--summary', 'x', '--amend', ID, '--allow-pushed']); + it('parses --amend', () => { + const parsed = parseArgs(['--summary', 'x', '--amend', ID]); expect(parsed.amend).toBe(ID); - expect(parsed.allowPushed).toBe(true); }); - it('leaves amend null and allowPushed false when both are omitted', () => { + it('leaves amend null when omitted', () => { const parsed = parseArgs(['--summary', 'x']); expect(parsed.amend).toBeNull(); - expect(parsed.allowPushed).toBe(false); }); it('rejects an --amend id that is not a bare filename stem', () => { @@ -561,7 +487,7 @@ describe(runCapture, () => { } }); - it('amends an unpushed event, editing content while preserving provenance and unsupplied fields', async () => { + it('amends an event, editing content while preserving provenance and unsupplied fields', async () => { const { home } = await makeStore('codeassembly'); const repo = await makeRepoWithRemote('git@github.com:williamthorsen/codeassembly.git'); @@ -656,49 +582,4 @@ describe(runCapture, () => { expect(result.error).toBe('amend-not-found'); } }); - - it('refuses to amend a pushed event, then allows it with --allow-pushed', async () => { - const { storePath, home } = await makeGitBackedStore('codeassembly'); - - const created = await runCapture({ - argv: ['--store', '@default', '--summary', 'Pushed summary'], - stdin: bodyStream('Pushed body.'), - cwd: storePath, - env: { CLAUDE_CODE_SESSION_ID: 'session-original' }, - now: NOW, - home, - }); - expect(created.ok).toBe(true); - if (!created.ok) return; - await commitAndPublish(storePath); - - const refused = await runCapture({ - argv: ['--store', '@default', '--amend', created.id, '--summary', 'Reworded'], - stdin: bodyStream('Reworded body.'), - cwd: storePath, - env: {}, - now: NOW, - home, - }); - expect(refused.ok).toBe(false); - if (!refused.ok) { - expect(refused.error).toBe('event-pushed'); - expect(refused.message).toContain('--allow-pushed'); - } - - const forced = await runCapture({ - argv: ['--store', '@default', '--amend', created.id, '--summary', 'Reworded', '--allow-pushed'], - stdin: bodyStream('Reworded body.'), - cwd: storePath, - env: {}, - now: NOW, - home, - }); - expect(forced.ok).toBe(true); - if (forced.ok) { - const written = await readFile(forced.path, 'utf8'); - expect(written).toContain('Reworded body.'); - expect(written).not.toContain('Pushed body.'); - } - }); }); diff --git a/packages/agents/src/capture-event/__tests__/prepare-event.test.ts b/packages/agents/src/capture-event/__tests__/prepare-event.test.ts index a2a1eaf8..19c3a779 100644 --- a/packages/agents/src/capture-event/__tests__/prepare-event.test.ts +++ b/packages/agents/src/capture-event/__tests__/prepare-event.test.ts @@ -18,7 +18,6 @@ function argsFor(overrides: Partial): ParsedArgs { tags: [], impact: null, amend: null, - allowPushed: false, ...overrides, }; } diff --git a/packages/agents/src/capture-event/cli.ts b/packages/agents/src/capture-event/cli.ts index 12f4545a..024b959a 100644 --- a/packages/agents/src/capture-event/cli.ts +++ b/packages/agents/src/capture-event/cli.ts @@ -26,7 +26,6 @@ import { type FlagSpec, scanFlags, valueFlagMap } from '../lib/parse-flags.ts'; import { readAll } from '../lib/stream-helpers.ts'; import { isEnoent } from '../lib/type-guards.ts'; import { parseRemoteToOwnerRepo } from '../shared/parse-remote-url.ts'; -import { isEventPushed } from './event-push-state.ts'; import { prepareEvent } from './prepare-event.ts'; import type { CaptureContext, CaptureResult, ParsedArgs } from './types.ts'; import { writeEvent } from './write-event.ts'; @@ -43,7 +42,6 @@ const FLAGS: readonly FlagSpec[] = [ { name: 'tags', takesValue: true }, { name: 'impact', takesValue: true }, { name: 'amend', takesValue: true }, - { name: 'allow-pushed', takesValue: false }, ]; /** Executes the helper from `process.argv` and writes the JSON result to stdout. */ @@ -77,7 +75,7 @@ if (isEntryPoint()) { * {@link amendEvent}. * * Recoverable failures (invalid args, an omitted `--store`, an unregistered/readonly store, no configured default, - * schema validation, and the amend-specific not-found/parse/pushed cases) become structured `{ ok: false, ... }` + * schema validation, and the amend-specific not-found/parse cases) become structured `{ ok: false, ... }` * results. System failures (out-of-disk, permission denied) propagate to the caller's try/catch. * * @internal - Exported to allow testing. @@ -174,7 +172,7 @@ export async function runCapture(input: { /** * Parses the helper's argv. Each value-bearing flag accepts both `--flag value` and `--flag=value`; `--tags` accepts a - * comma-separated list, `--impact` accepts one declared impact level, and `--allow-pushed` is a boolean flag. Unknown + * comma-separated list, and `--impact` accepts one declared impact level. Unknown * flags, an unexpected positional, an empty value for any value-bearing flag, a missing `--summary`, an out-of-enum * `--impact`, or an `--amend` id that is not a bare filename stem throw with a usage-style message. The body comes from * stdin rather than the command line, so the layout is flag-only. @@ -220,7 +218,6 @@ export function parseArgs(argv: readonly string[]): ParsedArgs { tags: raw.tags === undefined ? [] : parseTagList(raw.tags), impact, amend, - allowPushed: flags.some((flag) => flag.name === 'allow-pushed'), }; } @@ -230,10 +227,8 @@ export function parseArgs(argv: readonly string[]): ParsedArgs { * Amends an existing event in place. It rewrites `summary` and the body from the invocation and overrides * `skill`/`model`/`tags`/`impact` only when they are supplied, preserving everything else: provenance (`id`, * `captured-at`, `session`, `cwd`, `repo`, `harness`), `addressed-by`, and any curatorial field the caller did not - * restate. The overwrite is refused when the event is already pushed to the store's remote unless `--allow-pushed` was - * given, so a pushed event stays immutable while an unpushed one remains editable. The filename id is authoritative, so - * a corrupted frontmatter id cannot redirect the write. A missing or unparseable target becomes a structured - * `amend-not-found`/`amend-parse` result. + * restate. The filename id is authoritative, so a corrupted frontmatter id cannot redirect the write. A missing or + * unparseable target becomes a structured `amend-not-found`/`amend-parse` result. */ async function amendEvent(input: { args: ParsedArgs; @@ -270,14 +265,6 @@ async function amendEvent(input: { }; } - if (!args.allowPushed && (await isEventPushed({ storePath: store.path, id }))) { - return { - ok: false, - error: 'event-pushed', - message: `event ${id} is already pushed to the remote; re-run with --allow-pushed to amend it anyway, or capture a new event instead`, - }; - } - const updated = amendRecord(parsed.record, args, body); const rendered = renderEvent(updated); diff --git a/packages/agents/src/capture-event/event-push-state.ts b/packages/agents/src/capture-event/event-push-state.ts deleted file mode 100644 index 6ac25a98..00000000 --- a/packages/agents/src/capture-event/event-push-state.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { execFile } from 'node:child_process'; -import { promisify } from 'node:util'; - -import { buildEventPath } from '@codeassembly/kb/layout'; - -const execFileAsync = promisify(execFile); - -/** - * Reports whether an event has been pushed to the store's git remote, which is what makes it immutable. For a - * git-backed KB, "pushed" means the event's file is present in the working tree's configured upstream (`@{upstream}`, - * typically `origin/main`). The check asks git whether the event's file exists in that commit. `git push` - * updates the local upstream ref, so this is an offline, fetch-free read of the last pushed state. - * - * Any indeterminate case — the store is not a git repository, has no upstream configured, or `git` is unavailable — - * resolves to `false`, treating the event as unpushed and therefore editable. Immutability begins only at a confirmed - * push. - */ -export async function isEventPushed(input: { storePath: string; id: string }): Promise { - // `@{upstream}` reads the branch's configured upstream rather than a fixed ref name, so a store tracking anything - // other than `origin/main` is handled without special-casing. - const objectPath = buildEventPath(input.id); - try { - await execFileAsync('git', ['-C', input.storePath, 'cat-file', '-e', `@{upstream}:${objectPath}`]); - return true; - } catch { - return false; - } -} diff --git a/packages/agents/src/capture-event/prepare-event.ts b/packages/agents/src/capture-event/prepare-event.ts index 8c3b489c..927fc483 100644 --- a/packages/agents/src/capture-event/prepare-event.ts +++ b/packages/agents/src/capture-event/prepare-event.ts @@ -34,8 +34,7 @@ export type PrepareOutcome = PrepareSuccess | PrepareFailure; * carries the stored `recordType: event` discriminant and the typed event spine (`id`, `captured-at`, `cwd`, `summary`, * plus `session` when the harness exposes one and any supplied `tags`/`impact`); `repo`/`skill`/`model`/`harness` have * no typed field and ride in `extra`, which `renderEvent` emits after the spine. No `updated`/`last-verified` field is - * written: an event carries a single canonical state, editable in place via `capture-event --amend` until it is pushed - * and immutable after. + * written: an event carries a single canonical state, editable in place via `capture-event --amend`. * * Rendering the composed record through the same `renderEvent`/`renderNote` path the amend path uses keeps a fresh * capture and its later amendments identical in field order. Validation round-trips the serialized note through diff --git a/packages/agents/src/capture-event/types.ts b/packages/agents/src/capture-event/types.ts index 160eaeba..f2e5e42a 100644 --- a/packages/agents/src/capture-event/types.ts +++ b/packages/agents/src/capture-event/types.ts @@ -28,8 +28,6 @@ export interface ParsedArgs { impact: EventImpact | null; /** The id of an existing event to amend instead of capturing a new one; `null` for a fresh capture. */ amend: string | null; - /** Whether to amend an event even though it is already pushed; ignored for a fresh capture. */ - allowPushed: boolean; } /** The auto-filled context an event carries beyond the agent-supplied fields. */ @@ -75,8 +73,7 @@ export type CaptureErrorCode = | 'no-default-store' | 'schema-validation' | 'amend-not-found' - | 'amend-parse' - | 'event-pushed'; + | 'amend-parse'; /** The helper's full stdout payload: a discriminated union on `ok`. */ export type CaptureResult = CaptureSuccess | CaptureFailure;