From cb879af4159f06fa08088975bceb0050edbe6325 Mon Sep 17 00:00:00 2001
From: Bob Lee <liwenbo628@gmail.com>
Date: Tue, 27 Oct 2026 10:47:25 +0800
Subject: [PATCH] fix(web): expand control preview and dismiss ended sessions

---
 .../ComputerUseControlCard.test.tsx           | 149 ++++++++++++++++--
 .../app/components/ComputerUseControlCard.tsx |  30 ++--
 2 files changed, 148 insertions(+), 31 deletions(-)

diff --git a/src/web-ui/src/app/components/ComputerUseControlCard.test.tsx b/src/web-ui/src/app/components/ComputerUseControlCard.test.tsx
index 5b6e803142..9c80924511 100644
--- a/src/web-ui/src/app/components/ComputerUseControlCard.test.tsx
+++ b/src/web-ui/src/app/components/ComputerUseControlCard.test.tsx
@@ -3,10 +3,11 @@ import React, { act } from 'react';
 import { createRoot, type Root } from 'react-dom/client';
 import { afterEach, beforeEach, expect, it, vi } from 'vitest';
 import { ComputerUseControlCard } from './ComputerUseControlCard';
-import type { ComputerUseControlSnapshot } from '@/infrastructure/api/service-api/ComputerUseControlAPI';
+import type { ComputerUseControlPreview, ComputerUseControlSnapshot } from '@/infrastructure/api/service-api/ComputerUseControlAPI';
 
 const mocks = vi.hoisted(() => ({ status: vi.fn(), stop: vi.fn(), preview: vi.fn(),
-  scope: { epoch: 1, isCurrent: () => true }, listeners: new Set<() => void>() }));
+  observeShare: vi.fn(), permissions: vi.fn(), targets: vi.fn(),
+  scope: { epoch: 1, isCurrent: () => true }, listeners: new Set<() => void>(), t: (key: string) => key }));
 vi.mock('@/infrastructure/api/service-api/ComputerUseControlAPI', async original => ({
   ...await original<object>(), computerUseControlAPI: mocks,
 }));
@@ -14,7 +15,7 @@ vi.mock('@/infrastructure/peer-device/deviceSurface', () => ({
   getActiveSurfaceScope: () => mocks.scope,
   onSurfaceActivated: (fn: () => void) => { mocks.listeners.add(fn); return () => mocks.listeners.delete(fn); },
 }));
-vi.mock('@/infrastructure/i18n', () => ({ useI18n: () => ({ t: (key: string) => key }) }));
+vi.mock('@/infrastructure/i18n', () => ({ useI18n: () => ({ t: mocks.t }) }));
 vi.mock('@openbitfun/ui', () => ({
   Button: ({ children, ...props }: React.ButtonHTMLAttributes<HTMLButtonElement>) => <button {...props}>{children}</button>,
   Card: ({ children }: React.PropsWithChildren) => <div>{children}</div>,
@@ -23,6 +24,7 @@ vi.mock('@openbitfun/ui', () => ({
   CardFooter: ({ children }: React.PropsWithChildren) => <div>{children}</div>,
   OverlayLayer: ({ children }: React.PropsWithChildren) => <div>{children}</div>,
   OverflowText: ({ children }: React.PropsWithChildren) => <span>{children}</span>,
+  Select: () => <select />,
 }));
 vi.mock('@/infrastructure/api/service-api/ApiClient', () => ({ api: {} }));
 globalThis.IS_REACT_ACT_ENVIRONMENT = true;
@@ -32,8 +34,15 @@ const active = (target: string, generation = 4): ComputerUseControlSnapshot => (
   supported: true, generation, owner: 'session', mode: 'background', state: 'active', target,
   action: null, sequence: 1, reason: null, pointer: null, capabilities: [],
 });
+const frame = (target = 'Window A', generation = 4): ComputerUseControlPreview => ({
+  generation, target, image_base64: 'AA==', mime_type: 'image/png', width: 100, height: 100,
+  origin_x: 0, origin_y: 0, span_width: 100, span_height: 100,
+});
+const button = (key: string) => [...container.querySelectorAll('button')].find(b => b.textContent === key)!;
 beforeEach(() => {
   vi.useFakeTimers(); mocks.status.mockReset(); mocks.stop.mockReset(); mocks.preview.mockReset();
+  mocks.observeShare.mockReset(); mocks.permissions.mockReset().mockResolvedValue({});
+  mocks.targets.mockReset().mockResolvedValue([]);
   mocks.scope = { epoch: 1, isCurrent: () => mocks.scope.epoch === 1 };
   vi.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockReturnValue(300);
   vi.spyOn(HTMLElement.prototype, 'clientHeight', 'get').mockReturnValue(200);
@@ -46,14 +55,14 @@ afterEach(async () => {
   vi.useRealTimers(); vi.restoreAllMocks(); vi.unstubAllGlobals();
 });
 
-it('stops the generation the user saw, then removes the stop action', async () => {
+it('stops the generation the user saw, then removes the card', async () => {
   mocks.status.mockResolvedValue(active('Window A'));
   mocks.stop.mockResolvedValue({ ...active('Window A', 5), state: 'stopped' });
   await act(async () => root.render(<ComputerUseControlCard />));
   const stop = [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.stop')!;
   await act(async () => stop.click());
   expect(mocks.stop).toHaveBeenCalledWith(4);
-  expect(container.textContent).toContain('computerControl.stopped');
+  expect(container.textContent).not.toContain('computerControl.title');
   expect([...container.querySelectorAll('button')].some(b => b.textContent === 'computerControl.stop')).toBe(false);
 });
 
@@ -83,7 +92,7 @@ it('does not restore active control from a read started before Stop completed',
   const stop = [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.stop')!;
   await act(async () => stop.click());
   await act(async () => resolveRead(active('Window A')));
-  expect(container.textContent).toContain('computerControl.stopped');
+  expect(container.textContent).not.toContain('computerControl.title');
   expect([...container.querySelectorAll('button')].some(b => b.textContent === 'computerControl.stop')).toBe(false);
 });
 
@@ -95,9 +104,6 @@ it('renders a retained click even when the instantaneous pointer is already up',
   mocks.preview.mockResolvedValue({ generation: 4, target: 'Window A', image_base64: 'AA==', mime_type: 'image/png',
     width: 100, height: 100, origin_x: 0, origin_y: 0, span_width: 100, span_height: 100 });
   await act(async () => root.render(<ComputerUseControlCard />));
-  const preview = [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.showPreview')!;
-  await act(async () => preview.click());
-  await act(async () => vi.advanceTimersByTime(1000));
   const ring = container.querySelector<HTMLElement>('[data-openbitfun-part="click"]');
   expect(ring?.style.left).toBe('25%');
   expect(ring?.style.top).toBe('50%');
@@ -113,8 +119,6 @@ it('does not show a preview from a different target in the same session', async
   mocks.preview.mockResolvedValue({ generation: 4, target: 'Window B', image_base64: 'AA==', mime_type: 'image/png',
     width: 100, height: 100, origin_x: 0, origin_y: 0, span_width: 100, span_height: 100 });
   await act(async () => root.render(<ComputerUseControlCard />));
-  const preview = [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.showPreview')!;
-  await act(async () => preview.click());
   await act(async () => vi.advanceTimersByTime(1000));
   expect(container.querySelector('img')).toBeNull();
   expect(container.textContent).toContain('computerControl.previewUnavailable');
@@ -129,7 +133,6 @@ it('keeps the session arrow after click feedback expires and clears it on stop',
     width: 100, height: 100, origin_x: 0, origin_y: 0, span_width: 100, span_height: 100 });
   mocks.stop.mockResolvedValue({ ...active('Window A', 5), state: 'stopped' });
   await act(async () => root.render(<ComputerUseControlCard />));
-  await act(async () => [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.showPreview')!.click());
   await act(async () => vi.advanceTimersByTime(1000));
   await act(async () => vi.advanceTimersByTime(2000));
   const arrow = container.querySelector<HTMLElement>('[data-openbitfun-part="pointer"]');
@@ -146,7 +149,6 @@ it('removes the old target pointer while waiting for the new target preview', as
   mocks.preview.mockResolvedValue({ generation: 4, target: 'Window A', image_base64: 'AA==', mime_type: 'image/png',
     width: 100, height: 100, origin_x: 0, origin_y: 0, span_width: 100, span_height: 100 });
   await act(async () => root.render(<ComputerUseControlCard />));
-  await act(async () => [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.showPreview')!.click());
   await act(async () => vi.advanceTimersByTime(1000));
   expect(container.querySelector('[data-openbitfun-part="pointer"]')).not.toBeNull();
   mocks.status.mockResolvedValue(active('Window B'));
@@ -162,7 +164,6 @@ it('discards a preview from an earlier visit to the same target', async () => {
     image_base64: 'AA==', mime_type: 'image/png', width: 100, height: 100,
     origin_x: 0, origin_y: 0, span_width: 100, span_height: 100 });
   await act(async () => root.render(<ComputerUseControlCard />));
-  await act(async () => [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.showPreview')!.click());
   await act(async () => vi.advanceTimersByTime(1000));
   expect(container.querySelector('img')).toBeNull();
   expect(container.textContent).toContain('computerControl.previewUnavailable');
@@ -182,7 +183,6 @@ it.each(['absent', 'rejected', 'wrong-target', 'stale-revision', 'invalid-image'
     });
     mocks.stop.mockResolvedValue({ ...active('Window A', 5), state: 'stopped' });
     await act(async () => root.render(<ComputerUseControlCard />));
-    await act(async () => [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.showPreview')!.click());
     await act(async () => vi.advanceTimersByTime(1000));
     expect(container.querySelector('img')).toBeNull();
     expect(container.textContent).toContain('computerControl.previewUnavailable');
@@ -190,7 +190,7 @@ it.each(['absent', 'rejected', 'wrong-target', 'stale-revision', 'invalid-image'
     expect(container.textContent).not.toContain('computerControl.unavailable');
     await act(async () => [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.stop')!.click());
     expect(mocks.stop).toHaveBeenCalledWith(4);
-    expect(container.textContent).toContain('computerControl.stopped');
+    expect(container.textContent).not.toContain('computerControl.title');
   },
 );
 
@@ -203,3 +203,120 @@ it('still reports a failed status read as unavailable and permits Stop', async (
   await act(async () => [...container.querySelectorAll('button')].find(b => b.textContent === 'computerControl.stop')!.click());
   expect(mocks.stop).toHaveBeenCalledWith(4);
 });
+it('shows the preview by default and hides the card when control ends externally', async () => {
+  mocks.status.mockResolvedValue(active('Window A'));
+  mocks.preview.mockResolvedValue({ generation: 4, target: 'Window A', image_base64: 'AA==', mime_type: 'image/png',
+    width: 100, height: 100 });
+  await act(async () => root.render(<ComputerUseControlCard />));
+  expect(container.querySelector('img')).not.toBeNull();
+  expect(container.textContent).toContain('computerControl.hidePreview');
+  mocks.status.mockResolvedValue({ ...active('Window A'), state: 'stopped', reason: 'task_cancelled' });
+  await act(async () => vi.advanceTimersByTime(1000));
+  expect(container.textContent).not.toContain('computerControl.title');
+  expect(container.textContent).not.toContain('task_cancelled');
+});
+
+it.each(['idle', 'stopped'])('does not show an existing %s session on mount', async state => {
+  mocks.status.mockResolvedValue({ ...active('Window A'), state });
+  await act(async () => root.render(<ComputerUseControlCard />));
+  expect(container.querySelector('.computer-control-card')).toBeNull();
+  expect(mocks.preview).not.toHaveBeenCalled();
+});
+
+it('keeps manual collapse for the current control and fetches a new generation immediately', async () => {
+  mocks.status.mockResolvedValue(active('Window A'));
+  mocks.preview.mockResolvedValue(frame());
+  await act(async () => root.render(<ComputerUseControlCard />));
+  await act(async () => button('computerControl.hidePreview').click());
+  await act(async () => vi.advanceTimersByTime(1000));
+  expect(container.querySelector('img')).toBeNull();
+  expect(mocks.preview).toHaveBeenCalledTimes(1);
+  await act(async () => button('computerControl.showPreview').click());
+  expect(container.querySelector('img')).not.toBeNull();
+  await act(async () => button('computerControl.hidePreview').click());
+
+  mocks.status.mockResolvedValue(active('Window B', 6));
+  mocks.preview.mockResolvedValue(frame('Window B', 6));
+  await act(async () => vi.advanceTimersByTime(1000));
+  expect(mocks.preview).toHaveBeenLastCalledWith(6);
+  expect(container.querySelector('img')).not.toBeNull();
+  expect(button('computerControl.hidePreview').getAttribute('aria-expanded')).toBe('true');
+});
+
+it('defaults to expanded on another device even when its generation matches the collapsed session', async () => {
+  mocks.status.mockResolvedValue(active('Local A'));
+  mocks.preview.mockResolvedValue(frame('Local A'));
+  await act(async () => root.render(<ComputerUseControlCard />));
+  await act(async () => button('computerControl.hidePreview').click());
+  mocks.status.mockResolvedValue(active('Peer B'));
+  mocks.preview.mockResolvedValue(frame('Peer B'));
+  await act(async () => {
+    mocks.scope = { epoch: 2, isCurrent: () => mocks.scope.epoch === 2 };
+    mocks.listeners.forEach(fn => fn());
+  });
+  expect(container.textContent).toContain('Peer B');
+  expect(container.textContent).not.toContain('Local A');
+  expect(container.querySelector('img')).not.toBeNull();
+  expect(mocks.preview).toHaveBeenCalledTimes(2);
+});
+
+it('discards a pending preview from the previous device', async () => {
+  let resolveOld!: (value: ComputerUseControlPreview) => void;
+  mocks.status.mockResolvedValue(active('Same target'));
+  mocks.preview.mockReturnValueOnce(new Promise(resolve => { resolveOld = resolve; }))
+    .mockResolvedValue({ ...frame('Same target'), image_base64: 'bmV3' });
+  await act(async () => root.render(<ComputerUseControlCard />));
+  await act(async () => {
+    mocks.scope = { epoch: 2, isCurrent: () => mocks.scope.epoch === 2 };
+    mocks.listeners.forEach(fn => fn());
+  });
+  await act(async () => resolveOld(frame('Same target')));
+  expect(container.querySelector('img')?.getAttribute('src')).toBe('data:image/png;base64,bmV3');
+});
+
+it('does not restore a stopped card when its pending preview arrives', async () => {
+  let resolvePreview!: (value: ComputerUseControlPreview) => void;
+  mocks.status.mockResolvedValue(active('Window A'));
+  mocks.preview.mockReturnValue(new Promise(resolve => { resolvePreview = resolve; }));
+  mocks.stop.mockResolvedValue({ ...active('Window A', 5), state: 'stopped' });
+  await act(async () => root.render(<ComputerUseControlCard />));
+  await act(async () => button('computerControl.stop').click());
+  await act(async () => resolvePreview(frame()));
+  expect(container.querySelector('.computer-control-card')).toBeNull();
+});
+
+it('retains the card and Stop action when stopping fails', async () => {
+  mocks.status.mockResolvedValue(active('Window A'));
+  mocks.preview.mockResolvedValue(frame());
+  mocks.stop.mockRejectedValue(new Error('Host unavailable'));
+  await act(async () => root.render(<ComputerUseControlCard />));
+  await act(async () => button('computerControl.stop').click());
+  expect(container.textContent).toContain('computerControl.unavailable');
+  expect(button('computerControl.stop').disabled).toBe(false);
+});
+
+it.each(['starting', 'awaiting_share'])('keeps the %s card available for Stop', async state => {
+  mocks.status.mockResolvedValue({ ...active('Window A'), state });
+  await act(async () => root.render(<ComputerUseControlCard />));
+  expect(button('computerControl.stop')).toBeDefined();
+  expect(mocks.preview).not.toHaveBeenCalled();
+});
+
+it('opens an existing shared preview by default and stops observing after the session ends', async () => {
+  const shared: ComputerUseControlSnapshot = { ...active('Window A'), target_revision: 1, mode: 'observe',
+    sharing: { phase: 'shared', requested_mode: 'observe', target_label: 'Window A', control_enabled: false, authorization_revision: 0 } };
+  mocks.status.mockResolvedValue(shared);
+  mocks.observeShare.mockResolvedValue({ control: shared, tree: null, frame: {
+    ...frame(), screenshot_id: 'frame', content: { left: 0, top: 0, width: 100, height: 100 }, bounds: null,
+  } });
+  await act(async () => root.render(<ComputerUseControlCard />));
+  expect(container.querySelector('img')).not.toBeNull();
+  expect(mocks.observeShare).toHaveBeenCalledExactlyOnceWith(shared);
+  expect(mocks.preview).not.toHaveBeenCalled();
+  mocks.status.mockResolvedValue({ ...shared, generation: 5, state: 'stopped' });
+  await act(async () => vi.advanceTimersByTime(1000));
+  expect(container.querySelector('.computer-control-card')).toBeNull();
+  const calls = mocks.observeShare.mock.calls.length;
+  await act(async () => vi.advanceTimersByTime(1000));
+  expect(mocks.observeShare).toHaveBeenCalledTimes(calls);
+});
diff --git a/src/web-ui/src/app/components/ComputerUseControlCard.tsx b/src/web-ui/src/app/components/ComputerUseControlCard.tsx
index ad86d007e3..c78c18c685 100644
--- a/src/web-ui/src/app/components/ComputerUseControlCard.tsx
+++ b/src/web-ui/src/app/components/ComputerUseControlCard.tsx
@@ -30,22 +30,22 @@ export function ComputerUseControlCard() {
   const { t } = useI18n('common');
   const scope = useSyncExternalStore(onSurfaceActivated, getActiveSurfaceScope, getActiveSurfaceScope);
   const [view, setView] = useState<View>();
-  const [expanded, setExpanded] = useState(false);
-  const expandedRef = useRef(false);
-  expandedRef.current = expanded;
+  const [collapsed, setCollapsed] = useState<string>();
+  const collapsedRef = useRef(collapsed);
+  collapsedRef.current = collapsed;
   const mutation = useRef(0);
   const [stopping, setStopping] = useState(false);
   const [clickFeedback, setClickFeedback] = useState<{ x: number; y: number } | null>(null);
-  const [dismissed, setDismissed] = useState<string>();
   const current = view?.epoch === scope.epoch ? view : undefined;
   const snapshot = current?.snapshot;
+  const identity = `${scope.epoch}:${snapshot?.generation}`;
+  const expanded = collapsed !== identity;
   const active = snapshot?.state === 'active' || snapshot?.state === 'starting' || snapshot?.state === 'awaiting_share';
 
   useEffect(() => {
     let disposed = false;
     let timer: ReturnType<typeof setTimeout> | undefined;
     setView(undefined);
-    setExpanded(false);
     setStopping(false);
     const poll = async () => {
       const revision = mutation.current;
@@ -57,7 +57,9 @@ export function ComputerUseControlCard() {
             && previous.snapshot.target === next.target
             && (previous.snapshot.target_revision ?? 0) === (next.target_revision ?? 0) ? { frame: previous.frame, framePointer: previous.framePointer } : {}),
         }));
-        if (expandedRef.current && next.supported && next.state === 'active' && !next.sharing) {
+        // A collapse applies only to this device and control generation. New
+        // control must request its first frame in the same poll as its status.
+        if (collapsedRef.current !== `${scope.epoch}:${next.generation}` && next.supported && next.state === 'active' && !next.sharing) {
           try {
             const frame = await computerUseControlAPI.preview(next.generation);
             if (disposed || !scope.isCurrent() || revision !== mutation.current) return;
@@ -126,8 +128,7 @@ export function ComputerUseControlCard() {
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [clickIdentity, expanded]);
 
-  const identity = `${scope.epoch}:${snapshot?.generation}`;
-  if (!snapshot?.supported || snapshot.state === 'idle' || !snapshot.owner || dismissed === identity) return null;
+  if (!snapshot?.supported || !active || !snapshot.owner) return null;
   const pointer = current?.frame ? projectControlPointer(current.frame, current.framePointer ?? null) : null;
   const mode = snapshot.mode === 'observe' ? t('computerControl.observe')
     : snapshot.mode === 'foreground' ? t('computerControl.foreground') : t('computerControl.background');
@@ -139,18 +140,17 @@ export function ComputerUseControlCard() {
       <CardHeader>{t('computerControl.title')}</CardHeader>
       <CardBody className="computer-control-card__body">
         <p role="status" className="computer-control-card__status" data-openbitfun-component="computer-control" data-openbitfun-part="status">
-          {current?.error ? t('computerControl.unavailable') : snapshot.state === 'awaiting_share' ? t('computerSharing.selectApp') : active ? mode : t('computerControl.stopped')}
+          {current?.error ? t('computerControl.unavailable') : snapshot.state === 'awaiting_share' ? t('computerSharing.selectApp') : mode}
         </p>
         {snapshot.target && <OverflowText>{snapshot.sharing?.target_label || snapshot.target}</OverflowText>}
-        {active && snapshot.sharing && <ComputerUseSharingControls snapshot={snapshot}
+        {snapshot.sharing && <ComputerUseSharingControls snapshot={snapshot}
           onMutation={() => { mutation.current += 1; }}
           onSnapshot={next => {
             if (!scope.isCurrent()) return;
             mutation.current += 1;
             setView(previous => !acceptsUpdate(previous?.snapshot, next) ? previous : { epoch: scope.epoch, snapshot: next });
-            if (next.sharing?.phase === 'shared') setExpanded(true);
+            if (next.sharing?.phase === 'shared') setCollapsed(undefined);
           }} />}
-        {snapshot.reason && !active && <p role="status">{snapshot.reason}</p>}
         {expanded && snapshot.sharing?.phase === 'shared' && <SharedPreview key={`${scope.epoch}:${snapshot.generation}:${snapshot.target_revision}`}
           snapshot={snapshot} onSnapshot={next => {
             if (!scope.isCurrent()) return;
@@ -170,12 +170,12 @@ export function ComputerUseControlCard() {
       </CardBody>
       <CardFooter>
         {snapshot.state === 'active' && <Button size="sm" variant="outline" aria-expanded={expanded}
-          onClick={() => setExpanded(value => !value)}>
+          onClick={() => setCollapsed(expanded ? identity : undefined)}>
           {expanded ? t('computerControl.hidePreview') : t('computerControl.showPreview')}
         </Button>}
-        {active ? <Button size="sm" variant="primary" disabled={stopping} onClick={() => void stop()}>
+        <Button size="sm" variant="primary" disabled={stopping} onClick={() => void stop()}>
           {stopping ? t('computerControl.stopping') : t('computerControl.stop')}
-        </Button> : <Button size="sm" variant="outline" onClick={() => setDismissed(identity)}>{t('actions.close')}</Button>}
+        </Button>
       </CardFooter>
     </Card>
     </div>