test(ui): run the live Playwright suite in CI against the built UI image

.github/workflows/ci.yaml

@@ -42,7 +42,9 @@ jobs:
       VERSION: v0.0.1-test
       SUBSTRATE_VERSION: 0.2.0-beta2
     runs-on: blacksmith-4vcpu-ubuntu-2404
-    timeout-minutes: 40
+    # 50: main's own 40, kept as headroom. The job itself takes around 10 minutes,
+    # UI image and browser suite included, on a warm buildx cache.
+    timeout-minutes: 50
     steps:
       - name: Checkout repository
         uses: actions/checkout@v6
@@ -115,7 +117,6 @@ jobs:
           KMCP_ENABLED: "false"
           KAGENT_HELM_EXTRA_ARGS: >-
             --cleanup-on-fail=false
-            --set ui.replicas=0
             --set kagent-tools.enabled=false
             --set grafana-mcp.enabled=false
             --set controller.substrate.enabled=true
@@ -133,11 +134,21 @@ jobs:
           BUILDX_BUILDER_NAME=$(docker buildx inspect | awk '$1 == "Name:" { print $2; exit }')
           test -n "$BUILDX_BUILDER_NAME"
           export BUILDX_BUILDER_NAME
+          # Before the fan-out. Each `build-*` runs it again — it is not passed `-o` the
+          # way `proto-generate` is below — which is harmless only because the target
+          # documents itself as safe to run concurrently.
           make buildx-create
-          printf '%s\n' controller golang-adk claude-harness codex-harness byo-a2a | xargs -P4 -n1 bash -c '
+          # Likewise once, and for a sharper reason: `build-ui` and `build-golang-adk` both
+          # depend on it, and `buf generate` writes `ui/src/generated` as well as
+          # `go/api/gen` — so in parallel one make would write into the directory the
+          # other is tarring as a build context. `-o proto-generate` below says it is done.
+          make proto-generate
+          # `ui` is here so the browser suite below runs against the image an operator
+          # gets: nginx, the SPA fallback, and the env-config.js rendered at pod start.
+          printf '%s\n' controller ui golang-adk claude-harness codex-harness byo-a2a | xargs -P5 -n1 bash -c '
             image="$1"
             DOCKER_BUILD_ARGS="--platform=linux/amd64 --push" \
-              make GIT_COMMIT=e2e BUILD_DATE=1970-01-01 "build-${image}"
+              make -o proto-generate GIT_COMMIT=e2e BUILD_DATE=1970-01-01 "build-${image}"
           ' _
           make helm-install-provider
           kubectl rollout status deployment/kagent-controller -n kagent --timeout=120s
@@ -178,6 +189,111 @@ jobs:
           export KAGENT_E2E_OTLP_LISTEN_ADDRESS=:14317
           go test -v github.com/kagent-dev/kagent/go/core/test/e2e -failfast -shuffle=on -parallel 4 -count=1
 
