fix(tools/git): the tea diagnostic must not suppress the API fallback
ci/woodpecker/pr/ci Pipeline failed
ci/woodpecker/pr/ci Pipeline failed
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
This commit is contained in:
Mos
@@ -156,7 +156,7 @@ case "$PLATFORM" in
|
||||
exit 0
|
||||
fi
|
||||
echo "Warning: tea issue create failed, trying Gitea API fallback..." >&2
|
||||
declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist
|
||||
{ declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist; } || true
|
||||
fi
|
||||
gitea_issue_create_api
|
||||
;;
|
||||
|
||||
@@ -71,7 +71,7 @@ elif [[ "$PLATFORM" == "gitea" ]]; then
|
||||
exit 0
|
||||
fi
|
||||
echo "Warning: tea issue view failed, trying Gitea API fallback..." >&2
|
||||
declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist
|
||||
{ declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist; } || true
|
||||
fi
|
||||
gitea_issue_view_api
|
||||
else
|
||||
|
||||
@@ -219,7 +219,7 @@ case "$PLATFORM" in
|
||||
exit 0
|
||||
fi
|
||||
echo "Warning: tea pr create failed, trying Gitea API fallback..." >&2
|
||||
declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist
|
||||
{ declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist; } || true
|
||||
gitea_pr_create_api
|
||||
;;
|
||||
*)
|
||||
|
||||
+70
@@ -0,0 +1,70 @@
|
||||
#!/bin/bash
|
||||
# Regression: the tea-failure diagnostic must be STATUS-NEUTRAL.
|
||||
#
|
||||
# Found by be-coder-08 reviewing PR #1086. At all three call sites the diagnostic is
|
||||
# emitted immediately BEFORE the Gitea API fallback. Written as
|
||||
# declare -F explain_... >/dev/null && explain_...
|
||||
# it is the last command of an && list, so under `set -e` a FAILING diagnostic exits the
|
||||
# script and the fallback never runs -- a diagnostic that suppresses the recovery path it
|
||||
# exists to explain. Worse, it only misbehaves when the helper is PRESENT, so the
|
||||
# helper-absent path (the pre-#1086 behaviour) silently acts as the passing control.
|
||||
#
|
||||
# The control is the point: helper-absent MUST reach the fallback, and helper-present MUST
|
||||
# reach it too. A test asserting only "helper-present reaches fallback" would pass on a
|
||||
# build where the diagnostic never ran at all.
|
||||
set -uo pipefail
|
||||
fail=0
|
||||
GIT_DIR_UNDER_TEST="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
|
||||
probe() { # $1=present|absent $2=stderr path $3=source file to lift the construct from
|
||||
local helper="$1" errto="$2" src="$3"
|
||||
local CONSTRUCT
|
||||
CONSTRUCT=$(grep -m1 "explain_tea_user_does_not_exist" "$GIT_DIR_UNDER_TEST/$src" | sed "s/^[[:space:]]*//")
|
||||
[ -n "$CONSTRUCT" ] || { echo "no-construct-found"; return; }
|
||||
export CONSTRUCT
|
||||
local out rc
|
||||
out=$(
|
||||
set -e
|
||||
explain_tea_user_does_not_exist() { echo "diagnostic" >&2; }
|
||||
[ "$helper" = absent ] && unset -f explain_tea_user_does_not_exist
|
||||
# THE CONSTRUCT IS EXTRACTED FROM THE SHIPPED FILE, NOT RETYPED HERE.
|
||||
# Retyping it would make this probe correct-by-construction: it would pass on a
|
||||
# build whose real call sites still carry the bare && form. $CONSTRUCT is set by
|
||||
# the caller from the actual source line.
|
||||
eval "$CONSTRUCT"
|
||||
echo "FALLBACK_REACHED"
|
||||
) 2>"$errto"
|
||||
rc=$?
|
||||
printf '%s:%s' "$rc" "$(grep -qc FALLBACK_REACHED <<<"$out" && echo yes || echo no)"
|
||||
}
|
||||
|
||||
check() { # $1=label $2=actual $3=expected
|
||||
if [ "$2" = "$3" ]; then echo " PASS $1 ($2)"; else echo " FAIL $1: got $2, want $3"; fail=1; fi
|
||||
}
|
||||
|
||||
echo "== diagnostic must not alter exit status or skip the fallback =="
|
||||
# /dev/full makes every stderr write fail -- the real-world shape is a closed/full fd.
|
||||
# Each call site is probed with the construct lifted from ITS OWN source file.
|
||||
for src in pr-create.sh issue-view.sh issue-create.sh; do
|
||||
check "$src stderr OK / helper present" "$(probe present /dev/null "$src")" "0:yes"
|
||||
check "$src stderr OK / helper absent " "$(probe absent /dev/null "$src")" "0:yes"
|
||||
check "$src stderr FAILING / helper present" "$(probe present /dev/full "$src")" "0:yes"
|
||||
check "$src stderr FAILING / helper absent " "$(probe absent /dev/full "$src")" "0:yes"
|
||||
done
|
||||
|
||||
echo "== all three call sites use the status-neutral form =="
|
||||
for f in pr-create.sh issue-view.sh issue-create.sh; do
|
||||
p="$GIT_DIR_UNDER_TEST/$f"
|
||||
if grep -q '{ declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist; } || true' "$p"; then
|
||||
echo " PASS $f guarded"
|
||||
else
|
||||
echo " FAIL $f: diagnostic is not status-neutral"; fail=1
|
||||
fi
|
||||
# bare form must be gone entirely
|
||||
if grep -qE '^\s*declare -F explain_tea_user_does_not_exist >/dev/null && explain_tea_user_does_not_exist\s*$' "$p"; then
|
||||
echo " FAIL $f: bare && form still present"; fail=1
|
||||
fi
|
||||
done
|
||||
|
||||
[ "$fail" -eq 0 ] && echo "OK diagnostic is status-neutral" || echo "FAILED"
|
||||
exit "$fail"
|
||||
@@ -6,7 +6,7 @@
|
||||
"url": "https://git.mosaicstack.dev/mosaicstack/stack.git",
|
||||
"directory": "packages/mosaic"
|
||||
},
|
||||
"description": "Mosaic agent framework — installation wizard and meta package",
|
||||
"description": "Mosaic agent framework \u2014 installation wizard and meta package",
|
||||
"type": "module",
|
||||
"main": "dist/index.js",
|
||||
"types": "dist/index.d.ts",
|
||||
@@ -25,7 +25,7 @@
|
||||
"lint": "eslint src",
|
||||
"typecheck": "tsc --noEmit",
|
||||
"test": "vitest run --passWithNoTests && pnpm run test:framework-shell",
|
||||
"test:framework-shell": "bash framework/tools/quality/scripts/check-test-enumeration.sh && bash framework/tools/quality/scripts/test-check-test-enumeration.sh && python3 src/lease-broker/daemon_deadline_unittest.py && python3 src/lease-broker/normative_fragments_unittest.py && python3 src/lease-broker/receipt_challenge_unittest.py && python3 src/lease-broker/context_recovery_unittest.py && python3 src/lease-broker/recovery_runtime_unittest.py && python3 src/lease-broker/recovery_b1_adversarial_unittest.py && python3 src/lease-broker/framework_skill_portability_unittest.py && python3 src/mutator-gate/runtime_tools_unittest.py && python3 src/mutator-gate/runtime_launch_guard_unittest.py && python3 src/mutator-gate/version_coupling_unittest.py && python3 framework/tools/lease-broker/check-runtime-launches.py --root ../.. && bash framework/tools/codex/test-pr-diff-context.sh && bash framework/tools/qa/test-deps-preflight.sh && bash framework/tools/git/test-pr-review-gitea-comment.sh && bash framework/tools/git/test-pr-review-repo-host-override.sh && bash framework/tools/git/test-ci-queue-wait-branch-absent.sh && bash framework/tools/git/test-ci-queue-wait-tristate.sh && bash framework/tools/git/test-ci-queue-wait-github-checks.sh && bash framework/tools/git/test-pr-merge-queue-branch.sh && bash framework/tools/git/test-pr-merge-head-pin.sh && bash framework/tools/git/test-pr-merge-message-field.sh && bash framework/tools/git/test-git-credential-mosaic.sh && bash framework/tools/git/test-gitea-token-identity.sh && bash framework/tools/woodpecker/test-terminal-green-contract.sh && bash framework/tools/_scripts/test-install-ordering-guard.sh && bash framework/tools/tmux/agent-send.test.sh && bash framework/tools/wake/test-wake-store-ack.sh && bash framework/tools/wake/test-wake-store-enqueue-race.sh && bash framework/tools/wake/test-wake-digest-hmac.sh && bash framework/tools/wake/test-wake-digest-quarantine.sh && bash framework/tools/wake/test-wake-detector.sh && bash framework/tools/wake/test-wake-fn-oracle.sh && bash framework/tools/wake/test-wake-reconcile.sh && bash framework/tools/wake/test-wake-beacon.sh && bash framework/tools/wake/test-wake-preimage.sh && bash framework/tools/wake/test-wake-install.sh"
|
||||
"test:framework-shell": "bash framework/tools/quality/scripts/check-test-enumeration.sh && bash framework/tools/quality/scripts/test-check-test-enumeration.sh && python3 src/lease-broker/daemon_deadline_unittest.py && python3 src/lease-broker/normative_fragments_unittest.py && python3 src/lease-broker/receipt_challenge_unittest.py && python3 src/lease-broker/context_recovery_unittest.py && python3 src/lease-broker/recovery_runtime_unittest.py && python3 src/lease-broker/recovery_b1_adversarial_unittest.py && python3 src/lease-broker/framework_skill_portability_unittest.py && python3 src/mutator-gate/runtime_tools_unittest.py && python3 src/mutator-gate/runtime_launch_guard_unittest.py && python3 src/mutator-gate/version_coupling_unittest.py && python3 framework/tools/lease-broker/check-runtime-launches.py --root ../.. && bash framework/tools/codex/test-pr-diff-context.sh && bash framework/tools/qa/test-deps-preflight.sh && bash framework/tools/git/test-pr-review-gitea-comment.sh && bash framework/tools/git/test-pr-review-repo-host-override.sh && bash framework/tools/git/test-ci-queue-wait-branch-absent.sh && bash framework/tools/git/test-ci-queue-wait-tristate.sh && bash framework/tools/git/test-ci-queue-wait-github-checks.sh && bash framework/tools/git/test-pr-merge-queue-branch.sh && bash framework/tools/git/test-pr-merge-head-pin.sh && bash framework/tools/git/test-pr-merge-message-field.sh && bash framework/tools/git/test-git-credential-mosaic.sh && bash framework/tools/git/test-gitea-token-identity.sh && bash framework/tools/git/test-explain-diagnostic-status-neutral.sh && bash framework/tools/woodpecker/test-terminal-green-contract.sh && bash framework/tools/_scripts/test-install-ordering-guard.sh && bash framework/tools/tmux/agent-send.test.sh && bash framework/tools/wake/test-wake-store-ack.sh && bash framework/tools/wake/test-wake-store-enqueue-race.sh && bash framework/tools/wake/test-wake-digest-hmac.sh && bash framework/tools/wake/test-wake-digest-quarantine.sh && bash framework/tools/wake/test-wake-detector.sh && bash framework/tools/wake/test-wake-fn-oracle.sh && bash framework/tools/wake/test-wake-reconcile.sh && bash framework/tools/wake/test-wake-beacon.sh && bash framework/tools/wake/test-wake-preimage.sh && bash framework/tools/wake/test-wake-install.sh"
|
||||
},
|
||||
"dependencies": {
|
||||
"@mosaicstack/brain": "workspace:*",
|
||||
|
||||
Reference in New Issue
Block a user