fix(showcase-dashboard): tighten error semantics and trigger return type in ops hooks

R3-C.1: useTriggerProbe.trigger now returns Promise<TriggerResponse | null>.
The supersession path resolves to `null` (typed) instead of the previous
`undefined as unknown as TriggerResponse` cast, which lied about the
return shape and would crash any caller that read fields off the result.

R3-C.2: useProbeDetail clears `error` on id change so the detail panel no
longer renders a stale error message under a freshly-selected probe header.

R3-C.3: useProbes clears `error` on dep change (e.g. baseUrl swap) so a
prior dep tuple's failure does not persist under the new one.

R3-C.4: useTriggerProbe clears `error` at the start of each trigger call
so a successful retry leaves no sticky error from the prior failure.

Bonus: parseJson now attaches the original parse error as `cause` on the
wrapper so debuggers can walk back to the SyntaxError name/stack without
re-parsing the wrapped message.

Tests: red-green tests added for each behavior; full file suite 49/49.

Author: jpr5

showcase/shell-dashboard/src/hooks/use-probes.test.ts

@@ -181,6 +181,37 @@ describe("useProbes", () => {
     expect(arg.baseUrl).toBe("http://ops.test");
   });
 
+  it("clears stale error on baseUrl change (R3-C.3)", async () => {
+    // baseUrl A errors → error set. Switch to baseUrl B (success) → error
+    // must be cleared before B's fetch resolves so consumers don't render
+    // a stale error against a fresh dep tuple.
+    fetchProbesMock.mockRejectedValueOnce(new Error("a failed"));
+    let resolveB: ((v: ProbesResponse) => void) | null = null;
+    fetchProbesMock.mockImplementationOnce(
+      () =>
+        new Promise<ProbesResponse>((r) => {
+          resolveB = r;
+        }),
+    );
+
+    const { result, rerender } = renderHook(
+      ({ baseUrl }: { baseUrl: string }) => useProbes({ baseUrl }),
+      { initialProps: { baseUrl: "http://a.test" } },
+    );
+    await waitFor(() => expect(result.current.error?.message).toBe("a failed"));
+
+    // Swap baseUrl — error must clear before B resolves.
+    rerender({ baseUrl: "http://b.test" });
+    await waitFor(() => expect(result.current.error).toBeNull());
+
+    // Resolve B — still no error.
+    await act(async () => {
+      resolveB!(emptyProbes());
+      await Promise.resolve();
+    });
+    expect(result.current.error).toBeNull();
+  });
+
   it("does not setData with stale data when deps change rapidly (CR-B1.3)", async () => {
     // Drive a sequence where the first effect's fetch resolves AFTER the
     // dep change has triggered a new effect. With the old aliveRef pattern,
@@ -339,6 +370,42 @@ describe("useProbeDetail", () => {
     await waitFor(() => expect(result.current.data?.probe.id).toBe("deep"));
   });
 
+  it("clears stale error on id change (R3-C.2)", async () => {
+    // First id errors → error populated. Switch id → error must clear before
+    // the new fetch resolves so the panel never renders a stale error
+    // beneath a different probe header.
+    fetchProbeDetailMock.mockRejectedValueOnce(new Error("first failed"));
+    let resolveSecond:
+      | ((v: { probe: ProbeScheduleEntry; runs: ProbeRun[] }) => void)
+      | null = null;
+    fetchProbeDetailMock.mockImplementationOnce(
+      () =>
+        new Promise<{ probe: ProbeScheduleEntry; runs: ProbeRun[] }>((r) => {
+          resolveSecond = r;
+        }),
+    );
+
+    const { result, rerender } = renderHook(
+      ({ id }: { id: string | null }) => useProbeDetail(id),
+      { initialProps: { id: "smoke" as string | null } },
+    );
+    await waitFor(() =>
+      expect(result.current.error?.message).toBe("first failed"),
+    );
+
+    // Switch id — error must clear immediately (before the second fetch
+    // resolves).
+    rerender({ id: "deep" });
+    await waitFor(() => expect(result.current.error).toBeNull());
+
+    // Resolve second fetch — still no error.
+    await act(async () => {
+      resolveSecond!({ probe: entry("deep"), runs: [] });
+      await Promise.resolve();
+    });
+    expect(result.current.error).toBeNull();
+  });
+
   it("does not setData with stale data when id changes rapidly (CR-B1.3)", async () => {
     let resolveA:
       | ((v: { probe: ProbeScheduleEntry; runs: ProbeRun[] }) => void)
@@ -424,7 +491,7 @@ describe("useTriggerProbe", () => {
     );
     const { result } = renderHook(() => useTriggerProbe({ token: "t" }));
     expect(result.current.pending).toBe(false);
-    let p: Promise<TriggerResponse>;
+    let p: Promise<TriggerResponse | null>;
     act(() => {
       p = result.current.trigger("smoke");
     });
@@ -490,10 +557,10 @@ describe("useTriggerProbe", () => {
     const { result, unmount } = renderHook(() =>
       useTriggerProbe({ token: "t" }),
     );
-    let triggerPromise: Promise<TriggerResponse> | null = null;
+    let triggerPromise: Promise<TriggerResponse | null> | null = null;
     act(() => {
       triggerPromise = result.current.trigger("smoke");
-      triggerPromise.catch(() => {});
+      triggerPromise?.catch(() => {});
     });
     await waitFor(() => expect(triggerProbeMock).toHaveBeenCalled());
     expect(capturedSignal).toBeDefined();
@@ -550,7 +617,7 @@ describe("useTriggerProbe", () => {
       },
     );
     const { result } = renderHook(() => useTriggerProbe({ token: "t" }));
-    let firstPromise: Promise<TriggerResponse> | null = null;
+    let firstPromise: Promise<TriggerResponse | null> | null = null;
     act(() => {
       firstPromise = result.current.trigger("smoke");
       // Swallow potential rejection so unhandled-rejection guards don't
@@ -560,7 +627,7 @@ describe("useTriggerProbe", () => {
     await waitFor(() => expect(triggerProbeMock).toHaveBeenCalledTimes(1));
 
     // Fire second call — this aborts the first.
-    let secondPromise: Promise<TriggerResponse> | null = null;
+    let secondPromise: Promise<TriggerResponse | null> | null = null;
     act(() => {
       secondPromise = result.current.trigger("smoke");
     });
@@ -570,24 +637,89 @@ describe("useTriggerProbe", () => {
 
     // The first promise must NOT reject with AbortError surfaced — the
     // hook should swallow it. We resolve via the supersession path.
-    let firstResult: unknown = "pending";
+    let firstResult: TriggerResponse | null | undefined = undefined;
     let firstError: unknown = null;
-    firstPromise!
+    await firstPromise!
       .then((v) => {
         firstResult = v;
       })
       .catch((e) => {
         firstError = e;
       });
-    // Yield enough for any pending settlements.
-    await Promise.resolve();
-    await Promise.resolve();
-    // Per spec: first promise resolves silently (returns undefined) when
+    // Per spec: first promise resolves silently (returns null) when
     // superseded; AbortError must NOT be thrown to the caller.
     expect((firstError as { name?: string })?.name).not.toBe("AbortError");
-    void firstResult;
+    // R3-C.1: supersession resolves to null (typed), not undefined-cast.
+    expect(firstResult).toBeNull();
 
     // Error state must remain null.
     expect(result.current.error).toBeNull();
   });
+
+  it("supersession path resolves to null and is discriminable (R3-C.1)", async () => {
+    // Caller-facing contract: a superseded trigger resolves to `null`, never
+    // to a fake TriggerResponse. The type system reflects this so callers
+    // can `if (r === null)` to detect supersession.
+    triggerProbeMock.mockImplementationOnce(
+      (_id: string, opts: { signal?: AbortSignal }) =>
+        new Promise<TriggerResponse>((_resolve, reject) => {
+          opts.signal?.addEventListener("abort", () => {
+            reject(new DOMException("aborted", "AbortError"));
+          });
+        }),
+    );
+    triggerProbeMock.mockImplementationOnce(() =>
+      Promise.resolve(triggerOk()),
+    );
+    const { result } = renderHook(() => useTriggerProbe({ token: "t" }));
+
+    let firstPromise!: Promise<TriggerResponse | null>;
+    act(() => {
+      firstPromise = result.current.trigger("smoke");
+      firstPromise.catch(() => {});
+    });
+    await waitFor(() => expect(triggerProbeMock).toHaveBeenCalledTimes(1));
+
+    // Supersede with a second call.
+    let secondPromise!: Promise<TriggerResponse | null>;
+    act(() => {
+      secondPromise = result.current.trigger("smoke");
+    });
+    const secondValue = await act(async () => secondPromise);
+
+    // Second call returns a real response.
+    expect(secondValue).not.toBeNull();
+    expect(secondValue?.runId).toBe("run-1");
+
+    // First call resolves to null (the discriminator).
+    const firstValue = await firstPromise;
+    expect(firstValue).toBeNull();
+    // Discriminate via strict null check.
+    expect(firstValue === null).toBe(true);
+  });
+
+  it("clears error on next successful trigger (R3-C.4)", async () => {
+    // First call fails → error set. Second call succeeds → error cleared.
+    triggerProbeMock.mockRejectedValueOnce(new Error("forbidden"));
+    triggerProbeMock.mockResolvedValueOnce(triggerOk());
+
+    const { result } = renderHook(() => useTriggerProbe({ token: "t" }));
+
+    await act(async () => {
+      try {
+        await result.current.trigger("smoke");
+      } catch {
+        // expected
+      }
+    });
+    await waitFor(() =>
+      expect(result.current.error?.message).toBe("forbidden"),
+    );
+
+    await act(async () => {
+      await result.current.trigger("smoke");
+    });
+    // Error must be cleared by the successful trigger.
+    await waitFor(() => expect(result.current.error).toBeNull());
+  });
 });

showcase/shell-dashboard/src/hooks/use-probes.ts

@@ -92,6 +92,10 @@ export function useProbes(opts?: {
     // Surface a "refreshing" indicator across dep changes (baseUrl swap)
     // so consumers can render a loading state instead of stale data.
     setLoading(true);
+    // R3-C.3: clear any error from the prior dep tuple (e.g. previous
+    // baseUrl). Otherwise an error from a stale baseUrl persists under the
+    // new one until a fresh failure or successful fetch overwrites it.
+    setError(null);
     void run();
     const timer = setInterval(() => {
       void run();
@@ -154,6 +158,11 @@ export function useProbeDetail(
     // the panel header (driven by the prop) is never out of sync with the
     // data body. The new fetch will repopulate this on resolution.
     setData(null);
+    // R3-C.2: also clear any error from the prior probe so the detail panel
+    // doesn't render a stale error message under the new probe header. Set
+    // before flipping `loading` so the UI never shows error+loading at once
+    // for the new id.
+    setError(null);
     setLoading(true);
 
     async function run(): Promise<void> {
@@ -200,7 +209,18 @@ export function useProbeDetail(
 // ─────────────────────────────────────────────────────────────────────────
 
 export interface UseTriggerProbeResult {
-  trigger: (probeId: string, slugs?: string[]) => Promise<TriggerResponse>;
+  /**
+   * Trigger a probe run.
+   *
+   * Resolves to a `TriggerResponse` on success. Resolves to `null` when the
+   * call is superseded by a back-to-back trigger (the AbortError is swallowed
+   * — see R2-C.2). Callers that care about the response must null-check.
+   * Throws on real failures (missing token, server error, etc.).
+   */
+  trigger: (
+    probeId: string,
+    slugs?: string[],
+  ) => Promise<TriggerResponse | null>;
   pending: boolean;
   error: Error | null;
 }
@@ -235,7 +255,10 @@ export function useTriggerProbe(opts?: {
   }, []);
 
   const trigger = useCallback(
-    async (probeId: string, slugs?: string[]): Promise<TriggerResponse> => {
+    async (
+      probeId: string,
+      slugs?: string[],
+    ): Promise<TriggerResponse | null> => {
       if (!token) {
         const err = new Error(
           "useTriggerProbe: token is required to trigger a probe run",
@@ -245,6 +268,8 @@ export function useTriggerProbe(opts?: {
       }
       if (aliveRef.current) {
         setPending(true);
+        // R3-C.4: clear any prior trigger error eagerly so a successful
+        // call doesn't leave a stale error hanging on the result object.
         setError(null);
       }
       // Replace any prior in-flight controller — back-to-back triggers
@@ -268,10 +293,11 @@ export function useTriggerProbe(opts?: {
           (err as { name?: string })?.name === "AbortError" ||
           controller.signal.aborted;
         if (isAbort) {
-          // Caller's promise resolves with `undefined as TriggerResponse`.
-          // Real callers either await the new trigger or ignore the old
-          // promise; none should consume undefined as a TriggerResponse.
-          return undefined as unknown as TriggerResponse;
+          // R3-C.1: resolve with `null` (typed in the signature) so callers
+          // can discriminate supersession without a type-system lie. The
+          // previous `undefined as unknown as TriggerResponse` violated the
+          // declared contract.
+          return null;
         }
         const e = err instanceof Error ? err : new Error(String(err));
         if (aliveRef.current) setError(e);

showcase/shell-dashboard/src/lib/ops-api.ts

@@ -173,7 +173,12 @@ async function parseJson<T>(response: Response, url: string): Promise<T> {
       throw parseErr;
     }
     const msg = (parseErr as Error)?.message ?? "unknown";
-    throw new Error(`ops-api JSON parse failed at ${url}: ${msg}`);
+    // R3-C bonus: preserve the original error as `cause` so debuggers can
+    // walk back to the SyntaxError name/stack without re-parsing the
+    // wrapped message string.
+    throw new Error(`ops-api JSON parse failed at ${url}: ${msg}`, {
+      cause: parseErr,
+    });
   }
 }