feat(nip-fi): admin disconnect/deny API with in-memory deny-until-TTL map (S4)

[wpfleger96]

Implements S4 of the buzz-enterprise-identity program: the NIP-FI admin disconnect API with deny-until-TTL semantics.

Merge order: This branch carries a pre-#7224 S3 snapshot (merge-base 88687876f) and merges after #7224; base reconciliation follows.

What this adds

buzz-auth

crates/buzz-auth/src/nip_fi/deny_map.rs — NipFiDenyMap

In-memory deny set, no persistence (Option B, relay restart amnesia is accepted and documented as issuer-re-push).

• Per-issuer DashMap shards — cross-issuer capacity starvation is impossible
• Merge rule: max(existing_until, incoming_until) on same-key collision — a delayed shorter-until command never shortens an active deny [FI-TRACE-DENY-SET]
• Past-until commands: never create or shorten entries; atomically calls the session-close path but skips the deny-entry write [FI-TRACE-DENY-SET]
• Per-issuer hard capacity cap, fail-closed (DenySetFull) — spec requires 503 and the jti is not consumed on full [VerifyCommandJwt step 7]
• Self-evicting TTL (lazy eviction on read and write)
• Atomic jti reservation + deny-entry insertion in one shard lock (both-or-neither) [VerifyCommandJwt step 7]
• is_denied(issuer, pubkey, now) — the clean interface S5 (HTTP enforcement) consumes

crates/buzz-auth/src/nip_fi/command.rs — CommandVerifier<S>

Verifies typ=nip-fi-command+jwt tokens against the same issuer JWKS as assertions.

• Validates method, path, target_pubkey, aud, iat, exp, jti, and body hash claims
• CommandIssuerPolicy carries maximum_command_age_seconds (normative ≤ 60), authorized_principals (sub allowlist), and per-issuer deny_set_capacity
• jti reserved at the final admission step — after authz check + body-match — closing the DoS seam where early reservation would let a crafted rejected command consume jti slots [VerifyCommandJwt step 7]
• Reuses the same parsing/validation primitives as the assertion verifier via pub(super) helper promotion — no crypto duplication

buzz-relay

crates/buzz-relay/src/api/nip_fi.rs — POST /api/nip-fi/disconnect

• Extracts command JWT from Nostr-Federated-Identity: Bearer <token> header
• Validates JSON body pubkey (lowercase hex, exactly 32 bytes)
• Delegates to CommandVerifier::verify — jti + deny entry written atomically on success
• Closes all live sessions for the target pubkey via ConnectionManager::disconnect_nip_fi
• Response contract: 200 {"disconnected": true} on success; 400/401/403/503 per the spec rejection table
• Endpoint is not a protected HTTP surface — no NIP-98, no NIP-FI assertion; auth is entirely inside CommandVerifier::verify
• Cross-pod propagation failures increment buzz_nip_fi_disconnect_propagation_failures_total (no iss/pubkey in labels [FI-TRACE-PRIVACY-NONPUBLIC])
• build_nip_fi_command_components — startup builder callable from main.rs

crates/buzz-relay/src/state.rs

• ConnectionManager::disconnect_nip_fi — issuer-global (unfenced) cross-community session close, sends NOTICE before cancel; sets AuthorizationDenied via first-writer-wins publish_disconnect_reason (writes only when slot is None) so the send loop's cancel branch emits a 1008 POLICY close frame with reason "authorization denied" [spec: deny applies across all communities under the issuer]
• CommunityConnectionControl::publish_disconnect_reason — atomic first-writer-wins helper; all writers (disconnect_community, disconnect_nip_fi, the expiry task, key-pairing) route through it to prevent concurrent CommunityDeleted / AuthorizationDenied causes from misattributing the close frame
• AppState::nip_fi_deny_map and nip_fi_command_verifier fields — None-initialized; endpoint returns 503 until startup wires them in

crates/buzz-relay/src/router.rs + src/api/mod.rs

• Route wired: POST /api/nip-fi/disconnect sits outside all NIP-98 middleware layers

crates/buzz-relay/src/handlers/auth.rs + audio/handler.rs

• is_denied checked at WS admission after pubkey registration (spec steps 5+6 ordering): root WS (handlers/auth.rs:346), audio (audio/handler.rs:349), and the pre-upgrade early-bounce path — authorization_denied frame + explicit 1008 POLICY close frame + close on match; NIP-FI expiry sends the same policy close on all paths including the audio pre-send-loop window (check_cancel!() arms and JoinCommitError::Expired exit)

crates/buzz-relay/src/main.rs

• install_nip_fi_command_components wired at startup (main.rs:543) with shared-Arc JWKS source

Test coverage

20 new tests in nip_fi::deny_map and 26 new tests in nip_fi::command:

• Both command delivery orders → max(until) [FI-TRACE-DENY-SET oracle]
• Merge rule both directions
• Past-until commands: absent entry inserts expired; active entry left unchanged
• Capacity exhaustion: fail-closed, nothing inserted, jti not consumed
• jti replay rejection
• Cross-issuer capacity isolation
• CommandIssuerPolicy construction validation (zero age, >60 age, empty principals, zero capacity, empty issuer)

22 unit tests in api::nip_fi: header extraction contract (401/403), pubkey parsing (uppercase rejected, wrong length). 4 additional ConnectionManager/CommunityConnectionControl tests: conn_manager_disconnect_nip_fi_sets_authorization_denied_reason, conn_manager_disconnect_nip_fi_ignores_unproven_connection, community_disconnect_then_nip_fi_keeps_community_deleted_reason (first-writer-wins: CommunityDeleted not clobbered), and nip_fi_disconnect_then_community_keeps_authorization_denied_reason (first-writer-wins: AuthorizationDenied not clobbered). W_FIX1: pre-send-loop drain emits restricted JSON then 1008 POLICY close.

All spec oracle cases are falsifiable: a mutation that violates the merge rule, the past-until invariant, or the capacity semantics will break the corresponding test.

[wesbillman]

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

Changes requested

Reviewed head 2a42ddff049aaaf4bcfdf994680c0b1b4e2785d7 against base ac5a18697c8294e9237f505a2e01ec6fc374849a, source-only. No PR code was checked out, built, tested, or executed.

