Keep at least two shells visible in the Terminal rail, and let its sections resize - #2301
Conversation
…ctions resize The Terminal tab's left rail stacked Shells, Agent tasks, Background tasks and Ports as fixed-shrink blocks with percentage max-heights that never resolved, so a long agent-task list pushed the Shells list down to a sliver: the open terminals became invisible while the rail filled with task rows. Every section is now a `terminal-rail-section` sharing the rail's height. A section's minimum is its header plus two rows, and Shells is additionally held to at least a third of the rail, so at least two terminals stay visible however many tasks or ports are listed. Divider handles between visible sections drag (or ArrowUp/ArrowDown when focused) to rebalance adjacent sections without crossing either minimum; handles hide when a neighbour is hidden. The rail scrolls rather than crushing sections when the minimums exceed its height. Ported from an uncommitted worktree on the old lineage; unit tests cover the minimums, handle visibility and pointer/keyboard resizing, and the supervised tasks e2e spec now also drags the two handles and checks each section holds its minimum. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
🖥️ PR preview
|
jonathanKingston
left a comment
There was a problem hiding this comment.
Review: terminal rail resizer
Verdict: changes needed. The resizer hangs the renderer on boot, which is why all 16 app-loading demo specs in the build job timed out. Small fix, reproduced end to end.
Blocking: infinite MutationObserver loop
src/renderer/views/terminal-rail-resizer.ts:161-165 and :175-180. sync() assigns handle.hidden = section.hidden || before === null on every call, and the MutationObserver on root (subtree: true, attributeFilter: ['hidden']) sees the handles because they are children of root. Per the DOM spec, setting hidden to true when it is already true still queues a mutation record ("change an attribute" always enqueues), while hidden = false on an absent attribute does not. So as soon as any section is hidden at mount, sync reschedules itself forever on the microtask queue. In production that is every boot: mountSupervisedTasks (supervised-tasks.ts:17-20) and mountPortsSection (ports-section.ts:66-69) both create their section with hidden: true, and Agent tasks hides itself when empty.
Evidence:
- Minimal page with the compiled resizer and two hidden sections: headless Chromium never finishes and is killed by the timeout. The same page with no hidden sections finishes instantly.
- PR demo build (
pnpm run build && pnpm run build:demo, served fromdist/demo):/?scenario=chat-layout-stylingand/?scenario=settings-footerboth time out onpage.gotoat 20 s and onpage.evaluate('1+1')at 10 s. With the guard below and onlybuild:demorerun, the same scenarios load in about 5 s with 4 sections, 3 handles, 2 hidden handles, and the prompt input present. - A spy observer counting
hiddenrecords after mount reports 52 (guard tripped) on the PR code and 2 with the fix.
Minimal fix:
const hidden = section.hidden || before === null
if (handle.hidden !== hidden) handle.hidden = hiddenObserving the sections individually instead of root with subtree would also work; the guard is smaller and cheaper.
Please also add a regression test. The current tests miss this because records are delivered on a microtask and the first test calls destroy() synchronously, disconnecting the observer before delivery; the second test has no hidden section. happy-dom does deliver same-value records (probe: two same-value writes produced 2 records), so a test that mounts with a hidden section, awaits a setTimeout, and counts hidden mutation records via a spy observer with a guard that calls destroy() above about 50 records (so it fails instead of hanging the runner) catches it: 52 today, 2 with the fix.
Validation
| Check | Result |
|---|---|
pnpm test -- terminal-rail-resizer agent-tasks ports-section supervised-tasks |
31/31 pass |
pnpm run build && pnpm run build:demo |
pass |
| Demo scenarios on the PR build | hang |
| Demo scenarios on the patched build | responsive |
Dry merge with origin/main (2 behind) |
clean |
Non-blocking
- Screenshot evidence.
tests/e2e/supervised-tasks.e2e.tscallssaveAppScreenshot('supervised-tasks-waiting.png')after the two drags, so the reference will change, but no PNG is in the diff because e2e never ran. Commit the candidate from thereference-screenshot-candidates-<run-id>artifact once CI is green. The spec logic itself looks right:__copseE2e.setPortRowsexists behindCOPSE_E2E=1(preload/index.ts:1306,register-handlers.ts:2615), Agent tasks is hidden with no tasks so "Resize Shells and Background tasks" is the correct label, anddragAndDrop({x, y})has precedent inmonaco-selection-chat.e2e.ts:48. sectionMinimumwrites 8px minimums while#terminals-list-hostis stillhidden(index.html:63; all rects are 0). The ResizeObserver repairs this when the host is shown, but an early return onroot.clientHeight === 0would be cleaner.- The focusable
role="separator"should exposearia-valuenow,aria-valueminandaria-valuemax. Thearia-labelis also left stale whenbeforeis null, harmless because the handle is hidden then. - After a drag, visible sections carry
flex: <px> 1 0pxwhile a section shown later keeps the stylesheetflex: 1 1 33.333%. It works, but consider resettingstyle.flexwhen a section transitions hidden to visible. - Empty sections get a header-plus-padding minimum since
measuredRowHeight(:17-22) falls back to a missing first child. Fine given empty sections are hidden and Shells has the one-third floor; a comment would help.
ResizeObserver feedback checked and not a loop: the host is flex: 1; min-height: 0; overflow-y: auto, so child min-height writes change scrollHeight only, and sync is idempotent once a scrollbar appears.
Generated by Claude Code
…erver `sync()` wrote `handle.hidden` unconditionally on every run. The resizer's MutationObserver watches `root` with `subtree: true` and an `attributeFilter` of `['hidden']`, and the handles are children of `root`, so every write was observed — and setting `hidden` to the value it already has still queues a mutation record. As soon as any section started hidden (Background tasks and Ports do), the first sync hid its handle, the observer re-ran sync, sync hid it again, and the renderer looped forever on the microtask queue and hung on boot. Now the handle is only written when its visibility actually changes. Also skip writing section minimums while the host measures at 0px (still hidden) so meaningless 8px minimums are not written, and note at `measuredRowHeight` why an empty section collapsing to header + padding is fine. The new test mounts the rail with hidden sections, waits a macrotask so the mutation records are delivered, and counts `hidden` records with a spy observer; a guard tears the resizer down above 50 records so a regression fails instead of hanging the runner. It fails on the previous code (52 records, cut off by the guard) and passes with the fix (at most 2). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011jaQbed96h8aSZLijfiHNB
|
Pushed 6245525 with the fix from the review: Generated by Claude Code |
Reference screenshotsCI run 33917334411 published no changed screenshot candidates for |
Screenshot candidates rendered for parent PR #2301 at `6245525db58d0744e88542672581ca768c05f437` by [CI run 33859423498](https://github.com/copse-dev/agent-pane/actions/runs/33859423498). Review GitHub's image diffs, then merge this PR (or enable auto-merge) to apply the accepted references to `claude/top-terminals-visibility-161e66`. This branch contains only PNG candidates from the immutable `reference-screenshot-candidates-33859423498` artifact and never targets `main`. If the parent branch has advanced beyond the source SHA above, do not merge this PR; the successful CI run for the new head will replace it. Co-authored-by: jonathanKingston <338988+jonathanKingston@users.noreply.github.com>
Problem
The Terminal tab's left rail stacks Shells, Agent tasks, Background tasks and Ports as fixed-shrink blocks whose percentage
max-heights never resolve. A long agent-task list therefore pushes the Shells list down to a sliver, so the open terminals become invisible while the rail fills with task rows.Change
terminal-rail-sectionthat shares the rail's height (flex: 1 1 33%), and the rail scrolls instead of crushing sections when their minimums exceed its height.terminal-rail-resizer.tsgives each visible section a minimum of its header plus two rows, and holds Shells to at least a third of the rail, so at least two terminals stay visible however many tasks or ports are listed.aria-labelnaming the two sections.This ports work that was left uncommitted in a worktree on the old lineage (
copse/for-shells-can-we-make-the-panel-sections-5d38eb); it never reached a PR.Verification
terminal-rail-resizer.test.tscovers the minimums, handle visibility and pointer/keyboard resizing;pnpm test -- terminal-rail-resizer agent-tasks ports-section supervised-taskspasses (30 tests).pnpm run typecheck,pnpm run lintandpnpm run format:checkare clean.tests/e2e/supervised-tasks.e2e.tsnow seeds ports, drags both handles and asserts every section holds its minimum. It could not be run on my machine: the unmodified spec frommainfails identically in itsbefore allhook (the known localreloadSessiontimeout), so CI is the check for it.app.cssand the compiled resizer, 3 shells + 15 agent tasks in a 900px rail: Shells held 300px with all 3 shells visible, agent tasks scrolled within their section; dragging the Shells handle up stopped at the Shells minimum, dragging down stopped at Agent tasks' two-row minimum.🤖 Generated with Claude Code