diff --git a/.github/actions/spelling/allow/allow.txt b/.github/actions/spelling/allow/allow.txt index 0faae3da1..32b2f225b 100644 --- a/.github/actions/spelling/allow/allow.txt +++ b/.github/actions/spelling/allow/allow.txt @@ -210,6 +210,8 @@ screenshots serde servicebus sessionless +shrx +shtx sidechain sideload sids diff --git a/tools/wta/src/app.rs b/tools/wta/src/app.rs index a2a4cbfe8..257fd22c5 100644 --- a/tools/wta/src/app.rs +++ b/tools/wta/src/app.rs @@ -25,6 +25,20 @@ struct DeferredAcpParams { master_ext_rx: Option>, shell_mgr: Arc, wt_connected: bool, + /// Pipe-mode marker. When `Some`, [`App::try_start_acp`] reconnects + /// via [`run_acp_client_over_pipe`] instead of the direct + /// [`run_acp_client`] path. Pre-stashed at boot in helper mode + /// (main.rs) so that a post-FRE-login reconnect goes back through + /// wta-master — without this, the reconnected helper would spawn + /// its own copilot CLI subprocess directly, master would never see + /// it, and ext-methods like `intellterm.wta/sessions/list` would + /// fail with "Method not found" (the ACP SDK forwards unknown + /// extensions to the agent with a `_` prefix, and the agent CLI + /// doesn't know about master-side extensions). + master_pipe_name: Option, + /// Owner tab id for pipe-mode reconnect (mirrors the original + /// `--owner-tab-id` CLI arg). Unused in direct mode. + owner_tab_id: Option, } mod turn_state; @@ -2242,11 +2256,79 @@ impl App { master_ext_rx: Some(master_ext_rx), shell_mgr, wt_connected, + master_pipe_name: None, + owner_tab_id: None, + }); + } + + /// Stash pipe-mode launch parameters on App so that a post-FRE-login + /// reconnect via [`Self::try_start_acp`] goes back through + /// [`run_acp_client_over_pipe`] (talking to wta-master) instead of + /// the direct [`run_acp_client`] path (which would spawn its own + /// copilot CLI subprocess and bypass master entirely). + /// + /// The bug this guards against: in helper mode (`--connect-master`), + /// the initial `run_acp_client_over_pipe` task fails immediately with + /// `Authentication required` if the user is in FRE / not yet logged + /// in. The helper falls into the setup screen, the user logs in, and + /// `LoginComplete` fires `try_start_acp`. Without this pre-stash, + /// `LoginComplete` finds `deferred_acp.is_none()` and synthesizes a + /// direct-mode `DeferredAcpParams`, then `try_start_acp` calls + /// `run_acp_client` — bypassing master. The resulting helper: + /// * never registers with master (no `HelperId` in master log), + /// * gets `Method not found` for every `intellterm.wta/...` + /// ext-request (the ACP SDK forwards them to the agent CLI + /// prefixed with `_`, and the agent doesn't know them), + /// * has `session_hook` events that go nowhere (the channel's + /// receiver was already consumed by the dead pipe-mode task). + /// + /// All `_rx` fields are seeded `None`; `try_start_acp` creates fresh + /// channels on reconnect and re-binds the `_tx` halves on App, plus + /// re-creates the `session_hook` channel and re-binds + /// `self.session_hook_tx`. + pub fn set_master_pipe_acp_params( + &mut self, + pipe_name: String, + agent_cmd: String, + acp_model: Option, + owner_tab_id: Option, + shell_mgr: Arc, + wt_connected: bool, + ) { + self.deferred_acp = Some(DeferredAcpParams { + agent_cmd, + acp_model, + prompt_rx: None, + cancel_rx: None, + new_session_rx: None, + load_session_rx: None, + drop_session_rx: None, + rename_session_rx: None, + restart_rx: None, + master_ext_rx: None, + shell_mgr, + wt_connected, + master_pipe_name: Some(pipe_name), + owner_tab_id, }); } /// Try to start the ACP client if login just completed. /// Creates fresh channels if previous ones were consumed by a failed attempt. + /// + /// **Pipe-mode branch.** When `deferred_acp.master_pipe_name.is_some()` + /// (set at boot by [`Self::set_master_pipe_acp_params`] in helper + /// mode), we route the reconnect through + /// [`run_acp_client_over_pipe`] so the rebuilt helper talks to the + /// shared wta-master singleton — same as the cold-boot helper path. + /// We also rebuild the `session_hook` channel and re-bind the `_tx` + /// half on `self.session_hook_tx`, because the original receiver was + /// consumed (and dropped) by the dead initial pipe-mode task. + /// + /// **Direct-mode branch.** When `master_pipe_name.is_none()` we keep + /// the legacy behavior: spawn [`run_acp_client`], which runs its own + /// agent CLI subprocess. This is the FRE path for non-helper + /// invocations (`wta` launched without `--connect-master`). pub fn try_start_acp(&mut self) { if !self.pending_acp_start { return; @@ -2303,31 +2385,93 @@ impl App { params.restart_rx.take(), params.master_ext_rx.take(), ) { - // Resolve the agent executable path (bare "copilot" may not - // be on PATH in packaged apps — use WinGet Links fallback). - let agent_cmd = resolve_agent_cmd(¶ms.agent_cmd); let acp_model = params.acp_model.clone(); - let owner_tab_id = self.tab_id.clone(); let event_tx = tx.clone(); let shell_mgr = Arc::clone(¶ms.shell_mgr); let wt_connected = params.wt_connected; - - tokio::task::spawn_local(crate::protocol::acp::client::run_acp_client( - agent_cmd, - acp_model, - owner_tab_id, - event_tx, - prompt_rx, - cancel_rx, - new_session_rx, - load_session_rx, - drop_session_rx, - rename_session_rx, - restart_rx, - master_ext_rx, - shell_mgr, - wt_connected, - )); + let pipe_name_opt = params.master_pipe_name.clone(); + let owner_tab_opt = params.owner_tab_id.clone(); + + if let Some(pipe_name) = pipe_name_opt { + // Pipe-mode reconnect (helper after FRE login). + // Rebuild the session_hook channel — the original + // rx was consumed and dropped with the dead initial + // task, leaving `self.session_hook_tx` pointing at a + // closed channel (every `publish_session_hook` call + // logs "channel closed"). Reinstall a live tx so + // hooks reach master again, and hand the matching + // rx to the new pipe-mode task. + let (shtx, shrx) = mpsc::unbounded_channel(); + self.session_hook_tx = Some(shtx); + tracing::info!( + target: "acp", + pipe = %pipe_name, + "try_start_acp: reconnecting via master pipe" + ); + let event_tx_for_pipe = event_tx.clone(); + tokio::task::spawn_local(async move { + if let Err(e) = + crate::protocol::acp::client::run_acp_client_over_pipe( + pipe_name, + acp_model, + owner_tab_opt, + None, // initial_load_session_id: already handled by the dead initial task + event_tx_for_pipe.clone(), + prompt_rx, + cancel_rx, + new_session_rx, + load_session_rx, + drop_session_rx, + rename_session_rx, + restart_rx, + shrx, + master_ext_rx, + shell_mgr, + wt_connected, + ) + .await + { + tracing::error!( + target: "helper", + error = %e, + "run_acp_client_over_pipe failed on reconnect" + ); + let failure = crate::protocol::acp::failure::classify_anyhow( + &e, + crate::protocol::acp::failure::HandshakeStage::Initialize, + ); + let _ = event_tx_for_pipe.send(AppEvent::AgentError { + session_id: None, + failure, + message: format!( + "helper ACP transport failed on reconnect: {e:#}" + ), + }); + } + }); + } else { + // Direct-mode reconnect (non-helper FRE path). + // Resolve the agent executable path (bare "copilot" may not + // be on PATH in packaged apps — use WinGet Links fallback). + let agent_cmd = resolve_agent_cmd(¶ms.agent_cmd); + let owner_tab_id = self.tab_id.clone(); + tokio::task::spawn_local(crate::protocol::acp::client::run_acp_client( + agent_cmd, + acp_model, + owner_tab_id, + event_tx, + prompt_rx, + cancel_rx, + new_session_rx, + load_session_rx, + drop_session_rx, + rename_session_rx, + restart_rx, + master_ext_rx, + shell_mgr, + wt_connected, + )); + } } } } @@ -5984,6 +6128,8 @@ impl App { master_ext_rx: None, shell_mgr: Arc::clone(&self.shell_mgr), wt_connected: self.wt_connected, + master_pipe_name: None, + owner_tab_id: None, }); } self.pending_acp_start = true; diff --git a/tools/wta/src/main.rs b/tools/wta/src/main.rs index dc46e131d..9a337b934 100644 --- a/tools/wta/src/main.rs +++ b/tools/wta/src/main.rs @@ -2369,6 +2369,31 @@ async fn run_acp_app( app_state.set_session_hook_tx(session_hook_tx); } + // Pipe-mode reconnect pre-stash. In helper mode the initial + // `run_acp_client_over_pipe` task fails immediately with + // `Authentication required` if the user is in FRE (not yet + // logged in). The post-login `LoginComplete` handler fires + // `try_start_acp`; without this stash it would synthesize a + // direct-mode `DeferredAcpParams` and spawn `run_acp_client`, + // bypassing master and breaking every `intellterm.wta/...` + // ext-method (e.g. `sessions/list` — session view would stay + // empty on the first tab forever). With the stash in place, + // `try_start_acp` sees `master_pipe_name = Some(...)` and + // routes the reconnect back through master. + // + // No effect when the initial connection succeeds: the + // stashed params just sit unused for the helper's lifetime. + if let Some(ref pipe_name) = connect_master_pipe { + app_state.set_master_pipe_acp_params( + pipe_name.clone(), + agent_cmd.clone(), + cli.acp_model.clone(), + cli.owner_tab_id.clone(), + Arc::clone(&shell_mgr), + wt_connected, + ); + } + // ── Preflight: check the agent CLI before connecting ────────── // Skip preflight when FRE is active — FRE has its own agent // selection + auth flow and doesn't need the preflight wizard.