mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
fix(ai-vault): ignore cancellations after request settlement (#20980)
Co-authored-by: m4air <m4air@Mac.localdomain>
This commit is contained in:
@@ -0,0 +1,51 @@
|
||||
# AI Vault scanner late cancellation
|
||||
|
||||
The scanner child kept cancellation IDs after their requests had already settled.
|
||||
Its response can still be in transit when the parent sends a cancellation, so this
|
||||
does not require an invalid caller. The completed request has already run its
|
||||
cleanup; nothing remains to delete the newly inserted ID.
|
||||
|
||||
The fix admits cancellation only while the existing `pending` set owns the request.
|
||||
That set includes both queued and running requests. Their cancellation and cleanup
|
||||
remain unchanged. No protocol or history-retention policy changes.
|
||||
|
||||
## Proof
|
||||
|
||||
Run from the repository root with dependencies installed:
|
||||
|
||||
```sh
|
||||
ORCA_BACKGROUND_LAUNCH=1 node --max-old-space-size=128 docs/audits/scanner-late-cancel/reproduce.mjs
|
||||
```
|
||||
|
||||
The script loads the checked-out production entry and derives the before version by
|
||||
removing only the three-line `pending` membership guard in memory. It checks that
|
||||
the guard occurs exactly once; no historical commit or Git access is needed.
|
||||
Esbuild strips TypeScript before both versions run in separate VM contexts. Only
|
||||
imported collaborators are stubbed: the production message handler, request lanes,
|
||||
sets, and cleanup run. After each synthetic first-prompt request completes, its
|
||||
matching cancel arrives.
|
||||
|
||||
The script asserts the counts and emits JSON with both source SHA-256 hashes and
|
||||
Node/platform/heap-limit provenance. [results.json](./results.json) records a run on
|
||||
Node v26.6.0 with a 128 MiB old-space limit. This measures retained entries, not RSS.
|
||||
|
||||
| Source | Requests/responses | Pending | Controllers | Retained cancel IDs |
|
||||
| ------ | -----------------: | ------: | ----------: | ------------------: |
|
||||
| Before | 1,000 / 1,000 | 0 | 0 | 1,000 |
|
||||
| After | 1,000 / 1,000 | 0 | 0 | 0 |
|
||||
|
||||
The regression test imports the production entry and directly observes its existing
|
||||
cancellation set through an admitted cancellation. It repeats late cancels after
|
||||
both successful and failed requests, verifies queued/running cancellation, and
|
||||
checks shutdown cleanup. No production diagnostics or test-only exports were added.
|
||||
|
||||
```sh
|
||||
ORCA_BACKGROUND_LAUNCH=1 node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/main/ai-vault/session-scanner-service-cancellation.test.ts src/main/ai-vault/session-scanner-service-entry.test.ts src/main/ai-vault/session-scanner-service-client.test.ts
|
||||
```
|
||||
|
||||
## Incident scope
|
||||
|
||||
The same unconditional insertion exists in release `v1.4.198`. This retains numeric
|
||||
IDs in the scanner child, not transcript contents in Electron main. It is a concrete
|
||||
small leak; it does not explain the reported roughly 26 MB/s main-process growth in
|
||||
#19768 or establish the cause of #19831's scope-level peak memory measurements.
|
||||
@@ -0,0 +1,94 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import { createHash } from 'node:crypto'
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { getHeapStatistics } from 'node:v8'
|
||||
import { runInNewContext } from 'node:vm'
|
||||
import { transform } from 'esbuild'
|
||||
|
||||
if (process.env.ORCA_BACKGROUND_LAUNCH !== '1') {
|
||||
throw new Error('Run with ORCA_BACKGROUND_LAUNCH=1')
|
||||
}
|
||||
|
||||
const sourcePath = 'src/main/ai-vault/session-scanner-service-entry.ts'
|
||||
const source = readFileSync(new URL(`../../../${sourcePath}`, import.meta.url), 'utf8')
|
||||
const guard = ' if (!pending.has(raw.id)) {\n return\n }\n'
|
||||
const requestCount = 1000
|
||||
assert.equal(source.split(guard).length, 2, 'Review changed baseline transform')
|
||||
|
||||
async function run(version) {
|
||||
const original = version === 'before' ? source.replace(guard, '') : source
|
||||
const { code } = await transform(original, { loader: 'ts', format: 'esm', target: 'es2022' })
|
||||
const entry = code.replace(/^import[\s\S]*?from ['"][^'"]+['"];?\n/gm, '')
|
||||
assert.equal(/^import\b/m.test(entry), false, 'Unexpected production import shape')
|
||||
const processStub = new EventEmitter()
|
||||
let responses = 0
|
||||
processStub.send = (message) => {
|
||||
if (message.type === 'result') {
|
||||
responses++
|
||||
}
|
||||
}
|
||||
processStub.pid = 1
|
||||
const context = {
|
||||
process: processStub,
|
||||
performance,
|
||||
AbortController,
|
||||
requestSessionSearchRoots: () => undefined,
|
||||
SessionScannerServiceSearch: class {
|
||||
handles() {
|
||||
return false
|
||||
}
|
||||
},
|
||||
AI_VAULT_SERVICE_PROTOCOL_VERSION: 1,
|
||||
aiVaultServiceLane: () => 'interactive',
|
||||
isAiVaultServiceRequest: (raw) => raw.type === 'request',
|
||||
readAiVaultFirstUserPrompt: async () => ({ prompt: null }),
|
||||
inspect: undefined
|
||||
}
|
||||
runInNewContext(
|
||||
`${entry}\ninspect = () => ({ pending: pending.size, controllers: controllers.size, cancelled: cancelled.size })`,
|
||||
context,
|
||||
{ timeout: 1000, filename: fileURLToPath(new URL(`../../../${sourcePath}`, import.meta.url)) }
|
||||
)
|
||||
try {
|
||||
processStub.emit('message', { type: 'init', protocol: 1 })
|
||||
for (let id = 1; id <= requestCount; id++) {
|
||||
processStub.emit('message', {
|
||||
type: 'request',
|
||||
id,
|
||||
operation: 'firstPrompt',
|
||||
request: { agent: 'claude', filePath: '/synthetic' }
|
||||
})
|
||||
await new Promise(setImmediate)
|
||||
processStub.emit('message', { type: 'cancel', id })
|
||||
}
|
||||
const retained = context.inspect()
|
||||
assert.equal(responses, requestCount)
|
||||
assert.equal(retained.pending, 0)
|
||||
assert.equal(retained.controllers, 0)
|
||||
assert.equal(retained.cancelled, version === 'before' ? requestCount : 0)
|
||||
return {
|
||||
source: version === 'before' ? 'working tree without pending guard' : 'working tree',
|
||||
sourceSha256: createHash('sha256').update(original).digest('hex'),
|
||||
requests: requestCount,
|
||||
responses,
|
||||
...retained
|
||||
}
|
||||
} finally {
|
||||
processStub.removeAllListeners()
|
||||
}
|
||||
}
|
||||
|
||||
const results = {
|
||||
node: process.version,
|
||||
platform: process.platform,
|
||||
architecture: process.arch,
|
||||
heapLimitBytes: getHeapStatistics().heap_size_limit,
|
||||
sourcePath,
|
||||
baselineTransform: 'Remove only the three-line pending-membership guard in memory',
|
||||
harness: 'Production entry in isolated VM contexts; imported collaborators stubbed',
|
||||
before: await run('before'),
|
||||
after: await run('after')
|
||||
}
|
||||
process.stdout.write(`${JSON.stringify(results, null, 2)}\n`)
|
||||
@@ -0,0 +1,27 @@
|
||||
{
|
||||
"node": "v26.6.0",
|
||||
"platform": "darwin",
|
||||
"architecture": "arm64",
|
||||
"heapLimitBytes": 234881024,
|
||||
"sourcePath": "src/main/ai-vault/session-scanner-service-entry.ts",
|
||||
"baselineTransform": "Remove only the three-line pending-membership guard in memory",
|
||||
"harness": "Production entry in isolated VM contexts; imported collaborators stubbed",
|
||||
"before": {
|
||||
"source": "working tree without pending guard",
|
||||
"sourceSha256": "846f9af9577dbaf14d05aa5a2eea9e16a44a28ef7993e6b04c43b65db1f1f44f",
|
||||
"requests": 1000,
|
||||
"responses": 1000,
|
||||
"pending": 0,
|
||||
"controllers": 0,
|
||||
"cancelled": 1000
|
||||
},
|
||||
"after": {
|
||||
"source": "working tree",
|
||||
"sourceSha256": "e300d4922da94da09abb3c14bbb240ef9593d8a4834a37598c1c2f77b14dcbf5",
|
||||
"requests": 1000,
|
||||
"responses": 1000,
|
||||
"pending": 0,
|
||||
"controllers": 0,
|
||||
"cancelled": 0
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,187 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type {
|
||||
AiVaultServiceChildMessage,
|
||||
AiVaultServiceParentMessage
|
||||
} from './session-scanner-service-protocol'
|
||||
import { AI_VAULT_SERVICE_PROTOCOL_VERSION } from './session-scanner-service-protocol'
|
||||
|
||||
const scanAiVaultSessions = vi.hoisted(() => vi.fn())
|
||||
const flushSessionParseCachePersist = vi.hoisted(() => vi.fn(async () => undefined))
|
||||
const closeSearch = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('./session-scanner', () => ({ scanAiVaultSessions }))
|
||||
vi.mock('./session-scanner-parse-cache', () => ({ invalidateSessionParseCacheEntry: vi.fn() }))
|
||||
vi.mock('./session-parse-cache-persistence', () => ({
|
||||
flushSessionParseCachePersist,
|
||||
initSessionParseCachePersistence: vi.fn()
|
||||
}))
|
||||
vi.mock('./session-scanner-service-search', () => ({
|
||||
SessionScannerServiceSearch: class {
|
||||
handles(): boolean {
|
||||
return false
|
||||
}
|
||||
close(): void {
|
||||
closeSearch()
|
||||
}
|
||||
}
|
||||
}))
|
||||
|
||||
const result = { sessions: [], issues: [], scannedAt: '2026-09-15' }
|
||||
const sent: AiVaultServiceChildMessage[] = []
|
||||
const disconnect = vi.fn()
|
||||
let restoreProcess = (): void => undefined
|
||||
|
||||
function emit(message: AiVaultServiceParentMessage): void {
|
||||
process.emit('message', message)
|
||||
}
|
||||
|
||||
function scan(id: number): void {
|
||||
emit({ type: 'request', id, operation: 'scan', options: {} })
|
||||
}
|
||||
|
||||
function settle(): Promise<void> {
|
||||
return new Promise((resolve) => setImmediate(resolve))
|
||||
}
|
||||
|
||||
function cancelAndObserveSet(id: number): Set<unknown> {
|
||||
const add = vi.spyOn(Set.prototype, 'add')
|
||||
try {
|
||||
emit({ type: 'cancel', id })
|
||||
const index = add.mock.calls.findIndex(([value]) => value === id)
|
||||
const retained = add.mock.contexts[index]
|
||||
if (!(retained instanceof Set)) {
|
||||
throw new Error('Expected an admitted cancellation.')
|
||||
}
|
||||
return retained
|
||||
} finally {
|
||||
add.mockRestore()
|
||||
}
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.resetModules()
|
||||
vi.clearAllMocks()
|
||||
sent.length = 0
|
||||
scanAiVaultSessions.mockResolvedValue(result)
|
||||
const sendDescriptor = Object.getOwnPropertyDescriptor(process, 'send')
|
||||
const disconnectDescriptor = Object.getOwnPropertyDescriptor(process, 'disconnect')
|
||||
const messageListeners = new Set(process.listeners('message'))
|
||||
const disconnectListeners = new Set(process.listeners('disconnect'))
|
||||
Object.defineProperty(process, 'send', {
|
||||
configurable: true,
|
||||
value: (message: AiVaultServiceChildMessage) => {
|
||||
sent.push(message)
|
||||
return true
|
||||
}
|
||||
})
|
||||
Object.defineProperty(process, 'disconnect', { configurable: true, value: disconnect })
|
||||
restoreProcess = () => {
|
||||
for (const listener of process.listeners('message')) {
|
||||
if (!messageListeners.has(listener)) {
|
||||
process.removeListener('message', listener)
|
||||
}
|
||||
}
|
||||
for (const listener of process.listeners('disconnect')) {
|
||||
if (!disconnectListeners.has(listener)) {
|
||||
process.removeListener('disconnect', listener)
|
||||
}
|
||||
}
|
||||
if (sendDescriptor) {
|
||||
Object.defineProperty(process, 'send', sendDescriptor)
|
||||
} else {
|
||||
Reflect.deleteProperty(process, 'send')
|
||||
}
|
||||
if (disconnectDescriptor) {
|
||||
Object.defineProperty(process, 'disconnect', disconnectDescriptor)
|
||||
} else {
|
||||
Reflect.deleteProperty(process, 'disconnect')
|
||||
}
|
||||
}
|
||||
await import('./session-scanner-service-entry')
|
||||
emit({
|
||||
type: 'init',
|
||||
protocol: AI_VAULT_SERVICE_PROTOCOL_VERSION,
|
||||
sessionParseCache: null,
|
||||
sessionSearch: null
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
emit({ type: 'shutdown' })
|
||||
await settle()
|
||||
restoreProcess()
|
||||
})
|
||||
|
||||
describe('AI Vault service cancellation ownership', () => {
|
||||
it.each(['result', 'error'] as const)(
|
||||
'does not retain late cancellations after a %s response',
|
||||
async (responseType) => {
|
||||
const first = Promise.withResolvers<typeof result>()
|
||||
scanAiVaultSessions.mockReturnValueOnce(first.promise)
|
||||
scan(1)
|
||||
await settle()
|
||||
const cancelled = cancelAndObserveSet(1)
|
||||
first.resolve(result)
|
||||
await settle()
|
||||
expect(cancelled.size).toBe(0)
|
||||
|
||||
if (responseType === 'error') {
|
||||
scanAiVaultSessions.mockRejectedValue(new Error('Synthetic parse failure'))
|
||||
}
|
||||
for (let id = 2; id <= 65; id++) {
|
||||
scan(id)
|
||||
await settle()
|
||||
expect(sent).toContainEqual(expect.objectContaining({ type: responseType, id }))
|
||||
// The parent can cancel while this completed response is still in transit.
|
||||
emit({ type: 'cancel', id })
|
||||
}
|
||||
emit({ type: 'cancel', id: 999 })
|
||||
expect(cancelled.size).toBe(0)
|
||||
}
|
||||
)
|
||||
|
||||
it('cancels running and queued requests and releases both IDs when they settle', async () => {
|
||||
const first = Promise.withResolvers<typeof result>()
|
||||
const signals: AbortSignal[] = []
|
||||
scanAiVaultSessions.mockImplementation(({ signal }: { signal: AbortSignal }) => {
|
||||
signals.push(signal)
|
||||
return signals.length === 1 ? first.promise : Promise.resolve(result)
|
||||
})
|
||||
scan(1)
|
||||
scan(2)
|
||||
await settle()
|
||||
const cancelled = cancelAndObserveSet(1)
|
||||
emit({ type: 'cancel', id: 2 })
|
||||
expect(signals).toHaveLength(1)
|
||||
expect(signals[0]?.aborted).toBe(true)
|
||||
expect(cancelled.size).toBe(2)
|
||||
first.resolve(result)
|
||||
await settle()
|
||||
expect(signals).toHaveLength(2)
|
||||
expect(signals[1]?.aborted).toBe(true)
|
||||
expect(cancelled.size).toBe(0)
|
||||
expect(sent.filter((message) => message.type === 'result')).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('aborts the running request and closes the service before ignoring later cancels', async () => {
|
||||
const first = Promise.withResolvers<typeof result>()
|
||||
let signal: AbortSignal | undefined
|
||||
scanAiVaultSessions.mockImplementation((options: { signal: AbortSignal }) => {
|
||||
signal = options.signal
|
||||
return first.promise
|
||||
})
|
||||
scan(1)
|
||||
await settle()
|
||||
const cancelled = cancelAndObserveSet(1)
|
||||
emit({ type: 'shutdown' })
|
||||
expect(signal?.aborted).toBe(true)
|
||||
first.resolve(result)
|
||||
await settle()
|
||||
expect(cancelled.size).toBe(0)
|
||||
expect(closeSearch).toHaveBeenCalledOnce()
|
||||
expect(flushSessionParseCachePersist).toHaveBeenCalledOnce()
|
||||
expect(disconnect).toHaveBeenCalledOnce()
|
||||
emit({ type: 'cancel', id: 1 })
|
||||
expect(cancelled.size).toBe(0)
|
||||
})
|
||||
})
|
||||
@@ -187,6 +187,9 @@ process.on('message', (raw: AiVaultServiceParentMessage) => {
|
||||
return
|
||||
}
|
||||
if (raw?.type === 'cancel') {
|
||||
if (!pending.has(raw.id)) {
|
||||
return
|
||||
}
|
||||
cancelled.add(raw.id)
|
||||
controllers.get(raw.id)?.abort()
|
||||
return
|
||||
|
||||
Reference in New Issue
Block a user