diff --git a/CHANGELOG.md b/CHANGELOG.md index 3831cda..897d011 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,25 @@ summary: Chronological history of repository and skill changes. # Changelog -## 2026-07-31 — Added the review-fix-loop cross-cutting evaluation corpus, recorded the first review-fix-loop `update_pr` fix cycle +## 2026-07-31 — Packaged and documented the standalone review-fix-loop skill, added the review-fix-loop cross-cutting evaluation corpus, recorded the first review-fix-loop `update_pr` fix cycle +- feat(review-fix-loop): package and document the standalone skill for discovery + (issue #102, epic #95, the epic's final child) — list `skills/review-fix-loop` + in the README's "Current reusable agent skills" section with its + `local_commit`/`update_pr` policies and the tracked #103/#104/#105 + caller-migration follow-ups, correct the eight-skills count to nine, and wire + its `just test-review-fix-loop`/`just eval-review-fix-loop` targets into the + Quick Start and evaluation sections; add `review-fix-loop` to + `scripts/validate_plugins.py`'s `REQUIRED_SKILLS` set so a missing install or + a missing `agents/openai.yaml` fails plugin packaging validation in CI, + matching every other repository-owned skill; refresh + `agents/claude-code.md`/`agents/openai.yaml`, stale since issue #98, to + describe the `local_commit`/`update_pr` workflows issues #99-#101 actually + delivered instead of only document validation and one review pass; and add a + "Publication policy and retained commits" section to `SKILL.md` itself stating + that both workflows keep fixes local until convergence, that `update_pr` + publishes exactly once, and that every non-converged terminal result reports + its retained unpushed commits via `unpushed_commits`/`operator_action` - fix(review-fix-loop): configure `user.email`/`user.name` on both git clones `scripts/evals/corpus.py`'s `up_sequential_publication_race_second_clone_loses` scenario creates, @@ -16,7 +33,7 @@ summary: Chronological history of repository and skill changes. with "Author identity unknown" in GitHub Actions even though every local run passed; reproduced the exact CI failure locally under a forced no-identity condition, confirmed the fix resolves it, and confirmed GitHub Actions' own - `ci` check on PR #113 is green + `ci` check on PR #113 is green (`a44fc2f3397349ca4d38ad7456dc97b95bba0648`) - fix(review-fix-loop): consolidate `scripts/evals/helpers.py`'s five fixtures that were byte-identical or functionally identical to `scripts/tests/helpers.py`'s own (`init_repo`, `CLEAN_TEMPLATE`, diff --git a/README.md b/README.md index 654751a..e826c60 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ A personal monorepo for agent skills and supporting scripts. ## Installation -Install the plugin when you need any composed workflow. It packages all eight +Install the plugin when you need any composed workflow. It packages all nine skills together so stable-name dependencies such as `implement-ticket`, `review-code-change`, and `babysit-pr` are available in the same fresh session. @@ -84,6 +84,18 @@ Current reusable agent skills: is not justified by real requirements or repository constraints - `skills/review-code-simplicity` — reduce local cognitive load through behavior-preserving reuse, DRY, control-flow, and test simplification +- `skills/review-fix-loop` — take cooperative ownership of an existing committed + candidate, run the complete repository review suite in a fresh read-only + subagent by default, apply ticket-scoped fixes in isolated attempt worktrees, + and repeat until review converges or a bounded stop condition is reached; + supports a `local_commit` policy (every fix stays local — the operator + publishes them separately) and an `update_pr` policy (one expected-old, + fast-forward-only Git push immediately after convergence). Standalone today: + no existing caller skill invokes it yet — see + [issues #103](https://github.com/shaug/agent-scripts/issues/103), + [#104](https://github.com/shaug/agent-scripts/issues/104), and + [#105](https://github.com/shaug/agent-scripts/issues/105) for the tracked + `implement-ticket`/`babysit-pr`/`carve-changesets` migration follow-ups. The composed implementation dependency chain is: @@ -135,8 +147,10 @@ just test-review-suite just test-babysit-pr just test-implement-ticket just test-implement-epic +just test-review-fix-loop just eval-implement-ticket just eval-implement-epic +just eval-review-fix-loop ``` Validate a review packet and result together: @@ -151,6 +165,7 @@ Run deterministic local evaluation harnesses without an agent runtime: just eval-carve-changesets just eval-implement-ticket just eval-implement-epic +just eval-review-fix-loop ``` The carve-changesets command first runs its objective integration self-test, @@ -180,6 +195,13 @@ runtime, pass its stdin/stdout JSON adapter through just eval-implement-ticket-claude ``` +`just eval-review-fix-loop` drives `review-fix-loop`'s own cross-cutting, +result-blind corpus: twenty scenarios that run the real `local_commit`/ +`update_pr` engine against disposable Git repositories with scripted +reviewer/decide/apply_fix fixtures (no subprocess boundary, no model call, no +network) and grade the result against independently derived Git evidence. See +[its README](skills/review-fix-loop/evals/README.md) for the corpus contract. + ### Result-blind review replay evaluation `review-suite/evals/` is the canonical evaluator that measures what the review diff --git a/scripts/validate_plugins.py b/scripts/validate_plugins.py index 8ae1aae..d1fa24b 100644 --- a/scripts/validate_plugins.py +++ b/scripts/validate_plugins.py @@ -19,6 +19,7 @@ "review-code-change", "review-code-simplicity", "review-correctness", + "review-fix-loop", "review-solution-simplicity", } REVIEW_SKILLS = { diff --git a/skills/review-fix-loop/SKILL.md b/skills/review-fix-loop/SKILL.md index 3bc4fb6..9f22d9c 100644 --- a/skills/review-fix-loop/SKILL.md +++ b/skills/review-fix-loop/SKILL.md @@ -280,6 +280,33 @@ unreachable remote, the remote-target lock actually being exercised through `run_update_pr`, and rejection of an invalid invocation or a `local_commit` invocation at the API boundary. +## Publication policy and retained commits + +Both standalone workflows keep every intermediate fix commit strictly local +until the aggregate review converges. Neither workflow ever publishes a fix as +it is made; there is no partial-publication path. + +- `local_commit` never writes to a remote at all, converged or not. A converged + result reports `publication.status: not_applicable` and lists every commit it + made in `unpushed_commits` — this is the expected, non-error shape of success, + and `operator_action` names how the operator publishes those commits through + their own workflow. +- `update_pr` publishes exactly once, only in the instant the aggregate review + first comes back clean, via one expected-old, fast-forward-only Git push (see + [Run the standalone `update_pr` workflow](#run-the-standalone-update_pr-workflow) + above). Before that instant, and on every non-`converged` exit + (`changes_remaining` or `blocked`, including `remote_advanced` and + `publication_failed`), the commits this invocation made remain local and + unpushed. +- **Every non-converged terminal result — under either policy — reports its + retained, unpushed commits explicitly in `unpushed_commits` and names the + concrete next step in `operator_action`.** A caller must not assume "no error" + means "nothing to do": read `terminal_state`, `unpushed_commits`, and + `operator_action` together before treating an invocation as finished, and see + [`references/CONTRACT.md`](references/CONTRACT.md)'s "Terminal result" section + for the complete per-state publication and retained-commit rules the validator + enforces. + ## Evaluation [`evals/README.md`](evals/README.md) documents the cross-cutting, result-blind diff --git a/skills/review-fix-loop/agents/claude-code.md b/skills/review-fix-loop/agents/claude-code.md index 7b3f426..dc5b1b1 100644 --- a/skills/review-fix-loop/agents/claude-code.md +++ b/skills/review-fix-loop/agents/claude-code.md @@ -4,9 +4,22 @@ Optional discovery metadata for Claude Code and Claude Agent SDK runtimes. It does not constrain the skill's portable contract. - Display name: Review Fix Loop. -- Suggested prompt: "Use the review-fix-loop skill to validate this - invocation/checkpoint/terminal-result document, or to run and record one - complete review pass for the current candidate." +- Suggested prompt: "Use the review-fix-loop skill to run the standalone + review/fix/converge loop for this committed candidate — `local_commit` if + fixes should stay local for me to publish myself, or `update_pr` if it should + push once, immediately after convergence — or to validate this + invocation/checkpoint/terminal-result document, or to run and record just one + complete review pass." +- Standalone workflows: `scripts/local_commit.py`'s `run_local_commit(...)` and + `scripts/update_pr.py`'s `run_update_pr(...)` are the two full end-to-end + entry points (see + [`references/local-commit.md`](../references/local-commit.md) and + [`references/update-pr.md`](../references/update-pr.md)). Every intermediate + fix commit stays local under both policies; only `update_pr` publishes, and + only once, immediately after the aggregate review comes back clean. A + non-`converged` terminal result always reports its retained, unpushed commits + in `unpushed_commits` plus an `operator_action` naming what the operator must + do next — never silently drop them. - Fresh read-only review context: invoke repository-owned `review-code-change` in a subagent (Agent tool) restricted to `Read, Grep, Glob, Bash, Agent, Task, Skill` — no `Edit`, `Write`, diff --git a/skills/review-fix-loop/agents/openai.yaml b/skills/review-fix-loop/agents/openai.yaml index 1bc1c14..43b66cc 100644 --- a/skills/review-fix-loop/agents/openai.yaml +++ b/skills/review-fix-loop/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "Review Fix Loop" - short_description: "Validate review-fix-loop documents and run one complete review pass" - default_prompt: "Use $review-fix-loop to validate this invocation/checkpoint/terminal-result document, or to run and record one complete review pass for the current candidate." + short_description: "Run the standalone review/fix/converge loop (local_commit or update_pr) for a committed candidate" + default_prompt: "Use $review-fix-loop to run the standalone review/fix/converge loop for this committed candidate under local_commit or update_pr, or to validate this invocation/checkpoint/terminal-result document, or to run and record just one complete review pass."