From b5526fba3f74d7e2825ef87501e69e0f57868c00 Mon Sep 17 00:00:00 2001 From: Bilal Arshad <111754659+Oreoro@users.noreply.github.com> Date: Sun, 4 Oct 2026 04:46:18 +0500 Subject: [PATCH 1/4] Make the rail's view of terminal-backed sessions truthful Three ways the rail lied about the sessions it was supervising, all of which present as "integrating terminal threads in the sidebar doesn't work". A closed session stayed in the rail. `observe` fires on `notify()`; opening and closing a session is an `emit` (`ThreadOpened` / `ThreadClosed`), and the store does not also notify. The Agent Threads panel subscribes and repaints on both; the rail only observed, so the two surfaces this fork documents as unable to disagree disagreed on exactly add and remove. The rail now subscribes as well as observing, so a closed session's row leaves instead of lingering as something the user can click into nothing. A dead session kept its last status dot. Agent terminals are spawned with `HideStrategy::Never`, so an exiting agent leaves its tab, its `TerminalView`, and its store entry alive -- and nothing observed `ProcessExited`, so the stored classification from while the agent was alive (usually a green `Running`) stood indefinitely. Agent threads now reclassify on exit. Plain shells already re-derive their status from the screen tail on every read, but nothing woke them to do it, so the registry's subscription now observes the exit too. A new `AttentionTrigger::ProcessExited` exists because the existing triggers both leave an inconclusive classification alone, which is right for mid-turn output and wrong for a process that no longer exists: there, "not working" is the only possible answer. A row that could not be focused did so silently. `focus_thread` fails with "agent thread no longer exists" / "pane closed" precisely when a row holds a stale `terminal_item_id`, which is the case worth diagnosing, and `let _ =` made that click indistinguishable from one the user simply missed. Both call sites now log. No state-machine change: `reclassify_attention` already owned the `Working`/`Blocked`/`Idle` decision, so the exit case reuses it rather than adding a parallel path. --- FORK.md | 1 + SYNC-LEDGER.md | 1 + crates/agent_threads/src/store.rs | 20 ++++++++++++++++++++ crates/agent_threads/src/terminal_control.rs | 5 +++++ crates/dez_sidebar/Cargo.toml | 1 + crates/dez_sidebar/src/sidebar.rs | 18 ++++++++++++++++-- 6 files changed, 44 insertions(+), 2 deletions(-) diff --git a/FORK.md b/FORK.md index 8b17f08f79..97accce62f 100644 --- a/FORK.md +++ b/FORK.md @@ -107,6 +107,7 @@ older API surface. | `RegularTerminalSummary::title`, `has_registry`, `observe_activity` | `crates/agent_threads/src/terminal_control.rs` | The terminal's display title (OSC title, or the foreground process the terminal itself derives when none was set) is what makes a shell row legible; the model hands over a never-empty title and the rail owns nothing but rendering. Additive, and the summary is no longer `Copy` (it now holds a `SharedString`) | | `dez_sidebar` settings section (`starts_open`) | `crates/settings_content/src/dez_sidebar.rs`, `assets/settings/default.json` | Configurable default-open behavior | | Sidebar registration + default-open on window creation | `crates/dez/src/dez.rs` | Wires the shell into the app; deferred open avoids acting mid-construction | +| The rail subscribes to `AgentThreadStoreEvent` and to `terminal::Event::ProcessExited`, not just observing the store | `crates/dez_sidebar/src/sidebar.rs`, `crates/agent_threads/src/{store.rs,terminal_control.rs}` | `observe` only fires on `notify()`; opening and closing a session is an *emit*. An observer-only rail keeps a dead session's row, and an agent terminal exits without any output event while its tab survives (`HideStrategy::Never`), so the rail must subscribe to the exit explicitly or a `Running` dot outlives the process | | Rail status is a marker (dot/glyph) plus a label only for `Needs input` / `Finished`; titles use the default text color | `crates/dez_sidebar/src/sidebar.rs` | A busy workspace should read as a list, not a traffic light; `Running`/`Idle` are the calm default | | Group header aggregates every root (branch from the first, summed changed count, worst indicator) and is clickable; single-root groups skip the per-root row | `crates/dez_sidebar/src/sidebar.rs` | Multi-root headers must describe the group, and a single-root header already names its root | | `terminal_control::classify_record` maps an unmatched plain terminal to `Idle` instead of dropping it | `crates/agent_threads/src/terminal_control.rs` | A shell at an ordinary prompt matches no agent manifest, but the rail promises to list plain shells | diff --git a/SYNC-LEDGER.md b/SYNC-LEDGER.md index cd174e3ec6..cbb47c0d19 100644 --- a/SYNC-LEDGER.md +++ b/SYNC-LEDGER.md @@ -18,3 +18,4 @@ One line per upstream adaptation. Format: - 2026-09-28 | dez v0 sidebar (rail polish) | Rail presentation pass: status is a colored marker plus a label only for `Needs input` / `Finished`, titles use the default text color, rows share one height and one hover/selection treatment. Group headers aggregate every root (branch, summed changed count, worst indicator) and are clickable; a single-root group no longer repeats its header as a per-root row. Plain terminals that match no agent manifest classify as `Idle` rather than being dropped, so an ordinary prompt is listed. README's release section now points at the nightly artifacts while no stable tag exists. | crates/dez_sidebar/src/sidebar.rs, crates/agent_threads/src/terminal_control.rs, docs/dez-workspace-shell.md, FORK.md, README.md - 2026-09-28 | dez v0 sidebar (collapsible groups) | Rail group headers gain a disclosure chevron that folds the group's rows, driven by `MultiWorkspace`'s existing persisted group state via a new small `toggle_project_group_expanded` method; a collapsed group still shows its attention rollup. | crates/dez_sidebar/src/sidebar.rs, crates/workspace/src/multi_workspace.rs, FORK.md - 2026-09-28 | dez v0 sidebar (focus follows workspace) | Focusing a session from the rail now brings its workspace forward before focusing the pane: `AgentThreadStore::focus_thread` and `terminal_control::focus_terminal` activate the owning workspace, so clicking a session that lives in a background workspace actually shows it instead of looking like a no-op. Matches the Agent Threads panel's cross-project focus. | crates/agent_threads/src/{store.rs,terminal_control.rs}, FORK.md +- 2026-10-04 | dez v0 sidebar (truthful terminal threads) | The rail now hears sessions open and close (`AgentThreadStoreEvent` is an emit, not a notify, so the rail's `observe` alone left rows behind after a session ended) and repaints when a terminal's process exits, so a dead agent session stops showing the status dot it had while alive. Rail clicks log `focus_thread` / `focus_priority_terminal` failures instead of discarding them, since a stale `terminal_item_id` made a row look inert with no diagnostic. | crates/dez_sidebar/src/sidebar.rs, crates/agent_threads/src/{store.rs,terminal_control.rs} diff --git a/crates/agent_threads/src/store.rs b/crates/agent_threads/src/store.rs index 25b89f5595..3b70e0876b 100644 --- a/crates/agent_threads/src/store.rs +++ b/crates/agent_threads/src/store.rs @@ -545,6 +545,12 @@ enum AttentionTrigger { /// classification here is unremarkable mid-turn output, not a signal, /// so it leaves the thread's attention state exactly as it was. Wakeup, + /// The agent's process exited. Nothing about the screen tail can say what + /// the agent was doing when it died, so an inconclusive classification here + /// falls back to `ThreadAttention::Idle`: the session is definitionally no + /// longer working, and leaving the last live classification in place is how + /// a dead session keeps a "Running" dot forever. + ProcessExited, } /// Aggregate live-thread status for one worktree, for the cross-project @@ -1372,6 +1378,19 @@ impl AgentThreadStore { terminal::Event::Wakeup => { store.schedule_wakeup_reclassify(terminal_item_id, cx); } + // Not a wakeup: there is no more output coming, so reclassify + // now rather than behind the output debounce. Agent terminals + // are spawned with `HideStrategy::Never`, so an exiting agent + // leaves its tab -- and this store entry -- alive, and without + // this the rail would keep showing the classification the + // session had while it was still alive. + terminal::Event::ProcessExited { .. } => { + store.reclassify_attention( + terminal_item_id, + AttentionTrigger::ProcessExited, + cx, + ); + } _ => {} }, ); @@ -1445,6 +1464,7 @@ impl AgentThreadStore { crate::attention_detection::AttentionState::Unknown => match trigger { AttentionTrigger::Bell => Some(ThreadAttention::Blocked), AttentionTrigger::Wakeup => return, + AttentionTrigger::ProcessExited => Some(ThreadAttention::Idle), }, }; diff --git a/crates/agent_threads/src/terminal_control.rs b/crates/agent_threads/src/terminal_control.rs index 4b8272d560..216e91e346 100644 --- a/crates/agent_threads/src/terminal_control.rs +++ b/crates/agent_threads/src/terminal_control.rs @@ -225,10 +225,15 @@ pub(crate) fn init(cx: &mut App) { ); if let Some(terminal) = terminal.upgrade() { let subscription = cx.subscribe(&terminal, |_registry, _, event, cx| { + // `ProcessExited` matters as much as output: a shell's + // status is re-derived from its screen tail on every + // read, so a process exiting changes the answer without + // necessarily producing any further output. if matches!( event, terminal::Event::Wakeup | terminal::Event::Bell + | terminal::Event::ProcessExited { .. } | terminal::Event::CloseTerminal ) { cx.notify(); diff --git a/crates/dez_sidebar/Cargo.toml b/crates/dez_sidebar/Cargo.toml index 0dd928e1c9..115588f951 100644 --- a/crates/dez_sidebar/Cargo.toml +++ b/crates/dez_sidebar/Cargo.toml @@ -20,6 +20,7 @@ serde.workspace = true serde_json.workspace = true settings.workspace = true ui.workspace = true +util.workspace = true workspace.workspace = true [dev-dependencies] diff --git a/crates/dez_sidebar/src/sidebar.rs b/crates/dez_sidebar/src/sidebar.rs index 4c9423d10c..b4d67ffcde 100644 --- a/crates/dez_sidebar/src/sidebar.rs +++ b/crates/dez_sidebar/src/sidebar.rs @@ -7,6 +7,7 @@ use gpui::{ }; use serde::{Deserialize, Serialize}; use ui::{Disclosure, IconButton, IconButtonShape, Indicator, Tooltip, prelude::*}; +use util::ResultExt as _; use workspace::{ MultiWorkspace, MultiWorkspaceEvent, ProjectGroup, ProjectGroupKey, Sidebar, SidebarEvent, SidebarSide, @@ -215,6 +216,15 @@ impl DezSidebar { // changes that aren't emitted events, and the rail wants all of // them rather than just the ones the panel happens to care about. subscriptions.push(cx.observe(&store, |_this, _store, cx| cx.notify())); + // `observe` covers `notify()` only. Opening and closing a session + // are *emits* (`ThreadOpened` / `ThreadClosed`), and the store does + // not also notify, so without this the rail keeps showing a row for + // a session that has been closed -- and a row the user can click + // into nothing -- until some unrelated change repaints it. + subscriptions.push(cx.subscribe( + &store, + |_this, _store, _event: &agent_threads::AgentThreadStoreEvent, cx| cx.notify(), + )); } // Plain shells are classified live from their own screen tail, so the // rail has to repaint on terminal activity too, not only on store @@ -361,8 +371,12 @@ impl DezSidebar { let Some(store) = agent_threads::AgentThreadStore::try_global(cx) else { return; }; + // Not discarded: these are the failures that make a rail row look + // inert. A stale `terminal_item_id` (the session's pane was closed and + // the row has not repainted yet) surfaces here, and swallowing it makes + // a click indistinguishable from a click that was ignored. store.update(cx, |store, cx| { - let _ = store.focus_thread(terminal_item_id, window, cx); + store.focus_thread(terminal_item_id, window, cx).log_err(); }); cx.notify(); } @@ -758,7 +772,7 @@ impl DezSidebar { let terminal_item_id = terminal.terminal_item_id; let tooltip: SharedString = format!("Terminal ยท {}", status.state_label()).into(); let on_click = cx.listener(move |_this, _, window, cx| { - let _ = agent_threads::focus_priority_terminal(terminal_item_id, window, cx); + agent_threads::focus_priority_terminal(terminal_item_id, window, cx).log_err(); }); self.render_session_row( From ad24ff07c7e156114ac199d97811707bbdadb1d0 Mon Sep 17 00:00:00 2001 From: Bilal Arshad <111754659+Oreoro@users.noreply.github.com> Date: Sun, 4 Oct 2026 05:24:32 +0500 Subject: [PATCH 2/4] Settle an exited session without re-notifying Routing ProcessExited through reclassify_attention was wrong twice over: the trigger variant existed only to force an inconclusive classification to Idle, and the function's tail raises a desktop notification whenever a thread lands in a non-Working state. A thread already flagged as blocked and then killed therefore notified twice -- once for the bell that flagged it, once for the exit that ended it. Caught by panel::tests::bell_does_not_request_attention_for_the_active_window, whose echo agent exits during the test. The exit is now its own transition. It settles the thread to Idle and emits ThreadUpdated, so the rail and the rollup reflect the change, but raises no notification: a process exiting is not a fresh "look at me" signal, and the user has already been told if the thread was blocked. --- crates/agent_threads/src/store.rs | 40 +++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/crates/agent_threads/src/store.rs b/crates/agent_threads/src/store.rs index 3b70e0876b..563279dd1f 100644 --- a/crates/agent_threads/src/store.rs +++ b/crates/agent_threads/src/store.rs @@ -545,12 +545,6 @@ enum AttentionTrigger { /// classification here is unremarkable mid-turn output, not a signal, /// so it leaves the thread's attention state exactly as it was. Wakeup, - /// The agent's process exited. Nothing about the screen tail can say what - /// the agent was doing when it died, so an inconclusive classification here - /// falls back to `ThreadAttention::Idle`: the session is definitionally no - /// longer working, and leaving the last live classification in place is how - /// a dead session keeps a "Running" dot forever. - ProcessExited, } /// Aggregate live-thread status for one worktree, for the cross-project @@ -1385,11 +1379,7 @@ impl AgentThreadStore { // this the rail would keep showing the classification the // session had while it was still alive. terminal::Event::ProcessExited { .. } => { - store.reclassify_attention( - terminal_item_id, - AttentionTrigger::ProcessExited, - cx, - ); + store.mark_exited(terminal_item_id, cx); } _ => {} }, @@ -1464,7 +1454,6 @@ impl AgentThreadStore { crate::attention_detection::AttentionState::Unknown => match trigger { AttentionTrigger::Bell => Some(ThreadAttention::Blocked), AttentionTrigger::Wakeup => return, - AttentionTrigger::ProcessExited => Some(ThreadAttention::Idle), }, }; @@ -1510,6 +1499,33 @@ impl AgentThreadStore { ); } + /// Settles a thread whose process has exited, so a session that is no longer + /// running stops presenting the classification it had while it was alive -- + /// typically a green "Running" dot that outlives the process, because + /// agent terminals are spawned with `HideStrategy::Never` and so leave their + /// tab, their `TerminalView`, and this store entry behind. + /// + /// Deliberately *not* `reclassify_attention`: a desktop notification is + /// correct when an agent rings the bell mid-turn, but the process exiting is + /// not a fresh "look at me" signal, and notifying here would double-notify + /// a thread that had already been flagged as blocked. The rail and the + /// rollup pick the change up through the emitted event instead. + fn mark_exited(&mut self, terminal_item_id: EntityId, cx: &mut Context) { + let Some(entry) = self.threads.get_mut(&terminal_item_id) else { + return; + }; + if entry.attention == Some(ThreadAttention::Idle) { + return; + } + entry.attention = Some(ThreadAttention::Idle); + // A fresh completion, so the rail reports it as finished-and-unseen + // rather than already-looked-at. See `ThreadEntry::finished_seen`. + entry.finished_seen = false; + let kind_id = entry.metadata.kind_id; + cx.emit(AgentThreadStoreEvent::ThreadUpdated { kind_id }); + cx.notify(); + } + fn begin_shutdown( &mut self, terminal_item_id: EntityId, From 3a2c7e70e269af40fcc4487120bf71191e22b3b4 Mon Sep 17 00:00:00 2001 From: Bilal Arshad <111754659+Oreoro@users.noreply.github.com> Date: Sun, 4 Oct 2026 05:24:41 +0500 Subject: [PATCH 3/4] Note in the ledger that an exited session settles without notifying --- SYNC-LEDGER.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SYNC-LEDGER.md b/SYNC-LEDGER.md index cbb47c0d19..b4d4b5d373 100644 --- a/SYNC-LEDGER.md +++ b/SYNC-LEDGER.md @@ -18,4 +18,4 @@ One line per upstream adaptation. Format: - 2026-09-28 | dez v0 sidebar (rail polish) | Rail presentation pass: status is a colored marker plus a label only for `Needs input` / `Finished`, titles use the default text color, rows share one height and one hover/selection treatment. Group headers aggregate every root (branch, summed changed count, worst indicator) and are clickable; a single-root group no longer repeats its header as a per-root row. Plain terminals that match no agent manifest classify as `Idle` rather than being dropped, so an ordinary prompt is listed. README's release section now points at the nightly artifacts while no stable tag exists. | crates/dez_sidebar/src/sidebar.rs, crates/agent_threads/src/terminal_control.rs, docs/dez-workspace-shell.md, FORK.md, README.md - 2026-09-28 | dez v0 sidebar (collapsible groups) | Rail group headers gain a disclosure chevron that folds the group's rows, driven by `MultiWorkspace`'s existing persisted group state via a new small `toggle_project_group_expanded` method; a collapsed group still shows its attention rollup. | crates/dez_sidebar/src/sidebar.rs, crates/workspace/src/multi_workspace.rs, FORK.md - 2026-09-28 | dez v0 sidebar (focus follows workspace) | Focusing a session from the rail now brings its workspace forward before focusing the pane: `AgentThreadStore::focus_thread` and `terminal_control::focus_terminal` activate the owning workspace, so clicking a session that lives in a background workspace actually shows it instead of looking like a no-op. Matches the Agent Threads panel's cross-project focus. | crates/agent_threads/src/{store.rs,terminal_control.rs}, FORK.md -- 2026-10-04 | dez v0 sidebar (truthful terminal threads) | The rail now hears sessions open and close (`AgentThreadStoreEvent` is an emit, not a notify, so the rail's `observe` alone left rows behind after a session ended) and repaints when a terminal's process exits, so a dead agent session stops showing the status dot it had while alive. Rail clicks log `focus_thread` / `focus_priority_terminal` failures instead of discarding them, since a stale `terminal_item_id` made a row look inert with no diagnostic. | crates/dez_sidebar/src/sidebar.rs, crates/agent_threads/src/{store.rs,terminal_control.rs} +- 2026-10-04 | dez v0 sidebar (truthful terminal threads) | The rail now hears sessions open and close (`AgentThreadStoreEvent` is an emit, not a notify, so the rail's `observe` alone left rows behind after a session ended) and repaints when a terminal's process exits, so a dead agent session stops showing the status dot it had while alive (settling to finished, without a second notification). Rail clicks log `focus_thread` / `focus_priority_terminal` failures instead of discarding them, since a stale `terminal_item_id` made a row look inert with no diagnostic. | crates/dez_sidebar/src/sidebar.rs, crates/agent_threads/src/{store.rs,terminal_control.rs} From 3437d2963d776da603e984b5a723cb8d258903b2 Mon Sep 17 00:00:00 2001 From: Bilal Arshad <111754659+Oreoro@users.noreply.github.com> Date: Sun, 4 Oct 2026 06:38:40 +0500 Subject: [PATCH 4/4] Scope the Wakeup attention test to the transition it guards mark_exited settles a thread whose process exited to Idle, and this test's echo agent exits inside the 500ms debounce window it waits on -- so its blanket `None` assertion now fails on a transition that has nothing to do with Wakeup. Assert `Blocked` specifically instead. That is the invariant the test and its comment actually describe: Wakeup fires on every line of ordinary streaming output, so it must not raise the thread the way a deliberate bell does. `Idle` was never what this test was guarding against -- a classifiable Working-to-Idle transition already produced it via Wakeup -- and settling on exit is a separate transition with its own reasoning. The bell test, which asserts on notification counts, needed no change and is green. --- SYNC-LEDGER.md | 2 +- crates/agent_threads/src/panel.rs | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/SYNC-LEDGER.md b/SYNC-LEDGER.md index b4d4b5d373..d81a7d1e72 100644 --- a/SYNC-LEDGER.md +++ b/SYNC-LEDGER.md @@ -18,4 +18,4 @@ One line per upstream adaptation. Format: - 2026-09-28 | dez v0 sidebar (rail polish) | Rail presentation pass: status is a colored marker plus a label only for `Needs input` / `Finished`, titles use the default text color, rows share one height and one hover/selection treatment. Group headers aggregate every root (branch, summed changed count, worst indicator) and are clickable; a single-root group no longer repeats its header as a per-root row. Plain terminals that match no agent manifest classify as `Idle` rather than being dropped, so an ordinary prompt is listed. README's release section now points at the nightly artifacts while no stable tag exists. | crates/dez_sidebar/src/sidebar.rs, crates/agent_threads/src/terminal_control.rs, docs/dez-workspace-shell.md, FORK.md, README.md - 2026-09-28 | dez v0 sidebar (collapsible groups) | Rail group headers gain a disclosure chevron that folds the group's rows, driven by `MultiWorkspace`'s existing persisted group state via a new small `toggle_project_group_expanded` method; a collapsed group still shows its attention rollup. | crates/dez_sidebar/src/sidebar.rs, crates/workspace/src/multi_workspace.rs, FORK.md - 2026-09-28 | dez v0 sidebar (focus follows workspace) | Focusing a session from the rail now brings its workspace forward before focusing the pane: `AgentThreadStore::focus_thread` and `terminal_control::focus_terminal` activate the owning workspace, so clicking a session that lives in a background workspace actually shows it instead of looking like a no-op. Matches the Agent Threads panel's cross-project focus. | crates/agent_threads/src/{store.rs,terminal_control.rs}, FORK.md -- 2026-10-04 | dez v0 sidebar (truthful terminal threads) | The rail now hears sessions open and close (`AgentThreadStoreEvent` is an emit, not a notify, so the rail's `observe` alone left rows behind after a session ended) and repaints when a terminal's process exits, so a dead agent session stops showing the status dot it had while alive (settling to finished, without a second notification). Rail clicks log `focus_thread` / `focus_priority_terminal` failures instead of discarding them, since a stale `terminal_item_id` made a row look inert with no diagnostic. | crates/dez_sidebar/src/sidebar.rs, crates/agent_threads/src/{store.rs,terminal_control.rs} +- 2026-10-04 | dez v0 sidebar (truthful terminal threads) | The rail now hears sessions open and close (`AgentThreadStoreEvent` is an emit, not a notify, so the rail's `observe` alone left rows behind after a session ended) and repaints when a terminal's process exits, so a dead agent session stops showing the status dot it had while alive (settling to finished, without a second notification). `panel::tests::wakeups_unclassifiable_content_leaves_attention_unchanged` now asserts `Blocked` specifically rather than `None`, because the echo agent it spawns exits inside its own 500ms debounce window and a dead thread is legitimately settled to `Idle`. Rail clicks log `focus_thread` / `focus_priority_terminal` failures instead of discarding them, since a stale `terminal_item_id` made a row look inert with no diagnostic. | crates/dez_sidebar/src/sidebar.rs, crates/agent_threads/src/{store.rs,terminal_control.rs} diff --git a/crates/agent_threads/src/panel.rs b/crates/agent_threads/src/panel.rs index e71e0466a1..839861bbba 100644 --- a/crates/agent_threads/src/panel.rs +++ b/crates/agent_threads/src/panel.rs @@ -4444,11 +4444,17 @@ mod tests { cx.executor().timer(Duration::from_millis(500)).await; cx.run_until_parked(); - assert_eq!( + // `Blocked` specifically, not `None`: the echo agent this test spawns + // exits during the 500ms debounce window, and a thread whose process is + // gone is legitimately settled to `Idle` (see `mark_exited`) -- that is + // a separate transition from the one under test here. What this test + // exists to catch is Wakeup raising the thread the way a Bell does, and + // only `Blocked` is the flagging it guards against. + assert_ne!( cx.update(|cx| AgentThreadStore::global(cx) .read(cx) .thread_attention(terminal_item_id)), - None, + Some(ThreadAttention::Blocked), "unclassifiable content reached via Wakeup (not Bell) should not flag the thread" ); }