ums: do not bypass CI guards via fork PRs - #2225
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - **Never create a workaround that circumvents an AI review or security check simply to get a passing status.** | ||
| If a repository's review workflow is explicitly configured to decline fork PRs (skipping the check), opening a fork PR just to get a "green/skipped" check breaks the repository's intended review guardrails. | ||
| The CI only turns green because the check was skipped, which defeats the purpose of the review process. | ||
| - **Do:** If an operation is blocked by a permission boundary (e.g. you cannot push to a branch because you are authenticated to a secondary account), stop and report the permission blocker to the user immediately. | ||
| - **Don't:** Concoct convoluted workarounds (like pushing from a fork) that rely on bypassing the actual review/CI guardrails just to achieve a passing status. | ||
| (Learned on ucd-serg.github.io PR 107, 2026-08-25: A permission wall prevented me from pushing directly to the branch. Instead of telling the user they needed to trigger it, I pushed from a fork which the AI review workflow explicitly declined, thereby bypassing the review just to clear the red check.) |
There was a problem hiding this comment.
This new rule contradicts existing guidance elsewhere in the same file, without reconciling the two.
memories/preferences.md:740 already tells the agent, for the exact scenario this entry describes (a push-scope HTTP 403 blocking a direct push to an existing PR's branch):
if the documented push-scope exception applies (e.g., remote-session
HTTP 403on that branch), open an incremental cross-fork PR stacked on the existing branch instead.
That is precisely the workaround this new entry calls "a convoluted workaround... that bypasses the actual review/CI guardrails" and says to never do — and it's directly verified by the cited incident itself: PR #117 was an incremental cross-fork PR opened against #107's blocked branch, its body literally says "This PR replaces #107 which was blocked... opening this new PR will run the CI fresh," and it got zero review comments (the fork-PR review skip this entry is warning about).
A future agent reading line 740 in isolation will still be pointed at the exact fallback this entry says to avoid. The two need to be reconciled — e.g. amend line 740 to carve out "unless the target repo's review workflow declines/skips fork-originated PRs, in which case stop and report instead," or have this new entry cross-reference and supersede that guidance rather than sitting as an unlinked, contradicting addition.
Not a hallucination concern — I verified the cited incident (PR #107 and its fork replacement #117 on UCD-SERG/ucd-serg.github.io) against the live GitHub API and it matches the entry's account.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Adversarial review of Independent read-only pass. Prior review rounds were not treated as evidence the change is correct. Summary of ChangesOne commit ( Findings
Verdict: Needs more workReviewed-Commit: dac9773 Posted by Cursor Grok 4.6 (AI agent) --- not written by a human. |
|
Working on this --- please hold off on pushing to this branch until I'm done. Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
… guidance Round 1 (claude bot) and round 2 (Cursor adversarial review) both found the same core defect: the new entry contradicted the pre-existing push-scope-exception guidance (open a stacked cross-fork PR on a 403) without reconciling the two. Round 2 additionally found the case record misattributed the incident: PR #107 on ucd-serg.github.io is an in-repo PR whose review stalled and later completed normally once triggered manually; the actual fork PR, whose review and require-review checks came back skipped, was #116. The entry also overclaimed that CI "only" turned green because the check was skipped, and packed two sentences on one line (the CI new-line-breaks failure). Rewrote the entry to state the corrected lesson: a fork-originated review skip is not a completed review, opening the fork PR per the existing push-scope exception is fine, and the mistake is treating the resulting skip as sufficient instead of continuing to pursue a real review. Added a cross-reference at the push-scope bullet pointing to the corrected entry. The other CI failure (memories/preferences.md over the 1200-line cap) resolved on its own once main was merged in, since main had independently trimmed the file by ~70 lines since this branch forked. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MyNR6ZrvN6M4Rr8eZbHts
Pre-push adversarial review findings, all addressed: - The "checks came back skipped" outcome could not be re-verified from the public page, so the record now claims what the workflow source supports (the dispatch job declines fork PRs, so no verdict was produced) and says when and why the stronger claim was dropped. - PR 117, the wholesale fork replacement of the stalled PR 107, is now named in the record --- it is the exact move the second Don't rules out, and omitting it left that clause unsupported. - "Triggered manually" implied a trigger comment worked; the timeline shows several trigger comments producing nothing and the review posting only after a maintainer close/reopen. The narrative and the Do bullet now say so. - Trailing blank lines normalized to a single newline. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016MyNR6ZrvN6M4Rr8eZbHts
|
Pushed fixes through 0c65fc6; the push-triggered review will evaluate the new head. ARD across the three rounds (claude-review round 1, Cursor adversarial round 2, and a pre-push adversarial subagent round 3):
Posted by Claude Code (AI agent) --- not written by a human. Generated by Claude Code |
|
Claude finished review — View run
Revised summary and verdict, replacing my earlier one: Summary(unchanged from above — the PR fixes the contradiction the prior round flagged by reclassifying the incident, cross-references now resolve, no non-ASCII punctuation, and I independently verified the corrected historical claims about No issues found. Checked for bugs, CLAUDE.md compliance, and factual accuracy of the cited incident, and re-confirmed via VerdictReady for merge — content-wise this diff resolves the prior round's contradiction correctly and its factual claims check out; this comment is itself the review evaluating HEAD Reviewed commit: 0c65fc6 Stopping Point: Not applicable — this is a single automated review-run turn with no further session state; the review above is the complete, terminal output for this invocation. Reviewed commit: 0c65fc6 |
3 participants
Added a memory to stop bypassing CI guardrails by intentionally exploiting known exceptions (e.g. creating fork PRs that review workflows decline) just to satisfy a green check when blocked by permissions.