From 2949d429be05aea44b2923e37eb0a28bb1f32c1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Sun, 25 Oct 2026 12:47:52 -0300 Subject: [PATCH 1/4] Format the Rust code with rustfmt cargo fmt --check failed on 8 spots in ai.rs and lib.rs. This commit only reformats them, so the next one can add the check to pnpm verify. --- src-tauri/src/ai.rs | 25 +++++++++++++++++++------ src-tauri/src/lib.rs | 19 +++++++++++++++---- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src-tauri/src/ai.rs b/src-tauri/src/ai.rs index 8c83e71..0f1dbb0 100644 --- a/src-tauri/src/ai.rs +++ b/src-tauri/src/ai.rs @@ -37,7 +37,11 @@ fn api_key() -> Option { return Some(key); } } - keychain().ok()?.get_password().ok().filter(|k| !k.trim().is_empty()) + keychain() + .ok()? + .get_password() + .ok() + .filter(|k| !k.trim().is_empty()) } #[tauri::command] @@ -157,7 +161,9 @@ async fn send( if status.is_success() { return Ok(Ok(value)); } - let message = value["error"]["message"].as_str().unwrap_or("unknown error"); + let message = value["error"]["message"] + .as_str() + .unwrap_or("unknown error"); Ok(Err(match status.as_u16() { 401 => "no_key: The API key was rejected. Check it and try again.".to_string(), 429 => format!("api: Rate limited. Try again in a moment. ({message})"), @@ -169,10 +175,14 @@ async fn send( fn parse_response(response: &Value) -> Result { match response["stop_reason"].as_str() { Some("refusal") => { - let why = response["stop_details"]["explanation"].as_str().unwrap_or("no reason given"); + let why = response["stop_details"]["explanation"] + .as_str() + .unwrap_or("no reason given"); return Err(format!("refused: Claude declined this request ({why}).")); } - Some("max_tokens") => return Err("api: The answer was cut off. Try a shorter selection.".into()), + Some("max_tokens") => { + return Err("api: The answer was cut off. Try a shorter selection.".into()) + } _ => {} } let text = response["content"] @@ -223,7 +233,10 @@ mod tests { #[test] fn reports_refusals() { - let response = json!({"stop_reason": "refusal", "stop_details": {"explanation": "x"}, "content": []}); - assert!(parse_response(&response).unwrap_err().starts_with("refused:")); + let response = + json!({"stop_reason": "refusal", "stop_details": {"explanation": "x"}, "content": []}); + assert!(parse_response(&response) + .unwrap_err() + .starts_with("refused:")); } } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 6f68699..2e8c887 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -9,11 +9,17 @@ const DOCUMENT_EXTENSIONS: [&str; 3] = ["md", "markdown", "txt"]; fn document_path(path: &str) -> Result<&Path, String> { let target = Path::new(path); - let ext = target.extension().and_then(|e| e.to_str()).unwrap_or("").to_lowercase(); + let ext = target + .extension() + .and_then(|e| e.to_str()) + .unwrap_or("") + .to_lowercase(); if DOCUMENT_EXTENSIONS.contains(&ext.as_str()) { Ok(target) } else { - Err(format!("Undertext only opens and saves .md, .markdown, and .txt files: {path}")) + Err(format!( + "Undertext only opens and saves .md, .markdown, and .txt files: {path}" + )) } } @@ -50,7 +56,9 @@ fn write_text(path: String, contents: String, expected: Option) -> Resul /// Only web and mail links, with no spaces or control characters. fn safe_url(url: &str) -> Result<&str, String> { let lower = url.to_lowercase(); - let scheme_ok = ["http://", "https://", "mailto:"].iter().any(|s| lower.starts_with(s)); + let scheme_ok = ["http://", "https://", "mailto:"] + .iter() + .any(|s| lower.starts_with(s)); if scheme_ok && !url.chars().any(|c| c.is_whitespace() || c.is_control()) { Ok(url) } else { @@ -115,7 +123,10 @@ mod tests { assert!(write_text(path.clone(), "edited".into(), Some("mine".into())).is_ok()); // Another app changed it: refused, and the file keeps their text. fs::write(&file, "theirs").unwrap(); - assert_eq!(write_text(path.clone(), "again".into(), Some("edited".into())), Err(CHANGED_ON_DISK.into())); + assert_eq!( + write_text(path.clone(), "again".into(), Some("edited".into())), + Err(CHANGED_ON_DISK.into()) + ); assert_eq!(fs::read_to_string(&file).unwrap(), "theirs"); // Without `expected` (Keep mine), it overwrites. assert!(write_text(path.clone(), "again".into(), None).is_ok()); From 9ad77e78131f3255295cf97baf6f3b79c0a1771c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Sun, 25 Oct 2026 12:47:58 -0300 Subject: [PATCH 2/4] Add pnpm verify, a Definition of Done, and CI pnpm verify runs every check: both TypeScript projects, vitest, the web build, and cargo fmt, clippy, and tests. It takes about 5 seconds warm. It records a hash of the tree in .git/undertext/ only when every check passes and no file changed during the run, so a later hook can tell whether a change was verified. A docs-only change needs no run. A change to CI files alone is never recorded. - CLAUDE.md says what done means and to run pnpm verify. - .github/workflows/verify.yml runs it on macOS for pushes to main and pull requests. The actions are pinned by commit. - docs/agents.md says what is checked, what is only asked, and the known limits. - scripts/verify.test.mjs runs copies of the scripts in throwaway repositories. - .gitignore now covers .claude/settings.local.json. Claude Code writes it, and a change to it would void a pass. --- .github/workflows/verify.yml | 36 ++++++ .gitignore | 1 + CLAUDE.md | 21 +++- README.md | 7 +- docs/agents.md | 90 +++++++++++++++ package.json | 1 + scripts/tree.mjs | 87 +++++++++++++++ scripts/verify-steps.mjs | 43 ++++++++ scripts/verify.mjs | 95 ++++++++++++++++ scripts/verify.test.mjs | 207 +++++++++++++++++++++++++++++++++++ 10 files changed, 582 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/verify.yml create mode 100644 docs/agents.md create mode 100644 scripts/tree.mjs create mode 100644 scripts/verify-steps.mjs create mode 100644 scripts/verify.mjs create mode 100644 scripts/verify.test.mjs diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml new file mode 100644 index 0000000..ab483eb --- /dev/null +++ b/.github/workflows/verify.yml @@ -0,0 +1,36 @@ +# Runs `pnpm verify`, the same checks as on your Mac, on every push to main and every pull request. +# macOS because the app is built and tested there, and Tauri on Linux needs extra WebKit packages. +name: verify + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +# A newer push to the same branch cancels the older run. A cancelled run is not a failure. +concurrency: + group: verify-${{ github.ref }} + cancel-in-progress: true + +jobs: + verify: + runs-on: macos-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0 + with: + version: 10.28.1 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: 22 + cache: pnpm + - run: rustup component add clippy rustfmt + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + workspaces: src-tauri + - run: pnpm install --frozen-lockfile + - run: pnpm verify diff --git a/.gitignore b/.gitignore index a547bf3..931e878 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ node_modules dist dist-ssr *.local +.claude/settings.local.json # Editor directories and files .vscode/* diff --git a/CLAUDE.md b/CLAUDE.md index 06a173a..9123672 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,13 +12,26 @@ What comes next, and the decisions behind the editor: the "Next" section of pnpm install pnpm dev # the editor in a plain browser: no files, fake AI results pnpm tauri dev # the desktop app, with live reload -pnpm test # editor, format, Lab, and palette tests (vitest, no DOM) -pnpm exec tsc --noEmit -p . # type check -cd src-tauri && cargo test --lib # Rust tests +pnpm verify # every check CI runs, in about 10 seconds +pnpm test # only the vitest tests pnpm tauri build --bundles app # src-tauri/target/release/bundle/macos/Undertext.app ``` -`cargo` must be on `PATH`. With Homebrew's rustup it lives in `/opt/homebrew/opt/rustup/bin`. +`pnpm tauri` needs `cargo` on `PATH`. With Homebrew's rustup it lives in +`/opt/homebrew/opt/rustup/bin`. `pnpm verify` finds it there on its own. + +## Done + +- Done means proven: a test that failed before the change and passes after, or the change seen + in the app. Reading the code or a passing compile is not proof. +- Run `pnpm verify` after each change, and before you call anything done. It records the tree + when every check passes on it, or when only docs changed since its last pass. +- It can't see the app window (WebKit), real AI calls, file dialogs, or the keychain. When a + change depends on them, check it in `pnpm tauri dev`, or say it is not verified. +- Never weaken a check to pass it: no skipped or deleted tests, no loosened assertions. If a + check is wrong, stop and say why. +- Report what ran, what it showed, and what you did not verify. How this works: + `docs/agents.md`. ## How the editor works diff --git a/README.md b/README.md index fe9a822..e16e9f6 100644 --- a/README.md +++ b/README.md @@ -166,11 +166,14 @@ limits. If `claude` isn't in `~/.local/bin`, `/opt/homebrew/bin`, or `/usr/local ## Development ```bash -pnpm test # editor and format tests +pnpm verify # every check CI runs: types, tests, web build, Rust format, lint, and tests +pnpm test # only the vitest tests pnpm icons # rebuild the app icons from brand/ (needs ImageMagick) -cd src-tauri && cargo test ``` +`pnpm verify` is also what coding agents run to prove their work. See +[docs/agents.md](docs/agents.md). + The editor is [CodeMirror](https://codemirror.net) inside a [Tauri](https://tauri.app) app. The page text is plain Markdown; alternatives, ghosts, and versions are ranges kept beside it (`src/editor/layers.ts`). The icon borrows the mark of [Klipstack](brand/README.md), with a handwritten u in place of its clip. diff --git a/docs/agents.md b/docs/agents.md new file mode 100644 index 0000000..99e47dc --- /dev/null +++ b/docs/agents.md @@ -0,0 +1,90 @@ +# Working with coding agents + +Undertext is built with coding agents, mostly Claude Code. This page says what the repository +checks on its own, what it only asks agents to do, and what is still up to you. + +The idea: an agent proves its work before it calls it done, without you asking. A passing check +is proof; "I read the code" is not. + +## The layers so far + +1. **`pnpm verify`** runs every check, in about 10 seconds, and records the tree when they all + pass. +2. **CI** runs `pnpm verify` on GitHub for every push to `main` and every pull request. +3. **`CLAUDE.md`** tells every agent session what done means and to run `pnpm verify`. + +## Rules, and what holds them + +| Rule | What holds it | How sure | +| --- | --- | --- | +| Types, tests, web build, and Rust format, lint, and tests pass | `pnpm verify`, then CI | Checked. CI shows a red run, but nothing blocks a push to `main`. | +| Run `pnpm verify` before calling work done | `CLAUDE.md` | Asked | +| Never weaken a check to pass it | `CLAUDE.md` | Asked | +| Say what was not verified | `CLAUDE.md` | Asked | + +"Asked" means an agent can forget it. Later steps turn some of these into checks. + +## `pnpm verify` + +It runs, in order, and prints one line per check: + +| Check | Command | +| --- | --- | +| types | `tsc --noEmit -p .` | +| config types | `tsc` on `vite.config.ts` | +| tests | `vitest run`, with no `.only` allowed (fails if no test ran) | +| web build | `vite build`, into a scratch folder, so `dist/` stays as it was | +| rust format | `cargo fmt --check` | +| rust lint | `cargo clippy --all-targets --locked -- -D warnings` | +| rust tests | `cargo test --lib --locked` (fails if no test ran) | + +- It never picks a subset of the checks. They are fast, so there is nothing to gain. It runs all + of them, or none in the three cases below. +- On a failure it prints the end of that check's log and the full log's path. Logs live in + `.git/undertext/logs/`, or under `.git/worktrees//` in a linked worktree. +- **It records a pass** only when every check passed and no file changed during the run. The + record is a hash of every file's contents, untracked files included, in + `.git/undertext/verified.json`. Each worktree keeps its own. A failed run deletes the record. +- **Nothing changed** since the last pass: it says so and stops. `pnpm verify --force` runs the + checks anyway, for example after `rustup update`. +- **Only docs changed** since the last pass (`docs/`, `LICENSE`, and `*.md` outside `src/` and + `src-tauri/`): nothing to check. It records the new tree. +- **Only CI files changed** (`.github/`): local checks can't see them. It records nothing, even + with `--force`. Push and read the CI run. +- It finds `cargo` on `PATH`, in `~/.cargo/bin`, or in Homebrew's rustup folder. Without cargo it + fails. It never skips a check it can't run. + +**What it can't see:** the app window, which is WebKit, while `pnpm dev` runs in Chromium; real AI +calls; file dialogs; and the keychain. For those, run `pnpm tauri dev` and try the change. + +**Known limits:** + +- It compares the tree before and after the run. An edit made and undone while it runs goes + unseen. +- It checks the files on disk, not what is staged. When you stage only part of a change, the + commit holds a tree that was never checked. CI checks the commit after you push. +- A stray `.only` fails it, as in CI. A `.skip` does not. + +The script is `scripts/verify.mjs`, the checks are in `scripts/verify-steps.mjs`, and its tests are +in `scripts/verify.test.mjs`. The tests run copies of the scripts in throwaway repositories. + +## CI + +`.github/workflows/verify.yml` runs `pnpm verify` on a GitHub macOS machine. It is free for public +repositories. + +- **Why macOS:** the app is built and tested there. On Linux, Tauri needs extra WebKit packages. +- **It is a signal, not a gate.** `main` has no branch protection, so a red run blocks nothing. + You push to `main` yourself, and that stays as it is. +- **Cancelled runs are not failures.** A newer push to the same branch cancels the older run. A + run that never got a machine or timed out needs a re-run, not a code change. +- **Rust is not pinned.** CI uses the latest stable Rust. A new Clippy lint can fail CI before it + fails on your Mac. Run `rustup update`, then `pnpm verify --force`. +- Each action is pinned to one commit, with its version in a comment, so a changed tag can't + change what runs. + +## Where the ideas come from + +Kent Beck, Simon Willison, Mitchell Hashimoto, Anthropic's and Amp's guidance on coding agents, +OpenAI's harness engineering, Lauren Tan's pstack, Matt Pocock's skills, and Jesse Vincent's +superpowers. diff --git a/package.json b/package.json index 8ab0a40..d6d16a1 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "preview": "vite preview", "tauri": "tauri", "test": "vitest run", + "verify": "node scripts/verify.mjs", "icons": "node scripts/icons.mjs" }, "dependencies": { diff --git a/scripts/tree.mjs b/scripts/tree.mjs new file mode 100644 index 0000000..1f94107 --- /dev/null +++ b/scripts/tree.mjs @@ -0,0 +1,87 @@ +// The checkout's files by content hash, and the record of the last tree that passed `pnpm verify`. +import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import { lstatSync, mkdirSync, readFileSync, readlinkSync, rmSync, writeFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; + +/** The environment without git's own variables, so git acts on `root` even when run from a git hook. */ +let cleanEnv; +function gitEnv() { + if (!cleanEnv) { + cleanEnv = { ...process.env }; + for (const name of execFileSync("git", ["rev-parse", "--local-env-vars"], { encoding: "utf8" }).split("\n")) { + if (name) delete cleanEnv[name]; + } + } + return cleanEnv; +} + +export const git = (root, ...args) => + execFileSync("git", ["-C", root, ...args], { env: gitEnv(), encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); + +/** + * Every file git sees in `root`: tracked, staged and untracked, but not ignored. Hashes contents, so + * an edit in the same second counts, and writes nothing to git's object store. + */ +export function snapshot(root) { + const paths = new Set(git(root, "ls-files", "-z", "--cached", "--others", "--exclude-standard").split("\0")); + const files = {}; + for (const path of [...paths].filter(Boolean).sort()) { + const hash = hashFile(join(root, path)); + if (hash) files[path] = hash; + } + const fingerprint = createHash("sha256").update(JSON.stringify(files)).digest("hex"); + return { fingerprint, files }; +} + +function hashFile(file) { + let stat; + try { + stat = lstatSync(file); + } catch { + return null; // deleted + } + if (stat.isSymbolicLink()) return `link:${readlinkSync(file)}`; + if (!stat.isFile()) return null; + const mode = stat.mode & 0o111 ? "x" : "-"; + return createHash("sha256").update(mode).update(readFileSync(file)).digest("hex"); +} + +/** Paths that differ between two snapshots' files. */ +export function changedPaths(before, after) { + const paths = new Set([...Object.keys(before), ...Object.keys(after)]); + return [...paths].filter((path) => before[path] !== after[path]).sort(); +} + +/** + * Docs: no check reads them, so a change to them alone needs no verify. Markdown under src/ or + * src-tauri/ may be a fixture the code reads, so it counts as code. + */ +export const isDoc = (path) => + !/^src(-tauri)?\//.test(path) && (path.endsWith(".md") || path.startsWith("docs/") || path === "LICENSE"); + +/** Only CI reads these. Local checks can't vouch for them. */ +export const isCiOnly = (path) => path.startsWith(".github/"); + +/** Per checkout: a linked worktree keeps its own record. */ +export const stateDir = (root) => resolve(root, git(root, "rev-parse", "--git-path", "undertext").trim()); + +const passFile = (root) => join(stateDir(root), "verified.json"); + +/** The last pass, or null when there is none or the record is unreadable. */ +export function readPass(root) { + try { + const pass = JSON.parse(readFileSync(passFile(root), "utf8")); + return typeof pass?.fingerprint === "string" && pass.files && typeof pass.files === "object" ? pass : null; + } catch { + return null; + } +} + +export function recordPass(root, snap) { + const file = passFile(root); + mkdirSync(dirname(file), { recursive: true }); + writeFileSync(file, JSON.stringify({ ...snap, at: new Date().toISOString() })); +} + +export const forgetPass = (root) => rmSync(passFile(root), { force: true }); diff --git a/scripts/verify-steps.mjs b/scripts/verify-steps.mjs new file mode 100644 index 0000000..36a0cb5 --- /dev/null +++ b/scripts/verify-steps.mjs @@ -0,0 +1,43 @@ +// The checks `pnpm verify` runs, in order. CI runs the same list through `pnpm verify`. +import { existsSync } from "node:fs"; +import { homedir } from "node:os"; +import { delimiter, join } from "node:path"; +import { stateDir } from "./tree.mjs"; + +/** The folder with cargo: PATH, rustup's default, or Homebrew's rustup, which fish leaves off PATH. */ +function cargoDir() { + const dirs = [...(process.env.PATH ?? "").split(delimiter), join(homedir(), ".cargo/bin"), "/opt/homebrew/opt/rustup/bin"]; + return dirs.find((dir) => dir && existsSync(join(dir, "cargo"))) ?? null; +} + +/** How many tests passed, from the runner's summary. Zero means the step checked nothing. */ +export const vitestPassed = (log) => Number(/Tests\s+.*?(\d+) passed/.exec(log)?.[1] ?? 0); +export const cargoPassed = (log) => + [...log.matchAll(/test result: \w+\. (\d+) passed/g)].reduce((sum, m) => sum + Number(m[1]), 0); + +const ranTests = (count) => (log) => (count(log) > 0 ? null : "No tests ran."); + +export function steps(root) { + const bin = (name) => join(root, "node_modules/.bin", name); + const rust = join(root, "src-tauri"); + const cargo = cargoDir(); + const cargoStep = (name, args, check) => + cargo + ? { name, cmd: join(cargo, "cargo"), args, cwd: rust, env: { PATH: `${cargo}${delimiter}${process.env.PATH}` }, check } + : { name, missing: "cargo not found. Install Rust (see README), or put cargo on PATH." }; + return [ + { name: "types", cmd: bin("tsc"), args: ["--noEmit", "-p", "."] }, + // composite would write tsconfig.node.tsbuildinfo into the tree. + { name: "config types", cmd: bin("tsc"), args: ["--noEmit", "-p", "tsconfig.node.json", "--composite", "false"] }, + // CI rejects a stray `.only`; so does this, or a local pass would fail there. + { name: "tests", cmd: bin("vitest"), args: ["run", "--allowOnly=false"], check: ranTests(vitestPassed) }, + // Outside the tree, so dist/ stays as the last real build left it. + { name: "web build", cmd: bin("vite"), args: ["build", "--outDir", join(stateDir(root), "web"), "--emptyOutDir"] }, + cargoStep("rust format", ["fmt", "--check"]), + cargoStep("rust lint", ["clippy", "--all-targets", "--locked", "--", "-D", "warnings"]), + cargoStep("rust tests", ["test", "--lib", "--locked"], ranTests(cargoPassed)), + ]; +} + +/** What these checks can't see. */ +export const unseen = "the app window (WebKit), real AI calls, file dialogs, and the keychain"; diff --git a/scripts/verify.mjs b/scripts/verify.mjs new file mode 100644 index 0000000..a40caa1 --- /dev/null +++ b/scripts/verify.mjs @@ -0,0 +1,95 @@ +// pnpm verify: every check CI runs, in seconds. Records the tree only when all of them pass on it. +// `--force` runs the checks even when nothing changed since the last pass. +import { spawnSync } from "node:child_process"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { dirname, join, relative, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { changedPaths, forgetPass, isCiOnly, isDoc, readPass, recordPass, snapshot, stateDir } from "./tree.mjs"; +import { steps, unseen } from "./verify-steps.mjs"; + +// The checkout this script lives in, wherever it is run from. +const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const force = process.argv.includes("--force"); +const seconds = (ms) => `${(ms / 1000).toFixed(1)}s`; +const list = (paths) => (paths.length > 3 ? `${paths.slice(0, 3).join(", ")} and ${paths.length - 3} more` : paths.join(", ")); + +const before = snapshot(root); +const last = readPass(root); +const changed = last ? changedPaths(last.files, before.files) : null; +// Local checks can't vouch for CI files, so a change to them alone is never recorded, even with --force. +const ciOnly = changed !== null && changed.some(isCiOnly) && changed.every((path) => isDoc(path) || isCiOnly(path)); +const ciNote = () => { + console.log(`Only CI files changed since the last pass (${list(changed.filter(isCiOnly))}).`); + console.log("Local checks can't see them. Push and read the CI run. Nothing recorded."); +}; +if (changed && !force) { + if (changed.length === 0) { + console.log(`Nothing changed since the last pass (${last.at}). Use --force to run the checks anyway.`); + process.exit(0); + } + if (changed.every(isDoc)) { + recordPass(root, before); + console.log(`Only docs changed since the last pass (${list(changed)}). Nothing to check.`); + process.exit(0); + } + if (ciOnly) { + ciNote(); + process.exit(0); + } +} + +const logs = join(stateDir(root), "logs"); +mkdirSync(logs, { recursive: true }); +const started = Date.now(); +const failed = []; + +for (const step of steps(root)) { + const log = join(logs, `${step.name.replaceAll(" ", "-")}.log`); + const begin = Date.now(); + let output; + let problem = null; + if (step.missing) { + output = step.missing; + problem = step.missing; + } else { + const result = spawnSync(step.cmd, step.args, { + cwd: step.cwd ?? root, + env: { ...process.env, ...step.env, FORCE_COLOR: "0", NO_COLOR: "1" }, + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + }); + output = `$ ${step.cmd} ${step.args.join(" ")}\n${result.stdout ?? ""}${result.stderr ?? ""}`; + if (result.error) problem = `Could not start: ${result.error.message}. Run pnpm install?`; + else if (result.status !== 0) problem = `Exited with ${result.status ?? result.signal}.`; + else problem = step.check?.(output) ?? null; + } + writeFileSync(log, problem ? `${output}\n${problem}\n` : output); + console.log(`${problem ? "✗" : "✓"} ${step.name.padEnd(13)} ${seconds(Date.now() - begin)}`); + if (problem) { + failed.push(step.name); + const tail = readFileSync(log, "utf8").trimEnd().split("\n").slice(-25); + console.log(tail.map((line) => ` ${line}`).join("\n")); + console.log(` Full log: ${relative(process.cwd(), log) || log}`); + } +} + +const after = snapshot(root); +const moved = changedPaths(before.files, after.files); +// A failed run forgets an earlier pass. +if (failed.length > 0) { + forgetPass(root); + console.log(`Failed: ${failed.join(", ")}. Nothing recorded.`); + process.exit(1); +} +if (moved.length > 0) { + forgetPass(root); + console.log(`Files changed during the run (${list(moved)}). Nothing recorded. Run it again.`); + process.exit(1); +} +if (ciOnly) { + ciNote(); + process.exit(0); +} +recordPass(root, after); +console.log(`Passed in ${seconds(Date.now() - started)}. Recorded ${after.fingerprint.slice(0, 12)}.`); +console.log(`Not checked: ${unseen}.`); diff --git a/scripts/verify.test.mjs b/scripts/verify.test.mjs new file mode 100644 index 0000000..b0c2487 --- /dev/null +++ b/scripts/verify.test.mjs @@ -0,0 +1,207 @@ +// Runs copies of the verify scripts in throwaway repositories, never in this checkout. A fake check +// stands in for the real ones: it counts its runs, fails while a `broken` file exists, and writes a +// file into the tree while a `touch` file exists. +import { spawnSync } from "node:child_process"; +import { chmodSync, copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterEach, describe, expect, it } from "vitest"; +import { git, snapshot } from "./tree.mjs"; +import { cargoPassed, vitestPassed } from "./verify-steps.mjs"; + +const here = dirname(fileURLToPath(import.meta.url)); +const made = []; +afterEach(() => made.splice(0).forEach((dir) => rmSync(dir, { recursive: true, force: true }))); + +const tempDir = () => { + const dir = mkdtempSync(join(tmpdir(), "undertext-verify-test-")); + made.push(dir); + return dir; +}; + +const FAKE_CHECK = `const fs = require("fs"); +fs.appendFileSync(process.env.RUNS, "x"); +if (fs.existsSync("broken")) process.exit(1); +if (fs.existsSync("touch")) fs.writeFileSync("made-during-run.txt", "x");`; + +const fakeSteps = (step = `{ name: "check", cmd: process.execPath, args: ["-e", ${JSON.stringify(FAKE_CHECK)}] }`) => + `export const unseen = "nothing";\nexport const steps = () => [${step}];\n`; + +function makeRepo(steps = fakeSteps()) { + const dir = tempDir(); + mkdirSync(join(dir, "scripts")); + for (const file of ["verify.mjs", "tree.mjs"]) copyFileSync(join(here, file), join(dir, "scripts", file)); + writeFileSync(join(dir, "scripts/verify-steps.mjs"), steps); + writeFileSync(join(dir, "app.js"), "ok\n"); + writeFileSync(join(dir, "README.md"), "# App\n"); + writeFileSync(join(dir, ".gitignore"), "ignored/\n"); + git(dir, "init", "-q"); + git(dir, "add", "."); + git(dir, "-c", "commit.gpgsign=false", "-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "-qm", "Start"); + return dir; +} + +function verify(dir, args = [], env = {}) { + const runs = join(tempDir(), "runs"); + writeFileSync(runs, ""); + const result = spawnSync(process.execPath, [join(dir, "scripts/verify.mjs"), ...args], { + cwd: tmpdir(), + env: { ...process.env, RUNS: runs, ...env }, + encoding: "utf8", + }); + return { status: result.status, out: result.stdout + result.stderr, runs: readFileSync(runs, "utf8").length }; +} + +const passFile = (dir) => join(dir, ".git/undertext/verified.json"); +const recorded = (dir) => (existsSync(passFile(dir)) ? JSON.parse(readFileSync(passFile(dir), "utf8")).fingerprint : null); + +describe("pnpm verify", () => { + it("records a pass, then skips an unchanged tree unless forced", () => { + const dir = makeRepo(); + const first = verify(dir); + expect(first).toMatchObject({ status: 0, runs: 1 }); + expect(first.out).toContain("✓ check"); + expect(first.out).toContain("Not checked: nothing."); + expect(recorded(dir)).toBe(snapshot(dir).fingerprint); + + const again = verify(dir); + expect(again).toMatchObject({ status: 0, runs: 0 }); + expect(again.out).toContain("Nothing changed since the last pass"); + expect(verify(dir, ["--force"])).toMatchObject({ status: 0, runs: 1 }); + }); + + it("records nothing when a check fails, and forgets the earlier pass", () => { + const dir = makeRepo(); + verify(dir); + writeFileSync(join(dir, "broken"), ""); + const failed = verify(dir); + expect(failed).toMatchObject({ status: 1, runs: 1 }); + expect(failed.out).toContain("✗ check"); + expect(failed.out).toMatch(/Full log: .*check\.log/); + expect(failed.out).toContain("Failed: check. Nothing recorded."); + expect(recorded(dir)).toBeNull(); + }); + + it("records nothing when a check exits cleanly but ran no tests", () => { + const dir = makeRepo(fakeSteps(`{ name: "tests", cmd: process.execPath, args: ["-e", ""], check: () => "No tests ran." }`)); + const result = verify(dir); + expect(result.status).toBe(1); + expect(result.out).toContain("✗ tests"); + expect(result.out).toContain("No tests ran."); + expect(recorded(dir)).toBeNull(); + }); + + it("runs the checks when the record is unreadable", () => { + const dir = makeRepo(); + mkdirSync(dirname(passFile(dir)), { recursive: true }); + writeFileSync(passFile(dir), '{"fingerprint":"abc"}'); + expect(verify(dir)).toMatchObject({ status: 0, runs: 1 }); + expect(recorded(dir)).toBe(snapshot(dir).fingerprint); + }); + + it("records nothing when files change during the run", () => { + const dir = makeRepo(); + writeFileSync(join(dir, "touch"), ""); + const result = verify(dir); + expect(result.status).toBe(1); + expect(result.out).toContain("Files changed during the run (made-during-run.txt)"); + expect(recorded(dir)).toBeNull(); + }); + + it("checks nothing when only docs changed since the last pass, and records that tree", () => { + const dir = makeRepo(); + verify(dir); + writeFileSync(join(dir, "README.md"), "# App, with docs\n"); + const result = verify(dir); + expect(result).toMatchObject({ status: 0, runs: 0 }); + expect(result.out).toContain("Only docs changed since the last pass (README.md)"); + expect(recorded(dir)).toBe(snapshot(dir).fingerprint); + + mkdirSync(join(dir, "src")); + writeFileSync(join(dir, "src/fixture.md"), "Code may read this.\n"); + expect(verify(dir)).toMatchObject({ status: 0, runs: 1 }); + }); + + it("records nothing when only CI files changed, and checks again once code changes", () => { + const dir = makeRepo(); + verify(dir); + const before = recorded(dir); + mkdirSync(join(dir, ".github/workflows"), { recursive: true }); + writeFileSync(join(dir, ".github/workflows/ci.yml"), "on: push\n"); + const ci = verify(dir); + expect(ci).toMatchObject({ status: 0, runs: 0 }); + expect(ci.out).toContain("Only CI files changed since the last pass (.github/workflows/ci.yml)"); + expect(recorded(dir)).toBe(before); + const forced = verify(dir, ["--force"]); + expect(forced).toMatchObject({ status: 0, runs: 1 }); + expect(forced.out).toContain("Nothing recorded."); + expect(recorded(dir)).toBe(before); + + writeFileSync(join(dir, "app.js"), "changed\n"); + expect(verify(dir)).toMatchObject({ status: 0, runs: 1 }); + expect(recorded(dir)).toBe(snapshot(dir).fingerprint); + }); + + it("fails clearly when a check can't start or its tool is missing", () => { + const cantStart = verify(makeRepo(fakeSteps(`{ name: "check", cmd: "/no/such/tool", args: [] }`))); + expect(cantStart.status).toBe(1); + expect(cantStart.out).toContain("Could not start"); + + const missing = verify(makeRepo(fakeSteps(`{ name: "rust", missing: "cargo not found." }`))); + expect(missing.status).toBe(1); + expect(missing.out).toContain("✗ rust"); + expect(missing.out).toContain("cargo not found."); + }); + + it("keeps a separate record in a linked worktree", () => { + const dir = makeRepo(); + const tree = join(tempDir(), "linked"); + git(dir, "worktree", "add", "-q", tree); + expect(verify(tree)).toMatchObject({ status: 0, runs: 1 }); + expect(existsSync(join(dir, ".git/worktrees", basename(tree), "undertext/verified.json"))).toBe(true); + expect(recorded(dir)).toBeNull(); + }); + + it("acts on its own checkout when git's hook variables point elsewhere", () => { + const dir = makeRepo(); + const other = makeRepo(); + const env = { GIT_DIR: join(other, ".git"), GIT_WORK_TREE: other, GIT_INDEX_FILE: join(other, ".git/index") }; + expect(verify(dir, [], env)).toMatchObject({ status: 0, runs: 1 }); + expect(recorded(dir)).toBe(snapshot(dir).fingerprint); + expect(recorded(other)).toBeNull(); + }); +}); + +describe("the tree's fingerprint", () => { + it("sees untracked files, same-second edits and deletions, but not ignored files", () => { + const dir = makeRepo(); + const start = snapshot(dir).fingerprint; + mkdirSync(join(dir, "ignored")); + writeFileSync(join(dir, "ignored/cache"), "x"); + expect(snapshot(dir).fingerprint).toBe(start); + + writeFileSync(join(dir, "new.js"), "x"); + expect(snapshot(dir).fingerprint).not.toBe(start); + rmSync(join(dir, "new.js")); + writeFileSync(join(dir, "app.js"), "ko\n"); // same size, same second + expect(snapshot(dir).fingerprint).not.toBe(start); + rmSync(join(dir, "app.js")); + expect(snapshot(dir).fingerprint).not.toBe(start); + writeFileSync(join(dir, "app.js"), "ok\n"); // an undone edit is the same tree + expect(snapshot(dir).fingerprint).toBe(start); + chmodSync(join(dir, "app.js"), 0o755); + expect(snapshot(dir).fingerprint).not.toBe(start); + }); +}); + +describe("test counts", () => { + // Summary lines copied from real vitest 5 and cargo runs. + it("reads how many tests passed, and zero when none ran", () => { + expect(vitestPassed(" Tests 73 passed (73)")).toBe(73); + expect(vitestPassed(" Tests 4 failed | 6 passed (10)")).toBe(6); + expect(vitestPassed("No test files found, exiting with code 1")).toBe(0); + expect(cargoPassed("test result: ok. 11 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out")).toBe(11); + expect(cargoPassed("test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out")).toBe(0); + }); +}); From b070bc11098fe63fdf3ec1d1e44095642011ba8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Sun, 25 Oct 2026 13:27:17 -0300 Subject: [PATCH 3/4] Send Claude back once when a turn ends with unverified code Two Claude Code hooks in .claude/settings.json run scripts/guard.mjs. When a turn starts, it records a hash of every file. When Claude stops, it compares: if the turn changed code (not only docs) that pnpm verify hasn't passed, it sends Claude back once, naming the files. The second stop goes through, so Claude can stop on purpose and say what is not verified. Turns that changed nothing, changed only docs, or undid their edits pass. Any error lets the turn through with a message. The same file has ask rules for git push, gh pr merge, and --no-verify, so Claude Code asks before it pushes or merges, also in auto mode. - The guard relies on its own record for "once", not on stop_hook_active, which any Stop hook sets. - Records live in the shared git folder, one per session, with the checkout the turn started in. - scripts/guard.test.mjs runs copies of the guard in throwaway repositories. The helpers moved from verify.test.mjs to test-repo.mjs; no test changed. - Proven in headless Claude Code sessions: an unverified edit was sent back once, and push and merge were refused even when allowed. --- .claude/settings.json | 35 +++++++ CLAUDE.md | 3 + docs/agents.md | 56 ++++++++++- scripts/guard.mjs | 82 +++++++++++++++++ scripts/guard.test.mjs | 199 ++++++++++++++++++++++++++++++++++++++++ scripts/test-repo.mjs | 57 ++++++++++++ scripts/tree.mjs | 7 +- scripts/verify.test.mjs | 58 +----------- 8 files changed, 439 insertions(+), 58 deletions(-) create mode 100644 .claude/settings.json create mode 100644 scripts/guard.mjs create mode 100644 scripts/guard.test.mjs create mode 100644 scripts/test-repo.mjs diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..215c7b1 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,35 @@ +{ + "$schema": "https://json.schemastore.org/claude-code-settings.json", + "permissions": { + "ask": [ + "Bash(git push *)", + "Bash(gh pr merge *)", + "Bash(* --no-verify)", + "Bash(* --no-verify *)" + ] + }, + "hooks": { + "UserPromptSubmit": [ + { + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/scripts/guard.mjs\" turn-start", + "timeout": 10 + } + ] + } + ], + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/scripts/guard.mjs\" turn-end", + "timeout": 30 + } + ] + } + ] + } +} diff --git a/CLAUDE.md b/CLAUDE.md index 9123672..73b5b6a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,6 +30,9 @@ pnpm tauri build --bundles app # src-tauri/target/release/bundle/macos/Undert change depends on them, check it in `pnpm tauri dev`, or say it is not verified. - Never weaken a check to pass it: no skipped or deleted tests, no loosened assertions. If a check is wrong, stop and say why. +- At the end of a turn that changed code `pnpm verify` hasn't passed, a hook sends you back + once. Run it, or say what is not verified and stop. `git push` and `gh pr merge` ask the + owner first. - Report what ran, what it showed, and what you did not verify. How this works: `docs/agents.md`. diff --git a/docs/agents.md b/docs/agents.md index 99e47dc..b377e74 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -10,20 +10,70 @@ is proof; "I read the code" is not. 1. **`pnpm verify`** runs every check, in about 10 seconds, and records the tree when they all pass. -2. **CI** runs `pnpm verify` on GitHub for every push to `main` and every pull request. -3. **`CLAUDE.md`** tells every agent session what done means and to run `pnpm verify`. +2. **The end-of-turn gate** sends Claude back once when a turn changed code that `pnpm verify` + hasn't passed. +3. **Ask first:** Claude Code asks you before it pushes or merges. +4. **CI** runs `pnpm verify` on GitHub for every push to `main` and every pull request. +5. **`CLAUDE.md`** tells every agent session what done means and to run `pnpm verify`. ## Rules, and what holds them | Rule | What holds it | How sure | | --- | --- | --- | | Types, tests, web build, and Rust format, lint, and tests pass | `pnpm verify`, then CI | Checked. CI shows a red run, but nothing blocks a push to `main`. | -| Run `pnpm verify` before calling work done | `CLAUDE.md` | Asked | +| Run `pnpm verify` before calling work done | The end-of-turn gate, `CLAUDE.md` | Checked once per turn. Claude may stop the second time, and must say what is not verified. | +| Push or merge only when you say so | Claude Code `ask` rules | Checked for the usual commands. A speed bump, not a lock. | | Never weaken a check to pass it | `CLAUDE.md` | Asked | | Say what was not verified | `CLAUDE.md` | Asked | "Asked" means an agent can forget it. Later steps turn some of these into checks. +## The end-of-turn gate + +Two Claude Code hooks in `.claude/settings.json` run `scripts/guard.mjs`: + +- When a turn starts, it records a hash of every file, as `pnpm verify` does. +- When Claude stops, it compares the tree with that record. If the turn changed code (anything + but docs) and `pnpm verify` hasn't passed on the tree as it is now, it sends Claude back with + the files it changed and what to do. +- It does this **once per turn**. The second stop goes through, so Claude can stop on purpose. + It then has to say what is not verified. Another Stop hook, such as a loop plugin, doesn't turn + it off. +- Turns that changed nothing, changed only docs, or undid their edits go through. +- A turn that changed only CI files can't be verified on your Mac. The gate asks Claude to say so. + +**It fails open.** If the guard breaks, it lets the turn through. Usually it shows a message that +starts with "Undertext guard". If it crashes outright or takes too long, Claude Code shows its own +hook error instead. Either way, the turn was not checked. + +**Limits:** + +- It sees every change in the turn, also yours: an edit you make in another editor while Claude + works counts. +- It checks the checkout it lives in, or a linked worktree of it that the session is in. Edits in + another repository are not checked. A turn that moves to another checkout is not checked, and + the message says so. +- Two sessions in the same checkout see each other's edits. Each may send its Claude back once for + the other's work. +- Only Claude Code runs it. Codex, Amp, and Cursor are not set up here. + +Its records live in `.git/undertext/turns/`, one file per session, shared by every worktree. Files +older than a week are deleted. Its tests, in `scripts/guard.test.mjs`, run copies of the guard in throwaway +repositories. + +## Ask first + +`.claude/settings.json` has `ask` rules for `git push`, `gh pr merge`, and any command with +`--no-verify`. Claude Code then asks you, even in auto mode, and even inside a longer command such +as `cd x && git push`. + +- **It is a speed bump.** It matches the commands Claude usually writes. A different form of the + same thing, such as a script that pushes, gets through. +- `--no-verify` skips git hooks. This repository has none today, so that rule guards nothing yet. + It misses the short form, `git commit -n`. +- The real lock would be branch protection on GitHub. You push to `main` yourself, so there is + none. + ## `pnpm verify` It runs, in order, and prints one line per check: diff --git a/scripts/guard.mjs b/scripts/guard.mjs new file mode 100644 index 0000000..fa89da3 --- /dev/null +++ b/scripts/guard.mjs @@ -0,0 +1,82 @@ +// The end-of-turn gate for Claude Code, run by the hooks in .claude/settings.json with the hook's +// JSON on stdin. `turn-start` (UserPromptSubmit) records the tree. `turn-end` (Stop) sends Claude +// back once when the turn changed code that `pnpm verify` hasn't passed. On any error it lets the +// turn through and says why. +import { mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const own = realpathSync(resolve(dirname(fileURLToPath(import.meta.url)), "..")); +const WEEK = 7 * 24 * 60 * 60 * 1000; +const list = (paths) => (paths.length > 5 ? `${paths.slice(0, 5).join(", ")} and ${paths.length - 5} more` : paths.join(", ")); + +/** The checkout this script lives in, or a linked worktree of it that Claude is in. Never another repository. */ +function checkout(git, cwd) { + if (!cwd) return own; + try { + const top = realpathSync(git(cwd, "rev-parse", "--show-toplevel").trim()); + const common = (dir) => realpathSync(resolve(dir, git(dir, "rev-parse", "--git-common-dir").trim())); + return common(top) === common(own) ? top : own; + } catch { + return own; + } +} + +function turnStart({ git, snapshot }, input, file) { + rmSync(file, { force: true }); // if the snapshot fails, turn-end finds no record and says so + mkdirSync(dirname(file), { recursive: true }); + for (const name of readdirSync(dirname(file))) { + const path = join(dirname(file), name); + try { + if (Date.now() - statSync(path).mtimeMs > WEEK) rmSync(path, { force: true }); + } catch { + // another session removed it first + } + } + const root = checkout(git, input.cwd); + writeFileSync(file, JSON.stringify({ root, ...snapshot(root) })); + return null; +} + +function turnEnd({ changedPaths, git, isCiOnly, isDoc, readPass, snapshot }, input, file) { + let start; + try { + start = JSON.parse(readFileSync(file, "utf8")); + } catch { + return { systemMessage: "Undertext guard: no record of this turn's start, so this turn was not checked." }; + } + // Once per turn: the second stop goes through, so Claude can stop on purpose and say what's + // unfinished. Not `stop_hook_active`: any Stop hook sets that, and it would turn the gate off. + if (start.blocked) return null; + const root = checkout(git, input.cwd); + if (start.root && root !== start.root) { + return { systemMessage: `Undertext guard: this turn moved from ${start.root} to ${root}, so it was not checked.` }; + } + const now = snapshot(root); + const touched = changedPaths(start.files, now.files).filter((path) => !isDoc(path)); + if (touched.length === 0) return null; // nothing changed, only docs, or the edits were undone + const pass = readPass(root); + const unverified = pass ? changedPaths(pass.files, now.files).filter((path) => !isDoc(path)) : touched; + if (unverified.length === 0) return null; + + writeFileSync(file, JSON.stringify({ ...start, blocked: true })); + const reason = touched.every(isCiOnly) + ? `This turn changed CI files (${list(touched)}). Local checks can't see them. Say they are not verified until CI runs, then stop.` + : `This turn changed code that \`pnpm verify\` hasn't passed: ${list(touched)}. Run \`pnpm verify\` and fix what fails. If you are stopping on purpose, say what is not verified, then stop again.`; + return { decision: "block", reason, systemMessage: "Undertext guard: sent Claude back to verify this turn's changes." }; +} + +try { + // Imported here, so that a broken tree.mjs still ends in the message below. + const tree = await import("./tree.mjs"); + const input = JSON.parse(readFileSync(0, "utf8") || "{}"); + const event = process.argv[2]; + if (event !== "turn-start" && event !== "turn-end") throw new Error(`unknown event "${event}"`); + // One folder for every checkout of this repository, so a session's record is found wherever it ends. + const turns = join(realpathSync(resolve(own, tree.git(own, "rev-parse", "--git-common-dir").trim())), "undertext/turns"); + const file = join(turns, `${String(input.session_id ?? "unknown").replace(/[^\w-]/g, "_")}.json`); + const out = event === "turn-start" ? turnStart(tree, input, file) : turnEnd(tree, input, file); + if (out) process.stdout.write(JSON.stringify(out)); +} catch (error) { + process.stdout.write(JSON.stringify({ systemMessage: `Undertext guard failed, so it let this turn through: ${error.message}` })); +} diff --git a/scripts/guard.test.mjs b/scripts/guard.test.mjs new file mode 100644 index 0000000..661939d --- /dev/null +++ b/scripts/guard.test.mjs @@ -0,0 +1,199 @@ +// Runs copies of the guard in throwaway repositories (see test-repo.mjs), the way Claude Code's +// hooks do: the hook's JSON on stdin, from another folder. +import { spawnSync } from "node:child_process"; +import { copyFileSync, existsSync, mkdirSync, readFileSync, utimesSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { makeRepo, tempDir, verify } from "./test-repo.mjs"; +import { git } from "./tree.mjs"; + +function guard(dir, event, input = {}, env = {}) { + const result = spawnSync(process.execPath, [join(dir, "scripts/guard.mjs"), event], { + cwd: tmpdir(), + input: typeof input === "string" ? input : JSON.stringify({ session_id: "one", cwd: dir, ...input }), + env: { ...process.env, ...env }, + encoding: "utf8", + }); + expect(result.status).toBe(0); // a hook that errors would stop Claude for the wrong reason + return result.stdout ? JSON.parse(result.stdout) : null; +} + +const write = (dir, path, text) => writeFileSync(join(dir, path), text); + +describe("the end-of-turn gate", () => { + it("lets a turn through that changed nothing, only docs, or undid its edits", () => { + const dir = makeRepo(); + guard(dir, "turn-start"); + expect(guard(dir, "turn-end")).toBeNull(); + + guard(dir, "turn-start"); + write(dir, "README.md", "# App, documented\n"); + expect(guard(dir, "turn-end")).toBeNull(); + + guard(dir, "turn-start"); + write(dir, "app.js", "changed\n"); + write(dir, "app.js", "ok\n"); + expect(guard(dir, "turn-end")).toBeNull(); + }); + + it("sends Claude back when a turn changed code without a pass, naming the files", () => { + const dir = makeRepo(); + guard(dir, "turn-start"); + write(dir, "app.js", "changed\n"); + write(dir, "new.js", "untracked\n"); + const out = guard(dir, "turn-end"); + expect(out.decision).toBe("block"); + expect(out.reason).toContain("app.js, new.js"); + expect(out.reason).toContain("Run `pnpm verify`"); + expect(out.systemMessage).toContain("sent Claude back"); + }); + + it("sends Claude back only once per turn, even when another Stop hook continued it", () => { + const dir = makeRepo(); + guard(dir, "turn-start"); + write(dir, "app.js", "changed\n"); + expect(guard(dir, "turn-end", { stop_hook_active: true }).decision).toBe("block"); + expect(guard(dir, "turn-end", { stop_hook_active: true })).toBeNull(); + + guard(dir, "turn-start"); + write(dir, "app.js", "changed again\n"); + expect(guard(dir, "turn-end").decision).toBe("block"); // a new turn starts fresh + }); + + it("lets the turn through once pnpm verify passed, also after later docs edits", () => { + const dir = makeRepo(); + guard(dir, "turn-start"); + write(dir, "app.js", "changed\n"); + expect(verify(dir).status).toBe(0); + write(dir, "README.md", "# App, documented\n"); + expect(guard(dir, "turn-end")).toBeNull(); + }); + + it("sends Claude back when code changed after the pass, even in the same second", () => { + const dir = makeRepo(); + guard(dir, "turn-start"); + write(dir, "app.js", "v1\n"); + expect(verify(dir).status).toBe(0); + write(dir, "app.js", "v2\n"); + expect(guard(dir, "turn-end").decision).toBe("block"); + }); + + it("asks Claude to say CI files are not verified, since local checks can't see them", () => { + const dir = makeRepo(); + expect(verify(dir).status).toBe(0); + guard(dir, "turn-start"); + mkdirSync(join(dir, ".github")); + write(dir, ".github/ci.yml", "on: push\n"); + const out = guard(dir, "turn-end"); + expect(out.decision).toBe("block"); + expect(out.reason).toContain("CI files (.github/ci.yml)"); + + // Code an earlier turn left unverified doesn't change what this turn is told. + write(dir, "app.js", "changed earlier\n"); + guard(dir, "turn-start"); + write(dir, ".github/ci.yml", "on: pull_request\n"); + expect(guard(dir, "turn-end").reason).toContain("CI files (.github/ci.yml)"); + }); + + it("keeps sessions apart", () => { + const dir = makeRepo(); + guard(dir, "turn-start", { session_id: "one" }); + write(dir, "app.js", "changed\n"); + guard(dir, "turn-start", { session_id: "two" }); + expect(guard(dir, "turn-end", { session_id: "two" })).toBeNull(); + expect(guard(dir, "turn-end", { session_id: "one" }).decision).toBe("block"); + }); + + it("lets a turn through, and says so, when its start wasn't recorded", () => { + const dir = makeRepo(); + write(dir, "app.js", "changed\n"); + const out = guard(dir, "turn-end"); + expect(out.decision).toBeUndefined(); + expect(out.systemMessage).toContain("not checked"); + }); + + it("says the turn was not checked when its start failed, instead of using the last turn's record", () => { + const dir = makeRepo(); + guard(dir, "turn-start"); + write(dir, "app.js", "changed\n"); + expect(guard(dir, "turn-end").decision).toBe("block"); + expect(guard(dir, "turn-end")).toBeNull(); + + const index = readFileSync(join(dir, ".git/index")); + writeFileSync(join(dir, ".git/index"), "not an index"); // git can't list files + expect(guard(dir, "turn-start").systemMessage).toContain("guard failed"); + writeFileSync(join(dir, ".git/index"), index); + write(dir, "app.js", "changed again\n"); + expect(guard(dir, "turn-end").systemMessage).toContain("not checked"); + }); + + it("deletes turn records older than a week", () => { + const dir = makeRepo(); + const turns = join(dir, ".git/undertext/turns"); + mkdirSync(turns, { recursive: true }); + write(dir, ".git/undertext/turns/old.json", "{}"); + write(dir, ".git/undertext/turns/recent.json", "{}"); + const eightDaysAgo = new Date(Date.now() - 8 * 24 * 60 * 60 * 1000); + utimesSync(join(turns, "old.json"), eightDaysAgo, eightDaysAgo); + guard(dir, "turn-start"); + expect(existsSync(join(turns, "old.json"))).toBe(false); + expect(existsSync(join(turns, "recent.json"))).toBe(true); + }); + + it("fails open, and says so, when it can't read its input or the tree", () => { + const dir = makeRepo(); + expect(guard(dir, "turn-end", "not json").systemMessage).toContain("guard failed, so it let this turn through"); + expect(guard(dir, "turn-ended").systemMessage).toContain('unknown event "turn-ended"'); + + const notGit = tempDir(); + mkdirSync(join(notGit, "scripts")); + for (const file of ["guard.mjs", "tree.mjs"]) copyFileSync(join(dir, "scripts", file), join(notGit, "scripts", file)); + expect(guard(notGit, "turn-start", { cwd: notGit }).systemMessage).toContain("guard failed"); + + write(dir, "scripts/tree.mjs", "export const broken = ;\n"); + expect(guard(dir, "turn-end").systemMessage).toContain("guard failed"); + }); + + it("checks its own checkout, not another repository Claude is in", () => { + const dir = makeRepo(); + const other = makeRepo(); + guard(dir, "turn-start", { cwd: other }); + write(dir, "app.js", "changed\n"); + expect(guard(dir, "turn-end", { cwd: other }).decision).toBe("block"); + }); + + it("follows Claude into a linked worktree of its own checkout", () => { + const dir = makeRepo(); + const tree = join(tempDir(), "linked"); + git(dir, "worktree", "add", "-q", tree); + guard(dir, "turn-start", { cwd: tree }); + write(tree, "app.js", "changed in the worktree\n"); + expect(guard(dir, "turn-end", { cwd: tree }).decision).toBe("block"); + + guard(dir, "turn-start", { cwd: tree }); + write(tree, "app.js", "changed again\n"); + expect(verify(tree).status).toBe(0); + expect(guard(dir, "turn-end", { cwd: tree })).toBeNull(); + }); + + it("says the turn was not checked when it moved to another checkout", () => { + const dir = makeRepo(); + const tree = join(tempDir(), "linked"); + git(dir, "worktree", "add", "-q", tree); + guard(dir, "turn-start", { cwd: dir }); + write(tree, "app.js", "changed in the worktree\n"); + const out = guard(dir, "turn-end", { cwd: tree }); + expect(out.decision).toBeUndefined(); + expect(out.systemMessage).toContain("this turn moved"); + }); + + it("acts on its own checkout when git's hook variables point elsewhere", () => { + const dir = makeRepo(); + const other = makeRepo(); + const env = { GIT_DIR: join(other, ".git"), GIT_WORK_TREE: other, GIT_INDEX_FILE: join(other, ".git/index") }; + guard(dir, "turn-start", {}, env); + write(dir, "app.js", "changed\n"); + expect(guard(dir, "turn-end", {}, env).decision).toBe("block"); + }); +}); diff --git a/scripts/test-repo.mjs b/scripts/test-repo.mjs new file mode 100644 index 0000000..aa40c7c --- /dev/null +++ b/scripts/test-repo.mjs @@ -0,0 +1,57 @@ +// Throwaway repositories for the tooling tests. Each holds copies of the scripts, never this +// checkout. A fake check stands in for the real ones: it counts its runs, fails while a `broken` +// file exists, and writes a file into the tree while a `touch` file exists. +import { spawnSync } from "node:child_process"; +import { copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterEach } from "vitest"; +import { git } from "./tree.mjs"; + +const here = dirname(fileURLToPath(import.meta.url)); +const made = []; +afterEach(() => made.splice(0).forEach((dir) => rmSync(dir, { recursive: true, force: true }))); + +export function tempDir() { + const dir = mkdtempSync(join(tmpdir(), "undertext-tooling-test-")); + made.push(dir); + return dir; +} + +const FAKE_CHECK = `const fs = require("fs"); +fs.appendFileSync(process.env.RUNS, "x"); +if (fs.existsSync("broken")) process.exit(1); +if (fs.existsSync("touch")) fs.writeFileSync("made-during-run.txt", "x");`; + +export const fakeSteps = (step = `{ name: "check", cmd: process.execPath, args: ["-e", ${JSON.stringify(FAKE_CHECK)}] }`) => + `export const unseen = "nothing";\nexport const steps = () => [${step}];\n`; + +export function makeRepo(steps = fakeSteps()) { + const dir = tempDir(); + mkdirSync(join(dir, "scripts")); + for (const file of ["verify.mjs", "tree.mjs", "guard.mjs"]) copyFileSync(join(here, file), join(dir, "scripts", file)); + writeFileSync(join(dir, "scripts/verify-steps.mjs"), steps); + writeFileSync(join(dir, "app.js"), "ok\n"); + writeFileSync(join(dir, "README.md"), "# App\n"); + writeFileSync(join(dir, ".gitignore"), "ignored/\n"); + git(dir, "init", "-q"); + git(dir, "add", "."); + git(dir, "-c", "commit.gpgsign=false", "-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "-qm", "Start"); + return dir; +} + +/** Runs the repository's copy of `pnpm verify` from another folder. `runs` counts the fake check's runs. */ +export function verify(dir, args = [], env = {}) { + const runs = join(tempDir(), "runs"); + writeFileSync(runs, ""); + const result = spawnSync(process.execPath, [join(dir, "scripts/verify.mjs"), ...args], { + cwd: tmpdir(), + env: { ...process.env, RUNS: runs, ...env }, + encoding: "utf8", + }); + return { status: result.status, out: result.stdout + result.stderr, runs: readFileSync(runs, "utf8").length }; +} + +export const passFile = (dir) => join(dir, ".git/undertext/verified.json"); +export const recorded = (dir) => (existsSync(passFile(dir)) ? JSON.parse(readFileSync(passFile(dir), "utf8")).fingerprint : null); diff --git a/scripts/tree.mjs b/scripts/tree.mjs index 1f94107..155dde9 100644 --- a/scripts/tree.mjs +++ b/scripts/tree.mjs @@ -17,7 +17,12 @@ function gitEnv() { } export const git = (root, ...args) => - execFileSync("git", ["-C", root, ...args], { env: gitEnv(), encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); + execFileSync("git", ["-C", root, ...args], { + env: gitEnv(), + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + maxBuffer: 256 * 1024 * 1024, + }); /** * Every file git sees in `root`: tracked, staged and untracked, but not ignored. Hashes contents, so diff --git a/scripts/verify.test.mjs b/scripts/verify.test.mjs index b0c2487..4f4b224 100644 --- a/scripts/verify.test.mjs +++ b/scripts/verify.test.mjs @@ -1,61 +1,11 @@ -// Runs copies of the verify scripts in throwaway repositories, never in this checkout. A fake check -// stands in for the real ones: it counts its runs, fails while a `broken` file exists, and writes a -// file into the tree while a `touch` file exists. -import { spawnSync } from "node:child_process"; -import { chmodSync, copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +// Runs copies of the verify scripts in throwaway repositories (see test-repo.mjs). +import { chmodSync, existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; import { basename, dirname, join } from "node:path"; -import { fileURLToPath } from "node:url"; -import { afterEach, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; +import { fakeSteps, makeRepo, passFile, recorded, tempDir, verify } from "./test-repo.mjs"; import { git, snapshot } from "./tree.mjs"; import { cargoPassed, vitestPassed } from "./verify-steps.mjs"; -const here = dirname(fileURLToPath(import.meta.url)); -const made = []; -afterEach(() => made.splice(0).forEach((dir) => rmSync(dir, { recursive: true, force: true }))); - -const tempDir = () => { - const dir = mkdtempSync(join(tmpdir(), "undertext-verify-test-")); - made.push(dir); - return dir; -}; - -const FAKE_CHECK = `const fs = require("fs"); -fs.appendFileSync(process.env.RUNS, "x"); -if (fs.existsSync("broken")) process.exit(1); -if (fs.existsSync("touch")) fs.writeFileSync("made-during-run.txt", "x");`; - -const fakeSteps = (step = `{ name: "check", cmd: process.execPath, args: ["-e", ${JSON.stringify(FAKE_CHECK)}] }`) => - `export const unseen = "nothing";\nexport const steps = () => [${step}];\n`; - -function makeRepo(steps = fakeSteps()) { - const dir = tempDir(); - mkdirSync(join(dir, "scripts")); - for (const file of ["verify.mjs", "tree.mjs"]) copyFileSync(join(here, file), join(dir, "scripts", file)); - writeFileSync(join(dir, "scripts/verify-steps.mjs"), steps); - writeFileSync(join(dir, "app.js"), "ok\n"); - writeFileSync(join(dir, "README.md"), "# App\n"); - writeFileSync(join(dir, ".gitignore"), "ignored/\n"); - git(dir, "init", "-q"); - git(dir, "add", "."); - git(dir, "-c", "commit.gpgsign=false", "-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "-qm", "Start"); - return dir; -} - -function verify(dir, args = [], env = {}) { - const runs = join(tempDir(), "runs"); - writeFileSync(runs, ""); - const result = spawnSync(process.execPath, [join(dir, "scripts/verify.mjs"), ...args], { - cwd: tmpdir(), - env: { ...process.env, RUNS: runs, ...env }, - encoding: "utf8", - }); - return { status: result.status, out: result.stdout + result.stderr, runs: readFileSync(runs, "utf8").length }; -} - -const passFile = (dir) => join(dir, ".git/undertext/verified.json"); -const recorded = (dir) => (existsSync(passFile(dir)) ? JSON.parse(readFileSync(passFile(dir), "utf8")).fingerprint : null); - describe("pnpm verify", () => { it("records a pass, then skips an unchanged tree unless forced", () => { const dir = makeRepo(); From f1690065bedb518234adf0383780af92f2254f7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Sun, 25 Oct 2026 13:43:12 -0300 Subject: [PATCH 4/4] Add skills for verifying, fixing, building, and refactoring Five skills in .claude/skills/ give each kind of work its first step and its proof: verify, fix-bug, build-feature, refactor, and learn. CLAUDE.md tells Claude to use them without being asked, and its new Tests section holds the test rules. A fresh reviewer applied those rules to all 98 existing tests; the wording now accepts the good cases it flagged at first. - scripts/docs.test.mjs checks that the instructions name and link only files that exist, with their exact case, name only pnpm commands that exist, and that each skill says when to use it. - CLAUDE.md, README.md, docs/agents.md, and .claude/ now count as code, not docs, so pnpm verify and the gate run that test when they change. Three tooling tests now use another file as their docs example. - docs/agents.md says what each skill is for, and what to say when asking for a bug fix, a feature, or a refactor. - .claude/launch.json, the browser preview config, is committed. - Each skill loaded on its own in headless Claude Code sessions, from plain requests such as "Fix it" or "Add a shortcut". --- .claude/launch.json | 11 +++++ .claude/skills/build-feature/SKILL.md | 34 +++++++++++++ .claude/skills/fix-bug/SKILL.md | 29 +++++++++++ .claude/skills/learn/SKILL.md | 22 +++++++++ .claude/skills/refactor/SKILL.md | 25 ++++++++++ .claude/skills/verify/SKILL.md | 62 ++++++++++++++++++++++++ CLAUDE.md | 42 ++++++++++++++-- docs/agents.md | 51 +++++++++++++++++-- scripts/docs.test.mjs | 70 +++++++++++++++++++++++++++ scripts/guard.test.mjs | 4 +- scripts/tree.mjs | 10 +++- scripts/verify.test.mjs | 8 ++- 12 files changed, 354 insertions(+), 14 deletions(-) create mode 100644 .claude/launch.json create mode 100644 .claude/skills/build-feature/SKILL.md create mode 100644 .claude/skills/fix-bug/SKILL.md create mode 100644 .claude/skills/learn/SKILL.md create mode 100644 .claude/skills/refactor/SKILL.md create mode 100644 .claude/skills/verify/SKILL.md create mode 100644 scripts/docs.test.mjs diff --git a/.claude/launch.json b/.claude/launch.json new file mode 100644 index 0000000..a699d6e --- /dev/null +++ b/.claude/launch.json @@ -0,0 +1,11 @@ +{ + "version": "0.0.1", + "configurations": [ + { + "name": "web", + "runtimeExecutable": "pnpm", + "runtimeArgs": ["dev"], + "port": 1420 + } + ] +} diff --git a/.claude/skills/build-feature/SKILL.md b/.claude/skills/build-feature/SKILL.md new file mode 100644 index 0000000..8295f38 --- /dev/null +++ b/.claude/skills/build-feature/SKILL.md @@ -0,0 +1,34 @@ +--- +name: build-feature +description: Builds a new feature that fits the editor's existing shape, with a test for each behavior. Use when asked to "add", "implement", "support", "build a feature", or "make it possible to" do something new. +--- + +# Build a feature + +1. **Read the closest feature** and follow its shape. For an editor command, that is its function + in `src/editor/commands.ts` or `src/editor/keys.ts`, its key in `src/editor/setup.ts`, its entry + in `src/palette/commands.ts`, and its tests. +2. **Settle the decisions in one round.** List the choices only the owner can make (behavior, + shortcut, names, file format), each with your recommendation, and ask them all at once. Done + when each one has an answer. +3. **Write done as behaviors** a test or a look at the app can check. For example: "⌘⇧D on a + paragraph with versions adds one more, and undo removes it." +4. **Build in thin slices, test first,** at the public interface: a command that returns a + `TransactionSpec`, tested on `EditorState`. For each slice: write the test, see it fail, make it + pass, and run `pnpm verify`. +5. **Update what the change touches,** from the table below. +6. Report with the `verify` skill. + +| When you touch | Also update | +| --- | --- | +| A shortcut or command | The keymap in `src/editor/setup.ts`, or the keys in `src/App.tsx`; `src/palette/commands.ts`; the Shortcuts table in `README.md`; the context menu (`buildMenu` in `src/App.tsx`) if it belongs there | +| The file format | `src/editor/format.ts` and its round-trip tests; the File format section of `README.md`. Older files must still open. | +| Something a new user should learn | The welcome document, `src/welcome.ts` | +| A Rust command | `generate_handler!` in `src-tauri/src/lib.rs`; the `invoke` calls that use it, in `src/lib/files.ts`, `src/lib/links.ts`, or `src/ai/client.ts`; `src-tauri/capabilities/default.json` if it needs a permission; a Rust test | +| A new kind of layer | `src/editor/layers.ts`; `src/editor/format.ts` and the File format section of `README.md`; `src/editor/clipboard.ts`; `src/editor/ink.ts`; undo and redo tests | +| An AI task | `src/ai/tasks.ts`; a `mock` result for the browser preview; the AI section of `README.md` | +| How the editor is built | "How the editor works" in `CLAUDE.md` | +| A decision with reasons | "Decisions taken" in `docs/codemirror-plan.md` | + +If the code you build on needs restructuring, do that first, as its own change, with the +`refactor` skill. diff --git a/.claude/skills/fix-bug/SKILL.md b/.claude/skills/fix-bug/SKILL.md new file mode 100644 index 0000000..8b2c233 --- /dev/null +++ b/.claude/skills/fix-bug/SKILL.md @@ -0,0 +1,29 @@ +--- +name: fix-bug +description: Fixes a bug so it stays fixed, with a regression test that proves the fix is needed. Use when something is "broken", "wrong", "a bug", "a regression", "doesn't work", or "used to work". +--- + +# Fix a bug + +1. **Restate the symptom** in one line: what you did, what happened, what should happen. If the + report is vague, write your restatement and ask before you start. +2. **Reproduce it at the lowest layer that shows it.** A test on `EditorState` in + `src/editor/*.test.ts` beats one through React. A test in the Rust file's `mod tests` beats the + app. Run it: it must fail, for the reason in the report. Done when you have that failing test. + - Example, for "Enter in a list item leaves a stray space", in `src/editor/keys.test.ts`. This + bug is real and still open: + `expect(run(stateFor("1. one‸ two"), enter).text).toBe("1. one\n2. ‸two")` + - Some bugs show only in WebKit: caret, IME, dead keys. Reproduce those in `pnpm tauri dev` and + write the steps down. Still add a test for any part the editor state can show. +3. **Find the root cause.** Say why it happens and cite the line. Don't patch the symptom in a + caller. +4. **Make the smallest fix.** No restructuring in the same change. Note it for the `refactor` + skill instead. +5. **Prove the fix is needed:** undo it and see the test fail, then restore it and see it pass. +6. Run `pnpm verify`, then report with the `verify` skill. + +The new test stays as the regression test. Never change an existing test's expected value to make +it pass, unless the owner says the old behavior was wrong. + +After three fixes that didn't work, stop. Say what you tried and what you learned, then rethink the +cause before you try again. diff --git a/.claude/skills/learn/SKILL.md b/.claude/skills/learn/SKILL.md new file mode 100644 index 0000000..d59cce4 --- /dev/null +++ b/.claude/skills/learn/SKILL.md @@ -0,0 +1,22 @@ +--- +name: learn +description: Turns a correction or a repeated mistake into a check that catches it next time. Use when the owner says "don't do that again", "you keep doing this", "remember this", "this happened before", or corrects the same thing twice. +--- + +# Learn from a mistake + +1. **Name the mistake** in one line, with the example that showed it. +2. **Pick the strongest structure that holds it,** in this order: + 1. a type, in TypeScript or Rust + 2. a test, in `src/`, `src-tauri/`, or `scripts/` + 3. a script check: a step in `scripts/verify-steps.mjs`, or the gate in `scripts/guard.mjs` + 4. a line in `CLAUDE.md` or a skill, only for what no code can check + + A rule that is already written down and still broken goes straight to code. +3. **Prove it catches the mistake:** make the mistake on purpose and see the check fail. +4. **Delete the instructions it replaces** in `CLAUDE.md`, the skills, and `docs/`. +5. **Update the rules table** in `docs/agents.md`: the rule, what holds it, and how sure. +6. Make it its own small change. Commit it when the owner asks. + +Claude's memory notes hold the owner's preferences, such as how they like commits. They are not +the place for a rule a check can hold. diff --git a/.claude/skills/refactor/SKILL.md b/.claude/skills/refactor/SKILL.md new file mode 100644 index 0000000..4deda55 --- /dev/null +++ b/.claude/skills/refactor/SKILL.md @@ -0,0 +1,25 @@ +--- +name: refactor +description: Changes the structure of code without changing what it does, bugs included. Use when asked to "refactor", "clean up", "restructure", "move", "rename", "split", "extract", or "tidy" code. +--- + +# Refactor + +The structure changes. The behavior doesn't, bugs included. A bug you find goes in a note, for a +separate fix with the `fix-bug` skill. + +1. **Pin today's behavior** at the public interface, with tests that pass now. For UI, take + "before" screenshots in the browser preview. Done when the pins cover everything you will move. +2. **Delete proven-dead code** first: nothing calls it, and `pnpm verify` passes without it. Its + own tests go with it; name them in the report. +3. **Move one concept at a time,** in small steps. Run `pnpm verify` after each step. The pins stay + green. +4. **Migrate every caller,** then delete the old API. No shims left behind. +5. **Compare** "after" screenshots with "before". +6. **Keep it only if you can name what got easier to read.** If you can't, revert it. + +Existing tests may change how they reach the code: paths, names, call sites. Never their inputs or +expected values. When the editor moved to CodeMirror, 1 of the 8 list tests was ported, and a fix +for stray spaces broke without any test noticing. Port every case. + +Report with the `verify` skill. Name the test files whose expected values you left unchanged. diff --git a/.claude/skills/verify/SKILL.md b/.claude/skills/verify/SKILL.md new file mode 100644 index 0000000..3103840 --- /dev/null +++ b/.claude/skills/verify/SKILL.md @@ -0,0 +1,62 @@ +--- +name: verify +description: Proves a change works, and reports what was and wasn't verified. Use when finishing any change, or when someone asks "does it work?", "did you test it?", "is it done?", or "check it". +--- + +# Verify a change + +1. Run `pnpm verify` and fix every failure. Done when it exits 0: it prints "Passed", or says + nothing but docs changed since its last pass. +2. Get the proof the table asks for. +3. Check that the proof can fail: undo the change or break it on purpose, see the test or the app + show it, then restore it. +4. Write the report below. + +| Kind of change | Proof | +| --- | --- | +| Editor command, key, layer, or Lab | A test on `EditorState` in `src/editor/*.test.ts` that fails without the change | +| File format (`src/editor/format.ts`) | A round-trip test in `src/editor/format.test.ts`, and an older file that still opens the same | +| Live preview or ink (`src/editor/preview.ts`, `src/editor/ink.ts`) | The browser preview, with the cursor on the text and away from it, in light and dark | +| React panel, palette, menu, or dialog | The browser preview: a screenshot, no console errors, at 640 px wide and full size | +| Caret, IME, dead keys, spellcheck, or drag | The desktop app. WebKit behaves differently from the browser preview. | +| Rust command (`src-tauri/src/`) | A test in that file's `mod tests`, then the desktop app | +| AI request or prompt | A test on the parsing in `src/ai/tasks.ts`. The real call only works in the app, with a key or a subscription. | +| Tooling in `scripts/` | A test in `scripts/*.test.mjs`. Tests that run git or write files use a throwaway repository from `scripts/test-repo.mjs`. | +| Instructions: `CLAUDE.md`, `README.md`, `docs/agents.md`, skills | `pnpm verify` runs `scripts/docs.test.mjs` on them. Also read what the words claim against the code. | +| Other docs | Nothing to run. | + +## The browser preview + +- Start the `web` entry in `.claude/launch.json` (`pnpm dev`, port 1420). +- It has no file access. AI calls return the `mock` result passed to `callClaude` in + `src/ai/client.ts`. There are no accounts to sign in to. +- Check: a screenshot, the console for errors, the window at 640 × 480 (the app's minimum) and at + full size, and both light and dark (the app follows the system theme). The UI is English only. + +## The desktop app + +- `pnpm tauri dev`. It needs `cargo` on `PATH` (see `CLAUDE.md`). +- Use it, with computer use if you can, for WebKit behavior, file dialogs, the keychain, and real + AI calls. If you can't run it, say so under "Not verified". + +## The report + +Rate each claim on this ladder: + +1. **said:** you believe it +2. **cited:** you quote the line that shows it +3. **traced:** you followed the code path +4. **ran:** a test or script ran the code +5. **seen:** you saw it in the app + +- "Fixed", "works", and "safe" need ran or seen. A fact visible in the code needs cited. +- A claim below the rung it needs goes under "Not verified", with the reason. +- When the change touches code others rely on (the file format, layers, Rust commands, or + `scripts/`), name the one fact that makes it safe, and how you proved it. + +``` +Ran: pnpm verify (passed, recorded ); +Showed: +Safe because: () +Not verified: , because +``` diff --git a/CLAUDE.md b/CLAUDE.md index 73b5b6a..ee7d9cc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,14 +28,24 @@ pnpm tauri build --bundles app # src-tauri/target/release/bundle/macos/Undert when every check passes on it, or when only docs changed since its last pass. - It can't see the app window (WebKit), real AI calls, file dialogs, or the keychain. When a change depends on them, check it in `pnpm tauri dev`, or say it is not verified. -- Never weaken a check to pass it: no skipped or deleted tests, no loosened assertions. If a - check is wrong, stop and say why. +- Never weaken a check to pass it: no test skipped or deleted to get a pass, no loosened + assertions. If a check is wrong, stop and say why. - At the end of a turn that changed code `pnpm verify` hasn't passed, a hook sends you back once. Run it, or say what is not verified and stop. `git push` and `gh pr merge` ask the owner first. - Report what ran, what it showed, and what you did not verify. How this works: `docs/agents.md`. +## Skills + +Use these without being asked. They are in `.claude/skills/`. + +- `verify`: before you call any change done. +- `fix-bug`: anything broken, wrong, or a regression. +- `build-feature`: anything new. +- `refactor`: structure only, always as its own change. +- `learn`: after the owner corrects you, or when a mistake repeats. + ## How the editor works - The CodeMirror document is the page as plain Markdown. Undertext's own syntax is not in it. @@ -52,9 +62,33 @@ pnpm tauri build --bundles app # src-tauri/target/release/bundle/macos/Undert - `src-tauri/src/lib.rs`: file reads and writes (a save refuses to overwrite a change from another app unless told to), and `open_url` for web and mail links. +## Tests + +- Editor tests sit beside the code, in `src/editor/*.test.ts`, and run on `EditorState` without a + DOM. `src/editor/keys.test.ts` shows the `stateFor` and `run` helpers, with `‸` for the caret and + `« »` for a selection. Rust tests live in each file's `mod tests`. Tooling tests are + `scripts/*.test.mjs`; the ones that run git use `scripts/test-repo.mjs`. +- Each editor command gets a test. A command that changes layers also gets one that undoes and + redoes it and checks the saved Markdown. +- Never take an expected value from the code under test: write the text, positions, and messages + out, and above all any value another part relies on, such as an error code the TypeScript side + matches. A hash or id may come from the code when another test pins that code. Then assert equal + or not equal. +- Assert outcomes: the text, the saved Markdown, the layers, the file on disk. Check after each + step, not only at the end. +- Never fake code in `src/` that decides the result. Stubs the test never calls are fine. The + network is the one boundary: give the answer straight to the function that reads it, as the + `applyFixes` tests do. +- See each new test fail once, by breaking the code or undoing the fix. Say how it failed in your + report. +- `.only` fails `pnpm verify`. Skip a test only when it needs the network or an account, with a + comment saying how to run it, like the `#[ignore]` test in `src-tauri/src/claude_code.rs`. +- `expect(x).not.toBeNull()` alone proves little. It's fine as a guard before using `x`, or when + null is the failure you rule out and a nearby case pins the null, as for `matchScore`. +- Change a test's expected value only when the owner asked for the new behavior. A refactor may + change how a test reaches the code, never its inputs or expected values. + ## Conventions -- Tests run on `EditorState`, without a DOM. Add one for each command, with undo when it touches - layers. - Commands return a `TransactionSpec`, or null when they don't apply. - Docs and UI text: plain words, short sentences, active voice. diff --git a/docs/agents.md b/docs/agents.md index b377e74..5dc5d0d 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -13,8 +13,11 @@ is proof; "I read the code" is not. 2. **The end-of-turn gate** sends Claude back once when a turn changed code that `pnpm verify` hasn't passed. 3. **Ask first:** Claude Code asks you before it pushes or merges. -4. **CI** runs `pnpm verify` on GitHub for every push to `main` and every pull request. -5. **`CLAUDE.md`** tells every agent session what done means and to run `pnpm verify`. +4. **Skills** give each kind of work its first step and its proof: verify, fix a bug, build a + feature, refactor, learn from a mistake. +5. **CI** runs `pnpm verify` on GitHub for every push to `main` and every pull request. +6. **`CLAUDE.md`** tells every agent session what done means, which skill to use, and how tests + are written. ## Rules, and what holds them @@ -23,6 +26,9 @@ is proof; "I read the code" is not. | Types, tests, web build, and Rust format, lint, and tests pass | `pnpm verify`, then CI | Checked. CI shows a red run, but nothing blocks a push to `main`. | | Run `pnpm verify` before calling work done | The end-of-turn gate, `CLAUDE.md` | Checked once per turn. Claude may stop the second time, and must say what is not verified. | | Push or merge only when you say so | Claude Code `ask` rules | Checked for the usual commands. A speed bump, not a lock. | +| Fix a bug with a failing test first; refactor without changing assertions | The `fix-bug` and `refactor` skills | Asked | +| Expected values written by hand; each new test seen failing once | `CLAUDE.md` | Asked | +| Instructions name only files and `pnpm` commands that exist, and each skill says when to use it | `scripts/docs.test.mjs`, in `pnpm verify` | Checked | | Never weaken a check to pass it | `CLAUDE.md` | Asked | | Say what was not verified | `CLAUDE.md` | Asked | @@ -74,6 +80,41 @@ as `cd x && git push`. - The real lock would be branch protection on GitHub. You push to `main` yourself, so there is none. +## Skills + +The skills are in `.claude/skills/`. Claude Code reads each one's description and loads the full +text only when a task matches, so they cost little until used. `CLAUDE.md` tells Claude to use +them without being asked. + +| Skill | For | Its first step | Its proof | +| --- | --- | --- | --- | +| `verify` | Finishing any change | `pnpm verify` | The proof for that kind of change, from its table, and a report that says what was not verified | +| `fix-bug` | Anything broken | A test that fails on the exact symptom | Undo the fix: the test fails. Restore it: it passes. | +| `build-feature` | Anything new | Read the closest feature; settle your decisions in one round | One test per behavior, written first | +| `refactor` | Structure only | Tests that pin today's behavior | The same tests pass, with their expected values untouched | +| `learn` | A correction, or a mistake that repeats | Pick the strongest structure: type, test, script check, then a line of text | The check fails when the mistake is made on purpose | + +## Asking for work + +| To start | Say | Your part | +| --- | --- | --- | +| A bug fix | What you did, what happened, what should happen | Confirm Claude's restatement if the report was vague | +| A feature | What it should do, and how you'd check it works | Answer the decisions Claude lists, all at once | +| A refactor | What should get easier, and that behavior must not change | Read the list of tests whose expected values stayed the same | + +Any request works better with: + +- **The goal**, and a done check that can pass or fail: "pasting a link over a selection makes a + link, and undo restores the text". +- **The proof you want**: a test, a screenshot, a run in the app. +- **What you already know**, and real constraints: "only in the desktop app", "don't touch the + file format". + +Leave out the method and your theory of the cause, unless you are sure. They anchor Claude on +them. Start a new session for unrelated work, so it starts with a clean context. + +Then read the report's "Not verified" part. Try in the app what Claude couldn't. + ## `pnpm verify` It runs, in order, and prints one line per check: @@ -97,8 +138,10 @@ It runs, in order, and prints one line per check: `.git/undertext/verified.json`. Each worktree keeps its own. A failed run deletes the record. - **Nothing changed** since the last pass: it says so and stops. `pnpm verify --force` runs the checks anyway, for example after `rustup update`. -- **Only docs changed** since the last pass (`docs/`, `LICENSE`, and `*.md` outside `src/` and - `src-tauri/`): nothing to check. It records the new tree. +- **Only docs changed** since the last pass: nothing to check. It records the new tree. Docs are + `docs/`, `LICENSE`, and Markdown outside `src/` and `src-tauri/`, but not the instructions: + `CLAUDE.md`, `README.md`, `docs/agents.md`, and everything in `.claude/`. The docs test reads + those, so a change to them runs every check. - **Only CI files changed** (`.github/`): local checks can't see them. It records nothing, even with `--force`. Push and read the CI run. - It finds `cargo` on `PATH`, in `~/.cargo/bin`, or in Homebrew's rustup folder. Without cargo it diff --git a/scripts/docs.test.mjs b/scripts/docs.test.mjs new file mode 100644 index 0000000..ad76894 --- /dev/null +++ b/scripts/docs.test.mjs @@ -0,0 +1,70 @@ +// The instructions name only files and commands that exist, and every skill says when to use it. +// Reads this checkout's files; writes nothing. +import { readFileSync } from "node:fs"; +import { basename, dirname, join, normalize, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; +import { git, isInstructions } from "./tree.mjs"; + +const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const read = (path) => readFileSync(join(root, path), "utf8"); +// Exact names, as git sees them: macOS would find `src/Editor` too. +const files = git(root, "ls-files", "-z", "--cached", "--others", "--exclude-standard").split("\0").filter(Boolean); +const docs = files.filter((path) => isInstructions(path) && path.endsWith(".md")); +const skills = files.map((path) => /^\.claude\/skills\/([^/]+)\/SKILL\.md$/.exec(path)?.[1]).filter(Boolean); + +const isFileOrFolder = (path) => files.includes(path) || files.some((file) => file.startsWith(`${path.replace(/\/$/, "")}/`)); +const names = new Set(files.map((file) => basename(file))); + +/** Why a path named in a doc is wrong, or null. */ +function problem(path) { + if (/[<>~\s]|^https?:|^mailto:/.test(path)) return null; // placeholders, home folders, the web + if (path.includes("*")) return files.some((file) => file.startsWith(path.slice(0, path.indexOf("*")))) ? null : "matches nothing"; + if (path.includes("/")) return isFileOrFolder(path) ? null : "doesn't exist"; + return names.has(path) ? null : "no file has this name"; +} + +const SOURCE = /^(src|src-tauri|scripts|docs|brand|\.claude|\.github|\.agents)\//; +const FILE_NAME = /^[\w.-]+\.(tsx?|mjs|js|json|md|rs|toml|ya?ml|html|css)$/; +const named = (text) => + [...text.matchAll(/`([^`\n]+)`/g)].map((match) => match[1].replace(/:\d+$/, "")).filter((path) => SOURCE.test(path) || FILE_NAME.test(path)); +const linked = (text, doc) => + [...text.matchAll(/\]\(([^)\s#]+)[^)]*\)/g)] + .map((match) => match[1]) + .filter((target) => !/^(https?|mailto):/.test(target)) + .map((target) => normalize(join(dirname(doc), target))); + +const scripts = Object.keys(JSON.parse(read("package.json")).scripts); +const namedCommands = (text) => [...text.matchAll(/(?:^|`)pnpm ([\w:-]+)/gm)].map((match) => match[1]); + +describe("instructions", () => { + it("cover CLAUDE.md, the README, the agents page, and every skill", () => { + expect(docs).toEqual(expect.arrayContaining(["CLAUDE.md", "README.md", "docs/agents.md", ".claude/skills/verify/SKILL.md"])); + }); + + it.each(docs)("%s names and links only files that exist", (doc) => { + const text = read(doc); + const wrong = [...named(text), ...linked(text, doc)] + .map((path) => [path, problem(path)]) + .filter(([, why]) => why) + .map(([path, why]) => `${path}: ${why}`); + expect(wrong).toEqual([]); + }); + + it.each(docs)("%s names only pnpm commands that exist", (doc) => { + const unknown = namedCommands(read(doc)).filter((name) => !scripts.includes(name) && !["install", "exec"].includes(name)); + expect(unknown).toEqual([]); + }); + + it("CLAUDE.md lists every skill, and only those", () => { + const section = /## Skills\n([\s\S]*?)\n## /.exec(read("CLAUDE.md"))?.[1] ?? ""; + const listed = [...section.matchAll(/^- `([\w-]+)`:/gm)].map((match) => match[1]); + expect(listed.sort()).toEqual([...skills].sort()); + }); + + it.each(skills)("the %s skill is named for its folder and says when to use it", (name) => { + const head = /^---\nname: (.+)\ndescription: (.+)\n---\n/.exec(read(`.claude/skills/${name}/SKILL.md`)); + expect(head?.[1]).toBe(name); + expect(head?.[2]).toMatch(/\. Use when /); + }); +}); diff --git a/scripts/guard.test.mjs b/scripts/guard.test.mjs index 661939d..34077eb 100644 --- a/scripts/guard.test.mjs +++ b/scripts/guard.test.mjs @@ -28,7 +28,7 @@ describe("the end-of-turn gate", () => { expect(guard(dir, "turn-end")).toBeNull(); guard(dir, "turn-start"); - write(dir, "README.md", "# App, documented\n"); + write(dir, "LICENSE", "Documented.\n"); expect(guard(dir, "turn-end")).toBeNull(); guard(dir, "turn-start"); @@ -66,7 +66,7 @@ describe("the end-of-turn gate", () => { guard(dir, "turn-start"); write(dir, "app.js", "changed\n"); expect(verify(dir).status).toBe(0); - write(dir, "README.md", "# App, documented\n"); + write(dir, "LICENSE", "Documented.\n"); expect(guard(dir, "turn-end")).toBeNull(); }); diff --git a/scripts/tree.mjs b/scripts/tree.mjs index 155dde9..c2d08a4 100644 --- a/scripts/tree.mjs +++ b/scripts/tree.mjs @@ -58,12 +58,18 @@ export function changedPaths(before, after) { return [...paths].filter((path) => before[path] !== after[path]).sort(); } +/** Instructions for agents and people. `scripts/docs.test.mjs` checks them, so they count as code. */ +export const isInstructions = (path) => + ["CLAUDE.md", "README.md", "docs/agents.md"].includes(path) || /^\.(claude|agents)\//.test(path); + /** * Docs: no check reads them, so a change to them alone needs no verify. Markdown under src/ or - * src-tauri/ may be a fixture the code reads, so it counts as code. + * src-tauri/ may be a fixture the code reads, so it counts as code, as do the instructions. */ export const isDoc = (path) => - !/^src(-tauri)?\//.test(path) && (path.endsWith(".md") || path.startsWith("docs/") || path === "LICENSE"); + !/^src(-tauri)?\//.test(path) && + !isInstructions(path) && + (path.endsWith(".md") || path.startsWith("docs/") || path === "LICENSE"); /** Only CI reads these. Local checks can't vouch for them. */ export const isCiOnly = (path) => path.startsWith(".github/"); diff --git a/scripts/verify.test.mjs b/scripts/verify.test.mjs index 4f4b224..98887b9 100644 --- a/scripts/verify.test.mjs +++ b/scripts/verify.test.mjs @@ -62,12 +62,16 @@ describe("pnpm verify", () => { it("checks nothing when only docs changed since the last pass, and records that tree", () => { const dir = makeRepo(); verify(dir); - writeFileSync(join(dir, "README.md"), "# App, with docs\n"); + mkdirSync(join(dir, "docs")); + writeFileSync(join(dir, "docs/notes.md"), "Notes.\n"); const result = verify(dir); expect(result).toMatchObject({ status: 0, runs: 0 }); - expect(result.out).toContain("Only docs changed since the last pass (README.md)"); + expect(result.out).toContain("Only docs changed since the last pass (docs/notes.md)"); expect(recorded(dir)).toBe(snapshot(dir).fingerprint); + // The docs test reads the instructions, so they count as code. + writeFileSync(join(dir, "README.md"), "# App, with instructions\n"); + expect(verify(dir)).toMatchObject({ status: 0, runs: 1 }); mkdirSync(join(dir, "src")); writeFileSync(join(dir, "src/fixture.md"), "Code may read this.\n"); expect(verify(dir)).toMatchObject({ status: 0, runs: 1 });