Enhance silent-failure-hunter with subprocess stderr and exit code patterns

Resolved 💬 3 comments Opened Dec 18, 2025 by athola Closed Feb 14, 2026

Summary

During code review, three common patterns for silent failures were identified that aren't explicitly covered by the silent-failure-hunter agent in pr-review-toolkit:

Proposed Enhancements

1. Subprocess stderr Handling

Pattern: When subprocess calls fail, stderr is often discarded instead of being logged.

# ❌ Current - stderr lost on failure
result = subprocess.run(cmd, capture_output=True)
if result.returncode != 0:
    return False  # What went wrong? No one knows.

# ✅ Recommended - stderr reported
result = subprocess.run(cmd, capture_output=True, text=True)
if result.returncode != 0:
    if result.stderr:
        print(f"[ERROR] {cmd[0]}: {result.stderr.strip()}")
    return False

Impact: Users see "failed" with no information about why (network errors, auth failures, missing dependencies, etc.).

2. Script Exit Codes for CI/CD

Pattern: Scripts that process multiple items should exit non-zero when any operation fails.

# ❌ Current - always exits 0
def main():
    failed_items = process_all_items()
    print_summary(failed_items)
    # Implicit exit(0) even with failures!

# ✅ Recommended - proper exit code
def main():
    failed_items = process_all_items()
    print_summary(failed_items)
    if failed_items:
        sys.exit(1)

Impact: CI/CD pipelines can't detect failures since the script always exits successfully.

3. Silent Config/YAML Fallbacks

Pattern: Config parsing that silently returns defaults when structure is unexpected.

# ❌ Current - silent fallback
return yaml.safe_load(content) if isinstance(result, dict) else {}

# ✅ Recommended - logged fallback  
if not isinstance(result, dict):
    logger.warning(f"Expected dict in {file}, got {type(result).__name__}")
    return {}
return result

Impact: Users get empty configs with no indication their file structure was wrong.

Suggested Changes

Update agents/silent-failure-hunter.md section "### 4. Check for Hidden Failures" to include:

- Subprocess calls that discard stderr on failure
- Scripts that exit 0 even when operations fail (CI/CD compatibility)
- Config/YAML parsers that return defaults without logging unexpected structures

Context

These patterns were discovered during PR review and represent common real-world issues that cause debugging nightmares. The existing silent-failure-hunter agent covers try-catch and error handling well, but these subprocess/script-level patterns slip through.

View original on GitHub ↗

This issue has 3 comments on GitHub. Read the full discussion on GitHub ↗