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 👍 / 👎.
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 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.