Skip to content

Merge updates from base#1182

Closed
bainco wants to merge 6 commits into
peaceiris:mainfrom
bainco:main
Closed

Merge updates from base#1182
bainco wants to merge 6 commits into
peaceiris:mainfrom
bainco:main

Conversation

@bainco
Copy link
Copy Markdown

@bainco bainco commented Jun 3, 2026

Summary by CodeRabbit

  • Chores
    • Updated Node.js runtime from version 20 to version 24.
    • Upgraded GitHub Actions dependencies to latest stable versions.
    • Updated CI/CD workflows to support latest Ubuntu, macOS, and Windows environments.
    • Streamlined deployment configuration.

@bainco bainco requested a review from peaceiris as a code owner June 3, 2026 00:20
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR upgrades the action runtime to Node.js 24, updates CI workflows to use modern action versions and broader OS matrices (ubuntu-latest, macOS, Windows), and adds a new standalone child process runner script. The release workflow uses actions/checkout@v6, test workflows migrate from custom setup-node to actions/setup-node@v6, deploy conditions are simplified, and a new DNS domain is configured for deployments.

Changes

Node.js Runtime and CI Infrastructure Updates

Layer / File(s) Summary
Node.js version baseline
.nvmrc, action.yml
Node.js is pinned to version 24.14.0 in .nvmrc and action runtime is set to node24.
Release and test workflow action upgrades
.github/workflows/release.yml, .github/workflows/test.yml
Release workflow upgrades actions/checkout to v6. Test workflow updates the OS matrix to include ubuntu-latest, macos-latest, and windows-latest, replaces peaceiris/workflows/setup-node with actions/setup-node@v6 while keeping .nvmrc for version source, removes github.event.repository.fork == false from the deploy condition, and updates the deployment domain from actions-gh-pages.peaceiris.com to actions-gh-pages.bainco.com.

Child Process Execution Script

Layer / File(s) Summary
Child process runner implementation
lib/exec-child.js
New standalone script that reads JSON parameters from a file, executes a command via child_process.exec, streams both stdout and stderr to configured files and the parent process, conditionally pipes input when provided, and sets process.exitCode based on command success or failure. The script includes a guard to prevent accidental require() loading.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • peaceiris/actions-gh-pages#1156: Updates actions/checkout and related action references in CI/release workflows to newer major versions; this PR extends that pattern further and adds commit SHA pinning.
  • peaceiris/actions-gh-pages#1147: Shares the same Node.js 24 baseline updates across .nvmrc, action.yml, and test workflows with this PR.

Suggested labels

cicd

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Merge updates from base' is vague and generic, providing no meaningful information about the specific changes (Node.js version updates, workflow changes, action runtime changes). Use a more descriptive title that summarizes the main change, such as 'Update Node.js to v24 and upgrade GitHub Actions' or 'Upgrade Node.js 24 and update CI/CD workflows'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch main

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 and usage tips.

@actions-gh-pages actions-gh-pages Bot added dependencies Pull requests that update a dependency file cicd labels Jun 3, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/exec-child.js`:
- Around line 8-11: Add a guard to validate process.argv[2] (paramFilePath)
before using it: check that process.argv[2] exists and is a non-empty string,
and if not log a clear error and exit (or throw) rather than letting
fs.readFileSync/JSON.parse fail with obscure errors. Also wrap the
fs.readFileSync + JSON.parse sequence (serializedParams/params) in a try/catch
that rethrows or logs a descriptive message including paramFilePath and the
original error to make failures deterministic and debuggable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 81482b88-20b2-4cb1-b848-f1d56e9055af

📥 Commits

Reviewing files that changed from the base of the PR and between c750ac4 and f6a34a2.

📒 Files selected for processing (6)
  • .github/workflows/release.yml
  • .github/workflows/test.yml
  • .nvmrc
  • action.yml
  • lib/exec-child.js
  • lib/index.js

Comment thread lib/exec-child.js
Comment on lines +8 to +11
var paramFilePath = process.argv[2];

var serializedParams = fs.readFileSync(paramFilePath, 'utf8');
var params = JSON.parse(serializedParams);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ls -la
sed -n '1,120p' lib/exec-child.js

Repository: peaceiris/actions-gh-pages

Length of output: 3121


Add explicit argument validation for process.argv[2] in lib/exec-child.js.

Without a guard, missing/invalid process.argv[2] causes fs.readFileSync / JSON.parse to throw with little context about the missing CLI argument. A small upfront check makes failures clearer and deterministic.

Minimal hardening
 var paramFilePath = process.argv[2];
+if (!paramFilePath) {
+  throw new Error('Missing params file path at process.argv[2]');
+}
 
 var serializedParams = fs.readFileSync(paramFilePath, 'utf8');
 var params = JSON.parse(serializedParams);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var paramFilePath = process.argv[2];
var serializedParams = fs.readFileSync(paramFilePath, 'utf8');
var params = JSON.parse(serializedParams);
var paramFilePath = process.argv[2];
if (!paramFilePath) {
throw new Error('Missing params file path at process.argv[2]');
}
var serializedParams = fs.readFileSync(paramFilePath, 'utf8');
var params = JSON.parse(serializedParams);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/exec-child.js` around lines 8 - 11, Add a guard to validate
process.argv[2] (paramFilePath) before using it: check that process.argv[2]
exists and is a non-empty string, and if not log a clear error and exit (or
throw) rather than letting fs.readFileSync/JSON.parse fail with obscure errors.
Also wrap the fs.readFileSync + JSON.parse sequence (serializedParams/params) in
a try/catch that rethrows or logs a descriptive message including paramFilePath
and the original error to make failures deterministic and debuggable.

@peaceiris peaceiris closed this Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cicd dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants