fix(cli): reject missing plugin ids before config writes by ai-hpc · Pull Request #73554 · openclaw/openclaw

ai-hpc · 2026-04-28T12:18:47Z

Summary

Problem: openclaw plugins enable <id> and plugins disable <id> accepted nonexistent plugin ids and could write stale plugins.entries.<id> config.
Why it matters: A typo could leave persistent junk config and repeated stale-plugin warnings until manual cleanup.
What changed: Resolve the requested plugin id against the discovered plugin registry before mutating config; missing ids now exit 1 without writing config.
What did NOT change (scope boundary): This does not change plugin discovery, install/uninstall behavior, Gateway plugin loading, or cleanup of stale entries already written by older versions.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes [Bug]: plugins enable <nonexistent-id> writes a stale plugin config entry and exits 0 #73551
This PR fixes a bug or regression

Root Cause (if applicable)

Root cause: The plugin policy mutation commands used the raw CLI argument directly for config writes before verifying that the id matched a discovered plugin.
Missing detection / guardrail: Existing policy tests covered successful enable/disable paths, but not nonexistent plugin ids or no-write behavior on failure.
Contributing context (if known): The stale-plugin warning path can identify missing config entries, but the enable/disable write path did not use registry knowledge as a gate.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: src/cli/plugins-cli.policy.test.ts
Scenario the test should lock in: plugins enable missing-plugin and plugins disable missing-plugin exit 1 and do not call config write or registry refresh seams.
Why this is the smallest reliable guardrail: The bug is in the CLI policy mutation handler, so a focused command test can assert the write seam is not reached.
Existing test that already covers this (if any): None.
If no new test is added, why not: N/A

User-visible / Behavior Changes

openclaw plugins enable <missing-id> and openclaw plugins disable <missing-id> now fail with a clear plugin-not-found error and do not mutate openclaw.json.

Diagram (if applicable)

Before:
plugins enable fake-id -> write plugins.entries.fake-id -> stale warning later

After:
plugins enable fake-id -> registry lookup fails -> exit 1, no config write

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (No)
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)
If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

OS: Ubuntu/Linux
Runtime/container: Node 22, pnpm dev checkout
Model/provider: N/A
Integration/channel (if any): Plugins CLI
Relevant config (redacted): plugins.entries policy config

Steps

Run openclaw plugins enable totally-fake-plugin-xyz.
Inspect openclaw.json.
Run openclaw plugins disable totally-fake-plugin-xyz.

Expected

Missing plugin ids fail before config mutation.
No stale plugins.entries.<id> entry is written.

Actual

Before this change, the enable path could write plugins.entries.<id> = { enabled: true } and exit 0.
After this change, missing ids exit 1 and the config write seam is not called.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Local verification:

pnpm test src/cli/plugins-cli.policy.test.ts -- --reporter=verbose
pnpm test src/cli/plugins-cli.policy.test.ts src/cli/plugins-cli.list.test.ts src/cli/plugins-cli.uninstall.test.ts src/cli/plugins-cli.install.test.ts src/cli/plugins-config.test.ts src/plugins/enable.test.ts -- --reporter=verbose
pnpm check:changed -- --base upstream/main
git diff --check

Human Verification (required)

Verified scenarios: Missing plugin ids for plugins enable and plugins disable reject before config writes; adjacent plugin CLI tests still pass.
Edge cases checked: Existing successful enable/disable policy paths still pass with discovered plugin ids. The registry snapshot includes disabled plugins, so valid disabled plugins remain enableable.
What you did not verify: Full packaged CLI invocation against a real user config.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps: N/A

Risks and Mitigations

Risk: Existing stale plugin entries written by older behavior are not automatically removed by this PR.
- Mitigation: This PR prevents new stale entries through enable/disable; cleanup of already-stale config remains a separate doctor/fix concern.

greptile-apps · 2026-04-28T12:20:55Z

Greptile Summary

This PR fixes a bug where plugins enable <id> and plugins disable <id> would write stale config entries for nonexistent plugin IDs. The fix adds a registry lookup via buildPluginRegistrySnapshotReport before any config mutation, and the new tests cover both the rejection path and the id-over-name priority case.

Confidence Score: 5/5

