Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/app/block/block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { getWaveObjectAtom, makeORef, useWaveObjectValue } from "@/store/wos";
import { focusedBlockId, getElemAsStr } from "@/util/focusutil";
import { isBlank, useAtomValueSafe } from "@/util/util";
import { HelpViewModel } from "@/view/helpview/helpview";
import { TermViewModel } from "@/view/term/term";
import { TermViewModel } from "@/view/term/term-model";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Fix TermViewModel import path (build blocker)

Other modules import from "@/app/view/term/term-model"; this file uses "@/view/term/term-model". Likely breaks module resolution.

- import { TermViewModel } from "@/view/term/term-model";
+ import { TermViewModel } from "@/app/view/term/term-model";

To verify consistency:


🏁 Script executed:

#!/bin/bash
# Find mismatched imports for term-model
rg -nP '"@/view/term/term-model"|"/app/view/term/term-model"' -g '!**/dist/**' -S

Length of output: 87



🏁 Script executed:

#!/bin/bash
# Find the actual term-model file location
fd -e ts -e tsx 'term-model' -t f

Length of output: 36


🏁 Script executed:

#!/bin/bash
# Search for all TermViewModel imports across the codebase
rg -nP 'import\s+.*TermViewModel' -g '!**/dist/**'

Length of output: 387


🏁 Script executed:

#!/bin/bash
# Check if the "@/view" path exists at all
fd 'view' -t d -g '!**/node_modules/**' -g '!**/dist/**' | head -20

Length of output: 164


Fix TermViewModel import path (build blocker)

The import uses an incorrect path that doesn't match the file location or other modules in the codebase. The actual file is at frontend/app/view/term/term-model.ts, which corresponds to the alias @/app/view/term/term-model. Three other files (term-wsh.tsx, termtheme.ts, and term.tsx) correctly import from this path.

- import { TermViewModel } from "@/view/term/term-model";
+ import { TermViewModel } from "@/app/view/term/term-model";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { TermViewModel } from "@/view/term/term-model";
import { TermViewModel } from "@/app/view/term/term-model";
🤖 Prompt for AI Agents
In frontend/app/block/block.tsx around line 30 the import path for TermViewModel
is incorrect; update the import to use the correct module alias path that
matches the actual file location (frontend/app/view/term/term-model) — replace
the current "@/view/term/term-model" with "@/app/view/term/term-model" to match
the other files (term-wsh.tsx, termtheme.ts, term.tsx) and ensure builds resolve
the module.

import { WaveAiModel } from "@/view/waveai/waveai";
import { WebViewModel } from "@/view/webview/webview";
import clsx from "clsx";
Expand Down
8 changes: 6 additions & 2 deletions frontend/app/element/iconbutton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useLongClick } from "@/app/hook/useLongClick";
import { makeIconClass } from "@/util/util";
import clsx from "clsx";
import { atom, useAtom } from "jotai";
import { forwardRef, memo, useMemo, useRef } from "react";
import { CSSProperties, forwardRef, memo, useMemo, useRef } from "react";
import "./iconbutton.scss";

