Skip to content

Add timeout to explain summary generation#876

Open
peyton-alt wants to merge 4 commits intomainfrom
fix/explain-summary-timeout
Open

Add timeout to explain summary generation#876
peyton-alt wants to merge 4 commits intomainfrom
fix/explain-summary-timeout

Conversation

@peyton-alt
Copy link
Copy Markdown
Contributor

@peyton-alt peyton-alt commented Apr 8, 2026

Summary

Adds a timeout to checkpoint summary generation for entire explain --generate so the command cannot hang indefinitely when the Claude CLI stalls.

What Changed

  • scopes the 30 second timeout to explain --generate in explain.go
  • keeps summarize.GenerateFromTranscript generic for other call sites
  • preserves shorter parent deadlines and clamps longer parent deadlines to the 30 second safety timeout
  • improves Claude CLI cancellation and deadline handling so context cancellation surfaces cleanly
  • prints Generating checkpoint summary... while explain --generate is waiting
  • adds unit coverage for default timeout, shorter parent deadline, longer parent deadline, and cancellation behavior

Why

A stalled Claude summary generation path currently leaves users waiting without a bounded failure mode. This change keeps the fix narrowly scoped to explain --generate instead of changing shared summarize behavior for other callers.

Current state before this fix:
entire explain --generate
^Cfailed to generate summary: failed to generate summary: claude CLI failed (exit -1):

Verification

  • go test ./cmd/entire/cli -run 'TestGenerateCheckpointAISummary|TestExplainCmd_RejectsPositionalArgs'
  • go test ./cmd/entire/cli/summarize
  • mise run lint
  • mise run test

Entire-Checkpoint: c8bb257f3d46
Copilot AI review requested due to automatic review settings April 8, 2026 19:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a default timeout around AI summary generation to prevent the explain --generate (and other shared summarization call sites) from hanging indefinitely.

Changes:

  • Add a 30s context timeout wrapper inside summarize.GenerateFromTranscript.
  • Improve Claude CLI generator error handling to surface context cancellation/timeouts consistently.
  • Update explain summary generation to emit a user-facing “Generating…” message and add a unit test for timeout behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
cmd/entire/cli/summarize/summarize.go Adds a default timeout and timeout-specific error handling around generator execution.
cmd/entire/cli/summarize/generator_config_test.go Adds a new unit test intended to validate timeout behavior.
cmd/entire/cli/summarize/claude.go Returns context.DeadlineExceeded / context.Canceled when the context ends during Claude CLI execution.
cmd/entire/cli/explain.go Wires through an error writer and prints progress text while generating a checkpoint summary.

@peyton-alt
Copy link
Copy Markdown
Contributor Author

@BugBot review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1b2851e. Configure here.

@peyton-alt peyton-alt marked this pull request as ready for review April 8, 2026 21:57
@peyton-alt peyton-alt requested a review from a team as a code owner April 8, 2026 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants