mirror of
https://github.com/msitarzewski/agency-agents.git
synced 2026-09-04 01:00:51 +03:00
fix(install): refuse --path only for tools that would overwrite each other; re-land installer test suite (#772)
Re-lands the install.sh regression suite from #772 (reverted in #827) together with the guard change that makes it pass, so CI goes green in one step. Background. #825 made --path refuse more than one --tool, on #819's report that several tools sharing one destination clobber each other. That was right in spirit and over-broad in practice, and it was implemented without verifying the premise. Measured by installing one agent with every tool into a sandbox and comparing what landed: <division>-<slug>.md (raw copy) claude-code, copilot <slug>.md (converted) gemini-cli, opencode, qwen, zcode agency-<slug>/SKILL.md antigravity, osaurus Tools in the same group write identical filenames and silently overwrite each other (qwen + gemini-cli lose a file while both print [OK]). Tools in different groups coexist (claude-code + codex, claude-code + qwen). Every other tool's output is distinct. The guard now refuses --path only for a colliding pair, naming both tools and the reason, and allows the rest. path_collision_group() holds the measured table; re-measure if a converter's naming changes. The suite's two-tool --path cases used claude-code + copilot, which collide: the "installs exactly one agent" count of 1 was passing because copilot had overwritten claude-code's identical file. They now use claude-code + codex and assert that BOTH outputs survive, which a single count cannot show; a new case asserts the colliding pair is refused. codex has no committed output, so those cases convert (only the two raw-copiers work under --no-convert in a fresh checkout). Verified under bash 3.2 (the macOS CI leg): 29 passed, 0 failed, 1 xfail. Guard spot-checked against all three measured groups plus a cross-group pair. Suite, workflow and CONTRIBUTING note by @SergiorCode (#772). Refs #772 #819 #825 #827. Co-Authored-By: SergiorCode <SergiorCode@users.noreply.github.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
SergiorCode
Claude Fable 5.1
parent
6ec263ae19
commit
fa0ac650e0
+28
-3
@@ -283,6 +283,20 @@ install_file() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
# resolve_dest <tool> <default> — --path > $ENV_VAR > default.
|
# resolve_dest <tool> <default> — --path > $ENV_VAR > default.
|
||||||
|
# path_collision_group <tool> — tools in the same group write identical
|
||||||
|
# filenames into a shared --path and would overwrite each other; empty means
|
||||||
|
# the tool's output is distinct and may share a path with anything. Derived by
|
||||||
|
# installing one agent with every tool into a sandbox and comparing what
|
||||||
|
# landed; re-measure if a converter's output naming changes.
|
||||||
|
path_collision_group() {
|
||||||
|
case "$1" in
|
||||||
|
claude-code|copilot) printf 'raw-source-md' ;; # <division>-<slug>.md
|
||||||
|
gemini-cli|opencode|qwen|zcode) printf 'slug-md' ;; # <slug>.md
|
||||||
|
antigravity|osaurus) printf 'agency-skill' ;; # agency-<slug>/SKILL.md
|
||||||
|
*) printf '' ;;
|
||||||
|
esac
|
||||||
|
}
|
||||||
|
|
||||||
resolve_dest() {
|
resolve_dest() {
|
||||||
local tool="$1" def="$2" var=""
|
local tool="$1" def="$2" var=""
|
||||||
[[ -n "$OVERRIDE_PATH" ]] && { printf '%s' "$OVERRIDE_PATH"; return; }
|
[[ -n "$OVERRIDE_PATH" ]] && { printf '%s' "$OVERRIDE_PATH"; return; }
|
||||||
@@ -1258,12 +1272,23 @@ main() {
|
|||||||
$duplicate || _cleaned+=("$_t")
|
$duplicate || _cleaned+=("$_t")
|
||||||
done
|
done
|
||||||
_tool_list=("${_cleaned[@]}")
|
_tool_list=("${_cleaned[@]}")
|
||||||
# --path is a single-destination override; with several tools every one of
|
# --path is one shared directory. Tools that write the same filenames into
|
||||||
# them would land in the same directory and clobber each other.
|
# it silently overwrite each other; tools with distinct outputs coexist.
|
||||||
|
# Refuse only the colliding combinations (see path_collision_group).
|
||||||
if [[ -n "$OVERRIDE_PATH" && ${#_tool_list[@]} -gt 1 ]]; then
|
if [[ -n "$OVERRIDE_PATH" && ${#_tool_list[@]} -gt 1 ]]; then
|
||||||
err "--path sets ONE destination; use it with exactly one --tool (got ${#_tool_list[@]}: ${_tool_list[*]})."
|
local _ta _tb _ga _gb
|
||||||
|
for _ta in "${_tool_list[@]}"; do
|
||||||
|
_ga="$(path_collision_group "$_ta")"; [[ -z "$_ga" ]] && continue
|
||||||
|
for _tb in "${_tool_list[@]}"; do
|
||||||
|
[[ "$_tb" == "$_ta" ]] && continue
|
||||||
|
_gb="$(path_collision_group "$_tb")"
|
||||||
|
if [[ "$_ga" == "$_gb" ]]; then
|
||||||
|
err "--path is one shared directory, and $_ta and $_tb write the same filenames into it — they would overwrite each other. Use one of them per --path (tools with distinct outputs may share one)."
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
done
|
||||||
|
done
|
||||||
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# Decide whether to show interactive UI
|
# Decide whether to show interactive UI
|
||||||
|
|||||||
+23
-4
@@ -224,19 +224,38 @@ assert_eq 0 "$(find "$home" -maxdepth 1 -name 'My' -o -maxdepth 1 -name 'Agents'
|
|||||||
echo ""
|
echo ""
|
||||||
echo "parallel workers"
|
echo "parallel workers"
|
||||||
|
|
||||||
|
# Two tools that write the same filenames into one shared --path would silently
|
||||||
|
# overwrite each other (claude-code and copilot both copy the raw source as
|
||||||
|
# <division>-<slug>.md). The installer must refuse, not clobber. Both tools work
|
||||||
|
# under --no-convert, which keeps this case cheap in CI.
|
||||||
|
home="$(sandbox path-collision)"
|
||||||
|
dest="$home/My [Agents]/dest dir"
|
||||||
|
run_install "$home" --tool claude-code,copilot --no-convert --agent "$FIRST_ENG_SLUG" --path "$dest"
|
||||||
|
assert_eq 1 "$RUN_STATUS" "two tools that write the same filenames into one --path are refused"
|
||||||
|
assert_eq 1 "$(printf '%s' "$RUN_OUT" | grep -c 'overwrite')" "the refusal explains the collision"
|
||||||
|
assert_eq 0 "$(count_md "$dest")" "a refused install writes nothing"
|
||||||
|
|
||||||
|
# The propagation cases below therefore use a NON-colliding pair: claude-code
|
||||||
|
# writes <division>-<slug>.md and codex writes <slug>.toml, so BOTH outputs must
|
||||||
|
# survive in the shared --path — which is a stronger check than one tool's count
|
||||||
|
# alone (a count of 1 cannot tell "two wrote, one clobbered" from "one wrote").
|
||||||
|
# codex has no committed output (integrations/ is generated and gitignored), so
|
||||||
|
# these cases let the installer convert.
|
||||||
home="$(sandbox parallel-serial-control)"
|
home="$(sandbox parallel-serial-control)"
|
||||||
dest="$home/My [Agents]/dest dir"
|
dest="$home/My [Agents]/dest dir"
|
||||||
list="$home/my agents list.txt"
|
list="$home/my agents list.txt"
|
||||||
{ echo "# same selection as the parallel case below"; echo "$FIRST_ENG_SLUG"; } > "$list"
|
{ echo "# same selection as the parallel case below"; echo "$FIRST_ENG_SLUG"; } > "$list"
|
||||||
run_install "$home" --tool claude-code,copilot --no-convert --agents-file "$list" --path "$dest"
|
run_install "$home" --tool claude-code,codex --agents-file "$list" --path "$dest"
|
||||||
assert_eq 0 "$RUN_STATUS" "serial control: two tools, spaced/globbed --path, exits 0"
|
assert_eq 0 "$RUN_STATUS" "serial control: two non-colliding tools, spaced/globbed --path, exits 0"
|
||||||
assert_eq 1 "$(count_md "$dest")" "serial control: installs exactly the one selected agent"
|
assert_eq 1 "$(count_md "$dest")" "serial control: claude-code installs exactly the one selected agent"
|
||||||
|
assert_eq 1 "$(find "$dest" -maxdepth 1 -name '*.toml' -type f 2>/dev/null | wc -l | tr -d ' ')" \
|
||||||
|
"serial control: codex's output survives alongside claude-code's"
|
||||||
|
|
||||||
home="$(sandbox parallel)"
|
home="$(sandbox parallel)"
|
||||||
dest="$home/My [Agents]/dest dir"
|
dest="$home/My [Agents]/dest dir"
|
||||||
list="$home/my agents list.txt"
|
list="$home/my agents list.txt"
|
||||||
{ echo "# one agent, listed in a file whose own path has spaces"; echo "$FIRST_ENG_SLUG"; } > "$list"
|
{ echo "# one agent, listed in a file whose own path has spaces"; echo "$FIRST_ENG_SLUG"; } > "$list"
|
||||||
run_install "$home" --tool claude-code,copilot --parallel --jobs 1 --no-convert --agents-file "$list" --path "$dest"
|
run_install "$home" --tool claude-code,codex --parallel --jobs 1 --agents-file "$list" --path "$dest"
|
||||||
assert_eq 0 "$RUN_STATUS" "--parallel with a spaced/globbed --path exits 0"
|
assert_eq 0 "$RUN_STATUS" "--parallel with a spaced/globbed --path exits 0"
|
||||||
xfail_eq 1 "$(count_md "$dest")" \
|
xfail_eq 1 "$(count_md "$dest")" \
|
||||||
"--parallel installs exactly the one selected agent (spaced --path + --agents-file)" "PR #755"
|
"--parallel installs exactly the one selected agent (spaced --path + --agents-file)" "PR #755"
|
||||||
|
|||||||
Reference in New Issue
Block a user