feat: remote agents over SSH — provider contract, buzz-backend-ssh, and where-runs-first - #3449
feat: remote agents over SSH — provider contract, buzz-backend-ssh, and where-runs-first#3449troyhoffman-oss wants to merge 22 commits into
Conversation
Buzz already defines a remote-execution seam: `managed_agents/backend.rs` describes `BackendKind::Provider`, where the desktop discovers a `buzz-backend-<id>` executable on PATH, writes one JSON request to its stdin, and reads one JSON response from its stdout. block#2859 states the gap plainly — that path "has no OSS implementation." This document specifies one, over SSH, ahead of the code that implements it. Everything that follows in this branch is an implementation of this file, so it lands first: a reviewer can judge the design here before reading a line of Rust. It covers the five ops (`info`, `check`, `discover_harnesses`, `probe_models`, `deploy`) and their request/response schemas; the optional `recovery` key, which is optional in both directions so an old desktop and a new provider still interoperate; the configuration surface and host prerequisites; the systemd unit and the env-file contract; the install ordering; and the security invariants the implementation is required to hold — secrets never reaching a log, an argv, or a `Debug` rendering, a deploy without a desktop-minted key failing closed, and trust-on-first-use being confined to addresses the local Tailscale daemon already lists as peers. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
The reference implementation of the `BackendKind::Provider` seam that `managed_agents/backend.rs` already describes and block#2859 says has no OSS implementation. One process per op, no daemon, no state: the desktop writes one JSON request to stdin and reads one JSON response from stdout, and this crate turns that into ssh. Six modules, one protocol implementation: protocol the wire types and the `Secret` that renders `[REDACTED]` ssh the transport; every secret rides the stdin channel tailscale peer enumeration and the re-auth recovery URL discover `check`, `discover_harnesses`, `probe_models` install resolve-or-install for `buzz-acp` and the `buzz` CLI deploy the remote script, the env file, the systemd unit They are one commit because they are one protocol implementation, and because they do not separate: `deploy.rs` reaches into `install` at 43 call sites and `include_str!`s the unit template, and `discover` and `deploy` are mutually dependent (`string_list` one way, `env_map` and `is_well_formed_env_key` the other). Any smaller unit would be a file state that never compiled. Three invariants hold across every op, stated in `main.rs`'s module doc and enforced at named sites: 1. Secrets never reach a log, an error string, a `Debug` rendering, or an argv. They travel only inside the script body on the SSH stdin channel, because the remote `ps` is world-readable. 2. A deploy without the desktop-minted `private_key_nsec` fails closed, inside `Agent::from_request` so no caller can route around it. Env values are written through a control-character guard, because a newline would split the assignment into a second, attacker-chosen line. 3. Tailscale is an enhancement, never a dependency: absent, logged out, or empty, the `info` schema is byte-identical to the plain one. Trust-on-first-use is allowed for exactly one class of address — a device this machine's own Tailscale daemon already lists as a peer, which required a WireGuard-authenticated tunnel to reach. For anything the user typed, host-key checking stays strict. The re-auth URL is constructed, never parsed out of host output, so a compromised host cannot make the desktop open an arbitrary URL. Harness discovery extends upstream's `hermes` preset rather than replacing it. Because a host commonly runs several Hermes profiles out of one install, `discover_harnesses` fans that single preset out into `hermes-<name>` entries carrying `--profile` args, so each configuration appears as its own harness. Profile names are untrusted input on their way into a catalog id and then an argument vector, so they are filtered against Hermes's own charset and capped — enforced twice, once in the script so the bytes are never sent and once on the way back in, because a host is free to ignore the script it was handed. The crate is deliberately NOT bundled with the desktop app. Provider discovery prepends the app bundle's own directory to the search path, so shipping it inside the bundle would give every install an auto-discovered SSH-deploy capability and quietly undermine the "Only use providers from trusted sources" warning the desktop already shows. It installs to `~/.local/bin`, which discovery already covers. 115 tests, no `unsafe`, no `unwrap`/`expect` on a production path. `base64` inherits from the workspace, matching the `sha2` line beside it and the comment above both, which already claimed the inheritance. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
`backend.rs` carried three separable concerns: what a provider is allowed to make the desktop *do*, which files on PATH are providers at all, and invoking one. This splits the first two out and gives provider failures a type that can carry an actionable recovery. - `provider_discovery.rs` — "is this file a provider I may execute?": the `buzz-backend-` prefix, the executability check, id validation, and `resolve_provider_binary`. Pure code motion out of `backend.rs`; every item moves with its doc comments and its tests, and the `managed_agents::*` re-export keeps callers' imports unchanged. - `provider_recovery.rs` — `ProviderFailure` / `ProviderRecovery` and the validation deciding what a discovered subprocess may ask for. Today the one recovery is a Tailscale re-auth URL, and it is validated on the way in rather than at the opener, so an unvalidated URL never exists in desktop memory at all. `from_response` is `pub(super)` so `invoke_provider` stays its only caller and no second, unguarded construction path can appear in the crate. `invoke_provider` now returns `ProviderFailure` rather than `String`. There is deliberately no `From<ProviderFailure> for String` — that is the type-level guard against a caller flattening the recovery away, which is the bug the type exists to prevent. A caller with nothing to offer therefore has to say so: `probe_backend_provider` takes `.message` explicitly, because `info` is a probe whose failure has no action behind it. A provider's stderr on a *successful* op is also no longer dropped. Providers write their non-fatal complaints there, and `provider_stderr_notice` gives the success path (which logs) and the failure paths (which fold it into the error) one shared cap, so the snippet a warning shows and the snippet an error reports are the same text. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
A remote agent's harness lives on the host, not on this computer, so every value the desktop derives from its own machine is wrong for one. This assembles the payload from the record instead. - `runtime/setup_payload.rs` extracts payload construction out of `runtime.rs`, which was deriving it inline from local runtime state. The remote case needs the same shape built from different inputs, and a function with named inputs is the seam that makes both readable. - `create_time_agent_args` (in `discovery/overrides.rs`) splits create-time argument derivation away from the update path. They looked interchangeable and are not: at create time the harness choice arrives as an explicit pin from the provider's `discover_harnesses` catalog, with no prior record to reconcile against. - Two new provider ops behind the same contract: `discover_harnesses` asks the host what is installed there (the local `KNOWN_ACP_RUNTIMES` probe describes THIS computer and says nothing about the host), and `probe_models` returns one harness's model catalog. `probe_models` carries its env under the `env_vars` key specifically because that is the only key `invoke_provider` scrubs from error surfaces — anywhere else and a provider could echo a credential back through an error string unredacted. - `probe_provider_models` normalizes through the same `normalize_agent_models` the local path uses, so the model picker needs no remote-specific rendering. That reader now prefers the schema's `name` field, falling back to the pre-standardization `displayName` some adapters still emit. - `CLAUDE_CODE_EXECUTABLE` joins `RESERVED_ENV_KEYS`: it selects which binary the harness runs, so a user-supplied value would silently override the harness pin the deploy payload just resolved. The two provider commands return `ProviderFailure` rather than `String` so a failure carrying a recovery keeps it to the frontend; failures with nothing to offer convert through `.into()` and simply have none. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
A persona sync used to overwrite every linked instance's harness the same way. That is right for a local record — the definition is the authority and a pin naming a runtime the definition no longer asks for is stale local preference — and wrong for a provider-backed one, whose `agent_command_override` names a harness on a remote host. The local "is this a known runtime?" test cannot answer a question about another machine's filesystem, so applying it there silently cleared a pin the user chose from the host's own catalog. `apply_persona_snapshot` is now backend-scoped: a provider record keeps its remote harness pin through a sync while still taking the definition's model/provider quad; a local record keeps today's behavior. This is the one behavioral change to existing desktop code in this series. Alongside it, persona deletion learns the same distinction. Deleting a persona cascade-deletes its instances, but a provider-backed instance is a deployed unit this app can forget and not stop — the provider protocol has no undeploy. `preflight_remote_deployed_cascade` makes that asymmetry explicit: the cascade is refused unless the caller passes `force_remote_delete`, mirroring `delete_managed_agent`'s existing flag, and the refusal names the instances. An IPC caller that never learned about the flag sees the error it saw before. `delete_persona` and its helpers move to `personas/delete.rs`, and the backend-scoped snapshot cases to `persona_events/tests/harness_pin.rs` — both sibling extractions in the house pattern, keeping the two touched files under the size ratchet rather than growing them. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
The TypeScript half of the provider contract: the vocabulary and bindings
the UI reads, split from the local-runtime surface rather than added to it.
Everything a backend provider answers is a question about a REMOTE machine,
and the modules next door answer the same questions about this one. Conflating
the two is precisely the mistake that makes an agent silently deploy against
the wrong host's harness catalog, so the remote vocabulary gets its own
`backendProviderTypes.ts` and its own `tauriBackendProviders.ts`. `types.ts`
re-exports the four remote types so existing importers are unaffected.
Note the `tauri.ts` change is a net shrink of a file that was already at its
size ceiling: the two provider bindings move out and nothing moves in, so a
file the ratchet had pinned at 1,206 lines ends at 1,188.
Two wire-boundary decisions carry their reasoning in the tests:
- `fromRawRemoteHarness` only carries `exclusive` when the provider asserted
it. Absent is the default and every consumer reads it as "no limit", so
spreading a `false` onto every other entry would have the desktop claiming
something the provider never said.
- `providerRecoveryOf` reads the structured `{message, recovery}` payload off
`TauriInvokeError` instead of parsing human-readable error text. It returns
`null` for every other error shape, which is the common case — a provider
failure without a recovery is an ordinary failure and renders as its
message alone. This is the frontend end of the `recovery: {action:
"open_url"}` key on the provider wire protocol.
`backendProviderHooks.ts` is its own module rather than another block in
`hooks.ts` for the same reason: providers are the one axis of the agent
surface that is about WHERE an agent lives, and the three surfaces that ask
share nothing else in that file. The per-provider `info` probe is deliberately
not folded into provider discovery — discovery is a PATH walk cheap enough for
any surface to run on mount, while probing spawns each binary, so only the
surface whose entire subject is the providers pays for it.
Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
`WhereToRunSection` and `whereToRunIntent` are your components -- 126 and 46 lines on main. This commit grows them along the seam they already left: the section keeps owning "where", and every field the answer scopes now reads from it instead of assuming this computer. The create dialog asked "where" last, or not at all. That ordering is wrong once a host can be somewhere else, because every question below it is scoped by the answer. The harness list comes from the chosen machine's catalog, and the model list comes from that harness -- so asking at the end means answering the dependent questions against the wrong computer and silently re-scoping them. "Where to run" moves to the top of the form and the rest of the dialog becomes a function of it: - `createRuntimeGate` holds the local catalog's authority. A remote create must not require a locally installed harness (that would make every remote-only harness unsubmittable) and must not disable an "unavailable" option -- availability describes the wrong machine. - `useRemoteAwareModelDiscovery` routes discovery to whichever machine will run the agent. A picked remote harness REPLACES local discovery rather than merging with it; the local catalog answers for a computer the agent is never going to run on. - `createGateHarnessId` decides whose credential contract applies. The deploy writes env on the host keyed off the remote command, so the local runtime id names the wrong contract. - `createRunSection` becomes a render prop so the host's model probe can carry this dialog's unsaved credential env, which the global layer does not have yet. `agentAiConfigurationPolicy` makes a typed model that the catalog does not know block Save instead of silently resolving to the adapter's default at runtime. This also fixes block#3220 more strongly than the fix proposed there: the `draftRef` spread in `WhereToRunSection` keeps the section's draft authoritative across the remount, so a rapid edit cannot be clobbered by an in-flight probe. Cited rather than absorbed -- close block#3220 or land both, your call. Extracting `AgentLlmProviderField` and `AgentDefinitionIdentityFields` keeps `AgentDefinitionDialog` under its size cap; `shouldRenderModelControl` moves beside `modelFieldStatus`, since both answer what the model catalog's state means for the model control. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
A remote agent's identity is its host. Once an agent can run somewhere else, every surface that shows one has to say where -- and the app has to stop answering questions about a remote record out of this computer's catalog. An `exclusive` catalog entry may back at most one agent. The provider marks entries that name a persistent IDENTITY on the host -- its own memory, sessions, credentials -- rather than an ephemeral runner. Deploying `claude` N times to one host is the point; two agents on one profile are two puppeteers on one body. `exclusiveRemoteHarness` decides "already taken" generically (same provider, same config, same resolved command+args), and the picker renders a taken entry disabled with an "(added)" suffix rather than inventing a badge. Nothing in the desktop knows what Hermes or a profile is. `pinnedHarness` is the single owner of "may this surface read the record instead of the local catalog?". It answers null for a local agent, whose catalog entry genuinely describes it and whose rendering does not change. For a provider record every local lookup is either a miss (a Hermes agent describing itself as "Buzz Agent") or a name collision, so the record answers from itself: its own harness derivation, its own avatar precedence chain, and its own editing route -- the instance dialog, never the definition one, since `to_definition_view` drops `backend` and `agent_command` by design. Liveness comes from relay presence, not a local "deployed" flag. That flag is written once on a successful deploy and never cleared -- the provider protocol has no undeploy -- so it means "this desktop deployed it" and keeps meaning that after the remote process dies. `managedAgentPresenceStatus` keeps the control plane authoritative for a local record and defers to relay presence for a provider-backed one, the same channel the delete path already trusts. Deleting a persona now names what it cannot stop. A cascade over provider-backed instances leaves their remote units running, so the confirm dialog lists them and the backend refuses the cascade without that acknowledgement -- an honest delete rather than a silent orphan. `agentRunsOnLabel` names the PROVIDER, never the host: a provider id has a bounded shape and a host does not, and reading `backend.config` would grow a host vocabulary per provider. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
Harnesses on this machine, then the machines that are not this one. Remote
servers sits under Agents rather than Compute because it is about where an
AGENT is hosted; Compute is about model serving.
The card is read-only by design and has no host list. A provider is a binary on
PATH, so "adding" one is an install, not a form -- and the host is a per-agent
decision the create dialog pins onto the record verbatim at create time. CRUD
here would either edit saved configs that deployed agents deliberately do not
re-read, which reads as a bug, or duplicate the create flow's ownership of the
host.
Three consequences that must not drift: this gallery is the only surface that
pays for an `info` probe per discovered binary (every other surface renders
ids); `"ready"` means "this binary answers the provider protocol", never "the
server is reachable", because `info` opens no connection; and every settled
probe must land in the probe map, since an absent entry is indistinguishable
from one still in flight and a dropped result is a row that spins forever.
This is also the frontend end of the `recovery: {action: "open_url"}` key the
provider contract introduces: a host failure the user can fix carries a typed
recovery, and "Authenticate in browser" is the button it renders. The message
always stands alone -- the recovery only ever ADDS a button, so a provider that
sends none degrades to an ordinary failure rather than an empty one. Nothing
auto-retries: the desktop cannot observe a browser it does not own, so "Check
the host again" stays the retry.
Onboarding gains the same vocabulary rather than a second one. The no-provider
sentence is `NO_BACKEND_PROVIDER_HINT`, stated once and rendered by all three
surfaces -- a user meets it in up to three places, and three spellings of one
fact read as three different facts.
Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
The SSH provider is a standalone binary a user puts on their own PATH, on every desktop platform Buzz ships to. Its Windows-specific surface is path resolution — `tailscale.exe` candidate directories, `ssh.exe` lookup — plus JSON framing, and none of that is exercised by a Linux or macOS runner. Untested path resolution on Windows is a shipped bug, so gate it in CI. The step goes in the existing `windows-rust` job, which already triggers on `crates/**`, so no new job and no new trigger surface — roughly 40s added to a job that already runs on every Rust change. Unlike the `buzz-dev-mcp` step above it, these tests need no `--test-threads=1`: the crate's tests mutate no process-global environment. The shell-script execution tests are `#[cfg(unix)]` and cover the remote side, which is always POSIX. `scripts/run-tests.sh` gets the same crate in its unit-test set. The deploy tests run the generated provisioning script against a local `/bin/sh` with a stubbed `HOME`, so they need no Postgres, no Redis, and no remote host — and they are the only place the shell-injection canary actually executes. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
…e gallery Two end-to-end pins for claims the unit tests cannot reach, because both are about which component mounts rather than what a function returns. The first is a routing pin. Every provider-created agent carries a personaId, and the profile panel's Edit action used to read that as "this is a persona, so open the DEFINITION dialog". That dialog projects an AgentDefinition, which has no slot for `backend` or `agent_command` — so a remote agent opened on a blank harness, this computer's local default was re-seeded over the record's own pin, and Save was then blocked behind a provider + API-key demand for a machine the agent does not run on. The spec asserts the positive (the instance dialog opens showing the record's own pinned harness) and the negative (the definition dialog never mounts), because the bug was a silently wrong surface, not an error — an assertion that only checked "a dialog opened" would still pass. The second covers the Remote servers card with no provider installed. The mock bridge answers `discover_backend_providers` with `[]`, which is also the overwhelmingly common real case: a provider is a separate binary that nothing bundles, so the teaching empty state is the copy most users will ever see. The spec additionally asserts the loading branch has settled to zero rather than merely that the empty state rendered — those are different failures, and only the second is a bug. Both go in the `smoke` project. `edit-agent-provider-routing.spec.ts` needs a `testMatch` entry; the harness-management case joins a file already listed. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
The host contract in docs/remote-agents.md is short, but two of its items fail in ways that do not look like misconfiguration. Without `loginctl enable-linger` the user manager is torn down when the last session ends — so the agent is killed the moment the deploy's own SSH session closes, and the symptom is "the agent keeps dying", not "lingering is off". Without lingering there is also no /run/user/$(id -u), so every `systemctl --user` call fails at the bus before it reaches a unit. This script turns both into a row in a table you read before deploying instead of a mystery you debug after. It fixes what it safely can (enables lingering, creates ~/.local/bin, appends the PATH export to ~/.profile only when absent) and installs nothing. Harness CLIs carry their own installers and their own interactive logins, which cannot run over a non-interactive SSH deploy; buzz-acp and the buzz CLI the deploy op itself resolves or pushes. Reporting them is useful, installing them here would duplicate a code path that already exists and is tested. Only three checks decide the exit code — lingering, ~/.local/bin on PATH, and a reachable systemd --user bus — because only those three make a deploy fail. Everything else, Tailscale included, is a note: Tailscale buys the desktop's device picker, and plain SSH is the unchanged fallback, so a host without it is correctly configured, not degraded. Three details are deliberate and easy to "clean up" wrongly. It refuses to run as root: the env file holding the minted nsec must land owned by the user whose home already holds the harness credentials, and a root run would provision the wrong home. It checks the systemd bus under the same missing-XDG_RUNTIME_DIR assumption the deploy makes, since a check that only passes in a login shell would pass on hosts the deploy then fails on. And it reads Tailscale's SSH state from prefs rather than from `Self.sshHostKeys` in the status document — that field is populated for peers and reads null when a node looks at itself, so the obvious source reports a false negative on an SSH-enabled host. POSIX sh, no jq, no bashisms: it has to run on a bare host, which is the one place none of those are guaranteed. Idempotent — a provisioned host is a no-op. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.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: 2a495cc189
ℹ️ 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".
| let stem = &stem[..stem.len().min(32)]; | ||
| let stem = stem.trim_end_matches('-'); | ||
| let stem = if stem.is_empty() { "agent" } else { stem }; | ||
| format!("{stem}-{}", short_hash(&self.name)) |
There was a problem hiding this comment.
When two agents with the same display name are deployed to the same SSH account, this produces the same slug, systemd instance, environment-file path, and backend_agent_id for both. The second deployment therefore overwrites the first agent's nsec and configuration, and starting either record controls whichever identity was written last. Derive the suffix from the stable agent pubkey or another unique record identifier rather than the non-unique name.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 449a5b9. The unit instance, env-file path and backend_agent_id now derive from the agent pubkey (sanitized name plus a 12-hex fragment). The deploy payload carries a required 64-hex pubkey, and a test pins two same-named agents to distinct units.
| }; | ||
| for entry in entries.flatten() { | ||
| let name = entry.file_name().to_string_lossy().to_string(); | ||
| if let Some(id) = name.strip_prefix(prefix) { |
There was a problem hiding this comment.
On Windows, Cargo installs this binary as buzz-backend-ssh.exe, so this derives the provider ID ssh.exe. Creation then passes that ID to resolve_provider_binary, whose [a-z0-9][a-z0-9_-]* validation rejects the dot, making the new SSH provider unusable from the Windows desktop despite its Windows-specific support. Strip .exe before deriving and deduplicating the ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 285924d. A trailing executable extension is stripped before the id is derived, and discovery now dedupes on the id rather than the filename, so .exe/.com spellings collapse to one ssh provider.
| template=$(cat <<'BUZZ_UNIT_EOF' | ||
| {unit}BUZZ_UNIT_EOF | ||
| ) | ||
| printf '%s\n' "${{template%%@BUZZ_ACP_BIN@*}}$acp${{template#*@BUZZ_ACP_BIN@}}" > "$unit_file.new" |
There was a problem hiding this comment.
When the optional buzz_acp_path resolves to an executable whose absolute path contains whitespace, this inserts the path verbatim into ExecStart, where systemd parses the remainder as separate arguments instead of part of the executable name. The schema accepts such paths and the SSH resolution code correctly shell-quotes them, but deployment then fails to start the unit; escape or quote $acp using systemd's command-line syntax before writing the template.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9bc0f62. The substituted path is written double-quoted with backslash and quote escaped per systemd command-line syntax. Covered by a spaced-path test.
dschwartzAI
left a comment
dschwartzAI
left a comment
There was a problem hiding this comment.
Two self-hosted parity/hardening findings from a real Ubuntu VPS pilot:
-
The generated
buzz-acp@.serviceshould setNoNewPrivileges=true. We mechanically exercised the harness, Buzz CLI, NIP-OA owner-reviewed draft creation, and repository branch pushes from that boundary; all remained functional. Without it, an arbitrary-code-capable harness can still acquire privilege through setuid/setgid executables or passwordlesssudogranted to the SSH user. I would keep this delta deliberately narrow: addNoNewPrivileges=trueplus an asset assertion, withoutProtectSystem/ProtectHomeuntil writable workspace paths are modeled. -
The documented
BUZZ_ACP_TEAM_INSTRUCTIONSlimitation is observable parity loss, not merely metadata: a team-linked remote agent silently starts without its standing team rules. If carrying the resolved text is too large for this PR, please make the provider-backed create/edit surface report that limitation explicitly rather than presenting the remote record as equivalent to local.
These do not require owner key material on the host and do not broaden deploy, database, Stripe, or migration authority.
|
One additional project-workspace parity point: local managed agents receive Desktop's At minimum, |
The systemd instance, env-file path and returned `backend_agent_id` were all derived from the agent's display name. A name is not unique: two agents called the same thing on one SSH account collapsed onto one unit and one env file, so the second deploy overwrote the first agent's minted nsec and starting either record drove whichever identity was written last. Key them on the agent's pubkey instead — the record's own primary key, and the only stable identifier the payload can carry. The slug keeps the name as its readable stem and appends a 12-hex fragment of the key, so `systemctl --user status` still reads as the agent a human recognizes. The payload field is required, not optional: falling back to the name on an absent `pubkey` would reintroduce the collision silently, so deploy refuses a payload without a 64-character hex key. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
Cargo installs the provider as `buzz-backend-ssh.exe` on Windows, and reading that filename literally derives the id `ssh.exe` — which `resolve_provider_binary`'s `[a-z0-9][a-z0-9_-]*` rule rejects for the dot. The SSH provider was therefore unusable from the Windows desktop, despite carrying Windows support. Strip a trailing executable extension before deriving the id, and deduplicate discovery on the id rather than the filename so a host carrying both spellings offers one provider rather than two the desktop reads as different. Stripping is unconditional rather than `cfg(windows)`: what id a file name means must not depend on which machine reads it, and a rule that only compiles on one platform is one most CI never exercises. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
ExecStart= splits an unquoted value on whitespace, so a "buzz-acp path on the server" naming a directory with a space in it — which the config schema accepts — made systemd run the first word with the rest as arguments, and the unit never started. The SSH resolution code already shell-quotes such a path; systemd's command-line syntax is a different question. Write the substituted path double-quoted, escaping backslash and double quote first since systemd unquotes C-style escapes inside double quotes. The escape pass filters through sed rather than editing in place, so the portability argument that kept sed -i out still holds. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
A remote agent runs arbitrary code by design, so the SSH user's own privileges are the intended ceiling. Without NoNewPrivileges the harness can climb past them through any setuid/setgid binary on the host, or through passwordless sudo granted to that user. A VPS pilot exercised the harness, the Buzz CLI, NIP-OA owner-reviewed draft creation and repository branch pushes with it set; all remained functional. Deliberately the whole hardening delta. ProtectSystem/ProtectHome belong in this unit too, but an agent has no modeled workspace yet, so until writable paths are something the protocol states those directives would be guessing at which of the user's home an agent legitimately needs. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
Local spawn resolves a linked team's instructions and hands them to the harness as BUZZ_ACP_TEAM_INSTRUCTIONS. The deploy payload has no team field, so a provider-backed record starts without its team's standing rules — the agent runs, answers, and simply does not know the rules its team was written to enforce. That is observable behaviour, not metadata, and the surfaces that offer or describe a remote agent were presenting the record as equivalent to a local one. Say it in both places. The create flow states it the moment "elsewhere" is the answer, before a team is even picked; the edit dialog states it for a record where it is already true, next to the pin that makes the record remote. One owner for the copy and the condition, so the two surfaces cannot drift into different accounts of the same limitation. Carrying the resolved text to the host is the real fix and a protocol change. Until then, disclosure is the honest answer. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
git-credential-nostr had no preflight row of its own. It is a third tool with a third policy: deploy resolves it, never installs it, and writes the agent's GIT_CONFIG_* block only when the host already has it — so a remote agent on a host without it cannot push to a Buzz repository, with no deploy-time warning and no error until the agent tries. Folding that into the non-fatal buzz CLI check hid a different failure behind a different tool's status. Give it its own row, still reported and still never mandatory. Record the workspace gap as a known limitation while it is one. A local managed agent runs inside Desktop's REPOS workspace; the unit here has no WorkingDirectory at all, so a remote agent has no Buzz-native project checkout and in practice operates in a separately provisioned one. Closing it is a protocol addition — a workspace field the desktop states, rather than the provider guessing which project to clone. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
Keying units on the agent pubkey changes the instance name, so a host provisioned before that rule gets a new unit alongside the name-keyed one on redeploy rather than a replacement — and the old one keeps running under Restart=always. There is no undeploy op to clean it up, so say what to remove by hand, including the env file that still holds an nsec. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
|
Thanks for the review. All three items are addressed, and the three Codex findings are answered inline.
Team instructions: 071974d discloses the limitation on the provider-backed create/edit surface. Local spawn resolves a linked team's instructions into Provisioning and docs: d45c25e gives Gates green on the new head: |
|
Ran this branch today against a self-hosted relay: provider built from the branch on macOS (arm64), target host a Debian 12 KVM guest behind an Three things we hit:
Minor: Happy to re-test as this moves. The |
|
To use Codex here, create a Codex account and connect to github. |
Round-1 review fixes from PR block#3449 (block/buzz), ported from troyhoffman-oss:feat/remote-agents: - remote systemd units keyed on the agent's pubkey, not its display name - the resolved harness path quoted in ExecStart - NoNewPrivileges=true on the unit template - the remote team-instructions parity loss disclosed in both agent surfaces - git-credential-nostr reported as its own readiness check, and the missing workspace projection recorded as a known limitation The Windows provider-id fix from that branch is not carried: remote-first's ExecNaming seam already derives ids without executable extensions, and does so under a PATHEXT-ranked allowlist that the ported version did not have. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
…ctive SSH A non-interactive `ssh host sh -s` reads no profile. On stock Debian that leaves PATH at `/usr/local/bin:/usr/bin:/bin:/usr/games`, which does not contain `~/.local/bin` — where every Buzz tool installs, and where `pipx` and `npm --prefix=~/.local` put the harness adapters. Every lookup that went through a bare `command -v` therefore answered for the wrong PATH. On a host with all twelve adapters installed, `discover` reported the entire catalog as `available: false`, and the deploy that followed refused the operator's pin with exit 91, "harness not found". The env file already pinned `PATH="$HOME/.local/bin:$PATH"` for the unit, so the contract was only half-applied: the spawned harness could see the install destination, but the scripts that resolved the harness could not. Prepend it in the scripts too, via one shared `install::PATH_PREAMBLE`, so both sides ask the same question. That also simplifies the env file's own PATH line. It now writes the deploy shell's `$PATH` verbatim, which already leads with the install destination — one prepend, in one place, instead of a second one that could drift from the first. `resolve` is unchanged: it tests `-x ~/.local/bin/<tool>` explicitly because it must also answer for an absolute configured path. Covered by two tests that run the generated scripts against a real `/bin/sh` with the stock Debian PATH and an adapter installed only to `~/.local/bin` — the reported case, in both the discover and the deploy direction. Both fail without the preamble. The provisioning preflight gets the same prepend for the same reason, so it stops reporting a tool MISSING on a host where the deploy finds it. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
The `~/.local/bin` row was mandatory and asked whether the directory was on the PATH of the process running the check. Over a non-interactive `ssh host /path/to/provision-buzz-host.sh` it never can be: ~/.profile is read by a login shell, so the check always landed in the NOTE arm and a fully provisioned host exited 1 against its own preflight. The strict form was not load-bearing. Nothing in the deploy path consults the login PATH — the deploy composes the unit's PATH itself, and both remote scripts prepend the install destination before they resolve anything. What is worth requiring is that the directory exists and that a login shell on the host is configured to find it, which the ~/.profile arms already guarantee. So those arms report OK and carry the difference in the ACTION column: whether a re-login is still owed. A host with neither the PATH entry nor the ~/.profile line still gets the line appended and still says so. Exit semantics are otherwise unchanged: a genuinely unsatisfied mandatory row — no lingering, no user bus — still exits 1. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
`BUZZ_ACP_RELAY_OBSERVER` is pinned `true` in every env file, but the frames are encrypted and addressed to the owner's pubkey — so with no owner resolved, `buzz-acp` logs that it will publish nothing and the setting is inert. That is easy to hit rather than theoretical: dropping the NIP-OA identity is the current workaround for the cross-machine mention gate (block#3277, block#2987), and a deploy with no `auth_tag` and `respond_to=allowlist` otherwise works — the agent answers mentions and pushes to repositories, and only its telemetry is quietly absent. Also brings the `PATH` line's description in step with how it is now composed, and states the `~/.local/bin` prepend the remote scripts and the provisioning preflight share. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
|
@Jud thanks for running it end to end. All three are fixed on the branch.
export PATH="${HOME:-}/.local/bin${PATH:+:$PATH}"
One simplification fell out of it. The env file's Two tests run the real generated scripts through
On Branch is at |
Ports Jud's three review findings from PR block#3449 (feat/remote-agents): harness resolution from ~/.local/bin over non-interactive SSH, the provisioning PATH check passing without a login shell, and the doc note that the pinned relay observer needs a resolved owner. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
|
@Jud checking in on this one. Your three field-test findings are fixed on the branch (head One tie-in since your test: the remote-agents spec in #3748 lists the Windows |
|
Re-tested on the new head ( All three findings from the last round are fixed.
Also confirmed on this pass: Three new things, in the order I'd weight them. 1. A desktop test fails whenever the provider is actually installed
The cause is CI stays green because CI has no provider installed, so this lands on exactly 2. The provider trusts a caller-supplied pubkey rather than deriving it
That reopens the collision 3.
|
|
Building a second provider (
Suggestion, and deliberately not to relax the check: an optional |
4 participants
Summary
BackendKind::Providerwire contract thatmanaged_agents/backend.rsalready describes, and shipbuzz-backend-sshas its reference implementationrecoverychannel so a host failure the user can fix renders a buttonprovision-buzz-host.shpreflight and Windows CI coverage for the provider crateFixes #3034.
Why
managed_agents/backend.rshas describedBackendKind::Providerfor a while: the desktop finds abuzz-backend-<id>executable on PATH, writes one JSON request to its stdin, reads one JSON response from its stdout. #2859 states the gap plainly — that path "has no OSS implementation," so the seam exists and nothing exercises it. #3034 is the user-facing shape of the same gap.Two things follow from filling it. The first is a provider:
crates/buzz-backend-sshimplements the five ops over SSH, one process per op, no daemon and no state. The second is that the desktop has to stop answering questions about a remote record out of this computer's catalog — the harness list, the model catalog, the credential contract, and the avatar precedence chain all change meaning once the host can be somewhere else. That is the bulk of the desktop diff, and it is why the create dialog's question order changes: every question below "where" is scoped by the answer to it, so asking last means answering the dependent questions against the wrong machine.The contract is specified in
docs/remote-agents.mdand lands in the first commit, ahead of the code, so the design can be judged before a line of Rust.Reading order
154 files, +18,069/−1,413 — 12,453 lines of production code, 4,686 of tests, 930 of markdown. 12 commits, each of which compiles and passes its tests standalone; review in order.
d307c1d1ddocs(remote-agents): specify the SSH provider contract — the five ops (info,check,discover_harnesses,probe_models,deploy), their schemas, the optionalrecoverykey, the systemd unit and env-file contract, and the security invariants the implementation is required to hold. Everything after this is an implementation of this file.1b14b6e2cfeat(backend-ssh): add the SSH remote-agent provider crate — the reference implementation. Six modules, one protocol:protocol(wire types, theSecretthat renders[REDACTED]),ssh(transport),tailscale(peer enumeration, re-auth URL),discover,install,deploy. One commit because they are one protocol implementation and do not separate —deployreaches intoinstallat 43 call sites, anddiscover/deployare mutually dependent. 115 tests.204ecf55bfeat(desktop): discover and invoke backend providers on PATH — splitsbackend.rsinto what a provider may make the desktop do (provider_recovery.rs) and which files on PATH are providers at all (provider_discovery.rs).invoke_providerreturnsProviderFailure, notString; there is deliberately noFrom<ProviderFailure> for String, because flattening the recovery away is the bug the type exists to prevent.489c1a59dfeat(desktop): build a remote deploy payload from a provider record — every value the desktop derives from its own machine is wrong for a remote agent. Addsdiscover_harnessesandprobe_models, and normalizes remote models through the samenormalize_agent_modelsthe local path uses so the picker needs no remote-specific rendering.c8ec0df04feat(desktop): scope persona runtime sync to local records — the one behavioral change to existing desktop code in this series. A persona sync used to clear a provider-backed record's harness pin, because the local "is this a known runtime?" test cannot answer a question about another machine's filesystem.cf22ca946feat(desktop): type and bind the backend-provider Tauri surface — the TypeScript half. Remote vocabulary gets its ownbackendProviderTypes.tsandtauriBackendProviders.tsrather than being added to the local-runtime modules next door;types.tsre-exports so existing importers are unaffected.9ec77b7b6feat(desktop): ask where an agent runs before anything else — grows yourWhereToRunSection(126 lines) andwhereToRunIntent(46) along the seam they already left: the section keeps owning "where", and every field the answer scopes now reads from it. "Where to run" moves to the top of the form;createRuntimeGate,useRemoteAwareModelDiscovery, andcreateGateHarnessIdmake the rest a function of it.dd04b629dfeat(desktop): one agent per exclusive host identity — anexclusivecatalog entry may back at most one agent, because it names a persistent identity on the host rather than an ephemeral runner.pinnedHarnessis the single owner of "may this surface read the record instead of the local catalog?"; liveness comes from relay presence, not the write-oncedeployedflag.e8552f932feat(settings): permanent Remote servers section under Settings > Agents — read-only by design and with no host list: a provider is a binary on PATH, so adding one is an install, and the host is a per-agent decision the create dialog pins onto the record. This is the frontend end ofrecovery: {action: "open_url"}; the message always stands alone and the recovery only ever adds a button.b0a0cb231ci(windows): run buzz-backend-ssh's tests on Windows — the crate's Windows-specific surface is path resolution (tailscale.execandidates,ssh.exelookup), which no Linux or macOS runner exercises. Goes in the existingwindows-rustjob, which already triggers oncrates/**.5cfd9924ftest(desktop): cover provider-backed edit routing and the empty remote gallery — two Playwright pins for claims about which component mounts. The routing spec asserts both the positive and the negative, because the bug it pins was a silently wrong surface rather than an error.2a495cc18feat(scripts): add a remote-host provisioning preflight — two items in the host contract fail in ways that do not look like misconfiguration (missingloginctl enable-lingerpresents as "the agent keeps dying"), so the preflight checks and names them.Security invariants
Debugrendering. The remotepsis world-readable.private_key_nsecfails closed, insideAgent::from_request, so no caller can route around it. Env values pass a control-character guard, because a newline would split the assignment into a second attacker-chosen line.infoschema is byte-identical to the plain one. Trust-on-first-use is confined to addresses the local Tailscale daemon already lists as peers; for anything the user typed, host-key checking stays strict. The re-auth URL is constructed, never parsed out of host output.~/.local/bin, which discovery already covers.Verification
cargo test -p buzz-backend-ssh— 115 passedcargo test --manifest-path desktop/src-tauri/Cargo.toml— 1,890 passedtsctypecheck and Biome cleanpnpm test:e2e:smokefor the two new specsshellcheckonscripts/provision-buzz-host.shSigned-off-byunsafe; no newunwrap/expecton a production pathRelationship to open work
buzz-backend-<id>convention the desktop already looks for and the precedent set by feat: first-class Crabbox backend for remote Buzz agents #2538 (Crabbox).draftRefspread inWhereToRunSection.tsx:159keeps the section's draft authoritative across the remount, so a rapid edit cannot be clobbered by an in-flight probe. That is a stronger fix than the one proposed on the issue, but it is cited rather than absorbed: close fix(desktop): stop create-agent form clobbering provider config on keystroke #3220 or land both, your call.ExecNamingseam fix(desktop): make backend providers runnable on Windows #3310 introduces, held verbatim so the split is preserved. Whichever lands second is a trivial rebase.hermespreset rather than replacing it: because a host commonly runs several Hermes profiles out of one install,discover_harnessesfans that single preset out intohermes-<name>entries carrying--profileargs, so each configuration appears as its own harness.Follow-ups
force_remote_deleteopt-in, rather than silently orphaning them.