Skip to content

refactor: emit ESM entries as .js and package tools/test - #422

Merged
oxwen11 merged 15 commits into
mainfrom
refactor/js-entries
Oct 6, 2026
Merged

oxwen11 merged 15 commits into
mainfrom
refactor/js-entries

Conversation

@oxwen11

@oxwen11 oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Requirement

Use ESM .js for Pie-owned entries, and make every tools/ directory a workspace package.

Expected behavior

@getpie/server#build emits dist/server.js. CLI bins are dist/cli.js and dist/relay.js. @getpie/core and oxlint plugins emit .js. Shared test helpers resolve through @getpie/test.

pie-pi-process stays pi-process.js. Daemon spawn, session create, and stored session data are unchanged.

Changes and risks

Root package.json is "type": "module", and tsdown fixedExtension is off so Node-platform builds follow that. Desktop pack hooks and the SSH daemon-record reader are ESM. tools/testing moved to tools/test (@getpie/test).

Installed apps and published CLI bins keep the old .mjs names until rebuilt. A remote host running the SSH launch script must accept node --input-type=module.

Verification

Not run as a suite. Confirmed tsdown emits dist/server.js, compatibility.js, and oxlint .js plugins. import.meta.resolve("@getpie/test/fake-pi") resolves from apps/app. Pre-commit oxlint/oxfmt passed on 8e91318e.

Packages are type module, so tsdown no longer forces .mjs. Shared test helpers live in @getpie/test.
@pkg-pr-new

pkg-pr-new Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
npx https://pkg.pr.new/oxwen11/pie/@getpie/cli@422

commit: c044e79

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit c044e79.

@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

Required CI blocker — not reviewed or merged

Head 8e91318ed5588366976ff043bad52f6a19eea702; current target/trusted rules 01f864fe30b4376714547bd6bd6576b5540e3d03. Check failure reports TS2339: BrowserPage.getSubmitButton missing in apps/app/e2e/helpers.ts, and BrowserPage.getBySlot missing in product-flow, message-view and tool-batch tests. @getpie/app#typecheck fails.

Stopped at CI, before full code review/independent E2E. Restore the browser-locator type integration in the candidate and obtain successful required checks, then restart review and independent acceptance. No rerun, implementation changes, conflict resolution or merge performed.

The setup file is only a runtime path, so tsc no longer saw getBySlot.
Include the locator module itself. Standalone scripts outside a package stay .mjs. The remote reader takes the record path from the environment.
tsdown may emit usesEmbeddedModules outside dist/server.js.
codeSplitting pulled usesEmbeddedModules out of server.js on CI. The daemon is spawned as that one file.
Keep ESM .js entries and @getpie/test. Keep main's seeded-provider and pie-verify HOME notes. codeSplitting stays off so the Pi 0.99.1 fold remains in server.js instead of a sibling .mjs chunk.
@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Merged origin/main into refactor/js-entries (no force push).

The only conflict was .agents/rules/topics/toolchain.md. Kept this PR's @getpie/test and fake-pi.js names, and main's seeded-provider note: pie-verify keeps HOME, and PI_CODING_AGENT_DIR must not point at an empty directory.

Main's bundled-node test scanned sibling .mjs chunks after the Pi 0.99.1 upgrade. This branch emits server.js with codeSplitting off, so the fold stays in that entry and the test reads only server.js. Locator types stay included from tools/test/browser-locators.ts in apps/app/tsconfig.json. The earlier TS2339 comment was already fixed on this branch and was not reopened by the merge.

Local: apps/app tsc passed; packages/ssh scripts.test.ts 11 passed; packages/server bundled-node.test.ts passed; desktop daemon-server-process.test.ts 8 passed.

@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Required Check failed on head 341094af (job): browser Vitest global setup throws [module runner] "import.meta.resolve" is not supported at apps/app/e2e/global-setup.ts:14, a file this PR changes. Treated as a real failure (no rerun). Blocks review; restart at CI on the next head.

@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Fixed the browser global-setup crash. Vite 5/8 module runner does not implement import.meta.resolve, so apps/app/e2e/global-setup.ts no longer calls it. @getpie/test/paths resolves fake-gh, fake-pi, and browser-locators from its own import.meta.url. The same replacement is in the browser Vitest config.

Merged origin/main (no force push). Local pnpm exec vitest run --config vitest.browser.config.mts: global setup ran; 99 passed, 1 failed. The failure is slash-command-menu.test.tsx ctrl+Enter expecting

/

and getting

/

