Gate D audit — durable, attributable report feedback¶
The runnable artifact for OGUR-60, the MVP-1 feedback gate. Re-run it with:
It lives at tests/unit/api/test_gate_d_audit.py, so make test and CI run it on
every push with no extra wiring. Every test name is a finding; a green run is the
verdict.
Why an audit rather than a build¶
Gate D's behaviour was built by three earlier issues and never verified in one place:
| Acceptance bullet | Built by | Discharged here by |
|---|---|---|
| One report-level box; per-section OUT | OGUR-71 | test_a_report_level_write_needs_only_text |
| Persist author, report, document identity, text, timestamp | OGUR-69 | test_every_persisted_field_comes_from_the_session_not_the_body, test_a_document_scoped_write_persists_the_stable_identity |
Anchor to the source-record layer, never Signal.id |
OGUR-69 | test_feedback_carries_no_signal_identity |
| Reject unauthenticated writes and another client's report | OGUR-64 / OGUR-65 | test_anonymous_writes_and_reads_are_refused, test_an_ungranted_served_report_is_indistinguishable_from_an_unserved_one |
| Survives process restart | OGUR-67 / OGUR-68 | test_feedback_survives_a_process_restart_through_the_http_boundary |
| seed → feedback → reseed still resolves | OGUR-69 | test_feedback_still_resolves_after_a_reseed_renumbers_signals |
| Writable volume backed up daily | OGUR-68 | test_the_snapshotted_volume_is_the_one_the_database_lives_on |
The evidence sat at three different altitudes — store unit tests, API unit tests, and TOML configuration nothing asserted — and two bullets were not actually proved. Both gaps are closed below.
The two bullets that were not proved¶
Restart durability had never crossed the API. The suite's default database is
in-memory SQLite on a StaticPool; it cannot demonstrate persistence, because it
dies with the process by construction.
tests/unit/store/test_feedback.py::test_feedback_survives_application_restart
works around that with a real file, but only at the store layer. The deployed
claim is about the API, so the audit's version drives the same ASGI app over a
file-backed SQLite, disposes the engine, rebuilds it against the same file, and
reads the row back through GET. It carries the session cookie across the
restart rather than logging in again: the claim a client cares about is that a
redeploy does not sign them out and does not lose what they wrote, and
re-authenticating would prove only the second half.
The cross-client refusal was proved to be a 404, not to be the same 404.
tests/unit/api/test_authorization.py:71 asserts the status. A 404 whose body,
shape, or wording differs between "this report is not yours" and "this report does
not exist" is a directory of the other client's landscapes.
ogur/api/deps.py:39-42 collapses the two on purpose; the audit compares the two
responses with the caller-supplied id masked, and confirms the refused write left
no row behind. This is only reachable because two packs are served — with one, the
ungranted case has nothing to compare against.
The anchor rule is asserted structurally, not behaviourally¶
test_feedback_carries_no_signal_identity reads Feedback.__table__ and pins the
exact column set. Every other feedback test asserts a behaviour that follows
from the no-Signal.id rule, and a re-added signal_id column would disturb
none of them: resolution by (document_source, document_source_id) keeps
working while a second, reseed-fragile anchor rides alongside it, until the first
MVP-2 rebuild orphans it. Adding that column back is one of the falsifications
below, and this is the only case that catches it.
The landscape_id that Feedback and Signal share is deliberately not treated
as a borrowed key — it is the report scope, it survives a reseed, and it is what
scopes resolution rather than what performs it. That scoping has its own case
(test_a_reseed_of_another_landscape_does_not_capture_the_feedback): the same NCT
is routinely projected into more than one landscape, and without the predicate
feedback would silently re-attach to whichever copy the query returned first.
Backups: what the green run does and does not claim¶
"Backed up daily" has no behaviour to exercise — configuration is the whole artifact — so it is asserted as configuration, across the two files that own the two halves:
Dockerfiledecides where SQLite writes (DATABASE_URL=sqlite:////data/ogur.db);fly.tomldecides what Fly snapshots ([mounts] destination = "/data",snapshot_retention = 14).
The audit parses both and asserts they name the same directory. The failure mode
worth catching is not an absent snapshot policy — someone would notice — but a
daily snapshot of a volume the database moved off, which passes every review
and restores nothing. A companion case pins that docker/entrypoint.sh still
refuses to overwrite an existing /data/ogur.db, since a redeploy that
re-materialized it from the baked content DB would discard every feedback row
while every backup check still passed.
Stated plainly: the daily layer is Fly's automatic volume snapshot. The offsite
copy is not daily. scripts/ops/backup_db.sh (make fly-backup) is manual, and
docs/deployment.md documents it as "at least weekly and
before deploys". The ticket's bullet is satisfied literally; a daily off-Fly
copy is not in place. It is named here rather than left to be inferred from a
passing test.
The falsifications¶
A gate nobody has watched fail is not known to work. Each asserted property was
removed from main's code and the audit re-run; each broke exactly the case that
claims it.
| Property removed | Failing case |
|---|---|
the grant check in require_report_access |
test_an_ungranted_served_report_is_indistinguishable_from_an_unserved_one |
snapshot_retention from fly.toml |
test_the_snapshotted_volume_is_the_one_the_database_lives_on |
DATABASE_URL repointed off /data |
test_the_snapshotted_volume_is_the_one_the_database_lives_on |
a signal_id column re-added to Feedback |
test_feedback_carries_no_signal_identity |
the existing-database guard in docker/entrypoint.sh |
test_the_writable_volume_is_never_overwritten_on_boot |
session.commit() in add_feedback |
5 cases, incl. the restart and reseed pair |
user_id no longer session-derived |
test_every_persisted_field_comes_from_the_session_not_the_body |
The pack roster is pinned, not inherited¶
The audit sets EXPLORE_REPORT_PACK_PATH itself, for the same reason the Gate C
audit does (gate-c-audit.md). Without it the verdict is a
property of the developer's .env: an in-progress pack from an unmerged worktree
does not merely add a report — a layer whose evidence_depth has no sentence
raises while the shared index is being built, so every pack 503s and this module
fails for reasons unrelated to feedback.
Grants are exercised, not waived. grant_all_landscapes() is the right tool for
suites whose subject is the pack layer; here the grant check is part of the
subject, so the audit writes real UserLandscapeAccess rows.
Deliberately not audited¶
- The feedback box's UI states. Pinned by 16 cases in
frontend/src/components/explore/__tests__/ReportFeedback.test.tsx, including the unresolved-write states (a 5xx can arrive after the row committed, so the client names the ambiguity instead of guessing). A pytest artifact cannot assert React state; re-deriving it here would duplicate rather than verify. - Per-section feedback. Not shipped in MVP-1 — whole-report granularity was
the accepted release floor (mvp-scope.md §2.2); per-section is roadmap input.
The API carries the
(document_source, document_source_id)anchor and the audit exercises it, because the anchor is what any future rebuild depends on — but the shipped surface writes report-level rows only. - Idempotency on
POST. There is none, which is why the client cannot settle "did my write commit?" on a dropped connection (frontend/src/components/explore/ReportFeedback.tsx). That was a product decision on PR #251 — author plus text is not an identity, and every heuristic built on it either duplicated the row or discarded the analyst's text. Not a gap to close here.