test: add comprehensive tests for v1-git-service query parameter changes

- Add tests verifying query parameters are used instead of path segments - Add tests for preserving slashes in paths (main fix purpose) - Add tests for session API key headers - Add tests for V1 to V0 status mapping - Add tests for getGitChangeDiff endpoint Co-authored-by: openhands <openhands@all-hands.dev>
Merge branch 'main' into fix/git-api-use-query-params
2026-04-29 03:00:45 -04:00 · 2026-03-02 02:59:27 +00:00 · 2026-03-01 21:39:52 -05:00 · 2026-03-01 05:09:30 +00:00
2 changed files with 180 additions and 25 deletions
--- a/frontend/tests/api/v1-git-service.test.ts
+++ b/frontend/tests/api/v1-git-service.test.ts
@@ -1,18 +1,175 @@
-import { test, expect, vi } from "vitest";
+import { describe, test, expect, vi, beforeEach } from "vitest";
 import axios from "axios";
 import V1GitService from "../../src/api/git-service/v1-git-service.api";

 vi.mock("axios");

-test("getGitChanges throws when response is not an array (dead runtime returns HTML)", async () => {
-  const htmlResponse = "<!DOCTYPE html><html>...</html>";
-  vi.mocked(axios.get).mockResolvedValue({ data: htmlResponse });
+describe("V1GitService", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });

-  await expect(
-    V1GitService.getGitChanges(
-      "http://localhost:3000/api/conversations/123",
-      "test-api-key",
-      "/workspace",
-    ),
-  ).rejects.toThrow("Invalid response from runtime");
+  describe("getGitChanges", () => {
+    test("throws when response is not an array (dead runtime returns HTML)", async () => {
+      const htmlResponse = "<!DOCTYPE html><html>...</html>";
+      vi.mocked(axios.get).mockResolvedValue({ data: htmlResponse });
+
+      await expect(
+        V1GitService.getGitChanges(
+          "http://localhost:3000/api/conversations/123",
+          "test-api-key",
+          "/workspace",
+        ),
+      ).rejects.toThrow("Invalid response from runtime");
+    });
+
+    test("uses query parameters instead of path segments for the path", async () => {
+      vi.mocked(axios.get).mockResolvedValue({ data: [] });
+
+      await V1GitService.getGitChanges(
+        "http://localhost:3000/api/conversations/123",
+        "test-api-key",
+        "/workspace/project",
+      );
+
+      expect(axios.get).toHaveBeenCalledTimes(1);
+      const [url, config] = vi.mocked(axios.get).mock.calls[0];
+
+      // URL should NOT contain the path - it should end with /api/git/changes
+      expect(url).toContain("/api/git/changes");
+      expect(url).not.toContain("/workspace/project");
+      expect(url).not.toContain(encodeURIComponent("/workspace/project"));
+
+      // Path should be passed as a query parameter
+      expect(config).toHaveProperty("params");
+      expect(config?.params).toEqual({ path: "/workspace/project" });
+    });
+
+    test("preserves slashes in path when using query parameters", async () => {
+      vi.mocked(axios.get).mockResolvedValue({ data: [] });
+
+      const pathWithSlashes = "/workspace/project/src/components";
+      await V1GitService.getGitChanges(
+        "http://localhost:3000/api/conversations/123",
+        "test-api-key",
+        pathWithSlashes,
+      );
+
+      const [, config] = vi.mocked(axios.get).mock.calls[0];
+
+      // Path should be preserved exactly as provided (slashes intact)
+      expect(config?.params).toEqual({ path: pathWithSlashes });
+    });
+
+    test("includes session API key in headers when provided", async () => {
+      vi.mocked(axios.get).mockResolvedValue({ data: [] });
+
+      await V1GitService.getGitChanges(
+        "http://localhost:3000/api/conversations/123",
+        "my-session-key",
+        "/workspace",
+      );
+
+      const [, config] = vi.mocked(axios.get).mock.calls[0];
+      expect(config?.headers).toEqual({ "X-Session-API-Key": "my-session-key" });
+    });
+
+    test("maps V1 git statuses to V0 format", async () => {
+      vi.mocked(axios.get).mockResolvedValue({
+        data: [
+          { status: "ADDED", path: "new-file.ts" },
+          { status: "DELETED", path: "removed-file.ts" },
+          { status: "UPDATED", path: "changed-file.ts" },
+          { status: "MOVED", path: "renamed-file.ts" },
+        ],
+      });
+
+      const result = await V1GitService.getGitChanges(
+        "http://localhost:3000/api/conversations/123",
+        "test-api-key",
+        "/workspace",
+      );
+
+      expect(result).toEqual([
+        { status: "A", path: "new-file.ts" },
+        { status: "D", path: "removed-file.ts" },
+        { status: "M", path: "changed-file.ts" },
+        { status: "R", path: "renamed-file.ts" },
+      ]);
+    });
+  });
+
+  describe("getGitChangeDiff", () => {
+    test("uses query parameters instead of path segments for the path", async () => {
+      vi.mocked(axios.get).mockResolvedValue({
+        data: { diff: "--- a/file.ts\n+++ b/file.ts\n..." },
+      });
+
+      await V1GitService.getGitChangeDiff(
+        "http://localhost:3000/api/conversations/123",
+        "test-api-key",
+        "/workspace/project/file.ts",
+      );
+
+      expect(axios.get).toHaveBeenCalledTimes(1);
+      const [url, config] = vi.mocked(axios.get).mock.calls[0];
+
+      // URL should NOT contain the path - it should end with /api/git/diff
+      expect(url).toContain("/api/git/diff");
+      expect(url).not.toContain("/workspace/project/file.ts");
+      expect(url).not.toContain(encodeURIComponent("/workspace/project/file.ts"));
+
+      // Path should be passed as a query parameter
+      expect(config).toHaveProperty("params");
+      expect(config?.params).toEqual({ path: "/workspace/project/file.ts" });
+    });
+
+    test("preserves slashes in file path when using query parameters", async () => {
+      vi.mocked(axios.get).mockResolvedValue({
+        data: { diff: "diff content" },
+      });
+
+      const filePath = "/workspace/project/src/components/Button.tsx";
+      await V1GitService.getGitChangeDiff(
+        "http://localhost:3000/api/conversations/123",
+        "test-api-key",
+        filePath,
+      );
+
+      const [, config] = vi.mocked(axios.get).mock.calls[0];
+
+      // Path should be preserved exactly as provided (slashes intact)
+      expect(config?.params).toEqual({ path: filePath });
+    });
+
+    test("includes session API key in headers when provided", async () => {
+      vi.mocked(axios.get).mockResolvedValue({
+        data: { diff: "diff content" },
+      });
+
+      await V1GitService.getGitChangeDiff(
+        "http://localhost:3000/api/conversations/123",
+        "my-session-key",
+        "/workspace/file.ts",
+      );
+
+      const [, config] = vi.mocked(axios.get).mock.calls[0];
+      expect(config?.headers).toEqual({ "X-Session-API-Key": "my-session-key" });
+    });
+
+    test("returns the diff data from the response", async () => {
+      const expectedDiff = {
+        diff: "--- a/file.ts\n+++ b/file.ts\n@@ -1,3 +1,4 @@\n+new line",
+      };
+      vi.mocked(axios.get).mockResolvedValue({ data: expectedDiff });
+
+      const result = await V1GitService.getGitChangeDiff(
+        "http://localhost:3000/api/conversations/123",
+        "test-api-key",
+        "/workspace/file.ts",
+      );
+
+      expect(result).toEqual(expectedDiff);
+    });
+  });
 });
