feat(web): terminal copy-on-select toggle and web-links polish

Wire copy-on-select in XtermSession (shared by the workspace terminal and the /machine terminal): a header checkbox toggles the behavior and persists to localStorage under spoon.terminal.copyOnSelect. On mount the stored value takes precedence over the copyOnSelect prop default. When enabled, onSelectionChange copies the current selection via navigator.clipboard.writeText — guarded (empty selections skipped; clipboard-unavailable/denied errors swallowed). The handler reads the toggle through a ref so live flips apply without reconnecting. WebLinksAddon remains loaded so printed URLs are clickable. Manual verification: in /machine and a workspace terminal, enabling copy-on-select and selecting text places it on the clipboard; a printed URL is clickable and opens in a new tab.
2026-07-11 13:17:46 -04:00
parent 148e060a9b
commit 12b1c98cfc
2 changed files with 248 additions and 20 deletions
@@ -13,6 +13,21 @@ import '@xterm/xterm/css/xterm.css';
 const TERMINAL_FONT =
  "var(--font-victor-mono), 'Symbols Nerd Font Mono', 'Geist Mono', ui-monospace, monospace";

+// Persisted user preference for copy-on-select. A stored value takes precedence
+// over the `copyOnSelect` prop default so the toggle sticks across sessions.
+const COPY_ON_SELECT_KEY = 'spoon.terminal.copyOnSelect';
+
+const readStoredCopyOnSelect = (): boolean | undefined => {
+  try {
+    const stored = window.localStorage.getItem(COPY_ON_SELECT_KEY);
+    if (stored === 'true') return true;
+    if (stored === 'false') return false;
+  } catch {
+    // localStorage may be unavailable (private mode, SSR); fall back to prop.
+  }
+  return undefined;
+};
+
 export type XtermStatus =
  | 'idle'
  | 'connecting'
