fix(web): hand resolved pi-coding-agent entry to the /web child process - #355
fix(web): hand resolved pi-coding-agent entry to the /web child process#355FHMinyi wants to merge 6 commits into
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.
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.
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.
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
Problem
Closes #341.
Users who install via
pi install npm:@tt-a1i/openpi(the documented install path) cannot start the Web Workbench:/webreportsOpenPI Web Workbench exited with code 1.and the real error is swallowed when the TUI resumes. The spawned standalone node process fails withCannot find module '@earendil-works/pi-coding-agent'because the package is apeerDependency, whichpi install npm:does not install. Dev checkouts never see this because the same-name devDependency resolves.Value
Makes
/webandopenpi webwork for every npm-installed user — currently the feature is broken for 100% of that install base. No behavior change for dev checkouts or for environments where resolution fails.Approach
The parent extension runs inside the Pi process, where the loader resolves the peer package. It resolves the entry path and hands it to the child through
OPENPI_PI_CODING_AGENT_ENTRY; the child (bin/openpi.js) maps the bare specifier to that absolute path via a jitialias.Why walk-up instead of
import.meta.resolve/createRequire(verified with a probe extension inside a real Pi process, fnm + node 24):pi-coding-agent/dist/core/extensions/loader.js) jiti-aliases the specifier to<pkg>/dist/index.js, so dynamicimport()works, butimport.meta.resolveandcreateRequire(import.meta.url)are not intercepted and fail from the extension's location.createRequire(process.argv[1])fails because argv[1] is a version-manager shim symlink (node does not realpath it); afterrealpathSync, CJS resolution is still rejected (exportshas norequirecondition).realpathSync(process.argv[1])→ walk up to thepackage.jsonnamed@earendil-works/pi-coding-agent→ use<root>/dist/index.js(the exact path Pi's own loader aliases to), verified withexistsSync.Design choices:
OPENPI_PI_CODING_AGENT_ENTRYis deleted, and the child behaves exactly as before.WebCommandDependenciesseam so tests fake it without touching the filesystem.Known limitation (not addressed here): bun/SEA compiled Pi binaries have no node_modules tree, so walk-up finds nothing and behavior is unchanged; the
/webspawn viaprocess.execPathalready assumes a node runtime.Validation
bun run testonmain @ 72fbba5: 1243 pass / 0 fail / 1 skipped.bun run check: config contract, discipline ledger, web syntax check, biome format + lint, tsc — all green.tests/extensions/web/index.test.ts: the launcher hands the resolved entry to the child env and drops a stale inherited value when resolution fails.tests/web/cli.test.ts: end-to-end — a stub package layout without the peer dependency proves the jiti alias redirects the bare@earendil-works/pi-coding-agentimport to the handed-over entry.OPENPI_PI_CODING_AGENT_ENTRY=<global pi dist/index.js> node bin/openpi.js web --no-workspace --no-open— WebHost started on127.0.0.1:53056, printed the authenticated ready screen, and released the host lease on shutdown./webpath (thectx.mode === "tui"gate cannot be automated here); the smoke exercised the identical child command the TUI spawns.Impact
/webandopenpi webnow start for npm-installed users; unchanged otherwise.