fix(showcase): CR Round 1 — resolve precedence + probe robustness

tylerslaton · claude · tylerslaton · commit 80a998d59aea · 2026-05-07T19:47:22.000-07:00
Bucket (a) findings from CR Round 1, fixed inline:

A1. shell-dashboard resolveD5Row precedence
    Multi-key D5 cells (beautiful-chat → 5 per-pill keys) returned
    the first non-null row as worst, only upgrading on red. A degraded
    row encountered after a green row was silently dropped → cell
    rendered green when it should have been amber. Replaced the
    "only red wins" check with a numeric rank table (red=3, degraded=2,
    green=1) so red &gt; degraded &gt; green holds regardless of iteration
    order. Added 5 multi-key fan-out tests covering: red-after-green,
    red-before-green, degraded-vs-green order independence, red-beats-
    degraded, all-green-stays-green.

    Symbols touched: resolveD5Row (live-status.ts:176), new
    D5_STATE_RANK constant. Call-site enumeration: resolveD5Row is
    called only by resolveCell at live-status.ts:371 — same input/
    output shape, no caller change needed.

A2. fixture _comment lies about aimock arg shape
    Both gen-ui-agent.json and shared-state-streaming.json's _comment
    claimed `arguments` MUST be JSON-stringified or aimock silently
    drops the call. aimock's `normalizeResponse` (verified in
    node_modules/@copilotkit/aimock/dist/fixture-loader.cjs:14-19)
    auto-stringifies object-valued arguments at load time, so both
    shapes work. Updated the comments to reflect reality and stop
    misleading future fixture authors.

A3. e2e-deep per-feature timeout race orphaned runFeature
    When the synthetic timer won the Promise.race, runFeature was
    abandoned but never told to tear down. Browser context stayed
    held until the global timeout eventually fired, while the outer
    Semaphore.release ran immediately — a NEW feature could acquire
    the slot while the orphan still held the context, silently
    exceeding FEATURE_CONCURRENCY's pool budget. Now: a per-feature
    AbortController forwards the parent abort signal to runFeature;
    when the timer wins, .abort() fires so runFeature's finally chain
    tears down its page/context. The setTimeout cleanup is in a
    try/finally so a thrown rejection (defensive — runFeature's
    contract says no) doesn't leak the timer. The parent-abort event
    listener is removed on cleanup to prevent listener accumulation
    over many feature iterations.

    Symbols touched: per-feature loop body in executeE2eDeepDriver
    (e2e-deep.ts:982). runFeature signature unchanged.

A5. d5-chat-css user-bubble inner selector substring too loose
    `[class*="bg-muted"]` matches `bg-muted-foreground` too. Real
    Tailwind output puts `bg-muted-foreground` on nested children of
    the user bubble; the probe could read computed styles off the
    wrong element and silently mis-validate. Switched to
    `[class~="bg-muted"]` (whole-token match in space-separated
    class lists), the standard CSS3 way to express "this exact class
    is present on the element."

A7. auth legacy fill/press swallowed errors silently
    Legacy-shape assertion's catch block dropped fill/press errors
    so a chat-input cascade mismatch (or disabled textarea after
    sign-out) produced a generic "error surface did not appear"
    timeout instead of the real cause. Now captures the error
    message and appends it to the eventual error string so the
    failure record names what actually broke.

