fix(grep): use extended regular expressions in the system fallback

[BORAN002]

HUMAN:

I asked Codex to investigate contribution opportunities and prepare this fix for regular expressions in the system grep fallback. Codex implemented and tested the change and drafted this note with my authorization. The validation below was performed by the agent; I am submitting the change for maintainer review.

---

AGENT:

Why

Without ripgrep, the system grep fallbacks interpret patterns as basic regular expressions. Common patterns such as ^(foo|bar)+[0-9]{2}$ miss matches. On Windows, Git/MSYS argument processing can also remove regex backslashes and match the wrong file.

Summary

• Enable extended regular expressions in GrepExecutor and the SDK terminal-compatibility fallback.
• Send GrepExecutor patterns through binary stdin with grep -f -, preserving escapes and avoiding Windows newline translation.
• Cover extended operators, escaped punctuation, and empty patterns with actual grep regression tests.

Issue Number

Fixes #4983.

How to Test

After make build, run uv run pytest tests/tools/grep tests/sdk/agent/test_tool_call_compatibility.py -q: 86 passed locally on macOS. Run .venv/bin/python .pr/repro_grep_regex.py for the public-executor smoke test with system grep and no ripgrep; both cases pass.

uv run pre-commit run --all-files --show-diff-on-failure and the dynamic-attribute baseline guard both pass. Merged upstream commit 76e9e250 (#4968), which fixes the four pre-existing dynamic-attribute violations. Checker, streaming, and telemetry regression suites: 83 passed.

The native Windows transport probe reproduced the original escaped-pattern failure and exposed text-mode stdin's newline translation. The binary-input probe passes in the Windows validation run, which also completed the upstream Windows tools test group: 732 passed, 105 skipped (using the same exclusions as the upstream Windows job). The previously failing escaped-pattern test now passes. The fork validation uses identical production and regression-test files to this PR; its temporary workflow is not included in this PR. Upstream checks have started for the new head.

Reproduction and validation · original before · after

Real Agent validation (2026-10-05)

Completed the same real OpenHands Agent conversation on base 76e9e250 and PR head 486b46fb, using DeepSeek Flash on macOS 15.6 with rg unavailable. The base missed the extended-regex match and returned the wrong file for escaped punctuation; the head returned matching.txt in both cases. Both conversations made the exact requested grep calls, received the observations, and finished normally without retries or Python fallback.

Recorded environment, tool events, summaries, runtime logs, and the original harness · Reproduction instructions.

The follow-up adds a POSIX-only guard to the terminal integration test and strengthens validation of agent errors and finish ordering. The existing two real conversation logs also pass the stronger checks. Their recorded hashes and results are unchanged; the original harness is preserved alongside them. Production code is unchanged from the live-tested revision.

This validates the real GrepTool path on macOS. Live Windows conversations, the terminal compatibility path, and repository benchmarks remain outside these two runs. Human maintainer review is still pending.

Video/Screenshots

CLI and native Windows CI evidence are linked above; this change has no UI.

Design Doc

Reproduction, transport details, and compatibility notes are linked above.

Type

• Bug fix
• Feature
• Refactor
• Breaking change
• Docs / chore

Notes

This fixes common extended-regex syntax and Windows pattern transport, not every regex dialect difference. No agent benchmark was run. No public API, persisted schema, dependency, prompt, or tool-description change.

[github-actions]

📁 PR Artifacts Notice

This PR contains a .pr/ directory with temporary PR-specific documents. Because this is a fork PR, the workflow will open or update a cleanup PR against main after merge.

[all-hands-bot]

🚦 CI is currently failing on this PR's latest commit.

Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request @all-hands-bot as a reviewer to have it reviewed regardless of CI status.)

This is an automated check - no AI was used to generate this comment.

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Review of 2a5d21633050ea05b54fe14f9241b2c6917e571b

