feat(auto-patch): enforce proposal editScope + run npm test before cherry-pick (#19)
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 <mayatnikov@me.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit was merged in pull request #19.
This commit is contained in:
Yuriy Mayatnikov
+1
-1
@@ -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",
|
||||
|
||||
+46
-10
@@ -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/<file>): <one-line summary>`.",
|
||||
"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/<file>): <one-line summary>`.",
|
||||
"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]);
|
||||
|
||||
@@ -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 };
|
||||
@@ -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/"]);
|
||||
});
|
||||
Reference in New Issue
Block a user