[spark-compete] Narrow bare except in git_summary + run_git subprocess helpers in system_map.py to (subprocess.SubprocessError, OSError)

[4gjnbzb4zf-sudo]

{
"schema": "spark-compete-hotfix-v1",
"event": "spark-compete-first-event",
"submission_mode": "public_repo_pr",
"submission_target_url": "#534",
"team": {
"name": "SparkThisUp",
"members": ["ValHallaBuilder", "Baz707", "DanFireDash"],
"github_accounts": ["4gjnbzb4zf-sudo"],
"llm_device_holder": "ValHallaBuilder",
"device_holder_github": "4gjnbzb4zf-sudo"
},
"target_repo": {
"id": "vibeforge1111/spark-cli",
"source": "https://github.com/vibeforge1111/spark-cli",
"owner_surface": "spark-cli"
},
"issue": {
"type": "bug",
"severity": "low",
"title": "git_summary and run_git subprocess helpers in system_map.py swallow programming bugs as generic Exception",
"actual_behavior": "src/spark_cli/system_map.py exposes two paired subprocess wrappers used across the system-map pipeline to capture per-repo git head info: git_summary(path) at L411 and run_git(path, args) at L427. Both invoke subprocess.run([...], check=False, timeout=N) and wrap the call in except Exception: return {available:True, head_short:None} / return 1, \"\". With check=False on subprocess.run, the only exceptions that call can raise are subprocess.TimeoutExpired (a SubprocessError subclass) when the timeout fires and FileNotFoundError/PermissionError (OSError subclasses) when git is missing or unexecutable. The bare Exception folds any unrelated runtime error (AttributeError on a typo, NameError from a refactor, KeyboardInterrupt during a long scan) into the same silent return -- the operator never finds out whether git was unavailable, the repo was timing out, or the helper itself broke.",
"expected_behavior": "The same silent-return short-circuit happens only for the documented failure modes: subprocess timeout (subprocess.SubprocessError) and missing/unexecutable git binary (OSError). Programming bugs propagate naturally so the operator sees them as a real traceback.",
"repro_steps": [
"gh pr checkout ",
"PYTHONPATH=src python -c "from spark_cli.system_map import git_summary, run_git; from pathlib import Path; import tempfile; d = Path(tempfile.mkdtemp()); print('non-git:', git_summary(d)); print('bogus:', run_git(Path('/'), ['rev-parse','--short','HEAD'], timeout=2))" -> non-git: {'available': False}; bogus: (128, '')",
"PYTHONPATH=src python -c "import subprocess; print(issubclass(subprocess.TimeoutExpired, subprocess.SubprocessError), issubclass(FileNotFoundError, OSError))" -> True True (narrow tuple is exhaustive)",
"Inverse: edit a typo into the subprocess.run call (e.g. subprocess.runZZZ([...])) -- before the change, the AttributeError gets swallowed and the helper silently returns head_short=None / (1, ''). After the change, the AttributeError propagates so the operator sees the real bug."
],
"affected_workflow": "system_map.py is the system-map compile module driven by spark report system-map. git_summary and run_git are called across the per-repo board snapshot pipeline. When the wrappers silently swallow a programming bug, downstream system-map output lies about git head state -- the operator sees head_short: null and assumes git is missing when really a code bug in the wrapper hid the real failure."
},
"evidence": {
"safe_links_only": true,
"before_after_proof": "Two paired try/except patterns. Before: except Exception: return {available:True, head_short:None} and except Exception: return 1, ''. After: except (subprocess.SubprocessError, OSError): return {available:True, head_short:None} and except (subprocess.SubprocessError, OSError): return 1, ''. The except bodies are byte-identical -- only the catch tuple changes. subprocess.SubprocessError is the base class for TimeoutExpired (the only exception subprocess.run with check=False can raise from the run itself); OSError covers FileNotFoundError and PermissionError raised when the git binary is missing or unexecutable. Diff: 2 insertions, 2 deletions, one file.",
"links": ["https://github.com//pull/534"],
"forbidden": ["pdf","zip","exe","unknown downloads","shortened links","archives","binaries","tokens","browser cookies","wallet material","raw logs","raw conversations","raw memory","raw patches","private repo maps","private scoring details"]
},
"proposed_fix": {
"approach": "Replace except Exception: with except (subprocess.SubprocessError, OSError): in both wrappers. The narrow tuple is exhaustive for subprocess.run with check=False -- TimeoutExpired (via SubprocessError) when the timeout fires, FileNotFoundError/PermissionError (via OSError) when the binary is missing. The catch tuple is the only thing that changes; the except body is byte-identical.",
"files_expected": ["src/spark_cli/system_map.py"],
"tests_or_smoke": "Manual smoke verified: non-git directory still returns {available:False} early (path check before subprocess); a real subprocess call with bogus args returns (128, '') as expected; TimeoutExpired and FileNotFoundError both flow through the narrow tuple; AttributeError propagates past it. AST parse of system_map.py is clean."
},
"pr": {
"branch": "sentinel/exception-narrow/system-map-git-subprocess-helpers",
"title_prefix": "[spark-compete]",
"author_github": "4gjnbzb4zf-sudo",
"body_must_include": ["packet","team","pr_author","repo","actual_behavior","expected_behavior","repro_steps","before_after_proof","tests_or_smoke","duplicate_notes","risk_notes","review_claim"],
"url": "#534"
},
"review_claim": {
"impact_claim": "low",
"evidence_types": ["redacted_terminal_excerpt"],
"duplicate_notes": "Pre-flight gh pr list --search 'git_summary run_git narrow' returned zero hits. Our own #526 narrowed six sqlite-introspection helpers in this same file, pending #532 narrows read_json/read_toml at L306/L314, pending #533 narrows the inspect_spawner_prd_auto_trace JSONL scanner at L865-L887. git_summary and run_git are at L411 and L427, distinct from all of those. No competitor PR touches these helpers. Same shape as our own #164 (whoami subprocess wrapper in spark-intelligence-builder), which narrows to (CalledProcessError, FileNotFoundError, OSError) -- this PR uses (SubprocessError, OSError) because check=False means CalledProcessError is unreachable here.",
"risk_notes": "Local scope: one file, two lines changed. The except bodies are byte-identical pre/post for both wrappers. The non-git-directory fast-path is unchanged. Subprocess timeout and missing-binary conditions still trigger the documented silent-return short-circuit. The only observable behavior change is that programming bugs in the wrappers no longer get silently swallowed -- the goal of the fix.",
"review_state_requested": "pr_review"
}
}

