Skip to content

fix(cli/doctor-pi): skip unrelated local packages when probing embedding runtime - #414

Open
qsgy-edge wants to merge 2 commits into
cortexkit:masterfrom
qsgy-edge:fix/doctor-pi-embedding-candidate-filter
Open

fix(cli/doctor-pi): skip unrelated local packages when probing embedding runtime#414
qsgy-edge wants to merge 2 commits into
cortexkit:masterfrom
qsgy-edge:fix/doctor-pi-embedding-candidate-filter

Conversation

@qsgy-edge

@qsgy-edge qsgy-edge commented Sep 3, 2026

Copy link
Copy Markdown

Problem

piPluginDirCandidates treats every non-npm: entry in Pi settings.json packages[] as a candidate plugin tree. Local dev-path extensions — any directory with a package.json, regardless of what package it is — get probed for the embedding runtime.

On a machine with a local-path Pi extension registered (e.g. D:\repo\my-extension), doctor --harness pi reports:

WARN Embedding provider: local — native runtime and WASM fallback both unavailable —
native: package is not installed; WASM: could not resolve onnxruntime-web
(MODULE_NOT_FOUND: Cannot find module '@huggingface/transformers'
Require stack:
- D:\repo\my-extension\package.json)

The doctor blames an unrelated extension's package.json for missing embedding deps it was never expected to have, and stops at the first broken candidate — never probing the real magic-context install (~/.pi/agent/npm/node_modules/@cortexkit/pi-magic-context), which may be perfectly healthy.

Fix

Two small changes in packages/cli/src/commands/doctor-pi.ts:

  1. isPiMagicContextPackageDir: local dev-path entries only qualify as embedding-runtime candidates when their package.json name is @cortexkit/pi-magic-context. Unrelated extensions are no longer probed (this reuses the same name check findPiMagicContextCacheDirs already applies).
  2. Broken candidates no longer abort the scan: a broken/stale local tree is recorded but the loop keeps probing remaining candidates; only if no candidate passes does doctor report the warning. A stale local dev tree can no longer mask a healthy managed install.

Repro (before)

// ~/.pi/agent/settings.json
{ "packages": ["npm:@cortexkit/pi-magic-context", "D:\repo\my-extension"] }
npx @cortexkit/magic-context@latest doctor --harness pi
# WARN Embedding provider: local — ... both unavailable (blames my-extension)

After

npx @cortexkit/magic-context@latest doctor --harness pi
# PASS Embedding provider: local (native runtime selected and OK)

Verified against the reporter's real environment: doctor goes from PASS 11 / WARN 1 / FAIL 0 to PASS 12 / WARN 0 / FAIL 0.

Tests

New regression test: registers an unrelated local-path plugin plus a broken local dev tree of the plugin itself alongside a healthy managed install, asserts PASS Embedding provider: local (native runtime selected and OK) and no false both unavailable WARN.

bun test src/commands/doctor-pi.test.ts: 15 pass, 3 pre-existing fails (DB schema / legacy config migration on Windows; reproduced on clean master with git stash), 72 expect calls. tsc --noEmit clean. Biome flags only pre-existing CRLF line-ending noise in the file (reproduced on master).


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes doctor --harness pi so unrelated local-path packages are no longer probed for the embedding runtime, and broken or degraded candidates no longer abort the scan before a healthy managed install is checked.

Bug Fixes

  • Only directories whose package.json names @cortexkit/pi-magic-context qualify as embedding-runtime candidates.
  • Broken candidates and WASM fallbacks are recorded while the loop keeps probing; the WARN is only raised if no candidate passes.
  • A WASM-only dev tree no longer masks a later native-capable managed install.
  • Adds regression tests covering an unrelated local extension, a broken dev tree, and a WASM-only dev tree alongside a healthy managed install.

Written for commit 296e340. Summary will update on new commits.

Review in cubic

Greptile Summary

The PR makes Pi doctor inspect only local paths that declare the Magic Context Pi package and continue past degraded candidates until it finds the best available runtime.

  • Filters unrelated local extensions by their declared package name.
  • Prefers a native-capable installation over earlier broken or WASM-fallback trees.
  • Adds regression coverage for unrelated, broken, fallback-only, and healthy candidate combinations.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported WASM-fallback masking issue is fixed by continuing the scan until a native-capable candidate is found.

