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..bda9b7713e3 --- /dev/null +++ b/src/main/agent-hooks/server-retired-pane-new-turn.test.ts @@ -0,0 +1,126 @@ +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', + // Why null for opencode today: its plugin emits no SessionStart, so it has no reachable + // boundary. A pending change adds one; this entry moves with that change, not before it. + opencode: null, + 'mimo-code': null, + 'command-code': null +} + +function reviveRetiredPane(source: unknown, 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 === undefined ? {} : { source }), + hookEventName, + payload: { + state: 'working', + prompt: 'after reuse', + agentType: typeof source === 'string' ? source : '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) + }) + + // Why these two: every case above passes `source`, so the source-less compatibility path — + // the branch added for older relays — would otherwise ship with no coverage at all. + it('revives on a literal boundary when an older relay omits source', () => { + expect(reviveRetiredPane(undefined, 'UserPromptSubmit')).toBe(true) + }) + + it('cannot revive a non-literal provider when an older relay omits source', () => { + // Why pinned: this is the accepted cost of the legacy shim, not an oversight. Widening the + // literal list to "fix" it would re-create the two-literal gate this change removes. + expect(reviveRetiredPane(undefined, 'before_agent_start')).toBe(false) + }) + + it('revives on any event from a provider this build does not recognize', () => { + // Why fail open: a newer host can relay a 19th provider whose boundary name is unknown here. + // `isAgentHookSource` rejects it, so it must not fall through to the legacy literals and + // strand the pane permanently. + expect(reviveRetiredPane('future-provider-19' as AgentHookSource, 'SomethingNewEntirely')).toBe( + true + ) + }) + + it.each([null, '', 19, { provider: 'future-provider-19' }])( + 'keeps malformed source value %j behind the retired-pane fence', + (source) => { + expect( + reviveRetiredPane(source, source === null ? 'SessionStart' : 'SomethingNewEntirely') + ).toBe(false) + } + ) + + it('leaves the pane retired for a source with no turn boundary', () => { + // Why mimo-code and command-code rather than opencode: these two have no boundary event in + // any planned state. Opencode is deliberately excluded — its plugin emits no SessionStart on + // main (verified: zero occurrences in opencode/hook-service.ts), so asserting either outcome + // for it would pin a synthetic event, and a pending change gives it a real one. + expect(reviveRetiredPane('mimo-code', 'SessionStart')).toBe(false) + expect(reviveRetiredPane('command-code', '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..2b0457e064a 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -1123,7 +1123,13 @@ export class AgentHookServer { private getAgentStatusDisposition( paneKey: string, - event?: { hookEventName?: string; isReplay?: boolean } + event?: { + source?: AgentHookSource + /** Raw wire value, so the gate can tell "field absent" from "field present but unknown". */ + rawSource?: unknown + hookEventName?: string + isReplay?: boolean + } ): 'accept' | 'restart' | 'suppress' { const ownerPaneKey = this.resolvePaneKeyAlias(paneKey) const paneRetired = @@ -1137,12 +1143,28 @@ 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 four branches: `source` collapses to undefined when an older relay omits the field, + // when a newer host sends an unknown string, and when the wire value is malformed. Only an + // unknown string is valid future-provider evidence. Unreachable from the local path, which + // 404s an unresolvable source. + const isNewTurn = + event?.source !== undefined + ? isNewTurnEvent(event.source, event.hookEventName) + : typeof event?.rawSource === 'string' && event.rawSource.trim().length > 0 + ? // Why fail OPEN for an unknown provider: its boundary event is unknowable here, and + // the costs are asymmetric — a stranded pane is invisible and permanent with no user + // recovery, while a spurious revive decays after AGENT_STATUS_STALE_AFTER_MS. + true + : event?.rawSource === undefined + ? // Why literals here: an older relay omits `source` entirely. Legacy shim only — it + // cannot revive a provider whose boundary event is named anything else. + event?.hookEventName === 'UserPromptSubmit' || event?.hookEventName === 'SessionStart' + : false + if (isNewTurn && event?.isReplay !== true) { this.closedAgentStatusPaneKeys.delete(paneKey) this.closedAgentStatusPaneKeys.delete(ownerPaneKey) return 'restart' @@ -2249,6 +2271,8 @@ export class AgentHookServer { ? envelope.compactTrigger : undefined const statusDisposition = this.getAgentStatusDisposition(paneKey, { + source, + rawSource: envelope.source, hookEventName, isReplay: envelope.isReplay === true }) @@ -2467,6 +2491,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 })