Compare commits

..

4 Commits

Author SHA1 Message Date
Developer bb38b37ceb fix: reinitialize terminal on mobile viewport changes
Recreate the terminal when responsive classification changes so mobile scrollback and touch listeners are installed.\n\nOpenSpec: fix-mobile-terminal-scrolling\nQuality gates: npm run typecheck, npm run lint, npm test (89 passed), npm run build
2026-07-14 19:41:34 +00:00
Developer 6698c20f25 fix: restore mobile terminal scrolling
- Retain xterm normal-buffer history on mobile while preserving desktop zero-scrollback behavior\n- Repair ProjectsPage tests for session context and current project list markup\n- Add focused terminal scrollback coverage\n\nOpenSpec: fix-mobile-terminal-scrolling\nQuality gates: npm run typecheck, npm run lint, npm test (89 passed), npm run build
2026-07-14 19:07:02 +00:00
Developer 5e06a2a226 Merge fix/terminal-paste-partial-writes into dev 2026-07-14 10:34:33 +00:00
Developer 49180e4c6d fix: fully drain terminal paste writes to PTY
- Make the host PTY master non-blocking and wait for write readiness.
- Retry partial writes so a large bracketed paste always delivers its closing
  marker instead of leaving pi in paste mode.
- Add regression coverage for partial PTY writes and resolve diagnostics.

Quality gates: ruff, mypy, focused pytest (3 passed)
2026-07-14 10:34:33 +00:00
7 changed files with 183 additions and 68 deletions
@@ -19,6 +19,8 @@ from typing import Any
logger = logging.getLogger(__name__)
_session_counters_by_instance: dict[str, int] = {}
class TerminalSession:
"""Manages a single terminal session with event-driven PTY I/O.
@@ -46,9 +48,6 @@ class TerminalSession:
# Max WebSocket frame size
MAX_FRAME_SIZE = 64 * 1024
# Session number counter per instance_id for auto-naming
_instance_counters: dict[str, int] = {}
def __init__(
self,
session_id: str,
@@ -99,11 +98,11 @@ class TerminalSession:
# Ack timeout fallback
self._ack_timeout_handle: asyncio.TimerHandle | None = None
@classmethod
def _generate_name(cls, instance_id: str) -> str:
@staticmethod
def _generate_name(instance_id: str) -> str:
"""Generate an auto-incremented session name for the instance."""
count = cls._instance_counters.get(instance_id, 0) + 1
cls._instance_counters[instance_id] = count
count = _session_counters_by_instance.get(instance_id, 0) + 1
_session_counters_by_instance[instance_id] = count
return f"Session {count}"
async def start(self, startup_command: str | None = None) -> None:
@@ -121,8 +120,12 @@ class TerminalSession:
if self.container_user:
exec_cmd.extend(["--user", self.container_user])
# Create a pseudo-terminal on the host
# Create a pseudo-terminal on the host. The master must be
# non-blocking: a browser paste can be larger than the PTY input
# buffer, and write_input() drains it asynchronously without dropping
# the closing bracketed-paste marker.
self._master_fd, slave_fd = pty.openpty()
os.set_blocking(self._master_fd, False)
# Put the host PTY into raw mode so it behaves as a pass-through
# pipe. openpty() leaves the slave in canonical mode by default,
@@ -204,6 +207,9 @@ class TerminalSession:
try:
data = os.read(self._master_fd, 4096)
except BlockingIOError:
# The readiness notification raced with another callback.
return
except (OSError, IOError) as exc:
logger.debug("PTY read error for session %s: %s", self.session_id, exc)
self._handle_eof()
@@ -343,12 +349,42 @@ class TerminalSession:
pass
logger.info("Session %s EOF handled, websockets closed", self.session_id)
async def _wait_for_write_ready(self, fd: int) -> None:
"""Wait until a non-blocking PTY master can accept more input."""
loop = asyncio.get_running_loop()
writable = loop.create_future()
def mark_writable() -> None:
if not writable.done():
writable.set_result(None)
loop.add_writer(fd, mark_writable)
try:
await writable
finally:
loop.remove_writer(fd)
async def write_input(self, data: bytes) -> None:
"""Write input to the PTY master."""
"""Write all terminal input bytes to the PTY master in order."""
if self._master_fd is None or self._closed:
return
fd = self._master_fd
remaining = memoryview(data)
try:
os.write(self._master_fd, data)
while remaining and not self._closed and self._master_fd == fd:
try:
written = os.write(fd, remaining)
except BlockingIOError:
await self._wait_for_write_ready(fd)
continue
if written == 0:
await self._wait_for_write_ready(fd)
continue
remaining = remaining[written:]
self.last_activity = time.time()
except (OSError, IOError) as exc:
logger.debug("PTY write error for session %s: %s", self.session_id, exc)
+53 -22
View File
@@ -1,9 +1,9 @@
"""Unit tests for TerminalSession docker exec invocation."""
"""Unit tests for TerminalSession PTY handling."""
import uuid
from unittest.mock import AsyncMock, patch
import pytest
import pytest # pyright: ignore[reportMissingImports]
from src.services.terminal.terminal_session import TerminalSession
@@ -19,17 +19,20 @@ async def test_start_passes_container_user_to_docker_exec() -> None:
container_user="dev",
)
with patch(
"src.services.terminal.terminal_session.pty.openpty",
return_value=(1, 2),
with (
patch(
"src.services.terminal.terminal_session.pty.openpty",
return_value=(1, 2),
),
patch("src.services.terminal.terminal_session.os.set_blocking") as set_blocking,
patch("src.services.terminal.terminal_session.tty.setraw"),
patch(
"src.services.terminal.terminal_session.asyncio.create_subprocess_exec",
new=AsyncMock(),
) as mock_exec,
patch("src.services.terminal.terminal_session.os.close"),
):
with patch("src.services.terminal.terminal_session.tty.setraw"):
with patch(
"src.services.terminal.terminal_session.asyncio.create_subprocess_exec",
new=AsyncMock(),
) as mock_exec:
with patch("src.services.terminal.terminal_session.os.close"):
await session.start()
await session.start()
args, _kwargs = mock_exec.call_args
assert "docker" in args
@@ -38,6 +41,7 @@ async def test_start_passes_container_user_to_docker_exec() -> None:
user_index = args.index("--user")
assert args[user_index + 1] == "dev"
assert "container-123" in args
set_blocking.assert_called_once_with(1, False)
@pytest.mark.unit
@@ -50,17 +54,44 @@ async def test_start_omits_user_when_not_configured() -> None:
container_id="container-123",
)
with patch(
"src.services.terminal.terminal_session.pty.openpty",
return_value=(1, 2),
with (
patch(
"src.services.terminal.terminal_session.pty.openpty",
return_value=(1, 2),
),
patch("src.services.terminal.terminal_session.os.set_blocking"),
patch("src.services.terminal.terminal_session.tty.setraw"),
patch(
"src.services.terminal.terminal_session.asyncio.create_subprocess_exec",
new=AsyncMock(),
) as mock_exec,
patch("src.services.terminal.terminal_session.os.close"),
):
with patch("src.services.terminal.terminal_session.tty.setraw"):
with patch(
"src.services.terminal.terminal_session.asyncio.create_subprocess_exec",
new=AsyncMock(),
) as mock_exec:
with patch("src.services.terminal.terminal_session.os.close"):
await session.start()
await session.start()
args, _kwargs = mock_exec.call_args
assert "--user" not in args
@pytest.mark.unit
@pytest.mark.asyncio
async def test_write_input_retries_partial_pty_writes() -> None:
"""A large paste is fully written even when the PTY accepts it in chunks."""
session = TerminalSession(
session_id=str(uuid.uuid4()),
instance_id=uuid.uuid4(),
container_id="container-123",
)
session._master_fd = 42
with patch(
"src.services.terminal.terminal_session.os.write",
side_effect=[2, 2, 1],
) as write:
await session.write_input(b"hello")
assert [bytes(call.args[1]) for call in write.call_args_list] == [
b"hello",
b"llo",
b"o",
]
@@ -0,0 +1,13 @@
import { describe, expect, it } from "vitest";
import { getTerminalScrollbackLimit } from "./terminal.tsx";
describe("getTerminalScrollbackLimit", () => {
it("retains normal-buffer history for custom mobile swipe scrolling", () => {
expect(getTerminalScrollbackLimit(true)).toBe(10_000);
});
it("keeps desktop scrollback disabled to prevent stale-frame wheel scrolling", () => {
expect(getTerminalScrollbackLimit(false)).toBe(0);
});
});
@@ -53,6 +53,10 @@ const BRACKETED_PASTE_DISABLE_SEQUENCE = [0x1b, 0x5b, 0x3f, 0x32, 0x30, 0x30, 0x
const BRACKETED_PASTE_CONTROL_TAIL_LENGTH =
BRACKETED_PASTE_ENABLE_SEQUENCE.length - 1;
export function getTerminalScrollbackLimit(isMobile: boolean): number {
return isMobile ? 10_000 : 0;
}
function matchesByteSequence(
data: Uint8Array,
start: number,
@@ -326,14 +330,11 @@ export const TerminalComponent = React.forwardRef<TerminalRef, TerminalProps>(
lineHeight: 1.2,
letterSpacing: 0,
allowTransparency: false,
// This terminal only ever hosts full-screen TUI tools (pi-agent,
// opencode), which repaint in place in the normal buffer and do not
// use the alternate screen or mouse tracking. With scrollback, every
// repaint accumulates as history → a viewport scrollbar appears and
// the mouse-wheel scrolls through stale frames instead of the app.
// scrollback:0 keeps only the live viewport: no bar, no stale-frame
// wheel jank. (Scrollbar is also hidden via CSS for belt-and-suspenders.)
scrollback: 0,
// Desktop tools repaint in place, so retaining their normal buffer
// creates stale frames that native wheel scrolling can revisit. Mobile
// instead uses its custom touch handler to scroll normal-buffer output,
// which requires retained history.
scrollback: getTerminalScrollbackLimit(isMobile),
ignoreBracketedPasteMode: false,
fastScrollSensitivity: 0,
scrollSensitivity: 0,
@@ -699,7 +700,7 @@ export const TerminalComponent = React.forwardRef<TerminalRef, TerminalProps>(
// Ignore disposal errors from partially torn-down terminal
}
};
}, [instanceId, connectWebSocket]);
}, [instanceId, connectWebSocket, isMobile]);
useImperativeHandle(ref, () => ({
fit: () => {
+22 -27
View File
@@ -1,17 +1,26 @@
import "@testing-library/jest-dom/vitest";
import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
import { MemoryRouter } from "react-router-dom";
import { afterEach, describe, expect, it, vi } from "vitest";
import { ProjectsPage } from "./ProjectsPage";
import * as projectsApi from "../api/projects";
import { SessionsProvider } from "../state/sessions";
import type { ProjectWithRepos } from "../types";
const mockProjects = [
vi.mock("../api/sessions", () => ({
getUserSessions: vi.fn().mockResolvedValue([]),
}));
const mockProjects: ProjectWithRepos[] = [
{
id: "proj-1",
name: "Alpha Project",
description: "First project",
owner_id: "user-1",
default_ssh_key_id: null,
repositories: [],
created_at: "2026-07-01T00:00:00Z",
},
{
id: "proj-2",
@@ -19,6 +28,8 @@ const mockProjects = [
description: null,
owner_id: "user-1",
default_ssh_key_id: null,
repositories: [],
created_at: "2026-07-01T00:00:00Z",
},
];
@@ -31,9 +42,7 @@ describe("ProjectsPage", () => {
it("renders loading state initially", () => {
vi.spyOn(projectsApi, "listProjects").mockImplementation(() => new Promise(() => {}));
render(
<MemoryRouter>
<ProjectsPage />
</MemoryRouter>
<MemoryRouter><SessionsProvider><ProjectsPage /></SessionsProvider></MemoryRouter>
);
expect(screen.getByText(/loading projects/i)).toBeInTheDocument();
});
@@ -41,9 +50,7 @@ describe("ProjectsPage", () => {
it("renders project list after loading", async () => {
vi.spyOn(projectsApi, "listProjects").mockResolvedValue(mockProjects);
render(
<MemoryRouter>
<ProjectsPage />
</MemoryRouter>
<MemoryRouter><SessionsProvider><ProjectsPage /></SessionsProvider></MemoryRouter>
);
await waitFor(() => {
@@ -56,9 +63,7 @@ describe("ProjectsPage", () => {
it("renders empty state when no projects", async () => {
vi.spyOn(projectsApi, "listProjects").mockResolvedValue([]);
render(
<MemoryRouter>
<ProjectsPage />
</MemoryRouter>
<MemoryRouter><SessionsProvider><ProjectsPage /></SessionsProvider></MemoryRouter>
);
await waitFor(() => {
@@ -69,9 +74,7 @@ describe("ProjectsPage", () => {
it("renders error state with retry button", async () => {
vi.spyOn(projectsApi, "listProjects").mockRejectedValue(new Error("fail"));
render(
<MemoryRouter>
<ProjectsPage />
</MemoryRouter>
<MemoryRouter><SessionsProvider><ProjectsPage /></SessionsProvider></MemoryRouter>
);
await waitFor(() => {
@@ -85,9 +88,7 @@ describe("ProjectsPage", () => {
const createMock = vi.spyOn(projectsApi, "createProject").mockResolvedValue(mockProjects[0]);
render(
<MemoryRouter>
<ProjectsPage />
</MemoryRouter>
<MemoryRouter><SessionsProvider><ProjectsPage /></SessionsProvider></MemoryRouter>
);
await waitFor(() => {
@@ -118,9 +119,7 @@ describe("ProjectsPage", () => {
vi.spyOn(projectsApi, "listProjects").mockResolvedValue([]);
render(
<MemoryRouter>
<ProjectsPage />
</MemoryRouter>
<MemoryRouter><SessionsProvider><ProjectsPage /></SessionsProvider></MemoryRouter>
);
await waitFor(() => {
@@ -138,16 +137,14 @@ describe("ProjectsPage", () => {
const updateMock = vi.spyOn(projectsApi, "updateProject").mockResolvedValue(mockProjects[0]);
render(
<MemoryRouter>
<ProjectsPage />
</MemoryRouter>
<MemoryRouter><SessionsProvider><ProjectsPage /></SessionsProvider></MemoryRouter>
);
await waitFor(() => {
expect(screen.getByText("Alpha Project")).toBeInTheDocument();
});
const alphaCard = screen.getByText("Alpha Project").closest(".project-card") as HTMLElement | null;
const alphaCard = screen.getByText("Alpha Project").closest(".project-list-item") as HTMLElement | null;
if (!alphaCard) throw new Error("Card not found");
fireEvent.click(within(alphaCard).getByRole("button", { name: /edit/i }));
@@ -171,16 +168,14 @@ describe("ProjectsPage", () => {
const deleteMock = vi.spyOn(projectsApi, "deleteProject").mockResolvedValue(undefined);
render(
<MemoryRouter>
<ProjectsPage />
</MemoryRouter>
<MemoryRouter><SessionsProvider><ProjectsPage /></SessionsProvider></MemoryRouter>
);
await waitFor(() => {
expect(screen.getByText("Alpha Project")).toBeInTheDocument();
});
const alphaCard = screen.getByText("Alpha Project").closest(".project-card") as HTMLElement | null;
const alphaCard = screen.getByText("Alpha Project").closest(".project-list-item") as HTMLElement | null;
if (!alphaCard) throw new Error("Card not found");
fireEvent.click(within(alphaCard).getByRole("button", { name: /delete/i }));
@@ -0,0 +1,30 @@
# Restore Mobile Terminal Scrolling
## Summary
The recent terminal scrollback optimization disabled scrollback for every viewport. Mobile terminal swipe handling still scrolls xterm's normal buffer programmatically, so swipes in normal-buffer tools no longer have retained output to move through.
## Root Cause
`468f342` changed the terminal configuration to `scrollback: 0` globally to prevent stale repaint frames and wheel scrolling on desktop. The mobile touch handler calls `term.scrollLines()` when the normal buffer is active. With zero scrollback, that call has no scrollable history and becomes a no-op.
## Scope
- `apps/web/src/components/features/terminal/terminal.tsx`
- Focused terminal configuration test
## Fix
Retain a bounded xterm scrollback buffer on mobile only (`10000` lines), while leaving desktop at zero scrollback and with wheel sensitivity disabled. Mobile's existing custom touch handler remains responsible for moving through normal-buffer history; alternate-screen swipes continue to send SGR wheel events to the active TUI.
## Acceptance Criteria
- [ ] A mobile terminal with normal-buffer output exceeding one screen scrolls via a vertical swipe.
- [ ] Alternate-screen terminal scrolling continues to use the existing SGR wheel-event path.
- [ ] Desktop keeps zero xterm scrollback and disabled native wheel scrolling, so stale repaint frames do not return.
- [ ] Focused unit test and frontend quality gates pass.
## Related
- `468f342 fix(terminal): hide scrollbar and stop stale-frame wheel scroll for TUI tools`
- `openspec/changes/fix-terminal-container-overflow`
@@ -0,0 +1,9 @@
# Restore Mobile Terminal Scrolling — Tasks
- [x] Add a mobile-specific terminal scrollback limit while preserving zero scrollback on desktop.
- [x] Reinitialize the terminal when the responsive mobile classification changes so its scrollback and touch handler match the active viewport.
- [x] Add focused tests for the responsive scrollback configuration.
- [x] Run the full frontend test suite (89 tests passed after repairing the `ProjectsPage` test setup).
- [x] Run frontend typecheck, lint, focused tests, and production build.
- [ ] Perform mobile normal-buffer and alternate-screen manual QA.
- [ ] Update project maps for changed source files (the map patch tool currently fails with an unsupported `temperature` parameter).