Compare commits

...

4 commits

Author SHA1 Message Date
Agent
a3eb33ccf7 fix: _validate_env_vars skips Anthropic-backend agents + missing sed escaping
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
ci/woodpecker/push/nomad-validate Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/nomad-validate Pipeline was successful
ci/woodpecker/pr/smoke-init Pipeline was successful
- bin/disinto: Remove '[ -n "$base_url" ] || continue' guard that caused
  all Anthropic-backend agents to be silently skipped during validation.
  The base_url check is now scoped only to backend-credential selection.

- lib/hire-agent.sh: Add sed escaping for ANTHROPIC_BASE_URL value before
  sed substitution (same pattern as ANTHROPIC_API_KEY at line 256).

Fixes AI review BLOCKER and MINOR issues on PR #866.
2026-04-16 12:29:00 +00:00
Agent
53a1fe397b fix: hire-an-agent does not persist per-agent secrets to .env (#847) 2026-04-16 12:29:00 +00:00
9248c533d4 Merge pull request 'fix: bug: TOML [agents.X] section name with dash crashes load-project.sh (#862)' (#865) from fix/issue-862 into main
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
2026-04-16 12:16:55 +00:00
Claude
721d7a6077 fix: bug: TOML [agents.X] section name with dash crashes load-project.sh (#862)
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/smoke-init Pipeline was successful
TOML allows dashes in bare keys, so `[agents.dev-qwen2]` is a valid
section. Before this fix, load-project.sh derived bash var names via
Python `.upper()` alone, which kept the dash and produced
`AGENT_DEV-QWEN2_BASE_URL` — an invalid shell identifier. Under
`set -euo pipefail` the subsequent `export` aborted the whole file,
silently taking the factory down on the N+1 run after a dashed agent
was hired via `disinto hire-an-agent`.

Normalize via `.upper().replace('-', '_')` to match the
`tr 'a-z-' 'A-Z_'` convention already used by hire-agent.sh (#834)
and generators.sh (#852). Also harden hire-agent.sh to reject invalid
agent names at hire time (before any Forgejo side effects), so
unparseable TOML sections never land on disk.

- `lib/load-project.sh` — dash-to-underscore in emitted shell var names
- `lib/hire-agent.sh` — validate agent name against
  `^[a-z]([a-z0-9]|-[a-z0-9])*$` up front
- `tests/lib-load-project.bats` — regression guard covering the parse
  path and the hire-time reject path

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-16 11:55:59 +00:00
5 changed files with 422 additions and 7 deletions

View file

@ -60,7 +60,7 @@ Usage:
Read CI logs from Woodpecker SQLite Read CI logs from Woodpecker SQLite
disinto release <version> Create vault PR for release (e.g., v1.2.0) disinto release <version> Create vault PR for release (e.g., v1.2.0)
disinto hire-an-agent <agent-name> <role> [--formula <path>] [--local-model <url>] [--model <name>] disinto hire-an-agent <agent-name> <role> [--formula <path>] [--local-model <url>] [--model <name>]
Hire a new agent (create user + .profile repo) Hire a new agent (create user + .profile repo; re-run to rotate credentials)
disinto agent <subcommand> Manage agent state (enable/disable) disinto agent <subcommand> Manage agent state (enable/disable)
disinto edge <verb> [options] Manage edge tunnel registrations disinto edge <verb> [options] Manage edge tunnel registrations
@ -1757,6 +1757,118 @@ _regen_file() {
fi fi
} }
# Validate that required environment variables are present for all services
# that reference them in docker-compose.yml
_validate_env_vars() {
local env_file="${FACTORY_ROOT}/.env"
local errors=0
local -a missing_vars=()
# Load env vars from .env file into associative array
declare -A env_vars
if [ -f "$env_file" ]; then
while IFS='=' read -r key value; do
# Skip empty lines and comments
[[ -z "$key" || "$key" =~ ^[[:space:]]*# ]] && continue
env_vars["$key"]="$value"
done < "$env_file"
fi
# Check for local-model agent services
# Each [agents.*] section in projects/*.toml requires:
# - FORGE_TOKEN_<USER_UPPER>
# - FORGE_PASS_<USER_UPPER>
# - ANTHROPIC_BASE_URL (local model) OR ANTHROPIC_API_KEY (Anthropic backend)
# Parse projects/*.toml for [agents.*] sections
local projects_dir="${FACTORY_ROOT}/projects"
for toml in "${projects_dir}"/*.toml; do
[ -f "$toml" ] || continue
# Extract agent config using Python
while IFS='|' read -r service_name forge_user base_url _api_key; do
[ -n "$service_name" ] || continue
[ -n "$forge_user" ] || continue
# Derive variable names (user -> USER_UPPER)
local user_upper
user_upper=$(echo "$forge_user" | tr 'a-z-' 'A-Z_')
local token_var="FORGE_TOKEN_${user_upper}"
local pass_var="FORGE_PASS_${user_upper}"
# Check token
if [ -z "${env_vars[$token_var]:-}" ]; then
missing_vars+=("$token_var (for agent ${service_name}/${forge_user})")
errors=$((errors + 1))
fi
# Check password
if [ -z "${env_vars[$pass_var]:-}" ]; then
missing_vars+=("$pass_var (for agent ${service_name}/${forge_user})")
errors=$((errors + 1))
fi
# Check backend URL or API key (conditional based on base_url presence)
if [ -n "$base_url" ]; then
# Local model: needs ANTHROPIC_BASE_URL
if [ -z "${env_vars[ANTHROPIC_BASE_URL]:-}" ]; then
missing_vars+=("ANTHROPIC_BASE_URL (for agent ${service_name})")
errors=$((errors + 1))
fi
else
# Anthropic backend: needs ANTHROPIC_API_KEY
if [ -z "${env_vars[ANTHROPIC_API_KEY]:-}" ]; then
missing_vars+=("ANTHROPIC_API_KEY (for agent ${service_name})")
errors=$((errors + 1))
fi
fi
done < <(python3 -c '
import sys, tomllib, re
with open(sys.argv[1], "rb") as f:
cfg = tomllib.load(f)
agents = cfg.get("agents", {})
for name, config in agents.items():
if not isinstance(config, dict):
continue
base_url = config.get("base_url", "")
model = config.get("model", "")
api_key = config.get("api_key", "")
forge_user = config.get("forge_user", f"{name}-bot")
safe_name = name.lower()
safe_name = re.sub(r"[^a-z0-9]", "-", safe_name)
print(f"{safe_name}|{forge_user}|{base_url}|{api_key}")
' "$toml" 2>/dev/null)
done
# Check for legacy ENABLE_LLAMA_AGENT services
if [ "${env_vars[ENABLE_LLAMA_AGENT]:-0}" = "1" ]; then
if [ -z "${env_vars[FORGE_TOKEN_LLAMA]:-}" ]; then
missing_vars+=("FORGE_TOKEN_LLAMA (ENABLE_LLAMA_AGENT=1)")
errors=$((errors + 1))
fi
if [ -z "${env_vars[FORGE_PASS_LLAMA]:-}" ]; then
missing_vars+=("FORGE_PASS_LLAMA (ENABLE_LLAMA_AGENT=1)")
errors=$((errors + 1))
fi
fi
if [ "$errors" -gt 0 ]; then
echo "Error: missing required environment variables:" >&2
for var in "${missing_vars[@]}"; do
echo " - $var" >&2
done
echo "" >&2
echo "Run 'disinto hire-an-agent <name> <role>' to create the agent and write credentials to .env" >&2
exit 1
fi
}
disinto_up() { disinto_up() {
local compose_file="${FACTORY_ROOT}/docker-compose.yml" local compose_file="${FACTORY_ROOT}/docker-compose.yml"
local caddyfile="${FACTORY_ROOT}/docker/Caddyfile" local caddyfile="${FACTORY_ROOT}/docker/Caddyfile"
@ -1766,6 +1878,9 @@ disinto_up() {
exit 1 exit 1
fi fi
# Validate environment variables before proceeding
_validate_env_vars
# Parse --no-regen flag; remaining args pass through to docker compose # Parse --no-regen flag; remaining args pass through to docker compose
local no_regen=false local no_regen=false
local -a compose_args=() local -a compose_args=()

View file

@ -26,6 +26,51 @@ ANTHROPIC_BASE_URL=http://host.docker.internal:8081 # llama-server endpoint
Then regenerate the compose file (`disinto init ...`) and bring the stack up. Then regenerate the compose file (`disinto init ...`) and bring the stack up.
## Hiring a new agent
Use `disinto hire-an-agent` to create a Forgejo user, API token, and password,
and write all required credentials to `.env`:
```bash
# Local model agent
disinto hire-an-agent dev-qwen dev \
--local-model http://10.10.10.1:8081 \
--model unsloth/Qwen3.5-35B-A3B
# Anthropic backend agent (requires ANTHROPIC_API_KEY in environment)
disinto hire-an-agent dev-qwen dev
```
The command writes the following to `.env`:
- `FORGE_TOKEN_<USER_UPPER>` — derived from the agent's Forgejo username (e.g., `FORGE_TOKEN_DEV_QWEN`)
- `FORGE_PASS_<USER_UPPER>` — the agent's Forgejo password
- `ANTHROPIC_BASE_URL` (local model) or `ANTHROPIC_API_KEY` (Anthropic backend)
## Rotation
Re-running `disinto hire-an-agent <same-name>` rotates credentials idempotently:
```bash
# Re-hire the same agent to rotate token and password
disinto hire-an-agent dev-qwen dev \
--local-model http://10.10.10.1:8081 \
--model unsloth/Qwen3.5-35B-A3B
# The command will:
# 1. Detect the user already exists
# 2. Reset the password to a new random value
# 3. Create a new API token
# 4. Update .env with the new credentials
```
This is the recommended way to rotate agent credentials. The `.env` file is
updated in place, so no manual editing is required.
If you need to manually rotate credentials, you can:
1. Generate a new token in Forgejo admin UI
2. Edit `.env` and replace `FORGE_TOKEN_<USER_UPPER>` and `FORGE_PASS_<USER_UPPER>`
3. Restart the agent service: `docker compose restart disinto-agents-<name>`
### Running all 7 roles (agents-llama-all) ### Running all 7 roles (agents-llama-all)
```bash ```bash

View file

@ -30,6 +30,29 @@ disinto_hire_an_agent() {
echo "Usage: disinto hire-an-agent <agent-name> <role> [--formula <path>] [--local-model <url>] [--model <name>] [--poll-interval <seconds>]" >&2 echo "Usage: disinto hire-an-agent <agent-name> <role> [--formula <path>] [--local-model <url>] [--model <name>] [--poll-interval <seconds>]" >&2
exit 1 exit 1
fi fi
# Validate agent name before any side effects (Forgejo user creation, TOML
# write, token issuance). The name flows through several systems that have
# stricter rules than the raw TOML spec:
# - load-project.sh emits shell vars keyed by the name (dashes are mapped
# to underscores via tr 'a-z-' 'A-Z_')
# - generators.sh emits a docker-compose service name `agents-<name>` and
# uppercases it for env var keys (#852 tracks the `^^` bug; we keep the
# grammar tight here so that fix can happen without re-validation)
# - Forgejo usernames are lowercase alnum + dash
# Constraint: start with a lowercase letter, contain only [a-z0-9-], end
# with a lowercase letter or digit (no trailing dash), no consecutive
# dashes. Rejecting at hire-time prevents unparseable TOML sections like
# [agents.dev-qwen2] from landing on disk and crashing load-project.sh on
# the next `disinto up` (#862).
if ! [[ "$agent_name" =~ ^[a-z]([a-z0-9]|-[a-z0-9])*$ ]]; then
echo "Error: invalid agent name '${agent_name}'" >&2
echo " Agent names must match: ^[a-z]([a-z0-9]|-[a-z0-9])*$" >&2
echo " (lowercase letters/digits/single dashes, starts with letter, ends with alphanumeric)" >&2
echo " Examples: dev, dev-qwen2, review-qwen, planner" >&2
exit 1
fi
shift 2 shift 2
# Parse flags # Parse flags
@ -229,6 +252,46 @@ disinto_hire_an_agent() {
export "${pass_var}=${user_pass}" export "${pass_var}=${user_pass}"
fi fi
# Step 1.7: Write backend credentials to .env (#847).
# Local-model agents need ANTHROPIC_BASE_URL; Anthropic-backend agents need ANTHROPIC_API_KEY.
# These must be persisted so the container can start with valid credentials.
echo ""
echo "Step 1.7: Writing backend credentials to .env..."
if [ -n "$local_model" ]; then
# Local model agent: write ANTHROPIC_BASE_URL
local backend_var="ANTHROPIC_BASE_URL"
local backend_val="$local_model"
local escaped_val
escaped_val=$(printf '%s\n' "$backend_val" | sed 's/[&/\]/\\&/g')
if grep -q "^${backend_var}=" "$env_file" 2>/dev/null; then
sed -i "s|^${backend_var}=.*|${backend_var}=${escaped_val}|" "$env_file"
echo " ${backend_var} updated"
else
printf '%s=%s\n' "$backend_var" "$backend_val" >> "$env_file"
echo " ${backend_var} saved"
fi
export "${backend_var}=${backend_val}"
else
# Anthropic backend: check if ANTHROPIC_API_KEY is set, write it if present
if [ -n "${ANTHROPIC_API_KEY:-}" ]; then
local backend_var="ANTHROPIC_API_KEY"
local backend_val="$ANTHROPIC_API_KEY"
local escaped_key
escaped_key=$(printf '%s\n' "$backend_val" | sed 's/[&/\]/\\&/g')
if grep -q "^${backend_var}=" "$env_file" 2>/dev/null; then
sed -i "s|^${backend_var}=.*|${backend_var}=${escaped_key}|" "$env_file"
echo " ${backend_var} updated"
else
printf '%s=%s\n' "$backend_var" "$backend_val" >> "$env_file"
echo " ${backend_var} saved"
fi
export "${backend_var}=${backend_val}"
else
echo " Note: ANTHROPIC_API_KEY not set — required for Anthropic backend agents"
fi
fi
# Step 1.6: Add the new agent as a write collaborator on the project repo (#856). # Step 1.6: Add the new agent as a write collaborator on the project repo (#856).
# Without this, PATCH /issues/{n} {assignees:[agent]} returns 403 Forbidden and # Without this, PATCH /issues/{n} {assignees:[agent]} returns 403 Forbidden and
# the dev-agent polls forever logging "claim lost to <none> — skipping" (see # the dev-agent polls forever logging "claim lost to <none> — skipping" (see

View file

@ -129,20 +129,26 @@ agents = cfg.get('agents', {})
for name, config in agents.items(): for name, config in agents.items():
if not isinstance(config, dict): if not isinstance(config, dict):
continue continue
# Normalize the TOML section key into a valid shell identifier fragment.
# TOML allows dashes in bare keys (e.g. [agents.dev-qwen2]), but POSIX
# shell var names cannot contain '-'. Match the 'tr a-z- A-Z_' convention
# used in hire-agent.sh (#834) and generators.sh (#852) so the var names
# stay consistent across the stack.
safe = name.upper().replace('-', '_')
# Emit variables in uppercase with the agent name # Emit variables in uppercase with the agent name
if 'base_url' in config: if 'base_url' in config:
print(f'AGENT_{name.upper()}_BASE_URL={config[\"base_url\"]}') print(f'AGENT_{safe}_BASE_URL={config[\"base_url\"]}')
if 'model' in config: if 'model' in config:
print(f'AGENT_{name.upper()}_MODEL={config[\"model\"]}') print(f'AGENT_{safe}_MODEL={config[\"model\"]}')
if 'api_key' in config: if 'api_key' in config:
print(f'AGENT_{name.upper()}_API_KEY={config[\"api_key\"]}') print(f'AGENT_{safe}_API_KEY={config[\"api_key\"]}')
if 'roles' in config: if 'roles' in config:
roles = ' '.join(config['roles']) if isinstance(config['roles'], list) else config['roles'] roles = ' '.join(config['roles']) if isinstance(config['roles'], list) else config['roles']
print(f'AGENT_{name.upper()}_ROLES={roles}') print(f'AGENT_{safe}_ROLES={roles}')
if 'forge_user' in config: if 'forge_user' in config:
print(f'AGENT_{name.upper()}_FORGE_USER={config[\"forge_user\"]}') print(f'AGENT_{safe}_FORGE_USER={config[\"forge_user\"]}')
if 'compact_pct' in config: if 'compact_pct' in config:
print(f'AGENT_{name.upper()}_COMPACT_PCT={config[\"compact_pct\"]}') print(f'AGENT_{safe}_COMPACT_PCT={config[\"compact_pct\"]}')
" "$_PROJECT_TOML" 2>/dev/null) || true " "$_PROJECT_TOML" 2>/dev/null) || true
if [ -n "$_AGENT_VARS" ]; then if [ -n "$_AGENT_VARS" ]; then

186
tests/lib-load-project.bats Normal file
View file

@ -0,0 +1,186 @@
#!/usr/bin/env bats
# =============================================================================
# tests/lib-load-project.bats — Regression guard for the #862 fix.
#
# TOML allows dashes in bare keys, so `[agents.dev-qwen2]` is a valid section
# header. Before #862, load-project.sh translated the section name into a
# shell variable name via Python's `.upper()` alone, which kept the dash and
# produced `AGENT_DEV-QWEN2_BASE_URL`. `export "AGENT_DEV-QWEN2_..."` is
# rejected by bash ("not a valid identifier"), and with `set -euo pipefail`
# anywhere up-stack that error aborts load-project.sh — effectively crashing
# the factory on the N+1 run after a dashed agent was hired.
#
# The fix normalizes via `.upper().replace('-', '_')`, matching the
# `tr 'a-z-' 'A-Z_'` convention already used in hire-agent.sh and
# generators.sh.
# =============================================================================
setup() {
ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)"
TOML="${BATS_TEST_TMPDIR}/test.toml"
}
@test "dashed [agents.*] section name parses without error" {
cat > "$TOML" <<EOF
name = "test"
repo = "test-owner/test-repo"
forge_url = "http://localhost:3000"
[agents.dev-qwen2]
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-qwen2"
compact_pct = 60
EOF
run bash -c "
set -euo pipefail
source '${ROOT}/lib/load-project.sh' '$TOML'
echo \"BASE=\${AGENT_DEV_QWEN2_BASE_URL:-MISSING}\"
echo \"MODEL=\${AGENT_DEV_QWEN2_MODEL:-MISSING}\"
echo \"ROLES=\${AGENT_DEV_QWEN2_ROLES:-MISSING}\"
echo \"FORGE_USER=\${AGENT_DEV_QWEN2_FORGE_USER:-MISSING}\"
echo \"COMPACT=\${AGENT_DEV_QWEN2_COMPACT_PCT:-MISSING}\"
"
[ "$status" -eq 0 ]
[[ "$output" == *"BASE=http://10.10.10.1:8081"* ]]
[[ "$output" == *"MODEL=unsloth/Qwen3.5-35B-A3B"* ]]
[[ "$output" == *"ROLES=dev"* ]]
[[ "$output" == *"FORGE_USER=dev-qwen2"* ]]
[[ "$output" == *"COMPACT=60"* ]]
}
@test "dashless [agents.*] section name still works" {
cat > "$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-llama"
compact_pct = 60
EOF
run bash -c "
set -euo pipefail
source '${ROOT}/lib/load-project.sh' '$TOML'
echo \"BASE=\${AGENT_LLAMA_BASE_URL:-MISSING}\"
echo \"MODEL=\${AGENT_LLAMA_MODEL:-MISSING}\"
"
[ "$status" -eq 0 ]
[[ "$output" == *"BASE=http://10.10.10.1:8081"* ]]
[[ "$output" == *"MODEL=qwen"* ]]
}
@test "multiple dashes in [agents.*] name all normalized" {
cat > "$TOML" <<EOF
name = "test"
repo = "test-owner/test-repo"
forge_url = "http://localhost:3000"
[agents.review-qwen-3b]
base_url = "http://10.10.10.1:8082"
model = "qwen-3b"
api_key = "sk-no-key-required"
roles = ["review"]
forge_user = "review-qwen-3b"
compact_pct = 60
EOF
run bash -c "
set -euo pipefail
source '${ROOT}/lib/load-project.sh' '$TOML'
echo \"BASE=\${AGENT_REVIEW_QWEN_3B_BASE_URL:-MISSING}\"
"
[ "$status" -eq 0 ]
[[ "$output" == *"BASE=http://10.10.10.1:8082"* ]]
}
@test "hire-agent rejects dash-starting agent name" {
run bash -c "
FACTORY_ROOT='${ROOT}' \
FORGE_URL='http://127.0.0.1:1' \
FORGE_TOKEN=x \
bash -c '
set -euo pipefail
source \"\${FACTORY_ROOT}/lib/hire-agent.sh\"
disinto_hire_an_agent -foo dev
'
"
[ "$status" -ne 0 ]
[[ "$output" == *"invalid agent name"* ]]
}
@test "hire-agent rejects uppercase agent name" {
run bash -c "
FACTORY_ROOT='${ROOT}' \
FORGE_URL='http://127.0.0.1:1' \
FORGE_TOKEN=x \
bash -c '
set -euo pipefail
source \"\${FACTORY_ROOT}/lib/hire-agent.sh\"
disinto_hire_an_agent DevQwen dev
'
"
[ "$status" -ne 0 ]
[[ "$output" == *"invalid agent name"* ]]
}
@test "hire-agent rejects underscore agent name" {
run bash -c "
FACTORY_ROOT='${ROOT}' \
FORGE_URL='http://127.0.0.1:1' \
FORGE_TOKEN=x \
bash -c '
set -euo pipefail
source \"\${FACTORY_ROOT}/lib/hire-agent.sh\"
disinto_hire_an_agent dev_qwen dev
'
"
[ "$status" -ne 0 ]
[[ "$output" == *"invalid agent name"* ]]
}
@test "hire-agent rejects trailing dash agent name" {
run bash -c "
FACTORY_ROOT='${ROOT}' \
FORGE_URL='http://127.0.0.1:1' \
FORGE_TOKEN=x \
bash -c '
set -euo pipefail
source \"\${FACTORY_ROOT}/lib/hire-agent.sh\"
disinto_hire_an_agent dev- dev
'
"
[ "$status" -ne 0 ]
[[ "$output" == *"invalid agent name"* ]]
}
@test "hire-agent rejects consecutive-dash agent name" {
run bash -c "
FACTORY_ROOT='${ROOT}' \
FORGE_URL='http://127.0.0.1:1' \
FORGE_TOKEN=x \
bash -c '
set -euo pipefail
source \"\${FACTORY_ROOT}/lib/hire-agent.sh\"
disinto_hire_an_agent dev--qwen dev
'
"
[ "$status" -ne 0 ]
[[ "$output" == *"invalid agent name"* ]]
}