chore(tools): replace bashlex (GPLv3) with tree-sitter-bash (MIT)#3237
chore(tools): replace bashlex (GPLv3) with tree-sitter-bash (MIT)#3237VascoSch92 wants to merge 2 commits into
Conversation
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable - Well-executed dependency migration that solves real license and maintenance problems.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
This PR changes terminal command parsing, which technically falls under the eval-risk category (terminal/stdin/stdout handling). However, the comprehensive test coverage (30 parsing tests + integration tests all passing) provides strong evidence that the behavior contract is preserved. The license incompatibility (GPLv3 bashlex in MIT repo) is a real blocker that justifies this migration. The new tree-sitter implementation is cleaner (19 fewer lines, eliminates 4 exception types, better abstractions with _PRESERVE_TYPES) and more maintainable (active project vs unmaintained since 2020).
Dependency age verification: tree-sitter-bash 0.25.1 uploaded 2025-12-02, well past the 7-day guardrail. ✅
VERDICT:
✅ Worth merging - License blocker resolved, comprehensive test coverage confirms behavior preservation, cleaner implementation.
KEY INSIGHT:
Replacing a GPL-licensed dependency with an MIT-licensed alternative while simultaneously improving code quality and maintainability - this is the right kind of technical debt paydown.
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable - Well-executed dependency migration with excellent test coverage.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
This changes terminal command parsing (eval-risk category per repo guidelines). While the comprehensive test coverage (30 parsing tests + integration tests all passing) provides strong evidence the behavior contract is preserved, changes to terminal/stdin/stdout handling require eval validation before merging.
VERDICT:
⏸️ Needs eval validation - Code quality is excellent and solves real license/maintenance problems, but please run a lightweight eval (e.g., SWE-bench Lite) to confirm no benchmark impact before merging.
KEY INSIGHT:
Clean refactor that eliminates license incompatibility (GPLv3→MIT) and improves error handling (graceful recovery vs exception catching), but falls under eval-risk category due to terminal parsing changes.
|
Evaluation Triggered
|
Coverage Report •
|
||||||||||||||||||||
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
✅ QA Report: PASS
Successfully verified license swap from bashlex (GPLv3+) to tree-sitter-bash (MIT) while preserving all functionality.
Does this PR achieve its stated goal?
Yes. The PR successfully removes the GPLv3+ bashlex dependency and replaces it with MIT-licensed tree-sitter-bash. I verified:
- License compliance achieved: Confirmed bashlex is removed and tree-sitter-bash (MIT) is installed
- API preserved: Both
split_bash_commands()andescape_bash_special_chars()maintain identical signatures - Behavior maintained: All 30 existing tests pass without modification
- Robustness improved: Graceful error recovery on malformed input (no crashes on unclosed quotes, backticks, etc.)
| Phase | Result |
|---|---|
| Environment Setup | ✅ Dependencies installed, PR branch built successfully |
| CI Status | ✅ All core checks passing (sdk-tests, workspace-tests, cross-tests, pre-commit) |
| Functional Verification | ✅ All tests pass, real-world scenarios work correctly, license change verified |
Functional Verification
Test 1: Existing Test Suite
Baseline established (no before/after needed — tests define expected behavior):
Ran the repository's 30 existing bash parsing tests:
uv run pytest tests/tools/terminal/test_terminal_parsing.py -vResult:
30 passed, 5 warnings in 0.30s
All tests pass without modification, confirming backward compatibility:
test_split_commands_util: Multi-line commands split correctlytest_heredoc: Heredoc parsing preservedtest_backslash_continuation: Line continuation handledtest_comments: Comments folded correctlytest_escape_bash_special_chars: Escape doubling works for\;,\|,\&,\<,\>test_escape_bash_special_chars_with_heredoc: Heredoc content untouchedtest_escape_bash_special_chars_with_command_substitution: Command substitutions preservedtest_invalid_syntax: Parse errors return input unchanged (graceful fallback)
Test 2: Real-World Command Patterns
Tested 7 realistic scenarios an AI agent might execute:
Scenario 1 — Multi-line script setup:
input = """mkdir -p test_dir
cd test_dir
echo "Hello" > file.txt
cat file.txt"""
result = split_bash_commands(input)
# Expected: 4 commands (split on newlines)
# Actual: 4 commands ✓Scenario 2 — Conditional execution:
input = "ls /nonexistent || echo 'Directory not found'"
result = split_bash_commands(input)
# Expected: 1 command (OR operator keeps together)
# Actual: 1 command ✓Scenario 3 — Chained commands:
input = "mkdir -p build && cd build && cmake .."
result = split_bash_commands(input)
# Expected: 1 command (AND chain stays together)
# Actual: 1 command ✓Scenario 4 — Complex heredoc:
input = """cat > config.yaml <<EOF
database:
host: localhost
port: 5432
EOF
echo "Config created" """
result = split_bash_commands(input)
# Expected: 2 commands (heredoc + echo)
# Actual: 2 commands ✓Scenario 5 — Background task:
input = "python server.py > server.log 2>&1 &"
result = split_bash_commands(input)
# Expected: 1 command (background operator keeps together)
# Actual: 1 command ✓Scenario 6 — Escaped special chars:
input = r"find . -name '*.txt' -exec grep 'pattern' {} \;"
result = escape_bash_special_chars(input)
# Expected: \; doubled to \\;
# Actual: Changed as expected ✓Scenario 7 — Git workflow:
input = """git add .
git commit -m "Update files"
git push origin main"""
result = split_bash_commands(input)
# Expected: 3 commands (split on newlines)
# Actual: 3 commands ✓All scenarios behaved correctly — splitting on newlines, preserving operators (&&, ||, |, &, ;), handling heredocs, and doubling escapes outside quotes.
Test 3: Crash Resistance
Tested inputs known to crash bashlex (from issue #7391, #8369):
problematic_inputs = [
("Unclosed backtick", "echo `unclosed"),
("Unclosed quote", 'echo "unclosed'),
("Deeply nested", "echo $((((1+1))))"),
("Mixed unclosed", 'echo "test `nested'),
("Unicode characters", "echo 'Hello 世界'"),
("Very long line", "echo " + "x" * 10000),
]Result: All handled gracefully (no exceptions), returning the input unchanged when parse errors occur. This matches the PR's claim that tree-sitter "recovers gracefully on malformed input" instead of crashing.
Test 4: License Compliance
Before (main branch):
uv pip list | grep bashlex
# bashlex==0.18 (GPLv3+)After (PR branch):
uv pip list | grep bashlex
# (no output — bashlex removed)
uv run python -c "import importlib.metadata; print(importlib.metadata.metadata('tree-sitter-bash')['License'])"
# MITVerified:
- bashlex dependency removed from
openhands-tools/pyproject.toml - tree-sitter (>=0.24) and tree-sitter-bash (>=0.23) added
- tree-sitter-bash is MIT-licensed
- No bashlex modules loaded at runtime
License goal achieved: GPLv3+ dependency removed, replaced with MIT.
Issues Found
None.
enyst
left a comment
enyst
left a comment
There was a problem hiding this comment.
Thank you! As a side effect, the code really looks cleaner.
One tiny question we may want to look into is if the dependencies are fairly heavy? But it’s good anyway to start with.
It would be nice to change every regex related bash command with tree-sitter :-) To answer your question I will wait that the eval is done (just for safety) and then I will merge :-) |
3 participants
Why
bashlex is GPLv3+ while this repo is MIT — a transitive copyleft dependency we want to drop. It's also unmaintained (last meaningful release ~2020) and crashes on a variety of valid shell constructs, which is why the wrapper currently catches four exception types and silently falls back to the raw string.
tree-sitter and tree-sitter-bash are both MIT, actively maintained, and recover gracefully on malformed input (an ERROR node in the tree, not an exception).
Summary
API & behavior
Public surface unchanged: split_bash_commands(commands: str) -> list[str] and escape_bash_special_chars(command: str) -> str.
Splitting contract preserved: statements separated by a newline split into separate entries; statements joined by ;, &&, ||, |, or & stay together. Comments and whitespace between statements fold into the preceding entry. On parse failure the input is returned unchanged — same silent-passthrough as the bashlex era.
Issue Number
Refs #2721
How to Test
test_multiline_command_loop) pass on both tmux and subprocess backends.
Type
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:bf3b062-pythonRun
All tags pushed for this build
About Multi-Architecture Support
bf3b062-python) is a multi-arch manifest supporting both amd64 and arm64bf3b062-python-amd64) are also available if needed