TESTING.md now describes `invite_restricted` as a restriction on the
claiming account or the account that owns it, matching the relay and the
client copy from 9ae85092; issuer restrictions return `invite_invalid`.
The `materialize_nip_oa_owner` comment no longer describes the removed
`Result<bool, DbError>` return.
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Luke Tornquist <tornquist@squareup.com>
Main's #8605 took migrations 0072 and 0073 for the user deletion
target, so the invitation migration moves to 0074_invite_revocation.sql.
Upgrade fixtures now start from main's schema through 0073, the migrator
test expects 74 contiguous migrations with the invitation assertions at
index 73, and both TESTING.md files name 0074.
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Luke Tornquist <tornquist@squareup.com>
Main's #8579 took migration 0071 for the unlisted channel setting, so
the invitation migration moves to 0072_invite_revocation.sql. Upgrade
fixtures now start from main's schema through 0071, the migrator test
expects 72 contiguous migrations with the unlisted assertion at index 70
and the invitation assertions at index 71, and both TESTING.md files
name 0072.
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Luke Tornquist <tornquist@squareup.com>
Main added migrations 0069 and 0070 (entity deletion approval), so the
invitation schema moves to 0071 and its upgrade fixtures start from the
complete main schema through 0070.
Main's NIP-FI owner linking (#8401) replaced the boolean owner
materialization with OwnerLink { Linked, Conflict, Unavailable }, which
already separates a recorded-owner conflict from a storage failure. Invite
minting now maps those variants to 403 invite_owner_mismatch and 503, and
this branch's Result<bool, DbError> variant of materialize_nip_oa_owner is
dropped, along with the call-site adaptations it required in auth, audio
and bridge.
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Luke Tornquist <tornquist@squareup.com>
Preserve entity deletion migration 0068 and move the schema-only invitation expansion to 0069. Keep both migration inventory contracts and exercise populated invitation upgrades from the complete main schema through 0068.
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: seiler <seiler@squareup.com>
Keep main migration 0067 intact and move the unpublished invitation expansion to 0068. Exercise populated upgrades from complete main through 0067, retaining guest channel admission policy on both schema paths.
Signed-off-by: seiler <seiler@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
## Why
Early relay failures can currently appear as a container restart without
a trustworthy in-process account of whether crypto, structured logging,
configuration, relay identity, or the metrics listener failed. Most of
those steps happen before the Prometheus exporter exists, so their
chronology belongs in logs rather than metrics.
Implements the logs-only early-startup slice of #7238. Post-bind
Prometheus exporter supervision is tracked separately in #7284.
## What changed
- create a process lifecycle recorder before the Tokio runtime and emit
a fixed, versioned JSON schema directly to stderr;
- record started and exactly one terminal event for `crypto_init`,
`tracing_init`, `config_load`, `key_load`, `metrics_bind`, and the
aggregate `process_telemetry` phase;
- keep every status and reason bounded and suppress raw errors that
could contain credentials, keys, URLs, or other secrets;
- return typed metrics-install errors so `metrics_bind` can be
classified without logging raw values, while preserving the existing
public `metrics::install` API;
- document the logs-only evidence contract and add real child-process
regressions for success and failure paths.
This PR adds **no startup metric families** and no dashboard contract.
Existing application metrics remain unchanged.
## Verification
Exact head: `8faf7526822a119efa035e58b2b3c59aa67fc81d`
- `cargo fmt --all -- --check`
- `cargo clippy -p buzz-relay --all-targets -- -D warnings`
- relay binary target: 13 passed, 1 PostgreSQL-only test ignored
- real relay child-process lifecycle target: 9 passed
- full relay package library target: 1,023 passed, 89 ignored; the same
six media tests failed at `crates/buzz-relay/src/api/media.rs:1145` with
`Sqlx(PoolTimedOut)` because local PostgreSQL is unavailable
- three independent exact-head reviews found no correctness, security,
compatibility, lifecycle-accounting, logs-only-scope, or test-adequacy
finding
All exact-head GitHub CI gates are green, including lint, unit tests,
PostgreSQL, relay/backend/desktop integration, both Linux server
cross-compiles, Windows/macOS builds, and security checks.
## Staging verification
- exact multi-architecture image:
`dev-sha-8faf7526822a119efa035e58b2b3c59aa67fc81d-run-33708188952-1`
- immutable manifest:
`sha256:26cad28266a6bb0b0e7081eb6091d374e5489f8bb78c475a4a65737dee86cc67`
- image workflow: https://github.com/block/buzz/actions/runs/33708188952
- focused staging deployment:
https://github.com/squareup/builderbot-platform-core-infrastructure/pull/314
- replacement ReplicaSet `buzz-d68764bc7` has two Ready pods with zero
restarts
- Datadog received one complete, contiguous sequence 1-12 from each pod;
both end with `process_telemetry/terminal/succeeded` at 3 ms
- queries scoped to the replacement ReplicaSet return no data for the
removed `buzz_startup_phase_terminal` or
`buzz_startup_phase_duration_seconds` families
The experimental Row 7 was removed from the Buzz Startup & Rollout
Safety dashboard. This logs-only PR deliberately adds no replacement
dashboard row.
---
**Update Sep 3, 12:26 ET:** Clarified the review boundary: this PR does
not close the broader #7238. Later exporter-task termination is
pre-existing runtime behavior and is now explicitly tracked in #7284; no
production code or staged image changed in this update.
Generated with Codex
Signed-off-by: Ravneet Arora <rarora@squareup.com>
Mining the last 25 PRs' review threads (45 substantive findings, 11
reviewed PRs, avg **4.8 review rounds** each) shows **53% of findings
are repeats** of five clusters: swallowed failures, stale-async-state
races, tests that don't bind the production seam, unbounded
resources/retry loops, and non-atomic multi-step persistence. PR #6956
alone burned 4 rounds converging on one of these classes.
A second, independent mining pass over **71 agent-review rooms (303
findings, Aug 18–29)** confirmed the same clusters and added outcome
data — how often authors actually fix each finding class once flagged:
test-seam binding and unbounded-resource findings **100%**, swallowed
errors **90%**, stale-state races **70%**. It also surfaced two clusters
the GitHub-thread pass under-sampled: **assistive-semantics defects**
(44 findings, second-largest cluster) and **input-modality divergence**
(27 findings), now rules 7–8.
This PR distills those clusters into eight imperative rules in AGENTS.md
so agents apply them **before writing code**, adds one
client-consumption invariant to ARCHITECTURE.md §5, and places the
test-quality rule in TESTING.md (per the team decision that testing docs
are the canonical guide for review standards), cross-referenced from
AGENTS.md. Each rule cites the PRs where it was litigated. Raw mining
data: `reviews.jsonl` / `comments.jsonl` +
`backfill/buzz-review-findings.jsonl` (review-mining artifacts, not
committed).
No code changes. CLAUDE.md is a symlink to AGENTS.md and picks this up
automatically.
🤖 Drafted by Jude's agent from automated mining of this repo's last 25
PRs' review threads and 71 agent-review rooms; every rule cites the PRs
where it was litigated. Jude reviews and owns the result. Mining method
+ raw cluster data available on request.
---------
Signed-off-by: Jude Edwards <judeedwards@squareup.com>
Require `BUZZ_RELAY_PRIVATE_KEY` on every relay startup and remove the
shared public fallback key completely.
For local development, `just bootstrap` now generates a random key once
in the gitignored `.env` file. Re-running bootstrap preserves that key,
so relay-authored events retain the same identity across restarts. The
local relay recipes reload `.env`, while isolated CI and E2E launchers
pass explicit per-run keys.
Startup fails immediately when the key is missing or invalid, before
connecting to Postgres or Redis. Deployed relays continue to receive
their stable key from the existing chart-managed secret.
Validated with:
- `./scripts/test-ensure-local-relay-key.sh`
- `cargo test -p buzz-relay --bin buzz-relay`
- `cargo clippy -p buzz-relay --bin buzz-relay -- -D warnings`
- `cargo fmt --all -- --check`
- `shellcheck scripts/ensure-local-relay-key.sh
scripts/test-ensure-local-relay-key.sh`
Signed-off-by: Jordan Mecom <jm@squareup.com>
## Summary
- return complete channel rosters instead of truncating at 1,000 members
- chunk `event_mentions` inserts inside one transaction so large kind
`39002` snapshots remain discoverable by every `p` tag
- add a targeted `buzz-admin reconcile-channels --channel <uuid>`
force-republish path for stale discovery snapshots
- cover a 1,501-member roster, 11,000-tag mention index, and kind
`39002` tag construction past member 1,000
## Why
The relay builds NIP-29 discovery and several authorization decisions
from `get_members()`, but that helper silently returned only the first
1,000 active members. Desktop then counted the truncated kind `39002`
event, while late members could be rejected by roster-scanning member
actions.
Removing the roster cap exposes PostgreSQL's 65,535 bind-parameter
ceiling in mention indexing, so the insert is chunked transactionally to
preserve all-or-nothing indexing.
The existing reconcilers only fill missing discovery events. The
targeted admin option bypasses the separately known 1,000-channel
reconciliation-list ceiling and replaces an existing channel snapshot
using the configured production relay key.
## Attribution
This supersedes and builds on #3166 by @LordMelkor. Thank you for
identifying the roster boundary and contributing the original
complete-roster and mention-index patch. The production roster/query
changes and the two PostgreSQL regressions retain that work's shape;
this PR rebases it onto current `main`, adds relay coverage, and adds
the targeted repair operation requested for rollout.
## Validation
Exact pushed head: `24d02e4f3824150ed84913c9d230e675502e5b12`
- `cargo check -p buzz-db -p buzz-admin`
- `cargo test -p buzz-db
channel::tests::get_members_returns_full_roster_beyond_1000 -- --ignored
--exact --nocapture`
- `cargo test -p buzz-db
feed::tests::insert_mentions_indexes_rosters_past_bind_parameter_cap --
--ignored --exact --nocapture`
- `cargo test -p buzz-relay --lib
handlers::side_effects::tests::group_members_snapshot_keeps_members_past_one_thousand
-- --exact`
- `cargo run -q -p buzz-admin -- reconcile-channels --help`
- mandatory pre-push hook: branch-skew, desktop checks/typecheck/tests,
mobile tests, Rust tests, and desktop Tauri checks all passed on the
pushed head
## Rollout
1. Deploy the relay/backend build.
2. Run `buzz-admin reconcile-channels --channel <general-channel-uuid>`
with `BUZZ_RELAY_PRIVATE_KEY` configured.
3. Verify the replacement kind `39002` roster count matches the active
database membership count.
No schema migration or desktop release is required.
Fixes#3156
Supersedes #3166
---------
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Carl <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
This change requires a valid signed Blossom authorization request and
current relay membership for every media GET and HEAD request. It
removes the unauthenticated compatibility path and updates desktop reads
to send the required authorization.
This blocks anonymous retrieval and access after relay-membership
revocation. It does not yet bind a blob to its originating channel, so
someone removed from a private channel can still read a known blob while
remaining a relay member. That channel-ACL follow-up remains required
before closing the full finding.
## Testing
- `git diff --check origin/main...codex/security-media-read-auth`
- Rebased onto `origin/main` at `5c98932`
- Full CI pending
Originating Buzz thread:
`buzz://message?channel=3928fe05-df61-4b5d-b9c7-d623b9b10ea1&id=3c6c02312f763fbe0d2bfc33a6c1a362f91d0354f3d18b039cf7a0558c1439d1`
---------
Signed-off-by: Jordan Mecom <jm@squareup.com>
Signed-off-by: Alex Rosenzweig <arosenzweig@squareup.com>
Signed-off-by: Eli Foster <efoster@squareup.com>
Co-authored-by: Eli Foster <efoster@squareup.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
## Problem
On SIGTERM the relay sends every live WebSocket a **1012 Service
Restart** close frame via `ConnectionManager::drain_all()` — all in the
same instant (`main.rs` shutdown task → `state.rs::drain_all`). On a pod
holding thousands of sessions, that makes every client reconnect
simultaneously: the thundering-herd reconnect behind the DB pool-timeout
bursts observed on each rolling deploy. Client-side jitter can't fix
this — the desktop client *resets* its backoff to base on a 1012 and
reconnects with only ±25% jitter (`relayClientSession.ts`), so the
spread has to come from the server.
## Change
Add `BUZZ_DRAIN_JITTER_MS` (default `0` = unchanged behavior). The two
paths are kept **deliberately separate** so the default is byte-for-byte
the previously shipped shutdown:
- **Jitter off (`0`/unset, the default):** the original synchronous,
all-at-once `drain_all()` runs unchanged — queue the 1012 on each
connection's control channel, cancel, return. No new machinery on the
default path.
- **Jitter on (`> 0`):** a separate async
`drain_all_jittered(jitter_ms)` spreads each connection's restart close
over an independent uniform delay in **`[1, jitter_ms]`**. Each delayed
close travels a dedicated `RestartClose` channel; the writer flushes the
1012 frame and **acknowledges the flush over a oneshot**, so drain waits
for confirmed delivery (up to `RESTART_CLOSE_ACK_TIMEOUT` = 5s) rather
than assuming it, falling back to cancellation if the channel is
full/closed or the ack times out. The drain future is **owned and
awaited** by the shutdown task, and the 30s hard-drain backstop is
aborted only after a clean drain — so a clean roll exits `0`.
The two methods can be unified and the old one dropped later once the
jittered path is proven for all cases.
- **`config.rs`** — `drain_jitter_ms`: non-negative parse, clamped to
`MAX_DRAIN_JITTER_MS` = **20s** (leaving 10s of the 30s budget for
flush). Junk fails loudly at startup; **empty/whitespace-only is treated
as unset (jitter off)** so a `BUZZ_DRAIN_JITTER_MS=""` kill switch does
not crashloop the relay (matches the sibling env vars in this file).
- **`state.rs`** — `drain_all()` (unchanged synchronous default) +
`drain_all_jittered()` (jittered + flush-ack). Both set the sticky
`draining` flag before the first await. A registration that lands
mid-shutdown always self-signals via the **immediate** control-frame +
cancel path — jitter smears already-established sockets, not late
arrivals.
- **`main.rs`** — shutdown task dispatches: `drain_jitter_ms == 0` →
`drain_all()`, else `drain_all_jittered(...).await`.
## Safety
- **Default off is the currently-committed path.** With jitter unset/0
the shutdown runs the original synchronous `drain_all()` — no restart
channel, no ack wait. Safe to deploy dark and dial up.
- **Shutdown-boundary race preserved.** Sticky flag set before any
await; a late registration self-signals its close with no jitter.
- **Owned + backstopped.** The jittered drain future is awaited; the 30s
hard-drain `process::exit(1)` remains the ceiling. `MAX_DRAIN_JITTER_MS`
(20s) + `RESTART_CLOSE_ACK_TIMEOUT` (5s) = 25s, inside the 30s budget;
5s pre-sleep + 25s = 30s against `terminationGracePeriodSeconds: 60`.
## Known behavior to note (not a blocker, flagged from review)
On a **successful** flush the jittered path deliberately does not cancel
the connection token — teardown then depends on the client echoing our
Close, or on process exit. Compliant clients echo; a silent client rides
to the 30s hard exit. The default (jitter-off) path cancels
deterministically as before.
## Tests
- `config::tests::drain_jitter_defaults_off_and_rejects_junk` — default
off, `20000`, clamp `60000`→`20000`, explicit `0`, junk `"soon"` fails,
**empty `""` and whitespace-only treated as off**.
- `state::tests::drain_all_is_immediate` — default path queues frame +
cancels synchronously.
- `state::tests::drain_all_sends_restart_close_and_cancels_every_conn`,
`drain_all_full_control_buffer_still_cancels`,
`register_after_drain_self_signals_restart_close_and_cancel`.
-
`state::tests::drain_all_jittered_defers_close_until_within_jitter_window`
(paused time).
-
`state::tests::drain_all_jittered_waits_for_writer_acknowledgement_without_cancelling`.
-
`state::tests::drain_all_jittered_cancels_when_restart_channel_is_full_or_closed`.
- `state::tests::drain_all_jittered_cancels_when_flush_ack_times_out`
(paused time — the 5s ack-timeout fallback).
Validation at `46c690940`: `cargo fmt -p buzz-relay --check`, `cargo
clippy -p buzz-relay --all-targets -- -D warnings`, and the drain/config
unit suite all clean. Local live SIGTERM test with a real relay process
+ 200 NIP-42-authenticated sockets — see the PR comment for the
before/after distribution and exit codes.
## Rollout
Ship with default `0`, then set `BUZZ_DRAIN_JITTER_MS` (e.g.
10000–20000) on bb-block first, watch the roll-window pool-timeout
metric, then bb-public. `""` is a safe kill switch. Complements the
preStop `sleep` (stops routing before close).
---------
Signed-off-by: npub1srl70fhzyu3fsnahl06vw2czvqc2w3ds37hyzvjnk8ve8f03ngcqg9le2w <80ffe7a6e22722984fb7fbf4c72b026030a745b08fae413253b1d993a5f19a30@buzz.block.builderlab.xyz>
Signed-off-by: npub128x7j3pwgm4vs8yra3c42fcgcwcvh94g3luwzkqa376du2q6l0esqcrwch <51cde9442e46eac81c83ec71552708c3b0cb96a88ff8e1581d8fb4de281afbf3@buzz.block.builderlab.xyz>
Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: npub1srl70fhzyu3fsnahl06vw2czvqc2w3ds37hyzvjnk8ve8f03ngcqg9le2w <80ffe7a6e22722984fb7fbf4c72b026030a745b08fae413253b1d993a5f19a30@buzz.block.builderlab.xyz>
Co-authored-by: npub128x7j3pwgm4vs8yra3c42fcgcwcvh94g3luwzkqa376du2q6l0esqcrwch <51cde9442e46eac81c83ec71552708c3b0cb96a88ff8e1581d8fb4de281afbf3@buzz.block.builderlab.xyz>
