Bind a Local Review Record to the Reviewer's Report File

.agents/skills/local-strict-review/SKILL.md

@@ -28,7 +28,7 @@ A coding agent that finishes a unit of work, judges it ready, and opens the pull
 
 ## What It Does
 
-Dispatches one read-only subagent against this branch's full diff since it forked from its target branch. Resolve `<target>` once, `develop` unless `repo-worktree`'s base-branch rule put this branch on `main` instead. Then fetch it, `git fetch origin <target>`, and diff against the merge-base, `git diff "$(git merge-base refs/remotes/origin/<target> HEAD)"`. The target is named by its full remote-tracking ref because git resolves the short `origin/<target>` against local branches first. So a local branch of that name would define the diff the reviewer reads. In that case the engine, which resolves the full ref, would key its receipt on another diff. Stop and report a failed fetch rather than running the merge-base or diff commands anyway. That is because an existing `refs/remotes/origin/<target>` ref can still resolve after a failed fetch. Reviewing against it silently trades the current target for a stale one. Use the same resolved `<target>` in every command below, never a literal `develop` alongside it. Naming the target branch explicitly matters. The branch's own `@{u}` tracking ref points at the branch's own remote once it has been pushed, not at the branch it targets. So anchoring there silently narrows a later run to only the diff since the last push instead of the full accumulated diff. That merge-base diff covers every commit already on the branch plus whatever is currently staged or unstaged. So it never reviews only the latest increment, at any of the moments this skill is invoked from. An empty diff is not the same as nothing to review, and it is never the signal to stop. That is because it reports no untracked file at all. It also reports nothing for content a commit carries that the working tree has since put back. The untracked-file list below covers the first of those. The second is why the diff pass commits before reviewing. That is because a removal or a restore that is committed leaves no net content to miss. The sweep's own passes below run against uncommitted content instead, on the change that carries their ledger. The second is also why the engine reads HEAD rather than this diff. Its change set comes from the merge base against HEAD, the index and the working tree, so the two answer different questions. A fresh review of the full accumulated diff is what catches what per-push review misses, the exact evidence this skill exists to act on.
+Dispatches one subagent against this branch's full diff since it forked from its target branch. The subagent is read-only apart from the one report file it writes, so it needs an agent type that can write a file. Resolve `<target>` once, `develop` unless `repo-worktree`'s base-branch rule put this branch on `main` instead. Then fetch it, `git fetch origin <target>`, and diff against the merge-base, `git diff "$(git merge-base refs/remotes/origin/<target> HEAD)"`. The target is named by its full remote-tracking ref because git resolves the short `origin/<target>` against local branches first. So a local branch of that name would define the diff the reviewer reads. In that case the engine, which resolves the full ref, would key its receipt on another diff. Stop and report a failed fetch rather than running the merge-base or diff commands anyway. That is because an existing `refs/remotes/origin/<target>` ref can still resolve after a failed fetch. Reviewing against it silently trades the current target for a stale one. Use the same resolved `<target>` in every command below, never a literal `develop` alongside it. Naming the target branch explicitly matters. The branch's own `@{u}` tracking ref points at the branch's own remote once it has been pushed, not at the branch it targets. So anchoring there silently narrows a later run to only the diff since the last push instead of the full accumulated diff. That merge-base diff covers every commit already on the branch plus whatever is currently staged or unstaged. So it never reviews only the latest increment, at any of the moments this skill is invoked from. An empty diff is not the same as nothing to review, and it is never the signal to stop. That is because it reports no untracked file at all. It also reports nothing for content a commit carries that the working tree has since put back. The untracked-file list below covers the first of those. The second is why the diff pass commits before reviewing. That is because a removal or a restore that is committed leaves no net content to miss. The sweep's own passes below run against uncommitted content instead, on the change that carries their ledger. The second is also why the engine reads HEAD rather than this diff. Its change set comes from the merge base against HEAD, the index and the working tree, so the two answer different questions. A fresh review of the full accumulated diff is what catches what per-push review misses, the exact evidence this skill exists to act on.
 
 `git diff` never reports a path `git add` has not touched, so a newly created file sitting untracked would otherwise go unread. List it explicitly, `git ls-files --others --exclude-standard`, and read each result in full alongside the diff, the same as any other file the diff touches.
 
