* fix(web): drop = prefix from set-option in mux-websocket (closes #1714) In tmux 3.4 the `=` exact-match prefix only works with `has-session` and `attach-session`. For `set-option`, the prefix is silently ignored, so `mouse on` and `status off` never get applied — breaking scroll wheel in the dashboard terminal and leaving the tmux status bar visible. Use the bare session id for the two `set-option` calls; keep `=` on `attach-session` where it is correct. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(web): move exactTmuxTarget next to attach-session Address review feedback: the `=`-prefixed target is only used by attach-session, so declare it adjacent to that call. Comment now sits above the set-option calls it actually explains. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Prateek <karnalprateek@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
0f539a3d4e
commit
d0dff3b93f
|
|
@ -1,12 +1,48 @@
|
|||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { SessionBroadcaster } from "../mux-websocket";
|
||||
import { EventEmitter } from "node:events";
|
||||
import type { SessionBroadcaster as SessionBroadcasterType } from "../mux-websocket";
|
||||
|
||||
// vi.mock factories run before module-level statements. Hoist the mock
|
||||
// fns so the factories close over the same instances the tests use.
|
||||
const { mockSpawn, mockPtySpawn } = vi.hoisted(() => ({
|
||||
mockSpawn: vi.fn(),
|
||||
mockPtySpawn: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("node:child_process", async (importOriginal) => {
|
||||
const actual = (await importOriginal()) as Record<string, unknown>;
|
||||
const spawnFn = (...args: unknown[]) => mockSpawn(...args);
|
||||
return {
|
||||
...actual,
|
||||
default: { ...(actual.default as object), spawn: spawnFn },
|
||||
spawn: spawnFn,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("node-pty", async (importOriginal) => {
|
||||
const actual = (await importOriginal()) as Record<string, unknown>;
|
||||
return {
|
||||
...actual,
|
||||
spawn: (...args: unknown[]) => mockPtySpawn(...args),
|
||||
};
|
||||
});
|
||||
|
||||
// Mock tmux-utils so resolveTmuxSession returns a deterministic session id
|
||||
// and we don't shell out to a real tmux binary.
|
||||
vi.mock("../tmux-utils.js", () => ({
|
||||
findTmux: () => "/usr/bin/tmux",
|
||||
validateSessionId: () => true,
|
||||
resolveTmuxSession: () => "ao-177",
|
||||
}));
|
||||
|
||||
const { SessionBroadcaster, TerminalManager } = await import("../mux-websocket");
|
||||
|
||||
// Mock global fetch
|
||||
const mockFetch = vi.fn();
|
||||
global.fetch = mockFetch;
|
||||
|
||||
describe("SessionBroadcaster", () => {
|
||||
let broadcaster: SessionBroadcaster;
|
||||
let broadcaster: SessionBroadcasterType;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
|
|
@ -225,3 +261,53 @@ describe("SessionBroadcaster", () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("TerminalManager.open — tmux target args (regression for #1714)", () => {
|
||||
beforeEach(() => {
|
||||
mockSpawn.mockReset();
|
||||
mockPtySpawn.mockReset();
|
||||
|
||||
// spawn() returns an object that emits "error" — we just need .on() to work.
|
||||
mockSpawn.mockImplementation(() => new EventEmitter());
|
||||
|
||||
// ptySpawn() returns a minimal IPty-like stub so terminal wiring doesn't crash.
|
||||
mockPtySpawn.mockImplementation(() => ({
|
||||
onData: vi.fn(),
|
||||
onExit: vi.fn(),
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(),
|
||||
kill: vi.fn(),
|
||||
}));
|
||||
});
|
||||
|
||||
it("invokes set-option mouse on with the bare session id (no = prefix)", () => {
|
||||
const mgr = new TerminalManager("/usr/bin/tmux");
|
||||
mgr.open("ao-177");
|
||||
|
||||
const mouseCall = mockSpawn.mock.calls.find(
|
||||
(call) => Array.isArray(call[1]) && call[1].includes("mouse"),
|
||||
);
|
||||
expect(mouseCall).toBeDefined();
|
||||
expect(mouseCall?.[1]).toEqual(["set-option", "-t", "ao-177", "mouse", "on"]);
|
||||
});
|
||||
|
||||
it("invokes set-option status off with the bare session id (no = prefix)", () => {
|
||||
const mgr = new TerminalManager("/usr/bin/tmux");
|
||||
mgr.open("ao-177");
|
||||
|
||||
const statusCall = mockSpawn.mock.calls.find(
|
||||
(call) => Array.isArray(call[1]) && call[1].includes("status"),
|
||||
);
|
||||
expect(statusCall).toBeDefined();
|
||||
expect(statusCall?.[1]).toEqual(["set-option", "-t", "ao-177", "status", "off"]);
|
||||
});
|
||||
|
||||
it("still uses the = exact-match prefix for attach-session", () => {
|
||||
const mgr = new TerminalManager("/usr/bin/tmux");
|
||||
mgr.open("ao-177");
|
||||
|
||||
expect(mockPtySpawn).toHaveBeenCalledTimes(1);
|
||||
const [, args] = mockPtySpawn.mock.calls[0];
|
||||
expect(args).toEqual(["attach-session", "-t", "=ao-177"]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -222,7 +222,7 @@ const REATTACH_RESET_GRACE_MS = 5_000;
|
|||
* TerminalManager manages PTY processes independently of WebSocket connections.
|
||||
* A single manager instance is shared across all mux connections.
|
||||
*/
|
||||
class TerminalManager {
|
||||
export class TerminalManager {
|
||||
private terminals = new Map<string, ManagedTerminal>();
|
||||
private TMUX: string;
|
||||
|
||||
|
|
@ -274,16 +274,18 @@ class TerminalManager {
|
|||
return tmuxSessionId;
|
||||
}
|
||||
|
||||
// Enable mouse mode
|
||||
const exactTmuxTarget = `=${tmuxSessionId}`;
|
||||
// tmux 3.4 only honours the `=` exact-match prefix on has-session and
|
||||
// attach-session; set-option silently ignores it, so we use the bare id
|
||||
// here. The `=`-prefixed form is built below for attach-session.
|
||||
|
||||
const mouseProc = spawn(this.TMUX, ["set-option", "-t", exactTmuxTarget, "mouse", "on"]);
|
||||
// Enable mouse mode
|
||||
const mouseProc = spawn(this.TMUX, ["set-option", "-t", tmuxSessionId, "mouse", "on"]);
|
||||
mouseProc.on("error", (err) => {
|
||||
console.error(`[MuxServer] Failed to set mouse mode for ${tmuxSessionId}:`, err.message);
|
||||
});
|
||||
|
||||
// Hide the status bar
|
||||
const statusProc = spawn(this.TMUX, ["set-option", "-t", exactTmuxTarget, "status", "off"]);
|
||||
const statusProc = spawn(this.TMUX, ["set-option", "-t", tmuxSessionId, "status", "off"]);
|
||||
statusProc.on("error", (err) => {
|
||||
console.error(`[MuxServer] Failed to hide status bar for ${tmuxSessionId}:`, err.message);
|
||||
});
|
||||
|
|
@ -305,7 +307,9 @@ class TerminalManager {
|
|||
throw new Error("node-pty not available");
|
||||
}
|
||||
|
||||
// Spawn PTY
|
||||
// Spawn PTY — use `=`-prefixed exact-match target so we never attach to
|
||||
// a session whose name happens to be a prefix of the requested id.
|
||||
const exactTmuxTarget = `=${tmuxSessionId}`;
|
||||
const pty = ptySpawn(this.TMUX, ["attach-session", "-t", exactTmuxTarget], {
|
||||
name: "xterm-256color",
|
||||
cols: 80,
|
||||
|
|
|
|||
Loading…
Reference in New Issue