mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 00:03:08 +00:00
fix(agents): don't fail-open echo verification for short prompts
The unmatchable fallback fired echo=true on any post-paste render for prompts folding under 8 alphanumeric chars (e.g. "fix ci"), so their submit Enter landed on a swallowing trust/login screen — the exact #7466 failure the echo gate exists to prevent. Restrict the any-render fallback to content that folds to nothing (punctuation/whitespace only); short but non-empty prompts now match on their head sample and correctly withhold the submit when the screen renders none of their characters. Also add prompt-echo-wait unit coverage (local echo/timeout/cancel plus the previously-untested remote transport, late-resolve disposal, and subscribe-rejection paths). Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -0,0 +1,142 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { watchForPromptEcho } from './prompt-echo-wait'
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
subscribeToPtyData: vi.fn(),
|
||||
isRemoteRuntimePtyId: vi.fn(),
|
||||
subscribeToRuntimeTerminalData: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@/components/terminal-pane/pty-data-sidecar-subscriptions', () => ({
|
||||
subscribeToPtyData: mocks.subscribeToPtyData
|
||||
}))
|
||||
vi.mock('@/runtime/runtime-terminal-inspection', () => ({
|
||||
isRemoteRuntimePtyId: mocks.isRemoteRuntimePtyId
|
||||
}))
|
||||
vi.mock('@/runtime/runtime-terminal-stream', () => ({
|
||||
subscribeToRuntimeTerminalData: mocks.subscribeToRuntimeTerminalData
|
||||
}))
|
||||
|
||||
const PROMPT =
|
||||
'Resolve the current merge conflicts UNIQUEMARKER7466 and report the final git status.'
|
||||
// Real codex composer echo: every word placed by a CSI cursor move.
|
||||
const ECHO =
|
||||
'\x1b[11;3H\x1b[22mResolve\x1b[11;11Hthe\x1b[11;15Hcurrent\x1b[11;23Hmerge\x1b[11;29Hconflicts\x1b[11;39HUNIQUEMARKER7466'
|
||||
|
||||
async function flushMicrotasks(iterations = 4): Promise<void> {
|
||||
for (let index = 0; index < iterations; index += 1) {
|
||||
await Promise.resolve()
|
||||
}
|
||||
}
|
||||
|
||||
describe('watchForPromptEcho', () => {
|
||||
let localObserver: ((data: string) => void) | null
|
||||
let unsubscribe: ReturnType<typeof vi.fn>
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
vi.stubGlobal('window', {
|
||||
setTimeout: globalThis.setTimeout,
|
||||
clearTimeout: globalThis.clearTimeout
|
||||
})
|
||||
localObserver = null
|
||||
unsubscribe = vi.fn()
|
||||
mocks.subscribeToPtyData.mockReset()
|
||||
mocks.subscribeToPtyData.mockImplementation((_ptyId: string, observer: (d: string) => void) => {
|
||||
localObserver = observer
|
||||
return unsubscribe
|
||||
})
|
||||
mocks.isRemoteRuntimePtyId.mockReset()
|
||||
mocks.isRemoteRuntimePtyId.mockReturnValue(false)
|
||||
mocks.subscribeToRuntimeTerminalData.mockReset()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
it('resolves true and unsubscribes once the pasted content renders (local)', async () => {
|
||||
const watch = watchForPromptEcho('pty-1', PROMPT, 10_000, {})
|
||||
localObserver?.(ECHO)
|
||||
await expect(watch.result).resolves.toBe(true)
|
||||
expect(unsubscribe).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('resolves false and unsubscribes on timeout with no echo (local)', async () => {
|
||||
const watch = watchForPromptEcho('pty-1', PROMPT, 10_000, {})
|
||||
localObserver?.('\x1b[2;2H\x1b[K') // redraw only, no pasted characters
|
||||
await vi.advanceTimersByTimeAsync(10_000)
|
||||
await expect(watch.result).resolves.toBe(false)
|
||||
expect(unsubscribe).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('resolves false and unsubscribes when cancelled', async () => {
|
||||
const watch = watchForPromptEcho('pty-1', PROMPT, 10_000, {})
|
||||
watch.cancel()
|
||||
await expect(watch.result).resolves.toBe(false)
|
||||
expect(unsubscribe).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not fire a late timer after the echo already resolved (local)', async () => {
|
||||
const watch = watchForPromptEcho('pty-1', PROMPT, 10_000, {})
|
||||
localObserver?.(ECHO)
|
||||
await expect(watch.result).resolves.toBe(true)
|
||||
// Advancing past the deadline must not flip or re-settle the result.
|
||||
await vi.advanceTimersByTimeAsync(10_000)
|
||||
await expect(watch.result).resolves.toBe(true)
|
||||
expect(unsubscribe).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
describe('remote runtime transport', () => {
|
||||
let remoteObserver: ((data: string) => void) | null
|
||||
let remoteUnsubscribe: ReturnType<typeof vi.fn>
|
||||
|
||||
beforeEach(() => {
|
||||
mocks.isRemoteRuntimePtyId.mockReturnValue(true)
|
||||
remoteObserver = null
|
||||
remoteUnsubscribe = vi.fn()
|
||||
})
|
||||
|
||||
it('subscribes through the host stream and resolves true on echo', async () => {
|
||||
mocks.subscribeToRuntimeTerminalData.mockImplementation(
|
||||
(_settings, _ptyId, _label, observer: (d: string) => void) => {
|
||||
remoteObserver = observer
|
||||
return Promise.resolve(remoteUnsubscribe)
|
||||
}
|
||||
)
|
||||
const watch = watchForPromptEcho('remote-pty', PROMPT, 10_000, {})
|
||||
await flushMicrotasks()
|
||||
remoteObserver?.(ECHO)
|
||||
await expect(watch.result).resolves.toBe(true)
|
||||
expect(remoteUnsubscribe).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('disposes a subscription that resolves after the watch already settled', async () => {
|
||||
let resolveSubscribe!: (u: ReturnType<typeof vi.fn>) => void
|
||||
const pending = new Promise<ReturnType<typeof vi.fn>>((resolve) => {
|
||||
resolveSubscribe = resolve
|
||||
})
|
||||
mocks.subscribeToRuntimeTerminalData.mockImplementation(
|
||||
(_settings, _ptyId, _label, observer: (d: string) => void) => {
|
||||
remoteObserver = observer
|
||||
return pending
|
||||
}
|
||||
)
|
||||
const watch = watchForPromptEcho('remote-pty', PROMPT, 10_000, {})
|
||||
watch.cancel()
|
||||
await expect(watch.result).resolves.toBe(false)
|
||||
// The subscription attaches only now — it must be torn down, not leaked.
|
||||
resolveSubscribe(remoteUnsubscribe)
|
||||
await flushMicrotasks()
|
||||
expect(remoteUnsubscribe).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('resolves false when the remote subscription rejects', async () => {
|
||||
mocks.subscribeToRuntimeTerminalData.mockRejectedValue(new Error('closed'))
|
||||
const watch = watchForPromptEcho('remote-pty', PROMPT, 10_000, {})
|
||||
await flushMicrotasks()
|
||||
await expect(watch.result).resolves.toBe(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -81,7 +81,7 @@ describe('createPromptEchoScanner', () => {
|
||||
expect(scanner.observe('unrelated')).toBe(true)
|
||||
})
|
||||
|
||||
it('treats content that folds below the minimum sample as echoed on any render', () => {
|
||||
it('treats punctuation-only content that folds to nothing as echoed on any render', () => {
|
||||
const scanner = createPromptEchoScanner('!?— \n')
|
||||
expect(scanner.observe('any repaint at all')).toBe(true)
|
||||
})
|
||||
@@ -90,4 +90,17 @@ describe('createPromptEchoScanner', () => {
|
||||
const scanner = createPromptEchoScanner('!?— \n')
|
||||
expect(scanner.observe('')).toBe(false)
|
||||
})
|
||||
|
||||
it('does not fire a short prompt on a swallow-screen redraw that renders none of it', () => {
|
||||
// #7466: "fix ci" folds to 5 chars — it must still match on its head
|
||||
// sample, not fall back to "any render", or the submit lands on the
|
||||
// swallowing screen exactly as before this fix.
|
||||
const scanner = createPromptEchoScanner('fix ci')
|
||||
expect(scanner.observe(CODEX_TRUST_SWALLOW_REDRAW)).toBe(false)
|
||||
})
|
||||
|
||||
it('fires a short prompt once its characters actually render', () => {
|
||||
const scanner = createPromptEchoScanner('fix ci')
|
||||
expect(scanner.observe('\x1b[11;3Hfix\x1b[11;7Hci')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -41,7 +41,6 @@ export function foldTerminalTextForEchoMatch(raw: string): string {
|
||||
const FOLDED_PLACEHOLDER_PATTERNS = [/pastedcontent\d+chars/, /pastedtext\d+/]
|
||||
|
||||
const SAMPLE_LENGTH = 24
|
||||
const MIN_SAMPLE_LENGTH = 8
|
||||
// Why: refold the whole raw ring on every chunk instead of delta-folding —
|
||||
// escape sequences straddling chunk seams make incremental folds misalign,
|
||||
// and the scanner only lives for the few seconds after one paste, so the
|
||||
@@ -58,10 +57,13 @@ export function createPromptEchoScanner(pastedContent: string): PromptEchoScanne
|
||||
const folded = foldTerminalTextForEchoMatch(pastedContent)
|
||||
const head = folded.slice(0, SAMPLE_LENGTH)
|
||||
const tail = folded.length > SAMPLE_LENGTH ? folded.slice(-SAMPLE_LENGTH) : ''
|
||||
// Why: content that folds to almost nothing (punctuation-only prompts)
|
||||
// cannot be matched reliably — treat any post-paste render as echo rather
|
||||
// than blocking delivery on an impossible match.
|
||||
const unmatchable = head.length < MIN_SAMPLE_LENGTH
|
||||
// Why: only content that folds to nothing (punctuation/whitespace-only
|
||||
// prompts) is genuinely impossible to match — treat any post-paste render as
|
||||
// echo there rather than blocking delivery forever. Short-but-nonempty
|
||||
// prompts (e.g. "fix ci") must still match on the head sample below so a
|
||||
// swallow screen that renders none of their characters withholds the submit
|
||||
// (#7466); firing on an arbitrary redraw would resubmit the exact bug.
|
||||
const unmatchable = folded.length === 0
|
||||
|
||||
let rawRing = ''
|
||||
let echoed = false
|
||||
|
||||
Reference in New Issue
Block a user