mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 16:02:45 +00:00
refactor(native-chat): cancel a structured launch through an AbortSignal
The settle loop's launch cancellation re-derived the standard poll-plus-eager- event primitive that `AbortSignal` already is, so it now takes one. The eager semantics are unchanged: the loop still cancels on the abort event rather than only polling after awaits, so a staged prompt is discarded before it reaches the provider, and it drops its listener on settle instead of leaving the signal holding the closure. Quick create owns the controller and bridges its store subscription to it. A cancel that lands after the refusal fallback already opened a terminal now carries that surface on the settlement. It is the fallback's tab that exists, so reporting the pre-launch one handed the caller a workspace with no agent in it.
This commit is contained in:
@@ -54,26 +54,18 @@ const fallbackResult = {
|
||||
primaryTabId: 'fallback-tab'
|
||||
}
|
||||
|
||||
/** A caller-side cancel signal: `fire` is what the caller's store subscription would call. */
|
||||
/** A caller-side cancel signal: `fire` is what the caller's store subscription would abort on. */
|
||||
function fakeCancellation(initiallyCancelled = false) {
|
||||
let cancelled = initiallyCancelled
|
||||
const unsubscribe = vi.fn()
|
||||
const listeners: (() => void)[] = []
|
||||
const controller = new AbortController()
|
||||
if (initiallyCancelled) {
|
||||
controller.abort()
|
||||
}
|
||||
const removeEventListener = vi.spyOn(controller.signal, 'removeEventListener')
|
||||
return {
|
||||
unsubscribe,
|
||||
fire: () => {
|
||||
cancelled = true
|
||||
for (const listener of listeners) {
|
||||
listener()
|
||||
}
|
||||
},
|
||||
hook: {
|
||||
isCancelled: () => cancelled,
|
||||
subscribe: vi.fn((onCancel: () => void) => {
|
||||
listeners.push(onCancel)
|
||||
return unsubscribe
|
||||
})
|
||||
}
|
||||
/** The loop must drop its listener on settle, not leave the signal holding the closure. */
|
||||
removeEventListener,
|
||||
fire: () => controller.abort(),
|
||||
signal: controller.signal
|
||||
}
|
||||
}
|
||||
|
||||
@@ -151,7 +143,7 @@ describe('settleStructuredAgentLaunch', () => {
|
||||
expect(claimDefinitiveRefusalFallback).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns cancelled and discards a legacy fallback that was mid-flight', async () => {
|
||||
it('reports the surface of a legacy fallback that finished before the cancel', async () => {
|
||||
fakeLaunch({
|
||||
launchResult: Promise.reject(new StructuredAgentSessionCreateRefusalError('unsupported'))
|
||||
})
|
||||
@@ -168,14 +160,18 @@ describe('settleStructuredAgentLaunch', () => {
|
||||
'worktree-1',
|
||||
'codex',
|
||||
{},
|
||||
{ legacyFallback, cancellation: cancellation.hook }
|
||||
{ legacyFallback, signal: cancellation.signal }
|
||||
)
|
||||
await vi.waitFor(() => expect(legacyFallback).toHaveBeenCalledOnce())
|
||||
cancellation.fire()
|
||||
finishFallback()
|
||||
|
||||
// Current behavior: the fallback's tab is not reported back even though it opened.
|
||||
await expect(settlement).resolves.toEqual({ kind: 'cancelled', sessionId: 'session-1' })
|
||||
// Why: the fallback's terminal outlives the cancel, so its tab is the caller's real surface.
|
||||
await expect(settlement).resolves.toEqual({
|
||||
kind: 'cancelled',
|
||||
sessionId: 'session-1',
|
||||
fallback: fallbackResult
|
||||
})
|
||||
expect(mocks.cancelStructuredAgentLaunch).toHaveBeenCalledExactlyOnceWith(
|
||||
'worktree-1',
|
||||
'session-1'
|
||||
@@ -231,7 +227,7 @@ describe('settleStructuredAgentLaunch', () => {
|
||||
{},
|
||||
{
|
||||
onStructuredReady,
|
||||
cancellation: fakeCancellation(true).hook
|
||||
signal: fakeCancellation(true).signal
|
||||
}
|
||||
)
|
||||
).resolves.toEqual({ kind: 'cancelled', sessionId: 'session-1' })
|
||||
@@ -251,7 +247,7 @@ describe('settleStructuredAgentLaunch', () => {
|
||||
{},
|
||||
{
|
||||
legacyFallback,
|
||||
cancellation: fakeCancellation(true).hook
|
||||
signal: fakeCancellation(true).signal
|
||||
}
|
||||
)
|
||||
).resolves.toEqual({ kind: 'cancelled', sessionId: 'session-1' })
|
||||
@@ -272,7 +268,7 @@ describe('settleStructuredAgentLaunch', () => {
|
||||
'worktree-1',
|
||||
'codex',
|
||||
{},
|
||||
{ onStructuredReady, cancellation: cancellation.hook }
|
||||
{ onStructuredReady, signal: cancellation.signal }
|
||||
)
|
||||
expect(mocks.cancelStructuredAgentLaunch).not.toHaveBeenCalled()
|
||||
cancellation.fire()
|
||||
@@ -281,12 +277,12 @@ describe('settleStructuredAgentLaunch', () => {
|
||||
'worktree-1',
|
||||
'session-1'
|
||||
)
|
||||
expect(cancellation.unsubscribe).not.toHaveBeenCalled()
|
||||
expect(cancellation.removeEventListener).not.toHaveBeenCalled()
|
||||
|
||||
resolveLaunch({ sessionId: 'session-1', fence: 1 })
|
||||
await expect(settlement).resolves.toEqual({ kind: 'cancelled', sessionId: 'session-1' })
|
||||
expect(onStructuredReady).not.toHaveBeenCalled()
|
||||
expect(cancellation.unsubscribe).toHaveBeenCalledOnce()
|
||||
expect(cancellation.removeEventListener).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('honours a cancellation that fired before the loop subscribed', async () => {
|
||||
@@ -297,15 +293,14 @@ describe('settleStructuredAgentLaunch', () => {
|
||||
'worktree-1',
|
||||
'codex',
|
||||
{},
|
||||
{ cancellation: cancellation.hook }
|
||||
{ signal: cancellation.signal }
|
||||
)
|
||||
expect(cancellation.hook.subscribe).toHaveBeenCalledOnce()
|
||||
expect(mocks.cancelStructuredAgentLaunch).toHaveBeenCalledExactlyOnceWith(
|
||||
'worktree-1',
|
||||
'session-1'
|
||||
)
|
||||
await expect(settlement).resolves.toEqual({ kind: 'cancelled', sessionId: 'session-1' })
|
||||
expect(cancellation.unsubscribe).toHaveBeenCalledOnce()
|
||||
expect(cancellation.removeEventListener).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('unsubscribes from the cancel signal once a launch settles without cancelling', async () => {
|
||||
@@ -313,9 +308,9 @@ describe('settleStructuredAgentLaunch', () => {
|
||||
const cancellation = fakeCancellation()
|
||||
|
||||
await expect(
|
||||
settleStructuredAgentLaunch('worktree-1', 'codex', {}, { cancellation: cancellation.hook })
|
||||
settleStructuredAgentLaunch('worktree-1', 'codex', {}, { signal: cancellation.signal })
|
||||
).resolves.toEqual({ kind: 'structured', sessionId: 'session-1' })
|
||||
expect(mocks.cancelStructuredAgentLaunch).not.toHaveBeenCalled()
|
||||
expect(cancellation.unsubscribe).toHaveBeenCalledOnce()
|
||||
expect(cancellation.removeEventListener).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -22,24 +22,25 @@ export type StructuredAgentLaunchSettlement =
|
||||
promptDeliveryResult?: Promise<StructuredPromptDeliveryResult>
|
||||
}
|
||||
| ({ kind: 'refused-then-legacy' } & StructuredAgentLegacyFallbackResult)
|
||||
| { kind: 'cancelled'; sessionId: string }
|
||||
| {
|
||||
kind: 'cancelled'
|
||||
sessionId: string
|
||||
/** The legacy surface the refusal fallback had already opened when the cancel arrived; it
|
||||
* outlives the cancel, so the caller must report its tab rather than the pre-launch one. */
|
||||
fallback?: StructuredAgentLegacyFallbackResult
|
||||
}
|
||||
| { kind: 'visibility-unknown'; sessionId: string }
|
||||
| { kind: 'failed'; error: unknown }
|
||||
|
||||
export type StructuredAgentLaunchCancellation = {
|
||||
isCancelled: () => boolean
|
||||
/** Fires the moment the caller abandons the launch. The loop cancels eagerly on it so a staged
|
||||
* prompt is discarded before it can reach the provider; a token read only after awaits is late. */
|
||||
subscribe: (onCancel: () => void) => () => void
|
||||
}
|
||||
|
||||
export type StructuredAgentLaunchHooks = {
|
||||
/** What this flow did before structured chat existed: activate with a startup payload, set the
|
||||
* first-message rename flag, run trust preflight. Runs at most once, only on definitive refusal.
|
||||
* Resume has no legacy equivalent, so a refusal without this hook settles as `failed`. */
|
||||
legacyFallback?: () => Promise<StructuredAgentLegacyFallbackResult>
|
||||
onStructuredReady?: (sessionId: string) => void
|
||||
cancellation?: StructuredAgentLaunchCancellation
|
||||
/** Abort the moment the caller abandons the launch. The loop cancels on the event, not only by
|
||||
* polling after awaits, so a staged prompt is discarded before it can reach the provider. */
|
||||
signal?: AbortSignal
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -54,8 +55,9 @@ export async function settleStructuredAgentLaunch(
|
||||
hooks: StructuredAgentLaunchHooks
|
||||
): Promise<StructuredAgentLaunchSettlement> {
|
||||
const launch = startStructuredAgentLaunch(worktreeId, agent, options)
|
||||
const signal = hooks.signal
|
||||
let cancelRequested = false
|
||||
const isCancelled = (): boolean => cancelRequested || hooks.cancellation?.isCancelled() === true
|
||||
const isCancelled = (): boolean => cancelRequested || signal?.aborted === true
|
||||
const cancelLaunch = (): void => {
|
||||
if (cancelRequested) {
|
||||
return
|
||||
@@ -63,7 +65,7 @@ export async function settleStructuredAgentLaunch(
|
||||
cancelRequested = true
|
||||
cancelStructuredAgentLaunch(worktreeId, launch.sessionId)
|
||||
}
|
||||
const unsubscribe = hooks.cancellation?.subscribe(cancelLaunch)
|
||||
signal?.addEventListener('abort', cancelLaunch, { once: true })
|
||||
// Why: the caller may have been abandoned between its own check and this subscription.
|
||||
if (isCancelled()) {
|
||||
cancelLaunch()
|
||||
@@ -85,7 +87,8 @@ export async function settleStructuredAgentLaunch(
|
||||
: null
|
||||
const cancelled = (): StructuredAgentLaunchSettlement => ({
|
||||
kind: 'cancelled',
|
||||
sessionId: launch.sessionId
|
||||
sessionId: launch.sessionId,
|
||||
...(fallback.result ? { fallback: fallback.result } : {})
|
||||
})
|
||||
try {
|
||||
const receipt = await launch.launchResult
|
||||
@@ -129,6 +132,6 @@ export async function settleStructuredAgentLaunch(
|
||||
}
|
||||
return { kind: 'failed', error }
|
||||
} finally {
|
||||
unsubscribe?.()
|
||||
signal?.removeEventListener('abort', cancelLaunch)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user