Repository navigation
Add root-cause diagnostics to generic failure issues - #67471
Conversation
…sues Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
|
🔎 PR Code Quality Reviewer is reviewing code quality for this pull request... |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. No ADR enforcement needed for PR #67471: the PR does not carry the "implementation" label and has 0 new lines in default business logic directories (threshold 100, no custom .design-gate.yml). Evidence: /tmp/gh-aw/agent/adr-prefetch-summary.json (has_implementation_label=false, default_business_additions=0, requires_adr_by_default_volume=false, file_count=2).
|
There was a problem hiding this comment.
🟡 Changes recommended
Diagnostics can contradict captured logs and falsely report unavailable attribution for successful infrastructure-error runs.
1 open finding
What changed in this PR
Adds root-cause diagnostics to generic Agent failure issues.
Changes:
- Captures bounded, redacted failing-step log excerpts.
- Adds attribution fallbacks when job data is unavailable.
- Expands tests for truncation, masking, API failures, and rendering.
| File | Description |
|---|---|
actions/setup/js/handle_agent_failure.cjs |
Collects and renders failure diagnostics. |
actions/setup/js/handle_agent_failure.test.cjs |
Tests diagnostic collection and issue output. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /tdd. This is a well-scoped, well-tested change — approving with one minor efficiency note.
📋 Key Themes & Highlights
Key Themes
- Minor perf nit:
getFailedAgentDiagnosticsredacts the log excerpt twice in a row (see inline comment) — not a bug, just avoidable duplicate work on a hot path.
Positive Highlights
- ✅
/tdd: Excellent test coverage — boundary conditions (failure-completion-second overlap), binary log formats (ArrayBuffer/Uint8Array), secret redaction across normalization layers, fence-injection prevention, and the dependency-results fallback are all exercised with clear, descriptive test names. - ✅
/codebase-design: The refactor from a bare string return to a structured{failingStep, logExcerpt, attributionUnavailable, logUnavailable}result is a clean, backward-compatible extension — call sites use spread (...failureDiagnostics) sobuildFailureDiagnosticsContextdidn't need restructuring. - ✅ Correctly reuses the existing
redactAndBoundDiagnostics/renderErrorDetails/collectAddMaskedValuesinfrastructure instead of inventing new sanitization logic, keeping the module's security invariants consistent. - ✅ Secret masks declared after the failing step's log window are still picked up via
collectAddMaskedValues(log)scanning the whole log, closing a real gap.
I reviewed /tmp/gh-aw/agent/pr-diff.patch directly (421 lines, 2 files) rather than invoking pr-triage, since the diff was small and self-contained enough for direct review.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 78.6 AIC · ⌖ 14.5 AIC · ⊞ 10.3K
Comment /matt to run again
|
@copilot must be privacy preserving and agent generated strings must be validated Z |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in |
Use Actions job metadata and validated driver exit codes only. Omit report_incomplete prose and log excerpts from generic failure issues and comments. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Group job, step, and driver metadata in one diagnostics section. Collapse attribution explanations, shorten incompletion guidance, and retain the explicit agent-text exclusion. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Generic failure issues often lack enough detail to triage. Missing Actions job data also leaves failures unattributed without explaining why.