diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 92f96de38a..f87eb00db3 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -7,7 +7,11 @@ on: concurrency: group: 'nightly-${{ github.ref }}' - cancel-in-progress: false + # cancel-in-progress ensures that a superseded run (manual dispatch on the + # same ref, or a new nightly cron trigger) does not hold runner capacity. + # With it off those runs serialize in the group and contend for the runner + # pool, which produced multi-hour queue-wait outliers (issue #3149). + cancel-in-progress: true permissions: checks: 'write' @@ -22,6 +26,7 @@ jobs: windows_ci: name: 'Windows CI (Nightly)' runs-on: '${{ matrix.os }}' + timeout-minutes: 30 strategy: fail-fast: false matrix: @@ -114,12 +119,6 @@ jobs: LLXPRT_COVERAGE: 'false' run: 'npm run test' - - name: 'Run script harness tests (macOS)' - if: matrix.os == 'macos-latest' - env: - CI: true - run: npm run test:scripts - - name: 'Smoke test CLI entry (launcher -> Bun, no Node in chain)' run: './packages/cli/bin/llxprt --version' @@ -210,6 +209,7 @@ jobs: echo "VITEST_POOL_TIMEOUT=60000" >> "$GITHUB_ENV" - name: 'Run tests and generate reports' + id: macos_main_tests env: # Provider configuration from repository secrets/variables OPENAI_API_KEY: ${{ secrets[vars.KEY_VAR_NAME] }} @@ -231,11 +231,17 @@ jobs: run: 'npm run test' - name: 'Run script harness tests' + # Run when the main test step completed (success or failure) but NOT + # when the workflow was cancelled — so macOS platform coverage of the + # script harness still runs after a test failure, without burning + # runner minutes after cancellation (issue #3149). + if: ${{ !cancelled() && (steps.macos_main_tests.outcome == 'success' || steps.macos_main_tests.outcome == 'failure') }} env: CI: true run: 'npm run test:scripts' - name: 'Run shell-script behavioral tests (#2606)' + if: ${{ !cancelled() && (steps.macos_main_tests.outcome == 'success' || steps.macos_main_tests.outcome == 'failure') }} env: CI: true run: 'npm run test:shell' @@ -316,6 +322,7 @@ jobs: e2e_full: name: 'E2E Full - ${{ matrix.os }} - ${{ matrix.sandbox }}' runs-on: '${{ matrix.os }}' + timeout-minutes: 30 strategy: fail-fast: false matrix: @@ -570,6 +577,7 @@ jobs: if: ${{ always() && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')) }} permissions: issues: 'write' + contents: 'read' steps: - name: 'Create Issue on Failure' env: @@ -639,6 +647,37 @@ jobs: fi } + # Resolve the milestone for the version currently being worked on in + # main, so the auto-created nightly failure issue surfaces in the + # release view (issue #3149). Source of truth is the root package.json + # `version` on main — not the dispatched ref, which may have bumped + # it. Resolved by exact title match against open milestones (not + # nearest due date). Fails soft: an unresolvable milestone logs a + # warning and returns empty, so the notification still goes out. + resolve_milestone() { + local package_json + if ! package_json="$(gh api "repos/${GH_REPO}/contents/package.json?ref=main" -H 'Accept: application/vnd.github.raw' 2>/dev/null)"; then + echo "Warning: could not read package.json from main; skipping milestone resolution" >&2 + return + fi + local current_version + current_version="$(printf '%s' "${package_json}" | jq -r '.version // empty' 2>/dev/null)" + if [[ -z "${current_version}" ]]; then + echo "Warning: package.json on main has no version field; skipping milestone resolution" >&2 + return + fi + # gh issue --milestone accepts a milestone TITLE, not a number. + # Resolve the title by exact match across ALL open milestone + # pages (issue #3149), then return it for assignment. + local milestone_title + milestone_title="$(gh api --paginate --slurp "repos/${GH_REPO}/milestones?state=open&per_page=100" --jq "[.[][] | select(.title == \"${current_version}\")] | .[0].title // empty" 2>/dev/null)" + if [[ -z "${milestone_title}" ]]; then + echo "Warning: no open milestone titled '${current_version}'; skipping milestone assignment" >&2 + return + fi + printf '%s' "${milestone_title}" + } + LABEL_ARGS=() if ensure_label "ci/cd" "e33539" "Issues with github and the workflow scripts and CI CD environment."; then LABEL_ARGS+=(--label "ci/cd") @@ -646,6 +685,12 @@ jobs: echo "Warning: continuing without ci/cd label" >&2 fi + MILESTONE_TITLE="$(resolve_milestone)" + MILESTONE_ARGS=() + if [[ -n "${MILESTONE_TITLE}" ]]; then + MILESTONE_ARGS+=(--milestone "${MILESTONE_TITLE}") + fi + retry_gh() { local attempt for attempt in 1 2 3 4; do @@ -701,11 +746,20 @@ jobs: echo "ERROR: Failed to comment on existing issue #${EXISTING_ISSUE}" >&2 exit 1 } + # Keep a long-lived failure issue on the current release milestone + # too, not just on first creation (issue #3149). + if [[ -n "${MILESTONE_TITLE}" ]]; then + retry_gh gh issue edit "${EXISTING_ISSUE}" \ + --repo "${GH_REPO}" \ + --milestone "${MILESTONE_TITLE}" || \ + echo "Warning: failed to set milestone '${MILESTONE_TITLE}' on existing issue #${EXISTING_ISSUE}" >&2 + fi else INTRODUCTION="The nightly test workflow failed. Affected jobs: ${FAILED_JOBS_TEXT}." build_body_file "${BODY_FILE}" "${INTRODUCTION}" CREATE_ARGS=(--repo "${GH_REPO}" --title "${ISSUE_TITLE}" --body-file "${BODY_FILE}") CREATE_ARGS+=("${LABEL_ARGS[@]}") + CREATE_ARGS+=("${MILESTONE_ARGS[@]}") retry_gh gh issue create "${CREATE_ARGS[@]}" || { echo "ERROR: Failed to create nightly failure issue" >&2 exit 1 diff --git a/packages/cli/run-bun-tests.ts b/packages/cli/run-bun-tests.ts index d10dd04fab..dfa85cbda7 100644 --- a/packages/cli/run-bun-tests.ts +++ b/packages/cli/run-bun-tests.ts @@ -334,6 +334,94 @@ export function parseCaseCounts(output: string): { }; } +/** + * React's "not wrapped in act(...)" warning is a fixed ~10-line block that can + * repeat dozens of times in a single React/Ink test file. A naive tail of the + * output is then entirely warning text, and the assertion failures that + * actually failed the file are unrecoverable from the log. This collapses each + * warning block to a one-line count before taking the excerpt, so the failures + * stay visible within the budget. (issue #3149) + */ +const ACT_WARNING_START = + /^An update to .* inside a test was not wrapped in act\(\.\.\.\)/; +const ACT_WARNING_END = + /Learn more at https:\/\/react\.dev\/link\/wrap-tests-with-act/; +const NEWLINE = String.fromCharCode(10); + +function collapseActWarnings(output: string): { + body: string; + elidedWarnings: number; +} { + const kept: string[] = []; + let elided = 0; + let skipping = false; + // Cap lines consumed after a warning start whose end marker never arrives + // (truncated at the source). The standard block is ~10 lines. + let skipLineCount = 0; + const MAX_WARNING_BODY_LINES = 15; + for (const line of output.split(NEWLINE)) { + if (!skipping) { + if (ACT_WARNING_START.test(line)) { + elided += 1; + skipLineCount = 1; + skipping = !ACT_WARNING_END.test(line); + } else { + kept.push(line); + } + } else { + skipLineCount++; + if (ACT_WARNING_END.test(line)) { + skipping = false; + } else if (ACT_WARNING_START.test(line)) { + elided += 1; + skipLineCount = 1; + skipping = !ACT_WARNING_END.test(line); + } else if (skipLineCount > MAX_WARNING_BODY_LINES) { + skipping = false; + kept.push(line); + } + } + } + return { body: kept.join(NEWLINE), elidedWarnings: elided }; +} + +/** + * Keeps an initial run and a final run of `text` so both the first failures + * and the trailing summary stay visible when the content exceeds the budget. + */ +function headTail(text: string, maxChars: number): string { + if (maxChars <= 0) return ''; + if (text.length <= maxChars) return text; + const marker = ` +[... output elided ...] +`; + const budget = Math.max(0, maxChars - marker.length); + const head = Math.ceil(budget / 2); + const tail = budget - head; + // Guard tail === 0: String.prototype.slice(-0) returns the whole string. + const tailSlice = tail > 0 ? text.slice(-tail) : ''; + return `${text.slice(0, head)}${marker}${tailSlice}`; +} + +/** + * Returns a bounded excerpt of a failing file's output that keeps assertion + * failures visible even when repetitive diagnostic noise would otherwise crowd + * them out of a fixed-size slice. + */ +export function failureExcerpt(output: string, maxChars: number): string { + if (output.length <= maxChars) { + return output; + } + const { body, elidedWarnings } = collapseActWarnings(output); + const banner = + elidedWarnings > 0 + ? `[${elidedWarnings} React "not wrapped in act(...)" warning block(s) elided] +` + : ''; + const excerpt = headTail(body, maxChars - banner.length); + return (banner + excerpt).slice(0, maxChars); +} + export function generateJUnit(results: readonly TestResult[]): string { const failedCount = results.filter((result) => !result.passed).length; const testCases = results @@ -349,7 +437,7 @@ export function generateJUnit(results: readonly TestResult[]): string { }s">TIMEOUT` : `${escapeXml(result.output.slice(-4000))}`; + }">${escapeXml(failureExcerpt(stripAnsi(result.output), 4000))}`; return ` ${failure}`; }) .join('\n'); @@ -414,7 +502,7 @@ async function main(): Promise { for (const result of failed) { console.error(`\n----- ${result.file} -----`); - console.error(result.output.slice(-6000)); + console.error(failureExcerpt(stripAnsi(result.output), 6000)); } const cases = results.reduce( diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index 4fd6c503a1..fbda47732c 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -30,8 +30,23 @@ export const render = ( const originalUnmount = renderResult.unmount; const originalRerender = renderResult.rerender; + const actWrappedStdin = new Proxy(renderResult.stdin, { + get(target, prop, receiver) { + if (prop === 'write') { + return (...args: Parameters) => { + act(() => { + target.write(...args); + }); + }; + } + const value = Reflect.get(target, prop, receiver); + return typeof value === 'function' ? value.bind(target) : value; + }, + }); + return { ...renderResult, + stdin: actWrappedStdin, unmount: () => { act(() => { originalUnmount(); diff --git a/packages/cli/test/run-bun-tests.test.ts b/packages/cli/test/run-bun-tests.test.ts index ef25cd239e..1d6b7b7af7 100644 --- a/packages/cli/test/run-bun-tests.test.ts +++ b/packages/cli/test/run-bun-tests.test.ts @@ -25,6 +25,7 @@ import { discoverTestFiles, escapeXml, exitCodeForRun, + failureExcerpt, isTestFile, fileTimeoutForFile, parseCaseCounts, @@ -285,3 +286,117 @@ describe('discoverTestFiles symlink safety', () => { } }); }); + +/** + * React's "not wrapped in act(...)" warning is a fixed ~10-line block. These + * tests pin the behaviour that keeps assertion failures visible in a truncated + * log instead of being crowded out by the repeated warning. (issue #3149) + */ +function actWarningBlock(component: string): string { + return [ + `An update to ${component} inside a test was not wrapped in act(...).`, + '', + 'When testing, code that causes React state updates should be wrapped into act(...):', + '', + 'act(() => {', + ' /* fire events that update state */', + '});', + '/* assert on the output */', + '', + "This ensures that you're testing the behavior the user would see in the browser. Learn more at https://react.dev/link/wrap-tests-with-act", + ].join(String.fromCharCode(10)); +} + +describe('failureExcerpt', () => { + it('returns short output unchanged', () => { + expect(failureExcerpt('short output', 100)).toBe('short output'); + }); + + it('handles empty string without throwing', () => { + expect(failureExcerpt('', 100)).toBe(''); + }); + + it('respects the budget even when it is smaller than the elision marker', () => { + const excerpt = failureExcerpt('x'.repeat(500), 10); + expect(excerpt.length).toBeLessThanOrEqual(10); + }); + + it('preserves the failing assertion that a naive tail would have dropped', () => { + const nl = String.fromCharCode(10); + const assertion = `error: expect(received).toBe(expected)${nl}`; + const assertionRepeat = assertion.repeat(3); + // Many warning blocks AFTER the assertion, large enough to push it out of + // a 600-character tail on their own. + const warnings = `${actWarningBlock('BaseSelectionList')}${nl}`.repeat(30); + const output = `${assertionRepeat}${warnings}5 fail${nl}`; + + const naiveTail = output.slice(-600); + const excerpt = failureExcerpt(output, 600); + + // The naive tail is entirely warning text and loses the assertion. + expect(naiveTail).not.toContain('expect(received)'); + // The excerpt keeps the assertion visible and counts the elided warnings. + expect(excerpt).toContain('expect(received)'); + expect(excerpt).toContain( + 'React "not wrapped in act(...)" warning block(s) elided', + ); + // The excerpt stays within the requested budget. + expect(excerpt.length).toBeLessThanOrEqual(600); + }); + + it('keeps the trailing summary when warnings are collapsed', () => { + const nl = String.fromCharCode(10); + const warnings = `${actWarningBlock('Dialog')}${nl}`.repeat(20); + const output = `${warnings}17 pass${nl}5 fail${nl}`; + const excerpt = failureExcerpt(output, 400); + expect(excerpt).toContain('5 fail'); + expect(excerpt).toContain('17 pass'); + }); + + it('uses head and tail when the remaining content still exceeds the budget', () => { + // No act() warnings, just a long output that must be truncated. + const nl = String.fromCharCode(10); + const head = `FIRST-LINE${nl}`; + const middle = 'x'.repeat(2000); + const tail = `${nl}LAST-LINE${nl}`; + const output = `${head}${middle}${tail}`; + const excerpt = failureExcerpt(output, 100); + expect(excerpt).toContain('FIRST-LINE'); + expect(excerpt).toContain('LAST-LINE'); + expect(excerpt).toContain('output elided'); + expect(excerpt.length).toBeLessThanOrEqual(100); + }); + + it('preserves head, tail, and warning count when both warnings and body are large', () => { + // Exercises the combined path: collapseActWarnings + banner + headTail. + const nl = String.fromCharCode(10); + const warnings = `${actWarningBlock('Menu')}${nl}`.repeat(5); + const middle = 'y'.repeat(2000); + const output = `HEAD${nl}${warnings}${middle}${nl}TAIL${nl}3 fail${nl}`; + const excerpt = failureExcerpt(output, 200); + expect(excerpt).toContain('warning block(s) elided'); + expect(excerpt).toContain('output elided'); + expect(excerpt).toContain('HEAD'); + expect(excerpt).toContain('3 fail'); + expect(excerpt.length).toBeLessThanOrEqual(200); + }); + + it('does not swallow assertions following a truncated (unterminated) warning block', () => { + const nl = String.fromCharCode(10); + // A warning block whose end marker never appears (truncated at the source) + // must not consume every subsequent line including the assertion. + const truncatedWarning = + 'An update to Foo inside a test was not wrapped in act(...).'; + // The standard warning block is ~10 lines; we need >15 lines of padding + // to exceed the cap and verify recovery. + const padding = Array.from( + { length: 20 }, + (_, i) => `padding line ${i}`, + ).join(nl); + const assertion = 'expect(received).toBe(expected)'; + const summary = '5 fail'; + const output = `${truncatedWarning}${nl}${padding}${nl}${assertion}${nl}${summary}${nl}`; + const excerpt = failureExcerpt(output, 600); + expect(excerpt).toContain(assertion); + }); +}); diff --git a/packages/core/src/services/shellJobWindowsSpawn.test.ts b/packages/core/src/services/shellJobWindowsSpawn.test.ts index 612fa583bb..233fa21809 100644 --- a/packages/core/src/services/shellJobWindowsSpawn.test.ts +++ b/packages/core/src/services/shellJobWindowsSpawn.test.ts @@ -23,6 +23,7 @@ import { escapePowerShellSingleQuoted, spawnWindowsBackground, } from './shellJobSpawn.js'; +import { boundedTaskkill } from './shellProcessKill.js'; /** * Timeout for probing whether a PowerShell executable (pwsh / powershell.exe) @@ -45,6 +46,15 @@ const UNREF_SLEEP_SECONDS = 30; */ const UNREF_SPAWN_TIMEOUT_MS = 25000; +/** + * Maximum time to wait for a background process to exit in runAndWait. The + * test commands are simple PowerShell one-liners that finish in a few seconds + * even on a cold Windows runner; 15s is generous for slow cold-starts while + * preventing a single hung process from consuming the entire per-file budget. + * (issue #3149) + */ +const RUN_AND_WAIT_TIMEOUT_MS = 15_000; + /** * Check whether a PID is alive using signal 0 (works on both Windows and * POSIX). Used to verify the unref contract: the background process survives @@ -229,6 +239,42 @@ describe.skipIf(!isWindows || availablePowerShellExes.length === 0)( return 'powershell.exe'; } + /** + * Await a SpawnedProcess's exit, bounded by a timeout that kills the + * child on expiry so a hung process cannot consume the entire per-file + * budget. The test commands are simple one-liners; a real exit in under + * 15s is the norm, and a timeout indicates a genuine process hang (e.g. + * ConPTY stall or interactive prompt) rather than slow cold-start. + */ + function awaitBoundedExit( + spawned: ReturnType, + timeoutMs = RUN_AND_WAIT_TIMEOUT_MS, + ): Promise<{ exitCode: number | null; signal: NodeJS.Signals | null }> { + let timer: ReturnType | undefined; + return Promise.race([ + spawned.exited, + new Promise((_, reject) => { + timer = setTimeout(() => { + // taskkill /T /F reaps the entire process tree (outer PowerShell + // wrapper and inner Start-Process child). Await its completion so + // the tree is fully terminated before rejecting (issue #3149). + void (async () => { + if (spawned.pid > 0) { + await boundedTaskkill(spawned.pid); + } + reject( + new Error( + `Background process (pid ${spawned.pid}) did not exit within ${timeoutMs}ms`, + ), + ); + })(); + }, timeoutMs); + }), + ]).finally(() => { + if (timer !== undefined) clearTimeout(timer); + }); + } + async function runAndWait( command: string, executable?: string, @@ -247,7 +293,7 @@ describe.skipIf(!isWindows || availablePowerShellExes.length === 0)( logPath, errLogPath, ); - const exitInfo = await spawned.exited; + const exitInfo = await awaitBoundedExit(spawned); const stdout = fs.existsSync(logPath) ? fs.readFileSync(logPath, 'utf8') : ''; @@ -361,7 +407,7 @@ describe.skipIf(!isWindows || availablePowerShellExes.length === 0)( errLogPath, ); expect(spawned.pid).toBeGreaterThan(0); - await spawned.exited; + await awaitBoundedExit(spawned); }); it('does not keep the spawner alive (production unref)', async () => { diff --git a/packages/providers/src/runtime/__tests__/profileApplication.lb.contextWindowTimeout.test.ts b/packages/providers/src/runtime/__tests__/profileApplication.lb.contextWindowTimeout.test.ts new file mode 100644 index 0000000000..3d6edec27c --- /dev/null +++ b/packages/providers/src/runtime/__tests__/profileApplication.lb.contextWindowTimeout.test.ts @@ -0,0 +1,172 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + * + * Behavioral test for issue #3149: a member provider whose getModels() never + * resolves must not hang load-balancer registration. The context-window lookup + * is advisory (an unresolved window degrades gracefully to the load balancer's + * configured context limit), so it is bounded by SUBPROFILE_CONTEXT_WINDOW_TIMEOUT_MS. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'bun:test'; +import type { + Profile, + LoadBalancerProfile, +} from '@vybestack/llxprt-code-settings'; +import { isResolvedSubProfile } from '../../loadBalancing/loadBalancerTypes.js'; +import { + switchActiveProviderMock, + setActiveModelMock, + updateActiveProviderBaseUrlMock, + updateActiveProviderApiKeyMock, + setActiveModelParamMock, + clearActiveModelParamMock, + getActiveModelParamsMock, + setEphemeralSettingMock, + getCliRuntimeServicesMock, + getActiveProviderOrThrowMock, + isCliStatelessProviderModeEnabledMock, + isCliRuntimeStatelessReadyMock, + createProviderKeyStorageMock, + providerManagerStub, + profileManagerStub, + wrapRegisterProviderToCaptureLB, + resetLbProfileApplicationStubs, + makeLbProfile, +} from './lbProfileApplicationTestSetup.js'; + +void vi.mock('../runtimeSettings.js', () => ({ + switchActiveProvider: switchActiveProviderMock, + setActiveModel: setActiveModelMock, + updateActiveProviderBaseUrl: updateActiveProviderBaseUrlMock, + updateActiveProviderApiKey: updateActiveProviderApiKeyMock, + setActiveModelParam: setActiveModelParamMock, + clearActiveModelParam: clearActiveModelParamMock, + getActiveModelParams: getActiveModelParamsMock, + setEphemeralSetting: setEphemeralSettingMock, + createProviderKeyStorage: createProviderKeyStorageMock, + getCliRuntimeServices: getCliRuntimeServicesMock, + getActiveProviderOrThrow: getActiveProviderOrThrowMock, + isCliStatelessProviderModeEnabled: isCliStatelessProviderModeEnabledMock, + isCliRuntimeStatelessReady: isCliRuntimeStatelessReadyMock, +})); + +const { applyProfileWithGuards } = await import('../profileApplication.js'); + +describe('Load balancer sub-profile context-window timeout (issue #3149)', () => { + beforeEach(() => { + resetLbProfileApplicationStubs(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('completes registration when a member provider getModels() never resolves', async () => { + // A provider whose getModels() hangs forever — this is the CI failure mode: + // a member endpoint that never responds (e.g. a slow or blocked network + // egress). Before the fix this blocked registration indefinitely. + providerManagerStub.registerProvider({ + name: 'hangprovider', + getDefaultModel: () => 'hang-model', + getModels: () => new Promise(() => {}), + }); + + profileManagerStub.loadProfile = vi.fn( + async (): Promise => ({ + version: 1, + provider: 'hangprovider', + model: 'hang-model', + modelParams: {}, + ephemeralSettings: { 'auth-key': 'k' }, + }), + ); + + const lbProfile: LoadBalancerProfile = makeLbProfile(['hangmember']); + const { getLBProvider } = wrapRegisterProviderToCaptureLB(); + + const start = Date.now(); + await applyProfileWithGuards(lbProfile, { profileName: 'myLB' }); + const elapsed = Date.now() - start; + + // Registration completed (this line being reached at all proves it did not + // hang) and the load balancer was registered despite the unreachable member. + expect(getLBProvider()).not.toBeNull(); + // The advisory context-window lookup is bounded at 3s; registration + // finishes near that bound plus overhead, never hanging. + expect(elapsed).toBeLessThan(10_000); + }, 20_000); + + it('resolves N hanging members concurrently, not sequentially', async () => { + // Three providers whose getModels() all hang. Sequential resolution would + // take 3 × timeout; concurrent (Promise.all) takes ~1 × timeout. + for (const name of ['p1', 'p2', 'p3']) { + providerManagerStub.registerProvider({ + name, + getDefaultModel: () => `${name}-model`, + getModels: () => new Promise(() => {}), + }); + } + + let callCount = 0; + profileManagerStub.loadProfile = vi.fn(async (name: string) => { + callCount++; + return { + version: 1, + provider: name, + model: `${name}-model`, + modelParams: {}, + ephemeralSettings: { 'auth-key': 'k' }, + } as Profile; + }); + + const lbProfile: LoadBalancerProfile = makeLbProfile(['p1', 'p2', 'p3']); + const { getLBProvider } = wrapRegisterProviderToCaptureLB(); + + const start = Date.now(); + await applyProfileWithGuards(lbProfile, { profileName: 'concurrentLB' }); + const elapsed = Date.now() - start; + + expect(getLBProvider()).not.toBeNull(); + expect(callCount).toBe(3); + // If sequential, elapsed would be ~9s (3 × 3s). Concurrent is ~3s. + // Use 8s (not tighter) to avoid flaking on slow Windows CI runners. + expect(elapsed).toBeLessThan(8_000); + }, 20_000); + + it('degrades a member to an undefined context window when getModels() rejects', async () => { + providerManagerStub.registerProvider({ + name: 'errprovider', + getDefaultModel: () => 'err-model', + getModels: async () => { + throw new Error('boom'); + }, + }); + + profileManagerStub.loadProfile = vi.fn( + async (): Promise => ({ + version: 1, + provider: 'errprovider', + model: 'err-model', + modelParams: {}, + ephemeralSettings: { 'auth-key': 'k' }, + }), + ); + + const lbProfile: LoadBalancerProfile = makeLbProfile(['errmember']); + const { getLBProvider } = wrapRegisterProviderToCaptureLB(); + + await applyProfileWithGuards(lbProfile, { profileName: 'myLB' }); + + const lbProvider = getLBProvider(); + expect(lbProvider).not.toBeNull(); + // Verify the member degraded gracefully: contextWindow should be undefined + // (not a numeric fallback), proving the rejection was handled, not hidden. + const subProfile = lbProvider!.selectNextSubProfile(); + expect(isResolvedSubProfile(subProfile)).toBe(true); + if (isResolvedSubProfile(subProfile)) { + expect(subProfile.contextWindow).toBeUndefined(); + } + }); +}); diff --git a/packages/providers/src/runtime/profile-application/loadBalancerProfile.ts b/packages/providers/src/runtime/profile-application/loadBalancerProfile.ts index 075a181775..51c2d9df38 100644 --- a/packages/providers/src/runtime/profile-application/loadBalancerProfile.ts +++ b/packages/providers/src/runtime/profile-application/loadBalancerProfile.ts @@ -133,6 +133,37 @@ async function resolveSubProfileAuthToken( } } +/** + * Maximum time to spend resolving a member's context window from its provider + * during load-balancer registration. The context window is advisory — an + * unresolved window degrades gracefully to the load-balancer's configured + * context limit — so a slow or unreachable member endpoint must never block + * registration or subagent launch (issue #3149: the LB orchestrator tests hung + * ~20s per member when getModels() hit an endpoint that never responded). + */ +const SUBPROFILE_CONTEXT_WINDOW_TIMEOUT_MS = 3_000; + +/** + * Resolves to the awaited value, or `undefined` if the timeout elapses first. + * Bounds best-effort provider lookups whose underlying fetch exposes no abort + * hook. The losing promise keeps a handler attached via Promise.race, so a + * late rejection is not reported as unhandled. + */ +function raceWithTimeout( + promise: Promise, + timeoutMs: number, +): Promise { + let timer: ReturnType | undefined; + const timeout = new Promise((resolve) => { + timer = setTimeout(() => resolve(undefined), timeoutMs); + }); + return Promise.race([promise, timeout]).finally(() => { + if (timer !== undefined) { + clearTimeout(timer); + } + }); +} + async function resolveSubProfileContextWindow( providerName: string, modelId: string, @@ -144,7 +175,17 @@ async function resolveSubProfileContextWindow( } try { - const models = await provider.getModels(); + const models = await raceWithTimeout( + provider.getModels(), + SUBPROFILE_CONTEXT_WINDOW_TIMEOUT_MS, + ); + if (models === undefined) { + deps.lbLogger.warn( + () => + `Timed out resolving context window for sub-profile model ${providerName}/${modelId} after ${SUBPROFILE_CONTEXT_WINDOW_TIMEOUT_MS}ms`, + ); + return undefined; + } const model = models.find((candidate) => candidate.id === modelId); return model?.contextWindow; } catch (error) { @@ -227,17 +268,21 @@ async function resolveLoadBalancerSubProfiles( profileInput: LoadBalancerProfile, deps: LoadBalancerResolutionDeps, ): Promise { - const resolvedSubProfiles: ResolvedSubProfile[] = []; - for (const profileName of profileInput.profiles) { - deps.lbLogger.debug(() => `Loading sub-profile: ${profileName}`); - const resolved = await resolveLoadBalancerSubProfile(profileName, deps); - resolvedSubProfiles.push(resolved); - deps.lbLogger.debug( - () => - `Resolved sub-profile ${profileName}: provider=${resolved.providerName}, model=${resolved.model}`, - ); - } - return resolvedSubProfiles; + // Members are independent of each other, so resolve them concurrently. This + // keeps registration latency bounded by the slowest member (each bounded by + // SUBPROFILE_CONTEXT_WINDOW_TIMEOUT_MS) rather than the sum of all of them, + // and preserves the input order in the resolved array (Promise.all). + return Promise.all( + profileInput.profiles.map(async (profileName) => { + deps.lbLogger.debug(() => `Loading sub-profile: ${profileName}`); + const resolved = await resolveLoadBalancerSubProfile(profileName, deps); + deps.lbLogger.debug( + () => + `Resolved sub-profile ${profileName}: provider=${resolved.providerName}, model=${resolved.model}`, + ); + return resolved; + }), + ); } export async function maybeRegisterLoadBalancerProfile( diff --git a/scripts/tests/nightly-bun-native-smoke.test.ts b/scripts/tests/nightly-bun-native-smoke.test.ts index fc5fca4983..798f281bc4 100644 --- a/scripts/tests/nightly-bun-native-smoke.test.ts +++ b/scripts/tests/nightly-bun-native-smoke.test.ts @@ -161,7 +161,10 @@ describe('nightly Windows Bun native-module smoke', () => { ? notifyJob?.needs : [notifyJob?.needs].filter((n): n is string => typeof n === 'string'); expect(needs).toContain('windows_bun_native_smoke'); - expect(notifyJob?.permissions).toEqual({ issues: 'write' }); + expect(notifyJob?.permissions).toEqual({ + issues: 'write', + contents: 'read', + }); expect(notifyStep?.env?.['GH_TOKEN']).toBe('${{ secrets.GITHUB_TOKEN }}'); expect(notifyStep?.env?.['GH_REPO']).toBe('${{ github.repository }}'); expect(notifyStep?.env?.['WINDOWS_BUN_NATIVE_SMOKE_RESULT']).toBe( diff --git a/scripts/tests/nightly-notifier-repository.test.ts b/scripts/tests/nightly-notifier-repository.test.ts index 1747d1bba1..4d29424d30 100644 --- a/scripts/tests/nightly-notifier-repository.test.ts +++ b/scripts/tests/nightly-notifier-repository.test.ts @@ -365,6 +365,9 @@ describe('nightly failure notifier repository targeting', () => { ), ).toBe(false); - expect(notifyFailureJob?.permissions).toEqual({ issues: 'write' }); + expect(notifyFailureJob?.permissions).toEqual({ + issues: 'write', + contents: 'read', + }); }); }); diff --git a/scripts/tests/release-process-b.test.ts b/scripts/tests/release-process-b.test.ts index 430d9874d4..1f54d67325 100644 --- a/scripts/tests/release-process-b.test.ts +++ b/scripts/tests/release-process-b.test.ts @@ -439,7 +439,7 @@ describe('.github/workflows/nightly.yml', () => { ); expect( asOptionalRecord(nightlyParsed?.concurrency)?.['cancel-in-progress'], - ).toBe(false); + ).toBe(true); }); it('runs lint:agents-api-surface before npm run test in the Windows CI job', () => {