Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .agents/skills/agent-conduct/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Read `GOVERNANCE.md` "Verification Discipline" before reporting success on anyth
- **A `raw.githubusercontent.com` 404 does not distinguish a private repository from a missing file.** Where visibility is not confirmed public, read content via `gh api "repos/<owner>/<repo>/contents/<path>?ref=<ref>"`, capturing the result before decoding it (`content=$(gh api ... --jq '.content') && printf '%s' "$content" | base64 -d`) rather than piping straight into `base64 -d`, whose own exit status is all a direct pipe reports, letting a failed fetch decode as an empty success. Never `2>&1` either form, which corrupts the decode with the error text instead of the payload. Verify the ref resolves before reading either failure as proof the content itself does not exist.
- **A test asserts the mechanism it names, and a gate has to be watched failing.** A case that passes for an incidental reason is worse than no case, because it is later cited as evidence.
- **Platform-specific code is verified only on the platform it runs on.** Reasoning about PowerShell, macOS, or WSL-specific behavior from a different host is not verification, however closely it matches an already-tested equivalent elsewhere. State an untested structural match as exactly that, never in the words used for a tested fact, and when no agent in the loop has access to the target platform, say so and defer or ship it labeled unverified.
- **PR-bound work runs `local-strict-review` before the claim.** Claiming a unit of work done, verified, green, or fixed for work that will become, or already is, a pull request means running `local-strict-review` against the branch's diff first, before a PR-hosted reviewer finds the same gap.
- **PR-bound work runs `local-strict-review` before the claim, and records the pass.** Claiming a unit of work done, verified, green, or fixed for work that will become, or already is, a pull request means running `local-strict-review` against the branch's diff first, before a PR-hosted reviewer finds the same gap, and recording that pass with `scripts/local_review.py` so a capture point can check it. Every push toward a pull request owes one, the fix pushes answering review findings included, which is the round it is most often skipped on.

Claims about a pull request being reviewed, clean, or mergeable are owned by the `pr-review-conduct` skill, and claims that a commit landed by `git-commit-conventions`.

Expand Down
12 changes: 8 additions & 4 deletions .agents/skills/drive-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent.
## The Drive Loop

1. Isolate into a worktree per repo-worktree, based on develop, before the first edit.
2. Run `local-strict-review` against the branch's current diff, then push the branch and open
the feature -> develop PR if it does not exist yet.
2. Commit the work, run `local-strict-review` and record its pass in the order that skill gives,
then push the branch and open the feature -> develop PR if it does not exist yet. A push the
`pre-push` hook refuses is that gate working rather than an obstacle to route around, and that
Comment thread
coderabbitai[bot] marked this conversation as resolved.
skill's refusal table says what each refusal means and what clears it.
3. Drive pr-review-conduct's review loop on it to the Merge Gate, disposing of every finding per
"Disposing of Every Finding" below.
4. Capture the branch's own tip before merging, `gh pr view [number] --json headRefOid --jq
Expand Down Expand Up @@ -102,8 +104,10 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent.

pr-review-conduct's five outcomes are the actual rule, this is the mapping to use while driving:

