feat(rules): move a rule between global and project scope, keeping its id, history, areas and edges (#4063)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 54s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / Python tests (push) Successful in 1m41s
CI & Build / Build & push image (push) Successful in 33s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 54s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / Python tests (push) Successful in 1m41s
CI & Build / Build & push image (push) Successful in 33s
A rule's home is its reach: a rulebook topic makes it global, a project makes it that project's. There was no way to change one, so a project rule decided to be global could only be recreated and the original trashed — losing the id every record cites, its edit history, its area tags and its relations. - services.rulebooks.move_rule(rule_id, user_id, topic_id= | project_id=): exactly one destination (the model's CHECK), owned by the caller, not the rule's current home. A topic already holding a live rule with the same title is refused with a message naming that rule, instead of uq_rule_per_topic failing the commit. Someone else's rule reads as not found. - Deliberately NOT done, and said in the docstring: no version (a version is what a rule said, milestone 323 decision 4), no duplicate gate (nothing new enters the corpus), no re-embed (retrieval reads the home at query time). - Both doors: MCP move_rule, REST POST /api/rules/<id>/move (rule 33). - UI: RuleHomePicker, one component in the rule editor (a global rule) and a project's rules tab (a project rule), so the two cannot drift on what a destination is. - using-scribe names move_rule under "Where a new rule goes". Plugin 2026.09.15.1626. Milestone 414 step 3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -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 |
|
||||
|
||||
|
||||
@@ -198,6 +198,14 @@ export async function updateRule(id: number, data: Partial<RuleWrite>): 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<Rule> {
|
||||
return apiPost(`/api/rules/${id}/move`, to);
|
||||
}
|
||||
|
||||
/** Draw a typed edge from one rule to another. Idempotent. */
|
||||
export async function relateRules(
|
||||
fromRuleId: number,
|
||||
|
||||
@@ -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);
|
||||
<div v-if="ruleDetails[r.id].expires_when">
|
||||
<strong>Ends when:</strong> {{ ruleDetails[r.id].expires_when }}
|
||||
</div>
|
||||
<RuleHomePicker
|
||||
:rule-id="r.id"
|
||||
:topic-id="r.topic_id"
|
||||
:project-id="projectId"
|
||||
@moved="load"
|
||||
/>
|
||||
<button class="delete-link" @click="removeProjectRule(r.id)">Delete</button>
|
||||
</div>
|
||||
</li>
|
||||
|
||||
@@ -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);
|
||||
</p>
|
||||
</fieldset>
|
||||
|
||||
<RuleHomePicker
|
||||
v-if="!isCreating && ruleId !== null && store.currentRule"
|
||||
:rule-id="ruleId"
|
||||
:topic-id="store.currentRule.topic_id"
|
||||
:project-id="store.currentRule.project_id"
|
||||
@moved="onMoved"
|
||||
/>
|
||||
|
||||
<section v-if="relations.length" class="relations">
|
||||
<h3>Related rules</h3>
|
||||
<ul>
|
||||
|
||||
@@ -0,0 +1,131 @@
|
||||
<script setup lang="ts">
|
||||
/**
|
||||
* Where a rule lives, and moving it (milestone 414).
|
||||
*
|
||||
* A rule's home IS its reach: in a rulebook topic it is global and applies to
|
||||
* every project; on a project it applies there alone. Moving keeps the rule's
|
||||
* id, edit history, areas and relations — the reason this is a move and not
|
||||
* "recreate it over there and delete this one".
|
||||
*
|
||||
* One component for both places a rule is read: the rule editor (a global
|
||||
* rule) and a project's rules tab (a project rule). Each would otherwise grow
|
||||
* its own picker, and the two would drift on what a destination is.
|
||||
*/
|
||||
import { computed, onMounted, ref } from "vue";
|
||||
import { apiErrorMessage, apiGet } from "@/api/client";
|
||||
import { listRulebooks, listTopics, moveRule, type Rule } from "@/api/rulebooks";
|
||||
|
||||
const props = defineProps<{
|
||||
ruleId: number;
|
||||
topicId: number | null;
|
||||
projectId: number | null;
|
||||
}>();
|
||||
const emit = defineEmits<{ moved: [rule: Rule] }>();
|
||||
|
||||
interface TopicChoice { id: number; label: string }
|
||||
interface ProjectChoice { id: number; title: string }
|
||||
|
||||
const topics = ref<TopicChoice[]>([]);
|
||||
const projects = ref<ProjectChoice[]>([]);
|
||||
// "topic:12" / "project:4" — one select, two kinds of destination.
|
||||
const destination = ref("");
|
||||
const loading = ref(true);
|
||||
const moving = ref(false);
|
||||
const error = ref("");
|
||||
|
||||
const currentLabel = computed(() => {
|
||||
if (props.topicId !== null) {
|
||||
const t = topics.value.find((x) => x.id === props.topicId);
|
||||
return t ? `Global — ${t.label}` : "Global";
|
||||
}
|
||||
const p = projects.value.find((x) => x.id === props.projectId);
|
||||
return p ? `Project — ${p.title}` : "Project";
|
||||
});
|
||||
|
||||
async function load() {
|
||||
loading.value = true;
|
||||
error.value = "";
|
||||
try {
|
||||
const [books, proj] = await Promise.all([
|
||||
listRulebooks(),
|
||||
apiGet<{ projects: ProjectChoice[] }>("/api/projects"),
|
||||
]);
|
||||
const perBook = await Promise.all(books.map(async (rb) => {
|
||||
const ts = await listTopics(rb.id);
|
||||
return ts.map((t) => ({ id: t.id, label: `${rb.title} › ${t.title}` }));
|
||||
}));
|
||||
topics.value = perBook.flat();
|
||||
projects.value = proj.projects.map((p) => ({ id: p.id, title: p.title }));
|
||||
} catch (e: unknown) {
|
||||
error.value = apiErrorMessage(e, "Could not load where this rule could live");
|
||||
} finally {
|
||||
loading.value = false;
|
||||
}
|
||||
}
|
||||
|
||||
async function move() {
|
||||
const [kind, raw] = destination.value.split(":");
|
||||
const id = Number(raw);
|
||||
if (!id) return;
|
||||
const where = kind === "topic"
|
||||
? "global — it will apply to every project"
|
||||
: "this project's only — other projects will stop receiving it";
|
||||
if (!confirm(`Move this rule? It becomes ${where}.`)) return;
|
||||
moving.value = true;
|
||||
error.value = "";
|
||||
try {
|
||||
const rule = await moveRule(props.ruleId, kind === "topic" ? { topic_id: id } : { project_id: id });
|
||||
destination.value = "";
|
||||
emit("moved", rule);
|
||||
} catch (e: unknown) {
|
||||
error.value = apiErrorMessage(e, "Could not move the rule");
|
||||
} finally {
|
||||
moving.value = false;
|
||||
}
|
||||
}
|
||||
|
||||
onMounted(load);
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<fieldset class="rule-home">
|
||||
<legend>Where this rule applies</legend>
|
||||
<p v-if="loading" class="state-msg">Loading…</p>
|
||||
<template v-else>
|
||||
<p class="rule-home-current">{{ currentLabel }}</p>
|
||||
<div class="rule-home-move">
|
||||
<select v-model="destination" class="fs-input" aria-label="Move this rule to">
|
||||
<option value="">Move to…</option>
|
||||
<optgroup v-if="topics.length" label="Global (a rulebook topic)">
|
||||
<option
|
||||
v-for="t in topics.filter((x) => x.id !== topicId)"
|
||||
:key="`topic-${t.id}`"
|
||||
:value="`topic:${t.id}`"
|
||||
>{{ t.label }}</option>
|
||||
</optgroup>
|
||||
<optgroup v-if="projects.length" label="One project">
|
||||
<option
|
||||
v-for="p in projects.filter((x) => x.id !== projectId)"
|
||||
:key="`project-${p.id}`"
|
||||
:value="`project:${p.id}`"
|
||||
>{{ p.title }}</option>
|
||||
</optgroup>
|
||||
</select>
|
||||
<button
|
||||
type="button"
|
||||
class="btn-secondary btn-compact"
|
||||
:disabled="!destination || moving"
|
||||
@click="move"
|
||||
>{{ moving ? "Moving…" : "Move" }}</button>
|
||||
</div>
|
||||
</template>
|
||||
<p v-if="error" class="error-msg">{{ error }}</p>
|
||||
</fieldset>
|
||||
</template>
|
||||
|
||||
<style scoped>
|
||||
.rule-home { margin-bottom: 1rem; }
|
||||
.rule-home-current { margin: 0 0 0.5rem; font-size: 0.88rem; }
|
||||
.rule-home-move { display: flex; gap: var(--fs-space-2); align-items: center; }
|
||||
.rule-home-move select { flex: 1; min-width: 0; }
|
||||
</style>
|
||||
@@ -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,
|
||||
|
||||
@@ -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"
|
||||
},
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -297,6 +297,27 @@ async def unrelate_rules(relation_id: int):
|
||||
return "", 204
|
||||
|
||||
|
||||
@rulebooks_bp.post("/rules/<int:rule_id>/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/<int:rule_id>")
|
||||
@login_required
|
||||
async def delete_rule(rule_id: int):
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
):
|
||||
|
||||
Reference in New Issue
Block a user