fix(snippets): close the recall-surface gaps found reviewing the Drafter

Four defects from the 2026-07-25 review of the recall (#227) and merge (#231)
milestones. The theme: a snippet could be recorded but not fully corrected, and
the agent and web surfaces had drifted apart.

- #2076 language was mis-derived from the first caller tag, so a snippet created
  with tags and no language read that tag back as its language — corrupting the
  tag set and the code fence on the next update. Only the FIRST tag can carry
  the language, since compose_tags emits [language, "snippet", *caller].
- #2077 MCP update_snippet mapped "" to "unchanged", so no field could ever be
  cleared and no snippet detached from its project. Now an omitted field is left
  alone, an empty string clears, and project_id follows the -1 = detach
  convention. A service-level UNSET sentinel keeps None available as the clear.
- #2078 surface parity: adds delete_snippet (MCP had none, so a wrong snippet
  could not be retired by the agent that recorded it), locations on MCP create
  and update, system_ids through the REST routes and the editor, and the
  near-duplicate gate on REST create with a "record it anyway" escape.
- #2079 project scoping: list_snippets takes project_id through the service, the
  MCP tool and the REST route, defaulting to every project — reaching across
  projects is the point when the helper you need was written elsewhere.

Sharing the list across owners is deliberately NOT in here: query_knowledge is
shared with the Knowledge browse surface, so widening it changes behaviour well
beyond snippets. Left open on #2079 for a scope decision.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLwAaV4DQEmVyn496HnEvt

tests/test_mcp_tool_snippets.py

@@ -82,6 +82,73 @@ async def test_update_snippet_missing_raises():
             await update_snippet(123, name="x")
 
 
+@pytest.mark.asyncio
+async def test_update_snippet_empty_string_clears_a_field():
+    # An omitted field must stay None ("leave alone"), but an explicit empty
+    # string has to reach the service as "" so a stale field can be removed.
+    updated = _fake_snippet()
+    with patch("scribe.services.snippets.update_snippet",
+               AsyncMock(return_value=updated)) as mock_update:
+        from scribe.mcp.tools.snippets import update_snippet
+        await update_snippet(1, signature="")
+    kwargs = mock_update.await_args.kwargs
+    assert kwargs["signature"] == ""      # cleared
+    assert kwargs["name"] is None         # untouched
+
+
+@pytest.mark.asyncio
+async def test_update_snippet_project_id_conventions():
+    from scribe.services import snippets as snippets_svc
+    updated = _fake_snippet()
+    cases = {0: snippets_svc.UNSET, -1: None, 5: 5}
+    for given, expected in cases.items():
+        with patch("scribe.services.snippets.update_snippet",
+                   AsyncMock(return_value=updated)) as mock_update:
+            from scribe.mcp.tools.snippets import update_snippet
+            await update_snippet(1, project_id=given)
+        assert mock_update.await_args.kwargs["project_id"] is expected
+
+
+@pytest.mark.asyncio
+async def test_create_and_update_pass_locations_through():
+    locs = [{"repo": "a", "path": "a.py", "symbol": "f"},
+            {"repo": "b", "path": "b.py", "symbol": "g"}]
+    created = _fake_snippet()
+    with patch("scribe.services.dedup.find_duplicate_note", AsyncMock(return_value=None)), \
+         patch("scribe.services.snippets.create_snippet",
+               AsyncMock(return_value=created)) as mock_create:
+        from scribe.mcp.tools.snippets import create_snippet
+        await create_snippet(name="f", code="x", locations=locs)
+    assert mock_create.await_args.kwargs["locations"] == locs
+
+    with patch("scribe.services.snippets.update_snippet",
+               AsyncMock(return_value=created)) as mock_update:
+        from scribe.mcp.tools.snippets import update_snippet
+        await update_snippet(1, locations=locs)
+    assert mock_update.await_args.kwargs["locations"] == locs
+
+
+@pytest.mark.asyncio
+async def test_delete_snippet_retires_or_raises():
+    from scribe.mcp.tools.snippets import delete_snippet
+    with patch("scribe.services.snippets.delete_snippet", AsyncMock(return_value=True)):
+        assert await delete_snippet(1) == {"deleted": True, "id": 1}
+    with patch("scribe.services.snippets.delete_snippet", AsyncMock(return_value=False)):
+        with pytest.raises(ValueError):
+            await delete_snippet(404)
+
+
+@pytest.mark.asyncio
+async def test_list_snippets_defaults_to_every_project():
+    with patch("scribe.services.snippets.list_snippets",
+               AsyncMock(return_value=([], 0))) as mock_list:
+        from scribe.mcp.tools.snippets import list_snippets
+        await list_snippets(q="debounce")
+        assert mock_list.await_args.kwargs["project_id"] is None
+        await list_snippets(q="debounce", project_id=3)
+        assert mock_list.await_args.kwargs["project_id"] == 3
+
+
 @pytest.mark.asyncio
 async def test_merge_snippets_requires_a_source():
     from scribe.mcp.tools.snippets import merge_snippets
