Skip to content

Commit 0989f29

Browse files
bchernybcherny
committed
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 (#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 <example> 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
1 parent 8b6ef81 commit 0989f29

2 files changed

Lines changed: 101 additions & 23 deletions

File tree

‎plugins/plugin-dev/skills/agent-development/scripts/validate-agent.sh‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -56,74 +56,82 @@ error_count=0
5656
warning_count=0
5757

5858
# Check name field
59-
NAME=$(echo "$FRONTMATTER" | grep '^name:' | sed 's/name: *//' | sed 's/^"\(.*\)"$/\1/')
59+
# The "|| true" on each field extraction keeps grep's no-match exit status from
60+
# killing the script under set -e; a missing field is reported below instead.
61+
NAME=$(echo "$FRONTMATTER" | grep '^name:' | sed 's/name: *//' | sed 's/^"\(.*\)"$/\1/' || true)
6062

6163
if [ -z "$NAME" ]; then
6264
echo "❌ Missing required field: name"
63-
((error_count++))
65+
error_count=$((error_count + 1))
6466
else
6567
echo "✅ name: $NAME"
6668

6769
# Validate name format
6870
if ! [[ "$NAME" =~ ^[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]$ ]]; then
6971
echo "❌ name must start/end with alphanumeric and contain only letters, numbers, hyphens"
70-
((error_count++))
72+
error_count=$((error_count + 1))
7173
fi
7274

7375
# Validate name length
7476
name_length=${#NAME}
7577
if [ $name_length -lt 3 ]; then
7678
echo "❌ name too short (minimum 3 characters)"
77-
((error_count++))
79+
error_count=$((error_count + 1))
7880
elif [ $name_length -gt 50 ]; then
7981
echo "❌ name too long (maximum 50 characters)"
80-
((error_count++))
82+
error_count=$((error_count + 1))
8183
fi
8284

8385
# Check for generic names
8486
if [[ "$NAME" =~ ^(helper|assistant|agent|tool)$ ]]; then
8587
echo "⚠️ name is too generic: $NAME"
86-
((warning_count++))
88+
warning_count=$((warning_count + 1))
8789
fi
8890
fi
8991

9092
# Check description field
91-
DESCRIPTION=$(echo "$FRONTMATTER" | grep '^description:' | sed 's/description: *//')
93+
# The description may span multiple lines (unquoted prose plus <example> blocks),
94+
# so capture everything from "description:" until the next top-level agent key.
95+
DESCRIPTION=$(echo "$FRONTMATTER" | awk '
96+
/^description:/ { in_description=1; sub(/^description:[[:space:]]*/, ""); print; next }
97+
/^(name|model|color|tools):/ { in_description=0 }
98+
in_description { print }
99+
')
92100

93101
if [ -z "$DESCRIPTION" ]; then
94102
echo "❌ Missing required field: description"
95-
((error_count++))
103+
error_count=$((error_count + 1))
96104
else
97105
desc_length=${#DESCRIPTION}
98106
echo "✅ description: ${desc_length} characters"
99107

100108
if [ $desc_length -lt 10 ]; then
101109
echo "⚠️ description too short (minimum 10 characters recommended)"
102-
((warning_count++))
110+
warning_count=$((warning_count + 1))
103111
elif [ $desc_length -gt 5000 ]; then
104112
echo "⚠️ description very long (over 5000 characters)"
105-
((warning_count++))
113+
warning_count=$((warning_count + 1))
106114
fi
107115

108116
# Check for example blocks
109117
if ! echo "$DESCRIPTION" | grep -q '<example>'; then
110118
echo "⚠️ description should include <example> blocks for triggering"
111-
((warning_count++))
119+
warning_count=$((warning_count + 1))
112120
fi
113121

114122
# Check for "Use this agent when" pattern
115123
if ! echo "$DESCRIPTION" | grep -qi 'use this agent when'; then
116124
echo "⚠️ description should start with 'Use this agent when...'"
117-
((warning_count++))
125+
warning_count=$((warning_count + 1))
118126
fi
119127
fi
120128

121129
# Check model field
122-
MODEL=$(echo "$FRONTMATTER" | grep '^model:' | sed 's/model: *//')
130+
MODEL=$(echo "$FRONTMATTER" | grep '^model:' | sed 's/model: *//' || true)
123131

124132
if [ -z "$MODEL" ]; then
125133
echo "❌ Missing required field: model"
126-
((error_count++))
134+
error_count=$((error_count + 1))
127135
else
128136
echo "✅ model: $MODEL"
129137

@@ -133,17 +141,17 @@ else
133141
;;
134142
*)
135143
echo "⚠️ Unknown model: $MODEL (valid: inherit, sonnet, opus, haiku)"
136-
((warning_count++))
144+
warning_count=$((warning_count + 1))
137145
;;
138146
esac
139147
fi
140148

141149
# Check color field
142-
COLOR=$(echo "$FRONTMATTER" | grep '^color:' | sed 's/color: *//')
150+
COLOR=$(echo "$FRONTMATTER" | grep '^color:' | sed 's/color: *//' || true)
143151

144152
if [ -z "$COLOR" ]; then
145153
echo "❌ Missing required field: color"
146-
((error_count++))
154+
error_count=$((error_count + 1))
147155
else
148156
echo "✅ color: $COLOR"
149157

@@ -153,13 +161,13 @@ else
153161
;;
154162
*)
155163
echo "⚠️ Unknown color: $COLOR (valid: blue, cyan, green, yellow, magenta, red)"
156-
((warning_count++))
164+
warning_count=$((warning_count + 1))
157165
;;
158166
esac
159167
fi
160168

161169
# Check tools field (optional)
162-
TOOLS=$(echo "$FRONTMATTER" | grep '^tools:' | sed 's/tools: *//')
170+
TOOLS=$(echo "$FRONTMATTER" | grep '^tools:' | sed 's/tools: *//' || true)
163171

164172
if [ -n "$TOOLS" ]; then
165173
echo "✅ tools: $TOOLS"
@@ -173,23 +181,23 @@ echo "Checking system prompt..."
173181

174182
if [ -z "$SYSTEM_PROMPT" ]; then
175183
echo "❌ System prompt is empty"
176-
((error_count++))
184+
error_count=$((error_count + 1))
177185
else
178186
prompt_length=${#SYSTEM_PROMPT}
179187
echo "✅ System prompt: $prompt_length characters"
180188

181189
if [ $prompt_length -lt 20 ]; then
182190
echo "❌ System prompt too short (minimum 20 characters)"
183-
((error_count++))
191+
error_count=$((error_count + 1))
184192
elif [ $prompt_length -gt 10000 ]; then
185193
echo "⚠️ System prompt very long (over 10,000 characters)"
186-
((warning_count++))
194+
warning_count=$((warning_count + 1))
187195
fi
188196

189197
# Check for second person
190198
if ! echo "$SYSTEM_PROMPT" | grep -q "You are\|You will\|Your"; then
191199
echo "⚠️ System prompt should use second person (You are..., You will...)"
192-
((warning_count++))
200+
warning_count=$((warning_count + 1))
193201
fi
194202

195203
# Check for structure

‎plugins/plugin-dev/skills/agent-development/scripts/validate-agent.test.sh‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#!/bin/bash
2+
# Regression tests for validate-agent.sh (anthropics/claude-code#83803):
3+
# - warnings must not abort the run: under `set -e`, `((x++))` returns nonzero
4+
# when x was 0, so the first warning killed the script with exit 1
5+
# - multi-line descriptions (prose plus <example> blocks) must not be
6+
# false-flagged as missing examples
7+
set -uo pipefail
8+
9+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
10+
VALIDATOR="$SCRIPT_DIR/validate-agent.sh"
11+
PLUGIN_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)"
12+
TMP_DIR="$(mktemp -d)" || exit 1
13+
trap 'rm -rf "$TMP_DIR"' EXIT
14+
15+
failures=0
16+
17+
check() {
18+
local label="$1" expected="$2" file="$3"
19+
local actual=0
20+
bash "$VALIDATOR" "$file" > "$TMP_DIR/out.txt" 2>&1 || actual=$?
21+
if [ "$actual" -eq "$expected" ]; then
22+
echo "PASS: $label (exit $actual)"
23+
else
24+
echo "FAIL: $label (expected exit $expected, got $actual)"
25+
cat "$TMP_DIR/out.txt"
26+
failures=$((failures + 1))
27+
fi
28+
}
29+
30+
# The plugin's own agents are valid and must pass.
31+
for agent in "$PLUGIN_ROOT"/agents/*.md; do
32+
check "own agent $(basename "$agent")" 0 "$agent"
33+
done
34+
35+
# A valid agent whose fields only trigger warnings must still exit 0,
36+
# and must reach the summary line instead of aborting at the first warning.
37+
cat > "$TMP_DIR/warning-agent.md" <<'EOF'
38+
---
39+
name: warning-agent
40+
description: A valid description that has no example blocks and no trigger phrase
41+
model: sonnet
42+
color: blue
43+
---
44+
45+
You are a test agent. Your job is to exist so the validator has something to warn about.
46+
EOF
47+
check "valid agent with warnings" 0 "$TMP_DIR/warning-agent.md"
48+
if ! grep -q "Validation passed" "$TMP_DIR/out.txt"; then
49+
echo "FAIL: summary line missing (script aborted before finishing)"
50+
failures=$((failures + 1))
51+
fi
52+
53+
# An invalid agent (bad name, missing color) must still fail with exit 1.
54+
cat > "$TMP_DIR/invalid-agent.md" <<'EOF'
55+
---
56+
name: x
57+
description: A valid description for an otherwise invalid agent file
58+
model: sonnet
59+
---
60+
61+
You are a test agent with an invalid name and no color field.
62+
EOF
63+
check "invalid agent" 1 "$TMP_DIR/invalid-agent.md"
64+
65+
echo ""
66+
if [ "$failures" -gt 0 ]; then
67+
echo "$failures test(s) failed"
68+
exit 1
69+
fi
70+
echo "All tests passed"

0 commit comments

Comments
 (0)