From 55f0f96016114ee784adb0fbc3bddbf2ada89116 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 4 Nov 2026 21:38:59 -0500 Subject: [PATCH] feat(6854): the server says which project a pending act would change, and whether this session may do it GET /api/plugin/write-scope answers the plugin's fence. The session's project comes from repo / project_id, as every plugin endpoint reads it. The act is either: - a directory (target_repo / target_project_id), for a file edit or a git command; or - an MCP call (tool, plus input: the call's short arguments). It returns allowed, the other projects the act would change, and the reason the operator is asked. It is a GET so a read-scoped key still fences file edits and git. services/write_scope.WRITE_TARGETS lists every write tool with the arguments that locate its write: a project id, or a task, note, milestone, System, rule, relation, trash batch, repo or family decision, each traced to its project. A write that lands in no project (install settings, user-level records, the judgments a session is asked for anywhere) has an empty entry. The one cross-project write allowed is create_task(kind="issue") as a new, unplanned, open issue. A project the caller cannot read is never named. tests/test_write_scope.py: - requires an entry for every write tool; - requires every declared argument to exist on the tool; - refuses a locating argument (project_id, task_id, milestone_id and the rest) that a write tool takes without declaring it. The integration test traces each locator from real rows. Milestone 583, step 2. Co-Authored-By: Claude Opus 5.5 --- docs/api-reference.md | 1 + plugin/PACKAGING.md | 2 +- src/grimoire/routes/plugin.py | 53 +++++ src/grimoire/services/write_scope.py | 307 ++++++++++++++++++++++++++ tests/test_integration_write_scope.py | 86 ++++++++ tests/test_write_scope.py | 207 +++++++++++++++++ 6 files changed, 655 insertions(+), 1 deletion(-) create mode 100644 src/grimoire/services/write_scope.py create mode 100644 tests/test_integration_write_scope.py create mode 100644 tests/test_write_scope.py diff --git a/docs/api-reference.md b/docs/api-reference.md index ee926247..afa5bf1c 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -251,6 +251,7 @@ These serve the plugin's hooks. | POST | `/api/plugin/rule-opened` | Records that a session opened a rule it was shown | | GET | `/api/plugin/shape-check` | The end-of-turn question about what this turn wrote | | GET | `/api/plugin/claim-session` · `/release-session` | Bind a session to its task claims / release them at session end | +| GET | `/api/plugin/write-scope` | Which projects a pending act would change (a directory, or an MCP call's `tool` and `input`), and whether a session in its own project may do it. Filing an issue elsewhere is allowed; every other cross-project write is answered `allowed: false` with the reason the operator is asked | | GET | `/api/plugin/processes` | Stored Processes for skill-stub sync | | GET | `/api/plugin/prior-art` | Write-path hint for the plugin hooks (params: `path`, `code`, `repo`, `shapes`, `exclude_ids`, `exclude_sync_ids`, `exclude_derive`); returns `context`, `note_ids`, `sync_note_ids`, `stamped`, `divergence`, `derive`, `derive_keys` | | GET / POST | `/api/projects//coverage`, `…/coverage/refresh` | Shape-ledger accounting (`pattern_coverage` line, counts, `derive_groups` — css groups carry `consumers`, `derive_new`, `unused_css`, `divergence`, `recheck`) | diff --git a/plugin/PACKAGING.md b/plugin/PACKAGING.md index f70bfc8d..9ee37e9a 100644 --- a/plugin/PACKAGING.md +++ b/plugin/PACKAGING.md @@ -15,7 +15,7 @@ another one means adding files, not moving or rewriting any. |---|---|---| | **The skills** | `plugin/skills/*/SKILL.md` | Agent Skills (the open SKILL.md format). They state every Grimoire reflex in full and name no client. `tests/test_guidance_ownership.py` fails if a skill names a particular client, or references anything outside its own folder. Every client package ships this folder verbatim. | | **The MCP server** | `/mcp` | HTTP, `Authorization: Bearer `. Its `_INSTRUCTIONS` is a client-neutral orientation — the workflow across tools (≤1,600 chars); each tool's description carries its contract; in-band responses (`placement`, `report_back`, `systems_hint`, the duplicate gate, the guessed-id refusal) fire in every client. | -| **The adapter API** | `/api/plugin/*` | Endpoints any client's hooks can call with the same key. `GET`, so read scope is enough: `context` (live session state), `retrieve` (rules, preferences and notes for a message), `prior-art` (records and shape-ledger hints for code being written), `tool-rules` (rules for a command about to run), `moment-tools` (which tools reach a mounted moment), `shape-check` (definitions written this turn that nobody has judged), `claim-session` / `release-session` (task claims), `processes` (stored Processes to expose as skills). `POST`, so write scope: `moment` (rules mounted on the moments a tool call reaches), `rule-opened` (a rule was read), `reply-rules` (the finished reply, held for an unopened rule or — when the turn closed a task — for missing completion sections). | +| **The adapter API** | `/api/plugin/*` | Endpoints any client's hooks can call with the same key. `GET`, so read scope is enough: `context` (live session state), `retrieve` (rules, preferences and notes for a message), `prior-art` (records and shape-ledger hints for code being written), `tool-rules` (rules for a command about to run), `moment-tools` (which tools reach a mounted moment), `shape-check` (definitions written this turn that nobody has judged), `claim-session` / `release-session` (task claims), `write-scope` (which project a pending write lands in, and whether this session may make it), `processes` (stored Processes to expose as skills). `POST`, so write scope: `moment` (rules mounted on the moments a tool call reaches), `rule-opened` (a rule was read), `reply-rules` (the finished reply, held for an unopened rule or — when the turn closed a task — for missing completion sections). | | **The API key** | Grimoire → Settings → MCP Access | One `fmcp_` key per install. Write scope covers the MCP tools and every hook; a read key runs only the `GET` hooks. | ## Added by each client diff --git a/src/grimoire/routes/plugin.py b/src/grimoire/routes/plugin.py index 3bf51eae..27a6fb47 100644 --- a/src/grimoire/routes/plugin.py +++ b/src/grimoire/routes/plugin.py @@ -8,6 +8,8 @@ fmcp_` API key (a read-scoped key suffices for this GET). """ from __future__ import annotations +import json + from quart import Blueprint, g, jsonify, request from grimoire.auth import admin_required, get_current_user_id, login_required @@ -21,6 +23,8 @@ from grimoire.services import report_check as report_check_svc from grimoire.services import rule_moment_judgments as judgments_svc from grimoire.services import shape_check as shape_check_svc from grimoire.services import task_claims as task_claims_svc +from grimoire.services import write_scope as write_scope_svc +from grimoire.services.access import can_read_project from grimoire.services.embeddings import memoized_query_embeddings from grimoire.services.settings import get_admin_setting, set_setting @@ -626,6 +630,55 @@ async def claim_session(): return jsonify({"claim": claim}) +@plugin_bp.get("/write-scope") +@login_required +async def write_scope(): + """May this session do this act? Asked by `grimoire_project_fence.sh` + before a file edit, a git command or a Grimoire write. + + A session belongs to the project its starting directory resolves to + (`repo` / `project_id`, as every plugin endpoint reads them). The act is + either a directory (`target_repo` / `target_project_id`) or an MCP call + (`tool`, with `input` as a JSON object of the call's short arguments: the + ids, kind and status that say where it writes). `act` words the act for the + reason, e.g. "git push in". + + Returns `{allowed, session_project, targets, issue, reason}` from + services/write_scope.check_act. A GET, so a read-scoped key still fences + file edits and git commands. + """ + session_project, _repo, _unbound = await _project_scope() + if session_project and not await can_read_project(g.user.id, session_project): + session_project = 0 + tool = (request.args.get("tool") or "").strip() + if tool.startswith("mcp__"): + tool = tool.rsplit("__", 1)[-1] + args: dict = {} + if tool: + try: + parsed = json.loads(request.args.get("input") or "{}") + except ValueError: + return jsonify({"error": "input must be a JSON object"}), 400 + if not isinstance(parsed, dict): + return jsonify({"error": "input must be a JSON object"}), 400 + args = parsed + target_project = 0 + if not tool: + try: + target_project = int(request.args.get("target_project_id") or 0) + except ValueError: + target_project = 0 + target_repo = (request.args.get("target_repo") or "").strip() + if not target_project and target_repo: + target_project = await repo_bindings_svc.resolve_project(g.user.id, target_repo) or 0 + result = await write_scope_svc.check_act( + g.user.id, session_project, tool=tool, args=args, + target_project=target_project, act=(request.args.get("act") or "")[:80], + ) + result["session_project"] = session_project or None + return jsonify(result) + + @plugin_bp.get("/release-session") @login_required async def release_session(): diff --git a/src/grimoire/services/write_scope.py b/src/grimoire/services/write_scope.py new file mode 100644 index 00000000..55dddb17 --- /dev/null +++ b/src/grimoire/services/write_scope.py @@ -0,0 +1,307 @@ +"""Which project a write lands in, so a session can be held to its own. + +A Claude Code session belongs to one project: the one its starting directory +is bound to. Toward any other project it may read anything and file an issue, +and every other write waits for the operator. The plugin's PreToolUse hook +enforces that, because only the hook knows which session is acting: the MCP +endpoint is stateless and sees nothing but the caller's key. This module +answers the hook's one question: given the session's project and a pending +act, which projects would the act change, and may it go ahead? + +An act is either a directory (a file edit or a git command; the hook resolves +it to a repo or a `.grimoire` marker) or an MCP tool call. For a tool call, +`WRITE_TARGETS` says, for every write tool, which of its arguments locate the +write. That means the record or project the write changes, never one it only +mentions: `arose_from_id` on a new task points at another task without +changing it. tests/test_write_scope.py holds the table to the registered tools, +so a new write tool can't be added without saying where it writes. +""" +from __future__ import annotations + +from sqlalchemy import or_, select + +# How an argument locates the project a write lands in. +PROJECT = "project" # a project id +NOTE = "note" # a task, note, lesson, snippet or process: its project +MILESTONE = "milestone" # a milestone: its project +SYSTEM = "system" # a System: its project +RULE = "rule" # a project rule: its project. A global rule: none +RELATION = "relation" # a rule relation: the projects of its two rules +TRASH = "trash" # a deleted batch: every project its rows belong to +REPO = "repo" # a repo remote: the project it is bound to +DECISION = "decision" # a family decision: its project, else its idea's home + +LOCATORS = frozenset({PROJECT, NOTE, MILESTONE, SYSTEM, RULE, RELATION, TRASH, REPO, DECISION}) + +# Every write tool, and the arguments that locate its write. An empty entry is +# a write that lands in no project: +# - install-level settings (moment mappings, retrieval dials); +# - user-level records (processes, design systems, rulebooks, topics, global +# rules and preferences); +# - judgments the system asks a session for in any project (a rule's outcome, +# a lesson-to-rule link, moment proposals, a menu verdict). Those describe how +# the corpus is retrieved; they don't change another project's work. +WRITE_TARGETS: dict[str, dict[str, str]] = { + # notes, tasks, planning + "create_note": {"project_id": PROJECT}, + "update_note": {"note_id": NOTE, "project_id": PROJECT}, + "delete_note": {"note_id": NOTE}, + "mark_note_verified": {"note_id": NOTE}, + "create_lesson": {"project_id": PROJECT}, + "update_lesson": {"lesson_id": NOTE}, + "delete_lesson": {"lesson_id": NOTE}, + "judge_lesson_link": {}, + "create_task": {"project_id": PROJECT, "milestone_id": MILESTONE, "parent_id": NOTE}, + "update_task": {"task_id": NOTE, "project_id": PROJECT, "milestone_id": MILESTONE}, + "delete_task": {"task_id": NOTE}, + "add_task_log": {"task_id": NOTE}, + "create_records": {"project_id": PROJECT, "milestone_id": MILESTONE}, + "start_planning": {"project_id": PROJECT}, + "create_milestone": {"project_id": PROJECT}, + "update_milestone": {"milestone_id": MILESTONE, "project_id": PROJECT}, + "delete_milestone": {"milestone_id": MILESTONE}, + # projects, Systems, repos + "create_project": {}, + "update_project": {"project_id": PROJECT}, + "delete_project": {"project_id": PROJECT}, + "decide_project_inception": {"project_id": PROJECT}, + "set_project_platforms": {"project_id": PROJECT}, + "create_system": {"project_id": PROJECT}, + "update_system": {"system_id": SYSTEM}, + "delete_system": {"system_id": SYSTEM}, + "map_system_to_canonical": {"system_id": SYSTEM}, + # Binding a repo changes the project it's bound to and, when it was bound + # elsewhere, the project it leaves. + "bind_repo": {"project_id": PROJECT, "repo_url": REPO}, + "unbind_repo": {"repo_url": REPO}, + # family canon. An idea is a record in its home project; an adoption is a + # project's answer; a conflict is between the two projects it names. + "propose_family_idea": {"note_id": NOTE}, + "promote_family_idea": {"note_id": NOTE}, + "retire_family_idea": {"note_id": NOTE}, + "revise_family_idea": {"note_id": NOTE}, + "set_family_references": {"note_id": NOTE}, + "set_family_topic": {"note_id": NOTE}, + "undo_family_decision": {"decision_id": DECISION}, + "assess_family_adoption": {"project_id": PROJECT}, + "resolve_family_conflict": {"canon_project_id": PROJECT, "other_project_id": PROJECT}, + # snippets, processes, the shape ledger + "create_snippet": {"project_id": PROJECT}, + "update_snippet": {"snippet_id": NOTE, "project_id": PROJECT}, + "delete_snippet": {"snippet_id": NOTE}, + "verify_snippet": {"snippet_id": NOTE}, + "merge_snippets": {"target_id": NOTE, "source_ids": NOTE}, + "unmerge_snippet": {"survivor_id": NOTE, "source_id": NOTE}, + "create_process": {}, + "update_process": {"process_id": NOTE}, + "delete_process": {"process_id": NOTE}, + "classify_shapes": {"project_id": PROJECT}, + "classify_shapes_by_rule": {"project_id": PROJECT}, + "confirm_shape_proposals": {"project_id": PROJECT}, + "refresh_pattern_coverage": {"project_id": PROJECT}, + "move_path": {"project_id": PROJECT}, + # design systems belong to their owner, not to a project; choosing one for + # a project is that project's write. + "create_design_system": {}, + "update_design_system": {}, + "delete_design_system": {}, + "create_design_token": {}, + "update_design_token": {}, + "delete_design_token": {}, + "set_project_design_system": {"project_id": PROJECT}, + # rules + "create_rulebook": {}, + "update_rulebook": {}, + "delete_rulebook": {}, + "create_topic": {}, + "update_topic": {}, + "delete_topic": {}, + "create_rule": {}, + "create_preference": {}, + "create_project_rule": {"project_id": PROJECT}, + "update_rule": {"rule_id": RULE}, + "update_preference": {"rule_id": RULE}, + "move_rule": {"rule_id": RULE, "project_id": PROJECT}, + "delete_rule": {"rule_id": RULE}, + "mark_rule_verified": {"rule_id": RULE}, + "relate_rules": {"from_rule_id": RULE, "to_rule_id": RULE}, + "unrelate_rules": {"relation_id": RELATION}, + "rule_outcome": {}, + # moments and retrieval: install-level, and judgments asked for anywhere + "tune_retrieval": {}, + "migrate_retrieval_floor": {}, + "map_action": {}, + "unmap_action": {}, + "propose_rule_moments": {}, + "judge_rule_moments": {}, + "rule_misfired": {}, + "judge_menu": {}, + # trash: a batch can hold rows from several projects + "restore": {"deleted_batch_id": TRASH}, + "purge_trash": {"deleted_batch_id": TRASH}, +} + +# Argument names that would locate a write wherever they appear. The guard test +# requires every write tool parameter with one of these names to be declared +# above, so a tool that gains one can't write past the fence unnoticed. +LOCATOR_NAMES = frozenset({ + "project_id", "task_id", "note_id", "lesson_id", "snippet_id", "process_id", + "milestone_id", "system_id", "rule_id", "deleted_batch_id", +}) + +# The one write a session may make in another project: filing an issue there, +# as a new open task outside any plan of theirs. +ISSUE_STATUSES = frozenset({"", "todo"}) + + +def is_issue_filing(tool: str, args: dict) -> bool: + """True for create_task(kind="issue") that opens a fresh, unplanned issue.""" + if tool != "create_task": + return False + if str(args.get("kind") or "") != "issue": + return False + if str(args.get("status") or "") not in ISSUE_STATUSES: + return False + return not _positive_ints(args.get("milestone_id")) and not _positive_ints(args.get("parent_id")) + + +def _positive_ints(value) -> list[int]: + """The ids an argument names. 0 and negative values mean "none" or "leave + unchanged" across the tools (update_task's project_id=-1 clears), so only + positive ids locate anything.""" + values = value if isinstance(value, (list, tuple)) else [value] + out = [] + for v in values: + try: + n = int(v) + except (TypeError, ValueError): + continue + if n > 0: + out.append(n) + return out + + +async def _projects_of(user_id: int, kind: str, value) -> set[int]: + """The projects the argument `value` of locator `kind` lands in.""" + from grimoire.models import async_session + from grimoire.models.family import FamilyDecision + from grimoire.models.milestone import Milestone + from grimoire.models.note import Note + from grimoire.models.rulebook import Rule, RuleRelation + from grimoire.models.system import System + from grimoire.services import repo_bindings + + if kind == REPO: + found = await repo_bindings.resolve_project(user_id, str(value or "")) + return {found} if found else set() + if kind == TRASH: + batch = str(value or "").strip() + if not batch: + return set() + async with async_session() as session: + found: set[int] = set() + for model in (Note, Milestone, System, Rule): + rows = await session.execute( + select(model.project_id).where(model.deleted_batch_id == batch) + ) + found.update(p for p in rows.scalars() if p) + return found + ids = _positive_ints(value) + if not ids: + return set() + if kind == PROJECT: + return set(ids) + async with async_session() as session: + if kind == RELATION: + rows = await session.execute( + select(Rule.project_id) + .join(RuleRelation, or_(RuleRelation.from_rule_id == Rule.id, + RuleRelation.to_rule_id == Rule.id)) + .where(RuleRelation.id.in_(ids)) + ) + return {p for p in rows.scalars() if p} + if kind == DECISION: + rows = await session.execute( + select(FamilyDecision.project_id, Note.project_id) + .join(Note, Note.id == FamilyDecision.idea_id) + .where(FamilyDecision.id.in_(ids)) + ) + return {own or home for own, home in rows.all() if own or home} + model = {NOTE: Note, MILESTONE: Milestone, SYSTEM: System, RULE: Rule}[kind] + rows = await session.execute(select(model.project_id).where(model.id.in_(ids))) + return {p for p in rows.scalars() if p} + + +async def tool_targets(user_id: int, tool: str, args: dict) -> set[int]: + """Every project the MCP call `tool(**args)` would change. Empty for a + read, a write that lands in no project, and a tool this server doesn't + have.""" + found: set[int] = set() + for name, kind in WRITE_TARGETS.get(tool, {}).items(): + if name in args: + found |= await _projects_of(user_id, kind, args[name]) + return found + + +async def _titles(ids: list[int]) -> dict[int, str]: + from grimoire.models import async_session + from grimoire.models.project import Project + + if not ids: + return {} + async with async_session() as session: + rows = await session.execute(select(Project.id, Project.title).where(Project.id.in_(ids))) + return {pid: title for pid, title in rows.all()} + + +def _named(project_id: int, titles: dict[int, str]) -> str: + title = titles.get(project_id) + return f"{title} (project {project_id})" if title else f"project {project_id}" + + +async def check_act( + user_id: int, session_project: int, *, tool: str = "", args: dict | None = None, + target_project: int = 0, act: str = "", +) -> dict: + """May a session in `session_project` do this act? + + The act is an MCP call (`tool` with `args`) or a directory already + resolved to `target_project`; `act` words it for the reason ("git push + in", "Editing a file in"). Returns: + - `allowed`: False when the act would change another project and isn't the + one cross-project write allowed; + - `targets`: the other projects, as {id, title}; + - `issue`: the act is that allowed write, filing an issue; + - `reason`: what the operator reads in the prompt, and the session after. + + A session with no project is held to nothing. A target the caller can't + read is left out: the write itself is refused there, so there is nothing + to ask about, and naming it would tell the caller where a record lives. + """ + from grimoire.services.access import can_read_project + + args = args or {} + if tool: + landing = await tool_targets(user_id, tool, args) + else: + landing = {target_project} if target_project else set() + others = [ + p for p in sorted(landing) + if p != session_project and await can_read_project(user_id, p) + ] + if not session_project or not others: + return {"allowed": True, "targets": [], "issue": False, "reason": ""} + titles = await _titles([session_project, *others]) + targets = [{"id": p, "title": titles.get(p, "")} for p in others] + if tool and is_issue_filing(tool, args): + return {"allowed": True, "targets": targets, "issue": True, "reason": ""} + there = ", ".join(_named(p, titles) for p in others) + what = act.strip() or (f"{tool} changes" if tool else "This changes") + first = others[0] + reason = ( + f"This session belongs to {_named(session_project, titles)}. {what} {there}, " + f"a different project. From here, the one change to make there is filing an " + f"issue: create_task(project_id={first}, kind=\"issue\") with how it was found " + f"and the evidence. Approve only if this session should change {there} itself." + ) + return {"allowed": False, "targets": targets, "issue": False, "reason": reason} diff --git a/tests/test_integration_write_scope.py b/tests/test_integration_write_scope.py new file mode 100644 index 00000000..21b53b3d --- /dev/null +++ b/tests/test_integration_write_scope.py @@ -0,0 +1,86 @@ +"""Real-Postgres proof that a write is traced to the project it lands in. + +Two projects of one user: the session's own and another. Each locator kind is +resolved from a real row, and check_act answers for a session standing in +the first. +""" +import pytest +import pytest_asyncio +from sqlalchemy import select + +from grimoire.models import async_session +from grimoire.models.milestone import Milestone +from grimoire.models.note import Note +from grimoire.models.project import Project +from grimoire.models.system import System +from grimoire.services import write_scope +from tests.helpers import ensure_user + +pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")] + +OWNER = "write_scope_owner" +BATCH = "write-scope-test-batch" + + +@pytest_asyncio.fixture +async def two_projects(): + async with async_session() as s: + user = await ensure_user(s, OWNER) + for model in (Note, Milestone, System, Project): + for row in (await s.execute(select(model).where(model.user_id == user.id))).scalars(): + await s.delete(row) + await s.flush() + home = Project(user_id=user.id, title="Home") + other = Project(user_id=user.id, title="Elsewhere") + s.add_all([home, other]) + await s.flush() + plan = Milestone(user_id=user.id, project_id=other.id, title="Their plan") + area = System(user_id=user.id, project_id=other.id, name="Their area") + s.add_all([plan, area]) + await s.flush() + theirs = Note(user_id=user.id, project_id=other.id, milestone_id=plan.id, + title="Their task", body="", note_type="note", task_kind="work", + status="todo") + mine = Note(user_id=user.id, project_id=home.id, title="My task", body="", + note_type="note", task_kind="work", status="todo") + trashed = Note(user_id=user.id, project_id=other.id, title="Trashed", body="", + note_type="note", deleted_batch_id=BATCH) + s.add_all([theirs, mine, trashed]) + await s.commit() + return {"user": user.id, "home": home.id, "other": other.id, "plan": plan.id, + "area": area.id, "theirs": theirs.id, "mine": mine.id} + + +async def test_each_locator_finds_the_project_of_the_row_it_names(two_projects): + ids = two_projects + other, home, uid = ids["other"], ids["home"], ids["user"] + + async def lands(tool, **args): + return await write_scope.tool_targets(uid, tool, args) + + assert await lands("add_task_log", task_id=ids["theirs"]) == {other} + assert await lands("update_task", task_id=ids["mine"]) == {home} + assert await lands("update_task", task_id=ids["mine"], milestone_id=ids["plan"]) == {home, other} + assert await lands("delete_milestone", milestone_id=ids["plan"]) == {other} + assert await lands("update_system", system_id=ids["area"]) == {other} + assert await lands("restore", deleted_batch_id=BATCH) == {other} + assert await lands("create_rule", topic_id=1) == set() + + +async def test_a_session_in_one_project_is_asked_before_writing_in_the_other(two_projects): + ids = two_projects + uid, home, other = ids["user"], ids["home"], ids["other"] + + own = await write_scope.check_act(uid, home, tool="add_task_log", + args={"task_id": ids["mine"]}) + assert own["allowed"] is True + + theirs = await write_scope.check_act(uid, home, tool="add_task_log", + args={"task_id": ids["theirs"]}) + assert theirs["allowed"] is False + assert theirs["targets"] == [{"id": other, "title": "Elsewhere"}] + assert "Home" in theirs["reason"] and "Elsewhere" in theirs["reason"] + + issue = await write_scope.check_act(uid, home, tool="create_task", + args={"project_id": other, "kind": "issue"}) + assert issue["allowed"] is True and issue["issue"] is True diff --git a/tests/test_write_scope.py b/tests/test_write_scope.py new file mode 100644 index 00000000..49d3b1b0 --- /dev/null +++ b/tests/test_write_scope.py @@ -0,0 +1,207 @@ +"""Where a write lands, so the plugin can hold a session to its own project. + +The guards here keep services/write_scope.WRITE_TARGETS whole: every write +tool says which of its arguments locate the write, and an argument that would +locate one can't be left out. The rest covers the one cross-project write +allowed (filing an issue) and the answer check_act gives. +""" +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest +from quart import g + +from grimoire.services import write_scope +from grimoire.services.write_scope import ( + LOCATOR_NAMES, LOCATORS, WRITE_TARGETS, check_act, is_issue_filing, +) + + +def _write_tool_params() -> dict[str, set[str]]: + from grimoire.mcp.server import _WRITE_TOOLS, build_mcp_server + + manager = build_mcp_server()._tool_manager + return { + name: set(manager.get_tool(name).parameters.get("properties", {})) + for name in _WRITE_TOOLS + } + + +def test_every_write_tool_says_where_it_writes(): + from grimoire.mcp.server import _WRITE_TOOLS + + missing = sorted(_WRITE_TOOLS - set(WRITE_TARGETS)) + assert not missing, ( + f"write tools with no entry in WRITE_TARGETS: {missing}. Say which " + "arguments locate each one's write, or {} when it lands in no project." + ) + extra = sorted(set(WRITE_TARGETS) - _WRITE_TOOLS) + assert not extra, f"WRITE_TARGETS names tools that aren't write tools: {extra}" + + +def test_every_declared_argument_is_one_the_tool_takes_and_has_a_known_kind(): + params = _write_tool_params() + for tool, locators in WRITE_TARGETS.items(): + for arg, kind in locators.items(): + assert arg in params[tool], f"{tool} has no argument {arg!r}" + assert kind in LOCATORS, f"{tool}.{arg}: unknown locator {kind!r}" + + +def test_an_argument_that_would_locate_a_write_cannot_be_left_out(): + params = _write_tool_params() + undeclared = sorted( + f"{tool}.{arg}" + for tool, names in params.items() + for arg in names & LOCATOR_NAMES + if arg not in WRITE_TARGETS[tool] + ) + assert not undeclared, ( + f"write tools take these locating arguments without declaring them in " + f"WRITE_TARGETS: {undeclared}" + ) + + +def test_the_locator_scan_sees_the_arguments_it_guards(): + # The guard above passes vacuously if the scan finds no parameters at all. + params = _write_tool_params() + assert "task_id" in params["update_task"] & LOCATOR_NAMES + assert "project_id" in params["create_task"] & LOCATOR_NAMES + + +@pytest.mark.parametrize("args, expected", [ + ({"project_id": 7, "kind": "issue"}, True), + ({"project_id": 7, "kind": "issue", "status": "todo"}, True), + ({"project_id": 7, "kind": "work"}, False), + ({"project_id": 7}, False), + ({"project_id": 7, "kind": "issue", "status": "in_progress"}, False), + ({"project_id": 7, "kind": "issue", "milestone_id": 3}, False), + ({"project_id": 7, "kind": "issue", "parent_id": 9}, False), + ({"project_id": 7, "kind": "issue", "milestone_id": 0}, True), +]) +def test_filing_an_issue_is_a_new_open_unplanned_issue(args, expected): + assert is_issue_filing("create_task", args) is expected + + +def test_only_create_task_files_an_issue(): + assert is_issue_filing("update_task", {"task_id": 4, "kind": "issue"}) is False + assert is_issue_filing("create_records", {"project_id": 7, "kind": "issue"}) is False + + +async def test_ids_of_zero_or_below_locate_nothing(): + # update_task's project_id=0 leaves it unchanged and -1 clears it. + assert await write_scope.tool_targets(1, "update_task", {"project_id": -1}) == set() + assert await write_scope.tool_targets(1, "create_task", {"project_id": 0}) == set() + assert await write_scope.tool_targets(1, "create_note", {"project_id": 12}) == {12} + + +async def test_reads_and_unknown_tools_land_nowhere(): + assert await write_scope.tool_targets(1, "get_task", {"task_id": 5}) == set() + assert await write_scope.tool_targets(1, "no_such_tool", {"project_id": 5}) == set() + + +def _patched(landing: set[int], readable: bool = True): + return ( + patch.object(write_scope, "tool_targets", AsyncMock(return_value=landing)), + patch("grimoire.services.access.can_read_project", AsyncMock(return_value=readable)), + patch.object(write_scope, "_titles", AsyncMock(return_value={2: "Home", 9: "Elsewhere"})), + ) + + +async def test_a_write_in_the_sessions_own_project_goes_ahead(): + a, b, c = _patched({2}) + with a, b, c: + out = await check_act(1, 2, tool="update_task", args={"task_id": 5}) + assert out == {"allowed": True, "targets": [], "issue": False, "reason": ""} + + +async def test_a_write_in_another_project_asks_and_says_how_to_file_an_issue_instead(): + a, b, c = _patched({9}) + with a, b, c: + out = await check_act(1, 2, tool="update_task", args={"task_id": 5}) + assert out["allowed"] is False + assert out["targets"] == [{"id": 9, "title": "Elsewhere"}] + assert "Home (project 2)" in out["reason"] + assert "Elsewhere (project 9)" in out["reason"] + assert 'create_task(project_id=9, kind="issue")' in out["reason"] + + +async def test_filing_an_issue_in_another_project_goes_ahead(): + a, b, c = _patched({9}) + with a, b, c: + out = await check_act(1, 2, tool="create_task", args={"project_id": 9, "kind": "issue"}) + assert out["allowed"] is True and out["issue"] is True + + +async def test_a_session_with_no_project_is_held_to_nothing(): + a, b, c = _patched({9}) + with a, b, c: + out = await check_act(1, 0, tool="update_task", args={"task_id": 5}) + assert out["allowed"] is True + + +async def test_a_project_the_caller_cannot_read_is_not_named(): + a, b, c = _patched({9}, readable=False) + with a, b, c: + out = await check_act(1, 2, tool="update_task", args={"task_id": 5}) + assert out == {"allowed": True, "targets": [], "issue": False, "reason": ""} + + +async def test_a_directory_in_another_project_asks_in_the_words_of_the_act(): + with patch("grimoire.services.access.can_read_project", AsyncMock(return_value=True)), \ + patch.object(write_scope, "_titles", AsyncMock(return_value={2: "Home", 9: "Elsewhere"})): + out = await check_act(1, 2, target_project=9, act="git push in") + same = await check_act(1, 2, target_project=2, act="git push in") + assert out["allowed"] is False + assert "git push in Elsewhere (project 9)" in out["reason"] + assert same["allowed"] is True + + +async def test_the_route_reads_the_session_the_tool_and_its_input(): + from grimoire.app import create_app + from grimoire.routes import plugin + + app = create_app() + answer = {"allowed": False, "targets": [{"id": 9, "title": "x"}], "issue": False, "reason": "r"} + check = AsyncMock(return_value=dict(answer)) + url = ("/api/plugin/write-scope?project_id=2" + "&tool=mcp__plugin_grimoire_grimoire__update_task" + "&input=%7B%22task_id%22%3A5%7D&act=x") + async with app.test_request_context(url): + g.user = SimpleNamespace(id=1) + with patch.object(plugin.write_scope_svc, "check_act", check), \ + patch.object(plugin, "can_read_project", AsyncMock(return_value=True)): + resp = await plugin.write_scope.__wrapped__() + body = await resp.get_json() + assert body["allowed"] is False and body["session_project"] == 2 + _, kwargs = check.call_args + assert check.call_args.args == (1, 2) + assert kwargs["tool"] == "update_task" and kwargs["args"] == {"task_id": 5} + + +async def test_the_route_resolves_a_target_repo_to_its_project(): + from grimoire.app import create_app + from grimoire.routes import plugin + + app = create_app() + check = AsyncMock(return_value={"allowed": True, "targets": [], "issue": False, "reason": ""}) + url = "/api/plugin/write-scope?project_id=2&target_repo=https://forge.example/o/r.git&act=git%20push%20in" + async with app.test_request_context(url): + g.user = SimpleNamespace(id=1) + with patch.object(plugin.write_scope_svc, "check_act", check), \ + patch.object(plugin, "can_read_project", AsyncMock(return_value=True)), \ + patch.object(plugin.repo_bindings_svc, "resolve_project", AsyncMock(return_value=9)): + await plugin.write_scope.__wrapped__() + assert check.call_args.kwargs["target_project"] == 9 + assert check.call_args.kwargs["act"] == "git push in" + + +async def test_the_route_refuses_input_that_is_not_an_object(): + from grimoire.app import create_app + from grimoire.routes import plugin + + app = create_app() + async with app.test_request_context("/api/plugin/write-scope?tool=update_task&input=%5B1%5D"): + g.user = SimpleNamespace(id=1) + with patch.object(plugin, "can_read_project", AsyncMock(return_value=True)): + resp, status = await plugin.write_scope.__wrapped__() + assert status == 400