Chat-first agent creation via portable buzz CLI drafts#1878
Conversation
dd5e4d0 to
4bcbf1c
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Requesting changes before this can meet the 9/10+ bar.
-
The new CLI does not pass the repository's required Rust lint gate. Both
Rust LintandWindows Rustfail onRespondToArg::to_wire(self)(crates/buzz-cli/src/lib.rs:235) withclippy::wrong_self_conventionunder-D warnings. This is introduced by this PR and must be fixed rather than merged red (for example, make the enumCopyor rename/change the receiver as appropriate). -
Ephemeral draft requests can be silently lost during Desktop startup/query refresh.
handleRelayObserverEventdrops a frame unless its agent is already inknownAgentPubkeys, and the management listener independently drops it unlessmanagedAgentsQuery.datais already populated. Kind-24200 requests are ephemeral and there is no queue/replay path, so a validbuzz agents draft-createcan print “Draft sent” while Desktop discards it if it arrives before the managed-agent query/effects settle. The listener is also torn down/re-added whenever that query data changes. Please make acceptance depend on an initialized, stable ownership source or buffer candidate requests until ownership is known, and add a test for a request arriving before managed-agent data resolves.
Security direction is otherwise good: verified NIP-OA binding, owner encryption, signed-agent/tag matching, parser allowlists, shared-channel authorization at save, and owner-confirmed write gates. I also verified the focused Rust contract tests (3/3) and the full Desktop helper suite (2834/2834). The branch is one current-main commit behind (#1826), so rebase and rerun CI before approval.
wpfleger96
left a comment
There was a problem hiding this comment.
Code Review — PR #1878
Scope: 39 files, +2102/−759 across Rust (buzz-cli, buzz-acp, buzz-agent, buzz-dev-mcp, nest) and Desktop TS (agent management, channels, welcome flow, E2E).
Three parallel review passes (Rust, Desktop TS, Security) plus manual source-level verification of all key files.
CI
Previous run @ 4bcbf1cb had Clippy wrong_self_convention on RespondToArg::to_wire — fixed in dcca7cb7 (added Copy derive, correct fix). E2E failures (global-agent-config-screenshots:119, home-collapsed-top-chrome:9) are pre-existing flakes on unmodified files. New CI run pending at dcca7cb7.
IMPORTANT — F1. Channel co-membership check deferred to submit-time
desktop/src/features/agents/useAgentManagement.ts:~86–95
The subscribeAgentManagementRequests callback checks isOwnedAgent and deduplicates, but does NOT call assertAgentCanActFromOrigin(channelId) before setRequest(next). That check fires only inside submitCreate/submitUpdate (~118–134). An owned agent in channel X can publish a draft with channelId = channel Y (neither agent nor owner is a member), the form opens pre-filled, and only fails on Submit.
No write occurs (the gate works), but the agent can render arbitrary text in the owner's UI before consent.
Fix: Move assertAgentCanActFromOrigin into the subscriber before setRequest(next) — silently drop or toast the out-of-scope draft.
IMPORTANT — F2. Update draft targets persona by display name — scope extends beyond self-update
desktop/src/features/agents/useAgentManagement.ts:~108–116
matchingPersonas resolves by case-insensitive displayName across ALL owned editable personas. assertAgentCanActFromOrigin validates the drafting agent's channel membership, not the target persona's agent. Agent in channel X can draft an update for any persona by display name, even if that persona's agent is only in channel Y.
Fix: Validate the target persona's managed agent pubkey matches sourceAgentPubkey.current (self-update only), or check the target agent is a member of the request's channel.
IMPORTANT — F4. Single-slot pending draft queue with no expiry
desktop/src/features/agents/useAgentManagement.ts:~93
if (pendingRequestId.current === null) gates all new requests. If the dialog is never closed, no management requests are processed for the session. A rogue owned agent can park a blocking draft. Consider a timeout or newest-supersedes-oldest policy.
MINOR
- M1. No secondary size bound on Desktop receipt (CLI enforces 120/20k chars, Desktop parser doesn't re-validate lengths).
- M2.
BUZZ_AUTH_TAGnow passes to all MCP subprocesses viaPASSTHROUGH_ENV— document the trust model decision for third-party MCP servers. - M3. Several explanatory comments in
ChannelScreen.tsxremoved as incidental cleanup unrelated to this PR. - M4.
rustls::crypto::ring::default_provider().install_default()inbuzz-dev-mcp— a one-line comment on why (CLI's HTTPS client needs crypto provider via MCP path) would help. - M5. E2E
readCommandLogglobal state relies on bridge mock logging contract — silent false-pass if contract changes.
Positives (not padding — genuinely strong)
- Parser allowlist (
hasOnlyKeysinagentManagement.ts) correctly blocks injection of extra fields (runtime, provider, model, respondTo, apiKey) in create drafts. - AddChannelBotDialog simplification (−759 lines) consolidates all creation through the canonical form — this is the right direction.
- CLI draft publishing (
agent_management.rs) has solid test coverage: contract, validation, rejection. Encrypted owner-scoped events, UUID validation, proper error handling. - Channel attachment partial success (
useCreatedAgentChannelAttachment) is well-designed: retry only attempts attach, never recreates. - Clippy fix in
dcca7cb7is correct (Copy on two-variant enum).
Blocking: F1, F2. F4 is borderline (stuck-UI, not auth).
|
🤖 Follow-up to @wesbillman’s review submitted July 14 at 23:08 UTC, which contained two requested changes:
Added focused tests proving a request is buffered before ownership resolves, then accepted for an owned agent or rejected for an unknown agent. Local typecheck, Biome, file-size checks, parser/buffer tests, clippy, and pre-push Rust/Desktop/Tauri/mobile gates pass. This comment predates and does not summarize @wpfleger96’s later review; those findings are being handled in their individual inline threads. |
wpfleger96
left a comment
There was a problem hiding this comment.
Thorough review of the chat-first agent creation flow. The architecture is well-structured — the CLI draft layer, allowlist parser, and AddChannelBotDialog simplification are all solid. Two IMPORTANT findings on the channel/agent validation path, plus notes below.
ad70e1b to
f6d6cf1
Compare
|
🤖 @wpfleger96 Follow-up to your code review: we worked through every finding individually; the detailed decisions and changes are in the now-resolved inline threads.
CI is fully green and there are no unresolved review threads. |
What this does
Makes creating your first agent a guided, chat-first experience — and moves conversational agent drafting onto the portable
buzzCLI so it works across agent runtimes.Welcome channel: chat-first agent creation
Conversational agent drafts via
buzzCLI (portable across runtimes)buzz agents draft-createandbuzz agents draft-update.create_agent/update_agentMCP tools that only worked for runtimes receivingbuzz-dev-mcp(Buzz Agent, Codex). The CLI path works for Claude Code and Goose too via their native shell tools.buzz-cliskill teach every runtime: ask only for name and what it should do — no runtime/provider/model/access questions. Desktop resolves machine defaults; new agents default to Only me.BUZZ_AUTH_TAGnow passes through Buzz Agent's MCP shell so drafts work there.Any owned agent can draft (not just Fizz)
agent_management_request).Channel attachment partial success
Form polish
respondTo, which was forcing it open).Verification
just ci-equivalent gates green on pre-commit/pre-push (rust, desktop, tauri, mobile tests).add_channel_membersretries without a secondcreate_managed_agent), agent picker states.