From d03f80b485293b6ec02a031b3efcd49b386b5ff0 Mon Sep 17 00:00:00 2001 From: Tobias Garcia Date: Tue, 21 Jul 2026 12:36:38 +0900 Subject: [PATCH 1/5] Fix: Force wait_for_user_reply: False on error path and added logging --- app/data/action/send_message_with_attachment.py | 16 ++++++++++++++-- app/internal_action_interface.py | 3 +++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/data/action/send_message_with_attachment.py b/app/data/action/send_message_with_attachment.py index 1546252d..30155dba 100644 --- a/app/data/action/send_message_with_attachment.py +++ b/app/data/action/send_message_with_attachment.py @@ -1,4 +1,5 @@ from agent_core import action +from agent_core.utils.logger import logger @action( @@ -81,10 +82,13 @@ async def send_message_with_attachment(input_data: dict) -> dict: errors.append(f"Cannot attach directory: {fp}") if errors: + logger.warning( + f"[send_message_with_attachment] Attachment(s) not ready, message not sent: {errors}" + ) return { "status": "error", "fire_at_delay": 0, - "wait_for_user_reply": wait_for_user_reply, + "wait_for_user_reply": False, "files_sent": 0, "errors": errors, } @@ -105,15 +109,23 @@ async def send_message_with_attachment(input_data: dict) -> dict: message, file_paths, session_id=session_id ) - fire_at_delay = 10800 if wait_for_user_reply else 0 files_sent = result.get("files_sent", 0) errors = result.get("errors") # Determine status based on whether all files were sent successfully if result.get("success", False): status = "ok" + fire_at_delay = 10800 if wait_for_user_reply else 0 else: status = "error" + # Nothing was actually sent, so there is nothing for the user to + # reply to - don't let the agent go dark waiting on a message that + # never arrived. + fire_at_delay = 0 + wait_for_user_reply = False + logger.warning( + f"[send_message_with_attachment] Failed to send attachment(s): {errors}" + ) response = { "status": status, diff --git a/app/internal_action_interface.py b/app/internal_action_interface.py index 708d5fd8..6f2f7d3c 100644 --- a/app/internal_action_interface.py +++ b/app/internal_action_interface.py @@ -417,6 +417,9 @@ async def do_chat_with_attachments( errors.append(f"Cannot attach directory: {fp}") if errors: + logger.warning( + f"[do_chat_with_attachments] Attachment(s) not ready: {errors}" + ) return {"success": False, "files_sent": 0, "errors": errors} ui_adapter = InternalActionInterface.ui_adapter From 6f5320470066c49a162353bbe99026061754697b Mon Sep 17 00:00:00 2001 From: Tobias Garcia Date: Tue, 21 Jul 2026 17:00:05 +0900 Subject: [PATCH 2/5] Fix 381: Decoupled active tasks from pagination. Initial load now fetches all active tasks and most recent ended tasks, get_tasks_before (pagination) now filters to ended status tasks only so that a paginated page cannot contain an active task --- app/ui_layer/adapters/browser_adapter.py | 25 ++- .../frontend/src/pages/Chat/ChatPage.tsx | 6 +- .../frontend/src/pages/Tasks/TasksPage.tsx | 6 +- .../frontend/src/store/selectors/tasks.ts | 14 +- .../frontend/src/store/slices/tasksSlice.ts | 7 +- .../browser/frontend/src/types/index.ts | 1 + .../browser/frontend/src/utils/taskStatus.ts | 7 + app/usage/action_storage.py | 176 ++++++++++-------- 8 files changed, 143 insertions(+), 99 deletions(-) create mode 100644 app/ui_layer/browser/frontend/src/utils/taskStatus.ts diff --git a/app/ui_layer/adapters/browser_adapter.py b/app/ui_layer/adapters/browser_adapter.py index b98085ae..cc642943 100644 --- a/app/ui_layer/adapters/browser_adapter.py +++ b/app/ui_layer/adapters/browser_adapter.py @@ -427,12 +427,13 @@ def __init__(self, adapter: "BrowserAdapter") -> None: self._adapter = adapter self._items: List[ActionItem] = [] self._storage = None + self._initial_has_more_ended = False self._init_storage() def _init_storage(self) -> None: """Initialize storage and load persisted actions.""" try: - from app.usage.action_storage import get_action_storage + from app.usage.action_storage import TERMINAL_STATUSES, get_action_storage self._storage = get_action_storage() @@ -442,8 +443,21 @@ def _init_storage(self) -> None: ) self._storage.mark_running_as_cancelled(exclude=restored_ids) - # Load recent tasks (and their child actions) from storage - stored_items = self._storage.get_recent_tasks_with_actions(task_limit=15) + # Load every active task plus the most recent ended tasks (and + # their child actions) from storage. Active tasks are loaded in + # full here rather than paginated, so they're never discovered + # later via scroll pagination and don't "pop in" above already + # rendered rows. + ended_limit = 15 + stored_items = self._storage.get_active_and_recent_ended_tasks( + ended_limit=ended_limit + ) + ended_count = sum( + 1 + for s in stored_items + if s.item_type == "task" and s.status in TERMINAL_STATUSES + ) + self._initial_has_more_ended = ended_count == ended_limit for stored in stored_items: self._items.append( ActionItem( @@ -878,6 +892,10 @@ def get_items(self) -> List[ActionItem]: """Get all loaded items.""" return self._items.copy() + def get_initial_has_more(self) -> bool: + """Whether there are more ended tasks beyond the initial load.""" + return self._initial_has_more_ended + def get_tasks_before( self, before_timestamp: float, task_limit: int = 15 ) -> List[ActionItem]: @@ -8805,6 +8823,7 @@ def _get_initial_state(self) -> Dict[str, Any]: } for a in self._action_panel.get_items() ], + "actionsHasMore": self._action_panel.get_initial_has_more(), "status": self._status_bar.get_status(), "dashboardMetrics": metrics.to_dict(), } diff --git a/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.tsx b/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.tsx index bf2c7e5d..28405afc 100644 --- a/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.tsx +++ b/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.tsx @@ -5,6 +5,7 @@ import { IconButton, StatusIndicator } from '../../components/ui' import { Chat } from '../../components/Chat' import { MascotDisplay } from '@mascot' import { getActivePlaceholder } from '../../utils/taskPlaceholder' +import { isEndedStatus } from '../../utils/taskStatus' import { useTaskListAutoScroll, useTaskListFLIP, useMascotVisibility } from '../../hooks' import type { ActionItem } from '../../types' import styles from './ChatPage.module.css' @@ -99,12 +100,11 @@ export function ChatPage() { // stays correct. const { tasks, activeTasks, endedTasks } = useMemo(() => { const taskItems = actions.filter(a => a.itemType === 'task') - const isEnded = (s: string) => s === 'completed' || s === 'error' || s === 'cancelled' const byNewestFirst = (a: ActionItem, b: ActionItem) => (b.createdAt ?? 0) - (a.createdAt ?? 0) const byNewestEnded = (a: ActionItem, b: ActionItem) => (b.completedAt ?? b.createdAt ?? 0) - (a.completedAt ?? a.createdAt ?? 0) - const active = taskItems.filter(t => !isEnded(t.status)).sort(byNewestFirst) - const ended = taskItems.filter(t => isEnded(t.status)).sort(byNewestEnded) + const active = taskItems.filter(t => !isEndedStatus(t.status)).sort(byNewestFirst) + const ended = taskItems.filter(t => isEndedStatus(t.status)).sort(byNewestEnded) return { tasks: [...active, ...ended], activeTasks: active, endedTasks: ended } }, [actions]) const [selectedTaskId, setSelectedTaskId] = useState(null) diff --git a/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.tsx b/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.tsx index 17bbe2e4..738770c1 100644 --- a/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.tsx +++ b/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.tsx @@ -6,6 +6,7 @@ import { StatusIndicator, Badge, Button, IconButton, SkillCreatorModal } from '. import type { ActionItem } from '../../types' import { useSkillCreator } from './useSkillCreator' import { getActivePlaceholder, type ActivePlaceholder } from '../../utils/taskPlaceholder' +import { isEndedStatus } from '../../utils/taskStatus' import { useTaskListAutoScroll, useTaskListFLIP } from '../../hooks' import { getActionRenderer, parseIO } from './actionRenderers/renderers' import styles from './TasksPage.module.css' @@ -595,12 +596,11 @@ export function TasksPage() { // and selection lookups work unchanged. const { tasks, activeTasks, endedTasks } = useMemo(() => { const taskItems = actions.filter(a => a.itemType === 'task') - const isEnded = (s: string) => s === 'completed' || s === 'error' || s === 'cancelled' const byNewestFirst = (a: ActionItem, b: ActionItem) => (b.createdAt ?? 0) - (a.createdAt ?? 0) const byNewestEnded = (a: ActionItem, b: ActionItem) => (b.completedAt ?? b.createdAt ?? 0) - (a.completedAt ?? a.createdAt ?? 0) - const active = taskItems.filter(t => !isEnded(t.status)).sort(byNewestFirst) - const ended = taskItems.filter(t => isEnded(t.status)).sort(byNewestEnded) + const active = taskItems.filter(t => !isEndedStatus(t.status)).sort(byNewestFirst) + const ended = taskItems.filter(t => isEndedStatus(t.status)).sort(byNewestEnded) return { tasks: [...active, ...ended], activeTasks: active, endedTasks: ended } }, [actions]) diff --git a/app/ui_layer/browser/frontend/src/store/selectors/tasks.ts b/app/ui_layer/browser/frontend/src/store/selectors/tasks.ts index cc2b7020..4aca999a 100644 --- a/app/ui_layer/browser/frontend/src/store/selectors/tasks.ts +++ b/app/ui_layer/browser/frontend/src/store/selectors/tasks.ts @@ -1,5 +1,6 @@ import type { RootState } from '../index' import { tasksAdapter } from '../slices/tasksSlice' +import { isEndedStatus } from '../../utils/taskStatus' const adapterSelectors = tasksAdapter.getSelectors((state) => state.tasks) @@ -25,16 +26,17 @@ export const selectResumingTaskId = (state: RootState): string | null => export const selectDeletingTaskId = (state: RootState): string | null => state.tasks.deletingTaskId -// For action_history pagination: cursor is the oldest task's createdAt -// (falling back to the oldest action of any kind if no tasks present). +// For action_history pagination: cursor is the oldest *ended* task's +// createdAt, since pagination only ever walks ended-task history — active +// tasks are always loaded in full up front (see tasksSlice.ts). export const selectOldestTaskCreatedAt = (state: RootState): number | undefined => { for (const id of state.tasks.ids) { const entry = state.tasks.entities[id] - if (entry?.itemType === 'task' && entry.createdAt !== undefined) return entry.createdAt + if (entry?.itemType === 'task' && isEndedStatus(entry.status) && entry.createdAt !== undefined) { + return entry.createdAt + } } - // Fallback: first entry's createdAt. - const firstId = state.tasks.ids[0] - return firstId !== undefined ? state.tasks.entities[firstId]?.createdAt : undefined + return undefined } export const selectHasAnyActions = (state: RootState): boolean => diff --git a/app/ui_layer/browser/frontend/src/store/slices/tasksSlice.ts b/app/ui_layer/browser/frontend/src/store/slices/tasksSlice.ts index a9d2a38e..2a721886 100644 --- a/app/ui_layer/browser/frontend/src/store/slices/tasksSlice.ts +++ b/app/ui_layer/browser/frontend/src/store/slices/tasksSlice.ts @@ -4,7 +4,8 @@ import { register } from '../socket/messageRegistry' // Tasks + actions are normalized by id. We keep insertion order rather than // re-sorting, since the backend pushes them in chronological order and the -// pagination cursor reads from the oldest entry's createdAt. +// pagination cursor reads from the oldest *ended* entry's createdAt (active +// tasks are always loaded in full up front, never paginated). const adapter = createEntityAdapter({ selectId: (a) => a.id, }) @@ -160,9 +161,9 @@ export default tasksSlice.reducer // --- inbound message handlers -------------------------------------------- register('init', (data, dispatch) => { - const d = data as { actions?: ActionItem[] } | undefined + const d = data as { actions?: ActionItem[]; actionsHasMore?: boolean } | undefined const actions = d?.actions || [] - const hasMore = actions.filter(a => a.itemType === 'task').length >= 15 + const hasMore = !!d?.actionsHasMore dispatch(setInitial({ actions, hasMore })) }) diff --git a/app/ui_layer/browser/frontend/src/types/index.ts b/app/ui_layer/browser/frontend/src/types/index.ts index a5f21f3e..1dec8f98 100644 --- a/app/ui_layer/browser/frontend/src/types/index.ts +++ b/app/ui_layer/browser/frontend/src/types/index.ts @@ -167,6 +167,7 @@ export interface InitialState { currentTask: { id: string; name: string } | null messages: ChatMessage[] actions: ActionItem[] + actionsHasMore?: boolean status: string dashboardMetrics?: DashboardMetrics needsHardOnboarding?: boolean diff --git a/app/ui_layer/browser/frontend/src/utils/taskStatus.ts b/app/ui_layer/browser/frontend/src/utils/taskStatus.ts new file mode 100644 index 00000000..212dfb81 --- /dev/null +++ b/app/ui_layer/browser/frontend/src/utils/taskStatus.ts @@ -0,0 +1,7 @@ +import type { ActionStatus } from '../types' + +const ENDED_STATUSES: ReadonlySet = new Set(['completed', 'error', 'cancelled']) + +export function isEndedStatus(status: ActionStatus): boolean { + return ENDED_STATUSES.has(status) +} diff --git a/app/usage/action_storage.py b/app/usage/action_storage.py index e9a00412..80f59ee9 100644 --- a/app/usage/action_storage.py +++ b/app/usage/action_storage.py @@ -22,6 +22,9 @@ logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") +TERMINAL_STATUSES = ("completed", "error", "cancelled") + + def _decode_skills(value: Optional[str]) -> List[str]: """Decode the JSON-encoded selected_skills column. Tolerates legacy NULL/garbage.""" if not value: @@ -431,17 +434,16 @@ def clear_terminal_tasks(self) -> List[str]: Returns: List of removed item IDs (terminal tasks + their child actions). """ - terminal_statuses = ("completed", "error", "cancelled") with sqlite3.connect(self._db_path) as conn: cursor = conn.cursor() - placeholders = ",".join("?" for _ in terminal_statuses) + placeholders = ",".join("?" for _ in TERMINAL_STATUSES) cursor.execute( f""" SELECT id FROM action_items WHERE item_type = 'task' AND status IN ({placeholders}) """, - terminal_statuses, + TERMINAL_STATUSES, ) terminal_task_ids = [row[0] for row in cursor.fetchall()] @@ -558,6 +560,48 @@ def mark_running_as_cancelled(self, exclude: Optional[set] = None) -> int: conn.commit() return cursor.rowcount + @staticmethod + def _items_for_ids(cursor: sqlite3.Cursor, task_ids: List[str]) -> List[StoredActionItem]: + """Fetch the given tasks plus all their child actions, ordered by created_at ascending.""" + if not task_ids: + return [] + + placeholders = ",".join("?" * len(task_ids)) + cursor.execute( + f""" + SELECT id, name, status, item_type, parent_id, created_at, + completed_at, input_data, output_data, error_message, + selected_skills, workflow_id, + input_tokens, output_tokens, cache_tokens + FROM action_items + WHERE id IN ({placeholders}) OR parent_id IN ({placeholders}) + ORDER BY created_at ASC + """, + task_ids + task_ids, + ) + rows = cursor.fetchall() + + return [ + StoredActionItem( + id=row[0], + name=row[1], + status=row[2], + item_type=row[3], + parent_id=row[4], + created_at=row[5], + completed_at=row[6], + input_data=row[7], + output_data=row[8], + error_message=row[9], + selected_skills=_decode_skills(row[10]), + workflow_id=row[11], + input_tokens=row[12], + output_tokens=row[13], + cache_tokens=row[14], + ) + for row in rows + ] + def get_recent_tasks_with_actions( self, task_limit: int = 15, @@ -573,7 +617,6 @@ def get_recent_tasks_with_actions( """ with sqlite3.connect(self._db_path) as conn: cursor = conn.cursor() - # Get recent task IDs cursor.execute( """ SELECT id FROM action_items @@ -584,46 +627,53 @@ def get_recent_tasks_with_actions( (task_limit,), ) task_ids = [row[0] for row in cursor.fetchall()] + return self._items_for_ids(cursor, task_ids) - if not task_ids: - return [] + def get_active_and_recent_ended_tasks( + self, + ended_limit: int = 15, + active_limit: int = 500, + ) -> List[StoredActionItem]: + """ + Get every active (non-terminal) task plus the N most recent ended + tasks, and all their child actions. Active tasks are returned in + full (not subject to `ended_limit`) so incomplete work is always + fully visible without needing to page through history for it. + + Args: + ended_limit: Maximum number of completed/error/cancelled tasks to return. + active_limit: Defensive cap on the number of active tasks returned. - # Get those tasks + all their child actions - placeholders = ",".join("?" * len(task_ids)) + Returns: + List of items (tasks + their actions) ordered by created_at ascending. + """ + with sqlite3.connect(self._db_path) as conn: + cursor = conn.cursor() + + placeholders = ",".join("?" for _ in TERMINAL_STATUSES) cursor.execute( f""" - SELECT id, name, status, item_type, parent_id, created_at, - completed_at, input_data, output_data, error_message, - selected_skills, workflow_id, - input_tokens, output_tokens, cache_tokens - FROM action_items - WHERE id IN ({placeholders}) OR parent_id IN ({placeholders}) - ORDER BY created_at ASC + SELECT id FROM action_items + WHERE item_type = 'task' AND status NOT IN ({placeholders}) + ORDER BY created_at DESC + LIMIT ? """, - task_ids + task_ids, + TERMINAL_STATUSES + (active_limit,), ) - rows = cursor.fetchall() + active_ids = [row[0] for row in cursor.fetchall()] - return [ - StoredActionItem( - id=row[0], - name=row[1], - status=row[2], - item_type=row[3], - parent_id=row[4], - created_at=row[5], - completed_at=row[6], - input_data=row[7], - output_data=row[8], - error_message=row[9], - selected_skills=_decode_skills(row[10]), - workflow_id=row[11], - input_tokens=row[12], - output_tokens=row[13], - cache_tokens=row[14], - ) - for row in rows - ] + cursor.execute( + f""" + SELECT id FROM action_items + WHERE item_type = 'task' AND status IN ({placeholders}) + ORDER BY created_at DESC + LIMIT ? + """, + TERMINAL_STATUSES + (ended_limit,), + ) + ended_ids = [row[0] for row in cursor.fetchall()] + + return self._items_for_ids(cursor, active_ids + ended_ids) def get_tasks_before( self, @@ -631,7 +681,9 @@ def get_tasks_before( task_limit: int = 15, ) -> List[StoredActionItem]: """ - Get tasks (and their actions) older than a given timestamp. + Get ended tasks (and their actions) older than a given timestamp. + Active (non-terminal) tasks are excluded since they're always + loaded up front rather than paginated. Args: before_timestamp: Unix timestamp upper bound (exclusive), in seconds. @@ -642,56 +694,18 @@ def get_tasks_before( """ with sqlite3.connect(self._db_path) as conn: cursor = conn.cursor() - # Get older task IDs + placeholders = ",".join("?" for _ in TERMINAL_STATUSES) cursor.execute( - """ + f""" SELECT id FROM action_items - WHERE item_type = 'task' AND created_at < ? + WHERE item_type = 'task' AND status IN ({placeholders}) AND created_at < ? ORDER BY created_at DESC LIMIT ? """, - (before_timestamp, task_limit), + TERMINAL_STATUSES + (before_timestamp, task_limit), ) task_ids = [row[0] for row in cursor.fetchall()] - - if not task_ids: - return [] - - placeholders = ",".join("?" * len(task_ids)) - cursor.execute( - f""" - SELECT id, name, status, item_type, parent_id, created_at, - completed_at, input_data, output_data, error_message, - selected_skills, workflow_id, - input_tokens, output_tokens, cache_tokens - FROM action_items - WHERE id IN ({placeholders}) OR parent_id IN ({placeholders}) - ORDER BY created_at ASC - """, - task_ids + task_ids, - ) - rows = cursor.fetchall() - - return [ - StoredActionItem( - id=row[0], - name=row[1], - status=row[2], - item_type=row[3], - parent_id=row[4], - created_at=row[5], - completed_at=row[6], - input_data=row[7], - output_data=row[8], - error_message=row[9], - selected_skills=_decode_skills(row[10]), - workflow_id=row[11], - input_tokens=row[12], - output_tokens=row[13], - cache_tokens=row[14], - ) - for row in rows - ] + return self._items_for_ids(cursor, task_ids) def get_task_count(self) -> int: """Get total number of tasks (not actions).""" From 20158b7a9d8372aa6919594e8ec2549c51f8cd19 Mon Sep 17 00:00:00 2001 From: Tobias Garcia Date: Tue, 21 Jul 2026 17:34:30 +0900 Subject: [PATCH 3/5] Fix 380: LLMInterface.reinitialize() rebuilds client/cache managers while preserving _session_system_prompts when model changes mid task --- agent_core/core/impl/llm/interface.py | 91 +++++++++++++++++++----- app/agent_base.py | 99 +++++++++++++++------------ tests/test_llm_reinitialize.py | 90 ++++++++++++++++++++++++ 3 files changed, 221 insertions(+), 59 deletions(-) create mode 100644 tests/test_llm_reinitialize.py diff --git a/agent_core/core/impl/llm/interface.py b/agent_core/core/impl/llm/interface.py index 945cb82a..4a76b60a 100644 --- a/agent_core/core/impl/llm/interface.py +++ b/agent_core/core/impl/llm/interface.py @@ -152,7 +152,8 @@ def __init__( self._initialized = False self._deferred = deferred - # Store for reinitialization + # Last-applied api_key/base_url, used by reinitialize() to detect + # whether a Settings save actually changed anything. self._init_api_key = api_key self._init_base_url = base_url @@ -297,6 +298,33 @@ def reinitialize( None # app context not available (e.g. agent_core standalone) ) + # Diff against the currently-live config so a Settings save that + # didn't actually change anything (or only changed the model within + # the same provider) doesn't have to nuke every active task's + # accumulated session state. `target_model is not None` guards the + # no-op fast path off in the app-context-unavailable edge case above, + # where we can't tell what "unchanged" means — fall through to the + # full-reset path there, matching prior behavior. + old_provider = self.provider + old_model = self.model + old_api_key = self._init_api_key + old_base_url = self._init_base_url + provider_unchanged = target_provider == old_provider + nothing_changed = ( + provider_unchanged + and target_model is not None + and target_model == old_model + and target_api_key == old_api_key + and target_base_url == old_base_url + ) + + if nothing_changed: + logger.info( + "[LLM] Reinitialize no-op — provider/model/credentials " + "unchanged, skipping reset" + ) + return self._initialized + try: logger.info( f"[LLM] Reinitializing with provider: {target_provider}, model: {target_model or 'registry default'}" @@ -329,15 +357,21 @@ def reinitialize( base_url=self.byteplus_base_url, model=self.model, ) - # Reset session system prompts and multi-turn message histories - self._session_system_prompts = {} - self._anthropic_session_messages = {} - self._bedrock_session_messages = {} - self._openrouter_anthropic_session_messages = {} - self._gemini_session_messages = {} - self._openai_compat_session_messages = {} else: self._byteplus_cache_manager = None + + if provider_unchanged: + # Model/credentials-only change: the message-history buffers + # are provider-agnostic message lists the new model can + # consume as-is, so keep them — the prefix cache takes one + # miss and re-warms instead of rebuilding from scratch. + logger.info( + f"[LLM] Model-only reinit within provider {self.provider}: " + f"preserving session histories" + ) + else: + # Real provider change: message formats differ across + # providers, so the accumulated histories aren't reusable. self._session_system_prompts = {} self._anthropic_session_messages = {} self._bedrock_session_messages = {} @@ -364,6 +398,11 @@ def reinitialize( ) self._consecutive_failures = 0 + # Track last-applied credentials so the next reinitialize() call + # can tell whether anything actually changed. + self._init_api_key = target_api_key + self._init_base_url = target_base_url + logger.info( f"[LLM] Reinitialized successfully with provider: {self.provider}, model: {self.model}" ) @@ -1570,15 +1609,35 @@ def _generate_byteplus_with_session( try: if not self._byteplus_cache_manager.has_session(task_id, call_type): - raise ValueError(f"No session cache found for {session_key}") + # The cache manager was rebuilt (e.g. a model-only Settings + # change recreates it since BytePlus sessions are server-side + # and model-bound), emptying its session registry — but the + # system prompt survives a model-only reinit, so reseed a + # fresh session instead of failing this turn outright. + system_prompt = self._session_system_prompts.get(session_key) + if not system_prompt: + raise ValueError(f"No session cache found for {session_key}") - result = self._byteplus_cache_manager.chat_with_session( - task_id=task_id, - call_type=call_type, - user_prompt=user_prompt, - temperature=self.temperature, - max_tokens=self.max_tokens, - ) + logger.info( + f"[BYTEPLUS] No session cache for {session_key} — " + f"reseeding a fresh session from the stored system prompt" + ) + result = self._byteplus_cache_manager.create_session_cache( + task_id=task_id, + call_type=call_type, + system_prompt=system_prompt, + user_prompt=user_prompt, + temperature=self.temperature, + max_tokens=self.max_tokens, + ) + else: + result = self._byteplus_cache_manager.chat_with_session( + task_id=task_id, + call_type=call_type, + user_prompt=user_prompt, + temperature=self.temperature, + max_tokens=self.max_tokens, + ) logger.info(f"BYTEPLUS SESSION RESPONSE: {result}") diff --git a/app/agent_base.py b/app/agent_base.py index 8a1b40e3..86b47541 100644 --- a/app/agent_base.py +++ b/app/agent_base.py @@ -3130,6 +3130,7 @@ def reinitialize_llm(self, provider: str | None = None) -> bool: llm_provider = provider or get_llm_provider() vlm_provider = get_vlm_provider() + old_llm_provider = self.llm.provider llm_ok = self.llm.reinitialize(llm_provider) vlm_ok = self.vlm.reinitialize(vlm_provider) @@ -3138,51 +3139,63 @@ def reinitialize_llm(self, provider: str | None = None) -> bool: f"[AGENT] LLM and VLM reinitialized with provider: {self.llm.provider}" ) - # Rebuild session caches for any task that was mid-flight when - # the provider switched. `LLMInterface.reinitialize()` wipes - # `_session_system_prompts` and all per-provider message-history - # buffers — without this rebuild step, `has_session_cache()` - # would return False for the rest of every active task and the - # router would fall back to the single-turn path, defeating - # session caching for the remainder of the task. - # - # Re-deriving the system prompt via `context_engine.make_prompt()` - # (inside `_create_session_caches`) means the new provider sees - # the *current* compiled prompt — so any todos / action-set - # changes since the original registration are picked up too. - # - # We also reset the event-stream sync point so the next call - # under the new provider hits the router's "first call" branch - # and resends the FULL prompt + accumulated event stream, - # establishing a fresh session-cache prefix instead of sending - # a tiny delta against an empty history. - try: - active_task_ids = ( - self.task_manager.get_active_task_ids() if self.task_manager else [] + # Only rebuild session caches when the LLM provider actually + # changed. `LLMInterface.reinitialize()` only wipes + # `_session_system_prompts` and the per-provider message-history + # buffers on a real provider change — a model-only (or no-op) + # Settings save preserves them, so `has_session_cache()` still + # returns True and no rebuild is needed. Rebuilding anyway would + # force every active task's next call to resend the FULL event + # stream on top of the already-preserved history, duplicating + # context instead of protecting it. + if self.llm.provider == old_llm_provider: + logger.info( + "[AGENT] Skipping session-cache rebuild — provider " + "unchanged, session state preserved" ) - if active_task_ids: - for task_id in active_task_ids: - self.task_manager.rebuild_session_caches(task_id) - if self.context_engine: - for call_type in ( - LLMCallType.REASONING, - LLMCallType.ACTION_SELECTION, - LLMCallType.GUI_REASONING, - LLMCallType.GUI_ACTION_SELECTION, - ): - self.context_engine.reset_event_stream_sync( - call_type, session_id=task_id - ) - logger.info( - f"[AGENT] Rebuilt session caches for " - f"{len(active_task_ids)} active task(s) under new " - f"provider {self.llm.provider}" + else: + # Rebuild session caches for any task that was mid-flight when + # the provider switched. + # + # Re-deriving the system prompt via `context_engine.make_prompt()` + # (inside `_create_session_caches`) means the new provider sees + # the *current* compiled prompt — so any todos / action-set + # changes since the original registration are picked up too. + # + # We also reset the event-stream sync point so the next call + # under the new provider hits the router's "first call" branch + # and resends the FULL prompt + accumulated event stream, + # establishing a fresh session-cache prefix instead of sending + # a tiny delta against an empty history. + try: + active_task_ids = ( + self.task_manager.get_active_task_ids() + if self.task_manager + else [] + ) + if active_task_ids: + for task_id in active_task_ids: + self.task_manager.rebuild_session_caches(task_id) + if self.context_engine: + for call_type in ( + LLMCallType.REASONING, + LLMCallType.ACTION_SELECTION, + LLMCallType.GUI_REASONING, + LLMCallType.GUI_ACTION_SELECTION, + ): + self.context_engine.reset_event_stream_sync( + call_type, session_id=task_id + ) + logger.info( + f"[AGENT] Rebuilt session caches for " + f"{len(active_task_ids)} active task(s) under new " + f"provider {self.llm.provider}" + ) + except Exception as e: + logger.warning( + f"[AGENT] Failed to rebuild session caches after " + f"provider switch: {e}" ) - except Exception as e: - logger.warning( - f"[AGENT] Failed to rebuild session caches after " - f"provider switch: {e}" - ) # Update GUI module provider if needed (only if GUI mode is enabled) gui_globally_enabled = os.getenv("GUI_MODE_ENABLED", "True") == "True" diff --git a/tests/test_llm_reinitialize.py b/tests/test_llm_reinitialize.py new file mode 100644 index 00000000..f578a776 --- /dev/null +++ b/tests/test_llm_reinitialize.py @@ -0,0 +1,90 @@ +# -*- coding: utf-8 -*- +"""Unit tests for LLMInterface.reinitialize() provider/model diffing. + +Covers the fix for: a model-only (or no-op) Settings save was +unconditionally wiping every active task's session-cache state +(_session_system_prompts + per-provider message histories), even though +those buffers are only invalidated by an actual *provider* change — the +message format they hold is provider-specific, not model-specific. +""" +from unittest.mock import patch + +import pytest + +from agent_core.core.impl.llm.interface import LLMInterface +from app.models.factory import ModelFactory + + +def _make_ctx(provider="anthropic", model="claude-a"): + return { + "provider": provider, + "model": model, + "client": object(), + "gemini_client": None, + "remote_url": None, + "byteplus": None, + "anthropic_client": object(), + "bedrock_client": None, + "initialized": True, + "auth_mode": "api_key", + } + + +@pytest.fixture +def llm_interface(): + with patch.object(ModelFactory, "create", return_value=_make_ctx()): + interface = LLMInterface( + provider="anthropic", model="claude-a", api_key="key-a", base_url="" + ) + # Seed accumulated session state as if a task were mid-flight. + interface._session_system_prompts["task-1:reasoning"] = "system prompt" + interface._anthropic_session_messages["task-1:reasoning"] = [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "hello"}, + ] + return interface + + +def test_reinitialize_noop_preserves_everything(llm_interface): + """A Settings save with no actual change must not touch live state.""" + with patch.object(ModelFactory, "create") as mock_create, patch( + "app.config.get_llm_model", return_value="claude-a" + ): + ok = llm_interface.reinitialize( + provider="anthropic", api_key="key-a", base_url="" + ) + + assert ok is True + mock_create.assert_not_called() + assert llm_interface._anthropic_session_messages["task-1:reasoning"] + assert llm_interface._session_system_prompts["task-1:reasoning"] == "system prompt" + + +def test_reinitialize_model_only_preserves_histories(llm_interface): + """Same provider, different model: histories survive, model updates.""" + with patch.object( + ModelFactory, "create", return_value=_make_ctx(model="claude-b") + ), patch("app.config.get_llm_model", return_value="claude-b"): + ok = llm_interface.reinitialize( + provider="anthropic", api_key="key-a", base_url="" + ) + + assert ok is True + assert llm_interface.model == "claude-b" + assert llm_interface._anthropic_session_messages["task-1:reasoning"] + assert llm_interface._session_system_prompts["task-1:reasoning"] == "system prompt" + + +def test_reinitialize_provider_change_clears_histories(llm_interface): + """An actual provider change still fully resets session state.""" + with patch.object( + ModelFactory, "create", return_value=_make_ctx(provider="openai", model="gpt-x") + ), patch("app.config.get_llm_model", return_value="gpt-x"): + ok = llm_interface.reinitialize( + provider="openai", api_key="key-b", base_url="" + ) + + assert ok is True + assert llm_interface.provider == "openai" + assert llm_interface._anthropic_session_messages == {} + assert llm_interface._session_system_prompts == {} From 21abed37282712f33fa64a6083c0291bee6267df Mon Sep 17 00:00:00 2001 From: Tobias Garcia Date: Wed, 22 Jul 2026 11:39:00 +0900 Subject: [PATCH 4/5] Fix: Unreported jittering issue when scrolling to bottom or top of tasks list --- .../src/hooks/useTaskListAutoScroll.ts | 70 +++++++++++-------- .../frontend/src/hooks/useTaskListFLIP.ts | 8 ++- .../src/pages/Chat/ChatPage.module.css | 1 + .../frontend/src/pages/Chat/ChatPage.tsx | 13 ++-- .../src/pages/Tasks/TasksPage.module.css | 1 + .../frontend/src/pages/Tasks/TasksPage.tsx | 13 ++-- .../frontend/src/store/selectors/tasks.ts | 11 ++- 7 files changed, 73 insertions(+), 44 deletions(-) diff --git a/app/ui_layer/browser/frontend/src/hooks/useTaskListAutoScroll.ts b/app/ui_layer/browser/frontend/src/hooks/useTaskListAutoScroll.ts index 99f88a5e..3aadeb17 100644 --- a/app/ui_layer/browser/frontend/src/hooks/useTaskListAutoScroll.ts +++ b/app/ui_layer/browser/frontend/src/hooks/useTaskListAutoScroll.ts @@ -8,6 +8,7 @@ interface Pagination { const NEAR_TOP_PX = 100 const NEAR_BOTTOM_PX = 100 +const RECENT_SCROLL_GUARD_MS = 200 /** * Auto-scroll + scroll-to-bottom pagination for a non-virtualized list whose @@ -16,8 +17,12 @@ const NEAR_BOTTOM_PX = 100 * - On the first render with items present, jumps to the top (latest). * - When the item count grows, sticks to the top only if the user was near * the top — if they scrolled down to inspect older entries, stays put. - * - When the user scrolls near the bottom, calls `loadMore()` and preserves - * the visible anchor so freshly appended items don't yank the viewport. + * Suppressed while the user is actively scrolling so the auto-follow + * doesn't fight live wheel/trackpad input. + * - When the user scrolls near the bottom, calls `loadMore()`. Items are + * always appended below the current view (lists are newest-at-top, + * oldest-at-bottom, and `loadMore()` only fetches older items), so the + * existing scrollTop stays valid on its own — no restore is performed. * * Shared by ChatPage's Tasks & Actions sidebar and TasksPage's All Tasks * list so the two stay in sync. @@ -28,13 +33,22 @@ export function useTaskListAutoScroll( { hasMore, loading, loadMore }: Pagination, ): void { const wasNearTopRef = useRef(true) + // Tracked alongside wasNearTopRef so the auto-follow below can tell a + // genuine "near top" apart from a short list where the whole scrollable + // range fits inside NEAR_TOP_PX, making "near top" and "near bottom" true + // simultaneously. + const wasNearBottomRef = useRef(false) const hasInitialScrolledRef = useRef(false) const prevItemCountRef = useRef(0) const prevLoadingRef = useRef(false) - // Captured on scroll-to-bottom before triggering pagination; cleared by the - // layout effect once the appended items have settled. - const pendingRestoreScrollTopRef = useRef(null) - const pendingRestoreScrollHeightRef = useRef(null) + // Guards against re-triggering loadMore() while a request is already in + // flight; self-healing (cleared whenever `loading` is false) so it can't + // wedge shut if loadMore() bails out internally without ever setting + // `loading` true. + const loadInFlightRef = useRef(false) + // Timestamp of the last scroll event, used to suppress the near-top + // auto-follow while the user is actively scrolling. + const lastScrollAtRef = useRef(0) // Mirror latest pagination props into a ref so the scroll listener doesn't // tear down and re-attach on every render of the parent. @@ -45,17 +59,13 @@ export function useTaskListAutoScroll( const el = ref.current if (!el) return const handleScroll = () => { + lastScrollAtRef.current = Date.now() wasNearTopRef.current = el.scrollTop < NEAR_TOP_PX const { hasMore: hm, loading: ld, loadMore: lm } = paginationRef.current const distFromBottom = el.scrollHeight - el.scrollTop - el.clientHeight - if ( - distFromBottom < NEAR_BOTTOM_PX && - hm && - !ld && - pendingRestoreScrollHeightRef.current === null - ) { - pendingRestoreScrollTopRef.current = el.scrollTop - pendingRestoreScrollHeightRef.current = el.scrollHeight + wasNearBottomRef.current = distFromBottom < NEAR_BOTTOM_PX + if (distFromBottom < NEAR_BOTTOM_PX && hm && !ld && !loadInFlightRef.current) { + loadInFlightRef.current = true lm() } } @@ -72,18 +82,14 @@ export function useTaskListAutoScroll( const grew = itemCount > prevItemCountRef.current prevItemCountRef.current = itemCount - // Pagination just finished (loading true→false): keep the user anchored - // where they were when they triggered the load. Newly appended items - // grow scrollHeight; preserving scrollTop alone is enough. - if ( - wasLoading && - !loading && - pendingRestoreScrollHeightRef.current !== null && - pendingRestoreScrollTopRef.current !== null - ) { - el.scrollTop = pendingRestoreScrollTopRef.current - pendingRestoreScrollHeightRef.current = null - pendingRestoreScrollTopRef.current = null + if (!loading) { + loadInFlightRef.current = false + } + + // Pagination just finished: items were appended below the current + // viewport, so the existing scrollTop is already correct — nothing to + // restore. + if (wasLoading && !loading) { return } @@ -95,8 +101,16 @@ export function useTaskListAutoScroll( return } - // New item while the user was following the head — auto-follow to top. - if (grew && wasNearTopRef.current) { + // New item while the user was following the head — auto-follow to top, + // unless the user is actively mid-scroll (avoids fighting live input) or + // the list is short enough that "near top" and "near bottom" overlap + // (avoids fighting an in-progress bottom pagination). + if ( + grew && + wasNearTopRef.current && + !wasNearBottomRef.current && + Date.now() - lastScrollAtRef.current > RECENT_SCROLL_GUARD_MS + ) { el.scrollTo({ top: 0, behavior: 'smooth' }) } }, [itemCount, loading, ref]) diff --git a/app/ui_layer/browser/frontend/src/hooks/useTaskListFLIP.ts b/app/ui_layer/browser/frontend/src/hooks/useTaskListFLIP.ts index ef1255fd..16959f2f 100644 --- a/app/ui_layer/browser/frontend/src/hooks/useTaskListFLIP.ts +++ b/app/ui_layer/browser/frontend/src/hooks/useTaskListFLIP.ts @@ -17,8 +17,12 @@ const ANIMATION_DURATION_MS = 250 * so user scrolling doesn't trigger spurious animations. * - Forces a synchronous reflow between Invert and Play so the inverted * transform commits in the same frame as it was applied — no flicker. + * - Re-measures only when `orderKey` changes (pass a stable string derived + * from the visible task id order), not on every render — renders caused by + * unrelated state (e.g. a loading spinner toggling) shouldn't re-animate + * rows that didn't actually move. */ -export function useTaskListFLIP() { +export function useTaskListFLIP(orderKey: string) { const elementsRef = useRef>(new Map()) const prevPositionsRef = useRef>(new Map()) @@ -50,7 +54,7 @@ export function useTaskListFLIP() { }) prevPositionsRef.current = newPositions - }) + }, [orderKey]) return setRef } diff --git a/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.module.css b/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.module.css index 2746cd7c..e20cda37 100644 --- a/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.module.css +++ b/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.module.css @@ -354,6 +354,7 @@ .actionList { flex: 1; overflow-y: auto; + overflow-anchor: none; padding: var(--space-2); } diff --git a/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.tsx b/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.tsx index 28405afc..73d788d4 100644 --- a/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.tsx +++ b/app/ui_layer/browser/frontend/src/pages/Chat/ChatPage.tsx @@ -107,6 +107,7 @@ export function ChatPage() { const ended = taskItems.filter(t => isEndedStatus(t.status)).sort(byNewestEnded) return { tasks: [...active, ...ended], activeTasks: active, endedTasks: ended } }, [actions]) + const taskOrderKey = useMemo(() => tasks.map(t => t.id).join(','), [tasks]) const [selectedTaskId, setSelectedTaskId] = useState(null) const getActionsForTask = (taskId: string) => @@ -126,7 +127,7 @@ export function ChatPage() { // FLIP animates a task sliding from active → ended (or vice-versa) and the // surrounding rows shifting up/down to accommodate. Each row registers its // outer
via `flipRef(task.id)`. - const flipRef = useTaskListFLIP() + const flipRef = useTaskListFLIP(taskOrderKey) const [mascotVisible] = useMascotVisibility() @@ -155,11 +156,6 @@ export function ChatPage() {

All Tasks

- {loadingOlderActions && ( -
- Loading older tasks... -
- )} {tasks.length === 0 ? (

No active tasks

@@ -329,6 +325,11 @@ export function ChatPage() { ) })} + {loadingOlderActions && ( +
+ Loading older tasks... +
+ )} ) })()} diff --git a/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.module.css b/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.module.css index cc01b941..3ddb862e 100644 --- a/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.module.css +++ b/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.module.css @@ -66,6 +66,7 @@ .listContent { flex: 1; overflow-y: auto; + overflow-anchor: none; padding: var(--space-2); } diff --git a/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.tsx b/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.tsx index 738770c1..ad7faf85 100644 --- a/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.tsx +++ b/app/ui_layer/browser/frontend/src/pages/Tasks/TasksPage.tsx @@ -603,6 +603,7 @@ export function TasksPage() { const ended = taskItems.filter(t => isEndedStatus(t.status)).sort(byNewestEnded) return { tasks: [...active, ...ended], activeTasks: active, endedTasks: ended } }, [actions]) + const taskOrderKey = useMemo(() => tasks.map(t => t.id).join(','), [tasks]) // Scroll behavior + scroll-to-top pagination for the All Tasks list. // Same hook as ChatPage's Tasks & Actions sidebar so the two behave @@ -617,7 +618,7 @@ export function TasksPage() { // FLIP animates a task sliding from active → ended (or vice-versa) and the // surrounding rows shifting up/down to accommodate. Operates on whatever //
each row registers via `flipRef(task.id)`. - const flipRef = useTaskListFLIP() + const flipRef = useTaskListFLIP(taskOrderKey) const selectedTask = useMemo( () => tasks.find(t => t.id === selectedTaskId) ?? null, @@ -830,11 +831,6 @@ export function TasksPage() {
- {loadingOlderActions && ( -
- Loading older tasks... -
- )} {tasks.length === 0 ? (

No tasks yet

@@ -960,6 +956,11 @@ export function TasksPage() { ) })} + {loadingOlderActions && ( +
+ Loading older tasks... +
+ )} ) })() diff --git a/app/ui_layer/browser/frontend/src/store/selectors/tasks.ts b/app/ui_layer/browser/frontend/src/store/selectors/tasks.ts index 4aca999a..842fe0f5 100644 --- a/app/ui_layer/browser/frontend/src/store/selectors/tasks.ts +++ b/app/ui_layer/browser/frontend/src/store/selectors/tasks.ts @@ -30,13 +30,20 @@ export const selectDeletingTaskId = (state: RootState): string | null => // createdAt, since pagination only ever walks ended-task history — active // tasks are always loaded in full up front (see tasksSlice.ts). export const selectOldestTaskCreatedAt = (state: RootState): number | undefined => { + // Entities are appended in fetch order (no sortComparer), so each new + // "older tasks" page lands after earlier entries in `ids` — scanning for + // the first match would freeze on the first page's oldest entry forever. + // Take the true minimum across everything loaded instead. + let oldest: number | undefined for (const id of state.tasks.ids) { const entry = state.tasks.entities[id] if (entry?.itemType === 'task' && isEndedStatus(entry.status) && entry.createdAt !== undefined) { - return entry.createdAt + if (oldest === undefined || entry.createdAt < oldest) { + oldest = entry.createdAt + } } } - return undefined + return oldest } export const selectHasAnyActions = (state: RootState): boolean => From 9e1b240d464a26af7fd3486489003fe413ef5066 Mon Sep 17 00:00:00 2001 From: Tobias Garcia Date: Wed, 22 Jul 2026 12:03:15 +0900 Subject: [PATCH 5/5] Fix: Redact API key from exposure in logs --- agent_core/core/impl/llm/interface.py | 8 +++++++- app/llm_interface.py | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/agent_core/core/impl/llm/interface.py b/agent_core/core/impl/llm/interface.py index 4a76b60a..a721bb03 100644 --- a/agent_core/core/impl/llm/interface.py +++ b/agent_core/core/impl/llm/interface.py @@ -181,7 +181,13 @@ def __init__( deferred=deferred, ) - logger.info(f"[LLM FACTORY] {ctx}") + _safe_ctx = {k: v for k, v in ctx.items() if k != "byteplus"} + if ctx.get("byteplus"): + _safe_ctx["byteplus"] = { + "api_key": "", + "base_url": ctx["byteplus"].get("base_url"), + } + logger.info(f"[LLM FACTORY] {_safe_ctx}") self.provider = ctx["provider"] self.model = ctx["model"] diff --git a/app/llm_interface.py b/app/llm_interface.py index 33ada511..8da54d7b 100644 --- a/app/llm_interface.py +++ b/app/llm_interface.py @@ -854,7 +854,13 @@ def __init__( deferred=deferred, ) - logger.info(f"[LLM FACTORY] {ctx}") + _safe_ctx = {k: v for k, v in ctx.items() if k != "byteplus"} + if ctx.get("byteplus"): + _safe_ctx["byteplus"] = { + "api_key": "", + "base_url": ctx["byteplus"].get("base_url"), + } + logger.info(f"[LLM FACTORY] {_safe_ctx}") self.provider = ctx["provider"] self.model = ctx["model"]