feat(relay): deliver pubkey mentions to relay companions - #7793
Conversation
🔐 Codex Security Review
|
4eacb6c to
43597fb
Compare
|
I reviewed this at exact head P1: deterministic HTTP failures consume the full retry budgetIn Could we classify outcomes so transport errors, P1: default redirect following can silently complete delivery at the wrong endpointThe reqwest client at Could we construct the client with Architecture question (not classified as a defect)Is deployment-global listener registration—and therefore release of mention metadata across community boundaries—an intentional operator-trust exception? Migration 0049 explicitly says registrations span communities, while the isolated-community direction in |
8bd65ce to
291c1cd
Compare
|
@TheSentinel454 fixed "deterministic HTTP failures consume the full retry budget" and "default redirect following can silently complete delivery at the wrong endpoint". Regarding "Architecture question" yes this is deliberately operator level as our service needs to be pushed information without knowing all the communities that the agents may be in. Therefore we cannot register with community fencing upfront. It's not a subscription registration in a normal user sense. |
291c1cd to
501f93e
Compare
c6c9ee0 to
398999b
Compare
398999b to
1416c87
Compare
baxen
left a comment
There was a problem hiding this comment.
🤖 Approving. The core design holds up: mentions are queued in the same transaction as the event insert, only for new events, on both insert paths; leases and claim tokens fence out stale workers; and migration 0050 updates the fence-exclusion function before it creates the tables. I also ran the Postgres suites that the testing section skips, at 1416c871: buzz-db passed 306/306, including the 7 new operator_listener tests, migration parity and the tenant fence, and the buzz-relay operator routes passed 16/16.
None of the following blocks the merge. They're follow-ups, in a later commit or a separate PR, whichever you prefer:
1. Open the transaction only for mention kinds (store/event.rs:352)
insert_event now opens a transaction for every write. That adds BEGIN/COMMIT round trips to side-effect, git, audio and workflow events, and none of them can ever queue a mention. Check is_listener_mention_kind first and keep the plain path for everything else.
2. Claim only rows this pod can route (claim_deliveries, store/operator_listener.rs:202)
Pass the configured listener pubkeys into the claim and add AND listener_pubkey = ANY($n). That lets you delete release_unroutable_delivery, the release path in the worker and their tests. It also fixes a real problem: when a listener is removed from config, its registrations keep producing rows for up to 30 days, and every 30s those rows are claimed, released and logged as warnings until they age out.
3. Fail fast on a bad BUZZ_OPERATOR_LISTENERS (config.rs:857)
A malformed value currently logs an error and starts the relay with the feature disabled. Registrations return 403 and delivery silently stops, and the only sign is a single log line at startup. Other operator-identity config returns ConfigError and stops startup, including RELAY_OWNER_PUBKEY, RELAY_OPERATOR_PUBKEYS and BUZZ_PUSH_GATEWAY_DELIVERY_URL, for the reason given in the comment on RELAY_OPERATOR_PUBKEYS. Making this ? would match them. It also lets you revert the log-capture test helper refactor, since the tests can simply assert is_err().
4. Drop the DeliveryStore / DeliveryTransport traits (operator_listener.rs:97-120)
Each trait has one production implementation, so the indirection only exists for tests, along with the mocks and the hand-rolled HTTP request parser. push_runtime.rs tests the same kind of signed outbound POST against a real local axum server. Doing the same here removes about 200 lines and tests the real reqwest and NIP-98 signing path, not a mock of it.
## Summary - Keep non-listener-mention event writes on the plain insert path without opening a transaction. - Preserve the transaction around listener mention event insertion and outbox enqueue. ## Validation - `cargo fmt --all -- --check` (Hermit) - Tests not run. Follow-up to Baxen’s review of #7793, point 1. Signed-off-by: Implementor <691cca7a870db1dad6990d5938d1b4a2a2ca9647ca81290b536c17b06b2e473f@buzz.block.builderlab.xyz> Co-authored-by: Implementor <691cca7a870db1dad6990d5938d1b4a2a2ca9647ca81290b536c17b06b2e473f@buzz.block.builderlab.xyz>
Addresses point 3 of [previous comment](#7793 (review)) "3. Fail fast on a bad BUZZ_OPERATOR_LISTENERS (config.rs:857)" ## Summary - Propagate malformed `BUZZ_OPERATOR_LISTENERS` values from `Config::from_env` as `ConfigError`, including invalid UTF-8. - Update startup config tests to assert returned errors and restore the original admin warning capture helper, removing the shared helper introduced only for listener log assertions. - Cover valid and absent listener values through `Config::from_env`. ## Context Follow-up to the review of #7793 specifically [reverting some config logging parts](https://github.com/block/buzz/pull/7793/changes#diff-eaa0eeb209ff1ad6212ffa56d3a983bb732858647c9fb227ef7483a3363b5a71). Invalid listener configuration currently logs an error and silently disables listener delivery. ## Verification - `cargo fmt --all -- --check` passed with Hermit. Signed-off-by: Implementor <691cca7a870db1dad6990d5938d1b4a2a2ca9647ca81290b536c17b06b2e473f@buzz.block.builderlab.xyz> Co-authored-by: Implementor <691cca7a870db1dad6990d5938d1b4a2a2ca9647ca81290b536c17b06b2e473f@buzz.block.builderlab.xyz>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> * origin/main: fix(relay): fail startup on invalid operator listener config (#7933) fix(db): limit event transactions to listener mention kinds (#7932) feat(relay): deliver pubkey mentions to relay companions (#7793) docs(protocol): propose simplified channel artifacts (#7791) feat(push): support configurable HTTP(S) delivery URLs (#7877) fix(ci): select runtime suites from PR changes only (#7843) test(desktop): synchronize upload edit smoke test (#7903) Signed-off-by: Hayt <211b96e6a2b7f45fd4047988976c7bbbeeda0c15f3ae7b32eec20834b5a55118@buzz.block.builderlab.xyz>
* origin/main: 🤖 docs(nip-fi): remove implementation references from the spec (#7912) fix(relay): fail startup on invalid operator listener config (#7933) fix(db): limit event transactions to listener mention kinds (#7932) feat(relay): deliver pubkey mentions to relay companions (#7793) docs(protocol): propose simplified channel artifacts (#7791) feat(push): support configurable HTTP(S) delivery URLs (#7877) fix(ci): select runtime suites from PR changes only (#7843) test(desktop): synchronize upload edit smoke test (#7903) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
We are introducing a companion service to the relay that needs pubkey event mentions for pubkeys it manages. This PR introduces an outbox for pubkey mentions that will be sent to these "operator listener" services.
Summary
Deliver mentions of registered pubkeys to deployment-global operator listeners.
BUZZ_OPERATOR_LISTENERS; listeners manage target pubkeys through NIP-98-authenticatedPOST/DELETE /operator/listener/pubkeysrequests.ptags to registrations and insert one outbox row per listener in the event transaction.Changed behavior
Listener outbox rows are enqueued inside the shared event-insert and thread-metadata transactions; enqueue failure rolls back the event.
event_mentionsbehavior is unchanged: ordinary event/reaction mention indexing remains post-commit best effort with warning-only failures.Testing
cargo fmt --allcargo test -p buzz-db --libcargo test -p buzz-relay --lib config::tests::operator_listener_routescargo check -p buzz-db -p buzz-relay