The review preserves the accepted RAM-only/restart-amnesia design. Startup wiring, WS admission deny checks, and S5 HTTP enforcement are explicitly deferred, not blockers by themselves. The issues below are in the delivered callable components; main.rs currently leaves the endpoint unconfigured.

P2: Close existing huddle sockets as well as Nostr sockets

Handler close call / new close scan.

The success path scans only ConnectionManager. Huddle audio sockets instead register with community_connections, have their own cancellation token, and prove their key through a separate NIP-42 handshake (audio handler, proof). They never enter this scan.

With a configured verifier, a target already connected to chat and a huddle receives a successful disconnect, loses the chat socket, but retains the independently authenticated audio socket and its audio access. NIP-FI's delivered disconnect contract closes all live WebSockets whose proven key matches (spec). This is not the deferred new-admission check. Include every existing key-proven socket type in targeted cancellation, and cover a target with simultaneous Nostr/audio sockets plus an unaffected other key.

P2: Preserve fractional NumericDate deadlines

command.rs:448–466.

The new parser floors every fractional NumericDate and reconstructs it with zero nanoseconds. For an otherwise valid signed command with until = T + 0.9, the stored deadline becomes T, so the deny map reports the key allowed at T + 0.1, before the issuer's signed deadline. Flooring iat can also admit evidence beyond the allowed future-skew boundary. The existing assertion parser already preserves nanoseconds (verifier.rs:916–952). Reuse that semantics and add signed-command tests through verify_at, asserting denial just before fractional until and expiry at equality, plus future-iat and ceiling boundaries.

P2: Keep issuer identities out of logs

api/nip_fi.rs:144–149 and configuration warnings.

A successful close emits the exact signed iss as caller_iss when debug logging is enabled; invalid configuration emits the issuer URI at warning level. This exports deployment-private identity coordinates to logs, contrary to the explicit NIP-FI privacy contract. Remove the issuer fields, retain fixed reason/count diagnostics, and bind a privacy regression to the actual success/configuration logging paths.

P2: Bound the replay map independently of deny-entry capacity

deny_map.rs:98–126.

Even with issuer deny capacity 1, repeated valid commands updating one active key bypass the entry-capacity guard and retain a new jti string each time. Every mutation then scans that growing replay map under the shard lock. There is no replay-count/byte budget or command rate bound in this path. A buggy or abusive authorized issuer can exceed its intended memory budget and increase lock-held work; this is not an anonymous attack or a measured outage. Large JTIs are bounded only by the total 64-KiB token limit, and accepted clock skew can extend retention to 360 seconds.

Add an explicit per-issuer replay-resource bound checked in the same atomic admission step. Exhaustion must leave both the new deny mutation and JTI reservation unapplied, without evicting unexpired replay identities. Test repeated same-key updates at the budget, concurrent reservations, and reuse of a rejected still-valid JTI after capacity frees.

Validation and smaller compatibility note

The changed tests exercise policy constructors, error helpers, header/pubkey parsing, and sequential deny-map operations. They do not exercise the new command verification or disconnect handler end to end; the referenced command/tests.rs is absent from the complete pinned head tree. The regression cases above need the production seams, not copies of predicates or response constants.

Non-blocking: missing-header 401 is missing WWW-Authenticate: Nostr required by the rejection table; plain_response only adds Content-Type. Existing DenialClass already provides the challenge value.

[wesbillman]

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

Changes requested

Reviewed head f03efb3f472600ba9ab2061a5136d22277a9fbe3 against base ac5a18697c8294e9237f505a2e01ec6fc374849a, source-only. No checkout, build, tests, imports, or PR-code execution. All three independent review lanes returned; findings below were independently reconciled against the source.

The original audio-scan omission, fractional NumericDate parser, explicit issuer-log sites, and unbounded replay-map implementation are corrected. The current source also delivers startup and cross-pod wiring despite the stale PR description. Three defects remain in that delivered integration.

P2: Do not turn remote capacity pressure into permanent issuer-wide denial

deny_map.rs:277–290,341–367, called by main.rs:1215–1258.

Source-derived witness: configure two maps/pods with issuer capacity 1. Before propagation, each accepts a command for a different key. Deliver the remote messages: each new-key merge hits capacity and inserts the issuer into blocked_issuers. Every subsequent is_denied call for that issuer returns true, including unrelated keys and times after both signed TTLs have expired. No expiry or recovery clears this flag; restart is required. A delayed, already-expired remote command against a full shard takes the same path.

This breaks the delivered shared deny interface’s targeted, self-expiring contract. It is not a claim of an already-wired admission outage: WS/S5 admission consumers remain deferred. The spec explicitly permits asynchronous propagation loss with issuer re-push; it does not require permanent denial of unrelated users. Remove the sticky issuer-wide transition on ordinary capacity exhaustion, retain existing live entries and observable recovery, and preserve target session closure for past-until delivery without creating future denial. Do not add persistence. Replace the test that codifies deny-all (deny_map.rs:831–879) with the two-map/capacity, unrelated-key, and post-TTL cases. Poisoned-lock handling is separate from normal capacity pressure.

P2: Sanitize configuration parse errors before they enter logs

nip_fi_config.rs:159–162 forwards the raw serde error into ConfigError; main.rs:163–165 logs that error.

An otherwise complete issuer entry containing "authorized_principals": "admin@private.example" instead of an array produces a type-error diagnostic containing the supplied principal string. This happens before the new index-only policy validation. Startup fails closed, but private principal/email data is exported to logs, violating NIP-FI’s explicit privacy contract. Keep a fixed error category and safe location information rather than {e}. Add a malformed-sensitive-value regression through the actual configuration/error-reporting path. The old explicit issuer log sites are fixed; this is a newly introduced path.

P2: Isolate the new route fixtures from process-environment mutation

api/nip_fi.rs:634–640 and the absent-verifier fixture at line 742 call Config::from_env().expect(...). That now reads NIP-FI environment configuration. In the same test binary, nip_fi_config.rs:399–462 sets the mode to permissive, or to enforce without issuers, under a mutex private to that module.

If a route fixture reads during either interval, configuration returns an error and the fixture panics before reaching the route under test. The private mutex does not protect those readers. Construct fixture configuration without process-env reads, or coordinate every relevant reader/writer using shared synchronization. Do not rely on serial execution of this test module. This is a source-derived parallel-test failure, not a claimed local reproduction.

