From 700ef20eb02c96ff7b4d291633c907fdce4b7677 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 28 Aug 2026 15:42:12 -0400 Subject: [PATCH] feat(notes): a note's check is writable through both doors, and empty means empty (#3164, milestone 317 step 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rules path's three lessons (#3096), inherited: EMPTY MEANS NULL. The sweep's whole signal is `verify_with IS NULL` = "this is a decision, there is nothing to go and check". A "" that is not NULL makes a norm look like a constraint nobody has verified — and never-checked sorts FIRST, so it would sit at the top of the sweep forever. CLEARING IS EXPLICIT. At the MCP door "" means "leave this alone", so an agent updating a body does not wipe a check it was never asked about — which leaves no value meaning "remove it". `clear` names the field, and naming it cannot happen by accident. The REST door, where a cleared form input arrives as "", reaches the same place through normalisation: two idioms, one outcome. THE STAMP CERTIFIES A CHECK, NOT A RECORD. Rewrite or clear `verify_with` and `verified_at` is dropped, so the note re-enters the sweep. A note wrongly listed as due costs one look; a note wrongly vouched for costs exactly what the sweep exists to catch. `verified_at` is also no longer settable through an ordinary edit — a stamp says somebody performed THIS check, and minting one from a write that ran no check is the one thing that would make the sweep lie. And one this path adds: not every record may carry a check. A task's decay is its status — a done issue records what happened rather than asserting something that can go false — and a snippet already has verify_snippet, which compares its recorded location and code against the repo and expires its own verdict. Both are refused with a message naming the alternative, never dropped silently (minted_kind's reasoning, #3129), and the gate lives at the service so the two doors cannot come to disagree. Written as an INVARIANT over the resulting record, not a filter on which fields were passed. That is what catches the sideways route — a checked note being turned into a task, a write that names no check at all and would sail past any per-field gate. The MCP docstrings carry the norm-vs-constraint test, because at that door the docstring IS the contract and a field described only as "how to verify this" gets filled in on every note. Step 5 does this properly across the instruction surfaces; this is the minimum that stops the field being misused on arrival. tests/helpers gains `drive_update_note` — the patch stack for driving update_note, written twice before this and now once. The note-shaped fakes gain the trio explicitly, for fake_note's own stated reason: an unset attribute is a truthy MagicMock, and a truthy verify_with reads as a check that is there. --- src/scribe/mcp/tools/notes.py | 52 ++++++- src/scribe/routes/notes.py | 11 +- src/scribe/services/notes.py | 90 ++++++++++- tests/helpers.py | 34 ++++- tests/test_mcp_tool_notes.py | 44 +++++- tests/test_services_notes_verification.py | 172 ++++++++++++++++++++++ tests/test_snippet_mirror_generic_door.py | 20 +-- 7 files changed, 398 insertions(+), 25 deletions(-) create mode 100644 tests/test_services_notes_verification.py diff --git a/src/scribe/mcp/tools/notes.py b/src/scribe/mcp/tools/notes.py index d8b3931..c50c2eb 100644 --- a/src/scribe/mcp/tools/notes.py +++ b/src/scribe/mcp/tools/notes.py @@ -103,6 +103,8 @@ async def create_note( project_id: int = 0, system_ids: list[int] | None = None, supersedes: list[int] | None = None, + verify_with: str = "", + expires_when: str = "", force: bool = False, ) -> dict: """Create a new note in Scribe. @@ -134,6 +136,24 @@ async def create_note( arrives labelled when it does surface. This records a CLAIM, not a verdict: it never says the older note was wrong, only that it is no longer the current answer. + verify_with: HOW TO CHECK this note is still true. Leave empty for + almost every note — that is the normal case, not an unfinished + one. + A note is a NORM or a CONSTRAINT. A norm is a decision ("we derive + versions from commit time"): it has no truth value and changes only + when its author changes it, which they know they did. A CONSTRAINT + asserts a fact about someone else's software ("AMO refuses to + re-sign a version", "this forge numbers CI runs per repository"), + and it goes false with nobody watching. Only constraints get a + check. + The test, in one question: COULD THIS NOTE BECOME FALSE WITHOUT + ANYONE EDITING IT? If no, leave this empty. + A command, a path, a URL, a query. Prose is allowed; something + runnable is better. + expires_when: The STATE that ends it — deliberately not a date. + "When Forgejo issues run numbers per workflow rather than per + repository", not "in six months". Constraints expire when the + ground moves, not on a schedule. force: Bypass the near-duplicate gate. By default, if a title- or meaning-similar note already exists in the same project, creation is BLOCKED and the existing note's id is returned so you update it @@ -161,6 +181,8 @@ async def create_note( body=body, tags=tags, project_id=project_id or None, + verify_with=verify_with, + expires_when=expires_when, ) if system_ids: await systems_svc.set_record_systems(uid, note.id, system_ids) @@ -185,6 +207,9 @@ async def update_note( project_id: int = 0, system_ids: list[int] | None = None, supersedes: list[int] | None = None, + verify_with: str = "", + expires_when: str = "", + clear: list[str] | None = None, ) -> dict: """Update an existing Scribe note. Only explicitly provided fields are changed. @@ -199,6 +224,25 @@ async def update_note( supersedes: Replace the ids of earlier notes this one replaces (set-semantics). None = leave unchanged; [] = clear all. See create_note for when to reach for it. + verify_with: How to check this note is still true. See create_note for + the norm-vs-constraint test that decides whether it should carry + one at all; the short form is "could this become false without + anyone editing it?". + expires_when: The STATE that ends it, not a date. + clear: Names of fields to UNSET — "verify_with", "expires_when". + Needed because "" means "leave this alone" here, so there is no + value that removes a field: an agent updating a body must not + silently wipe a check it was not asked about. A note that stops + being a constraint is cleared by naming the field, which cannot + happen by accident. + + Rewriting `verify_with` drops the note's verification stamp: a stamp + certifies a particular check, and carrying it across a rewrite would vouch + for something nobody has looked at. + + A task and a snippet are both REFUSED a check, with a message saying where + to go instead — a task's decay is its status, and a snippet has + verify_snippet. """ uid = current_user_id() fields: dict = {} @@ -210,7 +254,13 @@ async def update_note( fields["tags"] = tags if project_id: fields["project_id"] = project_id - note = await notes_svc.update_note(uid, note_id, **fields) + if verify_with: + fields["verify_with"] = verify_with + if expires_when: + fields["expires_when"] = expires_when + note = await notes_svc.update_note( + uid, note_id, clear=clear or (), **fields + ) if note is None: raise ValueError(f"note {note_id} not found") if system_ids is not None: diff --git a/src/scribe/routes/notes.py b/src/scribe/routes/notes.py index 17a13ad..bb28a80 100644 --- a/src/scribe/routes/notes.py +++ b/src/scribe/routes/notes.py @@ -112,6 +112,8 @@ async def create_note_route(): priority=priority, due_date=due_date, note_type=note_type, + verify_with=data.get("verify_with"), + expires_when=data.get("expires_when"), ) except ValueError as e: return jsonify({"error": str(e)}), 400 @@ -248,7 +250,14 @@ async def update_note_route(note_id: int): owner_uid = note_obj.user_id data = await request.get_json() fields = {} - for key in ("title", "body", "description", "parent_id", "project_id", "milestone_id", "status", "priority", "note_type"): + for key in ( + "title", "body", "description", "parent_id", "project_id", + "milestone_id", "status", "priority", "note_type", + # A cleared form input arrives as "" and the service reads that as + # NULL (NULLABLE_NOTE_TEXT), so this door expresses "remove the check" + # with its own idiom and needs no `clear` list (milestone 317). + "verify_with", "expires_when", + ): if key in data: fields[key] = data[key] if "due_date" in data: diff --git a/src/scribe/services/notes.py b/src/scribe/services/notes.py index 69ccc20..c2a3720 100644 --- a/src/scribe/services/notes.py +++ b/src/scribe/services/notes.py @@ -1,5 +1,6 @@ import logging import re +from collections.abc import Iterable from datetime import date, datetime, timezone from sqlalchemy import func, or_, select, text @@ -17,6 +18,45 @@ logger = logging.getLogger(__name__) _PARSED_FROM_BODY = frozenset({"title", "body", "tags"}) +# Text fields where EMPTY MEANS NULL (milestone 317). The sweep's whole signal +# is `verify_with IS NULL` = "this is a decision, there is nothing to go and +# check". An empty string that is not NULL makes a norm look like a constraint +# nobody has verified, forever — and it would sit at the top of the sweep, +# since never-checked sorts first. Sibling of rulebooks.NULLABLE_RULE_TEXT. +NULLABLE_NOTE_TEXT = ("verify_with", "expires_when") + + +def guard_check_fields(status: str | None, note_type: str | None) -> None: + """Raise unless a record in this shape may carry verify_with/expires_when. + + Stated as an INVARIANT over the resulting record rather than a filter on + which fields a caller passed, so it also catches the sideways route: a + checked note being turned into a task, which no per-field gate would see. + + Raises rather than dropping silently, for minted_kind's reason (#3129) — a + silently-corrected write is the defect that reasoning exists to end, and a + caller putting a check on the wrong record has an idea an error corrects + and a default hides. Lives at the service, not either door, so REST and MCP + cannot come to disagree about it. + """ + if status is not None: + raise ValueError( + "a task cannot carry verify_with/expires_when: a task's decay is " + "its status, and a done issue records what happened rather than " + "asserting something that can later go false. Put the check on the " + "note the fact lives in, or clear the check before making this a " + "task." + ) + if note_type == "snippet": + raise ValueError( + "a snippet already has a check: verify_snippet(), which compares " + "the recorded location and code against the repo and expires its " + "own verdict when the code moves. verify_with/expires_when are the " + "free-text form, for prose notes that assert a fact about " + "something outside the repo." + ) + + def embed_note(note) -> None: """Refresh a note's embedding, fire-and-forget. @@ -108,7 +148,16 @@ async def create_note( task_kind: str = "work", arose_from_id: int | None = None, data: dict | None = None, + verify_with: str | None = None, + expires_when: str | None = None, ) -> Note: + # Empty means empty (NULLABLE_NOTE_TEXT), then the invariant. Both run + # before anything is written, so an illegal shape never reaches the table. + verify_with = verify_with or None + expires_when = expires_when or None + if verify_with or expires_when: + guard_check_fields(status, note_type) + # Validate status/priority here so the MCP create_task path (which passes # them straight through) can't persist an out-of-enum value that the REST # route would have rejected — there's no DB CHECK on notes.status. @@ -152,6 +201,8 @@ async def create_note( task_kind=task_kind, arose_from_id=arose_from_id, data=data, + verify_with=verify_with, + expires_when=expires_when, ) session.add(note) await session.commit() @@ -404,7 +455,20 @@ def minted_kind(kind: str) -> str: raise ValueError(f"kind must be one of {MINTABLE_KINDS}, got {kind!r}") -async def update_note(user_id: int, note_id: int, **fields: object) -> Note | None: +async def update_note( + user_id: int, note_id: int, clear: Iterable[str] = (), **fields: object, +) -> Note | None: + """Partial update. `clear` names fields to UNSET; **fields carries values. + + Clearing is explicit and separate because a nullable field cannot be + emptied by passing it: the MCP door reads "" as "leave this alone", so an + agent filling two fields does not wipe the others, and a note that stops + being a constraint genuinely needs its check removed. Naming the field is + the one form that cannot happen by accident. The REST door, where a + cleared form input arrives as "", reaches the same place through the + NULLABLE_NOTE_TEXT normalisation below — two idioms, one outcome. + (Same shape as rulebooks.update_rule, milestone 312 step 2.) + """ async with async_session() as session: result = await session.execute( select(Note).where(Note.id == note_id, Note.user_id == user_id) @@ -416,6 +480,10 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No old_body = note.body old_title = note.title old_tags = list(note.tags or []) + check_before = note.verify_with + for key in clear: + if key in NULLABLE_NOTE_TEXT: + setattr(note, key, None) for key, value in fields.items(): if not hasattr(note, key): continue @@ -445,7 +513,27 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No ) elif key == "tags" and isinstance(value, list): value = _normalize_tags(value) + elif key in NULLABLE_NOTE_TEXT: + value = value or None + elif key == "verified_at": + # Not settable here. A stamp says somebody performed THIS + # check, so it is written by the verification path and by a + # restore, never by an ordinary edit that could mint one for a + # check nobody ran. + continue setattr(note, key, value) + # The invariant, over the RESULTING record rather than over what was + # passed — which is what catches a checked note being turned into a + # task. Raised before commit, so nothing is persisted. + if note.verify_with or note.expires_when: + guard_check_fields(note.status, note.note_type) + # A stamp certifies A CHECK, not a record. Rewrite or remove the check + # and the old stamp certifies something that no longer exists, so it is + # dropped and the note re-enters the sweep. The safe direction: a note + # wrongly listed as due costs one look; a note wrongly vouched for + # costs exactly what the sweep exists to catch. + if note.verify_with != check_before: + note.verified_at = None # A snippet's `data` is DERIVED from its body — so a write that moves # the body through this generic door must move the mirror with it # (#3128). Without this, PATCH /api/notes/ {body} left the diff --git a/tests/helpers.py b/tests/helpers.py index 5358312..0b32065 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -9,7 +9,33 @@ from __future__ import annotations from contextlib import contextmanager from datetime import datetime, timezone from types import SimpleNamespace -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock, MagicMock, patch + + +async def drive_update_note(note, **kwargs): + """Run `services/notes.update_note` against a stand-in row. + + The patch stack is the point: update_note reaches for a version snapshot, + an embedding refresh and a project reactivation on its way out, none of + which a unit test has. Written twice — once for the snippet mirror + (#3128) and once for the verification fields (#3182/317) — before being + consolidated here. + + Returns whatever update_note returned; assert on the `note` you passed in. + """ + from unittest.mock import AsyncMock as _AsyncMock + + session = make_mock_session() + result = MagicMock() + result.scalars.return_value.first.return_value = note + session.execute = _AsyncMock(return_value=result) + with patch("scribe.services.notes.async_session") as cls, \ + patch("scribe.services.notes.embed_note", MagicMock()), \ + patch("scribe.services.notes._maybe_reactivate_project", _AsyncMock()), \ + patch("scribe.services.note_versions.create_version", _AsyncMock()): + cls.return_value = session + from scribe.services.notes import update_note + return await update_note(user_id=7, note_id=note.id, **kwargs) def compiled_sql(element) -> str: @@ -102,6 +128,9 @@ def fake_note(**attrs) -> MagicMock: "id": 1, "title": "t", "body": "", "tags": [], "user_id": 7, "note_type": "note", "is_task": False, "task_kind": "work", "data": None, "deleted_at": None, + # Milestone 317: a truthy mock here reads as "this note carries a + # check", which trips the guard on records that may not have one. + "verify_with": None, "expires_when": None, "verified_at": None, }, attrs) @@ -111,6 +140,7 @@ def fake_task(**attrs) -> MagicMock: "id": 1, "title": "t", "body": "", "status": "todo", "priority": "none", "tags": [], "parent_id": None, "project_id": None, "is_task": True, "task_kind": "work", "user_id": 7, "deleted_at": None, + "verify_with": None, "expires_when": None, "verified_at": None, }, attrs) @@ -122,6 +152,8 @@ def fake_snippet(**attrs) -> MagicMock: "body": "```js\nreturn 1\n```\n", "tags": ["js", "snippet"], "note_type": "snippet", "is_task": False, "task_kind": "work", "user_id": 7, "data": None, "deleted_at": None, + "status": None, + "verify_with": None, "expires_when": None, "verified_at": None, }, attrs) diff --git a/tests/test_mcp_tool_notes.py b/tests/test_mcp_tool_notes.py index 7a6d2f3..1ea1b32 100644 --- a/tests/test_mcp_tool_notes.py +++ b/tests/test_mcp_tool_notes.py @@ -214,17 +214,53 @@ async def test_create_note_project_zero_becomes_none(): @pytest.mark.asyncio async def test_update_note_only_sends_non_default_fields(): """Omitted (default) fields must NOT reach the service — otherwise they'd - overwrite real data with empty strings.""" + overwrite real data with empty strings. + + `clear` is always forwarded and is not a field: it is how this door says + "unset these", and an empty one says "unset nothing". Same shape the rules + door took in #3096, and the same test that had to learn about it there.""" fake = fake_note() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock): await update_note(note_id=1, title="new title") - # Service got user_id, note_id positional + only the title kwarg args, kwargs = mock.call_args assert args == (7, 1) + assert kwargs.pop("clear") == (), "nothing was asked to be cleared" assert kwargs == {"title": "new title"} +@pytest.mark.asyncio +async def test_update_note_forwards_a_check_but_not_an_empty_one(): + """"" means "leave this alone" at this door — an agent updating a body must + not wipe a check it was never asked about (milestone 317).""" + mock = AsyncMock(return_value=fake_note()) + with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock): + await update_note(note_id=1, verify_with="curl the docs") + assert mock.call_args.kwargs["verify_with"] == "curl the docs" + + with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock): + await update_note(note_id=1, title="t") + assert "verify_with" not in mock.call_args.kwargs + + +@pytest.mark.asyncio +async def test_update_note_forwards_clear_so_a_check_can_be_removed(): + """The only way to unset a field at a door where "" means "leave alone".""" + mock = AsyncMock(return_value=fake_note()) + with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock): + await update_note(note_id=1, clear=["verify_with"]) + assert mock.call_args.kwargs["clear"] == ["verify_with"] + + +@pytest.mark.asyncio +async def test_create_note_forwards_the_check_fields(): + mock = AsyncMock(return_value=fake_note()) + with patch("scribe.mcp.tools.notes.notes_svc.create_note", mock): + await create_note(title="t", verify_with="curl", expires_when="AMO changes") + assert mock.call_args.kwargs["verify_with"] == "curl" + assert mock.call_args.kwargs["expires_when"] == "AMO changes" + + @pytest.mark.asyncio async def test_update_note_empty_tags_clears_explicitly(): """tags=[] is an explicit clear, distinct from tags=None (omit).""" @@ -232,7 +268,9 @@ async def test_update_note_empty_tags_clears_explicitly(): mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.notes.notes_svc.update_note", mock): await update_note(note_id=1, tags=[]) - assert mock.call_args.kwargs == {"tags": []} + kwargs = dict(mock.call_args.kwargs) + kwargs.pop("clear") + assert kwargs == {"tags": []} @pytest.mark.asyncio diff --git a/tests/test_services_notes_verification.py b/tests/test_services_notes_verification.py new file mode 100644 index 0000000..8b7debd --- /dev/null +++ b/tests/test_services_notes_verification.py @@ -0,0 +1,172 @@ +"""A note's own check — verify_with / expires_when (milestone 317 step 2). + +Three behaviours the rules path (#3096) had to get right and this one inherits: +empty means NULL, clearing is explicit, and rewriting the check drops the +stamp. Plus one this path adds: not every record may carry a check, and the +rule is an INVARIANT over the record rather than a filter on the write. +""" +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from tests.helpers import drive_update_note as _update +from tests.helpers import fake_note, make_mock_session + + +def _checkable(**over): + """A plain note — no status, no snippet type, no check. `fake_note` is a + MagicMock, so every one of these must be set explicitly: an unset + attribute is a truthy mock, which would look like a check that is there.""" + base = dict( + status=None, note_type="note", + verify_with=None, expires_when=None, verified_at=None, + project_id=None, + ) + base.update(over) + return fake_note(**base) + + +async def _create(**kwargs): + session = make_mock_session() + captured = {} + session.add = MagicMock(side_effect=lambda obj: captured.update( + verify_with=getattr(obj, "verify_with", "MISSING"), + expires_when=getattr(obj, "expires_when", "MISSING"), + )) + with patch("scribe.services.notes.async_session") as cls, \ + patch("scribe.services.notes.embed_note", MagicMock()), \ + patch("scribe.services.notes._maybe_reactivate_project", AsyncMock()): + cls.return_value = session + from scribe.services.notes import create_note + await create_note(user_id=1, title="t", **kwargs) + return captured + + +# ── empty means empty ──────────────────────────────────────────────────────── + +@pytest.mark.asyncio +async def test_an_empty_check_is_stored_as_null_not_as_a_blank(): + """The sweep's whole signal is `verify_with IS NULL` = "this is a decision, + there is nothing to check". A "" that is not NULL makes a norm look like a + constraint nobody has verified — and never-checked sorts FIRST, so it would + sit at the top of the sweep forever.""" + assert (await _create(verify_with="", expires_when=""))["verify_with"] is None + + note = _checkable(verify_with="curl the docs") + await _update(note, verify_with="") + assert note.verify_with is None + + +@pytest.mark.asyncio +async def test_a_check_is_stored_when_given(): + captured = await _create(verify_with="curl the AMO docs", expires_when="AMO allows re-signing") + assert captured["verify_with"] == "curl the AMO docs" + assert captured["expires_when"] == "AMO allows re-signing" + + +# ── clearing is explicit ───────────────────────────────────────────────────── + +@pytest.mark.asyncio +async def test_clear_unsets_a_field_the_mcp_door_cannot_empty(): + """At the MCP door "" means "leave this alone", so an agent updating a body + does not wipe a check it was never asked about. That leaves no value + meaning "remove it" — hence naming the field, which cannot happen by + accident.""" + note = _checkable(verify_with="a check", expires_when="a state") + await _update(note, clear=["verify_with"]) + assert note.verify_with is None + assert note.expires_when == "a state", "clearing one must not clear the other" + + +@pytest.mark.asyncio +async def test_clear_ignores_a_field_that_is_not_clearable(): + note = _checkable(title="keep me", verify_with="a check") + await _update(note, clear=["title"]) + assert note.title == "keep me" + + +# ── the stamp certifies a check, not a record ──────────────────────────────── + +@pytest.mark.asyncio +async def test_rewriting_the_check_drops_the_stamp(): + note = _checkable(verify_with="the old check", verified_at="2026-01-01") + await _update(note, verify_with="a different check") + assert note.verified_at is None + + +@pytest.mark.asyncio +async def test_clearing_the_check_drops_the_stamp(): + note = _checkable(verify_with="the old check", verified_at="2026-01-01") + await _update(note, clear=["verify_with"]) + assert note.verified_at is None + + +@pytest.mark.asyncio +async def test_an_unchanged_check_keeps_its_stamp(): + """Only a CHANGE invalidates it — otherwise every unrelated edit would + re-enter the note into the sweep and the signal would mean nothing.""" + note = _checkable(verify_with="the same check", verified_at="2026-01-01") + await _update(note, title="a new title", verify_with="the same check") + assert note.verified_at == "2026-01-01" + + +@pytest.mark.asyncio +async def test_a_stamp_cannot_be_set_through_an_ordinary_edit(): + """A stamp says somebody performed THIS check. Minting one from a write + that ran no check is the one thing that would make the sweep lie.""" + note = _checkable(verify_with="a check", verified_at=None) + await _update(note, verified_at="2026-08-28") + assert note.verified_at is None + + +# ── the invariant: which records may carry a check ─────────────────────────── + +@pytest.mark.asyncio +async def test_a_task_is_refused_a_check(): + with pytest.raises(ValueError, match="task"): + await _create(status="todo", verify_with="a check") + + +@pytest.mark.asyncio +async def test_a_snippet_is_refused_and_told_where_to_go(): + """The message has to name the alternative, or the caller is left with a + refusal and no route.""" + with pytest.raises(ValueError, match="verify_snippet"): + await _create(note_type="snippet", verify_with="a check") + + +@pytest.mark.asyncio +async def test_turning_a_checked_note_into_a_task_is_refused(): + """THE reason this is an invariant over the resulting record and not a + filter on which fields were passed. This write names no check at all, and + would sail past any per-field gate.""" + note = _checkable(verify_with="a check") + with pytest.raises(ValueError, match="task"): + await _update(note, status="todo") + + +@pytest.mark.asyncio +async def test_an_unchecked_note_can_still_become_a_task(): + """The invariant must not make ordinary promotion impossible.""" + note = _checkable() + await _update(note, status="todo") + assert note.status == "todo" + + +@pytest.mark.asyncio +async def test_clearing_the_check_in_the_same_write_lets_it_become_a_task(): + """The error tells the caller to clear the check first; doing both at once + has to actually work, or the advice is wrong.""" + note = _checkable(verify_with="a check") + await _update(note, status="todo", clear=["verify_with"]) + assert note.status == "todo" + assert note.verify_with is None + + +@pytest.mark.asyncio +async def test_an_ordinary_note_write_is_untouched_by_any_of_this(): + """The common case: no check, nothing to guard, nothing to reset.""" + note = _checkable(title="before") + await _update(note, title="after") + assert note.title == "after" + assert note.verify_with is None + assert note.verified_at is None diff --git a/tests/test_snippet_mirror_generic_door.py b/tests/test_snippet_mirror_generic_door.py index 96339b0..30af5e2 100644 --- a/tests/test_snippet_mirror_generic_door.py +++ b/tests/test_snippet_mirror_generic_door.py @@ -13,10 +13,9 @@ reverse lookup and to prior-art recall while displaying its new body — a recor surfaced with full authority and wrong, which the drift-check docstring calls worse than having no record at all (#3128). """ -from unittest.mock import AsyncMock, MagicMock, patch - import pytest -from tests.helpers import fake_note, fake_snippet, make_mock_session +from tests.helpers import drive_update_note as _update +from tests.helpers import fake_note, fake_snippet OLD_MIRROR = { "name": "debounce", @@ -33,21 +32,6 @@ MOVED_BODY = ( ) -async def _update(note, **fields): - session = make_mock_session() - result = MagicMock() - result.scalars.return_value.first.return_value = note - session.execute = AsyncMock(return_value=result) - with patch("scribe.services.notes.async_session") as cls, \ - patch("scribe.services.notes.embed_note", MagicMock()), \ - patch("scribe.services.notes._maybe_reactivate_project", AsyncMock()), \ - patch("scribe.services.note_versions.create_version", AsyncMock()): - cls.return_value = session - from scribe.services.notes import update_note - await update_note(user_id=7, note_id=note.id, **fields) - return note - - @pytest.mark.asyncio async def test_a_body_write_moves_the_mirror_with_it(): note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)