[AI-assisted] fix(plugins): scope startup metadata manifest reads#84628
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed May 25, 2026, 4:54 AM ET / 08:54 UTC. Summary PR surface: Source +1001, Tests +1063. Total +2064 across 23 files. Reproducibility: yes. Source inspection shows current main has no scoped metadata-snapshot startup path, and the PR body gives a concrete before/after diagnostic path through loadPluginLookUpTable and metadata snapshot loading. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance: Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land the scoped startup metadata approach only after refreshed proof against the latest head and plugin/Gateway owner review confirm that every uncertain owner falls back to unscoped discovery. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main has no scoped metadata-snapshot startup path, and the PR body gives a concrete before/after diagnostic path through loadPluginLookUpTable and metadata snapshot loading. Is this the best way to solve the issue? Likely yes, but not merge-ready without refreshed proof and owner review. Carrying a conservative plugin-id scope from the installed index is maintainable if every uncertain ownership case falls back to unscoped discovery. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against c791e4242bc8. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1001, Tests +1063. Total +2064 across 23 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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 PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
abf0aeb to
6693cdc
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
6693cdc to
d464cb2
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper hatch |
|
🦞👀 I queued a comment sync for this PR. If the egg is hatchable, ClawSweeper will generate the image once and update the existing review comment. |
|
@clawsweeper ask history of this problem, risks and who should review it |
|
🦞👀 I queued a lightweight read-only assist pass. It will post a separate answer comment and will not edit the durable ClawSweeper review comment or trigger close, merge, repair, label, or branch changes. Request: history of this problem, risks and who should review it |
|
ClawSweeper assist: This PR is the proposed fix for #70533, where restrictive plugin configs still paid full startup manifest discovery cost. The history is: issue #70533 reported ~104 bundled manifests being read/validated at boot despite a small allowlist; this PR narrowed startup metadata reads, then went through several follow-up commits/re-reviews to handle snapshot reuse, bootstrap rescoping, and legacy persisted indexes. Main risk is compatibility: if the new pre-manifest scope misses a startup owner, a plugin that used to load under the unscoped path could be omitted. Evidence:
Who should review: someone who owns plugin startup/discovery and Gateway bootstrap behavior, plus a maintainer familiar with installed plugin index persistence/migrations. If available, include reviewers previously mentioned on this PR timeline, Suggested next action: Ask a plugin/Gateway maintainer to review the compatibility-sensitive startup scoping, especially config-path activation owners, channels/providers, legacy persisted indexes, and snapshot reuse behavior. Source: #84628 (comment) |
d38b0c9 to
7d4cbfd
Compare
e901898 to
38e10eb
Compare
Limit plugin metadata snapshots to the channel, provider, and startup surfaces that need them, while preserving unscoped fallback for incomplete index data and provider runtime resolution. Refs openclaw#84628 Co-authored-by: IWhatsskill <IWhatsskill@users.noreply.github.com>
Limit plugin metadata snapshots to the channel, provider, and startup surfaces that need them, while preserving unscoped fallback for incomplete index data and provider runtime resolution. Refs openclaw#84628 Co-authored-by: IWhatsskill <IWhatsskill@users.noreply.github.com>
38e10eb to
15d43f2
Compare
|
Behavior addressed: plugin metadata registry reads for startup/channel/provider paths are scoped to the needed plugin ids, with unscoped fallback when installed-index data is incomplete and provider-runtime reuse only when the scoped snapshot covers the requested provider. Real environment tested: local macOS checkout on PR head 15d43f2 after rebasing onto current origin/main. Exact steps or command run after this patch:
Evidence after fix: targeted Vitest shards passed 318 tests total; targeted oxlint passed; tsgo core and core-test passed; final autoreview reported no accepted/actionable findings. Observed result after fix: scoped metadata paths retain provider/channel/startup coverage and fall back safely when the installed index cannot prove a narrowed scope. What was not tested: full local pnpm check:changed was not run end-to-end in this checkout; the selected changed-lane proof above was run locally and PR CI is running on the pushed head. |
Summary
pluginIds; lookup/startup paths pass a conservativepluginIdScope; startup scoping carries indexedactivation.onConfigPathsowners such asbrowser; persisted installed indexes preserve thatstartup.configPathsmetadata; scoped current snapshots require an exact caller-requested scope before reuse; unscoped Gateway config-validation snapshots are not reused for restrictive startup bootstrap.Motivation
#70533 is a startup stability problem: a restrictive plugin setup should not crash-loop or pay full manifest reconstruction cost just because unrelated installed plugins exist. This keeps the startup path on the Installed Plugin Index when it can safely prove the manifest subset, and falls back to the old unscoped behavior when it cannot.
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Real behavior proof (required for external PRs)
origin/mainata13468320ccompared with this patch.loadPluginLookUpTable -> loadPluginMetadataSnapshot -> loadPluginManifestRegistryForInstalledIndex -> resolveGatewayStartupPluginPlanFromRegistrypath using valid 96-plugin Installed Plugin Index fixtures.{ "label": "after-rebased-patch-confirm", "indexPluginCount": 96, "manifestPluginCount": 1, "startupPluginIds": ["openai"], "loadedPluginIds": ["openai"], "diagnosticCount": 0 }["openai"].{ "config": { "browser": { "enabled": true }, "channels": {}, "plugins": { "allow": ["openai"], "slots": { "memory": "none" } } }, "results": [ { "label": "indexed-config-path-scope", "indexPluginCount": 96, "manifestPluginCount": 2, "startupPluginIds": ["openai", "browser"], "loadedPluginIds": ["browser", "openai"], "diagnosticCount": 0 }, { "label": "persisted-index-config-path-roundtrip", "indexPluginCount": 96, "manifestPluginCount": 2, "startupPluginIds": ["openai", "browser"], "loadedPluginIds": ["browser", "openai"], "persistedBrowserConfigPaths": ["browser"], "diagnosticCount": 0 }, { "label": "legacy-index-safe-fallback", "indexPluginCount": 96, "manifestPluginCount": 96, "startupPluginIds": ["openai", "browser"], "diagnosticCount": 0 } ] }browser.enabled=trueis preserved under a restrictiveplugins.allow=["openai"]; new and persisted indexes keepstartup.configPaths=["browser"]and stay scoped to 2 manifests, while legacy indexes without indexed config-path metadata fall back unscoped instead of losingbrowser.{ "path": [ "readConfigFileSnapshotWithPluginMetadata", "loadGatewayStartupConfigSnapshot", "prepareGatewayPluginBootstrap" ], "validationSnapshot": { "pluginIds": "unscoped", "manifestPluginCount": 96 }, "bootstrapManifestLoads": [["browser"], ["browser"], ["browser", "openai"]], "bootstrapLookup": { "pluginIds": ["browser", "openai"], "manifestPluginCount": 2 }, "startupPluginIds": ["openai", "browser"] }["browser","openai"]manifests and starts the expected plugins.{ "label": "before-rebased-origin-main", "indexPluginCount": 96, "manifestPluginCount": 96, "startupPluginIds": ["openai"], "diagnosticCount": 0 }Root Cause (if applicable)
Regression Test Plan (if applicable)
src/commands/status.summary.redaction.test.tssrc/gateway/pr84628-startup-snapshot-proof.temp.test.ts(temporary proof test, not committed)src/plugins/plugin-lookup-table.test.tssrc/plugins/installed-plugin-index-store.test.tssrc/plugins/channel-plugin-ids.test.tssrc/plugins/plugin-metadata-snapshot.memo.test.tssrc/plugins/current-plugin-metadata-snapshot.test.tsplugins.allowstartup config passes a narrow scope to installed-index manifest reconstruction; config-path activation owners such asbrowserstay included; conservative fallback remains unscoped for compat discovery, legacy indexes, or unmappable providers.User-visible / Behavior Changes
Gateway startup with a restrictive plugin allowlist can avoid reading unrelated installed plugin manifests when the startup scope is safe to prove from the Installed Plugin Index. Unsafe configs keep existing unscoped behavior.
Diagram (if applicable)
Security Impact (required)
Yes/No) NoYes/No) NoYes/No) NoYes/No) NoYes/No) NoYes, explain risk + mitigation: N/ARepro + Verification
Environment
plugins.allow=["openai"],plugins.slots.memory="none"; blocker regression also usesbrowser.enabled=trueSteps
origin/main, run the diagnostic fixture throughloadPluginLookUpTable.Expected
openai.browser.enabled=truestill selectsbrowsereven whenplugins.allowonly listsopenai.Actual
manifestPluginCount=96,startupPluginIds=["openai"].manifestPluginCount=1,loadedPluginIds=["openai"],startupPluginIds=["openai"].manifestPluginCount=2,loadedPluginIds=["browser","openai"],startupPluginIds=["openai","browser"]forbrowser.enabled=true.persistedBrowserConfigPaths=["browser"],manifestPluginCount=2, anddiagnosticCount=0.pluginIds="unscoped"withmanifestPluginCount=96, but bootstrap reloads scoped["browser","openai"]and lookup ends atmanifestPluginCount=2.Evidence
Validation:
Additional note:
origin/main @ a13468320cfailedtest/tsconfig/tsconfig.core.test.jsonbefore this patch becausestatus.summary.redaction.test.tshad not been updated for the newerSessionStatusfields. This PR includes that test-only fixture update socheck:changedcan pass cleanly on the rebased branch.Human Verification (required)
What I personally verified (not just CI), and how:
Review Conversations
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoRisks and Mitigations