Skip to content

Expose legacy plugin manifest doctor lint findings#98695

Merged
giodl73-repo merged 4 commits into
openclaw:mainfrom
giodl73-repo:doctor-legacy-plugin-manifests-lint-findings
Jul 3, 2026
Merged

Expose legacy plugin manifest doctor lint findings#98695
giodl73-repo merged 4 commits into
openclaw:mainfrom
giodl73-repo:doctor-legacy-plugin-manifests-lint-findings

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • expose legacy plugin manifest contract migrations as default-disabled core/doctor/legacy-plugin-manifests lint findings
  • keep default doctor --lint behavior unchanged
  • leave real manifest rewriting in the existing legacy openclaw doctor --fix repair path; this PR is lint-only and does not add a structured repair/dry-run hook

Contract / compatibility notes

No public config, plugin manifest, SDK, persisted data-model, or repair-path contract changes. This PR only maps already-detected legacy manifest state into HealthFinding output for explicit lint selection / --all.

Real behavior proof

Source CLI proof with isolated state/config and a temp plugin manifest containing top-level tools showed:

  • default doctor --lint --json emitted zero core/doctor/legacy-plugin-manifests findings
  • doctor --lint --json --only core/doctor/legacy-plugin-manifests ran one selected check and reported the legacy manifest warning for legacy-plugin

Validation after Omar fix

  • node scripts/run-vitest.mjs src/commands/doctor-plugin-manifests.test.ts src/flows/doctor-health-contributions.test.ts (63 tests)
  • changed-file pnpm exec oxfmt --check
  • changed-file pnpm exec oxlint
  • node scripts/plugin-sdk-surface-report.mjs --check
  • git diff --check

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: S maintainer Maintainer-authored PR labels Jul 1, 2026
@giodl73-repo

giodl73-repo commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 2, 2026, 7:42 PM ET / 23:42 UTC.

Summary
The PR exposes legacy plugin manifest contract migrations as core/doctor/legacy-plugin-manifests structured lint findings without changing default lint behavior or the existing Doctor fix path.

PR surface: Source +32, Tests +112. Total +144 across 4 files.

Reproducibility: not applicable. as a bug reproduction; this PR adds a diagnostic lint capability. Source inspection shows current main has the legacy manifest detector and repair path but no structured lint check for it.

Review metrics: 1 noteworthy metric.

  • Doctor lint inventory: 1 default-disabled check added. Maintainers should notice that explicit doctor --lint --only/--all coverage expands while default lint behavior remains unchanged.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor-plugin-manifests.test.ts, migration/backfill/repair: src/commands/doctor-plugin-manifests.ts, migration/backfill/repair: src/flows/doctor-health-contributions.test.ts, migration/backfill/repair: src/flows/doctor-health-contributions.ts, serialized state: src/commands/doctor-plugin-manifests.ts, unknown-data-model-change: src/commands/doctor-plugin-manifests.ts, and 1 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Next step before merge

  • The protected maintainer label leaves the remaining action as human maintainer review or merge decision; no automated repair is needed.

Security
Cleared: The diff touches Doctor lint/test code only and does not add dependency, CI, credential, network, install, or new code-execution surface.

Review details

Best possible solution:

Land the narrow default-disabled lint exposure if maintainer review accepts the diagnostic surface, while keeping manifest rewriting owned by the existing Doctor repair flow.

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

Not applicable as a bug reproduction; this PR adds a diagnostic lint capability. Source inspection shows current main has the legacy manifest detector and repair path but no structured lint check for it.

Is this the best way to solve the issue?

Yes. Reusing the existing migration detector as a default-disabled lint check is the narrowest maintainable solution, and the latest head removed the earlier unreachable structured repair hook after review feedback.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d47308f8008c.

Label changes

Label justifications:

  • P3: This is a scoped Doctor diagnostics improvement with limited blast radius and no default runtime behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body reports after-fix CLI proof with isolated state and a temporary legacy manifest, showing default lint unchanged and explicit --only selection emitting the new finding.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports after-fix CLI proof with isolated state and a temporary legacy manifest, showing default lint unchanged and explicit --only selection emitting the new finding.
