Skip to content

Commit afd2bea

Browse files
DDKingerCopilot
andauthored
Fix /sessions resume: keep the pane and un-suppress the status fallback (#713)
* Keep a resumed pane bound to the session the user resumed `/sessions` resume creates the pane and binds it through `ResumePaneAssigned` before the agent CLI starts, so that pane belongs to exactly one session id. Copilot's `--resume` boots a throwaway bootstrap session first and only switches to the requested one seconds later, so its deferred SessionStart hook reports the bootstrap id together with the resumed pane's GUID. Master's `active_by_pane` handoff took that at face value and demoted the resumed row to Ended; terminal-state rows refuse resurrection, so the row stayed Ended for the rest of the CLI's life while the hookless watcher's status events were silently dropped. Mark a WTA-established pane binding `born_bound_pane` and refuse the handoff when the pane's current owner is a live born-bound row with a different key. The incoming session is still recorded, it just gets no pane binding. The flag clears itself when the row gives up the pane (SessionStopped, PaneClosed, end_entry) or once a SessionStarted for the same key claims it, so a genuinely new session typed into that pane still takes over and no timer is involved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd1be30e-1a90-4c72-af2e-1b138a6cc866 * Drop a stale hook_owned claim when a session is born-bound again `hook_owned` was sticky: once a session id landed there it stayed for the lifetime of the master process. Resuming a session that had already run once in the same process therefore added `born_bound` while the old claim remained, and because `apply_watcher_event` checks `hook_owned` first, every watcher status event for the resumed row was dropped. The row sat at Idle for its whole life even though the CLI was working and its transcript kept growing. That combination is easy to hit now that Copilot's `--resume` bootstrap session consumes the SessionStart hook: the resumed id itself never emits another hook, so the watcher is the only remaining status producer -- and it was suppressed. Observed: cd195d64 got real hooks at 04:05, was resumed at 06:05 in the same master process (started 03:01), and master then broadcast zero sessions/changed while events.jsonl grew to 209 KB. Restore the documented disjointness by removing the id from `hook_owned` on the binding-only path. A born-bound event means WTA has just relaunched that id, so the previous generation's claim is over, and a real hook re-claims ownership on its very next event. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd1be30e-1a90-4c72-af2e-1b138a6cc866 * Name the pane's current owner `owner` in the born-bound guard The three closures in the guard's chain bound the same name to three different things: two `&SessionId` for the pane's current owner, then `&SessionInfo` for that owner's row. `prev` also read as "the previous value of sid", which is the opposite of what it holds -- `sid` is the incoming event's session id, the one attempting to claim the pane, while the chain resolves whoever already owns it. `owner_sid` / `owner` matches the comment right above, which already calls it "the pane's born-bound owner". Rename only; no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd1be30e-1a90-4c72-af2e-1b138a6cc866 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd1be30e-1a90-4c72-af2e-1b138a6cc866
1 parent aef879b commit afd2bea

4 files changed

Lines changed: 334 additions & 4 deletions

File tree

doc/specs/hybrid-agent-session-tracking.md

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,19 @@ The master keeps **two disjoint** ownership sets (`master/mod.rs`):
153153

154154
`handle_session_hook` routes each inbound event: a binding-only event (the
155155
dedicated `intellterm.wta/session_born_bound` method, or a
156-
`ResumeDispatched`/`ResumePaneAssigned` resume-binding event) → `born_bound`;
157-
anything else (a real hook / ACP event) → `hook_owned` (and, if the session was
158-
born-bound, drops it from `born_bound` — a real hook **takes over**).
156+
`ResumeDispatched`/`ResumePaneAssigned` resume-binding event) → `born_bound`
157+
(and drops any stale `hook_owned` claim — see below); anything else (a real
158+
hook / ACP event) → `hook_owned` (and, if the session was born-bound, drops it
159+
from `born_bound` — a real hook **takes over**).
160+
161+
The two sets are disjoint **in both directions**. A born-bound event means WTA
162+
has just (re)launched that session id, so an ownership claim left by an earlier
163+
generation of the same id is over. Without the reverse removal, resuming a
164+
session that had already run once in the same master process left it in
165+
`hook_owned` forever; since `apply_watcher_event` checks `hook_owned` first,
166+
every watcher status event for the resumed row was dropped and the row sat at
167+
`Idle` for its whole life. A real hook re-claims ownership on its very next
168+
event, so nothing is lost when hooks are working.
159169

160170
`apply_watcher_event` then, in order:
161171

@@ -195,6 +205,21 @@ the hook-free resume binding, so `handle_session_hook` records them in
195205
treated as hook-owned and its row would sit at `Idle` forever even as the watcher
196206
saw activity.
197207

208+
**Resume pane ownership.** `ResumePaneAssigned` marks the row's pane binding
209+
`born_bound_pane` (`session_registry.rs`). WTA creates the resume pane and binds
210+
it *before* the agent CLI starts, so that pane belongs to exactly one session
211+
id. Copilot's `--resume` boots a throwaway bootstrap session and only switches
212+
to the requested one seconds later, so its deferred `SessionStart` hook reports
213+
the **bootstrap** id against the resumed pane's GUID. Master's `SessionStarted`
214+
reducer therefore refuses the `active_by_pane` handoff when the pane's current
215+
owner is a live born-bound row with a different key: the incoming session is
216+
still recorded, it just gets no pane binding. Without the guard the resumed row
217+
was demoted to `Ended`, and because terminal-state rows refuse resurrection it
218+
stayed there for the rest of the CLI's life — the watcher's status fallback
219+
silently dropped every event. The flag clears itself whenever the row gives up
220+
the pane (`SessionStopped`, `PaneClosed`, `end_entry`) or once a `SessionStarted`
221+
for the *same* key claims it, so the protection needs no timer.
222+
198223
### Liveness gate: scoping to this IT window
199224

