Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
30 changes: 26 additions & 4 deletions Resources/bin/claude
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,28 @@ clear_inherited_claude_auth_selection_env() {
normalize_claude_config_dir
}

# Inside cmux build variants that route child-process IO without a controlling
# terminal (e.g. "manual-io-minimal"), Ink-based TUIs such as Claude Code drop
# into non-interactive output because stdin is not a real PTY. Allocate one via
# BSD `script` before exec'ing claude so interactive TUIs render correctly.
# Set CMUX_CLAUDE_FORCE_PTY=0 to opt out.
# Ref: https://github.com/manaflow-ai/cmux/issues/4001
exec_real_claude() {
if [[ "${IN_CMUX:-0}" == "1" ]] \
&& [[ "${CMUX_CLAUDE_FORCE_PTY:-1}" != "0" ]] \
&& [[ ! -t 0 ]] \
&& [[ -x /usr/bin/script ]]; then
# BSD `script` calls tcgetattr on stdin and aborts when it cannot read
# attrs (some socket types). Probe with /usr/bin/true so a hostile
# stdin can never regress claude into "failed to start" — fall back to
# direct exec instead, preserving today's behavior.
if /usr/bin/script -q /dev/null /usr/bin/true >/dev/null 2>&1; then
exec /usr/bin/script -q /dev/null "$@"
fi
fi
exec "$@"
}
Comment on lines +183 to +197

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 PTY line-discipline processing may corrupt stdin data in automated invocations

A PTY's line discipline applies ONLCR (newline → CR+LF) and echo by default. In the common interactive case this is harmless, but the guard condition [[ ! -t 0 ]] fires precisely when stdin is a pipe or socket — i.e., the automated/scripted path. Any byte stream pumped into claude's stdin by cmux through that pipe (e.g., tool-result data, JSON payloads) passes through script's PTY master, where the line discipline can corrupt \n → \r\n before claude reads it. Consider calling stty -opost (or stty raw) on the PTY slave to suppress output processing if cmux ever uses stdio for structured data.


# Pass through if not in a cmux terminal, hooks are disabled, or the cmux
# socket is unavailable (stale env / app not running).
IN_CMUX=0
Expand All @@ -189,7 +211,7 @@ if [[ "$IN_CMUX" == "0" || "$CMUX_CLAUDE_HOOKS_DISABLED" == "1" ]] || ! cmux_soc
fi
clear_inherited_claude_auth_selection_env
REAL_CLAUDE="$(find_real_claude)" || { echo "Error: claude not found in PATH" >&2; exit 127; }
exec "$REAL_CLAUDE" "$@"
exec_real_claude "$REAL_CLAUDE" "$@"
fi

