From 4d22306d259a9c2b94c8ec6d120e673f2e5b00a4 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Fri, 23 Oct 2026 20:17:20 -0700 Subject: [PATCH] test: remove workflow string checks and strengthen provider assertions (#33017) Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .../replace-cached-nsis-elevate.test.mjs | 43 ---------------- config/scripts/space-sharing-copy.test.ts | 50 ++++++++++++++++--- .../vitest-sqlite-runtime-cache.test.ts | 45 +++++++++-------- ...hook-listener-codex-subagent-names.test.ts | 9 +++- ...k-listener-command-code-transcript.test.ts | 28 +++++++++-- src/shared/agent-hook-listener-jcode.test.ts | 33 ++++++++++-- ...gent-hook-listener-omp-plan-review.test.ts | 2 +- 7 files changed, 131 insertions(+), 79 deletions(-) diff --git a/config/scripts/replace-cached-nsis-elevate.test.mjs b/config/scripts/replace-cached-nsis-elevate.test.mjs index ffdcc0b4b5b..971f5a55a62 100644 --- a/config/scripts/replace-cached-nsis-elevate.test.mjs +++ b/config/scripts/replace-cached-nsis-elevate.test.mjs @@ -11,7 +11,6 @@ import { import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { parse } from 'yaml' import { findCachedElevatePaths, @@ -319,45 +318,3 @@ describe('a cached elevate.exe miss is not silent', () => { expect(readFileSync(join(nsisDir, 'elevate.exe'), 'utf8')).toBe('signpath-signed-elevate') }) }) - -describe('release-cut.yml swaps the cached elevate.exe through the resolver', () => { - function swapStep() { - const workflow = parse( - readFileSync(join(projectRoot, '.github/workflows/release-cut.yml'), 'utf8') - ) - const step = workflow.jobs.build.steps.find( - (candidate) => candidate.name === 'Replace cached elevate.exe with the signed copy' - ) - expect(step).toBeDefined() - return step - } - - it('delegates the cache lookup to the script instead of an inline path', () => { - const step = swapStep() - expect(step.run).toContain('node config/scripts/replace-cached-nsis-elevate.mjs $signed') - // The hardcoded miss that shipped v1.4.193/v1.4.194 unsigned. - expect(step.run).not.toContain('electron-builder\\Cache\\nsis') - expect(step.run).not.toContain('-ErrorAction SilentlyContinue') - }) - - it('fails the step when the swap reports a miss', () => { - const step = swapStep() - // Matched as an executed statement: downgrading this to a Write-Host restores - // the silent fail-open that let the unsigned helper ship. - expect(step.run).toMatch(/if \(\$LASTEXITCODE -ne 0\) \{/) - expect(step.run).toMatch(/^\s*throw \$message\s*$/m) - expect(step.run).toContain('GITHUB_STEP_SUMMARY') - }) - - // Why kept: windows-signing-rehearsal.yml shares the electron-builder-win- - // cache key, so dropping this guard would let a test certificate reach a release cache. - it('still refuses to stage anything but a SignPath-signed helper', () => { - const step = swapStep() - expect(step.run).toContain("$signature.Status -ne 'Valid'") - expect(step.run).toContain("$subject -notlike '*CN=SignPath Foundation*'") - }) - - it('blocks the release when the cache swap fails', () => { - expect(swapStep()['continue-on-error']).toBeUndefined() - }) -}) diff --git a/config/scripts/space-sharing-copy.test.ts b/config/scripts/space-sharing-copy.test.ts index 7d6eb32b5f6..cfc9d624d95 100644 --- a/config/scripts/space-sharing-copy.test.ts +++ b/config/scripts/space-sharing-copy.test.ts @@ -14,8 +14,6 @@ import { tmpdir } from 'node:os' import path from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import { - LINUX_REFLINK_ARGS, - MACOS_CLONE_ARGS, copyPrivateTree, hardlinkTree, makeTreeReadOnly, @@ -56,7 +54,7 @@ describe('asynchronous immutable sharing', () => { expect(readlinkSync(path.join(destination, 'relative-link'))).toBe(path.join('nested', 'file')) }) - it('keeps the event loop available while a sharing process is pending', async () => { + it('waits for the asynchronous sharing mechanism to finish', async () => { const { root, source } = makeTree() let finish!: () => void const pending = new Promise((resolve) => { @@ -155,9 +153,48 @@ describe('shareTree', () => { ) }) - it('fails loudly instead of degrading, on both copy-out mechanisms', () => { - expect(MACOS_CLONE_ARGS).toContain('-P') - expect(LINUX_REFLINK_ARGS).toContain('--reflink=always') + it('uses strict native-copy arguments and handles their failures', async () => { + const nativeCopy = vi.fn(() => { + throw new Error('native storage sharing unavailable') + }) + vi.doMock('node:child_process', () => ({ execFileSync: nativeCopy })) + vi.resetModules() + try { + const { shareTree: shareWithNativeCopy } = await import('./space-sharing-copy.mjs') + const { root, source } = makeTree() + const cloneDestination = path.join(root, 'clone') + expect(() => shareWithNativeCopy(source, cloneDestination, { platform: 'darwin' })).toThrow( + /Could not share storage/ + ) + expect(existsSync(cloneDestination)).toBe(false) + expect(nativeCopy).toHaveBeenNthCalledWith( + 1, + '/bin/cp', + ['-c', '-R', '-P', source, cloneDestination], + { stdio: 'ignore' } + ) + + const reflinkDestination = path.join(root, 'reflink') + expect(shareWithNativeCopy(source, reflinkDestination, { platform: 'linux' })).toBe( + 'hardlink' + ) + expect(nativeCopy).toHaveBeenNthCalledWith( + 2, + 'cp', + ['--reflink=always', '-a', source, reflinkDestination], + { stdio: 'ignore' } + ) + expect(statSync(path.join(reflinkDestination, 'nested', 'file')).ino).toBe( + statSync(path.join(source, 'nested', 'file')).ino + ) + expect(readFileSync(path.join(reflinkDestination, 'nested', 'file'), 'utf8')).toBe('contents') + expect(readlinkSync(path.join(reflinkDestination, 'relative-link'))).toBe( + path.join('nested', 'file') + ) + } finally { + vi.doUnmock('node:child_process') + vi.resetModules() + } }) }) @@ -217,6 +254,7 @@ describe('makeTreeReadOnly', () => { const { source } = makeTree() const executable = path.join(source, 'electron') writeFileSync(executable, 'binary', { mode: 0o755 }) + chmodSync(executable, 0o755) makeTreeReadOnly(source) // Verified on real ext4: 0o555. Windows has no execute bit -- the read-only attribute does // not gate execution there, confirmed by running a read-only hardlinked .exe on NTFS. diff --git a/config/scripts/vitest-sqlite-runtime-cache.test.ts b/config/scripts/vitest-sqlite-runtime-cache.test.ts index cef4a816fa5..e41f5ff699e 100644 --- a/config/scripts/vitest-sqlite-runtime-cache.test.ts +++ b/config/scripts/vitest-sqlite-runtime-cache.test.ts @@ -8,27 +8,32 @@ it('re-derives SQLite assignments from malformed disposable cache data', async ( .update(readFileSync(new URL('./vitest-sqlite-runtime-scan.mjs', import.meta.url))) .digest('hex') for (const files of [null, { 'fixture.test.ts': [1, 2, 'false'] }]) { - vi.doMock('node:fs', async (original) => ({ - ...(await original()), - globSync: () => ['fixture.test.ts'], - statSync: () => ({ mtimeMs: 1, size: 2 }), - readFileSync: (path: Parameters[0]) => - String(path).endsWith('.json') - ? JSON.stringify({ version, files }) - : String(path).endsWith('fixture.test.ts') - ? 'export {}' - : readFileSync(path), - mkdirSync: vi.fn(), - writeFileSync: vi.fn(), - renameSync: vi.fn() - })) - vi.resetModules() - try { - const { findSqliteRuntimeTests } = await import('./vitest-sqlite-runtime-scan.mjs') - expect(findSqliteRuntimeTests()).toEqual([]) - } finally { - vi.doUnmock('node:fs') + for (const [source, expected] of [ + ['export {}', []], + ["import './src/main/sqlite/sync-database'", ['fixture.test.ts']] + ] as const) { + vi.doMock('node:fs', async (original) => ({ + ...(await original()), + globSync: () => ['fixture.test.ts'], + statSync: () => ({ mtimeMs: 1, size: 2 }), + readFileSync: (path: Parameters[0]) => + String(path).endsWith('.json') + ? JSON.stringify({ version, files }) + : String(path).endsWith('fixture.test.ts') + ? source + : readFileSync(path), + mkdirSync: vi.fn(), + writeFileSync: vi.fn(), + renameSync: vi.fn() + })) vi.resetModules() + try { + const { findSqliteRuntimeTests } = await import('./vitest-sqlite-runtime-scan.mjs') + expect(findSqliteRuntimeTests()).toEqual(expected) + } finally { + vi.doUnmock('node:fs') + vi.resetModules() + } } } }) diff --git a/src/shared/agent-hook-listener-codex-subagent-names.test.ts b/src/shared/agent-hook-listener-codex-subagent-names.test.ts index ec587bbd68b..a36585f1c88 100644 --- a/src/shared/agent-hook-listener-codex-subagent-names.test.ts +++ b/src/shared/agent-hook-listener-codex-subagent-names.test.ts @@ -65,7 +65,14 @@ describe('Codex child names from provider metadata', () => { it('recovers a child first seen in a tool hook and retries metadata that was not yet written', () => { const { writeMeta, post } = fixture() - expect(post('PostToolUse')?.[0]?.description).toBeUndefined() + expect(post('PostToolUse')).toEqual([ + expect.objectContaining({ + id: 'child', + agentType: 'default', + description: undefined, + state: 'working' + }) + ]) writeMeta({ agent_path: '/root/review/mobile' }) expect(post('PostToolUse')?.[0]?.description).toBe('mobile') }) diff --git a/src/shared/agent-hook-listener-command-code-transcript.test.ts b/src/shared/agent-hook-listener-command-code-transcript.test.ts index 6e8159bf72e..6a72c8bbdc0 100644 --- a/src/shared/agent-hook-listener-command-code-transcript.test.ts +++ b/src/shared/agent-hook-listener-command-code-transcript.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createHash } from 'node:crypto' import { mkdtempSync, rmSync, statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -18,6 +19,7 @@ describe('shared agent-hook-listener', () => { }) afterEach(() => { + vi.restoreAllMocks() clearGrokSessionPathLookupCacheForTests() vi.unstubAllEnvs() }) @@ -422,7 +424,10 @@ describe('shared agent-hook-listener', () => { 'production' ) - expect(tool?.payload.prompt.startsWith('pasted prompt WWW')).toBe(true) + expect(tool?.payload.prompt).toBe(promptText.slice(0, 200)) + expect(tool?.promptInteractionKey?.split('-')[5]).toBe( + createHash('sha256').update(promptText).digest('hex').slice(0, 12) + ) } finally { rmSync(tmpDir, { recursive: true, force: true }) } @@ -466,7 +471,24 @@ describe('shared agent-hook-listener', () => { 'production' ) - expect(tool?.payload.prompt ?? '').toBe('') + expect(tool?.payload).toMatchObject({ + state: 'working', + agentType: 'command-code', + prompt: '', + toolName: 'shell_command', + toolInput: 'pwd' + }) + writeFileSync(transcriptPath, `${lines[0]}\n`, { flag: 'a' }) + const recent = normalizeHookPayload( + createHookListenerState(), + 'command-code', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'PreToolUse', transcript_path: transcriptPath } + }, + 'production' + ) + expect(recent?.payload.prompt).toBe('ancient prompt') } finally { rmSync(tmpDir, { recursive: true, force: true }) } @@ -525,7 +547,7 @@ describe('shared agent-hook-listener', () => { const filler = Array.from({ length: 900 }, (_value, index) => JSON.stringify({ role: 'assistant', - content: [{ type: 'text', text: `${'f'.repeat(200)}${index}` }] + content: [{ type: 'text', text: `😀${'f'.repeat(200)}${index}` }] }) ) const head = `${filler.join('\n')}\n` diff --git a/src/shared/agent-hook-listener-jcode.test.ts b/src/shared/agent-hook-listener-jcode.test.ts index bd79d7ef955..ef8d077c09d 100644 --- a/src/shared/agent-hook-listener-jcode.test.ts +++ b/src/shared/agent-hook-listener-jcode.test.ts @@ -1,4 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { createHookListenerState, type HookListenerState @@ -6,9 +9,6 @@ import { import { normalizeHookPayload } from './agent-hook-listener' import { PANE_KEY } from './agent-hook-listener-test-harness' -// Payloads below are jcode 0.87.1's own `JCODE_HOOK_PAYLOAD` objects, captured by -// pointing every `[hooks]` entry at a logging script; see -// docs/reference/jcode-hook-events.md. function ingest(state: HookListenerState, payload: Record) { return normalizeHookPayload( state, @@ -20,13 +20,17 @@ function ingest(state: HookListenerState, payload: Record) { describe('shared agent-hook-listener: jcode', () => { let state: HookListenerState + let jcodeHome: string beforeEach(() => { + jcodeHome = mkdtempSync(join(tmpdir(), 'orca-jcode-hooks-')) + vi.stubEnv('JCODE_HOME', jcodeHome) state = createHookListenerState() }) afterEach(() => { vi.unstubAllEnvs() + rmSync(jcodeHome, { recursive: true, force: true }) }) it('maps turn_start to working before any tool has run', () => { @@ -104,11 +108,20 @@ describe('shared agent-hook-listener: jcode', () => { toolName: 'request_permission' }) expect(JSON.parse(event?.payload?.interactivePrompt ?? '{}')).toMatchObject({ - action: 'delete the staging bucket' + action: 'delete the staging bucket', + reason: 'Why this needs approval' }) }) it('does not re-open a question on post_tool, which fires after the answer', () => { + const pending = ingest(state, { + event: 'pre_tool', + session_id: 'session_jc_2', + tool_name: 'request_permission', + tool_input: '{"action":"delete the staging bucket","reason":"Why this needs approval"}' + }) + expect(pending?.payload.state).toBe('waiting') + expect(pending?.payload.interactivePrompt).toBeDefined() const event = ingest(state, { event: 'post_tool', session_id: 'session_jc_2', @@ -116,6 +129,7 @@ describe('shared agent-hook-listener: jcode', () => { status: 'ok' }) expect(event?.payload).toMatchObject({ state: 'working' }) + expect(event?.payload.interactivePrompt).toBeUndefined() }) it('leaves unrelated tool names working even when they read like a question', () => { @@ -145,6 +159,13 @@ describe('shared agent-hook-listener: jcode', () => { }) it('surfaces the turn error instead of a stale reply when a turn fails', () => { + const previous = ingest(state, { + event: 'turn_end', + session_id: 'session_jc_3', + status: 'ok', + last_assistant_text: 'Stale reply.' + }) + expect(previous?.payload.lastAssistantMessage).toBe('Stale reply.') const event = ingest(state, { event: 'turn_end', session_id: 'session_jc_3', @@ -184,12 +205,13 @@ describe('shared agent-hook-listener: jcode', () => { }) it('clears the previous turn tool when a new turn starts', () => { - ingest(state, { + const previous = ingest(state, { event: 'pre_tool', session_id: 'session_jc_4', tool_name: 'bash', tool_input: '{"command":"pnpm lint"}' }) + expect(previous?.payload).toMatchObject({ toolName: 'bash', toolInput: 'pnpm lint' }) const event = ingest(state, { event: 'turn_start', session_id: 'session_jc_4', @@ -209,6 +231,7 @@ describe('shared agent-hook-listener: jcode', () => { }) expect(event?.payload).toMatchObject({ agentType: 'jcode', state: 'done' }) expect(event?.providerSession).toEqual({ key: 'session_id', id: 'session_jc_5' }) + expect(event?.providerSessionOnly).toBe(true) }) it('reads the lifecycle point from jcode\u2019s own `event` key', () => { diff --git a/src/shared/agent-hook-listener-omp-plan-review.test.ts b/src/shared/agent-hook-listener-omp-plan-review.test.ts index c0e5f362f28..046aac7d47e 100644 --- a/src/shared/agent-hook-listener-omp-plan-review.test.ts +++ b/src/shared/agent-hook-listener-omp-plan-review.test.ts @@ -35,7 +35,7 @@ describe('OMP plan review status', () => { state = createHookListenerState() }) - it('blocks on the review with a plan card whose choices reach the extension', () => { + it('blocks on the review with a parsed plan card and answer choices', () => { const blocked = requestReview(state, '# Add a greeting') expect(blocked?.state).toBe('blocked')