From d29cf60879dd1704d3b7e25302a446ee9026baf2 Mon Sep 17 00:00:00 2001 From: tebayoso Date: Fri, 8 May 2026 13:02:10 -0300 Subject: [PATCH] fix: bound /dev/tty write so hooks can't hang when Warp UI is unresponsive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OSC 777 notification write in warp-notify.sh blocks indefinitely if the slave PTY's output buffer fills up and stops draining — for example, when Warp's UI is hung and not reading from the master PTY. The block propagates back up the hook chain (warp-notify.sh -> on-*.sh -> claude), freezing the calling Claude Code session until Warp recovers. Observed in production as a 36-minute session lockup. Wrap the printf in a pure-bash watchdog: spawn the writer in the background, force-kill it after WARP_NOTIFY_TIMEOUT_SEC seconds (default 2) if the syscall hasn't returned. The script stays best-effort and always exits 0. A new WARP_NOTIFY_TARGET env var lets tests redirect output away from /dev/tty. Adds plugins/warp/tests/test-warp-notify.sh which simulates the failure mode via a FIFO with no reader (kernel-level identical to a slave PTY whose master isn't reading) and verifies the watchdog fires within the configured timeout. CI's globstar pattern picks up the new file automatically. Same patch applied to scripts/legacy/warp-notify.sh. --- plugins/warp/scripts/legacy/warp-notify.sh | 26 ++++- plugins/warp/scripts/warp-notify.sh | 33 +++++- plugins/warp/tests/test-warp-notify.sh | 126 +++++++++++++++++++++ 3 files changed, 181 insertions(+), 4 deletions(-) create mode 100755 plugins/warp/tests/test-warp-notify.sh diff --git a/plugins/warp/scripts/legacy/warp-notify.sh b/plugins/warp/scripts/legacy/warp-notify.sh index 6ca0588..e684c67 100755 --- a/plugins/warp/scripts/legacy/warp-notify.sh +++ b/plugins/warp/scripts/legacy/warp-notify.sh @@ -1,10 +1,32 @@ #!/bin/bash # Warp notification utility using OSC escape sequences # Usage: warp-notify.sh <body> +# +# The write to /dev/tty is bounded by WARP_NOTIFY_TIMEOUT_SEC (default 2) +# so an unresponsive Warp UI cannot block the caller indefinitely. TITLE="${1:-Notification}" BODY="${2:-}" +TARGET="${WARP_NOTIFY_TARGET:-/dev/tty}" +TIMEOUT_SEC="${WARP_NOTIFY_TIMEOUT_SEC:-2}" # OSC 777 format: \033]777;notify;<title>;<body>\007 -# Write directly to /dev/tty to ensure it reaches the terminal -printf '\033]777;notify;%s;%s\007' "$TITLE" "$BODY" > /dev/tty 2>/dev/null || true +SEQ=$(printf '\033]777;notify;%s;%s\007' "$TITLE" "$BODY") + +{ + printf '%s' "$SEQ" > "$TARGET" 2>/dev/null +} & +writer_pid=$! + +{ + sleep "$TIMEOUT_SEC" 2>/dev/null + kill -KILL "$writer_pid" 2>/dev/null +} & +watchdog_pid=$! + +wait "$writer_pid" 2>/dev/null + +kill -KILL "$watchdog_pid" 2>/dev/null +wait "$watchdog_pid" 2>/dev/null + +exit 0 diff --git a/plugins/warp/scripts/warp-notify.sh b/plugins/warp/scripts/warp-notify.sh index 523f873..7ebd697 100755 --- a/plugins/warp/scripts/warp-notify.sh +++ b/plugins/warp/scripts/warp-notify.sh @@ -4,6 +4,11 @@ # # For structured Warp notifications, title should be "warp://cli-agent" # and body should be a JSON string matching the cli-agent notification schema. +# +# The write to /dev/tty is bounded by WARP_NOTIFY_TIMEOUT_SEC (default 2). +# Without this bound, an unresponsive Warp UI — which leaves the controlling +# TTY's output buffer undrained — would block the calling Claude Code session +# indefinitely. Tests can redirect output via WARP_NOTIFY_TARGET. SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$SCRIPT_DIR/should-use-structured.sh" @@ -15,7 +20,31 @@ fi TITLE="${1:-Notification}" BODY="${2:-}" +TARGET="${WARP_NOTIFY_TARGET:-/dev/tty}" +TIMEOUT_SEC="${WARP_NOTIFY_TIMEOUT_SEC:-2}" # OSC 777 format: \033]777;notify;<title>;<body>\007 -# Write directly to /dev/tty to ensure it reaches the terminal -printf '\033]777;notify;%s;%s\007' "$TITLE" "$BODY" > /dev/tty 2>/dev/null || true +SEQ=$(printf '\033]777;notify;%s;%s\007' "$TITLE" "$BODY") + +# Spawn the writer in the background. If the target's output buffer is full +# and not draining (e.g. Warp UI hung), the open()/write() would otherwise +# block forever; the watchdog below caps that to TIMEOUT_SEC. +{ + printf '%s' "$SEQ" > "$TARGET" 2>/dev/null +} & +writer_pid=$! + +{ + sleep "$TIMEOUT_SEC" 2>/dev/null + kill -KILL "$writer_pid" 2>/dev/null +} & +watchdog_pid=$! + +wait "$writer_pid" 2>/dev/null + +# Tear down the watchdog (no-op if it already fired). +kill -KILL "$watchdog_pid" 2>/dev/null +wait "$watchdog_pid" 2>/dev/null + +# Notifications are best-effort; never propagate failure to the caller. +exit 0 diff --git a/plugins/warp/tests/test-warp-notify.sh b/plugins/warp/tests/test-warp-notify.sh new file mode 100755 index 0000000..530fd06 --- /dev/null +++ b/plugins/warp/tests/test-warp-notify.sh @@ -0,0 +1,126 @@ +#!/bin/bash +# Tests for warp-notify.sh hang protection. +# +# Verifies that warp-notify.sh: +# 1. Completes immediately when the target is writable (the happy path). +# 2. Exits cleanly within the configured timeout when the target's output +# buffer is full and never drained (the bug scenario: Warp UI hung). +# 3. Defaults to a sane upper bound (2s) without explicit configuration. +# 4. Same guarantees apply to the legacy variant. +# +# Implementation notes: +# - We simulate "Warp UI hung" by pointing WARP_NOTIFY_TARGET at a FIFO +# with no reader. The kernel blocks open()/write() on such a FIFO the +# same way it blocks writes to a slave PTY whose master isn't reading, +# which is the exact failure mode we observed in production. +# - We export WARP_CLI_AGENT_PROTOCOL_VERSION and WARP_CLIENT_VERSION so +# should_use_structured returns true and we exercise the write path. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../scripts" && pwd)" + +export WARP_CLI_AGENT_PROTOCOL_VERSION=1 +export WARP_CLIENT_VERSION="v0.2026.04.01.08.00.stable_00" + +PASSED=0 +FAILED=0 + +assert_eq() { + local test_name="$1" + local expected="$2" + local actual="$3" + if [ "$expected" = "$actual" ]; then + echo " ✓ $test_name" + PASSED=$((PASSED + 1)) + else + echo " ✗ $test_name" + echo " expected: $expected" + echo " actual: $actual" + FAILED=$((FAILED + 1)) + fi +} + +assert_lt() { + local test_name="$1" + local actual="$2" + local upper="$3" + if [ "$actual" -lt "$upper" ] 2>/dev/null; then + echo " ✓ $test_name ($actual < $upper)" + PASSED=$((PASSED + 1)) + else + echo " ✗ $test_name (got $actual, expected < $upper)" + FAILED=$((FAILED + 1)) + fi +} + +cleanup() { + [ -n "${FIFO:-}" ] && rm -f "$FIFO" +} +trap cleanup EXIT + +run_notify() { + local script="$1" + shift + local start end + start=$(date +%s) + bash "$script" "warp://cli-agent" '{"v":1,"agent":"claude","event":"test"}' "$@" + LAST_RC=$? + end=$(date +%s) + LAST_ELAPSED=$((end - start)) +} + +echo "=== warp-notify.sh hang protection ===" + +echo "" +echo "--- Fast path: writable target completes immediately ---" +WARP_NOTIFY_TARGET=/dev/null run_notify "$SCRIPT_DIR/warp-notify.sh" +assert_eq "writable target exits 0" "0" "$LAST_RC" +assert_lt "writable target completes under 2s" "$LAST_ELAPSED" "2" + +echo "" +echo "--- Hang protection: blocked target times out at configured limit ---" +FIFO=$(mktemp -u) +mkfifo "$FIFO" +WARP_NOTIFY_TARGET="$FIFO" WARP_NOTIFY_TIMEOUT_SEC=1 \ + run_notify "$SCRIPT_DIR/warp-notify.sh" +assert_eq "blocked target still exits 0 (best-effort)" "0" "$LAST_RC" +# Timeout=1s plus watchdog/teardown overhead — generous bound to avoid CI flake. +assert_lt "blocked target exits within 4s" "$LAST_ELAPSED" "4" +rm -f "$FIFO" + +echo "" +echo "--- Default timeout caps unbounded waits ---" +FIFO=$(mktemp -u) +mkfifo "$FIFO" +WARP_NOTIFY_TARGET="$FIFO" run_notify "$SCRIPT_DIR/warp-notify.sh" +assert_eq "default timeout still exits 0" "0" "$LAST_RC" +# Default is 2s; allow 5s for CI scheduling jitter. +assert_lt "default timeout exits within 5s" "$LAST_ELAPSED" "5" +rm -f "$FIFO" + +echo "" +echo "=== legacy/warp-notify.sh hang protection ===" + +echo "" +echo "--- Fast path: writable target completes immediately ---" +WARP_NOTIFY_TARGET=/dev/null run_notify "$SCRIPT_DIR/legacy/warp-notify.sh" +assert_eq "legacy writable target exits 0" "0" "$LAST_RC" +assert_lt "legacy writable target completes under 2s" "$LAST_ELAPSED" "2" + +echo "" +echo "--- Hang protection: blocked target times out ---" +FIFO=$(mktemp -u) +mkfifo "$FIFO" +WARP_NOTIFY_TARGET="$FIFO" WARP_NOTIFY_TIMEOUT_SEC=1 \ + run_notify "$SCRIPT_DIR/legacy/warp-notify.sh" +assert_eq "legacy blocked target still exits 0" "0" "$LAST_RC" +assert_lt "legacy blocked target exits within 4s" "$LAST_ELAPSED" "4" +rm -f "$FIFO" + +echo "" +echo "=== Results: $PASSED passed, $FAILED failed ===" + +if [ "$FAILED" -gt 0 ]; then + exit 1 +fi