refactor(plugins): require initialized registry collections by RomneyDa · Pull Request #100990 · openclaw/openclaw

RomneyDa · 2026-07-06T15:46:32Z

What Problem This Solves

PluginRegistry marked 16 collection fields optional even though the canonical registry constructor always initializes them. That mismatch forced callers and tests to preserve unreachable fallback branches, duplicate large empty-registry literals, and use unsafe casts that obscured the actual loader invariant.

Why This Change Was Made

Make only the collections initialized by createEmptyPluginRegistry() required, then consume them directly throughout plugin registration, loader snapshots, host cleanup, and gateway policy paths. True PluginRegistry test fixtures now delegate to the canonical empty constructor; the distinct SDK hook-runner registry keeps its legitimately optional trusted-policy collection. Plugin lifecycle unions, provider APIs, SDK facades, gateway architecture, and runtime behavior remain out of scope.

User Impact

No user-visible behavior changes. Maintainers get one explicit registry construction contract, fewer defensive branches, and test fixtures that fail at compile time if the canonical registry shape changes.

Evidence

Blacksmith Testbox: tbx_01kww0q91z73mwy5j0aepqecsr (violet-shrimp)
Focused remote Vitest proof: 5 shards, 290 tests passed across plugin registry/runtime/loader, host-hook and session contracts, gateway node policy, tools catalog, and HTTP route capability tests.
Remote type proof: core production tsgo passed; core test tsgo passed.
Remote formatting/lint: oxfmt --check passed for all 30 changed files; scripts/run-oxlint.mjs passed.
Structured autoreview: .agents/skills/autoreview/scripts/autoreview --mode local --stream-engine-output reported no accepted/actionable findings (patch is correct, confidence 0.84).
Diff: 197 insertions, 409 deletions; no changelog.
Known unrelated proof gap: extension tsgo is currently blocked on the remote checkout by extensions/qa-lab/src/cli.ts failing to resolve ./mantis/cli.js; this PR changes no extension files.

RomneyDa · 2026-07-06T16:03:01Z

@clawsweeper review

clawsweeper · 2026-07-06T16:03:03Z

🦞🧹
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 · 2026-07-06T16:38:32Z

Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 12:45 PM ET / 16:45 UTC.

Summary
This PR makes initialized PluginRegistry collection fields required and removes now-unreachable fallback/default registry literals from plugin, gateway, and test-helper paths.

PR surface: Source -147, Tests -57. Total -204 across 30 files.

Reproducibility: not applicable. this is an internal refactor PR rather than a bug report. Source inspection confirms current main still marks initialized PluginRegistry collections optional while createEmptyPluginRegistry initializes them.

Review metrics: 1 noteworthy metric.

Registry contract normalization: 16 optional registry fields made required. This is the central review surface because it changes TypeScript expectations for internal plugin registry fixtures and fallback branches without changing user-facing config or runtime behavior.

Stored data model
Persistent data-model change detected: vector/embedding metadata: src/gateway/server-methods/tools-catalog.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🌊 off-meta tidepool
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Next step before merge

No repair job is needed; this maintainer-protected PR has no blocking findings and should go through normal maintainer review and merge sequencing.

Security
Cleared: Cleared: the diff changes internal TypeScript registry contracts and tests, with no workflow, dependency, lockfile, secret, credential, or artifact-execution changes.

Review details

Best possible solution:

Review and land this as the PluginRegistry slice of #100977 if maintainers are comfortable with the internal type-contract cleanup; keep the remaining issue slices tracked separately.

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

Not applicable: this is an internal refactor PR rather than a bug report. Source inspection confirms current main still marks initialized PluginRegistry collections optional while createEmptyPluginRegistry initializes them.

Is this the best way to solve the issue?

Yes: making constructor-owned registry collections required and updating runtime/test callers is the narrow maintainable fix for this slice. Preserving optional fallbacks would keep the duplicate contract the PR is trying to remove.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P3: This is a low-risk internal maintainability refactor with no reported user-facing behavior change.
add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🌊 off-meta tidepool and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The contributor real-behavior proof gate does not apply to this maintainer-authored internal refactor; the PR body still reports Testbox, type, lint, and autoreview proof.

Label justifications:

P3: This is a low-risk internal maintainability refactor with no reported user-facing behavior change.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🌊 off-meta tidepool and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The contributor real-behavior proof gate does not apply to this maintainer-authored internal refactor; the PR body still reports Testbox, type, lint, and autoreview proof.

Evidence reviewed

PR surface:

Source -147, Tests -57. Total -204 across 30 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	15	96	243	-147
Tests	15	103	160	-57
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	30	199	403	-204

What I checked:

