Skip to content

Drop @dependabot close comment from prerelease close path #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zkoppert merged 1 commit into from
Jun 15, 2026
Merged

Drop @dependabot close comment from prerelease close path #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions .copilot/skills/triage-dependabot/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ by `dependabot[bot]` or `dependabot-preview[bot]`:
| Title or body references an excluded dependency AND notification reason is `mention`, `team_mention`, `author`, or `manual` | skip, leave notification in inbox for direct response |
| Draft PR | flag-for-review |
| Any non-bot human (other than me) reviewed or commented | flag-for-review |
| Target version is a prerelease (alpha / beta / rc / dev / preview) | close-prerelease (comment `@dependabot close` and force-close via `gh pr close`) |
| Target version is a prerelease (alpha / beta / rc / dev / preview) | close-prerelease (force-close via `gh pr close --delete-branch`) |
| `mergeStateStatus` is `behind` or `dirty` | rebase (suppressed if my last rebase comment is newer than the latest dependabot push) |
| Bump is major / minor / unknown AND repo coverage threshold below 90 | flag-for-review |
| CI status pending | skip this run; next hour retries |
Expand All @@ -52,19 +52,20 @@ unstable release. Recognized prerelease forms:

Docker build variants (`-slim`, `-alpine`, `-bookworm`) and PEP 440
post-releases (`1.0.0.post1`) are NOT treated as prereleases. The action
posts `@dependabot close` (so Dependabot's tracking records the
directive) and then immediately calls `gh pr close --delete-branch` to
force the PR shut via the API. The direct close exists because
Dependabot has historically ignored the comment for hours (see
calls `gh pr close --delete-branch` to force the PR shut via the API.
The script used to also post `@dependabot close` first, but Dependabot
has historically ignored that comment for hours (see
`github-community-projects/contributors#496`, where the hourly cron
posted the directive 12+ times before the PR actually closed). Only a
narrow "already closed / not found" race on the close call is
swallowed; any other failure (auth, rate limit, timeout, branch
deletion) propagates so the outer run loop records it and the next
cron tick retries instead of silently treating the PR as handled.
Dependabot may open a new PR if the upstream releases another
prerelease, and the next run closes that one too. For a permanent
skip, add an `ignore` rule in the repo's `.github/dependabot.yml`.
posted the directive 12+ times before the PR actually closed) and once
the direct close lands the comment is pure noise on the PR timeline, so
the comment was dropped. Only a narrow "already closed / not found"
race on the close call is swallowed; any other failure (auth, rate
limit, timeout, branch deletion) propagates so the outer run loop
records it and the next cron tick retries instead of silently treating
the PR as handled. Dependabot may open a new PR if the upstream
releases another prerelease, and the next run closes that one too. For
a permanent skip, add an `ignore` rule in the repo's
`.github/dependabot.yml`.

### Excluded dependencies

