pr-review: auto-fix on a reviewed head repairs from our submitted review

agents/chief.md

@@ -58,7 +58,7 @@ Follow-up issue rule: a push grant for the task's PR, the interactive default or
 
 Reply takeover rule: a push grant for the task's PR, the interactive default or a headless PR launch that does not withhold it, also permits finishing on that PR the reply obligations a launch returns in `handoffs`: posting the answer, settling on an adequate reply of ours already there, resolving its thread once the reply is verified, and editing a reply of ours on that comment through `reply.py --edit`, whichever task posted it, to an evidence-backed correction when it is stale or wrong, never left for the user to accept or authorize, or a second answer where `reply.py`'s `secondAnswer` permits one. It permits no delete, no reply to another comment, no review and no SonarCloud action by its units, and a dry run withholds it. Answer only when every point the comment raises as it stands now is fixed on the PR branch, refuted with evidence or deferred by a passed deferral; a valid point not yet fixed is fixed first, by the workflow or by your own writer through `adoptHead`. For all of a launch's handoffs, one `general-purpose` prep unit on `model: sonnet` loads `pr-reply`, works from the launch reading and the threads through `reply.py --inspect`, opening the output file for evidence the reading omits or cuts, and proposes per comment reuse, post, edit or blocked with the exact body and its evidence, posting nothing. One `finding-verifier` then judges, one verdict per comment, each proposal against its whole current comment, except a comment whose prep shows all of: its digest and the PR head are the ones the launch judged, the launch's final findings after its challenger cover every point of the comment with none held, and the body states those dispositions without changing their substance; a missing, duplicate or inconclusive verdict leaves that comment blocked. You then load `pr-reply` and run each `reply.py` operation yourself with the exact operations, bodies and digests, keeping its receipts verbatim. Answer each comment once per digest, and rerun its operation once after an uncertain receipt; an unchanged unresolved one is a blocker. Hand each verified reply to the next launch, a `reviews` or `both` one, as `replySettlements: [{ commentId, commentDigest, replyId, bodyDigest, headSha }]`, `headSha` the PR head its answer was judged on: pr-babysit re-reads both and settles the comment, or names why not in its `settlements`. For a code-scanning comment whose answer fixes or refutes rather than defers, add `sonar: { how, note }`, `how` `fixNote` when any point was fixed, else `refutation`, and `note` the exact reply body: pr-babysit then marks its SonarCloud issue under `markSonar`.
 
