Skip to content

Automate owner deletion preparation - #7830

Merged
TheSentinel454 merged 7 commits into
mainfrom
elrond/owner-deletion-auto-prepare
Sep 29, 2026
Merged

TheSentinel454 merged 7 commits into
mainfrom
elrond/owner-deletion-auto-prepare

Conversation

@TheSentinel454

@TheSentinel454 TheSentinel454 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Why

Owner deletion admission stops at submitted. Manual inventory and approval still stand between owner intent and the existing deletion executor, preventing self-serve deletion.

What

The existing buzz-admin deletions drain process prepares and automatically approves operator-attested owner requests. Operator-origin requests retain manual approval.

How

The drain prioritizes approved work, then claims one owner submission with the existing generation lease. It freezes inventory and records digest-bound owner_automatic approval in one transaction before entering the unchanged executor.

Before approval, the transaction rechecks archived state and current ownership. It locks the deletion advisory fence, community, owner memberships, and request in that order. A regression test pins privileged abort during a live preparation lease.

What got simpler: one drain still owns preparation and execution. There is no second worker, queue, lifecycle, or retry authority. The rebased branch contains only this feature and its review fixes.

Risk

Automatic approval permits irreversible deletion without a human grace period. Operator attestation is not a cryptographic owner signature. Provenance, locked authority checks, generation fences, and digest-bound approval constrain the operation.

This remains work in progress. Broad test failures and incomplete mobile checks are disclosed below; publication is not a merge-ready verdict.

Testing

Published head: e059ae9056b447ca3b5d0c2893da86550ef16f5c. All seven commit trees match tested source 5605f27eaece1e809f746cc1839b55c7ceef8d80; only attribution metadata changed. Source-SHA results below are not new-head execution receipts.

At source commit 5605f27eaece1e809f746cc1839b55c7ceef8d80, isolated PostgreSQL tests showed stale archive and ownership fixtures fail closed after the fix. Before the fix both reached automatic approval. Removing abort's generation increment or stage transition made the new live-lease regression fail; restored production passed.

just test failed in two ACP native-git fixtures and one agent body-timeout test. The failing source files match the pinned base, but there is no exact-base execution comparison proving those failures unrelated. just test-unit also failed in the ACP fixtures, leaving 18 tests unrun. just ci stopped in Flutter tooling package resolution; separately run non-mobile lanes do not constitute a full just ci pass.

No live tenant deletion or human testing was performed. A dedicated populated 0052 → 0053 approval-backfill fixture remains a coverage gap. Final-head hosted CI and security review are required.

Bigger picture

Rebased directly onto main 12670bd0f037c66a682272bb81c46c3f254fad74; prerequisite #7818/#7827 are already merged. Main owns 0052_channel_artifacts, so this feature's automatic-approval migration is now 0053. Client/KGoose and quota work are separate PRs and are not part of this diff.

Generated with Codex

@TheSentinel454
TheSentinel454 force-pushed the elrond/owner-deletion-auto-prepare branch from 41379ec to 4640245 Compare September 25, 2026 15:35
@TheSentinel454
TheSentinel454 marked this pull request as ready for review September 25, 2026 15:35
@TheSentinel454
TheSentinel454 requested a review from a team as a code owner September 25, 2026 15:35
@TheSentinel454
TheSentinel454 force-pushed the elrond/owner-deletion-auto-prepare branch from 4640245 to 035e4a4 Compare September 25, 2026 16:04

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No blocking source defect found at 035e4a42e5906ba0f66e48eb47cc41c0179f8476, against b79bd19c73b188d9fb785cbcd5ba50ccad74b295. This is not approval or a green-CI verdict.

Owner-only preparation, atomic inventory/approval, generation fencing, and the digest-checked executor preserve the intended no-grace flow; operator-origin requests retain manual approval. Early privileged abort is supported in this tree.

Existing PostgreSQL CI: 504/505 passed, including all five new engine preparation/dispatch tests, new store tests, and schema parity. Its synthetic merge bbd536e0a0bceca6369b9afc2a60e7987c301185 has the same tree as this head.

