Merge pull request #2095 from code-yeongyu/fix/issue-1934-exit-code-130-timeout
fix(run): add event watchdog and secondary timeout to prevent infinite hang in CI
This commit is contained in:
commit
c505989ad4
@ -1,4 +1,4 @@
|
|||||||
import { describe, it, expect, spyOn } from "bun:test"
|
const { describe, it, expect, spyOn } = require("bun:test")
|
||||||
import type { RunContext } from "./types"
|
import type { RunContext } from "./types"
|
||||||
import { createEventState } from "./events"
|
import { createEventState } from "./events"
|
||||||
import { handleSessionStatus, handleMessagePartUpdated, handleMessageUpdated, handleTuiToast } from "./event-handlers"
|
import { handleSessionStatus, handleMessagePartUpdated, handleMessageUpdated, handleTuiToast } from "./event-handlers"
|
||||||
@ -235,9 +235,7 @@ describe("handleMessagePartUpdated", () => {
|
|||||||
|
|
||||||
it("prints completion metadata once when assistant text part is completed", () => {
|
it("prints completion metadata once when assistant text part is completed", () => {
|
||||||
// given
|
// given
|
||||||
const nowSpy = spyOn(Date, "now")
|
const nowSpy = spyOn(Date, "now").mockReturnValue(3400)
|
||||||
nowSpy.mockReturnValueOnce(1000)
|
|
||||||
nowSpy.mockReturnValueOnce(3400)
|
|
||||||
|
|
||||||
const ctx = createMockContext("ses_main")
|
const ctx = createMockContext("ses_main")
|
||||||
const state = createEventState()
|
const state = createEventState()
|
||||||
@ -259,6 +257,7 @@ describe("handleMessagePartUpdated", () => {
|
|||||||
} as any,
|
} as any,
|
||||||
state,
|
state,
|
||||||
)
|
)
|
||||||
|
state.messageStartedAtById["msg_1"] = 1000
|
||||||
|
|
||||||
// when
|
// when
|
||||||
handleMessagePartUpdated(
|
handleMessagePartUpdated(
|
||||||
|
|||||||
@ -7,6 +7,8 @@ export interface EventState {
|
|||||||
currentTool: string | null
|
currentTool: string | null
|
||||||
/** Set to true when the main session has produced meaningful work (text, tool call, or tool result) */
|
/** Set to true when the main session has produced meaningful work (text, tool call, or tool result) */
|
||||||
hasReceivedMeaningfulWork: boolean
|
hasReceivedMeaningfulWork: boolean
|
||||||
|
/** Timestamp of the last received event (for watchdog detection) */
|
||||||
|
lastEventTimestamp: number
|
||||||
/** Count of assistant messages for the main session */
|
/** Count of assistant messages for the main session */
|
||||||
messageCount: number
|
messageCount: number
|
||||||
/** Current agent name from the latest assistant message */
|
/** Current agent name from the latest assistant message */
|
||||||
@ -54,6 +56,7 @@ export function createEventState(): EventState {
|
|||||||
lastPartText: "",
|
lastPartText: "",
|
||||||
currentTool: null,
|
currentTool: null,
|
||||||
hasReceivedMeaningfulWork: false,
|
hasReceivedMeaningfulWork: false,
|
||||||
|
lastEventTimestamp: Date.now(),
|
||||||
messageCount: 0,
|
messageCount: 0,
|
||||||
currentAgent: null,
|
currentAgent: null,
|
||||||
currentModel: null,
|
currentModel: null,
|
||||||
|
|||||||
@ -35,6 +35,9 @@ export async function processEvents(
|
|||||||
logEventVerbose(ctx, payload)
|
logEventVerbose(ctx, payload)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Update last event timestamp for watchdog detection
|
||||||
|
state.lastEventTimestamp = Date.now()
|
||||||
|
|
||||||
handleSessionError(ctx, payload, state)
|
handleSessionError(ctx, payload, state)
|
||||||
handleSessionIdle(ctx, payload, state)
|
handleSessionIdle(ctx, payload, state)
|
||||||
handleSessionStatus(ctx, payload, state)
|
handleSessionStatus(ctx, payload, state)
|
||||||
|
|||||||
@ -8,11 +8,15 @@ const DEFAULT_POLL_INTERVAL_MS = 500
|
|||||||
const DEFAULT_REQUIRED_CONSECUTIVE = 1
|
const DEFAULT_REQUIRED_CONSECUTIVE = 1
|
||||||
const ERROR_GRACE_CYCLES = 3
|
const ERROR_GRACE_CYCLES = 3
|
||||||
const MIN_STABILIZATION_MS = 1_000
|
const MIN_STABILIZATION_MS = 1_000
|
||||||
|
const DEFAULT_EVENT_WATCHDOG_MS = 30_000 // 30 seconds
|
||||||
|
const DEFAULT_SECONDARY_MEANINGFUL_WORK_TIMEOUT_MS = 60_000 // 60 seconds
|
||||||
|
|
||||||
export interface PollOptions {
|
export interface PollOptions {
|
||||||
pollIntervalMs?: number
|
pollIntervalMs?: number
|
||||||
requiredConsecutive?: number
|
requiredConsecutive?: number
|
||||||
minStabilizationMs?: number
|
minStabilizationMs?: number
|
||||||
|
eventWatchdogMs?: number
|
||||||
|
secondaryMeaningfulWorkTimeoutMs?: number
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function pollForCompletion(
|
export async function pollForCompletion(
|
||||||
@ -28,9 +32,15 @@ export async function pollForCompletion(
|
|||||||
options.minStabilizationMs ?? MIN_STABILIZATION_MS
|
options.minStabilizationMs ?? MIN_STABILIZATION_MS
|
||||||
const minStabilizationMs =
|
const minStabilizationMs =
|
||||||
rawMinStabilizationMs > 0 ? rawMinStabilizationMs : MIN_STABILIZATION_MS
|
rawMinStabilizationMs > 0 ? rawMinStabilizationMs : MIN_STABILIZATION_MS
|
||||||
|
const eventWatchdogMs =
|
||||||
|
options.eventWatchdogMs ?? DEFAULT_EVENT_WATCHDOG_MS
|
||||||
|
const secondaryMeaningfulWorkTimeoutMs =
|
||||||
|
options.secondaryMeaningfulWorkTimeoutMs ??
|
||||||
|
DEFAULT_SECONDARY_MEANINGFUL_WORK_TIMEOUT_MS
|
||||||
let consecutiveCompleteChecks = 0
|
let consecutiveCompleteChecks = 0
|
||||||
let errorCycleCount = 0
|
let errorCycleCount = 0
|
||||||
let firstWorkTimestamp: number | null = null
|
let firstWorkTimestamp: number | null = null
|
||||||
|
let secondaryTimeoutChecked = false
|
||||||
const pollStartTimestamp = Date.now()
|
const pollStartTimestamp = Date.now()
|
||||||
|
|
||||||
while (!abortController.signal.aborted) {
|
while (!abortController.signal.aborted) {
|
||||||
@ -59,7 +69,37 @@ export async function pollForCompletion(
|
|||||||
errorCycleCount = 0
|
errorCycleCount = 0
|
||||||
}
|
}
|
||||||
|
|
||||||
const mainSessionStatus = await getMainSessionStatus(ctx)
|
// Watchdog: if no events received for N seconds, verify session status via API
|
||||||
|
let mainSessionStatus: "idle" | "busy" | "retry" | null = null
|
||||||
|
if (eventState.lastEventTimestamp !== null) {
|
||||||
|
const timeSinceLastEvent = Date.now() - eventState.lastEventTimestamp
|
||||||
|
if (timeSinceLastEvent > eventWatchdogMs) {
|
||||||
|
// Events stopped coming - verify actual session state
|
||||||
|
console.log(
|
||||||
|
pc.yellow(
|
||||||
|
`\n No events for ${Math.round(
|
||||||
|
timeSinceLastEvent / 1000
|
||||||
|
)}s, verifying session status...`
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
// Force check session status directly
|
||||||
|
mainSessionStatus = await getMainSessionStatus(ctx)
|
||||||
|
if (mainSessionStatus === "idle") {
|
||||||
|
eventState.mainSessionIdle = true
|
||||||
|
} else if (mainSessionStatus === "busy" || mainSessionStatus === "retry") {
|
||||||
|
eventState.mainSessionIdle = false
|
||||||
|
}
|
||||||
|
|
||||||
|
// Reset timestamp to avoid repeated checks
|
||||||
|
eventState.lastEventTimestamp = Date.now()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Only call getMainSessionStatus if watchdog didn't already check
|
||||||
|
if (mainSessionStatus === null) {
|
||||||
|
mainSessionStatus = await getMainSessionStatus(ctx)
|
||||||
|
}
|
||||||
if (mainSessionStatus === "busy" || mainSessionStatus === "retry") {
|
if (mainSessionStatus === "busy" || mainSessionStatus === "retry") {
|
||||||
eventState.mainSessionIdle = false
|
eventState.mainSessionIdle = false
|
||||||
} else if (mainSessionStatus === "idle") {
|
} else if (mainSessionStatus === "idle") {
|
||||||
@ -81,6 +121,50 @@ export async function pollForCompletion(
|
|||||||
consecutiveCompleteChecks = 0
|
consecutiveCompleteChecks = 0
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Secondary timeout: if we've been polling for reasonable time but haven't
|
||||||
|
// received meaningful work via events, check if there's active work via API
|
||||||
|
// Only check once to avoid unnecessary API calls every poll cycle
|
||||||
|
if (
|
||||||
|
Date.now() - pollStartTimestamp > secondaryMeaningfulWorkTimeoutMs &&
|
||||||
|
!secondaryTimeoutChecked
|
||||||
|
) {
|
||||||
|
secondaryTimeoutChecked = true
|
||||||
|
// Check if session actually has pending work (children, todos, etc.)
|
||||||
|
const childrenRes = await ctx.client.session.children({
|
||||||
|
path: { id: ctx.sessionID },
|
||||||
|
query: { directory: ctx.directory },
|
||||||
|
})
|
||||||
|
const children = normalizeSDKResponse(childrenRes, [] as unknown[])
|
||||||
|
const todosRes = await ctx.client.session.todo({
|
||||||
|
path: { id: ctx.sessionID },
|
||||||
|
query: { directory: ctx.directory },
|
||||||
|
})
|
||||||
|
const todos = normalizeSDKResponse(todosRes, [] as unknown[])
|
||||||
|
|
||||||
|
const hasActiveChildren =
|
||||||
|
Array.isArray(children) && children.length > 0
|
||||||
|
const hasActiveTodos =
|
||||||
|
Array.isArray(todos) &&
|
||||||
|
todos.some(
|
||||||
|
(t: unknown) =>
|
||||||
|
(t as { status?: string })?.status !== "completed" &&
|
||||||
|
(t as { status?: string })?.status !== "cancelled"
|
||||||
|
)
|
||||||
|
const hasActiveWork = hasActiveChildren || hasActiveTodos
|
||||||
|
|
||||||
|
if (hasActiveWork) {
|
||||||
|
// Assume meaningful work is happening even without events
|
||||||
|
eventState.hasReceivedMeaningfulWork = true
|
||||||
|
console.log(
|
||||||
|
pc.yellow(
|
||||||
|
`\n No meaningful work events for ${Math.round(
|
||||||
|
secondaryMeaningfulWorkTimeoutMs / 1000
|
||||||
|
)}s but session has active work - assuming in progress`
|
||||||
|
)
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
// Track when first meaningful work was received
|
// Track when first meaningful work was received
|
||||||
if (firstWorkTimestamp === null) {
|
if (firstWorkTimestamp === null) {
|
||||||
|
|||||||
@ -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