From 0ce320b78e22c6e30b1dfbe90eec90892c25ebac Mon Sep 17 00:00:00 2001 From: Gulsah Sarsilmaz Date: Sat, 2 May 2026 19:26:39 -0300 Subject: [PATCH 1/2] fix(shared-session): hide "Copy session sharing link" when no link exists (GH9736) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The right-click "Copy session sharing link" menu item was offered whenever the session's `SharedSessionStatus` was anything other than `NotShared`. That set includes pending states (`SharePending`, `SharePendingPreBootstrap`) and the post-end `FinishedViewer` state — none of which have an actual sharing URL. After a session fails to share (issue #9735's reproduction), the status remains in `SharePending*` and the user is offered to copy a link that does not exist; selecting it writes a stale or empty value to the clipboard. Tighten the condition: introduce `SharedSessionStatus::has_active_share_link`, which returns true only for `ActiveSharer` and `ActiveViewer`. Use it as the gate for the "Copy session sharing link" menu entry. Pending and finished states no longer surface the entry, so a failed share simply leaves the menu showing only the unrelated entries (Share/Stop sharing). The existing `is_sharer_or_viewer` helper is kept because other call sites (e.g. the reconnection-banner guard at view_impl.rs:1571–1582) intentionally want to include pending states. Regression test in `app/src/terminal/shared_session/mod_test.rs` asserts `has_active_share_link` returns true only for the two active states and false for every pending / failed-pending / finished state. --- app/src/terminal/shared_session/mod.rs | 13 ++++++ app/src/terminal/shared_session/mod_test.rs | 41 ++++++++++++++++++- .../terminal/view/shared_session/view_impl.rs | 8 +++- 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/app/src/terminal/shared_session/mod.rs b/app/src/terminal/shared_session/mod.rs index b99ffff87..1f02ad1c9 100644 --- a/app/src/terminal/shared_session/mod.rs +++ b/app/src/terminal/shared_session/mod.rs @@ -149,6 +149,19 @@ impl SharedSessionStatus { !matches!(self, Self::NotShared) } + /// Returns `true` when the session currently has a usable sharing + /// link the user can copy — i.e. when the share has fully + /// established (sharer side) or been joined (viewer side). Pending + /// and finished states return `false` so the right-click "Copy + /// session sharing link" menu item is not offered when the link + /// would either not exist yet or no longer be valid (GH#9736). + pub fn has_active_share_link(&self) -> bool { + matches!( + self, + Self::ActiveSharer | Self::ActiveViewer { .. } + ) + } + pub fn as_keymap_context(&self) -> &'static str { match self { Self::NotShared => "SharedSessionStatus_NotShared", diff --git a/app/src/terminal/shared_session/mod_test.rs b/app/src/terminal/shared_session/mod_test.rs index f00d7d997..67df6d24a 100644 --- a/app/src/terminal/shared_session/mod_test.rs +++ b/app/src/terminal/shared_session/mod_test.rs @@ -1,4 +1,6 @@ -use super::{decode_scrollback, SharedSessionScrollbackType}; +use super::{decode_scrollback, SharedSessionScrollbackType, SharedSessionStatus}; +use session_sharing_protocol::common::Role; +use session_sharing_protocol::sharer::SessionSourceType; use crate::ai::blocklist::agent_view::AgentViewState; use crate::assert_lines_approx_eq; @@ -347,3 +349,40 @@ fn test_loading_scrollback_in_alt_screen() { // Make sure we're in the alt screen. assert!(model.is_alt_screen_active()); } + +/// Regression test for GH#9736: the right-click "Copy session sharing link" +/// menu item must only be offered when a sharing URL actually exists. Pending +/// shares (which can include shares that failed to establish) and finished +/// viewer states leave no link to copy and previously surfaced the menu item +/// anyway, writing a stale or empty value to the clipboard. +#[test] +fn has_active_share_link_only_true_for_active_states() { + // States with a usable link. + assert!(SharedSessionStatus::ActiveSharer.has_active_share_link()); + assert!( + SharedSessionStatus::ActiveViewer { role: Role::Reader }.has_active_share_link(), + "ActiveViewer (reader) must offer copy link" + ); + assert!( + SharedSessionStatus::ActiveViewer { + role: Role::Executor, + } + .has_active_share_link(), + "ActiveViewer (executor) must offer copy link" + ); + + // States without a usable link — these are the ones that produced + // the GH#9736 regression. + assert!(!SharedSessionStatus::NotShared.has_active_share_link()); + assert!(!SharedSessionStatus::SharePending.has_active_share_link()); + assert!( + !SharedSessionStatus::SharePendingPreBootstrap { + source_type: SessionSourceType::default(), + } + .has_active_share_link(), + "SharePendingPreBootstrap must not offer copy link — \ + this is the state a failed-to-share session sits in" + ); + assert!(!SharedSessionStatus::ViewPending.has_active_share_link()); + assert!(!SharedSessionStatus::FinishedViewer.has_active_share_link()); +} diff --git a/app/src/terminal/view/shared_session/view_impl.rs b/app/src/terminal/view/shared_session/view_impl.rs index 651269e3a..18eb55629 100644 --- a/app/src/terminal/view/shared_session/view_impl.rs +++ b/app/src/terminal/view/shared_session/view_impl.rs @@ -1630,7 +1630,13 @@ impl TerminalView { ); } - if model.shared_session_status().is_sharer_or_viewer() { + // Only offer "Copy session sharing link" when a link actually + // exists. Pending shares (SharePending / SharePendingPreBootstrap) + // have not yet produced a URL, and a session that finished or + // failed to share leaves no link to copy. Surfacing the menu + // entry in those states writes a stale or empty value to the + // clipboard — see GH#9736. + if model.shared_session_status().has_active_share_link() { items.push( MenuItemFields::new("Copy session sharing link") .with_on_select_action(TerminalAction::CopySharedSessionLink { From f8d949c195c2fa92060f1150b84f3194d1e4e2db Mon Sep 17 00:00:00 2001 From: Gulsah Sarsilmaz Date: Sat, 2 May 2026 19:38:36 -0300 Subject: [PATCH 2/2] fix(shared-session): also gate pane-header "Copy link" on active share (GH9736) Apply the same `has_active_share_link()` predicate to the "Copy link" entry in the pane-header overflow menu (`pane_impl.rs:657`) that this branch already applies to the right-click "Copy session sharing link" entry. Without this the second surface still offered a copy action during `SharePending` / `SharePendingPreBootstrap` / `FinishedViewer` states, leaving the GH#9736 regression visible from one of two menu paths. Scope: only the `Copy link` entry is moved under `has_active_share_link()`. The neighbouring `Stop sharing session` entry deliberately stays under the broader `is_sharer_or_viewer()` / `is_sharer()` checks so the user can still cancel a pending or stuck share. `Open on Desktop` likewise stays under the broader check; if that surface has the same stale-state issue it warrants its own follow-up. Per Oz's review of #9940. --- app/src/terminal/view/pane_impl.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/src/terminal/view/pane_impl.rs b/app/src/terminal/view/pane_impl.rs index 48b245775..c4e8347f6 100644 --- a/app/src/terminal/view/pane_impl.rs +++ b/app/src/terminal/view/pane_impl.rs @@ -652,7 +652,14 @@ impl BackingView for TerminalView { let shared_session_status = model.shared_session_status(); let is_ambient_agent = self.is_ambient_agent_session(ctx); if shared_session_status.is_sharer_or_viewer() { - if !is_ambient_agent { + // "Copy link" is gated on `has_active_share_link()` rather than + // the outer `is_sharer_or_viewer()` so that pending and failed + // states (which leave no usable URL) do not surface a copy + // entry — see GH#9736. The neighbouring "Stop sharing session" + // entry deliberately stays under `is_sharer_or_viewer()` / + // `is_sharer()` so the user can still cancel a stuck pending + // share. + if !is_ambient_agent && shared_session_status.has_active_share_link() { items.push( MenuItemFields::new("Copy link") .with_on_select_action(TerminalAction::CopySharedSessionLink { source })