diff --git a/src/apps/desktop/src/api/review_platform_api.rs b/src/apps/desktop/src/api/review_platform_api.rs index ec2440630f..a951512c7a 100644 --- a/src/apps/desktop/src/api/review_platform_api.rs +++ b/src/apps/desktop/src/api/review_platform_api.rs @@ -3,10 +3,10 @@ use crate::api::app_state::AppState; use log::error; use openbitfun_core::service::review_platform::{ - ReviewPlatformCiLog, ReviewPlatformDetailSection, ReviewPlatformError, - ReviewPlatformIssueEvidence, ReviewPlatformKind, ReviewPlatformPullRequestDetail, - ReviewPlatformPullRequestDetailPage, ReviewPlatformPullRequestReviewTarget, - ReviewPlatformService, ReviewPlatformWorkspaceSnapshot, + untrusted_repository_error_message, ReviewPlatformCiLog, ReviewPlatformDetailSection, + ReviewPlatformError, ReviewPlatformIssueEvidence, ReviewPlatformKind, + ReviewPlatformPullRequestDetail, ReviewPlatformPullRequestDetailPage, + ReviewPlatformPullRequestReviewTarget, ReviewPlatformService, ReviewPlatformWorkspaceSnapshot, }; use serde::Deserialize; use tauri::State; @@ -88,10 +88,7 @@ pub async fn review_platform_get_workspace_snapshot( "Failed to get review platform workspace snapshot: path={}, remote_id={:?}, error={}", request.repository_path, request.remote_id, error ); - format!( - "Failed to get review platform workspace snapshot: {}", - error - ) + review_platform_command_error("Failed to get review platform workspace snapshot", &error) }) } @@ -104,10 +101,13 @@ pub async fn review_platform_get_workspace_context( .await .map_err(|error| { error!( - "Failed to get review platform workspace context: path={}, remote_id={:?}, error={}", - request.repository_path, request.remote_id, error - ); - format!("Failed to get review platform workspace context: {}", error) + "Failed to get review platform workspace context: path={}, remote_id={:?}, error={}", + request.repository_path, request.remote_id, error + ); + review_platform_command_error( + "Failed to get review platform workspace context", + &error, + ) }) } @@ -130,7 +130,10 @@ pub async fn review_platform_get_pull_request_detail( request.pull_request_id, error ); - format!("Failed to get review platform pull request detail: {}", error) + review_platform_command_error( + "Failed to get review platform pull request detail", + &error, + ) }) } @@ -153,7 +156,7 @@ pub async fn review_platform_get_pull_request_review_target( request.pull_request_id, error ); - format!("Failed to prepare pull request Review target: {}", error) + review_platform_command_error("Failed to prepare pull request Review target", &error) }) } @@ -182,7 +185,7 @@ pub async fn review_platform_get_issue( request.issue_id, safe_error ); - format!("Failed to get provider Issue evidence: {safe_error}") + safe_review_platform_command_error("Failed to get provider Issue evidence", &error) }) } @@ -209,10 +212,26 @@ pub async fn review_platform_get_pull_request_review_target_by_identity( request.pull_request_id, safe_error ); - format!("Failed to prepare pull request Review target: {safe_error}") + safe_review_platform_command_error("Failed to prepare pull request Review target", &error) }) } +fn review_platform_command_error(context: &str, error: &ReviewPlatformError) -> String { + if let Some(repository_path) = error.untrusted_repository_path() { + return untrusted_repository_error_message(repository_path); + } + + format!("{context}: {error}") +} + +fn safe_review_platform_command_error(context: &str, error: &ReviewPlatformError) -> String { + if let Some(repository_path) = error.untrusted_repository_path() { + return untrusted_repository_error_message(repository_path); + } + + format!("{context}: {}", safe_review_platform_error(error)) +} + fn safe_review_platform_error(error: &ReviewPlatformError) -> String { match error { ReviewPlatformError::Http { status, .. } => format!("provider returned HTTP {status}"), @@ -228,6 +247,9 @@ fn safe_review_platform_error(error: &ReviewPlatformError) -> String { "requested Issue is a pull request".to_string() } ReviewPlatformError::InvalidRepository(_) => "invalid repository".to_string(), + ReviewPlatformError::RepositoryUntrusted { .. } => { + "repository ownership is not trusted".to_string() + } ReviewPlatformError::RemoteNotFound(_) => "provider remote was not found".to_string(), ReviewPlatformError::UnsupportedPlatform(_) => "unsupported provider".to_string(), ReviewPlatformError::Api(_) => "provider request was rejected".to_string(), @@ -259,9 +281,9 @@ pub async fn review_platform_get_pull_request_detail_page( request.per_page, error ); - format!( - "Failed to get review platform pull request detail page: {}", - error + review_platform_command_error( + "Failed to get review platform pull request detail page", + &error, ) }) } @@ -288,7 +310,7 @@ pub async fn review_platform_get_pull_request_ci_log( request.ci_item_id, error ); - format!("Failed to get review platform CI log: {}", error) + review_platform_command_error("Failed to get review platform CI log", &error) }) } @@ -351,6 +373,37 @@ mod tests { use super::*; use serde_json::json; + #[test] + fn review_platform_command_errors_preserve_the_repository_trust_code() { + let error = ReviewPlatformError::RepositoryUntrusted { + repository_path: "/srv/shared/repo".to_string(), + detail: "fatal: detected dubious ownership".to_string(), + }; + + assert_eq!( + review_platform_command_error("Failed to load review platform", &error), + "git_repository_untrusted: /srv/shared/repo" + ); + assert_eq!( + safe_review_platform_command_error("Failed to load review platform", &error), + "git_repository_untrusted: /srv/shared/repo" + ); + } + + #[test] + fn review_platform_command_errors_keep_context_for_other_failures() { + let error = ReviewPlatformError::RemoteNotFound("origin".to_string()); + + assert_eq!( + review_platform_command_error("Failed to load review platform", &error), + "Failed to load review platform: Remote not found: origin" + ); + assert_eq!( + safe_review_platform_command_error("Failed to load review platform", &error), + "Failed to load review platform: provider remote was not found" + ); + } + #[test] fn review_platform_request_wire_deserializes_issue_identity_fields() { let request: ReviewPlatformIssueRequest = serde_json::from_value(json!({ diff --git a/src/crates/assembly/core/src/service/review_platform/mod.rs b/src/crates/assembly/core/src/service/review_platform/mod.rs index a7d991be72..12e2fbb8c5 100644 --- a/src/crates/assembly/core/src/service/review_platform/mod.rs +++ b/src/crates/assembly/core/src/service/review_platform/mod.rs @@ -9,8 +9,9 @@ use crate::infrastructure::try_get_path_manager_arc; use std::sync::Arc; pub use openbitfun_services_integrations::review_platform::{ - ReviewAuthSource, ReviewAuthState, ReviewChecks, ReviewDecision, ReviewEvidenceCompleteness, - ReviewFileStatus, ReviewItemState, ReviewPlatformAccount, ReviewPlatformActionResult, + classify_git_command_failure, untrusted_repository_error_message, ReviewAuthSource, + ReviewAuthState, ReviewChecks, ReviewDecision, ReviewEvidenceCompleteness, ReviewFileStatus, + ReviewItemState, ReviewPlatformAccount, ReviewPlatformActionResult, ReviewPlatformApprovalRequest, ReviewPlatformAuthChallenge, ReviewPlatformAuthChallengeState, ReviewPlatformCapabilities, ReviewPlatformCiItem, ReviewPlatformCiLog, ReviewPlatformCommit, ReviewPlatformCreatePullRequestRequest, ReviewPlatformDetailSection, ReviewPlatformError, @@ -105,7 +106,8 @@ impl ReviewPlatformWorkspaceClassifier for CoreReviewPlatformWorkspaceClassifier } else { stderr }; - return Err(ReviewPlatformError::InvalidRepository( + return Err(classify_git_command_failure( + current_dir, message.trim().to_string(), )); } diff --git a/src/crates/services/services-integrations/src/git/trust.rs b/src/crates/services/services-integrations/src/git/trust.rs index 5ea2a1701a..1a8db8cf46 100644 --- a/src/crates/services/services-integrations/src/git/trust.rs +++ b/src/crates/services/services-integrations/src/git/trust.rs @@ -24,14 +24,24 @@ use super::GitError; use serde::{Deserialize, Serialize}; use std::path::Path; -/// Ownership rejection wordings emitted by the Git CLI and by libgit2. -const OWNERSHIP_REJECTION_MARKERS: [&str; 5] = [ - "dubious ownership", - "is owned by someone else", - "is not owned by current user", - "not owned by current user", - "owned by a different user", -]; +pub const REPOSITORY_UNTRUSTED_ERROR_PREFIX: &str = + crate::repository_trust::REPOSITORY_UNTRUSTED_ERROR_PREFIX; + +pub fn is_untrusted_repository_message(message: &str) -> bool { + crate::repository_trust::is_untrusted_repository_message(message) +} + +pub fn normalize_trust_path(raw: &str) -> Option { + crate::repository_trust::normalize_trust_path(raw) +} + +pub fn untrusted_repository_path_from_message(message: &str) -> Option { + crate::repository_trust::untrusted_repository_path_from_message(message) +} + +pub fn untrusted_repository_error_message(repository_path: &str) -> String { + crate::repository_trust::untrusted_repository_error_message(repository_path) +} // Repository-level phrasings only. A bare "does not exist" also matches Git // talking about an object, a ref or a pathspec inside a repository that is @@ -89,11 +99,6 @@ fn contains_any(message: &str, markers: &[&str]) -> bool { markers.iter().any(|marker| lowered.contains(marker)) } -/// Whether a Git/libgit2 diagnostic is an ownership rejection. -pub fn is_untrusted_repository_message(message: &str) -> bool { - contains_any(message, &OWNERSHIP_REJECTION_MARKERS) -} - /// Whether a Git/libgit2 diagnostic says no repository is there. /// /// What is neither this nor [`is_untrusted_repository_message`] is a probe that @@ -105,110 +110,6 @@ pub fn is_missing_repository_message(message: &str) -> bool { contains_any(message, &MISSING_REPOSITORY_MARKERS) } -/// True when the value carries a shape only Windows produces: a `\\server\share` -/// or `\\?\...` prefix, or a `C:\` / `C:/` drive root. -/// -/// This is a shape test on purpose, not `cfg!(windows)`: a Windows desktop -/// driving a remote Linux workspace normalizes that host's paths, and a POSIX -/// host can be handed a Windows path by a peer. -fn looks_like_windows_path(value: &str) -> bool { - if value.starts_with('\\') { - return true; - } - let mut chars = value.chars(); - matches!( - (chars.next(), chars.next(), chars.next()), - (Some(drive), Some(':'), Some('\\' | '/')) if drive.is_ascii_alphabetic() - ) -} - -/// Normalizes a repository path into the shape Git compares `safe.directory` -/// entries against: forward slashes, no extended-length prefix, no trailing -/// separator. -pub fn normalize_trust_path(raw: &str) -> Option { - let trimmed = raw.trim().trim_matches('\'').trim_matches('"').trim(); - if trimmed.is_empty() { - return None; - } - - // Only a Windows-shaped path may have its backslashes rewritten. Backslash - // is an ordinary filename character on POSIX, and rewriting it would turn - // `/srv/we\ird/repo` into a `safe.directory` entry that can never match and - // a manual command naming a directory that does not exist. - let mut value = if looks_like_windows_path(trimmed) { - trimmed.replace('\\', "/") - } else { - trimmed.to_string() - }; - // `\\?\UNC\server\share` is the extended-length spelling of the UNC path - // `\\server\share`. Dropping the whole prefix would leave - // `UNC/server/share`, which Git never matches — and the manual command we - // hand the user would name a path that does not exist. - let unc_prefix = value - .get(..8) - .filter(|prefix| prefix.eq_ignore_ascii_case("//?/UNC/")); - if unc_prefix.is_some() { - value = format!("//{}", &value[8..]); - } else if let Some(stripped) = value.strip_prefix("//?/") { - value = stripped.to_string(); - } - while value.len() > 1 && value.ends_with('/') && !value.ends_with(":/") { - value.pop(); - } - - (!value.is_empty()).then_some(value) -} - -/// Extracts the repository path Git named in an ownership rejection. -/// -/// Both wordings quote the path first, before any remediation hint: -/// `detected dubious ownership in repository at ''` (CLI) and -/// `repository path '' is not owned by current user` (libgit2). -/// -/// Only the line carrying the rejection itself is read. The CLI's advice block -/// repeats the path on later lines, and reading those would make the result -/// depend on how far the prose was truncated. -pub fn untrusted_repository_path_from_message(message: &str) -> Option { - if !is_untrusted_repository_message(message) { - return None; - } - - message - .lines() - .filter(|line| contains_any(line, &OWNERSHIP_REJECTION_MARKERS)) - .find_map(quoted_path_on_line) - // A wording we have not seen may put the path on its own line. Falling - // back to any quoted span still beats losing the path entirely. - .or_else(|| message.lines().find_map(quoted_path_on_line)) -} - -/// Takes the widest quoted span on a line: first quote to last quote of the same -/// kind. -/// -/// Git does not escape quotes inside the path it prints, so stopping at the -/// first closing quote truncates `/srv/a'b/repo` to `/srv/a` — and that -/// truncated path outranks the caller's in `classify_command_failure`, so it is -/// what would be written into the user's global `safe.directory`, never -/// reclaimed, and still not resolve the rejection. -fn quoted_path_on_line(line: &str) -> Option { - for quote in ['\'', '"'] { - let Some(open) = line.find(quote) else { - continue; - }; - let start = open + quote.len_utf8(); - let Some(end) = line.rfind(quote) else { - continue; - }; - if end < start { - continue; - } - if let Some(path) = normalize_trust_path(&line[start..end]) { - return Some(path); - } - } - None -} - /// The command a user can run themselves when the product cannot apply the /// decision (remote workspace, peer host, restricted configuration). /// @@ -280,18 +181,6 @@ fn shell_single_quote(value: &str) -> String { format!("'{}'", value.replace('\'', r"'\''")) } -/// Stable prefix every interface uses to carry an ownership rejection across a -/// command boundary that only transports strings (Tauri `Result`, -/// JSON-RPC error `data`). Frontends branch on this instead of on prose, which -/// is localized by Git itself and differs between the CLI and libgit2. -pub const REPOSITORY_UNTRUSTED_ERROR_PREFIX: &str = "git_repository_untrusted:"; - -/// Boundary error string for an ownership rejection. One producer so the -/// desktop and app-server surfaces cannot drift apart on the wire. -pub fn untrusted_repository_error_message(repository_path: &str) -> String { - format!("{REPOSITORY_UNTRUSTED_ERROR_PREFIX} {repository_path}") -} - fn untrusted_error(repository_path: Option, detail: impl Into) -> GitError { GitError::RepositoryUntrusted { repository_path: repository_path.unwrap_or_default(), diff --git a/src/crates/services/services-integrations/src/lib.rs b/src/crates/services/services-integrations/src/lib.rs index ef0bfd48f8..7ca6e7e521 100644 --- a/src/crates/services/services-integrations/src/lib.rs +++ b/src/crates/services/services-integrations/src/lib.rs @@ -72,6 +72,9 @@ pub mod miniapp_market; #[cfg(feature = "plugin-source")] pub mod plugin_source; +#[cfg(any(feature = "git", feature = "review-platform"))] +mod repository_trust; + #[cfg(feature = "remote-connect")] pub mod remote_connect; diff --git a/src/crates/services/services-integrations/src/repository_trust.rs b/src/crates/services/services-integrations/src/repository_trust.rs new file mode 100644 index 0000000000..5f59daf55e --- /dev/null +++ b/src/crates/services/services-integrations/src/repository_trust.rs @@ -0,0 +1,141 @@ +//! Feature-light Git repository ownership trust contract. +//! +//! Both the full Git service and Review Platform execute Git commands, but +//! Review Platform deliberately remains independently compilable without the +//! heavier `git` feature. Keep the ownership diagnostic classification and +//! stable boundary code here so those two capability slices cannot drift. + +/// Ownership rejection wordings emitted by the Git CLI and by libgit2. +const OWNERSHIP_REJECTION_MARKERS: [&str; 5] = [ + "dubious ownership", + "is owned by someone else", + "is not owned by current user", + "not owned by current user", + "owned by a different user", +]; + +fn contains_any(message: &str, markers: &[&str]) -> bool { + let lowered = message.to_lowercase(); + markers.iter().any(|marker| lowered.contains(marker)) +} + +/// Whether a Git/libgit2 diagnostic is an ownership rejection. +pub(crate) fn is_untrusted_repository_message(message: &str) -> bool { + contains_any(message, &OWNERSHIP_REJECTION_MARKERS) +} + +/// True when the value carries a shape only Windows produces: a `\\server\share` +/// or `\\?\...` prefix, or a `C:\` / `C:/` drive root. +fn looks_like_windows_path(value: &str) -> bool { + if value.starts_with('\\') { + return true; + } + let mut chars = value.chars(); + matches!( + (chars.next(), chars.next(), chars.next()), + (Some(drive), Some(':'), Some('\\' | '/')) if drive.is_ascii_alphabetic() + ) +} + +/// Normalizes a repository path into the shape Git compares `safe.directory` +/// entries against: forward slashes, no extended-length prefix, no trailing +/// separator. +pub(crate) fn normalize_trust_path(raw: &str) -> Option { + let trimmed = raw.trim().trim_matches('\'').trim_matches('"').trim(); + if trimmed.is_empty() { + return None; + } + + // Only a Windows-shaped path may have its backslashes rewritten. Backslash + // is an ordinary filename character on POSIX. + let mut value = if looks_like_windows_path(trimmed) { + trimmed.replace('\\', "/") + } else { + trimmed.to_string() + }; + if value + .get(..8) + .is_some_and(|prefix| prefix.eq_ignore_ascii_case("//?/UNC/")) + { + value = format!("//{}", &value[8..]); + } else if let Some(stripped) = value.strip_prefix("//?/") { + value = stripped.to_string(); + } + while value.len() > 1 && value.ends_with('/') && !value.ends_with(":/") { + value.pop(); + } + + (!value.is_empty()).then_some(value) +} + +/// Extracts the repository path Git named in an ownership rejection. +pub(crate) fn untrusted_repository_path_from_message(message: &str) -> Option { + if !is_untrusted_repository_message(message) { + return None; + } + + message + .lines() + .filter(|line| contains_any(line, &OWNERSHIP_REJECTION_MARKERS)) + .find_map(quoted_path_on_line) + .or_else(|| message.lines().find_map(quoted_path_on_line)) +} + +fn quoted_path_on_line(line: &str) -> Option { + for quote in ['\'', '"'] { + let Some(open) = line.find(quote) else { + continue; + }; + let start = open + quote.len_utf8(); + let Some(end) = line.rfind(quote) else { + continue; + }; + if end < start { + continue; + } + if let Some(path) = normalize_trust_path(&line[start..end]) { + return Some(path); + } + } + None +} + +/// Stable prefix used across string-only Desktop and JSON-RPC boundaries. +pub(crate) const REPOSITORY_UNTRUSTED_ERROR_PREFIX: &str = "git_repository_untrusted:"; + +/// Boundary error string for an ownership rejection. +pub(crate) fn untrusted_repository_error_message(repository_path: &str) -> String { + format!("{REPOSITORY_UNTRUSTED_ERROR_PREFIX} {repository_path}") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn classifies_cli_ownership_rejection_and_preserves_the_path() { + let message = concat!( + "fatal: detected dubious ownership in repository at '/srv/shared/repo'\n", + "To add an exception for this directory, call:\n", + "git config --global --add safe.directory /srv/shared/repo", + ); + + assert!(is_untrusted_repository_message(message)); + assert_eq!( + untrusted_repository_path_from_message(message).as_deref(), + Some("/srv/shared/repo") + ); + } + + #[test] + fn normalizes_windows_paths_without_rewriting_posix_backslashes() { + assert_eq!( + normalize_trust_path("\\\\?\\C:\\work\\repo\\").as_deref(), + Some("C:/work/repo") + ); + assert_eq!( + normalize_trust_path("/srv/we\\ird/repo").as_deref(), + Some("/srv/we\\ird/repo") + ); + } +} diff --git a/src/crates/services/services-integrations/src/review_platform.rs b/src/crates/services/services-integrations/src/review_platform.rs index fc3030a241..d2b7bec237 100644 --- a/src/crates/services/services-integrations/src/review_platform.rs +++ b/src/crates/services/services-integrations/src/review_platform.rs @@ -4,6 +4,9 @@ //! and provider-neutral review-platform response semantics. Concrete HTTP //! transport lives in `review_platform_http`. +use crate::repository_trust::{ + is_untrusted_repository_message, normalize_trust_path, untrusted_repository_path_from_message, +}; use crate::review_platform_http::{ send_json as send_review_json, send_json_response as send_review_json_response, send_json_response_bounded as send_review_json_response_bounded, @@ -64,6 +67,11 @@ static TOKEN_STORE_TEMP_NONCE: std::sync::atomic::AtomicU64 = std::sync::atomic: pub enum ReviewPlatformError { #[error("Invalid repository path: {0}")] InvalidRepository(String), + #[error("Repository ownership is not trusted: {repository_path}")] + RepositoryUntrusted { + repository_path: String, + detail: String, + }, #[error("Remote not found: {0}")] RemoteNotFound(String), #[error("Unsupported review platform: {0}")] @@ -84,6 +92,38 @@ pub enum ReviewPlatformError { TargetIsPullRequest { issue_id: String }, } +impl ReviewPlatformError { + pub fn untrusted_repository_path(&self) -> Option<&str> { + match self { + Self::RepositoryUntrusted { + repository_path, .. + } => Some(repository_path), + _ => None, + } + } +} + +/// Stable boundary error for a repository ownership rejection. +pub fn untrusted_repository_error_message(repository_path: &str) -> String { + crate::repository_trust::untrusted_repository_error_message(repository_path) +} + +/// Classifies a failed Git probe without making Review Platform depend on the +/// full Git capability feature. +pub fn classify_git_command_failure(repository_path: &str, message: String) -> ReviewPlatformError { + if is_untrusted_repository_message(&message) { + let repository_path = untrusted_repository_path_from_message(&message) + .or_else(|| normalize_trust_path(repository_path)) + .unwrap_or_else(|| repository_path.to_string()); + return ReviewPlatformError::RepositoryUntrusted { + repository_path, + detail: message, + }; + } + + ReviewPlatformError::InvalidRepository(message) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum ReviewPlatformKind { @@ -4385,7 +4425,7 @@ async fn execute_git_command( } else { String::from_utf8_lossy(&output.stderr).to_string() }; - Err(ReviewPlatformError::InvalidRepository(message)) + Err(classify_git_command_failure(current_dir, message)) } fn review_evidence_error(error: ReviewPlatformError, resource: &str) -> ReviewPlatformError { @@ -7487,6 +7527,42 @@ mod tests { }; use tokio::fs; + #[test] + fn git_ownership_rejection_keeps_a_stable_repository_trust_contract() { + let detail = concat!( + "fatal: detected dubious ownership in repository at '/srv/shared/repo'\n", + "To add an exception for this directory, call:\n", + "git config --global --add safe.directory /srv/shared/repo", + ); + + let error = classify_git_command_failure("/srv/controller/path", detail.to_string()); + + assert!(matches!( + error, + ReviewPlatformError::RepositoryUntrusted { + ref repository_path, + detail: ref captured_detail, + } if repository_path == "/srv/shared/repo" && captured_detail == detail + )); + assert_eq!(error.untrusted_repository_path(), Some("/srv/shared/repo")); + assert_eq!( + untrusted_repository_error_message("/srv/shared/repo"), + "git_repository_untrusted: /srv/shared/repo" + ); + } + + #[test] + fn ordinary_git_failure_remains_an_invalid_repository_error() { + let error = + classify_git_command_failure("/srv/project", "fatal: not a git repository".to_string()); + + assert!(matches!( + error, + ReviewPlatformError::InvalidRepository(ref detail) + if detail == "fatal: not a git repository" + )); + } + struct AlwaysRemoteWorkspace; #[async_trait::async_trait] diff --git a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.trust.test.tsx b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.trust.test.tsx new file mode 100644 index 0000000000..50ff711139 --- /dev/null +++ b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.trust.test.tsx @@ -0,0 +1,107 @@ +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest'; +import { ReviewPlatformPanel } from './ReviewPlatformPanel'; +import { resetGitTrustDecisions } from '@/shared/services/gitTrustService'; +import { TauriCommandError } from '@/infrastructure/api/errors/TauriCommandError'; + +const mocks = vi.hoisted(() => ({ + snapshot: vi.fn(), context: vi.fn(), confirm: vi.fn(), trust: vi.fn(), + state: { sessions: new Map(), activeSessionId: null }, +})); +vi.mock('@/infrastructure/api', () => ({ + reviewPlatformAPI: { getWorkspaceSnapshot: mocks.snapshot, getWorkspaceContext: mocks.context }, + systemAPI: {}, + gitAPI: { trustRepository: mocks.trust, getRepositoryTrust: vi.fn() }, +})); +vi.mock('@/infrastructure/confirm-dialog', () => ({ confirmWarning: mocks.confirm })); +vi.mock('@/infrastructure/i18n', () => ({ + i18nService: { t: (key: string) => key }, +})); +vi.mock('@/shared/utils/logger', () => ({ + createLogger: () => ({ error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() }), +})); +vi.mock('@/shared/notification-system', () => ({ + notificationService: { warning: vi.fn(), success: vi.fn(), error: vi.fn() }, +})); +vi.mock('@/flow_chat/store/FlowChatStore', () => ({ + flowChatStore: { getState: () => mocks.state, subscribe: () => () => {} }, +})); +vi.mock('@/flow_chat/components/DeepReviewConsentDialog', () => ({ + useDeepReviewConsent: () => ({ confirmDeepReviewLaunch: vi.fn(), deepReviewConsentDialog: null }), +})); +vi.mock('@/flow_chat/services/ReviewService', () => ({ + launchPreparedReviewSession: vi.fn(), prepareReviewLaunchFromPullRequest: vi.fn(), +})); +vi.mock('@/flow_chat/services/sessionActivation', () => ({ openMainSession: vi.fn() })); +vi.mock('@/flow_chat/services/btwSessionPane', () => ({ openBtwSessionInAuxPane: vi.fn() })); +vi.mock('@/shared/services/ide-control', () => ({ quickActions: {} })); +vi.mock('@/shared/stores/contextStore', () => ({ useContextStore: {} })); +vi.mock('@/infrastructure/markdown', () => ({ MarkdownRenderer: () => null })); +vi.mock('@openbitfun/ui', () => { + const Box = ({ children }: { children?: React.ReactNode }) =>
{children}
; + const Button = ({ children, onClick, disabled, 'aria-label': label }: { + children?: React.ReactNode; onClick?: () => void; disabled?: boolean; 'aria-label'?: string; + }) => ; + return { + Button, IconButton: Button, Icon: () => null, Input: () => null, Combobox: () => null, + Field: Box, ScrollArea: Box, TabGroup: () => null, Tooltip: Box, + Dialog: () => null, DialogBody: Box, DialogClose: Box, DialogHeader: Box, + DialogHeading: Box, DialogTitle: Box, + }; +}); + +let dom: { window: Window & typeof globalThis }; +let root: Root; +let container: HTMLDivElement; +const workspacePath = '/workspace/review-trust-test'; +const error = new TauriCommandError('Command failed', { + command: 'review_platform_get_workspace_snapshot', + originalError: `git_repository_untrusted: ${workspacePath}`, +}); + +beforeEach(async () => { + const { JSDOM } = await import('jsdom'); + dom = new JSDOM('', { url: 'http://localhost' }); + vi.stubGlobal('window', dom.window); + vi.stubGlobal('document', dom.window.document); + vi.stubGlobal('localStorage', dom.window.localStorage); + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + vi.resetAllMocks(); + resetGitTrustDecisions(); + mocks.snapshot.mockRejectedValue(error); + mocks.context.mockRejectedValue(error); +}); + +afterEach(() => { + act(() => root.unmount()); + dom.window.close(); + vi.unstubAllGlobals(); +}); + +describe('Review platform trust interaction', () => { + it.each([false, true])('does not prompt during automatic loading (detailOnly=%s)', async (detailOnly) => { + await act(async () => root.render()); + expect(detailOnly ? mocks.context : mocks.snapshot).toHaveBeenCalledTimes(1); + expect(mocks.confirm).not.toHaveBeenCalled(); + expect(mocks.trust).not.toHaveBeenCalled(); + expect(container.textContent).toContain('panels/git:trust.required'); + expect(container.textContent).not.toContain('git_repository_untrusted:'); + }); + + it.each([true, false])('asks on Retry and replays only after approval (approved=%s)', async (approved) => { + mocks.confirm.mockResolvedValue(approved); + mocks.trust.mockResolvedValue({ state: 'trusted', repositoryPath: workspacePath }); + await act(async () => root.render()); + const retry = Array.from(container.querySelectorAll('button')).find(button => button.textContent === 'Retry'); + expect(retry).toBeTruthy(); + await act(async () => retry!.click()); + expect(mocks.confirm).toHaveBeenCalledTimes(1); + expect(mocks.trust).toHaveBeenCalledTimes(approved ? 1 : 0); + expect(mocks.snapshot).toHaveBeenCalledTimes(approved ? 3 : 2); + expect(container.textContent).toContain('panels/git:trust.required'); + }); +}); diff --git a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx index ee21a32f02..f38316b583 100644 --- a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx +++ b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.tsx @@ -37,6 +37,10 @@ import { findLatestCodeReviewResultState, summarizeCodeReviewResult } from '@/fl import { parsePullRequestUrl, remoteMatchesPullRequestLink } from '@/shared/utils/pullRequestLinks'; import { useContextStore } from '@/shared/stores/contextStore'; import { quickActions } from '@/shared/services/ide-control'; +import { + describeGitTrustFailure, + withGitRepositoryTrustRecovery, +} from '@/shared/services/gitTrustService'; import type { PullRequestContext } from '@/shared/types/context'; import { currentPullRequestReviewStatusText, @@ -136,6 +140,11 @@ const detailPageCache = new Map(); const reviewLaunchesInFlight = new Set(); const EMPTY_REVIEW_THREADS: ReviewPlatformThread[] = []; +function reviewPlatformErrorMessage(error: unknown, fallback: string): string { + return describeGitTrustFailure(error) + ?? (error instanceof Error ? error.message : fallback); +} + function detailPageInfo(pagination: ReviewPlatformPagination, itemCount: number): PageInfo { const pageIndex = Math.max(0, (pagination.page || 1) - 1); const perPage = Math.max(1, pagination.perPage || itemCount || 1); @@ -756,7 +765,10 @@ export const ReviewPlatformPanel: React.FC = ({ [account, snapshot.remotes], ); - const loadSnapshot = useCallback(async (nextRemoteId?: string | null, options?: { force?: boolean; page?: number }) => { + const loadSnapshot = useCallback(async ( + nextRemoteId?: string | null, + options?: { force?: boolean; page?: number; userInitiated?: boolean }, + ) => { const requestSeq = ++snapshotRequestSeq.current; if (!workspacePath) { setSnapshot(emptySnapshot()); @@ -808,9 +820,17 @@ export const ReviewPlatformPanel: React.FC = ({ setLoading(true); setError(null); try { - const next = detailOnly - ? await reviewPlatformAPI.getWorkspaceContext(workspacePath, requestedRemoteId ?? null) - : await reviewPlatformAPI.getWorkspaceSnapshot(workspacePath, requestedRemoteId ?? null, requestedPage, PR_PAGE_SIZE); + const fetchSnapshot = () => detailOnly + ? reviewPlatformAPI.getWorkspaceContext(workspacePath, requestedRemoteId ?? null) + : reviewPlatformAPI.getWorkspaceSnapshot( + workspacePath, + requestedRemoteId ?? null, + requestedPage, + PR_PAGE_SIZE, + ); + const next = options?.userInitiated + ? await withGitRepositoryTrustRecovery(fetchSnapshot, { userInitiated: true }) + : await fetchSnapshot(); if (snapshotRequestSeq.current !== requestSeq) return; setSnapshot(next); const remoteId = next.selectedRemoteId ?? next.remotes[0]?.id ?? null; @@ -829,7 +849,7 @@ export const ReviewPlatformPanel: React.FC = ({ setSnapshotCacheState('cached'); } catch (err) { if (snapshotRequestSeq.current !== requestSeq) return; - const message = err instanceof Error ? err.message : 'Failed to load pull requests'; + const message = reviewPlatformErrorMessage(err, 'Failed to load pull requests'); setError(message); if (!cached) { setSnapshot(emptySnapshot()); @@ -883,7 +903,7 @@ export const ReviewPlatformPanel: React.FC = ({ } catch (err) { if (detailRequestSeq.current !== requestSeq) return; log.error('Failed to load pull request detail', { pullRequestId, error: err }); - setDetailError(err instanceof Error ? err.message : 'Failed to load pull request details.'); + setDetailError(reviewPlatformErrorMessage(err, 'Failed to load pull request details.')); if (!cached) { setDetail(null); } @@ -961,7 +981,7 @@ export const ReviewPlatformPanel: React.FC = ({ } catch (err) { if (detailSectionRequestSeq.current !== requestSeq) return; log.error('Failed to load pull request detail section', { pullRequestId, section, page, perPage, error: err }); - setDetailError(err instanceof Error ? err.message : 'Failed to load pull request details.'); + setDetailError(reviewPlatformErrorMessage(err, 'Failed to load pull request details.')); } finally { if (detailSectionRequestSeq.current === requestSeq) { setDetailLoading(false); @@ -1372,7 +1392,7 @@ export const ReviewPlatformPanel: React.FC = ({ setCiLogById(prev => ({ ...prev, [item.id]: nextLog })); return nextLog; } catch (err) { - const message = err instanceof Error ? err.message : 'Failed to load CI error log.'; + const message = reviewPlatformErrorMessage(err, 'Failed to load CI error log.'); setCiLogErrorById(prev => ({ ...prev, [item.id]: message })); log.error('Failed to load CI log', { itemId: item.id, error: err }); return null; @@ -1910,7 +1930,7 @@ export const ReviewPlatformPanel: React.FC = ({ aria-label="Refresh" className="review-platform__icon-button" size="sm" - onClick={() => void loadSnapshot(listRemoteId, { force: true, page: currentPageIndex + 1 })} + onClick={() => void loadSnapshot(listRemoteId, { force: true, page: currentPageIndex + 1, userInitiated: true })} loading={loading} icon={} /> @@ -1980,7 +2000,7 @@ export const ReviewPlatformPanel: React.FC = ({
{error} -
@@ -2108,7 +2128,7 @@ export const ReviewPlatformPanel: React.FC = ({ diff --git a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.i18n.test.ts b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.i18n.test.ts index 33eee758fb..85bc550057 100644 --- a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.i18n.test.ts +++ b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.i18n.test.ts @@ -17,6 +17,8 @@ const REQUIRED_ACTION_BAR_KEYS = [ 'deepReviewActionBar.managedCoverageProgress', 'deepReviewActionBar.managedCoverageDeferred', 'deepReviewActionBar.fixAndReviewRunning', + 'deepReviewActionBar.actionStartFailed', + 'deepReviewActionBar.actionStartFailedWithReason', 'deepReviewActionBar.minimizedStandard', 'deepReviewActionBar.minimizedReviewRunningDeep', 'deepReviewActionBar.minimizedReviewRunningStandard', diff --git a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.scss b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.scss index 68ead8797b..5073ef2b26 100644 --- a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.scss +++ b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.scss @@ -126,8 +126,9 @@ /* Status header */ &__status { - display: flex; - align-items: center; + display: grid; + grid-template-columns: auto minmax(0, 1fr); + align-items: start; gap: 6px; padding-right: 136px; } @@ -154,19 +155,19 @@ } &__status-title { + min-width: 0; font-weight: var(--openbitfun-type-label-selected-font-weight); font-size: var(--openbitfun-type-flow-control-font-size); color: var(--openbitfun-color-content-primary); } &__error-message { - margin-left: auto; + grid-column: 2; + min-width: 0; font-size: var(--openbitfun-type-flow-control-font-size); color: var(--openbitfun-color-status-danger-content); - max-width: 50%; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; + overflow-wrap: anywhere; + white-space: pre-wrap; } /* Remediation section */ diff --git a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.test.tsx b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.test.tsx index a02c0a25db..31bfab13ce 100644 --- a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.test.tsx +++ b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.test.tsx @@ -292,6 +292,63 @@ describeWithJsdom('DeepReviewActionBar', () => { expect(container.querySelector('[role="status"]')).toBeTruthy(); }); + it('localizes the stable dialog-start prefix without translating provider details', async () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-session', + parentSessionId: 'parent-session', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix the provider failure.'], + }, + phase: 'fix_failed', + }); + store.updatePhase( + 'fix_failed', + 'Failed to start dialog turn: provider quota exhausted', + 'child-session', + ); + + await act(async () => { + root.render(); + }); + + expect(container.textContent).toContain( + 'Unable to start this action: provider quota exhausted', + ); + expect(container.textContent).not.toContain('Failed to start dialog turn:'); + }); + + it.each([ + [new Error('Failed to start dialog turn: provider quota exhausted'), 'Unable to start this action: provider quota exhausted'], + [Object.assign(new Error('Network connection was interrupted before Review could start.'), { + launchErrorMessageKey: 'deepReviewActionBar.launchError.network', + originalMessage: 'Failed to start dialog turn: provider connection closed', + }), 'Network connection interrupted. Review failed to start.\nprovider connection closed'], + ])('shows the same launch error in the header and notification: %s', async (error, message) => { + const { notificationService } = await import('@/shared/notification-system'); + sendMessageMock.mockRejectedValueOnce(error); + useReviewActionBarStore.getState().showActionBar({ + childSessionId: 'child-session', + parentSessionId: 'parent-session', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix the provider failure.'], + }, + phase: 'review_completed', + }); + await act(async () => root.render()); + const startFixButton = Array.from(container.querySelectorAll('button')) + .find((button) => button.textContent?.includes('Start fixing')); + expect(startFixButton).toBeTruthy(); + await act(async () => { + startFixButton!.dispatchEvent(new dom.window.MouseEvent('click', { bubbles: true })); + }); + expect(notificationService.error).toHaveBeenCalledWith(message, { duration: 5000 }); + expect(container.textContent).toContain(message); + expect(container.textContent).not.toContain('Failed to start dialog turn:'); + }); + it('keeps remediation in progress after submitting a fix turn', async () => { flowChatSessionsMock.set('child-session', { sessionId: 'child-session', diff --git a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBarLayout.test.ts b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBarLayout.test.ts index 4b3a1b3504..aaf2b94992 100644 --- a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBarLayout.test.ts +++ b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBarLayout.test.ts @@ -51,4 +51,20 @@ describe('DeepReviewActionBar layout styles', () => { expect(stylesheet).not.toContain('--deep-review-action-bar-scrollbar-gutter'); expect(actions).not.toContain('calc('); }); + + it('keeps complete review and fix error details readable', () => { + const stylesheet = readActionBarStylesheet(); + const status = extractBlock(stylesheet, '&__status'); + const errorMessage = extractBlock(stylesheet, '&__error-message'); + + expect(status).toContain('display: grid;'); + expect(status).toContain('grid-template-columns: auto minmax(0, 1fr);'); + expect(errorMessage).toContain('grid-column: 2;'); + expect(errorMessage).toContain('overflow-wrap: anywhere;'); + expect(errorMessage).toContain('white-space: pre-wrap;'); + expect(errorMessage).not.toContain('overflow: hidden;'); + expect(errorMessage).not.toContain('text-overflow: ellipsis;'); + expect(errorMessage).not.toContain('white-space: nowrap;'); + expect(errorMessage).not.toContain('max-width: 50%;'); + }); }); diff --git a/src/web-ui/src/flow_chat/deep-review/action-bar/DeepReviewActionBar.tsx b/src/web-ui/src/flow_chat/deep-review/action-bar/DeepReviewActionBar.tsx index bb387834f6..0903dedf8b 100644 --- a/src/web-ui/src/flow_chat/deep-review/action-bar/DeepReviewActionBar.tsx +++ b/src/web-ui/src/flow_chat/deep-review/action-bar/DeepReviewActionBar.tsx @@ -45,7 +45,10 @@ import { isTauriRuntime } from '@/infrastructure/runtime'; import { useSettingsStore } from '@/app/scenes/settings/settingsStore'; import { useSceneStore } from '@/app/stores/sceneStore'; import type { SettingsPageId } from '@/app/scenes/settings/settingsTypes'; -import { formatElapsedTime } from './actionBarFormatting'; +import { + getReviewActionErrorMessage, + formatElapsedTime, +} from './actionBarFormatting'; import { CapacityQueueNotice } from './CapacityQueueNotice'; import { DecisionExecutionGate } from './DecisionExecutionGate'; import { buildInterruptionDiagnostics } from './interruptionDiagnostics'; @@ -516,14 +519,11 @@ export const ReviewActionBar: React.FC = ({ childSessionId log.error('Failed to start review remediation', { childSessionId, reviewMode, error }); const msg = error instanceof Error ? error.message : String(error); const isTimeout = /timeout/i.test(msg); - store.updatePhase(isTimeout ? 'fix_timeout' : 'fix_failed', msg, childSessionId); + const message = getReviewActionErrorMessage(error, t, t('deepReviewActionBar.actionStartFailed')); + store.updatePhase(isTimeout ? 'fix_timeout' : 'fix_failed', message, childSessionId); store.restore(childSessionId ?? undefined); notificationService.error( - error instanceof Error - ? error.message - : t('toolCards.codeReview.reviewFailed', { - error: t('toolCards.codeReview.unknownError'), - }), + message, { duration: 5000 }, ); } finally { @@ -657,7 +657,7 @@ export const ReviewActionBar: React.FC = ({ childSessionId reviewMode, error, }); - const message = normalizeActionErrorMessage(error); + const message = getReviewActionErrorMessage(error, t, t('deepReviewActionBar.actionStartFailed')); notificationService.error(message, { duration: 5000 }); } finally { store.setActiveAction(null, undefined, childSessionId); @@ -721,9 +721,7 @@ export const ReviewActionBar: React.FC = ({ childSessionId store.minimize(childSessionId); } catch (error) { log.error('Failed to start DeepReview retry coverage', { childSessionId, error }); - const message = error instanceof Error - ? error.message - : t('deepReviewActionBar.retryIncompleteFailed'); + const message = getReviewActionErrorMessage(error, t, t('deepReviewActionBar.retryIncompleteFailed')); notificationService.error(message, { duration: 5000 }); } finally { store.setActiveAction(null, undefined, childSessionId); @@ -829,6 +827,12 @@ export const ReviewActionBar: React.FC = ({ childSessionId } }, []); + const displayErrorMessage = useMemo(() => { + if (!errorMessage) return null; + + return getReviewActionErrorMessage(errorMessage, t, t('deepReviewActionBar.actionStartFailed')); + }, [errorMessage, t]); + const handleCopyDiagnostics = useCallback(async () => { const detail = interruption?.errorDetail; if (!detail) return; @@ -924,7 +928,7 @@ export const ReviewActionBar: React.FC = ({ childSessionId PhaseIcon={PhaseIcon} phaseIconClass={phaseConfig.iconClass} phaseTitle={phaseTitle} - errorMessage={errorMessage} + errorMessage={displayErrorMessage} minimizeLabel={t('deepReviewActionBar.minimize')} onMinimize={handleMinimize} /> diff --git a/src/web-ui/src/flow_chat/deep-review/action-bar/actionBarFormatting.test.ts b/src/web-ui/src/flow_chat/deep-review/action-bar/actionBarFormatting.test.ts index 1780866cf8..4fb03818e0 100644 --- a/src/web-ui/src/flow_chat/deep-review/action-bar/actionBarFormatting.test.ts +++ b/src/web-ui/src/flow_chat/deep-review/action-bar/actionBarFormatting.test.ts @@ -1,11 +1,56 @@ import { describe, expect, it } from 'vitest'; -import { formatElapsedTime } from './actionBarFormatting'; +import { + classifyReviewActionErrorMessage, + formatElapsedTime, + getReviewActionErrorMessage, +} from './actionBarFormatting'; +import { createTestI18nT } from '@/test/i18nTestUtils'; describe('action bar formatting', () => { + const translate = createTestI18nT('flow-chat'); + + it('prefers structured launch copy and preserves the original backend reason', () => { + const error = Object.assign(new Error('Failed to start dialog turn: legacy wrapper'), { + launchErrorMessageKey: 'deepReviewActionBar.launchError.network', + originalMessage: 'Failed to start dialog turn: provider connection closed', + }); + expect(getReviewActionErrorMessage(error, translate, 'fallback')).toBe( + 'Network connection interrupted. Review failed to start.\nprovider connection closed', + ); + }); + + it('supports legacy stored strings, plain errors, and empty failure payloads', () => { + expect(getReviewActionErrorMessage('Failed to start dialog turn: quota exhausted', translate, 'fallback')) + .toBe('Unable to start this action: quota exhausted'); + expect(getReviewActionErrorMessage(new Error('provider detail'), translate, 'fallback')).toBe('provider detail'); + expect(getReviewActionErrorMessage(null, translate, 'fallback')).toBe('fallback'); + }); + it('formats elapsed milliseconds without changing existing labels', () => { expect(formatElapsedTime(999)).toBe('0s'); expect(formatElapsedTime(12_000)).toBe('12s'); expect(formatElapsedTime(60_000)).toBe('1m 0s'); expect(formatElapsedTime(125_000)).toBe('2m 5s'); }); + + it('classifies only the stable product-owned dialog-start prefix', () => { + expect(classifyReviewActionErrorMessage( + 'Failed to start dialog turn: provider quota exhausted', + )).toEqual({ + kind: 'start_dialog_turn_failed', + reason: 'provider quota exhausted', + }); + + expect(classifyReviewActionErrorMessage('Failed to start dialog turn:')).toEqual({ + kind: 'start_dialog_turn_failed', + reason: null, + }); + }); + + it('keeps arbitrary backend and provider details as raw display data', () => { + expect(classifyReviewActionErrorMessage('Provider request failed in region us-east')).toEqual({ + kind: 'raw', + message: 'Provider request failed in region us-east', + }); + }); }); diff --git a/src/web-ui/src/flow_chat/deep-review/action-bar/actionBarFormatting.ts b/src/web-ui/src/flow_chat/deep-review/action-bar/actionBarFormatting.ts index 7cc4cef63c..490e0746e5 100644 --- a/src/web-ui/src/flow_chat/deep-review/action-bar/actionBarFormatting.ts +++ b/src/web-ui/src/flow_chat/deep-review/action-bar/actionBarFormatting.ts @@ -1,3 +1,28 @@ +import { getDeepReviewLaunchErrorMessage, type DeepReviewLaunchError } from '../launch/launchErrors'; + +const START_DIALOG_TURN_ERROR_PREFIX = 'Failed to start dialog turn:'; + +export type ReviewActionErrorPresentation = + | { kind: 'start_dialog_turn_failed'; reason: string | null } + | { kind: 'raw'; message: string }; + +export function classifyReviewActionErrorMessage(message: string): ReviewActionErrorPresentation { + const normalizedMessage = message.trim(); + + if (normalizedMessage.startsWith(START_DIALOG_TURN_ERROR_PREFIX)) { + const reason = normalizedMessage.slice(START_DIALOG_TURN_ERROR_PREFIX.length).trim(); + return { + kind: 'start_dialog_turn_failed', + reason: reason || null, + }; + } + + return { + kind: 'raw', + message: normalizedMessage, + }; +} + export function formatElapsedTime(ms: number): string { const seconds = Math.floor(ms / 1000); const minutes = Math.floor(seconds / 60); @@ -7,3 +32,29 @@ export function formatElapsedTime(ms: number): string { } return `${minutes}m ${remainingSeconds}s`; } + +export function getReviewActionErrorMessage( + error: unknown, + translate: (key: string, options?: { defaultValue?: string; reason?: string }) => string, + fallback: string, +): string { + const launchError = error as DeepReviewLaunchError | null | undefined; + if (launchError?.launchErrorMessageKey) { + const message = getDeepReviewLaunchErrorMessage(error, translate, fallback); + const original = launchError.originalMessage?.trim(); + const presentation = original ? classifyReviewActionErrorMessage(original) : null; + const reason = presentation?.kind === 'start_dialog_turn_failed' + ? presentation.reason + : original; + return reason && reason !== message ? `${message}\n${reason}` : message; + } + + const message = error instanceof Error ? error.message : typeof error === 'string' ? error : ''; + if (!message.trim()) return fallback; + + const presentation = classifyReviewActionErrorMessage(message); + if (presentation.kind === 'raw') return presentation.message; + return presentation.reason + ? translate('deepReviewActionBar.actionStartFailedWithReason', { reason: presentation.reason }) + : translate('deepReviewActionBar.actionStartFailed'); +} diff --git a/src/web-ui/src/flow_chat/deep-review/launch/targetResolver.test.ts b/src/web-ui/src/flow_chat/deep-review/launch/targetResolver.test.ts index addf12e4fb..7f4c9e7be4 100644 --- a/src/web-ui/src/flow_chat/deep-review/launch/targetResolver.test.ts +++ b/src/web-ui/src/flow_chat/deep-review/launch/targetResolver.test.ts @@ -14,6 +14,7 @@ const mockGitGetDiff = vi.fn(); const mockGitResolveRevision = vi.fn(); const mockWorkspaceReadFile = vi.fn(); const mockWorkspaceGetFileMetadata = vi.fn(); +const mockSystemCheckPathExists = vi.fn(); vi.mock('@/infrastructure/api', () => ({ gitAPI: { @@ -26,6 +27,9 @@ vi.mock('@/infrastructure/api', () => ({ readFileContent: (...args: any[]) => mockWorkspaceReadFile(...args), getFileMetadata: (...args: any[]) => mockWorkspaceGetFileMetadata(...args), }, + systemAPI: { + checkPathExists: (...args: any[]) => mockSystemCheckPathExists(...args), + }, })); describe('Deep Review target resolver', () => { @@ -53,6 +57,7 @@ describe('Deep Review target resolver', () => { isFile: true, size: 1024, }); + mockSystemCheckPathExists.mockResolvedValue(true); }); it('counts changed lines from unified diff without headers', () => { @@ -109,6 +114,90 @@ describe('Deep Review target resolver', () => { }); }); + it('normalizes an existing changed absolute POSIX target to the workspace', async () => { + mockGitGetStatus.mockResolvedValueOnce({ + staged: [{ path: 'tests/existing.ts', status: 'modified' }], + unstaged: [], + untracked: [], + conflicts: [], + current_branch: 'main', + ahead: 0, + behind: 0, + }); + mockGitGetChangedFiles.mockResolvedValueOnce([ + { path: 'tests/existing.ts', status: 'modified' }, + ]); + mockGitGetDiff.mockResolvedValueOnce('+changed\n'); + + const result = await resolveSlashCommandReviewTarget( + '/storage/Users/currentUser/files/git_code/OpenBitFun/tests/existing.ts', + '/storage/Users/currentUser/files/git_code/OpenBitFun', + ); + + expect(result.target.files.map((file) => file.normalizedPath)).toEqual([ + 'tests/existing.ts', + ]); + expect(result.targetEvidence).toMatchObject({ + source: 'workspace', + completeness: 'complete', + workspaceBinding: 'matching_dirty', + }); + expect(mockSystemCheckPathExists).not.toHaveBeenCalled(); + }); + + it.each([ + { path: 'src/old.ts', status: 'deleted', old_path: undefined }, + { path: 'src/new.ts', status: 'renamed', old_path: 'src/old.ts' }, + ])('keeps an explicit old path reviewable from Git evidence: $status', async (change) => { + mockSystemCheckPathExists.mockResolvedValue(false); + mockGitGetChangedFiles.mockResolvedValue([change]); + mockGitGetDiff.mockResolvedValue('-old line\n'); + const result = await resolveSlashCommandReviewTarget('src/old.ts', '/workspace'); + expect(mockSystemCheckPathExists).not.toHaveBeenCalled(); + expect(result.targetEvidence.limitations).not.toContain('explicit_target_path_not_found'); + expect(result.targetEvidence.files).toEqual([ + expect.objectContaining({ path: change.path, status: change.status }), + ]); + }); + + it('reports a missing absolute POSIX target', async () => { + mockSystemCheckPathExists.mockResolvedValueOnce(false); + + const result = await resolveSlashCommandReviewTarget( + '/storage/Users/currentUser/files/git_code/OpenBitFun/tests/missing.ts', + '/storage/Users/currentUser/files/git_code/OpenBitFun', + ); + + expect(mockSystemCheckPathExists).toHaveBeenCalledWith( + '/storage/Users/currentUser/files/git_code/OpenBitFun/tests/missing.ts', + ); + expect(result.target.files.map((file) => file.normalizedPath)).toEqual([ + 'tests/missing.ts', + ]); + expect(result.targetEvidence).toMatchObject({ + completeness: 'unknown', + limitations: ['explicit_target_path_not_found'], + }); + }); + + it('keeps the existing unchanged-path limitation distinct from a missing path', async () => { + const result = await resolveSlashCommandReviewTarget( + '/storage/Users/currentUser/files/git_code/OpenBitFun/tests/unchanged.ts', + '/storage/Users/currentUser/files/git_code/OpenBitFun', + ); + + expect(mockSystemCheckPathExists).toHaveBeenCalledWith( + '/storage/Users/currentUser/files/git_code/OpenBitFun/tests/unchanged.ts', + ); + expect(result.target.files.map((file) => file.normalizedPath)).toEqual([ + 'tests/unchanged.ts', + ]); + expect(result.targetEvidence).toMatchObject({ + completeness: 'unknown', + limitations: ['explicit_file_scope_has_no_workspace_changes'], + }); + }); + it('expands an explicit directory without widening outside it', async () => { mockGitGetStatus.mockResolvedValueOnce({ staged: [{ path: 'src/inside.ts', status: 'modified' }], @@ -302,6 +391,30 @@ describe('Deep Review target resolver', () => { }); }); + it.each(['exists', 'missing', 'offline'])('rejects an explicit remote target without probing when the host is %s', async (hostState) => { + mockSystemCheckPathExists.mockImplementation(async () => { + if (hostState === 'offline') throw new Error('remote host disconnected'); + return hostState === 'exists'; + }); + const result = await resolveSlashCommandReviewTarget( + '/remote/workspace/src/existing.ts', + '/remote/workspace', + 'remote-1', + ); + + expect(mockSystemCheckPathExists).not.toHaveBeenCalled(); + expect(mockWorkspaceReadFile).not.toHaveBeenCalled(); + expect(mockWorkspaceGetFileMetadata).not.toHaveBeenCalled(); + expect(mockGitGetStatus).not.toHaveBeenCalled(); + expect(mockGitGetChangedFiles).not.toHaveBeenCalled(); + expect(mockGitGetDiff).not.toHaveBeenCalled(); + expect(result.targetEvidence).toMatchObject({ + source: 'workspace', + completeness: 'unknown', + limitations: ['remote_workspace_review_unavailable'], + }); + }); + it('preserves both paths and rename semantics for an edited workspace rename', async () => { mockGitGetStatus.mockResolvedValueOnce({ staged: [{ path: 'src/new-name.ts', status: 'added' }], diff --git a/src/web-ui/src/flow_chat/deep-review/launch/targetResolver.ts b/src/web-ui/src/flow_chat/deep-review/launch/targetResolver.ts index a5c2a9e72a..7de04a4bf7 100644 --- a/src/web-ui/src/flow_chat/deep-review/launch/targetResolver.ts +++ b/src/web-ui/src/flow_chat/deep-review/launch/targetResolver.ts @@ -1,4 +1,4 @@ -import { gitAPI, workspaceAPI } from '@/infrastructure/api'; +import { gitAPI, systemAPI, workspaceAPI } from '@/infrastructure/api'; import { isGitRepositoryUntrustedError } from '@/infrastructure/api/errors/TauriCommandError'; import type { GitChangedFile, @@ -287,6 +287,32 @@ function workspaceFilePath(workspacePath: string, filePath: string): string { return `${rootPath.replace(/\/+$/, '')}/${normalizePath(filePath, workspacePath)}`; } +async function findMissingWorkspacePaths( + workspacePath: string, + paths: string[], +): Promise { + const results = await Promise.all([...new Set(paths)].map(async (path) => ({ + path, + exists: await systemAPI.checkPathExists( + workspaceFilePath(workspacePath, path), + ), + }))); + return results.flatMap(({ path, exists }) => exists ? [] : [path]); +} + +function missingExplicitTarget( + target: ReviewTargetClassification, +): ResolvedDeepReviewTarget { + return { + target, + changeStats: buildUnknownChangeStats(target), + targetEvidence: buildUnknownReviewTargetEvidence( + target, + 'explicit_target_path_not_found', + ), + }; +} + function countTextFileLines(content: string): number { if (!content) { return 0; @@ -604,15 +630,8 @@ export async function resolveSlashCommandReviewTarget( }; } - if (remoteConnectionId) { - return resolveCurrentFileReviewSnapshot( - workspacePath, - target, - remoteConnectionId, - ); - } - - if (!bindTargetToWorkspace(target, workspacePath)) { + const workspaceTarget = bindTargetToWorkspace(target, workspacePath); + if (!workspaceTarget) { return { target, changeStats: buildUnknownChangeStats(target), @@ -624,6 +643,14 @@ export async function resolveSlashCommandReviewTarget( } try { + if (remoteConnectionId) { + return resolveCurrentFileReviewSnapshot( + workspacePath, + workspaceTarget, + remoteConnectionId, + ); + } + const [status, changedFiles] = await Promise.all([ gitAPI.getStatus(workspacePath, 'review_explicit_scope_snapshot'), gitAPI.getChangedFiles(workspacePath, { @@ -641,18 +668,22 @@ export async function resolveSlashCommandReviewTarget( for (const path of collectWorkspaceDiffFilePaths(status)) { candidatePaths.add(normalizePath(path, workspacePath).replace(/\/+$/, '')); } - const requested = explicitFilePaths.map((path) => { - const normalized = normalizePath(path, workspacePath); - const normalizedPath = normalized.replace(/\/+$/, ''); - const exactFileExists = candidatePaths.has(normalizedPath); - const containsChangedFiles = [...candidatePaths].some((candidate) => ( - candidate.startsWith(`${normalizedPath}/`) - )); - return { - path: normalizedPath, - directory: /[\\/]$/.test(path) || (!exactFileExists && containsChangedFiles), - }; - }); + const requested = workspaceTarget.files + .filter((file) => !file.excluded) + .map((file) => { + const normalized = normalizePath(file.normalizedPath, workspacePath); + const normalizedPath = normalized.replace(/\/+$/, ''); + const exactFileExists = candidatePaths.has(normalizedPath); + const containsChangedFiles = [...candidatePaths].some((candidate) => ( + candidate.startsWith(`${normalizedPath}/`) + )); + return { + path: normalizedPath, + directory: + /[\\/]$/.test(file.normalizedPath) || + (!exactFileExists && containsChangedFiles), + }; + }); const matchesRequestedPath = (path: string): boolean => { const normalized = normalizePath(path, workspacePath).replace(/\/+$/, ''); return requested.some((entry) => ( @@ -681,12 +712,29 @@ export async function resolveSlashCommandReviewTarget( represented.add(normalized); } } + const requestedHasScopedChange = (entry: (typeof requested)[number]): boolean => + scopedChanges.some((change) => [change.path, change.oldPath] + .filter((path): path is string => Boolean(path)) + .map((path) => normalizePath(path, workspacePath).replace(/\/+$/, '')) + .some((path) => ( + path === entry.path || + (entry.directory && path.startsWith(`${entry.path}/`)) + ))); + const missingPaths = await findMissingWorkspacePaths( + workspacePath, + requested + .filter((entry) => !requestedHasScopedChange(entry)) + .map((entry) => entry.path), + ); + if (missingPaths.length > 0) { + return missingExplicitTarget(workspaceTarget); + } if (scopedChanges.length === 0) { return { - target, - changeStats: buildUnknownChangeStats(target), + target: workspaceTarget, + changeStats: buildUnknownChangeStats(workspaceTarget), targetEvidence: buildUnknownReviewTargetEvidence( - target, + workspaceTarget, 'explicit_file_scope_has_no_workspace_changes', ), }; diff --git a/src/web-ui/src/flow_chat/services/ReviewService.test.ts b/src/web-ui/src/flow_chat/services/ReviewService.test.ts index f31212105a..1c3ec2c3d8 100644 --- a/src/web-ui/src/flow_chat/services/ReviewService.test.ts +++ b/src/web-ui/src/flow_chat/services/ReviewService.test.ts @@ -556,6 +556,39 @@ describe('ReviewService', () => { )).rejects.toThrow('Remote workspace Review is not supported'); }); + it('reports a missing explicit file before spending reviewer capacity', async () => { + const manifest = runManifest('normal'); + mocks.resolveSlashCommandReviewTarget.mockResolvedValue({ + target: { + ...manifest.target, + source: 'slash_command_explicit_files', + files: [{ + ...manifest.target.files[0], + path: 'tests/missing.ts', + normalizedPath: 'tests/missing.ts', + source: 'slash_command_explicit_files', + status: 'unknown', + }], + }, + changeStats: { fileCount: 1, lineCountSource: 'unknown' }, + targetEvidence: { + ...targetEvidence(), + completeness: 'unknown', + workspaceBinding: 'unavailable', + files: [], + limitations: ['explicit_target_path_not_found'], + }, + }); + + await expect(prepareReviewLaunchFromSlashCommand( + '/review tests/missing.ts', + '/workspace/project', + )).rejects.toMatchObject({ + message: 'The requested file or directory does not exist in the current workspace.', + launchErrorMessageKey: 'deepReviewActionBar.launchError.missingExplicitScope', + }); + }); + it('blocks an empty confirmed workspace snapshot before spending reviewer capacity', async () => { const manifest = runManifest('normal'); mocks.resolveSlashCommandReviewTarget.mockResolvedValue({ diff --git a/src/web-ui/src/flow_chat/services/ReviewService.ts b/src/web-ui/src/flow_chat/services/ReviewService.ts index 955fae91a3..281d6b2431 100644 --- a/src/web-ui/src/flow_chat/services/ReviewService.ts +++ b/src/web-ui/src/flow_chat/services/ReviewService.ts @@ -287,6 +287,12 @@ async function prepareFromResolvedTarget(params: { 'deepReviewActionBar.launchError.unresolvedTarget', ); } + if (params.targetEvidence.limitations.includes('explicit_target_path_not_found')) { + throw reviewTargetError( + 'The requested file or directory does not exist in the current workspace.', + 'deepReviewActionBar.launchError.missingExplicitScope', + ); + } if (params.targetEvidence.limitations.includes('explicit_file_scope_has_no_workspace_changes')) { throw reviewTargetError( 'The requested files or directories contain no workspace changes.', diff --git a/src/web-ui/src/infrastructure/api/service-api/SystemAPI.test.ts b/src/web-ui/src/infrastructure/api/service-api/SystemAPI.test.ts index 3b4e4aa513..71f14a8fae 100644 --- a/src/web-ui/src/infrastructure/api/service-api/SystemAPI.test.ts +++ b/src/web-ui/src/infrastructure/api/service-api/SystemAPI.test.ts @@ -111,4 +111,17 @@ describe('SystemAPI', () => { await expect(systemAPI.setClipboard('device-code')).rejects.toThrow('Clipboard write failed'); expect(invokeMock).not.toHaveBeenCalled(); }); + + it('checks path existence on the active host', async () => { + invokeMock.mockResolvedValueOnce(true); + + await expect(systemAPI.checkPathExists( + '/workspace/src/existing.ts', + )).resolves.toBe(true); + expect(invokeMock).toHaveBeenCalledWith('check_path_exists', { + request: { + path: '/workspace/src/existing.ts', + }, + }); + }); }); diff --git a/src/web-ui/src/infrastructure/api/service-api/SystemAPI.ts b/src/web-ui/src/infrastructure/api/service-api/SystemAPI.ts index 62f68e3658..fa144e62d2 100644 --- a/src/web-ui/src/infrastructure/api/service-api/SystemAPI.ts +++ b/src/web-ui/src/infrastructure/api/service-api/SystemAPI.ts @@ -156,8 +156,8 @@ export class SystemAPI { async checkPathExists(path: string): Promise { try { - return await api.invoke('check_path_exists', { - request: { path } + return await api.invoke('check_path_exists', { + request: { path } }); } catch (error) { throw createTauriCommandError('check_path_exists', error, { path }); diff --git a/src/web-ui/src/locales/en-US/flow-chat.json b/src/web-ui/src/locales/en-US/flow-chat.json index e83db724eb..79f6315640 100644 --- a/src/web-ui/src/locales/en-US/flow-chat.json +++ b/src/web-ui/src/locales/en-US/flow-chat.json @@ -1093,6 +1093,8 @@ "fixRunning": "Fixing in progress...", "fixCompleted": "Fix completed", "fixFailed": "Fix failed", + "actionStartFailed": "Unable to start this action.", + "actionStartFailedWithReason": "Unable to start this action: {{reason}}", "fixTimeout": "Fix timed out", "fixInterrupted": "Fix was interrupted. Up to {{count}} selected items may still need attention.", "reviewWaitingCapacity": "Review waiting for capacity", @@ -1264,6 +1266,7 @@ "unresolvedGitRange": "The requested Git range could not be resolved. Check the ref or range and try again.", "emptyWorkspace": "There are no workspace changes to review.", "emptyExplicitScope": "The requested files or directories contain no workspace changes.", + "missingExplicitScope": "The requested file or directory does not exist in the current workspace.", "unresolvedTarget": "The Review target could not be prepared as bounded evidence. Open its workspace or narrow the target and try again.", "repositoryUntrusted": "Git will not read this repository because the folder is owned by another user. Trust the folder when prompted, then start Review again.", "uncertain": "The Review request may already be running. Its session was preserved so you can inspect or retry it safely.", diff --git a/src/web-ui/src/locales/zh-CN/flow-chat.json b/src/web-ui/src/locales/zh-CN/flow-chat.json index 4fe5668a48..914bbb6451 100644 --- a/src/web-ui/src/locales/zh-CN/flow-chat.json +++ b/src/web-ui/src/locales/zh-CN/flow-chat.json @@ -1093,6 +1093,8 @@ "fixRunning": "正在修复...", "fixCompleted": "修复完成", "fixFailed": "修复失败", + "actionStartFailed": "无法开始此次操作。", + "actionStartFailedWithReason": "无法开始此次操作：{{reason}}", "fixTimeout": "修复超时", "fixInterrupted": "修复已中断，最多有 {{count}} 个已选项目可能仍需处理。", "reviewWaitingCapacity": "审核正在等待容量", @@ -1264,6 +1266,7 @@ "unresolvedGitRange": "无法解析请求的 Git 范围，请检查引用或范围后重试", "emptyWorkspace": "当前工作区没有可审核的变更", "emptyExplicitScope": "请求的文件或目录没有工作区变更", + "missingExplicitScope": "请求的文件或目录在当前工作区中不存在", "unresolvedTarget": "无法将审核目标准备为有界证据，请打开对应工作区或缩小目标后重试", "repositoryUntrusted": "该目录属于其他用户，Git 拒绝读取此仓库。请在提示中信任该目录后重新启动审核", "uncertain": "审核请求可能已经在运行。会话已保留，可安全检查或重试", diff --git a/src/web-ui/src/locales/zh-TW/flow-chat.json b/src/web-ui/src/locales/zh-TW/flow-chat.json index fbe1a762c7..5671544648 100644 --- a/src/web-ui/src/locales/zh-TW/flow-chat.json +++ b/src/web-ui/src/locales/zh-TW/flow-chat.json @@ -1093,6 +1093,8 @@ "fixRunning": "正在修復...", "fixCompleted": "修復完成", "fixFailed": "修復失敗", + "actionStartFailed": "無法開始此次操作。", + "actionStartFailedWithReason": "無法開始此次操作：{{reason}}", "fixTimeout": "修復超時", "fixInterrupted": "修復已中斷，最多有 {{count}} 個已選項目可能仍需處理。", "reviewWaitingCapacity": "審核正在等待容量", @@ -1264,6 +1266,7 @@ "unresolvedGitRange": "無法解析要求的 Git 範圍，請檢查引用或範圍後重試", "emptyWorkspace": "目前工作區沒有可審核的變更", "emptyExplicitScope": "要求的檔案或目錄沒有工作區變更", + "missingExplicitScope": "要求的檔案或目錄不存在於目前工作區中", "unresolvedTarget": "無法將審核目標準備為有界證據，請開啟對應工作區或縮小目標後重試", "repositoryUntrusted": "該目錄屬於其他使用者，Git 拒絕讀取此儲存庫。請在提示中信任該目錄後重新啟動審核", "uncertain": "審核要求可能已在執行。工作階段已保留，可安全檢查或重試", diff --git a/src/web-ui/src/shared/services/gitTrustService.test.ts b/src/web-ui/src/shared/services/gitTrustService.test.ts index 40c85e02cb..ecda6aee2c 100644 --- a/src/web-ui/src/shared/services/gitTrustService.test.ts +++ b/src/web-ui/src/shared/services/gitTrustService.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { TauriCommandError } from '@/infrastructure/api/errors/TauriCommandError'; import { + describeGitTrustFailure, requestGitRepositoryTrust, resetGitTrustDecisions, withGitRepositoryTrustRecovery, @@ -67,6 +68,18 @@ beforeEach(() => { successMock.mockReset(); }); +describe('describeGitTrustFailure', () => { + it('turns the stable repository trust code into localized copy', () => { + expect(describeGitTrustFailure(untrustedError())).toBe( + `panels/git:trust.required|${JSON.stringify({ path: REPOSITORY_PATH })}`, + ); + }); + + it('leaves unrelated failures to the calling surface', () => { + expect(describeGitTrustFailure(new Error('provider unavailable'))).toBeUndefined(); + }); +}); + describe('requestGitRepositoryTrust', () => { it('grants trust only after the user confirms', async () => { confirmWarningMock.mockResolvedValue(true); diff --git a/src/web-ui/src/shared/services/gitTrustService.ts b/src/web-ui/src/shared/services/gitTrustService.ts index 3ddb777c06..4f70b93ae9 100644 --- a/src/web-ui/src/shared/services/gitTrustService.ts +++ b/src/web-ui/src/shared/services/gitTrustService.ts @@ -49,6 +49,14 @@ const promptQuietUntil = new Map(); */ const promptKey = repositoryPathKey; +/** Names an ownership rejection with localized, actionable copy. */ +export function describeGitTrustFailure(failure: unknown): string | undefined { + const repositoryPath = gitRepositoryUntrustedPath(failure); + return repositoryPath + ? i18nService.t('panels/git:trust.required', { path: repositoryPath }) + : undefined; +} + /** Test seam: forgets in-flight prompts and remembered decisions. */ export function resetGitTrustDecisions(): void { inFlightRequests.clear(); diff --git a/src/web-ui/src/tools/git/services/GitService.ts b/src/web-ui/src/tools/git/services/GitService.ts index cc8464a8dc..37c8d49231 100644 --- a/src/web-ui/src/tools/git/services/GitService.ts +++ b/src/web-ui/src/tools/git/services/GitService.ts @@ -3,15 +3,15 @@ */ import { gitAPI } from '@/infrastructure/api'; -import { - gitRepositoryUntrustedPath, - isGitRepositoryUntrustedError, -} from '@/infrastructure/api/errors/TauriCommandError'; +import { isGitRepositoryUntrustedError } from '@/infrastructure/api/errors/TauriCommandError'; import { createLogger } from '@/shared/utils/logger'; import { measureAsync } from '@/shared/utils/timing'; import { i18nService } from '@/infrastructure/i18n'; +import { describeGitTrustFailure } from '@/shared/services/gitTrustService'; import { gitStateManager } from '../state/GitStateManager'; +export { describeGitTrustFailure } from '@/shared/services/gitTrustService'; + const log = createLogger('GitService'); export type { GitRepository, @@ -44,24 +44,6 @@ import { GitLogParams } from '../types'; -/** - * Names the ownership wall behind a failed Git operation, or `undefined` when - * that is not what went wrong. - * - * The rejection reaches a caller as a stable code from two directions: a local - * executor throws it, and a remote one returns it in `result.error`. Neither is - * a sentence. Passing it through puts `git_repository_untrusted: - * /srv/shared/repo` in the panel next to a commit that did not happen, while - * the very same wall on a status read names the repository and points at the - * way out. - */ -export function describeGitTrustFailure(failure: unknown): string | undefined { - const repositoryPath = gitRepositoryUntrustedPath(failure); - return repositoryPath - ? i18nService.t('panels/git:trust.required', { path: repositoryPath }) - : undefined; -} - export class GitService { private static instance: GitService;