From 8671f76f3815187ef53a81d3ceccbcb0a8443367 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 18:03:26 -0400 Subject: [PATCH] fix(desktop): bind first-start scope before preflight and align mock summaries Signed-off-by: Logan Johnson --- .../src/commands/agents_local_start.rs | 107 +++++++++++++++--- desktop/src-tauri/src/relay.rs | 2 +- .../api/managedAgentStartScope.test.mjs | 107 ++++++++++++++++++ desktop/src/testing/e2eBridge.ts | 39 +++++-- .../e2e/agent-lifecycle-feedback.spec.ts | 96 ++++++++++++++++ docs/host-execution.md | 11 +- 6 files changed, 328 insertions(+), 34 deletions(-) create mode 100644 desktop/src/shared/api/managedAgentStartScope.test.mjs diff --git a/desktop/src-tauri/src/commands/agents_local_start.rs b/desktop/src-tauri/src/commands/agents_local_start.rs index c95151b228..05ef798b71 100644 --- a/desktop/src-tauri/src/commands/agents_local_start.rs +++ b/desktop/src-tauri/src/commands/agents_local_start.rs @@ -116,6 +116,33 @@ fn revalidate_first_start_scope( crate::relay::assert_expected_signer(Some(captured_owner), &workspace_owner_hex(state)?) } +// All local first-start callers share this suspension boundary. Explicit +// Start supplies its clicked scope; fresh create-start binds the command's +// entry scope here, BEFORE mesh discovery can suspend. Never bind a new +// workspace/owner on continuation merely because the caller was unscoped. +async fn scoped_local_preflight( + state: &AppState, + expected_relay_url: Option<&str>, + expected_signer_pubkey: Option<&str>, + preflight: impl std::future::Future>, +) -> Result< + ( + crate::relay::ScopedWorkspaceRelay, + crate::relay::ScopedWorkspaceSigner, + ), + String, +> { + let relay = crate::relay::bind_expected_relay_scope( + expected_relay_url, + crate::relay::relay_ws_url_with_override(state), + )?; + let owner = + crate::relay::bind_expected_signer(expected_signer_pubkey, workspace_owner_hex(state)?)?; + preflight.await?; + revalidate_first_start_scope(state, relay.as_str(), owner.as_str())?; + Ok((relay, owner)) +} + pub(in crate::commands) async fn start_local_agent_with_preflight( app: &AppHandle, state: &AppState, @@ -156,25 +183,13 @@ pub(in crate::commands) async fn start_local_agent_with_preflight( &personas, &global, ); - ensure_relay_mesh_for_record(app, mesh_model_id.as_deref(), allow_fresh_create_start).await?; - - // The mesh preflight above is the suspension window Projects callbacks - // capture their scope against: a community switch during that await - // would otherwise spawn this pair keyed to the *new* workspace relay. - // Read the workspace relay ONCE, assert the caller's captured scope - // against that exact read, and hand the same bound value to the spawn - // below — the check is tied to its use, so a switch landing after this - // point can no longer retarget the spawn (it only changes state this - // call no longer consults). - let workspace_relay_url = crate::relay::bind_expected_relay_scope( + let (workspace_relay_url, workspace_owner) = scoped_local_preflight( + state, expected_relay_url, - crate::relay::relay_ws_url_with_override(state), - )?; - // Bind the active owner after the same final await as the relay. A - // same-relay identity replacement during mesh preflight must not release - // the stale preflight owner to spawn. - let workspace_owner = - crate::relay::bind_expected_signer(expected_signer_pubkey, workspace_owner_hex(state)?)?; + expected_signer_pubkey, + ensure_relay_mesh_for_record(app, mesh_model_id.as_deref(), allow_fresh_create_start), + ) + .await?; if !allow_fresh_create_start { let app_for_start = app.clone(); @@ -276,6 +291,62 @@ pub(in crate::commands) async fn start_local_agent_with_preflight( mod scope_tests { use super::*; + #[tokio::test] + async fn create_start_preflight_retains_entry_scope_without_a_caller_pin() { + let state = crate::app_state::build_app_state(); + let owner = workspace_owner_hex(&state).unwrap(); + for relay in ["ws://localhost:3000", "wss://community.example"] { + *state.relay_url_override.lock().unwrap() = Some(relay.to_owned()); + let (bound_relay, bound_owner) = scoped_local_preflight(&state, None, None, async { + tokio::task::yield_now().await; + Ok(()) + }) + .await + .unwrap(); + assert_eq!(bound_relay.as_str(), relay); + assert_eq!(bound_owner.as_str(), owner); + } + } + + #[tokio::test] + async fn create_start_preflight_rejects_a_community_switch_without_a_caller_pin() { + let state = crate::app_state::build_app_state(); + *state.relay_url_override.lock().unwrap() = Some("wss://clicked.example".into()); + let error = scoped_local_preflight(&state, None, None, async { + tokio::task::yield_now().await; + *state.relay_url_override.lock().unwrap() = Some("wss://other.example".into()); + Ok(()) + }) + .await + .unwrap_err(); + assert!(error.contains("active community changed"), "{error}"); + } + + #[tokio::test] + async fn create_start_preflight_rejects_an_owner_switch_without_a_caller_pin() { + let state = crate::app_state::build_app_state(); + let error = scoped_local_preflight(&state, None, None, async { + tokio::task::yield_now().await; + *state.keys.lock().unwrap() = Keys::generate(); + Ok(()) + }) + .await + .unwrap_err(); + assert!(error.contains("active identity changed"), "{error}"); + } + + #[tokio::test] + async fn scoped_start_rejects_stale_scope_before_polling_preflight() { + let state = crate::app_state::build_app_state(); + *state.relay_url_override.lock().unwrap() = Some("wss://other.example".into()); + let error = scoped_local_preflight(&state, Some("wss://clicked.example"), None, async { + panic!("stale Start must not poll mesh preflight"); + }) + .await + .unwrap_err(); + assert!(error.contains("active community changed"), "{error}"); + } + // This exercises the production continuation (including its WS getter), // not just the generic relay helper. State is ephemeral: no app setup, // identity resolution, keyring, files, network or child process is used. diff --git a/desktop/src-tauri/src/relay.rs b/desktop/src-tauri/src/relay.rs index 1415617484..c2fbb64f1d 100644 --- a/desktop/src-tauri/src/relay.rs +++ b/desktop/src-tauri/src/relay.rs @@ -100,7 +100,7 @@ pub fn relay_http_base_url(relay_url: &str) -> String { mod scope; pub use scope::{ assert_expected_relay_scope, assert_expected_signer, bind_expected_relay_scope, - bind_expected_signer, ScopedWorkspaceRelay, + bind_expected_signer, ScopedWorkspaceRelay, ScopedWorkspaceSigner, }; pub fn relay_api_base_url() -> String { diff --git a/desktop/src/shared/api/managedAgentStartScope.test.mjs b/desktop/src/shared/api/managedAgentStartScope.test.mjs new file mode 100644 index 0000000000..3ce5b43c94 --- /dev/null +++ b/desktop/src/shared/api/managedAgentStartScope.test.mjs @@ -0,0 +1,107 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { fromRawManagedAgent } from "./managedAgentWire.ts"; +import { startManagedAgent } from "./tauriManagedAgents.ts"; +import { + startManagedAgentWithRules, + respawnManagedAgentWithRules, +} from "../../features/agents/lib/managedAgentControlActions.ts"; + +// Cross the consumed wire -> shared summary/profile actions -> real IPC wrapper, +// not an injected Start callback that could accidentally discard the scope. +const raw = (overrides = {}) => ({ + pubkey: "ab".repeat(32), + selected_relay_url: "wss://clicked.example", + selected_run_id: null, + relay_url: "wss://legacy-pin.example", + backend: { type: "local" }, + status: "stopped", + ...overrides, +}); + +async function withIpc(handler, run) { + const previous = globalThis.window; + globalThis.window = { __TAURI_INTERNALS__: { invoke: handler } }; + try { + await run(); + } finally { + globalThis.window = previous; + } +} + +const start = ({ pubkey, expectedRelayUrl }) => + startManagedAgent(pubkey, { expectedRelayUrl }); + +for (const backend of [ + { type: "local" }, + { type: "provider", id: "test", config: {} }, +]) { + test(`unstarted ${backend.type} summary carries selected workspace through IPC without a run`, async () => { + const summary = raw({ backend }); + const calls = []; + await withIpc( + async (command, args) => { + calls.push({ command, args }); + return summary; + }, + async () => { + await startManagedAgentWithRules({ + agent: fromRawManagedAgent(summary), + startManagedAgent: start, + }); + }, + ); + assert.deepEqual(calls, [ + { + command: "start_managed_agent", + args: { + pubkey: summary.pubkey, + expectedRelayUrl: summary.selected_relay_url, + expectedSignerPubkey: null, + }, + }, + ]); + }); +} + +test("missing wire scope never falls back to the legacy pin or invokes Start", async () => { + const summary = raw(); + delete summary.selected_relay_url; + await withIpc( + async () => assert.fail("must not invoke"), + async () => { + await assert.rejects( + startManagedAgentWithRules({ + agent: fromRawManagedAgent(summary), + startManagedAgent: start, + }), + /without a selected community/, + ); + }, + ); +}); + +test("Restart keeps the clicked wire scope through Stop and real Start IPC", async () => { + const agent = fromRawManagedAgent( + raw({ status: "running", selected_run_id: "clicked-run" }), + ); + await withIpc( + async (command, args) => { + assert.equal(command, "start_managed_agent"); + assert.equal(args.expectedRelayUrl, "wss://clicked.example"); + return raw(); + }, + async () => { + await respawnManagedAgentWithRules({ + agent, + startManagedAgent: start, + stopManagedAgent: async (input) => { + assert.equal(input.selectedRunId, "clicked-run"); + assert.equal(input.expectedRelayUrl, "wss://clicked.example"); + await Promise.resolve(); + agent.selectedRelayUrl = "wss://other.example"; + }, + }); + }, + ); +}); diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 9c12ebef4f..f3a23f0fbd 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -94,6 +94,8 @@ type MockCommandAvailability = { }; export type MockManagedAgentSeed = { + /** Persisted legacy pin, not the workspace selected for Start. */ + relayUrl?: string; pubkey: string; name: string; avatarUrl?: string | null; @@ -906,6 +908,8 @@ type RawRelayAgent = { }; type RawManagedAgent = { + selected_relay_url?: string | null; + selected_run_id?: string | null; pubkey: string; name: string; persona_id: string | null; @@ -1788,6 +1792,12 @@ function cloneRelayAgent(agent: RawRelayAgent): RawRelayAgent { function cloneManagedAgent(agent: MockManagedAgent): RawManagedAgent { return { + // Native build_managed_agent_summary selects the workspace pair even + // before first Start. A stored relay pin and a live run are NOT required. + selected_relay_url: /^[0-9a-f]{64}$/i.test(agent.pubkey) + ? activeMockRelayUrl(getConfig()) + : null, + selected_run_id: null, // The mock does not model native generation receipts. pubkey: agent.pubkey, name: agent.name, persona_id: agent.persona_id, @@ -2353,7 +2363,7 @@ function buildSeededManagedAgent(seed: MockManagedAgentSeed): MockManagedAgent { // Native serde always emits this key (`null` when unpinned) — the bridge // must mirror the wire shape, not omit the key. runtime: seed.runtime ?? null, - relay_url: DEFAULT_RELAY_WS_URL, + relay_url: seed.relayUrl ?? DEFAULT_RELAY_WS_URL, acp_command: "buzz-acp", agent_command: agentCommand, agent_args: agentArgs, @@ -4206,12 +4216,7 @@ function getRelayWsUrl(config: E2eConfig | undefined): string { * switch with `openDmDelayMs` / `sendMessageDelayMs` and prove the send * fails closed. */ -function assertExpectedRelayScope( - expectedRelayUrl: string | null | undefined, - config: E2eConfig | undefined, -): void { - const expected = expectedRelayUrl?.trim(); - if (!expected) return; +function activeMockRelayUrl(config: E2eConfig | undefined): string { let active: string | null = null; try { const activeId = window.localStorage.getItem("buzz-active-community-id"); @@ -4224,10 +4229,16 @@ function assertExpectedRelayScope( } catch { active = null; } - if ( - normalizeMockRelayUrl(active ?? getRelayWsUrl(config)) !== - normalizeMockRelayUrl(expected) - ) { + return normalizeMockRelayUrl(active ?? getRelayWsUrl(config)); +} + +function assertExpectedRelayScope( + expectedRelayUrl: string | null | undefined, + config: E2eConfig | undefined, +): void { + const expected = expectedRelayUrl?.trim(); + if (!expected) return; + if (activeMockRelayUrl(config) !== normalizeMockRelayUrl(expected)) { throw new Error( "active community changed before the message was submitted; not sent", ); @@ -9513,7 +9524,11 @@ async function handleStartManagedAgent( agent.pid = agent.pid ?? 42000 + mockManagedAgents.indexOf(agent); // The real command spawns a pair runtime keyed by the agent's effective // relay (`start_managed_agent_process`), so mirror that row here. - upsertMockManagedAgentRuntime(agent.pubkey, agent.relay_url, "ready"); + upsertMockManagedAgentRuntime( + agent.pubkey, + activeMockRelayUrl(config), + "ready", + ); } agent.updated_at = now; agent.last_started_at = now; diff --git a/desktop/tests/e2e/agent-lifecycle-feedback.spec.ts b/desktop/tests/e2e/agent-lifecycle-feedback.spec.ts index de94bb230f..2b5ccc81e3 100644 --- a/desktop/tests/e2e/agent-lifecycle-feedback.spec.ts +++ b/desktop/tests/e2e/agent-lifecycle-feedback.spec.ts @@ -393,3 +393,99 @@ test.describe("agent lifecycle feedback screenshots", () => { await expect(saveButton).toBeEnabled(); }); }); + +for (const switchBeforeStart of [false, true]) { + test(`summary Start uses its emitted workspace scope, not the legacy pin (switch=${switchBeforeStart})`, async ({ + page, + }) => { + const pubkey = CASCADE_AGENT_A_PUBKEY; + const relay = "wss://clicked.example"; + await installMockBridge( + page, + { + personas: [ + { + id: CASCADE_PERSONA_ID, + displayName: "Scope Agent", + systemPrompt: "Test scope.", + }, + ], + managedAgents: [ + { + pubkey, + name: "Scope Agent", + personaId: CASCADE_PERSONA_ID, + status: "stopped", + relayUrl: "wss://legacy-pin.example", + }, + ], + }, + { relayWsUrl: relay }, + ); + await openAgentsView(page); + const summary = await page.evaluate(async (key) => { + const rows = (await window.__BUZZ_E2E_INVOKE_MOCK_COMMAND__?.( + "list_managed_agents", + {}, + )) as Array<{ + pubkey: string; + selected_relay_url: string; + selected_run_id: string | null; + }>; + return rows.find((row) => row.pubkey === key); + }, pubkey); + expect(summary?.selected_relay_url).toBe(relay); + expect(summary?.selected_run_id).toBeNull(); + if (switchBeforeStart) { + // Change native-side authority while retaining the clicked UI summary. + await page.evaluate(() => { + const id = localStorage.getItem("buzz-active-community-id"); + const communities = JSON.parse( + localStorage.getItem("buzz-communities") ?? "[]", + ); + for (const community of communities) { + if (community.id === id) community.relayUrl = "wss://other.example"; + } + localStorage.setItem("buzz-communities", JSON.stringify(communities)); + }); + } + await page.getByTestId(`agent-runtime-start-${pubkey}`).click(); + if (switchBeforeStart) { + await expect( + page.getByText(/active community changed/).first(), + ).toBeVisible(); + await expect( + page.getByTestId(`agent-runtime-active-${pubkey}`), + ).toHaveCount(0); + } else { + await expect( + page.getByTestId(`agent-runtime-active-${pubkey}`), + ).toBeVisible(); + } + const { starts, runtimes } = await page.evaluate(async () => ({ + starts: window.__BUZZ_E2E_COMMAND_LOG__?.filter( + (entry) => entry.command === "start_managed_agent", + ), + runtimes: await window.__BUZZ_E2E_INVOKE_MOCK_COMMAND__?.( + "list_managed_agent_runtimes", + {}, + ), + })); + expect(starts).toHaveLength(1); + expect(starts?.[0].payload).toMatchObject({ + pubkey, + expectedRelayUrl: relay, + }); + expect(runtimes).toEqual( + switchBeforeStart + ? [] + : [ + expect.objectContaining({ + pubkey, + relayUrl: relay, + lifecycle: "ready", + }), + ], + ); + }); +} diff --git a/docs/host-execution.md b/docs/host-execution.md index e2b8ca0f33..da676b37eb 100644 --- a/docs/host-execution.md +++ b/docs/host-execution.md @@ -123,9 +123,14 @@ ordinary Stop reuses its original immutable local request; a live successor cannot be reported as stopped by replaying that earlier success. Summary/profile Start captures the selected community, and Restart retains that -same community across its Stop await. The native Start command binds its owner -and community before preflight, then revalidates after suspension; a stale -continuation cannot start in the newly selected workspace. +same community across its Stop await. The summary producer selects the active +workspace pair even for an unstarted agent: selected community does not require +a live generation and is not the legacy record relay pin. Missing summary scope +fails closed at the control action, rather than falling back to current UI state. +The native Start command binds its owner and community before preflight, then +revalidates after suspension; a stale continuation cannot start in the newly +selected workspace. Fresh create-start likewise binds its entry owner/community +before mesh preflight, even though it has no prior summary selection. An explicit ordinary Start after confirmed exact Stop or definite rejected Start, and the existing pair Restart action, enter the same ledger for a fresh generation.