-
Notifications
You must be signed in to change notification settings - Fork 1
feat(harness): govern ephemeral writes — see them, bound them, block the evasion #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| # Multi-edit keep-vs-delete review — scheduled 2026-08-12 | ||
|
|
||
| **Status:** deferred decision, awaiting execution | ||
| **Decided:** 2026-08-05 — delete, but not for one week | ||
| **Executed by:** cloud routine `trig_01D81bmfV4qWbt8NeRasDs3T` (one-shot, 2026-08-12 09:00 EDT) | ||
|
|
||
| This file is the routine's entire brief. It assumes zero context. | ||
|
|
||
| ## The decision being revisited | ||
|
|
||
| `interlinked multi-edit` was slated for deletion on 2026-08-05. The deletion was | ||
| held for one week so its replacement could accumulate real-world mileage. That | ||
| week is up. | ||
|
|
||
| ## Why it was slated for deletion | ||
|
|
||
| **It documents its own obsolescence.** `src/commands/multi-edit.ts` lines 11–15: | ||
|
|
||
| > This exists because the Edit tool applies one replacement at a time, and the | ||
| > tsc/biome diff-overlays check each intermediate state. Coordinated changes | ||
| > that cross multiple sites in one file … deadlock under serial Edits because | ||
| > one half of the change is invalid without the other. | ||
|
|
||
| That is a bypass lane around a gate this repo installs itself — not a capability | ||
| an agent would otherwise want. | ||
|
|
||
| **The gate stopped deadlocking.** `src/harness/transient-debt.ts` plus | ||
| `src/harness/evaluator/transient-debt-guard.ts` now *defer* that finding class: | ||
| the write is allowed, a transient debt opens, and the counterpart edit | ||
| discharges it. Verified live 2026-08-05 against the running daemon — a simulated | ||
| write adding an import of a not-yet-existing symbol returned `ALLOWED` with | ||
| `[interlinked:transient-debt] … Land that half next`. | ||
|
|
||
| **The usage record shows no compelling case** (measured 2026-08-05 over | ||
| `.interlinked/activity.jsonl` and `scratchpad-archive/`): | ||
|
|
||
| | Evidence | Result | | ||
| |---|---| | ||
| | Real invocations | ~40, **every one** via `--manifest <scratchpad file>` | | ||
| | Archived uses of the multi-file `batches` shape | **0** | | ||
| | Multi-file uses in live scratchpads | 4, all one session, all 2-file | | ||
| | Those 4 | union-member + `Record` key, config field + consumer — exactly what transient debt now allows | | ||
|
|
||
| **Its interface manufactured ephemeral writes.** `--manifest` takes a file path, | ||
| so every use staged a throwaway JSON manifest in a temp directory. ~20 of the | ||
| 148 archived scratchpad artifacts are these manifests. (`--stdin` always | ||
| accepted the multi-file `{batches}` form with no temp file; the help text | ||
| wrongly claimed it needed a positional `<path>`, which is what drove the | ||
| file-staging. Fixed 2026-08-05.) | ||
|
|
||
| ## Keep-side arguments — test these, do not assume they failed | ||
|
|
||
| 1. **Fallback if transient debt has a hole.** It was days old at decision time, | ||
| proven on one simulated case. | ||
| 2. **Gate runs once, not N times.** A 6-file refactor pays the tsc/biome overlay | ||
| once. Latency only. | ||
| 3. **True all-or-none across files.** Transient debt lets a half-landed state | ||
| exist on disk between edits; multi-edit never does. Git covers this. | ||
| 4. **Already written and tested.** ~1,015 lines of passing tests, zero | ||
| maintenance cost while nothing around it changes. | ||
|
|
||
| ## Step 1 — gather evidence (report every item; skip none) | ||
|
|
||
| - [ ] Does `src/harness/evaluator/transient-debt-guard.ts` still exist, and is | ||
| `applyTransientDebt` still called from | ||
| `src/harness/evaluator/write-content-guards.ts`? **If it was reverted or | ||
| disabled, STOP and recommend KEEP.** | ||
| - [ ] `git log --since=2026-08-05 --oneline` — any transient-debt revert, bug | ||
| fix, or commit message describing a coordinated-edit deadlock? Each is | ||
| evidence for KEEP. | ||
| - [ ] Production importers of `multi-edit*` outside `src/registrars/quality.ts` | ||
| (dynamic import) and `src/commands/completions.ts` (a string)? There were | ||
| **zero** on 2026-08-05. | ||
| - [ ] `npx vitest run src/harness/evaluator/transient-debt-guard.test.ts` — | ||
| green? | ||
| - [ ] `npm run typecheck` and the full `npx vitest run` — baseline green before | ||
| touching anything. | ||
|
|
||
| **You cannot see the local evidence.** `.interlinked/activity.jsonl`, | ||
| `ephemeral-writes.jsonl`, and `scratchpad-archive/` are gitignored and do not | ||
| exist in a cloud checkout. Say so explicitly in your report, and note that the | ||
| usage half of the argument rests on the 2026-08-05 measurement recorded above. | ||
|
|
||
| ## Step 2 — decide | ||
|
|
||
| Delete only if **all** hold: | ||
|
|
||
| 1. Transient debt is present, wired, and its tests pass. | ||
| 2. No commit since 2026-08-05 indicates it was reverted, disabled, or worked | ||
| around. | ||
| 3. No new production importer of the multi-edit modules appeared. | ||
|
|
||
| Otherwise **KEEP** and report which condition failed. A keep is a legitimate | ||
| outcome, not a failure of this task. | ||
|
|
||
| ## Step 3 — execute the deletion (only if Step 2 says delete) | ||
|
|
||
| 1. Delete `src/commands/multi-edit.ts`, `multi-edit-apply.ts`, | ||
| `multi-edit-manifest.ts`, `multi-edit.test.ts`, | ||
| `src/commands/__tests__/multi-edit.test.ts`. | ||
| 2. Remove the `multi-edit` command block from `src/registrars/quality.ts` and | ||
| the `"multi-edit"` entry from `src/commands/completions.ts`. | ||
| 3. Remove the `--batch` path from `src/commands/write.ts` and its registrar | ||
| option — the second overlapping primitive for the same non-problem. | ||
| 4. `src/registrars/quality.test.ts` pins the option list and **will fail** — | ||
| update it. | ||
| 5. Check these before assuming they are clean: | ||
| - `isTscFindingBlocking` is only *re-exported* by `multi-edit-apply.ts`; | ||
| canonical home is `src/harness/diff-overlay.ts:176`. Re-point importers. | ||
| - `countOccurrences` in `multi-edit-apply.ts` duplicates | ||
| `src/harness/edit-diagnostics.ts:219`. Deleting removes the clone; check | ||
| nothing imported the multi-edit copy. | ||
| - `atomicBatchWrite` had zero consumers. | ||
| 6. Grep for the string `multi-edit` across `src/`, `docs/`, `skills/` and update | ||
| every hit. Known: `skills/interlinked-verify/SKILL.md` (lines ~94–141), | ||
| `docs/design/multi-edit-atomic-coordinated-edits.md` (mark superseded, do not | ||
| delete the design record), `docs/generated/cli-reference.md` (regenerate with | ||
| `npm run docs`). | ||
| 7. The block message in `src/harness/evaluator/pre-tool-rules.ts` mentions | ||
| `interlinked multi-edit --stdin` as the atomic escape hatch. If the command | ||
| is gone, that clause must go too — steer entirely to sequential Edits and | ||
| transient debt. `src/harness/evaluator/pre-tool-rules.test.ts` pins this | ||
| message; update it. | ||
| 8. `npm run typecheck && npx vitest run` must be green. | ||
| 9. Open a PR titled `refactor: delete multi-edit — transient debt replaced its | ||
| reason to exist`. Do **not** push to `main`. | ||
|
|
||
| ## Step 4 — report | ||
|
|
||
| State the verdict, every evidence item with its result, what you changed, the | ||
| PR link, and anything you could not verify from a cloud checkout. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,11 +187,17 @@ async function readStdin(): Promise<string> { | |
| /** | ||
| * Commander action handler for `interlinked multi-edit`. | ||
| * | ||
| * Supports two invocation shapes: | ||
| * Supports three invocation shapes. Stdin is the preferred one for BOTH | ||
| * single- and multi-file work — it needs no temp file, which matters because | ||
| * the whole point of this command is to unblock coordinated edits, and making | ||
| * the agent stage a manifest on disk first just relocates the friction: | ||
| * interlinked multi-edit --stdin | ||
|
Comment on lines
+190
to
+194
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This change makes AGENTS.md reference: AGENTS.md:L37-L37 Useful? React with 👍 / 👎. |
||
| * Multi-file manifest ({ version: 1, batches: [{ path, edits }] }) on | ||
| * stdin. No positional path. THE default for coordinated cross-file edits. | ||
| * interlinked multi-edit <path> --stdin | ||
| * Reads a single-file manifest ({ version: 1, edits: [...] }) from stdin. | ||
| * Single-file manifest ({ version: 1, edits: [...] }) on stdin. | ||
| * interlinked multi-edit --manifest <path> | ||
| * Reads a single-file OR multi-file manifest from `path`. | ||
| * Either shape, read from a manifest already on disk. | ||
| */ | ||
| export async function multiEditCommand( | ||
| path: string | undefined, | ||
|
|
@@ -223,7 +229,7 @@ export async function multiEditCommand( | |
| error_detail: { | ||
| path: path || "", | ||
| message: | ||
| "Must supply either `<path> --stdin` (single file, manifest on stdin) or `--manifest <file>` (single or multi-file manifest).", | ||
| "Must supply --stdin or --manifest. Preferred (no temp file): pipe {version:1,batches:[{path,edits}]} to `interlinked multi-edit --stdin` for any number of files, or {version:1,edits:[...]} with a <path> for one file. `--manifest <file>` reads the same shapes from disk.", | ||
| }, | ||
| }); | ||
| process.exitCode = 1; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| // Tests for the ephemeral-write ledger: classification (the `.json` blind spot | ||
| // the placement guard never saw) and the never-throw append contract. | ||
|
|
||
| import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join } from "node:path"; | ||
| import { afterEach, describe, expect, it } from "vitest"; | ||
| import { | ||
| appendEphemeralWrite, | ||
| buildEphemeralWriteRecord, | ||
| classifyEphemeralWrite, | ||
| } from "./ephemeral-write-log.js"; | ||
|
|
||
| const temps: string[] = []; | ||
| const makeRoot = (withInterlinked: boolean): string => { | ||
| const root = mkdtempSync(join(tmpdir(), "ephemeral-log-")); | ||
| temps.push(root); | ||
| if (withInterlinked) mkdirSync(join(root, ".interlinked"), { recursive: true }); | ||
| return root; | ||
| }; | ||
|
|
||
| afterEach(() => { | ||
| while (temps.length > 0) { | ||
| const p = temps.pop(); | ||
| if (p) rmSync(p, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| describe("classifyEphemeralWrite", () => { | ||
| it("classifies code extensions", () => { | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/probe.mjs")).toBe("code"); | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/fix.py")).toBe("code"); | ||
| }); | ||
|
|
||
| it("classifies the .json manifest blind spot", () => { | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/def.json")).toBe("manifest"); | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/ci.yml")).toBe("manifest"); | ||
| }); | ||
|
|
||
| it("classifies captured external-agent output", () => { | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/codex-review-2-result.md")).toBe( | ||
| "agent-output", | ||
| ); | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/sol-audit.md")).toBe("agent-output"); | ||
| }); | ||
|
|
||
| it("does not claim every markdown note is agent output", () => { | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/notes.md")).toBe("other"); | ||
| }); | ||
|
|
||
| it("classifies bulk downloads", () => { | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/pkg.tgz")).toBe("bulk"); | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/shot.png")).toBe("bulk"); | ||
| }); | ||
|
|
||
| it("falls back to other for extensionless files", () => { | ||
| expect(classifyEphemeralWrite("/tmp/s/scratchpad/Makefile")).toBe("other"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("buildEphemeralWriteRecord", () => { | ||
| it("captures tool, byte length, extension, and blocked flag", () => { | ||
| const rec = buildEphemeralWriteRecord({ | ||
| sessionId: "s1", | ||
| tool: "Write", | ||
| absPath: "/tmp/s/scratchpad/def.json", | ||
| content: '{"a":1}', | ||
| blocked: true, | ||
| now: () => "2026-08-04T00:00:00.000Z", | ||
| }); | ||
| expect(rec).toEqual({ | ||
| ts: "2026-08-04T00:00:00.000Z", | ||
| session_id: "s1", | ||
| tool: "Write", | ||
| path: "/tmp/s/scratchpad/def.json", | ||
| ext: ".json", | ||
| bytes: 7, | ||
| kind: "manifest", | ||
| blocked: true, | ||
| }); | ||
| }); | ||
|
|
||
| it("measures bytes, not characters", () => { | ||
| const rec = buildEphemeralWriteRecord({ | ||
| sessionId: undefined, | ||
| tool: "Edit", | ||
| absPath: "/tmp/s/scratchpad/x.txt", | ||
| content: "é", | ||
| blocked: false, | ||
| }); | ||
| expect(rec.bytes).toBe(2); | ||
| }); | ||
| }); | ||
|
|
||
| describe("appendEphemeralWrite", () => { | ||
| const record = buildEphemeralWriteRecord({ | ||
| sessionId: "s1", | ||
| tool: "Write", | ||
| absPath: "/tmp/s/scratchpad/a.json", | ||
| content: "{}", | ||
| blocked: false, | ||
| now: () => "2026-08-04T00:00:00.000Z", | ||
| }); | ||
|
|
||
| it("appends one JSON line per call", () => { | ||
| const root = makeRoot(true); | ||
| appendEphemeralWrite(root, record); | ||
| appendEphemeralWrite(root, record); | ||
| const lines = readFileSync(join(root, ".interlinked", "ephemeral-writes.jsonl"), "utf-8") | ||
| .trim() | ||
| .split("\n"); | ||
| expect(lines).toHaveLength(2); | ||
| expect(JSON.parse(lines[0] as string).kind).toBe("manifest"); | ||
| }); | ||
|
|
||
| it("no-ops when .interlinked/ is absent rather than creating it", () => { | ||
| const root = makeRoot(false); | ||
| appendEphemeralWrite(root, record); | ||
| expect(existsSync(join(root, ".interlinked"))).toBe(false); | ||
| }); | ||
|
|
||
| it("never throws on an unwritable root", () => { | ||
| expect(() => appendEphemeralWrite("/proc/nonexistent-root", record)).not.toThrow(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds the simulation marker, but the PreToolUse pipeline still runs
evaluateAutoReservation, which ignoresevent.dry_runand callsreservations.checkAndReservefor every Write/Edit path (inspectedsrc/harness/evaluator/pre-tool-decision-phases.ts:226-240). Forinterlinked harness test --write src/foo.tsthat otherwise allows, the test can still leave atestlease and make a later real edit hit a reservation conflict, so the dry-run marker needs to suppress all persistence phases, not only the new ephemeral ledger.Useful? React with 👍 / 👎.