Skip fork PRs in Grumpy PR Reviewer instead of treating them as failures

[Copilot]

Proposed Changes

The Grumpy PR Reviewer ran on fork PRs via pull_request_target + roles: all, but the GitHub MCP server's integrity policy blocks reading a fork PR's diff (lower integrity than agent requires). The agent reported this blocked read as "missing data" and filed a failure issue on every fork PR.

• Gate to same-repository PRs: the activation if now requires the PR branch to live in the base repo, so fork PRs become a clean skipped no-op (no billed run, no error issue) instead of a spurious failure:

  if: >
    ${{ github.repository == 'ohcnetwork/care_fe' &&
        (github.event.pull_request == null ||
         github.event.pull_request.head.repo.full_name == github.repository) }}

• Dropped roles: all: its only rationale was reviewing community/fork PRs; reverting to the default [admin, maintainer, write] gate (which same-repo authors already satisfy) is now coherent. The compiler re-adds this via a pre_activation membership-check job.

• Updated description/comments in grumpy-reviewer.md to reflect same-repo-only scope.

• Recompiled grumpy-reviewer.lock.yml with the matching gh aw v0.85.4, so the generated diff contains only the intended changes.

Tagging: @ohcnetwork/care-fe-code-reviewers

Merge Checklist

• Add specs that demonstrate the bug or test the new feature.
• Update product documentation.
• Ensure that UI text is placed in I18n files.
• Prepare a screenshot or demo video for the changelog entry and attach it to the issue.
• Request peer reviews.
• Complete QA on mobile devices.
• Complete QA on desktop devices.
• Add or update Playwright tests for related changes

• Fixes [aw] Grumpy PR Reviewer is missing required data #16669

[Copilot]

Copilot wasn't able to review any files in this pull request.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cloudflare-workers-and-pages]

Deploying care-preview with    Cloudflare Pages

Latest commit:	c142de8
Status:	✅  Deploy successful!
Preview URL:	https://02334783.care-preview-a7w.pages.dev
Branch Preview URL:	https://copilot-feat14322home-page-r.care-preview-a7w.pages.dev

View logs

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Jacobjeevan]

@claude[agent] Alternatively, is there a way to include fork PRs as well? Adjust the security policy.

[Claude]

@claude[agent] Alternatively, is there a way to include fork PRs as well? Adjust the security policy.

Good question. It's possible, but I'd advise against simply loosening the guard, and here's why.

The block isn't a policy I hand-wrote in grumpy-reviewer.md — it's enforced by gh-aw's auto-generated lockdown step (determine_automatic_lockdown.cjs, which sets GITHUB_MCP_GUARD_MIN_INTEGRITY). Because the workflow triggers on pull_request_target, gh-aw automatically raises the MCP integrity floor when the head is a fork, so the GitHub MCP server refuses to hand the fork's diff to the agent. That's deliberate: pull_request_target runs in the base repo context with write scopes and access to COPILOT_GITHUB_TOKEN, while the diff/title/description are untrusted, attacker-controlled input. Lowering the integrity guard to feed that content to the model reintroduces a prompt-injection surface against a job that can post reviews and holds a secret — exactly what the guard exists to prevent.

The safer way to cover forks is the two-workflow split GitHub recommends: a pull_request workflow (no secrets, read-only) uploads the diff as a sanitized artifact, and a separate workflow_run workflow consumes that artifact in the trusted context to run the review. That keeps forks in scope without exposing secrets or write scopes to untrusted content, but it's a meaningfully larger change than this PR.

Given that, my recommendation is to keep this PR's same-repo-only fix (it stops the spurious failure issues today) and track fork coverage via the workflow_run split as a follow-up. Happy to open that as a separate PR if you'd like — let me know which way you'd prefer to go.

[github-actions]

CARE Review — Gate the Grumpy PR Reviewer to same-repo PRs so fork PRs skip cleanly instead of filing failure issues

Intent (high confidence): stop the spurious "missing data" failure issues on fork PRs by never activating on them, since gh-aw's pull_request_target lockdown makes the fork diff unreadable anyway. Lens 3 (UI/UX) skipped — no .tsx touched.

Approach: proportionate. The if gate is the smallest thing that fixes #16669, the comment rewrite explains why rather than what, and the lock file is a faithful recompile with the matching compiler version (v0.85.4, body_hash unchanged — only frontmatter_hash moved, which is exactly right for a frontmatter-only edit).

Two notes:

• One inline finding on dropping roles: all — it's a second, independent change riding along with the fork gate, and the pre_activation membership check it re-enables may skip bot-authored same-repo PRs (including this one's own author). Worth confirming or splitting.
• The if gate's github.event.pull_request == null arm correctly keeps workflow_dispatch working. Good.

Nothing else on the changed lines. The security reasoning in the new comment block is accurate and matches the workflow_run-split answer already given in the conversation thread.

Generated by CARE PR Reviewer for #16702 · opus50 · 77.5 AIC · ⌖ 5.37 AIC · ⊞ 19.4K

[github-actions]

Is the roles revert safe for bot-authored same-repo PRs?

The comment asserts a same-repo PR author "already has write access", but that is only reliably true for humans. Same-repo branches are also pushed by app/bot actors — the Copilot coding agent (which authored this very PR, head copilot/feat14322home-page-redesign) and Renovate. The recompiled lock adds a pre_activation job that gates on check_membership.cjs with admin,maintainer,write, and a GitHub App bot actor does not generally resolve to a repository collaborator permission level.

