test(ui): fix three flaky browser-suite waits (#2612)

---
*🤖 written by Claude (start)*

## Overview

- **#2609** — the collapsed nav icon is measured against its own row
rather than the rail's centre line, so the assertion no longer reports
which engine drew the frame.
- **#2610** — the chat fixture now says when a turn has finished, so the
specs wait on that instead of on a 5s deadline over a ~3s scripted
stream.
- **#2611** — the step that deliberately does not wait chooses its
namespace from the keyboard rather than by clicking a popup that is
hit-tested where it is drawn, and the sibling test reads the address
with `toHaveURL` instead of a bare `page.url()` that had no retry at
all.
- **Cluster setup** — `make create-kind-cluster` no longer races the
apiserver it just created. `kind create cluster` waits for the control
plane, and the MetalLB step waits on `/readyz` before its first apply —
which had been failing with `net/http: TLS handshake timeout`.

No timeouts were raised and no retries added; everything runs on
Playwright's defaults, and the two writes #2611 exists for still land
with nothing waited between them.

## Changelog

Three flaky browser-suite waits now watch the signal they were standing
in for.

Closes #2609
Closes #2610
Closes #2611

## Testing

1. `cd ui && yarn test:pw`.
2. Drop the `& .ant-menu-inline-collapsed .ant-menu-item` block from
`ui/src/components/Structure/AppSidebar.tsx` and rerun `yarn playwright
test shell-chrome` — it fails naming the row and both gaps.

## Release note

```
NONE
```

---
*🤖 written by Claude (end)*

---------

Signed-off-by: Nicholas Bucher <behappy54321@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: Charlesthebird

scripts/kind/setup-kind.sh

@@ -30,7 +30,8 @@ else
   export KIND_EXPERIMENTAL_PROVIDER="${CONTAINER_RUNTIME}"
   kind create cluster --name "${KIND_CLUSTER_NAME}" \
     --config scripts/kind/kind-config.yaml \
-    --image="kindest/node:v${KIND_IMAGE_VERSION}"
+    --image="kindest/node:v${KIND_IMAGE_VERSION}" \
+    --wait 60s
 fi
 
 # 3. Add the registry config to the nodes

scripts/kind/setup-metallb.sh

@@ -7,6 +7,16 @@ set -o nounset
 METALLB_VERSION=${METALLB_VERSION:-v0.15.3}
 KIND_CLUSTER_NAME=${KIND_CLUSTER_NAME:-kagent}
 
+# A just-created cluster can answer one request and stall on the next.
+for attempt in $(seq 1 30); do
+  kubectl --context "kind-${KIND_CLUSTER_NAME}" get --raw=/readyz >/dev/null 2>&1 && break
+  if [ "${attempt}" -eq 30 ]; then
+    echo "ERROR: apiserver for kind-${KIND_CLUSTER_NAME} was still not ready after 60s."
+    exit 1
+  fi
+  sleep 2
+done
+
 kubectl --context "kind-${KIND_CLUSTER_NAME}" apply -f https://raw.githubusercontent.com/metallb/metallb/${METALLB_VERSION}/config/manifests/metallb-native.yaml
 
 # Wait for MetalLB to become available.

ui/playwright/helpers/chat.ts

@@ -0,0 +1,85 @@
+/**
+ * Driving a turn, and knowing when it is over.
+ *
+ * `expect(chat-cancel).toHaveCount(0)` is a wait wearing an assertion's clothes, and
+ * its report reads "cancel should not be here" when the reply had simply not finished
+ * (#2610). `MockChatClient` publishes a tally of turns started and finished — as
+ * `src/mocks/transport.ts` does for RPC calls — so the wait can be on the event, and
+ * the assertion after it on the page, at the default budget.
+ */
+
+import { expect, type Page } from "@playwright/test";
+
+/** Where `src/api/chat/mockChatClient.ts` publishes what it is doing. */
+const PROPERTY = "__kagentMockChat";
+
+/** The tally, as the page holds it. */
+interface Turns {
+  started: number;
+  finished: number;
+}
+
+/**
+ * A sent turn, identified by the count of turns finished before it went — which is
+ * what keeps the wait below about this turn and not the previous one.
+ */
+export interface SentTurn {
+  readonly finishedBefore: number;
+}
+
+function readTurns(page: Page): Promise<Turns | null> {
+  return page.evaluate(
+    (property) =>
+      (window as unknown as Record<string, Turns | undefined>)[property] ?? null,
+    PROPERTY,
+  );
+}
+
+/**
+ * The token for the turn about to be sent. Polled because the counters are hung off
+ * the chat client, which is built the first time a conversation is read.
+ */
+export async function beginTurn(page: Page): Promise<SentTurn> {
+  await expect
+    .poll(async () => await readTurns(page), {
+      message:
+        `The chat fixture published nothing on window.${PROPERTY}. Either the app is ` +
+        `not running in mock mode, or this page never opened a conversation.`,
+    })
+    .not.toBeNull();
+
+  const turns = await readTurns(page);
+  return { finishedBefore: turns!.finished };
+}
+
+/** Types a message and sends it, returning the token its turn is waited on with. */
+export async function sendMessage(page: Page, text: string): Promise<SentTurn> {
+  const turn = await beginTurn(page);
+  await page.getByTestId("chat-input").fill(text);
+  await page.getByTestId("chat-send").click();
+  return turn;
+}
+
+/**
+ * Waits for the turn to stop streaming, then asserts the page has noticed. Both
+ * halves matter: the first alone races the script, the second alone proves nothing.
+ */
+export async function expectTurnFinished(page: Page, turn: SentTurn): Promise<void> {
+  await page.waitForFunction(
+    ({ property, after }) => {
+      const turns = (window as unknown as Record<string, Turns | undefined>)[property];
+      return turns !== undefined && turns.finished > after;
+    },
+    { property: PROPERTY, after: turn.finishedBefore },
+  );
+
+  await expect(
+    page.getByTestId("chat-cancel"),
+    "the composer should come back once the turn is over",
+  ).toHaveCount(0);
+}
+
+/** Sends a message and waits out the turn it starts, asserting nothing in between. */
+export async function sendAndAwaitTurn(page: Page, text: string): Promise<void> {
+  await expectTurnFinished(page, await sendMessage(page, text));
+}

ui/playwright/tests/chat/chat-errors.spec.ts

@@ -1,4 +1,5 @@
 import { test, expect } from "../../fixtures/test";
+import { sendAndAwaitTurn } from "../../helpers/chat";
 import {
   agentChat,
   agentNewChat,
@@ -238,9 +239,7 @@ test("chat: a question the agent is waiting on is said, and can be given up", as
     // The proof that the turn really closed is a *new* turn running, not an alert
     // that disappeared: a task still parked would refuse this.
     await page.goto(`${AGENT_CHAT}?chat=ok`);
-    await page.getByTestId("chat-input").fill("How many pods are running?");
-    await page.getByTestId("chat-send").click();
-    await expect(page.getByTestId("chat-cancel")).toHaveCount(0, { timeout: 30_000 });
+    await sendAndAwaitTurn(page, "How many pods are running?");
     await expect(page.getByTestId("chat-turn-error")).toHaveCount(0);
   });
 });
