refactor: thin main() methods, add run() delegates and MainTest for all non-hook CLI classes

Author: cowwoc

.cat/issues/v2/v2.1/thin-main-methods-and-add-run-tests/index.json

@@ -1 +1 @@
-{"status": "open", "dependencies": [], "blocks": []}
+{"status": "closed", "resolution": "implemented", "dependencies": [], "blocks": []}

.claude/rules/hooks.md

@@ -15,7 +15,7 @@ CAT has two categories of Java CLI tools, each with a different output contract:
 ### Hook Handlers (PreToolUse, PostToolUse, etc.)
 
 Hook handlers are invoked directly by Claude Code's hook execution engine. They must produce
-Claude Code's hook JSON format via `ClaudeHook`. Claude Code's hook engine parses this format.
+Claude Code's hook JSON format via `HookOutput`. Claude Code's hook engine parses this format.
 
 **Standard hook JSON output fields:**
 - `decision` (string) — e.g., `"block"` to indicate a blocked operation
@@ -30,10 +30,10 @@ cause stderr to be fed to Claude as plain text, losing the structured error.
 
 **Pattern for expected errors (IOException, IllegalArgumentException) in hook handlers:**
 ```java
-// Good — use ClaudeHook, write to stdout, exit 0
+// Good — use HookOutput, write to stdout, exit 0
 catch (IOException e)
 {
-  ClaudeHook hookOutput = new ClaudeHook(scope);
+  HookOutput hookOutput = new HookOutput(scope);
   System.out.println(hookOutput.block(e.getMessage()));
   System.exit(0);
 }
@@ -57,7 +57,7 @@ by the **skill itself**, not by Claude Code's hook engine. These tools may use a
 schema (e.g., `{"status":"...", "message":"..."}`) that the skill Markdown defines and parses.
 
 `{"status":"ERROR","message":"..."}` is correct for skill CLI tools — the skill parser reads `status`
-and `message` fields directly. `ClaudeHook.block()` would produce `{"decision":"block",...}` which
+and `message` fields directly. `HookOutput.block()` would produce `{"decision":"block",...}` which
 skill parsers do not recognize.
 
 **Pattern for expected errors (IOException) in skill CLI tools:**
@@ -73,7 +73,7 @@ catch (IOException e)
 
 **Pattern for unexpected errors (RuntimeException | AssertionError) in `main()`:**
 
-Unexpected errors in `main()` must be caught, logged, and converted to a `ClaudeHook.block()` response on stdout.
+Unexpected errors in `main()` must be caught, logged, and converted to a `HookOutput.block()` response on stdout.
 They must NOT be rethrown, as non-zero exit or uncaught exceptions prevent Claude Code from parsing the JSON response.
 
 ```java
