#991 Remove client-side event-immutability enforcement

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 <id>` 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 <id>` 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.

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 <id>` 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 <id>` 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
 

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`.

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<string> {
   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<void> {
-  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<void> {
-  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<void> {
-  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.');
-    }
-  });
 });

packages/agents/src/capture-event/__tests__/prepare-event.test.ts

@@ -18,7 +18,6 @@ function argsFor(overrides: Partial<ParsedArgs>): ParsedArgs {
     tags: [],
     impact: null,
     amend: null,
-    allowPushed: false,
     ...overrides,
   };
 }