diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dd18179ee4..d4826d985f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,15 +48,21 @@ jobs: - 'scripts/run-tests.sh' - 'justfile' desktop: + - 'scripts/check-file-sizes-core.mjs' + - 'scripts/check-file-sizes-core.test.mjs' - 'desktop/**' - '!desktop/src-tauri/**' - 'pnpm-lock.yaml' desktop-rust: - 'desktop/src-tauri/**' web: + - 'scripts/check-file-sizes-core.mjs' + - 'scripts/check-file-sizes-core.test.mjs' - 'web/**' - 'pnpm-lock.yaml' mobile: + - 'scripts/check-file-sizes-core.mjs' + - 'scripts/check-file-sizes-core.test.mjs' - 'mobile/**' - 'scripts/mobile-release.sh' - 'scripts/mobile-worktree-overrides.sh' @@ -76,6 +82,8 @@ jobs: scripts/test-mobile-release-candidate-publisher.sh - name: Mobile worktree identity contract run: scripts/test-mobile-worktree-overrides.sh + - name: File size ratchet unit tests + run: node --test scripts/check-file-sizes-core.test.mjs rust-lint: name: Rust Lint @@ -130,6 +138,8 @@ jobs: contents: read steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + fetch-depth: 2 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - uses: rui314/setup-mold@9c9c13bf4c3f1adef0cc596abc155580bcb04444 # v1 - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 @@ -213,7 +223,7 @@ jobs: desktop-smoke-e2e: name: Desktop Smoke E2E (${{ matrix.shard }}) runs-on: ubuntu-latest - timeout-minutes: 20 + timeout-minutes: 30 needs: [changes] if: github.event_name == 'push' || needs.changes.outputs.desktop == 'true' || needs.changes.outputs.desktop-rust == 'true' || needs.changes.outputs.rust == 'true' strategy: @@ -751,6 +761,8 @@ jobs: contents: read steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + fetch-depth: 2 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - name: Get pnpm store directory id: pnpm-cache @@ -784,6 +796,8 @@ jobs: contents: read steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + fetch-depth: 2 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - name: Compute Hermit cache key id: hermit-bin-hash @@ -822,6 +836,8 @@ jobs: with: path: ~/.pub-cache key: pub-${{ runner.os }}-${{ hashFiles('mobile/pubspec.lock') }} + - name: File size ratchet + run: node mobile/scripts/check-file-sizes.mjs - name: Format check run: cd mobile && dart format --output=none --set-exit-if-changed . - name: Analyze diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 53ea0f11c8..db0aea637f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -98,6 +98,37 @@ Hermit pins Rust, `just`, Node, pnpm, and other tools to the versions in upfront. If you don't use Hermit, ensure your toolchain meets the minimum versions in the table above. +#### Linux: Tauri system libraries + +Hermit pins language toolchains, not system libraries. On Linux, the desktop +app's Rust crates link against GTK and WebKitGTK, so `just ci` (and any +`just desktop-tauri-*` recipe) needs these installed system-wide first. On +Debian/Ubuntu: + +```bash +sudo apt-get install -y --no-install-recommends \ + build-essential curl file libasound2-dev libayatana-appindicator3-dev \ + libgtk-3-dev librsvg2-dev libssl-dev libwebkit2gtk-4.1-dev libxdo-dev \ + patchelf wget +``` + +This is the same list CI installs (see `.github/workflows/ci.yml`), so matching +it locally keeps your results comparable to CI. Other distributions ship these +under different package names — see the +[Tauri prerequisites](https://tauri.app/start/prerequisites/) for the +equivalents. + +Without them, `just ci` fails partway through `just check` with a pkg-config +error such as: + +``` +The system library `gdk-pixbuf-2.0` required by crate `gdk-pixbuf-sys` was not found. +``` + +If you're only touching the relay, CLI, or other server-side crates, you can +skip this and run the narrower recipes instead — `just fmt-check`, `just +clippy`, `just test-unit`, and `just test` need no GTK. + ### First-Time Setup ```bash diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index c0147baf1b..d629e6a037 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -20,6 +20,10 @@ use crate::usage::{TurnUsage, UsageTracker}; /// Lines exceeding this limit are rejected to prevent OOM from rogue agents. const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB +/// Env var that tells a goose ACP child not to start its cron scheduler. +/// Injected unconditionally by [`AcpClient::spawn`]; see the call site for why. +pub(crate) const GOOSE_SCHEDULER_DISABLED_ENV: &str = "GOOSE_ACP_SCHEDULER_DISABLED"; + /// An MCP server configuration passed to `session/new`. /// /// Corresponds to the `McpServerStdio` variant in the ACP schema. @@ -187,6 +191,17 @@ pub struct AcpClient { /// Other agents may leave this unset — readers must treat `None` as /// "no active run to steer into" and fall back to cancel+merge. active_run_id: Option, + /// Whether the agent advertised `_meta.steering.supported: true` in its + /// `initialize` response, meaning it implements the cross-adapter + /// [`ACP_STEER_METHOD`] extension. + /// + /// Set once by [`initialize`](Self::initialize); `false` for agents that + /// omit the key. This is the **only** gate on writing an + /// [`ACP_STEER_METHOD`] request. It must never be replaced by error-code + /// probing: codex-acp answers unrecognized extension methods with `{}` — + /// a JSON-RPC *success*, not `-32601` — which the main loop would read as + /// a delivered steer and drop the user's message from the queue. + steering_supported: bool, /// Per-turn channel for receiving goose-native non-cancelling steer /// requests from the main loop. Installed by /// [`install_steer_rx`](Self::install_steer_rx) at dispatch and @@ -344,6 +359,38 @@ pub(crate) fn build_codex_config_env( Ok(Some(serde_json::Value::Object(base).to_string())) } +/// goose's non-standard mid-turn steer method. Requires `expectedRunId`, so it +/// is only usable once a `session_info_update` has supplied +/// `_meta.goose.activeRunId`. Emitted by goose and buzz-agent only. +const GOOSE_STEER_METHOD: &str = "_goose/unstable/session/steer"; + +/// The cross-adapter mid-turn steer method, shipped by claude-agent-acp +/// (`src/acp-agent.ts:200`) and codex-acp (`src/AcpExtensions.ts:11`). +/// Params are `{sessionId, prompt}` — no run id — and the result is +/// `{outcome}`. Gated on [`AcpClient::steering_supported`]. +const ACP_STEER_METHOD: &str = "_session/steering"; + +/// `outcome` value meaning the steer was applied to the turn Buzz is waiting +/// on, which therefore keeps running. +const STEER_OUTCOME_INJECTED: &str = "injected"; + +/// `outcome` value meaning the turn Buzz was steering had already finished, so +/// the adapter began a fresh turn carrying the message. Still a delivery +/// success, but the awaited turn is over — see the steer-response arm for why +/// this must not renew the hard deadline. +const STEER_OUTCOME_STARTED_NEW_TURN: &str = "startedNewTurn"; + +/// Which wire method carried an in-flight steer request, recorded so the +/// response arm decodes the shape that method actually returns. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SteerTransport { + /// [`GOOSE_STEER_METHOD`] — any success result is a delivered steer. + Goose, + /// [`ACP_STEER_METHOD`] — success carries an `outcome` that must be + /// positively recognized before the steer counts as delivered. + AcpExtension, +} + fn build_client_capabilities() -> serde_json::Value { serde_json::json!({ // Signal to ACP adapters that Buzz can hand users to terminal-native @@ -460,6 +507,16 @@ impl AcpClient { cmd.env("CODEX_CONFIG", merged); } + // Buzz-managed agents must never execute the operator's personal cron + // schedule. A goose ACP child starts a scheduler over the shared + // `schedule.json`, so a pool of N children fires every scheduled job N + // times — under the wrong identity and racing standalone goose. + // + // Set last, and with no operator-wins escape hatch, so it beats both a + // conflicting persona `extra_env` entry and any inherited parent value. + // Agent builds that don't recognize the variable ignore it. + cmd.env(GOOSE_SCHEDULER_DISABLED_ENV, "true"); + // Spawn the agent in its own process group so SIGKILL doesn't propagate // to the harness's own process group on Unix. // tokio::process::Command::process_group is a stable tokio API (no extra imports needed). @@ -494,6 +551,7 @@ impl AcpClient { observer_agent_index: None, observer_context: ObserverContext::default(), active_run_id: None, + steering_supported: false, steer_rx: None, goose_usage: UsageTracker::default(), }) @@ -536,11 +594,20 @@ impl AcpClient { /// /// Must be called exactly once, before any other ACP method. /// The caller may inspect `agentCapabilities` in the returned value. + /// + /// Records `_meta.steering.supported` into + /// [`steering_supported`](Self::steering_supported) so the read loop's steer + /// arm can choose [`ACP_STEER_METHOD`] for adapters that implement it. + /// Parsed here rather than at each call site so no caller can forget it. pub async fn initialize(&mut self) -> Result { // Requesting version 2 is an intentional temporary pin — we are squatting // on ACP v2 ahead of the upstream ACP RFD. Revisit when that RFD merges. let params = build_initialize_params(); let result = self.send_request("initialize", params).await?; + self.steering_supported = result + .pointer("/_meta/steering/supported") + .and_then(|v| v.as_bool()) + .unwrap_or(false); tracing::debug!(target: "acp::init", "initialize response: {result}"); Ok(result) } @@ -778,6 +845,15 @@ impl AcpClient { self.active_run_id.as_deref() } + /// Whether the agent advertised the [`ACP_STEER_METHOD`] extension at + /// `initialize` time (`_meta.steering.supported`). + /// + /// The read loop's steer arm reads the field directly; this accessor exists + /// for the supervisor's post-initialize log line. + pub fn steering_supported(&self) -> bool { + self.steering_supported + } + /// Consume and return the per-turn usage record computed from the most /// recent `_goose/unstable/session/update` notification. /// @@ -1219,14 +1295,18 @@ impl AcpClient { // so the ack_tx oneshot is never leaked silently). let mut steer_rx = self.steer_rx.take(); - // Tracks the in-flight steer write: `(request_id, ack_tx)`. While - // `Some`, the steer arm is gated off so we don't stack writes, + // Tracks the in-flight steer write: `(request_id, transport, ack_tx)`. + // While `Some`, the steer arm is gated off so we don't stack writes, // and a response matching `id` is routed to the ack_tx instead - // of being treated as the prompt result. Drained on every return - // path with `PromptCompletedNeutral` so callers are never left - // hanging. - let mut pending_steer: Option<(u64, tokio::sync::oneshot::Sender)> = - None; + // of being treated as the prompt result. `transport` records which + // method was written so the response arm decodes the result shape + // that method actually returns. Drained on every return path with + // `PromptCompletedNeutral` so callers are never left hanging. + let mut pending_steer: Option<( + u64, + SteerTransport, + tokio::sync::oneshot::Sender, + )> = None; let now = Instant::now(); let mut idle_deadline = now + idle_timeout; @@ -1251,7 +1331,7 @@ impl AcpClient { // exists). Check the classified deadline here so a steady- // stream agent is still bounded. if Instant::now() >= next_deadline { - if let Some((_, ack_tx)) = pending_steer.take() { + if let Some((_, _, ack_tx)) = pending_steer.take() { // Prompt is timing out — release the withheld event via // PromptCompletedNeutral (no fallback signal: there is // no in-flight turn to signal once we return, and @@ -1286,39 +1366,64 @@ impl AcpClient { None => None, } }, if pending_steer.is_none() => { - // Selected: build steer params at write time using the - // lexical `session_id` and the freshest `active_run_id`. + // Selected: choose the steer transport and build its + // params at write time using the lexical `session_id` + // and the freshest `active_run_id`. // // `active_run_id` is updated by `session/update` // notifications inside this very loop; reading it here // (rather than snapshotting at dispatch) guarantees the // value matches what goose's run-id check will compare - // against. If it's `None`, no `session/update` has - // arrived yet so we cannot form a valid `expectedRunId` - // — ack `ExpectedRunIdMissing` and drop the request - // without writing anything. The main loop maps this to - // the universal cancel+merge `Steer` fallback. - match self.active_run_id.clone() { + // against. + // + // Transport precedence: + // Some(run_id) → GOOSE_STEER_METHOD. goose + // wins whenever a run id exists: `expectedRunId` is + // strictly more precise about *which* run is steered. + // None + steering_supported → ACP_STEER_METHOD, the + // cross-adapter extension (claude-agent-acp, + // codex-acp), which takes no run id. + // None + !steering_supported → write nothing and ack + // `ExpectedRunIdMissing`; the main loop maps this to + // the universal cancel+merge `Steer` fallback. + // + // The capability flag is the ONLY gate on writing + // ACP_STEER_METHOD. Probing an unknown method is unsafe: + // codex-acp answers unrecognized extension methods with + // `{}` — a JSON-RPC success — which would be read as a + // delivered steer and silently drop the user's message. + let prompt_block_refs: Vec<&str> = + req.prompt_blocks.iter().map(String::as_str).collect(); + let selected = match (&self.active_run_id, self.steering_supported) { + (Some(run_id), _) => Some(( + SteerTransport::Goose, + GOOSE_STEER_METHOD, + build_goose_steer_params(session_id, run_id, &prompt_block_refs), + )), + (None, true) => Some(( + SteerTransport::AcpExtension, + ACP_STEER_METHOD, + build_acp_steer_params(session_id, &prompt_block_refs), + )), + (None, false) => None, + }; + match selected { None => { tracing::warn!( - "goose-native steer: no active_run_id at write time \ - (no session/update seen yet) — falling back to cancel+merge" + "steer: no active_run_id and agent did not advertise \ + {ACP_STEER_METHOD} — falling back to cancel+merge" ); let _ = req.ack_tx.send(crate::pool::SteerAck::Err( crate::pool::SteerError::ExpectedRunIdMissing, )); } - Some(run_id) => { + Some((transport, method, params)) => { let id = self.next_id; self.next_id += 1; - let prompt_block_refs: Vec<&str> = - req.prompt_blocks.iter().map(String::as_str).collect(); - let params = - build_steer_params(session_id, &run_id, &prompt_block_refs); let msg = serde_json::json!({ "jsonrpc": "2.0", "id": id, - "method": "_goose/unstable/session/steer", + "method": method, "params": params, }); tracing::debug!( @@ -1328,11 +1433,11 @@ impl AcpClient { ); match self.write_ndjson(&msg).await { Ok(()) => { - pending_steer = Some((id, req.ack_tx)); + pending_steer = Some((id, transport, req.ack_tx)); } Err(e) => { tracing::warn!( - "goose-native steer write failed: {e} — releasing withheld event" + "steer write failed ({method}): {e} — releasing withheld event" ); let _ = req.ack_tx.send(crate::pool::SteerAck::Err( crate::pool::SteerError::Transport(e.to_string()), @@ -1351,7 +1456,7 @@ impl AcpClient { // would catch this anyway, but firing the deadline arm // here makes the wakeup immediate (no extra reader poll // round-trip when stdout is idle). - if let Some((_, ack_tx)) = pending_steer.take() { + if let Some((_, _, ack_tx)) = pending_steer.take() { let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); } if idle_fires_first { @@ -1375,13 +1480,13 @@ impl AcpClient { match read_result { None => { - if let Some((_, ack_tx)) = pending_steer.take() { + if let Some((_, _, ack_tx)) = pending_steer.take() { let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); } return Err(AcpError::AgentExited); } Some(Err(LinesCodecError::MaxLineLengthExceeded)) => { - if let Some((_, ack_tx)) = pending_steer.take() { + if let Some((_, _, ack_tx)) = pending_steer.take() { let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); } return Err(AcpError::Protocol( @@ -1389,7 +1494,7 @@ impl AcpClient { )); } Some(Err(e)) => { - if let Some((_, ack_tx)) = pending_steer.take() { + if let Some((_, _, ack_tx)) = pending_steer.take() { let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); } return Err(AcpError::Io(std::io::Error::other(e))); @@ -1432,13 +1537,14 @@ impl AcpClient { // share the `no method` guard. if let Some(id) = msg.get("id") { if msg.get("method").is_none() { - if let Some((steer_id, _)) = pending_steer.as_ref() { + if let Some((steer_id, _, _)) = pending_steer.as_ref() { if *id == serde_json::json!(*steer_id) { // Take the ack_tx out and route the // response. We do not return — keep // reading until the prompt response // arrives. - let (_, ack_tx) = pending_steer.take().expect("just checked"); + let (_, transport, ack_tx) = + pending_steer.take().expect("just checked"); let ack = if let Some(error) = msg.get("error") { let code = error .get("code") @@ -1449,16 +1555,83 @@ impl AcpClient { crate::pool::SteerError::AgentError { code, message }, ) } else { - let renew_now = Instant::now(); - let new_deadline = renew_now + max_duration; - if new_deadline > hard_deadline { - hard_deadline = new_deadline; - self.current_hard_deadline = Some(new_deadline); - tracing::info!( - "steer success: renewed hard deadline ({max_duration:?} from now)" - ); + // Success result. Whether it counts as + // a delivered steer — and whether the + // turn Buzz awaits is still running — + // depends on the transport. + let outcome = match transport { + // goose returns no outcome field; + // a success response means the + // steer landed in the live run. + SteerTransport::Goose => Some(STEER_OUTCOME_INJECTED), + // The outcome must be positively + // recognized. An unknown or absent + // value (codex-acp answers + // unrecognized ext methods with a + // bare `{}`) is a rejection, never + // a delivery — treating it as + // success would drop the event. + SteerTransport::AcpExtension => msg + .pointer("/result/outcome") + .and_then(|v| v.as_str()) + .filter(|o| { + *o == STEER_OUTCOME_INJECTED + || *o == STEER_OUTCOME_STARTED_NEW_TURN + }), + }; + match outcome { + Some(STEER_OUTCOME_STARTED_NEW_TURN) => { + // Delivered, but into a NEW + // turn: the one this read loop + // is awaiting had already + // finished. Renewing the hard + // deadline here would extend + // the clock on a settled turn, + // so leave it alone and let the + // prompt response land on its + // original budget. + tracing::info!( + "steer accepted as {STEER_OUTCOME_STARTED_NEW_TURN}: \ + awaited turn had ended — hard deadline not renewed" + ); + crate::pool::SteerAck::Success + } + Some(_) => { + let renew_now = Instant::now(); + let new_deadline = renew_now + max_duration; + if new_deadline > hard_deadline { + hard_deadline = new_deadline; + self.current_hard_deadline = Some(new_deadline); + tracing::info!( + "steer success: renewed hard deadline ({max_duration:?} from now)" + ); + } + crate::pool::SteerAck::Success + } + None => { + // Report the raw string when + // there is one, so logs read + // `failed` not `"failed"`; + // fall back to the JSON for a + // non-string value. + let reported = match msg.pointer("/result/outcome") + { + None => "".to_string(), + Some(serde_json::Value::String(s)) => s.clone(), + Some(other) => other.to_string(), + }; + tracing::warn!( + "steer rejected: {ACP_STEER_METHOD} returned \ + unrecognized outcome {reported} — releasing \ + withheld event for cancel+merge" + ); + crate::pool::SteerAck::Err( + crate::pool::SteerError::OutcomeRejected { + outcome: reported, + }, + ) + } } - crate::pool::SteerAck::Success }; let _ = ack_tx.send(ack); continue; @@ -1466,13 +1639,13 @@ impl AcpClient { } if *id == serde_json::json!(expected_id) { if let Some(error) = msg.get("error") { - if let Some((_, ack_tx)) = pending_steer.take() { + if let Some((_, _, ack_tx)) = pending_steer.take() { let _ = ack_tx .send(crate::pool::SteerAck::PromptCompletedNeutral); } return Err(agent_error_from_json(error)); } - if let Some((_, ack_tx)) = pending_steer.take() { + if let Some((_, _, ack_tx)) = pending_steer.take() { let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); } @@ -1531,7 +1704,10 @@ impl AcpClient { /// Takes `&mut self` (not `&self`) because some updates carry agent state /// the client must observe — notably goose's `session_info_update` with /// `_meta.goose.activeRunId`, which seeds [`active_run_id`](Self::active_run_id) - /// so callers can target `_goose/unstable/session/steer` at the correct run. + /// so the steer arm can target `_goose/unstable/session/steer` at the + /// correct run. Agents that never emit it (claude-agent-acp, codex-acp) + /// leave it `None` and are steered via `_session/steering` instead, which + /// needs no run id. fn handle_session_update(&mut self, msg: &serde_json::Value) -> bool { let update = &msg["params"]["update"]; let update_type = update @@ -1796,22 +1972,44 @@ fn build_prompt_params(session_id: &str, prompt_blocks: &[&str]) -> serde_json:: /// matches goose's *current* run (it advances on each `session/update`). /// See [`crate::pool::SteerRequest`] for why this is the read loop's job /// and not the main loop's. -fn build_steer_params( +fn build_goose_steer_params( session_id: &str, expected_run_id: &str, prompt_blocks: &[&str], ) -> serde_json::Value { - let blocks: Vec = prompt_blocks - .iter() - .map(|text| serde_json::json!({ "type": "text", "text": text })) - .collect(); serde_json::json!({ "sessionId": session_id, "expectedRunId": expected_run_id, - "prompt": blocks, + "prompt": steer_prompt_blocks(prompt_blocks), }) } +/// Build the params for an [`ACP_STEER_METHOD`] request. +/// +/// Wire shape: +/// ```json +/// { "sessionId": "...", "prompt": [{"type":"text","text":"..."}, ...] } +/// ``` +/// +/// Deliberately carries **no** `expectedRunId`: the cross-adapter method +/// steers whatever turn is currently running and neither claude-agent-acp nor +/// codex-acp emits a run id to target. +fn build_acp_steer_params(session_id: &str, prompt_blocks: &[&str]) -> serde_json::Value { + serde_json::json!({ + "sessionId": session_id, + "prompt": steer_prompt_blocks(prompt_blocks), + }) +} + +/// Render steer body strings as ACP `text` content blocks. Shared by both +/// steer transports so the prompt shape cannot drift between them. +fn steer_prompt_blocks(prompt_blocks: &[&str]) -> Vec { + prompt_blocks + .iter() + .map(|text| serde_json::json!({ "type": "text", "text": text })) + .collect() +} + /// Build a JSON-RPC permission response with `outcome: "selected"`. fn permission_response_selected(id: &serde_json::Value, option_id: &str) -> serde_json::Value { serde_json::json!({ @@ -2653,6 +2851,77 @@ mod tests { .expect("failed to spawn test script") } + /// Spawn a script that echoes the named env vars as the child observes + /// them, one per line. `` means the child did not receive the var. + async fn spawn_and_read_child_env( + vars: &[&str], + extra_env: &[(String, String)], + ) -> Vec { + let script = vars + .iter() + .map(|var| format!("printf '%s\\n' \"${{{var}:-}}\"")) + .collect::>() + .join("\n"); + let mut client = AcpClient::spawn("bash", &["-c".into(), script], extra_env, false) + .await + .expect("failed to spawn env probe script"); + let mut observed = Vec::with_capacity(vars.len()); + for var in vars { + observed.push( + client + .reader + .next() + .await + .unwrap_or_else(|| panic!("child produced no output for {var}")) + .expect("child stdout was not readable"), + ); + } + observed + } + + /// Every spawned agent must be told not to run the operator's cron + /// schedule, without the caller having to opt in. + #[tokio::test] + async fn spawn_injects_scheduler_disabled_env_by_default() { + let observed = spawn_and_read_child_env(&[GOOSE_SCHEDULER_DISABLED_ENV], &[]).await; + assert_eq!( + observed, + vec!["true"], + "{GOOSE_SCHEDULER_DISABLED_ENV} must be injected into every spawn" + ); + } + + /// Persona config must not be able to re-enable the scheduler: this is a + /// correctness invariant, not an operator-tunable default, so the + /// injection is set after (and therefore wins over) the `extra_env` loop. + /// + /// The control var pins that `extra_env` really did reach the child, so a + /// pass here means the conflicting entry lost the fight rather than + /// `extra_env` being dropped wholesale. + #[tokio::test] + async fn spawn_scheduler_disabled_env_overrides_conflicting_extra_env() { + let extra_env = vec![ + ( + GOOSE_SCHEDULER_DISABLED_ENV.to_string(), + "false".to_string(), + ), + ( + "BUZZ_ENV_PROBE_CONTROL".to_string(), + "delivered".to_string(), + ), + ]; + let observed = spawn_and_read_child_env( + &[GOOSE_SCHEDULER_DISABLED_ENV, "BUZZ_ENV_PROBE_CONTROL"], + &extra_env, + ) + .await; + assert_eq!( + observed, + vec!["true", "delivered"], + "a persona extra_env entry must not override {GOOSE_SCHEDULER_DISABLED_ENV}" + ); + } + #[tokio::test] async fn idle_timeout_fires_on_silent_process() { let mut client = spawn_script("sleep 10").await; @@ -3465,6 +3734,412 @@ mod tests { } } + // ── Cross-harness steer transport tests ─────────────────────────────── + // + // These cover the `_session/steering` transport added alongside the + // goose-native method: capability capture at `initialize`, write-time + // transport selection, and outcome decoding. Wire-shape assertions read + // the actual serialized request bytes via `capture_steer_request` rather + // than inferring the shape from response-id routing. + + /// Spawn a client whose script captures the first line written to its + /// stdin into `capture_path`, then emits `response` (already-serialized + /// JSON-RPC) and idles. + /// + /// The steer request is the first thing this read loop writes, so the + /// captured line IS the steer request bytes. + async fn spawn_steer_capture_script( + capture_path: &std::path::Path, + response: &str, + ) -> AcpClient { + let script = format!( + "read -r line; printf '%s' \"$line\" > {capture}; \ + printf '%s\\n' '{response}'; sleep 10", + capture = capture_path.display(), + response = response, + ); + spawn_script(&script).await + } + + /// Drive one steer through the read loop and return + /// `(captured_request_bytes, ack)`. + /// + /// `capture_path` may be absent afterwards when the arm wrote nothing — + /// callers assert on that. The read loop is expected to exit via a + /// timeout or EOF; the ack is what these tests care about. + async fn run_one_steer( + client: &mut AcpClient, + capture_path: &std::path::Path, + ) -> (Option, crate::pool::SteerAck) { + let (steer_tx, steer_rx) = tokio::sync::mpsc::channel::(1); + client.install_steer_rx(steer_rx); + + let (ack_tx, ack_rx) = tokio::sync::oneshot::channel::(); + let send_task = tokio::spawn(async move { + steer_tx + .send(crate::pool::SteerRequest { + prompt_blocks: vec!["steer body".into()], + ack_tx, + }) + .await + .expect("steer_tx send should succeed"); + }); + + let idle = std::time::Duration::from_millis(800); + let max_dur = std::time::Duration::from_secs(10); + let hard_deadline = tokio::time::Instant::now() + max_dur; + let _ = client + .read_until_response_with_idle_timeout("sess-test", 999, idle, hard_deadline, max_dur) + .await; + send_task.await.expect("send_task should complete"); + + let ack = ack_rx + .await + .expect("ack oneshot must have received a SteerAck"); + (std::fs::read_to_string(capture_path).ok(), ack) + } + + /// Unique temp path for one test's captured request bytes. + fn capture_path(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join("buzz-acp-steer-capture"); + std::fs::create_dir_all(&dir).expect("create capture dir"); + let path = dir.join(format!("{name}.json")); + let _ = std::fs::remove_file(&path); + path + } + + /// Mark a client as having advertised `_meta.steering.supported` without + /// running a real `initialize` handshake. The capability-parsing tests + /// cover the handshake itself. + fn set_steering_supported(client: &mut AcpClient) { + client.steering_supported = true; + } + + /// Run `initialize` against a script that replies with `init_result` as + /// the JSON-RPC result, and return the resulting `steering_supported`. + async fn steering_supported_after_initialize(init_result: &str) -> bool { + let script = format!( + "read -r _init; printf '%s\\n' '{{\"jsonrpc\":\"2.0\",\"id\":0,\"result\":{result}}}'; \ + sleep 5", + result = init_result, + ); + let mut client = spawn_script(&script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + client.steering_supported() + } + + /// Test 1a: an adapter advertising `_meta.steering.supported: true` + /// (claude-agent-acp `src/acp-agent.ts:1444`, codex-acp + /// `src/CodexAcpServer.ts:247`) is recorded as steering-capable. + #[tokio::test] + async fn initialize_records_steering_supported_when_advertised() { + let supported = steering_supported_after_initialize( + r#"{"protocolVersion":2,"agentCapabilities":{},"_meta":{"steering":{"supported":true}}}"#, + ) + .await; + assert!( + supported, + "_meta.steering.supported: true must set steering_supported" + ); + } + + /// Test 1b: no `_meta` at all (goose, buzz-agent, any older adapter) must + /// leave the capability off — this is what keeps a steer off the wire for + /// agents that never implemented it. + #[tokio::test] + async fn initialize_leaves_steering_unsupported_when_meta_absent() { + let supported = + steering_supported_after_initialize(r#"{"protocolVersion":2,"agentCapabilities":{}}"#) + .await; + assert!( + !supported, + "absent _meta must leave steering_supported false" + ); + } + + /// Test 1c: an explicit `supported: false` is respected, not treated as + /// "the key exists so it must work". + #[tokio::test] + async fn initialize_leaves_steering_unsupported_when_explicitly_false() { + let supported = steering_supported_after_initialize( + r#"{"protocolVersion":2,"_meta":{"steering":{"supported":false}}}"#, + ) + .await; + assert!( + !supported, + "_meta.steering.supported: false must leave steering_supported false" + ); + } + + /// Test 2: no `active_run_id` + capability advertised → the bytes on the + /// wire are an `_session/steering` request carrying `sessionId` and + /// `prompt`, and carrying **no** `expectedRunId` (the adapters reject + /// unknown required fields, and there is no run id to report anyway). + #[tokio::test] + async fn acp_steer_request_omits_expected_run_id_and_carries_session_and_prompt() { + let capture = capture_path("acp_shape"); + let mut client = spawn_steer_capture_script( + &capture, + r#"{"jsonrpc":"2.0","id":0,"result":{"outcome":"injected"}}"#, + ) + .await; + set_steering_supported(&mut client); + assert!( + client.active_run_id().is_none(), + "precondition: no active_run_id" + ); + + let (written, ack) = run_one_steer(&mut client, &capture).await; + + let written = written.expect("steer request must have been written"); + let msg: serde_json::Value = + serde_json::from_str(&written).expect("written line must be valid JSON"); + assert_eq!( + msg["method"].as_str(), + Some(ACP_STEER_METHOD), + "must use the cross-adapter steer method; wrote: {written}" + ); + assert_eq!(msg["params"]["sessionId"].as_str(), Some("sess-test")); + assert_eq!( + msg["params"]["prompt"][0]["text"].as_str(), + Some("steer body"), + "prompt must carry the steer body as a text block" + ); + assert!( + msg["params"].get("expectedRunId").is_none(), + "_session/steering must not carry expectedRunId; wrote: {written}" + ); + assert!( + matches!(ack, crate::pool::SteerAck::Success), + "injected outcome must ack Success, got {ack:?}" + ); + } + + /// Test 3: goose keeps priority. With both an `active_run_id` and the + /// advertised capability, the goose method wins — `expectedRunId` is + /// strictly more precise about which run is being steered. + #[tokio::test] + async fn goose_transport_wins_when_both_run_id_and_capability_present() { + let capture = capture_path("goose_priority"); + let mut client = + spawn_steer_capture_script(&capture, r#"{"jsonrpc":"2.0","id":0,"result":{}}"#).await; + set_steering_supported(&mut client); + let update = session_info_update_msg(Some(serde_json::json!("run-77"))); + let _ = client.handle_session_update(&update); + + let (written, ack) = run_one_steer(&mut client, &capture).await; + + let written = written.expect("steer request must have been written"); + let msg: serde_json::Value = + serde_json::from_str(&written).expect("written line must be valid JSON"); + assert_eq!( + msg["method"].as_str(), + Some(GOOSE_STEER_METHOD), + "goose method must win when a run id exists; wrote: {written}" + ); + assert_eq!(msg["params"]["expectedRunId"].as_str(), Some("run-77")); + // A bare `{}` result is a success on the goose transport (goose sends + // no `outcome`) — the OutcomeRejected guard applies only to + // `_session/steering`. + assert!( + matches!(ack, crate::pool::SteerAck::Success), + "goose success result must ack Success, got {ack:?}" + ); + } + + /// Test 7: codex-acp's third outcome, `failed` + /// (`src/AcpExtensions.ts:92`), is a delivery rejection despite being a + /// JSON-RPC success — release the event and fall back. + #[tokio::test] + async fn acp_steer_failed_outcome_acks_outcome_rejected() { + let capture = capture_path("outcome_failed"); + let mut client = spawn_steer_capture_script( + &capture, + r#"{"jsonrpc":"2.0","id":0,"result":{"outcome":"failed"}}"#, + ) + .await; + set_steering_supported(&mut client); + + let (_written, ack) = run_one_steer(&mut client, &capture).await; + + match ack { + crate::pool::SteerAck::Err(crate::pool::SteerError::OutcomeRejected { outcome }) => { + assert_eq!( + outcome, "failed", + "rejected outcome must report what the agent said, unquoted" + ); + } + other => panic!("expected Err(OutcomeRejected), got {other:?}"), + } + } + + /// Test 8: **codex `extMethod` silent-loss regression guard.** codex-acp's + /// ext dispatcher answers unrecognized methods with a bare `{}` — a + /// JSON-RPC *success*, not `-32601` (`src/CodexAcpServer.ts:255-258`). + /// Buzz maps `SteerAck::Success` to `queue.remove_event`, so decoding + /// `{}` as success would delete the user's message with no error, no + /// fallback, and no log. An absent `outcome` must therefore be a + /// rejection, which releases the event and fires cancel+merge. + #[tokio::test] + async fn acp_steer_missing_outcome_acks_outcome_rejected_and_never_drops_event() { + let capture = capture_path("outcome_absent"); + let mut client = + spawn_steer_capture_script(&capture, r#"{"jsonrpc":"2.0","id":0,"result":{}}"#).await; + set_steering_supported(&mut client); + + let (_written, ack) = run_one_steer(&mut client, &capture).await; + + match ack { + crate::pool::SteerAck::Err(crate::pool::SteerError::OutcomeRejected { outcome }) => { + assert_eq!( + outcome, "", + "a result with no outcome field must be reported as absent" + ); + } + other => panic!( + "expected Err(OutcomeRejected) for a bare {{}} success — \ + anything else risks dropping the event, got {other:?}" + ), + } + } + + /// Test 5: `injected` renews the hard deadline, so the turn survives past + /// its original one. Mirrors + /// `steer_success_renews_hard_deadline_and_survives_past_original` for + /// the `_session/steering` transport. + /// + /// Timeline: original hard deadline at t≈1s; steer response at t≈0.5s + /// renews it to t≈3.5s; prompt response at t≈1.5s lands inside it. + #[tokio::test] + async fn acp_steer_injected_renews_hard_deadline_and_survives_past_original() { + let script = "sleep 0.5; \ + echo '{\"jsonrpc\":\"2.0\",\"id\":0,\"result\":{\"outcome\":\"injected\"}}'; \ + sleep 1; \ + echo '{\"jsonrpc\":\"2.0\",\"id\":999,\"result\":{\"done\":true}}'"; + let mut client = spawn_script(script).await; + set_steering_supported(&mut client); + + let (steer_tx, steer_rx) = tokio::sync::mpsc::channel::(1); + client.install_steer_rx(steer_rx); + let (ack_tx, ack_rx) = tokio::sync::oneshot::channel::(); + let send_task = tokio::spawn(async move { + steer_tx + .send(crate::pool::SteerRequest { + prompt_blocks: vec!["steer body".into()], + ack_tx, + }) + .await + .expect("steer_tx send should succeed"); + }); + + let idle = std::time::Duration::from_secs(10); + let max_dur = std::time::Duration::from_secs(3); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(1); + let result = client + .read_until_response_with_idle_timeout("sess-test", 999, idle, hard_deadline, max_dur) + .await; + send_task.await.expect("send_task should complete"); + + assert!( + result.is_ok(), + "injected must renew the deadline so the prompt response still lands, got {result:?}" + ); + assert_eq!(result.unwrap()["done"], serde_json::json!(true)); + let ack = ack_rx.await.expect("ack must be received"); + assert!( + matches!(ack, crate::pool::SteerAck::Success), + "injected must ack Success, got {ack:?}" + ); + } + + /// Test 6: **red/green for the no-renewal rule.** `startedNewTurn` means + /// the turn Buzz was steering had already ended and the adapter began a + /// fresh, detached one. It acks `Success` (the message WAS delivered, so + /// the event must not be redelivered) but must NOT renew the hard + /// deadline — that clock belongs to a turn which is already settled. + /// + /// Same timeline as the `injected` test, so the only difference is the + /// outcome string: original hard deadline at t≈1s, steer response at + /// t≈0.5s, prompt response at t≈1.5s. With renewal the prompt response + /// would land and this returns `Ok`; without renewal the original + /// deadline fires first and we get `HardTimeout`. + #[tokio::test] + async fn acp_steer_started_new_turn_acks_success_without_renewing_hard_deadline() { + let script = "sleep 0.5; \ + echo '{\"jsonrpc\":\"2.0\",\"id\":0,\"result\":{\"outcome\":\"startedNewTurn\"}}'; \ + sleep 1; \ + echo '{\"jsonrpc\":\"2.0\",\"id\":999,\"result\":{\"done\":true}}'"; + let mut client = spawn_script(script).await; + set_steering_supported(&mut client); + + let (steer_tx, steer_rx) = tokio::sync::mpsc::channel::(1); + client.install_steer_rx(steer_rx); + let (ack_tx, ack_rx) = tokio::sync::oneshot::channel::(); + let send_task = tokio::spawn(async move { + steer_tx + .send(crate::pool::SteerRequest { + prompt_blocks: vec!["steer body".into()], + ack_tx, + }) + .await + .expect("steer_tx send should succeed"); + }); + + let idle = std::time::Duration::from_secs(10); + let max_dur = std::time::Duration::from_secs(3); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(1); + let result = client + .read_until_response_with_idle_timeout("sess-test", 999, idle, hard_deadline, max_dur) + .await; + send_task.await.expect("send_task should complete"); + + // The original deadline must still fire — renewal here would extend + // the clock on a turn the adapter has already finished. + assert!( + matches!(result, Err(AcpError::HardTimeout { .. })), + "startedNewTurn must NOT renew the hard deadline, so the original \ + one must still fire; got {result:?}" + ); + // Delivery still succeeded, so the withheld event must be dropped + // rather than released — hence Success, not an Err. + let ack = ack_rx.await.expect("ack must be received"); + assert!( + matches!(ack, crate::pool::SteerAck::Success), + "startedNewTurn is a delivery success, got {ack:?}" + ); + } + + /// Test 4 (companion to the existing + /// `native_steer_with_no_active_run_id_acks_expected_run_id_missing`): + /// no run id AND no advertised capability means nothing is written at + /// all. This is the gate that keeps a steer off the wire for adapters + /// that never implemented either method. + #[tokio::test] + async fn steer_writes_nothing_when_no_run_id_and_capability_absent() { + let capture = capture_path("no_transport"); + let mut client = + spawn_steer_capture_script(&capture, r#"{"jsonrpc":"2.0","id":0,"result":{}}"#).await; + assert!(!client.steering_supported(), "precondition: not advertised"); + assert!( + client.active_run_id().is_none(), + "precondition: no active_run_id" + ); + + let (written, ack) = run_one_steer(&mut client, &capture).await; + + assert!( + written.is_none(), + "no transport available must write nothing; wrote: {written:?}" + ); + match ack { + crate::pool::SteerAck::Err(crate::pool::SteerError::ExpectedRunIdMissing) => {} + other => panic!("expected Err(ExpectedRunIdMissing), got {other:?}"), + } + } + // ── Goose usage notification integration ────────────────────────────── /// Build a `_goose/unstable/session/update` JSON-RPC notification. diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index b11d96d8f7..d63f720c65 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1140,9 +1140,7 @@ fn any_respawn_in_flight(crash_history: &[SlotCircuit]) -> bool { /// Result of a background respawn task. struct RespawnResult { index: usize, - /// Tuple: (initialized client, protocol version, supports_goose_steer). - /// The third element is always `true` — the supervisor uses - /// try-and-tolerate for the steer extension. + /// Tuple: (initialized client, protocol version, agent name). result: Result<(AcpClient, u32, String)>, } @@ -2228,18 +2226,18 @@ async fn tokio_main() -> Result<()> { owner_cache.get(), ); if let Some(signal) = signal { - // Try-and-tolerate fork: when the mode - // wants a Steer, attempt the non-cancelling - // path first for any agent. On accept, + // Non-cancelling fork: when the mode + // wants a Steer, attempt the + // non-cancelling path first. On accept, // withhold the queued event and spawn an // ack watcher; the main loop's // `PoolEvent::SteerAck` arm decides // success/release/fallback. On reject - // (including `-32601 method_not_found` - // from agents that don't implement the - // extension), fall through to the universal - // cancel+merge `Steer` signal so the event - // still reaches the agent. + // (including agents that advertise no + // steer transport at all), fall through + // to the universal cancel+merge `Steer` + // signal so the event still reaches the + // agent. let native_attempted = matches!(signal, ControlSignal::Steer) && try_native_steer( &mut pool, @@ -2419,14 +2417,26 @@ async fn tokio_main() -> Result<()> { event_id, ack, })) => { - // Goose-native steer attempt resolved. Locked semantics - // (Eva + Max + Perci, unanimous on Option X): + // Mid-turn steer attempt resolved (either transport: + // `_goose/unstable/session/steer` or `_session/steering`). + // Locked semantics (Eva + Max + Perci, unanimous on Option X): // // Success // The agent received the steer via the non-cancelling // path. Drop the withheld event so normal dispatch // never redelivers it. // + // Also covers `_session/steering`'s `startedNewTurn` + // outcome: the message was delivered, but into a fresh + // turn because the one being steered had already + // finished. Delivery is what this arm keys on, so the + // event is still dropped. The read loop deliberately + // does NOT renew its hard deadline in that case (the + // awaited turn is settled), while + // `extend_in_flight_deadline` below still applies — + // the agent really is running more work, so the + // channel's in-flight budget should reflect it. + // // Err(_) where the write never landed (Transport / // ExpectedRunIdMissing): // Delivery state of the underlying message is "never @@ -2434,6 +2444,16 @@ async fn tokio_main() -> Result<()> { // queue front AND issue the cancel+merge fallback so // the message still reaches the agent. // + // Err(OutcomeRejected { .. }) + // A `_session/steering` request returned a JSON-RPC + // success whose `outcome` was not `injected` or + // `startedNewTurn` (codex's `failed`, an unknown value, + // or a bare `{}` with no `outcome` at all). The steer + // did not land, so this is treated exactly like a write + // that never happened: release withheld AND fire the + // cancel+merge fallback. Handled by the catch-all + // `Err(_)` arm below. + // // Err(AgentError { code: -32601, .. }) // The agent returned method_not_found — it does not // implement the steer extension. Release withheld AND @@ -2490,9 +2510,9 @@ async fn tokio_main() -> Result<()> { Ok(pool::SteerAck::Err(pool::SteerError::AgentError { .. })) => { (true, false, false) } - // Transport / ExpectedRunIdMissing: write never landed. - // Release and fire the cancel+merge fallback so the - // message still reaches the agent. + // Transport / ExpectedRunIdMissing / OutcomeRejected: the + // steer did not land. Release and fire the cancel+merge + // fallback so the message still reaches the agent. Ok(pool::SteerAck::Err(_)) => (true, false, true), Ok(pool::SteerAck::PromptCompletedNeutral) => (true, false, false), Err(_recv_err) => (true, false, false), @@ -2926,15 +2946,15 @@ fn dispatch_pending( let ctx_clone = Arc::clone(ctx); let agent_index = agent.index; - // Goose-native non-cancelling steer seam: snapshot capability before - // the agent moves into `run_prompt_task`, and install the per-turn - // steer receiver on the read loop so the main loop's mode-gate fork + // Mid-turn non-cancelling steer seam: install the per-turn steer + // receiver on the read loop so the main loop's mode-gate fork // (see the `if accepted && queue.is_channel_in_flight(...)` block // in the relay event branch of the main `select!` loop) can drive // it via the matching sender stored in `TaskMeta.steer_tx`. - // Install the steer channel for every prompt task — the supervisor - // uses try-and-tolerate: it attempts the steer for any agent and - // treats `-32601 method_not_found` as "fall back to cancel+merge". + // Installed for every prompt task: the read loop picks the steer + // transport at write time from `active_run_id` and the agent's + // advertised `_session/steering` capability, and acks + // `ExpectedRunIdMissing` (→ cancel+merge) when it has neither. let (tx, rx) = tokio::sync::mpsc::channel::(1); agent.acp.install_steer_rx(rx); let steer_tx = Some(tx); @@ -3783,7 +3803,8 @@ async fn initialize_agent_pool( .and_then(|info| info.get("name")) .and_then(|v| v.as_str()) .unwrap_or("unknown"), - "agent initialized — non-cancelling steer enabled (try-and-tolerate)" + steering_supported = acp.steering_supported(), + "agent initialized" ); acp.observe( "agent_initialized", diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 0c51fe954f..b1fd68d044 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -309,10 +309,13 @@ pub enum ControlSignal { /// for that — only a function parameter pass-through. /// /// If `active_run_id` is `None` at write time (no `session/update` seen yet -/// — e.g. agents that never emit run-id metadata), the steer cannot form a -/// valid `expectedRunId` and the read loop acks -/// [`SteerError::ExpectedRunIdMissing`]. The main loop maps this to the -/// "Err-before-pending" bucket: no withhold/mark was established at +/// — e.g. agents that never emit run-id metadata), the goose-native method +/// cannot form a valid `expectedRunId`, and the read loop falls back to the +/// cross-adapter `_session/steering` method when the agent advertised +/// `_meta.steering.supported` at `initialize`. That method takes no run id, so +/// no freshness concern applies to it. When neither transport is available the +/// read loop acks [`SteerError::ExpectedRunIdMissing`]. The main loop maps that +/// to the "Err-before-pending" bucket: no withhold/mark was established at /// `pool::send_steer` time because the request was rejected before any /// write, so the watcher only needs to release nothing and fall back to the /// universal `ControlSignal::Steer` cancel+merge path. @@ -326,7 +329,8 @@ pub struct SteerRequest { pub ack_tx: tokio::sync::oneshot::Sender, } -/// Why a goose-native steer failed. +/// Why a mid-turn steer failed, on either transport +/// (`_goose/unstable/session/steer` or `_session/steering`). /// /// String and integer fields are intentionally `Debug`-only — read by /// `tracing` macros in the main loop's `PoolEvent::SteerAck` arm via @@ -349,14 +353,28 @@ pub enum SteerError { /// Transport-level failure: write error, read EOF, JSON-RPC framing /// violation, etc. The string carries the underlying `AcpError`'s display. Transport(String), - /// At steer-write time `AcpClient::active_run_id` was `None`, so the - /// read loop couldn't form a valid `expectedRunId`. The read loop drops - /// the request without writing anything; the main loop should release - /// any withheld event and fall back to the universal cancel+merge + /// At steer-write time neither steer transport was available: no + /// `expectedRunId` (`AcpClient::active_run_id` was `None`, so the + /// goose-native method could not be formed) and the agent did not + /// advertise the cross-adapter `_session/steering` extension. The read + /// loop drops the request without writing anything; the main loop should + /// release any withheld event and fall back to the universal cancel+merge /// `ControlSignal::Steer` path. This is in the same "Err-before-pending" /// bucket as `Transport` write failures: no in-process state was /// established, so no in-process cleanup is needed. ExpectedRunIdMissing, + /// A `_session/steering` request returned a JSON-RPC *success* whose + /// `outcome` was not one of the two recognized delivery outcomes + /// (`injected`, `startedNewTurn`) — including `failed` (codex-acp) and a + /// missing `outcome` entirely. `outcome` carries what the agent actually + /// reported, for logs. + /// + /// The steer did NOT land, so the main loop must release the withheld + /// event and fire the cancel+merge fallback — exactly like a write that + /// never happened. Treating an unrecognized success as delivery would + /// drop the user's message: codex-acp answers unrecognized extension + /// methods with a bare `{}` success rather than `-32601`. + OutcomeRejected { outcome: String }, /// The read loop never got to dispatch the steer because the prompt /// completed first. Delivery state for the underlying message is /// unknown after prompt completion — the main loop must treat this as @@ -369,7 +387,7 @@ pub enum SteerError { PromptCompleted, } -/// Outcome of a goose-native steer, sent from the read loop back to the +/// Outcome of a mid-turn steer, sent from the read loop back to the /// main loop's ack watcher. #[derive(Debug)] pub enum SteerAck { diff --git a/deploy/charts/buzz/README.md b/deploy/charts/buzz/README.md index 2e4d378203..5411e6913a 100644 --- a/deploy/charts/buzz/README.md +++ b/deploy/charts/buzz/README.md @@ -12,7 +12,7 @@ This chart has two operating profiles selected by values: ## Quickstart (eval only) ```sh -helm install buzz oci://ghcr.io/block/buzz/charts/buzz --version 0.1.0 \ +helm install buzz oci://ghcr.io/block/buzz/charts/buzz --version 0.1.7 \ --create-namespace --namespace buzz \ --set quickstart=true \ --set postgresql.enabled=true \ diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 0d89b8e2d2..459fa75743 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -126,6 +126,7 @@ export default defineConfig({ "**/observer-archive-policy.spec.ts", "**/harness-management.spec.ts", "**/harness-catalog-screenshots.spec.ts", + "**/inline-custom-harness.spec.ts", ], use: { ...devices["Desktop Chrome"], diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index f8ee32dfab..326587b87f 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -46,673 +46,8 @@ const rules = [ }, ]; -// TEMP — these files exceed the 1000-line limit and are queued to be split. -// Do not add to this list; split the file instead. Remove each entry as its -// file is broken up. Tracked as a follow-up. -const overrides = new Map([ - // Inherited from origin/main: #2630 (agent emoji picker search) grew this - // file to 1026 lines with no override; this branch does not touch the file. - // Narrow ratchet so unrelated branches stay green; queued to split upstream. - ["src/features/agents/ui/AgentCreationPreview.tsx", 1026], - // Native Builderlab auth/community commands add a small registration surface - // to the existing Tauri composition root. The implementation lives in - // builderlab.rs; this narrowly ratchets the command wiring while lib.rs is - // queued for a broader composition-root split. Bumped for the - // archive/unarchive/transfer community-management commands (web parity). - ["src-tauri/src/lib.rs", 1013], - // persona-events rebase: build_deploy_payload threads `state` for the - // read-time relay-URL workspace fallback while keeping the create-time env - // pin (the credential-leak guard). Load-bearing feature growth from the - // rebase, queued to split with the rest of this list. - // persona-refresh-on-spawn: re-snapshot + retain_managed_agent_pending call - // in start_local_agent_with_preflight adds ~23 lines. Queued to split. - // rebase onto main (2026-06-25): main's agents.rs grew by ~17 lines since - // config-bridge: get_agent_config_surface/write_agent_config_field/put_agent_session_config - // commands add ~40 lines. Queued to split. - // branch cut; override bumped to cover the merged total. Queued to split. - // persona-blank-fallback: persona_snapshot_with_agent_config_fallback call - // sites add ~4 lines (extra fallback params + inline comments). build_deploy_payload - // fix (blank-persona provider/model fallback) adds ~6 lines. Bug fix. - // archive/mod_tests.rs carries the full test module for archive/mod.rs: - // unit tests + 4 real-relay integration tests (ignored, live-relay only). - // Production logic in mod.rs is now ~527 lines (under 1000). mod_tests.rs - // is test-only content; the override covers the test growth accumulated - // across the local-archive + agent-metric-archive PR series. store_tests.rs - // (~731 lines) is under 1000 so needs no override. - ["src-tauri/src/archive/mod_tests.rs", 1208], - // unified-agent-model 1A.1: profile reconcile split to agents_profile.rs, - // ratcheting 1443 -> 1295. Queued to split further in the A2 fold. - // global-agent-config: resolve_deploy_model_provider + visibility exports - // add ~40 lines on top of the 1A.1 ratchet. Queued to split. - // +29 (1340 -> 1369, main): agent-config-resolver — start_local_agent_with_preflight - // uses resolve_effective_relay_mesh_model_id at both preflight call sites; - // preview_prospective_persona_snapshot helper extracted; orphan guard threaded - // through restore path; start_local_agent_pairs_with_preflight resolver - // preflight. Load-bearing feature changes; queued to split. - // +47 (#2773): review fix — load_global_agent_config hoisted out of - // build_managed_agent_summary into callers, dangling-harness summaries render - // the deleted id, and spawn errors surface as sentences (tests included). - // +1: merge of the two deltas above (actual post-merge count). - ["src-tauri/src/commands/agents.rs", 1418], - // agent-lifecycle-fixes: cascade-delete in delete_persona restructured into - // 3-phase (stage/stop/commit) + commit_cascade_agents injectable helper for - // retry-safety. Load-bearing reviewer-required change; queued to split. - // Consolidation removed the legacy persona-card import/export codecs. - // #1418 read-path fix: get_thread_replies' blocker fix (shared TIMELINE_KINDS - // const + build_thread_replies_filter helper, mirroring the channel sibling so - // the two p-gate filters can't drift) plus two guard unit tests. The file was - // already at 995; this load-bearing correctness fix crossed 1000. Not generic - // debt growth. Approved override; queued to split with the rest of this list. - ["src-tauri/src/commands/messages.rs", 1082], - // Residual repos_dir integration in ensure_nest_at: REPOS is provisioned - // outside NEST_DIRS (it may be a symlink), so it needs its own create + - // chmod-only-when-real-dir handling plus integration test coverage. The - // self-contained repos_dir functions and their unit tests live in repos.rs; - // this is the seam that must stay in nest.rs. Approved override; still queued - // to split with the rest of this list. - // dev-nest namespace: OnceLock> + init_nest_dir + constants - // added to plumb the dev/prod discriminator. Load-bearing for the D2 nest fix. - // dev-build CLI symlink: cli_link_name helper + is_dev param on - // ensure_cli_symlink + prod/dev test variants add ~68 lines. Load-bearing; - // queued to split with the rest of this list. - // +4 lines: adopt shared create_symlink wrapper (behavior-preserving refactor - // for multi-line rustfmt expansion of the skills symlink call site). - // unified-agent-model 1A.1: inline test module moved to nest/tests.rs, - // ratcheting 1575 -> 679 (under the 1000 default; entry kept as a ratchet). - // observer-archive dev-default: path_is_dev_nest + nest_is_dev getters - // (+25 lines) so observer_archive_default_enabled() keys off the dev nest. - // Load-bearing; spends banked ratchet headroom, still well under 1000. - ["src-tauri/src/managed_agents/nest.rs", 704], - // keyring-dev-isolation: agent key migration added copy_agent_keys_between_stores - // and load_readonly support; file grew past 1000 default. Queued to split. - // +7 for try_delete_agent_key result-returning seam (snapshot-import rollback). - // +48 (1335 -> 1383): agents-everywhere pair re-key — pair-scoped runtime - // receipts (write_agent_runtime_receipt atomic JSON + remove/read_all - // helpers) replace the pubkey-keyed PID file, plus the hashed pair-scoped - // runtime log path. Load-bearing crash-recovery surface; queued to split. - // harness-log reader fix: the inline test module moved to storage_tests.rs - // (`#[path]`-included), ratcheting 1383 -> 826. Both halves are now under the - // 1000 default; entries kept as ratchets. - ["src-tauri/src/managed_agents/storage.rs", 826], - ["src-tauri/src/managed_agents/storage_tests.rs", 701], - // config-bridge setup-payload env-boundary fix adds readiness wiring in - // spawn_agent_child; load-bearing security fix, queued to split. - ["src-tauri/src/managed_agents/config_bridge/reader.rs", 1016], - // config-bridge-aware requirements: goose_requirements + injection tests - // (4 new tests in goose_file_config_tests module) + test-determinism fixes - // for the 3 existing goose tests that previously read real disk config. - // New file in this PR; queued to split. - // +2 readiness integration tests for flat-DATABRICKS_HOST canonicalization fix. - // +1 cargo fmt whitespace reformat (readiness.rs closures inline after rebase). - // +2 unit tests for cli_login_requirements resolve_command integration (DMG PATH fix). - // Doctor-CTA: reworked cli_login_requirements to carry AcpAvailabilityStatus, - // skip login probe for not-installed/adapter-missing/cli-missing states, and - // added 4 unit tests covering each arm. Load-bearing discoverability fix. - // Updated existing codex_not_ready test to use make_cli_runtime stub. - // +4 lines: #1640 persona-env-vars-refresh rebase added availability-classification - // growth in the live-persona env merge path. Feature plumbing, not generic debt. - // Windows-CI portability: replaced POSIX true/false probes with current_exe() - // stand-in + present_binary_str()/static_commands() helpers (+29 lines). - // Tests now pass on windows-latest CI shard without POSIX shell utilities. - // databricks-v1-to-v2-migration: databricks-v2 hyphen-alias added to all - // host/credential match arms + 30+ readiness tests for provider aliases, - // missing-host, and DATABRICKS_MODEL fallback. Load-bearing correctness fix. - // #1613 augmented-PATH readiness probes grew the file +3 past the prior cap. - // +16: resolve_effective_agent_env + global-config readiness wiring (#1448). - // +1 rebase merge: GlobalAgentConfig import added alongside AcpAvailabilityStatus. - // +2 rebase onto #1667: behavioral quad fields in AgentDefinition/ManagedAgentRecord. - // +3 rebase onto main (#1568 + #1613): identity-import-keyring + augmented-PATH probes. - // +18: CliConfigInvalid requirement surface for config-parse probe classification — - // new Requirement variant + updated cli_login_requirements + 3 new probe-layer tests. - // Load-bearing UX fix (bad config → clear diagnostic, not "run codex login"). - // codex-acp-package-swap: AdapterOutdated version-probe in cli_login_requirements - // (+22 lines). Load-bearing — blocks login gate for deprecated 0.16.x adapter. - // code-reviewer fix-round: codex readiness gate tests — 2 new tests for - // outdated-adapter and garbage-version-output paths through the codex id gate - // (+140 lines: make_codex_runtime helper, PATH_MUTEX serializer, 2 test fns). - // Load-bearing test coverage; queued to split with the file generally. - // +1: pub(crate) mod cli_probe declaration for doctor auth probe access. - // +3: auth_probe_args: None + login_hint: None added to make_cli_runtime and - // make_codex_runtime stubs (new KnownAcpRuntime fields). - // Git Bash readiness is intentionally colocated with buzz-agent's other - // setup-mode requirements. The Windows-only requirement and serialization - // test add eight lines; split remains queued with the existing file debt. - // Windows Doctor install fix: cli_install_commands_windows field added to test stubs. - // team-instructions-first-class: ManagedAgentRecord fixture gains the new - // team_id field (+1 line). - ["src-tauri/src/managed_agents/readiness.rs", 1863], - // Windows PATH-correctness fix: 3 #[cfg(windows)] test functions covering - // .cmd shim rejection, .bat shim rejection, and .exe acceptance for - // configure_runtime_cli (fix #2397). Test-only growth; queued to split. - // +7 (main): this PR's resolver tests land on top of main's #2397 Windows - // shim tests, plus main's restart_eligible orphan-gate tests. - // +34: BYOH custom-harness sweep condition unit tests — 3 tests validating - // the OR-gate fix for custom-binary orphan cleanup. - // +26: BYOH pass-2 I3 — 2 collector-decision tests for receipt path - // ownership (valid_agent_runtime_receipt uses buzz_sweep_owns_process). - ["src-tauri/src/managed_agents/runtime/tests.rs", 1320], - // runtime.rs re-entered the list after the #1968 merge: main's - // definition-authoritative resolver comments grew it to 982, and the BYOH - // typed harness-descriptor resolution in spawn_agent_child landed on top at - // 1020. The session-title env write in spawn_agent_child adds 12. - // Queued to shrink with the next runtime split pass (#2974 follow-up). - // +1: #3023 credential-helper slash normalization (MinGW bash treats - // backslashes as escapes). - ["src-tauri/src/managed_agents/runtime.rs", 1033], - // applyWorkspace reposDir parameter plus the validateReposDir binding, - // threaded through Tauri invokes for configurable repos_dir, plus the - // harness-persona-sync `harnessOverride` create-input bit — load-bearing - // parameter plumbing, not generic debt growth. Approved override; still - // queued to split. Read-path lanes 1+2 add server-side fetch bindings - // (getThreadReplies + getChannelMessagesBefore) and paged people-search - // reachability — load-bearing reachability plumbing, not generic debt. - // #1418 read-path fix: +3 doc-only lines correcting the getThreadReplies - // contract (replies-only, root excluded — the query keys on root_event_id, - // which root rows lack). Documentation accuracy, not code growth. - // linux-updater isAutoUpdateSupported() binding + onboarding has_profile_event field. - // config-bridge-aware requirements: getRuntimeFileConfig command adds ~15 lines. - // +26 lines from PRs landing on main between prior rebase and this rebase. - // baked-env-required-badge: getBakedBuildEnvKeys wrapper adds ~16 lines. Queued to split. - // restart-badge: started the queued split — start/stopManagedAgent moved to - // tauriManagedAgents.ts; limit ratcheted down 1388 → 1380 to bank the headroom. - // identity-import-keyring: identity wrappers (RawIdentity, getIdentity, getNsec, - // importIdentity, persistCurrentIdentity) moved to tauriIdentity.ts; - // limit ratcheted down 1380 → 1360 to bank the headroom (absorbs main-side - // growth landed between the split and the rebase). - // mention-alias fix: profile wrappers (RawProfile/RawUserProfileSummary types, - // getProfile/updateProfile/getUserProfile/getUsersBatch/searchUsers) moved to - // tauriProfiles.ts; limit ratcheted down 1360 → 1241 to bank the headroom. - // baked-env fold-in: getBakedBuildEnv + BakedEnvEntry type adds ~28 lines. - // doctor-npm-eacces-preflight: hint field on RawInstallStepResult + mapper - // passthrough (+2 lines). - // doctor-install-reliability: node_required + auth_status + login_hint fields - // added to RawAcpRuntimeCatalogEntry + fromRawAcpRuntimeCatalogEntry mapper (+8). - // codex-install-auto-restart: restarted_count + failed_restart_count added to - // RawInstallRuntimeResult + fromRawInstallRuntimeResult mapper (+2). - // Git Bash Doctor discovery adds the raw Tauri response and its camelCase - // mapper. This is the existing API boundary; split remains queued. - // team-instructions-first-class: createManagedAgent Tauri bridge threads the - // new teamId input through to the backend (+1 line). - // +2 for model_source field in RawManagedAgent + fromRawManagedAgent mapping. - ["src/shared/api/tauri.ts", 1307], - // doctor-npm-eacces-preflight: hint field added to InstallStepResult (+1 line). - // codex-acp-package-swap: "adapter_outdated" variant added to AcpAvailabilityStatus (+1 line). - // doctor-install-reliability: AuthStatus tagged union + nodeRequired/authStatus/ - // loginHint fields on AcpRuntimeCatalogEntry (+14 lines). Load-bearing new feature. - // agent-lifecycle-fixes: GlobalAgentConfigSaveResult type grows with - // failed_restart_count (+2 lines). Queued to split with the rest of this list. - // mcp-readonly-view rebase: PR2 MCP config surface FE-type fields force +1 over the grandfathered ceiling. - // Git Bash prerequisite payload adds four fields to the shared Tauri API - // contract. This is the canonical type location; split remains queued. - // signout-wipe: resetFailed field added to Identity type (+6 lines). - // team-instructions-first-class: CreateManagedAgentInput.teamId (+2, incl. - // doc comment) and AgentTeam/CreateTeamInput/UpdateTeamInput.instructions - // (+3) — the new team-id spawn link and the runtime-layered instructions - // field. - // byoh-env-roundtrip: AcpRuntimeCatalogEntry.definitionEnv field + JSDoc - // (+12 lines) so the edit form can read back existing env vars on save. - // Load-bearing correctness fix. Queued to split. - // +2: AcpRuntimeCatalogEntry.requiresExternalCli field added by main - // (#2680) to indicate runtimes that need a separate CLI install. - // +6: ManagedAgent.runtime record-level pin + JSDoc so the harness delete - // confirmation can count referencing agents (review fix for #2773). - // +21: CatalogSourceCoordinate + the `catalogSource` fields on AgentPersona - // and CreatePersonaInput. The coordinate is the only identifier a catalog - // copy keeps, so it is what stops the catalog re-offering "Add" for an - // already-added foreign entry. Queued to split. - ["src/shared/api/types.ts", 1079], - // harness-persona-sync feature growth, queued to split in the resolver-unify - // refactor followup. discovery.rs is dominated by the new test module - // (the effective_agent_command / divergent / create-time override matrix); - // alias-preservation coverage extends that matrix so create-time persona - // agents keep an installed runtime alias when the primary command is absent. - // Load-bearing, not generic debt. - // config-bridge: schema-driven field extraction adds ~26 lines. Queued to split. - // config-parity: max_tokens_env_var + context_limit_env_var fields added to - // KnownAcpRuntime (2 fields × 4 runtimes + discovery tests = ~13 lines). - // Load-bearing — required for buzz-agent normalized config parity. - // same-runtime-pin: update_time_agent_command_override + its override / - // same-runtime / alias / sentinel / non-override / persona-less test matrix - // (~135 lines, mostly tests) so a deliberate Custom pin survives the update - // path instead of being dropped back to inherit. Load-bearing, not debt. - // unified-agent-model 1A.1: inline test module moved to discovery/tests.rs, - // ratcheting 1259 -> 802 (under the 1000 default; entry kept as a ratchet). - // agent-config-propagation: the agent_command_override decision family - // (divergent / create-time / update-time / apply) moved to - // discovery/overrides.rs; ratcheting 802 -> 685 to bank the headroom. - // codex-acp-package-swap: probe_codex_acp_major_version (+24 lines) + - // AdapterOutdated version-gate in discover_acp_runtimes (+22 lines). Both - // load-bearing — required to detect the deprecated 0.16.x adapter and - // prevent silent relay breakage after the spawn-contract change. - // codex-acp-package-swap follow-up: tempfile-based bounded stdout read - // (+18 lines), codex_adapter_availability/is_outdated helpers (+16 lines), - // cross-platform probe contract. All load-bearing — required for correct - // probe behaviour on Windows and descendant-process edge cases. - // doctor-install-reliability: refreshable login_shell_path cache, - // find_nvm_default_bin + parse_semver_tag helpers, auth probe cache + - // probe_auth_status/cached_auth_status, runtime_needs_npm, probe_args_for, - // PartialEntry struct, and updated discover_acp_runtimes with parallel auth - // probes. Load-bearing fresh-install reliability fixes. (+289 lines) - // doctor-install-reliability review fixes: LoginShellPath enum + double-checked - // locking, is_safe_nvm_tag security validation, classify_probe_output helper, - // auth_probe_args on KnownAcpRuntime (removes probe_args_for indirection), - // process-level timeout replacing inner-thread pattern. (+75 lines) - // codex-install-auto-restart review-fixes: availability_drift pure predicate - // + updated adapter_availability_cached() signature (Option return, cold=None) - // prevents false restart badge on newly restarted agents. Correctness fix; - // load-bearing — required by Thufir's IMPORTANT findings. (+15 lines) - // Windows Doctor install fix: cli_install_commands_windows field, impl block - // for cli_install_commands_for_os(), command_basenames() + .cmd/.bat resolution, - // Windows well-known dirs in common_binary_paths(), login_shell_candidates(), - // path_candidates_from_env_raw(). Load-bearing Windows platform support. - // +13: fetch_login_shell_path_inner Windows guard (POSIX PATH → None). - // resolve_git_bash made pub(crate) for Windows test access. - // +1: login_shell_candidates doc comment expanded for resolve_bash_path. - // Buzz-managed Node path helpers and resolution tests moved to - // managed_node_paths.rs and discovery/tests/managed_path_resolution.rs; - // ratcheting 1366 -> 1392 after adding the managed-path probes to discovery. - // +17: BYOH custom harness catalog merge phase-3 — append custom definitions - // from custom_harnesses_dir with PATH-probe availability; source tagging. - // +148: BYOH F2/F3 — PRESET_HARNESSES static data (6 presets), Phase 2.5 in - // discover_acp_runtimes_from (PATH-probe each preset, build catalog entries, - // populate loaded-harness registry), record/effective command resolution now - // checks loaded registry for preset/custom ids. Queued to split presets out. - // +3: BYOH F5 — seen_ids rejects preset/builtin collisions from custom files. - // +79: BYOH pass-2 C1 — 4 registry lifecycle tests (warm→spawn, delete→ - // dangling, immediate save+start, edit with rename); try_record_agent_command - // typed error for dangling ids wired into spawn; readiness/spawn_hash now - // include definition env floor. - // +7: BYOH pass-2 I2 env round-trip — definition_env field populated in - // custom catalog entries + 2 discriminating tests (custom env preserved, - // builtin env empty). Load-bearing edit round-trip fix. - // +16: BYOH scope addition — Hermes Agent + OpenClaw preset entries (two - // data-only PresetHarness structs; no new logic or test functions). - // +29: rebase over main (#2680) — discover_acp_runtime_phase1 extracted - // helper + discover_acp_runtime_availability; both load-bearing for - // post-install verification. Semantic composition with BYOH changes. - // +17: merge of main (#2767) — codex_adapter_is_outdated_with_path split out - // so Codex adapter planning takes an explicit PATH. Auto-merged cleanly; only - // the ceiling needed composing with the BYOH growth above. - // +13: review fix for #2773 — discovery publishes the registry by re-reading - // the harness dir under persist_mutex (publish_harness_registry_from_dir call - // + doc comment), closing the stale-snapshot clobber race. - // +35: review round 2 (#2773) — cfg(test) pre_publish_test_hook seam so the - // stale-publish regression is pinned through the REAL discover_acp_runtimes_from - // path (Wren's finding: the seam-only tests stayed green under a stale-publish - // mutant). Test-only code, zero release-build footprint. - // +55: #2773 follow-up — PresetHarness.underlying_cli (Amp's amp-acp wraps - // the amp CLI) + preset_catalog_entry helper: adapter presence alone keeps - // deciding Available (adapter-present/CLI-absent stays selectable, Wren's - // regression catch); underlying_cli is consulted only when the adapter is - // absent, so AdapterMissing replaces the misleading NotInstalled. Includes - // the deliberate-divergence doc comments; net after the inline preset - // entries.push block collapsed into the helper. - // +6: legacy Goose Windows install dir (%USERPROFILE%\goose) probed in - // common_binary_paths so pre-#2680 standalone installs are discoverable. - // +19: codex-acp minimum-version gate — MIN_CODEX_ACP_VERSION plus the strict - // three-component parse in probe_codex_acp_version, so an outdated 1.x adapter - // is offered a reinstall instead of classifying as Available on major alone. - ["src-tauri/src/managed_agents/discovery.rs", 1860], - // BYOH — save_custom_harness_to_dir (backup-swap atomic write) + save_and_warm / - // delete_and_warm (persist-mutex serialization for concurrent-safe registry - // refresh, B-6). Also: id/collision/load/registry tests (from the file base) + - // B-4 real persistence tests (create, same-id edit, rename, backup cleanup) + - // B-3 env validation boundary tests (malformed key, reserved shape, NUL, - // size limit, ownership marker). Load-bearing correctness/security coverage; - // queued to extract helper module once the feature stabilizes. - // +153: review fix for #2773 — collision/dup filtering moved into - // load_custom_harnesses so warm + discovery inherit identical shadowing - // rules, publish_harness_registry_from_dir (mutex-scoped publish seam), and - // comma-in-args validation at validate_harness_definition, with tests. - // +34: review round 2 (#2773) — Dawn's mutation finding: the loader-boundary - // collision/dedup enforcement was untested (deleting it left the suite green). - // load_applies_id_collision_check now drives the real loader against a real - // shadowing file, plus a dedup twin; both verified to kill the mutants. - ["src-tauri/src/managed_agents/custom_harnesses.rs", 1232], - // rebase over codex-acp-package-swap: its version-probe tests union with the - // doctor-install-reliability nvm/login-shell/semver tests — each side alone - // stayed under the 1000 default; the union exceeds it. - // Windows Doctor install fix: command_basenames, cli_install_commands_for_os, - // and login_shell_candidates tests. Load-bearing platform-awareness coverage. - // +132: pass 2 — five cfg(windows) behavioral tests: command_basenames .cmd/.bat - // candidates, cli_install_commands_for_os PowerShell selection, login_shell_path - // None regression, .cmd shim resolution, no-git-bash error hint. - // +32: deterministic .cmd resolver + no-registry + install_shell_from tests. - // Managed-path resolution test split to discovery/tests/managed_path_resolution.rs. - // +227: BYOH pass-2 C1 — 4 registry lifecycle tests (warm→spawn, delete→dangling, - // immediate save+start, edit with rename) added to discovery/tests.rs. - // +64: BYOH pass-2 I2 env round-trip — 2 discriminating tests proving custom - // catalog entries carry definition_env and builtins do not. - // +90: review fix for #2773 — deterministic interleaving regressions for the - // discovery publish race (save-during-discovery survives publish; - // delete-during-discovery stays gone). - // +103: review round 2 (#2773) — production-path interleaving regressions: - // discovery_publish_path_survives_mid_flight_save / _drops_mid_flight_delete - // drive the real discover_acp_runtimes_from with a save/delete landed via the - // pre_publish_test_hook; verified to red under a stale-publish mutant. - // +18: flake fix — lock_path_mutex + registry_test_lock guards (with lock- - // order comments) on the four tests that drive discovery's global caches. - // +84: #2773 follow-up — preset_catalog_entry coverage (Amp-shaped adapter - // preset: AdapterMissing when CLI present, NotInstalled both-missing, - // Available both-present AND adapter-present/CLI-absent — the selectability - // regression guard), bound to an injectable resolver so the tests stay - // PATH-independent. - // +51: codex-acp minimum-version gate — probe_codex_acp_version assertions carry - // the full (major, minor, patch) triple instead of a bare major, plus - // below-the-floor and uncomparable-version (partial / prerelease) classification - // regressions for the fail-closed parse. - // +2 (1922 -> 1924): the AgentDefinition and ManagedAgentRecord fixtures each - // set the new mandatory `catalog_source` field. - ["src-tauri/src/managed_agents/discovery/tests.rs", 1924], - // identity-import-keyring: the identity resolution state machine's behavioral - // matrix (46 tests over FakeIdentityStore — probe × marker × file cells, - // adoption / read-back-corruption / marker-failure arms, recovery-mode - // gating). Load-bearing regression coverage for silent identity rotation, - // not generic debt growth. Approved override; split if the matrix grows. - ["src-tauri/src/app_state_tests.rs", 1420], - // migration_tests.rs carries the harness-sync migration coverage plus the - // patch_json_records owner-only writeback regression test (SECURITY.md:90 - // crash-safe 0o600 fallback). Load-bearing security + feature coverage, not - // generic debt growth. Approved override; still queued to split. Event-sync - // (persona/team event reconcile) tests were split out to event_sync_tests.rs - // and the limit ratcheted 1410 → 1110. - // unified-agent-model 1A.1: materialize tests live with their module in - // migration/materialize.rs; ratchet held at 1110. - ["src-tauri/src/migration_tests.rs", 1110], - ["src-tauri/src/nostr_convert.rs", 1126], - // degraded-network resilience: relay.rs grew past 1000 with the addition of - // relay_error_message hint-capping (oversized-hint test via loopback TCP) and - // the relay_admission freshness-verification test. The loopback mock was - // hardened (std::net + request-read-before-write) adding ~10 lines. - // Queued to split test helpers to relay/tests.rs. - // +30 (1047 -> 1077): agents-everywhere pair re-key — query_relay_at_with_keys - // (NIP-98 signed /query with explicit agent keys + optional x-auth-tag) for - // bounded-auth agent relay-membership discovery. Load-bearing; queued to - // split alongside the test-helper split. - ["src-tauri/src/relay.rs", 1077], - // degraded-network resilience: visibleChannelId field + getter/setter, NOTICE - // handler for relay back-pressure, and rate-limit gate imports add ~74 lines - // of load-bearing degraded-network recovery code. Queued to split. - ["src/shared/api/relayClientSession.ts", 1096], - // Boot-time event sync (persona/team/agent event reconcile) was split out - // to event_sync.rs, ratcheting this limit 1575 → 1310. Remaining content is - // the pre-identity data migrations; still queued to split further. - // unified-agent-model 1A.1: materialize_agent_runtimes split to - // migration/materialize.rs, ratcheting 1310 -> 1297. - // databricks-v1-to-v2-migration: reconcile_databricks_v1_to_v2 migration - // + inner fn with baked-env gate + 26 tests. Load-bearing correctness fix. - // am review fix: also clear stale V1 model field on provider rewrite + - // new model-clear test. Load-bearing chimera fix. - // keyring-dev-isolation: run_boot_migrations wires agent-key migration. - ["src-tauri/src/migration.rs", 1436], - // onMarkRead + isUnread prop threading (mirrors the onMarkUnread prop - // already here) for the single-toggle mark-read/unread menu item — a small - // overage from load-bearing per-message plumbing, not generic debt growth. - // Approved override; still queued to split with the rest of this list. - ["src/features/messages/ui/MessageThreadPanel.tsx", 1006], - // AgentConfigPanel footer fold into ProfileFieldGroup for the config-bridge - // panel — a small overage from load-bearing UI plumbing, not generic debt - // growth. Approved override; still queued to split with the rest of this list. - // +135 for AgentInfoFocusedView/DiagnosticsFocusedView/ChannelsFocusedView - // props restored after 826d735fe removal (UserProfilePanel.tsx still needs them). - ["src/features/profile/ui/UserProfilePanelSections.tsx", 1140], - // +14 for openEditAgent event subscription (config-nudge card "Open Edit Agent" action). - // +11 for editAgentFocus state + initialFocus prop threading (deep-link granularity). - ["src/features/profile/ui/UserProfilePanel.tsx", 1025], - // PersistBackend enum + marker-on-keyring-success plumbing and its three - // fail-closed regression tests (silent identity rotation on keyring outage). - // A small overage from load-bearing security plumbing on a file already at - // 893 lines, not generic debt growth. Approved override; still queued to split. - // cross-process keychain race fix (D3): interprocess lock + BlobLockGuard + - // uid-keyed lockfile path + behavioral tests add ~303 lines. Load-bearing - // security fix for the lost-update race that stranded agent keys. - // identity-import-keyring: KeyringLockedScreen, RecoveryScreen, - // load_readonly + load_all_readonly + store_all for safe cross-service reads. - // sign-out wipe: delete_all() method removes the entire keychain blob under - // the interprocess advisory lock; +8 lines. Load-bearing; queued to split. - // signout-wipe phase 2: delete_all_with_legacy_cleanup replaces delete_all; - // reads blob keys + deletes per-key legacy entries to prevent resurrection. - // + regression test for per-key resurrection via real OS keychain. - // Net growth ~36+32 lines over prior cap. Load-bearing correctness fix. - // signout-wipe pass-2 (F2): delete_all_with_legacy_cleanup DPK deletes now - // observable (propagate real errors); verify_fully_wiped checks all three - // keychain shapes (main blob, DPK blob, per-key "identity"). +73 lines. - ["src-tauri/src/secret_store.rs", 1307], - // keyring-dev-isolation: keyring_service() fn (7 lines) replaces the const - // to return "buzz-desktop-dev" in debug builds. Load-bearing isolation fix. - // +10 (1042 -> 1052): media_fetch_client with redirect::Policy::none() so a - // relay 3xx cannot forward the minted auth header cross-origin (SSRF fix). - // +16 (1052 -> 1068): extracted that client into `build_media_fetch_client()` - // -> Result so the fail-closed invariant is testable (no silent redirect- - // following fallback; startup panics loudly instead). The function belongs - // here beside `build_app_state` and its sibling client; its doc comment - // carries the load-bearing SSRF rationale. Extraction would only relocate, - // not reduce, the security-critical code. - // +5 (1068 -> 1073): merge with main, which independently added the - // managed_agent_profile_reconcile_enabled flag (field + doc + init) under - // its own 1042-line override. Union of two separately approved additions. - // +8 (1073 -> 1081): agents-everywhere pair re-key — managed_agent_processes - // and session_config_cache re-keyed by ManagedAgentRuntimeKey, the runtime - // transition lock doc broadened to cover all protected-PID transitions, and - // clear_agent_session_caches (per-pubkey retain) added alongside the - // per-key clear. Load-bearing identity-contract change; queued to split. - // +4 (1081 -> 1085): mesh recovery keeps one app-scoped state object beside - // the embedded runtime and coordinator. Probe/re-arm logic lives in - // mesh_llm/recovery.rs rather than growing AppState or command modules. - ["src-tauri/src/app_state.rs", 1085], - // multi-slot splitting + no-op suppression (#1309): the ReadStateManager - // class grew from ~700 lines to ~1019 with the addition of - // splitContextsIntoBudgetedSlots (pure fn + 5 tests), publishSplitSlots, - // publishOneSlot, deleteExtraSlots, and the no-op suppression integration - // test. Load-bearing feature growth, queued to split publishSplitSlots path - // into readStateManagerSplit.ts. - ["src/features/channels/readState/readStateManager.ts", 1030], - // review feedback on #1492 restored the two-line load-bearing comment - // documenting why `lastMessageAt` must not be an `activeReadAt` fallback - // (reply-inclusive; would clear unread state early). The file was already - // at the 1000 ceiling; comment-only overage, not code growth. Queued to - // split with the rest of this list. - // member-agent-flags: messageProfiles merge + ref stabilisation split out to - // useMessageProfiles.ts, ratcheting 1002 -> 972 (under the 1000 default; - // entry kept as a ratchet). +7 rebase onto main (#1698 timeline-window - // growth), 972 -> 979. - ["src/features/channels/ui/ChannelScreen.tsx", 979], - // forced-unread persistence: markChannelUnread now writes through to - // forcedUnreadStore (localStorage) so the sidebar badge survives reload and - // the rail observer can read it. Three clear points added (markChannelRead, - // markAllChannelsRead, drainSyncedAdvances). Load-bearing fix, not generic - // debt growth. Queued to split with the rest of this list. - ["src/features/channels/useUnreadChannels.ts", 1022], - // Shared UI was added to this guard after splitting globals/markdown so - // large shared renderers cannot grow further while follow-up splits land. - // +33 for config-nudge detect-and-render + author-auth gate (normalizePubkey guard). - ["src/shared/ui/markdown.tsx", 2152], - // +15 (2199 -> 2214): the video right-click Download/Copy menu's props, - // hook wiring, and render slot. The stateful menu logic (~52 lines) was - // extracted to useVideoContextMenu.tsx; what remains here is the component's - // public interface (downloadUrl/filename props) and cannot move out. - ["src/shared/ui/VideoPlayer.tsx", 2214], - ["src/shared/ui/sidebar.tsx", 1042], - // permission-outcome (fix #1381 regression): pendingPermissions state map, - // describePermissionOutcome helper, jsonRpcId key helper (handles both - // string and finite-number JSON-RPC ids per spec), and the acp_write - // response correlation branch are all tightly coupled to the existing - // request handler. Load-bearing logic growth, not generic debt. Queued to - // split into a dedicated permission module in the next transcript refactor. - // +123: observer parity — 4 new named session/update classifier cases - // (current_mode_update, usage_update, available_commands_update, - // config_option_update) + replaceLifecycleItem helper for usage coalescing + - // system-prompt ordering fix (turnId: null for per-channel items). - // +35: session/new reposition-on-refire fix — removeItem helper + - // upsertMetadata restart branch (remove+sealOpenMessages+push instead of - // replaceItem in-place) so system-prompt anchor moves to stream tail. - // Load-bearing feature growth; queued to split in next transcript refactor. - ["src/features/agents/ui/agentSessionTranscript.ts", 1202], - // catalog module; agent_models.rs retains the thin wrapper (~50 lines). - // File still exceeds 1000 due to OpenAI/Anthropic discovery + subprocess - // fallback. Queued to split into dedicated discovery modules. - // Kept activity-feed design fixture: realistic prompt context and tool-heavy - // chatter for render-class test/reference coverage. Queued to split with the - // rest of this list if it grows further. - // +2: baked build env folded under merged_env in both get_agent_models and - // discover_agent_models so in-process discovery sees baked provider config on - // a GUI-launched DMG (the discovery_env_with_baked_floor fold). - // +3: provider tri-state applied in update_managed_agent handler - // (if let Some(provider_update) = input.provider { record.provider = provider_update; }). - // +8: harness_override thread-through in update_managed_agent so a deliberate - // Custom pin routes to update_time_agent_command_override (comment + call). - // +22 (1079 -> 1101, main): Finding 2 — model discovery now resolves through - // resolve_effective_model_provider instead of raw record bytes, plus - // apply_model_provider_prompt_update's linked-instance write-guard - // extraction and its regression tests. - // +4 (1101 -> 1105): rebase onto agents-everywhere — agents.rs function - // signatures updated for ManagedAgentRuntimeKey-keyed runtimes map. - // +1 (#2773): model_discovery_error helper routes dangling-harness - // resolution errors through user_facing_harness_error (sentence, not raw - // DANGLING_HARNESS_ID sentinel) for the get_agent_models surface. The PR's - // descriptor path also deletes saved_agent_model_discovery_config, whose - // callers now use resolve_effective_model_provider + the descriptor env - // directly (net wash after the merge of the deltas above). - // +38 (1114 -> 1152): agent_model_discovery_config extracted as a pure, - // test-bindable seam (struct + helper + docs) so the linked-agent - // regression test kills the stale-record mutation at get_agent_models' - // consumption point (review finding, Wren + Dawn). - ["src-tauri/src/commands/agent_models.rs", 1152], - // global-agent-config: get_agent_config_surface / write_agent_config_field / - // put_agent_session_config commands + GlobalAgentConfig serde types. New file - // in this PR; queued to split with the command module refactor. - // +17: baked-env-global-unify: BUZZ_AGENT_THINKING_EFFORT added to - // is_safe_to_reveal allowlist + baked_env_thinking_effort_is_unmasked test. - // +1: doctor-install-reliability: login_hint: None added to goose_runtime test stub. - // +1: doctor-install-reliability review fixes: auth_probe_args: None added to stub. - // +11 (1021 -> 1032): agents-everywhere pair re-key — session-cache reads in - // get_agent_config_surface derive the ManagedAgentRuntimeKey (relay-URL - // fallback resolution) and put_agent_session_config gains a relay_url param. - // Load-bearing identity plumbing; queued to split. - // +18 (1032 -> 1050): review fix — put_agent_session_config reads the pair - // relay from the harness-attached payload relayUrl (with effective-relay - // fallback for older harnesses) instead of a required arg the frontend - // wrapper never passed, which silently broke the session-config cache. - // +60 (1050 -> 1110): agent-config-resolver — resolve_config_surface now - // clears a linked instance's own system_prompt/model/provider before - // computing had_* so stale materialized snapshot bytes can never be tagged - // BuzzExplicit and shadow the definition/global fallthrough; the dead - // persona-model re-tag branch replaced; two new regression tests added. - // +2 (1110 -> 1112): the agent_record and persona_with_model test fixtures - // each set the new mandatory `catalog_source` field. - ["src-tauri/src/commands/agent_config.rs", 1112], - // codex-install-auto-restart review-fixes: should_restart_after_install - // takes pid_alive:bool (pure predicate, no OS-dependent call); 3 racy - // cache tests replaced with 6 pure availability_drift predicate tests; - // dead-pid non-happy-path added. All load-bearing correctness fixes. - // (+17 lines net vs previous 1330 limit; rustfmt expanded some call sites) - // Git Bash Doctor discovery exposes a narrow async Tauri command at the - // existing discovery boundary. The ten-line addition preserves the platform - // neutral frontend contract; split remains queued. - // Windows Doctor install fix: resolve_install_shell() + install_shell_command() - // returns Result (Windows Git Bash resolution, CREATE_NO_WINDOW, taskkill timeout - // kill), cli_install_commands_for_os() callsite, unit tests for shell selection - // and per-OS install command accessor. Load-bearing Windows platform support. - // +53: pass 2 — three cfg(windows) install shell tests (resolve succeeds with - // Git, error hint content, install_shell_command succeeds). - // +8: install_shell_from pure seam extracted for deterministic testing. - // +287: is_powershell_command + install_powershell_command + build_install_command - // route PowerShell CLI installs natively on Windows (bypasses Git Bash PATH - // poisoning that resolved GNU tar instead of bsdtar → Codex install failure). - // Includes unit tests for detection, routing, and -Command body preservation. - // +16: test_powershell_command_goose_catalog_dequoted proves the \$→$ escape - // fix for the Goose Windows installer (PR #2680 interaction with #2750). - // +10: pass an explicit PATH through Codex adapter install planning so unit - // tests avoid the process-global login-shell PATH cache. - // +59 (main): run install commands under `pipefail` so a failing `curl` in a - // `curl … | bash` install fails the `cli` step instead of being masked by - // `bash`'s exit 0, plus tests for the arg shape and the real pipeline status. - // +81 (main): install_shell_args re-exports the composed PATH inside the command - // body so login startup files can't clear or reorder it, plus an isolated - // hostile-profile regression the pure composition tests structurally miss. - // +42 (main): gate that re-export off Windows, where join_paths is `;`-separated - // and bash would collapse it into one entry, plus a platform-shape test. - // +126 (#2773): BYOH — save_custom_harness (validate, atomic write, return - // entry) + delete_custom_harness (id-guard, builtin reject, remove file) - // commands; discover_acp_providers updated to pass AppHandle + - // custom_harnesses dir. - // +30: BYOH F5 — atomic-write-file dep, original_id rename/delete support. - // +13: BYOH pass-2 C1 — warm_harness_registry_from_dir call in save and - // delete commands now verifies transactional registry refresh. - // +2: BYOH pass-2 I2 env round-trip — definition_env carried through save - // return value so the frontend immediately has the updated env. - // +1: rebase over main (#2680) — requires_external_cli: false added to - // save_custom_harness catalog entry construction (new required field). - // -359: install command execution (spawn, output drain under timeout, retry - // with backoff, output truncation) extracted to agent_discovery/install_exec.rs - // alongside its tests, matching the managed_node.rs / post_install_verification.rs - // split. The entries above describe the file's history, not its current shape. - // +27: codex-acp minimum-version gate — test_plan_adapter_install_updates_older_ - // 1x_codex_binary pins that a 1.x adapter below the floor still plans a reinstall. - ["src-tauri/src/commands/agent_discovery.rs", 1835], - // draft-persistence predicate: submit-time `loadDraft` check + inline comment - // + deps-array entry in submitMessage closes the never-persisted-boundary - // defect (Thufir Pass-3 finding). Load-bearing correctness fix; queued to - // split MessageComposer into submit/edit/media sub-modules. - // +18: pendingImetaForPersistRef (local snapshot ref) + synchronous restore - // path writes in the draft-key effect body, fixing the image-drop bug on - // top-level nav switch (StrictMode simulate-unmount race on remount). - // +12 autoSubmitDraftKey/onAutoSubmitComplete props + onAutoSubmitCompleteRef - // + mount-only useEffect for the Drafts-panel "Send message" confirm-dialog - // flow. Load-bearing feature growth; queued to split with the rest of this - // list. - // +3: onLinkShortcutRef wiring (ref decl + editor option + assignment) for - // the ⌘K link-editor shortcut, mirroring the existing onEditLinkRef - // pattern. Queued to split with the rest of this list. - // +35: persistent audience scope/hook wiring and chip component handoff. The - // chip markup lives separately; remaining lines connect existing composer - // send state to the audience store. Queued with the existing split. - // +23: edit-to-add-mention notify (8ace8eed) — onEditSave/edit-branch - // mentionPubkeys threading + two snapshot refs (extractMentionPubkeys, - // ownerPubkey) feeding the newly-added-mentions diff. Diff logic itself - // lives in threading.ts (diffAddedMentionPubkeys); this is the minimal - // composer-side wiring. Queued to split with the rest of this list. - ["src/features/messages/ui/MessageComposer.tsx", 1114], - // global-agent-config: model-tuning section (BuzzAgentModelTuningFields via - // EditAgentAdvancedFields) + providerValid gate + effectiveProvider derivation - // + globalProvider threading into getPersonaProviderOptions. All load-bearing - // feature logic; queued to split with the rest of this list. - ["src/features/agents/ui/EditAgentDialog.tsx", 1088], - // global-agent-config rebase over #1639: AgentInstanceEditDialog (renamed from - // EditAgentDialog by #1639) gained initialFocus?/EditAgentFocusTarget prop - // threading from the deep-link focus feature, and isEditAgentProviderSaveValid - // extracted as a testable helper with originalRuntimeSupportsProvider to close - // the runtime-switch hole in Will's (b) providerValid gate narrowing. - // E2E-fix round: added globalProvider fallback to useRequiredCredentialState - // call site and buzz-agent auto-expand effect for model-tuning knob visibility. - // F1-fix: added globalEnvVars to useRequiredCredentialState so globally-satisfied - // credential keys are excluded from requiredEnvKeyMissing (display/gate parity). - // Feature logic, not generic debt. Approved override; still queued to split. - // +23 rebase onto #1667: behavioral quad fields (respond_to/parallelism/toolsets) - // plumbed through AgentInstanceEditDialog from PersonaAdvancedFields. - // +2 provider-aware effort: model/provider props threaded to BuzzAgentModelTuningFields. - // +15 provider/model dropdown fixes: useBakedBuildEnvKeysQuery + hideProviderIds - // for Databricks v1 gate; prospectiveRuntimeId default fallback for builtins. - // PR-B moves default/API-key derivation into shared hooks; the explicit - // hidden-key projection keeps the top-level secret out of Advanced rows. - // +6 (1195 -> 1201): rebase onto main — this PR's model-source label wiring - // lands on top of main's dialog growth. Queued to split. - ["src/features/agents/ui/AgentInstanceEditDialog.tsx", 1201], - // AgentDefinitionDialog grew past 1000 with the following load-bearing fixes: - // isRuntimeAutoSeededRef tracking for edit-mode seeding (Fizz shows models); - // runtimeSupportsLlmProviderSelection guard on discovery provider (codex fix); - // hideProviderIds computation for Databricks v1 gate. Queued to split. - ["src/features/agents/ui/AgentDefinitionDialog.tsx", 1035], - // #2630 emoji picker search: the shadow-root search-input autofocus effect - // (rAF retry loop) took this file 999 -> 1026 and landed without this entry, - // so main's Desktop Core went red. Queued to split with the rest of this list. - ["src/features/agents/ui/AgentCreationPreview.tsx", 1026], -]); - await runFileSizeCheck({ projectRoot, rules, - overrides, label: "Desktop", - scriptPath: "desktop/scripts/check-file-sizes.mjs", }); diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 08887a6bfb..35f4eae866 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -29,6 +29,8 @@ mod secret_store; mod shutdown; mod templates; mod util; +#[cfg(target_os = "linux")] +pub mod webkit_rendering; use app_state::{build_app_state, resolve_persisted_identity, AppState}; use builderlab::*; use commands::*; diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 13cb6c1b70..ebcc127683 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -2,5 +2,15 @@ #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] fn main() { + // Before anything else: WebKitGTK reads its rendering environment once at + // process start, and this is the only point where the process is still + // single threaded and no GTK object exists yet, which is what makes + // `std::env::set_var` sound. + #[cfg(target_os = "linux")] + if let Err(diagnostic) = buzz_lib::webkit_rendering::apply() { + eprintln!("buzz-desktop: {diagnostic}"); + std::process::exit(1); + } + buzz_lib::run() } diff --git a/desktop/src-tauri/src/webkit_rendering.rs b/desktop/src-tauri/src/webkit_rendering.rs new file mode 100644 index 0000000000..905da5eeed --- /dev/null +++ b/desktop/src-tauri/src/webkit_rendering.rs @@ -0,0 +1,208 @@ +//! WebKit rendering workarounds for Linux, applied before WebKit initializes. +//! +//! WebKitGTK's dmabuf renderer aborts the web process during startup on some +//! GPU/driver/compositor combinations, so Buzz comes up with no window at all +//! and the user has no way to fix it (#2338, upstream tauri#9394). Setting +//! `WEBKIT_DISABLE_DMABUF_RENDERER=1` avoids the abort by falling back to the +//! shared-memory buffer path. +//! +//! WebKit reads each of these variables exactly once per process, so the choice +//! has to be made before anything initializes — there is no runtime toggle and +//! no second chance later in the same process. This module therefore decides +//! from cheap preflight signals instead of reacting to a crash: +//! +//! * an NVIDIA GPU, the driver family behind most upstream reports; and +//! * AppImage packaging, where linuxdeploy's AppRun hook pins `GDK_BACKEND=x11` +//! and the dmabuf renderer buys nothing on that XWayland path (#2338). +//! +//! `--safe-rendering` is the manual escape hatch for a machine neither signal +//! recognises; it also disables accelerated compositing, for that launch only. +//! +//! This is the shape the Tauri ecosystem converged on: clash-verge-rev's +//! `utils/linux/workarounds.rs` and screenpipe's `linux_webkit_env.rs` both set +//! the same variable from the same signals at the same point in startup. + +use std::ffi::{OsStr, OsString}; +use std::path::Path; + +/// Force the safest rendering configuration for this launch. +const SAFE_RENDERING: &str = "--safe-rendering"; + +/// PCI vendor ID reported by NVIDIA devices under `/sys/class/drm`. +const NVIDIA_PCI_VENDOR: &str = "0x10de"; + +/// Where DRM devices advertise their PCI vendor. +const DRM_ROOT: &str = "/sys/class/drm"; + +/// Drops the zero-copy dmabuf buffer path. The workaround for #2338. +const DISABLE_DMABUF: &str = "WEBKIT_DISABLE_DMABUF_RENDERER"; +/// Drops accelerated compositing as well. `--safe-rendering` only. +const DISABLE_COMPOSITING: &str = "WEBKIT_DISABLE_COMPOSITING_MODE"; + +/// What the heuristic applies: the #2338 workaround alone, matching the +/// ecosystem precedents. `DISABLE_COMPOSITING` is deliberately not here — no +/// report has isolated it as necessary, and it costs more rendering than this. +const HEURISTIC: [&str; 1] = [DISABLE_DMABUF]; + +/// What `--safe-rendering` applies, which is also every variable this module may +/// set and therefore every variable a user assignment takes away from it. Being +/// the same list is the invariant: nothing outside it is ever written, so a user +/// value for any other WebKit variable is not a conflict. +const OWNED: [&str; 2] = [DISABLE_DMABUF, DISABLE_COMPOSITING]; + +/// Reads one environment variable. Injected so the decision is testable without +/// mutating the process environment. `OsString` rather than `String` because +/// presence is the test — a non-UTF-8 assignment is still the user's. +type EnvLookup<'a> = &'a dyn Fn(&str) -> Option; + +/// What this launch should do about its rendering environment. +#[derive(Debug, PartialEq, Eq)] +enum Plan { + /// Set each of these to `1`, then report `why`. + Apply { + vars: &'static [&'static str], + why: String, + }, + /// Change nothing, and report `why`. + Leave { why: String }, + /// The request cannot be delivered. Report it and exit non-zero rather than + /// starting an app that silently ignores what the user asked for. + Fatal { diagnostic: String }, +} + +/// Applies the workaround for this launch. +/// +/// Must be called from `main()` before `crate::run()`: WebKit memoizes these +/// variables at process start, and `std::env::set_var` is only sound while the +/// process is still single threaded, which it is nowhere else in Buzz. +/// +/// `Err` carries a user-facing diagnostic; the caller reports it and exits. +pub fn apply() -> Result<(), String> { + match plan( + std::env::args_os(), + &|key| std::env::var_os(key), + Path::new(DRM_ROOT), + ) { + Plan::Apply { vars, why } => { + for var in vars { + // Safe here and only here — see the doc comment above. + std::env::set_var(var, "1"); + } + let applied: Vec = vars.iter().map(|var| format!("{var}=1")).collect(); + eprintln!("buzz-desktop: {} — {why}", applied.join(" ")); + Ok(()) + } + Plan::Leave { why } => { + eprintln!("buzz-desktop: WebKit rendering left as-is — {why}"); + Ok(()) + } + Plan::Fatal { diagnostic } => Err(diagnostic), + } +} + +/// The whole decision, as a pure function of argv, the environment, and the DRM +/// device tree. +fn plan( + args: impl IntoIterator>, + env: EnvLookup<'_>, + drm_root: &Path, +) -> Plan { + let safe_rendering = args + .into_iter() + .any(|arg| arg.as_ref() == OsStr::new(SAFE_RENDERING)); + let user_set = user_set(env); + + if !user_set.is_empty() { + // A user who has assigned one of these has taken over the decision, so + // the heuristic stands down wholesale — writing the *other* variable + // behind their back would be exactly the surprise they opted out of. + return match safe_rendering { + // Two incompatible answers to one question, and no basis for + // picking: honouring the flag would overwrite configuration the + // user typed, honouring the environment would silently ignore a + // rescue flag from a user whose app does not start. + true => Plan::Fatal { + diagnostic: conflict(&user_set), + }, + false => Plan::Leave { + why: format!("{} set in the environment", describe(&user_set)), + }, + }; + } + + if safe_rendering { + return Plan::Apply { + vars: &OWNED, + why: format!("{SAFE_RENDERING} requested, this launch only"), + }; + } + + let signals = [ + (nvidia_gpu(drm_root), "NVIDIA GPU"), + (env("APPIMAGE").is_some(), "AppImage"), + ]; + let hits: Vec<&str> = signals + .iter() + .filter_map(|(hit, label)| hit.then_some(*label)) + .collect(); + + match hits.is_empty() { + true => Plan::Leave { + why: "no NVIDIA GPU and not an AppImage".to_string(), + }, + false => Plan::Apply { + vars: &HEURISTIC, + why: hits.join(", "), + }, + } +} + +/// Owned variables the environment already carries, keyed by name. +/// +/// Presence is the test, not truthiness: `VAR=0` and `VAR=` are both genuine +/// user assignments, and both take the decision away from this module. +fn user_set(env: EnvLookup<'_>) -> Vec<(&'static str, OsString)> { + OWNED + .iter() + .filter_map(|key| env(key).map(|value| (*key, value))) + .collect() +} + +/// User assignments rendered as `KEY=value`, for a log line or a diagnostic. +fn describe(user_set: &[(&str, OsString)]) -> String { + let shown: Vec = user_set + .iter() + .map(|(key, value)| format!("{key}={}", value.to_string_lossy())) + .collect(); + shown.join(", ") +} + +/// Whether any DRM device reports NVIDIA's PCI vendor ID. An unreadable device +/// tree is not a hit — the workaround has a real cost, so it needs evidence. +fn nvidia_gpu(drm_root: &Path) -> bool { + let Ok(entries) = std::fs::read_dir(drm_root) else { + return false; + }; + entries.flatten().any(|entry| { + std::fs::read_to_string(entry.path().join("device/vendor")) + .is_ok_and(|vendor| vendor.trim().eq_ignore_ascii_case(NVIDIA_PCI_VENDOR)) + }) +} + +/// The diagnostic for `--safe-rendering` against a user-set owned variable. +/// +/// The message both shows what is set and names the keys to unset — the two +/// things a user whose app will not start needs in order to act on it. +fn conflict(user_set: &[(&str, OsString)]) -> String { + let keys: Vec<&str> = user_set.iter().map(|(key, _)| *key).collect(); + format!( + "{SAFE_RENDERING} cannot be applied: {} already set in the environment. \ + Either unset {} and run {SAFE_RENDERING} again, or keep that \ + environment and drop the flag.", + describe(user_set), + keys.join(", "), + ) +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/webkit_rendering/tests.rs b/desktop/src-tauri/src/webkit_rendering/tests.rs new file mode 100644 index 0000000000..5be1612b21 --- /dev/null +++ b/desktop/src-tauri/src/webkit_rendering/tests.rs @@ -0,0 +1,250 @@ +//! Behaviour of the preflight decision. +//! +//! Every case goes through `plan`, which takes argv, the environment, and the +//! DRM root as arguments — so nothing here mutates the process environment and +//! the tests are order-independent. + +use super::*; + +const NO_ARGS: [&str; 0] = []; + +/// A `/sys/class/drm` stand-in. `vendors` are written as `card/device/vendor` +/// with the trailing newline the kernel emits. +fn drm(vendors: &[&str]) -> tempfile::TempDir { + let root = tempfile::tempdir().expect("tempdir"); + for (index, vendor) in vendors.iter().enumerate() { + let device = root.path().join(format!("card{index}")).join("device"); + std::fs::create_dir_all(&device).expect("device dir"); + std::fs::write(device.join("vendor"), format!("{vendor}\n")).expect("vendor"); + } + root +} + +fn env_from(pairs: &[(&str, &str)]) -> impl Fn(&str) -> Option { + let owned: Vec<(String, OsString)> = pairs + .iter() + .map(|(key, value)| (key.to_string(), OsString::from(value))) + .collect(); + move |key| { + owned + .iter() + .find(|(candidate, _)| candidate == key) + .map(|(_, value)| value.clone()) + } +} + +/// The variables a plan would set, or `None` for a plan that sets nothing. +fn applied(plan: &Plan) -> Option<&[&str]> { + match plan { + Plan::Apply { vars, .. } => Some(vars), + _ => None, + } +} + +// ── Detection ─────────────────────────────────────────────────────────────── + +#[test] +fn test_nvidia_gpu_disables_the_dmabuf_renderer() { + let drm = drm(&["0x10de"]); + let plan = plan(NO_ARGS, &env_from(&[]), drm.path()); + + assert_eq!( + applied(&plan), + Some(&["WEBKIT_DISABLE_DMABUF_RENDERER"][..]) + ); + let Plan::Apply { why, .. } = &plan else { + unreachable!() + }; + assert!(why.contains("NVIDIA"), "{why}"); +} + +#[test] +fn test_an_nvidia_gpu_alongside_another_vendor_still_counts() { + // Hybrid graphics: the integrated GPU enumerates first, and WebKit may + // still land on the discrete one. + let drm = drm(&["0x8086", "0x10de"]); + + assert_eq!( + applied(&plan(NO_ARGS, &env_from(&[]), drm.path())), + Some(&["WEBKIT_DISABLE_DMABUF_RENDERER"][..]) + ); +} + +#[test] +fn test_the_vendor_id_match_ignores_case() { + let drm = drm(&["0x10DE"]); + + assert_eq!( + applied(&plan(NO_ARGS, &env_from(&[]), drm.path())), + Some(&["WEBKIT_DISABLE_DMABUF_RENDERER"][..]) + ); +} + +#[test] +fn test_an_appimage_launch_disables_the_dmabuf_renderer() { + // No NVIDIA GPU: the AppImage signal has to carry this on its own, which is + // #2338's reporter (Intel Mesa under the AppRun's pinned XWayland backend). + let drm = drm(&["0x8086"]); + let env = env_from(&[("APPIMAGE", "/home/u/Buzz.AppImage")]); + let plan = plan(NO_ARGS, &env, drm.path()); + + assert_eq!( + applied(&plan), + Some(&["WEBKIT_DISABLE_DMABUF_RENDERER"][..]) + ); + let Plan::Apply { why, .. } = &plan else { + unreachable!() + }; + assert!(why.contains("AppImage"), "{why}"); +} + +#[test] +fn test_a_plain_non_nvidia_launch_changes_nothing() { + let drm = drm(&["0x8086", "0x1002"]); + + assert!(matches!( + plan(NO_ARGS, &env_from(&[]), drm.path()), + Plan::Leave { .. } + )); +} + +#[test] +fn test_an_unreadable_drm_tree_is_not_treated_as_a_hit() { + // Containers and hardened kernels can hide `/sys/class/drm` entirely. The + // workaround costs real rendering performance, so absent evidence is not + // evidence — this must not become an unconditional export. + let missing = std::path::Path::new("/nonexistent/class/drm"); + + assert!(matches!( + plan(NO_ARGS, &env_from(&[]), missing), + Plan::Leave { .. } + )); +} + +#[test] +fn test_a_device_without_a_vendor_file_is_skipped_not_fatal() { + // `/sys/class/drm` also contains connector entries (`card0-HDMI-A-1`) and + // `renderD*` nodes, which have no `device/vendor` under them. + let root = tempfile::tempdir().expect("tempdir"); + std::fs::create_dir_all(root.path().join("card0-HDMI-A-1")).expect("connector"); + let device = root.path().join("card1").join("device"); + std::fs::create_dir_all(&device).expect("device dir"); + std::fs::write(device.join("vendor"), "0x10de\n").expect("vendor"); + + assert_eq!( + applied(&plan(NO_ARGS, &env_from(&[]), root.path())), + Some(&["WEBKIT_DISABLE_DMABUF_RENDERER"][..]) + ); +} + +// ── User environment ──────────────────────────────────────────────────────── + +#[test] +fn test_a_user_set_variable_disables_the_heuristic_wholesale() { + // `0` is the value a truthiness check would drop: the user is asking for the + // dmabuf renderer *on*, on a machine the heuristic would have opted out. + let drm = drm(&["0x10de"]); + let env = env_from(&[(DISABLE_DMABUF, "0")]); + let plan = plan(NO_ARGS, &env, drm.path()); + + let Plan::Leave { why } = &plan else { + panic!("a user assignment must not be overwritten: {plan:?}"); + }; + assert!(why.contains("WEBKIT_DISABLE_DMABUF_RENDERER=0"), "{why}"); +} + +#[test] +fn test_an_empty_assignment_is_still_a_user_assignment() { + let drm = drm(&["0x10de"]); + let env = env_from(&[(DISABLE_DMABUF, "")]); + + assert!(matches!( + plan(NO_ARGS, &env, drm.path()), + Plan::Leave { .. } + )); +} + +#[test] +fn test_a_user_set_compositing_variable_also_stands_the_heuristic_down() { + // The heuristic never sets this one, but it is still ours to set under + // `--safe-rendering`, so a user value takes the whole decision away rather + // than leaving us free to write the sibling variable. + let drm = drm(&["0x10de"]); + let env = env_from(&[(DISABLE_COMPOSITING, "1")]); + + assert!(matches!( + plan(NO_ARGS, &env, drm.path()), + Plan::Leave { .. } + )); +} + +// ── --safe-rendering ──────────────────────────────────────────────────────── + +#[test] +fn test_safe_rendering_applies_the_safest_set_without_any_hardware_signal() { + // The escape hatch exists for the machine neither signal recognises, so it + // must not depend on either one. + let drm = drm(&["0x8086"]); + let args = ["buzz://channel/1", SAFE_RENDERING]; + let plan = plan(args, &env_from(&[]), drm.path()); + + assert_eq!( + applied(&plan), + Some( + &[ + "WEBKIT_DISABLE_DMABUF_RENDERER", + "WEBKIT_DISABLE_COMPOSITING_MODE" + ][..] + ) + ); +} + +#[test] +fn test_an_unrelated_flag_is_not_mistaken_for_safe_rendering() { + let drm = drm(&["0x8086"]); + + assert!(matches!( + plan(["--safe-renderingX"], &env_from(&[]), drm.path()), + Plan::Leave { .. } + )); +} + +#[test] +fn test_safe_rendering_against_a_user_set_variable_is_fatal_not_guessed() { + let drm = drm(&["0x8086"]); + let env = env_from(&[(DISABLE_DMABUF, "0")]); + let plan = plan([SAFE_RENDERING], &env, drm.path()); + + let Plan::Fatal { diagnostic } = &plan else { + panic!("the flag and the environment disagree; neither may be guessed: {plan:?}"); + }; + // The message has to name what is set and what to unset, or the user whose + // app will not start cannot act on it. + assert!(diagnostic.contains(SAFE_RENDERING), "{diagnostic}"); + assert!( + diagnostic.contains("WEBKIT_DISABLE_DMABUF_RENDERER=0"), + "{diagnostic}" + ); +} + +#[test] +fn test_a_non_utf8_user_assignment_is_reported_not_ignored() { + // Presence is the test, so this still stands the heuristic down; the + // diagnostic must name the key rather than dropping the whole entry. + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + + let drm = drm(&["0x10de"]); + let invalid = OsString::from_vec(vec![0xff, 0xfe]); + let env = |key: &str| match key == DISABLE_DMABUF { + true => Some(invalid.clone()), + false => None, + }; + + let Plan::Fatal { diagnostic } = plan([SAFE_RENDERING], &env, drm.path()) else { + panic!("a non-UTF-8 assignment is still a user assignment"); + }; + assert!(diagnostic.contains(DISABLE_DMABUF), "{diagnostic}"); + } +} diff --git a/desktop/src/features/agents/ui/AddCustomHarnessDialog.tsx b/desktop/src/features/agents/ui/AddCustomHarnessDialog.tsx new file mode 100644 index 0000000000..0c84e99275 --- /dev/null +++ b/desktop/src/features/agents/ui/AddCustomHarnessDialog.tsx @@ -0,0 +1,47 @@ +import { CustomHarnessForm } from "@/features/settings/ui/CustomHarnessForm"; +import { ChooserDialogContent } from "@/shared/ui/chooser-dialog-content"; +import { Dialog } from "@/shared/ui/dialog"; + +/** + * Registers a custom ACP harness from inside an agent dialog, so "New agent" + * is a complete entry point and not a dead end that sends the user to + * Settings. Hosts the same `CustomHarnessForm` the harness catalog uses. + */ +export function AddCustomHarnessDialog({ + onOpenChange, + onSaved, + open, +}: { + onOpenChange: (open: boolean) => void; + /** Called with the id of the harness that was just registered. */ + onSaved: (id: string) => void; + open: boolean; +}) { + return ( + + + + Register any ACP-speaking agent tool as a selectable harness. +