Remaining gate: cluster_global_probe_rotates_epoch_on_same_epoch_token_regression failed with MaskedActivity { masked: 1 }. That file is unchanged here, but the failure is unattributed, not proven flaky; the author/CI owner must resolve it. No rerun requested. Source-only review: no checkout, build, test, or execution of PR code; live interruption and mixed-version rollout were not exercised.

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Thanks for this. I reviewed head 035e4a42 against base b79bd19c (the #7827 head), so only this PR's two commits. Reusing the existing drain, lease and executor keeps this small, and the core transaction holds up. complete_owner_preparation computes one digest from the inventory, locks the request row, and commits the frozen inventory, the owner_automatic approval and the move to approved together, so there's never a committed "frozen but not approved" row. Abort during an active preparation lease is also safe: it clears the lease and bumps the generation, and both a stale completion and a stale heartbeat are rejected afterwards. We ran the PR head locally:

  • The Postgres lanes passed 326/326, including the new deletion-store, deletion-engine and migration tests.
  • A populated 0050 → 0051 migration kept the existing approval and backfilled it to operator.
  • An owner request admitted through the store and then run through the real buzz-admin deletions drain went submitted → approved → fenced → … → retention_pending. The request, inventory and approval digests all matched. An operator-origin submitted request in the same queue stayed at submitted with zero attempts.

Two things block this, though.

1. Automatic approval accepts an owner-origin row that has no owner. claim_owner_submission filters on request_origin = 'owner' and acknowledgement_version = 1, and complete_owner_preparation only requires a non-NULL mediating_operator_pubkey. Nothing on the automatic path requires owner_pubkey. Because the 0050 provenance CHECK evaluates to NULL rather than FALSE when owner fields are missing (the NULL gap from the #7818 review), a row with request_origin='owner', owner_pubkey = NULL, a valid mediator and acknowledgement 1 is schema-valid. We inserted exactly that row and ran the unmodified drain. It recorded an owner_automatic approval, purged the membership and took the community all the way to retention_pending with no error. The admission endpoint doesn't produce rows like this, so it isn't an HTTP exploit. But this PR is what turns an incomplete provenance row into an irreversible deletion; before, it sat at submitted until an operator acted. I think the automatic path should fail closed on its own: require owner_pubkey, mediating_operator_pubkey and acknowledgement_version to be non-NULL (and requested_by to match the owner) in the claim predicate and again inside complete_owner_preparation, and block the row with a clear reason rather than approving it. The CHECK fix itself can land in 0050 with #7818 since neither PR has merged. A regression test that inserts a malformed owner row and asserts it's never claimed or approved would cover it.

2. The blocked-row guards on the preparation path have no test that fails without them. There are three: blocked_at IS NULL in the claim, the same check in verify_owner_submission_lease, and blocked_reason.is_none() in complete_owner_preparation. With all three removed, all 326 Postgres tests still passed, and the rebuilt drain auto-approved a blocked fixture. The unchanged executor's own block check is the only reason it didn't go on to delete. I'd add one test where the row is blocked before the claim (never claimed) and one where it's blocked after the claim (heartbeat and completion both refuse, and no approval row exists).

Non-blocking:

  • There's no re-check at preparation time that the community is still archived and that owner_pubkey is still its owner. For rows admitted normally, the #7818 fence covers this: unarchive and transfer both returned DeletionPending after admission in our run. Still, this is now the last check before destructive work, so I think re-checking both under the row lock in complete_owner_preparation is cheap defense in depth.
  • ARCHITECTURE.md, the runbook, the Drain help text and the claim doc comments call this "authenticated owner intent". The relay authenticates the operator's signature, not the owner's (api/operator.rs), so I think "operator-attested owner intent" is more accurate and keeps anyone from reading it as cryptographic owner consent.
  • There's no test for aborting a submitted request while a preparation lease is live. The behavior is correct in our run, but a test would pin the generation/stage fencing that makes it safe.
  • owner_deletion_auto_approval_migration_matches_desired_schema checks substrings across whole files, and the 0029 parity check now skips community_deletion_approvals except for two invariants. A populated 0050 → 0051 backfill test in migration.rs would make the migration coverage falsifiable.

CI: PostgreSQL Tests is red with one failure, runtime::replica_fence::postgres_tests::cluster_global_probe_rotates_epoch_on_same_epoch_token_regression (MaskedActivity { masked: 1 }), in a file this PR doesn't touch. The other 504 passed. I haven't attributed a cause.

@TheSentinel454
TheSentinel454 force-pushed the elrond/owner-deletion-auto-prepare branch from 035e4a4 to 98f4e91 Compare September 25, 2026 18:14

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Source review clear: no actionable blocker found at this head. The prior malformed-owner-provenance hole is closed in the current base: both schema paths explicitly reject NULL owner, mediator and acknowledgement fields. The drain preserves owner-only preparation, atomic digest-bound approval and generation-fenced execution; operator-origin requests remain manual.

Remaining nonblocking gap: the prior blocked-row regression-test concern is not fully resolved; details and the bounded fix are inline. Production block predicates are present. Schema-parity assertions also remain weaker than a live populated-upgrade test, although the current migration/schema definitions agree.

Reviewed HEAD 98f4e91ef117f0f667f8391edd5e154be664405d against BASE 494d5744360f2d39c464e8ac8b83ae2276ccf0f6, following previously reviewed 035e4a42e5906ba0f66e48eb47cc41c0179f8476. Source-only on the pinned Blox: no checkout, build, test, migration or PR-code execution. The established no-grace flow is unchanged.

Existing CI passed unit, Rust lint, PostgreSQL (509/509), relay E2E, backend integration and desktop integration. PostgreSQL used synthetic merge a193ca9 of this head/base. Live interruption, real storage deletion and mixed-version rollout were not exercised by this review. This is a non-approving review comment.

Comment thread crates/buzz-db/src/store/deletion.rs
@TheSentinel454
TheSentinel454 added this pull request to stack #7902 September 25, 2026 20:37

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Re-review at 98f4e91e against base 494d5744 (the current #7827 head). One of my two blockers from 035e4a42 is resolved; the other is still open, so I'm still requesting changes.

This head is a rebase. Compared with 035e4a42, the PR's patch only lost content that now lives in the base: the abort extension to submitted/inventoried and its tests moved into #7818. The preparation code and its tests are otherwise the same as what I reviewed last time.

Resolved: malformed owner-origin rows. The 0050 provenance CHECK in the base now rejects a NULL owner_pubkey, mediating_operator_pubkey or acknowledgement_version on owner rows, and pins requested_by = owner_pubkey (0050:27-40). We confirmed that with raw inserts on #7818, so the row the drain auto-approved last time can no longer exist.

Still blocking: the blocked-row guards on the preparation path aren't pinned by any test. The three guards are unchanged: blocked_at IS NULL in claim_owner_submission (deletion.rs:1337), the same check in verify_owner_submission_lease (:3452), and blocked_reason.is_none() in complete_owner_preparation (:1436). I went through every test that calls the owner-preparation seams (claim, heartbeat, complete, block) in buzz-db and buzz-deletion. None of them attempts a claim, heartbeat or completion on a blocked owner row:

  • owner_preparation_retry_block_and_privileged_abort_are_recoverable blocks and then aborts.
  • drain_owner_preparation_persists_transient_and_permanent_failures only reads the stored blocked_reason.
  • The operator hold case exercises the generic executor heartbeat.

Last round, removing all three guards left the whole PostgreSQL suite green, and a rebuilt drain auto-approved a blocked fixture; only the unchanged executor's own block check stopped the deletion. That test code hasn't changed. Since this path turns owner intent into destructive work without a human in between, I'd want the guards to have their own tests:

  • One where the row is blocked before the claim, and neither claim_specific_owner_submission nor the claim-next path picks it up.
  • One where the row is blocked after the claim: heartbeat_owner_submission and complete_owner_preparation both refuse, and no owner_automatic approval row exists.

Carl's inline note on deletion.rs:5208 describes the same gap.

Non-blocking, carried over and still applicable:

  • complete_owner_preparation doesn't re-check, under the row lock, that the community is still archived and that owner_pubkey still owns it. The #7818 fence covers normally admitted rows, so this is defense in depth.
  • The doc comments and Drain help still say "authenticated owner intent" (e.g. deletion.rs:1302, :1401, buzz-deletion/src/lib.rs:293). The relay authenticates the operator's signature, so I think "operator-attested owner intent" is more accurate.
  • There's still no test for aborting a submitted request while a preparation lease is live.

CI: run 36172144382 at this head is green (PostgreSQL, relay E2E, integration). The cancelled run next to it is a superseded duplicate.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Source review clear; CI is not green. Reviewed HEAD b02926e2350602a3c82647203c04764538ce989d against BASE 494d5744360f2d39c464e8ac8b83ae2276ccf0f6.

The only change since the previous reviewed head is two blocked-owner regression tests. They now bind both claim paths, heartbeat and completion with a live lease, and assert no approval is created. The prior blocked-guard coverage concern is resolved. Owner-only preparation, atomic digest-bound approval and generation-fenced execution preserve the intended no-grace flow; operator-origin requests remain manual. No actionable source blocker found.

Before merge: resolve the stack/main migration-number collision. Existing PostgreSQL CI tested synthetic merge db3aacbb38e589a3c78a5278949fe8d87a16a9fa: both new tests passed, but the suite finished 532/584, with 52 migration failures because main and the base stack both supply 0050. Shift this PR’s following migration/version assertions as needed and obtain passing migration/unit evidence. The security-authorization check separately rejects stacked PRs not targeting main.

Nonblocking: migration-applied behavior, isolated same-executor generation fencing and retry-exhaustion coverage remain gaps. The deadline recovery instructions should distinguish submitted owner preparation: run <id> is execution-only; use a larger deadline or a staffed drain for preparation.

Source-only review: no checkout, build, test or PR-code execution. Populated upgrade, mixed-version rollout and live interruption were not exercised. This is a non-approving comment, not a merge-ready verdict.

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Re-review at b02926e2 against base 494d5744 (the #7827 head, unchanged since last round). My last blocker is resolved, so I'm no longer requesting changes.

The delta from 98f4e91e is one commit that only adds two PostgreSQL tests in deletion.rs. blocked_owner_submission_cannot_be_claimed covers a row blocked before claim through both claim_specific_owner_submission and claim_next_owner_submission. blocked_claimed_owner_submission_cannot_heartbeat_or_approve covers a row blocked after claim: it expects heartbeat and completion to refuse, the stage to stay submitted, and zero approval rows of any origin.

We ran the mutations locally at this head. With each guard removed on its own, a new test goes red:

  • The claim predicate at deletion.rs:1337 fails the specific-claim assertion.
  • The lease verifier at :3452 fails the heartbeat assertion.
  • The completion check at :1436 fails the completion assertion. Completion doesn't go through the heartbeat verifier, so this test is its only coverage.

With all three removed, both tests fail. The restored tree passes 319/319 in the buzz-db PostgreSQL suite. The blocked_at/blocked_reason pairing CHECK makes the mixed column checks equivalent.

CI needs attention before merge, though. Rust / Unit Tests is red on the synthetic merge (job): embedded_migrator_contains_consolidated_initial_schema sees 52 migrations instead of 51, and deletion_surface_parity_between_migration_0029_and_schema_sql can't find the 0050 guard. Main now has its own migrations/0050_operator_listener_mentions.sql from #7793, so the stack's 0050_owner_community_deletion_admission and this PR's 0051_owner_deletion_auto_approval need renumbering. The count and index assertions in migration.rs need to move with them. The pinned head/base pair itself is consistent, and the downstream PostgreSQL and E2E lanes were skipped because of this failure.

Non-blocking items carried over from last round and still open: complete_owner_preparation doesn't re-check archived state or ownership under the row lock; the comments and Drain help still say "authenticated owner intent" where "operator-attested" is more accurate; and there's no test for aborting a submitted request while a preparation lease is live.

TheSentinel454 pushed a commit that referenced this pull request Sep 28, 2026
Signed-off-by: Codex <noreply@openai.com>
@TheSentinel454

Copy link
Copy Markdown
Contributor Author

Published test-only repair at dc0a125, additive over 53c78cd.

The inventory future now signals a oneshot immediately after constructing its drop guard; lease revocation awaits that signal rather than guessing with a 20 ms sleep. Existing outcome/drop assertions are unchanged. What got simpler: the test no longer depends on preflight heartbeat latency or scheduling; no production state, cancellation behavior, schema or dependency changed.

Blox evidence at this exact commit (independently inspected by Gandalf): fmt/check pass; ordinary buzz-deletion package suite 17 passed / 14 ignored; supported PostgreSQL package lane 11 passed / 20 profile-skipped. A separately compiled cancellation mutant entered inventory, revoked the lease, then failed under an external 30-second runtime bound (124); the earlier mutant compile error is retained but not counted as detection. Final source was restored and the full PostgreSQL lane rerun cleanly. No local test execution; publication used --no-verify.

Evidence: buzz-community-self-delete:/home/bloxer/PR7830_BARRIER_FIX_53C78_REPORT.md, SHA256 6439b0369a5274ba421cac8afa5bb5cd910d60fff30f85b4ae3c7cf7c5f31b20. Evidence manifest is pr7830-barrier-fix-53c78-evidence/SHA256SUMS.

Known test-hardening debt: the committed test retains its pre-existing lack of a per-test deadline; cancellation regression detection used an external bound in the mutation experiment. This is not a claim of prompt bounded CI failure. Retaining the exact verified commit for this narrow repair; reviewers may recommend a separate timeout change. Actual Codex author/DCO retained; its redundant self-coauthor trailer was not rewritten merely for cosmetics.

Fresh review, terminal CI and targeted real readiness/outage/no-work-drain verification remain pending. This is not deletion E2E or an overall rollout PASS. Existing human review/security authorization and live-admission exception remain unchanged.

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Re-review at dc0a125c against base 464a4e60 (the current #7827 head). Still clear; the migration collision from last round is fixed.

Since b02926e2 there are two base merges and one new commit. I replayed the old head onto the new base with git merge-tree and compared the result with this head. Everything except migration.rs merged on its own with no differences. The only other changes are the ones the renumber needs:

  • 0051_owner_deletion_auto_approval.sql is renamed to 0052 with identical content, on top of main's 0050_operator_listener_mentions and the stack's 0051_owner_community_deletion_admission.
  • embedded_migrator_contains_consolidated_initial_schema resolves the conflict to 52 migrations, with [49]..[51] = 50..52 and approval_origin at [51]. The 0052 lookup in owner_deletion_auto_approval_migration_matches_desired_schema and the thread_window final version (52) moved with it.

The head contains main through #7341 (b37e4772). Main has since picked up #7805 and #7288. Neither adds a migration or touches deletion code, and this head still merges cleanly with current origin/main.

The base merge also brought in #7818's abort serialization, where owner convergence now takes the shared deletion lock inside lock_owner_mutation_admission. I checked how that interacts with this PR's preparation path. complete_owner_preparation only takes its request row FOR UPDATE. Abort takes the exclusive community lock and then the row. Convergence takes the shared community lock, then communities/relay_members FOR UPDATE, and only reads community_deletion_requests with a plain SELECT. None of these can wait on each other in a cycle, so there's no new deadlock path.

dc0a125c swaps the 20 ms sleep in drain_owner_preparation_cancels_inventory_after_lease_loss for a oneshot signal sent from inside the inventory future. Before, the revoke could run before the preparation's initial heartbeat, so build never ran and the drop assertion flaked. Now the revoke only happens once inventory has started, and if preparation exits before build runs, the dropped sender fails the test right away instead of hanging. prepare_owner_claim_with takes FnOnce, so moving the sender into the closure is fine.

Non-blocking items carried over and still open: complete_owner_preparation doesn't re-check archived state or ownership under the row lock; the doc comments, Drain help and runbook still say "authenticated owner intent" (e.g. deletion.rs:1302, :1401, buzz-deletion/src/lib.rs:293, docs/operator-community-deletion.md:147) where "operator-attested" is more accurate; and there's no test for aborting a submitted request while a preparation lease is live.

CI at this head: Rust Unit Tests, Rust Lint, PostgreSQL Tests, Relay E2E, Backend Integration, cross-compiles, and image builds are all green. The desktop lanes still running (Desktop Core, Smoke E2E shard 4, Desktop E2E Integration) are outside this diff. Authorize Security Review fails because the PR targets the stack branch rather than main, which is expected for a stacked PR.

@TheSentinel454

Copy link
Copy Markdown
Contributor Author

Sidebar required-check repair

Published #7827 5650ae7a5dcdbbc88dedd0e4b60a3270bd9ec959; #7830 additively refreshed to d8c920ce5cace151d34f7e1580f699faa750380f. The complete #7830 own-patch is byte-identical before/after (binary diff comparison).

Diagnosis: I inspected the failed #7830 CI job 109113919391, retry-1 trace. Navigation returned at 719774ms, the 500ms count expired at 720293ms, and the post-failure DOM at 720526ms contained all 14 exact snapshot IDs. The filmstrip shows the boot splash during the assertion. This attempt failed on late initial rendering, not a snapshot already replaced; fallback assertions never ran. #7790 converted the neighboring hash-mismatch test to existing DOM recording but left this fallback test with the transient 500ms count. That history is not a claim that #7790 alone caused late mounting.

Change: reuse existing trackSnapshotRows and assert every expected ID. Preserve the stale-hash → null-hash request sequence, visible live list, snapshot removal, persisted authoritative contents, and dedicated cold-boot paint deadline. No production change, timeout increase, retries, new helper, or diagnostics in the commit.

Evidence (Blox, Chromium, one worker, zero retries, unchanged application bundle):

  • Original assertion at normal speed: 10/10 passed (non-reproduction).
  • Controlled 6× CPU slowdown: original assertion 0/5; candidate 5/5. Slowdown is diagnostic-only and not in the commit.
  • At exact committed Schedule the deletion drain safely #7827 head: normal-speed 10/10; entire sidebar snapshot spec 10/10, exit 0.
  • Both controlled arms overlapped the prior full-smoke job on the same worker; this is not an idle-machine benchmark or proof of historical CI CPU load.
  • Full Desktop package tests/check/typecheck and just ci remain in progress. Fresh-head CI and independent reviews remain required. No overall PASS, human-check completion, or release-readiness claim.

What got simpler: removed a second boot-performance timing special case by reusing the in-file observation mechanism. Prior scroll fix remains intact. The old scroll job's evidence stays attributed to 131ef21a; no new #7830 runtime pass is claimed from patch equivalence.

@TheSentinel454

Copy link
Copy Markdown
Contributor Author

Additive main refresh

Current heads: #7827 7a26420b67c6137d1d60c3d2bbf670cb82c06a1d, #7830 3c8def0507da8d5e5b1622225b5ee5bfeb7520bd.

Main f1e50be4103addabd497563495e0ffe5bda07ef7 landed #7955's equivalent scroll readiness call plus explanatory comment. Resolved the one-file conflict by retaining main's exact scroll spec, using additive merges only. No history rewrite.

#7827's diff against main is 16 files: 15 infrastructure/docs files and the sidebar fix. Scroll-history and video-attachment are now identical to main.

#7830's deletion changes are preserved. Raw binary-patch comparison differs, because main #7854 added the unrelated GIN index and its assertion in schema.sql/runtime/migration.rs, changing blob IDs and hunk offsets. After excluding only those index headers and hunk line offsets, the complete own-patch compares identical. No deletion hunk changed.

Verification remains attributed to the SHA actually executed. Main contains production changes; old sidebar/scroll runs do not establish runtime success at these new merge heads. Fresh CI and exact-head Legolas/Gimli reviews requested.

Environment correction: I accidentally shared writable node_modules between Blox worktrees; bootstrap rewrote dependency links during the old full smoke, causing mixed Playwright collection errors. That broad run is invalid as a product result. Dependency trees are now independent; a clean full-sidebar rerun at old head5650 passed10/10. Full Desktop tests6731+92, check/typecheck passed at5650; the earlier repository just-ci run at131ef21a was blocked on absent glib-2.0.pc. Current verification continues asynchronously and is not an overall PASS.

Base automatically changed from elrond/generic-operator-cron to main September 28, 2026 22:23

@wpfleger96 wpfleger96 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Re-review at 3c8def05. The PR's own changes are still clear, but it needs a conflict resolution before it can merge.

The new head only brings in the refreshed #7827 base and main. I diffed this PR's own patch (7a26420b..3c8def05) against the one I last reviewed (464a4e60..dc0a125c), and they're identical apart from hunk offsets. On the 8 files this PR touches, the base only added the idx_events_tags_gin index to schema/schema.sql and a matching assertion in migration.rs. Neither interacts with the auto-approval migration or the preparation path. 0052 is still the next free migration on main.

#7827 then squash-merged as 6918dd3a, and GitHub now shows this PR as conflicting with main in ARCHITECTURE.md and docs/operator-community-deletion.md. Main's copies of both files are byte-identical to the #7827 head this branch was built on, so the fix is to take this branch's side in both. That reproduces exactly the patch reviewed here. I'll take another look once the resolved head is pushed.

Non-blocking items carried over and still open. First, complete_owner_preparation doesn't re-check archived state or ownership under the row lock. Second, the doc comments and runbook still say "authenticated owner intent" (e.g. deletion.rs:240, :302, :1401, docs/operator-community-deletion.md:147), where "operator-attested" is more accurate. Third, there's no test for aborting a submitted request while a preparation lease is live.

CI at this head: 52 jobs pass and 28 are skipped. The only failure is Authorize Security Review. That run started at 20:56Z, while the base was still the #7827 branch, so it should clear on the next push now that the base is main.

codex and others added 2 commits September 29, 2026 14:29
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Codex <noreply@openai.com>
codex and others added 5 commits September 29, 2026 14:29
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Signed-off-by: Codex <noreply@openai.com>
Signed-off-by: Codex <noreply@openai.com>
Signed-off-by: Codex <noreply@openai.com>
Signed-off-by: Codex <noreply@openai.com>
Signed-off-by: Codex <noreply@openai.com>
@TheSentinel454
TheSentinel454 force-pushed the elrond/owner-deletion-auto-prepare branch from 3c8def0 to e059ae9 Compare September 29, 2026 14:32
@github-actions

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 12670bd0f037c66a682272bb81c46c3f254fad74...e059ae9056b447ca3b5d0c2893da86550ef16f5c.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review e059ae9056b447ca3b5d0c2893da86550ef16f5c to authorize a new review.
Any previous review applies only to its recorded range.

@TheSentinel454
TheSentinel454 marked this pull request as draft September 29, 2026 14:32
@TheSentinel454

Copy link
Copy Markdown
Contributor Author

@buzz-security-review e059ae9

@TheSentinel454
TheSentinel454 marked this pull request as ready for review September 29, 2026 14:33

@wpfleger96 wpfleger96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Re-review at e059ae90 against main @ 12670bd0. Still clear. I found nothing blocking, and all three items I'd carried over from the last round are addressed.

The rebase is clean. The migration moved from 0052 to 0053 with an identical blob, since main took 0052_channel_artifacts. Beyond the renumber, the rewritten commits differ only in the migration count and index assertions.

The new revalidation in complete_owner_preparation looks right to me. It takes the shared deletion advisory, then the community row, then the owner rows in pubkey order, then the request row. The first three steps are exactly the order lock_owner_mutation_admission uses on main. abort takes the exclusive advisory before any row lock, so there's no cycle, and the heartbeat/retry/block paths never reach back for the community row. A stale lease is still checked first and still returns stale. Authority drift raises DeletionSafety, which the drain routes to block_owner_preparation. So it fails closed with nothing frozen or approved, and unblock and abort both stay available. The "operator-attested" wording is consistent across the files this PR touches. The new abort test confirms the lease was live before the abort, then checks the generation bump, the cleared lease, the stale heartbeat/completion, and that no inventory or approval exists afterwards.

We ran the deletion/PG suites at this head locally (353/353 PG, 152/152 non-ignored) and then removed each new guard one at a time. Removing the archive check, the whole owner-authority check, or abort's lease clear plus generation bump each turns a new test red. Four finer removals survive both suites:

  • deletion_state != "active"
  • deleted_at.is_some()
  • the sole-owner count
  • owner-pubkey equality on its own

The "owner changed" fixture demotes the only owner, which leaves zero owners, so it never exercises a different sole owner or an extra co-owner. I'm not treating this as blocking. While a request is live, main already refuses transfer and unarchive, so these conjuncts are defense in depth rather than the primary guard. Still, adding fixtures for a different sole owner, an extra owner, a non-active community and a set deleted_at would pin each of them.

One small inconsistency: admission and completion don't agree on what owner authority means. admit_owner_request accepts any current owner (deletion.rs:904-915), but completion requires a sole owner (:1469-1470). On a legacy co-owned archived community, the request gets a 202 and then blocks with "no longer archived under the admitted owner", which points the operator at the wrong cause. It fails closed either way. I'd make the two predicates match, either by rejecting co-owned communities at admission or by checking membership at completion, and word the error to fit.

A couple of small notes. buzz-relay/src/api/operator.rs:321 and :440 still say "authenticated owner intent". That file came from main and isn't touched here, so it can be a follow-up. All seven commits are now authored and signed off only as Codex <noreply@openai.com>, and the earlier human Signed-off-by is gone. DCO Check passes, so I'm only flagging it in case you want a human sign-off.

CI at this head: 53 pass, 28 skipped, no failures. The PostgreSQL Tests job ran 621/621, including both new tests.

@TheSentinel454
TheSentinel454 merged commit bba75a7 into main Sep 29, 2026
88 checks passed
@TheSentinel454
TheSentinel454 deleted the elrond/owner-deletion-auto-prepare branch September 29, 2026 15:39
wpfleger96 pushed a commit that referenced this pull request Sep 29, 2026
…-enforcement

* origin/main:
  Automate owner deletion preparation (#7830)

Signed-off-by: Hayt <211b96e6a2b7f45fd4047988976c7bbbeeda0c15f3ae7b32eec20834b5a55118@buzz.block.builderlab.xyz>
johnmatthewtennant pushed a commit that referenced this pull request Sep 29, 2026
…in-ui

* origin/main:
  feat(buzz-relay): NIP-FI stateless enforcement (S3) — upgrade gate, NIP-42 pairing, session lifetime, JWKS warm (#7224)
  feat(web): add Browse releases link next to invite download (#2255)
  docs(nips): fix stray angle brackets in created_at clauses (#4486)
  docs: specify desktop-driven mobile push suppression (#7809)
  feat(mobile): add the contextual identity-name resolver (#7894)
  Automate owner deletion preparation (#7830)
  feat(relay): implement NIP-AR channel artifacts (#7919)
  fix(desktop): resolve unlisted project channel requests (#7619)
  Schedule the deletion drain safely (#7827)
  fix(mobile): stale community selection during mobile invite setup (#7951)

Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants