From 0989f29b0bca412edc58018c462cae7344bb1a2f Mon Sep 17 00:00:00 2001 From: Boris Cherny Date: Mon, 24 Aug 2026 19:24:45 -0700 Subject: [PATCH] validate-agent.sh: don't abort at the first warning (set -e + ((x++))) and stop false-flagging valid agents Two defects made the validator fail on plugin-dev's own agent files (anthropics/claude-code#83803): 1. Under `set -e`, `((warning_count++))` / `((error_count++))` return a nonzero status when the counter was 0, so the script died at the first warning or error instead of finishing the run. Increments now use `count=$((count + 1))`, which always returns 0. 2. Field extractions like `TOOLS=$(... | grep '^tools:' ...)` aborted the script under `set -e` when the field was absent (grep exits 1 on no match), instead of reporting the missing field. They now end in `|| true`. 3. The description check only read the first physical line of the `description:` value, so multi-line descriptions with blocks (as in plugin-dev's own agents) were false-flagged as missing examples. The extraction now captures the full multi-line value. Adds validate-agent.test.sh: plugin-dev's own agents must exit 0, a warning-only file must complete with exit 0, and an invalid file must still exit 1 with all errors reported. No-Verification-Needed: standalone shell script in the public repo; driven end-to-end directly plus new regression harness --- .../scripts/validate-agent.sh | 54 ++++++++------ .../scripts/validate-agent.test.sh | 70 +++++++++++++++++++ 2 files changed, 101 insertions(+), 23 deletions(-) create mode 100755 plugins/plugin-dev/skills/agent-development/scripts/validate-agent.test.sh diff --git a/plugins/plugin-dev/skills/agent-development/scripts/validate-agent.sh b/plugins/plugin-dev/skills/agent-development/scripts/validate-agent.sh index ca4dfd4b92..5fa5b5a6f1 100755 --- a/plugins/plugin-dev/skills/agent-development/scripts/validate-agent.sh +++ b/plugins/plugin-dev/skills/agent-development/scripts/validate-agent.sh @@ -56,74 +56,82 @@ error_count=0 warning_count=0 # Check name field -NAME=$(echo "$FRONTMATTER" | grep '^name:' | sed 's/name: *//' | sed 's/^"\(.*\)"$/\1/') +# The "|| true" on each field extraction keeps grep's no-match exit status from +# killing the script under set -e; a missing field is reported below instead. +NAME=$(echo "$FRONTMATTER" | grep '^name:' | sed 's/name: *//' | sed 's/^"\(.*\)"$/\1/' || true) if [ -z "$NAME" ]; then echo "❌ Missing required field: name" - ((error_count++)) + error_count=$((error_count + 1)) else echo "✅ name: $NAME" # Validate name format if ! [[ "$NAME" =~ ^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$ ]]; then echo "❌ name must start/end with alphanumeric and contain only letters, numbers, hyphens" - ((error_count++)) + error_count=$((error_count + 1)) fi # Validate name length name_length=${#NAME} if [ $name_length -lt 3 ]; then echo "❌ name too short (minimum 3 characters)" - ((error_count++)) + error_count=$((error_count + 1)) elif [ $name_length -gt 50 ]; then echo "❌ name too long (maximum 50 characters)" - ((error_count++)) + error_count=$((error_count + 1)) fi # Check for generic names if [[ "$NAME" =~ ^(helper|assistant|agent|tool)$ ]]; then echo "⚠️ name is too generic: $NAME" - ((warning_count++)) + warning_count=$((warning_count + 1)) fi fi # Check description field -DESCRIPTION=$(echo "$FRONTMATTER" | grep '^description:' | sed 's/description: *//') +# The description may span multiple lines (unquoted prose plus blocks), +# so capture everything from "description:" until the next top-level agent key. +DESCRIPTION=$(echo "$FRONTMATTER" | awk ' + /^description:/ { in_description=1; sub(/^description:[[:space:]]*/, ""); print; next } + /^(name|model|color|tools):/ { in_description=0 } + in_description { print } +') if [ -z "$DESCRIPTION" ]; then echo "❌ Missing required field: description" - ((error_count++)) + error_count=$((error_count + 1)) else desc_length=${#DESCRIPTION} echo "✅ description: ${desc_length} characters" if [ $desc_length -lt 10 ]; then echo "⚠️ description too short (minimum 10 characters recommended)" - ((warning_count++)) + warning_count=$((warning_count + 1)) elif [ $desc_length -gt 5000 ]; then echo "⚠️ description very long (over 5000 characters)" - ((warning_count++)) + warning_count=$((warning_count + 1)) fi # Check for example blocks if ! echo "$DESCRIPTION" | grep -q ''; then echo "⚠️ description should include blocks for triggering" - ((warning_count++)) + warning_count=$((warning_count + 1)) fi # Check for "Use this agent when" pattern if ! echo "$DESCRIPTION" | grep -qi 'use this agent when'; then echo "⚠️ description should start with 'Use this agent when...'" - ((warning_count++)) + warning_count=$((warning_count + 1)) fi fi # Check model field -MODEL=$(echo "$FRONTMATTER" | grep '^model:' | sed 's/model: *//') +MODEL=$(echo "$FRONTMATTER" | grep '^model:' | sed 's/model: *//' || true) if [ -z "$MODEL" ]; then echo "❌ Missing required field: model" - ((error_count++)) + error_count=$((error_count + 1)) else echo "✅ model: $MODEL" @@ -133,17 +141,17 @@ else ;; *) echo "⚠️ Unknown model: $MODEL (valid: inherit, sonnet, opus, haiku)" - ((warning_count++)) + warning_count=$((warning_count + 1)) ;; esac fi # Check color field -COLOR=$(echo "$FRONTMATTER" | grep '^color:' | sed 's/color: *//') +COLOR=$(echo "$FRONTMATTER" | grep '^color:' | sed 's/color: *//' || true) if [ -z "$COLOR" ]; then echo "❌ Missing required field: color" - ((error_count++)) + error_count=$((error_count + 1)) else echo "✅ color: $COLOR" @@ -153,13 +161,13 @@ else ;; *) echo "⚠️ Unknown color: $COLOR (valid: blue, cyan, green, yellow, magenta, red)" - ((warning_count++)) + warning_count=$((warning_count + 1)) ;; esac fi # Check tools field (optional) -TOOLS=$(echo "$FRONTMATTER" | grep '^tools:' | sed 's/tools: *//') +TOOLS=$(echo "$FRONTMATTER" | grep '^tools:' | sed 's/tools: *//' || true) if [ -n "$TOOLS" ]; then echo "✅ tools: $TOOLS" @@ -173,23 +181,23 @@ echo "Checking system prompt..." if [ -z "$SYSTEM_PROMPT" ]; then echo "❌ System prompt is empty" - ((error_count++)) + error_count=$((error_count + 1)) else prompt_length=${#SYSTEM_PROMPT} echo "✅ System prompt: $prompt_length characters" if [ $prompt_length -lt 20 ]; then echo "❌ System prompt too short (minimum 20 characters)" - ((error_count++)) + error_count=$((error_count + 1)) elif [ $prompt_length -gt 10000 ]; then echo "⚠️ System prompt very long (over 10,000 characters)" - ((warning_count++)) + warning_count=$((warning_count + 1)) fi # Check for second person if ! echo "$SYSTEM_PROMPT" | grep -q "You are\|You will\|Your"; then echo "⚠️ System prompt should use second person (You are..., You will...)" - ((warning_count++)) + warning_count=$((warning_count + 1)) fi # Check for structure diff --git a/plugins/plugin-dev/skills/agent-development/scripts/validate-agent.test.sh b/plugins/plugin-dev/skills/agent-development/scripts/validate-agent.test.sh new file mode 100755 index 0000000000..f35acb97dd --- /dev/null +++ b/plugins/plugin-dev/skills/agent-development/scripts/validate-agent.test.sh @@ -0,0 +1,70 @@ +#!/bin/bash +# Regression tests for validate-agent.sh (anthropics/claude-code#83803): +# - warnings must not abort the run: under `set -e`, `((x++))` returns nonzero +# when x was 0, so the first warning killed the script with exit 1 +# - multi-line descriptions (prose plus blocks) must not be +# false-flagged as missing examples +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +VALIDATOR="$SCRIPT_DIR/validate-agent.sh" +PLUGIN_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" +TMP_DIR="$(mktemp -d)" || exit 1 +trap 'rm -rf "$TMP_DIR"' EXIT + +failures=0 + +check() { + local label="$1" expected="$2" file="$3" + local actual=0 + bash "$VALIDATOR" "$file" > "$TMP_DIR/out.txt" 2>&1 || actual=$? + if [ "$actual" -eq "$expected" ]; then + echo "PASS: $label (exit $actual)" + else + echo "FAIL: $label (expected exit $expected, got $actual)" + cat "$TMP_DIR/out.txt" + failures=$((failures + 1)) + fi +} + +# The plugin's own agents are valid and must pass. +for agent in "$PLUGIN_ROOT"/agents/*.md; do + check "own agent $(basename "$agent")" 0 "$agent" +done + +# A valid agent whose fields only trigger warnings must still exit 0, +# and must reach the summary line instead of aborting at the first warning. +cat > "$TMP_DIR/warning-agent.md" <<'EOF' +--- +name: warning-agent +description: A valid description that has no example blocks and no trigger phrase +model: sonnet +color: blue +--- + +You are a test agent. Your job is to exist so the validator has something to warn about. +EOF +check "valid agent with warnings" 0 "$TMP_DIR/warning-agent.md" +if ! grep -q "Validation passed" "$TMP_DIR/out.txt"; then + echo "FAIL: summary line missing (script aborted before finishing)" + failures=$((failures + 1)) +fi + +# An invalid agent (bad name, missing color) must still fail with exit 1. +cat > "$TMP_DIR/invalid-agent.md" <<'EOF' +--- +name: x +description: A valid description for an otherwise invalid agent file +model: sonnet +--- + +You are a test agent with an invalid name and no color field. +EOF +check "invalid agent" 1 "$TMP_DIR/invalid-agent.md" + +echo "" +if [ "$failures" -gt 0 ]; then + echo "$failures test(s) failed" + exit 1 +fi +echo "All tests passed"