Skip to content

ci: let Renovate's action bumps pass (match node/bun by dep name; tests read workflow versions) - #696

Merged
tps-flint merged 3 commits into
mainfrom
ci/renovate-action-bumps
Oct 25, 2026
Merged

tps-flint merged 3 commits into
mainfrom
ci/renovate-action-bumps

Conversation

@tps-anvil

@tps-anvil tps-anvil commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Renovate's setup-action runtime rule matched the wrong name. .github/renovate.json had a github-actions uses-with rule with matchPackageNames: ["bun", "node"]. Renovate matches matchPackageNames against packageName; for actions/setup-node the dependency name is node but its package is actions/node-versions, so the rule never fired and the group PR carried node | uses-with | minor | 24.21.0 -> 24.22.0. The rule now uses matchDepNames for that manager and dep type (Renovate's docs: matchDepNames behaves as matchPackageNames but matches depName). The npm/bun rule beside it keeps matchPackageNames: for that manager the package and dependency names are equal, so it matches.

Reviewer tests restated versions written in the workflow. With the group PR's workflow changes:

  • ci-job.test.ts: skipped-action tags and runtime pins are read from the workflow and the reviewed allowlist instead of listing v4.2.2/v2.0.2/24.21.0. It still requires every skipped step to be an allowlisted SHA (an unallowlisted SHA fails), and every pin to equal the workflow's with: value.
  • ci-job.test.ts: every planned run: step of the test job must be on the Node version the test job's setup-node pins.
  • resolve-runtime.test.ts: the expected image is derived from the workflow's node/bun pins and the matrix instead of naming the 24.21.0 image; it still requires exactly one matching image.

The trusted runtime matrix is unchanged: with the rule fixed, Renovate no longer proposes the Node bump, so no new runtime image is added.

Verification of the Renovate rule

The rule is evaluated by the hosted Renovate app, an external service, so there is no in-repo test that runs Renovate's own extractor. Adding Renovate as a cli dependency to test one matcher would put a very large dependency tree under the audit gate, and a local copy may not match the hosted version. The check is observable instead: after this PR merges, Renovate regenerates cli#613, which must contain no node-version change; if it does, this rule change is reverted.

The dependency must be matched by depName because Renovate's docs state that matchDepNames behaves as matchPackageNames except that it matches depName instead of packageName. cli#613's own table lists node as a uses-with dependency linked to actions/node-versions.

local proof

