Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ public void stop() {
if (sessionHistoryService != null) {
sessionHistoryService.stop();
}
if(sandboxService != null){
sandboxService.stop();
}
Comment on lines +145 to +147

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Calling sandboxService.stop() will not shut down the scheduled executor service for cleaning up expired sandboxes, which can lead to a resource leak. The close() method in SandboxService is designed for a complete shutdown, including the executor. You should call sandboxService.close() to ensure all resources are properly released.

Additionally, for coding style consistency, please add a space after if.

Suggested change
if(sandboxService != null){
sandboxService.stop();
}
if (sandboxService != null) {
sandboxService.close();
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
* </ul>
*/
public class MyAgentScopeAgentHandler extends AgentScopeAgentHandler {
private SandboxService sandboxService;
private static final Logger logger = LoggerFactory.getLogger(MyAgentScopeAgentHandler.class);
private final String apiKey;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public BrowserSandbox(
* @throws RuntimeException if sandbox is not healthy
*/
public String getDesktopUrl() {
return GuiMixin.getDesktopUrl(managerApi, sandboxId, baseUrl);
return GuiMixin.getDesktopUrl(managerApi, this, baseUrl);
}

public String navigate(String url) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public FilesystemSandbox(
* @throws RuntimeException if sandbox is not healthy
*/
public String getDesktopUrl() {
return GuiMixin.getDesktopUrl(managerApi, sandboxId, baseUrl);
return GuiMixin.getDesktopUrl(managerApi, this, baseUrl);
}

public String readFile(String path) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,27 @@ public class GuiMixin {
* SandboxService and sandboxId.
*
* @param managerApi The SandboxService instance
* @param sandboxId The sandbox ID
* @param sandbox The sandbox instance
* @param baseUrl Optional base URL (can be null)
* @return The desktop URL for VNC access
* @throws RuntimeException if sandbox is not healthy or info cannot be retrieved
*/
public static String getDesktopUrl(SandboxService managerApi, String sandboxId, String baseUrl) {
public static String getDesktopUrl(SandboxService managerApi, Sandbox sandbox, String baseUrl) {
// Check if sandbox is healthy by attempting to get info
ContainerModel info;
try {
info = managerApi.getInfo(sandboxId);
info = managerApi.getInfo(sandbox);
} catch (Exception e) {
throw new RuntimeException("Sandbox " + sandboxId + " is not healthy: " + e.getMessage(), e);
throw new RuntimeException("Sandbox " + sandbox.getSandboxId() + " is not healthy: " + e.getMessage(), e);
}

if (info == null) {
throw new RuntimeException("Sandbox " + sandboxId + " is not healthy: cannot retrieve info");
throw new RuntimeException("Sandbox " + sandbox.getSandboxId() + " is not healthy: cannot retrieve info");
}

String runtimeToken = info.getRuntimeToken();
if (runtimeToken == null || runtimeToken.isEmpty()) {
throw new RuntimeException("Sandbox " + sandboxId + " does not have a runtime token");
throw new RuntimeException("Sandbox " + sandbox.getSandboxId() + " does not have a runtime token");
}

String path = "/vnc/vnc_lite.html";
Expand All @@ -68,7 +68,7 @@ public static String getDesktopUrl(SandboxService managerApi, String sandboxId,
// Use direct URL from container info
String containerUrl = info.getBaseUrl();
if (containerUrl == null || containerUrl.isEmpty()) {
throw new RuntimeException("Sandbox " + sandboxId + " does not have a base URL");
throw new RuntimeException("Sandbox " + sandbox.getSandboxId() + " does not have a base URL");
}
// Ensure URL ends with / if not present
if (!containerUrl.endsWith("/")) {
Expand All @@ -81,7 +81,7 @@ public static String getDesktopUrl(SandboxService managerApi, String sandboxId,
} else {
// Use base_url with sandbox ID
String base = baseUrl.endsWith("/") ? baseUrl.substring(0, baseUrl.length() - 1) : baseUrl;
return base + "/desktop/" + sandboxId + remotePath + "?" + params;
return base + "/desktop/" + sandbox.getSandboxId() + remotePath + "?" + params;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public GuiSandbox(
* @throws RuntimeException if sandbox is not healthy
*/
public String getDesktopUrl() {
return GuiMixin.getDesktopUrl(managerApi, sandboxId, baseUrl);
return GuiMixin.getDesktopUrl(managerApi, this, baseUrl);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ public Sandbox(
this.environment = new HashMap<>(environment);
}

public void setSandboxId(String sandboxId) {
this.sandboxId = sandboxId;
}

public String getSandboxId() {
return sandboxId;
}
Expand All @@ -124,7 +128,7 @@ public Map<String, String> getEnvironment() {
return environment;
}

public FileSystemConfig getFileSystemStarter() {
public FileSystemConfig getFileSystemConfig() {
return fileSystemConfig;
}

Expand All @@ -149,7 +153,13 @@ private void initializeSandbox(){
@JsonIgnore
public ContainerModel getInfo() {
initializeSandbox();
return managerApi.getInfo(sandboxId);
try {
return managerApi.getInfo(this);
}
catch (Exception e) {
logger.error("Failed to get sandbox info: {}", e.getMessage());
throw new RuntimeException("Failed to get sandbox info", e);
}
}

public Map<String, Object> listTools() {
Expand All @@ -158,12 +168,24 @@ public Map<String, Object> listTools() {

public Map<String, Object> listTools(String toolType) {
initializeSandbox();
return managerApi.listTools(sandboxId, toolType);
try{
return managerApi.listTools(this, toolType);
}
catch (Exception e) {
logger.error("Failed to list tools: {}", e.getMessage());
throw new RuntimeException("Failed to list tools", e);
}
}

public String callTool(String name, Map<String, Object> arguments) {
initializeSandbox();
return managerApi.callTool(sandboxId, name, arguments);
try{
return managerApi.callTool(this, name, arguments);
}
catch (Exception e) {
logger.error("Failed to call tool {}: {}", name, e.getMessage());
throw new RuntimeException("Failed to call tool " + name, e);
}
}

public Map<String, Object> addMcpServers(Map<String, Object> serverConfigs) {
Expand All @@ -172,7 +194,13 @@ public Map<String, Object> addMcpServers(Map<String, Object> serverConfigs) {

public Map<String, Object> addMcpServers(Map<String, Object> serverConfigs, boolean overwrite) {
initializeSandbox();
return managerApi.addMcpServers(sandboxId, serverConfigs, overwrite);
try{
return managerApi.addMcpServers(this, serverConfigs, overwrite);
}
catch (Exception e) {
logger.error("Failed to add MCP servers: {}", e.getMessage());
throw new RuntimeException("Failed to add MCP servers", e);
}
}

@Override
Expand Down
Loading
Loading