Skip to content

fix(cli): package native npm binaries#42

Merged
khaliqgant merged 2 commits into
mainfrom
fix/npm-cli-binary-packaging
Apr 17, 2026
Merged

fix(cli): package native npm binaries#42
khaliqgant merged 2 commits into
mainfrom
fix/npm-cli-binary-packaging

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

@khaliqgant khaliqgant commented Apr 17, 2026

Summary

  • package platform-specific relayfile CLI binaries in the npm package
  • update the JS launcher/postinstall path to use included binaries before release downloads
  • build and upload CLI binaries during the publish workflow
  • complete and commit the Trail trajectory for this work

Verification

  • npm run build --workspace=packages/cli
  • npm pack --workspace=packages/cli
  • temp global install with npm_config_ignore_scripts=true runs relayfile -h
  • temp global install with npm_config_ignore_scripts=false creates bin/relayfile and runs relayfile -h
  • go test ./cmd/relayfile-cli
  • npm publish --dry-run --access public --ignore-scripts --workspace=packages/cli
  • ruby -e 'require "yaml"; YAML.load_file(".github/workflows/publish.yml")'

Open with Devin

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

@khaliqgant khaliqgant merged commit dbd1f39 into main Apr 17, 2026
6 checks passed
@khaliqgant khaliqgant deleted the fix/npm-cli-binary-packaging branch April 17, 2026 11:59
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 8 additional findings in Devin Review.

Open in Devin Review

Comment thread packages/cli/package.json
"postinstall": "node scripts/install.js || true"
"build": "node scripts/build-binaries.js",
"prepack": "npm run build",
"postinstall": "node scripts/install.js"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Removing || true from postinstall causes hard failure on unsupported platforms

The postinstall script was changed from "node scripts/install.js || true" to "node scripts/install.js". On unsupported platforms (e.g., FreeBSD, s390x), getPlatformSuffix() at packages/cli/scripts/install.js:30-34 calls process.exit(1) when the platform/arch isn't in the mapping. Without || true, this causes the entire npm install relayfile to fail. Previously, the || true catch-all allowed npm install to succeed even when the binary couldn't be installed, letting users see a clear error only at runtime. Now the package is completely uninstallable on any platform not in the 6 pre-built targets. While the packaged-binary happy path works for supported platforms, the process.exit(1) in getPlatformSuffix should be replaced with a graceful warning + process.exit(0) so npm install doesn't abort.

Prompt for agents
The postinstall script changed from `node scripts/install.js || true` to `node scripts/install.js`. This means any `process.exit(1)` in install.js will now fail `npm install` entirely. The specific problem is in `packages/cli/scripts/install.js` in the `getPlatformSuffix()` function (lines 30-34), which calls `process.exit(1)` on unsupported platforms. The fix should be in install.js rather than restoring `|| true` — change the unsupported platform handling to log a warning and exit(0) gracefully, so npm install succeeds but the user is told the CLI binary isn't available for their platform. This keeps failures visible for real errors while being tolerant of unsupported platforms.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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