reviewer skill posts change requests as issue comments, not pull reviews — webhook doesn't dispatch address-review #111

New issue

Closed

opened 2026-04-19 23:21:02 +00:00 by claude-desktop · 0 comments

claude-desktop commented

2026-04-19 23:21:02 +00:00

Collaborator

User story

As the operator, I want the reviewer agent to post REQUEST_CHANGES feedback via the formal pull-review API so the webhook's pull_request_rejected event fires on its own and boss gets auto-dispatched on address-review, instead of me having to manually escalate every change request.

What happens today

On PR #110 (2026-04-19), the reviewer posted its 🔴 REQUEST_CHANGES feedback via mcp__forgejo__create_issue_comment. The comment body renders correctly on the PR, but:

list_pull_reviews on the PR shows only the original REQUEST_REVIEW entry — no formal review with state: REQUEST_CHANGES exists.
Forgejo therefore fires no pull_request_rejected event.
webhook-handlers.ts::handleChangesRequested never runs, so boss is not dispatched on the address-review skill.

I had to manually call mcp__forgejo__create_pull_review with state: REQUEST_CHANGES to escalate issuecomment-6067 into a formal review, which then fired the webhook and dispatched boss. Happened silently — the only signal was "PR sitting un-addressed longer than it should".

Root cause

skills/review.md (or whichever skill the reviewer uses) instructs the agent to post its verdict via mcp__forgejo__create_issue_comment. That posts a comment, not a review. Forgejo's review lifecycle events only fire on formal reviews.

Fix

Update the reviewer skill to:

Post APPROVED verdicts via mcp__forgejo__create_pull_review with state: APPROVED (not a comment).
Post REQUEST_CHANGES verdicts via mcp__forgejo__create_pull_review with state: REQUEST_CHANGES.
Keep the body formatting as-is — just switch the API call.

Optional: COMMENT-state reviews for purely informational findings (non-blocking notes), though today the reviewer already folds those into the must-fix body.

Acceptance criteria

Skill

skills/review.md instructs the reviewer to call mcp__forgejo__create_pull_review (never mcp__forgejo__create_issue_comment for the verdict).
Skill documents the three state values and when to use each.
Skill success gate: one formal pull review exists with the correct state matching the verdict.

Regression

Next reviewer dispatch on a real PR with change requests: list_pull_reviews shows a state: REQUEST_CHANGES entry, and boss gets auto-dispatched on address-review without manual escalation.

Out of scope

Inline-comments (comments: [{path, new_position, body}]) — keep the reviewer's output as a single review body for now; per-line comments can come later.
Backfill on existing PRs — only new reviews follow the new path.

References

Live incident: PR #110 review on 2026-04-19, issuecomment-6067 (comment) vs the escalated issuecomment-6069 (formal review).
skills/review.md — the file to amend.
mcp__forgejo__create_pull_review — the correct API, takes state: APPROVED | REQUEST_CHANGES | COMMENT.
src/webhook-handlers.ts::handleChangesRequested — the dispatch target that requires the formal event.

Dependencies

Blocked by: nothing.
Blocks: clean review → address-review loop without manual escalation.
Branch off: main.

## User story As the **operator**, I want the reviewer agent to post `REQUEST_CHANGES` feedback via the formal pull-review API so the webhook's `pull_request_rejected` event fires on its own and boss gets auto-dispatched on `address-review`, instead of me having to manually escalate every change request. ## What happens today On PR #110 (2026-04-19), the reviewer posted its `🔴 REQUEST_CHANGES` feedback via `mcp__forgejo__create_issue_comment`. The comment body renders correctly on the PR, but: - `list_pull_reviews` on the PR shows only the original `REQUEST_REVIEW` entry — no formal review with `state: REQUEST_CHANGES` exists. - Forgejo therefore fires no `pull_request_rejected` event. - `webhook-handlers.ts::handleChangesRequested` never runs, so boss is not dispatched on the `address-review` skill. I had to manually call `mcp__forgejo__create_pull_review` with `state: REQUEST_CHANGES` to escalate issuecomment-6067 into a formal review, which then fired the webhook and dispatched boss. Happened silently — the only signal was "PR sitting un-addressed longer than it should". ## Root cause `skills/review.md` (or whichever skill the reviewer uses) instructs the agent to post its verdict via `mcp__forgejo__create_issue_comment`. That posts a comment, not a review. Forgejo's review lifecycle events only fire on formal reviews. ## Fix Update the reviewer skill to: - Post `APPROVED` verdicts via `mcp__forgejo__create_pull_review` with `state: APPROVED` (not a comment). - Post `REQUEST_CHANGES` verdicts via `mcp__forgejo__create_pull_review` with `state: REQUEST_CHANGES`. - Keep the body formatting as-is — just switch the API call. Optional: `COMMENT`-state reviews for purely informational findings (non-blocking notes), though today the reviewer already folds those into the must-fix body. ## Acceptance criteria ### Skill - [ ] `skills/review.md` instructs the reviewer to call `mcp__forgejo__create_pull_review` (never `mcp__forgejo__create_issue_comment` for the verdict). - [ ] Skill documents the three `state` values and when to use each. - [ ] Skill success gate: one formal pull review exists with the correct `state` matching the verdict. ### Regression - [ ] Next reviewer dispatch on a real PR with change requests: `list_pull_reviews` shows a `state: REQUEST_CHANGES` entry, and boss gets auto-dispatched on `address-review` without manual escalation. ## Out of scope - Inline-comments (`comments: [{path, new_position, body}]`) — keep the reviewer's output as a single review body for now; per-line comments can come later. - Backfill on existing PRs — only new reviews follow the new path. ## References - Live incident: PR #110 review on 2026-04-19, [issuecomment-6067](https://forge.jacquin.app/charles/claude-hooks/pulls/110#issuecomment-6067) (comment) vs the escalated [issuecomment-6069](https://forge.jacquin.app/charles/claude-hooks/pulls/110#issuecomment-6069) (formal review). - `skills/review.md` — the file to amend. - `mcp__forgejo__create_pull_review` — the correct API, takes `state: APPROVED | REQUEST_CHANGES | COMMENT`. - `src/webhook-handlers.ts::handleChangesRequested` — the dispatch target that requires the formal event. ## Dependencies - **Blocked by:** nothing. - **Blocks:** clean review → address-review loop without manual escalation. - **Branch off:** `main`.

claude-desktop added the

area:agents

type:bug

labels

2026-04-19 23:21:07 +00:00