diff --git a/docs/plans/2026-02-28-plugin-system-phase2-phase3-design.md b/docs/plans/2026-02-28-plugin-system-phase2-phase3-design.md new file mode 100644 index 000000000..464cfc487 --- /dev/null +++ b/docs/plans/2026-02-28-plugin-system-phase2-phase3-design.md @@ -0,0 +1,235 @@ +# PR #473 Phase 2/3 Rethink + +## What Changed + +The previous draft mixed control-plane work with schema-heavy plugin config too early. +This rethink narrows scope so implementation matches the current codebase: + +- Phase 2 is only plugin selection and runtime wiring. +- Phase 3 is introspection, linting, and optional plugin-specific settings. +- `plugin.Plugin` stays unchanged in both phases. + +## Hard Scope + +- In-process built-in plugins only. +- No runtime loader, no dynamic module loading, no hot reload. +- Runtime loading remains Phase 4 and must reuse Phase 2/3 abstractions. + +## Baseline + +- Baseline is PR #473 phase-0/phase-1 behavior (`pkg/plugin`, `pkg/hooks`, `pkg/agent/loop.go`). +- Existing deployments without `plugins` config must keep the same effective behavior. + +## Implemented Snapshot + +Implemented in current Phase 2/3 scope: + +- Phase 2: +- typed plugin config schema with `plugins.default_enabled`, `plugins.enabled`, `plugins.disabled`. +- deterministic plugin resolver. +- startup wiring in both `agent` and `gateway` paths. +- Phase 3: +- plugin metadata introspection for built-ins. +- CLI support for listing and linting plugin config. + +Command examples: + +```bash +picoclaw plugin list +picoclaw plugin list --format json +picoclaw plugin lint --config ~/.picoclaw/config.json +``` + +Precision note: + +- No dynamic runtime plugin loading/hot reload. +- Plugin-specific `settings` remain optional future work and are not required by the implemented Phase 2 selection plane. + +## Phase 2: Selection Plane (Minimal, Deterministic) + +### Goal + +Make plugin enable/disable operational from config with deterministic behavior and fail-fast handling. + +### Config (Phase 2 only) + +Project-facing examples should follow the repo default config format (JSON). + +```json +{ + "plugins": { + "default_enabled": true, + "enabled": ["policy-demo"], + "disabled": ["legacy_policy"] + } +} +``` + +No plugin-specific `settings` in Phase 2. + +### Resolution Rules (authoritative) + +For each built-in plugin name in sorted order: + +1. normalize names (`trim`, `lowercase`) for matching. +2. if in `disabled`, mark disabled. +3. else if `enabled` list is non-empty, enable only if listed. +4. else enable only if `default_enabled=true`. + +### Error Policy (Phase 2) + +- unknown name in `enabled`: startup error. +- unknown name in `disabled`: warning only. +- duplicates after normalization: dedupe and warn. +- overlap between `enabled` and `disabled`: disabled wins. + +### Required Code Changes + +- `pkg/config/config.go`, `pkg/config/defaults.go` + - add typed `plugins` block (`default_enabled`, `enabled`, `disabled`). +- `pkg/plugin/manager.go` + - add built-in registry map and deterministic resolver. + - expose resolution result buckets (`enabled`, `disabled`, `unknown`). +- `cmd/picoclaw/internal/agent/helpers.go` + - resolve plugins from config and wire into `loop.EnablePlugins(...)`. +- `cmd/picoclaw/internal/gateway/helpers.go` + - same as agent path. +- `pkg/agent/loop.go` + - keep existing plugin interface and lifecycle. + - add startup diagnostics with final resolved plugin names. + +### PR Slicing (Review-Friendly) + +Keep Phase 2 and Phase 3 as separate PR series. + +1. Phase 2 PR-A: config schema + resolver only. +2. Phase 2 PR-B: agent/gateway startup wiring + startup diagnostics. +3. Phase 2 PR-C: tests + docs updates. +4. Phase 3 PR-A: manager introspection + metadata side interface. +5. Phase 3 PR-B: `plugin list` and `plugin lint` commands. +6. Phase 3 PR-C: observability polishing + integration tests + docs. + +### Phase 2 Acceptance Gate + +- No `plugins` block: behavior matches baseline. +- Unknown name in `enabled` fails startup with actionable message. +- Resolution order and result are deterministic. +- Entry points actually wire resolved plugins (no silent no-op). +- Startup logs show enabled/disabled plugin sets. + +### Phase 2 Test Matrix + +1. No `plugins` block. +- Expected: same enabled plugin set as baseline. +- Expected log keys: `plugins.enabled=[]`, `plugins.disabled=[]`, `plugins.mode=baseline`. +2. `enabled=["policy-demo"]`, empty `disabled`. +- Expected: only `policy-demo` loaded. +- Expected log keys: `plugins.enabled=["policy-demo"]`, `plugins.disabled=[]`. +3. `disabled=["policy-demo"]`, empty `enabled`. +- Expected: `policy-demo` not loaded. +- Expected log keys: `plugins.enabled=[]`, `plugins.disabled=["policy-demo"]`. +4. Overlap: `enabled=["policy-demo"]`, `disabled=["policy-demo"]`. +- Expected: plugin disabled (disabled wins). +- Expected log keys: `plugins.conflict=["policy-demo"]`, `plugins.disabled=["policy-demo"]`. +5. Unknown in `enabled`: `enabled=["not_exists"]`. +- Expected: startup fails. +- Expected error text includes: `unknown plugin in enabled`. +6. Unknown in `disabled`: `disabled=["not_exists"]`. +- Expected: startup continues with warning. +- Expected warning text includes: `unknown plugin in disabled`. +7. Duplicates/case variants: `enabled=["Policy_Demo","policy_demo"]`. +- Expected: deduped after normalization and warning emitted. +- Expected warning text includes: `duplicate plugin name after normalization`. + +## Phase 3: Introspection Plane (DX + Validation) + +### Goal + +Make plugin state inspectable and config validation review-friendly. + +### Capabilities + +- Manager introspection: + - `DescribeAll() []PluginInfo` + - `DescribeEnabled() []PluginInfo` +- Optional metadata side interface for plugins: + +```go +type PluginDescriptor interface { + Info() PluginInfo +} +``` + +- CLI: + - `picoclaw plugin list` (text/json). + - `picoclaw plugin lint --config `. +- Diagnostics: + - structured startup fields: `plugin`, `status`, `disabled_reason`, `error_code`. + - hook invocation outcome fields: `plugin`, `hook`, `result`, `duration_ms`. + +### Optional Phase 3 Extension + +If needed after list/lint lands, introduce plugin-specific `settings` with strict schema validation. +This is explicitly Phase 3, not Phase 2. + +### Phase 3 Acceptance Gate + +- `plugin list` output is stable in text and JSON. +- `plugin lint` returns non-zero on invalid plugin names/config. +- Logs/events include plugin resolution and hook invocation outcomes. +- Tests cover one disabled path and one lint failure path. + +### Phase 3 Test Matrix + +1. `picoclaw plugin list` text output. +- Expected: deterministic ordering and fields: `name`, `status`, `api_version`. +2. `picoclaw plugin list --format json`. +- Expected: stable JSON schema and deterministic ordering. +3. `picoclaw plugin lint --config ` valid config. +- Expected: exit code `0`. +4. `picoclaw plugin lint --config ` invalid plugin name. +- Expected: non-zero exit. +- Expected error text includes: `unknown plugin`. +5. Hook invocation with disabled plugin. +- Expected: no callback execution from disabled plugin. +- Expected diagnostic includes: `disabled_reason`. + +## Rollback Runbook + +1. Revert to baseline behavior. +- Action: remove `plugins` block from config and restart process. +- Success signal: startup logs contain `plugins.mode=baseline`. +2. Recover from bad selection config. +- Action: clear `plugins.enabled` and `plugins.disabled`, restart. +- Success signal: startup logs contain resolved set equal to baseline set. +3. Recover from Phase 3 command regressions. +- Action: disable plugin command surface with feature flag and restart. +- Success signal: `plugin` command group hidden/disabled in CLI help. +4. Incident confirmation checks. +- Verify startup logs include: +- `plugins.enabled` +- `plugins.disabled` +- `plugins.unknown_enabled` +- `plugins.unknown_disabled` +- `plugins.resolution_status` + +## Why This Is More Sound + +- Matches current interfaces (`EnablePlugins` with concrete plugin instances). +- Avoids premature schema coupling before metadata/lint tooling exists. +- Eliminates silent rollout risk by making entrypoint wiring a Phase 2 gate. +- Keeps a clean migration path to Phase 4 runtime sources. + +## External Alignment + +This phase split aligns with common Go OSS progression: +- compile-time plugin selection first +- discovery/validation CLI second +- runtime loader and trust model last + +Reference patterns: +- Go `plugin` package caveats: `https://pkg.go.dev/plugin` +- HashiCorp `go-plugin` runtime model: `https://pkg.go.dev/github.com/hashicorp/go-plugin` +- module listing/validation command patterns (Caddy/Terraform style): + - `https://caddyserver.com/docs/command-line` + - `https://developer.hashicorp.com/terraform/cli/commands/providers/schema` diff --git a/docs/plans/2026-03-01-plugin-system-phase2-phase3-implementation.md b/docs/plans/2026-03-01-plugin-system-phase2-phase3-implementation.md new file mode 100644 index 000000000..09a9e6b79 --- /dev/null +++ b/docs/plans/2026-03-01-plugin-system-phase2-phase3-implementation.md @@ -0,0 +1,576 @@ +# Plugin System Phase 2/3 Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Implement Phase 2 (config-driven plugin selection + runtime wiring) and Phase 3 (plugin introspection + CLI list/lint + diagnostics) with small, reviewable PRs. + +**Architecture:** Keep in-process compile-time plugins as the only runtime model. Build a deterministic selection plane first (`config` + `plugin` resolver + bootstrap wiring), then add a non-breaking introspection plane (`PluginInfo`, list/lint commands) without changing `plugin.Plugin` contract. Use TDD for each slice and keep changes split into small commits. + +**Tech Stack:** Go, Cobra CLI, Testify, existing PicoClaw `pkg/config`, `pkg/plugin`, `pkg/agent`, `cmd/picoclaw/internal/*`. + +--- + +## Preflight Notes + +- Execute this plan inside the dedicated worktree created during design/brainstorming. +- Use `@test-driven-development` for every task (`red -> green -> refactor`). +- Before final handoff, use `@verification-before-completion`. +- Before opening PRs, use `@requesting-code-review`. + +### Task 1: Add Plugin Config Schema and Defaults (Phase 2) + +**Files:** +- Modify: `pkg/config/config.go` +- Modify: `pkg/config/defaults.go` +- Test: `pkg/config/config_test.go` + +**Step 1: Write the failing test** + +```go +func TestDefaultConfig_PluginsDefaults(t *testing.T) { + cfg := DefaultConfig() + if !cfg.Plugins.DefaultEnabled { + t.Fatal("plugins.default_enabled should default to true") + } + if len(cfg.Plugins.Enabled) != 0 || len(cfg.Plugins.Disabled) != 0 { + t.Fatal("plugins enabled/disabled should default empty") + } +} + +func TestConfig_PluginsJSONUnmarshal(t *testing.T) { + cfg := DefaultConfig() + err := json.Unmarshal([]byte(`{"plugins":{"default_enabled":false,"enabled":["policy-demo"],"disabled":["x"]}}`), cfg) + if err != nil { + t.Fatalf("unmarshal: %v", err) + } + if cfg.Plugins.DefaultEnabled { + t.Fatal("expected default_enabled=false from JSON") + } +} +``` + +**Step 2: Run test to verify it fails** + +Run: `go test ./pkg/config -run 'TestDefaultConfig_PluginsDefaults|TestConfig_PluginsJSONUnmarshal' -v` +Expected: FAIL with `cfg.Plugins undefined`. + +**Step 3: Write minimal implementation** + +```go +type PluginsConfig struct { + DefaultEnabled bool `json:"default_enabled"` + Enabled []string `json:"enabled,omitempty"` + Disabled []string `json:"disabled,omitempty"` +} + +type Config struct { + // ...existing fields... + Plugins PluginsConfig `json:"plugins,omitempty"` +} +``` + +```go +Plugins: PluginsConfig{ + DefaultEnabled: true, + Enabled: []string{}, + Disabled: []string{}, +}, +``` + +**Step 4: Run test to verify it passes** + +Run: `go test ./pkg/config -run 'TestDefaultConfig_PluginsDefaults|TestConfig_PluginsJSONUnmarshal' -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add pkg/config/config.go pkg/config/defaults.go pkg/config/config_test.go +git commit -m "feat(config): add plugins selection config schema" +``` + +### Task 2: Build Deterministic Selection Resolver (Phase 2) + +**Files:** +- Modify: `pkg/plugin/manager.go` +- Test: `pkg/plugin/manager_test.go` + +**Step 1: Write the failing test** + +```go +func TestResolveSelection_DefaultEnabled(t *testing.T) {} +func TestResolveSelection_EnabledListOnly(t *testing.T) {} +func TestResolveSelection_DisabledWinsOverlap(t *testing.T) {} +func TestResolveSelection_UnknownEnabledFails(t *testing.T) {} +func TestResolveSelection_UnknownDisabledWarns(t *testing.T) {} +func TestResolveSelection_NormalizeAndDedupe(t *testing.T) {} +``` + +In each test, assert deterministic sorted resolution and expected error/warning behavior. + +**Step 2: Run test to verify it fails** + +Run: `go test ./pkg/plugin -run 'TestResolveSelection_' -v` +Expected: FAIL with undefined resolver types/functions. + +**Step 3: Write minimal implementation** + +```go +type SelectionInput struct { + DefaultEnabled bool + Enabled []string + Disabled []string +} + +type SelectionResult struct { + EnabledNames []string + DisabledNames []string + UnknownEnabled []string + UnknownDisabled []string + Warnings []string +} + +func NormalizePluginName(s string) string { /* strings.TrimSpace + strings.ToLower */ } +func ResolveSelection(available []string, in SelectionInput) (SelectionResult, error) { /* deterministic rules */ } +``` + +Rules implemented exactly: +- unknown in `enabled` => error +- unknown in `disabled` => warning bucket +- overlap => disabled wins +- sorted deterministic output + +**Step 4: Run test to verify it passes** + +Run: `go test ./pkg/plugin -run 'TestResolveSelection_' -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add pkg/plugin/manager.go pkg/plugin/manager_test.go +git commit -m "feat(plugin): add deterministic plugin selection resolver" +``` + +### Task 3: Add Built-in Plugin Catalog Without Import Cycles (Phase 2) + +**Files:** +- Create: `pkg/plugin/builtin/catalog.go` +- Test: `pkg/plugin/builtin/catalog_test.go` + +**Step 1: Write the failing test** + +```go +func TestCatalogContainsPolicyDemo(t *testing.T) { + c := Catalog() + fn, ok := c["policy-demo"] + if !ok { + t.Fatal("expected policy-demo in builtin catalog") + } + if fn() == nil { + t.Fatal("expected non-nil plugin instance") + } +} +``` + +**Step 2: Run test to verify it fails** + +Run: `go test ./pkg/plugin/builtin -run TestCatalogContainsPolicyDemo -v` +Expected: FAIL with package/file missing. + +**Step 3: Write minimal implementation** + +```go +package builtin + +import ( + "github.com/sipeed/picoclaw/pkg/plugin" + "github.com/sipeed/picoclaw/pkg/plugin/demoplugin" +) + +type Factory func() plugin.Plugin + +func Catalog() map[string]Factory { + return map[string]Factory{ + "policy-demo": func() plugin.Plugin { + return demoplugin.NewPolicyDemoPlugin(demoplugin.PolicyDemoConfig{}) + }, + } +} + +func Names() []string { /* return sorted keys */ } +``` + +**Step 4: Run test to verify it passes** + +Run: `go test ./pkg/plugin/builtin -run TestCatalogContainsPolicyDemo -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add pkg/plugin/builtin/catalog.go pkg/plugin/builtin/catalog_test.go +git commit -m "feat(plugin): add builtin plugin catalog package" +``` + +### Task 4: Add Bootstrap Resolver Module for Agent/Gateway (Phase 2) + +**Files:** +- Create: `cmd/picoclaw/internal/pluginruntime/bootstrap.go` +- Test: `cmd/picoclaw/internal/pluginruntime/bootstrap_test.go` + +**Step 1: Write the failing test** + +```go +func TestResolveConfiguredPlugins_UnknownEnabledReturnsError(t *testing.T) {} +func TestResolveConfiguredPlugins_ReturnsDeterministicInstances(t *testing.T) {} +func TestResolveConfiguredPlugins_UnknownDisabledWarns(t *testing.T) {} +``` + +**Step 2: Run test to verify it fails** + +Run: `go test ./cmd/picoclaw/internal/pluginruntime -run 'TestResolveConfiguredPlugins_' -v` +Expected: FAIL with package/file missing. + +**Step 3: Write minimal implementation** + +```go +type Summary struct { + Enabled []string + Disabled []string + UnknownEnabled []string + UnknownDisabled []string + Warnings []string +} + +func ResolveConfiguredPlugins(cfg *config.Config) ([]plugin.Plugin, Summary, error) { + // 1) get catalog names from builtin.Names() + // 2) call plugin.ResolveSelection(...) + // 3) instantiate enabled plugins via builtin.Catalog factories + // 4) return instances + summary +} +``` + +**Step 4: Run test to verify it passes** + +Run: `go test ./cmd/picoclaw/internal/pluginruntime -run 'TestResolveConfiguredPlugins_' -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add cmd/picoclaw/internal/pluginruntime/bootstrap.go cmd/picoclaw/internal/pluginruntime/bootstrap_test.go +git commit -m "feat(cli): add plugin runtime bootstrap resolver" +``` + +### Task 5: Wire Phase 2 Plugin Bootstrap into `agent` and `gateway` + +**Files:** +- Modify: `cmd/picoclaw/internal/agent/helpers.go` +- Modify: `cmd/picoclaw/internal/gateway/helpers.go` +- Test: `cmd/picoclaw/internal/agent/command_test.go` +- Test: `cmd/picoclaw/internal/gateway/command_test.go` + +**Step 1: Write the failing test** + +Add focused assertions that command constructors remain stable after importing plugin bootstrap package and wiring helper calls (no regressions in command metadata). +If needed, add table-driven compile/runtime smoke tests in a new `_test.go` under each package. + +**Step 2: Run test to verify it fails** + +Run: `go test ./cmd/picoclaw/internal/agent ./cmd/picoclaw/internal/gateway -run 'TestNew.*Command|Test.*Plugin.*' -v` +Expected: FAIL once bootstrap calls are referenced but not integrated correctly. + +**Step 3: Write minimal implementation** + +```go +pluginsToEnable, summary, err := pluginruntime.ResolveConfiguredPlugins(cfg) +if err != nil { + return fmt.Errorf("resolve plugins: %w", err) +} +if len(pluginsToEnable) > 0 { + if err := agentLoop.EnablePlugins(pluginsToEnable...); err != nil { + return fmt.Errorf("enable plugins: %w", err) + } +} +logger.InfoCF("plugin", "Plugin selection resolved", map[string]any{ + "enabled": summary.Enabled, "disabled": summary.Disabled, + "unknown_disabled": summary.UnknownDisabled, +}) +``` + +**Step 4: Run test to verify it passes** + +Run: `go test ./cmd/picoclaw/internal/agent ./cmd/picoclaw/internal/gateway -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add cmd/picoclaw/internal/agent/helpers.go cmd/picoclaw/internal/gateway/helpers.go cmd/picoclaw/internal/agent/command_test.go cmd/picoclaw/internal/gateway/command_test.go +git commit -m "feat(cli): wire plugin selection into agent and gateway startup" +``` + +### Task 6: Expose Plugin Resolution in Startup Diagnostics (Phase 2) + +**Files:** +- Modify: `pkg/agent/loop.go` +- Test: `pkg/agent/loop_test.go` +- Test: `pkg/agent/plugin_test.go` + +**Step 1: Write the failing test** + +```go +func TestGetStartupInfo_IncludesPluginSummary(t *testing.T) { + // create AgentLoop, enable a test plugin, call GetStartupInfo + // assert "plugins" key exists with enabled list +} +``` + +**Step 2: Run test to verify it fails** + +Run: `go test ./pkg/agent -run 'TestGetStartupInfo_IncludesPluginSummary' -v` +Expected: FAIL because `plugins` section is absent. + +**Step 3: Write minimal implementation** + +```go +if al.pluginManager != nil { + info["plugins"] = map[string]any{ + "enabled": al.pluginManager.Names(), + "count": len(al.pluginManager.Names()), + } +} else { + info["plugins"] = map[string]any{ + "enabled": []string{}, + "count": 0, + } +} +``` + +**Step 4: Run test to verify it passes** + +Run: `go test ./pkg/agent -run 'TestGetStartupInfo_IncludesPluginSummary|TestSetPluginManagerInstallsHookRegistry' -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add pkg/agent/loop.go pkg/agent/loop_test.go pkg/agent/plugin_test.go +git commit -m "feat(agent): include plugin summary in startup diagnostics" +``` + +### Task 7: Add Non-Breaking Plugin Metadata Introspection (Phase 3) + +**Files:** +- Modify: `pkg/plugin/manager.go` +- Test: `pkg/plugin/manager_test.go` + +**Step 1: Write the failing test** + +```go +func TestDescribeAll_UsesDescriptorWhenImplemented(t *testing.T) {} +func TestDescribeAll_FallsBackForPlainPlugin(t *testing.T) {} +``` + +**Step 2: Run test to verify it fails** + +Run: `go test ./pkg/plugin -run 'TestDescribeAll_' -v` +Expected: FAIL with undefined `PluginInfo` / `DescribeAll`. + +**Step 3: Write minimal implementation** + +```go +type PluginInfo struct { + Name string `json:"name"` + APIVersion string `json:"api_version"` + Status string `json:"status"` +} + +type PluginDescriptor interface { + Info() PluginInfo +} + +func (m *Manager) DescribeAll() []PluginInfo { /* include fallback info */ } +func (m *Manager) DescribeEnabled() []PluginInfo { /* status=enabled */ } +``` + +**Step 4: Run test to verify it passes** + +Run: `go test ./pkg/plugin -run 'TestDescribeAll_|TestRegisterPluginAndTriggerHook' -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add pkg/plugin/manager.go pkg/plugin/manager_test.go +git commit -m "feat(plugin): add non-breaking plugin metadata introspection" +``` + +### Task 8: Add `picoclaw plugin list` Command (Phase 3) + +**Files:** +- Create: `cmd/picoclaw/internal/plugin/command.go` +- Create: `cmd/picoclaw/internal/plugin/list.go` +- Test: `cmd/picoclaw/internal/plugin/command_test.go` +- Test: `cmd/picoclaw/internal/plugin/list_test.go` +- Modify: `cmd/picoclaw/main.go` +- Modify: `cmd/picoclaw/main_test.go` + +**Step 1: Write the failing test** + +```go +func TestNewPluginCommand(t *testing.T) {} +func TestNewListSubcommand(t *testing.T) {} +func TestNewPicoclawCommand_IncludesPluginCommand(t *testing.T) {} +``` + +**Step 2: Run test to verify it fails** + +Run: `go test ./cmd/picoclaw/internal/plugin ./cmd/picoclaw -run 'TestNewPluginCommand|TestNewListSubcommand|TestNewPicoclawCommand' -v` +Expected: FAIL with missing package/command registration. + +**Step 3: Write minimal implementation** + +```go +func NewPluginCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "plugin", + Short: "Inspect and validate plugins", + RunE: func(cmd *cobra.Command, _ []string) error { return cmd.Help() }, + } + cmd.AddCommand(newListCommand()) + return cmd +} +``` + +`newListCommand()` should load config, resolve selection, and print text or JSON list. + +**Step 4: Run test to verify it passes** + +Run: `go test ./cmd/picoclaw/internal/plugin ./cmd/picoclaw -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add cmd/picoclaw/internal/plugin/command.go cmd/picoclaw/internal/plugin/list.go cmd/picoclaw/internal/plugin/command_test.go cmd/picoclaw/internal/plugin/list_test.go cmd/picoclaw/main.go cmd/picoclaw/main_test.go +git commit -m "feat(cli): add plugin list command" +``` + +### Task 9: Add `picoclaw plugin lint` Command (Phase 3) + +**Files:** +- Create: `cmd/picoclaw/internal/plugin/lint.go` +- Test: `cmd/picoclaw/internal/plugin/lint_test.go` +- Modify: `cmd/picoclaw/internal/plugin/command.go` +- Modify: `cmd/picoclaw/internal/pluginruntime/bootstrap.go` +- Modify: `cmd/picoclaw/internal/pluginruntime/bootstrap_test.go` + +**Step 1: Write the failing test** + +```go +func TestPluginLint_ValidConfigExitZero(t *testing.T) {} +func TestPluginLint_UnknownEnabledExitNonZero(t *testing.T) {} +``` + +**Step 2: Run test to verify it fails** + +Run: `go test ./cmd/picoclaw/internal/plugin ./cmd/picoclaw/internal/pluginruntime -run 'TestPluginLint_|TestResolveConfiguredPlugins_' -v` +Expected: FAIL with missing lint command/validation path. + +**Step 3: Write minimal implementation** + +```go +func newLintCommand() *cobra.Command { + var configPath string + cmd := &cobra.Command{ + Use: "lint", + Short: "Validate plugin configuration", + RunE: func(_ *cobra.Command, _ []string) error { + cfg, err := config.LoadConfig(configPath) + if err != nil { return err } + _, _, err = pluginruntime.ResolveConfiguredPlugins(cfg) + return err + }, + } + cmd.Flags().StringVar(&configPath, "config", internal.GetConfigPath(), "Path to config.json") + return cmd +} +``` + +**Step 4: Run test to verify it passes** + +Run: `go test ./cmd/picoclaw/internal/plugin ./cmd/picoclaw/internal/pluginruntime -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add cmd/picoclaw/internal/plugin/lint.go cmd/picoclaw/internal/plugin/lint_test.go cmd/picoclaw/internal/plugin/command.go cmd/picoclaw/internal/pluginruntime/bootstrap.go cmd/picoclaw/internal/pluginruntime/bootstrap_test.go +git commit -m "feat(cli): add plugin lint command" +``` + +### Task 10: Documentation and Final Verification + +**Files:** +- Modify: `docs/plugin-system-roadmap.md` +- Modify: `docs/plans/2026-02-28-plugin-system-phase2-phase3-design.md` +- Optional Modify: `README.md` (if command docs are surfaced there) + +**Step 1: Write docs-oriented failing checks** + +Add/update checklist assertions in docs PR description: +- Phase 2 gates explicitly checked. +- Phase 3 list/lint behavior and exit semantics documented. + +**Step 2: Run verification commands** + +Run: + +```bash +go test ./pkg/config ./pkg/plugin ./pkg/plugin/builtin ./pkg/agent ./cmd/picoclaw ./cmd/picoclaw/internal/plugin ./cmd/picoclaw/internal/pluginruntime -v +``` + +Expected: PASS. + +**Step 3: Minimal doc implementation** + +Document: +- JSON `plugins` config examples +- deterministic precedence rules +- `plugin list` usage (`--format json`) +- `plugin lint --config` usage and non-zero behavior + +**Step 4: Re-run verification** + +Run: + +```bash +go test ./pkg/config ./pkg/plugin ./pkg/plugin/builtin ./pkg/agent ./cmd/picoclaw ./cmd/picoclaw/internal/plugin ./cmd/picoclaw/internal/pluginruntime -v +``` + +Expected: PASS. + +**Step 5: Commit** + +```bash +git add docs/plugin-system-roadmap.md docs/plans/2026-02-28-plugin-system-phase2-phase3-design.md README.md +git commit -m "docs(plugin): document phase2/phase3 behavior and cli usage" +``` + +--- + +## PR Plan (maintainer-friendly) + +1. PR-1: Tasks 1-2 (`config` + resolver). +2. PR-2: Tasks 3-6 (catalog + bootstrap + startup diagnostics). +3. PR-3: Tasks 7-9 (metadata + plugin list/lint CLI). +4. PR-4: Task 10 docs-only cleanup if needed. + +Each PR should include: +- complete PR template fields +- AI disclosure +- test environment and command evidence +- unresolved comments = 0 before merge + diff --git a/docs/plugin-system-roadmap.md b/docs/plugin-system-roadmap.md new file mode 100644 index 000000000..a19a62efa --- /dev/null +++ b/docs/plugin-system-roadmap.md @@ -0,0 +1,119 @@ +# Plugin System Roadmap + +This document defines how PicoClaw evolves from hook-based extension points to a fuller plugin system in low-risk phases. + +## Current Status (Phase 0: Foundation) + +Implemented in current hooks PR: + +- Typed lifecycle hooks (`pkg/hooks`) +- Priority-based handler ordering +- Cancellation support for modifying hooks +- Panic recovery and error isolation +- Agent-loop integration via `agentLoop.SetHooks(...)` + +Compatibility: + +- If no hooks are registered, runtime behavior is unchanged. +- No config migration is required. + +## Non-Goals in Phase 0 + +- No dynamic runtime plugin loading +- No remote plugin marketplace/distribution +- No plugin sandboxing model +- No stable external plugin ABI yet +- No Go `.so` plugin loading as default direction + +## Phase Plan + +## Phase 1: Static Plugin Contract (Compile-time) — Implemented + +Goal: define a minimal public plugin contract for Go modules. + +Implemented: + +- Add `pkg/plugin` with a small interface: + - `Name() string` + - `APIVersion() string` + - `Register(*hooks.HookRegistry) error` +- Register plugins at startup in code. +- Add compatibility metadata (`plugin.APIVersion`) and registration-time checks. + +Exit criteria (met): + +- Example plugin module builds against the contract. +- Startup validation logs loaded plugins and registration errors clearly. + +## Phase 2: Config-driven Enable/Disable — Implemented + +Goal: operational control without code changes. + +Implemented: + +- Add typed plugin selection config in `config.json`: + - `plugins.default_enabled` + - `plugins.enabled` + - `plugins.disabled` +- Add deterministic plugin resolution and conflict handling in the plugin manager. +- Wire resolved plugins into startup for both `agent` and `gateway` entrypoints. + +Exit criteria (met): + +- Users can toggle built-in plugins without rebuilding. +- Invalid plugin selection in config is surfaced during startup/lint flow. + +## Phase 3: Metadata Introspection + CLI — Implemented + +Goal: make plugin state inspectable and config validation straightforward. + +Implemented: + +- Add plugin metadata introspection in the plugin manager. +- Add CLI inspection commands: + - `picoclaw plugin list` + - `picoclaw plugin list --format json` +- Add CLI lint command: + - `picoclaw plugin lint --config ` + +Exit criteria (met): + +- Operators can inspect plugin metadata in text/JSON outputs. +- Operators can validate plugin config before startup. + +## Future DX Work (Post-Phase 3) + +- Provide `examples/plugins/*` reference implementations. +- Publish plugin authoring guide (lifecycle map, best practices, safety constraints). +- Add plugin-focused test harness patterns for hook behavior verification. + +## Phase 4: Optional Dynamic Loading (Separate RFC) + +Goal: support runtime-loaded plugins only if security and operability are acceptable. + +Preferred direction: + +- Runtime plugins run as subprocesses. +- Host and plugin communicate via RPC/gRPC. +- Host manages lifecycle (spawn/health/timeout/restart), not in-process dynamic loading. + +Why this direction: + +- Go native `.so` plugin loading has strict toolchain/ABI coupling with host binary. +- Subprocess RPC model reduces coupling and improves fault isolation. +- Process boundary provides a cleaner place for permissions and sandbox controls. + +Preconditions: + +- Threat model approved +- Signature/trust model defined +- Sandboxing and permission boundaries defined +- Rollback and safe-disable behavior validated +- Versioned RPC handshake and capability negotiation defined +- Process supervision policy defined (timeouts, retries, crash loop backoff) + +Until then, compile-time registration remains the recommended model. + +## Maintainer Review Notes + +The current hooks PR should be reviewed as Phase 0+1 only. It intentionally establishes extension points while avoiding high-risk runtime plugin mechanics.