mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(split-pane): address the rest of Ark0N's PR #453 review
- Exclude popped-out (detached) sessions from the split picker:
SplitTerminalPane._sendResize() has no yield-to-detached-window check
the way the primary pane's sendResize() does, so splitting against a
detached session put its own window and Pane B in a fight over the
same PTY's dimensions. Simplest fix per the review: keep them out of
buildSplitPickerSessions() entirely.
- Show a visible dead state when Pane B's WebSocket drops. onData
already silently discards keystrokes while the socket isn't OPEN
(there is no reconnect for v1), so a dropped socket left the pane
looking normal while it quietly ate everything typed into it.
- openSplitPane() returns early with no active session, so a split
triggered from the home screen no longer creates and connects Pane B
behind the opaque welcome overlay with nothing to show for it.
- onMove() during a divider drag now bails when the split has
auto-collapsed mid-drag (the other pane's session ending) instead of
throwing on `divider.parentElement` being null.
- Promote Pane B via `selectSession(id, { auto: true })` when Pane A's
session ends — this is an app-driven selection, not the user clicking
a tab, so it must not spend the promoted session's idle alert.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
78dcb0aa24
commit
8d3bde5469
@@ -1526,10 +1526,16 @@ function clampDividerPercent(rawPercent, min = 20, max = 80) {
|
||||
return rawPercent;
|
||||
}
|
||||
|
||||
function buildSplitPickerSessions(sessions, sessionOrder, excludeId) {
|
||||
function buildSplitPickerSessions(sessions, sessionOrder, excludeId, detachedIds) {
|
||||
const result = [];
|
||||
for (const id of sessionOrder) {
|
||||
if (id === excludeId) continue;
|
||||
// A detached (popped-out) session's own window already yields its PTY
|
||||
// size (see sendResize's detachedElsewhere guard in terminal-ui.js) —
|
||||
// Pane B's SplitTerminalPane._sendResize() has no such check, so letting
|
||||
// one into the picker put its detached window and Pane B in a fight over
|
||||
// the same PTY's dimensions.
|
||||
if (detachedIds?.has?.(id)) continue;
|
||||
const session = sessions.get(id);
|
||||
if (!session) continue;
|
||||
result.push({ id, label: session.name || 'Session' });
|
||||
|
||||
@@ -99,9 +99,15 @@
|
||||
// closed socket per the WebSocket spec (no exception, no log). No
|
||||
// reconnect logic here — Pane B is deliberately plainer than the
|
||||
// primary pane (see the fileoverview above); a drop just stops
|
||||
// resizing until the parent recreates the pane.
|
||||
// resizing until the parent recreates the pane. But onData already
|
||||
// silently drops keystrokes while _wsReady is false (below), so
|
||||
// without a visible marker a dropped socket left Pane B looking
|
||||
// normal while it quietly ate everything typed into it. v1 scope is
|
||||
// "say so", not reconnect — collapsing the split would lose the
|
||||
// user's place in Pane B's scrollback for a transient blip.
|
||||
this.ws.onclose = () => {
|
||||
this._wsReady = false;
|
||||
this.terminal?.write('\r\n\x1b[2m[Pane B disconnected — close and reopen the split to reconnect]\x1b[0m\r\n');
|
||||
};
|
||||
|
||||
this.ws.onerror = () => {
|
||||
@@ -163,7 +169,8 @@ Object.assign(CodemanApp.prototype, {
|
||||
const candidates = window.CodemanSplitPane.buildSplitPickerSessions(
|
||||
this.sessions,
|
||||
this.sessionOrder,
|
||||
this.activeSessionId
|
||||
this.activeSessionId,
|
||||
this.detachedSessions
|
||||
);
|
||||
// Route a pre-existing menu through the SAME dismiss path used
|
||||
// everywhere else, instead of a raw `.remove()`: a genuinely still-open
|
||||
@@ -240,6 +247,11 @@ Object.assign(CodemanApp.prototype, {
|
||||
},
|
||||
|
||||
openSplitPane(sessionId) {
|
||||
// No active session means there is no `.terminal-wrap` to split against
|
||||
// (the welcome overlay is showing) — without this, a split opened from
|
||||
// the home screen still created the container and connected Pane B, just
|
||||
// behind the opaque overlay with nothing visible to show for it.
|
||||
if (!this.activeSessionId) return;
|
||||
// A stale picker click (opened before switching tabs) or clicking Pane
|
||||
// B's own session tab while split can otherwise land here with
|
||||
// sessionId === activeSessionId: two live WebSockets to the same
|
||||
@@ -314,6 +326,12 @@ Object.assign(CodemanApp.prototype, {
|
||||
const onMove = (e) => {
|
||||
if (!dragging) return;
|
||||
const container = divider.parentElement;
|
||||
// The split can auto-collapse mid-drag (the other pane's session
|
||||
// ending, or the picker's own close button) — closeSplitPane() removes
|
||||
// `.terminal-split-container` from the DOM, which detaches `divider`
|
||||
// too, so `divider.parentElement` is null on the very next mousemove
|
||||
// and every drag threw here until mouseup finally removed the listener.
|
||||
if (!container) return;
|
||||
const rect = container.getBoundingClientRect();
|
||||
const rawPercent = ((e.clientX - rect.left) / rect.width) * 100;
|
||||
const percent = window.CodemanSplitPane.clampDividerPercent(rawPercent);
|
||||
@@ -356,10 +374,14 @@ CodemanApp.prototype._onSessionDeleted = function (data) {
|
||||
this.closeSplitPane();
|
||||
} else if (this._splitPane && this.activeSessionId === data.id) {
|
||||
// Pane A's session ended: promote Pane B by closing the split and
|
||||
// selecting its session as the new (single) active pane.
|
||||
// selecting its session as the new (single) active pane. This is an
|
||||
// app-driven selection, not the user clicking a tab, so it must not
|
||||
// spend the promoted session's idle alert (see the Approvals Inbox
|
||||
// acknowledgement rule in CLAUDE.md — only a human opening a session
|
||||
// acknowledges it).
|
||||
const promoted = this._splitSessionId;
|
||||
this.closeSplitPane();
|
||||
if (promoted) this.selectSession(promoted);
|
||||
if (promoted) this.selectSession(promoted, { auto: true });
|
||||
}
|
||||
return _originalOnSessionDeleted.call(this, data);
|
||||
};
|
||||
|
||||
@@ -83,7 +83,9 @@ describe('terminal-split.js _onSessionDeleted wrapper (I6)', () => {
|
||||
expect(app.closeSplitPane).toHaveBeenCalledTimes(1);
|
||||
// Pinned ordering: selectSession must receive the id _splitSessionId held
|
||||
// BEFORE closeSplitPane ran (which nulls it), not whatever it holds after.
|
||||
expect(app.selectSession).toHaveBeenCalledWith('session-b');
|
||||
// { auto: true } because this is an app-driven promotion, not the user
|
||||
// clicking a tab — it must not spend the promoted session's idle alert.
|
||||
expect(app.selectSession).toHaveBeenCalledWith('session-b', { auto: true });
|
||||
expect(app.__originalDeletedCalls).toEqual([{ id: 'session-a' }]);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user