diff --git a/install.sh b/install.sh index d6d7d07c8..390609aed 100755 --- a/install.sh +++ b/install.sh @@ -221,8 +221,22 @@ echo "" # --- Update mode --- if [ "$UPDATE_ONLY" = true ]; then - # Find existing install. If --cmd was passed, update exactly that skill; - # otherwise preserve the historical "first installed agmsg skill" behavior. + # Find existing install. If --cmd was passed, update exactly that skill. + # Otherwise, scan for installs and require exactly one: a glob expands in + # collation order, not installation order, and nothing records which + # install came first, so guessing from a list of more than one is a + # silent coin flip on which install (and the shared ~/.agents/bin/codex + # shim it refreshes) gets updated (#599). A single install is unaffected + # -- this is the common case and it still "just works". + # + # No name-based exclusion for backup-shaped directories: --cmd has no + # reserved-name validation, so any pattern that would catch a real backup + # (e.g. "agmsg.bak-20260731") can equally match a legitimately chosen + # install name (e.g. "agmsg.bak-tool") -- there is no substring that is + # guaranteed to mean "not a real install" (co2 review, #659). A leftover + # backup directory that still carries the .agmsg marker is therefore just + # another candidate: it makes the set ambiguous, and ambiguous is exactly + # what this fix already refuses to guess through, below. if [ -n "$CMD_NAME" ]; then SKILL_DIR="$AGENTS_DIR/skills/$CMD_NAME" if [ ! -f "$SKILL_DIR/.agmsg" ]; then @@ -230,13 +244,23 @@ if [ "$UPDATE_ONLY" = true ]; then exit 1 fi else - SKILL_DIR="" + candidates=() for d in "$AGENTS_DIR"/skills/*/; do - if [ -f "${d}.agmsg" ]; then - SKILL_DIR="${d%/}" - break - fi + d="${d%/}" + [ -f "$d/.agmsg" ] && candidates+=("$d") done + case "${#candidates[@]}" in + 0) SKILL_DIR="" ;; + 1) SKILL_DIR="${candidates[0]}" ;; + *) + echo " ! Several agmsg installs found:" >&2 + for d in "${candidates[@]}"; do + echo " $(basename "$d")" >&2 + done + echo " ! --update with no --cmd cannot tell which one you mean. Pass --cmd to pick one." >&2 + exit 1 + ;; + esac fi if [ -z "$SKILL_DIR" ]; then echo " ! Not installed. Run ./install.sh first." >&2 diff --git a/tests/test_install.bats b/tests/test_install.bats index 5b011488d..5e7773dff 100644 --- a/tests/test_install.bats +++ b/tests/test_install.bats @@ -85,6 +85,52 @@ teardown() { grep -q "backup sentinel" "$backup/SKILL.md" } +@test "install: --update with no --cmd refuses to guess between two real installs (#599)" { + HOME="$FAKE_HOME" bash "$REPO_ROOT/install.sh" --cmd agmsg + HOME="$FAKE_HOME" bash "$REPO_ROOT/install.sh" --cmd agmsg-second + # Distinct per-install sentinels, not just each install's VERSION (which is + # the same source-derived string for both and would not distinguish "one of + # them got silently updated" from "neither did" -- co2 review, #659). + echo "agmsg sentinel" > "$FAKE_HOME/.agents/skills/agmsg/SKILL.md" + echo "agmsg-second sentinel" > "$FAKE_HOME/.agents/skills/agmsg-second/SKILL.md" + + run env HOME="$FAKE_HOME" AGMSG_FORCE_WINDOWS=1 bash "$REPO_ROOT/install.sh" --update + [ "$status" -ne 0 ] + [[ "$output" =~ "Several agmsg installs found" ]] + [[ "$output" =~ "agmsg" ]] + [[ "$output" =~ "agmsg-second" ]] + # Neither install was touched -- this is a refusal, not a guess. + grep -q "agmsg sentinel" "$FAKE_HOME/.agents/skills/agmsg/SKILL.md" + grep -q "agmsg-second sentinel" "$FAKE_HOME/.agents/skills/agmsg-second/SKILL.md" +} + +@test "install: --update with no --cmd treats a leftover backup-shaped directory as another candidate, not a silent exclusion (#599)" { + # No code in this repo creates a ".bak-"-named directory -- that name is a + # human backup convention, not something install.sh generates. A pattern + # narrow enough to exclude it is therefore also narrow enough to still + # exclude nothing on a real machine, while remaining broad enough to + # collide with a legitimately chosen --cmd name (--cmd has no reserved-name + # validation: "agmsg.bak-tool" installs today with no error). Two rounds of + # narrowing hit that same collision from co2 review on #659; the fix is to + # not special-case names at all. A directory that still carries the .agmsg + # marker is just another candidate, and more than one candidate is exactly + # the ambiguity this fix already refuses to guess through. + HOME="$FAKE_HOME" bash "$REPO_ROOT/install.sh" --cmd agmsg + local leftover="$FAKE_HOME/.agents/skills/agmsg.bak-20260731" + mkdir -p "$leftover/scripts" "$leftover/templates" "$leftover/db" "$leftover/agents" + touch "$leftover/.agmsg" + echo "leftover sentinel" > "$leftover/SKILL.md" + echo "agmsg sentinel" > "$FAKE_HOME/.agents/skills/agmsg/SKILL.md" + + run env HOME="$FAKE_HOME" AGMSG_FORCE_WINDOWS=1 bash "$REPO_ROOT/install.sh" --update + [ "$status" -ne 0 ] + [[ "$output" =~ "Several agmsg installs found" ]] + [[ "$output" =~ "agmsg" ]] + [[ "$output" =~ "agmsg.bak-20260731" ]] + grep -q "agmsg sentinel" "$FAKE_HOME/.agents/skills/agmsg/SKILL.md" + grep -q "leftover sentinel" "$leftover/SKILL.md" +} + @test "install: Claude Code command file gates actas/drop's fresh Monitor on delivery mode (#280)" { # actas/drop used to invoke a fresh Monitor unconditionally, ignoring # mode=off/turn (#280) — this is prompt-instruction text, not executable