+      # The `smoke` template is applied part-way through the step above, after four
+      # digest lookups. The browser journeys below need an agent to schedule and a
+      # template to list, so a step that died before that apply would fail them for a
+      # reason that has nothing to do with the UI — three retries each, against an empty
+      # cluster. `continue-on-error`, because a missing template means the Go step has
+      # already failed the job and this only decides whether the UI steps are worth
+      # running: `outcome` still reports the failure the gate below reads.
+      - name: Check the cluster has the smoke agent
+        id: smoke-agent
+        if: ${{ !cancelled() && steps.install-kagent.outcome == 'success' }}
+        continue-on-error: true
+        # Retried, because everything below is gated on this one command. Asked once, a
+        # transient API-server or kubeconfig hiccup skips the whole browser lane and the
+        # job still reports green — which is the shape of #2638, a suite that measured
+        # nothing while every check passed.
+        run: |
+          for attempt in $(seq 1 10); do
+            if kubectl get agenttemplate smoke -n kagent; then exit 0; fi
+            echo "attempt ${attempt}: not there yet"
+            sleep 6
+          done
+          kubectl get agenttemplate -A || true
+          exit 1
+
+      # And when it is genuinely absent, said out loud on the run. A skipped lane and a
+      # passing one look identical in the checks list otherwise.
+      - name: Say so if the UI suite was skipped
+        if: ${{ !cancelled() && steps.smoke-agent.outcome != 'success' }}
+        run: |
+          echo "::warning title=UI live suite skipped::No smoke agent on the cluster, so the live Playwright lane did not run."
+
+      # The UI, against this same cluster and the image built above. `ui-tests` runs on
+      # the in-browser mock, so a backend contract change cannot fail it — which is how
+      # #2638 emptied three Substrate fields with every check green.
+      #
+      # After the Go tests, not beside them: both create and delete resources in the same
+      # namespace, and these journeys read the lists back.
+      #
+      # Gated on the cluster being usable rather than on `success()`, so one failed Go
+      # test under `-failfast` does not also cost every UI signal. No cluster or no
+      # agent, no run.
+      - name: Setup Node.js
+        if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
+        uses: actions/setup-node@v7
+        with:
+          node-version-file: ui/.nvmrc
+
+      # Before the cache step, as in `ui-tests`: package.json pins Yarn 4, and the
+      # runner's own shim cannot read this lock file.
+      - name: Enable Corepack
+        if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
+        run: corepack enable
+
+      - name: Cache Yarn downloads
+        if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
+        uses: actions/cache@v6
+        with:
+          path: ui/.yarn/cache
+          key: yarn-${{ runner.os }}-${{ hashFiles('ui/yarn.lock') }}
+          restore-keys: yarn-${{ runner.os }}-
+
+      - name: Prepare the live browser suite
+        if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
+        working-directory: ./ui
+        # Chromium only: the live suite declares one project. The mock suite's second
+        # engine is there to disagree about layout, which no backend contract rests on.
+        run: |
+          yarn install --immutable
+          yarn playwright install --with-deps chromium
+
+      - name: Run live browser tests
+        if: ${{ !cancelled() && steps.smoke-agent.outcome == 'success' }}
+        working-directory: ./ui
+        run: |
+          kubectl -n kagent rollout status deploy/kagent-ui --timeout=5m
+          # Already a LoadBalancer on a MetalLB cluster, so this is the address the
+          # controller e2e step takes — not a port-forward to keep alive for the run.
+          UI_IP="$(kubectl get svc -n kagent kagent-ui -o jsonpath='{.status.loadBalancer.ingress[0].ip}')"
+          test -n "$UI_IP"
+          export UI_LOOP_LIVE_URL="http://${UI_IP}:8080"
+          echo "UI_LOOP_LIVE_URL: $UI_LOOP_LIVE_URL"
+          # `globalSetup` refuses the run if that address answers with fixtures, or is a
+          # dev server rather than the built image.
+          yarn test:pw:live
+
+      - name: Upload live browser report
+        # Any finished run, not just a failed one: CI retries twice, so a spec that
+        # fails and then passes leaves a green job and no trace of the failure — which
+        # is the run whose trace is worth the most. A clean run uploads the HTML report
+        # and nothing else, since `test-results` only has content when something failed.
+        if: ${{ !cancelled() && steps.install-kagent.outcome == 'success' }}
+        uses: actions/upload-artifact@v5
+        with:
+          name: ui-live-playwright-report
+          # The trace is the only record of what the cluster answered — there is no fixed
+          # fixture to re-read afterwards, as there would be for the mock suite. The HTML
+          # report is what links one to the other, and it exists because the config asks
+          # for `html` alongside `github`; `github` on its own writes annotations and no
+          # files, which is how this path came to be uploaded empty.
+          path: |
+            ui/playwright-report
+            ui/test-results
+          retention-days: 7
+          if-no-files-found: ignore
+
       - name: fail print info
         if: failure()
         run: |
@@ -190,6 +306,10 @@ jobs:
           kubectl get harnesses,agenttemplates -n kagent
           echo "::error::Kubectl logs -n kagent deployment/kagent-controller"
           kubectl logs -n kagent deployment/kagent-controller
+          # nginx logs every proxied request, so a browser failure that was really a
+          # backend failure says so here, as does an init.sh that rejected a value.
+          echo "::error::Kubectl logs -n kagent deployment/kagent-ui"
+          kubectl logs -n kagent deployment/kagent-ui --tail=200 || true
           kubectl get pods,actortemplates,workerpools -A
           echo "::error::Substrate logs"
           kubectl logs -n ate-system -l app --all-containers --prefix --tail=200 || true

ui/eslint.config.mjs

