CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / integration (push) Successful in 49s
CI & Build / TypeScript typecheck (push) Successful in 57s
CI & Build / Python tests (push) Failing after 1m3s
CI & Build / Build & push image (push) Skipped
A rule's home is its scope now: a rule in a rulebook topic is global, a rule on a project applies to that project, and retrieval reads that directly (#4074). A subscription had stopped changing anything a session received; a suppression muted rules from a subscription. Operator, 2026-09-15: "we have global and project scoped rules, we don't need the subscriptions now." What goes, whole (rule 22): - Migration 0101 drops project_rulebook_subscriptions, project_rule_suppressions and project_topic_suppressions, and strips subscribe_rulebooks (and 394's leftover exclude_always_on_rulebooks) from stored inception choices. - Service, MCP and REST: subscribe/unsubscribe and the four suppress/unsuppress operations. The Subscribers checklist, the subscribe chips, the skip buttons and the Suppressed section in the rules UI. - Inception asks two questions (design system, seed Systems). create_project and decide_project_inception lose subscribe_rulebooks. - Backup v15 stops exporting the three sections; older archives still restore, the keys simply unread. Trash no longer hard-deletes suppression rows. What changes meaning: - get_applicable_rules is a project's LISTING: its own rules, plus the global rules tagged to an area it works in. Untagged global rules apply everywhere and arrive by retrieval, so they are not listed. A co_surfaces partner on a different project is not dragged in. - list_rules(project_id) lists that project's own rules. - rules_payload drops subscribed_rulebooks and suppressed_*; the handshake's brief form is project_rules alone. - using-scribe's "Where a new rule goes" and inception sections, tool docstrings and docs say global vs project. Plugin 2026.09.15.1620. Milestone 414 step 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
451 lines
18 KiB
Python
451 lines
18 KiB
Python
"""Tests for MCP rulebook tools — patches the service layer."""
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
from tests.helpers import FakeMCP, fake_rule, fake_rulebook, fake_topic
|
|
from tests.helpers import plain_rule_detail as _plain_detail
|
|
|
|
|
|
pytestmark = pytest.mark.usefixtures("_bind_user")
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_list_rulebooks_wraps_in_dict():
|
|
rows = [fake_rulebook(id=1, title="t"), fake_rulebook(id=2, title="t")]
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.list_rulebooks",
|
|
AsyncMock(return_value=rows),
|
|
):
|
|
from scribe.mcp.tools.rulebooks import list_rulebooks
|
|
out = await list_rulebooks()
|
|
assert len(out["rulebooks"]) == 2
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_get_rulebook_includes_topics():
|
|
rb = fake_rulebook(id=1, title="t")
|
|
topics = [fake_topic(id=10, title="git"), fake_topic(id=11, title="git")]
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rulebook",
|
|
AsyncMock(return_value=rb),
|
|
), patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.list_topics",
|
|
AsyncMock(return_value=topics),
|
|
):
|
|
from scribe.mcp.tools.rulebooks import get_rulebook
|
|
out = await get_rulebook(rulebook_id=1)
|
|
assert out["id"] == 1
|
|
assert len(out["topics"]) == 2
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_get_rulebook_raises_when_not_found():
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rulebook",
|
|
AsyncMock(return_value=None),
|
|
):
|
|
from scribe.mcp.tools.rulebooks import get_rulebook
|
|
with pytest.raises(ValueError, match="rulebook 999 not found"):
|
|
await get_rulebook(rulebook_id=999)
|
|
|
|
|
|
# _plain_detail moved to tests/helpers on its second copy (#2825's own
|
|
# reason for existing): two stubs for one seam drift apart quietly, and a
|
|
# test stubbing it slightly differently asserts something slightly different
|
|
# than it appears to.
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_rule_passes_required_fields():
|
|
rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
|
|
mock = AsyncMock(return_value=rule)
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_rule", mock), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_rule
|
|
await create_rule(
|
|
topic_id=10, title="dev is home", statement="Work directly on dev",
|
|
)
|
|
kwargs = mock.call_args.kwargs
|
|
assert kwargs["user_id"] == 7
|
|
assert kwargs["topic_id"] == 10
|
|
assert kwargs["statement"] == "Work directly on dev"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_rule_blocked_by_duplicate_gate():
|
|
from scribe.services.dedup import DuplicateMatch
|
|
dup = DuplicateMatch(id=47, title="dev is home", similarity=1.0, reason="title")
|
|
create_mock = AsyncMock()
|
|
with patch("scribe.mcp.tools.rulebooks.dedup_svc.find_duplicate_rule",
|
|
AsyncMock(return_value=dup)), \
|
|
patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_rule", create_mock):
|
|
from scribe.mcp.tools.rulebooks import create_rule
|
|
out = await create_rule(topic_id=10, title="dev is home", statement="x")
|
|
assert out["duplicate"] is True
|
|
assert out["existing_id"] == 47
|
|
assert "update_rule" in out["message"]
|
|
create_mock.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_rule_force_bypasses_duplicate_gate():
|
|
find_mock = AsyncMock()
|
|
with patch("scribe.mcp.tools.rulebooks.dedup_svc.find_duplicate_rule", find_mock), \
|
|
patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_rule",
|
|
AsyncMock(return_value=fake_rule(id=5, title="r", statement="s", topic_id=10))), \
|
|
_plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_rule
|
|
out = await create_rule(topic_id=10, title="dev is home", statement="x", force=True)
|
|
assert out["id"] == 5
|
|
find_mock.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_update_rule_only_sends_non_default_fields():
|
|
rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
|
|
mock = AsyncMock(return_value=rule)
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.update_rule", mock), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import update_rule
|
|
await update_rule(rule_id=1, statement="new statement")
|
|
args, kwargs = mock.call_args
|
|
assert args == (1, 7)
|
|
# `clear` is always forwarded — an empty tuple is "clear nothing", which is
|
|
# a value, not an absent argument. Everything the caller left at its
|
|
# default stays out: that is the property this test pins.
|
|
assert kwargs == {"statement": "new statement", "clear": ()}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_update_rule_forwards_the_fields_named_for_clearing():
|
|
"""Naming a field is the only way to empty it through this door.
|
|
|
|
"" means "leave unchanged" here, so a caller has no value that means
|
|
"remove it" — which is what makes an explicit list necessary and what
|
|
stops a partial update from wiping the fields it did not mention.
|
|
"""
|
|
rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
|
|
mock = AsyncMock(return_value=rule)
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.update_rule", mock), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import update_rule
|
|
await update_rule(rule_id=1, clear_fields=["verify_with"])
|
|
_args, kwargs = mock.call_args
|
|
assert kwargs == {"clear": ["verify_with"]}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_update_rule_sends_the_check_fields_when_given():
|
|
rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
|
|
mock = AsyncMock(return_value=rule)
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.update_rule", mock), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import update_rule
|
|
await update_rule(
|
|
rule_id=1,
|
|
verify_with="cat CI-runner/renovate/config.js",
|
|
expires_when="approval is turned off",
|
|
)
|
|
_args, kwargs = mock.call_args
|
|
assert kwargs == {
|
|
"verify_with": "cat CI-runner/renovate/config.js",
|
|
"expires_when": "approval is turned off",
|
|
"clear": (),
|
|
}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_delete_rule_without_confirmed_returns_warning():
|
|
"""delete_rule with confirmed=False returns a preview, not an action."""
|
|
rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
|
|
AsyncMock(return_value=rule),
|
|
), patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.delete_rule",
|
|
AsyncMock(),
|
|
) as mock_delete:
|
|
from scribe.mcp.tools.rulebooks import delete_rule
|
|
out = await delete_rule(rule_id=1, confirmed=False)
|
|
assert out.get("confirmed_required") is True
|
|
assert "confirmed=True" in out.get("warning", "")
|
|
assert not mock_delete.called # service NOT called
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_delete_rule_with_confirmed_soft_deletes():
|
|
rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
|
|
mock_delete = AsyncMock(return_value="batch-1")
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
|
|
AsyncMock(return_value=rule),
|
|
), patch(
|
|
"scribe.mcp.tools.rulebooks.trash_svc.delete",
|
|
mock_delete,
|
|
):
|
|
from scribe.mcp.tools.rulebooks import delete_rule
|
|
out = await delete_rule(rule_id=1, confirmed=True)
|
|
assert out["deleted"] == 1
|
|
assert out["deleted_batch_id"] == "batch-1"
|
|
assert mock_delete.called
|
|
|
|
|
|
def test_register_attaches_every_tool():
|
|
"""Every tool in the module reaches the server.
|
|
|
|
The count is the guard: a function added to the module but left out of
|
|
register()'s tuple is invisible to callers and raises nothing. The name
|
|
said "sixteen" for ten tools' worth of growth — the number lives in the
|
|
assertion, not the title, so it cannot drift again.
|
|
"""
|
|
from scribe.mcp.tools.rulebooks import register
|
|
mcp = FakeMCP()
|
|
|
|
register(mcp)
|
|
# 26 through milestone 307, +2 for the staleness sweep (milestone 312),
|
|
# +1 for a rule's edit history (milestone 323), +2 for preferences
|
|
# (milestone 399).
|
|
# 28 since milestone 394 took list_always_on_rules and the two
|
|
# always-on exclusion tools with the tier they served.
|
|
# 22 since milestone 414 retired subscriptions and suppressions: the two
|
|
# subscribe tools and the four suppress/unsuppress tools.
|
|
assert len(mcp.names) == 22
|
|
# spot-check a few names
|
|
assert "list_rulebooks" in mcp.names
|
|
assert "create_rule" in mcp.names
|
|
# Preferences get their own WRITE door — create_rule's docstring is the
|
|
# approval gate, and a preference reached through it would be read
|
|
# through that prose. Reads stay shared deliberately, so there is no
|
|
# get_preference to look for here.
|
|
assert "create_preference" in mcp.names
|
|
assert "update_preference" in mcp.names
|
|
assert "create_project_rule" in mcp.names
|
|
# milestone 312: the sweep, and the stamp that answers it
|
|
assert "rules_due_for_verification" in mcp.names
|
|
assert "mark_rule_verified" in mcp.names
|
|
# milestone 323: what a rule used to say
|
|
assert "rule_history" in mcp.names
|
|
# milestone 414: a rule's home is its scope; nothing subscribes or mutes.
|
|
for gone in ("subscribe_project_to_rulebook", "unsubscribe_project_from_rulebook",
|
|
"suppress_rule_for_project", "unsuppress_rule_for_project",
|
|
"suppress_topic_for_project", "unsuppress_topic_for_project"):
|
|
assert gone not in mcp.names
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_project_rule_passes_required_fields():
|
|
rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
|
|
mock = AsyncMock(return_value=rule)
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_project_rule
|
|
await create_project_rule(
|
|
project_id=42,
|
|
statement="Always run migrations through alembic, not raw SQL.",
|
|
why="audit trail",
|
|
)
|
|
kwargs = mock.call_args.kwargs
|
|
assert kwargs["user_id"] == 7
|
|
assert kwargs["project_id"] == 42
|
|
assert kwargs["statement"].startswith("Always run migrations")
|
|
assert kwargs["why"] == "audit trail"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_project_rule_derives_title_from_statement():
|
|
rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
|
|
mock = AsyncMock(return_value=rule)
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_project_rule
|
|
await create_project_rule(
|
|
project_id=42,
|
|
statement="Avoid auto-generated docstrings. Reviewers find them noise.",
|
|
)
|
|
kwargs = mock.call_args.kwargs
|
|
# Title should be derived from the first sentence, capped at 50 chars
|
|
assert kwargs["title"] == "Avoid auto-generated docstrings"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_create_project_rule_uses_explicit_title_when_given():
|
|
rule = fake_rule(id=100, title="r", statement="s", topic_id=10)
|
|
mock = AsyncMock(return_value=rule)
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock), _plain_detail():
|
|
from scribe.mcp.tools.rulebooks import create_project_rule
|
|
await create_project_rule(
|
|
project_id=42,
|
|
statement="anything",
|
|
title="no auto-docstrings",
|
|
)
|
|
kwargs = mock.call_args.kwargs
|
|
assert kwargs["title"] == "no auto-docstrings"
|
|
|
|
|
|
# ── Typed edges between rules (milestone 307) ───────────────────────────
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_relate_rules_forwards_the_kind_and_the_why():
|
|
"""The edge exists so a shape stops being merged into one row. The `note`
|
|
travels with it for the same reason a rule carries `why`: whoever later
|
|
decides whether the edge still holds needs the reasoning."""
|
|
relation = MagicMock()
|
|
relation.to_dict.return_value = {"id": 9, "kind": "co_surfaces"}
|
|
mock = AsyncMock(return_value=relation)
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.add_rule_relation", mock):
|
|
from scribe.mcp.tools.rulebooks import relate_rules
|
|
out = await relate_rules(
|
|
from_rule_id=46, to_rule_id=144, kind="co_surfaces",
|
|
note="a stale channel tag and an unparseable version both read as "
|
|
"no update available",
|
|
)
|
|
assert out["id"] == 9
|
|
args = mock.call_args.args
|
|
assert args[0] == 7 and args[1] == 46 and args[2] == 144
|
|
assert args[3] == "co_surfaces"
|
|
assert "no update available" in args[4]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_relate_rules_raises_when_either_end_is_not_yours():
|
|
"""The service returns None when it cannot see both rules — a one-sided
|
|
edge would surface a rule the caller has no business reading."""
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.add_rule_relation",
|
|
AsyncMock(return_value=None)):
|
|
from scribe.mcp.tools.rulebooks import relate_rules
|
|
with pytest.raises(ValueError, match="not found"):
|
|
await relate_rules(from_rule_id=1, to_rule_id=2, kind="overrides")
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_unrelate_rules_raises_when_the_edge_is_gone():
|
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.remove_rule_relation",
|
|
AsyncMock(return_value=False)):
|
|
from scribe.mcp.tools.rulebooks import unrelate_rules
|
|
with pytest.raises(ValueError, match="not found"):
|
|
await unrelate_rules(relation_id=99)
|
|
|
|
|
|
# ── rule_history (milestone 323 step 3) ────────────────────────────────
|
|
|
|
def _fake_version(**over):
|
|
"""A RuleVersion-shaped stand-in. A real model instance rather than a
|
|
MagicMock, because the tool calls `to_dict` and a mock would hand back
|
|
another mock instead of failing."""
|
|
from scribe.models.rule_version import RuleVersion
|
|
from datetime import datetime, timezone
|
|
|
|
defaults = {
|
|
"id": 5, "rule_id": 100, "user_id": 1,
|
|
"title": "The runner has no bash", "statement": "Use sh.",
|
|
"why": "the image ships no bash", "how_to_apply": None,
|
|
"when_to_apply": None,
|
|
"verify_with": "read the workflow's shell setting",
|
|
"expires_when": None,
|
|
"created_at": datetime(2026, 8, 29, tzinfo=timezone.utc),
|
|
}
|
|
return RuleVersion(**{**defaults, **over})
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_rule_history_lists_without_the_heavy_text():
|
|
"""The listing answers "when, and by whom". A rule's statement runs to
|
|
thousands of characters, so a history carrying every field would cost
|
|
more to read than the answer is worth."""
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.list_rule_versions",
|
|
AsyncMock(return_value=[_fake_version(), _fake_version(id=4)]),
|
|
), patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
|
|
AsyncMock(return_value=fake_rule(
|
|
id=100, title="The runner has no bash", statement="s", topic_id=10,
|
|
)),
|
|
):
|
|
from scribe.mcp.tools.rulebooks import rule_history
|
|
out = await rule_history(rule_id=100)
|
|
|
|
assert out["total"] == 2
|
|
assert out["versions"][0]["title"] == "The runner has no bash"
|
|
assert "statement" not in out["versions"][0]
|
|
assert "why" not in out["versions"][0]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_rule_history_opens_one_version_in_full():
|
|
"""Passing a version id switches from the index to the text — which is
|
|
the whole reason the listing can afford to omit it."""
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule_version",
|
|
AsyncMock(return_value=_fake_version()),
|
|
):
|
|
from scribe.mcp.tools.rulebooks import rule_history
|
|
out = await rule_history(rule_id=100, version_id=5)
|
|
|
|
assert out["statement"] == "Use sh."
|
|
assert out["verify_with"] == "read the workflow's shell setting"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_rule_history_says_an_empty_history_is_ordinary():
|
|
"""Most rules have never been reworded, and nothing was written before
|
|
milestone 323. Without this line an empty list reads as a lost history or
|
|
a broken tool."""
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.list_rule_versions",
|
|
AsyncMock(return_value=[]),
|
|
), patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule",
|
|
AsyncMock(return_value=fake_rule(
|
|
id=100, title="The runner has no bash", statement="s", topic_id=10,
|
|
)),
|
|
):
|
|
from scribe.mcp.tools.rulebooks import rule_history
|
|
out = await rule_history(rule_id=100)
|
|
|
|
assert out["total"] == 0
|
|
assert "never been reworded" in out["note"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_rule_history_raises_when_the_rule_is_not_yours():
|
|
"""None from the service means "not readable", and the tool must not
|
|
turn that into an empty history — which would read as "this rule has no
|
|
past" rather than "this is not your rule"."""
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.list_rule_versions",
|
|
AsyncMock(return_value=None),
|
|
):
|
|
from scribe.mcp.tools.rulebooks import rule_history
|
|
with pytest.raises(ValueError, match="rule 100 not found"):
|
|
await rule_history(rule_id=100)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_rule_history_raises_for_a_version_on_another_rule():
|
|
with patch(
|
|
"scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule_version",
|
|
AsyncMock(return_value=None),
|
|
):
|
|
from scribe.mcp.tools.rulebooks import rule_history
|
|
with pytest.raises(ValueError, match="version 5 not found"):
|
|
await rule_history(rule_id=100, version_id=5)
|
|
|
|
|
|
def test_rule_history_docstring_says_what_a_version_HOLDS():
|
|
"""The one thing a reader gets wrong unaided: an entry is the text the
|
|
edit REPLACED, not the text it introduced. Read the other way, every
|
|
diff comes out backwards — so the docstring has to say it, and this is
|
|
the guard against a later tidy-up dropping the line."""
|
|
from scribe.mcp.tools.rulebooks import rule_history
|
|
|
|
doc = rule_history.__doc__ or ""
|
|
assert "REPLACED" in doc
|
|
assert "no restore" in doc.lower(), (
|
|
"the docstring no longer explains that a rule version cannot be "
|
|
"restored. A caller who assumes a revert exists will look for one "
|
|
"and, not finding it, is likely to hand-copy the old text back with "
|
|
"no record of why."
|
|
)
|