fix: make anti-slop advisory instead of auto-closing legitimate PRs#741
fix: make anti-slop advisory instead of auto-closing legitimate PRs#741anandgupta42 merged 1 commit intomainfrom
Conversation
The `anti-slop` workflow was closing legitimate team-member PRs within two minutes of opening. Root cause is a self-inflicted honeypot: the repo's `pull_request_template.md` embeds an HTML comment instructing any LLM/AI model to prepend the word "PINEAPPLE" to the PR description, and `anti-slop.yml` lists that same word in `blocked-terms`. Any AI-assisted PR from a team member follows the template instruction verbatim and then trips the blocked-terms check — a guaranteed close. PR #739 (an 8-file, 496-line BQ finops fix with a passing consensus code review and all other CI green) was auto-closed this way. There is no `TEAM_MEMBERS` carve-out in `anti-slop.yml` the way `pr-standards.yml` has one, so even listed members are subject to the auto-close. The action's `authorAssociation` check only recognizes GitHub's built-in OWNER/MEMBER/COLLABORATOR, not the repo's local `.github/TEAM_MEMBERS` file. Change: flip `close-pr: true` -> `close-pr: false`. All other anti-slop behavior is preserved — failure labels still land (`needs-review:blocked`), the failure message still comments on the PR, and the action still runs on every open/edit/synchronize. Maintainers can close manually when the signal is real. This keeps the anti-spam signal without the false-positive close. Two follow-ups are noted in the in-file comment and issue #740 but left out of scope here: 1. Whether to keep `PINEAPPLE` in `blocked-terms` at all given that the template actively induces its inclusion. 2. Whether to add a `TEAM_MEMBERS` carve-out to anti-slop mirroring `pr-standards.yml`'s pattern. Closes #740 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/anti-slop.yml (2)
18-20:⚠️ Potential issue | 🟡 MinorStale comment:
max-failures: 4no longer "closes PR" after the change.The comment at Lines 18-19 still describes this threshold as closing the PR, but with
close-pr: falsethe threshold now only gates labeling/commenting. Minor doc drift worth fixing while you're here.✏️ Suggested tweak
# --- Failure threshold --- - # Close PR after 4+ failed checks (default) + # Trigger failure actions (label + comment) after 4+ failed checks (default) max-failures: 4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/anti-slop.yml around lines 18 - 20, The comment describing the "max-failures: 4" behavior is stale; update the comment to reflect that max-failures only gates labeling/commenting when close-pr: false rather than closing the PR. Edit the YAML comment near the max-failures entry (reference: max-failures and close-pr) to remove "Close PR after 4+ failed checks (default)" and replace it with wording that documents the current behavior (e.g., "Trigger labeling/commenting after 4+ failed checks when close-pr: false" or similar) so the comment accurately matches the configuration.
89-97:⚠️ Potential issue | 🟡 MinorUpdate the failure message — it still claims the PR was auto-closed.
With
close-pr: false, PRs are no longer auto-closed, but the message at Line 90 still says "This PR was automatically closed by our quality checks." This will confuse contributors whose PRs remain open but receive this comment. Consider rewording to reflect the new advisory behavior (e.g., "This PR was flagged by our quality checks" and drop/replace the closure framing).✏️ Suggested rewording
failure-pr-message: | - 👋 This PR was automatically closed by our quality checks. + 👋 This PR was flagged by our automated quality checks and labeled `needs-review:blocked` for maintainer review. Common reasons: - New GitHub account with limited contribution history - PR description doesn't meet our guidelines - Contribution appears to be AI-generated without meaningful review - If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. + A maintainer will take a look. If you believe this was flagged in error, please add a comment explaining your intended contribution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/anti-slop.yml around lines 89 - 97, The failure-pr-message still says "This PR was automatically closed..." even though close-pr: false; update the failure-pr-message text (the multi-line value for failure-pr-message) to remove closure language and reflect advisory behavior (e.g., "This PR was flagged by our quality checks" and explain next steps), ensuring the message aligns with close-pr: false and keeps the listed common reasons and guidance for contributors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/anti-slop.yml:
- Around line 18-20: The comment describing the "max-failures: 4" behavior is
stale; update the comment to reflect that max-failures only gates
labeling/commenting when close-pr: false rather than closing the PR. Edit the
YAML comment near the max-failures entry (reference: max-failures and close-pr)
to remove "Close PR after 4+ failed checks (default)" and replace it with
wording that documents the current behavior (e.g., "Trigger labeling/commenting
after 4+ failed checks when close-pr: false" or similar) so the comment
accurately matches the configuration.
- Around line 89-97: The failure-pr-message still says "This PR was
automatically closed..." even though close-pr: false; update the
failure-pr-message text (the multi-line value for failure-pr-message) to remove
closure language and reflect advisory behavior (e.g., "This PR was flagged by
our quality checks" and explain next steps), ensuring the message aligns with
close-pr: false and keeps the listed common reasons and guidance for
contributors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 638c914a-b258-4eed-8bdd-7290f433155f
📒 Files selected for processing (1)
.github/workflows/anti-slop.yml
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/anti-slop.yml">
<violation number="1" location=".github/workflows/anti-slop.yml:86">
P3: Update the failure message to match the new advisory behavior; it still tells contributors the PR was automatically closed even though closure is now disabled.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -77,7 +77,13 @@ jobs: | |||
| master | |||
There was a problem hiding this comment.
P3: Update the failure message to match the new advisory behavior; it still tells contributors the PR was automatically closed even though closure is now disabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/anti-slop.yml, line 86:
<comment>Update the failure message to match the new advisory behavior; it still tells contributors the PR was automatically closed even though closure is now disabled.</comment>
<file context>
@@ -77,7 +77,13 @@ jobs:
+ # blocked-terms check and gets closed 2 minutes after opening. The
+ # labels and failure message still fire, so maintainers retain the
+ # signal and can close manually when warranted. See issue #740.
+ close-pr: false
lock-pr: false
failure-add-pr-labels: "needs-review:blocked"
</file context>
Patch release. User-facing: BigQuery finops tools were 100%-broken and are now fixed, and work in any BigQuery region (previously hardcoded to US). Changes since v0.6.0: - fix: BigQuery finops SQL — correct INFORMATION_SCHEMA columns + multi-region support (#739, closes #738). `error_message`/`error_code`/`total_rows` bugs plus `state='SUCCESS'` vs BQ's actual `'DONE'` (every completed job was being reported as FAILED). All 5 finops modules now read the BQ connection's `location` via a sanitised `{region}` placeholder. - fix: make anti-slop workflow advisory instead of auto-closing legitimate PRs (#741, closes #740). - fix: marker-guard hotfix for isValidDatabricksHost env-fallback path. - docs: 12 end-to-end showcase examples + `location` documented for BQ (#742). Review: 5-persona review (CTO/PM/DE/Tech Lead/Chaos Gremlin) ran on main — no P0s, no HOLDs. 1 P1 (silent `us` fallback compliance concern — addressed via docs + changelog warnings; behaviour-change follow-ups tracked in #754). 2 HIGH docs-accuracy items fixed pre-tag: `location` row in drivers.md and finops-tools.md. Tests: - 47 new adversarial tests in test/skill/release-v0.6.1-adversarial.test.ts (sanitizeBqRegion injection vectors, interpolateBqRegion idempotency, bqRegionFor registry edge cases, BIGQUERY_HISTORY_SQL column-name regression guards for all four #739 bugs, cross-module {region} guard, buildHistoryQuery full-pipeline incl. Snowflake/Databricks no-regression). - 409/409 tests pass across finops + skill suites. Full altimate suite previously green (2917/2917) on #739 merge. - Typecheck: 5/5 pass. Marker guard: clean. Pre-release sanity: all 4 green. Deferred: BQ finops UX + robustness follow-ups filed as #754 (surface queried region in responses, distinguish no-location vs invalid-location, warn at warehouse_add time, friendlier perms error, options-object refactor, e2e silent-skip warning). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
anti-slop.ymlauto-closes legitimate team-member PRs within ~2 minutes. Root cause is a self-inflicted honeypot:pull_request_template.mdinstructs any LLM to prepend "PINEAPPLE" to the description, andanti-slop.yml:50lists that same word inblocked-terms. AI-assisted PRs from team members follow the template instruction and then trip the check — a guaranteed close. PR #739 (a legitimate BQ finops fix with a passing consensus code review and all other CI green) was closed this way.anti-slophas noTEAM_MEMBERScarve-out the waypr-standards.ymldoes, so listed members are not exempt. The action's built-in association check only recognizes GitHub'sOWNER/MEMBER/COLLABORATOR, not the repo's local.github/TEAM_MEMBERSfile.Change: flip
close-pr: truetoclose-pr: false. All other signals remain — theneeds-review:blockedlabel still lands, the failure message still comments, the action still runs on every open/edit/synchronize. Maintainers can close manually when warranted.Test plan
bun run script/upstream/analyze.ts --markers --base main --strict).anti-slopis the only workflow withclose-pr: truein the fast path (the other auto-closer,close-stale-prs.yml, is schedule-driven and unaffected).Checklist
Follow-ups left out of scope and tracked in the issue:
PINEAPPLEshould stay inblocked-termswhen the template actively induces its inclusion.TEAM_MEMBERScarve-out toanti-slopmirroringpr-standards.yml's pattern.Closes #740
Summary by CodeRabbit
Summary by cubic
Make the
anti-slopworkflow advisory by settingclose-pr: falseto stop auto-closing legitimate team PRs triggered by the PR template’s “PINEAPPLE” word. The check still runs on open/edit/sync, adds theneeds-review:blockedlabel, and posts the failure message so maintainers can review and close manually.Written for commit 0f66ba7. Summary will update on new commits.