Skip to content

fix: resolve 5 open issues #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ImBIOS wants to merge 6 commits into
base: main
Choose a base branch
from
Open

fix: resolve 5 open issues #45

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bf70049
fix: resolve 5 open issues (31, 38, 43, 35, 24)
ImBIOS Nov 23, 2025
e6bb370
Please provide the file changes so I can generate a commit message.
ImBIOS Nov 23, 2025
f874a97
chore: remove job_log.txt
ImBIOS Nov 23, 2025
06c5731
feat: migrate to matrix and implement cleanup (fixes #24, #43)
ImBIOS Nov 23, 2025
c3a4bad
feat: implement automatic dockerfile generation and alpine tag check …
ImBIOS Nov 23, 2025
77977ef
fix: add directory check in cleanup to prevent deleting files
ImBIOS Nov 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ jobs:
- uses: oven-sh/setup-bun@635640504f6d7197d3bb29876a652f671028dc97
- run: bun install

- name: Download versions.json
run: gh release download versions -p versions.json || echo "{}" > versions.json
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

The fallback echo "{}" > versions.json when the release doesn't exist creates an empty JSON object, which will cause multiple downstream issues:

  1. check-bun-node.ts will fail when accessing versionsJson.nodejs[release.major] (see related comment)
  2. build_updated.sh will fail when trying to extract codename and latest_node_major from the JSON
  3. All versions will be considered "new" on first run

Consider creating a proper initial structure with empty nodejs and bun objects:

gh release download versions -p versions.json || echo '{"bun":{},"nodejs":{}}' > versions.json
Suggested change
run: gh release download versions -p versions.json || echo "{}" > versions.json
run: gh release download versions -p versions.json || echo '{"bun":{},"nodejs":{}}' > versions.json

Copilot uses AI. Check for mistakes.
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Check for new releases of nodejs and bun
if: ${{ inputs.nodejs-version == '' && inputs.bun-versions == '' && inputs.distros == '' }}
run: |
Expand Down Expand Up @@ -77,18 +82,12 @@ jobs:
BUN_VERSIONS_TO_BUILD: ${{ env.BUN_VERSIONS_TO_BUILD || inputs.bun-versions }}
DISTROS: ${{ env.DISTROS || inputs.distros }}

- name: Commit changes
run: ./commit_changes.sh
- name: Upload versions.json
run: |
gh release create versions versions.json --title "Versions State" --notes "Stores the current versions.json state" || \
gh release upload versions versions.json --clobber
Comment on lines +154 to +156
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

The gh release create command will fail if a release named "versions" already exists (which it will after the first run). The fallback with || will then try to upload, but the error from the first command might cause confusion in logs.

Additionally, using a release tag named "versions" is unconventional and might conflict with actual version tags in the future. Consider using a more specific tag like "state" or "metadata".

Improve the logic:

# Check if release exists first
if gh release view versions >/dev/null 2>&1; then
  gh release upload versions versions.json --clobber
else
  gh release create versions versions.json --title "Versions State" --notes "Stores the current versions.json state"
fi

Copilot uses AI. Check for mistakes.
env:
BUN_VERSIONS_TO_BUILD: ${{ env.BUN_VERSIONS_TO_BUILD }}
NODE_VERSIONS_TO_BUILD: ${{ env.NODE_VERSIONS_TO_BUILD }}
DISTROS: ${{ env.DISTROS }}
- name: Pull changes
run: git pull -r
- name: Push changes
uses: ad-m/github-push-action@77c5b412c50b723d2a4fbc6d71fb5723bcd439aa
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Comment on lines 153 to 158
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

When both NODE_VERSIONS_TO_BUILD and BUN_VERSIONS_TO_BUILD are empty (no updates detected), the build loop doesn't execute and versions.json is never modified. The "Upload versions.json" step will still run and attempt to create/update the Release with the unmodified (potentially empty) versions.json file from the download step.

This could lead to data loss if the Release doesn't exist yet (file would be {}). Consider adding a condition to skip the upload when no builds occurred, or ensure the file has valid structure before uploading.

Copilot uses AI. Check for mistakes.
rerun-failed-jobs:
runs-on: ubuntu-latest
needs: [build-job]
Expand Down
32 changes: 21 additions & 11 deletions build_updated.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,26 +43,24 @@ IFS=',' read -ra DISTROS <<<"$DISTROS"

# If NODE_VERSIONS_TO_BUILD is empty, but BUN_VERSIONS_TO_BUILD is not,
# build all versions from versions.json
# If NODE_VERSIONS_TO_BUILD is empty, but BUN_VERSIONS_TO_BUILD is not,
# we used to build all versions from versions.json.
# But for automatic updates, we want to build NOTHING if no Node versions are updated.
if [ -z "$NODE_VERSIONS_TO_BUILD" ]; then
IFS=',' read -ra NODE_MAJOR_VERSIONS <<<"$NODE_MAJOR_VERSIONS_TO_CHECK"
log "No Node.js versions to build."
NODE_VERSIONS=()
for node_major_version in "${NODE_MAJOR_VERSIONS[@]}"; do
node_version=$(cat versions.json | jq -r ".nodejs.\"${node_major_version}\".version")
if [ "$node_version" != "null" ]; then
NODE_VERSIONS+=("${node_version//v/}")
fi
done
fi

log "Building Node versions: ${NODE_VERSIONS[*]}"

# If BUN_VERSIONS_TO_BUILD is empty, but NODE_VERSIONS_TO_BUILD is not,
# build all versions from versions.json
# If BUN_VERSIONS_TO_BUILD is empty, but NODE_VERSIONS_TO_BUILD is not,
# we used to build all versions from versions.json.
# But for automatic updates, we want to build NOTHING if no Bun versions are updated.
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

Duplicate comment on lines 44-48 and 56-60. Both comments say "If NODE_VERSIONS_TO_BUILD is empty, but BUN_VERSIONS_TO_BUILD is not, we used to build all versions from versions.json." The first occurrence (lines 44-45) should be kept, and the duplicate (lines 46-48) should be removed.

Similarly, duplicate comments on lines 56-60 where "If BUN_VERSIONS_TO_BUILD is empty, but NODE_VERSIONS_TO_BUILD is not" is repeated.

Copilot uses AI. Check for mistakes.
if [ -z "$BUN_VERSIONS_TO_BUILD" ]; then
log "No Bun versions to build."
BUN_VERSIONS=()
for bun_version in $(cat versions.json | jq -r '.bun | keys[]'); do
BUN_VERSIONS+=("${bun_version//v/}")
done
fi
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

When NODE_VERSIONS and BUN_VERSIONS are both empty arrays, the nested loops (lines 139-172) won't execute, and versions.json will never be updated. This means the workflow will succeed but won't upload an updated versions.json to the GitHub Release.

If the intent is to skip the build when there are no updates, this is correct. However, it could leave the system in an inconsistent state on first run when the Release doesn't exist yet. Consider adding an initialization step or documenting this behavior.

Copilot uses AI. Check for mistakes.

log "Building Bun versions: ${BUN_VERSIONS[*]}"
Expand Down Expand Up @@ -116,7 +114,19 @@ generate_tags() {
echo "$REGISTRY/bun-node:latest-${codename}-${distro}"
fi

if [[ $is_canary == false && "$node_major" == "20" && $distro == "debian" ]]; then
# Only tag "latest" if this is the latest Node.js version and Debian distro
# We need to check if the current node_major is the latest one in versions.json
# This is a bit tricky in bash without parsing JSON again, but we can assume the loop order or check against a known latest.
# Better approach: The caller knows if it's the latest.
# For now, let's restrict it to the highest known major version we support (e.g. 25) or check if it's the last one in the list?
# Actually, the issue says "latest tag should be the last tag built".
# If we build in order, the last one overwrites 'latest'.
# BUT, if we build multiple versions, we might overwrite 'latest' with an older version if the loop isn't sorted or if we build an old version update.
# A safer way is to explicitly check if this node version is the "latest" defined in versions.json.
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

[nitpick] Verbose inline comments explaining the logic for determining the latest tag (lines 117-125) could be condensed for better code readability. The key insight—that we need to check against the highest major version in versions.json—could be captured in 1-2 lines rather than 9 lines of commentary.

Suggested change
# Only tag "latest" if this is the latest Node.js version and Debian distro
# We need to check if the current node_major is the latest one in versions.json
# This is a bit tricky in bash without parsing JSON again, but we can assume the loop order or check against a known latest.
# Better approach: The caller knows if it's the latest.
# For now, let's restrict it to the highest known major version we support (e.g. 25) or check if it's the last one in the list?
# Actually, the issue says "latest tag should be the last tag built".
# If we build in order, the last one overwrites 'latest'.
# BUT, if we build multiple versions, we might overwrite 'latest' with an older version if the loop isn't sorted or if we build an old version update.
# A safer way is to explicitly check if this node version is the "latest" defined in versions.json.
# Tag "latest" only if this is the highest Node.js major version in versions.json and the distro is Debian.

Copilot uses AI. Check for mistakes.

local latest_node_major=$(echo "${json_data}" | jq -r '.nodejs | keys | map(tonumber) | max')
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

The latest tag logic depends on versions.json existing and having the correct structure to determine latest_node_major. However, on first run or when the GitHub Release doesn't exist, the downloaded file will be {}, causing jq to return null for the max calculation.

This will cause the comparison "$node_major" == "$latest_node_major" to potentially fail or behave unexpectedly. Consider handling the case when versions.json is empty or malformed:

local latest_node_major=$(echo "${json_data}" | jq -r '.nodejs | keys | map(tonumber) | max // 25')

This provides a fallback value of 25 if the calculation fails.

Suggested change
local latest_node_major=$(echo "${json_data}" | jq -r '.nodejs | keys | map(tonumber) | max')
local latest_node_major=$(echo "${json_data}" | jq -r '.nodejs | keys | map(tonumber) | max // 25')

Copilot uses AI. Check for mistakes.

if [[ $is_canary == false && "$node_major" == "$latest_node_major" && $distro == "debian" ]]; then
echo "$REGISTRY/bun-node:latest"
fi

Expand Down
24 changes: 18 additions & 6 deletions check-bun-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,24 @@ const main = async () => {
}

if (process.argv.includes("--node")) {
console.log(
(await generateReleaseData())
.filter((release) => [20, 22, 24, 25].includes(release?.major || 0))
.map((release) => release?.versionWithPrefix.replace("v", ""))
.join(",")
);
const releases = await generateReleaseData();
const versionsJson = await Bun.file("versions.json").json();

const newVersions = releases
.filter((release) => [20, 22, 24, 25].includes(release?.major || 0))
.filter((release) => {
if (!release) return false;
const currentVersion = versionsJson.nodejs[release.major]?.version;
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

The code attempts to read versions.json from the file system, but if the GitHub Release download fails (e.g., first run, or release doesn't exist), an empty JSON object {} is created. This will cause a runtime error when trying to access versionsJson.nodejs[release.major] since nodejs property won't exist.

Consider adding a default structure or checking if the property exists:

const versionsJson = await Bun.file("versions.json").json();
const nodejs = versionsJson.nodejs || {};
const currentVersion = nodejs[release.major]?.version;
Suggested change
const currentVersion = versionsJson.nodejs[release.major]?.version;
const nodejs = versionsJson.nodejs || {};
const currentVersion = nodejs[release.major]?.version;

Copilot uses AI. Check for mistakes.
Copy link

@cursor cursor bot Nov 23, 2025

Choose a reason for hiding this comment

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

Bug: Missing optional chaining crashes on empty versions.json

When versions.json is downloaded and the GitHub release doesn't exist, the fallback creates an empty object {}. Accessing versionsJson.nodejs[release.major]?.version throws a TypeError because versionsJson.nodejs is undefined, and the optional chaining only applies to the .version access, not to [release.major]. This crashes on first run or when the release doesn't exist. The fix requires optional chaining on versionsJson.nodejs itself, like versionsJson.nodejs?.[release.major]?.version.

Fix in Cursor Fix in Web

// If current version is not set, or is different (assuming we only get newer versions from nodevu), it's an update.
// Actually, nodevu returns the LATEST version for that major.
// So if versions.json has v20.10.0 and nodevu says v20.11.0, we want to build.
// If versions.json has v20.11.0 and nodevu says v20.11.0, we don't want to build.
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

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

[nitpick] The inline comments on lines 214-217 are overly verbose and redundant. The core logic—comparing current version with the latest from nodevu—is clear from the code itself. Consider condensing to:

// Only build if the version has changed
return currentVersion !== release.versionWithPrefix;
Suggested change
// If current version is not set, or is different (assuming we only get newer versions from nodevu), it's an update.
// Actually, nodevu returns the LATEST version for that major.
// So if versions.json has v20.10.0 and nodevu says v20.11.0, we want to build.
// If versions.json has v20.11.0 and nodevu says v20.11.0, we don't want to build.
// Only build if the version has changed

Copilot uses AI. Check for mistakes.
return currentVersion !== release.versionWithPrefix;
})
.map((release) => release?.versionWithPrefix.replace("v", ""))
.join(",");

console.log(newVersions);
}
};

Expand Down
26 changes: 0 additions & 26 deletions commit_changes.sh
View file
Open in desktop

This file was deleted.

45 changes: 45 additions & 0 deletions docs/research_matrix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Research: Workflow Matrix vs Single Builder

## Context
The current build system uses a "Single Builder" pattern where a single job runs a script (`build_updated.sh`) that iterates over all versions and builds them sequentially. Issue #24 suggests investigating if a GitHub Actions Matrix strategy would be better.

## Comparison

| Feature | Single Builder (Current) | Workflow Matrix |
| :--- | :--- | :--- |
| **Parallelism** | Low (Sequential builds in loop) | High (Concurrent jobs) |
| **Failures** | One failure can stop the whole loop (unless handled carefully) | Isolated failures per job |
| **Resource Usage** | Single runner, longer duration | Multiple runners, shorter duration (burst) |
| **Complexity** | High (Bash script logic) | Medium (YAML configuration) |
| **Cost** | Lower (1 runner time) | Higher (Multiple runners init time overhead) |
| **Logs** | Mixed in one huge log | Separated per job |
| **Dynamic** | Hard (Need to generate JSON for matrix) | Native support for dynamic matrix |

## Analysis for bun-node

### Pros of Matrix
1. **Speed:** Building multiple Docker images is time-consuming. Parallelizing this would significantly reduce total build time.
2. **Isolation:** If one image build fails (e.g., specific Node version issue), it won't block others.
3. **Clarity:** GitHub UI shows exactly which build failed.

### Cons of Matrix
1. **Complexity of Dynamic Matrix:** Since we only want to build *updated* versions, we need a "setup" job that calculates the matrix (JSON) and passes it to the build job.
2. **Concurrency Limits:** GitHub Free tier has concurrency limits (20 jobs). If we have many versions, we might queue.
3. **Shared State:** The current script updates `versions.json` after each build. In a matrix, we'd need to aggregate these updates or handle them differently (e.g., one final commit job).

## Recommendation
**Migrate to Matrix.**
The benefits of parallelism and isolation outweigh the setup complexity.
The `versions.json` update issue (Issue 35) actually helps here: if we move `versions.json` to Releases, we don't need to commit to git from every job. We can have a final "Release" job that aggregates the successful builds and updates the `versions.json` in the Release assets.

## Implementation Strategy
1. **Job 1: Check Versions**
- Runs `check-bun-node.ts`.
- Outputs a JSON matrix of versions to build.
2. **Job 2: Build (Matrix)**
- `needs: [check-versions]`
- `strategy: matrix: ${{ fromJson(needs.check-versions.outputs.matrix) }}`
- Builds and pushes single image.
3. **Job 3: Update Release**
- `needs: [build]`
- Updates `versions.json` and uploads to GitHub Release.
24 changes: 0 additions & 24 deletions versions.json
View file
Open in desktop

This file was deleted.

Loading