github/OpenHands
1
1
Fork 0
mirror of https://github.com/All-Hands-AI/OpenHands.git synced 2026-04-29 03:00:45 -04:00
Code Issues Packages Projects Releases Wiki Activity

V1 Changes to Support Path Based Routing (#13120)

Browse Source 
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
chuckbutkus
2026-03-02 22:37:37 -05:00
committed by GitHub
parent 4dab34e7b0
commit 0c7ce4ad48
5 changed files with 241 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

179
frontend/__tests__/api/v1-git-service.test.ts
View File

@@ -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);
});
});
});

26
frontend/src/api/git-service/v1-git-service.api.ts
View File

@@ -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;
}
}

28
openhands/app_server/sandbox/remote_sandbox_service.py
View File

@@ -4,6 +4,7 @@ import logging
import os
from dataclasses import dataclass
from typing import Any, AsyncGenerator, Union
from urllib.parse import urlparse
from uuid import UUID
import base62
@@ -142,12 +143,13 @@ class RemoteSandboxService(SandboxService):
exposed_urls = []
url = runtime.get('url', None)
if url:
runtime_id = runtime['runtime_id']
exposed_urls.append(
ExposedUrl(name=AGENT_SERVER, url=url, port=AGENT_SERVER_PORT)
)
vscode_url = (
_build_service_url(url, 'vscode')
+ f'/?tkn={session_api_key}&folder=%2Fworkspace%2Fproject'
_build_service_url(url, 'vscode', runtime_id)
+ f'?tkn={session_api_key}&folder=%2Fworkspace%2Fproject'
)
exposed_urls.append(
ExposedUrl(name=VSCODE, url=vscode_url, port=VSCODE_PORT)
@@ -155,14 +157,14 @@ class RemoteSandboxService(SandboxService):
exposed_urls.append(
ExposedUrl(
name=WORKER_1,
url=_build_service_url(url, 'work-1'),
url=_build_service_url(url, 'work-1', runtime_id),
port=WORKER_1_PORT,
)
)
exposed_urls.append(
ExposedUrl(
name=WORKER_2,
url=_build_service_url(url, 'work-2'),
url=_build_service_url(url, 'work-2', runtime_id),
port=WORKER_2_PORT,
)
)
@@ -663,9 +665,21 @@ class RemoteSandboxService(SandboxService):
return results
def _build_service_url(url: str, service_name: str):
scheme, host_and_path = url.split('://')
return scheme + '://' + service_name + '-' + host_and_path
def _build_service_url(url: str, service_name: str, runtime_id: str) -> str:
"""Build a service URL for the given service name.
Handles both path-based and subdomain-based routing:
- Path mode (url path starts with /{runtime_id}): returns {scheme}://{netloc}/{runtime_id}/{service_name}
- Subdomain mode: returns {scheme}://{service_name}-{netloc}{path}
"""
parsed = urlparse(url)
scheme, netloc, path = parsed.scheme, parsed.netloc, parsed.path or '/'
# Path mode if runtime_url path starts with /{id}
path_mode = path.startswith(f'/{runtime_id}')
if path_mode:
return f'{scheme}://{netloc}/{runtime_id}/{service_name}'
else:
return f'{scheme}://{service_name}-{netloc}{path}'
async def poll_agent_servers(api_url: str, api_key: str, sleep_interval: int):

2
openhands/app_server/sandbox/sandbox_spec_service.py
View File

@@ -13,7 +13,7 @@ from openhands.sdk.utils.models import DiscriminatedUnionMixin
# The version of the agent server to use for deployments.
# Typically this will be the same as the values from the pyproject.toml
AGENT_SERVER_IMAGE = 'ghcr.io/openhands/agent-server:010e847-python'
AGENT_SERVER_IMAGE = 'ghcr.io/openhands/agent-server:2c1e72a-python'
class SandboxSpecService(ABC):

46
tests/unit/app_server/test_remote_sandbox_service.py
View File

@@ -1196,19 +1196,51 @@ class TestGetSandboxBySessionApiKey:
class TestUtilityFunctions:
"""Test cases for utility functions."""
def test_build_service_url(self):
"""Test _build_service_url function."""
def test_build_service_url_subdomain_mode(self):
"""Test _build_service_url function with subdomain-based routing."""
from openhands.app_server.sandbox.remote_sandbox_service import (
_build_service_url,
)
# Test HTTPS URL
result = _build_service_url('https://sandbox.example.com/path', 'vscode')
# Test HTTPS URL with path (subdomain mode)
result = _build_service_url(
'https://sandbox.example.com/path', 'vscode', 'runtime-123'
)
assert result == 'https://vscode-sandbox.example.com/path'
# Test HTTP URL
result = _build_service_url('http://localhost:8000', 'work-1')
assert result == 'http://work-1-localhost:8000'
# Test HTTP URL without path (subdomain mode)
result = _build_service_url(
'http://localhost:8000', 'work-1', 'different-runtime'
)
assert result == 'http://work-1-localhost:8000/'
# Test URL with empty path (subdomain mode)
result = _build_service_url('https://sandbox.example.com', 'work-2', 'some-id')
assert result == 'https://work-2-sandbox.example.com/'
def test_build_service_url_path_mode(self):
"""Test _build_service_url function with path-based routing."""
from openhands.app_server.sandbox.remote_sandbox_service import (
_build_service_url,
)
# Test path-based routing where URL path starts with /{runtime_id}
result = _build_service_url(
'https://sandbox.example.com/runtime-123', 'vscode', 'runtime-123'
)
assert result == 'https://sandbox.example.com/runtime-123/vscode'
# Test path-based routing with work-1
result = _build_service_url(
'https://sandbox.example.com/my-runtime-id', 'work-1', 'my-runtime-id'
)
assert result == 'https://sandbox.example.com/my-runtime-id/work-1'
# Test path-based routing with work-2
result = _build_service_url(
'http://localhost:8080/abc-xyz-123', 'work-2', 'abc-xyz-123'
)
assert result == 'http://localhost:8080/abc-xyz-123/work-2'
def test_hash_session_api_key(self):
"""Test _hash_session_api_key function."""