ultanio/cobot

Fork 4

CLI command registration bypasses the plugin system #75

New issue

Closed

opened 2026-02-22 12:11:32 +00:00 by nazim · 1 comment

nazim commented

2026-02-22 12:11:32 +00:00

Contributor

Problem

CLI command registration is baked into the Plugin base class and uses raw iteration instead of the plugin system's extension points.

Current flow (cli.py, line 663):

for plugin_class in plugin_classes:
    instance = plugin_class()
    instance.register_commands(cli)  # base class method, raw loop

What's wrong:

register_commands() is a method on Plugin base class (base.py, line 207) — every plugin inherits it whether it uses CLI or not
cli.py iterates over ALL plugins and calls it — no capabilities, no extension points, no graph visibility
Plugins that provide CLI commands (cron, memory, pairing, subagent) have no way to declare this in PluginMeta
The plugin graph (cobot plugins inspect) can only detect CLI contribution via method override heuristics — it's invisible to the architecture

Why it matters:
This is exactly the "hidden coupling" anti-pattern from the plugin design guide. CLI contribution is real coupling that should be declared, not hidden in base class inheritance.

Proposed Solution

Model CLI as a plugin with an extension point. Same pattern as loop.transform_history or context.system_prompt.

1. Create a `cli` plugin

class CLIPlugin(Plugin):
    meta = PluginMeta(
        id="cli",
        version="1.0.0",
        extension_points=["cli.commands"],
        priority=5,  # Foundation layer — load early
    )

2. Plugins declare their CLI contribution

# cron/plugin.py
meta = PluginMeta(
    id="cron",
    implements={"cli.commands": "register_commands"},
    ...
)

Same for memory, pairing, subagent, and any future CLI-contributing plugin.

3. CLI plugin collects commands via extension point

# cli plugin startup or cli.py init
results = registry.call_extension("cli.commands", cli=click_group)

Instead of iterating all plugins and calling a base class method.

4. Remove `register_commands()` from `Plugin` base class

It shouldn't be on the base class — not every plugin needs CLI. Move it to an explicit opt-in via implements.

Acceptance Criteria

register_commands() removed from Plugin base class in base.py
New cli plugin created with extension_points: ["cli.commands"]
Existing CLI plugins (cron, memory, pairing, subagent) declare implements: {"cli.commands": "register_commands"}
cli.py uses call_extension("cli.commands", ...) instead of raw plugin iteration
Plugin graph shows CLI contribution as implements edges to the cli plugin
All existing CLI commands still work
Existing tests pass

Plugin Design Guide (docs/plugin-design-guide.md) — Principle 2: Declare All Coupling
#54 — Plugin introspection (graph should show these edges)

## Problem CLI command registration is baked into the `Plugin` base class and uses raw iteration instead of the plugin system's extension points. **Current flow** (`cli.py`, line 663): ```python for plugin_class in plugin_classes: instance = plugin_class() instance.register_commands(cli) # base class method, raw loop ``` **What's wrong:** 1. `register_commands()` is a method on `Plugin` base class (`base.py`, line 207) — every plugin inherits it whether it uses CLI or not 2. `cli.py` iterates over ALL plugins and calls it — no capabilities, no extension points, no graph visibility 3. Plugins that provide CLI commands (cron, memory, pairing, subagent) have no way to declare this in `PluginMeta` 4. The plugin graph (`cobot plugins inspect`) can only detect CLI contribution via method override heuristics — it's invisible to the architecture **Why it matters:** This is exactly the "hidden coupling" anti-pattern from the plugin design guide. CLI contribution is real coupling that should be declared, not hidden in base class inheritance. ## Proposed Solution Model CLI as a plugin with an extension point. Same pattern as `loop.transform_history` or `context.system_prompt`. ### 1. Create a `cli` plugin ```python class CLIPlugin(Plugin): meta = PluginMeta( id="cli", version="1.0.0", extension_points=["cli.commands"], priority=5, # Foundation layer — load early ) ``` ### 2. Plugins declare their CLI contribution ```python # cron/plugin.py meta = PluginMeta( id="cron", implements={"cli.commands": "register_commands"}, ... ) ``` Same for memory, pairing, subagent, and any future CLI-contributing plugin. ### 3. CLI plugin collects commands via extension point ```python # cli plugin startup or cli.py init results = registry.call_extension("cli.commands", cli=click_group) ``` Instead of iterating all plugins and calling a base class method. ### 4. Remove `register_commands()` from `Plugin` base class It shouldn't be on the base class — not every plugin needs CLI. Move it to an explicit opt-in via `implements`. ## Acceptance Criteria - [ ] `register_commands()` removed from `Plugin` base class in `base.py` - [ ] New `cli` plugin created with `extension_points: ["cli.commands"]` - [ ] Existing CLI plugins (cron, memory, pairing, subagent) declare `implements: {"cli.commands": "register_commands"}` - [ ] `cli.py` uses `call_extension("cli.commands", ...)` instead of raw plugin iteration - [ ] Plugin graph shows CLI contribution as `implements` edges to the `cli` plugin - [ ] All existing CLI commands still work - [ ] Existing tests pass ## Related - Plugin Design Guide (docs/plugin-design-guide.md) — Principle 2: Declare All Coupling - #54 — Plugin introspection (graph should show these edges)

