Skip to content

Commit 2499152

Browse files
diagonalbotclaude
andcommitted
herdr: put both agents on one tab, side by side
tab 1 "review" orbit-diff, the whole tab tab 2 "agents" claude │ codex, an even left/right split tab 3 "setup" the provisioning script The agents are a pair you compare, so having both on screen beats tabbing between them. That's now the only split in the workspace — everything else still gets a whole tab, which is the point of the previous change. A failed split leaves the workspace tagged and returned, same as every other half-built case, so `d` and `orbit-diff reset` can still close it. Test added for that path since it's a new way for the build to die partway. tmux unchanged and re-verified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3f35299 commit 2499152

3 files changed

Lines changed: 63 additions & 27 deletions

File tree

README.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -296,18 +296,18 @@ it drives herdr, which is the one that owns the window it would build. Set
296296
`ORBIT_MUX=tmux|herdr` to force a backend.
297297

298298
**The review environment is shaped differently.** Under tmux a review is one
299-
window of four panes. Under herdr it's a **workspace of four single-pane tabs**:
299+
window of four panes. Under herdr it's a **workspace of three tabs**:
300300

301301
```
302302
tab 1 "review" orbit-diff, the whole tab
303-
tab 2 "claude" the Claude CLI
304-
tab 3 "codex" the Codex CLI (set `pr.codex`)
305-
tab 4 "setup" your provisioning script
303+
tab 2 "agents" claude │ codex, side by side (set `pr.codex`)
304+
tab 3 "setup" your provisioning script
306305
```
307306

308-
Nothing is split. Each of these is something you sit in and use full-size — a
309-
diff wants the width, an agent is a conversation, build output scrolls — so
310-
slicing a tab into thirds only made all three worse.
307+
The agents share a tab because they're a pair you compare — both on screen beats
308+
tabbing between them. Everything else gets a whole tab: a diff wants the width,
309+
build output scrolls. That's the only split, and slicing tab 1 into thirds as
310+
well only made all three of its occupants worse.
311311

312312
There's no `orbit-diff pr-status` panel under herdr: the viewer's `G` overview
313313
covers the same ground with room to render it, and the provisioned environment

