CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / TypeScript typecheck (push) Successful in 56s
CI & Build / integration (push) Successful in 1m4s
CI & Build / Python tests (push) Successful in 1m55s
CI & Build / Build & push image (push) Successful in 44s
Step 2 of the Scribe -> Grimoire rename. - src/scribe -> src/grimoire, and every import, patch target, module path, hypercorn entry point and `python -m grimoire.restore` with it; pyproject, uv.lock and the frontend package are named grimoire. - Image git.fabledsword.com/bvandeusen/fabledgrimoire; CI's Postgres service runs as grimoire / grimoire_test. - Compose files default the DB user, database and (prod) volume to grimoire, each overridable from .env, so an install created as Scribe keeps its data by setting POSTGRES_USER, POSTGRES_DB and PGDATA_VOLUME. - Backups are written as grimoire-<ts>.tar with format grimoire-backup-tar. Archives named scribe-* with the scribe-backup-tar manifest still restore and are still counted at the destination (folder and S3), with tests. - Exports, the MCP auth realm, the ASGI scope key and the persistent logger carry the new name. Kept on purpose: the secret-sealing key label stays "scribe.secrets.v1", because every stored secret was derived under it. Plugin identity (plugin name, hooks, env vars, .scribe marker, skills) is step 3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
243 lines
10 KiB
Python
243 lines
10 KiB
Python
"""The note staleness sweep.
|
|
|
|
The query is a sibling of the rules sweep, not a shared implementation — a
|
|
rule scopes by rulebook ownership, a note by the note ACL, so only the
|
|
BEHAVIOUR is common (services/verification). These pin the parts that would
|
|
fail silently rather than loudly: the ordering, which rows are eligible, and
|
|
the asymmetry of a failed check.
|
|
"""
|
|
from datetime import datetime, timedelta, timezone
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
from tests.helpers import compiled_sql, fake_note, make_mock_session
|
|
|
|
|
|
async def _sweep_sql(**kwargs):
|
|
"""The statement the sweep builds, as SQL text.
|
|
|
|
No database: the SHAPE of the query is what is under test, and the
|
|
ordering clause in particular cannot be checked any other way without one.
|
|
The await happens INSIDE the patch — a coroutine created in the block and
|
|
awaited outside it would run against the real session.
|
|
"""
|
|
captured = {}
|
|
session = make_mock_session()
|
|
|
|
async def _execute(stmt):
|
|
try:
|
|
captured["sql"] = compiled_sql(stmt)
|
|
except Exception:
|
|
# Literal-rendering a datetime bind is dialect-dependent and can
|
|
# raise. Only the tests asserting on a literal value (user_id,
|
|
# project_id) need that form, and none of those build a cutoff.
|
|
captured["sql"] = str(stmt)
|
|
result = MagicMock()
|
|
result.scalars.return_value.all.return_value = []
|
|
return result
|
|
|
|
session.execute = _execute
|
|
with patch("grimoire.services.notes.async_session") as cls:
|
|
cls.return_value = session
|
|
from grimoire.services.notes import notes_due_for_verification
|
|
await notes_due_for_verification(user_id=7, **kwargs)
|
|
return captured["sql"]
|
|
|
|
|
|
def _where(sql: str) -> str:
|
|
"""Just the WHERE clause. `select(Note)` names every column, so searching
|
|
the whole statement for a column name always finds the SELECT list — which
|
|
is how one of these tests first failed for the wrong reason.
|
|
|
|
Matched by regex rather than split on a literal, because the exact
|
|
whitespace SQLAlchemy emits around WHERE is not something a test should
|
|
depend on.
|
|
"""
|
|
import re
|
|
|
|
m = re.search(r"\bWHERE\b(.*?)(?:\bORDER BY\b|$)", sql, re.S)
|
|
assert m, "no WHERE clause — the sweep must never select the whole table"
|
|
return m.group(1)
|
|
|
|
|
|
# ── the ordering, which is the whole signal ──────────────────────────────────
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_never_checked_sorts_first_not_last():
|
|
"""THE thing most likely to be got wrong, and it would not error.
|
|
|
|
Postgres sorts NULLs LAST on ASC by default, so the obvious `ORDER BY
|
|
verified_at ASC` sinks every never-checked note below every checked one —
|
|
exactly inverting the signal the sweep exists to carry. A note nobody has
|
|
ever confirmed is a claim with no evidence behind it at all.
|
|
"""
|
|
sql = await _sweep_sql()
|
|
assert "ORDER BY notes.verified_at ASC NULLS FIRST" in sql
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_order_is_total():
|
|
"""A tiebreak, so two notes verified in the same transaction do not swap
|
|
places between calls and make a page boundary lie.
|
|
|
|
Asserted on the END of the statement, not by searching it: `select(Note)`
|
|
names every column, so the first `notes.id` in the text is the SELECT
|
|
list, not the ORDER BY."""
|
|
sql = (await _sweep_sql()).rstrip()
|
|
assert sql.endswith("ORDER BY notes.verified_at ASC NULLS FIRST, notes.id")
|
|
|
|
|
|
# ── which rows are eligible ──────────────────────────────────────────────────
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_only_notes_that_carry_a_check_appear():
|
|
"""A note with no `verify_with` is not overdue — it is a decision, and
|
|
listing it would dilute the result until nobody reads it."""
|
|
assert "notes.verify_with IS NOT NULL" in await _sweep_sql()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_trashed_notes_are_excluded():
|
|
assert "notes.deleted_at IS NULL" in await _sweep_sql()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_the_scope_is_browse_not_read():
|
|
"""A sweep is a PASSIVE surface, so a record shared
|
|
one-to-one must not arrive in one unasked. `note_shares` is the tell —
|
|
its presence would mean the read scope leaked in."""
|
|
sql = await _sweep_sql()
|
|
assert "notes.user_id = 7" in sql
|
|
assert "project_shares" in sql
|
|
assert "note_shares" not in sql
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_task_carrying_a_check_is_NOT_filtered_out():
|
|
"""Deliberate. The write path permits a check on nothing but a plain note,
|
|
so such a row would be in an ILLEGAL state — and this is the one surface
|
|
that could tell somebody. Hiding it to match the invariant would make the
|
|
sweep agree with a database it had stopped describing."""
|
|
where = _where(await _sweep_sql())
|
|
assert "notes.status" not in where
|
|
assert "notes.note_type" not in where
|
|
|
|
|
|
# ── the filters ──────────────────────────────────────────────────────────────
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_never_only_narrows_to_the_unexamined():
|
|
assert "notes.verified_at IS NULL" in await _sweep_sql(never_only=True)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_an_age_window_still_includes_the_never_checked():
|
|
"""They are the most overdue thing there is; a window that excluded them
|
|
would answer the opposite of the question."""
|
|
sql = await _sweep_sql(older_than_days=30)
|
|
assert "notes.verified_at IS NULL" in sql
|
|
assert "notes.verified_at <" in sql
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_never_only_wins_over_an_age_window():
|
|
"""Both together is a caller contradicting themselves; the narrower one is
|
|
the safe reading, and it must not emit a cutoff as well."""
|
|
sql = await _sweep_sql(never_only=True, older_than_days=30)
|
|
assert "notes.verified_at <" not in sql
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_project_filter_narrows():
|
|
assert "notes.project_id = 4" in await _sweep_sql(project_id=4)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_negative_window_raises_rather_than_meaning_everything():
|
|
"""Silently answering a different question is the failure this guards."""
|
|
with pytest.raises(ValueError, match="older_than_days"):
|
|
await _sweep_sql(older_than_days=-1)
|
|
|
|
|
|
# ── the stamp ────────────────────────────────────────────────────────────────
|
|
|
|
async def _mark(note, still_true=True, writable=True):
|
|
session = make_mock_session()
|
|
result = MagicMock()
|
|
result.scalars.return_value.first.return_value = note
|
|
session.execute = AsyncMock(return_value=result)
|
|
with patch("grimoire.services.notes.async_session") as cls, \
|
|
patch("grimoire.services.access.can_write_note",
|
|
AsyncMock(return_value=writable)):
|
|
cls.return_value = session
|
|
from grimoire.services.notes import mark_note_verified
|
|
return await mark_note_verified(note_id=1, user_id=7,
|
|
still_true=still_true), session
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_passing_check_stamps_the_note():
|
|
note = fake_note(verify_with="curl the docs", verified_at=None)
|
|
out, session = await _mark(note)
|
|
assert out is note
|
|
assert note.verified_at is not None
|
|
session.commit.assert_awaited()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_failing_check_writes_nothing():
|
|
"""The asymmetry IS the design. There is no "verified false" state,
|
|
because a note whose check failed is not in a special condition — it is
|
|
WRONG. Recording the failure as a flag would let it sit there being false
|
|
with the sweep quietly satisfied that somebody had looked."""
|
|
note = fake_note(verify_with="curl the docs", verified_at=None)
|
|
out, session = await _mark(note, still_true=False)
|
|
assert out is note
|
|
assert note.verified_at is None, "a failed check must leave the stamp alone"
|
|
session.commit.assert_not_awaited()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_note_with_no_check_cannot_be_verified():
|
|
"""Nothing to verify is a different answer from verified."""
|
|
note = fake_note(verify_with=None)
|
|
out, _ = await _mark(note)
|
|
assert out is None
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_a_reader_cannot_stamp():
|
|
"""Stamping is a mutation — an editor-share holder may make
|
|
it, a viewer may not."""
|
|
note = fake_note(verify_with="curl the docs", verified_at=None)
|
|
out, _ = await _mark(note, writable=False)
|
|
assert out is None
|
|
assert note.verified_at is None
|
|
|
|
|
|
# ── the row ──────────────────────────────────────────────────────────────────
|
|
|
|
def test_the_row_carries_the_check_in_full():
|
|
"""The opposite call from a listing: the caller is about to go and run it,
|
|
so the text IS the payload rather than the bloat."""
|
|
note = fake_note(
|
|
id=9, title="Versioning", project_id=2,
|
|
verify_with="curl the AMO docs", expires_when="AMO allows re-signing",
|
|
verified_at=datetime.now(timezone.utc) - timedelta(days=74),
|
|
)
|
|
from grimoire.services.notes import verification_row
|
|
row = verification_row(note)
|
|
assert row["verify_with"] == "curl the AMO docs"
|
|
assert row["expires_when"] == "AMO allows re-signing"
|
|
assert row["days_since_verified"] == 74
|
|
|
|
|
|
def test_a_never_checked_row_says_never_not_none():
|
|
"""None means "this is a decision, the question does not apply"; "never"
|
|
means "it asserts a fact and nobody has confirmed it". The second is the
|
|
one worth acting on, and collapsing them loses the sweep's point."""
|
|
from grimoire.services.notes import verification_row
|
|
row = verification_row(fake_note(verify_with="a check", verified_at=None))
|
|
assert row["last_verified"] == "never"
|
|
assert row["days_since_verified"] is None
|