mirror of
https://github.com/stablyai/orca.git
synced 2026-10-07 00:02:29 +00:00
fix(runtime): GC expired proven-absent leaf PTY verdicts (#12810)
* fix(runtime): GC expired proven-absent leaf PTY verdicts Sweep TTL-expired provenAbsentLeafPtyVerdicts on consult and insert so dead leaf ids do not retain Map entries for the process lifetime. Preserves the 15s no-reprobe cache for still-fresh verdicts. Fixes #12660 * fix(runtime): reclaim expired leaf PTY verdicts with bounded sweep frequency --------- Co-authored-by: m4air <m4air@Mac.localdomain>
This commit is contained in:
@@ -0,0 +1,136 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { OrcaRuntimeService } from './orca-runtime'
|
||||
import { PROVEN_ABSENT_LEAF_PTY_TTL_MS as TTL_MS } from './orca-runtime-core'
|
||||
|
||||
type VerdictInternals = {
|
||||
provenAbsentLeafPtyVerdicts: Map<string, number>
|
||||
isLeafPtyProvenAbsent: (ptyId: string) => Promise<boolean>
|
||||
}
|
||||
|
||||
function createRuntime(
|
||||
probePtyLiveness = vi.fn<(ptyId: string) => Promise<boolean | null>>(async () => false)
|
||||
) {
|
||||
const runtime = new OrcaRuntimeService()
|
||||
runtime.setPtyController({
|
||||
write: () => true,
|
||||
kill: () => true,
|
||||
getForegroundProcess: async () => null,
|
||||
hasPty: (id) => id === 'live',
|
||||
probePtyLiveness
|
||||
})
|
||||
const internals = runtime as unknown as VerdictInternals
|
||||
return {
|
||||
runtime,
|
||||
probe: probePtyLiveness,
|
||||
verdicts: internals.provenAbsentLeafPtyVerdicts,
|
||||
isAbsent: (id: string) => internals.isLeafPtyProvenAbsent(id)
|
||||
}
|
||||
}
|
||||
|
||||
afterEach(() => vi.restoreAllMocks())
|
||||
|
||||
describe('leaf PTY verdict expiry', () => {
|
||||
it('retires old unique IDs on a live-PTY consult without probing that live PTY', async () => {
|
||||
const now = vi.spyOn(Date, 'now').mockReturnValue(100_000)
|
||||
const { verdicts, isAbsent, probe } = createRuntime()
|
||||
for (let index = 0; index < 1_000; index++) {
|
||||
await expect(isAbsent(`retired-${index}`)).resolves.toBe(true)
|
||||
}
|
||||
expect(verdicts.size).toBe(1_000)
|
||||
now.mockReturnValue(100_000 + TTL_MS)
|
||||
|
||||
await expect(isAbsent('live')).resolves.toBe(false)
|
||||
|
||||
expect(verdicts.size).toBe(0)
|
||||
expect(probe).toHaveBeenCalledTimes(1_000)
|
||||
})
|
||||
|
||||
it('preserves every fresh verdict and the exact per-key TTL between bulk sweeps', async () => {
|
||||
const now = vi.spyOn(Date, 'now').mockReturnValue(100_000)
|
||||
const { verdicts, isAbsent, probe } = createRuntime()
|
||||
await isAbsent('initial')
|
||||
now.mockReturnValue(101_000)
|
||||
for (let index = 0; index < 1_000; index++) {
|
||||
await isAbsent(`fresh-${index}`)
|
||||
}
|
||||
now.mockReturnValue(100_000 + TTL_MS)
|
||||
await isAbsent('live')
|
||||
expect(verdicts.size).toBe(1_000)
|
||||
now.mockReturnValue(101_000 + TTL_MS - 1)
|
||||
probe.mockClear()
|
||||
for (let index = 0; index < 1_000; index++) {
|
||||
await expect(isAbsent(`fresh-${index}`)).resolves.toBe(true)
|
||||
}
|
||||
expect(probe).not.toHaveBeenCalled()
|
||||
now.mockReturnValue(101_000 + TTL_MS)
|
||||
probe.mockResolvedValue(null)
|
||||
|
||||
await expect(isAbsent('fresh-0')).resolves.toBe(false)
|
||||
|
||||
expect(probe).toHaveBeenCalledOnce()
|
||||
expect(verdicts.has('fresh-0')).toBe(false)
|
||||
})
|
||||
|
||||
it('sweeps at most once per TTL through a burst of probes and live sends', async () => {
|
||||
const now = vi.spyOn(Date, 'now').mockReturnValue(100_000)
|
||||
const { verdicts, isAbsent } = createRuntime()
|
||||
const iterations = vi.spyOn(verdicts, Symbol.iterator)
|
||||
for (let index = 0; index < 1_000; index++) {
|
||||
await isAbsent(`dead-${index}`)
|
||||
await isAbsent('live')
|
||||
}
|
||||
expect(iterations).toHaveBeenCalledOnce()
|
||||
now.mockReturnValue(100_000 + TTL_MS)
|
||||
for (let index = 0; index < 1_000; index++) {
|
||||
await isAbsent('live')
|
||||
}
|
||||
expect(iterations).toHaveBeenCalledTimes(2)
|
||||
expect(verdicts.size).toBe(0)
|
||||
})
|
||||
|
||||
it('cleans old entries when a delayed probe completes after the next sweep is due', async () => {
|
||||
const now = vi.spyOn(Date, 'now').mockReturnValue(100_000)
|
||||
const { verdicts, isAbsent, probe } = createRuntime()
|
||||
await isAbsent('old')
|
||||
let finish!: (value: boolean | null) => void
|
||||
probe.mockImplementationOnce(() => new Promise((resolve) => (finish = resolve)))
|
||||
const pending = isAbsent('new')
|
||||
now.mockReturnValue(100_000 + 2 * TTL_MS)
|
||||
finish(false)
|
||||
|
||||
await expect(pending).resolves.toBe(true)
|
||||
|
||||
expect([...verdicts]).toEqual([['new', 100_000 + 2 * TTL_MS]])
|
||||
})
|
||||
|
||||
it('resumes pruning after a backward clock adjustment without expiring future-dated evidence', async () => {
|
||||
const now = vi.spyOn(Date, 'now').mockReturnValue(100_000)
|
||||
const { verdicts, isAbsent, probe } = createRuntime()
|
||||
await isAbsent('future-dated')
|
||||
now.mockReturnValue(1_000)
|
||||
await isAbsent('after-clock-change')
|
||||
now.mockReturnValue(1_000 + TTL_MS)
|
||||
|
||||
await isAbsent('live')
|
||||
|
||||
expect([...verdicts]).toEqual([['future-dated', 100_000]])
|
||||
await expect(isAbsent('future-dated')).resolves.toBe(true)
|
||||
expect(probe).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('leaves unverifiable probes uncached and preserves concurrent probe coalescing', async () => {
|
||||
vi.spyOn(Date, 'now').mockReturnValue(100_000)
|
||||
let finish!: (value: boolean | null) => void
|
||||
const probe = vi.fn(() => new Promise<boolean | null>((resolve) => (finish = resolve)))
|
||||
const { verdicts, isAbsent } = createRuntime(probe)
|
||||
const first = isAbsent('ssh-id')
|
||||
const second = isAbsent('ssh-id')
|
||||
expect(first).toBe(second)
|
||||
finish(null)
|
||||
await expect(first).resolves.toBe(false)
|
||||
expect(verdicts.size).toBe(0)
|
||||
probe.mockRejectedValueOnce(new Error('host unavailable'))
|
||||
await expect(isAbsent('ssh-id')).resolves.toBe(false)
|
||||
expect(verdicts.size).toBe(0)
|
||||
})
|
||||
})
|
||||
@@ -1,6 +1,7 @@
|
||||
// @ts-nocheck -- mechanically split from OrcaRuntimeService; behavior is covered by AST equivalence and characterization tests.
|
||||
import { OrcaRuntimeWithResolveTerminalPane } from './orca-runtime-resolve-terminal-pane'
|
||||
import { PROVEN_ABSENT_LEAF_PTY_TTL_MS } from './orca-runtime-core'
|
||||
import { pruneExpiredProvenAbsentLeafPtyVerdicts } from './proven-absent-leaf-pty-verdicts'
|
||||
import type { RuntimeTerminalSend } from '../../shared/runtime-types'
|
||||
import type { RuntimeAgentPromptWriteOptions } from './runtime-terminal-contracts'
|
||||
import {
|
||||
@@ -10,6 +11,26 @@ import {
|
||||
import { buildAgentPromptPasteBytes } from '../../shared/agent-prompt-injection'
|
||||
|
||||
export class OrcaRuntimeWithControllerKnowsPtyIsLive extends OrcaRuntimeWithResolveTerminalPane {
|
||||
private lastProvenAbsentLeafPtyVerdictPruneAt: number | undefined
|
||||
|
||||
private pruneExpiredLeafPtyVerdicts(now: number): void {
|
||||
const lastPruneAt = this.lastProvenAbsentLeafPtyVerdictPruneAt
|
||||
// Per-key expiry stays exact; throttle whole-cache scans on the keystroke path.
|
||||
if (
|
||||
lastPruneAt !== undefined &&
|
||||
now >= lastPruneAt &&
|
||||
now - lastPruneAt < PROVEN_ABSENT_LEAF_PTY_TTL_MS
|
||||
) {
|
||||
return
|
||||
}
|
||||
this.lastProvenAbsentLeafPtyVerdictPruneAt = now
|
||||
pruneExpiredProvenAbsentLeafPtyVerdicts(
|
||||
this.provenAbsentLeafPtyVerdicts,
|
||||
now,
|
||||
PROVEN_ABSENT_LEAF_PTY_TTL_MS
|
||||
)
|
||||
}
|
||||
|
||||
protected controllerKnowsPtyIsLive(ptyId: string): boolean {
|
||||
try {
|
||||
return this.ptyController?.hasPty?.(ptyId) === true
|
||||
@@ -21,6 +42,7 @@ export class OrcaRuntimeWithControllerKnowsPtyIsLive extends OrcaRuntimeWithReso
|
||||
|
||||
/** True only on controller-proven absence; live, unknown, and probe errors all answer false. */
|
||||
protected isLeafPtyProvenAbsent(ptyId: string): Promise<boolean> {
|
||||
this.pruneExpiredLeafPtyVerdicts(Date.now())
|
||||
// Why hasPty and not ptysById: graph sync mirrors a connected record for
|
||||
// every leaf ptyId — including a prior process's — so runtime records can't
|
||||
// distinguish live from stale. The controller's exact-id hasPty is the
|
||||
@@ -50,7 +72,9 @@ export class OrcaRuntimeWithControllerKnowsPtyIsLive extends OrcaRuntimeWithReso
|
||||
if ((await probeLiveness(ptyId)) !== false) {
|
||||
return false
|
||||
}
|
||||
this.provenAbsentLeafPtyVerdicts.set(ptyId, Date.now())
|
||||
const now = Date.now()
|
||||
this.pruneExpiredLeafPtyVerdicts(now)
|
||||
this.provenAbsentLeafPtyVerdicts.set(ptyId, now)
|
||||
return true
|
||||
} catch {
|
||||
// Why: a failed probe is unknown, and unknown never rejects a write.
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { pruneExpiredProvenAbsentLeafPtyVerdicts } from './proven-absent-leaf-pty-verdicts'
|
||||
|
||||
describe('pruneExpiredProvenAbsentLeafPtyVerdicts', () => {
|
||||
it('removes only entries at or past the TTL without a re-probe', () => {
|
||||
const map = new Map<string, number>([
|
||||
['live-dead', 1_000],
|
||||
['still-fresh', 1_400],
|
||||
['exact-expiry', 1_000]
|
||||
])
|
||||
pruneExpiredProvenAbsentLeafPtyVerdicts(map, 1_000 + 15_000, 15_000)
|
||||
expect([...map.keys()]).toEqual(['still-fresh'])
|
||||
})
|
||||
|
||||
it('leaves an empty map alone', () => {
|
||||
const map = new Map<string, number>()
|
||||
pruneExpiredProvenAbsentLeafPtyVerdicts(map, Date.now(), 15_000)
|
||||
expect(map.size).toBe(0)
|
||||
})
|
||||
|
||||
it('clears everything when ttl is non-positive', () => {
|
||||
const map = new Map<string, number>([['a', 1]])
|
||||
pruneExpiredProvenAbsentLeafPtyVerdicts(map, 100, 0)
|
||||
expect(map.size).toBe(0)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,16 @@
|
||||
/** Drop cache entries whose TTL has elapsed without requiring a re-probe of that ptyId. */
|
||||
export function pruneExpiredProvenAbsentLeafPtyVerdicts(
|
||||
verdicts: Map<string, number>,
|
||||
nowMs: number,
|
||||
ttlMs: number
|
||||
): void {
|
||||
if (ttlMs <= 0) {
|
||||
verdicts.clear()
|
||||
return
|
||||
}
|
||||
for (const [ptyId, verdictAt] of verdicts) {
|
||||
if (nowMs - verdictAt >= ttlMs) {
|
||||
verdicts.delete(ptyId)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user