[sharpca]

Summary

• fetch_issues.py retried nothing: a single transient HTTP 502 from the API
  became a SystemExit, and since this fetch is the first of six steps the
  operator had to restart the whole dashboard run by hand. run_gh now retries
  the failures that can clear on their own (5xx, gateway timeouts, connection
  resets) with exponential backoff, and still fails fast on an auth error, a bad
  repo slug, or an unknown flag.
• The default cache and output paths no longer hardcode /tmp. A sandboxed run
  is granted its own scratch directory and cannot write /tmp at all, so the
  first fetch step failed before it ever reached the API. Both defaults now
  derive from tempfile.gettempdir() ($TMPDIR on POSIX, %TEMP% on Windows);
  TRACKER_STATS_CACHE and TRACKER_STATS_OUT still win when set.
• README, run.sh, the skill prose, the adopter template, and the step-2 eval
  system prompt follow the new default.

Type of change

• Python package (tools/*/ with pyproject.toml)
• Documentation (docs/, README.md, CONTRIBUTING.md)
• Skill change — prose only. The tracker-stats-dashboard SKILL.md
  restates the cache default; no behaviour change, so no new fixture.

Test plan

• For Python packages touched: uv run pytest / ruff check / mypy passes
  • pytest: 149 passed (142 existing + 7 new in tests/test_fetch_retry.py)
  • ruff check . and ruff format --check .: clean
  • mypy: no issues in 6 source files
• New tests in tests/test_fetch_retry.py cover transient-status detection,
  retry-then-succeed with a doubling backoff, give-up after the attempt
  limit, no retry on a 401, and both branches of the cache default.

RFC-AI-0004 compliance

• Sandbox — removes two writes that fell outside the sandbox's writable
  paths (/tmp/tracker-stats-cache, /tmp/airflow_s_monthly.html); no new
  host access or network reach is introduced.

Linked issues

Closes #1744

Notes for reviewers

• TRACKER_STATS_OUT was not named in the issue, but it hardcoded /tmp for the
  same reason and would have kept the run failing after the cache was fixed.
  Happy to split it out if you would rather keep this PR to the cache alone.
• The eval fixtures' report.md files keep Cache directory: /tmp/...: on a
  POSIX host without TMPDIR, tempfile.gettempdir() is /tmp, so that output
  is still exactly what the new default produces.
• Only 5xx / gateway-timeout / connection-reset count as transient. A 403
  secondary-rate-limit is deliberately not retried — it needs the operator to
  read the message, not a shorter backoff.

Generative AI disclosure

This change was authored with an AI coding agent (WorkBuddy AI, DeepSeek-V4.1-Flash)
under human review. The commit carries the Generated-by: trailer; the
contributor has reviewed the diff and owns the change.

[Kaap10]

Thanks for addressing #1744, @sharpca! Moving the cache defaults out of hardcoded /tmp to tempfile.gettempdir() solves the sandbox write restriction nicely, and introducing exponential backoff on 5xx transient errors makes the pipeline much more resilient. The test suite in test_fetch_retry.py is clean and covers the retry states well.

A few findings from review to address before this can land:

---

1. SKILL.md frontmatter drift (surface_hash & measured_tokens)

File: plugins/magpie-security/skills/tracker-stats-dashboard/SKILL.md (lines 19–21)

Because prose in Step 2 was modified (lines 210–212), the surface_hash and measured_tokens in the frontmatter are out of date:

• In this repository, prek runs skill-surface-hash and check-measured-tokens, which will fail CI once the workflows run on this PR.
• Fix: Running prek run --all-files (or updating the hash with the helper scripts) will automatically recalculate and update these frontmatter stamps.

---

2. Unhandled subprocess.TimeoutExpired in run_gh

File: tools/security-tracker-stats-dashboard/fetch_issues.py (lines 70–75)

In fetch_issues.py:

    for attempt in range(1, attempts + 1):
        r = subprocess.run(args, capture_output=True, text=True, timeout=timeout)
        if r.returncode == 0:
            return r

_TRANSIENT_FAILURE regex includes "timed out", which matches if gh itself outputs a timeout to stderr. However, if the process hits the 300-second subprocess limit, subprocess.run raises subprocess.TimeoutExpired rather than returning a CompletedProcess with non-zero exit code. Because this exception is unhandled, it will terminate execution immediately without retrying.

Suggested fix: Catch subprocess.TimeoutExpired so it participates in the retry loop:

    for attempt in range(1, attempts + 1):
        try:
            r = subprocess.run(args, capture_output=True, text=True, timeout=timeout)
        except subprocess.TimeoutExpired as exc:
            if attempt == attempts:
                raise SystemExit(f"gh timed out after {timeout}s: {exc}")
            delay = base_delay * 2 ** (attempt - 1)
            print(
                f"gh timed out (attempt {attempt}/{attempts}), retrying in {delay:g}s",
                file=sys.stderr,
            )
            time.sleep(delay)
            continue

        if r.returncode == 0:
            return r

---

3. Path consistency in run.sh

File: tools/security-tracker-stats-dashboard/run.sh (line 58)

echo "done: ${TRACKER_STATS_OUT:-${TMPDIR:-/tmp}/airflow_s_monthly.html}"

render.py resolves the path via Python's tempfile.gettempdir(), which resolves to %TEMP% on Windows, whereas bash's ${TMPDIR:-/tmp} defaults to /tmp. On non-POSIX environments, the completion message will point to /tmp while the file actually lives under %TEMP%. Not a blocker for POSIX hosts, but worth keeping aligned.

[potiuk]

Retrying the transient gh failures and moving the defaults off a hard-coded /tmp are both right, and the new tests cover the retry paths well.

I pushed one fixup on top (4c2667f) for the remaining points, including the three raised inline earlier: run_gh moves to gh_retry.py and fetch_roster.py uses it too, so a single 502 on the roster fetch no longer aborts the run; a call that hits its timeout is retried like a 504 instead of escaping as TimeoutExpired; run.sh no longer guesses the output path, since render.py already prints the one it wrote; and measured_tokens is restamped. There are three new tests in test_fetch_retry.py. I also merged main in. Please pull before pushing anything else to the branch.

Thank you for your first contribution!

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The findings
below are observations, not blockers; an Apache Magpie
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.

More on how Apache Magpie handles maintainer review:
Contributing guide.