fix(tmux): scope isolated session name per plugin instance (Oracle review)
Oracle flagged the previous commit: "omo-agents" was a shared constant, so when two plugin instances ran in the same tmux server they wrote into the same session. One instance's cleanup would then kill-session on the shared name and tear down the other instance's live attached panes. Replace the const ISOLATED_SESSION_NAME with getIsolatedSessionName(pid) which defaults to process.pid, so every opencode process owns its own "omo-agents-<pid>" session. spawnTmuxSession and cleanup both resolve the name through this helper. Discovery is straightforward from the host tmux via 'tmux list-sessions | grep omo-agents-'. Manager test covers two concurrent managers and asserts each kills a per-pid session name, proving they no longer collide on a global name.
This commit is contained in:
@@ -101,7 +101,7 @@ mock.module('../../shared/tmux', () => {
|
|||||||
spawnTmuxWindow: mockSpawnTmuxWindow,
|
spawnTmuxWindow: mockSpawnTmuxWindow,
|
||||||
spawnTmuxSession: mockSpawnTmuxSession,
|
spawnTmuxSession: mockSpawnTmuxSession,
|
||||||
killTmuxSessionIfExists: mockKillTmuxSessionIfExists,
|
killTmuxSessionIfExists: mockKillTmuxSessionIfExists,
|
||||||
ISOLATED_SESSION_NAME: 'omo-agents',
|
getIsolatedSessionName: (pid: number = 12345) => `omo-agents-${pid}`,
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -1856,7 +1856,7 @@ describe('TmuxSessionManager', () => {
|
|||||||
expect(mockExecuteAction).toHaveBeenCalledTimes(2)
|
expect(mockExecuteAction).toHaveBeenCalledTimes(2)
|
||||||
})
|
})
|
||||||
|
|
||||||
test('#given tmux isolation is "session" #when cleanup runs #then killTmuxSessionIfExists is invoked for the isolated session', async () => {
|
test('#given tmux isolation is "session" #when cleanup runs #then killTmuxSessionIfExists is invoked for the per-pid isolated session', async () => {
|
||||||
// given
|
// given
|
||||||
mockKillTmuxSessionIfExists.mockClear()
|
mockKillTmuxSessionIfExists.mockClear()
|
||||||
const { TmuxSessionManager } = await import('./manager')
|
const { TmuxSessionManager } = await import('./manager')
|
||||||
@@ -1870,7 +1870,32 @@ describe('TmuxSessionManager', () => {
|
|||||||
|
|
||||||
// then
|
// then
|
||||||
expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(1)
|
expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(1)
|
||||||
expect(mockKillTmuxSessionIfExists).toHaveBeenCalledWith('omo-agents')
|
expect(mockKillTmuxSessionIfExists.mock.calls[0]?.[0]).toMatch(/^omo-agents-\d+$/)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('#given two manager instances #when both cleanup #then each kills its own isolated session name, not a shared one', async () => {
|
||||||
|
// given
|
||||||
|
mockKillTmuxSessionIfExists.mockClear()
|
||||||
|
const { TmuxSessionManager } = await import('./manager')
|
||||||
|
const managerA = new TmuxSessionManager(createMockContext(), createTmuxConfig({
|
||||||
|
enabled: true,
|
||||||
|
isolation: 'session',
|
||||||
|
}), mockTmuxDeps)
|
||||||
|
const managerB = new TmuxSessionManager(createMockContext(), createTmuxConfig({
|
||||||
|
enabled: true,
|
||||||
|
isolation: 'session',
|
||||||
|
}), mockTmuxDeps)
|
||||||
|
|
||||||
|
// when
|
||||||
|
await managerA.cleanup()
|
||||||
|
await managerB.cleanup()
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(2)
|
||||||
|
const firstTarget = mockKillTmuxSessionIfExists.mock.calls[0]?.[0]
|
||||||
|
const secondTarget = mockKillTmuxSessionIfExists.mock.calls[1]?.[0]
|
||||||
|
expect(firstTarget).toMatch(/^omo-agents-\d+$/)
|
||||||
|
expect(secondTarget).toMatch(/^omo-agents-\d+$/)
|
||||||
})
|
})
|
||||||
|
|
||||||
test('#given tmux isolation is "inline" #when cleanup runs #then killTmuxSessionIfExists is NOT invoked', async () => {
|
test('#given tmux isolation is "inline" #when cleanup runs #then killTmuxSessionIfExists is NOT invoked', async () => {
|
||||||
|
|||||||
@@ -11,7 +11,7 @@ import {
|
|||||||
spawnTmuxWindow,
|
spawnTmuxWindow,
|
||||||
spawnTmuxSession,
|
spawnTmuxSession,
|
||||||
killTmuxSessionIfExists,
|
killTmuxSessionIfExists,
|
||||||
ISOLATED_SESSION_NAME,
|
getIsolatedSessionName,
|
||||||
} from "../../shared/tmux"
|
} from "../../shared/tmux"
|
||||||
import { queryWindowState } from "./pane-state-querier"
|
import { queryWindowState } from "./pane-state-querier"
|
||||||
import { decideSpawnActions, decideCloseAction, type SessionMapping } from "./decision-engine"
|
import { decideSpawnActions, decideCloseAction, type SessionMapping } from "./decision-engine"
|
||||||
@@ -970,15 +970,16 @@ export class TmuxSessionManager {
|
|||||||
this.isolatedWindowPaneId = undefined
|
this.isolatedWindowPaneId = undefined
|
||||||
|
|
||||||
if (this.tmuxConfig.isolation === "session") {
|
if (this.tmuxConfig.isolation === "session") {
|
||||||
|
const isolatedSessionName = getIsolatedSessionName()
|
||||||
try {
|
try {
|
||||||
const killed = await killTmuxSessionIfExists(ISOLATED_SESSION_NAME)
|
const killed = await killTmuxSessionIfExists(isolatedSessionName)
|
||||||
log("[tmux-session-manager] isolated session teardown", {
|
log("[tmux-session-manager] isolated session teardown", {
|
||||||
session: ISOLATED_SESSION_NAME,
|
session: isolatedSessionName,
|
||||||
killed,
|
killed,
|
||||||
})
|
})
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
log("[tmux-session-manager] isolated session teardown failed", {
|
log("[tmux-session-manager] isolated session teardown failed", {
|
||||||
session: ISOLATED_SESSION_NAME,
|
session: isolatedSessionName,
|
||||||
error: String(error),
|
error: String(error),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ export { spawnTmuxPane } from "./tmux-utils/pane-spawn"
|
|||||||
export { closeTmuxPane } from "./tmux-utils/pane-close"
|
export { closeTmuxPane } from "./tmux-utils/pane-close"
|
||||||
export { replaceTmuxPane } from "./tmux-utils/pane-replace"
|
export { replaceTmuxPane } from "./tmux-utils/pane-replace"
|
||||||
export { spawnTmuxWindow } from "./tmux-utils/window-spawn"
|
export { spawnTmuxWindow } from "./tmux-utils/window-spawn"
|
||||||
export { spawnTmuxSession, ISOLATED_SESSION_NAME } from "./tmux-utils/session-spawn"
|
export { spawnTmuxSession, getIsolatedSessionName } from "./tmux-utils/session-spawn"
|
||||||
export { killTmuxSessionIfExists } from "./tmux-utils/session-kill"
|
export { killTmuxSessionIfExists } from "./tmux-utils/session-kill"
|
||||||
|
|
||||||
export { applyLayout, enforceMainPaneWidth } from "./tmux-utils/layout"
|
export { applyLayout, enforceMainPaneWidth } from "./tmux-utils/layout"
|
||||||
|
|||||||
@@ -6,7 +6,11 @@ import { isInsideTmux } from "./environment"
|
|||||||
import { isServerRunning } from "./server-health"
|
import { isServerRunning } from "./server-health"
|
||||||
import { shellEscapeForDoubleQuotedCommand } from "../../shell-env"
|
import { shellEscapeForDoubleQuotedCommand } from "../../shell-env"
|
||||||
|
|
||||||
export const ISOLATED_SESSION_NAME = "omo-agents"
|
const ISOLATED_SESSION_NAME_PREFIX = "omo-agents"
|
||||||
|
|
||||||
|
export function getIsolatedSessionName(pid: number = process.pid): string {
|
||||||
|
return `${ISOLATED_SESSION_NAME_PREFIX}-${pid}`
|
||||||
|
}
|
||||||
|
|
||||||
async function getWindowDimensions(
|
async function getWindowDimensions(
|
||||||
tmux: string,
|
tmux: string,
|
||||||
@@ -87,12 +91,13 @@ export async function spawnTmuxSession(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const sessionAlreadyExists = await sessionExists(tmux, ISOLATED_SESSION_NAME)
|
const isolatedSessionName = getIsolatedSessionName()
|
||||||
|
const sessionAlreadyExists = await sessionExists(tmux, isolatedSessionName)
|
||||||
|
|
||||||
const args = sessionAlreadyExists
|
const args = sessionAlreadyExists
|
||||||
? [
|
? [
|
||||||
"new-window",
|
"new-window",
|
||||||
"-t", ISOLATED_SESSION_NAME,
|
"-t", isolatedSessionName,
|
||||||
"-P",
|
"-P",
|
||||||
"-F", "#{pane_id}",
|
"-F", "#{pane_id}",
|
||||||
opencodeCmd,
|
opencodeCmd,
|
||||||
@@ -100,7 +105,7 @@ export async function spawnTmuxSession(
|
|||||||
: [
|
: [
|
||||||
"new-session",
|
"new-session",
|
||||||
"-d",
|
"-d",
|
||||||
"-s", ISOLATED_SESSION_NAME,
|
"-s", isolatedSessionName,
|
||||||
...sizeArgs,
|
...sizeArgs,
|
||||||
"-P",
|
"-P",
|
||||||
"-F", "#{pane_id}",
|
"-F", "#{pane_id}",
|
||||||
@@ -109,7 +114,7 @@ export async function spawnTmuxSession(
|
|||||||
|
|
||||||
log("[spawnTmuxSession] spawning", {
|
log("[spawnTmuxSession] spawning", {
|
||||||
mode: sessionAlreadyExists ? "new-window" : "new-session",
|
mode: sessionAlreadyExists ? "new-window" : "new-session",
|
||||||
sessionName: ISOLATED_SESSION_NAME,
|
sessionName: isolatedSessionName,
|
||||||
})
|
})
|
||||||
|
|
||||||
const proc = spawn([tmux, ...args], { stdout: "pipe", stderr: "pipe" })
|
const proc = spawn([tmux, ...args], { stdout: "pipe", stderr: "pipe" })
|
||||||
@@ -140,6 +145,6 @@ export async function spawnTmuxSession(
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
log("[spawnTmuxSession] SUCCESS", { paneId, sessionName: ISOLATED_SESSION_NAME })
|
log("[spawnTmuxSession] SUCCESS", { paneId, sessionName: isolatedSessionName })
|
||||||
return { success: true, paneId }
|
return { success: true, paneId }
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user