Skip to content

address-pr-feedback: define how to wait for active reviews #27

Open

Description

@sjunepark

Problem

The active-review gate in skills/address-pr-feedback/SKILL.md says what to wait for, but not how to wait:

Wait for every observed active review and refresh feedback before assessment, ledger creation, checkout, edits, pushes, or replies. … Diagnose an explicit failure or a review that stalls beyond a reasonable wait window and report intake as blocked. (lines 63–66)

In practice, an agent can satisfy "don't act yet" by ending its turn, and ending the turn isn't waiting.

Observed on #26

The skill was invoked as /address-pr-feedback and merge, then release, then /sjskills sync. The collector showed:

  • CodeRabbit: check pending, so the review was active.
  • Codex: its summary reported the review as failed, with no findings.

The agent correctly held off on assessment and edits, then ended its turn with "waiting for CodeRabbit." The user had to prompt again, and the requested merge, release and sync chain stalled. Two things caused it:

  1. Conflicting host guidance: the host (Claude Code desktop) tells agents never to poll CI checks themselves: no gh polling loops, Monitor or scheduled wakeups. CodeRabbit shows up as a check, so the agent treated waiting for it as forbidden CI polling and left the waiting to the app's PR monitor.
  2. No wake-up guarantee: the app's PR monitor had auto-fix and address-comments off, so nothing would re-invoke the agent when the review completed.

Proposed changes

  1. Define waiting as staying in the task. Poll review state with a bounded loop (for example, every 60–90 s for up to about 20 min). While a review is active, don't end the turn; when the bound is reached, report intake as blocked. Replace "a reasonable wait window" with that concrete bound.
  2. Separate review polling from CI polling. State that polling review completion is part of this skill's gate and is authorized, even where a host forbids polling CI. CI results stay out of scope unless the task, such as a merge, requires green checks.
  3. Name the completion signal for each bot:
    • CodeRabbit: its completed review or final summary, or its check concluding.
    • Codex: +1 from chatgpt-codex-connector[bot], a Codex-authored review, or a failed status in its summary comment.
    • An explicit failure ends the wait and doesn't count as an active review.
  4. Fix collector encoding. scripts/collect_pr_feedback.py line 237 calls subprocess.run(command, text=True, capture_output=True, check=False) without an encoding. On Windows with a non-UTF-8 console (cp949), decoding gh output crashes with UnicodeDecodeError, and the script then fails with 'NoneType' object has no attribute 'strip'. Passing encoding="utf-8" (and possibly errors="replace") fixes it; PYTHONUTF8=1 works around it today.

Acceptance

  • Waiting: an agent handling a PR with an in-progress CodeRabbit review waits within the same task until the review completes or the bound expires. It then continues with assessment, or reports the review as blocked.
  • Encoding: the collector runs on Windows cp949 without PYTHONUTF8.
  • Evals: add an eval for the in-progress-review case (the skill has no evals/ yet).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions