feat(plugins): keep a plugin's note metadata and test the SDK against the app

This commit is contained in:
CREDO23 2026-09-30 14:43:10 +02:00
parent 4a4a88767b
commit 3bd8475362
14 changed files with 434 additions and 10 deletions

View file

@ -24,6 +24,7 @@ jobs:
outputs:
manifest_rules_changed: ${{ steps.changed_paths.outputs.manifest_rules_changed }}
sdk_changed: ${{ steps.changed_paths.outputs.sdk_changed }}
sdk_or_app_changed: ${{ steps.changed_paths.outputs.sdk_or_app_changed }}
steps:
- name: Checkout code
@ -42,6 +43,11 @@ jobs:
sdk_changed:
- 'plugins/core/sdk/**'
- '.github/workflows/plugins-pull-request-checks.yml'
# Either side can break the other: a route the SDK calls, or the SDK.
sdk_or_app_changed:
- 'plugins/core/sdk/**'
- 'surfsense_local/backend/**'
- '.github/workflows/plugins-pull-request-checks.yml'
check-manifest-rules:
name: Check manifest rules
@ -113,3 +119,32 @@ jobs:
- name: Run unit tests
working-directory: plugins/core/sdk
run: uv run pytest -m unit
check-sdk-against-the-app:
name: Check SDK against the app
runs-on: ubuntu-latest
needs: detect-changes
if: github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.sdk_or_app_changed == 'true'
steps:
- name: Checkout code
uses: actions/checkout@v7
- name: Setup uv
uses: astral-sh/setup-uv@v10.2.0
with:
enable-cache: true
python-version: '3.12'
# The contract tests start the real backend, so it needs its own packages.
- name: Install the app's dependencies
working-directory: surfsense_local/backend
run: uv sync
- name: Install the SDK's dependencies
working-directory: plugins/core/sdk
run: uv sync
- name: Run contract tests
working-directory: plugins/core/sdk
run: uv run pytest -m contract

View file

@ -89,6 +89,7 @@ pre-commit run --all-files
| MCP | `cd surfsense_mcp && uv run pytest` |
| Plugins: manifest rules | `cd plugins/core/manifest && uv run pytest -m unit` |
| Plugins: SDK | `cd plugins/core/sdk && uv run pytest -m unit` |
| Plugins: SDK against the app | `cd plugins/core/sdk && uv run pytest -m contract` (starts the backend; `uv sync` it first) |
| Desktop | `cd surfsense_local/electron && pnpm test` |
CI: `.github/workflows/`. New behavior: one failing test, then the minimum code to pass it. Use the `tdd` skill. Tests hit public seams, not internals.

View file

@ -25,9 +25,9 @@ A workspace holds a library of sources: uploaded files, notes written in the app
| Method | Path | Does |
|---|---|---|
| `GET` | `/workspaces/{workspace_id}/documents` | list (below) |
| `POST` | `/workspaces/{workspace_id}/documents` | write a note; `201` with its body |
| `POST` | `/workspaces/{workspace_id}/documents` | write a note, with an optional `document_metadata` object; `201` with its body, `422` for a field it does not know |
| `POST` | `/workspaces/{workspace_id}/documents/upload` | upload files; `201` with `created`, `duplicates` and `rejected` |
| `GET` | `/workspaces/{workspace_id}/documents/{document_id}` | one document with its `content`; `null` for a `FILE` the worker has not written yet |
| `GET` | `/workspaces/{workspace_id}/documents/{document_id}` | one document with its `content` and `document_metadata`; `content` is `null` for a `FILE` the worker has not written yet |
| `PATCH` | `/workspaces/{workspace_id}/documents/{document_id}` | rename; edit a note's content |
| `DELETE` | `/workspaces/{workspace_id}/documents/{document_id}` | delete; `409` while `processing` |
| `POST` | `/workspaces/{workspace_id}/documents/{document_id}/retry` | requeue a `failed` or `cancelled` document |
@ -53,6 +53,8 @@ Chunks have no router of their own.
A note is a document the user writes, with no file behind it. Creating one commits the row as `pending` and enqueues ingest: nothing needs parsing, but the note is not `ready` until it is chunked and indexed. A note never touches the filesystem and carries no dedup key, so two notes with the same text are two documents.
A note may carry `document_metadata`, stored as given. A plugin names itself there on the notes it adds, with `plugin_id`, `plugin_version`, `action` and `run_id`; a note the user writes has none, and editing a note leaves it alone. Writing a note with a field the API does not know is refused rather than dropped, so a client that names a field differently finds out at once.
## Upload
- The request is multipart, one `files` part per file. Each file streams to a temporary file inside the workspace directory in 1 MB reads, hashed with SHA-256 on the way past and never held whole in memory. A file over 500 MB is refused with `413`. The temporary file sits on the same filesystem as its destination, because a rename is only atomic within one filesystem.

