mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 00:03:52 +00:00
fix(agent-hooks): revive a retired pane on each provider's own new-turn event
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.
This commit is contained in:
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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<AgentHookSource, string | null> = {
|
||||
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<AgentHookSource, …> — 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)
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user