diff --git a/PROJECT.md b/PROJECT.md index 91a5268..7c31154 100644 --- a/PROJECT.md +++ b/PROJECT.md @@ -378,3 +378,8 @@ _Last updated: 2026-08-24 (review-triage session)_ - **Decisions parked for PM** (also listed in projects/DECISIONS.md Open): #55 v1/v2 ADR, #54 wide-char strategy, #57 hover scope. - **Review verdict worth keeping:** the `$()`/exit-code/fd-3 contract, Docker 3.2/4.4/5.x matrix, and load-bearing gotcha comments are the differentiators — don't trade them away while hardening. fd-3 fix (#48) must keep the persistent-fd design, only make the number configurable. - **Next**: worker picks up #41 → #42 → #43 → #44 in order (`claude --cwd ~/lib/fissible/shellframe "Work on issue #41"`), then #55 ADR before touching #46/#53 so consolidations go the right direction. Cut v0.6.0 when M5 closes. +- **Launch-blocker fixes session (2026-08-24)**: All four Phase 8 launch blockers closed — [#41](https://github.com/fissible/shellframe/issues/41) (app-runtime `declare -F` guards + NEXT assertion, exit 1 with diagnostic), [#42](https://github.com/fissible/shellframe/issues/42) (all 40 `%b` specifiers across confirm/alert/table/action-list converted to `%s`; behavior-neutral for `$'..'` byte constants, immune to literal-backslash color overrides), [#43](https://github.com/fissible/shellframe/issues/43) (INT→130/TERM→143 split traps; cleanup runs exactly once), [#44](https://github.com/fissible/shellframe/issues/44) (EOF discriminator in `_shellframe_shell_read_key`: rc≤128+empty = EOF → clean quit, shellframe_shell returns 1; rc>128 = timeout tick). Suite: **1527/1527 across 49 files**, stable ×3. + - **Cross-repo (fissible/ptyunit), commit `96d6f04` on branch `hardening/review-2026-08`**: #43's PTY test only failed under the suite runner. Root cause: bash sets SIGINT/SIGQUIT ignored in async worker jobs; the ignore inherits fork+exec, and POSIX forbids trapping an entry-ignored signal — guest TUIs could never see Ctrl-C under any async runner. Fix: pty_run.py/pty_session.py restore SIG_DFL in the forked child pre-exec; plus all waitpids routed through bounded exception-safe `_reap()` (early-exit children can no longer lose status or wedge blocking waits). ptyunit suite 761/761. + - **PM/consumer flag**: ptyunit fix is on a branch, not merged to main — needs merge + submodule/reference bump in shellframe, shellql, seed (shellframe CI uses the sibling checkout so it picks it up automatically; brew-installed copy is stale until formula bump). + - **Gotcha worth keeping**: pty_run buffers all output until child exit — a hung child looks like "zero output" and misled two debug rounds; use xtrace-to-side-file for liveness, not captured stdout. + - **Next**: #55 ADR (v1/v2 sunset plan) before touching #46/#53; then M-severity batch (#45–#47). Cut v0.6.0 when #45 closes. diff --git a/docs/api.md b/docs/api.md index 7163f7e..99595a3 100644 --- a/docs/api.md +++ b/docs/api.md @@ -136,6 +136,22 @@ Call inside a `shellframe_raw_enter` session. Compare results against the Uses `read -d ''` (NUL delimiter) so Enter (`\n`) is captured rather than consumed as the line terminator (see [Hard-won lessons](hard-won-lessons.md#7-bash-read-converts-r-to-n-internally--use-read--d--for-enter)). +Sets `SHELLFRAME_KEY_EOF=1` when stdin has reached EOF; widget loops must +check it and exit cancelled — looping on an EOF read spins at 100% CPU (#44). + +**Exit/return-code contracts** + +| Call | Returns | +|---|---| +| `shellframe_confirm` | 0 = yes, 1 = no/cancelled (incl. stdin EOF) | +| `shellframe_action_list`, `shellframe_table` | 0 = confirmed, 1 = quit (incl. stdin EOF) | +| `shellframe_shell` | 0 = user exited normally, 1 = stdin EOF (runtime lost its input source) | +| `shellframe_app` | 1 + stderr diagnostic on misconfiguration (unknown screen, missing render/handler, handler that never sets `_SHELLFRAME_APP_NEXT`) | + +While a runtime is active it owns EXIT/INT/TERM trap handling (terminal +state must be restored even on crashes); the caller's own traps are saved at +entry and restored verbatim on normal return. + --- ## `src/draw.sh` @@ -149,7 +165,7 @@ so its `${#raw}` byte count equals its visible character count. ```bash local raw="~/bin/gflow" local rendered="${SHELLFRAME_GRAY}~/bin/${SHELLFRAME_RESET}${SHELLFRAME_BOLD}gflow${SHELLFRAME_RESET}" -printf '%b' "$(shellframe_pad_left "$raw" "$rendered" 20)" +printf '%s' "$(shellframe_pad_left "$raw" "$rendered" 20)" ``` Color constants `SHELLFRAME_BOLD`, `SHELLFRAME_RESET`, `SHELLFRAME_GREEN`, `SHELLFRAME_RED`, diff --git a/docs/hard-won-lessons.md b/docs/hard-won-lessons.md index 54e68f3..0215b8b 100644 --- a/docs/hard-won-lessons.md +++ b/docs/hard-won-lessons.md @@ -111,7 +111,7 @@ and use its `${#raw}` length to compute padding manually. printf "%-20b" "${SHELLFRAME_GREEN}hello${SHELLFRAME_RESET}" # ✓ — measure raw, output rendered + explicit padding -printf '%b' "$(shellframe_pad_left "hello" "${SHELLFRAME_GREEN}hello${SHELLFRAME_RESET}" 20)" +printf '%s' "$(shellframe_pad_left "hello" "${SHELLFRAME_GREEN}hello${SHELLFRAME_RESET}" 20)" ``` --- diff --git a/docs/showcase.md b/docs/showcase.md index dd983c5..d139298 100644 --- a/docs/showcase.md +++ b/docs/showcase.md @@ -110,7 +110,7 @@ _draw_row() { (( i == SHELLFRAME_AL_SELECTED )) && cursor="> " local -a acts; IFS=' ' read -r -a acts <<< "$acts_str" local action="${acts[$aidx]}" - printf "%b%-14s [ %-8s]\n" "$cursor" "$label" "$action" + printf "%s%-14s [ %-8s]\n" "$cursor" "$label" "$action" } shellframe_action_list "_draw_row" "" \ diff --git a/src/app.sh b/src/app.sh index 3132773..267a7cf 100644 --- a/src/app.sh +++ b/src/app.sh @@ -78,6 +78,14 @@ _SHELLFRAME_APP_QUESTION="" _SHELLFRAME_APP_TITLE="" _SHELLFRAME_APP_DETAILS=() +# Map widget return code → event name string +# Configuration-error diagnostic. Callers print this then `return 1` from +# shellframe_app — deliberately NOT exit: app.sh is a sourced library and +# must never kill the host script (#41 review). +_shellframe_app_die() { + printf 'shellframe: %s\n' "$1" >&2 +} + # Map widget return code → event name string _shellframe_app_event() { local _type="$1" _rc="$2" @@ -112,7 +120,18 @@ shellframe_app() { # Get screen type (pure — subshell OK), run render hook (direct — can mutate globals) local _type + if ! declare -F "${_prefix}_${_current}_type" >/dev/null 2>&1; then + _shellframe_app_die "unknown screen '${_current}' (prefix '${_prefix}') — missing ${_prefix}_${_current}_type()" + return 1 + fi _type=$("${_prefix}_${_current}_type") + # Render hook: same existence discipline (#41 review) — a typo'd + # render name would otherwise run the widget against stale context + # globals and silently succeed. + if ! declare -F "${_prefix}_${_current}_render" >/dev/null 2>&1; then + _shellframe_app_die "unknown screen '${_current}' (prefix '${_prefix}') — missing ${_prefix}_${_current}_render()" + return 1 + fi "${_prefix}_${_current}_render" # Run the widget for this screen type @@ -148,14 +167,27 @@ shellframe_app() { fi _rc=$? ;; + *) + _shellframe_app_die "screen '${_current}': widget type '${_type}' is not action-list|table|confirm|alert" + return 1 + ;; esac # Map rc → event name, call event handler directly (not in $() — safe to # mutate globals). Handler must set _SHELLFRAME_APP_NEXT to the next screen name. - local _event + local _event _event_fn _event=$(_shellframe_app_event "$_type" "$_rc") + _event_fn="${_prefix}_${_current}_${_event}" + if ! declare -F "$_event_fn" >/dev/null 2>&1; then + _shellframe_app_die "screen '${_current}': missing ${_event_fn}() handler for widget type '${_type}'" + return 1 + fi _SHELLFRAME_APP_NEXT="" - "${_prefix}_${_current}_${_event}" + "$_event_fn" + if [[ -z "${_SHELLFRAME_APP_NEXT:-}" ]]; then + _shellframe_app_die "screen '${_current}': ${_event_fn}() did not set _SHELLFRAME_APP_NEXT" + return 1 + fi _current="$_SHELLFRAME_APP_NEXT" done } diff --git a/src/draw.sh b/src/draw.sh index 2a30b5e..a73fef8 100644 --- a/src/draw.sh +++ b/src/draw.sh @@ -16,12 +16,12 @@ # Usage: # local raw="~/bin/gflow" # local rendered="${GRAY}~/bin/${RESET}${BOLD}gflow${RESET}" -# printf '%b' "$(shellframe_pad_left "$raw" "$rendered" 20)" +# printf '%s' "$(shellframe_pad_left "$raw" "$rendered" 20)" shellframe_pad_left() { local raw="$1" rendered="$2" width="$3" local pad=$(( width - ${#raw} )) (( pad < 0 )) && pad=0 - printf '%b%*s' "$rendered" "$pad" '' + printf '%s%*s' "$rendered" "$pad" '' } # ── Color constants ─────────────────────────────────────────────────────────── diff --git a/src/input.sh b/src/input.sh index dbc7e92..be20221 100644 --- a/src/input.sh +++ b/src/input.sh @@ -114,6 +114,12 @@ SHELLFRAME_MOUSE_SHIFT=0 SHELLFRAME_MOUSE_META=0 SHELLFRAME_MOUSE_CTRL=0 +# Output variable set by shellframe_read_key when stdin has reached EOF +# (#44). 1 = EOF; reset to 0 at the start of every call. The initial read is +# untimed, so EOF (rc=1, empty value) is unambiguous on all bash versions. +# Widget loops must check this flag and exit cancelled instead of spinning. +SHELLFRAME_KEY_EOF=0 + # Read one keypress (including full escape sequences) into a variable. # # Usage: @@ -136,7 +142,12 @@ SHELLFRAME_MOUSE_CTRL=0 shellframe_read_key() { local _out_var="${1:-_SHELLFRAME_KEY}" local _k _c - IFS= read -r -n1 -d '' _k + SHELLFRAME_KEY_EOF=0 + IFS= read -r -n1 -d '' _k || { [[ -z "$_k" ]] && SHELLFRAME_KEY_EOF=1; } + if (( SHELLFRAME_KEY_EOF )); then + printf -v "$_out_var" '%s' "" + return 0 + fi if [[ "$_k" == $'\x1b' ]]; then IFS= read -r -n1 -d '' -t 1 _c _k+="${_c}" diff --git a/src/shell.sh b/src/shell.sh index 7978dde..eb757f2 100644 --- a/src/shell.sh +++ b/src/shell.sh @@ -348,13 +348,33 @@ _shellframe_shell_draw_if_dirty() { _shellframe_shell_read_key() { local _out_var="${1:-_SHELLFRAME_KEY}" - local _k="" _c="" - - # Timeout: 1 second. If SIGWINCH fires, bash 3.2 on macOS will NOT - # interrupt read, but the 1s timeout ensures we re-check the flag. - IFS= read -r -n1 -d '' -t 1 _k || true + local _k="" _c="" _rc=0 + _SHELLFRAME_KEY_EOF=0 + + # Timeout handling — VERSION-DEPENDENT semantics (#44): + # bash >= 4.0: timer expiry returns >128; stdin EOF returns <=128. + # The return code alone discriminates reliably. + # bash 3.2: BOTH return 1 (verified against /bin/bash 3.2). The + # discriminator is TIME: EOF returns instantly while a + # timer expiry consumes the full window. So on 3.2 the + # window is widened to 2 s and measured with date +%s — + # an instant failure (delta 0) is EOF; anything that + # consumed seconds is a timeout/SIGWINCH tick. A false + # EOF here would quit the runtime during ordinary idling. + if (( BASH_VERSINFO[0] >= 4 )); then + _sf_read_t=1 + else + _sf_read_t=2 + _sf_t0=$(date +%s) + fi + IFS= read -r -n1 -d '' -t "$_sf_read_t" _k || _rc=$? if [[ -z "$_k" ]]; then + if (( BASH_VERSINFO[0] >= 4 )); then + (( _rc > 0 && _rc <= 128 )) && _SHELLFRAME_KEY_EOF=1 + elif (( _rc > 0 )) && (( $(date +%s) - _sf_t0 < 2 )); then + _SHELLFRAME_KEY_EOF=1 # failed instantly — stdin reached EOF + fi printf -v "$_out_var" '%s' "" return 0 fi @@ -406,6 +426,7 @@ shellframe_shell() { local _prefix="$1" local _current="${2:-ROOT}" _SHELLFRAME_SHELL_RUNNING=1 + _SHELLFRAME_SHELL_EOF=0 local _saved_stty _saved_stty=$(shellframe_raw_save) @@ -440,7 +461,24 @@ shellframe_shell() { } >> /tmp/shql-crash.log 2>/dev/null fi } - trap "_shellframe_shell_cleanup '$_saved_stty'" EXIT INT TERM + # Preserve any caller-installed EXIT/INT/TERM traps (review 2026-08-24): + # shellframe is sourced by other tools, and unconditionally leaving our + # traps installed after the runtime returns would silently drop the + # host script's own teardown. Contract: while the runtime is active it + # owns these signals (its cleanup must win); on normal return the + # caller's traps are restored verbatim below. Caller traps do NOT fire + # for signals that terminate the runtime abnormally mid-session. + _SF_PREV_EXIT_TRAP=$(trap -p EXIT) + _SF_PREV_INT_TRAP=$(trap -p INT) + _SF_PREV_TERM_TRAP=$(trap -p TERM) + + # EXIT alone runs cleanup on normal termination and on any error unwind. + # INT/TERM additionally restore-and-exit with the conventional signal exit + # codes (#43): traps are cleared first so the pending EXIT trap does not + # double-run cleanup. + trap "_shellframe_shell_cleanup '$_saved_stty'" EXIT + trap "trap - EXIT INT TERM WINCH; _shellframe_shell_cleanup '$_saved_stty'; exit 130" INT + trap "trap - EXIT INT TERM WINCH; _shellframe_shell_cleanup '$_saved_stty'; exit 143" TERM local _k_tab="${SHELLFRAME_KEY_TAB:-$'\t'}" local _k_shift_tab="${SHELLFRAME_KEY_SHIFT_TAB:-$'\033[Z'}" @@ -476,6 +514,12 @@ shellframe_shell() { local _key="" _shellframe_shell_read_key _key + # Stdin EOF (tty detached, stdin redirected closed): quit the + # runtime cleanly with a non-zero status instead of spinning (#44). + if (( ${_SHELLFRAME_KEY_EOF:-0} )); then + _SHELLFRAME_SHELL_EOF=1 + _current="__QUIT__"; _screen_done=1; continue + fi if [[ -z "$_key" ]]; then # Timeout: tick toasts so they expire even when user is idle if (( ${#_SHELLFRAME_TOAST_QUEUE[@]} > 0 )); then @@ -660,4 +704,13 @@ shellframe_shell() { elif [[ -w /dev/tty ]]; then printf '\033[?2004l\033[?1006l\033[?1000l\033[?25h\033[?1049l' >/dev/tty 2>/dev/null fi + # Hand the caller's own EXIT/INT/TERM traps back (saved at entry) so host + # scripts keep their teardown after the runtime returns. + [[ -n "$_SF_PREV_EXIT_TRAP" ]] && eval "$_SF_PREV_EXIT_TRAP" + [[ -n "$_SF_PREV_INT_TRAP" ]] && eval "$_SF_PREV_INT_TRAP" + [[ -n "$_SF_PREV_TERM_TRAP" ]] && eval "$_SF_PREV_TERM_TRAP" + # Stdin EOF quit (#44): report non-zero so callers can distinguish a + # clean user exit from the runtime losing its input source. + (( ${_SHELLFRAME_SHELL_EOF:-0} )) && return 1 + return 0 } diff --git a/src/widgets/action-list.sh b/src/widgets/action-list.sh index 21c7e40..d390339 100644 --- a/src/widgets/action-list.sh +++ b/src/widgets/action-list.sh @@ -57,7 +57,7 @@ _shellframe_al_default_draw_row() { local -a _dacts IFS=' ' read -r -a _dacts < <(printf '%s\n' "$_dacts_str") local _daction="${_dacts[$_daidx]}" - printf "%b%-24s [%s]\n" "$_dcursor" "$_dlabel" "$_daction" + printf "%s%-24s [%s]\n" "$_dcursor" "$_dlabel" "$_daction" } # _shellframe_action_list_on_key key n_items @@ -189,6 +189,11 @@ shellframe_action_list() { _prev_sel=$SHELLFRAME_AL_SELECTED shellframe_read_key _key + # stdin EOF: quit cancelled instead of spinning (#44) + if (( ${SHELLFRAME_KEY_EOF:-0} )); then + _al_retval=1; break + fi + _shellframe_action_list_on_key "$_key" "$_n" _krc=$? diff --git a/src/widgets/alert.sh b/src/widgets/alert.sh index 8fb52bc..28d7b1d 100644 --- a/src/widgets/alert.sh +++ b/src/widgets/alert.sh @@ -54,20 +54,20 @@ _shellframe_alert_render() { local _i # top border - printf '\033[%d;%dH%b+' "$_row" "$_c0" "$SHELLFRAME_GRAY" >&3 + printf '\033[%d;%dH%s+' "$_row" "$_c0" "$SHELLFRAME_GRAY" >&3 for (( _i=0; _i<_inner; _i++ )); do printf '-' >&3; done - printf '+%b' "$SHELLFRAME_RESET" >&3 + printf '+%s' "$SHELLFRAME_RESET" >&3 (( _row++ )) # blank - printf '\033[%d;%dH%b|%*s|%b' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" >&3 + printf '\033[%d;%dH%s|%*s|%s' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" >&3 (( _row++ )) # title (centered, bold) local _tl="${#_title}" local _tlpad=$(( (_inner - _tl) / 2 )) local _trpad=$(( _inner - _tl - _tlpad )) - printf '\033[%d;%dH%b|%b%*s%b%s%b%*s%b|%b' \ + printf '\033[%d;%dH%s|%s%*s%s%s%s%*s%s|%s' \ "$_row" "$_c0" \ "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" \ "$_tlpad" "" \ @@ -77,7 +77,7 @@ _shellframe_alert_render() { (( _row++ )) # blank - printf '\033[%d;%dH%b|%*s|%b' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" >&3 + printf '\033[%d;%dH%s|%*s|%s' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" >&3 (( _row++ )) # detail lines @@ -86,28 +86,28 @@ _shellframe_alert_render() { local _ll="${#_line}" local _rpad=$(( _inner - _ll - 2 )) (( _rpad < 0 )) && _rpad=0 - printf '\033[%d;%dH%b|%b %s%*s%b|%b' \ + printf '\033[%d;%dH%s|%s %s%*s%s|%s' \ "$_row" "$_c0" \ "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" \ "$_line" "$_rpad" "" \ "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" >&3 (( _row++ )) done - printf '\033[%d;%dH%b|%*s|%b' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" >&3 + printf '\033[%d;%dH%s|%*s|%s' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" >&3 (( _row++ )) fi # bottom border - printf '\033[%d;%dH%b+' "$_row" "$_c0" "$SHELLFRAME_GRAY" >&3 + printf '\033[%d;%dH%s+' "$_row" "$_c0" "$SHELLFRAME_GRAY" >&3 for (( _i=0; _i<_inner; _i++ )); do printf '-' >&3; done - printf '+%b' "$SHELLFRAME_RESET" >&3 + printf '+%s' "$SHELLFRAME_RESET" >&3 (( _row++ )) # footer hint local _hint="Any key to continue" local _hcol=$(( _c0 + (_box_w - ${#_hint}) / 2 )) (( _hcol < 1 )) && _hcol=1 - printf '\033[%d;%dH%b%s%b' "$_row" "$_hcol" "$SHELLFRAME_GRAY" "$_hint" "$SHELLFRAME_RESET" >&3 + printf '\033[%d;%dH%s%s%s' "$_row" "$_hcol" "$SHELLFRAME_GRAY" "$_hint" "$SHELLFRAME_RESET" >&3 } shellframe_alert() { diff --git a/src/widgets/confirm.sh b/src/widgets/confirm.sh index 4a0e0e8..e9e7468 100644 --- a/src/widgets/confirm.sh +++ b/src/widgets/confirm.sh @@ -49,9 +49,9 @@ _shellframe_confirm_draw_buttons() { local _brpad=$(( _inner - _btn_raw - _blpad )) (( _blpad < 1 )) && _blpad=1 (( _brpad < 0 )) && _brpad=0 - printf '\033[%d;%dH%b|%b' "$_brow" "$_c0" "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" - printf '%*s%b %b%*s' "$_blpad" "" "$_yes_str" "$_no_str" "$_brpad" "" - printf '%b|%b' "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" + printf '\033[%d;%dH%s|%s' "$_brow" "$_c0" "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" + printf '%*s%s %s%*s' "$_blpad" "" "$_yes_str" "$_no_str" "$_brpad" "" + printf '%s|%s' "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" } # _shellframe_confirm_on_key key @@ -161,13 +161,13 @@ shellframe_confirm() { local _i # top border - printf '\033[%d;%dH%b+' "$_row" "$_c0" "$SHELLFRAME_GRAY" + printf '\033[%d;%dH%s+' "$_row" "$_c0" "$SHELLFRAME_GRAY" for (( _i=0; _i<_inner; _i++ )); do printf '-'; done - printf '+%b' "$SHELLFRAME_RESET" + printf '+%s' "$SHELLFRAME_RESET" (( _row++ )) # blank - printf '\033[%d;%dH%b|%*s|%b' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" + printf '\033[%d;%dH%s|%*s|%s' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" (( _row++ )) # detail lines @@ -177,7 +177,7 @@ shellframe_confirm() { local _ll="${#_line}" local _rpad=$(( _inner - _ll - 2 )) (( _rpad < 0 )) && _rpad=0 - printf '\033[%d;%dH%b|%b %s%*s%b|%b' \ + printf '\033[%d;%dH%s|%s %s%*s%s|%s' \ "$_row" "$_c0" \ "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" \ "$_line" "$_rpad" "" \ @@ -185,7 +185,7 @@ shellframe_confirm() { (( _row++ )) done # blank separator - printf '\033[%d;%dH%b|%*s|%b' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" + printf '\033[%d;%dH%s|%*s|%s' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" (( _row++ )) fi @@ -193,7 +193,7 @@ shellframe_confirm() { local _ql="${#_question}" local _qlpad=$(( (_inner - _ql) / 2 )) local _qrpad=$(( _inner - _ql - _qlpad )) - printf '\033[%d;%dH%b|%b%*s%b%s%b%*s%b|%b' \ + printf '\033[%d;%dH%s|%s%*s%s%s%s%*s%s|%s' \ "$_row" "$_c0" \ "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" \ "$_qlpad" "" \ @@ -203,7 +203,7 @@ shellframe_confirm() { (( _row++ )) # blank - printf '\033[%d;%dH%b|%*s|%b' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" + printf '\033[%d;%dH%s|%*s|%s' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" (( _row++ )) # buttons (row == _btn_row) @@ -211,20 +211,20 @@ shellframe_confirm() { (( _row++ )) # blank - printf '\033[%d;%dH%b|%*s|%b' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" + printf '\033[%d;%dH%s|%*s|%s' "$_row" "$_c0" "$SHELLFRAME_GRAY" "$_inner" "" "$SHELLFRAME_RESET" (( _row++ )) # bottom border - printf '\033[%d;%dH%b+' "$_row" "$_c0" "$SHELLFRAME_GRAY" + printf '\033[%d;%dH%s+' "$_row" "$_c0" "$SHELLFRAME_GRAY" for (( _i=0; _i<_inner; _i++ )); do printf '-'; done - printf '+%b' "$SHELLFRAME_RESET" + printf '+%s' "$SHELLFRAME_RESET" (( _row++ )) # footer hint local _hint="←/→ select y/n quick Enter confirm" local _hcol=$(( _c0 + (_box_w - ${#_hint}) / 2 )) (( _hcol < 1 )) && _hcol=1 - printf '\033[%d;%dH%b%s%b' "$_row" "$_hcol" "$SHELLFRAME_GRAY" "$_hint" "$SHELLFRAME_RESET" + printf '\033[%d;%dH%s%s%s' "$_row" "$_hcol" "$SHELLFRAME_GRAY" "$_hint" "$SHELLFRAME_RESET" else # ── Partial redraw (_dirty=1): button row only ───────────────── @@ -241,6 +241,12 @@ shellframe_confirm() { while true; do local _key _krc shellframe_read_key _key + + # stdin EOF: exit cancelled instead of spinning (#44) + if (( ${SHELLFRAME_KEY_EOF:-0} )); then + break # _retval stays 1 (cancelled) + fi + _shellframe_confirm_on_key "$_key" _krc=$? if (( _krc == 2 )); then diff --git a/src/widgets/editor.sh b/src/widgets/editor.sh index ed8e6ec..5f1e490 100644 --- a/src/widgets/editor.sh +++ b/src/widgets/editor.sh @@ -1077,9 +1077,16 @@ shellframe_editor_on_key() { if [[ "$_key" == "$_k_paste_start" ]]; then # Bracketed paste: drain all keys until paste-end, then insert as one # batch — single ensure_visible / vmap rebuild at the end. + # stdin EOF ends the drain too (#44) — otherwise a lost paste-end + # marker would consume keystrokes forever. local _paste_buf="" _paste_key="" while true; do shellframe_read_key _paste_key + if (( ${SHELLFRAME_KEY_EOF:-0} )); then + # stdin vanished mid-paste: keep what the editor already had, + # do NOT submit (rc=2) or report unhandled (rc=1) + shellframe_shell_mark_dirty; return 0 + fi [[ "$_paste_key" == "$_k_paste_end" ]] && break _paste_buf="${_paste_buf}${_paste_key}" done diff --git a/src/widgets/table.sh b/src/widgets/table.sh index 117b60a..d64b4bc 100644 --- a/src/widgets/table.sh +++ b/src/widgets/table.sh @@ -89,7 +89,7 @@ _shellframe_tbl_default_draw_row() { local -a _dacts IFS=' ' read -r -a _dacts <<< "$_dacts_str" local _daction="${_dacts[$_daidx]}" - printf "%b%-24s [%s]" "$_dcursor" "$_dlabel" "$_daction" + printf "%s%-24s [%s]" "$_dcursor" "$_dlabel" "$_daction" } SHELLFRAME_TBL_SAVED_STTY="" SHELLFRAME_TBL_COLS=0 @@ -238,27 +238,27 @@ shellframe_table() { local _content_top=1 local _fi if [[ -n "$SHELLFRAME_TBL_PAGE_TITLE" || -n "$SHELLFRAME_TBL_PAGE_H1" ]]; then - printf '\033[1;1H%b%b %s\033[K%b' \ + printf '\033[1;1H%s%s %s\033[K%s' \ "$SHELLFRAME_REVERSE" "$SHELLFRAME_BOLD" \ "$SHELLFRAME_TBL_PAGE_TITLE" \ "$SHELLFRAME_RESET" - printf '\033[2;1H%b %s%b' \ + printf '\033[2;1H%s %s%s' \ "$SHELLFRAME_BOLD$SHELLFRAME_WHITE" \ "$SHELLFRAME_TBL_PAGE_H1" \ "$SHELLFRAME_RESET" - printf '\033[3;1H%b' "$SHELLFRAME_GRAY" + printf '\033[3;1H%s' "$SHELLFRAME_GRAY" for (( _fi=0; _fi<_cols; _fi++ )); do printf '─'; done - printf '%b' "$SHELLFRAME_RESET" + printf '%s' "$SHELLFRAME_RESET" _content_top=4 fi # ── Page chrome: bottom ─────────────────────────────────────── local _content_bottom=$_rows if [[ -n "$SHELLFRAME_TBL_PAGE_FOOTER" ]]; then - printf '\033[%d;1H%b' "$(( _rows - 1 ))" "$SHELLFRAME_GRAY" + printf '\033[%d;1H%s' "$(( _rows - 1 ))" "$SHELLFRAME_GRAY" for (( _fi=0; _fi<_cols; _fi++ )); do printf '─'; done - printf '%b' "$SHELLFRAME_RESET" - printf '\033[%d;1H%b %s\033[K%b' \ + printf '%s' "$SHELLFRAME_RESET" + printf '\033[%d;1H%s %s\033[K%s' \ "$_rows" \ "$SHELLFRAME_GRAY" \ "$SHELLFRAME_TBL_PAGE_FOOTER" \ @@ -299,13 +299,13 @@ shellframe_table() { for (( _hi=0; _hi<_n_headers; _hi++ )); do local _hdr="${SHELLFRAME_TBL_HEADERS[$_hi]}" local _hw="${SHELLFRAME_TBL_COL_WIDTHS[$_hi]:-${#_hdr}}" - printf '%b%-*s%b' \ + printf '%s%-*s%s' \ "$SHELLFRAME_BOLD$SHELLFRAME_WHITE" "$_hw" "$_hdr" \ "$SHELLFRAME_RESET" done - printf '\033[%d;1H %b' "$(( _content_top + 1 ))" "$SHELLFRAME_GRAY" + printf '\033[%d;1H %s' "$(( _content_top + 1 ))" "$SHELLFRAME_GRAY" for (( _fi=0; _fi<_table_width-2; _fi++ )); do printf '─'; done - printf '%b' "$SHELLFRAME_RESET" + printf '%s' "$SHELLFRAME_RESET" fi # ── Data rows ───────────────────────────────────────────────── @@ -338,14 +338,14 @@ shellframe_table() { fi done - printf '\033[%d;1H\033[2K %b%s%b' \ + printf '\033[%d;1H\033[2K %s%s%s' \ "$_hint_row" \ "$SHELLFRAME_GRAY" "$_footer" "$SHELLFRAME_RESET" if (( _show_panel )); then local _sep_row for (( _sep_row=_content_top; _sep_row<=_content_bottom; _sep_row++ )); do - printf '\033[%d;%dH%b│%b' \ + printf '\033[%d;%dH%s│%s' \ "$_sep_row" "$(( _table_width + 1 ))" \ "$SHELLFRAME_GRAY" "$SHELLFRAME_RESET" done @@ -355,9 +355,9 @@ shellframe_table() { fi if (( _below_total > 0 )) && [[ -n "$SHELLFRAME_TBL_BELOW_FN" ]]; then - printf '\033[%d;1H\033[2K %b' "$(( _hint_row + 1 ))" "$SHELLFRAME_GRAY" + printf '\033[%d;1H\033[2K %s' "$(( _hint_row + 1 ))" "$SHELLFRAME_GRAY" for (( _fi=0; _fi<_table_width-2; _fi++ )); do printf '─'; done - printf '%b' "$SHELLFRAME_RESET" + printf '%s' "$SHELLFRAME_RESET" "$SHELLFRAME_TBL_BELOW_FN" \ "$(( _hint_row + 2 ))" 1 "$_cols" "$_below_rows" fi @@ -411,6 +411,11 @@ shellframe_table() { _prev_sel=$SHELLFRAME_TBL_SELECTED # snapshot before key handling shellframe_read_key _key + # stdin EOF: quit cancelled instead of spinning (#44) + if (( ${SHELLFRAME_KEY_EOF:-0} )); then + _tbl_retval=1; break + fi + _shellframe_table_on_key "$_key" "$_n" local _krc=$? diff --git a/tests/docker/run-matrix.sh b/tests/docker/run-matrix.sh index 419de36..3368be2 100755 --- a/tests/docker/run-matrix.sh +++ b/tests/docker/run-matrix.sh @@ -21,9 +21,18 @@ if ! command -v docker >/dev/null 2>&1; then fi # Resolve ptyunit installation to mount into containers (no Homebrew inside Alpine) -_ptyunit_host=$(brew --prefix ptyunit 2>/dev/null)/libexec -if [[ ! -f "$_ptyunit_host/run.sh" ]]; then - printf 'error: ptyunit not found at %s\n' "$_ptyunit_host" >&2 +# Prefer a sibling checkout (fissible workspace layout — also lives under /Users, +# which Docker Desktop shares by default; /opt/homebrew is typically NOT shared +# and makes the mount fail with "Mounts denied"), fall back to Homebrew. +# This file lives at /tests/docker/: the workspace parent is three levels up. +_ptyunit_host="" +if [[ -f "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)/ptyunit/run.sh" ]]; then + _ptyunit_host="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)/ptyunit" +elif _prefix=$(brew --prefix ptyunit 2>/dev/null); then + _ptyunit_host="$_prefix/libexec" +fi +if [[ -z "$_ptyunit_host" ]] || [[ ! -f "$_ptyunit_host/run.sh" ]]; then + printf 'error: ptyunit not found (sibling checkout or brew)\n' >&2 printf 'Run: bash bootstrap.sh\n' >&2 exit 1 fi diff --git a/tests/fixtures/confirm-eof.sh b/tests/fixtures/confirm-eof.sh new file mode 100644 index 0000000..89b5751 --- /dev/null +++ b/tests/fixtures/confirm-eof.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash +# tests/fixtures/confirm-eof.sh — v1 confirm widget with detached stdin (#44) +# +# shellframe_read_key's untimed first read hits EOF instantly; the widget +# loop must exit cancelled instead of spinning at 100% CPU. +set -u +source "$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)/shellframe.sh" + +exec 0 "$_fifo" ) & +exec 0< "$_fifo" + +_sl_ITEMS=("apple") +SHELLFRAME_LIST_CTX="sl" +SHELLFRAME_LIST_ITEMS=("${_sl_ITEMS[@]}") +shellframe_list_init "sl" 10 + +_sl_ROOT_render() { + local _rows _cols + _shellframe_shell_terminal_size _rows _cols + shellframe_shell_region "list" 1 1 "$_cols" "$(( _rows - 1 ))" + shellframe_shell_region "footer" "$_rows" 1 "$_cols" 1 nofocus +} + +_sl_ROOT_list_render() { + SHELLFRAME_LIST_CTX="sl" + shellframe_list_render "$@" +} + +shellframe_shell "_sl" "ROOT" +_sf_rc=$? +rm -f "$_fifo" +printf 'idle-shell-returned:%d\n' "$_sf_rc" diff --git a/tests/integration/test-confirm.sh b/tests/integration/test-confirm.sh index ee5e5eb..d8c45f2 100755 --- a/tests/integration/test-confirm.sh +++ b/tests/integration/test-confirm.sh @@ -47,4 +47,12 @@ ptyunit_test_begin "confirm: Esc key — cancelled" out=$(_pty ESC) assert_contains "$out" "Cancelled" + +# ── #44 review follow-up: confirm exits cancelled on stdin EOF ─────────────── + +ptyunit_test_begin "confirm #44: detached stdin exits promptly with rc=1" +_ce_rc=0 +out=$( python3 "$PTY_RUN" "$SHELLFRAME_DIR/tests/fixtures/confirm-eof.sh" ) || _ce_rc=$? +assert_contains "$out" "confirm-rc:1" +assert_eq "0" "$_ce_rc" ptyunit_test_summary diff --git a/tests/integration/test-mouse-routing.sh b/tests/integration/test-mouse-routing.sh index 08d1b9d..9179d34 100644 --- a/tests/integration/test-mouse-routing.sh +++ b/tests/integration/test-mouse-routing.sh @@ -70,4 +70,33 @@ ptyunit_test_begin "mouse-routing: scroll-down then click visible row selects co out=$(_pty $'\x1b[<65;1;1M' ENTER) assert_contains "$out" "Selected: apple" +# ── #43: Ctrl-C exits the shell runtime cleanly ────────────────────────────── +# With stty isig (default), \x03 raises SIGINT. The runtime must restore the +# terminal and exit 130 — not keep running with a restored/broken terminal. + +ptyunit_test_begin "shell-runtime #43: Ctrl-C mid-loop exits 130" +# --expect holds the keystroke until the list has actually rendered, so the +# \x03 cannot land in the PTY input buffer before the fixture's stty/trap +# setup completes (race observed under parallel-suite load). +_irc=0 +_out=$( _pty "--expect" "apple" $'\x03' ) || _irc=$? +assert_eq "130" "$_irc" +assert_not_null "$_out" + +# ── #44: stdin EOF quits the runtime instead of busy-spinning ──────────────── + +ptyunit_test_begin "shell-runtime #44: stdin EOF exits promptly with rc=1" +_eof_rc=0 +_eout=$( python3 "$PTY_RUN" "$SHELLFRAME_DIR/tests/fixtures/shell-eof.sh" ) || _eof_rc=$? +assert_contains "$_eout" "shell-returned:1" +assert_eq "0" "$_eof_rc" +ptyunit_test_summary + +# ── #44 follow-up (review): idle survival then clean EOF quit ──────────────── + +ptyunit_test_begin "shell-runtime #44b: survives idle ticks, quits on later EOF" +_ie_rc=0 +_ieout=$( python3 "$PTY_RUN" "$SHELLFRAME_DIR/tests/fixtures/shell-idle-eof.sh" ) || _ie_rc=$? +assert_contains "$_ieout" "idle-shell-returned:1" +assert_eq "0" "$_ie_rc" ptyunit_test_summary diff --git a/tests/unit/test-action-list.sh b/tests/unit/test-action-list.sh index 42e6bd2..fda494c 100644 --- a/tests/unit/test-action-list.sh +++ b/tests/unit/test-action-list.sh @@ -132,4 +132,26 @@ SHELLFRAME_AL_SELECTED=0 _out=$(_shellframe_al_default_draw_row 0 "apple" "eat skip" 0 "") assert_contains "$_out" "[eat]" "action index 0 shows eat" +# ── #42: %b escape-injection regression ───────────────────────────────────── +# A consumer may define color constants with literal backslash notation +# (SHELLFRAME_BOLD='\033[1m'). Row rendering must pass strings through %s so +# backslashes stay literal — never re-interpreted via %b. + +_sf_al_saved_bold="$SHELLFRAME_BOLD" +_sf_al_saved_reset="$SHELLFRAME_RESET" +SHELLFRAME_BOLD='\033[1m' +SHELLFRAME_RESET='\033[0m' + +ptyunit_test_begin "al draw_row #42: literal-backslash colors render literally" +_out=$(_shellframe_al_default_draw_row 0 "apple" "eat skip" 0 "") +assert_contains "$_out" '\033[1m' "literal backslash sequence preserved" +if [[ "$_out" != *$'\x1b'* ]]; then + ptyunit_pass +else + ptyunit_fail "real ESC byte injected into output (escape expansion)" +fi + +SHELLFRAME_BOLD="$_sf_al_saved_bold" +SHELLFRAME_RESET="$_sf_al_saved_reset" + ptyunit_test_summary diff --git a/tests/unit/test-app.sh b/tests/unit/test-app.sh index 2a2fa4c..aa2fb44 100644 --- a/tests/unit/test-app.sh +++ b/tests/unit/test-app.sh @@ -82,4 +82,57 @@ assert_called_times "shellframe_confirm" 1 # unregistered names, so assert_not_called works correctly without a mock. assert_not_called "shellframe_alert" +# ── #41: missing screen/handler guards ─────────────────────────────────────── +# shellframe_app must diagnose and exit non-zero instead of looping forever. +# Each case runs under a watchdog: pre-fix behavior (command-not-found loop) +# is killed after 3s and reports rc=137, which fails the exact rc assertion. + +# Run shellframe_app in a background subshell with a kill watchdog. +# Sets __appbg_output, __appbg_rc. +__appbg_file=$(mktemp "${TMPDIR:-/tmp}/sf-app-test.XXXXXX") +_run_app_with_watchdog() { + __appbg_output="" + __appbg_rc=0 + ( "$@" ) > "$__appbg_file" 2>&1 & + local _pid=$! + ( sleep 3; kill -9 "$_pid" 2>/dev/null ) >/dev/null 2>&1 & + local _wd=$! + wait "$_pid"; __appbg_rc=$? + kill "$_wd" 2>/dev/null + wait "$_wd" 2>/dev/null + __appbg_output=$(< "$__appbg_file") +} + +ptyunit_test_begin "app #41: transition to nonexistent screen → diagnostic + exit 1" +ptyunit_mock shellframe_alert --exit 0 +_appB_ROOT_type() { printf 'alert'; } +_appB_ROOT_render() { _SHELLFRAME_APP_TITLE="x"; } +_appB_ROOT_dismiss() { _SHELLFRAME_APP_NEXT="NOWHERE"; } # screen has no _type fn +_run_app_with_watchdog shellframe_app "_appB" "ROOT" +assert_eq "1" "$__appbg_rc" +assert_contains "$__appbg_output" "unknown screen" + +ptyunit_test_begin "app #41: handler does not set _SHELLFRAME_APP_NEXT → diagnostic + exit 1" +_appC_ROOT_type() { printf 'alert'; } +_appC_ROOT_render() { _SHELLFRAME_APP_TITLE="x"; } +_appC_ROOT_dismiss(){ :; } # forgets NEXT +_run_app_with_watchdog shellframe_app "_appC" "ROOT" +assert_eq "1" "$__appbg_rc" +assert_contains "$__appbg_output" "_SHELLFRAME_APP_NEXT" + +ptyunit_test_begin "app #41: missing event handler function → diagnostic + exit 1" +_appD_ROOT_type() { printf 'confirm'; } +_appD_ROOT_render() { _SHELLFRAME_APP_QUESTION="?"; } +ptyunit_mock shellframe_confirm --exit 1 # rc=1 → event 'no', but _appD_ROOT_no undefined +_run_app_with_watchdog shellframe_app "_appD" "ROOT" +assert_eq "1" "$__appbg_rc" +assert_contains "$__appbg_output" "handler" + +ptyunit_test_begin "app #41: empty initial screen name → diagnostic + exit 1" +_run_app_with_watchdog shellframe_app "_appE" "" +assert_eq "1" "$__appbg_rc" +assert_contains "$__appbg_output" "unknown screen" + ptyunit_test_summary + +rm -f "$__appbg_file" diff --git a/tests/unit/test-table.sh b/tests/unit/test-table.sh index fb93165..522aed9 100644 --- a/tests/unit/test-table.sh +++ b/tests/unit/test-table.sh @@ -179,4 +179,25 @@ SHELLFRAME_TBL_SELECTED=0 _out=$(_shellframe_tbl_default_draw_row 0 "apple" "eat skip" 0 "") assert_contains "$_out" "[eat]" "action index 0 shows eat" +# ── #42: %b escape-injection regression ───────────────────────────────────── +# See test-action-list.sh for rationale: color constants defined with literal +# backslash notation must survive row rendering unexpanded (%s, never %b). + +_sf_tbl_saved_bold="$SHELLFRAME_BOLD" +_sf_tbl_saved_reset="$SHELLFRAME_RESET" +SHELLFRAME_BOLD='\033[1m' +SHELLFRAME_RESET='\033[0m' + +ptyunit_test_begin "tbl draw_row #42: literal-backslash colors render literally" +_out=$(_shellframe_tbl_default_draw_row 0 "apple" "eat skip" 0 "") +assert_contains "$_out" '\033[1m' "literal backslash sequence preserved" +if [[ "$_out" != *$'\x1b'* ]]; then + ptyunit_pass +else + ptyunit_fail "real ESC byte injected into output (escape expansion)" +fi + +SHELLFRAME_BOLD="$_sf_tbl_saved_bold" +SHELLFRAME_RESET="$_sf_tbl_saved_reset" + ptyunit_test_summary