diff --git a/src/scribe/mcp/tools/snippets.py b/src/scribe/mcp/tools/snippets.py index 78425ce..f591eeb 100644 --- a/src/scribe/mcp/tools/snippets.py +++ b/src/scribe/mcp/tools/snippets.py @@ -207,6 +207,13 @@ async def get_snippet(snippet_id: int) -> dict: the source moved on — trust the location over the cached body and consider verify_snippet after you look. + A record kept VERBATIM is confirmed by containment. A deliberately + ANNOTATED one — commentary the source does not carry — cannot be, so it + reads "current" on the authority of a standing `ok` verdict stamped at + the very commit just fetched (#2782); `verification` in the same payload + shows that basis. Edit the record, or let the file move past that commit, + and it reads "diverged" again until someone re-runs verify_snippet. + When the shape ledger has judgments against this snippet, the response carries `instances` (shapes classified as conforming to it — the structured consumer map) and/or `variants` (named departures, each with diff --git a/src/scribe/services/snippets.py b/src/scribe/services/snippets.py index 5365b68..064b685 100644 --- a/src/scribe/services/snippets.py +++ b/src/scribe/services/snippets.py @@ -987,6 +987,46 @@ async def _refresh_provenance(note, commit_sha: str) -> None: await notes_svc.update_note(note.user_id, note.id, data=data) +def _verdict_still_vouches(note, fields: dict, fetched_commit_sha: str) -> bool: + """Does a standing `ok` verdict still speak for this body, at this commit? + + Containment (cached code ∈ fetched file) is the fast path, and it is right + for a record kept verbatim. It is WRONG for a deliberately annotated one + (#2782): a record whose job is to say why the shape is what it is carries + commentary the source does not, so containment fails forever and the record + reads `diverged` on every pull. That turns the one honest drift signal into + a permanent false positive — and annotation is a sanctioned record style, + so this is two deliberate designs colliding, not a malformed record. + + The escape hatch is the verdict itself. `verify_snippet` is precisely where + a human or agent already judged this body a faithful rendering of that + source, and `verification.commit_sha` records the repo commit they judged + it at — a field whose own docstring (#2688) anticipated this use: "makes + 'the REPO moved on since the check' computable, once the forge integration + can compare it against the current head." This is that comparison. + + All four conditions, and none is optional: + - the verdict says `ok`; + - it has not EXPIRED — `verification_view` recomputes `code_sha` against + the record's current body, so editing the record retires the verdict; + - it was not INVALIDATED by a push touching the location (#2691); + - the file we just fetched is at the very commit the verdict was stamped + at. Any later commit means nobody has judged what is there now. + + The last one is what keeps this honest: it vouches for a body against ONE + known commit, never against whatever the source has become since. The + moment the file moves, containment resumes as the authority and the record + reads `diverged` until someone re-verifies — which is the correct outcome, + because at that point nobody has looked. + """ + if not fetched_commit_sha: + return False + view = verification_view(note, fields) + if view.get("status") != VERIFY_OK or view.get("needs_attention"): + return False + return view.get("commit_sha") == fetched_commit_sha + + async def attach_live_body(note, data: dict) -> None: """Decorate a PULL response with forge-checked freshness (#2690). @@ -1115,6 +1155,14 @@ async def attach_live_body(note, data: dict) -> None: _refresh_provenance(note, fetched.commit_sha), site="pull provenance-refresh", ) + elif _verdict_still_vouches(note, fields, fetched.commit_sha or ""): + # Containment failed, but an unexpired `ok` verdict stamped at exactly + # this commit already judged this body a faithful rendering of it — + # the annotated-record case (#2782). Trust the judgment over the + # substring test; `data["verification"]` travels in the same payload, + # so a reader can see the basis rather than take "current" on faith. + data["body_source"] = "forge" + data["body_freshness"] = "current" else: data["body_source"] = "cache" data["body_freshness"] = "diverged" diff --git a/tests/test_snippet_live_body.py b/tests/test_snippet_live_body.py index 47eea42..004e95b 100644 --- a/tests/test_snippet_live_body.py +++ b/tests/test_snippet_live_body.py @@ -272,3 +272,89 @@ async def test_forge_failure_inside_lookup_never_breaks_the_pull(): data = _data() await svc.attach_live_body(_note(), data) assert "body_source" not in data + + +# --- #2782: an annotated record is not a diverged one ------------------------ +# Containment is right for a verbatim record and wrong for a deliberately +# annotated one: the commentary that makes the record worth reading is exactly +# what makes `cached in fetched` false, forever. These pin the escape hatch — +# a standing `ok` verdict stamped at the commit we just fetched — and, just as +# importantly, every condition that must switch it back off. + +ANNOTATED = "# Membership is the contract — this record says WHY, the source can't.\n" + CODE + + +def _ok_verdict(code=ANNOTATED, commit=SHA, **extra): + verdict = svc.compose_verification( + status=svc.VERIFY_OK, checked_code_sha=svc.code_sha(code), commit_sha=commit + ) + verdict.update(extra) + return verdict + + +async def _freshness(data, *, file_commit=SHA, content=CODE): + forge = _forge_with(lambda r: _file_response(content, commit_sha=file_commit)) + with _patched(forge), patch.object(svc.notes_svc, "update_note", AsyncMock()): + await svc.attach_live_body(_note(), data) + await background.drain() + return data["body_source"], data["body_freshness"] + + +async def test_annotated_record_with_a_standing_verdict_reads_current(): + """The bug: the record's commentary is absent from the source, so + containment fails and every pull said `diverged`. A verdict that already + judged this body faithful, at this very commit, outranks the substring.""" + data = _data(code=ANNOTATED, verification=_ok_verdict()) + assert await _freshness(data) == ("forge", "current") + assert data["snippet"]["code"] == ANNOTATED # still never rewritten + + +async def test_the_verdict_vouches_for_one_commit_only(): + """The guard that keeps the hatch honest. The file has moved past the + commit the verdict was stamped at, so nobody has judged what is there + now — containment resumes as the authority and the record reads diverged + until someone re-verifies.""" + data = _data(code=ANNOTATED, verification=_ok_verdict(commit="a" * 40)) + assert await _freshness(data) == ("cache", "diverged") + + +async def test_an_expired_verdict_does_not_vouch(): + """The record was edited after the check, so `code_sha` no longer matches + and the verdict describes a body that is not this one.""" + data = _data(code=ANNOTATED, verification=_ok_verdict(code="def other(): pass")) + assert await _freshness(data) == ("cache", "diverged") + + +async def test_a_push_invalidated_verdict_does_not_vouch(): + """A push touched the recorded location since the check (#2691) — the repo + moved under the verdict even though the record didn't.""" + data = _data(code=ANNOTATED, verification=_ok_verdict(invalidated_by="c" * 40)) + assert await _freshness(data) == ("cache", "diverged") + + +async def test_only_an_ok_verdict_vouches(): + """A drifted verdict is evidence AGAINST the body, not for it.""" + verdict = svc.compose_verification( + status=svc.VERIFY_CHANGED, checked_code_sha=svc.code_sha(ANNOTATED), commit_sha=SHA + ) + data = _data(code=ANNOTATED, verification=verdict) + assert await _freshness(data) == ("cache", "diverged") + + +async def test_a_verbatim_record_still_takes_the_containment_path(): + """No regression: the happy path does not route through the hatch, and an + unverified verbatim record is still confirmed by containment alone.""" + data = _data(code=CODE) + assert await _freshness(data) == ("forge", "current") + + +async def test_a_verdict_predating_commit_stamping_does_not_vouch(): + """Verdicts recorded before `commit_sha` existed (#2688) carry no commit to + compare, so they cannot tie the body to a known state of the source. They + fall through to containment rather than vouching on age alone.""" + verdict = svc.compose_verification( + status=svc.VERIFY_OK, checked_code_sha=svc.code_sha(ANNOTATED) + ) + assert "commit_sha" not in verdict + data = _data(code=ANNOTATED, verification=verdict) + assert await _freshness(data) == ("cache", "diverged")