Regression coverage and stable exit criteria

Keep the real signed-verifier and real-router tests; they are a substantial improvement. Finish the already-requested corrective witnesses: signed verify_at fractional until just-before/equality, future-iat and ceiling boundaries; an actual audio-handler registration plus target chat/audio and unaffected-peer disconnect; and privacy-path capture. Existing registry tests manually populate the key, so removing audio/handler.rs:252 survives them. Integer-only command fixtures do not catch reintroducing timestamp flooring. The “production assembly” test calls only the component builder; its comment at api/nip_fi.rs:990–995 incorrectly claims deleting main’s state assignments makes it fail. Correct that claim or bind the actual assembly seam. These are source-inspected coverage limits, not test-run results.

Exit criteria are the three bounded defects above and regression witnesses for the corrective paths. Preserve RAM-only/restart amnesia, asynchronous propagation/re-push, and the separately deferred WS admission/S5 HTTP work. No broader session-lifecycle rewrite, distributed completion guarantee, or persistence requirement is added. The missing-header WWW-Authenticate: Nostr note is also resolved.

[wesbillman]

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

Changes requested

Reviewed head b6f5ba30de70c06f581fc9201c090726210d9679 against base ac5a18697c8294e9237f505a2e01ec6fc374849a, focused on the two corrective commits since f03efb3f472600ba9ab2061a5136d22277a9fbe3 and the prior exit criteria. Source/metadata review only: no checkout, build, tests, imports, or execution of PR code.

The same three P2 findings remain. Moving the blocked flag under the shard mutex improves serialization, but preserves the incorrect permanent issuer-wide denial. The new publisher/consumer and startup seams improve testability; they do not close the policy/privacy/fixture findings below.

1. P2: Ordinary remote capacity exhaustion still permanently denies unrelated keys

deny_map.rs:401–409, is_denied:316–327, consumer:317–343

Source-derived witness: two pods each have issuer capacity 1 and independently accept different target keys before propagation. Deliver the remote entries: the new-key capacity miss sets IssuerShard.blocked = true. is_denied_or_blocked returns that bit for every key (deny_map.rs:115–119), and TTL eviction never clears it (101–105). Both unrelated keys and the missed target remain denied even after all signed until values expire. A delayed, already-expired remote entry against a full shard causes the same transition.

This is the same targeted/self-expiring shared-interface defect as before, not a claimed live outage in the deferred WS/S5 admission consumers. The contract permits asynchronous loss and issuer re-push; it does not authorize permanent denial of unrelated users. Holding the flag under a mutex makes the wrong state transition atomic, not correct.

On ordinary CapacityExceeded, retain existing live entries, return the capacity outcome, close only the delivered target’s sessions through the consumer, and leave recovery to the accepted re-push policy. Remove the sticky issuer-wide transition; poisoned-lock handling can remain separately fail-closed. Replace the tests that require unrelated-key denial (deny_map.rs:889–938,1099–1141; api/nip_fi.rs:1241–1354) with two-map capacity, unrelated-key, and post-TTL cases. No persistence or distributed completion guarantee is requested.

2. P2: Configuration parsing still exports private principal values to startup logs

nip_fi_config.rs:159–162, main.rs:163–165

An otherwise complete issuer entry with "authorized_principals": "admin@private.example" instead of an array still produces a serde type error containing the supplied string. The parser interpolates that raw error into ConfigError; startup logs it verbatim. This happens before the new index-only missing-command-field validation. Startup rejects the configuration, but leaks the principal/email in doing so, contrary to the explicit privacy contract.

Keep a fixed error category and safe location information instead of raw {e}. Add a malformed-sensitive-value regression through the actual configuration/error-reporting path. The new missing-command-fields test does not cover this failure mode.

3. P2: Existing and new route fixtures still race process-environment writers

api/nip_fi.rs:895–902, new consumer fixture:1261, nip_fi_config.rs:459–477

The route fixtures still call Config::from_env().expect(...); the corrective commits add more copies at api/nip_fi.rs:1261,1378,1513 alongside 899,1003. That loader reads NIP-FI config (config.rs:1266). In the same test binary, NIP-FI tests set BUZZ_NIP_FI_MODE=permissive, or Enforce with issuers absent, under a mutex private to that module (nip_fi_config.rs:413). The route readers do not take it. A concurrent read returns a configuration error and panics before the intended route/consumer/startup assertion.

Construct fixture configuration without process-environment reads, or coordinate all relevant readers/writers with shared synchronization. Overriding DB/Redis fields after loading does not fix the fallible ambient read. This is a source-derived parallel-test failure, not a claimed local reproduction.

Corrective coverage and scope

The new fractional_deadline_survives_publisher_wire_and_consumer_equality_boundary test (api/nip_fi.rs:1371–1490) exercises the real publisher mapping, encode/decode, apply helper and map equality comparison. Credit that wire-path regression. It starts from a synthetic CommandResult, so it does not close the previously requested signed fractional-NumericDate verify_at boundary witness. Integer future-iat and until ceiling cases already exist; the remaining gap is fractional parsing and boundary preservation on the signed path. command.rs is unchanged from the previous reviewed head.

The replacement installer test (api/nip_fi.rs:1508–1613) now invokes the production helper that owns both AppState assignments and verifies that command verification writes to the same map. The former builder-only assembly criticism is resolved. It seeds the key snapshot and checks the warmup result; the final shutdown store is not an observed refresh-task exit, so do not claim that lifecycle was tested.

The actual audio-handler registration plus targeted chat/audio and unaffected-peer witness remains uncovered in the inspected tests. The registry tests manually set the proven key (state.rs:2650–2742), so they bypass the production registration at audio/handler.rs:250–252; the new consumer tests assert map state without live sessions on both transports. Finish the previously requested registration-to-dual-transport-close witness. This is a carried-forward coverage gap, not a new production regression. All three independent lanes have returned and been reconciled. No runtime or mutation-test results are claimed here.

Stable exit criteria remain these three bounded defects and the previously requested corrective witnesses. Preserve RAM-only restart amnesia, asynchronous propagation/issuer re-push, and separately deferred WS admission/S5 HTTP. No broad session-lifecycle rewrite or additional persistence requirement is added. Existing HTTP status/body classes, pubkey-targeted close behavior, and command/JWKS verification contracts were traced through the changed wiring; unchanged cryptographic internals were not reopened as a fresh audit.

addressed

[wesbillman]

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

Changes requested

Reviewed head 5e41fffaf995d53b6eac7aaa5a440f425be7f778 against exact base 3c7f288c60d67df78577b237e27c3dfc8831aaa1, using the previous six-finding review as the corrective contract. Source-only: no checkout, build, test, import, runtime reproduction, or mutation execution.

Credit: root pairing, post-registration deny-set, and admin-disconnect producers now use the shared winner-only transition and reserved terminal sender. This fixes the prior ordinary-queue saturation and pairing/post-auth versus community-delete schedules. Five prior findings remain unchanged; F2 is partially fixed with a reverse-order race in the newly added lifecycle helper. The corrective tests also introduce a parallel-run hang described separately below.

F1 · P1 · Observer EVENT still bypasses session authority

crates/buzz-relay/src/handlers/event.rs:682–693, crates/buzz-relay/src/handlers/event.rs:990–1138.

Kind 24200 returns through handle_agent_observer_event before either EVENT permit. Its complete helper awaits signature verification and potentially owner lookup, then publishes to Redis and local subscribers without acquiring a session effect permit. Source-derived schedule: hold validation/owner lookup for a valid owner-to-agent control frame, expire or admin-disconnect the publisher, then resume. The already-spawned task can still deliver that control to a live agent. The dispatch-time cancellation check cannot fence this resumed task, and recipient access checks do not validate publisher authority.

Acquire the existing permit after validation and before publication, hold it through fan-out, and add a production barrier witness for cancellation before acquisition. Preserve already-permitted bounded effects.

F2 · P2 · Finish terminal ordering in the new lifecycle cancellation helper

crates/buzz-relay/src/state.rs:347–357, crates/buzz-relay/src/state.rs:279–305, crates/buzz-relay/src/connection.rs:560–587.

The root producers requested previously are wired correctly now, but lifecycle_cancel releases the transition lock before cancelling and leaves the reason unset. Concrete reverse-order schedule: lifecycle cancellation acquires/releases the lock and pauses at line 356; a manager denial then acquires it, wins AuthorizationDenied, and pauses before try_send; lifecycle resumes at 357 and wakes the send loop. It drains an empty terminal queue and emits a 1008 authorization-denied close before the NOTICE is enqueued. No stalled writer is required. Heartbeat/backpressure/teardown now call this helper in production.

Serialize lifecycle cancellation itself with terminal publication and prevent a later writer from installing a new cause after lifecycle cancellation has already won. This can be completed with the existing lock/token rather than another queue abstraction. The new lifecycle/manager witnesses force denial-first ordering (crates/buzz-relay/src/state.rs:4408–4469); add the reverse order and verify the actual writer observes one consistent terminal result.

F3 · P2 · Transactional JOIN still drops the lifecycle generation

crates/buzz-relay/src/audio/handler.rs:1159–1172, crates/buzz-relay/src/audio/handler.rs:2245–2265.

The handler computes lifecycle_generation but does not pass it into commit_participant_join; the persisted/fanned-out kind 48101 content still omits it while LEFT/ENDED include it. An already-hydrated Desktop observer receiving a new START/JOIN assigns "pending" (desktop/src/features/huddle/lib/huddlePresenceRuntime.ts:264–272). The next authoritative liveness response has the real generation, so reconciliation clears admissions (desktop/src/features/huddle/lib/huddlePresence.ts:464–483). In-huddle indicators disappear while audio remains connected. This affects Off as well as Enforce.

Pass the existing generation through the transaction-owned JOIN builder and test the producer-to-JOIN-to-liveness sequence, including Off mode.

F4 · P2 · Audio deadline rejection still cancels its denial producer

crates/buzz-relay/src/audio/handler.rs:918–938, crates/buzz-relay/src/audio/handler.rs:1220–1246, crates/buzz-relay/src/nip_fi_session.rs:200–228.

acquire_effect can observe an elapsed wall-clock deadline before the expiry task runs. Both audio rejection paths still call raw cancel.cancel() before awaiting that task. Its unbiased select can take the now-ready cancellation arm and return without publishing expiry_deny_terminal. The handler then drains no restricted JSON, finds no disconnect reason, and returns without an explicit 1008 policy close. Awaiting task completion does not prove denial publication. The new lifecycle wiring does not change these rejection branches.

Publish the serialized deadline cause before self-cancellation, respecting an already-winning cause. Add delayed-expiry-task wire witnesses at add-peer and commit rejection. Admission is denied already; this is a denial-delivery defect, not an authorization bypass.

F5 · P2 · Admin cancellation still skips the root quiescence barrier

crates/buzz-relay/src/nip_fi_session.rs:227, crates/buzz-relay/src/connection.rs:457–479, crates/buzz-relay/src/handlers/req.rs:307–351.

Only the timer arm waits for existing effect permits. With a future deadline, admin disconnect wakes the cancellation arm, which returns immediately; root cleanup awaits it and removes subscriptions without joining detached REQ handlers. Hold a REQ after permit acquisition but before registration, let admin cancellation finish cleanup, then resume it. Registration recreates the removed connection entry (crates/buzz-relay/src/subscription.rs:150–155) and retains topics after the only cleanup pass. Repeating this schedule accumulates orphan registry entries/topic references.

Keep socket cancellation immediate, but wait for already-permitted bounded effects before resource removal on admin cancellation, without producing a second terminal frame. Add a held-permit/admin-cancel witness proving cleanup follows the effect and leaves no subscription/topic state.

F6 · P2 · Ambient NIP-FI fixture races remain

crates/buzz-relay/src/router.rs:1508–1527, crates/buzz-relay/src/state.rs:2183–2187, crates/buzz-relay/src/nip_fi_config.rs:478–497.

