feat(#704): surface agent errors in workflow logs#764
feat(#704): surface agent errors in workflow logs#764fullsend-ai-coder[bot] wants to merge 2 commits into
Conversation
When an agent run fails, the root cause error is buried in the transcript JSONL artifact. Operators must download and manually search the artifact to diagnose failures. This adds two improvements: 1. In-process error surfacing (run.go): After transcript extraction, parse the JSONL for result events with is_error=true and emit ::error:: annotations to stderr. These appear in the GitHub Actions job summary, making failures diagnosable without downloading artifacts. 2. Standalone extraction script (extract-transcript-error.sh): A shell script that extracts errors from transcript files or directories. Usable by post-scripts, the triage agent (after downloading artifacts via gh run download), and operators debugging failed runs. The transcript parser is intentionally lightweight: it scans only for result-type lines (fast string contains check) and parses the last one. Error messages are truncated to 2000 chars and sanitized to prevent GHA command injection. New files: - internal/cli/transcript.go: JSONL parsing and error emission - internal/cli/transcript_test.go: 15 unit tests - internal/scaffold/.../extract-transcript-error.sh: standalone tool Modified: - internal/cli/run.go: call extractTranscriptErrors after iteration loop when agent exits non-zero Closes #704 Signed-off-by: fullsend-code <fullsend-code@users.noreply.github.com>
|
fullsend review is working on this — view logs |
Site previewPreview: https://c7b29a71-site.fullsend-ai.workers.dev Commit: |
Review: #764Head SHA: 1a4e166 SummaryThis PR adds well-structured transcript error surfacing for agent failures, directly addressing issue #704. The Go implementation ( FindingsMediumNone. Low
Info
FooterOutcome: approve Previous runReview: #764Head SHA: b8e7569 SummaryClean, well-scoped change that adds transcript error surfacing to GitHub Actions workflow logs. The Go implementation is solid — proper use of FindingsMedium
Low
Info
FooterOutcome: approve |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/u/271249281?s=60&v=4){kind=small}
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
Well-scoped, useful feature that directly addresses #704. The Go implementation is solid overall — clean separation of concerns, good test coverage, and correct integration point in run.go. Five findings below, the most important being unsanitized values interpolated into GHA workflow commands (filename and subtype) and inconsistent sanitization in the shell script.
| // agent failures diagnosable without downloading artifacts. | ||
| func emitTranscriptErrors(w io.Writer, summaries []transcriptErrorSummary) { | ||
| for _, s := range summaries { | ||
| // Sanitize the error message to prevent GHA command injection. |
There was a problem hiding this comment.
[critical] s.Source (derived from the JSONL filename via filepath.Base) is interpolated directly into the ::error:: workflow command without sanitization. Only msg goes through sanitizeOutput. A crafted filename containing %0A or :: sequences could inject additional GHA commands.
The fallback on line 142 has the same issue with s.Subtype.
Fix:
fmt.Fprintf(w, "::error title=Agent Error (%s)::%s\n", sanitizeOutput(s.Source), msg)And for the fallback:
msg = fmt.Sprintf("agent terminated with error (subtype: %s)", sanitizeOutput(s.Subtype))|
|
||
| # Emit GHA annotation if running in GitHub Actions. | ||
| if [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then | ||
| # Sanitize for GHA: replace :: with : : to prevent command injection. |
There was a problem hiding this comment.
[important] The sanitization here only strips :: but does not handle %0A/%0D (URL-encoded newlines) that GitHub Actions also interprets as line breaks in workflow commands. The Go sanitizeOutput function handles these correctly. An attacker-controlled transcript message containing %0A::warning::spoofed would inject additional annotations.
Fix — add after the existing :: replacement:
safe_msg="${safe_msg//%0A/ }"
safe_msg="${safe_msg//%0a/ }"
safe_msg="${safe_msg//%0D/ }"
safe_msg="${safe_msg//%0d/ }"| } | ||
|
|
||
| // truncateError trims an error message to maxTranscriptErrorLength. | ||
| // If truncated, appends an ellipsis indicator. |
There was a problem hiding this comment.
[moderate] truncateError uses byte length (len(msg)) and byte slicing (msg[:maxTranscriptErrorLength]). If the 2000-byte boundary falls in the middle of a multi-byte UTF-8 character (emoji, CJK, etc.), the result contains an incomplete rune.
Fix — walk back to a valid rune boundary, e.g.:
truncated := msg[:maxTranscriptErrorLength]
for len(truncated) > 0 && !utf8.Valid([]byte(truncated)) {
truncated = truncated[:len(truncated)-1]
}
return truncated + "… (truncated)"|
|
||
| // isResultLine does a fast prefix/contains check to avoid parsing every | ||
| // JSONL line. Claude Code transcripts can be very large. | ||
| func isResultLine(line []byte) bool { |
There was a problem hiding this comment.
[minor] isResultLine converts []byte to string via strings.Contains(string(line), ...), which allocates a new string (up to 1 MB per line, twice). Since this is meant to be a cheap pre-filter to avoid json.Unmarshal, use bytes.Contains instead to avoid the allocation:
func isResultLine(line []byte) bool {
return bytes.Contains(line, []byte(`"type":"result"`)) ||
bytes.Contains(line, []byte(`"type": "result"`))
}| // File 2: success result (should not appear in summaries). | ||
| ok2 := `{"type":"result","subtype":"success","is_error":false,"result":"all good"}` | ||
| if err := os.WriteFile(filepath.Join(dir, "agent2.jsonl"), []byte(ok2), 0o644); err != nil { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
[nit] Typo: TestExtractTranscriptErrors_MultiplFiles → TestExtractTranscriptErrors_MultipleFiles (missing 'e' in 'Multiple').
|
/fix those inline comments, please |
|
fullsend fix is working on this — view logs |
- Sanitize s.Source and s.Subtype in emitTranscriptErrors to prevent GHA command injection via crafted filenames or subtypes [critical] - Add %0A/%0D sanitization to shell script's GHA annotation output to match the Go sanitizeOutput behavior [important] - Fix truncateError to walk back to valid UTF-8 rune boundary before truncating, avoiding incomplete multi-byte characters [moderate] - Switch isResultLine from strings.Contains(string(line)) to bytes.Contains to avoid unnecessary allocations [minor] - Fix typo: MultiplFiles → MultipleFiles in test name [nit] Addresses review feedback on #764 Signed-off-by: fullsend-fix <fullsend-code@users.noreply.github.com>
🔧 Fix agent — iteration 1 (human-triggered)Addressed all 5 inline review findings from ralphbean: sanitized Source/Subtype in GHA commands (critical), added %0A/%0D sanitization to shell script (important), fixed UTF-8 truncation (moderate), switched to bytes.Contains (minor), fixed test name typo (nit). All 14 transcript tests pass. Fixed (5):
Tests: passed Updated by fullsend fix agent |
|
fullsend review is working on this — view logs |
1 participant
When an agent run fails, the root cause error is buried in the transcript JSONL artifact. Operators must download and manually search the artifact to diagnose failures.
This adds two improvements:
In-process error surfacing (run.go): After transcript extraction,
parse the JSONL for result events with is_error=true and emit
::error:: annotations to stderr. These appear in the GitHub
Actions job summary, making failures diagnosable without
downloading artifacts.
Standalone extraction script (extract-transcript-error.sh): A
shell script that extracts errors from transcript files or
directories. Usable by post-scripts, the triage agent (after
downloading artifacts via gh run download), and operators
debugging failed runs.
The transcript parser is intentionally lightweight: it scans only for result-type lines (fast string contains check) and parses the last one. Error messages are truncated to 2000 chars and sanitized to prevent GHA command injection.
New files:
Modified:
loop when agent exits non-zero
Closes #704
Post-script verification
agent/704-surface-transcript-errors)a97ff3b4fbc3f8942a369347ee2f47240737314f..HEAD)