[TheSentinel454]

What

This is phase 1 of 2 for retiring five single-table guard triggers (BUZZ-318). Their invariants now hold in the application writers. All five triggers and their functions stay for mixed-version pods. A later PR drops them.

Trigger / function	App-side enforcement
trg_channels_community_id_immutable	By construction. Inserts bind the tenant once. No production UPDATE channels assigns community_id, and ChannelUpdate has no community field. A source policy pins this.
communities_deletion_tombstone / enforce_community_tombstone()	One state machine. begin_quiescing, fence, purge_postgres and abort all go through transition_community, using a FOR UPDATE row snapshot in the caller's transaction (quiesce, fence and abort also hold the community deletion advisory lock; purge does not, and its tombstone transition fails if a concurrent abort won). Allowed transitions: active→quiescing (plus a quiescing retry); quiescing→fenced, which increments the generation; fenced→tombstone at the same generation, with a one-way deleted_at; active/quiescing/fenced→active on abort, which also increments the generation. Every transition rejects an already-deleted row. The UPDATE is conditioned on the snapshot's state and generation. set_community_icon updates only an active, undeleted row and returns AccessDenied otherwise. Archive and unarchive lock an active row, and their UPDATEs also carry deletion_state = 'active', so every non-lifecycle communities UPDATE or upsert carries the active predicate (policy-pinned: a non-active row is immutable outside the state machine). No production DELETE FROM communities exists, and the policy forbids adding one. Executor GUCs are unchanged.
community_deletion_request_retargeting_guard	Mostly by construction, plus predicates. No production UPDATE assigns target identity or provenance (policy-pinned). Both initial inventory freezes (freeze_inventory, complete_owner_preparation) require stage = 'submitted' AND blocked_at IS NULL AND inventory_frozen_at IS NULL in their UPDATE. Every request UPDATE that assigns an inventory column must carry inventory_frozen_at IS NULL, and one that assigns destructive_storage_* must carry the frozen-or-equal guard (policy-pinned). The destructive-manifest freeze now also refuses a row that is marked frozen but is missing its manifest JSON.
community_deletion_approval_removal_guard	By construction. Approvals are insert-only (approve, complete_owner_preparation). The policy forbids any production UPDATE, ON CONFLICT … DO UPDATE upsert or DELETE.
community_deletion_manifest_keys_guard	One shared check. Append, pre-freeze clear and verified cleanup each lock the request row and check its stage and freeze status through guard_manifest_chunk_write. Chunk content is never updated: the policy rejects every chunk UPDATE or conflict upsert except mark_manifest_chunk_deleted, the only writer that stamps deleted_at. That stamp must be the function's only chunk write, a plain UPDATE of exactly SET deleted_at = now(), with request_id = $1, chunk_no = $2 and deleted_at IS NULL as plain conjuncts (no OR, no subquery), so it stays one-way.

The app is stricter than the triggers, not equivalent to them. enforce_community_tombstone() admits any change when the executor GUC matches, before it looks at lifecycle columns, so the transition graph above lives only in transition_community. The chunk DELETE trigger also allows cleanup when the parent request is missing or unfrozen, while guard_manifest_chunk_write allows only fenced-and-unfrozen or verified cleanup. Every current caller already runs at those stages.

Writer inventory: I searched every production writer of these tables and columns across all crates, excluding #[cfg(test)]. That covers buzz-db, buzz-deletion, buzz-relay handlers, side effects and operator/admin APIs, and buzz-audit. Relay, operator and executor entry points all call the store methods above. No other crate writes these tables directly.

Tests

The six PG tests are in store::deletion::postgres_tests. Each disables only its own trigger, then checks that the app path rejects the forbidden mutation and permits the allowed one:

• single_table_guard_channels_remain_tenant_bound_without_trigger
• single_table_guard_community_lifecycle_rejects_deleted_row_without_trigger: icon refused while quiescing; fence refused on a deleted row; then the full fence→tombstone sequence; abort after tombstone fails.
• single_table_guard_abort_invalidates_generation_without_trigger: abort from quiescing and from fenced, for unarchived and pre-archived communities; asserts active state, community and request generation = previous + 1, restored archived_at, null deleted_at.
• single_table_guard_request_inventory_freeze_is_one_way_without_trigger: re-freeze is refused on both the operator and owner-preparation paths. The owner path leaves no approval evidence behind. Owner request-ID retargeting is refused, and a frozen manifest with missing JSON can't be filled in.
• single_table_guard_approval_evidence_is_append_only_without_trigger: asserts the exact error.
• single_table_guard_manifest_chunks_stop_at_freeze_without_trigger

