Skip to content

fix(ui): stop the install progress bar from garbling non-scroll-region terminals#155

Merged
fullstackjam merged 1 commit into
mainfrom
claude/npm-install-terminal-ui-79274a
Jul 17, 2026
Merged

fix(ui): stop the install progress bar from garbling non-scroll-region terminals#155
fullstackjam merged 1 commit into
mainfrom
claude/npm-install-terminal-ui-79274a

Conversation

@fullstackjam

Copy link
Copy Markdown
Member

What does this PR do?

Removes the scroll-region "sticky" progress bar; the install log now renders top-to-bottom on every terminal.

Why?

The install progress bar (StickyProgress) reserved the bottom rows as a frozen status bar using a DECSTBM scroll region (\x1b[1;Nr). On terminals that report a normal TERM but don't actually honour scroll regions, every newline scrolls the whole screen, so the reserved divider+status bar got dragged into the log and reprinted on every 80ms tick — the install read as repeated, overprinted garbage instead of clean top-to-bottom output.

It was most visible during npm (its per-package installs finish fast, so the bar redraws rapidly), but the code is shared with brew, so both were affected. IsScrollRegionSupported() only checked TTY + TERM + terminal height — it never verified real scroll-region support, so it couldn't tell a supporting terminal from a non-supporting one.

The fix drops the scroll-region path entirely and always draws a plain in-place \r\033[K status line that trails the output. Each package prints on its own line, in order, with one live spinner line at the bottom — on every terminal. This deletes scrollregion.go and its whole failure mode (net −262 lines).

Testing

  • go vet ./... passes
  • Relevant tests added or updated (removed the now-obsolete scroll-region tests; kept a Start/Finish smoke test)
  • Reproduced the corruption and verified the fix by driving the real StickyProgress code through a PTY and replaying the byte stream in a terminal emulator (pyte):
    • On a scroll-region-ignoring terminal: dividers rained through the output before the fix; clean top-to-bottom after.
    • On a compliant terminal: clean before and after (no regression), at 24×40, 40×100, and with long wrapping error lines.
  • Full make test-unit (L1, incl. archtest) green; e2e/vm-tagged build compiles.

Cross-repo checklist

  • Does this need a docs/content update in openboot.dev? — No.
  • Does this change the CLI ↔ server API contract? — No.

Notes for reviewer

  • Docs updated to match: docs/HARNESS.md "What's intentionally NOT in the harness" now records why the scroll region was removed and says not to reintroduce it; internal/ui/AGENTS.md drops the scrollregion.go row.
  • Out of scope, flagged separately: npmStepDone counts npm failures as "installed" in the summary line (bar.Increment() instead of IncrementWithStatus(ok), unlike brew). That's a counting bug, not this rendering one, so it's left for a follow-up.

…minals

StickyProgress reserved the bottom rows as a sticky status bar via a DECSTBM
scroll region. On terminals that report a normal TERM but don't honour scroll
regions, every newline scrolled the whole screen, so the reserved bar was
dragged into the log and reprinted on every tick — the install read as
repeated, overprinted garbage. Most visible during npm, whose fast
per-package installs redraw the bar rapidly.

IsScrollRegionSupported() only checked TTY + TERM + height; it could not
detect actual scroll-region support, so it enabled the fragile path on the
terminals that break it. Replace it with a plain in-place \r\033[K status
line that trails top-to-bottom output on every terminal, and remove the
scroll-region plumbing (scrollregion.go) and its failure mode entirely.
@github-actions github-actions Bot added tests Tests only ui Terminal UI docs labels Jul 17, 2026
@fullstackjam
fullstackjam enabled auto-merge (squash) July 17, 2026 16:39
@fullstackjam
fullstackjam merged commit 88a179c into main Jul 17, 2026
13 checks passed
@fullstackjam
fullstackjam deleted the claude/npm-install-terminal-ui-79274a branch July 17, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs tests Tests only ui Terminal UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant