fix(buzz-acp): tell a refused author instead of dropping them silently - #1
Closed
QuicksilverSlick wants to merge 10 commits into
Closed
fix(buzz-acp): tell a refused author instead of dropping them silently#1QuicksilverSlick wants to merge 10 commits into
QuicksilverSlick wants to merge 10 commits into
Conversation
An inbound event from an author outside the gate was dropped with a tracing::debug! and a `continue`. Nothing was posted, no reaction was added, and debug is off in practice — so the sender got silence and the owner never learned anyone had tried to reach the agent. RespondTo::default() is OwnerOnly, so this is the first thing a newly invited collaborator hits: their opening message vanishes, and both sides believe the other is unresponsive. The refusal now: - logs at warn! rather than debug! — a person who got no answer is not a debug-level event; - posts one in-channel notice per (channel, author), anchored to the sender's thread, saying what happened, that their message is not lost, and what has to change for it to be picked up; - p-tags the owner in channels, so they learn through the normal mention path, in the project the request arrived in. The agent never opens a DM: keeping the exchange with its project preserves the context, and an inbox of agent DMs is an inbox people stop reading. One notice per person per channel, not per event: silence strands the sender, but replying every time lets a noisy author use the agent as a flooder. The memo is bounded, since anyone can mint a pubkey and post — at the cap it clears rather than evicting, because the cost of forgetting is one repeat notice to someone already told. In a DM the notice carries no owner mention and does not name them: the sender is an unknown party and the owner's pubkey is not theirs to learn. The existing DM hardening is untouched — this changes only whether a refusal is visible, never who is allowed to steer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The four dead-letter notices named the mechanism, leaked internals, and
gave instructions the reader usually could not follow:
"the turn exceeded the maximum duration (7200s)"
"Please re-authenticate the CLI (e.g. run `claude /login`)"
format!("{e}") interpolated straight into the channel
"Turn", "batch" and a raw seconds count are our vocabulary, not the
reader's. Telling a collaborator to run `claude /login` is worse than
useless: they have no access to the machine it would run on, so the
message describes a fix they cannot perform and names nobody who can.
And interpolating a Display impl into a channel can carry paths, hosts
or tokens to anyone with read access.
Each notice now says what happened, what it means for the reader, and
what happens next:
- the raw error stays in the log; the channel gets a plain description
of the state via failure_reason_text();
- the auth notice says the work is blocked until someone with access
restores it, rather than prescribing a command;
- notices stop claiming a re-send will help when the retry budget is
already spent — that trains people to repeat themselves into silence;
- the owner is p-tagged, so a failure the reader cannot fix reaches
somebody who can instead of sitting in a channel nobody is watching.
post_failure_notice and post_gate_notice were near-identical, so they
collapse into one post_notice() carrying mentions. Net -83 lines in
pool.rs.
Notice routing stays channel-based here. Role-based routing — the
requester in their own conversation, the person who can fix it in the
project — needs the orchestrator model that does not exist yet, and a
message promising delivery the code does not perform would be a
regression, not an improvement.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two paths in EventQueue::push drop a request somebody sent, and both were observable only as a log line nobody was necessarily reading: - DedupMode::Drop discarded events for an in-flight channel at tracing::debug! — invisible in practice, so work could go missing with no way to establish afterwards that it had; - the per-channel depth cap evicts the OLDEST queued event, meaning the request that has waited longest is the one lost. Neither kept a running total, so "did we lose anything?" was not an answerable question — only "is there a line in the log I happened to catch". Both now increment a DropCounts, logged with the running total the way relay.rs already accounts for gated_observer_dropped. The dedup drop moves from debug! to warn!, matching the depth-cap drop: discarding a request is not a debug-level event. A run that discarded anything says so on shutdown. Scattered mid-run lines are only useful to someone watching at the time; a process that loses work and then exits quietly leaves nothing to answer the question later. drop_counts() exposes the totals so a supervisor outside this queue can observe the loss — the process doing the dropping is the least reliable place to report it from, which is the same reason the watchdog belongs outside the agent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This fork is Dreamforge. The rename covers what a person reads and nothing else: RENAMED window title, product name, 228 user-visible strings UNCHANGED crate names (buzz-*), so upstream `use` paths resolve UNCHANGED BUZZ_* environment variables UNCHANGED wire tags (buzz-channel, buzz-protect, buzz-visibility) UNCHANGED event kinds and the app identifier The exclusions are the point. Upstream changed 367,825 lines in the last fortnight, 5,412 of which touch the name: 1,943 crate/module references and 870 environment variables, against ~350 mentions of the bare word. Renaming the first two categories is where a fork breaks. An environment variable is the worst case. Upstream adds BUZZ_NEW_THING, its code reads BUZZ_NEW_THING, and a renamed prefix means it reads a variable nobody sets — compiles, runs, silently takes a default. A wire tag is worse still: no error, just two systems that no longer agree. Display strings have no such coupling. Nothing reads them back. The app identifier stays xyz.block.buzz.app deliberately. Changing it moves the app data directory and keyring service, orphaning the identity and settings of anyone who has already onboarded. It is invisible to users, so the rebrand does not need it. Code comments keep saying Buzz — 108 of them. They are invisible to users and every one rewritten is a merge conflict against upstream for no reader's benefit. Identity surfaces (window title, terminal panel, shared compute) now read from shared/constants/brand.ts. Prose keeps the literal word: turning every sentence into a template literal would add conflict surface without making a future rename meaningfully easier. Verified: tsc clean, vite build clean, and biome flags exactly the same 7 pre-existing files as before the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"Buzz-native", "Buzz-hosted" and "Buzz-curated" survived the first sweep because the pattern excluded a trailing hyphen — deliberately, to keep wire tags like buzz-channel intact. These are safe: wire tags are lowercase, and a capital B never appears in one. All three are prose a person reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Foundation for durable tickets: a request is written down when it arrives so it cannot be silently dropped, and something outside the agent sweeps for ones that stopped moving. This commit is the part with no I/O — kinds and the fold — so the model can be reviewed before anything depends on it. TWO KINDS, NOT ONE KIND_TICKET 30623 the request KIND_TICKET_STATUS 30624 one live row per (ticket, actor) The NIP-33 replacement key is (community, kind, pubkey, d_tag), so the author pubkey is part of an addressable event's identity. A single addressable ticket could therefore only ever be advanced by its original signer — an assignee, a reviewer, or the watchdog could never move it. Splitting root from status turns that binding from an obstacle into the authorization mechanism: each actor owns their own row, and no actor can forge another's. WHY 3062x AND NOT THE EMPTY 47000 BLOCK Storage class is decided purely by numeric range, so a 4xxxx kind can never be replaced — every transition would be a new immutable row — and extract_d_tag returns None outside 30000–39999, leaving d_tag NULL and no indexed way to find all events for a ticket. 30623/30624 are the next free slots after 30620/30621/30622; verified free across .rs/.ts/.sql. Adding a kind to the registry does not make the relay accept it: required_scope_for_kind still ends in "restricted: unknown event kind", so ingest wiring is a separate, deliberate step. THE FOLD State is derived from status events rather than stored in a column, so the events stay canonical and a replayed log reproduces the same answer. Four rules, each with a test: - terminal wins, so one actor's stale Progress cannot keep a finished ticket alive; - otherwise the furthest-advanced live state wins; - ties break on recency; - the deadline is the EARLIEST live one, not the latest — a watchdog must fire on the first thing that should have happened, and taking the latest would let one long-running actor mask another's stall. Escalated is deliberately NOT terminal. Raising something to a human is not resolving it, and an escalation that is then ignored is exactly the case this exists to catch. Unknown `s` tag values are rejected rather than guessed: a ticket in an unrecognised state must not silently look healthy. 9 tests. fmt and clippy clean; buzz-core suite green at 271 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fold's rules are judgement calls, not implementation details — one person finishing a ticket while another is mid-work SHOULD arguably end it, but that is a decision someone has to agree with, and it is cheaper to disagree now than after the relay and watchdog depend on the semantics. Eight scenarios, printed with what the fold decides and whether the watchdog would act: cargo run -p buzz-core --example ticket_scenarios Covers the cases that actually happen: nobody picked it up, healthy progress, went quiet mid-work, one worker stalled behind a slow one, finished while another was still working, gave up with a reason, escalated then ignored, and out-of-order delivery. Written so the behaviour can be reviewed without reading Rust. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An adversarial review compiled this module standalone and executed the cases, rather than reasoning about them. Three holes were real, all in the same direction: the fold could stop watching a ticket that nobody had finished. 1. A LIVE ROW WITH NO DEADLINE READ AS "NEVER DUE". One Acknowledged row with not_before: None gave deadline: None, and is_overdue returned false at every representable timestamp. Any actor could silence the sweeper permanently by acknowledging without a deadline — and it was strictly worse than posting nothing, because open_deadline is consulted only when there are no rows at all. So acknowledging a ticket made it LESS watched than ignoring it. The module's premise — every non-terminal state carries the instant it must be acted on by — was documented and enforced nowhere. is_overdue now treats a live ticket with no deadline as due. "Nobody said when to check this" is not "never check this". Sweeping early is noisy; sweeping never is the failure this exists to prevent. The fold also falls back to open_deadline for a live row that named none. 2. A FAILURE COULD CARRY NO REASON. fold returned state: Failed, reason: None. The requirement is failed-with-reason and the struct's own comment said required, but nothing checked. A failure a person cannot read is a silent failure wearing a loud label. Missing reasons now render as MISSING_REASON rather than an empty field the reader has to interpret. 3. SAME-SECOND ROWS FROM ONE ACTOR RESOLVED BY STATE RANK. Nostr timestamps are whole seconds, so two rows from one actor in the same second is ordinary. Rank ordering let a same-second Done swallow the actor's own newer live row and terminalise the ticket forever. Ties now resolve fail-safe: prefer the non-terminal row, then the tighter deadline. An extra escalation on a finished ticket is noise; a dropped request is not recoverable. Deliberately NOT ordered by state rank, which was the reviewer's suggested fix. The confirmation pass ran the counterexample: with Acknowledged@100/nb=1000 against Progress@100/nb=130, rank ordering picks the loose deadline and goes silent at now=200 — producing exactly the failure it claimed to prevent. Rank and deadline tightness are independent, so tightness decides. 7 regression tests, one per hole plus order-independence and a guard that finished tickets do not sweep forever. 16 ticket tests, buzz-core green at 278. fmt and clippy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The watchdog's clock and a worker's completion can land in the same second. The terminal row was chosen with max_by_key((created_at, state)), and Failed ranks above Done, so the watchdog's timeout won — the requester was told their request failed, with a reason that had not actually happened, while the work had in fact completed. A false failure is worse than a late success. Owner's call, and the right one: completion wins. Ties now select the lower-ranked terminal state via Reverse, and the test asserts it in both arrival orders, since the fold must not depend on which row the relay hands over first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Research into how mature systems handle multi-actor work items turned up
one near-universal rule that the previous model broke: no single
participant's exit may write the outcome of shared work. Temporal states
it flatly — "an Activity Failure will never directly cause a Workflow
Failure". PagerDuty records a responder as joined or declined on the
RESPONDER. ITIL has the requester confirm closure, never the implementer,
because the party who did the work is the least able to judge whether the
need was met and the most motivated to call it finished.
The old fold had one enum and rule 1 was "terminal wins — if ANY actor
says done or failed, the ticket is finished". So a Claude Code session
that could not push a branch closed Craig's bug report for everyone,
permanently, while a reviewer was still working it. The example program
flagged that as a judgement call; it was a bug.
Split into two axes that cannot be confused:
Participation acknowledged / working / blocked / delivered / abandoned
what ONE actor says about ITSELF; never terminal
Outcome done(confirmation) / archived
how the TICKET ended; only an Authority may write it
Consequences, each with a test:
- A worker giving up records Abandoned with its reason against itself.
The ticket stays live, the reason is kept, and the actor drops out of
the live set. This is the owner's instinct — archived with the reason
so it stays tracked — honored at the right altitude.
- Everyone leaving yields Unowned: not terminal, still swept, needing
reassignment. That state previously had no name and folded to a
deadline-less limbo.
- Delivered is LIVE, not terminal. Finishing is not the same as being
confirmed, and an unconfirmed delivery must not sit unnoticed.
- An Outcome from an actor without authority is DEMOTED to the matching
participation value and its author named in unauthorized_close.
Obeying it is the bug; dropping it silently is the other bug.
- Done carries a Confirmation, so a close because a person agreed and a
close because nobody objected are different facts. The second names
who delivered and how long the window was, and can never render as the
first.
- A close is deliberately not blocked by live work, so anyone
interrupted is named rather than silently orphaned.
Blocked (was Escalated) stays available to every actor: refusing an
agent the ability to say it is stuck would itself be a silent failure.
Which rung of the escalation ladder to use remains the watchdog's
decision, so no actor can page a human harder by asking.
Deliberately NOT adopted from the research: Kubernetes finalizers and
Jira sub-task blocking conditions, both write-time enforcement with no
implementation on this substrate, and the finalizer pattern imports a
deadlock whose only documented remedy is an admin override that does not
exist here.
Recorded as a known gap rather than papered over: kind:30624 is
addressable, so an actor can replace its own row and erase the
Abandoned reason that triggered an escalation. Closing that needs status
rows to become head pointers over a chain of regular transition events —
a change to the event model, not to this fold.
19 tests. Nothing outside buzz-core consumes this yet, so the break is
free to take now and would not be later.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
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.
What this fixes
An inbound event from an author outside the gate was dropped with a
tracing::debug!and acontinue— no reply, no reaction, nothing above debug level:RespondTo::default()isOwnerOnly(asserted inconfig.rs), so this is the first thing a newly invited collaborator hits. Their opening message vanishes, they conclude the agent is broken, and the owner never learns anyone tried. It is silent at both ends.What changes
warn!instead ofdebug!— a person who got no answer is not a debug-level event.(channel, author), anchored to the sender's thread: what happened, that their message is not lost, and what has to change for it to be picked up.The agent never opens a DM. Keeping the exchange with its project preserves the context, and an inbox of agent DMs is an inbox people stop reading.
Design notes
Once per person per channel, not per event. Silence strands the sender; replying every time lets a noisy author use the agent as a flooder. The memo is bounded — anyone can mint a pubkey and post, so the key space is attacker-controlled. At the cap it clears rather than evicting per-entry: the cost of forgetting is one repeat notice to someone already told, which is cheaper than the bookkeeping an LRU would need on a path that exists purely to suppress duplicates.
DMs carry no owner mention and the text does not name the owner — the sender is an unknown party and the owner's pubkey is not theirs to learn.
The gate itself is untouched. This changes only whether a refusal is visible, never who is allowed to steer. The existing DM hardening is unmodified, and all 12 pre-existing
author_gate_testsstill pass.Message text
It names the state, not the mechanism, and never asks for a resend of a message that is already there — re-sending cannot change the outcome, and asking for it trains people to repeat themselves into silence.
Testing
5 new tests in
author_gate_tests, covering: the notice says what happened and what happens next; it never asks for a resend; it does not leak the owner in a DM; it fires once per person per channel; and the memo stays bounded under attacker-supplied keys.cargo fmtclean,cargo clippy --all-targetsclean.On the suite:
cargo test -p buzz-acpreports 28 failures — these are pre-existing onmain, verified by stashing this change and re-running:mainSame 28 failures, +5 passing — exactly the tests added here.