Merge pull request 'Fix terminal input reordering and Linux terminal rendering' (#46) from fix/terminal-input-ordering-and-linux-rendering into main

app/src-tauri/src/main.rs

@@ -26,12 +26,27 @@
 /// their own init time, which happens inside the Tauri builder that
 /// function calls into, not at binary load.
 ///
-/// A user who has already set this themselves is left alone. That includes
+/// A user who has already set this themselves is left alone — with one
-/// setting it to `0`, on the assumption WebKitGTK treats it as a boolean
+/// correction. The earlier version of this function left *any* pre-set value
-/// rather than presence-only — not verified against WebKitGTK's own source,
+/// alone, including `0`, on the assumption WebKitGTK reads the variable as a
-/// so if it turns out to be presence-only, `=0` still reads as "set" here
+/// boolean. WebKitGTK reads it as presence-only, so `WEBKIT_DISABLE_DMABUF_
-/// and disables DMA-BUF the same as any other value, which is at least the
+/// RENDERER=0` disabled DMA-BUF exactly like `=1` did, and there was no value
-/// safe direction to be wrong in.
+/// at all a user could set to get the accelerated path back: the escape hatch
+/// the comment described did not exist. `0`, `false` and empty are now treated
+/// as an explicit opt-out and the variable is *removed*, which is the only
+/// thing WebKitGTK reads as "enabled". The default is unchanged — unset still
+/// means disabled on Linux, so nobody who was not deliberately overriding this
+/// sees any difference.
+///
+/// That matters more than it looks, because the trade described above is not
+/// the trade actually being made. `@xterm/addon-webgl` does not fall back to
+/// the canvas renderer here: its constructor throws only when WebGL is
+/// *absent*, and with DMA-BUF disabled WebGL is still present — served by
+/// software rasterisation. So the addon loads happily and every terminal frame
+/// is rendered on the CPU and copied, which is slower than the canvas renderer
+/// this comment assumed it would degrade to, not faster. See
+/// `terminal_gpu_rendering` in `AppSettings` for the switch that decides
+/// whether the addon is loaded at all.
 ///
 /// This env var also leaks to whatever the app spawns afterwards — notably
 /// a cold-launched default browser via the `opener` plugin's `xdg-open`
@@ -39,10 +54,77 @@
 /// URL; most non-WebKitGTK browsers ignore the variable entirely), but
 /// worth knowing before chasing the "links don't open" half of triple-c#34
 /// as a separate, unrelated cause.
+#[cfg(target_os = "linux")]
+const DMABUF_VAR: &str = "WEBKIT_DISABLE_DMABUF_RENDERER";
+
+/// What to do with `WEBKIT_DISABLE_DMABUF_RENDERER`, given whatever it is
+/// already set to. Split from the mutation so it can be tested without
+/// touching process-wide environment state from a parallel test runner.
+#[cfg(target_os = "linux")]
+#[derive(Debug, PartialEq, Eq)]
+enum DmabufAction {
+    /// Not set by the user — apply the workaround.
+    Disable,
+    /// Explicitly opted out. WebKitGTK reads presence, not value, so the only
+    /// way to express "enabled" is for the variable not to exist.
+    Remove,
+    /// Set to something meaning "disabled". Already what we want; leave it.
+    LeaveAlone,
+}
+
+#[cfg(target_os = "linux")]
+fn dmabuf_action(current: Option<&str>) -> DmabufAction {
+    match current {
+        None => DmabufAction::Disable,
+        Some(value) => match value.trim().to_ascii_lowercase().as_str() {
+            "" | "0" | "false" | "no" => DmabufAction::Remove,
+            _ => DmabufAction::LeaveAlone,
+        },
+    }
+}
+
 #[cfg(target_os = "linux")]
 fn apply_webkit_wayland_workaround() {
-    if std::env::var_os("WEBKIT_DISABLE_DMABUF_RENDERER").is_none() {
+    let current = std::env::var(DMABUF_VAR).ok();
-        std::env::set_var("WEBKIT_DISABLE_DMABUF_RENDERER", "1");
+    match dmabuf_action(current.as_deref()) {
+        DmabufAction::Disable => std::env::set_var(DMABUF_VAR, "1"),
+        DmabufAction::Remove => std::env::remove_var(DMABUF_VAR),
+        DmabufAction::LeaveAlone => {}
+    }
+}
+
+#[cfg(all(test, target_os = "linux"))]
+mod tests {
+    use super::{dmabuf_action, DmabufAction};
+
+    #[test]
+    fn unset_gets_the_workaround() {
+        assert_eq!(dmabuf_action(None), DmabufAction::Disable);
+    }
+
+    #[test]
+    fn falsey_values_opt_out_by_removing_the_variable() {
+        // The bug this replaces: these all previously read as "user set it,
+        // leave it alone", and WebKitGTK then disabled DMA-BUF anyway because
+        // it only checks presence. There was no way to ask for the GPU path.
+        for value in ["0", "false", "no", "", "  0  ", "FALSE", "No"] {
+            assert_eq!(
+                dmabuf_action(Some(value)),
+                DmabufAction::Remove,
+                "{value:?} should opt out"
+            );
+        }
+    }
+
+    #[test]
+    fn other_values_are_left_alone() {
+        for value in ["1", "true", "yes", "anything"] {
+            assert_eq!(
+                dmabuf_action(Some(value)),
+                DmabufAction::LeaveAlone,
+                "{value:?} should be left alone"
+            );
+        }
     }
 }
 

app/src-tauri/src/models/app_settings.rs

@@ -135,6 +135,26 @@ pub struct AppSettings {
     pub gateway: GatewaySettings,
     #[serde(default)]
     pub global_claude_code_settings: Option<ClaudeCodeSettings>,
+    /// Whether the terminal loads `@xterm/addon-webgl`.
+    ///
+    /// `None` is "auto", and auto is not the same answer on every platform.
+    /// On Linux the app disables WebKitGTK's DMA-BUF renderer at startup (see
+    /// `apply_webkit_wayland_workaround` in `main.rs`, and triple-c#34), which
+    /// does not remove WebGL — it leaves it backed by software rasterisation.
+    /// The addon therefore loads successfully and then renders every frame on
+    /// the CPU, which is slower than the canvas renderer it would otherwise
+    /// have fallen back to. So auto means enabled on macOS and Windows, and
+    /// disabled on Linux.
+    ///
+    /// `Some(true)` / `Some(false)` force it either way on any platform. A
+    /// Linux user running X11, or one whose driver stack is unaffected, can
+    /// turn it back on; anyone seeing terminal lag can turn it off without
+    /// waiting for a release. Deliberately `Option<bool>` rather than `bool`:
+    /// the zero value has to mean "we choose", not "off", or every existing
+    /// settings file would silently pin the answer at whatever the default was
+    /// the day it was written.
+    #[serde(default)]
+    pub terminal_gpu_rendering: Option<bool>,
 }
 
 fn default_stt_model() -> String {
@@ -226,6 +246,7 @@ impl Default for AppSettings {
             stt: SttSettings::default(),
             gateway: GatewaySettings::default(),
             global_claude_code_settings: None,
+            terminal_gpu_rendering: None,
         }
     }
 }

app/src/components/settings/SettingsPanel.tsx

@@ -15,6 +15,8 @@ import type { EnvVar } from "../../lib/types";
 import Tooltip from "../ui/Tooltip";
 import AccordionSection from "../ui/AccordionSection";
 import Toggle from "../ui/Toggle";
+import SegmentedControl from "../ui/SegmentedControl";
+import { resolveTerminalGpuRendering } from "../../lib/terminalRenderer";
 import WebTerminalSettings from "./WebTerminalSettings";
 import SttSettings from "./SttSettings";
 import SharedAuthSettings from "./SharedAuthSettings";
@@ -67,6 +69,14 @@ export default function SettingsPanel() {
     }
   };
 