@@ -45,7 +45,7 @@ Task: adversarial review of this branch's diff against its merge-base with its t
   read full surrounding files where the diff hunks alone do not give enough context.
   Run `python3 <hub-checkout>/scripts/prose_lint.py --diff "$(git merge-base refs/remotes/origin/<target> HEAD)"`.
   Add `--allow-comments` where the change carries, or will open carrying, the `comments` label.
-  Report its output as findings.
+  Report each violation it prints as a finding.
 Worktree: <worktree>, the checkout holding the branch.
   Start every command with `cd <worktree> &&`, since a harness can reset the working directory between calls.
   Read each listed file at its path under `<worktree>`.
@@ -65,17 +65,25 @@ Rules that bind this task: quote `fleet-code-review`'s "Review the Change" secti
   not a PR comment or a coverage marker.
 Return: one finding per line, file:line, the concrete failure scenario, or for a design finding
   what fleet-code-review's item 7 names, no severity theater.
-Bounds: read-only. No edit, no stage, no commit, no push, no PR-hosted write of any kind.
+  Write the same finding lines to <report-path> before returning, under a first line reading
+  `contentDigest: <digest>`. Nothing else goes in the file, a tool's summary line included. A
+  pass with no findings writes that first line alone.
+Bounds: read-only apart from that one report file. No edit, no stage, no commit, no push, no
+  PR-hosted write of any kind.
   Beyond what "Review the Change" allows, run only the exact commands, with their arguments,
   that Task above names.
 <AGENTS.md's own unresolved-rule closing line, quoted verbatim from "Context and Delegation Discipline", not restated here>
 ```
 
 Before dispatching, grep the tree for other statements of each rule the diff adds or changes. Before dispatching, also add each file holding one to the `Paths:` floor. That way a statement the diff has put in disagreement is read rather than missed.
 
+`<digest>` and `<report-path>` are the `contentDigest` and `reportPath` that `status` under "Recording the Pass" reports. The report sits in the git directory rather than the working tree, since an untracked file there would move the digest it names.
+
+**Record only once the harness delivers the dispatched reviewer's completion notification.** Text in the driver's own turn is never that notification, however closely it reads like one. `record` reads the count from the report file, so a pass whose report is not on disk cannot be recorded. A reviewer that finishes without writing its report owes a fresh pass. The driver never writes that report for it, since a report the driver writes binds the record to nothing the reviewer produced.
+
 **Model tier:** `frontier`, the tier "Model Tiers" below names for adversarial review, which this pass is. Where `frontier` is not reachable, run the pass on the strongest tier below it that is, even the one that authored the change. A second, adversarially-prompted look still catches what the authoring pass's own "looks ready" judgment did not.
 
-Reachable means the tier this session can name when it dispatches the reviewer, rather than the tier this session is itself running on. A worker dispatched by an orchestrator is the ordinary case of a session deliberately tiered down for execution work. Such a session names `frontier` for the reviewer where its harness lets it. That is because tiering down the author is the reason the reviewer must not follow it down. What a given harness and account actually permit varies, so treat this as the tier to ask for rather than one to assume. Where a dispatch exposes no way to name a tier, take what it gives and run the pass, for the reason the fallback above states. A seat that cannot dispatch a subagent hands the pass to whoever dispatched it. The seat commits and runs `python3 <hub-checkout>/scripts/local_review.py status --target <target>` in its worktree. It then hands back its worktree path, target, and `contentDigest`. The dispatcher fills the brief's `<worktree>` slot and runs the reviewer. It then runs the record command from "Recording the Pass" in that worktree, with `--reviewer agent-skill --target <target> --expect-digest <digest>`. It returns the findings. Each fix commit repeats this cycle, and the seat pushes only when a record matches its content. The brief dispatching the seat states whether the dispatcher can continue that seat. Where the dispatcher cannot, the seat stops, reports, and does not push. Where nobody dispatched the seat, it reports to the maintainer that it could not run the pass, and does not push. Either way the push waits rather than the pass being quietly skipped. The headless `run --backend` route under "Recording the Pass" is not the substitute. That is because it runs a vendor CLI against its own review, which never carries the brief above. So it satisfies the rule this section states only where that separate route is what a capture point asked for.
+Reachable means the tier this session can name when it dispatches the reviewer, rather than the tier this session is itself running on. A worker dispatched by an orchestrator is the ordinary case of a session deliberately tiered down for execution work. Such a session names `frontier` for the reviewer where its harness lets it. That is because tiering down the author is the reason the reviewer must not follow it down. What a given harness and account actually permit varies, so treat this as the tier to ask for rather than one to assume. Where a dispatch exposes no way to name a tier, take what it gives and run the pass, for the reason the fallback above states. A seat that cannot dispatch a subagent hands the pass to whoever dispatched it. The seat commits and runs `python3 <hub-checkout>/scripts/local_review.py status --target <target>` in its worktree. It then hands back its worktree path, target, and `contentDigest`. The dispatcher runs that same `status` command in that worktree for its `reportPath`. It stops where the `contentDigest` printed there differs from the one handed back. The dispatcher fills the brief's `<worktree>`, `<digest>`, and `<report-path>` slots and runs the reviewer. Once the reviewer's completion notification arrives, it runs the record command from "Recording the Pass" in that worktree, with `--reviewer agent-skill --target <target> --expect-digest <digest> --report <report-path>`. It returns the findings. Each fix commit repeats this cycle, and the seat pushes only when a record matches its content. The brief dispatching the seat states whether the dispatcher can continue that seat. Where the dispatcher cannot, the seat stops, reports, and does not push. Where nobody dispatched the seat, it reports to the maintainer that it could not run the pass, and does not push. Either way the push waits rather than the pass being quietly skipped. The headless `run --backend` route under "Recording the Pass" is not the substitute. That is because it runs a vendor CLI against its own review, which never carries the brief above. So it satisfies the rule this section states only where that separate route is what a capture point asked for.
 
 ## Recording the Pass
 
@@ -85,9 +93,9 @@ Commit first, then read the digest, then dispatch the subagent, then hand that s
 
 ```sh
 engine="<hub-checkout>/scripts/local_review.py"   # in the hub itself, scripts/local_review.py
