From 73b40a7665b6da06b49d13c1e071d2adce3b52f7 Mon Sep 17 00:00:00 2001 From: satyaborg Date: Sat, 30 May 2026 15:04:07 +1000 Subject: [PATCH] fix: improve tui back navigation --- README.md | 2 +- devloop | 167 ++++++++++++++++++++++++++++++++++-------- tests/devloop_test.sh | 14 +++- 3 files changed, 148 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index f568db0..092e76b 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ When stdout is a terminal, running `devloop` without arguments opens a menu: - `Settings`: view spec search paths, add or remove one custom spec path, and set the run timeout. - `Doctor`: verify required commands, optional UI tools, and installed skills. -Nested menu screens keep `Back` as the final option so you can return to the previous menu without exiting Devloop. +Nested menu screens keep `Back` as the final option, and Esc/cancel also returns to the previous menu without exiting Devloop. Interactive screens redraw in place instead of appending a fresh UI after each selection. `gum` powers the branded help screen, prompts, confirmations, status output, paging, and setup screens. `fzf` powers searchable pickers for specs, tracks, and reports. diff --git a/devloop b/devloop index 3c81367..ed0bef8 100755 --- a/devloop +++ b/devloop @@ -33,6 +33,7 @@ UI_OK_COLOR="141" UI_DIM_COLOR="244" UI_BORDER_COLOR="141" UI_BACK=false +UI_NOTICE="" EVENT_IDS=() EVENT_TITLES=() @@ -155,7 +156,6 @@ devloop_logo() { ░█░█░█▀▀░▀▄▀░█░░░█░█░█░█░█▀▀ ░▀▀░░▀▀▀░░▀░░▀▀▀░▀▀▀░▀▀▀░▀░░ EOF - printf 'v%s\n' "$DEVLOOP_VERSION" } welcome_plain() { @@ -623,6 +623,23 @@ ui_color() { fi } +ui_clear_screen() { + if [ "$USE_TUI" = true ] && [ -t 2 ]; then + printf '\033[2J\033[H' >&2 + fi +} + +ui_print_notice() { + local notice="$UI_NOTICE" + if [ -z "$notice" ]; then return 0; fi + UI_NOTICE="" + if ui_has_gum; then + gum style --foreground "$UI_DIM_COLOR" "$notice" >&2 + else + printf '%s\n' "$(ui_color dim "$notice")" >&2 + fi +} + ui_logo() { local target="${1:-stderr}" local logo @@ -645,6 +662,7 @@ ui_logo() { ui_header() { local title="$1" local subtitle="${2:-}" + ui_clear_screen if [ "$title" = "devloop" ]; then ui_logo stderr if [ -n "$subtitle" ]; then @@ -654,6 +672,7 @@ ui_header() { printf '%s\n' "$subtitle" >&2 fi fi + ui_print_notice return fi if ui_has_gum; then @@ -665,6 +684,7 @@ ui_header() { printf '\n%s\n' "$title" >&2 if [ -n "$subtitle" ]; then printf '%s\n' "$subtitle" >&2; fi fi + ui_print_notice } ui_print_key_values() { @@ -991,8 +1011,18 @@ interactive_menu() { if [ "$UI_BACK" = true ]; then continue; fi return "$code" ;; - "Continue a run") continue_command; return $? ;; - "Open reports") reports_command; return $? ;; + "Continue a run") + interactive_continue_run + code=$? + if [ "$UI_BACK" = true ]; then continue; fi + return "$code" + ;; + "Open reports") + interactive_open_report + code=$? + if [ "$UI_BACK" = true ]; then continue; fi + return "$code" + ;; "Settings") interactive_settings code=$? @@ -1006,27 +1036,46 @@ interactive_menu() { } interactive_run_spec() { - local spec - if ! spec="$(select_spec_file)"; then - printf 'no specs found under %s\n' "$(spec_search_label)" >&2 - if ui_confirm "Create a spec now?"; then - interactive_create_spec - return $? + local spec code message + spec="$(select_spec_file)" + code=$? + if [ "$code" -ne 0 ]; then + if [ "$code" -eq 130 ]; then + ui_go_back + return 0 fi + message="no specs found under $(spec_search_label)" + if [ "$USE_TUI" = true ]; then + printf '%s\n' "$message" >&2 + if ui_confirm "Create a spec now?"; then + interactive_create_spec + return $? + fi + UI_NOTICE="$message" + ui_go_back + return 0 + fi + printf '%s\n' "$message" >&2 return 1 fi interactive_run_setup "$spec" } interactive_create_spec() { - local agent context - agent="$(ui_choose "Spec agent" "Codex" "Claude Code" "Back")" || return 130 - if [ "$agent" = "Back" ]; then + local agent context code + agent="$(ui_choose "Spec agent" "Codex" "Claude Code" "Back")" + code=$? + if [ "$code" -ne 0 ] || [ "$agent" = "Back" ]; then ui_go_back return 0 fi agent="$(agent_choice_value "$agent")" - context="$(ui_input "Describe the change, or leave blank for interview mode" "")" || return 130 + context="$(ui_input "Describe the change, or leave blank for interview mode" "")" + code=$? + if [ "$code" -ne 0 ]; then + ui_go_back + return 0 + fi if [ -n "$context" ]; then spec_command --agent "$agent" "$context" else @@ -1035,7 +1084,7 @@ interactive_create_spec() { } interactive_settings() { - local choice custom_spec_dir custom_timeout scope saved value timeout_display + local choice custom_spec_dir custom_timeout scope saved value timeout_display code local choices=() while true; do custom_spec_dir="$(configured_spec_dir || true)" @@ -1057,36 +1106,41 @@ interactive_settings() { if [ -n "$custom_timeout" ]; then choices=("${choices[@]:0:${#choices[@]}-1}" "Remove timeout" "Back") fi - choice="$(ui_choose "Spec paths" "${choices[@]}")" || return 130 + choice="$(ui_choose "Spec paths" "${choices[@]}")" + code=$? + if [ "$code" -ne 0 ]; then + ui_go_back + return 0 + fi case "$choice" in "Add spec path") - value="$(ui_input "Spec path" "")" || return 130 + value="$(ui_input "Spec path" "")" || continue if saved="$(write_config_spec_dir local "$value")"; then - printf 'spec path saved: %s\n' "$saved" >&2 + UI_NOTICE="spec path saved: $saved" else - printf '%s\n' "spec path not saved" >&2 + UI_NOTICE="spec path not saved" fi ;; "Remove spec path") scope="$(configured_spec_dir_scope || true)" if [ -n "$scope" ]; then remove_config_spec_dir "$scope" - printf '%s\n' "spec path removed" >&2 + UI_NOTICE="spec path removed" fi ;; "Set timeout") - value="$(ui_input "Timeout minutes, 1-1440" "$(devloop_timeout_minutes)")" || return 130 + value="$(ui_input "Timeout minutes, 1-1440" "$(devloop_timeout_minutes)")" || continue if saved="$(write_config_timeout_minutes local "$value")"; then - printf 'timeout saved: %s minutes\n' "$saved" >&2 + UI_NOTICE="timeout saved: $saved minutes" else - printf '%s\n' "timeout not saved" >&2 + UI_NOTICE="timeout not saved" fi ;; "Remove timeout") scope="$(configured_timeout_minutes_scope || true)" if [ -n "$scope" ]; then remove_config_timeout_minutes "$scope" - printf '%s\n' "timeout removed" >&2 + UI_NOTICE="timeout removed" fi ;; "Back") ui_go_back; return 0 ;; @@ -1128,7 +1182,12 @@ interactive_run_setup() { "Toggle worktree mode" \ "Toggle PR creation" \ "Change report format" \ - "Back")" || return 130 + "Back")" + code=$? + if [ "$code" -ne 0 ]; then + ui_go_back + return 0 + fi case "$choice" in "Start run") if [ "$create_pr" = true ] && ! ui_confirm "Push the accepted branch and open a PR after acceptance?"; then @@ -1141,29 +1200,29 @@ interactive_run_setup() { return "$(final_exit_code "$code")" ;; "Change coder") - value="$(ui_choose "Implementation agent" "Codex" "Claude Code" "Back")" || return 130 + value="$(ui_choose "Implementation agent" "Codex" "Claude Code" "Back")" || continue [ "$value" = "Back" ] && continue coder="$(agent_choice_value "$value")" ;; "Change reviewer") - value="$(ui_choose "Review agent" "Claude Code" "Codex" "Back")" || return 130 + value="$(ui_choose "Review agent" "Claude Code" "Codex" "Back")" || continue [ "$value" = "Back" ] && continue reviewer="$(agent_choice_value "$value")" ;; "Change pass limit") - value="$(ui_input "Pass limit, 1-10" "$max")" || return 130 + value="$(ui_input "Pass limit, 1-10" "$max")" || continue if printf '%s\n' "$value" | grep -Eq '^[0-9]+$' && [ "$value" -ge 1 ] && [ "$value" -le 10 ]; then max="$value" else - printf '%s\n' "pass limit must be an integer between 1 and 10" >&2 + UI_NOTICE="pass limit must be an integer between 1 and 10" fi ;; "Change timeout") - value="$(ui_input "Timeout minutes, 1-1440" "$timeout_minutes")" || return 130 + value="$(ui_input "Timeout minutes, 1-1440" "$timeout_minutes")" || continue if timeout_minutes="$(normalize_timeout_minutes "$value")"; then : else - printf '%s\n' "timeout must be an integer between 1 and 1440 minutes" >&2 + UI_NOTICE="timeout must be an integer between 1 and 1440 minutes" timeout_minutes="$(devloop_timeout_minutes)" fi ;; @@ -1177,7 +1236,7 @@ interactive_run_setup() { if [ "$create_pr" = true ]; then create_pr=false; else create_pr=true; fi ;; "Change report format") - value="$(ui_choose "Report format" "html" "markdown" "Back")" || return 130 + value="$(ui_choose "Report format" "html" "markdown" "Back")" || continue [ "$value" = "Back" ] && continue report_format="$value" ;; @@ -1190,13 +1249,57 @@ select_spec_file() { local list selected list="$(mktemp "${TMPDIR:-/tmp}/devloop-specs.XXXXXX")" list_spec_files > "$list" + if [ ! -s "$list" ]; then + rm -f "$list" + return 1 + fi selected="$(ui_pick_from_file "$list" "Select spec")" local code=$? rm -f "$list" - if [ "$code" -ne 0 ] || [ -z "$selected" ]; then return 1; fi + if [ "$code" -ne 0 ] || [ -z "$selected" ]; then return 130; fi printf '%s\n' "$selected" } +interactive_open_report() { + local list selected code + list="$(mktemp "${TMPDIR:-/tmp}/devloop-reports.XXXXXX")" + list_artifact_files ".codex/reports" > "$list" + if [ ! -s "$list" ]; then + rm -f "$list" + UI_NOTICE="no reports found" + ui_go_back + return 0 + fi + selected="$(ui_pick_from_file "$list" "Open report")" + code=$? + rm -f "$list" + if [ "$code" -ne 0 ] || [ -z "$selected" ]; then + ui_go_back + return 0 + fi + view_file "$selected" +} + +interactive_continue_run() { + local list selected code + list="$(mktemp "${TMPDIR:-/tmp}/devloop-tracks.XXXXXX")" + list_artifact_files ".codex/tracks" > "$list" + if [ ! -s "$list" ]; then + rm -f "$list" + UI_NOTICE="no devloop tracks found" + ui_go_back + return 0 + fi + selected="$(ui_pick_from_file "$list" "Continue run")" + code=$? + rm -f "$list" + if [ "$code" -ne 0 ] || [ -z "$selected" ]; then + ui_go_back + return 0 + fi + run_from_track "$selected" +} + reports_command() { if [ "$#" -gt 0 ]; then printf '%s\n' "usage: devloop reports" >&2 diff --git a/tests/devloop_test.sh b/tests/devloop_test.sh index b338134..395978c 100755 --- a/tests/devloop_test.sh +++ b/tests/devloop_test.sh @@ -50,7 +50,6 @@ contains "$help" "--create-pr" "help" contains "$help" "--no-shell" "help" contains "$help" "--enter-worktree" "help" contains "$help" "--version" "help" -contains "$help" "v$version" "help" contains "$help" "--timeout-minutes" "help" ok "help output" @@ -373,7 +372,7 @@ session_output=$'unrelated 11111111-1111-4111-8111-111111111111\nTo continue thi equals "$(extract_session_id "$session_output")" "22222222-2222-4222-8222-222222222222" "extract_session_id uses session marker" contains "$(devloop_logo)" "░█▀▄░█▀▀" "devloop logo" -contains "$(devloop_logo)" "v$version" "devloop logo version" +if [[ "$(devloop_logo)" == *"v$version"* ]]; then fail "devloop logo included version"; fi ui_logo stdout >/dev/null equals "$(ui_color_code accent)" "38;5;141" "accent color" equals "$(ui_color_code rec)" "38;5;135" "run color" @@ -387,14 +386,25 @@ USE_TUI="$old_use_tui" if ! ( ui_choose() { printf '%s\n' "Back"; }; UI_BACK=false; interactive_create_spec >/dev/null 2>&1; [ "$UI_BACK" = true ] ); then fail "create spec back navigation"; fi if ! ( ui_choose() { printf '%s\n' "Back"; }; UI_BACK=false; interactive_settings >/dev/null 2>&1; [ "$UI_BACK" = true ] ); then fail "settings back navigation"; fi if ! ( ui_choose() { printf '%s\n' "Back"; }; UI_BACK=false; interactive_run_setup "spec.md" >/dev/null 2>&1; [ "$UI_BACK" = true ] ); then fail "run setup back navigation"; fi +if ! ( ui_choose() { return 130; }; UI_BACK=false; interactive_create_spec >/dev/null 2>&1; [ "$UI_BACK" = true ] ); then fail "create spec escape navigation"; fi +if ! ( ui_choose() { return 130; }; UI_BACK=false; interactive_run_setup "spec.md" >/dev/null 2>&1; [ "$UI_BACK" = true ] ); then fail "run setup escape navigation"; fi if ! ( ui_choose() { printf '%s\n' "Quit"; }; UI_BACK=false; interactive_menu >/dev/null 2>&1 ); then fail "menu quit failed"; fi empty_spec_repo="$work/empty-spec-repo" mkdir -p "$empty_spec_repo" old_use_tui="$USE_TUI" USE_TUI=false if ( cd "$empty_spec_repo" && interactive_run_spec >/dev/null 2>&1 ); then fail "interactive_run_spec accepted missing specs"; fi +USE_TUI=true +if ! ( cd "$empty_spec_repo" && UI_BACK=false; interactive_run_spec >/dev/null 2>&1; [ "$UI_BACK" = true ] ); then fail "interactive_run_spec missing specs did not go back"; fi +if ! ( cd "$empty_spec_repo" && UI_BACK=false; interactive_continue_run >/dev/null 2>&1; [ "$UI_BACK" = true ] ); then fail "interactive_continue_run missing tracks did not go back"; fi +if ! ( cd "$empty_spec_repo" && UI_BACK=false; interactive_open_report >/dev/null 2>&1; [ "$UI_BACK" = true ] ); then fail "interactive_open_report missing reports did not go back"; fi USE_TUI="$old_use_tui" +cancel_spec_repo="$work/cancel-spec-repo" +mkdir -p "$cancel_spec_repo/.specs" +printf '%s\n' "# Cancel" > "$cancel_spec_repo/.specs/cancel.md" +if ! ( cd "$cancel_spec_repo" && ui_pick_from_file() { return 130; }; UI_BACK=false; interactive_run_spec >/dev/null 2>&1; [ "$UI_BACK" = true ] ); then fail "interactive_run_spec escape navigation"; fi + picker_file="$work/picker.txt" printf '%s\n' "alpha" "beta" > "$picker_file" old_use_tui="$USE_TUI"