Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions backend/druks/extensions/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/<name> 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]
Expand Down Expand Up @@ -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
(``/<subject_type>`` → 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
Expand All @@ -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":
Expand Down
7 changes: 0 additions & 7 deletions backend/druks/extensions/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion backend/druks/usage/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
93 changes: 62 additions & 31 deletions backend/tests/test_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
18 changes: 9 additions & 9 deletions docs/writing-an-extension.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 — `<subject_type>` 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

Expand Down