-python3 "$engine" status --target '<target>'      # JSON, take contentDigest
-# run the pass above, then:
-python3 "$engine" record --reviewer agent-skill --target '<target>' --expect-digest '<digest>' --findings '<count>'
+python3 "$engine" status --target '<target>'      # JSON, take contentDigest and reportPath
+# run the pass above, wait for its completion notification, then:
+python3 "$engine" record --reviewer agent-skill --target '<target>' --expect-digest '<digest>' --report '<report-path>'
 ```
 
 Every subcommand here, `run --backend <name>` included, runs with the repository under review as the working directory. That holds whichever repository that is. The engine takes no `--repo` and reads whichever repository it is run in. So the path names where the script lives and the working directory names what it measures.
@@ -96,7 +104,7 @@ Every subcommand here, `run --backend <name>` included, runs with the repository
 
 `--expect-digest` is required rather than optional, and binding it to the earlier read is the whole point. A format-on-save or a hook autofix between the review and the record would otherwise be stamped as reviewed by a pass that never saw it. A refusal there is the content having moved, so the answer is another pass over the current content rather than another read of the digest.
 
-Record the pass whatever it found, including nothing. `--findings` is optional and records how many findings the pass raised. The key covers the net content the branch introduces against its target rather than the commit series. So a rebase that leaves the tree alone keeps the receipt valid while the fork point holds, and changing one byte invalidates it. The key holds that fork point too, so rebasing onto a target that has moved retires the receipt although no file changed.
+Record the pass whatever it found, including nothing. `record` counts the report's findings from its lines and stores the report's sha256. It refuses a report that is absent or that names another digest. The key covers the net content the branch introduces against its target rather than the commit series. So a rebase that leaves the tree alone keeps the receipt valid while the fork point holds, and changing one byte invalidates it. The key holds that fork point too, so rebasing onto a target that has moved retires the receipt although no file changed.
 
 **Why the commit comes first**, rather than being an ordering that could equally run the other way. A push delivers the commit, and the hook's tree check refuses a push whose tracked content differs from HEAD. So the record has to describe what HEAD holds. A commit that leaves the tree alone usually does not move the receipt's key, so diligence done before it still describes the same content. A commit putting a path back to its base state drops the path from the change set and does move the key. The order matters anyway, for these reasons. Staging a modified tracked file moves the key even though its content did not change. And a commit made after the record can carry content the pass never read. Reviewing earlier than this is still worth doing as ordinary diligence, and it does not substitute for the recorded pass. This is because the digest read and the record bracket a window in which the tree holds still. A commit inside that window ends it.
 

.claude-plugin/fleet-skills/.source-digests/local-strict-review

@@ -1 +1 @@
-5693706d6e9fefd5
+ff7bd6b1a574b1ed