fix(cli): make doctor runtime-origin aware for OPENCODEX_BUN_PATH guidance (#848) - #898
fix(cli): make doctor runtime-origin aware for OPENCODEX_BUN_PATH guidance (#848)#898Yuxin-Qiao wants to merge 1 commit into
Conversation
…dance (lidge-jun#848) ocx doctor told Windows users to set OPENCODEX_BUN_PATH even when the service already runs on that override, making the guidance circular. - GET /api/system/memory now reports the service process's own runtime source (bundled/override/process) as a scalar enum; never the path. - doctor's Memory/runtime section shows the Bun revision and identifies an active override instead of re-suggesting the env var; bundled known-bad guidance is unchanged. - regression tests cover both bundled and override runtimes reporting the same version, plus endpoint-level override detection.
📝 WalkthroughWalkthroughThe memory endpoint now returns Bun runtime-source metadata. ChangesBun runtime diagnostics
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: be761e7925
ℹ️ 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".
| platform: process.platform, | ||
| // Scalar enum only, never the runtime path: lets diagnostics tell a | ||
| // bundled runtime from an OPENCODEX_BUN_PATH override (#848). | ||
| runtime: { source: durableBunRuntime().source }, |
There was a problem hiding this comment.
When a user follows the documented service workflow—sets OPENCODEX_BUN_PATH only in the install shell and reruns ocx service install—the Windows wrapper/WinSW definition bakes that path as the executable but does not propagate OPENCODEX_BUN_PATH into the service environment. Consequently, this call reevaluates the now-absent environment variable, reports bundled, and ocx doctor repeats the same circular override guidance that this change is intended to remove. Determine the source from process.execPath versus the bundled/override paths, or persist a non-sensitive source marker in the service definition; the new test currently masks the defect by setting the variable inside the already-running process.
AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/cli/doctor.ts`:
- Around line 526-527: Preserve the BunRuntimeSource contract by exporting and
reusing a union for bundled, override, and process values, and validate the
doctor response before storing it. Update formatServiceMemoryLines to handle all
three valid sources plus absent or invalid values so process is not treated as
bundled and unknown values remain safe. Add regression coverage for each valid
source and invalid input, and update the five Windows-memory pages if process
becomes user-visible.
🪄 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: 4bd63f1f-5c1e-48d3-85e2-6d7a8a05d3f9
📒 Files selected for processing (9)
docs-site/src/content/docs/ja/troubleshooting/windows-memory.mddocs-site/src/content/docs/ko/troubleshooting/windows-memory.mddocs-site/src/content/docs/ru/troubleshooting/windows-memory.mddocs-site/src/content/docs/troubleshooting/windows-memory.mddocs-site/src/content/docs/zh-cn/troubleshooting/windows-memory.mdsrc/cli/doctor.tssrc/server/management/system-routes.tstests/doctor.test.tstests/memory-watchdog.test.ts
| bunRevision?: string; | ||
| runtime?: { source: string }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 \
'durableBunRuntime|runtime.*source|source.*override|source.*process|OPENCODEX_BUN_PATH' \
src tests docs-siteRepository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- doctor outline ---'
ast-grep outline src/cli/doctor.ts --match 'ServiceMemoryData' --view expanded || true
printf '%s\n' '--- doctor type/fetch/format regions ---'
rg -n -C 12 'type ServiceMemoryData|interface ServiceMemoryData|fetchServiceMemory|formatServiceMemoryLines|runtime\.source|source ===' src/cli/doctor.ts
printf '%s\n' '--- runtime definition ---'
cat -n src/lib/bun-runtime.ts | sed -n '20,75p'
printf '%s\n' '--- doctor tests around runtime guidance ---'
cat -n tests/doctor.test.ts | sed -n '330,465p'
printf '%s\n' '--- all source and test references ---'
rg -n -C 3 'runtime:\s*\{[^}]*source|runtime\.source|source:\s*"(bundled|override|process)"|source === "(bundled|override|process)"' src testsRepository: lidge-jun/opencodex
Length of output: 39901
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- doctor imports and report types ---'
cat -n src/cli/doctor.ts | sed -n '1,45p'
cat -n src/cli/doctor.ts | sed -n '515,550p'
printf '%s\n' '--- durable runtime call sites and service artifact behavior ---'
rg -n -C 8 'durableBunRuntime\(\)|durableBunPath\(\)|buildWindowsServiceScript|process\.execPath' src tests
printf '%s\n' '--- language-page runtime wording ---'
for f 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
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
rg -n -C 2 'bundled runtime|bundled|OPENCODEX_BUN_PATH|运行时|ランタイム|рантайм|런타임' "$f" | head -80
else
printf '%s\n' "MISSING: $f"
fi
done
printf '%s\n' '--- standalone behavior verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
doctor = Path("src/cli/doctor.ts").read_text()
runtime = Path("src/lib/bun-runtime.ts").read_text()
assert 'runtime?: { source: string };' in doctor
assert 'typeof body.runtime.source === "string"' in doctor
assert 'd.runtime?.source === "override"' in doctor
assert '"process"' in runtime
def parsed_source(value):
# Mirrors fetchServiceMemory's present-string acceptance.
return value if isinstance(value, str) else None
def guidance_branch(source):
# Mirrors formatServiceMemoryLines's Windows auto-known-bad branch.
return "override" if source == "override" else "bundled-guidance"
for value in ("bundled", "override", "process", "future", 17, None):
accepted = parsed_source(value)
print(f"input={value!r} stored={accepted!r} branch={guidance_branch(accepted)!r}")
assert guidance_branch("process") == "bundled-guidance"
assert guidance_branch("future") == "bundled-guidance"
assert parsed_source("future") == "future"
print("deterministic behavior matches the source branches")
PYRepository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Windows memory documentation files and runtime statements ---'
find docs-site/src/content/docs -type f -path '*/troubleshooting/windows-memory.md' -print
for f in $(find docs-site/src/content/docs -type f -path '*/troubleshooting/windows-memory.md' | sort); do
printf '\n--- %s ---\n' "$f"
rg -n -C 1 'bundled runtime|bundled|OPENCODEX_BUN_PATH' "$f" | head -40
done
printf '%s\n' '--- service script runtime selection ---'
cat -n src/service.ts | sed -n '45,53p'
cat -n src/service.ts | sed -n '1261,1315p'
printf '%s\n' '--- standalone behavior verifier ---'
python3 - <<'PY'
from pathlib import Path
doctor = Path("src/cli/doctor.ts").read_text()
runtime = Path("src/lib/bun-runtime.ts").read_text()
assert 'runtime?: { source: string };' in doctor
assert 'typeof body.runtime.source === "string"' in doctor
assert 'd.runtime?.source === "override"' in doctor
assert 'source: "process"' in runtime
def fetch_stores(value):
return value if isinstance(value, str) else None
def format_branch(value):
stored = fetch_stores(value)
return "override branch" if stored == "override" else "non-override branch"
for value in ("bundled", "override", "process", "future", 17, None):
print(f"{value!r}: stored={fetch_stores(value)!r}, {format_branch(value)}")
assert fetch_stores("future") == "future"
assert format_branch("process") == "non-override branch"
assert format_branch("future") == "non-override branch"
print("source acceptance and formatting branches match the implementation")
PYRepository: lidge-jun/opencodex
Length of output: 11308
Preserve and validate the BunRuntimeSource contract.
src/lib/bun-runtime.ts:23-26,55-60 emits "bundled", "override", or "process". src/cli/doctor.ts:527,585 widens this value to string and accepts unknown strings. formatServiceMemoryLines handles only "override" at lines 633 and 666-670. Therefore "process" and unknown values omit the source label and show bundled-runtime guidance. "process" uses process.execPath when the bundled Bun binary is unavailable, so it is not necessarily bundled.
Export and reuse a BunRuntimeSource union. Validate the response before storing it. Handle "bundled", "override", "process", and absent or invalid values separately. Add regression tests for all three valid values and invalid input. Update the five Windows-memory pages if "process" becomes user-visible.
🤖 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 526 - 527, Preserve the BunRuntimeSource
contract by exporting and reusing a union for bundled, override, and process
values, and validate the doctor response before storing it. Update
formatServiceMemoryLines to handle all three valid sources plus absent or
invalid values so process is not treated as bundled and unknown values remain
safe. Add regression coverage for each valid source and invalid input, and
update the five Windows-memory pages if process becomes user-visible.
Source: Path instructions
|
Closing as a duplicate: the #848 fix already landed in #892 via the wt5 workstream. On codex/wt2-zero-leak-impl, doctor reads the launch-time provenance marker ( |
2 participants
Summary
ocx doctoron Windows told users to setOPENCODEX_BUN_PATHeven when the running service already uses that override, making the guidance circular (#848).GET /api/system/memorynow reports the service process's own runtime source (bundled/override/process) as a scalar enum, never the runtime path.Verification
bun run typecheck- passedbun scripts/test.ts tests/doctor.test.ts tests/memory-watchdog.test.ts- 41 pass / 0 fail, including the new [Bug]: doctor repeats OPENCODEX_BUN_PATH guidance when a Bun canary override is already active #848 regression testsbun scripts/test.ts- 6869 pass, 8 skip; the 16 unique failing tests reproduce on a pristineorigin/devcheckout with identical counts (environmental:bunmissing from PATH in the isolated test env, missinggui/node_modules, provider-validation baseline); none touch changed filesbun run privacy:scan- passedgit diff --check- cleanChecklist
Summary by CodeRabbit
New Features
ocx doctornow reports the Bun revision and whether the runtime is bundled or overridden throughOPENCODEX_BUN_PATH.Bug Fixes
Documentation