Skip to content

OCPBUGS-123720: Fix agent TUI timeout #346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pawanpinjarkar wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −6
Open

OCPBUGS-123720: Fix agent TUI timeout #346

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions tools/agent_tui/ui/timeout_modal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ func (u *UI) startCountdownTimer(
onTick func(remaining float64),
onTimeout func(),
) {
start := time.Now()
ticker := time.NewTicker(1 * time.Second)
secondsRemaining := int(duration.Seconds())

go func() {
defer ticker.Stop()
Expand All @@ -167,15 +167,14 @@ func (u *UI) startCountdownTimer(
case <-cancelChan:
return

case t := <-ticker.C:
elapsed := t.Sub(start)
if elapsed >= duration {
case <-ticker.C:
secondsRemaining--
if secondsRemaining <= 0 {
onTimeout()

@zaneb zaneb Sep 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We're kind of mixing two different types of concurrency here. The ticker operates in a goroutine on wall-clock time, while the cancel comes from an event handler in the main loop.
So currently onTimeout() immediately exits the program as soon as the wall clock time has expired. But I think what it needs to do is post an event to the main loop (using app.QueueUpdate()) that checks whether the timeout has been cancelled, and only exits if it has not.
That way if user events get delayed past the wall-clock timeout, they will still be processed in order and serve to prevent the app from exiting.

@zaneb zaneb Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Correction: ShowTimeoutDialog() calls Stop() directly.
ShowRendezvousIPTimeoutDialog(), which is the relevant one here, does use QueueUpdate(), but it doesn't check whether the timeout has already been cancelled by a previous event.

return
}

remaining := duration.Seconds() - elapsed.Seconds()
onTick(remaining)
onTick(float64(secondsRemaining))
Comment thread
zaneb marked this conversation as resolved.
}
}
}()
Expand Down