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
39 changes: 21 additions & 18 deletions .agents/skills/backlog-burndown/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ it was invoked for is narrower than the rule requires, deliberately.
- **The grant is bounded by the session it was named in.** A run interrupted and resumed in a new
session needs the skill named again, which costs one sentence and is the difference between a
grant and a mode. A grant read back from a note is one nobody gave.
- The grant does not weaken the `pr-review-conduct` Merge Gate. It answers that gate's item 5 for
- The grant does not weaken the `pr-review-conduct` Merge Gate. It answers that gate's explicit-permission item for
this run's feature -> develop merges and nothing else, so a pull request with one open finding
still does not merge.
Comment thread
ptr727 marked this conversation as resolved.
- It is never authorization to merge a develop -> main promotion pull request, to dispatch a
Expand Down Expand Up @@ -210,15 +210,16 @@ Brief on `AGENTS.md` "Context and Delegation Discipline"'s subagent shape.
- **The worker drives its group to a develop merge**, by invoking `drive-pr` with the target
stated as develop only. That skill owns the review loop, the finding disposition, and the merge,
so brief the group and the bounds rather than restating the loop.
- **The worker creates its own worktree**, always, as `drive-pr` step 1 and `repo-worktree`'s
task-start mandate already require of the task itself. No worker inherits another's worktree,
- **The worker creates its own worktree**, always, as `drive-pr`'s worktree isolation and
`repo-worktree`'s task-start mandate already require of the task itself. No worker inherits another's worktree,
which is why "Bounding the Wait on a Worker" either removes a dead worker's tree and its
branch or leaves that tree untouched for the maintainer, and never passes it on.
- **The worker does no cleanup**, which is this skill's one stated override of `drive-pr` step 4
and of `repo-worktree`'s post-merge procedure. Say so in the brief, because a worker following
either alone will clean up. The worker still performs step 4's merge itself, and what the override
moves is that step's two cleanup halves, the worktree procedure and the verify-then-delete of the
merged remote branch, **both** rather than only the first. "Cleanup Is the Orchestrator's" below, in this
- **The worker does no cleanup**, which is this skill's one stated override of `drive-pr`'s
post-merge cleanup and of `repo-worktree`'s post-merge procedure. Say so in the brief, because
a worker following either alone will clean up. The worker still performs the merge itself, and
what the override moves is the two cleanup halves `drive-pr` runs after it, the worktree
procedure and the verify-then-delete of the merged remote branch, **both** rather than only the
first. "Cleanup Is the Orchestrator's" below, in this
same section, says why and what it covers.
- **The worker runs `local-strict-review` before every push**, including one that only fixes a
review finding. That pass dispatches a reviewer of its own, so a harness where a subagent cannot
Expand Down Expand Up @@ -260,8 +261,8 @@ finish the procedure from inside its own worktree, since removing that worktree
working directory in which to delete its branch.

