fix(doctor): discover load-path plugin contracts by jalehman · Pull Request #77477 · openclaw/openclaw

jalehman · 2026-05-04T19:01:12Z

Summary

Fixes OpenClaw doctor so plugin doctor contract sidecars are discovered for explicit filesystem plugins loaded via plugins.load.paths.

This matters for plugins like Lossless Claw during local/checkout-based development: the runtime can load the plugin successfully, but openclaw doctor previously built its doctor-contract registry from the installed/persisted plugin registry without the active config, so load-path plugins were invisible to doctor.

What changed

Pass active config into plugin doctor contract discovery so plugins.load.paths plugins are included.
Pass active config through doctor warning collection.
Extend doctor --fix compatibility migration collection so configured plugin entries, not just channel ids, can contribute plugin doctor normalizers.
Add focused coverage for load-path plugin doctor sidecars and non-channel plugin fix paths.

Validation

pnpm exec oxfmt --check --threads=1 CHANGELOG.md src/plugins/doctor-contract-registry.ts src/plugins/doctor-contract-registry.load-paths.test.ts src/channels/plugins/legacy-config.ts src/commands/doctor/shared/legacy-config-issues.ts src/commands/doctor/shared/channel-legacy-config-migrate.ts src/commands/doctor/shared/channel-legacy-config-migrate.test.ts
# Passed

pnpm test src/plugins/doctor-contract-registry.test.ts src/plugins/doctor-contract-registry.load-paths.test.ts src/commands/doctor/shared/channel-legacy-config-migrate.test.ts -- --reporter=verbose
# Passed: 12 tests across 3 files

pnpm build
# Passed

Manual combined-branch validation with Lossless PR #600:

pnpm openclaw doctor now shows Lossless warnings for summaryModel and fallbackProviders requiring plugins.entries.lossless-claw.llm.allowModelOverride / allowedModels.
A temp-config doctor --fix run adds allowModelOverride: true and the configured allowedModels, without adding allowAgentIdOverride.

Real behavior proof

Behavior or issue addressed: openclaw doctor / openclaw doctor --fix now discovers doctor contract sidecars from explicit plugins.load.paths plugins. This lets load-path plugins such as lossless-claw surface legacy config warnings and lets doctor --fix apply plugin-owned normalizers before validation, instead of only considering unresolved channel ids.
Real environment tested: Josh’s local OpenClaw checkout on macOS, using the real OpenClaw CLI and real configured Lossless plugin entry. The destructive fix path was tested against a copied temp config so Josh’s real config was not modified.

Exact steps or command run after this patch:

pnpm openclaw doctor | rg -i "lossless|allowModelOverride|allowedModels|summaryModel|fallbackProviders"

OPENCLAW_CONFIG_PATH="$tmp/openclaw.json" pnpm openclaw doctor --fix --non-interactive

node -e '<inspect temp copied config>'

Evidence after fix:

Plain `pnpm openclaw doctor` reported Lossless warnings for `summaryModel` / `fallbackProviders`.

Temp copied-config `doctor --fix --non-interactive` applied the Lossless fix.

Inspecting the copied config after the fix showed:
- allowModelOverride: true
- allowedModels includes openai-codex/gpt-5.4-mini
- allowedModels includes minimax/MiniMax-M2.7
- allowAgentIdOverride absent

Observed result after fix: The load-path Lossless doctor contract was discovered from the configured plugin entry, read-only doctor showed the expected Lossless warnings, and doctor --fix --non-interactive updated the copied config with the expected Lossless llm policy.
What was not tested: The fix path was intentionally run against a copied temp config, not Josh’s real live config. Full local pnpm test later failed because vitest.unit-fast.config.ts hit the no-output watchdog twice; focused tests, pnpm build, and pnpm check passed.

clawsweeper · 2026-05-04T19:04:47Z

Codex review: needs changes before merge.

Summary
The PR passes active OpenClaw config into plugin doctor contract discovery, session owner discovery, and legacy repair paths; adds load-path doctor contract coverage; updates the changelog; and includes small lint/test/export cleanup.

Reproducibility: yes. at source level. Current main omits active config when resolving plugin doctor contracts while manifest discovery depends on config.plugins.load.paths, so a load-path-only plugin can be runtime-visible but doctor-invisible.

Real behavior proof
Sufficient (live_output): The PR body includes after-fix real CLI output for doctor and doctor --fix against a load-path Lossless plugin setup using a copied temp config.

Next step before merge
A narrow PR-branch repair can adjust fallback id selection and add the missing regression test; maintainer review still gates merge because of the protected label.

Security
Cleared: The diff expands existing doctor contract discovery to configured plugin load paths and adds tests/changelog, with no new dependency, CI, package, secret, or artifact-download surface.

Review findings

[P2] Exclude handled channels from plugin fallback — src/commands/doctor/shared/channel-legacy-config-migrate.ts:48-50

Review details

Best possible solution:

Keep the active-config pass-through, but compute plugin fallback ids from unresolved channels plus configured non-channel plugin entries and cover a handled bundled channel plus non-channel plugin regression.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level. Current main omits active config when resolving plugin doctor contracts while manifest discovery depends on config.plugins.load.paths, so a load-path-only plugin can be runtime-visible but doctor-invisible.