# Find real claude.
Expand Down Expand Up @@ -365,7 +387,7 @@ should_inject_claude_hooks() {
# invocations pass through so new Claude subcommands do not need cmux changes.
if ! should_inject_claude_hooks "$@"; then
clear_inherited_claude_auth_selection_env
exec "$REAL_CLAUDE" "$@"
exec_real_claude "$REAL_CLAUDE" "$@"
fi

# Unset CLAUDECODE to avoid "nested session" detection — cmux terminals are
Expand Down Expand Up @@ -421,8 +443,8 @@ fi
HOOKS_JSON='{"preferredNotifChannel":"notifications_disabled","hooks":{"SessionStart":[{"matcher":"","hooks":[{"type":"command","command":"\"${CMUX_CLAUDE_HOOK_CMUX_BIN:-cmux}\" hooks claude session-start","timeout":10}]}],"Stop":[{"matcher":"","hooks":[{"type":"command","command":"\"${CMUX_CLAUDE_HOOK_CMUX_BIN:-cmux}\" hooks claude stop","timeout":10}]},{"matcher":"","hooks":[{"type":"command","command":"\"${CMUX_CLAUDE_HOOK_CMUX_BIN:-cmux}\" hooks feed --source claude","timeout":10,"async":true}]}],"SessionEnd":[{"matcher":"","hooks":[{"type":"command","command":"\"${CMUX_CLAUDE_HOOK_CMUX_BIN:-cmux}\" hooks claude session-end","timeout":1}]}],"Notification":[{"matcher":"","hooks":[{"type":"command","command":"\"${CMUX_CLAUDE_HOOK_CMUX_BIN:-cmux}\" hooks claude notification","timeout":10}]}],"UserPromptSubmit":[{"matcher":"","hooks":[{"type":"command","command":"\"${CMUX_CLAUDE_HOOK_CMUX_BIN:-cmux}\" hooks claude prompt-submit","timeout":10}]}],"PreToolUse":[{"matcher":"CronCreate","hooks":[{"type":"command","command":"\"${CMUX_CLAUDE_HOOK_CMUX_BIN:-cmux}\" hooks claude cron-create-guard","timeout":5}]},{"matcher":"","hooks":[{"type":"command","command":"\"${CMUX_CLAUDE_HOOK_CMUX_BIN:-cmux}\" hooks claude pre-tool-use","timeout":5,"async":true}]}],"PermissionRequest":[{"matcher":"","hooks":[{"type":"command","command":"\"${CMUX_CLAUDE_HOOK_CMUX_BIN:-cmux}\" hooks feed --source claude","timeout":125}]}]}}'

if [[ "$SKIP_SESSION_ID" == true ]]; then
exec "$REAL_CLAUDE" --settings "$HOOKS_JSON" "$@"
exec_real_claude "$REAL_CLAUDE" --settings "$HOOKS_JSON" "$@"
else
SESSION_ID="$(uuidgen | tr '[:upper:]' '[:lower:]')"
exec "$REAL_CLAUDE" --session-id "$SESSION_ID" --settings "$HOOKS_JSON" "$@"
exec_real_claude "$REAL_CLAUDE" --session-id "$SESSION_ID" --settings "$HOOKS_JSON" "$@"
fi
115 changes: 115 additions & 0 deletions tests/test_claude_wrapper_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,120 @@ def test_stale_socket_skips_hook_injection(failures: list[str]) -> None:
expect(hook_cmux_bin == "__UNSET__", f"stale socket: expected hook cmux unset, got {hook_cmux_bin!r}", failures)


def run_wrapper_pty_probe(
*,
argv: list[str],
socket_state: str = "live",
) -> tuple[int, str, str]:
"""
Drive the wrapper with stdin=/dev/null (non-TTY) and capture whether the
fake real claude saw a TTY on its own stdin. Mirrors run_wrapper's faked
PATH layout but trims the parts that aren't relevant to the PTY check.
"""
with tempfile.TemporaryDirectory(prefix="cmux-claude-wrapper-pty-") as td:
tmp = Path(td)
wrapper_dir = tmp / "wrapper-bin"
real_dir = tmp / "real-bin"
wrapper_dir.mkdir(parents=True, exist_ok=True)
real_dir.mkdir(parents=True, exist_ok=True)

wrapper = wrapper_dir / "claude"
shutil.copy2(SOURCE_WRAPPER, wrapper)
wrapper.chmod(0o755)

tty_log = tmp / "real-tty-state.log"
cmux_log = tmp / "cmux.log"
socket_path = str(tmp / "cmux.sock")

make_executable(
real_dir / "claude",
"""#!/usr/bin/env bash
set -euo pipefail
if [ -t 0 ]; then
printf 'tty\\n' > "$FAKE_REAL_TTY_LOG"
else
printf 'not_a_tty\\n' > "$FAKE_REAL_TTY_LOG"
fi
""",
)

make_executable(
wrapper_dir / "cmux",
"""#!/usr/bin/env bash
set -euo pipefail
printf '%s\\n' "$*" >> "$FAKE_CMUX_LOG"
if [[ "${1:-}" == "--socket" ]]; then
shift 2
fi
if [[ "${1:-}" == "ping" ]]; then
if [[ "${FAKE_CMUX_PING_OK:-0}" == "1" ]]; then
exit 0
fi
exit 1
fi
exit 0
""",
)

test_socket: socket.socket | None = None
if socket_state in {"live", "stale"}:
test_socket = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
test_socket.bind(socket_path)

env = os.environ.copy()
env["PATH"] = f"{wrapper_dir}:{real_dir}:{env.get('PATH', '/usr/bin:/bin')}"
env["CMUX_SURFACE_ID"] = "surface:test"
env["CMUX_SOCKET_PATH"] = socket_path
env["FAKE_REAL_TTY_LOG"] = str(tty_log)
env["FAKE_CMUX_LOG"] = str(cmux_log)
env["FAKE_CMUX_PING_OK"] = "1" if socket_state == "live" else "0"
env.pop("NODE_OPTIONS", None)
env.pop("CMUX_CLAUDE_FORCE_PTY", None)

try:
proc = subprocess.run(
["claude", *argv],
cwd=tmp,
env=env,
stdin=subprocess.DEVNULL,
capture_output=True,
text=True,
check=False,
)
finally:
if test_socket is not None:
test_socket.close()

tty_state = tty_log.read_text(encoding="utf-8").strip() if tty_log.exists() else "<missing>"
return proc.returncode, tty_state, proc.stderr.strip()


def test_inside_cmux_without_tty_allocates_pty_for_real_claude(failures: list[str]) -> None:
"""
Regression for https://github.com/manaflow-ai/cmux/issues/4001.

cmux build variants that wire child-process IO without a controlling
terminal cause Ink-based TUIs (e.g. `claude`, `claude agents`) to drop
into non-interactive mode because stdin is not a real PTY. The wrapper
must allocate one so the spawned claude sees a TTY on stdin.
"""
if not os.access("/usr/bin/script", os.X_OK):
return # BSD `script` is required for the workaround.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Opt-out path (CMUX_CLAUDE_FORCE_PTY=0) not tested

The comment in the wrapper documents CMUX_CLAUDE_FORCE_PTY=0 as the escape hatch for the PTY allocation. run_wrapper_pty_probe pops this variable from the environment to ensure the default path is exercised, but there is no corresponding test that sets CMUX_CLAUDE_FORCE_PTY=0 and asserts the real claude still sees a non-TTY stdin. If the condition logic regresses (e.g., the != "0" check is changed), the only way a breakage would surface is in production.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

cases: list[tuple[str, list[str]]] = [
("interactive entry", []),
("agents subcommand", ["agents"]),
]
for label, argv in cases:
code, tty_state, stderr = run_wrapper_pty_probe(argv=argv)
expect(code == 0, f"pty alloc ({label}): wrapper exited {code}: {stderr}", failures)
expect(
tty_state == "tty",
f"pty alloc ({label}): expected real claude stdin to be a tty, got {tty_state!r}",
failures,
)


def main() -> int:
failures: list[str] = []
test_live_socket_injects_supported_hooks_without_unlocking_bypass(failures)
Expand All @@ -1010,6 +1124,7 @@ def main() -> int:
test_missing_socket_skips_hook_injection(failures)
test_disabled_integration_skips_hook_injection(failures)
test_stale_socket_skips_hook_injection(failures)
test_inside_cmux_without_tty_allocates_pty_for_real_claude(failures)

if failures:
print("FAIL: claude wrapper regression checks failed")
Expand Down