@@ -273,9 +272,7 @@ test("chat: a question can be discarded instead of answered", async ({ page }) =
 
   await test.step("3. and an unrelated message is accepted again", async () => {
     await page.goto(`${AGENT_CHAT}?chat=ok`);
-    await page.getByTestId("chat-input").fill("How many pods are running?");
-    await page.getByTestId("chat-send").click();
-    await expect(page.getByTestId("chat-cancel")).toHaveCount(0, { timeout: 30_000 });
+    await sendAndAwaitTurn(page, "How many pods are running?");
     await expect(page.getByTestId("chat-turn-error")).toHaveCount(0);
   });
 });

ui/playwright/tests/chat/chat.spec.ts

@@ -1,5 +1,12 @@
 import { test, expect } from "../../fixtures/test";
 import { SIBLING_OF_READY, agentChat, instances, loadPage } from "../../helpers/app";
+import {
+  beginTurn,
+  expectTurnFinished,
+  sendAndAwaitTurn,
+  sendMessage,
+  type SentTurn,
+} from "../../helpers/chat";
 
 /**
  * Chat — the conversation journey.
@@ -63,6 +70,8 @@ test("chat: history, sending, streaming, and tool rendering", async ({ page }) =
   // Scoped by role: the agent quotes the question back in its reply, so a plain
   // text filter matches the answer as well as the question that prompted it.
   const userMessages = page.locator('[data-testid="chat-message"][data-role="user"]');
+  // Held across steps because a turn is sent in one and finishes in another.
+  let turn: SentTurn;
 
   await test.step("1. the page opens on the conversation, ready to be typed into", async () => {
     await loadPage(page, AGENT_CHAT);
@@ -123,8 +132,7 @@ test("chat: history, sending, streaming, and tool rendering", async ({ page }) =
   });
 
   await test.step("7. sending a message adds it to the transcript immediately", async () => {
-    await page.getByTestId("chat-input").fill(FIRST_QUESTION);
-    await page.getByTestId("chat-send").click();
+    turn = await sendMessage(page, FIRST_QUESTION);
 
     // The reader's own words, before the server has said anything. The fixture
     // does not echo them back and neither does the gateway, so this can only pass
@@ -159,8 +167,8 @@ test("chat: history, sending, streaming, and tool rendering", async ({ page }) =
   });
 
   await test.step("11. the turn finishes, the composer comes back, and the indicator settles", async () => {
+    await expectTurnFinished(page, turn);
     await expect(page.getByTestId("chat-send")).toBeVisible();
-    await expect(page.getByTestId("chat-cancel")).toHaveCount(0);
     // Nothing reported once the turn is over: the status line belongs to a turn in
     // flight, so a finished one leaves it with nothing to say.
     await expect(page.getByTestId("chat-status")).toHaveCount(0);
@@ -169,15 +177,14 @@ test("chat: history, sending, streaming, and tool rendering", async ({ page }) =
   await test.step("12. a second question behaves exactly like the first", async () => {
     // The report was that further questions behaved the same way — only the agent's
     // replies appeared. So the second turn is driven, not assumed from the first.
-    await page.getByTestId("chat-input").fill(SECOND_QUESTION);
-    await page.getByTestId("chat-send").click();
+    turn = await sendMessage(page, SECOND_QUESTION);
 
     await expect(
       userMessages.filter({ hasText: SECOND_QUESTION }),
       "the second question should be on screen as soon as it is sent, like the first",
     ).toHaveCount(1);
 
-    await expect(page.getByTestId("chat-cancel")).toHaveCount(0);
+    await expectTurnFinished(page, turn);
     await expect(
       messages.last().getByTestId("chat-message-text"),
       "the second reply should assemble exactly once",
@@ -230,9 +237,7 @@ test("chat: history, sending, streaming, and tool rendering", async ({ page }) =
      * not something a glance at a still will catch.
      */
     await page.setViewportSize({ width: 1440, height: 420 });