So the whole procedure moves to the orchestrator, which runs it from the base clone at the round's
cleanup step, while no worker is live in a tree it touches. It carries `drive-pr` step 4's remote
half too, verifying the merged branch's tip against the pull request's `headRefOid` before
cleanup step, while no worker is live in a tree it touches. It carries the remote half of `drive-pr`'s
post-merge cleanup too, verifying the merged branch's tip against the pull request's `headRefOid` before
`git push origin --delete`, since taking that step from the worker without naming a new owner
would leave a live remote branch behind every group. It covers every group that is done with its tree,
which is the finished ones **and the abandoned ones**: a group told to abandon its branch keeps a
Expand Down Expand Up @@ -398,11 +399,13 @@ has carried.
can still owe a promotion pull request, for work an earlier round landed and no promotion has
yet carried. A count of zero is the only case with nothing to promote, and the round reports
that instead of attempting one.
2. Drive its review loop per `drive-pr` steps 5 through 8, **with a review-round budget set before
the first one**, the same discipline "Bounding a Prose Group" applies to a feature branch. That
loop repeats until the promotion pull request carries no open finding, and nothing in it
terminates on its own, so when the budget is reached, stop and put the state to the maintainer
rather than continuing to spend the run's only forward gear on one pull request.
2. Drive its review loop per the promotion half of `drive-pr` "The Drive Loop", **with a
review-round budget set before the first one**, the same discipline "Bounding a Prose Group"
applies to a feature branch. That loop repeats until the promotion pull request meets every
`pr-review-conduct` Merge Gate item except the maintainer's explicit permission to merge, and
nothing in that loop terminates on its own, so when the budget is reached, stop and put the
state to the maintainer rather than continuing to spend the run's only forward gear on one pull
request.
3. Put the ready pull request to the maintainer through the interface's own prompt mechanism,
naming the merge as the action that unblocks the run. The maintainer's merge is the run's clock, so one
reported in a closing paragraph and never actually asked about stalls every round behind it.
Expand All @@ -411,8 +414,8 @@ has carried.
it lands as its own feature -> develop pass, and that landing moving its head is expected, since
its head **is** develop. **That pass is dispatched as a worker like any other**, which is the
one push the freeze permits and the reason the orchestrator still opens no branch of its own.
`drive-pr` step 6 sends the seat driving a promotion pull request back through its own steps 1
to 4 for such a fix, and here that seat dispatches rather than drives it.
`drive-pr` "The Drive Loop" sends the seat driving a promotion pull request back through its
own feature -> develop pass for such a fix, and here that seat dispatches rather than drives it.
5. **A promotion fix outranks any file claim.** A group holding a file it needs yields, because the
promotion pull request is what the whole run is queued behind. A holder that is merely parked
yields by handing the file over. A holder that already pushed and has an open pull request
Expand All @@ -430,7 +433,7 @@ has carried.
6. **Nothing else pushes, and nothing else is dispatched.** The promotion fix of step 4 is the one
exception to both, and everything in this step is said of the next round's work rather than of
it. That round's preparation is orchestrator work and continues: rank, group, and verify claims.
Its dispatch waits, because a worker has exactly one procedure, `drive-pr`, whose second step
Its dispatch waits, because a worker has exactly one procedure, `drive-pr`, which
pushes and opens a pull request, so a next-round worker dispatched under the freeze would either
break it or sit in a state that procedure does not describe. None is left running across the
wait either, since a worker held idle for an unbounded maintainer wait is one doing nothing at a
Expand Down
83 changes: 54 additions & 29 deletions .agents/skills/drive-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@
name: drive-pr
description: >-
Drives a ptr727/ProjectTemplate fleet pull request through its review loop, feature branch into
develop and, when asked, on to a mergeable develop -> main promotion PR, applying the
pr-review-conduct disposition to every reviewer finding along the way: fix it, decline it with
evidence, defer it behind a filed issue, or put the call to the maintainer and wait for an
explicit answer in the same turn, escalating to whoever dispatched the drive instead where the
maintainer cannot be reached from that seat. Use this whenever asked to drive, land, take, chase, or push
develop and, when asked, on to a mergeable develop -> main promotion PR, disposing of every
reviewer finding along the way under pr-review-conduct's outcomes, carried here whole as a
generated include, and escalating to whoever dispatched the drive where the drive's own seat
cannot reach the maintainer. Use this whenever asked to drive, land, take, chase, or push
a PR toward develop or main, or to run the review loop hands off instead of narrating each
round. When the request does not say how far ("drive this PR", "land it"), ask once whether the
target is develop or a mergeable main promotion PR, rather than guessing. Triggers even when
only one PR is named, because a finding raised against the develop -> main promotion PR
routinely needs its own feature -> develop fix cycle before the promotion PR can go green, and
stopping at the first promotion-PR finding is the early exit this skill exists to prevent. Ends
at develop merged, or at a promotion PR meeting the pr-review-conduct Merge Gate, never merges
main itself, that is the separate merge-and-release skill, its own go-ahead.
at develop merged, or at a promotion PR meeting every pr-review-conduct Merge Gate item except
the maintainer's explicit permission to merge, never merges main itself, that is the separate
merge-and-release skill, its own go-ahead.
---