, from main (#104), not this setup change.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

Head c044e793ef6d103c5a17044b37213e98ef576859; all checks green, MERGEABLE. Reviewed every hunk against the auto-merge rules (exclusions + docs-tooling + lint-format). Fail-closed reasons:

  • Non-presentational refactor of runtime/build artifacts (exclusion). Root package.json gains "type": "module", which changes how Node resolves every .js file in the repo. packages/server/tsdown.config.ts turns off fixedExtension and adds outputOptions.codeSplitting: false, so the forked server bundle is now built differently, not just renamed.
  • Product code paths change. apps/desktop/src/main/desktop-config.ts resolveServerEntry now spawns dist/server.js instead of server.mjs. Desktop pack hooks (after-pack.cjs to after-pack.js, before-pack.js, ad-hoc-sign.js, fff-island.js, electron-builder.yml) change how the packaged app is assembled.
  • Remote SSH behavior changes. packages/ssh/src/scripts.ts emit_daemon_record switches from CommonJS node - with argv to node --input-type=module with env vars. That runs on the user's remote host. The PR itself notes old Node versions on a remote must accept --input-type=module.
  • npm publishing (exclusion). packages/pie/package.json renames the published pie / pie-relay bins to dist/cli.js / dist/relay.js. The PR body says installed apps and published CLI bins keep the old .mjs names until rebuilt.
  • Not independently verified. The PR body says "Not run as a suite"; only tsdown output and one import.meta.resolve were confirmed. CI being green doesn't cover a packaged desktop build or a real SSH remote launch.

Needs a human review plus a packaged-desktop and SSH-remote smoke test before merge. Not merging.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review and verification record — approved, merging

  • Head c044e793ef6d103c5a17044b37213e98ef576859; base and trusted rules 74caa34913a6153175622aa57ec9886c2b3090e7.
  • Required CI on this head, all success:
  • The earlier failure (import.meta.resolve in the browser global setup) is fixed on this head.

Review

  • Every Pie-owned package is now "type": "module".
    • This includes the published @getpie/cli, so dist/cli.js is ESM outside the monorepo too.
    • @getpie/test is a devDependency only, so it is never a runtime dependency of the published CLI.
  • No remaining cli|server|relay|compatibility.mjs or tools/testing references in code, rules or skills (outside historical ADR/RFC/todos).
  • Rule and skill edits are mechanical renames.
  • SSH launch script: the daemon-record reader now gets its path and mode from env vars, not argv. The heredoc is still quoted, so there is no new injection surface. The remote already requires Node 24 (engine check), which supports --input-type=module.
  • No persistence, contract or stored-data change. Daemon compatibility is still keyed by githash, not by entry filename.

Independent verification

Clean worktree at this head. Locked install. pnpm build (Turbo).

  • Build output: packages/server/dist/server.js, packages/pie/dist/{cli,relay}.js, packages/core/dist/compatibility.js, pi-process.js. No .mjs entries are left in those dists.
  • Built CLI daemon (isolated PIE_HOME, own port):
    • node packages/pie/dist/cli.js daemon start and status work. The daemon runs dist/cli.js serve.
    • A real model turn ran bash (ESM-422-OK) through dist/pi-process/pi-process.js.
    • daemon stop works.
  • Published shape: pnpm pack @getpie/cli, npm install the tarball into an empty directory, then pie --help runs. The package is "type": "module".
  • Packaged Desktop (electron-builder --dir, macOS arm64):
    • ESM before-pack.js downloaded the bundled Bun into Resources/vendor. ESM after-pack.js applied the ad-hoc signature.
    • The asar contains node_modules/@getpie/server/dist/server.js.
    • Launched with an isolated PIE_HOME and isolated userData. The daemon ran as Pie Helper …/app.asar/node_modules/@getpie/server/dist/server.js, health was ok, and the renderer loaded pie://app/draft.
    • A real model turn through that packaged daemon ran pi-process.js from app.asar.unpacked (PKG-422-OK).
    • App quit and daemon stopped.
  • Dev Desktop via pie-verify desktop (the verify tool itself is changed here): launch, doctor (ticket 401/200) and cleanup all OK. The renderer was at /draft.
  • SSH scripts: packages/ssh/src/scripts.test.ts passed 11/11. These tests execute the generated script, including attach through the ESM daemon-record reader under ~/.pie and PIE_HOME.
  • No explicit model was selected, so the shared Pi default was not written. All owned processes were stopped.

Gaps

  • No live SSH launch. A remote resolves npx @getpie/cli@latest, which is the already-published CLI, so a remote run would not exercise this head. The script itself is covered by the executed script tests.
  • No non-macOS packaging.
  • As stated in the PR body, existing installed apps and published CLIs keep .mjs until they are rebuilt or upgraded.

@oxwen11
oxwen11 merged commit 9d2ea78 into main Oct 6, 2026
6 checks passed
@oxwen11
oxwen11 deleted the refactor/js-entries branch October 6, 2026 03:19
@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Merged as 9d2ea78 (squash, head guard c044e793); follows the record above.

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.

2 participants