src/herdr.mjs

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -532,13 +532,14 @@ export function createHerdrBackend({ run = defaultRun, env = process.env, resolv
532532
// tabs:
533533
//
534534
// tab 1 "review" orbit-diff, the whole tab
535-
// tab 2 "claude" the Claude CLI
536-
// tab 3 "codex" the Codex CLI
537-
// tab 4 "setup" the provisioning script
535+
// tab 2 "agents" claude │ codex, side by side
536+
// tab 3 "setup" the provisioning script
538537
//
539-
// No splits at all. Each of these is something you sit in and use full-size —
540-
// a diff wants the width, an agent is a conversation, and build output scrolls
541-
// — so slicing the first tab into thirds only made all three worse.
538+
// The only split left is the one between the two agents, and it earns its
539+
// place: they're a pair you compare, so having both on screen beats tabbing
540+
// between them. Everything else gets a whole tab — a diff wants the width,
541+
// build output scrolls — which is why slicing tab 1 into thirds made all
542+
// three of its occupants worse.
542543
//
543544
// There used to be a fifth thing here, an `orbit-diff pr-status` pane showing
544545
// branch/PR/checks/env. The viewer's `G` overview covers all of it and more,
@@ -575,12 +576,28 @@ export function createHerdrBackend({ run = defaultRun, env = process.env, resolv
575576
tag(diffPane, "diff", worktreePath);
576577

577578
const panes = { diff: diffPane };
578-
for (const [role, label] of [["claude", "claude"], ["codex", "codex"], ["setup", "setup"]]) {
579-
const tab = addTab(window, worktreePath, label);
580-
if (tab.error) return { error: tab.error, window };
581-
tag(tab.pane, role, worktreePath);
582-
panes[role] = tab.pane;
583-
}
579+
580+
// Tab 2: both agents, an even left/right split.
581+
const agents = addTab(window, worktreePath, "agents");
582+
if (agents.error) return { error: agents.error, window };
583+
tag(agents.pane, "claude", worktreePath);
584+
panes.claude = agents.pane;
585+
586+
const right = run([
587+
"pane", "split", agents.pane, "--direction", "right", "--ratio", "0.50",
588+
"--cwd", worktreePath, "--no-focus",
589+
]);
590+
if (right.status !== 0) return { error: (right.stderr || "herdr pane split failed").trim(), window };
591+
const codexPane = idFrom(right.stdout, ["pane_id", "id"], { bare: true });
592+
if (!codexPane) return { error: "couldn't parse the herdr pane id", window };
593+
tag(codexPane, "codex", worktreePath);
594+
panes.codex = codexPane;
595+
596+
// Tab 3: the provisioning script.
597+
const setup = addTab(window, worktreePath, "setup");
598+
if (setup.error) return { error: setup.error, window };
599+
tag(setup.pane, "setup", worktreePath);
600+
panes.setup = setup.pane;
584601

585602
if (diffCmd) runInPane(panes.diff, diffCmd);
586603
if (claudeCmd) runInPane(panes.claude, claudeCmd);

src/herdr.test.mjs

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { sessionKey } from "./session.mjs";
77
// for the CLI. That covers the argument-building and parsing — the parts we can
88
// be sure about — and deliberately not the parts that depend on a live server.
99
//
10-
// A worktree's review is a herdr WORKSPACE of four tabs, so "window" ids in
10+
// A worktree's review is a herdr WORKSPACE of three tabs, so "window" ids in
1111
// this backend are workspace ids. See the header of herdr.mjs for why.
1212

1313
const IN_HERDR = { HERDR_PANE_ID: "w1:p1" };
@@ -338,7 +338,7 @@ function buildingHerdr({ focusedAfter = null } = {}) {
338338
return { run, calls };
339339
}
340340

341-
test("a review is a workspace of four single-pane tabs", () => {
341+
test("a review is a workspace of three tabs, agents sharing the middle one", () => {
342342
const { run, calls } = buildingHerdr();
343343
const built = createHerdrBackend({ run, env: IN_HERDR }).buildReviewWindow({
344344
worktreePath: "/wt/feature",
@@ -353,16 +353,19 @@ test("a review is a workspace of four single-pane tabs", () => {
353353
expect(built.window).toBe("w5"); // the workspace
354354
expect(Object.keys(built.panes).sort()).toEqual(["claude", "codex", "diff", "setup"]);
355355

356-
// Nothing is split any more: every tab is one full-size pane. A diff wants
357-
// the width, an agent is a conversation, build output scrolls.
358-
expect(calls.filter((c) => c[1] === "split")).toHaveLength(0);
359-
360-
// The workspace's own first tab is the diff; the other three are created.
356+
// The workspace's own first tab is the diff; two more are created.
361357
expect(built.panes.diff).toBe("w5:p1");
362358
const tabs = calls.filter((c) => is("tab", "create")(c));
363-
expect(tabs.map((t) => t[t.indexOf("--label") + 1])).toEqual(["claude", "codex", "setup"]);
359+
expect(tabs.map((t) => t[t.indexOf("--label") + 1])).toEqual(["agents", "setup"]);
364360
for (const t of tabs) expect(t[t.indexOf("--workspace") + 1]).toBe("w5");
365361

362+
// Exactly one split, and it's the agents' tab down the middle.
363+
const splits = calls.filter((c) => c[1] === "split");
364+
expect(splits).toHaveLength(1);
365+
expect(splits[0][2]).toBe(built.panes.claude); // claude keeps the left
366+
expect(splits[0]).toContain("right");
367+
expect(splits[0][splits[0].indexOf("--ratio") + 1]).toBe("0.50");
368+
366369
// Every command lands in the right place.
367370
const ran = calls.filter((c) => c[1] === "run").map((c) => [c[2], c[3]]);
368371
expect(ran).toEqual([
@@ -373,6 +376,22 @@ test("a review is a workspace of four single-pane tabs", () => {
373376
]);
374377
});
375378

379+
test("a failed agent split reports the workspace so it can still be closed", () => {
380+
const calls = [];
381+
const run = (args) => {
382+
calls.push(args);
383+
if (is("workspace", "create")(args)) {
384+
return { status: 0, stdout: reply("workspace_create", { workspace_id: "w5", pane_id: "w5:p1" }), stderr: "" };
385+
}
386+
if (is("tab", "create")(args)) return { status: 0, stdout: reply("tab_create", { pane_id: "w5:t1p" }), stderr: "" };
387+
if (args[1] === "split") return { status: 1, stdout: "", stderr: "no room" };
388+
return { status: 0, stdout: "", stderr: "" };
389+
};
390+
const built = createHerdrBackend({ run, env: IN_HERDR }).buildReviewWindow({ worktreePath: "/wt/x" });
391+
expect(built.error).toBe("no room");
392+
expect(built.window).toBe("w5");
393+
});
394+
376395
// The pr-status pane is gone: `G` covers it, with room to render it properly.
377396
test("no status pane is built, and a statusCmd is ignored rather than run", () => {
378397
const { run, calls } = buildingHerdr();

0 commit comments

Comments
 (0)