mirror of
https://github.com/msitarzewski/agency-agents.git
synced 2026-10-17 16:45:02 +00:00
fix(install): one failing tool no longer stops the tools after it
A sequential install called install_tool bare under set -e, so the first tool that returned 1 exited the whole script. With copilot, cursor, qwen, codex and hermes detected and no integrations/cursor: [1/5] copilot [OK] [2/5] cursor [ERR] integrations/cursor missing. Run convert.sh first. (exit 1) qwen, codex and hermes were never attempted, and nothing said so. The same happens for any ensure_converted failure or an OpenClaw registration error. Each tool now runs in a subshell with errexit on. It is not `install_tool "$t" || failed+=(...)`: bash ignores errexit for everything on the left of ||, subshells included, so a failing cp inside a tool would carry on as if it had worked. Instead the parent turns errexit off for that one command, and the subshell turns it back on for itself. A failed tool is recorded and the loop moves on. The done box reads "Installed 2 of 3 tool(s)" instead of "Done!", and the run ends with "Failed: cursor — see the [ERR] line under each above" and exit 1. A clean install is unchanged. Nothing in install_* sets globals the parent reads afterwards, and the SKIPPED_LOG cleanup trap does not fire in a ( ) subshell, so running tools in a subshell loses nothing. Parallel mode is untouched; its log replay on failure is item 1 of #917. test-install-continue-after-failure.sh builds a fixture repo with gemini-cli and qwen output and no cursor output, installs all three, and checks that qwen still installs, cursor's error is shown, the summary names cursor, and the exit is non-zero. It also checks that a clean run still exits 0. It fails on main at "qwen ... was never installed" and runs in check-tools.yml.
This commit is contained in:
hotragn
@@ -56,3 +56,6 @@ jobs:
|
||||
|
||||
- name: Reject unclosed agent frontmatter
|
||||
run: bash scripts/test-frontmatter-closing.sh
|
||||
|
||||
- name: Keep installing after one tool fails, then exit non-zero
|
||||
run: bash scripts/test-install-continue-after-failure.sh
|
||||
|
||||
+30
-4
@@ -1676,7 +1676,8 @@ main() {
|
||||
fi
|
||||
printf "\n"
|
||||
|
||||
local installed=0 t i=0
|
||||
local installed=0 t i=0 rc
|
||||
local failed=()
|
||||
if $use_parallel; then
|
||||
local install_out_dir
|
||||
install_out_dir="$(mktemp -d)"
|
||||
@@ -1695,16 +1696,37 @@ main() {
|
||||
progress_bar "$i" "$n_selected"
|
||||
printf "\n"
|
||||
printf " ${C_DIM}[%s/%s]${C_RESET} %s\n" "$i" "$n_selected" "$t"
|
||||
install_tool "$t"
|
||||
(( installed++ )) || true
|
||||
# One tool failing must not cost the tools after it. A bare
|
||||
# install_tool under set -e exited the whole script at the first
|
||||
# `return 1`, so a missing integrations/cursor meant qwen, codex and
|
||||
# every later tool were never tried and nothing said so.
|
||||
#
|
||||
# Not `install_tool "$t" || ...`: bash ignores errexit inside anything
|
||||
# run on the left of || (subshell included), so a failing cp inside a
|
||||
# tool would carry on as if it had worked. The subshell turns errexit
|
||||
# back on for itself while the parent's is off for this one command.
|
||||
set +e
|
||||
( set -e; install_tool "$t" )
|
||||
rc=$?
|
||||
set -e
|
||||
if (( rc == 0 )); then
|
||||
(( installed++ )) || true
|
||||
else
|
||||
failed+=("$t")
|
||||
fi
|
||||
done
|
||||
fi
|
||||
|
||||
# Done box
|
||||
local msg=" Done! Installed $installed tool(s)."
|
||||
(( ${#failed[@]} )) && msg=" Installed $installed of $n_selected tool(s)."
|
||||
printf "\n"
|
||||
box_top
|
||||
box_row "${C_GREEN}${C_BOLD}${msg}${C_RESET}"
|
||||
if (( ${#failed[@]} )); then
|
||||
box_row "${C_YELLOW}${C_BOLD}${msg}${C_RESET}"
|
||||
else
|
||||
box_row "${C_GREEN}${C_BOLD}${msg}${C_RESET}"
|
||||
fi
|
||||
box_bot
|
||||
printf "\n"
|
||||
if [[ -s "$SKIPPED_LOG" ]]; then
|
||||
@@ -1714,6 +1736,10 @@ main() {
|
||||
fi
|
||||
dim " Run ./scripts/convert.sh to regenerate after adding or editing agents."
|
||||
printf "\n"
|
||||
if (( ${#failed[@]} )); then
|
||||
err "Failed: ${failed[*]} — see the [ERR] line under each above. The other tools installed."
|
||||
exit 1
|
||||
fi
|
||||
}
|
||||
|
||||
main "$@"
|
||||
|
||||
+60
@@ -0,0 +1,60 @@
|
||||
#!/usr/bin/env bash
|
||||
# One tool failing in a sequential install must not cost the tools after it,
|
||||
# and the install must still exit non-zero and name the tool that failed.
|
||||
#
|
||||
# Before: install_tool ran bare under set -e, so the first tool whose output
|
||||
# was missing exited the script. With gemini-cli, cursor and qwen selected and
|
||||
# no integrations/cursor, gemini-cli installed, cursor printed its [ERR], and
|
||||
# qwen was never attempted and never mentioned.
|
||||
set -euo pipefail
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
tmp="$(mktemp -d "${TMPDIR:-/tmp}/agency-continue.XXXXXX")"
|
||||
trap 'rm -rf "$tmp"' EXIT
|
||||
|
||||
fail() { echo "FAIL: $*" >&2; sed 's/^/ /' "$tmp/output" >&2; exit 1; }
|
||||
|
||||
mkdir -p "$tmp/repo/scripts" "$tmp/repo/engineering" \
|
||||
"$tmp/repo/integrations/gemini-cli/agents" "$tmp/repo/integrations/qwen/agents" \
|
||||
"$tmp/home/project"
|
||||
cp "$SCRIPT_DIR/install.sh" "$SCRIPT_DIR/lib.sh" "$tmp/repo/scripts/"
|
||||
cat > "$tmp/repo/divisions.json" <<'EOF'
|
||||
{
|
||||
"divisions": {
|
||||
"engineering": {}
|
||||
}
|
||||
}
|
||||
EOF
|
||||
cat > "$tmp/repo/engineering/agent.md" <<'EOF'
|
||||
---
|
||||
name: Sample Agent
|
||||
description: Example agent
|
||||
---
|
||||
Instructions.
|
||||
EOF
|
||||
printf 'Gemini output\n' > "$tmp/repo/integrations/gemini-cli/agents/sample-agent.md"
|
||||
printf 'Qwen output\n' > "$tmp/repo/integrations/qwen/agents/sample-agent.md"
|
||||
# integrations/cursor is deliberately absent: cursor is the tool that fails.
|
||||
|
||||
status=0
|
||||
(cd "$tmp/home/project" && HOME="$tmp/home" PATH=/usr/bin:/bin \
|
||||
bash "$tmp/repo/scripts/install.sh" --no-interactive --no-convert \
|
||||
--tool gemini-cli,cursor,qwen > "$tmp/output" 2>&1) || status=$?
|
||||
|
||||
[[ "$status" -ne 0 ]] || fail "install exited 0 although cursor failed"
|
||||
[[ -f "$tmp/home/.gemini/agents/sample-agent.md" ]] || fail "gemini-cli, before the failure, did not install"
|
||||
[[ -f "$tmp/home/project/.qwen/agents/sample-agent.md" ]] \
|
||||
|| fail "qwen, after the failing cursor, was never installed"
|
||||
grep -q 'integrations/cursor missing' "$tmp/output" || fail "cursor's own error was not shown"
|
||||
grep -q 'Failed: cursor' "$tmp/output" || fail "the summary does not name the failed tool"
|
||||
grep -q 'Installed 2 of 3 tool(s)' "$tmp/output" || fail "the summary does not count the tools that installed"
|
||||
echo "PASS: a failing tool is reported and the tools after it still install"
|
||||
|
||||
# The clean path is unchanged: every tool installs and the install exits 0.
|
||||
mkdir -p "$tmp/repo/integrations/cursor/rules" "$tmp/home2/project"
|
||||
printf -- '---\ndescription: x\n---\nCursor output\n' > "$tmp/repo/integrations/cursor/rules/sample-agent.mdc"
|
||||
(cd "$tmp/home2/project" && HOME="$tmp/home2" PATH=/usr/bin:/bin \
|
||||
bash "$tmp/repo/scripts/install.sh" --no-interactive --no-convert \
|
||||
--tool gemini-cli,cursor,qwen > "$tmp/output" 2>&1) || fail "a clean install exited non-zero"
|
||||
grep -q 'Done! Installed 3 tool(s)' "$tmp/output" || fail "a clean install did not report all three tools"
|
||||
echo "PASS: a clean multi-tool install still exits 0"
|
||||
Reference in New Issue
Block a user