Skip to content

Fix #863: add ProcessRunner utility to capabilities-common #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
orpiske merged 1 commit into from
Mar 3, 2026
Merged

Fix #863: add ProcessRunner utility to capabilities-common #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
102 changes: 102 additions & 0 deletions capabilities-common/src/main/java/ai/wanaku/capabilities/sdk/common/ProcessRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package ai.wanaku.capabilities.sdk.common;

import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.Map;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import ai.wanaku.capabilities.sdk.api.exceptions.WanakuException;

/**
* Convenience utility for running external processes. This is intended for use by capabilities and CLI code.
* It MUST NOT be used in router code.
*/
public class ProcessRunner {
private static final Logger LOG = LoggerFactory.getLogger(ProcessRunner.class);

private ProcessRunner() {}

public static String runWithOutput(String... command) {
try {
ProcessBuilder processBuilder = new ProcessBuilder(command);
// Redirect output and error streams to a pipe
processBuilder.redirectOutput(ProcessBuilder.Redirect.PIPE);
processBuilder.redirectError(ProcessBuilder.Redirect.PIPE);
Comment on lines +24 to +26
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot Mar 3, 2026

Choose a reason for hiding this comment

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

issue (bug_risk): Reading only stdout while piping stderr risks deadlock for verbose error output.

In runWithOutput, both stdout and stderr are piped but only stdout is read. If the subprocess writes heavily to stderr, its buffer can fill and block the process so waitFor() never returns. Consider either using redirectErrorStream(true) and reading a single stream, or consuming stderr concurrently (e.g., separate thread) / redirecting it to inheritIO or logging instead of a pipe.


final Process process = processBuilder.start();
LOG.info("Waiting for process to finish...");

String output = readOutput(process);

waitForExit(process);
return output;
} catch (IOException e) {
LOG.error("I/O Error: {}", e.getMessage(), e);
throw new WanakuException(e);
} catch (InterruptedException e) {
LOG.error("Interrupted: {}", e.getMessage(), e);
throw new WanakuException(e);
}
}

private static String readOutput(Process process) throws IOException {
// Read the output from the process
BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
StringBuilder output = new StringBuilder();
String line;
while ((line = reader.readLine()) != null) {
Comment on lines +44 to +49
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot Mar 3, 2026

Choose a reason for hiding this comment

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

suggestion (bug_risk): Close the BufferedReader/InputStream via try-with-resources to avoid leaking OS resources.

The BufferedReader (and underlying InputStream) remains open for the lifetime of the process, which can cause resource leaks in long-running or repeated executions. Wrap the reader in a try-with-resources (or close it in a finally block) so it’s always released:

try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()))) {
    StringBuilder output = new StringBuilder();
    String line;
    while ((line = reader.readLine()) != null) {
        output.append(line).append("\n");
    }
    return output.toString();
}

output.append(line).append("\n");
}
return output.toString();
}

public static void run(File directory, String... command) {
run(directory, null, command);
}

public static void run(File directory, Map<String, String> environmentVariables, String... command) {
try {
LOG.info("About to run command: {}", String.join(" ", command));
ProcessBuilder processBuilder = new ProcessBuilder();
processBuilder.command(command).inheritIO().directory(directory);

if (environmentVariables != null) {
processBuilder.environment().putAll(environmentVariables);
}

final Process process = processBuilder.start();

LOG.info("Waiting for process to finish...");
waitForExit(process);
} catch (IOException e) {
LOG.error("I/O Error: {}", e.getMessage(), e);
throw new WanakuException(e);
} catch (InterruptedException e) {
LOG.error("Interrupted: {}", e.getMessage(), e);
throw new WanakuException(e);
}
}

private static void waitForExit(Process process) throws InterruptedException {
Thread thread = new Thread(() -> {
if (process.isAlive()) {
process.destroy();
}
});

try {
Runtime.getRuntime().addShutdownHook(thread);

final int ret = process.waitFor();
if (ret != 0) {
LOG.warn("Process did not execute successfully: (return status {})", ret);
}
} finally {
if (!process.isAlive()) {
Runtime.getRuntime().removeShutdownHook(thread);
}
}
}
}
18 changes: 18 additions & 0 deletions capabilities-common/src/test/java/ai/wanaku/capabilities/sdk/common/ProcessRunnerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package ai.wanaku.capabilities.sdk.common;

