peter
955c8c3445
Deep review of the debt burn: - `refusal_reason` told the user to reconnect on ANY token-endpoint failure — a 5xx, a 429, a misconfigured client — while the grant stayed and the account still read as connected, with no connect button. Only `invalid_grant` (the one that drops the grant) means reconnect now. - `pins.reorder` writes only the rows that change, so two overlapping reorders each skipped rows that matched in their own stale snapshot and interleaved into two pins on one slot. It locks the user's pins first; the second waits and deals from the first's result. - Adding a whitelist entry committed the invite and the activation it implies separately, and its audit row was written before a new invite had an id (target NULL). One transaction, flushed for the id. - The mutation convention test skipped async routes and dicts returned by name: request-access, purge-discovery and the subscription sync answered without `ok`. They do now, and the test covers both. The helper-commit check looked only at `_`-names; it reads every non-route function, which listed two deliberate committers (upsert_pending_invite, ensure_welcome) — now allowlisted with their reasons.
92 lines
4.1 KiB
Python
92 lines
4.1 KiB
Python
"""Two rows of CLAUDE.md's canonical-patterns table, held in code rather than prose.
|
|
|
|
A rule stated only in a comment drifts: this one had — 63 mutation responses without `ok`, and
|
|
twelve route helpers that committed on their own, among them an approval whose decision, activation
|
|
and token landed in three separate transactions. The debt burn of 2026-09 fixed the drift; these keep
|
|
it fixed.
|
|
"""
|
|
|
|
import ast
|
|
from pathlib import Path
|
|
|
|
ROUTES = Path(__file__).parent.parent / "app" / "routes"
|
|
MUTATING = {"post", "put", "patch", "delete"}
|
|
|
|
# A route helper that commits where it stands, and why the commit has to be there rather than in
|
|
# the handler. Anything else flushes and leaves the commit to its handler.
|
|
COMMITTING_HELPERS = {
|
|
"auth.py:_issue_token": "the token must be durable before the link that carries it is mailed",
|
|
"auth.py:_consume_token": "burning a single-use token is its own unit: a later failure must "
|
|
"not hand the token back",
|
|
"auth.py:_complete_link": "the tail of the Google callback route, which answers with its result",
|
|
"auth.py:_register_account": "a background task with its own session, i.e. a job wrapper",
|
|
"search.py:_ingest_candidates": "stages around YouTube calls: the stubs must survive an enrich "
|
|
"that fails",
|
|
"search.py:_classify_shorts": "the last stage of that ingest, after its probes",
|
|
"auth.py:upsert_pending_invite": "commits only when it writes — the contract its three "
|
|
"callers rely on (`_register_account` persists its own flushed user because a no-op here "
|
|
"commits nothing)",
|
|
"messages.py:ensure_welcome": "the websocket push after it must announce a row the client "
|
|
"can already fetch",
|
|
}
|
|
|
|
|
|
def _modules():
|
|
for path in sorted(ROUTES.rglob("*.py")):
|
|
yield path.relative_to(ROUTES).as_posix(), ast.parse(path.read_text(encoding="utf-8"))
|
|
|
|
|
|
FUNCTIONS = (ast.FunctionDef, ast.AsyncFunctionDef)
|
|
|
|
|
|
def _is_route(fn: ast.FunctionDef | ast.AsyncFunctionDef) -> bool:
|
|
return any(
|
|
isinstance(d, ast.Call) and getattr(d.func, "attr", "") in MUTATING | {"get", "websocket"}
|
|
for d in fn.decorator_list
|
|
)
|
|
|
|
|
|
def _is_mutation_route(fn: ast.FunctionDef | ast.AsyncFunctionDef) -> bool:
|
|
return any(
|
|
isinstance(d, ast.Call) and getattr(d.func, "attr", "") in MUTATING
|
|
for d in fn.decorator_list
|
|
)
|
|
|
|
|
|
def test_every_mutation_answers_ok():
|
|
"""`{"ok": true, …}` (or `{"deleted": id}`) for every mutation, sync or async — one field a
|
|
client can read the same way everywhere. A serializer's return (a call) is exempt: it describes
|
|
a resource, not an acknowledgement. A dict built elsewhere and returned by name is not: it hid
|
|
two acknowledgements without `ok` from the literal-only scan, so it is spread into a literal."""
|
|
missing = []
|
|
for name, tree in _modules():
|
|
for fn in ast.walk(tree):
|
|
if not (isinstance(fn, FUNCTIONS) and _is_mutation_route(fn)):
|
|
continue
|
|
for node in ast.walk(fn):
|
|
if not isinstance(node, ast.Return) or isinstance(node.value, ast.Call):
|
|
continue
|
|
keys = (
|
|
{k.value for k in node.value.keys if isinstance(k, ast.Constant)}
|
|
if isinstance(node.value, ast.Dict)
|
|
else set()
|
|
)
|
|
if not keys & {"ok", "deleted"}:
|
|
missing.append(f"{name}:{node.lineno} {fn.name}")
|
|
assert missing == []
|
|
|
|
|
|
def test_route_helpers_flush_and_leave_the_commit_to_the_handler():
|
|
"""Every function in a route module that is not itself a route is a helper — whatever its name
|
|
(two public ones committed unseen while this looked only at `_`-names) and sync or async."""
|
|
committing = set()
|
|
for name, tree in _modules():
|
|
for fn in ast.walk(tree):
|
|
if isinstance(fn, FUNCTIONS) and not _is_route(fn):
|
|
if any(
|
|
isinstance(n, ast.Call) and getattr(n.func, "attr", "") == "commit"
|
|
for n in ast.walk(fn)
|
|
):
|
|
committing.add(f"{name}:{fn.name}")
|
|
assert committing == set(COMMITTING_HELPERS)
|