Skip to content

feat(terminal): hint at structured tools when bash fails to parse a command - #25862

Open
sparkeros wants to merge 1 commit into
NousResearch:mainfrom
sparkeros:pr/feat-bash-quoting-hint
Open

feat(terminal): hint at structured tools when bash fails to parse a command#25862
sparkeros wants to merge 1 commit into
NousResearch:mainfrom
sparkeros:pr/feat-bash-quoting-hint

Conversation

@sparkeros

Copy link
Copy Markdown

What does this PR do?

When the `terminal` tool's `bash -c ` invocation fails to parse the command (almost always an unclosed quote, e.g. an apostrophe inside a single-quoted JSON body), bash exits 2 with a recognizable message in stdout. Today the model just sees:

```
/usr/bin/bash: eval: line 3: unexpected EOF while looking for matching ''
```

…and frequently retries the same broken quoting because the error doesn't say what to do.

This PR adds a detector that catches the parser-error markers (with `returncode == 2`) and attaches an actionable `_hint` to the tool result, pointing the model at:

  1. The new `http` tool (feat(tools): add http tool for structured HTTP requests #25861) — eliminates the shell entirely for any HTTP-shaped call. Single best fix.
  2. End-quote-concat or double-quoted alternatives for raw shell.

Standalone — works without #25861, but recommends it.

Type of Change

  • 🐛 Bug fix (non-breaking change that improves recovery from a recurring class of failures)

Changes Made

  • `tools/terminal_tool.py`:
    • New module-level `_BASH_QUOTING_ERROR_MARKERS` tuple covering the bash parser's known failure messages.
    • New `_detect_bash_quoting_error(output, returncode)` helper. Conservative gate: requires BOTH `returncode == 2` AND a known marker present in `output`.
    • Result-package wiring: when the helper returns a hint, attach it to `result_dict["_hint"]`.
  • `tests/tools/test_terminal_quoting_hint.py` (9 tests).

How to Test

`pytest tests/tools/test_terminal_quoting_hint.py -q` — 9 passed.

Checklist

  • Read the Contributing Guide
  • Conventional Commits (`feat(terminal):`)
  • Searched for existing PRs
  • Only the one fix
  • Tests added + passing on Ubuntu 24.04 / WSL2 (aarch64)
  • No documentation, config, or schema changes needed
  • Pure-Python string check; cross-platform

When bash fails to parse a command — almost always an unclosed quote
in a curl payload, e.g. an apostrophe inside a single-quoted JSON body
like `-d '{"body":"It's done"}'` — it exits 2 with a recognizable
message in stdout. Today the model just sees:

  /usr/bin/bash: eval: line 3: unexpected EOF while looking for matching ''

and frequently retries the same broken quoting. Add a detector that
catches the parser-error markers (with returncode==2) and attaches an
actionable `_hint` to the tool result pointing at:

  1. The new `http` tool — eliminates the shell entirely for any
     HTTP-shaped call. Single best fix.
  2. End-quote-concat or double-quoted alternatives for raw shell.

The detector is conservative: requires BOTH exit code 2 AND a known
parser-error marker. A real command that prints the marker text in
stdout but exits 0 (e.g. `echo unexpected EOF ...`) does not trigger.

Tests in tests/tools/test_terminal_quoting_hint.py cover the four
documented markers, the exit-code gate, and that the hint advertises
the http tool explicitly.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets tool/terminal Terminal execution and process management labels May 14, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for targeting a real recovery gap: current foreground execution still reaches bash -c through tools/environments/base.py:933-936 and tools/environments/local.py:1003, while tools/terminal_tool.py:2740-2785 returns no quoting-specific guidance.

Problems

  • tools/terminal_tool.py:1575 tells the model to call http, but current main has no tools/http_tool.py and no http entry in toolsets.py:31-80; linked PR #25861 is still open. This would direct the model to an unavailable tool.
  • The change only packages a foreground result. Background commands also run through a shell (tools/process_registry.py:764-776) and expose their output via process poll/log/wait (tools/process_registry.py:2233-2239), without the detector. The added tests only call the helper directly.

Suggested changes

  • Keep the generic quoting guidance, but remove or availability-gate the http recommendation.
  • Add a foreground result-packaging test; if background shell failures are in scope, cover the process-result path too.

Automated hermes-sweeper review.

Comment thread tools/terminal_tool.py
"mismatched quote. Common cause: a single-quoted string with a "
"literal apostrophe inside (e.g. `-d '{\"body\":\"It's done\"}'`). "
"Fastest fixes: "
"(1) If this was an HTTP request, call the `http` tool instead — "

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

http is not available on current main: tools/http_tool.py is absent, _HERMES_CORE_TOOLS has no http entry, and linked PR #25861 is still open. Remove this unconditional recommendation or gate it on actual tool availability so the recovery hint cannot direct the model to a nonexistent tool.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/terminal Terminal execution and process management type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants