Skip to content
Open
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
20 changes: 20 additions & 0 deletions generator/tests/test_bot_review_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
/ "bot-review-state.sh"
)
REVIEW_SKILL = BOT_REVIEW_STATE.parent.parent / "skills" / "review" / "SKILL.md"
RUNNING_IN_CI_SKILL = (
BOT_REVIEW_STATE.parent.parent / "skills" / "running-in-ci" / "SKILL.md"
)

BOT = "tend-bot"
HEAD = "head000"
Expand Down Expand Up @@ -539,3 +542,20 @@ def test_review_skill_spares_the_approval_when_the_comment_withholds_nothing() -
assert "posts a COMMENT that withholds the verdict" in skill
assert "A COMMENT that withholds nothing does not qualify" in skill
assert "whenever this round posts a COMMENT rather than an approval" not in skill


def test_a_dismissal_path_exists_for_an_invalidation_that_is_not_an_event() -> None:
"""Every dismissal site in `review` and `weekly` is keyed on something that
happened *on* the approved PR — a review round, a rewrite, a red check. An
approval superseded by a *different* PR merging reaches none of them, so it
stands until a human clears it. The generic rule lives in `running-in-ci`,
which every skill that can reach that conclusion loads, and it fires on the
conclusion rather than on a post — the dedup rules routinely (and rightly)
suppress the comment that would otherwise carry it."""
skill = RUNNING_IN_CI_SKILL.read_text()

assert ".standing_approval_id" in skill
assert "reviews/$STANDING/dismissals" in skill
assert "-X PUT" in skill
# Not keyed on this session posting anything.
assert "whether or not this session posts" in skill
18 changes: 18 additions & 0 deletions plugins/tend-ci-runner/skills/running-in-ci/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,24 @@ If it moved, `git fetch` and read the new commits before verifying: drop whateve

When merging the default branch into a PR branch, **never use `--allow-unrelated-histories`**: if `git merge` fails because no merge base exists, the checkout is broken (usually shallow — re-checkout with `fetch-depth: 0`), and forcing the merge creates add/add conflicts in every file. If the merge fails because untracked files would be overwritten, stash them (`git stash --include-untracked`, merge, `git stash pop`) rather than deleting them.

## Dismiss a standing bot approval the moment you conclude the PR shouldn't merge

A bot `APPROVED` keeps deciding the PR until a dismissal or a `CHANGES_REQUESTED` replaces it. A later COMMENT doesn't, and neither does the event that actually invalidated it: another PR merging and superseding this one, a dependency bump that turns this PR into a downgrade, an approach the thread has since rejected. None of those touch the approved PR, so no review round fires and the approval stands indefinitely — with nothing between it and a merge once the branch stops conflicting. Whichever session reaches the conclusion is the one that has to clear it, whether or not this session posts anything.

Keying the dismissal to a post is what leaves it standing: **Recheck Before Posting** rightly suppresses a second deferral comment when one already stands, and a dismissal that rides on that comment is suppressed with it. Dismiss on the conclusion, then say so in the summary — where this session does post a review carrying that conclusion, dismiss after the post lands, so a failed post doesn't leave the PR with neither a verdict nor findings.

```bash
REPO=$(gh repo view --json nameWithOwner --jq '.nameWithOwner')
# "" once a dismissal or a later CHANGES_REQUESTED has cleared it, so a second
# session over the same PR dismisses nothing.
STANDING=$(${CLAUDE_PLUGIN_ROOT}/scripts/bot-review-state.sh <number> | jq -r '.standing_approval_id')
# PUT, not POST — the dismiss endpoint requires it.
[ -z "$STANDING" ] || gh api "repos/$REPO/pulls/<number>/reviews/$STANDING/dismissals" \
-X PUT -f message="<what invalidated the approval>"
```

The test is the merge, not tidiness: a finding you'd have left as a review comment is no reason to withdraw a verdict the code still earns, and neither is a branch that merely can't merge yet — a conflicting PR whose code the approval still covers keeps it. Dismiss when merging the PR, once it could merge, would be the wrong outcome.

## CI Monitoring

After pushing, what to do depends on whether a red result creates a follow-up.
Expand Down
Loading