200225
The four CLIs write their session state to **per-user** roots, so the watcher

tools/wta/src/master/mod.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6149,14 +6149,23 @@ async fn handle_session_hook(
61496149
//
61506150
// * binding-only (#266 delegate born-bound + resume binding events): record
61516151
// in `born_bound` so the watcher may still supply STATUS when no real hook
6152-
// is installed — without re-binding the pane.
6152+
// is installed — without re-binding the pane. Also drop any **stale**
6153+
// `hook_owned` claim: the two sets are disjoint by contract, and a
6154+
// born-bound event means WTA has just (re)launched this session id, so an
6155+
// ownership claim left by a previous generation of the same session is
6156+
// over. Without this a `/sessions` resume of a session that ran earlier
6157+
// in the same master process stayed `hook_owned` forever, and because
6158+
// `apply_watcher_event` checks `hook_owned` first, every watcher status
6159+
// event for the resumed row was dropped — the row sat at Idle for the
6160+
// whole session. A real hook re-claims ownership on its very next event.
61536161
// * real hook / ACP agent-pane event: authoritative for binding AND
61546162
// activity. Record in `hook_owned` (full watcher suppression) and, if the
61556163
// session was previously born-bound, drop it from `born_bound` — the real
61566164
// hook now owns it.
61576165
if let Some(key) = &refresh_key {
61586166
let sid = acp::schema::v1::SessionId::new(key.clone());
61596167
if binding_only {
6168+
state.hook_owned.lock().await.remove(&sid);
61606169
state.born_bound.lock().await.insert(sid);
61616170
} else {
61626171
state.hook_owned.lock().await.insert(sid.clone());

tools/wta/src/master/tests.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10395,3 +10395,76 @@ async fn resume_binding_events_are_born_bound_not_hook_owned() {
1039510395
);
1039610396
assert!(!state.hook_owned.lock().await.contains(&sid));
1039710397
}
10398+
10399+
#[tokio::test]
10400+
async fn resume_binding_events_clear_a_stale_hook_ownership_claim() {
10401+
// Regression: resuming a session that already ran once in THIS master
10402+
// process. The earlier run's hooks put the id in `hook_owned`, and
10403+
// `hook_owned` used to be sticky — the resume added `born_bound` but left
10404+
// the stale claim in place, so `apply_watcher_event`'s first check dropped
10405+
// every watcher status event and the resumed row sat at Idle for its whole
10406+
// life. The two sets are disjoint by contract; a born-bound event means WTA
10407+
// just relaunched the id, so the previous generation's claim is over.
10408+
let state = make_state();
10409+
let sid = acp::schema::v1::SessionId::new("sid-rerun".to_string());
10410+
10411+
let first_run = crate::agent_sessions::SessionEvent::SessionStarted {
10412+
key: "sid-rerun".to_string(),
10413+
cli_source: crate::agent_sessions::CliSource::Copilot,
10414+
pane_session_id: "pane-old".to_string(),
10415+
cwd: std::path::PathBuf::from("C:\\repo"),
10416+
title: String::new(),
10417+
};
10418+
handle_session_hook(&state, first_run, false)
10419+
.await
10420+
.expect("real hook accepted");
10421+
assert!(state.hook_owned.lock().await.contains(&sid));
10422+
10423+
let dispatched = crate::agent_sessions::SessionEvent::ResumeDispatched {
10424+
key: "sid-rerun".to_string(),
10425+
};
10426+
handle_session_hook(&state, dispatched, false)
10427+
.await
10428+
.expect("resume dispatched accepted");
10429+
10430+
assert!(
10431+
!state.hook_owned.lock().await.contains(&sid),
10432+
"the resume must drop the previous run's hook_owned claim, or the \
10433+
watcher's status fallback stays suppressed for the whole session"
10434+
);
10435+
assert!(
10436+
state.born_bound.lock().await.contains(&sid),
10437+
"the resumed session is born-bound"
10438+
);
10439+
}
10440+
10441+
#[tokio::test]
10442+
async fn born_bound_delegate_clears_a_stale_hook_ownership_claim() {
10443+
// Same invariant for the dedicated born-bound method (`?<prompt>`
10444+
// delegation), which reaches `handle_session_hook` with is_born_bound=true.
10445+
let state = make_state();
10446+
let sid = acp::schema::v1::SessionId::new("sid-delegate".to_string());
10447+
10448+
let earlier = crate::agent_sessions::SessionEvent::ToolStarting {
10449+
key: "sid-delegate".to_string(),
10450+
tool_name: "Bash".to_string(),
10451+
};
10452+
handle_session_hook(&state, earlier, false)
10453+
.await
10454+
.expect("real hook accepted");
10455+
assert!(state.hook_owned.lock().await.contains(&sid));
10456+
10457+
let born = crate::agent_sessions::SessionEvent::SessionStarted {
10458+
key: "sid-delegate".to_string(),
10459+
cli_source: crate::agent_sessions::CliSource::Copilot,
10460+
pane_session_id: "pane-new".to_string(),
10461+
cwd: std::path::PathBuf::from("C:\\repo"),
10462+
title: String::new(),
10463+
};
10464+
handle_session_hook(&state, born, true)
10465+
.await
10466+
.expect("born-bound accepted");
10467+
10468+
assert!(!state.hook_owned.lock().await.contains(&sid));
10469+
assert!(state.born_bound.lock().await.contains(&sid));
10470+
}

0 commit comments

Comments
 (0)