From a6a50769bffec0fb3d09978cf44b748ebc86ef08 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 26 Jul 2026 20:43:56 -0700 Subject: [PATCH] fix(notifications): restore the blocked-child waiting state on non-Stop lead events The lead-Stop gate revert (84880f2711) dropped the effective-state read from the local lead path entirely, not just from the Stop case. That regressed a separate contract: a child blocked on a permission prompt no longer held the pane at `waiting` when the lead kept working, so the pane read `working` and the user never saw what it was blocked on. Restore the read, but AFTER the roster reap rather than before it. Post-reap, a lead Stop sees an empty roster and still reports `done`, which is what agent-hooks/server.ts requires; only non-Stop events observe live children. The pre-reap `leadStopWithLiveSubagents` flag is unchanged and still carries the "children were live at Stop" signal for the notification gate. Both orderings are now pinned by tests: reading pre-reap breaks the Stop contract, dropping the read breaks the blocked-child contract. --- src/shared/agent-hook-listener.ts | 9 +++- ...odex-lead-state-vs-subagent-roster.test.ts | 51 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 src/shared/codex-lead-state-vs-subagent-roster.test.ts diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index e4e6c2e67f7..0f2a308fac0 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -3316,8 +3316,15 @@ function normalizeCodexEvent( normalizeOptionalField(hookPayload['model'], AGENT_MODEL_MAX_LENGTH) ?? (eventName === 'SessionStart' ? undefined : previousLead?.model) }) + // Why: read the roster AFTER the reap. A blocked child must still hold the pane at `waiting` on + // non-Stop lead events, but on Stop the roster is already empty so this stays `done` — the root + // Stop contract in agent-hooks/server.ts depends on that. + const effectiveState = codexRosterEffectiveState( + state.codexSubagentRosterByPaneKey.get(paneKey), + stateName + ) return buildCodexStatusPayload(state, eventName, promptText, paneKey, hookPayload, { - stateName, + stateName: effectiveState, updateLead: true, leadStopWithLiveSubagents }) diff --git a/src/shared/codex-lead-state-vs-subagent-roster.test.ts b/src/shared/codex-lead-state-vs-subagent-roster.test.ts new file mode 100644 index 00000000000..8e3ddcca80c --- /dev/null +++ b/src/shared/codex-lead-state-vs-subagent-roster.test.ts @@ -0,0 +1,51 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { createHookListenerState, normalizeHookPayload } from './agent-hook-listener' +import { makePaneKey } from './stable-pane-id' + +const PANE_KEY = makePaneKey('tab-1', '11111111-1111-4111-8111-111111111111') +let state: ReturnType + +function codexEvent(payload: Record) { + return normalizeHookPayload(state, 'codex', { paneKey: PANE_KEY, payload }, 'production') +} + +function withBlockedChild() { + codexEvent({ hook_event_name: 'SessionStart' }) + codexEvent({ hook_event_name: 'SubagentStart', agent_id: 'child-1', agent_type: 'explore' }) + codexEvent({ hook_event_name: 'PermissionRequest', agent_id: 'child-1' }) +} + +// Why: the roster reap and the effective-state read must stay in this order. Swapping them +// breaks one of the two contracts below, and each was a shipped bug at some point (#4375). +describe('Codex lead state vs. a live subagent roster', () => { + beforeEach(() => { + state = createHookListenerState() + }) + + it('holds the pane at waiting on a non-Stop lead event while a child is blocked', () => { + withBlockedChild() + // Read pre-reap: a lead that keeps working must not paper over a child's permission prompt, + // or the pane looks busy and the user never sees what it is blocked on. + expect(codexEvent({ hook_event_name: 'PostToolUse', tool_name: 'read' })?.payload.state).toBe( + 'waiting' + ) + }) + + it('still reports done on a lead Stop, flagging that children were live', () => { + withBlockedChild() + const stopped = codexEvent({ hook_event_name: 'Stop' }) + // Read post-reap: agent-hooks/server.ts retires child rows on a root Stop and reports 'done'. + // The pre-reap flag carries the "children were live" signal instead of downgrading the state. + expect(stopped?.payload.state).toBe('done') + expect(stopped?.payload.leadStopWithLiveSubagents).toBe(true) + }) + + it('omits the flag on a lead Stop with no live children', () => { + codexEvent({ hook_event_name: 'SessionStart' }) + codexEvent({ hook_event_name: 'SubagentStart', agent_id: 'child-1', agent_type: 'explore' }) + codexEvent({ hook_event_name: 'SubagentStop', agent_id: 'child-1' }) + const stopped = codexEvent({ hook_event_name: 'Stop' }) + expect(stopped?.payload.state).toBe('done') + expect(stopped?.payload.leadStopWithLiveSubagents).toBeUndefined() + }) +})