Skip to content

fix(doctor): repair allow-only official plugins#77573

Merged
vincentkoc merged 1 commit into
mainfrom
fix/doctor-allow-only-official-77155
May 4, 2026
Merged

fix(doctor): repair allow-only official plugins#77573
vincentkoc merged 1 commit into
mainfrom
fix/doctor-allow-only-official-77155

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

Summary

  • Problem: the 2026.5.2 release doctor repair skipped plugins.allow ids that had no material plugins.entries entry, then stale cleanup could remove official external plugins such as lobster.
  • Why it matters: upgraded configs can silently lose allow-only official workflow/runtime plugins used by cron or ClawFlow jobs.
  • What changed: the release configured-plugin repair now includes allow-only ids only when they resolve through the official external plugin catalog.
  • What did NOT change: arbitrary/unofficial allow-only ids are still not treated as configured usage, and general allowlist semantics are unchanged.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

  • Root cause: collectReleaseConfiguredPluginIds collected material plugin entries and configured surfaces, but not allow-only official external plugin ids.
  • Missing detection / guardrail: the existing release collector test covered allow-only exclusion generally, but not the migration exception for official external catalog entries.
  • Contributing context: stale plugin cleanup can remove missing allow entries after the release install repair misses them.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/commands/doctor/shared/release-configured-plugin-installs.test.ts
  • Scenario the test should lock in: allow-only official external plugin ids are repair candidates; unofficial allow-only ids are skipped; ids with material entries do not need the allow-only catalog path.
  • Why this is the smallest reliable guardrail: it exercises the exact release collector before install repair and stale cleanup run.
  • Existing test that already covers this (if any): existing allow-only exclusion test remains in place for unofficial ids.
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

openclaw doctor --fix preserves configured allow-only official external plugins during the release repair path by installing them before stale cleanup can remove them.

Diagram (if applicable)

Before:
plugins.allow = ["lobster"] -> release repair ignores lobster -> stale cleanup may remove it

After:
plugins.allow = ["lobster"] -> official catalog match -> release repair installs lobster -> config survives

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: macOS local worktree; Blacksmith Testbox for changed gate
  • Runtime/container: Node/pnpm via repo scripts
  • Model/provider: N/A
  • Integration/channel (if any): doctor plugin migration repair
  • Relevant config (redacted): plugins.allow: ["lobster", "unofficial-custom"]

Steps

  1. Configure an allow-only official external plugin id such as lobster.
  2. Run the release configured-plugin repair collector.
  3. Verify lobster is included only because it is in the official external plugin catalog.
  4. Verify arbitrary allow-only ids are still excluded.

Expected

  • Official external allow-only ids are sent to missing-install repair.
  • Unofficial allow-only ids are not installed.
  • Material plugin entries keep their existing path.

Actual

  • Before this change, allow-only official external ids were skipped by the release collector.

Evidence

  • pnpm exec oxfmt --check --threads=1 CHANGELOG.md src/commands/doctor/shared/release-configured-plugin-installs.ts src/commands/doctor/shared/release-configured-plugin-installs.test.ts
  • pnpm test:serial src/commands/doctor/shared/release-configured-plugin-installs.test.ts — 12 passed
  • pnpm test:serial src/commands/doctor/shared/release-configured-plugin-installs.test.ts src/commands/doctor/shared/missing-configured-plugin-install.test.ts src/commands/doctor/shared/stale-plugin-config.test.ts — 62 passed
  • Testbox pnpm check:changed — passed on https://github.com/openclaw/openclaw/actions/runs/25348393861 before the final clean rebase; final rebase had no conflicts and the branch diff is materially unchanged
  • git diff --check after final rebase

Human Verification (required)

  • Confirmed the diff is limited to doctor release repair, its regression test, and changelog.
  • Confirmed the fix preserves general allowlist behavior for unofficial allow-only ids.

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: S labels May 4, 2026
@vincentkoc vincentkoc self-assigned this May 4, 2026
@openclaw-barnacle openclaw-barnacle Bot added the maintainer Maintainer-authored PR label May 4, 2026
@vincentkoc vincentkoc marked this pull request as ready for review May 4, 2026 23:09
@clawsweeper

clawsweeper Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
The PR adds official-external-catalog-gated plugins.allow collection to the doctor release configured-plugin repair, with regression tests and a changelog entry.

Reproducibility: yes. Source inspection on current main shows allow-only official plugin ids are omitted from the release repair set, while stale cleanup can remove missing plugins.allow ids; I did not run doctor because this review was read-only.

Next step before merge
No repair job is needed; this is an active maintainer-owned implementation with no discrete ClawSweeper-fixable finding, so the next action is normal review and merge gating.

Security
Cleared: The diff reuses the existing official external catalog and doctor install-repair path without adding new permissions, secrets handling, package sources, or unrelated code execution.

Review details

Best possible solution:

Land the narrow doctor repair after current required checks or a fresh changed gate are green, preserving passive allowlist behavior for unofficial ids.

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

Yes. Source inspection on current main shows allow-only official plugin ids are omitted from the release repair set, while stale cleanup can remove missing plugins.allow ids; I did not run doctor because this review was read-only.

Is this the best way to solve the issue?

Yes. The PR takes the narrow maintainable path by adding only official external catalog matches from plugins.allow to the release migration set while leaving arbitrary allow-only ids passive.

Acceptance criteria:

  • pnpm exec oxfmt --check --threads=1 CHANGELOG.md src/commands/doctor/shared/release-configured-plugin-installs.ts src/commands/doctor/shared/release-configured-plugin-installs.test.ts
  • pnpm test:serial src/commands/doctor/shared/release-configured-plugin-installs.test.ts src/commands/doctor/shared/missing-configured-plugin-install.test.ts src/commands/doctor/shared/stale-plugin-config.test.ts
  • pnpm check:changed

What I checked:

Likely related people:

  • steipete: GitHub file history shows Peter introduced the 2026.5.2 configured-plugin doctor repair in fix: repair configured plugin installs #76129 and has recent commits in the same release repair and stale cleanup paths. (role: introduced behavior and recent maintainer; confidence: high; commits: b63d098e8cee, 725754e50032, 75dd28e0838a; files: src/commands/doctor/shared/release-configured-plugin-installs.ts, src/commands/doctor/shared/missing-configured-plugin-install.ts, src/commands/doctor/shared/stale-plugin-config.ts)
  • vincentkoc: Current-main history has Vincent maintaining adjacent doctor/plugin allowlist, stale plugin, and install repair paths before this PR, including commits that keep plugin allowlists passive and update stale bundled install record handling. (role: recent adjacent maintainer; confidence: medium; commits: eeed33e61e51, 6b7f9eafed4b, 4e0e6f8ef324; files: src/commands/doctor/shared/missing-configured-plugin-install.ts, src/commands/doctor/shared/stale-plugin-config.ts, src/commands/doctor/shared/release-configured-plugin-installs.ts)

Remaining risk / open question:

  • The broad changed-gate proof cited in the PR body should be refreshed or replaced by current CI evidence because the referenced Testbox run URL currently reports cancelled via the Actions API.

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

@vincentkoc vincentkoc merged commit ae142ca into main May 4, 2026
127 of 128 checks passed
@vincentkoc vincentkoc deleted the fix/doctor-allow-only-official-77155 branch May 4, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands Command implementations maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doctor --fix removes allow-only externalized plugins (lobster) during v2026.5.2 one-time migration

1 participant