Evidence reviewed

PR surface:

Source +32, Tests +112. Total +144 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 37 5 +32
Tests 2 112 0 +112
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 149 5 +144

What I checked:

  • Repository policy applied: Root AGENTS.md was read fully; no scoped AGENTS.md owns src/commands or src/flows, and no matching maintainer notes were present. (AGENTS.md:1, d47308f8008c)
  • Current main lacks this structured lint exposure: Current main registers doctor:legacy-plugin-manifests with the legacy runLegacyPluginManifestHealth path only, so there is no structured health check id for lint selection yet. (src/flows/doctor-health-contributions.ts:1426, d47308f8008c)
  • Existing repair owner is unchanged: Current main already detects and rewrites legacy top-level plugin manifest capability keys through collectLegacyPluginManifestContractMigrations and maybeRepairLegacyPluginManifestContracts. (src/commands/doctor-plugin-manifests.ts:94, d47308f8008c)
  • PR head maps existing migrations to findings: The PR head adds legacyPluginManifestContractMigrationToHealthFinding, carrying the check id, warning severity, manifest path, target plugin id, requirement, and fix hint. (src/commands/doctor-plugin-manifests.ts:157, 2a019933bb38)
  • PR head keeps lint opt-in and repair legacy-owned: The PR registers core/doctor/legacy-plugin-manifests with defaultEnabled: false, maps the existing detector to lint findings, and still leaves run: runLegacyPluginManifestHealth in place for Doctor repair. (src/flows/doctor-health-contributions.ts:1340, 2a019933bb38)
  • Lint selection contract supports the approach: runDoctorLintChecks skips default-disabled checks unless --all or --only selects them, matching the PR's unchanged default lint behavior claim. (src/flows/doctor-lint-flow.ts:38, d47308f8008c)