+ } + onCancel={() => onOpenChange(false)} + onSaved={(id) => { + // Dismiss on save as well as cancel — both exits belong to this + // dialog, so callers only handle the resulting selection. + onOpenChange(false); + onSaved(id); + }} + /> + + + ); +} diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index 45031e0371..5425131448 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -83,6 +83,12 @@ import { import { useProviderApiKeyFieldState } from "./providerApiKeyFieldState"; import { buildRuntimeModelProviderPayload } from "./agentDefinitionSubmitPayload"; import { AgentDefinitionDialogFooter } from "./AgentDefinitionDialogFooter"; +import { AddCustomHarnessDialog } from "./AddCustomHarnessDialog"; +import { + ADD_CUSTOM_HARNESS_OPTION, + runtimeDropdownAction, + usePendingHarnessSelection, +} from "./addCustomHarness"; type AgentDefinitionDialogProps = { open: boolean; @@ -167,6 +173,7 @@ export function AgentDefinitionDialog({ const [isAvatarUploadPending, setIsAvatarUploadPending] = React.useState(false); const [hasUserChanges, setHasUserChanges] = React.useState(false); + const [isAddHarnessOpen, setIsAddHarnessOpen] = React.useState(false); const { globalConfig, inheritedDefaults: { @@ -308,6 +315,7 @@ export function AgentDefinitionDialog({ setShowAdvancedFields(false); setIsAvatarUploadPending(false); setHasUserChanges(false); + setIsAddHarnessOpen(false); // isRuntimeAutoSeededRef and hasSeededForOpenRef are NOT reset here — the // [initialValues, open] effect resets both when the dialog re-opens. } @@ -578,6 +586,7 @@ export function AgentDefinitionDialog({ runtimes, runtimesLoading, }); + runtimeDropdownOptions.push(ADD_CUSTOM_HARNESS_OPTION); const runtimeSummaryLabel = selectedRuntime ? formatRuntimeOptionLabel(selectedRuntime) : runtime.trim() || "Not configured"; @@ -662,9 +671,13 @@ export function AgentDefinitionDialog({ } function handleRuntimeDropdownChange(nextValue: string) { + const action = runtimeDropdownAction(nextValue); + if (action.kind === "add-custom-harness") { + setIsAddHarnessOpen(true); + return; + } setHasUserChanges(true); - const nextRuntime = - nextValue === NO_RUNTIME_DROPDOWN_VALUE ? "" : nextValue; + const nextRuntime = action.runtimeId; // The user made an explicit choice — no longer auto-seeded. isRuntimeAutoSeededRef.current = false; setRuntime(nextRuntime); @@ -680,6 +693,15 @@ export function AgentDefinitionDialog({ ); } + // Routed through the normal change handler so a harness registered inline + // resets model/provider exactly as a hand-picked one would. Scoped to `open` + // so a pending id can't outlive the dialog that started the registration. + const selectSavedHarness = usePendingHarnessSelection( + runtimes, + handleRuntimeDropdownChange, + open, + ); + function handleProviderDropdownChange(nextValue: string) { setHasUserChanges(true); const nextProvider = @@ -944,6 +966,12 @@ export function AgentDefinitionDialog({ returnFocusRef={aiDefaultsTriggerRef} /> + + {isCreateMode ? createRunSection : null}
diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 601d57f95d..f3c410e2ff 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -83,6 +83,12 @@ import { useProviderApiKeyFieldState } from "./providerApiKeyFieldState"; import { resolveModelFieldStatusMessage } from "./agentConfigControls"; import { AdvancedRequiredBadge } from "./AdvancedRequiredBadge"; import { showAgentProfileSyncWarning } from "./agentProfileSyncWarning"; +import { AddCustomHarnessDialog } from "./AddCustomHarnessDialog"; +import { + ADD_CUSTOM_HARNESS_OPTION, + runtimeDropdownAction, + usePendingHarnessSelection, +} from "./addCustomHarness"; const ADVANCED_FIELDS_MOTION_TRANSITION = { duration: 0.18, @@ -157,6 +163,7 @@ export function AgentInstanceEditDialog({ const [avatarUrl, setAvatarUrl] = React.useState(agent.avatarUrl ?? ""); const [isAvatarUploadPending, setIsAvatarUploadPending] = React.useState(false); + const [isAddHarnessOpen, setIsAddHarnessOpen] = React.useState(false); const shouldReduceMotion = useReducedMotion(); // Runtime selector: defaults to "custom" until the dialog opens and the @@ -191,6 +198,7 @@ export function AgentInstanceEditDialog({ setAvatarUrl(agent.avatarUrl ?? ""); setShowAdvancedFields(false); setIsAvatarUploadPending(false); + setIsAddHarnessOpen(false); runtimeTouched.current = false; const matched = runtimes.find((r) => r.command?.trim() === agent.agentCommand.trim()) ?? @@ -244,6 +252,7 @@ export function AgentInstanceEditDialog({ value: selectedRuntimeId, }); } + options.push(ADD_CUSTOM_HARNESS_OPTION); return options; }, [sortedRuntimes, selectedRuntimeId]); @@ -484,8 +493,12 @@ export function AgentInstanceEditDialog({ } function handleRuntimeDropdownChange(nextValue: string) { - const nextRuntimeId = - nextValue === NO_RUNTIME_DROPDOWN_VALUE ? "" : nextValue; + const action = runtimeDropdownAction(nextValue); + if (action.kind === "add-custom-harness") { + setIsAddHarnessOpen(true); + return; + } + const nextRuntimeId = action.runtimeId; const previousRuntimeId = selectedRuntimeId; const nextRuntime = runtimes.find((r) => r.id === nextRuntimeId); @@ -532,6 +545,16 @@ export function AgentInstanceEditDialog({ ); } + // Routed through the normal change handler so a harness registered inline + // pins its command and resets model/provider like a hand-picked one. Scoped + // to `open` so a pending id can't outlive the dialog that started the + // registration. + const selectSavedHarness = usePendingHarnessSelection( + runtimes, + handleRuntimeDropdownChange, + open, + ); + function handleProviderDropdownChange(nextValue: string) { const nextProvider = nextValue === AUTO_PROVIDER_DROPDOWN_VALUE ? "" : nextValue; @@ -949,6 +972,11 @@ export function AgentInstanceEditDialog({

) : null} +
{selectedRuntimeId === "custom" && !inheritHarness ? (
diff --git a/desktop/src/features/agents/ui/addCustomHarness.test.mjs b/desktop/src/features/agents/ui/addCustomHarness.test.mjs new file mode 100644 index 0000000000..6c0aa32daf --- /dev/null +++ b/desktop/src/features/agents/ui/addCustomHarness.test.mjs @@ -0,0 +1,344 @@ +/** + * Behavior tests for the inline "Add custom harness…" dropdown entry shared by + * AgentDefinitionDialog and AgentInstanceEditDialog. + * + * Two seams carry the feature, and both are pinned here: + * + * 1. ROUTING (`runtimeDropdownAction`) — the sentinel must resolve to "open + * the form", never to a selection. If it ever resolved to a selection the + * dialogs would write "\u0000add-custom-harness" into `runtime` and try to + * spawn an agent on a harness that does not exist. + * 2. DEFERRED SELECTION (`usePendingHarnessSelection`) — saving only writes + * the definition file; the harness becomes a catalog entry when the + * invalidated discovery query refetches. Selecting on save would pick an + * id no entry backs. The hook must wait for the catalog, fire exactly + * once, stay silent when the user cancels, and drop the pending id when + * its dialog closes — the host dialogs stay mounted, so a stale id would + * otherwise select into reset form state on a later publish. + * + * The hook is mounted for real (react-dom/client + act) rather than simulated, + * so its effect wiring — including the guard that survives the dialogs' + * non-memoized change handlers — is what gets tested. + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +// ── Minimal DOM shim ───────────────────────────────────────────────────────── +// react-dom/client needs a container element and a document; node has neither. +// The harness renders null, so no real node operations are exercised. + +class ElementShim { + constructor() { + this.children = []; + this.childNodes = []; + this.nodeType = 1; + this.nodeName = "DIV"; + this.tagName = "DIV"; + this.namespaceURI = "http://www.w3.org/1999/xhtml"; + } + get ownerDocument() { + return globalThis.document; + } + addEventListener() {} + removeEventListener() {} + appendChild(child) { + this.children.push(child); + this.childNodes.push(child); + return child; + } + removeChild(child) { + this.children = this.children.filter((current) => current !== child); + this.childNodes = this.childNodes.filter((current) => current !== child); + return child; + } + insertBefore(child) { + return this.appendChild(child); + } + contains(target) { + return this === target; + } +} + +globalThis.document = { + activeElement: null, + addEventListener() {}, + createElement: () => new ElementShim(), + get defaultView() { + return globalThis.window; + }, + nodeType: 9, + removeEventListener() {}, +}; +// react-dom derives update priority from window.event and walks iframe +// boundaries via window.HTMLIFrameElement during commit. +Object.defineProperty(globalThis, "window", { + configurable: true, + value: { + addEventListener() {}, + document: globalThis.document, + event: undefined, + HTMLIFrameElement: ElementShim, + removeEventListener() {}, + }, +}); +globalThis.HTMLElement = ElementShim; +globalThis.Node = ElementShim; +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +import React from "react"; +import { act } from "react"; +import { createRoot } from "react-dom/client"; + +import { NO_RUNTIME_DROPDOWN_VALUE } from "./agentConfigOptions.tsx"; +import { + ADD_CUSTOM_HARNESS_OPTION, + ADD_CUSTOM_HARNESS_VALUE, + readyHarnessId, + runtimeDropdownAction, + usePendingHarnessSelection, +} from "./addCustomHarness.ts"; + +// ── Routing: the sentinel opens the form, it is never a selection ──────────── + +test("selecting the add-custom entry requests the form and yields no runtime id", () => { + const action = runtimeDropdownAction(ADD_CUSTOM_HARNESS_VALUE); + assert.equal(action.kind, "add-custom-harness"); + // The dialogs read `action.runtimeId` on the select branch; the sentinel + // must not carry one, or it could leak into form state. + assert.equal("runtimeId" in action, false); +}); + +test("selecting a harness yields that harness id", () => { + assert.deepEqual(runtimeDropdownAction("my-harness"), { + kind: "select", + runtimeId: "my-harness", + }); +}); + +test("selecting the no-runtime entry yields the empty id", () => { + assert.deepEqual(runtimeDropdownAction(NO_RUNTIME_DROPDOWN_VALUE), { + kind: "select", + runtimeId: "", + }); +}); + +test("the add-custom sentinel cannot collide with a backend-valid harness id", () => { + // Backend ids match [a-z0-9_][a-z0-9_-]* (custom_harnesses.rs), so a + // NUL-prefixed value is unreachable as a real id. + assert.equal(ADD_CUSTOM_HARNESS_VALUE.startsWith("\u0000"), true); + assert.equal(ADD_CUSTOM_HARNESS_OPTION.value, ADD_CUSTOM_HARNESS_VALUE); + assert.equal(ADD_CUSTOM_HARNESS_OPTION.label, "Add custom harness…"); +}); + +// ── Readiness: an id is selectable only once the catalog publishes it ──────── + +test("a pending id absent from the catalog is not ready", () => { + assert.equal(readyHarnessId([{ id: "claude" }], "my-harness"), null); +}); + +test("a pending id present in the catalog is ready", () => { + assert.equal( + readyHarnessId([{ id: "claude" }, { id: "my-harness" }], "my-harness"), + "my-harness", + ); +}); + +test("no pending id is never ready even against a populated catalog", () => { + assert.equal(readyHarnessId([{ id: "claude" }], null), null); +}); + +// ── Deferred selection: mounted hook ───────────────────────────────────────── + +/** + * Mount the real hook over a mutable catalog. Returns the setter the dialogs + * call on save, a `setRuntimes` to simulate the discovery refetch, a `setOpen` + * to simulate the owning dialog closing and reopening, and the log of ids the + * hook handed back for selection. + */ +async function mountPendingSelection(initialRuntimes = []) { + const selected = []; + const control = {}; + + function Harness() { + const [runtimes, setRuntimes] = React.useState(initialRuntimes); + const [open, setOpen] = React.useState(true); + // Deliberately NOT memoized: both dialogs pass a plain function + // declaration, so `onReady` has a fresh identity on every render. + const onReady = (id) => selected.push(id); + control.save = usePendingHarnessSelection(runtimes, onReady, open); + control.setRuntimes = setRuntimes; + control.setOpen = setOpen; + return null; + } + + const root = createRoot(new ElementShim()); + await act(async () => { + root.render(React.createElement(Harness)); + }); + return { control, root, selected }; +} + +test("saving a harness selects it only once the catalog publishes it", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // Save returns before discovery refetches — nothing to select yet. + await act(async () => control.save("my-harness")); + assert.deepEqual(selected, []); + + // The invalidated discovery query resolves with the new entry. + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + assert.deepEqual(selected, ["my-harness"]); + + await act(async () => root.unmount()); +}); + +test("a published harness is selected exactly once across later catalog updates", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + await act(async () => control.save("my-harness")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + assert.deepEqual(selected, ["my-harness"]); + + // Any later refetch re-renders with a new array identity and a new onReady + // identity. Re-firing here would clobber a selection the user made in + // between, so the pending id must have been cleared. + await act(async () => + control.setRuntimes([ + { id: "claude" }, + { id: "my-harness" }, + { id: "codex" }, + ]), + ); + assert.deepEqual(selected, ["my-harness"]); + + await act(async () => root.unmount()); +}); + +test("cancelling the form leaves the current selection untouched", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // Cancel never reports a saved id, so no selection is ever requested — even + // as the catalog keeps refreshing underneath. + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "codex" }]), + ); + assert.deepEqual(selected, []); + + await act(async () => root.unmount()); +}); + +test("a saved harness discovery never publishes is never selected", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // e.g. the definition file was written but the entry failed to load. The + // hook must stall rather than select an id no catalog entry backs. + await act(async () => control.save("ghost-harness")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "codex" }]), + ); + assert.deepEqual(selected, []); + + await act(async () => root.unmount()); +}); + +test("two harnesses registered in a row are each selected when published", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + await act(async () => control.save("first")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "first" }]), + ); + await act(async () => control.save("second")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "first" }, { id: "second" }]), + ); + assert.deepEqual(selected, ["first", "second"]); + + await act(async () => root.unmount()); +}); + +test("a second save before the first publishes selects only the later harness", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // The dropdown holds one harness, so the latest registration wins: the + // first id is dropped rather than queued behind the second. + await act(async () => control.save("first")); + await act(async () => control.save("second")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "first" }, { id: "second" }]), + ); + assert.deepEqual(selected, ["second"]); + + await act(async () => root.unmount()); +}); + +// ── Lifecycle: a pending id never outlives the dialog that created it ──────── + +test("a harness published after its dialog closed is never selected", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // Both host dialogs stay mounted when closed, so the hook keeps running. + await act(async () => control.save("my-harness")); + await act(async () => control.setOpen(false)); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + + // Selecting here would write into form state the close already reset. + assert.deepEqual(selected, []); + + await act(async () => root.unmount()); +}); + +test("reopening after closing mid-registration does not select the abandoned harness", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + await act(async () => control.save("my-harness")); + await act(async () => control.setOpen(false)); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + // The reopened dialog seeds from its own initial values; a stale pending id + // must not overwrite them. + await act(async () => control.setOpen(true)); + assert.deepEqual(selected, []); + + await act(async () => root.unmount()); +}); + +test("a harness saved after reopening is still selected when published", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + await act(async () => control.setOpen(false)); + await act(async () => control.setOpen(true)); + await act(async () => control.save("my-harness")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + assert.deepEqual(selected, ["my-harness"]); + + await act(async () => root.unmount()); +}); diff --git a/desktop/src/features/agents/ui/addCustomHarness.ts b/desktop/src/features/agents/ui/addCustomHarness.ts new file mode 100644 index 0000000000..f9c2143530 --- /dev/null +++ b/desktop/src/features/agents/ui/addCustomHarness.ts @@ -0,0 +1,104 @@ +/** + * Shared pieces of the inline "Add custom harness…" entry the agent dialogs + * append to their harness dropdown. + * + * Registering a custom harness used to be reachable only from Settings, so + * anyone whose first stop was "New agent" never learned the path existed. + * These helpers keep the entry identical across the dropdowns, keep its + * sentinel value out of form state, and defer selecting a freshly registered + * harness until discovery has actually published it. + */ + +import * as React from "react"; + +import { + NO_RUNTIME_DROPDOWN_VALUE, + type PersonaDropdownOption, +} from "./agentConfigOptions"; + +/** + * Dropdown value for the add-custom-harness entry. NUL-prefixed so it can + * never collide with a harness id (`[a-z0-9_][a-z0-9_-]*`) — same trick as the + * harness catalog's `CUSTOM_ENTRY_ID`. + */ +export const ADD_CUSTOM_HARNESS_VALUE = "\u0000add-custom-harness"; + +export const ADD_CUSTOM_HARNESS_OPTION: PersonaDropdownOption = { + label: "Add custom harness…", + value: ADD_CUSTOM_HARNESS_VALUE, +}; + +export type RuntimeDropdownAction = + | { kind: "add-custom-harness" } + | { kind: "select"; runtimeId: string }; + +/** + * Route a harness-dropdown change. The add-custom entry only opens the + * registration form — it is never a selection, so its sentinel can't reach + * form state. Every other value selects, with the no-runtime sentinel + * normalized to the empty id. + */ +export function runtimeDropdownAction(value: string): RuntimeDropdownAction { + if (value === ADD_CUSTOM_HARNESS_VALUE) { + return { kind: "add-custom-harness" }; + } + return { + kind: "select", + runtimeId: value === NO_RUNTIME_DROPDOWN_VALUE ? "" : value, + }; +} + +/** + * The pending harness id once discovery has published it, else `null`. + * + * Saving only writes the definition file — the harness becomes a catalog entry + * when the invalidated discovery query refetches. Selecting before then would + * pick an id no entry backs: the create dialog would block Save on an unknown + * availability, and the instance dialog could not read the command to pin. + */ +export function readyHarnessId( + runtimes: ReadonlyArray<{ id: string }>, + pendingId: string | null, +): string | null { + return runtimes.some((runtime) => runtime.id === pendingId) + ? pendingId + : null; +} + +/** + * Selects a newly registered custom harness once discovery publishes it. + * + * Returns the setter to hand the saved id; `onReady` then fires with it, so + * callers reuse their normal dropdown-change path instead of growing a second + * selection code path. + * + * `active` is the owning dialog's open state. The wait is only meaningful + * while that dialog is open: both host dialogs stay mounted across closes, so + * a pending id would otherwise survive the close and select into reset — or + * hidden — form state whenever discovery caught up. Going inactive both blocks + * `onReady` and drops the pending id, so a later publish is a no-op and + * reopening starts clean. A second save before the first publishes replaces + * it: the field holds one harness, so the latest save wins. + */ +export function usePendingHarnessSelection( + runtimes: ReadonlyArray<{ id: string }>, + onReady: (id: string) => void, + active: boolean, +): (id: string) => void { + const [pendingId, setPendingId] = React.useState(null); + // Gated at render, not just in the effect, so a catalog update landing in + // the same commit as the close cannot slip a selection through. + const readyId = active ? readyHarnessId(runtimes, pendingId) : null; + + React.useEffect(() => { + if (!active) { + setPendingId(null); + return; + } + if (readyId === null) return; + setPendingId(null); + onReady(readyId); + }, [active, onReady, readyId]); + + return setPendingId; +} diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index 59c92c7a69..08a57fa4c6 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -251,11 +251,6 @@ export function MessageThreadPanel({ // conditional activity accessory (agent working and/or someone typing). const hasComposerBottomActivity = activityAccessoryVisible || threadTypingPubkeys.length > 0; - useComposerHeightPadding( - threadBodyRef, - threadComposerWrapperRef, - isSinglePanelView, - ); // Live ref so onCaptureSendContext can read reply state at submit time // (before any async mention-flow awaits change navigation state). @@ -490,19 +485,31 @@ export function MessageThreadPanel({ threadHead, ]); - const { isAtBottom, newMessageCount, onScroll, scrollToBottom } = - useAnchoredScroll({ - channelId: threadHeadId, - contentRef: threadContentRef, - isLoading: threadRepliesPending || repliesRenderState === "pending", - messages: threadMessages, - highlightTargetMessage: scrollTargetHighlights, - onTargetReached: onScrollTargetResolved, - onTargetSettled: onScrollTargetSettled, - pinTargetCentered: !scrollTargetHighlights, - scrollContainerRef: threadBodyRef, - targetMessageId: scrollTargetId, - }); + const { + isAtBottom, + newMessageCount, + onScroll, + scrollToBottom, + settleAtBottomAfterLayout, + } = useAnchoredScroll({ + channelId: threadHeadId, + contentRef: threadContentRef, + isLoading: threadRepliesPending || repliesRenderState === "pending", + messages: threadMessages, + highlightTargetMessage: scrollTargetHighlights, + onTargetReached: onScrollTargetResolved, + onTargetSettled: onScrollTargetSettled, + pinTargetCentered: !scrollTargetHighlights, + scrollContainerRef: threadBodyRef, + targetMessageId: scrollTargetId, + }); + useComposerHeightPadding( + threadBodyRef, + threadComposerWrapperRef, + isSinglePanelView, + "padding", + settleAtBottomAfterLayout, + ); const knownAgentPubkeys = useKnownAgentPubkeys(); const initialAgentPubkeys = React.useMemo(() => { diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs index 507f654cd0..1fdfb05857 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs +++ b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs @@ -218,7 +218,8 @@ function makePinnedCenterNodes() { disconnect() {} observe(target) { - this.target = target; + this.targets ??= []; + this.targets.push(target); } }; @@ -246,6 +247,18 @@ function Harness({ channelId, onTargetSettled, refs }) { return null; } +function BottomStateHarness({ messages, onState, refs }) { + const anchored = useAnchoredScroll({ + channelId: "conversation", + contentRef: refs.content, + isLoading: false, + messages, + scrollContainerRef: refs.container, + }); + onState(anchored); + return null; +} + function VirtualTargetHarness({ refs }) { const didRun = React.useRef(false); const bottomApi = useVirtualizedBottomSettle( @@ -294,7 +307,10 @@ test("channel change attaches pinned-center observers after refs mount", async ( }); assert.equal(nodes.resizeObservers.length, 1); - assert.equal(nodes.resizeObservers[0].target, nodes.content); + assert.deepEqual(nodes.resizeObservers[0].targets, [ + nodes.content, + nodes.container, + ]); assert.equal(nodes.container.listeners.get("wheel")?.length, 1); await act(async () => { @@ -313,7 +329,47 @@ test("channel change attaches pinned-center observers after refs mount", async ( }); }); -test("pinned target settles only after resize correction and a paint frame", async () => { +test("container resize clears a stale new-message state at the physical floor", async () => { + const refs = { + container: { current: null }, + content: { current: null }, + }; + const root = createRoot(document.createElement("div")); + const nodes = makePinnedCenterNodes(); + refs.container.current = nodes.container; + refs.content.current = nodes.content; + let state = null; + const render = (messages) => + root.render( + React.createElement(BottomStateHarness, { + messages, + onState: (nextState) => { + state = nextState; + }, + refs, + }), + ); + + await act(async () => render([{ id: "first" }])); + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + nodes.container.scrollTop = 100; + await act(async () => state.onScroll()); + nodes.container.scrollTop = 100; + await act(async () => state.onScroll()); + await act(async () => render([{ id: "first" }, { id: "second" }])); + assert.equal(state.isAtBottom, false); + assert.equal(state.newMessageCount, 1); + + // A taller viewport reaches the floor without producing a native scroll. + nodes.container.clientHeight = 900; + await act(async () => nodes.resizeObservers[0].callback()); + + assert.equal(state.isAtBottom, true); + assert.equal(state.newMessageCount, 0); + await act(async () => root.unmount()); +}); + +test("pinned target resize reconciles bottom state before retiring", async () => { const refs = { container: { current: null }, content: { current: null }, diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index ead5771294..add9439599 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -73,6 +73,9 @@ type UseAnchoredScrollResult = { highlightedMessageId: string | null; /** Imperative: scroll to bottom. */ scrollToBottom: (behavior?: ScrollBehavior) => void; + /** Re-pins after a layout owner changes trailing geometry. Returns true when + * the hook handled the settlement, including a preserved pinned target. */ + settleAtBottomAfterLayout: () => boolean; /** Arm a one-shot scroll-to-bottom that fires on the next appended message * (used by the composer's send flow). */ scrollToBottomOnNextUpdate: () => void; @@ -383,6 +386,35 @@ export function useAnchoredScroll({ forceBottomOnNextAppendRef.current = true; }, []); + const settleAtBottomAfterLayout = React.useCallback(() => { + const container = scrollContainerRef.current; + if (!container) return false; + if (anchorRef.current.kind === "pinned-center") { + repinPinnedCenter(); + const atBottom = isAtBottomNow(container); + setIsAtBottom((previous) => + previous === atBottom ? previous : atBottom, + ); + if (atBottom) setNewMessageCount(0); + schedulePinnedTargetSettle(anchorRef.current.messageId); + return true; + } + if (!isAtBottomNow(container)) return false; + + anchorRef.current = { kind: "at-bottom" }; + setIsAtBottom(true); + setNewMessageCount(0); + if (!virtualizerOwnsPrependAnchoring) { + container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); + } + return true; + }, [ + repinPinnedCenter, + schedulePinnedTargetSettle, + scrollContainerRef, + virtualizerOwnsPrependAnchoring, + ]); + const highlightMessage = React.useCallback((messageId: string) => { if (highlightTimeoutRef.current !== null) { window.clearTimeout(highlightTimeoutRef.current); @@ -743,10 +775,8 @@ export function useAnchoredScroll({ const observer = new ResizeObserver(() => { const container = scrollContainerRef.current; if (!container) return; - if (anchorRef.current.kind === "pinned-center") { - repinPinnedCenter(); - schedulePinnedTargetSettle(anchorRef.current.messageId); - } else if ( + if (settleAtBottomAfterLayout()) return; + if ( anchorRef.current.kind === "at-bottom" && !virtualizerOwnsPrependAnchoring ) { @@ -754,6 +784,8 @@ export function useAnchoredScroll({ } }); observer.observe(content); + const container = scrollContainerRef.current; + if (container && container !== content) observer.observe(container); return () => { observer.disconnect(); if (targetSettleRafRef.current !== null) { @@ -764,9 +796,8 @@ export function useAnchoredScroll({ }, [ channelId, contentRef, - repinPinnedCenter, - schedulePinnedTargetSettle, scrollContainerRef, + settleAtBottomAfterLayout, virtualizerOwnsPrependAnchoring, ]); @@ -919,6 +950,7 @@ export function useAnchoredScroll({ newMessageCount, highlightedMessageId, scrollToBottom: scrollToBottomImperative, + settleAtBottomAfterLayout, scrollToBottomOnNextUpdate, scrollToMessage: scrollToMessageImperative, onVirtualizerAtBottomStateChange, diff --git a/desktop/src/features/settings/ui/CustomHarnessForm.tsx b/desktop/src/features/settings/ui/CustomHarnessForm.tsx index 60f3a66af1..52e7906265 100644 --- a/desktop/src/features/settings/ui/CustomHarnessForm.tsx +++ b/desktop/src/features/settings/ui/CustomHarnessForm.tsx @@ -216,7 +216,8 @@ export function CustomHarnessForm({ * delete the old file when the id changes. */ originalId?: string; onCancel: () => void; - onSaved: () => void; + /** Receives the id the harness was saved under (the form may rewrite it). */ + onSaved: (id: string) => void; /** Render without the bordered card chrome (for embedding in the catalog * dialog detail pane). */ chromeless?: boolean; @@ -273,11 +274,9 @@ export function CustomHarnessForm({ return; } try { - await save.mutateAsync({ - definition: definitionFromFormValues(form), - originalId, - }); - onSaved(); + const definition = definitionFromFormValues(form); + await save.mutateAsync({ definition, originalId }); + onSaved(definition.id); } catch (err) { setError(err instanceof Error ? err.message : String(err)); } diff --git a/desktop/src/features/sidebar/ui/CommunityRail.tsx b/desktop/src/features/sidebar/ui/CommunityRail.tsx index b15e0bab71..386ee20691 100644 --- a/desktop/src/features/sidebar/ui/CommunityRail.tsx +++ b/desktop/src/features/sidebar/ui/CommunityRail.tsx @@ -370,7 +370,7 @@ export function CommunityRail({ return (