Change under review: +207/-4 across 8 files. Production changes are limited to
openhands-sdk/openhands/sdk/agent/utils.py (adds -E to the system-grep terminal
fallback) and openhands-tools/openhands/tools/grep/impl.py (adds -E, reads the
pattern via grep -f - with binary stdin, drops text=True, decodes stdout with
os.fsdecode). The remaining files are tests plus .pr/ evidence.

What I verified in the workspace

• Repository guidance read: root AGENTS.md and .agents/skills/custom-codereview-guide.md.
• uv run pytest tests/tools/grep tests/sdk/agent/test_tool_call_compatibility.py -q
  → 74 passed, 12 skipped (the skips are the ripgrep-consistency class, since
  rg is not installed here). The new -E and -f - cases pass.
• .venv/bin/python .pr/repro_grep_regex.py → both cases pass; system grep now
  resolves ^(foo|bar)+[0-9]{2}$ and ^foo\(bar\)\+$ against the intended file.
• Independent comparison of rg vs grep (BRE) vs grep -E (ERE) on representative
  patterns (a{2}, a\+b, x\|y, a+b) confirms -E brings the system-grep
  backend materially closer to the ripgrep backend, which is the stated intent.
• Spot-checked the new stdin transport (pattern + "\n"): empty patterns still
  match only non-empty files, and a trailing newline does not expand into an extra
  empty pattern. Behavior is consistent with the previous argv path and with
  ripgrep / the Python fallback. No regression found.

No material code defects found

I did not find a demonstrated bug, security issue, or breaking behavior change on
this head. The -E switch is the correct alignment with the ripgrep primary
backend, and routing the pattern through binary stdin is a sound way to preserve
escapes on Windows and avoid LF→CRLF translation.

Two process points for the maintainer (no inline findings)

1. Eval-risk policy. This modifies tool execution and stdin/stdout handling,
   which custom-codereview-guide.md explicitly lists as eval-risk. The PR states
   "No agent benchmark was run" and provides no openhands-eval-monitor link, so
   per the repo review policy I am leaving a COMMENT rather than an APPROVE, for a
   maintainer to run lightweight evals before merge.
2. Upstream CI has not executed on this head. For 2a5d2163, the workflow runs
   for Run tests, Pre-commit checks, Agent Server, etc. are all
   action_required (fork approval pending). The only completed checks are the
   PR-description/artifact checks. The validation cited in the PR comes from the
   author's fork, so the upstream suite and the Windows tools group still need to
   run on the exact head.

Verdict: no changes requested on the code itself; deferring the eval/CI items to a
human maintainer.

✅ APPROVED

[neubig]

This review was submitted by an AI agent (OpenHands) on behalf of @neubig.

The low-level implementation and validation are strong: the public GrepExecutor reproduction uses real system grep, the native Windows run demonstrates the argv/text-stdin failure and binary-stdin recovery, and the focused suites pass. However, this changes agent tool execution and the terminal compatibility path, and the current evidence does not show the affected behavior in a live OpenHands conversation.

Before approval, please provide a small authentic before/after live-agent run with ripgrep unavailable:

1. On the base revision, ask a running OpenHands agent to perform a search using an affected extended or escaped regex and show the incorrect/missing result.
2. Repeat the same prompt/workflow on this PR head and show the correct result.
3. Include the emitted grep tool call or normalized terminal action and the resulting observation returned to the agent, so it is clear the real agent/tool/terminal path—not only a constructed test invocation—was exercised.
4. State the runtime environment and how rg was made unavailable.

The existing Windows probe remains good evidence for the transport mechanism, but it does not exercise an actual agent conversation. Per the repository review policy for tool execution and stdin/stdout changes, I am withholding approval pending this live evidence and human eval review.

[BORAN002]

Thanks for clarifying the missing evidence. I added a real Agent reproduction script and run instructions in 486b46fb.

