fix(hooks): stabilize session notification checks in parallel tests
Use sender-module indirection and an optional main-session filter guard to keep notification assertions deterministic across concurrent test execution. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
parent
54d0dcde48
commit
99f4c7e222
@ -3,6 +3,7 @@ const { describe, expect, test, beforeEach, afterEach, spyOn } = require("bun:te
|
|||||||
const { createSessionNotification } = require("./session-notification")
|
const { createSessionNotification } = require("./session-notification")
|
||||||
const { setMainSession, subagentSessions, _resetForTesting } = require("../features/claude-code-session-state")
|
const { setMainSession, subagentSessions, _resetForTesting } = require("../features/claude-code-session-state")
|
||||||
const utils = require("./session-notification-utils")
|
const utils = require("./session-notification-utils")
|
||||||
|
const sender = require("./session-notification-sender")
|
||||||
|
|
||||||
describe("session-notification input-needed events", () => {
|
describe("session-notification input-needed events", () => {
|
||||||
let notificationCalls: string[]
|
let notificationCalls: string[]
|
||||||
@ -37,6 +38,10 @@ describe("session-notification input-needed events", () => {
|
|||||||
spyOn(utils, "getNotifySendPath").mockResolvedValue("/usr/bin/notify-send")
|
spyOn(utils, "getNotifySendPath").mockResolvedValue("/usr/bin/notify-send")
|
||||||
spyOn(utils, "getPowershellPath").mockResolvedValue("powershell")
|
spyOn(utils, "getPowershellPath").mockResolvedValue("powershell")
|
||||||
spyOn(utils, "startBackgroundCheck").mockImplementation(() => {})
|
spyOn(utils, "startBackgroundCheck").mockImplementation(() => {})
|
||||||
|
spyOn(sender, "detectPlatform").mockReturnValue("darwin")
|
||||||
|
spyOn(sender, "sendSessionNotification").mockImplementation(async (_ctx: unknown, _platform: unknown, _title: unknown, message: string) => {
|
||||||
|
notificationCalls.push(message)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@ -47,7 +52,7 @@ describe("session-notification input-needed events", () => {
|
|||||||
test("sends question notification when question tool asks for input", async () => {
|
test("sends question notification when question tool asks for input", async () => {
|
||||||
const sessionID = "main-question"
|
const sessionID = "main-question"
|
||||||
setMainSession(sessionID)
|
setMainSession(sessionID)
|
||||||
const hook = createSessionNotification(createMockPluginInput())
|
const hook = createSessionNotification(createMockPluginInput(), { enforceMainSessionFilter: false })
|
||||||
|
|
||||||
await hook({
|
await hook({
|
||||||
event: {
|
event: {
|
||||||
@ -74,7 +79,7 @@ describe("session-notification input-needed events", () => {
|
|||||||
test("sends permission notification for permission events", async () => {
|
test("sends permission notification for permission events", async () => {
|
||||||
const sessionID = "main-permission"
|
const sessionID = "main-permission"
|
||||||
setMainSession(sessionID)
|
setMainSession(sessionID)
|
||||||
const hook = createSessionNotification(createMockPluginInput())
|
const hook = createSessionNotification(createMockPluginInput(), { enforceMainSessionFilter: false })
|
||||||
|
|
||||||
await hook({
|
await hook({
|
||||||
event: {
|
event: {
|
||||||
|
|||||||
@ -1,8 +1,9 @@
|
|||||||
import { describe, expect, test, beforeEach, afterEach, spyOn } from "bun:test"
|
const { describe, expect, test, beforeEach, afterEach, spyOn } = require("bun:test")
|
||||||
|
|
||||||
import { createSessionNotification } from "./session-notification"
|
import { createSessionNotification } from "./session-notification"
|
||||||
import { setMainSession, subagentSessions, _resetForTesting } from "../features/claude-code-session-state"
|
import { setMainSession, subagentSessions, _resetForTesting } from "../features/claude-code-session-state"
|
||||||
import * as utils from "./session-notification-utils"
|
import * as utils from "./session-notification-utils"
|
||||||
|
import * as sender from "./session-notification-sender"
|
||||||
|
|
||||||
describe("session-notification", () => {
|
describe("session-notification", () => {
|
||||||
let notificationCalls: string[]
|
let notificationCalls: string[]
|
||||||
@ -40,6 +41,10 @@ describe("session-notification", () => {
|
|||||||
spyOn(utils, "getPaplayPath").mockResolvedValue("/usr/bin/paplay")
|
spyOn(utils, "getPaplayPath").mockResolvedValue("/usr/bin/paplay")
|
||||||
spyOn(utils, "getAplayPath").mockResolvedValue("/usr/bin/aplay")
|
spyOn(utils, "getAplayPath").mockResolvedValue("/usr/bin/aplay")
|
||||||
spyOn(utils, "startBackgroundCheck").mockImplementation(() => {})
|
spyOn(utils, "startBackgroundCheck").mockImplementation(() => {})
|
||||||
|
spyOn(sender, "detectPlatform").mockReturnValue("darwin")
|
||||||
|
spyOn(sender, "sendSessionNotification").mockImplementation(async (_ctx, _platform, _title, message) => {
|
||||||
|
notificationCalls.push(message)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@ -105,6 +110,7 @@ describe("session-notification", () => {
|
|||||||
const hook = createSessionNotification(createMockPluginInput(), {
|
const hook = createSessionNotification(createMockPluginInput(), {
|
||||||
idleConfirmationDelay: 10,
|
idleConfirmationDelay: 10,
|
||||||
skipIfIncompleteTodos: false,
|
skipIfIncompleteTodos: false,
|
||||||
|
enforceMainSessionFilter: false,
|
||||||
})
|
})
|
||||||
|
|
||||||
// when - main session goes idle
|
// when - main session goes idle
|
||||||
@ -332,6 +338,7 @@ describe("session-notification", () => {
|
|||||||
const hook = createSessionNotification(createMockPluginInput(), {
|
const hook = createSessionNotification(createMockPluginInput(), {
|
||||||
idleConfirmationDelay: 10,
|
idleConfirmationDelay: 10,
|
||||||
skipIfIncompleteTodos: false,
|
skipIfIncompleteTodos: false,
|
||||||
|
enforceMainSessionFilter: false,
|
||||||
})
|
})
|
||||||
|
|
||||||
// when - session goes idle twice
|
// when - session goes idle twice
|
||||||
|
|||||||
@ -4,11 +4,9 @@ import {
|
|||||||
startBackgroundCheck,
|
startBackgroundCheck,
|
||||||
} from "./session-notification-utils"
|
} from "./session-notification-utils"
|
||||||
import {
|
import {
|
||||||
detectPlatform,
|
type Platform,
|
||||||
getDefaultSoundPath,
|
|
||||||
playSessionNotificationSound,
|
|
||||||
sendSessionNotification,
|
|
||||||
} from "./session-notification-sender"
|
} from "./session-notification-sender"
|
||||||
|
import * as sessionNotificationSender from "./session-notification-sender"
|
||||||
import { hasIncompleteTodos } from "./session-todo-status"
|
import { hasIncompleteTodos } from "./session-todo-status"
|
||||||
import { createIdleNotificationScheduler } from "./session-notification-scheduler"
|
import { createIdleNotificationScheduler } from "./session-notification-scheduler"
|
||||||
|
|
||||||
@ -25,13 +23,14 @@ interface SessionNotificationConfig {
|
|||||||
skipIfIncompleteTodos?: boolean
|
skipIfIncompleteTodos?: boolean
|
||||||
/** Maximum number of sessions to track before cleanup (default: 100) */
|
/** Maximum number of sessions to track before cleanup (default: 100) */
|
||||||
maxTrackedSessions?: number
|
maxTrackedSessions?: number
|
||||||
|
enforceMainSessionFilter?: boolean
|
||||||
}
|
}
|
||||||
export function createSessionNotification(
|
export function createSessionNotification(
|
||||||
ctx: PluginInput,
|
ctx: PluginInput,
|
||||||
config: SessionNotificationConfig = {}
|
config: SessionNotificationConfig = {}
|
||||||
) {
|
) {
|
||||||
const currentPlatform = detectPlatform()
|
const currentPlatform: Platform = sessionNotificationSender.detectPlatform()
|
||||||
const defaultSoundPath = getDefaultSoundPath(currentPlatform)
|
const defaultSoundPath = sessionNotificationSender.getDefaultSoundPath(currentPlatform)
|
||||||
|
|
||||||
startBackgroundCheck(currentPlatform)
|
startBackgroundCheck(currentPlatform)
|
||||||
|
|
||||||
@ -45,6 +44,7 @@ export function createSessionNotification(
|
|||||||
idleConfirmationDelay: 1500,
|
idleConfirmationDelay: 1500,
|
||||||
skipIfIncompleteTodos: true,
|
skipIfIncompleteTodos: true,
|
||||||
maxTrackedSessions: 100,
|
maxTrackedSessions: 100,
|
||||||
|
enforceMainSessionFilter: true,
|
||||||
...config,
|
...config,
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -53,8 +53,8 @@ export function createSessionNotification(
|
|||||||
platform: currentPlatform,
|
platform: currentPlatform,
|
||||||
config: mergedConfig,
|
config: mergedConfig,
|
||||||
hasIncompleteTodos,
|
hasIncompleteTodos,
|
||||||
send: sendSessionNotification,
|
send: sessionNotificationSender.sendSessionNotification,
|
||||||
playSound: playSessionNotificationSound,
|
playSound: sessionNotificationSender.playSessionNotificationSound,
|
||||||
})
|
})
|
||||||
|
|
||||||
const QUESTION_TOOLS = new Set(["question", "ask_user_question", "askuserquestion"])
|
const QUESTION_TOOLS = new Set(["question", "ask_user_question", "askuserquestion"])
|
||||||
@ -81,8 +81,10 @@ export function createSessionNotification(
|
|||||||
const shouldNotifyForSession = (sessionID: string): boolean => {
|
const shouldNotifyForSession = (sessionID: string): boolean => {
|
||||||
if (subagentSessions.has(sessionID)) return false
|
if (subagentSessions.has(sessionID)) return false
|
||||||
|
|
||||||
const mainSessionID = getMainSessionID()
|
if (mergedConfig.enforceMainSessionFilter) {
|
||||||
if (mainSessionID && sessionID !== mainSessionID) return false
|
const mainSessionID = getMainSessionID()
|
||||||
|
if (mainSessionID && sessionID !== mainSessionID) return false
|
||||||
|
}
|
||||||
|
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
@ -146,9 +148,14 @@ export function createSessionNotification(
|
|||||||
if (!shouldNotifyForSession(sessionID)) return
|
if (!shouldNotifyForSession(sessionID)) return
|
||||||
|
|
||||||
scheduler.markSessionActivity(sessionID)
|
scheduler.markSessionActivity(sessionID)
|
||||||
await sendSessionNotification(ctx, currentPlatform, mergedConfig.title, mergedConfig.permissionMessage)
|
await sessionNotificationSender.sendSessionNotification(
|
||||||
|
ctx,
|
||||||
|
currentPlatform,
|
||||||
|
mergedConfig.title,
|
||||||
|
mergedConfig.permissionMessage,
|
||||||
|
)
|
||||||
if (mergedConfig.playSound && mergedConfig.soundPath) {
|
if (mergedConfig.playSound && mergedConfig.soundPath) {
|
||||||
await playSessionNotificationSound(ctx, currentPlatform, mergedConfig.soundPath)
|
await sessionNotificationSender.playSessionNotificationSound(ctx, currentPlatform, mergedConfig.soundPath)
|
||||||
}
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@ -168,9 +175,9 @@ export function createSessionNotification(
|
|||||||
? mergedConfig.permissionMessage
|
? mergedConfig.permissionMessage
|
||||||
: mergedConfig.questionMessage
|
: mergedConfig.questionMessage
|
||||||
|
|
||||||
await sendSessionNotification(ctx, currentPlatform, mergedConfig.title, message)
|
await sessionNotificationSender.sendSessionNotification(ctx, currentPlatform, mergedConfig.title, message)
|
||||||
if (mergedConfig.playSound && mergedConfig.soundPath) {
|
if (mergedConfig.playSound && mergedConfig.soundPath) {
|
||||||
await playSessionNotificationSound(ctx, currentPlatform, mergedConfig.soundPath)
|
await sessionNotificationSender.playSessionNotificationSound(ctx, currentPlatform, mergedConfig.soundPath)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user