Skip to content

Commit fcbd417

Browse files
authored
Open real auxiliary panes on the native terminal backend (#1222)
* Open real auxiliary panes on the native terminal backend Dev-server and git-operation panes were raw tmux splits against dev3-task-<id>, a session a native task never has. The git panes threw; the dev-server split failed inside a best-effort catch, so the dev script kept running invisibly in a tmux session a native task should never touch at all. Add a backend-neutral seam that owns auxiliary panes: it derives ownership from the command a pane was launched with (as the tmux code already did), replaces the pane a purpose already owns instead of stacking a second one, and hands focus back to the pane that had it. Route the dev server and the shared git-operation pane through it, and make the dev server's liveness, status and teardown read the right host per backend. Also fixes two leaks the same blind spot caused: task teardown skipped the dev server entirely on native, and the agent hand-off prompts (rebase conflicts, Create PR, Commit, scheduled messages) were tmux-only, so they silently did nothing. * Cover the native auxiliary-pane seam and hand-off with tests Backend matrix over the seam, the dev server and the agent hand-off: the native paths must make zero tmux calls, must not stack a second pane on a repeated action, and must leave the agent pane focused. The tmux cases assert the split arguments unchanged, so the old behaviour stays pinned. Renderer: an auxiliary pane is named in the narrow pane pager and in the close-pane picker instead of falling back to "Pane N".
1 parent ba30dc8 commit fcbd417

19 files changed

Lines changed: 1130 additions & 131 deletions
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Short: Dev server and Rebase panes work natively
2+
3+
On a task using the native terminal backend, starting the dev server or running Rebase opens a real visible pane in the task's own terminal instead of failing invisibly — the dev server used to run hidden in a tmux session a native task should never touch, and the git panes errored out. Repeated clicks reuse the one pane, the agent pane keeps its focus, and closing a task now also stops a native dev server instead of leaking its processes and ports.
Lines changed: 330 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,330 @@
1+
/**
2+
* task-aux-panes tests (seq 1376).
3+
*
4+
* Two guarantees, one per backend:
5+
* • native never reaches tmux, dedups its own pane, and hands focus back;
6+
* • tmux behaves exactly as it did before the module existed.
7+
*/
8+
import { describe, it, expect, vi, beforeEach } from "vitest";
9+
import type { Task } from "../../shared/types";
10+
11+
const { FakeTmuxError } = vi.hoisted(() => ({
12+
FakeTmuxError: class FakeTmuxError extends Error {
13+
constructor(readonly args: string[], readonly exitCode: number, readonly stderr: string) {
14+
super(`tmux ${args[0] ?? ""} failed (exit ${exitCode}): ${stderr || "unknown error"}`);
15+
this.name = "TmuxError";
16+
}
17+
},
18+
}));
19+
20+
const mocks = vi.hoisted(() => ({
21+
// tmux singleton — every method is a spy so "no tmux happened" is provable.
22+
tmuxListPanes: vi.fn(),
23+
tmuxSplitWindow: vi.fn(),
24+
tmuxSelectPane: vi.fn(),
25+
tmuxKillPane: vi.fn(),
26+
// native-task-panes
27+
nativeTaskPanesState: vi.fn(),
28+
nativeTaskPaneCommands: vi.fn(),
29+
splitNativeTaskPane: vi.fn(),
30+
closeNativeTaskPane: vi.fn(),
31+
focusNativeTaskPane: vi.fn(),
32+
}));
33+
34+
/** Every tmux method the module could possibly reach. */
35+
const TMUX_METHODS = [mocks.tmuxListPanes, mocks.tmuxSplitWindow, mocks.tmuxSelectPane, mocks.tmuxKillPane];
36+
37+
vi.mock("../logger", () => ({
38+
createLogger: () => ({ debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }),
39+
}));
40+
41+
// Nothing in this module may spawn a process directly.
42+
vi.mock("../spawn", () => ({ spawn: vi.fn(), spawnSync: vi.fn() }));
43+
44+
vi.mock("../tmux", () => ({
45+
PANE_START_COMMAND_FORMAT: { formatString: "#{pane_id}\t#{pane_start_command}", parse: () => [] },
46+
TmuxError: FakeTmuxError,
47+
taskSessionName: (taskId: string) => `dev3-${taskId.slice(0, 8)}`,
48+
tmux: {
49+
listPanes: mocks.tmuxListPanes,
50+
splitWindow: mocks.tmuxSplitWindow,
51+
selectPane: mocks.tmuxSelectPane,
52+
killPane: mocks.tmuxKillPane,
53+
},
54+
}));
55+
56+
vi.mock("../native-task-panes", () => ({
57+
nativeTaskPanesState: mocks.nativeTaskPanesState,
58+
nativeTaskPaneCommands: mocks.nativeTaskPaneCommands,
59+
splitNativeTaskPane: mocks.splitNativeTaskPane,
60+
closeNativeTaskPane: mocks.closeNativeTaskPane,
61+
focusNativeTaskPane: mocks.focusNativeTaskPane,
62+
}));
63+
64+
import {
65+
auxPaneAlive,
66+
auxPaneMarker,
67+
auxPurposeOfCommand,
68+
AuxPaneUnavailableError,
69+
closeAuxPane,
70+
findAuxPane,
71+
nativeAuxPaneShellPid,
72+
openAuxPane,
73+
} from "../task-aux-panes";
74+
import { spawn } from "../spawn";
75+
76+
// ── Fixtures ─────────────────────────────────────────────────────────────────
77+
78+
const TASK_ID = "aaaaaaaa-0000-0000-0000-000000000001";
79+
const SESSION = `dev3-${TASK_ID.slice(0, 8)}`;
80+
const SOCKET = "dev3-sock";
81+
82+
const nativeTask = { id: TASK_ID, terminalBackend: "native" } as unknown as Task;
83+
const tmuxTask = { id: TASK_ID } as unknown as Task;
84+
85+
const DEV_MARKER = auxPaneMarker(TASK_ID, "devServer");
86+
87+
function spec(task: Task, overrides: Partial<Parameters<typeof openAuxPane>[0]> = {}) {
88+
return {
89+
task,
90+
purpose: "devServer" as const,
91+
placement: "below" as const,
92+
size: "20%",
93+
cwd: "/tmp/wt",
94+
env: { DEV3_TASK_ID: TASK_ID },
95+
socket: SOCKET,
96+
title: "Dev Server",
97+
tmuxCommand: `bash ${DEV_MARKER}`,
98+
nativeLaunch: { executable: "/bin/bash", argv: [DEV_MARKER] },
99+
...overrides,
100+
};
101+
}
102+
103+
function nativePane(paneId: string, command: string[], alive = true) {
104+
return { paneId, sessionId: `sess-${paneId}`, command, shellPid: 4242, alive };
105+
}
106+
107+
function nativeState(paneIds: string[], activePaneId: string | null) {
108+
return {
109+
taskId: TASK_ID,
110+
panes: paneIds.map((paneId) => ({
111+
paneId,
112+
sessionId: `sess-${paneId}`,
113+
hostPid: 100,
114+
shellPid: 101,
115+
cols: 80,
116+
rows: 24,
117+
alive: true,
118+
})),
119+
layout: null,
120+
activePaneId,
121+
};
122+
}
123+
124+
beforeEach(() => {
125+
vi.clearAllMocks();
126+
mocks.nativeTaskPanesState.mockResolvedValue(nativeState(["pane-1"], "pane-1"));
127+
mocks.nativeTaskPaneCommands.mockResolvedValue([]);
128+
mocks.splitNativeTaskPane.mockResolvedValue({ paneId: "pane-2", state: nativeState(["pane-1", "pane-2"], "pane-2") });
129+
mocks.closeNativeTaskPane.mockResolvedValue({ sessionTornDown: false, state: nativeState(["pane-1"], "pane-1") });
130+
mocks.focusNativeTaskPane.mockResolvedValue(nativeState(["pane-1", "pane-2"], "pane-1"));
131+
mocks.tmuxSplitWindow.mockResolvedValue({ paneId: "%7", stderr: "" });
132+
mocks.tmuxSelectPane.mockResolvedValue(undefined);
133+
mocks.tmuxKillPane.mockResolvedValue(undefined);
134+
mocks.tmuxListPanes.mockResolvedValue([]);
135+
});
136+
137+
// ── Native backend ───────────────────────────────────────────────────────────
138+
139+
describe("openAuxPane (native)", () => {
140+
it("splits from the coordinator's active pane and returns the native handle", async () => {
141+
const handle = await openAuxPane(spec(nativeTask));
142+
143+
expect(mocks.splitNativeTaskPane).toHaveBeenCalledWith(TASK_ID, "pane-1", "vertical", {
144+
cwd: "/tmp/wt",
145+
env: { DEV3_TASK_ID: TASK_ID },
146+
launch: { executable: "/bin/bash", argv: [DEV_MARKER] },
147+
});
148+
expect(handle).toEqual({ backend: "native", paneId: "pane-2" });
149+
});
150+
151+
it("makes ZERO tmux calls", async () => {
152+
await openAuxPane(spec(nativeTask));
153+
154+
for (const method of TMUX_METHODS) expect(method).not.toHaveBeenCalled();
155+
expect(spawn).not.toHaveBeenCalled();
156+
});
157+
158+
it("hands focus back to the pane that had it before the split", async () => {
159+
await openAuxPane(spec(nativeTask));
160+
expect(mocks.focusNativeTaskPane).toHaveBeenCalledWith(TASK_ID, "pane-1");
161+
});
162+
163+
it("forces focus nowhere when the coordinator had no active pane", async () => {
164+
mocks.nativeTaskPanesState.mockResolvedValue(nativeState(["pane-1"], null));
165+
166+
await openAuxPane(spec(nativeTask));
167+
168+
expect(mocks.splitNativeTaskPane).toHaveBeenCalledWith(TASK_ID, "pane-1", "vertical", expect.anything());
169+
expect(mocks.focusNativeTaskPane).not.toHaveBeenCalled();
170+
});
171+
172+
it("closes the purpose's existing pane before opening the new one", async () => {
173+
mocks.nativeTaskPaneCommands.mockResolvedValue([
174+
nativePane("pane-1", ["/bin/zsh"]),
175+
nativePane("pane-9", ["/bin/bash", DEV_MARKER]),
176+
]);
177+
178+
await openAuxPane(spec(nativeTask));
179+
180+
expect(mocks.closeNativeTaskPane).toHaveBeenCalledTimes(1);
181+
expect(mocks.closeNativeTaskPane).toHaveBeenCalledWith(TASK_ID, "pane-9");
182+
expect(mocks.closeNativeTaskPane.mock.invocationCallOrder[0]).toBeLessThan(
183+
mocks.splitNativeTaskPane.mock.invocationCallOrder[0],
184+
);
185+
expect(mocks.splitNativeTaskPane).toHaveBeenCalledTimes(1);
186+
});
187+
188+
it("sweeps a dead owned pane before opening a new one", async () => {
189+
mocks.nativeTaskPaneCommands.mockResolvedValue([nativePane("pane-9", ["/bin/bash", DEV_MARKER], false)]);
190+
191+
await openAuxPane(spec(nativeTask));
192+
193+
expect(mocks.closeNativeTaskPane).toHaveBeenCalledWith(TASK_ID, "pane-9");
194+
expect(mocks.splitNativeTaskPane).toHaveBeenCalledTimes(1);
195+
});
196+
197+
it("throws AuxPaneUnavailableError when the native terminal is not running", async () => {
198+
mocks.nativeTaskPanesState.mockResolvedValue(null);
199+
200+
await expect(openAuxPane(spec(nativeTask))).rejects.toBeInstanceOf(AuxPaneUnavailableError);
201+
expect(mocks.splitNativeTaskPane).not.toHaveBeenCalled();
202+
for (const method of TMUX_METHODS) expect(method).not.toHaveBeenCalled();
203+
});
204+
205+
it("throws AuxPaneUnavailableError when the pane set is empty", async () => {
206+
mocks.nativeTaskPanesState.mockResolvedValue(nativeState([], null));
207+
208+
await expect(openAuxPane(spec(nativeTask))).rejects.toBeInstanceOf(AuxPaneUnavailableError);
209+
for (const method of TMUX_METHODS) expect(method).not.toHaveBeenCalled();
210+
});
211+
});
212+
213+
describe("native pane lookup by launch-command marker", () => {
214+
it("findAuxPane resolves the pane carrying the marker", async () => {
215+
mocks.nativeTaskPaneCommands.mockResolvedValue([
216+
nativePane("pane-1", ["/bin/zsh"]),
217+
nativePane("pane-9", ["/bin/bash", DEV_MARKER]),
218+
]);
219+
220+
await expect(findAuxPane(nativeTask, "devServer", SOCKET)).resolves.toEqual({
221+
backend: "native",
222+
paneId: "pane-9",
223+
});
224+
});
225+
226+
it("findAuxPane returns null for an ordinary pane", async () => {
227+
mocks.nativeTaskPaneCommands.mockResolvedValue([nativePane("pane-1", ["/bin/zsh"])]);
228+
await expect(findAuxPane(nativeTask, "devServer", SOCKET)).resolves.toBeNull();
229+
});
230+
231+
it("auxPaneAlive is true only while the marked pane's process runs", async () => {
232+
mocks.nativeTaskPaneCommands.mockResolvedValue([nativePane("pane-9", ["/bin/bash", DEV_MARKER])]);
233+
await expect(auxPaneAlive(nativeTask, "devServer", SOCKET)).resolves.toBe(true);
234+
235+
mocks.nativeTaskPaneCommands.mockResolvedValue([nativePane("pane-9", ["/bin/bash", DEV_MARKER], false)]);
236+
await expect(auxPaneAlive(nativeTask, "devServer", SOCKET)).resolves.toBe(false);
237+
});
238+
239+
it("auxPaneAlive is false for an ordinary pane", async () => {
240+
mocks.nativeTaskPaneCommands.mockResolvedValue([nativePane("pane-1", ["/bin/zsh"])]);
241+
await expect(auxPaneAlive(nativeTask, "devServer", SOCKET)).resolves.toBe(false);
242+
});
243+
244+
it("nativeAuxPaneShellPid returns the live pane's pid, null otherwise", async () => {
245+
mocks.nativeTaskPaneCommands.mockResolvedValue([nativePane("pane-9", ["/bin/bash", DEV_MARKER])]);
246+
await expect(nativeAuxPaneShellPid(nativeTask, "devServer")).resolves.toBe(4242);
247+
248+
mocks.nativeTaskPaneCommands.mockResolvedValue([nativePane("pane-1", ["/bin/zsh"])]);
249+
await expect(nativeAuxPaneShellPid(nativeTask, "devServer")).resolves.toBeNull();
250+
});
251+
252+
it("auxPurposeOfCommand labels a command only by its own marker", () => {
253+
expect(auxPurposeOfCommand(TASK_ID, ["/bin/bash", DEV_MARKER])).toBe("devServer");
254+
expect(auxPurposeOfCommand(TASK_ID, ["/bin/bash", auxPaneMarker(TASK_ID, "gitOp") + "rebase.sh"])).toBe("gitOp");
255+
expect(auxPurposeOfCommand(TASK_ID, ["/bin/zsh"])).toBeNull();
256+
});
257+
});
258+
259+
// ── tmux backend (regression guard: behaviour must be unchanged) ──────────────
260+
261+
describe("openAuxPane (tmux)", () => {
262+
it("splits the task session below at the requested size", async () => {
263+
const handle = await openAuxPane(spec(tmuxTask));
264+
265+
expect(mocks.tmuxSplitWindow).toHaveBeenCalledWith({
266+
target: SESSION,
267+
orientation: "vertical",
268+
size: "20%",
269+
printPaneId: true,
270+
env: { DEV3_TASK_ID: TASK_ID },
271+
cwd: "/tmp/wt",
272+
command: `bash ${DEV_MARKER}`,
273+
socket: SOCKET,
274+
});
275+
expect(handle).toEqual({ backend: "tmux", paneId: "%7" });
276+
});
277+
278+
it("maps placement 'right' to a horizontal split", async () => {
279+
await openAuxPane(spec(tmuxTask, { placement: "right", size: "50%" }));
280+
281+
expect(mocks.tmuxSplitWindow).toHaveBeenCalledWith(
282+
expect.objectContaining({ orientation: "horizontal", size: "50%" }),
283+
);
284+
});
285+
286+
it("titles the new pane", async () => {
287+
await openAuxPane(spec(tmuxTask));
288+
expect(mocks.tmuxSelectPane).toHaveBeenCalledWith("%7", { socket: SOCKET, title: "Dev Server" });
289+
});
290+
291+
it("surfaces a failed split as a readable error", async () => {
292+
mocks.tmuxSplitWindow.mockRejectedValue(new FakeTmuxError(["split-window"], 1, "no such session"));
293+
294+
await expect(openAuxPane(spec(tmuxTask))).rejects.toThrow(/tmux split-window failed/);
295+
});
296+
297+
it("makes ZERO native calls", async () => {
298+
await openAuxPane(spec(tmuxTask));
299+
300+
expect(mocks.nativeTaskPanesState).not.toHaveBeenCalled();
301+
expect(mocks.splitNativeTaskPane).not.toHaveBeenCalled();
302+
expect(mocks.closeNativeTaskPane).not.toHaveBeenCalled();
303+
expect(mocks.focusNativeTaskPane).not.toHaveBeenCalled();
304+
expect(mocks.nativeTaskPaneCommands).not.toHaveBeenCalled();
305+
});
306+
});
307+
308+
describe("closeAuxPane", () => {
309+
it("kills the tmux pane best-effort", async () => {
310+
mocks.tmuxListPanes.mockResolvedValue([{ paneId: "%7", startCommand: `bash ${DEV_MARKER}` }]);
311+
312+
await closeAuxPane(tmuxTask, "devServer", SOCKET);
313+
314+
expect(mocks.tmuxKillPane).toHaveBeenCalledWith("%7", { socket: SOCKET, bestEffort: true });
315+
});
316+
317+
it("closes the native pane through the native backend", async () => {
318+
mocks.nativeTaskPaneCommands.mockResolvedValue([nativePane("pane-9", ["/bin/bash", DEV_MARKER])]);
319+
320+
await closeAuxPane(nativeTask, "devServer", SOCKET);
321+
322+
expect(mocks.closeNativeTaskPane).toHaveBeenCalledWith(TASK_ID, "pane-9");
323+
expect(mocks.tmuxKillPane).not.toHaveBeenCalled();
324+
});
325+
326+
it("does nothing when the purpose owns no pane", async () => {
327+
await closeAuxPane(tmuxTask, "devServer", SOCKET);
328+
expect(mocks.tmuxKillPane).not.toHaveBeenCalled();
329+
});
330+
});

src/bun/lifecycle/__tests__/native-teardown.test.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,16 +154,21 @@ describe("destroyTaskPty", () => {
154154
});
155155

156156
describe("killDevServer", () => {
157-
it("is skipped for a native task, which owns no tmux dev session", async () => {
158-
await executeLifecycleEffect(effect("killDevServer"), context(task({ terminalBackend: "native" })));
159-
160-
expect(killDevServerSession).not.toHaveBeenCalled();
157+
// A native task hosts its dev server in an auxiliary pane rather than a tmux
158+
// session, so teardown must still run — skipping it here leaked the whole
159+
// dev-server process tree, ports included.
160+
it("tears the dev server down for a native task too", async () => {
161+
const nativeTask = task({ terminalBackend: "native" });
162+
await executeLifecycleEffect(effect("killDevServer"), context(nativeTask));
163+
164+
expect(killDevServerSession).toHaveBeenCalledWith(nativeTask, "dev3", "/tmp/wt");
161165
});
162166

163167
it("still tears the dev session down for an unmarked task", async () => {
164-
await executeLifecycleEffect(effect("killDevServer"), context(task()));
168+
const tmuxTask = task();
169+
await executeLifecycleEffect(effect("killDevServer"), context(tmuxTask));
165170

166-
expect(killDevServerSession).toHaveBeenCalledWith(TASK_ID, "dev3", "/tmp/wt");
171+
expect(killDevServerSession).toHaveBeenCalledWith(tmuxTask, "dev3", "/tmp/wt");
167172
});
168173
});
169174

src/bun/lifecycle/executor.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -763,11 +763,11 @@ export async function executeLifecycleEffect(
763763
}
764764
return {};
765765
case "killDevServer":
766-
// A dev server is a tmux session; a native task has none, and probing tmux
767-
// for it is exactly what the native path must never do.
768-
if (taskTerminalBackendIdentity(ctx.sourceTask) === "native") return {};
766+
// Both backends host a dev server (tmux: a nested session, native: an
767+
// auxiliary pane), and killDevServerSession picks the right one. Skipping
768+
// native here used to leak the whole dev-server process tree on teardown.
769769
await killDevServerSession(
770-
ctx.sourceTask.id,
770+
ctx.sourceTask,
771771
ctx.sourceTask.tmuxSocket ?? DEFAULT_TMUX_SOCKET,
772772
ctx.sourceTask.worktreePath,
773773
);

src/bun/lifecycle/service.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ export interface LifecycleActorRuntime {
3030
prPending?: boolean;
3131
prPromoted?: boolean;
3232
prSignalKey?: string;
33-
gitOpPaneId?: string;
3433
branchChecks?: Map<string, Promise<BranchStatus>>;
3534
activeActivities?: Set<LifecycleActivity>;
3635
}
@@ -230,7 +229,6 @@ class LifecycleService {
230229
clearTaskRuntime: (id) => {
231230
const runtime = this.actors.runtime(id);
232231
delete runtime.mergePromptReservation;
233-
delete runtime.gitOpPaneId;
234232
runtime.branchChecks?.clear();
235233
delete runtime.branchChecks;
236234
delete runtime.mergeNextDue;

0 commit comments

Comments
 (0)