perf(webapp): resolve the agent card's links in one request

Author: kathiekiwi

apps/webapp/app/components/dashboard-agent/resolve-uris.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, it } from "vitest";
+import { MAX_URIS_PER_RESOLVE_REQUEST, planUriBatches } from "./resolve-uris";
+
+const uri = (index: number) => `trigger://runs/run_${index}`;
+
+describe("planUriBatches", () => {
+  it("resolves a card's twenty citations in one request", () => {
+    const batches = planUriBatches(Array.from({ length: 20 }, (_, index) => uri(index)));
+
+    expect(batches).toHaveLength(1);
+    expect(batches[0]).toHaveLength(20);
+  });
+
+  it("asks about each URI once", () => {
+    const batches = planUriBatches([uri(1), uri(1), uri(2)]);
+
+    expect(batches).toEqual([[uri(1), uri(2)]]);
+  });
+
+  it("caps a request and carries the rest over", () => {
+    const count = MAX_URIS_PER_RESOLVE_REQUEST + 3;
+    const batches = planUriBatches(Array.from({ length: count }, (_, index) => uri(index)));
+
+    expect(batches).toHaveLength(2);
+    expect(batches[0]).toHaveLength(MAX_URIS_PER_RESOLVE_REQUEST);
+    expect(batches[1]).toHaveLength(3);
+  });
+
+  it("has nothing to send for nothing", () => {
+    expect(planUriBatches([])).toEqual([]);
+  });
+});

apps/webapp/app/components/dashboard-agent/resolve-uris.ts

@@ -0,0 +1,25 @@
+/**
+ * Batching for `trigger://` resolution. An investigation card cites ten to twenty targets, and
+ * each request re-authorises and re-resolves the environment — so they go in one request.
+ */
+
+/** One environment lookup and one repo lookup serve a whole batch. */
+export const MAX_URIS_PER_RESOLVE_REQUEST = 25;
+
+/** A transient failure is worth retrying; a third one isn't. */
+export const MAX_RESOLVE_ATTEMPTS = 3;
+
+export const RESOLVE_RETRY_DELAY_MS = 1_000;
+
+/** Deduplicates, then splits into requests no bigger than the cap. */
+export function planUriBatches(
+  uris: readonly string[],
+  cap: number = MAX_URIS_PER_RESOLVE_REQUEST
+): string[][] {
+  const unique = [...new Set(uris)];
+  const batches: string[][] = [];
+  for (let index = 0; index < unique.length; index += cap) {
+    batches.push(unique.slice(index, index + cap));
+  }
+  return batches;
+}

apps/webapp/app/components/dashboard-agent/useTriggerUriResolver.ts

@@ -1,14 +1,21 @@
 import { isTriggerUri } from "@internal/dashboard-agent-contracts";
 import { useCallback, useEffect, useRef, useState } from "react";
 import type { ResolvedUri } from "./ReportView";
+import { MAX_RESOLVE_ATTEMPTS, planUriBatches, RESOLVE_RETRY_DELAY_MS } from "./resolve-uris";
 
