Skip to content

Commit dc00cfb

Browse files
MCamnerclaude
andauthored
fix(dispatch): propagate delegated exit codes (#75)
22 top-level branches invoked a script under $BASE_DIR and then ended in an unconditional `return 0`. A failing delegate was reported as success: mqlaunch repos LIST → argparse error on stderr, exit 0 mqlaunch skills no-such-subcommand → argparse error on stderr, exit 0 tests/delegated-exit-code-smoke.sh stubbed shell functions, which only reaches the agent and HAL families. Those propagate correctly; the branches delegating to external scripts were never covered, which is why this survived. Each branch now captures `command_status` immediately after the delegate and returns it — the idiom the `review` and `stack` routes already used. The capture sits before `pause_enter` on the routes that call it, so the pause cannot overwrite the status. Success paths are unchanged. Tests first, and they fail on main: two behavioural cases through the full launcher, plus a structural check that fails if any delegating branch reverts to an unconditional `return 0` — verified by planting one. Refs #74 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 6b11b1e commit dc00cfb

3 files changed

Lines changed: 143 additions & 27 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ All notable changes to this project will be documented in this file.
2424

2525
### Fixed
2626

27+
* 22 dispatch branches invoked a script under `$BASE_DIR` and then ended in an
28+
unconditional `return 0`, so a failing delegate was reported as success:
29+
`mqlaunch repos LIST` and `mqlaunch skills no-such-subcommand` both wrote a
30+
usage error to stderr and exited 0. Every delegating branch now captures
31+
`command_status` and returns it, matching the idiom the agent and HAL routes
32+
already used. `pause_enter` no longer overwrites the status on the routes that
33+
call it. `tests/delegated-exit-code-smoke.sh` gained the two behavioural cases
34+
and a structural check that fails if any delegating branch reverts to an
35+
unconditional `return 0`.
2736
* `mqlaunch` dispatched `memory` from two different branches. The
2837
`srm|memory|repo-memory` branch claims it first, so the `memory` listed in the
2938
brain-bridge branch below it could never match — `mqlaunch memory` has always

terminal/launchers/mqlaunch-command-mode.sh

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,8 @@ dispatch_cli_command() {
391391
"$BASE_DIR/automation/workflows/workspace.sh" "$sub" "${3:-}"
392392
;;
393393
esac
394-
return 0
394+
command_status=$?
395+
return "$command_status"
395396
;;
396397

397398
demo)
@@ -506,7 +507,8 @@ dispatch_cli_command() {
506507
ask|/ask)
507508
shift
508509
"$BASE_DIR/tools/scripts/ask.sh" "$@"
509-
return 0
510+
command_status=$?
511+
return "$command_status"
510512
;;
511513

512514
srm|memory|repo-memory)
@@ -534,49 +536,57 @@ dispatch_cli_command() {
534536
return "$command_status"
535537
fi
536538
"$BASE_DIR/tools/scripts/srm.sh" "$@"
537-
return 0
539+
command_status=$?
540+
return "$command_status"
538541
;;
539542

540543
skills|skill)
541544
shift
542545
"$BASE_DIR/tools/scripts/mq-skills.py" "$@"
546+
command_status=$?
543547
[[ "${1:-}" != "--json" ]] && pause_enter
544-
return 0
548+
return "$command_status"
545549
;;
546550

547551
repos)
548552
shift
549553
case "${1:-}" in
550554
""|menu|hub)
551555
"$BASE_DIR/bin/mqlaunch" hub
556+
command_status=$?
552557
;;
553558
list|roadmaps|skills|status|wiki-status|diff-summary)
554559
"$BASE_DIR/tools/scripts/mq-repos.py" "$@"
560+
command_status=$?
555561
pause_enter
556562
;;
557563
*)
558564
"$BASE_DIR/tools/scripts/mq-repos.py" "$@"
565+
command_status=$?
559566
pause_enter
560567
;;
561568
esac
562-
return 0
569+
return "$command_status"
563570
;;
564571

565572
fix|/fix)
566573
shift
567574
"$BASE_DIR/tools/scripts/fix.sh" "$@"
568-
return 0
575+
command_status=$?
576+
return "$command_status"
569577
;;
570578

571579
chat|/chat)
572580
"$BASE_DIR/tools/scripts/chat.sh"
573-
return 0
581+
command_status=$?
582+
return "$command_status"
574583
;;
575584

576585
release-check|/release-check|check-release)
577586
"$BASE_DIR/terminal/release/mq-release-check.sh" "${@:2}"
587+
command_status=$?
578588
pause_enter
579-
return 0
589+
return "$command_status"
580590
;;
581591

582592
review-brain|/review-brain)
@@ -623,14 +633,16 @@ dispatch_cli_command() {
623633

624634
doctor|/doctor)
625635
"$BASE_DIR/tools/scripts/doctor.sh" "${@:2}"
636+
command_status=$?
626637
[[ "${2:-}" != "--json" && -z "${MQ_NO_TUI:-}" ]] && pause_enter
627-
return 0
638+
return "$command_status"
628639
;;
629640

