fix(doctor): report Bun runtime provenance - #861
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBun runtime selection now returns provenance labels. Launchers, services, shims, and the Windows tray pass those labels through ChangesBun 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
|
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. |
1f34a42 to
d061583
Compare
|
Superseded by #887, which lands this on current Your shape was the right one — one allowlisted
Your review feedback about pairing path and source from one The constraints from #848 are preserved: Closing in favor of #887. Thanks for the groundwork. |
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