feat(systems): a System names its files — path patterns stored, validated and matched (milestone 444 step 3, #4756)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 15s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / integration (push) Successful in 1m6s
CI & Build / Python tests (push) Successful in 1m50s
CI & Build / Build & push image (push) Successful in 43s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 15s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / integration (push) Successful in 1m6s
CI & Build / Python tests (push) Successful in 1m50s
CI & Build / Build & push image (push) Successful in 43s
A System gains path_patterns: globs relative to the repo root (* within one directory, ** across any depth, a plain directory covering everything under it). One service validates them for every door, so the web UI and MCP refuse the same bad pattern with the same message. systems_for_paths resolves paths to every active System that covers them, which step 4 (#4757) uses to deliver an area's rulings when its files are touched. - schema: systems.path_patterns JSONB NOT NULL default [] (migration 0113) - service: normalize_path_patterns, path_matches, systems_for_paths - routes + MCP create_system/update_system accept it; [] clears - web UI: a Files field in the create and edit forms, patterns on the card - backup carries it through export and restore - using-scribe reflex 7: tagging work keeps a System's files current Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -205,6 +205,9 @@ TOPICS: tuple[Topic, ...] = (
|
||||
Topic("code says what a thing does, not what was wanted", U,
|
||||
("rulings", "unconfirmed"),
|
||||
"code tells you what a thing does, not what was wanted"),
|
||||
Topic("a System names its files, and tagging work keeps them current", U,
|
||||
("path_patterns", "update_system"),
|
||||
"which files are which area is your call"),
|
||||
# ── process arcs — owned by their skills ──
|
||||
Topic("plan in a milestone, steps created together", "skill:writing-plans", ("start_planning", "{{ref:"),
|
||||
"a milestone earns its place when the work has an arc", index=("start_planning",)),
|
||||
|
||||
@@ -101,6 +101,23 @@ async def test_get_system_splits_records_by_kind():
|
||||
assert [r["id"] for r in result["notes"]] == [12]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_path_patterns_reach_the_service_from_both_tools():
|
||||
"""Omitted means unchanged; an empty list is a value — it clears them."""
|
||||
with patch("scribe.mcp.tools.systems.current_user_id", return_value=1), \
|
||||
patch("scribe.mcp.tools.systems.systems_svc") as svc:
|
||||
svc.assess_system_name = AsyncMock(return_value=_NO_MATCH)
|
||||
svc.create_system = AsyncMock(return_value=fake_system(name="Billing"))
|
||||
svc.update_system = AsyncMock(return_value=fake_system(name="Billing"))
|
||||
from scribe.mcp.tools.systems import create_system, update_system
|
||||
await create_system(project_id=5, name="Billing", path_patterns=["src/billing"])
|
||||
assert svc.create_system.await_args.kwargs["path_patterns"] == ["src/billing"]
|
||||
await update_system(system_id=1, name="Billing")
|
||||
assert "path_patterns" not in svc.update_system.await_args.kwargs
|
||||
await update_system(system_id=1, path_patterns=[])
|
||||
assert svc.update_system.await_args.kwargs["path_patterns"] == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_system_not_found_raises():
|
||||
with patch("scribe.mcp.tools.systems.current_user_id", return_value=1), \
|
||||
|
||||
@@ -126,3 +126,127 @@ async def test_assess_fails_open_so_a_naming_aid_cannot_block_a_create():
|
||||
async def test_assess_says_nothing_about_a_nameless_system():
|
||||
from scribe.services.systems import assess_system_name
|
||||
assert await assess_system_name(1, 5, " ") == {"duplicate": None, "canonical": None}
|
||||
|
||||
|
||||
# ── path patterns — the files that ARE the area (milestone 444, #4756) ──
|
||||
|
||||
def test_patterns_are_tidied_to_repo_relative_and_deduplicated():
|
||||
from scribe.services.systems import normalize_path_patterns
|
||||
out = normalize_path_patterns([
|
||||
" ./src/billing/ ", "/src/billing", "", "src\\api\\*.py", " ",
|
||||
"frontend/src/**/Billing*.vue",
|
||||
])
|
||||
assert out == ["src/billing", "src/api/*.py", "frontend/src/**/Billing*.vue"]
|
||||
assert normalize_path_patterns(None) == []
|
||||
assert normalize_path_patterns([]) == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad", [
|
||||
"src/billing", # a bare string, not a list
|
||||
["../other-repo/src"], # leaves the repo
|
||||
["src/../../etc"],
|
||||
[42],
|
||||
["x" * 301],
|
||||
[f"src/f{i}.py" for i in range(51)],
|
||||
])
|
||||
def test_patterns_that_cannot_mean_an_area_are_refused(bad):
|
||||
from scribe.services.systems import normalize_path_patterns
|
||||
with pytest.raises(ValueError):
|
||||
normalize_path_patterns(bad)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("pattern,path,expected", [
|
||||
# A plain directory covers everything under it, at any depth.
|
||||
("src/billing", "src/billing/invoice.py", True),
|
||||
("src/billing", "src/billing/deep/er/x.py", True),
|
||||
("src/billing", "src/billing", True),
|
||||
("src/billing", "src/billingual/x.py", False),
|
||||
# `*` stays inside one segment; `**` spans any number, including none.
|
||||
("src/*.py", "src/app.py", True),
|
||||
("src/*.py", "src/pkg/app.py", False),
|
||||
("src/**/*.py", "src/pkg/sub/app.py", True),
|
||||
("src/**/*.py", "src/app.py", True),
|
||||
("**/Billing*.vue", "frontend/src/components/BillingCard.vue", True),
|
||||
# Case counts, and the path is read the way the pattern was written.
|
||||
("src/billing", "Src/billing/x.py", False),
|
||||
("src/billing", "./src/billing/x.py", True),
|
||||
("src/billing", "", False),
|
||||
])
|
||||
def test_path_matches(pattern, path, expected):
|
||||
from scribe.services.systems import path_matches
|
||||
assert path_matches(pattern, path) is expected
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_systems_for_paths_returns_every_area_a_path_belongs_to():
|
||||
"""Areas overlap, and each one a path belongs to answers for it — the
|
||||
lookup never picks a winner. A System that named no files claims none."""
|
||||
api = MagicMock(id=1, path_patterns=["src/scribe/routes", "src/scribe/mcp/tools"])
|
||||
data = MagicMock(id=2, path_patterns=["src/scribe/models", "src/scribe/**/systems.py"])
|
||||
unnamed = MagicMock(id=3, path_patterns=[])
|
||||
with patch("scribe.services.systems.list_systems",
|
||||
AsyncMock(return_value=[api, data, unnamed])):
|
||||
from scribe.services.systems import systems_for_paths
|
||||
out = await systems_for_paths(1, 5, [
|
||||
"src/scribe/routes/systems.py", "./README.md", "",
|
||||
])
|
||||
assert [(s.id, paths) for s, paths in out] == [
|
||||
(1, ["src/scribe/routes/systems.py"]),
|
||||
(2, ["src/scribe/routes/systems.py"]),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_systems_for_paths_with_no_paths_reads_nothing():
|
||||
lister = AsyncMock(return_value=[])
|
||||
with patch("scribe.services.systems.list_systems", lister):
|
||||
from scribe.services.systems import systems_for_paths
|
||||
assert await systems_for_paths(1, 5, ["", " "]) == []
|
||||
lister.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_refuses_a_bad_pattern_before_touching_the_row():
|
||||
with patch("scribe.services.systems.async_session") as mock_cls:
|
||||
from scribe.services.systems import update_system
|
||||
with pytest.raises(ValueError):
|
||||
await update_system(1, 9, path_patterns=["../elsewhere"])
|
||||
mock_cls.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_with_an_empty_list_clears_the_patterns():
|
||||
"""`[]` is a value, not "leave unchanged" — the service skips only None."""
|
||||
system = MagicMock(deleted_at=None, project_id=5, path_patterns=["src/old"])
|
||||
session = make_mock_session()
|
||||
session.get = AsyncMock(return_value=system)
|
||||
with patch("scribe.services.systems.async_session", return_value=session), \
|
||||
patch("scribe.services.systems.access") as acc, \
|
||||
patch("scribe.services.systems.embed_system"):
|
||||
acc.can_write_project = AsyncMock(return_value=True)
|
||||
from scribe.services.systems import update_system
|
||||
await update_system(1, 9, path_patterns=[])
|
||||
assert system.path_patterns == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_stores_tidied_patterns():
|
||||
mock_session = make_mock_session()
|
||||
captured = {}
|
||||
mock_session.add = MagicMock(side_effect=lambda obj: captured.update(
|
||||
patterns=getattr(obj, "path_patterns", "MISSING")))
|
||||
with patch("scribe.services.systems.async_session", return_value=mock_session), \
|
||||
patch("scribe.services.systems.access") as acc, \
|
||||
patch("scribe.services.systems.embed_system"):
|
||||
acc.can_write_project = AsyncMock(return_value=True)
|
||||
from scribe.services.systems import create_system
|
||||
await create_system(1, 5, "Billing", path_patterns=["./src/billing/"])
|
||||
assert captured["patterns"] == ["src/billing"]
|
||||
|
||||
|
||||
def test_system_to_dict_carries_its_patterns():
|
||||
from scribe.models.system import System
|
||||
system = System(id=1, user_id=1, project_id=5, name="Billing",
|
||||
path_patterns=["src/billing"])
|
||||
assert system.to_dict()["path_patterns"] == ["src/billing"]
|
||||
assert System(id=2, user_id=1, project_id=5, name="X").to_dict()["path_patterns"] == []
|
||||
|
||||
Reference in New Issue
Block a user