From 2feecae70de1c4041b285b30485a52b52e161bb2 Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Thu, 27 Aug 2026 14:59:17 +0200 Subject: [PATCH] Refine implementation findings --- FINDINGS.md | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/FINDINGS.md b/FINDINGS.md index ada6d1c..fe8ac1d 100644 --- a/FINDINGS.md +++ b/FINDINGS.md @@ -39,13 +39,16 @@ proves one snapshot is taken for a directory operation. `crates/storage/src/git.rs` classify authentication, TLS, and network failures using substrings from third-party error messages. -Use structured `reqwest` status/connect/timeout information and matchable `gix` -error variants wherever available. Keep a bounded textual diagnostic only as a -fallback; it must not drive a more specific classification unless the library -exposes no structured alternative. +Use structured `reqwest` status/connect/timeout information, error sources, and +matchable `gix` variants wherever available. Classify TLS only when a structured +source identifies it. When `gix` has erased the underlying status or cause, +return a generic error with a bounded diagnostic instead of guessing a specific +class from rendered text. -Acceptance: tests classify HTTP 401/403, TLS failure, connection/timeout, and an -unknown protocol failure without depending on English error text. +Acceptance: tests classify direct HTTP 401/403 and structured connection, +timeout, and TLS failures without depending on English error text. A `gix` +failure without a structured cause remains generic and its diagnostic is +bounded. ## 4. Make config replacement durable @@ -59,20 +62,7 @@ if the rename succeeds but the directory sync fails. Acceptance: focused tests cover the post-rename sync failure boundary. Do not add config locking unless concurrent writers are an explicit supported use case. -## 5. Do not hold secret-store state across backend I/O - -`SecretStore` in `crates/storage/src/secret_store.rs` holds its mutex while -calling potentially blocking backend operations (`unlock`, `lock`, `create`, -`retrieve`, `replace`, and `delete`). - -Reduce the lock scope around backend calls while preserving atomic lock-state -and cache behavior. Do not weaken biometric/keychain protection or allow a -secret operation to complete as unlocked after a concurrent `lock()`. - -Acceptance: concurrency tests cover `lock()` racing with at least retrieval and -mutation, and prove no stale secret is inserted into the cache. - -## 6. Reap completed TUI task handles +## 5. Reap completed TUI task handles `AsyncExecutor::submit` in `apps/tui/src/runtime.rs` retains every `JoinHandle` until executor drop.