Skip to content

Commit 71986cf

Browse files
mizu-junclaudehappy-otter
authored
fix: resync clients on broadcast lag (audit round 3, P4) (#27)
When a client's broadcast receiver overflowed, both forwarders logged the RecvError::Lagged and continued, so a dropped GridDiff left the screen corrupt until the next unrelated FullRefresh. Add Session::focused_window_full_refresh, which rebuilds a FullRefresh for every pane in the focused window, and call it from both forwarders on lag: - IPC forwarder (session_dispatch): replay each pane's current grid to the client, capturing the sessions Arc + session name into the task. - WebSocket forwarder (ws): same, converted to text for xterm.js. Also update the audit report with the implementation status. Tested: focused_window_full_refresh_covers_every_pane (ignore-gated like the other PTY-spawning tests). cargo test / clippy -D warnings / fmt --check green. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Happy <yesreply@happy.engineering>
1 parent 07e27ff commit 71986cf

4 files changed

Lines changed: 102 additions & 2 deletions

File tree

docs/plans/audit-round3-2026h2.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
- **Predecessors:** Round 1 (completed at v1.1.0), Round 2 (70 items, most HIGH resolved).
88
This round only reports findings not already tracked or resolved there.
99

10+
## Implementation status
11+
12+
- **Done:** P1, B1, P2, R1 (PR #26, squashed to `master`), P4 (this change).
13+
- **Deferred (still tracked below):** P3, C2, P5, P6, P7, S1 (LOW), R5/A5.
14+
1015
## Baseline statistics
1116

1217
| Metric | Value | Note |

nexterm-server/src/ipc/session_dispatch.rs

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ pub(super) async fn handle_attach(ctx: &mut DispatchContext<'_>, session_name: &
7979
};
8080
if let Some(mut bcast_rx) = bcast_rx {
8181
let fwd_tx = tx.clone();
82+
// Captured so the forwarder can rebuild a full refresh when the
83+
// broadcast receiver lags (audit round 3, P4).
84+
let resync_sessions = manager.sessions();
85+
let resync_name = session_name.to_string();
8286
if let Some(h) = ctx.bcast_forwarder.take() {
8387
let _: () = h.abort();
8488
}
@@ -91,10 +95,35 @@ pub(super) async fn handle_attach(ctx: &mut DispatchContext<'_>, session_name: &
9195
}
9296
}
9397
Err(tokio::sync::broadcast::error::RecvError::Lagged(n)) => {
98+
// The client fell behind and lost `n` messages;
99+
// a dropped GridDiff would leave the screen
100+
// corrupt, so replay the current grid of every
101+
// pane in the focused window to resync (P4).
94102
tracing::warn!(
95-
"broadcast: skipped {} messages (buffer overflow)",
103+
"broadcast: client lagged by {} messages; resyncing with full refresh",
96104
n
97105
);
106+
let refreshes = {
107+
let sessions = resync_sessions.lock().await;
108+
sessions
109+
.get(&resync_name)
110+
.map(|s| s.focused_window_full_refresh())
111+
.unwrap_or_default()
112+
};
113+
let mut disconnected = false;
114+
for (pane_id, grid) in refreshes {
115+
if fwd_tx
116+
.send(ServerToClient::FullRefresh { pane_id, grid })
117+
.await
118+
.is_err()
119+
{
120+
disconnected = true;
121+
break;
122+
}
123+
}
124+
if disconnected {
125+
break;
126+
}
98127
continue;
99128
}
100129
Err(tokio::sync::broadcast::error::RecvError::Closed) => break,

nexterm-server/src/session.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,23 @@ impl Session {
137137
self.windows.get_mut(&self.focused_window_id)
138138
}
139139

140+
/// Build a full refresh for every pane in the focused window.
141+
///
142+
/// Used to resync a client after its broadcast receiver lagged and dropped
143+
/// `GridDiff` messages (audit round 3, finding P4). A single dropped diff can
144+
/// leave the screen permanently corrupt, so on overflow the forwarder replays
145+
/// the current grid of each pane instead of silently continuing.
146+
pub fn focused_window_full_refresh(&self) -> Vec<(u32, nexterm_proto::Grid)> {
147+
match self.focused_window() {
148+
Some(window) => window
149+
.pane_ids()
150+
.into_iter()
151+
.filter_map(|id| window.pane(id).map(|p| (p.id, p.make_full_refresh())))
152+
.collect(),
153+
None => Vec::new(),
154+
}
155+
}
156+
140157
/// Attach a client and return its `broadcast::Receiver`.
141158
///
142159
/// Supports multiple simultaneous clients. PTY output is automatically delivered to every
@@ -1400,6 +1417,30 @@ mod tests {
14001417
assert!(!session.broadcast);
14011418
}
14021419

1420+
#[tokio::test]
1421+
#[ignore = "spawns a PTY; hangs on interactive shell close in regular CI"]
1422+
async fn focused_window_full_refresh_covers_every_pane() {
1423+
// Audit round 3 (P4): the lag-resync path replays a full refresh for
1424+
// each pane in the focused window. Verify one entry per pane, keyed by id.
1425+
let shell = nexterm_config::ShellConfig::default();
1426+
let session =
1427+
Session::new("resync".to_string(), 80, 24, shell.program, shell.args).unwrap();
1428+
1429+
let expected_ids = session
1430+
.focused_window()
1431+
.map(|w| w.pane_ids())
1432+
.unwrap_or_default();
1433+
assert!(!expected_ids.is_empty(), "a new session has one pane");
1434+
1435+
let refreshes = session.focused_window_full_refresh();
1436+
assert_eq!(refreshes.len(), expected_ids.len());
1437+
for (pane_id, grid) in &refreshes {
1438+
assert!(expected_ids.contains(pane_id));
1439+
assert_eq!(grid.width, 80);
1440+
assert_eq!(grid.height, 24);
1441+
}
1442+
}
1443+
14031444
#[tokio::test]
14041445
#[ignore = "spawns a PTY; hangs on interactive shell close in regular CI"]
14051446
async fn session_info_returns_correct_metadata() {

nexterm-server/src/web/handlers/ws.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,32 @@ async fn handle_socket(mut socket: WebSocket, manager: Arc<SessionManager>, sess
123123
}
124124
}
125125
Err(tokio::sync::broadcast::error::RecvError::Closed) => break,
126-
Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {}
126+
Err(tokio::sync::broadcast::error::RecvError::Lagged(n)) => {
127+
// The browser fell behind and lost `n` diffs; replay the
128+
// focused window's panes so the terminal resyncs instead
129+
// of rendering on top of a corrupt screen (audit round 3, P4).
130+
warn!("WebSocket: client lagged by {} messages; resyncing", n);
131+
let refreshes = {
132+
let sessions = sessions_arc.lock().await;
133+
sessions
134+
.get(&session_name)
135+
.map(|s| s.focused_window_full_refresh())
136+
.unwrap_or_default()
137+
};
138+
let mut disconnected = false;
139+
for (pane_id, grid) in refreshes {
140+
let msg = nexterm_proto::ServerToClient::FullRefresh { pane_id, grid };
141+
if let Some(text) = pty_message_to_text(&msg)
142+
&& socket.send(Message::Text(text)).await.is_err()
143+
{
144+
disconnected = true;
145+
break;
146+
}
147+
}
148+
if disconnected {
149+
break;
150+
}
151+
}
127152
}
128153
}
129154
result = socket.recv() => {

0 commit comments

Comments
 (0)