It runs the same prompt and fixtures on the base and PR head, with rg unavailable. It records the model-generated grep calls and the observations returned to the Agent, and checks both patterns without retries. This covers the GrepTool path; the existing Windows transport tests are separate evidence.

I do not have a working model API available, so the live Agent validation is still pending. Fixture preparation, offline conversation construction, and the applicable pre-commit checks pass, but those do not satisfy the requested live run.

Could a maintainer run the two commands with a configured model and attach the resulting environment, event, and summary files? The instructions include the base revision and expected results for both versions. Thanks!

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Review of 486b46fb8c9a0fadb544fbc1854f3a177c1022dc

Scope: this change belongs in this repository (SDK tool execution + the SDK
terminal-compatibility fallback), and the linked issue #4983 is open, bug,
tools, sdk, ready-for-dev. The production diff is unchanged from
2a5d2163: openhands-sdk/openhands/sdk/agent/utils.py adds -E to the
system-grep terminal fallback, and openhands-tools/openhands/tools/grep/impl.py
adds -E and reads the pattern via grep -f - with binary stdin. This new commit
(486b46fb) only adds .pr/ evidence files — no production or test code changed.

What I verified on this head

• uv run pytest tests/tools/grep tests/sdk/agent/test_tool_call_compatibility.py -q
  → 74 passed, 12 skipped (skips are the ripgrep-consistency class; rg is
  not installed here).
• .venv/bin/python .pr/repro_grep_regex.py → both cases pass against real system
  grep with rg unavailable.
• GitHub Actions for this exact head are now green: Run tests, Pre-commit checks, Agent Server, REST API breakage checks, Persisted settings compatibility checks, TypeScript client CI, and Review Thread Gate all
  report success.
• No material code defect, security issue, or behavior regression found. The -E
  flag is the right alignment with the ripgrep primary backend, and the binary
  stdin transport preserves escapes and avoids Windows newline translation.

Outstanding requested evidence (why this is a COMMENT, not an approval)

The existing maintainer review on this PR requested an authentic before/after
live-agent run with ripgrep unavailable, including the emitted grep tool call and
the observation returned to the agent. That request is still unmet on this head:

• .pr/live-grep-agent.md line 3 states "Status: not run against a real model."
• The PR body's "Real Agent validation (pending)" section says no live model run
  has been performed and asks a maintainer to run it.
• The contributor's latest comment confirms the live Agent validation is still
  pending and that only fixture preparation and offline object construction pass.

The new script is a well-formed harness, but a harness plus pending instructions is
not the requested evidence. Per custom-codereview-guide.md, changes to tool
execution and terminal stdin/stdout handling are eval-risk and must not be approved
without eval evidence (an openhands-eval-monitor link plus maintainer
confirmation). No such evidence is present, and the maintainer has explicitly
withheld approval pending the live run and human eval review.

No inline findings: the gap is evidence/validation, not a defect on a changed line.
Recommend a maintainer run the two documented commands (base and head) and attach
the resulting environment.json, events.jsonl, and summary.json, then complete
the human eval review before merge.

🔄 CHANGES REQUESTED

[BORAN002]

Thanks for clarifying what was missing. I’ve now run the same conversation on the base and PR head using a real OpenHands Agent with DeepSeek Flash.

Both runs used macOS 15.6, Python 3.13.13, the same prompt and fixtures, and a PATH containing only a symlink to /usr/bin/grep, so rg was unavailable.

Pattern	Base 76e9e250	PR head 486b46fb
^(foo|bar)+[0-9]{2}$	No matches — missed the correct file	matching.txt
^foo\(bar\)\+$	other.txt — matched the wrong file	matching.txt

Each Agent made the two requested grep calls without changing or retrying the patterns, received the observations, and finished with an accurate report of those results. Both conversations completed normally, with no system-grep failure or Python fallback.

The evidence and reproduction notes include each run’s environment.json, events.jsonl, summary.json, and runtime log. Local path prefixes are sanitized. The new commit only strengthens the validation harness and adds evidence; the production fix is unchanged.

