Skip to content

fix: preserve load-path plugins with installed index#100325

Closed
qingminglong wants to merge 4 commits into
openclaw:mainfrom
qingminglong:codex/99185-load-path-index
Closed

fix: preserve load-path plugins with installed index#100325
qingminglong wants to merge 4 commits into
openclaw:mainfrom
qingminglong:codex/99185-load-path-index

Conversation

@qingminglong

@qingminglong qingminglong commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #99185

What Problem This Solves

Fixes an issue where users with managed installed plugins would lose workspace plugins configured through plugins.load.paths when plugin metadata rebuilt from the installed-plugin index, causing stale plugin not found config warnings even though the workspace plugin files still existed.

Why This Change Was Made

The installed-index manifest registry reconstruction now also discovers the current explicit plugins.load.paths candidates and merges them with installed-index candidates. Scoped contribution and current-snapshot registry reuse now keep enabled load-path plugins that are present in the manifest registry even when they are not recorded in the installed index. The merge keeps the existing installed-index fast path while preserving plugin-id scoping, disabled-plugin filtering, and required-plugin diagnostics that know about already indexed plugins.

User Impact

Operators can keep managed npm plugins and explicit workspace load-path plugins enabled together. Workspace plugins configured in plugins.entries or plugins.allow no longer disappear from config validation or contribution owner resolution solely because a managed plugin index exists.

Evidence

Current head: a1af1391966e0076c9328a9f4631fa1d3c6926fd.

  • Claim proved: installed-index registry reconstruction includes explicit plugins.load.paths plugins; scoped contribution/current-snapshot registry paths keep enabled load-path plugins outside the installed index; scoped requirements and disabled configured plugins remain covered.
  • Source/path evidence:
    • src/plugins/manifest-registry-installed.test.ts now covers an installed managed-plugin index plus a configured load-path plugin and asserts both installed and configured are present with no diagnostics.
    • src/plugins/plugin-registry-contributions.current-snapshot.test.ts now covers current metadata reuse so a load-path plugin outside the installed index still contributes provider ownership.
    • src/plugins/manifest-registry-installed.ts merges installed-index candidates with discovered plugins.load.paths candidates before config validation sees the registry, which is the path that previously produced the stale plugin not found warning.
  • Commands / artifacts:
    • node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-pr100325.tsbuildinfo --pretty false - passed on this PR head.
    • node scripts/run-vitest.mjs src/plugins/manifest-registry-installed.test.ts src/plugins/plugin-registry-contributions.current-snapshot.test.ts src/commands/channel-setup/plugin-install.test.ts - passed on this PR head, 3 Vitest shards, 50 passed / 1 skipped.
    • GitHub CI run 28742921375 for this head: plugin-relevant lanes passed, including check-prod-types, check-lint, check-test-types, checks-fast-contracts-plugins-a, checks-fast-contracts-plugins-b, Critical Quality (plugin-boundary), and build-artifacts.
    • GitHub Real behavior proof run 28742991218 passed the external-PR proof/context gate for this PR.
    • git diff --check upstream/main...HEAD - passed locally in a clean PR worktree.
  • Not tested / proof gaps:
    • I did not run a live gateway restart against a real npm-installed @openclaw/llama-cpp-provider plus the reporter's workspace plugin set; the source-level installed-index path and contribution paths are covered by focused tests instead.
    • Local rerun of the focused Vitest command from this linked worktree was blocked because node_modules is absent; per repo policy I did not run pnpm install locally just to reconcile a Codex worktree.
    • Current CI has one failing unrelated gateway shard: checks-node-compact-small-5 failed in src/gateway/server.chat.gateway-server-chat-b.test.ts (chat.send does not recreate a session deleted while admission waits). The failure is outside this PR's changed plugin files and the single-job rerun was not permitted by GitHub.
    • Earlier autoreview was run on the previous patch head and found no accepted/actionable findings; no fresh autoreview was run after the final narrow follow-up commit.

@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 12:00 PM ET / 16:00 UTC.

Summary
The PR merges explicit plugins.load.paths candidates into installed-index manifest reconstruction and updates contribution/current-snapshot scoping so load-path plugins outside the installed index remain visible.

PR surface: Source +171, Tests +161. Total +332 across 6 files.

Reproducibility: yes. at source level: current main can build the installed-index manifest registry without current plugins.load.paths candidates, which leaves configured plugin ids missing and reaches the stale warning path. No live managed-install reproduction was run in this read-only review.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/plugins/manifest-registry-installed.test.ts, vector/embedding metadata: src/plugins/plugin-registry-contributions.current-snapshot.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99185
Summary: This PR is the candidate fix for the linked managed-plugin/install-index regression in load-path plugin discovery.

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: 🐚 platinum hermit
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 live terminal output or logs showing a managed npm plugin and a plugins.load.paths workspace plugin both resolve without stale warnings after restart.
  • Keep plugin-relevant checks green on the exact head after adding proof.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides focused tests and CI, but not a real after-fix run; add redacted terminal/log proof of the managed npm plugin plus load-path plugin startup/list path before merge, then update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.

