Skip to content

Conversation

@enyst
Copy link
Collaborator

@enyst enyst commented Nov 28, 2025

I had the agent do this for itself in an overnight experiment where it created itself a TUI to use it for E2E testing of switching LLM Profiles

The TUI it made is here (it's not pretty but it worked without humans 😅):

I'm not sure when it needed disabling history expansion, but it seems like a good idea to me?


(Agent write-up)
This PR disables bash history expansion in the tmux-based terminal backend to prevent unintended "!" expansion that can mangle commands.

Change:

  • Prepend set +H; to the initialization command when configuring the shell in TmuxTerminal.initialize so that ! characters are not interpreted by history expansion.

Rationale:

  • When running commands containing ! (e.g., URLs, JSON, or other content), bash history expansion can alter the input unexpectedly. Disabling it leads to more predictable behavior for programmatic terminal execution.

Implementation details:

  • File: openhands-tools/openhands/tools/terminal/terminal/tmux_terminal.py
  • Modified the self.pane.send_keys(...) line to include set +H; before setting PROMPT_COMMAND and PS2.

This is a minimal, backwards-compatible change that only affects the initialization of the interactive shell used by the tmux terminal backend.

@enyst can click here to continue refining the PR


Agent Server images for this PR

GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant Architectures Base Image Docs / Tags
java amd64, arm64 eclipse-temurin:17-jdk Link
python amd64, arm64 nikolaik/python-nodejs:python3.12-nodejs22 Link
golang amd64, arm64 golang:1.21-bookworm Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:2a61f33-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-2a61f33-python \
  ghcr.io/openhands/agent-server:2a61f33-python

All tags pushed for this build

ghcr.io/openhands/agent-server:2a61f33-golang-amd64
ghcr.io/openhands/agent-server:2a61f33-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:2a61f33-golang-arm64
ghcr.io/openhands/agent-server:2a61f33-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:2a61f33-java-amd64
ghcr.io/openhands/agent-server:2a61f33-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:2a61f33-java-arm64
ghcr.io/openhands/agent-server:2a61f33-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:2a61f33-python-amd64
ghcr.io/openhands/agent-server:2a61f33-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-amd64
ghcr.io/openhands/agent-server:2a61f33-python-arm64
ghcr.io/openhands/agent-server:2a61f33-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-arm64
ghcr.io/openhands/agent-server:2a61f33-golang
ghcr.io/openhands/agent-server:2a61f33-java
ghcr.io/openhands/agent-server:2a61f33-python

About Multi-Architecture Support

  • Each variant tag (e.g., 2a61f33-python) is a multi-arch manifest supporting both amd64 and arm64
  • Docker automatically pulls the correct architecture for your platform
  • Individual architecture tags (e.g., 2a61f33-python-amd64) are also available if needed

Co-authored-by: openhands <openhands@all-hands.dev>
…ng during init handshake\n\nCo-authored-by: openhands <openhands@all-hands.dev>
Copy link
Collaborator

@xingyaoww xingyaoww left a comment

Choose a reason for hiding this comment

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

LGTM! Can we add a few tests for it?

@enyst
Copy link
Collaborator Author

enyst commented Nov 29, 2025

@OpenHands Let’s add unit tests for this PR. Make sure to find the right place for these tests, probably in the same test file(s) where other functions in these files are tested.

@openhands-ai
Copy link

openhands-ai bot commented Nov 29, 2025

I'm on it! enyst can track my progress at all-hands.dev

…isabled in tmux and subprocess backends

- Mock tmux to assert initialize sends 'set +H;' in TmuxTerminal
- E2E check in SubprocessTerminal that '!' is preserved (no history expansion)

Co-authored-by: openhands <openhands@all-hands.dev>
@openhands-ai
Copy link

openhands-ai bot commented Nov 29, 2025

Summary of changes made

  • Added targeted unit tests to verify bash history expansion is disabled in terminal backends:
    • tests/tools/terminal/test_disable_history_expansion.py
      • test_tmux_terminal_initialization_disables_history_expansion
        • Mocks libtmux to avoid requiring a real tmux server.
        • Verifies TmuxTerminal.initialize sends a command containing set +H;.
        • Uses monkeypatch to no-op clear_screen during initialize to avoid isinstance checks with the stubbed pane.
      • test_subprocess_terminal_disables_history_expansion_e2e
        • End-to-end check for SubprocessTerminal by running echo A!B and asserting no “event not found” and that “A!B” appears.

Validation steps performed

  • Pre-commit run on the new test file (ruff format/lint, pyright, etc.): passed.
  • Ran the new tests with uv run pytest for the file: 2 passed.
  • Committed only the new test file and pushed to the existing branch fix/disable-bash-history-expansion with the required Co-authored-by trailer.

Checklist

  • Placed tests in the correct location (tests/tools/terminal).
  • Avoided test classes; minimal tests covering the change.
  • Defensive to CI environment constraints (e.g., skips if subprocess bash unavailable).
  • Kept changes concise with no unrelated modifications.
  • Pushed updates to the current PR branch.

View full conversation

with patch(
"openhands.tools.terminal.terminal.tmux_terminal.libtmux.Server", FakeServer
):
# Avoid isinstance check in clear_screen during initialize since we stub Pane
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔 i think we do have some unit tests that are actually running a real libtmux Server?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

@github-actions
Copy link
Contributor

github-actions bot commented Nov 29, 2025

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands-tools/openhands/tools/terminal/terminal
   subprocess_terminal.py25521515%33–34, 59–62, 65–69, 75–76, 82–83, 85–87, 94–95, 99–100, 106–107, 110–113, 115, 118, 120–122, 136–139, 141, 144–145, 148, 151–152, 157–158, 165–167, 169, 172–173, 175, 179–180, 182–183, 185–191, 193–199, 202–207, 209–210, 212–213, 218, 220–228, 232–234, 236–237, 239–240, 243–245, 247–250, 252–253, 255–256, 258–263, 268–270, 272, 275, 278–279, 282–283, 289–291, 293–300, 302–305, 309–317, 330–331, 333, 350–351, 354–355, 357, 359, 361–363, 366–367, 369–371, 373–374, 376–377, 386–387, 391, 393–398, 402–403, 405–408, 410–416, 418–419, 421–425, 429–430, 432–438, 442–443, 446–447, 449–450, 452–454
   tmux_terminal.py862472%42, 51, 99, 103, 105, 116, 127, 141, 153–160, 168–169, 171–172, 174, 176–178
TOTAL12708594553% 

enyst and others added 4 commits November 29, 2025 20:39
… subprocess backends

- In test_terminal_session.py, add test_history_expansion_disabled to assert '!' is preserved and no history expansion occurs across both backends

Co-authored-by: openhands <openhands@all-hands.dev>
…eal parametrized test

- Delete tests/tools/terminal/test_disable_history_expansion.py as redundant
- test_history_expansion_disabled in test_terminal_session.py now covers both tmux and subprocess

Co-authored-by: openhands <openhands@all-hands.dev>
- Add @parametrize_terminal_types to ensure the test runs for both tmux and subprocess and fixture exists

Co-authored-by: openhands <openhands@all-hands.dev>
@enyst enyst requested a review from xingyaoww November 29, 2025 22:09
@OpenHands OpenHands deleted a comment from openhands-ai bot Nov 29, 2025
@enyst enyst merged commit 7ffe922 into main Nov 29, 2025
22 checks passed
@enyst enyst deleted the fix/disable-bash-history-expansion branch November 29, 2025 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants