diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 564cd74e9dd..f045221be56 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -125,8 +125,9 @@ jobs: max-parallelism = 2 - name: Log in to GHCR - # Skip on pull_request from forks — no GHCR creds, build-only. - if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository + # Only block/buzz owns the default registry caches. Downstream forks + # build pull requests without trying to write to Block's GHCR scope. + if: github.event_name != 'pull_request' || github.repository == 'block/buzz' uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4.2.0 with: registry: ghcr.io @@ -179,7 +180,7 @@ jobs: cache-from: | type=registry,ref=${{ env.IMAGE_NAME }}-buildcache:${{ matrix.arch }} cache-to: | - ${{ (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && format('type=registry,ref={0}-buildcache:{1},mode=max,compression=zstd', env.IMAGE_NAME, matrix.arch) || '' }} + ${{ (github.event_name != 'pull_request' || github.repository == 'block/buzz') && format('type=registry,ref={0}-buildcache:{1},mode=max,compression=zstd', env.IMAGE_NAME, matrix.arch) || '' }} - name: Build and push debug image by digest id: build-debug @@ -377,7 +378,7 @@ jobs: [worker.oci] max-parallelism = 2 - name: Log in to GHCR - if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository + if: github.event_name != 'pull_request' || github.repository == 'block/buzz' uses: docker/login-action@650006c6eb7dba73a995cc03b0b2d7f5ca915bee # v4.2.0 with: registry: ghcr.io @@ -402,7 +403,7 @@ jobs: labels: ${{ steps.meta.outputs.labels }} outputs: type=image,name=ghcr.io/block/buzz-push-gateway,push-by-digest=true,name-canonical=true,push=${{ github.event_name != 'pull_request' }} cache-from: type=registry,ref=ghcr.io/block/buzz-push-gateway-buildcache:${{ matrix.arch }} - cache-to: ${{ (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && format('type=registry,ref=ghcr.io/block/buzz-push-gateway-buildcache:{0},mode=max,compression=zstd', matrix.arch) || '' }} + cache-to: ${{ (github.event_name != 'pull_request' || github.repository == 'block/buzz') && format('type=registry,ref=ghcr.io/block/buzz-push-gateway-buildcache:{0},mode=max,compression=zstd', matrix.arch) || '' }} - name: Export digest if: github.event_name != 'pull_request' env: diff --git a/.release/desktop-candidate.json b/.release/desktop-candidate.json index 1ba64765ffe..cc2c2670191 100644 --- a/.release/desktop-candidate.json +++ b/.release/desktop-candidate.json @@ -1,8 +1,8 @@ { "schema": 1, - "version": "0.5.4", - "base_sha": "6de85fe31d781122756aecf954bae7d357a56b9a", - "previous_tag": "desktop-v0.5.3", - "tag": "desktop-v0.5.4", - "commit_count": 40 + "version": "0.5.5", + "base_sha": "25a9cf1be6d245fbd7373cb1160dbc790baf5bd5", + "previous_tag": "desktop-v0.5.4", + "tag": "desktop-v0.5.5", + "commit_count": 44 } diff --git a/AGENTS.md b/AGENTS.md index d6c59dae24d..bb46ce57f55 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -607,6 +607,7 @@ reconnects preserve pending avatar verification work): - `resetRenderScopedReactionHydration()` — reaction hydration cache - `clearSearchHitEventCache()` — search result event cache - `clearMarkdownNodeCache()` — markdown parse-node cache +- `resetLinkPreviewTitleCache()` — link preview title cache (Buzz entity titles come from relay events) **If you add a new module-level cache, Map, or class instance that holds community-scoped data, you must add its reset to `resetCommunityState()`.** diff --git a/CHANGELOG.md b/CHANGELOG.md index e30941a355b..171f260d3e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,59 @@ # Changelog +## v0.5.5 + +### Desktop and shared changes + +- feat: paste composer text without formatting ([#4801](https://github.com/block/buzz/pull/4801)) ([`25a9cf1be6d245fbd7373cb1160dbc790baf5bd5`](https://github.com/block/buzz/commit/25a9cf1be6d245fbd7373cb1160dbc790baf5bd5)) +- Revert "chore(release): release Buzz Desktop version 0.5.5" ([#4808](https://github.com/block/buzz/pull/4808)) ([`79c52166cfe6b6d36bdc7686f943595c74e2f578`](https://github.com/block/buzz/commit/79c52166cfe6b6d36bdc7686f943595c74e2f578)) +- chore(release): release Buzz Desktop version 0.5.5 ([#4800](https://github.com/block/buzz/pull/4800)) ([`a0ed13de14ee64dd90c32335790f7d3b4e94330d`](https://github.com/block/buzz/commit/a0ed13de14ee64dd90c32335790f7d3b4e94330d)) +- fix: reauthenticate databricks model discovery ([#4008](https://github.com/block/buzz/pull/4008)) ([`4a2305170eef565bf1836e2859247e67c030f8af`](https://github.com/block/buzz/commit/4a2305170eef565bf1836e2859247e67c030f8af)) +- Revert "chore(release): release Buzz Desktop version 0.5.5" ([#4797](https://github.com/block/buzz/pull/4797)) ([`8faf09f9aedb4989e57c7b6c5bd1052a444a3370`](https://github.com/block/buzz/commit/8faf09f9aedb4989e57c7b6c5bd1052a444a3370)) +- feat: Buzz entity links — rich preview cards + in-app navigation for repos, PRs, and issues ([#4695](https://github.com/block/buzz/pull/4695)) ([`a1d78f2959b41c63f063ff818076d38c31071a47`](https://github.com/block/buzz/commit/a1d78f2959b41c63f063ff818076d38c31071a47)) +- fix(desktop): serialize tray channel actions for frontend ([#4762](https://github.com/block/buzz/pull/4762)) ([`4c665aeac366fca5097eaa1088fb87f3d248eac7`](https://github.com/block/buzz/commit/4c665aeac366fca5097eaa1088fb87f3d248eac7)) +- chore(release): release Buzz Desktop version 0.5.5 ([#4788](https://github.com/block/buzz/pull/4788)) ([`b948c54792c4933b4e003d2b227dc6e1f7c05fb4`](https://github.com/block/buzz/commit/b948c54792c4933b4e003d2b227dc6e1f7c05fb4)) +- feat(projects): support multiple repositories ([#4671](https://github.com/block/buzz/pull/4671)) ([`e30db7028f9f1dc7646b5814ed03b4c54a4d2a48`](https://github.com/block/buzz/commit/e30db7028f9f1dc7646b5814ed03b4c54a4d2a48)) +- fix(desktop): widen post-Enter timeouts in empty-edit-delete spec ([#4792](https://github.com/block/buzz/pull/4792)) ([`7bcfe7e0a141900d6e1e5bd0b3bce488b57d6453`](https://github.com/block/buzz/commit/7bcfe7e0a141900d6e1e5bd0b3bce488b57d6453)) +- fix(desktop): wait for terminal frame before splash ([#4781](https://github.com/block/buzz/pull/4781)) ([`65f7a100353b9a5302da2614f2d85edee1c136a2`](https://github.com/block/buzz/commit/65f7a100353b9a5302da2614f2d85edee1c136a2)) +- fix(desktop): integer-align custom reaction emoji ([#4779](https://github.com/block/buzz/pull/4779)) ([`8b8d86c5d26e2fa8cf419fdd8d0e56433f95d71a`](https://github.com/block/buzz/commit/8b8d86c5d26e2fa8cf419fdd8d0e56433f95d71a)) +- Polish Huddle voice controls ([#4694](https://github.com/block/buzz/pull/4694)) ([`ce3cf3cd2591f132f286fbc0a42a9e6699d0b08d`](https://github.com/block/buzz/commit/ce3cf3cd2591f132f286fbc0a42a9e6699d0b08d)) +- fix(local-archive): default both archive settings to enabled ([#4750](https://github.com/block/buzz/pull/4750)) ([`5179726737108a4a91076d262c30a53d4a7237e9`](https://github.com/block/buzz/commit/5179726737108a4a91076d262c30a53d4a7237e9)) +- fix(desktop): close reconnect gaps that previously required CMD+R ([#4737](https://github.com/block/buzz/pull/4737)) ([`e5efd047050f5e2a64fe6cd9e3faed1685b03f5c`](https://github.com/block/buzz/commit/e5efd047050f5e2a64fe6cd9e3faed1685b03f5c)) +- Dock Buzz Term within channel workspace ([#4724](https://github.com/block/buzz/pull/4724)) ([`cb4a73e17d0760eba6c3c01811da07e1d3a6b85e`](https://github.com/block/buzz/commit/cb4a73e17d0760eba6c3c01811da07e1d3a6b85e)) +- fix(agents): canonicalize stale persona harness pins ([#4631](https://github.com/block/buzz/pull/4631)) ([`0c33a8a55f0aa0763f8d65ad90dc8af56215d2e8`](https://github.com/block/buzz/commit/0c33a8a55f0aa0763f8d65ad90dc8af56215d2e8)) +- Refine community invite links ([#4734](https://github.com/block/buzz/pull/4734)) ([`e1287c92cc7ea9b52f10b80515b98cdd1c7f9a31`](https://github.com/block/buzz/commit/e1287c92cc7ea9b52f10b80515b98cdd1c7f9a31)) +- feat(desktop): persist sidebar observed-unread across webview reload ([#3976](https://github.com/block/buzz/pull/3976)) ([`0afeac8a7c173fd3ede8a22e27919e63161bf07c`](https://github.com/block/buzz/commit/0afeac8a7c173fd3ede8a22e27919e63161bf07c)) +- feat(desktop): surface config diff in restart-required badge ([#3637](https://github.com/block/buzz/pull/3637)) ([`f86dfc58838a272a5d0504ebf216a79b7288f027`](https://github.com/block/buzz/commit/f86dfc58838a272a5d0504ebf216a79b7288f027)) +- Polish sidebar unread hierarchy ([#4573](https://github.com/block/buzz/pull/4573)) ([`540b58920cef205b838da8be8442aae62bceaaa5`](https://github.com/block/buzz/commit/540b58920cef205b838da8be8442aae62bceaaa5)) +- fix(desktop): show cached display names on startup ([#3317](https://github.com/block/buzz/pull/3317)) ([`d0d4acd4fa02893ad2460b447d7e13da00506be3`](https://github.com/block/buzz/commit/d0d4acd4fa02893ad2460b447d7e13da00506be3)) +- Remove blur from Welcome composer guidance ([#4691](https://github.com/block/buzz/pull/4691)) ([`d0af845a1d489ab3fce6a73adbb0e82ebb4b0fa1`](https://github.com/block/buzz/commit/d0af845a1d489ab3fce6a73adbb0e82ebb4b0fa1)) +- Refine desktop timeline activity presentation ([#4582](https://github.com/block/buzz/pull/4582)) ([`a5bf3c5ae1e2f3b9a1783cd90b859d027fc92b9a`](https://github.com/block/buzz/commit/a5bf3c5ae1e2f3b9a1783cd90b859d027fc92b9a)) +- Defer desktop media uploads until send ([#4522](https://github.com/block/buzz/pull/4522)) ([`f18a9cb10688deaa3f618869170bfe9303c4be62`](https://github.com/block/buzz/commit/f18a9cb10688deaa3f618869170bfe9303c4be62)) +- fix(desktop): stop clipping focus ring on channel intro action cards (#2392) ([#4374](https://github.com/block/buzz/pull/4374)) ([`ddcf0aef9f1b3c81ec5a9b709dd62d2fcc773996`](https://github.com/block/buzz/commit/ddcf0aef9f1b3c81ec5a9b709dd62d2fcc773996)) +- Polish mobile inbox and media flows ([#4512](https://github.com/block/buzz/pull/4512)) ([`feccf4eabc23fdba94ce3537a194357ed17b197c`](https://github.com/block/buzz/commit/feccf4eabc23fdba94ce3537a194357ed17b197c)) +- feat: ship Buzz Term ([#4347](https://github.com/block/buzz/pull/4347)) ([`631b05c883f58e9533e9038b4669ebdfb1d9cf27`](https://github.com/block/buzz/commit/631b05c883f58e9533e9038b4669ebdfb1d9cf27)) +- feat(mobile): sync per-group channel sorting ([#4231](https://github.com/block/buzz/pull/4231)) ([`b42b093613edfb7138acb0961a0ad9218b39691a`](https://github.com/block/buzz/commit/b42b093613edfb7138acb0961a0ad9218b39691a)) +- feat(desktop): redesign the Huddle experience ([#4281](https://github.com/block/buzz/pull/4281)) ([`b29c8cdaa456307ecdd63e565de4beb14402128e`](https://github.com/block/buzz/commit/b29c8cdaa456307ecdd63e565de4beb14402128e)) +- feat(agents): model-tuning parity in global Agent Defaults editor ([#4578](https://github.com/block/buzz/pull/4578)) ([`985cdcc6eac33ccd77bc50c26e22c701d07eda4e`](https://github.com/block/buzz/commit/985cdcc6eac33ccd77bc50c26e22c701d07eda4e)) +- Polish Share Compute settings ([#3735](https://github.com/block/buzz/pull/3735)) ([`027a74a61c8643a1d1086d3e8307fad89d7735f7`](https://github.com/block/buzz/commit/027a74a61c8643a1d1086d3e8307fad89d7735f7)) +- fix(reactions): wrap long popover names ([#3834](https://github.com/block/buzz/pull/3834)) ([`79815978483ef0ab78f7159c0add3492da6457a1`](https://github.com/block/buzz/commit/79815978483ef0ab78f7159c0add3492da6457a1)) +- fix(desktop): clarify inherited agent parallelism ([#4010](https://github.com/block/buzz/pull/4010)) ([`d4a4570b9769743899d97480b3bf482860b51d9c`](https://github.com/block/buzz/commit/d4a4570b9769743899d97480b3bf482860b51d9c)) +- feat(desktop): make onboarding model defaults skippable ([#3968](https://github.com/block/buzz/pull/3968)) ([`5c98932c59ee5344e9e8c14525c51f3de16ad2c2`](https://github.com/block/buzz/commit/5c98932c59ee5344e9e8c14525c51f3de16ad2c2)) + +### Other repository changes + +- fix(ci): make desktop cache test version agnostic ([#4791](https://github.com/block/buzz/pull/4791)) ([`383d9e1eafd569b44b9c835200dba69ef7cec9dc`](https://github.com/block/buzz/commit/383d9e1eafd569b44b9c835200dba69ef7cec9dc)) +- fix(mobile): stop oversized read-state retry loop ([#4595](https://github.com/block/buzz/pull/4595)) ([`7bee84da8267605ada939c4f911d90f1b0ff1a11`](https://github.com/block/buzz/commit/7bee84da8267605ada939c4f911d90f1b0ff1a11)) +- perf(relay): index channel-id lookups and skip trace-only reads ([#4647](https://github.com/block/buzz/pull/4647)) ([`bc9e6528a7ba6007c5a25f6a0aca9c05d72e9d2c`](https://github.com/block/buzz/commit/bc9e6528a7ba6007c5a25f6a0aca9c05d72e9d2c)) +- docs(acp): explain per-channel session model in base prompt ([#4729](https://github.com/block/buzz/pull/4729)) ([`56003ebf98c22367fb6357f295494e26efbd8ae6`](https://github.com/block/buzz/commit/56003ebf98c22367fb6357f295494e26efbd8ae6)) +- docs(nip-am): normative amendment — cache SHOULD/MUST + pricingIdentity + consumer cost guidance ([#4632](https://github.com/block/buzz/pull/4632)) ([`0542bc8b955756a62b4133aa70f84441d93616ee`](https://github.com/block/buzz/commit/0542bc8b955756a62b4133aa70f84441d93616ee)) +- feat(mobile): add channel scroll navigation ([#4239](https://github.com/block/buzz/pull/4239)) ([`d5da74e4e078a9551b9ce9e47e77cf9ed5840596`](https://github.com/block/buzz/commit/d5da74e4e078a9551b9ce9e47e77cf9ed5840596)) +- feat(mobile): bring channel menus to desktop parity ([#3940](https://github.com/block/buzz/pull/3940)) ([`ede8d22dd5b336f146e0a6d760fd9dff78a42613`](https://github.com/block/buzz/commit/ede8d22dd5b336f146e0a6d760fd9dff78a42613)) +- ci: add guarded desktop release cache prewarm ([#4575](https://github.com/block/buzz/pull/4575)) ([`e1f6da7c42b0cac6f307023f0479e1e2c3a6d1c0`](https://github.com/block/buzz/commit/e1f6da7c42b0cac6f307023f0479e1e2c3a6d1c0)) +- fix(mobile): recover stale relay sessions ([#4372](https://github.com/block/buzz/pull/4372)) ([`ce56e34411d2940e70a6c0de653ffae36d334701`](https://github.com/block/buzz/commit/ce56e34411d2940e70a6c0de653ffae36d334701)) + +[Compare desktop-v0.5.4...desktop-v0.5.5](https://github.com/block/buzz/compare/desktop-v0.5.4...desktop-v0.5.5) + ## v0.5.4 ### Desktop and shared changes diff --git a/Justfile b/Justfile index d80341ecacf..c3d755ffeb2 100644 --- a/Justfile +++ b/Justfile @@ -213,24 +213,17 @@ desktop-terminal-performance-test: cargo test --manifest-path desktop/src-tauri/crates/buzz-terminal/Cargo.toml --release --test latency g3_renderer_acquire_stays_within_frame_budget -- --ignored --exact --nocapture # Verify compiled-flag behavior under both compile states (clean + internal). -# Runs the observer_archive focused test twice with independently supplied +# Runs the auto-connect compiled-flag test twice with independently supplied # expected values; build.rs rerun-if-env-changed triggers recompilation. desktop-tauri-test-compiled-flags: _ensure-sidecar-stubs #!/usr/bin/env bash set -euo pipefail cd desktop/src-tauri echo "=== Clean build (no flag) → expect false ===" - env -u BUZZ_BUILD_OBSERVER_ARCHIVE_DEFAULT \ - -u BUZZ_BUILD_AUTO_CONNECT_DEFAULT_RELAY \ - BUZZ_TEST_EXPECTED_OBSERVER_ARCHIVE_DEFAULT=false \ - cargo test observer_archive_default_enabled_matches_expected -- --ignored --nocapture env -u BUZZ_BUILD_AUTO_CONNECT_DEFAULT_RELAY \ BUZZ_TEST_EXPECTED_AUTO_CONNECT_DEFAULT_RELAY=false \ cargo test compiled_flag_matches_expected -- --ignored --nocapture - echo "=== Internal build (flags set) → expect true ===" - BUZZ_BUILD_OBSERVER_ARCHIVE_DEFAULT=1 \ - BUZZ_TEST_EXPECTED_OBSERVER_ARCHIVE_DEFAULT=true \ - cargo test observer_archive_default_enabled_matches_expected -- --ignored --nocapture + echo "=== Internal build (flag set) → expect true ===" BUZZ_BUILD_AUTO_CONNECT_DEFAULT_RELAY=1 \ BUZZ_TEST_EXPECTED_AUTO_CONNECT_DEFAULT_RELAY=true \ cargo test compiled_flag_matches_expected -- --ignored --nocapture diff --git a/crates/buzz-acp/src/base_prompt.md b/crates/buzz-acp/src/base_prompt.md index 12e5c42909e..1d85221f113 100644 --- a/crates/buzz-acp/src/base_prompt.md +++ b/crates/buzz-acp/src/base_prompt.md @@ -23,6 +23,7 @@ The `buzz` CLI is your primary interface. Auth env vars: `BUZZ_RELAY_URL`, `BUZZ | `buzz feed` | `get` | | `buzz social` | `publish`, `notes` | | `buzz repos` | `create`, `get`, `list` | +| `buzz issues` | `create`, `get`, `list`, `status` | | `buzz pr` | `open`, `update`, `get`, `list`, `status` | | `buzz upload` | `file` | @@ -30,6 +31,8 @@ Run `buzz --help` or `buzz --help` for full usage. For multiline message When opening a pull request in response to channel work, always pass `--channel ` using the UUID from `[Context]`. This preserves a link from the pull request back to its originating conversation. +`buzz pr open`, `buzz issues create`, and `buzz repos create` return a `link` field (a `buzz://` deep link). When you announce that work in a channel message, include the `link` value verbatim — Buzz Desktop renders it as a rich preview card that opens the PR, issue, or repo in-app, the same way GitHub links render. Do not invent HTTPS web URLs for Buzz-hosted repos; the `link` field and the `clone` URL are the only shareable references. + ## Conversational Agent Creation When someone asks to create an agent, ask for at most two things: the agent's name and what it should do day-to-day. Turn the user's rough purpose into the `--system-prompt` yourself; do not separately ask for purpose, tone, constraints, access, runtime, provider, or model unless the user's request is genuinely ambiguous. diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 64edf68ee26..ddc0330d9f2 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -31,8 +31,8 @@ use uuid::Uuid; use crate::acp::{ extract_model_config_options, extract_model_state, model_in_catalog, - resolve_model_switch_method, AcpClient, AcpError, McpServer, ModelSwitchMethod, StopReason, - SystemPromptTransport, + resolve_model_switch_method, AcpClient, AcpError, EnvVar, McpServer, ModelSwitchMethod, + StopReason, SystemPromptTransport, }; use crate::config::{compose_session_title, DedupMode, PermissionMode}; use crate::observer; @@ -867,13 +867,13 @@ const UNKNOWN_CHANNEL_NAME: &str = "unknown"; async fn resolve_new_session_channel_context( channel_info: &ChannelInfoResolver, channel_id: Uuid, -) -> (bool, Option) { +) -> (bool, Option, Option) { let Some(info) = channel_info.resolve(channel_id).await else { - return (true, None); + return (true, None, None); }; let is_dm = info.channel_type == "dm"; let title_channel = (!is_dm && info.name != UNKNOWN_CHANNEL_NAME).then_some(info.name); - (is_dm, title_channel) + (is_dm, title_channel, Some(info.channel_type)) } /// Create a new ACP session via `session_new_full()`, populate model capabilities @@ -888,6 +888,8 @@ async fn create_session_and_apply_model( agent_core: Option<&str>, agent_canvas: Option<&str>, channel_name: Option<&str>, + channel_id: Option, + channel_type: Option<&str>, ) -> Result { // Build base_prompt + system_prompt + agent core + canvas metadata into a // single prompt. Standard protocol-v2 agents receive it in `session/new`; @@ -911,12 +913,18 @@ async fn create_session_and_apply_model( .session_title .as_deref() .map(|agent_name| compose_session_title(agent_name, channel_name)); + let mcp_servers = mcp_servers_with_git_origin( + &ctx.mcp_servers, + channel_id, + channel_type, + ctx.session_title.as_deref(), + ); let resp = agent .acp .session_new_full( &ctx.cwd, - ctx.mcp_servers.clone(), + mcp_servers, session_new_system_prompt( is_goose, agent.protocol_version, @@ -1019,6 +1027,34 @@ async fn create_session_and_apply_model( Ok(resp.session_id) } +fn mcp_servers_with_git_origin( + servers: &[McpServer], + channel_id: Option, + channel_type: Option<&str>, + agent_name: Option<&str>, +) -> Vec { + let mut servers = servers.to_vec(); + let origin = match (channel_id, channel_type) { + (Some(channel_id), Some("stream")) => Some(EnvVar { + name: "BUZZ_GIT_ORIGIN_CHANNEL_ID".into(), + value: channel_id.to_string(), + }), + (Some(_), _) => agent_name + .filter(|name| !name.trim().is_empty()) + .map(|name| EnvVar { + name: "BUZZ_GIT_ORIGIN_AGENT_NAME".into(), + value: name.trim().to_string(), + }), + (None, _) => None, + }; + if let Some(origin) = origin { + for server in &mut servers { + server.env.push(origin.clone()); + } + } + servers +} + /// Send the appropriate ACP model-switch request with a timeout. /// /// On timeout or error, logs a warning and returns — the caller proceeds @@ -1519,14 +1555,15 @@ pub async fn run_prompt_task( // Channel name for the session title, from the same single resolve the // canvas DM check uses — see `resolve_new_session_channel_context`. let mut title_channel: Option = None; + let mut origin_channel_type: Option = None; if let PromptSource::Channel(cid) = &source { let is_new_channel_session = !agent.state.sessions.contains_key(cid); let needs_canvas = is_new_channel_session && !agent.state.canvas_sections.contains_key(cid); - let needs_title = is_new_channel_session && ctx.session_title.is_some(); - if needs_canvas || needs_title { - let (is_dm, resolved_channel) = + if is_new_channel_session { + let (is_dm, resolved_channel, resolved_channel_type) = resolve_new_session_channel_context(&ctx.channel_info, *cid).await; title_channel = resolved_channel; + origin_channel_type = resolved_channel_type; // A confirmed DM never receives a canvas section; an undeterminable // channel type fails closed as a DM for the same reason. if needs_canvas && !is_dm { @@ -1571,6 +1608,8 @@ pub async fn run_prompt_task( agent_core.as_deref(), agent_canvas.as_deref(), title_channel.as_deref(), + Some(*cid), + origin_channel_type.as_deref(), ) .await { @@ -1618,7 +1657,9 @@ pub async fn run_prompt_task( if let Some(sid) = &agent.state.heartbeat_session { (sid.clone(), false) } else { - match create_session_and_apply_model(&mut agent, &ctx, None, None, None).await { + match create_session_and_apply_model(&mut agent, &ctx, None, None, None, None, None) + .await + { Ok(sid) => { tracing::info!( target: "pool::session", @@ -3989,6 +4030,50 @@ mod tests { use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; use serde_json::json; + fn test_mcp_server() -> McpServer { + McpServer { + name: "dev".into(), + command: "buzz-dev-mcp".into(), + args: vec![], + env: vec![], + } + } + + #[test] + fn public_session_forwards_channel_origin_to_mcp() { + let channel_id = Uuid::new_v4(); + let servers = mcp_servers_with_git_origin( + &[test_mcp_server()], + Some(channel_id), + Some("stream"), + None, + ); + assert!(servers[0].env.iter().any(|entry| { + entry.name == "BUZZ_GIT_ORIGIN_CHANNEL_ID" && entry.value == channel_id.to_string() + })); + assert!(!servers[0] + .env + .iter() + .any(|entry| entry.name == "BUZZ_GIT_ORIGIN_AGENT_NAME")); + } + + #[test] + fn private_session_forwards_agent_name_without_channel_id() { + let servers = mcp_servers_with_git_origin( + &[test_mcp_server()], + Some(Uuid::new_v4()), + Some("dm"), + Some("Builder"), + ); + assert!(servers[0].env.iter().any(|entry| { + entry.name == "BUZZ_GIT_ORIGIN_AGENT_NAME" && entry.value == "Builder" + })); + assert!(!servers[0] + .env + .iter() + .any(|entry| entry.name == "BUZZ_GIT_ORIGIN_CHANNEL_ID")); + } + // These pin the initial_message dispatch path (run_prompt_task, ~line 855): // a legacy agent WITH a base_prompt must get [Base] prepended to the user // message. This is the exact regression that shipped in the round-2 bug. @@ -6833,12 +6918,14 @@ mod tests { let response = channel_metadata_response(id, &[["name", "buzz-dev"], ["t", "stream"]]); let (resolver, requests, server) = counting_resolver(response).await; - let (is_dm, title_channel) = resolve_new_session_channel_context(&resolver, id).await; + let (is_dm, title_channel, channel_type) = + resolve_new_session_channel_context(&resolver, id).await; assert!(!is_dm, "a stream channel is not a DM"); assert_eq!(title_channel.as_deref(), Some("buzz-dev")); + assert_eq!(channel_type.as_deref(), Some("stream")); assert_eq!(requests.load(Ordering::SeqCst), 1); - let (_, again) = resolve_new_session_channel_context(&resolver, id).await; + let (_, again, _) = resolve_new_session_channel_context(&resolver, id).await; assert_eq!(again.as_deref(), Some("buzz-dev")); assert_eq!( requests.load(Ordering::SeqCst), @@ -6856,8 +6943,10 @@ mod tests { let response = channel_metadata_response(id, &[["name", "DM"], ["t", "dm"]]); let (resolver, _requests, server) = counting_resolver(response).await; - let (is_dm, title_channel) = resolve_new_session_channel_context(&resolver, id).await; + let (is_dm, title_channel, channel_type) = + resolve_new_session_channel_context(&resolver, id).await; assert!(is_dm); + assert_eq!(channel_type.as_deref(), Some("dm")); assert_eq!( title_channel, None, "a DM name must never reach the session title" @@ -6874,7 +6963,7 @@ mod tests { let response = channel_metadata_response(id, &[["t", "stream"]]); let (resolver, _requests, server) = counting_resolver(response).await; - let (is_dm, title_channel) = resolve_new_session_channel_context(&resolver, id).await; + let (is_dm, title_channel, _) = resolve_new_session_channel_context(&resolver, id).await; assert!(!is_dm, "a nameless stream channel is still not a DM"); assert_eq!( title_channel, None, @@ -6894,10 +6983,11 @@ mod tests { let (resolver, requests, server) = counting_resolver(json!([])).await; - let (is_dm, title_channel) = + let (is_dm, title_channel, channel_type) = resolve_new_session_channel_context(&resolver, Uuid::new_v4()).await; assert!(is_dm, "an undeterminable channel type must fail closed"); assert_eq!(title_channel, None, "unresolved channels get a bare title"); + assert_eq!(channel_type, None); assert_eq!( requests.load(Ordering::SeqCst), 2, diff --git a/crates/buzz-agent/src/catalog.rs b/crates/buzz-agent/src/catalog.rs index aa2a121c996..0aaa2da7ea5 100644 --- a/crates/buzz-agent/src/catalog.rs +++ b/crates/buzz-agent/src/catalog.rs @@ -7,12 +7,17 @@ //! //! - Static bearer (`DATABRICKS_TOKEN`): returned immediately. //! - PKCE cache hit: returned from disk without a network round-trip. -//! - PKCE cache empty / no token: returns `Err(AgentError::LlmAuth)` — the -//! caller degrades gracefully; no browser, no hang. +//! - PKCE cache empty / no token: returns `Err(AgentError::LlmAuth)`. +//! +//! This helper never opens a browser. Callers choose whether to reject, degrade, +//! or start a separate interactive authentication flow. + +use std::sync::Arc; use reqwest::Client; use crate::{ + auth::TokenSource, config::{Config, Provider}, llm::build_token_source, types::AgentError, @@ -26,57 +31,22 @@ pub struct ModelEntry { pub name: String, } -/// Known Databricks AI Gateway v2 models — used as a fallback when the -/// `api/ai-gateway/v2/endpoints` call returns an empty list. +/// Known Databricks AI Gateway v2 models — used only when an authenticated +/// `api/ai-gateway/v2/endpoints` call succeeds with an empty list. /// Mirrors goose's `DATABRICKS_V2_KNOWN_MODELS`. pub const DATABRICKS_V2_KNOWN_MODELS: &[&str] = &["databricks-gpt-5-5", "databricks-claude-opus-4-7"]; -/// Returns the discovery-failure fallback catalog for a Databricks provider. -/// -/// This is the list of models advertised by `session/new` when -/// `discover_databricks_models` returns an error (e.g., no token available). -/// -/// - `DatabricksV2` falls back to the configured model plus -/// [`DATABRICKS_V2_KNOWN_MODELS`] so the model-picker is always populated for -/// AI Gateway v2 users. The configured model leads: without it a fallback -/// catalog can omit the very model the agent is running, leaving the picker -/// unable to represent the current selection. -/// - Legacy `Databricks` falls back to only the configured model — the -/// `DATABRICKS_V2_KNOWN_MODELS` IDs are AI Gateway v2 endpoints that the -/// `/serving-endpoints/{model}/invocations` API may not serve. -/// -/// Extracting this as a pure function makes the split testable without -/// spawning an async runtime or making network calls. -pub fn discovery_failure_fallback(provider: Provider, configured_model: &str) -> Vec { - // `resolve_model` does not trim, so a padded `DATABRICKS_MODEL` reaches here: - // normalize once, or the dedupe below misses and the picker lists the model - // twice (once padded, once from the known slate). - let configured_model = configured_model.trim(); - let configured = ModelEntry { - id: configured_model.to_string(), - name: configured_model.to_string(), - }; - match provider { - Provider::DatabricksV2 => { - let mut entries = Vec::with_capacity(DATABRICKS_V2_KNOWN_MODELS.len() + 1); - if !configured_model.is_empty() { - entries.push(configured); - } - entries.extend( - DATABRICKS_V2_KNOWN_MODELS - .iter() - .filter(|id| **id != configured_model) - .map(|id| ModelEntry { - id: id.to_string(), - name: id.to_string(), - }), - ); - entries - } - Provider::Databricks => vec![configured], - _ => vec![configured], - } +const AUTHENTICATED_EMPTY_CATALOG_SUFFIX: &str = " (default catalog)"; + +fn authenticated_empty_v2_catalog() -> Vec { + DATABRICKS_V2_KNOWN_MODELS + .iter() + .map(|id| ModelEntry { + id: id.to_string(), + name: format!("{id}{AUTHENTICATED_EMPTY_CATALOG_SUFFIX}"), + }) + .collect() } /// Heuristic: `true` when a v2 AI Gateway endpoint name looks like it serves @@ -109,23 +79,47 @@ pub(crate) fn is_chat_capable_endpoint(name: &str) -> bool { /// /// Returns a non-empty `Vec` on success. Returns /// `Err(AgentError::LlmAuth)` when no token is available (no static token, -/// no PKCE cache) — callers should degrade gracefully rather than hanging. +/// no PKCE cache). The helper itself never starts interactive authentication. /// /// # Panics /// Never panics. pub async fn discover_databricks_models(cfg: &Config) -> Result, AgentError> { - let token_source = build_token_source(cfg)?; - let bearer = token_source.bearer_no_browser().await?; + discover_databricks_models_with_token_source(cfg, build_token_source(cfg)?).await +} +async fn discover_databricks_models_with_token_source( + cfg: &Config, + token_source: Arc, +) -> Result, AgentError> { + let mut bearer = token_source.bearer_no_browser().await?; let http = Client::new(); let host = cfg.base_url.trim_end_matches('/'); + let mut refreshed = false; + + loop { + let result = match cfg.provider { + Provider::Databricks => fetch_v1_models(&http, host, &bearer).await, + Provider::DatabricksV2 => fetch_v2_models(&http, host, &bearer).await, + _ => { + return Err(AgentError::InvalidParams( + "discover_databricks_models called for non-Databricks provider".into(), + )); + } + }; - match cfg.provider { - Provider::Databricks => fetch_v1_models(&http, host, &bearer).await, - Provider::DatabricksV2 => fetch_v2_models(&http, host, &bearer).await, - _ => Err(AgentError::InvalidParams( - "discover_databricks_models called for non-Databricks provider".into(), - )), + match result { + Err(AgentError::LlmAuth(_)) if !refreshed => { + refreshed = true; + let fresh = token_source.refresh_now(&bearer).await?; + if fresh == bearer { + return Err(AgentError::LlmAuth( + "Databricks rejected the configured credential".into(), + )); + } + bearer = fresh; + } + result => return result, + } } } @@ -149,6 +143,11 @@ async fn fetch_v1_models( let status = response.status(); if !status.is_success() { let body = response.text().await.unwrap_or_default(); + if status.as_u16() == 401 { + return Err(AgentError::LlmAuth(format!( + "Databricks model discovery HTTP {status}" + ))); + } return Err(AgentError::Llm(format!( "Databricks model discovery HTTP {status}: {body}" ))); @@ -264,6 +263,11 @@ async fn fetch_v2_models( let status = response.status(); if !status.is_success() { let body = response.text().await.unwrap_or_default(); + if status.as_u16() == 401 { + return Err(AgentError::LlmAuth(format!( + "Databricks v2 model discovery HTTP {status}" + ))); + } return Err(AgentError::Llm(format!( "Databricks v2 model discovery HTTP {status}: {body}" ))); @@ -286,13 +290,7 @@ async fn fetch_v2_models( // Fall back to known-model list if the API returned nothing. if all_endpoints.is_empty() { - return Ok(DATABRICKS_V2_KNOWN_MODELS - .iter() - .map(|id| ModelEntry { - id: id.to_string(), - name: id.to_string(), - }) - .collect()); + return Ok(authenticated_empty_v2_catalog()); } sort_v2_endpoints_newest_first(&mut all_endpoints); @@ -396,6 +394,77 @@ pub(crate) fn parse_v2_endpoints_page( #[cfg(test)] mod tests { use super::*; + use async_trait::async_trait; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct RefreshingTestTokenSource { + refreshes: AtomicUsize, + } + + #[async_trait] + impl TokenSource for RefreshingTestTokenSource { + async fn bearer(&self) -> Result { + Ok("rejected".into()) + } + + async fn refresh_now(&self, rejected: &str) -> Result { + assert_eq!(rejected, "rejected"); + self.refreshes.fetch_add(1, Ordering::SeqCst); + Ok("fresh".into()) + } + } + + #[tokio::test] + async fn discovery_refreshes_rejected_bearer_once_then_retries_successfully() { + use axum::{ + extract::Query, + http::{HeaderMap, StatusCode}, + routing::get, + Json, Router, + }; + use std::collections::HashMap; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let host = format!("http://{}", listener.local_addr().unwrap()); + let requests = Arc::new(AtomicUsize::new(0)); + let requests_for_route = requests.clone(); + let app = Router::new().route( + "/api/ai-gateway/v2/endpoints", + get( + move |headers: HeaderMap, Query(_query): Query>| { + let requests = requests_for_route.clone(); + async move { + requests.fetch_add(1, Ordering::SeqCst); + match headers + .get("authorization") + .and_then(|value| value.to_str().ok()) + { + Some("Bearer fresh") => Ok(Json(serde_json::json!({ + "endpoints": [{"name": "discovered-model"}], + "next_page_token": null, + }))), + _ => Err((StatusCode::UNAUTHORIZED, "rejected")), + } + } + }, + ), + ); + tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let source = Arc::new(RefreshingTestTokenSource { + refreshes: AtomicUsize::new(0), + }); + let cfg = Config::for_discovery(Provider::DatabricksV2, String::new(), host); + let models = discover_databricks_models_with_token_source(&cfg, source.clone()) + .await + .unwrap(); + + assert_eq!(models[0].id, "discovered-model"); + assert_eq!(source.refreshes.load(Ordering::SeqCst), 1); + assert_eq!(requests.load(Ordering::SeqCst), 2); + } #[test] fn v1_parse_filters_ready_chat_endpoints() { @@ -574,6 +643,17 @@ mod tests { ); } + #[test] + fn authenticated_empty_v2_catalog_marks_fallback_provenance() { + let models = authenticated_empty_v2_catalog(); + let ids: Vec<&str> = models.iter().map(|model| model.id.as_str()).collect(); + + assert_eq!(ids, DATABRICKS_V2_KNOWN_MODELS); + assert!(models.iter().all(|model| { + model.name == format!("{}{AUTHENTICATED_EMPTY_CATALOG_SUFFIX}", model.id) + })); + } + #[test] fn is_chat_capable_endpoint_keeps_unrecognised_names() { // Prefer including over silently dropping — an unknown family is kept. @@ -585,47 +665,4 @@ mod tests { assert!(!is_chat_capable_endpoint("databricks-gte-large-en")); assert!(!is_chat_capable_endpoint("databricks-qwen3-embedding-0-6b")); } - - #[test] - fn v2_discovery_failure_fallback_leads_with_configured_model() { - let result = discovery_failure_fallback(Provider::DatabricksV2, "databricks-claude-opus-5"); - let ids: Vec<&str> = result.iter().map(|m| m.id.as_str()).collect(); - - // The running model must be representable in the picker even when - // discovery failed, so it leads the fallback catalog. - assert_eq!(ids.first(), Some(&"databricks-claude-opus-5")); - for known in DATABRICKS_V2_KNOWN_MODELS { - assert!(ids.contains(known), "fallback must retain '{known}'"); - } - } - - #[test] - fn v2_discovery_failure_fallback_does_not_duplicate_configured_model() { - let configured = DATABRICKS_V2_KNOWN_MODELS[0]; - let result = discovery_failure_fallback(Provider::DatabricksV2, configured); - let occurrences = result.iter().filter(|m| m.id == configured).count(); - assert_eq!(occurrences, 1, "got: {result:?}"); - assert_eq!(result.len(), DATABRICKS_V2_KNOWN_MODELS.len()); - } - - #[test] - fn v2_discovery_failure_fallback_tolerates_blank_configured_model() { - for configured in ["", " "] { - let result = discovery_failure_fallback(Provider::DatabricksV2, configured); - let ids: Vec<&str> = result.iter().map(|m| m.id.as_str()).collect(); - assert_eq!(ids, DATABRICKS_V2_KNOWN_MODELS.to_vec()); - } - } - - #[test] - fn v2_discovery_failure_fallback_dedupes_a_padded_configured_model() { - // `DATABRICKS_MODEL=" databricks-gpt-5-5 "` reaches here untrimmed, and an - // untrimmed comparison would list the model twice — once padded, once from - // the known slate. - let configured = DATABRICKS_V2_KNOWN_MODELS[0]; - let result = - discovery_failure_fallback(Provider::DatabricksV2, &format!(" {configured} ")); - let ids: Vec<&str> = result.iter().map(|m| m.id.as_str()).collect(); - assert_eq!(ids, DATABRICKS_V2_KNOWN_MODELS.to_vec()); - } } diff --git a/crates/buzz-agent/src/lib.rs b/crates/buzz-agent/src/lib.rs index 6cd7b6808ff..940bd2a9c2a 100644 --- a/crates/buzz-agent/src/lib.rs +++ b/crates/buzz-agent/src/lib.rs @@ -54,10 +54,10 @@ struct App { llm: Arc, sessions: Mutex>, /// Cached model catalog for Databricks providers. Populated lazily on the - /// first successful `session/new` discovery call. When discovery fails (e.g. - /// auth missing or a transient network error) the cell is intentionally left - /// empty so the next `session/new` call retries — a transient failure never - /// pins the degraded fallback catalog for the process lifetime. + /// first successful `session/new` discovery call. Failed discovery is never + /// cached: static-token authentication errors reject session creation, while + /// OAuth authentication and non-auth errors use the configured model for that + /// response and retry on the next session. models_cache: tokio::sync::OnceCell>, } @@ -135,6 +135,12 @@ pub fn run() -> Result<(), Box> { Ok(()) } +pub async fn authenticate_databricks(host: &str) -> Result<(), AgentError> { + auth::PkceOAuthTokenSource::new(llm::databricks_pkce_config(host))? + .interactive_login() + .await +} + /// `buzz-agent auth ` — run the interactive auth flow for a /// provider and persist the result, then exit. Today this supports Databricks /// OAuth 2.0 PKCE. Reads `DATABRICKS_HOST` from env; needs a browser on the @@ -145,18 +151,7 @@ async fn auth_subcommand(args: &[String]) -> Result<(), Box { let host = std::env::var("DATABRICKS_HOST") .map_err(|_| "auth databricks: DATABRICKS_HOST required")?; - let pkce = auth::PkceOAuthConfig { - discovery_url: format!( - "{}/oidc/.well-known/oauth-authorization-server", - host.trim_end_matches('/') - ), - client_id: "databricks-cli".into(), - scopes: vec!["all-apis".into(), "offline_access".into()], - cache_namespace: "databricks".into(), - cache_dir_override: None, - }; - let src = auth::PkceOAuthTokenSource::new(pkce)?; - src.interactive_login().await?; + authenticate_databricks(&host).await?; eprintln!("Authenticated. Token cached under ~/.config/buzz-agent/oauth/databricks/."); Ok(()) } @@ -317,26 +312,27 @@ async fn initialize(id: Value, params: Value, wire_tx: &WireSender) { /// /// Tries to use a previously-cached successful discovery result. If the cache is empty, /// runs `discover` and — on success — populates the cache for future calls. On failure -/// the cell is intentionally left empty so the next session retries; the provider-aware -/// fallback is returned for the immediate response only. +/// the error is returned and the cell is intentionally left empty so the next session retries. /// /// Extracted from `session_new` so that tests can drive this path with an injected /// discovery future without requiring a full `App` / transport stack. async fn resolve_models_catalog( cache: &tokio::sync::OnceCell>, - provider: crate::config::Provider, - model: &str, discover: impl std::future::Future, AgentError>>, -) -> Vec { - match cache.get_or_try_init(|| discover).await { - Ok(cached) => cached.clone(), - Err(e) => { - tracing::warn!( - "model catalog discovery failed: {e}; using fallback (will retry next session)" - ); - crate::catalog::discovery_failure_fallback(provider, model) - } - } +) -> Result, AgentError> { + cache.get_or_try_init(|| discover).await.cloned() +} + +/// Return the configured model as a one-entry catalog for this response. +/// +/// This value is never written to `models_cache`; failed discovery must be retried by +/// the next session rather than pinning degraded state for the process lifetime. +fn configured_model_fallback(model: &str) -> Vec { + let model = model.trim().to_string(); + vec![ModelEntry { + id: model.clone(), + name: model, + }] } async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSender) { @@ -400,6 +396,50 @@ async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSen } Arc::from(prompt) }; + // Resolve the model catalog before spawning MCP servers or registering a + // session. A configured static credential cannot recover interactively, so + // its authentication failure rejects before allocation. OAuth authentication + // failures and other catalog failures use only the configured model for this + // response, without caching, so session/prompt can run the existing PKCE flow. + let available_models: Vec = { + use crate::config::Provider; + match app.cfg.provider { + Provider::Databricks | Provider::DatabricksV2 => { + let models = match resolve_models_catalog( + &app.models_cache, + discover_databricks_models(&app.cfg), + ) + .await + { + Ok(models) => models, + Err(error @ AgentError::LlmAuth(_)) if !app.cfg.api_key.is_empty() => { + return reject(wire_tx, id, error.json_rpc_code(), &error.to_string()) + .await; + } + Err(error @ AgentError::LlmAuth(_)) => { + tracing::warn!( + error = %error, + "Databricks OAuth model catalog unavailable; using configured model" + ); + configured_model_fallback(&app.cfg.model) + } + Err(error) => { + tracing::warn!( + error = %error, + "Databricks model catalog unavailable; using configured model" + ); + configured_model_fallback(&app.cfg.model) + } + }; + models + .iter() + .map(|m| json!({ "modelId": m.id, "name": m.name })) + .collect() + } + _ => vec![json!({ "modelId": app.cfg.model, "name": app.cfg.model })], + } + }; + let mcp = match McpRegistry::spawn_all(&app.cfg, &p.mcp_servers, &p.cwd).await { Ok(m) => Arc::new(m), Err(e) => return reject(wire_tx, id, e.json_rpc_code(), &e.to_string()).await, @@ -445,36 +485,6 @@ async fn session_new(app: &Arc, id: Value, params: Value, wire_tx: &WireSen ); drop(sessions); - // Build a models catalog for the `session/new` response. For Databricks - // providers this advertises available models so the desktop ModelPicker and - // pool can resolve `session/set_model` switches. For Anthropic/OpenAI we - // report only the configured model — live switching on those providers - // effectively requires respawn. - // - // `models_cache` caches only a successful discovery result (`get_or_try_init` - // leaves the cell empty on error so the next `session/new` call retries). On - // discovery failure the fallback is used for the immediate response without - // being written to the cell. - let available_models: Vec = { - use crate::config::Provider; - match app.cfg.provider { - Provider::Databricks | Provider::DatabricksV2 => { - let models = resolve_models_catalog( - &app.models_cache, - app.cfg.provider, - &app.cfg.model, - discover_databricks_models(&app.cfg), - ) - .await; - models - .iter() - .map(|m| json!({ "modelId": m.id, "name": m.name })) - .collect() - } - _ => vec![json!({ "modelId": app.cfg.model, "name": app.cfg.model })], - } - }; - wire::send( wire_tx, wire::ok( @@ -870,8 +880,7 @@ fn session_token() -> Result { #[cfg(test)] mod tests { - use crate::catalog::{discovery_failure_fallback, ModelEntry, DATABRICKS_V2_KNOWN_MODELS}; - use crate::config::Provider; + use crate::catalog::ModelEntry; use crate::types::AgentError; /// Regression: a discovery error must not pin the models_cache for the process lifetime. @@ -884,23 +893,14 @@ mod tests { #[tokio::test] async fn models_cache_does_not_pin_on_discovery_error() { let cache: tokio::sync::OnceCell> = tokio::sync::OnceCell::new(); - let provider = Provider::DatabricksV2; - let model = "my-configured-model"; - // First call — discovery fails. Cell must remain empty; fallback returned. - let first = crate::resolve_models_catalog(&cache, provider, model, async { - Err::, AgentError>(AgentError::LlmAuth("transient failure".into())) + // First call — discovery failure is surfaced and leaves the cell empty. + let error = crate::resolve_models_catalog(&cache, async { + Err::, AgentError>(AgentError::Llm("transient failure".into())) }) - .await; - assert!( - cache.get().is_none(), - "cell must be empty after a discovery error — next session must retry" - ); - let expected_fallback = discovery_failure_fallback(provider, model); - assert_eq!( - first, expected_fallback, - "error path must return the provider-aware fallback" - ); + .await + .unwrap_err(); + assert!(matches!(error, AgentError::Llm(_))); // Second call — discovery succeeds. Cell is now populated and returned. let discovered = vec![ModelEntry { @@ -908,10 +908,11 @@ mod tests { name: "databricks-meta-llama-3-1-70b-instruct".into(), }]; let discovered_clone = discovered.clone(); - let second = crate::resolve_models_catalog(&cache, provider, model, async move { + let second = crate::resolve_models_catalog(&cache, async move { Ok::, AgentError>(discovered_clone) }) - .await; + .await + .unwrap(); assert_eq!( second, discovered, "second call must return the discovered catalog" @@ -927,78 +928,40 @@ mod tests { ); } - /// Regression: legacy `Provider::Databricks` must not advertise v2 AI Gateway model IDs - /// on discovery failure (Wes W1). This test calls `discovery_failure_fallback` directly — - /// the same helper used by `session_new` — and verifies the split behavior. It FAILS if - /// the arm is un-split (i.e., if both providers return the v2 catalog on failure). - #[test] - fn databricks_discovery_failure_fallback_legacy_returns_configured_model_only() { - let configured = "my-serving-endpoint"; - let result = discovery_failure_fallback(Provider::Databricks, configured); - - // Legacy Databricks must advertise exactly the configured model — nothing more. - assert_eq!( - result.len(), - 1, - "legacy Databricks fallback must contain exactly one entry, got: {result:?}" - ); - assert_eq!( - result[0].id, configured, - "legacy Databricks fallback must be the configured model" - ); + #[tokio::test] + async fn models_catalog_does_not_cache_oauth_auth_fallback() { + let cache: tokio::sync::OnceCell> = tokio::sync::OnceCell::new(); + let error = crate::resolve_models_catalog(&cache, async { + Err::, AgentError>(AgentError::LlmAuth("sign in again".into())) + }) + .await + .unwrap_err(); - // Crucially: must NOT contain any DATABRICKS_V2_KNOWN_MODELS entry. - let v2_ids: Vec<&str> = DATABRICKS_V2_KNOWN_MODELS.to_vec(); - for id in &result { - assert!( - !v2_ids.contains(&id.id.as_str()), - "legacy Databricks fallback must not include v2 ID '{}' — that endpoint \ - may not be served by /serving-endpoints/{{model}}/invocations", - id.id - ); - } - } + assert!(matches!(error, AgentError::LlmAuth(_))); + assert!(cache.get().is_none()); - #[test] - fn databricks_discovery_failure_fallback_v2_returns_known_models_catalog() { - let configured = "my-configured-model"; - let result = discovery_failure_fallback(Provider::DatabricksV2, configured); + let discovered = vec![ModelEntry { + id: "authenticated-model".into(), + name: "authenticated-model".into(), + }]; + let result = crate::resolve_models_catalog(&cache, async { + Ok::, AgentError>(discovered.clone()) + }) + .await + .unwrap(); - // DatabricksV2 must return the full DATABRICKS_V2_KNOWN_MODELS list, - // plus the configured model so the picker can still represent the model - // the agent is actually running. - assert_eq!( - result.len(), - DATABRICKS_V2_KNOWN_MODELS.len() + 1, - "DatabricksV2 fallback must return all known models plus the configured model" - ); - let result_ids: Vec<&str> = result.iter().map(|m| m.id.as_str()).collect(); - for known_id in DATABRICKS_V2_KNOWN_MODELS { - assert!( - result_ids.contains(known_id), - "DatabricksV2 fallback must include known model '{known_id}'" - ); - } - assert!( - result_ids.contains(&configured), - "DatabricksV2 fallback must include the configured model" - ); + assert_eq!(result, discovered); + assert_eq!(cache.get(), Some(&discovered)); } #[test] - fn databricks_discovery_failure_fallback_split_verified() { - // This test FAILS if the v1/v2 arms are merged back into one — it directly verifies - // that the two providers' error-path behavior diverges (Wes W1 protection). - let v1 = discovery_failure_fallback(Provider::Databricks, "my-endpoint"); - let v2 = discovery_failure_fallback(Provider::DatabricksV2, "my-endpoint"); - - let v1_ids: Vec<&str> = v1.iter().map(|m| m.id.as_str()).collect(); - let v2_ids: Vec<&str> = v2.iter().map(|m| m.id.as_str()).collect(); - - assert_ne!( - v1_ids, v2_ids, - "Provider::Databricks and Provider::DatabricksV2 must return different \ - fallback catalogs — if they are equal, the W1 arm split has been reverted" + fn configured_model_fallback_is_trimmed_and_singular() { + assert_eq!( + crate::configured_model_fallback(" configured-model "), + vec![ModelEntry { + id: "configured-model".into(), + name: "configured-model".into(), + }] ); } } diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 73c7e1faf2e..220d99f9a42 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -1910,6 +1910,22 @@ where unreachable!("loop always returns on its final iteration (attempt + 1 == MAX_RETRIES)"); } +pub(crate) fn databricks_pkce_config(host: &str) -> PkceOAuthConfig { + PkceOAuthConfig { + discovery_url: format!( + "{}/oidc/.well-known/oauth-authorization-server", + host.trim_end_matches('/') + ), + client_id: DATABRICKS_CLIENT_ID.into(), + scopes: DATABRICKS_OAUTH_SCOPES + .iter() + .map(|scope| (*scope).into()) + .collect(), + cache_namespace: "databricks".into(), + cache_dir_override: None, + } +} + /// Build the `TokenSource` for the configured provider. /// /// - `Provider::Anthropic`: a static source seeded from `cfg.api_key`. It's @@ -1929,21 +1945,9 @@ pub(crate) fn build_token_source(cfg: &Config) -> Result, A if !cfg.api_key.is_empty() { return Ok(Arc::new(StaticTokenSource::new(cfg.api_key.clone()))); } - let discovery_url = format!( - "{}/oidc/.well-known/oauth-authorization-server", - cfg.base_url.trim_end_matches('/') - ); - let pkce = PkceOAuthConfig { - discovery_url, - client_id: DATABRICKS_CLIENT_ID.into(), - scopes: DATABRICKS_OAUTH_SCOPES - .iter() - .map(|s| (*s).into()) - .collect(), - cache_namespace: "databricks".into(), - cache_dir_override: None, - }; - Ok(PkceOAuthTokenSource::new(pkce)?) + Ok(PkceOAuthTokenSource::new(databricks_pkce_config( + &cfg.base_url, + ))?) } } } diff --git a/crates/buzz-agent/tests/databricks_oauth.rs b/crates/buzz-agent/tests/databricks_oauth.rs index 52acee1076e..fbe0dc1f862 100644 --- a/crates/buzz-agent/tests/databricks_oauth.rs +++ b/crates/buzz-agent/tests/databricks_oauth.rs @@ -20,6 +20,7 @@ use axum::{routing::get, routing::post, Json, Router}; use buzz_agent::auth::{PkceOAuthConfig, PkceOAuthTokenSource, TokenSource}; use serde::Deserialize; use serde_json::json; +use sha2::{Digest, Sha256}; use tempfile::TempDir; #[derive(Deserialize)] @@ -457,6 +458,7 @@ struct AgentHarness { stdin: tokio::process::ChildStdin, stdout: BufReader, next_id: i64, + _home: Option, } impl Drop for AgentHarness { @@ -467,20 +469,65 @@ impl Drop for AgentHarness { impl AgentHarness { async fn spawn_provider(provider: &str, base_url: &str, model: &str) -> Self { + Self::spawn_provider_with_options(provider, base_url, model, 1, Some("test-bearer")).await + } + + async fn spawn_oauth_provider( + provider: &str, + base_url: &str, + model: &str, + max_sessions: usize, + ) -> Self { + Self::spawn_provider_with_options(provider, base_url, model, max_sessions, None).await + } + + async fn spawn_provider_with_max_sessions( + provider: &str, + base_url: &str, + model: &str, + max_sessions: usize, + ) -> Self { + Self::spawn_provider_with_options( + provider, + base_url, + model, + max_sessions, + Some("test-bearer"), + ) + .await + } + + async fn spawn_provider_with_options( + provider: &str, + base_url: &str, + model: &str, + max_sessions: usize, + token: Option<&str>, + ) -> Self { let bin = env!("CARGO_BIN_EXE_buzz-agent"); + let home = token + .is_none() + .then(|| TempDir::new().expect("create isolated OAuth home")); let mut cmd = tokio::process::Command::new(bin); cmd.env("BUZZ_AGENT_PROVIDER", provider) .env("DATABRICKS_HOST", base_url) .env("DATABRICKS_MODEL", model) - .env("DATABRICKS_TOKEN", "test-bearer") + .env_remove("DATABRICKS_TOKEN") .env("BUZZ_AGENT_LLM_TIMEOUT_SECS", "5") .env("BUZZ_AGENT_TOOL_TIMEOUT_SECS", "5") .env("BUZZ_AGENT_MAX_ROUNDS", "2") + .env("BUZZ_AGENT_MAX_SESSIONS", max_sessions.to_string()) .env("BUZZ_AGENT_MCP_INIT_TIMEOUT_SECS", "2") .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::null()) .kill_on_drop(true); + if let Some(token) = token { + cmd.env("DATABRICKS_TOKEN", token); + } + if let Some(home) = &home { + cmd.env("HOME", home.path()); + } let mut child = cmd.spawn().expect("spawn buzz-agent"); let stdin = child.stdin.take().unwrap(); let stdout = BufReader::new(child.stdout.take().unwrap()); @@ -489,9 +536,17 @@ impl AgentHarness { stdin, stdout, next_id: 1, + _home: home, } } + fn oauth_home(&self) -> &std::path::Path { + self._home + .as_ref() + .expect("harness was not started in OAuth mode") + .path() + } + async fn send(&mut self, method: &str, params: serde_json::Value) -> i64 { let id = self.next_id; self.next_id += 1; @@ -938,3 +993,288 @@ async fn session_set_model_empty_model_id_returns_error() { "error message must mention modelId, got: {msg}" ); } + +#[tokio::test] +async fn model_discovery_surfaces_rejected_static_token_as_auth_failure() { + use axum::http::StatusCode; + use buzz_agent::config::{Config, Provider}; + use buzz_agent::discover_databricks_models; + + let requests = Arc::new(AtomicU64::new(0)); + let requests_for_route = requests.clone(); + let listener = tokio::net::TcpListener::bind(SocketAddr::from(([127, 0, 0, 1], 0))) + .await + .unwrap(); + let host = format!("http://{}", listener.local_addr().unwrap()); + let app = Router::new().route( + "/api/ai-gateway/v2/endpoints", + get(move || { + let requests = requests_for_route.clone(); + async move { + requests.fetch_add(1, Ordering::SeqCst); + (StatusCode::UNAUTHORIZED, "rejected bearer rejected") + } + }), + ); + tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let cfg = Config::for_discovery(Provider::DatabricksV2, "rejected".into(), host); + let error = discover_databricks_models(&cfg).await.unwrap_err(); + + assert!( + error.to_string().starts_with("llm auth:"), + "401 must retain auth semantics: {error}" + ); + assert!( + !error.to_string().contains("rejected bearer"), + "auth errors must not propagate provider bodies that may echo credentials: {error}" + ); + assert_eq!( + requests.load(Ordering::SeqCst), + 1, + "a static token cannot refresh, so discovery must not issue a duplicate request" + ); +} + +fn databricks_oauth_cache_path(home: &std::path::Path, host: &str) -> std::path::PathBuf { + let discovery_url = format!( + "{}/oidc/.well-known/oauth-authorization-server", + host.trim_end_matches('/') + ); + let mut hasher = Sha256::new(); + hasher.update(discovery_url.as_bytes()); + hasher.update(b"|"); + hasher.update(b"databricks-cli"); + hasher.update(b"|"); + hasher.update(b"all-apis,offline_access"); + let hash = hex::encode(hasher.finalize()); + home.join(".config") + .join("buzz-agent") + .join("oauth") + .join("databricks") + .join(format!("{hash}.json")) +} + +fn write_cached_oauth_token(home: &std::path::Path, host: &str, access_token: &str) { + let path = databricks_oauth_cache_path(home, host); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write( + path, + serde_json::to_vec(&json!({ + "access_token": access_token, + "refresh_token": null, + "expires_at": SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs() + + 3600, + })) + .unwrap(), + ) + .unwrap(); +} + +#[tokio::test] +async fn oauth_missing_token_uses_configured_model_then_retries_discovery() { + let attempts = Arc::new(AtomicU64::new(0)); + let attempts_for_route = attempts.clone(); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let host = format!("http://{}", listener.local_addr().unwrap()); + let app = Router::new().route( + "/api/ai-gateway/v2/endpoints", + get(move || { + let attempts = attempts_for_route.clone(); + async move { + attempts.fetch_add(1, Ordering::SeqCst); + Json(json!({ + "endpoints": [{"name": "authenticated-model"}], + "next_page_token": null, + })) + } + }), + ); + tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let configured_model = " configured-model "; + let mut h = + AgentHarness::spawn_oauth_provider("databricks_v2", &host, configured_model, 2).await; + let initialize = h + .send( + "initialize", + json!({ "protocolVersion": 1, "clientCapabilities": {} }), + ) + .await; + assert!(h.recv_for(initialize).await.get("result").is_some()); + + let first = h + .send("session/new", json!({ "cwd": "/tmp", "mcpServers": [] })) + .await; + let first_response = h.recv_for(first).await; + assert!( + first_response["result"]["sessionId"].is_string(), + "missing OAuth token blocked session creation: {first_response}" + ); + assert_eq!( + first_response["result"]["models"]["availableModels"], + json!([{"modelId": "configured-model", "name": "configured-model"}]) + ); + assert_eq!(attempts.load(Ordering::SeqCst), 0); + + write_cached_oauth_token(h.oauth_home(), &host, "cached-bearer"); + + let second = h + .send("session/new", json!({ "cwd": "/tmp", "mcpServers": [] })) + .await; + let second_response = h.recv_for(second).await; + assert!( + second_response["result"]["sessionId"].is_string(), + "later authenticated session failed: {second_response}" + ); + assert_eq!( + second_response["result"]["models"]["availableModels"], + json!([{"modelId": "authenticated-model", "name": "authenticated-model"}]) + ); + assert_eq!( + attempts.load(Ordering::SeqCst), + 1, + "OAuth fallback was cached instead of retrying discovery" + ); +} + +#[tokio::test] +async fn non_auth_discovery_failure_uses_configured_model_without_caching_fallback() { + use axum::http::StatusCode; + + let attempts = Arc::new(AtomicU64::new(0)); + let attempts_for_route = attempts.clone(); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let host = format!("http://{}", listener.local_addr().unwrap()); + let app = Router::new().route( + "/api/ai-gateway/v2/endpoints", + get(move || { + let attempts = attempts_for_route.clone(); + async move { + attempts.fetch_add(1, Ordering::SeqCst); + (StatusCode::SERVICE_UNAVAILABLE, "catalog unavailable") + } + }), + ); + tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let configured_model = " configured-model "; + let normalized_configured_model = configured_model.trim(); + let mut h = + AgentHarness::spawn_provider_with_max_sessions("databricks_v2", &host, configured_model, 2) + .await; + let initialize = h + .send( + "initialize", + json!({ "protocolVersion": 1, "clientCapabilities": {} }), + ) + .await; + assert!(h.recv_for(initialize).await.get("result").is_some()); + + for expected_attempts in 1..=2 { + let request = h + .send("session/new", json!({ "cwd": "/tmp", "mcpServers": [] })) + .await; + let response = h.recv_for(request).await; + assert!( + response["result"]["sessionId"].is_string(), + "non-auth catalog failure blocked session creation: {response}" + ); + assert_eq!( + response["result"]["models"]["availableModels"], + json!([{"modelId": normalized_configured_model, "name": normalized_configured_model}]) + ); + assert_eq!(attempts.load(Ordering::SeqCst), expected_attempts); + } +} + +#[tokio::test] +async fn rejected_static_token_does_not_consume_capacity_or_spawn_mcp() { + use axum::http::StatusCode; + + let attempts = Arc::new(AtomicU64::new(0)); + let attempts_for_route = attempts.clone(); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let host = format!("http://{}", listener.local_addr().unwrap()); + let app = Router::new().route( + "/api/ai-gateway/v2/endpoints", + get(move || { + let attempts = attempts_for_route.clone(); + async move { + if attempts.fetch_add(1, Ordering::SeqCst) == 0 { + Err((StatusCode::UNAUTHORIZED, "rejected")) + } else { + Ok(Json(json!({ + "endpoints": [{"name": "discovered-model"}], + "next_page_token": null, + }))) + } + } + }), + ); + tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let mut h = AgentHarness::spawn_provider("databricks_v2", &host, "discovered-model").await; + let initialize = h + .send( + "initialize", + json!({ "protocolVersion": 1, "clientCapabilities": {} }), + ) + .await; + assert!(h.recv_for(initialize).await.get("result").is_some()); + + let pid_dir = TempDir::new().unwrap(); + let pid_file = pid_dir.path().join("mcp.pid"); + let fake_mcp = env!("CARGO_BIN_EXE_fake-mcp"); + let mcp_servers = json!([{ + "name": "must-not-spawn", + "command": fake_mcp, + "args": [], + "env": [{ + "name": "FAKE_MCP_PID_FILE", + "value": pid_file.to_string_lossy(), + }], + }]); + + let failed = h + .send( + "session/new", + json!({ "cwd": "/tmp", "mcpServers": mcp_servers }), + ) + .await; + let failed_response = h.recv_for(failed).await; + assert!(failed_response.get("error").is_some(), "{failed_response}"); + assert!( + failed_response["error"]["message"] + .as_str() + .unwrap_or_default() + .contains("llm auth"), + "rejected static token did not retain auth semantics: {failed_response}" + ); + tokio::time::sleep(Duration::from_millis(100)).await; + assert!( + !pid_file.exists(), + "MCP process spawned before failed discovery was resolved" + ); + + let retry = h + .send("session/new", json!({ "cwd": "/tmp", "mcpServers": [] })) + .await; + let retry_response = h.recv_for(retry).await; + assert!( + retry_response["result"]["sessionId"].is_string(), + "failed discovery consumed the sole session slot: {retry_response}" + ); + assert_eq!(attempts.load(Ordering::SeqCst), 2); +} diff --git a/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json b/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json index 28fe6ce90e1..beffc294408 100644 --- a/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json +++ b/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json @@ -22,6 +22,7 @@ "owner_pubkey": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "policy_env": { "BUZZ_ACP_AGENTS": "10", + "BUZZ_ACP_DISPLAY_NAME": "worker", "BUZZ_ACP_LAZY_POOL": "true", "BUZZ_ACP_MODEL": "gpt-5", "BUZZ_ACP_RELAY_OBSERVER": "true", diff --git a/crates/buzz-cli/src/client.rs b/crates/buzz-cli/src/client.rs index d0dd2677a9e..ee8868ad927 100644 --- a/crates/buzz-cli/src/client.rs +++ b/crates/buzz-cli/src/client.rs @@ -1387,19 +1387,25 @@ pub fn extract_p_tags(event: &serde_json::Value) -> Vec { .unwrap_or_default() } -/// Return a create-command response with an entity ID injected. -pub fn create_response_with_id(resp: &str, id_key: &str, id_val: &str) -> String { +/// Return a create-command response, injecting the entity ID **only** when the +/// relay accepted the event (`"accepted": true`). When the relay rejected the +/// event, emitting the locally-computed link would be misleading — callers +/// that copy or share the link would reference an event that was never stored. +pub fn create_response_with_id_if_accepted(resp: &str, id_key: &str, id_val: &str) -> String { let mut v: serde_json::Value = serde_json::from_str(resp).unwrap_or(serde_json::json!({})); - v[id_key] = serde_json::json!(id_val); - if v.get("accepted").is_none() { - v["accepted"] = serde_json::json!(true); + let accepted = v.get("accepted").and_then(|a| a.as_bool()).unwrap_or(false); + if accepted { + v[id_key] = serde_json::json!(id_val); } v.to_string() } /// Print a create-command response, injecting the generated entity ID. pub fn print_create_response(resp: &str, id_key: &str, id_val: &str) { - println!("{}", create_response_with_id(resp, id_key, id_val)); + println!( + "{}", + create_response_with_id_if_accepted(resp, id_key, id_val) + ); } /// Extract a JSON field from relay write response messages shaped as @@ -2297,7 +2303,8 @@ mod retry_policy_tests { #[cfg(test)] mod tests { use super::{ - advance_query_cursor, create_response_with_id, extract_relay_response_field, BuzzClient, + advance_query_cursor, create_response_with_id_if_accepted, extract_relay_response_field, + BuzzClient, }; use nostr::{EventBuilder, Keys, Kind, Tag}; @@ -2345,15 +2352,30 @@ mod tests { } #[test] - fn create_response_with_id_overrides_local_id_with_relay_id() { + fn create_response_with_id_if_accepted_injects_id_when_accepted() { let raw = r#"{"event_id":"abc","accepted":true,"message":"response:{\"workflow_id\":\"relay-id\"}"}"#; - let out = create_response_with_id(raw, "workflow_id", "relay-id"); + let out = create_response_with_id_if_accepted(raw, "workflow_id", "relay-id"); let v: serde_json::Value = serde_json::from_str(&out).unwrap(); + // ID injected and original fields preserved when accepted. assert_eq!(v["workflow_id"].as_str(), Some("relay-id")); assert_eq!(v["event_id"].as_str(), Some("abc")); assert_eq!(v["accepted"].as_bool(), Some(true)); } + #[test] + fn create_response_with_id_if_accepted_omits_id_when_rejected() { + let raw = r#"{"event_id":"abc","accepted":false,"message":"duplicate"}"#; + let out = create_response_with_id_if_accepted(raw, "workflow_id", "local-id"); + let v: serde_json::Value = serde_json::from_str(&out).unwrap(); + // ID must not be present when relay rejected the event; emitting a + // link to an event that was never stored would mislead callers. + assert!( + v.get("workflow_id").is_none(), + "link field must be absent on rejected create" + ); + assert_eq!(v["accepted"].as_bool(), Some(false)); + } + // --- (a) auth-suppression regression pair --- fn make_auth_tag() -> (Tag, String) { diff --git a/crates/buzz-cli/src/commands/issues.rs b/crates/buzz-cli/src/commands/issues.rs index 3d7d92a1b4a..91c64a3915c 100644 --- a/crates/buzz-cli/src/commands/issues.rs +++ b/crates/buzz-cli/src/commands/issues.rs @@ -1,4 +1,5 @@ use crate::client::BuzzClient; +use crate::commands::with_git_provenance; use crate::error::CliError; use crate::validate::{read_or_stdin, sdk_err, validate_hex64, validate_repo_id}; use buzz_sdk::{GitIssueMeta, GitRepoCoord, GitStatusMeta}; @@ -26,10 +27,16 @@ pub async fn cmd_create_issue( id: repo_id.to_string(), }; - let builder = buzz_sdk::build_git_issue(&repo, subject, &body, &meta).map_err(sdk_err)?; + let builder = with_git_provenance( + buzz_sdk::build_git_issue(&repo, subject, &body, &meta).map_err(sdk_err)?, + )?; let event = client.sign_event(builder)?; + let event_id = event.id.to_hex(); let resp = client.submit_event(event).await?; - println!("{resp}"); + // `link` renders as a rich preview card in Buzz Desktop when included in + // a chat message — agents announce issues with it (see base_prompt.md). + let link = crate::links::issue_link(&event_id, repo_owner, repo_id); + crate::client::print_create_response(&resp, "link", &link); Ok(()) } @@ -137,7 +144,8 @@ pub async fn cmd_issue_status( applied_as_commits: vec![], }; - let builder = buzz_sdk::build_git_status(status, &body, &meta).map_err(sdk_err)?; + let builder = + with_git_provenance(buzz_sdk::build_git_status(status, &body, &meta).map_err(sdk_err)?)?; let event = client.sign_event(builder)?; let resp = client.submit_event(event).await?; println!("{resp}"); diff --git a/crates/buzz-cli/src/commands/mod.rs b/crates/buzz-cli/src/commands/mod.rs index 1ccc37a7027..ad2c36e200c 100644 --- a/crates/buzz-cli/src/commands/mod.rs +++ b/crates/buzz-cli/src/commands/mod.rs @@ -21,6 +21,55 @@ pub mod users; pub mod workflows; use crate::{client::normalize_write_response, error::CliError}; +use nostr::{EventBuilder, Tag}; + +const GIT_ORIGIN_CHANNEL_ENV: &str = "BUZZ_GIT_ORIGIN_CHANNEL_ID"; +const GIT_ORIGIN_AGENT_ENV: &str = "BUZZ_GIT_ORIGIN_AGENT_NAME"; + +/// Add trusted, session-scoped provenance supplied by the ACP harness. +/// +/// Public channels use the standard NIP-29 `h` tag. Private conversations +/// intentionally omit their channel coordinate and retain only the agent's +/// display name. +pub(crate) fn with_git_provenance(builder: EventBuilder) -> Result { + apply_git_provenance( + builder, + std::env::var(GIT_ORIGIN_CHANNEL_ENV).ok().as_deref(), + std::env::var(GIT_ORIGIN_AGENT_ENV).ok().as_deref(), + ) +} + +fn apply_git_provenance( + builder: EventBuilder, + channel_id: Option<&str>, + agent_name: Option<&str>, +) -> Result { + if let Some(channel_id) = channel_id { + let channel_id = channel_id.trim(); + uuid::Uuid::parse_str(channel_id) + .map_err(|_| CliError::Other("invalid git origin channel ID".into()))?; + let origin_tag = Tag::parse(["h", channel_id]) + .map_err(|error| CliError::Other(format!("invalid git origin tag: {error}")))?; + return Ok(builder.tag(origin_tag)); + } + + if let Some(agent_name) = agent_name { + let agent_name = agent_name.trim(); + if agent_name.is_empty() + || agent_name.len() > 256 + || agent_name.chars().any(char::is_control) + { + return Err(CliError::Other( + "invalid private-conversation agent name".into(), + )); + } + let origin_tag = Tag::parse(["buzz-origin-agent", agent_name]) + .map_err(|error| CliError::Other(format!("invalid git origin tag: {error}")))?; + return Ok(builder.tag(origin_tag)); + } + + Ok(builder) +} /// Parse a relay write-response JSON blob, mapping a duplicate (dominated) /// write to [`CliError::Conflict`] with the caller-supplied message. @@ -46,3 +95,47 @@ pub fn parse_write_response(raw: &str, conflict_msg: &str) -> Result, agent_name: Option<&str>) -> nostr::Event { + apply_git_provenance( + EventBuilder::new(Kind::Custom(1621), "issue"), + channel_id, + agent_name, + ) + .expect("apply provenance") + .sign_with_keys(&Keys::generate()) + .expect("sign event") + } + + #[test] + fn public_channel_origin_uses_h_tag_and_suppresses_agent_name() { + let channel_id = "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50"; + let event = event_with_origin(Some(channel_id), Some("Builder")); + assert!(event + .tags + .iter() + .any(|tag| tag.as_slice() == ["h", channel_id])); + assert!(!event + .tags + .iter() + .any(|tag| tag.as_slice().first().map(String::as_str) == Some("buzz-origin-agent"))); + } + + #[test] + fn private_origin_exposes_only_agent_name() { + let event = event_with_origin(None, Some("Builder")); + assert!(event + .tags + .iter() + .any(|tag| tag.as_slice() == ["buzz-origin-agent", "Builder"])); + assert!(!event + .tags + .iter() + .any(|tag| tag.as_slice().first().map(String::as_str) == Some("h"))); + } +} diff --git a/crates/buzz-cli/src/commands/patches.rs b/crates/buzz-cli/src/commands/patches.rs index 13f1714d061..413934a3c11 100644 --- a/crates/buzz-cli/src/commands/patches.rs +++ b/crates/buzz-cli/src/commands/patches.rs @@ -1,4 +1,5 @@ use crate::client::BuzzClient; +use crate::commands::with_git_provenance; use crate::error::CliError; use crate::validate::{ read_file_or_stdin, read_or_stdin, sdk_err, validate_hex64, validate_repo_id, @@ -47,7 +48,8 @@ pub async fn cmd_send_patch( id: repo_id.to_string(), }; - let builder = buzz_sdk::build_git_patch(&repo, &content, &meta).map_err(sdk_err)?; + let builder = + with_git_provenance(buzz_sdk::build_git_patch(&repo, &content, &meta).map_err(sdk_err)?)?; let event = client.sign_event(builder)?; let resp = client.submit_event(event).await?; println!("{resp}"); @@ -180,7 +182,8 @@ pub async fn cmd_patch_status( applied_as_commits: applied_as_commit.to_vec(), }; - let builder = buzz_sdk::build_git_status(status, &body, &meta).map_err(sdk_err)?; + let builder = + with_git_provenance(buzz_sdk::build_git_status(status, &body, &meta).map_err(sdk_err)?)?; let event = client.sign_event(builder)?; let resp = client.submit_event(event).await?; println!("{resp}"); diff --git a/crates/buzz-cli/src/commands/pr.rs b/crates/buzz-cli/src/commands/pr.rs index 4272c2bfd81..74c580a6d6a 100644 --- a/crates/buzz-cli/src/commands/pr.rs +++ b/crates/buzz-cli/src/commands/pr.rs @@ -1,4 +1,5 @@ use crate::client::BuzzClient; +use crate::commands::with_git_provenance; use crate::error::CliError; use crate::validate::{ read_file_or_stdin, read_or_stdin, sdk_err, validate_hex64, validate_repo_id, @@ -55,10 +56,16 @@ pub async fn cmd_open_pr( revision_of: revision_of.map(str::to_string), }; - let builder = buzz_sdk::build_git_pull_request(&repo, &content, &meta).map_err(sdk_err)?; + let builder = with_git_provenance( + buzz_sdk::build_git_pull_request(&repo, &content, &meta).map_err(sdk_err)?, + )?; let event = client.sign_event(builder)?; + let event_id = event.id.to_hex(); let resp = client.submit_event(event).await?; - println!("{resp}"); + // `link` renders as a rich preview card in Buzz Desktop when included in + // a chat message — agents announce PRs with it (see base_prompt.md). + let link = crate::links::pull_request_link(&event_id, repo_owner, repo_id); + crate::client::print_create_response(&resp, "link", &link); Ok(()) } @@ -97,7 +104,9 @@ pub async fn cmd_update_pr( merge_base: merge_base.map(str::to_string), }; - let builder = buzz_sdk::build_git_pr_update(&repo, &content, &meta).map_err(sdk_err)?; + let builder = with_git_provenance( + buzz_sdk::build_git_pr_update(&repo, &content, &meta).map_err(sdk_err)?, + )?; let event = client.sign_event(builder)?; let resp = client.submit_event(event).await?; println!("{resp}"); @@ -206,7 +215,8 @@ pub async fn cmd_pr_status( applied_as_commits: vec![], }; - let builder = buzz_sdk::build_git_status(status, &content, &meta).map_err(sdk_err)?; + let builder = + with_git_provenance(buzz_sdk::build_git_status(status, &content, &meta).map_err(sdk_err)?)?; let event = client.sign_event(builder)?; let resp = client.submit_event(event).await?; println!("{resp}"); diff --git a/crates/buzz-cli/src/commands/repos.rs b/crates/buzz-cli/src/commands/repos.rs index 15e064d9c35..e54b95ef20e 100644 --- a/crates/buzz-cli/src/commands/repos.rs +++ b/crates/buzz-cli/src/commands/repos.rs @@ -261,8 +261,12 @@ pub async fn cmd_create_repo( channel, )?; let event = client.sign_event(builder)?; + let owner = event.pubkey.to_hex(); let resp = client.submit_event(event).await?; - println!("{resp}"); + // `link` renders as a rich preview card in Buzz Desktop when included in + // a chat message — agents announce repos with it (see base_prompt.md). + let link = crate::links::repo_link(&owner, repo_id); + crate::client::print_create_response(&resp, "link", &link); Ok(()) } diff --git a/crates/buzz-cli/src/lib.rs b/crates/buzz-cli/src/lib.rs index f745e7b2801..8a8bb053b0f 100644 --- a/crates/buzz-cli/src/lib.rs +++ b/crates/buzz-cli/src/lib.rs @@ -2,6 +2,7 @@ pub mod agent_management; mod client; mod commands; mod error; +mod links; mod validate; use clap::{Parser, Subcommand}; diff --git a/crates/buzz-cli/src/links.rs b/crates/buzz-cli/src/links.rs new file mode 100644 index 00000000000..043bdc48b05 --- /dev/null +++ b/crates/buzz-cli/src/links.rs @@ -0,0 +1,51 @@ +//! Canonical `buzz://` deep links for Buzz-hosted git entities. +//! +//! Buzz Desktop renders these links as rich preview cards in chat and +//! navigates in-app when they are clicked. The desktop parser lives in +//! `desktop/src/shared/lib/entityLink.ts` — the two implementations must +//! stay format-compatible (see `golden_format_matches_desktop` below and +//! the mirror test in `entityLink.test.mjs`). +//! +//! Callers are expected to validate inputs first (`validate_hex64`, +//! `validate_repo_id`); the identifier charsets need no URL encoding. + +/// Build a `buzz://repo` link for a repository announcement (kind 30617). +pub fn repo_link(owner: &str, repo_id: &str) -> String { + format!("buzz://repo?owner={owner}&d={repo_id}") +} + +/// Build a `buzz://pr` link for a pull request event (kind 1618). +pub fn pull_request_link(event_id: &str, owner: &str, repo_id: &str) -> String { + format!("buzz://pr?id={event_id}&owner={owner}&d={repo_id}") +} + +/// Build a `buzz://issue` link for an issue event (kind 1621). +pub fn issue_link(event_id: &str, owner: &str, repo_id: &str) -> String { + format!("buzz://issue?id={event_id}&owner={owner}&d={repo_id}") +} + +#[cfg(test)] +mod tests { + use super::*; + + const OWNER: &str = "71d67180ba17e749ee825fc8819c9c6ee7003617e1c126504f9b658070ab9224"; + const EVENT_ID: &str = "c3b589fa5713ba25bad6dc095e2de00a4ac8f50050fdea00fc6444e603be1dd1"; + + // Golden strings shared with desktop/src/shared/lib/entityLink.test.mjs + // ("builders emit the canonical cross-language link format"). + #[test] + fn golden_format_matches_desktop() { + assert_eq!( + pull_request_link(EVENT_ID, OWNER, "buzz-world"), + format!("buzz://pr?id={EVENT_ID}&owner={OWNER}&d=buzz-world") + ); + assert_eq!( + issue_link(EVENT_ID, OWNER, "buzz-world"), + format!("buzz://issue?id={EVENT_ID}&owner={OWNER}&d=buzz-world") + ); + assert_eq!( + repo_link(OWNER, "buzz-world"), + format!("buzz://repo?owner={OWNER}&d=buzz-world") + ); + } +} diff --git a/desktop/package.json b/desktop/package.json index 036533308cd..a1fd2e919d8 100644 --- a/desktop/package.json +++ b/desktop/package.json @@ -1,7 +1,7 @@ { "name": "buzz", "private": true, - "version": "0.5.4", + "version": "0.5.5", "type": "module", "scripts": { "dev": "vite", diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 9fa7fb4d6a9..752d1c4d92a 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -109,6 +109,7 @@ export default defineConfig({ "**/send-channel-binding.spec.ts", "**/project-commit-detail.spec.ts", "**/project-inbox.spec.ts", + "**/project-issue-comments.spec.ts", "**/project-pr-review.spec.ts", "**/persona-model-combobox-screenshots.spec.ts", "**/drafts-screenshots.spec.ts", diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index da80c5b07ad..afd119c84d9 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -1060,7 +1060,7 @@ dependencies = [ [[package]] name = "buzz-desktop" -version = "0.5.4" +version = "0.5.5" dependencies = [ "anyhow", "arboard", diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index 3b97ff1fe9d..1ba814da47e 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -7,7 +7,7 @@ members = ["crates/buzz-terminal"] [package] name = "buzz-desktop" -version = "0.5.4" +version = "0.5.5" description = "Buzz desktop app" authors = ["you"] edition = "2021" diff --git a/desktop/src-tauri/build.rs b/desktop/src-tauri/build.rs index 0fb3747718a..2b997af8914 100644 --- a/desktop/src-tauri/build.rs +++ b/desktop/src-tauri/build.rs @@ -13,8 +13,6 @@ fn main() { println!("cargo:rerun-if-env-changed=BUZZ_BUILD_BUZZ_AGENT_MODEL"); println!("cargo:rerun-if-env-changed=BUZZ_BUILD_AGENT_ENV"); println!("cargo:rerun-if-env-changed=BUZZ_BUILD_RELAY_RECONNECT_CMD"); - println!("cargo:rerun-if-env-changed=BUZZ_BUILD_OBSERVER_ARCHIVE_DEFAULT"); - println!("cargo:rerun-if-env-changed=BUZZ_BUILD_AGENT_METRIC_ARCHIVE_DEFAULT"); println!("cargo:rerun-if-env-changed=BUZZ_BUILD_AUTO_CONNECT_DEFAULT_RELAY"); println!("cargo:rustc-check-cfg=cfg(buzz_updater_enabled)"); @@ -75,21 +73,6 @@ fn main() { println!("cargo:rustc-env=BUZZ_DESKTOP_BUILD_RELAY_RECONNECT_CMD={val}"); } - // Presence-only flag: when set (any non-empty value), observer-feed archive - // defaults to ON for the current identity on first run. OSS builds leave - // this unset → default OFF. No JSON validation needed — the command only - // checks `.is_some()`. - if std::env::var("BUZZ_BUILD_OBSERVER_ARCHIVE_DEFAULT").is_ok() { - println!("cargo:rustc-env=BUZZ_DESKTOP_BUILD_OBSERVER_ARCHIVE_DEFAULT=1"); - } - - // Presence-only flag: when set (any non-empty value), agent-turn-metric - // archive defaults to ON for the current identity on first run. OSS builds - // leave this unset → default OFF. - if std::env::var("BUZZ_BUILD_AGENT_METRIC_ARCHIVE_DEFAULT").is_ok() { - println!("cargo:rustc-env=BUZZ_DESKTOP_BUILD_AGENT_METRIC_ARCHIVE_DEFAULT=1"); - } - // Presence-only release capability: internal desktop builds opt into // auto-connecting their configured default relay on first run. OSS builds // leave this unset and retain explicit community selection. diff --git a/desktop/src-tauri/src/commands/agent_metric_archive.rs b/desktop/src-tauri/src/commands/agent_metric_archive.rs index 43cfc7b082c..77de8708712 100644 --- a/desktop/src-tauri/src/commands/agent_metric_archive.rs +++ b/desktop/src-tauri/src/commands/agent_metric_archive.rs @@ -1,35 +1,18 @@ -//! Build-time flag for agent-turn-metric archive default. +//! Agent-turn-metric archive default — always enabled. //! -//! When `BUZZ_BUILD_AGENT_METRIC_ARCHIVE_DEFAULT` is set at build time -//! (internal builds), `agent_metric_archive_default_enabled()` returns `true` -//! and the frontend auto-seeds an `owner_p` save subscription for kind 44200 -//! (agent turn metrics) on first run for the current identity. -//! -//! OSS builds (env var unset) return `false` — no auto-seeding, user opts in -//! manually via the Local Archive settings card. +//! `agent_metric_archive_default_enabled()` returns `true` unconditionally. +//! The frontend calls this once at startup to decide whether to seed the +//! `owner_p` [44200] save subscription for the current identity on first run. +//! The `hasExplicitChoice` guard in the TS seed hook ensures a user who has +//! explicitly opted out remains opted out. -/// Returns `true` when an internal build has agent-turn-metric archive -/// default-on. +/// Returns `true`: agent-turn-metric archive defaults to enabled for all builds. /// -/// The frontend calls this once at startup to decide whether to seed the -/// `owner_p` [44200] save subscription. The result is stable for the lifetime -/// of the binary — it is baked at compile time. +/// The frontend uses this to decide whether to auto-seed an `owner_p` [44200] +/// save subscription on first run. Existing explicit choices (stored in +/// localStorage per identity) are preserved by the seed hook's `hasExplicitChoice` +/// guard — this default only applies to identities that have never made a choice. #[tauri::command] pub fn agent_metric_archive_default_enabled() -> bool { - option_env!("BUZZ_DESKTOP_BUILD_AGENT_METRIC_ARCHIVE_DEFAULT").is_some() -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_agent_metric_archive_default_enabled_returns_false_in_oss_build() { - // In a standard OSS/test build (no BUZZ_DESKTOP_BUILD_AGENT_METRIC_ARCHIVE_DEFAULT - // baked in), this must return false. - assert!( - !agent_metric_archive_default_enabled(), - "expected false in OSS/test build" - ); - } + true } diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 7ce03b140b1..4704582372d 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -137,6 +137,7 @@ pub async fn get_agent_models( &effective_provider, &merged_env, persisted_model.clone(), + DatabricksAuthIntent::InteractiveModelPicker, ) .await? { @@ -307,9 +308,14 @@ pub async fn discover_agent_models( return Ok(models); } - if let Some(models) = - discover_databricks_models(&state.http_client, &effective_provider, &merged_env, None) - .await? + if let Some(models) = discover_databricks_models( + &state.http_client, + &effective_provider, + &merged_env, + None, + DatabricksAuthIntent::PassiveDraftDiscovery, + ) + .await? { return Ok(models); } @@ -681,97 +687,14 @@ async fn discover_anthropic_models( })) } -// --------------------------------------------------------------------------- -// Databricks model discovery (v1 + v2) -// --------------------------------------------------------------------------- -// -// Delegates to buzz_agent_pkg::catalog::discover_databricks_models, which -// acquires auth in-process via build_token_source: -// - Static bearer (DATABRICKS_TOKEN): returned immediately. -// - PKCE cache hit: returned from disk without a browser flow. -// - No token, no cache: returns Err(LlmAuth) → we return Ok(None) and fall -// through to run_agent_models_command. Never hangs, never opens a browser. - -fn is_databricks_provider(provider: Option<&str>) -> bool { - matches!( - provider - .map(str::trim) - .map(str::to_ascii_lowercase) - .as_deref(), - Some("databricks" | "databricks_v2" | "databricks-v2") - ) -} - -fn databricks_agent_provider(provider: &str) -> buzz_agent_pkg::config::Provider { - if provider.trim().eq_ignore_ascii_case("databricks_v2") - || provider.trim().eq_ignore_ascii_case("databricks-v2") - { - buzz_agent_pkg::config::Provider::DatabricksV2 - } else { - buzz_agent_pkg::config::Provider::Databricks - } -} - -async fn discover_databricks_models( - _client: &reqwest::Client, - provider: &DiscoveryProvider, - env: &BTreeMap, - selected_model: Option, -) -> Result, String> { - let provider_str = match provider.as_deref() { - Some(p) if is_databricks_provider(Some(p)) => p, - _ => return Ok(None), - }; - - let host = match env_or_process_value(env, "DATABRICKS_HOST") { - Some(h) => h, - None => return Ok(None), // no host → fall through to subprocess - }; - - // api_key = DATABRICKS_TOKEN (empty string = use PKCE cache). - let api_key = env_or_process_value(env, "DATABRICKS_TOKEN").unwrap_or_default(); - - let agent_provider = databricks_agent_provider(provider_str); - let cfg = buzz_agent_pkg::config::Config::for_discovery(agent_provider, api_key, host); - - // Build a redaction env so the token never appears in surfaced errors. - let token_for_redact = env_or_process_value(env, "DATABRICKS_TOKEN").unwrap_or_default(); - let redaction_env = redaction_env_with_value(env, "DATABRICKS_TOKEN", &token_for_redact); - - let entries = match buzz_agent_pkg::discover_databricks_models(&cfg).await { - Ok(e) => e, - Err(buzz_agent_pkg::AgentError::LlmAuth(_)) => { - // No token + no PKCE cache → fall through to subprocess. - return Ok(None); - } - Err(e) => { - let msg = crate::managed_agents::redact_env_values_in(&e.to_string(), &redaction_env); - return Err(format!("Databricks model discovery failed: {msg}")); - } - }; - - if entries.is_empty() { - return Err("Databricks model discovery returned no models".to_string()); - } - - let models = entries - .into_iter() - .map(|e| AgentModelInfo { - id: e.id, - name: Some(e.name), - description: None, - }) - .collect(); - - Ok(Some(AgentModelsResponse { - agent_name: provider_str.trim().to_string(), - agent_version: "models-api".to_string(), - models, - agent_default_model: None, - selected_model, - supports_switching: true, - })) -} +#[path = "agent_models_databricks.rs"] +mod databricks; +#[cfg(test)] +use databricks::{ + databricks_sign_in_required_error, databricks_static_token_error, is_databricks_provider, + should_start_interactive_auth, +}; +use databricks::{discover_databricks_models, DatabricksAuthIntent}; /// Apply an `UpdateManagedAgentRequest`'s model/provider/system_prompt patch /// to `record`, enforcing the linked-instance write guard: a definition-linked diff --git a/desktop/src-tauri/src/commands/agent_models_databricks.rs b/desktop/src-tauri/src/commands/agent_models_databricks.rs new file mode 100644 index 00000000000..63b4564e61d --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_models_databricks.rs @@ -0,0 +1,174 @@ +//! Databricks v1/v2 model discovery and interactive reauthentication. + +use std::collections::BTreeMap; +use std::sync::LazyLock; + +use crate::commands::agent_models_env::{ + env_or_process_value, redaction_env_with_value, DiscoveryProvider, +}; +use crate::managed_agents::AgentModelInfo; +use crate::managed_agents::AgentModelsResponse; + +// Model discovery can be triggered by multiple dialogs at once. Permit only one +// callback listener/browser flow for the process-wide OAuth cache. +static AUTH_GATE: LazyLock> = LazyLock::new(|| tokio::sync::Mutex::new(())); + +pub(super) fn is_databricks_provider(provider: Option<&str>) -> bool { + matches!( + provider + .map(str::trim) + .map(str::to_ascii_lowercase) + .as_deref(), + Some("databricks" | "databricks_v2" | "databricks-v2") + ) +} + +fn databricks_agent_provider(provider: &str) -> buzz_agent_pkg::config::Provider { + if provider.trim().eq_ignore_ascii_case("databricks_v2") + || provider.trim().eq_ignore_ascii_case("databricks-v2") + { + buzz_agent_pkg::config::Provider::DatabricksV2 + } else { + buzz_agent_pkg::config::Provider::Databricks + } +} + +pub(super) fn databricks_static_token_error( + error: &str, + redaction_env: &BTreeMap, +) -> String { + let message = crate::managed_agents::redact_env_values_in(error, redaction_env); + format!("Databricks rejected DATABRICKS_TOKEN; update it in agent settings: {message}") +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum DatabricksAuthIntent { + /// A saved agent's model picker was opened by the user. + InteractiveModelPicker, + /// Discovery was triggered automatically from unsaved form state. + PassiveDraftDiscovery, +} + +impl DatabricksAuthIntent { + fn allows_interactive_auth(self) -> bool { + matches!(self, Self::InteractiveModelPicker) + } +} + +pub(super) fn databricks_sign_in_required_error() -> String { + "Databricks sign-in is required; save this agent, then open its model picker to sign in, or run `buzz-agent auth databricks`" + .to_string() +} + +pub(super) fn should_start_interactive_auth( + api_key: &str, + auth_intent: DatabricksAuthIntent, +) -> bool { + api_key.is_empty() && auth_intent.allows_interactive_auth() +} + +pub(super) async fn discover_databricks_models( + _client: &reqwest::Client, + provider: &DiscoveryProvider, + env: &BTreeMap, + selected_model: Option, + auth_intent: DatabricksAuthIntent, +) -> Result, String> { + let provider_name = match provider.as_deref() { + Some(provider_name) if is_databricks_provider(Some(provider_name)) => provider_name, + _ => return Ok(None), + }; + + let host = match env_or_process_value(env, "DATABRICKS_HOST") { + Some(host) => host, + None => return Ok(None), + }; + let api_key = env_or_process_value(env, "DATABRICKS_TOKEN").unwrap_or_default(); + let config = buzz_agent_pkg::config::Config::for_discovery( + databricks_agent_provider(provider_name), + api_key.clone(), + host.clone(), + ); + let redaction_env = redaction_env_with_value(env, "DATABRICKS_TOKEN", &api_key); + + let entries = match buzz_agent_pkg::discover_databricks_models(&config).await { + Ok(entries) => entries, + Err(buzz_agent_pkg::AgentError::LlmAuth(_)) + if should_start_interactive_auth(&api_key, auth_intent) => + { + let _auth = AUTH_GATE.lock().await; + match buzz_agent_pkg::discover_databricks_models(&config).await { + Ok(entries) => entries, + Err(buzz_agent_pkg::AgentError::LlmAuth(_)) => { + buzz_agent_pkg::authenticate_databricks(&host) + .await + .map_err(|error| { + format_redacted_error( + "Databricks sign-in failed", + &error, + &redaction_env, + ) + })?; + buzz_agent_pkg::discover_databricks_models(&config) + .await + .map_err(|error| { + format_redacted_error( + "Databricks model discovery failed after sign-in", + &error, + &redaction_env, + ) + })? + } + Err(error) => { + return Err(format_redacted_error( + "Databricks model discovery failed", + &error, + &redaction_env, + )); + } + } + } + Err(buzz_agent_pkg::AgentError::LlmAuth(error)) if !api_key.is_empty() => { + return Err(databricks_static_token_error(&error, &redaction_env)); + } + Err(buzz_agent_pkg::AgentError::LlmAuth(_)) => { + return Err(databricks_sign_in_required_error()); + } + Err(error) => { + return Err(format_redacted_error( + "Databricks model discovery failed", + &error, + &redaction_env, + )); + } + }; + + if entries.is_empty() { + return Err("Databricks model discovery returned no models".to_string()); + } + + Ok(Some(AgentModelsResponse { + agent_name: provider_name.trim().to_string(), + agent_version: "models-api".to_string(), + models: entries + .into_iter() + .map(|entry| AgentModelInfo { + id: entry.id, + name: Some(entry.name), + description: None, + }) + .collect(), + agent_default_model: None, + selected_model, + supports_switching: true, + })) +} + +fn format_redacted_error( + context: &str, + error: &impl std::fmt::Display, + redaction_env: &BTreeMap, +) -> String { + let message = crate::managed_agents::redact_env_values_in(&error.to_string(), redaction_env); + format!("{context}: {message}") +} diff --git a/desktop/src-tauri/src/commands/agent_models_tests.rs b/desktop/src-tauri/src/commands/agent_models_tests.rs index 14c981d7302..e7d0e70fd0b 100644 --- a/desktop/src-tauri/src/commands/agent_models_tests.rs +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -576,6 +576,29 @@ fn is_databricks_provider_matches_both_variants() { assert!(!is_databricks_provider(None)); } +#[test] +fn databricks_interactive_auth_requires_explicit_intent_and_no_static_token() { + assert!(should_start_interactive_auth( + "", + DatabricksAuthIntent::InteractiveModelPicker + )); + assert!(!should_start_interactive_auth( + "", + DatabricksAuthIntent::PassiveDraftDiscovery + )); + assert!(!should_start_interactive_auth( + "static-token", + DatabricksAuthIntent::InteractiveModelPicker + )); +} + +#[test] +fn databricks_passive_auth_error_has_reachable_create_flow_guidance() { + let error = databricks_sign_in_required_error(); + assert!(error.contains("save this agent, then open its model picker")); + assert!(error.contains("buzz-agent auth databricks")); +} + #[test] fn model_discovery_error_converts_dangling_sentinel_to_sentence() { // get_agent_models is a user-facing surface: a dangling harness must @@ -881,3 +904,21 @@ fn draft_agent_model_discovery_env_layers_all_three_tiers_in_order() { ); } } + +#[test] +fn databricks_static_token_error_redacts_echoed_token() { + let token = "secret-databricks-token"; + let redaction_env = BTreeMap::from([("DATABRICKS_TOKEN".to_string(), token.to_string())]); + + let error = databricks_static_token_error( + &format!("Databricks rejected bearer {token}"), + &redaction_env, + ); + + assert!(error.contains("[REDACTED]"), "got: {error}"); + assert!(!error.contains(token), "token leaked in error: {error}"); + assert!( + error.contains("update it in agent settings"), + "error lost its remediation: {error}" + ); +} diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index e06f176216c..b90bf49b3b9 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -40,7 +40,9 @@ pub(super) fn build_launch_block( effective_model: Option<&str>, owner_pubkey: &str, ) -> serde_json::Value { - use crate::managed_agents::{known_acp_runtime, resolve_session_title, SESSION_TITLE_ENV_VAR}; + use crate::managed_agents::{ + known_acp_runtime, resolve_session_title, DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, + }; let runtime = known_acp_runtime(&descriptor.command); let mut policy_env = BTreeMap::new(); @@ -73,7 +75,8 @@ pub(super) fn build_launch_block( policy_env.insert("BUZZ_ACP_MAX_TURN_DURATION".into(), value.to_string()); } if let Some(value) = resolve_session_title(record.display_name.as_deref(), &record.name) { - policy_env.insert(SESSION_TITLE_ENV_VAR.into(), value); + policy_env.insert(SESSION_TITLE_ENV_VAR.into(), value.clone()); + policy_env.insert(DISPLAY_NAME_ENV_VAR.into(), value); } if let Some(value) = crate::managed_agents::spawn_snapshot::effective_team_instructions(record, teams) @@ -250,6 +253,7 @@ mod tests { "Coordinate" ); assert_eq!(launch["policy_env"]["BUZZ_ACP_SESSION_TITLE"], "Agent Name"); + assert_eq!(launch["policy_env"]["BUZZ_ACP_DISPLAY_NAME"], "Agent Name"); assert_eq!(launch["policy_env"]["BUZZ_ACP_SYSTEM_PROMPT"], "prompt"); assert_eq!(launch["policy_env"]["BUZZ_ACP_MODEL"], "model"); assert_eq!(launch["policy_env"]["BUZZ_ACP_IDLE_TIMEOUT"], "17"); diff --git a/desktop/src-tauri/src/commands/clipboard.rs b/desktop/src-tauri/src/commands/clipboard.rs index b4fe072ef8d..c904e4a2d94 100644 --- a/desktop/src-tauri/src/commands/clipboard.rs +++ b/desktop/src-tauri/src/commands/clipboard.rs @@ -34,3 +34,22 @@ pub fn with_clipboard( operation(stored.as_mut().expect("clipboard initialized")) .map_err(|e| format!("clipboard error: {e}")) } + +/// Read plain text from the system clipboard through the native shell. +/// +/// Browser clipboard reads are permission-gated or unavailable in embedded +/// webviews. Arboard provides one consistent path across WKWebView, WebView2, +/// and WebKitGTK. The operation runs on the main thread for macOS/AppKit safety. +#[tauri::command] +pub async fn read_clipboard_text(app: tauri::AppHandle) -> Result { + let (tx, rx) = std::sync::mpsc::sync_channel::>(1); + let clipboard_app = app.clone(); + app.run_on_main_thread(move || { + let result = with_clipboard(&clipboard_app, arboard::Clipboard::get_text); + let _ = tx.send(result); + }) + .map_err(|e| format!("main thread dispatch failed: {e}"))?; + + rx.recv() + .map_err(|_| "clipboard result channel closed unexpectedly".to_string())? +} diff --git a/desktop/src-tauri/src/commands/observer_archive.rs b/desktop/src-tauri/src/commands/observer_archive.rs index 707e86b63a7..d8b2832b927 100644 --- a/desktop/src-tauri/src/commands/observer_archive.rs +++ b/desktop/src-tauri/src/commands/observer_archive.rs @@ -1,54 +1,18 @@ -//! Build-time flag and runtime dev-nest check for observer-feed archive policy. +//! Observer-feed archive default — always enabled. //! -//! `observer_archive_default_enabled()` returns `true` when either: -//! - `BUZZ_BUILD_OBSERVER_ARCHIVE_DEFAULT` was set at build time (internal -//! builds bake in the flag via `build.rs`), **or** -//! - the running binary is using the dev nest (`~/.buzz-dev`), which is the -//! case for all dev builds launched with `just staging` or `just dev`. -//! -//! When `true`, the frontend reconciles the observer archive subscription -//! every startup — unconditionally ensuring kind 24200 exists in the DB -//! regardless of stale localStorage markers. -//! -//! OSS prod builds (baked flag unset, prod nest `~/.buzz`) return `false` — -//! no reconciliation; the user manages the subscription via Settings. +//! `observer_archive_default_enabled()` returns `true` unconditionally. +//! The frontend calls this every startup to decide whether to reconcile the +//! `owner_p` subscription for kind 24200 (observer frames). Kind 24200 events +//! are ephemeral — not stored by the relay — so local archiving is the only +//! way to retain them. -/// Returns `true` when observer-feed archive policy is enforced. +/// Returns `true`: observer-feed archive defaults to enabled for all builds. /// -/// True when the build has the internal baked flag set, or when the running -/// binary is using the dev nest (`~/.buzz-dev`). The frontend calls this -/// every startup to decide whether to reconcile the `owner_p` subscription. +/// The frontend reconciles the `owner_p` subscription every startup when this +/// returns `true`. A user who has explicitly disabled the toggle keeps it off +/// because the Settings card's explicit-opt-out path deletes the subscription +/// and the seed hook skips identities that already have an explicit choice. #[tauri::command] pub fn observer_archive_default_enabled() -> bool { - option_env!("BUZZ_DESKTOP_BUILD_OBSERVER_ARCHIVE_DEFAULT").is_some() - || crate::managed_agents::nest_is_dev() -} - -#[cfg(test)] -mod tests { - use super::*; - - // `nest_is_dev()` is deterministic-false in unit tests: NEST_DIR OnceLock - // is uninitialized → falls back to prod `~/.buzz` (nest.rs:101-106), so - // the compiled flag is the sole variable. No runner normalization needed. - // - // #[ignore]: requires BUZZ_TEST_EXPECTED_OBSERVER_ARCHIVE_DEFAULT to be - // set — `just desktop-tauri-test-compiled-flags` runs it explicitly with - // `--ignored` under both compile states; general `cargo test` skips it. - #[test] - #[ignore] - fn test_observer_archive_default_enabled_matches_expected() { - let result = observer_archive_default_enabled(); - let expected_str = std::env::var("BUZZ_TEST_EXPECTED_OBSERVER_ARCHIVE_DEFAULT").expect( - "BUZZ_TEST_EXPECTED_OBSERVER_ARCHIVE_DEFAULT must be set — \ - the dual-compile CI step supplies it; bare `cargo test` is \ - not sufficient to validate compiled-flag behavior", - ); - let expected = expected_str == "true" || expected_str == "1"; - assert_eq!( - result, expected, - "observer_archive_default_enabled() returned {result}, \ - expected {expected} (BUZZ_TEST_EXPECTED_OBSERVER_ARCHIVE_DEFAULT={expected_str:?})" - ); - } + true } diff --git a/desktop/src-tauri/src/commands/project_git_exec.rs b/desktop/src-tauri/src/commands/project_git_exec.rs index e4a8ad7b410..c616d39db1e 100644 --- a/desktop/src-tauri/src/commands/project_git_exec.rs +++ b/desktop/src-tauri/src/commands/project_git_exec.rs @@ -203,6 +203,22 @@ pub(crate) fn build_git_auth_config(state: &AppState) -> Result Result { + if validate_github_clone_url(clone_url).is_ok() { + return Ok(GitAuthConfig { + git_path: resolve_command("git") + .ok_or_else(|| "git was not found on PATH".to_string())?, + credential_helper: None, + nsec: String::new(), + allow_file_transport: false, + }); + } + build_git_auth_config(state) +} + pub(crate) fn build_git_auth_config_for_keys(keys: &Keys) -> Result { let git_path = resolve_command("git").ok_or_else(|| "git was not found on PATH".to_string())?; let credential_helper = resolve_command("git-credential-nostr"); @@ -288,6 +304,56 @@ pub(crate) fn validate_clone_url(clone_url: &str) -> Result<(), String> { Ok(()) } +fn validate_github_clone_url(clone_url: &str) -> Result<(), String> { + let parsed = Url::parse(clone_url).map_err(|error| format!("invalid clone URL: {error}"))?; + if parsed.scheme() != "https" + || parsed.host_str() != Some("github.com") + || parsed.port().is_some() + || !parsed.username().is_empty() + || parsed.password().is_some() + || parsed.query().is_some() + || parsed.fragment().is_some() + { + return Err("GitHub clone URL must use public https://github.com/owner/repository".into()); + } + let segments = parsed + .path_segments() + .map(|segments| { + segments + .filter(|segment| !segment.is_empty()) + .collect::>() + }) + .unwrap_or_default(); + let valid_segment = |segment: &&str| { + !segment.starts_with('-') + && !segment.contains("..") + && segment.chars().all(|character| { + character.is_ascii_alphanumeric() || matches!(character, '.' | '_' | '-') + }) + }; + if segments.len() != 2 || !segments.iter().all(valid_segment) { + return Err("GitHub clone URL must name one owner and repository".into()); + } + Ok(()) +} + +pub(crate) fn validate_local_clone_url(clone_url: &str) -> Result<(), String> { + if validate_clone_url(clone_url).is_ok() || validate_github_clone_url(clone_url).is_ok() { + return Ok(()); + } + Err("clone URL must point at a Buzz repository or public GitHub repository".into()) +} + +pub(crate) fn validate_local_clone_url_for_workspace( + clone_url: &str, + state: &AppState, +) -> Result<(), String> { + if validate_github_clone_url(clone_url).is_ok() { + return Ok(()); + } + validate_workspace_clone_url(clone_url, state) +} + pub(crate) fn clone_url_owner(clone_url: &str) -> Option { let parsed = Url::parse(clone_url).ok()?; let segments = parsed @@ -329,6 +395,7 @@ mod tests { use super::{ clean_branch, clean_target_ref, credential_helper_config_value, git_needs_credentials, git_subcommand, validate_clone_url, validate_clone_url_against_relay, + validate_local_clone_url, }; #[test] @@ -441,4 +508,15 @@ mod tests { ) .is_err()); } + + #[test] + fn local_clone_url_allows_only_public_github_https_urls() { + assert!(validate_local_clone_url("https://github.com/block/buzz").is_ok()); + assert!(validate_local_clone_url("https://github.com/block/buzz.git").is_ok()); + assert!(validate_local_clone_url("http://github.com/block/buzz").is_err()); + assert!(validate_local_clone_url("https://github.com/block/buzz/issues").is_err()); + assert!(validate_local_clone_url("https://user@github.com/block/buzz").is_err()); + assert!(validate_local_clone_url("https://github.com.evil.test/block/buzz").is_err()); + assert!(validate_local_clone_url("https://gitlab.com/block/buzz").is_err()); + } } diff --git a/desktop/src-tauri/src/commands/project_git_workflow.rs b/desktop/src-tauri/src/commands/project_git_workflow.rs index 624bbf4dfc8..9e06852762b 100644 --- a/desktop/src-tauri/src/commands/project_git_workflow.rs +++ b/desktop/src-tauri/src/commands/project_git_workflow.rs @@ -3,8 +3,9 @@ use super::project_git::{first_output_line, normalize_branch_option}; use super::project_git_diff::clean_commit; use super::project_git_exec::{ - build_git_auth_config, build_git_auth_config_for_keys, clone_url_owner, run_git, - validate_clone_url, validate_workspace_clone_url, GitAuthConfig, + build_git_auth_config_for_keys, build_git_clone_auth_config, clone_url_owner, run_git, + validate_local_clone_url, validate_local_clone_url_for_workspace, validate_workspace_clone_url, + GitAuthConfig, }; use super::project_repo_paths::{ canonical_repos_roots, canonicalize_repos_root, default_repos_root_candidates, @@ -353,7 +354,7 @@ pub(crate) fn clone_project_repository_blocking( default_branch: Option<&str>, auth: &GitAuthConfig, ) -> Result { - validate_clone_url(clone_url)?; + validate_local_clone_url(clone_url)?; let branch = normalize_branch_option(default_branch); if let Some(repo_dir) = find_local_repo_dir(repos_dir, project_dtag, Some(clone_url))? { return Ok(ProjectRepoCloneResult { @@ -411,8 +412,8 @@ pub async fn clone_project_repository( default_branch: Option, state: State<'_, AppState>, ) -> Result { - validate_workspace_clone_url(&clone_url, &state)?; - let auth = build_git_auth_config(&state)?; + validate_local_clone_url_for_workspace(&clone_url, &state)?; + let auth = build_git_clone_auth_config(&clone_url, &state)?; tauri::async_runtime::spawn_blocking(move || { clone_project_repository_blocking( repos_dir.as_deref(), diff --git a/desktop/src-tauri/src/commands/project_terminal.rs b/desktop/src-tauri/src/commands/project_terminal.rs index 31dbc74c6db..c583dd0db56 100644 --- a/desktop/src-tauri/src/commands/project_terminal.rs +++ b/desktop/src-tauri/src/commands/project_terminal.rs @@ -9,7 +9,10 @@ use crate::app_state::AppState; use super::project_git::{first_output_line, normalize_branch_option}; use super::project_git_diff::clean_commit; -use super::project_git_exec::{build_git_auth_config, run_git, validate_workspace_clone_url}; +use super::project_git_exec::{ + build_git_auth_config, build_git_clone_auth_config, run_git, + validate_local_clone_url_for_workspace, validate_workspace_clone_url, +}; use super::project_git_workflow::clone_project_repository_blocking; use super::project_repo_paths::find_local_repo_dir; @@ -99,9 +102,8 @@ fn launch_terminal_at(path: &std::path::Path) -> Result<(), String> { } /// Opens the OS terminal at the project's local checkout. When there is no -/// local checkout yet, clones the repository from `clone_url` (authenticated -/// with the identity key, same as push/snapshot) into the repos dir first, -/// then opens the terminal at the fresh checkout. +/// local checkout yet, clones the repository from `clone_url` into the repos +/// dir first, then opens the terminal at the fresh checkout. #[tauri::command] pub async fn open_project_terminal( repos_dir: Option, @@ -111,11 +113,16 @@ pub async fn open_project_terminal( state: State<'_, AppState>, ) -> Result { if let Some(clone_url) = clone_url.as_deref() { - validate_workspace_clone_url(clone_url, &state)?; + validate_local_clone_url_for_workspace(clone_url, &state)?; } - // Auth is only needed for the clone path — keep the result outside the - // blocking task so it owns no borrowed Tauri state. - let auth = build_git_auth_config(&state); + // Public GitHub clones stay anonymous; Buzz remotes use the workspace + // identity. Keep the result outside the blocking task so it borrows no + // Tauri state. + let auth = if let Some(clone_url) = clone_url.as_deref() { + build_git_clone_auth_config(clone_url, &state) + } else { + build_git_auth_config(&state) + }; tauri::async_runtime::spawn_blocking(move || { // An inaccessible repos root (fresh machine, nothing cloned yet) is // not fatal here — the clone path below creates the default root. A diff --git a/desktop/src-tauri/src/huddle/channel_name.rs b/desktop/src-tauri/src/huddle/channel_name.rs new file mode 100644 index 00000000000..ad8344652e2 --- /dev/null +++ b/desktop/src-tauri/src/huddle/channel_name.rs @@ -0,0 +1,15 @@ +pub(super) fn normalize_huddle_channel_name(candidate: Option, fallback: &str) -> String { + let normalized = candidate + .unwrap_or_default() + .split_whitespace() + .collect::>() + .join(" "); + + let name = if normalized.is_empty() { + fallback + } else { + normalized.as_str() + }; + + name.chars().take(80).collect() +} diff --git a/desktop/src-tauri/src/huddle/commands.rs b/desktop/src-tauri/src/huddle/commands.rs new file mode 100644 index 00000000000..993d8e54eba --- /dev/null +++ b/desktop/src-tauri/src/huddle/commands.rs @@ -0,0 +1,132 @@ +//! Small Huddle controls that mutate an active session. + +use std::sync::{atomic::Ordering, Arc}; + +use tauri::State; +use uuid::Uuid; + +use crate::{app_state::AppState, events, relay::submit_event}; + +use super::{relay_api::validate_pubkey_hex, HuddlePhase}; + +/// Update the clickable microphone control independently from the PTT shortcut. +#[tauri::command] +pub fn set_huddle_manual_mic_unmuted( + enabled: bool, + state: State<'_, AppState>, +) -> Result<(), String> { + let huddle = state.huddle()?; + if !matches!(huddle.phase, HuddlePhase::Connected | HuddlePhase::Active) { + return Err("no active huddle".to_string()); + } + huddle.manual_mic_unmuted.store(enabled, Ordering::Release); + Ok(()) +} + +/// Immediately interrupt the agent utterance that is currently speaking. +#[tauri::command] +pub fn interrupt_huddle_speech( + agent_pubkey: String, + state: State<'_, AppState>, +) -> Result<(), String> { + validate_pubkey_hex(&agent_pubkey)?; + let tts_pipeline = { + let huddle = state.huddle()?; + if !matches!(huddle.phase, HuddlePhase::Connected | HuddlePhase::Active) { + return Err("no active huddle".to_string()); + } + huddle.tts_pipeline.as_ref().map(Arc::clone) + }; + if let Some(tts_pipeline) = tts_pipeline { + tts_pipeline.cancel_active_speaker(&agent_pubkey); + } + Ok(()) +} + +/// Remove an agent from the active huddle without removing its parent-channel +/// membership. Keeping the parent membership intact means it remains available +/// to rejoin this huddle from the agent picker. +#[tauri::command] +pub async fn remove_agent_from_huddle( + agent_pubkey: String, + state: State<'_, AppState>, +) -> Result<(), String> { + validate_pubkey_hex(&agent_pubkey)?; + + let (ephemeral_channel_id, huddle_generation) = { + let huddle = state.huddle()?; + if !matches!(huddle.phase, HuddlePhase::Connected | HuddlePhase::Active) { + return Err("no active huddle".to_string()); + } + + let is_huddle_agent = huddle + .agent_pubkeys + .lock() + .unwrap_or_else(|error| error.into_inner()) + .iter() + .any(|pubkey| pubkey.eq_ignore_ascii_case(&agent_pubkey)); + if !is_huddle_agent { + return Err("agent is not in this huddle".to_string()); + } + + ( + huddle + .ephemeral_channel_id + .clone() + .ok_or("no ephemeral channel")?, + huddle.huddle_generation, + ) + }; + + let ephemeral_channel_uuid = + Uuid::parse_str(&ephemeral_channel_id).map_err(|error| error.to_string())?; + submit_event( + events::build_remove_member(ephemeral_channel_uuid, &agent_pubkey)?, + &state, + ) + .await?; + + let (roster_changed, tts_pipeline) = { + let mut huddle = state.huddle()?; + if !huddle.is_current_huddle(&ephemeral_channel_id, huddle_generation) { + (false, None) + } else { + let mut agent_pubkeys = huddle + .agent_pubkeys + .lock() + .unwrap_or_else(|error| error.into_inner()); + let initial_count = agent_pubkeys.len(); + agent_pubkeys.retain(|pubkey| !pubkey.eq_ignore_ascii_case(&agent_pubkey)); + let changed = agent_pubkeys.len() != initial_count; + drop(agent_pubkeys); + + if changed { + huddle + .participants + .retain(|pubkey| !pubkey.eq_ignore_ascii_case(&agent_pubkey)); + if let Some(settings_pubkey) = huddle + .agent_voice_settings + .keys() + .find(|pubkey| pubkey.eq_ignore_ascii_case(&agent_pubkey)) + .cloned() + { + huddle.agent_voice_settings.remove(&settings_pubkey); + } + } + let tts_pipeline = changed + .then_some(huddle.tts_pipeline.as_ref()) + .flatten() + .map(Arc::clone); + (changed, tts_pipeline) + } + }; + + if let Some(tts_pipeline) = tts_pipeline { + tts_pipeline.cancel_speaker(&agent_pubkey); + } + if roster_changed { + state.emit_huddle_state_changed(); + } + + Ok(()) +} diff --git a/desktop/src-tauri/src/huddle/mod.rs b/desktop/src-tauri/src/huddle/mod.rs index 668d5a030b9..e4e383ed970 100644 --- a/desktop/src-tauri/src/huddle/mod.rs +++ b/desktop/src-tauri/src/huddle/mod.rs @@ -27,6 +27,8 @@ mod agent_tts_routing; pub mod agent_voice; pub mod agents; pub mod audio_output; +mod channel_name; +mod commands; pub mod jitter; pub mod message_read_aloud; pub mod models; @@ -68,6 +70,9 @@ pub(super) fn drain_until_shutdown( // ── Re-exports ──────────────────────────────────────────────────────────────── +pub use commands::{ + interrupt_huddle_speech, remove_agent_from_huddle, set_huddle_manual_mic_unmuted, +}; pub use state::{HuddleJoinInfo, HuddlePhase, HuddleState, VoiceInputMode}; pub use transcription::{set_huddle_transcription_enabled, start_stt_pipeline}; pub use tts_settings::set_tts_enabled; @@ -85,6 +90,7 @@ use agent_tts_routing::{ classify_agent_tts_runtime, enqueue_agent_tts_text, normalize_agent_tts_text, AgentTtsRuntimeGate, }; +use channel_name::normalize_huddle_channel_name; pub use pipeline::check_pipeline_hotstart; use pipeline::{ await_inflight_tts_start, maybe_start_stt_pipeline, maybe_start_tts_pipeline, @@ -96,22 +102,6 @@ use relay_api::{ }; use window::close_huddle_window; -fn normalize_huddle_channel_name(candidate: Option, fallback: &str) -> String { - let normalized = candidate - .unwrap_or_default() - .split_whitespace() - .collect::>() - .join(" "); - - let name = if normalized.is_empty() { - fallback - } else { - normalized.as_str() - }; - - name.chars().take(80).collect() -} - // ── Tauri commands ──────────────────────────────────────────────────────────── /// Set the voice input mode (push-to-talk or voice-activity detection). @@ -869,11 +859,27 @@ pub async fn speak_agent_message( let sender = { let hs = state.huddle()?; + let agent_is_present = hs + .agent_pubkeys + .lock() + .unwrap_or_else(|error| error.into_inner()) + .iter() + .any(|pubkey| pubkey.eq_ignore_ascii_case(&speaker_pubkey)); + if !agent_is_present { + eprintln!( + "buzz-desktop: tts stage=queue status=dropped reason=speaker_removed route_id={route_id}" + ); + return Ok(()); + } hs.tts_pipeline .as_ref() .map(|pipeline| pipeline.text_sender()) + .map(|sender| { + let speaker_generation = sender.speaker_generation(&speaker_pubkey); + (sender, speaker_generation) + }) }; - let Some(sender) = sender else { + let Some((sender, speaker_generation)) = sender else { eprintln!( "buzz-desktop: tts stage=invoke status=failed reason=unavailable route_id={route_id}" ); @@ -881,7 +887,13 @@ pub async fn speak_agent_message( }; enqueue_agent_tts_text(route_id, text, move |route_id, text| { sender - .send(route_id, speaker_pubkey, voice_reference, text) + .send( + route_id, + speaker_pubkey, + speaker_generation, + voice_reference, + text, + ) .map_err(|error| format!("TTS queue closed while waiting to enqueue: {error}")) }) .await diff --git a/desktop/src-tauri/src/huddle/pipeline.rs b/desktop/src-tauri/src/huddle/pipeline.rs index 9572ac25bf0..e523ee22bf2 100644 --- a/desktop/src-tauri/src/huddle/pipeline.rs +++ b/desktop/src-tauri/src/huddle/pipeline.rs @@ -315,6 +315,7 @@ pub(crate) async fn maybe_start_stt_pipeline( expected_generation, stt_starting, ptt_active_for_stt, + manual_mic_unmuted_for_stt, old_stt, ) = { let mut hs = state.huddle()?; @@ -338,6 +339,11 @@ pub(crate) async fn maybe_start_stt_pipeline( } else { None }; + let manual_mic_unmuted = if hs.voice_input_mode == VoiceInputMode::PushToTalk { + Some(Arc::clone(&hs.manual_mic_unmuted)) + } else { + None + }; ( Arc::clone(&hs.tts_active), Arc::clone(&hs.agent_pubkeys), @@ -345,6 +351,7 @@ pub(crate) async fn maybe_start_stt_pipeline( hs.session_generation.load(Ordering::Acquire), stt_starting, ptt, + manual_mic_unmuted, old, ) }; @@ -352,7 +359,12 @@ pub(crate) async fn maybe_start_stt_pipeline( drop(old_stt); let constructed = tokio::task::spawn_blocking(move || { - stt::SttPipeline::new(model_dir, tts_active, ptt_active_for_stt) + stt::SttPipeline::new( + model_dir, + tts_active, + ptt_active_for_stt, + manual_mic_unmuted_for_stt, + ) }) .await; let (pipeline, text_rx) = match constructed { diff --git a/desktop/src-tauri/src/huddle/state.rs b/desktop/src-tauri/src/huddle/state.rs index 0fe3a46f5aa..7acf5fe633b 100644 --- a/desktop/src-tauri/src/huddle/state.rs +++ b/desktop/src-tauri/src/huddle/state.rs @@ -15,7 +15,7 @@ use super::{stt, tts}; /// Voice input mode: push-to-talk (PTT) or voice-activity detection (VAD). /// -/// PTT: mic is gated by a global shortcut (Ctrl+Space). Pressing the key sets +/// PTT (the default): mic is gated by a global shortcut (Ctrl+Space). Pressing the key sets /// `ptt_active` and immediately cancels any playing TTS. Releasing the key /// (after a 200 ms delay) stops mic capture and flushes the utterance. /// @@ -26,8 +26,8 @@ use super::{stt, tts}; #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "snake_case")] pub enum VoiceInputMode { - PushToTalk, #[default] + PushToTalk, VoiceActivity, } @@ -135,6 +135,10 @@ pub struct HuddleState { /// Shared with the STT pipeline for mic gating. #[serde(skip)] pub ptt_active: Arc, + /// True while the clickable microphone control is manually unmuted. + /// In PTT mode, either this flag or `ptt_active` opens the STT gate. + #[serde(skip)] + pub manual_mic_unmuted: Arc, } fn serialize_agent_pubkeys(v: &Arc>>, s: S) -> Result @@ -190,6 +194,7 @@ impl Clone for HuddleState { session_generation: Arc::clone(&self.session_generation), voice_input_mode: self.voice_input_mode.clone(), ptt_active: Arc::clone(&self.ptt_active), + manual_mic_unmuted: Arc::clone(&self.manual_mic_unmuted), } } } @@ -221,6 +226,7 @@ impl Default for HuddleState { session_generation: Arc::new(AtomicU64::new(0)), voice_input_mode: VoiceInputMode::default(), ptt_active: Arc::new(AtomicBool::new(false)), + manual_mic_unmuted: Arc::new(AtomicBool::new(true)), } } } @@ -332,6 +338,13 @@ mod tests { assert!(!state.maybe_auto_enable_transcription_for_agents()); } + #[test] + fn defaults_to_push_to_talk_with_an_open_microphone() { + let state = HuddleState::default(); + assert_eq!(state.voice_input_mode, super::VoiceInputMode::PushToTalk); + assert!(state.manual_mic_unmuted.load(Ordering::Acquire)); + } + #[test] fn explicit_user_disable_is_not_undone_by_agent_presence() { let mut state = HuddleState::default(); diff --git a/desktop/src-tauri/src/huddle/stt.rs b/desktop/src-tauri/src/huddle/stt.rs index 30a47f449a7..70a80886402 100644 --- a/desktop/src-tauri/src/huddle/stt.rs +++ b/desktop/src-tauri/src/huddle/stt.rs @@ -71,9 +71,10 @@ impl SttPipeline { /// therefore never cancel TTS. Push-to-talk and remote participant speech /// remain explicit, reliable barge-in paths. /// - /// `ptt_active` (optional) is the push-to-talk flag. When `Some`, the STT - /// pipeline only accumulates speech while the flag is true (key held). - /// When `None`, the pipeline runs in continuous VAD mode. + /// `ptt_active` and `manual_mic_unmuted` are present when the PTT shortcut + /// is enabled. The pipeline accepts speech while either input path is open; + /// manual unmute uses normal VAD flushing while a shortcut hold is grouped + /// into one utterance. /// /// Returns `Err` only if the thread cannot be spawned (OS error). /// If model files are missing, the worker logs and exits cleanly — @@ -87,6 +88,7 @@ impl SttPipeline { model_dir: PathBuf, tts_active: Arc, ptt_active: Option>, + manual_mic_unmuted: Option>, ) -> Result<(Self, tokio_mpsc::Receiver), String> { let (audio_tx, audio_rx) = mpsc::sync_channel::>(AUDIO_QUEUE_DEPTH); let (text_tx, text_rx) = tokio_mpsc::channel::(64); @@ -94,6 +96,7 @@ impl SttPipeline { let shutdown_worker = Arc::clone(&shutdown); let ptt_active_worker = ptt_active.as_ref().map(Arc::clone); + let manual_mic_unmuted_worker = manual_mic_unmuted.as_ref().map(Arc::clone); let handle = thread::Builder::new() .name("stt-worker".into()) .spawn(move || { @@ -104,6 +107,7 @@ impl SttPipeline { shutdown_worker, tts_active, ptt_active_worker, + manual_mic_unmuted_worker, ) }) .map_err(|e| format!("failed to spawn stt-worker thread: {e}"))?; @@ -203,6 +207,7 @@ fn stt_worker( shutdown: Arc, tts_active: Arc, ptt_active: Option>, + manual_mic_unmuted: Option>, ) { // ── 1. Initialise rubato resampler (48 kHz → 16 kHz, mono) ─────────────── use rubato::{Fft, FixedSync, Resampler}; @@ -275,9 +280,12 @@ fn stt_worker( // ── 5. Main loop ────────────────────────────────────────────────────────── let mut tts_was_active = false; - let mut ptt_was_active = ptt_active + let mut transmit_was_active = ptt_active .as_ref() - .is_some_and(|p| p.load(Ordering::Acquire)); + .is_some_and(|ptt| ptt.load(Ordering::Acquire)) + || manual_mic_unmuted + .as_ref() + .is_some_and(|manual| manual.load(Ordering::Acquire)); loop { // Check shutdown flag before blocking. if shutdown.load(Ordering::Acquire) { @@ -292,20 +300,22 @@ fn stt_worker( } tts_was_active = tts_now; - // Track PTT transitions — flush accumulated speech when key is released. - // The worklet stops sending frames when PTT is inactive, so the normal - // silence-accumulation flush path never runs. We must flush here on the - // active→inactive edge to avoid buffering speech across PTT presses. + // Track the combined manual/PTT transmission edge. When both paths + // close, the worklet stops sending frames, so flush here rather than + // waiting for silence that will never arrive. if let Some(ref ptt) = ptt_active { - let ptt_now = ptt.load(Ordering::Acquire); - if ptt_was_active && !ptt_now && in_speech && !speech_buf.is_empty() { + let transmit_now = ptt.load(Ordering::Acquire) + || manual_mic_unmuted + .as_ref() + .is_some_and(|manual| manual.load(Ordering::Acquire)); + if transmit_was_active && !transmit_now && in_speech && !speech_buf.is_empty() { flush_to_stt(&speech_buf, voiced_frames, &recognizer, &text_tx); speech_buf.clear(); silence_frames = 0; in_speech = false; voiced_frames = 0; } - ptt_was_active = ptt_now; + transmit_was_active = transmit_now; } // Use recv_timeout so we can periodically check the shutdown flag. @@ -343,6 +353,7 @@ fn stt_worker( &tts_active, &mut tts_stopped_at, ptt_active.as_ref(), + manual_mic_unmuted.as_ref(), ); } } @@ -385,11 +396,9 @@ fn resample_chunk(resampler: &mut rubato::Fft, chunk_48k: &[f32]) -> Vec, tts_stopped_at: &mut Option, ptt_active: Option<&Arc>, + manual_mic_unmuted: Option<&Arc>, ) { leftover.extend_from_slice(samples); @@ -413,13 +423,11 @@ fn process_16k_samples( let prob = vad.predict_f32(&clamped); let is_speech = prob > VAD_THRESHOLD; - // PTT gating: when PTT key is not held, treat as silence. - // This causes natural flush when the key is released — silence_frames - // accumulates and the existing flush logic kicks in after - // SILENCE_FLUSH_FRAMES. The 200 ms release delay + ~300 ms silence - // flush gives a natural utterance tail. + let manually_open = manual_mic_unmuted.is_some_and(|manual| manual.load(Ordering::Acquire)); + // Shortcut-enabled mode accepts input from either the held shortcut or + // a manually open microphone. let is_speech = if let Some(ptt) = ptt_active { - is_speech && ptt.load(Ordering::Acquire) + is_speech && (ptt.load(Ordering::Acquire) || manually_open) } else { is_speech }; @@ -478,11 +486,9 @@ fn process_16k_samples( speech_buf.extend_from_slice(&frame); *silence_frames += 1; - // In PTT mode, don't flush on silence — accumulate the entire - // key-hold as one utterance. The PTT release edge in the main - // loop handles the flush. In VAD mode, flush after the silence - // threshold so each natural pause becomes a separate message. - if ptt_active.is_none() && *silence_frames >= SILENCE_FLUSH_FRAMES { + // A manually open microphone behaves like normal VAD. A + // shortcut-only transmission stays grouped until key release. + if (ptt_active.is_none() || manually_open) && *silence_frames >= SILENCE_FLUSH_FRAMES { // End of utterance — transcribe. flush_to_stt(speech_buf, *voiced_frames, recognizer, text_tx); speech_buf.clear(); diff --git a/desktop/src-tauri/src/huddle/tts.rs b/desktop/src-tauri/src/huddle/tts.rs index 1901bb3d2ea..6a56f85444c 100644 --- a/desktop/src-tauri/src/huddle/tts.rs +++ b/desktop/src-tauri/src/huddle/tts.rs @@ -64,6 +64,11 @@ use audio::*; #[path = "tts_activity.rs"] mod activity; use activity::*; +#[path = "tts_pipeline_controls.rs"] +mod pipeline_controls; +#[path = "tts_speaker_cancellation.rs"] +mod speaker_cancellation; +use speaker_cancellation::*; // ── Constants ───────────────────────────────────────────────────────────────── @@ -126,7 +131,15 @@ const MAX_CHUNK_CHARS: usize = 200; /// Injected as a silent buffer between each synthesized sentence chunk. const INTER_SENTENCE_SILENCE: f32 = 0.1; -type WorkerControlState = (Arc, Arc, WorkerCancelSignals); +type WorkerControlState = ( + Arc, + Arc, + WorkerCancelSignals, + SpeakerGenerations, + ActiveSpeaker, + SpeakerCancellation, + PlaybackProbe, +); // ── Public pipeline handle ──────────────────────────────────────────────────── @@ -154,6 +167,15 @@ pub struct TtsPipeline { voice: Arc>, /// Tags messages so a voice change drops only pre-change queue entries. voice_generation: Arc, + /// Per-agent generations let removal invalidate that agent's queued and + /// in-flight text without poisoning speech queued after the agent rejoins. + speaker_generations: SpeakerGenerations, + /// Speaker whose audio currently owns the shared player queue. + active_speaker: ActiveSpeaker, + /// Targeted cancellation used when an agent leaves the huddle. + speaker_cancel: SpeakerCancellation, + /// Shared player handle used to reject Stop clicks after playback drains. + playback_probe: PlaybackProbe, /// Completed after the worker drains pre-change text and installs the new style. voice_change_ack: VoiceChangeAck, /// Worker thread handle — taken on drop to join cleanly. @@ -187,6 +209,14 @@ impl TtsPipeline { let voice_worker = Arc::clone(&voice); let voice_generation = Arc::new(AtomicU64::new(1)); let worker_voice_generation = Arc::clone(&voice_generation); + let speaker_generations = Arc::new(Mutex::new(HashMap::new())); + let worker_speaker_generations = Arc::clone(&speaker_generations); + let active_speaker = Arc::new(Mutex::new(None)); + let worker_active_speaker = Arc::clone(&active_speaker); + let speaker_cancel = Arc::new(Mutex::new(None)); + let worker_speaker_cancel = Arc::clone(&speaker_cancel); + let playback_probe = PlaybackProbe::new(); + let worker_playback_probe = playback_probe.clone(); let voice_change_ack = Arc::new(Mutex::new(None)); let worker_voice_change_ack = Arc::clone(&voice_change_ack); let model_dir_worker = model_dir.clone(); @@ -207,6 +237,10 @@ impl TtsPipeline { tts_active_worker, shutdown_worker, (cancel_worker, worker_voice_cancel), + worker_speaker_generations, + worker_active_speaker, + worker_speaker_cancel, + worker_playback_probe, ), output_device, activity_app, @@ -224,79 +258,14 @@ impl TtsPipeline { voice_cancel, voice, voice_generation, + speaker_generations, + active_speaker, + speaker_cancel, + playback_probe, voice_change_ack, thread: Some(handle), }) } - - /// Queue `text` for TTS synthesis and playback. - /// - /// Non-blocking. Returns `Err` if the queue is full (bounded at - /// `TEXT_QUEUE_DEPTH`) — caller may log and discard. - pub fn speak(&self, text: String) -> Result<(), String> { - self.text_tx - .try_send(QueuedText { - generation: self.voice_generation.load(Ordering::Acquire), - route_id: 0, - speaker_pubkey: None, - voice_reference: None, - text, - }) - .map_err(|e| { - eprintln!("buzz-desktop: TTS queue saturated, dropping message: {e}"); - format!("TTS queue full, dropping: {e}") - }) - } - - /// Clone the bounded queue sender so callers can apply backpressure without - /// holding the huddle mutex. Disabling TTS drops the receiver and unblocks - /// any waiting sender while the shared cancellation flag stops playback. - pub(crate) fn text_sender(&self) -> TtsTextSender { - TtsTextSender { - text_tx: self.text_tx.clone(), - generation: self.voice_generation.load(Ordering::Acquire), - } - } - - /// Select a bundled Pocket voice for subsequent speech. - /// - /// Current playback and queued text are cancelled immediately so content - /// cannot continue in the old voice. The worker keeps its warmed inference - /// engine and reloads only the reference style before the next utterance. - pub fn select_voice(&self, voice: &str) -> Option> { - let acknowledged = begin_voice_change( - &self.voice, - &self.voice_generation, - &self.voice_cancel, - &self.voice_change_ack, - voice, - ); - if acknowledged.is_some() { - eprintln!("buzz-desktop: tts stage=cancellation reason=voice_switch route_id=0"); - } - acknowledged - } - - /// Reconcile the voice of a pipeline that has not been published yet. - /// - /// No caller can enqueue text before publication, so raising the shared - /// cancellation flag here would create a race that could discard the first - /// message queued immediately after installation. - pub(crate) fn select_voice_before_publish(&self, voice: &str) { - *self.voice.lock().unwrap_or_else(|error| error.into_inner()) = voice.to_string(); - } - - /// Signal the worker thread to stop. - pub fn shutdown(&self) { - eprintln!("buzz-desktop: tts stage=cancellation reason=shutdown route_id=0"); - self.shutdown.store(true, Ordering::Release); - } - - /// Returns `true` if the worker thread has exited (init failure, crash, or normal exit). - /// Used by hot-start to detect dead pipelines and clear them for retry. - pub fn is_finished(&self) -> bool { - self.thread.as_ref().is_none_or(|h| h.is_finished()) - } } impl Drop for TtsPipeline { @@ -322,7 +291,15 @@ fn tts_worker( startup_tx: mpsc::SyncSender>, ) { let (selected_voice, voice_generation, voice_change_ack) = voice_state; - let (tts_active, shutdown, cancel_signals) = control_state; + let ( + tts_active, + shutdown, + cancel_signals, + speaker_generations, + active_speaker, + speaker_cancel, + playback_probe, + ) = control_state; let (cancel, voice_cancel) = cancel_signals; // ── 1. Initialise TTS engine ────────────────────────────────────────────── let model_dir_str = model_dir.to_string_lossy().to_string(); @@ -415,6 +392,7 @@ fn tts_worker( // Shared (Arc) with the barge-in monitor thread below, which needs to // silence it while this thread is blocked inside `synth_chunk`. let player = Arc::new(Player::connect_new(sink_handle.mixer())); + playback_probe.install(Arc::clone(&player)); // Prime the audio output stream with a short silent buffer. // On macOS, CoreAudio initializes the output device lazily on first use. @@ -443,103 +421,21 @@ fn tts_worker( } eprintln!("buzz-desktop: tts stage=startup status=ready"); - // ── 3b. Barge-in monitor thread ─────────────────────────────────────────── - // - // The worker loop only observes `cancel` between sentences — while it is - // blocked inside `synth_chunk` (hundreds of ms for a long sentence), - // nothing would silence the audio that is already playing. The monitor - // closes that gap: every MONITOR_TICK it checks the flag and, while set, - // silences the player and releases the mic gate. It does NOT consume the - // flag — the worker still owns that (drain queue, reset lead-in), so the - // monitor keeps re-clearing until the worker catches up, which also - // covers a sentence appended in the race window after the worker's own - // post-synthesis cancel check. - // - // `player_ops` closes the converse race (found in review): the monitor - // loads `cancel == true`, is preempted, the worker consumes the cancel - // and appends a fresh post-cancel utterance, then the monitor resumes - // from its stale branch and deletes audio that was meant to play. All - // worker player mutations (appends and cancel/shutdown clears) hold this - // lock, and the monitor re-checks `cancel` *while holding it* — so its - // clear either runs before fresh audio can be appended, or observes - // `cancel == false` and no-ops. The lock is uncontended except during an - // actual barge-in, so the hot path is unaffected. - let player_ops = Arc::new(Mutex::new(())); + let player_ops = Arc::clone(&playback_probe.player_ops); let activity_frames = Arc::new(Mutex::new(VecDeque::::new())); let monitor_stop = Arc::new(AtomicBool::new(false)); - let monitor = { - let player = Arc::clone(&player); - let cancel = Arc::clone(&cancel); - let voice_cancel = Arc::clone(&voice_cancel); - let tts_active = Arc::clone(&tts_active); - let stop = Arc::clone(&monitor_stop); - let player_ops = Arc::clone(&player_ops); - let activity_frames = Arc::clone(&activity_frames); - thread::Builder::new() - .name("tts-barge-in-monitor".into()) - .spawn(move || { - let mut last_activity_pubkey: Option = None; - let mut next_activity_tick = Instant::now(); - while !stop.load(Ordering::Acquire) { - if cancel.load(Ordering::Acquire) || voice_cancel.load(Ordering::Acquire) { - let _ops = lock_player_ops(&player_ops); - // Re-check under the lock: the worker may have - // consumed this cancel (and appended fresh audio) - // between the load above and the lock acquisition. - if cancel.load(Ordering::Acquire) || voice_cancel.load(Ordering::Acquire) { - // clear() pauses the persistent player; play() - // un-pauses (see handle_cancel_or_shutdown). - // Idempotent — safe to repeat every tick until - // the worker consumes the flag. - player.clear(); - player.play(); - tts_active.store(false, Ordering::Release); - } - } - if let Some(ref app) = activity_app { - if tts_active.load(Ordering::Acquire) { - let now = Instant::now(); - if now >= next_activity_tick { - let frame = activity_frames - .lock() - .unwrap_or_else(|error| error.into_inner()) - .pop_front(); - if let Some(frame) = frame { - use tauri::Emitter; - let _ = app.emit( - "huddle-tts-speaker-level", - TtsSpeakerActivityPayload { - pubkey: Some(frame.pubkey.clone()), - level: frame.level, - }, - ); - last_activity_pubkey = Some(frame.pubkey); - } - next_activity_tick = now + SPEAKER_ACTIVITY_TICK; - } - } else { - let had_activity = last_activity_pubkey.take().is_some(); - activity_frames - .lock() - .unwrap_or_else(|error| error.into_inner()) - .clear(); - if had_activity { - use tauri::Emitter; - let _ = app.emit( - "huddle-tts-speaker-level", - TtsSpeakerActivityPayload { - pubkey: None, - level: 0.0, - }, - ); - } - next_activity_tick = Instant::now(); - } - } - thread::sleep(MONITOR_TICK); - } - }) - }; + let monitor = spawn_tts_monitor(TtsMonitorState { + player: Arc::clone(&player), + cancel: Arc::clone(&cancel), + voice_cancel: Arc::clone(&voice_cancel), + tts_active: Arc::clone(&tts_active), + stop: Arc::clone(&monitor_stop), + player_ops: Arc::clone(&player_ops), + activity_frames: Arc::clone(&activity_frames), + active_speaker: Arc::clone(&active_speaker), + speaker_cancel: Arc::clone(&speaker_cancel), + activity_app, + }); if let Err(ref e) = monitor { // Degraded but functional: barge-in still works between sentences // via the worker's own checks, just not mid-synthesis. @@ -564,7 +460,8 @@ fn tts_worker( let mut deferred_text = VecDeque::new(); let append_audio = |prepared: PreparedModelAudio, route_id: u64, - speaker_pubkey: Option<&str>| { + speaker_pubkey: Option<&str>, + speaker_generation: u64| { let _ops = lock_player_ops(&player_ops); if cancel.load(Ordering::Acquire) || voice_cancel.load(Ordering::Acquire) @@ -582,6 +479,30 @@ fn tts_worker( ); return false; } + let speaker_is_current = speaker_pubkey.is_none_or(|pubkey| { + current_speaker_generation(&speaker_generations, pubkey) == speaker_generation + }); + if !speaker_is_current { + eprintln!( + "buzz-desktop: tts stage=synthesis status=cancelled reason=speaker_removed route_id={route_id}" + ); + return false; + } + if let Some(pubkey) = speaker_pubkey { + let mut active = active_speaker + .lock() + .unwrap_or_else(|error| error.into_inner()); + if player.empty() { + active.take(); + } + if active + .as_deref() + .is_some_and(|current| !current.eq_ignore_ascii_case(pubkey)) + { + return false; + } + active.get_or_insert_with(|| pubkey.to_ascii_lowercase()); + } if let Some(pubkey) = speaker_pubkey { activity_frames .lock() @@ -604,6 +525,17 @@ fn tts_worker( loop { let mut no_current_text = None; + if consume_speaker_cancel( + &speaker_cancel, + &active_speaker, + &speaker_generations, + &tts_active, + (&text_rx, &mut deferred_text, &mut no_current_text), + Some((&player, &player_ops)), + ) { + first_append = true; + continue; + } if handle_cancel_or_shutdown( (&cancel, &voice_cancel), &shutdown, @@ -647,6 +579,10 @@ fn tts_worker( // lead-in so the next utterance gets a fresh cushion. if player.empty() && !first_append { tts_active.store(false, Ordering::Release); + active_speaker + .lock() + .unwrap_or_else(|error| error.into_inner()) + .take(); eprintln!( "buzz-desktop: tts stage=player status=drained route_id={last_route_id}" ); @@ -679,6 +615,13 @@ fn tts_worker( let Some(queued_text) = queued_text else { continue; }; + if !queued_speaker_is_current(&speaker_generations, &queued_text) { + eprintln!( + "buzz-desktop: tts stage=queue status=dropped reason=speaker_removed route_id={}", + queued_text.route_id + ); + continue; + } if queued_text.generation < voice_generation.load(Ordering::Acquire) { eprintln!( "buzz-desktop: tts stage=queue status=dropped reason=voice_switch route_id={}", @@ -686,6 +629,22 @@ fn tts_worker( ); continue; } + if !player.empty() + && queued_text + .speaker_pubkey + .as_deref() + .is_some_and(|speaker| { + active_speaker + .lock() + .unwrap_or_else(|error| error.into_inner()) + .as_deref() + .is_some_and(|active| !active.eq_ignore_ascii_case(speaker)) + }) + { + deferred_text.push_front(queued_text); + thread::sleep(RECV_TIMEOUT); + continue; + } let requested_voice = queued_text.voice_reference.unwrap_or_else(|| { selected_voice .lock() @@ -694,9 +653,32 @@ fn tts_worker( }); let raw_text = queued_text.text; let speaker_pubkey = queued_text.speaker_pubkey; + let speaker_generation = queued_text.speaker_generation; let route_id = queued_text.route_id; eprintln!("buzz-desktop: tts stage=synthesis status=started route_id={route_id}"); + // If playback already drained while we were waiting for this item, + // release stale ownership before doing any potentially slow voice or + // synthesis work. Serialize the drain decision with Stop and append so + // those paths observe one coherent utterance boundary. + { + let _ops = lock_player_ops(&player_ops); + if player.empty() && !first_append { + tts_active.store(false, Ordering::Release); + active_speaker + .lock() + .unwrap_or_else(|error| error.into_inner()) + .take(); + eprintln!("buzz-desktop: tts stage=player status=drained route_id={last_route_id}"); + first_append = true; + } + } + + // From this point until the item finishes, an empty player can mean a + // voice-preparation or synthesis gap rather than a drained utterance. + // Stop must remain able to invalidate the in-flight speaker generation. + let _synthesis_flight = playback_probe.begin_synthesis(); + // The selected per-agent voice travels with the queue item, preserving // message order while allowing one warmed Pocket engine to alternate // between cached reference styles. @@ -714,20 +696,6 @@ fn tts_worker( continue; } - // If playback already drained while we were waiting for this item, - // the agent is silent — release the mic gate BEFORE preprocessing/ - // synthesis. Without this, an item arriving inside the recv timeout - // window would run the whole synthesis pass with `tts_active` stuck - // true and nothing playing, making STT discard human speech as - // "echo" during a silent window. (Pipelining is unaffected: when - // audio is still draining, `player.empty()` is false and the flag - // stays set across items.) - if player.empty() && !first_append { - tts_active.store(false, Ordering::Release); - eprintln!("buzz-desktop: tts stage=player status=drained route_id={last_route_id}"); - first_append = true; - } - // Preprocess text. let text = preprocess_for_tts(&raw_text); if text.is_empty() { @@ -846,7 +814,12 @@ fn tts_worker( silence_buf_len, player.empty(), ) { - if !append_audio(prepared, route_id, speaker_pubkey.as_deref()) { + if !append_audio( + prepared, + route_id, + speaker_pubkey.as_deref(), + speaker_generation, + ) { first_append = true; synthesis_outcome = "cancelled"; break 'playback_chunks; @@ -872,7 +845,12 @@ fn tts_worker( if let Some(prepared) = playback_audio.finish(&mut first_append, silence_buf_len, player.empty()) { - if !append_audio(prepared, route_id, speaker_pubkey.as_deref()) { + if !append_audio( + prepared, + route_id, + speaker_pubkey.as_deref(), + speaker_generation, + ) { first_append = true; synthesis_outcome = "cancelled"; break 'playback_chunks; diff --git a/desktop/src-tauri/src/huddle/tts_pipeline_controls.rs b/desktop/src-tauri/src/huddle/tts_pipeline_controls.rs new file mode 100644 index 00000000000..0ee472f0fd1 --- /dev/null +++ b/desktop/src-tauri/src/huddle/tts_pipeline_controls.rs @@ -0,0 +1,100 @@ +use super::*; + +impl TtsPipeline { + /// Queue `text` for TTS synthesis and playback. + /// + /// Non-blocking. Returns `Err` if the queue is full (bounded at + /// `TEXT_QUEUE_DEPTH`) — caller may log and discard. + pub fn speak(&self, text: String) -> Result<(), String> { + self.text_tx + .try_send(QueuedText { + generation: self.voice_generation.load(Ordering::Acquire), + route_id: 0, + speaker_pubkey: None, + speaker_generation: 0, + voice_reference: None, + text, + }) + .map_err(|e| { + eprintln!("buzz-desktop: TTS queue saturated, dropping message: {e}"); + format!("TTS queue full, dropping: {e}") + }) + } + + /// Clone the bounded queue sender so callers can apply backpressure without + /// holding the huddle mutex. Disabling TTS drops the receiver and unblocks + /// any waiting sender while the shared cancellation flag stops playback. + pub(crate) fn text_sender(&self) -> TtsTextSender { + TtsTextSender { + text_tx: self.text_tx.clone(), + generation: self.voice_generation.load(Ordering::Acquire), + speaker_generations: Arc::clone(&self.speaker_generations), + } + } + + /// Invalidate speech queued for one agent and cancel the player only when + /// that same agent currently owns it. + pub(crate) fn cancel_speaker(&self, speaker_pubkey: &str) { + request_speaker_cancel( + &self.speaker_generations, + &self.active_speaker, + &self.speaker_cancel, + speaker_pubkey, + ); + } + + /// Cancel exactly the speaker utterance currently owning playback. + /// + /// The speaker generation is advanced while ownership is locked, so a + /// stale Stop click cannot cancel a later utterance that starts after the + /// observed one drains. + pub(crate) fn cancel_active_speaker(&self, expected_speaker_pubkey: &str) -> bool { + request_active_speaker_cancel( + &self.speaker_generations, + &self.active_speaker, + &self.speaker_cancel, + &self.playback_probe, + expected_speaker_pubkey, + ) + } + + /// Select a bundled Pocket voice for subsequent speech. + /// + /// Current playback and queued text are cancelled immediately so content + /// cannot continue in the old voice. The worker keeps its warmed inference + /// engine and reloads only the reference style before the next utterance. + pub fn select_voice(&self, voice: &str) -> Option> { + let acknowledged = begin_voice_change( + &self.voice, + &self.voice_generation, + &self.voice_cancel, + &self.voice_change_ack, + voice, + ); + if acknowledged.is_some() { + eprintln!("buzz-desktop: tts stage=cancellation reason=voice_switch route_id=0"); + } + acknowledged + } + + /// Reconcile the voice of a pipeline that has not been published yet. + /// + /// No caller can enqueue text before publication, so raising the shared + /// cancellation flag here would create a race that could discard the first + /// message queued immediately after installation. + pub(crate) fn select_voice_before_publish(&self, voice: &str) { + *self.voice.lock().unwrap_or_else(|error| error.into_inner()) = voice.to_string(); + } + + /// Signal the worker thread to stop. + pub fn shutdown(&self) { + eprintln!("buzz-desktop: tts stage=cancellation reason=shutdown route_id=0"); + self.shutdown.store(true, Ordering::Release); + } + + /// Returns `true` if the worker thread has exited (init failure, crash, or normal exit). + /// Used by hot-start to detect dead pipelines and clear them for retry. + pub fn is_finished(&self) -> bool { + self.thread.as_ref().is_none_or(|h| h.is_finished()) + } +} diff --git a/desktop/src-tauri/src/huddle/tts_speaker_cancellation.rs b/desktop/src-tauri/src/huddle/tts_speaker_cancellation.rs new file mode 100644 index 00000000000..4b9c2824f73 --- /dev/null +++ b/desktop/src-tauri/src/huddle/tts_speaker_cancellation.rs @@ -0,0 +1,177 @@ +use super::*; + +pub(super) struct TtsMonitorState { + pub(super) player: Arc, + pub(super) cancel: Arc, + pub(super) voice_cancel: Arc, + pub(super) tts_active: Arc, + pub(super) stop: Arc, + pub(super) player_ops: Arc>, + pub(super) activity_frames: Arc>>, + pub(super) active_speaker: ActiveSpeaker, + pub(super) speaker_cancel: SpeakerCancellation, + pub(super) activity_app: Option, +} + +pub(super) fn spawn_tts_monitor(state: TtsMonitorState) -> std::io::Result> { + thread::Builder::new() + .name("tts-barge-in-monitor".into()) + .spawn(move || { + let mut last_activity_pubkey: Option = None; + let mut next_activity_tick = Instant::now(); + while !state.stop.load(Ordering::Acquire) { + if state.cancel.load(Ordering::Acquire) + || state.voice_cancel.load(Ordering::Acquire) + { + let _ops = lock_player_ops(&state.player_ops); + if state.cancel.load(Ordering::Acquire) + || state.voice_cancel.load(Ordering::Acquire) + { + state.player.clear(); + state.player.play(); + state + .active_speaker + .lock() + .unwrap_or_else(|error| error.into_inner()) + .take(); + state.tts_active.store(false, Ordering::Release); + } + } + silence_cancelled_speaker( + &state.speaker_cancel, + &state.active_speaker, + &state.player, + &state.player_ops, + &state.tts_active, + ); + if let Some(ref app) = state.activity_app { + if state.tts_active.load(Ordering::Acquire) { + let now = Instant::now(); + if now >= next_activity_tick { + let frame = state + .activity_frames + .lock() + .unwrap_or_else(|error| error.into_inner()) + .pop_front(); + if let Some(frame) = frame { + use tauri::Emitter; + let _ = app.emit( + "huddle-tts-speaker-level", + TtsSpeakerActivityPayload { + pubkey: Some(frame.pubkey.clone()), + level: frame.level, + }, + ); + last_activity_pubkey = Some(frame.pubkey); + } + next_activity_tick = now + SPEAKER_ACTIVITY_TICK; + } + } else { + let had_activity = last_activity_pubkey.take().is_some(); + state + .activity_frames + .lock() + .unwrap_or_else(|error| error.into_inner()) + .clear(); + if had_activity { + use tauri::Emitter; + let _ = app.emit( + "huddle-tts-speaker-level", + TtsSpeakerActivityPayload { + pubkey: None, + level: 0.0, + }, + ); + } + next_activity_tick = Instant::now(); + } + } + thread::sleep(MONITOR_TICK); + } + }) +} + +pub(super) fn silence_cancelled_speaker( + cancellation: &SpeakerCancellation, + active_speaker: &ActiveSpeaker, + player: &rodio::Player, + player_ops: &Mutex<()>, + tts_active: &AtomicBool, +) { + let Some(cancelled) = cancellation + .lock() + .unwrap_or_else(|error| error.into_inner()) + .clone() + else { + return; + }; + let _ops = lock_player_ops(player_ops); + if take_cancelled_active_speaker(&cancelled, active_speaker) { + player.clear(); + player.play(); + tts_active.store(false, Ordering::Release); + } +} + +fn take_cancelled_active_speaker(cancelled: &str, active_speaker: &ActiveSpeaker) -> bool { + let mut active = active_speaker + .lock() + .unwrap_or_else(|error| error.into_inner()); + if !active + .as_deref() + .is_some_and(|speaker| speaker.eq_ignore_ascii_case(cancelled)) + { + return false; + } + active.take(); + true +} + +pub(super) fn consume_speaker_cancel( + cancellation: &SpeakerCancellation, + active_speaker: &ActiveSpeaker, + generations: &SpeakerGenerations, + tts_active: &AtomicBool, + text_state: CancelTextState<'_>, + player: Option<(&rodio::Player, &Mutex<()>)>, +) -> bool { + let Some(cancelled) = cancellation + .lock() + .unwrap_or_else(|error| error.into_inner()) + .take() + else { + return false; + }; + let (text_rx, deferred_text, current_text) = text_state; + retain_current_speaker_text(generations, deferred_text, current_text, text_rx); + let mut cleared_player = false; + if let Some((player, player_ops)) = player { + let _ops = lock_player_ops(player_ops); + if take_cancelled_active_speaker(&cancelled, active_speaker) { + player.clear(); + player.play(); + tts_active.store(false, Ordering::Release); + cleared_player = true; + } + } + // The monitor may already have cleared the cancelled speaker while the + // worker was blocked. If another speaker has since claimed the player, + // preserve that speaker's activity flag and lead-in state. + cleared_player +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn stale_targeted_cancel_does_not_release_the_next_speaker() { + let active_speaker = Arc::new(Mutex::new(Some("bob".to_string()))); + + assert!(!take_cancelled_active_speaker("alice", &active_speaker)); + assert_eq!( + active_speaker.lock().expect("active speaker").as_deref(), + Some("bob") + ); + } +} diff --git a/desktop/src-tauri/src/huddle/tts_voice_selection_tests.rs b/desktop/src-tauri/src/huddle/tts_voice_selection_tests.rs index 044b1acf1e8..bff5ab4f76b 100644 --- a/desktop/src-tauri/src/huddle/tts_voice_selection_tests.rs +++ b/desktop/src-tauri/src/huddle/tts_voice_selection_tests.rs @@ -19,6 +19,10 @@ fn inert_pipeline(cancel: Arc) -> TtsPipeline { voice_cancel: Arc::new(AtomicBool::new(false)), voice: Arc::new(std::sync::Mutex::new("reference_sample".to_string())), voice_generation: Arc::new(AtomicU64::new(1)), + speaker_generations: Arc::new(std::sync::Mutex::new(HashMap::new())), + active_speaker: Arc::new(std::sync::Mutex::new(None)), + speaker_cancel: Arc::new(std::sync::Mutex::new(None)), + playback_probe: PlaybackProbe::new(), voice_change_ack: Arc::new(std::sync::Mutex::new(None)), thread: Some(thread), } @@ -168,6 +172,7 @@ fn an_in_hand_post_change_message_survives_cancellation() { generation: voice_generation.load(Ordering::Acquire), route_id: 1, speaker_pubkey: None, + speaker_generation: 0, voice_reference: None, text: "new message".to_string(), }) @@ -181,6 +186,7 @@ fn an_in_hand_post_change_message_survives_cancellation() { generation: 1, route_id: 2, speaker_pubkey: None, + speaker_generation: 0, voice_reference: None, text: "old message".to_string(), }, @@ -188,6 +194,7 @@ fn an_in_hand_post_change_message_survives_cancellation() { generation: voice_generation.load(Ordering::Acquire), route_id: 3, speaker_pubkey: None, + speaker_generation: 0, voice_reference: None, text: "later new message".to_string(), }, @@ -246,6 +253,7 @@ fn superseding_voice_change_removes_earlier_deferred_messages() { generation: voice_generation.load(Ordering::Acquire), route_id: 4, speaker_pubkey: None, + speaker_generation: 0, voice_reference: None, text: "message for Eve".to_string(), }); @@ -294,6 +302,7 @@ fn barge_in_clears_deferred_voice_change_messages() { generation: 2, route_id: 5, speaker_pubkey: None, + speaker_generation: 0, voice_reference: None, text: "deferred message".to_string(), }]); @@ -337,6 +346,7 @@ fn barge_in_during_a_voice_change_clears_post_change_messages() { generation: voice_generation.load(Ordering::Acquire), route_id: 6, speaker_pubkey: None, + speaker_generation: 0, voice_reference: None, text: "post-change message".to_string(), }); @@ -365,6 +375,7 @@ fn a_sender_captured_before_voice_change_is_stale_even_if_it_sends_after_drain() let old_sender = TtsTextSender { text_tx, generation: voice_generation.load(Ordering::Acquire), + speaker_generations: Arc::new(std::sync::Mutex::new(HashMap::new())), }; let shutdown = AtomicBool::new(false); let active = AtomicBool::new(true); @@ -392,6 +403,7 @@ fn a_sender_captured_before_voice_change_is_stale_even_if_it_sends_after_drain() .send( 7, "agent".to_string(), + 0, "reference_sample".to_string(), "late old message".to_string(), ) diff --git a/desktop/src-tauri/src/huddle/tts_voice_transition.rs b/desktop/src-tauri/src/huddle/tts_voice_transition.rs index 3a655537561..a60d3506ffa 100644 --- a/desktop/src-tauri/src/huddle/tts_voice_transition.rs +++ b/desktop/src-tauri/src/huddle/tts_voice_transition.rs @@ -1,5 +1,6 @@ use std::{ collections::{HashMap, VecDeque}, + fmt, path::Path, sync::{ atomic::{AtomicBool, AtomicU64, Ordering}, @@ -19,6 +20,9 @@ pub(super) struct PendingVoiceChange { pub(super) type VoiceChangeAck = Arc>>; pub(super) type WorkerVoiceState = (Arc>, Arc, VoiceChangeAck); pub(super) type WorkerCancelSignals = (Arc, Arc); +pub(super) type SpeakerGenerations = Arc>>; +pub(super) type ActiveSpeaker = Arc>>; +pub(super) type SpeakerCancellation = Arc>>; pub(super) type CancelTextState<'a> = ( &'a mpsc::Receiver, &'a mut VecDeque, @@ -26,11 +30,73 @@ pub(super) type CancelTextState<'a> = ( ); pub(super) type CancelSignals<'a> = (&'a AtomicBool, &'a AtomicBool); +#[derive(Clone)] +pub(super) struct PlaybackProbe { + player: Arc>>>, + pub(super) player_ops: Arc>, + synthesis_in_flight: Arc, +} + +pub(super) struct SynthesisFlightGuard { + playback_probe: PlaybackProbe, +} + +impl Drop for SynthesisFlightGuard { + fn drop(&mut self) { + self.playback_probe.set_synthesis_in_flight(false); + } +} + +impl PlaybackProbe { + pub(super) fn new() -> Self { + Self { + player: Arc::new(Mutex::new(None)), + player_ops: Arc::new(Mutex::new(())), + synthesis_in_flight: Arc::new(AtomicBool::new(false)), + } + } + + pub(super) fn install(&self, player: Arc) { + self.player + .lock() + .unwrap_or_else(|error| error.into_inner()) + .replace(player); + } + + pub(super) fn set_synthesis_in_flight(&self, in_flight: bool) { + let _ops = super::lock_player_ops(&self.player_ops); + self.synthesis_in_flight.store(in_flight, Ordering::Release); + } + + pub(super) fn begin_synthesis(&self) -> SynthesisFlightGuard { + self.set_synthesis_in_flight(true); + SynthesisFlightGuard { + playback_probe: self.clone(), + } + } + + fn player(&self) -> Option> { + self.player + .lock() + .unwrap_or_else(|error| error.into_inner()) + .clone() + } +} + +impl fmt::Debug for PlaybackProbe { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("PlaybackProbe") + .finish_non_exhaustive() + } +} + #[derive(Debug)] pub(super) struct QueuedText { pub(super) generation: u64, pub(super) route_id: u64, pub(super) speaker_pubkey: Option, + pub(super) speaker_generation: u64, pub(super) voice_reference: Option, pub(super) text: String, } @@ -39,6 +105,7 @@ pub(super) struct QueuedText { pub(crate) struct TtsTextSender { pub(super) text_tx: SyncSender, pub(super) generation: u64, + pub(super) speaker_generations: SpeakerGenerations, } impl TtsTextSender { @@ -46,6 +113,7 @@ impl TtsTextSender { &self, route_id: u64, speaker_pubkey: String, + speaker_generation: u64, voice_reference: String, text: String, ) -> Result<(), String> { @@ -54,11 +122,156 @@ impl TtsTextSender { generation: self.generation, route_id, speaker_pubkey: Some(speaker_pubkey), + speaker_generation, voice_reference: Some(voice_reference), text, }) .map_err(|error| error.to_string()) } + + pub(crate) fn speaker_generation(&self, speaker_pubkey: &str) -> u64 { + current_speaker_generation(&self.speaker_generations, speaker_pubkey) + } +} + +pub(super) fn current_speaker_generation( + generations: &SpeakerGenerations, + speaker_pubkey: &str, +) -> u64 { + generations + .lock() + .unwrap_or_else(|error| error.into_inner()) + .get(&speaker_pubkey.to_ascii_lowercase()) + .copied() + .unwrap_or(0) +} + +pub(super) fn advance_speaker_generation( + generations: &SpeakerGenerations, + speaker_pubkey: &str, +) -> u64 { + let mut generations = generations + .lock() + .unwrap_or_else(|error| error.into_inner()); + let generation = generations + .entry(speaker_pubkey.to_ascii_lowercase()) + .or_default(); + *generation = generation.saturating_add(1); + *generation +} + +pub(super) fn queued_speaker_is_current( + generations: &SpeakerGenerations, + queued: &QueuedText, +) -> bool { + queued + .speaker_pubkey + .as_deref() + .is_none_or(|speaker_pubkey| { + current_speaker_generation(generations, speaker_pubkey) == queued.speaker_generation + }) +} + +pub(super) fn request_speaker_cancel( + generations: &SpeakerGenerations, + active_speaker: &ActiveSpeaker, + cancellation: &SpeakerCancellation, + speaker_pubkey: &str, +) { + advance_speaker_generation(generations, speaker_pubkey); + let owns_player = active_speaker + .lock() + .unwrap_or_else(|error| error.into_inner()) + .as_deref() + .is_some_and(|active| active.eq_ignore_ascii_case(speaker_pubkey)); + if owns_player { + cancellation + .lock() + .unwrap_or_else(|error| error.into_inner()) + .replace(speaker_pubkey.to_ascii_lowercase()); + } +} + +pub(super) fn request_active_speaker_cancel( + generations: &SpeakerGenerations, + active_speaker: &ActiveSpeaker, + cancellation: &SpeakerCancellation, + playback_probe: &PlaybackProbe, + expected_speaker_pubkey: &str, +) -> bool { + let Some(player) = playback_probe.player() else { + return false; + }; + let _ops = super::lock_player_ops(&playback_probe.player_ops); + let playback_live = + !player.empty() || playback_probe.synthesis_in_flight.load(Ordering::Acquire); + request_active_speaker_cancel_while_locked( + generations, + active_speaker, + cancellation, + playback_live, + expected_speaker_pubkey, + ) +} + +fn request_active_speaker_cancel_while_locked( + generations: &SpeakerGenerations, + active_speaker: &ActiveSpeaker, + cancellation: &SpeakerCancellation, + playback_live: bool, + expected_speaker_pubkey: &str, +) -> bool { + if !playback_live { + return false; + } + let active = active_speaker + .lock() + .unwrap_or_else(|error| error.into_inner()); + let Some(speaker_pubkey) = active.as_deref() else { + return false; + }; + if !speaker_pubkey.eq_ignore_ascii_case(expected_speaker_pubkey) { + return false; + } + + // Keep ownership locked until the generation and cancellation request are + // committed. The drain path takes the same lock, so the request is bound + // to the utterance the Stop action actually observed. + let mut cancellation = cancellation + .lock() + .unwrap_or_else(|error| error.into_inner()); + if cancellation + .as_deref() + .is_some_and(|pending| pending.eq_ignore_ascii_case(speaker_pubkey)) + { + return false; + } + advance_speaker_generation(generations, speaker_pubkey); + cancellation.replace(speaker_pubkey.to_ascii_lowercase()); + true +} + +pub(super) fn retain_current_speaker_text( + generations: &SpeakerGenerations, + deferred_text: &mut VecDeque, + current_text: &mut Option, + text_rx: &mpsc::Receiver, +) { + deferred_text.retain(|text| queued_speaker_is_current(generations, text)); + if let Some(text) = current_text.take() { + if queued_speaker_is_current(generations, &text) { + deferred_text.push_front(text); + } else { + log_cancelled_route(text.route_id, "speaker_removed"); + } + } + while let Ok(text) = text_rx.try_recv() { + if queued_speaker_is_current(generations, &text) { + deferred_text.push_back(text); + } else { + log_cancelled_route(text.route_id, "speaker_removed"); + } + } } pub(super) fn has_pending_voice_change(voice_change_ack: &VoiceChangeAck) -> bool { @@ -258,3 +471,227 @@ pub(super) fn retain_cancelled_text( fn log_cancelled_route(route_id: u64, reason: &str) { eprintln!("buzz-desktop: tts stage=queue status=dropped reason={reason} route_id={route_id}"); } + +#[cfg(test)] +mod speaker_generation_tests { + use super::*; + + fn playback_probe(playback_live: bool) -> PlaybackProbe { + let channels = std::num::NonZero::new(1).expect("non-zero channels"); + let sample_rate = std::num::NonZero::new(24_000).expect("non-zero sample rate"); + let (mixer, _mixer_source) = rodio::mixer::mixer(channels, sample_rate); + let player = Arc::new(rodio::Player::connect_new(&mixer)); + if playback_live { + player.append(rodio::buffer::SamplesBuffer::new( + channels, + sample_rate, + vec![0.0; 24_000], + )); + } + let probe = PlaybackProbe::new(); + probe.install(player); + probe + } + + fn queued_speech(speaker_pubkey: &str, speaker_generation: u64) -> QueuedText { + QueuedText { + generation: 1, + route_id: 1, + speaker_pubkey: Some(speaker_pubkey.to_string()), + speaker_generation, + voice_reference: Some("pocket:mary".to_string()), + text: "Hello".to_string(), + } + } + + #[test] + fn removing_a_speaker_invalidates_only_that_speakers_queued_text() { + let generations = Arc::new(Mutex::new(HashMap::new())); + let alice = queued_speech("ALICE", current_speaker_generation(&generations, "alice")); + let bob = queued_speech("bob", current_speaker_generation(&generations, "bob")); + + advance_speaker_generation(&generations, "alice"); + + assert!(!queued_speaker_is_current(&generations, &alice)); + assert!(queued_speaker_is_current(&generations, &bob)); + + let rejoined_alice = + queued_speech("alice", current_speaker_generation(&generations, "alice")); + assert!(queued_speaker_is_current(&generations, &rejoined_alice)); + } + + #[test] + fn removing_a_silent_speaker_does_not_cancel_the_active_speaker() { + let generations = Arc::new(Mutex::new(HashMap::new())); + let active_speaker = Arc::new(Mutex::new(Some("alice".to_string()))); + let cancellation = Arc::new(Mutex::new(None)); + + request_speaker_cancel(&generations, &active_speaker, &cancellation, "bob"); + + assert!(cancellation.lock().expect("cancellation").is_none()); + assert_eq!( + active_speaker.lock().expect("active speaker").as_deref(), + Some("alice") + ); + } + + #[test] + fn targeted_cancellation_preserves_other_speakers_queue_entries() { + let generations = Arc::new(Mutex::new(HashMap::new())); + let active_speaker = Arc::new(Mutex::new(Some("alice".to_string()))); + let cancellation = Arc::new(Mutex::new(None)); + let alice = queued_speech("alice", 0); + let bob = queued_speech("bob", 0); + let (_text_tx, text_rx) = mpsc::sync_channel(1); + let mut deferred = VecDeque::from([alice, bob]); + let mut current = None; + + request_speaker_cancel(&generations, &active_speaker, &cancellation, "alice"); + retain_current_speaker_text(&generations, &mut deferred, &mut current, &text_rx); + + assert_eq!(deferred.len(), 1); + assert_eq!(deferred[0].speaker_pubkey.as_deref(), Some("bob")); + } + + #[test] + fn stop_request_is_bound_to_the_observed_speaker_generation() { + let generations = Arc::new(Mutex::new(HashMap::new())); + let active_speaker = Arc::new(Mutex::new(Some("alice".to_string()))); + let cancellation = Arc::new(Mutex::new(None)); + + assert!(request_active_speaker_cancel( + &generations, + &active_speaker, + &cancellation, + &playback_probe(true), + "alice", + )); + assert_eq!(current_speaker_generation(&generations, "alice"), 1); + assert_eq!( + cancellation.lock().expect("cancellation").as_deref(), + Some("alice") + ); + + active_speaker.lock().expect("active speaker").take(); + cancellation.lock().expect("cancellation").take(); + assert!(!request_active_speaker_cancel( + &generations, + &active_speaker, + &cancellation, + &playback_probe(true), + "alice", + )); + + let next_utterance = + queued_speech("alice", current_speaker_generation(&generations, "alice")); + assert!(queued_speaker_is_current(&generations, &next_utterance)); + assert!(cancellation.lock().expect("cancellation").is_none()); + } + + #[test] + fn stop_request_does_not_cancel_a_different_active_speaker() { + let generations = Arc::new(Mutex::new(HashMap::new())); + let active_speaker = Arc::new(Mutex::new(Some("bob".to_string()))); + let cancellation = Arc::new(Mutex::new(None)); + + assert!(!request_active_speaker_cancel( + &generations, + &active_speaker, + &cancellation, + &playback_probe(true), + "alice", + )); + assert_eq!(current_speaker_generation(&generations, "alice"), 0); + assert_eq!(current_speaker_generation(&generations, "bob"), 0); + assert!(cancellation.lock().expect("cancellation").is_none()); + assert_eq!( + active_speaker.lock().expect("active speaker").as_deref(), + Some("bob"), + ); + } + + #[test] + fn stop_request_during_empty_synthesis_gap_cancels_in_flight_speech() { + let generations = Arc::new(Mutex::new(HashMap::new())); + let active_speaker = Arc::new(Mutex::new(Some("alice".to_string()))); + let cancellation = Arc::new(Mutex::new(None)); + let next_chunk = queued_speech("alice", 0); + let probe = playback_probe(false); + let _synthesis_flight = probe.begin_synthesis(); + + assert!(request_active_speaker_cancel( + &generations, + &active_speaker, + &cancellation, + &probe, + "alice", + )); + + assert_eq!(current_speaker_generation(&generations, "alice"), 1); + assert!(!queued_speaker_is_current(&generations, &next_chunk)); + assert_eq!( + cancellation.lock().expect("cancellation").as_deref(), + Some("alice"), + ); + } + + #[test] + fn repeated_stop_for_same_in_flight_utterance_is_idempotent() { + let generations = Arc::new(Mutex::new(HashMap::new())); + let active_speaker = Arc::new(Mutex::new(Some("alice".to_string()))); + let cancellation = Arc::new(Mutex::new(None)); + let probe = playback_probe(false); + let _synthesis_flight = probe.begin_synthesis(); + + assert!(request_active_speaker_cancel( + &generations, + &active_speaker, + &cancellation, + &probe, + "alice", + )); + let speech_queued_after_first_stop = queued_speech("alice", 1); + + assert!(!request_active_speaker_cancel( + &generations, + &active_speaker, + &cancellation, + &probe, + "alice", + )); + + assert_eq!(current_speaker_generation(&generations, "alice"), 1); + assert!(queued_speaker_is_current( + &generations, + &speech_queued_after_first_stop, + )); + assert_eq!( + cancellation.lock().expect("cancellation").as_deref(), + Some("alice"), + ); + } + + #[test] + fn stop_request_after_playback_drains_preserves_queued_speech() { + let generations = Arc::new(Mutex::new(HashMap::new())); + let active_speaker = Arc::new(Mutex::new(Some("alice".to_string()))); + let cancellation = Arc::new(Mutex::new(None)); + let next_utterance = queued_speech("alice", 0); + + assert!(!request_active_speaker_cancel( + &generations, + &active_speaker, + &cancellation, + &playback_probe(false), + "alice", + )); + + assert_eq!(current_speaker_generation(&generations, "alice"), 0); + assert!(queued_speaker_is_current(&generations, &next_utterance)); + assert!(cancellation.lock().expect("cancellation").is_none()); + assert_eq!( + active_speaker.lock().expect("active speaker").as_deref(), + Some("alice"), + ); + } +} diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 0e5b38e8fe3..3fa8b4ef814 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -56,9 +56,10 @@ use huddle::reconnect::reconnect_huddle_audio; use huddle::{ add_agent_to_huddle, check_pipeline_hotstart, close_huddle_companion, confirm_huddle_active, download_voice_models, end_huddle, get_huddle_agent_pubkeys, get_huddle_state, - get_model_status, get_voice_input_mode, join_huddle, leave_huddle, open_huddle_window, - push_audio_pcm, set_huddle_transcription_enabled, set_tts_enabled, set_voice_input_mode, - speak_agent_message, start_huddle, start_stt_pipeline, HuddlePhase, + get_model_status, get_voice_input_mode, interrupt_huddle_speech, join_huddle, leave_huddle, + open_huddle_window, push_audio_pcm, remove_agent_from_huddle, set_huddle_manual_mic_unmuted, + set_huddle_transcription_enabled, set_tts_enabled, set_voice_input_mode, speak_agent_message, + start_huddle, start_stt_pipeline, HuddlePhase, }; use initial_window::*; use managed_agents::{ @@ -731,6 +732,7 @@ pub fn run() { fetch_media_bytes, copy_image_to_clipboard, copy_text_to_clipboard, + read_clipboard_text, fetch_snapshot_bytes, relay_requires_membership, list_relay_members, @@ -853,7 +855,9 @@ pub fn run() { huddle::agent_voice::set_huddle_agent_tts_enabled, huddle::agent_voice::set_huddle_agent_voice, speak_agent_message, + interrupt_huddle_speech, add_agent_to_huddle, + remove_agent_from_huddle, huddle::agents::sync_agents_to_active_huddle, check_pipeline_hotstart, confirm_huddle_active, @@ -861,6 +865,7 @@ pub fn run() { get_huddle_agent_pubkeys, set_voice_input_mode, get_voice_input_mode, + set_huddle_manual_mic_unmuted, list_audio_output_devices, set_audio_output_device, get_audio_output_device, diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 07705ee998d..9956f19b294 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -77,6 +77,9 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ "BUZZ_ACP_RESPOND_TO", "BUZZ_ACP_RESPOND_TO_ALLOWLIST", "BUZZ_ACP_AGENT_OWNER", + // Stable agent identity used for git attribution and private-conversation + // provenance must come from the managed-agent record, not user overrides. + "BUZZ_ACP_DISPLAY_NAME", // Remote lifetime/presence policy: user env must not disable the // desktop/provider-owned bounds while the saved record still promises them. "BUZZ_ACP_EXIT_AFTER_INACTIVITY", diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index c8f008836d7..a57676f0a97 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -106,31 +106,6 @@ pub fn nest_dir() -> Option { } } -/// Returns `true` iff `path` ends with the dev-nest directory name (`.buzz-dev`). -/// -/// Pure function — no globals — so it can be unit-tested without touching the -/// process-lifetime [`NEST_DIR`] `OnceLock`. -fn path_is_dev_nest(path: &std::path::Path) -> bool { - path.file_name() - .and_then(|n| n.to_str()) - .map(|n| n == NEST_DIR_DEV) - .unwrap_or(false) -} - -/// Returns `true` when the running binary is using the dev nest (`~/.buzz-dev`). -/// -/// This is `true` for all dev builds — `just staging` and `just dev` — because -/// [`init_nest_dir`] is called with `is_dev = true` when the Tauri app-data -/// directory starts with `"xyz.block.buzz.app.dev"`. -/// -/// Returns `false` when: -/// - The nest is the production nest (`~/.buzz`, signed DMG). -/// - [`init_nest_dir`] has not been called yet (unit tests, home dir -/// unresolvable) — the fallback path is always the prod nest. -pub fn nest_is_dev() -> bool { - nest_dir().map(|p| path_is_dev_nest(&p)).unwrap_or(false) -} - /// Creates the Buzz nest at `~/.buzz` if it doesn't already exist. /// /// Delegates to [`ensure_nest_at`] with the resolved nest directory. diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index 031b049a495..cbef171f6fd 100644 --- a/desktop/src-tauri/src/managed_agents/nest/tests.rs +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -923,39 +923,3 @@ fn refresh_skill_overwrites_on_version_bump() { "SKILL.md must be refreshed on version bump" ); } - -#[test] -fn test_path_is_dev_nest_dev_path_returns_true() { - let path = std::path::Path::new("/Users/someone/.buzz-dev"); - assert!( - path_is_dev_nest(path), - ".buzz-dev path must be identified as dev nest" - ); -} - -#[test] -fn test_path_is_dev_nest_prod_path_returns_false() { - let path = std::path::Path::new("/Users/someone/.buzz"); - assert!( - !path_is_dev_nest(path), - ".buzz path must not be identified as dev nest" - ); -} - -#[test] -fn test_path_is_dev_nest_unrelated_path_returns_false() { - let path = std::path::Path::new("/Users/someone/.buzz-staging"); - assert!( - !path_is_dev_nest(path), - "unrelated path must not be identified as dev nest" - ); -} - -#[test] -fn test_path_is_dev_nest_root_returns_false() { - let path = std::path::Path::new("/"); - assert!( - !path_is_dev_nest(path), - "root path must not be identified as dev nest" - ); -} diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 4041a4fd947..9fa9e0cce6a 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -22,7 +22,8 @@ pub(crate) use path::should_use_inherited; mod metadata; pub(crate) use metadata::{ - resolve_session_title, runtime_metadata_env_vars, SESSION_TITLE_ENV_VAR, + apply_agent_display_env, resolve_session_title, runtime_metadata_env_vars, + DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, }; mod stop; @@ -763,11 +764,10 @@ pub fn spawn_agent_child( // is display metadata only. The spawn-config snapshot records the same // resolve, so a rename raises the restart badge instead of leaving the // process stale. - if let Some(title) = resolve_session_title(record.display_name.as_deref(), &record.name) { - command.env(SESSION_TITLE_ENV_VAR, title); - } else { - command.env_remove(SESSION_TITLE_ENV_VAR); - } + apply_agent_display_env( + &mut command, + resolve_session_title(record.display_name.as_deref(), &record.name), + ); build_buzz_agent_provider_defaults(&mut command); if let Some(meta) = runtime_meta { for (key, value) in runtime_metadata_env_vars( diff --git a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs index 26d210e5c60..5aef424ea61 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs @@ -27,6 +27,23 @@ pub(crate) fn runtime_metadata_env_vars<'a>( /// Env var carrying the session title to the harness. Shared with /// `spawn_snapshot` so the restart badge records the same key the spawn writes. pub(crate) const SESSION_TITLE_ENV_VAR: &str = "BUZZ_ACP_SESSION_TITLE"; +/// Stable agent display name forwarded to the ACP tool surface for git +/// attribution and private-conversation provenance. +pub(crate) const DISPLAY_NAME_ENV_VAR: &str = "BUZZ_ACP_DISPLAY_NAME"; + +/// Apply the shared stable agent name to both session display metadata and +/// git attribution, clearing both keys when no usable name is available. +pub(crate) fn apply_agent_display_env(command: &mut std::process::Command, title: Option) { + if let Some(title) = title { + command + .env(SESSION_TITLE_ENV_VAR, &title) + .env(DISPLAY_NAME_ENV_VAR, title); + } else { + command + .env_remove(SESSION_TITLE_ENV_VAR) + .env_remove(DISPLAY_NAME_ENV_VAR); + } +} /// Resolve the session title for an agent: its `display_name` when it has one, /// otherwise its unique `name` handle. `None` when both are blank, so the diff --git a/desktop/src-tauri/src/tray_menu.rs b/desktop/src-tauri/src/tray_menu.rs index 6dcef0ecf78..3f9fe49fe08 100644 --- a/desktop/src-tauri/src/tray_menu.rs +++ b/desktop/src-tauri/src/tray_menu.rs @@ -213,7 +213,9 @@ struct TrayMenuState { pub enum TrayAction { NewChannel, OpenChannel { + #[serde(rename = "channelId")] channel_id: String, + #[serde(rename = "communityGeneration")] community_generation: u64, }, } @@ -614,6 +616,23 @@ pub fn update_tray_agent_activity( mod tests { use super::{requeue_actions, TrayAction, TrayActionQueue}; + #[test] + fn open_channel_action_serializes_with_frontend_field_names() { + let action = TrayAction::OpenChannel { + channel_id: "channel-123".into(), + community_generation: 7, + }; + + assert_eq!( + serde_json::to_value(action).expect("tray action should serialize"), + serde_json::json!({ + "kind": "openChannel", + "channelId": "channel-123", + "communityGeneration": 7, + }) + ); + } + #[test] fn stale_channel_actions_are_not_requeued_after_community_change() { let mut queue = TrayActionQueue { diff --git a/desktop/src-tauri/tauri.conf.json b/desktop/src-tauri/tauri.conf.json index 4bda55fd095..0d4417a754e 100644 --- a/desktop/src-tauri/tauri.conf.json +++ b/desktop/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "Buzz", - "version": "0.5.4", + "version": "0.5.5", "identifier": "xyz.block.buzz.app", "build": { "beforeDevCommand": { diff --git a/desktop/src/app/AppHuddleShell.tsx b/desktop/src/app/AppHuddleShell.tsx index 8370e8efd43..29dcd26cdfb 100644 --- a/desktop/src/app/AppHuddleShell.tsx +++ b/desktop/src/app/AppHuddleShell.tsx @@ -48,6 +48,13 @@ export function AppHuddleShell({ data-huddle-open={isDrawerOpen} data-huddle-window={isRoom} > +