Skip to content

Commit 1132247

Browse files
committed
fix(agent-framework): detect shell completion marker on output without trailing newline
When a command's stdout has no trailing newline (e.g. cat of a file without a final newline, or printf without \n), the line reader merges the trailing output and the completion marker into a single line, so ShellSession#collectOutput never matched the marker via startsWith and hung until the command timed out. Detect the marker anywhere in the line via indexOf, preserve any real output preceding it, and parse the exit code after it. Closes #4740
1 parent a376705 commit 1132247

2 files changed

Lines changed: 143 additions & 10 deletions

File tree

spring-ai-alibaba-agent-framework/src/main/java/com/alibaba/cloud/ai/graph/agent/tools/ShellSessionManager.java

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -386,18 +386,33 @@ private CommandResult collectOutput(String marker, long deadline, int maxOutputL
386386
}
387387

388388
String line = outputLine.content;
389-
390-
// Check for completion marker (only in stdout)
391-
if ("stdout".equals(outputLine.source) && line.startsWith(marker)) {
392-
String[] parts = line.split(" ", 2);
393-
if (parts.length > 1) {
394-
try {
395-
exitCode = Integer.parseInt(parts[1].trim());
396-
} catch (NumberFormatException e) {
397-
// Ignore
389+
boolean completed = false;
390+
391+
// Detect the completion marker (only in stdout). The marker is emitted by a
392+
// follow-up echo command, so it usually arrives on its own line. But when the
393+
// command's output has no trailing newline (e.g. `cat` of a file without a final
394+
// newline, or `printf` without `\n`), the line reader merges that trailing output
395+
// and the marker into a single line. Searching for the marker anywhere in the line
396+
// keeps completion detection working, and any real output preceding the marker is
397+
// preserved. See #4740.
398+
if ("stdout".equals(outputLine.source)) {
399+
int markerIndex = line.indexOf(marker);
400+
if (markerIndex >= 0) {
401+
String afterMarker = line.substring(markerIndex + marker.length()).trim();
402+
if (!afterMarker.isEmpty()) {
403+
try {
404+
exitCode = Integer.parseInt(afterMarker.split("\\s+")[0]);
405+
} catch (NumberFormatException e) {
406+
// Ignore
407+
}
408+
}
409+
// Keep any real output that was merged onto the marker line.
410+
line = line.substring(0, markerIndex);
411+
completed = true;
412+
if (line.isEmpty()) {
413+
break;
398414
}
399415
}
400-
break;
401416
}
402417

403418
totalLines++;
@@ -420,6 +435,10 @@ private CommandResult collectOutput(String marker, long deadline, int maxOutputL
420435
truncatedByLines = true;
421436
}
422437

438+
if (completed) {
439+
break;
440+
}
441+
423442
} catch (InterruptedException e) {
424443
Thread.currentThread().interrupt();
425444
break;
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/*
2+
* Copyright 2025-2026 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.alibaba.cloud.ai.graph.agent.tools;
17+
18+
import com.alibaba.cloud.ai.graph.RunnableConfig;
19+
20+
import org.junit.jupiter.api.Test;
21+
import org.junit.jupiter.api.Timeout;
22+
import org.junit.jupiter.api.condition.DisabledOnOs;
23+
import org.junit.jupiter.api.condition.OS;
24+
25+
import java.nio.file.Files;
26+
import java.nio.file.Path;
27+
import java.util.concurrent.TimeUnit;
28+
29+
import static org.junit.jupiter.api.Assertions.assertEquals;
30+
import static org.junit.jupiter.api.Assertions.assertFalse;
31+
32+
/**
33+
* Tests for {@link ShellSessionManager} command output collection.
34+
*
35+
* <p>
36+
* Reproduces #4740: a command whose output does not end with a newline (e.g. {@code cat}
37+
* of a file without a trailing newline) caused the completion marker to be merged onto the
38+
* last output line, so it was never detected and {@code collectOutput} hung until the
39+
* command timed out.
40+
*/
41+
@DisabledOnOs(OS.WINDOWS)
42+
class ShellSessionManagerTest {
43+
44+
private ShellSessionManager newManager(Path workspace) {
45+
return ShellSessionManager.builder()
46+
.workspaceRoot(workspace)
47+
.commandTimeout(10000)
48+
.build();
49+
}
50+
51+
@Test
52+
@Timeout(value = 20, unit = TimeUnit.SECONDS)
53+
void outputWithoutTrailingNewlineIsCapturedAndDoesNotHang() throws Exception {
54+
Path workspace = Files.createTempDirectory("shell_session_test_");
55+
ShellSessionManager manager = newManager(workspace);
56+
RunnableConfig config = RunnableConfig.builder().threadId("test-thread").build();
57+
manager.initialize(config);
58+
try {
59+
// printf without \n leaves no trailing newline, so the marker is merged onto
60+
// this line. Before the fix this hung until the command timed out.
61+
ShellSessionManager.CommandResult result = manager
62+
.executeCommand("printf 'hello-no-newline'", config);
63+
64+
assertFalse(result.isTimedOut(), "command should not time out");
65+
assertEquals("hello-no-newline", result.getOutput());
66+
assertEquals(0, result.getExitCode());
67+
}
68+
finally {
69+
manager.cleanup(config);
70+
}
71+
}
72+
73+
@Test
74+
@Timeout(value = 20, unit = TimeUnit.SECONDS)
75+
void exitCodeIsParsedWhenMergedWithOutput() throws Exception {
76+
Path workspace = Files.createTempDirectory("shell_session_test_");
77+
ShellSessionManager manager = newManager(workspace);
78+
RunnableConfig config = RunnableConfig.builder().threadId("test-thread").build();
79+
manager.initialize(config);
80+
try {
81+
// Output has no trailing newline AND the command fails: the non-zero exit code
82+
// must still be parsed from the line the marker was merged onto.
83+
ShellSessionManager.CommandResult result = manager
84+
.executeCommand("printf 'oops' && false", config);
85+
86+
assertFalse(result.isTimedOut(), "command should not time out");
87+
assertEquals("oops", result.getOutput());
88+
assertEquals(1, result.getExitCode());
89+
}
90+
finally {
91+
manager.cleanup(config);
92+
}
93+
}
94+
95+
@Test
96+
@Timeout(value = 20, unit = TimeUnit.SECONDS)
97+
void normalOutputWithTrailingNewlineStillWorks() throws Exception {
98+
Path workspace = Files.createTempDirectory("shell_session_test_");
99+
ShellSessionManager manager = newManager(workspace);
100+
RunnableConfig config = RunnableConfig.builder().threadId("test-thread").build();
101+
manager.initialize(config);
102+
try {
103+
ShellSessionManager.CommandResult result = manager.executeCommand("printf 'a\\nb\\n'", config);
104+
105+
assertFalse(result.isTimedOut(), "command should not time out");
106+
assertEquals("a\nb", result.getOutput());
107+
assertEquals(0, result.getExitCode());
108+
}
109+
finally {
110+
manager.cleanup(config);
111+
}
112+
}
113+
114+
}

0 commit comments

Comments
 (0)