Add runpane npm and PyPI installer packages#246
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fefa582cd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
parsakhaz
left a comment
There was a problem hiding this comment.
PR Review
Issue context: #243 - Ship npm + pip installer packages (runpane) as cross-platform installer/configurator wrappers for the Pane client and daemon.
Quality Gates
- Typecheck: PASS
- Lint: PASS (0 errors; existing warnings only)
Must-Fix (0)
None.
Should-Fix (1)
- Python
resolve_existing_pane_pathuses subprocess for macOS detection instead ofplatform.system()(packages/runpane-py/src/runpane/installers.py:34)
subprocess.run(["uname"], capture_output=True, text=True).stdout.strip() == "Darwin"is inconsistent withplatforms.pywhich already usesplatform.system(). More importantly,subprocess.run(["uname"])can raiseFileNotFoundErrorin minimal containers or constrained environments where/usr/bin/unameisn't available. Fix:import platformat the top and replace withelif platform.system().lower() == "darwin":.
Suggestions (2)
-
Both download implementations read entire response into memory (
packages/runpane/src/download.ts:57,packages/runpane-py/src/runpane/download.py:28)
Pane installers can be 100MB+. Python reads the entire response withresponse.read(), Node usesBuffer.from(await response.arrayBuffer()). Streaming would reduce peak memory: Python could useshutil.copyfileobj(response, target, length=1024*1024), Node could piperesponse.bodyto afs.createWriteStream. Not blocking, but worth considering for daemon installs on low-memory servers. -
matchesPlatformsubstring matching (packages/runpane/src/releases.ts:112,packages/runpane-py/src/runpane/releases.py:92)
"win" in name.lower()would match any asset name containing "darwin" (since "darwin" contains "win" at index 3). The current asset naming convention ("macOS", "Windows", "linux") avoids this, and the format filter provides a safety net, so this isn't a real bug today. But if asset names ever used "darwin" instead of "macOS", a Windows platform query could select a macOS artifact. Matching"windows" in lower or "win-" in lower or "win32" in lowerwould be more robust.
Completeness
The PR fully implements the plan from #243:
- npm wrapper package with all specified commands (install client/daemon, update, version, doctor)
- PyPI wrapper package with the same command contract
- Download attribution via
source=npm/source=pipquery params - SHA256 checksum verification
- Comprehensive parity tests between Node and Python parsers, platform detection, and artifact selection
- Version sync mechanism integrated into the release script
- CI additions for cross-OS/cross-runtime matrix testing
- Docs, README, Settings UI, and UpdateDialog copy all updated
Non-goal boundaries are respected: no process management, no config ownership, no Homebrew formula.
Summary
This is a well-structured PR. The two wrapper packages maintain behavioral parity with a solid test harness, the release pipeline integration is thorough, and the code is clean. The uname subprocess call is the only item I'd fix before merge; the memory and substring suggestions are nice-to-haves.
parsakhaz
left a comment
There was a problem hiding this comment.
PR Review
Issue context: #243 -- Ship thin CLI wrapper packages on npm (runpane) and PyPI (runpane) as cross-platform installer/configurator entry points for Pane.
Quality Gates
- Typecheck: PASS
- Lint: PASS (0 errors; 165 pre-existing warnings)
check:runpane-package-versions: PASStest:runpane-contract: PASStest:runpane-package-smoke: PASS
Must-Fix (0)
No blocking issues found.
Should-Fix (1)
install daemondownloads artifact before checking if it can reuse existing Pane (packages/runpane/src/cli.ts:44-48,packages/runpane-py/src/runpane/cli.py:941-942): In both wrappers,installOrUpdate/install_or_updatedownloads the full release artifact unconditionally, theninstallPaneArtifact/install_pane_artifactchecks for an existing Pane installation and returns early for daemon installs without using the download. On a headless server where Pane is already installed, this wastes a 100+ MB download. The fix is to checkshouldReuseExistingPane+resolveExistingPanePathbefore callingdownloadArtifact, and skip the download when an existing binary will be reused.
Suggestions (2)
-
UpdateDialog
npxfallback requires Node.js (frontend/src/components/UpdateDialog.tsx:275,main/src/ipc/updater.ts:11): The macOS manual-update command changed fromcurl -fsSL https://runpane.com/install.sh | shtonpx --yes runpane@latest update. Since 96% of installs come from website downloads (per the issue) and this is a fallback for when auto-update fails, the curl approach was more universally available because it doesn't require Node.js. Consider keeping curl for the macOS update dialog, or detectingnpxavailability and falling back. -
Minor Python style inconsistency (
packages/runpane-py/src/runpane/platforms.py:53):arch_aliasesuseslist[str]as a return annotation while other files (cli.py,installers.py, etc.) usetyping.List[str]. Works correctly at runtime due tofrom __future__ import annotations, and CI passes on Python 3.8, but it's inconsistent.
Completeness
All tasks from the issue are covered:
- npm package with
install client,install daemon,update,version,doctorcommands - PyPI package with the same command contract
- Download attribution via
source=npm/source=pip - GitHub release asset fallback when website route fails
- SHA256 checksum verification
- Platform detection (darwin/linux/win32 x x64/arm64)
- Daemon passthrough flags forwarded to
pane --remote-setup - Unknown daemon flags forwarded (future-proofing)
- Version sync tooling (
sync-runpane-package-versions.js) - Release script integration with version sync + verification
- Contract tests ensuring npm/Python parity
- Package smoke tests (npx, pnpm dlx, npm install, pip install)
- CI matrix across Node 18/22, Python 3.8/3.13, Linux/macOS/Windows
- npm and PyPI publish jobs in release workflow with trusted publishing + token fallback
- README, docs, Settings UI, and UpdateDialog copy updates
Summary
Well-structured PR. The dual-package contract testing is a standout -- parser parity, platform logic parity, and artifact selection parity between Node and Python are all verified in CI, which will prevent drift as both packages evolve. The version sync tooling integrated into the release script is solid.
The one should-fix (wasted download on daemon reuse) is straightforward to address. The UpdateDialog npx requirement is worth a second look since it narrows the audience for the manual-update fallback. Ready to merge after addressing the should-fix.
Summary
Fixes #243.
Adds
runpanewrapper packages for npm and PyPI so users can install or set up Pane from package-manager-native commands instead of choosing between platform-specific shell scripts.packages/runpane, a Node/TypeScript CLI withrunpane install client,runpane install daemon,runpane update,runpane version, andrunpane doctor.packages/runpane-py, a stdlib-only Python CLI exposing the same command contract throughpip,pipx,uvx, andpython -m runpane.runpane.com/api/downloadwithsource=npmorsource=pip, with GitHub release asset fallback.npx --yes runpane@latest/pnpm dlx/pipxpaths.docs/RUNPANE_CLI_CONTRACT.md, version sync checks, package-manager smoke tests, and cross-OS wrapper CI for Node/Python compatibility.Compatibility
>=18.17.0.>=3.8.>=22.14.0.Testing
Local checks run:
pnpm run check:runpane-package-versionspnpm run test:runpane-contractpnpm run test:runpane-package-smokenpx --yes node@18.17.0 packages/runpane/dist/cli.js --helpnpx --yes node@18.17.0 scripts/test-runpane-contract.jspnpm typecheckpnpm lint(0 errors; existing warnings remain)git diff --checkManual smoke also covered local tarball/package installs on WSL and Windows for
npx,npm install,pnpm dlx,pnpm add, and Python venv installs. Real registry publish and real Pane installation were not run from this branch.Release Notes
Before release, configure or verify trusted publishers:
runpane: repositorydcouple/Pane, workflowbuild.yml.runpane: repositorydcouple/Pane, workflowbuild.yml, environmentpypi.Temporary
NPM_TOKEN/PYPI_API_TOKENfallbacks are supported for first package reservation or recovery, but should be removed/rotated after use.