diff --git a/mqlaunch/lib/mqobsidian/doctor.sh b/mqlaunch/lib/mqobsidian/doctor.sh index 778acbd3..9f98c4f0 100644 --- a/mqlaunch/lib/mqobsidian/doctor.sh +++ b/mqlaunch/lib/mqobsidian/doctor.sh @@ -47,20 +47,23 @@ doctor_mqobsidian_manifest() { # Coordinates doctor mqobsidian views behavior. doctor_mqobsidian_views() { - local root key rel type path status=0 + # Both renames are zsh survival, not style: $path is tied to $PATH, and + # $status is read-only. This line used to declare locals for both, so the + # doctor worked from bash command mode and died from the zsh menu. + local root key rel type target rc=0 root="$(resolve_mqobsidian_dir)" while IFS= read -r key; do rel="$(resolve_view_relative_path "$key" 2>/dev/null)" type="$(resolve_view_type "$key" 2>/dev/null)" - path="$root/$rel" - if { [[ "$type" == "folder" && -d "$path" ]] || [[ "$type" == "file" && -f "$path" ]]; }; then + target="$root/$rel" + if { [[ "$type" == "folder" && -d "$target" ]] || [[ "$type" == "file" && -f "$target" ]]; }; then _doc_ok "view $key -> $rel" else _doc_missing "view $key -> $rel" - status=1 + rc=1 fi done < <(list_supported_views) - return $status + return $rc } # Coordinates doctor mqobsidian open command behavior. diff --git a/mqlaunch/lib/mqobsidian/manifest.sh b/mqlaunch/lib/mqobsidian/manifest.sh index 6e308fa7..befadd48 100644 --- a/mqlaunch/lib/mqobsidian/manifest.sh +++ b/mqlaunch/lib/mqobsidian/manifest.sh @@ -3,24 +3,50 @@ # read-only. The manifest is the single source for supported views. Depends on # errors.sh. +# Resolved here, at source time, and not inside the function below. +# +# bin/mqlaunch is bash but the interactive launcher is zsh, and zsh has no +# BASH_SOURCE — so reading it per call made command mode work and the menu +# fail. Source time is the only moment either shell can still say where this +# file lives: inside a zsh function $0 holds the function name, not the path. +# Same idiom as ui/terminal-ui/mq-ui.sh; `-` rather than `:-` so it survives a +# caller running under `set -u`. +_mqobs_manifest_self="${BASH_SOURCE[0]-}" +[ -n "$_mqobs_manifest_self" ] || _mqobs_manifest_self="$0" +_MQOBS_MANIFEST_DIR="$(cd "$(dirname "$_mqobs_manifest_self")/../../config/mqobsidian" 2>/dev/null && pwd)" +unset _mqobs_manifest_self + # Gets mqobsidian manifest path. get_mqobsidian_manifest_path() { - local dir - dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../config/mqobsidian" && pwd)" - printf '%s\n' "$dir/views.json" + if [[ -z "${_MQOBS_MANIFEST_DIR:-}" ]]; then + mqobsidian_error "Manifest directory not found: expected mqlaunch/config/mqobsidian next to the consumer lib" + return 1 + fi + printf '%s\n' "$_MQOBS_MANIFEST_DIR/views.json" +} + +# jq reads the manifest, so a missing jq means no view resolves at all. Without +# this the failure surfaced two steps later as "view key is not defined", which +# sends the operator to inspect views.json instead of their PATH. +_mqobs_require_jq() { + command -v jq >/dev/null 2>&1 && return 0 + mqobsidian_error "jq is required to read views.json. Install: brew install jq" + return 1 } # Coordinates list supported views behavior. list_supported_views() { local mf - mf="$(get_mqobsidian_manifest_path)" + _mqobs_require_jq || return 1 + mf="$(get_mqobsidian_manifest_path)" || return 1 jq -r '.[].key' "$mf" } # Resolves view relative path. resolve_view_relative_path() { local key="$1" mf out - mf="$(get_mqobsidian_manifest_path)" + _mqobs_require_jq || return 1 + mf="$(get_mqobsidian_manifest_path)" || return 1 out="$(jq -r --arg k "$key" '.[] | select(.key==$k) | .relative_path' "$mf")" if [[ -z "$out" ]]; then mqobsidian_error "Requested view key is not defined in views.json: $key" @@ -32,7 +58,8 @@ resolve_view_relative_path() { # Resolves view type. resolve_view_type() { local key="$1" mf out - mf="$(get_mqobsidian_manifest_path)" + _mqobs_require_jq || return 1 + mf="$(get_mqobsidian_manifest_path)" || return 1 out="$(jq -r --arg k "$key" '.[] | select(.key==$k) | .type' "$mf")" if [[ -z "$out" ]]; then mqobsidian_error "Requested view key is not defined in views.json: $key" @@ -44,6 +71,7 @@ resolve_view_type() { # Resolves view label. resolve_view_label() { local key="$1" mf - mf="$(get_mqobsidian_manifest_path)" + _mqobs_require_jq || return 1 + mf="$(get_mqobsidian_manifest_path)" || return 1 jq -r --arg k "$key" '.[] | select(.key==$k) | .label' "$mf" } diff --git a/mqlaunch/lib/mqobsidian/open.sh b/mqlaunch/lib/mqobsidian/open.sh index 50956cc0..5e570402 100644 --- a/mqlaunch/lib/mqobsidian/open.sh +++ b/mqlaunch/lib/mqobsidian/open.sh @@ -10,33 +10,37 @@ build_view_absolute_path() { printf '%s/%s\n' "$root" "$rel" } -# Coordinates assert view target exists behavior. +# `target`, never `path`: in zsh $path is a special array tied to $PATH, so a +# `local path` blanks PATH for the whole call tree. That is what made menu +# option 3 report "command not found: jq" on a machine with jq installed — +# resolve_view_relative_path ran inside a function that had shadowed PATH. +# Same family as the read-only $status trap. assert_view_target_exists() { - local key="$1" path type - path="$(build_view_absolute_path "$key")" || return 1 + local key="$1" target type + target="$(build_view_absolute_path "$key")" || return 1 type="$(resolve_view_type "$key")" || return 1 - if [[ "$type" == "folder" && ! -d "$path" ]]; then - mqobsidian_error "Target path from manifest does not exist (folder): $path" + if [[ "$type" == "folder" && ! -d "$target" ]]; then + mqobsidian_error "Target path from manifest does not exist (folder): $target" return 1 fi - if [[ "$type" == "file" && ! -f "$path" ]]; then - mqobsidian_error "Target path from manifest does not exist (file): $path" + if [[ "$type" == "file" && ! -f "$target" ]]; then + mqobsidian_error "Target path from manifest does not exist (file): $target" return 1 fi - printf '%s\n' "$path" + printf '%s\n' "$target" } # The single place that invokes the OS opener. Override MQOBS_OPENER (e.g. to # `echo`) for tests, or to route to an editor later. open_mqobsidian_path() { - local path="$1" - "${MQOBS_OPENER:-open}" "$path" + local target="$1" + "${MQOBS_OPENER:-open}" "$target" } # Opens mqobsidian target. open_mqobsidian_target() { - local key="$1" path - path="$(assert_view_target_exists "$key")" || return 1 - mqobsidian_info "Opening $key → $path" - open_mqobsidian_path "$path" + local key="$1" target + target="$(assert_view_target_exists "$key")" || return 1 + mqobsidian_info "Opening $key → $target" + open_mqobsidian_path "$target" } diff --git a/mqlaunch/lib/repo-picker.sh b/mqlaunch/lib/repo-picker.sh index a0ec67ff..6617310e 100755 --- a/mqlaunch/lib/repo-picker.sh +++ b/mqlaunch/lib/repo-picker.sh @@ -50,11 +50,25 @@ run_github_repo_picker() { row_bold "GITHUB REPO PICKER" empty_row - row "Hämtar dina repos från GitHub..." print_footer + # The gh call is about three quarters of a second of nothing before fzf takes + # over the screen, and the old static "Hämtar..." row could not tell a slow + # network from a hung one. Fetching first, behind ui_spinner, makes the wait + # legible. It has to be a fetch-then-pipe rather than wrapping the whole + # pipeline: fzf owns stdin, and ui_spinner backgrounds what it wraps. + local repos="" + repos="$(ui_spinner "Hämtar dina repos från GitHub" \ + "$gh_bin" repo list --limit 1000 --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null)" || repos="" + + if [[ -z "$repos" ]]; then + ui_err "Kunde inte hämta repos från GitHub. Kontrollera gh auth status." + pause_enter + return 1 + fi + selected="$( - "$gh_bin" repo list --limit 1000 --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null \ + printf '%s\n' "$repos" \ | "$fzf_bin" \ --reverse \ --border \ diff --git a/terminal/menus/mq-dev-menu.sh b/terminal/menus/mq-dev-menu.sh index dca28087..59c9469e 100755 --- a/terminal/menus/mq-dev-menu.sh +++ b/terminal/menus/mq-dev-menu.sh @@ -1,5 +1,15 @@ #!/usr/bin/env bash +# Resolved at source time because dev_repo_path's fallback needs it and cannot +# recompute it: BASH_SOURCE is unset under zsh (the interactive launcher), and +# inside a zsh function $0 holds the function name, not the file. `-` rather +# than `:-` so it survives a caller running under `set -u`. Same idiom as +# ui/terminal-ui/mq-ui.sh and mqlaunch/lib/mqobsidian/manifest.sh. +_mq_dev_menu_self="${BASH_SOURCE[0]-}" +[ -n "$_mq_dev_menu_self" ] || _mq_dev_menu_self="$0" +_MQ_DEV_MENU_DIR="$(cd "$(dirname "$_mq_dev_menu_self")" 2>/dev/null && pwd)" +unset _mq_dev_menu_self + # Runs a bundled dev script with a clear missing-file fallback. run_dev_script() { local label="$1" @@ -32,7 +42,11 @@ dev_repo_path() { return fi - script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + script_dir="${_MQ_DEV_MENU_DIR:-}" + if [[ -z "$script_dir" ]]; then + printf '%s\n' "$relative_path" + return 1 + fi repo_root="$(cd "$script_dir/../.." && pwd)" printf '%s/%s\n' "$repo_root" "$relative_path" } diff --git a/tests/dev-menu-smoke.sh b/tests/dev-menu-smoke.sh index fc3b5b3a..4b4d9a35 100755 --- a/tests/dev-menu-smoke.sh +++ b/tests/dev-menu-smoke.sh @@ -14,11 +14,11 @@ MENU="$ROOT/terminal/menus/mq-dev-menu.sh" echo "SMOKE: dev menu" -echo "[1/6] the menu file exists and parses" +echo "[1/7] the menu file exists and parses" test -f "$MENU" bash -n "$MENU" -echo "[2/6] every action reachable before the regrouping still has a route" +echo "[2/7] every action reachable before the regrouping still has a route" # Listed as the handler or script each choice must reach, not as menu text. # Anything dropped from the front menu has to reappear in a submenu; this is the # check that a regrouping did not quietly become a deletion. @@ -56,7 +56,7 @@ if [[ -n "$missing" ]]; then fi echo " ok: all 17 original actions still reachable" -echo "[3/6] the numbers printed are the numbers answered" +echo "[3/7] the numbers printed are the numbers answered" # Compares the two lists, so it fails on a gap, a duplicate, or an option with # no arm. # @@ -91,7 +91,7 @@ if sorted(printed) != answered: print(f" ok: 1-{len(printed)}, in order, each with an arm") PY -echo "[4/6] the front menu is within the operator-choice limit" +echo "[4/7] the front menu is within the operator-choice limit" # Counted from the numbered rows the panel prints, which is what the ROADMAP # limit is about — what an operator is asked to choose between on one screen. count="$(python3 - "$MENU" <<'PY' @@ -112,7 +112,7 @@ if (( count > 10 )); then fi echo " ok: $count numbered choices on the front menu" -echo "[5/6] each submenu answers every row it prints" +echo "[5/7] each submenu answers every row it prints" python3 - "$MENU" <<'PY' import re, sys @@ -136,7 +136,7 @@ sys.exit(1 if failed else 0) PY echo " ok: submenu rows and arms agree" -echo "[6/6] each grouped row actually opens its submenu" +echo "[6/7] each grouped row actually opens its submenu" # Steps 2-5 read the file. This runs the menu, because a case arm that names a # function proves nothing about whether the function opens. # @@ -174,4 +174,23 @@ for expected in Prompts Folders Menus; do done echo " ok: Prompts, Folders and Menus all open" +echo "[7/7] dev_repo_path resolves without BASE_DIR, under both shells" +# The fallback branch read ${BASH_SOURCE[0]} inside a function, which is unset +# under zsh — the same split that broke the mqobsidian manifest reader from the +# menu while command mode kept working. Only reachable with BASE_DIR unset, so +# assert it directly rather than trusting that the launcher always sets it. +for shell in bash zsh; do + out="$("$shell" -c " + set -u + unset BASE_DIR + source '$MENU' + dev_repo_path tools/scripts/lint.sh + " 2>&1)" + test "$out" = "$ROOT/tools/scripts/lint.sh" || { + echo "FAIL: $shell resolved dev_repo_path to: $out" >&2 + exit 1 + } +done +echo " ok: bash and zsh agree" + echo "OK: dev menu smoke test passed" diff --git a/tests/manifest.tsv b/tests/manifest.tsv index 550e0b1b..aa967ef1 100644 --- a/tests/manifest.tsv +++ b/tests/manifest.tsv @@ -95,3 +95,5 @@ pulse-cli-color-contract-smoke.sh active - operator-usage-message-smoke.sh active - menu-exit-contract-smoke.sh active - theme-manager-path-smoke.sh active - +ui-spinner-smoke.sh active - +mqobsidian-manifest-shell-parity-smoke.sh active - diff --git a/tests/mqobsidian-manifest-shell-parity-smoke.sh b/tests/mqobsidian-manifest-shell-parity-smoke.sh new file mode 100755 index 00000000..1bd6487b --- /dev/null +++ b/tests/mqobsidian-manifest-shell-parity-smoke.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +# The mqobsidian manifest reader has to work under both shells the launcher +# uses, and has to name the real problem when a dependency is missing. +# +# Two bugs shipped together in menu option 3 (open the roadmap doc): +# +# get_mqobsidian_manifest_path:2: BASH_SOURCE[0]: parameter not set +# get_mqobsidian_manifest_path:cd:2: no such file or directory: /../../config/mqobsidian +# resolve_view_relative_path:3: command not found: jq +# [mqobsidian][error] Requested view key is not defined in views.json: roadmap-doc +# +# The first is a shell split: bin/mqlaunch is bash, so command mode +# (`mqlaunch obsidian doctor`) resolved the manifest fine, while the +# interactive menu runs terminal/launchers/mqlaunch.sh, which is zsh — and +# zsh has no BASH_SOURCE. Sibling libs already handle this at source time; +# manifest.sh read it inside a function, where even zsh's $0 is no help +# because there it holds the function name. +# +# The second is the diagnosis: whatever went wrong upstream, the operator was +# told the view key was undefined. It is defined. Sending someone to inspect +# views.json when the actual fault is BASH_SOURCE or a missing jq is worse +# than saying nothing. +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +LIB="$ROOT/mqlaunch/lib/mqobsidian" +EXPECTED="$ROOT/mqlaunch/config/mqobsidian/views.json" + +echo "SMOKE: mqobsidian manifest reader, bash and zsh parity" + +echo "[1/8] libs and manifest exist" +test -f "$LIB/errors.sh" +test -f "$LIB/manifest.sh" +test -f "$EXPECTED" + +load="source '$LIB/errors.sh'; source '$LIB/manifest.sh'" + +echo "[2/8] bash resolves the manifest path" +got="$(bash -c "set -u; $load; get_mqobsidian_manifest_path")" +test "$got" = "$EXPECTED" + +echo "[3/8] zsh resolves the same path, with no unset-parameter error" +# `set -u` is what the launcher runs under (terminal/launchers/mqlaunch.sh:3). +out="$(zsh -c "set -u; $load; get_mqobsidian_manifest_path" 2>&1)" +test "$out" = "$EXPECTED" + +echo "[4/8] zsh resolves the view that option 3 opens" +out="$(zsh -c "set -u; $load; resolve_view_relative_path roadmap-doc" 2>&1)" +test "$out" = "docs/roadmap-token-reduction.md" + +echo "[5/8] zsh resolves type and label too" +out="$(zsh -c "set -u; $load; resolve_view_type roadmap-doc" 2>&1)" +test "$out" = "file" +out="$(zsh -c "set -u; $load; resolve_view_label roadmap-doc" 2>&1)" +test -n "$out" + +echo "[6/8] opening a view under zsh does not blank PATH" +# This is the fault behind "command not found: jq" on a machine that has jq. +# assert_view_target_exists declared `local path`, and in zsh $path is a +# special array tied to $PATH — so PATH was empty for everything it called, +# including the jq that reads the manifest. A fake vault keeps the assertion +# about the shell, not about what happens to be in the real one. +VAULT="$(mktemp -d)" +trap 'rm -rf "$VAULT"' EXIT +# systems/ and memory/ are what the resolver uses to recognise a vault. +mkdir -p "$VAULT/docs" "$VAULT/systems" "$VAULT/memory" +touch "$VAULT/docs/roadmap-token-reduction.md" + +open_load="source '$LIB/errors.sh'; source '$LIB/resolve.sh'; source '$LIB/manifest.sh'; source '$LIB/open.sh'" +out="$(zsh -c "set -u; export MQ_OBSIDIAN_DIR='$VAULT'; $open_load; assert_view_target_exists roadmap-doc" 2>&1)" +test "$out" = "$VAULT/docs/roadmap-token-reduction.md" + +# And no `local path` / `local status` may come back into the consumer lib. +! grep -qE '^[[:space:]]*local .*\b(path|status)\b' "$LIB"/*.sh + +echo "[7/8] the doctor runs under zsh, where \$status is read-only" +out="$(zsh -c "set -u; export MQ_OBSIDIAN_DIR='$VAULT'; $open_load; source '$LIB/doctor.sh'; doctor_mqobsidian_views" 2>&1 || true)" +! grep -qi "read-only variable" <<<"$out" +grep -q "view roadmap-doc" <<<"$out" + +echo "[8/8] a missing jq is reported as a missing jq" +# PATH is stripped after sourcing, so the source-time path resolution still +# has dirname; only the jq lookup fails. The old code blamed views.json. +rc=0 +out="$(zsh -c "set -u; $load; PATH=/nonexistent; resolve_view_relative_path roadmap-doc" 2>&1)" || rc=$? +test "$rc" -ne 0 +grep -q "jq" <<<"$out" +! grep -q "not defined in views.json" <<<"$out" + +echo "OK: mqobsidian manifest shell parity passed" diff --git a/tests/ui-spinner-smoke.sh b/tests/ui-spinner-smoke.sh new file mode 100755 index 00000000..efa05ed7 --- /dev/null +++ b/tests/ui-spinner-smoke.sh @@ -0,0 +1,142 @@ +#!/usr/bin/env bash +# Locks the contract of ui_spinner, the shell half of the progress surface. +# +# mq-agent has shown a spinner for slow work since it grew rich panels; the +# shell side had no progress primitive at all, so every menu that shelled out +# to gh, ollama or a test run simply went quiet. This adds one helper, and the +# risk of a helper that wraps arbitrary commands is that it quietly changes +# what those commands do. So the contract is exactly three promises: +# +# 1. the wrapped command's exit status is the helper's exit status +# 2. the wrapped command's stdout passes through untouched +# 3. nothing is drawn unless a human is watching a terminal +# +# (3) is the same rule plain-output-contract-smoke.sh enforces for the banner: +# frames and cursor codes must never reach a pipe. The pty step proves the +# other half — that a human *does* get frames — because a spinner that is +# merely safe is a spinner that never animates. +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +UI="$ROOT/ui/terminal-ui/mq-ui.sh" + +echo "SMOKE: ui_spinner progress helper" + +echo "[1/9] the helper exists" +test -f "$UI" +grep -q "^ui_spinner()" "$UI" + +echo "[2/9] a successful command exits 0 and its stdout passes through" +output="$(bash -c "source '$UI'; ui_spinner 'Working' printf 'hello\n'")" +test "$output" = "hello" + +echo "[3/9] a failing command's exit status is the helper's exit status" +rc=0 +bash -c "source '$UI'; ui_spinner 'Working' sh -c 'exit 7'" || rc=$? +test "$rc" -eq 7 + +echo "[4/9] headless output is byte-identical to running the command directly" +# No frames, no \r, no erase-line. A caller that pipes ui_spinner into a parser +# must not have to strip anything. +captured="$(bash -c "source '$UI'; ui_spinner 'Working' printf 'a\nb\n'" | od -c | head -3)" +direct="$(printf 'a\nb\n' | od -c | head -3)" +test "$captured" = "$direct" + +echo "[5/9] MQ_NO_SPINNER=1 disables animation even on a terminal" +python3 - "$UI" <<'PY' +import os, pty, sys + +ui = sys.argv[1] +# CI runs the whole suite with MQ_NO_TUI=1 (.github/workflows/quality.yml), +# which also suppresses the spinner. Drop it so this step proves MQ_NO_SPINNER +# and nothing else. +os.environ.pop("MQ_NO_TUI", None) +os.environ["MQ_NO_SPINNER"] = "1" +seen = bytearray() + + +def read(fd): + chunk = os.read(fd, 1024) + seen.extend(chunk) + return chunk + + +status = pty.spawn(["bash", "-c", f"source '{ui}'; ui_spinner 'Working' sleep 0.3"], read) +assert os.waitstatus_to_exitcode(status) == 0 +assert b"\xe2\xa0" not in seen, "braille frame leaked with MQ_NO_SPINNER=1" +PY + +echo "[6/9] on a real terminal a human sees frames, and they are cleaned up" +# The capture case is the one that matters: `out="$(ui_spinner … )"` is how a +# shell caller actually uses a slow command, and gating the animation on stdout +# being a terminal would silently disable the spinner in exactly that shape. +# Frames go to /dev/tty, so the capture stays clean while the human still sees +# something move. +python3 - "$UI" <<'PY' +import os, pty, sys + +ui = sys.argv[1] +# Same reason as step 5: this step is *about* the interactive path, so the +# headless switch CI sets globally has to come off first. +os.environ.pop("MQ_NO_TUI", None) +os.environ.pop("MQ_NO_SPINNER", None) +seen = bytearray() + + +def read(fd): + chunk = os.read(fd, 1024) + seen.extend(chunk) + return chunk + + +script = ( + f"source '{ui}'\n" + "ui_spinner 'Working' sleep 0.5\n" + "out=\"$(ui_spinner 'Capturing' printf 'captured\\n')\"\n" + "printf 'GOT:%s\\n' \"$out\"\n" +) +status = pty.spawn(["bash", "-c", script], read) +assert os.waitstatus_to_exitcode(status) == 0, "spinner changed the exit status" +# Braille frames are U+28xx, which is 0xE2 0xA0 in UTF-8. +assert b"\xe2\xa0" in seen, "no spinner frame reached the terminal" +assert b"\x1b[K" in seen, "the spinner line was never erased" +assert b"GOT:captured" in seen, "command substitution lost the wrapped output" +PY + +echo "[7/9] MQ_NO_TUI=1 suppresses the spinner, which is what CI relies on" +python3 - "$UI" <<'PYNOTUI' +import os, pty, sys + +ui = sys.argv[1] +os.environ.pop("MQ_NO_SPINNER", None) +os.environ["MQ_NO_TUI"] = "1" +seen = bytearray() + + +def read(fd): + chunk = os.read(fd, 1024) + seen.extend(chunk) + return chunk + + +status = pty.spawn(["bash", "-c", f"source '{ui}'; ui_spinner 'Working' sleep 0.3"], read) +assert os.waitstatus_to_exitcode(status) == 0 +assert b"\xe2\xa0" not in seen, "braille frame leaked with MQ_NO_TUI=1" +PYNOTUI + +echo "[8/9] a wrapped command that writes to stderr keeps writing to stderr" +err="$(bash -c "source '$UI'; ui_spinner 'Working' sh -c 'echo oops >&2'" 2>&1 >/dev/null)" +test "$err" = "oops" + +echo "[9/9] it runs under zsh, which is the shell the menus actually use" +# The regenerate-views fix was a bash-clean function that died under zsh on a +# read-only builtin. Background jobs plus `wait` are exactly the kind of +# construct that diverges between the two, so assert it here rather than assume. +output="$(zsh -c " + source '$UI' + ui_spinner 'Working' printf 'zsh-ok\n' +" 2>&1)" +grep -q "zsh-ok" <<<"$output" +! grep -qi "read-only variable\|parse error\|command not found" <<<"$output" + +echo "OK: ui_spinner smoke passed" diff --git a/tools/scripts/test-all.sh b/tools/scripts/test-all.sh index d870d1f7..97085940 100755 --- a/tools/scripts/test-all.sh +++ b/tools/scripts/test-all.sh @@ -23,6 +23,7 @@ echo "== Running mqlaunch headless checks ==" "$PROJECT_ROOT/tests/theme-lib-smoke.sh" "$PROJECT_ROOT/tests/panel-color-smoke.sh" "$PROJECT_ROOT/tests/prompt-lib-smoke.sh" +"$PROJECT_ROOT/tests/ui-spinner-smoke.sh" "$PROJECT_ROOT/tests/mq-stack-contract-smoke.sh" "$PROJECT_ROOT/tests/gitlaunch-menu-surface-smoke.sh" "$PROJECT_ROOT/tests/gitmerge-safe-smoke.sh" @@ -61,6 +62,7 @@ echo "== Running mqlaunch headless checks ==" "$PROJECT_ROOT/tests/mq-flow-routing-smoke.sh" "$PROJECT_ROOT/tests/mq-obsidian-menu-no-promotion-smoke.sh" "$PROJECT_ROOT/tests/mq-obsidian-python-smoke.sh" +"$PROJECT_ROOT/tests/mqobsidian-manifest-shell-parity-smoke.sh" "$PROJECT_ROOT/tests/mq-obsidian-regenerate-placeholder-ui-smoke.sh" "$PROJECT_ROOT/tests/mq-obsidian-regenerate-views-smoke.sh" "$PROJECT_ROOT/tests/mq-obsidian-triage-ui-smoke.sh" diff --git a/ui/terminal-ui/mq-ui.sh b/ui/terminal-ui/mq-ui.sh index a5ee3b1a..640aa79f 100644 --- a/ui/terminal-ui/mq-ui.sh +++ b/ui/terminal-ui/mq-ui.sh @@ -613,3 +613,87 @@ ui_err() { ui_info() { printf "%b%s%b\n" "$C_INFO" "$1" "$C_RESET" } + +# Braille frames, deliberately the same set rich uses on the mq-agent side, so +# `mqlaunch mq-agent 4` and a shell menu waiting on gh look like one tool. +MQ_SPINNER_FRAMES=(⠋ ⠙ ⠹ ⠸ ⠼ ⠴ ⠦ ⠧ ⠇ ⠏) + +# Runs a command and animates a spinner while it works. +# +# ui_spinner "Reading pull requests" gh pr list --state open +# +# The shell surface had no progress primitive at all: every menu that shelled +# out to gh (~0.75s per call), ollama (tens of seconds) or a test run simply +# went quiet, and a quiet terminal reads as a hang. mq-agent has had +# console.status for this since it grew rich panels; this is the same idea for +# the half of the stack written in shell. +# +# Three promises, locked by tests/ui-spinner-smoke.sh: +# +# * the wrapped command's exit status is returned unchanged, so this composes +# with the delegated-exit-code contract instead of fighting it +# * stdout and stderr pass through untouched — frames go to /dev/tty, never +# into a pipe (the same rule the banner follows, docs/RUNTIME_AUTHORITY.md), +# so `repos="$(ui_spinner 'Fetching' gh repo list)"` animates *and* captures +# * nothing is drawn unless there is a terminal to draw on +# +# Two limits come from running the command in the background, and both are +# deliberate: it cannot wrap something that prompts on stdin, and variables it +# sets do not reach the caller. Wrap the slow, silent, non-interactive part — +# that is the part you are waiting on anyway. +# +# The cursor is left visible on purpose. Hiding it means restoring it, and the +# only reliable restore is an INT/TERM trap — which gitlaunch and the git menu +# already use to put a repo back on its base branch. A spinner that clobbers +# that trap trades a cosmetic win for a real one, and a terminal left without a +# cursor is worse than one that kept it. +ui_spinner() { + local label="$1" + shift + + if [[ $# -eq 0 ]]; then + ui_err "ui_spinner: no command given" + return 2 + fi + + # What a spinner needs is a terminal to draw *on*, which is not the same as a + # terminal on stdout — the whole point is that `out="$(ui_spinner … )"` still + # animates while the caller keeps the output. So this gates on /dev/tty, not + # on mq_wants_plain_output. No controlling terminal (CI, a hook, a pipeline + # with no tty at all) means no audience, and then no animation. + if [[ -n "${MQ_NO_SPINNER:-}" || -n "${MQ_NO_TUI:-}" ]] || ! { true >/dev/tty; } 2>/dev/null; then + "$@" + return $? + fi + + # zsh in monitor mode announces every background job ("[1] 12345"). The menus + # run as scripts, where monitor is already off, but mq-ui.sh is sourceable. + if [[ -n "${ZSH_VERSION:-}" ]]; then + setopt local_options no_monitor + fi + + local width max_label + width="$(surface_terminal_width)" + max_label=$((width - 4)) + if ((max_label > 0)) && ((${#label} > max_label)); then + label="${label:0:$((max_label - 1))}…" + fi + + "$@" & + local pid=$! + local frame rc=0 + + while kill -0 "$pid" 2>/dev/null; do + for frame in "${MQ_SPINNER_FRAMES[@]}"; do + kill -0 "$pid" 2>/dev/null || break + printf '\r%b%s%b %s\033[K' "${C_INFO:-}" "$frame" "${C_RESET:-}" "$label" >/dev/tty + sleep "${MQ_SPINNER_INTERVAL:-0.08}" + done + done + + # `|| rc=$?` because a caller under `set -e` must not die on the wrapped + # command's failure before we have had a chance to clean the line. + wait "$pid" || rc=$? + printf '\r\033[K' >/dev/tty + return "$rc" +}