feat(forge): push webhook flags drift at the moment the repo moves (#2691)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / TypeScript typecheck (push) Successful in 22s
CI & Build / integration (push) Successful in 22s
CI & Build / Python tests (push) Successful in 53s
CI & Build / Build & push image (push) Successful in 39s

Second adapter consumer. POST /api/webhooks/forge validates Gitea's
X-Gitea-Signature (HMAC-SHA256, constant-time; no secret configured =
the endpoint 404s out of existence), extracts changed/removed paths,
and flags matched snippets by writing verification.invalidated_by
{commit_sha, at, path, removed} — the existing attention vocabulary
extended, not a new flag: needs_attention includes it, both filter
dialects (Python + jsonpath SQL) include it in 'attention' and exclude
it from 'ok', and recording ANY fresh verdict clears it by construction
because compose_verification builds a new dict. Unverified snippets are
skipped (already in their own bucket); replayed deliveries at the same
head commit are no-ops; processing failures return 200 with a WARNING +
AppLog canary so the forge never marks deliveries failed and operators
never disable the hook over a transient (#2663's lesson).

Matching goes through repo BINDINGS: recorded location repos are
free-form names ('Scribe') that cannot address a forge, so a snippet
reaches its forge repo through its project's binding — which also fixes
step 5's pull-time resolution for every real record via the same
fallback. O(bindings + snippets-in-project + changed files).

Settings: webhook secret beside the forge config (masked, sentinel-
skipped, Docker-secret env channel, endpoint documented in the UI).
Tests: signature gate, payload parsing, path semantics, both filter
dialects extended in the drift-check guard file, and real-Postgres
end-to-end (flag lands, attention lists it, replay quiet, re-verify
clears, unbound repo untouched).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-16 13:05:00 -04:00
co-authored by Claude Fable 5
parent eb760eb440
commit 89b07f7857
11 changed files with 559 additions and 12 deletions
+17 -4
View File
@@ -142,20 +142,25 @@ def verification_matches(data: dict | None, value: str) -> bool:
if not status:
return want == "unverified"
expired = verdict.get("code_sha") != (data or {}).get("code_sha")
# A push touched the recorded location since the verdict (#2691): the repo
# moved under it, so it needs a look even though it hasn't failed.
invalidated = bool(verdict.get("invalidated_by"))
if want == "unverified":
return False
if want == "drifted":
return status != "ok"
if want == "attention":
return status != "ok" or expired
return status != "ok" or expired or invalidated
if want == "ok":
return status == "ok" and not expired
return status == "ok" and not expired and not invalidated
return status == want
_VERIFY_DRIFTED_JSONPATH = '$.verification ? (@.status != "ok")'
_VERIFY_EXPIRED_JSONPATH = "$ ? (@.verification.code_sha != @.code_sha)"
_VERIFY_ANY_JSONPATH = "$.verification"
# A push touched the recorded location since the verdict (#2691).
_VERIFY_INVALIDATED_JSONPATH = "$.verification.invalidated_by"
def _verification_clause(value: str):
@@ -171,19 +176,23 @@ def _verification_clause(value: str):
if want == "drifted":
return Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH)
if want == "attention":
# Everything worth looking at: a failing verdict, OR an expired one.
# Everything worth looking at: a failing verdict, an expired one, OR
# one whose recorded location a push has since touched (#2691).
return or_(
Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH),
and_(has_verdict, Note.data.path_exists(_VERIFY_EXPIRED_JSONPATH)),
Note.data.path_exists(_VERIFY_INVALIDATED_JSONPATH),
)
if want == "ok":
# A clean bill of health that still describes the current code. The
# `~expired` half matters: without it this would quietly include records
# whose blessing has lapsed, which is the exact failure the feature is
# meant to catch.
# meant to catch. Same for push-invalidation — "ok" must mean the repo
# hasn't moved under the verdict either.
return and_(
Note.data.path_exists('$.verification ? (@.status == "ok")'),
~Note.data.path_exists(_VERIFY_EXPIRED_JSONPATH),
~Note.data.path_exists(_VERIFY_INVALIDATED_JSONPATH),
)
# A specific status: 'missing' | 'moved' | 'changed'.
return Note.data.path_exists(
@@ -229,6 +238,10 @@ def _note_to_item(note: Note) -> dict:
"detail": verdict.get("detail"),
"path": verdict.get("path"),
}
# Present only when a push has touched the recorded location since the
# verdict (#2691) — the "recheck me" marker, cleared by re-verifying.
if verdict.get("invalidated_by"):
item["verification"]["invalidated_by"] = verdict["invalidated_by"]
# Task fields — override note_type and add status/priority/due_date
if note.is_task:
+35
View File
@@ -100,6 +100,41 @@ async def list_bindings(user_id: int) -> list[RepoBinding]:
return list(rows.scalars().all())
async def keys_for_project(user_id: int, project_id: int) -> list[str]:
"""Every repo key bound to a project — the snippet→forge join (#2691).
Recorded snippet locations carry free-form repo names ("Scribe"), which
can't address a forge API. The project's binding is the identity that can:
a snippet reaches its forge repo through the project it belongs to.
"""
async with async_session() as session:
rows = await session.execute(
select(RepoBinding.repo_key).where(
RepoBinding.user_id == user_id,
RepoBinding.project_id == project_id,
)
)
return [k for (k,) in rows.all()]
async def bindings_for_key(raw_repo: str) -> list[RepoBinding]:
"""All bindings (ANY user) for a repo key — the webhook's entry point.
A push webhook carries no Scribe caller, only the repository it happened
to; the flag it writes is about each record's truth, so every user who
bound the repo gets their project's snippets considered — each write still
lands as that record's owner.
"""
key = normalize_repo_key(raw_repo)
if not key:
return []
async with async_session() as session:
rows = await session.execute(
select(RepoBinding).where(RepoBinding.repo_key == key)
)
return list(rows.scalars().all())
async def delete_binding(user_id: int, raw_repo: str) -> bool:
"""Remove a repo's binding. Returns True if a row was deleted."""
key = normalize_repo_key(raw_repo)
+122 -2
View File
@@ -468,10 +468,19 @@ def verification_view(note, fields: dict) -> dict:
"detail": stored.get("detail"),
"path": stored.get("path"),
"commit_sha": stored.get("commit_sha"),
# A push touched the recorded location since this verdict (#2691) —
# the repo moved under it. Cleared by the next verdict, which builds
# a fresh dict.
"invalidated_by": stored.get("invalidated_by"),
# What the operator actually wants to know: is there something to fix?
# An expired verdict counts as "needs looking at" even if it said ok,
# since the code it blessed is not the code that's there now.
"needs_attention": (not current) or stored["status"] in VERIFY_DRIFTED,
# since the code it blessed is not the code that's there now — and so
# does a push-invalidated one, for the same reason from the repo side.
"needs_attention": (
(not current)
or stored["status"] in VERIFY_DRIFTED
or bool(stored.get("invalidated_by"))
),
}
@@ -1020,7 +1029,18 @@ async def attach_live_body(note, data: dict) -> None:
data["body_source"] = "cache"
data["body_freshness"] = "no-recorded-location"
return
# Recorded location repos are free-form names ("Scribe"), which can't
# address a forge API — the project's repo BINDING is the identity that
# can (#2691). Try the location string first (it may be a real remote),
# then fall back to the bindings of the snippet's project.
repo = forge.resolve_repo(loc["repo"])
if repo is None and getattr(note, "project_id", None):
from scribe.services.repo_bindings import keys_for_project
for key in await keys_for_project(note.user_id, note.project_id):
repo = forge.resolve_repo(key)
if repo is not None:
break
if repo is None:
data["body_source"] = "cache"
data["body_freshness"] = "repo-not-on-this-forge"
@@ -1070,6 +1090,106 @@ async def attach_live_body(note, data: dict) -> None:
data["body_freshness"] = "diverged"
# --- push-time drift flagging (#2691) ----------------------------------------
def _path_touches(recorded: str, changed: str) -> bool:
"""The location-path semantics, applied to a pushed file: the recorded path
is the changed file itself, or a directory above it."""
recorded = (recorded or "").strip("/")
changed = (changed or "").strip("/")
if not recorded or not changed:
return False
return changed == recorded or changed.startswith(recorded + "/")
async def invalidate_for_push(
repo_key: str,
changed: list[str],
removed: list[str],
commit_sha: str,
) -> int:
"""Flag snippets whose recorded location a push just touched (#2691).
Writes ``verification.invalidated_by = {commit_sha, at, path, removed}``
onto matched snippets that CARRY a verdict — the flag means "the repo
moved under this verdict, recheck it", and it clears itself the moment a
fresh verdict is recorded because compose_verification builds a new dict.
Unverified snippets are skipped: they are already in the unverified
bucket, and stacking a second unchecked-flavored flag on them adds noise,
not information.
Matching goes through repo BINDINGS (any user's — a webhook has no
caller): each binding names a project, and that project's snippets are
path-matched against the pushed files. O(bindings + snippets-in-project +
changed files); nothing else is scanned. Returns how many records were
newly flagged (an already-flagged record at the same commit is skipped,
so replayed deliveries don't churn).
"""
from scribe.services.repo_bindings import bindings_for_key
bindings = await bindings_for_key(repo_key)
if not bindings or not (changed or removed):
return 0
touched = [(p, False) for p in changed] + [(p, True) for p in removed]
flagged = 0
for binding in bindings:
async with async_session() as session:
rows = await session.execute(
select(Note).where(
Note.user_id == binding.user_id,
Note.project_id == binding.project_id,
Note.note_type == SNIPPET_NOTE_TYPE,
Note.deleted_at.is_(None),
Note.data.path_exists("$.verification"),
)
)
notes = list(rows.scalars().all())
for note in notes:
fields = snippet_fields(note)
verdict = fields.get("verification") or {}
if not verdict.get("status"):
continue
hit = next(
(
(path, was_removed)
for loc in (fields.get("locations") or [])
for path, was_removed in touched
if _path_touches(loc.get("path") or "", path)
),
None,
)
if hit is None:
continue
existing = verdict.get("invalidated_by") or {}
if existing.get("commit_sha") == commit_sha:
continue # replayed delivery — already says exactly this
verdict = dict(verdict)
verdict["invalidated_by"] = {
"commit_sha": commit_sha,
"at": datetime.now(timezone.utc).isoformat(),
"path": hit[0],
# A removed file is the strongest signal — the recorded
# location may simply be gone. Surfaced so the attention row
# says which kind of look it needs.
"removed": hit[1],
}
data = compose_data(
name=fields.get("name", ""),
when_to_use=fields.get("when_to_use", ""),
signature=fields.get("signature", ""),
language=fields.get("language", ""),
code=fields.get("code", ""),
locations=fields.get("locations") or [],
merged_from=fields.get("merged_from") or [],
verification=verdict,
provenance=fields.get("provenance"),
)
await notes_svc.update_note(note.user_id, note.id, data=data)
flagged += 1
return flagged
async def delete_snippet(user_id: int, snippet_id: int) -> bool:
"""Retire a snippet to the trash (recoverable). Returns False if the id isn't
a snippet this user may WRITE.