Skip to content

feat(grug): board shows a re-review is running, without the two ways that broke it #819

Description

@quadseven

Why

Push a new commit to a PR and the board keeps showing the PREVIOUS verdict for the whole next review. "Grug say WAIT, 3 findings" sits above code Grug has not read yet. The check-run flips to in_progress; the board - the surface people actually read - does not.

This was implemented once (#673 item 1) and REMOVED before shipping, because two-axis review found the implementation worse than the problem. Both failure modes are hard requirements for any retry.

What must be true this time

1. It must not be able to touch a comment Grug did not write.
The removed version omitted app_id, so board_client.find_board could match any comment quoting the board marker - including a human's. Worse, because the key is elder, the legacy branch (if key != "elder" in board_client.py) DISCARDS existing text rather than preserving it as a section, so a human quoting the marker in a review discussion would have had their comment replaced wholesale by "Grug read new markings now. Wait."

2. Something must clear it on EVERY terminal path.
The removed version had exactly one clear path - the settled header write, gated on not check_publish_failed. So a check-publish failure, a consumer crash into the DLQ, post-settle ineligibility (_review_eligible false), and the fail_open_freshness path each left "Wait" on the board permanently, with the previous findings already overwritten. That is strictly worse than the staleness it exists to fix: a stale verdict is at least a verdict.

3. It must not destroy the prior findings.
Replacing the section means a stuck note has also erased the evidence. Prefer updating only the header, leaving the previous findings visible and clearly marked as from the prior pass.

Acceptance criteria

  • Passes app_id, and a test proves a non-Grug comment carrying the marker is never selected or modified
  • Every terminal path clears the note - enumerate them and test each: check-publish failure, DLQ exhaustion, ineligible-after-settle, fail-open-freshness, consumer SIGTERM
  • A review that dies leaves the board no worse than before it started
  • Prior findings survive the in-progress state
  • Never creates a comment (create_if_absent=False), so it can never cost an email
  • Full suite green

Size

Size: M

Out of scope

  • Run-posture disclosure (tier/arms/model). Operator decision 2026-08-02: that belongs on the dashboard, not the author's PR comment.

Dependencies

Refs #673, Refs #791

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