From eac9a760356ea7719d3b6d4acd0258cc8ed92ffa Mon Sep 17 00:00:00 2001 From: Paulo Date: Mon, 27 Jul 2026 17:09:40 +0200 Subject: [PATCH] The platform's routes go first, so nothing has to be policed --- backend/druks/extensions/base.py | 30 ++++----- backend/druks/extensions/exceptions.py | 7 -- backend/druks/usage/routes.py | 2 +- backend/tests/test_extensions.py | 93 +++++++++++++++++--------- docs/writing-an-extension.md | 18 ++--- 5 files changed, 85 insertions(+), 65 deletions(-) diff --git a/backend/druks/extensions/base.py b/backend/druks/extensions/base.py index f6867fca..eb63d327 100644 --- a/backend/druks/extensions/base.py +++ b/backend/druks/extensions/base.py @@ -11,7 +11,6 @@ from druks.models import StoredSubject from druks.user_settings.models import SettingsOverride -from .exceptions import RouteDeclarationError from .registry import agents as agent_registry from .registry import autodiscover from .registry import workflows as workflow_registry @@ -108,6 +107,11 @@ def __init_subclass__(cls, **kwargs: Any) -> None: f"extension name {name!r} must match {NAME_RE.pattern!r} — it keys the " "/api/ namespace, the version table, and settings keys" ) + if cls.subject_type == "transcripts": + raise TypeError( + f"extension {name!r} declares a 'transcripts' subject; that segment " + "serves every extension's agent-call reads. Name the subject for what it is" + ) cls.table_prefix = f"{name}_" if "package" not in cls.__dict__: cls.package = cls.__module__.rpartition(".")[0] @@ -250,7 +254,8 @@ def get_routers(cls, modules: list[ModuleType]) -> "list[APIRouter]": its ``routes`` modules, plus the generic read-side it gets for free — ``/transcripts`` always, and the subject read-side (``/`` → status + timeline + live stream) when it declares a - ``subject_type``. Override to add a router built outside a ``routes`` module.""" + ``subject_type``. The platform's come first: those two segments are its own. + Override to add a router built outside a ``routes`` module.""" # Local, not module-top: keeps FastAPI off the import graph so the loader # stays importable app-lessly; enumerating routers is where it's really needed. from fastapi import APIRouter @@ -264,22 +269,13 @@ def get_routers(cls, modules: list[ModuleType]) -> "list[APIRouter]": if isinstance(value, APIRouter) and id(value) not in seen: seen.add(id(value)) declared.append(value) - routers = [*declared, cls._get_transcript_routes()] + # The platform's routers are narrow — each confined to its own segment — so + # matching them first costs an extension nothing anywhere else and leaves it + # no way to take a read the platform serves, not even with a catch-all. + platform = [cls._get_transcript_routes()] if cls.subject_type: - # Declared routers mount ahead of these, so one that reaches into the - # subject's own segment takes the board or a detail read with it — and - # nothing collides at import, so the loss shows up as a 404 much later. - claimed = f"/{cls.subject_type}" - for router in declared: - for route in router.routes: - if route.path == claimed or route.path.startswith(f"{claimed}/"): - raise RouteDeclarationError( - f"extension {cls.name!r} declares a route at {route.path}; " - f"{claimed} is the subject read-side the platform mounts. " - "Name your router for what it serves." - ) - routers.append(cls._get_subject_routes(cls.subject_type)) - return routers + platform.append(cls._get_subject_routes(cls.subject_type)) + return [*platform, *declared] @classmethod def _get_transcript_routes(cls) -> "APIRouter": diff --git a/backend/druks/extensions/exceptions.py b/backend/druks/extensions/exceptions.py index b8fe8526..58e436c2 100644 --- a/backend/druks/extensions/exceptions.py +++ b/backend/druks/extensions/exceptions.py @@ -12,13 +12,6 @@ class SettingsDeclarationError(Exception): operator PATCH.""" -class RouteDeclarationError(Exception): - """An extension declares a route inside the subject namespace the platform mounts - its own reads on. Raised at load, where the author's routers are enumerated — - they mount first, so the collision would otherwise take the board and detail - reads silently.""" - - class SubscriberDeclarationError(Exception): """A subscriber's signature asks for a routing key — one a filter matches on but no body is handed. Raised at declaration, so it fails on import instead of diff --git a/backend/druks/usage/routes.py b/backend/druks/usage/routes.py index 1ae11a54..ea2bb53b 100644 --- a/backend/druks/usage/routes.py +++ b/backend/druks/usage/routes.py @@ -23,7 +23,7 @@ from druks.usage.trends import FIVE_HOUR_RANGE, WEEK_RANGE, downsample from druks.user_settings.models import HarnessSettings, UserSettings -router = APIRouter(tags=["usage"]) +router = APIRouter() # The /today bucket for calls whose model no current harness claims. UNATTRIBUTED = "unattributed" diff --git a/backend/tests/test_extensions.py b/backend/tests/test_extensions.py index 5e726acf..f078ed51 100644 --- a/backend/tests/test_extensions.py +++ b/backend/tests/test_extensions.py @@ -2,7 +2,6 @@ import pytest from druks.extensions import Extension, loader -from druks.extensions.exceptions import RouteDeclarationError from druks.extensions.loader import iter_extensions, load from fastapi import APIRouter, FastAPI from fastapi.testclient import TestClient @@ -100,50 +99,82 @@ def _routes_module(name: str, **routers: APIRouter) -> ModuleType: return module -def test_declared_routers_mount_ahead_of_the_platforms(): - """Order is the contract: the author's routers are matched first, so the platform's - own reads are the fallback, never the shadow.""" +def _mount(extension: type[Extension], monkeypatch) -> TestClient: + monkeypatch.setattr(loader, "iter_extensions", lambda: [extension]) + monkeypatch.setattr(loader, "import_extension_models", lambda: None) + app = FastAPI() + load(app) + from druks.accounts.dependencies import current_account - class Ordered(Extension): - name = "ordered" - package = "ordered" - subject_type = "widget" + app.dependency_overrides[current_account] = lambda: None + return TestClient(app) - module = _routes_module("ordered", router=APIRouter(prefix="/parts")) - paths = [router.prefix for router in Ordered.get_routers([module])] - assert paths == ["/parts", "/transcripts/{call_id}", "/widget"] +def test_nothing_an_extension_declares_can_take_a_read_the_platform_serves(monkeypatch): + """Not even a catch-all: the platform's two segments are matched before any router + an extension declares, so an author never has to know they are reserved.""" + greedy = APIRouter() -@pytest.mark.parametrize("prefix", ["/widget", "/widget/settings"]) -def test_a_router_reaching_into_the_subject_namespace_is_rejected(prefix: str): - """Declared routers mount first, so this one would take the board or a detail read - with it — and nothing collides at import, so it has to fail here.""" + @greedy.get("/{anything:path}") + def _greedy(anything: str) -> dict: + return {"who": "extension"} - class Claiming(Extension): - name = "claiming" - package = "claiming" + class Greedy(Extension): + name = "greedy" + package = "greedy" subject_type = "widget" - claimed = APIRouter(prefix=prefix) + @classmethod + def discover(cls) -> list[ModuleType]: + return [_routes_module("greedy", router=greedy)] - @claimed.get("") - def _read() -> dict: - return {} + @classmethod + def list_subjects(cls) -> list: + return [] - with pytest.raises(RouteDeclarationError, match="subject read-side"): - Claiming.get_routers([_routes_module("claiming", router=claimed)]) + client = _mount(Greedy, monkeypatch) + assert client.get("/api/greedy/widget").json() == {"rows": []} + assert client.get("/api/greedy/anything-else").json() == {"who": "extension"} -def test_a_router_named_like_the_subject_is_left_alone(): - """The rule is the segment, not the spelling — ``/widgets`` is an author's own - resource and says nothing about the platform's ``/widget``.""" +def test_a_composed_router_loads(monkeypatch): + """``parent.include_router(child)`` leaves a lazily-flattened entry behind, and an + extension that composes its routes is an ordinary one.""" + parent, child = APIRouter(prefix="/parts"), APIRouter(prefix="/nested") - class Neighbour(Extension): - name = "neighbour" - package = "neighbour" + @child.get("") + def _nested() -> dict: + return {"who": "nested"} + + parent.include_router(child) + + class Composed(Extension): + name = "composed" + package = "composed" subject_type = "widget" - Neighbour.get_routers([_routes_module("neighbour", router=APIRouter(prefix="/widgets"))]) + @classmethod + def discover(cls) -> list[ModuleType]: + return [_routes_module("composed", router=parent)] + + @classmethod + def list_subjects(cls) -> list: + return [] + + assert _mount(Composed, monkeypatch).get("/api/composed/parts/nested").json() == { + "who": "nested" + } + + +def test_a_subject_cannot_take_the_transcripts_segment(): + """Every extension's agent-call reads live there, so the collision is between two + platform surfaces — an author would never see which one answered.""" + with pytest.raises(TypeError, match="agent-call reads"): + + class Colliding(Extension): + name = "colliding" + package = "colliding" + subject_type = "transcripts" def test_the_extension_name_tags_every_route(monkeypatch): diff --git a/docs/writing-an-extension.md b/docs/writing-an-extension.md index 94ab330d..1d77a8b6 100644 --- a/docs/writing-an-extension.md +++ b/docs/writing-an-extension.md @@ -576,18 +576,18 @@ the prefix its own resource is called: router = APIRouter(prefix="/reviews") ``` -Two spellings run through druks, and they never mean the same thing: +Two spellings run through druks, and which one a segment wears says who owns it: | | | | --- | --- | -| `snake_case` | identity — a subject type, a run's kind, an event's attributes | -| `kebab-case` | a URL segment — your route prefixes, your frontend paths | - -So `pull_request` names the subject the platform serves reads for, and -`/reviews` names the resource your own POST creates. A router may not declare a -route inside a subject's own segment: yours mount first, so it would take the -board or a detail read with it, and druks refuses at load rather than let that -go quiet. +| `snake_case` | an identity the platform serves — your extension name, a subject type | +| `kebab-case` | a resource you named — your route prefixes, your frontend paths | + +So `/api/review/pull_request` is the board of review runs, keyed by subject, and +`/api/review/reviews` is the resource your own POST creates. The platform's two +segments — `` and `transcripts` — are matched before your routers, +so nothing you declare can take a read druks serves, not even a catch-all. Name a +router for its own resource and the question never comes up. ## Extension settings and checks