Safe to merge — fix is well-scoped and tests cover the critical paths.

Only a P2 test symmetry gap found; production logic is correct and the id-over-name priority is properly tested.

No files require special attention.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/cli/plugins-cli.policy.test.ts
Line: 119-132

Comment:
**Missing `setPluginEnabledInConfig` assertion in disable rejection test**

The `enable` rejection test explicitly asserts `expect(enablePluginInConfig).not.toHaveBeenCalled()`, but the symmetric `disable` rejection test doesn't assert that `setPluginEnabledInConfig` was not called. Adding this assertion would close the gap and make both tests structurally equivalent.

```suggestion
    expect(runtimeErrors).toContain(
      "Plugin not found: missing-plugin. Run `openclaw plugins list` to see installed plugins.",
    );
    expect(setPluginEnabledInConfig).not.toHaveBeenCalled();
    expect(writeConfigFile).not.toHaveBeenCalled();
    expect(refreshPluginRegistry).not.toHaveBeenCalled();
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (2): Last reviewed commit: "fix(cli): prefer exact plugin ids for po..." | Re-trigger Greptile}

clawsweeper · 2026-04-28T16:08:48Z

Codex review: needs changes before merge.

Summary
The PR adds pre-write registry validation for plugins enable and plugins disable, missing-id policy tests, selected compatibility alias tests, and a changelog entry.

Reproducibility: yes. Source inspection on current main shows both policy commands call config mutation and replaceConfigFile before any registry lookup, and the linked bug gives deterministic CLI steps; I did not run live commands in this read-only review.

Next step before merge
A narrow PR-branch repair can swap the guard to the registry-backed normalizer and add manifest-alias coverage.

Security
Cleared: Cleared: the diff is limited to CLI validation, tests, and changelog, with no dependency, workflow, package, secret, permission, or code-execution changes.

Review findings

[P2] Resolve registry aliases before rejecting plugin ids — src/cli/plugins-cli.ts:538-539

Review details

Best possible solution:

Use the registry-backed canonical plugin-id normalizer for policy command input, then reject only truly unknown ids before config mutation.

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

Yes. Source inspection on current main shows both policy commands call config mutation and replaceConfigFile before any registry lookup, and the linked bug gives deterministic CLI steps; I did not run live commands in this read-only review.

Is this the best way to solve the issue?

No, not yet. The pre-write validation boundary is right, but the PR should canonicalize through the manifest-aware registry alias contract instead of the static fallback.

Full review comments:

[P2] Resolve registry aliases before rejecting plugin ids — src/cli/plugins-cli.ts:538-539
The guard normalizes with normalizePluginId and then checks only plugin.id. That static fallback misses manifest contribution aliases such as byteplus-plan and stepfun-plan, so valid alias inputs that resolve through the registry normalizer can now fail with Plugin not found instead of writing the canonical plugin entry.
Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.88

Acceptance criteria:

pnpm test src/cli/plugins-cli.policy.test.ts src/plugins/plugin-registry.test.ts src/plugins/config-state.test.ts -- --reporter=verbose
pnpm exec oxfmt --check --threads=1 src/cli/plugins-cli.ts src/cli/plugins-cli.policy.test.ts CHANGELOG.md
git diff --check
pnpm check:changed -- --base openclaw/main

What I checked:

Current main enable path mutates before validation: plugins enable reads config, calls enablePluginInConfig(cfg, id), then writes the resulting config before any discovered-plugin lookup. (src/cli/plugins-cli.ts:111, 55c738ad4bed)
Current main disable path mutates before validation: plugins disable calls setPluginEnabledInConfig(cfg, id, false) and writes config before any discovered-plugin lookup. (src/cli/plugins-cli.ts:149, 55c738ad4bed)
Registry aliases are broader than static config-state aliases: createPluginRegistryIdNormalizer maps manifest providers, channels, setup providers, CLI backends, model catalog providers/aliases, and legacy ids to canonical plugin ids. (src/plugins/plugin-registry-id-normalizer.ts:22, 55c738ad4bed)
Concrete manifest alias exists: Bundled provider manifests still advertise aliases such as byteplus-plan, stepfun-plan, and volcengine-plan; these are not in normalizePluginId's static fallback list. (extensions/byteplus/openclaw.plugin.json:8, 55c738ad4bed)
PR guard uses static normalization plus exact id check: The PR normalizes with normalizePluginId(id) and rejects unless report.plugins contains that exact id, which misses manifest-driven aliases that resolve only through the registry normalizer. (src/cli/plugins-cli.ts:538, 38c7aef33274)
Changelog gap resolved: The current PR files include the required user-facing changelog entry for the CLI/plugins behavior change. (CHANGELOG.md:26, 38c7aef33274)

Likely related people:

vincentkoc: Prior same-cluster ClawSweeper history and PR discussion tie them to recent plugin registry, config normalization, and registry refresh work in the affected alias contract area. (role: plugin registry and config maintainer; confidence: high; commits: 74e7b8d47b18, 7308e72fac98, 95bc4179442e; files: src/plugins/config-state.ts, src/plugins/status.ts, src/plugins/plugin-registry-id-normalizer.ts)
hxy91819: They are assigned/requested on this PR, left the compatibility-id regression review, and authored the follow-up changelog commit on the branch. (role: reviewer and likely follow-up owner; confidence: medium; commits: 38c7aef33274; files: src/cli/plugins-cli.ts, src/cli/plugins-cli.policy.test.ts, CHANGELOG.md)
steipete: Prior same-cluster issue history identifies plugin config mutation, allowlist, and enable-path work adjacent to the affected CLI policy surface. (role: adjacent maintainer; confidence: medium; commits: 519517915045, 87603b5c45e5, 47216702f4f7; files: src/plugins/enable.ts, src/cli/plugins-cli.ts, src/config/plugins-allowlist.ts)

Remaining risk / open question:

The branch needs one more focused alias-normalization repair and regression test before it is safe to merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 55c738ad4bed.

ai-hpc · 2026-04-28T16:08:52Z

@greptile-apps

ai-hpc · 2026-04-28T19:11:19Z

Human verification update for daeda6e8f4b43a03e15a790f4d510f1afc419c7b:

Verified scenarios:

pnpm test src/cli/plugins-cli.policy.test.ts -- --reporter=verbose passed: 1 file, 8 tests.
pnpm check:changed passed. It selected lanes=all because the rebased fork branch history made the changed-lane detector conservative; completed checks included conflict markers, changelog attributions, guarded/plugin-sdk wildcard re-export guards, runtime sidecar loader guard, tsgo:all, oxlint shards, and runtime import cycles.
git diff --check upstream/main...HEAD passed.
GitHub currently reports mergeable: true, mergeable_state: clean.

Edge cases checked:

Exact plugin IDs still enable/disable and refresh the registry.
Nonexistent IDs exit 1 before config writes.
Display names are rejected for policy writes.
Exact ID wins when another plugin has the same display name.

What I did not verify:

This PR prevents new stale plugins.entries.<id> writes for plugins enable and plugins disable; I did not verify cleanup of stale entries already written by older versions.
I did not run a live Gateway scenario; this PR is covered by local CLI policy tests.
I did not test on macOS or Windows locally.

ai-hpc · 2026-04-29T03:55:08Z

Hello @vincentkoc
Could you review this PR please ?
Thanks very much.

hxy91819 · 2026-04-30T15:20:33Z

The missing-id guard is directionally right, but the exact-id check in src/cli/plugins-cli.ts:536 / src/cli/plugins-cli.ts:578 regresses current compatibility ids.

The repo still normalizes provider/channel/legacy ids such as openai-codex, google-gemini-cli, and minimax-portal-auth back to canonical plugin ids in src/plugins/config-state.ts:38 and src/plugins/plugin-registry.test.ts:293. Please resolve the user input through the existing alias-aware normalizer before rejecting it, and add command coverage that plugins enable|disable still accept those compatibility ids while still failing for truly unknown ids.

I do not think this is ready for /prepare-pr until that regression is fixed.

hxy91819

The missing-id guard is directionally right, but the exact-id check in src/cli/plugins-cli.ts:536 / src/cli/plugins-cli.ts:578 regresses current compatibility ids.

The bug this PR is trying to fix is real: plugins enable <missing-id> / plugins disable <missing-id> should fail before writing stale plugins.entries.<id> config. But I do not think this is the best fix as written, because it also rejects ids that current OpenClaw code and shipped manifests still treat as valid compatibility aliases.

Evidence:

src/plugins/config-state.ts:38 still normalizes compatibility ids such as openai-codex, google-gemini-cli, and minimax-portal-auth back to canonical plugin ids.
src/plugins/plugin-registry.test.ts:293 proves registry contribution aliases such as provider ids and channel ids normalize back to the owning plugin id.
src/plugins/config-state.test.ts:134 locks in legacy plugin-id normalization.
extensions/openai/openclaw.plugin.json:7, extensions/google/openclaw.plugin.json:7, and extensions/minimax/openclaw.plugin.json:7 still advertise those compatibility ids in current manifests.

Please resolve the user-supplied id through the existing alias-aware normalizer before rejecting it, then validate the canonical plugin id. Also add command coverage that plugins enable|disable still accept compatibility ids while still failing for truly unknown ids.

Given that regression, I do not think this is ready for /prepare-pr yet.

ai-hpc · 2026-04-30T15:39:58Z

Thanks, fixed in ee7433f52a.

The enable/disable guard now resolves the CLI argument through the existing normalizePluginId path before registry validation and mutation. That preserves compatibility ids such as openai-codex, google-gemini-cli, and minimax-portal-auth, while still rejecting truly unknown ids before any config write.

Added command coverage for both plugins enable and plugins disable accepting those compatibility ids and writing the canonical plugin entry.

Validation run:

pnpm test src/cli/plugins-cli.policy.test.ts src/plugins/config-state.test.ts src/plugins/plugin-registry.test.ts -- --reporter=verbose
pnpm check:changed --base openclaw/main
git diff --check

ai-hpc · 2026-05-02T14:11:31Z

Tested this PR locally.

Before/after regression check:

Before: parent commit b79e617ad1; a temporary regression test for plugins enable missing-plugin and plugins disable missing-plugin failed because both commands resolved instead of rejecting.
Before behavior was also observed directly: both missing-plugin paths reached writeConfigFile and refreshPluginRegistry.
After: PR commit ee7433f52a; the same regression test passed. Both commands reject with Plugin not found: missing-plugin..., and no config write, registry refresh, or enablePluginInConfig call occurs.

hxy91819 · 2026-05-03T03:49:22Z

Merged via squash.

Prepared head SHA: f0d3e61de26ae542d3626494d3a91a4f586a8f8a
Merge commit: 6a3f5d0b1f3536435f8242be0c92eafc9c3a60f6

Thanks @ai-hpc!

@hxy91819

) Merged via squash. Prepared head SHA: f0d3e61 Co-authored-by: ai-hpc <[email protected]> Co-authored-by: hxy91819 <[email protected]> Reviewed-by: @hxy91819

@hxy91819

) Merged via squash. Prepared head SHA: f0d3e61 Co-authored-by: ai-hpc <[email protected]> Co-authored-by: hxy91819 <[email protected]> Reviewed-by: @hxy91819

@hxy91819

) Merged via squash. Prepared head SHA: f0d3e61 Co-authored-by: ai-hpc <[email protected]> Co-authored-by: hxy91819 <[email protected]> Reviewed-by: @hxy91819

@hxy91819

) Merged via squash. Prepared head SHA: f0d3e61 Co-authored-by: ai-hpc <[email protected]> Co-authored-by: hxy91819 <[email protected]> Reviewed-by: @hxy91819

@hxy91819

) Merged via squash. Prepared head SHA: f0d3e61 Co-authored-by: ai-hpc <[email protected]> Co-authored-by: hxy91819 <[email protected]> Reviewed-by: @hxy91819

openclaw-barnacle Bot added cli CLI command changes size: S labels Apr 28, 2026

greptile-apps Bot reviewed Apr 28, 2026

View reviewed changes

Comment thread src/cli/plugins-cli.ts Outdated

clawsweeper Bot mentioned this pull request Apr 28, 2026

[Bug]: plugins enable <nonexistent-id> writes a stale plugin config entry and exits 0 #73551

Closed

ai-hpc force-pushed the fix/plugins-missing-id-policy branch from a4ccd5a to 0f43726 Compare April 28, 2026 16:04

ai-hpc force-pushed the fix/plugins-missing-id-policy branch 2 times, most recently from f762801 to daeda6e Compare April 29, 2026 00:41

clawsweeper Bot mentioned this pull request Apr 29, 2026

fix(cli): validate plugin ID before enabling #73932

Closed

clawsweeper Bot mentioned this pull request Apr 29, 2026

fix: reject unknown plugin ids in plugins enable/disable #74016

Closed

openclaw-barnacle Bot added size: M and removed size: S labels Apr 29, 2026

ai-hpc force-pushed the fix/plugins-missing-id-policy branch 2 times, most recently from 2591e5e to 6003f06 Compare April 29, 2026 12:17

openclaw-barnacle Bot added size: XS and removed size: M labels Apr 29, 2026

ai-hpc force-pushed the fix/plugins-missing-id-policy branch from 6003f06 to c5d9fb6 Compare April 29, 2026 13:26

openclaw-barnacle Bot added size: S and removed size: XS labels Apr 29, 2026

clawsweeper Bot mentioned this pull request Apr 30, 2026

fix: validate plugin id before writing config in plugins enable/disable #73623

Closed

hxy91819 self-requested a review April 30, 2026 01:23

clawsweeper Bot mentioned this pull request Apr 30, 2026

fix(plugins-cli): reject plugins enable/disable for unknown ids before mutating config #74902

Closed

hxy91819 self-assigned this Apr 30, 2026

hxy91819 requested changes Apr 30, 2026

View reviewed changes

ai-hpc requested a review from hxy91819 April 30, 2026 15:54

ai-hpc force-pushed the fix/plugins-missing-id-policy branch from ee5b670 to ee7433f Compare April 30, 2026 16:22

hxy91819 force-pushed the fix/plugins-missing-id-policy branch from 38c7aef to 04a3bf6 Compare May 3, 2026 03:45

ai-hpc and others added 3 commits May 3, 2026 11:47

fix(cli): reject missing plugin ids before config writes

09c5888

fix: add changelog entry for plugin id validation

0bc10bc

fix: add changelog entry for plugin id validation

f0d3e61

hxy91819 force-pushed the fix/plugins-missing-id-policy branch from 04a3bf6 to f0d3e61 Compare May 3, 2026 03:48

hxy91819 merged commit 6a3f5d0 into openclaw:main May 3, 2026
78 checks passed

github-actions Bot mentioned this pull request May 3, 2026

📡 Upstream Digest — 2026-05-03 06:22 UTC curtismercier/openclaw-mods#752

Open

clawsweeper Bot mentioned this pull request May 3, 2026

fix(cli): keep plugin toggles out of channel config #76525

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(cli): reject missing plugin ids before config writes#73554

fix(cli): reject missing plugin ids before config writes#73554
hxy91819 merged 3 commits into
openclaw:mainfrom
ai-hpc:fix/plugins-missing-id-policy

ai-hpc commented Apr 28, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

clawsweeper Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

ai-hpc commented Apr 28, 2026

Uh oh!

ai-hpc commented Apr 28, 2026 •

edited

Loading

Uh oh!

ai-hpc commented Apr 29, 2026

Uh oh!

hxy91819 commented Apr 30, 2026

Uh oh!

hxy91819 left a comment

Uh oh!

ai-hpc commented Apr 30, 2026 •

edited

Loading

Uh oh!

ai-hpc commented May 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

hxy91819 commented May 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

ai-hpc commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

ai-hpc commented Apr 28, 2026

Uh oh!

ai-hpc commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ai-hpc commented Apr 29, 2026

Uh oh!

hxy91819 commented Apr 30, 2026

Uh oh!

hxy91819 left a comment

Choose a reason for hiding this comment

Uh oh!

ai-hpc commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ai-hpc commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

hxy91819 commented May 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ai-hpc commented Apr 28, 2026 •

edited

Loading

greptile-apps Bot commented Apr 28, 2026 •

edited

Loading

clawsweeper Bot commented Apr 28, 2026 •

edited

Loading

ai-hpc commented Apr 28, 2026 •

edited

Loading

ai-hpc commented Apr 30, 2026 •

edited

Loading

ai-hpc commented May 2, 2026 •

edited

Loading