View file

@ -84,7 +84,7 @@ These were not in the design conversation. They are decided here so a developer
|---|---|
| The packaged app has no general Python. API and worker are PyInstaller binaries. | [`python/01-interpreter.md`](python/01-interpreter.md) ships a real interpreter. Plugins run on that, never inside the frozen binaries. |
| A document has to belong to a workspace. | A run is started in a workspace, and its id is in the plugin's environment. Verbs default to it. |
| A note has no way to say which plugin wrote it. `POST /workspaces/{id}/documents` takes `{title, content}` and nothing else, so `document.add()` cannot stamp which plugin, action and run wrote the note. | `NoteCreate` gains an optional metadata object, set into the `document_metadata` column that already exists, and the SDK fills it; the keys are in [`app/01-api.md`](app/01-api.md). A plugin could lie about it — loopback has no auth — so it is provenance for the user's benefit, not an audit trail. Without it, nothing in the library says where it came from. |
| A note has no way to say which plugin wrote it. `POST /workspaces/{id}/documents` takes `{title, content}` and nothing else, so `document.add()` cannot stamp which plugin, action and run wrote the note. | `NoteCreate` takes an optional `document_metadata` object, stored as given in the column of the same name, and the SDK fills it; the keys are in [`app/01-api.md`](app/01-api.md). A plugin could lie about it — loopback has no auth — so it is provenance for the user's benefit, not an audit trail. Without it, nothing in the library says where it came from. |
| The API port is chosen at boot, so nothing outside the app knows it. | Electron writes the URL to `api-url` in its data folder, `~/.surfsense` or `~/.surfsense-dev`, when it picks the port. The runner passes it to plugins as `SURFSENSE_PLUGIN_API_URL`; `surfsense-plugins invoke` reads the file so an author does not have to find it. |
| A plugin can ignore the facade and call any route, because loopback has no auth. | Accepted. The facade is what we sanction, not what we prevent, and review is the mechanism — the same position we took on not sandboxing. |
| A GitHub release on this repo is read by every installed app as an app update. | Plugins publish to a second repository's releases. The app downloads from `github.com`, which redirects to `release-assets.githubusercontent.com`; those are the two hosts install and refresh need. The plugin's own HTTP is not this path. |

View file