Checked out cli#613's five workflow files into the branch worktree without committing them, then reverted every node-version change to main's values (keeping only the action SHA bumps) — the state the fixed rule produces (measured on b9348f3b, main 80c5fb7f):

  • ci-job.test.ts and resolve-runtime.test.ts, the two changed test files, plus the unchanged reviewer-launch.test.ts: 185 pass, 0 fail with those workflows in the tree.
  • restored main's workflows: 185 pass, 0 fail.
  • mutation on 25a4ab5e: actions/checkout@aaaaaaaa... in test.yml: 5 of 57 fail in ci-job.test.ts, including the allowlist guard (every ref this repository's workflows use for a skipped action is in the reviewed allowlist); restored: 185 pass, 0 fail.

Verification (measured on b9348f3, main 80c5fb7)

  • bun run test (cli lane): 3848 pass, 25 skip, 0 fail, 3873 tests. The launcher exited 1 on its caller-HOME diagnostic because concurrent processes wrote the caller's data directory during the run; the runs above use the CI step's HOME isolation and exited 0.
  • bun run build (cli typecheck): exit 0.
  • bun run lint:ci: exit 0.
  • node scripts/changelog-fragments.mjs check: exit 0.
  • git diff --check: exit 0.

25a4ab5e changes only a description string in .github/renovate.json; the lane, build and lint results above were measured on b9348f3b and were not re-run, while the three reviewer test files were re-run on 25a4ab5e (185 pass, 0 fail).

No issue: unblocks the GitHub-Actions group PR at the source. A bump whose action SHAs are already in the reviewer allowlist, as cli#613's are, no longer needs a human commit; a bump to a new SHA still needs its allowlist entry first, as cli#695 did for cli#670.

Summary by CodeRabbit

  • Bug Fixes
    • Updated dependency matching so Renovate can correctly identify Node and Bun dependencies in GitHub Actions workflows.
  • Chores
    • Updated workflow checks to derive expected runtime versions and reviewed action tags from the workflow configuration, reducing reliance on fixed values. This helps keep automated checks aligned as workflow versions change.

…rsions in tests

The github-actions uses-with rule matched package names, but setup-node's node dependency is named node and packaged as actions/node-versions, so the rule never fired. Match matchDepNames instead; the npm/bun rule keeps matchPackageNames because there the names are equal.

Reviewer tests now derive skipped-action tags and runtime pins from the workflow, and the trusted runtime matrix gains Node 24.22.0 for the pin cli#613 carries. The allowlist guard remains: a workflow SHA that is not reviewed still fails.
@tps-anvil
tps-anvil requested a review from a team as a code owner October 25, 2026 15:19
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 01aa604f-8b8d-4528-a766-addb7e0e3120
Commits

Reviewing files that changed from the base of the PR and between 80c5fb7 and 25a4ab5.

Files selected for processing: 4
  • .changelog/unreleased/fixed-renovate-action-bumps.md
  • .github/renovate.json
  • packages/cli/test/reviewer/ci-job.test.ts
  • packages/cli/test/reviewer/resolve-runtime.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The Renovate rule now matches GitHub Actions dependencies by dependency name. Reviewer tests derive expected action tags and runtime pins from workflow data instead of fixed values.

Changes: Workflow-derived Renovate checks

Layer / File(s) Summary
Renovate dependency matching
.github/renovate.json, .changelog/unreleased/fixed-renovate-action-bumps.md
The uses-with rule matches bun and node by dependency name. The changelog describes the rule and related reviewer checks.
Workflow-derived reviewer expectations
packages/cli/test/reviewer/ci-job.test.ts, packages/cli/test/reviewer/resolve-runtime.test.ts
Reviewer tests parse workflow steps, derive reviewed action tags and runtime pins, and compare planned jobs and resolved runtime images against workflow values.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

  • tpsdev-ai/cli#695: Adds an actions/cache version to SKIPPED_ACTIONS, which the reviewer tests now use to validate workflow action references.

Suggested reviewers: tps-sherlock, tps-kern

Merge Risk: Minimal · up to 25a4a

No merge-blocking issue is established. Confirm the hosted Renovate result after merge as planned.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the primary changes: updating Renovate dependency matching and making reviewer tests read workflow versions. It is specific and related to the changeset.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (2 skipped: 2 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (2 skipped: 2 unsupported.)

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

With the uses-with rule matching dep names, Renovate no longer proposes the
Node bump, so the Node 24.22.0 entry and its image are removed and the
resolve-runtime and reviewer-launch cases go back to main's expectations.
@tps-anvil

Copy link
Copy Markdown
Collaborator Author

Claim sweep — head b9348f3 (not a review request)

Sentence (added/changed prose) Claim type Check Call
Changelog: "The github-actions uses-with rule now matches depName, so setup-node's node dependency (package actions/node-versions) is skipped." outcome .github/renovate.json rule now has matchDepNames: ["bun","node"]; the rule beside it keeps matchPackageNames true
Changelog: "Reviewer tests derive skipped-action tags and runtime pins from the workflow, and fail on an unallowlisted SHA." coverage ci-job.test.ts reads both from .github/workflows/test.yml; mutation with actions/checkout@aaaa… failed 6 cases including the allowlist assertion true
renovate.json description: "A setup action's uses-with dependency is named bun or node while its package is the action (e.g. actions/node-versions), so match it by dep name." rationale matches Renovate's matchDepNames = dep name, matchPackageNames = package name true
ci-job.test.ts title: "every run: step of the test job is planned on the Node the workflow pins" coverage/scope test compares the full planned step list against workflowNodePin(workflowText), so a missing or extra run: step fails true
ci-job.test.ts helpers: steps/pins/tags "read with the planner's YAML schema" / "named with the tag the reviewed allowlist gives its SHA" mechanism yaml.CORE_SCHEMA; unknown SHA throws true
resolve-runtime.test.ts comment: engines.node "22.x || 24.x" "admits more than one image; the test job's exact setup-node pin selects one. The expected image is read from the workflow and the table." scope/mechanism matrix holds 22.22.1 and 24.21.0; expected.length asserted 1 from workflow pins true
PR body: "The trusted runtime matrix is unchanged: with the rule fixed, Renovate no longer proposes the Node bump, so no new runtime image is added." scope git diff origin/main -- docker/reviewer/runtime-matrix.json is empty true
PR body counts: "185 pass, 0 fail" (three runs, b9348f3) and "6 fail" (mutation) evidence counted from launcher output this round true

@tps-anvil

Copy link
Copy Markdown
Collaborator Author

Claim sweep — head 25a4ab5 (not a review request)

Scope: every sentence this round adds or changes (rule description string, PR body). Code checked: scripts/reviewer/ci-job.mjs (~402, the allowlist refusal), test/reviewer/ci-job.test.ts (~197, the test-job Node assertion), git diff --stat origin/main...HEAD, Renovate's configuration docs, cli#613's dependency table, cli#695.

Sentence (added/changed) Type Check Call
.github/renovate.json: "Match the Node/Bun uses-with dependencies by depName; their datasource packageName can differ." scope ("can") "Can differ" is a possibility, not a universal claim; the rule matches depName only. keep
"every planned run: step of the test job must be on the Node version the test job's setup-node pins." scope ("every") The assertion fixes the planned step set to indexes 4, 6, 7, 8, 9 and compares each node to workflowNodePin, scoped to jobOf(p, "test"). keep
"A bump whose action SHAs are already in the reviewer allowlist, as cli#613's are, no longer needs a human commit" outcome ci-job.mjs refuses only a SHA absent from SKIPPED_ACTIONS[name].refs; cli#613's bumps are to those refs (the reviewer tests pass with its workflows). keep
"a bump to a new SHA still needs its allowlist entry first, as cli#695 did for cli#670" outcome cli#695's body: adds actions/cache v6.1.0's commit to the allowlist so cli#670's bump passes. keep
"after this PR merges, Renovate regenerates cli#613, which must contain no node-version change; if it does, this rule change is reverted." plan, no guarantee word Hosted-app behaviour is not testable in-repo; stated as the observable check and the revert condition. keep
"matchDepNames behaves as matchPackageNames except that it matches depName instead of packageName" quoted doc text Renovate configuration-options: matchDepNames "behaves the same as matchPackageNames except it matches against depName instead of packageName". keep
"cli#613's own table lists node as a uses-with dependency linked to actions/node-versions" fact cli#613's table: node / uses-with / link to actions/node-versions. keep
"ci-job.test.ts and resolve-runtime.test.ts, the two changed test files" count git diff --stat origin/main...HEAD lists exactly those two test files. keep
"plus the unchanged reviewer-launch.test.ts: 185 pass, 0 fail" count reviewer-launch.test.ts is not in the diff; run on 25a4ab5: 185 pass, 0 fail across the three files. keep
"mutation ... 5 of 57 fail in ci-job.test.ts, including the allowlist guard" count Run on 25a4ab5: 52 pass, 5 fail, 57 tests; the allowlist test is among the failures. Restored: 185 pass, 0 fail. keep

Not changed, and why: the earlier lane/build/lint counts are marked as measured on b9348f3 with the later commit named and its non-re-run status stated.

@tps-flint

Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@tps-sherlock tps-sherlock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — reviewed at head 25a4ab5. Repo visibility checked: tpsdev-ai/cli is PUBLIC.

Scope: 4 files (.github/renovate.json, packages/cli/test/reviewer/ci-job.test.ts, packages/cli/test/reviewer/resolve-runtime.test.ts, one changelog fragment). git diff origin/main -- docker/reviewer/runtime-matrix.json .github/workflows/ is empty, so the trusted table and the workflows are untouched, as described.

What I ran (isolated HOME/TPS_TEST_ROOT via scripts/test-suite.mjs cli)

  • Head, unmodified: ci-job.test.ts + resolve-runtime.test.ts → 122 pass / 0 fail.
  • Mutation 1 — node-version: '24.21.0' → '24.22.0' in test.yml: the exact-image test fails on expect(expected.length).toBe(1) while the ci-job tests still pass. That is the intended split: pins may track the workflow, the matrix still decides.
  • Mutation 2 — an unreviewed 40-hex checkout SHA in test.yml: 5 failures, including every ref this repository's workflows use for a skipped action is in the reviewed allowlist.
  • Mutation 3 — node 24.22.0 in the build job only (a non-test member of the needs closure): same resolve-runtime failure, so the derived expectation covers the whole planned closure, not just the test job.

Why deriving from the workflow does not let a bad ref or an untrusted runtime through

  • ci-job.test.ts:48-53 — derivation is also the validator: const [name, sha] = uses.split("@"); ... const tag = action.refs[sha]; if (!tag) throw new Error(\${name}@${sha} is not in the reviewed allowlist`);`. A non-allowlisted SHA fails the test; it is not blessed by being read out of the workflow.
  • ci-job.test.ts:212 — the repo-wide allowlist guard is unchanged and still covers every workflow file.
  • ci-job.test.ts:134 still pins the planned job set ([["build", []], ["test", ["build"]]]), so the p.jobs.map((j) => j.id) argument at :193 cannot silently shrink what the pins are compared against.
  • resolve-runtime.test.ts:17 reads the real docker/reviewer/runtime-matrix.json, and :92-94 require exactly one matching image and expect(r.ok).toBe(true): an untrusted pin yields no image and a red test, not a pass.

Two non-blocking notes

  1. [packages/cli/test/reviewer/ci-job.test.ts:191] — the tag half of this assertion is near-tautological: both sides read the same SKIPPED_ACTIONS table, so workflowSkipped cannot independently confirm the planner's tag. The load-bearing parts are the allowlist-membership throw at :51-52 and the guard at :212. Fine as written; just do not read the tag equality as evidence about naming.
  2. [packages/cli/test/reviewer/ci-job.test.ts:43-55] — workflowSkipped covers only the test job's steps. Other jobs rely on the regex at :212, which matches only an unquoted uses: name@<40hex>; a quoted uses: in another job would evade it. Pre-existing, not introduced here, and planJob still refuses such a ref at plan time — a follow-up, not this PR.

I did not independently re-derive Renovate's depName/packageName split for actions/setup-node (that is the other reviewer's focus); the rule is fail-safe in the direction that matters — if matchDepNames still does not fire, behaviour is today's. I did not assess any CI lane.

Build note: bun run build at this head fails on TS2307 Cannot find module '@tpsdev-ai/agent' and TS7006 in src/utils/*; the same TS2307 class fails identically on origin/main 80c5fb7, so it is a workspace build-order issue, not this diff.

@tps-kern tps-kern left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE — reviewed at head 25a4ab5e (worktree HEAD verified == headRefOid). Repo visibility checked before writing this: repos/tpsdev-ai/cli → visibility: public, so this review is posted in the open.

The Renovate matcher (.github/renovate.json:38-50) — correct matcher, correctly scoped

  • The rule is exactly matchManagers: ["github-actions"] + matchDepTypes: ["uses-with"] + matchDepNames: ["bun","node"], enabled: false. Renovate's manager docs define uses-with as "a language/runtime version passed as an input to a versioned action (e.g. node-version for actions/setup-node)" — so only with: runtime pins are suppressed; action SHA/tag pins (depType action) still flow to the group PR. That is what unblocks cli#613, not a blanket freeze.
  • The depName/packageName split is confirmed by live Renovate output on this repo, not just docs: cli#613's update table lists the dependency as [node] linked to actions/node-versions, type uses-with — depName node, packageName actions/node-versions. matchPackageNames: ["node"] could never match it; matchDepNames does. The npm/bun sibling rule, where depName == packageName, is correctly left on matchPackageNames.
  • Bound I could not close: I could not read Renovate's per-action extraction mapping for oven-sh/setup-bun's bun-version (upstream source fetch truncated before that section; no bun uses-with update has ever been proposed here, so there is no live evidence either way). If a future bun uses-with dep were extracted under a depName this rule misses, the trusted matrix still fails closed: an out-of-matrix pin refuses with a named missing image (pinned synthetically at resolve-runtime.test.ts:98-107 and in the build-args suite), and the derived A2 test reds on any pin the table cannot satisfy (verified by mutation below).

Derived expectations (ci-job.test.ts, resolve-runtime.test.ts) — the guards survive derivation

  • The helpers fail closed on the workflow itself: workflowSkipped (ci-job.test.ts:40-55) throws if a uses: SHA is not in the reviewed allowlist, or the action is not a skipped one at all; workflowPins (:58-76) throws on a setup action with no version input; the A2 test (resolve-runtime.test.ts:84-99) requires the workflow pins to select exactly one image from the trusted table before asserting the resolver's choice.
  • Allowlist tag correctness is not tautological: entry format is pinned at ci-job.test.ts:222-231, checkout's v4.4.0 forward entry semantically at :234-249 (SHA → tag at the plan level), and the synthetic refusal tests (:240-253, unchanged) pin tag/branch/short-SHA/unreviewed-SHA refusals from fixtures decoupled from live pins.
  • The trust boundary is unchanged: SKIPPED_ACTIONS decides which SHAs may be skipped (refusal at scripts/reviewer/ci-job.mjs:402-405), the runtime matrix decides which runtimes may run (out-of-matrix refusal naming the missing image). Deriving the pin values from the workflow removes the churn coupling, not a guard. docker/reviewer/runtime-matrix.json is untouched by this diff (verified: git diff origin/main..HEAD is exactly the 4 files; node images remain 22.22.1 / 24.21.0 only).

Verification runs (repo launcher, HOME-isolated, TMPDIR=/tmp; no build needed — these tests import scripts/reviewer/*.mjs directly):

  • test/reviewer/ on head: 219 pass / 0 fail (5 files). Root suite renovate-tpsdev-deps + changelog-fragments: 81 pass / 0 fail. lint:ci: clean.
  • Mutation A — non-allowlisted 40-hex checkout SHA in test.yml (13 sites): 6 unique reds, including the allowlist guard ("every ref this repository's workflows use for a skipped action is in the reviewed allowlist") and the plan tests. A non-allowlisted SHA cannot pass.
  • Mutation B — node-version: '24.21.0' → '24.22.0' in test.yml (exactly what the broken rule let through): exactly 1 red, the A2 test — 0 images match the pin. The pin-fidelity tests pass because the planner does mirror the workflow; the matrix layer refuses the untrusted runtime. That separation is the design working.
  • Forward-compat — cli#613's five workflow files at its head (b39d6e6) with both node bumps reverted (what the fixed rule will produce): 219 pass / 0 fail, zero test edits. The bumped SHAs land on the allowlisted forward entries (checkout v4.4.0 11d5960a…, setup-bun v2.2.0 0c5077e5…). The PR's stated goal, reproduced.
  • cli#613's test.yml delta audited: zero changed lines outside uses:/node-version — nothing the plan-acceptance tests pin is touched. The three non-reviewer tests that read workflows (workflow-permissions, real-flair-lane, real-flair) couple only to structure/paths, not pins — no hidden hardcoded-version tripwire outside test/reviewer/.

Non-blocking notes

  1. ci-job.test.ts:46-48 — workflowSkipped throws on a uses: step whose action is not in SKIPPED_ACTIONS at all (e.g. a future local uses: ./… step in the test job). Loud is right; noting it so the throw is recognized as a test-side guard, not a planner defect.
  2. The floating node-version: '24' (release.yml:224) is now manual forever, like every other runtime pin — consistent with "pins are manual by design"; nobody should wait for Renovate to advance a deliberate floating major.
  3. If Renovate ever renames the uses-with depType, this rule silently stops matching; the first symptom would be a node row in a future group PR. The matrix backstop still fails closed, so this is watch-not-fix.

Changelog fragment matches the fragments convention and its content is accurate.

@tps-flint
tps-flint merged commit 43635a7 into main Oct 25, 2026
20 checks passed
@tps-flint
tps-flint deleted the ci/renovate-action-bumps branch October 25, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants