Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/web/public/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -4359,9 +4359,9 @@ class CodemanApp {
bufferWasEmpty = true;
}

// Buffer load complete — unblock live SSE writes. chunkedTerminalWrite calls
// _finishBufferLoad internally (discarding queued events to prevent duplicate
// content); if we skipped the write (cache hit or empty), call it here.
// Buffer load complete — unblock live SSE writes. selectSession owns this
// gate across every cache/fetch/replay stage, so its chunked writes leave
// the transaction open and we finish it exactly once here.
// COD-144: when the load painted nothing, FLUSH the queued events instead of
// discarding — a new session's prompt arrives only as a queued SSE event.
if (this._isLoadingBuffer) {
Expand Down
63 changes: 34 additions & 29 deletions src/web/public/terminal-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@
(function (global) {
const TERMINAL_QUERY_RESPONSE_PATTERN = /^\x1b\[[\?>=]?[\d;]*[cnR]$/;
const TERMINAL_OSC_RESPONSE_PATTERN = /^\x1b\][\d;]*[^\x07\x1b]*(?:\x07|\x1b\\)$/;
// Grace window after a manual scroll-up gesture during which sticky-scroll is
// suppressed, so high-frequency Codex status redraws don't snap the viewport
// back to the bottom while the user is inspecting earlier output.
const USER_SCROLL_STICKY_SUPPRESS_MS = 1500;
// Mobile browsers synthesize trusted mouse events after touchend. During this
// short window, only the app's synthetic tap-to-position mouse event should
// reach xterm.
Expand Down Expand Up @@ -60,7 +56,6 @@
global.CodemanTerminalInput = {
isTerminalQueryResponse,
shouldSuppressTerminalQueryResponse,
USER_SCROLL_STICKY_SUPPRESS_MS,
TOUCH_COMPAT_MOUSE_SUPPRESS_MS,
};
global.CODEMAN_XTERM_THEMES = CODEMAN_XTERM_THEMES;
Expand Down Expand Up @@ -403,8 +398,7 @@ Object.assign(CodemanApp.prototype, {
this._sendSyntheticSgrWheel(ev.clientX, ev.clientY, lines);
return;
}
this._noteTerminalUserScroll(lines);
this.terminal.scrollLines(lines);
this._scrollTerminalLines(lines);
},
{ passive: false }
);
Expand All @@ -429,7 +423,7 @@ Object.assign(CodemanApp.prototype, {
if (!isTouching && Math.abs(velocity) > 0.3) {
// Momentum phase — convert pixel velocity to lines
const lines = Math.round(velocity / cellHeight());
if (lines !== 0) this.terminal.scrollLines(lines);
if (lines !== 0) this._scrollTerminalLines(lines);
velocity *= 0.92;
scrollFrame = requestAnimationFrame(scrollLoop);
} else if (!isTouching) {
Expand Down Expand Up @@ -490,8 +484,7 @@ Object.assign(CodemanApp.prototype, {
const ch = cellHeight();
const lines = Math.trunc(pixelAccum / ch);
if (lines !== 0) {
this._noteTerminalUserScroll(lines);
this.terminal.scrollLines(lines);
this._scrollTerminalLines(lines);
pixelAccum -= lines * ch;
}
}
Expand Down Expand Up @@ -580,7 +573,7 @@ Object.assign(CodemanApp.prototype, {
this._chunkedWriteGen = 0;
this._bufferLoadSeq = 0;
this._bufferLoadOwner = null;
this._lastUserScrollUpAt = null;
this._terminalScrollLocked = false;

// Handle resize with throttling for performance
this._resizeTimeout = null;
Expand Down Expand Up @@ -1959,20 +1952,25 @@ Object.assign(CodemanApp.prototype, {
return buffer.viewportY >= buffer.baseY - 2;
},

// Record manual scroll gestures so sticky-scroll can give an upward scroll a
// short grace window (see _hasRecentUserScrollUp). A downward scroll that
// lands back at the bottom clears the suppression immediately.
// Keep history anchored after an upward gesture until the reader explicitly
// returns to the bottom. A timer is insufficient here: long-running terminal
// output used to reclaim the viewport after 1.5s while the user was reading.
_noteTerminalUserScroll(lines) {
if (lines < 0) {
this._lastUserScrollUpAt = performance.now();
this._terminalScrollLocked = true;
} else if (this.isTerminalAtBottom()) {
this._lastUserScrollUpAt = null;
this._terminalScrollLocked = false;
}
},

_hasRecentUserScrollUp() {
if (typeof this._lastUserScrollUpAt !== 'number') return false;
return performance.now() - this._lastUserScrollUpAt < window.CodemanTerminalInput.USER_SCROLL_STICKY_SUPPRESS_MS;
_scrollTerminalLines(lines) {
if (!lines || !this.terminal) return;
this.terminal.scrollLines(lines);
this._noteTerminalUserScroll(lines);
},

_shouldPreserveTerminalScroll() {
return this._terminalScrollLocked === true;
},

batchTerminalWrite(data) {
Expand Down Expand Up @@ -2176,10 +2174,12 @@ Object.assign(CodemanApp.prototype, {
const activeSession = this.activeSessionId && this.sessions ? this.sessions.get(this.activeSessionId) : null;
const MAX_FRAME_BYTES = activeSession?.mode === 'codex' ? 32768 : 65536;
let deferred = false;
// If the user recently scrolled up, remember the viewport so we can restore
// it after the write — Codex status redraws would otherwise jump it.
// While the reader owns history, remember the viewport so Codex status
// redraws cannot move it.
const preserveViewportY =
this._hasRecentUserScrollUp() && this.terminal.buffer?.active ? this.terminal.buffer.active.viewportY : null;
this._shouldPreserveTerminalScroll() && this.terminal.buffer?.active
? this.terminal.buffer.active.viewportY
: null;

if (_joinedLen <= MAX_FRAME_BYTES) {
this.terminal.write(joined);
Expand Down Expand Up @@ -2211,10 +2211,8 @@ Object.assign(CodemanApp.prototype, {
);

// Sticky scroll: if user was at bottom, keep them there after new output.
// Give manual scroll-up gestures a short grace window so high-frequency
// Codex status ticks do not snap the viewport back while the user is
// trying to inspect earlier output.
if (this._wasAtBottomBeforeWrite && !this._hasRecentUserScrollUp()) {
// A manual history scroll owns the viewport until it reaches the bottom.
if (this._wasAtBottomBeforeWrite && !this._shouldPreserveTerminalScroll()) {
this.terminal.scrollToBottom();
}

Expand Down Expand Up @@ -2376,11 +2374,18 @@ Object.assign(CodemanApp.prototype, {
// Generation counter: if a newer chunkedTerminalWrite starts (tab switch),
// older writes abort instead of continuing to push stale data into the terminal.
const writeGen = ++this._chunkedWriteGen;
const bufferLoadOwner = this._beginBufferLoad(loadOwner);
// A caller-provided owner means a wider operation (selectSession) already
// owns the live-output gate. Do not close that transaction after this one
// replay; the caller still has resize/fetch/reconciliation work to finish.
const ownsBufferLoad = loadOwner == null;
const bufferLoadOwner = ownsBufferLoad ? this._beginBufferLoad() : loadOwner;
const finishOwnedBufferLoad = () => {
if (ownsBufferLoad) this._finishBufferLoad(bufferLoadOwner);
};

return new Promise((resolve) => {
if (!buffer || buffer.length === 0) {
this._finishBufferLoad(bufferLoadOwner);
finishOwnedBufferLoad();
resolve();
return;
}
Expand All @@ -2392,7 +2397,7 @@ Object.assign(CodemanApp.prototype, {
const finish = () => {
// Only finish if we're still the active write — a newer write owns buffer load state
if (this._chunkedWriteGen === writeGen) {
this._finishBufferLoad(bufferLoadOwner);
finishOwnedBufferLoad();
}
resolve();
};
Expand Down
38 changes: 35 additions & 3 deletions test/terminal-flush-budget.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,20 @@ describe('terminal flush budget', () => {
expect(finishBufferLoad).toHaveBeenCalledOnce();
});

it('leaves a caller-owned buffer load open after replaying a chunk', async () => {
const { app } = loadTerminalUiHarness('codex');
const beginBufferLoad = vi.fn(() => 'nested-owner');
const finishBufferLoad = vi.fn();
app._beginBufferLoad = beginBufferLoad;
app._finishBufferLoad = finishBufferLoad;
app.terminal.write = (_data: string, callback?: () => void) => callback?.();

await app.chunkedTerminalWrite('cached frame', 32 * 1024, 'select-owner');

expect(beginBufferLoad).not.toHaveBeenCalled();
expect(finishBufferLoad).not.toHaveBeenCalled();
});

it('keeps stale buffer load owners from finishing a newer load', () => {
const { app } = loadTerminalUiHarness('codex');

Expand All @@ -113,12 +127,12 @@ describe('terminal flush budget', () => {
expect(app._bufferLoadOwner).toBe(null);
});

it('does not snap back to bottom during Codex Working redraws right after the user scrolls up', () => {
it('does not snap back to bottom during Codex Working redraws while history is scroll-locked', () => {
const { app } = loadTerminalUiHarness('codex');
const scrollToBottom = vi.fn();
app.terminal.scrollToBottom = scrollToBottom;
app._wasAtBottomBeforeWrite = true;
app._lastUserScrollUpAt = 0;
app._terminalScrollLocked = true;
app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');

app.flushPendingWrites();
Expand All @@ -137,11 +151,29 @@ describe('terminal flush budget', () => {
buffer.viewportY = line;
});
app._wasAtBottomBeforeWrite = true;
app._lastUserScrollUpAt = 0;
app._terminalScrollLocked = true;
app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');

app.flushPendingWrites();

expect(buffer.viewportY).toBe(40);
});

it('keeps the history lock until a downward scroll actually reaches the bottom', () => {
const { app } = loadTerminalUiHarness('codex');
const buffer = { viewportY: 40, baseY: 50 };
app.terminal.buffer = { active: buffer };
app.terminal.scrollLines = vi.fn((lines: number) => {
buffer.viewportY = Math.max(0, Math.min(buffer.baseY, buffer.viewportY + lines));
});

app._scrollTerminalLines(-2);
expect(app._shouldPreserveTerminalScroll()).toBe(true);

app._scrollTerminalLines(2);
expect(app._shouldPreserveTerminalScroll()).toBe(true);

app._scrollTerminalLines(20);
expect(app._shouldPreserveTerminalScroll()).toBe(false);
});
});