-Exception for a headless PR review launch: the human's own message asking pr-review to review a PR with auto-post or auto-fix, carried and accepted under the headless PR launch exception's provenance and bound to its expected head, grants this chief invocation one review on that PR, as `COMMENT` or `REQUEST_CHANGES` with its inline comments and never `APPROVE`, and replies to and resolves of only our own review threads whose fix a recheck verified, solely through pr-review's `post.py --auto`, which you run. A relaunch of the same unfinished review on the same head first reconciles any earlier post by `post.py`'s `uncertain` and `partial` rules, never submitting a second review. A moved head, a finished review or a withdrawal ends it; `post.py` refuses a moved head, which stops publishing. It submits no answer to a contributor's pushback: `post.py --auto`, or under auto-fix a plain `post.py` once the repair ends, puts those in a pending review under the standing exception below. With auto-fix, the same message, under the headless PR launch exception above, also grants pr-babysit on that PR for the repairs of that review's findings and the bots' review, a fix note and a resolve on each of our own threads whose repair that pr-babysit pushed and a fix check verified on the PR head, solely through pr-review's `post.py --fixed`, which you run, and one more review, the recheck on the head those repairs leave, submitted the same way only while `ledger.py show --fixable` allows it, else created pending; a further repair of newly found findings needs a new request. Without auto-fix it permits no push; it never permits approval, dismissal, another PR or issue beyond the follow-up issue rule, and no onward delegation of the quoted message.
+Exception for a headless PR review launch: the human's own message asking pr-review to review a PR with auto-post or auto-fix, carried and accepted under the headless PR launch exception's provenance and bound to its expected head, grants this chief invocation one review on that PR, as `COMMENT` or `REQUEST_CHANGES` with its inline comments and never `APPROVE`, and replies to and resolves of only our own review threads whose fix a recheck verified, solely through pr-review's `post.py --auto`, which you run. A relaunch of the same unfinished review on the same head first reconciles any earlier post by `post.py`'s `uncertain` and `partial` rules, never submitting a second review. A moved head, a finished review or a withdrawal ends it; `post.py` refuses a moved head, which stops publishing. It submits no answer to a contributor's pushback: `post.py --auto`, or under auto-fix a plain `post.py` once the repair ends, puts those in a pending review under the standing exception below. With auto-fix, the same message, under the headless PR launch exception above, also grants pr-babysit on that PR for the repairs of that review's findings, or, on a head already reviewed, of our review submitted on it that prepare's `repair.submittedReview` names, and the bots' review, a fix note and a resolve on each of our own threads whose repair that pr-babysit pushed and a fix check verified on the PR head, solely through pr-review's `post.py --fixed`, which you run, and one more review, the recheck on the head those repairs leave, submitted the same way only while `ledger.py show --fixable` allows it, else created pending; a further repair of newly found findings needs a new request. Without auto-fix it permits no push; it never permits approval, dismissal, another PR or issue beyond the follow-up issue rule, and no onward delegation of the quoted message.
 
 Standing exception for pr-review pending reviews: under the user's standing grant in their instructions, a pr-review task may, with no exchange, create one pending review on the task's PR through pr-review's `post.py`, holding the saved draft's body, inline comments, fix notes and thread answers, and resolve, through `post.py` or `prepare.py`, only our own threads whose reply the human submitted. It never submits, edits or deletes a review or comment, and permits no other publishing.
 

skills/pr-review/SKILL.md

@@ -46,9 +46,11 @@ the mode: `full` on a first review or after a force-push or rebase,
 unchanged, `same` when this head was reviewed already. On `same`, launch in
 `discussion` mode, which only judges new replies on our threads and returns
 `nothing-new` when there are none; `full` reviews the head again instead, saved
-with the request's words as `--reason`. Under `auto-fix`, a discussion run
-repairs nothing, since step 6 of §4 needs a review this run submitted: a repair
-of an already-reviewed head takes `auto-fix full`. An `overCap` answer merged
+with the request's words as `--reason`. Under `auto-fix`, prepare's
+`repair.submittedReview` names our review submitted on this head, the human's
+submit included, and the run repairs from it (§4 step 6) without reviewing the
+head again; a null one repairs nothing, for `repair.submittedWhy`, and
+`auto-fix full` reviews the head again and repairs from that review. An `overCap` answer merged
 everything into one group: report it and launch.
 
 A sibling tree is locked for the run, so worktree-remove's cleanup of the
@@ -111,8 +113,10 @@ since the same head otherwise gets only a discussion run. Under `auto-fix`, also
 the head repository and branch, said plainly to be the contributor's branch for
 a fork, prepare's `repair` state and why (`unknown` is not `unavailable`), and
 that the run reviews, repairs, validates and rechecks the PR within
-pr-babysit's cycle accounting, or, on `same`, that it repairs nothing and
-`auto-fix full` would.
+pr-babysit's cycle accounting. On `same`, say instead that it repairs from
+submitted review `repair.submittedReview` without a whole-head review, with
+the `repair.deferredAnswers` answers waiting for the repair to end, or that it
+repairs nothing, for `submittedWhy`.
 
 ## 3. Publishing
 
