-
Notifications
You must be signed in to change notification settings - Fork 63
fix: auto refresh recents page #1212
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import tty | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import threading | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import requests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import typer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self._auto_refresh_thread.start() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _auto_refresh_loop(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Background loop to auto-refresh recents tab every interval.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while True: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Busy-wait risk:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # 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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logic bug: Refresh indicator cleared before redraw
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if self.running and self.current_tab == 0: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self._clear_and_redraw() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _get_webapp_domain(self) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Get the webapp domain based on environment.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return get_domain() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -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: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -685,9 +743,17 @@ def _open_agent_details(self): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _refresh(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Race condition: Manual refresh can run concurrently with background auto-refresh
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Runtime error:
self.org_idmay be undefined when unauthenticatedThe background thread can trigger
_load_agent_runs()which checksself.org_idbefore it's set for unauthenticated users, causingAttributeError. Initializeself.org_idunconditionally before starting the thread.