Skip to content

Prepare Hunk for npm packaging and install verification#8

Merged
benvinegar merged 2 commits into
masterfrom
oss/npm-packaging
Mar 20, 2026
Merged

Prepare Hunk for npm packaging and install verification#8
benvinegar merged 2 commits into
masterfrom
oss/npm-packaging

Conversation

@benvinegar
Copy link
Copy Markdown
Member

Summary

  • rename the published package to hunkdiff and point the hunk bin at a built Bun runtime bundle
  • add curated npm publish metadata plus build:npm, prepack, and pack-surface verification
  • add CI coverage for npm packing and a simulated global npm install -g smoke check

Validation

  • bun run typecheck
  • bun test
  • bun run test:tty-smoke
  • bun run build:npm && bun run check:pack
  • manual temp-prefix npm install -g smoke run

Comment thread scripts/build-npm.sh
Comment on lines +10 to +20
BUN_TMPDIR="${repo_root}/.bun-tmp" \
BUN_INSTALL="${repo_root}/.bun-install" \
bun build "${repo_root}/src/main.tsx" \
--target bun \
--format esm \
--outdir "${outdir}" \
--entry-naming main.js

chmod 0755 "${outdir}/main.js"

printf 'Built %s\n' "${outdir}/main.js"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The build-npm.sh script doesn't add a #!/usr/bin/env bun shebang to the bundled output, which will cause execution to fail in Node.js-only environments.
Severity: CRITICAL

Suggested Fix

Modify scripts/build-npm.sh to explicitly prepend #!/usr/bin/env bun to the bundled output file after the bun build command. For example: { printf '#!/usr/bin/env bun\n'; cat "${outdir}/main.js"; } > "${outdir}/main.js.tmp" && mv "${outdir}/main.js.tmp" "${outdir}/main.js".

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: scripts/build-npm.sh#L10-L20

Potential issue: The `build-npm.sh` script bundles `src/main.tsx`, which contains
Bun-specific APIs like `Bun.stdin.stream()`. However, the script fails to prepend the
necessary `#!/usr/bin/env bun` shebang to the final executable. When a user on a system
with only Node.js installed runs the command after `npm install -g`, the operating
system will not know to use the Bun runtime. This will lead to execution errors, such as
`ReferenceError: Bun is not defined`, because the script will be run by the wrong
interpreter or fail to run at all. This breaks the npm distribution for users without a
global Bun installation.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I checked this one end-to-end and did not apply the suggested shebang change. bun build --target bun is already preserving the #!/usr/bin/env bun shebang from src/main.tsx, so the bundled entrypoint is executable under Bun as-is.

The failing PR check turned out to be a different issue: the pack verifier was requiring CONTRIBUTING.md and SECURITY.md even though those docs only exist in the stacked follow-up PR. I fixed the verifier to require those files only when they exist in the repo, and also normalized the npm bin path so npm no longer needs to correct it during publish.

Responded by pi-coding-agent using openai/gpt-5.

@benvinegar benvinegar merged commit 0203393 into master Mar 20, 2026
7 checks passed
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.

1 participant