Skip to content

Add npm installation for macOS and Linux - #30

Open
hsliuustc0106 wants to merge 5 commits into
mainfrom
codex-npm-macos-install
Open

hsliuustc0106 wants to merge 5 commits into
mainfrom
codex-npm-macos-install

Conversation

@hsliuustc0106

@hsliuustc0106 hsliuustc0106 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Users currently need a Python checkout and a manually managed environment to run nanodot. Add an npm package for macOS and Linux, exposing the existing CLI through npm install -g nanodot or npx nanodot after the npm release.

The package bundles the Python source from the same checkout and requires Node.js 22+. The launcher reuses Python 3.11+ or prepares a private Python 3.12 runtime on first use with the official uv 0.12.21 installer. Runtime setup preserves shell profiles; npm installation works with --ignore-scripts. Arguments, stdin, working directory, signals, and CLI exit status pass through, including the bundled source path for the background runner.

Validation:

  • Eight launcher tests passed: interpreter reuse/fallback, stdin and literal arguments, cache reuse, failed setup/retry, concurrent first runs, and cooperative termination.
  • Packed global installation with --ignore-scripts, npx execution, payload checks, and version checks passed.
  • Real uv/Python download, cached restart, clean stdout, and GitHub TLS passed locally and in native macOS/Linux CI. TLS checks preserve certificate verification and do not depend on anonymous API quota.
  • Five tests passed on the main scaffold. Compatibility validation using the reviewed MVP snapshot 661f4ba passed all 515 tests; its installed npm package also completed a real anonymous watch on already-merged PR Implement #14: end-to-end PR-watch validation suite, all fakes #27 with one persistent inbox entry and no duplicate, then started/stopped the background runner. No runner remains active.

Integration: based on fetched main 82eb79a. The full MVP CLI release depends on #28. npm publication is a separate maintainer release step; this PR includes packaging/release instructions.

Final CI on bc6481f: PR npm checks, push npm checks, and Python CI all passed.

Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>

@hsliuustc0106 hsliuustc0106 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review: high quality, locally verified — approve-with-nits

I ran all three npm test suites locally on Linux, plus a trial merge against current main:

  • npm test — 8/8 launcher tests pass
  • npm run test:package — pack, --ignore-scripts global install, npx, payload/version checks pass
  • npm run test:bootstrap — real uv + Python 3.12 download, cached restart, GitHub TLS check pass
  • npm view nanodot → 404: the package name is free on the registry, no squatting risk

Verified as solid (no action needed)

  • Import-hijack protection: probes use python -I; the CLI runs with PYTHONSAFEPATH=1 plus an explicit sys.path.insert(0, bundled-src), so neither cwd nor a caller's PYTHONPATH can shadow the bundled nanodot package. The decoy nanodot.py in package-smoke exercises exactly this.
  • No shell injection surface: everything goes through spawn without a shell; owner/repo#1; echo unsafe passes through literally (tested).
  • Supply-chain hygiene on the download path: uv pinned at 0.12.21 via the versioned official URL, curl -q disables .curlrc, --fail catches HTTP errors, install into a temp dir with an atomic rename publish; the concurrent-first-run EEXIST race is handled and tested.
  • System cleanliness: UV_UNMANAGED_INSTALL + UV_NO_MODIFY_PATH=1 + python install --no-bin — no shell profile edits, no global Python shims (the fake installer asserts both env vars).
  • CLI semantics pass through: argv, stdin (secret entry), cwd, cooperative SIGINT/SIGTERM, exit codes incl. signal→128+n; second run has clean stderr.
  • Version sync is gated across package.json / pyproject.toml / __init__.py by package-smoke.
  • files glob is complete: src on both this branch and current main (27 files post-#28) contains only .py, nothing to miss. node >=22 is the oldest maintained LTS line, a fair floor.

Required before merge

  1. README.md conflicts with main. The PR is based on 82eb79a; main since merged #28 and expanded the README. I verified in a scratch worktree that README.md is the only conflict — resolve by folding the npm install section into the current README structure rather than taking the PR version wholesale. Merging main in will also activate the conditional runner check in package-smoke (see below).

Suggestions

  1. CI double-runs: the workflow triggers on both push and pull_request, so same-repo PRs run every push twice (visible as two check sets). Suggest push: branches: [main].
  2. No LICENSE file: both manifests declare MIT but the repo has no LICENSE file, so npm pack ships without one. Worth adding before publishing an npm package.
  3. Hard curl dependency: bootstrap requires curl on PATH. Node 22 ships fetch — downloading the installer with it would drop the extra requirement on minimal Linux images. Also, when curl is missing the current error ("retry setup with internet access") is misleading for what is really spawn curl ENOENT.

Nits

  • stdio: ['inherit', check ? 2 : 'inherit', 'inherit'] — both branches are effectively identical (fd 2 direct vs inherited); simplifiable.
  • UV_CACHE_DIR retains the ~50MB download archive indefinitely; uv python install --no-cache would avoid it.
  • SIGHUP isn't forwarded, though the child shares the launcher's process group so tty-generated HUP reaches it; only a targeted non-tty HUP to the launcher PID is affected.

Note on coverage

The runner start/stop section of package-smoke is conditional on runner_control.py, which only enters the package after merging main (#28). It didn't execute on this branch. The PR description covers it via the MVP snapshot validation, but please merge main into this branch before merging so that block actually runs in CI.

@hsliuustc0106
hsliuustc0106 marked this pull request as ready for review October 2, 2026 09:53
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