From 20c62b3b9aab75a0fde0f7b2815d12d693fd94a7 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 6 Feb 2026 22:32:14 -0800 Subject: [PATCH] Fix CI race condition on self-hosted runner (#19) * Fix zsh ZDOTDIR wrapper + log parsing with -- messages * Fix CI race condition: serialize self-hosted builds with concurrency group Two workflows racing on the same self-hosted runner caused DerivedData corruption (release's rm -rf nuked DerivedData while CI was building). Add shared concurrency group and scope DerivedData cleanup to project. --- .github/workflows/ci.yml | 3 + .github/workflows/release.yml | 5 +- CLI/cmuxterm.swift | 34 ++++++----- Resources/shell-integration/.zlogin | 16 ++++- Resources/shell-integration/.zprofile | 16 ++++- Resources/shell-integration/.zshenv | 30 ++++++++-- Sources/TerminalController.swift | 4 +- tests/cmux.py | 6 +- tests/test_shell_zdotdir_wrapper.py | 84 +++++++++++++++++++++++++++ tests/test_sidebar_log_parsing.py | 64 ++++++++++++++++++++ 10 files changed, 235 insertions(+), 27 deletions(-) create mode 100644 tests/test_shell_zdotdir_wrapper.py create mode 100644 tests/test_sidebar_log_parsing.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f2e0d61..a0b09cb7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,9 @@ on: jobs: ui-tests: runs-on: self-hosted + concurrency: + group: self-hosted-build + cancel-in-progress: false steps: - name: Checkout uses: actions/checkout@v4 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d93b6e85..26ee32a2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,6 +12,9 @@ permissions: jobs: build-sign-notarize: runs-on: self-hosted + concurrency: + group: self-hosted-build + cancel-in-progress: false steps: - name: Checkout uses: actions/checkout@v4 @@ -53,7 +56,7 @@ jobs: - name: Clear SPM cache run: | rm -rf ~/Library/Caches/org.swift.swiftpm - rm -rf ~/Library/Developer/Xcode/DerivedData + rm -rf ~/Library/Developer/Xcode/DerivedData/GhosttyTabs-* - name: Configure SwiftPM cache run: | diff --git a/CLI/cmuxterm.swift b/CLI/cmuxterm.swift index aace114f..e6842f8b 100644 --- a/CLI/cmuxterm.swift +++ b/CLI/cmuxterm.swift @@ -352,21 +352,25 @@ struct CMUXCLI { let response = try client.send(command: cmd) print(response) - case "log": - // Remove options by position (flag + following value), not by string value, - // so message tokens that happen to equal an option value aren't dropped. - let (level, argsWithoutLevel) = parseOption(commandArgs, name: "--level") - let (source, argsWithoutSource) = parseOption(argsWithoutLevel, name: "--source") - let (explicitTab, remaining) = parseOption(argsWithoutSource, name: "--tab") - let tabArg = explicitTab ?? ProcessInfo.processInfo.environment["CMUX_TAB_ID"] - let message = remaining.joined(separator: " ") - guard !message.isEmpty else { throw CLIError(message: "log requires a message") } - var cmd = "log \(message)" - if let level { cmd += " --level=\(level)" } - if let source { cmd += " --source=\(source)" } - if let tabArg { cmd += " --tab=\(tabArg)" } - let response = try client.send(command: cmd) - print(response) + case "log": + // Remove options by position (flag + following value), not by string value, + // so message tokens that happen to equal an option value aren't dropped. + let (level, argsWithoutLevel) = parseOption(commandArgs, name: "--level") + let (source, argsWithoutSource) = parseOption(argsWithoutLevel, name: "--source") + let (explicitTab, remaining) = parseOption(argsWithoutSource, name: "--tab") + let tabArg = explicitTab ?? ProcessInfo.processInfo.environment["CMUX_TAB_ID"] + let message = remaining.joined(separator: " ") + guard !message.isEmpty else { throw CLIError(message: "log requires a message") } + // TerminalController.parseOptions treats any --* token as an option until a + // `--` separator. Options must come before the message to preserve arbitrary + // message contents (including tokens like `--force`). + var cmd = "log" + if let level { cmd += " --level=\(level)" } + if let source { cmd += " --source=\(source)" } + if let tabArg { cmd += " --tab=\(tabArg)" } + cmd += " -- \(quoteOptionValue(message))" + let response = try client.send(command: cmd) + print(response) case "clear-log": let tabArg = optionValue(commandArgs, name: "--tab") ?? ProcessInfo.processInfo.environment["CMUX_TAB_ID"] diff --git a/Resources/shell-integration/.zlogin b/Resources/shell-integration/.zlogin index ae72ee41..b6782440 100644 --- a/Resources/shell-integration/.zlogin +++ b/Resources/shell-integration/.zlogin @@ -1,4 +1,16 @@ # cmuxterm ZDOTDIR wrapper — sources user's .zlogin +_cmux_wrapper_zdotdir="${ZDOTDIR:-}" _cmux_real_zdotdir="${CMUX_ORIGINAL_ZDOTDIR:-$HOME}" -[ -f "$_cmux_real_zdotdir/.zlogin" ] && source "$_cmux_real_zdotdir/.zlogin" -unset _cmux_real_zdotdir +if [ -f "$_cmux_real_zdotdir/.zlogin" ]; then + ZDOTDIR="$_cmux_real_zdotdir" + source "$_cmux_real_zdotdir/.zlogin" +fi + +# Restore whatever ZDOTDIR was for the current shell. +if [ -n "$_cmux_wrapper_zdotdir" ]; then + ZDOTDIR="$_cmux_wrapper_zdotdir" +else + unset ZDOTDIR +fi + +unset _cmux_real_zdotdir _cmux_wrapper_zdotdir diff --git a/Resources/shell-integration/.zprofile b/Resources/shell-integration/.zprofile index 63bfa3e9..510ed93b 100644 --- a/Resources/shell-integration/.zprofile +++ b/Resources/shell-integration/.zprofile @@ -1,4 +1,16 @@ # cmuxterm ZDOTDIR wrapper — sources user's .zprofile +_cmux_wrapper_zdotdir="${ZDOTDIR:-}" _cmux_real_zdotdir="${CMUX_ORIGINAL_ZDOTDIR:-$HOME}" -[ -f "$_cmux_real_zdotdir/.zprofile" ] && source "$_cmux_real_zdotdir/.zprofile" -unset _cmux_real_zdotdir +if [ -f "$_cmux_real_zdotdir/.zprofile" ]; then + ZDOTDIR="$_cmux_real_zdotdir" + source "$_cmux_real_zdotdir/.zprofile" +fi + +# Restore wrapper ZDOTDIR so zsh continues through our wrapper chain. +if [ -n "$_cmux_wrapper_zdotdir" ]; then + ZDOTDIR="$_cmux_wrapper_zdotdir" +else + unset ZDOTDIR +fi + +unset _cmux_real_zdotdir _cmux_wrapper_zdotdir diff --git a/Resources/shell-integration/.zshenv b/Resources/shell-integration/.zshenv index 1cc30c59..71cc236c 100644 --- a/Resources/shell-integration/.zshenv +++ b/Resources/shell-integration/.zshenv @@ -1,6 +1,28 @@ # cmuxterm ZDOTDIR wrapper — sources user's .zshenv -# NOTE: Do NOT restore ZDOTDIR here. It must stay pointed at our wrapper dir -# so that zsh finds our .zshrc next. Restoration happens in .zshrc. +# +# zsh resolves startup files relative to $ZDOTDIR. We point $ZDOTDIR at this +# wrapper directory so zsh loads our wrappers, but we must preserve the user's +# semantics when sourcing their real files. In particular, many setups rely on +# $ZDOTDIR inside early startup files, so source with ZDOTDIR temporarily +# restored to the original value. +_cmux_wrapper_zdotdir="${ZDOTDIR:-}" _cmux_real_zdotdir="${CMUX_ORIGINAL_ZDOTDIR:-$HOME}" -[ -f "$_cmux_real_zdotdir/.zshenv" ] && source "$_cmux_real_zdotdir/.zshenv" -unset _cmux_real_zdotdir +if [ -f "$_cmux_real_zdotdir/.zshenv" ]; then + ZDOTDIR="$_cmux_real_zdotdir" + source "$_cmux_real_zdotdir/.zshenv" +fi + +# For interactive shells, keep the wrapper chain intact so zsh loads our +# .zprofile/.zshrc wrappers next. For non-interactive shells, leave ZDOTDIR +# pointing at the real directory to avoid surprising script semantics. +case $- in + *i*) + if [ -n "$_cmux_wrapper_zdotdir" ]; then + ZDOTDIR="$_cmux_wrapper_zdotdir" + else + unset ZDOTDIR + fi + ;; +esac + +unset _cmux_real_zdotdir _cmux_wrapper_zdotdir diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 350bc7c0..3e812ec5 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -301,7 +301,7 @@ class TerminalController { set_status [--icon=X] [--color=#hex] - Set a status entry clear_status [--tab=X] - Remove a status entry list_status [--tab=X] - List all status entries - log [--level=X] [--source=X] [--tab=X] - Append a log entry + log [--level=X] [--source=X] [--tab=X] -- - Append a log entry clear_log [--tab=X] - Clear log entries list_log [--limit=N] [--tab=X] - List log entries set_progress <0.0-1.0> [--label=X] [--tab=X] - Set progress bar @@ -1189,7 +1189,7 @@ class TerminalController { guard tabManager != nil else { return "ERROR: TabManager not available" } let parsed = parseOptions(args) guard !parsed.positional.isEmpty else { - return "ERROR: Missing message — usage: log [--level=X] [--source=X] [--tab=X]" + return "ERROR: Missing message — usage: log [--level=X] [--source=X] [--tab=X] -- " } let message = parsed.positional.joined(separator: " ") let levelStr = parsed.options["level"] ?? "info" diff --git a/tests/cmux.py b/tests/cmux.py index f0b6b36a..9efc9876 100755 --- a/tests/cmux.py +++ b/tests/cmux.py @@ -497,13 +497,17 @@ class cmux: def log(self, message: str, level: str = None, source: str = None, tab: str = None) -> None: """Append a sidebar log entry.""" - cmd = f"log {message}" + # TerminalController.parseOptions treats any --* token as an option until + # a `--` separator. Put options first and then use `--` so messages can + # contain arbitrary tokens like `--force`. + cmd = "log" if level: cmd += f" --level={level}" if source: cmd += f" --source={source}" if tab: cmd += f" --tab={tab}" + cmd += f" -- {_quote_option_value(message)}" response = self._send_command(cmd) if not response.startswith("OK"): raise cmuxError(response) diff --git a/tests/test_shell_zdotdir_wrapper.py b/tests/test_shell_zdotdir_wrapper.py new file mode 100644 index 00000000..c899f55e --- /dev/null +++ b/tests/test_shell_zdotdir_wrapper.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python3 +""" +Regression: zsh wrapper startup files must source user files with the *original* +ZDOTDIR, not the wrapper directory. + +The cmuxterm zsh integration sets ZDOTDIR to the app's wrapper directory so zsh +loads wrapper .zshenv/.zprofile/.zshrc. Those wrappers must temporarily restore +ZDOTDIR while sourcing the user's real startup files so $ZDOTDIR semantics match +normal zsh behavior. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +import shutil +from pathlib import Path + + +def main() -> int: + root = Path(__file__).resolve().parents[1] + wrapper_dir = root / "Resources" / "shell-integration" + if not (wrapper_dir / ".zshenv").exists(): + print(f"SKIP: missing wrapper .zshenv at {wrapper_dir}") + return 0 + + base = Path("/tmp") / f"cmux_zdotdir_test_{os.getpid()}" + try: + if base.exists(): + for child in base.iterdir(): + try: + child.unlink() + except Exception: + pass + base.mkdir(parents=True, exist_ok=True) + + orig = base / "orig" + orig.mkdir(parents=True, exist_ok=True) + seen_path = base / "seen.txt" + + # User .zshenv that records the ZDOTDIR it sees. + (orig / ".zshenv").write_text( + 'echo "$ZDOTDIR" > "$CMUX_ZDOTDIR_TEST_OUTPUT"\n', + encoding="utf-8", + ) + + env = dict(os.environ) + env["ZDOTDIR"] = str(wrapper_dir) + env["CMUX_ORIGINAL_ZDOTDIR"] = str(orig) + env["CMUX_ZDOTDIR_TEST_OUTPUT"] = str(seen_path) + env["CMUX_SHELL_INTEGRATION"] = "0" + + # Non-interactive is enough: .zshenv is always sourced. + result = subprocess.run( + ["zsh", "-c", "true"], + env=env, + capture_output=True, + text=True, + ) + if result.returncode != 0: + print("FAIL: zsh exited non-zero") + print(result.stderr.strip()) + return 1 + + if not seen_path.exists(): + print("FAIL: user .zshenv did not run (no output file created)") + return 1 + + seen = seen_path.read_text(encoding="utf-8").strip() + expected = str(orig) + if seen != expected: + print(f"FAIL: user .zshenv saw ZDOTDIR={seen!r}, expected {expected!r}") + return 1 + + print("PASS: zsh user startup files see original ZDOTDIR") + return 0 + + finally: + shutil.rmtree(base, ignore_errors=True) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_sidebar_log_parsing.py b/tests/test_sidebar_log_parsing.py new file mode 100644 index 00000000..f7cc3e73 --- /dev/null +++ b/tests/test_sidebar_log_parsing.py @@ -0,0 +1,64 @@ +#!/usr/bin/env python3 +""" +Regression: sidebar log messages must preserve tokens that start with `--`. + +TerminalController.parseOptions() treats `--*` tokens as options until a `--` +separator. The log command must therefore send options before the message and +use `--` so arbitrary message contents round-trip correctly. + +Run with a tagged instance to avoid unix socket conflicts: + CMUX_TAG= python3 tests/test_sidebar_log_parsing.py +""" + +from __future__ import annotations + +import os +import sys +import time + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +from cmux import cmux, cmuxError # noqa: E402 + + +def _assert_contains(text: str, needle: str) -> None: + if needle not in (text or ""): + raise AssertionError(f"Expected to find: {needle}\n\nGot:\n{text}") + + +def main() -> int: + try: + with cmux() as client: + tab_id = client.new_tab() + client.select_tab(tab_id) + time.sleep(0.7) + + client._send_command(f"clear_log --tab={tab_id}") + + msg1 = "hello --force mid -- --level=not-an-option end" + client.log(msg1, level="warning", source="test", tab=tab_id) + + msg2 = "--force starts-with-dashdash" + client.log(msg2, level="info", source="test", tab=tab_id) + + time.sleep(0.2) + out = client._send_command(f"list_log --tab={tab_id} --limit=2") + _assert_contains(out, f"[warning] {msg1} (source=test)") + _assert_contains(out, f"[info] {msg2} (source=test)") + + try: + client.close_tab(tab_id) + except Exception: + pass + + print("PASS: log messages preserve `--*` tokens") + return 0 + + except (cmuxError, AssertionError) as exc: + print(f"FAIL: {exc}") + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) +