Doctor: expose plugin registry findings#96169
Conversation
|
@clawsweeper review Please review PR #96169 at head |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs maintainer review before merge. Reviewed June 27, 2026, 12:05 PM ET / 16:05 UTC. Summary PR surface: Source +196, Tests +152. Total +348 across 4 files. Reproducibility: not applicable. this is a new structured Doctor diagnostic surface rather than a reported current-main bug. The PR body provides copied live CLI output for the explicit Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the default-disabled structured plugin-registry check after maintainers accept the Do we have a high-confidence way to reproduce the issue? Not applicable; this is a new structured Doctor diagnostic surface rather than a reported current-main bug. The PR body provides copied live CLI output for the explicit Is this the best way to solve the issue? Yes, subject to maintainer acceptance: the PR reuses existing plugin-registry detection and repair ownership, keeps the check default-disabled, and leaves real mutation in the legacy Doctor path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f857e8d66eb3. Label changesLabel justifications:
Evidence reviewedPR surface: Source +196, Tests +152. Total +348 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
@clawsweeper re-review Current head |
8d4e214 to
ffacddb
Compare
|
@clawsweeper re-review Current head Validation:
Source CLI proof refreshed for |
|
🦞🧹 I asked ClawSweeper to review this item again. |
galiniliev
left a comment
There was a problem hiding this comment.
This PR adds an opt-in structured doctor lint check for plugin registry state while leaving the existing doctor plugin-registry repair path as the only real mutation owner. That matters because doctor --lint --all and explicit --only core/doctor/plugin-registry can now surface stale registry/package state as structured JSON without changing default lint runs.
LOC: +349/-1 (4 files)
By: Gio Della-Libera (@giodl73-repo, acct 2025-09-30) | OpenClaw: 187 PRs, 14 issues, 138 commits/12mo | GitHub: 6437 commits, 221 PRs, 29 issues, 17 reviews/12mo
Findings: No blocking findings.
Affected surface: src/commands/doctor-plugin-registry.ts and src/flows/doctor-health-contributions.ts, plus focused tests for the structured issue mapping and contribution registration.
Behavior being changed: core/doctor/plugin-registry becomes a default-disabled health check. Default doctor --lint still skips it; --all or --only core/doctor/plugin-registry can now report warning findings and nonzero lint status for missing/stale registry state, stale managed npm bundled plugin shadows, stale local bundled install records, and broken managed npm OpenClaw peer links.
Best-fix verdict: best, subject to maintainer acceptance of the opt-in diagnostic surface. The PR reuses the existing plugin registry detection/repair helpers and keeps maybeRepairPluginRegistryState as the ordered legacy mutation owner, which is the right boundary for this slice.
Alternatives considered:
- Move real repair into the structured health check now: rejected because the existing ordered doctor contribution already owns registry migration, stale shadow cleanup, peer relinking, and registry refresh sequencing before final config writes.
- Make the check default-enabled: rejected because a missing registry can be expected in fresh/isolated state and would change default
doctor --lintbehavior. - Keep only legacy note output: rejected because that would not provide the selectable structured JSON lint surface this PR is adding.
Code read: src/commands/doctor-plugin-registry.ts, src/commands/doctor-plugin-registry.test.ts, src/flows/doctor-health-contributions.ts, src/flows/doctor-health-contributions.test.ts, src/flows/doctor-lint-flow.ts, src/flows/doctor-repair-flow.ts, src/flows/health-check-adapter.ts, src/flows/health-checks.ts, and src/commands/doctor-lint.ts.
Validation run:
node scripts/run-vitest.mjs run src/commands/doctor-plugin-registry.test.ts src/flows/doctor-health-contributions.test.ts --reporter=verbose(62 passed)pnpm exec oxfmt --check src/commands/doctor-plugin-registry.ts src/commands/doctor-plugin-registry.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.tsgit diff --check origin/main...HEAD- CLI smoke: isolated
OPENCLAW_STATE_DIR+ empty config,node scripts/run-node.mjs doctor --lint --json --only core/doctor/plugin-registryexited 1 with the expectedPersisted plugin registry is missing or stale.warning.
Provenance: N/A; this is a structured diagnostic feature slice, not a regression fix.
Remaining uncertainty: I did not run broad CI/Testbox. The PR is still draft, so merge should still wait for explicit maintainer acceptance of the opt-in --all / --only diagnostic behavior.
ffacddb to
9c0d0fb
Compare
|
@clawsweeper re-review Current head \9c0d0fbdd2bb5d86af099c48fc3dae96d92c98d8\ rebases #96169 onto current main after #95979 merged. The structured plugin-registry lint check remains opt-in (\defaultEnabled: false\): default \doctor --lint\ is unchanged, while explicit \--only core/doctor/plugin-registry\ and \--all\ include it. No SDK/plugin/config contract change. Validation refreshed:
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
9c0d0fb to
cf43999
Compare
|
@clawsweeper re-review Current head Contract remains unchanged: Focused validation on the new head:
Maintainer prepare local gate note: |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Merged via squash.
Thanks @giodl73-repo! |
Merged via squash. Prepared head SHA: cf43999 Co-authored-by: giodl73-repo <[email protected]> Co-authored-by: giodl73-repo <[email protected]> Reviewed-by: @giodl73-repo
Merged via squash. Prepared head SHA: cf43999 Co-authored-by: giodl73-repo <[email protected]> Co-authored-by: giodl73-repo <[email protected]> Reviewed-by: @giodl73-repo
Merged via squash. Prepared head SHA: cf43999 Co-authored-by: giodl73-repo <[email protected]> Co-authored-by: giodl73-repo <[email protected]> Reviewed-by: @giodl73-repo
Merged via squash. Prepared head SHA: cf43999 Co-authored-by: giodl73-repo <[email protected]> Co-authored-by: giodl73-repo <[email protected]> Reviewed-by: @giodl73-repo
Summary
core/doctor/plugin-registryfindings for missing/stale registry state, stale managed npm bundled plugin shadows, stale local bundled install records, and managed npm OpenClaw peer-link issuesContract
doctor --lintskips this check because it isdefaultEnabled: false.--only core/doctor/plugin-registryor--allcan surface the finding.Validation
node scripts/run-vitest.mjs run src/commands/doctor-plugin-registry.test.ts src/flows/doctor-health-contributions.test.ts --reporter=dot(64 passed)pnpm exec oxfmt --check src/commands/doctor-plugin-registry.ts src/commands/doctor-plugin-registry.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.tspnpm exec oxlint src/commands/doctor-plugin-registry.ts src/commands/doctor-plugin-registry.test.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.tspnpm tsgo:corenode scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.test.src.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/test-src.tsbuildinfonode scripts/plugin-sdk-surface-report.mjs --checkgit diff --check origin/main...HEADpnpm buildpassed;pnpm checkfailed only on the npm shrinkwrap guard, andpnpm deps:shrinkwrap:checkreproduces the same stalenpm-shrinkwrap.jsonfailure on latestorigin/main, outside this PR's touched surface.Source CLI Proof
cf4399955e2ddc8a38f5bb0f34bed4c6afc54749OPENCLAW_STATE_DIRand empty config{}.node scripts/run-node.mjs doctor --lint --json --only core/doctor/plugin-registry1,ok: false,checksRun: 1,checksSkipped: 28.{ "checkId": "core/doctor/plugin-registry", "severity": "warning", "message": "Persisted plugin registry is missing or stale.", "path": "<temp-state>/state/openclaw.sqlite", "fixHint": "Run `openclaw doctor --fix` to rebuild the plugin registry from enabled plugins." }No dry-run preview consumer is included in this slice; real mutation remains in the existing plugin registry doctor repair path.