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 been inactive for 30 days. If the issue is still occurring, please comment to let us know. Otherwise, this issue will be automatically closed in 30 days for housekeeping purposes.
Closing for now — inactive for too long. Please open a new issue if this is still relevant.