ci: harden dependabot workflows and switch Claude fix to workflow_run#348
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR hardens two CI workflows that interact with Dependabot PRs. It prevents the Claude auto-review job from running on routine Dependabot bumps (reducing noise and API spend), and adds Changes
Review Notes
|
![claude-code-ably-assistant[bot]](https://avatars.githubusercontent.com/in/2109912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: ci: skip Claude review on dependabot PRs and harden lockfile regen
Both changes are correct and well-motivated. No issues to flag.
.github/workflows/claude-review.yml
The operator precedence is correct. The dependabot[bot] exclusion is added inside the first && group (already parenthesised), so it only applies to the pull_request event path. The explicit-mention || branches (issue_comment, pull_request_review_comment, assignee/review-requested) are unchanged — maintainers can still request a Claude review on any dependabot PR by @claude-mentioning. The bot username dependabot[bot] is the correct GitHub identity for Dependabot.
.github/workflows/dependabot-lockfile.yml
--ignore-scripts and --frozen-lockfile are orthogonal and compose correctly. The security rationale is sound: this install step runs against package versions a maintainer hasn't yet reviewed, so skipping postinstall hooks eliminates the supply chain risk of arbitrary code executing with the workflow's push-capable GitHub token. Build/test scripts run as normal in subsequent workflow runs after the lockfile commit.
Overall
Clean, minimal, well-scoped. Both changes do exactly what the PR description says.
umair-ably
changed the title
Two small hardening changes to the CI workflows that touch dependabot: - claude-review.yml: exclude PRs opened by dependabot[bot] from the auto-review job. Dependabot PRs are routine lockfile/version bumps — Claude review on them is noise, delays merge, and consumes API budget with no meaningful signal. The explicit-mention paths (issue_comment, pull_request_review_comment, etc.) are unchanged, so a maintainer can still request a review on a dependabot PR by @-mentioning Claude. - dependabot-lockfile.yml: pass --ignore-scripts to pnpm install in the lockfile-regeneration step. This step installs dependencies from a dependabot-authored PR, so any freshly-bumped package's postinstall hook would otherwise run with the workflow's token (including permissions to push to the PR branch). --ignore-scripts eliminates that arbitrary-code-execution surface during the regen. Build scripts on our own code aren't needed here — the step only resolves/installs to produce a consistent lockfile.
Replaces the 25-minute polling loop in dependabot-lockfile.yml with an
event-driven workflow that fires only after upstream CI finishes —
and only when there's actually something to fix. Also drops the
"empty success" run that was overwriting Claude's fix run in the
Actions tab history.
Architecture (two workflows, one purpose):
1. dependabot-lockfile.yml (renamed to "Regenerate Dependabot Lockfile"):
regenerates pnpm-lock.yaml on dependabot PRs and pushes the update.
Nothing else. The fix-failures job and its 25-minute wait-for-checks
polling loop are gone.
2. dependabot-claude-fix.yml (new, "Fix Dependabot PRs"): fires on
workflow_run.completed for the four CI workflows we care about
— Run Tests, E2E Tests, Web CLI E2E Tests (Parallel), Security
Audit. Each firing is a gate check, not an immediate trigger:
a. Look up the PR the triggering workflow ran on. Exit if the
author isn't dependabot[bot] or the PR is from a fork.
b. Loop-prevention guard (≤20 successful runs per branch), same
shape as the lockfile workflow's guard.
c. Compute expected workflows dynamically for this PR — check
the PR's changed files against each workflow's
on.pull_request.paths filter. Run Tests has a paths list;
the other three always run.
d. Gate: for every expected workflow, check its run status on
this HEAD SHA. If any is still queued/running, or hasn't
produced a run record yet, exit 0. A later workflow_run event
(from whichever upstream finishes next) will re-run the gate.
e. When the gate opens (last expected workflow has finished),
gather the failures from all of them in one pass, feed the
aggregated summary + logs into the existing Claude prompt,
and run the fix step.
Concurrency is keyed on HEAD SHA with cancel-in-progress=true:
near-simultaneous completions can fire two workflow_run events
that both see "all complete"; the second cancels the first and
we get a single Claude run per SHA. New pushes (Claude's fix,
another dependabot commit) get their own concurrency lane.
Why this shape:
- No runner time burned on green dependabot PRs. The old workflow
polled for up to 25 minutes on every PR, even when CI passed
outright.
- Actions tab history is self-describing. The old fix-failures job
always ran to completion and usually succeeded with nothing to do,
so the most recent "Fix Dependabot PRs" row in the Actions tab
was typically an empty success, hiding the earlier run that
actually did the migration work. Now the workflow only runs when
there's something to fix, so every row in its history represents
real work.
- Claude still sees all failures at once. The gate deliberately
waits for every expected workflow before proceeding, so the
single Claude invocation handles the full failure set — matching
the behaviour of the old polling loop.
Implementation notes:
- The "Run Tests" paths list is duplicated in the gate step (kept
in sync with test.yml on.pull_request.paths via a comment). The
matcher handles exact paths and DIR/** globs, which covers every
pattern this repo currently uses.
- workflow_run always uses the default-branch version of the
workflow file; prompt edits take effect on subsequent runs after
merge, not in the PR that edits them.
- workflow_run-triggered jobs can use write tokens, so the App
token flow for pushing Claude's fix commits works the same as
before.
- The Claude prompt is preserved verbatim except for the failure
inputs (now come from steps.gate.outputs instead of
steps.wait-for-checks.outputs) and the PR number reference for
the final gh pr comment call.
e3b0b54 to
7d94a81
Compare
|
You've now hardened dependabot-claude-fix.yml through three substantial rewrites (#333, #348). Each rewrite fixes real problems. but as far as I can tell there's no regression harness between rewrites, so every change risks silently undoing a behavior the previous round fixed. That's a rough spot to be in with a workflow that has contents:write and runs multiple times a day. We maintain an open-source tool (reelier, github.com/seldonframe/reelier) built for this: record a known-good agent fix run once, then replay it on every workflow change, deterministic steps replay at zero tokens, and fail CI when the run's behavior drifts from the recording. It effectively gives the workflow itself a test suite. Disclosure: I'm a maintainer, so treat this as a biased suggestion from someone who thinks your setup is one of the most advanced agent-in-CI deployments around. If it's interesting, I'm happy to open a PR with the recording + workflow wired up so the cost to evaluate is one review. If not, please feel free to close this. no follow-up from me either way. |
2 participants
Summary
Three changes to how our CI handles dependabot PRs:
claude-review.yml— exclude PRs opened bydependabot[bot]from the auto-review job. Explicit-mention paths (@claudecomment, review-requested, etc.) are unchanged, so maintainers can still ask for a review on a tricky bump.dependabot-lockfile.yml→ renamed to Regenerate Dependabot Lockfile. Its only job now isregen-lockfile(regeneratepnpm-lock.yamland push). The 25-minute polling loop and Claude fix step are removed from this workflow. Thepnpm installin the regen step gains--ignore-scripts.dependabot-claude-fix.yml(new) — Fix Dependabot PRs. Fires onworkflow_run.completedforRun Tests,E2E Tests,Web CLI E2E Tests (Parallel), andSecurity Audit. Each event is a gate check: computes which of those workflows were expected for this PR (based on each workflow'son.pull_request.pathsfilter vs the PR's changed files), then checks whether every expected one has completed on this HEAD SHA. If any is still queued/running, exits 0 — the nextworkflow_runevent will re-gate. When the last expected workflow finishes, aggregates the full failure set and runs Claude once.Motivation
Skip auto-review on dependabot PRs. GitHub runs workflows triggered by dependabot under a restricted permission scope — the
GITHUB_TOKENis read-only and repo secrets are not exposed to the workflow.claude-review.ymlneedssecrets.ANTHROPIC_API_KEY,secrets.CLAUDE_APP_ID, andsecrets.CLAUDE_APP_PRIVATE_KEYto run; on a dependabot PR those resolve to empty strings and the job fails outright. Gating the job off foruser.login == 'dependabot[bot]'stops the workflow from running at all (rather than running, failing, and showing a red ❌ on every dependabot PR). The mention paths are kept intact:@claudein a review comment re-runs the workflow under anissue_comment/pull_request_review_commentevent, which doesn't inherit the restricted dependabot scope, so a maintainer can still request a review on a tricky bump.--ignore-scriptsduring lockfile regen. The regen step installs dependencies from a dependabot-authored PR — a freshly-bumped set of versions nobody has reviewed. Any package'spostinstallwould otherwise execute with the workflow's App token (which can push back to the PR branch).--ignore-scriptseliminates that arbitrary-code-execution surface during regen. Real builds/tests run in subsequent workflows where scripts execute normally.Replace the polling loop with a
workflow_run-gated fix job. The previous fix-failures job polled for up to 25 minutes every time a dependabot PR moved, even when every check passed outright. It then completed successfully with nothing to do, and that "empty success" became the latest row in the Actions tab — overwriting the earlier run where Claude actually did migration work and making those logs harder to find.The new shape solves both problems:
workflow_runfires for all outcomes, but the gate collapses to a no-op for an all-green set).How the gate works
On each
workflow_run.completedevent:github.event.workflow_run.pull_requests[0].number→ PR →.user.login == 'dependabot[bot]'. PRs from forks (emptypull_requestsarray) and non-dependabot PRs exit 0. The PR author is stable across lockfile-regen pushes (those are attributed to the App, not the PR opener).on.pull_request.paths(if any) against the PR's changed files via a small shell matcher that handles exact paths andDIR/**globs.Run Testsis paths-filtered; the other three have no filter and are always expected./actions/runs?head_sha=...and evaluate per expected workflow. If any is stillqueued/in_progress, or hasn't produced a run yet, exit 0 — the next event re-gates. If all completed, proceed to aggregation.gh run view --log-failedfor each failed run, feed them into the existing Claude prompt as one combined input.Concurrency is keyed on
head_shawithcancel-in-progress: true. Near-simultaneous completions may produce two events that both see "all complete" — the second cancels the first and we get one surviving Claude run per SHA. New pushes (Claude's own fix commit, a fresh dependabot commit) get their own lane.Trade-offs to be aware of
workflow_runalways uses the default-branch copy of the workflow file. Once merged, prompt edits take effect on subsequent runs, not on runs inside the PR that edits them.Run Testsis duplicated in the gate step. Kept in sync withtest.yml'son.pull_request.pathsvia a comment. If any other monitored workflow adds apathsfilter, extend the matcher block.gh pr commentPR number reference are rewired from the oldsteps.wait-for-checks.outputs.*tosteps.gate.outputs.*/steps.pr.outputs.pr_number.Test plan
Claude PR Reviewdoes not run on open (no red ❌ from a secrets-starved job).@claudein a comment → confirm the review runs via the mention path (workflow runs with full secrets underissue_comment).package.json/pnpm-lock.yaml: confirmRegenerate Dependabot Lockfileruns with--ignore-scriptsand commits the regen.Fix Dependabot PRsfires on the last upstream CI workflow's completion (not on each one) and that its Actions-tab row only appears when at least one upstream workflow failed.Fix Dependabot PRsrun, combinedfailure_logspayload).Fix Dependabot PRsrow is created.