From 721d7a6077c96b1ea96624d75692d6439e094b63 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Apr 2026 11:55:56 +0000 Subject: [PATCH] fix: bug: TOML [agents.X] section name with dash crashes load-project.sh (#862) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- lib/hire-agent.sh | 23 +++++ lib/load-project.sh | 18 ++-- tests/lib-load-project.bats | 186 ++++++++++++++++++++++++++++++++++++ 3 files changed, 221 insertions(+), 6 deletions(-) create mode 100644 tests/lib-load-project.bats diff --git a/lib/hire-agent.sh b/lib/hire-agent.sh index 994103a..1140f73 100644 --- a/lib/hire-agent.sh +++ b/lib/hire-agent.sh @@ -30,6 +30,29 @@ disinto_hire_an_agent() { echo "Usage: disinto hire-an-agent [--formula ] [--local-model ] [--model ] [--poll-interval ]" >&2 exit 1 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-` 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 # Parse flags diff --git a/lib/load-project.sh b/lib/load-project.sh index 0745276..5ad23cc 100755 --- a/lib/load-project.sh +++ b/lib/load-project.sh @@ -129,20 +129,26 @@ agents = cfg.get('agents', {}) for name, config in agents.items(): if not isinstance(config, dict): 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 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: - print(f'AGENT_{name.upper()}_MODEL={config[\"model\"]}') + print(f'AGENT_{safe}_MODEL={config[\"model\"]}') 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: 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: - print(f'AGENT_{name.upper()}_FORGE_USER={config[\"forge_user\"]}') + print(f'AGENT_{safe}_FORGE_USER={config[\"forge_user\"]}') 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 if [ -n "$_AGENT_VARS" ]; then diff --git a/tests/lib-load-project.bats b/tests/lib-load-project.bats new file mode 100644 index 0000000..89e82be --- /dev/null +++ b/tests/lib-load-project.bats @@ -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" < "$TOML" < "$TOML" <