diff --git a/src-tauri/src/backend/event_translator.rs b/src-tauri/src/backend/event_translator.rs index 39f26c2c..b0180f00 100644 --- a/src-tauri/src/backend/event_translator.rs +++ b/src-tauri/src/backend/event_translator.rs @@ -394,6 +394,7 @@ pub(crate) fn translate_sse_event( "session.updated" => translate_session_created_or_updated(properties, state, true), "session.status" => translate_session_status(properties, state), "session.idle" => translate_session_idle(properties, state), + "session.error" => translate_session_error(properties, state), "permission.asked" => translate_sse_permission(properties, state), "question.asked" => translate_question_asked(properties, state), "question.replied" => translate_question_completed(properties), @@ -1447,13 +1448,16 @@ fn translate_session_status(properties: &Value, state: &mut SessionTranslationSt vec![build_turn_started(&thread_id, &synthetic_turn_id)] } "idle" => { - if turn_id.is_empty() { + if thread_id.is_empty() { return vec![]; } let mut events = Vec::new(); if let Some(msg_completed) = build_agent_message_completed(state, &thread_id) { events.push(msg_completed); } + // Background helper prompts can complete without ever emitting an + // "active" status, so still emit turn/completed with an empty turn + // id to unblock shared background collectors. events.push(build_turn_completed(&thread_id, &turn_id)); state.finish_turn(&thread_id); events @@ -1518,6 +1522,52 @@ fn translate_session_idle(properties: &Value, state: &mut SessionTranslationStat events } +fn translate_session_error(properties: &Value, state: &mut SessionTranslationState) -> Vec { + let session_id = properties + .get("sessionID") + .or_else(|| properties.get("session_id")) + .or_else(|| properties.get("id")) + .and_then(|v| v.as_str()) + .unwrap_or_default(); + + if !session_id.is_empty() { + state.session_id = session_id.to_string(); + } + + let thread_id = state.session_id.clone(); + if thread_id.is_empty() { + return vec![]; + } + + let turn_id = state + .get_turn_state(&thread_id) + .map(|ts| ts.turn_id.clone()) + .unwrap_or_default(); + + let error = properties.get("error").unwrap_or(&Value::Null); + let error_msg = error + .get("data") + .and_then(|data| data.get("message")) + .or_else(|| error.get("message")) + .or_else(|| error.get("error")) + .or_else(|| error.get("name")) + .and_then(|v| v.as_str()) + .unwrap_or("unknown error"); + + state.finish_turn(&thread_id); + vec![json!({ + "method": "error", + "params": { + "threadId": thread_id, + "turnId": turn_id, + "willRetry": false, + "error": { + "message": error_msg + } + } + })] +} + // --------------------------------------------------------------------------- // permission.updated — permission requests from the agent // --------------------------------------------------------------------------- @@ -2441,6 +2491,48 @@ mod tests { assert!(events.iter().any(|e| e["method"] == "turn/completed")); } + #[test] + fn session_status_idle_without_active_turn_still_completes() { + let mut state = SessionTranslationState::new(String::new()); + let event = json!({ + "type": "session.status", + "properties": { + "sessionID": "ses_background_1", + "status": { "type": "idle" } + } + }); + + let events = translate_sse_event(&event, &mut state); + assert_eq!(events.len(), 1); + assert_eq!(events[0]["method"], "turn/completed"); + assert_eq!(events[0]["params"]["threadId"], "ses_background_1"); + assert_eq!(events[0]["params"]["turn"]["id"], ""); + } + + #[test] + fn session_error_produces_error_event_without_active_turn() { + let mut state = SessionTranslationState::new(String::new()); + let event = json!({ + "type": "session.error", + "properties": { + "sessionID": "ses_background_2", + "error": { + "name": "BadRequestError", + "data": { + "message": "model not available" + } + } + } + }); + + let events = translate_sse_event(&event, &mut state); + assert_eq!(events.len(), 1); + assert_eq!(events[0]["method"], "error"); + assert_eq!(events[0]["params"]["threadId"], "ses_background_2"); + assert_eq!(events[0]["params"]["turnId"], ""); + assert_eq!(events[0]["params"]["error"]["message"], "model not available"); + } + #[test] fn session_status_active_produces_turn_started_when_missing() { let mut state = SessionTranslationState::new(String::new()); diff --git a/src-tauri/src/shared/codex_aux_core.rs b/src-tauri/src/shared/codex_aux_core.rs index 83b09a2e..ba181b61 100644 --- a/src-tauri/src/shared/codex_aux_core.rs +++ b/src-tauri/src/shared/codex_aux_core.rs @@ -4,8 +4,8 @@ use std::io::ErrorKind; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; -use tokio::sync::{mpsc, Mutex}; -use tokio::time::timeout; +use tokio::sync::Mutex; +use tokio::time::{sleep, timeout}; use crate::backend::app_server::{ build_codex_command_with_bin, build_codex_path_env, check_codex_installation, WorkspaceSession, @@ -265,8 +265,6 @@ pub(crate) async fn run_background_prompt_core( prompt: String, model: Option<&str>, on_hide_thread: F, - timeout_error: &str, - turn_error_fallback: &str, ) -> Result where F: Fn(&str, &str), @@ -302,85 +300,184 @@ where ); } - let (tx, mut rx) = mpsc::unbounded_channel::(); - { - let mut callbacks = session.background_thread_callbacks.lock().await; - callbacks.insert(thread_id.clone(), tx.clone()); - } - - let prompt_path = format!("/session/{}/prompt_async", &thread_id); + let prompt_path = format!("/session/{}/message", &thread_id); let mut prompt_body = json!({ "parts": [{ "type": "text", "text": prompt }], }); - if let Some(model_id) = model { - prompt_body["model"] = json!(model_id); + if let Some(model_override) = resolve_background_prompt_model(session.as_ref(), model).await { + prompt_body["model"] = model_override; } let _prompt_guard = session.prompt_lock.lock().await; - if let Err(error) = session.rest_post(&prompt_path, prompt_body).await { - { - let mut callbacks = session.background_thread_callbacks.lock().await; - callbacks.remove(&thread_id); + let async_path = format!("/session/{}/prompt_async", &thread_id); + session.rest_post(&async_path, prompt_body).await?; + poll_background_prompt_result(session.as_ref(), &prompt_path, &thread_id).await +} + +fn extract_text_from_message_response(response: &Value) -> Result { + let Some(parts) = response.get("parts").and_then(|value| value.as_array()) else { + return Err("Failed to parse helper response parts".to_string()); + }; + + let mut text = String::new(); + for part in parts { + let part_type = part.get("type").and_then(|value| value.as_str()).unwrap_or(""); + if part_type != "text" { + continue; } - return Err(error); + if let Some(value) = part.get("text").and_then(|value| value.as_str()) { + text.push_str(value); + } + } + + Ok(text.trim().to_string()) +} + +async fn resolve_background_prompt_model( + session: &WorkspaceSession, + requested_model: Option<&str>, +) -> Option { + let requested = requested_model?.trim(); + if requested.is_empty() { + return None; + } + + if let Some((provider_id, model_id)) = requested.split_once('/') { + let provider_id = provider_id.trim(); + let model_id = model_id.trim(); + if !provider_id.is_empty() && !model_id.is_empty() { + return Some(json!({ + "providerID": provider_id, + "modelID": model_id + })); + } + } + + let providers = if let Some(cached) = session.models_cache.lock().await.clone() { + cached + } else { + let fresh = session.rest_get("/config/providers").await.ok()?; + *session.models_cache.lock().await = Some(fresh.clone()); + fresh + }; + + let providers = providers + .get("providers") + .and_then(|value| value.as_array()) + .cloned() + .unwrap_or_default(); + + let mut matches = Vec::new(); + for provider in providers { + let provider_id = provider + .get("id") + .and_then(|value| value.as_str()) + .map(str::trim) + .unwrap_or_default(); + if provider_id.is_empty() { + continue; + } + + let Some(models) = provider.get("models").and_then(|value| value.as_object()) else { + continue; + }; + + let found = models.contains_key(requested) + || models.values().any(|model| { + model + .get("id") + .and_then(|value| value.as_str()) + .map(str::trim) + .unwrap_or_default() + == requested + }); + if found { + matches.push(provider_id.to_string()); + } + } + + if matches.len() == 1 { + Some(json!({ + "providerID": matches[0], + "modelID": requested + })) + } else { + None } +} + +async fn poll_background_prompt_result( + session: &WorkspaceSession, + messages_path: &str, + thread_id: &str, +) -> Result { + timeout(Duration::from_secs(90), async { + let mut idle_polls = 0usize; - let mut response_text = String::new(); - let collect_result = timeout(Duration::from_secs(60), async { loop { - let Some(event) = rx.recv().await else { - return Err("Background response stream closed before completion".to_string()); - }; - let method = event.get("method").and_then(|m| m.as_str()).unwrap_or(""); - match method { - "item/agentMessage/delta" => { - if let Some(params) = event.get("params") { - if let Some(delta) = params.get("delta").and_then(|d| d.as_str()) { - response_text.push_str(delta); - } - } - } - "turn/completed" => break, - "turn/error" => { - let error_msg = event - .get("params") - .and_then(|p| p.get("error")) - .and_then(|e| e.as_str()) - .unwrap_or(turn_error_fallback); - return Err(error_msg.to_string()); - } - "error" => { - let error_msg = event - .get("params") - .and_then(|p| p.get("error")) - .and_then(|e| e.get("message").or_else(|| e.get("error"))) - .and_then(|e| e.as_str()) - .unwrap_or(turn_error_fallback); - return Err(error_msg.to_string()); - } - _ => {} + let statuses = session.rest_get("/session/status").await?; + let status_type = statuses + .get(thread_id) + .and_then(|status| status.get("type")) + .and_then(|value| value.as_str()); + + let messages = session.rest_get(messages_path).await?; + if let Some(result) = extract_background_prompt_result_from_messages(&messages) { + return result; + } + + match status_type { + Some("busy") | Some("retry") => idle_polls = 0, + _ => idle_polls += 1, } + + if idle_polls >= 4 { + return Err("No response was generated".to_string()); + } + + sleep(Duration::from_millis(250)).await; } - Ok(()) }) - .await; + .await + .map_err(|_| "Timeout waiting for helper response".to_string())? +} - { - let mut callbacks = session.background_thread_callbacks.lock().await; - callbacks.remove(&thread_id); - } +fn extract_background_prompt_result_from_messages(messages: &Value) -> Option> { + let entries = messages.as_array()?; - match collect_result { - Ok(Ok(())) => {} - Ok(Err(error)) => return Err(error), - Err(_) => return Err(timeout_error.to_string()), - } + for entry in entries.iter().rev() { + let info = entry.get("info")?; + if info.get("role").and_then(|value| value.as_str()) != Some("assistant") { + continue; + } - let trimmed = response_text.trim().to_string(); - if trimmed.is_empty() { - return Err("No response was generated".to_string()); + if let Some(error) = info.get("error") { + let message = error + .get("data") + .and_then(|data| data.get("message")) + .or_else(|| error.get("message")) + .or_else(|| error.get("error")) + .or_else(|| error.get("name")) + .and_then(|value| value.as_str()) + .unwrap_or("Unknown error during helper prompt"); + return Some(Err(message.to_string())); + } + + let is_completed = info + .get("time") + .and_then(|time| time.get("completed")) + .is_some(); + if !is_completed { + return None; + } + + let text = extract_text_from_message_response(entry).unwrap_or_default(); + if text.is_empty() { + return Some(Err("No response was generated".to_string())); + } + return Some(Ok(text)); } - Ok(trimmed) + None } async fn remember_hidden_session_id( @@ -433,8 +530,6 @@ where prompt, model, on_hide_thread, - "Timeout waiting for commit message generation", - "Unknown error during commit message generation", ) .await } @@ -464,8 +559,6 @@ where metadata_prompt, None, on_hide_thread, - "Timeout waiting for metadata generation", - "Unknown error during metadata generation", ) .await?; @@ -474,7 +567,11 @@ where #[cfg(test)] mod tests { - use super::{build_commit_message_prompt_for_diff, parse_run_metadata_value}; + use super::{ + build_commit_message_prompt_for_diff, extract_text_from_message_response, + parse_run_metadata_value, + }; + use serde_json::json; #[test] fn build_commit_message_prompt_for_diff_requires_changes() { @@ -503,4 +600,18 @@ mod tests { "Missing title in metadata" ); } + + #[test] + fn extract_text_from_message_response_concatenates_text_parts() { + let response = json!({ + "info": { "id": "msg_1" }, + "parts": [ + { "type": "reasoning", "text": "thinking" }, + { "type": "text", "text": "fix: " }, + { "type": "text", "text": "update parser" } + ] + }); + let parsed = extract_text_from_message_response(&response).expect("helper text"); + assert_eq!(parsed, "fix: update parser"); + } } diff --git a/src/App.tsx b/src/App.tsx index 9346f54f..49a894a9 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1308,6 +1308,7 @@ function MainApp() { activeWorkspace, activeWorkspaceId, activeWorkspaceIdRef, + commitMessageModelId: selectedModelId, gitStatus, refreshGitStatus, refreshGitLog, diff --git a/src/features/about/components/AboutView.tsx b/src/features/about/components/AboutView.tsx index f0569443..70758c25 100644 --- a/src/features/about/components/AboutView.tsx +++ b/src/features/about/components/AboutView.tsx @@ -2,7 +2,7 @@ import { useEffect, useState } from "react"; import { getVersion } from "@tauri-apps/api/app"; import { openUrl } from "@tauri-apps/plugin-opener"; -const GITHUB_URL = "https://github.com/jacob/OpenCodeMonitor"; +const GITHUB_URL = "https://github.com/jacobjmc/OpenCodeMonitor"; const TWITTER_URL = "https://x.com/jacob"; export function AboutView() { diff --git a/src/features/app/hooks/useGitCommitController.ts b/src/features/app/hooks/useGitCommitController.ts index 9b686bc3..b78a725e 100644 --- a/src/features/app/hooks/useGitCommitController.ts +++ b/src/features/app/hooks/useGitCommitController.ts @@ -18,6 +18,7 @@ type GitCommitControllerOptions = { activeWorkspace: WorkspaceInfo | null; activeWorkspaceId: string | null; activeWorkspaceIdRef: RefObject; + commitMessageModelId: string | null; gitStatus: GitStatusState; refreshGitStatus: () => void; refreshGitLog?: () => void; @@ -53,6 +54,7 @@ export function useGitCommitController({ activeWorkspace, activeWorkspaceId, activeWorkspaceIdRef, + commitMessageModelId, gitStatus, refreshGitStatus, refreshGitLog, @@ -100,7 +102,7 @@ export function useGitCommitController({ setCommitMessageLoading(true); setCommitMessageError(null); try { - const message = await generateCommitMessage(workspaceId, null); + const message = await generateCommitMessage(workspaceId, commitMessageModelId); if (!shouldApplyCommitMessage(activeWorkspaceIdRef.current, workspaceId)) { return; } @@ -117,7 +119,12 @@ export function useGitCommitController({ setCommitMessageLoading(false); } } - }, [activeWorkspace, commitMessageLoading, activeWorkspaceIdRef]); + }, [ + activeWorkspace, + commitMessageLoading, + activeWorkspaceIdRef, + commitMessageModelId, + ]); useEffect(() => { setCommitMessage(""); diff --git a/src/features/threads/hooks/threadReducer/threadLifecycleSlice.ts b/src/features/threads/hooks/threadReducer/threadLifecycleSlice.ts index 3647d886..9d10cbaf 100644 --- a/src/features/threads/hooks/threadReducer/threadLifecycleSlice.ts +++ b/src/features/threads/hooks/threadReducer/threadLifecycleSlice.ts @@ -150,6 +150,14 @@ export function reduceThreadLifecycle( }; } case "removeThread": { + const hiddenForWorkspace = + state.hiddenThreadIdsByWorkspace[action.workspaceId] ?? {}; + const nextHiddenForWorkspace = hiddenForWorkspace[action.threadId] + ? hiddenForWorkspace + : { + ...hiddenForWorkspace, + [action.threadId]: true as const, + }; const list = state.threadsByWorkspace[action.workspaceId] ?? []; const filtered = list.filter((thread) => thread.id !== action.threadId); const nextActive = @@ -164,6 +172,10 @@ export function reduceThreadLifecycle( const { [action.threadId]: ______, ...restParents } = state.threadParentById; return { ...state, + hiddenThreadIdsByWorkspace: { + ...state.hiddenThreadIdsByWorkspace, + [action.workspaceId]: nextHiddenForWorkspace, + }, threadsByWorkspace: { ...state.threadsByWorkspace, [action.workspaceId]: filtered, diff --git a/src/features/threads/hooks/useThreadsReducer.test.ts b/src/features/threads/hooks/useThreadsReducer.test.ts index d5405ffd..155cb6d7 100644 --- a/src/features/threads/hooks/useThreadsReducer.test.ts +++ b/src/features/threads/hooks/useThreadsReducer.test.ts @@ -713,6 +713,38 @@ describe("threadReducer", () => { expect(next.turnDiffByThread["thread-1"]).toBeUndefined(); }); + it("keeps removed threads hidden on future syncs", () => { + const base: ThreadState = { + ...initialState, + threadsByWorkspace: { + "ws-1": [{ id: "thread-1", name: "Agent 1", updatedAt: 1 }], + }, + activeThreadIdByWorkspace: { "ws-1": "thread-1" }, + }; + + const removed = threadReducer(base, { + type: "removeThread", + workspaceId: "ws-1", + threadId: "thread-1", + }); + + expect(removed.hiddenThreadIdsByWorkspace["ws-1"]?.["thread-1"]).toBe(true); + + const synced = threadReducer(removed, { + type: "setThreads", + workspaceId: "ws-1", + sortKey: "updated_at", + threads: [ + { id: "thread-1", name: "Archived thread", updatedAt: 10 }, + { id: "thread-2", name: "Visible thread", updatedAt: 11 }, + ], + }); + + expect(synced.threadsByWorkspace["ws-1"]?.map((thread) => thread.id)).toEqual([ + "thread-2", + ]); + }); + it("hides background threads and keeps them hidden on future syncs", () => { const withThread = threadReducer(initialState, { type: "ensureThread",