feat(buzz-relay): idempotent owner community deletion with quota reservation - #7969
Conversation
Signed-off-by: tornquist <tornquist@squareup.com> Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: tornquist <tornquist@squareup.com>
Owner admission records durable intent with no owner-facing cancellation, so an operator needs a recovery path when preparation cannot continue. Abort already reversed approved and fenced requests; extend it to the submitted and inventoried stages, which have destroyed nothing. Aborting releases the durable request fence over owner listing, unarchive, and owner rotation. It reverses deletion intent only: the community stays archived and the owner restores it explicitly. Stages from drained onward stay closed. Lock ordering and the active-serving-write-lease guard are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
The owner branch of community_deletion_owner_provenance tested only the shape of each provenance column. A bare `col ~ '...'` on a NULL column evaluates to NULL, and a CHECK constraint is satisfied by NULL, so `FALSE OR NULL` admitted owner-origin rows with no owner key, no mediating operator, or no acknowledgement version. Reject a NULL in each column before testing its shape. Spell it `NOT (col IS NULL)`: pgschema drops a named CHECK whose body contains `IS NOT NULL` and still exits 0, which would leave the desired-state bootstrap silently unguarded while the migration path stayed correct. Assert one shared case table from both schema sources -- the migration upgrade path and the pgschema desired-state bootstrap -- so the two cannot drift into different owner-provenance guarantees. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
The malformed-body coverage sent a single request carrying several invalid fields at once, including an unparseable `request_id`. Request deserialization runs before the host and pubkey guards, so that request was refused by serde and the guards it claimed to cover were never executed — dropping the `owner_pubkey` validation entirely left the test passing. Give each guard its own request whose remaining fields are valid, so the case reaches the guard under test, and assert no deletion intent persists for any refusal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: tornquist <tornquist@squareup.com>
`/operator/communities/delete` admits an irreversible request, but the endpoint's own tests only exercised the happy path and relied on generic operator-auth coverage elsewhere. Nothing pinned that this route refuses an unsigned caller, the X-Pubkey dev fallback, a non-operator signer, or a signature bound to a different method, URL, or body. Add direct coverage for each binding failure, and for idempotent replay: an identical resubmission converges on the stored request, while a resubmission that changes the owner, the host, or the acknowledgement version is refused and leaves the stored intent untouched. Every rejection asserts no deletion request was persisted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: tornquist <tornquist@squareup.com>
The admission path reads as if the relay had verified the owner's consent. It has not: the mediating operator authenticates the owner and collects the acknowledgement out of band, and the request reaching the relay carries only that operator's NIP-98 signature. The relay checks operator authority and current ownership, and stores the owner pubkey, operator pubkey, and acknowledgement version as provenance for the upstream ceremony — never an owner-signed attestation. Also record how manual handoff converges. Owner provenance pins `requested_by` to `owner_pubkey`, so `buzz-admin deletions submit` must repeat the owner pubkey to adopt an admitted request; the operator's own pubkey conflicts with the one-active-request invariant instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
The runbook told operators to target `cronjob/<release>-buzz-deletion-drain`. `buzz.fullname` collapses to the release name when it already contains the chart name, so the documented name is wrong for the documented install: `helm install buzz ...` renders `buzz-deletion-drain`. Discover the CronJob by its component label instead, and describe the name rule rather than a single guessed spelling. Three more corrections: `activeDeadlineSeconds` was presented as if a timed-out run were just another retry. It is not recorded as one — shutdown releases the claim without recording a retry, and only the object-store drain resumes mid-stage — so a deadline landing repeatedly inside a non-resumable stage loops forever with a rising `attempts`, a flat `retry_count`, and no block. Document how to spot that from both the request and Kubernetes, how to size the deadline, and how to recover. `terminationGracePeriodSeconds` was presented as a clean handoff. Document that a pod still working at the end of the window is SIGKILLed holding its lease, and that recovery is lease expiry plus reclaim under a new generation. An empty `serviceAccountName` was described as a neutral default. It inherits the relay's service account; `automountServiceAccountToken: false` hides the projected token but does not detach cloud IAM bindings resolved through the node metadata path. Recommend a dedicated pre-created account when the executor's IAM blast radius should be smaller than the relay's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Exercise both supported credential shapes through the CI render matrix and bind the typed job guards, schema boundary, pod identity, and default sidecar annotation. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: tornquist <tornquist@squareup.com>
Share a suffix-preserving 52-character name helper between the deletion drain and storage accounting CronJobs while retaining their existing short names. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: tornquist <tornquist@squareup.com>
Distinguish the disabled Kubernetes API token mount from provider workload-identity credentials while retaining the dedicated service-account recommendation. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: tornquist <tornquist@squareup.com>
Bind each context-menu interaction to the exact mock message returned by the emitter so same-second ordering cannot select the wrong video. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: tornquist <tornquist@squareup.com>
Signed-off-by: Codex <noreply@openai.com> Co-authored-by: Codex <noreply@openai.com>
Signed-off-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: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Signed-off-by: Codex <noreply@openai.com> Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: tornquist <tornquist@squareup.com>
The runbook told operators to target `cronjob/<release>-buzz-deletion-drain`. `buzz.fullname` collapses to the release name when it already contains the chart name, so the documented name is wrong for the documented install: `helm install buzz ...` renders `buzz-deletion-drain`. Discover the CronJob by its component label instead, and describe the name rule rather than a single guessed spelling. Three more corrections: `activeDeadlineSeconds` was presented as if a timed-out run were just another retry. It is not recorded as one — shutdown releases the claim without recording a retry, and only the object-store drain resumes mid-stage — so a deadline landing repeatedly inside a non-resumable stage loops forever with a rising `attempts`, a flat `retry_count`, and no block. Document how to spot that from both the request and Kubernetes, how to size the deadline, and how to recover. `terminationGracePeriodSeconds` was presented as a clean handoff. Document that a pod still working at the end of the window is SIGKILLed holding its lease, and that recovery is lease expiry plus reclaim under a new generation. An empty `serviceAccountName` was described as a neutral default. It inherits the relay's service account; `automountServiceAccountToken: false` hides the projected token but does not detach cloud IAM bindings resolved through the node metadata path. Recommend a dedicated pre-created account when the executor's IAM blast radius should be smaller than the relay's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
Signed-off-by: Codex <noreply@openai.com>
Signed-off-by: Codex <noreply@openai.com> Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Codex <noreply@openai.com>
Signed-off-by: Codex <noreply@openai.com>
🔐 Codex Security Review
|
Signed-off-by: Codex <noreply@openai.com>
Signed-off-by: Codex <noreply@openai.com>
…lifetime owner cap Drop POST /operator/communities/delete/receipt. Owner deletion admission already converges on an existing request UUID with the same tuple before any owner or archive check, so resending delete returns 202 with the current status at any stage and admits no new work. Count every non-aborted owner deletion, completed or not, toward a lifetime cap of 20 communities per owner (never below the active cap) on create and transfer-in, so create-then-delete cannot squat tombstoned hosts. The owner-list can_create projection reflects both caps. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
…create contract Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
…upported, not a conflict Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
|
E2E: EXERCISED AND WORKS (scoped) — tested head
Source receipts: |
Signed-off-by: OpenAI Codex <codex@openai.com>
Signed-off-by: OpenAI Codex <codex@openai.com>
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Reviewed at e21151f4 against main @ 8519db15. Clear. I found nothing blocking.
Replay recovery looks right. NIP-98 verification, replay consumption and the operator allowlist all finish before the body is parsed or the UUID is looked up, so a non-operator can't learn anything about a request. admit_owner_request checks the ack version first, takes the per-UUID advisory lock, then compares host, owner origin, owner and version against the stored row. A matching resend returns the stored stage without needing surviving membership, and it never admits new work.
The quota invariant holds across all its writers, not just the ones the PR names. Create and transfer-in take the same per-recipient advisory lock around count and grant, so no two grants can both take the last slot. Owner deletion admission reserves a community that's already counted, and the UNION dedupes membership against intent, so admission doesn't change the count and doesn't need that lock. Purge keeps the reservation. Logical completion frees the active slot but keeps the lifetime count, and abort only happens before destruction, while membership is still live. Tenant role changes, invites, the admin CLI and the allowlist backfill can't grant owner. The startup/legacy provisioning exception was already there at base.
I also ran it live on the head binary with isolated PG/Redis/MinIO:
- A base 0001–0053 database upgraded to 0054 on boot, and the index is there.
- A resend after membership purge, after a real abort and after a full drain returned 202 with the current status. A changed host or owner got 409
deletion_request_conflict, an unsupported version got 400, an outsider got 403 and a reused NIP-98 proof got 401. Eight concurrent same-UUID submissions left one row. - After 20 real create/archive/delete/drain cycles, create and transfer-in both returned
limit_reached, withquota_used=0andcan_create=false. - Mixed, create-only and transfer-only races at 19/20 each let exactly one through. Ten attempts at 20/20 let none through.
- Removing only the lifetime-cap conjunct made both new quota tests fail.
A few small things, none blocking:
docs/operator-community-deletion.md:158says any changed tuple under a known UUID gets409. A changed ack version actually gets400 unsupported_acknowledgement_version, which the next paragraph and the test both say. Worth adding that exception here and in the PR summary.- Lifetime usage includes live ownership, so with the absolute cap no owner can hold more than 20 live communities, whatever
BUZZ_MAX_COMMUNITIES_PER_OWNERsays. The doc onmax_communities_per_owner(relay_members.rs:572-573) still says operators can raise the cap. With an override above 20, the owner list reports aquota_limitthe owner can never reach. I didn't find a deployment that sets it above 20, so this is just a doc fix (or a startup warning). - The new lifecycle errors carry a stable
code, butlimit_reachedon transfer and create is still only a message prefix (operator.rs:725,community_provisioning.rs:290). The doc listslimit_reachednext to the coded errors, so clients may expect acodethere too. - The rollout note says to recreate any database that ran the earlier
0054. That fits disposable preview DBs, since the draft file has a different checksum and index predicate. I'd say so explicitly so nobody applies it to a database that holds real data.
CI at this head is green. The PostgreSQL Tests job ran 707/707 and Unit Tests ran all the new cases.
Give create and transfer-in limit_reached rejections a stable code, keeping the message prefix for older clients. Document that an unsupported acknowledgement version returns 400 before the tuple comparison, that active-cap overrides above the lifetime cap are unreachable, and the sole-current-owner guard on the delete handler. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Give create and transfer-in limit_reached rejections a stable code, keeping the message prefix for older clients. Document that an unsupported acknowledgement version returns 400 before the tuple comparison, that active-cap overrides above the lifetime cap are unreachable, and the sole-current-owner guard on the delete handler. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Main took migration version 54 for the owner-deletion quota index (#7969), which collided with this branch's 0054_personal_read_state.sql. The personal read migration is renumbered to 0055 with its SQL unchanged; no database has applied it from a published build. Follow-on edits from the renumber: - migration.rs: 55 embedded migrations, version 55 creates the personal read tables, and the schema-parity test reads the personal read surface from version 55 (it would otherwise have read main's index migration). - thread_window_postgres_tests.rs: the production migrator now ends at 55. - docs/buzz-v1-read-state.md: rollout names migration 0055. Co-authored-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
…i-port * origin/main: fix(agents): stop built-in prompts from teaching sleep polling (#7992) feat(relay): add direct staff ban/timeout/delete with staff guard (#7883) fix(ci): gate security review on repo write access (#7986) feat(acp): wrap workers at the subprocess launch boundary (#7985) feat(buzz-relay): idempotent owner community deletion with quota reservation (#7969) feat(mobile): show contextual names in lists, Search and Pulse (#7896) Add Kimi Code's default install path to managed-agent binary discovery (#5997) Co-authored-by: Will Pfleger <wpfleger@block.xyz> Signed-off-by: Will Pfleger <wpfleger@block.xyz>
Follow-up to #7830: reject owner-origin deletion admission for a legacy co-owned community before creating a request. Under the existing community-row lock, admission locks all current owner memberships in pubkey order and requires exactly one matching asserted owner. Existing-request idempotent replay remains unchanged; completion retains its independent post-lock revalidation. **New optional operator field: `community_id`** (`ab0cd713`, `766eb07f`). Owner-origin `POST /operator/communities/delete` now accepts an optional `community_id` that binds the request to the host's community: - **Fresh submission:** checked after sole-owner authority is proven, so a non-owner still gets `404 community_not_found`. A mismatch returns `409 community_id_mismatch` and writes no request row. - **Replay:** checked against the stored request, and only when the stored request is for the same host. A mismatch returns `409 community_id_mismatch`, and the stored request is unchanged. A known UUID sent with a different host goes through the existing convergence check and returns `409 deletion_request_conflict`. - **Omitted:** behaves as before. `admit_owner_request` takes `expected_community_id: Option<Uuid>` rather than a second public entry point, and existing callers pass `None`. - `docs/operator-community-deletion.md` documents the ordering. Also folds in the non-blocking review nits from #7969 (wpfleger96, at `e21151f4`): - **`limit_reached` has a stable `code`.** Create (`provision_community`) and transfer-in now return `409` with `code: "limit_reached"`, matching the other coded lifecycle errors. The `error` message keeps its `limit_reached:` prefix, so clients that match on the message still work. The provision limit test now asserts the code, and a new PostgreSQL test covers a transfer to an owner at the limit (coded 409, no membership change). - **Docs: an ack-version mismatch is a 400, not a 409.** `docs/operator-community-deletion.md` and the `delete_community` handler doc now say a changed host or owner under a known UUID is `409 deletion_request_conflict`, and an unsupported acknowledgement version is rejected first with `400 unsupported_acknowledgement_version`. - **Docs: active-cap overrides above the lifetime cap can't be reached.** The `max_communities_per_owner` doc and the quota section now say `MAX_LIFETIME_COMMUNITIES_PER_OWNER` (20) counts live ownership, so `BUZZ_MAX_COMMUNITIES_PER_OWNER` above 20 can't be reached. - **Legolas P2s:** - The UUID-conflict docs now list every 409 case, including a stored ack version that doesn't match and a UUID held by an operator-origin request. - The runbook now says a legacy co-owned community is rejected as `404 community_not_found` and should be converged with a transfer first. - The preparation drift test now pins `DeletionSafety` plus "sole-owner authority drifted". - **Guard wording:** the `delete_community` handler doc now says "sole current owner", matching the admission check. What got simpler: admission and automatic preparation now enforce the same sole-owner authority, so no accepted-but-unexecutable co-owned request needs a new recovery path. `deletion_api_error` is renamed to `coded_api_error`, since it now carries non-deletion codes too. There's one helper for coded operator errors instead of a second one. Rebased onto main after #7969 merged (`d7a35afa`). The sole-owner commit applied cleanly. Earlier focused Blox verification (at the pre-rebase head `161e9857`): the new admission regression was red before the change and green after it. Five temporary, restored mutants each failed its matching production-seam regression. At the current head `766eb07f`, the relay `api::operator::` suite passes 23/0 on Blox. Each new `community_id` guard, removed one at a time, fails its own regression test. CI owns the full suites. Remaining debt: the separate P2-A purge/completion membership/community lock-order cycle is unchanged. Generated with Codex --------- Signed-off-by: Codex <noreply@openai.com> Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Summary
This is the relay side of owner community deletion.
POST /operator/communities/deletechecks the request UUID first.202with the request's currentstatus. That holds at any stage, even after membership is purged, and no new work is admitted.409 deletion_request_conflict. An unsupported acknowledgement version is rejected first, with400 unsupported_acknowledgement_version.0054adds the supporting index. Migrations 0052 and 0053 are unchanged.limit_reached.acknowledgement_versionin the 202.quota_used,quota_limitandcan_create.can_createreflects both caps.limit_reachedis authoritative.docs/operator-community-deletion.md): the acknowledgement version is a compile-time constant. Admission and the executor's claim/lease both filter on it. A version bump must keep replaying and executing old-version requests.This follows on from #7830. The foundation PRs #7818, #7827 and #7830 are merged.
Testing
New tests:
owner_delete_resubmission_reports_current_status_without_new_intentsubmitted, including after membership purge.deletion_request_conflict.aborted.completed_owner_deletions_count_toward_lifetime_capcan_createis false.LimitReached.owner_quota_admits_only_under_active_and_lifetime_caps: a unit test.Fellowship gate: PASS at
59375c00. The code review was clean. The E2E run covered authenticated KGoose → relay → the real drain executor. The later commits only change the operator doc.Rollout
Complexity
Net simpler:
getare removed. Recovery reuses the existing idempotent admission path.OwnerQuota::admits()backs create, transfer and the projection.