A9. d5-feature-mapping header listed removed `hitl-steps`
    The header's "destinations" list still showed `hitl-steps : 1
    demo` even though my PR's narrative says it was removed in
    genuine-pass Phase 0 — falsifying a claim my own diff makes.
    Updated the header to reflect the current REGISTRY_TO_D5 shape:
    `hitl-text-input` covers the 3 in-chat HITL variants (including
    the legacy `hitl` alias) and mcp-apps/subagents are split.

A6 reclassified to bucket (b) — the 6s waste on chip-driven probes is
sub-10% of the new 5-min per-feature timeout and not load-bearing for
convergence. Documented in the round summary; can be addressed in a
follow-up via a `noSend` ConversationTurn option.

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/showcase/harness/fixtures/d5/gen-ui-agent.json b/showcase/harness/fixtures/d5/gen-ui-agent.json
@@ -1,5 +1,5 @@
 {
-  "_comment": "D5 fixture for /demos/gen-ui-agent. Three pill prompts (product launch / team offsite / competitor research) each produce a `set_steps` tool call with three pill-specific steps, so the per-pill assertion can verify steps differ across pills. The fixture matches on a unique substring per pill. `arguments` MUST be a JSON-stringified string (not a raw object) — aimock follows the OpenAI tool-call wire shape, and a raw object silently drops the call so the frontend's useAgent never sees a state mutation.",
+  "_comment": "D5 fixture for /demos/gen-ui-agent. Three pill prompts (product launch / team offsite / competitor research) each produce a `set_steps` tool call with three pill-specific steps, so the per-pill assertion can verify steps differ across pills. The fixture matches on a unique substring per pill. `arguments` is shown JSON-stringified here for parity with the OpenAI wire shape; aimock's `normalizeResponse` (see node_modules/@copilotkit/aimock/dist/fixture-loader) auto-stringifies object-valued arguments at load time, so raw-object form would also work — pick one and stay consistent within a file.",
   "fixtures": [
     {
       "match": { "userMessage": "Plan a product launch" },
diff --git a/showcase/harness/fixtures/d5/shared-state-streaming.json b/showcase/harness/fixtures/d5/shared-state-streaming.json
@@ -1,5 +1,5 @@
 {
-  "_comment": "D5 fixture for /demos/shared-state-streaming. Three pill prompts produce `write_document` tool calls with non-trivial content payloads (≥ 100 chars) so the document streams into shared state and the DocumentView's char-count assertion can verify a substantive document was written. `arguments` MUST be a JSON-stringified string (not a raw object) — aimock follows the OpenAI tool-call wire shape; a raw object silently drops the call so write_document never executes and the document never streams.",
+  "_comment": "D5 fixture for /demos/shared-state-streaming. Three pill prompts produce `write_document` tool calls with non-trivial content payloads (≥ 100 chars) so the document streams into shared state and the DocumentView's char-count assertion can verify a substantive document was written. `arguments` is shown JSON-stringified here for parity with the OpenAI wire shape; aimock's `normalizeResponse` (see node_modules/@copilotkit/aimock/dist/fixture-loader) auto-stringifies object-valued arguments at load time, so raw-object form would also work.",
   "fixtures": [
     {
       "match": { "userMessage": "poem about autumn leaves" },
diff --git a/showcase/harness/src/probes/drivers/e2e-deep.ts b/showcase/harness/src/probes/drivers/e2e-deep.ts
@@ -986,31 +986,67 @@ export function createE2eDeepDriver(
             // global hit). The synthetic timeout result is shaped like
             // a normal runFeature failure so the rest of the loop is
             // unchanged.
+            //
+            // Cancellation: per-feature abort controller's signal is
+            // forwarded to runFeature. When the timer wins the race we
+            // call `.abort()` so runFeature's in-flight Playwright ops
+            // (and the page/context they hold) get torn down via the
+            // existing finally chain — instead of orphaning the browser
+            // context until the global timeout eventually fires. This
+            // matters because Semaphore.release() runs immediately
+            // after the race resolves, and a new feature acquiring the
+            // slot while an orphan still holds a context can silently
+            // exceed FEATURE_CONCURRENCY's pool budget.
+            //
+            // Timer cleanup is in a try/finally so a thrown rejection
+            // from runFeature (defensive — the contract says it
+            // doesn't throw) doesn't leak the unref'd setTimeout.
+            const featureAbort = new AbortController();
+            // Propagate the parent abort (global timeout / external
+            // abort) into the per-feature controller so runFeature
+            // sees both signals through one input.
+            const onParentAbort = (): void => featureAbort.abort();
+            if (abort.signal.aborted) featureAbort.abort();
+            else
+              abort.signal.addEventListener("abort", onParentAbort, {
+                once: true,
+              });
+
             let featureTimer: ReturnType<typeof setTimeout> | undefined;
-            const featureResult = await Promise.race([
-              runFeature({
-                browser: browserRef,
-                url,
-                pageTimeoutMs,
-                script,
-                buildCtx: {
-                  integrationSlug: slug,
-                  featureType: ft,
-                  baseUrl: backendUrl,
-                },
-                abortSignal: abort.signal,
-              }),
-              new Promise<Awaited<ReturnType<typeof runFeature>>>((resolve) => {
-                featureTimer = setTimeout(() => {
-                  resolve({
-                    ok: false,
-                    errorClass: "feature-timeout",
-                    errorDesc: `feature exceeded ${featureTimeoutMs}ms wall-clock`,
-                  });
-                }, featureTimeoutMs);
-              }),
-            ]);
-            if (featureTimer) clearTimeout(featureTimer);
+            let featureResult: Awaited<ReturnType<typeof runFeature>>;
+            try {
+              featureResult = await Promise.race([
+                runFeature({
+                  browser: browserRef,
+                  url,
+                  pageTimeoutMs,
+                  script,
+                  buildCtx: {
+                    integrationSlug: slug,
+                    featureType: ft,
+                    baseUrl: backendUrl,
+                  },
+                  abortSignal: featureAbort.signal,
+                }),
+                new Promise<Awaited<ReturnType<typeof runFeature>>>(
+                  (resolve) => {
+                    featureTimer = setTimeout(() => {
+                      // Tell the in-flight runFeature to tear down so
+                      // its browser context releases promptly.
+                      featureAbort.abort();
+                      resolve({
+                        ok: false,
+                        errorClass: "feature-timeout",
+                        errorDesc: `feature exceeded ${featureTimeoutMs}ms wall-clock`,
+                      });
+                    }, featureTimeoutMs);
+                  },
+                ),
+              ]);
+            } finally {
+              if (featureTimer) clearTimeout(featureTimer);
+              abort.signal.removeEventListener("abort", onParentAbort);
+            }
 
             if (featureResult.ok) {
               await sideEmit(ctx, {
diff --git a/showcase/harness/src/probes/helpers/d5-feature-mapping.ts b/showcase/harness/src/probes/helpers/d5-feature-mapping.ts
@@ -45,12 +45,19 @@ import type { D5FeatureType } from "./d5-registry.js";
  *     (headless chat surfaces — simple is text-only post-refactor;
  *     complete still drives the full gen-UI surface)
  *   - `gen-ui-custom`          : 1 demo
- *   - `hitl-text-input`        : 2 demos (in-chat HITL variants using useHumanInTheLoop)
- *   - `hitl-steps`             : 1 demo (step-selection confirmation)
+ *   - `hitl-text-input`        : 3 demos (the two in-chat HITL variants
+ *     using useHumanInTheLoop, plus the legacy `hitl` alias which was
+ *     repointed here after the standalone `hitl-steps` D5 script was
+ *     removed in genuine-pass Phase 0)
  *   - `hitl-approve-deny`      : 1 demo (modal/in-app approval)
  *   - `shared-state-read|write`: 1 demo, 2 D5 types (one-to-many)
- *   - `mcp-apps`               : 1 demo
- *   - `subagents`              : 1 demo
+ *   - `mcp-apps`               : 1 demo (own probe; was previously
+ *     bundled with subagents, split in Phase 2A)
+ *   - `subagents`              : 1 demo (split alongside mcp-apps)
+ *   - other registry families (auth, multimodal, voice, frontend-tools,
+ *     reasoning-display, gen-ui-*, byoc, beautiful-chat-*, …) follow
+ *     the same `<registry-id>: [<d5-feature-types>]` shape and live
+ *     directly in REGISTRY_TO_D5 below.
  */
 /**
  * Exported for the dashboard drift test (`d5-mapping-drift.test.ts`),
diff --git a/showcase/harness/src/probes/scripts/d5-auth.ts b/showcase/harness/src/probes/scripts/d5-auth.ts
@@ -214,6 +214,14 @@ export function buildAuthAssertion(
       );
     }
     await new Promise<void>((r) => setTimeout(r, 500));
+    // Try to push a probe message through; if fill or press fails
+    // (textarea not found, selector cascade mismatch, disabled control),
+    // capture the error so the eventual "no error surface" failure
+    // names the real cause instead of a generic timeout. The error
+    // surface MAY already be visible from the sign-out click alone —
+    // the polling loop below handles that case — so we don't fail
+    // hard on fill/press; we just preserve the diagnostic.
+    let probeSendError: string | null = null;
     try {
       await page.fill(
         '[data-testid="copilot-chat-textarea"]',
@@ -223,16 +231,19 @@ export function buildAuthAssertion(
       await page.press('[data-testid="copilot-chat-textarea"]', "Enter", {
         timeout: 2_000,
       });
-    } catch {
-      // Fall through — error surface may already be visible.
+    } catch (err) {
+      probeSendError = err instanceof Error ? err.message : String(err);
     }
     const errorDeadline = Date.now() + remaining;
     while (Date.now() < errorDeadline) {
       if (await probeErrorSurfaceVisible(page)) return;
       await new Promise<void>((r) => setTimeout(r, POLL_INTERVAL_MS));
     }
+    const sendNote = probeSendError
+      ? ` — probe send failed: ${probeSendError.slice(0, 140)}`
+      : "";
     throw new Error(
-      `auth: legacy shape — banner flipped to unauthenticated but neither ${ERROR_BANNER_SELECTOR} nor ${ERROR_BOUNDARY_SELECTOR} appeared within ${remaining}ms after probe send — auth gate may have regressed`,
+      `auth: legacy shape — banner flipped to unauthenticated but neither ${ERROR_BANNER_SELECTOR} nor ${ERROR_BOUNDARY_SELECTOR} appeared within ${remaining}ms after probe send — auth gate may have regressed${sendNote}`,
     );
   };
 }
diff --git a/showcase/harness/src/probes/scripts/d5-chat-css.ts b/showcase/harness/src/probes/scripts/d5-chat-css.ts
@@ -42,8 +42,14 @@ export const ASSISTANT_BUBBLE_SELECTOR =
   ".copilotKitMessage.copilotKitAssistantMessage";
 /** The user bubble's inner v2 element — paints the halcyon-paper-elevated
  *  background plus the ember left border. The outer message wrapper is
- *  transparent in the new theme; signals all live on this inner node. */
-export const USER_BUBBLE_INNER_SELECTOR = `${USER_BUBBLE_SELECTOR} [class*="bg-muted"]`;
+ *  transparent in the new theme; signals all live on this inner node.
+ *
+ *  Uses `[class~="bg-muted"]` (whole-token match in space-separated
+ *  class lists) rather than `[class*="bg-muted"]` (substring match):
+ *  the latter accidentally also matches `bg-muted-foreground`, which
+ *  appears on nested children in real Tailwind output and would cause
+ *  the probe to read computed styles off the wrong element. */
+export const USER_BUBBLE_INNER_SELECTOR = `${USER_BUBBLE_SELECTOR} [class~="bg-muted"]`;
 
 // ── HALCYON theme anchors (langgraph-python) ──────────────────────────
 /** halcyon-ember rgb anchor (`#c44a1f`) on the user-bubble inner border-left. */
