mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 16:03:17 +00:00
Stop duplicate agent-task-complete notifications from background plugin hooks
Claude-Mem and similar Claude Code plugins run background hooks (PreToolUse/PostToolUse/Stop) on the same pane AFTER the user's turn has already finished. The renderer's agent-completion coordinator treated ANY 'working' frame as the start of a new turn, so each background memory write produced a fresh working->done cycle that re-armed and re-fired a completion notification the user had already seen. Plumb the existing main-process turn-boundary signals (hookEventName and hasExplicitPrompt, already on the AgentHookServer listener payload) through the agentStatus:set IPC to the coordinator. A 'working' frame now only starts a new turn when it carries an explicit user prompt or a non-tool-progress hook event; a tool-progress frame (PreToolUse/PostToolUse) that arrives after we already notified completion for the current turn is treated as background churn and suppressed, along with the matching background 'done' and the title-spinner flicker Claude renders during the memory write. Agents that emit no hook event name (Codex, title-only sources) and genuine new UserPromptSubmit turns keep re-arming exactly as before. Fixes #4630
This commit is contained in:
@@ -816,6 +816,8 @@ function openMainWindow(): BrowserWindow {
|
||||
stateStartedAt,
|
||||
launchToken,
|
||||
providerSession,
|
||||
hookEventName,
|
||||
hasExplicitPrompt,
|
||||
isReplay
|
||||
}) => {
|
||||
if (mainWindow?.isDestroyed()) {
|
||||
@@ -834,6 +836,11 @@ function openMainWindow(): BrowserWindow {
|
||||
connectionId,
|
||||
receivedAt,
|
||||
stateStartedAt,
|
||||
// Why: forward the turn-boundary signals so the renderer's completion
|
||||
// coordinator can distinguish a real user turn from background plugin
|
||||
// hook churn (e.g. Claude-Mem memory writes) and not re-notify.
|
||||
...(hookEventName ? { hookEventName } : {}),
|
||||
...(hasExplicitPrompt === true ? { hasExplicitPrompt: true } : {}),
|
||||
...(providerSession ? { providerSession } : {}),
|
||||
...(orchestration ? { orchestration } : {})
|
||||
})
|
||||
|
||||
@@ -4,6 +4,13 @@ import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-termina
|
||||
|
||||
export type AgentCompletionStatusSnapshot = ParsedAgentStatusPayload & {
|
||||
stateStartedAt?: number
|
||||
/** Raw agent hook event name (e.g. UserPromptSubmit, PreToolUse, Stop), when
|
||||
* the hook IPC path forwards it. Absent on the OSC/title and remote-runtime
|
||||
* paths, which carry no hook event identity. */
|
||||
hookEventName?: string
|
||||
/** True when the originating hook event carried prompt text directly — the
|
||||
* new-turn boundary signal. Absent unless the hook IPC path forwarded it. */
|
||||
hasExplicitPrompt?: boolean
|
||||
}
|
||||
|
||||
export type AgentCompletionDispatchMeta = {
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
resetAgentCompletionCoordinatorIdentitiesForTest
|
||||
} from './agent-completion-coordinator'
|
||||
import { resetAgentProcessInspectionQueueForTests } from './agent-process-inspection-queue'
|
||||
import type { AgentCompletionCoordinatorOptions } from './agent-completion-coordinator-types'
|
||||
import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-terminal-inspection'
|
||||
|
||||
async function flushAsyncTicks(count = 4): Promise<void> {
|
||||
@@ -1113,4 +1114,187 @@ describe('agent completion coordinator', () => {
|
||||
|
||||
expect(dispatchCompletion).toHaveBeenCalledWith('experimental-agent-observability')
|
||||
})
|
||||
|
||||
describe('background plugin hook churn (Claude-Mem)', () => {
|
||||
function createHookCoordinator(
|
||||
dispatchCompletion: AgentCompletionCoordinatorOptions['dispatchCompletion']
|
||||
) {
|
||||
return createAgentCompletionCoordinator({
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
getPtyId: () => 'pty-1',
|
||||
getSettings: () => null,
|
||||
inspectProcess: vi.fn(),
|
||||
dispatchCompletion,
|
||||
isLive: () => true
|
||||
})
|
||||
}
|
||||
|
||||
it('notifies once for a UserPromptSubmit-driven turn', () => {
|
||||
const dispatchCompletion = vi.fn()
|
||||
const coordinator = createHookCoordinator(dispatchCompletion)
|
||||
|
||||
coordinator.observeHookStatus({
|
||||
state: 'working',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'UserPromptSubmit',
|
||||
hasExplicitPrompt: true,
|
||||
stateStartedAt: 1_700_000_000_000
|
||||
})
|
||||
coordinator.observeHookStatus({
|
||||
state: 'done',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'Stop',
|
||||
stateStartedAt: 1_700_000_010_000
|
||||
})
|
||||
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
|
||||
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not re-notify when a background tool-progress turn runs after the user turn', () => {
|
||||
const dispatchCompletion = vi.fn()
|
||||
const coordinator = createHookCoordinator(dispatchCompletion)
|
||||
|
||||
coordinator.observeHookStatus({
|
||||
state: 'working',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'UserPromptSubmit',
|
||||
hasExplicitPrompt: true,
|
||||
stateStartedAt: 1_700_000_000_000
|
||||
})
|
||||
coordinator.observeHookStatus({
|
||||
state: 'done',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'Stop',
|
||||
stateStartedAt: 1_700_000_010_000
|
||||
})
|
||||
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
|
||||
|
||||
// Why: Claude-Mem writes memory on the same pane after the turn — its hooks
|
||||
// arrive as tool-progress 'working' with no explicit user prompt, then Stop.
|
||||
vi.advanceTimersByTime(8_000)
|
||||
coordinator.observeHookStatus({
|
||||
state: 'working',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'PostToolUse',
|
||||
stateStartedAt: 1_700_000_020_000
|
||||
})
|
||||
coordinator.observeHookStatus({
|
||||
state: 'done',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'Stop',
|
||||
stateStartedAt: 1_700_000_030_000
|
||||
})
|
||||
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
|
||||
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('still notifies for a genuinely new UserPromptSubmit turn after background churn', () => {
|
||||
const dispatchCompletion = vi.fn()
|
||||
const coordinator = createHookCoordinator(dispatchCompletion)
|
||||
|
||||
coordinator.observeHookStatus({
|
||||
state: 'working',
|
||||
prompt: 'first',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'UserPromptSubmit',
|
||||
hasExplicitPrompt: true,
|
||||
stateStartedAt: 1_700_000_000_000
|
||||
})
|
||||
coordinator.observeHookStatus({
|
||||
state: 'done',
|
||||
prompt: 'first',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'Stop',
|
||||
stateStartedAt: 1_700_000_010_000
|
||||
})
|
||||
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
|
||||
|
||||
coordinator.observeHookStatus({
|
||||
state: 'working',
|
||||
prompt: 'first',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'PostToolUse',
|
||||
stateStartedAt: 1_700_000_020_000
|
||||
})
|
||||
coordinator.observeHookStatus({
|
||||
state: 'done',
|
||||
prompt: 'first',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'Stop',
|
||||
stateStartedAt: 1_700_000_030_000
|
||||
})
|
||||
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
|
||||
|
||||
coordinator.observeHookStatus({
|
||||
state: 'working',
|
||||
prompt: 'second',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'UserPromptSubmit',
|
||||
hasExplicitPrompt: true,
|
||||
stateStartedAt: 1_700_000_040_000
|
||||
})
|
||||
coordinator.observeHookStatus({
|
||||
state: 'done',
|
||||
prompt: 'second',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'Stop',
|
||||
stateStartedAt: 1_700_000_050_000
|
||||
})
|
||||
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
|
||||
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('ignores title spinner flicker during a background memory write', () => {
|
||||
const dispatchCompletion = vi.fn()
|
||||
const coordinator = createHookCoordinator(dispatchCompletion)
|
||||
|
||||
coordinator.observeHookStatus({
|
||||
state: 'working',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'UserPromptSubmit',
|
||||
hasExplicitPrompt: true,
|
||||
stateStartedAt: 1_700_000_000_000
|
||||
})
|
||||
coordinator.observeHookStatus({
|
||||
state: 'done',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'Stop',
|
||||
stateStartedAt: 1_700_000_010_000
|
||||
})
|
||||
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
|
||||
|
||||
// Background tool-progress hook marks the turn as background churn.
|
||||
vi.advanceTimersByTime(8_000)
|
||||
coordinator.observeHookStatus({
|
||||
state: 'working',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'PostToolUse',
|
||||
stateStartedAt: 1_700_000_020_000
|
||||
})
|
||||
|
||||
// Title flips to a working spinner then back to idle well past the 1s
|
||||
// replay guard — this flicker must not arm a fresh completion.
|
||||
vi.advanceTimersByTime(4_000)
|
||||
coordinator.observeTitle('⠋ Claude')
|
||||
coordinator.observeTitle('✳ Claude')
|
||||
|
||||
expect(dispatchCompletion).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -42,10 +42,29 @@ const PENDING_TITLE_MAX_TTL_MS = Math.max(30_000, PENDING_TITLE_TTL_MS)
|
||||
const COMPLETION_REPLAY_GUARD_MS = 1_000
|
||||
const HOOK_DONE_QUIET_MS = 1_500
|
||||
|
||||
// Why: mirror the main-side TOOL_PROGRESS_HOOK_EVENTS (src/main/agent-hooks/
|
||||
// server.ts) so a tool-lifecycle 'working' frame is not mistaken for a new
|
||||
// user turn. Plugins like Claude-Mem run background tool/Stop hooks on the same
|
||||
// pane after the user's turn already finished; those must not re-arm a second
|
||||
// completion notification.
|
||||
const TOOL_PROGRESS_HOOK_EVENTS = new Set(['PreToolUse', 'PostToolUse', 'PostToolUseFailure'])
|
||||
|
||||
function isCompletionHookState(state: ParsedAgentStatusPayload['state']): boolean {
|
||||
return state === 'done' || state === 'waiting' || state === 'blocked'
|
||||
}
|
||||
|
||||
// Why: a 'working' frame only starts a genuine new turn when it carries an
|
||||
// explicit user prompt or a non-tool-progress hook event (e.g.
|
||||
// UserPromptSubmit / SessionStart). Background plugin churn arrives as
|
||||
// PreToolUse/PostToolUse with no explicit prompt and must not re-arm.
|
||||
function isBackgroundToolProgressWorkingFrame(payload: AgentCompletionStatusSnapshot): boolean {
|
||||
return (
|
||||
payload.hasExplicitPrompt !== true &&
|
||||
payload.hookEventName !== undefined &&
|
||||
TOOL_PROGRESS_HOOK_EVENTS.has(payload.hookEventName)
|
||||
)
|
||||
}
|
||||
|
||||
export function createAgentCompletionCoordinator(
|
||||
options: AgentCompletionCoordinatorOptions
|
||||
): AgentCompletionCoordinator {
|
||||
@@ -63,6 +82,11 @@ export function createAgentCompletionCoordinator(
|
||||
let lastCompletionIdentity: LastCompletionIdentity | null = null
|
||||
let lastForegroundAgent: RecognizedAgentProcess | null = null
|
||||
let requiresFreshWorking = false
|
||||
// Why: the turn number for which we suppressed a background tool-progress
|
||||
// 'working' frame (e.g. Claude-Mem). While set, the matching background
|
||||
// 'done' must not re-arm via the done-only new-turn branch. Cleared by any
|
||||
// genuine new turn. null when no background churn is being suppressed.
|
||||
let suppressedBackgroundTurn: number | null = null
|
||||
let pollTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let pendingTitleTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let pendingHookDoneTimer: ReturnType<typeof setTimeout> | null = null
|
||||
@@ -524,6 +548,16 @@ export function createAgentCompletionCoordinator(
|
||||
) {
|
||||
return false
|
||||
}
|
||||
if (suppressedBackgroundTurn === currentTurn && lastCompletedTurn === currentTurn) {
|
||||
// Why: while a background plugin (e.g. Claude-Mem) writes memory after the
|
||||
// turn finished, Claude re-renders a working spinner then reverts to idle.
|
||||
// That title flicker belongs to the already-notified turn, not a new one —
|
||||
// re-arming would re-fire the completion the user already saw. The 1s
|
||||
// replay guard above is far too short for multi-second memory writes, so
|
||||
// key off turn identity instead. A genuine new turn clears
|
||||
// suppressedBackgroundTurn via the hook working path before this runs.
|
||||
return false
|
||||
}
|
||||
workingStatusObserved = true
|
||||
requiresFreshWorking = false
|
||||
lastCompletionIdentityByPaneKey.delete(options.paneKey)
|
||||
@@ -614,9 +648,27 @@ export function createAgentCompletionCoordinator(
|
||||
establishAgentEvidence()
|
||||
}
|
||||
if (payload.state === 'working') {
|
||||
// Why: after we already notified completion for this turn, a background
|
||||
// plugin (e.g. Claude-Mem) can fire tool-progress hooks on the same pane
|
||||
// with no explicit user prompt. Treating those as a new turn re-arms a
|
||||
// duplicate completion notification on the next 'done'. Suppress the
|
||||
// re-arm for that case only; real new turns (explicit prompt / a non-tool
|
||||
// hook event) and sources that emit no hook event name still re-arm.
|
||||
if (
|
||||
isBackgroundToolProgressWorkingFrame(payload) &&
|
||||
!workingStatusObserved &&
|
||||
lastCompletionSource === 'hook' &&
|
||||
lastCompletedTurn === currentTurn
|
||||
) {
|
||||
suppressedBackgroundTurn = currentTurn
|
||||
clearPendingHookDone()
|
||||
dropPendingTitle()
|
||||
return
|
||||
}
|
||||
clearPendingHookDone()
|
||||
workingStatusObserved = true
|
||||
requiresFreshWorking = false
|
||||
suppressedBackgroundTurn = null
|
||||
lastCompletionIdentity = null
|
||||
currentTurn += 1
|
||||
dropPendingTitle()
|
||||
@@ -642,6 +694,19 @@ export function createAgentCompletionCoordinator(
|
||||
}
|
||||
return
|
||||
}
|
||||
if (
|
||||
suppressedBackgroundTurn === currentTurn &&
|
||||
payload.state === 'done' &&
|
||||
payload.hasExplicitPrompt !== true &&
|
||||
!workingStatusObserved &&
|
||||
lastCompletedTurn === currentTurn
|
||||
) {
|
||||
// Why: this 'done' terminates a background plugin turn (e.g. Claude-Mem's
|
||||
// memory write) whose 'working' frame we suppressed above. Do NOT treat
|
||||
// it as a new turn — that would re-fire the completion notification the
|
||||
// user already saw for this turn.
|
||||
return
|
||||
}
|
||||
if (
|
||||
!workingStatusObserved &&
|
||||
lastCompletionSource === 'hook' &&
|
||||
@@ -708,6 +773,7 @@ export function createAgentCompletionCoordinator(
|
||||
lastCompletionSource = null
|
||||
lastCompletionIdentity = null
|
||||
lastForegroundAgent = null
|
||||
suppressedBackgroundTurn = null
|
||||
requiresFreshWorking = options.requireFreshWorking ?? false
|
||||
inspectionGeneration += 1
|
||||
}
|
||||
|
||||
@@ -539,4 +539,65 @@ describe('agent hook completion notifications', () => {
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('does not re-notify when Claude-Mem runs a background hook turn after the user turn', async () => {
|
||||
const { observeAgentHookCompletionForNotification } =
|
||||
await import('./agent-hook-completion-notifications')
|
||||
|
||||
// Real user turn: UserPromptSubmit carries an explicit prompt, then Stop.
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: {
|
||||
state: 'working',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'UserPromptSubmit',
|
||||
hasExplicitPrompt: true,
|
||||
stateStartedAt: 1_700_000_000_000
|
||||
}
|
||||
})
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: {
|
||||
state: 'done',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'Stop',
|
||||
stateStartedAt: 1_700_000_010_000
|
||||
}
|
||||
})
|
||||
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
|
||||
expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1)
|
||||
|
||||
// Claude-Mem's background memory write: tool-progress working (no explicit
|
||||
// prompt) then Stop, on the same pane. Must not fire a second notification.
|
||||
vi.advanceTimersByTime(8_000)
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: {
|
||||
state: 'working',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'PostToolUse',
|
||||
stateStartedAt: 1_700_000_020_000
|
||||
}
|
||||
})
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey,
|
||||
worktreeId: 'wt-1',
|
||||
payload: {
|
||||
state: 'done',
|
||||
prompt: 'do the thing',
|
||||
agentType: 'claude',
|
||||
hookEventName: 'Stop',
|
||||
stateStartedAt: 1_700_000_030_000
|
||||
}
|
||||
})
|
||||
vi.advanceTimersByTime(HOOK_DONE_QUIET_MS)
|
||||
|
||||
expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -2906,10 +2906,16 @@ export function useIpcEvents(): void {
|
||||
// Why: local Codex/Claude hooks arrive through this main-process IPC
|
||||
// path, not the PTY OSC fallback, so task-complete notifications must
|
||||
// observe accepted hook state here as well.
|
||||
const notificationPayload =
|
||||
typeof data.stateStartedAt === 'number'
|
||||
? { ...resolvedPayload, stateStartedAt: data.stateStartedAt }
|
||||
: resolvedPayload
|
||||
const notificationPayload = {
|
||||
...resolvedPayload,
|
||||
...(typeof data.stateStartedAt === 'number'
|
||||
? { stateStartedAt: data.stateStartedAt }
|
||||
: {}),
|
||||
// Why: carry the turn-boundary signals so the coordinator can ignore
|
||||
// background plugin hook churn (Claude-Mem) after the turn finished.
|
||||
...(data.hookEventName ? { hookEventName: data.hookEventName } : {}),
|
||||
...(data.hasExplicitPrompt === true ? { hasExplicitPrompt: true } : {})
|
||||
}
|
||||
observeAgentHookCompletionForNotification({
|
||||
paneKey: data.paneKey,
|
||||
worktreeId: statusWorktreeId,
|
||||
|
||||
@@ -182,6 +182,14 @@ export type AgentStatusIpcPayload = ParsedAgentStatusPayload & {
|
||||
receivedAt: number
|
||||
/** Timestamp (ms) when the current state first appeared for this pane. */
|
||||
stateStartedAt: number
|
||||
/** Raw agent hook event name (e.g. UserPromptSubmit, PreToolUse, Stop).
|
||||
* Why: lets the renderer's completion coordinator tell a real user-initiated
|
||||
* turn from background plugin churn (e.g. Claude-Mem's post-turn memory
|
||||
* writes) so it doesn't re-arm a duplicate completion notification. */
|
||||
hookEventName?: string
|
||||
/** True when this hook event carried prompt text directly rather than reusing
|
||||
* the listener's cached prompt — the main-process new-turn signal. */
|
||||
hasExplicitPrompt?: boolean
|
||||
orchestration?: AgentStatusOrchestrationContext
|
||||
providerSession?: AgentProviderSessionMetadata
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user