From 5333ad8aac0ad41c2034c4b13c179989cb7c89d4 Mon Sep 17 00:00:00 2001 From: Juliet Shen Date: Thu, 2 Jul 2026 18:02:33 -0400 Subject: [PATCH 1/6] Add rule-drafts API with pluggable submission backends (GitHub, local) New ui-api blueprint for authoring SML rule drafts from the UI, with submission routed through a pluggable backend so each deployment picks where drafts go for review. Endpoints (all gated by a new CAN_EDIT_RULE_DRAFTS ability, granted to super_user): get-source, validate, vocabulary, submit, pending, and parse-into-builder. Validate and submit splice the draft into the engine's loaded sources and re-run the same AST validation the engine uses; submit re-validates server-side before touching any backend. Backends implement the RuleSubmissionBackend Protocol and are selected by OSPREY_RULES_SUBMISSION_BACKEND: - null (default): fails fast with 503 so an unconfigured install never writes anything - github: opens a PR via the REST API; supports GitHub Enterprise - local: writes into a mounted rules directory Contract and safety details: - SubmissionResult/PendingDraft.to_json spread extras first so a backend-specific extra can't shadow the canonical title/url/ main_sml_updated fields the UI depends on - Forge transport failures (connection refused, timeout) become the structured 502 the UI renders, not an unhandled 500, via a shared _rule_drafts_git_common.request() helper that also holds the branch-name and main.sml Require helpers - main.sml is rejected as a draft path: wholesale-replacing the engine entry point is not a draft; wiring a rule in is the controlled wire_into_main append Adopter docs for the env vars are in docs/user/manage.md. Follow-ups add the rule-editor UI, a GitLab backend, and a Tangled (ATProto) backend. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VZ4RQtuHCCgurfpjfPXXAM --- .gitignore | 3 + docs/user/manage.md | 39 + .../lib/acls/definitions/super_user.json | 4 + .../src/osprey/worker/lib/osprey_engine.py | 8 + .../src/osprey/worker/ui_api/osprey/app.py | 2 + .../worker/ui_api/osprey/lib/abilities.py | 1 + .../osprey/views/_rule_drafts_backend.py | 142 +++ .../osprey/views/_rule_drafts_git_common.py | 62 ++ .../osprey/views/_rule_drafts_github.py | 311 ++++++ .../ui_api/osprey/views/_rule_drafts_local.py | 110 +++ .../ui_api/osprey/views/_rule_drafts_null.py | 41 + .../worker/ui_api/osprey/views/rule_drafts.py | 602 ++++++++++++ .../osprey/views/tests/test_rule_drafts.py | 900 ++++++++++++++++++ 13 files changed, 2225 insertions(+) create mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_backend.py create mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_git_common.py create mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_github.py create mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_local.py create mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_null.py create mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py create mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py diff --git a/.gitignore b/.gitignore index afa35534..fdea2ca3 100644 --- a/.gitignore +++ b/.gitignore @@ -307,5 +307,8 @@ Cargo.lock .claude +# Local docker compose overrides (env vars, port remaps, secrets) +docker-compose.override.yaml + # docs output book/ diff --git a/docs/user/manage.md b/docs/user/manage.md index 36ed4225..d5ef0642 100644 --- a/docs/user/manage.md +++ b/docs/user/manage.md @@ -56,3 +56,42 @@ The list is paginated (50 per page) and can be filtered and sorted: - **Sort**: by name, most referenced, or least referenced Each row shows the rule's name, source file, description, reference count, and line number within the source file. + +## Rule Authoring (Experimental feature) + +Users can draft SML rules directly in the UI. Submit opens a review unit against a configured git remote so authoring, review, and merge use the same tools users already have. + +The editor validates every keystroke against the same AST validator the running engine uses, so compile-time errors surface before the pull request opens. The Rule Builder view expresses the common shape (name, conditions, outcomes) as a form and generates SML; the Code Editor view accepts arbitrary SML for anything the builder can't represent. + +### Rule submission backends + +The Submit button routes drafts through a pluggable backend. Pick one for your deployment by setting `OSPREY_RULES_SUBMISSION_BACKEND` on the `osprey-ui-api` process: + +| Value | What it does | Required env vars | +|---|---|---| +| `null` (default) | Returns 503 on any submit or list call. Ships as the default so an unconfigured install never writes anything. | none | +| `github` | Opens a pull request on a configured repo. Works with github.com and GitHub Enterprise. | `OSPREY_RULES_REPO`, `OSPREY_GITHUB_TOKEN` (+ optionals) | +| `local` | Writes SML directly to a mounted directory. For self-hosted setups whose deploy pipeline already syncs a rules directory into the engine. | `OSPREY_RULES_LOCAL_PATH` | + +Env vars shared across every backend that targets a git host: + +- `OSPREY_RULES_BASE_BRANCH` (default `main`) — the branch the review targets. +- `OSPREY_RULES_PATH_IN_REPO` (default empty) — subdirectory inside the target repo where rule files live, e.g. `example_rules`. Leave empty if rules sit at the repo root. + +#### `github` + +| Var | Default | Notes | +|---|---|---| +| `OSPREY_RULES_REPO` | _required_ | `owner/name` of the repo to PR against. | +| `OSPREY_GITHUB_TOKEN` | _required_ | Fine-grained PAT with `Contents: read/write` and `Pull requests: read/write` on the repo. | +| `OSPREY_GITHUB_API_URL` | `https://api.github.com` | Set for GitHub Enterprise: e.g. `https://github.acme.example/api/v3`. | + +#### `local` + +| Var | Default | Notes | +|---|---|---| +| `OSPREY_RULES_LOCAL_PATH` | _required_ | Absolute path to the directory the backend writes SML into. Must already exist. Submissions take effect immediately; there's no review queue. | + +### Adding a rule submission backend + +Add a Python module next to `_rule_drafts_github.py` that implements the `RuleSubmissionBackend` Protocol defined in `_rule_drafts_backend.py`, then wire it into `load_backend()`. See the module docstring on `_rule_drafts_backend.py` for the contract; the existing HTTP-backed module (`_rule_drafts_github.py`) is a working template. diff --git a/osprey_worker/src/osprey/worker/lib/acls/definitions/super_user.json b/osprey_worker/src/osprey/worker/lib/acls/definitions/super_user.json index c824783c..2ee45783 100644 --- a/osprey_worker/src/osprey/worker/lib/acls/definitions/super_user.json +++ b/osprey_worker/src/osprey/worker/lib/acls/definitions/super_user.json @@ -40,6 +40,10 @@ { "name": "CAN_VIEW_EVENTS_BY_ACTION", "allow_all": true + }, + { + "name": "CAN_EDIT_RULE_DRAFTS", + "allow_all": true } ], "ability_groups": ["CAN_VIEW_BASIC_USER_DATA"] diff --git a/osprey_worker/src/osprey/worker/lib/osprey_engine.py b/osprey_worker/src/osprey/worker/lib/osprey_engine.py index 2e6318d4..45618ae8 100644 --- a/osprey_worker/src/osprey/worker/lib/osprey_engine.py +++ b/osprey_worker/src/osprey/worker/lib/osprey_engine.py @@ -157,6 +157,14 @@ def _handle_updated_sources(self) -> None: def execution_graph(self) -> ExecutionGraph: return self._execution_graph + @property + def udf_registry(self) -> UDFRegistry: + return self._udf_registry + + @property + def validator_registry(self) -> ValidatorRegistry: + return self._validator_registry + @property def config(self) -> SourcesConfig: return self._execution_graph.validated_sources.sources.config diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/app.py b/osprey_worker/src/osprey/worker/ui_api/osprey/app.py index 407462fe..4292b3df 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/app.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/app.py @@ -68,6 +68,7 @@ def create_app() -> Flask: events, features, queries, + rule_drafts, rules, rules_visualizer, saved_queries, @@ -111,6 +112,7 @@ def create_app() -> Flask: _register_with_prefix(app, events.blueprint) _register_with_prefix(app, features.blueprint) _register_with_prefix(app, rules.blueprint) + _register_with_prefix(app, rule_drafts.blueprint) _register_with_prefix(app, queries.blueprint) _register_with_prefix(app, config.blueprint) _register_with_prefix(app, docs.blueprint) diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/lib/abilities.py b/osprey_worker/src/osprey/worker/ui_api/osprey/lib/abilities.py index e56fffd2..3bdf1748 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/lib/abilities.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/lib/abilities.py @@ -535,6 +535,7 @@ def _get_query_filter(self) -> dict[str, Any] | None: CanViewSavedQueries = register_ability('CAN_VIEW_SAVED_QUERIES')(make_marker_ability()) CanCreateAndEditSavedQueries = register_ability('CAN_CREATE_AND_EDIT_SAVED_QUERIES')(make_marker_ability()) CanBulkAction = register_ability('CAN_BULK_ACTION')(make_marker_ability()) +CanEditRuleDrafts = register_ability('CAN_EDIT_RULE_DRAFTS')(make_marker_ability()) def require_ability_with_request(request_model: ModelT, ability_class: Type[Ability[ModelT, ItemT]]) -> None: diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_backend.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_backend.py new file mode 100644 index 00000000..755f1eb3 --- /dev/null +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_backend.py @@ -0,0 +1,142 @@ +"""Backend abstraction for rule-draft submission. + +The Osprey engine doesn't care where rules live. Different deployments use +different hosting: GitHub or Enterprise, GitLab, Tangled, an internal Gerrit, +or a filesystem on a shared volume. Each is one implementation of the +RuleSubmissionBackend Protocol below. + +`load_backend()` reads `OSPREY_RULES_SUBMISSION_BACKEND` and instantiates the +chosen backend with its own env vars. Defaults to `null` so an unconfigured +install ships safe; adopters opt into a backend explicitly. + +Adopter docs (env vars per backend, how to choose one): see +`docs/user/manage.md`. + +Adding a new backend: implement a class with `submit_draft` and +`list_pending_drafts` matching the Protocol below, add a case in +`load_backend()`, and update the "unknown backend" error message here plus +the "no backend configured" message in `_rule_drafts_null.py`. The existing +`_rule_drafts_github.py` module is a working template for HTTP-backed +adapters; `_rule_drafts_local.py` for filesystem. +""" + +from __future__ import annotations + +import os +from dataclasses import dataclass, field +from typing import Any, Protocol + + +class RuleDraftBackendError(Exception): + """Raised by any backend method when the operation cannot complete.""" + + def __init__(self, message: str, status_code: int = 502): + super().__init__(message) + self.message = message + self.status_code = status_code + + +@dataclass(frozen=True) +class SubmissionResult: + """Backend-neutral submit_draft return value. + + `title` and `url` are what the UI surfaces in the success banner; `extras` + carries backend-specific fields (PR number, branch, etc.) for adopters + whose UI variants want to render more detail. + """ + + title: str + url: str | None + main_sml_updated: bool = False + extras: dict[str, Any] = field(default_factory=dict) + + def to_json(self) -> dict[str, Any]: + # Spread extras first so the canonical fields always win: a backend that + # happens to name an extra `title`/`url`/`main_sml_updated` can't shadow + # the contract fields the UI depends on. + return { + **self.extras, + 'title': self.title, + 'url': self.url, + 'main_sml_updated': self.main_sml_updated, + } + + +@dataclass(frozen=True) +class PendingDraft: + """Backend-neutral entry for the pending-drafts list.""" + + title: str + url: str + author: str + created_at: str + touched_files: list[str] + extras: dict[str, Any] = field(default_factory=dict) + + def to_json(self) -> dict[str, Any]: + # Spread extras first so backend-specific keys can't shadow the + # canonical fields the UI depends on. + return { + **self.extras, + 'title': self.title, + 'url': self.url, + 'author': self.author, + 'created_at': self.created_at, + 'touched_files': self.touched_files, + } + + +class RuleSubmissionBackend(Protocol): + """The contract every submission backend implements. + + Implementations: + - submit a draft (create whatever the backend's review unit is) + - optionally wire the new rule into main.sml as part of the same submission + - list whatever's currently in review + + Implementations raise `RuleDraftBackendError` for any failure path. + """ + + name: str + + def submit_draft( + self, + *, + draft_path: str, + sml_source: str, + rule_name: str, + summary: str, + author_email: str, + is_new_rule: bool, + wire_into_main: bool, + ) -> SubmissionResult: ... + + def list_pending_drafts(self) -> list[PendingDraft]: ... + + +def load_backend() -> RuleSubmissionBackend: + """Select and instantiate the configured backend. + + `OSPREY_RULES_SUBMISSION_BACKEND` picks one of: github, local, null. + Unset or empty defaults to `null`. Unknown values raise so a typo doesn't + silently degrade to no-op submission. + """ + name = (os.environ.get('OSPREY_RULES_SUBMISSION_BACKEND') or 'null').strip().lower() + + # Imports are deferred to keep the Protocol module dependency-free. + if name == 'null': + from ._rule_drafts_null import NullBackend + + return NullBackend() + if name == 'github': + from ._rule_drafts_github import GitHubBackend + + return GitHubBackend.from_env() + if name == 'local': + from ._rule_drafts_local import LocalBackend + + return LocalBackend.from_env() + raise RuleDraftBackendError( + f'Unknown OSPREY_RULES_SUBMISSION_BACKEND {name!r}; valid values are github, local, null.', + status_code=500, + ) diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_git_common.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_git_common.py new file mode 100644 index 00000000..5f9ad575 --- /dev/null +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_git_common.py @@ -0,0 +1,62 @@ +"""Shared helpers for git-forge submission backends. + +The GitHub, GitLab, and Tangled adapters all need the same three things: a +cosmetic branch name, a check for whether main.sml already wires a rule in, and +the append that adds the wiring. They also all talk to a remote over HTTP and +must turn a dropped connection into the same structured error the UI renders +rather than an unhandled 500. This module is the one place those live. +""" + +from __future__ import annotations + +import re +import time +from typing import Any + +import requests + +from ._rule_drafts_backend import RuleDraftBackendError + +DEFAULT_TIMEOUT_SECONDS = 15 + + +def request(method: str, url: str, *, error_action: str, **kwargs: Any) -> requests.Response: + """Issue an HTTP request, converting transport failures to RuleDraftBackendError. + + A forge outage (connection refused, DNS failure, timeout) is an expected + operational state for a backend whose job is talking to a remote host, so it + should surface as the 502 JSON shape the editor knows how to display, not as + an unhandled Flask 500. HTTP status errors are left for the caller to map, + since the right status code depends on what was being attempted. + """ + kwargs.setdefault('timeout', DEFAULT_TIMEOUT_SECONDS) + try: + return requests.request(method, url, **kwargs) + except requests.RequestException as exc: + raise RuleDraftBackendError( + f'Could not reach the git host while {error_action}: {exc}', + status_code=502, + ) from exc + + +def generate_branch_name(rule_name: str, author_email: str, *, prefix: str = 'rule-draft') -> str: + """Cosmetic source-branch label. Timestamped so retries don't collide.""" + short_email = author_email.split('@', 1)[0] + slug = re.sub(r'[^A-Za-z0-9_-]+', '-', short_email).strip('-') or 'osprey-ui' + rule_slug = re.sub(r'[^A-Za-z0-9_-]+', '-', rule_name).strip('-') or 'rule' + return f'{prefix}/{slug}/{rule_slug}-{int(time.time())}' + + +def require_already_present(main_sml: str, draft_path: str) -> bool: + pattern = re.compile( + r"Require\s*\(\s*rule\s*=\s*['\"]" + re.escape(draft_path) + r"['\"]\s*\)", + re.MULTILINE, + ) + return bool(pattern.search(main_sml)) + + +def append_require_to_main(main_sml: str, draft_path: str) -> str: + suffix = f"\nRequire(rule='{draft_path}')\n" + if not main_sml.endswith('\n'): + suffix = '\n' + suffix + return main_sml + suffix diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_github.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_github.py new file mode 100644 index 00000000..1e9678e6 --- /dev/null +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_github.py @@ -0,0 +1,311 @@ +"""GitHub submission backend. + +Implements RuleSubmissionBackend by opening a pull request against a configured +repo. Works with github.com and with GitHub Enterprise (set OSPREY_GITHUB_API_URL +to the Enterprise API root, e.g. https://github.mycompany.com/api/v3). +""" + +from __future__ import annotations + +import base64 +import os +from dataclasses import dataclass +from typing import Any + +import requests + +from . import _rule_drafts_git_common as git_common +from ._rule_drafts_backend import ( + PendingDraft, + RuleDraftBackendError, + SubmissionResult, +) + +DEFAULT_GITHUB_API = 'https://api.github.com' + + +@dataclass(frozen=True) +class GitHubConfig: + api_url: str + repo: str + base_branch: str + rules_path: str + token: str + + @property + def repo_url(self) -> str: + return f'{self.api_url.rstrip("/")}/repos/{self.repo}' + + @classmethod + def from_env(cls) -> 'GitHubConfig': + api_url = (os.environ.get('OSPREY_GITHUB_API_URL') or DEFAULT_GITHUB_API).strip() + repo = os.environ.get('OSPREY_RULES_REPO', '').strip() + base = os.environ.get('OSPREY_RULES_BASE_BRANCH', 'main').strip() or 'main' + rules_path = os.environ.get('OSPREY_RULES_PATH_IN_REPO', '').strip().strip('/') + token = os.environ.get('OSPREY_GITHUB_TOKEN', '').strip() + if not repo: + raise RuleDraftBackendError( + 'OSPREY_RULES_REPO is not configured; set it to "owner/name" to enable PR submission.', + status_code=503, + ) + if not token: + raise RuleDraftBackendError( + 'OSPREY_GITHUB_TOKEN is not configured; set a service-account PAT with repo write access.', + status_code=503, + ) + return cls(api_url=api_url, repo=repo, base_branch=base, rules_path=rules_path, token=token) + + +def _headers(cfg: GitHubConfig) -> dict[str, str]: + return { + 'Authorization': f'Bearer {cfg.token}', + 'Accept': 'application/vnd.github+json', + 'X-GitHub-Api-Version': '2022-11-28', + } + + +def _full_path(cfg: GitHubConfig, draft_path: str) -> str: + draft_path = draft_path.lstrip('/') + if cfg.rules_path: + return f'{cfg.rules_path}/{draft_path}' + return draft_path + + +def _get(cfg: GitHubConfig, url: str, **kwargs: Any) -> requests.Response: + return git_common.request('GET', url, error_action='contacting GitHub', headers=_headers(cfg), **kwargs) + + +def _post(cfg: GitHubConfig, url: str, json: dict[str, Any]) -> requests.Response: + return git_common.request('POST', url, error_action='contacting GitHub', headers=_headers(cfg), json=json) + + +def _put(cfg: GitHubConfig, url: str, json: dict[str, Any]) -> requests.Response: + return git_common.request('PUT', url, error_action='contacting GitHub', headers=_headers(cfg), json=json) + + +def _raise_for_github(response: requests.Response, action: str) -> None: + if response.ok: + return + body = response.text[:500] + # 4xx from GitHub is still a backend failure from the caller's POV. + raise RuleDraftBackendError( + f'GitHub returned {response.status_code} while {action}: {body}', + status_code=502, + ) + + +def _get_base_sha(cfg: GitHubConfig) -> str: + res = _get(cfg, f'{cfg.repo_url}/git/ref/heads/{cfg.base_branch}') + _raise_for_github(res, f'looking up base branch {cfg.base_branch!r}') + return res.json()['object']['sha'] + + +def _get_file_sha(cfg: GitHubConfig, path_in_repo: str, ref: str) -> str | None: + res = _get(cfg, f'{cfg.repo_url}/contents/{path_in_repo}', params={'ref': ref}) + if res.status_code == 404: + return None + _raise_for_github(res, f'reading {path_in_repo!r} on {ref!r}') + payload = res.json() + if isinstance(payload, list): + return None + return payload.get('sha') + + +def _create_branch(cfg: GitHubConfig, branch: str, sha: str) -> None: + res = _post(cfg, f'{cfg.repo_url}/git/refs', json={'ref': f'refs/heads/{branch}', 'sha': sha}) + if res.status_code == 422: + raise RuleDraftBackendError( + f'Branch {branch!r} already exists on {cfg.repo}. Pick a different name.', + status_code=409, + ) + _raise_for_github(res, f'creating branch {branch!r}') + + +def _commit_file( + cfg: GitHubConfig, + branch: str, + path_in_repo: str, + contents: str, + message: str, + existing_sha: str | None, +) -> None: + payload: dict[str, Any] = { + 'message': message, + 'content': base64.b64encode(contents.encode('utf-8')).decode('ascii'), + 'branch': branch, + } + if existing_sha is not None: + payload['sha'] = existing_sha + res = _put(cfg, f'{cfg.repo_url}/contents/{path_in_repo}', json=payload) + _raise_for_github(res, f'committing {path_in_repo!r} to {branch!r}') + + +def _open_pr(cfg: GitHubConfig, branch: str, title: str, body: str) -> dict[str, Any]: + res = _post( + cfg, + f'{cfg.repo_url}/pulls', + json={'title': title, 'head': branch, 'base': cfg.base_branch, 'body': body}, + ) + _raise_for_github(res, f'opening PR from {branch!r}') + return res.json() + + +def _main_sml_path(cfg: GitHubConfig) -> str: + return _full_path(cfg, 'main.sml') + + +def _fetch_file_on_ref(cfg: GitHubConfig, path_in_repo: str, ref: str) -> tuple[str, str] | None: + res = _get(cfg, f'{cfg.repo_url}/contents/{path_in_repo}', params={'ref': ref}) + if res.status_code == 404: + return None + _raise_for_github(res, f'reading {path_in_repo!r} on {ref!r}') + payload = res.json() + if isinstance(payload, list): + return None + encoded = payload.get('content', '') + sha = payload.get('sha') + if sha is None: + return None + try: + decoded = base64.b64decode(encoded).decode('utf-8') + except Exception as exc: + raise RuleDraftBackendError(f'could not decode {path_in_repo!r}: {exc}', status_code=502) + return decoded, sha + + +class GitHubBackend: + name = 'github' + + def __init__(self, cfg: GitHubConfig): + self._cfg = cfg + + @classmethod + def from_env(cls) -> 'GitHubBackend': + return cls(GitHubConfig.from_env()) + + def submit_draft( + self, + *, + draft_path: str, + sml_source: str, + rule_name: str, + summary: str, + author_email: str, + is_new_rule: bool, + wire_into_main: bool, + ) -> SubmissionResult: + cfg = self._cfg + path_in_repo = _full_path(cfg, draft_path) + base_sha = _get_base_sha(cfg) + existing_sha = _get_file_sha(cfg, path_in_repo, cfg.base_branch) + + if is_new_rule and existing_sha is not None: + raise RuleDraftBackendError( + f'A file already exists at {path_in_repo!r} on {cfg.base_branch}. ' + 'Pick a different filename, or edit the existing rule instead of creating a new one.', + status_code=409, + ) + + branch = git_common.generate_branch_name(rule_name, author_email) + _create_branch(cfg, branch, base_sha) + + verb = 'Add' if is_new_rule else 'Update' + commit_message = f'{verb} rule {rule_name}\n\nAuthored via Osprey UI by {author_email}.' + _commit_file( + cfg, + branch=branch, + path_in_repo=path_in_repo, + contents=sml_source, + message=commit_message, + existing_sha=existing_sha, + ) + + main_sml_updated = False + if wire_into_main: + main_path = _main_sml_path(cfg) + fetched = _fetch_file_on_ref(cfg, main_path, cfg.base_branch) + if fetched is None: + raise RuleDraftBackendError( + f'wire_into_main requested but {main_path!r} does not exist on {cfg.base_branch}.', + status_code=409, + ) + main_contents, main_sha = fetched + if not git_common.require_already_present(main_contents, draft_path): + new_main = git_common.append_require_to_main(main_contents, draft_path) + _commit_file( + cfg, + branch=branch, + path_in_repo=main_path, + contents=new_main, + message=f'Wire {rule_name} into main.sml\n\nAuthored via Osprey UI by {author_email}.', + existing_sha=main_sha, + ) + main_sml_updated = True + + title = f'{verb} rule {rule_name}' + touched = f'`{path_in_repo}`' + if main_sml_updated: + touched += f', `{_main_sml_path(cfg)}`' + body = ( + f'{summary.strip() or "_(no summary provided)_"}\n\n' + f'---\n' + f'Drafted in the Osprey rules UI by `{author_email}`.\n' + f'Touches: {touched}.' + ) + pr = _open_pr(cfg, branch=branch, title=title, body=body) + pr_number = pr.get('number') + pr_url = pr.get('html_url') + return SubmissionResult( + title=f'Pull request #{pr_number} opened', + url=pr_url, + main_sml_updated=main_sml_updated, + extras={ + 'pr_number': pr_number, + 'pr_url': pr_url, + 'branch': branch, + 'path_in_repo': path_in_repo, + }, + ) + + def list_pending_drafts(self) -> list[PendingDraft]: + cfg = self._cfg + res = _get( + cfg, + f'{cfg.repo_url}/pulls', + params={'state': 'open', 'base': cfg.base_branch, 'per_page': 30}, + ) + _raise_for_github(res, 'listing open pull requests') + open_prs = res.json() + + out: list[PendingDraft] = [] + for pr in open_prs: + number = pr.get('number') + if number is None: + continue + files_res = _get(cfg, f'{cfg.repo_url}/pulls/{number}/files', params={'per_page': 50}) + if not files_res.ok: + continue + files = files_res.json() + touched = [ + f['filename'] + for f in files + if isinstance(f.get('filename'), str) + and (not cfg.rules_path or f['filename'].startswith(cfg.rules_path + '/')) + and f['filename'].endswith('.sml') + ] + if not touched: + continue + out.append( + PendingDraft( + title=pr.get('title', ''), + url=pr.get('html_url', ''), + author=(pr.get('user') or {}).get('login', ''), + created_at=pr.get('created_at', ''), + touched_files=touched, + extras={ + 'pr_number': number, + 'branch': (pr.get('head') or {}).get('ref', ''), + }, + ) + ) + return out diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_local.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_local.py new file mode 100644 index 00000000..2d815eb6 --- /dev/null +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_local.py @@ -0,0 +1,110 @@ +"""Filesystem submission backend for self-hosted setups. + +Writes the SML straight to a configured rules directory. No review, no PR. +Adopters whose deploy pipeline already syncs a rules directory into the engine +(etcd push, file watcher, etc.) wire this backend up so the UI drops the SML +in the right place and lets the downstream pipeline take it from there. +""" + +from __future__ import annotations + +import os +from dataclasses import dataclass +from pathlib import Path + +from . import _rule_drafts_git_common as git_common +from ._rule_drafts_backend import ( + PendingDraft, + RuleDraftBackendError, + SubmissionResult, +) + + +@dataclass(frozen=True) +class LocalConfig: + rules_dir: Path + + @classmethod + def from_env(cls) -> 'LocalConfig': + raw = os.environ.get('OSPREY_RULES_LOCAL_PATH', '').strip() + if not raw: + raise RuleDraftBackendError( + 'OSPREY_RULES_LOCAL_PATH is not configured; set it to the directory the local backend should write to.', + status_code=503, + ) + path = Path(raw) + if not path.is_dir(): + raise RuleDraftBackendError( + f'OSPREY_RULES_LOCAL_PATH {raw!r} is not a directory.', + status_code=503, + ) + return cls(rules_dir=path) + + +class LocalBackend: + name = 'local' + + def __init__(self, cfg: LocalConfig): + self._cfg = cfg + + @classmethod + def from_env(cls) -> 'LocalBackend': + return cls(LocalConfig.from_env()) + + def _resolve(self, draft_path: str) -> Path: + """Resolve a draft path within rules_dir, refusing anything that would + escape via `..` or symlink traversal.""" + candidate = (self._cfg.rules_dir / draft_path).resolve() + try: + candidate.relative_to(self._cfg.rules_dir.resolve()) + except ValueError as exc: + raise RuleDraftBackendError( + f'Draft path {draft_path!r} escapes the configured rules directory.', + status_code=400, + ) from exc + return candidate + + def submit_draft( + self, + *, + draft_path: str, + sml_source: str, + rule_name: str, + summary: str, + author_email: str, + is_new_rule: bool, + wire_into_main: bool, + ) -> SubmissionResult: + target = self._resolve(draft_path) + if is_new_rule and target.exists(): + raise RuleDraftBackendError( + f'A file already exists at {draft_path!r}. Pick a different filename or edit the existing rule.', + status_code=409, + ) + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(sml_source, encoding='utf-8') + + main_sml_updated = False + if wire_into_main: + main_path = self._cfg.rules_dir / 'main.sml' + if not main_path.exists(): + raise RuleDraftBackendError( + f'wire_into_main requested but main.sml does not exist at {main_path}.', + status_code=409, + ) + main_contents = main_path.read_text(encoding='utf-8') + if not git_common.require_already_present(main_contents, draft_path): + main_path.write_text(git_common.append_require_to_main(main_contents, draft_path), encoding='utf-8') + main_sml_updated = True + + verb = 'Created' if is_new_rule else 'Updated' + return SubmissionResult( + title=f'{verb} {draft_path} in local rules directory', + url=None, + main_sml_updated=main_sml_updated, + extras={'path_on_disk': str(target)}, + ) + + def list_pending_drafts(self) -> list[PendingDraft]: + # Local backend has no review queue; submissions take effect immediately. + return [] diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_null.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_null.py new file mode 100644 index 00000000..8ee303ab --- /dev/null +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_null.py @@ -0,0 +1,41 @@ +"""Default submission backend: nothing is configured, so every call fails fast. + +Ships as the default so an unconfigured upstream install never opens a PR or +writes a file without an adopter explicitly opting into a backend. +""" + +from __future__ import annotations + +from ._rule_drafts_backend import ( + PendingDraft, + RuleDraftBackendError, + SubmissionResult, +) + + +class NullBackend: + name = 'null' + + @staticmethod + def _err() -> RuleDraftBackendError: + return RuleDraftBackendError( + 'No rule-submission backend is configured. Set OSPREY_RULES_SUBMISSION_BACKEND ' + 'to one of: github, local. See docs for what each one needs.', + status_code=503, + ) + + def submit_draft( + self, + *, + draft_path: str, + sml_source: str, + rule_name: str, + summary: str, + author_email: str, + is_new_rule: bool, + wire_into_main: bool, + ) -> SubmissionResult: + raise self._err() + + def list_pending_drafts(self) -> list[PendingDraft]: + raise self._err() diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py new file mode 100644 index 00000000..588a9942 --- /dev/null +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py @@ -0,0 +1,602 @@ +from __future__ import annotations + +import logging +import re +from collections.abc import Iterable +from typing import Any + +from flask import Blueprint, jsonify, request +from osprey.engine.ast.error_utils import SpanWithHint +from osprey.engine.ast.grammar import ( + Assign, + BinaryComparison, + Call, + FormatString, + Name, + Not, + Number, + Source, + String, + UnaryOperation, +) +from osprey.engine.ast.grammar import ( + List as AstList, +) +from osprey.engine.ast.sources import Sources +from osprey.engine.ast_validator import validate_sources +from osprey.engine.ast_validator.validation_context import ( + ValidationError, + ValidationFailed, + ValidationWarning, +) +from osprey.worker.lib.singletons import ENGINE +from osprey.worker.ui_api.osprey.lib.abilities import CanEditRuleDrafts, require_ability +from osprey.worker.ui_api.osprey.lib.auth import get_current_user_email + +from . import _rule_drafts_backend as backends +from ._engine_ast_utils import get_func_identifier + +logger = logging.getLogger(__name__) + +blueprint = Blueprint('rule_drafts', __name__) + +_VALID_PATH = re.compile(r'^[A-Za-z0-9_./-]+\.sml$') +_VALID_RULE_NAME = re.compile(r'^[A-Za-z_][A-Za-z0-9_]*$') + + +def _format_validation_message(msg: ValidationError | ValidationWarning) -> dict[str, Any]: + identifier: str | None = None + try: + node = msg.span.ast_node + if isinstance(node, Name): + identifier = node.identifier + except Exception: + pass + + defined_in: list[str] = [] + for additional in msg.additional_spans: + span = additional.span if isinstance(additional, SpanWithHint) else additional + defined_in.append(span.source.path) + + return { + 'message': msg.message, + 'hint': msg.hint, + 'source_path': msg.source.path, + 'line': msg.span.start_line, + 'column': msg.span.start_pos, + 'rendered': msg.rendered(), + 'identifier': identifier, + 'defined_in_source_paths': defined_in, + } + + +def _suggest_imports_from_errors( + draft_path: str, + errors: list[dict[str, Any]], +) -> list[str]: + """Collect source paths the draft references but doesn't import. + + Pulled from each error's `defined_in_source_paths`. main.sml is the engine + entry point and is never importable; the draft can't import itself either. + """ + suggested: set[str] = set() + for err in errors: + for path in err.get('defined_in_source_paths') or []: + if path == 'main.sml' or path == draft_path: + continue + suggested.add(path) + return sorted(suggested) + + +def _validate_path(path: str) -> str | None: + if not _VALID_PATH.match(path): + return f'Path {path!r} is not a valid SML source path (must end .sml and contain only [A-Za-z0-9_./-]).' + if '..' in path.split('/'): + return f'Path {path!r} contains a parent-directory segment.' + return None + + +def _current_sources_dict() -> dict[str, str]: + engine = ENGINE.instance() + return engine.execution_graph.validated_sources.sources.to_dict() + + +@blueprint.route('/rule-drafts/source', methods=['GET']) +@require_ability(CanEditRuleDrafts) +def get_source() -> Any: + path = request.args.get('path', '').strip() + err = _validate_path(path) + if err: + return jsonify({'error': err}), 400 + + engine = ENGINE.instance() + source: Source | None = engine.execution_graph.validated_sources.sources.get_by_path(path) + if source is None: + return jsonify({'error': f'No source found at {path!r}.'}), 404 + return jsonify({'path': source.path, 'contents': source.contents}) + + +@blueprint.route('/rule-drafts/validate', methods=['POST']) +@require_ability(CanEditRuleDrafts) +def validate_draft() -> Any: + """Splice the draft into the engine's sources and re-run AST validation. + + A 200 with `{ok: false, errors: [...]}` means the SML failed validation; the + response is still JSON so the editor can render structured errors inline. + A 400 means the request itself was malformed (bad path, missing source). + """ + payload = request.get_json(silent=True) or {} + path = (payload.get('path') or '').strip() + source_text = payload.get('source', '') + + path_err = _validate_path(path) + if path_err: + return jsonify({'error': path_err}), 400 + if not isinstance(source_text, str): + return jsonify({'error': 'source must be a string.'}), 400 + + spliced = _current_sources_dict() + spliced[path] = source_text + + try: + sources = Sources.from_dict(spliced) + except Exception as exc: + # Sources.from_dict asserts on shape (e.g., missing main.sml). Surface as a structured error + # so the editor can show "you broke main.sml" without crashing. + return jsonify( + { + 'ok': False, + 'errors': [{'message': str(exc), 'hint': '', 'source_path': path, 'line': 0, 'column': 0}], + 'warnings': [], + } + ), 400 + + engine = ENGINE.instance() + try: + validated = validate_sources( + sources, + udf_registry=engine.udf_registry, + validator_registry=engine.validator_registry, + ) + except ValidationFailed as exc: + formatted_errors = [_format_validation_message(e) for e in exc.errors] + return jsonify( + { + 'ok': False, + 'errors': formatted_errors, + 'warnings': [_format_validation_message(w) for w in exc.warnings], + 'suggested_imports': _suggest_imports_from_errors(path, formatted_errors), + } + ) + + return jsonify( + { + 'ok': True, + 'errors': [], + 'warnings': [_format_validation_message(w) for w in validated.warnings], + 'suggested_imports': [], + } + ) + + +def _iter_top_level_assigns(sources: Iterable[Source]) -> Iterable[tuple[Source, Assign]]: + for source in sources: + for statement in source.ast_root.statements: + if isinstance(statement, Assign): + yield source, statement + + +def _is_rule_call(node: Any) -> bool: + return isinstance(node, Call) and get_func_identifier(node) == 'Rule' + + +def _collect_features(sources: Iterable[Source]) -> list[dict[str, Any]]: + out: list[dict[str, Any]] = [] + seen: set[str] = set() + for source, assign in _iter_top_level_assigns(sources): + name = assign.target.identifier + if name in seen: + continue + # Skip `MyRule = Rule(...)` assigns; the builder dropdown is for values + # a user can reference inside conditions, not for rule definitions. + if _is_rule_call(assign.value): + continue + seen.add(name) + out.append( + { + 'name': name, + 'source_path': source.path, + 'source_line': assign.span.start_line, + } + ) + out.sort(key=lambda item: item['name']) + return out + + +def _collect_udfs() -> list[dict[str, Any]]: + engine = ENGINE.instance() + udf_registry = engine.udf_registry + out: list[dict[str, Any]] = [] + for func in sorted(udf_registry.iter_functions(), key=lambda f: f.__name__): + try: + args_type = func.get_arguments_type() + rvalue_type = func.get_rvalue_type() + except Exception: + continue + arguments: list[dict[str, Any]] = [] + try: + items = args_type.items().items() + except Exception: + items = [] + for arg_name, arg_type in items: + arguments.append( + { + 'name': arg_name, + 'type_name': getattr(arg_type, '__name__', str(arg_type)), + } + ) + out.append( + { + 'name': func.__name__, + 'return_type': getattr(rvalue_type, '__name__', str(rvalue_type)), + 'arguments': arguments, + } + ) + return out + + +def _collect_effects(sources: Iterable[Source]) -> list[str]: + """Names of UDFs that appear inside a `WhenRules(then=[...])` block. + + Used as the effect dropdown. Sourced from real usage rather than from the + UDF registry because the registry holds every UDF and we want a shortlist + of things users actually use as actions. + """ + seen: set[str] = set() + + def _walk(node: Any) -> None: + if isinstance(node, Call): + ident = get_func_identifier(node) + if ident: + seen.add(ident) + for arg in node.arguments: + _walk(arg.value) + elif isinstance(node, AstList): + for item in node.items: + _walk(item) + elif isinstance(node, Assign): + _walk(node.value) + + for source in sources: + for statement in source.ast_root.statements: + call_node: Call | None = None + if isinstance(statement, Call) and get_func_identifier(statement) == 'WhenRules': + call_node = statement + elif ( + isinstance(statement, Assign) + and isinstance(statement.value, Call) + and get_func_identifier(statement.value) == 'WhenRules' + ): + call_node = statement.value + if call_node is None: + continue + then_arg = call_node.find_argument('then') + if then_arg is None: + continue + _walk(then_arg.value) + + return sorted(seen) + + +@blueprint.route('/rule-drafts/vocabulary', methods=['GET']) +@require_ability(CanEditRuleDrafts) +def vocabulary() -> Any: + engine = ENGINE.instance() + sources = list(engine.execution_graph.validated_sources.sources) + features = _collect_features(sources) + udfs = _collect_udfs() + effects = _collect_effects(sources) + source_files = sorted(s.path for s in sources) + return jsonify( + { + 'features': features, + 'udfs': udfs, + 'effects': effects, + 'source_files': source_files, + } + ) + + +@blueprint.route('/rule-drafts/submit', methods=['POST']) +@require_ability(CanEditRuleDrafts) +def submit_draft() -> Any: + payload = request.get_json(silent=True) or {} + path = (payload.get('path') or '').strip() + source_text = payload.get('source', '') + rule_name = (payload.get('rule_name') or '').strip() + summary = (payload.get('summary') or '').strip() + is_new_rule = bool(payload.get('is_new_rule', False)) + wire_into_main = bool(payload.get('wire_into_main', False)) + + path_err = _validate_path(path) + if path_err: + return jsonify({'error': path_err}), 400 + if path == 'main.sml': + # main.sml is the engine entry point. Submitting a draft *as* main.sml + # would wholesale-replace it (immediate live effect on the local + # backend). Wiring a rule in is a controlled one-line append handled by + # the wire_into_main option, not a draft submission. + return jsonify( + { + 'error': 'main.sml is the engine entry point and cannot be submitted as a draft. ' + 'Use "turn this rule on" to add a Require line instead.' + } + ), 400 + if not isinstance(source_text, str) or not source_text.strip(): + return jsonify({'error': 'source must be a non-empty string.'}), 400 + if not _VALID_RULE_NAME.match(rule_name): + return jsonify({'error': 'rule_name must be a valid SML identifier ([A-Za-z_][A-Za-z0-9_]*).'}), 400 + + # Re-validate server-side so a client that skips the validate step still cannot push uncompilable SML. + spliced = _current_sources_dict() + spliced[path] = source_text + try: + sources = Sources.from_dict(spliced) + validate_sources( + sources, + udf_registry=ENGINE.instance().udf_registry, + validator_registry=ENGINE.instance().validator_registry, + ) + except ValidationFailed as exc: + return jsonify( + { + 'error': 'Validation failed; fix errors before submitting.', + 'errors': [_format_validation_message(e) for e in exc.errors], + } + ), 400 + except Exception as exc: + return jsonify({'error': f'Could not assemble sources: {exc}'}), 400 + + try: + backend = backends.load_backend() + result = backend.submit_draft( + draft_path=path, + sml_source=source_text, + rule_name=rule_name, + summary=summary, + author_email=get_current_user_email(), + is_new_rule=is_new_rule, + wire_into_main=wire_into_main, + ) + except backends.RuleDraftBackendError as exc: + return jsonify({'error': exc.message}), exc.status_code + + return jsonify(result.to_json()) + + +@blueprint.route('/rule-drafts/pending', methods=['GET']) +@require_ability(CanEditRuleDrafts) +def pending_drafts() -> Any: + try: + backend = backends.load_backend() + drafts = backend.list_pending_drafts() + except backends.RuleDraftBackendError as exc: + return jsonify({'error': exc.message, 'pending': []}), exc.status_code + return jsonify({'pending': [d.to_json() for d in drafts]}) + + +# The set of comparator strings the Rule Builder UI can render. +_BUILDER_COMPARATORS = {'==', '!=', '>', '<', '>=', '<='} + + +def _condition_from_value(node: Any, feature: str, operator: str) -> dict[str, Any] | None: + """Build a builder Condition row from an RHS node, returning None if the node + isn't a literal or a bare Name (the only RHS shapes the builder supports).""" + if isinstance(node, Name): + return {'feature': feature, 'operator': operator, 'rhs': node.identifier, 'rhsIsFeature': True} + if isinstance(node, String): + return {'feature': feature, 'operator': operator, 'rhs': node.value, 'rhsIsFeature': False} + if isinstance(node, Number): + return {'feature': feature, 'operator': operator, 'rhs': str(node.value), 'rhsIsFeature': False} + return None + + +def _parse_text_contains_call(call: Call, operator: str) -> dict[str, Any] | None: + """Convert a `TextContains(text=Name, phrase=...)` call to an includes/excludes row. + + Returns None if the call doesn't have the exact shape the builder emits. + """ + if get_func_identifier(call) != 'TextContains': + return None + text_arg = call.find_argument('text') + phrase_arg = call.find_argument('phrase') + if text_arg is None or phrase_arg is None: + return None + if not isinstance(text_arg.value, Name): + return None + feature = text_arg.value.identifier + return _condition_from_value(phrase_arg.value, feature, operator) + + +def _parse_condition(node: Any) -> dict[str, Any] | None: + if isinstance(node, UnaryOperation) and isinstance(node.operator, Not) and isinstance(node.operand, Call): + return _parse_text_contains_call(node.operand, 'excludes') + if isinstance(node, Call): + return _parse_text_contains_call(node, 'includes') + if isinstance(node, BinaryComparison): + if not isinstance(node.left, Name): + return None + operator = node.comparator.original_comparator + if operator not in _BUILDER_COMPARATORS: + return None + return _condition_from_value(node.right, node.left.identifier, operator) + return None + + +def _parse_outcome_arg(arg: Any) -> dict[str, Any] | None: + """Convert one `Call.arguments[i]` into a builder OutcomeArg, or None for + anything richer than a literal or bare Name reference.""" + val = arg.value + if isinstance(val, Name): + return {'name': arg.name, 'value': val.identifier, 'isFeature': True} + if isinstance(val, String): + return {'name': arg.name, 'value': val.value, 'isFeature': False} + if isinstance(val, Number): + return {'name': arg.name, 'value': str(val.value), 'isFeature': False} + return None + + +def _parse_outcome(node: Any) -> dict[str, Any] | None: + if not isinstance(node, Call): + return None + effect = get_func_identifier(node) + if effect is None: + return None + args: list[dict[str, Any]] = [] + for arg in node.arguments: + parsed = _parse_outcome_arg(arg) + if parsed is None: + return None + args.append(parsed) + return {'effect': effect, 'args': args} + + +def _parse_into_builder_model(source: Source) -> dict[str, Any]: + """Walk the AST of a single draft Source and either return a populated + builder model JSON or `{supported: False, reason: ...}`. + + The builder's expressible subset is deliberately narrow: optional Import + and Require statements (ignored for the model), exactly one + `RuleName = Rule(when_all=[...], description='...')`, and an optional + `WhenRules(rules_any=[RuleName], then=[...])` whose `then` entries are + UDF calls with literal or Name arguments. Anything richer means the file + can't round-trip and the user must use Code Editor. + """ + try: + statements = source.ast_root.statements + except Exception as exc: + return {'supported': False, 'reason': f'could not parse SML: {exc}'} + + rule_assign: Assign | None = None + when_rules_call: Call | None = None + + for stmt in statements: + if isinstance(stmt, Call): + ident = get_func_identifier(stmt) + if ident in ('Import', 'Require'): + continue + if ident == 'WhenRules': + if when_rules_call is not None: + return { + 'supported': False, + 'reason': 'multiple WhenRules blocks; Rule Builder edits one rule at a time', + } + when_rules_call = stmt + continue + return {'supported': False, 'reason': f'top-level call to `{ident}` is not supported by Rule Builder'} + if isinstance(stmt, Assign) and isinstance(stmt.value, Call) and get_func_identifier(stmt.value) == 'Rule': + if rule_assign is not None: + return { + 'supported': False, + 'reason': 'multiple Rule definitions in one file; Rule Builder edits one rule at a time', + } + rule_assign = stmt + continue + if isinstance(stmt, Assign): + return { + 'supported': False, + 'reason': f'helper assignment `{stmt.target.identifier} = ...` is not supported by Rule Builder', + } + return {'supported': False, 'reason': f'unsupported top-level statement: {type(stmt).__name__}'} + + if rule_assign is None: + return {'supported': False, 'reason': 'no Rule(...) definition found in this file'} + + rule_name = rule_assign.target.identifier + rule_call = rule_assign.value + assert isinstance(rule_call, Call) + + description = '' + description_arg = rule_call.find_argument('description') + if description_arg is not None: + if isinstance(description_arg.value, String): + description = description_arg.value.value + elif isinstance(description_arg.value, FormatString): + # Round-trip the raw template; the builder doesn't expose format-string editing. + description = description_arg.value.format_string + else: + return {'supported': False, 'reason': 'rule description must be a string literal'} + + when_all_arg = rule_call.find_argument('when_all') + if when_all_arg is None or not isinstance(when_all_arg.value, AstList): + return {'supported': False, 'reason': 'Rule must have `when_all=[...]`'} + + conditions: list[dict[str, Any]] = [] + for item in when_all_arg.value.items: + cond = _parse_condition(item) + if cond is None: + return { + 'supported': False, + 'reason': 'one or more conditions use expressions Rule Builder cannot represent', + } + conditions.append(cond) + if not conditions: + # Builder needs at least one row to render anything sensible; matching the EMPTY_BUILDER_MODEL default. + conditions = [{'feature': '', 'operator': '==', 'rhs': '', 'rhsIsFeature': False}] + + outcomes: list[dict[str, Any]] = [] + if when_rules_call is not None: + rules_any_arg = when_rules_call.find_argument('rules_any') + if rules_any_arg is not None and isinstance(rules_any_arg.value, AstList): + for item in rules_any_arg.value.items: + if not isinstance(item, Name) or item.identifier != rule_name: + return { + 'supported': False, + 'reason': 'WhenRules.rules_any must reference only the rule being edited', + } + then_arg = when_rules_call.find_argument('then') + if then_arg is not None and isinstance(then_arg.value, AstList): + for item in then_arg.value.items: + outcome = _parse_outcome(item) + if outcome is None: + return { + 'supported': False, + 'reason': 'one or more outcomes use expressions Rule Builder cannot represent', + } + outcomes.append(outcome) + if not outcomes: + outcomes = [{'effect': '', 'args': []}] + + return { + 'supported': True, + 'model': { + 'ruleName': rule_name, + 'description': description, + 'conditions': conditions, + 'outcomes': outcomes, + }, + } + + +@blueprint.route('/rule-drafts/parse-into-builder', methods=['POST']) +@require_ability(CanEditRuleDrafts) +def parse_into_builder() -> Any: + """Attempt to render an existing SML file as a Rule Builder model. + + Returns `{supported: true, model: {...}}` if the file fits the builder's + expressible subset, or `{supported: false, reason: "..."}` otherwise. The + UI uses this to decide whether to enable the Rule Builder toggle when + editing an existing rule. + """ + payload = request.get_json(silent=True) or {} + path = (payload.get('path') or '').strip() + source_text = payload.get('source', '') + + path_err = _validate_path(path) + if path_err: + return jsonify({'error': path_err}), 400 + if not isinstance(source_text, str): + return jsonify({'error': 'source must be a string.'}), 400 + + source = Source(path=path, contents=source_text) + return jsonify(_parse_into_builder_model(source)) diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py new file mode 100644 index 00000000..dbe18e43 --- /dev/null +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py @@ -0,0 +1,900 @@ +import base64 +import json +import tempfile +from pathlib import Path +from unittest.mock import patch + +import pytest +import requests +import requests_mock as requests_mock_module +from flask import Response, url_for +from flask.testing import FlaskClient +from osprey.worker.lib.snowflake import Snowflake + + +@pytest.fixture(autouse=True) +def _mock_audit_snowflake(): + # The after_request audit hook mints a snowflake id, which normally means an + # HTTP call to the snowflake-id-worker service. Tests that wrap a request in + # a requests_mock.Mocker would otherwise trip NoMockAddress on that call, so + # neutralize it here (the audit log's persist() is already mocked in the + # shared conftest). + with patch('osprey.worker.ui_api.osprey.lib.audit.generate_snowflake', return_value=Snowflake(1)): + yield + + +def _set_github_backend(monkeypatch: pytest.MonkeyPatch, **overrides: str) -> None: + """Configure the github backend with sensible defaults for tests; overrides win.""" + defaults = { + 'OSPREY_RULES_SUBMISSION_BACKEND': 'github', + 'OSPREY_RULES_REPO': 'roostorg/osprey-rules', + 'OSPREY_GITHUB_TOKEN': 'gh_fake_token', + 'OSPREY_RULES_BASE_BRANCH': 'main', + } + for k, v in {**defaults, **overrides}.items(): + monkeypatch.setenv(k, v) + + +_acl_with_draft_ability = json.dumps( + { + 'ui_config': {}, + 'labels': {}, + 'acl': { + 'users': { + 'local-dev@localhost': { + 'abilities': [ + {'name': 'CAN_VIEW_DOCS', 'allow_all': True}, + {'name': 'CAN_EDIT_RULE_DRAFTS', 'allow_all': True}, + ], + }, + }, + }, + } +) + +_acl_without_draft_ability = json.dumps( + { + 'ui_config': {}, + 'labels': {}, + 'acl': { + 'users': { + 'local-dev@localhost': {'abilities': [{'name': 'CAN_VIEW_DOCS', 'allow_all': True}]}, + }, + }, + } +) + +_base_sources = { + 'config.yaml': _acl_with_draft_ability, + 'models/base.sml': """ + UserId: str = JsonData(path='$.user_id') + PostText: str = JsonData(path='$.post_text') + """, + 'main.sml': """ + Import(rules=['models/base.sml']) + + ContainsHello = Rule( + when_all=[PostText == 'hello'], + description='Post contains hello', + ) + + WhenRules( + rules_any=[ContainsHello], + then=[DeclareVerdict(verdict=UserId)], + ) + """, +} + + +@pytest.mark.use_rules_sources(_base_sources) +def test_get_source_returns_contents(client: 'FlaskClient[Response]') -> None: + res = client.get(url_for('rule_drafts.get_source'), query_string={'path': 'main.sml'}) + assert res.status_code == 200 + assert res.json is not None + assert res.json['path'] == 'main.sml' + assert 'ContainsHello' in res.json['contents'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_get_source_rejects_bad_path(client: 'FlaskClient[Response]') -> None: + res = client.get(url_for('rule_drafts.get_source'), query_string={'path': '../etc/passwd.sml'}) + assert res.status_code == 400 + + +@pytest.mark.use_rules_sources(_base_sources) +def test_get_source_404_for_unknown_path(client: 'FlaskClient[Response]') -> None: + res = client.get(url_for('rule_drafts.get_source'), query_string={'path': 'rules/does_not_exist.sml'}) + assert res.status_code == 404 + + +@pytest.mark.use_rules_sources( + { + 'config.yaml': _acl_without_draft_ability, + 'main.sml': "UserId: str = JsonData(path='$.user_id')", + } +) +def test_endpoints_require_can_edit_rule_drafts(client: 'FlaskClient[Response]') -> None: + res = client.get(url_for('rule_drafts.get_source'), query_string={'path': 'main.sml'}) + assert res.status_code == 401 + res = client.post( + url_for('rule_drafts.validate_draft'), + json={'path': 'rules/x.sml', 'source': ''}, + ) + assert res.status_code == 401 + res = client.post( + url_for('rule_drafts.parse_into_builder'), + json={'path': 'rules/x.sml', 'source': ''}, + ) + assert res.status_code == 401 + res = client.get(url_for('rule_drafts.vocabulary')) + assert res.status_code == 401 + res = client.get(url_for('rule_drafts.pending_drafts')) + assert res.status_code == 401 + + +@pytest.mark.use_rules_sources(_base_sources) +def test_validate_clean_draft_returns_ok(client: 'FlaskClient[Response]') -> None: + res = client.post( + url_for('rule_drafts.validate_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + }, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['ok'] is True + assert body['errors'] == [] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_validate_broken_draft_returns_structured_errors(client: 'FlaskClient[Response]') -> None: + res = client.post( + url_for('rule_drafts.validate_draft'), + json={ + 'path': 'rules/broken.sml', + 'source': 'this is not valid SML at all *** !!!', + }, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['ok'] is False + assert len(body['errors']) >= 1 + err = body['errors'][0] + assert set(err.keys()) >= {'message', 'hint', 'source_path', 'line', 'column', 'rendered'} + + +@pytest.mark.use_rules_sources( + { + 'config.yaml': _acl_with_draft_ability, + 'main.sml': "Import(rules=['models/post.sml'])", + 'models/post.sml': "PostText: str = JsonData(path='$.post_text')", + } +) +def test_validate_returns_suggested_imports_for_unimported_identifier(client: 'FlaskClient[Response]') -> None: + res = client.post( + url_for('rule_drafts.validate_draft'), + json={ + 'path': 'rules/uses_post_text.sml', + 'source': "MyRule = Rule(when_all=[PostText == 'hi'], description='hi')", + }, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['ok'] is False + assert body['suggested_imports'] == ['models/post.sml'] + assert any(e.get('identifier') == 'PostText' for e in body['errors']) + + +@pytest.mark.use_rules_sources(_base_sources) +def test_validate_clean_draft_has_empty_suggested_imports(client: 'FlaskClient[Response]') -> None: + res = client.post( + url_for('rule_drafts.validate_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + }, + ) + assert res.status_code == 200 + assert res.json is not None + assert res.json['suggested_imports'] == [] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_validate_rejects_bad_path(client: 'FlaskClient[Response]') -> None: + res = client.post( + url_for('rule_drafts.validate_draft'), + json={'path': 'rules/x.txt', 'source': ''}, + ) + assert res.status_code == 400 + + +@pytest.mark.use_rules_sources(_base_sources) +def test_vocabulary_returns_features_udfs_effects(client: 'FlaskClient[Response]') -> None: + res = client.get(url_for('rule_drafts.vocabulary')) + assert res.status_code == 200 + body = res.json + assert body is not None + assert set(body.keys()) == {'features', 'udfs', 'effects', 'source_files'} + + feature_names = {f['name'] for f in body['features']} + assert {'UserId', 'PostText'}.issubset(feature_names) + assert 'ContainsHello' not in feature_names + + udf_names = {u['name'] for u in body['udfs']} + assert 'JsonData' in udf_names + assert 'Rule' in udf_names + assert 'DeclareVerdict' in body['effects'] + + assert 'main.sml' in body['source_files'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_returns_503_when_no_backend_configured( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.delenv('OSPREY_RULES_SUBMISSION_BACKEND', raising=False) + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': 'demo', + 'is_new_rule': True, + }, + ) + assert res.status_code == 503 + body = res.json + assert body is not None + assert 'No rule-submission backend is configured' in body['error'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_returns_503_when_github_missing_required_env( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'github') + monkeypatch.delenv('OSPREY_RULES_REPO', raising=False) + monkeypatch.delenv('OSPREY_GITHUB_TOKEN', raising=False) + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': 'demo', + 'is_new_rule': True, + }, + ) + assert res.status_code == 503 + body = res.json + assert body is not None + assert 'OSPREY_RULES_REPO' in body['error'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_blocks_invalid_sml_before_calling_github( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + _set_github_backend(monkeypatch) + + with requests_mock_module.Mocker() as m: + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/bad.sml', + 'source': 'this is not valid SML !!!', + 'rule_name': 'BadRule', + 'summary': 'should not submit', + 'is_new_rule': True, + }, + ) + + assert res.status_code == 400 + assert m.call_count == 0 + body = res.json + assert body is not None + assert body['error'].startswith('Validation failed') + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_rejects_bad_rule_name(client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch) -> None: + _set_github_backend(monkeypatch) + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/new.sml', + 'source': "X = Rule(when_all=[PostText == 'x'], description='x')", + 'rule_name': '1-not-an-identifier', + 'summary': '', + 'is_new_rule': True, + }, + ) + assert res.status_code == 400 + + +def test_submission_result_extras_cannot_shadow_canonical_fields() -> None: + from osprey.worker.ui_api.osprey.views._rule_drafts_backend import SubmissionResult + + result = SubmissionResult( + title='real title', + url='https://real.example/pr/1', + extras={'title': 'spoofed', 'url': 'https://evil.example', 'pr_number': 7}, + ) + out = result.to_json() + assert out['title'] == 'real title' + assert out['url'] == 'https://real.example/pr/1' + assert out['main_sml_updated'] is False + # Non-colliding extras still pass through for adopters that want them. + assert out['pr_number'] == 7 + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_rejects_main_sml_as_draft_path( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + _set_github_backend(monkeypatch) + with requests_mock_module.Mocker() as m: + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'main.sml', + 'source': 'Import(rules=[])', + 'rule_name': 'Whatever', + 'summary': '', + 'is_new_rule': False, + }, + ) + # Guard fires before any network call: the entry point is never a draft. + assert m.call_count == 0 + assert res.status_code == 400 + body = res.json + assert body is not None + assert 'main.sml' in body['error'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_surfaces_github_connection_error_as_502( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + _set_github_backend(monkeypatch) + repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' + with requests_mock_module.Mocker() as m: + m.get(f'{repo_url}/git/ref/heads/main', exc=requests.exceptions.ConnectTimeout) + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': '', + 'is_new_rule': True, + }, + ) + # A forge outage is a structured 502, not an unhandled 500. + assert res.status_code == 502 + body = res.json + assert body is not None + assert 'Could not reach the git host' in body['error'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_happy_path_creates_branch_commits_and_opens_pr( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + _set_github_backend(monkeypatch, OSPREY_RULES_PATH_IN_REPO='rules') + + repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' + + with requests_mock_module.Mocker() as m: + m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) + m.get(f'{repo_url}/contents/rules/new_rule.sml', status_code=404) + m.post(f'{repo_url}/git/refs', status_code=201, json={}) + m.put(f'{repo_url}/contents/rules/new_rule.sml', status_code=201, json={}) + m.post( + f'{repo_url}/pulls', + status_code=201, + json={'number': 42, 'html_url': 'https://github.com/roostorg/osprey-rules/pull/42'}, + ) + + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + # Bare filename: OSPREY_RULES_PATH_IN_REPO='rules' prepends the + # subdirectory, so the file lands at rules/new_rule.sml. + 'path': 'new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': 'add bye rule', + 'is_new_rule': True, + }, + ) + + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['title'] == 'Pull request #42 opened' + assert body['url'] == 'https://github.com/roostorg/osprey-rules/pull/42' + assert body['main_sml_updated'] is False + # GitHub-specific extras are surfaced for adopters that want them. + assert body['pr_number'] == 42 + assert body['pr_url'] == 'https://github.com/roostorg/osprey-rules/pull/42' + assert body['path_in_repo'] == 'rules/new_rule.sml' + assert body['branch'].startswith('rule-draft/local-dev/AnotherRule-') + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_wire_into_main_appends_require_line( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + _set_github_backend(monkeypatch) + + repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' + existing_main = "Import(rules=['models/post.sml'])\n\nRequire(rule='rules/post_contains_hello.sml')\n" + encoded_main = base64.b64encode(existing_main.encode('utf-8')).decode('ascii') + + with requests_mock_module.Mocker() as m: + m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) + m.get(f'{repo_url}/contents/rules/new_rule.sml', status_code=404) + m.post(f'{repo_url}/git/refs', status_code=201, json={}) + m.put(f'{repo_url}/contents/rules/new_rule.sml', status_code=201, json={}) + m.get(f'{repo_url}/contents/main.sml', json={'sha': 'MAIN_SHA', 'content': encoded_main, 'type': 'file'}) + main_put = m.put(f'{repo_url}/contents/main.sml', status_code=200, json={}) + m.post( + f'{repo_url}/pulls', + status_code=201, + json={'number': 99, 'html_url': 'https://github.com/roostorg/osprey-rules/pull/99'}, + ) + + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': 'add bye rule and wire it in', + 'is_new_rule': True, + 'wire_into_main': True, + }, + ) + + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['main_sml_updated'] is True + + main_put_request = main_put.last_request + assert main_put_request is not None + posted = main_put_request.json() + decoded_new_main = base64.b64decode(posted['content']).decode('utf-8') + assert "Require(rule='rules/new_rule.sml')" in decoded_new_main + # The existing Require for post_contains_hello.sml should still be present untouched. + assert "Require(rule='rules/post_contains_hello.sml')" in decoded_new_main + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_wire_into_main_skips_when_require_already_present( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + _set_github_backend(monkeypatch) + + repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' + existing_main = "Require(rule='rules/already_here.sml')\n" + encoded_main = base64.b64encode(existing_main.encode('utf-8')).decode('ascii') + + with requests_mock_module.Mocker() as m: + m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) + m.get(f'{repo_url}/contents/rules/already_here.sml', status_code=404) + m.post(f'{repo_url}/git/refs', status_code=201, json={}) + m.put(f'{repo_url}/contents/rules/already_here.sml', status_code=201, json={}) + m.get(f'{repo_url}/contents/main.sml', json={'sha': 'MAIN_SHA', 'content': encoded_main, 'type': 'file'}) + main_put = m.put(f'{repo_url}/contents/main.sml', status_code=200, json={}) + m.post( + f'{repo_url}/pulls', + status_code=201, + json={'number': 100, 'html_url': 'https://github.com/roostorg/osprey-rules/pull/100'}, + ) + + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/already_here.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': '', + 'is_new_rule': True, + 'wire_into_main': True, + }, + ) + + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['main_sml_updated'] is False + # main.sml fetch happens but the PUT to update it must not. + assert main_put.call_count == 0 + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_409_if_new_rule_file_already_exists( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + _set_github_backend(monkeypatch) + + repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' + + with requests_mock_module.Mocker() as m: + m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) + m.get( + f'{repo_url}/contents/new_rule.sml', + json={'sha': 'EXISTING_BLOB_SHA', 'type': 'file'}, + ) + + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': '', + 'is_new_rule': True, + }, + ) + + assert res.status_code == 409 + + +@pytest.mark.use_rules_sources(_base_sources) +def test_pending_filters_to_rules_path_and_sml( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + _set_github_backend(monkeypatch, OSPREY_RULES_PATH_IN_REPO='rules') + + repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' + + with requests_mock_module.Mocker() as m: + m.get( + f'{repo_url}/pulls', + json=[ + { + 'number': 1, + 'title': 'Add rule X', + 'html_url': 'https://github.com/roostorg/osprey-rules/pull/1', + 'head': {'ref': 'rule-draft/local-dev/X-1'}, + 'user': {'login': 'someone'}, + 'created_at': '2026-06-30T12:00:00Z', + }, + { + 'number': 2, + 'title': 'Update README', + 'html_url': 'https://github.com/roostorg/osprey-rules/pull/2', + 'head': {'ref': 'docs/readme'}, + 'user': {'login': 'someone'}, + 'created_at': '2026-06-30T13:00:00Z', + }, + ], + ) + m.get( + f'{repo_url}/pulls/1/files', + json=[{'filename': 'rules/x.sml'}], + ) + m.get( + f'{repo_url}/pulls/2/files', + json=[{'filename': 'README.md'}], + ) + + res = client.get(url_for('rule_drafts.pending_drafts')) + + assert res.status_code == 200 + body = res.json + assert body is not None + assert len(body['pending']) == 1 + entry = body['pending'][0] + assert entry['title'] == 'Add rule X' + assert entry['url'] == 'https://github.com/roostorg/osprey-rules/pull/1' + assert entry['touched_files'] == ['rules/x.sml'] + # GitHub-specific extras carried through for adopters that want them. + assert entry['pr_number'] == 1 + + +@pytest.mark.use_rules_sources(_base_sources) +def test_parse_into_builder_round_trips_a_builder_shaped_rule(client: 'FlaskClient[Response]') -> None: + source = """ +Import(rules=['models/post.sml']) + +ContainsCat = Rule( + when_all=[ + TextContains(text=PostText, phrase='cat'), + EventType == 'create_post', + ], + description='looks for cat', +) + +WhenRules( + rules_any=[ContainsCat], + then=[ + LabelAdd(entity=UserId, label='meow'), + ], +) +""" + res = client.post( + url_for('rule_drafts.parse_into_builder'), + json={'path': 'rules/contains_cat.sml', 'source': source}, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['supported'] is True + model = body['model'] + assert model['ruleName'] == 'ContainsCat' + assert model['description'] == 'looks for cat' + assert model['conditions'] == [ + {'feature': 'PostText', 'operator': 'includes', 'rhs': 'cat', 'rhsIsFeature': False}, + {'feature': 'EventType', 'operator': '==', 'rhs': 'create_post', 'rhsIsFeature': False}, + ] + assert model['outcomes'] == [ + { + 'effect': 'LabelAdd', + 'args': [ + {'name': 'entity', 'value': 'UserId', 'isFeature': True}, + {'name': 'label', 'value': 'meow', 'isFeature': False}, + ], + } + ] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_parse_into_builder_handles_excludes(client: 'FlaskClient[Response]') -> None: + source = "BlocksCat = Rule(when_all=[not TextContains(text=PostText, phrase='cat')], description='no cat')\n" + res = client.post( + url_for('rule_drafts.parse_into_builder'), + json={'path': 'rules/blocks_cat.sml', 'source': source}, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['supported'] is True + assert body['model']['conditions'] == [ + {'feature': 'PostText', 'operator': 'excludes', 'rhs': 'cat', 'rhsIsFeature': False}, + ] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_parse_into_builder_rejects_multiple_rules(client: 'FlaskClient[Response]') -> None: + source = ( + "A = Rule(when_all=[PostText == 'a'], description='a')\nB = Rule(when_all=[PostText == 'b'], description='b')\n" + ) + res = client.post( + url_for('rule_drafts.parse_into_builder'), + json={'path': 'rules/multi.sml', 'source': source}, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['supported'] is False + assert 'multiple Rule definitions' in body['reason'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_parse_into_builder_rejects_helper_assigns(client: 'FlaskClient[Response]') -> None: + source = "Helper = 'cat'\nA = Rule(when_all=[PostText == Helper], description='a')\n" + res = client.post( + url_for('rule_drafts.parse_into_builder'), + json={'path': 'rules/helper.sml', 'source': source}, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['supported'] is False + assert 'helper assignment' in body['reason'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_parse_into_builder_rejects_complex_condition(client: 'FlaskClient[Response]') -> None: + # A boolean operator inside `when_all` would need Code Editor; the builder is AND-only via row repetition. + source = "A = Rule(when_all=[PostText == 'a' and EventType == 'create_post'], description='a')\n" + res = client.post( + url_for('rule_drafts.parse_into_builder'), + json={'path': 'rules/complex.sml', 'source': source}, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['supported'] is False + assert 'Rule Builder cannot represent' in body['reason'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_parse_into_builder_rejects_file_with_no_rule(client: 'FlaskClient[Response]') -> None: + source = "Import(rules=['models/post.sml'])\n" + res = client.post( + url_for('rule_drafts.parse_into_builder'), + json={'path': 'rules/empty.sml', 'source': source}, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['supported'] is False + assert 'no Rule(...)' in body['reason'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_parse_into_builder_rejects_syntax_error(client: 'FlaskClient[Response]') -> None: + res = client.post( + url_for('rule_drafts.parse_into_builder'), + json={'path': 'rules/broken.sml', 'source': 'this is not valid SML at all !!!'}, + ) + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['supported'] is False + assert 'could not parse SML' in body['reason'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_unknown_backend_value_returns_500(client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'gerrit') + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': '', + 'is_new_rule': True, + }, + ) + assert res.status_code == 500 + body = res.json + assert body is not None + assert "Unknown OSPREY_RULES_SUBMISSION_BACKEND 'gerrit'" in body['error'] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_pending_returns_empty_list_for_null_backend( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.delenv('OSPREY_RULES_SUBMISSION_BACKEND', raising=False) + res = client.get(url_for('rule_drafts.pending_drafts')) + assert res.status_code == 503 + body = res.json + assert body is not None + assert body['pending'] == [] + + +@pytest.mark.use_rules_sources(_base_sources) +def test_submit_github_enterprise_url_is_threaded_into_requests( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + """GitHub Enterprise customers point OSPREY_GITHUB_API_URL at their own host; every API call must use it.""" + _set_github_backend( + monkeypatch, + OSPREY_GITHUB_API_URL='https://github.acme.test/api/v3', + OSPREY_RULES_REPO='acme/rules', + ) + + repo_url = 'https://github.acme.test/api/v3/repos/acme/rules' + + with requests_mock_module.Mocker() as m: + m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) + m.get(f'{repo_url}/contents/rules/new_rule.sml', status_code=404) + m.post(f'{repo_url}/git/refs', status_code=201, json={}) + m.put(f'{repo_url}/contents/rules/new_rule.sml', status_code=201, json={}) + m.post( + f'{repo_url}/pulls', + status_code=201, + json={'number': 5, 'html_url': 'https://github.acme.test/acme/rules/pull/5'}, + ) + + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': '', + 'is_new_rule': True, + }, + ) + + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['url'] == 'https://github.acme.test/acme/rules/pull/5' + # If any call had hit api.github.com instead, the Mocker would have raised NoMockAddress. + + +@pytest.mark.use_rules_sources(_base_sources) +def test_local_backend_writes_file_and_returns_no_url( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + rules_dir = Path(tmpdir) + (rules_dir / 'main.sml').write_text("Import(rules=['models/post.sml'])\n", encoding='utf-8') + + monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'local') + monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(rules_dir)) + + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': 'add bye rule', + 'is_new_rule': True, + 'wire_into_main': False, + }, + ) + + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['url'] is None + assert 'rules/new_rule.sml' in body['title'] + assert body['main_sml_updated'] is False + + written = (rules_dir / 'rules' / 'new_rule.sml').read_text(encoding='utf-8') + assert 'AnotherRule' in written + + +@pytest.mark.use_rules_sources(_base_sources) +def test_local_backend_wires_into_main_when_requested( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + rules_dir = Path(tmpdir) + (rules_dir / 'main.sml').write_text("Import(rules=['models/post.sml'])\n", encoding='utf-8') + + monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'local') + monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(rules_dir)) + + res = client.post( + url_for('rule_drafts.submit_draft'), + json={ + 'path': 'rules/new_rule.sml', + 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", + 'rule_name': 'AnotherRule', + 'summary': '', + 'is_new_rule': True, + 'wire_into_main': True, + }, + ) + + assert res.status_code == 200 + body = res.json + assert body is not None + assert body['main_sml_updated'] is True + updated_main = (rules_dir / 'main.sml').read_text(encoding='utf-8') + assert "Require(rule='rules/new_rule.sml')" in updated_main + + +@pytest.mark.use_rules_sources(_base_sources) +def test_local_backend_rejects_path_traversal(client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + rules_dir = Path(tmpdir) + monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'local') + monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(rules_dir)) + + # The view layer already rejects '..' in the path, so to actually exercise the + # local backend's own guard we bypass the view-level check by going around the + # API: instantiate the backend and call submit_draft directly with a traversal path. + from osprey.worker.ui_api.osprey.views._rule_drafts_backend import RuleDraftBackendError + from osprey.worker.ui_api.osprey.views._rule_drafts_local import LocalBackend, LocalConfig + + backend = LocalBackend(LocalConfig(rules_dir=rules_dir)) + with pytest.raises(RuleDraftBackendError) as exc_info: + backend.submit_draft( + draft_path='../escape.sml', + sml_source='x = 1', + rule_name='X', + summary='', + author_email='test@local', + is_new_rule=True, + wire_into_main=False, + ) + assert 'escapes the configured rules directory' in exc_info.value.message From 8a483fb143778f60240a6df2e7fd8be92d55e0df Mon Sep 17 00:00:00 2001 From: Juliet Shen Date: Mon, 13 Jul 2026 21:52:07 -0400 Subject: [PATCH 2/6] Replace rule-submission backends with a draft rules table Feedback on #402 was to drop the github/gitlab/etc submission backends and instead keep drafts in Osprey itself. This does that: rule drafts now live in a rule_drafts Postgres table that the people who operate Osprey can reference, edit, and deploy from, with no external code host. - Add the RuleDraft model (one row per rule path, upserted on save) and register it so the table is created. - Rework the view around the table: create/list/get plus a deploy endpoint that re-validates, writes the SML into OSPREY_RULES_LOCAL_PATH, and optionally wires a Require line into main.sml. Keep the backend-agnostic authoring endpoints (source, validate, vocabulary, parse-into-builder). - Remove the five _rule_drafts_* backend modules and the OSPREY_RULES_SUBMISSION_BACKEND config surface. - Rewrite the tests for the table workflow; update docs and CHANGELOG. A DB-backed SourcesProvider that loads deployed drafts straight from the table (removing the filesystem hand-off) is noted as future direction. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X --- CHANGELOG.md | 1 + docs/user/manage.md | 38 +- .../src/osprey/worker/lib/storage/postgres.py | 1 + .../osprey/worker/lib/storage/rule_drafts.py | 110 +++ .../osprey/views/_rule_drafts_backend.py | 142 ---- .../osprey/views/_rule_drafts_git_common.py | 62 -- .../osprey/views/_rule_drafts_github.py | 311 ------- .../ui_api/osprey/views/_rule_drafts_local.py | 110 --- .../ui_api/osprey/views/_rule_drafts_null.py | 41 - .../worker/ui_api/osprey/views/rule_drafts.py | 202 +++-- .../osprey/views/tests/test_rule_drafts.py | 766 ++++-------------- 11 files changed, 422 insertions(+), 1362 deletions(-) create mode 100644 osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py delete mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_backend.py delete mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_git_common.py delete mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_github.py delete mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_local.py delete mode 100644 osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_null.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0715b425..10473d2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ For more information about each release including git tags and artifacts, see [R - Per-action health metrics in the executor ([#191](https://github.com/roostorg/osprey/pull/191) by [@cmttt](https://github.com/cmttt)) - Option to suppress cached errors to reduce metric bloat ([#180](https://github.com/roostorg/osprey/pull/180) by [@lithium-powered](https://github.com/lithium-powered)) - Experimental asyncio-native worker with metrics and engine/coordinator improvements ([#341](https://github.com/roostorg/osprey/pull/341) by [@cmttt](https://github.com/cmttt)) +- Experimental in-app rule authoring: draft SML rules validate against the live engine, save to a `rule_drafts` table, and deploy into the configured rules directory ([#402](https://github.com/roostorg/osprey/pull/402) by [@julietshen](https://github.com/julietshen)) ### Changed diff --git a/docs/user/manage.md b/docs/user/manage.md index d5ef0642..4d1c54ea 100644 --- a/docs/user/manage.md +++ b/docs/user/manage.md @@ -59,39 +59,25 @@ Each row shows the rule's name, source file, description, reference count, and l ## Rule Authoring (Experimental feature) -Users can draft SML rules directly in the UI. Submit opens a review unit against a configured git remote so authoring, review, and merge use the same tools users already have. +Users can draft SML rules directly in the UI. Drafts are saved to a `rule_drafts` table so the people who operate Osprey can reference, edit, and deploy them without any external code host. -The editor validates every keystroke against the same AST validator the running engine uses, so compile-time errors surface before the pull request opens. The Rule Builder view expresses the common shape (name, conditions, outcomes) as a form and generates SML; the Code Editor view accepts arbitrary SML for anything the builder can't represent. +The editor validates every keystroke against the same AST validator the running engine uses, so compile-time errors surface before a draft is saved. The Rule Builder view expresses the common shape (name, conditions, outcomes) as a form and generates SML; the Code Editor view accepts arbitrary SML for anything the builder can't represent. -### Rule submission backends +### The draft rules table -The Submit button routes drafts through a pluggable backend. Pick one for your deployment by setting `OSPREY_RULES_SUBMISSION_BACKEND` on the `osprey-ui-api` process: +Drafts live in a Postgres table (`rule_drafts`), one row per rule file path. The API (all gated by the `CAN_EDIT_RULE_DRAFTS` ability, granted to `super_user`): -| Value | What it does | Required env vars | -|---|---|---| -| `null` (default) | Returns 503 on any submit or list call. Ships as the default so an unconfigured install never writes anything. | none | -| `github` | Opens a pull request on a configured repo. Works with github.com and GitHub Enterprise. | `OSPREY_RULES_REPO`, `OSPREY_GITHUB_TOKEN` (+ optionals) | -| `local` | Writes SML directly to a mounted directory. For self-hosted setups whose deploy pipeline already syncs a rules directory into the engine. | `OSPREY_RULES_LOCAL_PATH` | - -Env vars shared across every backend that targets a git host: +- `POST /rule-drafts` — re-validates the SML server-side, then upserts the draft. +- `GET /rule-drafts` — lists every draft (the table operators work from). +- `GET /rule-drafts/` — fetches a single draft. +- `POST /rule-drafts//deploy` — re-validates, writes the SML into the rules directory, and marks the draft `deployed`. Pass `wire_into_main: true` to also append a `Require(rule=...)` line to `main.sml` so the rule takes effect (a rule file is inert until something requires it). -- `OSPREY_RULES_BASE_BRANCH` (default `main`) — the branch the review targets. -- `OSPREY_RULES_PATH_IN_REPO` (default empty) — subdirectory inside the target repo where rule files live, e.g. `example_rules`. Leave empty if rules sit at the repo root. +### Deploying -#### `github` +Deploy writes the draft's SML into a rules directory that the engine's sources provider already loads (a filesystem hand-off: whatever pipeline syncs that directory activates the rule). | Var | Default | Notes | |---|---|---| -| `OSPREY_RULES_REPO` | _required_ | `owner/name` of the repo to PR against. | -| `OSPREY_GITHUB_TOKEN` | _required_ | Fine-grained PAT with `Contents: read/write` and `Pull requests: read/write` on the repo. | -| `OSPREY_GITHUB_API_URL` | `https://api.github.com` | Set for GitHub Enterprise: e.g. `https://github.acme.example/api/v3`. | - -#### `local` - -| Var | Default | Notes | -|---|---|---| -| `OSPREY_RULES_LOCAL_PATH` | _required_ | Absolute path to the directory the backend writes SML into. Must already exist. Submissions take effect immediately; there's no review queue. | - -### Adding a rule submission backend +| `OSPREY_RULES_LOCAL_PATH` | _required for deploy_ | Absolute path to the rules directory the engine loads. Deploy writes SML here; must already exist. If unset, `POST /deploy` returns 503 (drafting and validation still work). | -Add a Python module next to `_rule_drafts_github.py` that implements the `RuleSubmissionBackend` Protocol defined in `_rule_drafts_backend.py`, then wire it into `load_backend()`. See the module docstring on `_rule_drafts_backend.py` for the contract; the existing HTTP-backed module (`_rule_drafts_github.py`) is a working template. +> **Future direction:** a DB-backed `SourcesProvider` could let the engine load deployed drafts straight from the `rule_drafts` table, removing the filesystem hand-off and making rule management work with zero external infrastructure. This PR keeps the filesystem deploy; the table is already the source of truth for drafts. diff --git a/osprey_worker/src/osprey/worker/lib/storage/postgres.py b/osprey_worker/src/osprey/worker/lib/storage/postgres.py index b0f67045..fb302e1b 100644 --- a/osprey_worker/src/osprey/worker/lib/storage/postgres.py +++ b/osprey_worker/src/osprey/worker/lib/storage/postgres.py @@ -56,6 +56,7 @@ def _init(config: Config) -> None: bulk_label_task, pg_stored_execution, queries, + rule_drafts, temporary_ability_token, ) diff --git a/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py b/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py new file mode 100644 index 00000000..ac520021 --- /dev/null +++ b/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py @@ -0,0 +1,110 @@ +from __future__ import annotations + +from datetime import datetime, timezone +from enum import StrEnum + +from sqlalchemy import BigInteger, Column, DateTime, Enum, Text + +from .postgres import Model, scoped_session + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +class RuleDraftStatus(StrEnum): + DRAFT = 'draft' + DEPLOYED = 'deployed' + + +class RuleDraft(Model): + """A staged SML rule draft. + + Drafts are authored and validated in the UI and live here so the people who + operate Osprey can reference, edit, and deploy them without any external code + host. Deploying writes the SML into the configured rules directory; the draft + row stays as the record of what was deployed. + + One row per rule `path` (upserted on submit), so the table reads as the + current set of drafts rather than an append-only history. + """ + + __tablename__ = 'rule_drafts' + + id: int = Column(BigInteger, primary_key=True, autoincrement=True) + path: str = Column(Text, nullable=False, unique=True) + rule_name: str = Column(Text, nullable=False) + sml_source: str = Column(Text, nullable=False) + summary: str = Column(Text, nullable=False, default='') + author_email: str = Column(Text, nullable=False) + status: RuleDraftStatus = Column( + Enum(RuleDraftStatus, native_enum=False, length=32), + nullable=False, + default=RuleDraftStatus.DRAFT, + ) + created_at: datetime = Column(DateTime(timezone=True), nullable=False, default=_now) + updated_at: datetime = Column(DateTime(timezone=True), nullable=False, default=_now, onupdate=_now) + deployed_at: datetime | None = Column(DateTime(timezone=True), nullable=True) # type: ignore[misc] + + def to_json(self) -> dict[str, object]: + return { + 'id': self.id, + 'path': self.path, + 'rule_name': self.rule_name, + 'source': self.sml_source, + 'summary': self.summary, + 'author': self.author_email, + 'status': str(self.status), + 'created_at': self.created_at.isoformat() if self.created_at else None, + 'updated_at': self.updated_at.isoformat() if self.updated_at else None, + 'deployed_at': self.deployed_at.isoformat() if self.deployed_at else None, + } + + @classmethod + def upsert(cls, *, path: str, rule_name: str, sml_source: str, summary: str, author_email: str) -> 'RuleDraft': + """Create the draft for `path`, or update it in place if one already exists. + + Editing a deployed draft moves it back to `DRAFT` so the table reflects + that the in-flight SML no longer matches what was last deployed. + """ + with scoped_session(commit=True) as session: + draft = session.query(cls).filter(cls.path == path).first() + if draft is None: + draft = cls(path=path) + session.add(draft) + draft.rule_name = rule_name + draft.sml_source = sml_source + draft.summary = summary + draft.author_email = author_email + draft.status = RuleDraftStatus.DRAFT + session.flush() + session.expunge(draft) + return draft + + @classmethod + def list_all(cls) -> list['RuleDraft']: + with scoped_session() as session: + drafts = session.query(cls).order_by(cls.updated_at.desc()).all() + for draft in drafts: + session.expunge(draft) + return drafts + + @classmethod + def get_one(cls, draft_id: int) -> 'RuleDraft | None': + with scoped_session() as session: + draft = session.query(cls).filter(cls.id == draft_id).first() + if draft is not None: + session.expunge(draft) + return draft + + @classmethod + def mark_deployed(cls, draft_id: int) -> 'RuleDraft | None': + with scoped_session(commit=True) as session: + draft = session.query(cls).filter(cls.id == draft_id).first() + if draft is None: + return None + draft.status = RuleDraftStatus.DEPLOYED + draft.deployed_at = _now() + session.flush() + session.expunge(draft) + return draft diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_backend.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_backend.py deleted file mode 100644 index 755f1eb3..00000000 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_backend.py +++ /dev/null @@ -1,142 +0,0 @@ -"""Backend abstraction for rule-draft submission. - -The Osprey engine doesn't care where rules live. Different deployments use -different hosting: GitHub or Enterprise, GitLab, Tangled, an internal Gerrit, -or a filesystem on a shared volume. Each is one implementation of the -RuleSubmissionBackend Protocol below. - -`load_backend()` reads `OSPREY_RULES_SUBMISSION_BACKEND` and instantiates the -chosen backend with its own env vars. Defaults to `null` so an unconfigured -install ships safe; adopters opt into a backend explicitly. - -Adopter docs (env vars per backend, how to choose one): see -`docs/user/manage.md`. - -Adding a new backend: implement a class with `submit_draft` and -`list_pending_drafts` matching the Protocol below, add a case in -`load_backend()`, and update the "unknown backend" error message here plus -the "no backend configured" message in `_rule_drafts_null.py`. The existing -`_rule_drafts_github.py` module is a working template for HTTP-backed -adapters; `_rule_drafts_local.py` for filesystem. -""" - -from __future__ import annotations - -import os -from dataclasses import dataclass, field -from typing import Any, Protocol - - -class RuleDraftBackendError(Exception): - """Raised by any backend method when the operation cannot complete.""" - - def __init__(self, message: str, status_code: int = 502): - super().__init__(message) - self.message = message - self.status_code = status_code - - -@dataclass(frozen=True) -class SubmissionResult: - """Backend-neutral submit_draft return value. - - `title` and `url` are what the UI surfaces in the success banner; `extras` - carries backend-specific fields (PR number, branch, etc.) for adopters - whose UI variants want to render more detail. - """ - - title: str - url: str | None - main_sml_updated: bool = False - extras: dict[str, Any] = field(default_factory=dict) - - def to_json(self) -> dict[str, Any]: - # Spread extras first so the canonical fields always win: a backend that - # happens to name an extra `title`/`url`/`main_sml_updated` can't shadow - # the contract fields the UI depends on. - return { - **self.extras, - 'title': self.title, - 'url': self.url, - 'main_sml_updated': self.main_sml_updated, - } - - -@dataclass(frozen=True) -class PendingDraft: - """Backend-neutral entry for the pending-drafts list.""" - - title: str - url: str - author: str - created_at: str - touched_files: list[str] - extras: dict[str, Any] = field(default_factory=dict) - - def to_json(self) -> dict[str, Any]: - # Spread extras first so backend-specific keys can't shadow the - # canonical fields the UI depends on. - return { - **self.extras, - 'title': self.title, - 'url': self.url, - 'author': self.author, - 'created_at': self.created_at, - 'touched_files': self.touched_files, - } - - -class RuleSubmissionBackend(Protocol): - """The contract every submission backend implements. - - Implementations: - - submit a draft (create whatever the backend's review unit is) - - optionally wire the new rule into main.sml as part of the same submission - - list whatever's currently in review - - Implementations raise `RuleDraftBackendError` for any failure path. - """ - - name: str - - def submit_draft( - self, - *, - draft_path: str, - sml_source: str, - rule_name: str, - summary: str, - author_email: str, - is_new_rule: bool, - wire_into_main: bool, - ) -> SubmissionResult: ... - - def list_pending_drafts(self) -> list[PendingDraft]: ... - - -def load_backend() -> RuleSubmissionBackend: - """Select and instantiate the configured backend. - - `OSPREY_RULES_SUBMISSION_BACKEND` picks one of: github, local, null. - Unset or empty defaults to `null`. Unknown values raise so a typo doesn't - silently degrade to no-op submission. - """ - name = (os.environ.get('OSPREY_RULES_SUBMISSION_BACKEND') or 'null').strip().lower() - - # Imports are deferred to keep the Protocol module dependency-free. - if name == 'null': - from ._rule_drafts_null import NullBackend - - return NullBackend() - if name == 'github': - from ._rule_drafts_github import GitHubBackend - - return GitHubBackend.from_env() - if name == 'local': - from ._rule_drafts_local import LocalBackend - - return LocalBackend.from_env() - raise RuleDraftBackendError( - f'Unknown OSPREY_RULES_SUBMISSION_BACKEND {name!r}; valid values are github, local, null.', - status_code=500, - ) diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_git_common.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_git_common.py deleted file mode 100644 index 5f9ad575..00000000 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_git_common.py +++ /dev/null @@ -1,62 +0,0 @@ -"""Shared helpers for git-forge submission backends. - -The GitHub, GitLab, and Tangled adapters all need the same three things: a -cosmetic branch name, a check for whether main.sml already wires a rule in, and -the append that adds the wiring. They also all talk to a remote over HTTP and -must turn a dropped connection into the same structured error the UI renders -rather than an unhandled 500. This module is the one place those live. -""" - -from __future__ import annotations - -import re -import time -from typing import Any - -import requests - -from ._rule_drafts_backend import RuleDraftBackendError - -DEFAULT_TIMEOUT_SECONDS = 15 - - -def request(method: str, url: str, *, error_action: str, **kwargs: Any) -> requests.Response: - """Issue an HTTP request, converting transport failures to RuleDraftBackendError. - - A forge outage (connection refused, DNS failure, timeout) is an expected - operational state for a backend whose job is talking to a remote host, so it - should surface as the 502 JSON shape the editor knows how to display, not as - an unhandled Flask 500. HTTP status errors are left for the caller to map, - since the right status code depends on what was being attempted. - """ - kwargs.setdefault('timeout', DEFAULT_TIMEOUT_SECONDS) - try: - return requests.request(method, url, **kwargs) - except requests.RequestException as exc: - raise RuleDraftBackendError( - f'Could not reach the git host while {error_action}: {exc}', - status_code=502, - ) from exc - - -def generate_branch_name(rule_name: str, author_email: str, *, prefix: str = 'rule-draft') -> str: - """Cosmetic source-branch label. Timestamped so retries don't collide.""" - short_email = author_email.split('@', 1)[0] - slug = re.sub(r'[^A-Za-z0-9_-]+', '-', short_email).strip('-') or 'osprey-ui' - rule_slug = re.sub(r'[^A-Za-z0-9_-]+', '-', rule_name).strip('-') or 'rule' - return f'{prefix}/{slug}/{rule_slug}-{int(time.time())}' - - -def require_already_present(main_sml: str, draft_path: str) -> bool: - pattern = re.compile( - r"Require\s*\(\s*rule\s*=\s*['\"]" + re.escape(draft_path) + r"['\"]\s*\)", - re.MULTILINE, - ) - return bool(pattern.search(main_sml)) - - -def append_require_to_main(main_sml: str, draft_path: str) -> str: - suffix = f"\nRequire(rule='{draft_path}')\n" - if not main_sml.endswith('\n'): - suffix = '\n' + suffix - return main_sml + suffix diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_github.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_github.py deleted file mode 100644 index 1e9678e6..00000000 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_github.py +++ /dev/null @@ -1,311 +0,0 @@ -"""GitHub submission backend. - -Implements RuleSubmissionBackend by opening a pull request against a configured -repo. Works with github.com and with GitHub Enterprise (set OSPREY_GITHUB_API_URL -to the Enterprise API root, e.g. https://github.mycompany.com/api/v3). -""" - -from __future__ import annotations - -import base64 -import os -from dataclasses import dataclass -from typing import Any - -import requests - -from . import _rule_drafts_git_common as git_common -from ._rule_drafts_backend import ( - PendingDraft, - RuleDraftBackendError, - SubmissionResult, -) - -DEFAULT_GITHUB_API = 'https://api.github.com' - - -@dataclass(frozen=True) -class GitHubConfig: - api_url: str - repo: str - base_branch: str - rules_path: str - token: str - - @property - def repo_url(self) -> str: - return f'{self.api_url.rstrip("/")}/repos/{self.repo}' - - @classmethod - def from_env(cls) -> 'GitHubConfig': - api_url = (os.environ.get('OSPREY_GITHUB_API_URL') or DEFAULT_GITHUB_API).strip() - repo = os.environ.get('OSPREY_RULES_REPO', '').strip() - base = os.environ.get('OSPREY_RULES_BASE_BRANCH', 'main').strip() or 'main' - rules_path = os.environ.get('OSPREY_RULES_PATH_IN_REPO', '').strip().strip('/') - token = os.environ.get('OSPREY_GITHUB_TOKEN', '').strip() - if not repo: - raise RuleDraftBackendError( - 'OSPREY_RULES_REPO is not configured; set it to "owner/name" to enable PR submission.', - status_code=503, - ) - if not token: - raise RuleDraftBackendError( - 'OSPREY_GITHUB_TOKEN is not configured; set a service-account PAT with repo write access.', - status_code=503, - ) - return cls(api_url=api_url, repo=repo, base_branch=base, rules_path=rules_path, token=token) - - -def _headers(cfg: GitHubConfig) -> dict[str, str]: - return { - 'Authorization': f'Bearer {cfg.token}', - 'Accept': 'application/vnd.github+json', - 'X-GitHub-Api-Version': '2022-11-28', - } - - -def _full_path(cfg: GitHubConfig, draft_path: str) -> str: - draft_path = draft_path.lstrip('/') - if cfg.rules_path: - return f'{cfg.rules_path}/{draft_path}' - return draft_path - - -def _get(cfg: GitHubConfig, url: str, **kwargs: Any) -> requests.Response: - return git_common.request('GET', url, error_action='contacting GitHub', headers=_headers(cfg), **kwargs) - - -def _post(cfg: GitHubConfig, url: str, json: dict[str, Any]) -> requests.Response: - return git_common.request('POST', url, error_action='contacting GitHub', headers=_headers(cfg), json=json) - - -def _put(cfg: GitHubConfig, url: str, json: dict[str, Any]) -> requests.Response: - return git_common.request('PUT', url, error_action='contacting GitHub', headers=_headers(cfg), json=json) - - -def _raise_for_github(response: requests.Response, action: str) -> None: - if response.ok: - return - body = response.text[:500] - # 4xx from GitHub is still a backend failure from the caller's POV. - raise RuleDraftBackendError( - f'GitHub returned {response.status_code} while {action}: {body}', - status_code=502, - ) - - -def _get_base_sha(cfg: GitHubConfig) -> str: - res = _get(cfg, f'{cfg.repo_url}/git/ref/heads/{cfg.base_branch}') - _raise_for_github(res, f'looking up base branch {cfg.base_branch!r}') - return res.json()['object']['sha'] - - -def _get_file_sha(cfg: GitHubConfig, path_in_repo: str, ref: str) -> str | None: - res = _get(cfg, f'{cfg.repo_url}/contents/{path_in_repo}', params={'ref': ref}) - if res.status_code == 404: - return None - _raise_for_github(res, f'reading {path_in_repo!r} on {ref!r}') - payload = res.json() - if isinstance(payload, list): - return None - return payload.get('sha') - - -def _create_branch(cfg: GitHubConfig, branch: str, sha: str) -> None: - res = _post(cfg, f'{cfg.repo_url}/git/refs', json={'ref': f'refs/heads/{branch}', 'sha': sha}) - if res.status_code == 422: - raise RuleDraftBackendError( - f'Branch {branch!r} already exists on {cfg.repo}. Pick a different name.', - status_code=409, - ) - _raise_for_github(res, f'creating branch {branch!r}') - - -def _commit_file( - cfg: GitHubConfig, - branch: str, - path_in_repo: str, - contents: str, - message: str, - existing_sha: str | None, -) -> None: - payload: dict[str, Any] = { - 'message': message, - 'content': base64.b64encode(contents.encode('utf-8')).decode('ascii'), - 'branch': branch, - } - if existing_sha is not None: - payload['sha'] = existing_sha - res = _put(cfg, f'{cfg.repo_url}/contents/{path_in_repo}', json=payload) - _raise_for_github(res, f'committing {path_in_repo!r} to {branch!r}') - - -def _open_pr(cfg: GitHubConfig, branch: str, title: str, body: str) -> dict[str, Any]: - res = _post( - cfg, - f'{cfg.repo_url}/pulls', - json={'title': title, 'head': branch, 'base': cfg.base_branch, 'body': body}, - ) - _raise_for_github(res, f'opening PR from {branch!r}') - return res.json() - - -def _main_sml_path(cfg: GitHubConfig) -> str: - return _full_path(cfg, 'main.sml') - - -def _fetch_file_on_ref(cfg: GitHubConfig, path_in_repo: str, ref: str) -> tuple[str, str] | None: - res = _get(cfg, f'{cfg.repo_url}/contents/{path_in_repo}', params={'ref': ref}) - if res.status_code == 404: - return None - _raise_for_github(res, f'reading {path_in_repo!r} on {ref!r}') - payload = res.json() - if isinstance(payload, list): - return None - encoded = payload.get('content', '') - sha = payload.get('sha') - if sha is None: - return None - try: - decoded = base64.b64decode(encoded).decode('utf-8') - except Exception as exc: - raise RuleDraftBackendError(f'could not decode {path_in_repo!r}: {exc}', status_code=502) - return decoded, sha - - -class GitHubBackend: - name = 'github' - - def __init__(self, cfg: GitHubConfig): - self._cfg = cfg - - @classmethod - def from_env(cls) -> 'GitHubBackend': - return cls(GitHubConfig.from_env()) - - def submit_draft( - self, - *, - draft_path: str, - sml_source: str, - rule_name: str, - summary: str, - author_email: str, - is_new_rule: bool, - wire_into_main: bool, - ) -> SubmissionResult: - cfg = self._cfg - path_in_repo = _full_path(cfg, draft_path) - base_sha = _get_base_sha(cfg) - existing_sha = _get_file_sha(cfg, path_in_repo, cfg.base_branch) - - if is_new_rule and existing_sha is not None: - raise RuleDraftBackendError( - f'A file already exists at {path_in_repo!r} on {cfg.base_branch}. ' - 'Pick a different filename, or edit the existing rule instead of creating a new one.', - status_code=409, - ) - - branch = git_common.generate_branch_name(rule_name, author_email) - _create_branch(cfg, branch, base_sha) - - verb = 'Add' if is_new_rule else 'Update' - commit_message = f'{verb} rule {rule_name}\n\nAuthored via Osprey UI by {author_email}.' - _commit_file( - cfg, - branch=branch, - path_in_repo=path_in_repo, - contents=sml_source, - message=commit_message, - existing_sha=existing_sha, - ) - - main_sml_updated = False - if wire_into_main: - main_path = _main_sml_path(cfg) - fetched = _fetch_file_on_ref(cfg, main_path, cfg.base_branch) - if fetched is None: - raise RuleDraftBackendError( - f'wire_into_main requested but {main_path!r} does not exist on {cfg.base_branch}.', - status_code=409, - ) - main_contents, main_sha = fetched - if not git_common.require_already_present(main_contents, draft_path): - new_main = git_common.append_require_to_main(main_contents, draft_path) - _commit_file( - cfg, - branch=branch, - path_in_repo=main_path, - contents=new_main, - message=f'Wire {rule_name} into main.sml\n\nAuthored via Osprey UI by {author_email}.', - existing_sha=main_sha, - ) - main_sml_updated = True - - title = f'{verb} rule {rule_name}' - touched = f'`{path_in_repo}`' - if main_sml_updated: - touched += f', `{_main_sml_path(cfg)}`' - body = ( - f'{summary.strip() or "_(no summary provided)_"}\n\n' - f'---\n' - f'Drafted in the Osprey rules UI by `{author_email}`.\n' - f'Touches: {touched}.' - ) - pr = _open_pr(cfg, branch=branch, title=title, body=body) - pr_number = pr.get('number') - pr_url = pr.get('html_url') - return SubmissionResult( - title=f'Pull request #{pr_number} opened', - url=pr_url, - main_sml_updated=main_sml_updated, - extras={ - 'pr_number': pr_number, - 'pr_url': pr_url, - 'branch': branch, - 'path_in_repo': path_in_repo, - }, - ) - - def list_pending_drafts(self) -> list[PendingDraft]: - cfg = self._cfg - res = _get( - cfg, - f'{cfg.repo_url}/pulls', - params={'state': 'open', 'base': cfg.base_branch, 'per_page': 30}, - ) - _raise_for_github(res, 'listing open pull requests') - open_prs = res.json() - - out: list[PendingDraft] = [] - for pr in open_prs: - number = pr.get('number') - if number is None: - continue - files_res = _get(cfg, f'{cfg.repo_url}/pulls/{number}/files', params={'per_page': 50}) - if not files_res.ok: - continue - files = files_res.json() - touched = [ - f['filename'] - for f in files - if isinstance(f.get('filename'), str) - and (not cfg.rules_path or f['filename'].startswith(cfg.rules_path + '/')) - and f['filename'].endswith('.sml') - ] - if not touched: - continue - out.append( - PendingDraft( - title=pr.get('title', ''), - url=pr.get('html_url', ''), - author=(pr.get('user') or {}).get('login', ''), - created_at=pr.get('created_at', ''), - touched_files=touched, - extras={ - 'pr_number': number, - 'branch': (pr.get('head') or {}).get('ref', ''), - }, - ) - ) - return out diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_local.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_local.py deleted file mode 100644 index 2d815eb6..00000000 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_local.py +++ /dev/null @@ -1,110 +0,0 @@ -"""Filesystem submission backend for self-hosted setups. - -Writes the SML straight to a configured rules directory. No review, no PR. -Adopters whose deploy pipeline already syncs a rules directory into the engine -(etcd push, file watcher, etc.) wire this backend up so the UI drops the SML -in the right place and lets the downstream pipeline take it from there. -""" - -from __future__ import annotations - -import os -from dataclasses import dataclass -from pathlib import Path - -from . import _rule_drafts_git_common as git_common -from ._rule_drafts_backend import ( - PendingDraft, - RuleDraftBackendError, - SubmissionResult, -) - - -@dataclass(frozen=True) -class LocalConfig: - rules_dir: Path - - @classmethod - def from_env(cls) -> 'LocalConfig': - raw = os.environ.get('OSPREY_RULES_LOCAL_PATH', '').strip() - if not raw: - raise RuleDraftBackendError( - 'OSPREY_RULES_LOCAL_PATH is not configured; set it to the directory the local backend should write to.', - status_code=503, - ) - path = Path(raw) - if not path.is_dir(): - raise RuleDraftBackendError( - f'OSPREY_RULES_LOCAL_PATH {raw!r} is not a directory.', - status_code=503, - ) - return cls(rules_dir=path) - - -class LocalBackend: - name = 'local' - - def __init__(self, cfg: LocalConfig): - self._cfg = cfg - - @classmethod - def from_env(cls) -> 'LocalBackend': - return cls(LocalConfig.from_env()) - - def _resolve(self, draft_path: str) -> Path: - """Resolve a draft path within rules_dir, refusing anything that would - escape via `..` or symlink traversal.""" - candidate = (self._cfg.rules_dir / draft_path).resolve() - try: - candidate.relative_to(self._cfg.rules_dir.resolve()) - except ValueError as exc: - raise RuleDraftBackendError( - f'Draft path {draft_path!r} escapes the configured rules directory.', - status_code=400, - ) from exc - return candidate - - def submit_draft( - self, - *, - draft_path: str, - sml_source: str, - rule_name: str, - summary: str, - author_email: str, - is_new_rule: bool, - wire_into_main: bool, - ) -> SubmissionResult: - target = self._resolve(draft_path) - if is_new_rule and target.exists(): - raise RuleDraftBackendError( - f'A file already exists at {draft_path!r}. Pick a different filename or edit the existing rule.', - status_code=409, - ) - target.parent.mkdir(parents=True, exist_ok=True) - target.write_text(sml_source, encoding='utf-8') - - main_sml_updated = False - if wire_into_main: - main_path = self._cfg.rules_dir / 'main.sml' - if not main_path.exists(): - raise RuleDraftBackendError( - f'wire_into_main requested but main.sml does not exist at {main_path}.', - status_code=409, - ) - main_contents = main_path.read_text(encoding='utf-8') - if not git_common.require_already_present(main_contents, draft_path): - main_path.write_text(git_common.append_require_to_main(main_contents, draft_path), encoding='utf-8') - main_sml_updated = True - - verb = 'Created' if is_new_rule else 'Updated' - return SubmissionResult( - title=f'{verb} {draft_path} in local rules directory', - url=None, - main_sml_updated=main_sml_updated, - extras={'path_on_disk': str(target)}, - ) - - def list_pending_drafts(self) -> list[PendingDraft]: - # Local backend has no review queue; submissions take effect immediately. - return [] diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_null.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_null.py deleted file mode 100644 index 8ee303ab..00000000 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/_rule_drafts_null.py +++ /dev/null @@ -1,41 +0,0 @@ -"""Default submission backend: nothing is configured, so every call fails fast. - -Ships as the default so an unconfigured upstream install never opens a PR or -writes a file without an adopter explicitly opting into a backend. -""" - -from __future__ import annotations - -from ._rule_drafts_backend import ( - PendingDraft, - RuleDraftBackendError, - SubmissionResult, -) - - -class NullBackend: - name = 'null' - - @staticmethod - def _err() -> RuleDraftBackendError: - return RuleDraftBackendError( - 'No rule-submission backend is configured. Set OSPREY_RULES_SUBMISSION_BACKEND ' - 'to one of: github, local. See docs for what each one needs.', - status_code=503, - ) - - def submit_draft( - self, - *, - draft_path: str, - sml_source: str, - rule_name: str, - summary: str, - author_email: str, - is_new_rule: bool, - wire_into_main: bool, - ) -> SubmissionResult: - raise self._err() - - def list_pending_drafts(self) -> list[PendingDraft]: - raise self._err() diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py index 588a9942..f0edc5e1 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py @@ -1,8 +1,10 @@ from __future__ import annotations import logging +import os import re from collections.abc import Iterable +from pathlib import Path from typing import Any from flask import Blueprint, jsonify, request @@ -30,10 +32,10 @@ ValidationWarning, ) from osprey.worker.lib.singletons import ENGINE +from osprey.worker.lib.storage.rule_drafts import RuleDraft from osprey.worker.ui_api.osprey.lib.abilities import CanEditRuleDrafts, require_ability from osprey.worker.ui_api.osprey.lib.auth import get_current_user_email -from . import _rule_drafts_backend as backends from ._engine_ast_utils import get_func_identifier logger = logging.getLogger(__name__) @@ -307,29 +309,50 @@ def vocabulary() -> Any: ) -@blueprint.route('/rule-drafts/submit', methods=['POST']) +def _revalidate(path: str, source_text: str) -> tuple[Any, int] | None: + """Re-run the engine's AST validation with the draft spliced into the loaded + sources. Returns a Flask error tuple on failure, or None if it validates.""" + spliced = _current_sources_dict() + spliced[path] = source_text + try: + sources = Sources.from_dict(spliced) + validate_sources( + sources, + udf_registry=ENGINE.instance().udf_registry, + validator_registry=ENGINE.instance().validator_registry, + ) + except ValidationFailed as exc: + return jsonify( + { + 'error': 'Validation failed; fix errors before submitting.', + 'errors': [_format_validation_message(e) for e in exc.errors], + } + ), 400 + except Exception as exc: + return jsonify({'error': f'Could not assemble sources: {exc}'}), 400 + return None + + +@blueprint.route('/rule-drafts', methods=['POST']) @require_ability(CanEditRuleDrafts) -def submit_draft() -> Any: +def create_draft() -> Any: + """Validate a draft and upsert it into the rule_drafts table (one row per path).""" payload = request.get_json(silent=True) or {} path = (payload.get('path') or '').strip() source_text = payload.get('source', '') rule_name = (payload.get('rule_name') or '').strip() summary = (payload.get('summary') or '').strip() - is_new_rule = bool(payload.get('is_new_rule', False)) - wire_into_main = bool(payload.get('wire_into_main', False)) path_err = _validate_path(path) if path_err: return jsonify({'error': path_err}), 400 if path == 'main.sml': - # main.sml is the engine entry point. Submitting a draft *as* main.sml - # would wholesale-replace it (immediate live effect on the local - # backend). Wiring a rule in is a controlled one-line append handled by - # the wire_into_main option, not a draft submission. + # main.sml is the engine entry point; a draft never replaces it wholesale. + # Deploying a draft optionally wires it into main.sml with a single Require line. return jsonify( { - 'error': 'main.sml is the engine entry point and cannot be submitted as a draft. ' - 'Use "turn this rule on" to add a Require line instead.' + 'error': 'main.sml is the engine entry point and cannot be saved as a draft. ' + 'Deploy a rule with wire_into_main to add a Require line instead.' } ), 400 if not isinstance(source_text, str) or not source_text.strip(): @@ -337,52 +360,131 @@ def submit_draft() -> Any: if not _VALID_RULE_NAME.match(rule_name): return jsonify({'error': 'rule_name must be a valid SML identifier ([A-Za-z_][A-Za-z0-9_]*).'}), 400 - # Re-validate server-side so a client that skips the validate step still cannot push uncompilable SML. - spliced = _current_sources_dict() - spliced[path] = source_text - try: - sources = Sources.from_dict(spliced) - validate_sources( - sources, - udf_registry=ENGINE.instance().udf_registry, - validator_registry=ENGINE.instance().validator_registry, + # Re-validate server-side so a client that skips the validate step still cannot store uncompilable SML. + error = _revalidate(path, source_text) + if error is not None: + return error + + draft = RuleDraft.upsert( + path=path, + rule_name=rule_name, + sml_source=source_text, + summary=summary, + author_email=get_current_user_email(), + ) + return jsonify(draft.to_json()) + + +@blueprint.route('/rule-drafts', methods=['GET']) +@require_ability(CanEditRuleDrafts) +def list_drafts() -> Any: + """The draft rules table: every staged draft, newest-edited first.""" + return jsonify({'drafts': [d.to_json() for d in RuleDraft.list_all()]}) + + +@blueprint.route('/rule-drafts/', methods=['GET']) +@require_ability(CanEditRuleDrafts) +def get_draft(draft_id: int) -> Any: + draft = RuleDraft.get_one(draft_id) + if draft is None: + return jsonify({'error': f'No draft with id {draft_id}.'}), 404 + return jsonify(draft.to_json()) + + +def _rules_dir_or_error() -> tuple[Path | None, tuple[Any, int] | None]: + """The directory deploy writes into (OSPREY_RULES_LOCAL_PATH), or an error tuple. + + Deploying into the engine's own rules source is a filesystem hand-off, the same + contract the retired `local` backend used: whatever pipeline already syncs that + directory (etcd push, file watcher) activates the rule. A DB-backed + SourcesProvider that lets the engine read deployed drafts straight from this + table would remove that dependency entirely; see the PR notes. + """ + raw = os.environ.get('OSPREY_RULES_LOCAL_PATH', '').strip() + if not raw: + return None, ( + jsonify( + { + 'error': 'Deploy is not configured. Set OSPREY_RULES_LOCAL_PATH to the rules ' + 'directory the engine loads so deployed drafts are written there.' + } + ), + 503, ) - except ValidationFailed as exc: - return jsonify( - { - 'error': 'Validation failed; fix errors before submitting.', - 'errors': [_format_validation_message(e) for e in exc.errors], - } - ), 400 - except Exception as exc: - return jsonify({'error': f'Could not assemble sources: {exc}'}), 400 + rules_dir = Path(raw) + if not rules_dir.is_dir(): + return None, (jsonify({'error': f'OSPREY_RULES_LOCAL_PATH {raw!r} is not a directory.'}), 503) + return rules_dir, None + +def _resolve_within(rules_dir: Path, draft_path: str) -> Path | None: + """Resolve draft_path inside rules_dir, or None if it would escape via `..`/symlink.""" + candidate = (rules_dir / draft_path).resolve() try: - backend = backends.load_backend() - result = backend.submit_draft( - draft_path=path, - sml_source=source_text, - rule_name=rule_name, - summary=summary, - author_email=get_current_user_email(), - is_new_rule=is_new_rule, - wire_into_main=wire_into_main, - ) - except backends.RuleDraftBackendError as exc: - return jsonify({'error': exc.message}), exc.status_code + candidate.relative_to(rules_dir.resolve()) + except ValueError: + return None + return candidate + + +def _main_requires(main_sml: str, draft_path: str) -> bool: + pattern = re.compile(r"Require\s*\(\s*rule\s*=\s*['\"]" + re.escape(draft_path) + r"['\"]\s*\)", re.MULTILINE) + return bool(pattern.search(main_sml)) - return jsonify(result.to_json()) +def _append_require(main_sml: str, draft_path: str) -> str: + suffix = f"\nRequire(rule='{draft_path}')\n" + if main_sml and not main_sml.endswith('\n'): + suffix = '\n' + suffix + return main_sml + suffix -@blueprint.route('/rule-drafts/pending', methods=['GET']) + +@blueprint.route('/rule-drafts//deploy', methods=['POST']) @require_ability(CanEditRuleDrafts) -def pending_drafts() -> Any: - try: - backend = backends.load_backend() - drafts = backend.list_pending_drafts() - except backends.RuleDraftBackendError as exc: - return jsonify({'error': exc.message, 'pending': []}), exc.status_code - return jsonify({'pending': [d.to_json() for d in drafts]}) +def deploy_draft(draft_id: int) -> Any: + """Write a draft's SML into the configured rules directory and mark it deployed. + + With `wire_into_main`, also append a `Require(rule=...)` line to main.sml so the + rule takes effect (the file on its own is inert until something requires it). + """ + draft = RuleDraft.get_one(draft_id) + if draft is None: + return jsonify({'error': f'No draft with id {draft_id}.'}), 404 + + payload = request.get_json(silent=True) or {} + wire_into_main = bool(payload.get('wire_into_main', False)) + + # Re-validate at deploy time: the loaded sources may have changed since the draft was saved. + error = _revalidate(draft.path, draft.sml_source) + if error is not None: + return error + + rules_dir, dir_error = _rules_dir_or_error() + if dir_error is not None: + return dir_error + assert rules_dir is not None + + target = _resolve_within(rules_dir, draft.path) + if target is None: + return jsonify({'error': f'Draft path {draft.path!r} escapes the rules directory.'}), 400 + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(draft.sml_source, encoding='utf-8') + + main_sml_updated = False + if wire_into_main: + main_path = rules_dir / 'main.sml' + if not main_path.exists(): + return jsonify({'error': f'wire_into_main requested but main.sml does not exist at {main_path}.'}), 409 + main_contents = main_path.read_text(encoding='utf-8') + if not _main_requires(main_contents, draft.path): + main_path.write_text(_append_require(main_contents, draft.path), encoding='utf-8') + main_sml_updated = True + + deployed = RuleDraft.mark_deployed(draft_id) + result = deployed.to_json() if deployed is not None else draft.to_json() + result['main_sml_updated'] = main_sml_updated + result['path_on_disk'] = str(target) + return jsonify(result) # The set of comparator strings the Rule Builder UI can render. diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py index dbe18e43..fb1d97df 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py @@ -1,12 +1,7 @@ -import base64 import json -import tempfile -from pathlib import Path from unittest.mock import patch import pytest -import requests -import requests_mock as requests_mock_module from flask import Response, url_for from flask.testing import FlaskClient from osprey.worker.lib.snowflake import Snowflake @@ -15,26 +10,13 @@ @pytest.fixture(autouse=True) def _mock_audit_snowflake(): # The after_request audit hook mints a snowflake id, which normally means an - # HTTP call to the snowflake-id-worker service. Tests that wrap a request in - # a requests_mock.Mocker would otherwise trip NoMockAddress on that call, so - # neutralize it here (the audit log's persist() is already mocked in the - # shared conftest). + # HTTP call to the snowflake-id-worker service. Neutralize it here so tests + # don't reach for that service (the audit log's persist() is already mocked + # in the shared conftest). with patch('osprey.worker.ui_api.osprey.lib.audit.generate_snowflake', return_value=Snowflake(1)): yield -def _set_github_backend(monkeypatch: pytest.MonkeyPatch, **overrides: str) -> None: - """Configure the github backend with sensible defaults for tests; overrides win.""" - defaults = { - 'OSPREY_RULES_SUBMISSION_BACKEND': 'github', - 'OSPREY_RULES_REPO': 'roostorg/osprey-rules', - 'OSPREY_GITHUB_TOKEN': 'gh_fake_token', - 'OSPREY_RULES_BASE_BRANCH': 'main', - } - for k, v in {**defaults, **overrides}.items(): - monkeypatch.setenv(k, v) - - _acl_with_draft_ability = json.dumps( { 'ui_config': {}, @@ -128,7 +110,11 @@ def test_endpoints_require_can_edit_rule_drafts(client: 'FlaskClient[Response]') assert res.status_code == 401 res = client.get(url_for('rule_drafts.vocabulary')) assert res.status_code == 401 - res = client.get(url_for('rule_drafts.pending_drafts')) + res = client.get(url_for('rule_drafts.list_drafts')) + assert res.status_code == 401 + res = client.post(url_for('rule_drafts.create_draft'), json={}) + assert res.status_code == 401 + res = client.post(url_for('rule_drafts.deploy_draft', draft_id=1), json={}) assert res.status_code == 401 @@ -232,669 +218,209 @@ def test_vocabulary_returns_features_udfs_effects(client: 'FlaskClient[Response] assert 'main.sml' in body['source_files'] -@pytest.mark.use_rules_sources(_base_sources) -def test_submit_returns_503_when_no_backend_configured( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - monkeypatch.delenv('OSPREY_RULES_SUBMISSION_BACKEND', raising=False) - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': 'demo', - 'is_new_rule': True, - }, - ) - assert res.status_code == 503 - body = res.json - assert body is not None - assert 'No rule-submission backend is configured' in body['error'] +# --- Draft table: create / list / get ------------------------------------- + +_VALID_DRAFT = "Import(rules=['models/base.sml'])\nSomeRule = Rule(when_all=[PostText == 'bye'], description='bye')" @pytest.mark.use_rules_sources(_base_sources) -def test_submit_returns_503_when_github_missing_required_env( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'github') - monkeypatch.delenv('OSPREY_RULES_REPO', raising=False) - monkeypatch.delenv('OSPREY_GITHUB_TOKEN', raising=False) +def test_create_draft_persists_and_lists(client: 'FlaskClient[Response]') -> None: res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': 'demo', - 'is_new_rule': True, - }, + url_for('rule_drafts.create_draft'), + json={'path': 'rules/spam.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': 'catch spam'}, ) - assert res.status_code == 503 - body = res.json - assert body is not None - assert 'OSPREY_RULES_REPO' in body['error'] + assert res.status_code == 200 + draft = res.json + assert draft is not None + assert draft['path'] == 'rules/spam.sml' + assert draft['rule_name'] == 'SomeRule' + assert draft['summary'] == 'catch spam' + assert draft['status'] == 'draft' + assert draft['id'] is not None + + res = client.get(url_for('rule_drafts.list_drafts')) + assert res.status_code == 200 + assert res.json is not None + assert any(d['path'] == 'rules/spam.sml' for d in res.json['drafts']) @pytest.mark.use_rules_sources(_base_sources) -def test_submit_blocks_invalid_sml_before_calling_github( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - _set_github_backend(monkeypatch) - - with requests_mock_module.Mocker() as m: - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/bad.sml', - 'source': 'this is not valid SML !!!', - 'rule_name': 'BadRule', - 'summary': 'should not submit', - 'is_new_rule': True, - }, - ) +def test_create_draft_upserts_same_path_in_place(client: 'FlaskClient[Response]') -> None: + first = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/dupe.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': 'v1'}, + ) + assert first.status_code == 200 + assert first.json is not None + original_id = first.json['id'] - assert res.status_code == 400 - assert m.call_count == 0 - body = res.json - assert body is not None - assert body['error'].startswith('Validation failed') + second = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/dupe.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': 'v2'}, + ) + assert second.status_code == 200 + assert second.json is not None + assert second.json['id'] == original_id + assert second.json['summary'] == 'v2' + + res = client.get(url_for('rule_drafts.list_drafts')) + assert res.json is not None + assert len([d for d in res.json['drafts'] if d['path'] == 'rules/dupe.sml']) == 1 @pytest.mark.use_rules_sources(_base_sources) -def test_submit_rejects_bad_rule_name(client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch) -> None: - _set_github_backend(monkeypatch) +def test_create_draft_rejects_invalid_sml(client: 'FlaskClient[Response]') -> None: res = client.post( - url_for('rule_drafts.submit_draft'), + url_for('rule_drafts.create_draft'), json={ - 'path': 'rules/new.sml', - 'source': "X = Rule(when_all=[PostText == 'x'], description='x')", - 'rule_name': '1-not-an-identifier', + 'path': 'rules/broken.sml', + 'rule_name': 'Broken', + 'source': "AnotherRule = Rule(when_all=[NonexistentFeature == 'x'], description='x')", 'summary': '', - 'is_new_rule': True, }, ) assert res.status_code == 400 - - -def test_submission_result_extras_cannot_shadow_canonical_fields() -> None: - from osprey.worker.ui_api.osprey.views._rule_drafts_backend import SubmissionResult - - result = SubmissionResult( - title='real title', - url='https://real.example/pr/1', - extras={'title': 'spoofed', 'url': 'https://evil.example', 'pr_number': 7}, - ) - out = result.to_json() - assert out['title'] == 'real title' - assert out['url'] == 'https://real.example/pr/1' - assert out['main_sml_updated'] is False - # Non-colliding extras still pass through for adopters that want them. - assert out['pr_number'] == 7 + assert res.json is not None + assert 'error' in res.json @pytest.mark.use_rules_sources(_base_sources) -def test_submit_rejects_main_sml_as_draft_path( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - _set_github_backend(monkeypatch) - with requests_mock_module.Mocker() as m: - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'main.sml', - 'source': 'Import(rules=[])', - 'rule_name': 'Whatever', - 'summary': '', - 'is_new_rule': False, - }, - ) - # Guard fires before any network call: the entry point is never a draft. - assert m.call_count == 0 +def test_create_draft_rejects_main_sml(client: 'FlaskClient[Response]') -> None: + res = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'main.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, + ) assert res.status_code == 400 - body = res.json - assert body is not None - assert 'main.sml' in body['error'] @pytest.mark.use_rules_sources(_base_sources) -def test_submit_surfaces_github_connection_error_as_502( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - _set_github_backend(monkeypatch) - repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' - with requests_mock_module.Mocker() as m: - m.get(f'{repo_url}/git/ref/heads/main', exc=requests.exceptions.ConnectTimeout) - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': '', - 'is_new_rule': True, - }, - ) - # A forge outage is a structured 502, not an unhandled 500. - assert res.status_code == 502 - body = res.json - assert body is not None - assert 'Could not reach the git host' in body['error'] +def test_create_draft_rejects_bad_rule_name(client: 'FlaskClient[Response]') -> None: + res = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/x.sml', 'rule_name': '9 not valid', 'source': _VALID_DRAFT, 'summary': ''}, + ) + assert res.status_code == 400 @pytest.mark.use_rules_sources(_base_sources) -def test_submit_happy_path_creates_branch_commits_and_opens_pr( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - _set_github_backend(monkeypatch, OSPREY_RULES_PATH_IN_REPO='rules') - - repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' - - with requests_mock_module.Mocker() as m: - m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) - m.get(f'{repo_url}/contents/rules/new_rule.sml', status_code=404) - m.post(f'{repo_url}/git/refs', status_code=201, json={}) - m.put(f'{repo_url}/contents/rules/new_rule.sml', status_code=201, json={}) - m.post( - f'{repo_url}/pulls', - status_code=201, - json={'number': 42, 'html_url': 'https://github.com/roostorg/osprey-rules/pull/42'}, - ) - - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - # Bare filename: OSPREY_RULES_PATH_IN_REPO='rules' prepends the - # subdirectory, so the file lands at rules/new_rule.sml. - 'path': 'new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': 'add bye rule', - 'is_new_rule': True, - }, - ) +def test_get_draft_returns_one_and_404s(client: 'FlaskClient[Response]') -> None: + created = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/getme.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, + ) + assert created.json is not None + draft_id = created.json['id'] + res = client.get(url_for('rule_drafts.get_draft', draft_id=draft_id)) assert res.status_code == 200 - body = res.json - assert body is not None - assert body['title'] == 'Pull request #42 opened' - assert body['url'] == 'https://github.com/roostorg/osprey-rules/pull/42' - assert body['main_sml_updated'] is False - # GitHub-specific extras are surfaced for adopters that want them. - assert body['pr_number'] == 42 - assert body['pr_url'] == 'https://github.com/roostorg/osprey-rules/pull/42' - assert body['path_in_repo'] == 'rules/new_rule.sml' - assert body['branch'].startswith('rule-draft/local-dev/AnotherRule-') - - -@pytest.mark.use_rules_sources(_base_sources) -def test_submit_wire_into_main_appends_require_line( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - _set_github_backend(monkeypatch) - - repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' - existing_main = "Import(rules=['models/post.sml'])\n\nRequire(rule='rules/post_contains_hello.sml')\n" - encoded_main = base64.b64encode(existing_main.encode('utf-8')).decode('ascii') - - with requests_mock_module.Mocker() as m: - m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) - m.get(f'{repo_url}/contents/rules/new_rule.sml', status_code=404) - m.post(f'{repo_url}/git/refs', status_code=201, json={}) - m.put(f'{repo_url}/contents/rules/new_rule.sml', status_code=201, json={}) - m.get(f'{repo_url}/contents/main.sml', json={'sha': 'MAIN_SHA', 'content': encoded_main, 'type': 'file'}) - main_put = m.put(f'{repo_url}/contents/main.sml', status_code=200, json={}) - m.post( - f'{repo_url}/pulls', - status_code=201, - json={'number': 99, 'html_url': 'https://github.com/roostorg/osprey-rules/pull/99'}, - ) + assert res.json is not None + assert res.json['path'] == 'rules/getme.sml' - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': 'add bye rule and wire it in', - 'is_new_rule': True, - 'wire_into_main': True, - }, - ) + res = client.get(url_for('rule_drafts.get_draft', draft_id=999999)) + assert res.status_code == 404 - assert res.status_code == 200 - body = res.json - assert body is not None - assert body['main_sml_updated'] is True - main_put_request = main_put.last_request - assert main_put_request is not None - posted = main_put_request.json() - decoded_new_main = base64.b64decode(posted['content']).decode('utf-8') - assert "Require(rule='rules/new_rule.sml')" in decoded_new_main - # The existing Require for post_contains_hello.sml should still be present untouched. - assert "Require(rule='rules/post_contains_hello.sml')" in decoded_new_main +# --- Draft table: deploy -------------------------------------------------- @pytest.mark.use_rules_sources(_base_sources) -def test_submit_wire_into_main_skips_when_require_already_present( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +def test_deploy_writes_sml_and_marks_deployed( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: - _set_github_backend(monkeypatch) - - repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' - existing_main = "Require(rule='rules/already_here.sml')\n" - encoded_main = base64.b64encode(existing_main.encode('utf-8')).decode('ascii') - - with requests_mock_module.Mocker() as m: - m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) - m.get(f'{repo_url}/contents/rules/already_here.sml', status_code=404) - m.post(f'{repo_url}/git/refs', status_code=201, json={}) - m.put(f'{repo_url}/contents/rules/already_here.sml', status_code=201, json={}) - m.get(f'{repo_url}/contents/main.sml', json={'sha': 'MAIN_SHA', 'content': encoded_main, 'type': 'file'}) - main_put = m.put(f'{repo_url}/contents/main.sml', status_code=200, json={}) - m.post( - f'{repo_url}/pulls', - status_code=201, - json={'number': 100, 'html_url': 'https://github.com/roostorg/osprey-rules/pull/100'}, - ) - - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/already_here.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': '', - 'is_new_rule': True, - 'wire_into_main': True, - }, - ) + monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + created = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/deploy.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, + ) + assert created.json is not None + draft_id = created.json['id'] + res = client.post(url_for('rule_drafts.deploy_draft', draft_id=draft_id), json={}) assert res.status_code == 200 - body = res.json - assert body is not None - assert body['main_sml_updated'] is False - # main.sml fetch happens but the PUT to update it must not. - assert main_put.call_count == 0 - - -@pytest.mark.use_rules_sources(_base_sources) -def test_submit_409_if_new_rule_file_already_exists( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - _set_github_backend(monkeypatch) - - repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' - - with requests_mock_module.Mocker() as m: - m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) - m.get( - f'{repo_url}/contents/new_rule.sml', - json={'sha': 'EXISTING_BLOB_SHA', 'type': 'file'}, - ) - - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': '', - 'is_new_rule': True, - }, - ) + assert res.json is not None + assert res.json['status'] == 'deployed' + assert res.json['deployed_at'] is not None + assert res.json['main_sml_updated'] is False - assert res.status_code == 409 + written = tmp_path / 'rules' / 'deploy.sml' + assert written.exists() + assert written.read_text() == _VALID_DRAFT @pytest.mark.use_rules_sources(_base_sources) -def test_pending_filters_to_rules_path_and_sml( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +def test_deploy_wire_into_main_appends_require( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: - _set_github_backend(monkeypatch, OSPREY_RULES_PATH_IN_REPO='rules') - - repo_url = 'https://api.github.com/repos/roostorg/osprey-rules' - - with requests_mock_module.Mocker() as m: - m.get( - f'{repo_url}/pulls', - json=[ - { - 'number': 1, - 'title': 'Add rule X', - 'html_url': 'https://github.com/roostorg/osprey-rules/pull/1', - 'head': {'ref': 'rule-draft/local-dev/X-1'}, - 'user': {'login': 'someone'}, - 'created_at': '2026-06-30T12:00:00Z', - }, - { - 'number': 2, - 'title': 'Update README', - 'html_url': 'https://github.com/roostorg/osprey-rules/pull/2', - 'head': {'ref': 'docs/readme'}, - 'user': {'login': 'someone'}, - 'created_at': '2026-06-30T13:00:00Z', - }, - ], - ) - m.get( - f'{repo_url}/pulls/1/files', - json=[{'filename': 'rules/x.sml'}], - ) - m.get( - f'{repo_url}/pulls/2/files', - json=[{'filename': 'README.md'}], - ) - - res = client.get(url_for('rule_drafts.pending_drafts')) - - assert res.status_code == 200 - body = res.json - assert body is not None - assert len(body['pending']) == 1 - entry = body['pending'][0] - assert entry['title'] == 'Add rule X' - assert entry['url'] == 'https://github.com/roostorg/osprey-rules/pull/1' - assert entry['touched_files'] == ['rules/x.sml'] - # GitHub-specific extras carried through for adopters that want them. - assert entry['pr_number'] == 1 - - -@pytest.mark.use_rules_sources(_base_sources) -def test_parse_into_builder_round_trips_a_builder_shaped_rule(client: 'FlaskClient[Response]') -> None: - source = """ -Import(rules=['models/post.sml']) - -ContainsCat = Rule( - when_all=[ - TextContains(text=PostText, phrase='cat'), - EventType == 'create_post', - ], - description='looks for cat', -) - -WhenRules( - rules_any=[ContainsCat], - then=[ - LabelAdd(entity=UserId, label='meow'), - ], -) -""" - res = client.post( - url_for('rule_drafts.parse_into_builder'), - json={'path': 'rules/contains_cat.sml', 'source': source}, - ) - assert res.status_code == 200 - body = res.json - assert body is not None - assert body['supported'] is True - model = body['model'] - assert model['ruleName'] == 'ContainsCat' - assert model['description'] == 'looks for cat' - assert model['conditions'] == [ - {'feature': 'PostText', 'operator': 'includes', 'rhs': 'cat', 'rhsIsFeature': False}, - {'feature': 'EventType', 'operator': '==', 'rhs': 'create_post', 'rhsIsFeature': False}, - ] - assert model['outcomes'] == [ - { - 'effect': 'LabelAdd', - 'args': [ - {'name': 'entity', 'value': 'UserId', 'isFeature': True}, - {'name': 'label', 'value': 'meow', 'isFeature': False}, - ], - } - ] - - -@pytest.mark.use_rules_sources(_base_sources) -def test_parse_into_builder_handles_excludes(client: 'FlaskClient[Response]') -> None: - source = "BlocksCat = Rule(when_all=[not TextContains(text=PostText, phrase='cat')], description='no cat')\n" - res = client.post( - url_for('rule_drafts.parse_into_builder'), - json={'path': 'rules/blocks_cat.sml', 'source': source}, + (tmp_path / 'main.sml').write_text("Import(rules=['models/base.sml'])\n") + monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + created = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/wired.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, ) - assert res.status_code == 200 - body = res.json - assert body is not None - assert body['supported'] is True - assert body['model']['conditions'] == [ - {'feature': 'PostText', 'operator': 'excludes', 'rhs': 'cat', 'rhsIsFeature': False}, - ] - + assert created.json is not None + draft_id = created.json['id'] -@pytest.mark.use_rules_sources(_base_sources) -def test_parse_into_builder_rejects_multiple_rules(client: 'FlaskClient[Response]') -> None: - source = ( - "A = Rule(when_all=[PostText == 'a'], description='a')\nB = Rule(when_all=[PostText == 'b'], description='b')\n" - ) - res = client.post( - url_for('rule_drafts.parse_into_builder'), - json={'path': 'rules/multi.sml', 'source': source}, - ) + res = client.post(url_for('rule_drafts.deploy_draft', draft_id=draft_id), json={'wire_into_main': True}) assert res.status_code == 200 - body = res.json - assert body is not None - assert body['supported'] is False - assert 'multiple Rule definitions' in body['reason'] + assert res.json is not None + assert res.json['main_sml_updated'] is True + assert "Require(rule='rules/wired.sml')" in (tmp_path / 'main.sml').read_text() @pytest.mark.use_rules_sources(_base_sources) -def test_parse_into_builder_rejects_helper_assigns(client: 'FlaskClient[Response]') -> None: - source = "Helper = 'cat'\nA = Rule(when_all=[PostText == Helper], description='a')\n" - res = client.post( - url_for('rule_drafts.parse_into_builder'), - json={'path': 'rules/helper.sml', 'source': source}, +def test_deploy_wire_into_main_is_idempotent( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path +) -> None: + (tmp_path / 'main.sml').write_text("Import(rules=['models/base.sml'])\nRequire(rule='rules/already.sml')\n") + monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + created = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/already.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, ) - assert res.status_code == 200 - body = res.json - assert body is not None - assert body['supported'] is False - assert 'helper assignment' in body['reason'] + assert created.json is not None + draft_id = created.json['id'] - -@pytest.mark.use_rules_sources(_base_sources) -def test_parse_into_builder_rejects_complex_condition(client: 'FlaskClient[Response]') -> None: - # A boolean operator inside `when_all` would need Code Editor; the builder is AND-only via row repetition. - source = "A = Rule(when_all=[PostText == 'a' and EventType == 'create_post'], description='a')\n" - res = client.post( - url_for('rule_drafts.parse_into_builder'), - json={'path': 'rules/complex.sml', 'source': source}, - ) + res = client.post(url_for('rule_drafts.deploy_draft', draft_id=draft_id), json={'wire_into_main': True}) assert res.status_code == 200 - body = res.json - assert body is not None - assert body['supported'] is False - assert 'Rule Builder cannot represent' in body['reason'] + assert res.json is not None + assert res.json['main_sml_updated'] is False + assert (tmp_path / 'main.sml').read_text().count("Require(rule='rules/already.sml')") == 1 @pytest.mark.use_rules_sources(_base_sources) -def test_parse_into_builder_rejects_file_with_no_rule(client: 'FlaskClient[Response]') -> None: - source = "Import(rules=['models/post.sml'])\n" - res = client.post( - url_for('rule_drafts.parse_into_builder'), - json={'path': 'rules/empty.sml', 'source': source}, +def test_deploy_wire_into_main_409_when_main_missing( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path +) -> None: + monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + created = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/nomain.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, ) - assert res.status_code == 200 - body = res.json - assert body is not None - assert body['supported'] is False - assert 'no Rule(...)' in body['reason'] - + assert created.json is not None + draft_id = created.json['id'] -@pytest.mark.use_rules_sources(_base_sources) -def test_parse_into_builder_rejects_syntax_error(client: 'FlaskClient[Response]') -> None: - res = client.post( - url_for('rule_drafts.parse_into_builder'), - json={'path': 'rules/broken.sml', 'source': 'this is not valid SML at all !!!'}, - ) - assert res.status_code == 200 - body = res.json - assert body is not None - assert body['supported'] is False - assert 'could not parse SML' in body['reason'] + res = client.post(url_for('rule_drafts.deploy_draft', draft_id=draft_id), json={'wire_into_main': True}) + assert res.status_code == 409 @pytest.mark.use_rules_sources(_base_sources) -def test_unknown_backend_value_returns_500(client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'gerrit') - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': '', - 'is_new_rule': True, - }, +def test_deploy_503_when_rules_dir_unset(client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv('OSPREY_RULES_LOCAL_PATH', raising=False) + created = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/nodir.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, ) - assert res.status_code == 500 - body = res.json - assert body is not None - assert "Unknown OSPREY_RULES_SUBMISSION_BACKEND 'gerrit'" in body['error'] - + assert created.json is not None + draft_id = created.json['id'] -@pytest.mark.use_rules_sources(_base_sources) -def test_pending_returns_empty_list_for_null_backend( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - monkeypatch.delenv('OSPREY_RULES_SUBMISSION_BACKEND', raising=False) - res = client.get(url_for('rule_drafts.pending_drafts')) + res = client.post(url_for('rule_drafts.deploy_draft', draft_id=draft_id), json={}) assert res.status_code == 503 - body = res.json - assert body is not None - assert body['pending'] == [] - - -@pytest.mark.use_rules_sources(_base_sources) -def test_submit_github_enterprise_url_is_threaded_into_requests( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - """GitHub Enterprise customers point OSPREY_GITHUB_API_URL at their own host; every API call must use it.""" - _set_github_backend( - monkeypatch, - OSPREY_GITHUB_API_URL='https://github.acme.test/api/v3', - OSPREY_RULES_REPO='acme/rules', - ) - - repo_url = 'https://github.acme.test/api/v3/repos/acme/rules' - - with requests_mock_module.Mocker() as m: - m.get(f'{repo_url}/git/ref/heads/main', json={'object': {'sha': 'BASE_SHA'}}) - m.get(f'{repo_url}/contents/rules/new_rule.sml', status_code=404) - m.post(f'{repo_url}/git/refs', status_code=201, json={}) - m.put(f'{repo_url}/contents/rules/new_rule.sml', status_code=201, json={}) - m.post( - f'{repo_url}/pulls', - status_code=201, - json={'number': 5, 'html_url': 'https://github.acme.test/acme/rules/pull/5'}, - ) - - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': '', - 'is_new_rule': True, - }, - ) - - assert res.status_code == 200 - body = res.json - assert body is not None - assert body['url'] == 'https://github.acme.test/acme/rules/pull/5' - # If any call had hit api.github.com instead, the Mocker would have raised NoMockAddress. - - -@pytest.mark.use_rules_sources(_base_sources) -def test_local_backend_writes_file_and_returns_no_url( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch -) -> None: - with tempfile.TemporaryDirectory() as tmpdir: - rules_dir = Path(tmpdir) - (rules_dir / 'main.sml').write_text("Import(rules=['models/post.sml'])\n", encoding='utf-8') - - monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'local') - monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(rules_dir)) - - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': 'add bye rule', - 'is_new_rule': True, - 'wire_into_main': False, - }, - ) - - assert res.status_code == 200 - body = res.json - assert body is not None - assert body['url'] is None - assert 'rules/new_rule.sml' in body['title'] - assert body['main_sml_updated'] is False - - written = (rules_dir / 'rules' / 'new_rule.sml').read_text(encoding='utf-8') - assert 'AnotherRule' in written @pytest.mark.use_rules_sources(_base_sources) -def test_local_backend_wires_into_main_when_requested( - client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch +def test_deploy_404_for_unknown_draft( + client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: - with tempfile.TemporaryDirectory() as tmpdir: - rules_dir = Path(tmpdir) - (rules_dir / 'main.sml').write_text("Import(rules=['models/post.sml'])\n", encoding='utf-8') - - monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'local') - monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(rules_dir)) - - res = client.post( - url_for('rule_drafts.submit_draft'), - json={ - 'path': 'rules/new_rule.sml', - 'source': "Import(rules=['models/base.sml'])\nAnotherRule = Rule(when_all=[PostText == 'bye'], description='bye')", - 'rule_name': 'AnotherRule', - 'summary': '', - 'is_new_rule': True, - 'wire_into_main': True, - }, - ) - - assert res.status_code == 200 - body = res.json - assert body is not None - assert body['main_sml_updated'] is True - updated_main = (rules_dir / 'main.sml').read_text(encoding='utf-8') - assert "Require(rule='rules/new_rule.sml')" in updated_main - - -@pytest.mark.use_rules_sources(_base_sources) -def test_local_backend_rejects_path_traversal(client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch) -> None: - with tempfile.TemporaryDirectory() as tmpdir: - rules_dir = Path(tmpdir) - monkeypatch.setenv('OSPREY_RULES_SUBMISSION_BACKEND', 'local') - monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(rules_dir)) - - # The view layer already rejects '..' in the path, so to actually exercise the - # local backend's own guard we bypass the view-level check by going around the - # API: instantiate the backend and call submit_draft directly with a traversal path. - from osprey.worker.ui_api.osprey.views._rule_drafts_backend import RuleDraftBackendError - from osprey.worker.ui_api.osprey.views._rule_drafts_local import LocalBackend, LocalConfig - - backend = LocalBackend(LocalConfig(rules_dir=rules_dir)) - with pytest.raises(RuleDraftBackendError) as exc_info: - backend.submit_draft( - draft_path='../escape.sml', - sml_source='x = 1', - rule_name='X', - summary='', - author_email='test@local', - is_new_rule=True, - wire_into_main=False, - ) - assert 'escapes the configured rules directory' in exc_info.value.message + monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + res = client.post(url_for('rule_drafts.deploy_draft', draft_id=999999), json={}) + assert res.status_code == 404 From 0ec81dfa57f7601e137ff646ad5bbf49724fae04 Mon Sep 17 00:00:00 2001 From: Juliet Shen Date: Wed, 15 Jul 2026 14:11:23 -0400 Subject: [PATCH 3/6] Reject drafts that reuse another draft's rule name SML rule names are global identifiers, so two drafts sharing a name would collide once both deploy. Server-side validation only sees deployed rules, not other rows in the rule_drafts table, so it can't catch the draft-vs-draft case. Add RuleDraft.other_with_rule_name() and have create_draft return 409 when a different path already uses the name (re-saving the same path is still an in-place update, not a conflict). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X --- .../osprey/worker/lib/storage/rule_drafts.py | 14 ++++++++++ .../worker/ui_api/osprey/views/rule_drafts.py | 10 +++++++ .../osprey/views/tests/test_rule_drafts.py | 26 +++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py b/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py index ac520021..6108fdc2 100644 --- a/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py +++ b/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py @@ -97,6 +97,20 @@ def get_one(cls, draft_id: int) -> 'RuleDraft | None': session.expunge(draft) return draft + @classmethod + def other_with_rule_name(cls, rule_name: str, *, exclude_path: str) -> 'RuleDraft | None': + """A draft at a different path that already uses `rule_name`, if one exists. + + Rule names are global identifiers in SML, so two drafts sharing a name would + collide once both deploy. Validation only sees deployed rules, not other + drafts, so this catches the draft-vs-draft case that validation can't. + """ + with scoped_session() as session: + draft = session.query(cls).filter(cls.rule_name == rule_name, cls.path != exclude_path).first() + if draft is not None: + session.expunge(draft) + return draft + @classmethod def mark_deployed(cls, draft_id: int) -> 'RuleDraft | None': with scoped_session(commit=True) as session: diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py index f0edc5e1..98f6b25e 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py @@ -365,6 +365,16 @@ def create_draft() -> Any: if error is not None: return error + # Validation only sees deployed rules; guard the draft-vs-draft name collision it can't. + conflict = RuleDraft.other_with_rule_name(rule_name, exclude_path=path) + if conflict is not None: + return jsonify( + { + 'error': f'Another draft ({conflict.path}) already uses the rule name {rule_name!r}. ' + 'Rename this rule, or edit that draft instead.' + } + ), 409 + draft = RuleDraft.upsert( path=path, rule_name=rule_name, diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py index fb1d97df..2c24afd4 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py @@ -268,6 +268,32 @@ def test_create_draft_upserts_same_path_in_place(client: 'FlaskClient[Response]' assert len([d for d in res.json['drafts'] if d['path'] == 'rules/dupe.sml']) == 1 +@pytest.mark.use_rules_sources(_base_sources) +def test_create_draft_rejects_duplicate_rule_name_across_drafts(client: 'FlaskClient[Response]') -> None: + first = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/first.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, + ) + assert first.status_code == 200 + + # A different path reusing the same rule name collides: rule names are global in SML, + # and validation can't see the other draft (it only knows deployed rules). + second = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/second.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, + ) + assert second.status_code == 409 + assert second.json is not None + assert 'SomeRule' in second.json['error'] + + # Re-saving the same path with the same name is an update, not a collision. + again = client.post( + url_for('rule_drafts.create_draft'), + json={'path': 'rules/first.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': 'edit'}, + ) + assert again.status_code == 200 + + @pytest.mark.use_rules_sources(_base_sources) def test_create_draft_rejects_invalid_sml(client: 'FlaskClient[Response]') -> None: res = client.post( From 62beabb1e7c2bd0e528ae8d2fd56370200d559d1 Mon Sep 17 00:00:00 2001 From: Juliet Shen Date: Sun, 19 Jul 2026 13:28:57 -0700 Subject: [PATCH 4/6] Address review: harden validation, deploy, and the upsert Feedback from CodeRabbit and @ThisIsMissEm on the rule-drafts backend: - Don't leak raw exception text to clients; log server-side and return a generic message when sources can't be assembled (both /validate and the create/deploy re-validation). - Share one `_validate_draft_source` helper between /validate and the server-side re-validation so the two can't drift. - Catch the specific RuntimeError/AttributeError from `span.ast_node` instead of a bare `except: pass` when extracting the identifier. - Deploy: verify main.sml exists before writing the rule file, so a missing main.sml no longer leaves the file written while the request 409s. - Deploy: report `path_on_disk` relative to the rules directory rather than leaking the absolute server path. - Make the path upsert atomic with INSERT ... ON CONFLICT DO UPDATE so two concurrent saves of the same path can't race the unique constraint. - Reject absolute paths in `_validate_path`; drop the stray `.` from the path character class; use `expunge_all()`; note that Osprey has no users table (identity is an email + ACLs). - Tests: clear the rule_drafts table between tests (the DB is session-scoped) and assert the deploy 409 leaves no file behind. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X --- .../osprey/worker/lib/storage/rule_drafts.py | 37 +++-- .../worker/ui_api/osprey/views/rule_drafts.py | 150 +++++++++++------- .../osprey/views/tests/test_rule_drafts.py | 13 ++ 3 files changed, 126 insertions(+), 74 deletions(-) diff --git a/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py b/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py index 6108fdc2..069a485a 100644 --- a/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py +++ b/osprey_worker/src/osprey/worker/lib/storage/rule_drafts.py @@ -4,6 +4,7 @@ from enum import StrEnum from sqlalchemy import BigInteger, Column, DateTime, Enum, Text +from sqlalchemy.dialects.postgresql import insert as pg_insert from .postgres import Model, scoped_session @@ -36,6 +37,8 @@ class RuleDraft(Model): rule_name: str = Column(Text, nullable=False) sml_source: str = Column(Text, nullable=False) summary: str = Column(Text, nullable=False, default='') + # Osprey has no users table; identity is just an email with ACLs applied, so + # this stores the author's email rather than a foreign key. author_email: str = Column(Text, nullable=False) status: RuleDraftStatus = Column( Enum(RuleDraftStatus, native_enum=False, length=32), @@ -64,20 +67,29 @@ def to_json(self) -> dict[str, object]: def upsert(cls, *, path: str, rule_name: str, sml_source: str, summary: str, author_email: str) -> 'RuleDraft': """Create the draft for `path`, or update it in place if one already exists. - Editing a deployed draft moves it back to `DRAFT` so the table reflects - that the in-flight SML no longer matches what was last deployed. + Uses a single `INSERT ... ON CONFLICT DO UPDATE` so two concurrent saves of + the same path can't both see "no row" and then race the unique constraint. + Editing a deployed draft moves it back to `DRAFT` so the table reflects that + the in-flight SML no longer matches what was last deployed. """ + now = _now() + mutable = { + 'rule_name': rule_name, + 'sml_source': sml_source, + 'summary': summary, + 'author_email': author_email, + 'status': RuleDraftStatus.DRAFT, + 'updated_at': now, + } + statement = ( + pg_insert(cls.__table__) + .values(path=path, created_at=now, **mutable) + .on_conflict_do_update(index_elements=[cls.path], set_=mutable) + ) with scoped_session(commit=True) as session: - draft = session.query(cls).filter(cls.path == path).first() - if draft is None: - draft = cls(path=path) - session.add(draft) - draft.rule_name = rule_name - draft.sml_source = sml_source - draft.summary = summary - draft.author_email = author_email - draft.status = RuleDraftStatus.DRAFT + session.execute(statement) session.flush() + draft = session.query(cls).filter(cls.path == path).one() session.expunge(draft) return draft @@ -85,8 +97,7 @@ def upsert(cls, *, path: str, rule_name: str, sml_source: str, summary: str, aut def list_all(cls) -> list['RuleDraft']: with scoped_session() as session: drafts = session.query(cls).order_by(cls.updated_at.desc()).all() - for draft in drafts: - session.expunge(draft) + session.expunge_all() return drafts @classmethod diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py index 98f6b25e..36d1e339 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py @@ -4,6 +4,7 @@ import os import re from collections.abc import Iterable +from dataclasses import dataclass, field from pathlib import Path from typing import Any @@ -42,18 +43,19 @@ blueprint = Blueprint('rule_drafts', __name__) -_VALID_PATH = re.compile(r'^[A-Za-z0-9_./-]+\.sml$') +_VALID_PATH = re.compile(r'^[A-Za-z0-9_/-]+\.sml$') _VALID_RULE_NAME = re.compile(r'^[A-Za-z_][A-Za-z0-9_]*$') def _format_validation_message(msg: ValidationError | ValidationWarning) -> dict[str, Any]: identifier: str | None = None try: + # `ast_node` is a property that raises RuntimeError when the span has no node. node = msg.span.ast_node - if isinstance(node, Name): - identifier = node.identifier - except Exception: - pass + except (RuntimeError, AttributeError): + node = None + if isinstance(node, Name): + identifier = node.identifier defined_in: list[str] = [] for additional in msg.additional_spans: @@ -92,7 +94,11 @@ def _suggest_imports_from_errors( def _validate_path(path: str) -> str | None: if not _VALID_PATH.match(path): - return f'Path {path!r} is not a valid SML source path (must end .sml and contain only [A-Za-z0-9_./-]).' + return f'Path {path!r} is not a valid SML source path (must end .sml and contain only [A-Za-z0-9_/-]).' + if path.startswith('/'): + # Rule paths are relative to the rules directory; an absolute path would + # escape it. Deploy also guards this with _resolve_within, but reject early. + return f'Path {path!r} must be relative to the rules directory, not absolute.' if '..' in path.split('/'): return f'Path {path!r} contains a parent-directory segment.' return None @@ -118,6 +124,57 @@ def get_source() -> Any: return jsonify({'path': source.path, 'contents': source.contents}) +@dataclass +class _DraftValidation: + """The outcome of validating a draft spliced into the loaded sources.""" + + ok: bool + errors: list[dict[str, Any]] = field(default_factory=list) + warnings: list[dict[str, Any]] = field(default_factory=list) + suggested_imports: list[str] = field(default_factory=list) + # Set when the sources couldn't even be assembled (e.g. broken main.sml), + # which is distinct from the SML compiling but failing validation. + assemble_error: str | None = None + + +def _validate_draft_source(path: str, source_text: str) -> _DraftValidation: + """Splice the draft into the loaded sources and run AST validation. + + Shared by POST /rule-drafts/validate (which reports the result) and the + server-side re-validation on create/deploy (which rejects on failure), so the + two paths can't drift apart. + """ + spliced = _current_sources_dict() + spliced[path] = source_text + try: + sources = Sources.from_dict(spliced) + except Exception: + # Don't echo the raw exception to the client (it can leak internals). + logger.exception('failed to assemble sources for draft at %r', path) + return _DraftValidation(ok=False, assemble_error='Could not assemble the rule sources for validation.') + + engine = ENGINE.instance() + try: + validated = validate_sources( + sources, + udf_registry=engine.udf_registry, + validator_registry=engine.validator_registry, + ) + except ValidationFailed as exc: + errors = [_format_validation_message(e) for e in exc.errors] + return _DraftValidation( + ok=False, + errors=errors, + warnings=[_format_validation_message(w) for w in exc.warnings], + suggested_imports=_suggest_imports_from_errors(path, errors), + ) + + return _DraftValidation( + ok=True, + warnings=[_format_validation_message(w) for w in validated.warnings], + ) + + @blueprint.route('/rule-drafts/validate', methods=['POST']) @require_ability(CanEditRuleDrafts) def validate_draft() -> Any: @@ -137,46 +194,23 @@ def validate_draft() -> Any: if not isinstance(source_text, str): return jsonify({'error': 'source must be a string.'}), 400 - spliced = _current_sources_dict() - spliced[path] = source_text - - try: - sources = Sources.from_dict(spliced) - except Exception as exc: - # Sources.from_dict asserts on shape (e.g., missing main.sml). Surface as a structured error - # so the editor can show "you broke main.sml" without crashing. + result = _validate_draft_source(path, source_text) + if result.assemble_error is not None: + # main.sml (or another source) is broken; surface it so the editor can show it. return jsonify( { 'ok': False, - 'errors': [{'message': str(exc), 'hint': '', 'source_path': path, 'line': 0, 'column': 0}], + 'errors': [{'message': result.assemble_error, 'hint': '', 'source_path': path, 'line': 0, 'column': 0}], 'warnings': [], } ), 400 - engine = ENGINE.instance() - try: - validated = validate_sources( - sources, - udf_registry=engine.udf_registry, - validator_registry=engine.validator_registry, - ) - except ValidationFailed as exc: - formatted_errors = [_format_validation_message(e) for e in exc.errors] - return jsonify( - { - 'ok': False, - 'errors': formatted_errors, - 'warnings': [_format_validation_message(w) for w in exc.warnings], - 'suggested_imports': _suggest_imports_from_errors(path, formatted_errors), - } - ) - return jsonify( { - 'ok': True, - 'errors': [], - 'warnings': [_format_validation_message(w) for w in validated.warnings], - 'suggested_imports': [], + 'ok': result.ok, + 'errors': result.errors, + 'warnings': result.warnings, + 'suggested_imports': result.suggested_imports, } ) @@ -310,26 +344,14 @@ def vocabulary() -> Any: def _revalidate(path: str, source_text: str) -> tuple[Any, int] | None: - """Re-run the engine's AST validation with the draft spliced into the loaded - sources. Returns a Flask error tuple on failure, or None if it validates.""" - spliced = _current_sources_dict() - spliced[path] = source_text - try: - sources = Sources.from_dict(spliced) - validate_sources( - sources, - udf_registry=ENGINE.instance().udf_registry, - validator_registry=ENGINE.instance().validator_registry, - ) - except ValidationFailed as exc: - return jsonify( - { - 'error': 'Validation failed; fix errors before submitting.', - 'errors': [_format_validation_message(e) for e in exc.errors], - } - ), 400 - except Exception as exc: - return jsonify({'error': f'Could not assemble sources: {exc}'}), 400 + """Re-run validation server-side, rejecting on failure. Returns a Flask error + tuple to return, or None if the draft validates. Shares `_validate_draft_source` + with the /validate endpoint so create/deploy can't accept SML the editor rejected.""" + result = _validate_draft_source(path, source_text) + if result.assemble_error is not None: + return jsonify({'error': result.assemble_error}), 400 + if not result.ok: + return jsonify({'error': 'Validation failed; fix errors before submitting.', 'errors': result.errors}), 400 return None @@ -477,14 +499,18 @@ def deploy_draft(draft_id: int) -> Any: target = _resolve_within(rules_dir, draft.path) if target is None: return jsonify({'error': f'Draft path {draft.path!r} escapes the rules directory.'}), 400 + + # If wiring is requested, verify main.sml exists before writing anything, so a + # missing main.sml doesn't leave the rule file written while the deploy 409s. + main_path = rules_dir / 'main.sml' + if wire_into_main and not main_path.exists(): + return jsonify({'error': 'wire_into_main requested but main.sml does not exist in the rules directory.'}), 409 + target.parent.mkdir(parents=True, exist_ok=True) target.write_text(draft.sml_source, encoding='utf-8') main_sml_updated = False if wire_into_main: - main_path = rules_dir / 'main.sml' - if not main_path.exists(): - return jsonify({'error': f'wire_into_main requested but main.sml does not exist at {main_path}.'}), 409 main_contents = main_path.read_text(encoding='utf-8') if not _main_requires(main_contents, draft.path): main_path.write_text(_append_require(main_contents, draft.path), encoding='utf-8') @@ -493,7 +519,9 @@ def deploy_draft(draft_id: int) -> Any: deployed = RuleDraft.mark_deployed(draft_id) result = deployed.to_json() if deployed is not None else draft.to_json() result['main_sml_updated'] = main_sml_updated - result['path_on_disk'] = str(target) + # Report the path relative to the rules directory, not the absolute server path + # (which would leak the deployment's directory layout to the client). + result['path_on_disk'] = str(target.relative_to(rules_dir.resolve())) return jsonify(result) diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py index 2c24afd4..d0d8779f 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py @@ -5,6 +5,17 @@ from flask import Response, url_for from flask.testing import FlaskClient from osprey.worker.lib.snowflake import Snowflake +from osprey.worker.lib.storage.postgres import scoped_session +from osprey.worker.lib.storage.rule_drafts import RuleDraft + + +@pytest.fixture(autouse=True) +def _clear_rule_drafts(): + # The test database is session-scoped, so drafts persist across tests. Start each + # test with an empty table, or leaked rows trip the rule-name uniqueness check. + with scoped_session(commit=True) as session: + session.query(RuleDraft).delete() + yield @pytest.fixture(autouse=True) @@ -427,6 +438,8 @@ def test_deploy_wire_into_main_409_when_main_missing( res = client.post(url_for('rule_drafts.deploy_draft', draft_id=draft_id), json={'wire_into_main': True}) assert res.status_code == 409 + # The rule file must not be written when the deploy 409s on a missing main.sml. + assert not (tmp_path / 'rules' / 'nomain.sml').exists() @pytest.mark.use_rules_sources(_base_sources) From 4089033994ff05a8bfec03f64c8f149ebd58516a Mon Sep 17 00:00:00 2001 From: Juliet Shen Date: Sun, 19 Jul 2026 13:46:31 -0700 Subject: [PATCH 5/6] Clarify get_source: it serves on-disk rules, not drafts @ThisIsMissEm noted the /rule-drafts/source endpoint reads any deployed rule's source, not a draft's. Reword the 404 to "No rule found at ..." (it doesn't consult the drafts table, so "draft" would mislead) and add a docstring saying it's for editing an existing on-disk rule, while a draft's own SML comes from GET /rule-drafts/. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X --- .../src/osprey/worker/ui_api/osprey/views/rule_drafts.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py index 36d1e339..dd3f1ba1 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py @@ -112,6 +112,11 @@ def _current_sources_dict() -> dict[str, str]: @blueprint.route('/rule-drafts/source', methods=['GET']) @require_ability(CanEditRuleDrafts) def get_source() -> Any: + """Return the source of a rule already loaded by the engine (i.e. on disk). + + This is for editing an existing rule; it does not read the rule_drafts table. + A draft's own SML is loaded from the table via `GET /rule-drafts/`. + """ path = request.args.get('path', '').strip() err = _validate_path(path) if err: @@ -120,7 +125,7 @@ def get_source() -> Any: engine = ENGINE.instance() source: Source | None = engine.execution_graph.validated_sources.sources.get_by_path(path) if source is None: - return jsonify({'error': f'No source found at {path!r}.'}), 404 + return jsonify({'error': f'No rule found at {path!r}.'}), 404 return jsonify({'path': source.path, 'contents': source.contents}) From 2c65500d6b2fd0eef679dab5b3fc8d522d77f71d Mon Sep 17 00:00:00 2001 From: Juliet Shen Date: Tue, 21 Jul 2026 22:50:17 -0700 Subject: [PATCH 6/6] Address review: config-read rules dir, clearer path error, list default Feedback from @chimosky on the rule-drafts view: - Read OSPREY_RULES_LOCAL_PATH via CONFIG.get_str() instead of os.environ, for consistency with how the rest of the UI API reads configuration. The deploy tests set it directly on the bound config since CONFIG binds once at app setup. - Make the invalid-path error human-readable ("letters, numbers, underscores, slashes, and hyphens") instead of echoing the raw character class. - Use `[]` as the default in _suggest_imports_from_errors rather than `or []`. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X --- .../worker/ui_api/osprey/views/rule_drafts.py | 12 +++++---- .../osprey/views/tests/test_rule_drafts.py | 25 ++++++++++++++----- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py index dd3f1ba1..5af4baa5 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/rule_drafts.py @@ -1,7 +1,6 @@ from __future__ import annotations import logging -import os import re from collections.abc import Iterable from dataclasses import dataclass, field @@ -32,7 +31,7 @@ ValidationFailed, ValidationWarning, ) -from osprey.worker.lib.singletons import ENGINE +from osprey.worker.lib.singletons import CONFIG, ENGINE from osprey.worker.lib.storage.rule_drafts import RuleDraft from osprey.worker.ui_api.osprey.lib.abilities import CanEditRuleDrafts, require_ability from osprey.worker.ui_api.osprey.lib.auth import get_current_user_email @@ -85,7 +84,7 @@ def _suggest_imports_from_errors( """ suggested: set[str] = set() for err in errors: - for path in err.get('defined_in_source_paths') or []: + for path in err.get('defined_in_source_paths', []): if path == 'main.sml' or path == draft_path: continue suggested.add(path) @@ -94,7 +93,10 @@ def _suggest_imports_from_errors( def _validate_path(path: str) -> str | None: if not _VALID_PATH.match(path): - return f'Path {path!r} is not a valid SML source path (must end .sml and contain only [A-Za-z0-9_/-]).' + return ( + f'Path {path!r} is not a valid SML source path. It must end in .sml and contain only ' + 'letters, numbers, underscores, slashes, and hyphens.' + ) if path.startswith('/'): # Rule paths are relative to the rules directory; an absolute path would # escape it. Deploy also guards this with _resolve_within, but reject early. @@ -437,7 +439,7 @@ def _rules_dir_or_error() -> tuple[Path | None, tuple[Any, int] | None]: SourcesProvider that lets the engine read deployed drafts straight from this table would remove that dependency entirely; see the PR notes. """ - raw = os.environ.get('OSPREY_RULES_LOCAL_PATH', '').strip() + raw = CONFIG.instance().get_str('OSPREY_RULES_LOCAL_PATH', '').strip() if not raw: return None, ( jsonify( diff --git a/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py index d0d8779f..1d0ac5dd 100644 --- a/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py +++ b/osprey_worker/src/osprey/worker/ui_api/osprey/views/tests/test_rule_drafts.py @@ -4,11 +4,24 @@ import pytest from flask import Response, url_for from flask.testing import FlaskClient +from osprey.worker.lib.singletons import CONFIG from osprey.worker.lib.snowflake import Snowflake from osprey.worker.lib.storage.postgres import scoped_session from osprey.worker.lib.storage.rule_drafts import RuleDraft +def _set_rules_dir(monkeypatch: pytest.MonkeyPatch, path: object) -> None: + # Deploy reads OSPREY_RULES_LOCAL_PATH via CONFIG, which is bound once at app + # setup, so set it on the already-bound config for the deploy handler to see. + monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(path)) + CONFIG.instance()._config_dict['OSPREY_RULES_LOCAL_PATH'] = str(path) + + +def _unset_rules_dir(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv('OSPREY_RULES_LOCAL_PATH', raising=False) + CONFIG.instance()._config_dict.pop('OSPREY_RULES_LOCAL_PATH', None) + + @pytest.fixture(autouse=True) def _clear_rule_drafts(): # The test database is session-scoped, so drafts persist across tests. Start each @@ -364,7 +377,7 @@ def test_get_draft_returns_one_and_404s(client: 'FlaskClient[Response]') -> None def test_deploy_writes_sml_and_marks_deployed( client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: - monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + _set_rules_dir(monkeypatch, tmp_path) created = client.post( url_for('rule_drafts.create_draft'), json={'path': 'rules/deploy.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, @@ -389,7 +402,7 @@ def test_deploy_wire_into_main_appends_require( client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: (tmp_path / 'main.sml').write_text("Import(rules=['models/base.sml'])\n") - monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + _set_rules_dir(monkeypatch, tmp_path) created = client.post( url_for('rule_drafts.create_draft'), json={'path': 'rules/wired.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, @@ -409,7 +422,7 @@ def test_deploy_wire_into_main_is_idempotent( client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: (tmp_path / 'main.sml').write_text("Import(rules=['models/base.sml'])\nRequire(rule='rules/already.sml')\n") - monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + _set_rules_dir(monkeypatch, tmp_path) created = client.post( url_for('rule_drafts.create_draft'), json={'path': 'rules/already.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, @@ -428,7 +441,7 @@ def test_deploy_wire_into_main_is_idempotent( def test_deploy_wire_into_main_409_when_main_missing( client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: - monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + _set_rules_dir(monkeypatch, tmp_path) created = client.post( url_for('rule_drafts.create_draft'), json={'path': 'rules/nomain.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, @@ -444,7 +457,7 @@ def test_deploy_wire_into_main_409_when_main_missing( @pytest.mark.use_rules_sources(_base_sources) def test_deploy_503_when_rules_dir_unset(client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.delenv('OSPREY_RULES_LOCAL_PATH', raising=False) + _unset_rules_dir(monkeypatch) created = client.post( url_for('rule_drafts.create_draft'), json={'path': 'rules/nodir.sml', 'rule_name': 'SomeRule', 'source': _VALID_DRAFT, 'summary': ''}, @@ -460,6 +473,6 @@ def test_deploy_503_when_rules_dir_unset(client: 'FlaskClient[Response]', monkey def test_deploy_404_for_unknown_draft( client: 'FlaskClient[Response]', monkeypatch: pytest.MonkeyPatch, tmp_path ) -> None: - monkeypatch.setenv('OSPREY_RULES_LOCAL_PATH', str(tmp_path)) + _set_rules_dir(monkeypatch, tmp_path) res = client.post(url_for('rule_drafts.deploy_draft', draft_id=999999), json={}) assert res.status_code == 404