Reviewed and merged GitHub PR #922

Summary

Closed, Completed

Understand
Read PR, linked issues, and comments.
Understand
Understand
Analyzed code changes and main branch files.
Understand
Understand
Reviewed documentation for updates.
Understand
Edit
Fixed changelog attribution.
Edit
Build
Merged PR to main branch with updates.
Build

You are given one or more GitHub PR URLs: https://github.com/badlogic/pi-mono/pull/922

For each PR URL, do the following in order:

Read the PR page in full. Include description, all comments, all commits, and all changed files.
Identify any linked issues referenced in the PR body, comments, commit messages, or cross links. Read each issue in full, including all comments.
Analyze the PR diff. Read all relevant code files in full with no truncation from the current main branch and compare against the diff. Do not fetch PR file blobs unless a file is missing on main or the diff context is insufficient. Include related code paths that are not in the diff but are required to validate behavior.
Check for a changelog entry in the relevant packages/*/CHANGELOG.md files. Report whether an entry exists. If missing, state that a changelog entry is required before merge and that you will add it if the user decides to merge. Follow the changelog format rules in AGENTS.md. Verify:
- Entry uses correct section (### Breaking Changes, ### Added, ### Fixed, etc.)
- External contributions include PR link and author: Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))
- Breaking changes are in ### Breaking Changes, not just ### Fixed
Check if packages/coding-agent/README.md, packages/coding-agent/docs/.md, packages/coding-agent/examples/**/.md require modification. This is usually the case when existing features have been changed, or new features have been added.
Provide a structured review with these sections:
- Good: solid choices or improvements
- Bad: concrete issues, regressions, missing tests, or risks
- Ugly: subtle or high impact problems
Add Questions or Assumptions if anything is unclear.
Add Change summary and Tests.

Output format per PR: PR: Changelog:

... Good:
... Bad:
... Ugly:
... Questions or Assumptions:
... Change summary:
... Tests:
...

If no issues are found, say so under Bad and Ugly.

>cd /Users/badlogic/workspaces/pi-mono && gh pr view 922 --json title,body,comments,commits,files,author,state,labels,headRefName,baseRefName

{"author":{"id":"MDQ6VXNlcjIyMDY1ODM=","is_bot":false,"login":"dannote","name":"Danila Poyarkov"},"baseRefName":"main","body":"Couldn't find the commits @badlogic mentioned in #666 (d409b37a, ed042e3d), so reimplementing the fix.\n\n**Problem**\n\n1. `bun run build` causes infinite recursion because Bun doesn't properly handle npm's `-w` flag\n2. Dynamic imports for Node.js modules (fs, os, path, crypto, http) don't load in Bun because runtime detection only checks `process.versions.node`\n\n**Solution**\n\n1. Replace `-w @package-name` syntax with `cd packages/dir && npm run script` in root package.json\n2. Update runtime detection to include Bun: `process.versions?.node || process.versions?.bun`\n\n**Changes**\n\n- `package.json`: Rewrite build, dev, dev:tsc, check scripts to use cd-based approach\n- `packages/ai/src/stream.ts`: Add Bun runtime detection\n- `packages/ai/src/providers/openai-codex-responses.ts`: Add Bun runtime detection\n- `packages/ai/src/utils/oauth/openai-codex.ts`: Add Bun runtime detection","comments":[],"commits":[{"authoredDate":"2026-01-23T16:31:16Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjIyMDY1ODM=","login":"dannote","name":"Danila Poyarkov"}],"committedDate":"2026-01-23T16:31:16Z","messageBody":"","messageHeadline":"fix: Bun compatibility for build scripts and runtime detection","oid":"6d0c544e18070c4d29c1274e3c8c11d443403688"}],"files":[{"path":"package.json","additions":4,"deletions":4},{"path":"packages/ai/CHANGELOG.md","additions":4,"deletions":0},{"path":"packages/ai/src/providers/openai-codex-responses.ts","additions":1,"deletions":1},{"path":"packages/ai/src/stream.ts","additions":2,"deletions":2},{"path":"packages/ai/src/utils/oauth/openai-codex.ts","additions":1,"deletions":1}],"headRefName":"fix/bun-compatibility","labels":[],"state":"OPEN","title":"fix: Bun compatibility for build scripts and runtime detection"}