Is this the best way to solve the issue?

No for the current PR head. Passing active config through doctor discovery is the narrow maintainable fix, but fallback id selection should exclude bundled channels already handled by channel doctor normalizers.

Full review comments:

[P2] Exclude handled channels from plugin fallback — src/commands/doctor/shared/channel-legacy-config-migrate.ts:48-50
collectPluginDoctorCompatibilityIds() filters collectRelevantDoctorPluginIds() only against unresolved channel ids. That helper returns configured channels as well as plugins, so a config with a handled bundled channel such as slack plus a non-channel plugin entry sends slack back through applyPluginDoctorCompatibilityMigrations after the bundled normalizer already ran. Build the fallback set from unresolved channels plus configured non-channel plugin entries.
Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.87

Acceptance criteria:

pnpm exec oxfmt --check --threads=1 CHANGELOG.md src/plugins/doctor-contract-registry.ts src/plugins/doctor-contract-registry.load-paths.test.ts src/channels/plugins/legacy-config.ts src/channels/plugins/legacy-config.test.ts src/commands/doctor/shared/legacy-config-issues.ts src/commands/doctor/shared/channel-legacy-config-migrate.ts src/commands/doctor/shared/channel-legacy-config-migrate.test.ts src/commands/doctor-session-state-providers.ts src/plugins/plugin-registry-snapshot.test.ts
pnpm test src/plugins/doctor-contract-registry.test.ts src/plugins/doctor-contract-registry.load-paths.test.ts src/channels/plugins/legacy-config.test.ts src/commands/doctor/shared/channel-legacy-config-migrate.test.ts -- --reporter=verbose
pnpm build

What I checked:

Current-main doctor gap: Current main resolves plugin doctor contracts through loadPluginManifestRegistryForPluginRegistry without passing the active config, so plugins that exist only in plugins.load.paths are not included in this cold doctor path. (src/plugins/doctor-contract-registry.ts:267, aed96bb60c7c)
Manifest registry dependency: The manifest registry reads config.plugins and passes normalized load paths as discovery extraPaths, so callers need to supply config for plugins.load.paths discovery to work. (src/plugins/manifest-registry.ts:814, aed96bb60c7c)
Documented expected scope: Plugin architecture docs state that compatibility checks must include discovery roots such as plugins.load.paths and the default agent workspace. Public docs: docs/plugins/architecture.md. (docs/plugins/architecture.md:154, aed96bb60c7c)
PR implements the narrow direction: The PR adds config to resolvePluginDoctorContracts and passes it to loadPluginManifestRegistryForPluginRegistry; it also propagates config into warning, fix, and session-owner callers. (src/plugins/doctor-contract-registry.ts:266, 244a1381b397)
Remaining fallback bug: The PR's collectPluginDoctorCompatibilityIds filters collectRelevantDoctorPluginIds only against unresolved channel ids; collectRelevantDoctorPluginIds includes configured channel ids, so handled channels can be sent through plugin fallback after their bundled normalizer already ran. (src/commands/doctor/shared/channel-legacy-config-migrate.ts:48, 244a1381b397)
Bundled channel sidecar exists: Slack exposes a bundled doctor contract normalizer, demonstrating that a handled bundled channel id can re-enter the same doctor-contract path if it is included in plugin fallback ids. (extensions/slack/src/doctor-contract.ts:91, aed96bb60c7c)

Likely related people:

steipete: Recent commits on doctor-contract-registry and doctor session-state repair make this person a strong routing candidate for plugin doctor discovery and session repair behavior. (role: recent maintainer; confidence: high; commits: b17bb63b9eaa, f43a184103b6, 40ffada812b9; files: src/plugins/doctor-contract-registry.ts, src/commands/doctor-session-state-providers.ts)
vincentkoc: History shows this person on filtered doctor compatibility, bundled channel compat migration, and channel legacy rule fast paths that the PR modifies. (role: introduced and maintained doctor legacy config paths; confidence: high; commits: 7d54f2a3c2dd, 66e06b50bac3, 5c28cfbf09ee; files: src/commands/doctor/shared/channel-legacy-config-migrate.ts, src/channels/plugins/legacy-config.ts, src/plugins/doctor-contract-registry.ts)
gumadeiras: The lazy doctor plugin path work modified doctor-contract-registry shortly before this PR and is directly adjacent to the load-path discovery behavior. (role: adjacent owner; confidence: medium; commits: 66add9fcd96a; files: src/plugins/doctor-contract-registry.ts)

Remaining risk / open question:

The PR has the protected maintainer label, so merge and cleanup should remain maintainer-gated even after the narrow repair.
This read-only review did not execute the test suite; validation here is source inspection plus the PR body's focused, build, check, and live-output proof.
The branch includes unrelated lint/test/export cleanup outside the doctor surface, which maintainers may choose to split before merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against aed96bb60c7c.

jalehman · 2026-05-04T19:18:43Z

This was opened with the wrong base. Fixing.

jalehman · 2026-05-06T18:50:53Z

Merged via squash.

Prepared head SHA: d428fd47f5f0736204b3c02e093168e0154a653a
Merge commit: b22c8998ca87bf3187e55b6134f712ba0f5b12b8

Thanks @jalehman!

@jalehman