Ignore Dependabot security updates in the failure watcher - #200
Conversation
📝 WalkthroughWalkthroughThe Dependabot failure watcher documents run-title categories and filters recent failed runs to exclude security updates and selected e2e directories. ChangesDependabot failure watcher
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
🟡 Not ready to approve
The updated shell step can still incorrectly report success if gh/jq fails, undermining the watcher’s reliability unless strict/pipefail mode is enabled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Updates the Dependabot Failure Watcher workflow to avoid being kept perpetually red by Dependabot security-update failures, so genuine version-update scan failures remain visible and actionable.
Changes:
- Exclude Dependabot security-update runs (and selected noisy directories) by filtering on
displayTitle. - Add detailed inline documentation explaining the title-based heuristics and their constraints.
- Use
gh run list --createdto bound the query server-side instead of filtering all historical runs locally.
File summaries
| File | Description |
|---|---|
| .github/workflows/dependabot-failure-watcher.yml | Filters out security-update workflow runs by title and bounds the gh run list query by creation date, with extensive rationale documented inline. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| GH_TOKEN: ${{ github.token }} | ||
| REPO: ${{ github.repository }} | ||
| run: | | ||
| since=$(date -u -d '8 days ago' +%Y-%m-%dT%H:%M:%SZ) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/dependabot-failure-watcher.yml:
- Around line 88-98: The Dependabot run query must fail closed when the fetched
results may be truncated. After the `gh run list` call, use `jq length` to
detect when the result count reaches the `--limit 500` cap and exit with an
error before reporting all-clear; preserve normal filtering and success behavior
when fewer than 500 runs are returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1799b988-bba4-43a6-85d3-759650c21b12
📒 Files selected for processing (1)
.github/workflows/dependabot-failure-watcher.yml
| # --created filters server-side, so --limit applies to runs already | ||
| # narrowed to the window rather than to all of history. Runs come back | ||
| # newest-first, so reaching the limit would drop the oldest in-window | ||
| # runs and this step would report all-clear without them -- hence a | ||
| # limit far above any plausible week's worth of runs. | ||
| runs=$(gh run list \ | ||
| --repo "$REPO" \ | ||
| --workflow "Dependabot Updates" \ | ||
| --limit 100 \ | ||
| --json conclusion,createdAt,displayTitle,url \ | ||
| --jq "[.[] | select((.conclusion == \"failure\" or .conclusion == \"startup_failure\" or .conclusion == \"timed_out\") and .createdAt >= \"$since\")]") | ||
| --created ">=$since" \ | ||
| --limit 500 \ | ||
| --json conclusion,createdAt,displayTitle,url) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
sed -n '1,180p' .github/workflows/dependabot-failure-watcher.yml 2>/dev/null || true
echo
echo "== related references =="
rg -n "run list|--limit|failed|all-clear|No failed|Dependabot run query|since|run_limit|jq" .github/workflows/dependabot-failure-watcher.yml .github/workflows 2>/dev/null || true
echo
echo "== gh run list docs local? =="
if command -v gh >/dev/null 2>&1; then
gh run list --help 2>&1 | sed -n '1,220p'
else
echo "gh not installed in sandbox"
fiRepository: maxmind/mod_maxminddb
Length of output: 11852
Fail closed when the run list is truncated.
--limit 500 caps fetched runs, and because runs are returned newest-first, an older failed run in the 8-day window can be dropped and the watcher can still print “No failed Dependabot version update runs.” Track whether jq length reached the limit and exit with an error in that case, or fetch until all in-window runs are collected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/dependabot-failure-watcher.yml around lines 88 - 98, The
Dependabot run query must fail closed when the fetched results may be truncated.
After the `gh run list` call, use `jq length` to detect when the result count
reaches the `--limit 500` cap and exit with an error before reporting all-clear;
preserve normal filtering and success behavior when fewer than 500 runs are
returned.
Source: MCP tools
GitHub runs both Dependabot version updates and Dependabot security
updates under one workflow name, "Dependabot Updates", and the watcher
counted both. Security updates routinely fail for reasons no pull request
can fix -- the advisory is against a dependency this project does not
declare directly, or no patched version is reachable. Left alone the
watcher stays red every week on those and trains everyone to ignore it.
Filter those runs out by title. Version updates are unaffected.
The title check is subtler than it looks, so document it properly. A
security job is marked by "/." AND a " for " suffix together; version
updates are either "/." with no " for " (the scheduled scan) or "/" with
one (the pull request). Both halves of " in /. for " are therefore
load-bearing -- matching on " in /." alone would discard every scan run,
which is most of the version-update runs and the shape the failures this
watcher was written for actually took.
That "/." spelling only separates the two at the repo root. In a
subdirectory a security update and a version update's pull request render
identically, so drop " in /e2e/{js,ts} for " by name as well. The Node
repos this workflow is shared with carry committed lockfiles under e2e/js
and e2e/ts, whose transitive dev dependencies attract advisories no pull
request can fix, and nothing in either is shipped code. That is 16
unactionable failures in each of GeoIP2-node and minfraud-api-node over
retained history; repos without those directories are unaffected.
Unlike the root filter, this one is not free. Both Node repos configure
npm with directories: ["/", "**/*"], and that glob does match e2e/js and
e2e/ts, so those directories do get version updates -- there is an open
version-update pull request under e2e/ts in both repos as this is
written. Dropping the pattern discards their pull-request refresh
failures along with the security jobs, and the ecosystem label is no help
because Dependabot writes "npm_and_yarn" for both. Taken anyway: the
scheduled scan is what this watcher primarily exists to catch and is
still reported for those directories, so what is given up is the narrower
"one open pull request has gone stale" signal for two directories of test
scaffolding. After filtering, 4 genuine failures remain reported in
GeoIP2-node and 3 in minfraud-api-node.
Reading the directories out of dependabot.yml would look more general and
was the earlier plan here, but it fails green. Entries may use globs, and
minfraud-api-dotnet's directories: ["**/*"] yields titles like "nuget in
/**/*" for the scan and "nuget in /MaxMind.MinFraud for
System.Net.Http.Json" for the pull request, neither of which any literal
comparison against the configured value matches -- so its two real nuget
failures would have been dropped without a word. A stale denylist
re-introduces noise, which is loud; a stale allowlist hides failures.
Name the three kinds of run in the comment while here, because the
scheduled scan and the per-pull-request refresh are easy to conflate: the
refresh runs are one per open pull request and are triggered by pushes to
the base branch or by rebases, not by the schedule, so they arrive in
bursts after merges. The scan is the kind this watcher primarily exists
to catch, which is what makes hiding a hypothetical refresh failure under
e2e an acceptable cost rather than a hole.
Bound the query server-side with --created instead of fetching all of
history and filtering by date locally, so --limit now caps an
already-narrowed window rather than standing in for one, and the run list
drops from several API pages to one. --limit rises 100 -> 500 as a
backstop: it still applies before the title filter, and reaching it would
silently drop the oldest in-window runs.
This workflow is shared verbatim across MaxMind repos. The change was
developed in maxmind/device-android and is applied here unmodified; see
that repo's commit for the measurements it was derived from.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
c950213 to
e54c5cd
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/dependabot-failure-watcher.yml (1)
99-109:⚠️ Potential issue | 🟠 MajorThe 500-run cap can still hide an older failure.
--creatednarrows the server-side query, but--limit 500remains a hard cap. If more than 500 runs match the eight-day window, older failed runs may be omitted and the jq stage can report all-clear. Fail closed when the fetched count reaches 500 or paginate until the window is exhausted. GitHub documents--limitas the maximum number fetched and filtered workflow-run searches as capped at 1,000 results. (cli.github.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/dependabot-failure-watcher.yml around lines 99 - 109, Update the workflow-run retrieval in the Dependabot failure watcher so it cannot report all-clear when the result set reaches the 500-run cap. Either fail closed when the fetched count is 500, or paginate additional results until the eight-day window is exhausted, while preserving the existing jq filtering behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.github/workflows/dependabot-failure-watcher.yml:
- Around line 99-109: Update the workflow-run retrieval in the Dependabot
failure watcher so it cannot report all-clear when the result set reaches the
500-run cap. Either fail closed when the fetched count is 500, or paginate
additional results until the eight-day window is exhausted, while preserving the
existing jq filtering behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6348d713-33e7-4657-8f45-f69e053f4573
📒 Files selected for processing (1)
.github/workflows/dependabot-failure-watcher.yml
GitHub runs Dependabot version updates and Dependabot security updates under one
workflow name,
Dependabot Updates, and the watcher counted both. Securityupdates routinely fail for reasons no pull request can fix -- the advisory is
against a dependency the project does not declare directly, or no patched
version is reachable -- so the watcher stays red every week on those and trains
everyone to ignore it.
This filters security-update runs out by title, documents why both halves of
" in /. for "are load-bearing, and bounds thegh run listquery server-sidewith
--createdinstead of fetching all of history and filtering locally.See the commit message for the full reasoning, including why reading the
directory list out of
dependabot.ymlwas tried and rejected.This workflow is shared verbatim across MaxMind repos. The change was developed
in maxmind/device-android (maxmind/device-android#71)
and is applied here unmodified; the resulting file is byte-identical in every
repo.
Summary by CodeRabbit