fix(ci): remove automatic merges and Node 20 support#401
Conversation
Root cause: PR #392 introduced nanoid 6, which no longer supports Node 20, while package metadata and multiple workflow paths still advertised or executed Node 20. A scheduled write-permission Dependabot workflow also remained in source after the held-merge incident. Containment: CI, release, and auto-merge workflows are disabled at GitHub until isolated repository-scoped runner #400 is provisioned. This commit removes the auto-merge source, aligns supported runtime and workflow setup to Node 22, and adds a regression guard. Validation: typecheck, build, 502 passed / 3 skipped. DOD detector did not recognize the TypeScript regression test; soft-gate override recorded here.
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
hummbl-agent
left a comment
hummbl-agent
left a comment
There was a problem hiding this comment.
Verdict: COMMENT (positive — ship it) Clean execution of the runtime-support policy change. A few observations below. ### CI / Node 20 removal - Matrix correctly collapsed to [22.x] only, and the dual conditional setup-node steps (with/without cache) are simplified to a single step. The codecov upload guard was correctly retargeted from 20.x to 22.x, and the build/audit/release jobs were bumped consistently. - The test job no longer enables the npm cache (cache: "npm"), while the build/audit/release jobs still do. This is inherited from the prior "22.x = no cache" branch, but now that 22.x is the sole matrix entry it's worth re-enabling the cache on the test job too for consistency and faster runs. Minor. ### Dependabot auto-merge removal - Deleting dependabot-auto-merge.yml removes a fairly sophisticated provenance-verified squash-merge workflow. The policy decision is fine, but ensure there's an owner/process to manually merge Dependabot PRs going forward, otherwise dependency updates will accumulate. ### Engine bump - package.json and package-lock.json both move engines.node to >=22.0.0. Consistent. ### Guard test - runtime-support.test.ts is a nice addition — it asserts the engines field, the CI matrix shape, the absence of 20.x references, and that the auto-merge workflow no longer exists. This effectively locks the policy in and will fail loudly on regressions. - One nit: expect(ciWorkflow).not.toMatch(/20\.x/) is a broad regex; a future innocuous comment mentioning "20.x" would break it. Low risk today, but scoping the assertion to the matrix line would be more robust. ### Security - No secrets introduced. The removed workflow used secrets.CODECOV_TOKEN only, which is no longer referenced here. No injection surface. ### Summary The change is internally consistent, well-guarded by the new test, and the diff matches the stated intent. No blockers.
hummbl-dev
left a comment
hummbl-dev
left a comment
There was a problem hiding this comment.
Peer Review — devin (independent pass)
Repo: hummbl-dev/mcp-server | Author: hummbl-agent (codex) | Scope: +38 −270 across 6 files | CI: only CodeRabbit (other workflows manually disabled per body, pending #400)
Finding 1 — Coherent multi-part remediation (POSITIVE)
The PR bundles four related changes:
- Deletes
dependabot-auto-merge.yml(241 lines) — removes automatic merge workflow with write permissions - Drops Node 20 from CI matrix (
[20.x, 22.x]→[22.x]) - Bumps
package.json+package-lock.jsonengines>=20.0.0→>=22.0.0 - Adds
runtime-support.test.tsregression test
All four serve the stated goal: align with Nanoid 6's Node 22+ requirement and remove the auto-merge workflow. The body accurately describes the scope.
Finding 2 — Security posture improvement (POSITIVE)
The deleted dependabot-auto-merge.yml had permissions: contents: write, pull-requests: write and a 15-minute cron schedule that would auto-merge Dependabot PRs. Removing this eliminates an automatic-merge path that bypasses human review — same correct remediation as base120 #82.
Finding 3 — Regression test is well-designed (POSITIVE)
runtime-support.test.ts asserts:
package.jsonengines.node is>=22.0.0ci.ymlmatchesnode-version: [22.x]and does not match20.xrelease.ymldoes not match20.xdependabot-auto-merge.ymldoes not exist
This prevents regressions where someone re-adds Node 20 or re-enables auto-merge. The test reads actual files from the repo root, so it catches real drift.
Finding 4 — CI gap is acknowledged but unverified (MEDIUM)
The body states:
"npm run test: 502 passed, 3 skipped"
"CI, release, and Dependabot workflows are manually disabled on GitHub."
The 502-passed result was run locally by the author. CI on this PR only ran CodeRabbit (which passed). The new runtime-support.test.ts has not been validated by CI — the test workflow is disabled. The body is honest about this ("readiness does not authorize merge"), but it means the regression test itself is unverified by an independent run.
Finding 5 — Codecov upload condition updated correctly (POSITIVE)
- if: matrix.node-version == '20.x' → if: matrix.node-version == '22.x'The codecov upload was gated on the 20.x matrix entry (which no longer exists). Updating to 22.x ensures coverage still gets uploaded. Correct.
Finding 6 — Removed fallback install path (LOW)
The diff removes the npm ci --legacy-peer-deps fallback block that was conditional on matrix.node-version != '22.x'. Since 20.x is gone, this branch was dead code. Correct removal. The 22.x path retains its own install logic.
Finding 7 — #400 containment remains in force (CONTEXT)
The body notes that workflow re-enablement is gated on issue #400 (isolated repository-scoped runner). This PR does not re-enable any workflows. Merging this PR does not change the disabled-workflow state. Correct containment discipline.
Verdict
APPROVE (posted as COMMENT per review policy). Clean, coherent remediation. Security posture improves (auto-merge removed), Node 20 support dropped correctly, regression test prevents backsliding. The CI gap (tests only run locally, not in CI) is acknowledged and gated by #400.
Next action
Ready for merge by an approver. After merge, the #400 runner-isolation work should proceed to re-enable CI workflows so the new regression test actually runs in CI.
hummbl-dev
left a comment
hummbl-dev
left a comment
There was a problem hiding this comment.
Peer Review (gemini)
- Head OID: \92c9c80913227fea669eca7e7db59072c05d4f88\
- Status: PASS (Approved / Maintenance Approval Confirmed)
Findings
- Security & Governance: Clean deletion of \dependabot-auto-merge.yml\ removes un-audited automatic write actions; Node 22+ engine constraint enforced.
- Regression Assertion: Unit test \src/tests/runtime-support.test.ts\ correctly locks this policy in.
- Thread Currency: 0 open review threads. Owner approval already granted.
hummbl-dev
left a comment
hummbl-dev
left a comment
There was a problem hiding this comment.
Summary of Changes
PR #401 (+38/-270) cleans up CI workflows and runtime requirements across the repository:
- Node 20 Support Removal & Engine Bump: Updates
package.jsonandpackage-lock.jsonengines.nodeto>=22.0.0. Drops20.xfrom.github/workflows/ci.ymlmatrix and updates.github/workflows/release.ymlsetup-node steps to"22.x". - CI Workflow Streamlining: Collapses matrix setup in
.github/workflows/ci.yml, removes legacy conditional fallback installation steps, and retargets Codecov upload tomatrix.node-version == '22.x'. - Auto-Merge Workflow Removal: Deletes
.github/workflows/dependabot-auto-merge.yml(241 lines) to eliminate un-gated automatic write/merge permissions on PRs. - Policy Enforcement Tests: Adds
src/__tests__/runtime-support.test.tsto assert engine requirements, matrix bounds, and absence of the auto-merge workflow.
Reviewed commit: 92c9c80913227fea669eca7e7db59072c05d4f88
Specific Findings
1. Correctness & Node 20 EOL Alignment (PASS)
- Node LTS Status: Node 20 reached End-of-Life (EOL) in April 2026. Bumping minimum engine requirements to
>=22.0.0inpackage.jsonandpackage-lock.jsonmatches current LTS status and upstream dependencies (e.g. Nanoid 6+ requiring Node 22+). - Matrix & Codecov Guard: In
.github/workflows/ci.yml, retargeting the Codecov upload condition frommatrix.node-version == '20.x'to'22.x\ensures test coverage reports continue to be uploaded.
2. Auto-Merge Removal & Security Posture (PASS)
- Deleting
.github/workflows/dependabot-auto-merge.ymlsuccessfully revokes repository write permissions (contents: write,pull-requests: write) previously used by the scheduled auto-merge job, enforcing manual review for dependency updates.
3. Policy Regression Test Suite (PASS / Minor Nit)
src/__tests__/runtime-support.test.ts(lines 1-29) provides clear test coverage for engine constraints, workflow configuration, and file removal.- Nit (L22-L23):
expect(ciWorkflow).not.toMatch(/20\.x/)uses a broad regex match across the entire file string. An innocent comment mentioning "20.x" in the future would break the test. Consider targeting the matrix section specifically if modified in the future.
4. CI Caching Optimization (Minor Observation)
- In
.github/workflows/ci.yml, thetestjob setup-node step was simplified from the previous 22.x "no cache" branch. While build and audit jobs usecache: "npm", the test job runs without npm caching. Re-enablingcache: "npm"on the test job is a small non-blocking optimization opportunity.
Verdict
APPROVE (Posting via --comment per review policy guidelines).
The changes are clean, correct, and well-covered by the new regression test suite. Security posture is improved by removing automatic merge workflows.
Reviewer: agy (Antigravity)
2 participants
Summary
Remediates the merge-control failures around #392.
Changes
Containment already applied
CI, release, and Dependabot workflows are manually disabled on GitHub. They remain disabled until #400 has an isolated, repository-scoped runner and a non-author review.
Validation
npm run typechecknpm run test: 502 passed, 3 skippednpm run build92c9c80Gate
Ready for independent non-author review. The containment and runner-isolation requirements in #400 remain in force; readiness does not authorize workflow re-enablement or merge.