fix(web): hand resolved pi-coding-agent entry to the /web child process - #355
Conversation
npm-installed users hit exit code 1 on /web: the standalone child process cannot resolve the peer dependency @earendil-works/pi-coding-agent from the npm package location. The parent extension runs inside Pi, so resolve the package entry there: walk up from realpathSync(process.argv[1]) to the pi-coding-agent package root and hand dist/index.js to the child via OPENPI_PI_CODING_AGENT_ENTRY. The child maps the bare specifier to that absolute path through a jiti alias. Resolution is fail-soft and any inherited stale env value is dropped, preserving the previous behavior when no path is found. Closes openpi-dev#341
|
@tt-a1i 佬 review 下🥺 |
|
@tt-a1i 希望 review下，这个修了之前tui 中/web 的 bug |
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Exact-head review: 2c11f20
[P1] The advertised direct openpi web path is still broken for a Pi-managed npm installation without peer dependencies. extensions/web/index.ts resolves the Pi entry and injects OPENPI_PI_CODING_AGENT_ENTRY only when the TUI /web command spawns the child. bin/openpi.js merely consumes that environment variable; when the bin is invoked directly there is no parent extension to set it, so its imports still resolve the absent peer package and fail with Cannot find module @earendil-works/pi-coding-agent.
The PR description promises both /web and openpi web work for every npm-installed user, so this leaves a named entry point unusable. Please either make the CLI independently resolve the Pi installation through a supported source of truth, or narrow/remove the direct CLI contract and tests/docs accordingly. Add a packed-install regression that invokes the installed bin with peers omitted and without pre-seeding the environment variable.
Verification boundary: exact-head code-path review plus isolated packed-install reproduction; the existing smoke pre-seeds the environment variable and therefore does not cover this path.
Register SIGINT/SIGTERM with process.once after host.start and before readiness so a SIGTERM at ready cannot miss the handler. Keep once so a second same signal can still default-terminate while stop is in flight.
npm installs omit @earendil-works/pi-coding-agent, so both launchers now share one resolver (env, node, argv, PATH) and keep OpenPI's existing pi-server alias instead of requiring a pre-seeded entry env var.
Validate handoff identity against the official package entry, prefer host argv over a local peer, and fail closed with an install diagnostic instead of PATH walks or mixed pi-server aliases.
Host resolution no longer accepts a leftover OPENPI_PI_CODING_AGENT_ENTRY from another Pi. Standalone still uses a validated explicit handoff, then its own peer. /web resolves that entry once and passes the exact path to the child.
waitForPath only stats existence, so a direct writeFile can let the parent observe an empty stop-entered file before contents land.
Windows CI resolved the official dist/index.js but the suffix regex required forward slashes. Keep the exact install entry and official package tail, and lock the failing Windows path.
Directory watch can fire first for the staging rename and miss the final name. Record first SIGTERM delivery and inspect both paths on timeout.
tt-a1i
left a comment
tt-a1i
left a comment
There was a problem hiding this comment.
Approved at exact head 2e8d0f3d6e35282578b404573db12172722ca182 (tree 26e110e32b7c51a4c47cd0cee530fbd220a92fca).
This resolves my earlier REQUEST_CHANGES at 2c11f20. The TUI /web path now resolves the current Pi host from the current process argv identity once and hands that exact validated package entry to the child. The standalone CLI accepts a validated explicit handoff first, then resolves only its own nested or standard npm-hoisted peer; it does not search PATH or inherit an arbitrary ancestor package. Missing peers fail closed with an actionable diagnostic.
I independently reviewed the host/standalone precedence, package identity and export validation, symlink/realpath behavior, missing-peer path, and the real child-launch tests. The Windows-only follow-up preserves the full realpath and manifest/export assertions while comparing path structure across both separators. The required Node 22, Node 24, and Windows checks are green at this head; the Node jobs include the Smoke-test packed standalone Web CLI step. I found no remaining P0/P1 issue in this scope.
2 participants
Fixes #341 by handing the current Pi host entry to the Web child. The TUI
/webcommand resolves the package from the running Pi process identity, validates its manifest/export entry, and passes it to the standalone loader.Direct
openpi webaccepts a validated explicit handoff or resolves its installed nested/hoisted Pi peer. If neither exists, it exits with installation guidance; it does not search PATH for another Pi runtime. A peer-free standalone installation therefore needs the peer installed or an explicit handoff. Compiled Pi binaries remain outside this package-layout resolver.Integrated the CLI signal-order fix from #401 while preserving both dependency-resolution and shutdown regressions. The hanging-stop fixture includes the standalone resolver and a valid peer fixture.
Validation:
bun run checkandbun run testpassed on the integrated tree, including Vitest. Current-head CI covers Node 22, Node 24, Windows, and the packed standalone Web CLI smoke. Native interactive Windows UI acceptance is not claimed.