mirror of
https://github.com/block/buzz.git
synced 2026-11-13 04:41:12 +00:00
aa9b44ef36
> Builds on #8219 (merged); rebased onto `main` at `93ffc37cb` (includes the rustls bump, #8319). ## What This closes the identified debt from #8212. Every remaining `ON DELETE CASCADE` / `SET NULL` foreign key was audited against every supported way its parent row gets removed, with FK actions retired in the per-test schema (`test_support::retire_foreign_key_delete_actions`). One production change was needed. The rest is already explicit and is now covered by tests. **Production change:** `attach_scheduled_workflow_run` (`store/workflow.rs`) now links a scheduled fire only to a run of the **same** workflow (`EXISTS` on `(community_id, workflow_id, id)` in the existing admitted `UPDATE`). Before this, a fire for workflow B could reference a run of A. That FK is already `NO ACTION`, so the link would block A's explicit run delete. The only production caller (`buzz-workflow` cron tick) attaches the run it just created for the same workflow, so live behavior is unchanged. A mismatched attach now returns `false` like any other non-match, and the caller already treats that as best-effort. ## FK audit (all 12 delete actions) | Child FK | Action | Parent removal path → handling | |---|---|---| | `scheduled_workflow_fires → workflows` | CASCADE | Workflow delete removes fires explicitly (#8212) | | `workflow_runs → workflows` | CASCADE | Explicit (#8212) | | `workflow_approvals → workflows` | CASCADE | Explicit (#8212) | | `workflow_approvals → workflow_runs` | CASCADE | Explicit, including cross-workflow approvals (#8212) | | `join_policy_acceptances → relay_members` | CASCADE | Explicit (#8212) | | `channel_members → channels` | CASCADE | Single-channel removal is a soft delete. The community purge deletes members before channels. **Pinned here.** | | `personal_read_frontiers → personal_read_accounts` | CASCADE | No single-account hard delete exists. The purge deletes frontiers before accounts. **Pinned here.** | | `personal_read_frontiers → channels` | CASCADE | Same as above. **Pinned here.** | | `users.agent_owner_pubkey → users` | SET NULL | The only production hard delete of users is the purge deleting all of a community's users in one statement. **Pinned here** (owner + owned agent). No single-user hard delete exists. | | `product_feedback → communities` | SET NULL | Communities are tombstoned, never hard-deleted. The purge already nulls feedback `community_id` explicitly. **Pinned here.** | | `community_deletion_manifest_keys → community_deletion_requests` | CASCADE | Requests are retained. `mark_logically_verified` deletes chunks explicitly, and the chunk guard trigger is unchanged. **The existing lifecycle test now runs with FK actions retired.** | | `community_deletion_executor_heartbeats → community_deletion_requests` | SET NULL | Requests are retained, so this action is never reached. **Pinned here:** the heartbeat stays linked through the purge. | Audit method: searched `crates/**/*.rs` for `DELETE FROM` against each parent table. Every direct `DELETE FROM communities` / `users` hit is in `#[cfg(test)]` fixtures. Production removes rows only through the scoped purge loop in `store/deletion.rs` and the channel soft delete in `store/channel.rs`. **Not added:** per-parent delete APIs. They would duplicate the community purge for parents that are never hard-deleted. I also left out a source-text "no hard delete" policy test, because it would pass without any production fix. If a single-user, channel, read-account, request or community hard delete is ever added, it has to clear `agent_owner_pubkey` (only that column) and delete these children explicitly first. ## Tests - `attach_rejects_run_from_another_workflow`: a claim for B can't be linked to A's run, and the fire stays unlinked. - `purge_removes_fk_children_explicitly_without_fk_actions`: on the retired-FK schema, `channel_members` and `personal_read_frontiers` children are seeded through an admitted transaction, and then the production quiesce → fence → purge runs. Every child must be deleted explicitly, both users must be removed, feedback must survive with `community_id = NULL`, and the retained request's heartbeat (selected by `executor_id`) must stay linked. The quiesce barrier against open admitted writers is already pinned by `begin_quiescing_waits_for_open_admitted_writer_note_update`; this test does not re-claim it. - `manifest_key_chunks_bind_freeze_execution_and_cleanup` now retires FK actions first. Each test was checked to fail when its production behavior is removed: dropping the `EXISTS` guard, dropping `personal_read_frontiers` from the purge order (`23503`), dropping feedback provenance clearing, and dropping the explicit manifest chunk delete. All four failed as expected. ## What got simpler The production change is one predicate. No new concepts or APIs. The FK audit is now settled, so the follow-up migration can drop all 12 actions at once. No new delete code was needed. **Identified debt (pre-existing, outside this diff):** the cron tick in `buzz-workflow/src/lib.rs` logs only `Err` from `attach_scheduled_workflow_run`; an `Ok(false)` (no matching unlinked claim) leaves the fire unlinked without a trace. **Identified debt (unrelated, pre-existing):** `writer_pool_rejects_non_read_committed_database_default` hangs with no active query on clean `177551dda` as well (bounded after ~270 s on both checkouts). ## Verification (Blox, at the exact tree of 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 | | `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` | 510 pass, 2 fail: the pre-existing hang above, plus one 150 ms `PoolTimedOut` in `routed_fallback_spends_one_acquire_budget_when_aurora_cache_is_cold` (pool setup under full-lane load; it passed on the clean-baseline lane and passes in isolation at this tree, 1/1 in 1.7 s) | The new tests are selected by the `postgres-ci` nextest filter `(test(/postgres_tests::/) or binary(/^postgres_/)) and not test(/(^|::)external_infra[^:]*::/)`. --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Signed-off-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>