Push the search namespace into the tools read

Empty-query enumeration filtered tools by exact integration slug after listing everything. Pass it as the read filter instead, so the read refreshes only that integration. Ranked search stays unscoped: its namespace match is token-prefix.

Rhys Sullivan committed 3 months ago

Commit 92116ff1 parent 071b9701 file · +31 −14Browse files

Unified · Split

packages/core/execution/src/tool-invoker.ts+31−14

   authToolFailure,
   isUserActionableError,
   isToolResult,
+  IntegrationSlug,
   ToolResult,
   ToolAddress,
   parseToolAddress,
   });
   const emptyQuery = normalizeSearchText(query).length === 0;
-  const hasNamespace =
+  // The exact integration slug an empty-query enumeration reads under; null
-    options?.namespace !== undefined && normalizeSearchText(options.namespace).length > 0;
+  // when the caller named no usable namespace.
+  const scope =
+    options?.namespace !== undefined && normalizeSearchText(options.namespace).length > 0
+      ? IntegrationSlug.make(options.namespace.trim())
+      : null;
   // An empty query with no namespace stays empty: it carries neither a
   // ranking signal nor a scope, and listing the whole workspace "by default"
   // is exactly the arbitrary dump the ranked search refuses to be.
-  if (emptyQuery && !hasNamespace) {
+  if (emptyQuery && scope === null) {
     return {
       items: [],
       total: 0,
     } satisfies PagedResult<ToolDiscoveryResult>;
   }
-  const all = yield* executor.tools.list({ includeAnnotations: false }).pipe(
+  // Enumeration's scope is an exact integration slug (see below), so it is the
-    Effect.mapError(
+  // read's filter, not a post-read predicate: pushing it down lets the list
-      (cause) =>
+  // narrow its own catalog refresh to that one integration instead of
-        new ExecutionToolError({
+  // re-listing every stale connection in the workspace. Ranked search is
-          message: "Failed to list tools for search",
+  // deliberately NOT scoped here — `matchesNamespace` is token-prefix, so a
-          cause,
+  // scoped read would drop the prefix-sibling integrations it means to match.
-        }),
+  const all = yield* executor.tools
-    ),
+    .list({
-  );
+      includeAnnotations: false,
+      ...(emptyQuery && scope !== null ? { integration: scope } : {}),
+    })
+    .pipe(
+      Effect.mapError(
+        (cause) =>
+          new ExecutionToolError({
+            message: "Failed to list tools for search",
+            cause,
+          }),
+      ),
+    );
   const searchable = all.map(toSearchableTool);
   // An empty query WITH a namespace is enumeration, not search: there is no
   // sweep in prefix-sibling integrations (namespace "google" matching
   // google_gmail and google_sheets), which would silently break the census
   // guarantee: `total` here must reconcile against
-  // `executor.integrations.list`'s per-integration toolCount.
+  // `executor.integrations.list`'s per-integration toolCount. That exact match
+  // is now the `integration` filter on the read above, so `searchable` is
+  // already the namespace's catalog and re-filtering it here would be a no-op.
   const ranked: readonly ToolDiscoveryResult[] = emptyQuery
     ? searchable
-        .filter((tool) => tool.integration === options?.namespace?.trim())
         .sort((left, right) => left.path.localeCompare(right.path))
         .map((tool) => ({
           path: tool.path,