feat(rules): project-scoped rules (S3)
Rules can now belong to either a rulebook topic OR a single project,
enforced by a CHECK constraint (exactly-one of topic_id/project_id).
Adds the create_project_rule MCP tool + REST endpoint, surfaces
project-scoped rules in get_project/get_task/start_planning under a
new project_rules field, and adds a project Rules tab section with an
inline create form so the operator can author project rules from the
UI without rulebook ceremony.
- migration 0059: rules.project_id (FK projects ON DELETE CASCADE),
topic_id now nullable, CHECK ck_rule_topic_xor_project, index on
project_id
- model: Rule gains project_id; to_dict exposes it
- service: create_project_rule with project-ownership guard; list_rules
with project_id filter UNIONs subscription-derived + project-scoped;
get_applicable_rules adds a project_rules field; get_rule / update_rule
/ delete_rule fetch via a shared _fetch_owned_rule that handles both
rulebook and project ownership paths
- trash: project delete cascades to project-scoped rules
- MCP: create_project_rule tool registered; _INSTRUCTIONS mentions both
create_rule and create_project_rule paths
- REST: POST /api/projects/<id>/rules (statement required, title derived
if omitted)
- frontend: Rule type gains nullable topic_id + project_id; createProjectRule
client; ProjectRulesTab.vue gains a "Project rules" section with inline
create form and per-rule expand/delete
- tests: register count → 18; create_project_rule unit tests (required
fields, title derivation, explicit-title pass-through); applicable_rules
shape tests now include project_rules; trash cascade test updated to
expect 5 executions
S1+S2 (always_on flag + Scribe-first prompt) shipped in 658348f.
S4 (enter_project handshake) follows.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -179,12 +179,13 @@ def test_register_attaches_all_sixteen_tools():
|
||||
return decorator
|
||||
|
||||
register(FakeMCP())
|
||||
assert len(registered) == 17
|
||||
assert len(registered) == 18
|
||||
# spot-check a few names
|
||||
assert "list_rulebooks" in registered
|
||||
assert "create_rule" in registered
|
||||
assert "subscribe_project_to_rulebook" in registered
|
||||
assert "list_always_on_rules" in registered
|
||||
assert "create_project_rule" in registered
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -235,3 +236,51 @@ async def test_update_rulebook_omits_always_on_when_none():
|
||||
kwargs = mock.call_args.kwargs
|
||||
assert "always_on" not in kwargs
|
||||
assert kwargs["title"] == "new title"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_project_rule_passes_required_fields():
|
||||
rule = _fake_rule()
|
||||
mock = AsyncMock(return_value=rule)
|
||||
with patch("fabledassistant.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock):
|
||||
from fabledassistant.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()
|
||||
mock = AsyncMock(return_value=rule)
|
||||
with patch("fabledassistant.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock):
|
||||
from fabledassistant.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()
|
||||
mock = AsyncMock(return_value=rule)
|
||||
with patch("fabledassistant.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock):
|
||||
from fabledassistant.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"
|
||||
|
||||
@@ -44,7 +44,8 @@ def test_service_signatures_require_user_id():
|
||||
"create_rulebook", "list_rulebooks", "get_rulebook",
|
||||
"update_rulebook", "delete_rulebook", "find_rulebook_by_title",
|
||||
"create_topic", "list_topics", "get_topic", "update_topic", "delete_topic",
|
||||
"create_rule", "list_rules", "list_always_on_rules",
|
||||
"create_rule", "create_project_rule",
|
||||
"list_rules", "list_always_on_rules",
|
||||
"get_rule", "update_rule", "delete_rule",
|
||||
"subscribe_project", "unsubscribe_project", "get_applicable_rules",
|
||||
):
|
||||
@@ -52,6 +53,20 @@ def test_service_signatures_require_user_id():
|
||||
assert "user_id" in sig.parameters, f"{fn_name} missing user_id param"
|
||||
|
||||
|
||||
def test_rule_model_carries_project_id_and_topic_id_nullable():
|
||||
"""Migration 0059 made topic_id nullable and added project_id."""
|
||||
from fabledassistant.models.rulebook import Rule
|
||||
assert "project_id" in Rule.__table__.columns
|
||||
assert Rule.__table__.columns["topic_id"].nullable is True
|
||||
assert Rule.__table__.columns["project_id"].nullable is True
|
||||
|
||||
|
||||
def test_create_project_rule_route_exists():
|
||||
"""POST /api/projects/<id>/rules — the frontend fast-path endpoint."""
|
||||
from fabledassistant.routes import rulebooks as rb_routes
|
||||
assert callable(getattr(rb_routes, "create_project_rule"))
|
||||
|
||||
|
||||
def test_rulebook_model_carries_always_on():
|
||||
"""Migration 0058 added rulebooks.always_on — verify the model declares it."""
|
||||
from fabledassistant.models.rulebook import Rulebook
|
||||
|
||||
@@ -243,7 +243,7 @@ async def test_subscribe_project_requires_owned_rulebook():
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_applicable_rules_returns_shape():
|
||||
"""get_applicable_rules returns {rules, truncated, subscribed_rulebooks}."""
|
||||
"""get_applicable_rules returns {rules, project_rules, truncated, subscribed_rulebooks}."""
|
||||
mock_session = _make_mock_session()
|
||||
sub_result = MagicMock()
|
||||
sub_result.all.return_value = [(1, "FabledSword family")]
|
||||
@@ -252,7 +252,9 @@ async def test_get_applicable_rules_returns_shape():
|
||||
(i, f"Rule {i}", f"Statement {i}", "git-workflow", "FabledSword family")
|
||||
for i in range(50)
|
||||
]
|
||||
mock_session.execute = AsyncMock(side_effect=[sub_result, rules_result])
|
||||
proj_rules_result = MagicMock()
|
||||
proj_rules_result.all.return_value = []
|
||||
mock_session.execute = AsyncMock(side_effect=[sub_result, rules_result, proj_rules_result])
|
||||
|
||||
with patch("fabledassistant.services.rulebooks.async_session") as mock_cls:
|
||||
mock_cls.return_value = mock_session
|
||||
@@ -260,10 +262,12 @@ async def test_get_applicable_rules_returns_shape():
|
||||
result = await get_applicable_rules(project_id=3, user_id=7, limit=50)
|
||||
|
||||
assert "rules" in result
|
||||
assert "project_rules" in result
|
||||
assert "truncated" in result
|
||||
assert "subscribed_rulebooks" in result
|
||||
assert result["subscribed_rulebooks"] == [{"id": 1, "title": "FabledSword family"}]
|
||||
assert len(result["rules"]) == 50
|
||||
assert result["project_rules"] == []
|
||||
assert result["truncated"] is False # exactly 50, not over
|
||||
|
||||
|
||||
@@ -278,7 +282,9 @@ async def test_get_applicable_rules_truncates_when_over_limit():
|
||||
rules_result.all.return_value = [
|
||||
(i, f"r{i}", "stmt", "topic", "rb") for i in range(51)
|
||||
]
|
||||
mock_session.execute = AsyncMock(side_effect=[sub_result, rules_result])
|
||||
proj_rules_result = MagicMock()
|
||||
proj_rules_result.all.return_value = []
|
||||
mock_session.execute = AsyncMock(side_effect=[sub_result, rules_result, proj_rules_result])
|
||||
|
||||
with patch("fabledassistant.services.rulebooks.async_session") as mock_cls:
|
||||
mock_cls.return_value = mock_session
|
||||
@@ -287,3 +293,28 @@ async def test_get_applicable_rules_truncates_when_over_limit():
|
||||
|
||||
assert result["truncated"] is True
|
||||
assert len(result["rules"]) == 50
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_applicable_rules_includes_project_scoped_rules():
|
||||
"""Project-scoped rules surface in the project_rules field."""
|
||||
mock_session = _make_mock_session()
|
||||
sub_result = MagicMock()
|
||||
sub_result.all.return_value = []
|
||||
rules_result = MagicMock()
|
||||
rules_result.all.return_value = []
|
||||
proj_rules_result = MagicMock()
|
||||
proj_rules_result.all.return_value = [
|
||||
(100, "Use alembic", "Always run migrations via alembic, never raw SQL."),
|
||||
(101, "PR-bound", "Land schema changes in their own PR."),
|
||||
]
|
||||
mock_session.execute = AsyncMock(side_effect=[sub_result, rules_result, proj_rules_result])
|
||||
|
||||
with patch("fabledassistant.services.rulebooks.async_session") as mock_cls:
|
||||
mock_cls.return_value = mock_session
|
||||
from fabledassistant.services.rulebooks import get_applicable_rules
|
||||
result = await get_applicable_rules(project_id=3, user_id=7)
|
||||
|
||||
assert len(result["project_rules"]) == 2
|
||||
assert result["project_rules"][0]["title"] == "Use alembic"
|
||||
assert result["project_rules"][1]["id"] == 101
|
||||
|
||||
@@ -48,18 +48,18 @@ async def test_delete_returns_none_when_not_found():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_project_cascades_three_tables():
|
||||
async def test_delete_project_cascades_to_notes_milestones_and_project_rules():
|
||||
session = _make_mock_session()
|
||||
# exists-check + 3 cascade updates (notes, milestones, project)
|
||||
# exists-check + 4 cascade updates (notes, milestones, project-scoped rules, project)
|
||||
session.execute = AsyncMock(side_effect=[
|
||||
_exists_result(True), MagicMock(), MagicMock(), MagicMock(),
|
||||
_exists_result(True), MagicMock(), MagicMock(), MagicMock(), MagicMock(),
|
||||
])
|
||||
with patch("fabledassistant.services.trash.async_session") as cls:
|
||||
cls.return_value = session
|
||||
from fabledassistant.services.trash import delete
|
||||
batch = await delete(user_id=1, entity_type="project", entity_id=3)
|
||||
assert isinstance(batch, str)
|
||||
assert session.execute.await_count == 4
|
||||
assert session.execute.await_count == 5
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in New Issue
Block a user