fix: Compose generator should detect duplicate service names at generate-time (#850)

Key `FORGE_BOT_USER_*` on `$user_upper` (forge_user normalized with
`tr 'a-z-' 'A-Z_'`) instead of `${service_name^^}`, matching the
`FORGE_TOKEN_<FORGE_USER>` / `FORGE_PASS_<FORGE_USER>` convention two
lines above in the same emitted block. For `[agents.llama]` with
`forge_user = "dev-qwen"` this emits `FORGE_BOT_USER_DEV_QWEN: "dev-qwen"`
instead of the legacy `FORGE_BOT_USER_LLAMA`.

No external consumers read `FORGE_BOT_USER_*` today (verified via grep),
so no fallback/deprecation shim is needed — this is purely a one-site
fix at the sole producer.

Adds `tests/lib-generators.bats` as a regression guard. Follows the
existing `tests/lib-*.bats` pattern (developer-run, not CI-wired).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

lib/generators.sh

@@ -66,6 +66,28 @@ _get_primary_woodpecker_repo_id() {
   echo "$max_id"
 }
 
+# Track service names for duplicate detection
+declare -A _seen_services
+declare -A _service_sources
+
+# Record a service name and its source; return 0 if unique, 1 if duplicate
+_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.
 # Writes service definitions to stdout; caller handles insertion into compose file.
 _generate_local_model_services() {
@@ -97,6 +119,16 @@ _generate_local_model_services() {
         POLL_INTERVAL) poll_interval_val="$value" ;;
         ---)
           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).
             # 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
@@ -109,11 +141,14 @@ _generate_local_model_services() {
             # the service caused `disinto up` (without COMPOSE_PROFILES)
             # to treat the hired container as an orphan and silently
             # remove it via --remove-orphans.
+            # Compute the actual service name that will be emitted
+            local full_service_name="agents-${service_name}"
+
             local user_upper
             user_upper=$(echo "$forge_user" | tr 'a-z-' 'A-Z_')
             cat >> "$temp_file" <<EOF
 
-  agents-${service_name}:
+  ${full_service_name}:
     image: ghcr.io/disinto/agents:\${DISINTO_IMAGE_TAG:-latest}
     container_name: disinto-agents-${service_name}
     restart: unless-stopped
@@ -149,7 +184,7 @@ _generate_local_model_services() {
       PROJECT_REPO_ROOT: /home/agent/repos/${PROJECT_NAME:-project}
       WOODPECKER_DATA_DIR: /woodpecker-data
       WOODPECKER_REPO_ID: "${wp_repo_id}"
-      FORGE_BOT_USER_${service_name^^}: "${forge_user}"
+      FORGE_BOT_USER_${user_upper}: "${forge_user}"
       POLL_INTERVAL: "${poll_interval_val}"
       GARDENER_INTERVAL: "${GARDENER_INTERVAL:-21600}"
       ARCHITECT_INTERVAL: "${ARCHITECT_INTERVAL:-21600}"
@@ -265,6 +300,17 @@ _generate_compose_impl() {
     return 0
   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
+
   # Extract primary woodpecker_repo_id from project TOML files
   local wp_repo_id
   wp_repo_id=$(_get_primary_woodpecker_repo_id)
@@ -425,6 +471,14 @@ COMPOSEEOF
   # Local-Qwen dev agent — gated on ENABLE_LLAMA_AGENT so factories without
   # a local llama endpoint don't try to start it.  See docs/agents-llama.md.
   if [ "${ENABLE_LLAMA_AGENT:-0}" = "1" ]; then
+    # Check for duplicate with TOML-configured agents
+    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
+
     cat >> "$compose_file" <<'LLAMAEOF'
 
   agents-llama:
@@ -740,7 +794,10 @@ COMPOSEEOF
   fi
 
   # 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.
   # docker-compose.yml references ${CLAUDE_BIN_DIR} so the value must be set.

tests/lib-generators.bats

@@ -0,0 +1,94 @@
+#!/usr/bin/env bats
+# =============================================================================
+# tests/lib-generators.bats — Regression guard for the #849 fix.
+#
+# Before #849, `_generate_local_model_services` emitted the forge-user env
+# variable keyed by service name (`FORGE_BOT_USER_${service_name^^}`), so for
+# an `[agents.llama]` block with `forge_user = "dev-qwen"` the compose file
+# contained `FORGE_BOT_USER_LLAMA: "dev-qwen"`. That suffix diverges from the
+# `FORGE_TOKEN_<FORGE_USER>` / `FORGE_PASS_<FORGE_USER>` convention that the
+# same block uses two lines above, and it doesn't even round-trip through a
+# dash-containing service name (`dev-qwen` → `DEV-QWEN`, which is not a valid
+# shell identifier — see #852).
+#
+# The fix keys on `$user_upper` (already computed from `forge_user` via
+# `tr 'a-z-' 'A-Z_'`), yielding `FORGE_BOT_USER_DEV_QWEN: "dev-qwen"`.
+# =============================================================================
+
+setup() {
+  ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)"
+  export FACTORY_ROOT="${BATS_TEST_TMPDIR}/factory"
+  mkdir -p "${FACTORY_ROOT}/projects"
+
+  # Minimal compose skeleton that `_generate_local_model_services` can splice into.
+  # It only needs a `volumes:` marker line and nothing below it that would be
+  # re-read after the splice.
+  cat > "${FACTORY_ROOT}/docker-compose.yml" <<'EOF'
+services:
+  agents:
+    image: placeholder
+
+volumes:
+  agent-data:
+EOF
+}
+
+@test "local-model agent service emits FORGE_BOT_USER keyed by forge_user (#849)" {
+  cat > "${FACTORY_ROOT}/projects/test.toml" <<'EOF'
+name      = "test"
+repo      = "test-owner/test-repo"
+forge_url = "http://localhost:3000"
+
+[agents.llama]
+base_url    = "http://10.10.10.1:8081"
+model       = "qwen"
+api_key     = "sk-no-key-required"
+roles       = ["dev"]
+forge_user  = "dev-qwen"
+compact_pct = 60
+EOF
+
+  run bash -c "
+    set -euo pipefail
+    source '${ROOT}/lib/generators.sh'
+    _generate_local_model_services '${FACTORY_ROOT}/docker-compose.yml'
+    cat '${FACTORY_ROOT}/docker-compose.yml'
+  "
+
+  [ "$status" -eq 0 ]
+  # New, forge_user-keyed suffix is present with the right value.
+  [[ "$output" == *'FORGE_BOT_USER_DEV_QWEN: "dev-qwen"'* ]]
+  # Legacy service-name-keyed suffix must not be emitted.
+  [[ "$output" != *'FORGE_BOT_USER_LLAMA'* ]]
+}
+
+@test "local-model agent service keys FORGE_BOT_USER to forge_user even when it differs from service name (#849)" {
+  # Exercise the case the issue calls out: two agents in the same factory
+  # whose service names are identical (`[agents.llama]`) but whose
+  # forge_users diverge would previously both have emitted
+  # `FORGE_BOT_USER_LLAMA`. With the fix each emission carries its own
+  # forge_user-derived suffix.
+  cat > "${FACTORY_ROOT}/projects/a.toml" <<'EOF'
+name      = "a"
+repo      = "a/a"
+forge_url = "http://localhost:3000"
+
+[agents.dev]
+base_url   = "http://10.10.10.1:8081"
+model      = "qwen"
+api_key    = "sk-no-key-required"
+roles      = ["dev"]
+forge_user = "review-qwen"
+EOF
+
+  run bash -c "
+    set -euo pipefail
+    source '${ROOT}/lib/generators.sh'
+    _generate_local_model_services '${FACTORY_ROOT}/docker-compose.yml'
+    cat '${FACTORY_ROOT}/docker-compose.yml'
+  "
+
+  [ "$status" -eq 0 ]
+  [[ "$output" == *'FORGE_BOT_USER_REVIEW_QWEN: "review-qwen"'* ]]
+  [[ "$output" != *'FORGE_BOT_USER_DEV:'* ]]
+}

tests/smoke-init.sh

@@ -423,6 +423,49 @@ export CLAUDE_SHARED_DIR="$ORIG_CLAUDE_SHARED_DIR"
 export CLAUDE_CONFIG_DIR="$ORIG_CLAUDE_CONFIG_DIR"
 rm -rf /tmp/smoke-claude-shared /tmp/smoke-home-claude
 
+# ── 8. Test duplicate service name detection ──────────────────────────────
+echo "=== 8/8 Testing duplicate service name detection ==="
+
+# Clean up for duplicate test
+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 ──────────────────────────────────────────────────────────────────
 echo ""
 if [ "$FAILED" -ne 0 ]; then

tests/test-duplicate-service-detection.sh

@@ -0,0 +1,210 @@
+#!/usr/bin/env bash
+# tests/test-duplicate-service-detection.sh — Unit test for duplicate service detection
+#
+# Tests that the compose generator correctly detects duplicate service names
+# between ENABLE_LLAMA_AGENT=1 and [agents.llama] TOML configuration.
+
+set -euo pipefail
+
+# Get the absolute path to the disinto root
+DISINTO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+TEST_DIR=$(mktemp -d)
+trap "rm -rf \"\$TEST_DIR\"" EXIT
+
+FAILED=0
+
+fail() { printf 'FAIL: %s\n' "$*" >&2; FAILED=1; }
+pass() { printf 'PASS: %s\n' "$*"; }
+
+# Test 1: Duplicate between ENABLE_LLAMA_AGENT and [agents.llama]
+echo "=== Test 1: Duplicate between ENABLE_LLAMA_AGENT and [agents.llama] ==="
+
+# Create projects directory and test project TOML with an agent named "llama"
+mkdir -p "${TEST_DIR}/projects"
+cat > "${TEST_DIR}/projects/test-project.toml" <<'TOMLEOF'
+name = "test-project"
+description = "Test project for duplicate detection"
+
+[ci]
+woodpecker_repo_id = "123"
+
+[agents.llama]
+base_url = "http://localhost:8080"
+model = "qwen:latest"
+roles = ["dev"]
+forge_user = "llama-bot"
+TOMLEOF
+
+# Create a minimal compose file
+cat > "${TEST_DIR}/docker-compose.yml" <<'COMPOSEEOF'
+# Test compose file
+services:
+  agents:
+    image: test:latest
+    command: echo "hello"
+
+volumes:
+  test-data:
+
+networks:
+  test-net:
+COMPOSEEOF
+
+# Set up the test environment
+export FACTORY_ROOT="${TEST_DIR}"
+export PROJECT_NAME="test-project"
+export ENABLE_LLAMA_AGENT="1"
+export FORGE_TOKEN=""
+export FORGE_PASS=""
+export CLAUDE_TIMEOUT="7200"
+export POLL_INTERVAL="300"
+export GARDENER_INTERVAL="21600"
+export ARCHITECT_INTERVAL="21600"
+export PLANNER_INTERVAL="43200"
+export SUPERVISOR_INTERVAL="1200"
+
+# Source the generators module and run the compose generator directly
+source "${DISINTO_ROOT}/lib/generators.sh"
+
+# Delete the compose file to force regeneration
+rm -f "${TEST_DIR}/docker-compose.yml"
+
+# Run the compose generator directly
+if _generate_compose_impl 3000 false 2>&1 | tee "${TEST_DIR}/output.txt"; then
+  # Check if the output contains the duplicate error message
+  if grep -q "Duplicate service name 'agents-llama'" "${TEST_DIR}/output.txt"; then
+    pass "Duplicate detection: correctly detected conflict between ENABLE_LLAMA_AGENT and [agents.llama]"
+  else
+    fail "Duplicate detection: should have detected conflict between ENABLE_LLAMA_AGENT and [agents.llama]"
+    cat "${TEST_DIR}/output.txt" >&2
+  fi
+else
+  # Generator should fail with non-zero exit code
+  if grep -q "Duplicate service name 'agents-llama'" "${TEST_DIR}/output.txt"; then
+    pass "Duplicate detection: correctly detected conflict and returned non-zero exit code"
+  else
+    fail "Duplicate detection: should have failed with duplicate error"
+    cat "${TEST_DIR}/output.txt" >&2
+  fi
+fi
+
+# Test 2: No duplicate when only ENABLE_LLAMA_AGENT is set (no conflicting TOML)
+echo ""
+echo "=== Test 2: No duplicate when only ENABLE_LLAMA_AGENT is set ==="
+
+# Remove the projects directory created in Test 1
+rm -rf "${TEST_DIR}/projects"
+
+# Create a fresh compose file
+cat > "${TEST_DIR}/docker-compose.yml" <<'COMPOSEEOF'
+# Test compose file
+services:
+  agents:
+    image: test:latest
+
+volumes:
+  test-data:
+
+networks:
+  test-net:
+COMPOSEEOF
+
+# Set ENABLE_LLAMA_AGENT
+export ENABLE_LLAMA_AGENT="1"
+
+# Delete the compose file to force regeneration
+rm -f "${TEST_DIR}/docker-compose.yml"
+
+if _generate_compose_impl 3000 false 2>&1 | tee "${TEST_DIR}/output2.txt"; then
+  if grep -q "Duplicate" "${TEST_DIR}/output2.txt"; then
+    fail "No duplicate: should not detect duplicate when only ENABLE_LLAMA_AGENT is set"
+  else
+    pass "No duplicate: correctly generated compose without duplicates"
+  fi
+else
+  # Non-zero exit is fine if there's a legitimate reason (e.g., missing files)
+  if grep -q "Duplicate" "${TEST_DIR}/output2.txt"; then
+    fail "No duplicate: should not detect duplicate when only ENABLE_LLAMA_AGENT is set"
+  else
+    pass "No duplicate: generator failed for other reason (acceptable)"
+  fi
+fi
+
+# Test 3: Duplicate between two TOML agents with same name
+echo ""
+echo "=== Test 3: Duplicate between two TOML agents with same name ==="
+
+rm -f "${TEST_DIR}/docker-compose.yml"
+
+# Create projects directory for Test 3
+mkdir -p "${TEST_DIR}/projects"
+
+cat > "${TEST_DIR}/projects/project1.toml" <<'TOMLEOF'
+name = "project1"
+description = "First project"
+
+[ci]
+woodpecker_repo_id = "1"
+
+[agents.llama]
+base_url = "http://localhost:8080"
+model = "qwen:latest"
+roles = ["dev"]
+forge_user = "llama-bot1"
+TOMLEOF
+
+cat > "${TEST_DIR}/projects/project2.toml" <<'TOMLEOF'
+name = "project2"
+description = "Second project"
+
+[ci]
+woodpecker_repo_id = "2"
+
+[agents.llama]
+base_url = "http://localhost:8080"
+model = "qwen:latest"
+roles = ["dev"]
+forge_user = "llama-bot2"
+TOMLEOF
+
+cat > "${TEST_DIR}/docker-compose.yml" <<'COMPOSEEOF'
+# Test compose file
+services:
+  agents:
+    image: test:latest
+
+volumes:
+  test-data:
+
+networks:
+  test-net:
+COMPOSEEOF
+
+unset ENABLE_LLAMA_AGENT
+
+# Delete the compose file to force regeneration
+rm -f "${TEST_DIR}/docker-compose.yml"
+
+if _generate_compose_impl 3000 false 2>&1 | tee "${TEST_DIR}/output3.txt"; then
+  if grep -q "Duplicate service name 'agents-llama'" "${TEST_DIR}/output3.txt"; then
+    pass "Duplicate detection: correctly detected conflict between two [agents.llama] blocks"
+  else
+    fail "Duplicate detection: should have detected conflict between two [agents.llama] blocks"
+    cat "${TEST_DIR}/output3.txt" >&2
+  fi
+else
+  if grep -q "Duplicate service name 'agents-llama'" "${TEST_DIR}/output3.txt"; then
+    pass "Duplicate detection: correctly detected conflict and returned non-zero exit code"
+  else
+    fail "Duplicate detection: should have failed with duplicate error"
+    cat "${TEST_DIR}/output3.txt" >&2
+  fi
+fi
+
+# Summary
+echo ""
+if [ "$FAILED" -ne 0 ]; then
+  echo "=== TESTS FAILED ==="
+  exit 1
+fi
+echo "=== ALL TESTS PASSED ==="