The source policy lives in crates/buzz-db/tests/observability_source.rs (every_production_single_table_guard_writer_preserves_its_invariants). It scans every crate's src directory and reuses the file's existing cfg-test stripping, comment and literal masking, function slicing and SQL normalization. Each rule is checked per function. A minimum count of discovered writers per table makes the test fail if discovery stops matching, and exact sets of production functions hit by each rule (lifecycle exemption, stamp exemption, chunk-guard callers, active-community writers, request-freeze writers) make it fail if a parser stops matching. The parser handles ON CONFLICT … DO UPDATE SET (with or without a space before (), tuple SET (a, b) =, and rejects column-less community inserts. A guard counts only as a plain conjunct of the statement's own WHERE: it must follow the WHERE start or an AND (optionally table-qualified, never NOT), with no whole-word OR outside the frozen-or-equal groups and no subquery, and every destructive_storage_* assignment must preserve a frozen value with COALESCE. Negative and accept fixtures cover every rule. The shared source filter now also skips test_support.rs, which is compiled only under cfg(test).

Each check was confirmed to fail when its fix is removed. Seven mutants each failed on an assertion: the inventory_frozen_at predicate removed from each of the two freeze paths, and each of the five policy rules disabled. Review rework also falsified the injected SET icon = NULL writer, an unguarded inventory/destructive-storage writer, abort keeping the old generation, and the stamp without deleted_at IS NULL; every new fixture test fails against the previous policy. Round 1 also falsified the lifecycle deleted_at check, the manifest append guard, the icon predicate and the destructive-manifest predicate.

What got simpler

The four lifecycle UPDATE statements, each with its own generation arithmetic and its own GUC setup, are now one transition function with one SQL writer. Both manifest removal modes share one admission check. For inventory freeze, the check lives in the existing atomic UPDATE, with no new helper or transaction. Channel immutability and approval append-only behavior stay construction rules, not duplicated runtime checks. The source-policy scans now share one Rust crate walker, which this PR extracted into collect_production_rs_files.

Known policy limits: the policy is a textual check of Rust source, not a SQL parser or a control-flow analysis. None of these gaps hides an unsafe writer today, but phase-2 reviewers should not read the policy as more than that:

• Only INSERT, UPDATE and DELETE statements are recognized, including CTE and ON CONFLICT forms. MERGE and TRUNCATE are not. UPDATE ONLY <table> and DELETE FROM ONLY <table> are not recognized either, because the table name must follow the verb directly.
• A guard negated after itself (deletion_state = 'active' IS FALSE, … = false) is not caught.
• A SQL comment inside the literal (-- AND deletion_state = 'active') can supply the guard text.
• The frozen-or-equal OR text is removed before the OR check without checking how it is grouped.
• guard_manifest_chunk_write( is checked for presence in the writing function only. The policy does not show the result is awaited and propagated, or that the guard runs in the same transaction as the write. With the trigger disabled, single_table_guard_manifest_chunks_stop_at_freeze_without_trigger shows append and pre-freeze clear refusing after freeze; verified cleanup has no trigger-disabled test.
• Function discovery uses a fixed list of signature prefixes, and dynamic SQL is recognized only in known shapes such as set_parts.push.
• The shared source filter skips files named test_support.rs by name. Both such files are compiled only under cfg(test) today.

Phase 2 (not here): a forward migration plus a desired-state change will drop only these five triggers and functions and update validate_serving_catalog_on and the schema assertions. It ships after every serving and executor pod runs these writers.

Identified debt (pre-existing, outside this diff): purge can take the request-row FK lock (checkpoint insert) before the community lock, while abort locks community then request (deletion.rs purge vs abort). PostgreSQL detects that as a deadlock and cancels one side, which fails closed; the order is unchanged from the base. runtime::postgres_tests::writer_pool_rejects_non_read_committed_database_default hangs until its 120 s bound on clean 177551dda as well. Some test-only fallbacks still hardcode localhost:5432, and one relay test helper hardcodes Redis 6379.

Verification (Blox, byte-identical tree to this commit)

Gate	Result
cargo fmt --all -- --check	pass
cargo clippy --workspace --all-targets -- -D warnings	pass
cargo nextest run -p buzz-db --lib	166/166
cargo nextest run -p buzz-db --test observability_source	30/30
scripts/test-postgres-test-discovery.sh	pass
scripts/postgres-test-run.sh -p buzz-admin / audit / deletion / push-gateway / relay / search / workflow	all rc 0 (4, 7, 11, 11, 402, 20, 2)
scripts/postgres-test-run.sh -p buzz-db	515 pass, 1 fail: the pre-existing hang above

The new PG tests are selected by the postgres-ci filter (test(/postgres_tests::/) or binary(/^postgres_/)) and not test(/(^|::)external_infra[^:]*::/). The policy tests run in just test-unit (--test observability_source).

[github-actions]

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

• Exact PR diff: 60de1db0b8813766d8e004297798f2ff759d3089...11fb45ba1e171f3c215ac0ddf19b97d2914c0b29
• Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.

---

Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability findings identified in the authorized PR range.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

• Review used read-only inspection of the diff, surrounding source, callers, schema guards, and tests. Builds and tests were not run.

---

_{Generated by Codex Security Review |
Requested by: @TheSentinel454 |
Workflow run}

[wpfleger96]

🤖 Automated review of 1e2c3f8f6966bdfbf5ba1ec257b1cdf0acdf5f24 (base d25384a4f3c3a940b5b999f26105091c58ab76d6): blocking — three gaps in the source policy.

Thanks for this. The production writers hold up: two independent source reviews found no new state-transition or frozen-data violation, and all five triggers still protect the database in this phase. The three blockers are all in every_production_single_table_guard_writer_preserves_its_invariants. That test is what keeps these invariants once phase 2 drops the triggers, and in each case it accepts a writer the matching trigger refuses, contradicting what the description says the policy enforces. Each one is a small fix plus a negative fixture.

Blocking

1. Approval upserts get past the append-only rule (crates/buzz-db/tests/observability_source.rs:2203-2207). The approval branch only looks for UPDATE community_deletion_approvals and DELETE FROM community_deletion_approvals through sql_table_suffixes. It never calls sql_update_sets(), the helper that understands conflict updates. If someone added ON CONFLICT (request_id) DO UPDATE SET note = EXCLUDED.note to the approve insert, the policy would pass and the exact rule-hit sets wouldn't change, even though approval evidence would now be mutable. The description says the policy forbids "any production UPDATE" on approvals. Fix: also reject a conflict update on community_deletion_approvals, and add a negative upsert fixture.

2. Calling guard_manifest_chunk_write lets content UPDATEs through (:2209-2228). Any function in the deletion store that writes a manifest chunk passes as long as it contains guard_manifest_chunk_write(, whatever the write does. So UPDATE community_deletion_manifest_keys SET keys = $n added to append_manifest_key_chunk passes, even though the trigger never allows chunk content to change. The accept fixture at :2516-2548 actually treats a guarded SET keys = $1 UPDATE as acceptable. Fix: reject manifest UPDATEs except the deleted_at stamp, checked strictly as described in item 3. Keep insert, pre-freeze delete and verified delete as the only guarded operations, and turn the guarded content UPDATE into a negative fixture. The guard check also only shows the call is present. It doesn't show the result is awaited and propagated, or that the guard runs inside the writer's transaction. Add that to the "Known policy limit" paragraph rather than presenting presence as proof.

3. The stamp exemption skips the plain-conjunct rule (:2215-2219). It accepts the stamp when the WHERE contains("and deleted_at is null"), not when it passes plain_guard(). So WHERE request_id = $1 AND chunk_no = $2 AND deleted_at IS NULL OR true still gets the exemption with every exact set and count unchanged. After the triggers are dropped, that would restamp already-deleted chunks across requests. A subquery containing the same text also passes. An operator-precedence slip in this predicate is an easy real mistake to make, and the description says every guard is a plain conjunct with no OR and no subquery. Fix: run the exemption through plain_guard(), or the same statement-local conjunct, OR and subquery check, and add negative fixtures for both forms.

Optional

• The six new PG tests run ALTER TABLE … DISABLE TRIGGER and never turn the trigger back on. That's safe under the postgres-ci wrapper, which gives each test its own database. A plain cargo test -- --ignored against the default local buzz database leaves all five triggers off there. The roster tests use create_scratch_db instead.
• More textual limits that belong in the "Known policy limit" paragraph alongside trailing negation: a SQL comment inside the literal can hold the guard text (-- AND deletion_state = 'active'); the frozen-or-equal OR text is removed without checking its grouping (the accept fixture at :2387 has an unparenthesized OR); function discovery uses a fixed list of signature prefixes (:935-947); and dynamic SQL is only recognized in specific shapes such as set_parts.push (:2093-2096). None of these hides an unsafe writer today. The paragraph should just not read as general SQL or control-flow enforcement.
• The shared source filter skips any file named test_support.rs in every crate. That's right today, since both such files are compiled only under cfg(test), but the exemption is keyed on the filename alone.
• The table rows that say the app "matches the trigger" are narrower than the triggers. enforce_community_tombstone() accepts a matching executor GUC before it looks at lifecycle changes, so it doesn't encode the new transition graph itself. The chunk DELETE trigger allows missing-parent or any unfrozen-parent cleanup, while guard_manifest_chunk_write only allows fenced-and-unfrozen or verified cleanup. Current callers already require those stages, so nothing breaks, but the wording should say the app is stricter.
• The new chunk-freeze PG test stays green when AND deleted_at IS NULL is removed from the stamp. The existing manifest_key_chunks_bind_freeze_execution_and_cleanup test and the source policy catch it, so a one-way stamp assertion in the new test is optional.
• Already on main, not caused by this PR: purge can take the request-row foreign-key lock (checkpoint insert) before the community lock, while abort locks community then request (deletion.rs:2646-2648 vs 3887-3895). PostgreSQL resolves that as a deadlock and cancels one side, which fails closed. Purge used the same order at the base.
• The description doesn't carry buzz-review-completed yet (AGENTS.md step 4).

What was checked

• Each of the five trigger functions at the base against the app code. transition_community reproduces the four old lifecycle UPDATEs: quiesce keeps the generation, fence and abort add 1, tombstone keeps the fenced generation and now requires deleted_at IS NULL (stricter than the old COALESCE), and abort restores archived_at. Both inventory freezes use atomic unfrozen predicates. The destructive freeze preserves frozen values with COALESCE and refuses a frozen row with missing manifest JSON.
• Concurrency. Purge keeps its deletes, the tombstone transition and the request advance in one transaction. If an abort wins first, the transition refuses and the deletes roll back. If purge wins, abort can't restore the tombstone. Lock order for purge versus abort is unchanged from the base. Icon updates are atomic and limited to active, undeleted rows. Archive and unarchive lock an active row before updating.
• Writer inventory across every crate's src: outside cfg(test), every production writer of channels.community_id, communities, deletion requests, approvals and manifest chunks is one the PR lists. That includes the rolled-back bootstrap community inserts in store/channel_members.rs and runtime/replica_fence.rs.
• Behavior change: set_community_icon now returns AccessDenied for a missing or non-active community. Its only production caller, the kind 9033 handler in relay_admin.rs, propagates the error before returning success.
• Local run against a private PostgreSQL 16 cluster: all six single_table_guard_* tests, all 11 buzz-deletion PG tests and all 30 observability_source cases pass. Three mutations each turned a test red: removing inventory_frozen_at IS NULL from freeze_inventory, abort keeping the old generation, and removing AND deleted_at IS NULL from the stamp. Hosted CI at this head shows the six new tests as named PASS in the PostgreSQL job and the source-policy test passing in the unit job.

[wpfleger96]

🤖 Automated review of 88c6a3231022c1de19fd6cbcca8becbbc16a3341 (base d25384a4f3c3a940b5b999f26105091c58ab76d6): blocking, one gap left in the upsert detection that two of the earlier fixes depend on.

Thanks for the quick turnaround. The stamp fix is complete, and the first two fixes work for the spelling in the new fixtures. Both of those fixes rely on sql_update_sets() to find conflict updates, though, and it only recognizes one spelling.

Blocking

1. ON CONFLICT( with no space isn't detected as an upsert (crates/buzz-db/tests/observability_source.rs:1978-1984). sql_update_sets() looks for the substring " on conflict ", and normalize_sql_source() doesn't put a space before (. So ON CONFLICT(request_id) DO UPDATE SET …, which is valid PostgreSQL, returns no conflict update. Two consequences:

   • Adding ON CONFLICT(request_id) DO UPDATE SET note = EXCLUDED.note to the approve insert gets past the new append-only branch (:2203-2208).
   • Adding ON CONFLICT(request_id, chunk_no) DO UPDATE SET keys = EXCLUDED.keys to the append_manifest_key_chunk insert leaves sets empty with zero direct UPDATEs. The existing guard call then admits it at :2231-2234, so chunk content becomes mutable.

   Neither change moves the writer counts or the exact rule-hit sets. This isn't an unusual spelling: buzz-push-gateway/src/postgres.rs:347,485 already writes ON CONFLICT( at the base. The triggers still protect phase 1, but this test is what holds these rules once phase 2 drops them, and the description says it rejects these upserts. Fix: match on conflict at a token boundary (followed by a space or (), and add negative fixtures for the no-space spelling on both tables. Keep the plain INSERT and DO NOTHING accept fixtures.

Optional

• UPDATE ONLY <table> and DELETE FROM ONLY <table> aren't recognized either, since sql_table_suffixes() needs the table name right after the verb. That's worth a line in "Known policy limits" next to MERGE/TRUNCATE.
• Still open from the first review: the six PG tests disable their trigger and never turn it back on (fine under the per-test postgres-ci databases, but it leaves the default local buzz database without triggers), and the description doesn't carry buzz-review-completed yet (AGENTS.md step 4).

What was checked

• The delta 1e2c3f8f..88c6a323 touches only the source-policy test. Two independent source reviews found nothing loosened: every change tightens admission, and the production writers, discovery floors and expected rule-hit sets are unchanged.
• The spaced approval upsert and the guarded SET keys UPDATE are now rejected.
• The stamp exemption is closed. It requires the deletion-store path, one direct chunk UPDATE that is the function's only chunk write, exactly SET deleted_at = now(), and request_id = $1, chunk_no = $2 and deleted_at IS NULL through plain_guard(). The fixtures reject OR true, the subquery, NULL stamps, extra assignments, missing predicates, extra writes, renamed or out-of-file exemptions and conflict-upsert stamps. The production mark_manifest_chunk_deleted matches the exact form.
• The updated "Known policy limits" list now covers the SQL-comment, trailing-negation, OR-grouping, guard-presence/transaction, function-discovery, dynamic-SQL and filename-filter limits from the first review.
• cargo test -p buzz-db --test observability_source passes 32/32 at this head locally. Hosted Rust / Unit Tests shows the source-policy tests as named PASS, and CI overall is 56 pass, 0 fail.

[TheSentinel454]

Thanks @wpfleger96 — addressed at 592e97158855182be01ca77a30bc2452f29fd0e5 (test-only, crates/buzz-db/tests/observability_source.rs; production tree unchanged since 1e2c3f8f).

Latest review

• ON CONFLICT( without a space (blocking): fixed. sql_update_sets() now finds on conflict and do update set at identifier boundaries (new sql_token_suffixes(), which sql_table_suffixes() now reuses), so ON CONFLICT(…) DO UPDATE is detected like the spaced form. Added no-space negatives for both community_deletion_approvals (DO UPDATE SET note = EXCLUDED.note) and community_deletion_manifest_keys (guarded DO UPDATE SET keys = EXCLUDED.keys), plus a no-space DO NOTHING accept fixture next to the existing plain INSERT / spaced DO NOTHING ones. Reverting just the matcher turns the new negatives red.
• UPDATE ONLY / DELETE FROM ONLY: documented in "Known policy limits" next to MERGE/TRUNCATE.
• Trigger re-enable in the six PG tests: still declined. The supported runner gives each test its own database, and phase 2 deletes these DISABLE TRIGGER lines together with the triggers. Moving them to scratch databases would add a fourth copy of the create_scratch_db helper for code that is about to go away.
• Status: these fixes are pushed; the independent re-review and the review gate at this head are still pending, and buzz-review-completed is not set.

First review (fixed at 88c6a323): approval ON CONFLICT DO UPDATE is rejected; every manifest-chunk UPDATE/upsert other than the stamp is rejected even when guarded; the stamp exemption requires a single plain UPDATE, SET deleted_at = now() only, and plain_guard on request_id = $1, chunk_no = $2 and deleted_at is null. The Known policy limits and the "stricter than the triggers" wording are in the description; the pre-existing purge/abort lock order is recorded as identified debt.

[wpfleger96]

🤖 Automated review of 11fb45ba1e171f3c215ac0ddf19b97d2914c0b29 (base d25384a4f3c3a940b5b999f26105091c58ab76d6): clear, nothing blocking.

Thanks for the fix. sql_token_suffixes() now matches on conflict, do update set and set at identifier boundaries. Since the source is already lowercased and whitespace-collapsed, ON CONFLICT(…) DO UPDATE reaches the approval append-only rejection and the manifest content-UPDATE rejection, the same as the spaced form. ON CONFLICT ON CONSTRAINT … DO UPDATE, INSERT … AS alias, CTE-wrapped and public.-prefixed upserts also reach the check. SET(a, b) = … is caught in plain UPDATEs and in DO UPDATE. Ordinary aliases still work (UPDATE t SET, UPDATE t alias SET, UPDATE t AS alias SET), and names like asset or offset don't take the set match.

Optional

• Worth one more line in "Known policy limits": sql_update_sets() cuts each statement at the first " or ; (this predates the PR), so quoted identifiers and some literals hide what comes after. ON CONFLICT ("request_id") DO UPDATE …, INSERT INTO community_deletion_approvals AS "a" … ON CONFLICT …, or a string value with a ; before ON CONFLICT would leave no detected conflict UPDATE. For a guarded manifest INSERT, that means the upsert is admitted. A quoted table name (UPDATE "channels") also escapes table discovery. No current approval or chunk INSERT uses any of these forms.
• The DISABLE TRIGGER lines in the six PG tests (deletion.rs:4580,4632,4691,4780,4917,4978) still never re-enable. Your reasoning holds for the supported per-test-database runner, and phase 2 removes them. Running one of those ignored tests directly against a shared local database leaves that guard off, though.
• buzz-review-completed isn't in the description yet (AGENTS.md step 4).

Verified

• cargo test -p buzz-db --test observability_source passes 32/32 at this head. Swapping in the old sql_update_sets() body fails exactly the three new no-space fixtures (single_table_policy_rejects_approval_mutations, single_table_policy_rejects_manifest_content_updates, single_table_policy_checks_tuple_assignments). Restoring it passes 32/32 again. cargo test -p buzz-db passes 203 with 0 failures (PG-dependent ignored tests not run).
• Hosted Rust / Unit Tests names all three as PASS. CI is 56 pass, 0 fail.
• The UPDATE ONLY / DELETE FROM ONLY limit from the last review is now in the description.

[wesbillman]

🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security/concurrency), and Groot (product/operator/retry) reviews.

No actionable findings in this phase-1 change. Reviewed head 11fb45ba1e171f3c215ac0ddf19b97d2914c0b29 against base/merge-base d25384a4f3c3a940b5b999f26105091c58ab76d6.

The shared lifecycle transition preserves generation increments, abort archive restoration, transactional tombstoning and rejection of deleted rows. Manifest writes serialize on the request row with freeze; verified cleanup follows the stage update in the same transaction. The added freeze predicates reject repeated inventory mutation and malformed frozen manifests without JSON. Icon/archive predicates preserve the supported callers’ lifecycle boundaries. I also reviewed the source-policy rules and fixtures, retained trigger definitions, and public description/discussion; no attached images or actionable disclosure found.

Limitations: source analysis only. No tests, builds, SQL, live deletion, concurrency or performance experiments were run. A read-only check-run snapshot for this head reports 56 successful and 28 skipped checks; that is not independent runtime validation. The textual policy does not prove arbitrary SQL or control flow safe. Its documented limits, the pre-existing purge/abort lock-order issue, and the disposable-database requirement for trigger-disabling tests remain relevant. This review covers retaining all five triggers, not the later trigger-removal phase.

No changes requested; submitted as a non-blocking COMMENT, not an approval.