fix(cli): stop set-add-policy erasing the agent profile - #6576
Draft
cyberzero000 wants to merge 1 commit into
Draft
fix(cli): stop set-add-policy erasing the agent profile#6576cyberzero000 wants to merge 1 commit into
cyberzero000 wants to merge 1 commit into
Conversation
cyberzero000
marked this pull request as draft
August 23, 2026 18:37
cyberzero000
force-pushed
the
fix/cli-set-add-policy-profile
branch
from
August 24, 2026 00:47
1cbc8af to
5e74b42
Compare
cyberzero000
marked this pull request as ready for review
August 24, 2026 00:55
cyberzero000
marked this pull request as draft
August 26, 2026 18:13
cyberzero000
force-pushed
the
fix/cli-set-add-policy-profile
branch
2 times, most recently
from
August 28, 2026 23:38
10f6e21 to
ad52699
Compare
🔐 Codex Security Review
|
cyberzero000
force-pushed
the
fix/cli-set-add-policy-profile
branch
from
August 29, 2026 03:28
ad52699 to
727336f
Compare
`cmd_set_add_policy` published a kind:10100 whose content was only
`{"channel_add_policy": ...}`. The kind is replaceable, and the relay
projects just that one field into a column
(`crates/buzz-relay/src/handlers/side_effects.rs`) — `channel_ids`, `name`,
`display_name` and `respond_to` live solely in the event body clients read.
One policy change wiped the rest of the profile, which took the agent out of
every channel's @mention picker until an operator republished by hand.
Read the current profile, merge the field, republish. Three things the merge
has to get right, each covered by a test:
- A result this command cannot fully read is an error rather than an empty
profile. A body that is not a JSON object is refused, and so is a result
row missing a readable `content` or `created_at`. Treating either as
absent would republish a single-field profile and cause the exact wipe
this fixes.
- The write out-bids the stored copy's `created_at` instead of tying it.
`Db::replace_addressable_event` breaks a same-second tie by lowest event
id, so publishing at `now` is a coin flip against a peer republishing the
profile concurrently, and a deterministic loss against a copy stamped
later by a skewed peer.
- The lead carries headroom under the relay's 900s tolerance. That window is
measured against the relay's clock and this budget against ours, so a copy
accepted at `S` when relay time was `R` proves only `S - R <= 900`, and
out-bidding it at `S + 1` can sit up to 901s from relay time.
Read-modify-write on a replaceable event has no compare-and-set, so this
re-reads after publishing. Comparing the stored head's id to ours is not
enough on its own: kind:10100 is global, so the filter carries neither a
channel pin nor an `until`, which makes it `RoutePredicate::Bounded` and lets
the relay serve it from a read replica whenever
`BUZZ_REPLICA_READ_MAX_AGE_MS` is set. That budget bounds how recently the
replica proved its replay position, not whether a write from a moment ago is
visible, so a read issued straight after the publish can return the pre-write
event.
The relay's own replace rule separates that lag from a real loss: a stored
copy that loses the `created_at`/lowest-id comparison to the event we just
published cannot have replaced it. So the read-back is classified three ways:
- Landed — our event is the stored head. Success.
- Replaced — a copy that beats ours is stored. Re-merge onto it and publish
again, and return exit 5 if every attempt is replaced.
- Unconfirmed — the read cannot see our write, or it failed. The relay
accepted the event and nothing that could have replaced it is stored, so
this is a success we could not confirm: exit 0 with the documented
`{event_id, accepted, message}` plus an additive `warning`. Reporting a
conflict here would tell a caller to retry a change that had already
landed, and the retry would rebuild the same body at the same stamp, come
back `duplicate:`, and look like a conflict again.
A confirmation read that fails outright is Unconfirmed rather than the
command's error: the mutation has already happened by then, so borrowing the
read's error reported a stored policy change as a network failure.
Profile-read failures also keep their `CliError` variant instead of
flattening to `Other`. `exit_code` and `is_retryable_error` classify by
variant, so flattening reported a retryable 503 as exit 4 with
`retryable: false`, and an expired `BUZZ_AUTH_TAG` as exit 4 instead of 3 —
telling an agent to abandon both as permanent.
The window is narrowed, not closed: a peer publishing between our read and
our write still loses its change, because a replaceable write carries the
whole body and nothing records what we replaced.
Signed-off-by: cyberzero000 <user1@cyberzerosystems.com>
cyberzero000
force-pushed
the
fix/cli-set-add-policy-profile
branch
from
August 29, 2026 03:58
727336f to
aa9f5da
Compare
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
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.
buzz channels set-add-policypublishes a kind:10100 whose content is only{"channel_add_policy": ...}(crates/buzz-cli/src/commands/channels.rs). Thekind is replaceable, so that one field becomes the entire profile body.
The relay projects
channel_add_policyinto a column(
handle_agent_profile,crates/buzz-relay/src/handlers/side_effects.rs:1219)and stores the rest only as event content. Clients read the remaining fields
straight out of that content:
agents_from_events(
desktop/src-tauri/src/nostr_convert.rs:445) parsesname,display_name,agent_type,capabilitiesandstatusfrom the profile body, and those arewhat a policy change destroys. The agent then falls back to its npub as a
display name and shows empty capabilities in the directory until someone
republishes the profile by hand.
Scoping this accurately, since it is narrower than it first looks:
channel_idsis not affected.
relay_agents_from_legacy_events(
desktop/src-tauri/src/nostr_convert/agent_directory.rs:50) clears it on everykind:10100 entry — "channel membership is authoritative only in relay-signed
kind:39002" — and the directory refills it from membership events, so mention
eligibility never depended on the profile copy.
respond_tois likewise not akind:10100 field; it comes from owner-signed kind:30177 records.
Worth flagging for review:
set-add-policyis currently the only writer ofkind:10100 in this repo, so the fields it preserves are ones other publishers
put there. That is an argument for the merge, not against it — a command that
owns one field should not be the reason every other publisher's field
disappears.
Now it reads the current profile, merges the field, and republishes.
Three decisions the merge has to get right
Each is a pure function with tests, because getting any of them wrong
reintroduces the wipe.
A result the command cannot fully read is an error, not an empty profile.
parse_stored_profilereturns an empty body only when the identity genuinelyhas no profile. A body that is not a JSON object is refused rather than
replaced, and so is a result row missing a readable
contentorcreated_at—treating either as absent would republish a single-field profile and cause the
exact wipe this fixes.
The write out-bids the stored copy's
created_atrather than tying it.Db::replace_addressable_eventbreaks a same-second replaceable tie by lowestevent id, so publishing at
nowis a coin flip against a peer republishing theprofile in the same second, and a deterministic loss against a copy stamped
later by a skewed peer. Retrying at
nowcannot win either.The lead is bounded with headroom under the relay's 900s tolerance. Stamping
stored + 1is only ever one second ahead of whichever writer produced thestored copy, so the lead measured against our own clock is just this host's skew
from that writer's — and refusing on a few seconds of it made the command
unusable wherever a peer's clock ran ahead of the operator's. The relay's window
is measured against its clock and this budget against ours, so the two are not
directly comparable: a copy accepted at
Swhen relay time wasRproves onlyS - R <= 900, and out-bidding it atS + 1can sit up to 901s from relaytime. The 600s budget leaves room for that overshoot.
Confirming the write
Read-modify-write on a replaceable event has no compare-and-set, so this
re-reads after publishing.
duplicate:means the relay took the write androlled it back — it arrives as
accepted: true, so reading onlyacceptedreports success for a discarded write.
Comparing the stored head's id to ours is not enough on its own. That read
cannot be pinned to the primary: kind:10100 is global, so the filter carries
neither a channel pin nor an
until, which makes itRoutePredicate::Boundedand lets the relay serve it from a read replica whenever
BUZZ_REPLICA_READ_MAX_AGE_MSis set. That budget bounds how recently thereplica proved its replay position, not whether a write from a moment ago is
visible — so a read issued straight after the publish can legitimately return
the pre-write event.
The relay's own replace rule separates that lag from a real loss: a stored copy
that loses the
created_at/lowest-id comparison to the event we just publishedcannot have replaced it. So the read-back is classified three ways:
warning, exit 0Unconfirmed is a success because the relay accepted the event and nothing that
could have replaced it is stored. Calling it a conflict instead would republish
a byte-identical event, draw
duplicate:, and return exit 5 withretryable: truefor a policy change that was already stored — a callertreating 5 as "retry me" would loop on a landed write. Its stdout keeps the
documented
{event_id, accepted, message}shape with an additivewarningkey,the same way the create commands inject a new entity id.
A confirmation read that fails outright is Unconfirmed too, not the command's
error: the mutation has already happened by then, so borrowing the read's error
would report a stored policy change as a network failure.
Exit 5 is still returned when the profile is genuinely replaced out from under
us on every attempt.
The window is narrowed, not closed: a peer publishing between our read and our
write still loses its change, because a replaceable write carries the whole body
and nothing records what we replaced.
Error classification
Profile-read failures keep their
CliErrorvariant instead of flattening toOther.exit_codeandis_retryable_errorclassify by variant, and thosecodes are the CLI's agent-facing contract, so flattening reported a retryable
503 as exit 4 /
retryable: falseand an expiredBUZZ_AUTH_TAGas exit 4instead of 3 — telling an agent to abandon both as permanent.
Deliberately not done
Three things this PR leaves alone on purpose, so a reviewer does not have to ask:
--forcefor a malformed stored profile. The relay does not validatekind:10100 content shape (
handle_agent_profileis a side effect whosefailures are only logged), so another client can leave a non-object body
stored, which this command then refuses. A flag is a wider change than the
bug warrants; the refusal message now names a recovery path that exists
(sign a corrected kind:10100 and submit it to
POST /events).created_atis guarded withcontent, not separately. One check coversthe whole unreadable-row case rather than two independent guards for one
malformed response.
MAX_PROFILE_PUBLISH_LEAD_SECSstays at 600. The 901s worst case aboveneeds a stored copy ~10 minutes in the relay's future plus skew at the exact
boundary. The constant is fine; only the comment claiming it "sits well
inside" the relay window was wrong.
Testing
cargo test -p buzz-cli— 375 passed, 0 failed. Twelve new tests.Mutation-checked, each mutation caught by exactly the test that covers it:
a_read_that_cannot_see_our_write_is_not_a_conflictCliError::Othera_transient_profile_read_failure_keeps_its_exit_codeunwrap_ordefaults forcontentandcreated_ata_result_row_we_cannot_read_is_refused_rather_than_treated_as_absentan_unreadable_profile_is_refused_rather_than_replacednowinstead of out-biddinga_merge_out_bids_the_stored_copy_instead_of_tying_it,a_stored_stamp_past_the_relay_window_is_refused_not_out_bidcargo clippy -p buzz-cli --all-targets -- -D warningsandcargo fmt --checkclean.
just file-size-checkclean.Against a live relay
Unit tests cover the merge as a pure function; this is the end-to-end check
against a running relay, run twice from the same seeded state — once with the
binary built from
4baccd539(this PR's merge-base) and once with this branch.A throwaway keypair was seeded with a populated kind:10100 signed directly to
POST /events, since no CLI subcommand writes that kind except the one undertest:
{"channel_add_policy":"anyone","channel_ids":["aaaaaaaa-0000-4000-8000-000000000001"],"name":"probe","respond_to":["mention"]}Then
buzz channels set-add-policy --policy owner_only, and the profile readback:
Also exercised on the same relay:
{"channel_add_policy":"nobody"}, exit 0, so the merge does not break theempty-profile path;
set-add-policycalls back to back inside one second — bothaccepted,both exit 0, last policy stored, no spurious exit 5.
Note
Split out of #5806, which found this while debugging why an agent answered
@mentions in one channel and was silent in another. It stands alone and has nodependency on the rest of that branch.