Repository navigation
fix(workflows): report PR content integrity blockers accurately - #69866
Conversation
Keep approved integrity, stop after the first content denial, and distinguish reviewed, blocked, unavailable, and unattempted PRs. Preserve incomplete status and explicitly retain the gateway content-trust dependency. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Unable to retrieve PR metadata.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Design Decision Gate 🏗️Outcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
Comment MemoryPeek at saved memory (pr-code-quality-reviewer)Note This comment is managed by comment memory.Expand the saved memory block to view or edit the persistent context for this thread.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
The integrity/coverage direction is better, but one of the new instructions is still brittle enough to break discovery before the workflow reaches the reporting logic.
Blocking theme
- The prompt now reads like it expects
list_pull_requeststo support response field projection. If the agent turns that into extra request parameters, the run fails for a tool-contract reason instead of producing the bounded coverage report this PR is trying to guarantee.
Static review only; I did not execute the workflow.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 51.1 AIC · ⌖ 6.01 AIC · ⊞ 15.9K
Comment /review to run again
| Use GitHub MCP tools to list open pull requests from `${{ github.repository }}` with at most | ||
| `perPage: 5`, ordered by `created_at` descending. Record: | ||
| Use GitHub MCP `list_pull_requests` for `${{ github.repository }}` with `state: open`, | ||
| `sort: created`, `direction: desc`, and `perPage: 5`. Request only metadata fields: |
There was a problem hiding this comment.
This wording reads like an instruction to pass a field-selection argument to list_pull_requests, which can make discovery fail before the workflow even reaches the new coverage/reporting logic.
💡 Why this is risky and how to fix it
The MCP call contract here was previously just sort/pagination. Adding Request only metadata fields is easy for an agent to interpret as "ask the tool for a reduced schema", but the workflow does not establish that list_pull_requests supports field projection. If the agent tries to send unsupported selector parameters, the run degrades into an immediate tool-call error and reports incomplete coverage for the wrong reason.
Please rephrase this to operate on the response instead of on the request, for example: "From the returned metadata, record only number, title, user, and html_url." That preserves the bounded-discovery rule without steering the agent toward an invalid API shape.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs (root-cause fix of reporting failure in #69862) and /grill-with-docs (new concepts: four coverage states, integrity-content boundary). No blocking issues found.
📋 Verification performed
- Ran the new
scripts/test_daily_pr_review_integrity.pyagainst the actual committed PR content (the local working tree had unrelated uncommitted reverts that initially made it look failing — verified againstgit show HEAD:...instead): all 9 tests pass. - Confirmed
min-integrityandguard-policiesare real, supported compiler fields (pkg/workflow/cache_integrity.go,compiler_difc_proxy.go), not invented config. - Confirmed the pinned
gh-aw-mcpg v0.4.35reference in the prompt matchesconstants.DefaultMCPGatewayVersioninpkg/constants/version_constants.go— the stated content-labeling boundary claim is accurate, not stale. - Confirmed
report_incompleteis auto-defaulted/built-in (pkg/workflow/imports.go,missing_issue_reporting.go), so the prompt correctly relies on it without a new frontmatter declaration. - The string-matching regression-test style (asserting exact prose fragments against the compiled prompt) follows existing precedent in
scripts/test_model_routing_golden.pyandscripts/test_super_linter_scope.py, consistent with repo conventions.
✅ Positive highlights
- Clear separation of the four coverage states (Reviewed/Blocked/Unavailable/Not attempted) prevents the exact failure mode from run 39101615256 (treating unattempted PRs as denied).
- Explicit prohibition on alternate transports (
get_files, shell, local checkout, raw URLs, pre-fetched patches) after a denial closes an obvious workaround path. - PR body is transparent about the unresolved dependency (gateway content trust) rather than overclaiming the fix restores meaningful diff review.
Approving — no actionable issues identified on the high-impact files (daily-pr-review-cursor.md, the new test script, and the regenerated lock).
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 63.7 AIC · ⌖ 14.8 AIC · ⊞ 10.1K
Comment /matt to run again
There was a problem hiding this comment.
🟡 Changes recommended
The compiled policy broadens repository access on visibility-check failures, and the new regression suite is not registered for automated validation.
2 open issues
What changed in this PR
Updates daily PR quality reporting for #69862 to distinguish integrity blockers from unattempted reviews. Meaningful diff review remains unavailable.
Changes:
- Makes approved integrity explicit and bounds discovery to five PRs.
- Stops after the first integrity denial and reports coverage accurately.
- Adds nine contract checks and regenerates the workflow lock.
| File | Description |
|---|---|
scripts/test_daily_pr_review_integrity.py |
Adds integrity and coverage contract checks. |
.github/workflows/daily-pr-review-cursor.md |
Defines bounded reads, coverage statuses, and incomplete reporting. |
.github/workflows/daily-pr-review-cursor.lock.yml |
Regenerates executable workflow configuration. |
Review effort: Max
| github: | ||
| mode: local | ||
| toolsets: [repos, pull_requests] | ||
| min-integrity: approved |
| if __name__ == "__main__": | ||
| unittest.main() |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
🎉 This pull request is included in a new release. Release: |
Summary
Mitigates the Daily PR Code Quality Review reporting failure in #69862 without weakening integrity protections. This is a reporting/fail-fast fix, not a restoration of meaningful diff review.
min-integrity: approvedexplicitly and bound discovery to five open-PR metadata records.N/Arather than clean quality signals for unread content, and retainreport_incompletewhenever coverage is incomplete.Root cause and unresolved dependency
Run 39101615256 listed PR metadata but could not read diff/file content under
approvedintegrity. Actual attempts:get_difffor #69848/#69831 andget_filesfor #69813. #69772/#69586 were not attempted.The pinned MCPG v0.4.35 content-labeling boundary assigns
noneintegrity topull_request_readdiff/file content independently of PR metadata trust. Selecting member-authored or same-repository PRs, or inventing approval labels, cannot resolve this. Meaningful diff review remains unavailable until the gateway provides content-specific trust compatible withapproved. No alternate transport, trust mutation, integrity relaxation, live run, or membership/approval change is included.Validation
python3 scripts/test_daily_pr_review_integrity.py: 9 passed.make recompile: 344/344 workflows compiled; only this workflow's generated lock changed../gh-aw compile daily-pr-review-cursor --dry-run --schedule-seed github/gh-aw --no-check-update: passed, including native ShellCheck; existing low-severity experimental-feature notice remains.PATH="/opt/homebrew/bin:$PATH" make agent-report-progress: passed; no changed Go files, so impacted Go tests were skipped; all workflow locks are in sync.concurrency.queuesyntax diagnostics.Authored-source security overview
approved.