Skip to content

fix: auto refresh recents page #1212

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

Merged
merged 1 commit into from
Aug 19, 2025
Merged

fix: auto refresh recents page #1212

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
66 changes: 66 additions & 0 deletions src/codegen/cli/tui/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import tty
from datetime import datetime
from typing import Any
import threading
import time

import requests
import typer
Expand Down Expand Up @@ -35,14 +37,66 @@ def __init__(self):
self.tabs = ["recents", "new", "web"]
self.current_tab = 0

# Refresh state
self.is_refreshing = False
self._auto_refresh_interval_seconds = 10
self._refresh_lock = threading.Lock()

# New tab state
self.prompt_input = ""

self.cursor_position = 0
self.input_mode = False # When true, we're typing in the input box

# Set up signal handler for Ctrl+C
signal.signal(signal.SIGINT, self._signal_handler)

# Start background auto-refresh thread (daemon)
self._auto_refresh_thread = threading.Thread(target=self._auto_refresh_loop, daemon=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Runtime error: self.org_id may be undefined when unauthenticated
The background thread can trigger _load_agent_runs() which checks self.org_id before it's set for unauthenticated users, causing AttributeError. Initialize self.org_id unconditionally before starting the thread.

Suggested change
self._auto_refresh_thread = threading.Thread(target=self._auto_refresh_loop, daemon=True)
def __init__(self):
self.token = get_current_token()
self.is_authenticated = bool(self.token)
self.org_id = None # ensure attribute is always defined
if self.is_authenticated:
self.org_id = resolve_org_id()
self.agent_runs: list[dict[str, Any]] = []
self.selected_index = 0
self.running = True
self.show_action_menu = False
self.action_menu_selection = 0
# Tab management
self.tabs = ["recents", "new", "web"]
self.current_tab = 0
# Refresh state
self.is_refreshing = False
self._auto_refresh_interval_seconds = 10
self._refresh_lock = threading.Lock()
# New tab state
self.prompt_input = ""
self.cursor_position = 0
self.input_mode = False # When true, we're typing in the input box
# Set up signal handler for Ctrl+C
signal.signal(signal.SIGINT, self._signal_handler)
# Start background auto-refresh thread (daemon)
self._auto_refresh_thread = threading.Thread(target=self._auto_refresh_loop, daemon=True)
self._auto_refresh_thread.start()

self._auto_refresh_thread.start()

def _auto_refresh_loop(self):
"""Background loop to auto-refresh recents tab every interval."""
while True:
Copy link
Contributor

Choose a reason for hiding this comment

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

Busy-wait risk: while True loop sleeps unconditionally even after running is set False
If running becomes False, you still sleep another interval before exiting. Check running before sleeping to exit promptly.

Suggested change
while True:
def _auto_refresh_loop(self):
"""Background loop to auto-refresh recents tab every interval."""
while self.running:
# Sleep first so we don't immediately spam a refresh on start
time.sleep(self._auto_refresh_interval_seconds)
if not self.running:
break
if self.current_tab == 0 and not self.is_refreshing:
acquired = self._refresh_lock.acquire(blocking=False)
if not acquired:
continue
try:
if self.running and self.current_tab == 0 and not self.is_refreshing:
self._background_refresh()
finally:
self._refresh_lock.release()

# Sleep first so we don't immediately spam a refresh on start
time.sleep(self._auto_refresh_interval_seconds)

if not self.running:
break

# Only refresh when on recents tab and not currently refreshing
if self.current_tab == 0 and not self.is_refreshing:
# Try background refresh; if lock is busy, skip this tick
acquired = self._refresh_lock.acquire(blocking=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential deadlock/missed refresh: Skipping refresh when lock not acquired can starve updates under frequent manual refreshes
If the lock is often held by manual refreshes, the thread may keep skipping and never update. Use a short timeout instead of non-blocking acquire.

Suggested change
acquired = self._refresh_lock.acquire(blocking=False)
def _auto_refresh_loop(self):
"""Background loop to auto-refresh recents tab every interval."""
while self.running:
time.sleep(self._auto_refresh_interval_seconds)
if not self.running:
break
if self.current_tab == 0 and not self.is_refreshing:
# Try acquire with a small timeout to avoid starvation
acquired = self._refresh_lock.acquire(timeout=0.25)
if not acquired:
continue
try:
if self.running and self.current_tab == 0 and not self.is_refreshing:
self._background_refresh()
finally:
self._refresh_lock.release()

if not acquired:
continue
try:
# Double-check state after acquiring lock
if self.running and self.current_tab == 0 and not self.is_refreshing:
self._background_refresh()
finally:
self._refresh_lock.release()

def _background_refresh(self):
"""Refresh data without disrupting selection/menu state; redraw if still on recents."""
self.is_refreshing = True
# Do not redraw immediately to reduce flicker; header shows indicator on next paint

previous_index = self.selected_index
try:
if self._load_agent_runs():
# Preserve selection but clamp to new list bounds
if self.agent_runs:
self.selected_index = max(0, min(previous_index, len(self.agent_runs) - 1))
else:
self.selected_index = 0
finally:
self.is_refreshing = False

# Redraw only if still on recents and app running
Copy link
Contributor

Choose a reason for hiding this comment

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

Logic bug: Refresh indicator cleared before redraw
is_refreshing is set to False before calling _clear_and_redraw(), so the header never shows the "Refreshing…" indicator during auto-refresh. Keep the flag set through the redraw, then clear it.

Suggested change
# Redraw only if still on recents and app running
def _background_refresh(self):
"""Refresh data without disrupting selection/menu state; redraw if still on recents."""
self.is_refreshing = True
previous_index = self.selected_index
try:
if self._load_agent_runs():
if self.agent_runs:
self.selected_index = max(0, min(previous_index, len(self.agent_runs) - 1))
else:
self.selected_index = 0
finally:
# Redraw while the indicator is still visible
if self.running and self.current_tab == 0:
self._clear_and_redraw()
# Now clear the indicator
self.is_refreshing = False

if self.running and self.current_tab == 0:
self._clear_and_redraw()
Copy link
Contributor

Choose a reason for hiding this comment

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

Thread-safety/UI race: Background thread redraws can interleave with main-thread redraws
Printing from both threads can corrupt terminal output. Signal the main loop to redraw instead of drawing from the background thread.

Suggested change
self._clear_and_redraw()
def _background_refresh(self):
"""Refresh data without disrupting selection/menu state; request redraw on main thread."""
self.is_refreshing = True
previous_index = self.selected_index
try:
if self._load_agent_runs():
if self.agent_runs:
self.selected_index = max(0, min(previous_index, len(self.agent_runs) - 1))
else:
self.selected_index = 0
finally:
# Request a redraw by main loop instead of drawing from this thread
self._pending_redraw = True
self.is_refreshing = False
# In run(), trigger redraws when requested by background thread
# (Place near the top of the while loop, before waiting for keypress)
# while self.running:
# try:
# if getattr(self, "_pending_redraw", False):
# self._clear_and_redraw()
# self._pending_redraw = False
# key = self._get_char()
# self._handle_keypress(key)
# if self.running:
# self._clear_and_redraw()
# except KeyboardInterrupt:
# break


def _get_webapp_domain(self) -> str:
"""Get the webapp domain based on environment."""
return get_domain()
Expand Down Expand Up @@ -72,6 +126,10 @@ def _format_status_line(self, left_text: str) -> str:
instructions_line = f"\033[90m{left_text}\033[0m"
org_line = f"{purple_color}• {org_name}{reset_color}"

# Append a subtle refresh indicator when a refresh is in progress
if getattr(self, "is_refreshing", False):
org_line += " \033[90m■ Refreshing…\033[0m"

return f"{instructions_line}\n{org_line}"

def _load_agent_runs(self) -> bool:
Expand Down Expand Up @@ -685,9 +743,17 @@ def _open_agent_details(self):

def _refresh(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Race condition: Manual refresh can run concurrently with background auto-refresh
Without locking, _refresh can overlap with _auto_refresh_loop, leading to concurrent updates and redraws. Guard the manual refresh with the same lock.

Suggested change
def _refresh(self):
def _refresh(self):
"""Refresh the agent runs list."""
# Prevent concurrent refresh if background refresh is running
acquired = self._refresh_lock.acquire(blocking=False)
if not acquired:
return
try:
# Indicate refresh and redraw immediately so the user sees it
self.is_refreshing = True
self._clear_and_redraw()
if self._load_agent_runs():
self.selected_index = 0 # Reset selection
finally:
# Clear refresh indicator and redraw with updated data
self.is_refreshing = False
self._clear_and_redraw()
self._refresh_lock.release()

"""Refresh the agent runs list."""
# Indicate refresh and redraw immediately so the user sees it
self.is_refreshing = True
self._clear_and_redraw()

if self._load_agent_runs():
self.selected_index = 0 # Reset selection

# Clear refresh indicator and redraw with updated data
self.is_refreshing = False
self._clear_and_redraw()

def _clear_and_redraw(self):
"""Clear screen and redraw everything."""
# Move cursor to top and clear screen from cursor down
Expand Down
Loading