- Real, so fix it. Run `local-strict-review` against the branch's current diff, push it, reply
with its commit SHA (outcome 1).
- Real, so fix it, then step 2's own order again before replying with the fixing commit SHA
(outcome 1). This is the round the pass is most often skipped on, since the fix looks small and
the branch was already reviewed once, and a fix push carries content no pass has read exactly as
the first push did.
- Not real, or real but out of scope here, so decline in the thread with evidence: the command
and its output, the code path, or the rule that governs it. An assertion never closes a finding
on its own (outcome 2).
Expand Down
43 changes: 41 additions & 2 deletions .agents/skills/local-strict-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,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 origin/<target> HEAD)"`. Stop and report a failed fetch rather than running the merge-base or diff commands anyway: an existing local `origin/<target>` ref can still resolve after a failed fetch, and 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 is never empty and never reviews only the latest increment, at any of the moments this skill is invoked from. 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 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 origin/<target> HEAD)"`. Stop and report a failed fetch rather than running the merge-base or diff commands anyway: an existing local `origin/<target>` ref can still resolve after a failed fetch, and 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: it reports no untracked file at all, and it 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 this skill commits before reviewing, since a removal or a restore that is committed leaves no net content to miss, and why the engine reads HEAD rather than this diff, its change set coming 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.

Expand Down Expand Up @@ -53,19 +53,58 @@ Bounds: read-only. No edit, no stage, no commit, no push, no PR-hosted write of

**Model tier:** the strongest tier this session can reach, per `AGENTS.md` "Match the model tier to the judgment" and "Never tier down the seat holding the judgment", applied here to the reviewer rather than the author. Run the pass on the same tier that authored the change when only one tier is reachable, a second, adversarially-prompted look still catches what the authoring pass's own "looks ready" judgment did not.

## Recording the Pass

`scripts/local_review.py` is what makes this rule checkable rather than something each session has to remember. It holds no review logic: the pass above is the review, and the engine records that it happened, keyed on the content the reviewer actually saw. That receipt is what a capture point reads, this repository's `.husky/pre-push` hook being the first of them.

Commit first, then read the digest, then dispatch the subagent, then hand that same value back. Nothing may change the tree between the read and the record. Staging a modified tracked file is such a change, moving the digest although the content did not, and a commit can move it too, since HEAD decides which paths are in the change set at all. Reading after the commit is what leaves neither of them between the read and the record.

```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 N]
```

Both commands run with the repository under review as the working directory, 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.

`<target>` is the same branch "What It Does" resolved for the review, passed to both commands. Leaving it off defaults them to `develop`, and on a `main`-based branch that computes the digest against a merge base the reviewer never read, so the receipt would attest to a change set nobody looked at. A receipt is only valid against the target it names, so the two have to agree.

`--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. The key covers the net content the branch introduces against its target rather than the commit series, so an interactive rebase that leaves the tree alone keeps the receipt valid, and changing one byte invalidates it.

**Why the commit comes first**, rather than being an ordering that could equally run the other way. A push delivers the commit, so a receipt recorded over uncommitted work describes something else, and a capture point gating a push refuses on exactly that. Two smaller reasons point the same way: 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: the digest read and the record bracket a window in which the tree holds still, and a commit inside that window ends it.

The engine is hub-hosted per `GOVERNANCE.md` "Hub-Hosted Tooling", so a downstream repository reaches a hub checkout's copy rather than carrying one, which is what the path above is for.

## Disposing of Findings

Every finding maps to one of `pr-review-conduct`'s five outcomes before the pull request opens: fixed, evidence-disproven, filed as a deferred issue, escalated to the maintainer for an explicit call, or, if it keeps recurring, taken as a signal to fix the class. A finding this pass raised and not fixed is never the agent's own call to just leave. Per outcome 3, that decision needs the maintainer's explicit answer, the same way a PR-hosted finding would. Running this pass is expected before every push toward a pull request, per `agent-conduct`. Its findings stay advisory: a finding it raises does not by itself block `git commit` or `gh pr create`, the disposition above is what closes it, the same posture local lint holds today. It posts nothing to GitHub, it only reports to the session driving the work. A finding raised here and not fixed is not thereby resolved: the same finding shape reaching a PR-hosted reviewer later still gets its own fresh disposition, per `pr-review-conduct`'s "a disposition decided on one PR does not carry to the next."
Every finding maps to one of `pr-review-conduct`'s five outcomes before the pull request opens: fixed, evidence-disproven, filed as a deferred issue, escalated to the maintainer for an explicit call, or, if it keeps recurring, taken as a signal to fix the class. A finding this pass raised and not fixed is never the agent's own call to just leave. Per outcome 3, that decision needs the maintainer's explicit answer, the same way a PR-hosted finding would. Running this pass is required before every push toward a pull request, per `agent-conduct`. Two claims sit next to each other here and they point opposite ways, so they are stated apart rather than in one sentence. **The pass is mandatory**, and where a capture point enforces it, a push carrying content no recorded pass covers is refused. That refusal is the gate working rather than a fault to route around. **The findings stay advisory**, and the count a pass raises gates nothing at all, since a pass records that a review ran and never that the content is clean. The disposition above is what closes each finding, the same posture local lint holds today. It posts nothing to GitHub, it only reports to the session driving the work. A finding raised here and not fixed is not thereby resolved: the same finding shape reaching a PR-hosted reviewer later still gets its own fresh disposition, per `pr-review-conduct`'s "a disposition decided on one PR does not carry to the next."

## When to Run It

- Before the first push toward a pull request (`drive-pr`'s Drive Loop step 2, `pr-review-conduct`'s Expected review loop step 1).
- Before pushing a fix for a reviewer finding, the same self-review blind spot applies to a fix as to the original diff (`drive-pr`'s "Disposing of Every Finding", `pr-review-conduct`'s outcome 1).
- Whenever `agent-conduct`'s "about to claim work is done, verified, green, or fixed" trigger fires for work that will become, or already is, a pull request.

