From bedb6d278da6412c2d9a840598e4b81b484e36db Mon Sep 17 00:00:00 2001 From: Mathias Date: Sun, 30 Aug 2026 11:48:56 +0200 Subject: [PATCH] fix(test): stop the Antigravity cases failing on a host without jq MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main has been red since e9ad692. The gate passed; the shell suite failed on three Antigravity cases. Not a bug in the wiring. install.sh treats jq as optional by design — every other target is dependency-free, so wire_antigravity warns and skips when jq is missing. It did exactly that on the runner: [skills] Antigravity: jq not found — skipping The tests were the problem: they inherited whatever jq the host happened to have. koala has it and reported 12/12; the runner does not and reported 3 failures on the same commit. Works-on-my-machine, caught by CI doing its job. Two changes: - The three merge-behaviour cases now run only where jq exists, and print a SKIP naming what is not covered. A case that fails because the host lacks an optional dependency is a false red, and false reds are how a suite stops being read. - The skip path gets a real test of its own, because it is a contract rather than a fallback. install.sh gains SKILLS_JQ so the jq-absent branch is exercisable on a host that has jq — the same seam pattern as DOCTOR_LATEST_TAG_CMD. It asserts no skills.json is written, the skip is reported, and the other harnesses still wire. Verified both ways on koala by building a jq-free PATH: with jq: install: 13 passed, 0 failed, 0 skipped without jq: install: 10 passed, 0 failed, 3 skipped Consequence worth naming: CI now SKIPS the merge assertions rather than running them, so idempotency and entry-preservation are untested there until the runner gets jq. Filed as #42. Refs #41 Bump-Type: patch --- install.sh | 10 +++- tests/install_test.sh | 129 ++++++++++++++++++++++++++++-------------- 2 files changed, 92 insertions(+), 47 deletions(-) diff --git a/install.sh b/install.sh index 788377f..42ca1b4 100755 --- a/install.sh +++ b/install.sh @@ -62,6 +62,10 @@ CRUSH_DIR="${CRUSH_SKILLS_DIR:-$HOME/.config/crush/skills}" OPENCODE_DIR="${OPENCODE_SKILLS_DIR:-$HOME/.config/opencode/skills}" ANTIGRAVITY_SKILLS_JSON="${ANTIGRAVITY_SKILLS_JSON:-$HOME/.gemini/config/skills.json}" EXTRA_SOURCES="${SKILLS_EXTRA_SOURCES:-}" +# Overridable so the jq-absent branch is testable on a host that has jq. That +# branch is a contract, not a fallback: jq is optional here while every other +# target in this script is dependency-free. +JQ="${SKILLS_JQ:-jq}" # Recorded beside the managed directory rather than inside it: a stray file in # there would look like a skill to every harness reading the dir. SOURCES_MANIFEST="$(dirname "$AGENTS_DIR")/$(basename "$AGENTS_DIR")-sources" @@ -226,7 +230,7 @@ link_dir() { # config (json_configs.md), so a hand-added entry (e.g. a personal skills # dir) must survive a re-run untouched. wire_antigravity() { - if ! command -v jq >/dev/null 2>&1; then + if ! command -v "$JQ" >/dev/null 2>&1; then log "Antigravity: jq not found — skipping (needed to merge $ANTIGRAVITY_SKILLS_JSON safely)" return fi @@ -240,7 +244,7 @@ wire_antigravity() { existing='{}' fi - already="$(printf '%s' "$existing" | jq --arg p "$AGENTS_DIR" \ + already="$(printf '%s' "$existing" | "$JQ" --arg p "$AGENTS_DIR" \ '[.entries // [] | .[] | select(.path == $p)] | length' 2>/dev/null)" || { log "Antigravity: $ANTIGRAVITY_SKILLS_JSON is not valid JSON — leaving it untouched" return 1 @@ -251,7 +255,7 @@ wire_antigravity() { return fi - updated="$(printf '%s' "$existing" | jq --arg p "$AGENTS_DIR" \ + updated="$(printf '%s' "$existing" | "$JQ" --arg p "$AGENTS_DIR" \ '.entries = ((.entries // []) + [{path: $p}])')" printf '%s\n' "$updated" > "$ANTIGRAVITY_SKILLS_JSON" log "Antigravity: registered $AGENTS_DIR in $ANTIGRAVITY_SKILLS_JSON" diff --git a/tests/install_test.sh b/tests/install_test.sh index 9e9f8f1..9226148 100644 --- a/tests/install_test.sh +++ b/tests/install_test.sh @@ -17,6 +17,7 @@ SUT="$ROOT/install.sh" pass=0 fail=0 +skipped=0 WORK=${TMPDIR:-/tmp}/install_test.$$ if [ ! -f "$SUT" ]; then @@ -157,6 +158,40 @@ else no 'configured sources are recorded for doctor' "$out" fi +# --- Antigravity: jq is optional, so BOTH branches must be tested ----------- +# These three cases inherited whatever jq the host happened to have. koala has +# it, the CI runner does not, so they passed locally and failed in CI — the +# classic works-on-my-machine gate. install.sh treats jq as optional by design, +# which means the skip is a contract and needs a test of its own. + +# run_install_nojq — same as run_install, with the jq lookup pointed +# at a binary that does not exist. Deterministic regardless of the host. +run_install_nojq() { + _f=$1 + _out=$(HOME="$_f/home" \ + SKILLS_SKIP_FETCH=1 \ + SKILLS_CHECKOUT_DIR="$_f/ck" \ + AGENTS_SKILLS_DIR="$_f/home/.agents/skills" \ + CRUSH_SKILLS_DIR="$_f/home/.config/crush/skills" \ + OPENCODE_SKILLS_DIR="$_f/home/.config/opencode/skills" \ + SKILLS_JQ="definitely-not-jq-$$" \ + bash "$SUT" 2>&1) + rc=$? + printf '%s' "$_out" +} + +f=$(new_fixture antigravity_nojq) +out=$(run_install_nojq "$f") +if [ -e "$f/home/.gemini/config/skills.json" ]; then + no 'without jq, no skills.json is written' "$out" +elif ! printf '%s' "$out" | grep -qi 'jq not found'; then + no 'without jq, the skip is reported' "$out" +elif [ ! -L "$f/home/.agents/skills/alpha" ]; then + no 'without jq, the other harnesses are still wired' "$out" +else + ok 'without jq, Antigravity is skipped and the rest still wires' +fi + # --- 8. surfaces: the library link set is not "every skill" ----------------- # claude.ai has no shell, no cluster and no checkout, so a chat-only skill # linked into Crush is a skill that can never fire, competing for selection @@ -184,53 +219,59 @@ else no 'a source with no surfaces convention is linked as-is' "$out" fi -# --- 9. Antigravity: the managed agents dir is registered, not the raw checkout -# skills.json entries must point at $AGENTS_DIR — that's what already carries -# surfaces-filtering, collision handling and multi-source merge. Pointing at -# the checkout directly would bypass all of it and give Antigravity a -# different skill set than every other harness. -f=$(new_fixture antigravity) -out=$(run_install "$f") -AG="$f/home/.gemini/config/skills.json" -if [ ! -f "$AG" ]; then - no 'Antigravity skills.json is created' "$out" -elif ! command -v jq >/dev/null 2>&1; then - no 'jq available to verify Antigravity wiring' "(jq missing on test host)" +# --- 9-11. Antigravity's merge behaviour, only where jq exists --------------- +# These three assert what jq DOES. install.sh treats jq as optional, so on a +# host without it there is nothing to assert — and failing there is what turned +# main red: they passed on koala (jq present) and failed on the runner (absent). +# A skipped case is honest; a case that fails because the host lacks an optional +# dependency is a false red, and false reds are how a suite stops being read. +if ! command -v jq >/dev/null 2>&1; then + skipped=$((skipped + 3)) + printf ' SKIP jq absent — Antigravity merge behaviour not covered on this host (3 cases)\n' else - registered=$(jq -r --arg p "$f/home/.agents/skills" '[.entries[]? | select(.path == $p)] | length' "$AG") - if [ "$registered" = "1" ]; then - ok 'Antigravity registers the managed agents dir, not the raw checkout' + # skills.json entries must point at $AGENTS_DIR — that carries the + # surfaces-filtering, collision handling and multi-source merge. Pointing at + # the checkout directly would give Antigravity a different skill set than + # every other harness. + f=$(new_fixture antigravity) + out=$(run_install "$f") + AG="$f/home/.gemini/config/skills.json" + if [ ! -f "$AG" ]; then + no 'Antigravity skills.json is created' "$out" else - no 'Antigravity registers the managed agents dir, not the raw checkout' "$(cat "$AG")" + registered=$(jq -r --arg p "$f/home/.agents/skills" '[.entries[]? | select(.path == $p)] | length' "$AG") + if [ "$registered" = "1" ]; then + ok 'Antigravity registers the managed agents dir, not the raw checkout' + else + no 'Antigravity registers the managed agents dir, not the raw checkout' "$(cat "$AG")" + fi + fi + + f=$(new_fixture antigravity_idempotent) + run_install "$f" >/dev/null + out=$(run_install "$f") + AG="$f/home/.gemini/config/skills.json" + count=$(jq '.entries | length' "$AG" 2>/dev/null || echo "?") + if [ "$count" = "1" ]; then + ok 'Antigravity wiring is idempotent (no duplicate entries)' + else + no 'Antigravity wiring is idempotent (no duplicate entries)' "entries=$count"$'\n'"$out" + fi + + # A user may hand-edit skills.json to point at their own personal skills dir + # (json_configs.md documents this). Our merge must not clobber it. + f=$(new_fixture antigravity_preserve) + mkdir -p "$f/home/.gemini/config" + printf '{"entries":[{"path":"/Users/mathias/personal-skills"}]}' > "$f/home/.gemini/config/skills.json" + out=$(run_install "$f") + AG="$f/home/.gemini/config/skills.json" + personal=$(jq -r '[.entries[]? | select(.path == "/Users/mathias/personal-skills")] | length' "$AG" 2>/dev/null || echo 0) + if [ "$personal" = "1" ]; then + ok 'Antigravity wiring preserves pre-existing hand-authored entries' + else + no 'Antigravity wiring preserves pre-existing hand-authored entries' "$(cat "$AG")"$'\n'"$out" fi fi -# --- 10. Antigravity wiring is idempotent ------------------------------------ -f=$(new_fixture antigravity_idempotent) -run_install "$f" >/dev/null -out=$(run_install "$f") -AG="$f/home/.gemini/config/skills.json" -count=$(jq '.entries | length' "$AG" 2>/dev/null || echo "?") -if [ "$count" = "1" ]; then - ok 'Antigravity wiring is idempotent (no duplicate entries)' -else - no 'Antigravity wiring is idempotent (no duplicate entries)' "entries=$count"$'\n'"$out" -fi - -# --- 11. Antigravity wiring preserves entries it doesn't own ----------------- -# A user may hand-edit skills.json to point at their own personal skills dir -# (json_configs.md documents this). Our merge must not clobber it. -f=$(new_fixture antigravity_preserve) -mkdir -p "$f/home/.gemini/config" -printf '{"entries":[{"path":"/Users/mathias/personal-skills"}]}' > "$f/home/.gemini/config/skills.json" -out=$(run_install "$f") -AG="$f/home/.gemini/config/skills.json" -personal=$(jq -r '[.entries[]? | select(.path == "/Users/mathias/personal-skills")] | length' "$AG" 2>/dev/null || echo 0) -if [ "$personal" = "1" ]; then - ok 'Antigravity wiring preserves pre-existing hand-authored entries' -else - no 'Antigravity wiring preserves pre-existing hand-authored entries' "$(cat "$AG")"$'\n'"$out" -fi - -printf '\ninstall: %d passed, %d failed\n' "$pass" "$fail" +printf '\ninstall: %d passed, %d failed, %d skipped\n' "$pass" "$fail" "$skipped" [ "$fail" -eq 0 ]