ci(e2e): make the agent e2e lane actually load-bearing

.github/workflows/agent-e2e.yml

@@ -5,13 +5,32 @@ name: Agent E2E
 # the agent runtime on by default from 3.32.0-rc.8 onward. A separate agent
 # server JAR is no longer used here.
 #
-# These tests call real LLMs via the OPENAI_API_KEY / ANTHROPIC_API_KEY
-# repo secrets. The suites themselves never read the keys (asserted by
-# Suite2ToolCallingCredentials) — only the server process gets them.
-# Fork PRs cannot see repo secrets, so for them the run fails at the
-# silently-empty guard rather than passing vacuously.
-
-on: [pull_request, workflow_dispatch]
+# These tests call real LLMs via the OPENAI_API_KEY / ANTHROPIC_API_KEY repo
+# secrets. LLM and tool execution happens server-side, so the keys are consumed by
+# the server process. Only one suite reads a key at all: Suite7MediaTools gates its
+# live image-generation test on OPENAI_API_KEY.
+#
+# BaseTest.MODEL defaults to openai/gpt-4o-mini, so OPENAI_API_KEY is the one that
+# must be present; ANTHROPIC_API_KEY matters only when the model is overridden.
+#
+# Fork PRs cannot see repo secrets. The server still boots healthy — /health does not
+# check provider credentials — so the suites run and fail on their LLM tasks, and the
+# job fails at "Run e2e suites".
+#
+# There is no separate "did anything actually run?" guard: BaseTest fails hard rather
+# than assumeTrue-skipping when the server is unreachable, so a green run that
+# verified nothing is not reachable.
+
+# Runs on push-to-main as well as PRs, so a regression on main is caught at merge
+# rather than waiting for someone to open the next PR. Matches the sibling SDKs,
+# whose e2e jobs run on push+PR; no workflow there uses a schedule, so cost tracks
+# merge frequency rather than a clock, and the concurrency group above collapses
+# rapid pushes.
+on:
+  push:
+    branches: [main]
+  pull_request:
+  workflow_dispatch:
 
 permissions:
   contents: read
@@ -31,6 +50,9 @@ jobs:
       OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
       ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
       CONDUCTOR_SERVER_URL: http://localhost:8080/api
+      # Pinned rather than left to BaseTest's default, so CI cost cannot shift silently
+      # if that default changes.
+      CONDUCTOR_AGENT_LLM_MODEL: openai/gpt-4o-mini
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
@@ -41,13 +63,6 @@ jobs:
           distribution: temurin
           java-version: '21'
 
-      # Python is only needed for mcp-testkit and the XML guard.
-      # No `cache: pip` — it hard-fails without a requirements file.
-      - name: Set up Python
-        uses: actions/setup-python@v5
-        with:
-          python-version: '3.12'
-
       - name: Cache server JAR
         id: jar_cache
         uses: actions/cache@v4
@@ -61,15 +76,10 @@ jobs:
           curl -fL --retry 3 -o conductor-server.jar \
             "https://repo1.maven.org/maven2/org/conductoross/conductor-server/${CONDUCTOR_SERVER_VERSION}/conductor-server-${CONDUCTOR_SERVER_VERSION}-boot.jar"
 
-      - name: Install mcp-testkit
-        run: |
-          python -m pip install --upgrade pip
-          pip install mcp-testkit
-
-      - name: Start mcp-testkit
-        run: |
-          mcp-testkit --transport http --port 3001 &
-          sleep 2
+      # No MCP broker is started: Suite4McpTools is plan()-only and asserts against a
+      # fabricated http://localhost:9999/mcp, so nothing ever connected to the broker
+      # this job used to run on port 3001. Wiring live MCP execution means adding one
+      # back and pointing the suite at it.
 
       - name: Start server
         run: |
@@ -90,28 +100,6 @@ jobs:
       - name: Run e2e suites
         run: ./gradlew :e2e:test -Pe2e
 
-      # BaseTest assumeTrue-skips every suite when the server is unreachable —
-      # without this guard a boot failure after the health gate (or a future
-      # gate regression) would yield a green job that ran nothing.
-      - name: Guard against silently-empty runs
-        if: always()
-        run: |
-          python - <<'EOF'
-          import glob
-          import sys
-          import xml.etree.ElementTree as ET
-
-          total = executed = 0
-          for path in glob.glob("e2e/build/test-results/test/TEST-*.xml"):
-              root = ET.parse(path).getroot()
-              t = int(root.get("tests", 0))
-              sk = int(root.get("skipped", 0))
-              total += t
-              executed += t - sk
-          print(f"executed {executed}/{total} tests")
-          sys.exit(0 if executed > 0 else 1)
-          EOF
-
       - name: Upload results
         if: always()
         uses: actions/upload-artifact@v4

