Skip to content

fix: prevent SSH error pattern false positives on agent response text#794

Merged
pedramamini merged 1 commit intorcfrom
fix/answer-parsing
Apr 12, 2026
Merged

fix: prevent SSH error pattern false positives on agent response text#794
pedramamini merged 1 commit intorcfrom
fix/answer-parsing

Conversation

@chr1syy
Copy link
Copy Markdown
Contributor

@chr1syy chr1syy commented Apr 11, 2026

Summary

  • StdoutHandler: SSH error patterns are now only checked against non-JSON lines. Valid JSONL lines contain structured agent output (assistant messages, results) whose text content could false-positive match patterns like bash:.*opencode.*command not found when the agent quoted shell commands or error patterns in its response.
  • ExitHandler: SSH error check at exit now only scans stderr, not the combined stdout+stderr. Stdout contains all accumulated JSONL which includes the full agent response — scanning it caused false positives when response text mentioned SSH error keywords.
  • Added test coverage for SSH error pattern false-positive prevention in both handlers.

Root Cause

When running over SSH (sshRemoteId set), error patterns were matched against raw JSONL lines and combined output buffers. The .* wildcards in patterns like /bash:.*opencode.*command not found/i matched across the JSON structure, hitting keywords embedded in the agent's response text. This caused the entire response to be classified as an SSH agent_crashed error.

Test plan

  • Scoped tests pass (61/61)
  • Type check passes (pre-existing errors unrelated to changes)
  • Prettier clean
  • Verify SSH sessions no longer false-positive on agent responses containing error keywords
  • Verify real SSH errors (e.g., missing binary on remote) are still correctly detected via stderr

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced SSH error detection accuracy to reduce false positives
    • Fixed issue where JSON-formatted responses could incorrectly trigger SSH error detection
  • Tests

    • Added comprehensive test coverage for SSH error pattern detection and false-positive prevention scenarios
    • Verified proper handling of SSH-related errors across different output types

SSH error patterns (e.g., "command not found") were matched against raw
JSONL stdout lines and combined stdout+stderr at exit. When an agent's
response text quoted shell commands or error patterns, the regex would
match across the JSON structure and incorrectly classify the entire
response as an SSH error.

- StdoutHandler: skip SSH pattern check when line parses as valid JSON
- ExitHandler: check only stderr for SSH patterns, not stdout (which
  contains structured JSONL agent output)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3af4920c-6351-4f40-9fc7-5e65d9e62bd8

📥 Commits

Reviewing files that changed from the base of the PR and between b34f55a and ac704c9.

📒 Files selected for processing (4)
  • src/__tests__/main/process-manager/handlers/ExitHandler.test.ts
  • src/__tests__/main/process-manager/handlers/StdoutHandler.test.ts
  • src/main/process-manager/handlers/ExitHandler.ts
  • src/main/process-manager/handlers/StdoutHandler.ts

📝 Walkthrough

Walkthrough

The changes refine SSH error detection logic in the process manager's handlers to prevent false positives. SSH pattern matching now targets only stderr content in the exit handler, and only processes non-JSON stdout lines in the stdout handler, eliminating erroneous pattern matches against JSON responses or stdout-only content.

Changes

Cohort / File(s) Summary
SSH Error Detection Tests
src/__tests__/main/process-manager/handlers/ExitHandler.test.ts, src/__tests__/main/process-manager/handlers/StdoutHandler.test.ts
Added comprehensive test suites validating SSH error pattern matching isolation. Covers scenarios where stdout contains SSH-like keywords but stderr is harmless, confirming that SSH detection only triggers on stderr content (ExitHandler) and non-JSON lines (StdoutHandler).
SSH Error Detection Implementation
src/main/process-manager/handlers/ExitHandler.ts, src/main/process-manager/handlers/StdoutHandler.ts
Modified SSH pattern matching logic to use only stderrBuffer in exit handler and only process non-JSON stdout lines in stdout handler. Removed stdout/combined output inclusion and adjusted logging to reflect stderr-only evaluation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Through stderr's careful, focused gaze,
No JSON tricks or stdout haze!
False alarms now fade away,
SSH errors caught at last, hooray! 🐇✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: preventing false positives in SSH error pattern detection when processing agent response text.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/answer-parsing

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 11, 2026

