Merge pull request #4206 from jeongjin0/fix/session-ready-background-tasks
fix(notification): suppress ready alerts during background tasks
This commit is contained in:
@@ -1,6 +1,11 @@
|
|||||||
|
/// <reference types="bun-types" />
|
||||||
import { afterEach, beforeEach, describe, expect, jest, spyOn, test } from "bun:test"
|
import { afterEach, beforeEach, describe, expect, jest, spyOn, test } from "bun:test"
|
||||||
|
import { mkdtempSync, rmSync } from "node:fs"
|
||||||
|
import { tmpdir } from "node:os"
|
||||||
|
import { join } from "node:path"
|
||||||
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 { setContinuationMarkerSource } from "../features/run-continuation-state"
|
||||||
import * as utils from "./session-notification-utils"
|
import * as utils from "./session-notification-utils"
|
||||||
import * as sender from "./session-notification-sender"
|
import * as sender from "./session-notification-sender"
|
||||||
|
|
||||||
@@ -57,7 +62,7 @@ function createShellMock(options: {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function createMockInput(shell: ReturnType<typeof createShellMock>): MockPluginInput {
|
function createMockInput(shell: ReturnType<typeof createShellMock>, directory = "/tmp/test"): MockPluginInput {
|
||||||
const input = {} as MockPluginInput
|
const input = {} as MockPluginInput
|
||||||
return Object.assign(input, {
|
return Object.assign(input, {
|
||||||
$: shell,
|
$: shell,
|
||||||
@@ -66,17 +71,24 @@ function createMockInput(shell: ReturnType<typeof createShellMock>): MockPluginI
|
|||||||
todo: async () => ({ data: [] }),
|
todo: async () => ({ data: [] }),
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
directory: "/tmp/test",
|
directory,
|
||||||
project: "/tmp/test",
|
project: directory,
|
||||||
worktree: "/tmp/test",
|
worktree: directory,
|
||||||
serverUrl: "http://localhost",
|
serverUrl: "http://localhost",
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
describe("session-notification", () => {
|
describe("session-notification", () => {
|
||||||
let notificationCalls: string[]
|
let notificationCalls: string[]
|
||||||
|
const tempDirs: string[] = []
|
||||||
|
|
||||||
function createMockPluginInput(): MockPluginInput {
|
function createTempDir(): string {
|
||||||
|
const directory = mkdtempSync(join(tmpdir(), "omo-session-notification-"))
|
||||||
|
tempDirs.push(directory)
|
||||||
|
return directory
|
||||||
|
}
|
||||||
|
|
||||||
|
function createMockPluginInput(directory = "/tmp/test"): MockPluginInput {
|
||||||
return createMockInput(
|
return createMockInput(
|
||||||
createShellMock({
|
createShellMock({
|
||||||
capture: (cmdStr) => {
|
capture: (cmdStr) => {
|
||||||
@@ -85,7 +97,8 @@ describe("session-notification", () => {
|
|||||||
notificationCalls.push(cmdStr)
|
notificationCalls.push(cmdStr)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
})
|
}),
|
||||||
|
directory,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -126,6 +139,12 @@ describe("session-notification", () => {
|
|||||||
Date.now = originalDateNow
|
Date.now = originalDateNow
|
||||||
subagentSessions.clear()
|
subagentSessions.clear()
|
||||||
_resetForTesting()
|
_resetForTesting()
|
||||||
|
while (tempDirs.length > 0) {
|
||||||
|
const directory = tempDirs.pop()
|
||||||
|
if (directory) {
|
||||||
|
rmSync(directory, { recursive: true, force: true })
|
||||||
|
}
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
test("should not trigger notification for subagent session", async () => {
|
test("should not trigger notification for subagent session", async () => {
|
||||||
@@ -203,6 +222,58 @@ describe("session-notification", () => {
|
|||||||
expect(notificationCalls.length).toBeGreaterThanOrEqual(1)
|
expect(notificationCalls.length).toBeGreaterThanOrEqual(1)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("should not trigger ready notification while background tasks are active", async () => {
|
||||||
|
// given - a main session has active background work marker
|
||||||
|
const mainSessionID = "main-bg-active"
|
||||||
|
const directory = createTempDir()
|
||||||
|
setMainSession(mainSessionID)
|
||||||
|
setContinuationMarkerSource(directory, mainSessionID, "background-task", "active", "1 background task active")
|
||||||
|
|
||||||
|
const hook = createSessionNotification(createMockPluginInput(directory), {
|
||||||
|
idleConfirmationDelay: 10,
|
||||||
|
enforceMainSessionFilter: false,
|
||||||
|
})
|
||||||
|
|
||||||
|
// when - main session goes idle before background work completes
|
||||||
|
await hook({
|
||||||
|
event: {
|
||||||
|
type: "session.idle",
|
||||||
|
properties: { sessionID: mainSessionID },
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 100))
|
||||||
|
|
||||||
|
// then - ready notification should not be sent
|
||||||
|
expect(notificationCalls).toHaveLength(0)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("should trigger ready notification when background task marker is idle", async () => {
|
||||||
|
// given - a main session has no active background work marker
|
||||||
|
const mainSessionID = "main-bg-idle"
|
||||||
|
const directory = createTempDir()
|
||||||
|
setMainSession(mainSessionID)
|
||||||
|
setContinuationMarkerSource(directory, mainSessionID, "background-task", "idle")
|
||||||
|
|
||||||
|
const hook = createSessionNotification(createMockPluginInput(directory), {
|
||||||
|
idleConfirmationDelay: 10,
|
||||||
|
enforceMainSessionFilter: false,
|
||||||
|
})
|
||||||
|
|
||||||
|
// when - main session goes idle after background work completes
|
||||||
|
await hook({
|
||||||
|
event: {
|
||||||
|
type: "session.idle",
|
||||||
|
properties: { sessionID: mainSessionID },
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 100))
|
||||||
|
|
||||||
|
// then - ready notification should be sent
|
||||||
|
expect(notificationCalls.length).toBeGreaterThanOrEqual(1)
|
||||||
|
})
|
||||||
|
|
||||||
test("should skip notification for subagent even when mainSessionID is set", async () => {
|
test("should skip notification for subagent even when mainSessionID is set", async () => {
|
||||||
// given - both mainSessionID and subagent session exist
|
// given - both mainSessionID and subagent session exist
|
||||||
const mainSessionID = "main-999"
|
const mainSessionID = "main-999"
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ import { buildReadyNotificationContent } from "./session-notification-content"
|
|||||||
import { type Platform } from "./session-notification-sender"
|
import { type Platform } from "./session-notification-sender"
|
||||||
import * as sessionNotificationSender from "./session-notification-sender"
|
import * as sessionNotificationSender from "./session-notification-sender"
|
||||||
import { getEventToolName, getQuestionText, getSessionID } from "./session-notification-event-properties"
|
import { getEventToolName, getQuestionText, getSessionID } from "./session-notification-event-properties"
|
||||||
import { hasIncompleteTodos } from "./session-todo-status"
|
import { hasPendingSessionWork } from "./session-todo-status"
|
||||||
import { createIdleNotificationScheduler } from "./session-notification-scheduler"
|
import { createIdleNotificationScheduler } from "./session-notification-scheduler"
|
||||||
import { createSessionNotificationInit } from "./session-notification-init"
|
import { createSessionNotificationInit } from "./session-notification-init"
|
||||||
import { resolveSessionEventID } from "../shared/event-session-id"
|
import { resolveSessionEventID } from "../shared/event-session-id"
|
||||||
@@ -49,7 +49,7 @@ export function createSessionNotification(ctx: PluginInput, config: SessionNotif
|
|||||||
const scheduler = createIdleNotificationScheduler({
|
const scheduler = createIdleNotificationScheduler({
|
||||||
ctx,
|
ctx,
|
||||||
config: mergedConfig,
|
config: mergedConfig,
|
||||||
hasIncompleteTodos,
|
hasIncompleteTodos: hasPendingSessionWork,
|
||||||
send: async (hookCtx, sessionID) => {
|
send: async (hookCtx, sessionID) => {
|
||||||
const platform = ensureNotificationPlatform()
|
const platform = ensureNotificationPlatform()
|
||||||
if (typeof hookCtx.client.session.get !== "function" && typeof hookCtx.client.session.messages !== "function") {
|
if (typeof hookCtx.client.session.get !== "function" && typeof hookCtx.client.session.messages !== "function") {
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import type { PluginInput } from "@opencode-ai/plugin"
|
import type { PluginInput } from "@opencode-ai/plugin"
|
||||||
|
import { readContinuationMarker } from "../features/run-continuation-state"
|
||||||
import { normalizeSDKResponse } from "../shared"
|
import { normalizeSDKResponse } from "../shared"
|
||||||
|
|
||||||
interface Todo {
|
interface Todo {
|
||||||
@@ -18,3 +19,12 @@ export async function hasIncompleteTodos(ctx: PluginInput, sessionID: string): P
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export async function hasPendingSessionWork(ctx: PluginInput, sessionID: string): Promise<boolean> {
|
||||||
|
const marker = readContinuationMarker(ctx.directory, sessionID)
|
||||||
|
if (marker?.sources["background-task"]?.state === "active") {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
return hasIncompleteTodos(ctx, sessionID)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user