Repository navigation
fix(renovate): hold build-tool image updates until the image is confirmed published - #571
Conversation
…rmed Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-opus-5.5 copilot=1.0.88 |
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
The new package rule never matches, so bst2 bumps still automerge. The rest of the PR (docs, test) describes behaviour the config does not produce.
Mechanism, from renovate.json at f4dc6fc:
- The bst2 custom manager sets
depNameTemplate: "bst2"andpackageNameTemplate: "https://gitlab.com/freedesktop-sdk/infrastructure/freedesktop-sdk-docker-images.git"(renovate.json:39-40). - The new rule uses
matchPackageNames: ["bst2"](renovate.json:54). Renovate'sPackageNameMatchercomparesmatchPackageNamesagainstpackageNameonly — there is no fallback todepName(lib/util/package-rules/package-names.ts,matches({ packageName }, …)→matchRegexOrGlobList(packageName, matchPackageNames)). packageNamefor this dep is the GitLab URL, notbst2, so the rule is skipped:automerge: falseand theprBodyNotesare never applied. The existing rule two lines above (renovate.json:47-48) already follows the correct convention — it matches the git-refs dep by its URL.
The new test (tests/unit/test_ci_workflows.py:172-180) only asserts the JSON shape, so it passes against a rule that is a no-op; it would not have caught this.
Fix is a one-liner — either:
"matchDepNames": ["bst2"],or
"matchDatasources": ["git-refs"],
"matchPackageNames": ["https://gitlab.com/freedesktop-sdk/infrastructure/freedesktop-sdk-docker-images.git"],The second mirrors the freedesktop-sdk rule directly above it. The docs change in docs/skills/ci-tooling.md is fine as written once the rule actually applies. Everything else about the approach (hold via automerge: false + skopeo note, since the docker datasource can't order 40-hex tags) reads correct to me.
Confidence: 2/5 (needs attention) — 1 high finding; 1 medium finding
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
matchPackageNames compares against packageName, which the bst2 manager sets to the docker-images repository URL, so a rule naming "bst2" never applied and the bump still automerged. Match the URL (and the git-refs datasource) and make the test derive the expected name from the manager. Hive-Fix-Attempt: 1/6 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
Resolve the ci-tooling.md conflict with #553: keep #553's "a failed upstream build leaves it unpublished" and this branch's automerge hold in the same three lines, so the file stays at its 500-line cap. Assisted-by: Claude Opus 5.5 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jeefy
left a comment
There was a problem hiding this comment.
Approving at 1339c2f. I merged main in to resolve the ci-tooling.md conflict with #553 in the same three lines, so the file stays at its 500-line cap.
The rule now matches the manager's packageName (the GitLab URL) and comes last in packageRules, so automerge: false wins. One correction to the description: CI would not catch an unpullable tag. No workflow pulls bst2_image, and #548, #574 and #578 all went through the queue with build skipped. This rule is the only guard, which is one more reason to land it.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Summary: Re-reviewed only the merge of the latest main branch into this change. The merge is clean except for one sentence in the CI tooling guide that was rewritten while resolving a conflict and now says the build-tool image is never pulled by automated checks. It is pulled on every pull request, so the sentence is wrong and should be corrected before merge.
What I need from you: Fix the one sentence in the docs (wording below).
docs-currency (low, in-scope)
docs/skills/ci-tooling.md:74(head 1339c2f) now reads "A failed upstream build leaves it unpublished and no CI job pulls it, so a package rule keeps these bumps from automerging." The "no CI job pulls it" clause is false:.github/workflows/build.yml:150runsjust validateonpull_request;Justfile:89(validate) callsjust bst show …;Justfile:46-47(bst) runspodman image exists/podman pullon${bst2_image}. The PR body itself says the opposite ("CI would also fail on an unpullable tag"). The previous head (6cb7d2d) had an accurate sentence; this wording came in with the conflict resolution against #553. Suggested replacement, same line count (the file is at exactly the 500-line cap,.github/scripts/docs-checks.py:100,113): "A failed upstream build leaves it unpublished;just validatein CI then fails on the pull, and a package rule also keeps these bumps from automerging so a human confirms the tag first."
No findings from: correctness, security, intent-alignment, style.
Confidence: 3/5 (needs attention) — 1 perspective requested changes
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Needs approval from an approver in each of these files:Approvers can indicate their approval by writing |
Summary: The automated update bot could propose a newer build-tool image version before that image had actually been published, or for a build that failed, which would break builds. Such updates are now held for a person to confirm the image exists instead of merging on their own.
Why: Renovate's
git-refsdatasource (renovate.json bst2 manager) reports any newmastercommit of freedesktop-sdk-docker-images, but the matchingbst2:<sha>tag only appears after that repo's pipeline succeeds. The docker datasource can't replace it (40-hex tags are unorderable; the existing test pinsgit-refs), so this adds a bst2 package rule that disables automerge and adds a PR note with askopeo inspectcheck. CI would also fail on an unpullable tag. Docs in docs/skills/ci-tooling.md updated.Closes #565
— hive: agent=scanner backend=copilot model=claude-opus-5.5 copilot=1.0.88