bugfix: show full stack trace in preprocessor error block

Author: cowwoc

.cat/issues/v2/v2.1/fix-preprocessor-error-stacktrace/index.json

@@ -1,3 +1,5 @@
 {
-  "status": "open"
-}
+  "status" : "closed",
+  "resolution" : "implemented",
+  "target_branch" : "v2.1"
+}

.cat/issues/v2/v2.1/fix-preprocessor-error-stacktrace/plan.md

@@ -2,7 +2,145 @@
 
 ## Goal
 
-Fix preprocessor error to show full stacktrace instead of first line only. When a preprocessor directive fails, the error message currently shows only the exception class and method (e.g., `io.github.cowwoc.cat.hooks.skills.GetOutput.<init>()`). It should show the complete stack trace including all frames and cause chains to aid debugging.
+Fix preprocessor error to show full stacktrace instead of first line only. When a preprocessor directive fails,
+the error message currently shows only the exception class and method (e.g.,
+`io.github.cowwoc.cat.hooks.skills.GetOutput.<init>()`). It should show the complete stack trace including all
+frames and cause chains to aid debugging.
+
+## Research Findings
+
+### Root Cause
+
+In `client/src/main/java/io/github/cowwoc/cat/hooks/util/GetSkill.java`, the method `invokeSkillOutput()`
+(around lines 650-692) catches exceptions and extracts only `e.getMessage()` or falls back to
+`e.getClass().getName()`. For `NoSuchMethodException`, `getMessage()` returns just the constructor signature
+(e.g., `io.github.cowwoc.cat.hooks.skills.GetOutput.<init>()`), which is not human-readable.
+
+The `buildPreprocessorErrorMessage()` method (around lines 704-720) formats the `**Preprocessor Error**`
+block shown to agents. It currently includes only the short error message, not the full stack trace.
+
+Logback is configured to write to `System.err` only (`client/src/main/resources/logback.xml`). Since the
+binary runs as a subprocess, stderr is discarded — logging is useless for this error.
+
+### Fix Approach
+
+1. Capture the full stack trace via `StringWriter`/`e.printStackTrace(pw)` in both catch blocks in
+   `invokeSkillOutput()`.
+2. Update `buildPreprocessorErrorMessage()` to accept the stack trace string and include it as a
+   `**Stack Trace:**` section in the `**Preprocessor Error**` block.
+3. Use `e.toString()` instead of `e.getMessage()` as the primary error line (includes exception type).
+4. For `InvocationTargetException`, unwrap to the cause first, then capture its stack trace.
+
+### Key File
+
+`client/src/main/java/io/github/cowwoc/cat/hooks/util/GetSkill.java`
+
+Current catch blocks in `invokeSkillOutput()`:
+```java
+catch (InvocationTargetException e)
+{
+    Throwable cause = e.getCause();
+    if (cause == null)
+        cause = e;
+    String errorMsg = cause.getMessage();
+    if (errorMsg == null)
+        errorMsg = cause.getClass().getName();
+    return buildPreprocessorErrorMessage(originalDirective, errorMsg);
+}
+catch (Exception e)
+{
+    String errorMsg = e.getMessage();
+    if (errorMsg == null)
+        errorMsg = e.getClass().getName();
+    return buildPreprocessorErrorMessage(originalDirective, errorMsg);
+}
+```
+
+Current `buildPreprocessorErrorMessage()`:
+```java
+private static String buildPreprocessorErrorMessage(String originalDirective, String errorMsg)
+{
+    return """
+        ---
+        **Preprocessor Error**
+
+        A preprocessor directive failed while loading this skill.
+
+        **Directive:** `%s`
+        **Error:** %s
+
+        To report this bug, run: `/cat:feedback`
+        ---
+        """.formatted(originalDirective, errorMsg);
+}
+```
+
+### Updated Code
+
+Updated catch blocks in `invokeSkillOutput()`:
+```java
+catch (InvocationTargetException e)
+{
+    Throwable cause = e.getCause();
+    if (cause == null)
+        cause = e;
+    StringWriter sw = new StringWriter();
+    cause.printStackTrace(new PrintWriter(sw));
+    return buildPreprocessorErrorMessage(originalDirective, cause.toString(), sw.toString());
+}
+catch (Exception e)
+{
+    StringWriter sw = new StringWriter();
+    e.printStackTrace(new PrintWriter(sw));
+    return buildPreprocessorErrorMessage(originalDirective, e.toString(), sw.toString());
+}
+```
+
+Updated `buildPreprocessorErrorMessage()`:
+```java
+private static String buildPreprocessorErrorMessage(String originalDirective, String errorMsg,
+    String stackTrace)
+{
+    return """
+        ---
+        **Preprocessor Error**
+
+        A preprocessor directive failed while loading this skill.
+
+        **Directive:** `%s`
+        **Error:** %s
+
+        **Stack Trace:**
+        ```
+        %s
+        ```
+
+        To report this bug, run: `/cat:feedback`
+        ---
+        """.formatted(originalDirective, errorMsg, stackTrace.stripTrailing());
+}
+```
+
+### Required Imports
+
+Ensure these imports exist in `GetSkill.java`:
+- `java.io.PrintWriter`
+- `java.io.StringWriter`
+
+### Test Location
+
+Add regression test in:
+`client/src/test/java/io/github/cowwoc/cat/hooks/util/GetSkillTest.java`
+
+If that file doesn't exist, look for related test files near
+`client/src/test/java/io/github/cowwoc/cat/hooks/` and add to the most appropriate one, or create
+`GetSkillTest.java` with a license header.
+
+The test should:
+1. Simulate a failed directive invocation (or call `buildPreprocessorErrorMessage` directly if it's
+   accessible, otherwise test via `invokeSkillOutput` at a higher level).
+2. Assert the returned string contains a `**Stack Trace:**` section.
+3. Assert the stack trace section is non-empty (contains at least one frame).
 
 ## Pre-conditions
 