.github/workflows/ci.yml

@@ -134,9 +134,12 @@ jobs:
           distribution: "zulu"
           java-version: "21"
 
+      # `testClasses` is requested explicitly: `-x test` prunes compileTestJava along
+      # with the excluded `test` task, so without it test sources are only type-checked
+      # in the continue-on-error "Run Tests" step below.
       - name: Build
         id: build
-        run: ./gradlew clean build -x test
+        run: ./gradlew clean build testClasses -x test
 
       - name: Run Tests
         id: tests

e2e/build.gradle

@@ -46,4 +46,10 @@ tasks.withType(Test) {
 jacocoTestReport {
     dependsOn test // tests are required to run before generating the report
 }
-tasks.forEach(task -> task.onlyIf { project.hasProperty('e2e') })
+// Gate execution only. Gating *every* task (as this used to) also skips
+// compileTestJava and spotlessCheck, so `./gradlew build` never type-checked these
+// suites and they could rot to compile errors unnoticed. JacocoReport is gated
+// alongside Test because jacocoTestReport dependsOn test — left ungated it would try
+// to build a report from a skipped test task.
+tasks.withType(Test).configureEach { onlyIf { project.hasProperty('e2e') } }
+tasks.withType(JacocoReport).configureEach { onlyIf { project.hasProperty('e2e') } }

e2e/src/test/java/BaseTest.java

@@ -25,14 +25,13 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 import static org.junit.jupiter.api.Assertions.fail;
-import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
 /**
  * Base class for all e2e tests.
  *
  * <p>Provides:
  * <ul>
- *   <li>Server health check that skips tests if server is not available</li>
+ *   <li>Server health check that fails tests if the server is not available</li>
  *   <li>Helper methods to fetch workflow data from the server</li>
  *   <li>Helper to extract agentDef from a plan() result</li>
  * </ul>
@@ -43,8 +42,24 @@ public abstract class BaseTest {
     protected static final String SERVER_URL =
             System.getenv().getOrDefault("CONDUCTOR_SERVER_URL", "http://localhost:8080/api");
 
-    /** Base URL (without /api) for health checks and workflow fetches. */
-    protected static final String BASE_URL = SERVER_URL.replace("/api", "");
+    /** Base URL (without the {@code /api} suffix) for health checks and workflow fetches. */
+    protected static final String BASE_URL = stripApiSuffix(SERVER_URL);
+
+    /**
+     * Strip a single trailing {@code /api}, tolerating a trailing slash.
+     *
+     * <p>Deliberately not {@code replace("/api", "")}, which strips *every* occurrence:
+     * {@code https://api.example.com/api} collapsed to {@code https:/.example.com}, because
+     * the pattern also matches inside {@code //api...}. Harmless against localhost, broken
+     * against any hosted server.
+     */
+    private static String stripApiSuffix(String url) {
+        String resolved = url;
+        while (resolved.endsWith("/")) {
+            resolved = resolved.substring(0, resolved.length() - 1);
+        }
+        return resolved.endsWith("/api") ? resolved.substring(0, resolved.length() - "/api".length()) : resolved;
+    }
 
     /** LLM model to use in e2e tests. */
     protected static final String MODEL = System.getenv().getOrDefault("CONDUCTOR_AGENT_LLM_MODEL", "openai/gpt-4o-mini");
@@ -56,7 +71,14 @@ public abstract class BaseTest {
 
     /**
      * Check that the server is available before any tests in the class run.
-     * If the server is not reachable or unhealthy, all tests in the class are skipped.
+     * If the server is not reachable or unhealthy, all tests in the class fail.
+     *
+     * <p>Deliberately a hard failure, not {@code assumeTrue}. A skipped JUnit test is a
+     * <em>passing</em> JUnit test, so skipping here meant an unreachable server produced a
+     * green run that had verified nothing. Reaching these suites at all requires opting in
+     * with {@code -Pe2e} (see build.gradle), so by this point the caller has explicitly
+     * asked for e2e: being unable to reach a server is a failure of what they asked for,
+     * not a reason to quietly do nothing.
      */
     @BeforeAll
     static void checkServerHealth() {
@@ -75,12 +97,29 @@ static void checkServerHealth() {
                 Object h = body.get("healthy");
                 healthy = Boolean.TRUE.equals(h);
             }
-            assumeTrue(healthy, "Server at " + BASE_URL + " is not healthy — skipping e2e tests");
+            if (!healthy) {
+                throw new IllegalStateException(unreachable("server reported unhealthy"));
+            }
+        } catch (IllegalStateException e) {
+            throw e;
         } catch (Exception e) {
-            assumeTrue(false, "Server not available at " + BASE_URL + ": " + e.getMessage());
+            // ConnectException and friends often carry a null message — naming the type is
+            // more useful than printing "(null)".
+            String detail = e.getMessage() != null && !e.getMessage().isBlank()
+                    ? e.getClass().getSimpleName() + ": " + e.getMessage()
+                    : e.getClass().getSimpleName();
+            throw new IllegalStateException(unreachable(detail), e);
         }
     }
 
+    private static String unreachable(String detail) {
+        return "e2e suites need a reachable Conductor server, but " + BASE_URL + " is not usable ("
+                + detail + ").\n"
+                + "  Set CONDUCTOR_SERVER_URL, or start a server with the agent runtime enabled\n"
+                + "  (conductor-oss >= 3.32.0-rc.8). These suites also need server-side LLM\n"
+                + "  credentials: OPENAI_API_KEY for the default openai/gpt-4o-mini model.";
+    }
+
     /**
      * Fetch a full workflow execution from the server.
      *

e2e/src/test/java/SuiteHttpApi404.java

@@ -18,8 +18,8 @@
 import io.orkes.conductor.client.AgentClient;
 import io.orkes.conductor.client.ApiClient;
 import io.orkes.conductor.client.exceptions.AgentAPIException;
-import io.orkes.conductor.client.exceptions.AgentNotFoundException;
 import io.orkes.conductor.client.exceptions.AgentException;
+import io.orkes.conductor.client.exceptions.AgentNotFoundException;
 import io.orkes.conductor.client.http.OrkesAgentClient;
 
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;

tests/build.gradle

@@ -38,4 +38,7 @@ tasks.withType(Test) {
 jacocoTestReport {
     dependsOn test // tests are required to run before generating the report
 }
-tasks.forEach(task -> task.onlyIf { project.hasProperty('IntegrationTests') })
+// Gate execution only — see the equivalent note in e2e/build.gradle. Gating every
+// task also skipped compileTestJava and spotlessCheck for this module.
+tasks.withType(Test).configureEach { onlyIf { project.hasProperty('IntegrationTests') } }
+tasks.withType(JacocoReport).configureEach { onlyIf { project.hasProperty('IntegrationTests') } }

tests/src/test/java/io/orkes/conductor/client/http/EnvironmentClientTests.java

@@ -12,17 +12,18 @@
  */
 package io.orkes.conductor.client.http;
 
-import io.orkes.conductor.client.EnvironmentClient;
-import io.orkes.conductor.client.model.Tag;
-import io.orkes.conductor.client.model.environment.EnvironmentVariable;
-import io.orkes.conductor.client.util.ClientTestUtil;
+import java.util.List;
+import java.util.Optional;
+import java.util.UUID;
+
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
-import java.util.List;
-import java.util.Optional;
-import java.util.UUID;
+import io.orkes.conductor.client.EnvironmentClient;
+import io.orkes.conductor.client.model.Tag;
+import io.orkes.conductor.client.model.environment.EnvironmentVariable;
+import io.orkes.conductor.client.util.ClientTestUtil;
 
 public class EnvironmentClientTests {
 

tests/src/test/java/io/orkes/conductor/client/http/PromptClientTests.java

@@ -16,6 +16,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.conductoross.conductor.client.model.ai.PromptTemplate;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
@@ -29,7 +30,6 @@
 import io.orkes.conductor.client.model.integration.Category;
 import io.orkes.conductor.client.model.integration.IntegrationApiUpdate;
 import io.orkes.conductor.client.model.integration.IntegrationUpdate;
-import org.conductoross.conductor.client.model.ai.PromptTemplate;
 import io.orkes.conductor.client.util.ClientTestUtil;
 
 public class PromptClientTests {