From 9406f8577d0053f0f1891175a1509a19bcab7204 Mon Sep 17 00:00:00 2001 From: Hey-Yetunde <235164239+Hey-Yetunde@users.noreply.github.com> Date: Sat, 17 Oct 2026 14:39:03 +0100 Subject: [PATCH 1/3] test: fix syntax error blocking notifications collection The Phase 10 budget-alert test merged in #500 has an invalid parameter list (self/ app), which prevents pytest from collecting tests/test_notifications.py at all. Restore the comma so the suite can run. --- tests/test_notifications.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_notifications.py b/tests/test_notifications.py index df144a8..66929eb 100644 --- a/tests/test_notifications.py +++ b/tests/test_notifications.py @@ -183,7 +183,7 @@ def test_repeated_crossings_alert_once_per_period(self, app, make_user): alerts = check_budget_alerts(workspace.id, spend=150, limit=100, period="2024-01") assert len(alerts) == 0 - def test_new_period_resets_deduplication(self/ app, make_user): + def test_new_period_resets_deduplication(self, app, make_user): from app.services.budget_alerts import check_budget_alerts user = make_user() From cbb5ce66de5763d8a3fc8592856cf314fc4a6fed Mon Sep 17 00:00:00 2001 From: Hey-Yetunde <235164239+Hey-Yetunde@users.noreply.github.com> Date: Sat, 17 Oct 2026 14:39:03 +0100 Subject: [PATCH 2/3] feat(pricing): add documented default price table (#439) Ship per-1k input/output estimates for every model exposed by the built-in providers, with the mock provider pinned at zero cost. Entries are explicitly marked as estimates via ModelPrice.estimated and ESTIMATE_DISCLAIMER; unknown provider/model lookups return None rather than assuming zero. Lookups are case/whitespace insensitive, token counts are validated, and price_table() exposes the grouped table for operator-facing surfaces. --- app/services/pricing.py | 159 ++++++++++++++++++++++++++++++++++++++++ docs/providers.md | 37 ++++++++++ tests/test_pricing.py | 94 ++++++++++++++++++++++++ 3 files changed, 290 insertions(+) create mode 100644 app/services/pricing.py create mode 100644 tests/test_pricing.py diff --git a/app/services/pricing.py b/app/services/pricing.py new file mode 100644 index 0000000..1233966 --- /dev/null +++ b/app/services/pricing.py @@ -0,0 +1,159 @@ +"""Default LLM price table for common providers and models (issue #439). + +The prices here are **estimates**, expressed in USD per 1,000 tokens, for +turning token counts into rough cost figures (pre-flight estimates, dashboards, +budgets). They are not a billing source of truth: vendors change prices and +introduce dated model snapshots, so operators must verify the numbers against +their provider contract before relying on them. + +The offline ``mock`` provider is the one entry that is not an estimate: it +never calls a vendor and therefore always costs zero. + +Lookups are provider/model exact matches after normalizing case and +surrounding whitespace. An unknown provider or model returns ``None`` rather +than guessing, so callers can distinguish "free" from "we do not know". +""" + +from __future__ import annotations + +from dataclasses import asdict, dataclass +from typing import Any + +#: Shown next to the table in docs/UI wherever costs are displayed. +ESTIMATE_DISCLAIMER = ( + "Default prices are estimates in USD per 1,000 tokens. " + "Verify them against your provider's current pricing before relying on them." +) + + +@dataclass(frozen=True) +class ModelPrice: + """Estimated input/output price for one provider model. + + Attributes: + provider: Registered provider name (e.g. ``openai``). + model: Model identifier as accepted by the provider. + input_per_1k_usd: Estimated USD cost per 1,000 prompt tokens. + output_per_1k_usd: Estimated USD cost per 1,000 completion tokens. + estimated: ``False`` only for models with a known, guaranteed price + (the offline mock provider). Everything else is a default estimate. + """ + + provider: str + model: str + input_per_1k_usd: float + output_per_1k_usd: float + estimated: bool = True + + def to_dict(self) -> dict[str, Any]: + """Return a JSON-friendly representation, including the disclaimer.""" + data = asdict(self) + data["disclaimer"] = ESTIMATE_DISCLAIMER + return data + + +def _price( + provider: str, + model: str, + input_per_1k_usd: float, + output_per_1k_usd: float, + *, + estimated: bool = True, +) -> ModelPrice: + return ModelPrice( + provider=provider, + model=model, + input_per_1k_usd=input_per_1k_usd, + output_per_1k_usd=output_per_1k_usd, + estimated=estimated, + ) + + +#: Default, operator-verifiable estimates for the models registered by the +#: built-in providers. Keys are ``(provider, model)`` normalized to lowercase. +DEFAULT_PRICE_TABLE: dict[tuple[str, str], ModelPrice] = { + # The offline provider is deterministic and free (not an estimate). + ("mock", "mock-1"): _price("mock", "mock-1", 0.0, 0.0, estimated=False), + # OpenAI defaults (see app/services/providers/openai.py). + ("openai", "gpt-4o-mini"): _price("openai", "gpt-4o-mini", 0.00015, 0.0006), + ("openai", "gpt-4o"): _price("openai", "gpt-4o", 0.0025, 0.01), + ("openai", "gpt-4.1-mini"): _price("openai", "gpt-4.1-mini", 0.0004, 0.0016), + ("openai", "gpt-4.1"): _price("openai", "gpt-4.1", 0.002, 0.008), + # Anthropic defaults (see app/services/providers/anthropic.py). + ("anthropic", "claude-3-5-sonnet-latest"): _price( + "anthropic", "claude-3-5-sonnet-latest", 0.003, 0.015 + ), + ("anthropic", "claude-3-5-haiku-latest"): _price( + "anthropic", "claude-3-5-haiku-latest", 0.0008, 0.004 + ), +} + + +def _normalize(value: str | None) -> str: + return (value or "").strip().lower() + + +def get_model_price(provider: str | None, model: str | None) -> ModelPrice | None: + """Return the default price entry for ``provider``/``model``, or ``None``. + + Matching is case-insensitive and ignores surrounding whitespace. Unknown + combinations return ``None`` (cost unknown) rather than assuming zero. + """ + return DEFAULT_PRICE_TABLE.get((_normalize(provider), _normalize(model))) + + +def list_model_prices(provider: str | None = None) -> list[ModelPrice]: + """Return the default price entries, optionally filtered by ``provider``. + + Results are sorted by provider then model so the table (and any API built + on it) is stable across runs. + """ + wanted = _normalize(provider) + entries = [ + price + for (entry_provider, _), price in DEFAULT_PRICE_TABLE.items() + if not wanted or entry_provider == wanted + ] + return sorted(entries, key=lambda price: (price.provider, price.model)) + + +def price_table() -> dict[str, list[dict[str, Any]]]: + """Return the whole default table grouped by provider (JSON-friendly).""" + grouped: dict[str, list[dict[str, Any]]] = {} + for price in list_model_prices(): + grouped.setdefault(price.provider, []).append(price.to_dict()) + return grouped + + +def estimate_cost_usd( + provider: str | None, + model: str | None, + *, + prompt_tokens: int = 0, + completion_tokens: int = 0, +) -> float | None: + """Estimate USD cost for a completion, or ``None`` when the price is unknown. + + Token counts must be non-negative integers; anything else raises + :class:`ValueError` so callers cannot silently report a negative cost. + """ + prompt = int(prompt_tokens) + completion = int(completion_tokens) + if prompt < 0 or completion < 0: + raise ValueError("Token counts must be non-negative.") + + price = get_model_price(provider, model) + if price is None: + return None + return prompt / 1000 * price.input_per_1k_usd + completion / 1000 * price.output_per_1k_usd + + +__all__ = [ + "DEFAULT_PRICE_TABLE", + "ESTIMATE_DISCLAIMER", + "ModelPrice", + "estimate_cost_usd", + "get_model_price", + "list_model_prices", + "price_table", +] diff --git a/docs/providers.md b/docs/providers.md index 17f0f52..97eadaa 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -60,6 +60,43 @@ environment does not provide one. A conversation's `model`, `temperature`, and historical name) is an alias of `ProviderError`, so existing `except LLMProviderError` blocks keep working. +## Default price table (issue #439) + +`app/services/pricing.py` ships default cost estimates for the models exposed +by the built-in providers, in USD per 1,000 tokens: + +| Provider | Model | Input / 1k | Output / 1k | Source | +| --- | --- | --- | --- | --- | +| `mock` | `mock-1` | `0.00000` | `0.00000` | Offline provider, always free | +| `openai` | `gpt-4o-mini` | `0.00015` | `0.00060` | Estimate | +| `openai` | `gpt-4o` | `0.00250` | `0.01000` | Estimate | +| `openai` | `gpt-4.1-mini` | `0.00040` | `0.00160` | Estimate | +| `openai` | `gpt-4.1` | `0.00200` | `0.00800` | Estimate | +| `anthropic` | `claude-3-5-sonnet-latest` | `0.00300` | `0.01500` | Estimate | +| `anthropic` | `claude-3-5-haiku-latest` | `0.00080` | `0.00400` | Estimate | + +> **These are estimates, not billing data.** Vendors change prices and add +> dated model snapshots, so operators must verify the numbers against their +> current provider contract before using them for budgets or invoices. The +> table exposes `ESTIMATE_DISCLAIMER` (`estimated: true` on every vendor +> entry) for exactly that reason; only the offline `mock` provider is a +> guaranteed zero-cost entry. + +```python +from app.services.pricing import estimate_cost_usd, get_model_price, price_table + +price = get_model_price("openai", "gpt-4o") +cost = estimate_cost_usd( + "openai", "gpt-4o", prompt_tokens=2_000, completion_tokens=500 +) +# 2 * price.input_per_1k_usd + 0.5 * price.output_per_1k_usd +``` + +Lookups ignore case and surrounding whitespace. Unknown providers or models +return `None` (cost unknown) instead of assuming zero, and negative token +counts raise `ValueError`. `price_table()` returns the full table grouped by +provider for operator-facing surfaces. + ## Adding a provider 1. Create `app/services/providers/.py` with a class that subclasses diff --git a/tests/test_pricing.py b/tests/test_pricing.py new file mode 100644 index 0000000..280fc40 --- /dev/null +++ b/tests/test_pricing.py @@ -0,0 +1,94 @@ +"""Tests for the documented default price table (issue #439).""" + +import pytest + +from app.services.pricing import ( + DEFAULT_PRICE_TABLE, + ESTIMATE_DISCLAIMER, + estimate_cost_usd, + get_model_price, + list_model_prices, + price_table, +) +from app.services.providers.anthropic import DEFAULT_MODELS as ANTHROPIC_MODELS +from app.services.providers.mock import MockProvider +from app.services.providers.openai import DEFAULT_MODELS as OPENAI_MODELS + + +class TestDefaultTable: + def test_mock_provider_is_free_and_not_an_estimate(self): + price = get_model_price("mock", "mock-1") + assert price is not None + assert price.input_per_1k_usd == 0 + assert price.output_per_1k_usd == 0 + assert price.estimated is False + assert estimate_cost_usd("mock", "mock-1", prompt_tokens=10_000) == 0 + + @pytest.mark.parametrize("model", OPENAI_MODELS) + def test_every_builtin_openai_model_has_a_default(self, model): + price = get_model_price("openai", model) + assert price is not None + assert price.estimated is True + assert price.input_per_1k_usd >= 0 + assert price.output_per_1k_usd >= 0 + + @pytest.mark.parametrize("model", ANTHROPIC_MODELS) + def test_every_builtin_anthropic_model_has_a_default(self, model): + price = get_model_price("anthropic", model) + assert price is not None + assert price.estimated is True + assert price.input_per_1k_usd >= 0 + assert price.output_per_1k_usd >= 0 + + def test_registered_mock_provider_model_is_covered(self): + assert get_model_price(MockProvider.name, MockProvider.models[0]) is not None + + def test_lookup_is_case_and_whitespace_insensitive(self): + assert get_model_price(" OpenAI ", " GPT-4O-MINI ") == get_model_price( + "openai", "gpt-4o-mini" + ) + + def test_unknown_provider_and_model_return_none(self): + assert get_model_price("does-not-exist", "gpt-4o") is None + assert get_model_price("openai", "gpt-0") is None + assert get_model_price(None, None) is None + + def test_entries_are_documented_as_estimates(self): + for price in DEFAULT_PRICE_TABLE.values(): + if price.provider == "mock": + assert price.estimated is False + else: + assert price.estimated is True + assert price.to_dict()["disclaimer"] == ESTIMATE_DISCLAIMER + + def test_listing_and_grouping(self): + assert [p.provider for p in list_model_prices("mock")] == ["mock"] + assert len(list_model_prices()) == len(DEFAULT_PRICE_TABLE) + grouped = price_table() + assert set(grouped) == {"mock", "openai", "anthropic"} + assert grouped["mock"][0]["model"] == "mock-1" + + +class TestCostEstimate: + def test_cost_uses_input_and_output_rates(self): + price = get_model_price("openai", "gpt-4o") + cost = estimate_cost_usd( + "openai", + "gpt-4o", + prompt_tokens=2_000, + completion_tokens=1_000, + ) + expected = 2 * price.input_per_1k_usd + 1 * price.output_per_1k_usd + assert cost == pytest.approx(expected) + + def test_zero_tokens_cost_nothing_for_known_models(self): + assert estimate_cost_usd("openai", "gpt-4o") == 0 + + def test_unknown_model_cost_is_none(self): + assert estimate_cost_usd("openai", "unknown", prompt_tokens=10, completion_tokens=5) is None + + def test_negative_tokens_are_rejected(self): + with pytest.raises(ValueError): + estimate_cost_usd("openai", "gpt-4o", prompt_tokens=-1) + with pytest.raises(ValueError): + estimate_cost_usd("openai", "gpt-4o", completion_tokens=-1) From 91d44c4745923be970f5bb1ed330a88acf24d825 Mon Sep 17 00:00:00 2001 From: Hey-Yetunde <235164239+Hey-Yetunde@users.noreply.github.com> Date: Sat, 17 Oct 2026 14:39:04 +0100 Subject: [PATCH 3/3] feat(plugins): gate updates by compatibility, bound execution, add Stellar example Refs #405, #408, #415. - #405: add an owner-only update endpoint plus update_plugin(), identity-bound to the registered id/entry point. The manifest's PEP 440 compatibility range is enforced before any write, with an error naming the range and the running app version. - #408: run plugin event handlers and lifecycle hooks with a configurable PLUGIN_EXECUTION_TIMEOUT budget in a daemon thread. Overruns are abandoned, recorded as one bounded error report, and dispatch/lifecycle state changes continue safely. - #415: add examples/plugins/stellar_notifier, a read-only, offline example that formats bounded summaries for stellar.analysis.completed, with schema/load/event/bounding tests. --- .env.example | 5 + app/config.py | 7 + app/plugins/routes.py | 89 +++++-- app/services/events.py | 7 +- app/services/plugin_compat.py | 40 +++ app/services/plugin_execution.py | 139 ++++++++++ app/services/plugin_ops.py | 61 +++++ app/services/plugins.py | 3 +- docs/plugins.md | 65 ++++- examples/plugins/stellar_notifier/README.md | 54 ++++ .../plugins/stellar_notifier/manifest.json | 14 + examples/plugins/stellar_notifier/plugin.py | 141 ++++++++++ tests/test_plugin_timeout.py | 198 ++++++++++++++ tests/test_plugin_update.py | 250 ++++++++++++++++++ tests/test_stellar_notifier_example.py | 184 +++++++++++++ 15 files changed, 1234 insertions(+), 23 deletions(-) create mode 100644 app/services/plugin_execution.py create mode 100644 examples/plugins/stellar_notifier/README.md create mode 100644 examples/plugins/stellar_notifier/manifest.json create mode 100644 examples/plugins/stellar_notifier/plugin.py create mode 100644 tests/test_plugin_timeout.py create mode 100644 tests/test_plugin_update.py create mode 100644 tests/test_stellar_notifier_example.py diff --git a/.env.example b/.env.example index 027ae5c..c612985 100644 --- a/.env.example +++ b/.env.example @@ -43,6 +43,11 @@ LLM_CACHE_MAX_ENTRIES=256 PLUGIN_TRUST_POLICY=if-present PLUGIN_TRUSTED_KEYS={} +# Plugin execution budget (Phase 9, issue #408): seconds an event handler or +# lifecycle hook may run before it is abandoned (daemon thread), recorded as a +# bounded error report, and dispatch continues. 0 disables the bound. +PLUGIN_EXECUTION_TIMEOUT=5 + # Plugin outbound network policy (Phase 8, issue #193). Plugin code must make # requests through app.services.plugin_network; the guard is https-only, denies # private/loopback/link-local/reserved targets by default, and only reaches diff --git a/app/config.py b/app/config.py index 4566568..ad867c8 100644 --- a/app/config.py +++ b/app/config.py @@ -122,6 +122,13 @@ class Config: # discoverable plugins. PLUGIN_DISCOVERY_DIR = os.getenv("PLUGIN_DISCOVERY_DIR", str(BASE_DIR / "plugins")) + # Plugin execution budget (#408). Event handlers and lifecycle hooks run in + # a daemon thread and are abandoned once they exceed this many seconds, so + # one slow plugin cannot stall a request. The timeout is recorded as a + # bounded error report and dispatch continues with the remaining handlers. + # Set to 0 (or negative) to disable the bound for local debugging. + PLUGIN_EXECUTION_TIMEOUT = float(os.getenv("PLUGIN_EXECUTION_TIMEOUT", "5")) + # Plugin outbound network policy (#193). Every plugin-triggered request must # pass ``app.services.plugin_network``: it is https-only, denies # private/loopback/link-local/reserved targets by default, and only reaches diff --git a/app/plugins/routes.py b/app/plugins/routes.py index 4c884d7..8211d0a 100644 --- a/app/plugins/routes.py +++ b/app/plugins/routes.py @@ -24,10 +24,21 @@ from app.services.events import emit_event from app.services.permissions import require_workspace_capability, resolve_workspace, role_for from app.services.plugin_audit import record_plugin_audit -from app.services.plugin_compat import compatibility_status +from app.services.plugin_compat import ( + IncompatibleCompatibilityError, + InvalidCompatibilityError, + compatibility_status, + enforce_compatible, +) from app.services.plugin_config import validate_plugin_config from app.services.plugin_errors import list_workspace_error_reports -from app.services.plugin_ops import discover_plugins +from app.services.plugin_ops import ( + PluginEntryPointMismatchError, + PluginNotFoundError, + PluginUpdateError, + discover_plugins, + update_plugin, +) from app.services.plugins import ManifestValidationError, PluginError, PluginManifest @@ -204,21 +215,10 @@ def api_install_plugin(workspace_id: int): # Reject a plugin whose explicit PEP 440 compatibility range does not # include the running application version (an absent field is unrestricted). - if manifest.compatibility is not None: - status = compatibility_status(manifest.compatibility) - if not status["compatible"]: - return ( - jsonify( - { - "error": ( - f"Plugin {manifest.id} requires application " - f"{manifest.compatibility}, but this app is " - f"{status['app_version']}." - ) - } - ), - 400, - ) + try: + enforce_compatible(manifest.compatibility, subject=f"Plugin {manifest.id}") + except (IncompatibleCompatibilityError, InvalidCompatibilityError) as exc: + return jsonify({"error": str(exc)}), 400 plugin = Plugin.query.filter_by(id=manifest.id).first() if plugin is None: @@ -262,6 +262,61 @@ def api_install_plugin(workspace_id: int): return jsonify(_serialize(plugin, workspace_id)), 201 +@bp.route("/api/workspaces//plugins//update", methods=["POST"]) +@login_required +@require_workspace_capability("manage_plugins") +def api_update_plugin(workspace_id: int, plugin_id: str): + """Update a registered plugin from a validated manifest (owner only). + + The manifest is fully validated and identity-bound: its ``id`` must match + the plugin being updated and its ``entry_point`` must not change, so an + update can never point an installed plugin at different code. The declared + PEP 440 ``compatibility`` range is enforced before anything is written; an + update that excludes the running application version is refused with + ``400`` and a message naming both the range and the current version. The + registered plugin (and every workspace installation) is left untouched. + + Capabilities and per-workspace configuration are not modified by an update; + grants stay explicit and workspace-scoped. + """ + installation = _installation(plugin_id, workspace_id) + if installation is None: + return jsonify({"error": "Plugin is not installed in this workspace."}), 404 + + data = request.get_json(silent=True) or {} + manifest_data = data.get("manifest") + if not isinstance(manifest_data, dict): + return jsonify({"error": "A manifest object is required."}), 400 + + try: + manifest = PluginManifest.from_dict( + manifest_data, + trusted_publishers=current_app.config["PLUGIN_TRUSTED_KEYS"], + trust_policy=current_app.config["PLUGIN_TRUST_POLICY"], + ) + except ManifestValidationError as exc: + return jsonify({"error": f"Invalid plugin manifest: {exc}"}), 400 + except PluginError as exc: + return jsonify({"error": str(exc)}), 400 + + try: + plugin = update_plugin(plugin_id, manifest) + except (IncompatibleCompatibilityError, InvalidCompatibilityError) as exc: + db.session.rollback() + return jsonify({"error": str(exc)}), 400 + except PluginEntryPointMismatchError as exc: + db.session.rollback() + return jsonify({"error": str(exc)}), 409 + except PluginUpdateError as exc: + db.session.rollback() + return jsonify({"error": str(exc)}), 400 + except PluginNotFoundError: + db.session.rollback() + return jsonify({"error": "Plugin not found."}), 404 + + return jsonify(_serialize(plugin, workspace_id)) + + @bp.route("/api/workspaces//plugins//enable", methods=["POST"]) @login_required @require_workspace_capability("manage_plugins") diff --git a/app/services/events.py b/app/services/events.py index f256e8e..62044cc 100644 --- a/app/services/events.py +++ b/app/services/events.py @@ -28,6 +28,7 @@ from collections.abc import Callable from dataclasses import dataclass from datetime import UTC, datetime +from functools import partial from typing import Any from app.extensions import db @@ -35,6 +36,7 @@ from app.services.capabilities import Capability, CapabilityStore from app.services.permissions import role_for from app.services.plugin_audit import record_plugin_audit +from app.services.plugin_execution import run_with_timeout logger = logging.getLogger(__name__) @@ -319,7 +321,10 @@ def dispatch( ) continue try: - handler(event=event) + run_with_timeout( + partial(handler, event=event), + operation=f"dispatch:{event.event_type}", + ) results["successful"] += 1 except Exception as e: results["failed"] += 1 diff --git a/app/services/plugin_compat.py b/app/services/plugin_compat.py index 1a454e7..1c0c2d4 100644 --- a/app/services/plugin_compat.py +++ b/app/services/plugin_compat.py @@ -40,6 +40,10 @@ class InvalidCompatibilityError(ValueError): """Raised when a compatibility field is not a valid PEP 440 specifier.""" +class IncompatibleCompatibilityError(ValueError): + """Raised when a valid compatibility range excludes the running app.""" + + @lru_cache(maxsize=1) def _pyproject_version() -> str | None: """Read the ``project.version`` from the repository ``pyproject.toml``.""" @@ -123,3 +127,39 @@ def compatibility_status(compatibility: str | None) -> dict[str, object]: "valid": valid, "compatible": plugin_compatible(compatibility) if valid else False, } + + +def enforce_compatible(compatibility: str | None, *, subject: str = "Plugin") -> None: + """Refuse a plugin whose declared range excludes the running app version. + + ``subject`` names the thing being checked (for example + ``"Plugin stellar-tools v1.2.0"``) and appears verbatim in the error, which + always reports both the declared range and the running version so the + operator can act on it. An omitted/empty range is unrestricted and never + raises. + + Raises: + InvalidCompatibilityError: If ``compatibility`` is not valid PEP 440. + IncompatibleCompatibilityError: If the range excludes this app version. + """ + status = compatibility_status(compatibility) + if not status["valid"]: + raise InvalidCompatibilityError(f"Invalid compatibility specifier: {compatibility!r}") + if status["compatible"]: + return + raise IncompatibleCompatibilityError( + f"{subject} requires application {status['compatibility']}, " + f"but this app is {status['app_version']}." + ) + + +__all__ = [ + "IncompatibleCompatibilityError", + "InvalidCompatibilityError", + "app_version", + "compatibility_status", + "enforce_compatible", + "is_valid_compatibility", + "parse_compatibility", + "plugin_compatible", +] diff --git a/app/services/plugin_execution.py b/app/services/plugin_execution.py new file mode 100644 index 0000000..a734ac3 --- /dev/null +++ b/app/services/plugin_execution.py @@ -0,0 +1,139 @@ +"""Bounded execution for plugin callbacks (issue #408). + +A plugin handler or lifecycle hook is third-party code: it may block on I/O, +loop forever, or simply take far longer than a request can afford. This module +runs such callbacks in a short-lived daemon thread and stops waiting once the +configured budget expires, so one slow plugin cannot stall a request. + +The budget is ``PLUGIN_EXECUTION_TIMEOUT`` (seconds, default +:data:`DEFAULT_TIMEOUT_SECONDS`). A value of ``0`` (or negative) disables the +bound for local debugging. + +Limitations (Python cannot kill threads): + +* A callback that exceeds the budget is **abandoned**, not interrupted. It is a + daemon thread, so it never blocks interpreter shutdown, but it may still be + running (and finish) in the background. Callbacks must therefore be + idempotent and must not be relied on to have stopped. +* The current Flask application context is copied into the worker thread when + one is active, so plugins can still use ``current_app``. The calling thread + blocks until the callback finishes or the budget expires. +""" + +from __future__ import annotations + +import logging +import threading +from collections.abc import Callable +from typing import Any + +logger = logging.getLogger(__name__) + +#: Fallback budget used when the application configuration is unavailable. +DEFAULT_TIMEOUT_SECONDS = 5.0 + +#: Upper bound applied to the operation label stored in error reports. +_MAX_OPERATION_CHARS = 160 + + +class PluginTimeoutError(TimeoutError): + """Raised when a plugin callback exceeds its execution budget.""" + + def __init__(self, operation: str, timeout: float) -> None: + self.operation = str(operation)[:_MAX_OPERATION_CHARS] + self.timeout = timeout + super().__init__(f"Plugin operation '{self.operation}' timed out after {timeout:g}s") + + +def configured_timeout() -> float: + """Return the configured callback budget in seconds (``<= 0`` disables it).""" + value: Any = DEFAULT_TIMEOUT_SECONDS + try: + from flask import current_app, has_app_context + + if has_app_context(): + value = current_app.config.get("PLUGIN_EXECUTION_TIMEOUT", DEFAULT_TIMEOUT_SECONDS) + except Exception: # pragma: no cover - Flask is always available in the app + logger.debug("Could not read the plugin execution timeout", exc_info=True) + try: + return float(value) + except (TypeError, ValueError): # pragma: no cover - defensive + return DEFAULT_TIMEOUT_SECONDS + + +def _preserve_app_context(func: Callable[[], Any]) -> Callable[[], Any]: + """Return ``func`` wrapped so an active Flask app context is available.""" + try: + from flask import copy_current_app_context, has_app_context + + if not has_app_context(): + return func + return copy_current_app_context(func) + except Exception: # pragma: no cover - context copying is best effort + return func + + +def run_with_timeout( + func: Callable[[], Any], + *, + operation: str, + timeout: float | None = None, +) -> Any: + """Call ``func`` and raise :class:`PluginTimeoutError` if it overruns. + + Args: + func: Zero-argument callable to execute. + operation: Short label (for example ``dispatch:project.created``) used + in the timeout error and the recorded error report. + timeout: Budget in seconds; defaults to :func:`configured_timeout`. + A value ``<= 0`` disables the bound and calls ``func`` directly. + + Returns: + Whatever ``func`` returns. + + Raises: + PluginTimeoutError: If ``func`` does not finish within the budget. + Exception: Any exception raised by ``func`` is re-raised unchanged. + """ + limit = configured_timeout() if timeout is None else float(timeout) + if limit <= 0: + return func() + + call = _preserve_app_context(func) + outcome: dict[str, Any] = {} + finished = threading.Event() + + def _target() -> None: + try: + outcome["result"] = call() + except BaseException as exc: # re-raised in the caller thread below + outcome["error"] = exc + finally: + finished.set() + + worker = threading.Thread( + target=_target, + name=f"plugin-callback:{str(operation)[:_MAX_OPERATION_CHARS]}", + daemon=True, + ) + worker.start() + + if not finished.wait(limit): + logger.warning( + "Plugin operation %s exceeded its %.3fs budget; abandoning the callback", + operation, + limit, + ) + raise PluginTimeoutError(operation, limit) + + if "error" in outcome: + raise outcome["error"] + return outcome.get("result") + + +__all__ = [ + "DEFAULT_TIMEOUT_SECONDS", + "PluginTimeoutError", + "configured_timeout", + "run_with_timeout", +] diff --git a/app/services/plugin_ops.py b/app/services/plugin_ops.py index d065c8f..00f52e3 100644 --- a/app/services/plugin_ops.py +++ b/app/services/plugin_ops.py @@ -24,6 +24,7 @@ from app.extensions import db from app.models import Plugin +from app.services.plugin_compat import enforce_compatible from app.services.plugins import ( ManifestValidationError, PluginError, @@ -41,6 +42,14 @@ class PluginNotFoundError(ValueError): """Raised when a plugin id does not exist in the database.""" +class PluginUpdateError(ValueError): + """Raised when a manifest cannot be applied to a registered plugin.""" + + +class PluginEntryPointMismatchError(PluginUpdateError): + """Raised when an update would retarget a registered plugin's entry point.""" + + class PluginDependencyResolutionError(ValueError): """Raised when a plugin cannot be enabled because dependencies are unsatisfied.""" @@ -194,16 +203,68 @@ def install_from_local_dir(path: str) -> Plugin: return plugin +def update_plugin(plugin_id: str, manifest: PluginManifest) -> Plugin: + """Apply a validated manifest to an already-registered plugin. + + Updates are identity-bound like installs: the manifest id must equal + ``plugin_id`` and the entry point must not change, so an update can never + retarget an installed plugin at different code. + + The declared PEP 440 ``compatibility`` range is enforced against the + running application *before* anything is written. An update whose range + excludes this app version raises + :class:`~app.services.plugin_compat.IncompatibleCompatibilityError` naming + both the range and the current version, and leaves the registered plugin + untouched. Invalid specifiers raise + :class:`~app.services.plugin_compat.InvalidCompatibilityError`. + + Capability grants are intentionally not modified: they stay explicit and + per-workspace, and are managed only through the capability endpoints. + """ + plugin = get_plugin(plugin_id) + + if manifest.id != plugin_id: + raise PluginUpdateError( + f"Manifest id {manifest.id!r} does not match the plugin being updated " + f"({plugin_id!r})." + ) + if manifest.entry_point != plugin.entry_point: + raise PluginEntryPointMismatchError( + f"Manifest entry_point does not match the registered plugin {plugin_id}." + ) + + enforce_compatible( + manifest.compatibility, + subject=f"Plugin {plugin_id} {manifest.version}", + ) + + plugin.name = manifest.name + plugin.version = manifest.version + plugin.description = manifest.description + plugin.author = manifest.author + plugin.capabilities = list(manifest.capabilities) + plugin.permissions = list(manifest.permissions or []) + plugin.dependencies = list(manifest.dependencies or []) + plugin.compatibility = manifest.compatibility + plugin.configuration = dict(manifest.configuration or {}) + plugin.trust_state = manifest.trust_state + plugin.trust_publisher = manifest.trust_publisher + db.session.commit() + return plugin + + __all__ = [ "ManifestValidationError", "PluginDependencyResolutionError", "PluginError", "PluginManifest", "PluginNotFoundError", + "PluginUpdateError", "discover_plugins", "get_plugin", "install_from_local_dir", "list_plugins", "resolve_plugin_dependencies", "set_plugin_enabled", + "update_plugin", ] diff --git a/app/services/plugins.py b/app/services/plugins.py index 2c1a197..e59914b 100644 --- a/app/services/plugins.py +++ b/app/services/plugins.py @@ -26,6 +26,7 @@ from app.services.plugin_compat import is_valid_compatibility from app.services.plugin_deps import PluginRef +from app.services.plugin_execution import run_with_timeout logger = logging.getLogger(__name__) @@ -498,7 +499,7 @@ def invoke_hook(self, hook: str, app: Any | None = None) -> dict[str, Any]: if not callable(method): return {"hook": hook, "called": False, "ok": True, "error": None} try: - method() + run_with_timeout(method, operation=f"hook:{hook}") except Exception as exc: logger.error( "Plugin %s hook %s failed: %s", diff --git a/docs/plugins.md b/docs/plugins.md index 0eb3eaf..6eaa04e 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -74,6 +74,9 @@ separated ranges). Enforcement lives in - **Invalid** specifier → the manifest is rejected during validation. - **Incompatible** range → installation is refused with a clear message naming the required range and the current app version. +- **Updates are gated too**: `POST .../plugins//update` refuses a + manifest whose range excludes the running version *before* writing anything, + so an incompatible release can never replace a working installation. - Compatible plugins register normally, and the management API exposes a compatibility badge in plugin metadata (`compatibility`, `compatible_with_app`, and `app_version`). @@ -185,6 +188,23 @@ Per-workspace lifecycle is also observable through the dispatcher events enable/disable and an owner-only `uninstall` action that revokes every workspace capability grant and removes the workspace installation. +### Execution timeout (#408) + +Plugin event handlers and lifecycle hooks run with a bounded execution budget +(`app/services/plugin_execution.py`). The budget is +`PLUGIN_EXECUTION_TIMEOUT` seconds (default `5`; `0` or negative disables it): + +- the callback runs in a short-lived daemon thread while the request waits; +- if it overruns, the dispatcher **stops waiting**, records one bounded error + report (`plugin_error_reports`, operation `dispatch:` or + `hook:`), and continues with the next handler or lifecycle step; +- state changes still land: enabling/disabling always completes, and dispatch + keeps running the remaining subscribers. + +Python cannot interrupt a thread, so an over-budget callback is **abandoned** +rather than killed. It may still finish in the background, which is why plugin +code must be idempotent and must not depend on being stopped. + ## Plugin management (API + UI) ### Operator CLI @@ -227,6 +247,7 @@ management operations. | `GET` | `/plugins/api/workspaces//plugins` | List plugins + installation state | any member | | `GET` | `/plugins/api/workspaces//plugins/` | Inspect one plugin | any member | | `POST` | `/plugins/api/workspaces//plugins/install` | Install a trusted/local manifest | owner | +| `POST` | `/plugins/api/workspaces//plugins//update` | Update a registered plugin from a manifest (compatibility-gated) | owner | | `POST` | `/plugins/api/workspaces//plugins//enable` | Enable the workspace installation | owner | | `POST` | `/plugins/api/workspaces//plugins//disable` | Disable the workspace installation | owner | | `POST` | `/plugins/api/workspaces//plugins//capabilities` | Explicitly grant/revoke capabilities (`{"grant": [...], "revoke": [...]}`) | owner | @@ -249,6 +270,26 @@ Capabilities granted through the API must be declared in the plugin's manifest (declared vs granted are kept distinct). The `manage_plugins` role gate is the only way to grant/revoke through the UI; grants are never implicit. +### Updating a plugin + +`POST .../plugins//update` accepts the same `{"manifest": {...}}` +shape as install, but applies it to an already-registered plugin. The manifest +is validated and identity-bound: + +- `id` must equal the plugin being updated; +- `entry_point` must not change (`409`), so an update can never point an + installed plugin at different code; +- the declared PEP 440 `compatibility` range is enforced against the running + application version **before** anything is written. An incompatible range is + refused with `400` and a message naming both the range and the current + version; the registered plugin and its workspace installations are left + untouched. + +Updates change the plugin's metadata (version, description, capabilities, +dependencies, configuration defaults, trust fields) but never capability grants +or per-workspace configuration, which stay explicit and workspace-scoped. The +plugin must be installed in the authorizing workspace (`404` otherwise). + ### Enable / disable `enable`/`disable` operate on the **workspace installation** @@ -280,10 +321,11 @@ plugin id, the operation (`dispatch:`, `hook:`, …), the exception type, and a length-bounded message. **No stack traces, event payloads, or secrets are stored by default.** -Recording happens automatically when a plugin dispatch handler raises and when -a lifecycle hook fails inside an app context; it is best-effort and never -changes the existing failure-isolation behavior. Reports are read back -owner-scoped via `GET /plugins/api/workspaces//plugin-errors` (owner only, +Recording happens automatically when a plugin dispatch handler raises (including +when it exceeds `PLUGIN_EXECUTION_TIMEOUT`) and when a lifecycle hook fails or +times out inside an app context; it is best-effort and never changes the +existing failure-isolation behavior. Reports are read back owner-scoped via +`GET /plugins/api/workspaces//plugin-errors` (owner only, workspace-isolated) and operator-wide via `flask plugins errors [--json]`. ## Audit trail (capabilities & state) @@ -364,6 +406,21 @@ only when the host dispatches a live event (see [Security](#security)). The manifest schema and the load/register flow are covered by `tests/test_hello_plugin_example.py`. +### Stellar notifier example (#415) + +[`examples/plugins/stellar_notifier/`](../examples/plugins/stellar_notifier) +shows an event-driven, read-only integration for Stellar analysis: + +- declares `STELLAR_READ` and subscribes to `stellar.analysis.completed`; +- formats a one-line summary (`Project 7 — Stellar detected, confidence 0.98`) + that is capped at `MAX_SUMMARY_CHARS`, with each field truncated and the + in-memory history bounded to `MAX_RECENT_SUMMARIES`; +- copies the payload instead of mutating it and makes **no network or database + calls**, so it runs offline under the mock network; +- is validated end-to-end by + `tests/test_stellar_notifier_example.py` (schema, load/register, event path, + bounding, read-only behaviour, and an offline socket guard). + ## Security See [security.md](security.md) for the full security model. In short: diff --git a/examples/plugins/stellar_notifier/README.md b/examples/plugins/stellar_notifier/README.md new file mode 100644 index 0000000..4e9c0cd --- /dev/null +++ b/examples/plugins/stellar_notifier/README.md @@ -0,0 +1,54 @@ +# Stellar notifier plugin (example) + +A read-only example plugin that reacts to `stellar.analysis.completed` and +formats a short, bounded summary. It is used by +[`docs/plugins.md`](../../../docs/plugins.md) and +[`tests/test_stellar_notifier_example.py`](../../../tests/test_stellar_notifier_example.py). + +- `manifest.json` — manifest (id `stellar-notifier`, entry point, declared + `STELLAR_READ` capability). +- `plugin.py` — `StellarNotifierPlugin`, which subscribes to + `stellar.analysis.completed` and records at most + `MAX_RECENT_SUMMARIES` summaries. + +## Behaviour + +For each event the plugin reads only `project_id`, `detected`, `confidence`, +and an optional `network`: + +``` +Project 7 — Stellar detected, confidence 0.98 +``` + +Every field is truncated (`MAX_FIELD_CHARS`) and the final summary is capped at +`MAX_SUMMARY_CHARS`. The event payload is copied, never mutated. The plugin +performs **no network requests and no database writes**, so it runs entirely +offline under the mock network. + +Nothing is granted automatically: the plugin must be installed, enabled in a +workspace, and explicitly granted `STELLAR_READ` before its handler runs. + +## Try it + +```bash +# Register the plugin (operator scope; no code is executed, no grants created) +flask plugins install examples/plugins/stellar_notifier +flask plugins inspect stellar-notifier +flask plugins enable stellar-notifier +``` + +Make the entry point importable during local development: + +```bash +# POSIX +export PYTHONPATH="$PWD/examples/plugins:$PYTHONPATH" +# PowerShell +$env:PYTHONPATH = "$PWD\examples\plugins;$env:PYTHONPATH" +``` + +Then install it into a workspace and grant `STELLAR_READ` through the +management API or UI, or run the offline demo: + +```bash +python examples/plugins/stellar_notifier/plugin.py +``` diff --git a/examples/plugins/stellar_notifier/manifest.json b/examples/plugins/stellar_notifier/manifest.json new file mode 100644 index 0000000..fb908a8 --- /dev/null +++ b/examples/plugins/stellar_notifier/manifest.json @@ -0,0 +1,14 @@ +{ + "id": "stellar-notifier", + "name": "Stellar Notifier", + "version": "0.1.0", + "description": "Example read-only plugin that formats a bounded summary for stellar.analysis.completed events.", + "author": "AI Code Assistant contributors", + "entry_point": "stellar_notifier.plugin:StellarNotifierPlugin", + "compatibility": ">=0.1.0", + "capabilities": ["STELLAR_READ"], + "permissions": ["read:stellar:analysis"], + "configuration": { + "include_confidence": true + } +} diff --git a/examples/plugins/stellar_notifier/plugin.py b/examples/plugins/stellar_notifier/plugin.py new file mode 100644 index 0000000..96a469b --- /dev/null +++ b/examples/plugins/stellar_notifier/plugin.py @@ -0,0 +1,141 @@ +"""Stellar notifier example plugin (issue #415). + +Subscribes to ``stellar.analysis.completed`` and turns each event into a short, +bounded, human-readable summary. The plugin is deliberately: + +* **read-only** - it copies the event payload and never mutates it, and never + writes to the database; +* **offline** - it performs no network requests, so the whole example runs + against the mock network (and in tests) without credentials; +* **bounded** - every field and the final summary are truncated, and the + in-memory history keeps at most :data:`MAX_RECENT_SUMMARIES` entries, so a + noisy or hostile event cannot grow memory without limit. + +Importing this module has no side effects: nothing is subscribed until +``register()`` (called from ``on_enable``) runs. +""" + +from __future__ import annotations + +import logging + +from app.services.events import get_dispatcher + +logger = logging.getLogger(__name__) + +PLUGIN_ID = "stellar-notifier" +EVENT_TYPE = "stellar.analysis.completed" + +#: Maximum characters kept for the formatted one-line summary. +MAX_SUMMARY_CHARS = 280 +#: Maximum characters kept for an individual payload field in the summary. +MAX_FIELD_CHARS = 80 +#: Maximum number of summaries retained in memory. +MAX_RECENT_SUMMARIES = 20 +#: Field names copied from the event payload (everything else is ignored). +SUMMARY_FIELDS = ("project_id", "detected", "confidence", "network") + + +def _bounded_with_flag(value: object, limit: int) -> tuple[str, bool]: + """Return ``value`` as a bounded single-line string and whether it was cut.""" + if value is None: + return "", False + text = " ".join(str(value).split()) + if len(text) > limit: + return text[: max(limit - 1, 0)].rstrip() + "…", True + return text, False + + +def _bounded(value: object, limit: int) -> str: + """Return ``value`` as a single-line string of at most ``limit`` chars.""" + return _bounded_with_flag(value, limit)[0] + + +class StellarNotifierPlugin: + """Format bounded summaries for ``stellar.analysis.completed`` events.""" + + def __init__(self, app=None, manifest=None): + self.app = app + self.manifest = manifest + self.recent_summaries: list[dict] = [] + + @staticmethod + def format_summary(event) -> dict: + """Return a bounded, read-only summary of ``event``. + + Only :data:`SUMMARY_FIELDS` are read from the payload; free-form or + unrelated keys are never copied, so the summary cannot leak arbitrary + event content. The payload itself is not modified. + """ + data = dict(event.data or {}) + project_id = data.get("project_id") + detected = bool(data.get("detected")) + confidence = data.get("confidence") + project_text, project_cut = _bounded_with_flag(project_id, MAX_FIELD_CHARS) + confidence_text, confidence_cut = _bounded_with_flag(confidence, 32) + network, network_cut = _bounded_with_flag(data.get("network"), MAX_FIELD_CHARS) + + parts = [f"Project {project_text or 'unknown'}"] + parts.append("Stellar detected" if detected else "No Stellar detected") + if confidence is not None: + parts.append(f"confidence {confidence_text}") + if network: + parts.append(f"network {network}") + raw_summary = f"{parts[0]} — " + ", ".join(parts[1:]) + summary = _bounded(raw_summary, MAX_SUMMARY_CHARS) + + return { + "plugin": PLUGIN_ID, + "event": event.event_type, + "project_id": project_id, + "detected": detected, + "confidence": confidence, + "network": network or None, + "summary": summary, + "truncated": project_cut or confidence_cut or network_cut or summary != raw_summary, + } + + def register(self) -> None: + """Subscribe to ``stellar.analysis.completed`` (idempotent).""" + dispatcher = get_dispatcher() + dispatcher.unsubscribe(EVENT_TYPE, self.on_event) + dispatcher.subscribe(EVENT_TYPE, self.on_event, plugin_id=PLUGIN_ID) + + def on_event(self, event): + """Handle one event and keep a bounded in-memory history.""" + summary = self.format_summary(event) + self.recent_summaries.append(summary) + # Keep only the most recent entries; never let the history grow. + del self.recent_summaries[:-MAX_RECENT_SUMMARIES] + logger.info("stellar-notifier: %s", summary["summary"]) + return summary + + def on_enable(self) -> None: + """Lifecycle hook: subscribe once the plugin is enabled.""" + self.register() + + def on_disable(self) -> None: + """Lifecycle hook: stop receiving events while disabled.""" + get_dispatcher().unsubscribe(EVENT_TYPE, self.on_event) + + +def main() -> None: + """Offline demo: register, handle one synthetic event, unsubscribe.""" + from app.services.events import Event + + instance = StellarNotifierPlugin() + instance.register() + try: + summary = instance.on_event( + Event( + event_type=EVENT_TYPE, + data={"project_id": 1, "detected": True, "confidence": 0.98}, + ) + ) + print(f"{PLUGIN_ID}: {summary['summary']}") + finally: + instance.on_disable() + + +if __name__ == "__main__": # pragma: no cover - manual demo entry point + main() diff --git a/tests/test_plugin_timeout.py b/tests/test_plugin_timeout.py new file mode 100644 index 0000000..c284efc --- /dev/null +++ b/tests/test_plugin_timeout.py @@ -0,0 +1,198 @@ +"""Tests for bounded plugin execution (issue #408). + +A deliberately slow handler/hook must not hold a request hostage: dispatch +abandons it after the configured budget, records one bounded error report, and +continues with the remaining handlers. +""" + +import threading +import time +import types + +import pytest + +from app.extensions import db +from app.models import Plugin as PluginRow +from app.models import PluginErrorReport +from app.models.plugin_error_report import MAX_ERROR_MESSAGE_CHARS +from app.services.events import Event, EventDispatcher, get_dispatcher +from app.services.plugin_execution import ( + DEFAULT_TIMEOUT_SECONDS, + PluginTimeoutError, + configured_timeout, + run_with_timeout, +) +from app.services.plugins import Plugin, PluginManifest, PluginRegistry + + +class TestRunWithTimeout: + def test_returns_the_callback_result(self): + assert run_with_timeout(lambda: 42, operation="test", timeout=1) == 42 + + def test_reraises_callback_errors_unchanged(self): + error = ValueError("boom") + + def fail(): + raise error + + with pytest.raises(ValueError) as excinfo: + run_with_timeout(fail, operation="test", timeout=1) + assert excinfo.value is error + + def test_slow_callback_times_out(self): + release = threading.Event() + try: + with pytest.raises(PluginTimeoutError) as excinfo: + run_with_timeout( + lambda: release.wait(5), + operation="dispatch:project.created", + timeout=0.05, + ) + finally: + release.set() + + assert excinfo.value.operation == "dispatch:project.created" + assert excinfo.value.timeout == pytest.approx(0.05) + assert "timed out" in str(excinfo.value) + + def test_non_positive_timeout_disables_the_bound(self): + assert run_with_timeout(lambda: time.sleep(0.05), operation="test", timeout=0) is None + assert run_with_timeout(lambda: 7, operation="test", timeout=-1) == 7 + + def test_configured_timeout_reads_the_app_config(self, app): + app.config["PLUGIN_EXECUTION_TIMEOUT"] = 0.25 + assert configured_timeout() == pytest.approx(0.25) + + def test_configured_timeout_falls_back_when_unset(self, app): + app.config.pop("PLUGIN_EXECUTION_TIMEOUT", None) + assert configured_timeout() == DEFAULT_TIMEOUT_SECONDS + + +class TestDispatchTimeout: + def _slow(self, release): + def handler(event): + release.wait(5) + + return handler + + def test_slow_handler_does_not_block_other_handlers(self, app): + app.config["PLUGIN_EXECUTION_TIMEOUT"] = 0.05 + release = threading.Event() + calls = [] + dispatcher = get_dispatcher() + + def fast(event): + calls.append("fast") + + dispatcher.subscribe("project.created", self._slow(release)) + dispatcher.subscribe("project.created", fast) + try: + started = time.perf_counter() + result = dispatcher.dispatch(Event(event_type="project.created", data={})) + elapsed = time.perf_counter() - started + finally: + release.set() + + assert result["failed"] == 1 + assert result["successful"] == 1 + assert calls == ["fast"] + assert isinstance(result["errors"][0][1], PluginTimeoutError) + assert elapsed < 1.0 + + def test_disabled_timeout_allows_slow_handler(self, app): + app.config["PLUGIN_EXECUTION_TIMEOUT"] = 0 + dispatcher = EventDispatcher() + + def slow(event): + time.sleep(0.05) + + dispatcher.subscribe("project.created", slow) + result = dispatcher.dispatch(Event(event_type="project.created", data={})) + + assert result["successful"] == 1 + assert result["failed"] == 0 + + def test_timed_out_plugin_handler_records_a_bounded_error_report(self, app): + app.config["PLUGIN_EXECUTION_TIMEOUT"] = 0.05 + db.session.add( + PluginRow( + id="slow-plugin", + name="Slow plugin", + version="0.1.0", + entry_point="slow_plugin:SlowPlugin", + capabilities=["GITHUB_READ"], + enabled=True, + ) + ) + db.session.commit() + + release = threading.Event() + dispatcher = get_dispatcher() + dispatcher.subscribe("github.connected", self._slow(release), plugin_id="slow-plugin") + try: + result = dispatcher.dispatch(Event(event_type="github.connected", data={})) + finally: + release.set() + + assert result["failed"] == 1 + report = ( + PluginErrorReport.query.filter_by(plugin_id="slow-plugin") + .order_by(PluginErrorReport.id.desc()) + .first() + ) + assert report is not None + assert report.operation == "dispatch:github.connected" + assert "timed out" in report.message + assert len(report.message) <= MAX_ERROR_MESSAGE_CHARS + + +class TestLifecycleHookTimeout: + def _registry_with_slow_hook(self, release): + class SlowHookPlugin: + def __init__(self, app=None, manifest=None): + pass + + def on_enable(self): + release.wait(5) + + module = types.ModuleType("slow_hook_mod") + module.SlowHookPlugin = SlowHookPlugin + manifest = PluginManifest( + id="slow-hook", + name="Slow hook", + version="0.1.0", + description="test", + author="tester", + entry_point="slow_hook_mod:SlowHookPlugin", + capabilities=["PROJECT_READ"], + ) + plugin = Plugin(manifest) + plugin.module = module + registry = PluginRegistry() + registry.register(plugin) + return registry, plugin + + def test_slow_lifecycle_hook_times_out_but_state_changes(self, app): + app.config["PLUGIN_EXECUTION_TIMEOUT"] = 0.05 + release = threading.Event() + registry, plugin = self._registry_with_slow_hook(release) + # Plugins start enabled; disable first so enable() actually runs the hook. + plugin.enabled = False + + try: + started = time.perf_counter() + registry.enable("slow-hook") + elapsed = time.perf_counter() - started + finally: + release.set() + + assert plugin.enabled is True + assert elapsed < 1.0 + report = ( + PluginErrorReport.query.filter_by(plugin_id="slow-hook") + .order_by(PluginErrorReport.id.desc()) + .first() + ) + assert report is not None + assert report.operation == "hook:on_enable" + assert "timed out" in report.message diff --git a/tests/test_plugin_update.py b/tests/test_plugin_update.py new file mode 100644 index 0000000..1087c58 --- /dev/null +++ b/tests/test_plugin_update.py @@ -0,0 +1,250 @@ +"""Tests for the compatibility gate on plugin updates (issue #405).""" + +import pytest + +from app.extensions import db +from app.models import Plugin, Workspace +from app.services.plugin_compat import ( + IncompatibleCompatibilityError, + InvalidCompatibilityError, + app_version, + enforce_compatible, +) +from app.services.plugin_ops import ( + PluginEntryPointMismatchError, + PluginUpdateError, + update_plugin, +) +from app.services.plugins import ManifestValidationError, PluginManifest + +PLUGIN_ID = "update-plugin" + +BASE_MANIFEST = { + "id": PLUGIN_ID, + "name": "Update Plugin", + "version": "1.0.0", + "description": "first release", + "author": "tester", + "entry_point": "plugins.test:TestPlugin", + "capabilities": ["PROJECT_READ"], +} + + +@pytest.fixture() +def workspace(app, make_user, login): + user = make_user() + login() + ws = Workspace(user_id=user.id, name="Update workspace") + db.session.add(ws) + db.session.commit() + return ws + + +def _install(client, workspace_id, manifest=None): + resp = client.post( + f"/plugins/api/workspaces/{workspace_id}/plugins/install", + json={"manifest": manifest or dict(BASE_MANIFEST)}, + ) + assert resp.status_code == 201, resp.get_json() + return resp + + +def _update(client, workspace_id, manifest): + return client.post( + f"/plugins/api/workspaces/{workspace_id}/plugins/{PLUGIN_ID}/update", + json={"manifest": manifest}, + ) + + +class TestCompatibilityGate: + def test_incompatible_update_is_refused_with_clear_message(self, app, client, workspace): + _install(client, workspace.id) + + resp = _update( + client, + workspace.id, + dict(BASE_MANIFEST, version="2.0.0", compatibility=">=99.0.0"), + ) + + assert resp.status_code == 400 + error = resp.get_json()["error"] + assert ">=99.0.0" in error + assert app_version() in error + plugin = db.session.get(Plugin, PLUGIN_ID) + assert plugin.version == "1.0.0" # nothing was written + + def test_compatible_update_is_applied(self, app, client, workspace): + _install(client, workspace.id) + manifest = dict( + BASE_MANIFEST, + version="1.1.0", + description="second release", + compatibility=f"<={app_version()}", + ) + + resp = _update(client, workspace.id, manifest) + + assert resp.status_code == 200 + data = resp.get_json() + assert data["version"] == "1.1.0" + assert data["compatible_with_app"] is True + plugin = db.session.get(Plugin, PLUGIN_ID) + assert plugin.version == "1.1.0" + assert plugin.description == "second release" + + def test_update_without_compatibility_is_unrestricted(self, app, client, workspace): + _install(client, workspace.id) + + resp = _update(client, workspace.id, dict(BASE_MANIFEST, version="1.2.0")) + + assert resp.status_code == 200 + assert db.session.get(Plugin, PLUGIN_ID).version == "1.2.0" + + def test_invalid_specifier_is_rejected_before_update(self, app, client, workspace): + _install(client, workspace.id) + + resp = _update( + client, + workspace.id, + dict(BASE_MANIFEST, version="2.0.0", compatibility="not-a-specifier"), + ) + + assert resp.status_code == 400 + assert "Invalid compatibility specifier" in resp.get_json()["error"] + assert db.session.get(Plugin, PLUGIN_ID).version == "1.0.0" + + +class TestUpdateIdentityAndScope: + def test_entry_point_change_is_refused(self, app, client, workspace): + _install(client, workspace.id) + + resp = _update( + client, + workspace.id, + dict(BASE_MANIFEST, version="2.0.0", entry_point="plugins.other:OtherPlugin"), + ) + + assert resp.status_code == 409 + assert db.session.get(Plugin, PLUGIN_ID).version == "1.0.0" + + def test_manifest_id_must_match_the_plugin(self, app, client, workspace): + _install(client, workspace.id) + + resp = _update( + client, + workspace.id, + dict(BASE_MANIFEST, id="other-plugin", version="2.0.0"), + ) + + assert resp.status_code == 400 + assert db.session.get(Plugin, PLUGIN_ID).version == "1.0.0" + + def test_plugin_must_be_installed_in_the_workspace(self, app, client, workspace): + owner_id = db.session.get(Workspace, workspace.id).user_id + other_ws = Workspace(user_id=owner_id, name="Other workspace") + db.session.add(other_ws) + db.session.commit() + + resp = _update(client, other_ws.id, dict(BASE_MANIFEST, version="2.0.0")) + + assert resp.status_code == 404 + + def test_manifest_object_is_required(self, client, workspace): + _install(client, workspace.id) + + resp = client.post( + f"/plugins/api/workspaces/{workspace.id}/plugins/{PLUGIN_ID}/update", + json={}, + ) + + assert resp.status_code == 400 + assert "manifest" in resp.get_json()["error"].lower() + + +class TestUpdateService: + def test_service_refuses_incompatible_manifest(self, app, workspace): + plugin = Plugin( + id=PLUGIN_ID, + name="Update Plugin", + version="1.0.0", + description="first release", + author="tester", + entry_point="plugins.test:TestPlugin", + capabilities=["PROJECT_READ"], + ) + db.session.add(plugin) + db.session.commit() + + with pytest.raises(IncompatibleCompatibilityError) as excinfo: + update_plugin( + PLUGIN_ID, + PluginManifest.from_dict(dict(BASE_MANIFEST, compatibility=">=99.0.0")), + ) + + message = str(excinfo.value) + assert ">=99.0.0" in message + assert app_version() in message + assert db.session.get(Plugin, PLUGIN_ID).version == "1.0.0" + + def test_service_refuses_entry_point_change(self, app, workspace): + db.session.add( + Plugin( + id=PLUGIN_ID, + name="Update Plugin", + version="1.0.0", + description="first release", + author="tester", + entry_point="plugins.test:TestPlugin", + capabilities=["PROJECT_READ"], + ) + ) + db.session.commit() + + with pytest.raises(PluginEntryPointMismatchError): + update_plugin( + PLUGIN_ID, + PluginManifest.from_dict( + dict(BASE_MANIFEST, entry_point="plugins.other:OtherPlugin") + ), + ) + + def test_service_refuses_id_mismatch(self, app, workspace): + db.session.add( + Plugin( + id=PLUGIN_ID, + name="Update Plugin", + version="1.0.0", + description="first release", + author="tester", + entry_point="plugins.test:TestPlugin", + capabilities=["PROJECT_READ"], + ) + ) + db.session.commit() + + with pytest.raises(PluginUpdateError): + update_plugin( + PLUGIN_ID, + PluginManifest.from_dict(dict(BASE_MANIFEST, id="other-plugin")), + ) + + +class TestEnforceCompatible: + def test_absent_and_empty_ranges_are_unrestricted(self): + enforce_compatible(None) + enforce_compatible("") + + def test_incompatible_range_reports_range_and_version(self): + with pytest.raises(IncompatibleCompatibilityError) as excinfo: + enforce_compatible(">=99.0.0", subject="Plugin demo 1.0.0") + assert "Plugin demo 1.0.0" in str(excinfo.value) + assert ">=99.0.0" in str(excinfo.value) + assert app_version() in str(excinfo.value) + + def test_invalid_range_raises_validation_error(self): + with pytest.raises(InvalidCompatibilityError): + enforce_compatible("bogus") + + def test_manifest_validation_still_rejects_invalid_range(self): + with pytest.raises(ManifestValidationError): + PluginManifest.from_dict(dict(BASE_MANIFEST, compatibility="bogus")) diff --git a/tests/test_stellar_notifier_example.py b/tests/test_stellar_notifier_example.py new file mode 100644 index 0000000..a85fac8 --- /dev/null +++ b/tests/test_stellar_notifier_example.py @@ -0,0 +1,184 @@ +"""Tests for the Stellar notifier example plugin (issue #415). + +The example must stay valid against the published manifest schema, load through +the real plugin services, and handle the ``stellar.analysis.completed`` event +path offline with a bounded, read-only summary. +""" + +from __future__ import annotations + +import copy +import importlib +import json +import re +import socket +import sys +from pathlib import Path + +import pytest + +from app.services.events import Event, get_dispatcher +from app.services.plugins import Plugin, PluginManifest, PluginRegistry + +REPO_ROOT = Path(__file__).resolve().parents[1] +EXAMPLE_DIR = REPO_ROOT / "examples" / "plugins" / "stellar_notifier" +PLUGINS_DIR = REPO_ROOT / "examples" / "plugins" +MANIFEST_PATH = EXAMPLE_DIR / "manifest.json" +SCHEMA_PATH = REPO_ROOT / "plugins" / "plugin.schema.json" + + +def _load_json(path: Path) -> dict: + return json.loads(path.read_text(encoding="utf-8")) + + +@pytest.fixture() +def example_module(monkeypatch): + """Import ``stellar_notifier.plugin`` from the examples directory.""" + monkeypatch.syspath_prepend(str(PLUGINS_DIR)) + sys.modules.pop("stellar_notifier.plugin", None) + yield importlib.import_module("stellar_notifier.plugin") + sys.modules.pop("stellar_notifier.plugin", None) + + +class TestManifest: + def test_manifest_matches_published_schema(self): + manifest = _load_json(MANIFEST_PATH) + schema = _load_json(SCHEMA_PATH) + properties = schema["properties"] + + assert set(schema["required"]) <= set(manifest) + assert schema["additionalProperties"] is False + assert set(manifest) <= set(properties) + + assert re.match(properties["id"]["pattern"], manifest["id"]) + assert re.match(properties["version"]["pattern"], manifest["version"]) + assert re.match(properties["entry_point"]["pattern"], manifest["entry_point"]) + + allowed = set(properties["capabilities"]["items"]["enum"]) + assert manifest["capabilities"] == ["STELLAR_READ"] + assert set(manifest["capabilities"]) <= allowed + + def test_manifest_parses_with_app_validator(self): + manifest = PluginManifest.from_file(MANIFEST_PATH) + + assert manifest.id == "stellar-notifier" + assert manifest.entry_point == "stellar_notifier.plugin:StellarNotifierPlugin" + assert manifest.capabilities == ["STELLAR_READ"] + assert manifest.compatibility == ">=0.1.0" + + +class TestLoadAndRegister: + def test_plugin_loads_and_registers(self, app, example_module): + manifest = PluginManifest.from_file(MANIFEST_PATH) + registry = PluginRegistry() + plugin = Plugin(manifest) + + plugin.load() + registry.register(plugin) + + assert registry.get("stellar-notifier") is plugin + assert plugin.module is not None + assert getattr(plugin.module, "StellarNotifierPlugin", None) is not None + + def test_plugin_subscribes_to_the_stellar_event(self, app, example_module): + plugin = Plugin(PluginManifest.from_file(MANIFEST_PATH)) + plugin.load() + instance = plugin.get_instance() + + instance.register() + try: + subscribers = get_dispatcher().list_subscribers("stellar.analysis.completed") + assert "stellar-notifier" in subscribers["stellar.analysis.completed"] + finally: + instance.on_disable() + + assert ( + "stellar-notifier" + not in get_dispatcher().list_subscribers("stellar.analysis.completed")[ + "stellar.analysis.completed" + ] + ) + + +class TestEventPath: + def test_event_produces_a_summary(self, app, example_module): + instance = example_module.StellarNotifierPlugin() + event = Event( + event_type="stellar.analysis.completed", + data={"project_id": 7, "detected": True, "confidence": 0.98}, + ) + + summary = instance.on_event(event) + + assert summary["plugin"] == "stellar-notifier" + assert summary["event"] == "stellar.analysis.completed" + assert summary["project_id"] == 7 + assert summary["detected"] is True + assert "Project 7" in summary["summary"] + assert "Stellar detected" in summary["summary"] + assert len(summary["summary"]) <= example_module.MAX_SUMMARY_CHARS + assert instance.recent_summaries == [summary] + + def test_summary_is_bounded_for_large_payloads(self, app, example_module): + instance = example_module.StellarNotifierPlugin() + event = Event( + event_type="stellar.analysis.completed", + data={ + "project_id": "p" * 500, + "detected": False, + "confidence": "9" * 500, + "network": "n" * 500, + "summary": "s" * 10_000, + }, + ) + + summary = instance.on_event(event) + + assert len(summary["summary"]) <= example_module.MAX_SUMMARY_CHARS + assert len(summary["network"]) <= example_module.MAX_FIELD_CHARS + assert summary["truncated"] is True + + def test_handler_does_not_mutate_the_event_payload(self, app, example_module): + instance = example_module.StellarNotifierPlugin() + payload = {"project_id": 11, "detected": True, "confidence": 0.5} + event = Event(event_type="stellar.analysis.completed", data=payload) + before = copy.deepcopy(event.data) + + instance.on_event(event) + + assert event.data == before + + def test_history_stays_bounded(self, app, example_module): + instance = example_module.StellarNotifierPlugin() + for project_id in range(example_module.MAX_RECENT_SUMMARIES + 15): + instance.on_event( + Event( + event_type="stellar.analysis.completed", + data={"project_id": project_id, "detected": True}, + ) + ) + + assert len(instance.recent_summaries) == example_module.MAX_RECENT_SUMMARIES + assert instance.recent_summaries[0]["project_id"] == 15 + + def test_event_handling_uses_no_network(self, app, example_module, monkeypatch): + def forbidden(*args, **kwargs): + raise AssertionError("the example plugin must stay offline") + + monkeypatch.setattr(socket.socket, "connect", forbidden) + monkeypatch.setattr(socket, "create_connection", forbidden) + + instance = example_module.StellarNotifierPlugin() + summary = instance.on_event( + Event( + event_type="stellar.analysis.completed", + data={"project_id": 3, "detected": True, "confidence": 0.4}, + ) + ) + + assert summary["project_id"] == 3 + + def test_demo_main_runs_offline(self, example_module, capsys): + example_module.main() + + assert "stellar-notifier: Project 1" in capsys.readouterr().out