Require PR review completion comments in github-pr-review skill

[neubig]

Summary

• update github-pr-review skill instructions to require a completion comment on the PR issue after posting a review
• align the skill README with the new completion-comment guidance
• make the docs explicit that the review is not complete unless both the review object and completion comment were created

Testing

• not run (skill/documentation change only)

This PR was created by an AI agent (OpenHands) on behalf of the user.

[all-hands-bot]

🟡 Acceptable - Technically sound but raises pragmatic concerns about necessity.

[all-hands-bot]

Completed OpenHands review: submitted a COMMENT review on commit f70256e.

[all-hands-bot]

Additional Review Analysis

[IMPROVEMENT OPPORTUNITIES]

• Pragmatism: The completion comment requirement may be solving an imaginary problem. GitHub reviews already trigger notifications and appear in the timeline. Consider documenting the specific problem this addresses or making it optional.
• Clarity: The example placeholders ({COMMENT|APPROVE|REQUEST_CHANGES}, {commit_sha}) could be misinterpreted - clarify they should match values from the review JSON.
• Error Handling: "Surface the failure clearly" is vague - specify what agents should do when API calls fail.

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟢 LOW

This is a documentation/skill change only - no executable code is modified. Agents following the new instructions will post an extra PR comment, which adds minor noise but does not break functionality. The change is backwards-compatible (existing reviews still work, they just won't have the completion comment).

VERDICT:
✅ Worth merging with clarifications: The change is technically sound and might serve a purpose in agent-driven workflows, but the PR would benefit from explaining WHY the completion comment is required.

KEY INSIGHT:
Adding mandatory process steps should be justified - document the specific problem being solved so future maintainers understand the reasoning and don't remove it as "unnecessary complexity."

---

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Completion comments are required because [specific reason]"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

[neubig]

@OpenHands /iterate

[openhands-ai]

I'm on it! neubig can track my progress at all-hands.dev

[neubig]

Addressed review feedback in 08a448e: documented why the completion comment exists, clarified that it must reuse the review event and commit SHA, and made the failure handling contract explicit. Ready for another look.

This comment was created by an AI agent (OpenHands) on behalf of the user.

[all-hands-bot]

🟡 Acceptable - Clear implementation but raises pragmatic concerns about adding mandatory overhead for a problem that may be better solved at the orchestration layer.

[all-hands-bot]

🟡 Suggestion: The error guidance "keep trying or ask for help" is underspecified. Consider clarifying:

• Retry logic (how many attempts? exponential backoff?)
• Which HTTP status codes warrant retry (e.g., 429, 502, 503) vs abort (e.g., 401, 403, 404)?
• How to avoid posting duplicate comments if retrying

This matters because agents need concrete retry contracts, not open-ended "keep trying" instructions.

[all-hands-bot]

🟡 Style: File should end with a newline character (POSIX standard).

Suggested change

	10. Do not finish unless both the review object and completion comment were created successfully
	10. Do not finish unless both the review object and completion comment were created successfully

[all-hands-bot]

🟡 Style: File should end with a newline character (POSIX standard).

Suggested change

	8. Do not finish unless both the review object and completion comment were created successfully
	8. Do not finish unless both the review object and completion comment were created successfully

[all-hands-bot]

Completed OpenHands review: submitted a COMMENT review on commit 08a448e.

[RISK ASSESSMENT]
🟢 LOW - Documentation-only change with no code execution. Adds workflow overhead but does not break existing functionality or introduce security risks.

VERDICT:
✅ Worth merging - Documentation is clear and consistent. The mandatory completion comment adds friction, but the implementation is technically sound.

KEY INSIGHT:
This PR shifts the review completion contract from "API call succeeded" to "API call succeeded AND confirmation comment posted." While this adds a second failure mode, the error handling guidance could be more specific about retry logic and duplicate detection.

This review was created by an AI agent (OpenHands) on behalf of the user.

[openhands-ai]

Final summary since my last summary:

• I investigated the example workflow run you linked: OpenHands/agent-server-gui run 25133543954, job 73665828257.
• I confirmed that run completed successfully but produced no PR review and no completion comment on PR docs: add AGENTS.md and refresh README #46.
• I checked the workflow logs and found the run was using OpenHands/extensions@main at commit fd838919..., which predates the newer github-pr-review skill documentation change.
• I also inspected the plugins/pr-review runtime behavior and confirmed the core issue: the workflow currently marks success when the conversation finishes, but it does not verify that the agent actually created:
  • a PR review object, or
  • the follow-up completion comment.

Checklist:

• Determined whether the missing review was caused by the merged docs change not being present in that run.
• Determined whether the workflow runtime enforces review creation.
• Answered the root-cause question directly.

Conciseness:

• No new code changes were made in this investigation.
• No extraneous modifications were introduced.

Bottom line:

• The linked run did not include the newer skill-doc change.
• More importantly, the current plugin implementation has no runtime enforcement, so a run can go green even when no review is posted.
• The real fix should be implemented in plugins/pr-review/scripts/agent_script.py by verifying after the agent run that a new review (and optionally the completion comment) was actually created for the current PR/commit.