From e9e9dde75334a9d6f93ffa40239fd1a708cea783 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:07:23 -0700 Subject: [PATCH] fix(agent-hooks): revive a retired pane on each provider's own new-turn event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The un-retirement gate matched two raw event-name literals, UserPromptSubmit and SessionStart. Only 5 of 18 hook sources name their turn boundary that way, so for the rest a reused pane stayed rowless forever: the user starts a new turn and no row ever appears in the sidebar or dashboard. Measured, not estimated — 10 providers fail the new test on main: gemini (BeforeAgent), antigravity (PreInvocation), amp (agent.start), cursor (beforeSubmitPrompt), pi/omp/prime-agent (before_agent_start), grok (user_prompt_submit), copilot (sessionStart, seen raw), hermes (pre_llm_call). The gate was also wrong in the other direction: opencode has no turn boundary at all, yet a literal SessionStart revived its pane. Both directions are covered. The correct per-provider classifier, isNewTurnEvent, was already imported into this file and already used 160 lines below — #14706 added that call specifically so consumers would stop re-deriving boundaries from literals, and left this one. Keeps the literal check when the remote envelope omits source: that field is optional, an older relay does not send it, and requiring it would have left every source-less remote pane retired forever. --- .../server-closed-tab-suppression.test.ts | 16 +++- .../server-retired-pane-new-turn.test.ts | 87 +++++++++++++++++++ src/main/agent-hooks/server.ts | 19 ++-- 3 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 src/main/agent-hooks/server-retired-pane-new-turn.test.ts diff --git a/src/main/agent-hooks/server-closed-tab-suppression.test.ts b/src/main/agent-hooks/server-closed-tab-suppression.test.ts index 9fe4ac51f9e..fb0694c241a 100644 --- a/src/main/agent-hooks/server-closed-tab-suppression.test.ts +++ b/src/main/agent-hooks/server-closed-tab-suppression.test.ts @@ -515,6 +515,12 @@ describe('AgentHookServer listener replay', () => { // Why: closedAgentStatusTabIds is LRU-bounded, so the tab fence is not permanent. // If restoring a sibling key lifted the closed tab's PANE fence too, eviction of the // tab id would leave nothing at all holding that pane shut. + // + // Why a non-boundary event: a genuine new turn deliberately lifts the pane fence + // (STA-3386), so it cannot be used to prove the fence exists. Measured on the pre-fix + // tree, `claude` + `UserPromptSubmit` already revived this pane; this case only ever + // passed with `before_agent_start` because the gate matched two raw literals and did not + // recognize pi's boundary. The control below keeps it from passing for that reason again. it('leaves a closed tab pane fenced once its tab id is evicted', async () => { const server = new AgentHookServer() await server.start({ env: 'production' }) @@ -545,10 +551,18 @@ describe('AgentHookServer listener replay', () => { } await postHook( - { hook_event_name: 'before_agent_start', prompt: 'after eviction' }, + { hook_event_name: 'agent_end', prompt: 'after eviction' }, { paneKey: detachedPane, tabId: 'tab-2' } ) expect(server.getStatusSnapshot()).toEqual([]) + + // Why: proves the empty snapshot above came from the fence and not from an inert + // event — the same post on an unfenced pane must produce a row. + await postHook( + { hook_event_name: 'agent_end', prompt: 'unfenced control' }, + { paneKey: makePaneKey('tab-9', LEAF_3), tabId: 'tab-9' } + ) + expect(server.getStatusSnapshot()).toHaveLength(1) } finally { server.stop() } diff --git a/src/main/agent-hooks/server-retired-pane-new-turn.test.ts b/src/main/agent-hooks/server-retired-pane-new-turn.test.ts new file mode 100644 index 00000000000..42025a8c499 --- /dev/null +++ b/src/main/agent-hooks/server-retired-pane-new-turn.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { AgentHookSource } from '../../shared/agent-hook-relay' +import { AgentHookServer, _internals } from './server' +import { PANE } from './server.test-fixtures' + +const { trackMock, getCohortAtEmitMock } = vi.hoisted(() => ({ + trackMock: vi.fn(), + getCohortAtEmitMock: vi.fn() +})) +vi.mock('../telemetry/client', () => ({ track: trackMock })) +vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: getCohortAtEmitMock })) + +beforeEach(() => { + _internals.resetCachesForTests() + trackMock.mockReset() + getCohortAtEmitMock.mockReset() + getCohortAtEmitMock.mockReturnValue({ nth_repo_added: 2 }) +}) +afterEach(() => vi.restoreAllMocks()) + +/** Each source's own new-turn boundary, as `isNewTurnEvent` classifies it. `null` means the + * provider has no turn boundary at all, so a retired pane there stays retired by design. */ +const NEW_TURN_EVENT: Record = { + claude: 'SessionStart', + kimi: 'UserPromptSubmit', + codex: 'SessionStart', + gemini: 'BeforeAgent', + antigravity: 'PreInvocation', + amp: 'agent.start', + cursor: 'beforeSubmitPrompt', + pi: 'before_agent_start', + omp: 'before_agent_start', + 'prime-agent': 'before_agent_start', + droid: 'UserPromptSubmit', + grok: 'user_prompt_submit', + copilot: 'sessionStart', + hermes: 'pre_llm_call', + devin: 'UserPromptSubmit', + opencode: null, + 'mimo-code': null, + 'command-code': null +} + +function reviveRetiredPane(source: AgentHookSource, hookEventName: string): boolean { + const server = new AgentHookServer() + // Why: retirement is what command completion leaves behind on a reusable shell pane. + server.retirePaneAuthority(PANE) + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + source, + hookEventName, + payload: { state: 'working', prompt: 'after reuse', agentType: 'claude' } + }, + 'conn-1' + ) + return server.getStatusSnapshot().some((entry) => entry.paneKey === PANE) +} + +describe("retired pane un-retires on each provider's own new-turn event", () => { + // Why: the gate matched two raw literals, so only the 5 sources that happen to name their + // boundary UserPromptSubmit/SessionStart could ever revive — the rest stayed rowless forever. + // Why: keys of a Record — a new source fails typecheck here rather than + // silently skipping coverage, which is the same guarantee the runtime list would give. + const revivable = (Object.keys(NEW_TURN_EVENT) as AgentHookSource[]).filter( + (source) => NEW_TURN_EVENT[source] !== null + ) + + it.each(revivable)('%s', (source) => { + const hookEventName = NEW_TURN_EVENT[source] + expect(hookEventName).not.toBeNull() + expect(reviveRetiredPane(source, hookEventName as string)).toBe(true) + }) + + it('leaves the pane retired for a source with no turn boundary', () => { + // Why: opencode/mimo-code/command-code have no new-turn event; reviving them would need a + // different signal, so the gate must stay closed rather than guess. + expect(reviveRetiredPane('opencode', 'SessionStart')).toBe(false) + }) + + it('leaves the pane retired for a non-boundary event on a revivable source', () => { + // Why: guards the inverse — the gate must not open on any event that merely mentions a session. + expect(reviveRetiredPane('gemini', 'AfterAgent')).toBe(false) + }) +}) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 58ad669784b..077e3b3ad85 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -1123,7 +1123,7 @@ export class AgentHookServer { private getAgentStatusDisposition( paneKey: string, - event?: { hookEventName?: string; isReplay?: boolean } + event?: { source?: AgentHookSource; hookEventName?: string; isReplay?: boolean } ): 'accept' | 'restart' | 'suppress' { const ownerPaneKey = this.resolvePaneKeyAlias(paneKey) const paneRetired = @@ -1137,12 +1137,17 @@ export class AgentHookServer { return 'accept' } // Why: command completion retires launch authority but leaves its shell pane reusable. - // A live SessionStart proves a new agent process owns the retired pane just like a + // A live new-turn event proves a new agent process owns the retired pane just like a // fresh prompt does — without it, a session resumed in a reused pane stays rowless (STA-3386). - if ( - (event?.hookEventName === 'UserPromptSubmit' || event?.hookEventName === 'SessionStart') && - event.isReplay !== true - ) { + // Why the classifier, not literals: only 5 of 18 sources name their boundary + // `UserPromptSubmit`/`SessionStart`; the rest stayed retired forever. + // Why the literal fallback: `source` is optional on the remote envelope, and an older + // relay omits it — without this, every source-less remote pane would stay retired forever. + const isNewTurn = + event?.source !== undefined + ? isNewTurnEvent(event.source, event.hookEventName) + : event?.hookEventName === 'UserPromptSubmit' || event?.hookEventName === 'SessionStart' + if (isNewTurn && event?.isReplay !== true) { this.closedAgentStatusPaneKeys.delete(paneKey) this.closedAgentStatusPaneKeys.delete(ownerPaneKey) return 'restart' @@ -2249,6 +2254,7 @@ export class AgentHookServer { ? envelope.compactTrigger : undefined const statusDisposition = this.getAgentStatusDisposition(paneKey, { + source, hookEventName, isReplay: envelope.isReplay === true }) @@ -2467,6 +2473,7 @@ export class AgentHookServer { const normalized = this.normalizeLocalHookPayload(source, aliasedBody) const statusDisposition = normalized.event ? this.getAgentStatusDisposition(normalized.event.paneKey, { + source, hookEventName: normalized.event.hookEventName, isReplay: normalized.event.isReplay })