630641
scan|/scan)
631642
"$BASE_DIR/tools/scripts/scan.sh"
643+
command_status=$?
632644
pause_enter
633-
return 0
645+
return "$command_status"
634646
;;
635647

636648
atlas|/atlas)
@@ -676,7 +688,8 @@ dispatch_cli_command() {
676688
return 2
677689
;;
678690
esac
679-
return 0
691+
command_status=$?
692+
return "$command_status"
680693
;;
681694

682695
git)
@@ -710,7 +723,8 @@ dispatch_cli_command() {
710723
return 2
711724
;;
712725
esac
713-
return 0
726+
command_status=$?
727+
return "$command_status"
714728
;;
715729

716730
dev)
@@ -858,17 +872,20 @@ dispatch_cli_command() {
858872
else
859873
"$BASE_DIR/tools/scripts/hal-terminal-guide.sh"
860874
fi
861-
return 0
875+
command_status=$?
876+
return "$command_status"
862877
;;
863878

864879
docfunc|document-functions)
865880
MQ_WORK_DIR="$PWD" "$BASE_DIR/terminal/menus/mq-tools-menu.sh" docfunc
866-
return 0
881+
command_status=$?
882+
return "$command_status"
867883
;;
868884

869885
docwrite|document-functions-write|update-comments)
870886
MQ_WORK_DIR="$PWD" "$BASE_DIR/terminal/menus/mq-tools-menu.sh" docwrite
871-
return 0
887+
command_status=$?
888+
return "$command_status"
872889
;;
873890

874891
version)
@@ -982,37 +999,44 @@ dispatch_cli_command() {
982999
else
9831000
"$BASE_DIR/bin/mqlaunch" hub
9841001
fi
985-
return 0
1002+
command_status=$?
1003+
return "$command_status"
9861004
;;
9871005

9881006
git-log|gitlog|glog)
9891007
declare -f fzf_git_log >/dev/null && fzf_git_log || "$BASE_DIR/bin/mqlaunch" git-log
990-
return 0
1008+
command_status=$?
1009+
return "$command_status"
9911010
;;
9921011

9931012
git-branch|branch-switch|gbranch)
9941013
declare -f fzf_git_branch >/dev/null && fzf_git_branch || "$BASE_DIR/bin/mqlaunch" git-branch
995-
return 0
1014+
command_status=$?
1015+
return "$command_status"
9961016
;;
9971017

9981018
kill-process|killp|pkill-fzf)
9991019
declare -f fzf_kill_process >/dev/null && fzf_kill_process || "$BASE_DIR/bin/mqlaunch" kill-process
1000-
return 0
1020+
command_status=$?
1021+
return "$command_status"
10011022
;;
10021023

10031024
kill-port|killport|port-kill)
10041025
declare -f fzf_kill_port >/dev/null && fzf_kill_port || "$BASE_DIR/bin/mqlaunch" kill-port
1005-
return 0
1026+
command_status=$?
1027+
return "$command_status"
10061028
;;
10071029

10081030
snippets|snippet|scripts)
10091031
declare -f fzf_run_snippet >/dev/null && fzf_run_snippet || "$BASE_DIR/bin/mqlaunch" snippets
1010-
return 0
1032+
command_status=$?
1033+
return "$command_status"
10111034
;;
10121035

10131036
recent|recent-files|rf)
10141037
declare -f fzf_recent_files >/dev/null && fzf_recent_files || "$BASE_DIR/bin/mqlaunch" recent
1015-
return 0
1038+
command_status=$?
1039+
return "$command_status"
10161040
;;
10171041

10181042
brain)

tests/delegated-exit-code-smoke.sh

