Skip to content

Conversation

@ryoppippi
Copy link
Member

@ryoppippi ryoppippi commented Dec 21, 2025

Summary

  • Add nix build derivation to generate npm package tarball reproducibly
  • Add pre-commit hook to auto-update pnpm deps hash when lockfile changes
  • Update CI workflows to use nix build for publishing

What Changed

  • flake.nix: Add packages.default derivation using pnpm_10.fetchDeps for reproducible builds
  • scripts/update-pnpm-hash.sh: Script to calculate and update pnpm deps hash automatically
  • lefthook.yaml: Pre-commit hook to run hash update on pnpm-lock.yaml changes
  • release.yaml / dry-publish.yaml: Use nix build instead of pnpm pack
  • ci.yaml: Split build-and-test into separate parallel jobs

Why

Using Nix for package building ensures:

  • Reproducible builds across CI and local environments
  • Same tarball hash for identical inputs
  • Automatic dependency hash management via pre-commit hook

Summary by cubic

Adds a reproducible Nix build to generate the npm tarball and updates CI to publish from that output. Also adds an auto-update hook for the pnpm deps hash to keep Nix builds working.

  • New Features

    • Nix derivation (packages.default) builds a deterministic tarball using pnpm_10.fetchDeps.
    • Pre-commit hook updates the pnpm deps hash when pnpm-lock.yaml changes.
    • Ignore Nix build result symlink in .gitignore.
    • Remove the preinstall only-allow pnpm script.
  • CI

    • Release workflow uses nix build and publishes from result/*.tgz.
    • Split build and test into separate jobs for parallel runs and clearer failures.
    • Use --print-build-logs for better build visibility.

Written for commit 5878e77. Summary will update automatically on new commits.

Add packages.default derivation to flake.nix that builds the npm
package tarball using pnpm_10.fetchDeps for reproducible builds.

- Use stdenv.mkDerivation with pnpm_10.configHook
- Fetch dependencies with fixed hash for reproducibility
- Run pnpm build and pnpm pack in buildPhase/installPhase
- Output tarball to $out directory

This enables `nix build` to generate a reproducible tarball that can
be used for npm publishing, ensuring consistent builds across CI and
local environments.
Add script and lefthook configuration to automatically update the
pnpm deps hash in flake.nix when pnpm-lock.yaml changes.

- Add scripts/update-pnpm-hash.sh to calculate and update hash
- Configure lefthook to run script on pnpm-lock.yaml changes
- Auto-stage updated flake.nix after hash update

This ensures the Nix build remains functional after dependency
updates without manual hash maintenance.
Update release and dry-publish workflows to use `nix build` instead
of `pnpm pack` for generating package tarballs.

- release.yaml: Build with nix, publish from result/*.tgz
- dry-publish.yaml: Build with nix, pass tarball to pkg-pr-new

This ensures CI uses the same reproducible build process as local
development, improving build consistency and reliability.
Separate the combined build-and-test job into distinct jobs:
- build: Uses nix build for reproducible package generation
- test: Runs pnpm test in nix develop shell

This allows build and test to run in parallel and makes it clearer
which step failed when issues occur.
Remove the preinstall script that enforces pnpm usage. This check
is unnecessary as the project uses nix develop which provides
the correct pnpm version, and it can cause issues in CI environments.
Copilot AI review requested due to automatic review settings December 21, 2025 12:05
@ryoppippi ryoppippi requested a review from a team as a code owner December 21, 2025 12:05
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 8 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="scripts/update-pnpm-hash.sh">

<violation number="1" location="scripts/update-pnpm-hash.sh:37">
P1: Script silently masks errors when `nix build` fails for reasons other than hash mismatch. If nix is not installed, flake has syntax errors, or any other failure occurs, the script will misleadingly report &quot;Hash is up to date&quot; and exit successfully. Consider checking the exit status and error type before assuming the hash is up to date.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR


echo "Updated $FLAKE_FILE with new hash"
else
echo "Hash is up to date"
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 21, 2025

Choose a reason for hiding this comment

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

P1: Script silently masks errors when nix build fails for reasons other than hash mismatch. If nix is not installed, flake has syntax errors, or any other failure occurs, the script will misleadingly report "Hash is up to date" and exit successfully. Consider checking the exit status and error type before assuming the hash is up to date.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/update-pnpm-hash.sh, line 37:

<comment>Script silently masks errors when `nix build` fails for reasons other than hash mismatch. If nix is not installed, flake has syntax errors, or any other failure occurs, the script will misleadingly report &quot;Hash is up to date&quot; and exit successfully. Consider checking the exit status and error type before assuming the hash is up to date.</comment>

<file context>
@@ -0,0 +1,38 @@
+
+  echo &quot;Updated $FLAKE_FILE with new hash&quot;
+else
+  echo &quot;Hash is up to date&quot;
+fi
</file context>
Fix with Cubic

Different platforms download different platform-specific npm packages
(e.g., darwin-arm64 vs linux-x64 binaries), resulting in different
hashes for pnpm deps.

- Add pnpmDepsHash map with per-system hashes
- Update hash script to detect current system and update only that hash
- Add hashes for x86_64-linux (CI) and aarch64-darwin (local dev)
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Nix-based reproducible tarball builds to replace the traditional pnpm pack workflow, ensuring consistent builds across CI and local environments through content-addressable dependency management.

Key Changes:

  • Introduces a Nix package derivation in flake.nix that uses pnpm_10.fetchDeps for reproducible dependency fetching
  • Adds automated pnpm dependency hash management via pre-commit hook and update script
  • Migrates CI/CD workflows from pnpm pack to nix build for all package creation steps

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
flake.nix Adds packages.default derivation with nodejs_24, pnpm_10, and build/install phases for creating npm tarball
scripts/update-pnpm-hash.sh Bash script to automatically extract and update pnpm deps hash in flake.nix when lockfile changes
lefthook.yaml Adds pre-commit hook to trigger hash update script when pnpm-lock.yaml is modified
.github/workflows/release.yaml Replaces pnpm pack with nix build and updates tarball path to result/*.tgz
.github/workflows/dry-publish.yaml Adds explicit build step using nix build and updates publish command to use result tarball
.github/workflows/ci.yaml Splits combined build-and-test job into separate parallel build and test jobs
package.json Removes preinstall script that enforced pnpm-only usage
.gitignore Adds result directory (nix build output symlink) to gitignore

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -0,0 +1,42 @@
#!/usr/bin/env bash
set -euo pipefail

Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The script lacks a proper shebang validation mechanism and doesn't verify that required commands (nix, sed, grep) are available before use. While the script uses set -euo pipefail for strict error handling, it would be more maintainable to check for command availability upfront and provide clear error messages if dependencies are missing.

Suggested change
# Ensure the script is running under Bash
if [[ -z "${BASH_VERSION:-}" ]]; then
echo "Error: This script must be run with bash." >&2
exit 1
fi
# Verify required commands are available
for cmd in nix sed grep; do
if ! command -v "$cmd" >/dev/null 2>&1; then
echo "Error: Required command '$cmd' not found in PATH." >&2
exit 1
fi
done

Copilot uses AI. Check for mistakes.
shell: bash
run: |
PACKAGE_TGZ=$(ls *.tgz | head -n 1)
PACKAGE_TGZ=$(ls result/*.tgz | head -n 1)
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The command ls result/*.tgz | head -n 1 assumes that the nix build command creates exactly one .tgz file in the result directory. If multiple .tgz files exist or if the build produces a different file structure, this will either select an unexpected file or fail. Consider being more explicit about the expected filename pattern or adding error handling to verify exactly one tarball exists before attempting to publish.

Suggested change
PACKAGE_TGZ=$(ls result/*.tgz | head -n 1)
set -euo pipefail
shopt -s nullglob
files=(result/*.tgz)
if [ "${#files[@]}" -eq 0 ]; then
echo "Error: No .tgz package files found in result/." >&2
exit 1
elif [ "${#files[@]}" -gt 1 ]; then
echo "Error: Multiple .tgz package files found in result/: ${files[*]}" >&2
exit 1
fi
PACKAGE_TGZ="${files[0]}"

Copilot uses AI. Check for mistakes.

- name: 🚀 Dry Run Publish Package
run: nix develop --command pnpm dlx pkg-pr-new publish --pnpm
run: nix develop --command pnpm dlx pkg-pr-new publish result/*.tgz
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The glob pattern result/*.tgz is passed to pkg-pr-new publish which may not handle glob patterns correctly. Shell glob expansion happens before the command is executed, but if no files match or multiple files match, the behavior might be unexpected. Consider explicitly using shell expansion with quotes or verifying the exact tarball path before passing it to the command.

Suggested change
run: nix develop --command pnpm dlx pkg-pr-new publish result/*.tgz
run: |
shopt -s nullglob
tgzs=(result/*.tgz)
if [ "${#tgzs[@]}" -ne 1 ]; then
echo "Expected exactly one .tgz file in result/, found ${#tgzs[@]}." >&2
exit 1
fi
nix develop --command pnpm dlx pkg-pr-new publish "${tgzs[0]}"

Copilot uses AI. Check for mistakes.
packageJson = builtins.fromJSON (builtins.readFile ./package.json);
pnpmDepsHash = {
x86_64-linux = "sha256-PrCGXf5r03gfsoGJAzew592Al1G5dx6xa/qFxazuqUo=";
aarch64-linux = "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The package name "stackone-ai" in the derivation doesn't match the actual npm package name "@stackone/ai" from package.json. While this doesn't break the build since pnpm.fetchDeps doesn't publish to npm, it creates inconsistency and could cause confusion. Consider using the actual package name without the @ symbol (e.g., "stackone-ai-node" to match the GitHub repo name, or extract it from package.json).

Copilot uses AI. Check for mistakes.
echo "Error: $FLAKE_FILE not found"
exit 1
fi

Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The script assumes the presence of the nix command and doesn't check for its availability. If nix is not installed or not in PATH, the script will fail with an unclear error message. Consider adding a check to verify nix is available before attempting to run it, or handle the error more gracefully with a helpful error message.

Suggested change
# Check if nix is available
if ! command -v nix >/dev/null 2>&1; then
echo "Error: 'nix' command not found. Please install Nix and ensure it is in your PATH." >&2
exit 1
fi

Copilot uses AI. Check for mistakes.
stage_fixed: true
- name: update-pnpm-hash
glob: 'pnpm-lock.yaml'
run: ./scripts/update-pnpm-hash.sh
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The script path ./scripts/update-pnpm-hash.sh uses a relative path and won't work correctly if the git hook is triggered from a subdirectory. Lefthook runs commands from the repository root, but the script should be executable. Consider adding execute permissions to the script file or using bash ./scripts/update-pnpm-hash.sh explicitly to ensure it runs correctly.

Suggested change
run: ./scripts/update-pnpm-hash.sh
run: bash ./scripts/update-pnpm-hash.sh

Copilot uses AI. Check for mistakes.
"lint:oxlint": "oxlint --max-warnings=0 --type-aware --type-check",
"lint:knip": "knip",
"preinstall": "npx only-allow pnpm",
"prepack": "npm pkg delete scripts.preinstall && pnpm run build",
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The prepack script attempts to delete scripts.preinstall with npm pkg delete scripts.preinstall, but the preinstall script is being removed in this PR. This will cause the prepack script to fail or have unexpected behavior. Since the preinstall script no longer exists, this deletion step should be removed from the prepack script.

Suggested change
"prepack": "npm pkg delete scripts.preinstall && pnpm run build",
"prepack": "pnpm run build",

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +29
# Extract the new hash
NEW_HASH=$(echo "$OUTPUT" | grep "got:" | sed 's/.*got:[[:space:]]*//' | tr -d '[:space:]')