@@ -95,6 +110,29 @@ export const XtermSession = ({
  const [status, setStatus] = useState<XtermStatus>('idle');
  const [errorText, setErrorText] = useState<string>();
  const [reconnectKey, setReconnectKey] = useState(0);
+  // localStorage overrides the prop default; the prop is only the initial value
+  // when nothing has been persisted yet.
+  const [copyEnabled, setCopyEnabled] = useState<boolean>(
+    () => readStoredCopyOnSelect() ?? copyOnSelect ?? false,
+  );
+  // The connect effect registers onSelectionChange once; it reads the current
+  // toggle through this ref so live flips take effect without a reconnect.
+  const copyEnabledRef = useRef(copyEnabled);
+  useEffect(() => {
+    copyEnabledRef.current = copyEnabled;
+  }, [copyEnabled]);
+
+  const toggleCopyOnSelect = () => {
+    setCopyEnabled((prev) => {
+      const next = !prev;
+      try {
+        window.localStorage.setItem(COPY_ON_SELECT_KEY, String(next));
+      } catch {
+        // Persistence is best-effort; ignore storage failures.
+      }
+      return next;
+    });
+  };

  // Update the live terminal's theme without tearing down the session.
  useEffect(() => {
@@ -164,13 +202,22 @@ export const XtermSession = ({
      termRef.current = term;
      fitRef.current = fit;

-      // Task 11 will wire clipboard copy-on-select behavior here. Accept the
-      // prop now so callers (and the effect deps) are stable across that change.
-      if (copyOnSelect) {
-        term.onSelectionChange(() => {
-          // no-op placeholder until Task 11
-        });
-      }
+      // Copy-on-select: when enabled, mirror the selection to the clipboard.
+      // Registered once; the ref lets live toggle flips apply without a
+      // reconnect. Clipboard access is guarded — it may be unavailable or
+      // denied, and empty selections are skipped.
+      term.onSelectionChange(() => {
+        if (!copyEnabledRef.current) return;
+        const selection = term.getSelection();
+        if (!selection) return;
+        try {
+          void navigator.clipboard.writeText(selection).catch(() => {
+            // clipboard write denied/unavailable; ignore
+          });
+        } catch {
+          // navigator.clipboard missing entirely; ignore
+        }
+      });

      const sendResize = () => {
        if (ws?.readyState !== WebSocket.OPEN) return;
@@ -285,16 +332,27 @@ export const XtermSession = ({
                    ? 'Terminal not configured'
                    : 'Connection error'}
        </p>
-        {status === 'closed' || status === 'error' ? (
-          <Button
-            type='button'
-            variant='outline'
-            size='sm'
-            onClick={() => setReconnectKey((key) => key + 1)}
-          >
-            Reconnect
-          </Button>
-        ) : null}
+        <div className='flex items-center gap-3'>
+          <label className='text-muted-foreground flex cursor-pointer items-center gap-1.5 text-xs select-none'>
+            <input
+              type='checkbox'
+              className='accent-primary size-3.5'
+              checked={copyEnabled}
+              onChange={toggleCopyOnSelect}
+            />
+            Copy on select
+          </label>
+          {status === 'closed' || status === 'error' ? (
+            <Button
+              type='button'
+              variant='outline'
+              size='sm'
+              onClick={() => setReconnectKey((key) => key + 1)}
+            >
+              Reconnect
+            </Button>
+          ) : null}
+        </div>
      </div>
      {status === 'unconfigured' ? (
        <div className='text-muted-foreground flex flex-1 items-center justify-center p-6 text-center text-sm'>
@@ -1,9 +1,17 @@
-import { render, screen, waitFor } from '@testing-library/react';
+import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

 import { XtermSession } from '../../src/components/agent-workspace/xterm-session';

-const { openSpy, wsInstances, fetchSpy, fitSpy } = vi.hoisted(() => ({
+const {
+  openSpy,
+  wsInstances,
+  fetchSpy,
+  fitSpy,
+  selectionHandlers,
+  getSelectionSpy,
+  writeTextSpy,
+} = vi.hoisted(() => ({
  openSpy: vi.fn(),
  wsInstances: [] as Array<{
    url: string;
@@ -12,6 +20,11 @@ const { openSpy, wsInstances, fetchSpy, fitSpy } = vi.hoisted(() => ({
  }>,
  fetchSpy: vi.fn(),
  fitSpy: vi.fn(),
+  selectionHandlers: [] as (() => void)[],
+  getSelectionSpy: vi.fn<() => string>(() => ''),
+  writeTextSpy: vi.fn<(text: string) => Promise<void>>(() =>
+    Promise.resolve(),
+  ),
 }));

 class FakeTerminal {
@@ -22,7 +35,10 @@ class FakeTerminal {
  loadAddon = vi.fn();
  onData = vi.fn();
  onResize = vi.fn();
-  onSelectionChange = vi.fn();
+  onSelectionChange = vi.fn((cb: () => void) => {
+    selectionHandlers.push(cb);
+  });
+  getSelection = getSelectionSpy;
  dispose = vi.fn();
  refresh = vi.fn();
  write = vi.fn();
@@ -64,8 +80,32 @@ class FakeWebSocket {

 beforeEach(() => {
  wsInstances.length = 0;
+  selectionHandlers.length = 0;
  openSpy.mockClear();
  fitSpy.mockClear();
+  getSelectionSpy.mockReset();
+  getSelectionSpy.mockReturnValue('');
+  writeTextSpy.mockReset();
+  writeTextSpy.mockResolvedValue(undefined);
+  const store = new Map<string, string>();
+  const localStorageMock = {
+    getItem: (key: string) => (store.has(key) ? store.get(key)! : null),
+    setItem: (key: string, value: string) => store.set(key, String(value)),
+    removeItem: (key: string) => store.delete(key),
+    clear: () => store.clear(),
+    key: (index: number) => Array.from(store.keys())[index] ?? null,
+    get length() {
+      return store.size;
+    },
+  };
+  Object.defineProperty(window, 'localStorage', {
+    configurable: true,
+    value: localStorageMock,
+  });
+  Object.defineProperty(navigator, 'clipboard', {
+    configurable: true,
+    value: { writeText: writeTextSpy },
+  });
  fetchSpy.mockReset();
  fetchSpy.mockResolvedValue({
    ok: true,
@@ -93,6 +133,7 @@ beforeEach(() => {
 });

 afterEach(() => {
+  cleanup();
  vi.unstubAllGlobals();
 });

@@ -193,4 +234,133 @@ describe('XtermSession', () => {
    expect(ws.close).not.toHaveBeenCalled();
    expect(wsInstances).toHaveLength(1);
  });
+
+  it('copies the selection to the clipboard when copy-on-select is enabled', async () => {
+    render(
+      <XtermSession
+        active
+        copyOnSelect
+        tokenUrl='/api/box/terminal-token'
+        sessionKey='box-1'
+      />,
+    );
+
+    await waitFor(() => expect(selectionHandlers.length).toBeGreaterThan(0));
+    getSelectionSpy.mockReturnValue('hello world');
+    selectionHandlers.forEach((cb) => cb());
+
+    await waitFor(() =>
+      expect(writeTextSpy).toHaveBeenCalledWith('hello world'),
+    );
+  });
+
+  it('does not copy when copy-on-select is disabled', async () => {
+    render(
+      <XtermSession active tokenUrl='/api/box/terminal-token' sessionKey='box-1' />,
+    );
+
+    await waitFor(() => expect(selectionHandlers.length).toBeGreaterThan(0));
+    getSelectionSpy.mockReturnValue('should not copy');
+    selectionHandlers.forEach((cb) => cb());
+    await Promise.resolve();
+
+    expect(writeTextSpy).not.toHaveBeenCalled();
+  });
+
+  it('skips empty selections even when enabled', async () => {
+    render(
+      <XtermSession
+        active
+        copyOnSelect
+        tokenUrl='/api/box/terminal-token'
+        sessionKey='box-1'
+      />,
+    );
+
+    await waitFor(() => expect(selectionHandlers.length).toBeGreaterThan(0));
+    getSelectionSpy.mockReturnValue('');
+    selectionHandlers.forEach((cb) => cb());
+    await Promise.resolve();
+
+    expect(writeTextSpy).not.toHaveBeenCalled();
+  });
+
+  it('swallows clipboard errors without crashing', async () => {
+    writeTextSpy.mockRejectedValue(new Error('denied'));
+    render(
+      <XtermSession
+        active
+        copyOnSelect
+        tokenUrl='/api/box/terminal-token'
+        sessionKey='box-1'
+      />,
+    );
+
+    await waitFor(() => expect(selectionHandlers.length).toBeGreaterThan(0));
+    getSelectionSpy.mockReturnValue('boom');
+    selectionHandlers.forEach((cb) => cb());
+
+    await waitFor(() => expect(writeTextSpy).toHaveBeenCalledWith('boom'));
+    // Rejection is caught; the component stays mounted.
+    expect(screen.getByRole('checkbox')).toBeInTheDocument();
+  });
+
+  it('renders the header toggle and persists flips to localStorage', async () => {
+    render(
+      <XtermSession active tokenUrl='/api/box/terminal-token' sessionKey='box-1' />,
+    );
+
+    await waitFor(() => expect(selectionHandlers.length).toBeGreaterThan(0));
+    const checkbox = screen.getByRole('checkbox');
+    expect(checkbox).not.toBeChecked();
+
+    // Off: a selection is not copied.
+    getSelectionSpy.mockReturnValue('nope');
+    selectionHandlers.forEach((cb) => cb());
+    await Promise.resolve();
+    expect(writeTextSpy).not.toHaveBeenCalled();
+
+    // Flip on: persists and now copies.
+    fireEvent.click(checkbox);
+    expect(checkbox).toBeChecked();
+    expect(
+      window.localStorage.getItem('spoon.terminal.copyOnSelect'),
+    ).toBe('true');
+
+    getSelectionSpy.mockReturnValue('now copied');
+    selectionHandlers.forEach((cb) => cb());
+    await waitFor(() =>
+      expect(writeTextSpy).toHaveBeenCalledWith('now copied'),
+    );
+
+    // Flip off again: persists false.
+    fireEvent.click(checkbox);
+    expect(checkbox).not.toBeChecked();
+    expect(
+      window.localStorage.getItem('spoon.terminal.copyOnSelect'),
+    ).toBe('false');
+  });
+
+  it('lets the persisted localStorage value override the prop default', () => {
+    window.localStorage.setItem('spoon.terminal.copyOnSelect', 'true');
+    render(
+      <XtermSession active tokenUrl='/api/box/terminal-token' sessionKey='box-1' />,
+    );
+
+    const checkbox = screen.getByRole('checkbox');
+    expect(checkbox).toBeChecked();
+
+    window.localStorage.setItem('spoon.terminal.copyOnSelect', 'false');
+    render(
+      <XtermSession
+        active
+        copyOnSelect
+        tokenUrl='/api/box/terminal-token'
+        sessionKey='box-2'
+      />,
+    );
+    // The second instance's checkbox reads the persisted false over prop=true.
+    const checkboxes = screen.getAllByRole('checkbox');
+    expect(checkboxes[checkboxes.length - 1]).not.toBeChecked();
+  });
 });