Skip to content

fix(selection-polish): 選區潤色預覽確認按鈕偶發無反應 #1045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
leonoxo wants to merge 1 commit into
base: beta
Choose a base branch
from
Draft

fix(selection-polish): 選區潤色預覽確認按鈕偶發無反應 #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions openless-all/app/crates/openless-core/src/selection_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,16 @@ impl SelectionServiceInner {

fn fail_if_active(&self, session_id: SessionId) -> bool {
let mut state = self.state.write().expect("selection state lock poisoned");
// 只对「还在进行中」的 session 结算：Cancelled 是用户主动结束，
// Completed 是已粘贴成功（race：complete 与 fail 判断之间的窄窗口，
// 若误标 Failed 会把成功状态覆盖掉）。
if state.snapshot.session_id == Some(session_id)
&& !matches!(state.snapshot.phase, SelectionPhase::Cancelled)
&& matches!(
state.snapshot.phase,
SelectionPhase::Capturing
| SelectionPhase::Preview
| SelectionPhase::Applying
)
{
state.snapshot.phase = SelectionPhase::Failed;
let snapshot = state.snapshot.clone();
Expand All @@ -495,6 +503,38 @@ impl SelectionServiceInner {
}
}

/// confirm 失败结算：session 已失效（stale / 目标变更 / 并发占用）结算为
/// Failed 并隐藏预览；瞬时的平台错误（焦点恢复 / 目标复核抖动）回退到
/// Preview 保持可重试——直接失败掉会让预览窗被隐藏、编辑内容丢失，
/// 用户看到的只是「点确认没反应」。
fn settle_confirm_failure(&self, session_id: SessionId, error: &BackendError) -> bool {
let settled = matches!(
error.code,
BackendErrorCode::Cancelled
| BackendErrorCode::InvalidState
| BackendErrorCode::InvalidArgument
| BackendErrorCode::Busy
);
let mut state = self.state.write().expect("selection state lock poisoned");
let active = state.snapshot.session_id == Some(session_id)
&& !matches!(state.snapshot.phase, SelectionPhase::Cancelled);
if !active {
return false;
}
state.snapshot.phase = if settled {
SelectionPhase::Failed
} else {
SelectionPhase::Preview
};
let snapshot = state.snapshot.clone();
drop(state);
self.events.publish(
Some(session_id),
BackendEventKind::SelectionStateChanged(snapshot),
);
settled
}

fn begin_revert(&self, session_id: SessionId) -> Result<(), BackendError> {
let mut state = self.state.write().expect("selection state lock poisoned");
if state.snapshot.session_id != Some(session_id) {
Expand Down Expand Up @@ -650,7 +690,10 @@ impl SelectionApi for SelectionService {
Ok(())
}
Err(error) => {
if inner.fail_if_active(session_id) {
// 分流：session 已失效（stale / 并发 confirm）必须结算；瞬时的
// 平台错误（焦点恢复 / 目标复核抖动）保持 preview 可重试——
// 否则窗口被隐藏、busy 卡死，表现为「点确认没反应」。
if inner.settle_confirm_failure(session_id, &error) {
let _ = inner.polisher.cancel(session_id).await;
let _ = inner.runtime.cancel(session_id).await;
inner.hide_preview();
Expand Down
103 changes: 95 additions & 8 deletions openless-all/app/crates/openless-core/tests/selection_contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ struct RecordingSelectionRuntime {
capture: SelectionCapture,
applied: Arc<Mutex<Vec<(SessionId, String, String)>>>,
apply_outcome: InsertOutcome,
apply_error: Option<BackendError>,
apply_error: Arc<Mutex<Option<BackendError>>>,
apply_gate: Option<(Arc<tokio::sync::Notify>, Arc<tokio::sync::Notify>)>,
reverted: Arc<Mutex<Vec<SessionId>>>,
revert_outcome: Option<InsertOutcome>,
Expand All @@ -42,19 +42,23 @@ impl RecordingSelectionRuntime {
},
applied: Arc::new(Mutex::new(Vec::new())),
apply_outcome: InsertOutcome::Inserted,
apply_error: None,
apply_error: Arc::new(Mutex::new(None)),
apply_gate: None,
reverted: Arc::new(Mutex::new(Vec::new())),
revert_outcome: None,
cancels: Arc::new(std::sync::atomic::AtomicUsize::new(0)),
}
}

fn with_apply_error(mut self, error: BackendError) -> Self {
self.apply_error = Some(error);
fn with_apply_error(self, error: BackendError) -> Self {
*self.apply_error.lock().expect("apply error lock poisoned") = Some(error);
self
}

fn release_apply_error(&self) {
*self.apply_error.lock().expect("apply error lock poisoned") = None;
}

fn with_revert_outcome(mut self, outcome: InsertOutcome) -> Self {
self.revert_outcome = Some(outcome);
self
Expand Down Expand Up @@ -96,10 +100,14 @@ impl SelectionRuntimeAdapter for RecordingSelectionRuntime {
) -> BoxFuture<'static, Result<InsertOutcome, BackendError>> {
let applied = Arc::clone(&self.applied);
let outcome = self.apply_outcome;
let error = self.apply_error.clone();
let error_slot = Arc::clone(&self.apply_error);
let gate = self.apply_gate.clone();
Box::pin(async move {
if let Some(error) = error {
if let Some(error) = error_slot
.lock()
.expect("apply error lock poisoned")
.clone()
{
return Err(error);
}
applied.lock().expect("runtime lock poisoned").push((
Expand Down Expand Up @@ -633,7 +641,7 @@ async fn shutdown_cancels_an_active_selection_and_hides_its_preview() {
}

#[tokio::test]
async fn failed_preview_apply_hides_the_preview_and_releases_the_target() {
async fn transient_platform_failure_keeps_the_preview_retryable() {
let runtime = RecordingSelectionRuntime::new("source text").with_apply_error(
BackendError::new(BackendErrorCode::Platform, "fixture apply failed"),
);
Expand Down Expand Up @@ -668,8 +676,87 @@ async fn failed_preview_apply_hides_the_preview_and_releases_the_target() {
.await
.expect_err("platform failure must be returned");

// 瞬时平台错误（焦点恢复/目标复核抖动）：错误返回、预览窗保持、
// session 回到 Preview 可直接重试——不能隐藏窗口把用户晾在「点了没反应」。
assert_eq!(error.code, BackendErrorCode::Platform);
assert_eq!(runtime.cancel_count(), 1);
assert_eq!(runtime.cancel_count(), 0);
assert_eq!(host.actions(), vec![HostAction::ShowSelectionPreview]);
assert_eq!(
backend
.services()
.selection
.snapshot()
.await
.expect("selection snapshot should remain readable")
.phase,
SelectionPhase::Preview
);

// 目标重新可用时重试应成功完成。
runtime.release_apply_error();
backend
.services()
.selection
.confirm(session_id, None)
.await
.expect("retry after a transient platform failure should apply");
assert_eq!(
backend
.services()
.selection
.snapshot()
.await
.expect("selection snapshot should remain readable")
.phase,
SelectionPhase::Completed
);

backend.shutdown().await.expect("backend should stop");
let _ = std::fs::remove_dir_all(data_dir);
}

#[tokio::test]
async fn stale_preview_apply_settles_the_session_and_hides_the_preview() {
// apply 报「目标已失效」类错误（Cancelled 语义）时 session 必须结算，
// 预览隐藏、不允许无限重试一个已经不存在的目标。
let runtime = RecordingSelectionRuntime::new("source text").with_apply_error(
BackendError::new(
BackendErrorCode::Cancelled,
"selection target is no longer active",
),
);
let host = openless_core::testing::RecordingHostActions::default();
let (backend, data_dir) = backend_with_selection_parts_and_host(
runtime.clone(),
Arc::new(openless_core::testing::FixtureTextPolisher::successful(
"polished preview",
)),
Arc::new(UnsupportedCredentialStore),
Arc::new(host.clone()),
);
backend.start().await.expect("backend should start");
let mut preferences = backend.get_preferences();
preferences.selection_polish_output_mode = SelectionPolishOutputMode::PreviewConfirm;
write_preferences(&backend, preferences);
let session_id = backend
.services()
.selection
.begin_polish(SelectionPolishRequest {
selected_text: None,
mode: PolishMode::Light,
instruction: None,
})
.await
.expect("selection polish should produce a preview");

let error = backend
.services()
.selection
.confirm(session_id, None)
.await
.expect_err("stale target must be returned");

assert_eq!(error.code, BackendErrorCode::Cancelled);
assert_eq!(
host.actions(),
vec![
Expand Down
Loading