fix(hooks): a locate miss is an answer, not a locate failure

[KageBinary]

Step 1 of ix-infrastructure/Ix#539, which names this plugin's ix-bash.sh as one of the consumers that must tolerate a non-zero ix locate before the CLI starts producing one.

What actually breaks here

Not what I expected. The hook already survives a non-zero exit — it redirects stdout to a temp file and reads the file regardless of status, so the body is never lost.

The problem is the ledger. wait "$_LOC_PID" || ix_capture_async ... "locate failed" files an error for every miss. Once #539 lands, every symbol a user asks about that does not happen to exist becomes a recorded locate failed, burying the real failures.

Now only an empty body is a genuine failure; a non-zero exit that still produced output is logged as a miss and used.

Two things worth flagging

wait cannot be bare here. These hooks run under set -euo pipefail, so an unguarded non-zero wait kills the hook outright. My first attempt did exactly that and took 19 of the 83 tests with it. The suite caught it immediately — which is a good argument for #36, since nothing gates these tests in CI yet.

The shape that works is _loc_status=0; wait || _loc_status=$? — set -e-safe, and it keeps $?, which if ! wait would have replaced with the negation's own result.

The mock needed a new knob. IX_MOCK_FAIL suppresses output as well as failing, and the whole point of #539 is a failing exit code with a usable payload. Hence IX_MOCK_LOCATE_EXIT=N, which exits N after printing the fixture.

Tests

86 passed, 0 failed (83 existing + 3 new).

Being precise about what they buy: reverting the fix fails exactly one — non-zero locate with a body is a miss, not a failure. The other two pass on the old code too and are deliberate guards: one locks in that the body still reaches the user, the other that an empty body stays a real failure. They would catch a refactor to command substitution, which is the obvious way someone reintroduces this.

Verified with bash -n and shellcheck -S error.

[KageBinary]

Heads-up on a latent bug in the mock change here — I hit the identical one writing #38 and only found it by accident.

tests/mock-ix.sh:

[ -n "${IX_MOCK_LOCATE_EXIT:-}" ] && exit "${IX_MOCK_LOCATE_EXIT}"

When IX_MOCK_LOCATE_EXIT is unset, [ -n "" ] is false, so the && list evaluates to status 1. It is the last command in the locate) branch, so that becomes the mock's own exit status — ix locate exits 1 on every call, in every test, not just the ones setting the variable.

It is invisible in this PR precisely because the fix makes the hooks tolerate a non-zero exit with a body. I found mine only when I reverted hooks/ for a mutation check and five unrelated tests went red that had nothing to do with the change.

The fix is to use if, which is status 0 when the condition is false:

if [ -n "${IX_MOCK_LOCATE_EXIT:-}" ]; then exit "${IX_MOCK_LOCATE_EXIT}"; fi

#38 does it that way and carries a comment explaining why, so if this lands first the two will disagree on style in the same file — worth aligning either way.

Worth doing before merge: as it stands, this branch's mock cannot tell you whether the locate tests still pass when ix locate succeeds normally.

[KageBinary]

Pushed the fix to this branch (d072655). One file, tests/mock-ix.sh.

Confirmed the bug on this branch before touching it

$ bash tests/mock-ix.sh locate   >/dev/null 2>&1; echo $?   ->  1
$ bash tests/mock-ix.sh overview >/dev/null 2>&1; echo $?   ->  0   (untouched branch)

ix locate exited 1 on every call, in every test — not only the ones setting IX_MOCK_LOCATE_EXIT. Now an if, which is status 0 when the condition is false:

unset -> 0    =1 -> 1    =3 -> 3    body still printed before exiting

What it actually cost, having measured rather than assumed

The suite is 86 passed / 0 failed both before and after the mock fix, so the bug was never causing a failure. What it removed was the success path: with ix locate failing on every call, no test exercised a locate that exits 0, so a regression there would have gone unnoticed. Your fix makes the hooks tolerate a non-zero-with-body, which is exactly why nothing complained.

Your coverage is genuine — I re-ran the check the bug had made untrustworthy

My first attempt at this was itself a no-op, worth flagging: git stash push hooks/ stashes uncommitted changes, and your hook fix is committed on the branch, so the suite ran with the fix still in place and "passed". Reverting the committed file instead:

$ git checkout origin/main -- hooks/ix-lib.sh
$ bash tests/test_hooks.sh
FAIL  intercept/non-zero locate with a body is a miss, not a failure
Results: 85 passed, 1 failed

Exactly one test fails, and it is the right one. The fix in this PR is properly pinned; only the harness was wrong.

Note on #38

I hit the identical bug writing #38 and fixed it there the same way, for IX_MOCK_OVERVIEW_EXIT / IX_MOCK_IMPACT_EXIT / IX_MOCK_INVENTORY_EXIT. Both branches now use the if form with a comment explaining why, so whichever merges first the other should apply cleanly apart from an obvious overlap in the header comment block.