diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ecb342b2d7..06d7c578c0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -372,6 +372,14 @@ jobs: - name: Run file watch contract tests run: cargo test --locked -p openbitfun-services-integrations --no-default-features --features file-watch --test file_watch_contracts + # Exercise native child cleanup outside the Tauri test harness. In + # particular, Windows update/restart cleanup must leave the host alive. + - name: Run process lifecycle contract tests + shell: bash + run: | + cargo test --locked -p openbitfun-services-core --no-default-features --features process-runtime --test process_runtime_contracts + cargo test --locked -p openbitfun-services-core --no-default-features --features process-runtime --lib process_tree::tests:: + # Search tools share matching but retain provider-specific path and # symlink handling. Keep the search module in the full OS matrix. - name: Run search tool tests diff --git a/src/crates/services/services-core/AGENTS.md b/src/crates/services/services-core/AGENTS.md index 3654fba5a6..b4055886a8 100644 --- a/src/crates/services/services-core/AGENTS.md +++ b/src/crates/services/services-core/AGENTS.md @@ -68,6 +68,11 @@ crate. resource-limit safety. Unix descendants that deliberately create a new session/process group are outside this boundary and must be treated as a disclosed residual risk until a platform supervisor is introduced. +- Windows final cleanup closes only registered `process_tree` child Jobs and + rejects new managed spawns after shutdown begins. It must not initialize or + close the Job used by `contain_current_process_tree`: that explicit CLI/SDK + host-lifetime guard includes the host itself and stays alive until process + exit. Keep updater/restart handoff processes outside managed child trees. ## Verification @@ -88,6 +93,7 @@ cargo test -p openbitfun-services-core --no-default-features --features local-st cargo test -p openbitfun-services-core --no-default-features --features local-storage --test session_write_lock_contracts cargo test -p openbitfun-services-core --no-default-features --features token-usage-statistics --lib token_usage:: cargo test -p openbitfun-services-core --no-default-features --features process-runtime --test process_runtime_contracts +cargo test -p openbitfun-services-core --no-default-features --features process-runtime --lib process_tree::tests:: cargo test --locked -p openbitfun-services-core --no-default-features --features tls-provider --lib tls_provider::tests pnpm run check:core-boundaries ``` diff --git a/src/crates/services/services-core/src/process_manager.rs b/src/crates/services/services-core/src/process_manager.rs index b6b4207bad..22e9ab4b5b 100644 --- a/src/crates/services/services-core/src/process_manager.rs +++ b/src/crates/services/services-core/src/process_manager.rs @@ -1,6 +1,11 @@ -//! Unified process management to avoid Windows child process leaks +//! Hidden process creation and explicit host-lifetime containment. +//! +//! Host containment belongs to long-lived CLI/SDK services. Graceful cleanup +//! only closes managed child trees; it must never close a Job containing the +//! calling host before an updater, restart, or shutdown can finish. use std::process::Command; +#[cfg(windows)] use std::sync::LazyLock; #[cfg(target_os = "macos")] use std::sync::OnceLock; @@ -21,6 +26,7 @@ use win32job::Job; #[cfg(windows)] const CREATE_NO_WINDOW: u32 = 0x08000000; +#[cfg(windows)] static GLOBAL_PROCESS_MANAGER: LazyLock = LazyLock::new(ProcessManager::new); pub struct ProcessManager { @@ -29,6 +35,7 @@ pub struct ProcessManager { } impl ProcessManager { + #[cfg(windows)] fn new() -> Self { let manager = Self { #[cfg(windows)] @@ -68,17 +75,7 @@ impl ProcessManager { } pub fn cleanup_all(&self) { - #[cfg(windows)] - { - let mut job_guard = match self.job.lock() { - Ok(guard) => guard, - Err(poisoned) => { - warn!("Process manager job mutex was poisoned during cleanup, recovering lock"); - poisoned.into_inner() as std::sync::MutexGuard<'_, Option> - } - }; - job_guard.take(); - } + crate::process_tree::cleanup_all_process_trees(); } } @@ -160,11 +157,17 @@ fn build_macos_path_env() -> Option { std::env::join_paths(merged).ok() } +/// Stop managed child trees without creating or closing host containment. +/// Safe to call repeatedly, including when no child process was ever started. pub fn cleanup_all_processes() { - GLOBAL_PROCESS_MANAGER.cleanup_all(); + // Accessing the lazy host manager here would assign this process to a Job + // for the first time during Desktop exit. Keep that initialization exclusive + // to contain_current_process_tree(), and keep its handle alive until exit. + crate::process_tree::cleanup_all_process_trees(); } -/// Keep descendants of a long-lived service in the process-wide Job. +/// Keep descendants of a long-lived service in the process-wide Job until the +/// host exits. This lifetime guard is independent of managed-child cleanup. pub fn contain_current_process_tree() -> std::io::Result<()> { #[cfg(windows)] if GLOBAL_PROCESS_MANAGER diff --git a/src/crates/services/services-core/src/process_tree.rs b/src/crates/services/services-core/src/process_tree.rs index 5c7d6caac4..4637a9d83e 100644 --- a/src/crates/services/services-core/src/process_tree.rs +++ b/src/crates/services/services-core/src/process_tree.rs @@ -7,6 +7,8 @@ use std::fmt; use std::io; +#[cfg(windows)] +use std::sync::{Arc, LazyLock, Mutex, Weak}; use std::time::Duration; #[cfg(unix)] use std::time::Instant; @@ -94,7 +96,7 @@ impl ProcessTreeChild { #[cfg(windows)] { let _ = grace; - let had_job = self.platform.job.take().is_some(); + let had_job = self.platform.job.close(); if !parent_exited { self.child.wait().await?; } @@ -135,7 +137,7 @@ impl Drop for ProcessTreeChild { } #[cfg(windows)] { - self.platform.job.take(); + self.platform.job.close(); } if !parent_exited { let _ = self.child.start_kill(); @@ -200,7 +202,48 @@ fn process_group_is_alive(process_group_id: i32) -> bool { #[cfg(windows)] struct PlatformProcessTree { - job: Option, + job: Arc, +} + +#[cfg(windows)] +struct ManagedWindowsJob(Mutex>); + +#[cfg(windows)] +impl ManagedWindowsJob { + fn close(&self) -> bool { + self.0 + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .take() + .is_some() + } +} + +#[cfg(windows)] +#[derive(Default)] +struct ManagedWindowsJobs { + shutting_down: bool, + jobs: Vec>, +} + +#[cfg(windows)] +static MANAGED_WINDOWS_JOBS: LazyLock> = + LazyLock::new(|| Mutex::new(ManagedWindowsJobs::default())); + +/// Emergency/final cleanup for managed children. Protocol shutdown belongs to +/// callers and runs first. Host-lifetime Jobs are deliberately not registered. +pub(crate) fn cleanup_all_process_trees() { + #[cfg(windows)] + { + let mut registry = MANAGED_WINDOWS_JOBS + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + registry.shutting_down = true; + // Also serialize repeated callers until every registered handle closes. + for job in registry.jobs.drain(..).filter_map(|job| job.upgrade()) { + job.close(); + } + } } #[cfg(windows)] @@ -217,6 +260,17 @@ async fn spawn_windows_process_tree(command: &mut Command) -> io::Result io::Result 0); + registry.jobs.push(Arc::downgrade(&job)); + Ok(job) })(); - if let Err(error) = attach_result { - drop(job); - let _ = child.kill().await; - return Err(error); - } + let job = match attach_result { + Ok(job) => job, + Err(error) => { + let _ = child.kill().await; + return Err(error); + } + }; Ok(ProcessTreeChild { child, - platform: PlatformProcessTree { job: Some(job) }, + platform: PlatformProcessTree { job }, }) } @@ -360,7 +420,11 @@ mod tests { let mut command = Command::new("sh"); command .arg("-c") - .arg("\"$OPENBITFUN_PROCESS_TREE_TEST_EXE\" --exact process_tree::tests::unix_detached_fixture_process --nocapture") + // Keep the shell as the managed process-group leader. Some shells + // replace themselves with their final foreground command, which + // would make the fixture a process-group leader and cause setsid() + // to fail on macOS before it can publish its PID. + .arg("\"$OPENBITFUN_PROCESS_TREE_TEST_EXE\" --exact process_tree::tests::unix_detached_fixture_process --nocapture & wait") .env("OPENBITFUN_PROCESS_TREE_TEST_EXE", executable) .env("OPENBITFUN_DETACHED_FIXTURE", "1") .env("OPENBITFUN_DESCENDANT_PID_FILE", &pid_file) diff --git a/src/crates/services/services-core/tests/process_runtime_contracts.rs b/src/crates/services/services-core/tests/process_runtime_contracts.rs index c1cd82a8bb..b4e404c18e 100644 --- a/src/crates/services/services-core/tests/process_runtime_contracts.rs +++ b/src/crates/services/services-core/tests/process_runtime_contracts.rs @@ -9,3 +9,238 @@ fn system_check_command_preserves_missing_command_shape() { assert!(!result.exists); assert_eq!(result.path, None); } + +#[cfg(windows)] +mod windows_process_cleanup { + use openbitfun_services_core::process_manager::{ + cleanup_all_processes, contain_current_process_tree, create_command, + }; + use openbitfun_services_core::process_tree::ProcessTreeChild; + use std::path::Path; + use std::process::Stdio; + use std::time::{Duration, Instant}; + use tokio::io::AsyncReadExt; + use windows::Win32::Foundation::{CloseHandle, ERROR_INVALID_PARAMETER, HANDLE, WAIT_OBJECT_0}; + use windows::Win32::System::Threading::{ + OpenProcess, WaitForSingleObject, PROCESS_SYNCHRONIZE, + }; + + const FIXTURE_TEST: &str = "windows_process_cleanup::fixture_process"; + const FIXTURE_ROLE: &str = "OPENBITFUN_PROCESS_CLEANUP_FIXTURE"; + const FIXTURE_DIR: &str = "OPENBITFUN_PROCESS_CLEANUP_DIR"; + const HOST_PID: &str = "OPENBITFUN_PROCESS_CLEANUP_HOST_PID"; + + #[tokio::test] + async fn empty_cleanup_returns_and_handoff_survives_host_exit() { + run_owner_fixture("empty").await; + } + + #[tokio::test] + async fn cleanup_stops_managed_descendants_and_preserves_handoff() { + run_owner_fixture("managed").await; + } + + #[tokio::test] + async fn cleanup_preserves_explicit_service_host_lifetime_containment() { + run_owner_fixture("contained").await; + } + + async fn run_owner_fixture(mode: &str) { + let temporary = tempfile::tempdir().expect("create process cleanup fixture directory"); + let directory = temporary.path(); + let mut command = tokio::process::Command::from(fixture_command(mode, directory)); + command.stderr(Stdio::piped()); + // An outer child Job protects the test runner and removes fixture + // descendants even when an assertion or the old self-kill bug fires. + let mut owner = ProcessTreeChild::spawn(&mut command) + .await + .expect("spawn isolated cleanup owner"); + let mut stderr = owner.take_stderr().expect("capture owner errors"); + let status = tokio::time::timeout(Duration::from_secs(20), owner.wait()) + .await + .expect("cleanup owner must finish") + .expect("wait for cleanup owner"); + let mut errors = String::new(); + stderr.read_to_string(&mut errors).await.unwrap(); + assert!(status.success(), "cleanup owner failed: {errors}"); + // Windows Job self-termination can return exit code 0. An explicit + // post-cleanup marker is required to distinguish it from success. + assert!( + directory.join("cleanup-complete").is_file(), + "host did not return from cleanup: {errors}" + ); + assert!(!directory.join("unexpected-spawn").exists()); + + if mode == "contained" { + let pid = read_pid(&directory.join("lifetime.pid")); + match ProcessHandle::open(pid) { + Ok(process) => process.assert_exited(), + Err(error) => assert_eq!( + error.code(), + windows::core::HRESULT::from_win32(ERROR_INVALID_PARAMETER.0), + "an unavailable fixture PID must already have exited" + ), + } + } else { + for role in ["handoff-before", "handoff-after"] { + wait_for_file(&directory.join(format!("{role}.complete"))).await; + } + } + } + + fn fixture_command(role: &str, directory: &Path) -> std::process::Command { + let mut command = create_command(std::env::current_exe().expect("locate test executable")); + command + .args(["--exact", FIXTURE_TEST, "--nocapture"]) + .env(FIXTURE_ROLE, role) + .env(FIXTURE_DIR, directory) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + command + } + + #[test] + fn fixture_process() { + let Ok(role) = std::env::var(FIXTURE_ROLE) else { + return; + }; + let directory = std::path::PathBuf::from(std::env::var_os(FIXTURE_DIR).unwrap()); + match role.as_str() { + "managed-parent" => { + let _child = fixture_command("managed-leaf", &directory) + .spawn() + .expect("spawn managed descendant"); + idle(); + } + "managed-leaf" | "lifetime" => { + let file = if role == "lifetime" { + "lifetime.pid" + } else { + "descendant.pid" + }; + std::fs::write(directory.join(file), std::process::id().to_string()).unwrap(); + idle(); + } + "handoff-before" | "handoff-after" => { + let host = ProcessHandle::open(std::env::var(HOST_PID).unwrap().parse().unwrap()) + .expect("open handoff parent before it exits"); + std::fs::write(directory.join(format!("{role}.ready")), "ready").unwrap(); + host.assert_exited(); + std::fs::write(directory.join(format!("{role}.complete")), "survived").unwrap(); + } + "unexpected" => { + std::fs::write(directory.join("unexpected-spawn"), "started").unwrap(); + } + "empty" | "managed" | "contained" => { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap() + .block_on(run_cleanup_owner(&role, &directory)); + } + _ => panic!("unknown cleanup fixture role: {role}"), + } + } + + async fn run_cleanup_owner(mode: &str, directory: &Path) { + if mode == "contained" { + contain_current_process_tree().expect("establish explicit host lifetime containment"); + } else { + spawn_handoff("handoff-before", directory).await; + } + + let mut managed = None; + let mut descendant = None; + if mode != "empty" { + let mut command = + tokio::process::Command::from(fixture_command("managed-parent", directory)); + managed = Some(ProcessTreeChild::spawn(&mut command).await.unwrap()); + let pid_file = directory.join("descendant.pid"); + wait_for_file(&pid_file).await; + descendant = Some(ProcessHandle::open(read_pid(&pid_file)).unwrap()); + } + + cleanup_all_processes(); + cleanup_all_processes(); + if let Some(mut tree) = managed { + tokio::time::timeout(Duration::from_secs(5), tree.wait()) + .await + .expect("managed parent must exit after cleanup") + .unwrap(); + descendant.unwrap().assert_exited(); + } + + let mut command = tokio::process::Command::from(fixture_command("unexpected", directory)); + let error = ProcessTreeChild::spawn(&mut command) + .await + .expect_err("shutdown must reject new managed children before they run"); + assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe); + std::fs::write(directory.join("cleanup-complete"), "completed").unwrap(); + + if mode == "contained" { + // Cleanup must leave the explicit host Job intact: even a raw + // descendant started afterwards still ends when this host exits. + let _child = fixture_command("lifetime", directory).spawn().unwrap(); + wait_for_file(&directory.join("lifetime.pid")).await; + } else { + spawn_handoff("handoff-after", directory).await; + } + } + + async fn spawn_handoff(role: &str, directory: &Path) { + let _child = fixture_command(role, directory) + .env(HOST_PID, std::process::id().to_string()) + .spawn() + .expect("start independent handoff process"); + wait_for_file(&directory.join(format!("{role}.ready"))).await; + } + + async fn wait_for_file(path: &Path) { + let deadline = Instant::now() + Duration::from_secs(5); + while !std::fs::metadata(path).is_ok_and(|metadata| metadata.len() > 0) { + assert!(Instant::now() < deadline, "fixture did not write {path:?}"); + tokio::time::sleep(Duration::from_millis(10)).await; + } + } + + fn read_pid(path: &Path) -> u32 { + std::fs::read_to_string(path) + .unwrap() + .trim() + .parse() + .unwrap() + } + + fn idle() -> ! { + loop { + std::thread::park(); + } + } + + struct ProcessHandle(HANDLE); + + impl ProcessHandle { + fn open(pid: u32) -> windows::core::Result { + // SAFETY: the PID comes from an owned fixture; this handle only + // permits waiting and is closed by Drop. + unsafe { OpenProcess(PROCESS_SYNCHRONIZE, false, pid).map(Self) } + } + + fn assert_exited(&self) { + // SAFETY: the handle is owned and valid throughout this wait. + assert_eq!( + unsafe { WaitForSingleObject(self.0, 10_000) }, + WAIT_OBJECT_0, + "fixture process survived its lifecycle boundary" + ); + } + } + + impl Drop for ProcessHandle { + fn drop(&mut self) { + // SAFETY: this handle is owned and closed exactly once. + let _ = unsafe { CloseHandle(self.0) }; + } + } +} diff --git a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionStatusIndicator.scss b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionStatusIndicator.scss index b237b37eb1..54c02093d6 100644 --- a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionStatusIndicator.scss +++ b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionStatusIndicator.scss @@ -8,11 +8,6 @@ &[data-status='running'] > [data-openbitfun-component='icon'] { animation: session-status-running 1.4s linear infinite; } - - &:focus-visible { - outline: var(--openbitfun-border-width-default) solid var(--openbitfun-color-accent-default); - outline-offset: 2px; - } } @keyframes session-status-running { diff --git a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionStatusIndicator.tsx b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionStatusIndicator.tsx index bdd6ea5ecc..c58e38f284 100644 --- a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionStatusIndicator.tsx +++ b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionStatusIndicator.tsx @@ -1,5 +1,5 @@ import { memo } from 'react'; -import { Icon, Tooltip } from '@openbitfun/ui'; +import { Icon } from '@openbitfun/ui'; import { CircleAlert, CircleCheck, CirclePause, CircleStop, CloudOff, Hand, Loader2, MessageCircleQuestion } from 'lucide-react'; import { useI18n } from '@/infrastructure/i18n'; import { useSessionNavStatus } from '@/flow_chat/hooks/useSessionNavStatus'; @@ -27,30 +27,23 @@ export const SessionStatusIndicator = memo(function SessionStatusIndicator({ ses : t(appearance.label) : ''; - return ( - - {appearance ? ( - - - - - ) : null} + return appearance ? ( + + - ); + ) : null; }); diff --git a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.scss b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.scss index 1a335455fd..a3eb0371ad 100644 --- a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.scss +++ b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.scss @@ -238,13 +238,25 @@ white-space: nowrap; } - &__inline-item-status { - display: inline-flex; + // Status and the menu share one trailing cell, including when status is idle. + // Visibility changes never alter the title's available width. + &__inline-item-trailing { + display: grid; flex: 0 0 var(--openbitfun-space-5); - align-items: center; - justify-content: center; + place-items: center; + inline-size: var(--openbitfun-space-5); block-size: var(--openbitfun-space-5); - margin-inline-end: calc(var(--openbitfun-space-5) + var(--openbitfun-space-1)); + } + + &__inline-item-status { + grid-area: 1 / 1; + pointer-events: none; + + .openbitfun-nav-panel__inline-item:hover &, + .openbitfun-nav-panel__inline-item:focus-within &, + .openbitfun-nav-panel__inline-item.is-menu-open & { + visibility: hidden; + } } &__inline-item-label { @@ -408,23 +420,17 @@ } &__inline-item-actions { - position: absolute; - top: 50%; - right: 4px; - transform: translateY(-50%); + grid-area: 1 / 1; display: flex; align-items: center; - gap: 4px; - visibility: hidden; + // Keep the menu button keyboard-reachable; focus-within reveals it before + // paint. Switch immediately so status and actions never overlap mid-fade. opacity: 0; pointer-events: none; - transition: opacity var(--openbitfun-motion-duration-fast) var(--openbitfun-motion-easing-standard), - visibility var(--openbitfun-motion-duration-fast) var(--openbitfun-motion-easing-standard); .openbitfun-nav-panel__inline-item:hover &, .openbitfun-nav-panel__inline-item:focus-within &, &.is-open { - visibility: visible; opacity: 1; pointer-events: auto; } @@ -746,12 +752,6 @@ } } - &__inline-item-actions { - transition: - opacity 100ms ease, - visibility 100ms ease; - } - &__inline-item-action-btn, &__inline-item-edit-btn { border-radius: var(--openbitfun-radius-sm); diff --git a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.tsx b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.tsx index a28a3fb30f..0318e07bdd 100644 --- a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.tsx +++ b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.tsx @@ -1834,23 +1834,28 @@ const SessionsSection: React.FC = ({ ) : null} - -
- + +
{openMenuSessionId === session.sessionId && createPortal( { expect(stylesheet).toContain('margin-top: 0;'); }); - it('reserves an independent status slot so menu actions cannot cover it or shift the title', () => { + it('shares one trailing slot between status and menu without shifting the title', () => { const stylesheet = readSessionsSectionStylesheet(); const inlineItemBlock = extractBlock(stylesheet, '&__inline-item'); const mainBlock = extractInlineItemBlock(stylesheet, 'main'); @@ -56,20 +56,27 @@ describe('SessionsSection layout styles', () => { expect(stylesheet).toContain('&__inline-item-main {\n flex: 1 1 0;'); expect(inlineItemBlock).toContain('position: relative;'); expect(mainBlock).not.toContain('padding-right'); + const trailingBlock = extractInlineItemBlock(stylesheet, 'trailing'); + expect(trailingBlock).toContain('display: grid;'); + expect(trailingBlock).toContain('flex: 0 0 var(--openbitfun-space-5);'); const statusBlock = extractInlineItemBlock(stylesheet, 'status'); - expect(statusBlock).toContain('flex: 0 0 var(--openbitfun-space-5);'); - expect(statusBlock).toContain('margin-inline-end: calc(var(--openbitfun-space-5) + var(--openbitfun-space-1));'); + expect(statusBlock).toContain('grid-area: 1 / 1;'); + expect(statusBlock).not.toContain('margin-inline-end'); + expect(statusBlock).toContain('.openbitfun-nav-panel__inline-item:hover &'); + expect(statusBlock).toContain('.openbitfun-nav-panel__inline-item:focus-within &'); + expect(statusBlock).toContain('.openbitfun-nav-panel__inline-item.is-menu-open &'); + expect(statusBlock).toContain('visibility: hidden;'); expect(stylesheet).not.toContain('padding-right: 24px;'); expect(actionsBlock).not.toContain('display: none;'); - expect(actionsBlock).toContain('position: absolute;'); - expect(actionsBlock).toContain('right: 4px;'); - expect(actionsBlock).toContain('gap: 4px;'); - expect(actionsBlock).toContain('visibility: hidden;'); + expect(actionsBlock).not.toContain('position: absolute;'); + expect(actionsBlock).toContain('grid-area: 1 / 1;'); + expect(actionsBlock).not.toContain('visibility: hidden;'); expect(actionsBlock).toContain('opacity: 0;'); expect(actionsBlock).toContain('pointer-events: none;'); expect(actionsBlock).toContain('.openbitfun-nav-panel__inline-item:hover &'); + expect(actionsBlock).toContain('.openbitfun-nav-panel__inline-item:focus-within &'); expect(actionsBlock).toContain('&.is-open'); - expect(actionsBlock).toContain('visibility: visible;'); + expect(actionsBlock).toContain('opacity: 1;'); }); it('keeps session menu buttons at the compact row size', () => { diff --git a/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx b/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx index 7f367fcef8..feebb56057 100644 --- a/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx +++ b/src/web-ui/src/app/components/NavPanel/sections/workspaces/WorkspaceItem.tsx @@ -122,6 +122,7 @@ const WorkspaceItem: React.FC = ({ ); useGitBasicInfo(workspace.rootPath, gitBasicInfoOptions); const [menuOpen, setMenuOpen] = useState(false); + const [menuContextPoint, setMenuContextPoint] = useState<{ x: number; y: number } | null>(null); const [deleteDialogOpen, setDeleteDialogOpen] = useState(false); const [resetDialogOpen, setResetDialogOpen] = useState(false); const [relatedPathsDialogOpen, setRelatedPathsDialogOpen] = useState(false); @@ -153,6 +154,7 @@ const WorkspaceItem: React.FC = ({ const menuPosition = useSideAnchoredPopoverPosition({ open: menuOpen, anchorRef: menuAnchorRef, + anchorPoint: menuContextPoint, popoverRef: menuPopoverRef, layoutRevision: `${acpClientsLoading}:${acpClients.length}`, }); @@ -446,9 +448,19 @@ const WorkspaceItem: React.FC = ({ }, [tFiles, workspaceSearchIndex]); const handleMenuTriggerClick = useCallback(() => { + setMenuContextPoint(null); setMenuOpen(open => !open); }, []); + const handleContextMenu = useCallback((event: React.MouseEvent) => { + // Portalled menus bubble through the card without belonging to its DOM subtree. + if (!event.currentTarget.contains(event.target as Node)) return; + event.preventDefault(); + event.stopPropagation(); + setMenuContextPoint({ x: event.clientX, y: event.clientY }); + setMenuOpen(true); + }, []); + useEffect(() => { if (!menuOpen) return; const handleOutside = (event: MouseEvent) => { @@ -826,6 +838,7 @@ const WorkspaceItem: React.FC = ({ onDragStart={onDragStart} onDragEnd={onDragEnd} onClick={handleCollapseToggle} + onContextMenu={handleContextMenu} style={{ cursor: 'pointer' }} data-testid="nav-workspace-card" data-workspace-id={workspace.id} @@ -1114,6 +1127,7 @@ const WorkspaceItem: React.FC = ({ onDragStart={onDragStart} onDragEnd={onDragEnd} onClick={handleCollapseToggle} + onContextMenu={handleContextMenu} style={{ cursor: 'pointer' }} data-testid="nav-workspace-card" data-workspace-id={workspace.id} diff --git a/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.appearance.ts b/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.appearance.ts index aba171a627..d31e978d2e 100644 --- a/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.appearance.ts +++ b/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.appearance.ts @@ -9,7 +9,7 @@ export const chatInputApprovalBandAppearanceDescriptor: AppearanceSurfaceDescrip id: 'permission-request-panel', parts: [ { id: 'root' }, { id: 'request' }, { id: 'risk' }, { id: 'error' }, - { id: 'actions' }, { id: 'scope' }, + { id: 'actions' }, { id: 'scope' }, { id: 'grantScope' }, ], states: [ { id: 'responding', selector: { kind: 'self', suffix: '[data-openbitfun-state~="responding"]' } }, diff --git a/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.scss b/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.scss index ce6910d471..6be22f0d36 100644 --- a/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.scss +++ b/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.scss @@ -1,157 +1,67 @@ // The approval band normally sits in the composer stack, directly above the // capsule. Embedded child-session panels reuse the same compact surface when // no child composer exists. +@use '@/shared/styles/surface-recipes' as surfaces; + .openbitfun-chat-input-approval { - display: flex; - flex-direction: column; - gap: 5px; box-sizing: border-box; // The composer centers its children, so constrain the band before laying out // long resources; otherwise its intrinsic width can exceed the composer. width: 100%; min-width: 0; - max-width: 100%; - margin-bottom: 6px; - padding: 8px 10px; - border: 1px solid var(--openbitfun-color-status-warning-border); - border-radius: 12px; - background: var(--openbitfun-color-status-warning-surface); - color: var(--openbitfun-color-content-primary); - font-size: var(--openbitfun-type-flow-control-font-size); - animation: openbitfun-chat-input-approval-in 160ms cubic-bezier(0.23, 1, 0.32, 1); - - &__request { - display: flex; - min-width: 0; - align-items: center; - gap: 5px; - } + max-width: min(100%, var(--openbitfun-overlay-dialog-max-inline-size-large)); + margin-inline: auto; + margin-bottom: var(--openbitfun-space-3); - &__icon { - flex: none; - color: var(--openbitfun-color-status-warning-emphasis); + &__surface { + @include surfaces.floating; } - &__action { - flex: none; - font-weight: var(--openbitfun-type-label-selected-font-weight); + &__title { + flex-shrink: 0; } - &__separator { - flex: none; + &__owner { color: var(--openbitfun-color-content-secondary); + font-size: var(--openbitfun-type-flow-support-font-size); + font-weight: var(--openbitfun-type-flow-support-font-weight); + line-height: var(--openbitfun-type-flow-support-line-height); } // Keep the full command readable without letting it widen the composer or // push the approval actions out of view. Only the resource body scrolls. - .openbitfun-chat-input-approval__resource { - flex: none; - min-width: 0; + &__resource { + flex: 1; max-height: min(160px, 25vh); - overflow: auto; - white-space: pre-wrap; - overflow-wrap: anywhere; - } - - &__owner { - flex: 0 1 auto; - min-width: 0; - overflow: hidden; - color: var(--openbitfun-color-content-secondary); - font-size: var(--openbitfun-type-flow-support-font-size); - white-space: nowrap; + color: var(--openbitfun-color-content-primary); + font-size: var(--openbitfun-type-code-sm-font-size); + line-height: var(--openbitfun-type-code-sm-line-height); } - &__count { - flex: none; - margin-left: auto; - padding: 1px 6px; - border-radius: 8px; - color: var(--openbitfun-color-content-on-light); - background: var(--openbitfun-color-status-warning-emphasis); - font-size: var(--openbitfun-type-flow-support-font-size); - font-weight: var(--openbitfun-type-heading-page-font-weight); - cursor: default; + &__note { + overflow-wrap: anywhere; } - &__note { + &__grant-note { margin: 0; - overflow-wrap: anywhere; - color: var(--openbitfun-color-status-warning-content); + color: var(--openbitfun-color-content-secondary); font-size: var(--openbitfun-type-flow-support-font-size); - - &--error { - color: var(--openbitfun-color-status-danger-content); - } + font-weight: var(--openbitfun-type-flow-support-font-weight); + line-height: var(--openbitfun-type-flow-support-line-height); + overflow-wrap: anywhere; } &__actions { - display: flex; flex-wrap: wrap; - align-items: center; - gap: 6px; - } - - &__spacer { - flex: 1 1 auto; } - &__scope { - display: inline-flex; - flex: none; + &__buttons { + display: flex; + min-width: 0; + flex-wrap: wrap; align-items: center; - padding: 2px; - border: 1px solid var(--openbitfun-color-border-default); - border-radius: 7px; - background: var(--openbitfun-color-surface-canvas); - } - - &__scope-option { - min-height: 20px; - padding: 1px 7px; - border: none; - border-radius: 5px; - color: var(--openbitfun-color-content-secondary); - background: transparent; - font-size: var(--openbitfun-type-flow-support-font-size); - cursor: pointer; - - &--active { - color: var(--openbitfun-color-content-primary); - background: color-mix(in srgb, var(--openbitfun-color-status-warning-emphasis) 18%, transparent); - } - - &:focus-visible { - outline: 2px solid var(--openbitfun-color-status-warning-emphasis); - outline-offset: 1px; - } - } - -} - -// A narrow composer drops secondary identity information while the shared -// buttons wrap as complete, readable actions. -@media (max-width: 560px) { - .openbitfun-chat-input-approval { - &__owner { - display: none; - } - } -} - -@keyframes openbitfun-chat-input-approval-in { - from { - opacity: 0; - transform: translateY(3px); - } - to { - opacity: 1; - transform: translateY(0); - } -} - -@media (prefers-reduced-motion: reduce) { - .openbitfun-chat-input-approval { - animation: none; + justify-content: flex-end; + gap: var(--openbitfun-space-2); + margin-inline-start: auto; } } diff --git a/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.test.tsx b/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.test.tsx index 086e3107dd..dffda09b76 100644 --- a/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.test.tsx +++ b/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.test.tsx @@ -4,6 +4,7 @@ import React, { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { PermissionRequest } from '@/infrastructure/api/service-api/AgentAPI'; +import { copyTextToClipboard } from '@/shared/utils/textSelection'; import { ChatInputApprovalBand } from './ChatInputApprovalBand'; globalThis.IS_REACT_ACT_ENVIRONMENT = true; @@ -13,7 +14,9 @@ const TRANSLATIONS: Record = { 'permission.actions.bash': 'Run command', 'permission.actions.other': 'Other action', 'permission.allowOnce': 'Allow once', - 'permission.allowAlways': 'Always allow', + 'permission.allowAlways': 'Always allow this scope', + 'permission.allowAlwaysCommand': 'Always allow this command', + 'permission.allowAlwaysCommandDescription': 'Applies only to this exact command, including its arguments, in the current project. Other commands are not authorized.', 'permission.allowCurrentAndFollowing': 'Allow all', 'permission.reject': 'Reject', 'permission.rejectCurrentAndFollowing': 'Reject all', @@ -21,14 +24,17 @@ const TRANSLATIONS: Record = { 'permission.responseFailed': 'The reply could not be delivered.', 'permission.scopeThis': 'This one', 'permission.scopeAll': 'All', + 'toolCards.common.copy': 'Copy', + 'toolCards.common.copied': 'Copied', }; -vi.mock('react-i18next', () => ({ +vi.mock('react-i18next', async importOriginal => ({ + ...await importOriginal(), useTranslation: () => ({ t: (key: string, values?: Record) => { if (key === 'permission.subagentOwner') return `${values?.subagent} subagent`; if (key === 'permission.allowAlwaysTooltip') { - return `Always allow saves matching access for ${values?.projectPath}`; + return `Remember approval in ${values?.projectPath}, limited to:\n${values?.resources}`; } if (key === 'permission.risks.pageSave') { return `Save ${values?.slug} as ${values?.visibility} without deploying.`; @@ -43,10 +49,9 @@ vi.mock('@openbitfun/ui', async importOriginal => ({ Tooltip: ({ children }: { children: React.ReactElement }) => <>{children}, })); -vi.mock('./CopyableTextPreview', () => ({ - CopyableTextPreview: ({ text, className }: { text: string; className?: string }) => ( - {text} - ), +vi.mock('@/shared/utils/textSelection', async importOriginal => ({ + ...await importOriginal(), + copyTextToClipboard: vi.fn(async () => true), })); function request(overrides: Partial = {}): PermissionRequest { @@ -91,11 +96,17 @@ describe('ChatInputApprovalBand', () => { }); }; + const scopeOption = (value: 'this' | 'all') => container.querySelector( + `[data-testid="chat-input-approval-scope"] [role="radio"][data-openbitfun-value="${value}"]`, + ); + it('says what is being asked for and who is asking', async () => { + const resources = ['src/main.rs', 'src/components/permissions/confirmation.rs']; await act(async () => { root.render( { expect(band?.textContent).toContain('Edit files'); expect(band?.textContent).toContain('src/main.rs'); expect(band?.textContent).toContain('Explore subagent'); + const resource = band?.querySelector('code'); + expect(resource?.textContent).toBe(resources.join('\n')); + expect(resource?.querySelector('[data-openbitfun-component="overflow-text"]')).toBeNull(); }); it('keeps the risk on its own line so it cannot be answered unread', async () => { @@ -152,7 +166,7 @@ describe('ChatInputApprovalBand', () => { }); // A lone request has nothing to scope, so the toggle stays out of the way. - expect(container.querySelector('[data-testid="chat-input-approval-scope-all"]')).toBeNull(); + expect(scopeOption('all')).toBeNull(); expect(container.querySelector('[data-testid="chat-input-approval-pending-count"]')).toBeNull(); await click('chat-input-approval-allow'); @@ -182,7 +196,25 @@ describe('ChatInputApprovalBand', () => { ?.dataset.approvalScope, ).toBe('this'); - await click('chat-input-approval-scope-all'); + expect(scopeOption('this')?.tabIndex).toBe(0); + expect(scopeOption('all')?.tabIndex).toBe(-1); + await act(async () => { + scopeOption('this')?.focus(); + scopeOption('this')?.dispatchEvent(new KeyboardEvent('keydown', { + key: 'ArrowRight', bubbles: true, + })); + }); + expect(document.activeElement).toBe(scopeOption('all')); + expect(scopeOption('all')?.getAttribute('aria-checked')).toBe('true'); + expect(onRespond).not.toHaveBeenCalled(); + expect(onRespondBatch).not.toHaveBeenCalled(); + await act(async () => { + scopeOption('all')?.dispatchEvent(new KeyboardEvent('keydown', { + key: 'Home', bubbles: true, + })); + }); + expect(scopeOption('this')?.getAttribute('aria-checked')).toBe('true'); + await act(async () => scopeOption('all')?.click()); const band = container.querySelector('[data-testid="chat-input-approval-band"]'); expect(band?.dataset.approvalScope).toBe('all'); expect(band?.textContent).toContain('Reject all'); @@ -250,7 +282,7 @@ describe('ChatInputApprovalBand', () => { await act(async () => { root.render( undefined)} /> @@ -261,6 +293,9 @@ describe('ChatInputApprovalBand', () => { expect(container.querySelector('[data-openbitfun-part="error"]')?.textContent).toBe( 'The reply could not be delivered.', ); + expect(container.querySelector('[data-openbitfun-part="risk"]')?.textContent).toBe( + 'This changes project files.', + ); expect( container.querySelector('[data-testid="chat-input-approval-band"]') ?.dataset.openbitfunState, @@ -270,6 +305,115 @@ describe('ChatInputApprovalBand', () => { ).toBe(false); }); + it('describes the exact command and project scope before saving its approval', async () => { + const command = 'git status --short'; + const onRespond = vi.fn(async () => undefined); + const onRespondBatch = vi.fn(async () => undefined); + await act(async () => { + root.render( + , + ); + }); + + const allowAlways = container.querySelector('[data-testid="chat-input-approval-allow-always"]'); + const descriptionId = allowAlways?.getAttribute('aria-describedby'); + expect(allowAlways?.textContent).toBe('Always allow this command'); + expect(descriptionId).toBeTruthy(); + expect(document.getElementById(descriptionId!)?.textContent).toBe( + 'Applies only to this exact command, including its arguments, in the current project. Other commands are not authorized.', + ); + + await click('chat-input-approval-allow-always'); + expect(onRespond).toHaveBeenCalledWith('request-1', 'always', undefined); + expect(onRespondBatch).not.toHaveBeenCalled(); + }); + + it.each([ + { action: 'bash', resources: ['git status'], saveResources: ['git diff'] }, + { action: 'edit', resources: ['src/main.rs'], saveResources: ['src/*'] }, + ])('labels $action saved resources as a scope when they differ from the request', async overrides => { + await act(async () => { + root.render( + undefined)} + onRespondBatch={vi.fn(async () => undefined)} + />, + ); + }); + + expect(container.querySelector('[data-testid="chat-input-approval-allow-always"]')?.textContent).toBe( + 'Always allow this scope', + ); + expect(container.querySelector('[data-openbitfun-part="grantScope"]')).toBeNull(); + }); + + it('copies the complete command without repeating the tool name or answering permission', async () => { + const resources = ['Get-Location;', ' Get-ChildItem -LiteralPath "my project" -Name']; + const onRespond = vi.fn(async () => undefined); + const onRespondBatch = vi.fn(async () => undefined); + await act(async () => { + root.render( + , + ); + }); + + expect(container.querySelector('[data-openbitfun-part="request"]')?.textContent).toBe('Run command'); + await click('chat-input-approval-copy'); + expect(copyTextToClipboard).toHaveBeenCalledWith(resources.join('\n')); + expect(container.querySelector('[data-testid="chat-input-approval-copy"]')?.getAttribute('aria-label')).toBe('Copied'); + expect(onRespond).not.toHaveBeenCalled(); + expect(onRespondBatch).not.toHaveBeenCalled(); + }); + + it('shows the active response as busy and prevents changing scope while it is delivered', async () => { + let finishResponse!: () => void; + const pendingResponse = new Promise(resolve => { finishResponse = resolve; }); + const onRespond = vi.fn(() => pendingResponse); + await act(async () => { + root.render( + undefined)} + />, + ); + }); + + await click('chat-input-approval-allow'); + const allow = container.querySelector('[data-testid="chat-input-approval-allow"]'); + expect(allow?.getAttribute('aria-busy')).toBe('true'); + const approvalButtons = container.querySelectorAll( + '[data-openbitfun-component="permission-request-panel"][data-openbitfun-part="actions"] button', + ); + expect(Array.from(approvalButtons).every(button => button.disabled)).toBe(true); + expect(container.querySelector('[data-testid="chat-input-approval-copy"]')?.disabled).toBe(false); + await act(async () => { + allow?.click(); + scopeOption('all')?.click(); + }); + expect(onRespond).toHaveBeenCalledTimes(1); + expect(scopeOption('this')?.getAttribute('aria-checked')).toBe('true'); + + await act(async () => finishResponse()); + expect(allow?.disabled).toBe(false); + expect(allow?.hasAttribute('aria-busy')).toBe(false); + expect(scopeOption('all')?.disabled).toBe(false); + }); + it('renders nothing when there is nothing to approve', async () => { await act(async () => { root.render( diff --git a/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.tsx b/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.tsx index dbe21c3dfd..0690e16cfc 100644 --- a/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.tsx +++ b/src/web-ui/src/flow_chat/components/ChatInputApprovalBand.tsx @@ -11,15 +11,29 @@ * their own; those surfaces intentionally omit the optional typed reason. */ -import React, { useState } from 'react'; -import { OverflowText, Button } from '@openbitfun/ui'; +import React, { useId, useState } from 'react'; +import { + Alert, + Button, + Card, + CardFooter, + CardHeader, + Icon, + IconButton, + NumberBadge, + OverflowText, + ScrollArea, + SegmentedControl, + Stack, + Tooltip, +} from '@openbitfun/ui'; import { ShieldAlert } from 'lucide-react'; import { useTranslation } from 'react-i18next'; -import { Tooltip, Icon } from '@openbitfun/ui'; import type { PermissionReplyKind, PermissionRequest, } from '@/infrastructure/api/service-api/AgentAPI'; +import { useCopyTextAction } from '../hooks/useCopyTextAction'; import { CopyableTextPreview } from './CopyableTextPreview'; import './ChatInputApprovalBand.scss'; @@ -108,6 +122,7 @@ function permissionRisk( } type ApprovalScope = 'this' | 'all'; +type ApprovalAnswer = PermissionReplyKind | 'rejectWithReason'; export const ChatInputApprovalBand: React.FC = ({ requests, @@ -118,9 +133,11 @@ export const ChatInputApprovalBand: React.FC = ({ onRespondBatch, }) => { const { t } = useTranslation('flow-chat'); + const grantDescriptionId = useId(); const [scope, setScope] = useState('this'); - const [responding, setResponding] = useState(false); + const [pendingAnswer, setPendingAnswer] = useState(null); const [error, setError] = useState(false); + const responding = pendingAnswer !== null; const request = requests[0]; const pendingCount = Math.max(totalPendingCount ?? requests.length, requests.length); @@ -129,18 +146,47 @@ export const ChatInputApprovalBand: React.FC = ({ const canAnswerAll = pendingCount > 1; const effectiveScope: ApprovalScope = canAnswerAll ? scope : 'this'; const reason = rejectReason.trim(); + const resourceSummary = request?.resources.join('\n') ?? ''; + const { copied, copy } = useCopyTextAction({ + getText: () => resourceSummary, + successMessage: t('toolCards.common.copied'), + failureMessage: t('toolCards.common.copyFailed'), + showSuccessNotification: false, + }); + const copyLabel = copied ? t('toolCards.common.copied') : t('toolCards.common.copy'); if (!request) return null; const risk = permissionRisk(request, t); - const alwaysAllowTooltip = request.saveResources?.length + // Built-in tool names repeat the action. External identities and delegated + // owners provide additional context that the action label cannot express. + const showSourceIdentity = request.source.kind !== 'tool_call' + || request.action === 'mcp' + || request.action === 'custom_tool' + || !PERMISSION_ACTION_LABEL_KEYS[request.action]; + const ownerLabel = request.delegation + ? t('permission.subagentOwner', { subagent: request.delegation.subagentType }) + : showSourceIdentity ? request.source.identity : undefined; + const saveResources = request.saveResources ?? []; + const isExactCommandGrant = request.action === 'bash' + && request.resources.length === 1 + && saveResources.length === 1 + && saveResources[0] === request.resources[0]; + const alwaysAllowLabel = isExactCommandGrant + ? t('permission.allowAlwaysCommand') + : t('permission.allowAlways'); + const savedResourceSummary = saveResources.join('\n'); + const alwaysAllowTooltip = saveResources.length ? request.projectPath?.trim() - ? t('permission.allowAlwaysTooltip', { projectPath: request.projectPath.trim() }) - : t('permission.allowAlwaysTooltipCurrentProject') + ? t('permission.allowAlwaysTooltip', { + projectPath: request.projectPath.trim(), + resources: savedResourceSummary, + }) + : t('permission.allowAlwaysTooltipCurrentProject', { resources: savedResourceSummary }) : t('permission.allowAlwaysTooltipNoGrant'); const answer = async (reply: PermissionReplyKind, withReason: boolean) => { - setResponding(true); + setPendingAnswer(withReason ? 'rejectWithReason' : reply); setError(false); const feedback = reply === 'reject' && withReason && reason ? reason : undefined; try { @@ -156,11 +202,10 @@ export const ChatInputApprovalBand: React.FC = ({ } catch { setError(true); } finally { - setResponding(false); + setPendingAnswer(null); } }; - const resourceSummary = request.resources.join('\n'); const answersAll = effectiveScope === 'all'; const allowLabel = answersAll ? t('permission.allowCurrentAndFollowing') @@ -177,172 +222,189 @@ export const ChatInputApprovalBand: React.FC = ({ className="openbitfun-chat-input-approval" role="group" aria-label={t('permission.title')} + aria-busy={responding || undefined} data-testid="chat-input-approval-band" data-approval-scope={effectiveScope} > -
- - - {permissionActionLabel(request.action, t)} - - · - {request.delegation ? ( - - {t('permission.subagentOwner', { subagent: request.delegation.subagentType })} - - ) : ( - {request.source.identity} - )} - {canAnswerAll ? ( - - - +{pendingCount - 1} - - - ) : null} -
- - - - {/* The risk is the reason to read the band at all, so it keeps its own - line rather than hiding in a tooltip. */} - {error ? ( -

- {t('permission.responseFailed')} -

- ) : risk ? ( -

- {risk} -

- ) : null} + } + title={ + + + {permissionActionLabel(request.action, t)} + + {ownerLabel ? ( + + {ownerLabel} + + ) : null} + + } + actions={canAnswerAll ? ( + + + + ) : undefined} + /> + -
- {canAnswerAll ? ( + + + + + + {resourceSummary.trim() ? ( + + + } + onClick={copy} + /> + + ) : null} + + + + {/* Keep the risk visible while a failed response is retried. */} + {risk ? (
- {(['this', 'all'] as const).map(option => ( - - ))} + +
+ ) : null} + {error ? ( +
+
) : null} - - - {/* Rejecting is the safe answer, so it leads and never depends on - anything else being in the right state. */} - - {/* The composer is the reason field. It is offered rather than assumed, - so a half-typed next message cannot become a rejection reason and - typing one cannot block the allow buttons. */} - {reason ? ( - - - + {isExactCommandGrant && !answersAll ? ( +

+ {t('permission.allowAlwaysCommandDescription')} +

) : null} - - {/* "Always" writes a saved grant, so it is only offered when this - request has a scope to save, and only for the request in front of - the reader — a saved grant is not something to apply in bulk to - requests they have not read. */} - {request.saveResources?.length && !answersAll ? ( - - - - ) : null} -
+ + {canAnswerAll ? ( +
+ setScope(value === 'all' ? 'all' : 'this')} + /> +
+ ) : null} + +
+ {/* Rejecting is the safe answer, so it leads. */} + + {/* A draft becomes a reason only through this explicit action. */} + {reason ? ( + + + + ) : null} + {/* Saved grants apply only to the request the reader has seen. */} + {saveResources.length > 0 && !answersAll ? ( + + + + ) : null} + +
+ + + ); }; diff --git a/src/web-ui/src/flow_chat/components/CopyableTextPreview.scss b/src/web-ui/src/flow_chat/components/CopyableTextPreview.scss index 965faf341d..955b340655 100644 --- a/src/web-ui/src/flow_chat/components/CopyableTextPreview.scss +++ b/src/web-ui/src/flow_chat/components/CopyableTextPreview.scss @@ -10,6 +10,20 @@ font-family:var(--openbitfun-type-body-sm-font-family); } +.copyable-text-preview--multiline { + display: block; + padding: 0; + border: none; + border-radius: 0; + background: none; + font-size: inherit; + font-weight: var(--openbitfun-type-body-sm-font-weight); + line-height: inherit; + overflow: visible; + white-space: pre-wrap; + overflow-wrap: anywhere; +} + .copyable-text-preview--compact { font-size: var(--openbitfun-type-flow-support-font-size); line-height: var(--openbitfun-type-flow-support-line-height); diff --git a/src/web-ui/src/flow_chat/components/CopyableTextPreview.tsx b/src/web-ui/src/flow_chat/components/CopyableTextPreview.tsx index 91ac40787f..41bc11a01d 100644 --- a/src/web-ui/src/flow_chat/components/CopyableTextPreview.tsx +++ b/src/web-ui/src/flow_chat/components/CopyableTextPreview.tsx @@ -9,6 +9,8 @@ interface CopyableTextPreviewProps extends React.HTMLAttributes { text?: string | null; emptyText: React.ReactNode; as?: 'span' | 'code'; + /** Preserve full resource details instead of using single-line overflow. */ + multiline?: boolean; className?: string; tooltipContent?: React.ReactNode; tooltipPlacement?: 'top' | 'bottom' | 'left' | 'right'; @@ -18,6 +20,7 @@ export const CopyableTextPreview = React.forwardRef{emptyText}; - const resolvedClassName = `copyable-text-preview${className ? ` ${className}` : ''}`; + const resolvedClassName = `copyable-text-preview${multiline ? ' copyable-text-preview--multiline' : ''}${className ? ` ${className}` : ''}`; + const preview = multiline ? content : ( + {content} + ); const copyText = typeof tooltipContent === 'string' && tooltipContent.trim() ? tooltipContent : undefined; @@ -40,11 +46,11 @@ export const CopyableTextPreview = React.forwardRef - {content} + {preview} ) : ( - {content} + {preview} ); diff --git a/src/web-ui/src/infrastructure/update/UpdateInstallProgressModal.scss b/src/web-ui/src/infrastructure/update/UpdateInstallProgressModal.scss index 35bd48e267..299acff59a 100644 --- a/src/web-ui/src/infrastructure/update/UpdateInstallProgressModal.scss +++ b/src/web-ui/src/infrastructure/update/UpdateInstallProgressModal.scss @@ -1,6 +1,41 @@ .openbitfun-update-progress { - padding: 8px 0 4px; - min-width: 280px; + min-inline-size: 0; +} + +.openbitfun-update-progress__ready { + display: flex; + align-items: flex-start; + gap: var(--openbitfun-space-2); + font-family: var(--openbitfun-type-body-md-font-family); + font-size: var(--openbitfun-type-body-md-font-size); + font-weight: var(--openbitfun-type-body-md-font-weight); + line-height: var(--openbitfun-type-body-md-line-height); + letter-spacing: var(--openbitfun-type-body-md-letter-spacing); +} + +.openbitfun-update-progress__status-icon { + display: inline-flex; + flex: 0 0 auto; + align-items: center; + block-size: 1lh; +} + +.openbitfun-update-progress__summary { + display: flex; + flex: 1; + flex-direction: column; + gap: var(--openbitfun-space-2); + min-inline-size: 0; + overflow-wrap: anywhere; +} + +.openbitfun-update-progress__version { + margin: 0; + color: var(--openbitfun-color-content-primary); +} + +.openbitfun-update-progress__ready + * { + margin-block-start: var(--openbitfun-space-4); } .openbitfun-update-progress__bar { @@ -53,9 +88,11 @@ .openbitfun-update-progress__actions { display: flex; + flex-wrap: wrap; + align-items: center; justify-content: flex-end; - gap: 8px; - margin-top: 12px; + gap: var(--openbitfun-space-2); + min-inline-size: 0; } @media (prefers-reduced-motion: reduce) { diff --git a/src/web-ui/src/infrastructure/update/UpdateInstallProgressModal.tsx b/src/web-ui/src/infrastructure/update/UpdateInstallProgressModal.tsx index 0dd9b50a99..8da1aaa028 100644 --- a/src/web-ui/src/infrastructure/update/UpdateInstallProgressModal.tsx +++ b/src/web-ui/src/infrastructure/update/UpdateInstallProgressModal.tsx @@ -1,5 +1,5 @@ /** - * Full-screen style modal showing download progress for in-app updates. + * Download progress and installation confirmation for in-app updates. */ import { @@ -8,9 +8,12 @@ import { Dialog, DialogBody, DialogClose, + DialogDescription, + DialogFooter, DialogHeader, DialogHeading, DialogTitle, + Icon, } from '@openbitfun/ui'; import React, { useMemo } from 'react'; import { useI18n } from '@/infrastructure/i18n'; @@ -83,29 +86,29 @@ export const UpdateInstallProgressModal: React.FC -
- -
-

{t('update.installWarning')}

- {errorMessage ? : null} -
- {errorMessage && onDownloadAgain ? ( - - ) : null} - - +
+ +
+

+ {t('update.readyVersion', { version: version ?? '' })} +

+
+ {t('update.installWarning')} +
+
+ {errorMessage ? ( +
+ +
+ ) : null} ); } else { @@ -155,16 +158,37 @@ export const UpdateInstallProgressModal: React.FC {!installing && (!!error || !!installed) && } - -
- {body} -
- + +
+ {body} +
+ + {installed ? ( + +
+ {errorMessage && onDownloadAgain ? ( + + ) : null} + + +
+ + ) : null} ); }; diff --git a/src/web-ui/src/locales/en-US/common.json b/src/web-ui/src/locales/en-US/common.json index 0de37d0efc..60b53b8189 100644 --- a/src/web-ui/src/locales/en-US/common.json +++ b/src/web-ui/src/locales/en-US/common.json @@ -1115,7 +1115,7 @@ "downloadingTitle": "Downloading update", "downloadFailedTitle": "Update could not be installed", "installedTitle": "Update ready", - "readyVersion": "Version {{version}} is downloaded and ready to install.", + "readyVersion": "Version {{version}} is downloaded", "installWarning": "Installing will restart OpenBitFun on this device and interrupt its active sessions.", "installAndRestart": "Install and restart", "installing": "Installing…", diff --git a/src/web-ui/src/locales/en-US/flow-chat.json b/src/web-ui/src/locales/en-US/flow-chat.json index d1fc92f993..8324b17cb8 100644 --- a/src/web-ui/src/locales/en-US/flow-chat.json +++ b/src/web-ui/src/locales/en-US/flow-chat.json @@ -4,9 +4,11 @@ "subagentOwner": "{{subagent}} subagent", "responseFailed": "The permission response could not be delivered. Try again.", "allowOnce": "Allow once", - "allowAlways": "Always allow", - "allowAlwaysTooltip": "Always allow saves matching access for {{projectPath}}.", - "allowAlwaysTooltipCurrentProject": "Always allow saves matching access for the current project.", + "allowAlways": "Always allow this scope", + "allowAlwaysCommand": "Always allow this command", + "allowAlwaysCommandDescription": "Applies only to this exact command, including its arguments, in the current project. Other commands are not authorized.", + "allowAlwaysTooltip": "Remember approval for this action in {{projectPath}}, limited to the following scope:\n{{resources}}", + "allowAlwaysTooltipCurrentProject": "Remember approval for this action in the current project, limited to the following scope:\n{{resources}}", "allowAlwaysTooltipNoGrant": "This request has no savable access scope; always allow applies only to this request.", "risks": { "pageSave": "Save a new immutable version of “{{slug}}” with {{visibility}} visibility without changing production.", diff --git a/src/web-ui/src/locales/zh-CN/common.json b/src/web-ui/src/locales/zh-CN/common.json index d5cccb73e4..4d13976e8c 100644 --- a/src/web-ui/src/locales/zh-CN/common.json +++ b/src/web-ui/src/locales/zh-CN/common.json @@ -1115,7 +1115,7 @@ "downloadingTitle": "正在下载更新", "downloadFailedTitle": "无法完成更新", "installedTitle": "更新已准备就绪", - "readyVersion": "版本 {{version}} 已下载，可以安装并重启。", + "readyVersion": "版本 {{version}} 已下载", "installWarning": "安装将重启本设备上的 OpenBitFun，并中断此设备上的活动会话。", "installAndRestart": "安装并重启", "installing": "正在安装…", diff --git a/src/web-ui/src/locales/zh-CN/flow-chat.json b/src/web-ui/src/locales/zh-CN/flow-chat.json index 64956e69c1..67d85e2797 100644 --- a/src/web-ui/src/locales/zh-CN/flow-chat.json +++ b/src/web-ui/src/locales/zh-CN/flow-chat.json @@ -4,9 +4,11 @@ "subagentOwner": "{{subagent}} 子 Agent", "responseFailed": "权限回复未能送达，请重试。", "allowOnce": "允许一次", - "allowAlways": "始终允许", - "allowAlwaysTooltip": "始终允许会为 {{projectPath}} 保存匹配的访问权限。", - "allowAlwaysTooltipCurrentProject": "始终允许会为当前项目保存匹配的访问权限。", + "allowAlways": "始终允许此范围", + "allowAlwaysCommand": "始终允许此命令", + "allowAlwaysCommandDescription": "仅对当前项目中完全相同的命令（含参数）生效，不会放行其他命令。", + "allowAlwaysTooltip": "在 {{projectPath}} 中记住当前操作的授权，仅对以下范围生效：\n{{resources}}", + "allowAlwaysTooltipCurrentProject": "在当前项目中记住当前操作的授权，仅对以下范围生效：\n{{resources}}", "allowAlwaysTooltipNoGrant": "此请求没有可保存的访问范围；始终允许仅应用于本次请求。", "risks": { "pageSave": "以{{visibility}}可见性保存“{{slug}}”的新不可变版本，不改变当前生产版本。", diff --git a/src/web-ui/src/locales/zh-TW/common.json b/src/web-ui/src/locales/zh-TW/common.json index a1fa846e7a..825a60a1c1 100644 --- a/src/web-ui/src/locales/zh-TW/common.json +++ b/src/web-ui/src/locales/zh-TW/common.json @@ -1115,7 +1115,7 @@ "downloadingTitle": "正在下載更新", "downloadFailedTitle": "無法完成更新", "installedTitle": "更新已準備就緒", - "readyVersion": "版本 {{version}} 已下載，可以安裝並重啟。", + "readyVersion": "版本 {{version}} 已下載", "installWarning": "安裝將重啟本裝置上的 OpenBitFun，並中斷此裝置上的活動會話。", "installAndRestart": "安裝並重啟", "installing": "正在安裝…", diff --git a/src/web-ui/src/locales/zh-TW/flow-chat.json b/src/web-ui/src/locales/zh-TW/flow-chat.json index 85a40b0cc0..a8e519e965 100644 --- a/src/web-ui/src/locales/zh-TW/flow-chat.json +++ b/src/web-ui/src/locales/zh-TW/flow-chat.json @@ -4,9 +4,11 @@ "subagentOwner": "{{subagent}} 子 Agent", "responseFailed": "權限回覆未能送達，請重試。", "allowOnce": "允許一次", - "allowAlways": "始終允許", - "allowAlwaysTooltip": "始終允許會為 {{projectPath}} 儲存相符的存取權限。", - "allowAlwaysTooltipCurrentProject": "始終允許會為目前項目儲存相符的存取權限。", + "allowAlways": "始終允許此範圍", + "allowAlwaysCommand": "始終允許此命令", + "allowAlwaysCommandDescription": "僅對目前專案中完全相同的命令（含參數）生效，不會放行其他命令。", + "allowAlwaysTooltip": "在 {{projectPath}} 中記住目前操作的授權，僅對以下範圍生效：\n{{resources}}", + "allowAlwaysTooltipCurrentProject": "在目前專案中記住目前操作的授權，僅對以下範圍生效：\n{{resources}}", "allowAlwaysTooltipNoGrant": "此請求沒有可儲存的存取範圍；始終允許僅套用於這次請求。", "risks": { "pageSave": "以{{visibility}}可見性儲存「{{slug}}」的新不可變版本，不變更目前正式版本。", diff --git a/src/web-ui/src/shared/utils/useSideAnchoredPopoverPosition.ts b/src/web-ui/src/shared/utils/useSideAnchoredPopoverPosition.ts index 686ffaa651..fd239199ab 100644 --- a/src/web-ui/src/shared/utils/useSideAnchoredPopoverPosition.ts +++ b/src/web-ui/src/shared/utils/useSideAnchoredPopoverPosition.ts @@ -19,6 +19,8 @@ export interface SideAnchoredPopoverLayout { interface UseSideAnchoredPopoverPositionOptions { open: boolean; anchorRef: RefObject; + /** Viewport point used when a context menu is opened by right click. */ + anchorPoint?: { x: number; y: number } | null; popoverRef: RefObject; preferredPlacement?: SideAnchoredPopoverPlacement; gap?: number; @@ -35,10 +37,11 @@ const sameLayout = ( && current.placement === next.placement && current.alignment === next.alignment; -/** Keeps a portalled submenu beside its owning menu item without viewport clipping. */ +/** Keeps a portalled menu beside its trigger or context point without viewport clipping. */ export function useSideAnchoredPopoverPosition({ open, anchorRef, + anchorPoint, popoverRef, preferredPlacement = 'right', gap = 6, @@ -51,9 +54,11 @@ export function useSideAnchoredPopoverPosition({ const updatePosition = useCallback(() => { const anchor = anchorRef.current; const popover = popoverRef.current; - if (!anchor || !popover || typeof window === 'undefined') return; + if ((!anchor && !anchorPoint) || !popover || typeof window === 'undefined') return; - const anchorBounds = anchor.getBoundingClientRect(); + const anchorBounds = anchorPoint + ? { left: anchorPoint.x, right: anchorPoint.x, top: anchorPoint.y, bottom: anchorPoint.y } + : anchor!.getBoundingClientRect(); const popoverBounds = popover.getBoundingClientRect(); // Opening animations can scale the painted bounds. Position against the // layout dimensions so the fully expanded menu still clears the viewport. @@ -86,7 +91,7 @@ export function useSideAnchoredPopoverPosition({ alignment, }; setLayout(current => sameLayout(current, nextLayout) ? current : nextLayout); - }, [anchorRef, gap, padding, popoverRef, preferredPlacement]); + }, [anchorPoint, anchorRef, gap, padding, popoverRef, preferredPlacement]); const schedulePositionUpdate = useCallback(() => { if (frameRef.current !== null) return;