-// Synchronous facade over the panel's async `resolve` action: the first render of a
-// URI returns null. Failures cache as null, so a URI is asked about exactly once.
+/**
+ * Synchronous facade over the panel's async `resolve-many` action: the first render of a URI
+ * returns null. A "nothing to open" answer is cached, a transient failure is retried a few times.
+ */
 export function useTriggerUriResolver(actionPath: string): (uri: string) => ResolvedUri | null {
   const [resolved, setResolved] = useState<Record<string, ResolvedUri | null>>({});
+  // Mirrors `resolved` for the flush, which runs after a delay and must not read a stale closure.
+  const answered = useRef<Record<string, ResolvedUri | null>>({});
   // URIs the cards asked about this render; a ref because it's written during render.
   const seen = useRef(new Set<string>());
-  const requested = useRef(new Set<string>());
+  const inFlight = useRef(new Set<string>());
+  const attempts = useRef(new Map<string, number>());
+  const retryTimer = useRef<number | undefined>(undefined);
 
   const resolveUri = useCallback(
     (uri: string): ResolvedUri | null => {
@@ -19,28 +26,78 @@ export function useTriggerUriResolver(actionPath: string): (uri: string) => Reso
     [resolved]
   );
 
-  // No dependency array on purpose: after every render, fetch whatever the
-  // cards asked about that hasn't been requested yet.
-  useEffect(() => {
-    const toFetch = [...seen.current].filter((uri) => !requested.current.has(uri));
-    for (const uri of toFetch) {
-      requested.current.add(uri);
+  const record = useCallback((entries: Record<string, ResolvedUri | null>) => {
+    answered.current = { ...answered.current, ...entries };
+    setResolved((previous) => ({ ...previous, ...entries }));
+  }, []);
+
+  const flushRef = useRef<() => void>(() => {});
+
+  const flush = useCallback(() => {
+    const pending = [...seen.current].filter(
+      (uri) => !(uri in answered.current) && !inFlight.current.has(uri)
+    );
+    if (pending.length === 0) return;
+
+    for (const batch of planUriBatches(pending)) {
+      for (const uri of batch) inFlight.current.add(uri);
+
       const body = new FormData();
-      body.set("intent", "resolve");
-      body.set("uri", uri);
+      body.set("intent", "resolve-many");
+      body.set("uris", JSON.stringify(batch));
+
       fetch(actionPath, { method: "POST", body })
-        .then((res) => (res.ok ? (res.json() as Promise<{ path?: string; label?: string }>) : null))
+        .then(async (res) => {
+          if (!res.ok) throw new Error(`Failed to resolve links (${res.status})`);
+          return (await res.json()) as {
+            resolved?: Record<string, { path?: string; label?: string } | null>;
+          };
+        })
         .then((data) => {
-          setResolved((prev) => ({
-            ...prev,
-            [uri]: data?.path ? { url: data.path, label: data.label ?? uri } : null,
-          }));
+          // A 200 is the definitive answer, including "nothing to open": cached for good.
+          const entries: Record<string, ResolvedUri | null> = {};
+          for (const uri of batch) {
+            const hit = data.resolved?.[uri];
+            entries[uri] = hit?.path ? { url: hit.path, label: hit.label ?? uri } : null;
+          }
+          record(entries);
         })
         .catch(() => {
-          setResolved((prev) => ({ ...prev, [uri]: null }));
+          // Transient: a 5xx, a network blip, a deploy. Retried, then given up on.
+          const exhausted: Record<string, ResolvedUri | null> = {};
+          for (const uri of batch) {
+            const tried = (attempts.current.get(uri) ?? 0) + 1;
+            attempts.current.set(uri, tried);
+            if (tried >= MAX_RESOLVE_ATTEMPTS) exhausted[uri] = null;
+          }
+          if (Object.keys(exhausted).length > 0) record(exhausted);
+          // One timer for all batches; a render may never follow, so it can't be the trigger.
+          if (retryTimer.current === undefined) {
+            retryTimer.current = window.setTimeout(() => {
+              retryTimer.current = undefined;
+              flushRef.current();
+            }, RESOLVE_RETRY_DELAY_MS);
+          }
+        })
+        .finally(() => {
+          for (const uri of batch) inFlight.current.delete(uri);
         });
     }
+  }, [actionPath, record]);
+
+  // No dependency array on purpose: after every render, fetch whatever the cards
+  // asked about that hasn't been answered yet.
+  useEffect(() => {
+    flushRef.current = flush;
+    flush();
   });
 
+  useEffect(
+    () => () => {
+      if (retryTimer.current !== undefined) window.clearTimeout(retryTimer.current);
+    },
+    []
+  );
+
   return resolveUri;
 }

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts

@@ -32,6 +32,7 @@ import {
   MESSAGE_TOO_LARGE_CODE,
   MESSAGE_TOO_LARGE_ERROR,
 } from "~/components/dashboard-agent/message-limits";
+import { MAX_URIS_PER_RESOLVE_REQUEST } from "~/components/dashboard-agent/resolve-uris";
 import { $replica } from "~/db.server";
 import { env } from "~/env.server";
 import { findProjectBySlug } from "~/models/project.server";
@@ -82,6 +83,7 @@ const ActionBody = z.object({
     "delete",
     "read",
     "resolve",
+    "resolve-many",
     "watch-cancel",
     "watch-create",
   ]),
@@ -94,6 +96,8 @@ const ActionBody = z.object({
   pinned: z.enum(["true", "false"]).optional(),
   // A `trigger://` URI, for `resolve`.
   uri: z.string().optional(),
+  // A JSON array of `trigger://` URIs, for `resolve-many`.
+  uris: z.string().optional(),
   // The watch to cancel, for `watch-cancel`.
   watchId: z.string().min(1).optional(),
   // The configured card, for `watch-create`: a JSON `WatchDraft`.
@@ -207,6 +211,20 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
   });
 };
 
+/** Only a source URI needs the connected repository, so a batch without one skips the read. */
+async function findRepositoryForSourceUris(projectId: string, uris: string[]) {
+  if (!uris.some((uri) => uri.includes("/source/"))) return null;
+
+  const connected = await $replica.connectedGithubRepository.findFirst({
+    where: {
+      projectId,
+      repository: { installation: { deletedAt: null, suspendedAt: null } },
+    },
+    select: { repository: { select: { fullName: true } } },
+  });
+  return connected?.repository ?? null;
+}
+
 function messageTooLarge() {
   return json({ error: MESSAGE_TOO_LARGE_ERROR, code: MESSAGE_TOO_LARGE_CODE }, { status: 413 });
 }
@@ -335,21 +353,9 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
     const environment = await findEnvironmentBySlug(project.id, envParam, userId);
     if (!environment) return json({ error: "Environment not found" }, { status: 404 });
 
-    // Only a source URI needs the connected repository.
-    const repository = uri.includes("/source/")
-      ? await $replica.connectedGithubRepository.findFirst({
-          where: {
-            projectId: project.id,
-            repository: { installation: { deletedAt: null, suspendedAt: null } },
-          },
-          select: { repository: { select: { fullName: true } } },
-        })
-      : null;
+    const repository = await findRepositoryForSourceUris(project.id, [uri]);
 
-    const resolved = resolveTriggerUri(
-      { ...environment, repository: repository?.repository ?? null },
-      uri
-    );
+    const resolved = resolveTriggerUri({ ...environment, repository }, uri);
     if (!resolved) return json({ error: "Nothing to open for that link" }, { status: 404 });
 
     return json({
@@ -359,6 +365,43 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
     });
   }
 
+  // The card's citations in one request: one environment lookup and one repo lookup for the
+  // whole batch, same environment scope as `resolve`.
+  if (parsed.data.intent === "resolve-many") {
+    let uris: string[];
+    try {
+      const list = JSON.parse(parsed.data.uris ?? "") as unknown;
+      if (!Array.isArray(list) || list.some((uri) => typeof uri !== "string")) {
+        return json({ error: "uris is required" }, { status: 400 });
+      }
+      uris = [...new Set(list as string[])];
+    } catch {
+      return json({ error: "uris is required" }, { status: 400 });
+    }
+
+    if (uris.length === 0) return json({ error: "uris is required" }, { status: 400 });
+    if (uris.length > MAX_URIS_PER_RESOLVE_REQUEST) {
+      return json({ error: "Too many links in one request" }, { status: 400 });
+    }
+
+    const environment = await findEnvironmentBySlug(project.id, envParam, userId);
+    if (!environment) return json({ error: "Environment not found" }, { status: 404 });
+
+    const repository = await findRepositoryForSourceUris(project.id, uris);
+    const scope = { ...environment, repository };
+
+    // A null entry is the definitive "nothing to open": the client caches it.
+    const resolved: Record<string, { path: string; label: string; external: boolean } | null> = {};
+    for (const uri of uris) {
+      const hit = resolveTriggerUri(scope, uri);
+      resolved[uri] = hit
+        ? { path: hit.url, label: hit.label, external: hit.external ?? false }
+        : null;
+    }
+
+    return json({ resolved });
+  }
+
   // The configuration card's submit path. The environment comes from the URL and goes
   // through the same re-authorization a background tick passes, never from the body.
   if (parsed.data.intent === "watch-create") {