nip_fi_enforce_state and shared state::tests::test_state still use Config::from_env().expect(...). Concurrent tests set an invalid mode or Enforce without issuers under a module-private mutex; those fixture readers do not share it. They can fail before their intended assertion. Overriding config.nip_fi afterward does not repair the fallible read. The same ambient reads remain in the bounded root-auth fixtures (handlers/auth.rs:468,520), fanout fixture (handlers/event.rs:2060), and audio transaction fixture (audio/handler.rs:5039). The API fixture's existing hermetic fix remains credited.

Use the existing Config::hermetic_for_test constructor for these fixtures, or synchronize every relevant reader/writer while preserving normal parallel execution. No local failure execution is claimed.

New corrective-test regression · P2 · Global race hooks can deadlock parallel tests

crates/buzz-relay/src/state.rs:2109–2136, arm sites crates/buzz-relay/src/state.rs:4320–4323, crates/buzz-relay/src/state.rs:4431–4438, crates/buzz-relay/src/state.rs:4566–4569.

The three new manager race tests share one unkeyed process-global callback, but each installs a different blocking Barrier. Under parallel libtest, test B can replace A's callback before A's worker calls manager_disconnect_nip_fi; A's worker then enters B's barrier while A's main thread waits indefinitely on A's never-invoked barrier. An unrelated manager-disconnect test can also invoke the armed callback. The mutex protects individual slot reads/writes, not callback ownership for a whole test, and the barriers have no timeout.

Make hooks connection/control-scoped (or otherwise isolate all arming and invoking tests), with bounded rendezvous. Per-test-process nextest isolation can mask this, but ordinary parallel cargo test remains a supported path. Do not solve it by requiring the whole package to run serially.

Scope and exit criteria

The six prior boundaries were rechecked across root/audio registration, denial, effect admission and cleanup, the JOIN producer/Desktop consumer, and fixtures. The command verifier, deny map, JWKS/config, router, and pubsub blobs are unchanged from the previously reviewed head; the API delta is registration-fixture wiring only. Their previously credited capacity, privacy, fractional-time, and propagation fixes remain credited. All independent review lanes are integrated; conclusions above are checked against pinned source, not inferred from claimed test results.

RAM-only restart amnesia, asynchronous propagation/issuer re-push, and deferred S5 HTTP enforcement remain accepted. The previously withdrawn parent-revocation TOCTOU, uncertain huddle-liveness gate claim, unrelated rebase features, and separate #7224 findings are not added to these exit criteria. No persistence, synchronous cross-pod completion, generic stalled-writer overhaul, or broader auth rewrite is requested.

Exit criteria: close the five unchanged findings, finish F2's new lifecycle ordering, and isolate the newly introduced race hooks, with production-bound regression witnesses. No CI or runtime pass is claimed.

[wesbillman]

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

Changes requested

Reviewed head d4708a36043a50e8f207cbd1f91614026c9fbf44 against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, using the prior seven-finding review as the corrective contract. Three P2 defects remain, plus the previously requested production-bound audio regression coverage. Source-only review: no checkout, build, tests, repository-code execution, or runtime reproduction.

Credit: F1 observer publication now acquires and holds the session permit; F2’s lifecycle transition now claims its sentinel and cancels under the shared lock; F5 external cancellation now quiesces before root cleanup; F7 manager hooks are connection-scoped. F4’s two production audio branches now publish the deadline denial before self-cancellation. These fixes are accepted in source. The new root-writer merge integration below is separate from the now-correct terminal producers.

1 · P2 · Integrate the reserved denial channel into every bounded writer exit

connection.rs:743–800, connection.rs:823–915.

The merged writer drains terminal_ctrl_rx only in the outer cancellation arm. Cancellation during an ordinary control send, data feed, batched feed, or flush instead calls flush_terminal_frames, which has no terminal receiver, then exits. Source-derived schedule: enter one of those sink operations; a pairing/expiry/admin producer queues the canonical denial and cancels; the biased operation returns Cancelled; the helper sends the reason-derived 1008 close without the queued NOTICE. This loses the required denial payload despite correct producer ordering.

No stalled socket is needed: the existing b3_root_pairing_denial_precedes_close_through_send_loop fills ordinary control, queues denial, and pre-cancels. The top-of-loop drain necessarily takes that helper path, contradicting the test’s denial assertion. This is source evaluation, not a claimed test run.

The complementary branch at 831–835 sends the terminal frame with an unbounded send().await, before entering the existing timeout helper. A queued denial, ready cancellation, empty ordinary queues, and a sink stuck in poll_ready retain the writer; teardown awaits it at 619, before registry cleanup and semaphore release. This bypasses the base writer’s bounded-exit behavior, rather than requiring a generic writer overhaul.

Repair: drain the reserved receiver first inside the existing shared-deadline terminal helper and use it on every cancellation exit. Preserve the existing writer witnesses and cover cancellation during sink operations plus a never-ready sink with a queued denial.

2 · P2 · JOIN serializes the generation under the wrong wire key (F3)

audio/handler.rs:2284–2296.

The generation now reaches the transaction-owned JOIN builder, but it emits "lifecycle_generation". Desktop’s lifecycle parser reads "generation", as do the existing LEFT/ENDED producers.

A hydrated observer receiving START then this JOIN sees no generation and records "pending". The next authoritative liveness response supplies the real generation; reconciliation clears admissions. In-huddle indicators can disappear while audio remains connected, including in Off mode. The new test at audio/handler.rs:5363–5371 asserts the incompatible key itself.

Repair: emit "generation" and bind the regression to the actual JOIN parser/liveness contract, including Off mode. No new schema or abstraction is needed.

3 · P2 · Shared fixture readers still race the NIP-FI environment tests (F6)

state.rs:2240–2265, nip_fi_config.rs:478–497.

The primary test_state is hermetic now, but test_state_with_database_url and test_state_with_database_pool still call Config::from_env().expect(...), which parses NIP-FI. Their live callers include the unavailable-database auth test and the fake-database shutdown test; neither requires an external database to reach this read.

Parallel schedule: unknown_mode_is_rejected sets BUZZ_NIP_FI_MODE=permissive and pauses before its RAII cleanup; either fixture reads it and panics before its intended assertion. Enforce without issuers has the same problem. The writer’s module-private lock does not protect these readers, so normal parallel libtest remains nondeterministic.

Repair: use the existing hermetic constructor for ambient fixture readers, retaining their explicit database/pool overrides. Keep env-parser tests synchronized with any genuine env readers; do not serialize the package.

