Skip to content

Correct the baseY-is-zero rationale for alt-buffer agents (CROW-1020 follow-up) - #1022

Open
dgershman wants to merge 1 commit into
mainfrom
fix/crow-1020-baseY-comment
Open

Correct the baseY-is-zero rationale for alt-buffer agents (CROW-1020 follow-up)#1022
dgershman wants to merge 1 commit into
mainfrom
fix/crow-1020-baseY-comment

Conversation

@dgershman

Copy link
Copy Markdown
Collaborator

Comment-only follow-up to #1021. No behaviour change.

#1021's updateTerminalScrollbar explains its baseY > 0 gate like this:

baseY … is 0 in the alternate screen — which is where alt-buffer agents like Claude Code live

The gate is right. That reason is not, and it happens to be the exact trap crow-tmux.conf spends a paragraph warning about:

set -gas terminal-overrides ',xterm*:smcup@:rmcup@,screen*:smcup@:rmcup@'

That is why app.js must NOT route the hybrid scroll model on term.buffer.active.type === 'alternate': with this strip in place the web client is permanently in its main buffer, so that test is always false.

The strip cancels the client terminal's alt-screen capability, so the browser's xterm never enters its alternate buffer no matter what the tmux pane is doing. What actually zeroes baseY on a Claude Code surface is applySurfaceScrollback pinning options.scrollback = 0, which caps the main buffer at rows.

Left as-is this reads as license to rewrite the gate as buffer.active.type === 'alternate' — which is always false on this client, so the bar would then show on Claude Code tabs with a full-height slider and nothing to scroll. ADR-0013 already had to learn this once for the wheel routing (#824); the comment now says so rather than implying the opposite.

This missed #1021 by 27 minutes — the crow:merge watcher squashed the branch at 09:36 and the correction was pushed at 10:04, after the PR had already closed.

  • node --check on app.js: clean
  • npm run test:ci (web-tests): exit 0, 0 failures

🤖 Generated with Claude Code

The gate is right; the reason given for it was not. crow-tmux.conf strips
smcup/rmcup toward the CLIENT (`terminal-overrides ,xterm*:smcup@:rmcup@`),
so the web xterm is permanently in its main buffer — `buffer.active.type`
is never 'alternate' there, which is the same trap ADR-0013 calls out for
the wheel routing.

What actually zeroes baseY on a Claude Code surface is
applySurfaceScrollback pinning `options.scrollback = 0`, capping the main
buffer at `rows`. Comment only; no behaviour change.

🐦‍⬛ Generated with Claude Code, orchestrated by Crow

Co-Authored-By: Claude <noreply@anthropic.com>
Crow-Session: FE1DC79B-EC98-4859-B79D-CC30AB6FD455

@dhilgaertner dhilgaertner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code & Security Review

Critical Issues (if any)

None.

Security Review

Strengths:

  • Comment-only change; no runtime behavior, attack surface, or data-handling paths are modified.

Concerns:

  • None.

Code Quality

  • The corrected comment in Packages/CrowDaemon/Sources/CrowDaemon/Resources/web/app.js accurately documents why baseY > 0 hides the scrollbar on alt-buffer agents (Claude Code): the web xterm stays in its main buffer because crow-tmux.conf strips smcup/rmcup toward the client, so buffer.active.type === 'alternate' is never true here; applySurfaceScrollback pinning scrollback = 0 is what keeps baseY at zero. This aligns with ADR-0013 and the existing crow-tmux.conf commentary, and closes the trap that could invite rewriting the gate on buffer type.
  • The updateTerminalScrollbar gate (buf.baseY > 0) is unchanged and remains correct.
  • node --check on app.js: clean.
  • npm run test:ci (web-tests): exit 0, 0 failures (including terminal-scrollbar.test.js).

Summary Table

Color Meaning Verdict effect
Red Must fix Request changes
Yellow Should fix Request changes
Green Consider Approve allowed

Recommendation: Approve — driven by [0 Red, 0 Yellow, 1 Green] findings.


🐦‍⬛ Reviewed by Crow via Cursor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crow:merge Crow auto-merge on green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants