From 54066b05fdad401803e1d129dc6595f802879fbe Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Mon, 24 Aug 2026 01:54:16 +0200 Subject: [PATCH] docs(f3): plugin checklist + architecture requirements section in dev guide --- docs/plugin-checklist.md | 72 ++++++++++++++++++++++++++++ docs/plugin-development-guide.md | 80 ++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+) create mode 100644 docs/plugin-checklist.md diff --git a/docs/plugin-checklist.md b/docs/plugin-checklist.md new file mode 100644 index 0000000..c549ec1 --- /dev/null +++ b/docs/plugin-checklist.md @@ -0,0 +1,72 @@ +# Plugin Checklist — MUST-PASS before merge + +> Every new or changed plugin must pass this checklist. Each item maps to a +> failure class that actually occurred in this codebase (see PROGRESS.md +> architecture-repair section). CI runs the automated checks; reviewers verify +> the rest. + +## Automated checks (CI) + +- [ ] `python scripts/check_cross_plugin_imports.py` → **0 violations** + (no direct imports from other plugins — use `get_contract()`) +- [ ] `python scripts/check_migration_hashes.py` → OK + (migrations ≤0092 unchanged) +- [ ] `npx tsc --noEmit` clean (if frontend contributions changed) +- [ ] pytest smoke of affected suites passes + +## Architecture rules + +1. **No cross-plugin imports** — access other plugins only via + `from app.plugins.builtins.contracts import get_contract`. If a contract + is missing, define it in the owning plugin's `contracts.py`. + *Failure class: silent dead code when the imported symbol moved.* + +2. **Lifecycle symmetry** — everything registered in `on_activate` + (contracts, services, event handlers, hooks, tools, providers) must be + deregistered in `on_deactivate`, in the correct order: + own cleanup FIRST, then `super().on_deactivate()`. + *Failure class: stale services after deactivation; ImportError at every + deactivate because a helper was imported as module function.* + +3. **Every route has a permission** — no auth-only routes. Every + `FrontendMenuItem` and `FrontendPageRoute` carries its `permission` field. + *Failure class: dead guards checking permissions that don't exist.* + +4. **Declare dependencies** — if your plugin uses another plugin's data or + contracts, declare it in `manifest.dependencies`. Activation order is + topological; your plugin cannot be deactivated while dependents are active. + +5. **Entities via registration** — return models from `get_entity_models()`; + never assume core tables. The `/entity-permissions/registry` endpoint is + generated dynamically from these registrations. + +6. **Frontend components via manifest** — pages, menu items, settings pages, + detail tabs, dashboard widgets come from the manifest. Register new page + components in `frontend/src/components/plugins/PluginLoader.tsx` + STATIC_COMPONENT_MAP so production builds can chunk them. + *Failure class: ghost components — manifest references a component that + does not exist; tab shows error boundary in production.* + +7. **Migrations follow convention** — Alembic revisions touching plugin-owned + tables must be conditional (`to_regclass` guard) with an idempotent + plugin-side SQL migration for dual-path convergence. + *Failure class: fresh-install breaks because Alembic ran before plugins.* + +8. **Audit log on every mutation** — use `log_audit` from `app.core.audit`. + +9. **datetime only with UTC** — `datetime.now(UTC)`, never `utcnow()`. + +10. **Pydantic schemas validate input** — no raw dict bodies on routes. + +11. **Agent tools carry permissions** — any tool registered in the tool + registry declares the permission of the underlying endpoint. + +12. **Tests against real PostgreSQL** — ephemeral DB per run (see + `app/plugins/builtins/automation/tests/test_automation.py` fixture); + create real tenant/user rows instead of random UUIDs for FK columns. + +## Review checklist (human/agent reviewer) + +- [ ] Checklist items above verified, not assumed +- [ ] New mechanisms documented in `docs/plugin-development-guide.md` +- [ ] PROGRESS.md updated with finding ID + commit hash + verification proof diff --git a/docs/plugin-development-guide.md b/docs/plugin-development-guide.md index 8395317..c76bba6 100644 --- a/docs/plugin-development-guide.md +++ b/docs/plugin-development-guide.md @@ -366,6 +366,86 @@ class MyPlugin(BasePlugin): --- +## 3.1 Architecture Requirements (v3) — MUST READ + +> **Every plugin must pass `docs/plugin-checklist.md` before merge.** The +> checklist maps each rule to a failure class that actually occurred in this +> codebase. Summary of the mechanisms introduced by the architecture repair +> (Blocks A–C): + +### Contracts — the ONLY way to reach other plugins + +```python +from app.plugins.builtins.contracts import get_contract + +contract = get_contract("kommunikation") +if contract is None or not hasattr(contract, "send_message"): + logger.warning("kommunikation contract unavailable - skipping") + return # or degrade gracefully — NEVER import from the other plugin directly +result = await contract.send_message(...) +``` + +Rules: +- If a contract is missing, define it in the **owning** plugin's `contracts.py` + (self-registers via `get_contract_registry().register()`). +- Contracts must be **unregistered on deactivate** and re-registered on + activate (symmetry). +- CI gate: `python scripts/check_cross_plugin_imports.py` → 0 violations. + +### Lifecycle symmetry & ordering + +Everything registered in `on_activate` (contracts, services, event handlers, +hooks, tools, search providers) MUST be deregistered in `on_deactivate`. Own +cleanup runs FIRST, then `super().on_deactivate()`. On activate, call +`super().on_activate()` FIRST, then own registrations. + +Hook helpers live on the registry singleton, NOT as module functions: + +```python +from app.core.hooks import get_hook_registry +get_hook_registry().register_action("wiki.article.created", handler, owner_tag="wiki") +# deactivate: +get_hook_registry().unregister_all_for_plugin("wiki") +``` + +### Dependencies (`dependencies=[...]`) + +Declare every plugin you depend on in the manifest. Activation order is +topological (Kahn), activation fails with a clear error if a dependency is +inactive, and deactivation of a dependency is **blocked** while your plugin +is active. + +### Permission fields are MANDATORY on UI contributions + +`FrontendMenuItem.permission`, `FrontendPageRoute.permission`, +`FrontendSettingsPage.permission`, `FrontendDashboardWidget.permission` — +empty string means "any authenticated user". The route renderer enforces them +via `ProtectedRoute`; the sidebar/settings filter by them. + +### Frontend components: register for production builds + +New page components must be added to `STATIC_COMPONENT_MAP` in +`frontend/src/components/plugins/PluginLoader.tsx`. Vite cannot chunk runtime-built +import paths — unregistered components work in dev but fail in production. +Also verify the component file actually exists (ghost references show an +error boundary in production). + +### Migrations touching plugin-owned tables + +Alembic revisions must guard with `to_regclass` and skip when the table does +not exist yet; the plugin-side SQL migration adds the same schema idempotently +(dual-path convergence). See migrations 0119–0141 + plugin migration files +for the pattern. + +### Tests run against real PostgreSQL + +Use the ephemeral-database fixture pattern from +`app/plugins/builtins/automation/tests/test_automation.py`: create/drop a DB +per run, enable pgvector, import all models before `create_all`, and create +real tenant/user rows instead of random UUIDs for FK columns. + +--- + ## 4. Plugin Lifecycle ### 4.1 Installation (`on_install`)