Add edit text support to stirling engine (#6245)

# Description of Changes Hooks up the (alpha) PDF Editor backend to the AI engine Edit Agent via an intermediary API which is easier for the agent to call. It suffers from all the same issues that the PDF Editor does in actually editing the text, but should also benefit from any fixes to that. It also adds protection against the underlying tools misbehaving by hanging, and fixes a hanging bug in the PDF Editor. --------- Co-authored-by: EthanHealy01 <[email protected]>
2026-07-14 10:34:06 +02:00 · 2026-05-11 09:57:41 +00:00
parent 294b616a63
commit 575684ee4b
17 changed files with 1937 additions and 52 deletions
@@ -76,6 +76,7 @@ public class ApplicationProperties {
    private ProcessExecutor processExecutor = new ProcessExecutor();
    private PdfEditor pdfEditor = new PdfEditor();
    private AiEngine aiEngine = new AiEngine();
+    private InternalApi internalApi = new InternalApi();

    @Bean
    public PropertySource<?> dynamicYamlPropertySource(ConfigurableEnvironment environment)
@@ -246,6 +247,19 @@ public class ApplicationProperties {
        private int longRunningTimeoutSeconds = 600;
    }

+    /**
+     * HTTP timeouts for loopback calls to internal Stirling API endpoints, used by the AI workflow
+     * executor and the pipeline processor. A bounded read timeout prevents a hung tool (e.g. an
+     * infinite loop in a PDF processing service) from stalling the entire chat workflow forever.
+     * Tools that legitimately need longer than the read timeout should be invoked through the async
+     * job executor instead of synchronously.
+     */
+    @Data
+    public static class InternalApi {
+        private int connectTimeoutSeconds = 10;
+        private int readTimeoutSeconds = 300;
+    }
+
    @Data
    public static class Legal {
        private String termsAndConditions;
@@ -0,0 +1,19 @@
+package stirling.software.common.model.api.general;
+
+import io.swagger.v3.oas.annotations.media.Schema;
+
+import lombok.Data;
+import lombok.EqualsAndHashCode;
+
+@Data
+@EqualsAndHashCode
+public class EditTextOperation {
+
+    @Schema(description = "The literal text to find.", requiredMode = Schema.RequiredMode.REQUIRED)
+    private String find;
+
+    @Schema(
+            description = "The replacement text. May be empty to delete the matched text.",
+            requiredMode = Schema.RequiredMode.REQUIRED)
+    private String replace;
+}
@@ -5,6 +5,7 @@ import java.io.UncheckedIOException;
 import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.time.Duration;
 import java.util.regex.Pattern;

 import org.springframework.beans.factory.annotation.Autowired;
@@ -12,15 +13,18 @@ import org.springframework.core.env.Environment;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.core.io.Resource;
 import org.springframework.http.*;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
 import org.springframework.stereotype.Service;
 import org.springframework.util.MultiValueMap;
 import org.springframework.web.client.RequestCallback;
+import org.springframework.web.client.ResourceAccessException;
 import org.springframework.web.client.RestTemplate;

 import jakarta.servlet.ServletContext;

 import lombok.extern.slf4j.Slf4j;

+import stirling.software.common.model.ApplicationProperties;
 import stirling.software.common.model.enumeration.Role;
 import stirling.software.common.util.TempFile;
 import stirling.software.common.util.TempFileManager;
@@ -50,16 +54,29 @@ public class InternalApiClient {
    private final UserServiceInterface userService;
    private final TempFileManager tempFileManager;
    private final Environment environment;
+    private final Duration readTimeout;
+    private final RestTemplate restTemplate;

    public InternalApiClient(
            ServletContext servletContext,
            @Autowired(required = false) UserServiceInterface userService,
            TempFileManager tempFileManager,
-            Environment environment) {
+            Environment environment,
+            ApplicationProperties applicationProperties) {
        this.servletContext = servletContext;
        this.userService = userService;
        this.tempFileManager = tempFileManager;
        this.environment = environment;
+        ApplicationProperties.InternalApi internalApi = applicationProperties.getInternalApi();
+        // A bounded read timeout is what protects the workflow when an internal tool hangs
+        // (e.g. an infinite loop in a PDF processing service). The connect timeout is short
+        // because this is a loopback call; if connecting takes longer than a few seconds the
+        // local server is itself unhealthy.
+        this.readTimeout = Duration.ofSeconds(internalApi.getReadTimeoutSeconds());
+        SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory();
+        factory.setConnectTimeout(Duration.ofSeconds(internalApi.getConnectTimeoutSeconds()));
+        factory.setReadTimeout(readTimeout);
+        this.restTemplate = new RestTemplate(factory);
    }

    /**
@@ -74,7 +91,6 @@ public class InternalApiClient {
        validateUrl(endpointPath);
        String url = getBaseUrl() + endpointPath;

-        RestTemplate restTemplate = new RestTemplate();
        HttpHeaders headers = new HttpHeaders();
        String apiKey = getApiKeyForUser();
        if (apiKey != null && !apiKey.isEmpty()) {
@@ -84,26 +100,38 @@ public class InternalApiClient {
        HttpEntity<MultiValueMap<String, Object>> entity = new HttpEntity<>(body, headers);
        RequestCallback requestCallback = restTemplate.httpEntityCallback(entity, Resource.class);

-        return restTemplate.execute(
-                url,
-                HttpMethod.POST,
-                requestCallback,
-                response -> {
-                    try {
-                        TempFile tempFile = tempFileManager.createManagedTempFile("internal-api");
-                        Files.copy(
-                                response.getBody(),
-                                tempFile.getPath(),
-                                java.nio.file.StandardCopyOption.REPLACE_EXISTING);
-                        String filename = extractFilename(response.getHeaders());
-                        TempFileResource resource = new TempFileResource(tempFile, filename);
-                        return ResponseEntity.status(response.getStatusCode())
-                                .headers(response.getHeaders())
-                                .body(resource);
-                    } catch (IOException e) {
-                        throw new UncheckedIOException(e);
-                    }
-                });
+        try {
+            return restTemplate.execute(
+                    url,
+                    HttpMethod.POST,
+                    requestCallback,
+                    response -> {
+                        try {
+                            TempFile tempFile =
+                                    tempFileManager.createManagedTempFile("internal-api");
+                            Files.copy(
+                                    response.getBody(),
+                                    tempFile.getPath(),
+                                    java.nio.file.StandardCopyOption.REPLACE_EXISTING);
+                            String filename = extractFilename(response.getHeaders());
+                            TempFileResource resource = new TempFileResource(tempFile, filename);
+                            return ResponseEntity.status(response.getStatusCode())
+                                    .headers(response.getHeaders())
+                                    .body(resource);
+                        } catch (IOException e) {
+                            throw new UncheckedIOException(e);
+                        }
+                    });
+        } catch (ResourceAccessException e) {
+            // RestTemplate wraps low-level I/O failures in ResourceAccessException. Only the
+            // SocketTimeoutException-rooted case is a real timeout; other I/O failures (connection
+            // refused, DNS, etc.) propagate as-is so the upstream generic handler can describe
+            // them accurately.
+            if (e.getCause() instanceof java.net.SocketTimeoutException) {
+                throw new InternalApiTimeoutException(endpointPath, readTimeout, e);
+            }
+            throw e;
+        }
    }

    /**
@@ -0,0 +1,36 @@
+package stirling.software.common.service;
+
+import java.time.Duration;
+
+/**
+ * Thrown when an internal Stirling tool invocation exceeds its configured read timeout or otherwise
+ * fails at the transport layer. Distinguishes a hung/timed-out tool from a tool that returned a
+ * non-2xx HTTP response (which is reported via the response status itself), so callers can present
+ * a clear "the tool didn't respond in time" message to the user instead of a generic stack trace.
+ */
+public class InternalApiTimeoutException extends RuntimeException {
+
+    private final String endpointPath;
+    private final Duration readTimeout;
+
+    public InternalApiTimeoutException(String endpointPath, Duration readTimeout, Throwable cause) {
+        super(buildMessage(endpointPath, readTimeout, cause), cause);
+        this.endpointPath = endpointPath;
+        this.readTimeout = readTimeout;
+    }
+
+    public String getEndpointPath() {
+        return endpointPath;
+    }
+
+    public Duration getReadTimeout() {
+        return readTimeout;
+    }
+
+    private static String buildMessage(String endpointPath, Duration readTimeout, Throwable cause) {
+        String reason = cause != null && cause.getMessage() != null ? cause.getMessage() : "";
+        return String.format(
+                "Internal tool %s did not respond within %ds (%s)",
+                endpointPath, readTimeout.toSeconds(), reason);
+    }
+}
@@ -6,15 +6,18 @@ import java.util.List;

 import lombok.extern.slf4j.Slf4j;

-import stirling.software.common.model.api.security.RedactionArea;
-
-import tools.jackson.core.type.TypeReference;
 import tools.jackson.databind.DeserializationFeature;
+import tools.jackson.databind.JavaType;
 import tools.jackson.databind.ObjectMapper;
 import tools.jackson.databind.json.JsonMapper;

+/**
+ * Spring property editor that decodes a JSON string into a typed {@link ArrayList}. Used to bind
+ * complex list parameters (e.g. {@code List<RedactionArea>}, {@code List<EditTextOperation>}) from
+ * multipart form fields, where Spring's default binding cannot deserialize a JSON array.
+ */
@Slf4j
-public class StringToArrayListPropertyEditor extends PropertyEditorSupport {
+public class StringToArrayListPropertyEditor<T> extends PropertyEditorSupport {

    private final ObjectMapper objectMapper =
            JsonMapper.builder()
@@ -22,6 +25,12 @@ public class StringToArrayListPropertyEditor extends PropertyEditorSupport {
                    .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
                    .build();

+    private final Class<T> elementType;
+
+    public StringToArrayListPropertyEditor(Class<T> elementType) {
+        this.elementType = elementType;
+    }
+
    @Override
    public void setAsText(String text) throws IllegalArgumentException {
        if (text == null || text.trim().isEmpty()) {
@@ -29,8 +38,11 @@ public class StringToArrayListPropertyEditor extends PropertyEditorSupport {
            return;
        }
        try {
-            TypeReference<ArrayList<RedactionArea>> typeRef = new TypeReference<>() {};
-            List<RedactionArea> list = objectMapper.readValue(text, typeRef);
+            JavaType listType =
+                    objectMapper
+                            .getTypeFactory()
+                            .constructCollectionType(ArrayList.class, elementType);
+            List<T> list = objectMapper.readValue(text, listType);
            setValue(list);
        } catch (Exception e) {
            log.error("Exception while converting {}", e);
@@ -22,11 +22,13 @@ import org.springframework.mock.env.MockEnvironment;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 import org.springframework.web.client.RequestCallback;
+import org.springframework.web.client.ResourceAccessException;
 import org.springframework.web.client.ResponseExtractor;
 import org.springframework.web.client.RestTemplate;

 import jakarta.servlet.ServletContext;

+import stirling.software.common.model.ApplicationProperties;
 import stirling.software.common.util.TempFile;
 import stirling.software.common.util.TempFileManager;

@@ -42,8 +44,19 @@ class InternalApiClientTest {
    @BeforeEach
    void setUp() {
        lenient().when(servletContext.getContextPath()).thenReturn("");
+        client = newClient();
+    }
+
+    /**
+     * Build a fresh client. Tests that use {@link org.mockito.Mockito#mockConstruction} to
+     * intercept {@link RestTemplate} must call this from inside their {@code mockConstruction}
+     * block, since the client now caches one RestTemplate per instance at construction time.
+     */
+    private InternalApiClient newClient() {
        MockEnvironment environment = new MockEnvironment().withProperty("server.port", "8080");
-        client = new InternalApiClient(servletContext, userService, tempFileManager, environment);
+        ApplicationProperties applicationProperties = new ApplicationProperties();
+        return new InternalApiClient(
+                servletContext, userService, tempFileManager, environment, applicationProperties);
    }

    @Test
@@ -75,7 +88,10 @@ class InternalApiClientTest {
                                    .thenAnswer(inv -> fakeOkResponse(inv.getArgument(3)));
                        })) {

-            ResponseEntity<Resource> response = client.post("/api/v1/general/merge-pdfs", body);
+            // Reconstruct the client so its cached RestTemplate is the mocked one.
+            InternalApiClient mockedClient = newClient();
+            ResponseEntity<Resource> response =
+                    mockedClient.post("/api/v1/general/merge-pdfs", body);

            assertNotNull(response);
            assertEquals(HttpStatus.OK, response.getStatusCode());
@@ -86,6 +102,72 @@ class InternalApiClientTest {
        }
    }

+    @Test
+    void postWrapsSocketTimeoutAsInternalApiTimeoutException() {
+        // Simulates the read-timeout case: RestTemplate wraps SocketTimeoutException in
+        // ResourceAccessException when the underlying socket times out. The client must repackage
+        // that into a typed timeout exception that carries the failing endpoint and the configured
+        // read timeout, so the workflow layer can surface a clean "tool didn't respond" message
+        // to the user.
+        MultiValueMap<String, Object> body = new LinkedMultiValueMap<>();
+        body.add("fileInput", namedResource("input.pdf", "data"));
+
+        try (var ignored =
+                mockConstruction(
+                        RestTemplate.class,
+                        (rt, ctx) -> {
+                            when(rt.httpEntityCallback(any(), eq(Resource.class)))
+                                    .thenAnswer(inv -> (RequestCallback) req -> {});
+                            when(rt.execute(anyString(), eq(HttpMethod.POST), any(), any()))
+                                    .thenThrow(
+                                            new ResourceAccessException(
+                                                    "I/O error on POST request: Read timed out",
+                                                    new java.net.SocketTimeoutException(
+                                                            "Read timed out")));
+                        })) {
+
+            InternalApiClient mockedClient = newClient();
+            InternalApiTimeoutException thrown =
+                    assertThrows(
+                            InternalApiTimeoutException.class,
+                            () -> mockedClient.post("/api/v1/general/merge-pdfs", body));
+
+            assertEquals("/api/v1/general/merge-pdfs", thrown.getEndpointPath());
+            assertNotNull(thrown.getReadTimeout());
+            assertTrue(thrown.getMessage().contains("/api/v1/general/merge-pdfs"));
+        }
+    }
+
+    @Test
+    void postRethrowsNonTimeoutResourceAccessExceptionAsIs() {
+        // ResourceAccessException covers more than just timeouts (e.g. connection refused, DNS
+        // failure). Only SocketTimeoutException-rooted failures are timeouts; everything else
+        // must propagate so the upstream generic handler can label it correctly instead of lying
+        // about a "tool didn't respond" timeout.
+        MultiValueMap<String, Object> body = new LinkedMultiValueMap<>();
+        body.add("fileInput", namedResource("input.pdf", "data"));
+
+        try (var ignored =
+                mockConstruction(
+                        RestTemplate.class,
+                        (rt, ctx) -> {
+                            when(rt.httpEntityCallback(any(), eq(Resource.class)))
+                                    .thenAnswer(inv -> (RequestCallback) req -> {});
+                            when(rt.execute(anyString(), eq(HttpMethod.POST), any(), any()))
+                                    .thenThrow(
+                                            new ResourceAccessException(
+                                                    "I/O error on POST request: Connection refused",
+                                                    new java.net.ConnectException(
+                                                            "Connection refused")));
+                        })) {
+
+            InternalApiClient mockedClient = newClient();
+            assertThrows(
+                    ResourceAccessException.class,
+                    () -> mockedClient.post("/api/v1/general/merge-pdfs", body));
+        }
+    }
+
    @Test
    void postRejectsDisallowedPath() {
        MultiValueMap<String, Object> body = new LinkedMultiValueMap<>();
@@ -125,8 +207,10 @@ class InternalApiClientTest {
                                    .thenAnswer(inv -> fakeOkResponse(inv.getArgument(3)));
                        })) {

+            // Reconstruct the client so its cached RestTemplate is the mocked one.
+            InternalApiClient mockedClient = newClient();
            ResponseEntity<Resource> response =
-                    client.post("/api/v1/ai/tools/pdf-comment-agent", body);
+                    mockedClient.post("/api/v1/ai/tools/pdf-comment-agent", body);

            assertNotNull(response);
            assertEquals(HttpStatus.OK, response.getStatusCode());
@@ -11,11 +11,11 @@ import stirling.software.common.model.api.security.RedactionArea;

 class StringToArrayListPropertyEditorTest {

-    private StringToArrayListPropertyEditor editor;
+    private StringToArrayListPropertyEditor<RedactionArea> editor;

    @BeforeEach
    void setUp() {
-        editor = new StringToArrayListPropertyEditor();
+        editor = new StringToArrayListPropertyEditor<>(RedactionArea.class);
    }

    @Test