Commit Graph
2 Commits
Author SHA1 Message Date
mos 6f1924f32b test(tools/git): make the status-neutral probe actually exercise the shipped line
be-coder-08 found two defects in the regression test I added with the fix. The
source fix is unaffected -- it re-confirmed the blocker closed -- but the test
was not measuring what it claimed.

1. `out=$( ... ) 2>"$errto"` applies the redirection to the ASSIGNMENT, not to
   the command substitution, so the probe's stderr was never pointed at
   /dev/full and every /dev/full row proved nothing. Verified directly:

       out=$( echo x >&2 ) 2>/dev/full     # leaks to the terminal, rc=0
       out=$( { echo x >&2 ; } 2>/dev/full )  # rc=1

   The redirect has to be inside the substitution.

2. `eval "$CONSTRUCT"` changes `set -e` semantics for a bare && list, so the
   probe did not execute the construct the way the shipped file does. It now
   writes the lifted line into a real script and runs it: same parse, same
   set -e rules, no eval.

Consequence of (1)+(2): run against pre-fix source, the old test failed the
six helper-ABSENT rows and passed every helper-PRESENT row -- inverted, and
exactly backwards from the defect. It would have gone green on a broken tree
for the wrong reason.

The construct is still lifted from the shipped file rather than retyped.

Faithful control, matching be-coder-08's prediction exactly:
  fixed source: all 12 behavioural rows 0:yes
  pre-fix source: 1:no on helper-present + failing-stderr ONLY (3 rows, one per
  call site); all 9 other rows 0:yes

Reported-by: be-coder-08
2026-08-06 23:47:17 -05:00
mos 97dbd1bf4a fix(tools/git): the tea diagnostic must not suppress the API fallback
be-coder-08, reviewing #1086, found the diagnostic is not diagnostic-only.

At all three call sites it was written as the last command of an && list:

    declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist

and it sits immediately BEFORE the Gitea API fallback. Under `set -e` a failing
diagnostic (stderr closed or full) therefore exits the script and the fallback
never runs -- a diagnostic that suppresses the recovery path it exists to explain.

The asymmetry is what makes it dangerous: the fault only appears when the helper
is PRESENT, so the helper-absent path -- the pre-#1086 behaviour -- keeps working
and reads as a passing control. Measured on /dev/full:

    helper present  rc=1  fallback NOT reached
    helper absent   rc=0  fallback reached

Wrapping in `{ ...; } || true` makes the diagnostic status-neutral, which is what
the PR claimed to be in the first place.

test-explain-diagnostic-status-neutral.sh probes all four combinations of
{helper present, absent} x {stderr ok, failing} for each of the three call sites,
and lifts the construct FROM THE SHIPPED FILE rather than restating it -- a probe
that retypes the fixed form passes on a build whose real call sites still carry
the bare && form. Verified RED on the pre-fix tree (16 failures, behavioural half
included) and GREEN here. Enumerated on test:framework-shell.

Reported-by: be-coder-08
2026-08-06 23:47:17 -05:00