Repository navigation
Improve upstream plugin compatibility and add CLI extensions - #201
Conversation
|
Warning Review limit reached
Next review available in: 8 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (24)
📝 WalkthroughWalkthroughChangesPlugin compatibility and CLI integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant OpenClawCli
participant PluginCliCommands
participant pluginbridgemjs
participant Plugin
User->>OpenClawCli: enter plugin root command
OpenClawCli->>PluginCliCommands: request command discovery
PluginCliCommands->>pluginbridgemjs: start isolated describe process
pluginbridgemjs->>Plugin: initialize plugin in metadata mode
Plugin-->>pluginbridgemjs: register CLI command
pluginbridgemjs-->>PluginCliCommands: return command descriptor
PluginCliCommands->>pluginbridgemjs: start command execution
pluginbridgemjs->>Plugin: execute command with arguments and options
Plugin-->>User: write CLI output
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR expands OpenClaw.NET’s upstream plugin interoperability by adding api.registerCli() support (lazy root-command discovery + one-shot Node execution), strengthening plugin discovery/installation safety (staging + compatibility floors), and introducing safe detection/mapping of Codex/Claude/Cursor “bundle” plugin layouts without executing arbitrary bundle JavaScript. It also preserves structured tool output (output schema + JSON details) end-to-end and surfaces richer plugin health metadata in operator views.
Changes:
- Add descriptor-first
registerCli()bridging: discover root commands lazily, enforce collisions/eligibility rules, and run plugin CLI commands in an isolated Node process with terminal inheritance. - Harden plugin discovery/installation: prefer runtime entrypoints, validate package compatibility floors/integrity metadata, stage installs and only replace after successful initialization.
- Detect Codex/Claude/Cursor bundles and map safe content (skills + markdown commands) while reporting detected-but-unmapped surfaces for operator visibility.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/OpenClaw.Tests/SkillTests.cs | Adds test coverage for mapping bundle commands/*.md into user-invocable skills. |
| src/OpenClaw.Tests/PublicCompatibilitySmokeTests.cs | Adds “latest canary” smoke lane and improves npm plugin diagnostics labels. |
| src/OpenClaw.Tests/PluginTests.cs | Adds tests for runtime entry precedence, bundle detection, and structured tool output schema preservation. |
| src/OpenClaw.Tests/PluginCommandsTests.cs | Adds coverage for CLI inspection/execution, bundle inspection, install staging safety, and API-floor blocking. |
| src/OpenClaw.Tests/PluginBridgeIntegrationTests.cs | Adds integration tests for structured tool results and registerCli() reporting; stabilizes restart test behavior. |
| src/OpenClaw.Tests/CompatibilityCommandsTests.cs | Updates catalog expectations to reflect CLI-plugin compatibility scenario. |
| src/OpenClaw.Gateway/PluginHealthService.cs | Extends plugin health snapshots with bundle format and CLI command counts; improves declared-surface summary. |
| src/OpenClaw.Gateway/Composition/RuntimeInitializationExtensions.CompositionStages.cs | Propagates new plugin report fields into combined reports. |
| src/OpenClaw.Dashboard/Pages/Ops.razor | Updates ops UI to show richer plugin status/surface info and adds mutate actions (enable/disable/review/quarantine). |
| src/OpenClaw.Dashboard/Models/PluginInfo.cs | Expands plugin model to match richer health payload and derive status/detail fields. |
| src/OpenClaw.Core/Skills/SkillLoader.cs | Maps bundle command markdown (commands/*.md) into skills when scanning plugin command roots. |
| src/OpenClaw.Core/Plugins/PluginPackageCompatibility.cs | Introduces package-declared compatibility floor validation before plugin code runs. |
| src/OpenClaw.Core/Plugins/PluginModels.cs | Extends core plugin models with bundle metadata, CLI registrations, and structured tool output (OutputSchema, Details). |
| src/OpenClaw.Core/Plugins/PluginDiscovery.cs | Adds .cjs support, bundle detection path, runtime-entry preference, and package metadata extraction. |
| src/OpenClaw.Core/Plugins/PluginCapabilityPolicy.cs | Adds cli capability identifier. |
| src/OpenClaw.Core/Plugins/PluginBundleDetector.cs | Implements safe bundle detection and capability mapping without executing bundle JS. |
| src/OpenClaw.Core/Models/Session.cs | Updates source-gen JSON context to include new CLI registration types. |
| src/OpenClaw.Core/Models/OperatorApiModels.cs | Extends operator API models with bundle format and CLI command counts. |
| src/OpenClaw.Core/Compatibility/PublicCompatibilityCatalog.cs | Updates catalog summaries/guidance for CLI plugin compatibility scenario. |
| src/OpenClaw.Core/Abstractions/ITool.cs | Adds optional IToolOutputSchema interface for structured tool output schemas. |
| src/OpenClaw.Cli/Program.cs | Adds lazy fallback: try plugin CLI root execution before unknown-command error. |
| src/OpenClaw.Cli/PluginCommands.cs | Adds plugins inspect, stages installs atomically, and adds runtime inspection path. |
| src/OpenClaw.Cli/PluginCliCommands.cs | Implements lazy plugin CLI root discovery and one-shot execution with safety checks. |
| src/OpenClaw.Cli/OpenClaw.Cli.csproj | Ships plugin-bridge.mjs with CLI output/publish so runtime inspection & CLI bridging work. |
| src/OpenClaw.Agent/Plugins/PluginHost.cs | Adds bundle loading path and package compatibility validation before bridge initialization. |
| src/OpenClaw.Agent/Plugins/PluginBridgeProcess.cs | Preserves structured tool details, hardens restart/monitor lifecycle to avoid stale monitor teardown. |
| src/OpenClaw.Agent/Plugins/plugin-bridge.mjs | Adds CLI registration/describe/run modes and structured tool outputSchema/details support. |
| src/OpenClaw.Agent/Plugins/BridgedPluginTool.cs | Exposes tool OutputSchema via new IToolOutputSchema. |
| src/OpenClaw.Agent/OpenClawToolExecutor.cs | Plumbs OutputSchema into model function declarations as returnJsonSchema. |
| docs/zh-CN/COMPATIBILITY.md | Updates zh-CN compatibility matrix to include registerCli() support. |
| docs/USER_GUIDE.md | Documents dry-run/staged install safety, bundle behavior, and CLI root execution. |
| docs/COMPATIBILITY.md | Updates compatibility guide for bundles, staged installs, structured tool outputs, and CLI support. |
| compat/public-smoke.json | Updates pinned public smoke scenario to reflect Supermemory CLI compatibility. |
| .github/workflows/ci.yml | Adds non-blocking “latest canary” job and uploads its TRX alongside pinned smoke results. |
Suppressed comments (1)
src/OpenClaw.Dashboard/Pages/Ops.razor:106
- The action labels "Clear quarantine", "Clear review", and "Mark reviewed" are hard-coded English strings on an otherwise localized page. They should be moved into the locale resources and referenced via
L[...]so zh-CN (and any other locales) render correctly.
<MudButton Size="Size.Small"
Variant="Variant.Outlined"
Color="Color.Warning"
OnClick="@(() => MutatePlugin(context.Item, "clear-quarantine"))">
Clear quarantine
</MudButton>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/OpenClaw.Core/Plugins/PluginBundleDetector.cs:150
- Bundle detection for Claude falls back to treating any directory that contains
skills/orcommands/as a Claude bundle (even without.claude-plugin/plugin.json). InPluginDiscovery.ScanDirectory, this check runs before theindex.js/mjs/cjs/tsfallback, so a standalone plugin directory that happens to includecommands/(orskills/) but relies onindex.jsdiscovery can be misclassified as a bundle and never executed. Consider tightening the heuristic soskills/orcommands/alone is not sufficient to classify a directory as a Claude bundle (require additional bundle markers such asagents/,hooks/,.mcp.json,.lsp.json, orsettings.json).
if (Directory.Exists(Path.Combine(rootPath, "skills")) ||
Directory.Exists(Path.Combine(rootPath, "commands")) ||
Directory.Exists(Path.Combine(rootPath, "agents")) ||
Directory.Exists(Path.Combine(rootPath, "hooks")) ||
File.Exists(Path.Combine(rootPath, ".mcp.json")) ||
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
src/OpenClaw.Dashboard/Pages/Ops.razor:93
- The status chip now renders raw English status strings (
quarantined,disabled,loaded,not loaded) instead of localized resources (previously usedL["common.enabled"]/L["common.disabled"]). This is a localization regression for non-English dashboards; consider adding locale keys for the new states and mapping them in the UI.
Color="@(context.Item.Loaded && context.Item.Enabled ? Color.Success : context.Item.Quarantined ? Color.Error : Color.Default)"
Variant="@(context.Item.Loaded && context.Item.Enabled ? Variant.Filled : Variant.Outlined)">
@context.Item.Status
</MudChip>
src/OpenClaw.Agent/Plugins/plugin-bridge.mjs:600
parseCliInvocationtreats any required option value starting with-as missing, so valid invocations like--threshold -1fail with "requires a value". This diverges from Commander behavior and makes it impossible to pass negative numbers (or other dash-prefixed values) unless users rewrite args.
} else if (option.requiredValue || option.optionalValue) {
const next = inlineValue ?? argv[index + 1];
if (next === undefined || (option.requiredValue && next.startsWith("-"))) {
throw new Error(`Option ${flag} requires a value.`);
}
src/OpenClaw.Cli/PluginCliCommands.cs:182
DescribeAsyncdeserializes the entire stdout payload as JSON. If a plugin (or its dependencies) writes anything to stdout during CLI metadata registration, discovery will fail even though the bridge still writes a valid JSON line. Parsing only the last non-empty line makes discovery resilient while still keeping stdout "owned" by the bridge.
var commands = JsonSerializer.Deserialize(
stdout,
CoreJsonContext.Default.BridgeCliCommandRegistrationArray);
return commands is null
? PluginCliDescribeResult.Failure("Plugin CLI discovery returned unreadable metadata.")
: new PluginCliDescribeResult { Success = true, Commands = commands };
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
src/OpenClaw.Agent/Plugins/plugin-bridge.mjs:25
- In socket/hybrid mode
connectSocketTransport()can attach an error handler that referencespluginIdbeforepluginIdis initialized (it’s declared later withlet). If the socket connect fails quickly, the handler can run whilepluginIdis still in the temporal-dead-zone and throwReferenceError, masking the real transport error. Declare/initializepluginId(andlogger) before any transport setup, or avoid referencingpluginIdin early transport error paths until after init.
const standaloneCliMode = process.argv[2] === "--cli-run";
const standaloneCliDescribeMode = process.argv[2] === "--cli-describe";
// Runtime bridge traffic owns stdout. Standalone CLI execution inherits stdout
// so plugin commands can render output and use an interactive terminal.
if (!standaloneCliMode) {
console.log = console.error;
console.info = console.error;
}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (8)
src/OpenClaw.Cli/Program.cs (1)
86-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument plugin root commands in
PrintHelp.Any unrecognized command now dispatches to an installed plugin.
PrintHelpdoes not mention this, so the primary new capability is undiscoverable fromopenclaw --help. The "Plugin management" section at lines 230-235 lists onlyopenclaw plugins ...subcommands.Add a short note that installed plugins can register root commands, with one example.
♻️ Proposed help text addition
Plugin management: openclaw plugins install <package-name> Install from npm/ClawHub openclaw plugins install ./local-plugin Install from local path openclaw plugins remove <plugin-name> Remove a plugin openclaw plugins list List installed plugins openclaw plugins search <query> Search npm for plugins + + Installed plugins can also register root commands. Built-in commands + take precedence. Disabled and quarantined plugins are not dispatched. + openclaw <plugin-command> --help Show a plugin command's help🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Cli/Program.cs` around lines 86 - 88, Update PrintHelp to document that installed plugins may register root-level commands, adding one concise example alongside the existing “Plugin management” help section. Keep the current plugin-management subcommand descriptions unchanged.src/OpenClaw.Cli/PluginCliCommands.cs (1)
103-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSurface the describe failure reason.
PluginCliDescribeResult.Failurerecords a message inError, but no caller reads it. Lines 109-110 discard the result andcontinue.A plugin whose descriptor fails to load is therefore skipped in silence. If no other plugin matches, the operator sees only "Unknown command", with no indication that a plugin failed to report its commands. Node not being installed, a plugin crash on load, and a describe timeout all produce the same opaque output.
Write the reason to stderr, or emit it behind a verbose flag.
♻️ Proposed diagnostic output
if (!description.Success) + { + Console.Error.WriteLine( + $"Plugin '{plugin.Manifest.Id}' did not report CLI commands: {description.Error}"); continue; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Cli/PluginCliCommands.cs` around lines 103 - 110, Update the failed-result branch in the plugin description flow around DescribeAsync to surface description.Error to stderr, or emit it through the existing verbose diagnostic mechanism. Preserve the continue behavior after reporting the failure so other plugins are still evaluated.src/OpenClaw.Core/Skills/SkillLoader.cs (1)
285-288: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog a warning when a bundle command overwrites an existing skill name.
results[skill.Name] = skillreplaces any earlier entry with the same name. Two command files in different subdirectories that declare the same frontmatternamecollide, and the path-ordered last file wins with no record.
ScanDirectoryalready logs duplicate owner-qualified slugs at line 222. Add the same visibility here so a silently dropped command is diagnosable.♻️ Proposed duplicate warning
var skill = ParseSkillContent(normalized, commandDir, SkillSource.Plugin); - if (skill is not null) - results[skill.Name] = skill; - else + if (skill is null) + { logger.LogWarning("Failed to map bundle command at {Path} into a skill", commandFile); + continue; + } + + if (results.ContainsKey(skill.Name)) + { + logger.LogWarning( + "Bundle command at {Path} overwrites existing skill '{Name}'", + commandFile, + skill.Name); + } + + results[skill.Name] = skill;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Core/Skills/SkillLoader.cs` around lines 285 - 288, Update the result insertion logic in ScanDirectory so it detects when results already contains skill.Name before overwriting it, logs a warning identifying the duplicate command and relevant paths, then preserves the existing assignment behavior; keep the current null-skill warning unchanged.src/OpenClaw.Tests/PluginBridgeIntegrationTests.cs (1)
1515-1522: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed delay with a bounded poll.
The loop kills the child process and then waits exactly 150 ms. The restart is asynchronous, and this test runs for three transport modes and five iterations. On a loaded CI machine the fixed delay makes the assertion timing-sensitive. Poll the echo call until it succeeds, with an overall timeout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Tests/PluginBridgeIntegrationTests.cs` around lines 1515 - 1522, Replace the fixed Task.Delay in the restart loop around tool.ExecuteAsync with a bounded polling mechanism that repeatedly attempts the expected echo response until the restarted child is ready. Apply an overall timeout and preserve cancellation through TestContext.Current.CancellationToken, while keeping the existing attempt-specific expected text and failure behavior.src/OpenClaw.Cli/PluginCommands.cs (1)
717-723: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDispose the temporary
JsonDocument.
JsonDocument.Parse("{}")rents pooled buffers. The document is never disposed, so those buffers are not returned.Clone()detaches the element, so disposal is safe here.♻️ Proposed change
+ using var emptyConfig = JsonDocument.Parse("{}"); var initRequest = new BridgeInitRequest { EntryPath = Path.GetFullPath(entryPath), PluginId = pluginId, - Config = JsonDocument.Parse("{}").RootElement.Clone(), + Config = emptyConfig.RootElement.Clone(), Transport = new BridgeTransportRuntimeConfig { Mode = "stdio" } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Cli/PluginCommands.cs` around lines 717 - 723, Dispose the temporary JsonDocument created for Config in the BridgeInitRequest initializer. Update the initialization around BridgeInitRequest so JsonDocument.Parse("{}") is scoped and disposed after cloning its RootElement, while preserving the detached Config value.src/OpenClaw.Tests/PluginCommandsTests.cs (3)
347-367: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHandle the wait timeout and share this probe.
WaitForExit(3000)returns a bool that this code ignores. If the wait times out,process.ExitCodethrows, the catch returnsfalse, and thenodeprocess stays alive after disposal. Check the return value and kill the process on timeout.HasNodealso exists inPluginBridgeIntegrationTests.csandPublicCompatibilitySmokeTests.cs. Move one copy to a shared test helper.♻️ Proposed change
- process?.WaitForExit(3000); - return process is { ExitCode: 0 }; + if (process is null) + return false; + if (!process.WaitForExit(3000)) + { + try { process.Kill(entireProcessTree: true); } catch { } + return false; + } + + return process.ExitCode == 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Tests/PluginCommandsTests.cs` around lines 347 - 367, Update HasNode to check the boolean result from WaitForExit, kill the process when the timeout expires, and only read ExitCode after confirmed termination. Remove the duplicate HasNode implementations from the test classes and move the shared probe into a common test helper, updating callers to use it.
61-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen the CLI acceptance assertion.
The static scan in
InspectUnsupportedRuntimeSurfaceschecks onlyregisterGatewayMethod. No code path can produceunsupported_cli_registrationduring static inspection, so line 70 passes for any input and does not prove thatregisterCliis accepted. Assert the positive outcome instead, for example that no error-severity diagnostic exists, and add a negative case withapi.registerGatewayMethod(...)that must be blocked.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Tests/PluginCommandsTests.cs` around lines 61 - 70, Strengthen the test around PluginCommands.InspectCandidate by asserting the registerCli fixture has no error-severity diagnostics rather than checking the unreachable unsupported_cli_registration code. Add a separate fixture using api.registerGatewayMethod(...) and assert static inspection rejects it with an error diagnostic.
114-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the silent runtime-guard returns with xUnit skip assertions.
The current
returnmakes these tests pass when Node.js is unavailable. In this xUnit v3 test project, useAssert.SkipUnless(HasNode(), "Node.js is not available on this machine.");so missing Node coverage is reported as skipped rather than green.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Tests/PluginCommandsTests.cs` around lines 114 - 115, Replace the silent runtime-guard return in the affected xUnit test with an Assert.SkipUnless(HasNode(), "Node.js is not available on this machine.") assertion, so tests are reported as skipped when Node.js is unavailable rather than passing without coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 338-349: Update the “Report latest-package compatibility drift”
step to distinguish compatibility mismatches from npm, process, Node, or test
infrastructure failures instead of relying only on
steps.latest_plugin_canary.outcome. Use the canary diagnostics or test result to
classify drift, report a generic canary failure for other failures, and include
the test result in the warning and step summary while preserving the
non-blocking behavior.
In `@compat/public-smoke.json`:
- Around line 65-72: Update the compatibility catalog entry for
`@supermemory/openclaw-supermemory` to include expected CLI metadata, then extend
PublicCompatibilitySmokeTests.VerifyNpmPluginAsync to validate the discovered
root command through the plugin’s registered CLI. Ensure the smoke test fails
when api.registerCli() is missing or produces an unexpected command, rather than
relying on the descriptive cli-plugin category.
In `@docs/zh-CN/COMPATIBILITY.md`:
- Line 36: Update the api.registerCli() entry in the Chinese compatibility
matrix to use the caveated support status, preserving the existing lazy root
discovery and one-shot Node bridge details while also documenting the key
limitations: root precedence, duplicate-root rejection, plugin enablement,
configuration validation, and Commander-subset support.
In `@src/OpenClaw.Agent/OpenClawToolExecutor.cs`:
- Around line 1245-1258: Update the output-schema handling in the declaration
creation flow to catch JsonException from parsing a non-empty tool.OutputSchema,
dispose any returnSchemaDocument, and continue with a null returnJsonSchema so
malformed output schemas do not remove the tool. Preserve valid-schema cloning,
and wrap the using (returnSchemaDocument) statement in braces to make its
disposal scope explicit before returning from CreateDeclaration.
In `@src/OpenClaw.Agent/Plugins/plugin-bridge.mjs`:
- Around line 1130-1145: Update runStandaloneCli so the normal executeCli path
does not call process.exit(exitCode) immediately; assign the returned exit code
to process.exitCode and allow Node.js to exit naturally, preserving the existing
describe-mode flush behavior and error handling.
In `@src/OpenClaw.Agent/Plugins/PluginBridgeProcess.cs`:
- Around line 134-144: Update the result handling in ExecuteToolAsync: require
the content property to have JsonValueKind.Array before passing it to
EnumerateArray, otherwise return the raw result safely. When details is present
alongside content, preserve the model-facing text by appending or otherwise
combining the extracted content unless the existing contract explicitly requires
details-only precedence.
In `@src/OpenClaw.Cli/PluginCliCommands.cs`:
- Around line 223-235: Update ExecuteAsync so the ExecuteTimeout-linked
cancellation and timeout kill path apply only when the child process has
redirected stdin; for interactive commands that own the terminal, await process
completion using the outer cancellationToken directly and preserve Ctrl-C
cancellation behavior. Keep the existing timeout exit code and TryKill handling
for non-interactive runs.
- Around line 160-182: Bound child-process output in the plugin discovery flow
around the process execution method: create the linked timeout token before
starting reads, replace both StandardOutput and StandardError ReadToEndAsync
calls with ReadCappedAsync using timeout.Token, and await both tasks so timeout
cancellation is observed. Treat a null capped-read result for either stream as
PluginCliDescribeResult.Failure, while preserving existing exit-code and JSON
parsing behavior for successful reads.
- Around line 83-117: Cache DescribeAsync results across CLI invocations using
each plugin’s entry path and last-write timestamp as the cache key, while
preserving the full match collection needed for ambiguity detection. Update the
plugin-discovery flow around DescribeAsync in the command handler to reuse
cached descriptors instead of spawning a Node process for every plugin; also
bypass plugin lookup when command starts with “-”.
In `@src/OpenClaw.Cli/PluginCommands.cs`:
- Around line 1162-1223: Update AddUnsupportedSurfaceDiagnostic to detect only
calls through the plugin API object, matching api.registerGatewayMethod( rather
than standalone text, so comments and strings do not produce errors; in
InspectUnsupportedRuntimeSurfaces, skip source files exceeding the chosen size
limit before reading them. Add or update tests proving commented mentions do not
block installation and that oversized files are ignored, while preserving
fail-fast diagnostics for actual supported-pattern calls.
- Around line 785-798: Update ResolveBridgeScriptPath so it never derives the
fallback bridge script from Directory.GetCurrentDirectory() in unrestricted
builds. Only enable the source-tree fallback behind an explicit trusted
development gate, such as the established development environment flag or `#if`
DEBUG; otherwise return null immediately when the packaged script is
unavailable, preserving the existing packaged-path behavior.
- Around line 903-911: Update InspectCandidate around
PluginDiscovery.DiscoverWithDiagnostics so that when discovery.Plugins contains
no plugin, it evaluates discovery.Reports for relevant load failures such as
invalid_manifest, invalid_package_json, or entry_outside_root and merges them
into the returned PluginInstallInspection result. Preserve the existing bundle
and entry-path handling for successfully discovered plugins, while preventing
report-only failures from producing a manifest-valid CanInstall result.
- Around line 1125-1136: Update the extension-selection logic in
InspectCandidate so an openclaw object lacking both runtimeExtensions and
extensions falls through to the standard entry candidates instead of returning
null; retain array validation when an extension property is present. Add
coverage for a package declaring only openclaw.compat and shipping index.js,
confirming the standard entry file resolves.
- Around line 657-671: Update HasLocalJiti to accept the staged root directory
and stop ancestor traversal once that boundary is reached; update its call site
to pass stagedInspection.EntryPath and stagingDir, ensuring only
node_modules/jiti within the staged plugin root is considered.
In `@src/OpenClaw.Core/Plugins/PluginDiscovery.cs`:
- Around line 194-201: Update PluginDiscovery.ScanDirectory and every recursive
call to carry a recursion depth, enforce a finite maximum depth, and stop
scanning once that limit is reached. Configure Directory.EnumerateDirectories
with EnumerationOptions.AttributesToSkip including FileAttributes.ReparsePoint,
matching SkillLoader.ScanDirectory, while preserving the existing node_modules
and .git exclusions.
- Around line 565-618: Update the plugin installation/materialization flow
associated with ReadPackageMetadata and DiscoveredPlugin to compute the package
digest and compare it with ExpectedIntegrity before reporting or accepting the
plugin. Reject the plugin when the expected value is missing, mismatched, or
verification is unavailable; do not expose ExpectedIntegrity as validated
metadata until this check is enforced.
In `@src/OpenClaw.Core/Plugins/PluginPackageCompatibility.cs`:
- Around line 54-62: Update the version normalization logic in
PluginPackageCompatibility to remove npm-style caret (^) and tilde (~) prefixes
in addition to the existing range operators before parsing. After truncating
suffixes, detect single-component versions and pad them to a parseable form such
as major.0, preserving existing normalization for multi-component versions.
In `@src/OpenClaw.Core/Skills/SkillLoader.cs`:
- Around line 297-314: Update NormalizeBundleCommandContent to accept only a
top-level frontmatter line beginning with name:, rejecting indented or nested
keys such as metadata.value.name:. Reuse a single computed frontmatter
terminator index instead of calling IndexOf twice, while preserving insertion of
the commandName when no valid top-level name exists.
In `@src/OpenClaw.Tests/PublicCompatibilitySmokeTests.cs`:
- Around line 69-70: Update the canary guard in the relevant compatibility test
so that when IsLatestCanaryEnabled() is true but HasNode() is false, the test
fails instead of returning successfully; preserve the existing skip behavior
when the latest-canary mode is disabled, and ensure the failure clearly reports
that Node is required to execute the canary.
---
Nitpick comments:
In `@src/OpenClaw.Cli/PluginCliCommands.cs`:
- Around line 103-110: Update the failed-result branch in the plugin description
flow around DescribeAsync to surface description.Error to stderr, or emit it
through the existing verbose diagnostic mechanism. Preserve the continue
behavior after reporting the failure so other plugins are still evaluated.
In `@src/OpenClaw.Cli/PluginCommands.cs`:
- Around line 717-723: Dispose the temporary JsonDocument created for Config in
the BridgeInitRequest initializer. Update the initialization around
BridgeInitRequest so JsonDocument.Parse("{}") is scoped and disposed after
cloning its RootElement, while preserving the detached Config value.
In `@src/OpenClaw.Cli/Program.cs`:
- Around line 86-88: Update PrintHelp to document that installed plugins may
register root-level commands, adding one concise example alongside the existing
“Plugin management” help section. Keep the current plugin-management subcommand
descriptions unchanged.
In `@src/OpenClaw.Core/Skills/SkillLoader.cs`:
- Around line 285-288: Update the result insertion logic in ScanDirectory so it
detects when results already contains skill.Name before overwriting it, logs a
warning identifying the duplicate command and relevant paths, then preserves the
existing assignment behavior; keep the current null-skill warning unchanged.
In `@src/OpenClaw.Tests/PluginBridgeIntegrationTests.cs`:
- Around line 1515-1522: Replace the fixed Task.Delay in the restart loop around
tool.ExecuteAsync with a bounded polling mechanism that repeatedly attempts the
expected echo response until the restarted child is ready. Apply an overall
timeout and preserve cancellation through TestContext.Current.CancellationToken,
while keeping the existing attempt-specific expected text and failure behavior.
In `@src/OpenClaw.Tests/PluginCommandsTests.cs`:
- Around line 347-367: Update HasNode to check the boolean result from
WaitForExit, kill the process when the timeout expires, and only read ExitCode
after confirmed termination. Remove the duplicate HasNode implementations from
the test classes and move the shared probe into a common test helper, updating
callers to use it.
- Around line 61-70: Strengthen the test around PluginCommands.InspectCandidate
by asserting the registerCli fixture has no error-severity diagnostics rather
than checking the unreachable unsupported_cli_registration code. Add a separate
fixture using api.registerGatewayMethod(...) and assert static inspection
rejects it with an error diagnostic.
- Around line 114-115: Replace the silent runtime-guard return in the affected
xUnit test with an Assert.SkipUnless(HasNode(), "Node.js is not available on
this machine.") assertion, so tests are reported as skipped when Node.js is
unavailable rather than passing without coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a188d48c-35b6-45ac-b2d8-3a15bc69b344
📒 Files selected for processing (36)
.github/workflows/ci.ymlcompat/public-smoke.jsondocs/COMPATIBILITY.mddocs/USER_GUIDE.mddocs/zh-CN/COMPATIBILITY.mdsrc/OpenClaw.Agent/OpenClawToolExecutor.cssrc/OpenClaw.Agent/Plugins/BridgedPluginTool.cssrc/OpenClaw.Agent/Plugins/PluginBridgeProcess.cssrc/OpenClaw.Agent/Plugins/PluginHost.cssrc/OpenClaw.Agent/Plugins/plugin-bridge.mjssrc/OpenClaw.Cli/OpenClaw.Cli.csprojsrc/OpenClaw.Cli/PluginCliCommands.cssrc/OpenClaw.Cli/PluginCommands.cssrc/OpenClaw.Cli/Program.cssrc/OpenClaw.Core/Abstractions/ITool.cssrc/OpenClaw.Core/Compatibility/PublicCompatibilityCatalog.cssrc/OpenClaw.Core/Models/OperatorApiModels.cssrc/OpenClaw.Core/Models/Session.cssrc/OpenClaw.Core/Plugins/PluginBundleDetector.cssrc/OpenClaw.Core/Plugins/PluginCapabilityPolicy.cssrc/OpenClaw.Core/Plugins/PluginDiscovery.cssrc/OpenClaw.Core/Plugins/PluginModels.cssrc/OpenClaw.Core/Plugins/PluginPackageCompatibility.cssrc/OpenClaw.Core/Skills/SkillLoader.cssrc/OpenClaw.Dashboard/Models/PluginInfo.cssrc/OpenClaw.Dashboard/Pages/Ops.razorsrc/OpenClaw.Dashboard/wwwroot/locales/en-US.jsonsrc/OpenClaw.Dashboard/wwwroot/locales/zh-CN.jsonsrc/OpenClaw.Gateway/Composition/RuntimeInitializationExtensions.CompositionStages.cssrc/OpenClaw.Gateway/PluginHealthService.cssrc/OpenClaw.Tests/CompatibilityCommandsTests.cssrc/OpenClaw.Tests/PluginBridgeIntegrationTests.cssrc/OpenClaw.Tests/PluginCommandsTests.cssrc/OpenClaw.Tests/PluginTests.cssrc/OpenClaw.Tests/PublicCompatibilitySmokeTests.cssrc/OpenClaw.Tests/SkillTests.cs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/OpenClaw.Cli/PluginCommands.cs:1233
AddUnsupportedSurfaceDiagnosticflags any call toregisterGatewayMethod(, but the diagnostic message claims the plugin referencedapi.registerGatewayMethod(). This can mislead operators (and makes it harder to correlate the reported match to the source). Align the message with what the regex actually detects, or tighten the regex to only matchapi.registerGatewayMethodspecifically.
if (!System.Text.RegularExpressions.Regex.IsMatch(
source,
$@"\b{System.Text.RegularExpressions.Regex.Escape(apiName)}\s*\(",
System.Text.RegularExpressions.RegexOptions.CultureInvariant) ||
diagnostics.Any(item => string.Equals(item.Code, code, StringComparison.Ordinal)))
src/OpenClaw.Core/Plugins/PluginDiscovery.cs:498
TryAddPluginPackreturnstruefrom thecatch, which causesScanDirectoryto stop scanning this folder entirely. That means a plugin/bundle with an unreadable or malformedpackage.jsonbecomes undiscoverable even if it has a validopenclaw.plugin.jsonorindex.*entry. Consider returningfalsehere so discovery can fall back to conventional entry/bundle detection after recording the diagnostic.
Path = Path.GetFullPath(packageJsonPath)
}
]
});
return true;
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/OpenClaw.Cli/PluginCommands.cs (1)
569-572: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDrain both redirected streams concurrently.
RunProcessAsyncawaits stdout to EOF before it starts reading stderr. If npm fills stderr before stdout closes, it can block and the dependency install can hang. Start bothReadToEndAsynctasks before awaiting process exit, then await both tasks. Add a regression test that writes enough stderr data to fill a redirected pipe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Cli/PluginCommands.cs` around lines 569 - 572, Update RunProcessAsync to start stdout and stderr ReadToEndAsync operations concurrently before awaiting process exit, then await both stream tasks after the process completes so redirected pipes cannot deadlock. Add a regression test covering a process that writes enough stderr to fill the redirected pipe.Source: Coding guidelines
src/OpenClaw.Core/Plugins/PluginBundleDetector.cs (1)
74-83: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject non-object plugin manifests before building
DiscoveredPlugin.
JsonDocument.Parseaccepts valid JSON arrays,null, and scalar values. This code then falls back to the directory name and returns a normalDiscoveredPlugin.PluginHost.LoadBundlecan report the bundle as loaded because no error diagnostic exists.Check the manifest root when
manifestDocumentis not null. Returninvalid_bundle_manifestfor every root that is not a JSON object. Add a regression test for[]andnull.As per coding guidelines, keep plugin compatibility explicit and fail-fast; do not silently degrade unsupported surfaces.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenClaw.Core/Plugins/PluginBundleDetector.cs` around lines 74 - 83, In the manifest-processing flow around manifestRoot and before deriving rawId or constructing DiscoveredPlugin, validate that a non-null manifestDocument has a JSON object root; otherwise return the invalid_bundle_manifest result with an error diagnostic. Preserve existing object-manifest ID resolution, and add regression coverage for [] and null roots.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/OpenClaw.Core/Plugins/PluginBundleDetector.cs`:
- Around line 15-35: The marker classification in HasExplicitOrStrongMarker must
preserve conventional index-entry precedence: either restrict override-strong
detection to bundle-specific markers only, or update
PluginDiscovery.ScanDirectory so index.js/index.mjs/index.cjs/index.ts remain
preferred when generic markers are present. Add regression tests covering each
conventional entry alongside the relevant markers, preserving the intended
precedence behavior.
---
Outside diff comments:
In `@src/OpenClaw.Cli/PluginCommands.cs`:
- Around line 569-572: Update RunProcessAsync to start stdout and stderr
ReadToEndAsync operations concurrently before awaiting process exit, then await
both stream tasks after the process completes so redirected pipes cannot
deadlock. Add a regression test covering a process that writes enough stderr to
fill the redirected pipe.
In `@src/OpenClaw.Core/Plugins/PluginBundleDetector.cs`:
- Around line 74-83: In the manifest-processing flow around manifestRoot and
before deriving rawId or constructing DiscoveredPlugin, validate that a non-null
manifestDocument has a JSON object root; otherwise return the
invalid_bundle_manifest result with an error diagnostic. Preserve existing
object-manifest ID resolution, and add regression coverage for [] and null
roots.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 140ab2ed-887d-4498-9e89-ef779b6f702b
📒 Files selected for processing (10)
src/OpenClaw.Agent/Plugins/plugin-bridge.mjssrc/OpenClaw.Cli/PluginCliCommands.cssrc/OpenClaw.Cli/PluginCommands.cssrc/OpenClaw.Core/Plugins/PluginBundleDetector.cssrc/OpenClaw.Core/Plugins/PluginDiscovery.cssrc/OpenClaw.Dashboard/Pages/Ops.razorsrc/OpenClaw.Dashboard/wwwroot/locales/en-US.jsonsrc/OpenClaw.Dashboard/wwwroot/locales/zh-CN.jsonsrc/OpenClaw.Tests/PluginCommandsTests.cssrc/OpenClaw.Tests/PluginTests.cs
🚧 Files skipped from review as they are similar to previous changes (7)
- src/OpenClaw.Dashboard/Pages/Ops.razor
- src/OpenClaw.Cli/PluginCliCommands.cs
- src/OpenClaw.Dashboard/wwwroot/locales/zh-CN.json
- src/OpenClaw.Tests/PluginCommandsTests.cs
- src/OpenClaw.Core/Plugins/PluginDiscovery.cs
- src/OpenClaw.Tests/PluginTests.cs
- src/OpenClaw.Agent/Plugins/plugin-bridge.mjs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/OpenClaw.Cli/PluginCommands.cs:1224
- The unsupported-surface detection regex matches any
registerGatewayMethod(call, even if it’s a locally-defined function and notapi.registerGatewayMethod(...). That can produce false-positive install blocks for plugins that happen to define a helper with the same name.
Consider tightening the match to the actual API call form (e.g., api.registerGatewayMethod().
if (!System.Text.RegularExpressions.Regex.IsMatch(
source,
$@"\b{System.Text.RegularExpressions.Regex.Escape(apiName)}\s*\(",
System.Text.RegularExpressions.RegexOptions.CultureInvariant) ||
diagnostics.Any(item => string.Equals(item.Code, code, StringComparison.Ordinal)))
return;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
src/OpenClaw.Core/Skills/SkillLoader.cs:314
- NormalizeBundleCommandContent treats any file starting with "---" and containing a later "\n---" as YAML frontmatter. For arbitrary bundle command Markdown, a leading horizontal-rule (---) plus another rule later can be misdetected as frontmatter, causing incorrect skill metadata injection/parsing. Consider requiring the opening delimiter to be exactly "---\n"/"---\r\n" and the closing delimiter to be on its own line ("\n---\n" or "\n---\r\n") before treating the content as frontmatter.
results[skill.Name] = skill;
}
catch (Exception ex) when (IsPathException(ex))
{
logger.LogWarning(ex, "Skipping inaccessible bundle command at {Path}", commandFile);
src/OpenClaw.Cli/PluginCliCommands.cs:258
- ExecuteAsync only applies the 10-minute timeout when Console.IsInputRedirected. That makes interactive plugin CLI runs unbounded, which conflicts with the PR description/user guidance stating plugin commands time out after ten minutes. If the intent is a hard bound, apply ExecuteTimeout unconditionally (or document the interactive exception explicitly).
}
try
{
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/OpenClaw.Cli/PluginCommands.cs:1236
EnumeratePluginSourceFileswalks the directory tree using the defaultDirectory.EnumerateDirectories/EnumerateFilesoverloads, which may follow reparse points (symlinks/junctions). A malicious or accidental symlink inside a plugin tree can cause inspection to traverse outside the plugin root, potentially scanning large/unexpected parts of the filesystem or hitting cycles.
Use EnumerationOptions with IgnoreInaccessible=true and AttributesToSkip=FileAttributes.ReparsePoint (as PluginDiscovery does) to keep inspection bounded to real directories.
private static IEnumerable<string> EnumeratePluginSourceFiles(string rootPath)
{
var pending = new Stack<string>();
pending.Push(rootPath);
while (pending.Count > 0)
{
var directory = pending.Pop();
foreach (var child in Directory.EnumerateDirectories(directory))
{
var name = Path.GetFileName(child);
if (name is not "node_modules" and not ".git")
pending.Push(child);
}
foreach (var file in Directory.EnumerateFiles(directory)
.Where(file => Path.GetExtension(file) is ".js" or ".mjs" or ".cjs" or ".ts"))
yield return file;
}
src/OpenClaw.Core/Plugins/PluginPackageCompatibility.cs:81
ValidateFloortreats "<=" as a valid prefix operator and then compares the parsed version as if it were a minimum floor. This can incorrectly block plugins that declare upper-bounded ranges (e.g. "<=2026.7.0") by reporting them as requiring a newer host/plugin API version.
Treat "<"/"<=" ranges as invalid for these minimum floor checks (or ignore them) so we only enforce true minimum constraints like ">=", "^", "~", "=".
var normalized = declaredRange.Trim();
foreach (var rangeOperator in new[] { ">=", "<=", "==", ">", "=", "^", "~" })
{
if (!normalized.StartsWith(rangeOperator, StringComparison.Ordinal))
continue;
normalized = normalized[rangeOperator.Length..].Trim();
break;
}
src/OpenClaw.Dashboard/Pages/Ops.razor:475
PluginAccentcurrently returns the “enabled” color whenever the plugin is not disabled/quarantined, even if it failed to load. In the updated grid, that makes the status dot show as healthy/active (green) while the Status chip and label can say “Not loaded”, which is inconsistent and can mislead operators.
Consider aligning the accent dot with the same status logic as the chip (quarantined → red, loaded+enabled → green, otherwise → gray).
private static string PluginAccent(PluginInfo p)
=> p.Enabled ? "#10b981" : "#475569";
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/OpenClaw.Core/Plugins/PluginDiscovery.cs:594
FindEntryFileusesDirectory.GetFiles(pluginRoot, ext)without handling IO/permission failures. If the plugin root is partially inaccessible (ACLs, transient IO issues), plugin discovery can throw and abort scanning instead of reporting a diagnostic / continuing best-effort.
// Fallback: any .ts, .js, or .mjs file in root
foreach (var ext in new[] { "*.js", "*.mjs", "*.cjs", "*.ts" })
{
var files = Directory.GetFiles(pluginRoot, ext);
if (files.Length == 1)
return files[0];
}
src/OpenClaw.Core/Plugins/PluginPackageCompatibility.cs:81
ValidateFloortreats ranges starting with "<"/"<=" as if they were minimum-version floors by stripping the operator and comparing the parsed version againstsupportedVersion. That can silently accept or reject plugins based on an unrelated maximum constraint (e.g., "<=2026.1.0"), which is not a floor and is likely not intended for theseopenclaw.compat.*fields.
var normalized = declaredRange.Trim();
foreach (var rangeOperator in new[] { ">=", "<=", "==", ">", "=", "^", "~" })
{
Summary
api.registerCli()compatibility with safe lazy root dispatch, built-in precedence, duplicate detection, quarantine/enablement checks, bounded one-shot Node execution, terminal inheritance, and common Commander-style arguments/options/actionsWhy
The existing bridge rejected every plugin that called
registerCli(), classified compatible content bundles as unsupported native plugins, and could replace a working installation before proving the staged plugin initialized successfully. This made ecosystem compatibility narrower and less trustworthy than the underlying runtime capabilities.This change adds useful upstream interoperability while keeping unsupported remote gateway methods fail-closed and preserving the existing security boundary around bundle code, operator state, command collisions, and package/runtime validation.
User impact
Users can invoke supported plugin-owned root commands such as:
Built-in commands always win. Disabled or quarantined plugins are not eligible, duplicate roots fail closed, arguments are passed without shell interpolation, and commands time out after ten minutes.
Plugin commands that hard-code upstream-specific configuration paths may still require manual reconciliation with the configured OpenClaw.NET settings path; the user guide calls this out explicitly.
Validation
dotnet test src/OpenClaw.Tests/OpenClaw.Tests.csproj --no-restore— 2,461 passedOPENCLAW_PUBLIC_SMOKE=1pinned public-package smoke suite — passedosx-arm64plus executable help smoke — passednode --check src/OpenClaw.Agent/Plugins/plugin-bridge.mjs— passedgit diff --check— passedSummary by CodeRabbit
New Features
Bug Fixes
Documentation