doxios commented

2026-02-22 12:14:19 +00:00

Collaborator

Architecture Review: CLI Extension Point Proposal

Reviewer: Plugin Architecture Subagent

Verdict: ✅ APPROVE (with minor suggestions)

This is a well-reasoned proposal that correctly identifies a real architectural inconsistency and proposes a clean solution following established patterns.

Analysis

1. Problem Verification ✅

The problem is real and accurately described:

register_commands() is defined on Plugin base class (line 112 in base.py)
cli.py iterates ALL plugins via raw loop in register_plugin_commands() (line ~495)
Currently only 2 plugins implement it: memory and pairing
CLI contribution is invisible to the plugin graph — it cannot be inferred from PluginMeta

This is indeed "hidden coupling" — a plugin's CLI contribution is only discoverable by method introspection, not by declared metadata.

2. Proposed Solution Assessment ✅

The cli.commands extension point follows the established pattern already used by:

loop.on_message, loop.transform_history, etc.
memory.store, memory.search, etc.
context.system_prompt

Using implements: {"cli.commands": "register_commands"} is consistent with how other inter-plugin communication works. The plugin graph will correctly show edges from CLI-contributing plugins to the cli plugin.

3. Implementation Scope ✅

Current CLI-contributing plugins are:

memory/plugin.py — cobot memory {store,get,search,list}
pairing/plugin.py — pairing commands