@@ -145,8 +149,7 @@ finds still standing is raised again on a new comment, unless someone reopened
 its thread since, which then still states it. A recheck review the
 ledger cannot vouch for (`ledger.py show --fixable`'s `autoPost` false) stays
 pending for the human. One repair round: what the recheck finds new is
-reported, and repairing it needs a new request, with `full` while the head is
-still the rechecked one (§1).
+reported, and repairing it needs a new request (§1).
 
 ## 4. Launch
 
@@ -182,6 +185,11 @@ sequence:
    or {choice: 'boards', boards: [rows]}, hardwareRelevant, autoPost,
    dimensions?}` (`mode: 'discussion'` for prepare's `same`). `dimensions`
    comes from the project's instruction file when it names review dimensions.
+   Under auto-fix with `repair.submittedReview`, steps 3 to 5 are skipped when
+   `repair.state` is `available` or `repair` has `deferredAnswers` (the answers
+   an earlier auto-fix deferred on that review): a discussion cannot change what
+   step 6 repairs, whose findings with pushback stay held, and step 6.4 judges
+   that pushback. Otherwise the discussion runs as without auto-fix.
 4. `result.py --output <the launch's output file>`; when its `status` is
    `reviewed`, `ledger.py save --pr N --output <it>`. Any other status saves
    and posts nothing: step 7 reports it with its reason.
@@ -190,7 +198,8 @@ sequence:
    through a unit, with `--expected-head`; a
    `discussion` run always takes plain `post.py`, since its answers only go
    into a pending review. Its exit 1 names what it could not confirm.
-6. Auto-fix, only once step 5 reports `posted`; a failed, partial, uncertain
+6. Auto-fix, only once step 5 reports `posted`, or on `same` with
+   `repair.submittedReview`, whose head is then the reviewed head; a failed, partial, uncertain
    or handed-over post ends it there. After a handed-over one, its deferred
    answers wait for the human's submit (§5): plain `post.py` refuses them
    until then, so it is not run. Its answers to pushback, `answers`
@@ -245,6 +254,9 @@ sequence:
 7. Report the verdict and its reasons, the counts, coverage lost, the CI and
    HIL rows, the receipts, `post.py`'s `firstLineUnchecked`
    and the result's `heldThreads`; never the draft's text. Under auto-fix also the repair state,
