merge: panel quit latency
This commit is contained in:
commit
db7bad7a64
|
|
@ -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.
|
||||||
|
|
@ -2,7 +2,7 @@
|
||||||
title: 'Panel Quit 時の断続的な遅延を調査して解消する'
|
title: 'Panel Quit 時の断続的な遅延を調査して解消する'
|
||||||
state: 'inprogress'
|
state: 'inprogress'
|
||||||
created_at: '2026-06-13T10:04:55Z'
|
created_at: '2026-06-13T10:04:55Z'
|
||||||
updated_at: '2026-06-13T11:27:38Z'
|
updated_at: '2026-06-13T11:40:01Z'
|
||||||
assignee: null
|
assignee: null
|
||||||
readiness: 'spike_needed'
|
readiness: 'spike_needed'
|
||||||
risk_flags: ['tui-panel', 'shutdown-latency', 'async-cancellation']
|
risk_flags: ['tui-panel', 'shutdown-latency', 'async-cancellation']
|
||||||
|
|
|
||||||
|
|
@ -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.
|
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.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<!-- event: implementation_report author: hare at: 2026-06-13T11:35:16Z -->
|
||||||
|
|
||||||
|
## 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.
|
||||||
|
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<!-- event: review author: hare at: 2026-06-13T11:40:01Z status: approve -->
|
||||||
|
|
||||||
|
## 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.
|
||||||
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
|
||||||
|
|
@ -126,6 +126,7 @@ pub(crate) async fn run(
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut pending_reload = PendingReload::default();
|
let mut pending_reload = PendingReload::default();
|
||||||
|
let mut pending_queue_attention_notice = PendingQueueAttentionNotice::default();
|
||||||
if let Some(mode) = app.enter_reload.take() {
|
if let Some(mode) = app.enter_reload.take() {
|
||||||
if pending_reload.start(mode) {
|
if pending_reload.start(mode) {
|
||||||
app.refreshing = true;
|
app.refreshing = true;
|
||||||
|
|
@ -134,11 +135,13 @@ pub(crate) async fn run(
|
||||||
let mut next_poll = Instant::now() + MULTI_POD_POLL_INTERVAL;
|
let mut next_poll = Instant::now() + MULTI_POD_POLL_INTERVAL;
|
||||||
|
|
||||||
loop {
|
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 {
|
if let Some(result) = pending_reload.finish_if_ready().await {
|
||||||
app.apply_reload_result(result);
|
app.apply_reload_result(result);
|
||||||
if let Some(request) = app.prepare_orchestrator_queue_attention_notice() {
|
if let Some(request) = app.prepare_orchestrator_queue_attention_notice() {
|
||||||
let result = dispatch_orchestrator_queue_attention_notice(request).await;
|
pending_queue_attention_notice.start(request);
|
||||||
app.finish_orchestrator_queue_attention_notice(result);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -159,7 +162,13 @@ pub(crate) async fn run(
|
||||||
match read()? {
|
match read()? {
|
||||||
TermEvent::Key(key) => match app.handle_key(key) {
|
TermEvent::Key(key) => match app.handle_key(key) {
|
||||||
MultiPodAction::None => {}
|
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 => {
|
MultiPodAction::Open => {
|
||||||
if let Some(request) = app.prepare_open() {
|
if let Some(request) = app.prepare_open() {
|
||||||
terminal.draw(|f| draw(f, app))?;
|
terminal.draw(|f| draw(f, app))?;
|
||||||
|
|
@ -168,6 +177,7 @@ pub(crate) async fn run(
|
||||||
}
|
}
|
||||||
MultiPodAction::DispatchTicketAction(request) => {
|
MultiPodAction::DispatchTicketAction(request) => {
|
||||||
pending_reload.abort();
|
pending_reload.abort();
|
||||||
|
pending_queue_attention_notice.abort();
|
||||||
terminal.draw(|f| draw(f, app))?;
|
terminal.draw(|f| draw(f, app))?;
|
||||||
let result = dispatch_ticket_action(request).await;
|
let result = dispatch_ticket_action(request).await;
|
||||||
app.finish_ticket_action_dispatch(result);
|
app.finish_ticket_action_dispatch(result);
|
||||||
|
|
@ -178,6 +188,7 @@ pub(crate) async fn run(
|
||||||
}
|
}
|
||||||
MultiPodAction::LaunchIntake(request) => {
|
MultiPodAction::LaunchIntake(request) => {
|
||||||
pending_reload.abort();
|
pending_reload.abort();
|
||||||
|
pending_queue_attention_notice.abort();
|
||||||
terminal.draw(|f| draw(f, app))?;
|
terminal.draw(|f| draw(f, app))?;
|
||||||
let result = launch_intake_with_handoff(request).await;
|
let result = launch_intake_with_handoff(request).await;
|
||||||
app.finish_intake_launch(result);
|
app.finish_intake_launch(result);
|
||||||
|
|
@ -188,6 +199,7 @@ pub(crate) async fn run(
|
||||||
}
|
}
|
||||||
MultiPodAction::SendCompanion(request) => {
|
MultiPodAction::SendCompanion(request) => {
|
||||||
pending_reload.abort();
|
pending_reload.abort();
|
||||||
|
pending_queue_attention_notice.abort();
|
||||||
terminal.draw(|f| draw(f, app))?;
|
terminal.draw(|f| draw(f, app))?;
|
||||||
let result = dispatch_companion_message(request).await;
|
let result = dispatch_companion_message(request).await;
|
||||||
app.finish_companion_send(result);
|
app.finish_companion_send(result);
|
||||||
|
|
@ -267,6 +279,75 @@ impl Drop for PendingReload {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
struct PendingQueueAttentionNotice {
|
||||||
|
handle: Option<tokio::task::JoinHandle<OrchestratorQueueAttentionNoticeResult>>,
|
||||||
|
}
|
||||||
|
|
||||||
|
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<OrchestratorQueueAttentionNoticeResult>,
|
||||||
|
) -> bool {
|
||||||
|
if self.handle.is_some() {
|
||||||
|
handle.abort();
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
self.handle = Some(handle);
|
||||||
|
true
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn finish_if_ready(&mut self) -> Option<OrchestratorQueueAttentionNoticeResult> {
|
||||||
|
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<PathBuf, MultiPodError> {
|
fn default_store_dir() -> Result<PathBuf, MultiPodError> {
|
||||||
manifest::paths::sessions_dir().ok_or_else(|| {
|
manifest::paths::sessions_dir().ok_or_else(|| {
|
||||||
MultiPodError::Io(io::Error::new(
|
MultiPodError::Io(io::Error::new(
|
||||||
|
|
@ -4875,6 +4956,10 @@ fn truncate_with_ellipsis(s: &str, max_width: usize) -> String {
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
use std::sync::{
|
||||||
|
Arc,
|
||||||
|
atomic::{AtomicBool, Ordering},
|
||||||
|
};
|
||||||
#[test]
|
#[test]
|
||||||
fn orchestration_worktree_layout_is_stable_under_original_workspace_root() {
|
fn orchestration_worktree_layout_is_stable_under_original_workspace_root() {
|
||||||
let root = Path::new("/tmp/Yoi Workspace");
|
let root = Path::new("/tmp/Yoi Workspace");
|
||||||
|
|
@ -6074,6 +6159,65 @@ mod tests {
|
||||||
assert!(app.notice.as_deref().unwrap().contains("Refresh failed"));
|
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<AtomicBool>);
|
||||||
|
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]
|
#[test]
|
||||||
fn multi_idle_live_selected_target_is_open_eligible() {
|
fn multi_idle_live_selected_target_is_open_eligible() {
|
||||||
let app = test_app(vec![live_info("idle", PodStatus::Idle)]);
|
let app = test_app(vec![live_info("idle", PodStatus::Idle)]);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user