Remaining F4 acceptance gap: test the production audio rejection paths

nip_fi_session.rs:449–582.

Both newly named “wire” tests directly invoke control.expiry_deny_terminal; neither executes add-peer rejection nor JoinCommitError::Expired. Removing either corrected production call at audio/handler.rs:930–933 or 1243–1246 leaves these tests’ behavior unchanged. The source ordering is fixed today; this is unmet regression coverage, not another current authorization or delivery defect. Finish the already-requested delayed-expiry-task witnesses through both real rejection branches, asserting restricted JSON before 1008 Close. Reuse existing hooks/sinks.

Scope, validation, and stable exit criteria

Range-diff preserves the prior 27 patches and isolates the corrective commit plus post-merge fixture adaptation. The reviewed boundaries were root/audio admission and terminal delivery, observer publication, cleanup ownership, JOIN-to-Desktop reconciliation, and fixture/hook isolation. Previously credited capacity, privacy, fractional-time, command, and propagation fixes remain credited. RAM-only restart amnesia, asynchronous propagation/issuer re-push, and separate S5 HTTP enforcement remain accepted. Withdrawn claims, unrelated rebased features, PR #7224, persistence, synchronous cross-pod completion, and broad auth/writer rewrites remain outside this review.

The independent exact-head CI snapshot found Desktop lint/format failures in the two CSS files; other checks were still running at that snapshot. No CI rerun/monitoring or independent test pass is claimed. The three tracked .pack-cache heartbeat files are unrelated cleanup, not a blocker.

Exit: repair these three defects and complete F4’s production-bound regression witnesses. F1, F2’s transition primitive, F5, and F7 do not need another redesign.

[wesbillman]

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

Changes requested: the corrective contract is unchanged

Reviewed ef86d4f6786ed7874267c34f99838b3c214e6a4b against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, continuing review 5273266708. The sole new commit restores Desktop CSS, removes three stale heartbeat files, and adds their directory to .gitignore. That cleanup is credited. The source behind all three P2 defects and the existing F4 coverage gap is unchanged, and each was rechecked at this head.

1. P2: include the reserved denial channel in every bounded writer exit

connection.rs:743–800, 823–915.

Cancellation during ordinary control send, data feed, batched feed, or flush calls flush_terminal_frames, which receives only ctrl_rx, not terminal_ctrl_rx. Schedule: the producer queues the canonical denial and cancels while one of these operations is active; its biased cancellation branch invokes the helper and exits without the reserved NOTICE. Correct producer ordering does not fix this consumer omission.

The existing pre-cancelled/full-control-queue witness necessarily takes that helper path, contradicting its denial assertion in source. Conversely, with ordinary queues empty, the outer cancellation arm sends the reserved frame using unbounded send().await at 831–835. A never-ready sink retains the writer, which teardown awaits before deregistration and semaphore release.

Repair: drain the reserved receiver first inside the existing shared-deadline helper and use it on every cancellation exit. Cover cancellation during sink operations and a never-ready sink with a queued denial. No broad writer rewrite is requested.

2. P2: JOIN still uses the wrong generation wire key (F3)

audio/handler.rs:2284–2296 emits "lifecycle_generation"; Desktop’s parser reads "generation".

A hydrated observer receiving START then JOIN can record "pending". The next authoritative liveness response supplies the real generation and clears admissions, losing in-huddle indicators while audio remains connected, including Off mode. The Rust test at audio/handler.rs:5363–5371 asserts the incompatible key.

Repair: emit "generation" and bind regression coverage to the actual JOIN parser/liveness contract, including Off mode.

3. P2: two shared fixtures still read the mutable test environment (F6)

state.rs:2240–2265 still calls Config::from_env().expect(...) in the URL/pool fixtures, despite the primary test_state being hermetic.

Parallel schedule: unknown_mode_is_rejected sets BUZZ_NIP_FI_MODE=permissive; an unrelated fixture reads it before cleanup and panics before its intended assertion. Enforce without issuers has the same problem. The module-private writer lock does not protect fixture readers. Live callers include the unavailable-database auth test and fake-database shutdown test; neither needs an external database to reach the read.

Repair: use Config::hermetic_for_test() in both fixtures, retaining explicit URL/pool overrides. Synchronize genuine environment readers, not the whole package.

F4: production ordering accepted; production-bound regression coverage still missing

Both audio rejection branches correctly publish denial before self-cancel at audio/handler.rs:930–933 and 1243–1246. However, the two named wire tests call expiry_deny_terminal directly. Removing either production call leaves those tests unchanged. This is the existing acceptance gap, not a fourth current authorization/delivery defect: finish the delayed-expiry-task witnesses through both real rejection branches, asserting restricted JSON before 1008 Close.

Scope and validation

F1 observer permits, F2 terminal serialization, F5 quiescence, F7 connection-scoped hooks, and F4 production ordering remain credited. RAM-only restart amnesia with issuer re-push, asynchronous propagation, and separate S5 HTTP enforcement remain accepted. This was a bounded corrective review, not a renewed audit of unrelated rebased features or PR #7224.

Source-only review on the pinned Blox host with independent metadata/history reconciliation. No checkout, build, tests, repository-code execution, or runtime reproduction. The single exact-head CI snapshot shows Desktop and many other lanes passing, but PostgreSQL Domain / PostgreSQL Tests and aggregate PostgreSQL Tests failed. No cause is attributed to those failures here; no CI rerun or monitoring was performed.

Stable exit: repair these three defects and complete F4’s production-bound witnesses. Previously accepted fixes do not need redesign.

[wesbillman]

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

Changes requested: one unresolved defect and one corrective-test regression

Reviewed 5dfc5fdf0e2ba0b7bfcff605461cd22acc16882a against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, continuing review 5273544749. This is the bounded corrective review of the three intervening commits, not a renewed S3/mesh/HTTP audit.

Credited: the root writer now drains its reserved denial receiver first under one shared deadline on every cancellation exit (connection.rs:745–934); the URL/pool fixtures now use the hermetic constructor while retaining explicit overrides (state.rs:2226–2263). Neither needs redesign.

1. P2: JOIN still publishes the wrong generation wire key (existing F3)

