fix: eliminate real-time waits that cause test suite hangs#823
Merged
Conversation
waitForApproval accepted poll interval as int seconds, clamping to a 1s minimum. Tests could not go below 1s, adding ~23s of real-time sleeps to the cli package test run. Change interval to time.Duration so tests can pass time.Millisecond, and make slowDownBackoff a var so the SlowDown test avoids a 5s real wait. Entire-Checkpoint: 9492acb5b894
The attach tests called runAttach with force=false, which triggers an interactive huh confirmation form. This blocks indefinitely in TTY environments (e.g. running mise run test from zsh). None of the tests assert on the amend prompt behavior, so force=true is correct here. Entire-Checkpoint: 21ab878a620a
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes real-time waits in the CLI unit tests that were causing long runtimes and occasional hangs, primarily by allowing sub-second polling intervals in login device-authorization tests and bypassing interactive confirmation in attach tests.
Changes:
- Updated
waitForApprovalto accept atime.Durationpolling interval (instead of whole seconds), enabling millisecond polling in tests. - Made the login “slow_down” backoff configurable for tests to avoid multi-second sleeps.
- Updated attach tests to run with
force=trueso they don’t block on interactivehuhprompts in TTY environments.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/entire/cli/login.go | Changes polling interval handling to time.Duration and makes slow-down backoff overrideable for tests. |
| cmd/entire/cli/login_test.go | Uses millisecond intervals and overrides slow-down backoff to eliminate real sleeps. |
| cmd/entire/cli/attach_test.go | Forces non-interactive attach behavior to prevent TTY hangs. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Tests created fresh transcript files that appeared "live" to waitForTranscriptFlush, causing it to poll for 3s per test waiting for a sentinel that never appears. Three fixes: - Backdate transcript file mtime in setupClaudeTranscript and strategy mid_turn_commit tests so the stale check fast-paths - Move PrepareTranscript call inside the file-exists check in resolveAndValidateTranscript — don't poll on non-existent files during agent auto-detection (fixes Cursor's 3s poll on miss) Entire-Checkpoint: c0dbb5d73ef9
… state Replace the package-level var with a const (defaultSlowDownBackoff) and pass the backoff duration as a parameter to waitForApproval. Tests pass time.Millisecond, avoiding real-time waits without mutating shared state. Fixes data race flagged by Copilot and Cursor. Entire-Checkpoint: 1696f1f9188b
Contributor
Author
|
Bugbot run |
computermode
approved these changes
Apr 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
intervalparameter fromint(seconds) totime.Durationand addedslowDownBackoffas an explicit parameter (was a mutable package-level var). Tests passtime.Millisecondfor both, eliminating ~23s of real-time sleeps and a potential data race under-race.forceparameter fromfalsetotrueto skip the interactivehuhconfirmation prompt that blocks indefinitely in TTY environments (e.g.mise run testfrom zsh).waitForTranscriptFlushhits the stale fast-path instead of polling for 3s. Also movedPrepareTranscriptinside the file-exists check inresolveAndValidateTranscriptto avoid Cursor's 3s poll on non-existent files during agent auto-detection.Net result:
clipackage 34s → 8s,strategypackage 20s → 16s.Test plan
go build ./...andgo vet ./...passmise run lint— zero issuesgo test -race -run TestWaitForApproval ./cmd/entire/cli/— no data racesTestWaitForApproval_*tests pass in ~0.01s (down from ~23s)TestAttach_*tests pass in ~0.05s each (down from ~3s), no TTY hangTestSessionHasNewContentFromLiveTranscript_*tests pass in ~0.03s (down from ~3s)go test ./...passes🤖 Generated with Claude Code
Note
Low Risk
Low risk: primarily refactors polling/wait behavior to avoid unnecessary sleeps and test hangs, with minimal user-facing behavior change (only when/if
PrepareTranscriptruns).Overview
Eliminates several sources of real-time waiting and test suite hangs in the CLI.
attachnow only calls agentPrepareTranscriptafter confirming the transcript file exists, avoiding 3s polling during agent auto-detection; its tests run withforce=trueand backdate transcript mtimes so the flush-wait fast path is taken.loginrefactorswaitForApprovalto taketime.Durationvalues (and an explicitslowDownBackoff) and updates tests to use millisecond intervals, removing long sleeps and avoiding reliance on mutable package-level timing state.Written by Cursor Bugbot for commit 281fb45. Configure here.