Greptile Summary

This PR fixes false-positive SSH error detection by scoping pattern matching to non-JSON stdout lines in StdoutHandler and to stderrBuffer-only in ExitHandler. The root cause was that .*-wildcarded patterns like /bash:.*opencode.*command not found/i matched across JSON structure when scanned against raw JSONL lines or the combined stdout+stderr buffer, misclassifying legitimate agent responses as SSH agent_crashed errors.

Confidence Score: 5/5

Safe to merge — the fix is correct, well-scoped, and backed by new test coverage.

All findings are P2 or lower. The core logic change is sound: SSH transport errors (bash 'command not found', connection drops) always come from the shell's stderr, never from JSONL agent responses on stdout. The parsed === null guard in StdoutHandler and the stderr-only check in ExitHandler correctly target the actual signal while eliminating the false-positive source. Batch-mode behavior is unchanged since stdoutBuffer was already empty in that path. Five new tests validate both prevention and positive-detection cases.

No files require special attention.

Important Files Changed

Filename Overview
src/main/process-manager/handlers/StdoutHandler.ts Added parsed === null guard to SSH pattern check in processLine — skips JSON lines, preserving detection on plain-text non-JSON lines where real SSH errors appear.
src/main/process-manager/handlers/ExitHandler.ts Removed stdoutBuffer/streamedText from SSH error check at exit, using only stderrBuffer; also removed stale stdout-related log fields. Logic is sound for all modes.
src/tests/main/process-manager/handlers/StdoutHandler.test.ts Added two SSH false-positive prevention tests: one verifying matchSshErrorPattern is not called on valid JSON lines, another verifying it is called on non-JSON lines.
src/tests/main/process-manager/handlers/ExitHandler.test.ts Added three SSH false-positive prevention tests covering stderr-only checking, no false-positive on JSONL stdout, and positive detection from real stderr errors.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[stdout line received] --> B{isStreamJsonMode?}
    B -- No batch mode --> C[Accumulate in jsonBuffer\nno SSH check during stream]
    B -- Yes --> D[JSON.parse line]
    D --> E{parsed !== null?}
    E -- Yes valid JSON --> F[Agent parser: detectErrorFromParsed\nSSH check SKIPPED ← fix]
    E -- No plain text --> G[Agent parser: detectErrorFromLine\nSSH matchSshErrorPattern line]
    G --> H{SSH pattern match?}
    H -- Yes --> I[emit agent-error]
    H -- No --> J[emit as data]

    subgraph ExitHandler at process exit
        K{errorEmitted?} -- No --> L{sshRemoteId set AND\ncode≠0 OR stderrBuffer?}
        L -- Yes --> M[matchSshErrorPattern stderrBuffer ONLY ← fix]
        M --> N{match?}
        N -- Yes --> O[emit agent-error]
        N -- No --> P[log warn if code≠0]
    end

    style F fill:#f9c,stroke:#c66
    style M fill:#f9c,stroke:#c66
Loading

Reviews (1): Last reviewed commit: "fix: prevent SSH error pattern false pos..." | Re-trigger Greptile

@chr1syy chr1syy added the ready to merge This PR is ready to merge label Apr 11, 2026
@pedramamini pedramamini merged commit 365d3c6 into rc Apr 12, 2026
5 checks passed
@pedramamini pedramamini deleted the fix/answer-parsing branch April 12, 2026 20:02
chr1syy added a commit to chr1syy/Maestro that referenced this pull request Apr 16, 2026
…RunMaestro#794)

SSH error patterns (e.g., "command not found") were matched against raw
JSONL stdout lines and combined stdout+stderr at exit. When an agent's
response text quoted shell commands or error patterns, the regex would
match across the JSON structure and incorrectly classify the entire
response as an SSH error.

- StdoutHandler: skip SSH pattern check when line parses as valid JSON
- ExitHandler: check only stderr for SSH patterns, not stdout (which
  contains structured JSONL agent output)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge This PR is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants