Make the rail's view of terminal-backed sessions truthful

[Oreoro]

Summary

Three ways the rail lied about the sessions it supervises. All three present as "integrating terminal threads in the sidebar doesn't work".

1. 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 (panel.rs:741-758); the rail only observed (sidebar.rs:317-322). 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.

2. A dead session kept its last status dot. Agent terminals are spawned with HideStrategy::Never, so when codex quits the tab, the TerminalView, and the store entry all survive — and nothing observed ProcessExited, so the stored classification from while the agent was alive (usually a green Running) stood indefinitely. A new AgentThreadStore::mark_exited settles the thread to Idle and emits ThreadUpdated; plain shells already re-derive status from the screen tail on every read, so the registry's subscription just needed to observe the exit too.

mark_exited deliberately does not route through reclassify_attention. My first attempt did, and it failed panel::tests::bell_does_not_request_attention_for_the_active_window: that function's tail raises a desktop notification whenever a thread lands in a non-Working state, so a thread already flagged blocked and then killed notified twice. A process exiting is not a fresh "look at me" signal — the user has already been told if it was blocked — so the exit updates state and emits, without notifying.

3. 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 — the case worth diagnosing — and let _ = made that click indistinguishable from one the user simply missed. The panel logs these (panel.rs:1260-1264); the rail did not.

Rules

No state-machine change: mark_exited sets the same ThreadAttention::Idle / finished_seen pair reclassify_attention sets for a finished turn, so the rail's existing Finished vs Idle projection is untouched. Additive util dependency for ResultExt::log_err.

Known gap, deliberately not in this PR

AgentThreadStore::restore_attempted (store.rs:431, inserted at store.rs:3316, never removed) looked like it would make a reloaded workspace unable to restore, because it is keyed on database_id(), which survives a reload while the guard is meant to be per-app-session.

I traced this and it is not the cause. workspace::reload is a full process restart — reload() → prepare_to_close → cx.restart() (workspace.rs:8867-8921) → platform re-exec (gpui_macos/src/platform.rs:528-567). AgentThreadStore is created fresh in init_global, so restore_attempted is empty after every reload. Session-id keying across the restart is also correct: writes use session.id() (current), reads use last_session_id() (previous), and pruning keeps exactly {current, previous}. Every option for re-keying the guard is either a no-op on this path or a regression — dropping it would let two restores of one session id race in the window between selection and register, and would resurrect a thread the user deliberately closed. Leaving it alone.

One real defect the guard does have, not fixed here: it is consumed even when the attempt selects zero records (empty snapshot, kind hidden in settings, tie mismatch, all resumes failed), and nothing ever retries for that workspace in that process. The likely producer is records_to_restore_for_workspace's tie routing, which reads linked worktrees synchronously while that scan is cold right after a restart. I have not proven the causal chain, so I am not claiming it as a fix — tracking separately.

Verification

Not built locally by request. rustfmt --check clean on all touched files. clippy green; tests is the gate (the exit path is covered by the existing agent_threads panel tests).

Suggested .rules additions

• cx.observe(&entity, ..) fires only on notify(). Anything that opens or removes children must also cx.subscribe, because cx.emit does not notify — crates/agent_threads/src/store.rs emits on register/shutdown with no notify(), so an observer-only view keeps rows for dead sessions.
• Agent terminals are spawned with HideStrategy::Never (store.rs:3097-3113), so a terminal surviving its process is normal here. Anything rendering a session's liveness must observe terminal::Event::ProcessExited; Wakeup and Bell stop at process exit.
• Do not implement a state transition by adding a trigger variant to reclassify_attention unless the transition should also raise a desktop notification. It notifies on every non-Working landing; a dedicated transition is usually what you want.
• workspace::reload is a process restart (cx.restart()), not an in-process teardown. In-memory state — including AgentThreadStore::restore_attempted — does not survive it, so don't reason about reload as if it does.

Release Notes:

• Fixed closed agent sessions lingering in the workspace rail, agent sessions showing a stale status after their process exited, and rail session rows failing to focus silently.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: d511e70e-aaf5-4713-b063-9f54e4b439b8

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[graphify-labs]

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

---

Graphify review — findings

Makes the rail track session lifecycle. It now subscribes to AgentThreadStoreEvent, so opened and closed sessions repaint immediately instead of leaving stale rows, and it repaints on terminal::Event::ProcessExited. An exited agent's inconclusive classification now falls back to ThreadAttention::Idle via a new ProcessExited trigger rather than keeping its last Running dot. Rail clicks also log_err failures from focus_thread and focus_priority_terminal instead of discarding them, so a stale terminal_item_id leaves a diagnostic.

Worth a look

• Exited agent keeps a Running status when its final screen tail still classifies as working — crates/agent_threads/src/store.rs:1467 · Escalate · medium
  • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 450 functions depend on the 297 functions this change touches.

Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):

• spawn_live_codex_thread() — 20 callers, 6 callees (high)
• dispatch_terminal() — 10 callers, 10 callees (high)
• prepare_managed_agent() — 4 callers, 7 callees (medium)
• dispatch() — 4 callers, 6 callees (medium)
• retie_thread() — 6 callers, 4 callees (medium)
• start() — 2 callers, 9 callees (medium)
• init_test_with_state() — 1 callers, 16 callees (high)
• launch_seeded_thread_at() — 2 callers, 7 callees (medium)
• …and 2 more

Verification — 450 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

• verification_scope: 450 function(s) in the blast radius were not formally verified this run