@@ -89,7 +89,7 @@ public static void main(String[] args)
     {
       Logger log = LoggerFactory.getLogger(ClassName.class);
       log.error("Unexpected error", e);
-      System.out.println(new ClaudeHook(scope).block(
+      System.out.println(new HookOutput(scope).block(
         Objects.toString(e.getMessage(), e.getClass().getSimpleName())));
     }
   }

.claude/rules/java.md

@@ -2010,6 +2010,53 @@ public void displayWidthCalculatesFromConfiguration() throws IOException
 
 **Guideline:** If your test would fail when run on a different machine with different configuration (but the code still works correctly), you're testing implementation details, not requirements.
 
+### Test Through run(), Not Internal Methods
+When a CLI class has a `run()` method that parses command-line arguments and delegates to business logic, tests should
+exercise `run()` directly rather than calling the internal methods it delegates to. This ensures the argument parsing,
+error handling, and exit codes are tested as a unit.
+
+```java
+// ✅ CORRECT: Test through run() — exercises arg parsing + business logic together
+@Test
+public void missingTypeReturnsOne() throws IOException
+{
+  Path tempDir = Files.createTempDirectory("test-");
+  try (JvmScope scope = new TestClaudeTool(tempDir, tempDir))
+  {
+    ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+    PrintStream out = new PrintStream(buffer, true, StandardCharsets.UTF_8);
+    int result = GetAddOutput.run(scope, new String[]{"--name", "foo", "--version", "2.1"}, out);
+    requireThat(result, "result").isEqualTo(1);
+  }
+  finally
+  {
+    TestUtils.deleteDirectoryRecursively(tempDir);
+  }
+}
+
+// ❌ WRONG: Test internal method directly — skips arg parsing and exit code
+@Test
+public void missingTypeReturnsOne() throws IOException
+{
+  Path tempDir = Files.createTempDirectory("test-");
+  try (JvmScope scope = new TestClaudeTool(tempDir, tempDir))
+  {
+    GetAddOutput output = new GetAddOutput(scope);
+    String result = output.getOutput(new String[]{"--name", "foo", "--version", "2.1"});
+    // Doesn't test run()'s arg parsing, error messages, or exit codes
+  }
+  finally
+  {
+    TestUtils.deleteDirectoryRecursively(tempDir);
+  }
+}
+```
+
+**When internal method tests ARE acceptable:**
+- The internal method has complex logic independent of arg parsing (e.g., formatting, calculation)
+- The method is reused by multiple callers, not just `run()`
+- Testing through `run()` would require impractical setup (live git repo, network access)
+
 ### Testability Over Convenience
 If code cannot be tested in a thread-safe way (e.g., it reads from `System.in` or writes to `System.out`), ask the
 user's permission to update the API to make it testable. For example, add a method overload that accepts an

client/pom.xml

@@ -148,6 +148,11 @@
 				<configuration>
 					<!-- Force UTF-8 encoding for forked test JVM -->
 					<argLine>-Dstdin.encoding=UTF-8 -Dstdout.encoding=UTF-8 -Dstderr.encoding=UTF-8</argLine>
+					<!-- Add main classes to flat classpath so surefire's class scanner can resolve
+					     cross-module supertype references (e.g. TestSkillOutput implements SkillOutput) -->
+					<additionalClasspathElements>
+						<additionalClasspathElement>${project.build.outputDirectory}</additionalClasspathElement>
+					</additionalClasspathElements>
 					<environmentVariables>
 						<CLAUDE_PLUGIN_ROOT>${project.basedir}/../plugin</CLAUDE_PLUGIN_ROOT>
 					</environmentVariables>

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

@@ -175,6 +175,19 @@ private static String validateAgentId(JsonNode data)
     return value;
   }
 
+  /**
+   * Returns the path to the Claude environment file.
+   * <p>
+   * Subclasses must implement this to provide the env file path from {@code CLAUDE_ENV_FILE}
+   * or an injected value.
+   *
+   * @return the path to the env file
+   * @throws AssertionError if {@code CLAUDE_ENV_FILE} is not set in the environment
+   * @throws IllegalStateException if this scope is closed
+   */
+  @Override
+  public abstract Path getEnvFile();
+
   @Override
   public Path getProjectPath()
   {

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

@@ -25,25 +25,29 @@ public abstract class AbstractClaudeTool extends AbstractJvmScope implements Cla
   private final String sessionId;
   private final Path projectPath;
   private final Path pluginRoot;
+  private final Path envFile;
 
   /**
    * Creates a new abstract Claude tool scope with the given environment values.
    *
    * @param sessionId the Claude session ID
    * @param projectPath the project's root directory path (must be absolute)
    * @param pluginRoot the Claude plugin root directory path (must be absolute)
+   * @param envFile the path to the Claude environment file
    * @throws IllegalArgumentException if {@code sessionId} is blank, or if {@code projectPath} or
    *   {@code pluginRoot} are not absolute paths
-   * @throws NullPointerException if {@code projectPath} or {@code pluginRoot} are null
+   * @throws NullPointerException if {@code projectPath}, {@code pluginRoot}, or {@code envFile} are null
    */
-  protected AbstractClaudeTool(String sessionId, Path projectPath, Path pluginRoot)
+  protected AbstractClaudeTool(String sessionId, Path projectPath, Path pluginRoot, Path envFile)
   {
     requireThat(sessionId, "sessionId").isNotBlank();
     requireThat(projectPath, "projectPath").isNotNull().isAbsolute();
     requireThat(pluginRoot, "pluginRoot").isNotNull().isAbsolute();
+    requireThat(envFile, "envFile").isNotNull();
     this.sessionId = sessionId;
     this.projectPath = projectPath;
     this.pluginRoot = pluginRoot;
+    this.envFile = envFile;
   }
 
   @Override
@@ -66,4 +70,11 @@ public Path getPluginRoot()
     ensureOpen();
     return pluginRoot;
   }
