Skip to content

Commit 3db3ff8

Browse files
committed
refactor: add skill test case format schema, validation hook, and migrate instruction-builder-agent tests
1 parent 735918e commit 3db3ff8

20 files changed

Lines changed: 1177 additions & 441 deletions
Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,5 @@
1-
{"status": "open"}
1+
{
2+
"status" : "closed",
3+
"target_branch" : "v2.1",
4+
"resolution" : "implemented"
5+
}

client/src/main/java/io/github/cowwoc/cat/hooks/PreWriteHook.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import io.github.cowwoc.cat.hooks.write.EnforcePluginFileIsolation;
1515
import io.github.cowwoc.cat.hooks.write.EnforceWorktreePathIsolation;
1616
import io.github.cowwoc.cat.hooks.write.StateSchemaValidator;
17+
import io.github.cowwoc.cat.hooks.write.ValidateSkillTestFormat;
1718
import io.github.cowwoc.cat.hooks.write.WarnBaseBranchEdit;
1819
import tools.jackson.databind.JsonNode;
1920

@@ -45,7 +46,8 @@ public final class PreWriteHook implements HookHandler
4546
* <p>
4647
* Handlers are checked in order. EnforceWorkflowCompletion warns first, then WarnBaseBranchEdit
4748
* warns (non-blocking), followed by blocking handlers (StateSchemaValidator,
48-
* BlockGitconfigFileWrite, EnforcePluginFileIsolation, EnforceWorktreePathIsolation).
49+
* BlockGitconfigFileWrite, ValidateSkillTestFormat, EnforcePluginFileIsolation,
50+
* EnforceWorktreePathIsolation).
4951
*
5052
* @param scope the JVM scope providing project directory and shared services
5153
* @throws NullPointerException if {@code scope} is null
@@ -58,6 +60,7 @@ public PreWriteHook(ClaudeHook scope)
5860
new WarnBaseBranchEdit(scope),
5961
new StateSchemaValidator(scope),
6062
new BlockGitconfigFileWrite(),
63+
new ValidateSkillTestFormat(),
6164
new EnforcePluginFileIsolation(),
6265
new EnforceWorktreePathIsolation(scope));
6366
}
Lines changed: 298 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,298 @@
1+
/*
2+
* Copyright (c) 2026 Gili Tzabari. All rights reserved.
3+
*
4+
* Licensed under the CAT Commercial License.
5+
* See LICENSE.md in the project root for license terms.
6+
*/
7+
package io.github.cowwoc.cat.hooks.write;
8+
9+
import static io.github.cowwoc.cat.hooks.skills.JsonHelper.getStringOrDefault;
10+
import static io.github.cowwoc.requirements13.java.DefaultJavaValidators.requireThat;
11+
import static java.nio.charset.StandardCharsets.UTF_8;
12+
13+
import io.github.cowwoc.cat.hooks.FileWriteHandler;
14+
15+
import tools.jackson.databind.JsonNode;
16+
17+
import java.io.IOException;
18+
import java.nio.file.Files;
19+
import java.nio.file.Path;
20+
import java.util.ArrayList;
21+
import java.util.LinkedHashMap;
22+
import java.util.List;
23+
import java.util.Map;
24+
import java.util.Set;
25+
import java.util.regex.Matcher;
26+
import java.util.regex.Pattern;
27+
28+
/**
29+
* Validates skill test case markdown files on write.
30+
* <p>
31+
* Test case files are located at {@code plugin/skills/<skill>/test/*.md}. Each file must contain
32+
* YAML frontmatter with the required fields {@code type} and {@code category}, and the required
33+
* sections {@code ## Scenario}, {@code ## Tier 1 Assertion}, and
34+
* {@code ## Tier 2 Assertion}.
35+
* <p>
36+
* This hook runs as a PreToolUse handler for Write and Edit operations. Files that do not match the
37+
* {@code test/*.md} path pattern are passed through without validation.
38+
*/
39+
public final class ValidateSkillTestFormat implements FileWriteHandler
40+
{
41+
private static final Pattern TEST_MD_PATTERN =
42+
Pattern.compile("(?:^|/)plugin/skills/[^/]+/test/[^/]+\\.md$");
43+
private static final Pattern FRONTMATTER_PATTERN =
44+
Pattern.compile("\\A---\\n(.*?)\\n---\\n?", Pattern.DOTALL);
45+
private static final Set<String> REQUIRED_FRONTMATTER_FIELDS = Set.of("type", "category");
46+
private static final Map<String, Pattern> FRONTMATTER_FIELD_PATTERNS;
47+
48+
static
49+
{
50+
FRONTMATTER_FIELD_PATTERNS = new LinkedHashMap<>();
51+
for (String field : REQUIRED_FRONTMATTER_FIELDS)
52+
FRONTMATTER_FIELD_PATTERNS.put(field, Pattern.compile("^" + Pattern.quote(field) + ":\\s*(.+)$",
53+
Pattern.MULTILINE));
54+
}
55+
private static final Set<String> VALID_TYPES = Set.of("should-trigger", "should-not-trigger", "behavior");
56+
private static final List<String> REQUIRED_SECTIONS =
57+
List.of("## Scenario", "## Tier 1 Assertion", "## Tier 2 Assertion");
58+
59+
/**
60+
* Creates a new ValidateSkillTestFormat instance.
61+
*/
62+
public ValidateSkillTestFormat()
63+
{
64+
}
65+
66+
/**
67+
* Check if the write should be blocked due to skill test file format violations.
68+
*
69+
* @param toolInput the tool input JSON
70+
* @param sessionId the session ID
71+
* @return the check result
72+
* @throws NullPointerException if {@code toolInput} or {@code sessionId} are null
73+
* @throws IllegalArgumentException if {@code sessionId} is blank
74+
*/
75+
@Override
76+
public FileWriteHandler.Result check(JsonNode toolInput, String sessionId)
77+
{
78+
requireThat(toolInput, "toolInput").isNotNull();
79+
requireThat(sessionId, "sessionId").isNotBlank();
80+
81+
String filePath = getStringOrDefault(toolInput, "file_path", "");
82+
if (filePath.isEmpty())
83+
return FileWriteHandler.Result.allow();
84+
85+
if (!TEST_MD_PATTERN.matcher(filePath).find())
86+
return FileWriteHandler.Result.allow();
87+
88+
String content = getStringOrDefault(toolInput, "content", "");
89+
if (content.isEmpty())
90+
{
91+
// "content" is only present for Write operations. For Edit operations, the tool provides
92+
// "old_string" and "new_string" instead. Reconstruct the expected post-edit file content by
93+
// applying the replacement to the on-disk file so we can validate the result.
94+
String newString = getStringOrDefault(toolInput, "new_string", "");
95+
if (newString.isEmpty())
96+
return FileWriteHandler.Result.allow();
97+
String oldString = getStringOrDefault(toolInput, "old_string", "");
98+
EditResult editResult = applyEdit(filePath, oldString, newString);
99+
if (editResult.exception != null)
100+
{
101+
return FileWriteHandler.Result.block("""
102+
Edit validation failed: Could not read %s to validate post-edit content.
103+
This may indicate a file system issue or permission problem.
104+
Error: %s: %s
105+
106+
Verify the file path is correct and that you have read access. If the file was created \
107+
in this session, ensure the Write tool completed successfully before using the Edit tool.""".
108+
formatted(filePath, editResult.exception.getClass().getSimpleName(),
109+
editResult.exception.getMessage()));
110+
}
111+
content = editResult.content;
112+
if (content.isEmpty())
113+
return FileWriteHandler.Result.block("""
114+
Edit rejected: old_string not found in %s.
115+
The file on disk does not contain the text being replaced.
116+
117+
Read the current file content first to get the exact text, then retry the Edit with \
118+
an old_string that matches the current file exactly.""".formatted(filePath));
119+
}
120+
121+
return validateContent(content, filePath);
122+
}
123+
124+
/**
125+
* Validate the content of a skill test case markdown file.
126+
*
127+
* @param content the markdown content to validate
128+
* @param filePath the file path, used in error messages
129+
* @return validation result
130+
*/
131+
private FileWriteHandler.Result validateContent(String content, String filePath)
132+
{
133+
Matcher frontmatterMatcher = FRONTMATTER_PATTERN.matcher(content);
134+
if (!frontmatterMatcher.find())
135+
{
136+
return FileWriteHandler.Result.block("""
137+
Skill test format violation in %s: missing YAML frontmatter.
138+
139+
Test case files must begin with a YAML frontmatter block containing:
140+
type: <should-trigger|should-not-trigger|behavior>
141+
category: <semantic category, e.g. routing>
142+
143+
See plugin/concepts/skill-test.md for the complete format specification.""".
144+
formatted(filePath));
145+
}
146+
147+
String frontmatterBody = frontmatterMatcher.group(1);
148+
FileWriteHandler.Result frontmatterResult = validateFrontmatter(frontmatterBody, filePath);
149+
if (frontmatterResult.blocked())
150+
return frontmatterResult;
151+
152+
return validateSections(content, filePath);
153+
}
154+
155+
/**
156+
* Validate YAML frontmatter fields.
157+
*
158+
* @param frontmatterBody the raw YAML frontmatter body (between the {@code ---} delimiters)
159+
* @param filePath the file path for error messages
160+
* @return validation result
161+
*/
162+
private FileWriteHandler.Result validateFrontmatter(String frontmatterBody, String filePath)
163+
{
164+
List<String> missingFields = new ArrayList<>();
165+
String typeValue = "";
166+
167+
for (Map.Entry<String, Pattern> entry : FRONTMATTER_FIELD_PATTERNS.entrySet())
168+
{
169+
String field = entry.getKey();
170+
Matcher matcher = entry.getValue().matcher(frontmatterBody);
171+
if (matcher.find())
172+
{
173+
String value = matcher.group(1).strip();
174+
if (field.equals("type"))
175+
typeValue = value;
176+
}
177+
else
178+
{
179+
missingFields.add(field);
180+
}
181+
}
182+
183+
if (!missingFields.isEmpty())
184+
{
185+
return FileWriteHandler.Result.block("""
186+
Skill test format violation in %s: missing required frontmatter field(s): %s.
187+
188+
Required frontmatter fields:
189+
type: <should-trigger|should-not-trigger|behavior>
190+
category: <semantic category, e.g. routing>
191+
192+
See plugin/concepts/skill-test.md for the complete format specification.""".
193+
formatted(filePath, String.join(", ", missingFields)));
194+
}
195+
196+
if (!typeValue.isEmpty() && !VALID_TYPES.contains(typeValue))
197+
{
198+
return FileWriteHandler.Result.block("""
199+
Skill test format violation in %s: invalid 'type' value '%s'.
200+
201+
'type' must be one of: should-trigger, should-not-trigger, behavior
202+
203+
See plugin/concepts/skill-test.md for the complete format specification.""".
204+
formatted(filePath, typeValue));
205+
}
206+
207+
return FileWriteHandler.Result.allow();
208+
}
209+
210+
/**
211+
* Validate that all required markdown sections are present.
212+
*
213+
* @param content the full markdown content
214+
* @param filePath the file path for error messages
215+
* @return validation result
216+
*/
217+
private FileWriteHandler.Result validateSections(String content, String filePath)
218+
{
219+
List<String> missingSections = new ArrayList<>();
220+
for (String section : REQUIRED_SECTIONS)
221+
{
222+
if (!content.contains(section))
223+
missingSections.add(section);
224+
}
225+
226+
if (!missingSections.isEmpty())
227+
{
228+
return FileWriteHandler.Result.block("""
229+
Skill test format violation in %s: missing required section(s): %s.
230+
231+
Each test case file must contain all of these sections:
232+
## Scenario
233+
## Tier 1 Assertion
234+
## Tier 2 Assertion
235+
236+
See plugin/concepts/skill-test.md for the complete format specification.""".
237+
formatted(filePath, String.join(", ", missingSections)));
238+
}
239+
240+
return FileWriteHandler.Result.allow();
241+
}
242+
243+
/**
244+
* Container for applyEdit result with optional exception.
245+
* <p>
246+
* When {@code content} is non-empty, the edit was successfully applied. When {@code content} is
247+
* empty and {@code exception} is non-null, the file could not be read. When both are empty/null,
248+
* the old_string was not found in the file.
249+
*/
250+
private static class EditResult
251+
{
252+
final String content;
253+
final IOException exception;
254+
255+
EditResult(String content)
256+
{
257+
this.content = content;
258+
this.exception = null;
259+
}
260+
261+
EditResult(IOException exception)
262+
{
263+
this.content = "";
264+
this.exception = exception;
265+
}
266+
}
267+
268+
/**
269+
* Apply an Edit tool's string replacement to the on-disk file content.
270+
* <p>
271+
* Returns an EditResult containing either the post-edit content or an IOException if the file
272+
* could not be read.
273+
*
274+
* @param filePath the path to the file on disk
275+
* @param oldString the substring to replace
276+
* @param newString the replacement string
277+
* @return EditResult with post-edit content if successful, empty if old_string not found, or
278+
* exception if file unreadable
279+
*/
280+
private EditResult applyEdit(String filePath, String oldString, String newString)
281+
{
282+
try
283+
{
284+
String diskContent = Files.readString(Path.of(filePath), UTF_8);
285+
// Replace only the first occurrence to match Edit tool semantics
286+
int index = diskContent.indexOf(oldString);
287+
if (index == -1)
288+
return new EditResult(""); // oldString not found — caller blocks
289+
String result = diskContent.substring(0, index) + newString +
290+
diskContent.substring(index + oldString.length());
291+
return new EditResult(result);
292+
}
293+
catch (IOException e)
294+
{
295+
return new EditResult(e);
296+
}
297+
}
298+
}

0 commit comments

Comments
 (0)