mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 06:29:42 +02:00
fix(tmux): isolate sessions on a dedicated socket + raise pane nofile limit (fixes new-session crash after tmux upgrade) (#96)
* fix: isolate codeman tmux sessions
* fix(tmux): unify all sessions onto a single dedicated socket
Remove the per-session `tmuxSocket` field that recorded which tmux server
each session lived on (default vs the `codeman` socket). That field was a
persisted cache of physical reality and could drift — causing live sessions
to be wrongly marked dead ("tab shows no session found") and spawning
duplicate "Restored:" tabs.
All Codeman sessions now live on one process-wide socket (`tmux -L codeman`,
overridable via CODEMAN_TMUX_SOCKET), exposed via TmuxManager.muxSocket on
the TerminalMultiplexer interface. reconcileSessions() collapses from a
multi-socket scan (locate / re-pin / cross-socket dedup) to a single
`list-panes` query. loadSessions() strips the obsolete field from on-disk
records so it stops being written back.
Also fix two sibling bare-`tmux` call sites the unification would otherwise
leave broken (same #80 regression class — bare tmux hits the user's default
server and never finds a session on the codeman socket):
- session.ts queryTmuxWindowSize(): add `-L <socket>` (was silently falling
back to 120x40 on re-attach, losing scrollback)
- session-routes.ts send-key (Shift+Enter / Ctrl+Enter newline): route
through ctx.mux.muxSocket
SSH chooser scripts (tmux-manager.sh, tmux-chooser.sh) route every tmux call
through `tmux -L $CODEMAN_TMUX_SOCKET`, matching the TS default.
---------
Co-authored-by: Teigen <teigen@TeigendeMac-mini.local>
This commit is contained in:
@@ -90,6 +90,7 @@ export function createMockRouteContext(options?: { sessionId?: string }) {
|
||||
|
||||
// -- InfraPort --
|
||||
mux: {
|
||||
muxSocket: 'codeman',
|
||||
createSession: vi.fn(),
|
||||
killSession: vi.fn(),
|
||||
listSessions: vi.fn(() => []),
|
||||
|
||||
@@ -7,6 +7,14 @@
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js';
|
||||
|
||||
// Mock execFile so the send-key route's `tmux` invocation is observable (not run for real).
|
||||
const { execFile } = vi.hoisted(() => ({ execFile: vi.fn() }));
|
||||
vi.mock('node:child_process', async (orig) => {
|
||||
const actual = await orig<typeof import('node:child_process')>();
|
||||
return { ...actual, execFile };
|
||||
});
|
||||
|
||||
import { registerSessionRoutes } from '../../src/web/routes/session-routes.js';
|
||||
|
||||
describe('session-routes', () => {
|
||||
@@ -20,6 +28,44 @@ describe('session-routes', () => {
|
||||
await harness.app.close();
|
||||
});
|
||||
|
||||
// ========== POST /api/sessions/:id/send-key ==========
|
||||
|
||||
describe('POST /api/sessions/:id/send-key', () => {
|
||||
it('routes tmux send-keys through the dedicated Codeman socket (-L)', async () => {
|
||||
// Regression guard: bare `tmux` would hit the user's default server and never
|
||||
// find a session that lives only on the Codeman socket (#80 regression class).
|
||||
execFile.mockReset();
|
||||
execFile.mockImplementation((_bin: string, _argv: string[], _opts: unknown, cb: (e: Error | null) => void) =>
|
||||
cb(null)
|
||||
);
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/sessions/test-session-1/send-key',
|
||||
payload: { key: 'S-Enter' },
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(execFile).toHaveBeenCalledTimes(1);
|
||||
const [bin, argv] = execFile.mock.calls[0];
|
||||
expect(bin).toBe('tmux');
|
||||
expect((argv as string[]).slice(0, 2)).toEqual(['-L', 'codeman']);
|
||||
expect(argv).toContain('send-keys');
|
||||
expect(argv).toContain('-H');
|
||||
});
|
||||
|
||||
it('rejects keys outside the hex allowlist without invoking tmux', async () => {
|
||||
execFile.mockReset();
|
||||
const res = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/sessions/test-session-1/send-key',
|
||||
payload: { key: 'rm -rf' },
|
||||
});
|
||||
expect(JSON.parse(res.body).success).toBe(false);
|
||||
expect(execFile).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
// ========== GET /api/sessions ==========
|
||||
|
||||
describe('GET /api/sessions', () => {
|
||||
|
||||
@@ -77,9 +77,24 @@ describe('TmuxManager (unit)', () => {
|
||||
});
|
||||
|
||||
describe('getAttachArgs', () => {
|
||||
it('should return attach-session args', () => {
|
||||
it('should attach every session through the dedicated Codeman socket', () => {
|
||||
const args = manager.getAttachArgs('codeman-abc12345');
|
||||
expect(args).toEqual(['attach-session', '-t', 'codeman-abc12345']);
|
||||
expect(args).toEqual(['-L', 'codeman', 'attach-session', '-t', 'codeman-abc12345']);
|
||||
});
|
||||
|
||||
it('should attach registered sessions on the same dedicated socket (no per-session socket)', () => {
|
||||
manager.registerSession({
|
||||
sessionId: 'some-session',
|
||||
muxName: 'codeman-abc12345',
|
||||
pid: 12345,
|
||||
createdAt: Date.now(),
|
||||
workingDir: '/tmp',
|
||||
mode: 'claude',
|
||||
attached: false,
|
||||
});
|
||||
|
||||
const args = manager.getAttachArgs('codeman-abc12345');
|
||||
expect(args).toEqual(['-L', 'codeman', 'attach-session', '-t', 'codeman-abc12345']);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -8,7 +8,8 @@
|
||||
* the next frame — visible flicker and one lost repaint of scrollback.
|
||||
*
|
||||
* The fix queries tmux for the actual window geometry first via
|
||||
* `tmux display -t <name> -p '#{window_width} #{window_height}'`. We cover:
|
||||
* `tmux -L <socket> display -t <name> -p '#{window_width} #{window_height}'`
|
||||
* (the `-L <socket>` targets the isolated Codeman socket). We cover:
|
||||
* - Happy path: tmux reports valid geometry → those numbers are used.
|
||||
* - Browser-resize-between-attaches: tmux reports a non-default size
|
||||
* (because a prior client resized it) → the helper picks that up.
|
||||
@@ -49,7 +50,7 @@ beforeEach(() => {
|
||||
describe('queryTmuxWindowSize — happy path', () => {
|
||||
it('returns the geometry tmux reports', () => {
|
||||
execFileSync.mockReturnValue('200 50\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual({ cols: 200, rows: 50 });
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual({ cols: 200, rows: 50 });
|
||||
});
|
||||
|
||||
it('picks up a non-default size left behind by a prior client (browser-resize-between-attaches)', () => {
|
||||
@@ -57,12 +58,12 @@ describe('queryTmuxWindowSize — happy path', () => {
|
||||
// tmux keeps the last-attached geometry. Client B re-attaches and should spawn
|
||||
// its PTY at 220x60, not 120x40 — that's the whole point of #80.
|
||||
execFileSync.mockReturnValue('220 60');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual({ cols: 220, rows: 60 });
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual({ cols: 220, rows: 60 });
|
||||
});
|
||||
|
||||
it('tolerates trailing whitespace and newlines in tmux output', () => {
|
||||
execFileSync.mockReturnValue(' 180 45 \n\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual({ cols: 180, rows: 45 });
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual({ cols: 180, rows: 45 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -75,7 +76,7 @@ describe('queryTmuxWindowSize — fallback paths', () => {
|
||||
err.status = 1;
|
||||
throw err;
|
||||
});
|
||||
expect(queryTmuxWindowSize('bogus')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('bogus', 'codeman')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux dies between query and parse (ETIMEDOUT / ENOENT)', () => {
|
||||
@@ -85,63 +86,65 @@ describe('queryTmuxWindowSize — fallback paths', () => {
|
||||
err.code = 'ETIMEDOUT';
|
||||
throw err;
|
||||
});
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux returns empty output', () => {
|
||||
execFileSync.mockReturnValue('');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux returns whitespace-only output', () => {
|
||||
execFileSync.mockReturnValue(' \n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux returns non-numeric output', () => {
|
||||
execFileSync.mockReturnValue('not a size\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when only one dimension is present', () => {
|
||||
execFileSync.mockReturnValue('200\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when a dimension is zero (degenerate geometry)', () => {
|
||||
// tmux reporting `0` would crash node-pty downstream — must not propagate.
|
||||
execFileSync.mockReturnValue('0 40\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual(DEFAULT);
|
||||
execFileSync.mockReturnValue('120 0\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when a dimension is negative', () => {
|
||||
execFileSync.mockReturnValue('-200 -50\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual(DEFAULT);
|
||||
});
|
||||
|
||||
it('falls back when tmux returns NaN-producing tokens', () => {
|
||||
execFileSync.mockReturnValue('abc def\n');
|
||||
expect(queryTmuxWindowSize('codeman-abc')).toEqual(DEFAULT);
|
||||
expect(queryTmuxWindowSize('codeman-abc', 'codeman')).toEqual(DEFAULT);
|
||||
});
|
||||
});
|
||||
|
||||
describe('queryTmuxWindowSize — call shape', () => {
|
||||
it('invokes tmux with display -t <name> -p ... via argv (not a shell)', () => {
|
||||
it('invokes tmux on the dedicated socket via argv (not a shell)', () => {
|
||||
execFileSync.mockReturnValue('120 40\n');
|
||||
queryTmuxWindowSize('codeman-abc');
|
||||
queryTmuxWindowSize('codeman-abc', 'codeman');
|
||||
expect(execFileSync).toHaveBeenCalledTimes(1);
|
||||
const [bin, argv, opts] = execFileSync.mock.calls[0];
|
||||
expect(bin).toBe('tmux');
|
||||
expect(argv).toEqual(['display', '-t', 'codeman-abc', '-p', '#{window_width} #{window_height}']);
|
||||
// `-L <socket>` MUST lead: querying the default server would never find a
|
||||
// session that lives on the isolated Codeman socket (the #80 regression).
|
||||
expect(argv).toEqual(['-L', 'codeman', 'display', '-t', 'codeman-abc', '-p', '#{window_width} #{window_height}']);
|
||||
// execFileSync — not execSync — so muxName is never substituted into a shell string.
|
||||
expect(opts).toMatchObject({ encoding: 'utf8' });
|
||||
});
|
||||
|
||||
it('uses a bounded timeout so a hung tmux server cannot block startup forever', () => {
|
||||
execFileSync.mockReturnValue('120 40\n');
|
||||
queryTmuxWindowSize('codeman-abc');
|
||||
queryTmuxWindowSize('codeman-abc', 'codeman');
|
||||
const [, , opts] = execFileSync.mock.calls[0];
|
||||
// Whatever the exact constant, the contract is: ≤5s so the user-visible
|
||||
// attach path can't hang on a stuck tmux server.
|
||||
@@ -152,12 +155,12 @@ describe('queryTmuxWindowSize — call shape', () => {
|
||||
|
||||
it('passes a muxName that looks like a tmux flag as an argv element (no option injection)', () => {
|
||||
execFileSync.mockReturnValue('120 40\n');
|
||||
queryTmuxWindowSize('-x 1 -y 1; rm -rf');
|
||||
queryTmuxWindowSize('-x 1 -y 1; rm -rf', 'codeman');
|
||||
const [, argv] = execFileSync.mock.calls[0];
|
||||
// The whole "name" lives in a single argv slot, so tmux interprets it as a
|
||||
// target session name, not as additional flags. The `-t` flag preceding it
|
||||
// pins it as the target argument.
|
||||
expect(argv?.[2]).toBe('-x 1 -y 1; rm -rf');
|
||||
// The whole "name" lives in a single argv slot (index 4, after `-L codeman
|
||||
// display -t`), so tmux interprets it as a target session name, not as
|
||||
// additional flags. The `-t` flag preceding it pins it as the target.
|
||||
expect(argv?.[4]).toBe('-x 1 -y 1; rm -rf');
|
||||
expect((argv as string[]).indexOf('-x')).toBe(-1);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user