Make android reviewer skill offline-only#11335
Conversation
Move GitHub publishing responsibility to the agentic workflow so local skill invocations only prepare findings while the /review workflow posts comments through safe outputs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the PR conversation comment terse so the submitted review remains the single detailed verdict summary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
✅ LGTM
Clean infrastructure change that properly separates the android-reviewer skill (analysis) from the workflow (publishing). All changes are internally consistent.
What this PR does well:
- Clear separation of concerns: SKILL.md prepares structured findings, workflow.md publishes via safe-output tools
- The new
add_commentsafe output (max: 1) gives reviewees a terse notification that a review was posted - The structured JSON schema in SKILL.md makes the skill reusable across different publishing contexts
- The new constraint "Do not use direct GitHub write APIs" is good defense-in-depth
CI: ✅ All checks passing (dotnet-android, license/cla)
| Severity | Count |
|---|---|
| 💡 suggestion | 1 |
One minor suggestion about making the copilot_pr_summary_prefix_required handling explicit in the workflow instructions. No blocking issues.
Generated by Android PR Reviewer for issue #11335 · ● 2.2M
| 3. Publish the prepared findings through safe-output tools: | ||
| - Use `add_comment` once to post a terse PR conversation comment stating that the `/review` run completed and that detailed findings are in the submitted review. | ||
| - Use `create_pull_request_review_comment` for each inline finding. | ||
| - Use `submit_pull_request_review` once with the overall review summary. Use `COMMENT` for clean or informational reviews and `REQUEST_CHANGES` when blocking issues are found. |
There was a problem hiding this comment.
🤖 💡 Documentation — The SKILL.md (line 135) introduces copilot_pr_summary_prefix_required and says "so a publishing workflow can prefix the review summary with @copilot ", but this step 3 doesn't mention consuming that flag. Since the agent reads both documents it would likely handle it implicitly, but consider adding an explicit instruction here, e.g.:
If the PR is authored by Copilot and the skill set
copilot_pr_summary_prefix_requiredtotrue, prefix the review summary body with@copilot.
This keeps the workflow self-contained and avoids relying on the agent inferring the connection between the two documents.
Rule: Docs describe intent not reality (Postmortem #59)
Keep structured JSON available for automation while making CLI/chat invocations readable by default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Validation