Risk before merge

  • [P1] Real behavior proof is still tests/CI only; no redacted live run or log shows a managed npm plugin and a plugins.load.paths workspace plugin surviving the gateway restart/list path together.
  • [P1] The current status rollup includes an unrelated failing gateway shard, so exact-head merge still needs normal required-check gating even though plugin-relevant lanes passed.

Maintainer options:

  1. Decide the mitigation before merge
    Land this after a redacted live CLI or gateway run proves a managed npm plugin and a configured load-path workspace plugin are both present without stale-config warnings.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Manual PR follow-up only: the remaining blocker is contributor-supplied real behavior proof, not an automatable code repair.

Security
Cleared: No concrete security or supply-chain regression found; the patch reuses existing configured-path discovery and does not add dependencies, workflows, secrets handling, or new execution sources.

Review details

Best possible solution:

Land this after a redacted live CLI or gateway run proves a managed npm plugin and a configured load-path workspace plugin are both present without stale-config warnings.

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

Yes at source level: current main can build the installed-index manifest registry without current plugins.load.paths candidates, which leaves configured plugin ids missing and reaches the stale warning path. No live managed-install reproduction was run in this read-only review.

Is this the best way to solve the issue?

Yes for the code shape: merging configured load-path candidates into installed-index reconstruction preserves the fast path and fixes the owner-boundary issue without adding runtime shims. The merge gate should still require real behavior proof for the reported upgrade/startup scenario.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add 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 tests and CI, but not a real after-fix run; add redacted terminal/log proof of the managed npm plugin plus load-path plugin startup/list path before merge, then update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.

Label justifications:

  • P2: This fixes a bounded plugin discovery regression affecting mixed managed-plugin and plugins.load.paths setups, matching the linked P2 issue impact.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • 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 tests and CI, but not a real after-fix run; add redacted terminal/log proof of the managed npm plugin plus load-path plugin startup/list path before merge, then update the PR body to trigger re-review or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +171, Tests +161. Total +332 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 3 225 54 +171
Tests 3 165 4 +161
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 390 58 +332

What I checked:

Likely related people:

  • vincentkoc: Recent GitHub history and local blame point to installed manifest registry and plugin contribution helper work in the implicated files. (role: recent area contributor; confidence: high; commits: 94ae918d8f40, 8246e8dace5f, 9313471fa579; files: src/plugins/manifest-registry-installed.ts, src/plugins/plugin-registry-contributions.ts, src/plugins/plugin-registry-snapshot.ts)
  • steipete: GitHub history shows the persisted plugin install index and several plugin metadata hot-path/cache changes that this PR builds on. (role: introduced adjacent persisted-index behavior; confidence: high; commits: 5443baa8527e, 9a21e4e6c247, 86d7beab9933; files: src/plugins/plugin-metadata-snapshot.ts, src/plugins/plugin-registry-snapshot.ts, src/plugins/manifest-registry-installed.ts)
  • RomneyDa: Recent history shows metadata snapshot and discovery threading through gateway/plugin owner paths, adjacent to the contribution snapshot behavior reviewed here. (role: adjacent feature contributor; confidence: medium; commits: 8ccb11cbfca7; files: src/plugins/plugin-registry-snapshot.ts, src/plugins/plugin-metadata-snapshot.ts)
  • galiniliev: Recent history shows metadata cache behavior work in plugin-metadata-snapshot.ts, which is part of the same current-snapshot path. (role: recent adjacent contributor; confidence: medium; commits: ce6d97d580d8; files: src/plugins/plugin-metadata-snapshot.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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-05T13:13:43.808Z sha 3edbdbd :: needs real behavior proof before merge. :: [P2] Keep load-path plugins in scoped registry paths

@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. P2 Normal backlog priority with limited blast radius. labels Jul 5, 2026
@openclaw-barnacle openclaw-barnacle Bot added the commands Command implementations label Jul 5, 2026
@clawsweeper clawsweeper Bot removed 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. labels Jul 5, 2026
@qingminglong

qingminglong commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

PR body Evidence has been refreshed for current head a1af1391966e0076c9328a9f4631fa1d3c6926fd, including the installed-index plus plugins.load.paths source-path proof, CI/test evidence, and the current unrelated gateway shard gap.

@clawsweeper

clawsweeper Bot commented Jul 5, 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: 🧂 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. labels Jul 5, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 6, 2026
Merged
@vincentkoc

vincentkoc commented Jul 6, 2026

Copy link
Copy Markdown
Member

Superseded by #99196, which landed the canonical fix at 0f01930.

The landed change fixes this at the persisted plugin-index owner boundary: stale indexes are rejected and rebuilt when configured plugins.load.paths winners or precedence no longer match, so all registry consumers share one canonical index. That avoids carrying separate manifest/contribution filtering paths.

Thank you for the useful sibling-path analysis and regression coverage here; it helped verify the broader consumer surface before landing #99196.

@vincentkoc vincentkoc closed this Jul 6, 2026
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 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")

2 participants

@qingminglong @vincentkoc