mirror of
https://github.com/herdrdev/herdr.git
synced 2026-09-22 00:01:06 +00:00
fix: auto-expire worktree action error banners (#4063)
* fix: auto-expire worktree action error banners refs #1827 * fix: restart endpoint error timeout on repeated errors Route every transient client-side action error through a setter that installs a fresh deadline, so an identical repeated message cannot inherit an older, nearly-expired deadline. refs #1827 * test: derive endpoint error expiry assertions from stored deadline refs #1827
This commit is contained in:
+3
-1
@@ -2026,7 +2026,9 @@ async fn run_client_loop(
|
||||
outcome.actions.extend(actions);
|
||||
}
|
||||
let (effects, notification_repaint) = shell.tick_notifications(now);
|
||||
outcome.repaint |= notification_repaint | shell.tick_copy_feedback(now);
|
||||
outcome.repaint |= notification_repaint
|
||||
| shell.tick_copy_feedback(now)
|
||||
| shell.tick_endpoint_error(now);
|
||||
let frame = outcome
|
||||
.repaint
|
||||
.then(|| shell.compose(state.reported_size.0, state.reported_size.1))
|
||||
|
||||
+13
-13
@@ -192,9 +192,8 @@ impl ClientShellState {
|
||||
.then(|| candidate.command_id.clone())
|
||||
});
|
||||
let Some(command_id) = command_id else {
|
||||
self.endpoint_error = Some(
|
||||
"custom command is not available on this endpoint; reload configuration"
|
||||
.to_owned(),
|
||||
self.set_endpoint_error(
|
||||
"custom command is not available on this endpoint; reload configuration",
|
||||
);
|
||||
outcome.repaint = true;
|
||||
return;
|
||||
@@ -608,8 +607,7 @@ impl ClientShellState {
|
||||
(false, Vec::new())
|
||||
}
|
||||
Ok(_) => {
|
||||
self.endpoint_error =
|
||||
Some("endpoint returned an unexpected selection result".to_owned());
|
||||
self.set_endpoint_error("endpoint returned an unexpected selection result");
|
||||
(true, Vec::new())
|
||||
}
|
||||
Err(_) => (true, Vec::new()),
|
||||
@@ -667,8 +665,7 @@ impl ClientShellState {
|
||||
(false, replay_action(replay))
|
||||
}
|
||||
Ok(_) => {
|
||||
self.endpoint_error =
|
||||
Some("endpoint returned an unexpected link result".to_owned());
|
||||
self.set_endpoint_error("endpoint returned an unexpected link result");
|
||||
(true, replay_action(replay))
|
||||
}
|
||||
Err(error)
|
||||
@@ -706,8 +703,9 @@ impl ClientShellState {
|
||||
),
|
||||
Ok(crate::api::schema::ResponseResult::PaneCopyMotion { .. }) => (false, false),
|
||||
Ok(_) => {
|
||||
self.endpoint_error =
|
||||
Some("endpoint returned an unexpected copy-motion result".to_owned());
|
||||
self.set_endpoint_error(
|
||||
"endpoint returned an unexpected copy-motion result",
|
||||
);
|
||||
(true, false)
|
||||
}
|
||||
Err(_) => (true, false),
|
||||
@@ -761,8 +759,9 @@ impl ClientShellState {
|
||||
}
|
||||
Ok(_) => {
|
||||
self.cancel_deferred_copy_after_search(generation);
|
||||
self.endpoint_error =
|
||||
Some("endpoint returned an unexpected copy-search result".to_owned());
|
||||
self.set_endpoint_error(
|
||||
"endpoint returned an unexpected copy-search result",
|
||||
);
|
||||
(true, false)
|
||||
}
|
||||
Err(_) => {
|
||||
@@ -777,8 +776,9 @@ impl ClientShellState {
|
||||
let repaint = match result {
|
||||
Ok(crate::api::schema::ResponseResult::ConfigReload { .. }) => false,
|
||||
Ok(_) => {
|
||||
self.endpoint_error =
|
||||
Some("endpoint returned an unexpected config reload result".to_owned());
|
||||
self.set_endpoint_error(
|
||||
"endpoint returned an unexpected config reload result",
|
||||
);
|
||||
true
|
||||
}
|
||||
Err(_) => true,
|
||||
|
||||
@@ -63,7 +63,7 @@ impl ClientShellState {
|
||||
remote_collapsed_groups,
|
||||
};
|
||||
if let Err(error) = preferences::store(path, preferences) {
|
||||
self.endpoint_error = Some(error);
|
||||
self.set_endpoint_error(error);
|
||||
outcome.repaint = true;
|
||||
}
|
||||
}
|
||||
@@ -97,7 +97,7 @@ impl ClientShellState {
|
||||
.config
|
||||
.apply_snapshot_keybindings(profile.as_deref(), &commands)
|
||||
{
|
||||
self.endpoint_error = Some(err);
|
||||
self.set_endpoint_error(err);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -158,6 +158,7 @@ impl ClientShellState {
|
||||
pub(super) fn handle_raw_events(&mut self, events: Vec<RawInputEvent>) -> ClientShellInput {
|
||||
let mut outcome = ClientShellInput::default();
|
||||
if !events.is_empty() && self.endpoint_error.take().is_some() {
|
||||
self.endpoint_error_deadline = None;
|
||||
outcome.repaint = true;
|
||||
}
|
||||
for event in events {
|
||||
|
||||
@@ -123,8 +123,7 @@ impl ClientShellState {
|
||||
Ok(_) => {
|
||||
self.pane_scroll_queued.remove(&pane_id);
|
||||
self.pane_scroll_targets.remove(&pane_id);
|
||||
self.endpoint_error =
|
||||
Some("endpoint returned an unexpected pane-scroll result".to_owned());
|
||||
self.set_endpoint_error("endpoint returned an unexpected pane-scroll result");
|
||||
true
|
||||
}
|
||||
Err(_) => {
|
||||
|
||||
@@ -165,7 +165,7 @@ impl ClientShellState {
|
||||
outcome: &mut ClientShellInput,
|
||||
) -> bool {
|
||||
if let Err(error) = crate::config::write_edit(edit) {
|
||||
self.endpoint_error = Some(error);
|
||||
self.set_endpoint_error(error);
|
||||
outcome.repaint = true;
|
||||
return false;
|
||||
}
|
||||
@@ -303,8 +303,8 @@ impl ClientShellState {
|
||||
.min(settings.integrations.len().saturating_sub(1));
|
||||
}
|
||||
Ok(_) => {
|
||||
self.endpoint_error = Some(
|
||||
"endpoint returned an unexpected integration list result".into(),
|
||||
self.set_endpoint_error(
|
||||
"endpoint returned an unexpected integration list result",
|
||||
);
|
||||
}
|
||||
Err(_) => {}
|
||||
|
||||
@@ -3,6 +3,7 @@ use super::*;
|
||||
pub(super) const MIN_TAB_WIDTH: u16 = 8;
|
||||
pub(super) const NEW_TAB_WIDTH: u16 = 3;
|
||||
pub(super) const WORKSPACE_HEADER_ROWS: u16 = 2;
|
||||
const ENDPOINT_ERROR_TIMEOUT_SECS: u64 = 5;
|
||||
|
||||
fn pane_surface_row<'a>(
|
||||
surface: &'a PaneSurfaceFrame,
|
||||
@@ -974,6 +975,7 @@ pub(crate) struct ClientShellState {
|
||||
pub(super) local_config_diagnostic: Option<String>,
|
||||
pub(super) config_diagnostic: Option<String>,
|
||||
pub(super) endpoint_error: Option<String>,
|
||||
pub(super) endpoint_error_deadline: Option<std::time::Instant>,
|
||||
pub(super) dismissed_product_announcement: Option<(String, String)>,
|
||||
}
|
||||
|
||||
@@ -1132,6 +1134,7 @@ impl ClientShellState {
|
||||
config_diagnostic: local_config_diagnostic.clone(),
|
||||
local_config_diagnostic,
|
||||
endpoint_error: None,
|
||||
endpoint_error_deadline: None,
|
||||
dismissed_product_announcement: None,
|
||||
}
|
||||
}
|
||||
@@ -1273,6 +1276,7 @@ impl ClientShellState {
|
||||
self.endpoint_notice_seen.clear();
|
||||
self.visible_endpoint_notice = None;
|
||||
self.endpoint_error = None;
|
||||
self.endpoint_error_deadline = None;
|
||||
self.navigate_workspace_id = None;
|
||||
self.overlay = self
|
||||
.config
|
||||
@@ -1389,7 +1393,7 @@ impl ClientShellState {
|
||||
snapshot.server_keybindings_toml.as_deref(),
|
||||
&snapshot.commands,
|
||||
) {
|
||||
self.endpoint_error = Some(err);
|
||||
self.set_endpoint_error(err);
|
||||
} else if active_keymap_changed
|
||||
&& matches!(
|
||||
self.mode,
|
||||
@@ -1693,6 +1697,7 @@ impl ClientShellState {
|
||||
}
|
||||
self.hits.popup = None;
|
||||
self.endpoint_error = None;
|
||||
self.endpoint_error_deadline = None;
|
||||
}
|
||||
if next_popup.is_some() {
|
||||
self.popup_pending = false;
|
||||
@@ -1852,6 +1857,33 @@ impl ClientShellState {
|
||||
repaint
|
||||
}
|
||||
|
||||
/// Show a transient client-side action error, restarting its lifetime.
|
||||
///
|
||||
/// Every assignment must go through this setter so a repeated identical
|
||||
/// message gets a fresh deadline instead of inheriting the previous one.
|
||||
pub(super) fn set_endpoint_error(&mut self, message: impl Into<String>) {
|
||||
self.endpoint_error = Some(message.into());
|
||||
self.endpoint_error_deadline = Some(
|
||||
std::time::Instant::now() + std::time::Duration::from_secs(ENDPOINT_ERROR_TIMEOUT_SECS),
|
||||
);
|
||||
}
|
||||
|
||||
pub(crate) fn tick_endpoint_error(&mut self, now: std::time::Instant) -> bool {
|
||||
if self.endpoint_error.is_none() {
|
||||
self.endpoint_error_deadline = None;
|
||||
return false;
|
||||
}
|
||||
if self
|
||||
.endpoint_error_deadline
|
||||
.is_some_and(|deadline| now >= deadline)
|
||||
{
|
||||
self.endpoint_error = None;
|
||||
self.endpoint_error_deadline = None;
|
||||
return true;
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
pub(crate) fn timer_delay(&self, now: std::time::Instant) -> std::time::Duration {
|
||||
let default = std::time::Duration::from_millis(100);
|
||||
self.selection_autoscroll_deadline
|
||||
|
||||
@@ -1087,6 +1087,71 @@ fn unavailable_worktree_create_does_not_wedge_the_overlay() {
|
||||
.is_some_and(|notice| notice.key.code == "worktree.create"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn worktree_action_errors_expire_without_more_input() {
|
||||
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
|
||||
state.set_snapshot(Box::new(snapshot()));
|
||||
state.set_pane_surface(surface());
|
||||
|
||||
let mut guard = ClientShellInput::default();
|
||||
state.record_binding(
|
||||
crate::input::KeybindMatch::Action(crate::input::KeybindAction::RemoveWorktree),
|
||||
&mut guard,
|
||||
);
|
||||
let message = "This workspace is not a Herdr-managed worktree checkout.";
|
||||
assert_eq!(state.endpoint_error.as_deref(), Some(message));
|
||||
|
||||
let deadline = state.endpoint_error_deadline.expect("deadline");
|
||||
assert!(!state.tick_endpoint_error(deadline - std::time::Duration::from_secs(1)));
|
||||
assert_eq!(state.endpoint_error.as_deref(), Some(message));
|
||||
|
||||
assert!(state.tick_endpoint_error(deadline + std::time::Duration::from_millis(1)));
|
||||
assert!(state.endpoint_error.is_none());
|
||||
|
||||
// A repeated identical message must start a fresh lifetime instead of
|
||||
// inheriting the earlier deadline.
|
||||
let before_repeat = std::time::Instant::now();
|
||||
state.set_endpoint_error(message);
|
||||
assert!(
|
||||
state.endpoint_error_deadline.expect("deadline")
|
||||
>= before_repeat + std::time::Duration::from_secs(5)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn worktree_prepare_rejection_notice_expires() {
|
||||
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
|
||||
state.set_snapshot(Box::new(snapshot()));
|
||||
state.set_pane_surface(surface());
|
||||
let mut prepare = ClientShellInput::default();
|
||||
state.record_binding(
|
||||
crate::input::KeybindMatch::Action(crate::input::KeybindAction::NewWorktree),
|
||||
&mut prepare,
|
||||
);
|
||||
let [ClientShellAction::Endpoint { request, .. }] = &prepare.actions[..] else {
|
||||
panic!("new worktree should prepare through worktree.list");
|
||||
};
|
||||
let request_id = request.id.clone();
|
||||
state.handle_endpoint_result(
|
||||
"boot-1",
|
||||
&request_id,
|
||||
Err(ClientShellEndpointError {
|
||||
code: Some("not_git_worktree".into()),
|
||||
message: "Herdr worktree actions require a workspace inside a Git work tree".into(),
|
||||
}),
|
||||
);
|
||||
let notice = state
|
||||
.visible_endpoint_notice
|
||||
.as_ref()
|
||||
.expect("rejection notice");
|
||||
assert!(notice.key.code.contains("not_git_worktree"));
|
||||
let deadline = notice.deadline;
|
||||
|
||||
let (_, repaint) = state.tick_notifications(deadline + std::time::Duration::from_millis(1));
|
||||
assert!(repaint);
|
||||
assert!(state.visible_endpoint_notice.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn worktree_open_filters_and_clicks_a_stable_public_entry() {
|
||||
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
|
||||
|
||||
@@ -189,8 +189,9 @@ impl ClientShellState {
|
||||
other => {
|
||||
if matches!(other, Ok(value) if !matches!(value, crate::api::schema::ResponseResult::PaneSelection { .. }))
|
||||
{
|
||||
self.endpoint_error =
|
||||
Some("endpoint returned an unexpected word-selection result".to_owned());
|
||||
self.set_endpoint_error(
|
||||
"endpoint returned an unexpected word-selection result",
|
||||
);
|
||||
}
|
||||
self.cancel_word_selection();
|
||||
return (true, Vec::new());
|
||||
|
||||
@@ -213,9 +213,8 @@ impl ClientShellState {
|
||||
.is_some_and(|worktree| worktree.is_linked_worktree);
|
||||
let kind = match action {
|
||||
KeybindAction::NewWorktree | KeybindAction::OpenWorktree if linked => {
|
||||
self.endpoint_error = Some(
|
||||
"New and open worktree actions start from the repo parent workspace."
|
||||
.to_owned(),
|
||||
self.set_endpoint_error(
|
||||
"New and open worktree actions start from the repo parent workspace.",
|
||||
);
|
||||
outcome.repaint = true;
|
||||
return;
|
||||
@@ -227,8 +226,7 @@ impl ClientShellState {
|
||||
workspace_id: workspace_id.clone(),
|
||||
},
|
||||
KeybindAction::RemoveWorktree if !linked => {
|
||||
self.endpoint_error =
|
||||
Some("This workspace is not a Herdr-managed worktree checkout.".to_owned());
|
||||
self.set_endpoint_error("This workspace is not a Herdr-managed worktree checkout.");
|
||||
outcome.repaint = true;
|
||||
return;
|
||||
}
|
||||
@@ -442,7 +440,7 @@ impl ClientShellState {
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
if entries.is_empty() {
|
||||
self.endpoint_error = Some("No Git worktrees found for this repo.".to_owned());
|
||||
self.set_endpoint_error("No Git worktrees found for this repo.");
|
||||
} else {
|
||||
self.overlay = Some(ClientShellOverlay::WorktreeOpen(
|
||||
ClientWorktreeOpenOverlay {
|
||||
@@ -477,8 +475,9 @@ impl ClientShellState {
|
||||
},
|
||||
));
|
||||
} else {
|
||||
self.endpoint_error =
|
||||
Some("This workspace is not a Herdr-managed worktree checkout.".to_owned());
|
||||
self.set_endpoint_error(
|
||||
"This workspace is not a Herdr-managed worktree checkout.",
|
||||
);
|
||||
}
|
||||
true
|
||||
}
|
||||
@@ -543,8 +542,7 @@ impl ClientShellState {
|
||||
Err(_),
|
||||
) => true,
|
||||
(_, Ok(_)) => {
|
||||
self.endpoint_error =
|
||||
Some("endpoint returned an unexpected worktree result".to_owned());
|
||||
self.set_endpoint_error("endpoint returned an unexpected worktree result");
|
||||
true
|
||||
}
|
||||
(
|
||||
|
||||
Reference in New Issue
Block a user