fix(provider): address Qdrant review findings

l1shen · l1shen · commit 8940069f4d11 · 2026-07-28T14:10:43.000+08:00
diff --git a/src/providers/qdrant/actions.ts b/src/providers/qdrant/actions.ts
@@ -5,15 +5,33 @@ import { defineProviderAction } from "../../core/provider-definition.ts";
 
 const service = "qdrant";
 
-const collectionNameSchema = s.nonEmptyString("The Qdrant collection name.");
-const pointIdSchema = s.union([s.nonNegativeInteger("A non-negative numeric point ID."), s.uuid("A UUID point ID.")]);
+const collectionNameSchema = {
+  ...s.nonEmptyString("The Qdrant collection name. The names `.` and `..` are not supported."),
+  not: { enum: [".", ".."] },
+};
+export const qdrantUuidPattern =
+  "^(?:[0-9A-Fa-f]{32}|[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}|[uU][rR][nN]:[uU][uU][iI][dD]:[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12})$";
+const pointIdSchema = s.union(
+  [
+    s.nonNegativeInteger("A numeric point ID within the JavaScript safe-integer range.", {
+      maximum: Number.MAX_SAFE_INTEGER,
+    }),
+    s.stringPattern(qdrantUuidPattern, {
+      description: "A Qdrant UUID point ID in simple, hyphenated, or URN form.",
+    }),
+  ],
+  { description: "A Qdrant numeric or UUID point ID." },
+);
 const vectorSchema = s.array("A dense unnamed vector.", s.number("One vector component."), { minItems: 1 });
 const payloadSchema = s.looseObject("A JSON object stored with the point.");
 const filterSchema = s.looseObject("A Qdrant filter. Nested conditions are validated by Qdrant.", {
   must: s.unknown("Conditions that must match."),
   must_not: s.unknown("Conditions that must not match."),
   should: s.unknown("Conditions where at least one should match."),
-  min_should: s.unknown("Minimum number of should conditions that must match."),
+  min_should: s.requiredObject("Conditions where at least `min_count` entries must match.", {
+    conditions: s.array("The Qdrant filter conditions to evaluate.", s.unknown("One Qdrant filter condition.")),
+    min_count: s.nonNegativeInteger("The minimum number of conditions that must match."),
+  }),
 });
 
 const pointSchema = s.object(
@@ -134,7 +152,11 @@ export const qdrantActions: ProviderActionDefinition[] = [
     ),
     outputSchema: s.actionOutput(
       {
-        operationId: s.nullable(s.nonNegativeInteger("The Qdrant operation ID when returned.")),
+        operationId: s.nullable(
+          s.nonNegativeInteger("The Qdrant operation ID when returned, within the JavaScript safe-integer range.", {
+            maximum: Number.MAX_SAFE_INTEGER,
+          }),
+        ),
         status: s.stringEnum("The Qdrant write status.", ["acknowledged", "completed", "wait_timeout"]),
       },
       "The Qdrant upsert operation result.",
diff --git a/src/providers/qdrant/definition.ts b/src/providers/qdrant/definition.ts
@@ -23,9 +23,9 @@ export const provider: ProviderDefinition = {
           inputType: "text",
           required: true,
           secret: false,
-          placeholder: "https://your-cluster.cloud.qdrant.io:6333",
+          placeholder: "https://your-cluster.cloud.qdrant.io",
           description:
-            "The HTTPS REST URL for a Qdrant Cloud cluster. Only official cloud.qdrant.io endpoints on port 6333 are supported.",
+            "The HTTPS REST URL for a Qdrant Cloud cluster. Only official cloud.qdrant.io endpoints on port 443 or 6333 are supported.",
         },
         {
           key: "apiKey",
diff --git a/src/providers/qdrant/runtime.test.ts b/src/providers/qdrant/runtime.test.ts
@@ -1,6 +1,8 @@
 import type { ProviderFetch } from "../provider-runtime.ts";
 
 import { describe, expect, it, vi } from "vitest";
+import { validateActionInput } from "../../core/validation.ts";
+import { qdrantActions } from "./actions.ts";
 import { createQdrantContext, qdrantActionHandlers, validateQdrantCredential } from "./runtime.ts";
 
 const clusterUrl = "https://test-cluster.cloud.qdrant.io:6333";
@@ -86,6 +88,51 @@ describe("Qdrant runtime", () => {
     expect(requestOf(point.fetcher).url).toBe(`${clusterUrl}/collections/my%20collection/points/42`);
   });
 
+  it("keeps the public point ID schema aligned with Qdrant UUID forms and safe numeric IDs", async () => {
+    const action = qdrantActions.find((candidate) => candidate.name === "get_point")!;
+    const ids = [
+      "936DA01F9ABD4d9d80C702AF85C822A8",
+      "01890f3e-9c4a-7cc2-b3e4-8fef7d7c9b3a",
+      "urn:uuid:F9168C5E-CEB2-4faa-B6BF-329BF39FA1E4",
+      Number.MAX_SAFE_INTEGER,
+    ];
+
+    for (const id of ids) {
+      expect(validateActionInput(action, { collectionName: "documents", id }).valid).toBe(true);
+      const request = setup(jsonResponse({ id }));
+      await expect(
+        qdrantActionHandlers.get_point({ collectionName: "documents", id }, request.context),
+      ).resolves.toEqual({ point: { id } });
+      expect(requestOf(request.fetcher).url).toBe(
+        `${clusterUrl}/collections/documents/points/${encodeURIComponent(String(id))}`,
+      );
+    }
+
+    const unsafeId = Number.MAX_SAFE_INTEGER + 1;
+    expect(validateActionInput(action, { collectionName: "documents", id: unsafeId }).valid).toBe(false);
+    const invalid = setup();
+    await expect(
+      qdrantActionHandlers.get_point({ collectionName: "documents", id: unsafeId }, invalid.context),
+    ).rejects.toMatchObject({ status: 400 });
+    expect(invalid.fetcher).not.toHaveBeenCalled();
+  });
+
+  it("preserves collection name whitespace and rejects dot-segment names", async () => {
+    const spaced = setup(jsonResponse({ status: "green" }));
+    await qdrantActionHandlers.get_collection({ collectionName: " documents " }, spaced.context);
+    expect(requestOf(spaced.fetcher).url).toBe(`${clusterUrl}/collections/%20documents%20`);
+
+    const action = qdrantActions.find((candidate) => candidate.name === "get_collection")!;
+    for (const collectionName of [".", ".."]) {
+      expect(validateActionInput(action, { collectionName }).valid).toBe(false);
+      const invalid = setup();
+      await expect(qdrantActionHandlers.get_collection({ collectionName }, invalid.context)).rejects.toMatchObject({
+        status: 400,
+      });
+      expect(invalid.fetcher).not.toHaveBeenCalled();
+    }
+  });
+
   it("normalizes a null operation ID and rejects excluded point fields", async () => {
     const upsert = setup(jsonResponse({ operation_id: null, status: "wait_timeout" }));
     await expect(
@@ -105,6 +152,35 @@ describe("Qdrant runtime", () => {
     expect(invalid.fetcher).not.toHaveBeenCalled();
   });
 
+  it("rejects point and operation IDs that cannot be represented without precision loss", async () => {
+    const point = setup(jsonResponse({ id: Number.MAX_SAFE_INTEGER + 1 }));
+    await expect(
+      qdrantActionHandlers.get_point({ collectionName: "documents", id: 1 }, point.context),
+    ).rejects.toMatchObject({
+      status: 502,
+      message: "Qdrant returned an invalid point ID",
+    });
+
+    const query = setup(jsonResponse({ points: [{ id: Number.MAX_SAFE_INTEGER + 1 }] }));
+    await expect(
+      qdrantActionHandlers.query_points({ collectionName: "documents", vector: [0.1] }, query.context),
+    ).rejects.toMatchObject({
+      status: 502,
+      message: "Qdrant returned an invalid query point ID",
+    });
+
+    const operation = setup(jsonResponse({ operation_id: Number.MAX_SAFE_INTEGER + 1, status: "completed" }));
+    await expect(
+      qdrantActionHandlers.upsert_points(
+        { collectionName: "documents", points: [{ id: 1, vector: [0.1] }] },
+        operation.context,
+      ),
+    ).rejects.toMatchObject({
+      status: 502,
+      message: "Qdrant returned an invalid operation_id",
+    });
+  });
+
   it("maps query and scroll inputs and normalizes the next page offset", async () => {
     const query = setup(jsonResponse({ points: [{ id: 1, version: 2, score: 0.9 }] }));
     const queryResult = await qdrantActionHandlers.query_points(
@@ -148,6 +224,15 @@ describe("Qdrant runtime", () => {
     });
   });
 
+  it("maps invalid response pagination offsets to provider errors", async () => {
+    const { context } = setup(jsonResponse({ points: [], next_page_offset: "not-a-point-id" }));
+
+    await expect(qdrantActionHandlers.scroll_points({ collectionName: "documents" }, context)).rejects.toMatchObject({
+      status: 502,
+      message: "Qdrant returned an invalid next_page_offset",
+    });
+  });
+
   it("preserves execute-phase permission errors and maps validation permission errors", async () => {
     const execute = setup(errorResponse("forbidden", 403));
     await expect(
@@ -165,10 +250,18 @@ describe("Qdrant runtime", () => {
   });
 
   it("rejects unsafe or non-Qdrant Cloud cluster URLs", () => {
+    for (const validUrl of [
+      "https://test-cluster.cloud.qdrant.io",
+      "https://test-cluster.cloud.qdrant.io:443",
+      clusterUrl,
+    ]) {
+      expect(() => createQdrantContext({ clusterUrl: validUrl, apiKey }, vi.fn() as ProviderFetch)).not.toThrow();
+    }
+
     for (const invalidUrl of [
       "http://test-cluster.cloud.qdrant.io:6333",
       "https://test-cluster.example.com:6333",
-      "https://test-cluster.cloud.qdrant.io:443",
+      "https://test-cluster.cloud.qdrant.io:6334",
       "https://test-cluster.cloud.qdrant.io:6333/path",
       "https://test-cluster.cloud.qdrant.io:6333?secret=1",
       "https://user:pass@test-cluster.cloud.qdrant.io:6333",
diff --git a/src/providers/qdrant/runtime.ts b/src/providers/qdrant/runtime.ts
@@ -1,8 +1,15 @@
 import type { CredentialValidationResult } from "../../core/types.ts";
 import type { ProviderFetch, ProviderRuntimeHandler } from "../provider-runtime.ts";
 
-import { compactObject, optionalInteger, optionalRecord, optionalString, requiredString } from "../../core/cast.ts";
-import { assertPublicHttpUrl } from "../../core/request.ts";
+import {
+  compactObject,
+  optionalInteger,
+  optionalRawString,
+  optionalRecord,
+  optionalString,
+  requiredString,
+} from "../../core/cast.ts";
+import { assertPublicHttpUrl, encodePathSegment } from "../../core/request.ts";
 import {
   createProviderTimeout,
   isAbortSignalError,
@@ -11,11 +18,11 @@ import {
   providerUserAgent,
   readProviderTextBody,
 } from "../provider-runtime.ts";
+import { qdrantUuidPattern } from "./actions.ts";
 
 const service = "qdrant";
 const requestTimeoutMs = 30_000;
 const qdrantHostnameSuffix = ".cloud.qdrant.io";
-const qdrantPort = "6333";
 
 type QdrantRequestPhase = "validate" | "execute";
 type QdrantHttpMethod = "GET" | "POST" | "PUT";
@@ -39,7 +46,7 @@ export const qdrantActionHandlers: Record<string, ProviderRuntimeHandler<QdrantC
     const collectionName = readCollectionName(input);
     const payload = await requestQdrantJson(
       context,
-      `/collections/${encodeURIComponent(collectionName)}`,
+      `/collections/${encodePathSegment(collectionName)}`,
       "GET",
       undefined,
       "execute",
@@ -56,7 +63,7 @@ export const qdrantActionHandlers: Record<string, ProviderRuntimeHandler<QdrantC
     }
     const payload = await requestQdrantJson(
       context,
-      `/collections/${encodeURIComponent(collectionName)}`,
+      `/collections/${encodePathSegment(collectionName)}`,
       "PUT",
       { vectors: { size: vectorSize, distance } },
       "execute",
@@ -69,7 +76,7 @@ export const qdrantActionHandlers: Record<string, ProviderRuntimeHandler<QdrantC
     const points = readPoints(input.points);
     const payload = await requestQdrantJson(
       context,
-      `/collections/${encodeURIComponent(collectionName)}/points?wait=true`,
+      `/collections/${encodePathSegment(collectionName)}/points?wait=true`,
       "PUT",
       { points },
       "execute",
@@ -86,12 +93,16 @@ export const qdrantActionHandlers: Record<string, ProviderRuntimeHandler<QdrantC
     const id = readPointId(input.id);
     const payload = await requestQdrantJson(
       context,
-      `/collections/${encodeURIComponent(collectionName)}/points/${encodeURIComponent(String(id))}`,
+      `/collections/${encodePathSegment(collectionName)}/points/${encodePathSegment(id)}`,
       "GET",
       undefined,
       "execute",
     );
-    return { point: requireObjectResult(payload, "Qdrant point response") };
+    const point = requireObjectResult(payload, "Qdrant point response");
+    if (!isPointId(point.id)) {
+      throw providerResponseError("Qdrant returned an invalid point ID");
+    }
+    return { point };
   },
 
   async query_points(input, context) {
@@ -107,13 +118,13 @@ export const qdrantActionHandlers: Record<string, ProviderRuntimeHandler<QdrantC
     };
     const payload = await requestQdrantJson(
       context,
-      `/collections/${encodeURIComponent(collectionName)}/points/query`,
+      `/collections/${encodePathSegment(collectionName)}/points/query`,
       "POST",
       compactObject(body),
       "execute",
     );
     const result = requireObjectResult(payload, "Qdrant query response");
-    return { points: Array.isArray(result.points) ? result.points : [] };
+    return { points: readPointRecords(result.points, "query") };
   },
 
   async scroll_points(input, context) {
@@ -128,15 +139,15 @@ export const qdrantActionHandlers: Record<string, ProviderRuntimeHandler<QdrantC
     };
     const payload = await requestQdrantJson(
       context,
-      `/collections/${encodeURIComponent(collectionName)}/points/scroll`,
+      `/collections/${encodePathSegment(collectionName)}/points/scroll`,
       "POST",
       compactObject(body),
       "execute",
     );
     const result = requireObjectResult(payload, "Qdrant scroll response");
     const nextOffset = readNullablePointId(result.next_page_offset);
     return {
-      points: Array.isArray(result.points) ? result.points : [],
+      points: readPointRecords(result.points, "scroll"),
       nextOffset,
       complete: nextOffset === null,
     };
@@ -241,8 +252,8 @@ function normalizeQdrantClusterUrl(value: string | undefined): URL {
   if (url.username || url.password) {
     throw providerInputError("clusterUrl must not include credentials");
   }
-  if (url.port !== qdrantPort) {
-    throw providerInputError("clusterUrl must use port 6333");
+  if (url.port !== "" && url.port !== "6333") {
+    throw providerInputError("clusterUrl must use port 443 or 6333");
   }
   if (url.pathname !== "/" || url.search || url.hash) {
     throw providerInputError("clusterUrl must not include a path, query, or fragment");
@@ -284,7 +295,14 @@ function createQdrantError(status: number, payload: unknown, phase: QdrantReques
 }
 
 function readCollectionName(input: Record<string, unknown>): string {
-  return requiredString(input.collectionName, "collectionName", providerInputError);
+  const collectionName = optionalRawString(input.collectionName);
+  if (collectionName === undefined || collectionName.length === 0) {
+    throw providerInputError("collectionName is required");
+  }
+  if (collectionName === "." || collectionName === "..") {
+    throw providerInputError("collectionName must not be . or ..");
+  }
+  return collectionName;
 }
 
 function readPoints(value: unknown): Record<string, unknown>[] {
@@ -315,17 +333,35 @@ function readPoints(value: unknown): Record<string, unknown>[] {
 }
 
 function readPointId(value: unknown): number | string {
-  if (typeof value === "number" && Number.isSafeInteger(value) && value >= 0) {
+  if (isPointId(value)) {
     return value;
   }
-  if (typeof value === "string" && uuidPattern.test(value)) {
+  throw providerInputError("id must be a non-negative safe integer or UUID");
+}
+
+function readNullablePointId(value: unknown): number | string | null {
+  if (value === null || value === undefined) {
+    return null;
+  }
+  if (isPointId(value)) {
     return value;
   }
-  throw providerInputError("id must be a non-negative integer or UUID");
+  throw providerResponseError("Qdrant returned an invalid next_page_offset");
 }
 
-function readNullablePointId(value: unknown): number | string | null {
-  return value === null || value === undefined ? null : readPointId(value);
+function isPointId(value: unknown): value is number | string {
+  return (
+    (typeof value === "number" && Number.isSafeInteger(value) && value >= 0) ||
+    (typeof value === "string" && uuidPattern.test(value))
+  );
+}
+
+function readPointRecords(value: unknown, operation: string): unknown[] {
+  const points = Array.isArray(value) ? value : [];
+  if (points.some((point) => !isPointId(optionalRecord(point)?.id))) {
+    throw providerResponseError(`Qdrant returned an invalid ${operation} point ID`);
+  }
+  return points;
 }
 
 function readVector(value: unknown, fieldName: string): number[] {
@@ -370,7 +406,9 @@ function readOptionalNumber(value: unknown, fieldName: string): number | undefin
 function readNullableInteger(value: unknown, fieldName: string): number | null {
   if (value === null || value === undefined) return null;
   const integer = optionalInteger(value);
-  if (integer === undefined || integer < 0) throw providerResponseError(`Qdrant returned an invalid ${fieldName}`);
+  if (integer === undefined || !Number.isSafeInteger(integer) || integer < 0) {
+    throw providerResponseError(`Qdrant returned an invalid ${fieldName}`);
+  }
   return integer;
 }
 
@@ -394,7 +432,7 @@ function isDistance(value: string): value is "Cosine" | "Euclid" | "Dot" | "Manh
   return value === "Cosine" || value === "Euclid" || value === "Dot" || value === "Manhattan";
 }
 
-const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
+const uuidPattern = new RegExp(qdrantUuidPattern);
 
 function providerInputError(message: string): ProviderRequestError {
   return new ProviderRequestError(400, message);