-    await page.getByTestId("chat-input").fill("one more, to make the page scroll");
-    await page.getByTestId("chat-send").click();
-    await expect(page.getByTestId("chat-cancel")).toHaveCount(0);
+    await sendAndAwaitTurn(page, "one more, to make the page scroll");
 
     /*
      * The box that scrolls must end above the box you type in.
@@ -388,6 +393,8 @@ test("chat: an agent's Markdown renders as elements, not as characters", async (
   // the render, and the assertion below is about the render.
   await expect(page.getByTestId("chat-message")).toHaveCount(4);
 
+  // Taken before the message is sent, so the wait below is about this turn.
+  const turn = await beginTurn(page);
   await page.getByTestId("chat-input").fill("How many pods are running?");
   await expect(page.getByTestId("chat-send")).toBeEnabled();
   await page.getByTestId("chat-send").click();
@@ -396,7 +403,7 @@ test("chat: an agent's Markdown renders as elements, not as characters", async (
   // transcript before the reply exists, and without the second it can read a half-streamed
   // list whose item count is whatever had arrived.
   await expect(page.getByTestId("chat-cancel")).toBeVisible();
-  await expect(page.getByTestId("chat-cancel")).toHaveCount(0);
+  await expectTurnFinished(page, turn);
 
   const answer = page.getByTestId("chat-message").last().getByTestId("chat-message-text");
 

ui/playwright/tests/lists/list-filters.spec.ts

@@ -34,11 +34,31 @@ import { dataRows, expectSettled, loadPage, rowNamed, routes } from "../../helpe
 /** Opens a filter's popup and ticks one option by the label the reader sees. */
 async function chooseFilter(page: Page, filterTestId: string, label: string) {
   await page.getByTestId(filterTestId).click();
-  await page.locator(`.ant-select-item-option[title="${label}"]`).click();
+  const option = page.locator(`.ant-select-item-option[title="${label}"]`);
+  await option.click();
+  // The click, confirmed where it happened: a click on a popup that has moved or
+  // closed under it selects nothing, silently, and is reported much later as a pill
+  // that never appeared.
+  await expect(option).toHaveAttribute("aria-selected", "true");
   // Otherwise the popup covers the pill row the next step asserts on.
   await page.keyboard.press("Escape");
 }
 
