fix(hermes): relay history scrolling and repaint reattached terminals
This commit is contained in:
parent
990a83d34b
commit
7a69e9ea5c
@ -1619,6 +1619,21 @@ function RootRedirect() {
|
||||
NODE
|
||||
RUN case "${HERMES_KANIKO_HEREDOC_COMPAT}" in 0) ;; 1) python /tmp/hermes-kaniko-heredoc-runner.py --dockerfile /tmp/hermes-agent.Dockerfile --block-index 9 ;; *) exit 2 ;; esac
|
||||
|
||||
# Reattachment reuses a live PTY, so a dashboard-only SIGCONT restores its
|
||||
# alternate screen and current viewport after a bounded replay.
|
||||
COPY dockerfiles/patch-hermes-terminal-replay.py /tmp/patch-hermes-terminal-replay.py
|
||||
COPY dockerfiles/hermes-terminal-replay-regression.py /tmp/hermes-terminal-replay-regression.py
|
||||
RUN /opt/hermes/.venv/bin/python /tmp/patch-hermes-terminal-replay.py \
|
||||
&& HERMES_SOURCE_ROOT=/opt/hermes /opt/hermes/.venv/bin/python \
|
||||
/tmp/hermes-terminal-replay-regression.py
|
||||
|
||||
# The dashboard keeps keyboard and paste handling unchanged, but forwards only
|
||||
# bounded SGR wheel reports when the TUI has explicitly enabled mouse tracking.
|
||||
COPY dockerfiles/patch-hermes-dashboard-wheel.js /tmp/patch-hermes-dashboard-wheel.js
|
||||
COPY dockerfiles/hermes-dashboard-terminal-input.ts /opt/hermes/web/src/lib/dashboard-terminal-input.ts
|
||||
COPY dockerfiles/hermes-dashboard-terminal-input.test.ts /opt/hermes/web/src/lib/dashboard-terminal-input.test.ts
|
||||
RUN node /tmp/patch-hermes-dashboard-wheel.js
|
||||
|
||||
COPY dockerfiles/patch-hermes-execution-safety.py /tmp/patch-hermes-execution-safety.py
|
||||
COPY dockerfiles/hermes-execution-safety-regression.py /tmp/hermes-execution-safety-regression.py
|
||||
COPY dockerfiles/hermes_execution_patch_support.py /tmp/hermes_execution_patch_support.py
|
||||
@ -1654,6 +1669,7 @@ COPY dockerfiles/hermes-session-migrate.py /opt/hermes/bin/hermes-session-migrat
|
||||
RUN rm -f /tmp/hermes-agent.Dockerfile /tmp/hermes-kaniko-heredoc-runner.py
|
||||
|
||||
RUN cd /opt/hermes/web \
|
||||
&& npm test -- src/lib/dashboard-terminal-input.test.ts \
|
||||
&& npm run build \
|
||||
&& grep -Fq 'await api.getSessions(1, 0' src/pages/ChatPage.tsx \
|
||||
&& grep -Fq 'api.getSessions(1, 0' src/components/ChatSidebar.tsx \
|
||||
@ -1685,6 +1701,8 @@ RUN cd /opt/hermes/web \
|
||||
/opt/hermes/hermes_cli/web_server.py \
|
||||
&& ! grep -Fq 'WebglAddon' src/pages/ChatPage.tsx \
|
||||
&& grep -Fq 'scheduleTerminalPaint' src/pages/ChatPage.tsx \
|
||||
&& ! grep -Fq 'attachCustomWheelEventHandler' src/pages/ChatPage.tsx \
|
||||
&& grep -Fq 'isSgrWheelReport(data)' src/pages/ChatPage.tsx \
|
||||
&& grep -Fq 'sessionTree.childrenByParent' \
|
||||
src/components/ChatSessionList.tsx \
|
||||
&& grep -Fq 'getSessions(SESSION_LIMIT, 0, scopeKey, "recent", true)' \
|
||||
|
||||
@ -5,6 +5,11 @@
|
||||
!dockerfiles/hermes-public-extract/**
|
||||
!dockerfiles/hermes-session-activity-panel.tsx
|
||||
!dockerfiles/hermes-session-migrate.py
|
||||
!dockerfiles/patch-hermes-terminal-replay.py
|
||||
!dockerfiles/hermes-terminal-replay-regression.py
|
||||
!dockerfiles/patch-hermes-dashboard-wheel.js
|
||||
!dockerfiles/hermes-dashboard-terminal-input.ts
|
||||
!dockerfiles/hermes-dashboard-terminal-input.test.ts
|
||||
!dockerfiles/Dockerfile.hermes-agent
|
||||
!dockerfiles/hermes-kaniko-heredoc-runner.py
|
||||
!dockerfiles/patch-hermes-execution-safety.py
|
||||
|
||||
20
dockerfiles/hermes-dashboard-terminal-input.test.ts
Normal file
20
dockerfiles/hermes-dashboard-terminal-input.test.ts
Normal file
@ -0,0 +1,20 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { isSgrWheelReport } from "./dashboard-terminal-input";
|
||||
|
||||
describe("isSgrWheelReport", () => {
|
||||
it("accepts xterm SGR wheel reports, including modifiers", () => {
|
||||
expect(isSgrWheelReport("\x1b[<64;80;25M")).toBe(true);
|
||||
expect(isSgrWheelReport("\x1b[<65;80;25M")).toBe(true);
|
||||
expect(isSgrWheelReport("\x1b[<84;80;25M")).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects clicks, releases, malformed reports, and arbitrary input", () => {
|
||||
expect(isSgrWheelReport("\x1b[<0;80;25M")).toBe(false);
|
||||
expect(isSgrWheelReport("\x1b[<64;80;25m")).toBe(false);
|
||||
expect(isSgrWheelReport("\x1b[<66;80;25M")).toBe(false);
|
||||
expect(isSgrWheelReport("\x1b[<64;10000;25M")).toBe(false);
|
||||
expect(isSgrWheelReport("\x1b[<64;80;25M\rmalicious")).toBe(false);
|
||||
expect(isSgrWheelReport("/submit\r")).toBe(false);
|
||||
});
|
||||
});
|
||||
27
dockerfiles/hermes-dashboard-terminal-input.ts
Normal file
27
dockerfiles/hermes-dashboard-terminal-input.ts
Normal file
@ -0,0 +1,27 @@
|
||||
/** Validate the only mouse reports the dashboard may relay to its PTY. */
|
||||
|
||||
// SGR wheel reports use button codes 64/65 plus optional Shift/Alt/Ctrl bits.
|
||||
// Keep coordinates bounded so arbitrary control text never reaches the PTY.
|
||||
const SGR_WHEEL_REPORT = /^\x1b\[<(\d{1,2});(\d{1,4});(\d{1,4})M$/;
|
||||
const MIN_WHEEL_BUTTON = 64;
|
||||
const MAX_WHEEL_BUTTON = 95;
|
||||
const MAX_TERMINAL_COORDINATE = 9_999;
|
||||
|
||||
/** Return whether ``data`` is one bounded SGR wheel report from xterm. */
|
||||
export function isSgrWheelReport(data: string): boolean {
|
||||
const match = SGR_WHEEL_REPORT.exec(data);
|
||||
if (!match) return false;
|
||||
|
||||
const button = Number(match[1]);
|
||||
const column = Number(match[2]);
|
||||
const row = Number(match[3]);
|
||||
return (
|
||||
button >= MIN_WHEEL_BUTTON &&
|
||||
button <= MAX_WHEEL_BUTTON &&
|
||||
(button & 3) <= 1 &&
|
||||
column >= 1 &&
|
||||
column <= MAX_TERMINAL_COORDINATE &&
|
||||
row >= 1 &&
|
||||
row <= MAX_TERMINAL_COORDINATE
|
||||
);
|
||||
}
|
||||
114
dockerfiles/hermes-terminal-replay-regression.py
Normal file
114
dockerfiles/hermes-terminal-replay-regression.py
Normal file
@ -0,0 +1,114 @@
|
||||
"""Exercise the installed dashboard PTY reattachment contract without model calls."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import os
|
||||
from pathlib import Path
|
||||
import signal
|
||||
from types import SimpleNamespace
|
||||
import unittest
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
|
||||
ROOT = Path(os.environ.get("HERMES_SOURCE_ROOT", "/opt/hermes"))
|
||||
|
||||
|
||||
def load_source(name: str):
|
||||
"""Load one installed upstream module without starting the dashboard."""
|
||||
spec = importlib.util.spec_from_file_location(name, ROOT / "hermes_cli" / f"{name}.py")
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
bridge_module = load_source("pty_bridge")
|
||||
session_module = load_source("pty_session")
|
||||
|
||||
|
||||
class DashboardRedrawTests(unittest.TestCase):
|
||||
"""Only the opted-in, living dashboard child receives a redraw signal."""
|
||||
|
||||
def test_spawn_requires_both_dashboard_and_alternate_mode(self):
|
||||
for env, expected in (
|
||||
({}, False),
|
||||
({"HERMES_TUI_DASHBOARD": "1"}, False),
|
||||
({"HERMES_TUI_INLINE": "0"}, False),
|
||||
({"HERMES_TUI_DASHBOARD": "1", "HERMES_TUI_INLINE": "1"}, False),
|
||||
({"HERMES_TUI_DASHBOARD": "1", "HERMES_TUI_INLINE": "0"}, True),
|
||||
):
|
||||
with self.subTest(env=env):
|
||||
proc = SimpleNamespace(fd=12, pid=321, isalive=lambda: True)
|
||||
spawn = Mock(return_value=proc)
|
||||
backend = SimpleNamespace(PtyProcess=SimpleNamespace(spawn=spawn))
|
||||
with patch.object(bridge_module, "ptyprocess", backend), patch.object(
|
||||
bridge_module, "_PTY_AVAILABLE", True
|
||||
), patch.object(bridge_module.os, "kill") as kill, patch.object(
|
||||
bridge_module.os, "write"
|
||||
) as write:
|
||||
bridge = bridge_module.PtyBridge.spawn(["node", "entry.js"], env=env)
|
||||
bridge.redraw_after_attach()
|
||||
self.assertEqual(kill.call_count, int(expected))
|
||||
if expected:
|
||||
kill.assert_called_once_with(321, signal.SIGCONT)
|
||||
write.assert_not_called()
|
||||
|
||||
def test_dead_or_closed_children_are_not_signalled(self):
|
||||
for alive, closed in ((False, False), (True, True)):
|
||||
proc = SimpleNamespace(fd=12, pid=321, isalive=lambda: alive)
|
||||
bridge = bridge_module.PtyBridge(proc)
|
||||
bridge._dashboard_redraw_on_attach = True
|
||||
bridge._closed = closed
|
||||
with patch.object(bridge_module.os, "kill") as kill:
|
||||
bridge.redraw_after_attach()
|
||||
kill.assert_not_called()
|
||||
|
||||
def test_child_exit_race_does_not_break_attachment(self):
|
||||
proc = SimpleNamespace(fd=12, pid=321, isalive=lambda: True)
|
||||
bridge = bridge_module.PtyBridge(proc)
|
||||
bridge._dashboard_redraw_on_attach = True
|
||||
with patch.object(bridge_module.os, "kill", side_effect=ProcessLookupError):
|
||||
bridge.redraw_after_attach()
|
||||
|
||||
|
||||
class ReplayTests(unittest.IsolatedAsyncioTestCase):
|
||||
"""Reattachment restores the terminal after bounded replay completes."""
|
||||
|
||||
async def test_truncated_replay_is_followed_by_redraw_without_input(self):
|
||||
events = []
|
||||
bridge = SimpleNamespace(redraw_after_attach=lambda: events.append("redraw"))
|
||||
session = session_module.PtySession("test", bridge, buffer_cap=8, read_timeout=.1)
|
||||
|
||||
class Socket:
|
||||
async def send_bytes(self, data):
|
||||
events.append(data)
|
||||
|
||||
async def close(self, code):
|
||||
events.append(code)
|
||||
|
||||
first = Socket()
|
||||
await session.attach(first)
|
||||
self.assertEqual(events, [])
|
||||
session.detach(first)
|
||||
session.buffer.append(b"\x1b[?1049h" + b"x" * 64)
|
||||
self.assertTrue(session.buffer.truncated)
|
||||
self.assertEqual(session.buffer.snapshot(), b"x" * 8)
|
||||
second = Socket()
|
||||
await session.attach(second)
|
||||
self.assertEqual(events, [b"x" * 8, "redraw"])
|
||||
self.assertTrue(session.attached)
|
||||
# A superseded socket must not detach the active viewer.
|
||||
session.detach(first)
|
||||
self.assertTrue(session.attached)
|
||||
|
||||
async def test_legacy_bridges_still_reattach(self):
|
||||
session = session_module.PtySession("test", object(), buffer_cap=8, read_timeout=.1)
|
||||
first = SimpleNamespace()
|
||||
await session.attach(first)
|
||||
session.detach(first)
|
||||
await session.attach(SimpleNamespace())
|
||||
self.assertTrue(session.attached)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
92
dockerfiles/patch-hermes-dashboard-wheel.js
Normal file
92
dockerfiles/patch-hermes-dashboard-wheel.js
Normal file
@ -0,0 +1,92 @@
|
||||
/** Route only validated xterm wheel reports to an alternate-screen dashboard PTY. */
|
||||
|
||||
const fs = require("node:fs");
|
||||
const path = process.env.HERMES_CHAT_PAGE_PATH || "/opt/hermes/web/src/pages/ChatPage.tsx";
|
||||
let source = fs.readFileSync(path, "utf8");
|
||||
|
||||
function replaceOnce(before, after, label) {
|
||||
const count = source.split(before).length - 1;
|
||||
if (count !== 1) {
|
||||
throw new Error(`${label} patch context changed: expected 1, found ${count}`);
|
||||
}
|
||||
source = source.replace(before, after);
|
||||
}
|
||||
|
||||
replaceOnce(
|
||||
'import { api } from "@/lib/api";\n',
|
||||
'import { api } from "@/lib/api";\n' +
|
||||
'import { isSgrWheelReport } from "@/lib/dashboard-terminal-input";\n',
|
||||
"dashboard wheel input import",
|
||||
);
|
||||
replaceOnce(
|
||||
` // Dashboard chat should scroll the browser-side transcript, not send
|
||||
// mouse-wheel protocol bytes through the PTY.
|
||||
term.attachCustomWheelEventHandler((ev) => {
|
||||
const delta = ev.deltaY;
|
||||
if (!delta) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const step = Math.max(1, Math.round(Math.abs(delta) / 50));
|
||||
term.scrollLines(delta > 0 ? step : -step);
|
||||
|
||||
ev.preventDefault();
|
||||
ev.stopPropagation();
|
||||
return false;
|
||||
});
|
||||
|
||||
`,
|
||||
"",
|
||||
"browser-local wheel handler",
|
||||
);
|
||||
replaceOnce(
|
||||
` // Browser-embedded chat runs the TUI in inline mode. Keep transcript
|
||||
// history in xterm.js so the browser wheel can scroll it directly.
|
||||
scrollback: 5000,
|
||||
`,
|
||||
` // Alternate-screen PTYs keep their own visible history. Retain a
|
||||
// modest xterm scrollback only for terminal protocol compatibility.
|
||||
scrollback: 5000,
|
||||
`,
|
||||
"dashboard alternate-screen scrollback comment",
|
||||
);
|
||||
const inputCommentStart = source.indexOf(" // Keystrokes → PTY.");
|
||||
const inputCommentEnd = source.indexOf(
|
||||
" // eslint-disable-next-line no-control-regex",
|
||||
inputCommentStart,
|
||||
);
|
||||
if (inputCommentStart < 0 || inputCommentEnd < 0) {
|
||||
throw new Error("dashboard PTY input comment patch context changed");
|
||||
}
|
||||
source =
|
||||
source.slice(0, inputCommentStart) +
|
||||
` // Keystrokes and paste retain their existing PTY path. Mouse reports
|
||||
// are fail-closed below: only a bounded SGR wheel report can pass.
|
||||
` +
|
||||
source.slice(inputCommentEnd);
|
||||
replaceOnce(
|
||||
String.raw` // eslint-disable-next-line no-control-regex -- intentional ESC byte in xterm SGR mouse report parser
|
||||
const SGR_MOUSE_RE = /^\x1b\[<(\d+);(\d+);(\d+)([Mm])$/;
|
||||
onDataDisposable = term.onData((data) => {
|
||||
if (ws.readyState !== WebSocket.OPEN) return;
|
||||
|
||||
if (SGR_MOUSE_RE.test(data)) {
|
||||
return;
|
||||
}
|
||||
|
||||
ws.send(data);
|
||||
});
|
||||
`,
|
||||
String.raw` onDataDisposable = term.onData((data) => {
|
||||
if (ws.readyState !== WebSocket.OPEN) return;
|
||||
if (data.startsWith("\x1b[<") && !isSgrWheelReport(data)) return;
|
||||
if (data.startsWith("\x1b[M")) return;
|
||||
ws.send(data);
|
||||
});
|
||||
`,
|
||||
"SGR wheel relay",
|
||||
);
|
||||
if (source.includes("attachCustomWheelEventHandler")) {
|
||||
throw new Error("dashboard wheel interception remained after patch");
|
||||
}
|
||||
fs.writeFileSync(path, source);
|
||||
83
dockerfiles/patch-hermes-terminal-replay.py
Normal file
83
dockerfiles/patch-hermes-terminal-replay.py
Normal file
@ -0,0 +1,83 @@
|
||||
"""Restore dashboard terminal modes and visible history on PTY reattachment."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def patch_sources(root: Path) -> None:
|
||||
"""Patch the pinned upstream sources, rejecting drift before writing files."""
|
||||
patches = {
|
||||
"hermes_cli/pty_bridge.py": (
|
||||
(
|
||||
" self._closed = False\n",
|
||||
" self._closed = False\n"
|
||||
" self._dashboard_redraw_on_attach = False\n",
|
||||
),
|
||||
(
|
||||
" return cls(proc)\n",
|
||||
" bridge = cls(proc)\n"
|
||||
" bridge._dashboard_redraw_on_attach = (\n"
|
||||
" spawn_env.get(\"HERMES_TUI_DASHBOARD\") == \"1\"\n"
|
||||
" and spawn_env.get(\"HERMES_TUI_INLINE\") == \"0\"\n"
|
||||
" )\n"
|
||||
" return bridge\n",
|
||||
),
|
||||
(
|
||||
" # -- I/O --------------------------------------------------------------\n",
|
||||
''' def redraw_after_attach(self) -> None:
|
||||
"""Ask an opted-in dashboard TUI to restore its screen without input."""
|
||||
if not self._dashboard_redraw_on_attach or not self.is_alive():
|
||||
return
|
||||
try:
|
||||
# Ink's SIGCONT handler restores alternate-screen/mouse modes and
|
||||
# paints the current viewport, even after replay bytes were evicted.
|
||||
os.kill(self.pid, signal.SIGCONT)
|
||||
except OSError:
|
||||
# The child may exit between the liveness check and the signal.
|
||||
pass
|
||||
|
||||
# -- I/O --------------------------------------------------------------
|
||||
''',
|
||||
),
|
||||
),
|
||||
"hermes_cli/pty_session.py": (
|
||||
(
|
||||
" self.attached = False\n"
|
||||
" self.last_detached_at: Optional[float] = None\n",
|
||||
" self.attached = False\n"
|
||||
" self._has_attached = False\n"
|
||||
" self.last_detached_at: Optional[float] = None\n",
|
||||
),
|
||||
(
|
||||
" if snap:\n await ws.send_bytes(snap)\n",
|
||||
" if snap:\n await ws.send_bytes(snap)\n"
|
||||
" # A byte-bounded replay is not a complete terminal snapshot.\n"
|
||||
" # Restore modes and repaint only a reused dashboard TUI.\n"
|
||||
" if self._has_attached:\n"
|
||||
" redraw = getattr(self.bridge, \"redraw_after_attach\", None)\n"
|
||||
" if redraw is not None:\n"
|
||||
" redraw()\n"
|
||||
" self._has_attached = True\n",
|
||||
),
|
||||
),
|
||||
}
|
||||
prepared = {}
|
||||
for relative, replacements in patches.items():
|
||||
path = root / relative
|
||||
source = path.read_text()
|
||||
for before, after in replacements:
|
||||
if source.count(before) != 1:
|
||||
raise ValueError(f"Hermes terminal replay patch context changed: {relative}")
|
||||
source = source.replace(before, after, 1)
|
||||
compile(source, str(path), "exec")
|
||||
prepared[path] = source
|
||||
for path, source in prepared.items():
|
||||
path.write_text(source)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--root", type=Path, default=Path("/opt/hermes"))
|
||||
patch_sources(parser.parse_args().root)
|
||||
Loading…
x
Reference in New Issue
Block a user