From 5a3ce462cca661651c3dae35dba5b6e68f463542 Mon Sep 17 00:00:00 2001
From: hermes <hermes@rasyidanaf.com>
Date: Wed, 14 Oct 2026 21:16:06 +0000
Subject: [PATCH] fix(agents): address agent smoke review

- Require bash 4.4+ up front, and match answer facts with jq regexes
  (Oniguruma, (?i) everywhere) instead of GNU-only grep -P.
- Unparseable agent output fails that run instead of aborting the
  matrix; check_run now also reports unparseable tool calls, which
  previously passed the tool check silently.
- Copy the Hermes model block with awk up to the next top-level key, so
  a final or differently ordered block is handled.
- --keep keeps run output but deletes the copied agent credentials.
- docs: full path to the tool golden.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 docs/SMOKE.md               |  5 +++--
 tests/agents/agent-smoke.sh | 21 ++++++++++++++++-----
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/docs/SMOKE.md b/docs/SMOKE.md
index 374abe0..2008abf 100644
--- a/docs/SMOKE.md
+++ b/docs/SMOKE.md
@@ -26,7 +26,7 @@ user-visible output changed. Regenerate with
 and commit the golden diff with the reason.
 
 `crates/idx-mcp/tests/mcp.rs` drives `idx-mcp` over stdio JSON-RPC with the
-mock provider. `tests/golden/tools.json` there pins what agents see (tool
+mock provider. `crates/idx-mcp/tests/golden/tools.json` pins what agents see (tool
 names, descriptions, input schemas); regenerate it the same way after an
 intended change.
 
@@ -91,7 +91,8 @@ tests/agents/agent-smoke.sh --live --case shareholders --keep          # real da
 - Each agent gets an isolated config that registers only the local server;
   your own agent settings and MCP servers are not loaded or changed. Logins
   are copied from the agent's home (Codex `auth.json`, pi and Hermes auth
-  files) into a temp dir that is deleted afterwards unless `--keep`.
+  files) into a temp dir that is deleted afterwards; `--keep` keeps the run
+  output but still deletes the copied credentials.
 - Tools are auto-approved for the run (Claude `dontAsk` with
   `mcp__idx__*`, Codex `default_tools_approval_mode="approve"`); Claude and pi
   also get no built-in tools, so they can only answer through MCP.
diff --git a/tests/agents/agent-smoke.sh b/tests/agents/agent-smoke.sh
index 7d5f4fc..f2a5957 100755
--- a/tests/agents/agent-smoke.sh
+++ b/tests/agents/agent-smoke.sh
@@ -59,6 +59,10 @@ while [[ $# -gt 0 ]]; do
 done
 
 [[ "$runs" =~ ^[1-9][0-9]*$ ]] || { echo "--runs must be a positive integer" >&2; exit 2; }
+if ((BASH_VERSINFO[0] < 4 || (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 4))); then
+    echo "agent-smoke: bash 4.4+ is required (run it inside nix develop)" >&2
+    exit 2
+fi
 for tool in jq cargo timeout; do
     command -v "$tool" >/dev/null || { echo "agent-smoke: $tool is required" >&2; exit 2; }
 done
@@ -77,7 +81,9 @@ done
 work="$(mktemp -d "${TMPDIR:-/tmp}/idx-agent-smoke.XXXXXX")"
 cleanup() {
     if [[ "$keep" == 1 ]]; then
-        echo "agent-smoke: kept $work"
+        # Never leave copied agent credentials behind.
+        rm -f -- "$work/codex/auth.json" "$work/pi/auth.json" "$work/hermes/auth.json" "$work/hermes/.env"
+        echo "agent-smoke: kept $work (copied credentials removed)"
     elif [[ "$work" == */idx-agent-smoke.* ]]; then
         rm -rf -- "$work"
     fi
@@ -187,7 +193,8 @@ setup_hermes() {
     copy_if_present "$src/auth.json" "$work/hermes"
     copy_if_present "$src/.env" "$work/hermes"
     {
-        [[ -f "$src/config.yaml" ]] && sed -n '/^model:/,/^[a-z_]*:/p' "$src/config.yaml" | sed '$d'
+        # The top-level `model:` block, up to the next top-level key.
+        [[ -f "$src/config.yaml" ]] && awk '/^[^[:space:]#]/ { in_model = /^model:/ } in_model' "$src/config.yaml"
         printf 'mcp_servers:\n  idx:\n    command: %s\n    enabled: true\n' "$launcher"
     } > "$work/hermes/config.yaml"
 }
@@ -242,11 +249,13 @@ check_run() {
               elif ($want.args // {}) as $args | ($matches | any(.args | contains($args))) | not
               then "\($want.name) called without \($want.args | tojson): \($matches | map(.args) | tojson)"
               else empty end
-          end'
+          end' || echo "could not parse the agent's tool calls (see $out.raw)"
     if [[ "$live" == 0 ]]; then
         local pattern
         while IFS= read -r pattern; do
-            grep -qP -- "$pattern" "$out.answer" || echo "answer does not match /$pattern/"
+            # jq regexes (Oniguruma) support (?i) on every platform, unlike grep -P.
+            jq -Rse --arg re "$pattern" 'test($re)' "$out.answer" >/dev/null \
+                || echo "answer does not match /$pattern/"
         done < <(jq -r '.expect_text[]?' <<< "$case_json")
     fi
 }
@@ -275,7 +284,9 @@ for agent in "${agents[@]}"; do
             : > "$out.calls"; : > "$out.answer"
             started=$SECONDS
             if timeout "$timeout_secs" bash -c "set -eo pipefail; $(declare -f "run_$agent"); $(declare -p work launcher exec_prefix); run_$agent \"\$1\" \"\$2\"" _ "$prompt" "$out" 2> "$out.stderr"; then
-                failures="$(check_run "$case_json" "$out")"
+                # Malformed agent output fails this run, not the whole matrix.
+                failures="$(check_run "$case_json" "$out")" \
+                    || failures="${failures:+$failures$'\n'}could not check the agent output (see $out.raw)"
             else
                 failures="agent exited with status $? (see $out.stderr)"
             fi