From 5a2d2f49bc479008129db753bfa1698c98f87608 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sat, 24 Oct 2026 05:05:06 -0700 Subject: [PATCH] test: strengthen RPC cancellation, release rejection, and mobile prompt coverage (#33146) Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .../runtime-serve-terminal-smoke.test.mjs | 10 ++- .../scan-release-artifacts-antivirus.test.mjs | 6 +- .../verify-windows-inner-signature.test.mjs | 21 +++-- .../mobile-native-chat-permission.test.ts | 4 +- .../use-mobile-native-chat-cancel-ask.test.ts | 15 +++- .../use-mobile-native-chat-prompts.test.ts | 88 ++++++++++++------- ...abortable-runtime-environment-call.test.ts | 9 +- .../src/runtime/host-route-call.test.ts | 15 +++- ...untime-rpc-client-pairing-revision.test.ts | 1 + .../runtime/runtime-worktree-selector.test.ts | 2 +- 10 files changed, 118 insertions(+), 53 deletions(-) diff --git a/config/scripts/runtime-serve-terminal-smoke.test.mjs b/config/scripts/runtime-serve-terminal-smoke.test.mjs index ff0c2a450d2..1d34877eacc 100644 --- a/config/scripts/runtime-serve-terminal-smoke.test.mjs +++ b/config/scripts/runtime-serve-terminal-smoke.test.mjs @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { afterEach, expect, it } from 'vitest' import { resolveCli } from './runtime-serve-terminal-smoke.mjs' -import { packagedNodeRuntimePath } from './build-orcad-node.mjs' +import { NODE_RUNTIME_ASSETS } from '../../src/shared/node-runtime-pin.ts' import { currentTarget } from './server-build-target.mjs' import { ORCAD_CLI_ENTRY_FILENAME } from '../../src/shared/orcad-artifacts.ts' @@ -30,7 +30,13 @@ it('drives Orcad with its packaged CLI and pinned Node even when a desktop CLI e mkdirSync(dirname(bundled), { recursive: true }) writeFileSync(bundled, '') expect(resolveCli('orcad', root)).toEqual({ - command: packagedNodeRuntimePath(directory, currentTarget()), + command: join( + root, + 'out', + 'runtimes', + `node-${NODE_RUNTIME_ASSETS[currentTarget()].executableSha256}`, + ...(process.platform === 'win32' ? ['node.exe'] : ['bin', 'node']) + ), prefix: [bundled] }) }) diff --git a/config/scripts/scan-release-artifacts-antivirus.test.mjs b/config/scripts/scan-release-artifacts-antivirus.test.mjs index ab373c67b3d..7e8c439c33f 100644 --- a/config/scripts/scan-release-artifacts-antivirus.test.mjs +++ b/config/scripts/scan-release-artifacts-antivirus.test.mjs @@ -78,12 +78,14 @@ describe('antivirus detection report', () => { known: true, scanned: 70, flagged: [ - { engine: 'TrendMicro', category: 'malicious', detection: 'Trojan.MSIL.MSILHERACLES' } + { engine: 'TrendMicro', category: 'malicious', detection: 'Trojan.MSIL.MSILHERACLES' }, + { engine: 'Microsoft', category: 'suspicious', detection: 'Behavior:Win32/Test' } ] }) - expect(report).toContain('1 of 70 engines flag this build') + expect(report).toContain('2 of 70 engines flag this build') expect(report).toContain('TrendMicro: Trojan.MSIL.MSILHERACLES') + expect(report).toContain('Microsoft: Behavior:Win32/Test') }) it('flags the release when any single artifact carries a verdict', () => { diff --git a/config/scripts/verify-windows-inner-signature.test.mjs b/config/scripts/verify-windows-inner-signature.test.mjs index d833fb64774..e293e797a79 100644 --- a/config/scripts/verify-windows-inner-signature.test.mjs +++ b/config/scripts/verify-windows-inner-signature.test.mjs @@ -121,15 +121,19 @@ describe('verify-windows-inner-signature', () => { }) it('rejects invalid status and unexpected signer subjects', () => { - expect(classifySignature({ ...validSignature, status: 'NotSigned' }).message).toMatch( - /status is NotSigned/ - ) + expect(classifySignature({ ...validSignature, status: 'NotSigned' })).toMatchObject({ + ok: false, + message: expect.stringMatching(/status is NotSigned/) + }) expect( classifySignature({ ...validSignature, signerSubject: 'CN=SignPath Foundation Test, O=SignPath Foundation, L=Vienna, C=AT' - }).message - ).toMatch(/Unexpected Windows inner executable signer/) + }) + ).toMatchObject({ + ok: false, + message: expect.stringMatching(/Unexpected Windows inner executable signer/) + }) }) it('accepts an expected thumbprint as an alternate explicit allowlist', () => { @@ -150,8 +154,11 @@ describe('verify-windows-inner-signature', () => { expectedSigners: [DEFAULT_EXPECTED_SIGNER], expectedThumbprints: ['001122'] } - ).message - ).toMatch(/Unexpected Windows inner executable signer/) + ) + ).toMatchObject({ + ok: false, + message: expect.stringMatching(/Unexpected Windows inner executable signer/) + }) expect( classifySignature(validSignature, { expectedSigners: [DEFAULT_EXPECTED_SIGNER], diff --git a/mobile/src/session/mobile-native-chat-permission.test.ts b/mobile/src/session/mobile-native-chat-permission.test.ts index 62c86563010..3227df5de39 100644 --- a/mobile/src/session/mobile-native-chat-permission.test.ts +++ b/mobile/src/session/mobile-native-chat-permission.test.ts @@ -109,7 +109,7 @@ describe('detectAgentPermission', () => { const long = `Do you want to proceed? ${'x'.repeat(300)}` const result = detectAgentPermission({ state: 'waiting', lastAssistantMessage: long }) expect(result).not.toBeNull() - expect((result?.detail ?? '').length).toBeLessThanOrEqual(160) + expect(result?.detail).toBe(`Do you want to proceed? ${'x'.repeat(135)}…`) }) it('shortens long numbered option labels', () => { @@ -120,7 +120,7 @@ describe('detectAgentPermission', () => { }) const second = result?.options[1] expect(second?.send).toBe('2') - expect((second?.label ?? '').length).toBeLessThanOrEqual(40) + expect(second?.label).toBe('No, and explain in great detail exactly…') }) }) diff --git a/mobile/src/session/use-mobile-native-chat-cancel-ask.test.ts b/mobile/src/session/use-mobile-native-chat-cancel-ask.test.ts index ecd2761b7a1..f12342ac519 100644 --- a/mobile/src/session/use-mobile-native-chat-cancel-ask.test.ts +++ b/mobile/src/session/use-mobile-native-chat-cancel-ask.test.ts @@ -72,7 +72,20 @@ describe('mobile question rejection', () => { expect(f.cancelPending).toHaveBeenCalledOnce() const sends = f.sendRequest.mock.calls.filter(([method]) => method === 'terminal.send') expect(sends).toHaveLength(1) - expect(sends[0]?.[1]).toMatchObject({ terminal: 'terminal', text: '\x1b', enter: false }) + expect(f.sendRequest).toHaveBeenCalledWith( + 'terminal.send', + { + terminal: 'terminal', + text: '\x1b', + enter: false, + requireWriteSettlement: true, + client: { id: 'device', type: 'mobile' } + }, + { timeoutMs: 15_000, budgetSpansConnect: true } + ) + expect(f.cancelPending.mock.invocationCallOrder[0]).toBeLessThan( + f.sendRequest.mock.invocationCallOrder[0]! + ) expect(f.onSendError).not.toHaveBeenCalled() }) diff --git a/mobile/src/session/use-mobile-native-chat-prompts.test.ts b/mobile/src/session/use-mobile-native-chat-prompts.test.ts index 968d865d6cc..c4b742c2597 100644 --- a/mobile/src/session/use-mobile-native-chat-prompts.test.ts +++ b/mobile/src/session/use-mobile-native-chat-prompts.test.ts @@ -1,11 +1,21 @@ import { createElement } from 'react' -import TestRenderer from 'react-test-renderer' -import { describe, expect, it, vi } from 'vitest' +import TestRenderer, { type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { AgentStatusEntry } from '../../../src/shared/agent-status-types' import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import { buildOmpPlanReviewAnswer } from '../../../src/shared/omp-plan-review-prompt' import { useMobileNativeChatPrompts } from './use-mobile-native-chat-prompts' +const renderers: ReactTestRenderer[] = [] +afterEach(() => { + TestRenderer.act(() => { + for (const renderer of renderers) { + renderer.unmount() + } + }) + renderers.length = 0 +}) + const APPROVAL = JSON.stringify({ approval: { tool: 'Bash', summary: 'pnpm build > build.log 2>&1' } }) @@ -30,7 +40,7 @@ function promptsFor( return null } TestRenderer.act(() => { - TestRenderer.create(createElement(Probe)) + renderers.push(TestRenderer.create(createElement(Probe))) }) return captured! } @@ -216,37 +226,47 @@ describe('useMobileNativeChatPrompts ask state gate', () => { }) describe('useMobileNativeChatPrompts in a structured chat', () => { - it('shows a Codex async question and answers it with an ordinary send', async () => { - const sendWithOutcome = vi.fn(async () => 'accepted' as const) - const questions = [{ title: 'Which color?', options: ['Red', 'Blue'] }] - const message: NativeChatMessage = { - id: 'ask', - role: 'assistant', - blocks: [ - { type: 'text', text: 'Which color?\n- Red\n- Blue' }, - { type: 'tool-call', name: 'request_user_input_async', input: { questions } } - ], - timestamp: null, - source: 'transcript' - } - let captured: ReturnType | undefined - function Probe(): null { - captured = useMobileNativeChatPrompts({ - enabled: true, - structured: { permission: null, question: null, sendWithOutcome }, - status: null, - messages: [message], - transcriptLoading: false + it.each([ + ['accepted', true], + ['queued', true], + ['unknown', true], + ['rejected', false] + ] as const)( + 'answers a Codex async question through an ordinary %s send', + async (outcome, accepted) => { + const sendWithOutcome = vi.fn(async () => outcome) + const questions = [{ title: 'Which color?', options: ['Red', 'Blue'] }] + const message: NativeChatMessage = { + id: 'ask', + role: 'assistant', + blocks: [ + { type: 'text', text: 'Which color?\n- Red\n- Blue' }, + { type: 'tool-call', name: 'request_user_input_async', input: { questions } } + ], + timestamp: null, + source: 'transcript' + } + let captured: ReturnType | undefined + function Probe(): null { + captured = useMobileNativeChatPrompts({ + enabled: true, + structured: { permission: null, question: null, sendWithOutcome }, + status: null, + messages: [message], + transcriptLoading: false + }) + return null + } + TestRenderer.act(() => { + renderers.push(TestRenderer.create(createElement(Probe))) }) - return null - } - TestRenderer.act(() => { - TestRenderer.create(createElement(Probe)) - }) - const ask = captured?.ask - expect(ask?.questions[0]?.question).toBe('Which color?') - await expect(ask && captured?.structuredAnswer?.(ask, [{ indices: [1] }])).resolves.toBe(true) - expect(sendWithOutcome).toHaveBeenCalledWith('> Which color?\n\nBlue') - }) + const ask = captured?.ask + expect(ask?.questions[0]?.question).toBe('Which color?') + await expect(ask && captured?.structuredAnswer?.(ask, [{ indices: [1] }])).resolves.toBe( + accepted + ) + expect(sendWithOutcome).toHaveBeenCalledWith('> Which color?\n\nBlue') + } + ) }) diff --git a/src/renderer/src/runtime/abortable-runtime-environment-call.test.ts b/src/renderer/src/runtime/abortable-runtime-environment-call.test.ts index 8a7df53e8e0..f6a31e6d935 100644 --- a/src/renderer/src/runtime/abortable-runtime-environment-call.test.ts +++ b/src/renderer/src/runtime/abortable-runtime-environment-call.test.ts @@ -39,7 +39,7 @@ it('cancels setup using the preselected id and unsubscribes a late handle', asyn it('keeps the response deadline while setup is pending', async () => { vi.useFakeTimers() - const subscribe = vi.fn(() => new Promise(() => {})) + const subscribe = vi.fn((_args: { subscriptionId?: string }) => new Promise(() => {})) const cancelSubscription = vi.fn().mockResolvedValue(undefined) stubRuntimeEnvironmentBridge({ subscribe, cancelSubscription }) const pending = callAbortableRuntimeEnvironment( @@ -50,7 +50,12 @@ it('keeps the response deadline while setup is pending', async () => { new AbortController().signal ) const rejection = expect(pending).rejects.toThrow('timed out') - await vi.advanceTimersByTimeAsync(15_000) + await vi.advanceTimersByTimeAsync(14_999) + expect(cancelSubscription).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(1) await rejection expect(cancelSubscription).toHaveBeenCalledOnce() + expect(cancelSubscription).toHaveBeenCalledWith({ + subscriptionId: subscribe.mock.calls[0]?.[0].subscriptionId + }) }) diff --git a/src/renderer/src/runtime/host-route-call.test.ts b/src/renderer/src/runtime/host-route-call.test.ts index e1f6cedc9e6..f15eb4aad57 100644 --- a/src/renderer/src/runtime/host-route-call.test.ts +++ b/src/renderer/src/runtime/host-route-call.test.ts @@ -21,15 +21,26 @@ beforeEach(() => { describe('callHostRoute', () => { it('calls the route target with the pairing revision captured before the call', async () => { callRuntimeRpc.mockResolvedValue({ ok: true }) + const signal = new AbortController().signal await expect( - callHostRoute(route, 'folderWorkspace.update', { id: 'f' }, { timeoutMs: 5 }) + callHostRoute( + route, + 'folderWorkspace.update', + { id: 'f' }, + { timeoutMs: 5, signal, expectedEnvironmentRuntimeId: 'runtime-a' } + ) ).resolves.toEqual({ ok: true }) expect(callRuntimeRpc).toHaveBeenCalledWith( route.target, 'folderWorkspace.update', { id: 'f' }, - { timeoutMs: 5, expectedEnvironmentPairingRevision: 7 } + { + timeoutMs: 5, + signal, + expectedEnvironmentRuntimeId: 'runtime-a', + expectedEnvironmentPairingRevision: 7 + } ) }) diff --git a/src/renderer/src/runtime/runtime-rpc-client-pairing-revision.test.ts b/src/renderer/src/runtime/runtime-rpc-client-pairing-revision.test.ts index d75b49ee294..f552fdd22cc 100644 --- a/src/renderer/src/runtime/runtime-rpc-client-pairing-revision.test.ts +++ b/src/renderer/src/runtime/runtime-rpc-client-pairing-revision.test.ts @@ -49,6 +49,7 @@ it('captures the pairing revision before awaiting the compatibility probe', asyn }) await expect(request).resolves.toEqual({ ok: true }) + expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(2) expect(runtimeEnvironmentCall).toHaveBeenNthCalledWith(1, { selector: 'env-cas', method: 'status.get', diff --git a/src/renderer/src/runtime/runtime-worktree-selector.test.ts b/src/renderer/src/runtime/runtime-worktree-selector.test.ts index 0aee6d0571c..d65475eb6cc 100644 --- a/src/renderer/src/runtime/runtime-worktree-selector.test.ts +++ b/src/renderer/src/runtime/runtime-worktree-selector.test.ts @@ -7,7 +7,7 @@ import { brandEphemeralSetupTerminalWorktreeId } from '../../../shared/ephemeral describe('toRuntimeWorktreeSelector', () => { it('addresses raw worktree IDs as runtime ID selectors', () => { - expect(toRuntimeWorktreeSelector('wt-1')).toBe('id:wt-1') + expect(toRuntimeWorktreeSelector(' wt-1 ')).toBe('id:wt-1') expect(toRuntimeWorktreeSelector('repo-1::C:/Users/me/orca/workspaces/orca/new-worktree')).toBe( 'id:repo-1::C:/Users/me/orca/workspaces/orca/new-worktree' )