Skip to content

fix(skills): harden UAT debug-bundle triage against malformed input #1914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
njhensley merged 2 commits into from
Jul 28, 2026
+36 −8
Merged

fix(skills): harden UAT debug-bundle triage against malformed input #1914

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8dc352b
fix(skills): harden UAT debug-bundle triage against malformed input
njhensley Jul 24, 2026
584882f
Merge branch 'main' into fix/uat-report-triage-robustness
njhensley Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 36 additions & 8 deletions .agents/skills/aicr-uat-report/uat_report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,31 @@ def failed_steps(repo, run_url):


def debug_artifacts(repo, run_id):
"""Return the run's non-expired debug artifacts, newest-largest first."""
"""Return every debug artifact on the run, expired ones included.

The caller filters for expiry so it can distinguish "never uploaded"
(infra failure before prep) from "aged out of retention" — different
classifications that a silent filter here would collapse into one.
"""
arts = gh_json(
["api", f"repos/{repo}/actions/runs/{run_id}/artifacts", "--jq", ".artifacts"]
)
return [a for a in arts if DEBUG_ARTIFACT_HINT in a.get("name", "")]


def slug(*parts):
"""Build a filesystem-safe directory name from run metadata.

Reservation names reach us through the workflow's run-name, and the title
regexes accept any non-whitespace token, so `svc`/`gpu` are not guaranteed
to be well-formed. Collapsing everything outside [a-z0-9-] removes both
separators and dot runs, so no component can traverse out of the download
directory.
"""
raw = "-".join(str(p) for p in parts).lower()
return re.sub(r"[^a-z0-9-]+", "-", raw).strip("-") or "unknown"
Comment on lines +123 to +133

@coderabbitai coderabbitai Bot Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add annotations to slug.

Ruff reports ANN002 for the untyped variadic parameter. Add parameter and return annotations.

Suggested fix 
-def slug(*parts):
+def slug(*parts: object) -> str:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def slug(*parts):
"""Build a filesystem-safe directory name from run metadata.
Reservation names reach us through the workflow's run-name, and the title
regexes accept any non-whitespace token, so `svc`/`gpu` are not guaranteed
to be well-formed. Collapsing everything outside [a-z0-9-] removes both
separators and dot runs, so no component can traverse out of the download
directory.
"""
raw = "-".join(str(p) for p in parts).lower()
return re.sub(r"[^a-z0-9-]+", "-", raw).strip("-") or "unknown"
def slug(*parts: object) -> str:
"""Build a filesystem-safe directory name from run metadata.
Reservation names reach us through the workflow's run-name, and the title
regexes accept any non-whitespace token, so `svc`/`gpu` are not guaranteed
to be well-formed. Collapsing everything outside [a-z0-9-] removes both
separators and dot runs, so no component can traverse out of the download
directory.
"""
raw = "-".join(str(p) for p in parts).lower()
return re.sub(r"[^a-z0-9-]+", "-", raw).strip("-") or "unknown"
🧰 Tools
🪛 Ruff (0.15.21)

[warning] 123-123: Missing type annotation for *parts

(ANN002)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.agents/skills/aicr-uat-report/uat_report.py around lines 123 - 133, Add
type annotations to the variadic parts parameter and return value of slug, using
types compatible with the existing str conversion and filesystem-safe string
result.

Source: Linters/SAST tools



def triage_digest(dest):
"""Summarize a downloaded bundle: manifest head + failing checks + inventory."""
lines = []
Expand All @@ -133,7 +151,11 @@ def triage_digest(dest):
if os.path.isfile(report):
try:
with open(report, encoding="utf-8") as fh:
tests = json.load(fh).get("results", {}).get("tests", [])
# `or {}` / `or []` rather than .get(k, default): a truncated
# report can carry an explicit null, which a default does not
# catch. Nothing here may raise — triage_digest is called
# per-run, so one bad file must not abort the whole download.
tests = (json.load(fh).get("results") or {}).get("tests") or []
bad = [t for t in tests if t.get("status") in ("failed", "other")]
lines.append(
f" report.json: {len(bad)}/{len(tests)} checks failing"
Expand All @@ -143,7 +165,7 @@ def triage_digest(dest):
else ""
)
)
except (OSError, ValueError, AttributeError):
except (OSError, ValueError, AttributeError, TypeError):
lines.append(" report.json present but unparseable")
# Presence, not a full listing: which top-level artifacts the run produced
# (is there a train-logs/? did evidence emit?) is the actionable part.
Expand Down Expand Up @@ -179,7 +201,7 @@ def download_debug(repo, failures, outdir, limit):
run_id = run_id_of(f["url"])
label = f"{f['svc']}/{f['gpu']}/{f['intent']} {f['ts']}Z run {run_id}"
dest = os.path.join(
outdir, f"{f['svc']}-{f['gpu']}-{f['intent']}-{run_id}".lower()
outdir, slug(f["svc"], f["gpu"], f["intent"], run_id)
)
try:
arts = debug_artifacts(repo, run_id)
Expand All @@ -202,12 +224,18 @@ def download_debug(repo, failures, outdir, limit):
)
continue
for art in live:
# One run normally carries one bundle, but a caller that fans out
# several cloud jobs under a single run_id would produce more.
# Unpacking those into a shared directory would silently merge two
# clusters' state and make the digest describe neither, so give
# each artifact its own subdirectory once there is more than one.
art_dest = dest if len(live) == 1 else os.path.join(dest, slug(art["name"]))
mb = art.get("size_in_bytes", 0) / 1e6
os.makedirs(dest, exist_ok=True)
os.makedirs(art_dest, exist_ok=True)
try:
subprocess.run(
["gh", "run", "download", run_id, "-R", repo,
"-n", art["name"], "-D", dest],
"-n", art["name"], "-D", art_dest],
capture_output=True, text=True, timeout=600, check=True,
)
except subprocess.SubprocessError as err:
Expand All @@ -217,8 +245,8 @@ def download_debug(repo, failures, outdir, limit):
f"({stderr or err})"
)
continue
print(f"- {label}: {art['name']} ({mb:.1f} MB) -> {dest}")
print("\n".join(triage_digest(dest)))
print(f"- {label}: {art['name']} ({mb:.1f} MB) -> {art_dest}")
print("\n".join(triage_digest(art_dest)))
print()


Expand Down
Loading