if [[ -z "$NEW_HASH" ]]; then
echo "Error: Could not extract new hash"
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The hash extraction pattern grep "got:" | sed 's/.*got:[[:space:]]*//' assumes a specific nix error message format. The pattern is fragile and may break if nix changes its error message format. Additionally, the sed regex pattern should be more explicit to match the expected format. Consider using a more robust extraction method or adding validation that the extracted hash matches the expected sha256 format (e.g., sha256-[A-Za-z0-9+/=]+).

Suggested change
# Extract the new hash
NEW_HASH=$(echo "$OUTPUT" | grep "got:" | sed 's/.*got:[[:space:]]*//' | tr -d '[:space:]')
if [[ -z "$NEW_HASH" ]]; then
echo "Error: Could not extract new hash"
# Extract the new hash by matching the expected sha256 format in the output
NEW_HASH=$(printf '%s\n' "$OUTPUT" | grep -oE 'sha256-[A-Za-z0-9+/=]+' | head -n1 || true)
if [[ -z "$NEW_HASH" || ! "$NEW_HASH" =~ ^sha256-[A-Za-z0-9+/=]+$ ]]; then
echo "Error: Could not extract valid sha256 hash from nix output"

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="flake.nix">

<violation number="1" location="flake.nix:26">
P1: Placeholder hash for `aarch64-linux` will cause build failures. The hash `sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=` is not a valid pnpm deps hash - it needs to be computed by running `nix build` on an aarch64-linux system and using the resulting hash from the error message.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