Protected maintainer PR: Live PR metadata shows this is an open maintainer-labeled PR by RomneyDa, so conservative cleanup should not auto-close it and should route it through maintainer review. (38c1139308b1)
Current-main mismatch remains: Current main still marks fields such as cliBackends, coreGatewayMethodNames, hostedMediaResolvers, and the host-hook collections optional in PluginRegistry. (src/plugins/registry-types.ts:451, cc147c6a4060)
Canonical constructor initializes the collections: createEmptyPluginRegistry initializes the same collections to arrays, supporting the PR's invariant that real registries are fully initialized. (src/plugins/registry-empty.ts:4, cc147c6a4060)
PR head applies the intended contract: At the PR head, PluginRegistry makes the initialized collections required rather than optional. (src/plugins/registry-types.ts:451, 38c1139308b1)
Registry creation path: createPluginRegistry starts from createEmptyPluginRegistry before registering plugin contributions, so direct collection access matches the owner constructor path. (src/plugins/registry.ts:407, cc147c6a4060)
Validation evidence in PR body and checks: The PR body reports Blacksmith Testbox focused Vitest, core production/test tsgo, oxfmt, oxlint, and autoreview proof; live status checks for the head are clean, including prod/test type, lint, plugin contract, gateway-runtime-boundary, and security scans. (38c1139308b1)

Likely related people:

RomneyDa: RomneyDa authored the protected umbrella issue and this PR plus the sibling script-contract PR for the same Wave 1 cleanup, making him the clearest routing owner for this review cycle even though no prior current-main history was found in these files. (role: likely follow-up owner; confidence: high; commits: 6a5ab7224fd7, 38c1139308b1, 6f480b0f05ac; files: src/plugins/registry-types.ts, src/plugins/registry.ts, src/plugins/loader.ts)
vincentkoc: Git history ties the split registry type surface and later type-surface repair work to Vincent Koc, including the files central to this PR. (role: registry type-surface contributor; confidence: high; commits: c87994bc9abd, 74e7b8d47b18, 81535d394d34; files: src/plugins/registry-types.ts, src/plugins/registry-empty.ts, src/plugins/registry.ts)
steipete: History shows Peter Steinberger introduced the plugin architecture and shared empty-registry factory that this PR refines. (role: feature introducer and adjacent owner; confidence: medium; commits: cf0c72a557af, 3783cd385048, 44ec4d05de4a; files: src/plugins/registry.ts, src/plugins/registry-empty.ts, src/plugins/loader.ts)
Ayaan Zaidi: Recent plugin registration rollback commits touched the same registry/loader behavior surface that depends on initialized registry collections. (role: recent plugin registry contributor; confidence: medium; commits: e8fd148437ae, 59d07f0ab478, c95507978f00; files: src/plugins/registry.ts, src/plugins/loader.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.

…#100990) * refactor(plugins): require initialized registry collections * test(plugins): keep empty registry imports lightweight

refactor(plugins): require initialized registry collections

6a5ab72

openclaw-barnacle Bot added gateway Gateway runtime size: L maintainer Maintainer-authored PR labels Jul 6, 2026

test(plugins): keep empty registry imports lightweight

38c1139

clawsweeper Bot mentioned this pull request Jul 6, 2026

[Feature]: Consolidate Wave 1 internal type contracts #100977

Open

clawsweeper Bot mentioned this pull request Jul 6, 2026

refactor(test): consolidate script declaration contracts #100992

Merged

RomneyDa merged commit a45c099 into main Jul 6, 2026
119 of 123 checks passed

RomneyDa deleted the refactor/plugin-registry-required-collections branch July 6, 2026 17:25

clawsweeper Bot mentioned this pull request Jul 6, 2026

refactor(gateway): consolidate client contracts #101012

Merged

github-actions Bot mentioned this pull request Jul 6, 2026

📡 Upstream Digest — 2026-07-06 19:44 UTC curtismercier/openclaw-mods#1224

Open

This was referenced Jul 6, 2026

fix(tlon): bound Memex upload JSON response read #101115

Merged

Complete SNES Asset Studio upload-to-runtime workflow #101104

Closed

This was referenced Jul 7, 2026

fix: plugin tests fail on hosts with a usable /tmp/openclaw or a source checkout #101896

Merged

[Bug]: plugin temp-dir tests fail on any host where /tmp/openclaw is preferable (env-coupled os.tmpdir assertions) #101876

Closed

steipete mentioned this pull request Jul 12, 2026

Refactor plugin registry into domain registrars #105758

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refactor(plugins): require initialized registry collections#100990

refactor(plugins): require initialized registry collections#100990
RomneyDa merged 2 commits into
mainfrom
refactor/plugin-registry-required-collections

RomneyDa commented Jul 6, 2026

Uh oh!

RomneyDa commented Jul 6, 2026

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

RomneyDa commented Jul 6, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

RomneyDa commented Jul 6, 2026

Uh oh!

clawsweeper Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading