diff --git a/.claude/rules/file-map.md b/.claude/rules/file-map.md index ec8d6226..c4ce62ca 100644 --- a/.claude/rules/file-map.md +++ b/.claude/rules/file-map.md @@ -174,8 +174,12 @@ Full module index for LocalChat. **Keep this current** — update in the same co | `src/tools/executor.py` | Ollama tool-call loop (multi-turn until final response) | | `src/tools/registry.py` | Tool registration with JSON schemas | | `src/tools/builtin.py` | Built-in tools: document search, calculator, datetime | -| `src/tools/plugin_loader.py` | Loads `.py` plugins from `plugins/` at startup | -| `plugins/example_plugin.py` | Reference plugin demo (`word_count`, `reverse_text` tools) | +| `src/tools/plugin_loader.py` | Loads `.py` plugins from `plugins/` at startup; one with `setup(services)` gets its manifest validated and its services built, one without loads as legacy. A failing plugin is disabled alone, with any tool it registered removed | +| `src/plugins/manifest.py` | PC-1 — `read_manifest()`: `PLUGIN_SCOPE`, `PLUGIN_MIN_ROLE`, `PLUGIN_HOOKS` (checked against the catalogue), `PLUGIN_CONFIG`; `PLUGIN_CONTRIBUTES` must be False. Raises `ManifestError` | +| `src/plugins/services.py` | PC-1 — the handles a plugin's `setup()` receives: `retrieval` (the request's workspace only, tier capped by the manifest), `llm.embed`, `config`, `identity` (read only), `tools.register` (refuses a taken name) | +| `src/plugins/__init__.py` | `build_services()` — types the plugin's settings from `PLUGIN__` and assembles its handles | +| `plugins/example_plugin.py` | Example plugin on the PC-1 contract — `setup(services)`, a manifest and a setting (`word_count`, `reverse_text` tools) | +| `tests/unit/test_plugin_contract.py` | PC-1 — the manifest (every malformed field refused by name), each handle (retrieval reads the request's workspace and refuses a tier beyond its reach or a key's), the executor hiding and refusing a tool below its role, the loader disabling one plugin alone and removing its half-registered tools, and the guard recording who it admitted | | `plugins/README.md` | Plugin authoring guide | | **Graph / Memory / Performance** | | | `src/graph/store.py` | `GraphStore` ABC + `PostgresGraphStore`, which reaches the entities through `db.entities` | @@ -242,6 +246,7 @@ Full module index for LocalChat. **Keep this current** — update in the same co | `tests/unit/test_injection_flags.py` | GR-1c — each kind is found by the text it names and stays quiet on the near-misses business documents carry (`System: 2`, "you are now logged in") | | `tests/unit/test_frontend_injection_flag_badge.py` | GR-1c — the Documents page badges a flagged document, names the kinds, and leaves an unflagged or pre-scan one alone | | `tests/integration/test_injection_fixtures.py` | GR-1's acceptance — eleven adversarial documents (text, Markdown, Word paragraph and table cell, spreadsheet cells, slide, white-on-white PDF) ingested for real: each flagged with its kind, a control not, the instruction kept inside its fence, nothing reaching memory extraction; PPTX speaker notes asserted not ingested | +| `src/utils/caller.py` | `RequestCaller` and `ROLE_LEVELS` (viewer < editor < owner < admin) — who the request was authorised as, bound for the chat stream like the scope, so the tool executor can check a tool's minimum role and a plugin's `identity` handle can read it (PC-1) | | `src/utils/text.py` | `STOP_WORDS` and `content_terms()` — a question's topic words, any script, operator-free; what the lexical arm searches for, and the stop-word set active learning shares | | **Infra / Config** | | | `requirements.in` | Runtime dependencies, hand-written — the input `requirements.txt` is compiled from | diff --git a/.claude/rules/plugins.md b/.claude/rules/plugins.md index 70ced308..50c9c50c 100644 --- a/.claude/rules/plugins.md +++ b/.claude/rules/plugins.md @@ -3,25 +3,20 @@ How plugins extend LocalChat without destabilising the core. Read alongside the "Plugin Contract" section in `CLAUDE.md` — that states the principle; this is the rule. -> ## ⚠ Status: designed, not built (verified 2026-08-27, re-checked 2026-10-06) -> -> **Everything below describes PC-1..PC-3, which have not been implemented.** They are -> ROADMAP Sprints 10–11, which now run after the quality plan (Initiative 12) has built the -> service layer they wrap. PC-4, the `core-without-plugins` job, was built on 2026-10-23 -> (QP-3). Apart from it, none of the following exists in the repository today: +> ## ⚠ Status: partly built (PC-1 built 2026-11-11; PC-4 2026-10-23) > > | Described here | Reality | > |---|---| -> | The services catalogue (`retrieval`, `llm`, `storage`, `config`, `identity`) | No service handles; a plugin imports `src.tools.registry` directly | -> | The hooks catalogue (`on_document_ingested`, `on_scheduler_tick`, …) | No hook bus | -> | The manifest (`PLUGIN_SCOPE`, `PLUGIN_MIN_ROLE`, `PLUGIN_HOOKS`, `PLUGIN_CONFIG`) | `plugin_loader.py` reads none of these; it reads an optional `PLUGIN_META` dict for display only | -> | `plugins/_echo/` reference plugin | Absent. `plugins/` holds `example_plugin.py` | +> | The manifest (`PLUGIN_SCOPE`, `PLUGIN_MIN_ROLE`, `PLUGIN_HOOKS`, `PLUGIN_CONFIG`) | **Built** (PC-1) in `src/plugins/manifest.py`; a malformed one disables that plugin only | +> | The services catalogue | **Built** (PC-1) in `src/plugins/services.py`: `retrieval`, `llm` (`embed` only), `config`, `identity`, and `tools` for registering. `storage` and `llm.complete` are deferred until a consumer needs them | +> | `PLUGIN_MIN_ROLE` | **Enforced by the core** (PC-1): the tool executor hides and refuses a tool below its minimum role | +> | The hooks catalogue (`on_document_ingested`, `on_scheduler_tick`, …) | No hook bus (PC-2). `PLUGIN_HOOKS` is validated against the names below and fires nothing | +> | `plugins/_echo/` reference plugin | Absent (PC-3). `plugins/` holds `example_plugin.py`, on the PC-1 contract | > | The `core-without-plugins` CI job | **Built** (2026-10-23) in `.github/workflows/tests.yml`; a required check since 2026-10-30 | > -> **What actually ships** is a much smaller thing, and it is documented accurately in -> [`plugins/README.md`](../../plugins/README.md): drop a `.py` file in `plugins/`, decorate -> functions with `@tool_registry.register`, and they become LLM-callable tools. That is the -> live contract. This file is the target. +> A plugin without `setup(services)` still loads as a *legacy* plugin that imports +> `src.tools.registry` directly. [`plugins/README.md`](../../plugins/README.md) documents +> what ships; the sections below are the target, and say where PC-1 departed from it. > > The document is kept in present tense below because it is a specification, and rewriting a > spec into the conditional makes it harder to build against. Read it as "shall", not "does". @@ -53,11 +48,12 @@ with a stable method surface and never imports the core module behind it. | Service | Surface the plugin gets | Behind it (plugin never touches) | |---------|------------------------|----------------------------------| -| `retrieval` | `retrieve(query, scope, workspace_id)` -> ranked chunks. `scope` is `local`/`global`/`hybrid`. | `src/rag/retrieval.py`, pgvector, reranker | -| `llm` | `complete(prompt, ...)`, `embed(text)` | `src/ollama_client.py`, model routing, warm-up | -| `storage` | A namespaced handle for the plugin's own tables; optional Clark-Wilson soft-delete helpers | `src/db/connection.py` pool, psycopg3 | -| `config` | Validated access to the plugin's declared keys + defaults | `src/config.py`, `.env` loading | -| `identity` | `require_role(min_role)` -> ok / 403 | `src/security_fastapi.py`, JWT claims | +| `retrieval` | `retrieve(query, knowledge_scope, top_k)` -> ranked chunks. `knowledge_scope` is `local`/`global`/`hybrid`, capped by `PLUGIN_SCOPE`; a workspace API key gets `local` only. **No workspace argument:** it searches the workspace the request was authorised for (decided 2026-11-11; the earlier `workspace_id` parameter would have let a plugin choose its target) | `src/rag/retrieval.py`, pgvector, reranker | +| `llm` | `embed(text)`. `complete(prompt, ...)` is deferred: tools run synchronously inside the chat stream, so a completion from one needs that changed first | `src/ollama_client.py`, model routing, warm-up | +| `storage` | *Deferred until a consumer needs it (2026-11-11).* A namespaced handle for the plugin's own tables; optional Clark-Wilson soft-delete helpers. Open: who migrates those tables, and whether RLS applies | `src/db/connection.py` pool, psycopg3 | +| `config` | Validated access to the plugin's declared keys + defaults, from `PLUGIN__` | `src/config.py` (`plugin_setting`), `.env` loading | +| `identity` | `current()` -> the caller, `has_role(min_role)` -> bool. Reads only: `PLUGIN_MIN_ROLE` is enforced by the core before a tool runs | `src/utils/caller.py`, bound from the workspace guard | +| `tools` | `register(name, description, parameters)`, tagged with the plugin and its minimum role; a taken name is refused | `src/tools/registry.py` | **Adding a service.** Only when a real consumer needs it, and only with a name that describes the capability generically. A `pricing_lookup` service is a leak; a @@ -84,8 +80,8 @@ Every plugin declares a manifest the core reads at load time. Minimal shape: ```python PLUGIN_SCOPE = "hybrid" # local | global | hybrid (retrieval reach) -PLUGIN_MIN_ROLE = "viewer" # viewer | user | admin (who may invoke) -PLUGIN_CONTRIBUTES = False # may it write to the Global Knowledge Base? +PLUGIN_MIN_ROLE = "viewer" # viewer | editor | owner | admin (who may invoke) +PLUGIN_CONTRIBUTES = False # may it write to the GKB? Must be False: no service writes PLUGIN_HOOKS = ["on_document_ingested", "on_scheduler_tick", "on_tool_invocation"] PLUGIN_CONFIG = { # keys the core loads into the config service "FEEDBACK_THRESHOLD": ("float", 0.30), diff --git a/CHANGELOG.md b/CHANGELOG.md index 73bca45b..2e3646c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,25 @@ reasoning attached, in [docs/history/LESSONS_LEARNED.md](docs/history/LESSONS_LE ## [Unreleased] +### Added + +- **Plugins get a manifest and services (PC-1).** A plugin defines `setup(services)` and + registers its tools through `services.tools.register`, importing nothing from `src`. Its + manifest declares how far its retrieval reaches (`PLUGIN_SCOPE`), who may use it + (`PLUGIN_MIN_ROLE`) and its settings (`PLUGIN_CONFIG`, read from + `PLUGIN__`). `services.retrieval` searches only the workspace the request was + authorised for. The core enforces the minimum role: a tool is not offered to a caller below + it, and is refused if the model names it anyway. A malformed manifest disables that plugin + alone. **Upgrading:** nothing to do. A plugin without `setup()` still loads as before, now + listed as legacy in `/api/plugins` with a warning in the log. + +### Fixed + +- **A failing plugin no longer leaves tools behind, and a reloaded one can be unloaded.** A + plugin that raised after registering a tool kept that tool. A reload recorded none of the + plugin's tools, so unloading it afterwards left them registered. And a loader given an + empty registry used the global one instead, since an empty registry is falsy. + ## [3.6.0] — 2026-11-11 The release of the QP-9 review's open findings (QP-10): R-8 to R-12 and item 13 are fixed, diff --git a/CLAUDE.md b/CLAUDE.md index b3ba4816..fa4f714b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -156,7 +156,7 @@ LocalChat supports plugins (LLM tools, connectors, background work) that extend The enforcing procedure is a CI job, `core-without-plugins`, that verifies architectural integrity: the core builds, lints, and passes its full test suite **with the `plugins/` directory and all private plugin code absent.** If that job goes red, a plugin dependency has leaked into the core. -> **It exists since 2026-10-23** (PC-4, built with QP-3): it deletes everything in `plugins/` but its README, then runs `ruff` and the fast suite. It has been a required check since 2026-10-30 (see Pull Requests). The rest of the contract — the services catalogue, the hook bus, the manifest, the `_echo` reference plugin — is likewise designed and unbuilt; what actually ships is the simpler tool-plugin loader described in [`plugins/README.md`](plugins/README.md). See the status banner in [.claude/rules/plugins.md](.claude/rules/plugins.md). +> **It exists since 2026-10-23** (PC-4, built with QP-3): it deletes everything in `plugins/` but its README, then runs `ruff` and the fast suite. It has been a required check since 2026-10-30 (see Pull Requests). The manifest and the services catalogue were built on 2026-11-11 (PC-1, `src/plugins/`), with `PLUGIN_MIN_ROLE` enforced by the core; `storage` and `llm.complete` wait for a consumer. The hook bus (PC-2) and the `_echo` reference plugin (PC-3) are designed and unbuilt. What ships is described in [`plugins/README.md`](plugins/README.md). See the status banner in [.claude/rules/plugins.md](.claude/rules/plugins.md). Full rules -> [.claude/rules/plugins.md](.claude/rules/plugins.md) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 0733cf03..1d093a8d 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -463,6 +463,7 @@ to empty, in which case the router falls back to the active model. > fixes this product at one node and one process; scale with a second node instead. | `LOG_FILE` | `logs/app.log` | Path for the file sink | | `PLUGINS_DIR` | `plugins` | Directory scanned for plugins | +| `PLUGIN__` | the plugin's declared default | A setting a plugin declares in `PLUGIN_CONFIG`. `` is its file name upper-cased, with anything but letters and digits made `_`. Read by `plugin_setting()` in `config.py`; see [plugins/README.md](../plugins/README.md) | | `PRESENCE_TTL_SECONDS` | `30` | How long a workspace presence entry stays live | | `CONNECTOR_LOCAL_ROOTS` | *(empty)* | Comma-separated absolute directories the `local_folder` connector may watch. **Empty disables that connector type** — see below | diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 18a44dc4..b6cd9cfc 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -594,7 +594,25 @@ The mechanism that lets plugins extend LocalChat without the core ever depending --- -### PC-1 — Service catalogue and manifest loader +### PC-1 — Service catalogue and manifest loader ✅ (built 2026-11-11) + +> **As built.** Four of the spec's points were decided differently by the maintainer on +> 2026-11-11, and `.claude/rules/plugins.md` was amended to match: +> - `retrieval` takes no `workspace_id`. It searches the workspace the request was +> authorised for, so a plugin cannot aim a search at one its caller was never admitted to. +> The tier is capped by `PLUGIN_SCOPE`, and a workspace API key gets `local` only. +> - `PLUGIN_MIN_ROLE` is enforced by the core, not by the plugin: the tool executor hides a +> tool from the model below its minimum role and refuses it if named anyway. The roles are +> the real ones (viewer < editor < owner, then admin); the workspace guard records the +> caller's, and the chat route binds it for the stream. `identity` only reads. +> `require_workspace_role_dep`, which the spec named, was deleted by QP-1. +> - `storage` is deferred until a consumer needs it, and so is `llm.complete`: tools run +> synchronously inside the chat stream, so a completion from one needs that changed first. +> - A plugin without `setup(services)` still loads, as *legacy*, so the release stays a minor. +> +> The built-in tools stay on the core directly: they are core code, and the contract governs +> plugins. Building it found two loader bugs, fixed here: an empty registry was swapped for +> the global one, and a reload recorded no tools, so a later unload left them registered. **Current foundation (do not rebuild from scratch).** The following already exists and works: - `src/tools/registry.py` — `ToolRegistry` with `@register` decorator, JSON-schema export, execute-by-name, `unregister` for hot-reload. @@ -2038,7 +2056,7 @@ the generation path is a deterministic pipeline unless an answer-level eval says | PG-0..PG-8 | **Production-grade hardening** ✅ — see [PRODUCTION_PLAN.md](history/PRODUCTION_PLAN.md). Gated everything below; **gate lifted 2026-08-31**. | done | | 8 | GKB-1 (schema + two-tier retrieval) ✅ 2026-10-09 — read-only global tier under RLS (0019) | 1 week | | 9 | GKB-2 (contribution workflow) ✓ backend 2026-10-19 (admin-only contribute, migration 0020), UI 2026-10-20 | 1 week | -| 10 | PC-1 + PC-2 (services, hooks, scheduler) | 1 week | +| 10 | PC-1 ✅ 2026-11-11 (manifest, services, the core's role gate) + PC-2 (hooks, scheduler) | 1 week | | 11 | PC-3 + PC-4 (echo plugin, CI gate) | 1 week | | 12 | PR-1 (pricing plugin — private repo) | 1–2 weeks | | 13 | CONN-1 (connector authorisation model — decision, no code) | 2–3 days | diff --git a/plugins/README.md b/plugins/README.md index 9fd813bd..ec4c7690 100644 --- a/plugins/README.md +++ b/plugins/README.md @@ -1,8 +1,8 @@ # LocalChat Plugin System -> Verified against `src/tools/` and `src/app_bootstrap.py` at `a04da4d` on 2026-11-10. This -> is the plugin mechanism that ships; the contract in `.claude/rules/plugins.md` is the -> designed, unbuilt target. +> Verified against `src/tools/`, `src/plugins/` and `src/app_bootstrap.py` on 2026-11-11. +> PC-1 of the contract in `.claude/rules/plugins.md` ships: the manifest and the services +> below. The hook bus (PC-2) and the `_echo` reference plugin (PC-3) do not exist yet. Drop a `.py` file here — LocalChat loads it automatically at startup. @@ -11,7 +11,7 @@ Drop a `.py` file here — LocalChat loads it automatically at startup. ## How it works 1. On startup, `PluginLoader.load_all()` scans this directory for `*.py` files (skips `_`-prefixed files). -2. Each file is dynamically imported. Any `@tool_registry.register` decorated functions in the file are registered as LLM-callable tools. +2. Each file is imported. If it defines `setup(services)`, the loader validates its manifest, builds its services and calls `setup`, where the plugin registers its tools. 3. The LLM can call your tools during chat just like the built-in ones (`search_documents`, `list_documents`, `calculate`, `get_current_datetime`). The tool loop runs only when retrieval found no document or web context for the question, and only with `TOOL_CALLING_ENABLED=true` (the default). No source-code changes needed — just drop a file and restart (or call `POST /api/plugins/reload`). @@ -21,40 +21,72 @@ No source-code changes needed — just drop a file and restart (or call `POST /a ## Plugin file contract ```python -# plugins/my_plugin.py +# plugins/my_plugin.py — imports nothing from src -from src.tools.registry import tool_registry +PLUGIN_META = {"name": "My Plugin", "version": "1.0.0", "description": "Shown in GET /api/plugins."} -# Optional — shown in GET /api/plugins -PLUGIN_META = { - "name": "My Plugin", - "version": "1.0.0", - "author": "Your Name", - "description": "What this plugin does.", +# The manifest. Every name is optional; these are the defaults. +PLUGIN_SCOPE = "local" # retrieval reach: local | global | hybrid +PLUGIN_MIN_ROLE = "viewer" # who may use its tools: viewer | editor | owner | admin +PLUGIN_HOOKS = [] # validated against the catalogue; nothing fires them until PC-2 +PLUGIN_CONFIG = { # KEY: (type, default); type is str | int | float | bool + "TOP_K": ("int", 5), } -@tool_registry.register( - name="my_tool", # unique name the LLM uses - description="What this tool does.", # shown to the LLM - parameters={ # JSON Schema for arguments - "type": "object", - "properties": { - "input": {"type": "string", "description": "The input value."}, - }, - "required": ["input"], - }, -) -def my_tool(input: str) -> str: - return f"You passed: {input}" + +def setup(services): + top_k = services.config.get("TOP_K") + + @services.tools.register( + name="my_tool", # unique; a name already taken is refused + description="What this tool does.", # shown to the LLM + parameters={"type": "object", "properties": {"q": {"type": "string"}}, "required": ["q"]}, + ) + def my_tool(q: str) -> str: + hits = services.retrieval.retrieve(q, top_k=top_k) + return "\n".join(h["text"] for h in hits) ``` +A malformed manifest, a setting that does not parse, or a `setup` that raises disables +**that plugin only**: the error shows in `GET /api/plugins`, any tool it registered before +failing is removed, and the application starts normally. + +### The services + +| Handle | What it does | What it cannot do | +|---|---|---| +| `services.tools.register(name, description, parameters)` | Registers a tool, tagged with this plugin and its `PLUGIN_MIN_ROLE` | Replace a tool that already exists | +| `services.retrieval.retrieve(query, knowledge_scope="local", top_k=5)` | Searches the documents the current request may read; returns `text`, `filename`, `chunk_index`, `similarity`, `tier` per chunk | Name a workspace: it always searches the one the request was authorised for. A tier beyond `PLUGIN_SCOPE`, or any but `local` for a workspace API key, raises `PermissionError` | +| `services.llm.embed(text)` | The embedding the core would store for `text` | Chat completion: not offered yet (see below) | +| `services.identity.current()`, `.has_role(role)` | Who is asking, and at what role | Grant anything: it only reads | +| `services.config.get(KEY)` | A declared setting, typed, from `PLUGIN__` or its default | Read anything the manifest did not declare | + +`PLUGIN__` is the plugin's file name upper-cased, with anything but letters and +digits made `_`: `TOP_K` for `my_plugin.py` is `PLUGIN_MY_PLUGIN_TOP_K`. + +**Who may use a tool is the core's decision.** A caller below `PLUGIN_MIN_ROLE` is never +offered the plugin's tools, and if the model names one anyway it is refused as an unknown +tool. Roles run viewer < editor < owner (the caller's role in the workspace, or a workspace +API key's) and then admin (a global administrator). + +**Not offered yet:** `storage` (tables of the plugin's own) and `llm.complete()`. Each waits +for a plugin that needs it. Tools run synchronously inside the chat stream, so a completion +from inside one needs that changed first. `PLUGIN_CONTRIBUTES` must be `False`. + +### Legacy plugins + +A file without `setup(services)` loads the way every plugin did before PC-1: it imports +`src.tools.registry` and registers at import time. It still works, is listed with +`"legacy": true` in `GET /api/plugins`, and logs a warning. Its tools are open to every +role. Move it to `setup(services)` — `example_plugin.py` shows the result. + --- ## API endpoints | Endpoint | Method | Description | |---|---|---| -| `/api/plugins` | `GET` | List all loaded plugins and their tools — admin only | +| `/api/plugins` | `GET` | List loaded plugins with their tools, manifest, `legacy` flag and any load error — admin only | | `/api/plugins/reload` | `POST` | Hot-reload all plugins from disk (no restart needed) — admin only | --- @@ -65,18 +97,25 @@ def my_tool(input: str) -> str: |---|---|---| | `PLUGINS_ENABLED` | `true` | Set to `false` to disable plugin loading | | `PLUGINS_DIR` | `plugins` | Directory to scan (relative to repo root) | +| `PLUGIN__` | the manifest's default | A plugin's declared setting | --- ## Rules -- Tool names must be **unique** across all plugins and builtins. A plugin that reuses an existing name **replaces** it, with only a warning logged — including a built-in such as `search_documents`, whose workspace scoping the replacement would then have to re-implement. Treat a plugin as code with the application's full authority (SECURITY.md, risk 4). +- The contract governs dependencies, not trust. A plugin runs **in the application's + process** and Python cannot stop it importing `src` or anything else. Treat a plugin as + code with the application's full authority (SECURITY.md, risk 4). What the contract buys + is that the core never depends on a plugin, and that a well-behaved plugin cannot reach a + workspace or a role its caller does not have. +- Tool names must be **unique**. Through `services.tools.register` a taken name is refused. +- Register tools **inside `setup()`**. The handle closes when `setup()` returns, so the loader knows every tool a plugin owns and can unload it. + A legacy plugin that reuses a name still **replaces** it, with only a warning logged. - Plugin files starting with `_` (e.g. `__init__.py`) are **skipped**. -- Plugins run in the same process — they have full access to the Python environment. -- Keep tool handlers **fast and side-effect-free** where possible; the LLM waits for the result. +- Keep tool handlers **fast**: they run inside the chat stream, and the LLM waits for the result. --- ## Example -See [`example_plugin.py`](example_plugin.py) for a working two-tool example (`word_count`, `reverse_text`). +See [`example_plugin.py`](example_plugin.py) for a working two-tool example (`word_count`, `reverse_text`) with a manifest and a setting. diff --git a/plugins/example_plugin.py b/plugins/example_plugin.py index 4021f37d..e988e681 100644 --- a/plugins/example_plugin.py +++ b/plugins/example_plugin.py @@ -6,8 +6,10 @@ LocalChat discovers and loads it automatically on the next startup, or immediately via ``POST /api/plugins/reload``. -Required: at least one ``@tool_registry.register`` decorated function. -Optional: a ``PLUGIN_META`` dict for display in ``GET /api/plugins``. +A plugin defines ``setup(services)`` and registers its tools there, through +``services.tools.register``. It imports nothing from ``src``: everything it needs +from the core arrives on ``services``. The ``PLUGIN_*`` names below are its manifest; +each is optional and shown with its default. See ``plugins/README.md``. To write your own plugin: 1. Copy this file to ``plugins/my_plugin.py``. @@ -15,66 +17,63 @@ 3. Restart or call ``POST /api/plugins/reload`` — done. """ -from src.tools.registry import tool_registry - # --------------------------------------------------------------------------- -# Metadata (optional — shown in GET /api/plugins) +# Manifest # --------------------------------------------------------------------------- PLUGIN_META = { "name": "Example Plugin", - "version": "1.0.0", + "version": "2.0.0", "author": "LocalChat Team", - "description": "Demonstrates the plugin system with two sample tools.", + "description": "Demonstrates the plugin contract with two sample tools.", } +PLUGIN_SCOPE = "local" # retrieval reach: local | global | hybrid +PLUGIN_MIN_ROLE = "viewer" # who may use its tools: viewer | editor | owner | admin +PLUGIN_CONFIG = { # read from PLUGIN_EXAMPLE_PLUGIN_; the default applies when unset + "MAX_CHARS": ("int", 10_000), +} -# --------------------------------------------------------------------------- -# Tool 1 — word_count -# --------------------------------------------------------------------------- -@tool_registry.register( - name="word_count", - description=( - "Count the number of words in the provided text. " - "Useful when the user asks how long a passage or document excerpt is." - ), - parameters={ - "type": "object", - "properties": { - "text": { - "type": "string", - "description": "The text whose words should be counted.", +def setup(services): + max_chars = services.config.get("MAX_CHARS") + + @services.tools.register( + name="word_count", + description=( + "Count the number of words in the provided text. " + "Useful when the user asks how long a passage or document excerpt is." + ), + parameters={ + "type": "object", + "properties": { + "text": { + "type": "string", + "description": "The text whose words should be counted.", + }, }, + "required": ["text"], }, - "required": ["text"], - }, -) -def word_count(text: str) -> str: - """Return word and character counts for *text*.""" - words = len(text.split()) - chars = len(text) - return f"{words} word(s), {chars} character(s)." - - -# --------------------------------------------------------------------------- -# Tool 2 — reverse_text -# --------------------------------------------------------------------------- + ) + def word_count(text: str) -> str: + """Return word and character counts for *text*.""" + text = text[:max_chars] + return f"{len(text.split())} word(s), {len(text)} character(s)." -@tool_registry.register( - name="reverse_text", - description="Reverse the characters in the provided text.", - parameters={ - "type": "object", - "properties": { - "text": { - "type": "string", - "description": "The text to reverse.", + @services.tools.register( + name="reverse_text", + description="Reverse the characters in the provided text.", + parameters={ + "type": "object", + "properties": { + "text": { + "type": "string", + "description": "The text to reverse.", + }, }, + "required": ["text"], }, - "required": ["text"], - }, -) -def reverse_text(text: str) -> str: - """Return *text* with its characters in reverse order.""" - return text[::-1] + ) + def reverse_text(text: str) -> str: + """Return *text* with its characters in reverse order.""" + return text[:max_chars][::-1] diff --git a/pyproject.toml b/pyproject.toml index 1b98fe26..a464bcbc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -134,6 +134,10 @@ module = [ "src.services.users", "src.services.workspaces", "src.routes_fastapi._services", + "src.plugins", + "src.plugins.manifest", + "src.plugins.services", + "src.utils.caller", ] # `strict = true` is global-only; these are the per-module flags it turns on. disallow_untyped_defs = true diff --git a/src/config.py b/src/config.py index b52dc41b..1485b159 100644 --- a/src/config.py +++ b/src/config.py @@ -520,6 +520,18 @@ def _production_errors(errors: list[str]) -> None: # Directory (relative to repo root) that is scanned for .py plugin files. PLUGINS_DIR: str = os.environ.get('PLUGINS_DIR', 'plugins') + +def plugin_setting(plugin: str, key: str) -> str | None: + """The raw value of a plugin's declared setting, ``PLUGIN__``, or None. + + A plugin declares its keys and defaults in ``PLUGIN_CONFIG`` and never reads the + environment itself; the loader asks here and types the value (PC-1). The names are + built from the manifest, so they are documented as a family in CONFIGURATION.md + rather than one by one. + """ + name = "".join(c if c.isalnum() else "_" for c in plugin) + return os.environ.get(f"PLUGIN_{name}_{key}".upper()) + # ============================================================================ # DOCS SYSTEM CONFIGURATION # ============================================================================ diff --git a/src/plugins/__init__.py b/src/plugins/__init__.py new file mode 100644 index 00000000..ffbde4a8 --- /dev/null +++ b/src/plugins/__init__.py @@ -0,0 +1,74 @@ +"""The plugin contract's core side: the manifest and the services a plugin receives (PC-1).""" + +from __future__ import annotations + +from typing import Any + +from .. import config +from ..tools.registry import ToolRegistry +from .manifest import ( + HOOKS, + SCOPES, + ConfigValue, + Manifest, + ManifestError, + parse_setting, + read_manifest, +) +from .services import ( + ConfigHandle, + IdentityHandle, + LlmHandle, + PluginServices, + RetrievalHandle, + ToolsHandle, +) + + +def build_services( + name: str, + manifest: Manifest, + registry: ToolRegistry, + *, + processor: Any = None, # Any: DocumentProcessor; the singleton unless a test passes one + client: Any = None, # Any: OllamaClient; likewise +) -> PluginServices: + """Assemble the handles for plugin *name*. ManifestError when a setting does not parse.""" + values: dict[str, ConfigValue] = {} + for key, (type_name, default) in manifest.config.items(): + raw = config.plugin_setting(name, key) + try: + values[key] = default if raw is None else parse_setting(raw, type_name) + except ManifestError as exc: + raise ManifestError(f"setting {key}: {exc}") from exc + + if processor is None: + from ..rag import doc_processor as processor + if client is None: + from ..ollama_client import ollama_client as client + + return PluginServices( + name=name, + manifest=manifest, + config=ConfigHandle(values), + llm=LlmHandle(client), + retrieval=RetrievalHandle(processor, manifest.scope), + identity=IdentityHandle(), + tools=ToolsHandle(registry, source=name, min_role=manifest.min_role), + ) + + +__all__ = [ + "HOOKS", + "SCOPES", + "ConfigHandle", + "IdentityHandle", + "LlmHandle", + "Manifest", + "ManifestError", + "PluginServices", + "RetrievalHandle", + "ToolsHandle", + "build_services", + "read_manifest", +] diff --git a/src/plugins/manifest.py b/src/plugins/manifest.py new file mode 100644 index 00000000..1a4b7de4 --- /dev/null +++ b/src/plugins/manifest.py @@ -0,0 +1,124 @@ +"""A plugin's manifest: what it may reach, who may use it, and what it configures. + +Read from module-level names once the plugin file has been imported. Anything wrong +raises :class:`ManifestError`, and the loader disables that plugin and nothing else. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass, field +from types import ModuleType +from typing import Any + +from ..utils.caller import ROLE_LEVELS + +#: How far the plugin's retrieval reaches: its workspace, the global tier, or both. +SCOPES = ("local", "global", "hybrid") +#: The hooks PC-2 will fire. Named now so a manifest is validated against the catalogue. +HOOKS = ("on_document_ingested", "on_scheduler_tick", "on_tool_invocation", "on_route_mount") +_CONFIG_TYPES: dict[str, type] = {"str": str, "int": int, "float": float, "bool": bool} +_KEY = re.compile(r"^[A-Z][A-Z0-9_]*$") + +ConfigValue = str | int | float | bool + + +class ManifestError(ValueError): + """The manifest is malformed; the plugin is disabled.""" + + +@dataclass(frozen=True) +class Manifest: + scope: str = "local" + min_role: str = "viewer" + hooks: tuple[str, ...] = () + #: key -> (type name, default) + config: dict[str, tuple[str, ConfigValue]] = field(default_factory=dict) + + def to_dict(self) -> dict[str, Any]: + return { + "scope": self.scope, + "min_role": self.min_role, + "hooks": list(self.hooks), + "config": {k: {"type": t, "default": d} for k, (t, d) in self.config.items()}, + } + + +def read_manifest(module: ModuleType) -> Manifest: + """Validate the ``PLUGIN_*`` names on *module*; an absent one takes its default.""" + scope = getattr(module, "PLUGIN_SCOPE", "local") + if scope not in SCOPES: + raise ManifestError(f"PLUGIN_SCOPE must be one of {', '.join(SCOPES)}, not {scope!r}") + + min_role = getattr(module, "PLUGIN_MIN_ROLE", "viewer") + if min_role not in ROLE_LEVELS: + raise ManifestError( + f"PLUGIN_MIN_ROLE must be one of {', '.join(ROLE_LEVELS)}, not {min_role!r}" + ) + + # Writing to the Global Knowledge Base is an admin's decision (GKB-2), and no service + # lets a plugin write anything yet. Accepting True would promise what nothing honours. + contributes = getattr(module, "PLUGIN_CONTRIBUTES", False) + if contributes is not False: + raise ManifestError("PLUGIN_CONTRIBUTES must be False: no contribution service exists") + + hooks = getattr(module, "PLUGIN_HOOKS", []) + if not isinstance(hooks, (list, tuple)) or not all(isinstance(h, str) for h in hooks): + raise ManifestError("PLUGIN_HOOKS must be a list of hook names") + unknown = sorted(set(hooks) - set(HOOKS)) + if unknown: + raise ManifestError(f"PLUGIN_HOOKS names unknown hook(s): {', '.join(unknown)}") + + return Manifest( + scope=scope, + min_role=min_role, + hooks=tuple(hooks), + config=_read_config(getattr(module, "PLUGIN_CONFIG", {})), + ) + + +def _read_config(declared: object) -> dict[str, tuple[str, ConfigValue]]: + if not isinstance(declared, dict): + raise ManifestError("PLUGIN_CONFIG must be a dict of KEY: (type, default)") + out: dict[str, tuple[str, ConfigValue]] = {} + for key, spec in declared.items(): + if not isinstance(key, str) or not _KEY.match(key): + raise ManifestError(f"PLUGIN_CONFIG key {key!r} must be UPPER_SNAKE_CASE") + if not (isinstance(spec, tuple) and len(spec) == 2 and spec[0] in _CONFIG_TYPES): + raise ManifestError( + f"PLUGIN_CONFIG[{key!r}] must be (type, default) with type one of " + f"{', '.join(_CONFIG_TYPES)}" + ) + type_name, default = spec + if not _is_type(default, type_name): + raise ManifestError(f"PLUGIN_CONFIG[{key!r}] default {default!r} is not a {type_name}") + out[key] = (type_name, default) + return out + + +def _is_type(value: object, type_name: str) -> bool: + # bool is an int in Python; a flag must not pass as a number, nor a number as a flag. + if type_name == "bool": + return isinstance(value, bool) + if isinstance(value, bool): + return False + if type_name == "float": + return isinstance(value, (int, float)) + return isinstance(value, _CONFIG_TYPES[type_name]) + + +def parse_setting(raw: str, type_name: str) -> ConfigValue: + """Type an environment value for a declared key; raise ManifestError if it does not parse.""" + if type_name == "str": + return raw + if type_name == "bool": + lowered = raw.strip().lower() + if lowered in ("true", "1", "yes"): + return True + if lowered in ("false", "0", "no"): + return False + raise ManifestError(f"{raw!r} is not a bool") + try: + return int(raw) if type_name == "int" else float(raw) + except ValueError as exc: + raise ManifestError(f"{raw!r} is not a {type_name}") from exc diff --git a/src/plugins/services.py b/src/plugins/services.py new file mode 100644 index 00000000..4c8d379b --- /dev/null +++ b/src/plugins/services.py @@ -0,0 +1,163 @@ +"""The capabilities a plugin is handed at load time (PC-1). + +A plugin defines ``setup(services)`` and reaches the core only through the handles on +:class:`PluginServices`. It never imports ``src``. Each handle is named for what it does, +not for any plugin that uses it (`.claude/rules/plugins.md`). + +What a handle cannot do matters as much as what it can: + +* ``retrieval`` reads the workspace the request was authorised for. It takes no + workspace argument, so a plugin cannot point a search at a workspace the caller was + never admitted to (P0-1, P0-2). +* ``identity`` only reads. Whether the caller may use a plugin's tools at all is the + core's decision, made from ``PLUGIN_MIN_ROLE`` before the tool runs. +* There is no ``storage`` handle and no ``llm.complete`` yet. Both wait for a consumer + that needs them; see the status banner in `.claude/rules/plugins.md`. +""" + +from __future__ import annotations + +from collections.abc import Callable +from dataclasses import dataclass +from typing import Any + +from ..rag.retrieval import KnowledgeScope +from ..tools.registry import ToolRegistry +from ..utils.caller import RequestCaller, caller_has_role, current_request_caller +from ..utils.logging_config import get_logger +from ..utils.scope import current_request_scope +from .manifest import ConfigValue, Manifest + +logger = get_logger(__name__) + +#: The tiers each PLUGIN_SCOPE may ask for. +_REACH: dict[str, frozenset[str]] = { + "local": frozenset({"local"}), + "global": frozenset({"global"}), + "hybrid": frozenset({"local", "global", "hybrid"}), +} + + +class ConfigHandle: + """The plugin's own declared settings, typed, with their defaults applied.""" + + def __init__(self, values: dict[str, ConfigValue]) -> None: + self._values = dict(values) + + def get(self, key: str) -> ConfigValue: + """The value of a key the manifest declared; KeyError for one it did not.""" + if key not in self._values: + raise KeyError(f"{key!r} is not declared in PLUGIN_CONFIG") + return self._values[key] + + def as_dict(self) -> dict[str, ConfigValue]: + return dict(self._values) + + +class LlmHandle: + """Embeddings from the core's model server.""" + + def __init__(self, client: Any) -> None: # Any: OllamaClient, imported lazily by the loader + self._client = client + + def embed(self, text: str) -> list[float]: + """The embedding the core would store for *text*; RuntimeError when none can be made.""" + model = self._client.get_embedding_model() + if not model: + raise RuntimeError("no embedding model is available") + ok, vector = self._client.generate_embedding(model, text) + if not ok: + raise RuntimeError("the embedding could not be generated") + return list(vector) + + +class RetrievalHandle: + """Search the documents the current request may read.""" + + def __init__(self, processor: Any, reach: str) -> None: # Any: DocumentProcessor, lazily imported + self._processor = processor + self._allowed = _REACH[reach] + + def retrieve( + self, query: str, knowledge_scope: KnowledgeScope = "local", top_k: int = 5, + ) -> list[dict[str, Any]]: + """Ranked chunks for *query*, from the request's workspace and the tiers allowed. + + PermissionError for a tier beyond the manifest's PLUGIN_SCOPE, or any tier but + ``local`` for a workspace API key, refused rather than narrowed as GKB-3 does: + a narrowed answer could be taken for the one asked for. + """ + if knowledge_scope not in self._allowed: + raise PermissionError(f"PLUGIN_SCOPE does not reach the {knowledge_scope!r} tier") + caller = current_request_caller() + if knowledge_scope != "local" and (caller is None or caller.via_api_key): + raise PermissionError("the global knowledge tier is not open to this caller") + results = self._processor.retrieve_context( + query, top_k=top_k, scope=current_request_scope(), knowledge_scope=knowledge_scope, + ) + return [ + { + "text": r.chunk_text, + "filename": r.filename, + "chunk_index": r.chunk_index, + "similarity": r.similarity, + "tier": r.metadata.get("knowledge_tier", "local"), + } + for r in results + ] + + +class IdentityHandle: + """Who the current request is from. Read only.""" + + def current(self) -> RequestCaller | None: + """The caller, or None outside a request.""" + return current_request_caller() + + def has_role(self, min_role: str) -> bool: + """True when the caller is at least *min_role*: viewer < editor < owner < admin.""" + return caller_has_role(min_role) + + +class ToolsHandle: + """Register LLM-callable tools, tagged with this plugin and its minimum role.""" + + def __init__(self, registry: ToolRegistry, source: str, min_role: str) -> None: + self._registry = registry + self._source = source + self._min_role = min_role + self._closed = False + + def close(self) -> None: + """Refuse registration from now on; the loader calls this when ``setup()`` returns.""" + self._closed = True + + def register( + self, name: str, description: str, parameters: dict[str, Any], + ) -> Callable[[Callable[..., Any]], Callable[..., Any]]: + """Decorator, as ``tool_registry.register``. A name already taken is refused. + + Refused rather than overwritten: a plugin replacing ``search_documents`` would + decide what every chat's document search returns. + """ + if self._closed: + raise RuntimeError("tools are registered in setup(); this handle is closed") + if name in self._registry: + raise ValueError(f"tool {name!r} is already registered") + return self._registry.register( + name=name, description=description, parameters=parameters, + source=self._source, min_role=self._min_role, + ) + + +@dataclass(frozen=True) +class PluginServices: + """Everything a plugin may ask the core for, handed to its ``setup()``.""" + + name: str + manifest: Manifest + config: ConfigHandle + llm: LlmHandle + retrieval: RetrievalHandle + identity: IdentityHandle + tools: ToolsHandle diff --git a/src/routes_fastapi/api_routes.py b/src/routes_fastapi/api_routes.py index 89d9fde8..b3bd63fb 100644 --- a/src/routes_fastapi/api_routes.py +++ b/src/routes_fastapi/api_routes.py @@ -19,6 +19,7 @@ require_auth, ) from ..services import chat +from ..utils.caller import RequestCaller, bind_request_caller, reset_request_caller from ..utils.logging_config import get_logger from ..utils.scope import ( Scope, @@ -26,7 +27,7 @@ request_scope, reset_request_scope, ) -from ..utils.workspace import get_scope, get_workspace_id +from ..utils.workspace import get_caller, get_scope, get_workspace_id from ._authz import deny as _deny from ._authz import deny_additional as _deny_additional from ._services import chat_service @@ -195,6 +196,7 @@ async def _generate_sse( agent_result: Any, routed_rationale: str | None, workspace_scope: Scope, + caller: RequestCaller | None = None, ) -> AsyncGenerator[str, None]: full_response: list[str] = [] model_used = "local" @@ -203,6 +205,9 @@ async def _generate_sse( # the request rather than from their arguments. Unbound they refuse, which is # the point — an unscoped retrieval used to mean every workspace (audit C3). scope_token = bind_request_scope(workspace_scope) + # The caller too: a tool below its minimum role is hidden from the model and refused + # if called anyway, and a plugin reads who is asking through its identity handle. + caller_token = bind_request_caller(caller) try: if plan is not None: yield f"data: {json.dumps({'plan': plan.to_dict()})}\n\n" @@ -242,6 +247,7 @@ async def _generate_sse( logger.exception("[CHAT API] Unexpected error generating response") yield f"data: {json.dumps({'error': 'GenerationError', 'message': 'Failed to generate response', 'done': True})}\n\n" finally: + reset_request_caller(caller_token) reset_request_scope(scope_token) # also ensures cleanup on client disconnect @@ -403,7 +409,7 @@ async def api_chat(request: Request, chat_svc: Chat) -> Any: _generate_sse( plan, tool_executor, active_model, app_state, messages, fields, sources, cloud_client, conversation_id, agent_result, routed_rationale, - scope, + scope, get_caller(request), ), media_type="text/event-stream", headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"}, diff --git a/src/security_fastapi.py b/src/security_fastapi.py index e73c019b..dc48ec81 100644 --- a/src/security_fastapi.py +++ b/src/security_fastapi.py @@ -491,6 +491,10 @@ def _check_api_key_access( # default workspace after authorising against the key's. request.state.resolved_workspace_id = key_workspace request.state.resolved_scope = key_workspace + # The role and identity a tool the model calls is checked against (PC-1). + request.state.resolved_role = key_role + request.state.resolved_user_id = None + request.state.resolved_via_api_key = True return None @@ -549,6 +553,8 @@ def check_workspace_access( # forgetting to pass anything (C1/C2). named = workspace_id or get_workspace_id(request) request.state.resolved_scope = named or ALL_WORKSPACES + request.state.resolved_role = "admin" + request.state.resolved_user_id = principal.user_id return None ws_id = workspace_id or get_workspace_id(request) db = getattr(request.app.state, "db", None) @@ -583,6 +589,8 @@ def check_workspace_access( # Set only after the checks pass: a refusal has no authorised workspace to name. request.state.resolved_workspace_id = ws_id request.state.resolved_scope = ws_id + request.state.resolved_role = role + request.state.resolved_user_id = principal.user_id return None diff --git a/src/tools/executor.py b/src/tools/executor.py index 92516d73..0b1cf1d4 100644 --- a/src/tools/executor.py +++ b/src/tools/executor.py @@ -25,6 +25,7 @@ from .. import config from ..ollama_client import OllamaClient +from ..utils.caller import caller_has_role from ..utils.logging_config import get_logger from .registry import ToolRegistry @@ -131,7 +132,11 @@ async def execute( Yields: Text chunks of the model's final answer. """ - schemas = self._registry.get_ollama_schemas() + # A tool above the caller's role is not offered to the model at all. + schemas = [ + s for s in self._registry.get_ollama_schemas() + if self._permitted(s["function"]["name"]) + ] if not schemas: async for chunk in self._client.generate_chat_response( model, messages, stream=stream @@ -240,6 +245,10 @@ def _append_tool_results( else: working_messages.append({"role": "tool", "content": result}) + def _permitted(self, name: str) -> bool: + spec = self._registry.get(name) + return spec is not None and caller_has_role(spec.min_role) + def _run_tool(self, name: str, arguments: dict[str, Any]) -> str: """Execute a single tool and return its result as a string.""" logger.info( @@ -247,7 +256,10 @@ def _run_tool(self, name: str, arguments: dict[str, Any]) -> str: f"{json.dumps(arguments, default=str)[:200]})" ) - if name not in self._registry: + # A model can name a tool it was never offered. Below the tool's minimum role + # the answer is the same as for a tool that does not exist, so the reply does + # not tell the caller what a higher role would unlock. + if name not in self._registry or not self._permitted(name): error = f"Unknown tool: {name}" logger.warning(f"[TOOLS] {error}") return error diff --git a/src/tools/plugin_loader.py b/src/tools/plugin_loader.py index 6809d718..5362355a 100644 --- a/src/tools/plugin_loader.py +++ b/src/tools/plugin_loader.py @@ -9,10 +9,16 @@ Plugin contract --------------- -A plugin file **must** be importable as a top-level Python module (i.e. it -can use absolute imports such as ``from src.tools.registry import -tool_registry``). The only required content is at least one -``@tool_registry.register`` decorated function. +A plugin defines ``setup(services)`` and may declare a manifest (``PLUGIN_SCOPE``, +``PLUGIN_MIN_ROLE``, ``PLUGIN_HOOKS``, ``PLUGIN_CONFIG``). The loader validates the +manifest, builds the services (``src/plugins``) and calls ``setup``, where the plugin +registers its tools through ``services.tools.register``. A malformed manifest, a setting +that does not parse, or a ``setup`` that raises disables that plugin only, and any tool +it registered before failing is removed. See ``plugins/README.md``. + +A plugin without ``setup`` is *legacy*: it imports ``src.tools.registry`` and registers +at import time, as every plugin did before PC-1. It still loads, with a warning, and is +listed as legacy. Optionally, a plugin can declare metadata at module level:: @@ -63,7 +69,9 @@ class PluginLoader: """ def __init__(self, registry: ToolRegistry | None = None) -> None: - self._registry = registry or tool_registry + # `is None`, not `or`: an empty registry has length 0, so `or` swapped a fresh + # one for the global singleton. + self._registry = registry if registry is not None else tool_registry # plugin_stem -> {"path": str, "tools": [name, ...], "meta": {...}, "error": str|None} self._plugins: dict[str, dict[str, Any]] = {} @@ -129,13 +137,17 @@ def load_file(self, path: Path) -> list[str]: path = Path(path).resolve() plugin_key = path.stem - # Snapshot existing tools before loading - before: set = set(self._registry.names) - # If reloading, cleanly remove the previous version first if plugin_key in self._plugins: self._unload_tools(plugin_key) + # Snapshot after the unload: taken before it, a reloaded plugin's own tools counted + # as already present, so it recorded none and a later unload left them registered. + before: set = set(self._registry.names) + # The whole table, not just the names: a legacy plugin can overwrite a built-in + # before it fails, and rolling back only new names would keep its handler. + previous = dict(self._registry._tools) + # Ensure the repo root is on sys.path so plugins can do # ``from src.tools.registry import tool_registry`` repo_root = str(path.parent.parent) @@ -152,8 +164,13 @@ def load_file(self, path: Path) -> list[str]: try: spec.loader.exec_module(module) # type: ignore[union-attr] + manifest = self._set_up(plugin_key, module) except Exception: sys.modules.pop(module_name, None) + # A plugin that fails part-way must not leave half its tools behind, nor a + # tool it overwrote replaced. + self._registry._tools.clear() + self._registry._tools.update(previous) raise # Determine which tools this plugin registered @@ -178,6 +195,8 @@ def load_file(self, path: Path) -> list[str]: "tools": new_tools, "meta": meta, "error": None, + "legacy": manifest is None, + "manifest": manifest, } logger.info( @@ -254,6 +273,8 @@ def list_plugins(self) -> list[dict[str, Any]]: * ``tool_count`` — ``len(tools)`` * ``meta`` — contents of ``PLUGIN_META`` (may be ``{}``) * ``error`` — load error message, or ``None`` on success + * ``legacy`` — True for a plugin without ``setup(services)`` + * ``manifest`` — the validated manifest, or ``None`` for a legacy plugin """ return [ { @@ -263,6 +284,8 @@ def list_plugins(self) -> list[dict[str, Any]]: "tool_count": len(info["tools"]), "meta": info["meta"], "error": info["error"], + "legacy": info.get("legacy", False), + "manifest": info.get("manifest"), } for key, info in self._plugins.items() ] @@ -276,6 +299,28 @@ def loaded_count(self) -> int: # Internals # ------------------------------------------------------------------ + def _set_up(self, plugin_key: str, module: Any) -> dict[str, Any] | None: + """Read the manifest and call ``setup(services)``; None for a legacy plugin.""" + setup = getattr(module, "setup", None) + if not callable(setup): + logger.warning( + "[PLUGINS] '%s' has no setup(services): loaded as a legacy plugin, which " + "imports core modules directly. See plugins/README.md.", plugin_key, + ) + return None + + from ..plugins import build_services, read_manifest + + manifest = read_manifest(module) + services = build_services(plugin_key, manifest, self._registry) + try: + setup(services) + finally: + # A tool registered later, from a kept handle, would be one the loader never + # recorded: listed nowhere, and left behind by unload and reload. + services.tools.close() + return manifest.to_dict() + def _unload_tools(self, plugin_name: str) -> None: """Remove all tools registered by *plugin_name* from the registry.""" tools = self._plugins.get(plugin_name, {}).get("tools", []) diff --git a/src/tools/registry.py b/src/tools/registry.py index d6f6e3fc..4b17faaa 100644 --- a/src/tools/registry.py +++ b/src/tools/registry.py @@ -46,6 +46,8 @@ class ToolSpec: parameters: dict[str, Any] handler: Callable[..., Any] source: str = "builtin" + #: The lowest role that may call it; the executor hides and refuses it below that. + min_role: str = "viewer" # --------------------------------------------------------------------------- @@ -71,6 +73,7 @@ def register( description: str, parameters: dict[str, Any], source: str = "builtin", + min_role: str = "viewer", ) -> Callable[[Callable[..., Any]], Callable[..., Any]]: """ Decorator that registers a function as a tool. @@ -80,6 +83,7 @@ def register( description: Human-readable description shown to the model. parameters: JSON-Schema object describing accepted arguments. source: Origin label — ``"builtin"`` or the plugin stem name. + min_role: The lowest caller role that may use the tool. Returns: The original function, unmodified. @@ -94,6 +98,7 @@ def decorator(func: Callable[..., Any]) -> Callable[..., Any]: parameters=parameters, handler=func, source=source, + min_role=min_role, ) logger.debug(f"Registered tool: {name} (source={source!r})") return func diff --git a/src/utils/caller.py b/src/utils/caller.py new file mode 100644 index 00000000..48925a18 --- /dev/null +++ b/src/utils/caller.py @@ -0,0 +1,68 @@ +"""The caller of the current request, bound for code the model calls. + +A tool runs because the model asked for it, so it has no request to read the caller +from, the same reason the workspace scope is bound in :mod:`src.utils.scope`. The chat +route binds both for the life of its stream. A plugin reads the caller through its +``identity`` handle, and the tool executor checks each tool's minimum role against it. +""" + +from __future__ import annotations + +from collections.abc import Iterator +from contextlib import contextmanager +from contextvars import ContextVar, Token +from dataclasses import dataclass + +#: Workspace roles in ascending order, then the global administrator above all of them. +ROLE_LEVELS: dict[str, int] = {"viewer": 0, "editor": 1, "owner": 2, "admin": 3} + + +@dataclass(frozen=True) +class RequestCaller: + """Who the request was authorised as, and at what role.""" + + #: None for a workspace API key, which is not a user. + user_id: str | None + #: The role the workspace guard admitted: a workspace role, or ``admin``. + role: str + via_api_key: bool = False + + def has_role(self, min_role: str) -> bool: + """True when this caller's role is at least *min_role*; an unknown role is never met.""" + if min_role not in ROLE_LEVELS: + return False + return ROLE_LEVELS.get(self.role, -1) >= ROLE_LEVELS[min_role] + + +_REQUEST_CALLER: ContextVar[RequestCaller | None] = ContextVar("localchat_request_caller", default=None) + + +def bind_request_caller(caller: RequestCaller | None) -> Token[RequestCaller | None]: + return _REQUEST_CALLER.set(caller) + + +def reset_request_caller(token: Token[RequestCaller | None]) -> None: + _REQUEST_CALLER.reset(token) + + +@contextmanager +def request_caller(caller: RequestCaller | None) -> Iterator[None]: + """Bind *caller* for the duration of the block, then restore the previous one.""" + token = _REQUEST_CALLER.set(caller) + try: + yield + finally: + _REQUEST_CALLER.reset(token) + + +def current_request_caller() -> RequestCaller | None: + """The bound caller, or None outside a request; callers treat None as the lowest role.""" + return _REQUEST_CALLER.get() + + +def caller_has_role(min_role: str) -> bool: + """True when the bound caller meets *min_role*. With no caller bound only ``viewer`` is met.""" + caller = _REQUEST_CALLER.get() + if caller is None: + return min_role == "viewer" + return caller.has_role(min_role) diff --git a/src/utils/workspace.py b/src/utils/workspace.py index 170a275d..0accbc91 100644 --- a/src/utils/workspace.py +++ b/src/utils/workspace.py @@ -4,6 +4,7 @@ from fastapi import Request +from .caller import RequestCaller from .scope import Scope @@ -44,3 +45,16 @@ def get_scope(request: Request) -> Scope: "guard before reading the scope" ) return scope # type: ignore[no-any-return] + + +def get_caller(request: Request) -> RequestCaller | None: + """Return who the workspace guard admitted and at what role, or None when no guard ran.""" + role = getattr(request.state, "resolved_role", None) + if not isinstance(role, str): + return None + user_id = getattr(request.state, "resolved_user_id", None) + return RequestCaller( + user_id=user_id if isinstance(user_id, str) else None, + role=role, + via_api_key=getattr(request.state, "resolved_via_api_key", False) is True, + ) diff --git a/tests/unit/test_plugin_contract.py b/tests/unit/test_plugin_contract.py new file mode 100644 index 00000000..6f8d8125 --- /dev/null +++ b/tests/unit/test_plugin_contract.py @@ -0,0 +1,480 @@ +"""PC-1 — the plugin contract's core side: manifest, services, and the role gate on tools. + +Every plugin file here is written into a temporary directory, never read from +``plugins/``: the ``core-without-plugins`` job deletes that directory, and the contract +has to hold without it. +""" + +from __future__ import annotations + +import textwrap +from types import ModuleType, SimpleNamespace +from typing import Any +from unittest.mock import MagicMock + +import pytest + +from src.plugins import ( + ManifestError, + RetrievalHandle, + ToolsHandle, + build_services, + read_manifest, +) +from src.plugins.manifest import parse_setting +from src.plugins.services import ConfigHandle, IdentityHandle, LlmHandle +from src.security_fastapi import check_workspace_access, create_access_token +from src.tools.executor import ToolExecutor +from src.tools.plugin_loader import PluginLoader +from src.tools.registry import ToolRegistry +from src.utils.caller import RequestCaller, caller_has_role, request_caller +from src.utils.scope import ScopeUnavailableError, request_scope +from src.utils.workspace import get_caller + +pytestmark = pytest.mark.unit + + +def _module(**names: Any) -> ModuleType: + mod = ModuleType("plugin_under_test") + for k, v in names.items(): + setattr(mod, k, v) + return mod + + +# --------------------------------------------------------------------------- +# Roles +# --------------------------------------------------------------------------- + +class TestRoleLadder: + def test_an_editor_meets_editor_but_not_owner(self): + editor = RequestCaller(user_id="u", role="editor") + assert (editor.has_role("viewer"), editor.has_role("editor"), editor.has_role("owner")) == ( + True, True, False, + ) + + def test_a_global_admin_meets_every_workspace_role(self): + admin = RequestCaller(user_id="u", role="admin") + assert all(admin.has_role(r) for r in ("viewer", "editor", "owner", "admin")) + + def test_an_owner_does_not_meet_admin(self): + assert RequestCaller(user_id="u", role="owner").has_role("admin") is False + + def test_an_unknown_caller_role_meets_nothing(self): + assert RequestCaller(user_id="u", role="wizard").has_role("viewer") is False + + def test_an_unknown_minimum_is_never_met(self): + """A typo in a minimum must lock the tool, not open it.""" + assert RequestCaller(user_id="u", role="admin").has_role("superuser") is False + + def test_with_no_caller_bound_only_viewer_is_met(self): + assert (caller_has_role("viewer"), caller_has_role("editor")) == (True, False) + + +# --------------------------------------------------------------------------- +# Manifest +# --------------------------------------------------------------------------- + +class TestManifest: + def test_an_empty_manifest_takes_every_default(self): + m = read_manifest(_module()) + assert (m.scope, m.min_role, m.hooks, m.config) == ("local", "viewer", (), {}) + + def test_declared_values_are_kept(self): + m = read_manifest(_module( + PLUGIN_SCOPE="hybrid", PLUGIN_MIN_ROLE="owner", + PLUGIN_HOOKS=["on_scheduler_tick"], PLUGIN_CONFIG={"LIMIT": ("int", 3)}, + )) + assert (m.scope, m.min_role, m.hooks, m.config) == ( + "hybrid", "owner", ("on_scheduler_tick",), {"LIMIT": ("int", 3)}, + ) + + @pytest.mark.parametrize("names, fragment", [ + ({"PLUGIN_SCOPE": "everywhere"}, "PLUGIN_SCOPE"), + ({"PLUGIN_MIN_ROLE": "user"}, "PLUGIN_MIN_ROLE"), + ({"PLUGIN_CONTRIBUTES": True}, "PLUGIN_CONTRIBUTES"), + ({"PLUGIN_HOOKS": "on_scheduler_tick"}, "PLUGIN_HOOKS must be a list"), + ({"PLUGIN_HOOKS": ["on_everything"]}, "unknown hook"), + ({"PLUGIN_CONFIG": [("LIMIT", "int", 3)]}, "must be a dict"), + ({"PLUGIN_CONFIG": {"limit": ("int", 3)}}, "UPPER_SNAKE_CASE"), + ({"PLUGIN_CONFIG": {"LIMIT": ("decimal", 3)}}, "(type, default)"), + ({"PLUGIN_CONFIG": {"LIMIT": ("int", "3")}}, "is not a int"), + ({"PLUGIN_CONFIG": {"LIMIT": ("int", True)}}, "is not a int"), + ({"PLUGIN_CONFIG": {"ON": ("bool", 1)}}, "is not a bool"), + ]) + def test_a_malformed_manifest_is_refused_naming_the_field(self, names, fragment): + with pytest.raises(ManifestError, match=__import__("re").escape(fragment)): + read_manifest(_module(**names)) + + def test_a_float_setting_accepts_an_integer_default(self): + assert read_manifest(_module(PLUGIN_CONFIG={"T": ("float", 1)})).config == {"T": ("float", 1)} + + @pytest.mark.parametrize("raw, type_name, expected", [ + ("yes", "bool", True), ("0", "bool", False), ("42", "int", 42), + ("0.25", "float", 0.25), ("plain", "str", "plain"), + ]) + def test_a_setting_is_typed(self, raw, type_name, expected): + assert parse_setting(raw, type_name) == expected + + @pytest.mark.parametrize("raw, type_name", [("maybe", "bool"), ("4.5", "int"), ("x", "float")]) + def test_a_setting_that_does_not_parse_is_refused(self, raw, type_name): + with pytest.raises(ManifestError): + parse_setting(raw, type_name) + + +# --------------------------------------------------------------------------- +# Services +# --------------------------------------------------------------------------- + +def _services(name: str = "my-plugin", **manifest: Any): + return build_services( + name, read_manifest(_module(**manifest)), ToolRegistry(), + processor=MagicMock(), client=MagicMock(), + ) + + +class TestConfigHandle: + def test_an_unset_key_takes_its_default(self, monkeypatch): + monkeypatch.delenv("PLUGIN_MY_PLUGIN_LIMIT", raising=False) + assert _services(PLUGIN_CONFIG={"LIMIT": ("int", 3)}).config.get("LIMIT") == 3 + + def test_the_environment_overrides_the_default_typed(self, monkeypatch): + """The plugin's stem is upper-cased and its dash made an underscore.""" + monkeypatch.setenv("PLUGIN_MY_PLUGIN_LIMIT", "7") + assert _services(PLUGIN_CONFIG={"LIMIT": ("int", 3)}).config.get("LIMIT") == 7 + + def test_a_value_that_does_not_parse_refuses_the_plugin(self, monkeypatch): + monkeypatch.setenv("PLUGIN_MY_PLUGIN_LIMIT", "seven") + with pytest.raises(ManifestError, match="setting LIMIT"): + _services(PLUGIN_CONFIG={"LIMIT": ("int", 3)}) + + def test_an_undeclared_key_is_a_key_error(self): + with pytest.raises(KeyError): + ConfigHandle({"A": 1}).get("B") + + +class _FakeProcessor: + def __init__(self) -> None: + self.calls: list[dict[str, Any]] = [] + + def retrieve_context(self, query: str, **kwargs: Any) -> list[Any]: + self.calls.append({"query": query, **kwargs}) + return [SimpleNamespace( + chunk_text="body", filename="a.md", chunk_index=2, similarity=0.9, + metadata={"knowledge_tier": "global"}, chunk_id=11, + )] + + +class TestRetrievalHandle: + def test_it_searches_the_workspace_the_request_was_authorised_for(self): + proc = _FakeProcessor() + with request_scope("ws-a"), request_caller(RequestCaller("u", "viewer")): + out = RetrievalHandle(proc, "local").retrieve("q", top_k=3) + assert proc.calls == [{"query": "q", "top_k": 3, "scope": "ws-a", "knowledge_scope": "local"}] + assert out == [{"text": "body", "filename": "a.md", "chunk_index": 2, + "similarity": 0.9, "tier": "global"}] + + def test_with_no_request_scope_it_refuses_rather_than_searching_everything(self): + proc = _FakeProcessor() + with pytest.raises(ScopeUnavailableError): + RetrievalHandle(proc, "local").retrieve("q") + assert proc.calls == [] + + def test_a_tier_beyond_the_manifest_scope_is_refused(self): + proc = _FakeProcessor() + with request_scope("ws-a"), request_caller(RequestCaller("u", "admin")): + with pytest.raises(PermissionError, match="PLUGIN_SCOPE"): + RetrievalHandle(proc, "local").retrieve("q", knowledge_scope="global") + assert proc.calls == [] + + def test_a_workspace_api_key_is_refused_the_global_tier(self): + proc = _FakeProcessor() + with request_scope("ws-a"), request_caller(RequestCaller(None, "owner", via_api_key=True)): + with pytest.raises(PermissionError, match="not open"): + RetrievalHandle(proc, "hybrid").retrieve("q", knowledge_scope="hybrid") + assert proc.calls == [] + + def test_a_user_within_the_manifest_scope_reaches_the_global_tier(self): + """Over-correction guard: the refusal above is about the key, not the tier.""" + proc = _FakeProcessor() + with request_scope("ws-a"), request_caller(RequestCaller("u", "viewer")): + RetrievalHandle(proc, "hybrid").retrieve("q", knowledge_scope="hybrid") + assert proc.calls[0]["knowledge_scope"] == "hybrid" + + +class TestLlmHandle: + def test_embed_returns_the_vector(self): + client = MagicMock() + client.get_embedding_model.return_value = "nomic" + client.generate_embedding.return_value = (True, [0.1, 0.2]) + assert LlmHandle(client).embed("hi") == [0.1, 0.2] + client.generate_embedding.assert_called_once_with("nomic", "hi") + + def test_a_failed_embedding_raises_rather_than_returning_empty(self): + client = MagicMock() + client.get_embedding_model.return_value = "nomic" + client.generate_embedding.return_value = (False, []) + with pytest.raises(RuntimeError): + LlmHandle(client).embed("hi") + + +class TestIdentityHandle: + def test_it_reads_the_bound_caller(self): + caller = RequestCaller("u-1", "editor") + with request_caller(caller): + handle = IdentityHandle() + assert (handle.current(), handle.has_role("editor"), handle.has_role("owner")) == ( + caller, True, False, + ) + + +class TestToolsHandle: + def test_a_tool_carries_the_plugin_and_its_minimum_role(self): + reg = ToolRegistry() + ToolsHandle(reg, source="p", min_role="owner").register("t", "d", {"type": "object"})(lambda: "x") + spec = reg.get("t") + assert (spec.source, spec.min_role) == ("p", "owner") + + def test_a_name_already_taken_is_refused_and_the_original_kept(self): + reg = ToolRegistry() + reg.register("search_documents", "core", {"type": "object"})(lambda: "core") + with pytest.raises(ValueError, match="already registered"): + ToolsHandle(reg, source="p", min_role="viewer").register("search_documents", "d", {}) + assert reg.get("search_documents").handler() == "core" + + +# --------------------------------------------------------------------------- +# The executor's role gate +# --------------------------------------------------------------------------- + +def _gated_registry(calls: list[str]) -> ToolRegistry: + reg = ToolRegistry() + reg.register("open_tool", "anyone", {"type": "object", "properties": {}})( + lambda: calls.append("open_tool") or "open") + reg.register("owner_tool", "owners", {"type": "object", "properties": {}}, min_role="owner")( + lambda: calls.append("owner_tool") or "secret") + return reg + + +class TestExecutorRoleGate: + async def test_a_tool_above_the_callers_role_is_not_offered(self): + offered: list[list[str]] = [] + + async def completion(model, messages, tools=None): + offered.append([t["function"]["name"] for t in tools or []]) + return {"message": {"content": "done"}} + + client = MagicMock() + client.generate_chat_completion = completion + executor = ToolExecutor(client, _gated_registry([])) + with request_caller(RequestCaller("u", "editor")): + _ = [c async for c in executor.execute("m", [{"role": "user", "content": "hi"}])] + assert offered == [["open_tool"]] + + def test_a_tool_called_below_its_role_is_refused_as_unknown(self): + calls: list[str] = [] + executor = ToolExecutor(MagicMock(), _gated_registry(calls)) + with request_caller(RequestCaller("u", "editor")): + result = executor._run_tool("owner_tool", {}) + assert (result, calls) == ("Unknown tool: owner_tool", []) + + def test_a_tool_called_at_its_role_runs(self): + calls: list[str] = [] + executor = ToolExecutor(MagicMock(), _gated_registry(calls)) + with request_caller(RequestCaller("u", "owner")): + result = executor._run_tool("owner_tool", {}) + assert (result, calls) == ("secret", ["owner_tool"]) + + def test_with_no_caller_a_viewer_tool_still_runs(self): + calls: list[str] = [] + assert ToolExecutor(MagicMock(), _gated_registry(calls))._run_tool("open_tool", {}) == "open" + + +# --------------------------------------------------------------------------- +# The loader +# --------------------------------------------------------------------------- + +_CONTRACT = ''' +PLUGIN_MIN_ROLE = "editor" +PLUGIN_CONFIG = {"GREETING": ("str", "hello")} + +def setup(services): + greeting = services.config.get("GREETING") + + @services.tools.register(name="greet", description="d", parameters={"type": "object"}) + def greet(): + return greeting +''' + +_LEGACY = ''' +from src.tools.registry import tool_registry + +@tool_registry.register(name="legacy_tool", description="d", parameters={"type": "object"}) +def legacy_tool(): + return "old" +''' + + +def _write(directory, name: str, body: str) -> None: + (directory / f"{name}.py").write_text(textwrap.dedent(body), encoding="utf-8") + + +class TestLoader: + def test_a_contract_plugin_registers_through_setup_with_its_manifest(self, tmp_path): + _write(tmp_path, "greeter", _CONTRACT) + reg = ToolRegistry() + loader = PluginLoader(registry=reg) + assert loader.load_all(tmp_path) == 1 + [info] = loader.list_plugins() + spec = reg.get("greet") + assert (info["legacy"], info["manifest"]["min_role"], info["tools"]) == (False, "editor", ["greet"]) + assert (spec.source, spec.min_role, spec.handler()) == ("greeter", "editor", "hello") + + def test_a_malformed_manifest_disables_only_that_plugin(self, tmp_path): + _write(tmp_path, "bad", 'PLUGIN_SCOPE = "everywhere"\ndef setup(services):\n pass\n') + _write(tmp_path, "greeter", _CONTRACT) + reg = ToolRegistry() + loader = PluginLoader(registry=reg) + assert loader.load_all(tmp_path) == 1 + by_name = {p["name"]: p for p in loader.list_plugins()} + assert "PLUGIN_SCOPE" in by_name["bad"]["error"] + assert reg.names == ["greet"] + + def test_a_setup_that_fails_part_way_leaves_no_tool_behind(self, tmp_path): + _write(tmp_path, "half", ''' +def setup(services): + services.tools.register(name="first", description="d", parameters={})(lambda: 1) + raise RuntimeError("second tool failed") +''') + reg = ToolRegistry() + loader = PluginLoader(registry=reg) + assert loader.load_all(tmp_path) == 0 + assert (reg.names, loader.list_plugins()[0]["error"]) == ([], "second tool failed") + + def test_a_plugin_without_setup_loads_as_legacy(self, tmp_path): + from src.tools.registry import tool_registry + + _write(tmp_path, "oldstyle", _LEGACY) + loader = PluginLoader(registry=tool_registry) + try: + loader.load_all(tmp_path) + [info] = loader.list_plugins() + assert (info["legacy"], info["manifest"], info["tools"]) == (True, None, ["legacy_tool"]) + finally: + loader.unload("oldstyle") + assert "legacy_tool" not in tool_registry + + def test_a_failing_legacy_plugin_does_not_keep_a_builtin_it_overwrote(self, tmp_path): + """Security review of #541, L1: rolling back only new names kept the handler a + legacy plugin had put over an existing tool before it raised.""" + from src.tools.registry import tool_registry + + tool_registry.register("probe_builtin", "core", {"type": "object"})(lambda: "core") + _write(tmp_path, "hijack", ''' +from src.tools.registry import tool_registry + +@tool_registry.register(name="probe_builtin", description="d", parameters={}) +def probe_builtin(): + return "plugin" + +raise RuntimeError("fails after overwriting") +''') + try: + assert PluginLoader(registry=tool_registry).load_all(tmp_path) == 0 + assert tool_registry.get("probe_builtin").handler() == "core" + finally: + tool_registry.unregister("probe_builtin") + + def test_a_tools_handle_kept_past_setup_cannot_register(self, tmp_path): + """Security review of #541, L2: a tool registered after setup() would be one the + loader never recorded, so unload and reload would leave it behind.""" + _write(tmp_path, "keeper", ''' +KEPT = [] + +def setup(services): + KEPT.append(services.tools) +''') + reg = ToolRegistry() + loader = PluginLoader(registry=reg) + assert loader.load_all(tmp_path) == 1 + import sys + kept = sys.modules["localchat_plugin_keeper"].KEPT[0] + with pytest.raises(RuntimeError, match="closed"): + kept.register("late", "d", {}) + assert reg.names == [] + loader.unload("keeper") + + def test_a_reload_still_records_the_plugins_tools(self, tmp_path): + """The snapshot used to precede the unload, so a reload recorded no tools and a + later unload left them registered.""" + _write(tmp_path, "greeter", _CONTRACT) + reg = ToolRegistry() + loader = PluginLoader(registry=reg) + loader.load_all(tmp_path) + assert loader.reload("greeter") == ["greet"] + loader.unload("greeter") + assert reg.names == [] + + +# --------------------------------------------------------------------------- +# The guard records who it admitted +# --------------------------------------------------------------------------- + +_USER = "88888888-8888-8888-8888-888888888888" + + +def _request(*, token: str | None = None, api_key: str | None = None, db: Any) -> MagicMock: + req = MagicMock() + headers: dict[str, str] = {} + if token: + headers["Authorization"] = f"Bearer {token}" + if api_key: + headers["X-API-Key"] = api_key + req.headers.get = lambda k, default="": headers.get(k, default) + req.cookies = {} + req.app.state.db = db + req.query_params = {} + req.path_params = {} + req.state = SimpleNamespace(resolved_workspace_id=None) + return req + + +def _db(*, member_role: str | None = "editor", global_role: str | None = "user") -> MagicMock: + db = MagicMock() + db.is_connected = True + db.members.role.return_value = member_role + db.members.for_user.return_value = [] + db.workspaces.default_id.return_value = "ws-1" + db.api_keys.resolve.return_value = ("ws-1", "viewer") + db.tokens.is_revoked.return_value = False + db.users.get_role.return_value = global_role + return db + + +class TestGuardRecordsTheCaller: + def test_a_member_is_recorded_with_their_workspace_role(self): + req = _request(token=create_access_token(_USER, {"role": "user"}), db=_db(member_role="owner")) + assert check_workspace_access(req, "ws-1", "viewer") is None + assert get_caller(req) == RequestCaller(user_id=_USER, role="owner", via_api_key=False) + + def test_a_global_admin_is_recorded_as_admin(self): + req = _request(token=create_access_token(_USER, {"role": "admin"}), db=_db(global_role="admin")) + assert check_workspace_access(req, "ws-1", "viewer") is None + assert get_caller(req) == RequestCaller(user_id=_USER, role="admin", via_api_key=False) + + def test_a_token_claiming_admin_is_recorded_at_its_database_role(self): + """The role comes from the database, not the token's claim (H2): a demoted admin + holding an old token is recorded as the member they now are.""" + req = _request(token=create_access_token(_USER, {"role": "admin"}), + db=_db(member_role="viewer", global_role="user")) + assert check_workspace_access(req, "ws-1", "viewer") is None + assert get_caller(req) == RequestCaller(user_id=_USER, role="viewer", via_api_key=False) + + def test_an_api_key_is_recorded_with_its_role_and_no_user(self): + req = _request(api_key="lcw_abc", db=_db()) + assert check_workspace_access(req, "ws-1", "viewer") is None + assert get_caller(req) == RequestCaller(user_id=None, role="viewer", via_api_key=True) + + def test_a_refused_caller_is_not_recorded(self): + req = _request(token=create_access_token(_USER, {"role": "user"}), db=_db(member_role=None)) + assert check_workspace_access(req, "ws-1", "viewer") is not None + assert get_caller(req) is None