Add extra caution to terminal color detection (#963)

.github/workflows/vhs.yaml

@@ -46,8 +46,8 @@ jobs:
           echo "BANNER_MARK=🚀" >> ${ENV_FILE}
           echo "ASSUME_ROLE_INACTIVE_MARK= " >> ${ENV_FILE}
           echo "PROMPT_HOST_MARK=(demo)" >> ${ENV_FILE}
-          mkdir -p "$HOME/.geodesic"
-          cat > "$HOME/.geodesic/preferences" <<-'EOF'
+          mkdir -p "$HOME/.geodesic/defaults"
+          cat > "$HOME/.geodesic/defaults/preferences" <<-'EOF'
 
           function demo-tofu-version() {
             printf '\nTofu Version as of when we recorded this demo:\n'

demo.tape

@@ -30,12 +30,13 @@ Set PlaybackSpeed 1
 
 Set Shell bash
 
-Type "# After having run `make all`, simply run `make run` (or `geodesic`) to enter the interactive shell" Enter
+Type "# After having run `DOCKER_BASE_TAG=latest make install`," Enter
+Type "# simply run `make run` (or `geodesic`) to enter the interactive shell" Enter
 Sleep 500ms
 
 Type "geodesic" Enter
 
-Sleep 4s
+Sleep 7s
 
 Type "#  Open Tofu is pre-installed:" Enter
 Type "demo-tofu-version" Enter

rootfs/etc/profile.d/_07-term-mode.sh

@@ -17,29 +17,33 @@
 # First, at startup, let's try an OSC query. If we get no response, we will assume light mode
 # and disable further queries.
 
+function _terminal_trace() {
+	if [[ $GEODESIC_TRACE =~ "terminal" ]]; then
+		# Use tput and sgr0 here because this may be early in the startup sequence and trace logging when color functions are not yet available.
+		echo "$(tput setaf 1)* TERMINAL TRACE: $*$(tput sgr0)" >&2
+	fi
+}
+
 function _verify_terminal_queries_are_supported() {
-	if tty -s && [[ -n "$(tput setaf 1 2>/dev/null)" ]]; then
-		local saved_state x fg_rgb bg_rgb exit_code
-		saved_state=$(stty -g)
-		trap 'stty "$saved_state"' EXIT
-		[[ $GEODESIC_TRACE =~ "terminal" ]] && echo "$(tput setaf 1)* TERMINAL TRACE: Checking if terminal responds to color queries...$(tput sgr0)" >&2
-		stty -echo
-		echo -ne '\e]10;?\a\e]11;?\a' >/dev/tty
-		# If 2 seconds is not enough at startup, then the terminal is either non-responsive or too slow.
-		IFS=: read -rs -t 2 -d $'\a' x fg_rgb
-		exit_code=$?
-		[[ $exit_code -gt 128 ]] || exit_code=0
-		IFS=: read -rs -t 0.5 -d $'\a' x bg_rgb
-		((exit_code += $?))
-		stty "$saved_state"
-		trap - EXIT
-		if [[ $exit_code -gt 128 ]] || [[ -z $fg_rgb ]] || [[ -z $bg_rgb ]]; then
-			if [[ $GEODESIC_TRACE =~ "terminal" ]]; then
-				echo "$(tput setaf 1)* TERMINAL TRACE: Terminal did not respond to OSC 10 and 11 queries. Disabling color mode detection.$(tput sgr0)" >&2
-			fi
-			export GEODESIC_TERM_COLOR_AUTO=unsupported
-		fi
+	local colors
+	colors=$(tput colors 2>/dev/null) || colors=0
+
+	if ! { [[ -t 0 ]] && [[ "$colors" -ge 8 ]]; }; then
+		# Do not use _terminal_trace here, because it uses color codes on terminals and we have just verified that is not supported here.
+		[[ $GEODESIC_TRACE =~ "terminal" ]] && echo "* TERMINAL TRACE: Not a (color) terminal. Disabling color detection." >&2
+		export GEODESIC_TERM_COLOR_AUTO=unsupported
+		return 1
 	fi
+
+	if ! { [[ -w /dev/tty ]] && [[ -r /dev/tty ]]; }; then
+		_terminal_trace "Terminal is not writable or readable. Skipping color detection."
+		_terminal_trace "You may need to run 'chmod o+rw /dev/tty' to enable color detection."
+		_terminal_trace "You can disable color detection with 'export GEODESIC_TERM_COLOR_AUTO=disabled'."
+		return 1
+	fi
+
+	[[ "${GEODESIC_TERM_COLOR_AUTO}" == "disabled" ]] || unset GEODESIC_TERM_COLOR_AUTO
+	return 0
 }
 
 _verify_terminal_queries_are_supported
@@ -50,7 +54,7 @@ _verify_terminal_queries_are_supported
 # and always returns true. With -l it outputs integer luminance values for foreground
 # and background colors. With -ll it outputs labels on the luminance values as well.
 function _is_term_dark_mode() {
-	[[ ${GEODESIC_TERM_COLOR_AUTO} == "unsupported" ]] && case "$1" in
+	[[ ${GEODESIC_TERM_COLOR_AUTO} == "unsupported" ]] || [[ ${GEODESIC_TERM_COLOR_AUTO} == "disabled" ]] && case "$1" in
 	-b) echo "false" ;;
 	-bb) echo "unknown" ;;
 	-m) echo "light" ;;