packageJson = builtins.fromJSON (builtins.readFile ./package.json);
pnpmDepsHash = {
x86_64-linux = "sha256-PrCGXf5r03gfsoGJAzew592Al1G5dx6xa/qFxazuqUo=";
aarch64-linux = "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 21, 2025

Choose a reason for hiding this comment

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

P1: Placeholder hash for aarch64-linux will cause build failures. The hash sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= is not a valid pnpm deps hash - it needs to be computed by running nix build on an aarch64-linux system and using the resulting hash from the error message.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At flake.nix, line 26:

<comment>Placeholder hash for `aarch64-linux` will cause build failures. The hash `sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=` is not a valid pnpm deps hash - it needs to be computed by running `nix build` on an aarch64-linux system and using the resulting hash from the error message.</comment>

<file context>
@@ -16,11 +16,16 @@
               packageJson = builtins.fromJSON (builtins.readFile ./package.json);
+              pnpmDepsHash = {
+                x86_64-linux = &quot;sha256-PrCGXf5r03gfsoGJAzew592Al1G5dx6xa/qFxazuqUo=&quot;;
+                aarch64-linux = &quot;sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=&quot;;
+                aarch64-darwin = &quot;sha256-GDY7RZUl6A0d3l8Rz6X1sHQfwHgM2GKpcJ65yAKOmrg=&quot;;
+              };
</file context>
Fix with Cubic

- Add --print-build-logs to nix build for better CI visibility
- Revert dry-publish to use pnpm directly as pkg-pr-new doesn't
  support pre-built tarballs
@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 21, 2025

Open in StackBlitz

npm i https://pkg.pr.new/StackOneHQ/stackone-ai-node/@stackone/ai@258

commit: 5878e77

@ryoppippi ryoppippi closed this Dec 21, 2025
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.

2 participants