(Note: The issue mentions cron and subagent, but these don't appear to have register_commands implementations in current codebase)

Migration is low-risk — only 2 plugins need implements declarations added.

Suggestions

1. Deprecation Period for Backward Compatibility

Removing register_commands() from base class is a breaking change for any external plugins. Consider:

# Phase 1: Keep base method but log deprecation warning
def register_commands(self, cli) -> None:
    if type(self).register_commands is not Plugin.register_commands:
        import warnings
        warnings.warn(
            f"{self.meta.id}: register_commands() is deprecated. "
            "Use implements: {{\"cli.commands\": \"register_commands\"}} in PluginMeta.",
            DeprecationWarning
        )

Remove the base method entirely in v0.4.0 or after one release cycle.

2. CLI Plugin Load Order

With priority: 5, the CLI plugin loads early. Ensure CLI command registration happens after all plugins are discovered but before CLI execution. The current register_plugin_commands() flow seems correct — just verify the timing with extension point dispatch.

3. Consider `capabilities: ["cli"]`

For consistency with other capabilities (["llm"], ["storage"]), you could also add:

meta = PluginMeta(
    id="memory",
    capabilities=["cli"],  # optional: declares "I provide CLI commands"
    implements={"cli.commands": "register_commands"},
    ...
)

The implements declaration is the authoritative one; capabilities is just for quick filtering.

Acceptance Criteria Assessment

Criterion	Status
Remove `register_commands()` from base class	🟡 Add deprecation phase
Create `cli` plugin with extension point	✅ Straightforward
Update memory, pairing with `implements`	✅ 2 files, minimal change
Update `cli.py` to use `call_extension`	✅ Cleaner than raw loop
Plugin graph shows CLI edges	✅ Will work automatically
Existing CLI commands work	✅ No functional change
Existing tests pass	✅ Expected

Conclusion

This refactoring makes CLI contribution explicit and discoverable, aligning with the extension point architecture used elsewhere. The implementation scope is small (2 plugins + new CLI plugin + cli.py changes).

Recommended approach:

Add cli plugin with extension_points: ["cli.commands"]
Add implements to memory and pairing plugins
Update cli.py to use extension dispatch
Deprecate (don't remove yet) base class register_commands()
Remove base method in next minor version

Ready for implementation. 👍

## Architecture Review: CLI Extension Point Proposal **Reviewer:** Plugin Architecture Subagent --- ### Verdict: ✅ APPROVE (with minor suggestions) This is a well-reasoned proposal that correctly identifies a real architectural inconsistency and proposes a clean solution following established patterns. --- ### Analysis #### 1. Problem Verification ✅ The problem is **real and accurately described**: - `register_commands()` is defined on `Plugin` base class (line 112 in `base.py`) - `cli.py` iterates ALL plugins via raw loop in `register_plugin_commands()` (line ~495) - Currently only 2 plugins implement it: `memory` and `pairing` - CLI contribution is invisible to the plugin graph — it cannot be inferred from `PluginMeta` This is indeed "hidden coupling" — a plugin's CLI contribution is only discoverable by method introspection, not by declared metadata. #### 2. Proposed Solution Assessment ✅ The `cli.commands` extension point follows the **established pattern** already used by: - `loop.on_message`, `loop.transform_history`, etc. - `memory.store`, `memory.search`, etc. - `context.system_prompt` Using `implements: {"cli.commands": "register_commands"}` is consistent with how other inter-plugin communication works. The plugin graph will correctly show edges from CLI-contributing plugins to the `cli` plugin. #### 3. Implementation Scope ✅ Current CLI-contributing plugins are: - `memory/plugin.py` — `cobot memory {store,get,search,list}` - `pairing/plugin.py` — pairing commands (Note: The issue mentions cron and subagent, but these don't appear to have `register_commands` implementations in current codebase) **Migration is low-risk** — only 2 plugins need `implements` declarations added. --- ### Suggestions #### 1. Deprecation Period for Backward Compatibility Removing `register_commands()` from base class is a **breaking change** for any external plugins. Consider: ```python # Phase 1: Keep base method but log deprecation warning def register_commands(self, cli) -> None: if type(self).register_commands is not Plugin.register_commands: import warnings warnings.warn( f"{self.meta.id}: register_commands() is deprecated. " "Use implements: {{\"cli.commands\": \"register_commands\"}} in PluginMeta.", DeprecationWarning ) ``` Remove the base method entirely in v0.4.0 or after one release cycle. #### 2. CLI Plugin Load Order With `priority: 5`, the CLI plugin loads early. Ensure CLI command registration happens **after** all plugins are discovered but **before** CLI execution. The current `register_plugin_commands()` flow seems correct — just verify the timing with extension point dispatch. #### 3. Consider `capabilities: ["cli"]` For consistency with other capabilities (`["llm"]`, `["storage"]`), you could also add: ```python meta = PluginMeta( id="memory", capabilities=["cli"], # optional: declares "I provide CLI commands" implements={"cli.commands": "register_commands"}, ... ) ``` The `implements` declaration is the authoritative one; `capabilities` is just for quick filtering. --- ### Acceptance Criteria Assessment | Criterion | Status | |-----------|--------| | Remove `register_commands()` from base class | 🟡 Add deprecation phase | | Create `cli` plugin with extension point | ✅ Straightforward | | Update memory, pairing with `implements` | ✅ 2 files, minimal change | | Update `cli.py` to use `call_extension` | ✅ Cleaner than raw loop | | Plugin graph shows CLI edges | ✅ Will work automatically | | Existing CLI commands work | ✅ No functional change | | Existing tests pass | ✅ Expected | --- ### Conclusion This refactoring makes CLI contribution **explicit and discoverable**, aligning with the extension point architecture used elsewhere. The implementation scope is small (2 plugins + new CLI plugin + cli.py changes). **Recommended approach:** 1. Add `cli` plugin with `extension_points: ["cli.commands"]` 2. Add `implements` to memory and pairing plugins 3. Update `cli.py` to use extension dispatch 4. Deprecate (don't remove yet) base class `register_commands()` 5. Remove base method in next minor version **Ready for implementation.** 👍