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
6 changes: 6 additions & 0 deletions Sources/TerminalController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2688,6 +2688,9 @@ class TerminalController {
#endif
v2MainSync {
let ws = tabManager.addWorkspace(workingDirectory: cwd, select: shouldFocus)
if !shouldFocus, let terminalPanel = ws.focusedTerminalPanel {
terminalPanel.surface.requestBackgroundSurfaceStartIfNeeded()
}
newId = ws.id
}
#if DEBUG
Expand Down Expand Up @@ -10297,6 +10300,9 @@ class TerminalController {
#endif
DispatchQueue.main.sync {
let workspace = tabManager.addTab(select: focus)
if !focus, let terminalPanel = workspace.focusedTerminalPanel {
terminalPanel.surface.requestBackgroundSurfaceStartIfNeeded()
}
newTabId = workspace.id
}
#if DEBUG
Expand Down
191 changes: 191 additions & 0 deletions tests_v2/test_cli_new_workspace_background_metadata.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
#!/usr/bin/env python3
"""Regression: CLI `new-workspace --cwd` should preload sidebar metadata without focus."""

from __future__ import annotations

import glob
import os
import shutil
import subprocess
import sys
import tempfile
import time
from pathlib import Path

sys.path.insert(0, str(Path(__file__).parent))
from cmux import cmux, cmuxError


SOCKET_PATH = os.environ.get("CMUX_SOCKET", "/tmp/cmux-debug.sock")


def _must(cond: bool, msg: str) -> None:
if not cond:
raise cmuxError(msg)


def _find_cli_binary() -> str:
env_cli = os.environ.get("CMUXTERM_CLI")
if env_cli and os.path.isfile(env_cli) and os.access(env_cli, os.X_OK):
return env_cli

fixed = os.path.expanduser("~/Library/Developer/Xcode/DerivedData/cmux-tests-v2/Build/Products/Debug/cmux")
if os.path.isfile(fixed) and os.access(fixed, os.X_OK):
return fixed

candidates = glob.glob(os.path.expanduser("~/Library/Developer/Xcode/DerivedData/**/Build/Products/Debug/cmux"), recursive=True)
candidates += glob.glob("/tmp/cmux-*/Build/Products/Debug/cmux")
candidates = [p for p in candidates if os.path.isfile(p) and os.access(p, os.X_OK)]
if not candidates:
raise cmuxError("Could not locate cmux CLI binary; set CMUXTERM_CLI")
candidates.sort(key=lambda p: os.path.getmtime(p), reverse=True)
return candidates[0]
Comment on lines +36 to +42

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make CLI binary discovery deterministic.

At Line 36-Line 42, picking the “most recently modified” cmux from broad globs can select an unrelated build and make this regression test validate the wrong binary.

Proposed hardening
-    candidates = [p for p in candidates if os.path.isfile(p) and os.access(p, os.X_OK)]
-    if not candidates:
-        raise cmuxError("Could not locate cmux CLI binary; set CMUXTERM_CLI")
-    candidates.sort(key=lambda p: os.path.getmtime(p), reverse=True)
-    return candidates[0]
+    candidates = [p for p in candidates if os.path.isfile(p) and os.access(p, os.X_OK)]
+    if not candidates:
+        raise cmuxError("Could not locate cmux CLI binary; set CMUXTERM_CLI")
+    if len(candidates) == 1:
+        return candidates[0]
+    raise cmuxError(
+        "Multiple cmux CLI binaries found; set CMUXTERM_CLI to the intended binary"
+    )
🧰 Tools
🪛 Ruff (0.15.2)

[error] 37-37: Probable insecure usage of temporary file or directory: "/tmp/cmux-*/Build/Products/Debug/cmux"

(S108)


[warning] 40-40: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests_v2/test_cli_new_workspace_background_metadata.py` around lines 36 - 42,
The test currently picks the newest file from a broad glob (using the candidates
list) which can return unrelated builds; make discovery deterministic by first
honoring an explicit CMUXTERM_CLI environment variable (validate it is a file
and executable and return it), and only if unset, gather glob candidates, filter
them to files/executables as before, then validate each candidate by invoking
the CLI (e.g., subprocess.run([candidate, "--version"] or a known probe) and
accept only those that respond with the expected version/identifier, and finally
deterministically pick the candidate by a stable key (e.g., sorted pathname)
instead of os.path.getmtime; raise cmuxError if none pass validation.



def _run_cli(cli: str, args: list[str]) -> str:
env = dict(os.environ)
env.pop("CMUX_WORKSPACE_ID", None)
env.pop("CMUX_SURFACE_ID", None)
env.pop("CMUX_TAB_ID", None)

cmd = [cli, "--socket", SOCKET_PATH] + args
proc = subprocess.run(cmd, capture_output=True, text=True, check=False, env=env)
if proc.returncode != 0:
merged = f"{proc.stdout}\n{proc.stderr}".strip()
raise cmuxError(f"CLI failed ({' '.join(cmd)}): {merged}")
return (proc.stdout or "").strip()


def _parse_sidebar_state(text: str) -> dict[str, str]:
parsed: dict[str, str] = {}
for raw in text.splitlines():
line = raw.strip()
if not line or "=" not in line:
continue
key, value = line.split("=", 1)
parsed[key.strip()] = value.strip()
return parsed


def _wait_for_sidebar_git_branch(cli: str, workspace: str, timeout: float = 15.0) -> dict[str, str]:
deadline = time.time() + timeout
last_state = ""

while time.time() < deadline:
state_text = _run_cli(cli, ["sidebar-state", "--workspace", workspace])
last_state = state_text
state = _parse_sidebar_state(state_text)
raw_branch = state.get("git_branch", "")
branch = raw_branch.split(" ", 1)[0]
if branch and branch != "none":
return state
time.sleep(0.1)

raise cmuxError(
"Timed out waiting for background git metadata on new workspace. "
f"Last sidebar-state: {last_state!r}"
)


def _create_git_repo(root: Path) -> tuple[Path, str]:
repo = root / "repo"
repo.mkdir(parents=True, exist_ok=True)

subprocess.run(
["git", "-c", "init.defaultBranch=main", "init"],
cwd=repo,
check=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
subprocess.run(
["git", "config", "user.name", "cmux-test"],
cwd=repo,
check=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
subprocess.run(
["git", "config", "user.email", "cmux-test@example.com"],
cwd=repo,
check=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)

(repo / "README.md").write_text("issue 915\n", encoding="utf-8")
subprocess.run(
["git", "add", "README.md"],
cwd=repo,
check=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
subprocess.run(
["git", "-c", "commit.gpgsign=false", "commit", "-m", "init"],
cwd=repo,
check=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)

branch = subprocess.check_output(
["git", "rev-parse", "--abbrev-ref", "HEAD"],
cwd=repo,
text=True,
).strip()
return repo, branch


def main() -> int:
cli = _find_cli_binary()
temp_root = Path(tempfile.mkdtemp(prefix="cmux_issue_915_"))
created_workspace: str | None = None

try:
repo_path, expected_branch = _create_git_repo(temp_root)

with cmux(SOCKET_PATH) as c:
baseline_workspace = c.current_workspace()

created = _run_cli(cli, ["new-workspace", "--cwd", str(repo_path)])
_must(created.startswith("OK "), f"new-workspace expected OK response, got: {created!r}")
created_workspace = created.removeprefix("OK ").strip()
_must(bool(created_workspace), f"new-workspace returned no workspace handle: {created!r}")

_must(
c.current_workspace() == baseline_workspace,
"new-workspace --cwd should preserve selected workspace",
)

sidebar_state = _wait_for_sidebar_git_branch(cli, created_workspace)
_must(
sidebar_state.get("cwd", "") == str(repo_path),
f"Expected sidebar cwd={repo_path!r}, got {sidebar_state.get('cwd', '')!r}",
)

raw_branch = sidebar_state.get("git_branch", "")
observed_branch = raw_branch.split(" ", 1)[0]
_must(
observed_branch == expected_branch,
f"Expected sidebar git branch {expected_branch!r}, got {raw_branch!r}",
)

_must(
c.current_workspace() == baseline_workspace,
"background metadata load should not switch selected workspace",
)
finally:
if created_workspace:
try:
_run_cli(cli, ["close-workspace", "--workspace", created_workspace])
except Exception:
pass
Comment on lines +179 to +183

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, check the file exists and view the relevant lines with context
head -200 tests_v2/test_cli_new_workspace_background_metadata.py | tail -30

Repository: manaflow-ai/cmux

Length of output: 1118


🏁 Script executed:

# Get exact line count and content around lines 179-183
wc -l tests_v2/test_cli_new_workspace_background_metadata.py

Repository: manaflow-ai/cmux

Length of output: 118


🏁 Script executed:

# View lines 170-190 to get proper context
sed -n '170,190p' tests_v2/test_cli_new_workspace_background_metadata.py

Repository: manaflow-ai/cmux

Length of output: 760


🏁 Script executed:

# Check what imports are at the top of the file
head -30 tests_v2/test_cli_new_workspace_background_metadata.py

Repository: manaflow-ai/cmux

Length of output: 745


🏁 Script executed:

# Search for cmuxError in the codebase to understand what it is
rg "cmuxError" --type py

Repository: manaflow-ai/cmux

Length of output: 50372


🏁 Script executed:

# Search for _run_cli definition to understand what exceptions it raises
rg "def _run_cli" --type py -A 10

Repository: manaflow-ai/cmux

Length of output: 15013


🏁 Script executed:

# Look for similar exception handling patterns in test files
rg "except.*pass" tests_v2/ --type py -B 2 -A 1

Repository: manaflow-ai/cmux

Length of output: 42


Don't silently swallow workspace cleanup failures.

The bare except Exception: pass at lines 182-183 hides cleanup failures and makes test flakes harder to diagnose. Since _run_cli raises cmuxError on failure, catch that specifically and log the error instead of silencing it.

Proposed fix
         if created_workspace:
             try:
                 _run_cli(cli, ["close-workspace", "--workspace", created_workspace])
-            except Exception:
-                pass
+            except cmuxError as exc:
+                print(
+                    f"WARN: failed to close workspace {created_workspace!r}: {exc}",
+                    file=sys.stderr,
+                )
📝 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
if created_workspace:
try:
_run_cli(cli, ["close-workspace", "--workspace", created_workspace])
except Exception:
pass
if created_workspace:
try:
_run_cli(cli, ["close-workspace", "--workspace", created_workspace])
except cmuxError as exc:
print(
f"WARN: failed to close workspace {created_workspace!r}: {exc}",
file=sys.stderr,
)
🧰 Tools
🪛 Ruff (0.15.2)

[error] 182-183: try-except-pass detected, consider logging the exception

(S110)


[warning] 182-182: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests_v2/test_cli_new_workspace_background_metadata.py` around lines 179 -
183, The cleanup block currently swallows all exceptions; change the except to
catch the specific cmuxError raised by _run_cli and log the failure instead of
silent pass. Locate the cleanup conditional that checks created_workspace and
replace the bare "except Exception: pass" with "except cmuxError as e:" (or the
proper import) and call a logger (or pytest.fail/logging.error) to record e so
cleanup failures are visible.

shutil.rmtree(temp_root, ignore_errors=True)

print("PASS: new-workspace --cwd preloads sidebar metadata without focus")
return 0


if __name__ == "__main__":
raise SystemExit(main())