fix(doctor): report Bun runtime provenance - #861
Conversation
📝 WalkthroughWalkthroughChangesBun runtime selection now returns provenance labels. Launchers, services, shims, and the Windows tray pass those labels through Bun runtime provenance
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Launcher
participant DurableRuntime
participant ServiceOrShim
participant BunProcess
participant MemoryAPI
participant Doctor
Launcher->>DurableRuntime: resolve path and runtime source
DurableRuntime-->>ServiceOrShim: return durable runtime metadata
ServiceOrShim->>BunProcess: set OCX_BUN_RUNTIME_SOURCE
BunProcess->>MemoryAPI: report bunRevision and runtime source
MemoryAPI->>Doctor: return validated diagnostics
Doctor->>Doctor: render source-aware guidance
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/winsw.ts (1)
77-99: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winResolve the Bun runtime once for WinSW XML generation.
buildWinswXml()emitsentry.bunbut writesOCX_BUN_RUNTIME_SOURCEfrom a separatedurableBunRuntime()call (src/lib/winsw.ts:76-77,99).defaultWinswEntry()also resolves the runtime independently throughdurableBunPath()(src/lib/winsw.ts:375-376). If the entries differ, WinSW launches one Bun executable while the service reports another runtime source, which can produce incorrectocx doctorguidance. Pass one resolvedDurableBunRuntimeobject through entry construction and XML generation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/winsw.ts` around lines 77 - 99, Update buildWinswXml(), defaultWinswEntry(), and their call path to resolve durableBunRuntime() once and pass the same DurableBunRuntime object through entry construction and XML generation. Use that shared object for both entry.bun and the OCX_BUN_RUNTIME_SOURCE environment value, replacing independent durableBunPath() and durableBunRuntime() resolutions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/winsw.ts`:
- Around line 77-99: Update buildWinswXml(), defaultWinswEntry(), and their call
path to resolve durableBunRuntime() once and pass the same DurableBunRuntime
object through entry construction and XML generation. Use that shared object for
both entry.bun and the OCX_BUN_RUNTIME_SOURCE environment value, replacing
independent durableBunPath() and durableBunRuntime() resolutions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f87a7015-0728-44c9-b890-60dcbae648a0
📒 Files selected for processing (12)
bin/ocx.mjssrc/cli/doctor.tssrc/lib/bun-runtime.tssrc/lib/winsw.tssrc/server/management/system-routes.tssrc/service.tsstructure/05_gui-and-management-api.mdtests/doctor.test.tstests/memory-watchdog.test.tstests/ocx-launcher-source.test.tstests/service.test.tstests/winsw.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bf4c76281
ℹ️ 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".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/codex-shim.test.ts (1)
61-72: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd explicit
overrideandbundledprovenance cases.These assertions verify only the default
"process"value. A builder that ignores a supplied non-defaultbunRuntimeSourcewould still pass.Call each Unix, CMD, and PowerShell builder with explicit
"override"and"bundled"values. Assert the generated environment assignment for each platform syntax.As per path instructions, “A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem.” The PR objectives also require bundled and override regression coverage.
Also applies to: 183-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/codex-shim.test.ts` around lines 61 - 72, Extend the existing Unix, Windows CMD, and PowerShell shim tests for their respective builder functions to call explicit "override" and "bundled" bunRuntimeSource values, asserting each generated script contains the platform-appropriate OCX_BUN_RUNTIME_SOURCE assignment. Keep the existing default "process" assertions and place focused coverage alongside the current codex shim tests.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/codex-shim.test.ts`:
- Around line 61-72: Extend the existing Unix, Windows CMD, and PowerShell shim
tests for their respective builder functions to call explicit "override" and
"bundled" bunRuntimeSource values, asserting each generated script contains the
platform-appropriate OCX_BUN_RUNTIME_SOURCE assignment. Keep the existing
default "process" assertions and place focused coverage alongside the current
codex shim tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cb3c37b0-030f-4181-9e77-df72e8ead7ee
📒 Files selected for processing (9)
docs-site/src/content/docs/troubleshooting/windows-memory.mdsrc/cli/doctor.tssrc/codex/shim.tssrc/lib/winsw.tssrc/tray/windows.tstests/codex-shim.test.tstests/doctor.test.tstests/windows-tray-runtime-source.test.tstests/winsw.test.ts
|
Follow-up review fixes are in
Local validation: 177 focused tests on Bun 1.4.0-canary.1 and bundled Bun 1.3.14, typecheck and privacy checks on both runtimes, plus a successful frozen docs install and production build. |
|
The follow-up test request is addressed in Validation: |
|
CI note for head The focused shim suite remains green on both Bun 1.4.0-canary.1 and bundled Bun 1.3.14 (34/34 on each), and typecheck passes. This appears to be a Bun runtime failure rather than a PR assertion regression; a repository maintainer can rerun the failed macOS job if desired. |
|
Thanks for carrying this through. The implementation now matches the #848 maintainer spec, and the review on our side is essentially done. The only remaining blocker is the macOS CI failure on the current head ( A similar macOS Bun crash pattern was recently addressed on We intend to merge promptly once CI is green. |
|
Diagnosis of the macOS failure on the current head: it is the known Bun-on-macOS isolate segfault, not a defect in this PR. Failing job: run 30710653575, |
f6d7d1c to
1f34a42
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/troubleshooting/windows-memory.md`:
- Around line 39-45: Synchronize the localized ocx doctor sections in the ja,
ko, ru, and zh-cn documentation with the English page by adding the
launcher-reported runtime source, its allowlisted override/bundled/process
values, unknown handling, and the conservative runtime gate for active
overrides. Preserve the existing OPENCODEX_BUN_PATH instructions and explicit
eager-relay opt-in text unchanged.
In `@src/cli/doctor.ts`:
- Around line 670-683: Add a dedicated bunRuntimeSource === "process" branch
within the Windows auto-known-bad handling before bundled-runtime guidance,
stating that the service uses process.execPath and recommending only an explicit
runtime override or reinstall with the desired runtime. Keep the existing
override and legacy-source branches unchanged, and add a regression test
covering the process-runtime message and ensuring bundled-runtime remediation is
omitted.
In `@src/server/management/system-routes.ts`:
- Line 82: Add focused regression coverage in tests/memory-watchdog.test.ts for
GET /api/system/memory, asserting runtime-source serialization for override,
bundled, and process values, while omitting invalid or unset environment
markers. Use the existing test setup and request helpers in that file, targeting
the endpoint response rather than client-side normalization.
In `@src/service.ts`:
- Around line 272-279: Add focused regression tests near the existing
service-generation tests for the launchd plist, Windows batch script, systemd
unit, and WinSW XML outputs. For each runtime source—override, bundled, and
process—assert that the generated `OCX_BUN_RUNTIME_SOURCE` entry is present and
paired with the selected Bun path, covering the generation paths around
`envLines` and the corresponding service writers.
- Around line 272-279: Update the service generators around cliEntry(),
durableBunRuntime(), and entry.bun so each generator resolves one
DurableBunRuntime and uses its executable path together with its source for
OCX_BUN_RUNTIME_SOURCE. Apply this consistently at the affected generator flows,
including the logic near the envLines construction, and update the service tests
to verify both values come from the same runtime object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 63663e40-8745-4cd9-913b-b7451b1e2143
📒 Files selected for processing (12)
bin/ocx.mjsdocs-site/src/content/docs/troubleshooting/windows-memory.mdsrc/cli/doctor.tssrc/codex/shim.tssrc/lib/bun-runtime.tssrc/lib/winsw.tssrc/server/management/system-routes.tssrc/service.tssrc/tray/windows.tsstructure/05_gui-and-management-api.mdtests/codex-shim.test.tstests/doctor.test.ts
| process's Bun version/revision, launcher-reported runtime source, RSS, | ||
| external/ArrayBuffers counters, JS-heap context, and stream-mode decision. | ||
| Source is allowlisted as `override`, `bundled`, or `process`; an older service | ||
| or invalid/missing marker is shown as unknown instead of being guessed from | ||
| the current shell. An active override remains unvalidated and keeps the | ||
| conservative runtime gate, but doctor no longer repeats the already-completed | ||
| `OPENCODEX_BUN_PATH` setup step. On the bundled Bun 1.3.14 runtime, `heapUsed` / |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
fd -i 'windows-memory.md' docs-site/src/content/docs | sort | while IFS= read -r file; do
echo "===== ${file} ====="
sed -n '28,55p' "$file"
doneRepository: lidge-jun/opencodex
Length of output: 9287
🏁 Script executed:
#!/bin/sh
set -eu
for file in \
docs-site/src/content/docs/troubleshooting/windows-memory.md \
docs-site/src/content/docs/ja/troubleshooting/windows-memory.md \
docs-site/src/content/docs/ko/troubleshooting/windows-memory.md \
docs-site/src/content/docs/ru/troubleshooting/windows-memory.md \
docs-site/src/content/docs/zh-cn/troubleshooting/windows-memory.md
do
echo "===== $file ($(wc -l < "$file") lines) ====="
rg -n -C 3 'ocx doctor|runtime source|launcher|OPENCODEX_BUN_PATH|streamMode|eager-relay|auto|override|unknown|service process|service.*Bun' "$file" || true
done
echo "===== relevant diff summary ====="
git diff --stat -- docs-site/src/content/docsRepository: lidge-jun/opencodex
Length of output: 15318
Synchronize the localized ocx doctor sections with the English page. The ja (lines 26–27), ko (25–27), ru (36–39), and zh-cn (25–27) pages omit the launcher-reported runtime source, its allowlisted values (override, bundled, process), unknown handling, and the conservative gate for active overrides. The OPENCODEX_BUN_PATH instructions and explicit eager-relay opt-in are otherwise consistent and should remain.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs-site/src/content/docs/troubleshooting/windows-memory.md` around lines 39
- 45, Synchronize the localized ocx doctor sections in the ja, ko, ru, and zh-cn
documentation with the English page by adding the launcher-reported runtime
source, its allowlisted override/bundled/process values, unknown handling, and
the conservative runtime gate for active overrides. Preserve the existing
OPENCODEX_BUN_PATH instructions and explicit eager-relay opt-in text unchanged.
Source: Path instructions
| // Runtime provenance is launcher-asserted and allowlisted. A missing marker is | ||
| // intentionally unknown; never infer an already-running service from this shell. | ||
| if (d.platform === "win32" && d.eagerRelay?.reason === "auto-known-bad") { | ||
| if (d.bunRuntimeSource === "override") { | ||
| lines.push(" OPENCODEX_BUN_PATH is already active, but this runtime remains unvalidated for automatic eager relay."); | ||
| lines.push(" The conservative auto-known-bad decision remains in effect; bunRevision is informational only."); | ||
| lines.push(" You can still opt into streamMode \"eager-relay\" via PUT /api/settings (crash risk on this runtime; see docs)."); | ||
| return lines; | ||
| } | ||
| if (d.bunRuntimeSource === undefined) { | ||
| lines.push(" runtime source was not reported by this legacy service/payload; an active override cannot be determined safely."); | ||
| lines.push(" Repair or reinstall the service to add provenance; the conservative auto-known-bad decision remains in effect."); | ||
| return lines; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not show bundled-runtime remediation for process source.
If bunRuntimeSource is process, this branch falls through to the bundled-runtime guidance at Lines 684-686. The durable resolver defines process as process.execPath, so waiting for a bundled Bun update does not replace the service executable.
Add a separate process branch. State that the service uses the process runtime. Offer only relevant remediation, such as installing an explicit override or reinstalling with a desired runtime. Add a regression test for this branch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/doctor.ts` around lines 670 - 683, Add a dedicated bunRuntimeSource
=== "process" branch within the Windows auto-known-bad handling before
bundled-runtime guidance, stating that the service uses process.execPath and
recommending only an explicit runtime override or reinstall with the desired
runtime. Keep the existing override and legacy-source branches unchanged, and
add a regression test covering the process-runtime message and ensuring
bundled-runtime remediation is omitted.
| pid: process.pid, | ||
| bunVersion: Bun.version, | ||
| bunRevision: Bun.revision, | ||
| bunRuntimeSource: reportedBunRuntimeSource(), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add endpoint coverage for runtime-source serialization.
tests/doctor.test.ts validates client-side normalization. It does not verify that GET /api/system/memory emits valid launcher provenance and omits missing or invalid markers. Add focused coverage in tests/memory-watchdog.test.ts for override, bundled, process, invalid input, and an unset environment variable.
As per path instructions, tests/**: “A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/management/system-routes.ts` at line 82, Add focused regression
coverage in tests/memory-watchdog.test.ts for GET /api/system/memory, asserting
runtime-source serialization for override, bundled, and process values, while
omitting invalid or unset environment markers. Use the existing test setup and
request helpers in that file, targeting the endpoint response rather than
client-side normalization.
Source: Path instructions
| const bunRuntime = durableBunRuntime(); | ||
| const log = logPath(); | ||
| const path = process.env.PATH ?? "/usr/local/bin:/usr/bin:/bin"; | ||
| const codexHome = process.env.CODEX_HOME?.trim(); | ||
| const opencodexHome = process.env.OPENCODEX_HOME?.trim(); | ||
| const envLines = [ | ||
| ` <key>OCX_SERVICE</key><string>1</string>`, | ||
| ` <key>${BUN_RUNTIME_SOURCE_ENV}</key><string>${bunRuntime.source}</string>`, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add regression tests for generated runtime-source environment entries.
The supplied test changes cover doctor output only. They do not verify the launchd plist, Windows batch script, systemd unit, or WinSW XML that persist OCX_BUN_RUNTIME_SOURCE. Add focused assertions for override, bundled, and process, including that each output pairs the source with the selected Bun path.
As per path instructions, tests/**: “A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem.”
Also applies to: 1037-1037, 1577-1584
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/service.ts` around lines 272 - 279, Add focused regression tests near the
existing service-generation tests for the launchd plist, Windows batch script,
systemd unit, and WinSW XML outputs. For each runtime source—override, bundled,
and process—assert that the generated `OCX_BUN_RUNTIME_SOURCE` entry is present
and paired with the selected Bun path, covering the generation paths around
`envLines` and the corresponding service writers.
Source: Path instructions
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
# Inspect the durable path helper and its paired runtime resolver.
fd '^bun-runtime\.ts$' src/lib -x sed -n '1,140p' {}
# Trace all generator definitions and callers, including explicit entries.
rg -nP -C5 '\b(cliEntry|buildPlist|buildUnit|buildWindowsServiceScript)\s*\(' src testsRepository: lidge-jun/opencodex
Length of output: 20197
🏁 Script executed:
#!/bin/sh
set -eu
printf '%s\n' '--- service imports and generator bodies ---'
sed -n '1,70p' src/service.ts
sed -n '270,315p' src/service.ts
sed -n '1020,1055p' src/service.ts
sed -n '1575,1615p' src/service.ts
printf '%s\n' '--- runtime helper call sites and explicit entry type ---'
rg -n -C4 'durableBun(Path|Runtime)|OCX_BUN_RUNTIME_SOURCE|buildWindowsServiceScript' src/service.ts tests/service.test.tsRepository: lidge-jun/opencodex
Length of output: 18440
🏁 Script executed:
#!/bin/sh
set -eu
python3 - <<'PY'
from pathlib import Path
import re
source = Path("src/service.ts").read_text()
for name, pattern in {
"buildPlist": r"export function buildPlist\(\): string \{(?P<body>.*?)\n\}",
"buildWindowsServiceScript": r"export function buildWindowsServiceScript\(.*?\): string \{(?P<body>.*?)\n\}",
"buildUnit": r"export function buildUnit\(\): string \{(?P<body>.*?)\n\}",
}.items():
body = re.search(pattern, source, re.S).group("body")
print(name)
print(" cliEntry calls:", len(re.findall(r"\bcliEntry\(\)", body)))
print(" durableBunRuntime calls:", len(re.findall(r"\bdurableBunRuntime\(\)", body)))
print(" executable source:", "entry.bun" if name == "buildWindowsServiceScript" else "cliEntry().bun")
print(" reported source:", "bunRuntime.source")
print()
calls = []
for path in Path(".").rglob("*"):
if path.is_file() and ".git" not in path.parts and path.suffix in {".ts", ".tsx", ".js", ".mjs"}:
text = path.read_text(errors="ignore")
if "buildWindowsServiceScript(" in text:
calls.extend(f"{path}:{line_no}:{line.strip()}"
for line_no, line in enumerate(text.splitlines(), 1)
if "buildWindowsServiceScript(" in line)
print("buildWindowsServiceScript call sites:")
print("\n".join(calls))
PY
printf '%s\n' '--- complete durable resolver definition ---'
sed -n '1,115p' src/lib/bun-runtime.tsRepository: lidge-jun/opencodex
Length of output: 4918
Pass one DurableBunRuntime through each service generator.
At src/service.ts:270-272 and src/service.ts:1575-1577, the generator gets the executable from cliEntry() and the source from a separate durableBunRuntime() call. At src/service.ts:1026-1028, entry.bun can be paired with an unrelated resolver result. Use one DurableBunRuntime object for both the executable path and OCX_BUN_RUNTIME_SOURCE, and update the service tests to assert that pairing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/service.ts` around lines 272 - 279, Update the service generators around
cliEntry(), durableBunRuntime(), and entry.bun so each generator resolves one
DurableBunRuntime and uses its executable path together with its source for
OCX_BUN_RUNTIME_SOURCE. Apply this consistently at the affected generator flows,
including the logic near the envLines construction, and update the service tests
to verify both values come from the same runtime object.
|
Rebased onto current dev at 48d59a8, which includes the merged #849 macOS isolate-worker fix.\n\nNew head: 1f34a42\n\nFresh validation is complete:\n- GitHub Actions: all checks pass, including macOS, Windows, Ubuntu, service lifecycle, and npm-global jobs.\n- Local focused provenance/launcher/service/doctor matrix: 160 passed on both Bun 1.4 canary and pinned Bun 1.3.14.\n- Typecheck, privacy scan, and diff check pass on the rebased head.\n\nThis should clear the rebase/CI follow-up requested in the latest maintainer note. |
2 participants
Fixes #848
Summary
override | bundled | processBun runtime provenance marker through the npm Node launcher, Windows Task Scheduler, native Windows WinSW, launchd, and systemd launch pathsbunRevisioninformational and leave the conservative eager-relay capability policy unchanged for canary and otherwise unknown buildsOPENCODEX_BUN_PATH” instruction when an explicit override is already activeCompatibility and safety
Validation
This incorporates the maintainer-requested corrections from the latest issue comment and is based on current
dev.Summary by CodeRabbit