Skip to content

Re-request reviews after addressing changes#276

Merged
dgershman merged 1 commit into
mainfrom
feature/crow-275-re-request-review-after-changes
May 15, 2026
Merged

Re-request reviews after addressing changes#276
dgershman merged 1 commit into
mainfrom
feature/crow-275-re-request-review-after-changes

Conversation

@dgershman
Copy link
Copy Markdown
Collaborator

Summary

  • Adds a re-request review instruction to the changesRequested auto-respond prompt in AutoRespondCoordinator
  • After Claude Code addresses review comments and pushes, it now also runs gh pr edit --add-reviewer <login> (GitHub) or glab mr update --reviewer <login> (GitLab) to re-request review from each reviewer who requested changes
  • Both the auto-respond and manual "Address Changes" quick action paths pick up the change since they share AutoRespondPrompts.build()

Closes #275

Test plan

  • Trigger a "changes requested" review on a PR managed by Crow
  • Verify the injected prompt includes the re-request instruction
  • Verify Claude Code fetches reviews, addresses comments, pushes, then runs gh pr edit --add-reviewer
  • Verify the reviewer receives a GitHub notification that their review was re-requested

🤖 Generated with Claude Code

Add a re-request review instruction to the changes-requested prompt so
Claude Code automatically runs `gh pr edit --add-reviewer` (or
`glab mr update --reviewer` for GitLab) after pushing fixes, notifying
the original reviewers that their feedback has been addressed.

🤖 Generated with Claude Code, orchestrated by Crow

Co-Authored-By: Claude <noreply@anthropic.com>
Crow-Session: 3D3AC58B-7D51-487C-8B99-DF6E514F6C4F
Copy link
Copy Markdown
Contributor

@dhilgaertner dhilgaertner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code & Security Review

Critical Issues

None — change is a 4-line string augmentation to the changesRequested auto-respond prompt.

Security Review

Strengths:

  • No new shell invocation surface in Crow itself; the appended hint is text injected into a managed terminal for Claude Code to act on. transition.prURL is already interpolated into the existing fetchHint in the same string, so this introduces no new injection vector.
  • No new API scopes, secrets, or auth flows. Consistent with the file's stated design (Crow does not fetch review bodies or CI logs itself).
  • Manual addressChanges quick-action path inherits the change for free via AutoRespondPrompts.build() (Sources/Crow/App/AutoRespondCoordinator.swift:168), avoiding drift between auto and manual flows.

Concerns:

  • None.

Code Quality

  • Yellow — glab mr update --reviewer semantics (Sources/Crow/App/AutoRespondCoordinator.swift:122): Unlike gh pr edit --add-reviewer, the GitLab glab mr update --reviewer <login> flag sets the reviewer list rather than appending. Running it once per reviewer (as the prompt instructs: "for each one") would only leave the last login as the reviewer, dropping the others. Consider either (a) instructing Claude to pass a comma-separated list in a single invocation, or (b) noting the set-vs-add distinction so Claude doesn't iterate. The GitHub path is fine — --add-reviewer is additive.
  • Green — Prompt length (Sources/Crow/App/AutoRespondCoordinator.swift:128): The changesRequested prompt is now noticeably longer, still on a single line. Given the existing single-line + \n contract documented at the top of AutoRespondPrompts (line 106-110), this is fine, but worth keeping an eye on if more hints accumulate.
  • Green — No tests: There are no tests covering AutoRespondPrompts.build() in the repo, so the substring change is verified manually only. A small snapshot-style test asserting the prompt contains both gh pr edit and --add-reviewer for GitHub and glab mr update/--reviewer for GitLab would lock down the contract cheaply. Not a blocker.

Summary Table

Priority Issue
🟡 Yellow glab mr update --reviewer replaces reviewers; iterating per-login will drop earlier ones (line 122)
🟢 Green Consider a snapshot test for the prompt strings to prevent silent regressions
🟢 Green Prompt is approaching a chunky single line — monitor as more hints are appended

Recommendation: Approve. The change is low-risk, small, and the manual quick-action path is correctly covered via shared AutoRespondPrompts.build(). The GitLab semantics nit is worth a follow-up but doesn't block merge — Claude Code can self-correct when the command behaves unexpectedly.


🤖 Reviewed by Crow via Claude Code

@dgershman dgershman merged commit b9b687f into main May 15, 2026
3 checks passed
@dgershman dgershman deleted the feature/crow-275-re-request-review-after-changes branch May 15, 2026 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Address review quick action should re-request reviews after changes

2 participants