fix(dashboard): follow scroll on implicit active-session resume (#93518)
pty_ws already fell back to the per-channel active-session file when a /chat WS connects with no ?resume= param, replaying the whole session into the PTY, but the frontend only pinned xterm's viewport to the bottom when resumeParam came from the URL (#59591). The implicit path had no way to learn a replay was happening, so the viewport stayed at the top of the scrollback. pty_ws now sends a one-off JSON control frame naming the session id it resolved from the active-session file, before any PTY bytes; PTY output itself always arrives as binary frames, so this is unambiguous on the wire. ChatPage tracks an `effectiveResume` value seeded from resumeParam and updated when this control frame arrives, and the existing follow-scroll/sanitizer/hydration logic keys off it instead of the URL param alone. Fixes #93518.
This commit is contained in:
@@ -17409,6 +17409,12 @@ async def pty_ws(ws: WebSocket) -> None:
|
||||
_forget_active_session_file(active_session_file)
|
||||
elif not resume:
|
||||
resume = _read_active_session_file(active_session_file)
|
||||
if resume:
|
||||
# The client only knows to pin the viewport to the bottom
|
||||
# when it requested `?resume=`. Tell it a replay is coming
|
||||
# anyway so the implicit active-session fallback gets the
|
||||
# same follow-scroll treatment as an explicit resume (#93518).
|
||||
await ws.send_json({"type": "resume", "id": resume})
|
||||
|
||||
resolve_kwargs = {
|
||||
"resume": resume,
|
||||
|
||||
@@ -83,6 +83,45 @@ def test_fresh_param_ignores_channel_active_session_file(pty_client, monkeypatch
|
||||
assert not active_file.exists()
|
||||
|
||||
|
||||
def test_active_session_fallback_sends_resume_control_message(pty_client, monkeypatch):
|
||||
"""Implicit resume (no `?resume=`) must tell the client which session.
|
||||
|
||||
Regression for #93518: the dashboard's stick-to-bottom replay logic only
|
||||
fires when the frontend can see a resume id. Without `?resume=` on the URL
|
||||
it previously had no way to learn that `pty_ws` fell back to the
|
||||
per-channel active-session file, so the viewport stayed pinned at the top
|
||||
of the replayed scrollback.
|
||||
"""
|
||||
ws, client, token = pty_client
|
||||
channel = "implicit-resume-chan"
|
||||
active_file = ws._active_session_file_for_channel(ws.app, channel)
|
||||
active_file.write_text(json.dumps({"session_id": "sess-old"}), encoding="utf-8")
|
||||
|
||||
monkeypatch.setattr(
|
||||
ws, "_resolve_chat_argv", lambda **kw: (["fake-hermes-tui"], None, None)
|
||||
)
|
||||
|
||||
with client.websocket_connect(_url(token, channel=channel)) as conn:
|
||||
assert conn.receive_json() == {"type": "resume", "id": "sess-old"}
|
||||
assert conn.receive_bytes() == b"ready"
|
||||
|
||||
|
||||
def test_explicit_resume_sends_no_control_message(pty_client, monkeypatch):
|
||||
"""An explicit `?resume=` already tells the client via the URL param."""
|
||||
ws, client, token = pty_client
|
||||
channel = "explicit-resume-chan"
|
||||
|
||||
monkeypatch.setattr(
|
||||
ws, "_resolve_chat_argv", lambda **kw: (["fake-hermes-tui"], None, None)
|
||||
)
|
||||
|
||||
with client.websocket_connect(
|
||||
_url(token, channel=channel, resume="sess-explicit")
|
||||
) as conn:
|
||||
# The first (and only) frame is PTY output, not a control message.
|
||||
assert conn.receive_bytes() == b"ready"
|
||||
|
||||
|
||||
def test_child_eof_closes_socket_and_bridge(pty_client, monkeypatch):
|
||||
"""Child EOF must close the WS server-side and reap the PTY.
|
||||
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { isViewportPinnedToBottom, shouldFollowPtyOutput } from "./pty-scroll";
|
||||
import {
|
||||
isViewportPinnedToBottom,
|
||||
parseResumeControlMessage,
|
||||
shouldFollowPtyOutput,
|
||||
} from "./pty-scroll";
|
||||
|
||||
describe("isViewportPinnedToBottom", () => {
|
||||
it("is pinned when the viewport sits on the bottom row", () => {
|
||||
@@ -47,3 +51,30 @@ describe("shouldFollowPtyOutput", () => {
|
||||
expect(shouldFollowPtyOutput("", true)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseResumeControlMessage", () => {
|
||||
it("extracts the id from a resume control frame", () => {
|
||||
// #93518: the implicit active-session fallback has no `?resume=` on the
|
||||
// URL, so the server names the session it resolved in a control frame.
|
||||
expect(
|
||||
parseResumeControlMessage('{"type":"resume","id":"sess-123"}'),
|
||||
).toBe("sess-123");
|
||||
});
|
||||
|
||||
it("ignores plain ANSI banner text sent as a text frame", () => {
|
||||
// pty_ws also sends "Chat unavailable: ..." error banners as text
|
||||
// frames; those must keep rendering into the terminal, not get
|
||||
// swallowed as a (mis-parsed) control message.
|
||||
expect(
|
||||
parseResumeControlMessage("\r\n\x1b[31mChat unavailable: x\x1b[0m\r\n"),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("ignores JSON of the wrong shape", () => {
|
||||
expect(parseResumeControlMessage('{"type":"other","id":"x"}')).toBeNull();
|
||||
expect(parseResumeControlMessage('{"type":"resume"}')).toBeNull();
|
||||
expect(parseResumeControlMessage('{"type":"resume","id":""}')).toBeNull();
|
||||
expect(parseResumeControlMessage("null")).toBeNull();
|
||||
expect(parseResumeControlMessage('"resume"')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -48,3 +48,31 @@ export function shouldFollowPtyOutput(
|
||||
): boolean {
|
||||
return Boolean(resumeParam) && stickToBottom;
|
||||
}
|
||||
|
||||
/**
|
||||
* When `pty_ws` falls back to the per-channel active-session file (no
|
||||
* `?resume=` on the URL), the server sends a one-off JSON text frame naming
|
||||
* the session it resolved before any PTY replay bytes arrive, since the
|
||||
* frontend has no other way to learn a replay is happening (#93518). PTY
|
||||
* output itself always arrives as binary frames, so any text frame is a
|
||||
* candidate; this returns the resume id on a match and `null` for anything
|
||||
* else (including the plain ANSI error banners `pty_ws` still sends as text
|
||||
* on failure, which must keep rendering into the terminal as before).
|
||||
*/
|
||||
export function parseResumeControlMessage(data: string): string | null {
|
||||
try {
|
||||
const parsed = JSON.parse(data);
|
||||
if (
|
||||
parsed &&
|
||||
typeof parsed === "object" &&
|
||||
parsed.type === "resume" &&
|
||||
typeof parsed.id === "string" &&
|
||||
parsed.id
|
||||
) {
|
||||
return parsed.id;
|
||||
}
|
||||
} catch {
|
||||
/* not JSON — an ANSI banner or other plain-text frame */
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -68,6 +68,7 @@ import {
|
||||
} from "@/lib/pty-keyboard-shortcuts";
|
||||
import {
|
||||
isViewportPinnedToBottom,
|
||||
parseResumeControlMessage,
|
||||
shouldFollowPtyOutput,
|
||||
} from "@/lib/pty-scroll";
|
||||
import {
|
||||
@@ -1064,6 +1065,11 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) {
|
||||
// ``return cleanup`` stays at the top level; handlers + disposables
|
||||
// are hoisted to ``let`` bindings the cleanup closes over.
|
||||
let unmounting = false;
|
||||
// The implicit active-session fallback (no `?resume=` on the URL) only
|
||||
// becomes known once the server's control frame arrives (see
|
||||
// `ws.onmessage` below) — everything gated on "is this a resume replay"
|
||||
// reads this instead of `resumeParam` directly (#93518).
|
||||
let effectiveResume = resumeParam;
|
||||
let onDataDisposable: { dispose(): void } | null = null;
|
||||
let onResizeDisposable: { dispose(): void } | null = null;
|
||||
let onScrollDisposable: { dispose(): void } | null = null;
|
||||
@@ -1088,7 +1094,7 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) {
|
||||
}
|
||||
};
|
||||
const noteResumePtyChunk = (chunkText: string) => {
|
||||
if (!resumeParam || unmounting) {
|
||||
if (!effectiveResume || unmounting) {
|
||||
return;
|
||||
}
|
||||
if (shouldFinishResumeHydrationOnChunk(chunkText)) {
|
||||
@@ -1256,14 +1262,42 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) {
|
||||
// in-place redraws through untouched. See pty-resume-sanitizer.ts.
|
||||
const decoder = new TextDecoder();
|
||||
const sanitizer = new PtyResumeSanitizer();
|
||||
const beginResumeReplay = () => {
|
||||
stickToBottomRef.current = true;
|
||||
if (!eraseSuppressionTimer) {
|
||||
eraseSuppressionTimer = setTimeout(() => {
|
||||
eraseSuppressionTimer = null;
|
||||
sanitizer.endEraseSuppression();
|
||||
}, PTY_RESUME_SANITIZE_WINDOW_MS);
|
||||
}
|
||||
if (!resumeMaxTimer) {
|
||||
setResumeHydrating(true);
|
||||
resumeMaxTimer = setTimeout(
|
||||
finishResumeHydration,
|
||||
PTY_RESUME_LOADING_MAX_MS,
|
||||
);
|
||||
}
|
||||
};
|
||||
if (resumeParam) {
|
||||
eraseSuppressionTimer = setTimeout(() => {
|
||||
eraseSuppressionTimer = null;
|
||||
sanitizer.endEraseSuppression();
|
||||
}, PTY_RESUME_SANITIZE_WINDOW_MS);
|
||||
beginResumeReplay();
|
||||
}
|
||||
|
||||
ws.onmessage = (ev) => {
|
||||
if (typeof ev.data === "string") {
|
||||
// The active-session fallback (no `?resume=` on the URL) tells us
|
||||
// via a one-off JSON control frame that a replay is starting (#93518,
|
||||
// see `pty_ws` in web_server.py). Real PTY output always arrives as
|
||||
// binary frames, so any text frame is a candidate; anything that
|
||||
// isn't this control shape (e.g. the ANSI "Chat unavailable" banners
|
||||
// pty_ws sends as text on failure) falls through to the write path
|
||||
// below unchanged.
|
||||
const resumeId = parseResumeControlMessage(ev.data);
|
||||
if (resumeId) {
|
||||
effectiveResume = resumeId;
|
||||
beginResumeReplay();
|
||||
return;
|
||||
}
|
||||
}
|
||||
const text =
|
||||
typeof ev.data === "string"
|
||||
? ev.data
|
||||
@@ -1274,13 +1308,13 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) {
|
||||
// sanitizer can turn a nonempty erase-only / all-newline / partial-CSI
|
||||
// resume frame into "" (pty-resume-sanitizer.ts); keying off raw `text`
|
||||
// would hide the wait notice while the terminal is still blank.
|
||||
const rendered = resumeParam ? sanitizer.next(text) : text;
|
||||
const rendered = effectiveResume ? sanitizer.next(text) : text;
|
||||
// Resume replay lands over many write chunks; pin the viewport to the
|
||||
// bottom as each chunk COMMITS (xterm write callback) instead of
|
||||
// guessing with a fixed delay, and release the pin the moment the user
|
||||
// scrolls up to read the backlog (#59591).
|
||||
const followScroll = shouldFollowPtyOutput(
|
||||
resumeParam,
|
||||
effectiveResume,
|
||||
stickToBottomRef.current,
|
||||
)
|
||||
? () => termRef.current?.scrollToBottom()
|
||||
@@ -1293,7 +1327,7 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) {
|
||||
// Drain buffered sanitizer state. A buffered partial escape is dropped
|
||||
// (writing an unterminated CSI would wedge xterm's parser); a buffered
|
||||
// newline run is emitted collapsed.
|
||||
if (resumeParam) {
|
||||
if (effectiveResume) {
|
||||
clearEraseSuppressionTimer();
|
||||
try {
|
||||
term.write(sanitizer.flush());
|
||||
|
||||
Reference in New Issue
Block a user