bvandeusen
700ef20eb0
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 14s
CI & Build / TypeScript typecheck (push) Successful in 38s
CI & Build / integration (push) Successful in 32s
CI & Build / Python tests (push) Failing after 49s
CI & Build / Build & push image (push) Skipped
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.
96 lines
4.3 KiB
Python
96 lines
4.3 KiB
Python
"""A snippet's `data` mirror survives the GENERIC note door.
|
|
|
|
`notes.data` is derived from the body. The snippet service always composed it
|
|
from the field set it had just merged, so `update_snippet` was never the
|
|
problem — the problem was every other way a snippet's body could be written.
|
|
`update_note` is a `hasattr` loop with no snippet awareness, and both doors
|
|
reach it: PATCH /api/notes/<id> and the MCP update_note tool. The Knowledge
|
|
feed handed you that path, because a snippet card there routed to /notes/:id.
|
|
|
|
The failure was silent and the wrong way round: `snippet_fields` PREFERS the
|
|
mirror, so the row went on reporting its old repo/path/symbol to the location
|
|
reverse lookup and to prior-art recall while displaying its new body — a record
|
|
surfaced with full authority and wrong, which the drift-check docstring calls
|
|
worse than having no record at all (#3128).
|
|
"""
|
|
import pytest
|
|
from tests.helpers import drive_update_note as _update
|
|
from tests.helpers import fake_note, fake_snippet
|
|
|
|
OLD_MIRROR = {
|
|
"name": "debounce",
|
|
"language": "javascript",
|
|
"locations": [{"repo": "Scribe", "path": "old/place.js", "symbol": "debounce"}],
|
|
"verification": {"status": "ok", "code_sha": "abc", "checked_at": "2026-01-01"},
|
|
"provenance": {"commit_sha": "deadbeef"},
|
|
}
|
|
|
|
MOVED_BODY = (
|
|
"**Locations:**\n"
|
|
"- `Scribe` · `new/place.ts` · `debounce`\n\n"
|
|
"```typescript\nexport const debounce = 1;\n```\n"
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_body_write_moves_the_mirror_with_it():
|
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
await _update(note, body=MOVED_BODY)
|
|
assert note.data["locations"] == [
|
|
{"repo": "Scribe", "path": "new/place.ts", "symbol": "debounce"}
|
|
], "the mirror still describes where the snippet used to live"
|
|
assert note.data["language"] == "typescript"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_verdict_and_provenance_are_carried_not_dropped():
|
|
"""Neither is in the body to parse, so recomposing must carry them. An
|
|
ordinary edit must not erase the last drift check — and it needs no
|
|
invalidation branch either: `code_sha` is recomputed from the new code, so
|
|
a verdict stamped against the old code expires itself on read."""
|
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
await _update(note, body=MOVED_BODY)
|
|
assert note.data["verification"] == OLD_MIRROR["verification"]
|
|
assert note.data["provenance"] == OLD_MIRROR["provenance"]
|
|
assert note.data["code_sha"] != OLD_MIRROR["verification"]["code_sha"]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_an_explicit_data_wins_over_recomposition():
|
|
"""`update_snippet` composes the mirror from the merged field set it holds
|
|
and passes it here. That caller knows things the body cannot be re-read for
|
|
— which locations were replaced, whether provenance survives the edit — so
|
|
an explicit mirror must not be recomputed out from under it."""
|
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
authoritative = {"name": "from the service", "locations": []}
|
|
await _update(note, body=MOVED_BODY, data=authoritative)
|
|
assert note.data == authoritative
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_plain_note_is_left_alone():
|
|
"""Only snippets carry a mirror; a note's `data` must not be invented."""
|
|
note = fake_note(note_type="note", data=None, project_id=None)
|
|
await _update(note, body="just some prose")
|
|
assert note.data is None
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_write_that_cannot_change_the_parse_does_not_touch_the_mirror():
|
|
"""Status, priority, project — none of them is an input to the body parser,
|
|
so recomposing on them would be work for nothing and would rebuild a mirror
|
|
from a body nobody claimed to have changed."""
|
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
await _update(note, project_id=4)
|
|
assert note.data == OLD_MIRROR
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_title_change_reaches_the_mirror_too():
|
|
"""A snippet's NAME lives in its title, not its body — `parse_snippet_fields`
|
|
reads both, so both are triggers."""
|
|
note = fake_snippet(data=dict(OLD_MIRROR), project_id=None)
|
|
await _update(note, title="throttle — cap a callback's rate")
|
|
assert note.data["name"] == "throttle"
|
|
assert note.data["when_to_use"] == "cap a callback's rate"
|