@@ -74,9 +74,14 @@ export default tseslint.config(
    *
    * `playwright/README.md` states these; a convention nothing checks is one that
    * regrows as an exception.
+   *
+   * Every folder that holds specs, not only `tests/`: `shared/` runs in every project
+   * and `live/` is the only suite that talks to a cluster, so a spec there trusting its
+   * own green matters more rather than less. Scoping this to `tests/` alone left both
+   * outside the guard from the day they were added.
    */
   {
-    files: ["playwright/tests/**/*.spec.ts"],
+    files: ["playwright/{tests,shared,live}/**/*.spec.ts"],
     rules: {
       "no-restricted-imports": [
         "error",

ui/playwright.config.ts

@@ -27,6 +27,21 @@ const EXTENSION_BASE_URL = `http://localhost:${EXTENSION_PORT}`;
 /** Specs that need an extension installed opt in by filename. */
 const EXTENSION_SPECS = /\.withExtension\.spec\.ts$/;
 
+/**
+ * Specs that run in both suites, claimed by every project below.
+ *
+ * Only what holds on either backend — no `?mock=` scenario, no fixture by name. That
+ * narrowness is the point and the risk: assertions true of both are the weakest ones,
+ * so this folder stays a smoke sweep rather than growing lifecycles.
+ * `conventions.test.ts` fails a spec here that reaches for a scenario.
+ *
+ * A glob, not a regex: Playwright matches a `testMatch` regex against the *absolute*
+ * path, so `/shared\//` also matches every spec in a checkout that happens to live
+ * under a directory of that name. A glob resolves against `testDir`, which is the
+ * thing actually meant here.
+ */
+const SHARED_SPECS = "shared/**/*.spec.ts";
+
 /**
  * The suite is the acceptance bar, so what it runs against cannot depend on the
  * shell it was started from: both servers are pinned to the in-browser mock
@@ -76,33 +91,71 @@ const LIVE = process.env.UI_LOOP_LIVE === "true";
  * server is about to want, or vice versa.
  */
 const LIVE_PORT = Number(process.env.UI_LOOP_LIVE_PORT ?? 8301);
-const LIVE_BASE_URL = `http://localhost:${LIVE_PORT}`;
+
+/**
+ * An app already running — in CI, the `kagent-ui` service — tested instead of one
+ * this config starts. That is the deployed image, so nginx, the SPA fallback and the
+ * `env-config.js` rendered at pod start are under test rather than approximated by
+ * Vite. A developer still gets the dev server by default; the alternative is building
+ * an image to run a test.
+ *
+ * Empty counts as absent, and the `||` is what makes that one question rather than
+ * three. Three things below ask it — what `baseURL` is, whether `globalSetup` checks
+ * for a deployed image, whether a dev server is started — and they had asked it three
+ * ways: a `??`, a `!== undefined` and a truthiness test. An exported-but-empty
+ * variable, which is what a `kubectl get svc` that found nothing leaves behind, then
+ * pointed the run at `""`, checked that for nginx, *and* started a dev server nothing
+ * would ever visit.
+ */
+const LIVE_EXTERNAL_URL = process.env.UI_LOOP_LIVE_URL || undefined;
+
+const LIVE_BASE_URL = LIVE_EXTERNAL_URL ?? `http://localhost:${LIVE_PORT}`;
 
 /** Read by `playwright/globalSetup.ts` to decide what to verify about a server. */
 export const LIVE_PROJECT = "chromium-live";
 
 /**
- * A live run reaches the backend through Vite's proxy, exactly as a deployed
- * build reaches it through nginx — so the app uses the same relative URLs either
- * way and this mode tests the addressing a real deployment uses.
- *
- * `VITE_API_MODE` is pinned as well as the runtime flag: the build-time pin is
- * the one thing an inherited `.env` cannot override, and a live suite that
- * silently answered from fixtures would be worse than a red one.
+ * Whether this live run is against a deployed app rather than a dev server, which
+ * `globalSetup` checks one more thing for — see `verifyLiveWiring`.
  */
-const LIVE_APP = { VITE_API_MODE: "live", ENABLE_MOCK_UI: "false" };
+export const LIVE_IS_DEPLOYED = LIVE_EXTERNAL_URL !== undefined;
 
 /**
- * How the live server is started.
- *
- * Named for the same reason the three env pins above are: a branch whose backend
- * needs more than a dev server — a credential minted per run, a port-forward
- * probed before Vite starts — replaces this line rather than the block below.
+ * How the *dev server* is configured for a live run; a deployment is configured by its
+ * chart instead. It proxies `/api` the way nginx does in a cluster, so the app uses the
+ * same relative URLs either way — standing in for nginx rather than being it, which is
+ * the gap `UI_LOOP_LIVE_URL` closes. `VITE_API_MODE` is pinned at build time as well as
+ * at runtime, being the one thing an inherited `.env` cannot override.
  */
+const LIVE_APP = { VITE_API_MODE: "live", ENABLE_MOCK_UI: "false" };
+
+/** How the live server is started. Unused when `UI_LOOP_LIVE_URL` names one already. */
 const LIVE_COMMAND = `yarn dev --port ${LIVE_PORT}`;
 
+/**
+ * The servers a live run starts, which is none when it was handed one: there is no
+ * process to own — the app is a pod. `globalSetup` checks the address serves the app.
+ */
+const LIVE_WEB_SERVERS = LIVE_EXTERNAL_URL
+  ? []
+  : [
+      {
+        command: LIVE_COMMAND,
+        url: LIVE_BASE_URL,
+        reuseExistingServer: false,
+        timeout: 120_000,
+        // Whatever starts the live server is the most useful output a failed
+        // live run has — something that cannot reach the backend says so there,
+        // and Playwright discards a web server's stdout unless asked to pass it
+        // through.
+        stdout: "pipe" as const,
+        stderr: "pipe" as const,
+        env: LIVE_APP,
+      },
+    ];
+
 export default defineConfig({
-  testDir: "./playwright/tests",
+  testDir: "./playwright",
   // Both servers have to be rendering, not merely listening, before any test
   // navigates — see the file for what goes wrong otherwise.
   globalSetup: "./playwright/globalSetup.ts",
@@ -130,7 +183,15 @@ export default defineConfig({
    * of the machine than the local runs that provoked it.
    */
   workers: process.env.CI ? "50%" : undefined,
-  reporter: process.env.CI ? "github" : "list",
+  /*
+   * Both in CI: `github` writes the annotations that put a failure on the diff, and it
+   * writes no files at all — so the report CI uploads as an artifact has to come from
+   * somewhere, and for a live failure the trace is the only account of what the cluster
+   * answered.
+   */
+  reporter: process.env.CI
+    ? [["github"], ["html", { open: "never" }]]
+    : [["list"]],
   /*
    * A real backend behind a port-forward answers in tens of seconds where the
    * in-browser mock answers in milliseconds, so the defaults that suit the mock
@@ -143,7 +204,20 @@ export default defineConfig({
    * and a mock-backed suite that needs more than thirty seconds for one test is saying
    * something is stuck, which is worth hearing rather than absorbing.
    */
-  ...(LIVE ? { timeout: 120_000, expect: { timeout: 30_000 } } : {}),
+  ...(LIVE
+    ? {
+        timeout: 120_000,
+        expect: { timeout: 30_000 },
+        /*
+         * One at a time. Every mock test owns a backend in its own page's memory; these
+         * share a cluster, so a spec creating a resource while another counts them is a
+         * failure with no defect behind it. A dozen tests, and one cluster to run them
+         * against — it costs little.
+         */
+        workers: 1,
+        fullyParallel: false,
+      }
+    : {}),
   use: {
     trace: "on-first-retry",
     screenshot: "only-on-failure",
@@ -153,7 +227,7 @@ export default defineConfig({
     ? [
         {
           name: LIVE_PROJECT,
-          testDir: "./playwright/live",
+          testMatch: ["live/**/*.spec.ts", SHARED_SPECS],
           use: {
             ...devices["Desktop Chrome"],
             baseURL: LIVE_BASE_URL,
@@ -167,6 +241,7 @@ export default defineConfig({
     : [
         {
           name: "chromium",
+          testMatch: ["tests/**/*.spec.ts", SHARED_SPECS],
           testIgnore: EXTENSION_SPECS,
           use: { ...devices["Desktop Chrome"], baseURL: BASE_URL },
         },
@@ -182,6 +257,7 @@ export default defineConfig({
           // The extension split below is a build-time difference, not a browser one,
           // so it stays on one engine rather than doubling for no new signal.
           name: "firefox",
+          testMatch: ["tests/**/*.spec.ts", SHARED_SPECS],
           testIgnore: EXTENSION_SPECS,
           use: { ...devices["Desktop Firefox"], baseURL: BASE_URL },
         },
@@ -200,21 +276,7 @@ export default defineConfig({
   // loud startup error instead; set UI_LOOP_PORT / UI_LOOP_EXTENSION_PORT to run
   // alongside a dev server you want to keep.
   webServer: LIVE
-    ? [
-        {
-          command: LIVE_COMMAND,
-          url: LIVE_BASE_URL,
-          reuseExistingServer: false,
-          timeout: 120_000,
-          // Whatever starts the live server is the most useful output a failed
-          // live run has — something that cannot reach the backend says so there,
-          // and Playwright discards a web server's stdout unless asked to pass it
-          // through.
-          stdout: "pipe",
-          stderr: "pipe",
-          env: LIVE_APP,
-        },
-      ]
+    ? LIVE_WEB_SERVERS
     : [
         {
           command: `yarn dev --port ${PORT}`,