This repository's `.husky/pre-push` hook checks the receipt at the push itself, so the moments above are where the pass is run rather than the only places it is noticed. A blocked push usually means one of them was skipped. The hook is a backstop under this skill and not a replacement for it: it fires only in a clone that enabled `core.hooksPath`, it says nothing about a repository that carries no such hook, and it is bypassable by design, `--no-verify` being the documented route for a genuine pickle rather than for a diff nobody read.

**Not every refusal is a missing pass, and re-running the pass at one of these does nothing.** Read the refusal itself, which names its own case. Some of these the hook decides before the engine runs, so there is no engine message under them, and the rows below say where each one's detail comes from.

| The refusal says | What it means | What clears it |
| --- | --- | --- |
| Tracked content differs from HEAD | A push delivers HEAD while a receipt covers the index and working tree, so the receipt does not describe this push | Commit what is being pushed, then the pass, then the record |
| The commit is not this worktree's HEAD | `git push origin some-other-branch` from a checkout sitting elsewhere, and the engine reads the checkout it runs in | Check the pushed branch out in its own worktree, per `repo-worktree` |
| Any wording saying the gate did not or could not run | An execution boundary rather than a verdict, which blocks because a gate that waves a push through when it could not run has stopped gating. The cause is named in that same message or in the engine error printed above it, and it is a missing Python interpreter, an unresolvable target, an unreadable receipt, a git command that failed, or any unexpected failure | Whatever the message names, most often installing an interpreter per `docs/host-setup.md` or fetching the target branch. Never another pass |
| A recorded pass names a branch the check did not measure | The hook reads `develop` and nothing else, so a branch based elsewhere is measured against `develop` whatever the pass targeted, and the engine deliberately prints no record command, since the one it would print records a pass over a diff nobody read | One more pass against the branch this work actually targets, where it does target the measured one. Where it does not, the gate cannot judge the branch at all and the bypass is its answer |

This table is the fleet's one enumeration of these, and every other surface states the principle and routes here rather than listing shapes or counting them. That is deliberate: through this skill's own review the count went from two to four, and every round left at least one restatement behind.

## Mechanics Live Elsewhere

- Review criteria: `code-review`.
- Delegation shape and model-tier discipline: `AGENTS.md` "Context and Delegation Discipline".
- Branch base rule (`develop` unless the task is explicitly `main`-only): `repo-worktree`.
- Finding disposition once a pull request exists, the Merge Gate, `scripts/pr_review.py`: `pr-review-conduct`, `drive-pr`.
- The receipt's key, its backends, and the three-valued exit contract a capture point folds: `scripts/README.md` "`local_review.py`".
8 changes: 5 additions & 3 deletions .agents/skills/pr-review-conduct/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ that says only "open a PR" is not such an instruction.
Run every `scripts/pr_review.py` command below from a hub checkout. The script is hosted there and
is never carried into a downstream repository.

Run `local-strict-review` against the branch's current diff before step 1's push, and again before any fix push under outcome 1 below.
Run `local-strict-review` against the branch's current diff before step 1's push, and again before any fix push under outcome 1 below. Follow that skill's own ordering and record each pass, which is what a capture point such as this repository's `pre-push` hook reads. A push that hook refuses is the gate working rather than an obstacle to route around, and that skill's refusal table says what each refusal means and what clears it.

1. Push changes to the PR branch and open the pull request when it does not exist.
2. Run `scripts/pr_review.py status` once in the foreground and read its output.
Expand All @@ -105,8 +105,10 @@ After an authorized merge, run the `repo-worktree` post-merge cleanup procedure

## Every finding ends in one of five outcomes

1. **Real, so fix it.** Run `local-strict-review` against the branch's current diff before pushing
the fix, then reply with the fixing commit SHA. For a finding on platform-specific code
1. **Real, so fix it.** Take the fix through `local-strict-review` the same way step 1's push
went, then reply with the fixing commit SHA. A branch already reviewed once
has not been reviewed for the fix, which is the round this gets dropped on and the churn
`local-strict-review` exists to stop. For a finding on platform-specific code
(PowerShell, a macOS- or WSL-only path), "fixed" means executed on that platform, per
`agent-conduct` "Before Claiming Done": a fix reasoned out by analogy to a tested equivalent
elsewhere is not yet fixed, and the reply says so rather than claiming the SHA closes it.
Expand Down
2 changes: 1 addition & 1 deletion .claude-plugin/fleet-skills/.source-digest
Original file line number Diff line number Diff line change
@@ -1 +1 @@
76568e633c063da3
438bd561ef5f233b
Loading