+/**
+ * The same choice, made from the keyboard.
+ *
+ * For the step that deliberately does not wait: a popup is hit-tested where it is
+ * drawn, so clicking one while another write is re-rendering the page underneath is a
+ * race of the test's own making, on top of the race it is trying to observe. Typing
+ * and pressing Enter goes to the control's own input, which does not move.
+ */
+async function typeFilter(page: Page, filterTestId: string, label: string) {
+  const input = page.getByTestId(filterTestId).locator("input");
+  await input.fill(label);
+  await input.press("Enter");
+  await input.press("Escape");
+}
+
 test("lists: a page's filter narrows that page's own rows", async ({ page }) => {
   /*
    * What the browser is needed to say, and nothing more.
@@ -82,8 +102,19 @@ test("lists: a page's filter narrows that page's own rows", async ({ page }) =>
      */
     await loadPage(page, routes.models, { title: "Models" });
     await expectSettled(page);
+
     await page.getByTestId("models-filters-search").fill("model");
-    await chooseFilter(page, "models-filters-filter-ns", "kagent");
+    await typeFilter(page, "models-filters-filter-ns", "kagent");
+
+    // Read from the address first: not a longer wait, a more specific one. Stopping
+    // here names the write that went missing, where a pill that never appeared could
+    // be that or a page that failed to draw what it had.
+    await expect(page, "the search term should survive the filter write").toHaveURL(
+      /[?&]q=model(&|$)/,
+    );
+    await expect(page, "the filter should survive the search write").toHaveURL(
+      /[?&]ns=kagent(&|$)/,
+    );
 
     await expect(page.getByTestId("models-filters-pill-search")).toBeVisible();
     await expect(page.getByTestId("models-filters-pill-ns-kagent")).toBeVisible();
@@ -97,11 +128,12 @@ test("lists: a narrowed view is an address, so it survives a reload", async ({ p
     await expectSettled(page);
 
     await page.getByTestId("models-filters-search").fill("config");
-    await chooseFilter(page, "models-filters-filter-ns", "kagent");
+    await typeFilter(page, "models-filters-filter-ns", "kagent");
 
-    const url = new URL(page.url());
-    expect(url.searchParams.get("q")).toBe("config");
-    expect(url.searchParams.getAll("ns")).toEqual(["kagent"]);
+    // `page.url()` was a bare read with no retry, so it reported the address as it
+    // was a tick before the second write reached it.
+    await expect(page).toHaveURL(/[?&]q=config(&|$)/);
+    await expect(page).toHaveURL(/[?&]ns=kagent(&|$)/);
   });
 
   await test.step("2. sorting is in the address too, and the header shows it", async () => {