@ -1,6 +1,6 @@
# App — API
> Owns: `surfsense_local/backend/modules/plugins/router.py`, `schemas.py`, the `plugin_secrets` table, and two changes outside the module: provenance on `NoteCreate`, and a list of hosts on the egress refusal.
> Owns: `surfsense_local/backend/modules/plugins/router.py`, `schemas.py`, the `plugin_secrets` table, and two changes outside the module: `document_metadata` on `NoteCreate`, and a list of hosts on the egress refusal.
> Routes the screen in [`02-screen.md`](02-screen.md) calls. Runtime: [`../runtime/01-process.md`](../runtime/01-process.md). Install: [`../install/01-install-update-uninstall.md`](../install/01-install-update-uninstall.md).
## Goal
@ -28,7 +28,7 @@ The screen can list plugins, install or update one, set a secret, and start a ru
Emit `plugin.run.updated` on the existing events broker when a run's status changes, carrying the run id, so the screen can invalidate.
Provenance: `NoteCreate` in `modules/documents/schemas.py` gains an optional `metadata` object, `{ "plugin_id", "plugin_version", "action", "run_id" }`, stored in the `document_metadata` column that already exists. The SDK's `document.add()` fills it: the id and stamped version from the plugin's packaged `manifest.json`, the action from its arguments, the run id from the environment. Nothing else sets it. It says where a note came from, for the user's benefit; loopback has no auth, so it is not an audit trail.
A plugin's note names the plugin: `NoteCreate` in `modules/documents/schemas.py` takes an optional `document_metadata` object, stored as given in the column of the same name, and reading the document returns it ([documents](../../../architecture/documents.md#notes)). A new note with a field the app does not know is refused, so the SDK and the app cannot disagree on a name silently. The SDK's `document.add()` fills it with `{ "plugin_id", "plugin_version", "action", "run_id" }`: the id and stamped version from the plugin's packaged `manifest.json`, the action from its arguments, the run id from the environment. Nothing else sets it. It says where a note came from, for the user's benefit; loopback has no auth, so it is not an audit trail.
Several hosts in one refusal: `EgressDeniedError` carries one destination today, and the 403 built by `egress_denied` in `api/main.py` names one `destination` and `host`, which is all the prompt reads. It gains a list. The 403 adds `hosts`, every host the action still needs, and keeps `destination` and `host` as the first of them, so today's callers do not change. The prompt's side is in [`02-screen.md`](02-screen.md). This changes the egress feature, so [`egress.md`](../../../architecture/egress.md) is updated in the same pull request.
@ -44,7 +44,7 @@ Electron writes `http://127.0.0.1:<port>` to `api-url` in its data folder, `~/.s
- Run of a `paid` plugin with no license returns 402. With a trial license, it runs.
- Run of a plugin with two declared hosts, neither allowed, returns 403 naming both, and does not insert a run.
- Run of an installed version blocked for this app returns 409 `cannot_run` with the reason and an offer, and list shows the same.
- A successful run of `plugins/example` returns a run id; polling the run reaches `succeeded`; a note exists in the workspace, and its metadata names the plugin, its version and the run.
- A successful run of `plugins/example` returns a run id; polling the run reaches `succeeded`; a note exists in the workspace, and its `document_metadata` names the plugin, its version and the run.
- An egress refusal for one host still carries `destination` and `host` as before, plus `hosts` with that one host.
- `GET` of secrets returns `set: true` after `PUT` and does not contain the value.
- A run started in one workspace is listed by that workspace's `GET /workspaces/{id}/plugin-runs` and not by another's.

View file

@ -22,7 +22,11 @@ packages = ["surfsense_plugin_sdk"]
[tool.pytest.ini_options]
testpaths = ["tests"]
addopts = "-v --tb=short -ra --strict-markers --import-mode=importlib"
markers = ["unit: runs a plugin against a stub app, never the real one"]
markers = [
"unit: runs a plugin against a stub app, never the real one",
"contract: runs the SDK against the real app, started for the test",
"covers(verb): the public verb a contract test calls, such as document.add",
]
[tool.pyright]
include = ["surfsense_plugin_sdk"]

View file

@ -44,7 +44,7 @@ def add(title: str, content: str) -> Document:
{
"title": title,
"content": content,
"metadata": {
"document_metadata": {
"plugin_id": run.manifest["id"],
"plugin_version": run.manifest.get("version"),
"action": run.action,

View file

@ -0,0 +1,115 @@
import json
import os
import shutil
import socket
import subprocess
import time
import urllib.error
import urllib.request
from collections.abc import Iterator
from dataclasses import dataclass
from pathlib import Path
import pytest
BACKEND_DIR = Path(__file__).resolve().parents[5] / "surfsense_local" / "backend"
# A first `uv run` may still be installing the backend's own packages.
STARTUP_SECONDS = 180
@dataclass
class RealApp:
"""The app's real API, which the SDK's verbs must keep working against."""
url: str
def call(self, method: str, path: str, body: object = None) -> object:
"""Calls the app the way any client does, to set up or check a test."""
request = urllib.request.Request(
self.url + path,
method=method,
data=None if body is None else json.dumps(body).encode(),
headers={} if body is None else {"Content-Type": "application/json"},
)
with urllib.request.urlopen(request, timeout=30) as response:
return json.loads(response.read() or b"null")
def new_workspace(self) -> int:
"""A workspace of the test's own, so no test sees another's documents."""
created = self.call("POST", "/workspaces", {"name": "Contract test"})
return int(created["id"]) # type: ignore[index]
def context(self, workspace_id: int) -> dict[str, str]:
"""The environment the app's runner gives a plugin run in that workspace."""
return {
"SURFSENSE_PLUGIN_API_URL": self.url,
"SURFSENSE_PLUGIN_WORKSPACE_ID": str(workspace_id),
"SURFSENSE_PLUGIN_RUN_ID": "41",
"SURFSENSE_PLUGIN_ID": "example",
}
def _free_port() -> int:
"""A port nothing listens on, for this session's app."""
with socket.socket() as probe:
probe.bind(("127.0.0.1", 0))
return probe.getsockname()[1]
def _answers(url: str) -> bool:
"""Whether the app is up: /health answers once migrations have run."""
try:
with urllib.request.urlopen(f"{url}/health", timeout=2) as response:
return response.status == 200
except (urllib.error.URLError, OSError):
return False
@pytest.fixture(scope="session")
def real_app(tmp_path_factory: pytest.TempPathFactory) -> Iterator[RealApp]:
"""The backend started on a data folder of its own, as Electron starts it."""
uv = shutil.which("uv")
if uv is None:
pytest.fail(
"the contract tests start the backend with uv, which is not on PATH"
)
port = _free_port()
url = f"http://127.0.0.1:{port}"
log = tmp_path_factory.mktemp("app") / "app.log"
# The SDK's own virtual environment is not the backend's.
environment = {k: v for k, v in os.environ.items() if k != "VIRTUAL_ENV"} | {
"SURFSENSE_LOCAL_DATA_DIR": str(tmp_path_factory.mktemp("app-data")),
"SURFSENSE_LOCAL_SECRET": "contract-tests",
"SURFSENSE_LOCAL_HOST": "127.0.0.1",
"SURFSENSE_LOCAL_PORT": str(port),
}
with log.open("w") as output:
app = subprocess.Popen(
[
uv,
"run",
"--directory",
str(BACKEND_DIR),
"python",
"-c",
"from api.server import serve; serve()",
],
env=environment,
stdout=output,
stderr=subprocess.STDOUT,
)
try:
deadline = time.monotonic() + STARTUP_SECONDS
while not _answers(url):
if app.poll() is not None or time.monotonic() > deadline:
pytest.fail(f"the app did not start:\n{log.read_text()[-3000:]}")
time.sleep(0.5)
yield RealApp(url)
finally:
app.terminate()
try:
app.wait(timeout=15)
except subprocess.TimeoutExpired:
app.kill()

View file

@ -0,0 +1,117 @@
import pytest
pytestmark = pytest.mark.contract
@pytest.mark.covers("document.add")
def test_add_leaves_the_note_and_its_document_metadata_in_the_app(
plugin, real_app
) -> None:
"""The app keeps the note, and names the plugin, action and run that wrote it."""
workspace = real_app.new_workspace()
plugin.write(
"""
from surfsense_plugin_sdk import action, document
@action("echo")
def echo(text: str) -> None:
print(document.add(title="Word count #1", content=text).id)
"""
)
finished = plugin.run(
"echo", {"text": "a b a"}, context=real_app.context(workspace)
)
assert finished.returncode == 0, finished.stderr
note = real_app.call(
"GET", f"/workspaces/{workspace}/documents/{finished.stdout.strip()}"
)
assert note["document_type"] == "NOTE"
assert (note["title"], note["content"]) == ("Word count #1", "a b a")
assert note["document_metadata"] == {
"plugin_id": "example",
"plugin_version": None,
"action": "echo",
"run_id": 41,
}
@pytest.mark.covers("document.list")
def test_list_returns_every_document_the_app_holds(plugin, real_app) -> None:
"""More than one of the app's pages, so a change to its page size shows here."""
workspace = real_app.new_workspace()
for number in range(201):
real_app.call(
"POST",
f"/workspaces/{workspace}/documents",
{"title": f"Note {number}", "content": "x"},
)
plugin.write(
"""
from surfsense_plugin_sdk import action, document
@action("echo")
def echo(text: str) -> None:
found = document.list()
print(len(found), found[0].title, found[-1].title)
"""
)
finished = plugin.run("echo", {"text": "hi"}, context=real_app.context(workspace))
assert finished.returncode == 0, finished.stderr
assert finished.stdout == "201 Note 200 Note 0\n"
@pytest.mark.covers("document.update")
def test_update_changes_the_note_in_the_app(plugin, real_app) -> None:
"""What the verb returns is what the app now holds."""
workspace = real_app.new_workspace()
created = real_app.call(
"POST",
f"/workspaces/{workspace}/documents",
{"title": "Draft", "content": "first"},
)
plugin.write(
f"""
from surfsense_plugin_sdk import action, document
@action("echo")
def echo(text: str) -> None:
updated = document.update({created["id"]}, title="Final", content=text)
print(updated.id, updated.title)
"""
)
finished = plugin.run(
"echo", {"text": "second"}, context=real_app.context(workspace)
)
assert finished.returncode == 0, finished.stderr
assert finished.stdout == f"{created['id']} Final\n"
note = real_app.call("GET", f"/workspaces/{workspace}/documents/{created['id']}")
assert (note["title"], note["content"]) == ("Final", "second")
def test_a_refusal_carries_the_apps_own_reason(plugin, real_app) -> None:
"""The message an author reads is the one the real app writes."""
workspace = real_app.new_workspace()
plugin.write(
"""
from surfsense_plugin_sdk import action, document
@action("echo")
def echo(text: str) -> None:
document.update(999999, title=text)
"""
)
finished = plugin.run("echo", {"text": "hi"}, context=real_app.context(workspace))
assert finished.returncode == 1
assert "AppRefused: document not found\n" in finished.stderr

View file

@ -0,0 +1,62 @@
import importlib.util
import inspect
from collections.abc import Callable, Iterator
from pathlib import Path
from types import ModuleType
import pytest
import surfsense_plugin_sdk
# With the unit tests, so a pull request that adds a verb fails the SDK's own
# check until it adds the verb's contract test too.
pytestmark = pytest.mark.unit
CONTRACT_TESTS = Path(__file__).parent
def _public_verbs() -> Iterator[str]:
"""Each function a plugin can call on the app, such as document.add."""
for name in surfsense_plugin_sdk.__all__:
module = getattr(surfsense_plugin_sdk, name)
if not (
isinstance(module, ModuleType)
and module.__name__.startswith(f"{surfsense_plugin_sdk.__name__}.app.")
):
continue
domain = module.__name__.rsplit(".", 1)[1]
for verb, thing in vars(module).items():
if (
inspect.isfunction(thing)
and thing.__module__ == module.__name__
and not verb.startswith("_")
):
yield f"{domain}.{verb}"
def _contract_tests() -> Iterator[Callable[..., object]]:
"""Every test function in the contract test files, read without running them."""
for path in CONTRACT_TESTS.glob("test_*.py"):
if path == Path(__file__):
continue
spec = importlib.util.spec_from_file_location(path.stem, path)
assert spec is not None and spec.loader is not None
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
yield from (
thing
for name, thing in vars(module).items()
if name.startswith("test_") and inspect.isfunction(thing)
)
def test_every_public_verb_has_a_contract_test() -> None:
"""A verb no contract test calls is one the app could break without anyone seeing."""
covered = {
mark.args[0]
for test in _contract_tests()
for mark in getattr(test, "pytestmark", [])
if mark.name == "covers"
}
assert set(_public_verbs()) - covered == set()

View file

@ -42,7 +42,7 @@ def echo(text: str) -> None:
assert request.body == {
"title": "Note hi",
"content": "hi",
"metadata": {
"document_metadata": {
"plugin_id": "example",
"plugin_version": None,
"action": "echo",

View file

@ -160,6 +160,7 @@ def create_note(
title=payload.title,
document_type=DocumentType.NOTE,
content=payload.content,
document_metadata=payload.document_metadata,
)
session.add(note)
session.commit()

View file

@ -1,7 +1,7 @@
from datetime import datetime
from typing import Annotated
from pydantic import BaseModel, ConfigDict, StringConstraints
from pydantic import BaseModel, ConfigDict, JsonValue, StringConstraints
from modules.documents.models import DocumentStatus, DocumentType
@ -13,8 +13,11 @@ DocumentTitle = Annotated[
class NoteCreate(BaseModel):
"""A document the user writes directly, with no file behind it."""
model_config = ConfigDict(extra="forbid")
title: DocumentTitle
content: str
document_metadata: dict[str, JsonValue] | None = None
class DocumentUpdate(BaseModel):
@ -42,6 +45,7 @@ class DocumentDetail(DocumentRead):
"""One document, including the text that was extracted from it."""
content: str | None
document_metadata: dict[str, JsonValue] | None
class ChunkRead(BaseModel):

View file

@ -70,6 +70,89 @@ async def test_a_note_starts_pending(client: AsyncClient, workspace_id: int) ->
assert response.json()["content"] == "# Kickoff\n\nagreed to ship"
PLUGIN_NOTE_METADATA = {
"plugin_id": "example",
"plugin_version": None,
"action": "count-words",
"run_id": 41,
}
async def test_a_note_keeps_the_document_metadata_it_was_written_with(
client: AsyncClient, workspace_id: int
) -> None:
"""A plugin names itself on the notes it adds, and reading the note gives it back."""
created = await client.post(
f"/workspaces/{workspace_id}/documents",
json={
"title": "Word count #1",
"content": "a b a",
"document_metadata": PLUGIN_NOTE_METADATA,
},
)
read = await client.get(
f"/workspaces/{workspace_id}/documents/{created.json()['id']}"
)
assert created.status_code == 201
assert read.json()["document_metadata"] == PLUGIN_NOTE_METADATA
async def test_a_note_written_without_document_metadata_has_none(
client: AsyncClient, workspace_id: int
) -> None:
"""A note the user writes in the app says nothing about where it came from."""
created = await client.post(
f"/workspaces/{workspace_id}/documents",
json={"title": "Kickoff", "content": "x"},
)
read = await client.get(
f"/workspaces/{workspace_id}/documents/{created.json()['id']}"
)
assert read.json()["document_metadata"] is None
async def test_editing_a_note_keeps_its_document_metadata(
client: AsyncClient, workspace_id: int
) -> None:
"""The note still came from the plugin after its text changes."""
created = await client.post(
f"/workspaces/{workspace_id}/documents",
json={
"title": "Word count #1",
"content": "a b a",
"document_metadata": PLUGIN_NOTE_METADATA,
},
)
document_id = created.json()["id"]
await client.patch(
f"/workspaces/{workspace_id}/documents/{document_id}",
json={"title": "Word count #2", "content": "b a b"},
)
read = await client.get(f"/workspaces/{workspace_id}/documents/{document_id}")
assert read.json()["document_metadata"] == PLUGIN_NOTE_METADATA
async def test_a_note_with_a_field_the_app_does_not_know_is_refused(
client: AsyncClient, workspace_id: int
) -> None:
"""A client and the app that disagree on a name find out at once, not by losing it."""
response = await client.post(
f"/workspaces/{workspace_id}/documents",
json={"title": "Kickoff", "content": "x", "metadata": {"plugin_id": "x"}},
)
listed = await client.get(f"/workspaces/{workspace_id}/documents")
assert response.status_code == 422
assert response.json()["detail"][0]["loc"] == ["body", "metadata"]
assert listed.json() == []
async def test_the_list_omits_document_content(
client: AsyncClient, workspace_id: int
) -> None: