CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 1m2s
CI & Build / integration (push) Successful in 1m0s
CI & Build / Python tests (push) Successful in 1m39s
CI & Build / Build & push image (push) Successful in 39s
Sessions predicted the ids their next creates would get and wrote them into
plan bodies and reference notes before the records existed. The database
never collides; the sequence is shared by every session and user, so any
concurrent create took the guessed numbers and the references pointed at
someone else's records.
- create_records (new MCP tool) and start_planning(body=, steps=) create
their records in ONE transaction: insert, flush for the real ids, rewrite
{{ref:N}} / {{ref:milestone}} placeholders as #id "title", commit. No
prediction, no waiting, no stub records left behind when a batch fails.
Ids need not be consecutive and nothing depends on it.
- Every MCP create/update of a note, task or milestone refuses a #N sitting
just above the highest assigned id (within 50): that can only be a guess.
Refusal, not warning. Numbers far above the max (PRs, forge issues) pass.
- notes.build_note splits validation out of create_note so the batch
validates records exactly as a single create does.
- writing-plans and using-scribe say to pass steps up front and never write
an unassigned id; plugin version minted.
Integration test runs six concurrent batches and checks each resolves its
placeholders to its own records, and that a failing batch writes nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
171 lines
7.9 KiB
Python
171 lines
7.9 KiB
Python
"""The batch create door, and the guessed-id refusal at every create/update door (#4016).
|
|
|
|
What the unit lane can pin: the door refuses a guessed id BEFORE the duplicate
|
|
gate or any write, a batch is refused whole, and a malformed record fails with
|
|
a message naming it. That the ids really come back resolved and atomic needs
|
|
Postgres — see test_integration_record_batch.py.
|
|
"""
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
from scribe.services import record_refs
|
|
from scribe.services.record_batch import BatchItem, BatchMilestone, check_batch
|
|
|
|
pytestmark = pytest.mark.usefixtures("_bind_user")
|
|
|
|
|
|
def _max(n: int):
|
|
return patch.object(record_refs, "_max_note_id", AsyncMock(return_value=n))
|
|
|
|
|
|
# ── the refusal reaches every door ─────────────────────────────────────────
|
|
|
|
|
|
@pytest.mark.parametrize(("module", "name", "kwargs"), [
|
|
("scribe.mcp.tools.tasks", "create_task", {"title": "t", "body": "after #101"}),
|
|
("scribe.mcp.tools.tasks", "update_task", {"task_id": 5, "body": "see #101"}),
|
|
("scribe.mcp.tools.notes", "create_note", {"title": "n", "body": "index: #101"}),
|
|
("scribe.mcp.tools.notes", "update_note", {"note_id": 5, "body": "index: #101"}),
|
|
("scribe.mcp.tools.milestones", "create_milestone", {"project_id": 1, "title": "m", "body": "#101"}),
|
|
("scribe.mcp.tools.milestones", "update_milestone", {"project_id": 1, "milestone_id": 2, "body": "#101"}),
|
|
("scribe.mcp.tools.tasks", "start_planning", {"project_id": 1, "title": "p", "body": "step #101"}),
|
|
("scribe.mcp.tools.tasks", "create_records", {"records": [{"title": "a", "body": "#101"}]}),
|
|
])
|
|
async def test_every_writing_door_refuses_a_guessed_id_before_writing(module, name, kwargs):
|
|
import importlib
|
|
|
|
mod = importlib.import_module(module)
|
|
tool = getattr(mod, name)
|
|
# Every service a door could reach is a MagicMock that must stay untouched:
|
|
# the refusal has to land before the duplicate gate and before the write.
|
|
with _max(100), \
|
|
patch("scribe.services.dedup.find_duplicate_note", AsyncMock()) as dedup, \
|
|
patch("scribe.services.notes.create_note", AsyncMock()) as create, \
|
|
patch("scribe.services.notes.update_note", AsyncMock()) as update, \
|
|
patch("scribe.services.milestones.create_milestone", AsyncMock()) as ms_create, \
|
|
patch("scribe.services.milestones.update_milestone", AsyncMock()) as ms_update, \
|
|
patch("scribe.services.record_batch.create_batch", AsyncMock()) as batch, \
|
|
patch("scribe.services.planning.start_planning", AsyncMock()) as plan:
|
|
with pytest.raises(ValueError, match="#101"):
|
|
await tool(**kwargs)
|
|
for mock in (dedup, create, update, ms_create, ms_update, batch, plan):
|
|
mock.assert_not_awaited()
|
|
|
|
|
|
async def test_a_real_id_passes_the_door():
|
|
from scribe.mcp.tools.tasks import create_task
|
|
|
|
fake = MagicMock(id=90, project_id=None)
|
|
fake.to_dict.return_value = {"id": 90}
|
|
with _max(100), \
|
|
patch("scribe.mcp.tools.tasks.dedup_svc.find_duplicate_note", AsyncMock(return_value=None)), \
|
|
patch("scribe.mcp.tools.tasks.notes_svc.create_note", AsyncMock(return_value=fake)), \
|
|
patch("scribe.mcp.tools.tasks.systems_tools.attach_systems", AsyncMock()):
|
|
out = await create_task(title="t", body="follows #42")
|
|
assert out["id"] == 90
|
|
|
|
|
|
# ── create_records ─────────────────────────────────────────────────────────
|
|
|
|
|
|
async def test_unknown_record_fields_are_refused_not_dropped():
|
|
from scribe.mcp.tools.tasks import create_records
|
|
|
|
with pytest.raises(ValueError, match=r"record 2 has unknown field\(s\) \['milestone'\]"):
|
|
await create_records(records=[{"title": "a"}, {"title": "b", "milestone": 4}])
|
|
|
|
|
|
async def test_a_record_type_must_be_task_or_note():
|
|
from scribe.mcp.tools.tasks import create_records
|
|
|
|
with pytest.raises(ValueError, match="type must be 'task' or 'note'"):
|
|
await create_records(records=[{"title": "a", "type": "rule"}])
|
|
|
|
|
|
async def test_one_duplicate_blocks_the_whole_batch():
|
|
from scribe.mcp.tools.tasks import create_records
|
|
|
|
dup = MagicMock(id=33, title="existing", similarity=1.0, reason="title")
|
|
gate = AsyncMock(side_effect=[None, dup])
|
|
with patch("scribe.mcp.tools.tasks.dedup_svc.find_duplicate_note", gate), \
|
|
patch("scribe.mcp.tools.tasks.batch_svc.create_batch", AsyncMock()) as batch:
|
|
out = await create_records(records=[{"title": "fresh"}, {"title": "existing"}], project_id=3)
|
|
assert out["duplicate"] is True and out["record"] == 2 and out["existing_id"] == 33
|
|
assert "Nothing in the batch was created" in out["message"]
|
|
batch.assert_not_awaited()
|
|
|
|
|
|
async def test_create_records_returns_ids_in_order():
|
|
from scribe.mcp.tools.tasks import create_records
|
|
|
|
notes = []
|
|
for nid in (51, 53):
|
|
n = MagicMock(id=nid)
|
|
n.to_dict.return_value = {"id": nid}
|
|
notes.append(n)
|
|
with patch("scribe.mcp.tools.tasks.dedup_svc.find_duplicate_note", AsyncMock(return_value=None)), \
|
|
patch("scribe.mcp.tools.tasks.batch_svc.create_batch", AsyncMock(return_value=(None, notes))) as batch:
|
|
out = await create_records(
|
|
records=[{"title": "a"}, {"title": "b", "type": "note", "body": "after {{ref:1}}"}],
|
|
milestone_id=8,
|
|
)
|
|
assert out["ids"] == [51, 53]
|
|
items = batch.call_args.args[1]
|
|
assert [i.is_task for i in items] == [True, False]
|
|
assert batch.call_args.kwargs["milestone_id"] == 8
|
|
|
|
|
|
async def test_start_planning_hands_its_steps_to_the_service():
|
|
from scribe.mcp.tools.tasks import start_planning
|
|
|
|
with patch("scribe.mcp.tools.tasks.dedup_svc.find_duplicate_note", AsyncMock(return_value=None)), \
|
|
patch("scribe.mcp.tools.tasks.planning_svc.start_planning",
|
|
AsyncMock(return_value={"milestone": {"id": 1}})) as svc:
|
|
await start_planning(project_id=3, title="Plan", body="see {{ref:1}}",
|
|
steps=[{"title": "Step 1", "kind": "spike"}])
|
|
kwargs = svc.call_args.kwargs
|
|
assert kwargs["body"] == "see {{ref:1}}"
|
|
assert [s.title for s in kwargs["steps"]] == ["Step 1"]
|
|
assert kwargs["steps"][0].task_kind == "spike"
|
|
|
|
|
|
# ── check_batch: refused whole, before any write ───────────────────────────
|
|
|
|
|
|
def test_a_placeholder_past_the_end_is_refused():
|
|
with pytest.raises(ValueError, match=r"\{\{ref:3\}\} names no record"):
|
|
check_batch([BatchItem(title="a"), BatchItem(title="b", body="{{ref:3}}")])
|
|
|
|
|
|
def test_the_milestone_placeholder_needs_a_milestone():
|
|
with pytest.raises(ValueError, match=r"\{\{ref:milestone\}\}"):
|
|
check_batch([BatchItem(title="a", body="in {{ref:milestone}}")])
|
|
|
|
|
|
def test_the_milestone_placeholder_is_valid_with_one():
|
|
check_batch([BatchItem(title="a", body="in {{ref:milestone}}")],
|
|
BatchMilestone(title="m", body="first {{ref:1}}"))
|
|
|
|
|
|
def test_every_record_needs_a_title():
|
|
with pytest.raises(ValueError, match="record 2 has no title"):
|
|
check_batch([BatchItem(title="a"), BatchItem(title=" ")])
|
|
|
|
|
|
def test_a_batch_has_a_ceiling():
|
|
from scribe.services.record_batch import MAX_BATCH
|
|
|
|
with pytest.raises(ValueError, match="at most"):
|
|
check_batch([BatchItem(title=f"t{i}") for i in range(MAX_BATCH + 1)])
|
|
|
|
|
|
async def test_start_planning_refuses_placeholders_with_no_steps():
|
|
from scribe.services.planning import start_planning
|
|
|
|
with patch("scribe.services.planning.projects_svc.get_project", AsyncMock(return_value=MagicMock())), \
|
|
patch("scribe.services.planning.milestones_svc.create_milestone", AsyncMock()) as create:
|
|
with pytest.raises(ValueError, match="no steps were given"):
|
|
await start_planning(user_id=7, project_id=3, title="p", body="see {{ref:1}}")
|
|
create.assert_not_awaited()
|