fix(tui): make queued follow-up edits recoverable
This commit is contained in:
@@ -71,6 +71,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- Pending-input preview rows now label delivery mode explicitly as steer
|
||||
pending, rejected steer, or queued follow-up, with wrapped continuation rows
|
||||
aligned under the label so busy-turn input state is easier to read (#2054).
|
||||
- Editing a queued follow-up is now an explicit pending-input state. Pressing
|
||||
`Esc` while editing a queued follow-up restores the original queued message
|
||||
instead of cancelling the active turn or silently dropping the queued work
|
||||
(#2054).
|
||||
- Sidebar hover details now use row-level metadata for truncated Work, Tasks,
|
||||
and Agents rows. Mouse hover opens a bordered, wrapping popover with the full
|
||||
underlying row text, long turn/agent ids, and current sub-agent progress
|
||||
|
||||
@@ -4706,6 +4706,18 @@ impl App {
|
||||
true
|
||||
}
|
||||
|
||||
/// Stop editing a queued follow-up and put the original queued message back
|
||||
/// at the tail where [`Self::pop_last_queued_into_draft`] took it from.
|
||||
pub fn cancel_queued_draft_edit(&mut self) -> bool {
|
||||
let Some(draft) = self.queued_draft.take() else {
|
||||
return false;
|
||||
};
|
||||
self.queued_messages.push_back(draft);
|
||||
self.clear_input_recoverable();
|
||||
self.needs_redraw = true;
|
||||
true
|
||||
}
|
||||
|
||||
/// Park a legacy pending steer. New keyboard handling routes running-turn
|
||||
/// drafts through Enter (same-turn steer) or Tab (next-turn follow-up).
|
||||
#[allow(dead_code)]
|
||||
@@ -7174,6 +7186,33 @@ mod tests {
|
||||
assert!(app.queued_draft.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cancel_queued_draft_edit_restores_original_message() {
|
||||
let mut app = App::new(test_options(false), &Config::default());
|
||||
app.queue_message(QueuedMessage::new("first".to_string(), None));
|
||||
app.queue_message(QueuedMessage::new(
|
||||
"original follow-up".to_string(),
|
||||
Some("skill".to_string()),
|
||||
));
|
||||
assert!(app.pop_last_queued_into_draft());
|
||||
app.input = "edited but not submitted".to_string();
|
||||
app.cursor_position = char_count(&app.input);
|
||||
|
||||
assert!(app.cancel_queued_draft_edit());
|
||||
|
||||
assert!(app.input.is_empty());
|
||||
assert!(app.queued_draft.is_none());
|
||||
assert_eq!(app.queued_messages.len(), 2);
|
||||
let restored = app.queued_messages.back().expect("restored message");
|
||||
assert_eq!(restored.display, "original follow-up");
|
||||
assert_eq!(restored.skill_instruction.as_deref(), Some("skill"));
|
||||
assert_eq!(
|
||||
app.clear_undo_buffer.as_deref(),
|
||||
Some("edited but not submitted"),
|
||||
"the interrupted edit remains recoverable via normal draft recovery"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn finalize_streaming_assistant_marks_existing_cell_interrupted() {
|
||||
let mut app = App::new(test_options(false), &Config::default());
|
||||
|
||||
@@ -16,10 +16,10 @@ pub(crate) enum EscapeAction {
|
||||
pub(crate) fn next_escape_action(app: &App, slash_menu_open: bool) -> EscapeAction {
|
||||
if slash_menu_open {
|
||||
EscapeAction::CloseSlashMenu
|
||||
} else if app.queued_draft.is_some() {
|
||||
EscapeAction::DiscardQueuedDraft
|
||||
} else if app.is_loading || matches!(app.runtime_turn_status.as_deref(), Some("in_progress")) {
|
||||
EscapeAction::CancelRequest
|
||||
} else if app.queued_draft.is_some() && app.input.is_empty() {
|
||||
EscapeAction::DiscardQueuedDraft
|
||||
} else if !app.input.is_empty() {
|
||||
EscapeAction::ClearInput
|
||||
} else {
|
||||
|
||||
@@ -3471,8 +3471,10 @@ async fn run_event_loop(
|
||||
}
|
||||
EscapeAction::DiscardQueuedDraft => {
|
||||
app.backtrack.reset();
|
||||
app.queued_draft = None;
|
||||
app.status_message = Some("Stopped editing queued message".to_string());
|
||||
if app.cancel_queued_draft_edit() {
|
||||
app.status_message =
|
||||
Some("Queued edit canceled; follow-up restored".to_string());
|
||||
}
|
||||
}
|
||||
EscapeAction::ClearInput => {
|
||||
app.backtrack.reset();
|
||||
@@ -6540,6 +6542,13 @@ fn build_pending_input_preview(app: &App) -> PendingInputPreview {
|
||||
.iter()
|
||||
.map(|m| m.display.clone())
|
||||
.collect();
|
||||
preview.editing_queued_message = app.queued_draft.as_ref().map(|draft| {
|
||||
if app.input.trim().is_empty() {
|
||||
draft.display.clone()
|
||||
} else {
|
||||
app.input.clone()
|
||||
}
|
||||
});
|
||||
preview
|
||||
}
|
||||
|
||||
|
||||
@@ -3995,6 +3995,22 @@ fn test_esc_discards_queued_draft_before_clearing_input() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_esc_prioritizes_queued_draft_edit_over_loading_cancel() {
|
||||
let mut app = create_test_app();
|
||||
app.is_loading = true;
|
||||
app.input = "editing queued follow-up".to_string();
|
||||
app.queued_draft = Some(crate::tui::app::QueuedMessage::new(
|
||||
"original queued follow-up".to_string(),
|
||||
None,
|
||||
));
|
||||
|
||||
assert_eq!(
|
||||
next_escape_action(&app, false),
|
||||
EscapeAction::DiscardQueuedDraft
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_esc_is_noop_when_idle() {
|
||||
let mut app = create_test_app();
|
||||
@@ -4200,6 +4216,17 @@ fn test_esc_priority_order_matches_cancel_stack() {
|
||||
app.input.clear();
|
||||
assert_eq!(next_escape_action(&app, false), EscapeAction::CancelRequest);
|
||||
|
||||
app.queued_draft = Some(crate::tui::app::QueuedMessage::new(
|
||||
"queued draft".to_string(),
|
||||
None,
|
||||
));
|
||||
app.input = "editing queued draft".to_string();
|
||||
assert_eq!(
|
||||
next_escape_action(&app, false),
|
||||
EscapeAction::DiscardQueuedDraft
|
||||
);
|
||||
|
||||
app.queued_draft = None;
|
||||
app.is_loading = false;
|
||||
app.input = "draft".to_string();
|
||||
assert_eq!(next_escape_action(&app, false), EscapeAction::ClearInput);
|
||||
@@ -7201,6 +7228,42 @@ fn build_pending_input_preview_populates_all_three_buckets() {
|
||||
assert_eq!(preview.queued_messages, vec!["queued-msg".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accidental_queue_edit_while_loading_is_labeled_and_recoverable() {
|
||||
let mut app = create_test_app();
|
||||
app.is_loading = true;
|
||||
app.queue_message(QueuedMessage::new(
|
||||
"original queued follow-up".to_string(),
|
||||
Some("skill body".to_string()),
|
||||
));
|
||||
|
||||
assert!(app.pop_last_queued_into_draft());
|
||||
assert_eq!(app.input, "original queued follow-up");
|
||||
app.input = "edited queued follow-up".to_string();
|
||||
app.cursor_position = app.input.chars().count();
|
||||
|
||||
let preview = build_pending_input_preview(&app);
|
||||
assert_eq!(
|
||||
preview.editing_queued_message.as_deref(),
|
||||
Some("edited queued follow-up")
|
||||
);
|
||||
assert!(
|
||||
preview.queued_messages.is_empty(),
|
||||
"the popped message should be shown as editing, not a second queued row"
|
||||
);
|
||||
assert_eq!(
|
||||
next_escape_action(&app, false),
|
||||
EscapeAction::DiscardQueuedDraft,
|
||||
"Esc should cancel the queued edit before cancelling the live turn"
|
||||
);
|
||||
|
||||
assert!(app.cancel_queued_draft_edit());
|
||||
assert!(app.input.is_empty());
|
||||
let restored = app.queued_messages.back().expect("follow-up restored");
|
||||
assert_eq!(restored.display, "original queued follow-up");
|
||||
assert_eq!(restored.skill_instruction.as_deref(), Some("skill body"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_pending_input_preview_includes_current_context_chips() {
|
||||
let tmpdir = TempDir::new().expect("tempdir");
|
||||
|
||||
@@ -27,6 +27,7 @@ const PREVIEW_LINE_LIMIT: usize = 3;
|
||||
const PENDING_STEER_PREFIX: &str = " ↳ Steer pending: ";
|
||||
const REJECTED_STEER_PREFIX: &str = " ↳ Rejected steer: ";
|
||||
const QUEUED_MESSAGE_PREFIX: &str = " ↳ Queued follow-up: ";
|
||||
const EDITING_QUEUED_PREFIX: &str = " ↳ Editing queued follow-up: ";
|
||||
|
||||
/// Description of the keybinding the hint line at the bottom should advertise
|
||||
/// for the "edit last queued message" action.
|
||||
@@ -46,6 +47,7 @@ pub struct PendingInputPreview {
|
||||
pub pending_steers: Vec<String>,
|
||||
pub rejected_steers: Vec<String>,
|
||||
pub queued_messages: Vec<String>,
|
||||
pub editing_queued_message: Option<String>,
|
||||
pub edit_binding: EditBinding,
|
||||
}
|
||||
|
||||
@@ -69,6 +71,7 @@ impl PendingInputPreview {
|
||||
pending_steers: Vec::new(),
|
||||
rejected_steers: Vec::new(),
|
||||
queued_messages: Vec::new(),
|
||||
editing_queued_message: None,
|
||||
edit_binding: EditBinding::UP,
|
||||
}
|
||||
}
|
||||
@@ -77,6 +80,7 @@ impl PendingInputPreview {
|
||||
!self.pending_steers.is_empty()
|
||||
|| !self.rejected_steers.is_empty()
|
||||
|| !self.queued_messages.is_empty()
|
||||
|| self.editing_queued_message.is_some()
|
||||
}
|
||||
|
||||
/// Build the (possibly empty) ordered line list this widget would render
|
||||
@@ -134,6 +138,21 @@ impl PendingInputPreview {
|
||||
&rejected_steer_indent,
|
||||
);
|
||||
}
|
||||
if let Some(draft) = self.editing_queued_message.as_deref() {
|
||||
let editing_indent = continuation_indent(EDITING_QUEUED_PREFIX);
|
||||
push_truncated_item(
|
||||
&mut lines,
|
||||
draft,
|
||||
width,
|
||||
dim_italic,
|
||||
EDITING_QUEUED_PREFIX,
|
||||
&editing_indent,
|
||||
);
|
||||
lines.push(Line::from(vec![Span::styled(
|
||||
" Esc restores queued follow-up".to_string(),
|
||||
dim,
|
||||
)]));
|
||||
}
|
||||
let queued_message_indent = continuation_indent(QUEUED_MESSAGE_PREFIX);
|
||||
for message in &self.queued_messages {
|
||||
push_truncated_item(
|
||||
@@ -371,6 +390,32 @@ mod tests {
|
||||
assert!(rows[2].contains("edit last queued message"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn editing_queued_message_renders_explicit_state_and_restore_hint() {
|
||||
let mut preview = PendingInputPreview::new();
|
||||
preview.editing_queued_message = Some("revise before sending".to_string());
|
||||
|
||||
let rows = render_to_string(&preview, 80);
|
||||
|
||||
assert!(rows[0].contains("Pending inputs"));
|
||||
assert!(
|
||||
rows.iter()
|
||||
.any(|row| row.contains("Editing queued follow-up: revise before sending")),
|
||||
"missing editing label: {rows:?}"
|
||||
);
|
||||
assert!(
|
||||
rows.iter()
|
||||
.any(|row| row.contains("Esc restores queued follow-up")),
|
||||
"missing restore hint: {rows:?}"
|
||||
);
|
||||
assert!(
|
||||
!rows
|
||||
.iter()
|
||||
.any(|row| row.contains("edit last queued message")),
|
||||
"editing mode should not also advertise opening a queued edit: {rows:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn context_items_render_before_queue_buckets() {
|
||||
let mut preview = PendingInputPreview::new();
|
||||
@@ -460,6 +505,7 @@ mod tests {
|
||||
preview.pending_steers.push("steer".to_string());
|
||||
preview.rejected_steers.push("rejected".to_string());
|
||||
preview.queued_messages.push("queued".to_string());
|
||||
preview.editing_queued_message = Some("editing".to_string());
|
||||
|
||||
let rows = render_to_string(&preview, 80);
|
||||
|
||||
@@ -477,6 +523,11 @@ mod tests {
|
||||
.any(|row| row.contains("Queued follow-up: queued")),
|
||||
"missing queued-follow-up label: {rows:?}"
|
||||
);
|
||||
assert!(
|
||||
rows.iter()
|
||||
.any(|row| row.contains("Editing queued follow-up: editing")),
|
||||
"missing queued-edit label: {rows:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -52,7 +52,7 @@ harvest/stewardship commits:
|
||||
| #2738 dense tool-call transcript collapse | Locally harvested with expansion, cache-key, and safety fixes. | Successful read/search/list-style tool runs collapse by default once they cross the density threshold; failures, running cells, shell/exec, patch/write/edit/delete, diff preview, plan update, and review cells stay visible. Users can expand a group with Enter/Space/mouse and can set `tool_collapse = "compact" | "expanded" | "calm"`. Credit @idling11 and issue #2692; comment/close the original after the integration branch is public. |
|
||||
| #2740 dense tool-run collapse follow-up | Superseded by the local #2738 harvest. | The PR carries the same #2692 product direction but its reviewed head still depended on folded-thinking state before collapse could render and omitted MCP status/name handling. The local #2738 harvest already covers common-case collapse, MCP success/tool-name grouping, expansion/cell-map behavior, and `tool_collapse` modes with focused transcript-collapse tests. Credit @idling11; comment/close after the integration branch is public. |
|
||||
| #2734 sidebar detail popovers | Locally harvested as the mouse-hover slice for #2694. | Work/Tasks/Agents hover metadata now stores row hitboxes, compact display text, and full source text so truncated checklist items, task/turn ids, and sub-agent ids/progress expand into a bordered wrapping popover. The harvest fixes reviewer risks from the PR by treating row metadata as authoritative, sizing by display width instead of bytes, and keeping source text untruncated. `cargo test -p codewhale-tui --bin codewhale-tui --locked sidebar_hover -- --nocapture`, `... work_hover_text_preserves_full_checklist_item ...`, and `... subagent_hover_text_preserves_full_agent_id_and_progress ...` passed. Credit @idling11; keep #2694 open for keyboard access, richer Work/Tasks/Agents metadata, redaction expansion, and clipping/snapshot coverage. |
|
||||
| #2532 pending-input delivery-mode labels | Locally re-harvested for #2054. | Pending-input preview rows now label steer-pending, rejected-steer, and queued-follow-up delivery modes, and wrapped continuation rows align under the label. `cargo test -p codewhale-tui --bin codewhale-tui --locked pending_input_preview -- --nocapture` passed. Credit @cyq1017; #2054 remains open for cancel/edit-mode affordance clarity. |
|
||||
| #2532 pending-input delivery-mode labels plus #2054 queued-edit recovery | Locally re-harvested and extended for #2054. | Pending-input preview rows label steer-pending, rejected-steer, queued-follow-up, and editing-queued-follow-up delivery modes. The accidental ↑ edit path is test-covered while loading, and `Esc` restores the original queued follow-up before cancelling the active turn. `cargo test -p codewhale-tui --bin codewhale-tui --locked pending_input_preview -- --nocapture`, `... queued_draft ...`, and `... accidental_queue_edit_while_loading_is_labeled_and_recoverable ...` passed. Credit @cyq1017; leave #2054 open only if row-level edit/drop/send controls are still required beyond the composer recovery fix. |
|
||||
| #2029 sub-agent checkpoint continuation | Locally implemented as the live-timeout recovery slice. | Sub-agents now persist `SubAgentCheckpoint` metadata through state, results, projections, and transcript handles. The runner checkpoints local messages before API calls and after model/tool cycles; per-step API timeout marks the child interrupted with `continuable=true`; `agent_eval { continue: true }` resumes only live checkpointed interrupted children. Reload preserves checkpoint metadata, but cold-restart continuation is intentionally not claimed because the child task/input channel is not rehydrated yet. `cargo test -p codewhale-tui --bin codewhale-tui --locked subagent -- --nocapture`, `cargo fmt --all -- --check`, `git diff --check`, and `cargo clippy -p codewhale-tui --locked -- -D warnings` passed. Credit @qiyuanlicn for the recovery report; keep #2029 open only if cold-restart continuation or broader checkpoint UX remains required. |
|
||||
| #1786 stale running task recovery | Locally implemented as the durable restart-safety slice. | `TaskManager::load_state` now marks tasks that were persisted as `running` in a prior process as failed with an explicit restart/interrupted error instead of requeueing them. Running tool-call summaries inside those stale tasks are also marked failed. `cargo test -p codewhale-tui --bin codewhale-tui --locked running_tasks_are_not_requeued_after_restart -- --nocapture` and `cargo test -p codewhale-tui --bin codewhale-tui --locked task_manager -- --nocapture` passed. Credit @bevis-wong; keep #1786 open for foreground shell hang root cause and careful LIVE-state watchdog work that does not abort legitimate foreground commands. |
|
||||
| #697/#1827 bounded auto-generated project context | Locally implemented from the stabilization audit. | When no project instructions exist, startup now writes `.codewhale/instructions.md` from the bounded Project Context Pack data instead of an unbounded summary/tree scan. The generated file avoids the dynamic `<project_context_pack>` marker when that setting is disabled, keeps later top-level folders visible, and omits noisy directory tails. `cargo test -p codewhale-tui --bin codewhale-tui --locked auto_generated_context_is_bounded_for_many_file_workspace -- --nocapture` and `cargo test -p codewhale-tui --bin codewhale-tui --locked project_context_pack -- --nocapture` passed. Credit reporters @NASLXTO and @wuxixing, plus earlier context-cap/startup work from @linzhiqin2003 and @merchloubna70-dot; leave #697/#1827 open pending real massive-repo/manual startup verification. |
|
||||
@@ -80,7 +80,7 @@ v0.9 branch so the remaining Windows/manual checks are explicit.
|
||||
| Sub-agent timeout and trust model (#1806, #719) | Fixed or covered in current branch. | `heartbeat_timeout_secs` clamp/default test passed, and `agent_open_description_explains_fresh_vs_forked_context_and_trust_model` asserts that sub-agent results are self-reports. |
|
||||
| Sub-agent checkpoint/resume (#2029) | Partially covered. | Live per-step API timeout now preserves a continuable checkpoint and `agent_eval { continue: true }` resumes the parked child; `cargo test -p codewhale-tui --bin codewhale-tui --locked subagent -- --nocapture` passed with checkpoint/projection/persistence/continuation coverage. Cold-restart continuation is not implemented because persisted child tasks are not rehydrated; decide whether #2029 can close as live-timeout recovery or should remain open for restart-resume UX. |
|
||||
| Live shell/session liveness (#1786) | Partially fixed, still release-blocking. | Durable task restart recovery now fails stale persisted `running` tasks instead of requeueing them, covered by `running_tasks_are_not_requeued_after_restart` and broader `task_manager` tests. Foreground shell hang root cause and LIVE-state watchdog recovery remain open; avoid aborting legitimate foreground `exec_shell` commands while adding stale-card recovery. |
|
||||
| Queued/live input feedback (#2054) | Partially covered; UX clarity still blocking. | Queued-message recovery/editing and pending-input delivery-mode labels are covered by `queued` and `pending_input_preview` focused tests. Still needs cancel/edit-mode affordance clarity and a repro for accidentally entering queued-draft edit while a turn is loading. |
|
||||
| Queued/live input feedback (#2054) | Common accidental edit path covered. | Queued-message recovery/editing, pending-input delivery-mode labels, explicit editing-queued-follow-up preview state, and Esc restore semantics are covered by `queued_draft`, `pending_input_preview`, and `accidental_queue_edit_while_loading_is_labeled_and_recoverable` focused tests. Keep open only if v0.9 also requires row-level edit/drop/send controls rather than composer-level recovery. |
|
||||
| Prompt/UI calmness (#1191) | Defer or narrow. | No release-blocking regression evidence yet; keep as polish unless a current user-facing prompt/UI failure is identified. |
|
||||
|
||||
## PR Harvest Queue
|
||||
|
||||
Reference in New Issue
Block a user