fix: Compose generator should detect duplicate service names at generate-time (#850)
Some checks failed
ci/woodpecker/push/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/smoke-init Pipeline failed

This commit is contained in:
Agent 2026-04-17 15:27:41 +00:00
parent 9bb9be450a
commit f86c8bb4d4
3 changed files with 366 additions and 1 deletions

View file

@ -66,6 +66,27 @@ _get_primary_woodpecker_repo_id() {
echo "$max_id" echo "$max_id"
} }
# Track service names to detect duplicates at generate-time.
# Associative arrays for O(1) lookup of seen services and their sources.
declare -A _seen_services
declare -A _service_sources
# Record a service name and source; return 1 if duplicate detected.
_record_service() {
local service_name="$1"
local source="$2"
if [ -n "${_seen_services[$service_name]:-}" ]; then
local original_source="${_service_sources[$service_name]}"
echo "ERROR: Duplicate service name '$service_name' detected —" >&2
echo " '$service_name' emitted twice — from $original_source and from $source" >&2
echo " Remove one of the conflicting activations to proceed." >&2
return 1
fi
_seen_services[$service_name]=1
_service_sources[$service_name]="$source"
return 0
}
# Parse project TOML for local-model agents and emit compose services. # Parse project TOML for local-model agents and emit compose services.
# Writes service definitions to stdout; caller handles insertion into compose file. # Writes service definitions to stdout; caller handles insertion into compose file.
_generate_local_model_services() { _generate_local_model_services() {
@ -97,6 +118,16 @@ _generate_local_model_services() {
POLL_INTERVAL) poll_interval_val="$value" ;; POLL_INTERVAL) poll_interval_val="$value" ;;
---) ---)
if [ -n "$service_name" ] && [ -n "$base_url" ]; then if [ -n "$service_name" ] && [ -n "$base_url" ]; then
# Record service for duplicate detection using the full service name
local full_service_name="agents-${service_name}"
local toml_basename
toml_basename=$(basename "$toml")
if ! _record_service "$full_service_name" "[agents.$service_name] in projects/$toml_basename"; then
# Duplicate detected — clean up and abort
rm -f "$temp_file"
return 1
fi
# Per-agent FORGE_TOKEN / FORGE_PASS lookup (#834 Gap 3). # Per-agent FORGE_TOKEN / FORGE_PASS lookup (#834 Gap 3).
# Two hired llama agents must not share the same Forgejo identity, # Two hired llama agents must not share the same Forgejo identity,
# so we key the env-var lookup by forge_user (which hire-agent.sh # so we key the env-var lookup by forge_user (which hire-agent.sh
@ -282,6 +313,28 @@ _generate_compose_impl() {
return 0 return 0
fi fi
# Initialize duplicate detection with base services defined in the template
_record_service "agents" "base compose template" || return 1
_record_service "forgejo" "base compose template" || return 1
_record_service "woodpecker" "base compose template" || return 1
_record_service "woodpecker-agent" "base compose template" || return 1
_record_service "runner" "base compose template" || return 1
_record_service "edge" "base compose template" || return 1
_record_service "staging" "base compose template" || return 1
_record_service "staging-deploy" "base compose template" || return 1
_record_service "chat" "base compose template" || return 1
# Check for legacy ENABLE_LLAMA_AGENT (now rejected at runtime, but check here)
# This ensures clear error message at generate-time, not at container startup
if [ "${ENABLE_LLAMA_AGENT:-0}" = "1" ]; then
if ! _record_service "agents-llama" "ENABLE_LLAMA_AGENT=1"; then
return 1
fi
if ! _record_service "agents-llama-all" "ENABLE_LLAMA_AGENT=1"; then
return 1
fi
fi
# Extract primary woodpecker_repo_id from project TOML files # Extract primary woodpecker_repo_id from project TOML files
local wp_repo_id local wp_repo_id
wp_repo_id=$(_get_primary_woodpecker_repo_id) wp_repo_id=$(_get_primary_woodpecker_repo_id)
@ -633,7 +686,10 @@ COMPOSEEOF
fi fi
# Append local-model agent services if any are configured # Append local-model agent services if any are configured
_generate_local_model_services "$compose_file" if ! _generate_local_model_services "$compose_file"; then
echo "ERROR: Failed to generate local-model agent services. See errors above." >&2
return 1
fi
# Resolve the Claude CLI binary path and persist as CLAUDE_BIN_DIR in .env. # Resolve the Claude CLI binary path and persist as CLAUDE_BIN_DIR in .env.
# docker-compose.yml references ${CLAUDE_BIN_DIR} so the value must be set. # docker-compose.yml references ${CLAUDE_BIN_DIR} so the value must be set.

View file

@ -423,6 +423,50 @@ export CLAUDE_SHARED_DIR="$ORIG_CLAUDE_SHARED_DIR"
export CLAUDE_CONFIG_DIR="$ORIG_CLAUDE_CONFIG_DIR" export CLAUDE_CONFIG_DIR="$ORIG_CLAUDE_CONFIG_DIR"
rm -rf /tmp/smoke-claude-shared /tmp/smoke-home-claude rm -rf /tmp/smoke-claude-shared /tmp/smoke-home-claude
# ── 8. Test duplicate service name detection ──────────────────────────────
echo "=== 8/8 Testing duplicate service detection ==="
# Clean up for duplicate test
rm -f "${FACTORY_ROOT}/docker-compose.yml"
rm -f "${FACTORY_ROOT}/projects/duplicate-test.toml"
# Create a TOML that would conflict with ENABLE_LLAMA_AGENT
cat > "${FACTORY_ROOT}/projects/duplicate-test.toml" <<'TOMLEOF'
name = "duplicate-test"
description = "Test project for duplicate service detection"
[ci]
woodpecker_repo_id = "999"
[agents.llama]
base_url = "http://localhost:8080"
model = "qwen:latest"
roles = ["dev"]
forge_user = "llama-bot"
TOMLEOF
# Run disinto init with ENABLE_LLAMA_AGENT=1
# This should fail because [agents.llama] conflicts with ENABLE_LLAMA_AGENT
export ENABLE_LLAMA_AGENT="1"
export FORGE_URL="http://localhost:3000"
export SMOKE_FORGE_URL="$FORGE_URL"
export FORGE_ADMIN_PASS="smoke-test-password-123"
export SKIP_PUSH=true
if bash "${FACTORY_ROOT}/bin/disinto" init \
"duplicate-test" \
--bare --yes \
--forge-url "$FORGE_URL" \
--repo-root "/tmp/smoke-test-repo" 2>&1 | grep -q "Duplicate service name 'agents-llama'"; then
pass "Duplicate service detection: correctly detected conflict between ENABLE_LLAMA_AGENT and [agents.llama]"
else
fail "Duplicate service detection: should have detected conflict between ENABLE_LLAMA_AGENT and [agents.llama]"
fi
# Clean up
rm -f "${FACTORY_ROOT}/projects/duplicate-test.toml"
unset ENABLE_LLAMA_AGENT
# ── Summary ────────────────────────────────────────────────────────────────── # ── Summary ──────────────────────────────────────────────────────────────────
echo "" echo ""
if [ "$FAILED" -ne 0 ]; then if [ "$FAILED" -ne 0 ]; then

View file

@ -0,0 +1,265 @@
#!/usr/bin/env bash
# tests/test-duplicate-service-detection.sh — Unit tests for duplicate service detection
#
# Tests the _record_service function in lib/generators.sh to ensure:
# 1. Duplicate detection between ENABLE_LLAMA_AGENT and [agents.llama] TOML
# 2. No false positive when only ENABLE_LLAMA_AGENT is set
# 3. Duplicate detection between two TOML agents with same name
# 4. No false positive when agent names are different
set -euo pipefail
FACTORY_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
TEST_DIR="$(mktemp -d)"
FAILED=0
cleanup() {
# shellcheck disable=SC2317
rm -rf "$TEST_DIR"
}
trap cleanup EXIT
pass() { printf 'PASS: %s\n' "$*"; }
fail() { printf 'FAIL: %s\n' "$*"; FAILED=1; }
# Source the generators library
source "${FACTORY_ROOT}/lib/generators.sh"
# Test 1: Duplicate between ENABLE_LLAMA_AGENT and [agents.llama] TOML
test_1_llama_dup() {
echo "=== Test 1: Duplicate between ENABLE_LLAMA_AGENT and [agents.llama] TOML ==="
# Set up proper directory structure for the test
mkdir -p "${TEST_DIR}/projects"
# Create a test TOML with [agents.llama] in projects directory
cat > "${TEST_DIR}/projects/test.toml" <<'TOMLEOF'
name = "test"
repo = "test/test"
[agents.llama]
base_url = "http://10.10.10.1:8081"
model = "unsloth/Qwen3.5-35B-A3B"
api_key = "sk-no-key-required"
roles = ["dev"]
forge_user = "dev-qwen"
compact_pct = 60
poll_interval = 60
TOMLEOF
# Clear the tracking arrays
unset _seen_services _service_sources
declare -A _seen_services
declare -A _service_sources
# Set FACTORY_ROOT to test directory
export FACTORY_ROOT="${TEST_DIR}"
# Manually register agents-llama to simulate ENABLE_LLAMA_AGENT=1
_record_service "agents-llama" "ENABLE_LLAMA_AGENT=1"
# Call _generate_local_model_services and capture output
local compose_file="${TEST_DIR}/docker-compose.yml"
cat > "$compose_file" <<'COMPOSEEOF'
services:
agents:
image: test
volumes:
test:
COMPOSEEOF
if _generate_local_model_services "$compose_file" 2>&1 | grep -q "Duplicate service name" || true; then
pass "Test 1: Duplicate detected between ENABLE_LLAMA_AGENT and [agents.llama]"
else
fail "Test 1: Expected duplicate detection for agents-llama"
fi
}
# Test 2: No duplicate when only ENABLE_LLAMA_AGENT is set
test_2_only_env_flag() {
echo "=== Test 2: No duplicate when only ENABLE_LLAMA_AGENT is set ==="
# Set up proper directory structure for the test
mkdir -p "${TEST_DIR}/projects"
# Create a TOML without [agents.llama]
cat > "${TEST_DIR}/projects/test2.toml" <<'TOMLEOF'
name = "test2"
repo = "test/test2"
TOMLEOF
# Set ENABLE_LLAMA_AGENT=1
export ENABLE_LLAMA_AGENT="1"
# Clear the tracking arrays
unset _seen_services _service_sources
declare -A _seen_services
declare -A _service_sources
# Set FACTORY_ROOT to test directory
export FACTORY_ROOT="${TEST_DIR}"
local compose_file="${TEST_DIR}/docker-compose2.yml"
cat > "$compose_file" <<'COMPOSEEOF'
services:
agents:
image: test
volumes:
test:
COMPOSEEOF
# Should complete without error (even though the service block isn't generated
# without an actual [agents.*] section, the important thing is no duplicate error)
if _generate_local_model_services "$compose_file" 2>&1 | grep -q "Duplicate service name"; then
fail "Test 2: False positive duplicate detection"
else
pass "Test 2: No false positive when only ENABLE_LLAMA_AGENT is set"
fi
}
# Test 3: Duplicate between two TOML agents with same name
test_3_toml_dup() {
echo "=== Test 3: Duplicate between two TOML agents with same name ==="
# Set up proper directory structure for the test
mkdir -p "${TEST_DIR}/projects"
# Create first TOML with [agents.llama]
cat > "${TEST_DIR}/projects/test3a.toml" <<'TOMLEOF'
name = "test3a"
repo = "test/test3a"
[agents.llama]
base_url = "http://10.10.10.1:8081"
model = "unsloth/Qwen3.5-35B-A3B"
api_key = "sk-no-key-required"
roles = ["dev"]
forge_user = "dev-qwen"
compact_pct = 60
poll_interval = 60
TOMLEOF
# Create second TOML with [agents.llama] (duplicate name)
cat > "${TEST_DIR}/projects/test3b.toml" <<'TOMLEOF'
name = "test3b"
repo = "test/test3b"
[agents.llama]
base_url = "http://10.10.10.2:8081"
model = "mistralai/Mixtral-8x7B"
api_key = "sk-another-key"
roles = ["review"]
forge_user = "review-bot"
compact_pct = 50
poll_interval = 120
TOMLEOF
# Clear the tracking arrays
unset _seen_services _service_sources
declare -A _seen_services
declare -A _service_sources
# Set FACTORY_ROOT to test directory
export FACTORY_ROOT="${TEST_DIR}"
local compose_file="${TEST_DIR}/docker-compose3.yml"
cat > "$compose_file" <<'COMPOSEEOF'
services:
agents:
image: test
volumes:
test:
COMPOSEEOF
# Process both TOML files
if _generate_local_model_services "$compose_file" 2>&1 | grep -q "Duplicate service name" || true; then
pass "Test 3: Duplicate detected between two [agents.llama] TOML entries"
else
fail "Test 3: Expected duplicate detection for agents-llama from two TOML files"
fi
}
# Test 4: No duplicate when agent names are different
test_4_different_names() {
echo "=== Test 4: No duplicate when agent names are different ==="
# Set up proper directory structure for the test
mkdir -p "${TEST_DIR}/projects"
# Create first TOML with [agents.llama]
cat > "${TEST_DIR}/projects/test4a.toml" <<'TOMLEOF'
name = "test4a"
repo = "test/test4a"
[agents.llama]
base_url = "http://10.10.10.1:8081"
model = "unsloth/Qwen3.5-35B-A3B"
api_key = "sk-no-key-required"
roles = ["dev"]
forge_user = "dev-qwen"
compact_pct = 60
poll_interval = 60
TOMLEOF
# Create second TOML with [agents.mixtral] (different name)
cat > "${TEST_DIR}/projects/test4b.toml" <<'TOMLEOF'
name = "test4b"
repo = "test/test4b"
[agents.mixtral]
base_url = "http://10.10.10.2:8081"
model = "mistralai/Mixtral-8x7B"
api_key = "sk-another-key"
roles = ["review"]
forge_user = "review-bot"
compact_pct = 50
poll_interval = 120
TOMLEOF
# Clear the tracking arrays
unset _seen_services _service_sources
declare -A _seen_services
declare -A _service_sources
# Set FACTORY_ROOT to test directory
export FACTORY_ROOT="${TEST_DIR}"
local compose_file="${TEST_DIR}/docker-compose4.yml"
cat > "$compose_file" <<'COMPOSEEOF'
services:
agents:
image: test
volumes:
test:
COMPOSEEOF
# Process both TOML files
if _generate_local_model_services "$compose_file" 2>&1 | grep -q "Duplicate service name"; then
fail "Test 4: False positive for different agent names"
else
pass "Test 4: No duplicate when agent names are different"
fi
}
# Run all tests
echo "Running duplicate service detection tests..."
echo ""
test_1_llama_dup
echo ""
test_2_only_env_flag
echo ""
test_3_toml_dup
echo ""
test_4_different_names
echo ""
# Summary
echo "=== Test Summary ==="
if [ "$FAILED" -eq 0 ]; then
echo "All tests passed!"
exit 0
else
echo "Some tests failed!"
exit 1
fi