+
+  @Override
+  public Path getEnvFile()
+  {
+    ensureOpen();
+    return envFile;
+  }
 }

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

@@ -33,6 +33,8 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 
+import static io.github.cowwoc.requirements13.java.DefaultJavaValidators.requireThat;
+
 /**
  * Exercises all handler code paths in a single JVM invocation for AOT training.
  * <p>
@@ -71,80 +73,103 @@ public static void main(String[] args) throws Exception
     try (AbstractClaudeHook scope = new MainClaudeHook())
     {
       System.setIn(originalIn);
+      System.exit(run(scope));
+    }
+  }
 
-      // Hook handlers all accept the unified ClaudeHook scope
-      new PreToolUseHook(scope).run(scope);
-      new PostBashHook().run(scope);
-      new PreReadHook(scope).run(scope);
-      new PostReadHook(scope).run(scope);
-      new PostToolUseHook(scope).run(scope);
-      new UserPromptSubmitHook(scope).run(scope);
-      new PreAskHook(scope).run(scope);
-      new PreWriteHook(scope).run(scope);
-      new PreIssueHook(scope).run(scope);
-      new SessionEndHook(scope).run(scope);
-      new SessionStartHook(scope, Path.of("/tmp/aot-training-env")).run(scope);
-      new SubagentStartHook(scope).run(scope);
+  /**
+   * Exercises all hook handler and skill constructor code paths for AOT training.
+   * <p>
+   * SYNC: Keep handler list synchronized with HANDLERS array in hooks/build-jlink.sh.
+   * When adding a new handler, update both locations:
+   * <ul>
+   *   <li>Add launcher entry to HANDLERS array in build-jlink.sh</li>
+   *   <li>Add training invocation to this method</li>
+   * </ul>
+   *
+   * @param scope the hook scope providing access to services and configuration
+   * @throws NullPointerException if {@code scope} is null
+   * @throws Exception if training fails
+   * @return 0 on success, non-zero on failure
+   */
+  @SuppressWarnings("ResultOfMethodCallIgnored")
+  public static int run(AbstractClaudeHook scope) throws Exception
+  {
+    requireThat(scope, "scope").isNotNull();
 
-      // Skill handlers - construct to load class graphs.
-      // Calling getOutput() would read the filesystem, which is unnecessary for training.
-      // GetDiffOutput and GetCleanupOutput accept JvmScope (no session required).
-      // GetStatusOutput and GetOutput require AbstractClaudeTool (session-aware); use referenceClass() instead.
-      new GetDiffOutput(scope);
-      new GetCleanupOutput(scope);
-      referenceClass(GetStatusOutput.class);
-      referenceClass(GetOutput.class);
+    // Hook handlers all accept the unified ClaudeHook scope
+    new PreToolUseHook(scope).run(scope);
+    new PostBashHook().run(scope);
+    new PreReadHook(scope).run(scope);
+    new PostReadHook(scope).run(scope);
+    new PostToolUseHook(scope).run(scope);
+    new UserPromptSubmitHook(scope).run(scope);
+    new PreAskHook(scope).run(scope);
+    new PreWriteHook(scope).run(scope);
+    new PreIssueHook(scope).run(scope);
+    new SessionEndHook(scope).run(scope);
+    new SessionStartHook(scope).run(scope);
+    new SubagentStartHook(scope).run(scope);
 
-      // VerifyAudit training - create temp directory with plan.md for prepare() and minimal JSON for report()
-      Path tempDir = Files.createTempDirectory("aot-training-");
-      try
-      {
-        Path planFile = tempDir.resolve("plan.md");
-        Files.writeString(planFile, """
-          # Plan
-          ## Post-conditions
-          - [ ] Test criterion
-          ## Files to Modify
-          - test.md
-          """);
+    // Skill handlers - construct to load class graphs.
+    // Calling getOutput() would read the filesystem, which is unnecessary for training.
+    // GetDiffOutput and GetCleanupOutput accept JvmScope (no session required).
+    // GetStatusOutput and GetOutput require AbstractClaudeTool (session-aware); use referenceClass() instead.
+    new GetDiffOutput(scope);
+    new GetCleanupOutput(scope);
+    referenceClass(GetStatusOutput.class);
+    referenceClass(GetOutput.class);
 
-        VerifyAudit audit = new VerifyAudit(scope);
-        String prepareArgs = """
-          {
-            "issue_id": "aot-training",
-            "issue_path": "%s",
-            "worktree_path": "%s"
-          }
-          """.formatted(tempDir.toString(), tempDir.toString());
-        audit.prepare(prepareArgs);
-        audit.report("test-issue", "{\"criteria_results\": [], \"file_results\": {\"modify\": {}, \"delete\": {}}}");
-      }
-      finally
-      {
-        Files.deleteIfExists(tempDir.resolve("plan.md"));
-        Files.deleteIfExists(tempDir);
-      }
+    // VerifyAudit training - create temp directory with plan.md for prepare() and minimal JSON for report()
+    Path tempDir = Files.createTempDirectory("aot-training-");
+    try
+    {
+      Path planFile = tempDir.resolve("plan.md");
+      Files.writeString(planFile, """
+        # Plan
+        ## Post-conditions
+        - [ ] Test criterion
+        ## Files to Modify
+        - test.md
+        """);
 
-      // Reference arg-based classes to force class loading without invoking main()
-      // (their main() calls System.exit on missing args)
-      referenceClass(EnforceStatusOutput.class);
-      referenceClass(TokenCounter.class);
-      referenceClass(GetCheckpointOutput.class);
-      referenceClass(GetIssueCompleteOutput.class);
-      referenceClass(GetNextIssueOutput.class);
-      referenceClass(SessionAnalyzer.class);
-      referenceClass(ProgressBanner.class);
-      referenceClass(EmpiricalTestRunner.class);
-      referenceClass(WorkPrepare.class);
-      referenceClass(MarkdownWrapper.class);
-      referenceClass(BatchReader.class);
-      referenceClass(GetSubagentStatusOutput.class);
-      referenceClass(HookRegistrar.class);
-      referenceClass(StatusAlignmentValidator.class);
-      referenceClass(GetSkill.class);
-      referenceClass(GetFile.class);
-      referenceClass(Feedback.class);
+      VerifyAudit audit = new VerifyAudit(scope);
+      String prepareArgs = """
+        {
+          "issue_id": "aot-training",
+          "issue_path": "%s",
+          "worktree_path": "%s"
+        }
+        """.formatted(tempDir.toString(), tempDir.toString());
+      audit.prepare(prepareArgs);
+      audit.report("test-issue", "{\"criteria_results\": [], \"file_results\": {\"modify\": {}, \"delete\": {}}}");
+    }
+    finally
+    {
+      Files.deleteIfExists(tempDir.resolve("plan.md"));
+      Files.deleteIfExists(tempDir);
     }
+
+    // Reference arg-based classes to force class loading without invoking main()
+    // (their main() calls System.exit on missing args)
+    referenceClass(EnforceStatusOutput.class);
+    referenceClass(TokenCounter.class);
+    referenceClass(GetCheckpointOutput.class);
+    referenceClass(GetIssueCompleteOutput.class);
+    referenceClass(GetNextIssueOutput.class);
+    referenceClass(SessionAnalyzer.class);
+    referenceClass(ProgressBanner.class);
+    referenceClass(EmpiricalTestRunner.class);
+    referenceClass(WorkPrepare.class);
+    referenceClass(MarkdownWrapper.class);
+    referenceClass(BatchReader.class);
+    referenceClass(GetSubagentStatusOutput.class);
+    referenceClass(HookRegistrar.class);
+    referenceClass(StatusAlignmentValidator.class);
+    referenceClass(GetSkill.class);
+    referenceClass(GetFile.class);
+    referenceClass(Feedback.class);
+    return 0;
   }
 
   /**

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

@@ -10,6 +10,8 @@
 import tools.jackson.databind.json.JsonMapper;
 import tools.jackson.databind.node.ObjectNode;
 
+import java.nio.file.Path;
+
 /**
  * A {@link JvmScope} for hook handler processes that combines the Claude session environment
  * (project path, plugin root, config dir), hook input data, and hook output building in a single
@@ -28,6 +30,15 @@ public interface ClaudeHook extends JvmScope
    */
   String getSessionId();
 
+  /**
+   * Returns the path to the Claude environment file.
+   *
+   * @return the path to the env file
+   * @throws AssertionError if {@code CLAUDE_ENV_FILE} is not set in the environment
+   * @throws IllegalStateException if this scope is closed
+   */
+  Path getEnvFile();
+
   // Hook input methods
 
   /**