# Drive PR
Expand Down Expand Up @@ -128,32 +128,57 @@ promotion PR once the fix lands, is the early exit this skill exists to prevent.
1 to 4 in its own worktree and branch, then return here.
7. The fix landing on develop updates the promotion PR's diff and head SHA on its own, re-request
a review on the new head and continue the loop.
8. Repeat 6 and 7 until the promotion PR itself carries no open finding and its checks are green
on the current head.
8. Repeat 6 and 7 until the promotion PR meets every pr-review-conduct Merge Gate item except the
maintainer's explicit permission to merge.
9. Report the promotion PR number and its ready state. Do not merge it.

## Disposing of Every Finding

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

- 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).
- Real and worth doing, but later, so file the issue first, then reply with its link (outcome 4).
- Real, fixable, but a value call rather than a scope boundary, or the agent genuinely does not
know which of the above applies, so ask the maintainer directly, whatever the runtime's own
interactive-question mechanism is, and get an explicit answer in the same turn, a plan to ask
later is resolution by silence (outcome 3). A drive that cannot reach the
maintainer directly, a dispatched one being the ordinary case, escalates to whoever dispatched
it and stops that unit of work there instead, per `pr-review-conduct`, which owns what the
receiving seat then does and how far the escalation travels.
- The same finding keeps recurring against correct code, fix the class, sharpen a name, add a
comment, or take the rule itself to the maintainer, rather than re-arguing the instance every
round (outcome 5).
The rule below is a generated include, so a defect in it is fixed in `pr-review-conduct` and
regenerated rather than edited here. A drive that cannot reach the maintainer directly, a
dispatched one being the ordinary case, escalates per `pr-review-conduct` "Escalate to the
maintainer when".

<!-- include: .agents/skills/pr-review-conduct/SKILL.md > Every finding ends in one of five outcomes -->

1. **Real, so fix it.** Take the fix through `local-strict-review` the same way the push that
opened the pull request went, per `pr-review-conduct` "Expected review loop", then reply with
the fixing commit SHA. A branch already reviewed once has not been reviewed for the fix, which
is the round the `local-strict-review` pass 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.
2. **Not real, or real but structurally out of scope, so decline in the thread with evidence.**
Disprove a wrong finding with the command and its output, the code path that makes it
impossible, or the rule that governs it. A finding that is factually correct but not this
repo's to fix (a verbatim-fidelity manifest entry byte-locking the section, ownership that
sits elsewhere) declines the same way: name the boundary and cite what proves it. Either shape
closes the thread on its own evidence. An assertion ("this is fine") does not close a finding,
a decline needs evidence the reviewer itself could check.
3. **Real, fixable here, but deliberately left as is, a value call rather than a scope
boundary, so it is the maintainer's, not the agent's.** Reach for this only once outcome 2 is
ruled out, since a scope boundary declines on its own evidence and never needs this outcome at
all. State the finding and why the fix is unwanted, and get an explicit answer in the same
turn, before moving to other work. A plan to ask later is resolution by silence the moment
attention moves elsewhere. If the maintainer is not reachable right now, leave the thread open
and say so, rather than treating the intention to ask as the asking.
4. **Real and worth doing later, so file the issue first, then reply with its link.** A deferral
noted only in a thread is lost the moment the PR merges.
5. **Keeps recurring, so fix the class, not the instance.** A finding raised repeatedly against
correct code means the code is not communicating something: add the comment, sharpen the name,
narrow the interface, or fix the rule if the rule is wrong. Bouncing the same point across
rounds is the signal to escalate the rule itself, not to keep re-arguing it.

