mirror of
https://github.com/mvschwarz/openrig.git
synced 2026-11-07 05:56:21 -10:00
fix(tmux): probe canonical session names without prefix matching (#449)
tmux session lookups are exact: a session whose name literally starts with '=' resolves to its own pane, so removing it kills the right pane and leaves a plain-named neighbour alone.
This commit is contained in:
1 parent
78ab5ceb93
commit
fb31f39650
6 files changed
+125
-12
No files matched your search
@@ -471,9 +471,16 @@ export class TmuxAdapter {
|
||||
}
|
||||
|
||||
async listPanes(target: string): Promise<TmuxPane[]> {
|
||||
// Callers name a session (optionally with a window), or an immutable tmux
|
||||
// id. Exact session matching prevents observing a prefix neighbor's pane.
|
||||
// A bare leading '=' belongs to the literal session name. Only qualified
|
||||
// targets already carry tmux's encoded exact-match syntax.
|
||||
const namedTarget = target.includes(":") && target.startsWith("=") ? target : `=${target}`;
|
||||
const exactTarget = /^[%$@]\d+$/.test(target) ? target
|
||||
: namedTarget.includes(":") ? namedTarget : `${namedTarget}:`;
|
||||
try {
|
||||
const output = await this.run(["tmux", "list-panes", "-t", target, "-F", PANE_FORMAT],
|
||||
`tmux list-panes -t ${shellQuote(target)} -F "${PANE_FORMAT}"`);
|
||||
const output = await this.run(["tmux", "list-panes", "-t", exactTarget, "-F", PANE_FORMAT],
|
||||
`tmux list-panes -t ${shellQuote(exactTarget)} -F "${PANE_FORMAT}"`);
|
||||
return parseLines(output, parsePaneLine);
|
||||
} catch (err) {
|
||||
if (isNoServerError(err) || isTmuxTransportAbsentError(err)) return [];
|
||||
@@ -496,8 +503,9 @@ export class TmuxAdapter {
|
||||
// Use `tmux has-session` directly for reliable existence check — avoids
|
||||
// parsing format-string output from `list-sessions` which can fail when
|
||||
// tab delimiters are malformed across tmux versions.
|
||||
await this.run(["tmux", "has-session", "-t", name],
|
||||
`tmux has-session -t ${shellQuote(name)}`);
|
||||
const target = `=${name}`; // canonical session name, never a prefix lookup
|
||||
await this.run(["tmux", "has-session", "-t", target],
|
||||
`tmux has-session -t ${shellQuote(target)}`);
|
||||
return { state: "present" }; // exit 0 = session exists
|
||||
} catch (err) {
|
||||
if (isSessionAbsenceError(err)) {
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { expect, it } from "vitest";
|
||||
|
||||
it("probes exact native session names so a live prefix neighbor cannot hide a detached seat", () => {
|
||||
const home = mkdtempSync(join(tmpdir(), "openrig-exact-probe-"));
|
||||
try {
|
||||
const result = spawnSync(process.execPath, [
|
||||
fileURLToPath(new URL("./fixtures/exact-session-probe.mjs", import.meta.url)), home,
|
||||
], { env: { ...process.env, HOME: home, TMUX: "" }, encoding: "utf8", timeout: 15000 });
|
||||
expect(result.error, result.stderr).toBeUndefined();
|
||||
expect(result.status, result.stderr).toBe(0);
|
||||
expect(JSON.parse(result.stdout.trim())).toEqual({ nativeTmux: true, argvAndLegacy: true, missingDetached: true, neighborPreserved: true, exactPaneListing: true });
|
||||
} finally { rmSync(home, { recursive: true, force: true }); }
|
||||
}, 20000);
|
||||
@@ -0,0 +1,83 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import { join } from "node:path";
|
||||
import { pathToFileURL, fileURLToPath } from "node:url";
|
||||
import Database from "better-sqlite3";
|
||||
const [home, sourceRoot] = process.argv.slice(2);
|
||||
const moduleRoot = sourceRoot ? join(sourceRoot, "packages/daemon/src") : fileURLToPath(new URL("../../dist/", import.meta.url));
|
||||
const load = name => import(pathToFileURL(join(moduleRoot, name + (sourceRoot ? ".ts" : ".js"))));
|
||||
const { TmuxAdapter } = await load("adapters/tmux");
|
||||
const { migrate } = await load("db/migrate"), { ALL_MIGRATIONS } = await load("db/all-migrations");
|
||||
const { RigRepository } = await load("domain/rig-repository"), { SessionRegistry } = await load("domain/session-registry");
|
||||
const { EventBus } = await load("domain/event-bus"), { Reconciler } = await load("domain/reconciler");
|
||||
const execute = promisify(execFile), socket = join(home, "tmux.sock");
|
||||
const native = async args => (await execute("tmux", ["-S", socket, ...args], { timeout: 5000 })).stdout;
|
||||
const quote = s => "'" + s.replace(/'/g, "'\"'\"'") + "'";
|
||||
const shell = async command => (await execute("sh", ["-c", command.replace(/^tmux /, `tmux -S ${quote(socket)} `)], { timeout: 5000 })).stdout;
|
||||
const argv = new TmuxAdapter(shell, undefined, args => native(args.slice(1)));
|
||||
const legacy = new TmuxAdapter(shell);
|
||||
const db = new Database(":memory:");
|
||||
try {
|
||||
await native(["new-session", "-d", "-s", "worker@demo2", "sleep 120"]);
|
||||
const missingProbes = [];
|
||||
for (const adapter of [argv, legacy]) {
|
||||
assert.equal((await adapter.probeSession("worker@demo2")).state, "present", "exact existing session");
|
||||
missingProbes.push((await adapter.probeSession("worker@demo")).state);
|
||||
const panes = await adapter.listPanes("worker@demo2");
|
||||
assert.equal(panes.length, 1, "the exact existing session pane remains visible");
|
||||
assert.equal((await adapter.listPanes("worker@demo2:0"))[0].id, panes[0].id, "named session window remains exact");
|
||||
assert.equal((await adapter.listPanes(panes[0].id))[0].id, panes[0].id, "immutable pane-id target remains supported");
|
||||
const ids = (await native(["display-message", "-p", "-t", "=worker@demo2:", "#{session_id} #{window_id}"])).trim().split(" ");
|
||||
for (const id of ids) assert.equal((await adapter.listPanes(id))[0].id, panes[0].id, "immutable session/window ids remain supported");
|
||||
assert.equal((await adapter.listPanes("=worker@demo2:0"))[0].id, panes[0].id, "already-exact target remains supported");
|
||||
await assert.rejects(adapter.listPanes("worker@demo"), /can't find (?:window|session)/,
|
||||
"missing list-panes target must not resolve to the live prefix neighbor");
|
||||
}
|
||||
migrate(db, ALL_MIGRATIONS);
|
||||
const repo = new RigRepository(db), registry = new SessionRegistry(db), eventBus = new EventBus(db);
|
||||
const rig = repo.createRig("demo"), neighborRig = repo.createRig("demo2");
|
||||
const missingNode = repo.addNode(rig.id, "worker", { runtime: "terminal" });
|
||||
const liveNode = repo.addNode(neighborRig.id, "worker", { runtime: "terminal" });
|
||||
const missing = registry.registerSession(missingNode.id, "worker@demo");
|
||||
const live = registry.registerSession(liveNode.id, "worker@demo2");
|
||||
registry.updateStatus(missing.id, "running"); registry.updateStatus(live.id, "running");
|
||||
const reconciler = new Reconciler({ db, sessionRegistry: registry, eventBus, tmuxAdapter: argv });
|
||||
assert.deepEqual(await reconciler.reconcile(rig.id), { checked: 1, detached: 1, errors: [] });
|
||||
assert.deepEqual(missingProbes, ["absent", "absent"], "missing session must not prefix-match its neighbor");
|
||||
assert.equal(registry.getSessionsForRig(rig.id)[0].status, "detached");
|
||||
assert.equal(registry.getSessionsForRig(neighborRig.id)[0].status, "running");
|
||||
assert.equal((await argv.probeSession("worker@demo2")).state, "present");
|
||||
// Literal-leading-equals case also reported by lab1207 in PR #460.
|
||||
// Each arrangement uses real tmux targets through both adapter APIs.
|
||||
const literal = "=worker@demo", plain = "worker@demo";
|
||||
for (const names of [[literal], [plain], [literal, plain]]) {
|
||||
await native(["kill-server"]);
|
||||
// Keep the private server live when the literal case removes its only pane.
|
||||
await native(["new-session", "-d", "-s", "fixture-anchor", "sleep 120"]);
|
||||
for (const name of names) await native(["new-session", "-d", "-s", name, "sleep 120"]);
|
||||
for (const adapter of [argv, legacy]) {
|
||||
for (const name of names) {
|
||||
const expected = (await native(["display-message", "-p", "-t", `=${name}:`, "#{pane_id}"])).trim();
|
||||
const panes = await adapter.listPanes(name);
|
||||
assert.equal(panes.length, 1);
|
||||
assert.equal(panes[0].id, expected, "a literal session name must select its own pane");
|
||||
assert.equal((await adapter.listPanes(`=${name}:0`))[0].id, expected, "encoded qualified target stays exact");
|
||||
assert.equal((await adapter.listPanes(expected))[0].id, expected, "pane id stays unchanged");
|
||||
const ids = (await native(["display-message", "-p", "-t", `=${name}:`, "#{session_id} #{window_id}"])).trim().split(" ");
|
||||
for (const id of ids) assert.equal((await adapter.listPanes(id))[0].id, expected);
|
||||
}
|
||||
for (const absent of [literal, plain].filter(name => !names.includes(name))) {
|
||||
assert.equal((await adapter.probeSession(absent)).state, "absent");
|
||||
await assert.rejects(adapter.listPanes(absent), /can't find (?:window|session)/);
|
||||
}
|
||||
}
|
||||
if (names.includes(literal)) {
|
||||
const [pane] = await argv.listPanes(literal);
|
||||
await native(["kill-pane", "-t", pane.id]);
|
||||
assert.equal((await argv.probeSession(literal)).state, "absent", "selected literal pane is really stopped");
|
||||
if (names.includes(plain)) assert.equal((await argv.probeSession(plain)).state, "present", "plain neighbor survives literal removal");
|
||||
}
|
||||
}
|
||||
console.log(JSON.stringify({ nativeTmux: true, argvAndLegacy: true, missingDetached: true, neighborPreserved: true, exactPaneListing: true }));
|
||||
} finally { db.close(); await native(["kill-server"]).catch(() => {}); }
|
||||
@@ -32,7 +32,9 @@ class FakeTmux {
|
||||
exec = async (command: string): Promise<string> => {
|
||||
this.commands.push(command);
|
||||
this.onCommand?.(command);
|
||||
const target = /-t '([^']+)'/.exec(command)?.[1];
|
||||
// Native list-panes uses an exact session/window target (=name:), while
|
||||
// has-session uses =name; immutable tmux ids retain their literal spelling.
|
||||
const target = /-t '([^']+)'/.exec(command)?.[1]?.replace(/^=/, "").replace(/:.*$/, "");
|
||||
const byTarget = () => [...this.sessions.entries()].find(([name, s]) => name === target || s.pane === target || s.id === target);
|
||||
if (command.startsWith("tmux new-session")) {
|
||||
const name = /-s '([^']+)'/.exec(command)![1]!;
|
||||
@@ -180,7 +182,7 @@ describe("#188 snapshot refresh probe cleanup", () => {
|
||||
const tmux = new FakeTmux(40);
|
||||
const seat = claudeSeat("%7");
|
||||
tmux.onCommand = (command) => {
|
||||
const name = /list-panes -t '(rigged-refresh-[^']+)'/.exec(command)?.[1];
|
||||
const name = /list-panes -t '=(rigged-refresh-[^':]+):'/.exec(command)?.[1];
|
||||
if (!name || tmux.sessions.get(name) === undefined) return;
|
||||
const rig = rigRepo.createRig("adopter");
|
||||
const node = rigRepo.addNode(rig.id, "dev.adopted", { role: "worker", runtime: "claude-code" });
|
||||
@@ -198,7 +200,7 @@ describe("#188 snapshot refresh probe cleanup", () => {
|
||||
const seat = claudeSeat("%7");
|
||||
let replacement: { id: string; pane: string } | undefined;
|
||||
tmux.onCommand = (command) => {
|
||||
const name = /list-panes -t '(rigged-refresh-[^']+)'/.exec(command)?.[1];
|
||||
const name = /list-panes -t '=(rigged-refresh-[^':]+):'/.exec(command)?.[1];
|
||||
if (!name || replacement) return;
|
||||
tmux.replace(name);
|
||||
replacement = { ...tmux.sessions.get(name)! };
|
||||
@@ -215,7 +217,7 @@ describe("#188 snapshot refresh probe cleanup", () => {
|
||||
tmux.listPanesFails = true;
|
||||
const seat = claudeSeat("%7");
|
||||
tmux.onCommand = (command) => {
|
||||
if (!/list-panes -t 'rigged-refresh-/.test(command) || tmux.sessions.has("unrelated")) return;
|
||||
if (!/list-panes -t '=rigged-refresh-/.test(command) || tmux.sessions.has("unrelated")) return;
|
||||
tmux.resetServer();
|
||||
tmux.create("unrelated"); // the new server hands out the helper's id again
|
||||
};
|
||||
|
||||
@@ -143,7 +143,7 @@ describe("TmuxAdapter", () => {
|
||||
|
||||
expect(exec).toHaveBeenCalledOnce();
|
||||
expect(exec.mock.calls[0]![0]).toBe(
|
||||
'tmux list-panes -t \'my-session:0\' -F "#{pane_id}|#{pane_index}|#{pane_current_path}|#{pane_width}|#{pane_height}|#{pane_active}"'
|
||||
'tmux list-panes -t \'=my-session:0\' -F "#{pane_id}|#{pane_index}|#{pane_current_path}|#{pane_width}|#{pane_height}|#{pane_active}"'
|
||||
);
|
||||
});
|
||||
|
||||
@@ -155,7 +155,7 @@ describe("TmuxAdapter", () => {
|
||||
|
||||
expect(exec).toHaveBeenCalledOnce();
|
||||
expect(exec.mock.calls[0]![0]).toBe(
|
||||
'tmux list-panes -t \'my session\'\"\'\"\'s:0\' -F "#{pane_id}|#{pane_index}|#{pane_current_path}|#{pane_width}|#{pane_height}|#{pane_active}"'
|
||||
'tmux list-panes -t \'=my session\'\"\'\"\'s:0\' -F "#{pane_id}|#{pane_index}|#{pane_current_path}|#{pane_width}|#{pane_height}|#{pane_active}"'
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -35,8 +35,10 @@ async function fixture() {
|
||||
commands.push(command); hooks.exec?.(command);
|
||||
if (command.startsWith("tmux new-session")) { panes.set("worker@fixture", "%0"); return ""; }
|
||||
if (command.startsWith("tmux has-session") || command.startsWith("tmux list-panes")) {
|
||||
const name = [...panes.keys()].find(name => command.includes(`'${name}'`));
|
||||
if (!name) throw Error("can't find session");
|
||||
// Both exact session probes (=name) and window-targeted pane listings
|
||||
// (=name:) address the same canonical session in the fixture server.
|
||||
const name = /-t '([^']+)'/.exec(command)?.[1]?.replace(/^=/, "").replace(/:$/, "");
|
||||
if (!name || !panes.has(name)) throw Error("can't find session");
|
||||
return command.startsWith("tmux list-panes") ? `${panes.get(name)}|0|/inert|80|24|1\n` : "";
|
||||
}
|
||||
return "";
|
||||
|
||||
Reference in new issue
Block a user