type IconButtonProps = { decl: IconButtonDecl; className?: string };
Expand All @@ -15,6 +15,10 @@ export const IconButton = memo(
const spin = decl.iconSpin ?? false;
useLongClick(ref, decl.click, decl.longClick, decl.disabled);
const disabled = decl.disabled ?? false;
const styleVal: CSSProperties = {};
if (decl.iconColor) {
styleVal.color = decl.iconColor;
}
Comment on lines +18 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Inconsistency: ToggleIconButton still uses the old styling pattern.

The conditional color styling here differs from ToggleIconButton (line 60), which still uses style={{ color: decl.iconColor ?? "inherit" }}. This means when decl.iconColor is undefined:

  • IconButton: no inline color style (relies on CSS cascade)
  • ToggleIconButton: explicit color: inherit

While functionally similar in most cases, this inconsistency could lead to subtle styling differences if CSS specificity or inheritance rules interact differently with the two patterns.

Consider applying the same conditional styling pattern to ToggleIconButton for consistency:

+        const styleVal: CSSProperties = {};
+        if (decl.iconColor) {
+            styleVal.color = decl.iconColor;
+        }
         return (
             <button
                 ref={ref}
                 className={clsx("wave-iconbutton", "toggle", className, decl.className, {
                     active,
                     disabled,
                     "no-action": decl.noAction,
                 })}
                 title={title}
                 aria-label={title}
-                style={{ color: decl.iconColor ?? "inherit" }}
+                style={styleVal}
                 onClick={() => setActive(!active)}
                 disabled={disabled}
             >

Also verify that removing the explicit color: inherit default doesn't break existing icon button styling across the application, particularly in contexts where CSS specificity might affect color inheritance.

Also applies to: 31-31


🏁 Script executed:

#!/bin/bash
# Examine the IconButton and ToggleIconButton components
cat frontend/app/element/iconbutton.tsx

Length of output: 2677


Verified: ToggleIconButton inconsistency is real and worth addressing.

The review comment accurately identifies an inconsistency. When decl.iconColor is undefined:

  • IconButton (lines 18–20, 31): renders with empty style={}, relying on CSS cascade
  • ToggleIconButton (line 59): renders with explicit style={{ color: "inherit" }}

While functionally equivalent in most cases, the inconsistent patterns within the same file warrant consistency. Applying the same conditional styleVal pattern to ToggleIconButton is a reasonable refactoring suggestion to improve maintainability.

🤖 Prompt for AI Agents
In frontend/app/element/iconbutton.tsx around lines 18–21 and line 59,
ToggleIconButton currently passes an explicit style={{ color: "inherit" }} while
IconButton builds a conditional styleVal object when decl.iconColor is present;
update ToggleIconButton to use the same conditional pattern as IconButton by
creating a styleVal: CSSProperties = {} and only setting styleVal.color =
decl.iconColor when decl.iconColor is defined, then pass style={styleVal} to the
rendered element so both components use the same conditional styling approach.

return (
<button
ref={ref}
Expand All @@ -24,7 +28,7 @@ export const IconButton = memo(
})}
title={decl.title}
aria-label={decl.title}
style={{ color: decl.iconColor ?? "inherit" }}
style={styleVal}
disabled={disabled}
>
{typeof decl.icon === "string" ? <i className={makeIconClass(decl.icon, true, { spin })} /> : decl.icon}
Expand Down
117 changes: 117 additions & 0 deletions frontend/app/view/term/shellblocking.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright 2025, Command Line Inc.
// SPDX-License-Identifier: Apache-2.0

// Always block (TUIs / pagers / multiplexers / known interactive UIs)
const ALWAYS_BLOCK = [
// multiplexers
"tmux", "screen", "byobu", "dtach", "abduco", "tmate",
// editors/pagers
"vim", "nvim", "emacs", "nano", "less", "more", "man", "most", "view",
// TUIs / tools
"htop", "top", "btop", "fzf", "ranger", "mc", "nnn", "k9s", "nmtui", "alsamixer",
"tig", "gdb", "lldb",
// mail/irc
"mutt", "neomutt", "alpine", "weechat", "irssi",
// dialog UIs
"dialog", "whiptail",
// DB shells
"psql", "mysql", "sqlite3", "mongo", "redis-cli",
];

// Bare REPLs only block when no args
const BARE_REPLS = [
"python", "python3", "python2", "node", "ruby", "perl", "php", "lua", "ipython", "bpython", "irb",
];

// Shells: block only if interactive/new shell
const SHELLS = [
"bash", "sh", "zsh", "fish", "ksh", "mksh", "dash", "ash", "tcsh", "csh",
"xonsh", "elvish", "nu", "nushell", "pwsh", "powershell", "cmd",
];

// Wrappers to skip
const WRAPPERS = [
"sudo", "doas", "pkexec", "rlwrap", "env", "time", "nice", "nohup",
"chrt", "stdbuf", "script", "scriptreplay", "sshpass",
];

function looksInteractiveShellArgs(args: string[]): boolean {
return (
args.length === 0 ||
args.includes("-i") ||
args.includes("--login") ||
args.includes("-l") ||
args.includes("-s")
);
}

function isNonInteractiveShellExec(args: string[]): boolean {
return (
args.includes("-c") ||
args.some((a) => a === "-Command" || a.startsWith("-Command")) ||
args.some((a) => a.endsWith(".sh") || a.includes("/"))
);
}

function isAttachLike(cmd: string, args: string[]): boolean {
if (cmd === "docker" || cmd === "podman") {
if (args[0] === "attach") return true;
if (args[0] === "exec") return args.some((a) => a === "-it" || a === "-i" || a === "-t");
}
if (cmd === "kubectl" || cmd === "k3s" || cmd === "oc") {
if (args[0] === "attach") return true;
if (args[0] === "exec") return args.some((a) => a === "-it" || a === "-i" || a === "-t");
}
if (cmd === "lxc" && args[0] === "exec") return args.some((a) => a === "-t" || a === "-T");
return false;
}

function isSshInteractive(args: string[]): boolean {
const hasForcedTty = args.includes("-t") || args.includes("-tt");
const hasRemoteCmd = args.some((a) => !a.startsWith("-") && a.includes(" "));
return hasForcedTty || !hasRemoteCmd;
}
Comment on lines +69 to +73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

SSH remote-command detection misclassifies non-interactive invocations.

ssh host cmd (and ssh host cmd arg...) is treated as interactive; isSshInteractive only looks for a space in a single arg. Parse the host position and treat additional args as a remote command.

-function isSshInteractive(args: string[]): boolean {
-    const hasForcedTty = args.includes("-t") || args.includes("-tt");
-    const hasRemoteCmd = args.some((a) => !a.startsWith("-") && a.includes(" "));
-    return hasForcedTty || !hasRemoteCmd;
-}
+function isSshInteractive(args: string[]): boolean {
+    const hasForcedTty = args.includes("-t") || args.includes("-tt");
+    // find first non-option token (host)
+    const hostIdx = args.findIndex((a) => !a.startsWith("-"));
+    if (hostIdx === -1) {
+        // e.g., `ssh -V` → non-interactive unless TTY forced
+        return hasForcedTty;
+    }
+    // any tokens after host imply a remote command
+    const hasRemoteCmd = args.length > hostIdx + 1;
+    return hasForcedTty || !hasRemoteCmd;
+}
🤖 Prompt for AI Agents
In frontend/app/view/term/shellblocking.ts around lines 69 to 73,
isSshInteractive currently treats any argument containing a space as a remote
command and thus misclassifies invocations like `ssh host cmd` as interactive.
Change the logic to: scan args left-to-right skipping option tokens (those
starting with "-" and their possible option values until you hit a non-option
token) to identify the host token position, then treat the ssh invocation as
non-interactive if there are any tokens after that host (i.e., remote command
present); still return interactive if -t or -tt is present (forced TTY). Ensure
the function handles a "--" separator (stop option parsing) and common SSH short
options so that host detection is correct.


export function getBlockingCommand(lastCommand: string | null, inAltBuffer: boolean): string | null {
if (!lastCommand) return null;

let words = lastCommand.trim().split(/\s+/);
if (words.length === 0) return null;

while (words.length && WRAPPERS.includes(words[0])) {
words.shift();
}
if (!words.length) return null;

const first = words[0].split("/").pop()!;
const args = words.slice(1);

if (inAltBuffer) return first;

if (ALWAYS_BLOCK.includes(first)) return first;

if (isAttachLike(first, args)) return first;

if (first === "ssh" || first === "mosh" || first === "telnet" || first === "rlogin") {
if (isSshInteractive(args)) return first;
return null;
}

if (first === "su" || first === "machinectl" || first === "chroot" || first === "nsenter" || first === "lxc") {
if (!args.length || SHELLS.includes(args[args.length - 1]?.split("/").pop() || "")) return first;
return null;
}

if (SHELLS.includes(first)) {
if (looksInteractiveShellArgs(args)) return first;
if (isNonInteractiveShellExec(args)) return null;
return null;
}

if (BARE_REPLS.includes(first)) {
if (args.length === 0) return first;
return null;
}

return null;
}
Loading
Loading