-
Notifications
You must be signed in to change notification settings - Fork 106
Prepare Hunk for npm packaging and install verification #8
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.
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #!/usr/bin/env bash | ||
| set -Eeuo pipefail | ||
|
|
||
| repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" | ||
| outdir="${repo_root}/dist/npm" | ||
|
|
||
| rm -rf "${outdir}" | ||
| mkdir -p "${outdir}" | ||
|
|
||
| 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" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| #!/usr/bin/env bun | ||
|
|
||
| import { existsSync } from "node:fs"; | ||
|
|
||
| interface PackedFile { | ||
| path: string; | ||
| size: number; | ||
| } | ||
|
|
||
| interface PackResult { | ||
| name: string; | ||
| version: string; | ||
| filename: string; | ||
| entryCount: number; | ||
| files: PackedFile[]; | ||
| } | ||
|
|
||
| const proc = Bun.spawnSync(["npm", "pack", "--dry-run", "--json"], { | ||
| cwd: process.cwd(), | ||
| stdin: "ignore", | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| env: process.env, | ||
| }); | ||
|
|
||
| const stdout = Buffer.from(proc.stdout).toString("utf8").trim(); | ||
| const stderr = Buffer.from(proc.stderr).toString("utf8").trim(); | ||
|
|
||
| if (proc.exitCode !== 0) { | ||
| throw new Error(stderr || stdout || "npm pack --dry-run failed"); | ||
| } | ||
|
|
||
| const jsonMatch = stdout.match(/(\[\s*\{[\s\S]*\}\s*\])\s*$/); | ||
| const jsonText = jsonMatch?.[1]; | ||
|
|
||
| if (!jsonText) { | ||
| throw new Error(`Could not find npm pack JSON output. Full stdout:\n${stdout}`); | ||
| } | ||
|
|
||
| const parsed = JSON.parse(jsonText) as PackResult[]; | ||
| const pack = parsed[0]; | ||
|
|
||
| if (!pack) { | ||
| throw new Error("npm pack --dry-run returned no pack result."); | ||
| } | ||
|
|
||
| const publishedPaths = new Set(pack.files.map((file) => file.path)); | ||
| const requiredPaths = ["dist/npm/main.js", "README.md", "LICENSE", "package.json"]; | ||
| const optionalPaths = ["CONTRIBUTING.md", "SECURITY.md"]; | ||
|
|
||
| for (const path of requiredPaths) { | ||
| if (!publishedPaths.has(path)) { | ||
| throw new Error(`Expected npm package to include ${path}.`); | ||
| } | ||
| } | ||
|
|
||
| for (const path of optionalPaths) { | ||
| if (existsSync(path) && !publishedPaths.has(path)) { | ||
| throw new Error(`Expected npm package to include ${path} when it exists in the repo.`); | ||
| } | ||
| } | ||
|
|
||
| const forbiddenPrefixes = [".github/", "src/", "test/", "scripts/", "tmp/"]; | ||
| const forbiddenPaths = ["AGENTS.md", "autoresearch.checks.sh", "autoresearch.sh", "bun.lock"]; | ||
|
|
||
| for (const file of pack.files) { | ||
| if (forbiddenPrefixes.some((prefix) => file.path.startsWith(prefix)) || forbiddenPaths.includes(file.path)) { | ||
| throw new Error(`Unexpected file in npm package: ${file.path}`); | ||
| } | ||
| } | ||
|
|
||
| if (pack.name !== "hunkdiff") { | ||
| throw new Error(`Expected npm package name to be hunkdiff, got ${pack.name}.`); | ||
| } | ||
|
|
||
| console.log(`Verified npm pack output for ${pack.name}@${pack.version} (${pack.entryCount} files).`); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The
build-npm.shscript doesn't add a#!/usr/bin/env bunshebang to the bundled output, which will cause execution to fail in Node.js-only environments.Severity: CRITICAL
Suggested Fix
Modify
scripts/build-npm.shto explicitly prepend#!/usr/bin/env bunto the bundled output file after thebun buildcommand. 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
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
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 bunis already preserving the#!/usr/bin/env bunshebang fromsrc/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.mdandSECURITY.mdeven 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 npmbinpath so npm no longer needs to correct it during publish.Responded by pi-coding-agent using openai/gpt-5.