audio/handler.rs:2332–2337 emits "lifecycle_generation", but Desktop's parser reads content.generation only. The revised Rust assertion still checks the incompatible key; changing its value to the Off-mode UUID and moving the wrapper to postgres_tests does not fix the wire contract.

Source-derived reproduction: an already-hydrated observer receives START then JOIN for a new live room. Desktop records "pending"; the next authoritative liveness response supplies the real UUID (Off-mode producer). The refresh passes the old active generations to reconciliation, which clears admissions on this mismatch. The in-huddle indicator disappears while audio remains connected.

Repair: emit "generation": lifecycle_generation, correct the Rust oracle, and bind coverage to the actual Desktop JOIN parser/liveness reconciliation contract, including Off mode.

2. P2: the rewritten race witnesses circularly wait until their hooks panic

Representative: state.rs:3761–3777 and 3805–3822. After hook_ready, the coordinator synchronously calls disconnect_community() before sending hook_proceed. That call acquires the same mutex the deny worker holds while its hook waits for hook_proceed.

The resulting schedule is deterministic: worker holds mutex → waits for proceed; coordinator waits for mutex → cannot send proceed. After five seconds, recv_timeout(...).expect(...) panics in the worker. The poisoned lock is recovered by the competing transition, but joining the worker then fails. These witnesses cannot reach their intended passing terminal-frame assertions. This is a test-only validation regression, not a production deadlock, and was established from source, not a test run.

The same ordering occurs in eight rewritten witnesses: w_cancel_race, w_expiry_cancel_race, w_pairing_cancel_race, w_auth_cancel_race, w_manager_cancel_race, w_lifecycle_cancel_race, w_root_manager_drain_race, and w_audio_registry_lifecycle_cancel_race (their blocking calls/proceed sends are at state.rs:3815–3818, 4000–4003, 4189–4192, 4348–4351, 4523–4526, 4721–4724, 4866–4869, and 5002–5005). lifecycle_cancel and drain_all enter the same transition mutex.

Repair: put the competing lock-taking operation on a separate thread, keep the coordinator free to release the hook, establish the blocked/uncancelled state while the hook is held, then release and join both operations before checking the terminal oracle. Preserve the per-control UUID isolation; the production serialization itself remains credited.

F4: production ordering remains accepted; existing coverage gap remains open

Both real rejection branches still enqueue denial before lifecycle cancellation (add-peer, commit). But the rewritten witnesses construct separate past-deadline and future-deadline gates and call expiry_deny_terminal directly. Neither enters the corresponding handle_active_audio_connection rejection branch. Removing either production call leaves these test bodies unchanged, contrary to their comments.

This is the existing acceptance gap, not another current authorization/delivery defect. Finish the independently delayed-expiry-task witnesses through both actual rejection seams, asserting restricted JSON before 1008 Close and failing when the corresponding production call is removed.

Scope, evidence, and stable exit

F1 observer permits, F2 terminal serialization, F5 quiescence, F7 connection-scoped hook design, and F4 production ordering stay credited. RAM-only restart amnesia/issuer re-push, asynchronous propagation, and separate S5 HTTP enforcement stay accepted. The new finding concerns the changed test rendezvous, not a reopening of those contracts.

Source-only review on pinned Blox, with independent metadata/history reconciliation and coordinator verification of producer/consumer and lock/hook ordering. No checkout, installation, build, tests, PR-code execution, or runtime reproduction. The one existing exact-head CI snapshot had 34 successful, 19 skipped, 3 in-progress checks, zero failures (56 total), including a successful Rust unit lane. That snapshot is not evidence that these particular witnesses ran; no CI failure is attributed to them, and no rerun or monitoring was performed.

Stable exit: fix the JOIN key/consumer regression, repair the eight changed race witnesses, and finish F4's already-required production-bound coverage. Previously accepted production fixes do not need redesign.

[wesbillman]

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

Changes requested: two inherited items remain; F4 coverage is closed

Reviewed e828136739f6360ae0407ee6b636f19c2bb83ed5 against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, continuing review 5273883265. This is the bounded corrective review of the two intervening commits, not a renewed S3/mesh/HTTP audit. The contract remains coherent huddle presence across JOIN/liveness and denial-before-close delivery with production-bound regression witnesses.

Credited: both new F4 witnesses now enter the actual audio rejection branches and assert restricted JSON followed by 1008 Close (add-peer, commit). They park the handler at its gate seam and cancel its token while the independent deadline task has not fired. This closes the previously missing caller binding; the older primitive tests are now honestly labelled. The restart-arm denial drain, added pre-send-loop rejection drains, and shared test-environment mutex are also credited from source. No test execution is claimed.

1. P2: JOIN still uses the wrong generation wire key (existing F3)

audio/handler.rs:2405–2410 still emits "lifecycle_generation", while Desktop’s parser reads only content.generation. The Rust oracle still asserts the incompatible key.

Source-derived reproduction: an already-hydrated observer receives START then JOIN for a new live room. Desktop records "pending"; the next authoritative liveness response supplies the UUID. Refresh passes the old generation map to reconciliation, which clears admissions on the mismatch. The in-huddle indicator disappears while audio remains connected, including Off mode.

The newly added liveness witness does not close this contract. It seeds KIND_HUDDLE_LIVENESS - 1 and discards insertion errors: that is 48103, not the 48100 required by huddle_started_links. Its fresh fixture therefore supplies no matching START link; absence of a liveness EVENT is explicitly allowed to pass. It also never consumes an actual JOIN or calls Desktop’s parser, so changing JOIN’s key/value cannot affect this witness. This is a defect in the attempted F3 coverage, not a separate product requirement.

Repair: emit "generation": lifecycle_generation, correct the Rust oracle, and bind coverage to the actual JOIN → Desktop parser → liveness reconciliation contract. Seed the proper START kind, fail on fixture errors/missing EVENT, and compare against a real JOIN rather than only the in-memory expected UUID. Include Off mode.

2. P2: five race witnesses still circularly wait until their hooks panic

Three witnesses now put the competing call on its own thread, which removes their circular wait. Five remain unchanged: w_cancel_race, w_expiry_cancel_race, w_pairing_cancel_race, w_auth_cancel_race, and w_manager_cancel_race.

