fix: harden cmux fallback retries and tmux runtime assertions
This commit is contained in:
@@ -16,7 +16,7 @@ export function createIdleNotificationScheduler(options: {
|
|||||||
platform: Platform
|
platform: Platform
|
||||||
config: SessionNotificationConfig
|
config: SessionNotificationConfig
|
||||||
hasIncompleteTodos: (ctx: PluginInput, sessionID: string) => Promise<boolean>
|
hasIncompleteTodos: (ctx: PluginInput, sessionID: string) => Promise<boolean>
|
||||||
send: (ctx: PluginInput, platform: Platform, sessionID: string) => Promise<void>
|
send: (ctx: PluginInput, platform: Platform, sessionID: string) => Promise<boolean>
|
||||||
playSound: (ctx: PluginInput, platform: Platform, soundPath: string) => Promise<void>
|
playSound: (ctx: PluginInput, platform: Platform, soundPath: string) => Promise<void>
|
||||||
}) {
|
}) {
|
||||||
const notifiedSessions = new Set<string>()
|
const notifiedSessions = new Set<string>()
|
||||||
@@ -134,9 +134,21 @@ export function createIdleNotificationScheduler(options: {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
notifiedSessions.add(sessionID)
|
const delivered = await options.send(options.ctx, options.platform, sessionID)
|
||||||
|
if (!delivered) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
await options.send(options.ctx, options.platform, sessionID)
|
if (notificationVersions.get(sessionID) !== version) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if (sessionActivitySinceIdle.has(sessionID)) {
|
||||||
|
sessionActivitySinceIdle.delete(sessionID)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
notifiedSessions.add(sessionID)
|
||||||
|
|
||||||
if (options.config.playSound && options.config.soundPath) {
|
if (options.config.playSound && options.config.soundPath) {
|
||||||
await options.playSound(options.ctx, options.platform, options.config.soundPath)
|
await options.playSound(options.ctx, options.platform, options.config.soundPath)
|
||||||
|
|||||||
@@ -499,6 +499,113 @@ describe("session-notification", () => {
|
|||||||
expect(notificationCalls).toHaveLength(1)
|
expect(notificationCalls).toHaveLength(1)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("retries idle notification when cmux fails on unsupported platform", async () => {
|
||||||
|
const sessionID = "cmux-unsupported-retry"
|
||||||
|
let cmuxSendCalls = 0
|
||||||
|
let cmuxNotifyCommandCalls = 0
|
||||||
|
let cmuxAvailable = true
|
||||||
|
const detectPlatformSpy = spyOn(sender, "detectPlatform").mockReturnValue("unsupported")
|
||||||
|
|
||||||
|
try {
|
||||||
|
const cmuxAdapter = createCmuxAdapter({
|
||||||
|
canSendViaCmux: () => cmuxAvailable,
|
||||||
|
hasDowngraded: () => !cmuxAvailable,
|
||||||
|
send: async () => {
|
||||||
|
cmuxSendCalls += 1
|
||||||
|
|
||||||
|
if (!cmuxAvailable) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
cmuxNotifyCommandCalls += 1
|
||||||
|
cmuxAvailable = false
|
||||||
|
return false
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
const hook = createSessionNotification(
|
||||||
|
createMockPluginInput(),
|
||||||
|
{
|
||||||
|
idleConfirmationDelay: 10,
|
||||||
|
skipIfIncompleteTodos: false,
|
||||||
|
enforceMainSessionFilter: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
resolvedMultiplexer: createCmuxRuntime(),
|
||||||
|
cmuxNotificationAdapter: cmuxAdapter,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
await hook({
|
||||||
|
event: {
|
||||||
|
type: "session.idle",
|
||||||
|
properties: { sessionID },
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 50))
|
||||||
|
|
||||||
|
await hook({
|
||||||
|
event: {
|
||||||
|
type: "session.idle",
|
||||||
|
properties: { sessionID },
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 50))
|
||||||
|
|
||||||
|
expect(cmuxSendCalls).toBe(2)
|
||||||
|
expect(cmuxNotifyCommandCalls).toBe(1)
|
||||||
|
expect(notificationCalls).toHaveLength(0)
|
||||||
|
} finally {
|
||||||
|
detectPlatformSpy.mockRestore()
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
test("skips unsupported idle scheduling when cmux was never available", async () => {
|
||||||
|
const sessionID = "cmux-unsupported-unavailable"
|
||||||
|
let cmuxSendCalls = 0
|
||||||
|
const detectPlatformSpy = spyOn(sender, "detectPlatform").mockReturnValue("unsupported")
|
||||||
|
|
||||||
|
try {
|
||||||
|
const cmuxAdapter = createCmuxAdapter({
|
||||||
|
canSendViaCmux: () => false,
|
||||||
|
hasDowngraded: () => false,
|
||||||
|
send: async () => {
|
||||||
|
cmuxSendCalls += 1
|
||||||
|
return false
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
const hook = createSessionNotification(
|
||||||
|
createMockPluginInput(),
|
||||||
|
{
|
||||||
|
idleConfirmationDelay: 10,
|
||||||
|
skipIfIncompleteTodos: false,
|
||||||
|
enforceMainSessionFilter: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
resolvedMultiplexer: createCmuxRuntime(),
|
||||||
|
cmuxNotificationAdapter: cmuxAdapter,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
await hook({
|
||||||
|
event: {
|
||||||
|
type: "session.idle",
|
||||||
|
properties: { sessionID },
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 50))
|
||||||
|
|
||||||
|
expect(cmuxSendCalls).toBe(0)
|
||||||
|
expect(notificationCalls).toHaveLength(0)
|
||||||
|
} finally {
|
||||||
|
detectPlatformSpy.mockRestore()
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
test("suppresses duplicate idle notifications while using cmux backend", async () => {
|
test("suppresses duplicate idle notifications while using cmux backend", async () => {
|
||||||
const sessionID = "cmux-duplicate"
|
const sessionID = "cmux-duplicate"
|
||||||
let cmuxSendCalls = 0
|
let cmuxSendCalls = 0
|
||||||
|
|||||||
@@ -88,15 +88,21 @@ export function createSessionNotification(
|
|||||||
mergedConfig.title,
|
mergedConfig.title,
|
||||||
mergedConfig.message,
|
mergedConfig.message,
|
||||||
)
|
)
|
||||||
if (!deliveredViaCmux && platform !== "unsupported") {
|
if (deliveredViaCmux) {
|
||||||
await sessionNotificationSender.sendSessionNotification(
|
return true
|
||||||
hookCtx,
|
|
||||||
platform,
|
|
||||||
mergedConfig.title,
|
|
||||||
mergedConfig.message,
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
return
|
|
||||||
|
if (platform === "unsupported") {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
await sessionNotificationSender.sendSessionNotification(
|
||||||
|
hookCtx,
|
||||||
|
platform,
|
||||||
|
mergedConfig.title,
|
||||||
|
mergedConfig.message,
|
||||||
|
)
|
||||||
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
const content = await buildReadyNotificationContent(hookCtx, {
|
const content = await buildReadyNotificationContent(hookCtx, {
|
||||||
@@ -107,14 +113,15 @@ export function createSessionNotification(
|
|||||||
|
|
||||||
const deliveredViaCmux = await cmuxNotificationAdapter.send(content.title, content.message)
|
const deliveredViaCmux = await cmuxNotificationAdapter.send(content.title, content.message)
|
||||||
if (deliveredViaCmux) {
|
if (deliveredViaCmux) {
|
||||||
return
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
if (platform === "unsupported") {
|
if (platform === "unsupported") {
|
||||||
return
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
await sessionNotificationSender.sendSessionNotification(hookCtx, platform, content.title, content.message)
|
await sessionNotificationSender.sendSessionNotification(hookCtx, platform, content.title, content.message)
|
||||||
|
return true
|
||||||
},
|
},
|
||||||
playSound: sessionNotificationSender.playSessionNotificationSound,
|
playSound: sessionNotificationSender.playSessionNotificationSound,
|
||||||
})
|
})
|
||||||
@@ -172,7 +179,13 @@ export function createSessionNotification(
|
|||||||
}
|
}
|
||||||
|
|
||||||
return async ({ event }: { event: { type: string; properties?: unknown } }) => {
|
return async ({ event }: { event: { type: string; properties?: unknown } }) => {
|
||||||
if (currentPlatform === "unsupported" && !cmuxNotificationAdapter.canSendViaCmux()) return
|
const cannotDeliverOnUnsupportedPlatform =
|
||||||
|
currentPlatform === "unsupported" && !cmuxNotificationAdapter.canSendViaCmux()
|
||||||
|
const shouldFastExitUnsupportedEvent =
|
||||||
|
cannotDeliverOnUnsupportedPlatform
|
||||||
|
&& (event.type !== "session.idle" || !cmuxNotificationAdapter.hasDowngraded())
|
||||||
|
|
||||||
|
if (shouldFastExitUnsupportedEvent) return
|
||||||
|
|
||||||
const props = event.properties as Record<string, unknown> | undefined
|
const props = event.properties as Record<string, unknown> | undefined
|
||||||
|
|
||||||
|
|||||||
@@ -1,7 +1,12 @@
|
|||||||
import { beforeEach, describe, expect, test } from "bun:test"
|
import { beforeEach, describe, expect, spyOn, test } from "bun:test"
|
||||||
import { resetMultiplexerPathCacheForTesting } from "../../tools/interactive-bash/tmux-path-resolver"
|
import { resetMultiplexerPathCacheForTesting } from "../../tools/interactive-bash/tmux-path-resolver"
|
||||||
import { resetResolvedMultiplexerRuntimeForTesting } from "../../shared/tmux"
|
import {
|
||||||
|
createDisabledMultiplexerRuntime,
|
||||||
|
resetResolvedMultiplexerRuntimeForTesting,
|
||||||
|
setResolvedMultiplexerRuntime,
|
||||||
|
} from "../../shared/tmux"
|
||||||
import { analyzePaneContent, getCurrentTmuxSession } from "../tmux"
|
import { analyzePaneContent, getCurrentTmuxSession } from "../tmux"
|
||||||
|
import * as tmuxPathResolver from "../../tools/interactive-bash/tmux-path-resolver"
|
||||||
|
|
||||||
describe("openclaw tmux helpers", () => {
|
describe("openclaw tmux helpers", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
@@ -21,16 +26,17 @@ describe("openclaw tmux helpers", () => {
|
|||||||
test("getCurrentTmuxSession does not synthesize a session from TMUX_PANE", async () => {
|
test("getCurrentTmuxSession does not synthesize a session from TMUX_PANE", async () => {
|
||||||
const originalTmux = process.env.TMUX
|
const originalTmux = process.env.TMUX
|
||||||
const originalTmuxPane = process.env.TMUX_PANE
|
const originalTmuxPane = process.env.TMUX_PANE
|
||||||
const originalDisableTmuxFlag = process.env.OH_MY_OPENCODE_DISABLE_TMUX
|
const getTmuxPathSpy = spyOn(tmuxPathResolver, "getTmuxPath").mockResolvedValue("/usr/bin/tmux")
|
||||||
|
|
||||||
try {
|
try {
|
||||||
process.env.TMUX = "/tmp/tmux-501/default,1,0"
|
process.env.TMUX = "/tmp/tmux-501/default,1,0"
|
||||||
process.env.TMUX_PANE = "%42"
|
process.env.TMUX_PANE = "%42"
|
||||||
process.env.OH_MY_OPENCODE_DISABLE_TMUX = "1"
|
setResolvedMultiplexerRuntime(createDisabledMultiplexerRuntime())
|
||||||
|
|
||||||
const sessionName = await getCurrentTmuxSession()
|
const sessionName = await getCurrentTmuxSession()
|
||||||
|
|
||||||
expect(sessionName).toBeNull()
|
expect(sessionName).toBeNull()
|
||||||
|
expect(getTmuxPathSpy).not.toHaveBeenCalled()
|
||||||
} finally {
|
} finally {
|
||||||
if (originalTmux === undefined) {
|
if (originalTmux === undefined) {
|
||||||
delete process.env.TMUX
|
delete process.env.TMUX
|
||||||
@@ -44,11 +50,7 @@ describe("openclaw tmux helpers", () => {
|
|||||||
process.env.TMUX_PANE = originalTmuxPane
|
process.env.TMUX_PANE = originalTmuxPane
|
||||||
}
|
}
|
||||||
|
|
||||||
if (originalDisableTmuxFlag === undefined) {
|
getTmuxPathSpy.mockRestore()
|
||||||
delete process.env.OH_MY_OPENCODE_DISABLE_TMUX
|
|
||||||
} else {
|
|
||||||
process.env.OH_MY_OPENCODE_DISABLE_TMUX = originalDisableTmuxFlag
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -1,10 +1,11 @@
|
|||||||
import { afterEach, describe, expect, test } from "bun:test"
|
import { afterEach, describe, expect, spyOn, test } from "bun:test"
|
||||||
import {
|
import {
|
||||||
resetResolvedMultiplexerRuntimeForTesting,
|
resetResolvedMultiplexerRuntimeForTesting,
|
||||||
setResolvedMultiplexerRuntime,
|
setResolvedMultiplexerRuntime,
|
||||||
type ResolvedMultiplexer,
|
type ResolvedMultiplexer,
|
||||||
} from "../../shared/tmux"
|
} from "../../shared/tmux"
|
||||||
import { createInteractiveBashTool, interactive_bash } from "./tools"
|
import { createInteractiveBashTool, interactive_bash } from "./tools"
|
||||||
|
import * as tmuxPathResolver from "./tmux-path-resolver"
|
||||||
|
|
||||||
const mockToolContext = {
|
const mockToolContext = {
|
||||||
sessionID: "test-session",
|
sessionID: "test-session",
|
||||||
@@ -47,20 +48,32 @@ describe("interactive_bash runtime resolution", () => {
|
|||||||
|
|
||||||
test("createInteractiveBashTool without runtime resolves current runtime on execute", async () => {
|
test("createInteractiveBashTool without runtime resolves current runtime on execute", async () => {
|
||||||
resetResolvedMultiplexerRuntimeForTesting()
|
resetResolvedMultiplexerRuntimeForTesting()
|
||||||
const tool = createInteractiveBashTool()
|
const getTmuxPathSpy = spyOn(tmuxPathResolver, "getTmuxPath").mockResolvedValue(null)
|
||||||
setResolvedMultiplexerRuntime(createTmuxEnabledRuntime())
|
|
||||||
|
|
||||||
const result = await tool.execute({ tmux_command: "capture-pane -p" }, mockToolContext)
|
try {
|
||||||
|
const tool = createInteractiveBashTool()
|
||||||
|
setResolvedMultiplexerRuntime(createTmuxEnabledRuntime())
|
||||||
|
|
||||||
expect(result).not.toContain("pane control is unavailable")
|
const result = await tool.execute({ tmux_command: "capture-pane -p" }, mockToolContext)
|
||||||
|
|
||||||
|
expect(result).toBe("Error: tmux executable is not reachable")
|
||||||
|
} finally {
|
||||||
|
getTmuxPathSpy.mockRestore()
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
test("interactive_bash singleton resolves current runtime on execute", async () => {
|
test("interactive_bash singleton resolves current runtime on execute", async () => {
|
||||||
resetResolvedMultiplexerRuntimeForTesting()
|
resetResolvedMultiplexerRuntimeForTesting()
|
||||||
setResolvedMultiplexerRuntime(createTmuxEnabledRuntime())
|
const getTmuxPathSpy = spyOn(tmuxPathResolver, "getTmuxPath").mockResolvedValue(null)
|
||||||
|
|
||||||
const result = await interactive_bash.execute({ tmux_command: "capture-pane -p" }, mockToolContext)
|
try {
|
||||||
|
setResolvedMultiplexerRuntime(createTmuxEnabledRuntime())
|
||||||
|
|
||||||
expect(result).not.toContain("pane control is unavailable")
|
const result = await interactive_bash.execute({ tmux_command: "capture-pane -p" }, mockToolContext)
|
||||||
|
|
||||||
|
expect(result).toBe("Error: tmux executable is not reachable")
|
||||||
|
} finally {
|
||||||
|
getTmuxPathSpy.mockRestore()
|
||||||
|
}
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user