buzz-acp: wake-ticket persistence for unconsumed mentions (today-slice) - #5940
Open
riddlef0x wants to merge 3 commits into
Open
buzz-acp: wake-ticket persistence for unconsumed mentions (today-slice)#5940riddlef0x wants to merge 3 commits into
riddlef0x wants to merge 3 commits into
Conversation
…y-slice) buzz-acp's event queue is RAM-only: a mention that's accepted but not yet consumed does not survive process death. A bounce (kickstart, crash, sleep) sets a fresh connect-time watermark, and any unconsumed mention is gone — this is the class of bug behind #01fa1107. This is the gated today-slice from PLANS/WAKE_TICKET_SPEC.md (Oksana, 2026-08-15): a durable per-agent ticket store, default off behind --wake-ticket-dir / BUZZ_ACP_WAKE_TICKET_DIR, side-binary/canary use only. - crates/buzz-acp/src/wake_ticket.rs: WakeTicketStore. One jsonl line per mention event_id, exclusive flock on the ticket dir for the process lifetime (dual-run is a crash, not a race), atomic tmp+rename writes, chmod 600, last-write-wins compaction on open, open -> claimed -> done (compacted) | drop (kept for audit) lifecycle. - lib.rs: ticket `open` written after the first successful queue.push (not inside EventQueue::push — requeues go through that too); `claimed` on successful dispatch; `done` only on PromptOutcome::Ok with mark_complete run, no pending queue depth, and no active retry throttle (mark_complete itself is a lock release, not completion); `drop` on dead-letter, non-retryable auth error, or channel removal. Boot replay runs before HarnessRelay::connect / set_startup_watermark: loads open+claimed tickets, re-validates membership + the author gate, pushes survivors into the queue, and seeds the relay's seen_ids so the 5s subscribe window can't double-deliver them. - relay.rs: new RelayCommand::SeedSeenIds plumbing for the above. - queue.rs: EventQueue::is_retry_throttled, and queued_event_count is no longer test-only (needed for the done predicate). Unit tests cover the ticket store directly (open/claimed/done-predicate/ drop/replay-after-restart/lock-fail/cap), per the gate. Out of scope for this slice, per the gate: nothing points launchd at the side binary, the shipped /Applications/Buzz.app binary is untouched, and no canary has run. That's the next step, gated on Oksana's review here plus a Zar-only bounce proof. Signed-off-by: Robin Leonard <robinleonardtrust@gmail.com>
…-08-15) Two real bugs from Oksana's review of PR block#5940: 1. Boot replay treated any REST transport failure (query error, unresolved channel type) as a confirmed deny and permanently dropped the ticket. The bounce this feature exists to survive is often network-adjacent (Railway blip, DNS hiccup) — a transient failure at boot must not delete work a moment's retry could recover. `is_still_member` now returns `Option<bool>` (`None` = query failed, not "not a member"). Replay validation is extracted into a testable `validate_ticket_for_replay` -> `ReplayDecision::{Push,Drop,SkipThisBoot}`, resolving channel type directly (not through the live path's `is_dm_channel`, which intentionally fails closed to DM on an unresolved fetch — exactly the behavior that was turning a metadata blip into a false author-gate rejection for a normal channel). `Drop` now only fires on a confirmed deny; `SkipThisBoot` leaves the ticket untouched for the next boot. 2. `done` was gated on the channel's queue being empty. A completed batch (A) whose channel had more traffic queued behind it (B, still open) was never marked done — it stayed `claimed` indefinitely, so a later bounce replayed an already-finished turn a second time. This is the exact `01fa1107` double-delivery class the ticket store exists to prevent. Only one batch can be claimed/in-flight per channel at a time, so `claimed_event_ids_for_channel` already scopes correctly to *this* batch regardless of what else is queued — the fix is to stop gating on queue depth, not to thread ids through a different path. Adds `wake_ticket_wiring_tests`: the state-machine-driving tests through `handle_prompt_result` and the replay-validation helpers that the store-only unit tests couldn't reach, per Oksana's ask — Ok-with-more- queued marks exactly this batch done (the regression above), a requeued failure leaves the ticket claimed (mark_complete alone is not done), a genuine dead-letter still drops, and both `is_still_member` and `validate_ticket_for_replay` treat an unreachable relay as "unknown", not "deny". Signed-off-by: Robin Leonard <robinleonardtrust@gmail.com>
…ana, 2026-08-15) Second pass: the two named blockers were fixed, but the residual I flagged (check_sibling_via_profile fails closed to false on a query timeout/error) turned out to be blocker 1 on the path this team actually uses — Zar's traffic is other agents, not Robin, so most replayed tickets need a fresh sibling lookup, and a Railway blink landing there was deleting them exactly like the membership/channel-type checks already fixed. Per instruction: do not fork the live author_allowed. Added a parallel replay-only decision tree instead: - extract_verified_sibling: the NIP-OA tag-parsing/verification logic pulled out of check_sibling_via_profile as a pure function, shared by both the live and replay paths so there's exactly one implementation to keep correct. - check_sibling_via_profile_for_replay / is_owner_or_sibling_for_replay / author_allowed_for_replay: tri-state mirrors of the live functions. A query timeout/error is None (unknown, not confirmed). A malformed pubkey, no owner configured, a completed query with no valid tag, or a completed query with a tag that fails signature verification are all Some(false) — confirmed, not a transport problem, no reason to wait for a retry. The live functions are untouched. - validate_ticket_for_replay now calls author_allowed_for_replay and maps None to SkipThisBoot. Cached sibling/owner results skip the network entirely on both paths, so this doesn't cost every replayed ticket a fresh profile query. Tests: unreachable relay on the sibling-lookup path skips both is_owner_or_sibling_for_replay and author_allowed_for_replay (not deny); cached results resolve without touching the network. Signed-off-by: Robin Leonard <robinleonardtrust@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Today-slice from
PLANS/WAKE_TICKET_SPEC.md, gated by Oksana's architecture review (RESEARCH/WAKE_TICKET_TODAY_SLICE_GATE_20260815.md, 2026-08-15).buzz-acp'sEventQueueis RAM-only: a mention that's accepted into the queue but not yet consumed does not survive process death. A bounce (kickstart, crash, sleep/wake) captures a fresh connect-time watermark, and any unconsumed mention is gone — this is the bug class behind the01fa1107incident referenced in the spec.This PR adds a durable wake-ticket store, default off behind
--wake-ticket-dir/BUZZ_ACP_WAKE_TICKET_DIR, for side-binary/canary use only. It does not touch the shipped/Applications/Buzz.appbinary, does not point launchd at anything, and does not run a canary — those are explicitly out of scope for this slice per the gate.crates/buzz-acp/src/wake_ticket.rs— newWakeTicketStore. One jsonl line per mentionevent_id, exclusiveflockon the ticket dir for the process lifetime (dual-run is a crash, not a race — lock failure is fatal), atomic tmp+rename writes,chmod 600, last-write-wins compaction onopen(). Lifecycle:open→claimed→done(compacted away) |drop(kept for audit, never replayed).lib.rs:openis written after the firstqueue.pushthat returnstrue(not insideEventQueue::push— requeues go through that path too).claimedon successful dispatch (dispatch_pending, right aftertry_claimsucceeds).doneonly whenPromptOutcome::Ok,mark_completehas run, the channel has no pending queued events, and no active retry throttle —mark_completeitself is a lock release, not a completion signal (see gate: hookingdonethere recreates the01fa1107bug).dropon dead-letter (hard-cap timeout with no recent activity, or retry budget exhausted), non-retryable auth error, or channel removal (both the live membership-notification drain path and the in-flight-batch removed-channel path).HarnessRelay::connect()/set_startup_watermark(): loadsopen+claimedtickets, re-validates channel membership and the author gate against a lightweight pre-connect REST client, pushes survivors into the queue, and seeds the relay'sseen_ids(newRelayCommand::SeedSeenIds) right after connect so the 5s subscribe window can't double-deliver them.queue.rs—EventQueue::is_retry_throttled;queued_event_countis no longer test-only (needed for thedonepredicate).relay.rs—RelayCommand::SeedSeenIdsplumbing for the above.Ticketalso carries theprompt_tagthat matched at write time (not in the spec's original field table) — boot replay reconstructs aQueuedEventdirectly from the ticket without re-running rule matching, since subscription rules aren't available that early in boot (no relay, no channel discovery yet) and may have changed since the ticket was written anyway.Test plan
cargo test -p buzz-acp --lib— 787 passed, 1 pre-existing unrelated failure (acp::tests::keepalive_resets_idle_past_deadline, a timing flake reproducible identically on unmodifiedmain).cargo clippy -p buzz-acp --all-targets— clean.cargo fmt -p buzz-acp -- --check— clean.cargo build --release -p buzz-acp— builds; smoke-tested--wake-ticket-dirend to end (store opens, lock file +chmod 700dir +chmod 600jsonl created, all before the relay connect attempt is logged).Rollout after this review (not part of this PR): worktree build → side binary at
~/Library/Application Support/xyz.block.buzz.app/harness/bin/buzz-acp-wake, first (and only, until proven) canary unit is Zar,run-zar.shpointed at the side binary with Desktopstart_on_app_launch=falsefor that key, proof = open ticket on disk survives a bounce with no second@.🤖 Generated with Claude Code