@@ -13,4 +151,37 @@ Fix preprocessor error to show full stacktrace instead of first line only. When
 - [ ] Bug fixed: preprocessor errors display the full stack trace, including all frames and cause chains
 - [ ] Regression test added: test verifies full stacktrace is included in the error output
 - [ ] No new issues introduced
-- [ ] E2E verification: reproduce the preprocessor error scenario and confirm the full stacktrace appears in the output
+- [ ] All existing tests pass (`mvn -f client/pom.xml test`)
+
+## Sub-Agent Waves
+
+### Wave 1
+
+- Implement fix in `GetSkill.java` and add regression test
+
+  Steps:
+  1. Read `client/src/main/java/io/github/cowwoc/cat/hooks/util/GetSkill.java` to find exact line
+     numbers and current code for `invokeSkillOutput()` and `buildPreprocessorErrorMessage()`.
+  2. Add `java.io.PrintWriter` and `java.io.StringWriter` imports if not already present.
+  3. Update the `InvocationTargetException` catch block: replace `getMessage()`/`getClass().getName()`
+     logic with `StringWriter`/`PrintWriter` stack capture using `cause.toString()` as error string.
+  4. Update the `Exception` catch block: replace `getMessage()`/`getClass().getName()` logic with
+     `StringWriter`/`PrintWriter` stack capture using `e.toString()` as error string.
+  5. Update `buildPreprocessorErrorMessage()` signature to accept `String stackTrace` as third
+     parameter and include `**Stack Trace:**` section in the formatted output.
+  6. Check whether `client/src/test/java/io/github/cowwoc/cat/hooks/util/GetSkillTest.java` exists.
+     - If it exists, add the new test to that file.
+     - If it does not exist, create it with a license header (Java block-comment format per
+       `.claude/rules/license-header.md`) and a TestNG class skeleton. Do NOT search for other
+       nearby test files — always use this exact path.
+  7. Add a test that triggers a preprocessor error and asserts:
+     - The returned string contains `**Stack Trace:**`
+     - The stack trace section is non-empty (contains at least one frame line, i.e., `at `)
+     - If `buildPreprocessorErrorMessage` is `private`, make it package-private (remove `private`,
+       no access modifier) so the test class in the same package can call it directly. Do NOT use
+       reflection.
+  8. Run `mvn -f client/pom.xml test` — all tests must pass.
+  9. Update `.cat/issues/v2/v2.1/fix-preprocessor-error-stacktrace/index.json` — set
+     `"status": "closed"`.
+  10. Commit all changes with message:
+      `bugfix: show full stack trace in preprocessor error block`

client/src/main/java/io/github/cowwoc/cat/hooks/util/GetSkill.java

@@ -16,6 +16,8 @@
 import io.github.cowwoc.cat.hooks.ShellParser;
 import java.io.IOException;
 import java.io.PrintStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.lang.reflect.InvocationTargetException;
 import java.net.URLDecoder;
 import java.net.URLEncoder;
