-
Notifications
You must be signed in to change notification settings - Fork 192
fix(terminal): filter terminal query sequences from captured output #2334
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.
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
Open
jpshackelford
wants to merge
7
commits into
main
Choose a base branch
from
fix/terminal-escape-filter-minimal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
795c06c
fix(terminal): filter terminal query sequences from captured output
jpshackelford a499579
refactor: use general OSC query pattern instead of specific codes
jpshackelford c3756f2
chore: add real-world test script for PR review
jpshackelford e167891
Merge branch 'main' into fix/terminal-escape-filter-minimal
jpshackelford 07f9ca1
Merge branch 'main' into fix/terminal-escape-filter-minimal
jpshackelford 58df820
fix(terminal): make escape filter stateful to handle split sequences
openhands-agent 72e3f6f
Merge branch 'main' into fix/terminal-escape-filter-minimal
jpshackelford File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 changes: 14 additions & 0 deletions
14
openhands-tools/openhands/tools/terminal/utils/__init__.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| """Terminal tool utilities.""" | ||
|
|
||
| from openhands.tools.terminal.utils.command import ( | ||
| escape_bash_special_chars, | ||
| split_bash_commands, | ||
| ) | ||
| from openhands.tools.terminal.utils.escape_filter import filter_terminal_queries | ||
|
|
||
|
|
||
| __all__ = [ | ||
| "escape_bash_special_chars", | ||
| "split_bash_commands", | ||
| "filter_terminal_queries", | ||
| ] |
83 changes: 83 additions & 0 deletions
83
openhands-tools/openhands/tools/terminal/utils/escape_filter.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| """Filter terminal query sequences from captured output. | ||
|
|
||
| When CLI tools (like `gh`, `npm`, etc.) run inside a PTY, they may send | ||
| terminal query sequences as part of their progress/spinner UI. These queries | ||
| get captured as output. When displayed, the terminal processes them and | ||
| responds, causing visible escape code garbage. | ||
|
|
||
| This module provides filtering to remove these query sequences while | ||
| preserving legitimate formatting escape codes (colors, bold, etc.). | ||
|
|
||
| See: https://github.com/OpenHands/software-agent-sdk/issues/2244 | ||
| """ | ||
|
|
||
| import re | ||
|
|
||
|
|
||
| # Terminal query sequences that trigger responses (and cause visible garbage) | ||
| # These should be stripped from captured output before display. | ||
| # | ||
| # Reference: ECMA-48, XTerm Control Sequences | ||
| # https://invisible-island.net/xterm/ctlseqs/ctlseqs.html | ||
|
|
||
| # DSR (Device Status Report) - cursor position query | ||
| # Format: ESC [ 6 n -> Response: ESC [ row ; col R | ||
| _DSR_PATTERN = re.compile(rb"\x1b\[6n") | ||
|
|
||
| # OSC (Operating System Command) queries | ||
| # Format: ESC ] Ps ; Pt (BEL | ST) | ||
| # Common queries: | ||
| # OSC 10 ; ? - foreground color query | ||
| # OSC 11 ; ? - background color query | ||
| # OSC 4 ; index ; ? - palette color query | ||
| # Terminators: BEL (\x07) or ST (ESC \) | ||
| _OSC_QUERY_PATTERN = re.compile( | ||
| rb"\x1b\]" # OSC introducer | ||
| rb"(?:10|11|4)" # Color query codes (10=fg, 11=bg, 4=palette) | ||
| rb"[^" # Match until terminator | ||
| rb"\x07\x1b]*" # (not BEL or ESC) | ||
| rb"(?:\x07|\x1b\\)" # BEL or ST terminator | ||
jpshackelford marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
|
|
||
| # DA (Device Attributes) primary query | ||
| # Format: ESC [ c or ESC [ 0 c | ||
| _DA_PATTERN = re.compile(rb"\x1b\[0?c") | ||
|
|
||
| # DA2 (Secondary Device Attributes) query | ||
| # Format: ESC [ > c or ESC [ > 0 c | ||
| _DA2_PATTERN = re.compile(rb"\x1b\[>0?c") | ||
|
|
||
| # DECRQSS (Request Selection or Setting) - various terminal state queries | ||
| # Format: ESC P $ q <setting> ST | ||
| _DECRQSS_PATTERN = re.compile( | ||
| rb"\x1bP\$q" # DCS introducer + DECRQSS | ||
| rb"[^\x1b]*" # Setting identifier | ||
| rb"\x1b\\" # ST terminator | ||
| ) | ||
|
|
||
|
|
||
| def filter_terminal_queries(output: str) -> str: | ||
| """Filter terminal query sequences from captured terminal output. | ||
|
|
||
| Removes escape sequences that would cause the terminal to respond | ||
| when the output is displayed, while preserving legitimate formatting | ||
| sequences (colors, cursor movement, etc.). | ||
|
|
||
| Args: | ||
| output: Raw terminal output that may contain query sequences. | ||
|
|
||
| Returns: | ||
| Filtered output with query sequences removed. | ||
| """ | ||
| # Convert to bytes for regex matching (escape sequences are byte-level) | ||
| output_bytes = output.encode("utf-8", errors="surrogateescape") | ||
|
|
||
| # Remove each type of query sequence | ||
jpshackelford marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| output_bytes = _DSR_PATTERN.sub(b"", output_bytes) | ||
| output_bytes = _OSC_QUERY_PATTERN.sub(b"", output_bytes) | ||
| output_bytes = _DA_PATTERN.sub(b"", output_bytes) | ||
| output_bytes = _DA2_PATTERN.sub(b"", output_bytes) | ||
| output_bytes = _DECRQSS_PATTERN.sub(b"", output_bytes) | ||
|
|
||
| # Convert back to string | ||
| return output_bytes.decode("utf-8", errors="surrogateescape") | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| """Tests for terminal escape sequence filtering. | ||
|
|
||
| See: https://github.com/OpenHands/software-agent-sdk/issues/2244 | ||
| """ | ||
|
|
||
| from openhands.tools.terminal.utils.escape_filter import filter_terminal_queries | ||
|
|
||
|
|
||
| class TestFilterTerminalQueries: | ||
| """Tests for the filter_terminal_queries function.""" | ||
|
|
||
| def test_dsr_query_removed(self): | ||
| """DSR (Device Status Report) queries should be removed.""" | ||
| # \x1b[6n is the cursor position query | ||
| output = "some text\x1b[6nmore text" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "some textmore text" | ||
|
|
||
| def test_osc_11_background_query_removed(self): | ||
| """OSC 11 (background color query) should be removed.""" | ||
| # \x1b]11;?\x07 queries background color | ||
| output = "start\x1b]11;?\x07end" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "startend" | ||
|
|
||
| def test_osc_10_foreground_query_removed(self): | ||
| """OSC 10 (foreground color query) should be removed.""" | ||
| output = "start\x1b]10;?\x07end" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "startend" | ||
|
|
||
| def test_osc_4_palette_query_removed(self): | ||
| """OSC 4 (palette color query) should be removed.""" | ||
| output = "start\x1b]4;?\x07end" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "startend" | ||
|
|
||
| def test_osc_with_st_terminator_removed(self): | ||
| """OSC queries with ST terminator should be removed.""" | ||
| # ST terminator is \x1b\\ | ||
| output = "start\x1b]11;?\x1b\\end" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "startend" | ||
|
|
||
| def test_da_primary_query_removed(self): | ||
| """DA (Device Attributes) primary queries should be removed.""" | ||
| # \x1b[c and \x1b[0c | ||
| output = "start\x1b[cend" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "startend" | ||
|
|
||
| output2 = "start\x1b[0cend" | ||
| result2 = filter_terminal_queries(output2) | ||
| assert result2 == "startend" | ||
|
|
||
| def test_da2_secondary_query_removed(self): | ||
| """DA2 (Secondary Device Attributes) queries should be removed.""" | ||
| # \x1b[>c and \x1b[>0c | ||
| output = "start\x1b[>cend" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "startend" | ||
|
|
||
| output2 = "start\x1b[>0cend" | ||
| result2 = filter_terminal_queries(output2) | ||
| assert result2 == "startend" | ||
|
|
||
| def test_decrqss_query_removed(self): | ||
| """DECRQSS (Request Selection or Setting) queries should be removed.""" | ||
| # \x1bP$q...\x1b\\ | ||
| output = "start\x1bP$qsetting\x1b\\end" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "startend" | ||
|
|
||
| def test_colors_preserved(self): | ||
| """ANSI color codes should NOT be removed.""" | ||
| # Red text: \x1b[31m | ||
| output = "normal \x1b[31mred text\x1b[0m normal" | ||
| result = filter_terminal_queries(output) | ||
| assert result == output | ||
|
|
||
| def test_cursor_movement_preserved(self): | ||
| """Cursor movement codes should NOT be removed.""" | ||
| # Move cursor: \x1b[H (home), \x1b[5A (up 5) | ||
| output = "start\x1b[Hmiddle\x1b[5Aend" | ||
| result = filter_terminal_queries(output) | ||
| assert result == output | ||
|
|
||
| def test_multiple_queries_removed(self): | ||
| """Multiple query sequences should all be removed.""" | ||
| output = "\x1b[6n\x1b]11;?\x07text\x1b[6n" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "text" | ||
|
|
||
| def test_mixed_queries_and_formatting(self): | ||
| """Queries removed while formatting preserved.""" | ||
| # Color + query + more color | ||
| output = "\x1b[32mgreen\x1b[6nmore\x1b]11;?\x07text\x1b[0m" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "\x1b[32mgreenmoretext\x1b[0m" | ||
|
|
||
| def test_empty_string(self): | ||
| """Empty string should return empty string.""" | ||
| assert filter_terminal_queries("") == "" | ||
|
|
||
| def test_no_escape_sequences(self): | ||
| """Plain text without escape sequences passes through.""" | ||
| output = "Hello, World!" | ||
| assert filter_terminal_queries(output) == output | ||
|
|
||
| def test_unicode_preserved(self): | ||
| """Unicode characters should be preserved.""" | ||
| output = "Hello 🌍 World \x1b[6n with emoji" | ||
| result = filter_terminal_queries(output) | ||
| assert result == "Hello 🌍 World with emoji" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.