diff --git a/CHANGELOG.md b/CHANGELOG.md index 89bd8b4..d380835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,12 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). extra row of viewport. ### Fixed +- Child processes spawned by an orchestrator are now killed when the + orchestrator dies, recursively through the tree. Applies whether the + parent was closed via MCP, Ctrl-C'd by the user, or exited on its + own — `reapChild` cascades a SIGTERM (escalating to SIGKILL after 2s) + to every direct child, and each descendant's own reap fires the same + cascade so the kill flows through arbitrary depth. - Killed agents no longer linger in the command palette. Agent entries that aren't running are filtered out of the switch list; session-persistent commands (which can be restarted) stay visible. @@ -85,6 +91,15 @@ loosely follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). cell, so any child write to that column (or `clearViewport`'s ECH) would erase it. The viewport is now one column narrower so the border has a dedicated column. +- Sidebar session-tree entries no longer get pushed downward when an + agent emits a scroll burst on startup. Codex (Ratatui) issues 8× RI + (`\x1bM`) right after setting its scroll region, which scrolls the + region down across every column — dragging the right-rail entries + with it. The chrome cache then hid the clobber because the computed + frame still matched. The viewport renderer now flags any chunk that + contains a scroll-triggering escape (RI / IND / NEL / SU / SD / IL / + DL) and `OnPTYOut` drops the sidebar cache when the flag is set, so + the next `drawSidebar` repaints over the drift. ## Conventions diff --git a/internal/app/app.go b/internal/app/app.go index c586785..eb27e18 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -409,6 +409,19 @@ func (st *uiState) OnPTYOut(childID string, chunk []byte) { _, _ = os.Stdout.Write(out) _, _ = os.Stdout.Write([]byte("\x1b[?7h")) st.outMu.Unlock() + // RI / IND / NEL / SU / SD / IL / DL scroll content within the host's + // scroll region, which spans every column — so any of them drags the + // right-hand sidebar's session-tree entries downward along with the + // main pane. (Codex emits an 8× RI burst on startup, which produced + // the original report.) The viewport renderer flags any chunk that + // contained one of those escapes; when set, drop the sidebar cache + // so the next drawSidebar repaints over the clobber instead of + // hitting the cache and leaving the gap visible. + if renderer.TookScrollAction() { + st.chromeCacheMu.Lock() + st.sidebarCache = "" + st.chromeCacheMu.Unlock() + } st.drawTabBar() st.drawSidebar() st.drawStatusLine() diff --git a/internal/app/viewport_renderer.go b/internal/app/viewport_renderer.go index 54ca666..00c3e82 100644 --- a/internal/app/viewport_renderer.go +++ b/internal/app/viewport_renderer.go @@ -19,6 +19,14 @@ type viewportRenderer struct { state viewportState buf []byte pending strings.Builder + + // scrolled is set when the chunk contained an escape that shifts + // content row-wise within the host's scroll region — RI / IND / + // NEL / SU / SD / IL / DL. DECSTBM constrains rows but not columns, + // so these scrolls drag the right-hand sidebar content with them. + // OnPTYOut consumes the flag and invalidates the sidebar chrome + // cache so the next drawSidebar repaints over the clobber. + scrolled bool } type viewportState int @@ -67,6 +75,20 @@ func (vr *viewportRenderer) ClearViewport() []byte { return []byte(vr.clearViewport()) } +// TookScrollAction reports whether the most recent Render emitted (or +// forwarded) a scroll-triggering escape — RI / IND / NEL / SU / SD / +// IL / DL — since the previous call. The flag is reset on read. +// Callers use it to invalidate sidebar-cache state, because the host's +// scroll region spans the full row width and any scroll there drags +// the sidebar content downward. +func (vr *viewportRenderer) TookScrollAction() bool { + vr.mu.Lock() + defer vr.mu.Unlock() + out := vr.scrolled + vr.scrolled = false + return out +} + func (vr *viewportRenderer) feed(b byte) { switch vr.state { case vpNormal: @@ -89,6 +111,18 @@ func (vr *viewportRenderer) feed(b byte) { vr.state = vpDCS case 'X', '^', '_': vr.state = vpSOSPMAPC + case 'M', 'D', 'E': + // RI (ESC M), IND (ESC D), NEL (ESC E). All three can scroll + // the host's scroll region when the cursor is at the top + // (RI) or bottom (IND/NEL) edge. The region spans the full + // row width, so the scroll drags the sidebar columns along + // with the main pane. Forward as-is and flag for sidebar + // cache invalidation. Codex emits 8× RI on startup, which + // is what motivated this branch. + vr.pending.Write(vr.buf) + vr.scrolled = true + vr.state = vpNormal + vr.buf = vr.buf[:0] default: vr.pending.Write(vr.buf) vr.state = vpNormal @@ -177,6 +211,15 @@ func (vr *viewportRenderer) emitCSI() { return } vr.pending.WriteString(vr.clearLine(n)) + case 'S', 'T', 'L', 'M': + // SU (S) / SD (T) / IL (L) / DL (M) all shift content within + // the host's scroll region row-wise across every column. The + // sidebar lives at the right of the host, inside the scroll + // region's row range, so any of these drag its cells along + // with the main pane. Forward verbatim and flag the chunk so + // the sidebar is repainted afterwards. + vr.pending.Write(vr.shifter.Shift(vr.buf)) + vr.scrolled = true default: vr.pending.Write(vr.shifter.Shift(vr.buf)) } diff --git a/internal/app/viewport_renderer_test.go b/internal/app/viewport_renderer_test.go index 3729b2f..111470b 100644 --- a/internal/app/viewport_renderer_test.go +++ b/internal/app/viewport_renderer_test.go @@ -102,3 +102,55 @@ func TestViewportRendererTracksPrintableCursor(t *testing.T) { t.Fatalf("clear-line after five chars should erase 15 cells: %q", got) } } + +func TestViewportRendererFlagsRIAsScrolling(t *testing.T) { + // Reproduces the sidebar-gap bug: codex emits `\x1b[1;1H` followed + // by 8× `\x1bM` (RI) on startup. RI at the top of the host scroll + // region scrolls the region down — across all columns — pushing + // sidebar content out of place. The renderer must flag the chunk + // so the sidebar cache gets invalidated and repainted afterwards. + vr := newViewportRenderer(newTerminalLayout(120, 40)) + if vr.TookScrollAction() { + t.Fatalf("scroll flag set before any input") + } + _ = vr.Render([]byte("\x1b[1;1H")) + if vr.TookScrollAction() { + t.Fatalf("plain CUP should not flag scroll") + } + _ = vr.Render([]byte("\x1bM")) + if !vr.TookScrollAction() { + t.Fatalf("RI (ESC M) should flag scroll") + } + if vr.TookScrollAction() { + t.Fatalf("flag should reset after read") + } +} + +func TestViewportRendererFlagsScrollVerbs(t *testing.T) { + cases := map[string][]byte{ + "IND": []byte("\x1bD"), + "NEL": []byte("\x1bE"), + "SU": []byte("\x1b[3S"), + "SD": []byte("\x1b[2T"), + } + for name, in := range cases { + t.Run(name, func(t *testing.T) { + vr := newViewportRenderer(newTerminalLayout(120, 40)) + _ = vr.Render(in) + if !vr.TookScrollAction() { + t.Fatalf("%s should flag scroll", name) + } + }) + } +} + +func TestViewportRendererForwardsRIVerbatim(t *testing.T) { + // We rely on the host terminal performing the scroll inside the + // DECSTBM region; the renderer must not eat or transform RI. If a + // future change ever rewrites RI, this test catches the regression. + vr := newViewportRenderer(newTerminalLayout(120, 40)) + got := string(vr.Render([]byte("\x1bM"))) + if got != "\x1bM" { + t.Fatalf("RI should pass through unchanged: got %q", got) + } +} diff --git a/internal/harness/scenarios/sidebar_survives_ri_scroll.json b/internal/harness/scenarios/sidebar_survives_ri_scroll.json new file mode 100644 index 0000000..0d2ee4c --- /dev/null +++ b/internal/harness/scenarios/sidebar_survives_ri_scroll.json @@ -0,0 +1,28 @@ +{ + "name": "sidebar_survives_ri_scroll", + "cols": 80, + "rows": 24, + "scripts": [ + { + "name": "riburst", + "body": "#!/bin/sh\n# Emulates codex's startup sequence: DECSTBM the full child\n# viewport, CUP to the top of the scroll region, and burst 8 RIs\n# (ESC M). RI at the top of the scroll region scrolls the region\n# down. The host's scroll region spans every column, so without\n# the sidebar cache-invalidation fix the right-hand session tree\n# gets dragged downward and the cache hides the broken state.\nprintf '\\033[1;21r'\nprintf '\\033[1;1H'\nprintf '\\033M\\033M\\033M\\033M\\033M\\033M\\033M\\033M'\nprintf '\\033[1;1HRIBURST READY\\n'\nsleep 5\n" + } + ], + "steps": [ + { + "type": "mcp_call", + "method": "spawn_process", + "params": { "kind": "command", "argv": ["riburst"], "name": "riburst" } + }, + { "type": "wait_text", "contains": "RIBURST READY", "timeout_ms": 5000 }, + { "type": "wait_stable", "timeout_ms": 2000 }, + { "type": "assert_contains", "contains": "Session tree" }, + { "type": "assert_contains", "contains": "● riburst" }, + { "type": "assert_contains", "contains": "Scratchpads" }, + { + "type": "assert_regex", + "regex": "(?s)Session tree[^\\n]*\\n[^─\\n]*─[─]+[^\\n]*\\n[^●\\n]*● riburst", + "timeout_ms": 2000 + } + ] +}