+   whether the repair took this run's review or reused submitted review `repair.submittedReview`,
+   the answers the closing plain `post.py` skipped as stale and the pending review it left, which blocks
+   the next run until the human settles it,
    the held findings, the repair and pr-babysit receipts, the `--fixed`
    receipts, the recheck's post,
    and the review state GitHub shows (`gh pr view N --json
@@ -274,7 +286,7 @@ rules). `post.py` reports `uncertain` or `partial` when it cannot prove what lan
 it never sends again blind, and the next run looks for it; reconcile by hand
 what it names. A `handedOver` auto review is the human's pending review, with
 its reason; once they submit it, `post.py --pr N --expected-head SHA` publishes
-its answers. A thread in `heldThreads` is one we resolved and someone reopened,
+its answers; a new `auto-fix` request instead repairs from it first (§1). A thread in `heldThreads` is one we resolved and someone reopened,
 whose resolve is unconfirmed, or whose repair note was deleted or edited before
 its resolve: it is never resolved again by itself, so name it for the human.
 

skills/pr-review/scripts/prepare.py

@@ -38,7 +38,10 @@
 edits from maintainers, push access to the base; `unavailable` when this account has neither; `unknown` when gh cannot
 tell. Only when available, it binds the review branch to the head branch through a remote of the head repository
 (head_remote, bind_upstream) and adds {remote, addedRemote}; a binding it cannot make refuses. Without --auto-fix
-nothing of this runs.
+nothing of this runs. On a `same` head `repair` also has `submittedReview`, the id of our review submitted on that
+head (by the human included), which the repair takes its findings from, else null with `submittedWhy`; one unsent
+answer bundle an earlier auto-fix deferred on that review no longer refuses the run: `deferredAnswers` counts its
+answers, which wait for the repair to end.
 
 --release unlocks the pr-review-<N> sibling whose lock carries the token, once
 the run that took it has ended; nothing locked by it is a no-op, and while
@@ -63,7 +66,8 @@
 from pathlib import Path
 
 sys.path.insert(0, str(Path(__file__).resolve().parent))
-from ledger import ONLINE, last, ledger_dir, ledger_path, load, locked, publications, refuse_unsettled, repo_of, store  # noqa: E402
+from ledger import (ONLINE, last, ledger_dir, ledger_path, load, locked, publications, refuse_unsettled, repo_of, store,  # noqa: E402
+                    submitted_last)
 from post import sync  # noqa: E402
 
 sys.path.insert(0, str(Path(__file__).resolve().parents[2] / 'pr-babysit' / 'scripts'))
@@ -223,6 +227,21 @@ def repair_target(repo, head_repo, info, branch, head):
     return {'state': state, 'why': why, 'remote': remote, 'addedRemote': added}
 
 
+def reused_review(led, head):
+    """repair's submittedReview or submittedWhy, and deferredAnswers when the one pending publication on `head` is
+    an unsent bundle an auto-fix deferred on that review (post.py --defer-answers); any other pending draft refuses."""
+    try:
+        rev = submitted_last(led, head)
+    except Unusable as e:
+        return {'submittedReview': None, 'submittedWhy': str(e)}
+    reuse = {'submittedReview': rev['receipts']['review'].get('reviewId')}
+    pending = [p for p, _ in publications(led) if p['head'] == head and p['status'] == 'pending']
+    one = pending[0] if len(pending) == 1 else None
+    if one and one.get('auto') and not one.get('receipts') and any(one is p for p in rev.get('answerPublications', [])):
+        reuse['deferredAnswers'] = len(one['answers'])
+    return reuse
+
+
 def remote_for(repo):
     """The remote whose URL is this repository: fetching another one would pin someone else's refs."""
     names = []
@@ -396,7 +415,6 @@ def prepare(a):
     with locked(path):
         led = load(path, repo, a.pr)
         synced = sync(repo, a.pr, led, lambda: store(path, led))
-        refuse_unsettled(led, head, ('pending',))
         online = [r for r, _ in publications(led) if r['status'] in ONLINE]
         if online and online[-1]['status'] == 'drafted':
             raise Unusable(f"your pending review of {online[-1]['head']} is still open on the PR: submit or delete it on GitHub first")
@@ -405,8 +423,11 @@ def prepare(a):
             raise Unusable(f"a review POST for {online[-1]['head']} may have landed unseen: reconcile it first with "
                            f"post.py --pr {a.pr} --expected-head {online[-1]['head']}{auto}, which only looks for it by its marker")
         mode, scope_base, prior = mode_of(led, head, merge_base, a.full)
+        reuse = reused_review(led, head) if a.auto_fix and mode == 'same' else {}
+        if 'deferredAnswers' not in reuse:
+            refuse_unsettled(led, head, ('pending',))
         wt, branch = worktree(top, a.pr, head, token)
-        repair = repair_target(repo, head_repo_of(info), info, branch, head) if a.auto_fix else None
+        repair = {**repair_target(repo, head_repo_of(info), info, branch, head), **reuse} if a.auto_fix else None
         pr_paths = changed(merge_base, head)
         scope = pr_paths if mode != 'incremental' else changed(scope_base, head)
         groups, over = groups_of(scope, a.max_groups)

tests/test_agents.py

@@ -189,6 +189,7 @@ def test_headless_pr_exception_is_bounded(self):
                        'Without auto-fix it permits no push; it never permits approval, dismissal, another PR or issue beyond the follow-up issue rule'):
             self.assertIn(phrase, review)
         self.assertIn('their request that pr-review review a PR with auto-fix', exception)
+        self.assertIn("of our review submitted on it that prepare's `repair.submittedReview` names", body)
         self.assertIn('An auto-fix repair phase is writer work: its repairs get the commit check, one simplification pass, validation '
                       'and a whole-task review from the reviewed head to the repair tip before pr-babysit adopts them', body)
         self.assertNotIn('fresh exchange', review)
@@ -203,7 +204,6 @@ def test_headless_pr_exception_is_bounded(self):
                        'a failed, partial, uncertain or handed-over post ends it there',
                        'report "review submitted; auto-fix blocked"', 'never a `held` one',
                        'A crash with repairs unpublished resumes this task with them, never a new prepare',
-                       'Under `auto-fix`, a discussion run repairs nothing', 'a repair of an already-reviewed head takes `auto-fix full`',
                        '`post.py --auto --defer-answers` (auto-fix)',
                        'whatever becomes of the recheck review; one the recheck finds still standing is raised again on a new comment, '
                        'unless someone reopened its thread since, which then still states it',