docs(dependency-management): verify SHA->tag, and make dep PRs run tests#116
Merged
Conversation
Two failure modes from one session, neither caught by actionlint or CI. Bulk SHA resolution: the section already says to resolve SHAs via the tags API, but not to verify the result. A loop printing 'action@<40-hex> # vX.Y.Z' looks authoritative either way. Observed twice in one run: - a pin whose SHA mapped to NO tag (6 of 7 were right, which is why the 7th nearly shipped); - a '^v?[0-9]+\.[0-9]+\.[0-9]+$' latest-picker that skipped two-part tags, so github/issue-labeler (v3.4, v3.3, v3.2) resolved to 'latest v2.4.1' -- a downgrade disguised as an upgrade. Adds the reverse SHA->tag check, prefers releases/latest over a tag regex (with its 404 caveat), and notes that a 'latest' resolving BELOW the current pin is the tell. Dependency PRs skipping tests: a paths filter of source globs does not match the manifest, so every dependency PR -- Dependabot's and Renovate's included -- skips the test job and merges untested, looking green because nothing ran. Observed on a Go provider whose unit_tests filter was '**.go': its upgrade PR ran only triage and DCO. Adding the manifest paths made unit_tests appear on the same PR and pass. Includes the check that verifies the job actually runs, rather than reading the YAML and assuming. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
github-actions
Bot
added
documentation
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds documentation on bulk SHA resolution and ensuring dependency PRs trigger test jobs. The review feedback suggests improving the provided jq command by using optional chaining to prevent errors when statusCheckRollup is null or missing.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
gh pr view returns statusCheckRollup as null when no checks have reported, and iterating null is a fatal jq error -- so the verification snippet would die in exactly the case it exists to detect (no test job ran). Verified: [.statusCheckRollup[].name] on null -> jq: error: Cannot iterate over null [.statusCheckRollup[]?.name] on null -> "TEST JOB DID NOT RUN" Still correct on a PR that does have checks. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
2 tasks
CybotTM
added a commit
that referenced
this pull request
Jul 20, 2026
…r workflow (#125) ## Came from `/retro` on a session working two TYPO3 extension PRs (netresearch/t3x-nr-image-optimize#115 / #116). This is a **C4** finding — the guidance existed and did not hold. Pitfall #6, added by an earlier `/retro` in #82, already says a CI gap belongs upstream in the shared workflow rather than in the consumer. I nonetheless added a local `acceptance` job with its own `actions/checkout` and `shivammathur/setup-php` to **both** consumer repos, and the maintainer had to correct it — then had to correct it a second time when I asked whether to SHA-pin a third-party action seconds after being told we don't use them. ## Why it didn't fire **The skill was never loaded.** The `description` said *"authoring reusable workflows"*, which reads as authoring the shared workflow — not as editing a consumer's `.github/workflows`. That is the case I was in, and no trigger word covered it. **The pitfall stated a principle without a test for it.** "Ask whether the shared repo owns that concern" needs judgement at exactly the moment you have already decided to write the job. A mechanical tell is easier to obey. ## Change - `SKILL.md` — `description` covers *consuming* reusable workflows and editing a repo's own `.github/workflows`; a `When to Use` entry points at pitfall #6. The file is at the 500-word limit, so this is paid for by folding the duplicated branch-protection note (it appeared both as a list item and as a standalone blockquote) into its list entry. **497 words** after the change, verified with `wc -w`. - `references/reusable-workflow-pitfalls.md` — pitfall #6 gains: - **the consumer-side tell** — every job in a consumer is a `uses:` of a shared workflow, so a job with its own `steps:`, or any non-org action, is the smell; with a grep that checks it. Notes that SHA-pinning such an action does not make it compliant. - **the upstream mechanics** — additive input defaulting to `false`, purely-additive diff check, pins copied verbatim from a neighbouring job, graceful skip for consumers without the underlying script, then a one-line flip in the consumer. - a warning against `unit-test-command`-style override inputs, which replace the job's instrumented default and silently drop what it did (in that session, the Codecov upload). The upstream path described here is the one that ended up being taken: netresearch/typo3-ci-workflows#136 added a `run-acceptance-tests` input defaulting to `false` (113 insertions, 0 deletions), and both consumers then enabled it with one line. ## Test plan - [x] pre-commit hooks incl. `markdownlint-cli2`, skill-structure validation and version parity — passed - [x] `wc -w skills/github-project/SKILL.md` → 497 (cap 500)
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
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.
Two failure modes from one session (netresearch/terraform-provider-ad#15). Neither is caught by
actionlintor by CI — both ship silently.1. Bulk SHA resolution — verify the SHA maps back to the tag
The section already says to resolve SHAs via the tags API. It doesn't say to verify the result, and a loop printing
action@<40-hex> # vX.Y.Zlooks authoritative whether or not it's right.Both of these happened in a single resolution run:
juliangruber/read-file-actiongot a pin whose SHA mapped to no tag at all. Six of seven resolved correctly — which is exactly why the seventh nearly shipped. A wrong-but-well-formed SHA pins silently; nothing downstream complains.^v?[0-9]+\.[0-9]+\.[0-9]+$"latest" filter skips two-part tags.github/issue-labelerpublishesv3.4,v3.3,v3.2— so it resolved to "latestv2.4.1": a downgrade disguised as an upgrade.Adds:
releases/latestinstead of deriving latest from a regex, with its 404 caveat for tag-only repos,2. Dependency PRs must trigger the test job
A
paths:filter listing source globs does not match the manifest, so every dependency PR — Dependabot's and Renovate's included — skips the test job and merges untested. It looks green because the tests never ran.Real case: a Go provider whose
unit_testsfilter was'**.go'. Its dependency-upgrade PR ran onlytriageandDCO. Adding the manifest paths madeunit_testsappear on that same PR and pass — every prior dependency bump in that repo had merged without a single test running.Adds the before/after YAML, the equivalent manifests per ecosystem, and the check that verifies the job actually appears on a manifest-only PR rather than reading the YAML and assuming.
Placement
Both go in
dependency-management.md: the SHA item extends the existing SHA-pinning section (which already tells you to resolve via the tags API — this is the missing verification step), and the paths item is squarely a dependency problem, since its whole blast radius is dep PRs.Verification
markdownlint-cli2: 0 errors. All 12 fenced YAML blocks parse viayaml.safe_load. Docs-only, no version bump — matching the surroundingdocs(<reference>):history. Every command shown was run in the originating session.