docs(f3): plugin checklist + architecture requirements section in dev guide
This commit is contained in:
@@ -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
|
||||||
@@ -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. Plugin Lifecycle
|
||||||
|
|
||||||
### 4.1 Installation (`on_install`)
|
### 4.1 Installation (`on_install`)
|
||||||
|
|||||||
Reference in New Issue
Block a user