feat(#191): render thinking, result, and system events in progress stream

[fullsend-ai-coder]

Extend the progress parser to handle additional Claude Code stream-json event types beyond tool_use. The parser now emits progress for:

• thinking content blocks (safe "Thinking" indicator, no content leaked)
• result events (completion status with subtype for errors)
• system events (session initialization)

Error results use ::warning:: in CI and StepWarn formatting. Sensitive data from thinking blocks and result messages is never surfaced.

---

Closes #191

Post-script verification

• Branch is not main/master (agent/191-render-agent-event-stream)
• Secret scan passed (gitleaks — f96750babbed5ada406a9ae04e8068449701d9c7..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[github-actions]

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

[fullsend-ai-review]

Review

Verdict: Approve

This PR extends the progress parser to handle thinking, result, and system event types from the Claude Code stream-json output. The implementation is clean, well-tested, and security-conscious.

What was reviewed

Dimension	Result
Correctness	✅ Logic is sound. Switch dispatch is correct, edge cases handled (missing subtype fallbacks, error vs success paths). Rename from parseAssistantToolUse → parseAssistantEvent is fully contained — no stale references found.
Security	✅ All untrusted values (evt.Subtype) are sanitized via sanitizeOutput() before interpolation into GHA workflow commands (::notice::, ::warning::). Thinking content, result messages, and session IDs are never surfaced. No data leakage paths found.
Intent & coherence	✅ Scope matches linked issue #191. feat prefix is correct — the new progress messages are user-visible in CLI and CI logs. No scope creep.
Style & conventions	✅ New functions follow established emit<Noun>Progress naming pattern. Test organization matches existing conventions.
Documentation	✅ No stale documentation references. The renamed function is unexported with no external documentation.
Cross-repo contracts	⏭ Skipped — no exported interfaces, schemas, or public APIs modified.

Observations

Sanitization pattern divergence (low): The new emitResultProgress and emitSystemProgress functions sanitize evt.Subtype before interpolation into fmt.Sprintf, while the existing emitToolProgress sanitizes the fully assembled message string. Both approaches are safe for the current code. The per-value approach is slightly more fragile for future modifications — if a new untrusted field is added to the format string without its own sanitizeOutput() call, the injection defense would have a gap. Consider aligning on the full-message pattern for consistency.

Mixed content block test gap (low): The tests cover thinking and tool_use in separate assistant messages, but no test exercises a single assistant message with both thinking and tool_use items in the same content array (e.g., {"type":"assistant","content":[{"type":"thinking",...},{"type":"tool_use",...}]}). The code handles this correctly via the switch in the item loop, but the edge case is untested.

JSON tag inconsistency (low): streamEvent.IsError uses json:"is_error,omitempty" while transcriptResult.IsError (in claude_transcript.go) uses json:"is_error" without omitempty. Both structs represent the same wire format. The difference is only relevant for serialization (not deserialization), so it has no runtime impact, but it's a minor inconsistency.

Previous run

Review

Findings

Low

• [injection] internal/runtime/claude_progress.go — Inconsistent sanitization pattern: emitToolProgress sanitizes the final assembled message string via sanitizeOutput(msg), while emitResultProgress and emitSystemProgress sanitize individual fields (evt.Subtype) before interpolation. Both approaches are currently safe — emitThinkingProgress has no user-controlled string inputs, and the format strings in the other two functions do not introduce :: sequences. However, the per-field approach is more fragile: if a future change adds another untrusted field to the format string without sanitizing it, the GHA command injection defense would be bypassed. Consider applying sanitizeOutput to the final assembled msg for defense-in-depth, matching emitToolProgress's pattern.

• [scope-creep] internal/runtime/claude_progress.go — The PR adds event types (thinking, result, system) beyond what issue fullsend run: render agent event stream live during execution #191 explicitly enumerates (tool calls, file edits, shell commands, reasoning summaries). These are reasonable extensions of "render agent event stream live" — thinking maps to "reasoning summaries," and result/system events are natural parts of the stream.

• [message formatting consistency] internal/runtime/claude_progress.go — Message format varies across emit functions: emitToolProgress uses ToolName: context (elapsed, N tools), while new functions use Thinking (elapsed, N tools), Result: subtype (elapsed, N tools), and System: subtype (elapsed). The differences reflect genuinely different data shapes (thinking has no context string; system events have no tool count) rather than careless inconsistency.

---

Labels: PR adds a new feature (event stream rendering) to the runtime component

[guyoron1]

/fs-review

[fullsend-ai-review]

[low] style/conventions

Sanitization pattern divergence: emitResultProgress and emitSystemProgress sanitize evt.Subtype before interpolation, while emitToolProgress sanitizes the fully assembled message. Both are safe, but the per-value approach is more fragile for future modifications.

Suggested fix: Consider aligning on the full-message sanitization pattern (sanitize msg after fmt.Sprintf, before emission) for consistency with emitToolProgress.

[fullsend-ai-review]

[low] style/conventions

streamEvent.IsError uses json:"is_error,omitempty" while transcriptResult.IsError in claude_transcript.go uses json:"is_error" without omitempty. Both represent the same wire format field. The difference has no runtime impact (only affects serialization, not deserialization) but is inconsistent.

Suggested fix: Use json:"is_error" (without omitempty) to match transcriptResult, since streamEvent is only used for deserialization.