thuanle/claw-email
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reviewer workflow must read PR discussion comments before posting verdict #16

New Issue
Closed
opened 2026-04-28 06:04:00 +07:00 by codex · 5 comments
codex commented 2026-04-28 06:04:00 +07:00
Collaborator
Copy Link
Copy Source

Summary

During review of PR #15, the reviewer read review objects but did not read the PR's normal discussion comments. That missed an explicit owner approval comment and led to an incorrect scope objection.

Problem

On Gitea, PR review state, inline review comments, and normal PR discussion comments are separate sources. Treating get_reviews as complete context is incorrect.

Why this matters

  • Reviewer can miss owner intent or approval recorded in normal PR discussion.
  • Review findings can be wrong even when diff analysis is otherwise correct.
  • This creates avoidable rework and trust issues in the review process.

Expected behavior

Before posting a verdict, the reviewer workflow should load and read PR discussion comments in addition to review objects and inline review comments.

Proposed scope

  • Update reviewer process documentation in AGENT.md.
  • Update .codex/skills/review-pr/SKILL.md to explicitly load PR discussion comments.
  • Keep the change minimal and focused on preventing this exact failure mode.
## Summary During review of PR #15, the reviewer read review objects but did not read the PR's normal discussion comments. That missed an explicit owner approval comment and led to an incorrect scope objection. ## Problem On Gitea, PR review state, inline review comments, and normal PR discussion comments are separate sources. Treating `get_reviews` as complete context is incorrect. ## Why this matters - Reviewer can miss owner intent or approval recorded in normal PR discussion. - Review findings can be wrong even when diff analysis is otherwise correct. - This creates avoidable rework and trust issues in the review process. ## Expected behavior Before posting a verdict, the reviewer workflow should load and read PR discussion comments in addition to review objects and inline review comments. ## Proposed scope - Update reviewer process documentation in `AGENT.md`. - Update `.codex/skills/review-pr/SKILL.md` to explicitly load PR discussion comments. - Keep the change minimal and focused on preventing this exact failure mode.
codex commented 2026-04-28 06:04:16 +07:00
Author
Collaborator
Copy Link
Copy Source

Final Requirements

Summary: Update reviewer workflow so PR discussion comments are always read before a verdict is posted.

Scope:

  • Update AGENT.md reviewer process to mention PR discussion comments explicitly.
  • Update .codex/skills/review-pr/SKILL.md to load PR discussion comments via issue_read with method: "get_comments" for the PR itself.
  • Clarify that reviews, inline review comments, and PR discussion comments are separate context sources.
  • Add a rule that verdict posting happens only after PR discussion comments are checked.

Out of scope:

  • Any broader workflow refactor.
  • Rename or restructure skills.
  • Additional consistency checks unrelated to PR discussion comment handling.

Acceptance criteria:

  • Reviewer workflow docs explicitly mention reading PR discussion comments.
  • review-pr skill no longer allows get_reviews alone to be treated as complete PR context.
  • The documented process would have prevented the miss on PR #15.
## Final Requirements **Summary:** Update reviewer workflow so PR discussion comments are always read before a verdict is posted. **Scope:** - Update `AGENT.md` reviewer process to mention PR discussion comments explicitly. - Update `.codex/skills/review-pr/SKILL.md` to load PR discussion comments via `issue_read` with `method: "get_comments"` for the PR itself. - Clarify that reviews, inline review comments, and PR discussion comments are separate context sources. - Add a rule that verdict posting happens only after PR discussion comments are checked. **Out of scope:** - Any broader workflow refactor. - Rename or restructure skills. - Additional consistency checks unrelated to PR discussion comment handling. **Acceptance criteria:** - Reviewer workflow docs explicitly mention reading PR discussion comments. - `review-pr` skill no longer allows `get_reviews` alone to be treated as complete PR context. - The documented process would have prevented the miss on PR #15.
codex commented 2026-04-28 06:04:28 +07:00
Author
Collaborator
Copy Link
Copy Source

Implementation Plan

  • Update AGENT.md reviewer steps so PR discussion comments are part of required review context.
  • Update .codex/skills/review-pr/SKILL.md workflow to load PR discussion comments explicitly.
  • Add a short rule in review-pr that verdict posting happens only after PR discussion comments are checked.
  • Validate locally with gofmt -l ., go vet ./..., and go test ./....

Proceeding in this session per direct user instruction.

## Implementation Plan - Update `AGENT.md` reviewer steps so PR discussion comments are part of required review context. - Update `.codex/skills/review-pr/SKILL.md` workflow to load PR discussion comments explicitly. - Add a short rule in `review-pr` that verdict posting happens only after PR discussion comments are checked. - Validate locally with `gofmt -l .`, `go vet ./...`, and `go test ./...`. Proceeding in this session per direct user instruction.
codex commented 2026-04-28 06:06:17 +07:00
Author
Collaborator
Copy Link
Copy Source

Implemented on branch feat/adr-workflow and delivered via PR #15.

What changed:

  • AGENT.md reviewer section now explicitly requires reading PR discussion comments.
  • .codex/skills/review-pr/SKILL.md now loads PR discussion comments and states that reviews, inline review comments, and PR discussion comments are separate context sources.
  • review-pr rules now state that verdict posting happens only after PR discussion comments are checked.

Validation run locally:

  • gofmt -l .: OK
  • go vet ./...: OK
  • go test ./...: OK
Implemented on branch `feat/adr-workflow` and delivered via PR #15. What changed: - `AGENT.md` reviewer section now explicitly requires reading PR discussion comments. - `.codex/skills/review-pr/SKILL.md` now loads PR discussion comments and states that reviews, inline review comments, and PR discussion comments are separate context sources. - `review-pr` rules now state that verdict posting happens only after PR discussion comments are checked. Validation run locally: - `gofmt -l .`: OK - `go vet ./...`: OK - `go test ./...`: OK
codex closed this issue 2026-04-28 06:06:54 +07:00
codex reopened this issue 2026-04-28 06:10:10 +07:00
codex commented 2026-04-28 06:10:15 +07:00
Author
Collaborator
Copy Link
Copy Source

Reopening this issue.

The process update commit aa61352 was pushed after PR #15 had already merged, so the fix is not present on main. I am opening a separate PR from origin/main that contains only this reviewer-workflow change.

Reopening this issue. The process update commit `aa61352` was pushed after PR #15 had already merged, so the fix is not present on `main`. I am opening a separate PR from `origin/main` that contains only this reviewer-workflow change.
codex commented 2026-04-28 06:11:59 +07:00
Author
Collaborator
Copy Link
Copy Source

Split correctly into its own branch from origin/main.

Branch: fix/reviewer-pr-discussion-comments
Commit: 28224cc (docs: require PR discussion comments in reviews)

Opening a separate PR now so this change is no longer coupled to PR #15.

Split correctly into its own branch from `origin/main`. Branch: `fix/reviewer-pr-discussion-comments` Commit: `28224cc` (`docs: require PR discussion comments in reviews`) Opening a separate PR now so this change is no longer coupled to PR #15.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
claudecode codex opencode thuanle
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: thuanle/claw-email#16