fix(tmux,run,hooks): the four pre-existing bugs the #37 evaluation found
- tmux targets are exact (`=name`, `=name:` for panes/options): a bare `-t agenthub-lane-1` resolves by prefix to agenthub-lane-12 once lane 1 is gone, so killing a finished lane could kill another. - getRun/killRun refuse ids that are not `agenthub-lane-<n>`, so DELETE /api/run/main can no longer kill a user's own tmux session. - router/apply reports restart_required only for a running pane; getRun returns a `gone` handle, so it was always true. - needs_action is cleared only by the session that raised it (new nullable lanes.needs_action_session); a second session in the same cwd used to clear it on its second hook.
This commit is contained in:
@@ -2437,6 +2437,7 @@ export interface Lane {
|
||||
gate_decision: string | null;
|
||||
ci_status: string | null;
|
||||
needs_action: string | null;
|
||||
needs_action_session?: string | null;
|
||||
links: Record<string, string>;
|
||||
stages: Record<string, { enteredAt: string; evidence: string | null; result: string | null }>;
|
||||
notes: string | null;
|
||||
|
||||
@@ -2072,6 +2072,8 @@ GET /api/run/:id Run handle (returns live run state)
|
||||
DELETE /api/run/:id Kill (SIGTERM → SIGKILL after 5 s)
|
||||
```
|
||||
|
||||
`:id` must be a lane pane name, `agenthub-lane-<n>`; anything else answers 404 without reaching tmux, so these two routes can never touch a tmux session the dashboard did not start.
|
||||
|
||||
**`GET /api/run/binary`:** Probes for the binary (`claude` or `codex`). Optional query parameter `?tool=codex` probes for Codex; omitted or `?tool=claude` probes for Claude. Returns `{ found: bool, path: string | null }`.
|
||||
|
||||
**`POST /api/run` (start/attach):** Requires `laneId` (the lane this run belongs to). Creates or attaches an existing tmux session named `agenthub-lane-<id>` in the lane's working directory. If that session already exists and its pane is idling at a shell prompt, the appropriate command line (including `--resume` and any initial prompt) is typed into that pane instead of being dropped; if the pane is running a program, the request adopts the session unchanged. Optionally accepts `initialPrompt` to immediately type/send into the session (if empty or omitted, the session is created/attached with no initial input). Returns `{ id, laneId, status, cwd, model, permissionMode, effort, resumeSessionId, sessionId, startedAt, promptPreview }` where `id` is the tmux session name. The dashboard self-heals a lane's `run_id`/`status` on every read if the tmux session has been killed externally.
|
||||
|
||||
+1
-1
@@ -738,7 +738,7 @@ This shows each lane's:
|
||||
- **status**: `running` (a session is active), `idle` (no active session), or `provisioning`
|
||||
- **liveness**: `active` (recently updated), `dead` (silent for too long), or `idle` (not expected to be running)
|
||||
- **progress**: percentage of the pipeline completed (0–100%)
|
||||
- **needs_action**: a notification if the session is blocked waiting for input
|
||||
- **needs_action**: a notification if the session is blocked waiting for input. `needs_action_session` records which session raised it, and only that session's next hook clears it — a second session in the same cwd (a hand-started terminal beside the dashboard pane) rebinds `session_id` but cannot clear a prompt it is not the one waiting on
|
||||
|
||||
A lane row also gets a `⇢ detected:<stage>` suffix when the server's inferred
|
||||
stage (see "Stage detection" below) leads the declared `stage` shown in the
|
||||
|
||||
@@ -165,7 +165,7 @@ function reloadPanes() {
|
||||
try {
|
||||
paneCmd = execFileSync(
|
||||
"tmux",
|
||||
["display-message", "-p", "-t", name, "#{pane_current_command}"],
|
||||
["display-message", "-p", "-t", `=${name}:`, "#{pane_current_command}"],
|
||||
{ encoding: "utf8", timeout: 15_000, stdio: ["ignore", "pipe", "pipe"] }
|
||||
).trim();
|
||||
} catch {
|
||||
@@ -184,8 +184,8 @@ function reloadPanes() {
|
||||
// detached and serves no requests.
|
||||
if (!first) execFileSync("sleep", ["1"], { timeout: 15_000 });
|
||||
first = false;
|
||||
execFileSync("tmux", ["send-keys", "-t", name, "-l", command], { timeout: 15_000 });
|
||||
execFileSync("tmux", ["send-keys", "-t", name, "Enter"], { timeout: 15_000 });
|
||||
execFileSync("tmux", ["send-keys", "-t", `=${name}:`, "-l", command], { timeout: 15_000 });
|
||||
execFileSync("tmux", ["send-keys", "-t", `=${name}:`, "Enter"], { timeout: 15_000 });
|
||||
}
|
||||
reloaded.push(name);
|
||||
} catch {
|
||||
|
||||
@@ -43,13 +43,13 @@ function stubTmux(sessions, panes = {}) {
|
||||
calls.push(args);
|
||||
if (args[0] === "list-sessions") return sessions.join("\n");
|
||||
if (args[0] === "has-session") {
|
||||
if (sessions.includes(args[2])) return "";
|
||||
if (sessions.includes(args[2].slice(1))) return "";
|
||||
const e = new Error("no such session");
|
||||
e.status = 1;
|
||||
throw e;
|
||||
}
|
||||
if (args[0] === "display-message") {
|
||||
const name = args[3];
|
||||
const name = args[3].slice(1, -1);
|
||||
const cmd = panes[name] ?? "claude";
|
||||
if (cmd === null) {
|
||||
const e = new Error("pane gone");
|
||||
@@ -59,7 +59,7 @@ function stubTmux(sessions, panes = {}) {
|
||||
return `${cmd}\n`;
|
||||
}
|
||||
if (args[0] === "send-keys") {
|
||||
if (panes[args[2]] === "__send-fails__") throw new Error("pane died mid-send");
|
||||
if (panes[args[2].slice(1, -1)] === "__send-fails__") throw new Error("pane died mid-send");
|
||||
return "";
|
||||
}
|
||||
return "";
|
||||
|
||||
@@ -856,9 +856,9 @@ describe("destructive lane lifecycle actions", () => {
|
||||
// Mock tmux so has-session always returns 0 (session exists), simulating a stuck session
|
||||
// Same shape as tmux.js's own exec: sync, argv only, throws on failure.
|
||||
tmux.__setExecImpl((args) => {
|
||||
if (args[0] === "has-session" && args[2] === runId) return "";
|
||||
if (args[0] === "has-session" && args[2] === `=${runId}`) return "";
|
||||
if (args[0] === "list-sessions") return `${runId}\n`;
|
||||
if (args[0] === "kill-session" && args[2] === runId) return "";
|
||||
if (args[0] === "kill-session" && args[2] === `=${runId}`) return "";
|
||||
throw Object.assign(new Error("mocked tmux failure"), { status: 1 });
|
||||
});
|
||||
|
||||
@@ -949,7 +949,12 @@ describe("destructive lane lifecycle actions", () => {
|
||||
|
||||
// Mock tmux so the session appears live
|
||||
tmux.__setExecImpl(async (cmd, args) => {
|
||||
if (cmd === "tmux" && args[0] === "has-session" && args[1] === "-t" && args[2] === runId) {
|
||||
if (
|
||||
cmd === "tmux" &&
|
||||
args[0] === "has-session" &&
|
||||
args[1] === "-t" &&
|
||||
args[2] === `=${runId}`
|
||||
) {
|
||||
return { code: 0, stdout: "", stderr: "" };
|
||||
}
|
||||
if (cmd === "tmux" && args[0] === "list-sessions") {
|
||||
@@ -1218,7 +1223,12 @@ describe("lane ensure, start mode, lane_id and releasing a finished run", () =>
|
||||
|
||||
// Mock tmux so the run appears to be live.
|
||||
tmux.__setExecImpl(async (cmd, args) => {
|
||||
if (cmd === "tmux" && args[0] === "has-session" && args[1] === "-t" && args[2] === runId) {
|
||||
if (
|
||||
cmd === "tmux" &&
|
||||
args[0] === "has-session" &&
|
||||
args[1] === "-t" &&
|
||||
args[2] === `=${runId}`
|
||||
) {
|
||||
return { code: 0, stdout: "", stderr: "" };
|
||||
}
|
||||
if (cmd === "tmux" && args[0] === "list-sessions") {
|
||||
@@ -1264,7 +1274,7 @@ describe("lane ensure, start mode, lane_id and releasing a finished run", () =>
|
||||
|
||||
// Mock tmux so the session appears to be gone (has-session fails with status 1).
|
||||
tmux.__setExecImpl((args) => {
|
||||
if (args[0] === "has-session" && args[1] === "-t" && args[2] === runId) {
|
||||
if (args[0] === "has-session" && args[1] === "-t" && args[2] === `=${runId}`) {
|
||||
const e = new Error("no such session");
|
||||
e.status = 1;
|
||||
throw e;
|
||||
|
||||
@@ -321,6 +321,27 @@ describe("/api/lanes", () => {
|
||||
await request("DELETE", `/api/lanes/${id}`);
|
||||
fs.rmSync(cwd, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("router apply asks for a restart only while the lane's pane is live", async () => {
|
||||
const tmux = require("../lib/tmux");
|
||||
const cwd = fs.mkdtempSync(path.join(os.tmpdir(), "lane-router-live-"));
|
||||
const id = (await request("POST", "/api/lanes", { cwd })).body.lane.id;
|
||||
try {
|
||||
for (const live of [false, true]) {
|
||||
tmux.__setExecImpl((args) => {
|
||||
if (args[0] === "has-session" && !live) throw new Error("no such session");
|
||||
return "";
|
||||
});
|
||||
const r = await request("POST", `/api/lanes/${id}/router/apply`, { profile: "native" });
|
||||
assert.equal(r.status, 200);
|
||||
assert.equal(r.body.restart_required, live);
|
||||
assert.equal(r.body.run_id, live ? `agenthub-lane-${id}` : null);
|
||||
}
|
||||
} finally {
|
||||
tmux.__reset();
|
||||
fs.rmSync(cwd, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("hook → lane binding", () => {
|
||||
@@ -396,7 +417,7 @@ describe("hook → lane binding", () => {
|
||||
assert.equal((await request("GET", `/api/lanes/${id}`)).body.lane.session_id, "sess-b");
|
||||
assert.equal((await request("GET", `/api/lanes/${id}`)).body.lane.needs_action, "blocked");
|
||||
|
||||
// Session B's second PostToolUse clears the flag (B is now bound)
|
||||
// B is bound now, but it still did not raise the flag: A is still waiting.
|
||||
await request("POST", "/api/hooks/event", {
|
||||
hook_type: "PostToolUse",
|
||||
data: {
|
||||
@@ -406,6 +427,18 @@ describe("hook → lane binding", () => {
|
||||
tool_use_id: "t2",
|
||||
},
|
||||
});
|
||||
assert.equal((await request("GET", `/api/lanes/${id}`)).body.lane.needs_action, "blocked");
|
||||
|
||||
// A answering its own prompt is what clears it.
|
||||
await request("POST", "/api/hooks/event", {
|
||||
hook_type: "PostToolUse",
|
||||
data: {
|
||||
session_id: "sess-a",
|
||||
cwd: "/tmp/lane-cross-session",
|
||||
tool_name: "Read",
|
||||
tool_use_id: "t3",
|
||||
},
|
||||
});
|
||||
assert.equal((await request("GET", `/api/lanes/${id}`)).body.lane.needs_action, null);
|
||||
|
||||
await request("DELETE", `/api/lanes/${id}`);
|
||||
|
||||
@@ -66,7 +66,7 @@ describe("pty-attach", () => {
|
||||
return "";
|
||||
});
|
||||
ptyAttach.attach(makeFakeWs(), "agenthub-lane-1", { cols: 80, rows: 24 });
|
||||
assert.deepEqual(calls[0], ["set-option", "-t", "agenthub-lane-1", "mouse", "on"]);
|
||||
assert.deepEqual(calls[0], ["set-option", "-t", "=agenthub-lane-1:", "mouse", "on"]);
|
||||
});
|
||||
|
||||
it("still attaches even if the mouse-mode set-option call fails", () => {
|
||||
|
||||
@@ -241,7 +241,18 @@ describe("pty-run", () => {
|
||||
return "";
|
||||
});
|
||||
assert.equal(pty.killRun("agenthub-lane-5"), true);
|
||||
assert.ok(calls.some((c) => c[0] === "kill-session" && c[2] === "agenthub-lane-5"));
|
||||
assert.ok(calls.some((c) => c[0] === "kill-session" && c[2] === "=agenthub-lane-5"));
|
||||
});
|
||||
|
||||
it("never touches a tmux session that is not a lane pane", () => {
|
||||
const calls = [];
|
||||
tmux.__setExecImpl((args) => {
|
||||
calls.push(args);
|
||||
return "";
|
||||
});
|
||||
assert.equal(pty.killRun("main"), false);
|
||||
assert.equal(pty.getRun("main"), null);
|
||||
assert.deepEqual(calls, []);
|
||||
});
|
||||
|
||||
it("listRuns reflects live tmux-session state, not cached memory", () => {
|
||||
|
||||
@@ -51,7 +51,7 @@ describe("tmux wrapper", () => {
|
||||
"--model",
|
||||
"opus",
|
||||
]);
|
||||
assert.deepEqual(calls[1], ["set-option", "-t", "agenthub-lane-1", "mouse", "on"]);
|
||||
assert.deepEqual(calls[1], ["set-option", "-t", "=agenthub-lane-1:", "mouse", "on"]);
|
||||
});
|
||||
|
||||
it("sendText types the literal string and presses Enter, with no quoting", () => {
|
||||
@@ -64,11 +64,25 @@ describe("tmux wrapper", () => {
|
||||
assert.deepEqual(calls[0], [
|
||||
"send-keys",
|
||||
"-t",
|
||||
"agenthub-lane-1",
|
||||
"=agenthub-lane-1:",
|
||||
"-l",
|
||||
'Reminder: run "agenthub stage implement"',
|
||||
]);
|
||||
assert.deepEqual(calls[1], ["send-keys", "-t", "agenthub-lane-1", "Enter"]);
|
||||
assert.deepEqual(calls[1], ["send-keys", "-t", "=agenthub-lane-1:", "Enter"]);
|
||||
});
|
||||
|
||||
it("targets a session exactly, so lane 1 never resolves to lane 12 by prefix", () => {
|
||||
const calls = [];
|
||||
tmux.__setExecImpl((args) => {
|
||||
calls.push(args);
|
||||
return "";
|
||||
});
|
||||
tmux.hasSession("agenthub-lane-1");
|
||||
tmux.killSession("agenthub-lane-1");
|
||||
assert.deepEqual(calls, [
|
||||
["has-session", "-t", "=agenthub-lane-1"],
|
||||
["kill-session", "-t", "=agenthub-lane-1"],
|
||||
]);
|
||||
});
|
||||
|
||||
it("killSession never throws when the session is already gone", () => {
|
||||
|
||||
@@ -695,6 +695,17 @@ try {
|
||||
}
|
||||
db.prepare("CREATE UNIQUE INDEX IF NOT EXISTS idx_lanes_cwd_tool ON lanes(cwd, tool)").run();
|
||||
|
||||
// Migrate: which session raised `needs_action`. Two sessions in one lane's cwd
|
||||
// (a hand-started terminal beside the dashboard pane) rebind `session_id` on
|
||||
// every hook, so "the bound session" is not "the session that is waiting".
|
||||
// Additive and nullable; null (a flag raised before this column) falls back to
|
||||
// the bound session. Placed after the rebuild above, which lists its columns.
|
||||
try {
|
||||
db.prepare("SELECT needs_action_session FROM lanes LIMIT 1").get();
|
||||
} catch {
|
||||
db.prepare("ALTER TABLE lanes ADD COLUMN needs_action_session TEXT").run();
|
||||
}
|
||||
|
||||
// Seed lane_stage_events from the snapshots that predate it, so upgrading does
|
||||
// not read as "every pipeline has never run". Each lane's live `stages` blob
|
||||
// becomes one run; each archived feature becomes one more. That is all the
|
||||
|
||||
@@ -69,6 +69,7 @@ const PATCHABLE = new Set([
|
||||
"gate_decision",
|
||||
"ci_status",
|
||||
"needs_action",
|
||||
"needs_action_session",
|
||||
"links",
|
||||
"notes",
|
||||
"session_id",
|
||||
|
||||
@@ -48,7 +48,7 @@ function attach(ws, runId, { cols, rows }) {
|
||||
} catch {
|
||||
/* session gone or tmux unavailable — attach below will surface that */
|
||||
}
|
||||
const pty = spawnImpl("tmux", ["attach-session", "-t", runId], {
|
||||
const pty = spawnImpl("tmux", ["attach-session", "-t", `=${runId}`], {
|
||||
name: "xterm-256color",
|
||||
cols: cols || 80,
|
||||
rows: rows || 24,
|
||||
|
||||
@@ -322,8 +322,10 @@ function spawnRun(args) {
|
||||
return getRun(id);
|
||||
}
|
||||
|
||||
// Only a dashboard lane pane is a run. Without this, `DELETE /api/run/main`
|
||||
// would kill the user's own tmux session named `main`.
|
||||
function killRun(id) {
|
||||
if (!id || !tmux.hasSession(id)) return false;
|
||||
if (laneIdFromRunId(id) === null || !tmux.hasSession(id)) return false;
|
||||
tmux.killSession(id);
|
||||
if (dashboardRuns) {
|
||||
dashboardRuns.patchRun({ id, status: "killed", endedAt: Date.now() });
|
||||
@@ -353,7 +355,7 @@ function publicRun(id) {
|
||||
}
|
||||
|
||||
function getRun(id) {
|
||||
if (!id) return null;
|
||||
if (laneIdFromRunId(id) === null) return null;
|
||||
return publicRun(id);
|
||||
}
|
||||
|
||||
|
||||
+15
-7
@@ -33,9 +33,16 @@ function __reset() {
|
||||
execImpl = (args) => execFileSync("tmux", args, EXEC_OPTS);
|
||||
}
|
||||
|
||||
// tmux resolves a bare `-t agenthub-lane-1` by PREFIX when no exact match
|
||||
// exists, so it would hit `agenthub-lane-12` once lane 1's session is gone —
|
||||
// and killSession would kill the wrong lane. `=` forces an exact match; pane
|
||||
// and option targets need the trailing `:` (tmux 3.4 rejects `=name` there).
|
||||
const session = (name) => `=${name}`;
|
||||
const pane = (name) => `=${name}:`;
|
||||
|
||||
function hasSession(name) {
|
||||
try {
|
||||
execImpl(["has-session", "-t", name]);
|
||||
execImpl(["has-session", "-t", session(name)]);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
@@ -71,7 +78,7 @@ function newSession({ name, cwd, argv, env }) {
|
||||
// this option existed — or before a server restart picked up the change —
|
||||
// still gets it; `set-option` is idempotent, so re-running it is harmless.
|
||||
function enableMouse(name) {
|
||||
execImpl(["set-option", "-t", name, "mouse", "on"]);
|
||||
execImpl(["set-option", "-t", pane(name), "mouse", "on"]);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -82,7 +89,8 @@ function enableMouse(name) {
|
||||
function paneCommand(name) {
|
||||
try {
|
||||
return (
|
||||
execImpl(["display-message", "-p", "-t", name, "#{pane_current_command}"]).trim() || null
|
||||
execImpl(["display-message", "-p", "-t", pane(name), "#{pane_current_command}"]).trim() ||
|
||||
null
|
||||
);
|
||||
} catch {
|
||||
return null;
|
||||
@@ -107,7 +115,7 @@ function sendCommand(name, argv, env, cwd) {
|
||||
"respawn-pane",
|
||||
"-k",
|
||||
"-t",
|
||||
name,
|
||||
pane(name),
|
||||
...(cwd ? ["-c", cwd] : []),
|
||||
...envFlags(env),
|
||||
"--",
|
||||
@@ -125,14 +133,14 @@ function sendCommand(name, argv, env, cwd) {
|
||||
* only the first line and leave the rest sitting unsent in the input box.
|
||||
*/
|
||||
function sendText(name, text) {
|
||||
execImpl(["send-keys", "-t", name, "-l", text]);
|
||||
execImpl(["send-keys", "-t", name, "Enter"]);
|
||||
execImpl(["send-keys", "-t", pane(name), "-l", text]);
|
||||
execImpl(["send-keys", "-t", pane(name), "Enter"]);
|
||||
}
|
||||
|
||||
/** Idempotent — a session that's already gone is not an error. */
|
||||
function killSession(name) {
|
||||
try {
|
||||
execImpl(["kill-session", "-t", name]);
|
||||
execImpl(["kill-session", "-t", session(name)]);
|
||||
} catch {
|
||||
/* already gone */
|
||||
}
|
||||
|
||||
+11
-3
@@ -210,10 +210,18 @@ function touchLaneFromHook(hookType, data) {
|
||||
if (data.session_id && lane.session_id !== data.session_id) patch.session_id = data.session_id;
|
||||
if (hookType === "Notification" && !idleNudge) {
|
||||
patch.needs_action = data.message || "needs you";
|
||||
} else if (lane.needs_action && data.session_id === lane.session_id) {
|
||||
// Clear only when the hook comes from the session currently bound to the lane
|
||||
// (evaluated before any rebinding, so a rebinding hook never clears in the same pass)
|
||||
patch.needs_action_session = data.session_id || null;
|
||||
} else if (
|
||||
lane.needs_action &&
|
||||
data.session_id &&
|
||||
data.session_id === (lane.needs_action_session || lane.session_id)
|
||||
) {
|
||||
// Clear only from the session that raised it: another session sharing the
|
||||
// cwd rebinds session_id on every hook but is not the one waiting. A flag
|
||||
// with no recorded raiser falls back to the bound session (evaluated
|
||||
// before any rebinding, so a rebinding hook never clears in the same pass).
|
||||
patch.needs_action = null;
|
||||
patch.needs_action_session = null;
|
||||
}
|
||||
|
||||
// Only idle↔running is mirrored: provisioning and failed are lifecycle
|
||||
|
||||
@@ -690,7 +690,7 @@ router.post("/:id/router/apply", sameOriginGuard, (req, res) => {
|
||||
// outside; the pane has to be restarted. Report it instead of letting the
|
||||
// switch look clean and then fail on the user's next prompt.
|
||||
const runId = runs.runIdForLane(lane.id);
|
||||
const live = !!runs.getRun(runId);
|
||||
const live = runs.getRun(runId)?.status === "running";
|
||||
// Every other mutating lane route broadcasts; without it the client's
|
||||
// cached row keeps the OLD router_profile, and the next lane switch
|
||||
// re-reads that stale row and flips the picker back.
|
||||
|
||||
Reference in New Issue
Block a user