@@ -64,31 +68,29 @@ function _is_term_dark_mode() {
 
 	# Do not try to auto-detect if we are not in a terminal
 	# or if termcap does not think we are in a color terminal
-	if tty -s && [[ -n "$(tput setaf 1 2>/dev/null)" ]]; then
+	if _verify_terminal_queries_are_supported; then
 		# Extract the RGB values of the foreground and background colors via OSC 10 and 11.
 		# Redirect output to `/dev/tty` in case we are in a subshell where output is a pipe,
 		# because this output has to go directly to the terminal.
 		saved_state=$(stty -g)
 		trap 'stty "$saved_state"' EXIT
-		[[ $GEODESIC_TRACE =~ "terminal" ]] && echo "$(tput setaf 1)* TERMINAL TRACE: Checking terminal color scheme...$(tput sgr0)" >&2
+		_terminal_trace 'Checking terminal color scheme...'
 		stty -echo
 		echo -ne '\e]10;?\a\e]11;?\a' >/dev/tty
 		# Timeout of 2 was not enough when waking for sleep.
 		# The second read should be part of the first response, should not need much time at all regardless.
 		# When in a signal handler, we might be waking from sleep or hibernation, so we give it a lot more time.
 		timeout_duration=$([[ ${GEODESIC_TERM_COLOR_SIGNAL} == "true" ]] && echo 30 || echo 1)
-		IFS=: read -rs -t "$timeout_duration" -d $'\a' x fg_rgb
+		IFS=: read -rs -t "$timeout_duration" -d $'\a' x fg_rgb </dev/tty
 		exit_code=$?
 		[[ $exit_code -gt 128 ]] || [[ -z $fg_rgb ]] && [[ ${GEODESIC_TERM_COLOR_SIGNAL} == "true" ]] && export GEODESIC_TERM_COLOR_AUTO=disabled
 		[[ $exit_code -gt 128 ]] || exit_code=0
-		IFS=: read -rs -t 0.5 -d $'\a' x bg_rgb
+		IFS=: read -rs -t 0.5 -d $'\a' x bg_rgb </dev/tty
 		((exit_code += $?))
 		stty "$saved_state"
 		trap - EXIT
 	else
-		if [[ $GEODESIC_TRACE =~ "terminal" ]]; then
-			echo "* TERMINAL TRACE: ${FUNCNAME[0]} called, but not running in a color terminal." >&2
-		fi
+		_terminal_trace "${FUNCNAME[0]} called, but not running in a color terminal."
 	fi
 
 	if [[ ${GEODESIC_TERM_COLOR_SIGNAL} == "true" ]] && [[ ${GEODESIC_TERM_COLOR_AUTO} == "disabled" ]]; then
@@ -98,9 +100,7 @@ function _is_term_dark_mode() {
 	fi
 
 	if [[ $exit_code -gt 128 ]] || [[ -z $fg_rgb ]] || [[ -z $bg_rgb ]]; then
-		if [[ $GEODESIC_TRACE =~ "terminal" ]] && tty -s; then
-			echo "$(tput setaf 1)* TERMINAL TRACE: Terminal did not respond to OSC 10 and 11 queries.$(tput sgr0)" >&2
-		fi
+		_terminal_trace "Terminal did not respond to OSC 10 and 11 queries."
 		# If we cannot determine the color scheme, we assume light mode for historical reasons.
 		if [[ "$*" =~ -b ]] || [[ "$*" =~ -m ]]; then
 			if [[ "$*" =~ -bb ]] || [[ "$*" =~ -mm ]]; then
@@ -118,7 +118,7 @@ function _is_term_dark_mode() {
 	if [[ "${x#*;}" != "rgb" ]]; then
 		# Always output this error, because we want to hear about
 		# other color formats users want us to support.
-		echo "$(tput set bold)($tput setaf 1)Terminal reported unknown color format: ${x#*;}$(tput sgr0)" >&2
+		echo "$(tput set bold)$(tput setaf 1)Terminal reported unknown color format: ${x#*;}$(tput sgr0)" >&2
 		return 1
 	fi
 
@@ -180,10 +180,7 @@ function _srgb_to_luminance() {
 	local red green blue
 
 	if [[ -z $color ]]; then
-		if [[ $GEODESIC_TRACE =~ "terminal" ]]; then
-			# Use tput and sgr0 here because this is early in the startup sequence and trace logging
-			echo "$(tput setaf 1)* TERMINAL TRACE: ${FUNCNAME[0]} called with empty or no argument.$(tput sgr0)" >&2
-		fi
+		_terminal_trace "${FUNCNAME[0]} called with empty or no argument."
 		echo "0"
 		return
 	fi

rootfs/etc/profile.d/_10-colors.sh

@@ -60,7 +60,10 @@ function get-terminal-color-mode() {
 }
 
 function _is_color_term() {
-	[[ -t 1 ]] && tty -s && [[ -n "$(tput op 2 2>/dev/null)" ]]
+	local colors
+	colors=$(tput colors 2>/dev/null) || colors=0
+
+	[[ -t 0 ]] && [[ "$colors" -ge 8 ]]
 }
 
 # We call `tput` several times for every prompt, and it can add up, so we cache the results.
@@ -70,18 +73,9 @@ function _geodesic_tput_cache_init() {
 	# Save the terminal type so we can invalidate the cache if it changes
 	_geodesic_tput_cache[TERM]="$TERM"
 
-	local color_off=$(tput op 2>/dev/null) # reset foreground and background colors to defaults
-
-	if ! tty -s || [[ -z $color_off ]]; then
-		if [[ $GEODESIC_TRACE =~ "terminal" ]]; then
-			if ! tty -s; then
-				echo '* TERMINAL TRACE: Not running in a terminal, not attempting to colorize output' >&2
-			elif [[ -z $color_off ]]; then
-				echo '* TERMINAL TRACE: `tput` did not output anything for "op", not attempting to colorize output' >&2
-			else
-				echo '* TERMINAL TRACE: Unknown error (script bug), not attempting to colorize output' >&2
-			fi
-		fi
+	local color_off # reset foreground and background colors to defaults
+
+	if ! { color_off=$(tput op 2>/dev/null) && _verify_terminal_queries_are_supported; }; then
 		if [[ $old_term != ${_geodesic_tput_cache[TERM]} ]]; then
 			_geodesic_generate_color_functions dummy
 			command -V geodesic_prompt_style &>/dev/null && geodesic_prompt_style

rootfs/etc/profile.d/_40-preferences.sh

@@ -37,10 +37,10 @@ elif [[ ! -d $GEODESIC_CONFIG_HOME ]]; then
 fi
 
 function _term_fold() {
-    	local cols
-    	cols=$(tput cols 2>/dev/null || echo "80")
-    	[ -z "$cols" ] || [ "$cols" = "0" ] && cols=80
-    	fold -w "$cols" -s
+	local cols
+	cols=$(tput cols 2>/dev/null || echo "80")
+	[ -z "$cols" ] || [ "$cols" = "0" ] && cols=80
+	fold -w "$cols" -s
 }
 
 [[ -n ${WORKSPACE_MOUNT} ]] || export WORKSPACE_MOUNT=/workspace