fix(janitor): reconcile #26/#50/#54 — stale-diff conflicts, no content overlap - #69
Merged
Merged
Conversation
…ent overlap Three open PRs (#26, #50, #54) all showed mergeable=CONFLICTING against current main. None of their real changes overlap line-for-line — the conflicts are purely stale-diff artifacts of main having moved on since each PR's fork point (#66's dual-org repo-enumeration restructure in particular). This reapplies all three fixes' actual intent by hand onto current main, in dependency order (both #50 and #54 touch dependabot-janitor.sh). #54 (classify() group-PR hardening, supersedes #23): a Dependabot "group" PR title no longer proves minor/patch. classify() now fetches the PR body via REST and requires every per-dependency "from A to B" marker to be same-major, failing closed otherwise. Reapplied verbatim except the body fetch now calls `repos/$repo/pulls/$num` directly (since $repo is already "org/name" post-#66) instead of #54's original `repos/$ORG/$repo/...`. Adds dependabot-janitor.test.sh (14 assertions, real fixtures) — verified byte-identical to #54's original test file aside from that same $ORG-> $repo adaptation; all 14 assertions pass unmodified. #50 (cortextos/conduit scope carve-out): reapplied by hand onto the current multi-org enumeration loop (#66) rather than #50's own stale single-org grep pipeline — added `|^cortextos$|^conduit$` to the still-present grep -E inside the `for _org in $ORGS` loop. #26 (mcp-server-release.yml always() gating): the release job's "Detect released version" step now runs with `if: always()`, and the docker job's gate is `if: always() && needs.release.outputs.released == 'true'`, so a non-fatal failure after semantic-release cuts a release (e.g. a flaky git-notes push) can't silently skip artifact publishing. Investigated the apparent second `id: detect` in the current file: it is an unrelated step in the (post-#26) mcpb job that checks for a pack:mcpb script, not a duplicate release-detection block — #26's fix applies to exactly the one occurrence it always targeted. Flagged in the CHANGELOG that mcpb's own released=='true' gate has an analogous latent exposure, left out of scope here since it predates neither #26 nor this reconciliation. Verified: shellcheck clean on dependabot-janitor.sh (baseline was already clean); dependabot-janitor.test.sh carries only pre-existing info-level notices inherited from #54's original file (one pre-existing SC2034 warning was fixed by dropping the now-unused $ORG var); all 14 tests pass; mcp-server-release.yml parses as valid YAML. Refs: #26, #50, #54
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.
Summary
#26, #50, and #54 were all open with
mergeable: CONFLICTINGagainst currentmain(tip692c0d0at investigation time). None of their real changes overlap line-for-line — the conflicts are stale-diff artifacts ofmainmoving on since each PR's fork point, most significantly #66's dual-org repo-enumeration restructure ofdependabot-janitor.sh. This PR reapplies all three fixes' actual intent by hand onto currentmain, in dependency order.#54 — classify() group-PR hardening (
fix/janitor-group-perdep-rebased, supersedes #23)A Dependabot "group" PR title (e.g. "bump the dev-dependencies group with 12 updates") no longer proves the PR is minor/patch — the title doesn't list the dependencies.
classify()now fetches the PR body over REST (outage-resilient vs.gh pr view's GraphQL) and requires every per-dependencyUpdates \pkg` from A to Bmarker to be same-major, failing closed on any cross-major, unparseable marker, zero markers, or a failed fetch. This is the fix for the realnode-datto-rmm#46incident, where a "group" title unconditionally short-circuited toELIGIBLE` and auto-merged a hidden major that broke main.Reapplied verbatim from #54's diff except one adaptation: the body fetch now calls
gh api "repos/$repo/pulls/$num"directly, since$repois already"org/name"post-#66's dual-org restructure, instead of #54's original single-orgrepos/$ORG/$repo/pulls/$num(which would reference an undefined$ORGunderset -uin the current file, or double/misdirect the org).Also adds
.github/scripts/dependabot-janitor.test.sh(14 assertions, fixtures from real Dependabot PR bodies) — verified byte-identical to #54's original test file aside from that same$ORG→$repoadaptation (and dropping the now-unusedORG=assignment, which also fixes a shellcheck SC2034 warning #54's original had). All 14 assertions pass unmodified against the reconciled script.#50 — cortextos/conduit scope carve-out (
fix/dependabot-janitor-scope-cortextos-conduit)cortextosandconduitdon't match the janitor's repo-selection regex (-mcp$|^mcp|^node-) and get zero Dependabot auto-merge coverage as a result (task_1785692635899_03153380). #50's own diff targets the pre-#66 single-orggreppipeline and no longer applies — reapplied by hand as|^cortextos$|^conduit$added to the still-presentgrep -Ecall, now living inside thefor _org in $ORGSmulti-org loop #66 introduced. Confirmed both repos currently live underWYRE-AI(one of the two defaultORGS), so the carve-out is reached by the existing enumeration without further changes.#26 — mcp-server-release.yml
always()gating (fix/release-resilient-artifact-gating)A non-fatal failure after semantic-release cuts a release (e.g. a flaky/duplicate git-notes push —
"cannot lock ref refs/notes/semantic-release-…: reference already exists") previously failed theSemantic Releasestep even though the release was already complete, which then skipped the (unconditional)Detect released versionstep, leftreleasedunset, and cascaded into skippingdocker/mcp-registry/security— a published version with no image, no registry listing, no deploy.Fix: the
detectstep now runs withif: always(), anddocker's gate isif: always() && needs.release.outputs.released == 'true'.Double-
id: detectinvestigation: the current workflow (877 lines vs. #26's much smaller base) does carry a second step namedid: detect, inside amcpbjob added after #26 was opened. Read both in full — it is not a duplicate release-detection block. It's an unrelated step ("Detect MCPB support") that checks for apack:mcpbscript inpackage.json, scoped to its own job. #26's fix applies to exactly the one release-detection occurrence and the onedockergate it always targeted; nothing was duplicated blindly.Flagged (not fixed, out of scope for this reconciliation): the
mcpbjob's own gate (if: needs.release.outputs.released == 'true') has an analogous latent exposure to the same post-release-hiccup skip, since it alsoneeds: [release]withoutalways(). It didn't exist when #26 was authored and isn't part of any of the three PRs' original content — noted in the CHANGELOG entry and here for a possible follow-up.Verification
shellcheck .github/scripts/dependabot-janitor.sh— clean (baselinemainwas already clean; no warnings introduced).shellcheck .github/scripts/dependabot-janitor.test.sh— only pre-existing info-levelSC2016/SC2015notices inherited verbatim from fix(janitor): classify grouped PRs per-dependency, fail-closed (supersedes #23) #54's original file; the one warning-levelSC2034(unused$ORG) that fix(janitor): classify grouped PRs per-dependency, fail-closed (supersedes #23) #54's original had is now fixed by the reapplication.bash .github/scripts/dependabot-janitor.test.sh— 14/14 pass.python3 -c "import yaml,sys; yaml.safe_load(open('.github/workflows/mcp-server-release.yml'))"— parses clean; gating logic inspected manually (if: always()ondetect,if: always() && needs.release.outputs.released == 'true'ondocker).classify()and diffed againstgit show pr-54:.github/scripts/dependabot-janitor.sh(branchfix/janitor-group-perdep-rebased) — only the deliberate, documented$ORG→$repochange. Full diff ofdependabot-janitor.test.shagainst fix(janitor): classify grouped PRs per-dependency, fail-closed (supersedes #23) #54's original — only the same adaptation plus the unused-var removal. No content silently dropped or altered from fix(janitor): classify grouped PRs per-dependency, fail-closed (supersedes #23) #54's original fix.Test plan
shellcheckon the reapplied shell script and new test scriptdependabot-janitor.test.sh— 14/14 assertions passmcp-server-release.ymldetectstep,dockerjob,mcpbjob) to confirm scopeclassify()and new test file against fix(janitor): classify grouped PRs per-dependency, fail-closed (supersedes #23) #54's original content🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.