Expand Down Expand Up @@ -94,10 +95,9 @@ conservative default.
notification stays open so the next push triggers another evaluation.
- **Label-and-merge**: `gh pr edit --add-label release` (only if the
repo defines a `release` label) followed by auto-merge.
- **Close-prerelease**: `gh pr comment --body "@dependabot close"`
followed by `gh pr close --delete-branch` (force-close via the API,
so we do not depend on Dependabot acting on the comment) for PRs
whose target version is an alpha / beta / rc / dev / preview;
- **Close-prerelease**: `gh pr close --delete-branch` (force-close via
the API, so we do not depend on Dependabot acting on a comment) for
PRs whose target version is an alpha / beta / rc / dev / preview;
notification is marked done and the cooldown is applied.
- **Flag-for-review**: a Q1 entry in
`~/repos/zkoppert-todo/todo.yml` under
Expand Down
12 changes: 6 additions & 6 deletions .copilot/skills/triage-dependabot/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ upgrades or seeing a backlog of dependabot notifications.
- `label-and-merge` - add the `release` label (if the repo defines
one) and enable auto-merge, for changes the Copilot sub-agent or
fallback regex classifies as security-related.
- `close-prerelease` - comment `@dependabot close` and then
force-close via `gh pr close --delete-branch` when the target
version is a prerelease (alpha / beta / rc / dev / preview).
Catches PRs like `bump python from 3.14.5-slim to 3.15.0b2-slim`
that should never auto-merge. The direct close exists because
Dependabot has historically ignored the comment for hours.
- `close-prerelease` - force-close via
`gh pr close --delete-branch` when the target version is a
prerelease (alpha / beta / rc / dev / preview). Catches PRs like
`bump python from 3.14.5-slim to 3.15.0b2-slim` that should
never auto-merge. The direct API close exists because Dependabot
has historically ignored `@dependabot close` comments for hours.
- `flag-for-review` - write a Q1 entry to
`~/repos/zkoppert-todo/todo.yml` for human attention.
4. Marks the notification done on GitHub when an action runs.
Expand Down
51 changes: 29 additions & 22 deletions .copilot/skills/triage-dependabot/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2889,7 +2889,7 @@ def test_load_and_write_todo_roundtrips_ruby_method_and_backticks(


# ---------------------------------------------------------------------------
# Bug 5 / FR: prerelease (alpha/beta/rc/dev/preview) bumps -> @dependabot close
# Bug 5 / FR: prerelease (alpha/beta/rc/dev/preview) bumps -> force-close via gh pr close
# ---------------------------------------------------------------------------


Expand Down Expand Up @@ -2987,32 +2987,37 @@ def test_do_dependabot_close_dry_run_no_subprocess() -> None:
mocked.assert_not_called()


def test_do_dependabot_close_posts_correct_comment() -> None:
def test_do_dependabot_close_force_closes_via_api() -> None:
with mock.patch.object(td, "run_gh") as mocked:
td.do_dependabot_close("github-community-projects/stale-repos", 520, dry_run=False)
assert mocked.call_count == 2
comment_call, close_call = mocked.call_args_list
comment_args = comment_call[0][0]
assert comment_args[:2] == ["pr", "comment"]
assert "520" in comment_args
assert "--repo" in comment_args
assert "github-community-projects/stale-repos" in comment_args
assert "--body" in comment_args
body_idx = comment_args.index("--body")
assert comment_args[body_idx + 1] == "@dependabot close"
close_args = close_call[0][0]
assert mocked.call_count == 1
close_args = mocked.call_args_list[0][0][0]
assert close_args[:2] == ["pr", "close"]
assert "520" in close_args
assert "--repo" in close_args
assert "github-community-projects/stale-repos" in close_args
assert "--delete-branch" in close_args


def test_do_dependabot_close_does_not_post_dependabot_close_comment() -> None:
"""Regression guard for the comment-spam pathology. Posting
``@dependabot close`` is pure noise on the PR timeline now that
``gh pr close --delete-branch`` closes the PR directly, so the
comment must never be sent."""
with mock.patch.object(td, "run_gh") as mocked:
td.do_dependabot_close("github-community-projects/stale-repos", 520, dry_run=False)
for call in mocked.call_args_list:
args = call[0][0]
assert args[:2] != ["pr", "comment"], (
"do_dependabot_close must not post any PR comment; got "
f"{args!r}"
)


def test_do_dependabot_close_swallows_already_closed_error() -> None:
"""If ``gh pr close`` fails with an 'already closed' stderr (a benign
race where Dependabot or a parallel run closed the PR between the
comment and the close), the comment has still landed and the tool
must not crash."""
race where Dependabot or a parallel run closed the PR before we
got there), the tool must not crash."""

def _run_gh_side_effect(args: list[str], *, timeout: int = 60) -> str:
if args[:2] == ["pr", "close"]:
Expand All @@ -3027,7 +3032,7 @@ def _run_gh_side_effect(args: list[str], *, timeout: int = 60) -> str:
td.do_dependabot_close(
"github-community-projects/contributors", 496, dry_run=False
)
assert mocked.call_count == 2
assert mocked.call_count == 1


def test_do_dependabot_close_propagates_real_close_failure() -> None:
Expand Down Expand Up @@ -3071,9 +3076,10 @@ def _run_gh_side_effect(args: list[str], *, timeout: int = 60) -> str:


def test_run_closes_prerelease_and_marks_notification_done(tmp_path: Path) -> None:
"""End-to-end: a Dependabot PR targeting a beta version posts
'@dependabot close', marks the notification done, and applies the
cooldown so the next run skips it."""
"""End-to-end: a Dependabot PR targeting a beta version is
force-closed via ``gh pr close --delete-branch``, the notification
is marked done, and the cooldown is applied so the next run skips
it."""
notif = {
"id": "thread-prerelease",
"reason": "subscribed",
Expand Down Expand Up @@ -3219,8 +3225,9 @@ def test_run_close_prerelease_cooldown_set_when_mark_done_fails(
Without this guard a notifications-API hiccup right after the
direct close would leave the cooldown unset, and the next cron
tick (within 1 hour) could re-encounter the still-listed
notification, re-post the @dependabot close comment, and reproduce
the contributors#496 spam pattern."""
notification and re-run the close path against an already-closed
PR (reproducing the contributors#496 churn pattern from a
different angle)."""
notif = {
"id": "thread-prerelease-flaky-mark",
"reason": "subscribed",
Expand Down
59 changes: 19 additions & 40 deletions .copilot/skills/triage-dependabot/triage_dependabot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ def is_prerelease_target(pr: dict[str, Any]) -> bool:

Scans both the title and body so single-package PRs and grouped PRs
are both detected. Conservative: any prerelease target anywhere in
the PR text routes the PR to ``@dependabot close`` rather than
the PR text routes the PR to the close-prerelease branch rather than
auto-merging an unstable release.
"""
title = pr.get("title") or ""
Expand Down Expand Up @@ -1203,53 +1203,32 @@ def _is_already_closed_stderr(stderr: str) -> bool:


