Skip to content

fix(cli): reject unowned command roots before plugin load#76379

Merged
steipete merged 1 commit intoopenclaw:mainfrom
neilofneils404:fix-unowned-cli-command-runtime-load
May 3, 2026
Merged

fix(cli): reject unowned command roots before plugin load#76379
steipete merged 1 commit intoopenclaw:mainfrom
neilofneils404:fix-unowned-cli-command-runtime-load

Conversation

@neilofneils404
Copy link
Copy Markdown
Contributor

Summary

  • gate network/proxy startup for unknown CLI roots on built-in catalog or manifest command ownership
  • stop plugin CLI lazy loading from falling back to all plugins when a supplied primary has no manifest owner
  • add regression coverage and a changelog note for openclaw foo

Fixes #75287.

Tests

  • pnpm test src/cli/run-main.test.ts src/cli/run-main.exit.test.ts src/cli/command-registration-policy.test.ts src/cli/command-path-policy.test.ts src/plugins/cli.test.ts
  • pnpm tsgo:core && pnpm tsgo:core:test
  • pnpm check:changed
  • live smoke: node scripts/run-node.mjs foo exits 1 with the unknown-command error and no surviving child processes

@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes size: S labels May 3, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 3, 2026

Codex review: needs maintainer review before merge.

Summary
The PR adds an unknown CLI-root gate before managed proxy startup, scopes plugin CLI loading by manifest or CLI metadata ownership, adds QA Lab CLI metadata, updates regression tests, and adds a changelog entry for the linked CLI bug.

Reproducibility: yes. at source level. The linked issue has concrete openclaw foo hot-child logs, and current main still shows unknown roots entering proxy startup plus broad plugin CLI loading; I did not run a live smoke in this read-only review.

Next step before merge
No repair lane is needed because the current PR has no actionable review findings and should proceed through ordinary exact-head checks and maintainer review.

Security
Cleared: The diff narrows CLI/plugin loading and touches tests, changelog, and a metadata entry; it does not add dependencies, workflow changes, secrets handling, package resolution changes, or new third-party code execution sources.

Review details

Best possible solution:

Land this PR or an equivalent narrow fix after exact-head validation confirms openclaw foo exits promptly without proxy/full plugin runtime loading and metadata-owned plugin commands still route correctly.

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

Yes, at source level. The linked issue has concrete openclaw foo hot-child logs, and current main still shows unknown roots entering proxy startup plus broad plugin CLI loading; I did not run a live smoke in this read-only review.

Is this the best way to solve the issue?

Yes. The latest diff is the narrow maintainable direction: validate builtin or plugin CLI root ownership through manifest and CLI metadata before managed proxy startup, then avoid full runtime plugin loading for truly unowned roots.

What I checked:

  • Current main failure path: Current main starts the managed proxy before command routing whenever shouldStartProxyForCli(normalizedArgv) is true, so an unknown root can start proxy work before failing. (src/cli/run-main.ts:347, 4795f3474ad5)
  • Current main broad plugin fallback: Current main scopes onlyPluginIds only when manifest activation planning finds an owner; no match falls through to loadOpenClawPlugins without an owner scope. (src/plugins/cli-registry-loader.ts:112, 4795f3474ad5)
  • Plugin CLI contract preserved: Docs define both commands and descriptors as top-level CLI metadata, with descriptors supporting root help/routing/lazy registration and commands remaining an eager compatibility path. Public docs: docs/plugins/sdk-overview.md. (docs/plugins/sdk-overview.md:217, 4795f3474ad5)
  • Latest PR diff addresses previous finding: The latest diff adds resolvePluginCliRootOwnerIds, resolves owners from manifest activation or CLI metadata, returns an empty registry for unowned roots, and adds QA Lab CLI metadata for the qa root. (src/plugins/cli-registry-loader.ts:85, 5c16a5c6c64a)
  • Regression coverage in PR: The PR adds tests that openclaw foo is rejected before proxy/plugin registration and that missing CLI metadata skips full runtime plugin loading. (src/cli/run-main.exit.test.ts:398, 5c16a5c6c64a)
  • Related issue reproduction: Issue comments include controlled openclaw foo process-group runs showing provider/runtime plugin loading, an unavailable-command error, and a hot child process still alive after the command should have exited.

Likely related people:

  • vincentkoc: API history for src/plugins/cli-registry-loader.ts shows a9c7c2e1edf5 introduced narrowed CLI loading via activation planning, the exact fallback this PR changes. (role: introduced behavior / plugin CLI loader maintainer; confidence: high; commits: a9c7c2e1edf5, 74e7b8d47b18; files: src/plugins/cli-registry-loader.ts, src/plugins/cli.ts)
  • jesse-merhi: API history for src/cli/command-path-policy.ts and src/cli/run-main.ts shows 2633b1491413 added operator-managed network proxy routing, whose default policy makes unowned roots proxy-eligible on current main. (role: introduced adjacent proxy policy; confidence: medium; commits: 2633b1491413; files: src/cli/command-path-policy.ts, src/cli/run-main-policy.ts, src/cli/run-main.ts)
  • steipete: API history shows recent maintenance on CLI command policy, plugin CLI registration caching, QA Lab CLI ownership, and plugin dependency/loading paths touched by this PR. (role: recent maintainer / adjacent owner; confidence: high; commits: d763b8385467, 3b3def03543b, bb60b5312489; files: src/cli/run-main-policy.ts, src/plugins/cli.ts, extensions/qa-lab/index.ts)
  • amknight: API history for src/cli/run-main.ts shows recent work moving gateway recovery before proxy bootstrap, adjacent to the startup ordering this PR changes. (role: recent adjacent CLI startup maintainer; confidence: medium; commits: aa9db998f7bf, 8142e67d633d; files: src/cli/run-main.ts, src/plugins/gateway-startup-plugin-ids.ts)

Remaining risk / open question:

  • I did not run the claimed tests or live smoke in this read-only review; exact-head CI and the openclaw foo no-child smoke should remain the merge gate.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4795f3474ad5.

@steipete steipete force-pushed the fix-unowned-cli-command-runtime-load branch from 5c16a5c to a12a9ff Compare May 3, 2026 19:02
@steipete steipete merged commit 904cbec into openclaw:main May 3, 2026
108 checks passed
@steipete
Copy link
Copy Markdown
Contributor

steipete commented May 3, 2026

Landed via squash merge after rewriting the fix onto current main.

  • Source head: a12a9ff
  • Merge commit: 904cbec
  • Gates: focused CLI/plugin/startup tests, pnpm tsgo:core, pnpm check:test-types, pnpm build, formatter/diff checks, and full GitHub PR checks on the exact head.
  • Live smoke: built CLI rejected openclaw foo 3/3 with exit 1 and zero plugin-load logs; CLI-metadata-owned openclaw memory --help still exited 0.

Thanks @neilofneils404!

lxe pushed a commit to lxe/openclaw that referenced this pull request May 6, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
Co-authored-by: Neil <neil@neilofneils.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unknown CLI commands load plugins and leave hot child process while gateway saturates

2 participants