@@ -128,5 +195,5 @@ def test_register_attaches_all_tools():
     snippets.register(FakeMcp())
     assert set(names) == {
         "list_snippets", "create_snippet", "get_snippet", "update_snippet",
-        "merge_snippets",
+        "delete_snippet", "merge_snippets",
     }

tests/test_routes_snippets.py

@@ -27,12 +27,51 @@ def test_snippet_handlers_callable():
 def test_service_functions_take_user_id():
     """Routes must call snippet services with user_id — verify the contract."""
     from scribe.services import snippets as svc
-    for fn_name in ("create_snippet", "list_snippets", "get_snippet", "update_snippet"):
+    for fn_name in (
+        "create_snippet", "list_snippets", "get_snippet", "update_snippet",
+        "delete_snippet", "merge_snippets",
+    ):
         fn = getattr(svc, fn_name)
         assert callable(fn)
         assert "user_id" in inspect.signature(fn).parameters
 
 
+def test_agent_and_web_surfaces_stay_at_parity():
+    """The MCP tools and the REST routes are two callers of one service; a
+    capability on one has to exist on the other (rule #33). This guard exists
+    because they drifted apart once: MCP had no delete or `locations`, and the
+    web side had no `system_ids` and no near-duplicate gate."""
+    from scribe.mcp.tools import snippets as tools
+    from scribe.routes import snippets as routes
+
+    # Every write verb the web surface offers, the agent surface offers too.
+    for verb in ("create", "get", "list", "update", "delete", "merge"):
+        assert callable(getattr(tools, f"{verb}_snippets", None) or
+                        getattr(tools, f"{verb}_snippet", None)), f"MCP lacks {verb}"
+
+    # Multi-location records are reachable from both.
+    assert "locations" in inspect.signature(tools.create_snippet).parameters
+    assert "locations" in inspect.signature(tools.update_snippet).parameters
+    assert "locations" in inspect.getsource(routes.create_snippet_route)
+    assert "locations" in inspect.getsource(routes.update_snippet_route)
+
+    # System association and the duplicate gate reach both.
+    assert "system_ids" in inspect.signature(tools.create_snippet).parameters
+    for route in (routes.create_snippet_route, routes.update_snippet_route):
+        assert "system_ids" in inspect.getsource(route)
+    assert "find_duplicate_note" in inspect.getsource(routes.create_snippet_route)
+
+
+def test_project_scoping_reaches_every_caller():
+    """A snippet search has to be narrowable to one project from both surfaces."""
+    from scribe.mcp.tools import snippets as tools
+    from scribe.routes import snippets as routes
+    from scribe.services import snippets as svc
+    assert "project_id" in inspect.signature(svc.list_snippets).parameters
+    assert "project_id" in inspect.signature(tools.list_snippets).parameters
+    assert "project_id" in inspect.getsource(routes.list_snippets_route)
+
+
 def test_update_field_map_matches_service_kwargs():
     """Every field the PATCH route forwards must be a real update_snippet kwarg
     (rule #33 interface-contract parity)."""

tests/test_services_snippets.py

@@ -62,6 +62,26 @@ def test_parse_falls_back_to_tag_for_language():
     assert got["language"] == "ruby"
 
 
+def test_parse_does_not_promote_a_caller_tag_to_language():
+    # compose_tags puts the language FIRST, so a leading "snippet" marker means
+    # no language was recorded — the tags after it are the caller's own and must
+    # not be mistaken for one (which would also corrupt the code fence on the
+    # next update).
+    tags = s.compose_tags("", ["auth"])
+    assert tags == ["snippet", "auth"]
+    got = s.parse_snippet_fields("n — u", s.compose_body(code="x = 1"), tags)
+    assert got["language"] == ""
+
+
+def test_caller_tag_survives_an_update_round_trip_without_a_language():
+    # Regression: the tag used to be read back as the language, then dropped
+    # from the extra-tag set on re-compose — so it silently disappeared.
+    tags = s.compose_tags("", ["auth"])
+    fields = s.parse_snippet_fields("n — u", s.compose_body(code="x = 1"), tags)
+    extra = [t for t in tags if t not in (s.SNIPPET_TAG, fields["language"])]
+    assert s.compose_tags(fields["language"], extra) == ["snippet", "auth"]
+
+
 def test_compose_body_multi_location_renders_bullet_list():
     body = s.compose_body(
         code="x = 1",