fix(ci): repair auto-label's dead trigger and truncated file list - #1443
Conversation
Two defects in `.github/workflows/auto-label.yml`, both silent. `pull_request.types` listed `synchronized`, which is not a GitHub activity type — the real one is `synchronize`. GitHub ignores unknown activity types rather than rejecting them, so the workflow never ran on pushes to an open PR. Labels were only ever applied at `opened`/`reopened`, and a PR whose diff grew to touch new languages kept its original label set. `pulls.listFiles` was called without pagination, so it returned only the first 30 files of the diff. A larger PR was labelled from an arbitrary prefix of its changed files. Now uses `github.paginate(..., per_page: 100)`, matching the existing convention in `pr-governance.yml`. Verified by extracting the embedded script from the workflow YAML and running it against a mocked `github` client with a 120-file diff whose 120th file is the only `.py`: - fixed script: labels ["javascript","python"] — 120th file reached - previous script: throws "unpaginated listFiles called" so the check fails against the unfixed workflow rather than passing vacuously. YAML parses and the embedded script passes `node --check`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018JcuWvy4pthLP3HUehNCxr
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe auto-label workflow now runs when new commits are pushed to a pull request. It also retrieves all changed files through paginated requests with up to 100 files per page. ChangesAuto-label workflow fixes
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Sentry Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped 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 |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
|
@coderabbitai full review Invoking review explicitly because auto-review skipped this PR — and the skip is itself worth recording, because this PR is a clean reproduction of #1424 on a PR that satisfies the gate. #1443 carries the That is the exact deadlock #1424 describes, observed here on a PR created minutes ago rather than inferred from older ones. It also demonstrates the compounding this PR's own description claims: with Two things follow that are worth having on the record:
Generated by Claude Code |
|
The reported sequence is valid review context: CodeRabbit can evaluate the required-label gate before 🐇🔍 ✏️ Learnings added
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/auto-label.yml:
- Around line 23-29: The auto-label workflow lacks a regression test proving
pagination processes files beyond the first 30. Add a test that extracts and
runs the embedded script with mocked GitHub responses containing 120 changed
files, then assert a file after the first 30 produces its expected label; ensure
it fails with the former single-request implementation and is discoverable by
the provided search.
🪄 Autofix
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: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f4270b10-83be-4cf8-b1ee-23d7321a1e39
📒 Files selected for processing (1)
.github/workflows/auto-label.yml
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: trivy
- GitHub Check: Generate and Upload Coverage
- GitHub Check: test
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Create or edit GitHub Actions workflows to add robust testing and verification for new features.
Files:
.github/workflows/auto-label.yml
.github/workflows/**
⚙️ CodeRabbit configuration file
GitHub Actions workflows. Check for missing permissions, insecure token handling, proper use of continue-on-error vs actual error handling, and Node.js version compatibility (Node 20 deprecation warning).
Files:
.github/workflows/auto-label.yml
🔍 Remote MCP GitHub Copilot
Relevant review context
- PR
#1443changes only.github/workflows/auto-label.yml: replacessynchronizedwithsynchronizeand usesgithub.paginate(..., per_page: 100)for changed files. The repository’spr-governance.ymlalready uses the same pagination convention. - The linked issue’s acceptance criteria additionally require a non-vacuous behavioral test against the embedded workflow script, YAML parsing, and JavaScript syntax validation.
- The PR reports those checks as locally completed, but the PR itself changes only the workflow file; no regression test is included in the PR diff. The repository does have workflow-level tests for
pr-governance.yml, providing an existing pattern for adding one. - Required validation is not yet complete:
trivy, coverage, Python security scan, build, and test jobs were still in progress; governance was neutral because the PR is draft. - The PR’s claim that it reproduces the CodeRabbit label-gate deadlock is partly confounded: the PR is currently draft, while
.coderabbit.yamlexplicitly setsauto_review.drafts: false; issue#1425confirms drafts are excluded independently of label configuration.
🔇 Additional comments (1)
.github/workflows/auto-label.yml (1)
4-6: LGTM!
CodeRabbit correctly flagged that the verification described on #1443 was run locally and never committed, so nothing in the repo prevented either defect from returning. This lands it as a real test. Follows the existing `tests/unit/test_pr_governance_workflow.py` pattern — load the workflow YAML, pull the embedded `github-script` body out of it, and assert against the script that actually ships rather than a copy. Static coverage asserts `synchronize` is the trigger (and `synchronized` is not) and that the listing paginates with `per_page: 100`. The behavioral test executes the embedded script under `node` against a mocked client with a 120-file diff whose 120th file is the only `.py`; the mock raises if the unpaginated `pulls.listFiles` is called at all. Non-vacuity checked by running the suite against the pre-fix workflow from `origin/main`: 3 failed, 1 passed (the pass is the YAML-validity check, true either way) and against the fixed workflow: 4 passed `node` is required only by the behavioral test, which skips without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018JcuWvy4pthLP3HUehNCxr
|
@coderabbitai review Both of your points were right. Addressed in 1. Missing regression test — fixedCorrect, and the important part of the catch: the verification in the PR description was run locally and never committed, so nothing in the repo prevented either defect from returning. A PR whose whole subject is "two silent defects shipped because nothing tested them" should not itself rely on an uncommitted check.
Non-vacuity checked both directions rather than asserted:
2. The deadlock claim was confounded — concededYou are right, and I'm correcting the record rather than defending it. #1443 is a draft and #1440 is the unconfounded case. It is not a draft — it was What survives from the original claim without qualification is the narrower and more consequential part: a green Generated by Claude Code |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- changed files in follow-up commit ---'
git show --stat --oneline 2fe3ecb
git diff 2fe3ecb^ 2fe3ecb -- .github/workflows/auto-label.yml tests/unit/test_auto_label_workflow.py
echo '--- candidate test outlines ---'
ast-grep outline tests/unit/test_auto_label_workflow.py --items all || true
ast-grep outline tests/unit/test_pr_governance_workflow.py --items all || true
echo '--- workflow with line numbers ---'
cat -n .github/workflows/auto-label.yml
echo '--- new test with line numbers ---'
cat -n tests/unit/test_auto_label_workflow.py
echo '--- existing workflow-test pattern with line numbers ---'
cat -n tests/unit/test_pr_governance_workflow.pyLength of output: 10828 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- checked-out revision and commit relations ---'
git status --short --branch
git rev-parse HEAD
git branch --contains 2fe3ecb || true
git merge-base --is-ancestor 2fe3ecb HEAD && echo '2fe3ecb is an ancestor of HEAD' || echo '2fe3ecb is not an ancestor of HEAD'
git show -s --format='commit=%H%nparents=%P%nsubject=%s' 2fe3ecb
echo '--- files committed by 2fe3ecb ---'
git ls-tree -r --name-only 2fe3ecb -- tests/unit/test_auto_label_workflow.py .github/workflows/auto-label.yml
git show 2fe3ecb:tests/unit/test_auto_label_workflow.py | cat -n
echo '--- existing committed test pattern at the same revision ---'
git ls-tree -r --name-only 2fe3ecb -- tests/unit/test_pr_governance_workflow.py
git show 2fe3ecb:tests/unit/test_pr_governance_workflow.py | cat -n
echo '--- target workflow at the same revision ---'
git show 2fe3ecb:.github/workflows/auto-label.yml | cat -n
echo '--- PR head and commit membership ---'
gh pr view 1443 --repo groupthinking/EventRelay --json headRefOid,commits --jq '{headRefOid, commits: [.commits[].oid]}'Length of output: 15805
The new regression test covers the shipped embedded workflow script. It detects both original defects. The behavioral test rejects direct I found no new correctness, security, or production-impacting issue in the follow-up change. I did not execute the repository test suite in this environment. The correction about this PR being a draft is accurate. Draft exclusion prevents this PR from serving as an unconfounded label-gate reproduction. The distinction between a successful status and a completed review remains important. 🐇 ✅ ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit!
|
|
@coderabbitai review Requesting explicitly because auto-review did not run on head The earlier finding on Generated by Claude Code |
|
✏️ Learnings added
|
Verification status — two boxes in the description are now staleAutomated PR triage. The description still carries these unchecked, and both are false as of head
Measured now:
Independently re-ran the new tests locally against this head: 4 passed, including the node-executed pagination test that fails against the pre-fix workflow. One correction to the
|
2 participants
Canonical issue
Closes #1442
Outcome
Auto Labelruns on pushes to an open PR, and labels the whole diff rather than its first 30 files.Two silent defects in
.github/workflows/auto-label.yml:pull_request.typeslistedsynchronizedsynchronize. GitHub ignores unknown activity types instead of rejecting the file, so there was no validation error. The trigger was simply inert, and labels were only ever applied atopened/reopened.pulls.listFilescalled without paginationper_page: 30. A larger PR was labelled from an arbitrary 30-file prefix of its diff. The job still concluded green.The pagination fix follows the convention already established in
.github/workflows/pr-governance.yml(github.paginate(github.rest.pulls.list, ...)); this call site had not adopted it.Scope
.github/workflows/auto-label.yml— the trigger type, the paginated listing, and a comment on each recording why. Plustests/unit/test_auto_label_workflow.py(new, 4 tests).Risk
addLabelsis already wrapped intry/catchwithcore.warning, so a rate-limit or permission error warns rather than fails the job. Labels are additive; nothing is removed.git revert. No runtime, deployed, or data surface.Verification
Head
2fe3ecb, base8cd4a10. Two files, workflow +9/−3 and a new test module.tests/unit/test_auto_label_workflow.pyfollows the existingtests/unit/test_pr_governance_workflow.pypattern: load the workflow YAML, extract the embeddedgithub-scriptbody, and assert against the script that actually ships rather than a copy of it.Focused tests — 4 tests. Static coverage asserts
synchronizeis present andsynchronizedis not, and that the listing paginates withper_page: 100. The behavioral test executes the embedded script undernodeagainst a mocked client with a 120-file diff whose 120th file is the only.py; the mock raises if the unpaginatedpulls.listFilesis called at all.Non-vacuity checked in both directions, not asserted:
ruff check— clean.black --check— clean.Required CI — not complete on this head.
Review threads — resolved. CodeRabbit requested changes on
f9f9688for exactly one thing: the verification above existed only locally and was never committed, so nothing in the repo prevented regression. Correct, and fixed in2fe3ecb. CodeRabbit re-verified against the commit and reported "no new correctness, security, or production-impacting issue."Production evidence
Not applicable. One GitHub Actions workflow file and one test module; no runtime or deployed surface, and nothing under
apps/web/**, soMERGE_POLICY.mdgate 4 (preview) does not apply.The honest limit on the evidence above: the corrected trigger cannot be observed firing from this PR. GitHub reads
pull_requesttriggers from the base branch, sosynchronizeonly takes effect once this is onmain. The post-merge check is to push a commit to any open PR and confirm anAuto Labelrun appears for thesynchronizeevent.On the #1424 link — what this PR does and does not evidence
An earlier comment claimed #1443 was a clean reproduction of the #1424 label-gate deadlock. That claim is withdrawn, and the reason it was withdrawn has since changed. Both states are recorded here rather than quietly overwritten.
.coderabbit.yamlsetsauto_review.drafts: false, which is sufficient on its own to explain a skip. Two sufficient causes, so the skip notice naming label configuration was suggestive but not decisive.linear-code[bot]marked this PR ready for review, so it is no longer a draft, and it carriespythonandci/cd— both in the required-labels list. TheCodeRabbitstatus at19:30:29Zstill readsReview skipped: excluded by label configuration.19:30:36Z, after that skip was written. The skip was therefore probably still evaluated against a draft. The ordering is too tight to call, so this PR remains confounded and should not be cited as proof.#1440 is the unconfounded case and is what #1424/#1425 should be argued from: not a draft when its status was written, carries
javascriptandtests(both in the required list), and still showsReview skipped: excluded by label configuration.What holds without qualification, and is the part that actually affects merge decisions: a green
CodeRabbitcommit status does not distinguish "reviewed" from "skipped" or "rate limited." This PR alone has produced skip statuses readingsuccessfor three different reasons — label configuration, draft exclusion, andpath_filters(tests/**) — every one of them green.Agent handoff
synchronizedis not a GitHub activity type #1442auto-label.ymlsynchronizedis not a GitHub activity type #1442, including the non-vacuous behavioral test2fe3ecb;mergeable_stateisunstableAgent provenance
Agent-authored, under the PR remediation runbook. Found while verifying #1440, whose notes flagged this typo as out of scope for a stacked branch and deferred it to a branch off
main— which is what this is.