From 070640f34c50066e3a0c1cc28c44166828e09edf Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 28 Sep 2026 00:56:56 -0400 Subject: [PATCH] fix(installer): guard shared paths for detected tools --- .github/workflows/check-tools.yml | 3 ++ scripts/install.sh | 38 +++++++------- scripts/test-install-all-path-collision.sh | 58 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 17 deletions(-) create mode 100644 scripts/test-install-all-path-collision.sh diff --git a/.github/workflows/check-tools.yml b/.github/workflows/check-tools.yml index ec13b3ec..bc6d670f 100644 --- a/.github/workflows/check-tools.yml +++ b/.github/workflows/check-tools.yml @@ -32,3 +32,6 @@ jobs: - name: Validate agent selection (install.sh --agent / --agents-file) run: bash scripts/test-agent-selection.sh + + - name: Validate shared destinations for detected tools + run: bash scripts/test-install-all-path-collision.sh diff --git a/scripts/install.sh b/scripts/install.sh index 50f46893..ef9cdef8 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -299,6 +299,24 @@ path_collision_group() { esac } +# Validate after tool selection so --tool all and the interactive picker get +# the same protection as an explicit comma-separated list. +validate_path_collisions() { + [[ -n "$OVERRIDE_PATH" && $# -gt 1 ]] || return 0 + local _ta _tb _ga _gb + for _ta in "$@"; do + _ga="$(path_collision_group "$_ta")"; [[ -z "$_ga" ]] && continue + for _tb in "$@"; 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)." + return 1 + fi + done + done +} + resolve_dest() { local tool="$1" def="$2" var="" [[ -n "$OVERRIDE_PATH" ]] && { printf '%s' "$OVERRIDE_PATH"; return; } @@ -1411,23 +1429,6 @@ main() { $duplicate || _cleaned+=("$_t") done _tool_list=("${_cleaned[@]}") - # --path is one shared directory. Tools that write the same filenames into - # 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 - 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 - fi - done - done - fi fi # Decide whether to show interactive UI @@ -1469,6 +1470,9 @@ main() { exit 0 fi + # --tool all and the interactive wizard only know their selected tools now. + validate_path_collisions "${SELECTED_TOOLS[@]}" + # --dry-run: print the plan and exit without writing anything. if $DRY_RUN; then local agents; agents="$(selected_agent_count)" diff --git a/scripts/test-install-all-path-collision.sh b/scripts/test-install-all-path-collision.sh new file mode 100644 index 00000000..b0c6a40e --- /dev/null +++ b/scripts/test-install-all-path-collision.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# Auto-detected tools must receive the same --path collision guard as explicit lists. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +tmp="$(mktemp -d "${TMPDIR:-/tmp}/agency-all-path.XXXXXX")" +trap 'rm -rf "$tmp"' EXIT +mkdir -p "$tmp/repo/scripts" "$tmp/repo/engineering" \ + "$tmp/repo/integrations/gemini-cli/agents" "$tmp/repo/integrations/qwen/agents" \ + "$tmp/home/.gemini" "$tmp/home/.qwen" +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: Shared Agent +description: Example agent +--- +Instructions. +EOF +printf 'Gemini output\n' > "$tmp/repo/integrations/gemini-cli/agents/shared-agent.md" +printf 'Qwen output\n' > "$tmp/repo/integrations/qwen/agents/shared-agent.md" + +# Both detected tools write shared-agent.md to the same --path. Before the +# guard, Qwen silently replaced Gemini's file and the install exited zero. +if HOME="$tmp/home" PATH=/usr/bin:/bin bash "$tmp/repo/scripts/install.sh" \ + --no-interactive --tool all --path "$tmp/dest" --no-convert > "$tmp/output" 2>&1; then + echo 'FAIL: --tool all accepted colliding per-agent outputs' >&2 + exit 1 +fi +if [[ -e "$tmp/dest/shared-agent.md" ]]; then + echo 'FAIL: collision was detected after writing output' >&2 + exit 1 +fi +grep -q 'would overwrite each other' "$tmp/output" || { + echo 'FAIL: collision did not explain the conflicting tools' >&2 + exit 1 +} +echo 'PASS: --tool all rejects a shared destination collision before writing' + +# Distinct filenames can safely share the override directory. +mkdir -p "$tmp/repo/integrations/codex/agents" "$tmp/compatible-home/.gemini" "$tmp/compatible-home/.codex" +printf 'name = "Shared Agent"\n' > "$tmp/repo/integrations/codex/agents/shared-agent.toml" +if ! HOME="$tmp/compatible-home" PATH=/usr/bin:/bin bash "$tmp/repo/scripts/install.sh" \ + --no-interactive --tool all --path "$tmp/compatible-dest" --no-convert > "$tmp/compatible-output" 2>&1; then + echo 'FAIL: --tool all rejected tools with distinct output filenames' >&2 + exit 1 +fi +[[ -f "$tmp/compatible-dest/shared-agent.md" && -f "$tmp/compatible-dest/shared-agent.toml" ]] || { + echo 'FAIL: compatible tools did not both install' >&2 + exit 1 +} +echo 'PASS: --tool all permits compatible tools to share a destination'