From 8c391bd158b94cfc459214d7bc7e090e1c26bc3a Mon Sep 17 00:00:00 2001 From: McAmner Date: Sat, 8 Aug 2026 14:26:02 +0200 Subject: [PATCH 1/4] feat(ui): give the shell surface a progress spinner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mq-agent has wrapped slow work in console.status since it grew rich panels; the shell half of the stack had no progress primitive at all. A grep across all 213 shell files found two hits, both false: ASCII art containing the word SPINNER, and watch.sh hiding the cursor. So 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. ui_spinner wraps a command and animates while it works. It gates on /dev/tty rather than on stdout being a terminal, which is what makes the common shell shape work: repos="$(ui_spinner 'Fetching' gh repo list)" both animates and captures, because frames never touch stdout. The cursor is deliberately left visible. 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. Clobbering that trades a cosmetic win for a real one. Two limits follow from backgrounding the wrapped command, and both are documented: it cannot wrap something that prompts on stdin, and variables it sets do not reach the caller. Wired into the GitHub repo picker as the first call site, which also fixes a silent failure there: an empty gh result used to fall through to fzf with nothing to pick from. Co-Authored-By: Claude Opus 5 --- mqlaunch/lib/repo-picker.sh | 18 +++++- tests/manifest.tsv | 1 + tests/ui-spinner-smoke.sh | 113 ++++++++++++++++++++++++++++++++++++ tools/scripts/test-all.sh | 1 + ui/terminal-ui/mq-ui.sh | 84 +++++++++++++++++++++++++++ 5 files changed, 215 insertions(+), 2 deletions(-) create mode 100755 tests/ui-spinner-smoke.sh 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/tests/manifest.tsv b/tests/manifest.tsv index 550e0b1b..65770e98 100644 --- a/tests/manifest.tsv +++ b/tests/manifest.tsv @@ -95,3 +95,4 @@ 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 - diff --git a/tests/ui-spinner-smoke.sh b/tests/ui-spinner-smoke.sh new file mode 100755 index 00000000..84778441 --- /dev/null +++ b/tests/ui-spinner-smoke.sh @@ -0,0 +1,113 @@ +#!/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/8] the helper exists" +test -f "$UI" +grep -q "^ui_spinner()" "$UI" + +echo "[2/8] 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/8] 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/8] 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/8] MQ_NO_SPINNER=1 disables animation even on a terminal" +python3 - "$UI" <<'PY' +import os, pty, sys + +ui = sys.argv[1] +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/8] 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] +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/8] 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 "[8/8] 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..9b5f02dc 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" 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" +} From 3667669250fa20312c7be5634c42a6d22138534e Mon Sep 17 00:00:00 2001 From: McAmner Date: Sat, 8 Aug 2026 15:23:40 +0200 Subject: [PATCH 2/4] fix(mqobsidian): make the manifest reader survive zsh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Menu option 3 (open the roadmap doc) failed with three errors that looked like three problems and were two, both of them zsh: get_mqobsidian_manifest_path:2: BASH_SOURCE[0]: parameter not set resolve_view_relative_path:3: command not found: jq [mqobsidian][error] Requested view key is not defined in views.json: roadmap-doc bin/mqlaunch is bash, so command mode resolved the manifest fine; the interactive menu runs terminal/launchers/mqlaunch.sh, which is zsh, and zsh has no BASH_SOURCE. Reading it inside a function made the split invisible until someone used the menu. Resolved at source time now — inside a zsh function even $0 is no help, it holds the function name. The jq error was not a PATH problem on the machine. assert_view_target_exists declared `local path`, and in zsh $path is a special array tied to $PATH, so PATH was empty for its whole call tree, including the jq that reads the manifest. Renamed to `target`. doctor_mqobsidian_views had both traps on one line (`local root key rel type path status=0`) — $status is read-only in zsh — so the doctor worked from bash command mode and died from the menu. Neither fault mentioned itself: the operator was told the view key was undefined. It is defined. jq is now checked up front and named. Co-Authored-By: Claude Opus 5 --- mqlaunch/lib/mqobsidian/doctor.sh | 13 +-- mqlaunch/lib/mqobsidian/manifest.sh | 42 +++++++-- mqlaunch/lib/mqobsidian/open.sh | 32 ++++--- tests/manifest.tsv | 1 + .../mqobsidian-manifest-shell-parity-smoke.sh | 90 +++++++++++++++++++ tools/scripts/test-all.sh | 1 + 6 files changed, 153 insertions(+), 26 deletions(-) create mode 100755 tests/mqobsidian-manifest-shell-parity-smoke.sh 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/tests/manifest.tsv b/tests/manifest.tsv index 65770e98..aa967ef1 100644 --- a/tests/manifest.tsv +++ b/tests/manifest.tsv @@ -96,3 +96,4 @@ 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/tools/scripts/test-all.sh b/tools/scripts/test-all.sh index 9b5f02dc..97085940 100755 --- a/tools/scripts/test-all.sh +++ b/tools/scripts/test-all.sh @@ -62,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" From 1ae93ef1732ae44b7cd35b350e4cba5195451ec3 Mon Sep 17 00:00:00 2001 From: McAmner Date: Sat, 8 Aug 2026 15:32:36 +0200 Subject: [PATCH 3/4] fix(dev-menu): close the last unguarded BASH_SOURCE read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dev_repo_path's 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. Latent rather than live, because the branch only runs with BASE_DIR unset and the launcher always sets it; that is a reason to guard it, not to leave it. Resolved at source time like the other two call sites, and the fallback now returns non-zero instead of silently handing back a relative path if even that fails. The dev-menu smoke test asserts it under both shells with BASE_DIR unset, which is the only way the branch is reachable at all. Co-Authored-By: Claude Opus 5 --- terminal/menus/mq-dev-menu.sh | 16 +++++++++++++++- tests/dev-menu-smoke.sh | 31 +++++++++++++++++++++++++------ 2 files changed, 40 insertions(+), 7 deletions(-) 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" From bf27bb97998357dc466e3bd35fa0a7f6f8d96c0a Mon Sep 17 00:00:00 2001 From: McAmner Date: Sat, 8 Aug 2026 15:36:57 +0200 Subject: [PATCH 4/4] test(ui): assert the spinner switches against the switch CI sets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pty step failed in CI and passed locally, because quality.yml runs the whole suite with MQ_NO_TUI=1 and ui_spinner honours it — so the step that exists to prove frames reach a terminal was asserting them in a run where drawing is deliberately off. The helper was right; the test's premise was not. Steps 5 and 6 now clear the headless switches they are not testing, and a new step 7 pins the behaviour that caused the confusion: MQ_NO_TUI=1 suppresses the spinner. That is a real contract — it is what lets the suite run headless — and it had no coverage. Verified both ways: MQ_NO_TUI=1 ./tools/scripts/test-all.sh (the CI invocation) and a plain local run. Co-Authored-By: Claude Opus 5 --- tests/ui-spinner-smoke.sh | 45 ++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/tests/ui-spinner-smoke.sh b/tests/ui-spinner-smoke.sh index 84778441..efa05ed7 100755 --- a/tests/ui-spinner-smoke.sh +++ b/tests/ui-spinner-smoke.sh @@ -22,31 +22,35 @@ UI="$ROOT/ui/terminal-ui/mq-ui.sh" echo "SMOKE: ui_spinner progress helper" -echo "[1/8] the helper exists" +echo "[1/9] the helper exists" test -f "$UI" grep -q "^ui_spinner()" "$UI" -echo "[2/8] a successful command exits 0 and its stdout passes through" +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/8] a failing command's exit status is the helper's exit status" +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/8] headless output is byte-identical to running the command directly" +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/8] MQ_NO_SPINNER=1 disables animation even on a terminal" +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() @@ -62,7 +66,7 @@ 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/8] on a real terminal a human sees frames, and they are cleaned up" +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. @@ -72,6 +76,10 @@ 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() @@ -95,11 +103,32 @@ 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/8] a wrapped command that writes to stderr keeps writing to stderr" +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 "[8/8] it runs under zsh, which is the shell the menus actually use" +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.