+  const handleGpuRenderingChange = async (value: "auto" | "on" | "off") => {
+    if (!appSettings) return;
+    await saveSettings({
+      ...appSettings,
+      terminal_gpu_rendering: value === "auto" ? null : value === "on",
+    });
+  };
+
   const handleAutoCheckToggle = async () => {
     if (!appSettings) return;
     await saveSettings({ ...appSettings, auto_check_updates: !appSettings.auto_check_updates });
@@ -242,6 +252,45 @@ export default function SettingsPanel() {
         <SttSettings />
       </AccordionSection>
 
+      <AccordionSection id="terminal" title="Terminal" defaultOpen={false}>
+        <div className="space-y-2">
+          <label className="text-xs text-[var(--text-secondary)]">GPU rendering</label>
+          <SegmentedControl
+            label="Terminal GPU rendering"
+            value={
+              appSettings?.terminal_gpu_rendering == null
+                ? "auto"
+                : appSettings.terminal_gpu_rendering
+                  ? "on"
+                  : "off"
+            }
+            onChange={handleGpuRenderingChange}
+            segments={[
+              {
+                value: "auto",
+                label: "Auto",
+                hint: resolveTerminalGpuRendering(null, navigator.userAgent)
+                  ? "On for this platform."
+                  : "Off on Linux — the DMA-BUF workaround leaves WebGL on software rendering, which is slower than the canvas renderer.",
+              },
+              {
+                value: "on",
+                label: "On",
+                hint: "Always load the WebGL renderer.",
+              },
+              {
+                value: "off",
+                label: "Off",
+                hint: "Always use xterm's canvas renderer. Try this if typing feels laggy.",
+              },
+            ]}
+          />
+          <p className="text-xs text-[var(--text-secondary)]">
+            Takes effect when a terminal tab is next switched to.
+          </p>
+        </div>
+      </AccordionSection>
+
       <AccordionSection id="updates" title="Updates" defaultOpen={false}>
         <div className="space-y-2">
           {appVersion && (

app/src/components/terminal/TerminalView.tsx

@@ -28,6 +28,7 @@ import UrlToast, {
   URL_TOAST_SHORTCUT,
 } from "./UrlToast";
 import { trimSelection } from "./trimSelection";
+import { resolveTerminalGpuRendering } from "../../lib/terminalRenderer";
 import TerminalContextMenu from "./TerminalContextMenu";
 
 interface Props {
@@ -95,6 +96,7 @@ export default function TerminalView({ sessionId, active }: Props) {
   const webglRef = useRef<WebglAddon | null>(null);
   const detectorRef = useRef<UrlDetector | null>(null);
   const { sendInput, pasteImage, resize, onOutput, onExit } = useTerminal();
+  const gpuRenderingSetting = useAppState(s => s.appSettings?.terminal_gpu_rendering ?? null);
   const setTerminalHasSelection = useAppState(s => s.setTerminalHasSelection);
   const setTerminalAtBottom = useAppState(s => s.setTerminalAtBottom);
   const setScrollActiveToBottom = useAppState(s => s.setScrollActiveToBottom);
@@ -491,7 +493,11 @@ export default function TerminalView({ sessionId, active }: Props) {
 
     // Handle user input -> backend
     const inputDisposable = term.onData((data) => {
-      sendInput(sessionId, data);
+      // Ordered and coalesced by the queue in `useTerminal`; a rejection here
+      // means the session is gone, which the exit listener already reports.
+      sendInput(sessionId, data).catch((e) =>
+        console.error("Failed to send terminal input:", e)
+      );
     });
 
     // Detect user-initiated scroll-up (mouse wheel) to pause auto-follow.
@@ -684,7 +690,16 @@ export default function TerminalView({ sessionId, active }: Props) {
     const term = termRef.current;
     if (!term) return;
 
-    if (active) {
+    // Auto on macOS/Windows, off on Linux, overridable either way — see
+    // `resolveTerminalGpuRendering`. Loading the addon under a software-GL
+    // WebKitGTK is slower than xterm's canvas renderer, not faster.
+    const useGpu = resolveTerminalGpuRendering(gpuRenderingSetting, navigator.userAgent);
+
+    // The renderer and the activation work are independent: a terminal with
+    // GPU rendering switched off still has to fit and take focus when its tab
+    // becomes active. Keeping these in one branch made "GPU off" silently mean
+    // "never re-fit, never focus".
+    if (active && useGpu) {
       // Attach WebGL renderer
       if (!webglRef.current) {
         try {
@@ -699,19 +714,21 @@ export default function TerminalView({ sessionId, active }: Props) {
           // WebGL not available, canvas renderer is fine
         }
       }
+    } else if (webglRef.current) {
+      // Release the context — for inactive terminals, and when the setting
+      // turns GPU rendering off while this terminal is on screen.
+      try { webglRef.current.dispose(); } catch { /* ignore */ }
+      webglRef.current = null;
+    }
+
+    if (active) {
       fitRef.current?.fit();
       if (autoFollowRef.current) {
         term.scrollToBottom();
       }
       term.focus();
-    } else {
-      // Release WebGL context for inactive terminals
-      if (webglRef.current) {
-        try { webglRef.current.dispose(); } catch { /* ignore */ }
-        webglRef.current = null;
     }
-    }
+  }, [active, gpuRenderingSetting]);
-  }, [active]);
 
   // Auto-dismiss toast after 30 seconds — unless the user is standing in it.
   // A keyboard user who has just jumped into the toast is mid-decision, and

app/src/hooks/useTerminal.test.ts

@@ -0,0 +1,83 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+// The queue lives at module scope in useTerminal, so the command layer is
+// mocked and the hook's `sendInput` is exercised through `renderHook`.
+const terminalInput = vi.fn<(sessionId: string, data: number[]) => Promise<void>>();
+
+vi.mock("../lib/tauri-commands", () => ({
+  terminalInput: (sessionId: string, data: number[]) => terminalInput(sessionId, data),
+  openTerminalSession: vi.fn(),
+  closeTerminalSession: vi.fn(),
+  terminalResize: vi.fn(),
+  pasteImageToTerminal: vi.fn(),
+  updateProject: vi.fn(),
+}));
+
+vi.mock("@tauri-apps/api/event", () => ({ listen: vi.fn() }));
+
+import { renderHook } from "@testing-library/react";
+import { useTerminal } from "./useTerminal";
+
+const decode = (bytes: number[]) => new TextDecoder().decode(new Uint8Array(bytes));
+
+describe("useTerminal input ordering", () => {
+  beforeEach(() => {
+    terminalInput.mockReset();
+  });
+
+  it("preserves order even when the underlying invokes resolve out of order", async () => {
+    // Make the *first* call the slowest, which is exactly the race that put a
+    // backspace behind the characters typed after it.
+    const resolvers: Array<() => void> = [];
+    terminalInput.mockImplementation(
+      () => new Promise<void>((resolve) => resolvers.push(resolve)),
+    );
+
+    const { result } = renderHook(() => useTerminal());
+
+    const first = result.current.sendInput("s1", "\x7f"); // backspace
+    const rest = ["a", "b", "c"].map((ch) => result.current.sendInput("s1", ch));
+
+    // Only one write may be in flight at a time.
+    expect(terminalInput).toHaveBeenCalledTimes(1);
+    expect(decode(terminalInput.mock.calls[0][1])).toBe("\x7f");
+
+    resolvers.shift()!();
+    await first;
+
+    // The three queued keystrokes coalesce into one ordered write.
+    expect(terminalInput).toHaveBeenCalledTimes(2);
+    expect(decode(terminalInput.mock.calls[1][1])).toBe("abc");
+
+    resolvers.shift()!();
+    await Promise.all(rest);
+
+    const sent = terminalInput.mock.calls.map((c) => decode(c[1])).join("");
+    expect(sent).toBe("\x7fabc");
+  });
+
+  it("settles each caller's promise and does not drop later writes on failure", async () => {
+    terminalInput.mockRejectedValueOnce(new Error("boom")).mockResolvedValue(undefined);
+
+    const { result } = renderHook(() => useTerminal());
+
+    await expect(result.current.sendInput("s2", "x")).rejects.toThrow("boom");
+    await expect(result.current.sendInput("s2", "y")).resolves.toBeUndefined();
+
+    expect(decode(terminalInput.mock.calls[1][1])).toBe("y");
+  });
+
+  it("keeps separate sessions independent", async () => {
+    terminalInput.mockResolvedValue(undefined);
+    const { result } = renderHook(() => useTerminal());
+
+    await Promise.all([
+      result.current.sendInput("a", "1"),
+      result.current.sendInput("b", "2"),
+    ]);
+
+    const bySession = terminalInput.mock.calls.map((c) => [c[0], decode(c[1])]);
+    expect(bySession).toContainEqual(["a", "1"]);
+    expect(bySession).toContainEqual(["b", "2"]);
+  });
+});

app/src/hooks/useTerminal.ts

@@ -4,6 +4,86 @@ import { listen } from "@tauri-apps/api/event";
 import { useAppState } from "../store/appState";
 import * as commands from "../lib/tauri-commands";
 
+/**
+ * Per-session ordered write queue.
+ *
+ * Every keystroke used to be its own `invoke("terminal_input")`, and because
+ * that command is `async` on the Rust side Tauri spawns each one as an
+ * independent task. Those tasks then race for the session mutex in
+ * `ExecSessionManager::send_input`, so nothing preserved the order the bytes
+ * were typed in — the visible symptom was a backspace landing *after* the
+ * characters typed behind it. The serial writer task downstream cannot help,
+ * because the order is already lost by the time anything reaches the channel.
+ *
+ * The queue restores ordering the same way the web terminal gets it for free:
+ * one write in flight at a time, the next only after the previous resolves.
+ * Anything typed while a write is in flight coalesces into the next chunk,
+ * which also collapses a burst of typing into a couple of IPC round trips
+ * rather than one per key. Concatenating the byte arrays is safe — a PTY
+ * cannot tell one write of "ab" from writes of "a" then "b" — and each
+ * caller's promise still settles only when its own bytes have gone, so
+ * `await sendInput(...)` keeps the meaning it had.
+ *
+ * Module scope, not hook scope, because `useTerminal()` is called from several
+ * components (App for speech-to-text, TerminalView for typing and image paste,
+ * useProjectActions for tile commands). A per-hook queue would give each caller
+ * its own ordering and leave them racing against each other.
+ */
+type PendingWrite = {
+  bytes: number[];
+  resolve: () => void;
+  reject: (reason: unknown) => void;
+};
+
+const inputQueues = new Map<string, { pending: PendingWrite[]; draining: boolean }>();
+
+async function drainInputQueue(sessionId: string): Promise<void> {
+  const q = inputQueues.get(sessionId);
+  if (!q || q.draining) return;
+
+  q.draining = true;
+  try {
+    while (q.pending.length > 0) {
+      // Take everything queued so far as one batch, preserving order.
+      const batch = q.pending.splice(0, q.pending.length);
+      const bytes = batch.flatMap((w) => w.bytes);
+      try {
+        await commands.terminalInput(sessionId, bytes);
+        batch.forEach((w) => w.resolve());
+      } catch (err) {
+        // Reject only the writes in this batch. Anything queued while it was
+        // in flight is still pending and gets its own attempt on the next lap.
+        batch.forEach((w) => w.reject(err));
+      }
+    }
+  } finally {
+    q.draining = false;
+    // Drop the entry once idle so closed sessions do not accumulate.
+    if (q.pending.length === 0) inputQueues.delete(sessionId);
+  }
+}
+
+function enqueueInput(sessionId: string, bytes: number[]): Promise<void> {
+  return new Promise<void>((resolve, reject) => {
+    let q = inputQueues.get(sessionId);
+    if (!q) {
+      q = { pending: [], draining: false };
+      inputQueues.set(sessionId, q);
+    }
+    q.pending.push({ bytes, resolve, reject });
+    void drainInputQueue(sessionId);
+  });
+}
+
+/** Drop any queued input for a session that is going away. */
+function discardInputQueue(sessionId: string): void {
+  const q = inputQueues.get(sessionId);
+  if (!q) return;
+  const dropped = q.pending.splice(0, q.pending.length);
+  dropped.forEach((w) => w.reject(new Error(`Session ${sessionId} closed`)));
+  if (!q.draining) inputQueues.delete(sessionId);
+}
+
 export function useTerminal() {
   const { sessions, activeSessionId, addSession, removeSession, setActiveSession } =
     useAppState(
@@ -33,6 +113,7 @@ export function useTerminal() {
       const session = currentSessions.find((s) => s.id === sessionId);
       const project = session ? projects.find((p) => p.id === session.projectId) : undefined;
 
+      discardInputQueue(sessionId);
       await commands.closeTerminalSession(sessionId);
       removeSession(sessionId);
 
@@ -54,7 +135,7 @@ export function useTerminal() {
   const sendInput = useCallback(
     async (sessionId: string, data: string) => {
       const bytes = Array.from(new TextEncoder().encode(data));
-      await commands.terminalInput(sessionId, bytes);
+      await enqueueInput(sessionId, bytes);
     },
     [],
   );

app/src/lib/terminalRenderer.test.ts

@@ -0,0 +1,39 @@
+import { describe, it, expect } from "vitest";
+import { isLinuxWebview, resolveTerminalGpuRendering } from "./terminalRenderer";
+
+const LINUX = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/605.1.15 Safari/605.1.15";
+const MAC = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 Safari/605.1.15";
+const WINDOWS = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 Chrome/120 Safari/537.36";
+const ANDROID = "Mozilla/5.0 (Linux; Android 14) AppleWebKit/537.36 Chrome/120 Mobile Safari/537.36";
+
+describe("isLinuxWebview", () => {
+  it("recognises desktop Linux", () => {
+    expect(isLinuxWebview(LINUX)).toBe(true);
+  });
+
+  it("does not count Android as desktop Linux", () => {
+    expect(isLinuxWebview(ANDROID)).toBe(false);
+  });
+
+  it("rejects the other desktop platforms", () => {
+    expect(isLinuxWebview(MAC)).toBe(false);
+    expect(isLinuxWebview(WINDOWS)).toBe(false);
+  });
+});
+
+describe("resolveTerminalGpuRendering", () => {
+  it("auto is off on Linux, where WebGL falls back to software rendering", () => {
+    expect(resolveTerminalGpuRendering(null, LINUX)).toBe(false);
+    expect(resolveTerminalGpuRendering(undefined, LINUX)).toBe(false);
+  });
+
+  it("auto is on elsewhere", () => {
+    expect(resolveTerminalGpuRendering(null, MAC)).toBe(true);
+    expect(resolveTerminalGpuRendering(null, WINDOWS)).toBe(true);
+  });
+
+  it("an explicit setting wins on every platform", () => {
+    expect(resolveTerminalGpuRendering(true, LINUX)).toBe(true);
+    expect(resolveTerminalGpuRendering(false, MAC)).toBe(false);
+  });
+});

app/src/lib/terminalRenderer.ts

@@ -0,0 +1,28 @@
+/**
+ * Decides whether the terminal loads `@xterm/addon-webgl`.
+ *
+ * Split out of `TerminalView` so it can be unit-tested without standing up a
+ * terminal, and so the platform rule lives in exactly one place.
+ */
+
+/** True when the webview is running on Linux (WebKitGTK), excluding Android. */
+export function isLinuxWebview(userAgent: string): boolean {
+  return /\bLinux\b/.test(userAgent) && !/\bAndroid\b/.test(userAgent);
+}
+
+/**
+ * Resolve the effective WebGL setting.
+ *
+ * `setting` is `AppSettings.terminal_gpu_rendering`: `true`/`false` force the
+ * answer, `null`/`undefined` mean auto. Auto is on everywhere except Linux —
+ * there the app disables WebKitGTK's DMA-BUF renderer at startup (triple-c#34),
+ * which leaves WebGL present but software-rasterised, so loading the addon is
+ * slower than the canvas renderer it would otherwise have fallen back to.
+ */
+export function resolveTerminalGpuRendering(
+  setting: boolean | null | undefined,
+  userAgent: string,
+): boolean {
+  if (typeof setting === "boolean") return setting;
+  return !isLinuxWebview(userAgent);
+}

app/src/lib/types.ts

@@ -290,6 +290,12 @@ export interface AppSettings {
   stt: SttSettings;
   gateway: GatewaySettings;
   global_claude_code_settings: ClaudeCodeSettings | null;
+  /** Whether the terminal loads the WebGL renderer. `null` is auto: on
+   *  everywhere except Linux, where the DMA-BUF workaround leaves WebGL
+   *  backed by software rasterisation and the addon ends up slower than the
+   *  canvas renderer it would otherwise fall back to. See
+   *  `resolveTerminalGpuRendering` in `lib/terminalRenderer.ts`. */
+  terminal_gpu_rendering: boolean | null;
 }
 
 /** What `preview_settings_import` returns before anything is applied —