@@ -677,31 +679,31 @@ private String invokeSkillOutput(String className, String[] arguments, String or
       Throwable cause = e.getCause();
       if (cause == null)
         cause = e;
-      String errorMsg = cause.getMessage();
-      if (errorMsg == null)
-        errorMsg = cause.getClass().getName();
-      return buildPreprocessorErrorMessage(originalDirective, errorMsg);
+      StringWriter stringWriter = new StringWriter();
+      cause.printStackTrace(new PrintWriter(stringWriter));
+      return buildPreprocessorErrorMessage(originalDirective, cause.toString(), stringWriter.toString());
     }
     catch (Exception e)
     {
-      String errorMsg = e.getMessage();
-      if (errorMsg == null)
-        errorMsg = e.getClass().getName();
-      return buildPreprocessorErrorMessage(originalDirective, errorMsg);
+      StringWriter stringWriter = new StringWriter();
+      e.printStackTrace(new PrintWriter(stringWriter));
+      return buildPreprocessorErrorMessage(originalDirective, e.toString(), stringWriter.toString());
     }
   }
 
   /**
    * Builds a user-friendly error message when a preprocessor directive fails.
    * <p>
-   * The message includes the directive that failed, the error details, and instructions for
-   * filing a bug report using {@code /cat:feedback}.
+   * The message includes the directive that failed, the error details, the full stack trace for
+   * debugging, and instructions for filing a bug report using {@code /cat:feedback}.
    *
    * @param originalDirective the original preprocessor directive text that failed
-   * @param errorMsg the error message from the exception
+   * @param errorMsg the error summary from the exception (via {@code toString()})
+   * @param stackTrace the full stack trace from the exception
    * @return a user-friendly error message with bug report instructions
    */
-  private static String buildPreprocessorErrorMessage(String originalDirective, String errorMsg)
+  static String buildPreprocessorErrorMessage(String originalDirective, String errorMsg,
+    String stackTrace)
   {
     return """
 
@@ -713,10 +715,15 @@ private static String buildPreprocessorErrorMessage(String originalDirective, St
       **Directive:** `%s`
       **Error:** %s
 
+      **Stack Trace:**
+      ```
+      %s
+      ```
+
       To report this bug, run: `/cat:feedback`
       ---
 
-      """.formatted(originalDirective, errorMsg);
+      """.formatted(originalDirective, errorMsg, stackTrace.stripTrailing());
   }
 
   /**

client/src/test/java/io/github/cowwoc/cat/hooks/test/GetSkillTest.java

@@ -2006,4 +2006,46 @@ public void runAcceptsJvmScopeWithoutStdin() throws IOException
       TestUtils.deleteDirectoryRecursively(tempPluginRoot);
     }
   }
+
+  /**
+   * Verifies that a preprocessor error includes the full stack trace.
+   * <p>
+   * When a directive references a class that does not exist, the resulting error message must contain
+   * a {@code **Stack Trace:**} section with at least one stack frame line (containing {@code at }).
+   *
+   * @throws IOException if an I/O error occurs
+   */
+  @Test
+  public void preprocessorErrorIncludesStackTrace() throws IOException
+  {
+    Path tempPluginRoot = Files.createTempDirectory("get-skill-test");
+    try (TestClaudeTool scope = new TestClaudeTool(tempPluginRoot, tempPluginRoot))
+    {
+      // Create a launcher file pointing to a class that does not exist so that Class.forName()
+      // throws ClassNotFoundException, triggering the catch (Exception e) block in invokeSkillOutput().
+      Path launcherDir = tempPluginRoot.resolve("client/bin");
+      Files.createDirectories(launcherDir);
+      Files.writeString(launcherDir.resolve("broken-launcher"),
+        "java -m test.module/com.example.nonexistent.BrokenClass\n",
+        UTF_8);
+
+      // Create a skill with a directive that references the broken launcher.
+      Path skillDir = tempPluginRoot.resolve("skills/broken-skill");
+      Files.createDirectories(skillDir);
+      Files.writeString(skillDir.resolve("first-use.md"),
+        "# Broken Skill\n!`\"broken-launcher\" arg1`\n",
+        UTF_8);
+
+      String agentId = UUID.randomUUID().toString();
+      GetSkill loader = new GetSkill(scope, List.of(agentId));
+      String output = loader.load("broken-skill");
+
+      requireThat(output, "output").contains("**Stack Trace:**");
+      requireThat(output, "output").contains("\tat ");
+    }
+    finally
+    {
+      TestUtils.deleteDirectoryRecursively(tempPluginRoot);
+    }
+  }
 }

plugin/rules/skill-workflow-compliance.md

@@ -8,3 +8,12 @@ mainAgent: true
 **ALWAYS**: Execute every step in sequence; if step doesn't apply, note why and continue
 
 Skills exist to enforce consistent processes. Shortcuts defeat their purpose.
+
+**Skills execute in the current agent's context — they are not subagents.**
+
+When the Skill tool returns content, that content is the skill's instructions. The current agent
+executes those instructions directly. There is no subprocess, no background task, and no
+`<task-notification>` to wait for. After the Skill tool returns, act on the instructions immediately.
+
+**NEVER** say "awaiting skill completion" or "the skill is running" — skills do not run independently.
+**ALWAYS** read the returned instructions and begin executing them in the next action.