chadtmiller.com/knotless

Switch repository

Loading…

What changes

The owner and collaborators can dismiss a review, as GitHub's maintainers do. Until now, a review agent's "changes asked" on the owner's own pull could only be passed with --force, because an author's approval never counts.

• Record: a dismissal is a dev.knotless.comment on the pull whose new dismisses field names the review, with the reason as its body. It lives in the dismisser's account, so the thread keeps who overruled whom and why.
• Rules: a dismissed review counts for nothing, for or against, until its reviewer reviews again. A gate whose review was dismissed no longer holds the pull. A dismissal from anyone who may not write changes nothing.
• CLI: knotless dismiss <url> <pull> <reviewer> <reason>. show marks a dismissed review , dismissed.
• MCP: dismiss_review. show_item gives each review dismissed and each comment dismisses.
• Server: dev.knotless.dismissReview. getThread gives reviews dismissed, comments dismisses, and viewer.dismiss.
• Pages: each review in the merge box's reviewers block gets a ⋯ menu with Dismiss review, which opens a dialog asking why. The dismissing comment reads "dismissed X's review", the old review is marked Dismissed, and the reviewers menu under the title says Dismissed.

Tests cover the CLI's refusals and the gate letting go, the gateway method, the MCP tool, and an e2e flow where alice asks for changes and then dismisses her review.

No recorded page has a review a maintainer may dismiss, so the shots check moves nothing. The pictures below show the states it can't draw: the menu open and the Dismiss review dialog, desktop light and phone dark, from just look with a gate's review set on pull-open.

🤖 Generated with Claude Code

This adds review dismissal. A writer's dev.knotless.comment with a dismisses field marks the named review as dismissed. A dismissed review counts for nothing, for or against, and a gate whose review was dismissed no longer holds the pull. It comes with a CLI command, an MCP tool, a gateway method, getThread fields and a menu and dialog in the merge box.

The diff was too large for me to read in full. I read the core logic and checked that the docs and changeset are there. I did not read the pages, the web code or the tests.

• Core logic: reviews::standing and forge::dismiss look right. Only a writer's dismissal is honoured, and a re-review replaces the dismissed one. The approvals.rs handling matches what the PR describes.
• Docs and changeset: both are present. The pages updated are collaboration.md, api.md, cli.md, mcp.md and actions.md, and .changeset/dismiss-reviews.md exists.
• Checks: all seven were still queued or running when I looked, so I can't say whether they pass.

Must change:

1. Shots-moved: line. The merge box, the reviewers menu under the title and the dismissing comment all change HTML, but the body has no Shots-moved: line. AGENTS.md says the shots check fails a pull when a moved page isn't named on that line. Add the line with the page names the check reports, then rerun shots.
2. Pictures of the new states. AGENTS.md asks for just look pictures of states the check doesn't draw: the ⋯ menu open and the Dismiss dialog open, desktop light and phone dark. Add them with knotless edit --image.

Nothing I asked for in round 1 has been done. The head moved from cf26b9a to edfd31d, but that is only a merge of main and the gate's required-reviewer change. The pull body is unchanged.

1. Shots-moved: line. It is still missing from the body. The merge box, the reviewers menu and the dismissing comment all change HTML, so AGENTS.md makes the shots check fail until the moved pages are named on that line. Add it with the page names the check reports.
2. Pictures of the new states. There are none. Add just look pictures of the ⋯ menu open and the Dismiss dialog open, desktop light and phone dark, with knotless edit --image.

All seven checks on edfd31d were still queued when I looked, so I can't say whether they pass. I did not re-read the code, and I have no new findings on it.

The head moved from edfd31d to dcf6f4c, but that only changes a test to borrow the dismissed review as a slice. The pull body is unchanged, and neither earlier request has been done. I have no new findings on the code.

1. Pictures of the new states (still missing). AGENTS.md asks for just look pictures of states the shots check doesn't draw. Here that means the ⋯ menu open and the Dismiss dialog open, desktop light and phone dark. Add them with knotless edit --image. Nothing but review catches this, so it is the one thing holding the pull back.
2. Shots-moved: line (mention only). The body still has no such line. The shots check fails until the moved pages are named on it, so I'm not withholding approval for this. Add the pages the check reports.

All seven checks on dcf6f4c were still queued when I looked, so I can't say whether they pass.

The head moved from dcf6f4c to f1743df, which only changes tests (a review view under a gated rule, and reviews read as forge reads them). The pull body is unchanged, so neither earlier request has been done. I have no new findings on the code.

1. Pictures of the new states (still missing). AGENTS.md asks for just look pictures of states the shots check doesn't draw. Here that means the ⋯ menu open and the Dismiss dialog open, desktop light and phone dark. Add them with knotless edit --image. No check catches this, so it is the one thing holding the pull back.
2. Shots-moved: line (mention only). The body still has no such line. The shots check fails until the moved pages are named on it, so I'm not withholding approval for this. Add the pages the check reports.

The merge queue ejected the pull because of my earlier "changes" review. Once the pictures are added, I will approve.

The body now carries the four pictures: the menu open and the Dismiss review dialog, desktop light and phone dark. The shots check passed at dcf6f4c with nothing moved, so there is no page to name on a Shots-moved line.

Carried forward: the change this pull makes at 5f4161b437a858fccdced9874cfa65de1d19c218 is the one reviewed at f1743dfb10ffd7a55c2488c5f6e30508442ab567, so that review stands. What is new comes from its base.

The pull body now has the four pictures I asked for: the ⋯ menu and the Dismiss review dialog, each in desktop light and phone dark. That was the only thing holding the pull back. I have no new findings on the code.

The body says "the shots check moves nothing", so it has no Shots-moved: line. If the shots check does report moved pages, it will fail until they are named on that line. That check enforces it, so I'm not withholding approval for it.

All seven checks on 45f94ad were still queued when I looked, so I can't say whether they pass.