broker: trusted-operation broker foundation with agents.* as its first capability - #6543
Open
baxen wants to merge 5 commits into
Open
broker: trusted-operation broker foundation with agents.* as its first capability#6543baxen wants to merge 5 commits into
baxen wants to merge 5 commits into
Conversation
Adds the request/result envelope for the Buzz trusted-operation broker,
with agent CRUD as its first capability. This is protocol and validation
only -- no host, no dispatch, no execution.
The envelope is deliberately minimal:
{ type, protocolVersion, requestId, capabilityVersion, capability, args }
It carries no owner, requester, or relay identity. Those are derived on
the host from the verified frame, because a request body that could name
its own owner would let any signer act on another owner's agents. There
is no `context` and no `authorization` field yet: a field that looks
security-bearing while enforcing nothing is worse than its absence. One
gets added, as a discriminated object, when a real grant format and
verifier exist.
There is no client-computed digest. Idempotency is decided host-side, so
only the host needs the hash -- shipping a second canonicalizer in a
second language would freeze two implementations that must agree
byte-for-byte forever. Retrying means resending identical bytes.
Results are a three-variant discriminated union so invalid combinations
are unrepresentable rather than merely discouraged:
Succeeded { outcome } | Failed { error } | Indeterminate { error }
Indeterminate is distinct from Failed on purpose. Failed promises no side
effects took hold; Indeterminate promises nothing and demands
reconciliation.
Capabilities stay three separate names (agents.create/update/delete)
rather than one agents.manage action union, so a host can permit one
without permitting the others. Only business operations are addressable:
signing, publishing, credential access, and tool execution are not
capabilities and cannot be named.
channelId appears only on create, where attachment genuinely needs it.
Update and delete identify their target by the agent itself.
Every args type is deny_unknown_fields, so a smuggled api key or nsec
fails to deserialize instead of reaching a mutation, and outcome types
can structurally hold only public identifiers.
Signed-off-by: ss-dev-00 <a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971@buzz.block.builderlab.xyz>
Co-authored-by: Bradley Axen <baxen@squareup.com>
Signed-off-by: Bradley Axen <baxen@squareup.com>
The trusted path lives in one Rust function: verified frame -> authorize -> validate -> claim -> dispatch -> complete It is a single function rather than coordinated steps because every stage boundary is somewhere a caller could otherwise substitute its own answer for "who is asking" or "has this already run", and a crash between two coordinated steps is a side effect nobody recorded. Identity arrives as VerifiedRequest, whose construction asserts that owner, requester, and relay scope came from the verified transport. Nothing in the request body can influence them, so a signer cannot name someone else as owner. A payload that tries fails to deserialize. The digest is computed here, over the exact decrypted bytes, after a size bound. No canonical encoding has to be agreed on and no second implementation can drift from this one. The execution log is keyed by (relay_scope, owner, requester, capability, request_id) and inserts `executing` before the first side effect. Terminal rows replay; a digest mismatch is a conflict, checked before state so a conflicting retry can never be answered with the first request's result; an existing `executing` row becomes `indeterminate` and is never blindly re-executed. Claim runs in an IMMEDIATE transaction -- a test races eight threads to show exactly one winner. This guarantees at-most-once plus indeterminate. Not exactly-once, and not durable completion: the log can prove execution started, never how far it got. Reconciling partial effects belongs to the capability that knows its own phases. Authorization is split from authentication. The broker proves who signed; whether that signer may manage agents is an agents.* scope rule, so a future capability cannot inherit it. The rule is that the requester must be one of this owner's managed agents, read from the authoritative Rust roster. It fails closed when the roster cannot be read, and refuses the owner key itself as a requester. Per the approved design, there is no channel restriction on the target of an update or delete: an owner's agent is theirs regardless of channel, and requiring co-membership would look tighter than it is while adding no real constraint. Channel appears only on create, where attachment needs it. Signed-off-by: ss-dev-00 <a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
The three capabilities dispatch to an AgentService trait rather than to the Tauri commands directly. Those commands need an AppHandle and a State, so depending on them here would make every handler untestable without booting an app and would entangle the capability layer with the desktop runtime. Behind the seam, handlers are pure and the tests below drive the real pipeline against a fake. The seams are async because the mutations they will call are: minting a key, writing the agent store, and publishing to the relay all await. That forces ExecutionLog to be a trait instead of a borrowed rusqlite::Connection, which is !Sync and must not be held across an .await. The production adapter confines each connection to one blocking task; the test double serializes behind a Mutex. Cross-call exclusion is SQLite's either way, since claim() does its read and insert inside a single IMMEDIATE transaction. ServiceError distinguishes Failed from Unknown. Failed promises no side effects persisted; Unknown promises nothing and becomes indeterminate. A service that mutated and then lost track must say Unknown, because reporting a clean failure would invite a retry of something that may already have happened. Nothing in AgentService returns secret material: a create reports the new agent's pubkey, never its nsec. A test serializes a create response and asserts the absence of nsec, privateKey, private_key, and secret, so the guarantee is checked rather than asserted in a comment. Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
Implements `AgentService` and `AgentRoster` against the same commands the UI calls — `create_managed_agent`, `update_managed_agent`, `delete_managed_agent` — rather than reimplementing the mutations. Two agent creation routines would drift, and the broker's would be the one nobody looks at. The credential boundary is narrowed here: `create_managed_agent` returns the minted `private_key_nsec`, and this is the one stack frame that sees it. The outcome type has no field that could carry it, so the secret cannot reach the pipeline, the execution log, or the wire. Request translation is pure and tested directly, because the policy it encodes is worth pinning: - A brokered create neither spawns a process nor sets `start_on_app_launch`. A remote requester asking for an agent to exist has not asked for a local process to run on a schedule nobody at the keyboard chose. - It links no definition or team — that would pin someone else's config snapshot onto the new agent, a decision the request never made. - An update sends `None` for every field the request did not name. The broker has no "clear to default" verb, so it never emits the explicit null that would wipe stored config. - A requested runtime is resolved through the catalog before it can become a stored harness pin, and an unknown id fails here instead of surfacing later as an unrelated spawn error. - A delete leaves `force_remote_delete` unset, so the deployed-remote guard still holds: orphaning provisioned infrastructure stays a human decision. Channel attachment is its own failable step after the agent exists, so it maps to `Unknown` rather than `Failed` — telling a requester nothing happened while an agent sits in the roster would be the worse lie. Name targets resolve case-insensitively but must be unambiguous. Two agents can legitimately share a name, and guessing would mutate or delete an agent the requester never identified. The roster refuses to answer for an owner this host does not hold keys for, rather than authorizing against the wrong agent set. Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
Closes the loop from a signed relay event to a signed result frame. The pipeline was already testable in isolation; this attaches the transport and the host's real parts, so nothing above the ingress layer learns what Tauri or the relay look like. `ingress::verify_frame` derives every trusted field from the frame rather than from anything a caller could assert. Owner, requester, and relay scope are transport-derived, which is why the wire envelope has no field for them: a payload that could name its own owner would be a payload that could pick its own authority. Verification refuses duplicate routing tags — a frame with two `agent` tags has two readings, and the one this host picks would not have to match the one the relay routed on. Freshness is re-checked here rather than trusted from the relay, because bounding replay of an old signed frame is not a hop this host controls. A frame that fails verification gets no answer at all. Emitting a signed refusal at an unverified sender would make any host a signing oracle aimed at a target of the sender's choosing. Frames that are simply not broker traffic — the overwhelming majority, since ordinary telemetry arrives on this same subscription — are ignored cheaply. `host.rs` keeps the command a transport hand-off rather than an authority hand-off: it takes a raw signed event and nothing else, so the renderer cannot say who is asking or which owner to act as. The return value is deliberately thin, since the requester's real answer travels in a signed frame; reporting an outcome through IPC would make the reply path look like a return value it is not. Results publish over the relay WebSocket because kind-24200 is rejected on the HTTP bridge, and as the owner's own identity with no NIP-OA auth tag — an auth tag is how a managed agent proves owner backing, and the owner is not one. On the TypeScript side the renderer forwards the *signed event*, never the decrypted plaintext, so the host verifies and decrypts independently. The routing check is shallow on purpose: it decides only whether a payload claims to be a broker request, not whether it is valid, so an ill-formed request still reaches the host and earns a structured refusal instead of silence. A broker request also returns before the session journal, because a payload with no seq, timestamp, or kind must not be filed as a transcript entry. Forwarding is fault-isolated: a broker host fault is not an observer transport fault, and must not be reported as one or tear down telemetry for every agent over a single failed mutation. `VerifiedRequest` gets a hand-written `Debug` that redacts the payload to a byte count. Decrypted request content includes system prompts, and a derived `Debug` would put them into any error or test failure that formats a request. Verified at this tree: 2916 desktop Tauri Rust tests (74 broker, 18 ingress), 5311 desktop TS tests, workspace clippy and both fmt gates clean. The two `buzz-pair-relay` integration failures are a pre-existing timing flake, reproduced on clean origin/main at 0745612 and independent of these crates. Signed-off-by: ss-dev-00 <a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1938c1dbbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+68
to
+70
| .await | ||
| .map_err(|error| format!("failed to publish broker result: {error}"))?; | ||
| Ok(()) |
There was a problem hiding this comment.
When the relay responds with a valid NIP-01 OK carrying accepted: false—for example because the owner-agent binding is stale or the frame is rejected by relay policy—buzz_ws_client_pkg::publish_event still returns Ok(OkResponse), and this code discards accepted and reports successful delivery. The mutation has already executed, but the requester receives no result and resultDelivered is incorrectly true; retries merely replay the result through the same rejected path. Inspect OkResponse::accepted and convert a negative acknowledgement into a delivery error.
Useful? React with 👍 / 👎.
Comment on lines
+299
to
+305
| let response = crate::commands::update_managed_agent( | ||
| request, | ||
| self.app.clone(), | ||
| self.app.state::<AppState>(), | ||
| ) | ||
| .await | ||
| .map_err(ServiceError::Failed)?; |
There was a problem hiding this comment.
When update_managed_agent returns an error after committing part of an update, this conversion records a terminal Failed result even though that status promises no persistent side effects. A concrete case is changing respondTo on a running local agent: agent_models_update.rs saves and publishes the new policy, then returns Err if restarting the runtime fails, leaving the policy changed. The broker will persist and replay a clean failure, misleading the requester and preventing reconciliation; post-commit command errors must map to ServiceError::Unknown or otherwise expose whether the mutation committed.
Useful? React with 👍 / 👎.
| if (isBrokerRequestPayload(parsed)) { | ||
| // Isolated: a broker host fault must not be reported as — or tear down — | ||
| // the observer telemetry subscription. See forwardBrokerFrameIsolated. | ||
| await forwardBrokerFrameIsolated(event); |
There was a problem hiding this comment.
If a community switch begins after the generation check but while this awaited IPC call is starting, resetting eventProcessingQueue does not cancel the already-running handler. The Rust command then re-reads the newly active relay URL, so a frame received from community A can be authorized against the global managed-agent roster and executed/logged/published in community B; agents.create can even create the local record before its A channel ID fails attachment on B. Pass the subscription's captured relay scope to the host and fail closed if it no longer matches the active workspace, or recheck cancellation before execution.
AGENTS.md reference: AGENTS.md:L537-L540
Useful? React with 👍 / 👎.
baxen
added a commit
that referenced
this pull request
Aug 27, 2026
Addresses the interface half of #6467. This is the contract for how a keyless agent asks a host to act on its behalf — and only that. No host, no transport, no signing, no Desktop code; the only implementation is a test double. An agent that holds no secret key still has to read its channels, answer a mention, react, keep its profile, address its memory, and create the agents it needs. This PR defines the vocabulary for those requests and the rules both sides must follow. **#6467's open question, answered: per-operation, not "sign this".** A raw signing primitive tells the host *who* is asking but not *what for*, so the only policies it can express are all-or-nothing. A closed set of named operations lets a host serve channel reads while refusing agent creation, and gives any later policy layer something to attach to. The cost is that adding an operation means changing the contract — which is the point. Signing, publishing, and the auth/encryption NIPs are deliberately *not* operations; they are mechanisms inside whoever holds the key, and a test pins that none of those names resolve. **Nine operations in v1:** read a channel (whole channel, one thread, or mentions-only — these differ by filter, not permission, so they share one operation), post, reply, react, set profile, derive a storage address, and create / update / delete agents. Presence and typing are deferred as housekeeping a host may decline regardless; streaming is deferred, so waking on a mention is a polled mentions-only read. Each operation has typed, validated arguments and a typed outcome. **What the contract enforces structurally rather than by instruction:** - *Retry means identical bytes.* A request is validated and serialized exactly once and the transport only ever sees that frozen form. There is no way to re-render it, so a retry can't drift and be mistaken for a conflicting request. - *Read cursors are opaque.* Host-issued strings, validated for shape only, never parsed or compared. The host owns ordering and paging stability instead of being locked into timestamps. - *Reads return signed Nostr events.* A keyless agent verifies authorship and content locally and trusts the host only for completeness and authorization. Reply ancestry and mentions are derived from the signed tags, so nothing can disagree with the signature. - *Validation is the only door to a response.* The transport primitive hands back an unjudged envelope; the only response a caller can actually obtain is one already checked against the request it answers — request id correlates, the outcome's operation matches, identifiers and cursors are well-formed. A mismatch counts as "no answer", not a failure verdict. The raw primitive can't be invoked from outside the crate (pinned by compile-failure checks), so skipping that step isn't an option. - *Authority is never asserted by a body.* The envelope has no requester, owner, scope, or credential field; all of that comes from the session the host authenticated. Ownership of a created agent is implicitly the requester, never a field — which is what lets agents own agents while the chain still ends at a human. - *No secret crosses the wire.* Agent creation returns a pubkey and handle, never the minted key. Every payload type rejects unknown fields, and a test pins the exact JSON key set of all eighteen argument and outcome types, so no field can be added without a reviewer touching that table. Two honest limits: a string can physically hold secret text, so keeping secrets out of message content is host policy; and nothing stops a host from *holding* keys — it just may not hand them over. - *Null is never legal; omission is the only spelling of absence.* Every optional member rejects an explicit null. Otherwise null and omitted would be indistinguishable, and a contradiction check on the response envelope could be skipped by sending one (a real hole in an earlier revision). Coverage is derived by walking every path in the real fixtures, so new optional members are covered automatically. Serializers that emit null by default (Go without `omitempty`, Python's default dump) need configuring — called out in the module docs. **Wire mapping.** One POST endpoint, JSON body is the frozen request, opaque bearer credential in the auth header. Every verdict the host reached — including failure and including a rejected credential — comes back as a well-formed envelope with 200, because a second copy of the verdict in the status line could only disagree with it. Clients still try to parse an envelope regardless of status (intermediaries may remap) and fall back to the status only when none is present. A rejected credential is a host verdict carrying the promise "the action did not run"; transport errors are reserved for "nothing can be concluded about side effects." Binding the credential to a specific agent and conversation is the host's job and deliberately unenforceable here. **Relationship to #6543.** Envelope, request-id rules, protocol version, the three-way succeeded / failed / indeterminate result, error codes, and the agent-management argument shapes carry over. Changed deliberately: pubkeys are a validated type rather than bare strings, "capability" became "action" throughout, and a new "unsupported" error lets a host decline a best-effort operation without it reading as a fault. Both PRs land in the same module path — whichever merges second needs a hand reconcile, which I own as the author of #6543. **Validation at `7c853e0fb`.** 299 tests in the sdk crate (full package run), workspace tests green apart from the pre-existing relay timeout flake, which I reproduced on unmodified `main`. Clippy with warnings denied, fmt, warning-free docs, and the pre-push gate all pass. Review findings were closed with mutation tests — each fix verified by reverting it and watching a named test fail. One of those exposed a gap in my own coverage (response-side fields weren't pinned); that's fixed and floored separately from the request side so neither can mask the other. **Size.** About 5,400 lines across 8 files, roughly half tests. Every source file is under the repo's 1,000-line cap except the test file; #6759 splits it mechanically, is stacked on this branch, retargets to `main` after merge, and lands before PR 2 of the stack. --------- Signed-off-by: ss-dev-00 <a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com> Signed-off-by: ss-core-02 <cca2c695c61d409321106cc399009c2cc674d72cc8900b2999a68fe54577a411@buzz.block.builderlab.xyz> Co-authored-by: ss-dev-00 <a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971@buzz.block.builderlab.xyz> Co-authored-by: ss-core-02 <cca2c695c61d409321106cc399009c2cc674d72cc8900b2999a68fe54577a411@buzz.block.builderlab.xyz>
|
The roster rule is the part I'd want a second look at, given your stated review focus.
I don't think that's wrong for PR 1 — the boundary you're defending here is authentication, and it holds. But it does mean "authorized" currently carries no notion of standing, and since So the question: is graduated authority meant to arrive later as the Asking because — Jenkins, Autropic (tom@autropic.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
2 participants
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 is
The start of a general Buzz trusted-operation broker: a way for a requester who cannot hold the owner's credentials to ask the owner's host to perform a small set of named business operations. Agent CRUD (
agents.create,agents.update,agents.delete) is its first capability, not its purpose.This replaces the earlier CRUD-specific
agent_crud_requestprotocol. That approach made agent management a one-off Desktop integration; every future trusted operation would have re-litigated authentication, idempotency, and result delivery. Here the broker core owns those once and capabilities plug into it.Desktop is host #1, behind an extractable host/handler boundary — a hosted authority can replace it without the wire protocol changing.
Scope
In: the wire envelope (
crates/buzz-sdk/src/broker/), the host pipeline, durable idempotency, the authorization seam, theagents.*capability family, transport ingress, and result delivery.Deliberately out (PR 2): the broker client and
buzz agents list|get|create|update|delete, structured CLI errors, and tool/skill/prompt guidance. This PR contains no end-user CLI or prompt wiring — nothing here is reachable by a user typing a command yet, which is what keeps it reviewable as a security boundary rather than as a feature.What the broker will never do
Only business capabilities. Never signing on a requester's behalf, never publishing arbitrary events, never credential access, never arbitrary tool or command execution. The capability enum is closed, and
BrokerErrorCode::UnknownCapabilityis the answer to anything outside it.The design decisions worth reviewing
A request cannot name its own authority. Owner, requester, and relay scope are absent from the envelope and derived from the verified transport frame. A payload with an
ownerfield is a payload that can pick its own owner.No
authorizationfield yet. There is no grant format and no verifier, so shipping a security-looking field that enforces nothing would be worse than shipping none. It arrives with its verifier.Authorize before claiming. A refusal leaves no durable record, so an unauthorized requester cannot poison a
requestIdfor the legitimate one.Idempotency is host-side, over the exact received bytes. No client-computed digest and no canonical encoding to agree on. Same
(owner, requester, capability, requestId)+ same digest replays the recorded outcome; same key + different digest is refused as a conflict rather than being answered with someone else's result. The digest is compared before state for that reason.Three outcomes, not two.
Succeeded { outcome }|Failed { error }|Indeterminate { error }as a discriminated union, so "succeeded with an error" is unrepresentable.Failedpromises no side effects;Indeterminatepromises nothing. An interrupted (executing) row becomesindeterminateand is never auto-retried — an ephemeral timeout is not evidence that nothing ran, and treating it as such is how you create two agents.The credential boundary is one stack frame.
create_managed_agentreturns the minted nsec; the outcome type has no field that could carry it, so it cannot reach the pipeline, the execution log, or the wire.Requester authentication reads the authoritative roster, never caller input: the requester must be one of this owner's managed agents. The owner's own key is explicitly not a valid requester. An unreadable roster fails closed.
agents.*scope policy lives with the capability family, not in broker core, so future capabilities do not inherit it.Known limitation
Desktop-offline execution is unsupported. Requests arrive as kind-24200 frames, which the relay routes to connected subscribers without storing. No queue holds a request for an offline host; the requester's wait expires. This is a deliberate limit of the transport, documented in
broker/mod.rs, not an oversight — a durable-delivery capability would need its own transport.Verification
At
1938c1dbb:clippyclean,fmt-check+desktop-tauri-fmt-checkclean, typecheck clean, file-size caps passOne caveat, stated plainly:
cargo test --workspacehas two failures inbuzz-pair-relay's integration tests (test_120s_timeout,test_cancellation_immediate). These are a pre-existing timing flake, not a regression here — they reproduce on cleanorigin/mainat074561233, they vary run to run at a fixed tree (1, 1, 1, 0, 2 failures across five identical runs), andbuzz-pair-relayhas no dependency onbuzz-coreorbuzz-sdkand is untouched by this branch.Review focus
Where the trust boundary sits. Specifically: that nothing above
ingress::verify_framecan be told who is asking, that a verification failure produces no signed output at all (a signed refusal aimed at an unverified sender would make every host a signing oracle), and that the digest-before-state ordering instore::claimreally does make a conflicting retry unable to receive another request's result.