fix: tui local shell consent UX (#1463)

- add local shell runner + denial notice + tests
- docs: describe ! local shell usage
- lint: drop unused Slack upload contentType
- cleanup: remove stray Swabble pins

Thanks @vignesh07.
Co-authored-by: Vignesh Natarajan <vigneshnatarajan92@gmail.com>

src/slack/send.ts

@@ -88,7 +88,7 @@ async function uploadSlackFile(params: {
   threadTs?: string;
   maxBytes?: number;
 }): Promise<string> {
-  const { buffer, contentType, fileName } = await loadWebMedia(params.mediaUrl, params.maxBytes);
+  const { buffer, fileName } = await loadWebMedia(params.mediaUrl, params.maxBytes);
   const basePayload = {
     channel_id: params.channelId,
     file: buffer,

src/tui/tui-input-history.test.ts

@@ -58,6 +58,24 @@ describe("createEditorSubmitHandler", () => {
     expect(editor.addToHistory).not.toHaveBeenCalled();
   });
 
+  it("does not add whitespace-only submissions to history", () => {
+    const editor = {
+      setText: vi.fn(),
+      addToHistory: vi.fn(),
+    };
+
+    const handler = createEditorSubmitHandler({
+      editor,
+      handleCommand: vi.fn(),
+      sendMessage: vi.fn(),
+      handleBangLine: vi.fn(),
+    });
+
+    handler("   ");
+
+    expect(editor.addToHistory).not.toHaveBeenCalled();
+  });
+
   it("routes slash commands to handleCommand", () => {
     const editor = {
       setText: vi.fn(),

src/tui/tui-local-shell.test.ts

@@ -0,0 +1,54 @@
+import { describe, expect, it, vi } from "vitest";
+
+import { createLocalShellRunner } from "./tui-local-shell.js";
+
+const createSelector = () => {
+  const selector = {
+    onSelect: undefined as ((item: { value: string; label: string }) => void) | undefined,
+    onCancel: undefined as (() => void) | undefined,
+    render: () => ["selector"],
+    invalidate: () => {},
+  };
+  return selector;
+};
+
+describe("createLocalShellRunner", () => {
+  it("logs denial on subsequent ! attempts without re-prompting", async () => {
+    const messages: string[] = [];
+    const chatLog = {
+      addSystem: (line: string) => {
+        messages.push(line);
+      },
+    };
+    const tui = { requestRender: vi.fn() };
+    const openOverlay = vi.fn();
+    const closeOverlay = vi.fn();
+    let lastSelector: ReturnType<typeof createSelector> | null = null;
+    const createSelectorSpy = vi.fn(() => {
+      lastSelector = createSelector();
+      return lastSelector;
+    });
+    const spawnCommand = vi.fn();
+
+    const { runLocalShellLine } = createLocalShellRunner({
+      chatLog,
+      tui,
+      openOverlay,
+      closeOverlay,
+      createSelector: createSelectorSpy,
+      spawnCommand,
+    });
+
+    const firstRun = runLocalShellLine("!ls");
+    expect(openOverlay).toHaveBeenCalledTimes(1);
+    lastSelector?.onSelect?.({ value: "no", label: "No" });
+    await firstRun;
+
+    await runLocalShellLine("!pwd");
+
+    expect(messages).toContain("local shell: not enabled");
+    expect(messages).toContain("local shell: not enabled for this session");
+    expect(createSelectorSpy).toHaveBeenCalledTimes(1);
+    expect(spawnCommand).not.toHaveBeenCalled();
+  });
+});

src/tui/tui-local-shell.ts

@@ -0,0 +1,137 @@
+import type { Component, SelectItem } from "@mariozechner/pi-tui";
+import { spawn } from "node:child_process";
+import { createSearchableSelectList } from "./components/selectors.js";
+
+type LocalShellDeps = {
+  chatLog: {
+    addSystem: (line: string) => void;
+  };
+  tui: {
+    requestRender: () => void;
+  };
+  openOverlay: (component: Component) => void;
+  closeOverlay: () => void;
+  createSelector?: (
+    items: SelectItem[],
+    maxVisible: number,
+  ) => Component & {
+    onSelect?: (item: SelectItem) => void;
+    onCancel?: () => void;
+  };
+  spawnCommand?: typeof spawn;
+  getCwd?: () => string;
+  env?: NodeJS.ProcessEnv;
+  maxOutputChars?: number;
+};
+
+export function createLocalShellRunner(deps: LocalShellDeps) {
+  let localExecAsked = false;
+  let localExecAllowed = false;
+  const createSelector = deps.createSelector ?? createSearchableSelectList;
+  const spawnCommand = deps.spawnCommand ?? spawn;
+  const getCwd = deps.getCwd ?? (() => process.cwd());
+  const env = deps.env ?? process.env;
+  const maxChars = deps.maxOutputChars ?? 40_000;
+
+  const ensureLocalExecAllowed = async (): Promise<boolean> => {
+    if (localExecAllowed) return true;
+    if (localExecAsked) return false;
+    localExecAsked = true;
+
+    return await new Promise<boolean>((resolve) => {
+      deps.chatLog.addSystem("Allow local shell commands for this session?");
+      deps.chatLog.addSystem(
+        "This runs commands on YOUR machine (not the gateway) and may delete files or reveal secrets.",
+      );
+      deps.chatLog.addSystem("Select Yes/No (arrows + Enter), Esc to cancel.");
+      const selector = createSelector(
+        [
+          { value: "no", label: "No" },
+          { value: "yes", label: "Yes" },
+        ],
+        2,
+      );
+      selector.onSelect = (item) => {
+        deps.closeOverlay();
+        if (item.value === "yes") {
+          localExecAllowed = true;
+          deps.chatLog.addSystem("local shell: enabled for this session");
+          resolve(true);
+        } else {
+          deps.chatLog.addSystem("local shell: not enabled");
+          resolve(false);
+        }
+        deps.tui.requestRender();
+      };
+      selector.onCancel = () => {
+        deps.closeOverlay();
+        deps.chatLog.addSystem("local shell: cancelled");
+        deps.tui.requestRender();
+        resolve(false);
+      };
+      deps.openOverlay(selector);
+      deps.tui.requestRender();
+    });
+  };
+
+  const runLocalShellLine = async (line: string) => {
+    const cmd = line.slice(1);
+    // NOTE: A lone '!' is handled by the submit handler as a normal message.
+    // Keep this guard anyway in case this is called directly.
+    if (cmd === "") return;
+
+    if (localExecAsked && !localExecAllowed) {
+      deps.chatLog.addSystem("local shell: not enabled for this session");
+      deps.tui.requestRender();
+      return;
+    }
+
+    const allowed = await ensureLocalExecAllowed();
+    if (!allowed) return;
+
+    deps.chatLog.addSystem(`[local] $ ${cmd}`);
+    deps.tui.requestRender();
+
+    await new Promise<void>((resolve) => {
+      const child = spawnCommand(cmd, {
+        shell: true,
+        cwd: getCwd(),
+        env,
+      });
+
+      let stdout = "";
+      let stderr = "";
+      child.stdout.on("data", (buf) => {
+        stdout += buf.toString("utf8");
+      });
+      child.stderr.on("data", (buf) => {
+        stderr += buf.toString("utf8");
+      });
+
+      child.on("close", (code, signal) => {
+        const combined = (stdout + (stderr ? (stdout ? "\n" : "") + stderr : ""))
+          .slice(0, maxChars)
+          .trimEnd();
+
+        if (combined) {
+          for (const line of combined.split("\n")) {
+            deps.chatLog.addSystem(`[local] ${line}`);
+          }
+        }
+        deps.chatLog.addSystem(
+          `[local] exit ${code ?? "?"}${signal ? ` (signal ${String(signal)})` : ""}`,
+        );
+        deps.tui.requestRender();
+        resolve();
+      });
+
+      child.on("error", (err) => {
+        deps.chatLog.addSystem(`[local] error: ${String(err)}`);
+        deps.tui.requestRender();
+        resolve();
+      });
+    });
+  };
+
+  return { runLocalShellLine };
+}

src/tui/tui.submit-handler.test.ts

@@ -50,6 +50,29 @@ describe("createEditorSubmitHandler", () => {
     expect(sendMessage).toHaveBeenCalledWith("!");
   });
 
+  it("does not treat leading whitespace before ! as a bang command", () => {
+    const editor = {
+      setText: vi.fn(),
+      addToHistory: vi.fn(),
+    };
+    const handleCommand = vi.fn();
+    const sendMessage = vi.fn();
+    const handleBangLine = vi.fn();
+
+    const onSubmit = createEditorSubmitHandler({
+      editor,
+      handleCommand,
+      sendMessage,
+      handleBangLine,
+    });
+
+    onSubmit("  !ls");
+
+    expect(handleBangLine).not.toHaveBeenCalled();
+    expect(sendMessage).toHaveBeenCalledWith("!ls");
+    expect(editor.addToHistory).toHaveBeenCalledWith("!ls");
+  });
+
   it("trims normal messages before sending and adding to history", () => {
     const editor = {
       setText: vi.fn(),

src/tui/tui.ts

@@ -6,7 +6,6 @@ import {
   Text,
   TUI,
 } from "@mariozechner/pi-tui";
-import { spawn } from "node:child_process";
 import { resolveDefaultAgentId } from "../agents/agent-scope.js";
 import { loadConfig } from "../config/config.js";
 import {
@@ -22,8 +21,8 @@ import { GatewayChatClient } from "./gateway-chat.js";
 import { editorTheme, theme } from "./theme/theme.js";
 import { createCommandHandlers } from "./tui-command-handlers.js";
 import { createEventHandlers } from "./tui-event-handlers.js";
-import { createSearchableSelectList } from "./components/selectors.js";
 import { formatTokens } from "./tui-formatters.js";
+import { createLocalShellRunner } from "./tui-local-shell.js";
 import { buildWaitingStatusMessage, defaultWaitingPhrases } from "./tui-waiting.js";
 import { createOverlayHandlers } from "./tui-overlays.js";
 import { createSessionActions } from "./tui-session-actions.js";
@@ -94,11 +93,6 @@ export async function runTui(opts: TuiOptions) {
   let toolsExpanded = false;
   let showThinking = false;
 
-  // Local "bash mode" (lines starting with '!') state.
-  // Permission is asked once per TUI session.
-  let localExecAsked = false;
-  let localExecAllowed = false;
-
   const deliverDefault = opts.deliver ?? false;
   const autoMessage = opts.message?.trim();
   let autoMessageSent = false;
@@ -518,102 +512,12 @@ export async function runTui(opts: TuiOptions) {
       formatSessionKey,
     });
 
-  const ensureLocalExecAllowed = async (): Promise<boolean> => {
-    if (localExecAllowed) return true;
-    if (localExecAsked) return false;
-    localExecAsked = true;
-
-    return await new Promise<boolean>((resolve) => {
-      chatLog.addSystem("Allow local shell commands for this session?");
-      chatLog.addSystem(
-        "This runs commands on YOUR machine (not the gateway) and may delete files or reveal secrets.",
-      );
-      chatLog.addSystem("Select Yes/No (arrows + Enter), Esc to cancel.");
-      const selector = createSearchableSelectList(
-        [
-          { value: "no", label: "No" },
-          { value: "yes", label: "Yes" },
-        ],
-        2,
-      );
-      selector.onSelect = (item) => {
-        closeOverlay();
-        if (item.value === "yes") {
-          localExecAllowed = true;
-          chatLog.addSystem("local shell: enabled for this session");
-          resolve(true);
-        } else {
-          chatLog.addSystem("local shell: not enabled");
-          resolve(false);
-        }
-        tui.requestRender();
-      };
-      selector.onCancel = () => {
-        closeOverlay();
-        chatLog.addSystem("local shell: cancelled");
-        tui.requestRender();
-        resolve(false);
-      };
-      openOverlay(selector);
-      tui.requestRender();
-    });
-  };
-
-  const runLocalShellLine = async (line: string) => {
-    // line starts with '!'
-    const cmd = line.slice(1);
-    // NOTE: A lone '!' is handled by the submit handler as a normal message.
-    // Keep this guard anyway in case this is called directly.
-    if (cmd === "") return;
-
-    const allowed = await ensureLocalExecAllowed();
-    if (!allowed) return;
-
-    chatLog.addSystem(`[local] $ ${cmd}`);
-    tui.requestRender();
-
-    await new Promise<void>((resolve) => {
-      const child = spawn(cmd, {
-        shell: true,
-        cwd: process.cwd(),
-        env: process.env,
-      });
-
-      let stdout = "";
-      let stderr = "";
-      child.stdout.on("data", (buf) => {
-        stdout += buf.toString("utf8");
-      });
-      child.stderr.on("data", (buf) => {
-        stderr += buf.toString("utf8");
-      });
-
-      child.on("close", (code, signal) => {
-        const maxChars = 40_000;
-        const combined = (stdout + (stderr ? (stdout ? "\n" : "") + stderr : ""))
-          .slice(0, maxChars)
-          .trimEnd();
-
-        if (combined) {
-          for (const line of combined.split("\n")) {
-            chatLog.addSystem(`[local] ${line}`);
-          }
-        }
-        chatLog.addSystem(
-          `[local] exit ${code ?? "?"}${signal ? ` (signal ${String(signal)})` : ""}`,
-        );
-        tui.requestRender();
-        resolve();
-      });
-
-      child.on("error", (err) => {
-        chatLog.addSystem(`[local] error: ${String(err)}`);
-        tui.requestRender();
-        resolve();
-      });
-    });
-  };
-
+  const { runLocalShellLine } = createLocalShellRunner({
+    chatLog,
+    tui,
+    openOverlay,
+    closeOverlay,
+  });
   updateAutocompleteProvider();
   editor.onSubmit = createEditorSubmitHandler({
     editor,