CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 15s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 57s
CI & Build / Build & push image (push) Successful in 29s
A note or task created through MCP was not semantically searchable until the next restart's backfill ran. Embedding fired at the five REST route handlers and nowhere else; the MCP tools call the service directly, so they skipped it. The shape of this bug is the reason to care: it is invisible on an instance that redeploys constantly (this one does, per rule 46) and permanent on one that doesn't. Rule 115 — the product has to stand up for the install that restarts twice a year, not just for the one that restarts hourly. Moved to services/notes.embed_note(), called from create_note and update_note, and deleted from all five routes. Every caller — REST, MCP, recurrence, snippets — now gets it by construction rather than by remembering. Two things fall out of having one implementation instead of six: - It uses note.user_id, the OWNER. The routes were inconsistent: some passed the caller's uid, some the owner's. On a shared record the caller's id mints a second embedding row that nothing reads. - services/snippets.py's _embed_snippet existed only because snippets are created via MCP and the routes couldn't cover them. Every one of its four call sites goes through notes_svc, so the helper and its four calls are gone, along with the eight test patches that existed to neutralise it. RuntimeError (no running loop — unit tests, scripts) and any indexing failure are both swallowed: a write that succeeded must not be failed by its index. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
181 lines
7.5 KiB
Python
181 lines
7.5 KiB
Python
"""Tests for un-merge (#2165) — reversing one source out of a merged survivor.
|
|
|
|
The design question this task said to settle first was how to subtract without
|
|
stripping call sites the survivor legitimately owns. The answer is per-source
|
|
attribution recorded AT MERGE TIME: each `merged_from` entry holds only what
|
|
that source actually added. These tests pin that rule, and the refusal that
|
|
guards the case where the attribution isn't there.
|
|
"""
|
|
from types import SimpleNamespace
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
from scribe.services import snippets as s
|
|
|
|
|
|
def _loc(path, repo="r", symbol=""):
|
|
return {"repo": repo, "path": path, "symbol": symbol}
|
|
|
|
|
|
def _survivor(locations, merged_from, tags=None, owner=7):
|
|
"""A survivor note whose `data` carries locations + merge provenance."""
|
|
return SimpleNamespace(
|
|
id=1, user_id=owner, note_type="snippet", deleted_at=None,
|
|
title="f — does a thing", tags=tags or ["python", "snippet"],
|
|
body=s.compose_body(code="x = 1", language="python", locations=locations,
|
|
merged_from=merged_from),
|
|
data=s.compose_data(name="f", when_to_use="does a thing", language="python",
|
|
code="x = 1", locations=locations,
|
|
merged_from=merged_from),
|
|
)
|
|
|
|
|
|
async def _run_unmerge(survivor, *, source_alive=None, restore=1):
|
|
"""Drive unmerge_snippet with the DB stubbed; return update_note's kwargs."""
|
|
async def fake_get(_uid, sid):
|
|
if sid == survivor.id:
|
|
return survivor
|
|
return source_alive
|
|
|
|
with (
|
|
patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)),
|
|
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
|
|
patch("scribe.services.trash.restore_entity", AsyncMock(return_value=restore)),
|
|
patch.object(s.notes_svc, "update_note",
|
|
AsyncMock(return_value=survivor)) as upd,
|
|
):
|
|
await s.unmerge_snippet(7, 1, 2)
|
|
return upd.await_args.kwargs
|
|
|
|
|
|
# --- the subtraction ------------------------------------------------------
|
|
|
|
|
|
async def test_unmerge_strips_only_what_the_source_contributed():
|
|
"""The survivor's own location survives; the source's is removed."""
|
|
mine, theirs = _loc("mine.py"), _loc("theirs.py")
|
|
survivor = _survivor(
|
|
[mine, theirs],
|
|
[{"id": 2, "locations": [theirs], "tags": ["helper"]}],
|
|
tags=["python", "snippet", "helper", "core"],
|
|
)
|
|
kwargs = await _run_unmerge(survivor)
|
|
assert kwargs["data"]["locations"] == [mine]
|
|
assert "helper" not in kwargs["tags"]
|
|
assert "core" in kwargs["tags"]
|
|
|
|
|
|
async def test_a_location_the_survivor_also_owned_is_never_stripped():
|
|
"""The central hazard the task named. If a source brought a location the
|
|
survivor ALREADY had, merge attributes nothing to it — so reversing must
|
|
leave that call site in place."""
|
|
shared = _loc("shared.py")
|
|
survivor = _survivor([shared], [{"id": 2, "locations": [], "tags": ["t"]}])
|
|
kwargs = await _run_unmerge(survivor)
|
|
assert kwargs["data"]["locations"] == [shared]
|
|
|
|
|
|
async def test_the_reversed_entry_leaves_the_provenance_list():
|
|
survivor = _survivor(
|
|
[_loc("a.py"), _loc("b.py")],
|
|
[{"id": 2, "locations": [_loc("b.py")]}, {"id": 3, "locations": []}],
|
|
)
|
|
kwargs = await _run_unmerge(survivor)
|
|
assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [3]
|
|
assert "#2" not in kwargs["body"]
|
|
# The other source's history is untouched — un-merge reverses one thing.
|
|
assert "**Merged from:** #3" in kwargs["body"]
|
|
|
|
|
|
async def test_unmerging_the_last_source_clears_the_provenance_line():
|
|
survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": [], "tags": ["t"]}])
|
|
kwargs = await _run_unmerge(survivor)
|
|
assert "Merged from" not in kwargs["body"]
|
|
|
|
|
|
# --- restoring the source -------------------------------------------------
|
|
|
|
|
|
async def test_an_already_restored_source_is_repaired_not_refused():
|
|
"""The scenario that motivated the feature: the operator restored the source
|
|
from the trash by hand, so both records claim its call sites and nothing ever
|
|
stripped the survivor's copy. Un-merge must fix that, not reject it."""
|
|
theirs = _loc("theirs.py")
|
|
survivor = _survivor([_loc("mine.py"), theirs],
|
|
[{"id": 2, "locations": [theirs]}])
|
|
alive = SimpleNamespace(id=2, user_id=7, note_type="snippet", deleted_at=None,
|
|
title="g — x", tags=["snippet"], body="", data=None)
|
|
with patch("scribe.services.trash.restore_entity", AsyncMock()) as revive:
|
|
kwargs = await _run_unmerge(survivor, source_alive=alive)
|
|
# Nothing to revive — it's already alive — but the subtraction still happens.
|
|
revive.assert_not_called()
|
|
assert kwargs["data"]["locations"] == [_loc("mine.py")]
|
|
|
|
|
|
async def test_a_purged_source_is_refused_and_the_survivor_is_untouched():
|
|
"""If the source can't come back, stripping the survivor would lose the
|
|
locations entirely — no record would claim them."""
|
|
survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": [_loc("a.py")]}])
|
|
|
|
async def fake_get(_uid, sid):
|
|
return survivor if sid == 1 else None
|
|
|
|
with (
|
|
patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)),
|
|
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
|
|
patch("scribe.services.trash.restore_entity", AsyncMock(return_value=None)),
|
|
patch.object(s.notes_svc, "update_note", AsyncMock()) as upd,
|
|
):
|
|
with pytest.raises(s.UnmergeError, match="purged"):
|
|
await s.unmerge_snippet(7, 1, 2)
|
|
upd.assert_not_called()
|
|
|
|
|
|
# --- refusals -------------------------------------------------------------
|
|
|
|
|
|
async def test_an_entry_without_attribution_is_refused_not_guessed():
|
|
"""Bare-id provenance comes from parsing the body, which can only hold ids.
|
|
Subtracting a guess could strip call sites the survivor owns — so refuse and
|
|
say what to do instead."""
|
|
survivor = _survivor([_loc("a.py")], [2])
|
|
|
|
async def fake_get(_uid, sid):
|
|
return survivor if sid == 1 else None
|
|
|
|
with (
|
|
patch.object(s, "get_snippet", AsyncMock(side_effect=fake_get)),
|
|
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
|
|
patch.object(s.notes_svc, "update_note", AsyncMock()) as upd,
|
|
):
|
|
with pytest.raises(s.UnmergeError, match="provenance"):
|
|
await s.unmerge_snippet(7, 1, 2)
|
|
upd.assert_not_called()
|
|
|
|
|
|
async def test_unmerging_something_never_absorbed_is_refused():
|
|
survivor = _survivor([_loc("a.py")], [{"id": 99, "locations": []}])
|
|
with (
|
|
patch.object(s, "get_snippet", AsyncMock(return_value=survivor)),
|
|
patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)),
|
|
):
|
|
with pytest.raises(s.UnmergeError, match="no record of absorbing"):
|
|
await s.unmerge_snippet(7, 1, 2)
|
|
|
|
|
|
async def test_unmerge_requires_write_access():
|
|
"""Same rule as merge: a read-only share can see the record, not rearrange it."""
|
|
survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": []}])
|
|
with (
|
|
patch.object(s, "get_snippet", AsyncMock(return_value=survivor)),
|
|
patch("scribe.services.access.can_write_note", AsyncMock(return_value=False)),
|
|
):
|
|
with pytest.raises(PermissionError):
|
|
await s.unmerge_snippet(7, 1, 2)
|
|
|
|
|
|
async def test_unmerge_on_a_missing_survivor_returns_none():
|
|
with patch.object(s, "get_snippet", AsyncMock(return_value=None)):
|
|
assert await s.unmerge_snippet(7, 1, 2) is None
|