From 82a8250a1280af8135c92dec59de805caba907d3 Mon Sep 17 00:00:00 2001 From: Yuriy Mayatnikov Date: Tue, 26 May 2026 10:39:18 +0300 Subject: [PATCH] feat(auto-patch): enforce proposal editScope + run npm test before cherry-pick (#19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two safety rails on the unattended self-improvement loop: 1. scripts/edit-scope.js + .test.js: pure helpers that parse `editScope: [...]` out of a proposal frontmatter (the field Phase 6 started writing) and validate a list of changed files against it. 13 tests covering null/missing/malformed frontmatter, directory prefix matching, exact-file matching, default-scope fallback. 2. scripts/auto-patch.js: - reads editScope from the proposal (falls back to ["runtime/"]) - injects the allowed paths into the Pi prompt so Pi knows the boundaries up front - validates the diff against scope + auto-allows any runtime/**/*.test.js files Pi added - runs `npm test` on the patched branch BEFORE cherry-picking; refuses to land a patch that breaks the suite Closes the "Smoke checks run before applying patch" item from plans/autonomous-survival-bot-prd.md §7 Phase 6. npm test now 92/92. Co-authored-by: Yuriy Mayatnikov Co-authored-by: Claude Opus 4.7 (1M context) --- package.json | 2 +- scripts/auto-patch.js | 56 +++++++++++++++++++++----- scripts/edit-scope.js | 58 +++++++++++++++++++++++++++ scripts/edit-scope.test.js | 82 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 187 insertions(+), 11 deletions(-) create mode 100644 scripts/edit-scope.js create mode 100644 scripts/edit-scope.test.js diff --git a/package.json b/package.json index 50e791e..88326cc 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "tui": "tsx tui/tui.tsx", "propose:apply": "node scripts/propose-apply.js", "stop": "bash scripts/stop.sh", - "test": "node --test runtime/skills/contract.test.js runtime/skills/groups.test.js runtime/skills/compat.test.js runtime/curriculum.test.js runtime/social/social.test.js runtime/stuck-incident.test.js runtime/compat.test.js" + "test": "node --test runtime/skills/contract.test.js runtime/skills/groups.test.js runtime/skills/compat.test.js runtime/curriculum.test.js runtime/social/social.test.js runtime/stuck-incident.test.js runtime/compat.test.js scripts/edit-scope.test.js" }, "dependencies": { "dotenv": "^16.4.5", diff --git a/scripts/auto-patch.js b/scripts/auto-patch.js index fb9fe1f..9d9ddce 100644 --- a/scripts/auto-patch.js +++ b/scripts/auto-patch.js @@ -21,6 +21,8 @@ import fs from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { parseEditScope, validateChangedFiles, effectiveScope } from "./edit-scope.js"; + const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); const REPO_ROOT = path.resolve(__dirname, ".."); @@ -76,6 +78,13 @@ if (proposal.status === "pending") { const proposalText = fs.readFileSync(proposal.path, "utf8"); +// Each proposal may declare an editScope in its frontmatter so the patch is +// kept to the specific module(s) the operator (or the stuck detector) marked +// as relevant. Older proposals without editScope fall back to ["runtime/"], +// matching the historical default. +const scope = effectiveScope(parseEditScope(proposalText)); +log("info", `edit scope: ${JSON.stringify(scope)}`); + // Capture current main HEAD so we can roll back to it if cherry-pick fails. const baseSha = git(["rev-parse", "HEAD"]).stdout.trim(); @@ -91,6 +100,7 @@ git(["branch", "-D", branch]); // ignore error if absent const checkout = git(["checkout", "-b", branch]); if (checkout.status !== 0) exit(2, `cannot create branch ${branch}: ${checkout.stderr}`); +const scopeBullet = scope.map((p) => ` - \`${p}\``).join("\n"); const prompt = [ "You are patching the pepa-pi-bot repo to address an automatically-detected failure.", "This is an UNATTENDED run — no operator will review your output before it lands on main.", @@ -102,12 +112,15 @@ const prompt = [ "", "## Hard rules (non-negotiable)", "", - "1. Touch only files under `runtime/`. Do NOT modify `tui/`, `extensions/`, `scripts/`, `docs/`, `package.json`, or anything in `state/`.", - "2. Do not introduce npm dependencies.", - "3. Do not push, do not open a PR. Commit on the current branch only.", - "4. Use a conventional commit message: `fix(runtime/): `.", - "5. If you can't safely fix the issue, write a short comment in the relevant runtime file explaining why and stop — do NOT make a speculative change.", - "6. Make exactly ONE commit. If you find multiple issues, focus on the one the proposal describes.", + "1. Touch ONLY files matching the edit scope below. Any other path will be rejected after you commit and the patch will be discarded:", + scopeBullet, + "2. You MAY add or update test files under `runtime/**/*.test.js` even if not listed above — tests for the fix are encouraged.", + "3. Do not introduce npm dependencies. Do not modify `package.json`, `tui/`, `extensions/`, `scripts/`, `docs/`, `.env`, or anything in `state/`.", + "4. Do not push, do not open a PR. Commit on the current branch only.", + "5. Use a conventional commit message: `fix(runtime/): `.", + "6. Run `npm test` mentally before committing — your patch must keep all existing tests green; the auto-patcher will run `npm test` and discard the patch if it fails.", + "7. If you can't safely fix the issue, write a short comment in the relevant runtime file explaining why and stop — do NOT make a speculative change.", + "8. Make exactly ONE commit. If you find multiple issues, focus on the one the proposal describes.", "", "After you commit, your job is done.", ].join("\n"); @@ -152,16 +165,39 @@ pi.on("exit", (code) => { exit(1, "pi made no commit"); } - // Verify the commit touched only runtime/. + // Verify the commit only touched files inside the declared edit scope. + // Test files under runtime/**/*.test.js are always allowed — they're how + // Pi proves the fix is safe and how the smoke gate below checks pass. const filesChanged = git(["diff", "--name-only", `${baseSha}..HEAD`]).stdout.trim().split("\n").filter(Boolean); - const outside = filesChanged.filter((f) => !f.startsWith("runtime/")); - if (outside.length > 0) { - log("error", `commit touched files outside runtime/: ${outside.join(", ")} — discarding`); + const testFiles = filesChanged.filter((f) => /^runtime\/.*\.test\.js$/.test(f)); + const scopeWithTests = [...scope, ...testFiles]; + const validation = validateChangedFiles(filesChanged, scopeWithTests); + if (!validation.ok) { + log("error", `commit touched files outside scope ${JSON.stringify(scope)}: ${validation.outsideFiles.join(", ")} — discarding`); git(["checkout", "main"]); git(["branch", "-D", branch]); exit(2, "patch touched off-limits files"); } + // Smoke gate: run `npm test` on the patched branch BEFORE cherry-picking. + // Anything that turns the suite red gets thrown away — even if Pi thinks + // the change is correct. + log("info", "running npm test smoke gate (timeout 5 min)"); + const smoke = spawnSync("npm", ["test"], { + cwd: REPO_ROOT, + encoding: "utf8", + env: { ...process.env, CI: "1" }, + timeout: 5 * 60 * 1000, + }); + if (smoke.status !== 0) { + const tail = ((smoke.stdout || "") + "\n" + (smoke.stderr || "")).split("\n").slice(-10).join("\n"); + log("error", `npm test FAILED on patched branch — discarding\n${tail}`); + git(["checkout", "main"]); + git(["branch", "-D", branch]); + exit(2, "patch failed smoke (npm test)"); + } + log("info", "smoke gate passed"); + // Cherry-pick onto main. git(["checkout", "main"]); const cherry = git(["cherry-pick", newHead]); diff --git a/scripts/edit-scope.js b/scripts/edit-scope.js new file mode 100644 index 0000000..2ca3780 --- /dev/null +++ b/scripts/edit-scope.js @@ -0,0 +1,58 @@ +// Helpers for auto-patch.js — kept separate so they can be unit-tested +// without spawning git/pi subprocesses. + +const DEFAULT_SCOPE = ["runtime/"]; + +// Parse `editScope: [...]` out of a proposal markdown frontmatter block. +// Returns the array of path prefixes, or null if absent or malformed. +// Callers should fall back to DEFAULT_SCOPE when null. +export function parseEditScope(proposalText) { + if (!proposalText) return null; + const m = String(proposalText).match(/^---\n([\s\S]*?)\n---/); + if (!m) return null; + const scopeLine = m[1].split("\n").find((l) => l.trim().startsWith("editScope:")); + if (!scopeLine) return null; + const raw = scopeLine.slice(scopeLine.indexOf(":") + 1).trim(); + try { + const parsed = JSON.parse(raw); + if (!Array.isArray(parsed)) return null; + const cleaned = parsed + .filter((p) => typeof p === "string" && p.length > 0) + .map((p) => p.replace(/^\.?\/+/, "")); // strip leading ./ or / + return cleaned.length > 0 ? cleaned : null; + } catch { + return null; + } +} + +// A changed file is in-scope when it matches any entry in `scope`. +// An entry ending with `/` is a directory prefix; anything else is an +// exact-match file path. The match is case-sensitive (we're on POSIX +// repos). +export function isFileInScope(file, scope) { + for (const entry of scope) { + if (!entry) continue; + if (entry.endsWith("/")) { + if (file === entry.slice(0, -1)) return true; // edge case + if (file.startsWith(entry)) return true; + } else if (file === entry) { + return true; + } else if (file.startsWith(`${entry}/`)) { + // allow "runtime/skills" to cover "runtime/skills/foo.js" + return true; + } + } + return false; +} + +export function validateChangedFiles(changedFiles, scope) { + const effective = Array.isArray(scope) && scope.length ? scope : DEFAULT_SCOPE; + const outside = (changedFiles ?? []).filter((f) => !isFileInScope(f, effective)); + return { ok: outside.length === 0, outsideFiles: outside, effectiveScope: effective }; +} + +export function effectiveScope(scope) { + return Array.isArray(scope) && scope.length ? scope : DEFAULT_SCOPE.slice(); +} + +export const _internal = { DEFAULT_SCOPE }; diff --git a/scripts/edit-scope.test.js b/scripts/edit-scope.test.js new file mode 100644 index 0000000..d254ccb --- /dev/null +++ b/scripts/edit-scope.test.js @@ -0,0 +1,82 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { parseEditScope, isFileInScope, validateChangedFiles, effectiveScope } from "./edit-scope.js"; + +test("parseEditScope returns null when no frontmatter present", () => { + assert.equal(parseEditScope(""), null); + assert.equal(parseEditScope("# no frontmatter\nbody"), null); +}); + +test("parseEditScope returns null when frontmatter has no editScope key", () => { + const text = "---\nkind: test\napproved: false\n---\nbody"; + assert.equal(parseEditScope(text), null); +}); + +test("parseEditScope reads a JSON array", () => { + const text = '---\nkind: test\neditScope: ["runtime/skills/eat.js","runtime/skills/"]\n---\nbody'; + assert.deepEqual(parseEditScope(text), ["runtime/skills/eat.js", "runtime/skills/"]); +}); + +test("parseEditScope strips leading ./ and / from entries", () => { + const text = '---\neditScope: ["./runtime/", "/scripts/edit-scope.js"]\n---\n'; + assert.deepEqual(parseEditScope(text), ["runtime/", "scripts/edit-scope.js"]); +}); + +test("parseEditScope returns null for malformed JSON", () => { + const text = "---\neditScope: not-json\n---\n"; + assert.equal(parseEditScope(text), null); +}); + +test("isFileInScope: directory prefix matches descendants", () => { + assert.equal(isFileInScope("runtime/skills/eat.js", ["runtime/skills/"]), true); + assert.equal(isFileInScope("runtime/bot.js", ["runtime/skills/"]), false); +}); + +test("isFileInScope: bare directory name matches as prefix", () => { + // "runtime/skills" should still cover "runtime/skills/foo.js" + assert.equal(isFileInScope("runtime/skills/foo.js", ["runtime/skills"]), true); + // but not "runtime/skillsX/foo.js" + assert.equal(isFileInScope("runtime/skillsX/foo.js", ["runtime/skills"]), false); +}); + +test("isFileInScope: exact file match", () => { + assert.equal(isFileInScope("runtime/bot.js", ["runtime/bot.js"]), true); + assert.equal(isFileInScope("runtime/bot.js.bak", ["runtime/bot.js"]), false); +}); + +test("validateChangedFiles: all in-scope → ok", () => { + const out = validateChangedFiles( + ["runtime/skills/eat.js", "runtime/skills/groups.js"], + ["runtime/skills/"], + ); + assert.equal(out.ok, true); + assert.deepEqual(out.outsideFiles, []); +}); + +test("validateChangedFiles: any out-of-scope → not ok, list returned", () => { + const out = validateChangedFiles( + ["runtime/skills/eat.js", "package.json", ".env"], + ["runtime/skills/"], + ); + assert.equal(out.ok, false); + assert.deepEqual(out.outsideFiles.sort(), [".env", "package.json"]); +}); + +test("validateChangedFiles: falls back to default scope when empty", () => { + const out = validateChangedFiles(["runtime/bot.js"], null); + assert.equal(out.ok, true); + assert.deepEqual(out.effectiveScope, ["runtime/"]); +}); + +test("validateChangedFiles: default scope rejects scripts/", () => { + const out = validateChangedFiles(["scripts/auto-patch.js"], []); + assert.equal(out.ok, false); + assert.deepEqual(out.outsideFiles, ["scripts/auto-patch.js"]); +}); + +test("effectiveScope mirrors fallback", () => { + assert.deepEqual(effectiveScope(null), ["runtime/"]); + assert.deepEqual(effectiveScope([]), ["runtime/"]); + assert.deepEqual(effectiveScope(["x/"]), ["x/"]); +});