import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class ProcessRunnerTest {

@Test
void runWithOutput_returnsCommandOutput() {
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot Mar 3, 2026

Choose a reason for hiding this comment

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

suggestion (testing): Add a test for runWithOutput when the process exits with a non-zero status

Because waitForExit only logs a warning on non-zero exits and doesn’t throw, please add a test that runs a command exiting with status 1 and asserts that runWithOutput does not throw and returns the expected output. This will lock in the intended behavior for failure cases and prevent regressions.

String output = ProcessRunner.runWithOutput("echo", "hello");

assertNotNull(output, "Output should not be null");
assertFalse(output.isEmpty(), "Output should not be empty");
assertTrue(output.contains("hello"), "Output should contain 'hello'");
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot Mar 3, 2026

Choose a reason for hiding this comment

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

suggestion (testing): Consider adding a test that covers stderr handling in runWithOutput

Currently we only cover a stdout-only command. Since runWithOutput pipes stderr but only reads stdout, please add a test where the child writes to both streams, and assert that it completes successfully and the returned value includes only the expected stdout content. This will make the intended stderr behavior explicit and protected by tests.

Suggested implementation:

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertFalse;

import org.junit.jupiter.api.Test;

public class ProcessRunnerTest {

    @Test
    void runWithOutput_returnsCommandOutput() {
        String output = ProcessRunner.runWithOutput("echo", "hello");

        assertNotNull(output, "Output should not be null");
        assertFalse(output.isEmpty(), "Output should not be empty");
        assertTrue(output.contains("hello"), "Output should contain 'hello'");
    }

    @Test
    void runWithOutput_ignoresStderrAndReturnsOnlyStdout() {
        String output = ProcessRunner.runWithOutput(
                "sh",
                "-c",
                "echo stdout && echo stderr 1>&2"
        );

        assertNotNull(output, "Output should not be null");
        assertFalse(output.isEmpty(), "Output should not be empty");
        assertTrue(output.contains("stdout"), "Output should contain only stdout content");
        assertFalse(output.contains("stderr"), "Output should not contain stderr content");
    }
}

If your test environment does not guarantee a POSIX-compatible sh shell (e.g., pure Windows without WSL or Git Bash), you will need to adapt the command invocation in runWithOutput_ignoresStderrAndReturnsOnlyStdout to a platform-appropriate shell or helper script that can write to both stdout and stderr.

}
Comment on lines +10 to +17
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot Mar 3, 2026

Choose a reason for hiding this comment

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

suggestion (testing): The test command may not be fully portable across platforms

Using echo as the command makes this test non-portable (e.g., on Windows echo is usually a shell built-in, not an executable). To keep the test reliable across CI environments, consider using a known binary available in the test environment (such as java) or choosing the command based on the detected OS.

Suggested change
@Test
void runWithOutput_returnsCommandOutput() {
String output = ProcessRunner.runWithOutput("echo", "hello");
assertNotNull(output, "Output should not be null");
assertFalse(output.isEmpty(), "Output should not be empty");
assertTrue(output.contains("hello"), "Output should contain 'hello'");
}
@Test
void runWithOutput_returnsCommandOutput() {
String osName = System.getProperty("os.name").toLowerCase();
String output;
if (osName.contains("win")) {
// On Windows, echo is a cmd.exe built-in
output = ProcessRunner.runWithOutput("cmd.exe", "/c", "echo hello");
} else {
// On Unix-like systems, echo is typically a shell built-in
output = ProcessRunner.runWithOutput("sh", "-c", "echo hello");
}
assertNotNull(output, "Output should not be null");
assertFalse(output.isEmpty(), "Output should not be empty");
assertTrue(output.contains("hello"), "Output should contain 'hello'");
}

}