mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
main
20 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
4f51b9e101 |
fix(mcp): support Goose discovery handshake (#8037)
## Summary Goose 1.52 starts MCP connections with `server/discover`. `buzz-dev-mcp` rejects that request with `ExpectedInitializeRequest`, preventing Goose from loading its Buzz tools. Upgrade the tool server to rmcp 3.4.1 and adopt the renamed SDK types. Subprocess tests exercise discovery and legacy initialization, tool listing, file reads/edits, invalid arguments, connection survival, and EOF shutdown. Include the server in both Rust unit-test runners and make the cancellation regression approve its shell call before testing cancellation. The native agent retains its independent rmcp 1.x client. Cargo also unifies the existing 3.x dependency used by the mesh smoke harness. ## Validation - Normal `git push` passed all hooks, including Rust tests, Tauri checks, and repository policy checks. - `just ci` passed on the final stack: repository checks, Rust and Tauri tests/lint, desktop tests/build, web build, and mobile checks/tests. - MCP: 78 existing tests and two subprocess lifecycle tests passed; the discovery regression fails with the old SDK. - All 54 native-agent regressions passed on the final stack, including cancellation of a real MCP shell process. - Previously exercised unmodified Goose 1.52 and 1.47 with Sandpit and the real tool server. Allowed workspace operations and permitted network traffic succeeded; disallowed file and network operations were denied. - Live staging desktop check with Goose 1.52: the reply appeared in Buzz, an allowed request returned HTTP 200, and a disallowed request returned CONNECT 403. ## Stack 1. **#8042 → `main`**: deterministic handoff regression. 2. **This PR → #8042**: Goose MCP discovery compatibility. Merge the lower PR first, then retarget this PR to `main`. This diff contains only the compatibility change and its tests; the relay build repair is already on `main` in #8036. --------- Signed-off-by: Alex Rosenzweig <arosenzweig@squareup.com> |
||
|
|
f7ff4294fa |
feat(nip-fi): harden Blossom kind-24242 verifier to NIP-FI spec (#7288)
Brings `buzz-media`, `buzz-relay`, all first-party kind-24242 minters, and desktop token minting into compliance with [NIP-FI §kind-24242](https://github.com/block/buzz/blob/main/docs/nips/NIP-FI.md). ## What changed **`buzz-media/src/auth.rs`** - Add `BlossomStrictness` enum (`Strict` | `Permissive`). `Strict` applies full NIP-FI rules (active modes); `Permissive` preserves pre-NIP-FI Off-mode behavior byte-identical [FI-INV-15]. - Rewrite `verify_blossom_auth_event_for_verb` with count-based cardinality: exactly one `t`/`expiration`/`server` in `Strict`; duplicate `x` also rejected. `Permissive` keeps main's boolean semantics exactly: any valued `t` other than the verb (including `""`) rejects, valueless `t` is ignored. - `Strict`: mandatory `server` tag on all proofs (upload + read); absent or mismatched → `evidence_rejected` (`ServerMismatch`). - `Strict`: 60-second proof window (`now - created_at <= 60s`, `expiration <= created_at + 60s`). `Permissive`: 3600s / optional server. - **[F1]** Permissive server validation uses any-match semantics: a proof with multiple `server` tags is accepted if any tag matches the bound host. This restores origin/main base behavior. Strict duplicate-rejection unchanged. - **[F1 compat]** Permissive server gate uses `server_values.is_empty()` (valued tags only) rather than `server_count > 0` (includes valueless tags). A lone valueless `["server"]` tag is treated as absent in Permissive/Off mode, matching pre-NIP-FI base behavior [FI-INV-15]. - **[F4]** Valueless `x` tags in Strict get reads are tracked before `filter_map` so they cannot become absent scope (host-wide authorization). Any `x` tag present in Strict must contain the exact requested sha256. - **[F3 unit]** Cases A–D in `auth.rs` are helper-level unit tests: they call `verify_blossom_upload_auth` and `verify_upload_hash_only` directly. They do not enter the upload pipeline (`process_upload`/`process_video_upload`). Retained as helper evidence. **`buzz-media/src/error.rs`** - Add `DuplicateTag(&'static str)` variant. - Add `blossom_denial_kind()` method: classifies each auth error as `MissingEvidence`, `EvidenceRejected`, or `AuthorizationDenied`. Non-auth errors return `None`. - `IntoResponse` retains the legacy JSON 401 shape [FI-INV-15]. **`buzz-relay/src/api/media.rs`** - Add `MediaDenial(MediaError, BlossomStrictness)` wrapper mapping to byte-exact NIP-FI fixed responses in `Strict`; falls through to legacy JSON in `Permissive`. - `blossom_strictness_from_state()` selects `Strict` when `state.config.nip_fi.is_enforce()`, otherwise `Permissive`. Strictness is threaded through the `admit_nip_fi_http_on_state` NIP-98 closures on upload and read, and through the post-admission X-SHA-256, hash-binding, and membership denials. - `extract_blossom_auth` keeps first-value `Authorization` extraction; Enforce rejects repeated fields at the `admit_nip_fi_http` cardinality gate, and Off keeps legacy behavior [FI-INV-15]. - Both membership-denial call sites use `media_denial(RelayMembershipRequired, strictness)` so Strict mode produces the correct fixed-text response. - `upload_blob` serving-write fence conversion (`:406`) uses `map_err(|e| media_denial(e, strictness))` — a community fence committed between auth admission and lease acquisition no longer resets Strict to Permissive. - Mode-driven regressions in `mod postgres_tests` (selected by the nextest `postgres-ci` profile via `test(/postgres_tests::/)`, `#[ignore = "requires Postgres"]`), run through the full relay router with a real Enforce verifier and assertion: Enforce rejects Permissive-only proofs (300s lifetime, missing `server` tag) on both upload routes and reads with `evidence rejected`; Off accepts the same proofs with pre-NIP-FI responses; DenyProtected answers 503 before any proof check; membership denials are `authorization denied` text/plain in Enforce and exact legacy JSON 403 in Off, with a member positive control. GET and HEAD are each discriminated (HEAD with a suppressed body). Both upload routes pin the handler's `X-SHA-256` missing/malformed/unsigned and post-body hash-mismatch denials in both modes. Off rejects an empty-valued `t` beside a valid one with legacy 401 JSON in either order, and ignores a valueless one. Exact status, Content-Type, challenge, and body bytes are pinned. **`buzz-media/src/upload.rs`** (`mod minio_tests`) - **[F3 pipeline]** Four tests exercising the actual post-body call sites (`process_upload` `:85`, `process_video_upload` `:413`) with real MinIO I/O: buffered/streaming × expired-proof-accept + mismatched-hash-reject. The positive cases use `fresh_strict_upload_auth` (`created_at = now-58`, `expiration = now+2`, lifetime = 60s) — Strict-admissible at sign time — followed by a real 3s sleep so the proof is expired when the upload completes. `nostr::Timestamp::now()` reads the OS wall clock directly; paused Tokio time does not advance it. The negative cases use a pre-expired proof with a mismatched `x` tag. All four are `#[ignore = "requires MinIO"]`; no CI lane runs them automatically. **`buzz-media/src/validation.rs`** - `minimal_valid_mp4()` promoted to `#[cfg(test)] pub(crate)` — a 618-byte pre-computed H.264/avc1 fast-start MP4 available to all `buzz-media` test modules; smoke-tested by `test_minimal_valid_mp4_passes_validate_video_file`. **`desktop/src-tauri/src/commands/media.rs`** - `sign_blossom_upload_auth`: `server` tag mandatory; upload expiry 60s (was 3600s/300s). - `MEDIA_GET_AUTH_EXPIRY_SECS`: 60s (was 600s). **`desktop/src-tauri/src/media_proxy.rs`** - Single re-mint+retry on 401/403 for range requests; non-auth failures do not retry. **`crates/buzz-cli/src/client.rs`** - Read expiry: 60s (was 600s). Upload expiry: 60s. `server` tag mandatory on upload. **`crates/buzz-dev-mcp/src/view_image.rs`** - `MEDIA_GET_AUTH_EXPIRY_SECS`: 60s (was 600s). Test updated. **`mobile/lib/shared/relay/media_auth.dart`** - `_mediaGetAuthLifetimeSeconds`: 60 (was 600). Refresh margin updated. **`mobile/lib/shared/relay/media_upload.dart`** - `_uploadAuthLifetimeSeconds`: 60 (was 300). `server` tag mandatory. **`mobile/lib/features/channels/media_viewer_page/video_viewer.dart`** - **[F2]** Remove Android-specific `VideoPlayerController.networkUrl` streaming path. `video_player_android` 2.9.5 freezes headers into static `DefaultHttpDataSource` request properties — a 60s proof minted at controller creation time becomes stale on seeks. All platforms use the authenticated local-file download path. - **[F2 transport]** `unawaited(request.sink.close())` before `client.send()`. `AbortableStreamedRequest` carries no request body; without closing the sink, `IOClient.send()` awaits `stream.pipe(ioRequest)` which blocks until the sink ends — every video download hung in loading indefinitely on every platform. - **[F2 pending-ctrl]** `pendingController` ref set before the first `await (initialize())`, cleared in all exit paths. Effect cleanup disposes `pendingController.value` so a close-during-init releases the native player even if the initialized event never arrives. - **[F2r(a)]** On native init failure (post-create, pre-initialized-event), the inner catch starts disposal detached via `unawaited(localController.dispose().catchError(...))` — the `.catchError` handler absorbs any disposal exception so it does not reach the test error zone — then rethrows the original load error. `video_player` 2.11.1 completes the init future with an error without disposing the native player. - **[F2r(b)]** Non-2xx error body cancelled via `_cancelVideoResponse()` (subscribe+cancel) instead of `drain()`. `drain()` waits for the upstream to close; a stalled server body blocked `initializeVideo` indefinitely. - **[F2r(d)]** `unawaited(localController.dispose().catchError(...))` in the catch — not `await`. Two failure mechanisms exist: (1) when `createWithOptions()` throws, `_creatingCompleter` is never completed and `await dispose()` deadlocks at `:682-683`, blocking the outer catch from setting error state. (2) when `createWithOptions()` succeeds but initialization fails, `dispose()` runs but its thrown exception becomes an unhandled async error reaching the test zone. Both are fixed: `unawaited` lets the outer catch run immediately and set `error.value`; `.catchError(...)` on the detached disposal future absorbs any disposal exception. **`mobile/test/features/channels/media_viewer_page/video_viewer_test.dart`** - `_FinalizingFakeClient`: calls `request.finalize().drain()` — a test without the sink fix times out because the drain never completes, validating the transport contract. - F2r(a): `forceInitError=true` fake emits `PlatformException` on subscribe; `disposeCallCount >= 1` verifies disposal even on init failure. - F2r(b): `stalledBody` `StreamController` with `onCancel` Completer (bounded completion signal, not a sleep). Asserts: (1) `bodyStreamCancelled=true` while body stream is still open (impossible with `drain()`); (2) error UI visible while body remains open. Restoring `drain()` breaks both. - F2r(c): `neverInitialize=true` fake never sends initialized event; unmount asserts `disposeCallCount >= 1` via `pendingController`. Distinct from the event-error case. - F2r(d): `forceCreateError=true` fake throws `PlatformException` from `createWithOptions()` itself (before any player ID is returned). Asserts error UI visible within a bounded deadline. With the old `await dispose()` the outer catch is blocked — `error.value` is never set and the error UI assertion fails at the deadline; with `unawaited(dispose())` the error text appears immediately. **`mobile/test/features/channels/media_viewer_page/video_viewer_transport_test.dart`** (separate file) - Real-IO loopback probes using `flutter_test` (not `package:test`) to avoid `depend_on_referenced_packages` lint. No `TestWidgetsFlutterBinding` — no HttpClient override interfering with `dart:io`. - Probe 1: `HttpServer.bind(loopbackIPv4, 0)` reads the full request body before replying. Without `sink.close()`, `req.drain()` hangs and the test times out. - Probe 2: server signals request arrival via a Completer (not a sleep); abort fires after confirmed arrival; `client.send()` must throw `RequestAbortedException` (typed assertion) within a bounded 5s deadline. **`scripts/test-video-upload.sh`** - Proof minted with `+60` expiry and mandatory `server` tag. **`buzz-test-client/tests/e2e_media*.rs`** + **`buzz-relay/src/api/media.rs` test fixtures** - All fixtures updated to 55s expiry + `server` tag. **`RELEASING.md`** - New "NIP-FI upload proof freshness (relay floor)" section documenting that 60s proofs require the first relay release carrying this PR's post-body hash-only check (the first tag > `v0.2.1` — update once cut) for reliable large-file uploads. Operators on older self-hosted relays (≤ `v0.2.1`) must upgrade before receiving these clients. ## Finding 3: 60s upload proofs vs old-relay compatibility Desktop/CLI/mobile upload proofs are now 60s. Older relays re-run full expiry verification after the body is transferred. A valid upload whose body transfer takes >60s will fail on an old relay. **Resolution (relay floor prerequisite):** `RELEASING.md` requires that verified supported deployments (relay releases confirmed to carry the post-body hash-only check, commit `75e9bef748`) be in place before distributing 60s-proof client builds. Unknown or unverified self-hosted relay deployments are excluded from distribution until they upgrade. This makes the verified-deployment status a prerequisite for release distribution, not a post-distribution advisory. The concrete tag (`> v0.2.1`) is to be recorded once cut. ## Sequencing note Builds on #7264 (merged): Blossom strictness is derived from its `config.nip_fi` mode in this PR, so enabling `BUZZ_NIP_FI_MODE=enforce` activates the strict verifier with no further wiring. ## GIF compatibility exception An unauthenticated Off-mode request on a configured tenant with no GIF provider changes from 404 to 401 (base `gifs.rs:269-275` checked an unconfigured provider first; the current code authenticates first). All other Off-mode Blossom responses are byte-identical to pre-NIP-FI behavior. --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> Co-authored-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> |
||
|
|
5621006bcf |
refactor: move agent Git bootstrap into ACP harness (#7819)
discovered in https://github.com/block/buzz/issues/7742#issuecomment-5783512985 ## Summary Goose uses its native shell, so it bypassed the Git identity and signing setup inside `buzz-dev-mcp`. Agent commits could use the host Git identity, while MCP shells had a separate config. This moves Git setup into `buzz-acp`, where both paths receive the same agent identity, Nostr signer, and relay-scoped credential helper. Before: ```text Buzz Desktop └─ buzz-acp ← Desktop supplied relay Git authentication ├─ Goose → native shell ← no agent Git author or signing setup └─ Buzz Agent └─ buzz-dev-mcp └─ shell ← MCP created the keyfile and Git signing config ``` After: ```text Buzz Desktop or Sprig └─ buzz-acp ← owns the keyfile, identity, signing, and scoped credentials ├─ Goose → native shell ← inherits the complete Git environment └─ Buzz Agent └─ buzz-dev-mcp └─ shell ← receives the same Git environment via mcpServers[].env ``` The harness keeps the signing key in a private temporary file and removes it on normal exit, startup failure, and graceful termination. It composes inherited `GIT_CONFIG_*` entries, forwards the complete block to MCP servers, and bundles the helpers through the ACP multicall entrypoint. For standard `buzz-acp` launches, Desktop and Sprig no longer write Git config. A custom Desktop ACP command bypasses the harness, so Desktop preserves its prior relay-scoped credential helper for that supported override. This changes Git settings for every ACP runtime. The startup signal path also changes so a termination during adapter initialization removes the keyfile. The temporary file can remain after SIGKILL or a machine crash. ### Related issue Related: #6177 covers stricter commit identity enforcement and touches the same Git paths. That PR and this one need to be reconciled before either lands. No issue was found for this runtime bootstrap gap. ### Testing - Ran the ignored `git_runtime_tests` with real Goose 1.50.0 and Buzz Agent ACP sessions. `real_goose_native_git_shell` passed again on this PR branch with `BUZZ_TEST_BIN_DIR` pointing to built binaries. Goose made a signed commit and tag through its native shell; Buzz Agent did the same through its MCP shell. Both verified signatures, identity, credential scoping, and keyfile cleanup. - Built the Linux Sprig image and ran `scripts/test-sprig-image.sh buzz-sprig:acp-git-bootstrap`. The container made and verified a signed commit and tag, then confirmed keyfile removal after SIGTERM. - `just ci` passed on the original branch. After rebasing onto `main` and addressing review feedback, the ACP Git bootstrap integration tests, focused desktop smoke test, custom-command regression test, formatting, and ACP/Desktop Clippy passed. The pre-push hook hit two timing-sensitive ACP deadline tests under parallel load; both passed alone and the complete ACP package passed serially (964 library, 2 Git bootstrap, 9 pool lifecycle tests). The push used `--no-verify` after those checks; GitHub CI is rerunning on the new head. - Under injected `BUZZ_ACP_ALLOWED_RESPOND_TO`, `BUZZ_ACP_LAZY_POOL`, and `BUZZ_ACP_IDLE_POOL_SLEEP`, the clean-env serial package command in `crates/buzz-acp/TESTING.md` passed: 964 active library tests, 2 Git bootstrap tests, and 9 pool lifecycle tests. Both named Databricks OAuth tests passed locally; another reviewer reproduced their environment-specific failures on `main`. - No authenticated relay clone or push was run. The local probes used disposable repositories and did not contact a relay. Generated with Codex --------- Signed-off-by: Salman Mohammed <smohammed@squareup.com> |
||
|
|
cae158ce7f |
fix(dev-mcp): extend shell timeout cap to 20 minutes and align outer budgets (#7185)
Raise the `buzz-dev-mcp` shell timeout ceiling to 1,200,000 ms (20 minutes) while keeping the omitted-value default at 120,000 ms (2 minutes), and align the two outer timeout layers that bound every MCP tool call. ## What changed **`crates/buzz-dev-mcp`** — shell timeout cap raised to 1,200,000 ms; tool and parameter descriptions updated to match. **`crates/buzz-agent`** — `BUZZ_AGENT_TOOL_TIMEOUT_SECS` default raised from 660 s to 1,260 s (20-min cap + 60 s cleanup headroom). The old 660 s default would have killed the MCP server at 11 min, before a 20-min shell call could finish. README updated. **`crates/buzz-acp`** — `DEFAULT_IDLE_TIMEOUT_SECS` raised from 900 s to 1,500 s and its doc comment updated (1,200 s max shell + 300 s breathing room). The old value was documented as "600 s max shell + 300 s" — stale arithmetic. ## Tests - Existing boundary tests in `buzz-dev-mcp` (default, exact-cap, over-cap, `u64::MAX`) all pass. - `default_idle_timeout_is_1500_seconds` — pinned constant replaces old 900 s assertion. - `default_tool_timeout_is_1260_seconds` — new pinned constant in `buzz-agent`. - `budget_ordering_invariant_shell_cap_plus_headroom_fits_within_idle_timeout` — new const assertion in `buzz-acp` that encodes the full three-layer ordering: shell cap (1,200 s) ≤ agent tool timeout (1,260 s) < ACP idle timeout (1,500 s) < max turn duration. A future edit that inverts any tier fails to compile. --------- Signed-off-by: Alia <d32955ad69077062930cc46cfe2df30ca9aaf6f8e76422681265e9e9af704d78@buzz.block.builderlab.xyz> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: Alia <d32955ad69077062930cc46cfe2df30ca9aaf6f8e76422681265e9e9af704d78@buzz.block.builderlab.xyz> Co-authored-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz> |
||
|
|
7f69b13b45 |
fix(buzz-dev-mcp): expand leading ~ in read_file/str_replace paths (#6271)
## Problem `resolve_path` (crates/buzz-dev-mcp/src/paths.rs) only ever treats a path as absolute or joins it onto the workspace root. A user-named tilde path like `~/.claude/skills/context-health-check/SKILL.md` is not absolute, so it resolves to `<workdir>/~/.claude/...` (e.g. `/app/~/.claude/...`), which never exists. The `shell` tool expands `~` via bash, so the file tools diverged from it. Surfaced in the PR #6261 named-path benchmark: all five agents initially passed a literal `~` to `read_file`, which resolved incorrectly under `/app/~`. They recovered, but the ergonomics are wrong — agents already expect `~` to work; the resolver violated that expectation. ## Fix Expand a leading `~` (bare `~` or `~/...`) to the user home directory at the single chokepoint `resolve_path`, matching shell semantics. Home is read from `$HOME` (`%USERPROFILE%` on Windows). Both `read_file` and `str_replace` route through `resolve_path`, so both are fixed. - `~user` (another user home) is intentionally NOT handled — it needs a passwd lookup and is out of scope, consistent with the conservative posture for un-mappable MSYS forms already in this file. `~user...` falls through untouched and fails with the clear `path not accessible` error. - `expand_tilde` takes `home` as a parameter so it stays pure and unit-testable without mutating process environment; env lookup lives in a thin `home_dir()` helper. - Tool/param descriptions are intentionally left unchanged: `~` is normal shared path behavior, not a per-tool feature. Advertising it in each `resolve_path` caller's description would spend prompt tokens on an implementation detail and invite drift across callers. The tests on `resolve_path` document the behavior. ## Tests - `expand_tilde_forms`: pure coverage of non-tilde passthrough, `~user`/`~foo` passthrough, bare `~`, `~/rest`, and unset/empty home. - `resolve_path_expands_tilde_against_home`: end-to-end through the real `home_dir()` env read (creates a marker file under `$HOME`, skips cleanly if no home). - Full package suite green: `cargo test -p buzz-dev-mcp` — 98 passed, 0 failed. `cargo clippy -p buzz-dev-mcp --all-targets` clean. Closes #6270 --- ## Follow-up (review by @wpfleger): honor `$HOME` on Windows + translate MSYS home The first cut read `%USERPROFILE%` on Windows and `$HOME` elsewhere. That diverges from the shell tool, which is the whole point of this fix: - `HOME` is passed through to the MCP child on **every** platform (`buzz-agent` `PASSTHROUGH_ENV`, not cfg-gated) and inherited verbatim by the `shell` tool (`shell.rs` spawns bash with no `env_clear`). Bash expands `~` against `HOME`. So on Windows the file tools and shell tool disagreed whenever `HOME != USERPROFILE` — e.g. a git-bash `HOME=/c/Users/x`, or an `mcpServers[].env` override setting `HOME`. - Ordering also mattered: `~` expansion runs **after** `msys_to_windows` in `resolve_path`, so a spliced-in MSYS home (`/c/Users/x`) never got translated and `canonicalize` rejected it on Windows. **Change (`paths.rs` only):** - Prefer non-empty `$HOME` on every platform; fall back to `%USERPROFILE%` only on Windows when `HOME` is unset. - On Windows, run the chosen home through `msys_to_windows` so `HOME=/c/Users/x` → `C:\Users\x`. An MSYS home with no Windows equivalent (`/home/x`) is left untranslated and fails with the clear `path not accessible` error — the intended conservative outcome. - Selection factored into a pure `select_home(home, userprofile)` so it is unit-testable without mutating process env. **Tests:** `select_home_prefers_home` (HOME-first + empty/unset fallback, all platforms) and `select_home_translates_msys_home_and_prefers_it` (Windows: MSYS translation + divergent `HOME`/`USERPROFILE`, which also fixes the prior test's silent no-op when `HOME` is unset on CI). `cargo test -p buzz-dev-mcp` — 99 passed, 0 failed; clippy clean. Pre-push `rust-tests` + `desktop-tauri-checks` green. HEAD `29e0082ff`. --------- Signed-off-by: Salman Mohammed <smohammed@squareup.com> Co-authored-by: Leo <5faf251baee50ee6bcde338aef6acdd70bb3e60115664c2cd490d94a55dfc488@buzz.block.builderlab.xyz> |
||
|
|
5694e78def |
fix(mcp): scope todo usage (#6216)
## Why The `todo` tool description does not say when the tool is unnecessary, so agents use it for single-turn bookkeeping. Scoping it to cross-turn persistence reduces avoidable control calls while preserving the checklist for compaction and genuinely multi-turn work. ## What - Scope `todo` to work that must continue across turns or survive context compaction - Tell agents not to use it for work they can finish in the current turn - Preserve read/replace semantics and the `_Stop` hook behavior ## Risk Assessment Low to medium — this changes agent tool-selection guidance, not the tool name, schema, or implementation. The benchmark covers single-turn completion but not restart, compaction, or long-lived multi-turn recovery. ## References - Companion base-prompt change: https://github.com/block/buzz/pull/6186 - Combined benchmark (PR 6186 prompt plus this description): 22/22 pass; active time 0.3438h → 0.2680h (-22.0%); tool calls 216 → 173 (-19.9%); todo calls 45 → 0 - Against PR 6186's prompt-only condition: active time 0.2926h → 0.2680h (-8.4%); tool calls 198 → 173 (-12.6%) - Results are directional because model and tool behavior is stochastic. Generated with Codex Signed-off-by: Salman Mohammed <smohammed@squareup.com> |
||
|
|
18eef633d8 |
feat(git): use agent display name as git author name (#3040)
Agent commits were authored by a raw 63-character npub, which makes `git log`, `git blame`, and GitHub's author column effectively unreadable. This uses the agent's display name for `user.name` instead, while leaving the pubkey where it does real work. ## What changes `build_git_env` in `crates/buzz-dev-mcp/src/shim.rs` now reads `BUZZ_ACP_DISPLAY_NAME`, sanitizes it, and uses the result as `user.name`. When the variable is absent or unusable it falls back to `info.npub` — byte-identical to today's behavior. `user.email`, `user.signingkey`, and the whole credential/signing block are untouched. The pubkey is what NIP-98 auth, NIP-GS signing, and contributor matching key on, and it stays in the email verbatim. `crates/buzz-acp/src/lib.rs` forwards the variable into the dev-mcp server's declared env, mirroring the existing `BUZZ_AUTH_TAG` block. It reads `std::env::var` directly rather than going through `Config`, so the variable is picked up whenever the process has it. `crates/buzz-agent/src/mcp.rs` adds one `PASSTHROUGH_ENV` entry so ACP clients that spawn `buzz-agent` without declaring the variable on the wire still propagate it. ## Why a dedicated variable `BUZZ_ACP_DISPLAY_NAME` is its own contract rather than a reuse of the ACP session title. Commits outlive sessions: a session title is per-session UI chrome and may be composed downstream into `Agent · #channel`, and if that composed form ever reached the env var, git attribution would change silently with no test able to catch it. Git identity gets a variable whose contract is "bare agent display name, never channel-qualified." Nothing writes it yet — a one-line Desktop write lands as a follow-up. Until then `std::env::var` returns `Err`, the npub fallback fires, and behavior is byte-for-byte current `main`. ## Sanitizing Strip control characters, Unicode format characters, and angle brackets; collapse whitespace runs, trim, cap at 80 characters (by `chars()`, so a multi-byte name is never split mid-UTF-8). Angle brackets go because git drops them silently rather than erroring: `Duncan <evil@x.com>` renders as `Duncan evil@x.com <hex@relay>`. It forges nothing, but it reads as though it might. The empty result also has to cover more than literal emptiness. git's `ident.c` treats a set of characters as "crud" — stripped from both ends, and fatal when a name is *nothing but* those characters: ``` $ git -c user.name=';;' commit -m t fatal: name consists only of disallowed characters: ;; ``` Verified against git 2.54.0 by committing with each ASCII byte 32..=126 as the entire `user.name`: exactly space, `"`, `'`, `,`, `:`, `;`, `<`, `>`, `\` abort, plus all control characters (the predicate is `c <= 32`). `.` is not crud in this version, despite older lore. Names that merely *contain* crud are fine — `O'Brien` and `Smith, Jr.` both commit cleanly — so the check is "at least one non-crud character survives," not "no crud present." Without it, a display name of `;;` or `""` would abort every commit that agent makes. ## Unicode format characters `char::is_control` covers only category `Cc`. Category `Cf` — zero-width spaces and joiners, bidi embedding and override marks, invisible math operators, tag characters — is neither control, nor whitespace, nor git crud, so those characters survived every one of the checks above. A display name of nothing but U+200B ZERO WIDTH SPACE therefore satisfied "at least one non-crud character survives" and git accepted the commit with a visually blank author: ``` # pre-fix, BUZZ_ACP_DISPLAY_NAME set to two U+200B $ git log -1 --format='%an' | xxd -p e2808be2808b0a ``` Embedded marks were the other half: a trailing U+202E RIGHT-TO-LEFT OVERRIDE reorders everything after it, so a stored author line renders as something other than what it stores — the same confusion class the angle-bracket filtering exists to prevent. `is_unicode_format` rejects the whole `Cf` category rather than the known-bad marks, because the boundary that matters is "invisible or reorders text", not "the codepoint someone thought of". The 21 ranges come from the UCD's `DerivedGeneralCategory.txt` (17.0.0), cross-checked against Python's `unicodedata` (16.0.0); both yield exactly the same set. They are inlined as a `matches!` rather than pulling in a Unicode-tables crate for one predicate, and a test asserts both endpoints of every range plus the codepoints immediately outside them — including U+2065, which sits inside the U+2060 block but is unassigned rather than `Cf`. Filtering happens inside the existing per-word filter, so a format-only name collapses to empty and falls out through the same `None` → npub path as a crud-only name. No new fallback logic. And because filtering precedes truncation, invisible padding cannot eat the 80-character budget. ## NUL is handled one layer up An interior NUL is a sibling constraint that cannot be fixed here: it makes `Command::env` fail the entire spawn before this code runs, so it has to die at the writer. #3028 establishes that pattern for the session title in `resolve_session_title` via `filter(|c| !c.is_control())`, and the Desktop follow-up that writes `BUZZ_ACP_DISPLAY_NAME` inherits it. The shim sanitizer is a second line of defense for values that arrive from somewhere other than Desktop. ## Verified end to end Driving the real `buzz-dev-mcp` binary over stdio MCP and committing inside its shimmed environment: ``` # BUZZ_ACP_DISPLAY_NAME="Duncan Idaho" Duncan Idaho <dcfd242e...0f95@buzz.block.builderlab.xyz> verify_exit=0 # BUZZ_ACP_DISPLAY_NAME unset npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e...0f95@buzz.block.builderlab.xyz> verify_exit=0 # BUZZ_ACP_DISPLAY_NAME=";;" (crud-only; would otherwise be fatal) npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e...0f95@buzz.block.builderlab.xyz> verify_exit=0 # BUZZ_ACP_DISPLAY_NAME=U+200B U+200B (format-only; would otherwise be blank) npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e...0f95@buzz.block.builderlab.xyz> verify_exit=0 # BUZZ_ACP_DISPLAY_NAME="Duncan" + U+202E (bidi override stripped) Duncan <dcfd242e...0f95@buzz.block.builderlab.xyz> verify_exit=0 # BUZZ_ACP_DISPLAY_NAME="Dun" + U+200B + "can" (zero-width removed, word not split) Duncan <dcfd242e...0f95@buzz.block.builderlab.xyz> verify_exit=0 ``` Signature verification passes in every case — the signing identity is unchanged. `Related: #3028` — it establishes the Desktop-side env plumbing this builds beside; the one-line Desktop follow-up that writes `BUZZ_ACP_DISPLAY_NAME` alongside the session title ships after it merges. Not a dependency: with the variable absent, `std::env::var` returns `Err` and the npub fallback keeps current behavior exactly. --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> |
||
|
|
5afa16157a |
fix(desktop): suppress Windows console flashes and reject WSL bash alias (#2587)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> |
||
|
|
a30d525155 |
fix(dev-mcp): remove buzz_send_message tool in favor of buzz CLI send path (#2043)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co> |
||
|
|
f308762852 |
feat(media): require auth for relay media reads (#1926)
Signed-off-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Tyler Longwell <tlongwell@block.xyz> |
||
|
|
54e174b5e1 | Chat-first agent creation via portable buzz CLI drafts (#1878) | ||
|
|
54638ff4bb | mesh: upgrade runtime, enforce membership, add shared compute provider (#1656) | ||
|
|
020ac7f405 |
fix(desktop): resolve Git Bash for Windows shell tool via PATH/git/registry fallback (#1821)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co> |
||
|
|
cfa2089831 |
refactor(shell): drop bundled PortableGit, add BUZZ_SHELL override + dialect hint (#1536)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co> |
||
|
|
73cc31cc52 |
chore: remove LLM-slop comments across the codebase (#1277)
Signed-off-by: Tyler Longwell <tlongwell@block.xyz> Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Max <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Mari <95cae996907d7cab9f5dbf43c0f53edeac6ab0b032a6feae4abfd784e467b3f5@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Quinn <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Sami <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Perci <5a968df9a7494b4e019b9ecf739e088ba61097b4312124e9a88ae5b42e3f5f3e@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Tyler Longwell <tlongwell@block.xyz> |
||
|
|
592508addd |
feat(windows): bundle full Git-for-Windows toolchain for the shell tool (#1145)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co> |
||
|
|
ec5cfa3796 |
fix(buzz-dev-mcp): resolve MSYS-absolute paths in the Windows file tools (#1169)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co> |
||
|
|
4d2eb52a6c |
fix(buzz-dev-mcp): resolve non-WSL bash so the MCP shell works on Windows (#1119)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co> |
||
|
|
4201311821 |
fix: make managed-agent spawn and teardown portable to Windows (#1097)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co> |
||
|
|
d99ad131f1 |
refactor: rename sprout backend to buzz (#958)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <wpfleger@block.xyz> Signed-off-by: Will Pfleger <wpfleger@squareup.com> Signed-off-by: Will Pfleger <wpfleger96@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co> Co-authored-by: npub1fgdl5qqnh3k3f2xkqrvt7cujalhm623x4s7fdjdj5yrtp5fzjl9qrjpucw <4a1bfa0013bc6d14a8d600d8bf6392efefbd2a26ac3c96c9b2a106b0d12297ca@sprout-oss.stage.blox.sqprod.co> Co-authored-by: npub16v54tttfqacx9ycvc3k0ut0npj564ahcuajzy6qjvh57ntmsf4uq4806j2 <d32955ad69077062930cc46cfe2df30ca9aaf6f8e76422681265e9e9af704d78@sprout-oss.stage.blox.sqprod.co> Co-authored-by: Will Pfleger <wpfleger96@gmail.com> |