fix(cli): clarify error when unknown subcommand is actually an agent tool name (#77214)#77221
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Codex review: needs real behavior proof before merge. Summary Reproducibility: yes. from source inspection. Current main can emit the misleading Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Keep the CLI diagnostic, but base the tool-owner branch on the same effective plugin/tool policy as runtime discovery, including Do we have a high-confidence way to reproduce the issue? Yes from source inspection. Current main can emit the misleading Is this the best way to solve the issue? No. The requested diagnostic is the right narrow fix, but this patch should also honor Full review comments:
Overall correctness: patch is incorrect What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against eaaef2dbf871. |
There was a problem hiding this comment.
Pull request overview
This PR tries to improve CLI diagnostics when openclaw <name> is not a real CLI subcommand by distinguishing agent-tool names from plugin command names. It extends plugin manifest lookup so the unknown-command path can point users toward model tool-use instead of the existing plugins.allow suggestion.
Changes:
- Added manifest-level tool-owner lookup alongside existing command-alias lookup.
- Wired the new tool-owner resolver into CLI unknown-subcommand handling.
- Added focused tests for tool-name detection and updated the changelog.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/plugins/manifest-command-aliases.ts |
Adds manifest-based tool-owner lookup and extends the registry shape with contracts.tools. |
src/plugins/manifest-command-aliases.test.ts |
Adds unit coverage for manifest-based tool-owner resolution. |
src/plugins/manifest-command-aliases.runtime.ts |
Exposes a runtime helper that loads manifest metadata and resolves tool owners. |
src/cli/run-main.ts |
Passes the new runtime tool-owner resolver into unknown-command policy handling. |
src/cli/run-main.test.ts |
Adds tests for the new agent-tool error messaging behavior. |
src/cli/run-main-policy.ts |
Introduces the new “agent tool, not CLI subcommand” error branch. |
CHANGELOG.md |
Documents the CLI diagnostic change. |
|
The Copilot threads' correctness concern is real — verified against Timing context. The unfiltered registry hits real cases. Suggested fix shape.
Once that lands the two outstanding Copilot threads should resolve naturally. Re-review progress:
|
…tool name When `openclaw <name>` does not match a CLI subcommand or a plugin id, the unknown-subcommand handler now first looks up <name> against the manifest plugin tool registry. If <name> is an agent tool declared by a plugin (e.g. `lcm_recent` from `lossless-claw`), emit a clear 'this is an agent tool, not a CLI subcommand' error pointing at model tool-use instead of the misleading `plugins.allow` suggestion. The previous error told the user to add the tool name to `plugins.allow`, but `plugins.allow` accepts plugin ids (not tool names), and config patches against it are rejected by the protected-config-paths guard. In the wild this sent an agent down 3 restart cycles trying to 'fix' config that was never the problem. Filtering: the runtime resolver consults the full manifest snapshot and filters through `isManifestPluginAvailableForControlPlane` (covering `plugins.allow`/`plugins.deny`/`plugins.entries[id].enabled`/installed index) plus per-tool `hasManifestToolAvailability` (covering `toolMetadata.configSignals` like Feishu's `appId`/`appSecret` gate). The diagnostic also re-checks `plugins.allow` and `plugins.entries[X].enabled` for the OWNING plugin before emitting, so denied/disabled plugins are not falsely attributed. Wording: when ownership is only manifest-provable (per-account `enabled` flags and per-tool toggles in the Feishu family are runtime-only and not declarable as configSignals, so the manifest is necessarily an over-approximation), emit a softer 'may be provided by the X plugin' message instead of asserting 'registered by'. The strong wording is reserved for the case where both control-plane availability and tool availability pass. Fall-throughs: if <name> is not a registered tool, the existing `plugins.allow` and `plugins.entries.<id>.enabled` suggestions are emitted unchanged. The new branch only fires when the tool-registry lookup matches AND the owning plugin passes availability filters. Closes #77214.
2852a98 to
ae62a99
Compare
|
Thanks @100yenadmin. I could not push the final rebase/fixup back to the fork (403), so I landed this via maintainer replacement #79693. It preserves your CLI/tool diagnostic fix, softens the wording to avoid overclaiming runtime registration, and adds the missing early proxy-preflight path so live Landed in 0c50957. |
Summary
openclaw <name>does not match a CLI subcommand or a plugin id, the unknown-subcommand handler now first looks up<name>against the runtime plugin tool registry. If<name>is an agent tool registered by a loaded plugin (e.g.lcm_recentfromlossless-claw), emit a clear "this is an agent tool, not a CLI subcommand" error pointing at model tool-use instead of the misleadingplugins.allowsuggestion.plugins.allow, butplugins.allowaccepts plugin ids (not tool names), and config patches against it are rejected by the protected-config-paths guard. In the wild this sent an agent down 3 restart cycles trying to "fix" config that was never the problem.<name>is not a registered tool, the existingplugins.allowandplugins.entries.<id>.enabledsuggestions are emitted unchanged. The new branch only fires when the tool-registry lookup matches.Reproduction
Before:
After:
Implementation
resolveManifestToolOwnerInRegistryinsrc/plugins/manifest-command-aliases.tswalksplugins[].contracts.tools(the same field the agent dispatch uses) and returns{toolName, pluginId}for the owning plugin.PluginManifestCommandAliasRegistryis extended with an optionalcontracts?: { tools?: readonly string[] }per plugin entry. The runtimePluginManifestRegistryalready populates this, so production wiring is structural.resolveMissingPluginCommandMessage(insrc/cli/run-main-policy.ts) gets a parallelresolveToolOwnercallback hook (mirroringresolveCommandAliasOwner) and consults the tool registry just before theplugins.allow excludesbranch.src/cli/run-main.tspasses the newresolveManifestToolOwnerruntime resolver into the policy call.Validation
pnpm exec vitest run src/cli/run-main.test.ts src/plugins/manifest-command-aliases.test.ts-- 33 passed (3 new inrun-main.test.ts, 1 new inmanifest-command-aliases.test.ts).pnpm exec vitest run src/cli/run-main.exit.test.ts src/cli/run-main.profile-env.test.ts-- 79 passed (sanity check on adjacent test files).pnpm exec oxlint --type-aware src/cli/run-main-policy.ts src/cli/run-main.ts src/cli/run-main.test.ts src/plugins/manifest-command-aliases.ts src/plugins/manifest-command-aliases.runtime.ts src/plugins/manifest-command-aliases.test.ts-- 0 warnings, 0 errors.pnpm tsgo:core-- clean.pnpm check:changelog-attributions-- clean.Closes #77214.