Skip to content

fix: keep load-path plugins after managed plugin installs#99192

Closed
harjothkhara wants to merge 1 commit into
openclaw:mainfrom
harjothkhara:codex/99185-load-path-installed-index
Closed

fix: keep load-path plugins after managed plugin installs#99192
harjothkhara wants to merge 1 commit into
openclaw:mainfrom
harjothkhara:codex/99185-load-path-installed-index

Conversation

@harjothkhara

@harjothkhara harjothkhara commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #99185

AI-assisted: yes (Codex).

What Problem This Solves

Fixes an issue where users with workspace plugins configured through plugins.load.paths would see those plugins dropped as stale after installing and enabling a managed npm plugin.

With a persisted installed-plugin index present, config validation could rebuild manifest metadata only from the installed index rows. That left load-path workspace plugins out of knownIds, producing warnings such as plugin not found: <id> (stale config entry ignored; remove it from plugins config) even though the configured plugin files still existed on disk.

Why This Change Was Made

The installed-index manifest reconstruction path now includes the same configured-scope discovery candidates used by normal plugin discovery. This keeps explicit plugins.load.paths and workspace-scoped plugins visible while preserving the persisted-index fast path for installed/shared plugin metadata.

This does not change shared/global root scanning for the installed-index fast path, and scoped metadata requests still filter configured candidates to the requested plugin ids.

Duplicate-work check: exact PR searches for #99185 found no active fix PR. Related PR #96080 handles configured plugins under the global extensions root for #96046, and PR #89882 handles recovered install records for #89606; neither covers the plugins.load.paths workspace-plugin path reported here.

User Impact

Operators can install and enable managed npm plugins without causing existing load-path workspace plugins to disappear from gateway startup or CLI validation. Existing workspace plugin entries and allowlist entries remain valid when their manifests are still present.

Evidence

Focused proof run on this branch:

node scripts/run-vitest.mjs src/plugins/manifest-registry-installed.test.ts
# 1 file passed, 18 tests passed

node scripts/run-vitest.mjs src/plugins/plugin-metadata-snapshot.memo.test.ts
# 1 file passed, 33 tests passed

node scripts/run-vitest.mjs src/config/config.plugin-validation.test.ts
# 1 file passed, 59 tests passed

node scripts/run-vitest.mjs src/plugins/discovery.test.ts
# 1 file passed, 76 tests passed

node node_modules/oxfmt/bin/oxfmt --check src/plugins/discovery.ts src/plugins/manifest-registry-installed.ts src/plugins/manifest-registry-installed.test.ts src/config/config.plugin-validation.test.ts
# passed

git diff --check
# passed

Autoreview note: attempted .agents/skills/autoreview/scripts/autoreview --mode local, but this environment's approval policy rejected the external Codex review-engine run because it would export the local patch. Keeping this PR draft so maintainers can run or trigger the normal review path.

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 2, 2026, 2:24 PM ET / 18:24 UTC.

Summary
The PR proposes keeping plugins configured via plugins.load.paths visible when an installed-plugin index exists after managed plugin installs.

PR surface: Source +86, Tests +137. Total +223 across 4 files.

Reproducibility: unclear. the PR body describes a focused repro and tests, but I could not inspect current source or run read-only verification in this response context.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/plugins/discovery.ts, serialized state: src/config/config.plugin-validation.test.ts, vector/embedding metadata: src/config/config.plugin-validation.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted after-fix real behavior proof showing a managed npm plugin install while an existing plugins.load.paths workspace plugin remains visible after gateway or CLI validation.
  • Let a maintainer run the normal full review path once the draft is ready.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides focused test and formatter output, but no after-fix real setup proof for the contributor's plugin install/load-path workflow. 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] Repository source, PR diff, maintainer notes, scoped AGENTS.md, and git history were not inspectable from this response context.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the PR open for a full maintainer review against current plugin discovery and installed-index behavior.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The PR needs full source review and contributor real behavior proof before maintainers can decide whether to land it.

Security
Cleared: No concrete security or supply-chain concern is apparent from the provided diff summary, but this should be rechecked during full source review.

Review details

Best possible solution:

Keep the PR open for a full maintainer review against current plugin discovery and installed-index behavior.

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

Unclear: the PR body describes a focused repro and tests, but I could not inspect current source or run read-only verification in this response context.

Is this the best way to solve the issue?

Unclear: the proposed path may be appropriate, but it needs review against current installed-index discovery, configured load paths, and sibling registry refresh fixes.

AGENTS.md: unclear because the file could not be read completely.

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

Label changes

Label justifications:

  • P2: The PR targets a normal plugin discovery regression with limited but real operator impact.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides focused test and formatter output, but no after-fix real setup proof for the contributor's plugin install/load-path workflow. 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 +86, Tests +137. Total +223 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 146 60 +86
Tests 2 137 0 +137
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 283 60 +223

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/plugins/manifest-registry-installed.test.ts.
  • [P1] node scripts/run-vitest.mjs src/plugins/plugin-metadata-snapshot.memo.test.ts.
  • [P1] node scripts/run-vitest.mjs src/config/config.plugin-validation.test.ts.
  • [P1] node scripts/run-vitest.mjs src/plugins/discovery.test.ts.
  • [P1] git diff --check.

Likely related people:

  • unknown: No repository history could be inspected in this response context. (role: likely follow-up owner; confidence: low; files: src/plugins/manifest-registry-installed.ts, src/plugins/discovery.ts, src/config/config.plugin-validation.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 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. P2 Normal backlog priority with limited blast radius. labels Jul 2, 2026
This was referenced Jul 2, 2026
Merged
Closed
@harjothkhara

harjothkhara commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of #99196, which fixes the same #99185 root cause and is further along.

Both PRs land the fix in the same place — loadPluginManifestRegistryForInstalledIndex now discovers plugins.load.paths candidates in addition to index rows. #99196 keeps the discovery scoped to explicit load.paths (matching what #99185 reports), whereas mine also re-scanned the workspace extensions root, which adds blast radius on a compatibility-sensitive path without being needed for this bug. #99196 is non-draft, rated proof: sufficient / ready-for-maintainer with green CI; mine is still draft and needs proof.

I left a note on #99196 offering my validateConfigObjectWithPlugins-level regression test as added coverage. No competing PR needed here.

@clawsweeper clawsweeper Bot mentioned this pull request Jul 22, 2026
Open
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

P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Installing a managed npm plugin breaks resolution of all plugins.load.paths workspace plugins ("plugin not found: stale config entry ignored")

1 participant

@harjothkhara