Lines changed: 88 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,23 +32,23 @@ assert_status() {
3232

3333
echo "SMOKE: delegated exit-code contract"
3434

35-
echo "[1/5] missing backend is non-zero"
35+
echo "[1/7] missing backend is non-zero"
3636
unset -f run_agent_command 2>/dev/null || true
3737
assert_status 1 dispatch_cli_command review
3838
grep -q 'bridge not loaded' "$TMPDIR_TEST/stderr"
3939

40-
echo "[2/5] usage and runtime failures propagate"
40+
echo "[2/7] usage and runtime failures propagate"
4141
run_agent_command() { return "${MQ_TEST_BACKEND_STATUS:-0}"; }
4242
MQ_TEST_BACKEND_STATUS=2 assert_status 2 dispatch_cli_command review
4343
MQ_TEST_BACKEND_STATUS=42 assert_status 42 dispatch_cli_command stack status
4444

45-
echo "[3/5] HAL pause does not overwrite backend status"
45+
echo "[3/7] HAL pause does not overwrite backend status"
4646
mq_hal_run() { return "${MQ_TEST_BACKEND_STATUS:-0}"; }
4747
rm -f "$TMPDIR_TEST/pause.log"
4848
MQ_TEST_BACKEND_STATUS=42 assert_status 42 dispatch_cli_command hal brief
4949
[[ -s "$TMPDIR_TEST/pause.log" ]]
5050

51-
echo "[4/5] JSON stdout stays clean"
51+
echo "[4/7] JSON stdout stays clean"
5252
mq_hal_run() {
5353
printf '{"schema":"hal.test.v1"}\n'
5454
return 42
@@ -59,7 +59,7 @@ assert_status 42 dispatch_cli_command hal brief --json
5959
[[ ! -e "$TMPDIR_TEST/pause.log" ]]
6060
[[ ! -s "$TMPDIR_TEST/stderr" ]]
6161

62-
echo "[5/5] full launcher returns backend status without double dispatch"
62+
echo "[5/7] full launcher returns backend status without double dispatch"
6363
mkdir -p "$TMPDIR_TEST/bin" "$TMPDIR_TEST/agent"
6464
cat > "$TMPDIR_TEST/bin/uv" <<EOF
6565
#!/usr/bin/env bash
@@ -87,5 +87,88 @@ set -e
8787
exit 1
8888
}
8989

90+
echo "[6/7] external script delegates propagate their status"
91+
# Steps 1-5 stub shell functions, which only reaches the agent and HAL families.
92+
# Most of the surface delegates to scripts under $BASE_DIR instead, and those
93+
# were never covered. Both cases below are real argparse failures.
94+
run_launcher() {
95+
set +e
96+
MACOS_SCRIPTS_HOME="$ROOT" MQ_NO_TUI=1 MQLAUNCH_HEADLESS=1 \
97+
timeout 60 "$ROOT/terminal/launchers/mqlaunch.sh" "$@" \
98+
</dev/null >"$TMPDIR_TEST/stdout" 2>"$TMPDIR_TEST/stderr"
99+
local status=$?
100+
set -e
101+
return "$status"
102+
}
103+
104+
for bad_case in "repos LIST" "skills no-such-subcommand"; do
105+
# shellcheck disable=SC2086
106+
set -- $bad_case
107+
if run_launcher "$@"; then
108+
echo "delegated failure reported success: mqlaunch $bad_case" >&2
109+
exit 1
110+
fi
111+
grep -q 'usage:' "$TMPDIR_TEST/stderr" || {
112+
echo "expected a delegate usage error on stderr for: mqlaunch $bad_case" >&2
113+
exit 1
114+
}
115+
done
116+
117+
# The success path must stay 0, so the fix cannot be "always return non-zero".
118+
run_launcher repos list || {
119+
echo "successful delegation no longer exits 0" >&2
120+
exit 1
121+
}
122+
123+
echo "[7/7] no delegating branch ends in an unconditional return 0"
124+
# The behavioural cases above pin two branches. This keeps the other nineteen
125+
# from drifting back, and stops new ones from being written that way.
126+
python3 - "$COMMAND_MODE" <<'PY'
127+
import re
128+
import sys
129+
130+
lines = open(sys.argv[1]).read().splitlines()
131+
start = next(i for i, l in enumerate(lines) if l.strip() == 'case "$area" in')
132+
133+
branch = re.compile(
134+
r'((?:"[^"]*"|[A-Za-z0-9_*/.\-])+(?:\|(?:"[^"]*"|[A-Za-z0-9_*/.\-])+)*)\)'
135+
)
136+
depth = 0
137+
current = None
138+
bodies = {}
139+
for i in range(start, len(lines)):
140+
s = lines[i].strip()
141+
if s.startswith("case ") and re.match(r'case\s+"?\$', s):
142+
depth += 1
143+
elif s.startswith("esac"):
144+
depth -= 1
145+
if depth == 0:
146+
break
147+
elif branch.match(s):
148+
if depth == 1:
149+
current = (branch.match(s).group(1), i + 1)
150+
bodies[current] = []
151+
elif current is not None:
152+
bodies[current].append(s)
153+
elif current is not None:
154+
bodies[current].append(s)
155+
156+
offenders = []
157+
for (name, line), body in bodies.items():
158+
text = "\n".join(body)
159+
if not re.search(r"\$BASE_DIR/(tools|terminal|bin|automation)", text):
160+
continue
161+
if re.search(r"^return 0$", text, re.M):
162+
offenders.append(f" line {line}: {name}")
163+
164+
if offenders:
165+
print(
166+
"these branches delegate and then discard the delegate's status:",
167+
file=sys.stderr,
168+
)
169+
print("\n".join(offenders), file=sys.stderr)
170+
sys.exit(1)
171+
PY
172+
90173
bash -n "$0"
91174
echo "OK: delegated failures preserve their exit status"

0 commit comments

Comments
 (0)