[4gjnbzb4zf-sudo]

QA write-up — for reviewer eyes

TL;DR. git_summary (L411) and run_git (L427) in src/spark_cli/system_map.py are paired subprocess wrappers used across the system-map pipeline to capture per-repo git head info. Both invoke subprocess.run([...], check=False, timeout=N) and wrap the call in except Exception:. With check=False, the only exceptions subprocess.run can raise are subprocess.TimeoutExpired (a SubprocessError subclass) when the timeout fires and FileNotFoundError/PermissionError (OSError) when the git binary is missing. The bare Exception folds programming bugs into the same silent return. Narrow each to (subprocess.SubprocessError, OSError).

Before

def git_summary(path: Path) -> dict[str, Any]:
    if not (path / ".git").exists():
        return {"available": False}
    try:
        result = subprocess.run(
            ["git", "-C", str(path), "rev-parse", "--short", "HEAD"],
            capture_output=True, text=True, timeout=2, check=False,
        )
    except Exception:
        return {"available": True, "head_short": None}
    ...


def run_git(path: Path, args: list[str], timeout: int = 3) -> tuple[int, str]:
    try:
        result = subprocess.run(
            ["git", "-C", str(path), *args],
            capture_output=True, text=True, timeout=timeout, check=False,
        )
    except Exception:
        return 1, ""
    ...

If a typo lands on the subprocess.run call, the AttributeError gets swallowed and the helper silently returns head_short: null / (1, '') — the operator looks at the system-map output, sees no git head, assumes git is missing, and the real bug never surfaces.

After

    except (subprocess.SubprocessError, OSError):
        return {"available": True, "head_short": None}

    except (subprocess.SubprocessError, OSError):
        return 1, ""

Same silent-return short-circuit for the documented failure modes (subprocess timeout, missing or unexecutable git binary). Programming bugs propagate so the operator gets a real traceback.

Smoke

$ PYTHONPATH=src python3 -c "
import subprocess
from spark_cli.system_map import git_summary, run_git
from pathlib import Path
import tempfile

d = Path(tempfile.mkdtemp())
print('non-git:', git_summary(d))
print('bogus repo:', run_git(Path('/'), ['rev-parse','--short','HEAD'], timeout=2))
print('TimeoutExpired is SubprocessError:', issubclass(subprocess.TimeoutExpired, subprocess.SubprocessError))
print('FileNotFoundError is OSError:', issubclass(FileNotFoundError, OSError))

try:
    raise AttributeError('typo')
except (subprocess.SubprocessError, OSError):
    print('FAIL')
except AttributeError:
    print('OK: AttributeError propagates past narrow tuple')
"
non-git: {'available': False}
bogus repo: (128, '')
TimeoutExpired is SubprocessError: True
FileNotFoundError is OSError: True
OK: AttributeError propagates past narrow tuple

What didn't change

One file, two lines changed. Except bodies byte-identical pre/post for both wrappers. The non-git-directory fast-path in git_summary is unchanged. Subprocess timeout and missing-binary failure modes still trigger the documented silent-return short-circuit. The only observable behavior change is that programming bugs no longer get silently swallowed — the goal of the fix.

Pattern-match: same shape as our own #164 (whoami subprocess wrapper in spark-intelligence-builder), narrower because check=False means CalledProcessError is unreachable.

[vibeforge1111]

Thanks, this was useful. We maintainer-adopted this exception-boundary improvement in #609 and merged it into master at 5e60f9a12c40438779a4aa1e0b9293e1fed7ce72.

What shipped:

• JSON/TOML helpers now catch parse and OS errors instead of broad Exception.
• Spawner PRD trace scanning catches line JSON decode failures and file OS failures at the right boundary.
• git helper subprocess failures catch expected subprocess/OS errors.
• revoke-all active mission pause write failures catch expected OS write errors.

Verification on the adopted PR:

• focused system-map and revoke-all tests passed
• full tests/test_system_map.py passed
• full tests/test_cli.py passed
• compileall, git diff --check, registry-pin verification, CodeQL, scorecard, secret-scan, and test-and-audit all passed

Source credit is recorded for this PR, and #609 itself should not receive participant credit. Points are still gated because this PR body had spark-compete-hotfix-v1 schema text but not a clean fenced JSON packet that the validator can extract. To unlock points review, add a clean fenced JSON packet in a PR comment or body update, then validate it at https://compete.sparkswarm.ai/api/packet/validate.

Closing this PR as adopted into #609. No code action is needed unless you believe #609 missed part of the issue.

[vibeforge1111]

R22/R23 final scoring update: this PR did receive final R22/R23 public leaderboard credit. Earlier gate, credit-review, or points-lock wording described the pre-final review state, not a final rejection or permanent zero. Final R22/R23 credit for this PR: 48 points. Thanks for the contribution.