Important Files Changed

Filename Overview
packages/cli/src/commands/doctor-pi.ts Narrows local candidate discovery and changes runtime probing to retain degraded outcomes while searching for a healthy native installation.
packages/cli/src/commands/doctor-pi.test.ts Adds focused regression cases covering unrelated packages, stale plugin trees, WASM fallback precedence, and absent managed installations.

Reviews (2): Last reviewed commit: "fix(cli/doctor-pi): don't let a WASM fal..." | Re-trigger Greptile

…ing runtime

piPluginDirCandidates treated every non-npm: entry in Pi packages[] as a
candidate plugin tree, so local dev-path extensions (any package.json,
regardless of name) were probed for the embedding runtime. The first broken
candidate made doctor report 'native runtime and WASM fallback both
unavailable' and stop, even when the real magic-context install was healthy.

Now only directories whose package.json names @cortexkit/pi-magic-context
qualify as candidates, and broken candidates no longer abort the scan —
a stale local dev tree cannot mask a healthy managed install.

Repro: register any local-path Pi extension (D:\repo\my-extension) in
settings.json packages[], run 'doctor --harness pi' — doctor blamed the
extension's package.json for missing onnxruntime-web deps instead of
reporting the actual plugin install.
Comment thread packages/cli/src/commands/doctor-pi.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/cli/src/commands/doctor-pi.test.ts
Comment thread packages/cli/src/commands/doctor-pi.ts
…nstall

Address review findings (Greptile P1, cubic-dev-ai P2): the loop still
stopped at the first candidate with a working WASM fallback, reporting a
degraded runtime even when a later managed install had the native binding.
Record the best degraded candidate and keep probing; only report the
fallback WARN when no candidate is fully OK.

Tests: add regression coverage for (1) unrelated local packages never
probed (unverified, not a broken-runtime WARN, when only unrelated
packages are registered), and (2) a WASM-only dev tree not masking a
later native-capable install.
@qsgy-edge

Copy link
Copy Markdown
Author

Both review findings addressed in 296e340:

  1. WASM fallback no longer masks a native-capable install (Greptile P1 + cubic P2): the loop previously break-ed at the first candidate with a working WASM fallback, so an earlier WASM-only dev tree hid a later healthy managed install. Now the fallback result is recorded (firstFallback) and probing continues; the fallback WARN is only reported when no candidate is fully OK.

  2. Regression test actually detects the skip (cubic P2): the original test could not fail if isPiMagicContextPackageDir were reverted, because the healthy managed install masked the unrelated package. Added reports unverified, not a broken-runtime WARN, when only unrelated local packages are registered — with only an unrelated package registered and no magic-context tree, doctor must report selected runtime unverified rather than blaming the unrelated package for missing onnxruntime deps.

Also added prefers a later native-capable install over an earlier WASM fallback to lock in finding #1.

Test run: 20 tests, 17 pass, 3 pre-existing failures (DB schema / legacy config migration, reproduced on clean master). tsc --noEmit clean.

@magic-alfonso magic-alfonso Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating this — the premise holds (on master piPluginDirCandidates probes every local-path entry and aborts on the first broken one), the package-name filter is the right direction, and your regression is red on master. Three asks before merge:

  1. Pick the best result across all candidates, native OK > WASM OK > broken. The loop still stops on an earlier wasm-fallback, so an earlier local tree with a broken native binding but working WASM makes a later healthy native managed install report as degraded (greptile's P1 — I reproduced it with a variant of your test). Please add that exact regression: broken-native + working-WASM candidate first, healthy native candidate later, assert the native PASS.

  2. When every candidate fails, name every candidate. Only firstBroken survives into the warning today; the all-broken message should list each path with its native/WASM reason so the user can tell which tree is stale.

  3. Treat a local dev path as a second loaded identity. Pi keys npm:@cortexkit/pi-magic-context and a local directory whose package.json carries that name as different identities and loads both; the duplicate-load conflict check still only recognizes the npm spelling, so the doctor output should not imply the managed install is the sole runtime that will load. Please make the conflict diagnostic identity-aware and cover the local+npm case.

One formatter violation at :801-803 — run the repo's pinned biome (packages/cli/node_modules/.bin/biome check). On this Mac the CLI suite is 385/0 on your head; I could not verify the three Windows-only failures you mention, so if they are real please paste them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant