cron: wake the PR-activity gate on comments and reviews - #301
Open
polyglotAI-bot wants to merge 2 commits into
Open
cron: wake the PR-activity gate on comments and reviews#301polyglotAI-bot wants to merge 2 commits into
polyglotAI-bot wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the github_pr_activity cron gate so PR-monitor jobs wake on review/comment activity (not just state/CI/head changes), and adds an ignore_actors denylist to prevent bot chatter from triggering expensive runs.
Changes:
- Add
comments/reviewsactivity to the PR fingerprint used byGitHubPrActivityGate. - Add
ignore_actorsconfig support (including normalization and config validation) to suppress activity from specified actors. - Document the
github_pr_activitygate indocs/cron.mdand add regression-focused tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
nerve/cron/gates.py |
Extends fingerprint inputs to include comments/reviews and adds ignore_actors parsing/normalization. |
tests/test_cron_gates.py |
Adds regression tests ensuring comment/review activity affects the fingerprint and that ignored actors do not. |
docs/cron.md |
Documents github_pr_activity, including ignore_actors behavior and examples. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
Pushed a099f79 addressing both Copilot comments — details in the threads. Verification after the change:
|
The gate hashed state/reviewDecision/headRefOid/statusCheckRollup, so a reviewer's comment on one of the author's own PRs moved nothing and the monitor stayed asleep until the 8h force-run fired. It is tempting to leave comments to the `github` sync source instead, but that path does not reach a consumer for a PR we authored: the notification carries reason=author rather than mention, so an inbox cron that delegates own-PR activity to this monitor discards it — after its poll has already advanced the durable consumer cursor, so the signal is never re-delivered and nothing else observes it. `reviewDecision` is not a stand-in either: GitHub only populates it with APPROVED/CHANGES_REQUESTED, so a COMMENTED review leaves it unchanged. Add `comments` and `reviews` to the fingerprint, keyed on (timestamp, author, includesCreatedEdit) so an edited-in-place comment still moves the hash. Comment activity needs a mute knob that CI status does not: coverage, lint and AI-review bots comment on every push, and left unfiltered they would wake the job as reliably as a human. Hence `ignore_actors`, a denylist matched case-insensitively with a trailing `[bot]` ignored. A denylist and not autodetection, because bots are not reliably identifiable here: `author.is_bot` is null inside gh's comments/reviews payloads and GraphQL strips the `[bot]` login suffix, while `authorAssociation` reports real maintainers as CONTRIBUTOR — the same value as some review bots, so filtering on it would silently drop human feedback. An unlisted bot costs one extra wake; a wrong autodetect costs a missed review. The gate's own `author` is always ignored, so its own replies cannot wake it. `spec_keys` gains the new field so `nerve config validate` accepts it, and docs/cron.md now documents this gate's fields, which ClickHouse#123 never did. Follow-up to ClickHouse#123, which added the gate.
Review follow-up. `spec.get("ignore_actors") or []` took any falsy value —
`0`, `false`, `""`, `{}` — as "not set", so a misconfigured job came up with
an empty denylist and kept waking on every bot comment while the config said
otherwise. Only a bare `ignore_actors:` (None) means unset now, matching how
a bare `run_if:` is the one value that means "no gates"; everything else is
refused.
Entries must also be non-empty. That one is load-bearing rather than
cosmetic: `_actor` reports "" for a deleted/ghost user, so an empty string in
the denylist would have muted exactly the ghost-authored comments that
test_ghost_author_counts_as_activity says must count. from_config now refuses
such an entry, and __init__ drops it, so a direct caller cannot poison the
set either.
Also assert the `--json` field list by token rather than substring:
`"reviews" in "...,latestReviews"` is true, so the old check would have
passed for a field name `_pr_detail` never reads.
polyglotAI-bot
force-pushed
the
polyglot/pr-gate-comment-activity
branch
from
August 10, 2026 14:22
a099f79 to
6d87782
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
github_pr_activitygate (added in #123) fingerprintsstate,reviewDecision,headRefOidandstatusCheckRollup. A reviewer's comment moves none of those, so a PR-monitor cron gated on it stays asleep through review feedback until the 8hforce_run_after_hoursnet fires.This adds
comments/reviewsto the fingerprint, plus anignore_actorsdenylist so chatty bots don't undo the cost saving the gate exists for.Why the sync source can't cover this
The natural objection is that comments should arrive via the
githubsync source instead. They don't, for PRs the account itself authored: the notification carriesreason=author, notmention, so an inbox cron that delegates own-PR activity to the monitor discards it — after its poll has advanced the durable consumer cursor, so it is never re-delivered and nothing else observes it.reviewDecisionis not a stand-in either: GitHub only populates it withAPPROVED/CHANGES_REQUESTED, so aCOMMENTEDreview leaves it untouched.What changed
comments/reviewsin the fingerprint, keyed on(timestamp, author, includesCreatedEdit)— the edit flag means an edited-in-place comment still moves the hash.ignore_actors(optional): logins whose comments/reviews don't count. Case-insensitive, trailing[bot]ignored, socodecovalso coverscodecov[bot]. The gate's ownauthoris always ignored so its replies can't wake it.A denylist rather than bot autodetection on purpose:
author.is_botisnullingh's comments/reviews payloads and GraphQL strips the[bot]suffix, whileauthorAssociationreports real maintainers asCONTRIBUTOR— the same value as some review bots. An unlisted bot costs one extra wake; a wrong autodetect costs a missed review.spec_keysgains the new field, sonerve config validatedoesn't flag a config that uses it.docs/cron.mdgains agithub_pr_activitysection. Slight scope expansion — the gate was never documented there, and shipping a new config key with nowhere to document it seemed worse. Happy to split it out if you'd rather.Test plan
pytest tests/test_cron_gates.py— 79 passed (17 new)gates.pyalone leaves the new tests failing — 15 failed, 64 passednerve config validateagainst a live config usingignore_actors→Config OK; without thespec_keyschange the same config reportsunknown: ['ignore_actors']Follow-up to #123.