From 91fdb3511188afa49c756f1ca19d6aaa023f212d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Apr 2026 12:58:51 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20Generated=20compose=20emits=20FORGE=5FBO?= =?UTF-8?q?T=5FUSER=5FLLAMA=20=E2=80=94=20legacy=20name,=20should=20derive?= =?UTF-8?q?=20from=20forge=5Fuser=20(#849)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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_PASS_` 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) --- lib/generators.sh | 2 +- tests/lib-generators.bats | 94 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 tests/lib-generators.bats diff --git a/lib/generators.sh b/lib/generators.sh index 1e97ebe..87d997b 100644 --- a/lib/generators.sh +++ b/lib/generators.sh @@ -149,7 +149,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}" diff --git a/tests/lib-generators.bats b/tests/lib-generators.bats new file mode 100644 index 0000000..0573579 --- /dev/null +++ b/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_PASS_` 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:'* ]] +}