From aecce221bd82d82cb04d4e2d5e05190c7b762df5 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:51:55 -0700 Subject: [PATCH] fix(test): deflake relay exec env, project boundary, and speech resume tests (#14446) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three test-suite problems, all root-caused in the tests rather than in production behavior. 1. src/relay/agent-exec-handler.test.ts (real failure, not a flake) The two spawn-argument assertions failed with "Number of calls: 1" — spawn ran, but the env differed. Cause: both assert `expect.objectContaining({ ...process.env, ... })`, which demands that every ambient variable reach the child verbatim. #7986 (1a6abc87d11) changed both sides at once: it rewrote the assertion from `env: process.env` to that objectContaining form, and in the same commit made the handler apply `applyTerminalGitCredentialPromptGuard`, which appends its own entries to Git's indexed-config protocol (GIT_CONFIG_COUNT / KEY_n / VALUE_n). So whenever the test runner's own environment already carries that protocol — exactly what Orca exports into its agent terminals — the snapshot expects GIT_CONFIG_COUNT=2 while the correctly guarded child gets 4. The test passes on a bare CI shell and fails when run from a guarded terminal. The implementation is right: appending the guard after the caller's config is the documented contract, and "guards wrapped agents after atomically replacing inherited indexed config" already covers it. Fixed the test instead, by clearing the guard-owned keys (GIT_CONFIG_* protocol and WSLENV) from the ambient env for the duration of the suite and restoring them afterwards, so the passthrough baseline is deterministic. No assertion was weakened or removed. 2. project-view-wrapper-source-context-boundary.test.ts (flake: 30s timeout) `buildProjectWorkItem` is a pure function, but it lived in ProjectViewWrapper.tsx, so importing it pulled in the store, sonner, lucide, and the whole UI kit — ~8.8s of transform and module evaluation for one assertion, which tipped past the 30s limit under parallel load. Extracted it to project-work-item.ts (its only dependency is githubProjectHost) and pointed the test there. Both test cases are unchanged. Also dropped the now-unneeded happy-dom environment, since nothing in the file touches the DOM any more. 9.15s -> 0.12s. 3. model-manager-download-resume.test.ts (flake: 30s timeout) "bounds a server that advances by pathologically tiny segments forever" drives the loop to the MAX_TOTAL_DOWNLOAD_REQUESTS ceiling of 4096. Each iteration did a real writeFileSync plus two statSync calls through getPartialDownloadBytes — ~12k synchronous filesystem syscalls in a tight loop. Fast on an idle disk, but it serializes against every other vitest worker on a loaded machine, which is what blew the per-test timeout. Stubbed getPartialDownloadBytes to read the byte counter the test already maintains, so the loop is pure CPU. The file was only ever a stand-in for that counter. Ceiling and rejection assertions are unchanged: 332ms -> 15ms. The two remaining ~1.1s cases in that file spend their time in the real 1s retry backoff around real stream and file-write plumbing; they are left on real timers because faking them would mean faking the transport too, and 1.1s leaves ample headroom. --- .../model-manager-download-resume.test.ts | 5 ++- src/relay/agent-exec-handler.test.ts | 21 +++++++++- .../github-project/ProjectViewWrapper.tsx | 38 +----------------- ...ew-wrapper-source-context-boundary.test.ts | 4 +- .../github-project/project-work-item.ts | 40 +++++++++++++++++++ 5 files changed, 66 insertions(+), 42 deletions(-) create mode 100644 src/renderer/src/components/github-project/project-work-item.ts diff --git a/src/main/speech/model-manager-download-resume.test.ts b/src/main/speech/model-manager-download-resume.test.ts index 60645d1aa28..2fb4329e5c5 100644 --- a/src/main/speech/model-manager-download-resume.test.ts +++ b/src/main/speech/model-manager-download-resume.test.ts @@ -37,6 +37,7 @@ type ModelManagerInternals = { redirectCount?: number, resumeOffset?: number ) => Promise + getPartialDownloadBytes: (filePath: string) => number } type ScriptedResponse = { @@ -288,11 +289,13 @@ describe('ModelManager download resume', () => { const manager = new ModelManager(dir) as unknown as ModelManagerInternals const filePath = join(dir, 'model.bin') let bytesWritten = 0 + // Why: the ceiling costs 4096 iterations, so read progress from memory — + // real per-iteration file I/O stalls this test under parallel load. + vi.spyOn(manager, 'getPartialDownloadBytes').mockImplementation(() => bytesWritten) // Advances one byte per request against a total larger than the request // ceiling, so it makes forward progress forever without ever completing. const downloadFileMock = vi.spyOn(manager, 'downloadFile').mockImplementation(() => { bytesWritten += 1 - writeFileSync(filePath, Buffer.alloc(bytesWritten)) return Promise.resolve() }) diff --git a/src/relay/agent-exec-handler.test.ts b/src/relay/agent-exec-handler.test.ts index c884c499dac..42a79469d42 100644 --- a/src/relay/agent-exec-handler.test.ts +++ b/src/relay/agent-exec-handler.test.ts @@ -1,5 +1,5 @@ import { execFile, spawn } from 'node:child_process' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type * as ChildProcess from 'node:child_process' import { createFakeChild, createHandlers, requestContext } from './agent-exec-handler-test-harness' import { TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV } from '../shared/terminal-git-credential-guard' @@ -18,10 +18,29 @@ const execFileMock = vi.mocked(execFile) type AgentExecResult = { exitCode: number | null; timedOut: boolean } +const GUARD_OWNED_ENV_RE = /^(?:GIT_CONFIG_(?:COUNT|KEY_\d+|VALUE_\d+)|WSLENV)$/ + describe('AgentExecHandler', () => { + let ambientGuardEnv: Record = {} + beforeEach(() => { spawnMock.mockReset() execFileMock.mockReset() + // Why: the guard rewrites these, so an already-guarded runner (Orca guards + // its own agent terminals) would not see its ambient values passed through. + ambientGuardEnv = {} + for (const key of Object.keys(process.env).filter((name) => GUARD_OWNED_ENV_RE.test(name))) { + ambientGuardEnv[key] = process.env[key] + delete process.env[key] + } + }) + + afterEach(() => { + for (const [key, value] of Object.entries(ambientGuardEnv)) { + if (value !== undefined) { + process.env[key] = value + } + } }) it('executes a non-interactive command with captured output and stdin', async () => { diff --git a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx index 7cac639ba8a..24941730c94 100644 --- a/src/renderer/src/components/github-project/ProjectViewWrapper.tsx +++ b/src/renderer/src/components/github-project/ProjectViewWrapper.tsx @@ -65,6 +65,7 @@ import { githubProjectHost, githubProjectIdentityKey } from '../../../../shared/github/project-identity' +import { buildProjectWorkItem } from './project-work-item' type Props = { selectedRepoIds: ReadonlySet @@ -94,43 +95,6 @@ function getProjectViewSourceScope(settings: Parameters label.name), - updatedAt: row.updatedAt, - author: null, - repoId, - prRepo - } -} - export default function ProjectViewWrapper({ selectedRepoIds }: Props): React.JSX.Element { const settings = useAppStore((s) => s.settings) const projectViewCache = useAppStore((s) => s.projectViewCache) diff --git a/src/renderer/src/components/github-project/project-view-wrapper-source-context-boundary.test.ts b/src/renderer/src/components/github-project/project-view-wrapper-source-context-boundary.test.ts index b7273b5e306..0ee72795a97 100644 --- a/src/renderer/src/components/github-project/project-view-wrapper-source-context-boundary.test.ts +++ b/src/renderer/src/components/github-project/project-view-wrapper-source-context-boundary.test.ts @@ -1,5 +1,3 @@ -// @vitest-environment happy-dom - import { readFileSync } from 'node:fs' import { join } from 'node:path' import { describe, expect, it } from 'vitest' @@ -21,7 +19,7 @@ function sourceBetween(source: string, startPattern: string, endPattern: string) describe('ProjectViewWrapper GitHub source context boundary', () => { it('builds project work items with a host-pinned repository identity', async () => { - const { buildProjectWorkItem } = await import('./ProjectViewWrapper') + const { buildProjectWorkItem } = await import('./project-work-item') const row: GitHubProjectRow = { id: 'PVTI_1', itemType: 'PULL_REQUEST', diff --git a/src/renderer/src/components/github-project/project-work-item.ts b/src/renderer/src/components/github-project/project-work-item.ts new file mode 100644 index 00000000000..cc68266fe88 --- /dev/null +++ b/src/renderer/src/components/github-project/project-work-item.ts @@ -0,0 +1,40 @@ +import { githubProjectHost } from '../../../../shared/github/project-identity' +import type { GitHubProjectRow } from '../../../../shared/github/project-types' +import type { GitHubWorkItem } from '../../../../shared/types' + +export function buildProjectWorkItem( + row: GitHubProjectRow, + repoId: string, + host?: string +): GitHubWorkItem | null { + if (row.itemType !== 'ISSUE' && row.itemType !== 'PULL_REQUEST') { + return null + } + if (row.content.number == null || !row.content.url) { + return null + } + const [owner, repo] = row.content.repository?.split('/') ?? [] + // Why: Project rows can reach mutation controls before detail hydration, so + // preserve their host-bearing repository identity on the initial item. + const prRepo = owner && repo ? { owner, repo, host: githubProjectHost(host) } : undefined + return { + id: `${row.itemType === 'PULL_REQUEST' ? 'pr' : 'issue'}:${row.content.number}`, + type: row.itemType === 'PULL_REQUEST' ? 'pr' : 'issue', + number: row.content.number, + title: row.content.title, + state: + row.content.state === 'MERGED' + ? 'merged' + : row.content.state === 'CLOSED' + ? 'closed' + : row.content.isDraft + ? 'draft' + : 'open', + url: row.content.url, + labels: row.content.labels.map((label) => label.name), + updatedAt: row.updatedAt, + author: null, + repoId, + prRepo + } +}