>cd /Users/badlogic/workspaces/pi-mono && gh pr diff 922

diff --git a/package.json b/package.json
index fcb085567..9099da721 100644
--- a/package.json
+++ b/package.json
@@ -9,10 +9,10 @@
 	],
 	"scripts": {
 		"clean": "npm run clean --workspaces",
-		"build": "npm run build -w @mariozechner/pi-tui && npm run build -w @mariozechner/pi-ai && npm run build -w @mariozechner/pi-agent-core && npm run build -w @mariozechner/pi-coding-agent && npm run build -w @mariozechner/pi-mom && npm run build -w @mariozechner/pi-web-ui && npm run build -w @mariozechner/pi",
-		"dev": "concurrently --names \"ai,agent,coding-agent,mom,web-ui,tui\" --prefix-colors \"cyan,yellow,red,white,green,magenta\" \"npm run dev -w @mariozechner/pi-ai\" \"npm run dev -w @mariozechner/pi-agent-core\" \"npm run dev -w @mariozechner/pi-coding-agent\" \"npm run dev -w @mariozechner/pi-mom\" \"npm run dev -w @mariozechner/pi-web-ui\" \"npm run dev -w @mariozechner/pi-tui\"",
-		"dev:tsc": "concurrently --names \"ai,web-ui\" --prefix-colors \"cyan,green\" \"npm run dev:tsc -w @mariozechner/pi-ai\" \"npm run dev:tsc -w @mariozechner/pi-web-ui\"",
-		"check": "biome check --write . && tsgo --noEmit && npm run check -w @mariozechner/pi-web-ui",
+		"build": "cd packages/tui && npm run build && cd ../ai && npm run build && cd ../agent && npm run build && cd ../coding-agent && npm run build && cd ../mom && npm run build && cd ../web-ui && npm run build && cd ../pods && npm run build",
+		"dev": "concurrently --names \"ai,agent,coding-agent,mom,web-ui,tui\" --prefix-colors \"cyan,yellow,red,white,green,magenta\" \"cd packages/ai && npm run dev\" \"cd packages/agent && npm run dev\" \"cd packages/coding-agent && npm run dev\" \"cd packages/mom && npm run dev\" \"cd packages/web-ui && npm run dev\" \"cd packages/tui && npm run dev\"",
+		"dev:tsc": "concurrently --names \"ai,web-ui\" --prefix-colors \"cyan,green\" \"cd packages/ai && npm run dev:tsc\" \"cd packages/web-ui && npm run dev:tsc\"",
+		"check": "biome check --write . && tsgo --noEmit && cd packages/web-ui && npm run check",
 		"test": "npm run test --workspaces --if-present",
 		"version:patch": "npm version patch -ws --no-git-tag-version && node scripts/sync-versions.js && rm -rf node_modules packages/*/node_modules package-lock.json && npm install",
 		"version:minor": "npm version minor -ws --no-git-tag-version && node scripts/sync-versions.js && rm -rf node_modules packages/*/node_modules package-lock.json && npm install",
diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md
index 7ecdc53aa..ebd714b93 100644
--- a/packages/ai/CHANGELOG.md
+++ b/packages/ai/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed Bun runtime detection for dynamic imports in browser-compatible modules (stream.ts, openai-codex-responses.ts, openai-codex.ts)
+
 ## [0.49.3] - 2026-01-22
 
 ### Added
diff --git a/packages/ai/src/providers/openai-codex-responses.ts b/packages/ai/src/providers/openai-codex-responses.ts
index abf21a5f8..8a8232806 100644
--- a/packages/ai/src/providers/openai-codex-responses.ts
+++ b/packages/ai/src/providers/openai-codex-responses.ts
@@ -1,6 +1,6 @@
 // NEVER convert to top-level import - breaks browser/Vite builds (web-ui)
 let _os: typeof import("node:os") | null = null;
-if (typeof process !== "undefined" && process.versions?.node) {
+if (typeof process !== "undefined" && (process.versions?.node || process.versions?.bun)) {
 	import("node:os").then((m) => {
 		_os = m;
 	});
diff --git a/packages/ai/src/stream.ts b/packages/ai/src/stream.ts
index c3804617d..ab71c9641 100644
--- a/packages/ai/src/stream.ts
+++ b/packages/ai/src/stream.ts
@@ -3,8 +3,8 @@ let _existsSync: typeof import("node:fs").existsSync | null = null;
 let _homedir: typeof import("node:os").homedir | null = null;
 let _join: typeof import("node:path").join | null = null;
 
-// Eagerly load in Node.js environment only
-if (typeof process !== "undefined" && process.versions?.node) {
+// Eagerly load in Node.js/Bun environment only
+if (typeof process !== "undefined" && (process.versions?.node || process.versions?.bun)) {
 	import("node:fs").then((m) => {
 		_existsSync = m.existsSync;
 	});
diff --git a/packages/ai/src/utils/oauth/openai-codex.ts b/packages/ai/src/utils/oauth/openai-codex.ts
index 1d9676408..8b0578ab5 100644
--- a/packages/ai/src/utils/oauth/openai-codex.ts
+++ b/packages/ai/src/utils/oauth/openai-codex.ts
@@ -8,7 +8,7 @@
 // NEVER convert to top-level imports - breaks browser/Vite builds (web-ui)
 let _randomBytes: typeof import("node:crypto").randomBytes | null = null;
 let _http: typeof import("node:http") | null = null;
-if (typeof process !== "undefined" && process.versions?.node) {
+if (typeof process !== "undefined" && (process.versions?.node || process.versions?.bun)) {
 	import("node:crypto").then((m) => {
 		_randomBytes = m.randomBytes;
 	});

Let me read the linked issue #666:

>cd /Users/badlogic/workspaces/pi-mono && gh issue view 666 --json title,body,comments,labels,state

{"body":"## Summary\n- Fix build script to work with Bun\n\n## Problem\nBun does not properly handle npm's `-w` flag for workspace filtering, causing recursive build invocations when running `bun run build`.\n\n## Solution\n- Replace `-w @mariozechner/pkg` syntax with `cd packages/pkg && npm run build` approach","comments":[{"id":"IC_kwDOPbFNk87e940U","author":{"login":"badlogic"},"authorAssociation":"OWNER","body":"Thanks for the PR! Implemented a more complete fix that covers all scripts (build, clean, dev, dev:tsc, check, test) plus browser compatibility fixes for pi-ai. See commits d409b37a and ed042e3d on main.","createdAt":"2026-01-12T22:15:15Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[{"content":"HEART","users":{"totalCount":1}}],"url":"https://github.com/badlogic/pi-mono/pull/666#issuecomment-3740765460","viewerDidAuthor":true},{"id":"IC_kwDOPbFNk87gNkBc","author":{"login":"dannote"},"authorAssociation":"CONTRIBUTOR","body":"@badlogic Sorry, I don't see those commits. Did you push them? 🙂","createdAt":"2026-01-16T20:17:17Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/666#issuecomment-3761651804","viewerDidAuthor":false}],"labels":[],"state":"CLOSED","title":"Fix build script for Bun compatibility"}

Now let me read the affected files on main to compare:

Now let me check the coding-agent documentation to see if any updates are needed:

Now I have all the information needed to provide a comprehensive review.

PR: https://github.com/badlogic/pi-mono/pull/922

Changelog:

Exists: Yes, a changelog entry has been added to packages/ai/CHANGELOG.md under ## [Unreleased] in the ### Fixed section

Format issue: The entry does not include the PR link and author attribution as required for external contributions. Should be:

- Fixed Bun runtime detection for dynamic imports in browser-compatible modules (stream.ts, openai-codex-responses.ts, openai-codex.ts) ([#922](https://github.com/badlogic/pi-mono/pull/922) by [@dannote](https://github.com/dannote))

- Fixed Bun runtime detection for dynamic imports in browser-compatible modules (stream.ts, openai-codex-responses.ts, openai-codex.ts) ([#922](https://github.com/badlogic/pi-mono/pull/922) by [@dannote](https://github.com/dannote))

Root package.json change: The root package.json script changes should have a separate changelog entry, possibly in a top-level changelog or noted somewhere since it affects the build system across all packages

Good:

The Bun runtime detection fix is correct: process.versions.bun exists in Bun runtime, so checking (process.versions?.node || process.versions?.bun) properly handles both Node.js and Bun
The fix is surgical and minimal, only changing the runtime detection condition in three files
Follows the existing pattern established in the codebase (dynamic imports for browser compatibility)
The contributor (@dannote) has previous contributions to this repo (MiniMax provider, tool argument validation fix)
The PR description clearly explains both problems and solutions

Bad:

Changelog attribution missing: External contribution should include PR link and author per AGENTS.md rules
Missing pkg label: The PR should have pkg:ai label since it modifies packages/ai/*
Build script changes not fully tested: The cd && npm run approach relies on shell behavior (each command runs in a subshell, so cd doesn't persist). The chained cd packages/tui && npm run build && cd ../ai && npm run build works because they're in one shell context with &&, but this is fragile. The original -w approach was more declarative.
Incomplete script migration: The PR changes build, dev, dev:tsc, and check, but leaves clean and test using the -w flag. If Bun has issues with -w, those scripts would also fail.

Ugly:

Root cause not addressed: The underlying issue is that Bun doesn't properly handle npm's -w flag. The workaround in this PR is valid, but the commits referenced by @badlogic (d409b37a, ed042e3d) were never pushed as noted in issue #666. This PR is a re-implementation of a fix that was supposedly already done.
The cd-chain approach is more error-prone: If a developer adds a new package or renames one, they need to remember to update the root package.json scripts. The -w @mariozechner/pkg approach was self-documenting because it used package names.

Questions or Assumptions:

Are the referenced commits (d409b37a, ed042e3d) from a local branch that was never pushed, or were they lost?
Should the clean and test scripts also be converted to the cd approach for Bun consistency?
Is Bun officially supported as a build environment for this monorepo? The README mentions "Build from source (requires Bun 1.0+)" for binary builds.

Change summary:

Root package.json: Rewrites build, dev, dev:tsc, and check scripts to use cd packages/X && npm run Y instead of npm run Y -w @package-name
packages/ai/src/stream.ts: Adds Bun runtime detection (process.versions?.bun) alongside Node.js detection
packages/ai/src/providers/openai-codex-responses.ts: Same Bun runtime detection fix
packages/ai/src/utils/oauth/openai-codex.ts: Same Bun runtime detection fix
packages/ai/CHANGELOG.md: Adds changelog entry for Bun runtime detection fix (missing attribution)

Tests:

No automated tests added or modified
The runtime detection changes are not covered by existing tests (they're conditional dynamic imports for browser compatibility)
Manual testing would require running bun run build from the repo root and verifying the build completes successfully
Manual testing would also require running pi-ai in Bun to verify the dynamic imports load correctly

Documentation Update Required:

No changes needed to packages/coding-agent/README.md or docs. The Bun support mentioned in the README for binary builds still works. This PR fixes bun run build which was broken, but that's a developer concern not documented as a user feature.

read package.json and see wtf is up

The current package.json on main still uses the -w @mariozechner/... syntax. The PR wants to change it to the cd packages/X && npm run Y approach.

Looking at the PR diff again:

diff

-"build": "npm run build -w @mariozechner/pi-tui && npm run build -w @mariozechner/pi-ai && ...",+"build": "cd packages/tui && npm run build && cd ../ai && npm run build && ...",

-"build": "npm run build -w @mariozechner/pi-tui && npm run build -w @mariozechner/pi-ai && ...",+"build": "cd packages/tui && npm run build && cd ../ai && npm run build && ...",

The issue mentioned in #666 is that Bun doesn't properly handle npm's -w flag. The workaround replaces workspace filtering with explicit cd commands.

The "wtf" here is that @badlogic claimed in issue #666 (closed) that commits d409b37a and ed042e3d fixed this, but those commits were never pushed to main. The contributor @dannote pointed this out in the issue comments, and this PR is a re-implementation of that supposedly-done fix.

So the current state is:

Issue #666 was closed as "fixed"
The fix was never actually pushed
This PR (#922) re-implements the fix
The runtime detection for Bun in the pi-ai package is also missing (also supposedly fixed in those ghost commits)

well, merge via gh cli pull from origin

>cd /Users/badlogic/workspaces/pi-mono && git fetch origin pull/922/head:fix/bun-compatibility && git checkout fix/bun-compatibility && git rebase main

From https://github.com/badlogic/pi-mono
 * [new ref]           refs/pull/922/head -> fix/bun-compatibility
Switched to branch 'fix/bun-compatibility'
Current branch fix/bun-compatibility is up to date.

>cd /Users/badlogic/workspaces/pi-mono && git checkout main && git merge fix/bun-compatibility --no-ff -m "fix: Bun compatibility for build scripts and runtime detection (#922)"

Switched to branch 'main'
Your branch is up to date with 'origin/main'.
Merge made by the 'ort' strategy.
 package.json                                        | 8 ++++----
 packages/ai/CHANGELOG.md                            | 4 ++++
 packages/ai/src/providers/openai-codex-responses.ts | 2 +-
 packages/ai/src/stream.ts                           | 4 ++--
 packages/ai/src/utils/oauth/openai-codex.ts         | 2 +-
 5 files changed, 12 insertions(+), 8 deletions(-)

Now let me fix the changelog attribution:

EditedCHANGELOG.md

>cd /Users/badlogic/workspaces/pi-mono && npm run check 2>&1

> pi-monorepo@0.0.3 check
> biome check --write . && tsgo --noEmit && cd packages/web-ui && npm run check

Checked 417 files in 284ms. No fixes applied.

> @mariozechner/pi-web-ui@0.49.3 check
> biome check --write . && tsc --noEmit && cd example && biome check --write . && tsc --noEmit

Checked 73 files in 38ms. No fixes applied.
Checked 3 files in 8ms. No fixes applied.

>cd /Users/badlogic/workspaces/pi-mono && git add packages/ai/CHANGELOG.md && git commit --amend --no-edit

Running formatting, linting, and type checking...

> pi-monorepo@0.0.3 check
> biome check --write . && tsgo --noEmit && cd packages/web-ui && npm run check

Checked 417 files in 262ms. No fixes applied.

> @mariozechner/pi-web-ui@0.49.3 check
> biome check --write . && tsc --noEmit && cd example && biome check --write . && tsc --noEmit

Checked 73 files in 38ms. No fixes applied.
Checked 3 files in 8ms. No fixes applied.
✅ All pre-commit checks passed!
[main 2cdf608d] fix: Bun compatibility for build scripts and runtime detection (#922)
 Date: Fri Jan 23 17:38:26 2026 +0100

>cd /Users/badlogic/workspaces/pi-mono && gh pr close 922 --comment "Merged via 2cdf608d. Thanks for the fix!" && git branch -d fix/bun-compatibility

X Pull request badlogic/pi-mono#922 (fix: Bun compatibility for build scripts and runtime detection) can't be closed because it was already merged


Command exited with code 1

>cd /Users/badlogic/workspaces/pi-mono && gh pr comment 922 --body "Thanks for the fix!" && git branch -d fix/bun-compatibility

https://github.com/badlogic/pi-mono/pull/922#issuecomment-3791160338
Deleted branch fix/bun-compatibility (was 6d0c544e).

Done. Merged PR #922 to main:

Rebased on main
Fixed changelog attribution to include PR link and author
Pushed to main (2cdf608d)
Commented on PR thanking the contributor
Cleaned up local branch

>git pull

From https://github.com/badlogic/pi-mono
   2cdf608d..3629f64d  main       -> origin/main
Updating 2cdf608d..3629f64d
Fast-forward
 packages/coding-agent/CHANGELOG.md                        |  1 +
 packages/coding-agent/src/core/footer-data-provider.ts    | 15 +++++++++++++--
 .../src/modes/interactive/components/footer.ts            |  5 +++++
 .../src/modes/interactive/interactive-mode.ts             | 12 ++++++++++++
 4 files changed, 31 insertions(+), 2 deletions(-)