From 9b09f08d95fec4e5b1a7ec1551b3391fe6d57b34 Mon Sep 17 00:00:00 2001 From: CW-B-W Date: Mon, 23 Jun 2025 13:10:27 +0800 Subject: [PATCH 1/3] feat(qdrant): add URL prefix handling for QdrantClient initialization --- .../__tests__/qdrant-client.spec.ts | 65 +++++++++++++++++++ .../code-index/vector-store/qdrant-client.ts | 2 + 2 files changed, 67 insertions(+) diff --git a/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts b/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts index 144571c28c..bc31e7bdf6 100644 --- a/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts +++ b/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts @@ -113,6 +113,7 @@ describe("QdrantVectorStore", () => { host: "qdrant.ashbyfam.com", https: true, port: 443, + prefix: undefined, // No prefix for root path apiKey: undefined, headers: { "User-Agent": "Roo-Code", @@ -127,6 +128,7 @@ describe("QdrantVectorStore", () => { host: "example.com", https: true, port: 9000, + prefix: undefined, // No prefix for root path apiKey: undefined, headers: { "User-Agent": "Roo-Code", @@ -145,6 +147,7 @@ describe("QdrantVectorStore", () => { host: "example.com", https: true, port: 443, + prefix: "/api/v1", // Should have prefix apiKey: undefined, headers: { "User-Agent": "Roo-Code", @@ -161,6 +164,7 @@ describe("QdrantVectorStore", () => { host: "example.com", https: false, port: 80, + prefix: undefined, // No prefix for root path apiKey: undefined, headers: { "User-Agent": "Roo-Code", @@ -175,6 +179,7 @@ describe("QdrantVectorStore", () => { host: "localhost", https: false, port: 8080, + prefix: undefined, // No prefix for root path apiKey: undefined, headers: { "User-Agent": "Roo-Code", @@ -193,6 +198,7 @@ describe("QdrantVectorStore", () => { host: "example.com", https: false, port: 80, + prefix: "/api/v1", // Should have prefix apiKey: undefined, headers: { "User-Agent": "Roo-Code", @@ -337,6 +343,65 @@ describe("QdrantVectorStore", () => { }) }) + describe("URL Prefix Handling", () => { + it("should pass the URL pathname as prefix to QdrantClient if not root", () => { + const vectorStoreWithPrefix = new QdrantVectorStore( + mockWorkspacePath, + "http://localhost:6333/some/path", + mockVectorSize, + ) + expect(QdrantClient).toHaveBeenLastCalledWith({ + host: "localhost", + https: false, + port: 6333, + prefix: "/some/path", + apiKey: undefined, + headers: { + "User-Agent": "Roo-Code", + }, + }) + expect((vectorStoreWithPrefix as any).qdrantUrl).toBe("http://localhost:6333/some/path") + }) + + it("should not pass prefix if the URL pathname is root ('/')", () => { + const vectorStoreWithoutPrefix = new QdrantVectorStore( + mockWorkspacePath, + "http://localhost:6333/", + mockVectorSize, + ) + expect(QdrantClient).toHaveBeenLastCalledWith({ + host: "localhost", + https: false, + port: 6333, + prefix: undefined, + apiKey: undefined, + headers: { + "User-Agent": "Roo-Code", + }, + }) + expect((vectorStoreWithoutPrefix as any).qdrantUrl).toBe("http://localhost:6333/") + }) + + it("should handle HTTPS URL with path as prefix", () => { + const vectorStoreWithHttpsPrefix = new QdrantVectorStore( + mockWorkspacePath, + "https://qdrant.ashbyfam.com/api", + mockVectorSize, + ) + expect(QdrantClient).toHaveBeenLastCalledWith({ + host: "qdrant.ashbyfam.com", + https: true, + port: 443, + prefix: "/api", + apiKey: undefined, + headers: { + "User-Agent": "Roo-Code", + }, + }) + expect((vectorStoreWithHttpsPrefix as any).qdrantUrl).toBe("https://qdrant.ashbyfam.com/api") + }) + }) + describe("initialize", () => { it("should create a new collection if none exists and return true", async () => { // Mock getCollection to throw a 404-like error diff --git a/src/services/code-index/vector-store/qdrant-client.ts b/src/services/code-index/vector-store/qdrant-client.ts index 7f4a04ed44..78632db672 100644 --- a/src/services/code-index/vector-store/qdrant-client.ts +++ b/src/services/code-index/vector-store/qdrant-client.ts @@ -57,6 +57,7 @@ export class QdrantVectorStore implements IVectorStore { host: urlObj.hostname, https: useHttps, port: port, + prefix: urlObj.pathname === "/" ? undefined : urlObj.pathname, apiKey, headers: { "User-Agent": "Roo-Code", @@ -64,6 +65,7 @@ export class QdrantVectorStore implements IVectorStore { }) } catch (urlError) { // If URL parsing fails, fall back to URL-based config + // Note: This fallback won't correctly handle prefixes, but it's a last resort for malformed URLs. this.client = new QdrantClient({ url: parsedUrl, apiKey, From 32ffeb4c52a66c74c423c26edb4e448396b13f28 Mon Sep 17 00:00:00 2001 From: CW-B-W Date: Tue, 24 Jun 2025 00:26:55 +0800 Subject: [PATCH 2/3] fix: Normalize Qdrant URL pathname to remove trailing slashes revised in accordance with the suggestions from @daniel-lxs * https://github.com/RooCodeInc/Roo-Code/pull/5033#discussion_r2161793239 * Normalize Qdrant URL pathname * https://github.com/RooCodeInc/Roo-Code/pull/5033#discussion_r2161793977 * More robust test cases --- .../__tests__/qdrant-client.spec.ts | 79 +++++++++++++++++++ .../code-index/vector-store/qdrant-client.ts | 2 +- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts b/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts index bc31e7bdf6..4b0df4018e 100644 --- a/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts +++ b/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts @@ -400,6 +400,85 @@ describe("QdrantVectorStore", () => { }) expect((vectorStoreWithHttpsPrefix as any).qdrantUrl).toBe("https://qdrant.ashbyfam.com/api") }) + + it("should normalize URL pathname by removing trailing slash for prefix", () => { + const vectorStoreWithTrailingSlash = new QdrantVectorStore( + mockWorkspacePath, + "http://localhost:6333/api/", + mockVectorSize, + ) + expect(QdrantClient).toHaveBeenLastCalledWith({ + host: "localhost", + https: false, + port: 6333, + prefix: "/api", // Trailing slash should be removed + apiKey: undefined, + headers: { + "User-Agent": "Roo-Code", + }, + }) + expect((vectorStoreWithTrailingSlash as any).qdrantUrl).toBe("http://localhost:6333/api/") + }) + + it("should handle multiple path segments correctly for prefix", () => { + const vectorStoreWithMultiSegment = new QdrantVectorStore( + mockWorkspacePath, + "http://localhost:6333/api/v1/qdrant", + mockVectorSize, + ) + expect(QdrantClient).toHaveBeenLastCalledWith({ + host: "localhost", + https: false, + port: 6333, + prefix: "/api/v1/qdrant", + apiKey: undefined, + headers: { + "User-Agent": "Roo-Code", + }, + }) + expect((vectorStoreWithMultiSegment as any).qdrantUrl).toBe("http://localhost:6333/api/v1/qdrant") + }) + + it("should handle complex URL with multiple segments, trailing slash, query params, and fragment", () => { + const complexUrl = "https://example.com/ollama/api/v1/?key=value#pos" + const vectorStoreComplex = new QdrantVectorStore( + mockWorkspacePath, + complexUrl, + mockVectorSize, + ) + expect(QdrantClient).toHaveBeenLastCalledWith({ + host: "example.com", + https: true, + port: 443, + prefix: "/ollama/api/v1", // Trailing slash removed, query/fragment ignored + apiKey: undefined, + headers: { + "User-Agent": "Roo-Code", + }, + }) + expect((vectorStoreComplex as any).qdrantUrl).toBe(complexUrl) + }) + + it("should ignore query parameters and fragments when determining prefix", () => { + const vectorStoreWithQueryParams = new QdrantVectorStore( + mockWorkspacePath, + "http://localhost:6333/api/path?key=value#fragment", + mockVectorSize, + ) + expect(QdrantClient).toHaveBeenLastCalledWith({ + host: "localhost", + https: false, + port: 6333, + prefix: "/api/path", // Query params and fragment should be ignored + apiKey: undefined, + headers: { + "User-Agent": "Roo-Code", + }, + }) + expect((vectorStoreWithQueryParams as any).qdrantUrl).toBe( + "http://localhost:6333/api/path?key=value#fragment", + ) + }) }) describe("initialize", () => { diff --git a/src/services/code-index/vector-store/qdrant-client.ts b/src/services/code-index/vector-store/qdrant-client.ts index 78632db672..c88e0a92ad 100644 --- a/src/services/code-index/vector-store/qdrant-client.ts +++ b/src/services/code-index/vector-store/qdrant-client.ts @@ -57,7 +57,7 @@ export class QdrantVectorStore implements IVectorStore { host: urlObj.hostname, https: useHttps, port: port, - prefix: urlObj.pathname === "/" ? undefined : urlObj.pathname, + prefix: urlObj.pathname === "/" ? undefined : urlObj.pathname.replace(/\/$/, ""), apiKey, headers: { "User-Agent": "Roo-Code", From e522eb3f11e34d2568c6180029e51d2a122d6fc9 Mon Sep 17 00:00:00 2001 From: CW-B-W Date: Tue, 24 Jun 2025 00:57:30 +0800 Subject: [PATCH 3/3] fix: Normalize Qdrant URL pathname to remove multiple trailing slashes The previous commit may not work if there are multiple trailing slashes --- .../__tests__/qdrant-client.spec.ts | 31 ++++++++++++++----- .../code-index/vector-store/qdrant-client.ts | 2 +- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts b/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts index 4b0df4018e..7ed9afb179 100644 --- a/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts +++ b/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts @@ -420,6 +420,25 @@ describe("QdrantVectorStore", () => { expect((vectorStoreWithTrailingSlash as any).qdrantUrl).toBe("http://localhost:6333/api/") }) + it("should normalize URL pathname by removing multiple trailing slashes for prefix", () => { + const vectorStoreWithMultipleTrailingSlashes = new QdrantVectorStore( + mockWorkspacePath, + "http://localhost:6333/api///", + mockVectorSize, + ) + expect(QdrantClient).toHaveBeenLastCalledWith({ + host: "localhost", + https: false, + port: 6333, + prefix: "/api", // All trailing slashes should be removed + apiKey: undefined, + headers: { + "User-Agent": "Roo-Code", + }, + }) + expect((vectorStoreWithMultipleTrailingSlashes as any).qdrantUrl).toBe("http://localhost:6333/api///") + }) + it("should handle multiple path segments correctly for prefix", () => { const vectorStoreWithMultiSegment = new QdrantVectorStore( mockWorkspacePath, @@ -438,14 +457,10 @@ describe("QdrantVectorStore", () => { }) expect((vectorStoreWithMultiSegment as any).qdrantUrl).toBe("http://localhost:6333/api/v1/qdrant") }) - - it("should handle complex URL with multiple segments, trailing slash, query params, and fragment", () => { - const complexUrl = "https://example.com/ollama/api/v1/?key=value#pos" - const vectorStoreComplex = new QdrantVectorStore( - mockWorkspacePath, - complexUrl, - mockVectorSize, - ) + + it("should handle complex URL with multiple segments, multiple trailing slashes, query params, and fragment", () => { + const complexUrl = "https://example.com/ollama/api/v1///?key=value#pos" + const vectorStoreComplex = new QdrantVectorStore(mockWorkspacePath, complexUrl, mockVectorSize) expect(QdrantClient).toHaveBeenLastCalledWith({ host: "example.com", https: true, diff --git a/src/services/code-index/vector-store/qdrant-client.ts b/src/services/code-index/vector-store/qdrant-client.ts index c88e0a92ad..c0f19af28b 100644 --- a/src/services/code-index/vector-store/qdrant-client.ts +++ b/src/services/code-index/vector-store/qdrant-client.ts @@ -57,7 +57,7 @@ export class QdrantVectorStore implements IVectorStore { host: urlObj.hostname, https: useHttps, port: port, - prefix: urlObj.pathname === "/" ? undefined : urlObj.pathname.replace(/\/$/, ""), + prefix: urlObj.pathname === "/" ? undefined : urlObj.pathname.replace(/\/+$/, ""), apiKey, headers: { "User-Agent": "Roo-Code",