--- a/frontend/src/api/git-service/v1-git-service.api.ts
+++ b/frontend/src/api/git-service/v1-git-service.api.ts
@@ -30,7 +30,7 @@ class V1GitService {

  /**
   * Get git changes for a V1 conversation
-   * Uses the agent server endpoint: GET /api/git/changes/{path}
+   * Uses the agent server endpoint: GET /api/git/changes?path={path}
   * Maps V1 status types (ADDED, DELETED, etc.) to V0 format (A, D, etc.)
   *
   * @param conversationUrl The conversation URL (e.g., "http://localhost:54928/api/conversations/...")
@@ -43,15 +43,14 @@ class V1GitService {
    sessionApiKey: string | null | undefined,
    path: string,
  ): Promise<GitChange[]> {
-    const encodedPath = encodeURIComponent(path);
-    const url = this.buildRuntimeUrl(
-      conversationUrl,
-      `/api/git/changes/${encodedPath}`,
-    );
+    const url = this.buildRuntimeUrl(conversationUrl, `/api/git/changes`);
    const headers = buildSessionHeaders(sessionApiKey);

    // V1 API returns V1GitChangeStatus types, we need to map them to V0 format
-    const { data } = await axios.get<V1GitChange[]>(url, { headers });
+    const { data } = await axios.get<V1GitChange[]>(url, {
+      headers,
+      params: { path },
+    });

    // Validate response is an array (could be HTML error page if runtime is dead)
    if (!Array.isArray(data)) {
@@ -69,7 +68,7 @@ class V1GitService {

  /**
   * Get git change diff for a specific file in a V1 conversation
-   * Uses the agent server endpoint: GET /api/git/diff/{path}
+   * Uses the agent server endpoint: GET /api/git/diff?path={path}
   *
   * @param conversationUrl The conversation URL (e.g., "http://localhost:54928/api/conversations/...")
   * @param sessionApiKey Session API key for authentication (required for V1)
@@ -81,14 +80,13 @@ class V1GitService {
    sessionApiKey: string | null | undefined,
    path: string,
  ): Promise<GitChangeDiff> {
-    const encodedPath = encodeURIComponent(path);
-    const url = this.buildRuntimeUrl(
-      conversationUrl,
-      `/api/git/diff/${encodedPath}`,
-    );
+    const url = this.buildRuntimeUrl(conversationUrl, `/api/git/diff`);
    const headers = buildSessionHeaders(sessionApiKey);

-    const { data } = await axios.get<GitChangeDiff>(url, { headers });
+    const { data } = await axios.get<GitChangeDiff>(url, {
+      headers,
+      params: { path },
+    });
    return data;
  }
 }