diff --git a/docs/api-reference.md b/docs/api-reference.md index c5d9bad..a5826b2 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -115,6 +115,7 @@ endpoint at `/mcp`, not these REST routes. | GET | `/api/rules` | List rules | | POST | `/api/rulebook-topics/:tid/rules` | Add a rule to a topic | | GET / PATCH / DELETE | `/api/rules/:id` | Read / update / delete a rule | +| POST | `/api/rules/:id/move` | Move a rule: `{topic_id}` makes it global, `{project_id}` makes it that project's | | GET | `/api/projects/:id/rules` | A project's own rules, and the global rules tagged to its areas | | POST | `/api/projects/:id/rules` | Create a project-scoped rule | diff --git a/frontend/src/api/rulebooks.ts b/frontend/src/api/rulebooks.ts index 5690a0c..5681571 100644 --- a/frontend/src/api/rulebooks.ts +++ b/frontend/src/api/rulebooks.ts @@ -198,6 +198,14 @@ export async function updateRule(id: number, data: Partial): Promise< return apiPatch(`/api/rules/${id}`, data); } +/** Give a rule a new home: a topic makes it global, a project makes it that + * project's. Keeps its id, history, areas and relations (milestone 414). */ +export async function moveRule( + id: number, to: { topic_id: number } | { project_id: number }, +): Promise { + return apiPost(`/api/rules/${id}/move`, to); +} + /** Draw a typed edge from one rule to another. Idempotent. */ export async function relateRules( fromRuleId: number, diff --git a/frontend/src/components/rules/ProjectRulesTab.vue b/frontend/src/components/rules/ProjectRulesTab.vue index 6aa9a46..69c0516 100644 --- a/frontend/src/components/rules/ProjectRulesTab.vue +++ b/frontend/src/components/rules/ProjectRulesTab.vue @@ -8,6 +8,7 @@ import { deleteRule, } from "@/api/rulebooks"; import type { ApplicableRules } from "@/api/rulebooks"; +import RuleHomePicker from "@/components/rules/RuleHomePicker.vue"; /** * A project's view of its rules (milestone 414). A rule's home is its reach: @@ -206,6 +207,12 @@ watch(() => props.projectId, load);
Ends when: {{ ruleDetails[r.id].expires_when }}
+ diff --git a/frontend/src/components/rules/RuleEditorSlideOver.vue b/frontend/src/components/rules/RuleEditorSlideOver.vue index 4eb0cee..955892c 100644 --- a/frontend/src/components/rules/RuleEditorSlideOver.vue +++ b/frontend/src/components/rules/RuleEditorSlideOver.vue @@ -3,6 +3,8 @@ import { computed, ref, watch, onMounted } from "vue"; import { useRulebooksStore } from "@/stores/rulebooks"; import { useCanonicalSystemsStore } from "@/stores/canonicalSystems"; import RuleHistoryPanel from "@/components/rules/RuleHistoryPanel.vue"; +import RuleHomePicker from "@/components/rules/RuleHomePicker.vue"; +import type { Rule } from "@/api/rulebooks"; const props = defineProps<{ ruleId: number | null; topicId: number | null }>(); const emit = defineEmits<{ close: [] }>(); @@ -115,6 +117,14 @@ async function save() { emit("close"); } +// A moved rule has left the topic this view lists (or joined another). The +// store re-places it, then the editor saves any text edits and closes, the +// way the backdrop does. +async function onMoved(rule: Rule) { + store.placeMovedRule(rule); + await save(); +} + async function remove() { if (props.ruleId === null) return; if (!confirm("Delete this rule? This cannot be undone.")) return; @@ -210,6 +220,14 @@ watch(() => props.ruleId, load);

+ +

Related rules

    diff --git a/frontend/src/components/rules/RuleHomePicker.vue b/frontend/src/components/rules/RuleHomePicker.vue new file mode 100644 index 0000000..5f93593 --- /dev/null +++ b/frontend/src/components/rules/RuleHomePicker.vue @@ -0,0 +1,131 @@ + + + + + diff --git a/frontend/src/stores/rulebooks.ts b/frontend/src/stores/rulebooks.ts index d4a917b..570960a 100644 --- a/frontend/src/stores/rulebooks.ts +++ b/frontend/src/stores/rulebooks.ts @@ -137,6 +137,20 @@ export const useRulebooksStore = defineStore("rulebooks", () => { return rule; } + /** After a move (milestone 414): take the rule out of whichever topic list + * held it, and into its new topic's list if that one is loaded. A rule moved + * onto a project belongs to no topic list at all. */ + function placeMovedRule(rule: Rule) { + if (currentRule.value?.id === rule.id) currentRule.value = rule; + for (const tid of Object.keys(rulesByTopic.value)) { + const key = Number(tid); + rulesByTopic.value[key] = rulesByTopic.value[key].filter((r) => r.id !== rule.id); + } + if (rule.topic_id !== null && rulesByTopic.value[rule.topic_id]) { + rulesByTopic.value[rule.topic_id].push(toHeader(rule)); + } + } + async function relateRules( fromRuleId: number, data: { to_rule_id: number; kind: api.RuleRelationKind; note?: string }, @@ -198,6 +212,7 @@ export const useRulebooksStore = defineStore("rulebooks", () => { return { rulebooks, topicsByRulebook, rulesByTopic, currentRule, rulesDue, lastSweepOpts, loading, + placeMovedRule, fetchRulebooks, fetchTopics, fetchRules, fetchRule, createRulebook, updateRulebook, deleteRulebook, createTopic, updateTopic, deleteTopic, diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index f695dee..885fc5f 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "scribe", "description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).", - "version": "2026.09.15.1620", + "version": "2026.09.15.1626", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/skills/using-scribe/SKILL.md b/plugin/skills/using-scribe/SKILL.md index 2b16369..9370e04 100644 --- a/plugin/skills/using-scribe/SKILL.md +++ b/plugin/skills/using-scribe/SKILL.md @@ -278,7 +278,10 @@ Names one project's specifics → project rule; a standard that holds wherever the kind of work it describes happens → global. Never put project-specific detail in a rulebook — it would reach every other project. A project that departs from a global rule writes its own and links it with -`relate_rules(kind="overrides")`, which says why. +`relate_rules(kind="overrides")`, which says why. A rule that turns out to be +in the wrong home — a project rule that holds everywhere, a global one only a +single project needs — moves with `move_rule`, which keeps its id, history, +areas and edges. Propose the move and make it on a yes. **Whichever home it gets, a rule needs `when_to_apply`.** It is the only thing that decides whether the rule is ever seen: nothing is preloaded, so a rule diff --git a/src/scribe/mcp/tools/rulebooks.py b/src/scribe/mcp/tools/rulebooks.py index 7fb92ba..630201f 100644 --- a/src/scribe/mcp/tools/rulebooks.py +++ b/src/scribe/mcp/tools/rulebooks.py @@ -862,6 +862,38 @@ async def rule_history(rule_id: int, version_id: int = 0) -> dict: } +async def move_rule(rule_id: int, topic_id: int = 0, project_id: int = 0) -> dict: + """Move a rule to a new home, keeping its id, history, areas and edges. + + A rule's home IS its reach. In a rulebook topic it is GLOBAL: it applies in + every project and reaches any session whose work matches it. On a project + it applies to that project only. So this is how a project rule that turns + out to hold everywhere becomes global (pass `topic_id`), and how a global + rule that only one project needs becomes that project's (pass + `project_id`). Name exactly one. + + Reach for this INSTEAD of recreating the rule in the other home and + deleting the original: that loses the id every record cites it by, its + edit history, its area tags and its relations. + + A move is a decision about where a rule binds, so propose it and move on a + yes, the way create_rule proposes a new rule — and record why where the + decision lives (a task or note). The rule's history does not record a + move: it holds what the rule SAID, and a move changes none of that. + + Refused with a message when: neither or both destinations are named, the + destination is not yours, the rule is already there, or the topic already + has a rule with this title (rename one first). + """ + uid = current_user_id() + rule = await rulebooks_svc.move_rule( + rule_id, uid, topic_id=topic_id, project_id=project_id, + ) + if rule is None: + raise ValueError(f"rule {rule_id} not found") + return await rulebooks_svc.rule_detail(uid, rule) + + async def delete_rule(rule_id: int, confirmed: bool = False) -> dict: """Move a rule to the trash (recoverable). Requires confirmed=True.""" uid = current_user_id() @@ -1019,7 +1051,7 @@ def register(mcp) -> None: list_rulebooks, get_rulebook, create_rulebook, update_rulebook, delete_rulebook, list_topics, create_topic, update_topic, delete_topic, list_rules, get_rule, - create_rule, create_project_rule, update_rule, delete_rule, + create_rule, create_project_rule, update_rule, move_rule, delete_rule, create_preference, update_preference, relate_rules, unrelate_rules, rules_due_for_verification, mark_rule_verified, diff --git a/src/scribe/routes/rulebooks.py b/src/scribe/routes/rulebooks.py index 263310f..b94ef57 100644 --- a/src/scribe/routes/rulebooks.py +++ b/src/scribe/routes/rulebooks.py @@ -297,6 +297,27 @@ async def unrelate_rules(relation_id: int): return "", 204 +@rulebooks_bp.post("/rules//move") +@login_required +async def move_rule(rule_id: int): + """Give a rule a new home: {topic_id} makes it global, {project_id} makes + it that project's. The MCP twin is move_rule (rule 33: same names).""" + data = await request.get_json() or {} + uid = get_current_user_id() + try: + rule = await rulebooks_svc.move_rule( + rule_id, uid, + topic_id=int(data.get("topic_id") or 0), + project_id=int(data.get("project_id") or 0), + ) + except (TypeError, ValueError) as exc: + msg = str(exc) + return jsonify({"error": msg}), 404 if "not found" in msg else 400 + if rule is None: + return jsonify({"error": "rule not found"}), 404 + return jsonify(await rulebooks_svc.rule_detail(uid, rule)) + + @rulebooks_bp.delete("/rules/") @login_required async def delete_rule(rule_id: int): diff --git a/src/scribe/services/rulebooks.py b/src/scribe/services/rulebooks.py index 52ed674..48eed7d 100644 --- a/src/scribe/services/rulebooks.py +++ b/src/scribe/services/rulebooks.py @@ -688,6 +688,71 @@ async def update_rule( return rule +async def move_rule( + rule_id: int, user_id: int, *, topic_id: int = 0, project_id: int = 0, +) -> Optional[Rule]: + """Give a rule a new home — into a rulebook topic (global) or onto a + project — keeping its id, history, Systems and relations (milestone 414). + + A rule's home IS its reach: in a topic it applies to every project, on a + project to that project alone. Recreating the rule in the other home and + trashing the original would lose its id (and every record citing it), its + edit history, its area tags and its typed edges, which is why this exists. + + Exactly one of `topic_id` / `project_id`, matching the model's CHECK + (migration 0059). Raises ValueError for: neither or both named, a target + the caller does not own, the rule already living there, or a topic that + already holds a live rule with this title (uq_rule_per_topic) — the message + names that rule, rather than letting the constraint fail the commit. + Returns None when the rule itself is not the caller's. + + WHAT A MOVE DOES NOT DO, deliberately: + + - No version. A rule's history records its TEXT (milestone 323, decision + 4); its place is not text, and folding it in would make "version" mean + two things. The rule's `updated_at` moves; say why a rule moved where + the decision is recorded. + - No duplicate gate. Nothing new enters the corpus — the same rule changes + home — so there is no second record to warn about. + - No re-embed. The rule's document is its title, statement and trigger; + retrieval reads the home from the row at query time. + """ + if bool(topic_id) == bool(project_id): + raise ValueError("name exactly one destination: topic_id (global) or project_id") + async with async_session() as session: + rule = await _fetch_owned_rule(session, rule_id, user_id) + if rule is None: + return None + if topic_id: + if rule.topic_id == topic_id: + raise ValueError(f"rule {rule_id} is already in topic {topic_id}") + await _assert_topic_owned(session, topic_id, user_id) + clash = (await session.execute( + select(Rule.id).where( + Rule.topic_id == topic_id, + Rule.title == rule.title, + Rule.deleted_at.is_(None), + Rule.id != rule.id, + ) + )).scalar_one_or_none() + if clash is not None: + raise ValueError( + f'topic {topic_id} already has a rule titled "{rule.title}" ' + f"(rule {clash}) — rename one before moving" + ) + rule.project_id = None + rule.topic_id = topic_id + else: + if rule.project_id == project_id: + raise ValueError(f"rule {rule_id} is already on project {project_id}") + await _assert_project_owned(session, project_id, user_id) + rule.topic_id = None + rule.project_id = project_id + await session.commit() + await session.refresh(rule) + return rule + + # ── Edit history (milestone 323) ─────────────────────────────────────── # # The ACL-scoped reads live HERE rather than in services/rule_versions.py, diff --git a/tests/test_integration_rule_move.py b/tests/test_integration_rule_move.py new file mode 100644 index 0000000..d37c4f7 --- /dev/null +++ b/tests/test_integration_rule_move.py @@ -0,0 +1,138 @@ +"""Real-Postgres tests for moving a rule between homes (milestone 414, step 3). + +A rule's home is its reach: a rulebook topic makes it global, a project makes +it that project's. The alternative to a move — recreate the rule in the other +home and trash the original — loses the id every record cites, the edit +history, the area tags and the typed edges. These pin that a move keeps all +four, and that the refusals happen before anything is written: the topic/ +project CHECK (migration 0059) and the per-topic title index would otherwise +fail the commit with a raw database error. +""" +import uuid +from unittest.mock import MagicMock, patch + +import pytest +import pytest_asyncio + +from scribe.models import async_session +from scribe.models.project import Project +from scribe.models.rulebook import Rule +from scribe.services import canonical_systems as canonical_svc +from scribe.services import rule_versions as rv_svc +from scribe.services import rulebooks as rulebooks_svc +from tests.helpers import ensure_user + +pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")] + + +@pytest.fixture(autouse=True) +def _no_reindex(): + """Rule writes detach an embedding refresh that outlives the test's loop + and races the next fixture; nothing here is about recall.""" + with patch("scribe.services.rulebooks._refresh_rule_embedding", MagicMock()): + yield + + +@pytest_asyncio.fixture +async def homes(): + """A project rule with a version, an area tag and an incoming edge, plus a + topic to move it into and a second project.""" + tag = uuid.uuid4().hex[:8] + async with async_session() as s: + owner = await ensure_user(s, f"rule_move_owner_{tag}") + stranger = await ensure_user(s, f"rule_move_stranger_{tag}") + home = Project(user_id=owner.id, title="Where it started") + other = Project(user_id=owner.id, title="Somewhere else") + theirs = Project(user_id=stranger.id, title="Not yours") + s.add_all([home, other, theirs]) + await s.flush() + ids = {"owner": owner.id, "stranger": stranger.id, "home": home.id, + "other": other.id, "theirs": theirs.id} + await s.commit() + + owner = ids["owner"] + book = await rulebooks_svc.create_rulebook(owner, "House style") + topic = await rulebooks_svc.create_topic(book.id, owner, "transport") + rule = await rulebooks_svc.create_project_rule( + ids["home"], owner, "Plain HTTP only", "No app-level TLS.", + when_to_apply="setting a cookie flag or a URL scheme", + ) + await rulebooks_svc.update_rule(rule.id, owner, statement="No app-level TLS, ever.") + area = await canonical_svc.find_by_name("CI & Release") + await rulebooks_svc.set_rule_systems(rule.id, owner, [area.id]) + downstream = await rulebooks_svc.create_project_rule( + ids["home"], owner, "No Secure-Context APIs", "Browsers withhold them.", + when_to_apply="reaching for the clipboard API", + ) + await rulebooks_svc.add_rule_relation(owner, downstream.id, rule.id, "elaborates") + ids.update(topic=topic.id, rule=rule.id, area=area.id) + return ids + + +async def _row(rule_id: int) -> Rule: + async with async_session() as s: + return await s.get(Rule, rule_id) + + +async def test_a_project_rule_becomes_global_and_keeps_everything(homes): + owner, rule_id = homes["owner"], homes["rule"] + moved = await rulebooks_svc.move_rule(rule_id, owner, topic_id=homes["topic"]) + + assert moved.id == rule_id + row = await _row(rule_id) + assert (row.topic_id, row.project_id) == (homes["topic"], None) + assert len(await rv_svc.list_versions(rule_id)) == 1, "the move must not drop history" + areas = await rulebooks_svc.list_rule_systems([rule_id]) + assert [a["id"] for a in areas[rule_id]] == [homes["area"]] + edges = await rulebooks_svc.list_rule_relations([rule_id]) + assert [e["kind"] for e in edges[rule_id]] == ["elaborates"] + + +async def test_a_move_writes_no_version(homes): + """A version records what a rule SAID (milestone 323, decision 4). A move + changes where it binds, not a word of it.""" + before = len(await rv_svc.list_versions(homes["rule"])) + await rulebooks_svc.move_rule(homes["rule"], homes["owner"], topic_id=homes["topic"]) + assert len(await rv_svc.list_versions(homes["rule"])) == before + + +async def test_a_global_rule_can_move_onto_a_project(homes): + owner, rule_id = homes["owner"], homes["rule"] + await rulebooks_svc.move_rule(rule_id, owner, topic_id=homes["topic"]) + await rulebooks_svc.move_rule(rule_id, owner, project_id=homes["other"]) + row = await _row(rule_id) + assert (row.topic_id, row.project_id) == (None, homes["other"]) + + +async def test_refusals_happen_before_anything_is_written(homes): + owner, rule_id = homes["owner"], homes["rule"] + + with pytest.raises(ValueError, match="exactly one"): + await rulebooks_svc.move_rule(rule_id, owner) + with pytest.raises(ValueError, match="exactly one"): + await rulebooks_svc.move_rule(rule_id, owner, topic_id=homes["topic"], + project_id=homes["other"]) + with pytest.raises(ValueError, match="already on project"): + await rulebooks_svc.move_rule(rule_id, owner, project_id=homes["home"]) + with pytest.raises(ValueError, match="not found"): + await rulebooks_svc.move_rule(rule_id, owner, project_id=homes["theirs"]) + + # A topic already holding a live rule with this title: named, not a raw + # IntegrityError from uq_rule_per_topic at commit. + clash = await rulebooks_svc.create_rule( + homes["topic"], owner, "Plain HTTP only", "Already here.", + when_to_apply="setting a cookie flag", + ) + with pytest.raises(ValueError, match=f"rule {clash.id}"): + await rulebooks_svc.move_rule(rule_id, owner, topic_id=homes["topic"]) + + row = await _row(rule_id) + assert (row.topic_id, row.project_id) == (None, homes["home"]) + + +async def test_someone_elses_rule_is_not_found(homes): + """None, like every other rule read the caller cannot see — not an error + that confirms the rule exists.""" + assert await rulebooks_svc.move_rule( + homes["rule"], homes["stranger"], project_id=homes["theirs"], + ) is None diff --git a/tests/test_mcp_tool_rulebooks.py b/tests/test_mcp_tool_rulebooks.py index db1f61d..6899261 100644 --- a/tests/test_mcp_tool_rulebooks.py +++ b/tests/test_mcp_tool_rulebooks.py @@ -204,8 +204,9 @@ def test_register_attaches_every_tool(): # 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 + # subscribe tools and the four suppress/unsuppress tools. 23 with move_rule + # (milestone 414 step 3), the way a rule changes home. + assert len(mcp.names) == 23 # spot-check a few names assert "list_rulebooks" in mcp.names assert "create_rule" in mcp.names @@ -216,6 +217,7 @@ def test_register_attaches_every_tool(): assert "create_preference" in mcp.names assert "update_preference" in mcp.names assert "create_project_rule" in mcp.names + assert "move_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 @@ -448,3 +450,27 @@ def test_rule_history_docstring_says_what_a_version_HOLDS(): "and, not finding it, is likely to hand-copy the old text back with " "no record of why." ) + + +@pytest.mark.asyncio +async def test_move_rule_passes_one_destination_and_returns_the_detail(): + """The tool is a thin door: the service decides what a valid move is, and + the reply is the same rule_detail every other write returns.""" + moved = fake_rule(id=94, topic_id=12, project_id=None) + move = AsyncMock(return_value=moved) + detail = AsyncMock(return_value={"id": 94, "topic_id": 12, "project_id": None}) + with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.move_rule", move), \ + patch("scribe.mcp.tools.rulebooks.rulebooks_svc.rule_detail", detail): + from scribe.mcp.tools.rulebooks import move_rule + out = await move_rule(rule_id=94, topic_id=12) + assert move.await_args.args[0] == 94 + assert move.await_args.kwargs == {"topic_id": 12, "project_id": 0} + assert out["topic_id"] == 12 + + +@pytest.mark.asyncio +async def test_move_rule_on_someone_elses_rule_is_not_found(): + with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.move_rule", AsyncMock(return_value=None)): + from scribe.mcp.tools.rulebooks import move_rule + with pytest.raises(ValueError, match="not found"): + await move_rule(rule_id=94, project_id=3) diff --git a/tests/test_routes_rulebooks.py b/tests/test_routes_rulebooks.py index 43b2cf8..66fe608 100644 --- a/tests/test_routes_rulebooks.py +++ b/tests/test_routes_rulebooks.py @@ -93,7 +93,7 @@ def test_rule_handlers_callable(): from scribe.routes import rulebooks as rb_routes for name in ( "list_rules", "create_rule", "get_rule", "update_rule", "delete_rule", - "get_project_rules", + "get_project_rules", "move_rule", # The typed edges — both doors carry them (rule 33). "relate_rules", "unrelate_rules", ):