fix(doctor): report Bun runtime provenance - #861
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 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. |
1 participant
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