From c507cc12846693bd6b3dd23ad3d16f905325d987 Mon Sep 17 00:00:00 2001 From: fujibee Date: Fri, 7 Aug 2026 14:39:37 -0700 Subject: [PATCH 1/3] fix(install): refuse to guess between multiple installs on --update with no --cmd The skill-directory selection loop broke on the first .agmsg- marked directory a glob yielded. A glob expands in collation order, not installation order, and nothing records which install came first, so the "historical first installed" comment described an invariant the code could not observe. On a machine with more than one install, --update silently touched whichever one sorted first -- including the shared ~/.agents/bin/codex shim it refreshes -- while the intended install, and the caller, saw nothing wrong. Enumerate all .agmsg-marked directories instead of stopping at the first. Zero or one candidate behaves exactly as before -- the common single-install case is unaffected. Two or more now fails closed: list the candidates and ask for --cmd, rather than guessing. Backup-shaped directory names (*.bak*) are excluded from the candidate set, since they carry the same .agmsg marker as a real install but are never the one a bare --update means. Fixes #599. --- install.sh | 32 +++++++++++++++++++++++++------- tests/test_install.bats | 27 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/install.sh b/install.sh index d6d7d07c8..69211acd8 100755 --- a/install.sh +++ b/install.sh @@ -221,8 +221,15 @@ 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". Backup-shaped + # directory names are skipped: they carry the same .agmsg marker as a + # real install but are never the one an unqualified --update means. if [ -n "$CMD_NAME" ]; then SKILL_DIR="$AGENTS_DIR/skills/$CMD_NAME" if [ ! -f "$SKILL_DIR/.agmsg" ]; then @@ -230,13 +237,24 @@ 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%/}" + case "$(basename "$d")" in *.bak*) continue ;; esac + [ -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..181a24b02 100644 --- a/tests/test_install.bats +++ b/tests/test_install.bats @@ -85,6 +85,33 @@ 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 + + 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. + [ "$(cat "$FAKE_HOME/.agents/skills/agmsg/VERSION" 2>/dev/null)" = "$(cat "$FAKE_HOME/.agents/skills/agmsg-second/VERSION" 2>/dev/null)" ] +} + +@test "install: --update with no --cmd still finds the one real install past a bak-named decoy (#599)" { + HOME="$FAKE_HOME" bash "$REPO_ROOT/install.sh" --cmd agmsg + local decoy="$FAKE_HOME/.agents/skills/agmsg.bak-20260731" + mkdir -p "$decoy/scripts" "$decoy/templates" "$decoy/db" "$decoy/agents" + touch "$decoy/.agmsg" + echo "decoy sentinel" > "$decoy/SKILL.md" + + run env HOME="$FAKE_HOME" AGMSG_FORCE_WINDOWS=1 bash "$REPO_ROOT/install.sh" --update + [ "$status" -eq 0 ] + [[ "$output" =~ "Updating agmsg..." ]] + [[ ! "$output" =~ "Updating agmsg.bak-20260731" ]] + grep -q "decoy sentinel" "$decoy/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 From 7eb076fecea7fdbf3bd3e1615861e16614305fb2 Mon Sep 17 00:00:00 2001 From: fujibee Date: Fri, 7 Aug 2026 14:44:21 -0700 Subject: [PATCH 2/3] fix(install): narrow the backup-directory exclusion, add a missed-owner test co2 review on #659: the "*.bak*" exclusion pattern also matched a real install whose --cmd name merely contains "bak" (e.g. "agmsg.bakery") -- --cmd has no reserved-name validation, so that install silently dropped out of the candidate set, reproducing the same silent-wrong-pick failure #599 exists to close. Narrow the pattern to the literal ".bak-" shape actually reported (agmsg.bak-20260731), which does not collide with a plausible command name. Also tighten the two-installs regression test: it was comparing VERSION strings, which are the same source-derived value for both installs and would not distinguish "one got silently updated" from "neither did". Compare distinct per-install sentinels instead. --- install.sh | 10 ++++++++-- tests/test_install.bats | 22 +++++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/install.sh b/install.sh index 69211acd8..8459956cc 100755 --- a/install.sh +++ b/install.sh @@ -229,7 +229,13 @@ if [ "$UPDATE_ONLY" = true ]; then # shim it refreshes) gets updated (#599). A single install is unaffected # -- this is the common case and it still "just works". Backup-shaped # directory names are skipped: they carry the same .agmsg marker as a - # real install but are never the one an unqualified --update means. + # real install but are never the one an unqualified --update means. The + # pattern is deliberately narrow (the literal ".bak-" shape reported in + # #599 and #599's comments, e.g. "agmsg.bak-20260731") rather than a bare + # "*.bak*": --cmd has no reserved-name validation, so a broader pattern + # would silently drop a real install whose chosen name merely contains + # "bak" (e.g. "agmsg.bakery") -- the same silent-wrong-pick failure this + # fix exists to close, just via exclusion instead of glob order. if [ -n "$CMD_NAME" ]; then SKILL_DIR="$AGENTS_DIR/skills/$CMD_NAME" if [ ! -f "$SKILL_DIR/.agmsg" ]; then @@ -240,7 +246,7 @@ if [ "$UPDATE_ONLY" = true ]; then candidates=() for d in "$AGENTS_DIR"/skills/*/; do d="${d%/}" - case "$(basename "$d")" in *.bak*) continue ;; esac + case "$(basename "$d")" in *.bak-*) continue ;; esac [ -f "$d/.agmsg" ] && candidates+=("$d") done case "${#candidates[@]}" in diff --git a/tests/test_install.bats b/tests/test_install.bats index 181a24b02..e08f1b848 100644 --- a/tests/test_install.bats +++ b/tests/test_install.bats @@ -88,6 +88,11 @@ teardown() { @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 ] @@ -95,7 +100,8 @@ teardown() { [[ "$output" =~ "agmsg" ]] [[ "$output" =~ "agmsg-second" ]] # Neither install was touched -- this is a refusal, not a guess. - [ "$(cat "$FAKE_HOME/.agents/skills/agmsg/VERSION" 2>/dev/null)" = "$(cat "$FAKE_HOME/.agents/skills/agmsg-second/VERSION" 2>/dev/null)" ] + 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 still finds the one real install past a bak-named decoy (#599)" { @@ -112,6 +118,20 @@ teardown() { grep -q "decoy sentinel" "$decoy/SKILL.md" } +@test "install: --update with no --cmd is not fooled by a legitimate install name that merely contains \"bak\" (#599)" { + # The exclusion pattern must match the reported backup shape (".bak-...") + # and nothing broader: --cmd has no reserved-name validation, so a real + # install can legally be named e.g. "agmsg.bakery" (co2 review, #659). If + # the exclusion were a bare "*.bak*" this install would be silently + # skipped, leaving zero candidates and reporting "Not installed" for an + # install that is, in fact, installed. + HOME="$FAKE_HOME" bash "$REPO_ROOT/install.sh" --cmd agmsg.bakery + + run env HOME="$FAKE_HOME" AGMSG_FORCE_WINDOWS=1 bash "$REPO_ROOT/install.sh" --update + [ "$status" -eq 0 ] + [[ "$output" =~ "Updating agmsg.bakery..." ]] +} + @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 From d11c72e1a3468c8db584739eff1605e78bc32ee1 Mon Sep 17 00:00:00 2001 From: fujibee Date: Fri, 7 Aug 2026 14:49:56 -0700 Subject: [PATCH 3/3] fix(install): drop the backup-name exclusion entirely, pure fail closed Two rounds of narrowing ("*.bak*", then "*.bak-*") hit the same collision: --cmd has no reserved-name validation, so any pattern that catches a real backup name (e.g. "agmsg.bak-20260731") can equally match a legitimately chosen install name (e.g. "agmsg.bak-tool" or "agmsg.bakery"), silently dropping a real install from the candidate set -- the exact failure #599 exists to close, just moved into the exclusion instead of the glob order. No code in this repo generates a ".bak-"-named directory; the reported name is a human backup convention, not something the installer produces, so no name-based pattern can be made safe here. Stop special-casing names. A directory that still carries the .agmsg marker is just another candidate; more than one candidate is exactly the ambiguity this fix already refuses to guess through. The regression test now shows a leftover backup-shaped directory landing in the same "Several agmsg installs found" refusal as any other second candidate, rather than being silently excluded or silently chosen. --- install.sh | 20 +++++++++--------- tests/test_install.bats | 47 ++++++++++++++++++++--------------------- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/install.sh b/install.sh index 8459956cc..390609aed 100755 --- a/install.sh +++ b/install.sh @@ -227,15 +227,16 @@ if [ "$UPDATE_ONLY" = true ]; then # 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". Backup-shaped - # directory names are skipped: they carry the same .agmsg marker as a - # real install but are never the one an unqualified --update means. The - # pattern is deliberately narrow (the literal ".bak-" shape reported in - # #599 and #599's comments, e.g. "agmsg.bak-20260731") rather than a bare - # "*.bak*": --cmd has no reserved-name validation, so a broader pattern - # would silently drop a real install whose chosen name merely contains - # "bak" (e.g. "agmsg.bakery") -- the same silent-wrong-pick failure this - # fix exists to close, just via exclusion instead of glob order. + # -- 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 @@ -246,7 +247,6 @@ if [ "$UPDATE_ONLY" = true ]; then candidates=() for d in "$AGENTS_DIR"/skills/*/; do d="${d%/}" - case "$(basename "$d")" in *.bak-*) continue ;; esac [ -f "$d/.agmsg" ] && candidates+=("$d") done case "${#candidates[@]}" in diff --git a/tests/test_install.bats b/tests/test_install.bats index e08f1b848..5e7773dff 100644 --- a/tests/test_install.bats +++ b/tests/test_install.bats @@ -104,32 +104,31 @@ teardown() { grep -q "agmsg-second sentinel" "$FAKE_HOME/.agents/skills/agmsg-second/SKILL.md" } -@test "install: --update with no --cmd still finds the one real install past a bak-named decoy (#599)" { - HOME="$FAKE_HOME" bash "$REPO_ROOT/install.sh" --cmd agmsg - local decoy="$FAKE_HOME/.agents/skills/agmsg.bak-20260731" - mkdir -p "$decoy/scripts" "$decoy/templates" "$decoy/db" "$decoy/agents" - touch "$decoy/.agmsg" - echo "decoy sentinel" > "$decoy/SKILL.md" - - run env HOME="$FAKE_HOME" AGMSG_FORCE_WINDOWS=1 bash "$REPO_ROOT/install.sh" --update - [ "$status" -eq 0 ] - [[ "$output" =~ "Updating agmsg..." ]] - [[ ! "$output" =~ "Updating agmsg.bak-20260731" ]] - grep -q "decoy sentinel" "$decoy/SKILL.md" -} - -@test "install: --update with no --cmd is not fooled by a legitimate install name that merely contains \"bak\" (#599)" { - # The exclusion pattern must match the reported backup shape (".bak-...") - # and nothing broader: --cmd has no reserved-name validation, so a real - # install can legally be named e.g. "agmsg.bakery" (co2 review, #659). If - # the exclusion were a bare "*.bak*" this install would be silently - # skipped, leaving zero candidates and reporting "Not installed" for an - # install that is, in fact, installed. - HOME="$FAKE_HOME" bash "$REPO_ROOT/install.sh" --cmd agmsg.bakery +@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" -eq 0 ] - [[ "$output" =~ "Updating agmsg.bakery..." ]] + [ "$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)" {