@@ -122,7 +128,7 @@ export async function probeChatCss(page: Page): Promise<ChatCssProbeResult> {
     // includes `bg-muted` on the bubble div.
     const userInner = (
       userOuter as { querySelector?(sel: string): unknown } | null
-    )?.querySelector?.('[class*="bg-muted"]');
+    )?.querySelector?.('[class~="bg-muted"]');
     const assistantEl = win.document.querySelector(
       ".copilotKitMessage.copilotKitAssistantMessage",
     );
diff --git a/showcase/shell-dashboard/src/lib/live-status.test.ts b/showcase/shell-dashboard/src/lib/live-status.test.ts
@@ -364,6 +364,71 @@ describe("resolveCell — post-Phase 3 (rollup uses health + e2e only)", () => {
     expect(c.d5.row).toBeNull();
     expect(c.d6.row).toBeNull();
   });
+
+  // ── multi-key D5 fan-out (e.g. beautiful-chat → 5 per-pill keys) ──
+  // The CATALOG_TO_D5_KEY mapping fans some catalog feature IDs to
+  // multiple D5 keys (beautiful-chat → 5 per-pill literals). The
+  // rolled-up cell must reflect the WORST-state row in the family —
+  // red > degraded > green — so a single amber pill turns the badge
+  // amber instead of staying green behind a co-iterated green sibling.
+  it("d5 multi-key fan-out: red beats green when red comes after green", () => {
+    const live = mapOf([
+      row("d5:agno/beautiful-chat-toggle-theme", "d5", "green"),
+      row("d5:agno/beautiful-chat-pie-chart", "d5", "red"),
+    ]);
+    const c = resolveCell(live, "agno", "beautiful-chat");
+    expect(c.d5.tone).toBe("red");
+    expect(c.d5.label).toBe("✗");
+  });
+
+  it("d5 multi-key fan-out: red beats green when red comes BEFORE green", () => {
+    const live = mapOf([
+      row("d5:agno/beautiful-chat-toggle-theme", "d5", "red"),
+      row("d5:agno/beautiful-chat-pie-chart", "d5", "green"),
+    ]);
+    const c = resolveCell(live, "agno", "beautiful-chat");
+    expect(c.d5.tone).toBe("red");
+  });
+
+  it("d5 multi-key fan-out: degraded beats green regardless of iteration order", () => {
+    // Pre-fix regression: only `red` could replace `worst`, so a degraded
+    // row encountered after a green row was silently dropped and the
+    // badge stayed green. With the fix, degraded > green wins.
+    const liveGreenFirst = mapOf([
+      row("d5:agno/beautiful-chat-toggle-theme", "d5", "green"),
+      row("d5:agno/beautiful-chat-pie-chart", "d5", "degraded"),
+    ]);
+    expect(resolveCell(liveGreenFirst, "agno", "beautiful-chat").d5.tone).toBe(
+      "amber",
+    );
+
+    const liveDegradedFirst = mapOf([
+      row("d5:agno/beautiful-chat-toggle-theme", "d5", "degraded"),
+      row("d5:agno/beautiful-chat-pie-chart", "d5", "green"),
+    ]);
+    expect(
+      resolveCell(liveDegradedFirst, "agno", "beautiful-chat").d5.tone,
+    ).toBe("amber");
+  });
+
+  it("d5 multi-key fan-out: red beats degraded", () => {
+    const live = mapOf([
+      row("d5:agno/beautiful-chat-toggle-theme", "d5", "degraded"),
+      row("d5:agno/beautiful-chat-pie-chart", "d5", "red"),
+    ]);
+    expect(resolveCell(live, "agno", "beautiful-chat").d5.tone).toBe("red");
+  });
+
+  it("d5 multi-key fan-out: all green stays green", () => {
+    const live = mapOf([
+      row("d5:agno/beautiful-chat-toggle-theme", "d5", "green"),
+      row("d5:agno/beautiful-chat-pie-chart", "d5", "green"),
+      row("d5:agno/beautiful-chat-bar-chart", "d5", "green"),
+      row("d5:agno/beautiful-chat-search-flights", "d5", "green"),
+      row("d5:agno/beautiful-chat-schedule-meeting", "d5", "green"),
+    ]);
+    expect(resolveCell(live, "agno", "beautiful-chat").d5.tone).toBe("green");
+  });
 });
 
 describe("formatTooltip behaviour (via resolveCell)", () => {
diff --git a/showcase/shell-dashboard/src/lib/live-status.ts b/showcase/shell-dashboard/src/lib/live-status.ts
@@ -173,6 +173,29 @@ export const CATALOG_TO_D5_KEY: Readonly<Record<string, readonly string[]>> = {
   voice: ["voice"],
 };
 
+/**
+ * Resolve the rolled-up D5 row for `(slug, featureId)`.
+ *
+ * Precedence across the multi-key set (red > degraded > green) — the
+ * cell's badge tone reflects the worst-state row in the family. A
+ * naive "first non-null wins" or "only red wins" implementation
+ * silently masks degraded sub-rows behind green ones; for a cell like
+ * `beautiful-chat` which fans out to 5 per-pill keys, that means an
+ * amber sub-row would render the cell green and operators would never
+ * see the partial regression.
+ *
+ * Missing rows are treated as not-yet-emitted and are NOT a signal of
+ * health — but they also can't be "worst" because we can't compare
+ * them to anything. The caller (`resolveCell`) renders gray when no
+ * row is returned, which surfaces "no data yet" distinctly from
+ * green.
+ */
+const D5_STATE_RANK: Readonly<Record<State, number>> = {
+  red: 3,
+  degraded: 2,
+  green: 1,
+};
+
 function resolveD5Row(
   live: LiveStatusMap,
   slug: string,
@@ -186,7 +209,9 @@ function resolveD5Row(
   for (const d5Key of d5Keys) {
     const row = live.get(keyFor("d5", slug, d5Key)) ?? null;
     if (!row) continue;
-    if (!worst || row.state === "red") worst = row;
+    if (!worst || D5_STATE_RANK[row.state] > D5_STATE_RANK[worst.state]) {
+      worst = row;
+    }
   }
   return worst;
 }

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`{`
`2`		- "_comment": "D5 fixture for /demos/gen-ui-agent. Three pill prompts (product launch / team offsite / competitor research) each produce a `set_steps` tool call with three pill-specific steps, so the per-pill assertion can verify steps differ across pills. The fixture matches on a unique substring per pill. `arguments` MUST be a JSON-stringified string (not a raw object) — aimock follows the OpenAI tool-call wire shape, and a raw object silently drops the call so the frontend's useAgent never sees a state mutation.",
	`2`	+ "_comment": "D5 fixture for /demos/gen-ui-agent. Three pill prompts (product launch / team offsite / competitor research) each produce a `set_steps` tool call with three pill-specific steps, so the per-pill assertion can verify steps differ across pills. The fixture matches on a unique substring per pill. `arguments` is shown JSON-stringified here for parity with the OpenAI wire shape; aimock's `normalizeResponse` (see node_modules/@copilotkit/aimock/dist/fixture-loader) auto-stringifies object-valued arguments at load time, so raw-object form would also work — pick one and stay consistent within a file.",
`3`	`3`	`"fixtures": [`
`4`	`4`	`{`
`5`	`5`	`"match": { "userMessage": "Plan a product launch" },`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`{`
`2`		- "_comment": "D5 fixture for /demos/shared-state-streaming. Three pill prompts produce `write_document` tool calls with non-trivial content payloads (≥ 100 chars) so the document streams into shared state and the DocumentView's char-count assertion can verify a substantive document was written. `arguments` MUST be a JSON-stringified string (not a raw object) — aimock follows the OpenAI tool-call wire shape; a raw object silently drops the call so write_document never executes and the document never streams.",
	`2`	+ "_comment": "D5 fixture for /demos/shared-state-streaming. Three pill prompts produce `write_document` tool calls with non-trivial content payloads (≥ 100 chars) so the document streams into shared state and the DocumentView's char-count assertion can verify a substantive document was written. `arguments` is shown JSON-stringified here for parity with the OpenAI wire shape; aimock's `normalizeResponse` (see node_modules/@copilotkit/aimock/dist/fixture-loader) auto-stringifies object-valued arguments at load time, so raw-object form would also work.",
`3`	`3`	`"fixtures": [`
`4`	`4`	`{`
`5`	`5`	`"match": { "userMessage": "poem about autumn leaves" },`