If that check resolves false, the reviewer silently skips exactly the PRs that most benefit from review — and the skip is invisible (no failure, no comment), which is the same failure mode this PR is fixing, just quieter.

The fork gate alone fixes #16669; dropping roles: all is a separate change riding along. Worth either keeping roles: all (the fork gate now makes it harmless, since untrusted heads never activate) or confirming a bot-authored same-repo PR still passes check_membership before merging.

[github-actions]

Conflicts have been detected against the base branch. Please merge the base branch into your branch.
cc: @copilot-swe-agent

See: https://docs.ohc.network/docs/contributing#how-to-resolve-merge-conflicts

[Jacobjeevan]

@copilot resolve the merge conflicts in this pull request

Recent PR was merged that recompiled the workflows.

[Copilot]

@copilot resolve the merge conflicts in this pull request

Recent PR was merged that recompiled the workflows.

Resolved the merge conflict in c142de85f. Merged the latest develop (which recompiled the workflows to gh aw v0.86.2) and recompiled grumpy-reviewer.md with the matching v0.86.2 compiler, so the lock file now carries both develop's compiler bump and this PR's same-repo/fork-gating changes. Unrelated recompiled files were restored to develop's state so the diff stays scoped to the grumpy reviewer.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[github-actions]

Well. I came here expecting to sharpen my knives over a home page redesign branch name, and instead got a workflow YAML gating fix. Fine.

The change itself is... actually reasonable. Skipping fork PRs at the activation if instead of letting the engine spin up, fail on the MCP integrity policy, and file a garbage issue on every community PR is the right call. The github.event.pull_request == null escape hatch for workflow_dispatch is correctly handled, and the comment block explaining why is unusually thorough — I begrudgingly approve of people who document their gating conditions.

Two things stopping me from a clean approval:

1. Dropping roles: all means every run now goes through check_membership on github.actor. Same-repo human authors are fine, but bot/app authors (like the one that opened this very PR) are a coin flip, and a failed membership check is a silent skip. Silent skips are how you end up with a reviewer bot that quietly stopped working three months ago and nobody noticed.
2. The description says v0.85.4, the lock header says v0.86.2. Pick one.

Nothing here is merge-blocking. Just do not make me come back and read a bug report about the bot mysteriously not running.

Generated by Grumpy PR Reviewer for #16702 · opus50 · 44.3 AIC · ⌖ 2.48 AIC · ⊞ 8.7K

[github-actions]

Dropping roles: all swaps one silent failure mode for another: the membership gate now runs against github.actor, and PRs opened by bots/apps (this very PR was authored by the Copilot app) are not guaranteed to resolve to admin/maintainer/write. When that check says no, the run is silently skipped with zero output — which is exactly the kind of thing someone will spend an afternoon debugging six months from now. Worth confirming the bot actors you care about actually pass check_membership.

[github-actions]

Generated file, so I will not yell about the contents — but the PR description says it was recompiled with gh aw v0.85.4 while this header says compiler_version: v0.86.2. One of the two is wrong, and stale-lockfile archaeology is nobody idea of a good time. Fix the description (or the recompile) so the next person can reproduce this diff.

[github-actions]

CARE Review — gate Grumpy PR Reviewer to same-repository PRs

Intent (high confidence): stop the reviewer from activating on fork PRs, where the MCP integrity policy blocks reading the diff and the agent files a spurious failure issue. Implemented as an added head.repo.full_name == github.repository clause on the top-level if:, plus removal of roles: all. The lock file is a faithful recompile of that frontmatter (v0.86.2) — no hand edits, and the extra hunks (pre_activation job, trace-id plumbing, GH_AW_NEEDS_PRE_ACTIVATION_OUTPUTS_ACTIVATED) are all compiler-generated consequences of dropping roles: all.

Approach: proportionate. The fix is one condition in the source file and a recompile; nothing new is abstracted, and the pull_request == null guard correctly keeps workflow_dispatch working.

Lens 3 (UI/UX): skipped — no .tsx changed.

The one thing worth confirming before merge

Dropping roles: all is a second, separable change riding along with the fork gate, and it is the only part of this diff that can change behavior on PRs you want reviewed. It reintroduces the pre_activation / check_membership gate with admin,maintainer,write — and the new source comment justifies it with "a same-repo PR's author already has write access", which holds for humans but not necessarily for GitHub App actors that push same-repo branches (the Copilot coding agent — which authored this very PR — and Renovate). If check_membership resolves false for those, the reviewer silently skips them: no failure, no comment, no signal. That is the same invisible-skip failure mode this PR is fixing, just quieter.

This was already raised in an existing review thread on grumpy-reviewer.md, so I won't duplicate it inline — flagging it here only because it is the single merge-blocking-ish question in the diff, and it is still open. Either keep roles: all (the fork gate now makes it harmless, since untrusted heads never activate at all) or verify one bot-authored same-repo PR actually activates after this lands.

Nothing else in the changed lines needs work. The prose comment in grumpy-reviewer.md is a genuine improvement — it now explains why fork PRs can't be reviewed rather than just asserting the repo check.

Generated by CARE PR Reviewer for #16702 · opus50 · 87.6 AIC · ⌖ 3.96 AIC · ⊞ 19.6K