[MEDIUM] summary/core.py: _shell_runner silently discards non-zero subprocess exit #9

New issue

Closed

opened 2026-05-19 04:42:47 +00:00 by foravo_admin · 1 comment

foravo_admin commented

2026-05-19 04:42:47 +00:00

Owner

Imported from GitHub issue M00C1FER/mesh-review#4.

Source: https://github.com/M00C1FER/mesh-review/issues/4
Original author: @M00C1FER
Original state: closed

Finding 3 — MEDIUM

File: src/mesh_review/summary/core.py
Function: _shell_runner()
Commit fixing this: 834f8d8441226ac383868a9cb8ac5c69b650581e (PR #1)

Description

Identical to the same issue in review/core.py (Finding 2). After subprocess.run(), proc.returncode is never checked:

proc = subprocess.run(
    cmd + [full_prompt], capture_output=True, text=True, timeout=timeout
)
# returncode never checked — error output treated as LLM output
return _parse_summary(cli, proc.stdout)

A non-zero exit feeds stderr/empty stdout into _parse_summary, silently producing a malformed SummaryDoc with no error indicated.

Impact

Silent failure for all error conditions: auth failures, missing model, rate limits, etc.
The SummaryDoc.error field exists precisely to communicate this, but it is never populated on subprocess error.

Fix Applied

if proc.returncode != 0:
    return SummaryDoc(cli=cli, error=f"{cli} exited {proc.returncode}: {proc.stderr.strip()[:300]}")

Imported from GitHub issue `M00C1FER/mesh-review#4`. Source: https://github.com/M00C1FER/mesh-review/issues/4 Original author: @M00C1FER Original state: closed  --- ## Finding 3 — MEDIUM **File:** `src/mesh_review/summary/core.py` **Function:** `_shell_runner()` **Commit fixing this:** 834f8d8441226ac383868a9cb8ac5c69b650581e (PR #1) ### Description Identical to the same issue in `review/core.py` (Finding 2). After `subprocess.run()`, `proc.returncode` is never checked: ```python proc = subprocess.run( cmd + [full_prompt], capture_output=True, text=True, timeout=timeout ) # returncode never checked — error output treated as LLM output return _parse_summary(cli, proc.stdout) ``` A non-zero exit feeds stderr/empty stdout into `_parse_summary`, silently producing a malformed `SummaryDoc` with no error indicated. ### Impact - Silent failure for all error conditions: auth failures, missing model, rate limits, etc. - The `SummaryDoc.error` field exists precisely to communicate this, but it is never populated on subprocess error. ### Fix Applied ```python if proc.returncode != 0: return SummaryDoc(cli=cli, error=f"{cli} exited {proc.returncode}: {proc.stderr.strip()[:300]}") ```

foravo_admin

2026-05-19 04:42:47 +00:00

closed this issue
added the
code-review
label

foravo_admin commented

2026-05-19 04:42:47 +00:00

Author

Owner

Imported from GitHub issue comment M00C1FER/mesh-review#4:4362220454.

Source: https://github.com/M00C1FER/mesh-review/issues/4#issuecomment-4362220454
Original author: @M00C1FER

Fixed by merged PR #1.

Imported from GitHub issue comment `M00C1FER/mesh-review#4:4362220454`. Source: https://github.com/M00C1FER/mesh-review/issues/4#issuecomment-4362220454 Original author: @M00C1FER  --- Fixed by merged PR #1.