diff --git a/backend/druks/extensions/base.py b/backend/druks/extensions/base.py index 53183620..a45b85bf 100644 --- a/backend/druks/extensions/base.py +++ b/backend/druks/extensions/base.py @@ -11,7 +11,7 @@ from druks.models import StoredSubject from druks.user_settings.models import SettingsOverride -from .exceptions import SettingsDeclarationError +from .exceptions import ExtensionSubjectContractError, SettingsDeclarationError from .registry import agents as agent_registry from .registry import autodiscover from .registry import workflows as workflow_registry @@ -23,7 +23,7 @@ ) if TYPE_CHECKING: - from fastapi import APIRouter, FastAPI + from fastapi import APIRouter from druks.agents import Agent from druks.doctor import CheckResult @@ -57,10 +57,10 @@ def clean(self) -> dict[str, str]: class Extension: """A pluggable application. Subclass it, set ``name``, and register the - subclass under the ``druks.extensions`` entry-point group. At boot the platform - calls ``load`` for every extension, which imports the package's - conventionally-named modules — that import is where the extension's webhooks, - workflows, agents, and subscribers self-register. + subclass under the ``druks.extensions`` entry-point group. At boot the loader + calls ``discover``, which imports the package's conventionally-named modules — + that import is where the extension's webhooks, workflows, agents, and + subscribers self-register. Used as a class, never instantiated: an extension is a stateless install singleton, so an instance would only be ceremony. @@ -181,20 +181,33 @@ def agents(cls) -> "list[Agent]": def workflows(cls) -> "list[type[Workflow]]": """The workflows living in this extension's package.""" prefix = cls.package + "." - return [wf for wf in workflow_registry.all() if wf.__module__.startswith(prefix)] + return [ + workflow + for workflow in workflow_registry.all() + if workflow.__module__.startswith(prefix) + ] @classmethod - def subject_classes(cls) -> "list[type[Subject] | type[StoredSubject]]": - """What this extension's runs are about, read off the workflows that declare - them — each one gets a board and a page, ordered by subject type so the routes - it mounts are stable.""" - declared = {wf.subject for wf in cls.workflows() if wf.subject} + def subjects(cls) -> "list[type[Subject] | type[StoredSubject]]": + """The subjects this extension's workflows declare, ordered by subject type. + Each must implement ``list_summaries()``. The check compares method identity + and does not call the method.""" + from druks.durable.datastructures import Subject + + stubs = {Subject.list_summaries.__func__, StoredSubject.list_summaries.__func__} + declared = {workflow.subject for workflow in cls.workflows() if workflow.subject} for subject_class in declared: if subject_class.subject_type == "transcripts": - raise TypeError( + raise ExtensionSubjectContractError( f"{subject_class.__name__} is a 'transcripts' subject; that segment " "serves every extension's agent-call reads. Name it for what it is" ) + if subject_class.list_summaries.__func__ in stubs: + raise ExtensionSubjectContractError( + f"extension {cls.name!r} declares subject {subject_class.__name__} " + f"without list_summaries(); the board calls it. Implement " + f"list_summaries() on {subject_class.__name__}." + ) return sorted(declared, key=lambda subject_class: subject_class.subject_type) @classmethod @@ -260,35 +273,6 @@ def frontend_dist(cls) -> Path | None: dist = package_dir / "dist" return dist if (dist / "entry.js").is_file() else None - @classmethod - def load(cls, app: "FastAPI") -> None: - """Wire the extension into the running API: import its capabilities - (``discover``), mount its routers under ``/api/``, and serve its - shipped frontend (if any) under ``/app/``. The loader calls this once - per extension at boot.""" - # Local, matching get_routers: the loader stays importable app-lessly. - from fastapi import Depends - - from druks.accounts.dependencies import current_account - - modules = cls.discover() - # /api/ wraps the author's own prefix so extensions can't shadow - # the platform or each other; every route sits behind the identity gate. - # The extension's name tags them all, so a router says only what it serves. - prefix = f"/api/{cls.name}" - for router in cls.get_routers(modules): - app.include_router( - router, - prefix=prefix, - tags=[cls.name], - dependencies=[Depends(current_account)], - ) - dist = cls.frontend_dist() - if dist: - # /app, not /api: unknown /api/* paths must stay JSON 404s, never fall - # through to an index.html. - app.frontend(f"/app/{cls.name}", directory=dist) - @classmethod def get_routers(cls, modules: list[ModuleType]) -> "list[APIRouter]": """Every router mounted under the extension's namespace: the ones it declares in @@ -315,7 +299,7 @@ def get_routers(cls, modules: list[ModuleType]) -> "list[APIRouter]": # no way to take a read the platform serves, not even with a catch-all. return [ cls._get_transcript_routes(), - *(cls._get_subject_routes(subject) for subject in cls.subject_classes()), + *(cls._get_subject_routes(subject) for subject in cls.subjects()), *declared, ] diff --git a/backend/druks/extensions/exceptions.py b/backend/druks/extensions/exceptions.py index 58e436c2..22434d02 100644 --- a/backend/druks/extensions/exceptions.py +++ b/backend/druks/extensions/exceptions.py @@ -19,8 +19,8 @@ class SubscriberDeclarationError(Exception): class ExtensionLoadError(Exception): - """An extension could not be loaded app-lessly. The concrete subclass names - which stage failed — nothing raises this base directly.""" + """An extension could not be loaded, app-lessly or at full boot. The concrete + subclass names the failed stage; nothing raises this base directly.""" class ExtensionNotFound(ExtensionLoadError): @@ -38,3 +38,8 @@ class ExtensionImportError(ExtensionLoadError): """Importing the extension's models or capability modules raised. The extension is installed and well-declared, but its own code failed on import — carries the original exception as its cause.""" + + +class ExtensionSubjectContractError(ExtensionLoadError): + """A declared subject fails the read-side contract: no ``list_summaries()`` + implementation, or it names the reserved ``transcripts`` segment.""" diff --git a/backend/druks/extensions/loader.py b/backend/druks/extensions/loader.py index a372a95d..692367ae 100644 --- a/backend/druks/extensions/loader.py +++ b/backend/druks/extensions/loader.py @@ -7,6 +7,7 @@ if TYPE_CHECKING: from importlib.metadata import EntryPoint + from types import ModuleType from fastapi import FastAPI @@ -70,8 +71,8 @@ def iter_extensions() -> list[type[Extension]]: if extension.name in seen: raise ValueError(f"duplicate extension name {extension.name!r}") seen.add(extension.name) - # Ownership registers before load(app)/discover() imports the capability - # modules, whose Workflow classes resolve their extension at definition. + # Ownership registers before discover() imports the capability modules, + # whose Workflow classes resolve their extension at definition. register_workflow_package(extension.package, extension.name) extensions.append(extension) return extensions @@ -91,18 +92,21 @@ def load_extension(name: str) -> type[Extension]: class, with every surface then enumerable off it (``workflows()``, ``routers()``, ``capability_modules()``, ``settings_model``, ``migrations_dir()``). The load path used by the CLI, tests, and evals — - no FastAPI, no ``load(app)``. + no FastAPI, nothing mounted. Fails loudly and by name: an uninstalled package raises ``ExtensionNotFound``; an entry point that doesn't resolve to an ``Extension`` raises ``MalformedExtension``; the extension's own code raising on import raises - ``ExtensionImportError``.""" + ``ExtensionImportError``; a declared subject that fails the read-side contract + raises ``ExtensionSubjectContractError``.""" extension = _resolve(name) try: import_extension_models(extension) extension.discover() except Exception as error: raise ExtensionImportError(f"extension {name!r} failed to import: {error}") from error + # Outside the try: a contract break is not an import error. + extension.subjects() return extension @@ -215,12 +219,35 @@ def import_extension_models(only: type[Extension] | None = None) -> None: ) +def mount(app: "FastAPI", extension: type[Extension], modules: list["ModuleType"]) -> None: + """Mount one discovered extension under ``/api/`` and ``/app/``. + The prefix and the identity gate are the loader's — no extension hook can + override them.""" + # Local, matching get_routers: the loader stays importable app-lessly. + from fastapi import Depends + + from druks.accounts.dependencies import current_account + + prefix = f"/api/{extension.name}" + for router in extension.get_routers(modules): + app.include_router( + router, + prefix=prefix, + tags=[extension.name], + dependencies=[Depends(current_account)], + ) + dist = extension.frontend_dist() + if dist: + # /app, not /api: unknown /api/* paths stay JSON 404s. + app.frontend(f"/app/{extension.name}", directory=dist) + + def load(app: "FastAPI") -> None: - """API boot entry: every extension imports its capabilities (self-registering its - webhooks, workflows, agents, subscribers) and mounts its routers under - ``/api/``.""" + """API boot: for each extension — discover, validate subjects, mount.""" # The table-prefix check runs here, not just in makemigrations — an author # who hand-writes migrations still can't boot with an unprefixed table. import_extension_models() for extension in iter_extensions(): - extension.load(app) + modules = extension.discover() + extension.subjects() + mount(app, extension, modules) diff --git a/backend/druks/extensions/routes.py b/backend/druks/extensions/routes.py index 7a0c718f..0306619c 100644 --- a/backend/druks/extensions/routes.py +++ b/backend/druks/extensions/routes.py @@ -14,7 +14,7 @@ async def list_extensions() -> list[ExtensionResponse]: icon=extension.icon, description=extension.description, builtin=extension.builtin, - subject_types=[subject.subject_type for subject in extension.subject_classes()], + subject_types=[subject.subject_type for subject in extension.subjects()], has_frontend=bool(extension.frontend_dist()), navigation=extension.navigation, ) diff --git a/backend/druks/scaffolding/extension_template/package/models.py-tpl b/backend/druks/scaffolding/extension_template/package/models.py-tpl index 8854caba..f3c633f4 100644 --- a/backend/druks/scaffolding/extension_template/package/models.py-tpl +++ b/backend/druks/scaffolding/extension_template/package/models.py-tpl @@ -7,4 +7,6 @@ from druks.db import Base, StoredSubject # ticket. Subclass ``StoredSubject`` for that one and point a workflow at it with # ``subject = ThatModel``, and druks shows it a board and a page. Something you keep no # row for subclasses ``druks.workflows.Subject`` instead — same board, same page. +# A declared subject must implement ``list_summaries()`` — the board reads it. +# Druks checks this at load and refuses the extension if it is missing. # After adding a model: ``druks makemigrations {{ name }} -m "..." && druks init-db``. diff --git a/backend/tests/test_extension_appless_load.py b/backend/tests/test_extension_appless_load.py index 377d9d9a..9d0a1940 100644 --- a/backend/tests/test_extension_appless_load.py +++ b/backend/tests/test_extension_appless_load.py @@ -9,6 +9,7 @@ ExtensionImportError, ExtensionLoadError, ExtensionNotFound, + ExtensionSubjectContractError, MalformedExtension, ) from druks.extensions.loader import load_extension @@ -93,6 +94,39 @@ class Settings(BaseModel): } +# A package whose declared ``StoredSubject`` omits ``list_summaries()``. Its own +# table keeps it off the conforming probe's shared metadata. +_BROKEN_STORED_FILES = { + "extension.py": """ + from druks.extensions import Extension + + + class BrokenStored(Extension): + name = "brokenstored" + """, + "models.py": """ + from druks.db import StoredSubject + + + class Ledger(StoredSubject): + __tablename__ = "brokenstored_ledgers" + # No list_summaries() — the load gate must reject it. + """, + "workflows.py": """ + from druks.workflows import Workflow + + from .models import Ledger + + + class Post(Workflow): + subject = Ledger + + async def run(self) -> None: + ... + """, +} + + def _write_package(root: Path, package: str, files: dict[str, str]) -> None: directory = root / package (directory / "migrations" / "versions").mkdir(parents=True) @@ -120,7 +154,9 @@ def external_extension(tmp_path_factory): sys.path.insert(0, str(root)) tables = set(Base.metadata.tables) - registries = {r: dict(r._items) for r in (agents, services, webhooks, workflows)} + registries = { + registry: dict(registry._items) for registry in (agents, services, webhooks, workflows) + } packages = dict(extensions_loader._workflow_packages) finished = signal("workflow.finished") receivers = dict(finished.receivers) @@ -325,3 +361,51 @@ def test_import_error_in_models_raises_extension_import_error(tmp_path, monkeypa with pytest.raises(ExtensionImportError, match="failed to import") as caught: load_extension("probe") assert isinstance(caught.value.__cause__, RuntimeError) + + +@pytest.fixture +def broken_stored_extension(tmp_path_factory, monkeypatch): + """The broken package as the only installed entry point; restores the globals + its load mutates.""" + from druks.extensions import loader as extensions_loader + from druks.extensions.registry import agents, services, webhooks, workflows + from druks.models import Base + + package = "druks_broken_stored" + root = tmp_path_factory.mktemp("broken_stored") + _write_package(root, package, _BROKEN_STORED_FILES) + sys.path.insert(0, str(root)) + + tables = set(Base.metadata.tables) + registries = { + registry: dict(registry._items) for registry in (agents, services, webhooks, workflows) + } + packages = dict(extensions_loader._workflow_packages) + entry = EntryPoint( + name="brokenstored", value=f"{package}.extension:BrokenStored", group="druks.extensions" + ) + monkeypatch.setattr(loader, "entry_points", lambda *, group: [entry]) + try: + yield + finally: + sys.path.remove(str(root)) + for name in set(Base.metadata.tables) - tables: + Base.metadata.remove(Base.metadata.tables[name]) + for registry, snapshot in registries.items(): + registry._items = snapshot + extensions_loader._workflow_packages.clear() + extensions_loader._workflow_packages.update(packages) + for name in [m for m in sys.modules if m == package or m.startswith(f"{package}.")]: + del sys.modules[name] + + +def test_appless_load_rejects_a_stored_subject_missing_list_summaries(broken_stored_extension): + """A row-backed subject is gated the same as ``Subject`` — typed, not an import error.""" + with pytest.raises(ExtensionSubjectContractError) as caught: + load_extension("brokenstored") + message = str(caught.value) + assert "brokenstored" in message # the extension name + assert "Ledger" in message # the subject class + assert "list_summaries()" in message # the missing method + assert "Implement list_summaries()" in message # the implementation direction + assert isinstance(caught.value, ExtensionLoadError) diff --git a/backend/tests/test_extensions.py b/backend/tests/test_extensions.py index 29407c31..63f67fae 100644 --- a/backend/tests/test_extensions.py +++ b/backend/tests/test_extensions.py @@ -2,6 +2,7 @@ import pytest from druks.extensions import Extension, loader +from druks.extensions.exceptions import ExtensionSubjectContractError from druks.extensions.loader import iter_extensions, load from druks.workflows import Subject from fastapi import APIRouter, FastAPI @@ -17,7 +18,7 @@ def list_summaries(cls) -> list: return [] -def _subject_classes(cls) -> list[type[Subject]]: +def _subjects(cls) -> list[type[Subject]]: return [Widget] @@ -119,7 +120,7 @@ def _routes_module(name: str, **routers: APIRouter) -> ModuleType: return module -def _mount(extension: type[Extension], monkeypatch) -> TestClient: +def _boot(extension: type[Extension], monkeypatch) -> TestClient: monkeypatch.setattr(loader, "iter_extensions", lambda: [extension]) monkeypatch.setattr(loader, "import_extension_models", lambda: None) app = FastAPI() @@ -142,13 +143,13 @@ def _greedy(anything: str) -> dict: class Greedy(Extension): name = "greedy" package = "greedy" - subject_classes = classmethod(_subject_classes) + subjects = classmethod(_subjects) @classmethod def discover(cls) -> list[ModuleType]: return [_routes_module("greedy", router=greedy)] - client = _mount(Greedy, monkeypatch) + client = _boot(Greedy, monkeypatch) assert client.get("/api/greedy/widget").json() == {"rows": []} assert client.get("/api/greedy/anything-else").json() == {"who": "extension"} @@ -167,13 +168,13 @@ def _nested() -> dict: class Composed(Extension): name = "composed" package = "composed" - subject_classes = classmethod(_subject_classes) + subjects = classmethod(_subjects) @classmethod def discover(cls) -> list[ModuleType]: return [_routes_module("composed", router=parent)] - assert _mount(Composed, monkeypatch).get("/api/composed/parts/nested").json() == { + assert _boot(Composed, monkeypatch).get("/api/composed/parts/nested").json() == { "who": "nested" } @@ -193,8 +194,73 @@ class Colliding(Extension): def workflows(cls): return [SimpleNamespace(subject=Transcripts)] - with pytest.raises(TypeError, match="agent-call reads"): - Colliding.subject_classes() + with pytest.raises(ExtensionSubjectContractError, match="agent-call reads"): + Colliding.subjects() + + +def test_a_declared_subject_missing_list_summaries_is_rejected_with_an_actionable_error(): + class Account(Subject): + pass # inherits the platform stub + + class Broken(Extension): + name = "broken" + package = "broken" + + @classmethod + def workflows(cls): + return [SimpleNamespace(subject=Account)] + + with pytest.raises(ExtensionSubjectContractError) as caught: + Broken.subjects() + message = str(caught.value) + assert "broken" in message # the extension name + assert "Account" in message # the subject class + assert "list_summaries()" in message # the missing method + assert "Implement list_summaries()" in message # the implementation direction + + +def test_full_boot_refuses_a_subject_missing_list_summaries(monkeypatch): + """An extension with nothing to mount still fails — the gate is a loader stage.""" + + class Account(Subject): + pass # inherits the platform stub + + class Broken(Extension): + name = "broken_boot" + package = "broken_boot" + + @classmethod + def discover(cls) -> list[ModuleType]: + return [] + + @classmethod + def workflows(cls): + return [SimpleNamespace(subject=Account)] + + monkeypatch.setattr(loader, "iter_extensions", lambda: [Broken]) + monkeypatch.setattr(loader, "import_extension_models", lambda: None) + with pytest.raises(ExtensionSubjectContractError, match="list_summaries"): + load(FastAPI()) + + +def test_a_concrete_inherited_list_summaries_satisfies_the_contract_without_calling_it(): + class Concrete(Subject): + @classmethod + def list_summaries(cls) -> list: + raise AssertionError("validation must not call list_summaries()") + + class Inheritor(Concrete): + pass + + class Fine(Extension): + name = "fine" + package = "fine" + + @classmethod + def workflows(cls): + return [SimpleNamespace(subject=Inheritor)] + + assert Fine.subjects() == [Inheritor] def test_an_extension_declaring_a_subject_type_is_rejected(): @@ -211,7 +277,7 @@ def test_an_extensions_subjects_come_from_its_workflows(): ship = next(extension for extension in iter_extensions() if extension.name == "ship") ship.discover() - assert [s.subject_type for s in ship.subject_classes()] == ["project_repo", "work_item"] + assert [subject.subject_type for subject in ship.subjects()] == ["project_repo", "work_item"] def test_the_extension_name_tags_every_route(monkeypatch): diff --git a/backend/tests/test_proof_extension.py b/backend/tests/test_proof_extension.py index fc64ad54..36dc078e 100644 --- a/backend/tests/test_proof_extension.py +++ b/backend/tests/test_proof_extension.py @@ -9,7 +9,7 @@ def test_boot_loads_the_external_extension(): assert extension.name == "field_notes" assert extension.package == _PACKAGE - assert [subject.__name__ for subject in extension.subject_classes()] == ["Note"] + assert [subject.__name__ for subject in extension.subjects()] == ["Note"] def test_discovery_registers_the_tables_and_capabilities(): diff --git a/backend/tests/test_proof_extension_install.py b/backend/tests/test_proof_extension_install.py index 3524f05e..1b659681 100644 --- a/backend/tests/test_proof_extension_install.py +++ b/backend/tests/test_proof_extension_install.py @@ -12,7 +12,7 @@ assert "field_notes" in names, names extension = load_extension("field_notes") - assert [subject.__name__ for subject in extension.subject_classes()] == ["Note"] + assert [subject.__name__ for subject in extension.subjects()] == ["Note"] assert extension.settings_model is not None assert list(extension.settings_model.model_fields) == [ "board_size", diff --git a/backend/tests/test_scaffolding.py b/backend/tests/test_scaffolding.py index aaedfead..41ee802f 100644 --- a/backend/tests/test_scaffolding.py +++ b/backend/tests/test_scaffolding.py @@ -2,7 +2,7 @@ import sys import pytest -from druks.extensions.loader import _workflow_packages, register_workflow_package +from druks.extensions.loader import _workflow_packages, mount, register_workflow_package from druks.scaffolding import create_extension from fastapi import FastAPI from fastapi.testclient import TestClient @@ -39,7 +39,7 @@ def test_create_extension_scaffolds_a_loadable_package(tmp_path): assert "Subject(" not in text # The generated extension.py must survive Extension.__init_subclass__ validation, - # and load() must mount both the API routes and the shipped dist/ frontend. + # and mounting must serve both the API routes and the shipped dist/ frontend. sys.path.insert(0, str(target)) try: module = importlib.import_module("druks_night_watch.extension") @@ -60,7 +60,7 @@ def test_create_extension_scaffolds_a_loadable_package(tmp_path): assert "extension=" not in (target / "druks_night_watch" / "workflows.py").read_text() app = FastAPI() - extension.load(app) + mount(app, extension, extension.discover()) # Scaffolding is under test, not the identity gate. from druks.accounts.dependencies import current_account diff --git a/docs/writing-an-extension.md b/docs/writing-an-extension.md index 6c6dc0bb..1c34ac54 100644 --- a/docs/writing-an-extension.md +++ b/docs/writing-an-extension.md @@ -497,6 +497,10 @@ Any id names one of these, so a detail read always answers. Override `get_for_subject_id()` to return None for a shape yours could never wear — `owner/repo#7` is a pull request, `nonsense` is a 404. +Each subject a workflow declares must implement `list_summaries()`. The board +reads it. Druks checks this at load. If the method is missing, the extension +does not load. The error names the extension, the subject, and the method. + Druks serves the same `/api/night_watch/repository` surface either way: a board, a page for one, and a live stream of either, mounted for every subject your workflows declare. Each response pairs your summary with the run's status,