Skip to content

fix(buzz-acp): stop dropping people's requests silently - #2

Merged
QuicksilverSlick merged 4 commits into
mainfrom
fix/acp-silent-failures
Aug 31, 2026
Merged

fix(buzz-acp): stop dropping people's requests silently#2
QuicksilverSlick merged 4 commits into
mainfrom
fix/acp-silent-failures

Conversation

@QuicksilverSlick

Copy link
Copy Markdown
Owner

Three places where buzz-acp lost work or told someone something useless, plus the gitignore rule for the debris that surfaced while fixing them.

1. A refused author was told nothing

An inbound event from someone outside the author gate hit tracing::debug! and continue — no reply, no reaction, nothing above debug level. Since RespondTo::default() is OwnerOnly, this is the first thing a newly invited collaborator hits: their opening message vanishes, and neither side learns the other tried.

Now: warn! not debug!, one in-channel notice per (channel, author), and the owner p-tagged so a request that cannot be served still reaches someone who can act. Bounded memo — the key space is attacker-controlled.

2. Failure notices described a fix the reader could not perform

"the turn exceeded the maximum duration (7200s)"
"Please re-authenticate the CLI (e.g. run \claude /login`)" format!("{e}")` interpolated straight into a channel

"Turn" and a raw seconds count are our vocabulary. Telling a collaborator to run claude /login names a remedy on a machine they have no access to, and names nobody who can. And a Display impl in a channel can carry paths or hosts to anyone with read access.

Each notice now says what happened, what it means, and what happens next. The raw error stays in the log.

3. Discarded work was uncounted

Two paths in EventQueue::push drop a request someone sent — dedup-on-in-flight (at debug level) and the depth cap, which evicts the oldest event, so the request that waited longest is the one lost. Neither kept a running total, so "did we lose anything?" was unanswerable.

Both now increment a DropCounts, logged with the running total the way relay.rs already accounts for gated_observer_dropped, and a run that discarded anything says so on shutdown.

4. Windows test debris

cargo test -p buzz-acp on Windows leaves ~25 empty files whose names are mangled absolute paths, because the capture writers build a temp path MSYS turns into a filename. They regenerate every run and get swept up by git add -A. Ignored by writer-defined stem; verified all 25 covered and no upstream file matches.

Testing

11 new tests. fmt and clippy clean.

cargo test -p buzz-acp reports 28 failures that are pre-existing on main — verified by stashing and re-running:

passed failed
clean main 800 28
with this branch 811 28

QuicksilverSlick and others added 4 commits August 31, 2026 10:39
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>
`cargo test -p buzz-acp` on Windows drops ~25 empty files into the crate
root whose NAMES are mangled absolute paths, e.g.
`crates/buzz-acp/C<U+F03A>UsersPCOWNE~1AppDataLocalTempbuzz-acp-steer-capture...`.
The capture writers build a temp path that MSYS translates into a
filename rather than a directory.

They regenerate on every run, so `git add -A` sweeps them into whatever
commit comes next — which is how 25 of them ended up in a rename commit.
Ignoring them is the fix that survives; deleting them is not.

Matched on the writer-defined stem rather than the mangled path, since
the mangling is machine-specific and the leading character is not ASCII.
Verified: all 25 are covered, and no file tracked on upstream/main
matches either pattern.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@QuicksilverSlick
QuicksilverSlick merged commit 8f2e56d into main Aug 31, 2026
@QuicksilverSlick
QuicksilverSlick deleted the fix/acp-silent-failures branch August 31, 2026 22:11
QuicksilverSlick added a commit that referenced this pull request Sep 3, 2026
The upstream merge adopted `InboundAuthorGate` wholesale for logging while
restoring only the in-channel notice, which left two observability
regressions against #2:

- The author-gate refusal logged at `debug!` again. The notice reached the
  sender, but the line the owner would grep for did not exist in practice.
  Restored to `warn!`, with the reasoning comment. Because it now lives in
  the gate rather than one call site, both production listeners are covered
  — the fork's original only covered the normal listener.
- The no-rule drop lost its `channel_id` and `kind` fields, leaving a bare
  message that says something was dropped but not what or where. The event
  is moved into the gate before that point, so the fields are captured
  alongside the notice inputs and logged from there.

cargo check clean; cargo test -p buzz-acp unchanged at 885 passed / 29
failed, matching the pristine upstream/main control.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant