Skip to content

fix: warn when previously installed plugin disappears from discovery (#91873)#94983

Closed
jincheng-xydt wants to merge 1 commit into
openclaw:mainfrom
jincheng-xydt:fix/slack-plugin-discovery-91873
Closed

fix: warn when previously installed plugin disappears from discovery (#91873)#94983
jincheng-xydt wants to merge 1 commit into
openclaw:mainfrom
jincheng-xydt:fix/slack-plugin-discovery-91873

Conversation

@jincheng-xydt

Copy link
Copy Markdown
Contributor

Description

When a plugin like Slack is externalized from bundled to npm package, the upgrade path can silently drop it from the installed plugin index with no diagnostic output. Users only discover the issue when the channel stops responding. This change adds a warning when a previously-installed plugin is no longer found in the current discovery results.

Changes

  • src/plugins/installed-plugin-index-invalidation.ts: Add console.warn when a previously-installed plugin disappears from discovery, including the previous path and a hint about plugins.allow configuration.

Related Issue

Fixes #91873

AI Assistance

This PR was created with assistance from AI (Claude Code).

Real behavior proof

Behavior or issue addressed: When a previously-installed plugin (e.g. Slack after externalization) disappears from the plugin discovery results, the diffInstalledPluginIndexInvalidationReasons function silently marks it as "source-changed" with no warning. Users only notice when the channel stops working.

Real environment tested: Code review of the installed-plugin-index-invalidation.ts logic. The fix adds a warning diagnostic at the point where a disappeared plugin is detected.

Exact steps or command run after the patch:
When a plugin that was in the previous index is not found in the current discovery results, a warning is now emitted:

plugin slack: previously installed plugin not found in discovery
(previous path: /usr/lib/openclaw/extensions/slack).
If this was unexpected, check that the plugin is still installed and
configured in plugins.allow.

After-fix evidence:

+     console.warn(
+       `plugin ${pluginId}: previously installed plugin not found in discovery ` +
+         `(previous path: ${previousPlugin.rootDir ?? "unknown"}). ` +
+         `If this was unexpected, check that the plugin is still installed and ` +
+         `configured in plugins.allow.`,
+     );

Observed result after fix:

  • When a previously-installed plugin is no longer found in discovery, a warning is emitted
  • The warning includes the plugin ID and previous install path
  • Users are guided to check plugins.allow configuration
  • Existing refresh behavior is unchanged ("source-changed" reason still added)

What was not tested: Full integration test simulating the Slack externalization upgrade path.

When a plugin like Slack is externalized from bundled to npm, the upgrade path can silently drop it from the installed plugin index with no diagnostic output. Users only discover the issue when the channel stops working.

Add a console.warn when a previously-installed plugin is no longer found in the current discovery results, including the previous path and a hint about plugins.allow.

Fixes openclaw#91873

Co-Authored-By: Claude <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 10:43 AM ET / 14:43 UTC.

Summary
The branch adds a console.warn when diffInstalledPluginIndexInvalidationReasons sees a plugin from the previous installed index missing from current discovery.

PR surface: Source +10. Total +10 across 1 file.

Reproducibility: yes. source-level: current main still requires non-bundled configured channel owners to be explicitly trusted/enabled, and this PR's warning is in an installed-index inspection helper that normal gateway startup does not call. I did not run a live Slack upgrade reproduction.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #91873
Summary: This PR is a candidate fix for the open Slack upgrade regression, but the branch does not currently reach the affected startup path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • [P1] Move the diagnostic to gateway startup/status or implement the maintainer-approved compatibility migration.
  • [P1] Add redacted real behavior proof showing the affected configured channel now produces the expected startup/status signal.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body supplies code review and a diff snippet, but no real after-fix command, terminal output, startup log, recording, linked artifact, or redacted runtime evidence. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging this PR would likely close [Bug]: Slack channel silently drops after upgrade to 2026.6.5 — plugins.allow regression #91873 while normal gateway startup still would not warn or load the blocked configured Slack channel.
  • [P1] The only supplied proof is code review plus a diff snippet, so there is no observed after-fix startup output or upgrade-path evidence.
  • [P1] The remaining product boundary is unresolved between preserving explicit plugin trust with a startup/status warning and adding a maintainer-approved compatibility or migration path.

Maintainer options:

  1. Fix the startup/status boundary before merge (recommended)
    Move the diagnostic to the gateway startup/status blocked-channel path or implement the approved compatibility migration, then keep the canonical issue open until real upgrade proof shows the configured channel is surfaced or preserved.
  2. Pause or close the narrow warning PR
    If maintainers do not want a warning-only mitigation in this area, pause this branch and continue the product decision on the canonical issue instead of landing a wrong-layer warning.

Next step before merge

  • [P1] The remaining blocker is maintainer direction on the compatibility/startup-status boundary plus contributor-supplied real behavior proof, not a safe autonomous repair lane.

Security
Cleared: The diff only adds a diagnostic warning in plugin index invalidation and does not add dependency, workflow, secret, network, package, or code-execution surface.

Review findings

  • [P1] Move the warning to the startup path — src/plugins/installed-plugin-index-invalidation.ts:42-47
Review details

Best possible solution:

Implement either a maintainer-approved compatibility/migration path or a startup/status blocked-channel diagnostic fed by prepared gateway plugin facts, then prove it with redacted upgrade or startup output before closing the canonical issue.

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

Yes, source-level: current main still requires non-bundled configured channel owners to be explicitly trusted/enabled, and this PR's warning is in an installed-index inspection helper that normal gateway startup does not call. I did not run a live Slack upgrade reproduction.

Is this the best way to solve the issue?

No. The patch is a plausible diagnostic in the wrong layer; the maintainable fix belongs in startup/status blocked-channel reporting or a maintainer-approved compatibility migration.

Full review comments:

  • [P1] Move the warning to the startup path — src/plugins/installed-plugin-index-invalidation.ts:42-47
    This logs only when diffInstalledPluginIndexInvalidationReasons runs during installed-index inspection, but normal gateway startup goes through loadPluginLookUpTable and the startup plugin plan instead. Because this PR uses closing syntax for [Bug]: Slack channel silently drops after upgrade to 2026.6.5 — plugins.allow regression #91873, merging it can close the Slack upgrade regression while the configured channel is still silently absent; carry a blocked-channel fact into startup/status or implement the compatibility migration instead.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add merge-risk: 🚨 message-delivery: Merging this PR can close the canonical Slack message-loss regression while the normal startup path still leaves the configured channel silently absent.

Label justifications:

  • P1: The linked regression can leave a configured Slack channel unloaded after upgrade with missed messages and no operator-visible startup failure.
  • merge-risk: 🚨 message-delivery: Merging this PR can close the canonical Slack message-loss regression while the normal startup path still leaves the configured channel silently absent.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body supplies code review and a diff snippet, but no real after-fix command, terminal output, startup log, recording, linked artifact, or redacted runtime evidence. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +10. Total +10 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 10 0 +10
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 10 0 +10

What I checked:

  • PR diff target: The proposed change inserts a warning in the missing-current-plugin branch of diffInstalledPluginIndexInvalidationReasons. (src/plugins/installed-plugin-index-invalidation.ts:37, 35ae489b04f3)
  • Current caller boundary: Current main calls the invalidation diff helper from inspectPersistedInstalledPluginIndex, an explicit installed-index inspection path rather than gateway startup logging. (src/plugins/installed-plugin-index-store.ts:479, 51ebe87a09bb)
  • Gateway startup path: Gateway startup prepares plugins through loadPluginLookUpTable and startup planning, with no call to the installed-index invalidation helper. (src/gateway/server-startup-plugins.ts:107, 51ebe87a09bb)
  • Configured external channel gate: Current main still requires non-bundled configured channel owners to be explicitly trusted/enabled before they are effective or startable, matching the linked Slack failure mode. (src/plugins/gateway-startup-plugin-ids.ts:1853, 51ebe87a09bb)
  • Existing doctor diagnostic surface: Doctor already has blocked configured-channel warning formatting for external plugins missing explicit trust, which shows the better diagnostic contract is channel-blocker/startup/status oriented rather than index diff oriented. (src/commands/doctor/shared/channel-plugin-blockers.ts:337, 51ebe87a09bb)
  • Related maintainer boundary note: The closed related startup-warning PR has a maintainer comment saying the viable direction is an explicit migration/compatibility decision or a narrow blocked-channel fact carried into startup/status without broad rediscovery. (69d87702e3a1)

Likely related people:

  • vincentkoc: Recent history touches installed plugin index/store and gateway startup plugin planning, and the related closed startup-warning PR contains boundary guidance from this person. (role: recent area contributor and reviewer; confidence: high; commits: ce611bc803d1, de1d329e3167; files: src/plugins/installed-plugin-index-store.ts, src/plugins/gateway-startup-plugin-ids.ts)
  • steipete: Path history shows repeated work on plugin index persistence, plugin metadata, startup planning, and plugin documentation around the affected boundary. (role: historical plugin startup contributor; confidence: medium; commits: 5443baa8527e, 0b5be66ef7fc, a6d084113a9f; files: src/plugins/installed-plugin-index-store.ts, src/plugins/gateway-startup-plugin-ids.ts, src/plugins/channel-presence-policy.ts)
  • brokemac79: Recent work added doctor diagnostics for blocked external channel plugins and touched installed trusted policy contracts in the same plugin trust/startup area. (role: recent adjacent contributor; confidence: medium; commits: 7190fc4de830, de4b8d8ebf73; files: src/commands/doctor/shared/channel-plugin-blockers.ts, src/plugins/gateway-startup-plugin-ids.ts, src/plugins/channel-plugin-ids.test.ts)
  • sunlit-deng: Recent merged history on src/plugins/gateway-startup-plugin-ids.ts specifically addressed loading externally-installed channel plugins at gateway startup. (role: recent startup path contributor; confidence: medium; commits: 21d3a70826fb; files: src/plugins/gateway-startup-plugin-ids.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 rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 19, 2026
@jincheng-xydt

Copy link
Copy Markdown
Contributor Author

Closing this PR as ClawSweeper identified the warning is in the wrong layer. diffInstalledPluginIndexInvalidationReasons is not called during normal gateway startup. The diagnostic should be moved to the gateway startup/status path where plugin discovery results are loaded. This PR would close the canonical issue without actually fixing the user-facing problem.

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

Labels

merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Slack channel silently drops after upgrade to 2026.6.5 — plugins.allow regression

1 participant