def do_dependabot_close(repo: str, number: int, *, dry_run: bool) -> None:
"""Close a prerelease bump PR via both a directive comment and a direct close.

Two-step closure:

1. Post ``@dependabot close`` so Dependabot's own tracking sees the
directive (helps it record that the PR was intentionally closed
and avoid immediately re-opening for the same prerelease).
2. Call ``gh pr close --delete-branch`` to force-close the PR
directly via the API. Dependabot has historically ignored the
``@dependabot close`` comment for hours (see
github-community-projects/contributors#496 where the cron posted
the directive 12+ times before the PR actually closed), so the
direct close guarantees the PR shuts on the first cron tick and
stops the comment spam.

Only the narrow "already closed / not found" race is swallowed (e.g.
a parallel run, or Dependabot acting on the comment between the two
calls). Every other ``gh pr close`` failure - auth, rate limit,
timeout, transient API error, branch-deletion failure - propagates
so the outer run loop records it in ``stats.errors`` and skips the
``mark_thread_done`` + cooldown that would otherwise hide the open
PR until the next cron tick.
"""Force-close a prerelease bump PR via the GitHub API.

Calls ``gh pr close --delete-branch`` to shut the PR directly. We
used to also post ``@dependabot close`` first, but Dependabot
historically ignored that comment for hours (see
github-community-projects/contributors#496 where the cron posted
the directive 12+ times before the PR actually closed) and once the
direct close lands the comment is pure noise on the PR timeline, so
the comment was dropped.

Only the narrow "already closed / not found" race is swallowed
(e.g. a parallel run, or Dependabot closing the PR itself between
cron ticks). Every other ``gh pr close`` failure - auth, rate
limit, timeout, transient API error, branch-deletion failure -
propagates so the outer run loop records it in ``stats.errors`` and
skips the ``mark_thread_done`` + cooldown that would otherwise hide
the open PR until the next cron tick.

If the upstream releases another prerelease later Dependabot may
open a new PR; the next run of this tool will close that one too.
For a permanent skip, add an ``ignore`` rule for prereleases in the
repo's ``.github/dependabot.yml``.
"""
if dry_run:
logger.info(
"dry-run: would post '@dependabot close' and force-close %s#%d",
repo,
number,
)
logger.info("dry-run: would force-close %s#%d", repo, number)
return
run_gh(
[
"pr",
"comment",
str(number),
"--repo",
repo,
"--body",
"@dependabot close",
],
timeout=30,
)
try:
run_gh(
[
Expand Down
Loading