diff --git a/.yoi/tickets/00001KV0723PC/artifacts/implementation_report.md b/.yoi/tickets/00001KV0723PC/artifacts/implementation_report.md new file mode 100644 index 00000000..c266e518 --- /dev/null +++ b/.yoi/tickets/00001KV0723PC/artifacts/implementation_report.md @@ -0,0 +1,33 @@ +Implementation report for Ticket 00001KV0723PC + +Commit: cfe411e50d7361228e509a18699477b13c4bc3e7 (`fix: avoid panel quit notice wait`) + +Observed / identified delay cause: +- In `crates/tui/src/multi_pod.rs`, the workspace Panel run loop handled a completed `PendingReload` before polling terminal input. +- When the reload result requested Orchestrator queue-attention notification, the loop awaited `dispatch_orchestrator_queue_attention_notice(request)` inline. +- That dispatch path can wait on Unix socket connect/read/write operations and their timeouts. While it was awaited inline, queued terminal events such as `Ctrl+C` / `Ctrl+D` could not be observed, so an explicit Quit could be delayed by non-essential notice dispatch work. + +Fix summary: +- Added `PendingQueueAttentionNotice`, a cancellable background task wrapper for queue-attention notice dispatch. +- Changed the Panel run loop to start queue-attention notice dispatch asynchronously after reload and only harvest the result when the task is already finished. +- Added explicit quit cleanup through `abort_panel_background_work_for_quit`, aborting both pending reload and pending queue-attention notice work before returning `MultiPodOutcome::Quit`. +- Also abort the background notice before foreground open/action paths that already abort reload work, so non-essential notice dispatch does not contend with user-directed operations. +- Preserved terminal/backend shutdown behavior by keeping the existing return path and relying on explicit abort plus Rust `Drop` cleanup for background tasks. + +Files changed: +- `crates/tui/src/multi_pod.rs` + +Regression coverage: +- Added `multi_quit_aborts_background_reload_and_notice_without_waiting`, which starts never-completing reload and notice tasks, exercises the quit abort helper under a timeout, and verifies both tasks are cancelled. +- Existing reload overlap behavior remains covered by `multi_poll_reload_does_not_overlap_in_flight_reload`. + +Validation commands/results: +- `cargo fmt --check` — passed +- `git diff --check` — passed +- `cargo test -p tui multi_quit_aborts_background_reload_and_notice_without_waiting --lib` — passed +- `cargo test -p tui multi_poll_reload_does_not_overlap_in_flight_reload --lib` — passed +- `cargo check -p tui --all-targets` — passed + +Residual risks / manual validation notes: +- No broad runtime-loop rewrite was done. Foreground user-directed operations that are already awaited by the Panel can still occupy the loop; this change targets the identified non-essential background notice dispatch delay path. +- I did not run an interactive manual `yoi panel` session from this Coder environment. Recommended manual check: start `yoi panel`, trigger a reload/queued Ticket condition that would dispatch queue-attention notice, then press `Ctrl+C`/`Ctrl+D` while the notice target is slow or unavailable; the Panel should exit promptly while terminal state is restored. diff --git a/.yoi/tickets/00001KV0723PC/item.md b/.yoi/tickets/00001KV0723PC/item.md index 8a6f8900..d2b6e011 100644 --- a/.yoi/tickets/00001KV0723PC/item.md +++ b/.yoi/tickets/00001KV0723PC/item.md @@ -2,7 +2,7 @@ title: 'Panel Quit 時の断続的な遅延を調査して解消する' state: 'inprogress' created_at: '2026-06-13T10:04:55Z' -updated_at: '2026-06-13T11:27:38Z' +updated_at: '2026-06-13T11:40:01Z' assignee: null readiness: 'spike_needed' risk_flags: ['tui-panel', 'shutdown-latency', 'async-cancellation'] diff --git a/.yoi/tickets/00001KV0723PC/thread.md b/.yoi/tickets/00001KV0723PC/thread.md index 2fcb07c4..c91ec5de 100644 --- a/.yoi/tickets/00001KV0723PC/thread.md +++ b/.yoi/tickets/00001KV0723PC/thread.md @@ -152,4 +152,73 @@ Critical risks / reviewer focus: Ticket evidence、relation records、existing OrchestrationPlan waiting/conflict notes、related completed Panel mouse integration、visible Pod/worktree state、and clean Orchestrator workspace were checked. The prior conflict/capacity blocker is resolved; no durable dependency blockers remain. Implementation can proceed in a dedicated child worktree with a first-step code-path spike and focused regression test before changing Quit behavior. +--- + + + +## Implementation report + +Implementation report for Ticket 00001KV0723PC + +Commit: cfe411e50d7361228e509a18699477b13c4bc3e7 (`fix: avoid panel quit notice wait`) + +Observed / identified delay cause: +- In `crates/tui/src/multi_pod.rs`, the workspace Panel run loop handled a completed `PendingReload` before polling terminal input. +- When the reload result requested Orchestrator queue-attention notification, the loop awaited `dispatch_orchestrator_queue_attention_notice(request)` inline. +- That dispatch path can wait on Unix socket connect/read/write operations and their timeouts. While it was awaited inline, queued terminal events such as `Ctrl+C` / `Ctrl+D` could not be observed, so an explicit Quit could be delayed by non-essential notice dispatch work. + +Fix summary: +- Added `PendingQueueAttentionNotice`, a cancellable background task wrapper for queue-attention notice dispatch. +- Changed the Panel run loop to start queue-attention notice dispatch asynchronously after reload and only harvest the result when the task is already finished. +- Added explicit quit cleanup through `abort_panel_background_work_for_quit`, aborting both pending reload and pending queue-attention notice work before returning `MultiPodOutcome::Quit`. +- Also abort the background notice before foreground open/action paths that already abort reload work, so non-essential notice dispatch does not contend with user-directed operations. +- Preserved terminal/backend shutdown behavior by keeping the existing return path and relying on explicit abort plus Rust `Drop` cleanup for background tasks. + +Files changed: +- `crates/tui/src/multi_pod.rs` + +Regression coverage: +- Added `multi_quit_aborts_background_reload_and_notice_without_waiting`, which starts never-completing reload and notice tasks, exercises the quit abort helper under a timeout, and verifies both tasks are cancelled. +- Existing reload overlap behavior remains covered by `multi_poll_reload_does_not_overlap_in_flight_reload`. + +Validation commands/results: +- `cargo fmt --check` — passed +- `git diff --check` — passed +- `cargo test -p tui multi_quit_aborts_background_reload_and_notice_without_waiting --lib` — passed +- `cargo test -p tui multi_poll_reload_does_not_overlap_in_flight_reload --lib` — passed +- `cargo check -p tui --all-targets` — passed + +Residual risks / manual validation notes: +- No broad runtime-loop rewrite was done. Foreground user-directed operations that are already awaited by the Panel can still occupy the loop; this change targets the identified non-essential background notice dispatch delay path. +- I did not run an interactive manual `yoi panel` session from this Coder environment. Recommended manual check: start `yoi panel`, trigger a reload/queued Ticket condition that would dispatch queue-attention notice, then press `Ctrl+C`/`Ctrl+D` while the notice target is slow or unavailable; the Panel should exit promptly while terminal state is restored. + + +--- + + + +## Review: approve + +Review: approve + +Evidence: +- Reviewed Ticket intent/criteria and `git diff 9dacc90e..HEAD` for commits `cfe411e5` and `8de82836`. +- The reported delay cause is plausible in the previous loop shape: a completed reload was processed before input polling and could inline-await queue-attention notice dispatch; that dispatch uses Pod socket connect/read/write paths, so queued `Ctrl+C` / `Ctrl+D` input could wait behind nonessential notice work. +- The implementation changes only `crates/tui/src/multi_pod.rs` plus the implementation report. Queue-attention notice dispatch is now wrapped in a cancellable `PendingQueueAttentionNotice`, started in the background, and harvested only after completion. Reload dispatch is guarded against overlap. +- Quit handling now calls `abort_panel_background_work_for_quit`, aborting pending reload and pending queue-attention notice tasks before returning the existing `MultiPodOutcome::Quit`; this keeps the established terminal/backend cleanup return path intact. +- Existing foreground open/action helper paths also abort the nonessential pending notice before returning user-directed outcomes, so stale notice dispatch does not block those paths. +- No Ticket lifecycle authority, Pod authority boundary, Panel row/action semantics, composer target semantics, prompt resources, or Ticket schema changes were introduced. +- The regression test starts never-completing reload and notice futures, exercises the quit abort helper under a timeout, and verifies cancellation of both tasks. This is a meaningful regression for the pending-background-work latency path rather than only checking that a helper was wired. + +Validation performed: +- `cargo test -p tui multi_quit_aborts_background_reload_and_notice_without_waiting --lib` — passed. +- `cargo test -p tui multi_poll_reload_does_not_overlap_in_flight_reload --lib` — passed. +- `cargo fmt --check` — passed. +- `git diff --check` — passed. +- `cargo check -p tui --all-targets` — passed. + +Risks / notes: +- I did not run an interactive `yoi panel` manual latency check. The code-path review and focused tests cover the identified nonessential reload/notice wait path. + + --- diff --git a/crates/tui/src/multi_pod.rs b/crates/tui/src/multi_pod.rs index af525b67..8da33598 100644 --- a/crates/tui/src/multi_pod.rs +++ b/crates/tui/src/multi_pod.rs @@ -126,6 +126,7 @@ pub(crate) async fn run( } let mut pending_reload = PendingReload::default(); + let mut pending_queue_attention_notice = PendingQueueAttentionNotice::default(); if let Some(mode) = app.enter_reload.take() { if pending_reload.start(mode) { app.refreshing = true; @@ -134,11 +135,13 @@ pub(crate) async fn run( let mut next_poll = Instant::now() + MULTI_POD_POLL_INTERVAL; loop { + if let Some(result) = pending_queue_attention_notice.finish_if_ready().await { + app.finish_orchestrator_queue_attention_notice(result); + } if let Some(result) = pending_reload.finish_if_ready().await { app.apply_reload_result(result); if let Some(request) = app.prepare_orchestrator_queue_attention_notice() { - let result = dispatch_orchestrator_queue_attention_notice(request).await; - app.finish_orchestrator_queue_attention_notice(result); + pending_queue_attention_notice.start(request); } } @@ -159,7 +162,13 @@ pub(crate) async fn run( match read()? { TermEvent::Key(key) => match app.handle_key(key) { MultiPodAction::None => {} - MultiPodAction::Quit => return Ok(MultiPodOutcome::Quit), + MultiPodAction::Quit => { + abort_panel_background_work_for_quit( + &mut pending_reload, + &mut pending_queue_attention_notice, + ); + return Ok(MultiPodOutcome::Quit); + } MultiPodAction::Open => { if let Some(request) = app.prepare_open() { terminal.draw(|f| draw(f, app))?; @@ -168,6 +177,7 @@ pub(crate) async fn run( } MultiPodAction::DispatchTicketAction(request) => { pending_reload.abort(); + pending_queue_attention_notice.abort(); terminal.draw(|f| draw(f, app))?; let result = dispatch_ticket_action(request).await; app.finish_ticket_action_dispatch(result); @@ -178,6 +188,7 @@ pub(crate) async fn run( } MultiPodAction::LaunchIntake(request) => { pending_reload.abort(); + pending_queue_attention_notice.abort(); terminal.draw(|f| draw(f, app))?; let result = launch_intake_with_handoff(request).await; app.finish_intake_launch(result); @@ -188,6 +199,7 @@ pub(crate) async fn run( } MultiPodAction::SendCompanion(request) => { pending_reload.abort(); + pending_queue_attention_notice.abort(); terminal.draw(|f| draw(f, app))?; let result = dispatch_companion_message(request).await; app.finish_companion_send(result); @@ -267,6 +279,75 @@ impl Drop for PendingReload { } } +struct PendingQueueAttentionNotice { + handle: Option>, +} + +impl PendingQueueAttentionNotice { + fn start(&mut self, request: OrchestratorQueueAttentionNoticeRequest) -> bool { + if self.handle.is_some() { + return false; + } + self.handle = Some(tokio::spawn(async move { + dispatch_orchestrator_queue_attention_notice(request).await + })); + true + } + + #[cfg(test)] + fn start_with_handle( + &mut self, + handle: tokio::task::JoinHandle, + ) -> bool { + if self.handle.is_some() { + handle.abort(); + return false; + } + self.handle = Some(handle); + true + } + + async fn finish_if_ready(&mut self) -> Option { + if !self.handle.as_ref()?.is_finished() { + return None; + } + let handle = self.handle.take()?; + Some(match handle.await { + Ok(result) => result, + Err(e) => OrchestratorQueueAttentionNoticeResult::failed( + String::new(), + format!("queue-attention notice task failed: {e}"), + ), + }) + } + + fn abort(&mut self) { + if let Some(handle) = self.handle.take() { + handle.abort(); + } + } +} + +impl Default for PendingQueueAttentionNotice { + fn default() -> Self { + Self { handle: None } + } +} + +impl Drop for PendingQueueAttentionNotice { + fn drop(&mut self) { + self.abort(); + } +} + +fn abort_panel_background_work_for_quit( + pending_reload: &mut PendingReload, + pending_queue_attention_notice: &mut PendingQueueAttentionNotice, +) { + pending_reload.abort(); + pending_queue_attention_notice.abort(); +} + fn default_store_dir() -> Result { manifest::paths::sessions_dir().ok_or_else(|| { MultiPodError::Io(io::Error::new( @@ -4875,6 +4956,10 @@ fn truncate_with_ellipsis(s: &str, max_width: usize) -> String { #[cfg(test)] mod tests { use super::*; + use std::sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + }; #[test] fn orchestration_worktree_layout_is_stable_under_original_workspace_root() { let root = Path::new("/tmp/Yoi Workspace"); @@ -6074,6 +6159,65 @@ mod tests { assert!(app.notice.as_deref().unwrap().contains("Refresh failed")); } + #[tokio::test] + async fn multi_quit_aborts_background_reload_and_notice_without_waiting() { + struct DropFlag(Arc); + impl Drop for DropFlag { + fn drop(&mut self) { + self.0.store(true, Ordering::SeqCst); + } + } + + let reload_cancelled = Arc::new(AtomicBool::new(false)); + let notice_cancelled = Arc::new(AtomicBool::new(false)); + let mut pending_reload = PendingReload::default(); + let mut pending_notice = PendingQueueAttentionNotice::default(); + let (reload_started_tx, reload_started_rx) = tokio::sync::oneshot::channel(); + let (notice_started_tx, notice_started_rx) = tokio::sync::oneshot::channel(); + + let reload_flag = Arc::clone(&reload_cancelled); + assert!(pending_reload.start_with_handle(tokio::spawn(async move { + let _drop_flag = DropFlag(reload_flag); + let _ = reload_started_tx.send(()); + std::future::pending::<()>().await; + Err(MultiPodError::Io(io::Error::other( + "unreachable reload completion", + ))) + }))); + + let notice_flag = Arc::clone(¬ice_cancelled); + assert!(pending_notice.start_with_handle(tokio::spawn(async move { + let _drop_flag = DropFlag(notice_flag); + let _ = notice_started_tx.send(()); + std::future::pending::<()>().await; + OrchestratorQueueAttentionNoticeResult::failed( + "unreachable".to_string(), + "unreachable notice completion", + ) + }))); + reload_started_rx.await.expect("reload task should start"); + notice_started_rx.await.expect("notice task should start"); + + tokio::time::timeout(Duration::from_millis(20), async { + abort_panel_background_work_for_quit(&mut pending_reload, &mut pending_notice); + }) + .await + .expect("quit abort should not wait for background task completion"); + + tokio::time::timeout(Duration::from_millis(100), async { + while !(reload_cancelled.load(Ordering::SeqCst) + && notice_cancelled.load(Ordering::SeqCst)) + { + tokio::task::yield_now().await; + } + }) + .await + .expect("quit abort should cancel reload and notice tasks"); + + assert!(pending_reload.finish_if_ready().await.is_none()); + assert!(pending_notice.finish_if_ready().await.is_none()); + } + #[test] fn multi_idle_live_selected_target_is_open_eligible() { let app = test_app(vec![live_info("idle", PodStatus::Idle)]);