fix(exit-codes): propagate function delegates, keep menu returns at 0 - #168
Merged
Conversation
Seven branches called a shell-function delegate and ended in `return 0`. Step 7 of the exit-code smoke never looked at them: it only inspects branches invoking a $BASE_DIR script. Each was measured with the delegate stubbed to exit 7 before deciding anything, and the measurement split them in a way reading them would not have. Six invocations are operations whose failure was reported as success — review-brain, signal-brain, learn-promote, and workflows, login and shortcuts when given arguments. learn-promote matters most: it runs `mq-agent learn promote <slug> --approve`, a Class C write. Four are deliberate. workflows, login and shortcuts open a menu when called bare, and atlas is an interactive session in both forms. The mixed branches are why a per-branch answer would have been wrong. A first attempt propagated in both cases and the full suite stayed green — but `mqlaunch shortcuts` with no terminal went from exit 0 to exit 1, because a menu loop exits non-zero on EOF by design (tests/menu-eof-smoke.sh). Propagating there reports "the command failed" for "there was no terminal". The suite did not catch it; running the real command did. Only the argument path propagates now. Steps 10 and 11 pin both halves, so the deliberate one cannot later be mistaken for the bug and "fixed". ask, fix and chat are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MCamner
added a commit
that referenced
this pull request
Aug 3, 2026
…y had (#172) * fix(exit): give system, theme and apps the menu split the rest already had Measured headless, all eleven menu entrypoints ended at their own prompt on EOF and three answered differently: git release shortcuts tools workflows dev hal performance exit 0 system theme apps exit 1 apps is a third outlier the first sweep missed, having read it as an AI command and excluded it on cost. #168 settled which answer is right: a menu loop exits non-zero without a terminal by design, so propagating that reports "the command failed" for "there was no terminal". All three take the split #168 gave workflows — menu path 0, argument path untouched. theme apply bogus still exits 1, theme bogusverb 1, system bogusverb 2, a failed apps ask keeps its status. Step 7 could not have caught this: it flags a branch only when the branch both invokes a $BASE_DIR script and ends in a bare return 0. theme invokes no script, and the mixed shape is not what the pattern describes. The fix then made the step fail for the wrong reason, since system and apps do call scripts and now hold a deliberate return 0. Rather than reword the rule until it passed, step 7 carries a named exception list and honours an entry only when the same branch propagates somewhere. An exception cannot cover a branch that discards status everywhere, and a stale entry fails the step. Behavioural proof stays separate: step 10 is ten propagating paths, step 11 eight deliberate zeros, step 12 stubs hal-terminal-guide.sh through a fake BASE_DIR for both apps paths. Both gates proven able to fail: restoring the theme defect stops the run at step 11, removing the propagating path from apps stops it at step 7. Lifting the menu case into a guard above the case removed the literal menu) arm, which the registry validator reads to confirm the declared subcommand exists. The menu path went back inside the case and returns 0 from within it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(exit): make an invalid theme argument 2, and hold the contract end to end The contract, written out because three commands had been guessing at it: a valid interactive menu with no terminal renders at most once and exits 0; an operation with arguments propagates its real status; an invalid argument is 2; repos keeps 1 as a documented exception, since it asks for a terminal-dependent picker while also offering headless subcommands. theme was the one command not meeting the third point. mq-zsh-theme-switcher.sh exited 1 for an unknown command word, for `apply` with no variant, and for a variant that does not exist — the code a caller reads as "the theme could not be applied" rather than "that is not a theme". All three are 2 now. Runtime failures keep 1, which is the point of the distinction. theme-command-surface-smoke.sh was unaffected: its exit-code step stubs theme_cmd with an arbitrary status and asserts propagation. tests/menu-exit-contract-smoke.sh holds all four points end to end through bin/mqlaunch rather than through stubs, headless and on a pty with closed stdin. Proven able to fail: putting `theme apply` back to 1 stops it at step 6, letting the theme menu propagate again stops it at step 2. The missing-theme-file case is checked by reading the switcher rather than running it, and that is a deliberate retreat. `apply` with a valid variant rewrites the caller's ~/.zshrc, and THEME_FILE is assigned unconditionally at line 6 with no override to steer it elsewhere. A first version of the step set THEME_FILE to force the runtime branch; it had no effect, and the switcher applied the theme to the machine running the suite. Driving `apply` for real needs that override to exist first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(exit): drop hal from the menu list — it is a delegation, not a menu The headless sweep found `hal` answering 0 and I read that as a menu ending. It is not: the arm calls mq_hal_run, a bridge into the mq-hal repo, and its status is that delegate's. On a machine without mq-hal checked out it returns 127, which is the correct answer rather than a contract breach — and CI is exactly that machine, where it failed as `hal=127`. `agent`, `obsidian` and `stack` were never on the list for the same reason. Every command that remains runs a menu or a script inside this repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(theme): resolve the switcher's root the way its siblings do mq-zsh-theme-switcher.sh read `${HOME}/macos-scripts` outright, where doctor.sh and scan.sh both read `${MACOS_SCRIPTS_HOME:-$HOME/macos-scripts}`. A checkout anywhere else could not run the switcher at all: it exited 1 with "Missing UI library" before reaching its first command. CI found it — `mqlaunch theme current` returned 1 there while returning 0 locally, because the runner checks out under /home/runner/work. That also made the runtime branch testable without side effects. `apply` with a valid variant rewrites $ZSHRC; the earlier attempt to steer it with THEME_FILE did nothing, since the switcher assigns that unconditionally and never reads the environment, and the theme was applied to the machine running the suite. MACOS_SCRIPTS_HOME is the handle that works. The step now runs the switcher against an isolated tree and an isolated HOME, requires exit 1 and the "Missing theme file" message, and requires that no .zshrc was written on the way there — replacing a structural read of the source. terminal/themes/mq-theme-manager.sh still hardcodes the path and is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: correct two claims the implementation had moved past Both were written before the later commits on this branch and never revised. The theme examples said `theme apply bogus` and `theme bogusverb` still exit 1. They exit 2 — that is what the third contract point required and what the switcher change delivered. The menu inventory counted `hal` among eleven local menus. The branch later established that `hal` is a delegation to mq_hal_run, not a menu, and removed it from the test list for that reason; the prose did not follow. The contract covers ten local menus, and both files now say why `hal` is not one of them. Documentation only. No code or test changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Measured before decided
Seven branches called a shell-function delegate and ended in
return 0. Step 7 of the exit-code smoke never looked at them — it only inspects branches invoking a$BASE_DIRscript.Each was measured with the delegate stubbed to exit 7 before deciding anything. The measurement split them in a way reading them would not have:
learn-promoteis the one that matters most: it runsmq-agent learn promote <slug> --approve, a Class C write, and reported success whatever happened.The mixed branches are why a per-branch answer would have been wrong
workflows,loginandshortcutsopen a menu when called bare and run an operation when given arguments.A first attempt propagated the status in both cases and the full suite stayed green. Then the real command:
A menu loop exits non-zero on EOF by design (
tests/menu-eof-smoke.sh). Propagating there reports "the command failed" for "there was no terminal". The suite did not catch it; running the real command did. Only the argument path propagates now.atlasis an interactive session in both forms — there is no non-interactive result for a caller to act on.Tests
Step 10 pins the six operation paths. Step 11 pins the four interactive entrypoints at 0, so the deliberate half cannot later be mistaken for the bug and "fixed".
Verification
tests/delegated-exit-code-smoke.sh: 11/11shortcuts,login,workflows,atlasall exit 0 without a terminalask,fixandchatare untouched.🤖 Generated with Claude Code