Enhance silent-failure-hunter with subprocess stderr and exit code patterns
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.
This issue has 3 comments on GitHub. Read the full discussion on GitHub ↗