workflows: add nsolid release automation#487
Conversation
c0ee188 to
f966eac
Compare
WalkthroughThis PR sets tracked Node.js release lines to 22 and 24, adds scheduled and manual GitHub Actions workflows, introduces watcher and proposal automation scripts, and documents the release automation flow. ChangesN|Solid release automation
Sequence Diagram(s)sequenceDiagram
participant WatchNodeReleasesWorkflow as Watch Node.js Releases workflow
participant watch_node_releases_mjs as tools/actions/watch-node-releases.mjs
participant GitHubAPI as GitHub REST API
participant create_nsolid_release_proposal_sh as tools/actions/create-nsolid-release-proposal.sh
WatchNodeReleasesWorkflow->>watch_node_releases_mjs: run on cron or workflow_dispatch
watch_node_releases_mjs->>GitHubAPI: fetch release index, refs, and issues
GitHubAPI-->>watch_node_releases_mjs: release data and existing refs/issues
watch_node_releases_mjs->>create_nsolid_release_proposal_sh: spawnSync(NODE_LINE, NODE_TAG, ISSUE_NUMBER)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
f966eac to
9584be8
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@tools/actions/create-nsolid-release-proposal.sh`:
- Line 130: The codename extraction in create-nsolid-release-proposal.sh is
matching NODE_VERSION_IS_LTS, which returns the LTS flag instead of the actual
release codename. Update the awk lookup to use the NODE_VERSION_LTS_CODENAME
symbol from src/node_version.h so CODENAME is populated with the real codename
used in the release title and changelog.
- Line 99: Remove the unused NSOLID_TAG assignment in
create-nsolid-release-proposal.sh since it is never referenced and triggers
lint-sh warnings; keep the script focused on the release proposal flow and only
retain tag-related variables if a later step actually consumes them. Check
nearby logic in create-nsolid-release-proposal.sh to ensure no remaining step
depends on NSOLID_TAG before deleting it.
- Around line 29-31: The unbounded fetch in the Node release-index request can
hang the workflow, so wrap the fetch call in create-nsolid-release-proposal.sh
with an AbortController-based timeout and handle the timeout error cleanly.
Apply the same bounded-fetch pattern to the equivalent release-index fetch in
watch-node-releases.mjs so both release lookups fail fast instead of waiting for
the job’s full duration.
- Around line 101-105: The early return in create-nsolid-release-proposal.sh
around the origin/${RELEASE_BRANCH} check can incorrectly report success on
reruns even when no PR exists. Update the release-branch existence handling to
inspect for an existing PR associated with origin/${RELEASE_BRANCH} before
exiting; if one exists, reuse it, and if not, fail with a clear message instead
of returning 0. Use the existing git rev-parse logic and the gh pr create flow
to locate the rerun path.
- Around line 1-3: The script currently violates the shell lint contract because
it uses a Bash shebang and Bash-only behavior. Update
create-nsolid-release-proposal.sh to be POSIX sh-compatible by using the sh
shebang and replacing any Bash-specific syntax such as set -euo pipefail, [[ ...
]], and local with POSIX equivalents, or otherwise adjust the linting setup to
explicitly treat this script as Bash. Use the script entrypoint and any affected
helper functions/conditionals as the reference points when making the changes.
In `@tools/actions/watch-node-releases.mjs`:
- Around line 40-44: The findIssue helper currently only inspects the first page
returned by githubRequest, so it can miss an existing tracking issue once the
repo has more than 100 issues. Update findIssue to either paginate through all
issue pages using the Link header or switch to the Search API with the
repository/title filters, and keep the lookup logic tied to findIssue and
githubRequest so it can locate matching non-pull_request issues regardless of
how many exist.
- Around line 141-148: The status handling in watch-node-releases.mjs is
special-casing result.status === 1, which can mask real failures from
create-nsolid-release-proposal.sh. Remove that branch, handle result.error and
result.signal explicitly after the spawn in the watch logic, and treat any
non-zero exit status as an error by propagating it through process.exitCode.
Keep the fix localized around the spawn result handling where nodeTag is logged.
🪄 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
Run ID: 96986999-e280-46e3-b6ac-aca4915624e5
📒 Files selected for processing (6)
.github/nsolid-release-lines.json.github/workflows/create-nsolid-release-proposal.yml.github/workflows/watch-node-releases.ymldoc/contributing/nsolid-release-automation.mdtools/actions/create-nsolid-release-proposal.shtools/actions/watch-node-releases.mjs
9584be8 to
c853745
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@tools/actions/create-nsolid-release-proposal.sh`:
- Around line 119-126: Treat the `gh pr list` result in
`create-nsolid-release-proposal.sh` as empty when `--jq '.[0].url'` returns
`null`, since `EXISTING_PR_URL` is then incorrectly considered non-empty. Update
the `EXISTING_PR_URL` handling around the `gh pr list` command and the
subsequent `if [ -n "$EXISTING_PR_URL" ]` check so that a literal `null` is
normalized to an empty value or explicitly rejected, ensuring the script follows
the failure path instead of reporting `PR: null`.
- Around line 27-45: The latest_node_tag() shell snippet is using top-level
await inside a node -e CommonJS invocation, so it will fail before fetch runs.
Update the node invocation in create-nsolid-release-proposal.sh to run as module
input or wrap the fetch logic in an async IIFE, keeping the existing
timeout/AbortController/error handling intact.
In `@tools/actions/watch-node-releases.mjs`:
- Around line 150-157: The proposal flow is using two different tag sources, so
`watch-node-releases.mjs` and `create-nsolid-release-proposal.sh` can drift
between issue tracking and PR creation. Update the proposal script to treat
`NODE_TAG` as the source of truth by reading and validating the passed
environment value inside `create-nsolid-release-proposal.sh`, and ensure the
watcher’s `spawnSync` call continues to pass the exact resolved `nodeTag`;
alternatively, if you prefer recomputing in the script, remove `NODE_TAG` from
the watcher and defer issue creation until after the script resolves the tag,
keeping `nodeTag`, `NODE_LINE`, and the issue/PR comments aligned.
🪄 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
Run ID: aab01cc5-481b-4580-b943-262710aa1162
📒 Files selected for processing (6)
.github/nsolid-release-lines.json.github/workflows/create-nsolid-release-proposal.yml.github/workflows/watch-node-releases.ymldoc/contributing/nsolid-release-automation.mdtools/actions/create-nsolid-release-proposal.shtools/actions/watch-node-releases.mjs
✅ Files skipped from review due to trivial changes (1)
- .github/nsolid-release-lines.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/create-nsolid-release-proposal.yml
| node -e ' | ||
| const line = Number(process.argv[1]); | ||
| const url = process.env.NODE_RELEASE_INDEX_URL; | ||
| const timeoutMs = Number(process.env.RELEASE_INDEX_TIMEOUT_MS || 30000); | ||
| const controller = new AbortController(); | ||
| const timeout = setTimeout(() => controller.abort(), timeoutMs); | ||
| let releases; | ||
| try { | ||
| const response = await fetch(url, { signal: controller.signal }); | ||
| if (!response.ok) throw new Error(`Failed to fetch ${url}: ${response.status}`); | ||
| releases = await response.json(); | ||
| } catch (error) { | ||
| if (error.name === "AbortError") { | ||
| throw new Error(`Timed out fetching ${url} after ${timeoutMs}ms`); | ||
| } | ||
| throw error; | ||
| } finally { | ||
| clearTimeout(timeout); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' tools/actions/create-nsolid-release-proposal.sh && printf '\n---\n' && node -e 'await Promise.resolve(); console.log("ok")'Repository: nodesource/nsolid
Length of output: 4344
🏁 Script executed:
sed -n '1,120p' tools/actions/create-nsolid-release-proposal.sh && printf '\n---\n' && node -e 'await Promise.resolve(); console.log("ok")'Repository: nodesource/nsolid
Length of output: 4344
latest_node_tag() needs module mode.
node -e runs this snippet as CommonJS, so the top-level await here will fail before the fetch runs. Switch this invocation to module input or wrap the body in an async IIFE.
Minimal fix
- node -e '
+ node --input-type=module -e '📝 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.
| node -e ' | |
| const line = Number(process.argv[1]); | |
| const url = process.env.NODE_RELEASE_INDEX_URL; | |
| const timeoutMs = Number(process.env.RELEASE_INDEX_TIMEOUT_MS || 30000); | |
| const controller = new AbortController(); | |
| const timeout = setTimeout(() => controller.abort(), timeoutMs); | |
| let releases; | |
| try { | |
| const response = await fetch(url, { signal: controller.signal }); | |
| if (!response.ok) throw new Error(`Failed to fetch ${url}: ${response.status}`); | |
| releases = await response.json(); | |
| } catch (error) { | |
| if (error.name === "AbortError") { | |
| throw new Error(`Timed out fetching ${url} after ${timeoutMs}ms`); | |
| } | |
| throw error; | |
| } finally { | |
| clearTimeout(timeout); | |
| } | |
| node --input-type=module -e ' | |
| const line = Number(process.argv[1]); | |
| const url = process.env.NODE_RELEASE_INDEX_URL; | |
| const timeoutMs = Number(process.env.RELEASE_INDEX_TIMEOUT_MS || 30000); | |
| const controller = new AbortController(); | |
| const timeout = setTimeout(() => controller.abort(), timeoutMs); | |
| let releases; | |
| try { | |
| const response = await fetch(url, { signal: controller.signal }); | |
| if (!response.ok) throw new Error(`Failed to fetch ${url}: ${response.status}`); | |
| releases = await response.json(); | |
| } catch (error) { | |
| if (error.name === "AbortError") { | |
| throw new Error(`Timed out fetching ${url} after ${timeoutMs}ms`); | |
| } | |
| throw error; | |
| } finally { | |
| clearTimeout(timeout); | |
| } |
🧰 Tools
🪛 GitHub Check: lint-sh
[failure] 27-27:
/home/runner/work/nsolid/nsolid/tools/actions/create-nsolid-release-proposal.sh:27:11: Expressions don't expand in single quotes, use double quotes for that.
🤖 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 `@tools/actions/create-nsolid-release-proposal.sh` around lines 27 - 45, The
latest_node_tag() shell snippet is using top-level await inside a node -e
CommonJS invocation, so it will fail before fetch runs. Update the node
invocation in create-nsolid-release-proposal.sh to run as module input or wrap
the fetch logic in an async IIFE, keeping the existing
timeout/AbortController/error handling intact.
| EXISTING_PR_URL="$(gh pr list \ | ||
| --repo "$GITHUB_REPOSITORY" \ | ||
| --head "$RELEASE_BRANCH" \ | ||
| --base "$BASE_BRANCH" \ | ||
| --state open \ | ||
| --json url \ | ||
| --jq '.[0].url' || true)" | ||
| if [ -n "$EXISTING_PR_URL" ]; then |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Treat null from gh pr list as “no PR”.
--jq '.[0].url' returns null when nothing matches. [ -n "$EXISTING_PR_URL" ] then succeeds, so the script can report success and comment PR: null instead of taking the failure path on Lines 138-139.
Minimal fix
- --jq '.[0].url' || true)"
+ --jq '.[0].url // empty' || true)"📝 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.
| EXISTING_PR_URL="$(gh pr list \ | |
| --repo "$GITHUB_REPOSITORY" \ | |
| --head "$RELEASE_BRANCH" \ | |
| --base "$BASE_BRANCH" \ | |
| --state open \ | |
| --json url \ | |
| --jq '.[0].url' || true)" | |
| if [ -n "$EXISTING_PR_URL" ]; then | |
| EXISTING_PR_URL="$(gh pr list \ | |
| --repo "$GITHUB_REPOSITORY" \ | |
| --head "$RELEASE_BRANCH" \ | |
| --base "$BASE_BRANCH" \ | |
| --state open \ | |
| --json url \ | |
| --jq '.[0].url // empty' || true)" | |
| if [ -n "$EXISTING_PR_URL" ]; then |
🤖 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 `@tools/actions/create-nsolid-release-proposal.sh` around lines 119 - 126,
Treat the `gh pr list` result in `create-nsolid-release-proposal.sh` as empty
when `--jq '.[0].url'` returns `null`, since `EXISTING_PR_URL` is then
incorrectly considered non-empty. Update the `EXISTING_PR_URL` handling around
the `gh pr list` command and the subsequent `if [ -n "$EXISTING_PR_URL" ]` check
so that a literal `null` is normalized to an empty value or explicitly rejected,
ensuring the script follows the failure path instead of reporting `PR: null`.
| const result = spawnSync('tools/actions/create-nsolid-release-proposal.sh', { | ||
| stdio: 'inherit', | ||
| env: { | ||
| ...process.env, | ||
| NODE_LINE: String(line), | ||
| NODE_TAG: nodeTag, | ||
| ISSUE_NUMBER: String(issue.number), | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use the resolved nodeTag as the proposal script’s source of truth.
The watcher creates/reuses the issue and checks existing refs for this exact nodeTag, but the proposal script shown in tools/actions/create-nsolid-release-proposal.sh:26-58 recomputes the latest tag from NODE_LINE instead of honoring NODE_TAG. If the release index changes between those two fetches, the PR/comments can target a different Node release than the tracking issue. Please update the proposal script to accept and validate NODE_TAG, or stop passing it here and move issue creation until after the proposal script resolves the tag.
🤖 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 `@tools/actions/watch-node-releases.mjs` around lines 150 - 157, The proposal
flow is using two different tag sources, so `watch-node-releases.mjs` and
`create-nsolid-release-proposal.sh` can drift between issue tracking and PR
creation. Update the proposal script to treat `NODE_TAG` as the source of truth
by reading and validating the passed environment value inside
`create-nsolid-release-proposal.sh`, and ensure the watcher’s `spawnSync` call
continues to pass the exact resolved `nodeTag`; alternatively, if you prefer
recomputing in the script, remove `NODE_TAG` from the watcher and defer issue
creation until after the script resolves the tag, keeping `nodeTag`,
`NODE_LINE`, and the issue/PR comments aligned.
Add automation to watch Node.js release lines and prepare N|Solid release proposal PRs.
The initial configuration watches v22 and v24. When a new Node.js release is found, the workflow creates or reuses a tracking issue and attempts to create the release proposal. If the merge conflicts, it reports the conflicted files in the issue.
Also adds documentation for maintaining the configured release lines.
Summary by CodeRabbit