docs(workflows): document the SHA-pin and check-name-collision fixes - #1728
Merged
Conversation
Two real problems surfaced on the caller PRs after they opened, both now fixed live (argos, mightyETL, newsdom-api, scopeweave all pinned; newsdom- api's branch protection required-check name updated) but not yet reflected in this repo's own docs and header comment: 1. Every caller example here still showed @main; corrected to the commit-SHA pattern now required for every reusable-workflow caller in this org, and the reusable workflow's own header comment now documents why (Devin's security finding: an unpinned mutable ref runs an unreviewed central change against every caller's PR checks). 2. Documents the required-status-check-name collision this consolidation caused on newsdom-api (converting a job to uses: renamed its published check from "dependency-review" to "dependency-review / dependency-review", which newsdom-api's branch protection required verbatim) and how it was fixed, as a gotcha for any future standalone-job-to-reusable-workflow conversion. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| cannot itself be what GitHub triggers on pull_request), gains a | ||
| `concurrency` block if it lacked one, and adds one job: | ||
| `uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main` | ||
| `uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@0bcd22d8bb07650aafb0a8f116e4c2bbb8744f03` |
Contributor
There was a problem hiding this comment.
📝 Info: Pinned workflow body remains equivalent
The documented 0bcd22d8… pin predates only header comments. Its executable workflow matches the current body, so the examples do not preserve stale behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+158
to
+188
| Two real problems surfaced after the four caller PRs opened, both caught | ||
| before any of them merged (except argos, fixed retroactively): | ||
|
|
||
| **1. Mutable `@main` reference (Devin, security finding).** The original | ||
| callers referenced `uses: .../dependency-review.yml@main` — the example | ||
| above now shows the corrected pattern. A mutable branch ref means an | ||
| unreviewed change to `.github`'s `main` (or a reference-tampering attack) | ||
| runs directly against every caller's PR checks with zero review in the | ||
| calling repo. Fixed by pinning every caller to the exact commit SHA that | ||
| added the file, `0bcd22d8bb07650aafb0a8f116e4c2bbb8744f03` (unchanged since | ||
| it merged) — `argos` retroactively (a follow-up PR after its original | ||
| merge), the other three before their first merge. This is now the | ||
| documented pattern in the reusable workflow's own header comment: pin | ||
| `uses:` to a commit SHA for every caller, the same way every *action* step | ||
| inside the reusable workflow itself is already SHA-pinned. | ||
|
|
||
| **2. Required-status-check name collision (Devin, bug finding on | ||
| newsdom-api).** Converting a job from inline steps to `uses: <reusable | ||
| workflow>` changes the check-run name GitHub publishes, from the caller | ||
| job's own name (e.g. `dependency-review`) to a combined | ||
| `<caller job name> / <called job name>` (here, | ||
| `dependency-review / dependency-review`). `newsdom-api`'s `develop` branch | ||
| protection required a status check named literally `dependency-review` — | ||
| after conversion, that exact name is never published again, so the | ||
| required check stays pending forever and blocks every future merge. | ||
| Verified live: `argos` and `mightyETL` have no branch protection at all | ||
| (nothing to break); `scopeweave`'s required checks don't include | ||
| `dependency-review`; only `newsdom-api` was affected. Fixed by updating | ||
| `newsdom-api`'s branch protection required-status-checks list directly | ||
| (`gh api -X PATCH repos/.../branches/develop/protection/required_status_checks`), | ||
| replacing `dependency-review` with the actual published name |
Contributor
There was a problem hiding this comment.
🔍 Live fixes lack local verification
The claims that all callers are pinned and branch protection changed depend on sibling repositories and live GitHub settings. This checkout cannot verify them.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
added a commit
that referenced
this pull request
Sep 2, 2026
Preserve the verified central-run read-authority repair while taking current protected main as the authoritative tree, including #1728 dependency-review documentation and #1723 retirement of completed PR1714/PR1715 source-fix assets. Reapply only #1717's four-file semantic delta. Keep both histories without force push or destructive rebase.
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
1 participant
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
dependency-review.yml's header comment and the doctoring doc's caller examples from@mainto the SHA-pinned pattern, and documents why: an unpinned mutable ref runs an unreviewed central change against every caller's PR checks.uses:renames its published check-run) that brokenewsdom-api's branch protection, and how it was fixed, as a gotcha for any future standalone-job-to-reusable-workflow conversion.feat/r-package-check-reusable-workflowbranch (feat(workflows): reusable R-CMD-check for kaefa/nonnest2 #1716) corrects the same@mainexample before it merges.Test plan
python3 -c "import yaml; yaml.safe_load(...)"on the edited workflowactionlint .github/workflows/dependency-review.ymlcoverage run -m pytest tests→ 2638 passed, 1 skipped🤖 Generated with Claude Code