feat: add npm distribution for one-command install#563
Merged
greynewell merged 3 commits intomainfrom Mar 3, 2026
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review WalkthroughThis PR enables npm-based distribution of the Uncompact project by introducing an npm package manifest, automated binary download-and-install scripts, CLI launcher, and supporting GitHub Actions workflows for testing and publishing to npm upon release tags. Changes
Sequence DiagramsequenceDiagram
actor User
participant npm as npm Registry
participant GitHub as GitHub API
participant Local as Local Filesystem
participant Binary as uncompact Binary
User->>npm: npm install -g uncompact
npm->>Local: extract package.json
npm->>Local: run postinstall script
Local->>Local: invoke npm/install.js
Local->>GitHub: fetch release info
GitHub-->>Local: return release metadata
Local->>GitHub: download binary asset
GitHub-->>Local: stream binary (tar.gz/zip)
Local->>Local: extract to npm/bin
npm-->>User: install complete
User->>Local: uncompact [args]
Local->>Local: npm/run.js resolves binary path
Local->>Binary: spawn with arguments
Binary->>Binary: process data
Binary-->>User: output result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Add npm package wrapper that downloads the appropriate Go binary from GitHub releases on postinstall. Users can now install via: npm install -g uncompact npx uncompact install The install script reads the package.json version and fetches the matching GitHub release, ensuring version consistency. The release workflow now publishes to npm after GoReleaser completes. Co-Authored-By: Grey Newell <greyshipscode@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Made-with: Cursor
greynewell
force-pushed
the
claude/npm-distribution-20260303-1310
branch
from
0571e26 to
23a69a8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
51-54: Make npm publish idempotent for workflow re-runs.If this workflow is re-run for the same tag,
npm publishwill fail because that version is already on npm. The suggested patch adds a quick check before publishing: it usesnpm view uncompact@$VERSION versionto see if that version already exists. If it does, the step exits successfully without publishing (idempotent behavior). If it doesn't, the publish proceeds normally.This is especially useful for recover-from-failure scenarios—you can just re-trigger the workflow without manual edits.
Suggested patch
- name: Publish to npm - run: npm publish --access public + run: | + VERSION="${GITHUB_REF_NAME#v}" + if npm view "uncompact@$VERSION" version >/dev/null 2>&1; then + echo "uncompact@$VERSION already exists on npm; skipping publish." + exit 0 + fi + npm publish --access public env: NODE_AUTH_TOKEN: ${{ secrets.SUPERMODEL_NPM_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 51 - 54, The Publish to npm step currently always runs "npm publish --access public" which fails if the same package version already exists; update the "Publish to npm" step to first check existence using "npm view uncompact@$VERSION version" (or equivalent variable for the package/version) and, if that command returns a version, exit the step successfully (skip publishing), otherwise proceed to run "npm publish --access public"; ensure the step uses the same NODE_AUTH_TOKEN env and that the existence check returns a zero status when found so the step becomes idempotent for re-runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/hooks/hooks.go`:
- Around line 189-190: mergeHooks currently fails to treat "show-hook.sh" as
equivalent for hooks["UserPromptSubmit"], causing duplicate hooks in
partial-install merges; update mergeHooks so the equivalence check for
UserPromptSubmit mirrors the detection check by treating "uncompact show-cache"
and "show-hook.sh" as equivalent (use commandExistsInHooks or the same
equivalence list used in detection when comparing hooks["UserPromptSubmit"]).
In `@internal/template/render.go`:
- Around line 145-162: The appended note can still push output over
opts.MaxTokens; after result += note and resultTokens = countTokens(result) add
a final hard-budget guard: if opts.MaxTokens > 0 and resultTokens >
opts.MaxTokens compute budget = max(1, opts.MaxTokens - noteTokens) and call
truncateToTokenBudget(graph, projectName, budget,
graph.Stats.CircularDependencyCycles, opts.WorkingMemory) to get a truncated
result, then rebuild note via buildNote(resultTokens) and reassign result and
resultTokens using countTokens(result) so the final string never exceeds
opts.MaxTokens. Include these checks around the existing result/ note handling
(referencing result, note, resultTokens, noteTokens, opts.MaxTokens,
truncateToTokenBudget, buildNote, countTokens).
In `@npm/install.js`:
- Around line 43-60: The httpsGet function can hang indefinitely on stalled
connections; modify it to call request.setTimeout(20000) immediately after
creating the request so the request will time out after 20s and trigger the
existing error handler (which will cause request.destroy()/error to propagate);
keep the current reject handling intact so the Promise is rejected on timeout
and redirects/responses still work; update references in the httpsGet function
where the request is created to include this timeout.
- Around line 67-79: The getRelease function currently falls back to the
"latest" release when a specific tagged release (computed as tag from version)
is not found, which breaks version-locking; remove the fallback behavior so that
when httpsGetJson for the tag throws in getRelease (and version is provided and
not "0.0.0") the error is propagated instead of calling the releases/latest
endpoint — update getRelease to only try the tagged URL when version is
specified and, on failure, rethrow the error (do not call the latest URL); keep
the behavior for when version is absent so callers requesting latest still work.
- Line 84: Replace shell-string execSync calls with execFileSync and pass
command arguments as an array to avoid shell quoting issues: for the tar
extraction that currently calls execSync(`tar -xzf "${tarPath}" -C
"${destDir}"`, { stdio: "pipe" }), call execFileSync("tar", ["-xzf", tarPath,
"-C", destDir], { stdio: "pipe" }); do the same for the other execSync uses that
invoke unzip/powershell (the other execSync occurrences in this file) by mapping
their command and flags into an array and using execFileSync with identical
stdio/options so paths like tarPath and destDir are passed directly without
shell parsing.
In `@npm/run.js`:
- Around line 54-56: The child process "close" handler currently calls
process.exit(code ?? 0) which masks signal terminations (code === null) and
returns 0; modify the handler for child.on("close", ...) to detect when code is
null and instead propagate the terminating signal to the current process (e.g.,
re-emit or kill with the same signal) and use a defensive numeric fallback of 1
for unexpected states; update the logic around child.on("close", (code, signal)
=> ...) so signal terminations are forwarded and non-signal failures exit with
code || 1.
In `@README.md`:
- Around line 64-68: Replace the non-mutating example shown as "npx uncompact
install" with a harmless/read-only command to avoid duplicating the actual
install step (which is shown later as "uncompact install"); specifically change
the example invocation string 'npx uncompact install' to a non-mutating command
like 'npx uncompact --help' (or any read-only command) in the README Step 1 so
the text "run without installing" correctly demonstrates a non-install action.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 51-54: The Publish to npm step currently always runs "npm publish
--access public" which fails if the same package version already exists; update
the "Publish to npm" step to first check existence using "npm view
uncompact@$VERSION version" (or equivalent variable for the package/version)
and, if that command returns a version, exit the step successfully (skip
publishing), otherwise proceed to run "npm publish --access public"; ensure the
step uses the same NODE_AUTH_TOKEN env and that the existence check returns a
zero status when found so the step becomes idempotent for re-runs.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/release.yml.gitignoreREADME.mdcmd/run.gointernal/hooks/hooks.gointernal/template/render.gonpm/install.jsnpm/run.jspackage.jsonscripts/uncompact-hook.sh
npm/run.js
Outdated
Comment on lines
+54
to
+56
| child.on("close", (code) => { | ||
| process.exit(code ?? 0); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "run.js" -path "*/npm/*" | head -20Repository: supermodeltools/Uncompact
Length of output: 81
🏁 Script executed:
cat -n npm/run.js | sed -n '40,70p'Repository: supermodeltools/Uncompact
Length of output: 569
🌐 Web query:
Node.js child_process close event code null signal behavior
💡 Result:
In Node.js, the ChildProcess 'close' event is emitted after the child has ended and all its stdio streams have been closed (so it can occur later than 'exit'). It is guaranteed to fire after 'exit', or after 'error' if the process failed to spawn. [1]
The 'close' handler receives (code, signal):
codeis the numeric exit code if the process exited normally; otherwise it isnull. [1]signalis the signal name if the process was terminated by a signal; otherwise it isnull. [1]- One of
codeorsignalwill always be non-null(socode === nullimpliessignalis a string like"SIGTERM"/"SIGKILL"). [1]
Example interpretation
'close'→(0, null): clean success exit.'close'→(null, 'SIGTERM'): terminated by signal, so no numeric exit code. [1]
Source: Node.js child_process docs for 'close' / 'exit'. [1]
References:
[1] Node.js v22.x child_process documentation (ChildProcess events 'close' / 'exit'). (nodejs.org)
Citations:
Propagate signal terminations, don't mask them.
When the child gets terminated by a signal (like SIGTERM), the code parameter is null. Your current code then exits with 0—making it look like everything was fine when it actually wasn't. Here's the concrete problem: if a deployment script kills the child with SIGTERM, your wrapper cheerfully reports success to the orchestrator, even though things went sideways.
The fix is straightforward: check if a signal happened, then propagate that signal to your own process. This lets the parent shell and any monitoring tools know what actually occurred. And change the fallback from 0 to 1 to be defensive about unexpected states.
diff --git a/npm/run.js b/npm/run.js
@@
- child.on("close", (code) => {
- process.exit(code ?? 0);
+ child.on("close", (code, signal) => {
+ if (signal) {
+ process.kill(process.pid, signal);
+ return;
+ }
+ process.exit(code ?? 1);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@npm/run.js` around lines 54 - 56, The child process "close" handler currently
calls process.exit(code ?? 0) which masks signal terminations (code === null)
and returns 0; modify the handler for child.on("close", ...) to detect when code
is null and instead propagate the terminating signal to the current process
(e.g., re-emit or kill with the same signal) and use a defensive numeric
fallback of 1 for unexpected states; update the logic around child.on("close",
(code, signal) => ...) so signal terminations are forwarded and non-signal
failures exit with code || 1.
- Add idempotent npm publish check to release workflow - Add 20s timeout and remove latest fallback in npm install script - Use execFileSync to avoid shell quoting issues in install script - Handle signals in npm run wrapper - Add final hard-budget guard in render logic to ensure token limit - Update README npx example to be non-mutating Co-Authored-By: Grey Newell <greyshipscode@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Made-with: Cursor
- Add .github/workflows/ci.yml to test Go build/vet and Node script syntax on PRs - Fix mergeHooks in internal/hooks/hooks.go to treat "show-hook.sh" as equivalent to "uncompact show-cache" for UserPromptSubmit to avoid duplicates Co-Authored-By: Grey Newell <greyshipscode@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Made-with: Cursor
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
npm install -g uncompactornpx uncompact installFiles Added
package.json- npm package definition with bin entry and postinstall hooknpm/install.js- downloads platform-specific binary from GitHub releasesnpm/run.js- wrapper script that spawns the Go binary.gitignore- ignores build artifacts and npm/bin/Test Plan
v0.2.0)npm install -g uncompactinstalls correctlynpx uncompact --helpworks without global installMade with Cursor
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Closes #562