**A disposition decided on one PR does not carry to the next.** The same finding shape recurring
on a sibling repo or PR, even within one batch or one session, gets its own outcome: its own
evidence-backed decline (outcome 2) or its own explicit maintainer answer (outcome 3). A prior
instance's outcome is context for the new one, never a standing answer to reuse in its place.

`pr-review-conduct` "Every finding ends in one of five outcomes" keeps the full rule, and the
`drive-pr` Skill carries it whole as a generated include, applying it while driving.

<!-- /include -->

## Mechanics Live Elsewhere

Expand Down
8 changes: 4 additions & 4 deletions .agents/skills/local-strict-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,11 @@ The digest is bound to the read for the same reason `--expect-digest` is above:

Each bullet is a rule down to its `Why:` line, which is rationale rather than rule, so a stale rationale is a cleanup rather than a defect.

- **Every finding ends in one of the five outcomes that `pr-review-conduct` "Every finding ends in one of five outcomes" enumerates, reached here with no thread to reply in.**
- **Every finding ends in one of the outcomes that `pr-review-conduct` "Every finding ends in one of five outcomes" enumerates, reached here with no thread to reply in.**
- `Why:` a local finding and a PR-hosted one deserve the same dispositions, and one home for the list is what stops two copies of it drifting apart.
- **The agent disposing of a pass's findings classes each one `style`, `introduced`, or `pre-existing`, in that order.** `style` is a preference between defensible forms. `introduced` is any other finding on text this change wrote, rewrote, or removed, on text this change should have written, on a precondition this change left false elsewhere, or load-bearing for a decision this change puts to the maintainer. `pre-existing` is every other finding.
- `Why:` the reviewer is asked to omit preferences and returns some anyway, and `style` is classed first so that a preference on text this change wrote is not owed a fix.
- **Another round is owed only while an `introduced` finding is open.** Unless evidence disproves it, an `introduced` finding is fixed within the budget below, or escalated where `pr-review-conduct` "Escalate to the maintainer when" says so, a `pre-existing` one is filed once and blocks nothing, and a `style` one is declined under outcome 2 on `code-review`'s own rule to omit preferences.
- **Another round is owed only while an `introduced` finding is open.** Unless evidence disproves it, an `introduced` finding is fixed within the budget below, or escalated where `pr-review-conduct` "Escalate to the maintainer when" says so, a `pre-existing` one is filed once and blocks nothing, and a `style` one is declined with evidence, per `pr-review-conduct` "Every finding ends in one of five outcomes", the evidence being `code-review` "Review the Change"'s own rule to omit preferences.
- `Why:` a finding count over prose never reaches zero, so a loop closing on "did it find anything" does not close, where one closing on the false claim, the unfollowable instruction, or the wrong behavior this change put there does.
- **A push allows two rounds of edits in answer to the passes it owes, one budget across both.** Where an `introduced` finding is still open after the second round, editing stops and what remains goes to the maintainer with its counts per class, per `pr-review-conduct` "Escalate to the maintainer when".
- `Why:` past the second round nearly every finding is against text the previous round's fix wrote, so the rounds are producing the defects they find rather than removing them.
Expand All @@ -142,8 +142,8 @@ Each bullet is a rule down to its `Why:` line, which is rationale rather than ru

## 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).
- Before the first push toward a pull request, the push that opens it in `drive-pr` "The Drive Loop" and in `pr-review-conduct` "Expected review loop".
- Before pushing a fix for a reviewer finding, the same self-review blind spot applies to a fix as to the original diff (the fix outcome of `pr-review-conduct` "Every finding ends in one of five outcomes", which `drive-pr` "Disposing of Every Finding" carries).
- 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.
- Before pushing a change that edits canonical content other repositories carry, or that newly carries some by widening the manifest, over each unit `check` names, per "The Carried-Content Pass" above.

Expand Down
Loading