Compare commits

..

2 Commits

Author SHA1 Message Date
openhands d67c4c5848 Fix terminal command echo issue (#6661) and update tests 2025-03-14 16:33:13 +00:00
openhands 36b378c561 Fix terminal command echo issue (#6661) 2025-03-14 16:10:41 +00:00
4 changed files with 51 additions and 13 deletions
+28 -8
View File
@@ -72,8 +72,10 @@ describe("useTerminal", () => {
wrapper: Wrapper,
});
expect(mockTerminal.writeln).toHaveBeenNthCalledWith(1, "echo hello");
expect(mockTerminal.writeln).toHaveBeenNthCalledWith(2, "hello");
// Input commands should be displayed
expect(mockTerminal.writeln).toHaveBeenCalledWith("echo hello");
// Output commands should be displayed
expect(mockTerminal.writeln).toHaveBeenCalledWith("hello");
});
it("should hide secrets in the terminal", () => {
@@ -97,13 +99,31 @@ describe("useTerminal", () => {
},
);
// BUG: `vi.clearAllMocks()` does not clear the number of calls
// therefore, we need to assume the order of the calls based
// on the test order
expect(mockTerminal.writeln).toHaveBeenNthCalledWith(
3,
// Input command should be displayed with secrets masked
expect(mockTerminal.writeln).toHaveBeenCalledWith(
`export GITHUB_TOKEN=${"*".repeat(10)},${"*".repeat(10)},${"*".repeat(10)}`,
);
expect(mockTerminal.writeln).toHaveBeenNthCalledWith(4, "*".repeat(10));
// Output command should be displayed with secrets masked
expect(mockTerminal.writeln).toHaveBeenCalledWith("*".repeat(10));
});
it("should prevent duplicate command display", () => {
const inputCommand = "ls -la";
const commands: Command[] = [
{ content: inputCommand, type: "input" },
{ content: `${inputCommand}\nfile1.txt\nfile2.txt`, type: "output" },
];
render(<TestTerminalComponent commands={commands} secrets={[]} />, {
wrapper: Wrapper,
});
// Input command should be displayed
expect(mockTerminal.writeln).toHaveBeenCalledWith(inputCommand);
// Output should not be displayed since it starts with the input command
// This prevents the duplicate display of the command
expect(mockTerminal.writeln).not.toHaveBeenCalledWith(`${inputCommand}\nfile1.txt\nfile2.txt`);
});
});
+21 -3
View File
@@ -86,6 +86,8 @@ export const useTerminal = ({
const handleEnter = (command: string) => {
terminal.current?.write("\r\n");
// Send the command to the backend but don't echo it back in the terminal
// The backend will include the command in its response
send(getTerminalCommand(command));
};
@@ -131,10 +133,26 @@ export const useTerminal = ({
content = content.replaceAll(secret, "*".repeat(10));
});
terminal.current?.writeln(
parseTerminalOutput(content.replaceAll("\n", "\r\n").trim()),
);
// Check if this is an output that starts with the previous input command
// This happens when the backend echoes back the command in the output
let shouldDisplayContent = true;
if (type === "output" && i > 0 && commands[i - 1].type === "input") {
const prevInputCommand = commands[i - 1].content.trim();
// If the output starts with the input command, remove it to avoid duplication
if (content.trim().startsWith(prevInputCommand)) {
// Skip displaying this part as it's a duplicate of the user's input
// that's already shown in the terminal
shouldDisplayContent = false;
}
}
if (shouldDisplayContent) {
terminal.current?.writeln(
parseTerminalOutput(content.replaceAll("\n", "\r\n").trim()),
);
}
// Add a new prompt after the output
if (type === "output") {
terminal.current.write(`\n$ `);
}
@@ -1,6 +1,6 @@
{% if repository_info %}
<REPOSITORY_INFO>
At the user's request, repository {{ repository_info.repo_name }} has been cloned to the current working directory {{ repository_info.repo_directory }}.
At the user's request, repository {{ repository_info.repo_name }} has been cloned to directory {{ repository_info.repo_directory }}.
</REPOSITORY_INFO>
{% endif %}
{% if repository_instructions -%}
+1 -1
View File
@@ -117,7 +117,7 @@ def test_prompt_manager_template_rendering(prompt_dir):
msg_content: str = initial_msg.content[0].text
assert '<REPOSITORY_INFO>' in msg_content
assert (
"At the user's request, repository owner/repo has been cloned to the current working directory /workspace/repo."
"At the user's request, repository owner/repo has been cloned to directory /workspace/repo."
in msg_content
)
assert '</REPOSITORY_INFO>' in msg_content