Likely related people:

  • giodl73-repo: GitHub commit history shows several recent merged Doctor lint exposure changes in src/flows/doctor-health-contributions.ts and the Doctor lint framework, beyond this PR. (role: recent adjacent area contributor; confidence: high; commits: 285c629ab34c, b2c507c5f04c, cdaafe198cb7; files: src/flows/doctor-health-contributions.ts, src/flows/doctor-lint-flow.ts)
  • steipete: GitHub commit history shows prior work moving legacy manifest migration into Doctor plus later plugin-manifest repair and docs changes. (role: prior plugin-manifest repair contributor; confidence: medium; commits: 77d15841d781, 2f7c4070f4cc, 9d6e8b872ac3; files: src/commands/doctor-plugin-manifests.ts, src/commands/doctor-plugin-manifests.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 1, 2026
@giodl73-repo
giodl73-repo marked this pull request as ready for review July 1, 2026 16:34
@giodl73-repo
giodl73-repo force-pushed the doctor-legacy-plugin-manifests-lint-findings branch from fd45267 to 5ca9488 Compare July 2, 2026 00:45
@giodl73-repo

giodl73-repo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review current head 5ca9488. Added structured dry-run repair effects/diffs for the legacy plugin manifest lint finding and reran focused proof listed in the PR body.

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 2, 2026
@giodl73-repo
giodl73-repo force-pushed the doctor-legacy-plugin-manifests-lint-findings branch from 5ca9488 to fa9810e Compare July 2, 2026 01:27
@giodl73-repo

giodl73-repo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review current head fa9810e. Fixed hosted check-lint findings from the dry-run update and reran pnpm lint plus focused proof listed in the PR body.

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. proof: sufficient ClawSweeper judged the real behavior proof convincing. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 2, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jul 2, 2026
@omarshahine

omarshahine commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Codex review finding:

The new structured repair() hook is not on a real Doctor repair path. The contribution adds repairLegacyPluginManifestContractFindings() under core/doctor/legacy-plugin-manifests (src/flows/doctor-health-contributions.ts:1342-1365), but it still supplies run: runLegacyPluginManifestHealth (src/flows/doctor-health-contributions.ts:1368). createDoctorHealthContribution only delegates to runStructuredDoctorHealthContribution when run is omitted (src/flows/doctor-health-contributions.ts:144-151), so openclaw doctor --fix continues through the legacy manifest runner and never calls this new repair hook. The shared structured repair pass only includes extension checks (src/flows/doctor-health-contributions.ts:440-453, src/flows/health-check-registry.ts:29-40), and --only is lint-only in the CLI (src/cli/program/register.maintenance.ts:42-83), so the advertised selected dry-run repair preview is not reachable through the CLI.

Best fix: wire this core check into an actual repair entry point before claiming dry-run repair support, or keep the PR detect-only and remove the unreachable repair hook/tests/body claim.

@giodl73-repo

giodl73-repo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Omar's reachability finding by making this PR lint-only again.

What changed:

  • Removed the structured repair() hook from core/doctor/legacy-plugin-manifests because core structured repair selection is not currently reachable from the supported Doctor CLI.
  • Removed the unreachable dry-run helper/tests and kept maybeRepairLegacyPluginManifestContracts as the legacy doctor --fix owner.
  • The PR now only exposes default-disabled lint findings for legacy manifest contract keys.

Validation:

  • node scripts/run-vitest.mjs src/commands/doctor-plugin-manifests.test.ts src/flows/doctor-health-contributions.test.ts (63 tests)
  • changed-file oxfmt --check
  • changed-file oxlint
  • node scripts/plugin-sdk-surface-report.mjs --check
  • git diff --check

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. labels Jul 2, 2026
@giodl73-repo

giodl73-repo commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper review final lint-only wording/body synced after Omar fix

@giodl73-repo
giodl73-repo merged commit 9238d9a into openclaw:main Jul 3, 2026
119 of 123 checks passed
@giodl73-repo

giodl73-repo commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Merged via head-pinned squash.

Closeout:

  • PR head: 2a019933bb3876e730b9840b278b4ed66909da22
  • Merge commit: 9238d9aeae8d3965efac909b86d63cc90cfd6069
  • Scope: default-disabled core/doctor/legacy-plugin-manifests lint findings only. Real manifest rewrites remain owned by the existing legacy doctor --fix path; no structured dry-run repair hook in this PR.
  • Public contract: no SDK/plugin/config contract change; SDK surface check passed.

Validation:

  • Hosted PR checks green on the merged head.
  • node scripts/run-vitest.mjs src/commands/doctor-plugin-manifests.test.ts src/flows/doctor-health-contributions.test.ts (63 passed)
  • changed-file pnpm exec oxfmt --check
  • changed-file pnpm exec oxlint
  • node scripts/plugin-sdk-surface-report.mjs --check
  • git diff --check

Review notes:

  • Omar's dry-run reachability concern was addressed before merge by making the PR lint-only again.
  • Latest ClawSweeper review on 2a019933bb rated the PR rating: platinum hermit, proof: sufficient, status: ready for maintainer look, with no P1/P2 action required.

Maintainer wrapper note: local WSL is currently failing to start and Git Bash is missing wrapper dependencies, so this used the same branch-protection-compatible head-pinned squash path as #98400 after scoped local proof and green hosted checks.

@github-actions github-actions Bot mentioned this pull request Jul 3, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 3, 2026
@giodl73-repo
Expose legacy plugin manifest doctor lint findings (openclaw#98695)
b6423c5 
Expose default-disabled Doctor lint findings for legacy plugin manifest contract migrations.

- Maps existing legacy manifest contract diagnostics into structured HealthFinding output.
- Keeps real manifest rewriting in the existing legacy doctor --fix path; no structured dry-run repair hook in this PR.
- Default doctor --lint remains unchanged; explicit --only core/doctor/legacy-plugin-manifests and --all can select it.

Validation:
- Hosted PR checks green on head 2a01993.
- node scripts/run-vitest.mjs src/commands/doctor-plugin-manifests.test.ts src/flows/doctor-health-contributions.test.ts (63 passed)
- changed-file pnpm exec oxfmt --check
- changed-file pnpm exec oxlint
- node scripts/plugin-sdk-surface-report.mjs --check
- git diff --check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

commands Command implementations maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@giodl73-repo @omarshahine