Skip to content

fix(#6905): capture token telemetry on cancelled Claude runs - #6908

Merged
ascerra merged 1 commit into
mainfrom
agent/6905-claude-cancelled-run-tokens
Sep 2, 2026
Merged

fix(#6905): capture token telemetry on cancelled Claude runs#6908
ascerra merged 1 commit into
mainfrom
agent/6905-claude-cancelled-run-tokens

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Cancelled Claude Code runs reported $0.00 cost and 0 tokens in telemetry and status comments despite consuming real API credits. The root cause: RunMetrics token fields were only populated from the terminal ResultEvent, which is never emitted when the process is killed via SIGTERM.

Changes

  • Cumulative token tracking (claude_progress.go): Per-message token counters are now folded into cumulative counters on each message_start, so TokensEvent carries run-wide totals instead of single-message values
  • TokensEvent handler (claude.go, progressParser): Added TokensEvent case to both metrics handlers to incrementally update RunMetrics. ResultEvent still overwrites with authoritative totals on successful runs
  • Deferred final emission (claude_progress.go): When the stream ends without a ResultEvent (cancelled run), a deferred function emits one last TokensEvent with the cumulative snapshot
  • Updated throttle test: Added ResultEvent to the throttle test to isolate in-stream throttle behavior from the new deferred EOF emission

Testing

  • TestProgressParserCancelledRunCapturesTokens — core regression test: stream with tool calls and token usage but no ResultEvent, verifies metrics are non-zero
  • TestProgressParserResultOverwritesIncrementalTokens — verifies ResultEvent authoritative totals overwrite incremental TokensEvent snapshot
  • TestParseClaudeStreamCumulativeTokensAcrossMessages — verifies cumulative token tracking across multiple API calls
  • TestParseClaudeStreamNoFinalTokensEventAfterResult — verifies no duplicate TokensEvent after ResultEvent
  • TestParseClaudeStreamFinalTokensEventOnCancel — verifies deferred emission when below throttle threshold
  • All existing runtime tests pass
  • go vet, gofmt, secret scan clean

Note: TotalCostUSD remains zero for cancelled runs because the Claude stream does not expose per-API-call cost. A separate strategy is needed (documented in #6905).


Closes #6905

Post-script verification

  • Branch is not main/master (agent/6905-claude-cancelled-run-tokens)
  • Secret scan passed (gitleaks — 7f0c80c8f8b5de7e148b1065a7f3a75029d78fdc..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

The Claude runtime only populated RunMetrics token fields from the
terminal ResultEvent, which is never emitted when the process is
killed (SIGTERM on cancellation). This left InputTokens,
OutputTokens, and cache token fields at zero for every cancelled
run, corrupting budget tracking and usage analytics.

The fix has three parts:

1. Cumulative token tracking in parseClaudeStream: per-message
   token counters are folded into cumulative counters on each
   message_start, so TokensEvent now carries run-wide totals
   instead of single-message values.

2. TokensEvent handler in the metrics handler (claude.go and
   progressParser): incrementally updates RunMetrics from the
   cumulative TokensEvent. ResultEvent still overwrites with
   authoritative totals on successful runs.

3. Deferred final TokensEvent at EOF: when the stream ends
   without a ResultEvent (cancelled run), a deferred function
   emits one last TokensEvent with the cumulative snapshot,
   ensuring the metrics handler captures all available data.

Cost (TotalCostUSD) remains zero for cancelled runs because the
Claude stream does not expose per-API-call cost; a separate
strategy is needed (noted in #6905).

Closes #6905
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner September 2, 2026 11:26
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Sep 2, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:28 AM UTC · Completed 11:47 AM UTC

Commit: 9d7aebc · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.12

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.80488% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/runtime/claude.go 0.00% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Sep 2, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Moderate risk driven primarily by high Tier 2 git history signals: these runtime files are a churn hotspot with 9 distinct contributors and heavy fix-commit density. The change itself is low-complexity — a well-scoped bug fix capturing token telemetry on cancelled runs with clear acceptance criteria and good test coverage.

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Low

  • [edge-case] internal/runtime/claude_progress.goReasoningTokens is never populated in TokensEvent by the stream parser, so cancelled runs will always show zero reasoning tokens. This is a pre-existing gap — the parser has never tracked reasoning tokens in message_start/message_delta events — and is not introduced by this PR.

  • [naming-consistency] internal/runtime/claude_progress.go — The new cumulative variables (cumulativeInput, cumulativeOutput, etc.) pair with pre-existing per-message variables named totalInput, totalOutput, etc. Since "total" typically connotes "cumulative," the naming is semantically inverted. Comments in the code clarify the distinction, and renaming the pre-existing variables is out of scope for this bug fix.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Sep 2, 2026
@ascerra
ascerra added this pull request to the merge queue Sep 2, 2026
Merged via the queue into main with commit 8a593ad Sep 2, 2026
77 of 78 checks passed
@ascerra
ascerra deleted the agent/6905-claude-cancelled-run-tokens branch September 2, 2026 21:24
@fullsend-ai-retro

fullsend-ai-retro Bot commented Sep 2, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 9:26 PM UTC · Completed 9:47 PM UTC

Commit: 9d7aebc · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.26

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #6908 — fix(#6905): capture token telemetry on cancelled Claude runs

Overall assessment: A well-executed single-commit bug fix with strong test coverage and fast turnaround (~10 hours from issue to merge). The code agent produced a correct, well-tested implementation on the first attempt with no rework cycles. The review agent found two valid low-severity observations but missed one medium-severity correctness bug (see proposal below).

Timeline

Time (UTC) Event Cost
10:33 Issue #6905 filed
11:03–11:10 Triage agent ran (run) $0.90
11:10–11:26 Code agent ran (run) $4.06
11:26 PR #6908 opened
11:26–11:48 Review agent ran (run) $7.12
12:38 Human reviewer approved
21:24 Merged by ascerra

Total agent cost: $12.08 across triage ($0.90), code ($4.06), and review ($7.12).

What went well

  • Zero rework: The code agent produced a correct, well-structured fix on the first attempt — no fix agent cycles needed.
  • Good triage guidance: The triage agent correctly identified the root cause, flagged the related issue (Claude runtime parser does not populate ReasoningTokens from Anthropic stream events #6904), proposed a concrete test case, and rated severity appropriately.
  • Thorough testing: The code agent added 5 new tests and updated 1 existing test, covering the core regression case, cumulative tracking, deferred emission, and ResultEvent overwrite behavior.
  • Appropriate review findings: Both low-severity review findings (ReasoningTokens gap, naming inconsistency) were accurate and correctly scoped as informational.

Evidence for existing issues

Proposals filed

yvonnedevlinrh pushed a commit to yvonnedevlinrh/fullsend that referenced this pull request Sep 10, 2026
GitHub Actions cancellation sends SIGINT followed by SIGTERM ~7.5 s
later. signal.NotifyContext stops listening after the first signal,
so the second signal hit the default "terminate" handler and killed
the process before metrics.json and the agent telemetry span could
be written — silently losing API credit usage data on every cancelled
Review run.

Two fixes:

1. Replace signal.NotifyContext with a persistent signal channel that
   absorbs all subsequent SIGINT/SIGTERM deliveries. The first signal
   still cancels the context; subsequent signals no longer terminate
   the process, giving the cleanup path time to flush metrics and
   telemetry before the process exits normally.

2. Add a cancellation short-circuit in the agent iteration loop: when
   ctx.Err() is non-nil after rt.Run returns, immediately aggregate
   metrics, finalize the agent span, and write metrics.json — before
   extraction and validation that would be pointless on a dead sandbox.
   This ensures the artifact upload step (if: always()) captures the
   partial usage data even if the process is subsequently killed.

Dollar cost (TotalCostUSD) remains zero on cancellation because it is
only available from the terminal ResultEvent, which a killed run never
emits. Token counts (input, output, cache_read, cache_creation) are
captured via the deferred TokensEvent from PR fullsend-ai#6908.

Closes fullsend-ai#6936
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Claude runtime loses all cost/token telemetry on cancelled runs

2 participants