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
7 changes: 4 additions & 3 deletions agents/github-pr-fix-planner.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ For each row, after Steps 2 and 3, read the file the thread anchors to and the d
- **Draft answer:** <the answer, on a question>
```

Recommend **agree** when the claim holds and the proposed change is in this pull request's scope, **disagree** when the claim does not hold or the change is outside the ticket, with the reason a reply would give, **discuss** when the code alone cannot settle it, and **answer** on a question. A claim that points at a sibling pull request of the stack or upstream, as D-006 says, is an agree whose reason says where it goes. A thread the pull request author opened is assessed like anyone's; one that asks nothing and only tells reviewers something, such as why the code is as it is, is an agree, reply only: `a note for reviewers, no change`.
Recommend **agree** when the claim holds and the proposed change is in this pull request's scope, **disagree** when the claim does not hold or the change is outside the ticket, with the reason a reply would give, **discuss** when the code alone cannot settle it, and **answer** on a question. A claim that points at a sibling pull request of the stack or upstream, in the repository the code belongs in, is an agree whose reason says where it goes. A thread the pull request author opened is assessed like anyone's; one that asks nothing and only tells reviewers something, such as why the code is as it is, is an agree, reply only: `a note for reviewers, no change`.

Then, after the entries, list the **Conflicts**: each place where a thread a person opened, the author or anyone else, and an AI finding (a thread from a findings review) ask for different things in the same code, one block each:

Expand Down Expand Up @@ -77,8 +77,9 @@ A findings thread's `Proposed:` line is its work item. The approach implements t
A comment can point at a symptom of a larger problem; classify that as Needs Clarification,
sized `changes the plan`.
5. **Classify the destination.** A comment that belongs in a sibling pull request of the stack,
read from the `part k of n` on the dump's `body.md` `Ticket:` line, or upstream as D-006 says,
is a **Move**: plan it with where it goes and no code change here.
read from the `part k of n` on the dump's `body.md` `Ticket:` line, or upstream, in the
repository the code belongs in, is a **Move**: plan it with where it goes and no code change
here.
6. **Write the plan entry.** Clear comments get a concrete approach — what changes, where, and why, against the Step 2 conventions. Needs-Clarification comments get a question precise enough that the answer unblocks implementation, and **no** proposed fix. You cannot ask it yourself — no user is present — so it goes in the plan for the calling session to ask.

## Step 5: Return the Fix Plan
Expand Down
4 changes: 2 additions & 2 deletions agents/github-pr-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ Evaluate every change against:
2. **Security** — injection, exposed secrets, unsafe deserialization, missing auth checks, path traversal.
3. **Performance** — needless allocations, O(n²) where O(n) is available, missing indexes, unbounded queries, repeated expensive work.
4. **Style and consistency** — naming, organization, and idioms measured against Step 2 and the surrounding code.
5. **Test coverage** — new paths tested, edge cases covered, existing tests updated for changed behavior, as the caller's test checklist sets out.
5. **Test coverage** — new paths tested, edge cases covered, existing tests updated for changed behavior, as the caller's test checklist sets out; and which tests the diff adds that CI does not run, read from its workflow's test commands and the tests' markers or paths, never by running them.
6. **Documentation** — public APIs documented, complex algorithms explained, breaking changes noted.
7. **Changelog and version** — whether an entry or bump is owed is the repo's call: infer it from what exists (`CHANGELOG.md`, `.changeset/`, `changelog.d/`; the version in `pyproject.toml`, `package.json`, `Cargo.toml`) and what comparable merges touched. Where release tooling generates them, nothing is owed.
8. **Unnecessary abstraction** — flag a new private helper that should be inlined at its call sites. The signs, for a function or method under 10 lines, are weighed together rather than flagged one at a time; a short helper called from many places may well earn its place:
Expand Down Expand Up @@ -168,7 +168,7 @@ Raise nothing the contract's "Do not flag" lists, nits included. Finding prose i

## Test Coverage Assessment

<Whether the changes are adequately tested. Name specific untested paths and missing edge cases.>
<Whether the changes are adequately tested. Name specific untested paths, missing edge cases, and each test the diff adds that CI's configuration or markers leave out.>

## CI/Workflow Status

Expand Down
8 changes: 5 additions & 3 deletions skills/pr-fix/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ description: >
argument-hint: "[PR number or URL] [scope] [skip the gate] [rebase]"
metadata:
author: llm-tooling
version: 2.8.0
version: 2.9.0
---

# PR Fix
Expand Down Expand Up @@ -69,7 +69,7 @@ When it is behind, **stop**: say by how many commits and whether the base's new

## 5. Disposition the Threads

Every thread ends this step with one of four dispositions: **agree**, **disagree** or **discuss**, the three verdict words of `pr-review`'s "Whose reply counts", or **answer** for a question. Step 7 refines an agree into one of two kinds, each posted as `Agree`: **move**, for a comment that belongs in a sibling pull request of the stack or upstream as D-006 says, whose reply names where it goes and whose code change here is none; and **reopen**, for a thread whose `Proposed:` line opens `changes the plan`, or a person's thread the author marks as design: the review showed the design is wrong.
Every thread ends this step with one of four dispositions: **agree**, **disagree** or **discuss**, the three verdict words of `pr-review`'s "Whose reply counts", or **answer** for a question. Step 7 refines an agree into one of two kinds, each posted as `Agree`: **move**, for a comment that belongs in a sibling pull request of the stack or upstream, in the repository the code belongs in, whose reply names where it goes and whose code change here is none; and **reopen**, for a thread whose `Proposed:` line opens `changes the plan`, or a person's thread the author marks as design: the review showed the design is wrong.

**Read them.** One `pr-reader` call in `state` mode, given the repository and the pull request, lists every thread, finding threads and people's threads alike, with its state in the script's words: `agree`, `disagree`, `discuss`, `unanswered`, `unparsed`, `answered` for a question, `none`, and `contested: yes`, and in its last column whether the verdict was judged from a reply in the person's own words (`yes, <login>: <basis>`), and `unclear: <basis>` where the judge could not tell. A contested row shows the first verdict's word in the verdict column and `yes` in the contested column, so read that column before the word.

Expand Down Expand Up @@ -160,7 +160,7 @@ Get explicit approval before committing. If the person wants changes, go back to

## 11. Commit

Commit the approved work through `/lasp-dev:commit`, which keeps `.review/` and the plan out — one commit per thread when the fixes are independent, a single commit when they are one coherent change. Use the repo's existing message style. Describe what changed, not that a reviewer asked for it.
Commit the approved work through `/lasp-dev:commit`, which keeps `.review/` and the plan out — one commit per thread, or per group of threads that share a concern, so each commit answers what its threads asked; a single commit when they are one coherent change. Use the repo's existing message style. Describe what changed, not that a reviewer asked for it.

When step 4's answer was **fix first, rebase before the push**, rebase the committed fixes onto `origin/<base>` now, after `git fetch origin <base>`. Conflicts stop the skill, as in step 4. Step 9's checks ran before the rebase: say so at step 12, and run step 9 again first when the person asks.

Expand All @@ -179,6 +179,8 @@ Its first line carries the decision when it is not yet on GitHub, so each thread

A conversation-tab comment is not a thread and is read at the harvest, not here; reply only if the person asks.

**A summary comment**, optional: when the round answers several threads, offer one comment on the pull request's conversation that lists what was fixed, moved, declined and left in discussion, each with its thread's link. It follows the Substance, Form and Attribution sections of `comment-style.md`, ends with the attribution line, and is drafted with the replies and counted among them, so it posts only under their yes; under **Push only** it is dropped, and the next run drafts it afresh.

**Ask once, for the push and the replies.** Show every reply in final form, with its thread's link, then ask, naming the branch, the commits and the count: **Push N commits to `<branch>` and post M replies** (Recommended); **Push only**, keeping the replies for the next run; **Edit the replies**; or **Stop here**, keeping the commits local. When the round has no new commits, the options are **Post M replies** (Recommended), **Edit the replies** or **Stop here**. Where the branch was rebased, at step 4 or step 11, the push is not a fast-forward: say so in the question, and push with `--force-with-lease`, which refuses if the remote moved since you fetched. **Never plain `--force`**, and never push to the base branch. Where the repository's instructions forbid an agent's push, the first option is to post the replies once the person has pushed, as `using-github`'s "Pushing" says. Silence or an ambiguous answer pushes and posts nothing.

If the push is rejected, the author pushed while you worked. Stop and let the person choose how to reconcile — do not rebase, merge, or force anything on your own, and post no reply that names a commit not on the remote.
Expand Down
2 changes: 1 addition & 1 deletion skills/pr-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ description: >
argument-hint: "[PR number or URL] [focus area]"
metadata:
author: lasp-dev
version: 6.13.0
version: 6.14.0
---

Every question, and every file it writes, follows `${CLAUDE_PLUGIN_ROOT}/skills/work/decision-points.md`. The plugin root is `${CLAUDE_PLUGIN_ROOT}`: a path written `<plugin root>/…` in a file this skill reads starts there.
Expand Down
2 changes: 2 additions & 0 deletions skills/pr-review/comment-style.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ notification, next to the code. Write for that moment.
happens here if `token` is expired?") instead of asserting a finding.
- Critique the code, not the author: "this drops the error", not "you forgot to
handle the error". Never speculate about intent or skill.
- No "just", "simply" or "obviously": they tell the author the fix is easy
and add nothing to it, as `writing`'s prose standards, "Overclaiming", say.
- Blocking concerns come before stylistic ones. An aside that cannot survive
being last gets dropped.

Expand Down
Loading