diff --git a/src/hooks/session-notification-scheduler.ts b/src/hooks/session-notification-scheduler.ts index afea12c7f..4d4c43b66 100644 --- a/src/hooks/session-notification-scheduler.ts +++ b/src/hooks/session-notification-scheduler.ts @@ -16,7 +16,7 @@ export function createIdleNotificationScheduler(options: { platform: Platform config: SessionNotificationConfig hasIncompleteTodos: (ctx: PluginInput, sessionID: string) => Promise - send: (ctx: PluginInput, platform: Platform, sessionID: string) => Promise + send: (ctx: PluginInput, platform: Platform, sessionID: string) => Promise playSound: (ctx: PluginInput, platform: Platform, soundPath: string) => Promise }) { const notifiedSessions = new Set() @@ -134,9 +134,21 @@ export function createIdleNotificationScheduler(options: { 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) { await options.playSound(options.ctx, options.platform, options.config.soundPath) diff --git a/src/hooks/session-notification.test.ts b/src/hooks/session-notification.test.ts index 6554290ac..bc98073f4 100644 --- a/src/hooks/session-notification.test.ts +++ b/src/hooks/session-notification.test.ts @@ -499,6 +499,113 @@ describe("session-notification", () => { 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 () => { const sessionID = "cmux-duplicate" let cmuxSendCalls = 0 diff --git a/src/hooks/session-notification.ts b/src/hooks/session-notification.ts index 5948d5cfe..706ddfb33 100644 --- a/src/hooks/session-notification.ts +++ b/src/hooks/session-notification.ts @@ -88,15 +88,21 @@ export function createSessionNotification( mergedConfig.title, mergedConfig.message, ) - if (!deliveredViaCmux && platform !== "unsupported") { - await sessionNotificationSender.sendSessionNotification( - hookCtx, - platform, - mergedConfig.title, - mergedConfig.message, - ) + if (deliveredViaCmux) { + return true } - return + + if (platform === "unsupported") { + return false + } + + await sessionNotificationSender.sendSessionNotification( + hookCtx, + platform, + mergedConfig.title, + mergedConfig.message, + ) + return true } const content = await buildReadyNotificationContent(hookCtx, { @@ -107,14 +113,15 @@ export function createSessionNotification( const deliveredViaCmux = await cmuxNotificationAdapter.send(content.title, content.message) if (deliveredViaCmux) { - return + return true } if (platform === "unsupported") { - return + return false } await sessionNotificationSender.sendSessionNotification(hookCtx, platform, content.title, content.message) + return true }, playSound: sessionNotificationSender.playSessionNotificationSound, }) @@ -172,7 +179,13 @@ export function createSessionNotification( } 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 | undefined diff --git a/src/openclaw/__tests__/tmux.test.ts b/src/openclaw/__tests__/tmux.test.ts index 6d20fcb12..81762f5f8 100644 --- a/src/openclaw/__tests__/tmux.test.ts +++ b/src/openclaw/__tests__/tmux.test.ts @@ -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 { resetResolvedMultiplexerRuntimeForTesting } from "../../shared/tmux" +import { + createDisabledMultiplexerRuntime, + resetResolvedMultiplexerRuntimeForTesting, + setResolvedMultiplexerRuntime, +} from "../../shared/tmux" import { analyzePaneContent, getCurrentTmuxSession } from "../tmux" +import * as tmuxPathResolver from "../../tools/interactive-bash/tmux-path-resolver" describe("openclaw tmux helpers", () => { beforeEach(() => { @@ -21,16 +26,17 @@ describe("openclaw tmux helpers", () => { test("getCurrentTmuxSession does not synthesize a session from TMUX_PANE", async () => { const originalTmux = process.env.TMUX 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 { process.env.TMUX = "/tmp/tmux-501/default,1,0" process.env.TMUX_PANE = "%42" - process.env.OH_MY_OPENCODE_DISABLE_TMUX = "1" + setResolvedMultiplexerRuntime(createDisabledMultiplexerRuntime()) const sessionName = await getCurrentTmuxSession() expect(sessionName).toBeNull() + expect(getTmuxPathSpy).not.toHaveBeenCalled() } finally { if (originalTmux === undefined) { delete process.env.TMUX @@ -44,11 +50,7 @@ describe("openclaw tmux helpers", () => { process.env.TMUX_PANE = originalTmuxPane } - if (originalDisableTmuxFlag === undefined) { - delete process.env.OH_MY_OPENCODE_DISABLE_TMUX - } else { - process.env.OH_MY_OPENCODE_DISABLE_TMUX = originalDisableTmuxFlag - } + getTmuxPathSpy.mockRestore() } }) }) diff --git a/src/tools/interactive-bash/tools.test.ts b/src/tools/interactive-bash/tools.test.ts index 9bac6a39a..8dfb3934b 100644 --- a/src/tools/interactive-bash/tools.test.ts +++ b/src/tools/interactive-bash/tools.test.ts @@ -1,10 +1,11 @@ -import { afterEach, describe, expect, test } from "bun:test" +import { afterEach, describe, expect, spyOn, test } from "bun:test" import { resetResolvedMultiplexerRuntimeForTesting, setResolvedMultiplexerRuntime, type ResolvedMultiplexer, } from "../../shared/tmux" import { createInteractiveBashTool, interactive_bash } from "./tools" +import * as tmuxPathResolver from "./tmux-path-resolver" const mockToolContext = { sessionID: "test-session", @@ -47,20 +48,32 @@ describe("interactive_bash runtime resolution", () => { test("createInteractiveBashTool without runtime resolves current runtime on execute", async () => { resetResolvedMultiplexerRuntimeForTesting() - const tool = createInteractiveBashTool() - setResolvedMultiplexerRuntime(createTmuxEnabledRuntime()) + const getTmuxPathSpy = spyOn(tmuxPathResolver, "getTmuxPath").mockResolvedValue(null) - 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 () => { 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() + } }) })