From a78f5b1b49da5cc0d1b0c8509230d31faee4e0bb Mon Sep 17 00:00:00 2001 From: Param Harrison Date: Thu, 24 Sep 2026 13:48:05 +0300 Subject: [PATCH] chore: update factory skills, hooks and settings to runner v2.5.2 Co-Authored-By: Claude Sonnet 5 --- .claude/agents/factory-reviewer.md | 8 +++-- .claude/agents/factory-verifier.md | 4 +-- .claude/hooks/guard-paths.sh | 3 +- .claude/settings.json | 4 ++- .claude/skills/factory-build/SKILL.md | 19 ++++++------ .../skills/factory-comment/assets/verdict.md | 2 +- .claude/skills/factory-plan/SKILL.md | 13 +++++++- .claude/skills/factory-triage/SKILL.md | 5 +++- .claude/skills/factory-verify/SKILL.md | 30 ++++++++++++++----- 9 files changed, 62 insertions(+), 26 deletions(-) diff --git a/.claude/agents/factory-reviewer.md b/.claude/agents/factory-reviewer.md index 761d90d..3f33aa0 100644 --- a/.claude/agents/factory-reviewer.md +++ b/.claude/agents/factory-reviewer.md @@ -27,9 +27,11 @@ build stage (or a human) fixes them. ## How to report -One finding per line, each naming the file and line, what's wrong, and why -it matters — not a style pass. Severity matters: separate "blocks this -verdict" from "worth a follow-up issue, not blocking." If you find +One finding per item, each with: severity (`must` fix, `should` fix, or +`could` fix), confidence 0-5, what is wrong, where (file and line), why it +matters, and the fix. Not a style pass. Confidence 0-2 means you could not +show it from the diff: leave it out. `must` and `should` at 3 or more block +the verdict; `could` does not. If you find nothing, say "None" plainly; do not manufacture a nitpick to look thorough. diff --git a/.claude/agents/factory-verifier.md b/.claude/agents/factory-verifier.md index a024464..2eb105c 100644 --- a/.claude/agents/factory-verifier.md +++ b/.claude/agents/factory-verifier.md @@ -14,8 +14,8 @@ Your job is to rule that out before you believe anything else. 1. Identify the non-test hunks in the diff (everything except the new or changed test files). -2. Revert only those hunks (`git stash`/`git apply -R`/manual edit — pick - whichever is clean for this diff), keeping the new tests in place. +2. Revert only those hunks (`git stash` or `git checkout -- `, + one command at a time; `git apply` is not allowed), keeping the new tests in place. 3. Run the new tests. They must fail, and fail for the stated reason (not a compile error, not an unrelated crash). If they pass without the implementation, the test proves nothing — reject. diff --git a/.claude/hooks/guard-paths.sh b/.claude/hooks/guard-paths.sh index 4bb2a91..9e24caf 100755 --- a/.claude/hooks/guard-paths.sh +++ b/.claude/hooks/guard-paths.sh @@ -25,7 +25,8 @@ project_dir = sys.argv[1] try: hook_input = json.load(sys.stdin) except Exception: - sys.exit(0) # can't parse our own input: fail open, never fail the session + print("guard-paths: blocked, could not parse the hook input, so paths can't be checked (fails closed)", file=sys.stderr) + sys.exit(2) tool_name = hook_input.get("tool_name", "") tool_input = hook_input.get("tool_input", {}) diff --git a/.claude/settings.json b/.claude/settings.json index c36b406..404c343 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,7 +6,9 @@ "Bash(git merge *)", "Bash(git reset --hard *)", "Bash(git push --force*)", - "Bash(git push -f*)" + "Bash(git push -f*)", + "Read(~/.config/gh/**)", + "Read(~/.ssh/**)" ] }, "hooks": { diff --git a/.claude/skills/factory-build/SKILL.md b/.claude/skills/factory-build/SKILL.md index 5ea36bb..d6e242c 100644 --- a/.claude/skills/factory-build/SKILL.md +++ b/.claude/skills/factory-build/SKILL.md @@ -35,9 +35,9 @@ Quote the exact gate line — the command and its pass/fail output — verbatim in the status comment. Do not paraphrase or summarize a failure as "some tests failed"; show the line that failed. -If the gate fails and you can see why, fix it and re-run. Don't loop more -than a few times guessing; if you can't get it green, say so in the status -comment and stop — `factory-verify` will catch a red gate anyway, but a +If the gate fails and you can see why, fix it and re-run. Stop after 3 +failed gate runs: write `"outcome": "blocked"` and say so in the status +comment. If you can't get it green, say so in the status comment and stop — `factory-verify` will catch a red gate anyway, but a build that knows it's broken shouldn't pretend otherwise. ## 4. Escape hatch: back to needs-info mid-build @@ -47,7 +47,7 @@ only a human can make, stop here rather than guessing: - Write `.factory/runs/issue-/question-comment.md` with the `factory-comment` skill's `question.md` template. -- Write `.factory/runs/issue-/build.json` with `"status": "needs_info"`. +- Write `.factory/runs/issue-/build.json` with `"status": "needs-info"`. - Leave the worktree and any partial commits as they are — the runner preserves both the worktree and the current stage so build can resume from here once the question is answered, instead of starting over. @@ -62,14 +62,15 @@ Write `.factory/runs/issue-/status-comment.md` using the ```json { "status": "green", - "gate_line": "make check: 42 pass, 0 fail", + "gate_line": "FACTORY_GATES: status=GREEN passed=1 failed=0 skipped=0", "rounds": 1 } ``` +`outcome` is optional: `complete` (the default), `blocked` (you cannot go on and a human must +look; put the reason in `summary`), or `failed`. No other fields are allowed. + `status` is one of `green`, `red` (gate never went green after reasonable -effort), or `needs-info` (see step 4). `rounds` is this issue's build -attempt count so far, including any verify-reject that sent you back here -— read the previous `build.json` if present and increment it yourself; the -runner does not track this for you. Leave the worktree exactly as you want +effort), or `needs-info` (see step 4). Write `"rounds": 1`: the runner counts +build attempts itself and replaces the value. Leave the worktree exactly as you want it committed — the runner commits and pushes it verbatim. diff --git a/.claude/skills/factory-comment/assets/verdict.md b/.claude/skills/factory-comment/assets/verdict.md index e8b56cb..e3d8e4a 100644 --- a/.claude/skills/factory-comment/assets/verdict.md +++ b/.claude/skills/factory-comment/assets/verdict.md @@ -3,7 +3,7 @@ ### Acceptance criteria evidence {{#each ac}} -- **AC-{{n}}:** {{pass_or_fail}} — {{evidence_command_and_result}} +- **AC-{{n}}:** {{pass|fail|unverified}} — {{evidence_command_and_result}} {{/each}} ### The test that bites diff --git a/.claude/skills/factory-plan/SKILL.md b/.claude/skills/factory-plan/SKILL.md index 95e1f56..473ffcc 100644 --- a/.claude/skills/factory-plan/SKILL.md +++ b/.claude/skills/factory-plan/SKILL.md @@ -16,6 +16,8 @@ runner posts and labels. Runs after `factory-triage` returned `proceed`. handoff: type, risk hint, done_when, files_expected. - `.factory/runs/issue-/revise.md` — present only when a human sent `/factory revise `: their feedback on the previous plan revision. + `revision.md` beside it repeats that feedback with every earlier round + of feedback and the previous summary; honour all of it. - `AGENTS.md`, `.factory/charter.md`, and the repo's skills index (`.claude/skills/*/SKILL.md`, minus `factory-*`) — what repo-specific skills exist to apply (e.g. `handling-money`). @@ -31,7 +33,7 @@ own context doing the same search yourself. ## 3. Write the plan -One line goal. Acceptance criteria `AC-1..n`, each checkable by a named +One line goal. Acceptance criteria `AC-1..n` (ids are never renumbered on a revision), each checkable by a named command or test. Non-goals `NG-1..n`: binding — the verifier fails a diff that crosses one, so write ones you actually mean. Files to touch. Tests to write first, named. Repo skills to apply, or "none". Risk: low, medium, or @@ -63,5 +65,14 @@ plan, or the previous revision + 1 when `revise.md` is present. Then write } ``` +`outcome` is optional: `complete` (the default), `blocked` (you cannot go on and a human must +look; put the reason in `summary`), or `failed`. No other fields are allowed. + `autoApproveEligible` is your judgment call, not just a mirror of `risk`: set it false for anything you'd want a second look at even at low risk. + +If you stop to ask a question instead (see the escape hatch in step 3), write +`question-comment.md` with the `factory-comment` skill's `question.md` +template and a `plan.json` of `{"status": "needs-info", "risk": "low", +"revision": 1, "files": [], "autoApproveEligible": false}`. The runner posts +the question and resumes planning once a trusted reply arrives. diff --git a/.claude/skills/factory-triage/SKILL.md b/.claude/skills/factory-triage/SKILL.md index 75c9249..909e04e 100644 --- a/.claude/skills/factory-triage/SKILL.md +++ b/.claude/skills/factory-triage/SKILL.md @@ -62,11 +62,14 @@ Use the `factory-comment` skill's `triage.md` template to write "risk": "low", "done_when": "one sentence, checkable by a command or test", "files_expected": ["src/…"], - "gate_level": "make check", + "gate_level": "full", "confidence": 0.8 } ``` +`outcome` is optional: `complete` (the default), `blocked` (you cannot go on and a human must +look; put the reason in `summary`), or `failed`. No other fields are allowed. + If `disposition` is `needs-info`, also write `.factory/runs/issue-/question-comment.md` using the `question.md` template: at most 3 numbered questions, 2-3 lettered options each, a diff --git a/.claude/skills/factory-verify/SKILL.md b/.claude/skills/factory-verify/SKILL.md index f253025..72fc3f8 100644 --- a/.claude/skills/factory-verify/SKILL.md +++ b/.claude/skills/factory-verify/SKILL.md @@ -11,12 +11,13 @@ the runner posts the verdict and moves the issue's label. ## 1. Read the inputs - `.factory/runs/issue-/issue.json`, `plan.json`, `plan-comment.md`, - `build.json` — the plan's AC-n and NG-n, and what build reports it did, - including build's own `rounds` count. + `build.json` — the plan's AC-n and NG-n, and what build reports it did. +- `.factory/runs/issue-/gate.json`: what the runner measured after build + (`line`, `status`, `tree`). If its `tree` equals `git rev-parse HEAD^{tree}`, + the gate result is current: use it and do not re-run the gates. If the tree + differs, or the file is missing, the evidence is stale: report `uncertain`. - The worktree at its current state (build's commits, uncommitted or not). -- `.factory/runs/issue-/verdict.json`, if present from a prior round, to - read its `rounds` so you increment it, not reset it. The runner tracks - the reject count itself for routing; this field is your own record. +- The runner counts verify rounds itself: write `"rounds": 1` and it replaces the value. ## 2. Run the subagents @@ -30,10 +31,16 @@ Dispatch to `factory-reviewer` (fresh context, read-only): correctness, security (injection, authz, secrets), and whether the diff crosses any NG-n. Collect its findings verbatim; do not soften or drop one. +Then re-check each finding yourself against the diff at the current head. Drop +one the code does not support (confidence 0-2); keep the rest. Never keep a +finding you could not reproduce from the diff. + ## 3. Decide the verdict - **pass** — every AC has evidence, the gate is green, no NG-n crossed, no - blocking reviewer finding. + blocking reviewer finding. A `pass` that lists a `must` or `should` finding + at confidence 3 or more, or a criterion that is not `pass`, is refused by the + runner and goes to a human. - **reject** — any AC unproven, gate red, an NG-n crossed, or a blocking finding. The runner sends the issue back to `factory-build` up to twice; a third reject is routed to a human automatically, so just report @@ -56,10 +63,19 @@ Then write `.factory/runs/issue-/verdict.json`: { "result": "pass", "rounds": 1, - "findings": [] + "findings": [], + "criteria": [{ "id": "AC-1", "status": "pass" }] } ``` +A finding is `{ "severity": "must|should|could", "confidence": 0-5, "what": "...", +"where": "file:line", "why": "...", "fix": "..." }` (`what` is required). A +criterion is `pass`, `fail`, or `unverified` (with a `gap`); AC ids come from the +plan and are never renumbered. No other fields are allowed, and the file must be +one JSON object under 16 KiB. + +Set `outcome` to `blocked` (with a `summary`) only if you could not review at all. + `result` is `pass`, `reject`, or `uncertain`. `findings` is the reviewer's list verbatim (empty array if none). On `reject`, the runner sends the issue back to `factory-build`, up to twice; a third reject, or an