Representative: the worker’s hook waits for hook_proceed while holding the transition mutex. After hook_ready, the coordinator synchronously calls disconnect_community before sending proceed; that call takes the same mutex. Worker waits for coordinator; coordinator waits for worker. After five seconds the hook times out and panics, and joining the worker fails. The intended terminal-frame assertions cannot complete. This is a test-only validation defect, not a production deadlock, established from source rather than execution.

The remaining blocking-call/proceed pairs are state.rs:3815/3818, 4000/4003, 4189/4192, 4348/4351, and 4523/4526.

Repair: move those five competing calls onto independent threads, keep the coordinator free to release the hook, then join both operations before the terminal oracle. Preserve connection-scoped hook isolation. Non-blocking refinement for the three repaired witnesses: their immediate spawn-then-release no longer forces contention, so their documented mutation detection is scheduler-dependent; establish the blocked/uncancelled state before release. This does not reopen the accepted production serialization.

Scope, validation, and stable exit

F1 observer permits, F2 terminal serialization, F5 quiescence, F7 connection-scoped hook design, the root-writer shared-deadline drain, and hermetic fixtures remain credited. RAM-only restart amnesia/issuer re-push, asynchronous propagation, and separate S5 HTTP enforcement remain accepted. F4 production ordering and its new caller-bound coverage are closed. No unrelated S3/mesh/HTTP changes or generic writer hardening are requested.

Source-only review on pinned Blox, with independent metadata/history reconciliation and coordinator verification of decisive producer/consumer, fixture/query, and lock/hook paths. No checkout, installation, build, tests, PR-code execution, or runtime reproduction. The one existing exact-head CI snapshot had 17 successful, 18 skipped, 12 in-progress, and 2 failed checks (49 total). Failures were Rust / Rust Lint and Rust / Windows Rust; Unit Tests was in progress and the relay PostgreSQL lane skipped. No failure cause is attributed here, and no rerun or monitoring was performed.

Stable exit: repair F3’s wire contract and meaningful consumer coverage, and remove the circular wait from the five remaining race witnesses. Do not redesign the credited fixes.

[wesbillman]

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

Changes requested: the same two exit items remain

Reviewed 497ac9e3f982315d0e29412e67ef35130dcf5c84 against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, continuing review 5274381697. The sole descendant commit changes only crates/buzz-relay/src/nip_fi_session.rs (+234/-9). The F3 producer/consumer and five race-witness files are byte-identical to the prior reviewed head. This is a bounded corrective review, not a reopened S3/mesh/HTTP audit.

Credited: F5 replaces the 50 ms sleep with a connection-keyed, bounded cancel-arm rendezvous and adds a subscription-registry cleanup witness. The new witness holds an effect permit, enters the real registry-disconnect/expiry path, models an in-flight subscription registration, then explicitly removes it after task completion and asserts zero remaining entries. That is useful source-level coverage; it is not an executed mutation result or a full production-teardown integration test. Existing F5 production quiescence remains credited, with no new blocker in this delta.

1. P2: JOIN still emits a generation key Desktop does not consume (F3)

JOIN serialization writes lifecycle_generation; Desktop’s parser reads only generation. The Rust oracle still asserts the incompatible key.

Source-derived reproduction/impact: an already-hydrated observer receives START then JOIN for a new live room. Desktop records "pending"; the next liveness response supplies the real UUID. Refresh passes the previous generation map to reconciliation, which clears admissions on that mismatch. The in-huddle indicator disappears while audio remains connected, including Off mode.

The attempted F3 liveness coverage remains vacuous: its fixture seeds KIND_HUDDLE_LIVENESS - 1 (48103 ENDED, not 48100 STARTED) and discards insertion errors. The database lookup requires STARTED, while the oracle explicitly passes without any liveness EVENT. It never consumes an actual JOIN or invokes Desktop’s parser.

Repair: emit "generation": lifecycle_generation, fix the Rust oracle, and cover the actual JOIN → Desktop parser → liveness reconciliation contract, including Off mode. Use a valid START fixture, fail on fixture errors/missing EVENT, and compare with the real JOIN rather than only the in-memory UUID.

2. P2: five race witnesses still circularly wait until the hook times out

The unchanged witnesses are w_cancel_race, w_expiry_cancel_race, w_pairing_cancel_race, w_auth_cancel_race, and w_manager_cancel_race.

Source-derived schedule/impact: the producer fires its test hook while holding the transition mutex; the hook waits for the coordinator’s proceed signal. After readiness, the coordinator synchronously calls disconnect_community before releasing the hook. That call takes the same mutex. Neither side can advance until the five-second hook timeout panics, so the worker join/terminal oracle cannot complete successfully. This is a test-only validation defect, not a production deadlock.

All five blocking-call/proceed pairs remain state.rs:3815/3818, 4000/4003, 4189/4192, 4348/4351, and 4523/4526.

Repair: run the competing calls on independent threads, leave the coordinator free to release the hook, then join both operations before asserting the terminal result. Preserve connection-scoped hook isolation. The previously noted contention refinement for the three already-repaired witnesses remains non-blocking.

Scope and validation

The product contract remains coherent huddle presence across JOIN/liveness and denial-before-close delivery with meaningful regression witnesses. F1 observer permits, F2 terminal serialization, F4 production ordering and caller-bound coverage, F5 quiescence, F7 hook isolation, the root-writer shared-deadline drain, and hermetic fixtures remain credited. RAM-only restart amnesia/issuer re-push, asynchronous propagation, and separate S5 HTTP enforcement remain accepted. No unrelated hardening is requested.

Source-only review on pinned Blox, with independent metadata/history reconciliation and coordinator verification of the decisive contracts. No checkout, installation, build, tests, PR-code execution, or runtime reproduction. One existing exact-head CI snapshot: 47 successful, 27 skipped, 6 failed, 1 cancelled displayed entries (81 total; aggregate duplicates included). Red entries concern Rust Lint, Windows Rust, and PostgreSQL Tests; Run Codex Security Review was cancelled. No logs were inspected, causes attributed, reruns requested, or monitoring performed.

Stable exit: repair F3’s wire contract and meaningful consumer coverage; remove the circular waits from the five remaining race witnesses. Preserve the credited fixes.