diff --git a/CHANGELOG.md b/CHANGELOG.md index f5b7c673..e30c1983 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Notable changes to house-rules. Versions follow [semantic versioning](https://se ## Unreleased - Make `check-reconciliation.mjs` fail closed on how much of the plan it read. It reports `covered N/M`: the open and deferred plan lines mapped to items over every such line in the plan files it was given, counting lines outside the `## Ledger` section and table rows it does not read as items. A short count is a gap (exit 2) or an error (exit 1), a ledger section with no item lines is an error, and a receipt that reads no plan file is partial with `covered 0/0` unless its new `plan` field says `"none: "`. The example receipt gains that field. +- Stop hooks: read a plan whose first line is a `# Title` (the title no longer ends the header, so `Stop-check: strict` under it counts), treat a `Status:` that is a sentence about progress as active instead of skipping the plan (only paused, blocked, waiting, on hold, held, complete, completed, done, archived, abandoned, cancelled, superseded or closed stop it), and find the plan a session used in a transcript whose records are not Claude Code's (a Codex rollout) by the plan paths its tool calls name (a plan only read counts too). Each gap let a reply that stopped with ready work pass unchecked. - Let a ledger item end with a machine-checkable `done-when: [; ]` clause, with conditions `pr-merged owner/repo#n`, `issue-closed owner/repo#n` and `path-absent `, all of which must hold. `plan-ledger.mjs` exposes the parsed clause on each open or deferred item (the clause must be the item's last field; one it cannot read is an error, and text in backticks is ignored); `check-open-work.mjs` prints a "done-when" group of closeable, pending or error lines and its exit code is unchanged; the new `done-when.mjs ` prints the same and writes nothing, and with `--apply` closes only the closeable lines through `close-ledger-line.mjs` with the evidence found. A failed lookup, an unknown reference, a pull request named as an issue (or the reverse) or a path that cannot be read is an error and never closes a line. No existing plan file changes. - Make `merge-pr.sh` work from a checkout of a fork. It takes the repository from the pull request's name, never from `gh repo view` (which names the upstream in every checkout that has an upstream remote), passes it to every gh call, and fetches and pushes through the checkout's remote whose URL matches that repository (https or ssh, any case; `origin` first, or `--remote`). It refuses only when no remote matches, and compares the `HR_PUSH_LANDED` list case-insensitively. A new `--explain` flag runs the same conditions read-only and prints `met` or `UNMET` for each (open, no dependents, review threads, bot review of the head, bot review rounds against `HR_REVIEW_ROUND_CAP`, other checks, fetched head, the repository's check, and for push-landed repositories the base ancestry and the head-branch lease), exits 0 when all are met and 1 otherwise, and lands nothing. - Keep the head branch in `merge-pr.sh` when another open pull request in any repository has it as its head, such as a fork's mirror of an upstream pull request. Deleting the branch closed that other pull request without notice. The landing still goes ahead (merge or fast-forward push), the script names the pull requests that keep the branch, and `verify-landed.sh` is not asked to expect it gone. A failed lookup of those pull requests, or one that finds no such branch, refuses the merge. diff --git a/evals/end-of-reply/README.md b/evals/end-of-reply/README.md index c6eb625d..218e7b8d 100644 --- a/evals/end-of-reply/README.md +++ b/evals/end-of-reply/README.md @@ -258,10 +258,10 @@ trigger ("blocked on", "Blocker:", "Trigger:", "reopens when", "recheck on `, `>>`, `tee`, `sed -i`). A `cd` earlier in the same command and a bare `plan.md` are // followed. Reading a plan does not count. A path that is not a file on disk is skipped. A session that never -// touched a plan file has nothing to check, and its reply passes. +// touched a plan file has nothing to check, and its reply passes. A transcript with no `user` or `assistant` +// records (a Codex rollout) has no such structure, so the plan paths its tool-call text names count instead, +// read or written, the latest mention being the plan checked; a path with a space or a comma in it is not found. // // The shell reading is a best effort, not a shell. It counts a stored command (`C="node …/close-ledger-line.mjs // plan.md"`) as naming that plan whether or not it is run, cannot tell which side of `||` or `&&` ran, does not carry a `cd` through the rest of a list that ends in `&`, treats a backtick substitution as plain text, lets a `cd` inside a brace group that feeds a pipe leak to later commands, does not skip the values of `node` or `bun` options (`--eval=…`, `--require x`) before the script, reads `<&` as a background marker, and resolves `..` in a plan path lexically, not through symlinks. @@ -556,6 +558,86 @@ export function planPaths(list, base) { return paths; } +// Characters that end a path token in command text; a path with one of them is not found by the fallback. +const PATH_END = /[\s"'`\\<>|;&(),={}[\]]/; +const PLAN_TOKEN = /^(?:~|\$HOME|\$\{HOME\}|)(?:\/[^/]+)*\/plans\/[A-Za-z0-9._-]+\/plan\.md$/; +const PLAN_TAIL = "/plan.md"; +const MAX_TOKEN = 512; + +/** + * The plan paths one string of command text names, in order. It finds each `/plan.md`, takes the token around it + * (at most MAX_TOKEN characters back to a delimiter) and keeps it when the token is a `plans//plan.md` + * path starting with `~`, `$HOME` or `/` that ends at the file name. It never scans more than MAX_TOKEN + * characters per occurrence, so a long run of path characters costs time in proportion to its length. + * @param {string} text + */ +function planTokens(text) { + /** @type {string[]} */ + const tokens = []; + for (let at = text.indexOf(PLAN_TAIL); at !== -1; at = text.indexOf(PLAN_TAIL, at + 1)) { + const end = at + PLAN_TAIL.length; + // The file name must end the token: the end of the text or a delimiter, so `plan.md~`, `plan.md.bak` and + // `plan.md2` are other files. + const after = text.charAt(end); + if (after !== "" && !PATH_END.test(after)) { + continue; + } + let start = at; + while (start > 0 && at - start < MAX_TOKEN && !PATH_END.test(text.charAt(start - 1))) { + start--; + } + // `}` ends a token (JSON and shell text), but `${HOME}` before the path is part of it. + if (text.slice(Math.max(0, start - 7), start) === "${HOME}") { + start -= 7; + } + const token = text.slice(start, end); + if (PLAN_TOKEN.test(token)) { + tokens.push(token); + } + } + return tokens; +} + +/** The strings of a Codex rollout's tool-call records: the code or arguments the model sent, not their output. */ +const toolCallText = (/** @type {any} */ entry) => + entry?.type === "response_item" && entry.payload && String(entry.payload.type).endsWith("_call") + ? [entry.payload.input, entry.payload.arguments].filter((value) => typeof value === "string") + : []; + +/** + * The plan files named in the tool calls of a transcript whose records are not Claude Code's (a Codex rollout), + * oldest mention first. The records' own structure is not parsed beyond finding the call text, so it cannot tell a + * plan the session wrote from one it only read: both count. + * @param {any[]} list + * @param {string} base the directory relative paths resolve against + */ +export function planPathsInCalls(list, base) { + /** @type {Map} */ + const last = new Map(); + // One counter step per mention, so two plans named in the same call keep the order they appear in. + let order = 0; + for (const entry of list) { + for (const text of toolCallText(entry)) { + for (const token of planTokens(text)) { + last.set(absolute(token, base), order++); + } + } + } + return [...last].sort((a, b) => a[1] - b[1]).map(([path]) => path); +} + +/** + * The plan files the session used, oldest first. A Claude Code transcript yields the ones its tool calls wrote or + * named, and reading a plan does not count. Any other transcript (no `user` or `assistant` records) yields the + * ones its tool calls name, read or written. + * @param {any[]} list the parsed transcript lines + * @param {string} base + */ +export function sessionPlanPaths(list, base) { + const claude = list.some((entry) => entry?.type === "assistant" || entry?.type === "user"); + return claude ? planPaths(list, base) : planPathsInCalls(list, base); +} + /** * @param {{ reply: string, planPath: string | undefined, planText: string | undefined }} state * @returns {{ block: boolean, reason?: string, note?: string }} @@ -601,7 +683,7 @@ export function evaluate(input, files) { if (!claimsDone(reply)) { return { block: false }; } - for (const planPath of planPaths(list, input.cwd ?? process.cwd()).toReversed()) { + for (const planPath of sessionPlanPaths(list, input.cwd ?? process.cwd()).toReversed()) { const planText = files.read(planPath); if (planText !== undefined) { return decide({ reply, planPath, planText }); diff --git a/scripts/stop-ready-work-check.mjs b/scripts/stop-ready-work-check.mjs index edad5c2a..02d8edfc 100644 --- a/scripts/stop-ready-work-check.mjs +++ b/scripts/stop-ready-work-check.mjs @@ -8,8 +8,10 @@ // A wait on an open item is a `trigger:` or `recheck:` field, `blocked-by:`, `owner:`, or the words "waiting // on/for", "waits on", "awaiting" or "blocked on/by" in the item. A `recheck:` date counts while it is still ahead, // and `owner:` only when it names someone other than the plan's own owner. Deferred `[~]` items do not count: they were -// parked on purpose and carry their own trigger. A plan whose `Status:` is not `active` (paused, -// blocked-on-user, completed) is skipped, because the user or the record already stopped it. +// parked on purpose and carry their own trigger. A plan whose `Status:` begins with paused, +// blocked, waiting, on hold, held, complete, completed, done, archived, abandoned, cancelled, superseded or closed is skipped, +// because the user or the record already stopped it; any other text (`active`, or a sentence about progress) counts +// as active. // // The reply passes when it holds any of: a coded offer (`Question 1` with options such as `1a`, a // "(Recommended" and a "reply `" line, outside any quoted example); a stated blocker or trigger ("blocked on", @@ -19,7 +21,7 @@ // (code spans, fenced blocks, double-quoted spans) is ignored first, so a reply that merely mentions a phrase // does not satisfy or trip the check. stop-claim-check.mjs separately holds back a false completion claim. // -// Strict reading, per plan: a plan whose header (first 40 lines, before the first heading) holds a `Stop-check: strict` line is blocked when ANY +// Strict reading, per plan: a plan whose header (first 40 lines, before the first heading after an optional title line) holds a `Stop-check: strict` line is blocked when ANY // open `[ ]` item is ready (names no wait), not only when none waits, and a "light" offer (an acceptance line, // "Recommended: reply `1a` or `go`", beside coded options but with no "Question N" heading) counts as an offer. // A strict plan also tolerates lint errors on its ledger lines (a done line without evidence); only a header @@ -35,7 +37,13 @@ import { existsSync, readFileSync, realpathSync } from "node:fs"; import { pathToFileURL } from "node:url"; import { parsePlan, RECHECK } from "../skills/hr-what-next/scripts/plan-ledger.mjs"; import { replyFacts } from "./continuity-boundary.mjs"; -import { entries, finalReply, planPaths, unquoted, unquotedProse } from "./stop-claim-check.mjs"; +import { + entries, + finalReply, + sessionPlanPaths, + unquoted, + unquotedProse, +} from "./stop-claim-check.mjs"; const WAIT = /\b(?:trigger|blocked-by):(?!\s*(?:none|n\/a|nothing)\s*(?:[.!?\n;]|$))|\b(?:waiting (?:on|for)|waits? on|awaiting|blocked (?:on|by))\b/i; @@ -108,9 +116,15 @@ const withoutFences = (text) => { */ const headerOf = (planText) => { const lines = withoutFences(planText.split(/\r?\n/).slice(0, 40).join("\n")).split("\n"); - const heading = lines.findIndex((line) => line.startsWith("#")); - return lines.slice(0, heading < 0 ? lines.length : heading).join("\n"); + // A level-1 title on the first non-blank line belongs to the header: it runs to the first heading after it. + const first = lines.findIndex((line) => line.trim() !== ""); + const start = first >= 0 && /^#\s/.test(lines[first] ?? "") ? first + 1 : 0; + const heading = lines.findIndex((line, index) => index >= start && line.startsWith("#")); + return lines.slice(start, heading < 0 ? lines.length : heading).join("\n"); }; +// A Status that says the work stopped. Any other text, such as a sentence about progress, is an active plan. +const STOPPED = + /^(?:paused|blocked|waiting|on[- ]hold|held|completed?|done|archived|abandoned|cancell?ed|superseded|closed)\b/i; const localDate = () => new Date().toLocaleDateString("en-CA"); const HANDBACK = /(? item.mark === " "); @@ -218,7 +232,7 @@ export function evaluate(input, files) { typeof input.last_assistant_message === "string" ? input.last_assistant_message : finalReply(list); - for (const planPath of planPaths(list, input.cwd ?? process.cwd()).toReversed()) { + for (const planPath of sessionPlanPaths(list, input.cwd ?? process.cwd()).toReversed()) { const planText = files.read(planPath); if (planText !== undefined) { return decide({ reply, planPath, planText }); diff --git a/test/stop-ready-work-check.test.mjs b/test/stop-ready-work-check.test.mjs index 74d1f3f5..269844de 100644 --- a/test/stop-ready-work-check.test.mjs +++ b/test/stop-ready-work-check.test.mjs @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import { spawnSync } from "node:child_process"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { homedir, tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { decide, evaluate } from "../scripts/stop-ready-work-check.mjs"; @@ -421,3 +421,124 @@ test("run as a program it blocks once, then passes the continuation, a corrupt t rmSync(dir, { recursive: true, force: true }); } }); + +const TITLED = (/** @type {string} */ header, status = "active") => + `# Release plan\n\nOwner: test\nStatus: ${status}\n${header}\n## Ledger\n- [ ] Write the next step\n- [ ] Land it; waiting on CI\n`; + +test("a title line does not end the plan header, so a strict marker under it counts", () => { + assert.equal(check(WAITING_LINE, TITLED("Stop-check: strict")).block, true); + assert.equal(check(WAITING_LINE, TITLED("")).block, false); +}); + +test("a Status that is a sentence about progress counts as active; a stopped word still skips the plan", () => { + const progress = "PR 49 landed; release preparation remains open"; + assert.equal(check(WAITING_LINE, TITLED("Stop-check: strict", progress)).block, true); + for (const status of [ + "paused", + "blocked-on-user", + "Completed 2026-10-29", + "waiting for the user", + "done", + ]) { + assert.equal(check(WAITING_LINE, TITLED("Stop-check: strict", status)).block, false, status); + } +}); + +test("a Codex rollout, whose records are not Claude's, still finds the plan the transcript names", () => { + const rollout = jsonl( + { type: "session_meta", payload: { id: "t" } }, + { + type: "response_item", + payload: { + type: "custom_tool_call", + name: "exec", + input: + 'tools.exec_command({cmd: "cd /work && python3 close.py /work/notes/plans/topic/plan.md"})', + }, + }, + ); + const planPath = "/work/notes/plans/topic/plan.md"; + const files = reader({ "/t.jsonl": rollout, [planPath]: TITLED("Stop-check: strict") }); + const input = { transcript_path: "/t.jsonl", last_assistant_message: WAITING_LINE }; + assert.equal(evaluate(input, files).block, true); + const none = reader({ "/t.jsonl": jsonl({ type: "session_meta", payload: {} }) }); + assert.equal(evaluate(input, none).block, false); +}); + +test("the transcript-text fallback is limited to non-Claude records and exact plan paths", () => { + const planPath = "/work/notes/plans/topic/plan.md"; + const input = { transcript_path: "/t.jsonl", last_assistant_message: WAITING_LINE }; + const call = (/** @type {string} */ text) => + jsonl({ type: "response_item", payload: { type: "custom_tool_call", input: text } }); + const withPlan = (/** @type {string} */ rollout) => + evaluate(input, reader({ "/t.jsonl": rollout, [planPath]: TITLED("Stop-check: strict") })) + .block; + assert.equal(withPlan(call(`cat ${planPath}`)), true); + // a backup file, a longer name, a path with no plans// parent, and tool output are not the plan + assert.equal(withPlan(call(`cat ${planPath}.bak`)), false); + assert.equal(withPlan(call(`cat ${planPath}~`)), false); + assert.equal(withPlan(call("cat /work/notes/plans/topic/plan.md2")), false); + assert.equal(withPlan(call("cat /work/plan.md")), false); + const output = jsonl({ + type: "response_item", + payload: { type: "custom_tool_call_output", output: planPath }, + }); + assert.equal(withPlan(output), false); + // a Claude transcript that only reads the plan keeps passing, as before + const read = jsonl(user, assistant([tool("Read", { file_path: planPath })])); + assert.equal(withPlan(read), false); +}); + +test("a run of path characters is scanned in linear time", () => { + const started = performance.now(); + const call = jsonl({ + type: "response_item", + payload: { type: "custom_tool_call", input: "/".repeat(2_000_000) }, + }); + evaluate( + { transcript_path: "/t.jsonl", last_assistant_message: WAITING_LINE }, + reader({ "/t.jsonl": call }), + ); + assert.ok(performance.now() - started < 1000); +}); + +test("a title after a blank line still ends no header, and held or complete skip the plan", () => { + assert.equal(check(WAITING_LINE, `\n${TITLED("Stop-check: strict")}`).block, true); + for (const status of ["held", "complete", "on-hold", "on hold"]) { + assert.equal(check(WAITING_LINE, TITLED("Stop-check: strict", status)).block, false, status); + } +}); + +test("the plan named last in a call is the one checked, even when an earlier plan shares the call", () => { + const active = "/work/notes/plans/active/plan.md"; + const done = "/work/notes/plans/done/plan.md"; + const call = jsonl( + { + type: "response_item", + payload: { type: "custom_tool_call", input: `cat ${active}` }, + }, + { + type: "response_item", + payload: { type: "custom_tool_call", input: `cat ${done} ${active}` }, + }, + ); + const files = reader({ + "/t.jsonl": call, + [active]: TITLED("Stop-check: strict"), + [done]: TITLED("Stop-check: strict", "completed"), + }); + const input = { transcript_path: "/t.jsonl", last_assistant_message: WAITING_LINE }; + assert.equal(evaluate(input, files).block, true); +}); + +test("a plan path written with a braced HOME keeps its prefix in the fallback", () => { + // the hook expands `${HOME}` with os.homedir(), which is not process.env.HOME on Windows + const planPath = `${homedir()}/notes/plans/topic/plan.md`; + const rollout = jsonl({ + type: "response_item", + payload: { type: "custom_tool_call", input: "cat ${HOME}/notes/plans/topic/plan.md" }, + }); + const files = reader({ "/t.jsonl": rollout, [planPath]: TITLED("Stop-check: strict") }); + const input = { transcript_path: "/t.jsonl", last_assistant_message: WAITING_LINE }; + assert.equal(evaluate(input, files).block, true); +});