mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 08:03:13 +00:00
fix(notifications): gate the Codex lead-Stop banner without downgrading its state
The first cut of this gate rewrote a lead Stop's `state` from 'done' to 'working' when the roster still held live children. That silently broke the opposite contract in src/main/agent-hooks/server.ts: a root Stop retires child rows and reports 'done'. Two pre-existing tests in server.test.ts went red. Both contracts are real and can't both live in `state`. The reap is also what destroys the signal: after it, a lead Stop and the final SubagentStop are byte-identical (state=done, subagents=undefined), so no downstream heuristic can tell a mid-turn stop from a turn end. So capture it before the reap and carry it as a live-only field. Normalization keeps reporting 'done'; the completion coordinator reads the flag and suppresses only the notification, which is all #4375 ever asked for. Stripped on persist so a rehydrated row can't re-suppress the first real completion after restart.
This commit is contained in:
@@ -1927,6 +1927,12 @@ export class AgentHookServer {
|
||||
continue
|
||||
}
|
||||
const { promptInteractionKey: _promptInteractionKey, ...persistedPayload } = payload
|
||||
// Why: leadStopWithLiveSubagents describes one live hook delivery, not restorable pane
|
||||
// state — a rehydrated one would re-suppress the first completion after restart (#4375).
|
||||
if (persistedPayload.payload?.leadStopWithLiveSubagents) {
|
||||
const { leadStopWithLiveSubagents: _live, ...inner } = persistedPayload.payload
|
||||
persistedPayload.payload = inner
|
||||
}
|
||||
entries[paneKey] = persistedPayload as EnrichedAgentHookEventPayload
|
||||
}
|
||||
const file: LastStatusFile = { version: LAST_STATUS_FILE_VERSION, entries }
|
||||
|
||||
@@ -811,6 +811,13 @@ export function createAgentCompletionCoordinator(
|
||||
return
|
||||
}
|
||||
if (isCompletionHookState(payload.state)) {
|
||||
if (payload.leadStopWithLiveSubagents) {
|
||||
// Why: a Codex lead Stop raised while its children still run ends no turn (#4375).
|
||||
// It stays `done` for the sidebar, but notifying here is the reported spam. Cancel any
|
||||
// provisional done too — the lead can Stop inside a quiet window opened by an earlier one.
|
||||
clearPendingHookDone()
|
||||
return
|
||||
}
|
||||
// Why: the turn is ending, so a debounced attention from an earlier pause must not fire after completion.
|
||||
clearPendingCodexAttention()
|
||||
if (isRecognizedAgentType(payload.agentType)) {
|
||||
|
||||
@@ -242,6 +242,43 @@ describe('issue #4375 — non-turn-end notification spam', () => {
|
||||
expect(restarted?.payload.state).toBe('working')
|
||||
})
|
||||
|
||||
it('gates notification without downgrading the reported state of a lead Stop', () => {
|
||||
// Why: an earlier cut of this fix gated by rewriting `state` to 'working', which silently
|
||||
// broke the sidebar/persistence contract that a root Stop retires child rows and reports
|
||||
// 'done' (server.test.ts). The suppression signal must ride alongside `done`, not replace it.
|
||||
const listenerState: HookListenerState = createHookListenerState()
|
||||
const send = (
|
||||
ev: string,
|
||||
p?: Record<string, unknown>
|
||||
): ReturnType<typeof normalizeHookPayload> =>
|
||||
normalizeHookPayload(
|
||||
listenerState,
|
||||
'codex',
|
||||
{
|
||||
paneKey: PANE_KEY,
|
||||
tabId: 'tab-1',
|
||||
worktreeId: WORKTREE_ID,
|
||||
hook_event_name: ev,
|
||||
payload: { hook_event_name: ev, ...p }
|
||||
},
|
||||
'test-env'
|
||||
)
|
||||
|
||||
send('UserPromptSubmit', { prompt: 'go' })
|
||||
send('SubagentStart', { agent_id: 'child-1', agent_type: 'reviewer' })
|
||||
|
||||
const leadStop = send('Stop')
|
||||
expect(leadStop?.payload.state).toBe('done')
|
||||
expect(leadStop?.payload.subagents).toBeUndefined()
|
||||
expect(leadStop?.payload.leadStopWithLiveSubagents).toBe(true)
|
||||
|
||||
// The child's own Stop is a real turn end: same shape, but no suppression flag.
|
||||
send('PreToolUse', { agent_id: 'child-1', tool_name: 'Bash', tool_input: { command: 'y' } })
|
||||
const childStop = send('SubagentStop', { agent_id: 'child-1' })
|
||||
expect(childStop?.payload.state).toBe('done')
|
||||
expect(childStop?.payload.leadStopWithLiveSubagents).toBeUndefined()
|
||||
})
|
||||
|
||||
// KNOWN-OPEN residual, deliberately out of scope for the roster fix. A bare mid-turn
|
||||
// Stop (no subagent) that resumes AFTER the 1.5s quiet window still notifies. There is
|
||||
// no principled bound to widen the window to, and widening it delays every real Codex
|
||||
|
||||
@@ -3169,7 +3169,11 @@ function buildCodexStatusPayload(
|
||||
promptText: string,
|
||||
paneKey: string,
|
||||
hookPayload: Record<string, unknown>,
|
||||
options: { stateName: 'working' | 'waiting' | 'done'; updateLead: boolean }
|
||||
options: {
|
||||
stateName: 'working' | 'waiting' | 'done'
|
||||
updateLead: boolean
|
||||
leadStopWithLiveSubagents?: boolean
|
||||
}
|
||||
): ParsedAgentStatusPayload | null {
|
||||
const snapshot = options.updateLead
|
||||
? resolveToolState(state, paneKey, extractToolFields('codex', eventName, hookPayload), {
|
||||
@@ -3189,7 +3193,8 @@ function buildCodexStatusPayload(
|
||||
toolInput: snapshot.toolInput,
|
||||
interactivePrompt: snapshot.interactivePrompt,
|
||||
lastAssistantMessage: snapshot.lastAssistantMessage,
|
||||
subagents: codexRosterToSnapshots(state.codexSubagentRosterByPaneKey.get(paneKey))
|
||||
subagents: codexRosterToSnapshots(state.codexSubagentRosterByPaneKey.get(paneKey)),
|
||||
leadStopWithLiveSubagents: options.leadStopWithLiveSubagents
|
||||
})
|
||||
}
|
||||
|
||||
@@ -3283,12 +3288,14 @@ function normalizeCodexEvent(
|
||||
return buildCodexChildDrivenStatusPayload(state, eventName, paneKey, hookPayload)
|
||||
}
|
||||
|
||||
// Why: a lead Stop isn't "done" while children still run, so the roster must gate it before
|
||||
// being retired. SessionStart is exempt — its roster belongs to the process that just exited.
|
||||
const effectiveState =
|
||||
eventName === 'SessionStart'
|
||||
? stateName
|
||||
: codexRosterEffectiveState(state.codexSubagentRosterByPaneKey.get(paneKey), stateName)
|
||||
// Why: the roster is retired below, which makes a lead Stop indistinguishable from the final
|
||||
// SubagentStop (both `done` with no children). Capture whether children were still live BEFORE
|
||||
// the reap so notification can tell a real turn end from a mid-turn one (#4375). SessionStart is
|
||||
// exempt — its roster belongs to the process that just exited.
|
||||
const leadStopWithLiveSubagents =
|
||||
eventName === 'Stop' &&
|
||||
codexRosterEffectiveState(state.codexSubagentRosterByPaneKey.get(paneKey), stateName) !==
|
||||
stateName
|
||||
if (eventName === 'SessionStart') {
|
||||
// Why: a pane can host a new Codex process after the old one exited without child Stop hooks.
|
||||
state.codexSubagentRosterByPaneKey.delete(paneKey)
|
||||
@@ -3304,8 +3311,9 @@ function normalizeCodexEvent(
|
||||
(eventName === 'SessionStart' ? undefined : previousLead?.model)
|
||||
})
|
||||
return buildCodexStatusPayload(state, eventName, promptText, paneKey, hookPayload, {
|
||||
stateName: effectiveState,
|
||||
updateLead: true
|
||||
stateName,
|
||||
updateLead: true,
|
||||
leadStopWithLiveSubagents
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -165,6 +165,11 @@ export type AgentStatusPayload = {
|
||||
interrupted?: boolean
|
||||
/** Live in-process children of the reporting session. See AgentStatusEntry. */
|
||||
subagents?: AgentSubagentSnapshot[]
|
||||
/** Live-only: this `done` is a Codex lead Stop raised while subagents were still
|
||||
* running, so it ends no turn. Set before the roster is retired, which is what
|
||||
* otherwise makes it identical to the final SubagentStop (#4375). Notification-only —
|
||||
* `state` stays `done` so sidebar/persistence behavior is unchanged. */
|
||||
leadStopWithLiveSubagents?: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -361,7 +366,10 @@ function normalizeAgentStatusObject(parsed: unknown): ParsedAgentStatusPayload |
|
||||
),
|
||||
// Why: only meaningful on `done`; coerce to undefined elsewhere so it can't leak stale truth across transitions.
|
||||
interrupted: obj.interrupted === true && state === 'done' ? true : undefined,
|
||||
subagents: normalizeSubagentsField(obj.subagents)
|
||||
subagents: normalizeSubagentsField(obj.subagents),
|
||||
// Why: like `interrupted`, only meaningful on `done`; anywhere else it would be stale truth.
|
||||
leadStopWithLiveSubagents:
|
||||
obj.leadStopWithLiveSubagents === true && state === 'done' ? true : undefined
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user