diff --git a/VERSION b/VERSION index 3df5a469..07a21997 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.0.29 +2.0.30 diff --git a/backend/app/api/browser.py b/backend/app/api/browser.py index e67bdd92..a8325848 100644 --- a/backend/app/api/browser.py +++ b/backend/app/api/browser.py @@ -45,6 +45,7 @@ ContentTransferResult, CopyMoveRequest, CreateItemRequest, + DirectoryIssueDetail, DirectoryListing, DirectorySearchResult, FileInfo, @@ -1075,6 +1076,7 @@ async def search_directories( connection_id: uuid.UUID, q: str = Query("", max_length=MAX_QUERY_BYTES, description="Directory search text; shared UTF-8 byte and term limits apply"), include_dot_directories: bool = Query(False, description="Whether to include directories whose path contains dot-prefixed segments"), + include_issue_details: bool = Query(False, description="Include bounded directory issue details for the status dialog"), current_user: User = Depends(get_current_user_with_auth_check), session: Session = Depends(get_session), ) -> DirectorySearchResult: @@ -1122,6 +1124,9 @@ async def search_directories( except asyncio.CancelledError: cancelled.set() raise + details, incomplete = ( + cache.directory_issue_details() if response.issue_count and response.coverage == "partial" else ([], False) + ) return DirectorySearchResult( results=[row.path for row in response.results], match_ranks={row.path: row.match_rank for row in response.results}, @@ -1133,6 +1138,9 @@ async def search_directories( accepted_revision=response.accepted_revision, coverage=response.coverage, coverage_issue_count=response.issue_count, + issue_details_available=bool(details), + issue_details=[DirectoryIssueDetail.model_validate(detail) for detail in details] if include_issue_details else None, + issue_details_incomplete=incomplete, last_verified_at_ms=response.last_verified_at_ms, status_message=cache.scan_error, monitoring_state=cache.monitoring_state, diff --git a/backend/app/models/file.py b/backend/app/models/file.py index da043d2c..33c180f5 100644 --- a/backend/app/models/file.py +++ b/backend/app/models/file.py @@ -144,6 +144,13 @@ def validate_factual_outcome(self) -> "ContentTransferResult": return self +class DirectoryIssueDetail(BaseModel): + """A directory that could not be fully read during verification.""" + + path: str + code: Literal["access_denied", "unsupported_encoding", "unsafe_traversal", "scan_failed"] + + class DirectorySearchResult(BaseModel): """Response model for directory search (quick navigate).""" @@ -157,6 +164,9 @@ class DirectorySearchResult(BaseModel): accepted_revision: int | None = Field(default=None, ge=0) coverage: str | None = None coverage_issue_count: int = Field(default=0, ge=0) + issue_details_available: bool = False + issue_details: list[DirectoryIssueDetail] | None = None + issue_details_incomplete: bool = False last_verified_at_ms: int | None = Field(default=None, ge=0) status_message: str | None = None monitoring_state: Literal["starting", "healthy", "recovering", "unavailable"] | None = None diff --git a/backend/app/services/directory_cache.py b/backend/app/services/directory_cache.py index 4e8be6bf..19aeb926 100644 --- a/backend/app/services/directory_cache.py +++ b/backend/app/services/directory_cache.py @@ -86,6 +86,7 @@ class CacheState(str, Enum): """State of a connection's directory cache.""" EMPTY = "empty" # No scan started yet + OPENING = "opening" BUILDING = "building" # Initial scan in progress READY = "ready" # Scan complete, cache is usable UPDATING = "updating" # Rescan in progress, stale data served @@ -254,9 +255,26 @@ def __init__( self._streamed_parent_reads: set[int] = set() self._completed_scan_reads: set[int] = set() self._pending_scan_issues: dict[int, dict[str, int]] = {} + self.issue_details: set[tuple[str, str]] = set() + self.issue_details_incomplete = False self.scan_commits = 0 self.on_scan_commit: Callable[[int], None] | None = None + def _publish_issue_details(self, parent: str, counts: dict[str, int]) -> None: + """Publish diagnostics only after the owning parent transaction succeeds.""" + self.issue_details = {detail for detail in self.issue_details if detail[0] != parent} + for code in counts: + if len(self.issue_details) < MAX_LOGGED_SCAN_ISSUES_PER_PASS: + self.issue_details.add((parent, code)) + else: + self.issue_details_incomplete = True + + def _discard_missing_issue_details(self, generation_id: int) -> None: + """Remove samples for subtrees pruned by committed parent snapshots.""" + self.issue_details = { + detail for detail in self.issue_details if not detail[0] or self.index.contains_directory(generation_id, detail[0]) + } + def start_scan_groups(self, *, reconcile: bool = False) -> None: with self.lock: self._scan_group = {} @@ -341,6 +359,9 @@ def flush_scan_group(self) -> None: if file_ids[generation]: options["file_ids"] = file_ids[generation] self.index.commit_scan_parents(generation, results, **options) + for parent, _children in results: + self._publish_issue_details(parent, issues.get(parent, {})) + self._discard_missing_issue_details(generation) self.scan_commits += 1 if self.on_scan_commit is not None: for generation in changed: @@ -579,6 +600,10 @@ def apply_pending(self) -> int: self.index.mutate_observed_path("add", mutation.path) elif mutation.action == FileAction.FILE_ACTION_REMOVED: self.index.mutate_observed_path("remove", mutation.path) + prefix = mutation.path + "/" + self.issue_details = { + detail for detail in self.issue_details if detail[0] != mutation.path and not detail[0].startswith(prefix) + } elif mutation.action == FileAction.FILE_ACTION_RENAMED_NEW_NAME: self.index.mutate_observed_path("rename", mutation.path, mutation.new_path) else: @@ -656,6 +681,9 @@ def record_issue(self, token: ScanReadToken, generation_id: int, code: str, *, r self.flush_scan_group_if_due() else: self.index.record_issue(generation_id, code, reconcile=reconcile) + counts = {previous_code: 1 for parent, previous_code in self.issue_details if parent == token.path} + counts[code] = 1 + self._publish_issue_details(token.path, counts) def _record_scan_issue(self, token: ScanReadToken, generation_id: int, record: Callable[[int], None]) -> None: with self.lock: @@ -689,6 +717,8 @@ def finish_parent(self, token: ScanReadToken, generation_id: int, *, reconcile: if token.producer_id in self._pending_scan_issues else None, ) + self._publish_issue_details(token.path, self._pending_scan_issues.get(token.producer_id, {})) + self._discard_missing_issue_details(generation_id) elif self._scan_group is not None: existing = self._scan_group.get(token.producer_id) snapshot_children = existing.children if existing is not None else [] @@ -1543,7 +1573,7 @@ async def start_scan(self) -> None: if self._scan_task is not None and not self._scan_task.done(): return self._owning_loop = asyncio.get_running_loop() - self._state = CacheState.BUILDING + self._state = CacheState.OPENING self._scan_task = asyncio.create_task(self._initialize()) async def _initialize(self) -> None: @@ -1706,6 +1736,12 @@ def _classify_source_error(error: Exception) -> str | None: return "unsupported_encoding" return "scan_failed" + def directory_issue_details(self) -> tuple[list[dict[str, str]], bool]: + """Return bounded, source-local diagnostics, never a persistent error history.""" + with self.coordinator.lock: + details = [{"path": parent or "/", "code": code} for parent, code in sorted(self.coordinator.issue_details)] + return details, self.coordinator.issue_details_incomplete + def _record_item_issue(self, token: ScanReadToken, generation: int, code: str, *, reconcile: bool, path: str | None = None) -> None: self.coordinator.record_issue(token, generation, code, reconcile=reconcile) self._scan_issue_counts[code] = self._scan_issue_counts.get(code, 0) + 1 @@ -2293,6 +2329,8 @@ def _scan_disk_queue(self) -> None: resuming = self.index.has_resumable_scan() generation, _selected = self.index.prepare_scan() if not resuming: + self.coordinator.issue_details.clear() + self.coordinator.issue_details_incomplete = False self._scan_epoch = (generation, baseline) if self.coordinator.observation_lost: while self.index.requeue_completed_scan_batch(generation): diff --git a/backend/app/services/directory_index.py b/backend/app/services/directory_index.py index 93398e91..f38d7eb6 100644 --- a/backend/app/services/directory_index.py +++ b/backend/app/services/directory_index.py @@ -954,6 +954,17 @@ def scan_parent_file_id(self, generation_id: int, parent: str) -> bytes | None: ) return None if row is None or row[0] is None else bytes(row[0]) + def contains_directory(self, generation_id: int, path: str) -> bool: + """Check whether a sampled directory remains in the committed generation.""" + validate_directory_path(path) + with self._writer_lock: + return ( + self._connection() + .execute("SELECT 1 FROM directories WHERE generation_id = ? AND path = ?", (generation_id, path)) + .fetchone() + is not None + ) + def write_scan_children( self, generation_id: int, children: Iterable[tuple[str, bool]], *, file_ids: dict[str, bytes] | None = None ) -> None: diff --git a/backend/tests/test_directory_cache.py b/backend/tests/test_directory_cache.py index 126a1dd1..906805bc 100644 --- a/backend/tests/test_directory_cache.py +++ b/backend/tests/test_directory_cache.py @@ -350,6 +350,7 @@ def test_enum_values(self): """Verify all expected cache states exist with correct values.""" assert CacheState.EMPTY.value == "empty" + assert CacheState.OPENING.value == "opening" assert CacheState.BUILDING.value == "building" assert CacheState.READY.value == "ready" assert CacheState.UPDATING.value == "updating" @@ -363,7 +364,14 @@ def test_enum_is_string(self): def test_enum_member_count(self): """Ensure no unexpected states were added.""" - assert len(CacheState) == 4 + expected_states = ( + CacheState.EMPTY, + CacheState.OPENING, + CacheState.BUILDING, + CacheState.READY, + CacheState.UPDATING, + ) + assert len(CacheState) == len(expected_states) # ============================================================================ diff --git a/backend/tests/test_directory_index_contract.py b/backend/tests/test_directory_index_contract.py index c244fbf3..121ee9ee 100644 --- a/backend/tests/test_directory_index_contract.py +++ b/backend/tests/test_directory_index_contract.py @@ -2205,6 +2205,31 @@ def unavailable(): assert "Wait for active workers" in sqlite_cache.scan_error assert started == [True] assert not sqlite_cache._opened.is_set() + assert sqlite_cache.state.value == "empty" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("next_state", ["building", "updating", "ready"]) +async def test_cold_scan_reports_opening_until_initialization_finishes(sqlite_cache, monkeypatch, next_state): + from app.services.directory_cache import CacheState + + entered = asyncio.Event() + release = asyncio.Event() + + async def initialize(): + entered.set() + await release.wait() + sqlite_cache._state = CacheState(next_state) + + monkeypatch.setattr(sqlite_cache, "_initialize", initialize) + await sqlite_cache.start_scan() + try: + await entered.wait() + assert sqlite_cache.state is CacheState.OPENING + finally: + release.set() + await sqlite_cache._scan_task + assert sqlite_cache.state.value == next_state @pytest.mark.asyncio @@ -5253,6 +5278,32 @@ async def get_source(**kwargs): assert response.json()["coverage"] == "partial" +@pytest.mark.parametrize("include_details", [False, True]) +def test_directory_api_returns_issue_details_only_on_request( + sqlite_cache, monkeypatch, client, auth_headers_user, test_connection, include_details +): + async def get_source(**kwargs): + return sqlite_cache + + sqlite_cache.index.seed_scan(1) + sqlite_cache.coordinator.start_scan_groups() + token = sqlite_cache.coordinator.begin_read("") + sqlite_cache.coordinator.record_issue(token, 1, "unsupported_encoding") + sqlite_cache.coordinator.finish_parent(token, 1) + sqlite_cache.coordinator.flush_scan_group() + assert sqlite_cache.index.finish_generation(1) + monkeypatch.setattr("app.services.directory_cache.get_directory_cache_manager", lambda: SimpleNamespace(get_or_create_cache=get_source)) + response = client.get( + f"/api/browse/{test_connection.id}/directories", + headers=auth_headers_user, + params={"q": "", "include_issue_details": str(include_details).lower()}, + ) + assert response.status_code == 200 + assert response.json()["issue_details_available"] is True + assert response.json()["issue_details"] == ([{"path": "/", "code": "unsupported_encoding"}] if include_details else None) + assert response.json()["issue_details_incomplete"] is False + + def test_directory_api_rejects_excess_terms_before_source_creation(monkeypatch, client, auth_headers_user, test_connection): def unexpected_manager(): raise AssertionError("Invalid queries must not initialize a directory source.") @@ -5452,6 +5503,14 @@ def scan(path, **kwargs): assert sqlite_cache.index.pending_scan_paths(1) == [] assert sqlite_cache._smb_base_path + "\\readable\\child" in reads assert sqlite_cache.index._connection().execute("SELECT code, count FROM coverage_issues").fetchall() == [("access_denied", 1)] + assert sqlite_cache.directory_issue_details() == ([{"path": "denied", "code": "access_denied"}], False) + + def empty_scan(*_args, **_kwargs): + yield from () + + monkeypatch.setattr("smbclient.scandir", empty_scan) + sqlite_cache._scan_disk_queue() + assert sqlite_cache.directory_issue_details() == ([], False) @pytest.mark.parametrize("failure", ["separator", "surrogate", "oversized", "folder_io", "denied"]) @@ -7376,12 +7435,14 @@ def test_scan_group_buffers_parent_issues(index, early_flush, reconcile): coordinator.flush_scan_group() coordinator.record_issue(token, 1, "unsupported_encoding", reconcile=reconcile) assert index.status().issue_count == 0 + assert not coordinator.issue_details coordinator.finish_parent(token, 1, reconcile=reconcile) coordinator.flush_scan_group() assert index.status().issue_count == 2 assert coordinator.scan_commits == 1 assert not coordinator._pending_scan_issues assert not coordinator._producers + assert coordinator.issue_details == {("", "unsupported_encoding")} @pytest.mark.parametrize("reconcile", [False, True]) @@ -7405,6 +7466,86 @@ def test_stale_scan_group_discards_parent_issues(index, reconcile): assert index.scan_parent_pending(1, "") assert index._connection().execute("SELECT count(*) FROM coverage_issues WHERE code = 'unsupported_encoding'").fetchone() == (0,) assert not coordinator._pending_scan_issues + assert not coordinator.issue_details + + +@pytest.mark.parametrize("parent_hint", [False, True]) +def test_issue_details_remove_deleted_subtree_after_reconciliation(index, parent_hint): + from smbprotocol.change_notify import FileAction + + from app.services.directory_cache import READ_PARENT_ACTION, DirectoryScanCoordinator, _DirectoryMutation + + index.seed_scan(1) + coordinator = DirectoryScanCoordinator(index) + coordinator.start_scan_groups() + for parent, children in [ + ("", [("private", True), ("private-backup", True)]), + ("private", [("private/child", True)]), + ("private/child", []), + ("private-backup", []), + ]: + with coordinator.read(parent) as token: + coordinator.commit_children(token, 1, children) + coordinator.record_issue(token, 1, "access_denied") + coordinator.finish_parent(token, 1) + coordinator.flush_scan_group() + coordinator.cancel_scan_groups() + assert index.finish_generation(1) + + mutation = _DirectoryMutation(READ_PARENT_ACTION, "") if parent_hint else _DirectoryMutation(FileAction.FILE_ACTION_REMOVED, "private") + assert coordinator.enqueue(mutation) + coordinator.apply_pending() + coordinator.start_scan_groups(reconcile=True) + with coordinator.read("") as token: + coordinator.commit_children(token, 1, [("private-backup", True)], reconcile=True) + coordinator.record_issue(token, 1, "access_denied", reconcile=True) + coordinator.finish_parent(token, 1, reconcile=True) + coordinator.flush_scan_group() + coordinator.cancel_scan_groups() + + assert not index.search("private/child").results + assert coordinator.issue_details == {("", "access_denied"), ("private-backup", "access_denied")} + + +def test_issue_details_preserved_when_subtree_deletion_rolls_back(index): + from smbprotocol.change_notify import FileAction + + from app.services.directory_cache import DirectoryScanCoordinator, _DirectoryMutation + + index.write_paths(["private", "private/child"]) + assert index.finish_generation(1) + coordinator = DirectoryScanCoordinator(index) + coordinator._publish_issue_details("private", {"access_denied": 1}) + coordinator._publish_issue_details("private/child", {"access_denied": 1}) + samples = coordinator.issue_details.copy() + index._connection().execute( + "CREATE TRIGGER reject_sample_deletion BEFORE DELETE ON directories " + "WHEN OLD.path = 'private' BEGIN SELECT RAISE(ABORT, 'injected deletion failure'); END" + ) + assert coordinator.enqueue(_DirectoryMutation(FileAction.FILE_ACTION_REMOVED, "private")) + with pytest.raises(sqlite3.IntegrityError, match="injected deletion failure"): + coordinator.apply_pending() + assert coordinator.issue_details == samples + assert index.contains_directory(1, "private/child") + assert coordinator.pending + index._connection().execute("DROP TRIGGER reject_sample_deletion") + coordinator.apply_pending() + assert not coordinator.issue_details + + +def test_issue_detail_samples_are_bounded_and_replaced_by_clean_reads(index): + from app.services.directory_cache import MAX_LOGGED_SCAN_ISSUES_PER_PASS, DirectoryScanCoordinator + + coordinator = DirectoryScanCoordinator(index) + for number in range(MAX_LOGGED_SCAN_ISSUES_PER_PASS + 1): + coordinator._publish_issue_details(f"parent-{number}", {"access_denied": 1}) + assert len(coordinator.issue_details) == MAX_LOGGED_SCAN_ISSUES_PER_PASS + assert coordinator.issue_details_incomplete + coordinator._publish_issue_details("parent-0", {}) + assert not any(parent == "parent-0" for parent, _code in coordinator.issue_details) + coordinator._publish_issue_details("parent-1", {"scan_failed": 1}) + assert ("parent-1", "scan_failed") in coordinator.issue_details + assert ("parent-1", "access_denied") not in coordinator.issue_details def test_reconciliation_groups_independent_pages(smb_source_cache, smb_directory_server, monkeypatch): diff --git a/backend/tests/test_directory_search.py b/backend/tests/test_directory_search.py index fa62b0ea..1289439e 100644 --- a/backend/tests/test_directory_search.py +++ b/backend/tests/test_directory_search.py @@ -666,7 +666,7 @@ def test_model_serialization(self): def test_model_all_cache_states(self): """Model should accept all valid cache states.""" - for state_value in ["empty", "building", "ready", "updating"]: + for state_value in ["empty", "opening", "building", "ready", "updating"]: result = DirectorySearchResult( results=[], total_matches=0, diff --git a/companion/package-lock.json b/companion/package-lock.json index 77fb2b82..3203fadf 100644 --- a/companion/package-lock.json +++ b/companion/package-lock.json @@ -1,12 +1,12 @@ { "name": "sambee-companion", - "version": "2.0.29", + "version": "2.0.30", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "sambee-companion", - "version": "2.0.29", + "version": "2.0.30", "dependencies": { "@tauri-apps/api": "^2.12.4", "@tauri-apps/plugin-autostart": "^2.7.0", diff --git a/companion/package.json b/companion/package.json index 27a62492..b88b7ba4 100644 --- a/companion/package.json +++ b/companion/package.json @@ -1,6 +1,6 @@ { "name": "sambee-companion", - "version": "2.0.29", + "version": "2.0.30", "private": true, "type": "module", "scripts": { diff --git a/companion/src-tauri/Cargo.lock b/companion/src-tauri/Cargo.lock index 6125ac3b..b9d9f83c 100644 --- a/companion/src-tauri/Cargo.lock +++ b/companion/src-tauri/Cargo.lock @@ -3867,7 +3867,7 @@ checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" [[package]] name = "sambee-companion" -version = "2.0.29" +version = "2.0.30" dependencies = [ "axum", "base64 0.22.1", diff --git a/companion/src-tauri/Cargo.toml b/companion/src-tauri/Cargo.toml index 73665dc1..71635ceb 100644 --- a/companion/src-tauri/Cargo.toml +++ b/companion/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sambee-companion" -version = "2.0.29" +version = "2.0.30" edition = "2021" description = "Sambee Companion — opens files from Sambee in native desktop editors" authors = [] diff --git a/companion/src-tauri/src/server/directory_index.rs b/companion/src-tauri/src/server/directory_index.rs index f5b2b09f..37839283 100644 --- a/companion/src-tauri/src/server/directory_index.rs +++ b/companion/src-tauri/src/server/directory_index.rs @@ -1277,6 +1277,15 @@ impl DirectoryIndex { .flatten()) } + pub fn contains_directory(&self, generation_id: i64, path: &str) -> Result { + validate_directory_path(path)?; + Ok(self.writer.query_row( + "SELECT EXISTS(SELECT 1 FROM directories WHERE generation_id = ? AND path = ?)", + params![generation_id, path], + |row| row.get(0), + )?) + } + pub fn commit_scan_parents(&mut self, generation_id: i64, results: &[ScanParentResult]) -> Result<(), IndexError> { self.commit_scan_parents_with_ids(generation_id, results, &HashMap::new()) } diff --git a/companion/src-tauri/src/server/directory_manager.rs b/companion/src-tauri/src/server/directory_manager.rs index c09d14d1..a5db187f 100644 --- a/companion/src-tauri/src/server/directory_manager.rs +++ b/companion/src-tauri/src/server/directory_manager.rs @@ -29,7 +29,7 @@ use super::directory_index::{ use super::directory_traversal::NativeIdentity; use super::directory_traversal::{source_error, NativeIo, NativeParent, NativeRoot, TraversalIssue, MOUNT_BOUNDARY_CAPABILITY_ERROR}; use super::drives::{volume_evidence, VolumeChange, VolumeEvidence}; -use super::models::{DirectorySearchResult, ScanProgress}; +use super::models::{DirectoryIssueDetail, DirectorySearchResult, ScanProgress}; const CONTRACT_JSON: &str = include_str!("../../../../shared/directory-index/contract.json"); const STALLED_READ_TIMEOUT: Duration = Duration::from_secs(30); @@ -524,6 +524,8 @@ struct Observations { streamed_parent_reads: HashSet, scan_commits: usize, scan_issue_counts: HashMap<&'static str, usize>, + issue_details: HashSet<(String, &'static str)>, + issue_details_incomplete: bool, pending_scan_issues: HashMap>, } @@ -598,6 +600,30 @@ impl InvalidationReason { } impl Observations { + fn publish_issue_details(&mut self, parent: &str, counts: &HashMap<&'static str, usize>) -> Result<(), IndexError> { + self.issue_details.retain(|(path, _code)| path != parent); + let maximum = limit("max_logged_scan_issues_per_pass")?; + for code in counts.keys() { + if self.issue_details.len() < maximum { + self.issue_details.insert((parent.to_owned(), code)); + } else { + self.issue_details_incomplete = true; + } + } + Ok(()) + } + + fn discard_missing_issue_details(&mut self, index: &DirectoryIndex, generation: i64) -> Result<(), IndexError> { + let mut missing = HashSet::new(); + for (path, _code) in &self.issue_details { + if !path.is_empty() && !index.contains_directory(generation, path)? { + missing.insert(path.clone()); + } + } + self.issue_details.retain(|(path, _code)| !missing.contains(path)); + Ok(()) + } + fn scan_issue_bytes(&self, producer: u64) -> usize { self.pending_scan_issues.get(&producer).map_or(0, |counts| { counts.iter().map(|(code, count)| code.len() + count.to_string().len()).sum() @@ -871,6 +897,8 @@ impl Drop for MutationObservation { } } +const INDEX_OPENING_STATE: &str = "opening"; + struct SourceStatus { state: &'static str, message: Option, @@ -1047,7 +1075,7 @@ impl LocalSource { watcher_reconciling: AtomicBool::new(true), observations: Mutex::new(Observations::default()), status: Mutex::new(SourceStatus { - state: "building", + state: INDEX_OPENING_STATE, message: None, progress: now, verified: now, @@ -1462,7 +1490,15 @@ impl LocalSource { break; }; writer.mutate_observed_path(mutation)?; - observations.bytes -= *bytes; + let bytes = *bytes; + if let DirectoryMutation::Remove { path } = mutation { + let prefix = format!("{path}/"); + let path = path.clone(); + observations + .issue_details + .retain(|(parent, _code)| parent != &path && !parent.starts_with(&prefix)); + } + observations.bytes -= bytes; observations.pending.pop_front(); } observations.retire_barriers(); @@ -1771,6 +1807,16 @@ impl LocalSource { issues: issues.get(&generation), }, )?; + for (parent, _children) in &results { + observations.publish_issue_details( + parent, + issues + .get(&generation) + .and_then(|parents| parents.get(parent)) + .unwrap_or(&HashMap::new()), + )?; + } + observations.discard_missing_issue_details(writer, generation)?; observations.scan_commits += 1; } let mut status = lock(&self.status)?; @@ -2029,6 +2075,13 @@ impl LocalSource { } else { writer.finish_scan_parent(generation, parent)?; } + let counts = observations + .pending_scan_issues + .get(&read.token.producer) + .cloned() + .unwrap_or_default(); + observations.publish_issue_details(parent, &counts)?; + observations.discard_missing_issue_details(writer, generation)?; lock(&self.status)?.index_status = Some(writer.status()?); Ok(()) } @@ -2099,6 +2152,14 @@ impl LocalSource { let writer = writer.as_mut().ok_or(IndexError::Invalid("Directory writer is unavailable."))?; writer.record_issue(generation, code)?; lock(&self.status)?.index_status = Some(writer.status()?); + let mut counts: HashMap<_, _> = observations + .issue_details + .iter() + .filter(|(path, _code)| path == parent) + .map(|(_path, code)| (*code, 1)) + .collect(); + counts.insert(code, 1); + observations.publish_issue_details(parent, &counts)?; } *observations.scan_issue_counts.entry(code).or_default() += 1; if observations.scan_issue_counts.values().sum::() <= limit("max_logged_scan_issues_per_pass")? { @@ -2337,6 +2398,13 @@ impl LocalSource { } else { writer.finish_scan_parent(generation, parent)?; } + let counts = observations + .pending_scan_issues + .get(&read.token.producer) + .cloned() + .unwrap_or_default(); + observations.publish_issue_details(parent, &counts)?; + observations.discard_missing_issue_details(writer, generation)?; lock(&self.status)?.index_status = Some(writer.status()?); Ok(()) } @@ -2945,6 +3013,8 @@ impl LocalSource { let resuming = index.has_resumable_scan()?; let (generation, selected) = index.prepare_scan()?; if !resuming { + observations.issue_details.clear(); + observations.issue_details_incomplete = false; observations.scan_epoch = Some((generation, baseline.token.epoch)); } if observations.observation_lost { @@ -2964,14 +3034,17 @@ impl LocalSource { observations.scan_commits = 0; observations.scan_issue_counts.clear(); let (done, queued, found) = index.scan_counts(generation)?; - lock(&self.status)?.scan_progress = Some(ScanProgress { + let mut status = lock(&self.status)?; + status.state = if selected { "building" } else { "updating" }; + status.scan_progress = Some(ScanProgress { phase: if selected { "building" } else { "updating" }, parents_done: done, parents_queued: queued, directories_found: found, started_at_ms: epoch_milliseconds()?, }); - lock(&self.status)?.scan_generation = Some(generation); + status.scan_generation = Some(generation); + drop(status); log::info!( "event=directory_scan_started operation=directory_index stage=scan source_id={} generation_id={} scan_generation={} scan_phase={} parents_done={} parents_queued={} directories_found={} native_opens={} group_commits=0 duration_ms={} abort_reason=none", self.source_id, @@ -3315,7 +3388,11 @@ impl LocalDirectoryManager { }; { let mut status = lock(&source.status)?; - status.state = if lock(&source.reader)?.is_some() { "updating" } else { "building" }; + status.state = if lock(&source.reader)?.is_some() { + "updating" + } else { + INDEX_OPENING_STATE + }; status.stalled = false; } source.progress()?; @@ -3485,6 +3562,23 @@ impl LocalDirectoryManager { Ok(()) } + pub fn issue_details(&self, drive: &str) -> Result<(Vec, bool), IndexError> { + let sources = lock(&self.sources)?; + let source = sources.get(drive).ok_or(IndexError::Interrupted)?; + source.ensure_available()?; + let observations = lock(&source.observations)?; + let mut details: Vec<_> = observations + .issue_details + .iter() + .map(|(parent, code)| DirectoryIssueDetail { + path: if parent.is_empty() { "/".to_owned() } else { parent.clone() }, + code, + }) + .collect(); + details.sort_by(|left, right| (&left.path, left.code).cmp(&(&right.path, right.code))); + Ok((details, observations.issue_details_incomplete)) + } + pub async fn search( self: &Arc, drive: String, @@ -3565,6 +3659,9 @@ impl LocalDirectoryManager { accepted_revision: Some(0), coverage: Some("partial".into()), coverage_issue_count: 0, + issue_details_available: false, + issue_details: None, + issue_details_incomplete: false, last_verified_at_ms: None, status_message: status.message.clone(), monitoring_state: Some( @@ -3622,6 +3719,10 @@ impl LocalDirectoryManager { result? }; source.ensure_available()?; + let observations = lock(&source.observations)?; + let has_details = result.coverage == "partial" && result.issue_count > 0 && !observations.issue_details.is_empty(); + let details_incomplete = observations.issue_details_incomplete; + drop(observations); let status = lock(&source.status)?; let wire_count = |value: i64| -> Result { if !(0..=MAX_WIRE_INTEGER).contains(&value) { @@ -3648,6 +3749,9 @@ impl LocalDirectoryManager { accepted_revision: Some(result.accepted_revision), coverage: Some(result.coverage), coverage_issue_count: wire_count(result.issue_count)?, + issue_details_available: has_details, + issue_details: None, + issue_details_incomplete: details_incomplete, last_verified_at_ms: result.last_verified_at_ms, status_message: status.message.clone(), monitoring_state: Some( @@ -3685,6 +3789,110 @@ impl Drop for LocalDirectoryManager { #[cfg(test)] mod tests { + #[test] + fn issue_details_remove_deleted_subtree_after_reconciliation() { + for parent_hint in [false, true] { + issue_details_remove_deleted_subtree(parent_hint); + } + } + + fn issue_details_remove_deleted_subtree(parent_hint: bool) { + let temporary = tempfile::tempdir().unwrap(); + let path = temporary.path().join("root"); + std::fs::create_dir_all(path.join("private/child")).unwrap(); + std::fs::create_dir(path.join("private-backup")).unwrap(); + let manager = LocalDirectoryManager::new(temporary.path().join("storage")).unwrap(); + let source = LocalSource::new(path.clone(), manager.storage_directory()); + source.scan(&manager.budget).unwrap(); + { + let mut observations = lock(&source.observations).unwrap(); + for parent in ["", "private", "private/child", "private-backup"] { + observations + .publish_issue_details(parent, &HashMap::from([("access_denied", 1)])) + .unwrap(); + } + observations + .enqueue( + if parent_hint { + DirectoryMutation::ReadParent { path: String::new() } + } else { + DirectoryMutation::Remove { path: "private".into() } + }, + limit("max_pending_observations").unwrap(), + limit("max_observation_bytes").unwrap(), + ) + .unwrap(); + } + std::fs::remove_dir_all(path.join("private")).unwrap(); + source.flush(&manager.budget).unwrap(); + source.reconcile(&manager.budget, &manager.scans).unwrap(); + assert_eq!( + lock(&source.observations).unwrap().issue_details, + HashSet::from([("private-backup".to_owned(), "access_denied")]) + ); + } + + #[test] + fn issue_details_preserved_when_subtree_deletion_rolls_back() { + let temporary = tempfile::tempdir().unwrap(); + let path = temporary.path().join("root"); + std::fs::create_dir_all(path.join("private/child")).unwrap(); + let manager = LocalDirectoryManager::new(temporary.path().join("storage")).unwrap(); + let source = LocalSource::new(path, manager.storage_directory()); + source.scan(&manager.budget).unwrap(); + let samples = HashSet::from([ + ("private".to_owned(), "access_denied"), + ("private/child".to_owned(), "access_denied"), + ]); + { + let mut observations = lock(&source.observations).unwrap(); + observations.issue_details = samples.clone(); + observations + .enqueue( + DirectoryMutation::Remove { path: "private".into() }, + limit("max_pending_observations").unwrap(), + limit("max_observation_bytes").unwrap(), + ) + .unwrap(); + } + let connection = open_directory_database(&source.index_path, OpenFlags::SQLITE_OPEN_READ_WRITE).unwrap(); + connection.execute_batch( + "CREATE TRIGGER reject_sample_deletion BEFORE DELETE ON directories WHEN OLD.path = 'private' BEGIN SELECT RAISE(ABORT, 'injected deletion failure'); END", + ).unwrap(); + assert!(matches!(source.flush(&manager.budget), Err(IndexError::Sql(_)))); + assert_eq!(lock(&source.observations).unwrap().issue_details, samples); + assert!(!lock(&source.observations).unwrap().pending.is_empty()); + assert!(lock(&source.writer) + .unwrap() + .as_ref() + .unwrap() + .contains_directory(1, "private/child") + .unwrap()); + connection.execute_batch("DROP TRIGGER reject_sample_deletion").unwrap(); + source.flush(&manager.budget).unwrap(); + assert!(lock(&source.observations).unwrap().issue_details.is_empty()); + } + + #[test] + fn issue_detail_samples_are_bounded_and_replaced_by_clean_reads() { + let mut observations = Observations::default(); + let maximum = limit("max_logged_scan_issues_per_pass").unwrap(); + for number in 0..=maximum { + observations + .publish_issue_details(&format!("parent-{number}"), &HashMap::from([("access_denied", 1)])) + .unwrap(); + } + assert_eq!(observations.issue_details.len(), maximum); + assert!(observations.issue_details_incomplete); + observations.publish_issue_details("parent-0", &HashMap::new()).unwrap(); + assert!(!observations.issue_details.iter().any(|(parent, _code)| parent == "parent-0")); + observations + .publish_issue_details("parent-1", &HashMap::from([("scan_failed", 1)])) + .unwrap(); + assert!(observations.issue_details.contains(&("parent-1".to_owned(), "scan_failed"))); + assert!(!observations.issue_details.contains(&("parent-1".to_owned(), "access_denied"))); + } + #[test] fn scan_group_buffers_parent_issues() { for (early_flush, stale, reconcile) in [ @@ -3728,6 +3936,7 @@ mod tests { .record_item_issue(1, "", read.token, "unsupported_encoding", &manager.budget) .unwrap(); assert_eq!(lock(&source.writer).unwrap().as_ref().unwrap().status().unwrap().issue_count, 0); + assert!(lock(&source.observations).unwrap().issue_details.is_empty()); let mut observations = lock(&source.observations).unwrap(); source .complete_scan_parent_locked(&mut observations, ScanGeneration { id: 1, read: read.token }, "", &manager.budget) @@ -3744,6 +3953,7 @@ mod tests { assert_eq!(observations.scan_commits, usize::from(!stale)); assert!(observations.pending_scan_issues.is_empty()); assert!(observations.producers.is_empty()); + assert_eq!(observations.issue_details.len(), usize::from(!stale)); assert_eq!( lock(&source.writer).unwrap().as_ref().unwrap().status().unwrap().issue_count, if stale { 0 } else { 2 } @@ -4289,6 +4499,25 @@ mod tests { } } + #[cfg(target_os = "linux")] + #[test] + fn cold_source_reports_opening_before_scan_progress() { + let temporary = tempfile::tempdir().unwrap(); + let path = temporary.path().join("root"); + std::fs::create_dir(&path).unwrap(); + let manager = LocalDirectoryManager::new(temporary.path().join("private")).unwrap(); + let source = LocalSource::new(path, manager.storage_directory()); + { + let status = lock(&source.status).unwrap(); + assert_eq!(status.state, INDEX_OPENING_STATE); + assert!(status.scan_progress.is_none()); + } + source.scan(&manager.budget).unwrap(); + let status = lock(&source.status).unwrap(); + assert_eq!(status.state, "ready"); + assert_eq!(status.scan_progress.as_ref().unwrap().phase, "idle"); + } + #[cfg(target_os = "linux")] #[test] fn full_pass_root_probes_have_a_fixed_open_budget() { diff --git a/companion/src-tauri/src/server/handlers.rs b/companion/src-tauri/src/server/handlers.rs index 34a7cc72..5f7d8859 100644 --- a/companion/src-tauri/src/server/handlers.rs +++ b/companion/src-tauri/src/server/handlers.rs @@ -5652,6 +5652,7 @@ pub struct DirectorySearchQuery { pub q: Option, /// Whether to include directories with dot-prefixed path segments. pub include_dot_directories: Option, + pub include_issue_details: Option, } /// `GET /api/browse/{drive}/directories` — search for directories on a local drive. @@ -5667,11 +5668,11 @@ pub async fn browse_search_directories( let q = query.q.unwrap_or_default(); let include_dot_directories = query.include_dot_directories.unwrap_or(false); super::directory_index::search_terms(&q).map_err(|error| ApiError::BadRequest(error.to_string()))?; - state + let details_drive = drive.clone(); + let mut response = state .0 .search(drive, base_path, q, include_dot_directories) .await - .map(Json) .map_err(|error| match error { super::directory_index::IndexError::QueryBusy => ApiError::TooManyRequests(error.to_string()), super::directory_index::IndexError::Invalid(message) => ApiError::BadRequest(message.to_owned()), @@ -5679,7 +5680,20 @@ pub async fn browse_search_directories( ApiError::TooManyRequests("Directory query was cancelled or timed out. Retry the search.".to_owned()) } _ => ApiError::Internal("Directory search failed. Check drive access and private local storage, then retry.".to_owned()), - }) + })?; + if query.include_issue_details.unwrap_or(false) { + let (details, incomplete) = if response.issue_details_available { + state + .0 + .issue_details(&details_drive) + .map_err(|error| ApiError::Internal(error.to_string()))? + } else { + (Vec::new(), false) + }; + response.issue_details = Some(details); + response.issue_details_incomplete = incomplete; + } + Ok(Json(response)) } // ─── Upload ────────────────────────────────────────────────────────────────── diff --git a/companion/src-tauri/src/server/models.rs b/companion/src-tauri/src/server/models.rs index 20fcf9ec..77161ad4 100644 --- a/companion/src-tauri/src/server/models.rs +++ b/companion/src-tauri/src/server/models.rs @@ -638,6 +638,12 @@ pub struct ScanProgress { pub started_at_ms: i64, } +#[derive(Debug, Serialize)] +pub struct DirectoryIssueDetail { + pub path: String, + pub code: &'static str, +} + #[derive(Debug, Serialize)] pub struct DirectorySearchResult { pub results: Vec, @@ -650,6 +656,9 @@ pub struct DirectorySearchResult { pub accepted_revision: Option, pub coverage: Option, pub coverage_issue_count: usize, + pub issue_details_available: bool, + pub issue_details: Option>, + pub issue_details_incomplete: bool, pub last_verified_at_ms: Option, pub status_message: Option, pub monitoring_state: Option, diff --git a/companion/src-tauri/tauri.conf.json b/companion/src-tauri/tauri.conf.json index ee622e00..ef45c607 100644 --- a/companion/src-tauri/tauri.conf.json +++ b/companion/src-tauri/tauri.conf.json @@ -2,7 +2,7 @@ "$schema": "https://raw.githubusercontent.com/nicegui/tauri-v2-schema/refs/heads/main/tauri.conf.json.schema", "productName": "Sambee Companion", "identifier": "app.sambee.companion", - "version": "2.0.29", + "version": "2.0.30", "build": { "beforeDevCommand": "npm run dev", "devUrl": "http://localhost:1420", diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 6cd30b9b..f4be94d6 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1,12 +1,12 @@ { "name": "sambee-frontend", - "version": "2.0.29", + "version": "2.0.30", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "sambee-frontend", - "version": "2.0.29", + "version": "2.0.30", "hasInstallScript": true, "dependencies": { "@codemirror/autocomplete": "^6.20.4", diff --git a/frontend/package.json b/frontend/package.json index 5b6bbdeb..05a41af9 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "sambee-frontend", - "version": "2.0.29", + "version": "2.0.30", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/components/FileBrowser/SearchStatusDialog.tsx b/frontend/src/components/FileBrowser/SearchStatusDialog.tsx new file mode 100644 index 00000000..d95d45a1 --- /dev/null +++ b/frontend/src/components/FileBrowser/SearchStatusDialog.tsx @@ -0,0 +1,75 @@ +import { Box, Button, CircularProgress, Typography } from "@mui/material"; +import { useEffect, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { ResponsiveDialogShell } from "../Dialog/ResponsiveDialogShell"; +import type { SearchStatusDetails, SearchStatusInfo } from "./search/types"; + +export function SearchStatusDialog({ status, onClose }: { status: SearchStatusInfo; onClose: () => void }) { + const { t } = useTranslation(); + const [details, setDetails] = useState(null); + const [failed, setFailed] = useState(false); + + useEffect(() => { + const controller = new AbortController(); + setDetails(null); + setFailed(false); + status + .loadDetails?.(controller.signal) + .then((value) => { + if (!controller.signal.aborted) setDetails(value); + }) + .catch(() => { + if (!controller.signal.aborted) setFailed(true); + }); + return () => controller.abort(); + }, [status]); + + const hasDetails = !!details?.message || !!details?.issues?.length; + return ( + + {t("common.actions.close")} + + } + > + {!details && !failed && } + {failed && {t("fileBrowser.search.statusDetails.failed")}} + {details && !hasDetails && {t("fileBrowser.search.statusDetails.unavailable")}} + {details && hasDetails && ( + <> + {t("fileBrowser.search.statusDetails.source")} + {details.source} + {details.message && {details.message}} + {!!details.issues?.length && ( + <> + + {t("fileBrowser.search.statusDetails.impact")} + + {details.incomplete && ( + + {t("fileBrowser.search.statusDetails.incomplete")} + + )} + + {details.issues.map((issue) => ( + + + {issue.path} + + + {t(`fileBrowser.search.statusDetails.reasons.${issue.code}`)} + + + ))} + + + )} + + )} + + ); +} diff --git a/frontend/src/components/FileBrowser/UnifiedSearchBar.tsx b/frontend/src/components/FileBrowser/UnifiedSearchBar.tsx index 194e7b34..a83225d1 100644 --- a/frontend/src/components/FileBrowser/UnifiedSearchBar.tsx +++ b/frontend/src/components/FileBrowser/UnifiedSearchBar.tsx @@ -41,7 +41,7 @@ import { } from "@mui/material"; import { useVirtualizer } from "@tanstack/react-virtual"; import type React from "react"; -import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useEffectEvent, useLayoutEffect, useMemo, useRef, useState } from "react"; import { flushSync } from "react-dom"; import { useTranslation } from "react-i18next"; import { usePillButtonMenu } from "../../hooks/usePillButtonMenu"; @@ -54,7 +54,8 @@ import { QuickBarResultGroupHeader, QuickBarResultRow, } from "./QuickBarResultRow"; -import type { SearchProvider, SearchResult, SearchSelectionAction, SearchSelectionBehavior } from "./search/types"; +import { SearchStatusDialog } from "./SearchStatusDialog"; +import type { SearchProvider, SearchResult, SearchSelectionAction, SearchSelectionBehavior, SearchStatusInfo } from "./search/types"; // ============================================================================ // Constants @@ -196,6 +197,7 @@ export function UnifiedSearchBar({ const [removeError, setRemoveError] = useState(false); const [isFocused, setIsFocused] = useState(false); const [isSearchPending, setIsSearchPending] = useState(false); + const [statusDialog, setStatusDialog] = useState(null); const theme = useTheme(); const { t } = useTranslation(); @@ -851,12 +853,28 @@ export function UnifiedSearchBar({ return height; }, [results]); + const resetOpenResults = useEffectEvent(() => { + const firstIndex = Math.max( + results.findIndex((result) => result.kind === "result"), + 0 + ); + selectedValueRef.current = results[firstIndex]?.value; + setSelectedIndex(firstIndex); + rowVirtualizer.scrollToOffset(0); + }); + + useLayoutEffect(() => { + if (listContainer) { + resetOpenResults(); + } + }, [listContainer]); + // ── Scroll selected item into view ───────────────────────────────────── useEffect(() => { - if (results.length > 0) { + if (listContainer && results.length > 0) { rowVirtualizer.scrollToIndex(selectedIndex, { align: "auto" }); } - }, [selectedIndex, results.length, rowVirtualizer]); + }, [listContainer, selectedIndex, results.length, rowVirtualizer]); // ── Focus activation ─────────────────────────────────────────────────── @@ -876,8 +894,9 @@ export function UnifiedSearchBar({ // handleClickAway // const handleClickAway = useCallback(() => { + if (statusDialog) return; dismissSearch(); - }, [dismissSearch]); + }, [dismissSearch, statusDialog]); // ── Re-open on typing after click-away ───────────────────────────────── // @@ -902,8 +921,8 @@ export function UnifiedSearchBar({ } if ( - (!hasSearched && query.length === 0 && effectiveMinQueryLength === 0) || - (newlyActivated && hasSearched && query.length >= effectiveMinQueryLength) + newlyActivated && + ((!hasSearched && query.length === 0 && effectiveMinQueryLength === 0) || (hasSearched && query.length >= effectiveMinQueryLength)) ) { void executeSearch(query); } @@ -993,7 +1012,10 @@ export function UnifiedSearchBar({ const shouldOpenOnActivation = shouldSearchOnActivation || shouldShowBelowMinimumOnActivation; const shouldPreserveSearchStateOnActivation = shouldSearchOnActivation && !providerChanged; - if (!shouldPreserveSearchStateOnActivation) { + setStatusDialog(null); + if (shouldPreserveSearchStateOnActivation) { + resetOpenResults(); + } else { setResults([]); setSelectedIndex(0); setHasSearched(false); @@ -1012,14 +1034,14 @@ export function UnifiedSearchBar({ lastResetProviderRef.current = provider; lastActivationTokenRef.current = activationToken; - activatedRef.current = isInputFocused; + activatedRef.current = isInputFocused || shouldSearchOnActivation; - if (shouldSearchOnActivation) { - void executeSearch(nextQuery); + if (activatedRef.current) { + providerRef.current.onActivate?.(); } - if (isInputFocused) { - providerRef.current.onActivate?.(); + if (shouldSearchOnActivation) { + void executeSearch(nextQuery); } }, [ activationToken, @@ -1069,14 +1091,17 @@ export function UnifiedSearchBar({ } }, [disableDropdown, executeSearch, getEffectiveMinQueryLength, query, refreshToken, suppressDropdown]); - useEffect(() => { - if (disableDropdown || suppressDropdown) { - return; + const resumeFocusedSearch = useEffectEvent(() => { + if (document.activeElement === effectiveInputRef.current && query.length >= getEffectiveMinQueryLength(query)) { + void executeSearch(query); } - if (document.activeElement === effectiveInputRef.current && provider.minQueryLength === 0) { - void executeSearch(""); + }); + + useEffect(() => { + if (!disableDropdown && !suppressDropdown && provider.minQueryLength === 0) { + resumeFocusedSearch(); } - }, [disableDropdown, effectiveInputRef, executeSearch, provider.minQueryLength, suppressDropdown]); + }, [disableDropdown, provider.minQueryLength, suppressDropdown]); useEffect(() => { if (isModeMenuOpen && isDropdownOpen) { @@ -1232,6 +1257,10 @@ export function UnifiedSearchBar({ {/* Status indicator */} {statusInfo && ( event.preventDefault() : undefined} + onClick={statusInfo.loadDetails ? () => setStatusDialog(statusInfo) : undefined} sx={{ px: 2, py: 0.5, @@ -1240,10 +1269,16 @@ export function UnifiedSearchBar({ gap: 1, borderBottom: results.length > 0 || showNoResults || showSearchError || showRemoveError ? 1 : 0, borderColor: "divider", + cursor: statusInfo.loadDetails ? "pointer" : undefined, + textDecoration: statusInfo.loadDetails ? "underline" : undefined, + "&:hover": statusInfo.loadDetails ? { color: "primary.main" } : {}, }} > {statusInfo.showSpinner && } - + {statusInfo.label} @@ -1408,6 +1443,7 @@ export function UnifiedSearchBar({ )} + {statusDialog && setStatusDialog(null)} />} ); diff --git a/frontend/src/components/FileBrowser/__tests__/UnifiedSearchBar.test.tsx b/frontend/src/components/FileBrowser/__tests__/UnifiedSearchBar.test.tsx index c3d9748b..7b8b2aee 100644 --- a/frontend/src/components/FileBrowser/__tests__/UnifiedSearchBar.test.tsx +++ b/frontend/src/components/FileBrowser/__tests__/UnifiedSearchBar.test.tsx @@ -4,13 +4,16 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { setLocale, translate } from "../../../i18n"; import { browserHistoryService } from "../../../services/browserHistoryService"; import { SambeeThemeProvider } from "../../../theme/ThemeContext"; +import { type FileEntry, FileType } from "../../../types"; import { getQuickBarResultRowHeight, QUICK_BAR_RESULT_GROUP_HEADER_HEIGHT, QUICK_BAR_RESULT_ITEM_HEIGHT } from "../QuickBarResultRow"; import { SEARCH_CONTRACT_VERSION } from "../search/recentHistory"; import type { SearchProvider, SearchResultsUpdate } from "../search/types"; import { useDirectorySearchProvider } from "../search/useDirectorySearchProvider"; +import { useFileSearchProvider } from "../search/useFileSearchProvider"; import { UnifiedSearchBar } from "../UnifiedSearchBar"; const virtualizerSizeCache = new Map(); +const virtualizerScrollToOffset = vi.fn(); const INPUT_DEBOUNCE_MS = 160; const QUIET_REFRESH_INTERVAL_MS = 500; const MOCK_HISTORY_RESPONSE_MS = 40; @@ -48,6 +51,7 @@ vi.mock("@tanstack/react-virtual", () => ({ return virtualItem; }); }, + scrollToOffset: virtualizerScrollToOffset, scrollToIndex: vi.fn(), }; }, @@ -634,6 +638,7 @@ describe("UnifiedSearchBar", () => { afterEach(async () => { cleanup(); virtualizerSizeCache.clear(); + virtualizerScrollToOffset.mockClear(); await setLocale("en"); }); @@ -694,6 +699,74 @@ describe("UnifiedSearchBar", () => { expect(screen.queryByText("Loading history")).not.toBeInTheDocument(); }); + it("opens status details by pointer without changing the query or list selection", async () => { + const user = userEvent.setup(); + const loadDetails = vi.fn(async () => ({ + source: "FS1", + issues: [{ path: "private", code: "access_denied" as const }], + incomplete: true, + })); + const status = { label: "FS1: not all directories could be indexed.", showSpinner: false, loadDetails }; + renderWithProvider( status }} />); + const input = screen.getByRole("textbox"); + await user.click(input); + await user.type(input, "reports"); + const link = await screen.findByRole("link", { name: status.label }); + expect(link).toHaveAttribute("tabindex", "-1"); + expect(link.querySelector("svg")).toBeNull(); + expect(loadDetails).not.toHaveBeenCalled(); + const selection = input.getAttribute("aria-activedescendant"); + await user.click(link); + expect(await screen.findByRole("dialog", { name: "Directory Search Status" })).toBeInTheDocument(); + expect(await screen.findByText("private")).toBeInTheDocument(); + expect(screen.getByText("Access denied")).toBeInTheDocument(); + expect(screen.getByText("Only some affected directories are listed.")).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Close" })); + await waitFor(() => expect(input).toHaveFocus()); + expect(input).toHaveValue("reports"); + expect(input.getAttribute("aria-activedescendant")).toBe(selection); + expect(screen.getByRole("option", { name: "Documents" })).toBeInTheDocument(); + }); + + it.each(["stale", "failed"])("shows %s details without losing the quick-bar query", async (outcome) => { + const user = userEvent.setup(); + const status = { + label: "FS1: not all directories could be indexed.", + showSpinner: false, + loadDetails: + outcome === "stale" + ? async () => ({ source: "FS1", issues: [] }) + : async () => { + throw new Error("Disconnected"); + }, + }; + renderWithProvider( status }} />); + const input = screen.getByRole("textbox"); + await user.click(input); + await user.type(input, "reports"); + await user.click(await screen.findByRole("link", { name: status.label })); + const message = + outcome === "stale" + ? "These details are no longer available. Refresh the status." + : "Could not load status details. Check the source connection and reopen this dialog."; + expect(await screen.findByText(message)).toBeInTheDocument(); + await user.keyboard("{Escape}"); + await waitFor(() => expect(input).toHaveFocus()); + expect(input).toHaveValue("reports"); + }); + + it("leaves statuses without additional information as plain text", async () => { + const user = userEvent.setup(); + const status = { label: "FS1: not all directories could be indexed.", showSpinner: false }; + renderWithProvider( status }} />); + await user.click(screen.getByRole("textbox")); + await user.type(screen.getByRole("textbox"), "reports"); + expect(await screen.findByText(status.label)).toBeInTheDocument(); + expect(screen.queryByRole("link")).not.toBeInTheDocument(); + await user.click(screen.getByText(status.label)); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + it("publishes selectable rows before the final response and rejects old-query updates", async () => { const user = userEvent.setup(); const updates = new Map(); @@ -1005,6 +1078,168 @@ describe("UnifiedSearchBar", () => { }); }); + it("fetches file history when the source updates during first activation", async () => { + const searchRecentFiles = vi.spyOn(browserHistoryService, "searchRecentFiles").mockResolvedValue({ results: [], result_limit: 50 }); + const initialFiles: FileEntry[] = []; + const loadedFiles = [{ name: "report.txt", type: FileType.FILE }] as FileEntry[]; + function SearchHarness({ active }: { active: boolean }) { + const fileProvider = useFileSearchProvider({ + connectionId: "connection-1", + currentPath: "", + files: active ? loadedFiles : initialFiles, + connectionName: "Demo", + resultLimit: 50, + getConnectionName: () => "Demo", + onOpenCurrentFile: vi.fn(), + onOpenRecentFile: vi.fn(), + }); + return ; + } + const { rerender } = renderWithProvider(); + + rerender( + + + + ); + await act(async () => screen.getByRole("textbox").focus()); + + expect(await screen.findByRole("option", { name: /report.txt/ })).toBeInTheDocument(); + expect(searchRecentFiles).toHaveBeenCalledTimes(1); + }); + + it("waits for file history before opening the initial file-search menu", async () => { + const user = userEvent.setup(); + let releaseHistory = () => {}; + vi.spyOn(browserHistoryService, "searchRecentFiles").mockImplementation( + () => + new Promise((resolve) => { + releaseHistory = () => resolve({ results: [], result_limit: 50 }); + }) + ); + const files = [{ name: "report.txt", type: FileType.FILE }] as FileEntry[]; + function SearchHarness() { + const provider = useFileSearchProvider({ + connectionId: "connection-1", + currentPath: "", + files, + connectionName: "Demo", + resultLimit: 50, + getConnectionName: () => "Demo", + onOpenCurrentFile: vi.fn(), + onOpenRecentFile: vi.fn(), + }); + return ; + } + renderWithProvider(); + + await user.click(screen.getByRole("textbox")); + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + + await act(async () => releaseHistory()); + expect(await screen.findByRole("option", { name: /report.txt/ })).toBeInTheDocument(); + }); + + it("starts one search when shortcut activation is followed by input focus", async () => { + const fetchResults = vi.fn(() => new Promise<[]>(() => {})); + const onActivate = vi.fn(); + const provider = { ...resultsProvider, fetchResults, onActivate }; + const { rerender } = renderWithProvider(); + + rerender( + + + + ); + await act(async () => screen.getByRole("textbox").focus()); + + expect(onActivate).toHaveBeenCalledTimes(1); + expect(fetchResults).toHaveBeenCalledTimes(1); + expect(onActivate.mock.invocationCallOrder[0]).toBeLessThan(fetchResults.mock.invocationCallOrder[0] ?? 0); + expect(fetchResults.mock.calls[0]?.[1]?.aborted).toBe(false); + }); + + it.each(["suppressDropdown", "disableDropdown"] as const)("preserves the query when %s is cleared", async (prop) => { + const user = userEvent.setup(); + const fetchResults = vi.fn(async (query) => [ + { + kind: "result", + id: query || "all-commands", + value: query || "all-commands", + icon: "command", + primaryText: query || "All commands", + }, + ]); + const provider = { ...commandsProvider, fetchResults }; + const { rerender } = renderWithProvider(); + + await user.click(screen.getByRole("textbox")); + await user.type(screen.getByRole("textbox"), "rename"); + expect(await screen.findByRole("option", { name: "rename" })).toBeInTheDocument(); + + rerender( + + + + ); + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + fetchResults.mockClear(); + + rerender( + + + + ); + + expect(await screen.findByRole("option", { name: "rename" })).toBeInTheDocument(); + expect(screen.getByRole("textbox")).toHaveValue("rename"); + expect(fetchResults).toHaveBeenCalledTimes(1); + expect(fetchResults.mock.calls[0]?.[0]).toBe("rename"); + }); + + it.each([ + { grouped: false, shortcut: true, close: true }, + { grouped: true, shortcut: true, close: true }, + { grouped: false, shortcut: false, close: true }, + { grouped: true, shortcut: false, close: true }, + { grouped: false, shortcut: true, close: false }, + { grouped: true, shortcut: true, close: false }, + ])("resets results on activation (grouped: $grouped, shortcut: $shortcut, close: $close)", async ({ grouped, shortcut, close }) => { + const user = userEvent.setup(); + const groupedProvider = createGroupedProvider(vi.fn()); + const provider = { + ...groupedProvider, + fetchResults: async (query: string, signal: AbortSignal) => + (await groupedProvider.fetchResults(query, signal)).filter((result) => grouped || result.kind === "result"), + }; + const { rerender } = renderWithProvider(); + const searchInput = screen.getByRole("textbox"); + + await user.click(searchInput); + expect(await screen.findByRole("option", { name: /Quarterly report/ })).toHaveAttribute("aria-selected", "true"); + await user.keyboard("{ArrowDown}"); + expect(screen.getByRole("option", { name: /Annual report/ })).toHaveAttribute("aria-selected", "true"); + if (close) { + await user.keyboard("{Escape}"); + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + } + virtualizerScrollToOffset.mockClear(); + + if (shortcut) { + rerender( + + + + ); + } else { + fireEvent.blur(searchInput); + fireEvent.focus(searchInput); + } + + expect(await screen.findByRole("option", { name: /Quarterly report/ })).toHaveAttribute("aria-selected", "true"); + expect(virtualizerScrollToOffset).toHaveBeenCalledWith(0); + }); + it("reopens the commands dropdown when the same mode is reactivated from outside the quick bar", async () => { const user = userEvent.setup(); diff --git a/frontend/src/components/FileBrowser/search/__tests__/useDirectorySearchProvider.test.ts b/frontend/src/components/FileBrowser/search/__tests__/useDirectorySearchProvider.test.ts index b8eb6d34..1832b03d 100644 --- a/frontend/src/components/FileBrowser/search/__tests__/useDirectorySearchProvider.test.ts +++ b/frontend/src/components/FileBrowser/search/__tests__/useDirectorySearchProvider.test.ts @@ -20,10 +20,68 @@ function wrapper({ children }: { children: ReactNode }) { } describe("getDirectoryResultPresentation", () => { + it.each([false, true])("links named partial coverage only when details exist (%s)", async (available) => { + vi.mocked(api.searchRecentDirectories).mockResolvedValue({ result_limit: 10, results: [] }); + const response = { + results: ["readable"], + total_matches: 1, + directory_count: 2, + cache_state: "ready" as const, + coverage: "partial" as const, + coverage_issue_count: 1, + issue_details_available: available, + search_contract_version: SEARCH_CONTRACT_VERSION, + }; + vi.mocked(api.searchDirectories).mockResolvedValue(response); + const { result } = renderHook(() => useDirectorySearchProvider("local-C", vi.fn(), { getConnectionName: () => "FS1" }), { wrapper }); + await act(async () => { + await result.current.fetchResults("readable", new AbortController().signal); + }); + const status = result.current.getStatusInfo(); + expect(status).toMatchObject({ label: "FS1: not all directories could be indexed.", showSpinner: false }); + expect(!!status?.loadDetails).toBe(available); + if (available) { + vi.mocked(api.searchDirectories).mockResolvedValue({ + ...response, + issue_details: [{ path: "private", code: "access_denied" }], + issue_details_incomplete: true, + }); + const signal = new AbortController().signal; + await expect(status?.loadDetails?.(signal)).resolves.toEqual({ + source: "FS1", + issues: [{ path: "private", code: "access_denied" }], + incomplete: true, + }); + expect(api.searchDirectories).toHaveBeenLastCalledWith("local-C", "", { + includeDotDirectories: false, + includeIssueDetails: true, + signal, + }); + } + }); + + it("does not attach history error details to an index failure", async () => { + vi.mocked(api.searchRecentDirectories).mockRejectedValue(new Error("History storage is offline.")); + vi.mocked(api.searchDirectories).mockRejectedValue({ + response: { status: 403, data: { detail: { message: "This source is not permitted." } } }, + }); + const { result } = renderHook(() => useDirectorySearchProvider("connection-1", vi.fn(), { getConnectionName: () => "FS1" }), { + wrapper, + }); + await act(async () => { + await result.current.fetchResults("reports", new AbortController().signal); + }); + await expect(result.current.getStatusInfo()?.loadDetails?.(new AbortController().signal)).resolves.toEqual({ + source: "FS1", + message: "This source is not permitted.", + }); + }); + it.each([ - { phase: "building", label: "Indexing... (123 directories found)" }, - { phase: "updating", label: "Updating index... (12 of ~42 directories checked)" }, - ] as const)("shows $phase progress before uncertain coverage", async ({ phase, label }) => { + { phase: "building", found: 0, label: "Indexing..." }, + { phase: "building", found: 123, label: "Indexing... (123 directories found)" }, + { phase: "updating", found: 123, label: "Updating index... (12 of ~42 directories checked)" }, + ] as const)("shows $phase progress before uncertain coverage", async ({ phase, found, label }) => { vi.mocked(api.searchRecentDirectories).mockResolvedValue({ result_limit: 10, results: [] }); vi.mocked(api.searchDirectories).mockResolvedValue({ results: [], @@ -32,16 +90,80 @@ describe("getDirectoryResultPresentation", () => { directory_count: 7, coverage: "uncertain", search_contract_version: SEARCH_CONTRACT_VERSION, - scan_progress: { phase, parents_done: 12, parents_queued: 30, directories_found: 123, started_at_ms: 1 }, + scan_progress: { phase, parents_done: 12, parents_queued: 30, directories_found: found, started_at_ms: 1 }, }); const { result } = renderHook(() => useDirectorySearchProvider("connection-1", vi.fn()), { wrapper }); await act(async () => { await result.current.fetchResults("reports", new AbortController().signal); }); - expect(result.current.getStatusInfo()).toEqual({ label, showSpinner: true }); + expect(result.current.getStatusInfo()).toEqual({ label: `connection-1: ${label}`, showSpinner: true }); expect(result.current.getRefreshIntervalMs?.()).toBe(500); }); + it.each([ + { cache_state: "building", directory_count: 0, label: "Indexing...", spinner: true }, + { cache_state: "building", directory_count: 3, label: "Indexing... (3 directories found)", spinner: true }, + { cache_state: "updating", directory_count: 3, label: "Updating index... (3 directories)", spinner: true }, + { cache_state: "ready", directory_count: 3, label: null, spinner: false }, + ] as const)( + "keeps opening silent before $cache_state with $directory_count directories", + async ({ cache_state, directory_count, label, spinner }) => { + vi.mocked(api.searchRecentDirectories).mockResolvedValue({ result_limit: 10, results: [] }); + vi.mocked(api.searchDirectories).mockResolvedValue({ + results: [], + total_matches: 0, + cache_state: "opening", + directory_count: 0, + coverage: "partial", + search_contract_version: SEARCH_CONTRACT_VERSION, + }); + const { result } = renderHook(() => useDirectorySearchProvider("connection-1", vi.fn()), { wrapper }); + expect(result.current.getStatusInfo()).toBeNull(); + await act(async () => { + await result.current.fetchResults("reports", new AbortController().signal); + }); + expect(result.current.getStatusInfo()).toBeNull(); + expect(result.current.getRefreshIntervalMs?.()).toBe(500); + vi.mocked(api.searchDirectories).mockResolvedValue({ + results: [], + total_matches: 0, + cache_state, + directory_count, + coverage: "complete", + search_contract_version: SEARCH_CONTRACT_VERSION, + }); + await act(async () => { + await result.current.fetchResults("reports", new AbortController().signal); + }); + expect(result.current.getStatusInfo()).toEqual(label ? { label: `connection-1: ${label}`, showSpinner: spinner } : null); + } + ); + + it("shows a recovery message even while opening", async () => { + vi.mocked(api.searchRecentDirectories).mockResolvedValue({ result_limit: 10, results: [] }); + const message = "Directory index storage is in use. Wait for active workers to finish, then retry."; + vi.mocked(api.searchDirectories).mockResolvedValue({ + results: [], + total_matches: 0, + cache_state: "opening", + directory_count: 0, + status_message: message, + retry_after_ms: 2000, + search_contract_version: SEARCH_CONTRACT_VERSION, + }); + const { result } = renderHook(() => useDirectorySearchProvider("connection-1", vi.fn()), { wrapper }); + await act(async () => { + await result.current.fetchResults("reports", new AbortController().signal); + }); + expect(result.current.getStatusInfo()).toEqual({ label: `connection-1: ${message}`, showSpinner: false }); + expect(result.current.getRefreshIntervalMs?.()).toBe(2000); + vi.mocked(api.searchDirectories).mockRejectedValue(new Error("Source is offline.")); + await act(async () => { + await result.current.fetchResults("reports", new AbortController().signal); + }); + expect(result.current.getStatusInfo()?.label).toContain("Directory search is unavailable"); + }); + it.each(["connection-1", "local-C"])("fences old generation responses and retry errors for %s", async (connectionId) => { type Response = Awaited>; const services = createBrowserContentServices([], new CompanionSession()); @@ -168,10 +290,11 @@ describe("getDirectoryResultPresentation", () => { await result.current.fetchResults("reports", new AbortController().signal); }); expect(commits).toBeGreaterThan(settledCommits); - expect(result.current.getStatusInfo?.()).toEqual({ label: statusMessage, showSpinner: false }); + expect(result.current.getStatusInfo?.()).toEqual({ label: `connection-1: ${statusMessage}`, showSpinner: false }); }); it.each([ + { cache_state: "opening", search_contract_version: SEARCH_CONTRACT_VERSION, expected: 500 }, { cache_state: "building", search_contract_version: SEARCH_CONTRACT_VERSION, expected: 500 }, { cache_state: "updating", search_contract_version: SEARCH_CONTRACT_VERSION, expected: 500 }, { cache_state: "ready", search_contract_version: SEARCH_CONTRACT_VERSION, expected: null }, @@ -245,8 +368,7 @@ describe("getDirectoryResultPresentation", () => { expect(api.searchDirectories).toHaveBeenCalledWith(connectionId, "alpha beta", expect.objectContaining({ signal: expect.anything() })); expect(rows.some((row) => row.kind === "result" && row.value === "directory:alpha/beta")).toBe(true); expect(result.current.getStatusInfo?.()).toEqual({ - label: - "Multi-term directory search is not confirmed for this source. Search one directory term or check backend and companion support.", + label: `${connectionId}: Multi-term directory search is not confirmed for this source. Search one directory term or check backend and companion support.`, showSpinner: false, }); vi.mocked(api.searchDirectories).mockResolvedValue({ ...response, search_contract_version: SEARCH_CONTRACT_VERSION }); @@ -352,7 +474,7 @@ describe("getDirectoryResultPresentation", () => { }); expect(rows.some((row) => row.kind === "result" && row.value === "directory:Reports")).toBe(true); expect(result.current.getStatusInfo?.()).toEqual({ - label: "Directory indexing is paused. Increase its disk budget and retry.", + label: "connection-1: Directory indexing is paused. Increase its disk budget and retry.", showSpinner: false, }); }); diff --git a/frontend/src/components/FileBrowser/search/__tests__/useFileSearchProvider.test.tsx b/frontend/src/components/FileBrowser/search/__tests__/useFileSearchProvider.test.tsx index 78a0eeea..3c451252 100644 --- a/frontend/src/components/FileBrowser/search/__tests__/useFileSearchProvider.test.tsx +++ b/frontend/src/components/FileBrowser/search/__tests__/useFileSearchProvider.test.tsx @@ -56,9 +56,10 @@ describe("useFileSearchProvider", () => { await pending; }); expect(await pending).toEqual(onUpdate.mock.calls[0]?.[0]); - expect(result.current.getStatusInfo()).toEqual({ - label: "Recent files are unavailable. Check the backend connection and retry.", + expect(result.current.getStatusInfo()).toMatchObject({ + label: "Reports server: Recent files are unavailable. Check the backend connection and retry.", showSpinner: false, + loadDetails: expect.any(Function), }); }); diff --git a/frontend/src/components/FileBrowser/search/recentHistory.ts b/frontend/src/components/FileBrowser/search/recentHistory.ts index 2da80028..7c0c3219 100644 --- a/frontend/src/components/FileBrowser/search/recentHistory.ts +++ b/frontend/src/components/FileBrowser/search/recentHistory.ts @@ -7,6 +7,22 @@ export const SEARCH_CONTRACT_VERSION = 1; const NORMALIZATION_KEYS = new Map(unicode.normalization.map(([source, key]) => [source ?? "", key ?? ""])); const QUERY_WHITESPACE = new Set(unicode.whitespace); +export function getSearchErrorMessage(error: unknown): string | undefined { + if (!error || typeof error !== "object") return undefined; + const response = "response" in error ? error.response : undefined; + const data = response && typeof response === "object" && "data" in response ? response.data : undefined; + const detail = data && typeof data === "object" && "detail" in data ? data.detail : undefined; + const message = + typeof detail === "string" + ? detail + : detail && typeof detail === "object" && "message" in detail + ? detail.message + : error instanceof Error + ? error.message + : undefined; + return typeof message === "string" && message.trim() ? message.trim() : undefined; +} + function isWordCharacter(character: string): boolean { const codepoint = character.codePointAt(0) ?? -1; let low = 0; diff --git a/frontend/src/components/FileBrowser/search/types.ts b/frontend/src/components/FileBrowser/search/types.ts index bfc32985..0bbaf22f 100644 --- a/frontend/src/components/FileBrowser/search/types.ts +++ b/frontend/src/components/FileBrowser/search/types.ts @@ -74,11 +74,19 @@ export type SearchResult = SearchResultItem | SearchResultGroupHeader; export type SearchResultsUpdate = (results: SearchResult[]) => void; /** Status information from the provider (e.g., indexing progress) */ +export interface SearchStatusDetails { + source: string; + message?: string; + issues?: import("../../../types").DirectoryIssueDetail[]; + incomplete?: boolean; +} + export interface SearchStatusInfo { /** Short text label describing current status */ label: string; /** Whether an activity spinner should be shown */ showSpinner: boolean; + loadDetails?: (signal: AbortSignal) => Promise; } export type SearchSelectionFocusTarget = "file-list" | "quick-bar" | "none"; diff --git a/frontend/src/components/FileBrowser/search/useDirectorySearchProvider.tsx b/frontend/src/components/FileBrowser/search/useDirectorySearchProvider.tsx index 8f785b6e..619d1a00 100644 --- a/frontend/src/components/FileBrowser/search/useDirectorySearchProvider.tsx +++ b/frontend/src/components/FileBrowser/search/useDirectorySearchProvider.tsx @@ -28,6 +28,7 @@ import { bestSearchRelevance, findSearchHighlights, findTextHighlight, + getSearchErrorMessage, getSearchRelevance, rankHistorySearchResults, removeRecentHistoryResult, @@ -232,14 +233,22 @@ export function useDirectorySearchProvider( options: DirectorySearchProviderOptions = {} ): SearchProvider { const { t } = useTranslation(); - const [cacheState, setCacheState] = useState("empty"); + const [cacheState, setCacheState] = useState("opening"); const [directoryCount, setDirectoryCount] = useState(0); const [totalMatches, setTotalMatches] = useState(0); const [coverageStatus, setCoverageStatus] = useState | null>(null); const [sourceFailure, setSourceFailure] = useState<"history" | "index" | null>(null); + const [failureDetail, setFailureDetail] = useState(); const [legacyIndex, setLegacyIndex] = useState(false); const indexFailureRef = useRef(false); const indexResponseRef = useRef(null); @@ -264,6 +273,7 @@ export function useDirectorySearchProvider( setCoverageStatus((previous) => previous?.coverage === response.coverage && previous?.coverage_issue_count === response.coverage_issue_count && + previous?.issue_details_available === response.issue_details_available && previous?.last_verified_at_ms === response.last_verified_at_ms && previous?.status_message === response.status_message && previous?.monitoring_state === response.monitoring_state && @@ -277,6 +287,7 @@ export function useDirectorySearchProvider( : { coverage: response.coverage, coverage_issue_count: response.coverage_issue_count, + issue_details_available: response.issue_details_available, last_verified_at_ms: response.last_verified_at_ms, status_message: response.status_message, monitoring_state: response.monitoring_state, @@ -297,7 +308,8 @@ export function useDirectorySearchProvider( terminalIndexRef.current = false; indexFailureRef.current = false; setSourceFailure(null); - setCacheState("empty"); + setFailureDetail(undefined); + setCacheState("opening"); setCoverageStatus(null); return () => { requestGenerationRef.current += 1; @@ -447,6 +459,15 @@ export function useDirectorySearchProvider( retryDelayRef.current = failure.delay; } setSourceFailure(indexFailureRef.current ? "index" : recentOutcome.status === "rejected" ? "history" : null); + setFailureDetail((previous) => + indexOutcome.status === "rejected" + ? getSearchErrorMessage(indexOutcome.reason) + : indexFailureRef.current + ? previous + : recentOutcome.status === "rejected" + ? getSearchErrorMessage(recentOutcome.reason) + : undefined + ); return mergeResults(); }, [ @@ -508,12 +529,15 @@ export function useDirectorySearchProvider( }; } if (coverageStatus?.status_message) return { label: coverageStatus.status_message, showSpinner: !!working }; + if (cacheState === "opening") return null; const progress = coverageStatus?.scan_progress; if (progress && progress.phase !== "idle") { return { label: progress.phase === "building" - ? t("fileBrowser.search.status.indexing", { count: progress.directories_found }) + ? progress.directories_found > 0 + ? t("fileBrowser.search.status.indexing", { count: progress.directories_found }) + : t("fileBrowser.search.status.indexingPending") : t("fileBrowser.search.status.updatingProgress", { done: progress.parents_done, total: progress.parents_done + progress.parents_queued, @@ -532,7 +556,10 @@ export function useDirectorySearchProvider( switch (cacheState) { case "building": return { - label: t("fileBrowser.search.status.indexing", { count: directoryCount }), + label: + directoryCount > 0 + ? t("fileBrowser.search.status.indexing", { count: directoryCount }) + : t("fileBrowser.search.status.indexingPending"), showSpinner: !!working, }; case "updating": @@ -556,7 +583,8 @@ export function useDirectorySearchProvider( if (!response || response.search_contract_version !== SEARCH_CONTRACT_VERSION) return null; if (response.retry_after_ms != null && response.retry_after_ms > 0) return directoryRetryDelay(response.retry_after_ms); if (response.scan_progress && response.scan_progress.phase !== "idle") return DIRECTORY_REFRESH_INTERVAL_MS; - if (response.cache_state === "building" || response.cache_state === "updating") return DIRECTORY_REFRESH_INTERVAL_MS; + if (response.cache_state === "opening" || response.cache_state === "building" || response.cache_state === "updating") + return DIRECTORY_REFRESH_INTERVAL_MS; return response.cache_state === "empty" || response.status_message || response.coverage === "uncertain" || @@ -586,6 +614,7 @@ export function useDirectorySearchProvider( indexResponseRef.current = result; indexFailureRef.current = false; setSourceFailure(null); + setFailureDetail(undefined); terminalIndexRef.current = result.search_contract_version != null && result.search_contract_version !== SEARCH_CONTRACT_VERSION; updateIndexStatus(result); } @@ -597,6 +626,7 @@ export function useDirectorySearchProvider( retryDelayRef.current = failure.delay; indexFailureRef.current = true; setSourceFailure("index"); + setFailureDetail(getSearchErrorMessage(error)); logger.warn("Directory cache activation is unavailable", {}, "directory-search-provider"); } }); @@ -641,7 +671,36 @@ export function useDirectorySearchProvider( fetchResults, onSelect, onRemoveSelected, - getStatusInfo, + getStatusInfo: () => { + const status = getStatusInfo(); + if (!status) return null; + const source = options.getConnectionName?.(connectionId) ?? connectionId; + const partial = coverageStatus?.coverage === "partial" && cacheState === "ready" && !sourceFailure && !coverageStatus.status_message; + return { + ...status, + label: t("fileBrowser.search.status.connectionStatus", { + connection: source, + message: partial ? t("fileBrowser.search.status.indexCoveragePartial") : status.label, + }), + ...(partial && coverageStatus.issue_details_available + ? { + loadDetails: async (signal: AbortSignal) => { + const response = await history.searchDirectories(connectionId, "", { + includeDotDirectories, + includeIssueDetails: true, + signal, + }); + return { source, issues: response.issue_details ?? [], incomplete: response.issue_details_incomplete }; + }, + } + : {}), + ...(sourceFailure && failureDetail && failureDetail !== status.label + ? { + loadDetails: async () => ({ source, message: failureDetail }), + } + : {}), + }; + }, getRefreshIntervalMs, invalidateResults: () => { recentResponseRef.current = null; diff --git a/frontend/src/components/FileBrowser/search/useFileSearchProvider.tsx b/frontend/src/components/FileBrowser/search/useFileSearchProvider.tsx index e52ec1dd..2825bf7f 100644 --- a/frontend/src/components/FileBrowser/search/useFileSearchProvider.tsx +++ b/frontend/src/components/FileBrowser/search/useFileSearchProvider.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useLayoutEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import type { BrowserOpenMode } from "../../../pages/FileBrowser/types"; import { type BrowserHistoryService, browserHistoryService } from "../../../services/browserHistoryService"; @@ -8,6 +8,7 @@ import { bestSearchRelevance, compareText, findTextHighlight, + getSearchErrorMessage, getSearchRelevance, normalizeSearchText, rankHistorySearchResults, @@ -62,8 +63,9 @@ export function useFileSearchProvider({ const requestGenerationRef = useRef(0); const sourceBindingRef = useRef({ connectionId, currentPath, files, history, resultLimit }); const [historyUnavailable, setHistoryUnavailable] = useState(false); + const [failureDetail, setFailureDetail] = useState(); - useEffect(() => { + useLayoutEffect(() => { sourceBindingRef.current = { connectionId, currentPath, files, history, resultLimit }; return () => { requestGenerationRef.current += 1; @@ -85,6 +87,7 @@ export function useFileSearchProvider({ sourceBindingRef.current.resultLimit === resultLimit; if (!isCurrent()) return []; setHistoryUnavailable(false); + setFailureDetail(undefined); const terms = searchTerms(query); const currentCandidates = files .filter((file) => file.type === "file" && (!terms.length || getSearchRelevance(file.name, terms).matchedTermCount > 0)) @@ -170,16 +173,16 @@ export function useFileSearchProvider({ ); }; const availableResponse: RecentFileSearchResponse = { results: [], result_limit: resultLimit }; - const availableResults = mergeResults(availableResponse); - onUpdate?.(availableResults); + if (terms.length > 0) onUpdate?.(mergeResults(availableResponse)); try { const recentResponse = await history.searchRecentFiles(query, resultLimit, signal); if (!isCurrent()) return []; return mergeResults(recentResponse); - } catch { + } catch (error: unknown) { if (!isCurrent()) return []; setHistoryUnavailable(true); - return availableResults; + setFailureDetail(getSearchErrorMessage(error)); + return mergeResults(availableResponse); } }, [connectionId, connectionName, currentPath, files, getConnectionName, history, resultLimit, t] @@ -201,8 +204,21 @@ export function useFileSearchProvider({ const getStatusInfo = useCallback( (): SearchStatusInfo | null => - historyUnavailable ? { label: t("fileBrowser.search.status.fileHistoryUnavailable"), showSpinner: false } : null, - [historyUnavailable, t] + historyUnavailable + ? { + label: t("fileBrowser.search.status.connectionStatus", { + connection: connectionName, + message: t("fileBrowser.search.status.fileHistoryUnavailable"), + }), + showSpinner: false, + ...(failureDetail && failureDetail !== t("fileBrowser.search.status.fileHistoryUnavailable") + ? { + loadDetails: async () => ({ source: connectionName, message: failureDetail }), + } + : {}), + } + : null, + [connectionName, failureDetail, historyUnavailable, t] ); const onRemoveSelected = useCallback( diff --git a/frontend/src/components/Settings/CompanionPolicyDelivery.tsx b/frontend/src/components/Settings/CompanionPolicyDelivery.tsx index 545655c7..5fd3d52b 100644 --- a/frontend/src/components/Settings/CompanionPolicyDelivery.tsx +++ b/frontend/src/components/Settings/CompanionPolicyDelivery.tsx @@ -20,8 +20,8 @@ export function CompanionPolicyDelivery() { const { t } = useTranslation(); const snapshot = useCompanionSettingsSync(); const busy = snapshot.phase === "synchronizing"; - const failed = snapshot.phase === "delivery_failed"; - const warning = failed || snapshot.phase === "authority_conflict" || snapshot.phase === "update_required"; + const needsRetry = snapshot.phase === "delivery_failed" || snapshot.phase === "pairing_required"; + const warning = needsRetry || snapshot.phase === "authority_conflict" || snapshot.phase === "update_required"; const phaseLabel = (phase: SettingsSyncPhase) => t(`settings.companionSync.${phase}`); return ( @@ -47,7 +47,7 @@ export function CompanionPolicyDelivery() { disabled={busy} onClick={() => companionSettingsSync.request(false, true)} > - {t(failed ? "common.actions.retry" : "settings.directorySearch.refresh")} + {t(needsRetry ? "common.actions.retry" : "settings.directorySearch.refresh")} diff --git a/frontend/src/components/Settings/__tests__/CompanionPolicyDelivery.test.tsx b/frontend/src/components/Settings/__tests__/CompanionPolicyDelivery.test.tsx index 1e8678eb..8639506f 100644 --- a/frontend/src/components/Settings/__tests__/CompanionPolicyDelivery.test.tsx +++ b/frontend/src/components/Settings/__tests__/CompanionPolicyDelivery.test.tsx @@ -36,6 +36,16 @@ describe("Companion shared synchronization status", () => { expect(request).toHaveBeenCalledExactlyOnceWith(false, true); }); + it("shows pairing recovery instructions instead of a delivery failure", async () => { + snapshot.phase = "pairing_required"; + render(); + expect(screen.getByText("Theme: Pairing required")).toBeInTheDocument(); + expect(screen.getByRole("status")).toHaveTextContent("Pair this browser in Settings > Local Drives"); + expect(screen.queryByText(/Delivery failed/)).not.toBeInTheDocument(); + await userEvent.click(screen.getByRole("button", { name: "Retry" })); + expect(request).toHaveBeenCalledExactlyOnceWith(false, true); + }); + it("disables refresh during delivery and keeps notices inside the surface", () => { snapshot.phase = "synchronizing"; render(); @@ -43,16 +53,21 @@ describe("Companion shared synchronization status", () => { expect(screen.getByTestId("companion-settings-surface")).toContainElement(screen.getByRole("status")); }); - it.each(["defaults_active", "synchronized", "synchronizing", "update_required", "authority_conflict", "delivery_failed"] as const)( - "keeps every %s state inside the shared surface", - (phase) => { - snapshot.phase = phase; - snapshot.sections = { theme: phase, localization: "synchronized", directory_index: "defaults_active" }; - render(); - const surface = screen.getByTestId("companion-settings-surface"); - expect(surface).toContainElement(screen.getByRole("status")); - expect(surface).toHaveTextContent("Localization: Synchronized"); - expect(surface).toHaveTextContent("Directory search policy: Built-in defaults"); - } - ); + it.each([ + "defaults_active", + "synchronized", + "synchronizing", + "update_required", + "authority_conflict", + "pairing_required", + "delivery_failed", + ] as const)("keeps every %s state inside the shared surface", (phase) => { + snapshot.phase = phase; + snapshot.sections = { theme: phase, localization: "synchronized", directory_index: "defaults_active" }; + render(); + const surface = screen.getByTestId("companion-settings-surface"); + expect(surface).toContainElement(screen.getByRole("status")); + expect(surface).toHaveTextContent("Localization: Synchronized"); + expect(surface).toHaveTextContent("Directory search policy: Built-in defaults"); + }); }); diff --git a/frontend/src/i18n/resources.ts b/frontend/src/i18n/resources.ts index 50790d92..2473133b 100644 --- a/frontend/src/i18n/resources.ts +++ b/frontend/src/i18n/resources.ts @@ -208,6 +208,7 @@ export const EN_TRANSLATIONS = { synchronizing: "Synchronizing", update_required: "Update required", authority_conflict: "Different settings authority", + pairing_required: "Pairing required", delivery_failed: "Delivery failed; last valid settings active", }, defaults_active: "Built-in defaults are active until settings arrive from a paired server.", @@ -215,6 +216,8 @@ export const EN_TRANSLATIONS = { synchronizing: "Settings are synchronizing. The last valid settings remain active.", update_required: "Update Sambee and Companion to use shared settings delivery.", authority_conflict: "This server does not manage Companion settings. Pair the intended server again to change authority.", + pairing_required: + "Companion could not authenticate this browser. Pair this browser in Settings > Local Drives, or pair it again to repair the connection. Settings checks are paused until you retry or pair successfully.", delivery_failed: "Settings delivery failed. Check the connection and retry. The last valid settings and directory search remain available.", authority: "Settings authority: {{origin}}", @@ -772,11 +775,13 @@ export const EN_TRANSLATIONS = { directoriesIndexed_other: "{{count}} directories indexed", }, status: { + connectionStatus: "{{connection}}: {{message}}", + indexingPending: "Indexing...", indexing: "Indexing... ({{count}} directories found)", updating: "Updating index... ({{count}} directories)", updatingProgress: "Updating index... ({{done}} of ~{{total}} directories checked)", startingIndex: "Starting index...", - indexCoveragePartial: "Directory index has incomplete coverage.", + indexCoveragePartial: "not all directories could be indexed.", indexCoverageUncertain: "Directory index is not verified. A new check is pending.", legacyDirectorySearch: "Multi-term directory search is not confirmed for this source. Search one directory term or check backend and companion support.", @@ -784,6 +789,20 @@ export const EN_TRANSLATIONS = { fileHistoryUnavailable: "Recent files are unavailable. Check the backend connection and retry.", indexUnavailable: "Directory search is unavailable. Check the source connection and retry.", }, + statusDetails: { + title: "Directory Search Status", + source: "Source", + impact: "Search results may not include directories within these locations.", + incomplete: "Only some affected directories are listed.", + unavailable: "These details are no longer available. Refresh the status.", + failed: "Could not load status details. Check the source connection and reopen this dialog.", + reasons: { + access_denied: "Access denied", + unsupported_encoding: "A directory name could not be read", + unsafe_traversal: "A directory could not be traversed safely", + scan_failed: "A directory could not be read", + }, + }, groups: { recentFiles: "Recent files", recentDirectories: "Recent directories", diff --git a/frontend/src/services/__tests__/browseApi.test.ts b/frontend/src/services/__tests__/browseApi.test.ts index 863a6554..e83af670 100644 --- a/frontend/src/services/__tests__/browseApi.test.ts +++ b/frontend/src/services/__tests__/browseApi.test.ts @@ -688,7 +688,7 @@ describe("Browse API Contract Tests", () => { }); it("should accept all valid cache states", async () => { - const cacheStates = ["empty", "building", "ready", "updating"] as const; + const cacheStates = ["empty", "opening", "building", "ready", "updating"] as const; for (const state of cacheStates) { const backendResponse: DirectorySearchResult = { diff --git a/frontend/src/services/__tests__/companionSettingsSync.test.ts b/frontend/src/services/__tests__/companionSettingsSync.test.ts index d428af93..9aa40b60 100644 --- a/frontend/src/services/__tests__/companionSettingsSync.test.ts +++ b/frontend/src/services/__tests__/companionSettingsSync.test.ts @@ -11,9 +11,11 @@ const mocks = vi.hoisted(() => ({ identity: null as null | (() => void), clear: null as null | (() => void), authenticated: true, + hasSecret: true, })); vi.mock("../api", () => ({ default: { getCompanionSettingsMetadata: mocks.metadata, issueCompanionSettingsGrant: mocks.grant } })); -vi.mock("../companion", () => ({ +vi.mock("../companion", async (importOriginal) => ({ + ...(await importOriginal()), default: { getSettingsStatus: mocks.status, requestSettingsChallenge: mocks.challenge, redeemSettingsGrant: mocks.redeem }, })); vi.mock("../authConfig", () => ({ isAuthRequired: async () => true })); @@ -36,7 +38,7 @@ vi.mock("../authSession", () => ({ subscribeToBootstrap: () => vi.fn(), }, })); -vi.mock("../companionSession", () => ({ companionSession: { hasSecret: () => true, subscribe: () => vi.fn() } })); +vi.mock("../companionSession", () => ({ companionSession: { hasSecret: () => mocks.hasSecret, subscribe: () => vi.fn() } })); vi.mock("../logger", () => ({ logger: { initializeBackendTracing: vi.fn().mockResolvedValue(undefined) } })); import { emitBackendRecoveryConfirmed } from "../backendRecoveryEvents"; @@ -85,6 +87,7 @@ describe("shared Companion settings lifecycle", () => { vi.useFakeTimers(); vi.clearAllMocks(); mocks.authenticated = true; + mocks.hasSecret = true; vi.spyOn(document, "visibilityState", "get").mockReturnValue("visible"); mocks.metadata.mockResolvedValue(metadata()); mocks.status.mockResolvedValue(status()); @@ -107,6 +110,45 @@ describe("shared Companion settings lifecycle", () => { expect(companionSettingsSync.getSnapshot().phase).toBe("synchronized"); }); + it("does not request settings without a stored pairing secret", async () => { + mocks.hasSecret = false; + companionSettingsSync.start(); + await settle(); + await vi.advanceTimersByTimeAsync(120_000); + expect(mocks.status).not.toHaveBeenCalled(); + expect(mocks.metadata).not.toHaveBeenCalled(); + }); + + it.each([ + { code: "companion_auth_signature_mismatch", detail: "Invalid authentication" }, + { code: "companion_pairing_not_found", detail: "Not paired with this origin" }, + { detail: "Invalid authentication" }, + { detail: "Not paired with this origin" }, + ])("blocks automatic retries after pairing failure: $detail ($code)", async (data) => { + mocks.status.mockRejectedValue({ response: { status: 403, data } }); + companionSettingsSync.start(); + await settle(); + + expect(companionSettingsSync.getSnapshot()).toMatchObject({ phase: "pairing_required", message: null }); + window.dispatchEvent(new Event("online")); + document.dispatchEvent(new Event("visibilitychange")); + emitBackendRecoveryConfirmed("pairing-retry-test"); + await vi.advanceTimersByTimeAsync(120_000); + expect(mocks.status).toHaveBeenCalledOnce(); + expect(mocks.grant).not.toHaveBeenCalled(); + expect(vi.getTimerCount()).toBe(0); + + companionSettingsSync.request(false, true); + await settle(); + expect(mocks.status).toHaveBeenCalledTimes(2); + + mocks.status.mockResolvedValue(status()); + notifyCompanionSettingsChanged(true); + await settle(); + expect(mocks.status).toHaveBeenCalledTimes(3); + expect(companionSettingsSync.getSnapshot().phase).toBe("synchronized"); + }); + it("delivers only a changed theme section", async () => { const native = status(); native.sections.theme!.marker = "old"; diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 1c19d848..5870089c 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -91,6 +91,7 @@ const COMPANION_TIMING_OTHER_ROUTE = "/{route}"; export interface DirectorySearchOptions { includeDotDirectories?: boolean; + includeIssueDetails?: boolean; signal?: AbortSignal; } @@ -1592,6 +1593,7 @@ class ApiService { params: { q: query, include_dot_directories: options?.includeDotDirectories ?? false, + ...(options?.includeIssueDetails ? { include_issue_details: true } : {}), }, signal: options?.signal, }); diff --git a/frontend/src/services/browserHistoryService.ts b/frontend/src/services/browserHistoryService.ts index de12b335..590fcfaf 100644 --- a/frontend/src/services/browserHistoryService.ts +++ b/frontend/src/services/browserHistoryService.ts @@ -13,7 +13,7 @@ export interface BrowserHistoryService { searchDirectories( connectionId: string, query: string, - options: { includeDotDirectories: boolean; signal: AbortSignal } + options: { includeDotDirectories: boolean; includeIssueDetails?: boolean; signal: AbortSignal } ): Promise; } diff --git a/frontend/src/services/companionSettingsSync.ts b/frontend/src/services/companionSettingsSync.ts index c80da385..93362f55 100644 --- a/frontend/src/services/companionSettingsSync.ts +++ b/frontend/src/services/companionSettingsSync.ts @@ -12,7 +12,7 @@ import api from "./api"; import { isAuthRequired } from "./authConfig"; import { authSession } from "./authSession"; import { subscribeBackendRecoveryConfirmed } from "./backendRecoveryEvents"; -import companionService from "./companion"; +import companionService, { isCompanionAuthSignatureMismatch, isCompanionPairingMissing } from "./companion"; import { companionSession } from "./companionSession"; import { COMPANION_SETTINGS_CHANGED_EVENT } from "./companionSettingsEvents"; @@ -25,6 +25,7 @@ export type SettingsSyncPhase = | "synchronizing" | "update_required" | "authority_conflict" + | "pairing_required" | "delivery_failed"; export interface SettingsSyncSnapshot { phase: SettingsSyncPhase; @@ -278,19 +279,21 @@ class CompanionSettingsSynchronizer { const status = isApiError(error) ? error.response?.status : undefined; const code = isApiError(error) ? error.response?.data?.code : undefined; const phase = - status === 404 || status === 426 || code === "settings_update_required" + status === 404 || status === 426 || code === "settings_update_required" || code === "companion_browser_update_required" ? "update_required" : code === "settings_authority_conflict" ? "authority_conflict" - : "delivery_failed"; - this.blocked = phase === "authority_conflict" || phase === "update_required"; + : isCompanionAuthSignatureMismatch(error) || isCompanionPairingMissing(error) + ? "pairing_required" + : "delivery_failed"; + this.blocked = phase === "authority_conflict" || phase === "update_required" || phase === "pairing_required"; this.publish({ ...this.snapshot, phase, - message: getApiErrorMessage( - error, - "Settings delivery failed. Check the connection and retry. The last valid settings remain active." - ), + message: + phase === "pairing_required" + ? null + : getApiErrorMessage(error, "Settings delivery failed. Check the connection and retry. The last valid settings remain active."), sections: Object.fromEntries(Object.keys(this.snapshot.sections).map((name) => [name, phase])), }); delay = RETRY_DELAYS_MS[this.retries++] ?? CHECK_INTERVAL_MS; diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index 02eef95f..cf80b2be 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -609,17 +609,25 @@ export interface ScanProgress { started_at_ms: number; } +export interface DirectoryIssueDetail { + path: string; + code: "access_denied" | "unsupported_encoding" | "unsafe_traversal" | "scan_failed"; +} + export interface DirectorySearchResult { search_contract_version?: number | null; results: string[]; match_ranks?: Record; total_matches: number; - cache_state: "empty" | "building" | "ready" | "updating"; + cache_state: "empty" | "opening" | "building" | "ready" | "updating"; directory_count: number; matched_term_counts?: Record; accepted_revision?: number | null; coverage?: "partial" | "complete" | "uncertain" | null; coverage_issue_count?: number; + issue_details_available?: boolean; + issue_details?: DirectoryIssueDetail[] | null; + issue_details_incomplete?: boolean; last_verified_at_ms?: number | null; status_message?: string | null; monitoring_state?: "starting" | "healthy" | "recovering" | "unavailable" | null; diff --git a/website-meta/docs-reports/docs-structure-report.html b/website-meta/docs-reports/docs-structure-report.html index 7d05afb2..922b319b 100644 --- a/website-meta/docs-reports/docs-structure-report.html +++ b/website-meta/docs-reports/docs-structure-report.html @@ -268,7 +268,7 @@

Content diff

- +