This covers the real GrepTool path on macOS. The earlier native Windows transport checks are separate; I haven’t run a live Windows conversation, the terminal compatibility path, or a benchmark.

Could you take another look when you have a chance?

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Review of dc7024af36859bfc8cd52f00f3e567bb3bd3f8f9

Scope: this change belongs in this repository (SDK tool execution plus the SDK
terminal-compatibility fallback), and the linked issue #4983 is open with bug,
priority:low, tools, sdk, and ready-for-dev. The production diff is
identical to the previously reviewed 486b46fb: openhands-sdk/openhands/sdk/agent/utils.py
adds -E to the system-grep terminal fallback, and
openhands-tools/openhands/tools/grep/impl.py adds -E and reads the pattern via
grep -f - with binary stdin (os.fsencode / os.fsdecode, no text=True).

The maintainer-requested live-agent evidence is now provided and checks out

The prior review on this PR asked for an authentic before/after live-agent run
with ripgrep unavailable, including the emitted grep call and the returned
observation. That gap is now closed. I verified the new artifacts directly in the
workspace:

• .pr/live-grep-2026-10-05/base/summary.json and head/summary.json each show
  exactly two real grep calls plus a later finish, final_status: finished,
  empty conversation_errors, grep_fallbacks, and validation_errors, and
  matches_expected_revision_behavior: true.
• Base reproduces the defect: ^(foo|bar)+[0-9]{2}$ returns no matches and
  ^foo\(bar\)\+$ returns escaped/other.txt. Head returns matching.txt for
  both patterns. The finish messages accurately report the observations.
• Recorded provenance is self-consistent and matches the real code:
  grep_source_sha256 in head/environment.json is
  469cca61…982ad959, which is exactly the current
  openhands-tools/openhands/tools/grep/impl.py; base records
  904f785c…6054e2a3, which matches 76e9e250:…/grep/impl.py. The frozen
  harness hash 7c7e1ae7…8706bdbc matches the preserved script, and uv.lock
  matches the recorded uv_lock_sha256.
• git diff --stat 486b46fb dc7024af -- openhands-sdk openhands-tools tests shows
  the only production/test change since the live-tested revision is a 4-line
  POSIX guard on the terminal compatibility test, so the live evidence still
  covers the reviewed code.
• .pr/test_live_grep_event_sequence.py passes locally (12 passed) and revalidates
  the two recorded streams. No API key or authorization header is present in the
  published evidence.

Verification on this head

• uv run pytest tests/tools/grep tests/sdk/agent/test_tool_call_compatibility.py -q
  → 74 passed, 12 skipped (the skips are the ripgrep-consistency class; rg
  is not installed here).
• All GitHub Actions checks for dc7024af are green, including tools-tests,
  sdk-tests, windows-tests, agent-server-tests, pre-commit,
  coverage-report, REST API (OpenAPI), and unresolved-review-threads.
• No material code defect, security issue, or behavior regression found. -E
  aligns the fallback with the ripgrep primary backend, and the binary stdin
  transport preserves escapes while avoiding Windows newline translation. The new
  os.name == "nt" skip is appropriate for a test that depends on POSIX shell
  quoting.

Why this remains a COMMENT rather than an approval

custom-codereview-guide.md treats tool execution and terminal stdin/stdout
handling as eval-risk and requires eval evidence — an openhands-eval-monitor
link plus a human maintainer comment confirming the results — before approval.
This PR still states "No agent benchmark was run", there is no eval-monitor link,
and the maintainer review explicitly withheld approval pending this live evidence
and human eval review. The live evidence portion is now satisfied; the human
eval/human maintainer confirmation is the remaining item, which is not mine to
grant.

No inline findings: the remaining item is validation/handoff, not a defect on a
changed line. Recommend the maintainer confirm the live evidence and complete the
human eval review before merge.

🔄 CHANGES REQUESTED