From 4b4987a2a23799c56cb16e69786a8a2686d4d757 Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Mon, 21 Sep 2026 16:08:10 -0700 Subject: [PATCH 1/6] feat(launch): install agent_requirements.txt when core starts with --enable-agent Core is gaining an --enable-agent flag and an agent_requirements.txt beside main.py, copying its own --enable-manager pattern: when the flag is set and the package is missing, core logs the pip install command, turns the flag off and keeps starting. Desktop already installs manager_requirements.txt at every point that can change an install; the launch is where the agent's file has to be installed, because nothing before it knows the flag is in play. The decision keys on the FINAL launch args, not on the PostHog grant. The flag reaches the args either from the user's own launch args or from a beta grant, and a core whose schema does not know it has already had it filtered out, so the assembled args are the only place that knows the agent is really starting. The install runs after that assembly and before the spawn, next to the manager config reconcile, and only for an install whose Desktop-managed Python environment the shared uv helper can drive - the same environment test the manager requirement sites make. It is deliberately unconditional per launch: no stamp, no hash, no new field on the installation record. uv audits an already-satisfied requirements file in about 10 ms, which is not worth the state. A failure never blocks the launch. It is reported in the launch output and the flag stays in the args, leaving core to print its own hint and disable the agent itself. Cancelling during the install cancels the launch, like every other pre-spawn step. Co-Authored-By: Claude Opus 5 --- locales/en.json | 1 + locales/zh.json | 1 + src/main/lib/agentRequirementsLaunch.test.ts | 205 +++++++++++++++++ src/main/lib/agentRequirementsLaunch.ts | 83 +++++++ .../lib/ipc/sessionActions/launch.test.ts | 214 ++++++++++++++++++ src/main/lib/ipc/sessionActions/launch.ts | 23 ++ src/main/lib/launchPhases.ts | 11 +- 7 files changed, 534 insertions(+), 4 deletions(-) create mode 100644 src/main/lib/agentRequirementsLaunch.test.ts create mode 100644 src/main/lib/agentRequirementsLaunch.ts diff --git a/locales/en.json b/locales/en.json index feda5029d..5cb177b87 100644 --- a/locales/en.json +++ b/locales/en.json @@ -481,6 +481,7 @@ "launchStart": "Starting ComfyUI…", "repair": "Repairing installation…", "torchRepair": "Restoring GPU PyTorch…", + "agentRequirements": "Installing agent requirements…", "securityScan": "Running security scan…", "mountLibraries": "Loading libraries…", "gpu": "Initializing GPU…", diff --git a/locales/zh.json b/locales/zh.json index 14c114ca7..ab96a2412 100644 --- a/locales/zh.json +++ b/locales/zh.json @@ -481,6 +481,7 @@ "launchStart": "正在启动 ComfyUI…", "repair": "正在修复安装…", "torchRepair": "正在恢复 GPU PyTorch…", + "agentRequirements": "正在安装智能体依赖…", "securityScan": "正在进行安全扫描…", "mountLibraries": "正在加载库…", "gpu": "正在初始化 GPU…", diff --git a/src/main/lib/agentRequirementsLaunch.test.ts b/src/main/lib/agentRequirementsLaunch.test.ts new file mode 100644 index 000000000..1d84ddeb2 --- /dev/null +++ b/src/main/lib/agentRequirementsLaunch.test.ts @@ -0,0 +1,205 @@ +import fs from 'fs' +import os from 'os' +import path from 'path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mirrors = { pypiMirror: 'https://mirror.example/simple/', useChineseMirrors: false } + +vi.mock('../settings', () => ({ + getMirrorConfig: () => mirrors +})) + +vi.mock('./pip', () => ({ + installFilteredRequirementsDetailed: vi.fn(async () => ({ code: 0, output: '' })) +})) + +import { installAgentRequirements, planAgentRequirementsInstall } from './agentRequirementsLaunch' +import { installFilteredRequirementsDetailed } from './pip' +import { getUvPath, getVenvPythonPath, getLegacyVenvUvPath } from './pythonEnv' +import type { InstallationRecord } from '../installations' + +const mockInstall = vi.mocked(installFilteredRequirementsDetailed) + +let installDir = '' + +/** Create a file (and its parents) the way the real layout has it on disk. */ +function touch(target: string): void { + fs.mkdirSync(path.dirname(target), { recursive: true }) + fs.writeFileSync(target, '') +} + +/** A standalone install: `standalone-env` uv + the managed `ComfyUI/.venv`. */ +function managedInstall(): InstallationRecord { + touch(getUvPath(installDir)) + touch(getVenvPythonPath(installDir)) + return { installPath: installDir } as unknown as InstallationRecord +} + +function writeAgentRequirements(): string { + const reqPath = path.join(installDir, 'ComfyUI', 'agent_requirements.txt') + fs.mkdirSync(path.dirname(reqPath), { recursive: true }) + fs.writeFileSync(reqPath, 'comfyui-agent==1.0.0\n') + return reqPath +} + +describe('planAgentRequirementsInstall', () => { + beforeEach(() => { + vi.clearAllMocks() + installDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-reqs-')) + fs.mkdirSync(path.join(installDir, 'ComfyUI'), { recursive: true }) + }) + + afterEach(() => { + fs.rmSync(installDir, { recursive: true, force: true }) + }) + + it('plans the install when the user typed the flag by hand', () => { + const reqPath = writeAgentRequirements() + const inst = managedInstall() + + expect( + planAgentRequirementsInstall(inst, ['-s', 'main.py', '--enable-agent', '--listen']) + ).toEqual({ + reqPath, + uvPath: getUvPath(installDir), + pythonPath: getVenvPythonPath(installDir), + installPath: installDir + }) + }) + + it('plans the install when a beta grant added the flag', () => { + // A grant reaches the final args ahead of the user's own, which is the only + // difference from the hand-typed case - the args are all this reads. + const reqPath = writeAgentRequirements() + const inst = managedInstall() + + const plan = planAgentRequirementsInstall(inst, [ + '-s', + 'main.py', + '--feature-flag', + 'show_signin_button=true', + '--enable-agent', + '--listen' + ]) + + expect(plan?.reqPath).toBe(reqPath) + }) + + it('plans nothing when the flag is absent', () => { + writeAgentRequirements() + const inst = managedInstall() + + expect(planAgentRequirementsInstall(inst, ['-s', 'main.py', '--listen'])).toBeNull() + }) + + it('plans nothing when core ships no agent requirements file', () => { + const inst = managedInstall() + + expect(planAgentRequirementsInstall(inst, ['--enable-agent'])).toBeNull() + }) + + it('plans nothing for an install with no Desktop-managed Python environment', () => { + // Portable, git and build installs: the requirements file may well be there, + // but there is no uv/venv pair to install it into. + writeAgentRequirements() + const inst = { installPath: installDir } as unknown as InstallationRecord + + expect(planAgentRequirementsInstall(inst, ['--enable-agent'])).toBeNull() + }) + + it('plans nothing when uv is missing from an otherwise managed install', () => { + writeAgentRequirements() + touch(getVenvPythonPath(installDir)) + const inst = { installPath: installDir } as unknown as InstallationRecord + + expect(planAgentRequirementsInstall(inst, ['--enable-agent'])).toBeNull() + }) + + it('targets the legacy venv for an adopted install', () => { + const reqPath = writeAgentRequirements() + const adoptedBaseDir = path.join(installDir, 'legacy') + const adoptedPythonPath = path.join(adoptedBaseDir, '.venv', 'python-for-test') + touch(getLegacyVenvUvPath(adoptedBaseDir)) + touch(adoptedPythonPath) + const inst = { + installPath: installDir, + adopted: true, + adoptedBaseDir, + adoptedPythonPath + } as unknown as InstallationRecord + + expect(planAgentRequirementsInstall(inst, ['--enable-agent'])).toEqual({ + reqPath, + uvPath: getLegacyVenvUvPath(adoptedBaseDir), + pythonPath: adoptedPythonPath, + installPath: installDir + }) + }) +}) + +describe('installAgentRequirements', () => { + const plan = { + reqPath: '/inst/ComfyUI/agent_requirements.txt', + uvPath: '/inst/standalone-env/bin/uv', + pythonPath: '/inst/ComfyUI/.venv/bin/python3', + installPath: '/inst' + } + + beforeEach(() => { + vi.clearAllMocks() + }) + + it('installs the planned file through the shared uv helper', async () => { + const sendOutput = vi.fn() + const signal = new AbortController().signal + + await installAgentRequirements(plan, sendOutput, signal) + + expect(mockInstall).toHaveBeenCalledWith( + plan.reqPath, + plan.uvPath, + plan.pythonPath, + plan.installPath, + '.launch-agent-reqs.txt', + sendOutput, + signal, + mirrors + ) + expect(sendOutput.mock.calls.join('')).toContain('Installing agent requirements') + }) + + it('reports a failed install and resolves so the launch continues', async () => { + mockInstall.mockResolvedValueOnce({ code: 2, output: 'No solution found\n' }) + const sendOutput = vi.fn() + + await expect(installAgentRequirements(plan, sendOutput)).resolves.toBeUndefined() + + const reported = sendOutput.mock.calls.join('') + expect(reported).toContain('exited with code 2') + expect(reported).toContain('No solution found') + }) + + it('reports a thrown install and resolves so the launch continues', async () => { + mockInstall.mockRejectedValueOnce(new Error('EACCES: permission denied')) + const sendOutput = vi.fn() + + await expect(installAgentRequirements(plan, sendOutput)).resolves.toBeUndefined() + + expect(sendOutput.mock.calls.join('')).toContain('EACCES: permission denied') + }) + + it('stays quiet about the exit code when the launch was cancelled mid-install', async () => { + // Cancelling kills uv, so its non-zero exit IS the cancellation; reporting it + // would put a spurious failure in the output of a launch the user stopped. + const abort = new AbortController() + mockInstall.mockImplementationOnce(async () => { + abort.abort() + return { code: 1, output: '' } + }) + const sendOutput = vi.fn() + + await installAgentRequirements(plan, sendOutput, abort.signal) + + expect(sendOutput.mock.calls.join('')).not.toContain('exited with code') + }) +}) diff --git a/src/main/lib/agentRequirementsLaunch.ts b/src/main/lib/agentRequirementsLaunch.ts new file mode 100644 index 000000000..031726462 --- /dev/null +++ b/src/main/lib/agentRequirementsLaunch.ts @@ -0,0 +1,83 @@ +import fs from 'fs' +import path from 'path' +import * as settings from '../settings' +import { installFilteredRequirementsDetailed } from './pip' +import { withOutputTail } from './logged-process' +import { getActivePythonPath, getActiveUvPath } from './pythonEnv' +import type { InstallationRecord } from '../installations' + +/** The Core flag that starts ComfyUI with the agent enabled. */ +const ENABLE_AGENT_ARG = '--enable-agent' + +/** Requirements file Core ships beside `main.py` for the agent. */ +const AGENT_REQUIREMENTS = 'agent_requirements.txt' + +export interface AgentRequirementsInstall { + reqPath: string + uvPath: string + pythonPath: string + installPath: string +} + +/** + * Decide whether this launch installs the agent's Python requirements, and + * resolve everything the install needs. + * + * `args` must be the FINAL spawn args. `--enable-agent` reaches them either + * from the user's own launch args or from a Core beta grant, and a core whose + * schema does not know the flag has already had it filtered out, so the args + * are the one place that knows whether the agent is really starting. + * + * The environment test is the one every `manager_requirements.txt` site makes, + * which is what confines this to the Desktop-managed installs (standalone and + * adopted). Portable, git and build installs drive a Python environment this + * helper has no uv binary for; Core's own `pip install -r` hint covers them. + */ +export function planAgentRequirementsInstall( + installation: InstallationRecord, + args: readonly string[] +): AgentRequirementsInstall | null { + if (!args.includes(ENABLE_AGENT_ARG)) return null + const reqPath = path.join(installation.installPath, 'ComfyUI', AGENT_REQUIREMENTS) + if (!fs.existsSync(reqPath)) return null + const uvPath = getActiveUvPath(installation) + const pythonPath = getActivePythonPath(installation) + if (!pythonPath || !fs.existsSync(uvPath)) return null + return { reqPath, uvPath, pythonPath, installPath: installation.installPath } +} + +/** + * Install the planned requirements, streaming uv's output into the launch. + * + * Never throws and reports nothing back: a failure here must not stop the + * launch. ComfyUI still starts with the flag, prints its own install hint and + * disables the agent itself, which beats refusing to start. + */ +export async function installAgentRequirements( + plan: AgentRequirementsInstall, + sendOutput: (text: string) => void, + signal?: AbortSignal +): Promise { + sendOutput('\nInstalling agent requirements…\n') + try { + const result = await installFilteredRequirementsDetailed( + plan.reqPath, + plan.uvPath, + plan.pythonPath, + plan.installPath, + '.launch-agent-reqs.txt', + sendOutput, + signal, + settings.getMirrorConfig() + ) + // A cancelled launch kills uv mid-install; that non-zero exit is the + // cancellation, not a failure worth showing. + if (result.code !== 0 && !signal?.aborted) { + sendOutput( + `\n${withOutputTail(`⚠ agent requirements install exited with code ${result.code}`, result.output)}\n` + ) + } + } catch (err) { + sendOutput(`⚠ ${AGENT_REQUIREMENTS} failed: ${(err as Error).message}\n`) + } +} diff --git a/src/main/lib/ipc/sessionActions/launch.test.ts b/src/main/lib/ipc/sessionActions/launch.test.ts index a1b20285c..2ce4bfd34 100644 --- a/src/main/lib/ipc/sessionActions/launch.test.ts +++ b/src/main/lib/ipc/sessionActions/launch.test.ts @@ -116,6 +116,26 @@ vi.mock('../../coreBetaGrants', async (importOriginal) => { return { ...actual, getCoreBetaGrantsAsync: async () => launchHarness.grants } }) +/** Stands in for the uv subprocess the agent-requirements step drives, so a launch + * under test installs nothing and a test can choose the outcome. */ +const pipHarness = vi.hoisted(() => ({ + calls: [] as unknown[][], + result: { code: 0, output: '' }, + duringInstall: null as null | (() => void) +})) + +vi.mock('../../pip', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + installFilteredRequirementsDetailed: async (...args: unknown[]) => { + pipHarness.calls.push(args) + pipHarness.duringInstall?.() + return pipHarness.result + } + } +}) + vi.mock('../../hardwareTap', async (importOriginal) => { const actual = await importOriginal() return { @@ -162,6 +182,9 @@ import type * as SharedModule from '../shared' import type * as ComfyArgsModule from '../../comfy-args' import type * as CoreBetaGrantsModule from '../../coreBetaGrants' import type * as HardwareTapModule from '../../hardwareTap' +import type * as PipModule from '../../pip' +import { getUvPath, getVenvPythonPath } from '../../pythonEnv' +import { getLogDir } from '../../logRotation' const installOf = (sourceId: string) => ({ sourceId }) as InstallationRecord @@ -1477,3 +1500,194 @@ describe('emitCoreBetaTelemetry', () => { ]) }) }) + +describe('agent requirements at launch', () => { + const AGENT_GRANT: CoreBetaGrant = { arg: '--enable-agent', minCoreVersion: '0.3.80' } + let installDir = '' + let sent: string[] = [] + let spawnArgs: string[] = [] + let spawned = 0 + + /** Create a file (and its parents) the way the real install layout has it. */ + const touch = (target: string): void => { + fs.mkdirSync(path.dirname(target), { recursive: true }) + fs.writeFileSync(target, '') + } + + const agentReqPath = (): string => path.join(installDir, 'ComfyUI', 'agent_requirements.txt') + + const harnessInstall = (): InstallationRecord => + ({ + id: 'agent-reqs-inst', + name: 'Agent Harness', + sourceId: 'harness-source', + installPath: installDir, + version: '0.3.81', + comfyVersion: { + commit: '61e5e3b5a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4', + baseTag: 'v0.3.81', + commitsAhead: 0, + baseTagVerified: true + } + }) as unknown as InstallationRecord + + const ctxFor = (installationId: string): ActionContext => ({ + event: { + sender: { + isDestroyed: () => false, + send: (_channel: string, payload: { text?: string }) => { + if (typeof payload?.text === 'string') sent.push(payload.text) + } + } + } as unknown as Electron.IpcMainInvokeEvent, + installationId, + inst: harnessInstall(), + actionData: {} + }) + + beforeEach(() => { + installDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-reqs-launch-')) + fs.mkdirSync(path.join(installDir, 'ComfyUI'), { recursive: true }) + // The managed pair the shared uv helper targets, plus the file Core ships. + touch(getUvPath(installDir)) + touch(getVenvPythonPath(installDir)) + fs.writeFileSync(agentReqPath(), 'comfyui-agent==1.0.0\n') + sent = [] + spawnArgs = [] + spawned = 0 + pipHarness.calls = [] + pipHarness.result = { code: 0, output: '' } + pipHarness.duringInstall = null + launchHarness.schemaThrows = false + launchHarness.registryThrows = false + launchHarness.betaEnabled = true + launchHarness.betaEnabledThrows = false + launchHarness.schemaNames = ['enable-agent', 'listen', 'feature-flag'] + launchHarness.grants = [] + launchHarness.duringResourceAcquire = null + launchHarness.spawn = (_cmd: unknown, args: unknown) => { + spawned += 1 + spawnArgs = args as string[] + const proc = new EventEmitter() as FakeChild + proc.stdout = new EventEmitter() + proc.stderr = new EventEmitter() + proc.pid = 4243 + proc.killed = false + proc.kill = () => true + return proc + } + launchHarness.launchCommand = { + cmd: process.execPath, + args: ['-s', path.join(installDir, 'ComfyUI', 'main.py'), '--enable-agent', '--listen'], + cwd: installDir, + skipPortWait: true + } + vi.spyOn(telemetry, 'emit').mockImplementation((() => {}) as unknown as typeof telemetry.emit) + }) + + afterEach(async () => { + // The launch log stream opens asynchronously after the spawn: removing the + // install dir first fails that open with an ENOENT nothing is listening for. + // The file appears exactly when the open lands, so waiting on it is the + // teardown's own gate rather than a guess at how long the open takes. + if (spawned > 0) { + const logFile = path.join(getLogDir(installDir), 'comfyui.log') + await vi.waitFor(() => expect(fs.existsSync(logFile)).toBe(true)) + } + vi.restoreAllMocks() + fs.rmSync(installDir, { recursive: true, force: true }) + }) + + it('installs the requirements when the user typed the flag by hand', async () => { + const res = await handleLaunch(ctxFor('agent-reqs-hand-typed')) + + expect(res.ok).toBe(true) + expect(pipHarness.calls).toHaveLength(1) + expect(pipHarness.calls[0]![0]).toBe(agentReqPath()) + expect(pipHarness.calls[0]![1]).toBe(getUvPath(installDir)) + expect(pipHarness.calls[0]![2]).toBe(getVenvPythonPath(installDir)) + expect(spawnArgs).toContain('--enable-agent') + }) + + it('installs the requirements when a beta grant added the flag', async () => { + // The grant is the only source of the flag here: the install's own args have none. + launchHarness.grants = [AGENT_GRANT] + launchHarness.launchCommand = { + cmd: process.execPath, + args: ['-s', path.join(installDir, 'ComfyUI', 'main.py'), '--listen'], + cwd: installDir, + skipPortWait: true + } + + const res = await handleLaunch(ctxFor('agent-reqs-granted')) + + expect(res.ok).toBe(true) + expect(pipHarness.calls).toHaveLength(1) + expect(spawnArgs).toContain('--enable-agent') + }) + + it('installs nothing when the flag never reaches the final args', async () => { + launchHarness.launchCommand = { + cmd: process.execPath, + args: ['-s', path.join(installDir, 'ComfyUI', 'main.py'), '--listen'], + cwd: installDir, + skipPortWait: true + } + + const res = await handleLaunch(ctxFor('agent-reqs-flag-absent')) + + expect(res.ok).toBe(true) + expect(pipHarness.calls).toEqual([]) + }) + + it('installs nothing when the running core cannot parse the flag', async () => { + // A core predating `--enable-agent` has it filtered out of the final args, + // so there is nothing to install for and nothing reaches the spawn either. + launchHarness.schemaNames = ['listen', 'feature-flag'] + + const res = await handleLaunch(ctxFor('agent-reqs-unsupported-core')) + + expect(res.ok).toBe(true) + expect(pipHarness.calls).toEqual([]) + expect(spawnArgs).not.toContain('--enable-agent') + }) + + it('installs nothing when core ships no agent requirements file', async () => { + fs.rmSync(agentReqPath()) + + const res = await handleLaunch(ctxFor('agent-reqs-file-absent')) + + expect(res.ok).toBe(true) + expect(pipHarness.calls).toEqual([]) + expect(spawnArgs).toContain('--enable-agent') + }) + + it('installs nothing for an install with no Desktop-managed Python environment', async () => { + fs.rmSync(getUvPath(installDir)) + + const res = await handleLaunch(ctxFor('agent-reqs-unmanaged-env')) + + expect(res.ok).toBe(true) + expect(pipHarness.calls).toEqual([]) + expect(spawnArgs).toContain('--enable-agent') + }) + + it('launches with the flag still set when the install fails', async () => { + pipHarness.result = { code: 1, output: 'No solution found\n' } + + const res = await handleLaunch(ctxFor('agent-reqs-install-failed')) + + expect(res.ok).toBe(true) + expect(spawnArgs).toContain('--enable-agent') + expect(sent.join('')).toContain('exited with code 1') + }) + + it('cancels the launch without spawning when it is aborted mid-install', async () => { + pipHarness.duringInstall = () => _operationAborts.get('agent-reqs-cancelled')?.abort() + + const res = await handleLaunch(ctxFor('agent-reqs-cancelled')) + + expect(res).toEqual({ ok: false, cancelled: true }) + expect(spawned).toBe(0) + }) +}) diff --git a/src/main/lib/ipc/sessionActions/launch.ts b/src/main/lib/ipc/sessionActions/launch.ts index 164dda849..19dfba7e2 100644 --- a/src/main/lib/ipc/sessionActions/launch.ts +++ b/src/main/lib/ipc/sessionActions/launch.ts @@ -90,6 +90,10 @@ import { } from '../../bootPhaseBuffer' import { appendLog } from '../../logsBroadcast' import { reconcileManagerConfigForLaunch } from '../../managerConfigLaunch' +import { + installAgentRequirements, + planAgentRequirementsInstall +} from '../../agentRequirementsLaunch' import { recoverInterruptedComfyOp } from '../../opMarker' import { waitLaunchSpawnHold } from '../../e2eOverrides' import { migrateEnvLayout } from '../../../sources/standalone/install' @@ -1015,6 +1019,25 @@ async function runLaunch( return { ok: false, message: i18n.t('errors.managerConfigWriteFailed') } } + // The args are final here, so this is the first point that knows the agent is + // actually starting - whether the user typed the flag or a beta grant added + // it, and whether the running core can parse it at all. The package is tens of + // megabytes, so it gets its own launch step; a failure is reported in the + // launch output and the flag is kept, leaving core to print its install hint + // and disable the agent itself. + const agentRequirements = planAgentRequirementsInstall(inst, launchCmd.args ?? []) + if (agentRequirements) { + preLaunchPhases.push('agentRequirements') + await armLaunchTracker() + sendProgress('agentRequirements', { percent: -1, status: '' }) + await installAgentRequirements( + agentRequirements, + makeSendOutput(sender, installationId), + abort.signal + ) + if (abort.signal.aborted) return { ok: false, cancelled: true } + } + const { preLaunchExtras, manageModelFolders, modelDirsForLaunch, modelSyncOptions } = applyStorageLaunchArgs(inst, installationId, launchCmd) diff --git a/src/main/lib/launchPhases.ts b/src/main/lib/launchPhases.ts index ca03c2cb4..dfcbb0f6e 100644 --- a/src/main/lib/launchPhases.ts +++ b/src/main/lib/launchPhases.ts @@ -89,14 +89,17 @@ export const DEFAULT_LAUNCH_PHASES: readonly LaunchPhaseDef[] = [ * boot milestone fires. Weights add on top of the base 1.0; the renderer * normalizes, so injection just shrinks every slot proportionally. * - * - `repair` interrupted-op source rollback was performed - * - `torchRepair` GPU PyTorch was restored after the v1.13.0 `--upgrade` bug + * - `repair` interrupted-op source rollback was performed + * - `torchRepair` GPU PyTorch was restored after the v1.13.0 `--upgrade` bug + * - `agentRequirements` the agent's Python packages were installed for a + * launch that starts Core with `--enable-agent` */ -export type PreLaunchPhase = 'repair' | 'torchRepair' +export type PreLaunchPhase = 'repair' | 'torchRepair' | 'agentRequirements' const PRE_LAUNCH_PHASES: Record = { repair: { phase: 'repair', match: NEVER, weight: 0.1, streaming: true }, - torchRepair: { phase: 'torchRepair', match: NEVER, weight: 0.1, streaming: true } + torchRepair: { phase: 'torchRepair', match: NEVER, weight: 0.1, streaming: true }, + agentRequirements: { phase: 'agentRequirements', match: NEVER, weight: 0.1, streaming: true } } /** Starter-template model download, shown as the LAST launch step. Synthetic + From 77761dcca5dee50b44c4ebcdb1797bb571567982 Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Mon, 21 Sep 2026 16:50:42 -0700 Subject: [PATCH 2/6] fix(launch): bound the agent requirements install and publish its step Two defects from a two-model review of the previous commit. The install awaited uv in front of the spawn with no deadline and no fail-open, so a stalled or very slow install stopped ComfyUI starting at all. That inverts core's own contract: core is built to start without these packages, log its install hint and disable the agent. It also went further than the user asked, because the flag can arrive from a beta grant rather than from the user's own launch args, and there is no per-launch skip. uv now runs under a controller this module owns, with a ceiling past which uv is killed and reaped and the launch proceeds with the flag still set. The launch's own signal is untouched, so a timeout continues the launch while a cancel still cancels it. The ceiling is a total, not an idle bound: uv streams nothing between "Downloading" and "Downloaded", so a slow transfer cannot be told from a stall. The neighbouring args-schema probe bounds and fails open the same way. The progress step was also lost whenever a torch repair ran first. That path arms the tracker, which freezes its phase list, so a later push never reached the steps payload and the renderer dropped the phase (it ignores progress for a phase it was never told about), leaving "Restoring GPU PyTorch" on screen for the whole install. The tracker now takes a phase discovered after arming, inserting it after the active one so the bar cannot regress and re-publishing the steps payload. The agent phase is no longer a PRE_LAUNCH_PHASES entry, which is what made the ordering load-bearing. Also corrects two comments the review found inaccurate, and silences the failure warning on a cancelled launch, matching the exit-code branch. Co-Authored-By: Claude Opus 5 --- src/main/lib/agentRequirementsLaunch.test.ts | 101 +++++++++++++++++- src/main/lib/agentRequirementsLaunch.ts | 66 ++++++++++-- .../lib/ipc/sessionActions/launch.test.ts | 17 ++- src/main/lib/ipc/sessionActions/launch.ts | 23 ++-- src/main/lib/launchPhases.ts | 22 ++-- src/main/lib/launchProgress.test.ts | 99 ++++++++++++++++- src/main/lib/launchProgress.ts | 18 +++- 7 files changed, 312 insertions(+), 34 deletions(-) diff --git a/src/main/lib/agentRequirementsLaunch.test.ts b/src/main/lib/agentRequirementsLaunch.test.ts index 1d84ddeb2..8420db84d 100644 --- a/src/main/lib/agentRequirementsLaunch.test.ts +++ b/src/main/lib/agentRequirementsLaunch.test.ts @@ -151,9 +151,8 @@ describe('installAgentRequirements', () => { it('installs the planned file through the shared uv helper', async () => { const sendOutput = vi.fn() - const signal = new AbortController().signal - await installAgentRequirements(plan, sendOutput, signal) + await installAgentRequirements(plan, sendOutput) expect(mockInstall).toHaveBeenCalledWith( plan.reqPath, @@ -162,7 +161,7 @@ describe('installAgentRequirements', () => { plan.installPath, '.launch-agent-reqs.txt', sendOutput, - signal, + expect.any(AbortSignal), mirrors ) expect(sendOutput.mock.calls.join('')).toContain('Installing agent requirements') @@ -188,6 +187,102 @@ describe('installAgentRequirements', () => { expect(sendOutput.mock.calls.join('')).toContain('EACCES: permission denied') }) + it('abandons an install that outlives the ceiling and lets the launch continue', async () => { + // The whole point of the bound: a stalled uv must not hold the user at the + // launcher. Core starts with the flag and disables the agent itself. + vi.useFakeTimers() + try { + let uvSignal: AbortSignal | undefined + mockInstall.mockImplementationOnce( + async (...args: Parameters) => { + uvSignal = args[6] + // Resolve only once something aborts uv, the way the real helper does. + return new Promise((resolve) => { + uvSignal!.addEventListener('abort', () => resolve({ code: 1, output: '' }), { + once: true + }) + }) + } + ) + const sendOutput = vi.fn() + + const pending = installAgentRequirements(plan, sendOutput) + await vi.advanceTimersByTimeAsync(120_000) + await expect(pending).resolves.toBeUndefined() + + expect(uvSignal?.aborted).toBe(true) + expect(sendOutput.mock.calls.join('')).toContain('starting ComfyUI without it') + } finally { + vi.useRealTimers() + } + }) + + it('does not cancel the launch when the ceiling fires', async () => { + // The deadline aborts a controller this module owns, never the launch's own + // signal - the launch must proceed, not report itself cancelled. + vi.useFakeTimers() + try { + const launchAbort = new AbortController() + mockInstall.mockImplementationOnce( + async (...args: Parameters) => { + const uvSignal = args[6]! + return new Promise((resolve) => { + uvSignal.addEventListener('abort', () => resolve({ code: 1, output: '' }), { + once: true + }) + }) + } + ) + + const pending = installAgentRequirements(plan, vi.fn(), launchAbort.signal) + await vi.advanceTimersByTimeAsync(120_000) + await pending + + expect(launchAbort.signal.aborted).toBe(false) + } finally { + vi.useRealTimers() + } + }) + + it('leaves the deadline behind no timer once the install finishes', async () => { + vi.useFakeTimers() + try { + await installAgentRequirements(plan, vi.fn()) + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + }) + + it('kills uv when the launch itself is cancelled', async () => { + const launchAbort = new AbortController() + let uvSignal: AbortSignal | undefined + mockInstall.mockImplementationOnce( + async (...args: Parameters) => { + uvSignal = args[6] + launchAbort.abort() + return { code: 1, output: '' } + } + ) + + await installAgentRequirements(plan, vi.fn(), launchAbort.signal) + + expect(uvSignal?.aborted).toBe(true) + }) + + it('stays quiet about a thrown install when the launch was cancelled', async () => { + const launchAbort = new AbortController() + mockInstall.mockImplementationOnce(async () => { + launchAbort.abort() + throw new Error('EIO') + }) + const sendOutput = vi.fn() + + await installAgentRequirements(plan, sendOutput, launchAbort.signal) + + expect(sendOutput.mock.calls.join('')).not.toContain('EIO') + }) + it('stays quiet about the exit code when the launch was cancelled mid-install', async () => { // Cancelling kills uv, so its non-zero exit IS the cancellation; reporting it // would put a spurious failure in the output of a launch the user stopped. diff --git a/src/main/lib/agentRequirementsLaunch.ts b/src/main/lib/agentRequirementsLaunch.ts index 031726462..22e3d2526 100644 --- a/src/main/lib/agentRequirementsLaunch.ts +++ b/src/main/lib/agentRequirementsLaunch.ts @@ -12,6 +12,22 @@ const ENABLE_AGENT_ARG = '--enable-agent' /** Requirements file Core ships beside `main.py` for the agent. */ const AGENT_REQUIREMENTS = 'agent_requirements.txt' +/** + * Ceiling on how long a launch waits for the install. + * + * The launch blocks on this so the agent is usable on the run that enables it, + * but core starts perfectly well without the packages, so holding a user at the + * launcher indefinitely to acquire them inverts that. Past the ceiling uv is + * killed and the launch goes ahead with the flag still set: core prints its own + * install hint and disables the agent. The matching bound one step earlier is + * the args-schema probe's 15s, which fails open the same way. + * + * The ceiling is generous rather than tight because uv streams nothing while a + * wheel downloads, so a slow transfer is indistinguishable from a stall. A link + * too slow to finish inside it never gets the agent from this path. + */ +const INSTALL_TIMEOUT_MS = 120_000 + export interface AgentRequirementsInstall { reqPath: string uvPath: string @@ -24,9 +40,12 @@ export interface AgentRequirementsInstall { * resolve everything the install needs. * * `args` must be the FINAL spawn args. `--enable-agent` reaches them either - * from the user's own launch args or from a Core beta grant, and a core whose - * schema does not know the flag has already had it filtered out, so the args - * are the one place that knows whether the agent is really starting. + * from the user's own launch args or from a Core beta grant, so the args are + * the one place that knows whether the agent is really starting. A core whose + * schema does not know the flag has normally had it filtered out by then, with + * one exception: when schema discovery itself failed the launch keeps the raw + * args, so a hand-typed flag survives to a core that cannot parse it. That + * launch fails on argparse either way; the cost is one wasted install. * * The environment test is the one every `manager_requirements.txt` site makes, * which is what confines this to the Desktop-managed installs (standalone and @@ -49,9 +68,15 @@ export function planAgentRequirementsInstall( /** * Install the planned requirements, streaming uv's output into the launch. * - * Never throws and reports nothing back: a failure here must not stop the - * launch. ComfyUI still starts with the flag, prints its own install hint and - * disables the agent itself, which beats refusing to start. + * Bounded and fail-open. Never throws and reports nothing back: neither a + * failure nor a timeout here may stop the launch. ComfyUI still starts with the + * flag, prints its own install hint and disables the agent itself, which beats + * refusing to start. + * + * `signal` is the launch's own, and stays untouched: uv is driven through a + * controller owned here, so the deadline ends the wait without cancelling the + * launch. Aborting it kills uv's process tree and resolves once it is reaped, + * so nothing is left writing to the environment ComfyUI is about to boot from. */ export async function installAgentRequirements( plan: AgentRequirementsInstall, @@ -59,6 +84,16 @@ export async function installAgentRequirements( signal?: AbortSignal ): Promise { sendOutput('\nInstalling agent requirements…\n') + const uvAbort = new AbortController() + const onLaunchAbort = (): void => uvAbort.abort() + if (signal?.aborted) uvAbort.abort() + else signal?.addEventListener('abort', onLaunchAbort, { once: true }) + let timedOut = false + const deadline = setTimeout(() => { + timedOut = true + uvAbort.abort() + }, INSTALL_TIMEOUT_MS) + try { const result = await installFilteredRequirementsDetailed( plan.reqPath, @@ -67,17 +102,26 @@ export async function installAgentRequirements( plan.installPath, '.launch-agent-reqs.txt', sendOutput, - signal, + uvAbort.signal, settings.getMirrorConfig() ) - // A cancelled launch kills uv mid-install; that non-zero exit is the - // cancellation, not a failure worth showing. - if (result.code !== 0 && !signal?.aborted) { + if (timedOut && !signal?.aborted) { + sendOutput( + `\n⚠ agent requirements install exceeded ${INSTALL_TIMEOUT_MS / 1000}s; starting ComfyUI without it\n` + ) + } else if (result.code !== 0 && !signal?.aborted) { + // A cancelled launch kills uv mid-install; that non-zero exit is the + // cancellation, not a failure worth showing. sendOutput( `\n${withOutputTail(`⚠ agent requirements install exited with code ${result.code}`, result.output)}\n` ) } } catch (err) { - sendOutput(`⚠ ${AGENT_REQUIREMENTS} failed: ${(err as Error).message}\n`) + if (!signal?.aborted) { + sendOutput(`⚠ ${AGENT_REQUIREMENTS} failed: ${(err as Error).message}\n`) + } + } finally { + clearTimeout(deadline) + signal?.removeEventListener('abort', onLaunchAbort) } } diff --git a/src/main/lib/ipc/sessionActions/launch.test.ts b/src/main/lib/ipc/sessionActions/launch.test.ts index 2ce4bfd34..953481c8b 100644 --- a/src/main/lib/ipc/sessionActions/launch.test.ts +++ b/src/main/lib/ipc/sessionActions/launch.test.ts @@ -1505,6 +1505,7 @@ describe('agent requirements at launch', () => { const AGENT_GRANT: CoreBetaGrant = { arg: '--enable-agent', minCoreVersion: '0.3.80' } let installDir = '' let sent: string[] = [] + let progress: { phase: string; steps?: { phase: string }[] }[] = [] let spawnArgs: string[] = [] let spawned = 0 @@ -1535,8 +1536,11 @@ describe('agent requirements at launch', () => { event: { sender: { isDestroyed: () => false, - send: (_channel: string, payload: { text?: string }) => { + send: (channel: string, payload: { text?: string; phase?: string }) => { if (typeof payload?.text === 'string') sent.push(payload.text) + if (channel === 'install-progress' && typeof payload?.phase === 'string') { + progress.push(payload as { phase: string; steps?: { phase: string }[] }) + } } } } as unknown as Electron.IpcMainInvokeEvent, @@ -1553,6 +1557,7 @@ describe('agent requirements at launch', () => { touch(getVenvPythonPath(installDir)) fs.writeFileSync(agentReqPath(), 'comfyui-agent==1.0.0\n') sent = [] + progress = [] spawnArgs = [] spawned = 0 pipHarness.calls = [] @@ -1682,6 +1687,16 @@ describe('agent requirements at launch', () => { expect(sent.join('')).toContain('exited with code 1') }) + it('publishes the install as a step the renderer can show', async () => { + // The renderer drops progress for a phase absent from the steps payload, so + // the payload is what makes the step visible at all. + await handleLaunch(ctxFor('agent-reqs-step-published')) + + const lastSteps = progress.filter((p) => p.phase === 'steps').at(-1) + expect(lastSteps?.steps?.map((s) => s.phase)).toContain('agentRequirements') + expect(progress.some((p) => p.phase === 'agentRequirements')).toBe(true) + }) + it('cancels the launch without spawning when it is aborted mid-install', async () => { pipHarness.duringInstall = () => _operationAborts.get('agent-reqs-cancelled')?.abort() diff --git a/src/main/lib/ipc/sessionActions/launch.ts b/src/main/lib/ipc/sessionActions/launch.ts index 19dfba7e2..aa3f93d3f 100644 --- a/src/main/lib/ipc/sessionActions/launch.ts +++ b/src/main/lib/ipc/sessionActions/launch.ts @@ -65,7 +65,7 @@ import { createAssetsTap } from '../../assetsTap' import { createExecutionTap } from '../../executionTap' import { createHardwareTap } from '../../hardwareTap' import { createLaunchProgressTracker } from '../../launchProgress' -import { buildLaunchPhases } from '../../launchPhases' +import { buildLaunchPhases, AGENT_REQUIREMENTS_PHASE } from '../../launchPhases' import { getTemplateDownloadState, summarizeTemplateState, @@ -1019,17 +1019,20 @@ async function runLaunch( return { ok: false, message: i18n.t('errors.managerConfigWriteFailed') } } - // The args are final here, so this is the first point that knows the agent is - // actually starting - whether the user typed the flag or a beta grant added - // it, and whether the running core can parse it at all. The package is tens of - // megabytes, so it gets its own launch step; a failure is reported in the - // launch output and the flag is kept, leaving core to print its install hint - // and disable the agent itself. + // The agent flag is final here (only path args are appended after this), so + // this is the first point that knows the agent is actually starting - whether + // the user typed the flag or a beta grant added it, and whether the running + // core can parse it at all. The package is tens of megabytes, so it gets its + // own launch step. Bounded and fail-open: a failure or a timeout is reported + // in the launch output and the flag is kept, leaving core to print its install + // hint and disable the agent itself. + // + // The step is added through `addLatePhase` rather than `preLaunchPhases` + // because a torch repair may already have armed the tracker, freezing that list. const agentRequirements = planAgentRequirementsInstall(inst, launchCmd.args ?? []) if (agentRequirements) { - preLaunchPhases.push('agentRequirements') - await armLaunchTracker() - sendProgress('agentRequirements', { percent: -1, status: '' }) + const tracker = await armLaunchTracker() + tracker.addLatePhase(AGENT_REQUIREMENTS_PHASE) await installAgentRequirements( agentRequirements, makeSendOutput(sender, installationId), diff --git a/src/main/lib/launchPhases.ts b/src/main/lib/launchPhases.ts index dfcbb0f6e..da86cd6fa 100644 --- a/src/main/lib/launchPhases.ts +++ b/src/main/lib/launchPhases.ts @@ -89,17 +89,25 @@ export const DEFAULT_LAUNCH_PHASES: readonly LaunchPhaseDef[] = [ * boot milestone fires. Weights add on top of the base 1.0; the renderer * normalizes, so injection just shrinks every slot proportionally. * - * - `repair` interrupted-op source rollback was performed - * - `torchRepair` GPU PyTorch was restored after the v1.13.0 `--upgrade` bug - * - `agentRequirements` the agent's Python packages were installed for a - * launch that starts Core with `--enable-agent` + * - `repair` interrupted-op source rollback was performed + * - `torchRepair` GPU PyTorch was restored after the v1.13.0 `--upgrade` bug */ -export type PreLaunchPhase = 'repair' | 'torchRepair' | 'agentRequirements' +export type PreLaunchPhase = 'repair' | 'torchRepair' const PRE_LAUNCH_PHASES: Record = { repair: { phase: 'repair', match: NEVER, weight: 0.1, streaming: true }, - torchRepair: { phase: 'torchRepair', match: NEVER, weight: 0.1, streaming: true }, - agentRequirements: { phase: 'agentRequirements', match: NEVER, weight: 0.1, streaming: true } + torchRepair: { phase: 'torchRepair', match: NEVER, weight: 0.1, streaming: true } +} + +/** The agent requirements install. Not one of `PRE_LAUNCH_PHASES`: whether it + * runs is only known once the launch args are final, which is after a torch + * repair may already have armed the tracker and frozen the phase list. It is + * handed to `addLatePhase` instead of being injected here. */ +export const AGENT_REQUIREMENTS_PHASE: LaunchPhaseDef = { + phase: 'agentRequirements', + match: NEVER, + weight: 0.1, + streaming: true } /** Starter-template model download, shown as the LAST launch step. Synthetic + diff --git a/src/main/lib/launchProgress.test.ts b/src/main/lib/launchProgress.test.ts index 5c17bfea1..ffc80d141 100644 --- a/src/main/lib/launchProgress.test.ts +++ b/src/main/lib/launchProgress.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect } from 'vitest' import fs from 'fs' import path from 'path' import { createLaunchProgressTracker } from './launchProgress' -import { DEFAULT_LAUNCH_PHASES, buildLaunchPhases } from './launchPhases' +import { DEFAULT_LAUNCH_PHASES, buildLaunchPhases, AGENT_REQUIREMENTS_PHASE } from './launchPhases' const FIXTURE = fs.readFileSync( path.join(__dirname, '__fixtures__', 'launch', 'first-run.log'), @@ -259,3 +259,100 @@ describe('buildLaunchPhases — extensibility', () => { expect(order).toContain('mountLibraries') }) }) + +describe('addLatePhase for work discovered after the tracker was armed', () => { + /** Arm a tracker over `phases` and return it with its emit log. */ + function armed(phases = buildLaunchPhases({}, { preLaunchPhases: ['torchRepair'] })): { + tracker: ReturnType + emits: Emit[] + } { + const emits: Emit[] = [] + const tracker = createLaunchProgressTracker({ + phases, + sendProgress: (phase, detail) => emits.push({ phase, ...detail }) + }) + tracker.start() + return { tracker, emits } + } + + it('re-emits the steps payload so the renderer knows the new phase', () => { + // Without the re-emit the renderer drops the phase's progress outright: + // progressStore returns early when the phase is absent from `steps`. + const { tracker, emits } = armed() + const initial = emits + .filter((e) => e.phase === 'steps') + .at(-1)! + .steps!.map((s) => s.phase) + expect(initial).not.toContain('agentRequirements') + + tracker.addLatePhase(AGENT_REQUIREMENTS_PHASE) + + const stepPayloads = emits.filter((e) => e.phase === 'steps') + expect(stepPayloads).toHaveLength(2) + expect(stepPayloads[1]!.steps?.map((s) => s.phase)).toContain('agentRequirements') + }) + + it('enters the new phase, so it is the active step while the work runs', () => { + const { tracker, emits } = armed() + const before = emits.length + + tracker.addLatePhase(AGENT_REQUIREMENTS_PHASE) + + const after = emits.slice(before).filter((e) => e.phase !== 'steps') + expect(after.map((e) => e.phase)).toEqual(['agentRequirements']) + expect(after[0]!.percent).toBe(-1) + }) + + it('inserts directly after the active phase so the bar cannot regress', () => { + // torchRepair is active (phase 0). The new phase must land at 1, ahead of + // it and ahead of nothing already completed. + const { tracker, emits } = armed() + + tracker.addLatePhase(AGENT_REQUIREMENTS_PHASE) + + const published = emits + .filter((e) => e.phase === 'steps') + .at(-1)! + .steps!.map((s) => s.phase) + expect(published.slice(0, 2)).toEqual(['torchRepair', 'agentRequirements']) + expect(published).toEqual([ + 'torchRepair', + 'agentRequirements', + ...DEFAULT_LAUNCH_PHASES.map((p) => p.phase) + ]) + }) + + it('still advances into the real boot phases afterwards', () => { + const { tracker, emits } = armed() + tracker.addLatePhase(AGENT_REQUIREMENTS_PHASE) + + tracker.ingest('Total VRAM 24576 MB\n') + + expect(phaseOrder(emits).at(-1)).toBe('gpu') + }) + + it('works when nothing else injected a phase (the common launch)', () => { + const { tracker, emits } = armed(buildLaunchPhases({})) + + tracker.addLatePhase(AGENT_REQUIREMENTS_PHASE) + + const published = emits + .filter((e) => e.phase === 'steps') + .at(-1)! + .steps!.map((s) => s.phase) + // launchStart is active at index 0, so the install lands right after it. + expect(published.slice(0, 2)).toEqual(['launchStart', 'agentRequirements']) + expect(phaseOrder(emits).at(-1)).toBe('agentRequirements') + }) + + it("does not mutate the caller's phase array", () => { + const phases = buildLaunchPhases({}) + const { tracker } = armed(phases) + const lengthBefore = phases.length + + tracker.addLatePhase(AGENT_REQUIREMENTS_PHASE) + + expect(phases).toHaveLength(lengthBefore) + expect(phases.map((p) => p.phase)).not.toContain('agentRequirements') + }) +}) diff --git a/src/main/lib/launchProgress.ts b/src/main/lib/launchProgress.ts index 273f048df..0c6bb2c02 100644 --- a/src/main/lib/launchProgress.ts +++ b/src/main/lib/launchProgress.ts @@ -73,6 +73,12 @@ export interface LaunchProgressTracker { start: () => void /** Feed a stdout/stderr chunk. Safe to call with partial lines. */ ingest: (chunk: string) => void + /** Add a phase for work that only became known after the tracker was armed, + * and enter it. The steps payload is re-emitted, without which the renderer + * drops the phase's progress entirely (it ignores a phase absent from the + * payload). Inserted directly after the active phase, so the bar advances + * into a new slot rather than regressing. */ + addLatePhase: (def: LaunchPhaseDef) => void /** Restart per-attempt `onPhaseEnter` observation for a boot retry. The UI * phase index stays monotonic (progress never regresses), but the retried * process re-logs its boot from the top - without this reset those re-hit @@ -95,7 +101,10 @@ export function createLaunchProgressTracker(opts: { * break progress, so it is swallowed. */ onPhaseEnter?: (phase: string) => void }): LaunchProgressTracker { - const { phases, sendProgress, onPhaseEnter } = opts + const { sendProgress, onPhaseEnter } = opts + // Copied, not aliased: `addLatePhase` splices into this list, and the caller's + // array must not change under it. + const phases = [...opts.phases] const nodeCount = opts.nodeCount && opts.nodeCount > 0 ? opts.nodeCount : 0 // Index of the currently-active phase; -1 until the first milestone. @@ -252,6 +261,13 @@ export function createLaunchProgressTracker(opts: { const lines = pending.split(/\r?\n/) pending = lines.pop() ?? '' for (const line of lines) handleLine(line) + }, + addLatePhase(def: LaunchPhaseDef): void { + const idx = activeIdx + 1 + phases.splice(idx, 0, { ...def }) + stepsSent = false + emitSteps() + enterPhase(idx) } } } From 17fcfa543358f0cc9fac5e3c2adb71ac877b9696 Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Mon, 21 Sep 2026 17:17:46 -0700 Subject: [PATCH 3/6] fix(launch): make the agent install ceiling a real bound Codex review on #1559. The ceiling only asked uv to stop. killProcTree sends SIGTERM to the process group on POSIX and swallows a failed taskkill on Windows, neither awaited, while the install settles only on the child's own exit. A uv that never took the signal therefore held the launch open with no limit, which is the exact failure the ceiling was added to prevent. The wait is now bounded twice: the ceiling asks uv to stop, and a grace period later the launch stops waiting whether or not it did, reporting that it has abandoned a still-running install. The grace is armed by whichever side raised the abort, so a user cancel cannot be held open either. The install promise is settled into a value rather than awaited directly, because losing the race leaves it pending and a later rejection with nothing awaiting it would surface as an unhandled rejection. The two new tests were checked against the unfixed code: the unbounded case hangs until the runner times it out, and the within-grace case proves the grace does not cut short a uv that is on its way out. Co-Authored-By: Claude Opus 5 --- src/main/lib/agentRequirementsLaunch.test.ts | 56 ++++++++++++ src/main/lib/agentRequirementsLaunch.ts | 92 +++++++++++++++----- 2 files changed, 126 insertions(+), 22 deletions(-) diff --git a/src/main/lib/agentRequirementsLaunch.test.ts b/src/main/lib/agentRequirementsLaunch.test.ts index 8420db84d..c63f080fe 100644 --- a/src/main/lib/agentRequirementsLaunch.test.ts +++ b/src/main/lib/agentRequirementsLaunch.test.ts @@ -217,6 +217,62 @@ describe('installAgentRequirements', () => { } }) + it('stops waiting for a uv that never exits after being killed', async () => { + // The ceiling only asks uv to stop: killProcTree sends SIGTERM and does not + // wait, and the helper settles on the child's exit, so a uv that ignores the + // signal would hold the launch open past the bound meant to prevent exactly + // that. The wait has to end on its own. + vi.useFakeTimers() + try { + mockInstall.mockImplementationOnce( + // Never settles, however it is signalled. + () => new Promise(() => {}) + ) + const sendOutput = vi.fn() + + const pending = installAgentRequirements(plan, sendOutput) + await vi.advanceTimersByTimeAsync(120_000) + await vi.advanceTimersByTimeAsync(10_000) + + await expect(pending).resolves.toBeUndefined() + expect(sendOutput.mock.calls.join('')).toContain('uv did not stop') + } finally { + vi.useRealTimers() + } + }) + + it('keeps waiting while uv is still within the grace period', async () => { + // The grace must not cut short a uv that is on its way out, or the warning + // would fire on every ordinary cancellation. + vi.useFakeTimers() + try { + mockInstall.mockImplementationOnce( + async (...args: Parameters) => { + const uvSignal = args[6]! + return new Promise((resolve) => { + uvSignal.addEventListener( + 'abort', + () => setTimeout(() => resolve({ code: 1, output: '' }), 2_000), + { once: true } + ) + }) + } + ) + const sendOutput = vi.fn() + + const pending = installAgentRequirements(plan, sendOutput) + await vi.advanceTimersByTimeAsync(120_000) + await vi.advanceTimersByTimeAsync(2_000) + await pending + + const reported = sendOutput.mock.calls.join('') + expect(reported).toContain('starting ComfyUI without it') + expect(reported).not.toContain('uv did not stop') + } finally { + vi.useRealTimers() + } + }) + it('does not cancel the launch when the ceiling fires', async () => { // The deadline aborts a controller this module owns, never the launch's own // signal - the launch must proceed, not report itself cancelled. diff --git a/src/main/lib/agentRequirementsLaunch.ts b/src/main/lib/agentRequirementsLaunch.ts index 22e3d2526..e2f437445 100644 --- a/src/main/lib/agentRequirementsLaunch.ts +++ b/src/main/lib/agentRequirementsLaunch.ts @@ -2,6 +2,7 @@ import fs from 'fs' import path from 'path' import * as settings from '../settings' import { installFilteredRequirementsDetailed } from './pip' +import type { UvPipResult } from './pip' import { withOutputTail } from './logged-process' import { getActivePythonPath, getActiveUvPath } from './pythonEnv' import type { InstallationRecord } from '../installations' @@ -28,6 +29,26 @@ const AGENT_REQUIREMENTS = 'agent_requirements.txt' */ const INSTALL_TIMEOUT_MS = 120_000 +/** + * Grace between asking uv to stop and the launch giving up on it. + * + * Killing is best-effort and not awaited: `killProcTree` sends SIGTERM to the + * process group on POSIX and swallows a failed `taskkill` on Windows, while the + * install settles only on the child's own exit. A uv that does not take the + * signal would therefore hold the launch open past the very ceiling that exists + * to stop that, so the wait is bounded here too and the launch proceeds either + * way. An abandoned uv may still be writing to the environment, which is worth + * one warning line and is strictly better than never starting. + */ +const KILL_GRACE_MS = 10_000 + +/** Which way the bounded wait ended: uv exited, it threw, or the launch stopped + * waiting for a uv that would not stop. */ +type InstallOutcome = + | { kind: 'settled'; result: UvPipResult } + | { kind: 'failed'; error: unknown } + | { kind: 'abandoned' } + export interface AgentRequirementsInstall { reqPath: string uvPath: string @@ -75,8 +96,9 @@ export function planAgentRequirementsInstall( * * `signal` is the launch's own, and stays untouched: uv is driven through a * controller owned here, so the deadline ends the wait without cancelling the - * launch. Aborting it kills uv's process tree and resolves once it is reaped, - * so nothing is left writing to the environment ComfyUI is about to boot from. + * launch. Because killing uv is best-effort, the wait is bounded twice over: + * the ceiling asks it to stop, and `KILL_GRACE_MS` later the launch stops + * waiting whether or not it did. Nothing downstream depends on which happened. */ export async function installAgentRequirements( plan: AgentRequirementsInstall, @@ -86,42 +108,68 @@ export async function installAgentRequirements( sendOutput('\nInstalling agent requirements…\n') const uvAbort = new AbortController() const onLaunchAbort = (): void => uvAbort.abort() + let timedOut = false + let graceTimer: ReturnType | undefined + let abandon = (): void => {} + const abandoned = new Promise((resolve) => { + abandon = () => resolve({ kind: 'abandoned' }) + }) + // Armed on whichever side raised the abort, so neither the ceiling nor a user + // cancel can be held open by a uv that never takes the signal. + uvAbort.signal.addEventListener( + 'abort', + () => { + graceTimer = setTimeout(abandon, KILL_GRACE_MS) + }, + { once: true } + ) if (signal?.aborted) uvAbort.abort() else signal?.addEventListener('abort', onLaunchAbort, { once: true }) - let timedOut = false const deadline = setTimeout(() => { timedOut = true uvAbort.abort() }, INSTALL_TIMEOUT_MS) + // Settled into a value rather than awaited directly: losing the race leaves + // this pending, and a later rejection with nothing awaiting it would surface + // as an unhandled rejection. + const install: Promise = installFilteredRequirementsDetailed( + plan.reqPath, + plan.uvPath, + plan.pythonPath, + plan.installPath, + '.launch-agent-reqs.txt', + sendOutput, + uvAbort.signal, + settings.getMirrorConfig() + ).then( + (result) => ({ kind: 'settled', result }), + (error: unknown) => ({ kind: 'failed', error }) + ) + try { - const result = await installFilteredRequirementsDetailed( - plan.reqPath, - plan.uvPath, - plan.pythonPath, - plan.installPath, - '.launch-agent-reqs.txt', - sendOutput, - uvAbort.signal, - settings.getMirrorConfig() - ) - if (timedOut && !signal?.aborted) { + const outcome = await Promise.race([install, abandoned]) + // A cancelled launch kills uv mid-install, so whatever it reports is the + // cancellation rather than a failure worth showing. + if (signal?.aborted) return + if (outcome.kind === 'abandoned') { + sendOutput( + `\n⚠ agent requirements install exceeded ${INSTALL_TIMEOUT_MS / 1000}s and uv did not stop; starting ComfyUI anyway\n` + ) + } else if (outcome.kind === 'failed') { + sendOutput(`⚠ ${AGENT_REQUIREMENTS} failed: ${(outcome.error as Error).message}\n`) + } else if (timedOut) { sendOutput( `\n⚠ agent requirements install exceeded ${INSTALL_TIMEOUT_MS / 1000}s; starting ComfyUI without it\n` ) - } else if (result.code !== 0 && !signal?.aborted) { - // A cancelled launch kills uv mid-install; that non-zero exit is the - // cancellation, not a failure worth showing. + } else if (outcome.result.code !== 0) { sendOutput( - `\n${withOutputTail(`⚠ agent requirements install exited with code ${result.code}`, result.output)}\n` + `\n${withOutputTail(`⚠ agent requirements install exited with code ${outcome.result.code}`, outcome.result.output)}\n` ) } - } catch (err) { - if (!signal?.aborted) { - sendOutput(`⚠ ${AGENT_REQUIREMENTS} failed: ${(err as Error).message}\n`) - } } finally { clearTimeout(deadline) + if (graceTimer !== undefined) clearTimeout(graceTimer) signal?.removeEventListener('abort', onLaunchAbort) } } From 6bba28441e6322b4db417ce8fafeed607360461d Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Mon, 21 Sep 2026 17:25:53 -0700 Subject: [PATCH 4/6] fix(launch): let the exit code decide a grace-period finish CodeRabbit review on #1559. The ceiling can fire while uv is already on its way out with a zero exit, and the report was branching on the timer before the exit code, so an install that genuinely succeeded was announced as skipped. Nothing behaved differently, but the launch output told the user the agent was not installed on a run that had just installed it. The exit code decides now, and the timer only chooses the wording for an install that did not succeed. Checked against the unfixed code: the new test fails with the "without it" message on a zero exit. Co-Authored-By: Claude Opus 5 --- src/main/lib/agentRequirementsLaunch.test.ts | 34 ++++++++++++++++++++ src/main/lib/agentRequirementsLaunch.ts | 11 ++++--- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/main/lib/agentRequirementsLaunch.test.ts b/src/main/lib/agentRequirementsLaunch.test.ts index c63f080fe..1dee434d6 100644 --- a/src/main/lib/agentRequirementsLaunch.test.ts +++ b/src/main/lib/agentRequirementsLaunch.test.ts @@ -273,6 +273,40 @@ describe('installAgentRequirements', () => { } }) + it('reports success for an install that finishes inside the grace period', async () => { + // The ceiling can fire while uv is already on its way out with a zero exit. + // Reporting that from the timer rather than the exit code would tell the + // user the agent was skipped on a launch that actually installed it. + vi.useFakeTimers() + try { + mockInstall.mockImplementationOnce( + async (...args: Parameters) => { + const uvSignal = args[6]! + return new Promise((resolve) => { + uvSignal.addEventListener( + 'abort', + () => setTimeout(() => resolve({ code: 0, output: '' }), 1_000), + { once: true } + ) + }) + } + ) + const sendOutput = vi.fn() + + const pending = installAgentRequirements(plan, sendOutput) + await vi.advanceTimersByTimeAsync(120_000) + await vi.advanceTimersByTimeAsync(1_000) + await pending + + const reported = sendOutput.mock.calls.join('') + expect(reported).not.toContain('without it') + expect(reported).not.toContain('uv did not stop') + expect(reported).not.toContain('exited with code') + } finally { + vi.useRealTimers() + } + }) + it('does not cancel the launch when the ceiling fires', async () => { // The deadline aborts a controller this module owns, never the launch's own // signal - the launch must proceed, not report itself cancelled. diff --git a/src/main/lib/agentRequirementsLaunch.ts b/src/main/lib/agentRequirementsLaunch.ts index e2f437445..7ce8b8e87 100644 --- a/src/main/lib/agentRequirementsLaunch.ts +++ b/src/main/lib/agentRequirementsLaunch.ts @@ -158,13 +158,14 @@ export async function installAgentRequirements( ) } else if (outcome.kind === 'failed') { sendOutput(`⚠ ${AGENT_REQUIREMENTS} failed: ${(outcome.error as Error).message}\n`) - } else if (timedOut) { - sendOutput( - `\n⚠ agent requirements install exceeded ${INSTALL_TIMEOUT_MS / 1000}s; starting ComfyUI without it\n` - ) } else if (outcome.result.code !== 0) { + // The exit code decides, not the timer that was racing it: an install + // that finished inside the grace period succeeded, however close to the + // ceiling it landed, and must not be reported as skipped. sendOutput( - `\n${withOutputTail(`⚠ agent requirements install exited with code ${outcome.result.code}`, outcome.result.output)}\n` + timedOut + ? `\n⚠ agent requirements install exceeded ${INSTALL_TIMEOUT_MS / 1000}s; starting ComfyUI without it\n` + : `\n${withOutputTail(`⚠ agent requirements install exited with code ${outcome.result.code}`, outcome.result.output)}\n` ) } } finally { From 6595d8e563cde2a03830fb6c1b9d8e3dfdb1528f Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Wed, 23 Sep 2026 01:19:01 -0700 Subject: [PATCH 5/6] fix(launch): print uv's failure output once Found on a real Windows run of a failing agent-requirements install: uv's error block appeared twice in app.log, once streamed live under the Installing line and again under the exited-with-code line. The shared helper streams into the same sink it captures from, so building the failure message with withOutputTail over the captured output reprints whatever uv had just said. The exit code is the only part that was not already on screen, so that is all the line carries now. Checked against the unfixed code: the new test counts two occurrences instead of one. Co-Authored-By: Claude Opus 5 --- src/main/lib/agentRequirementsLaunch.test.ts | 22 ++++++++++++++++++-- src/main/lib/agentRequirementsLaunch.ts | 7 +++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/main/lib/agentRequirementsLaunch.test.ts b/src/main/lib/agentRequirementsLaunch.test.ts index 1dee434d6..92f18a19e 100644 --- a/src/main/lib/agentRequirementsLaunch.test.ts +++ b/src/main/lib/agentRequirementsLaunch.test.ts @@ -173,9 +173,27 @@ describe('installAgentRequirements', () => { await expect(installAgentRequirements(plan, sendOutput)).resolves.toBeUndefined() + expect(sendOutput.mock.calls.join('')).toContain('exited with code 2') + }) + + it('does not reprint uv output it already streamed', async () => { + // The shared helper streams into the same sink it captures from, so + // appending the captured tail to the failure line put uv's error in the + // log twice - seen on a real Windows run of a failing install. + mockInstall.mockImplementationOnce( + async (...args: Parameters) => { + const stream = args[5] + stream('ERROR: No solution found for comfyui-agent\n') + return { code: 1, output: 'ERROR: No solution found for comfyui-agent\n' } + } + ) + const sendOutput = vi.fn() + + await installAgentRequirements(plan, sendOutput) + const reported = sendOutput.mock.calls.join('') - expect(reported).toContain('exited with code 2') - expect(reported).toContain('No solution found') + expect(reported.match(/No solution found/g)).toHaveLength(1) + expect(reported).toContain('exited with code 1') }) it('reports a thrown install and resolves so the launch continues', async () => { diff --git a/src/main/lib/agentRequirementsLaunch.ts b/src/main/lib/agentRequirementsLaunch.ts index 7ce8b8e87..b70ac3019 100644 --- a/src/main/lib/agentRequirementsLaunch.ts +++ b/src/main/lib/agentRequirementsLaunch.ts @@ -3,7 +3,6 @@ import path from 'path' import * as settings from '../settings' import { installFilteredRequirementsDetailed } from './pip' import type { UvPipResult } from './pip' -import { withOutputTail } from './logged-process' import { getActivePythonPath, getActiveUvPath } from './pythonEnv' import type { InstallationRecord } from '../installations' @@ -162,10 +161,14 @@ export async function installAgentRequirements( // The exit code decides, not the timer that was racing it: an install // that finished inside the grace period succeeded, however close to the // ceiling it landed, and must not be reported as skipped. + // + // Only the code, never a tail of the captured output: uv streams into + // this same sink as it runs, so appending what it captured reprints the + // error a second time in the log. sendOutput( timedOut ? `\n⚠ agent requirements install exceeded ${INSTALL_TIMEOUT_MS / 1000}s; starting ComfyUI without it\n` - : `\n${withOutputTail(`⚠ agent requirements install exited with code ${outcome.result.code}`, outcome.result.output)}\n` + : `\n⚠ agent requirements install exited with code ${outcome.result.code}\n` ) } } finally { From 631238b361641e726293bb0454f61386dd054a98 Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Wed, 23 Sep 2026 02:00:41 -0700 Subject: [PATCH 6/6] feat(launch): show what the agent install is doing From the Windows run: the launch row sat on one caption at 5 percent for the whole 27s download, so the step looked stuck. uv's stream is the only progress signal available, since it prints nothing between starting a wheel and finishing it. Its `Downloading NAME (SIZE)` lines now drive the row, and its `Prepared`/`Installed` handover switches it to installing. Percent stays indeterminate: no byte-level parsing, so there is nothing to drift out of step with reality. Anything unrecognised leaves the row alone rather than flickering through resolution counts and per-package acknowledgements. A failure or timeout now leaves a short terminal status on the row saying the agent install failed and ComfyUI is continuing, instead of the row completing as though nothing happened. The row is not marked errored: the launch itself succeeded. The mapping is a pure function over one line, so the parsing is tested directly, including that an unrecognised line yields nothing and that a milestone split across two chunks is still matched. `t()` resolves nothing under vitest, so what is pinned at the launch site is the contract between the mapper's keys and en.json. Co-Authored-By: Claude Opus 5 --- locales/en.json | 7 +- locales/zh.json | 7 +- src/main/lib/agentRequirementsLaunch.test.ts | 129 +++++++++++++++++- src/main/lib/agentRequirementsLaunch.ts | 67 ++++++++- .../lib/ipc/sessionActions/launch.test.ts | 20 +++ src/main/lib/ipc/sessionActions/launch.ts | 24 +++- 6 files changed, 248 insertions(+), 6 deletions(-) diff --git a/locales/en.json b/locales/en.json index 5cb177b87..6d440711b 100644 --- a/locales/en.json +++ b/locales/en.json @@ -1285,7 +1285,12 @@ "database": "Updating database…", "nodes": "Loading custom nodes…" }, - "viewLogs": "View logs" + "viewLogs": "View logs", + "agentRequirements": { + "downloading": "Downloading {name} ({size})", + "installing": "Installing…", + "failed": "Agent install failed; continuing without it" + } }, "devPlatform": { "signIn": { diff --git a/locales/zh.json b/locales/zh.json index ab96a2412..353094537 100644 --- a/locales/zh.json +++ b/locales/zh.json @@ -1285,7 +1285,12 @@ "database": "正在更新数据库…", "nodes": "正在加载自定义节点…" }, - "viewLogs": "查看日志" + "viewLogs": "查看日志", + "agentRequirements": { + "downloading": "正在下载 {name}({size})", + "installing": "正在安装…", + "failed": "智能体依赖安装失败;继续启动 ComfyUI" + } }, "devPlatform": { "signIn": { diff --git a/src/main/lib/agentRequirementsLaunch.test.ts b/src/main/lib/agentRequirementsLaunch.test.ts index 92f18a19e..ed17ec14e 100644 --- a/src/main/lib/agentRequirementsLaunch.test.ts +++ b/src/main/lib/agentRequirementsLaunch.test.ts @@ -13,7 +13,12 @@ vi.mock('./pip', () => ({ installFilteredRequirementsDetailed: vi.fn(async () => ({ code: 0, output: '' })) })) -import { installAgentRequirements, planAgentRequirementsInstall } from './agentRequirementsLaunch' +import { + agentInstallStatus, + installAgentRequirements, + planAgentRequirementsInstall +} from './agentRequirementsLaunch' +import type { AgentInstallStatus } from './agentRequirementsLaunch' import { installFilteredRequirementsDetailed } from './pip' import { getUvPath, getVenvPythonPath, getLegacyVenvUvPath } from './pythonEnv' import type { InstallationRecord } from '../installations' @@ -406,3 +411,125 @@ describe('installAgentRequirements', () => { expect(sendOutput.mock.calls.join('')).not.toContain('exited with code') }) }) + +describe('agentInstallStatus', () => { + it('reads the package and size out of uv download lines', () => { + // uv's real form has no space before the unit; the spaced form is accepted + // too rather than pinning the test to one version's formatting. + expect(agentInstallStatus('Downloading numpy (15.3MiB)')).toEqual({ + kind: 'downloading', + name: 'numpy', + size: '15.3MiB' + }) + expect(agentInstallStatus('Downloading comfy-agent (36.0 MiB)')).toEqual({ + kind: 'downloading', + name: 'comfy-agent', + size: '36.0 MiB' + }) + }) + + it("treats uv's own handover lines as the install phase", () => { + expect(agentInstallStatus('Prepared 2 packages in 838ms')).toEqual({ kind: 'installing' }) + expect(agentInstallStatus('Installed 2 packages in 17ms')).toEqual({ kind: 'installing' }) + }) + + it('leaves the status alone for anything it does not recognise', () => { + // Returning null is what keeps the row from flickering through uv's + // resolution counts and per-package acknowledgements. + for (const line of [ + 'Resolved 2 packages in 286ms', + ' Downloaded numpy', + 'Using CPython 3.12.13 environment at: .venv', + 'warning: some warning', + '' + ]) { + expect(agentInstallStatus(line)).toBeNull() + } + }) +}) + +describe('installAgentRequirements status reporting', () => { + const plan = { + reqPath: '/inst/ComfyUI/agent_requirements.txt', + uvPath: '/inst/standalone-env/bin/uv', + pythonPath: '/inst/ComfyUI/.venv/bin/python3', + installPath: '/inst' + } + + beforeEach(() => { + vi.clearAllMocks() + }) + + it("drives the row from uv's stream, across chunk boundaries", () => { + const seen: AgentInstallStatus[] = [] + mockInstall.mockImplementationOnce( + async (...args: Parameters) => { + const stream = args[5] + // A milestone split mid-line: the helper forwards raw chunks. + stream('Resolved 1 package in 12ms\nDownloading comfy-ag') + stream('ent (36.0 MiB)\n') + stream('Installed 1 package in 9ms\n') + return { code: 0, output: '' } + } + ) + + return installAgentRequirements(plan, vi.fn(), undefined, (s) => seen.push(s)).then(() => { + expect(seen).toEqual([ + { kind: 'downloading', name: 'comfy-agent', size: '36.0 MiB' }, + { kind: 'installing' } + ]) + }) + }) + + it('reports a failed install as a terminal row status', async () => { + const seen: AgentInstallStatus[] = [] + mockInstall.mockResolvedValueOnce({ code: 1, output: '' }) + + await installAgentRequirements(plan, vi.fn(), undefined, (s) => seen.push(s)) + + expect(seen).toEqual([{ kind: 'failed' }]) + }) + + it('reports a thrown install as a terminal row status', async () => { + const seen: AgentInstallStatus[] = [] + mockInstall.mockRejectedValueOnce(new Error('EACCES')) + + await installAgentRequirements(plan, vi.fn(), undefined, (s) => seen.push(s)) + + expect(seen).toEqual([{ kind: 'failed' }]) + }) + + it('reports a timed-out install as a terminal row status', async () => { + vi.useFakeTimers() + try { + const seen: AgentInstallStatus[] = [] + mockInstall.mockImplementationOnce( + async (...args: Parameters) => { + const uvSignal = args[6]! + return new Promise((resolve) => { + uvSignal.addEventListener('abort', () => resolve({ code: 1, output: '' }), { + once: true + }) + }) + } + ) + + const pending = installAgentRequirements(plan, vi.fn(), undefined, (s) => seen.push(s)) + await vi.advanceTimersByTimeAsync(120_000) + await pending + + expect(seen).toEqual([{ kind: 'failed' }]) + } finally { + vi.useRealTimers() + } + }) + + it('says nothing terminal when the install succeeds', async () => { + const seen: AgentInstallStatus[] = [] + mockInstall.mockResolvedValueOnce({ code: 0, output: '' }) + + await installAgentRequirements(plan, vi.fn(), undefined, (s) => seen.push(s)) + + expect(seen).toEqual([]) + }) +}) diff --git a/src/main/lib/agentRequirementsLaunch.ts b/src/main/lib/agentRequirementsLaunch.ts index b70ac3019..9113b15ec 100644 --- a/src/main/lib/agentRequirementsLaunch.ts +++ b/src/main/lib/agentRequirementsLaunch.ts @@ -41,6 +41,55 @@ const INSTALL_TIMEOUT_MS = 120_000 */ const KILL_GRACE_MS = 10_000 +/** + * What the launch row should say while the install runs. + * + * Structured rather than translated here so the mapping stays testable and the + * locale lookup stays with the other launch strings. `failed` is terminal: the + * row keeps it once the step is done, rather than completing silently on a + * launch that is starting without the agent. + */ +export type AgentInstallStatus = + | { kind: 'downloading'; name: string; size: string } + | { kind: 'installing' } + | { kind: 'failed' } + +/** uv prints one of these per wheel before the bytes move: `Downloading numpy + * (15.3MiB)`. The size is taken verbatim, since uv already formats it. */ +const UV_DOWNLOADING = /^\s*Downloading\s+(\S+)\s+\(([^)]+)\)\s*$/ + +/** uv's own handover from fetching to installing. */ +const UV_INSTALLING = /^\s*(?:Prepared|Installed)\s+\d+\s+package/ + +/** + * Map one line of uv's output to a status, or null to leave the row alone. + * + * Null is the common case and deliberately so: uv prints resolution counts, + * per-package `Downloaded` acknowledgements and warnings that would either + * flicker the row or say nothing a user can act on. + */ +export function agentInstallStatus(line: string): AgentInstallStatus | null { + const downloading = line.match(UV_DOWNLOADING) + if (downloading) return { kind: 'downloading', name: downloading[1]!, size: downloading[2]! } + if (UV_INSTALLING.test(line)) return { kind: 'installing' } + return null +} + +/** Feed uv's stream through the line matcher. Buffers a partial tail, because + * the helper forwards raw chunks and a milestone can straddle two of them. */ +function scanForStatus(onStatus: (status: AgentInstallStatus) => void): (text: string) => void { + let pending = '' + return (text: string): void => { + pending += text + const lines = pending.split(/\r?\n/) + pending = lines.pop() ?? '' + for (const line of lines) { + const status = agentInstallStatus(line) + if (status) onStatus(status) + } + } +} + /** Which way the bounded wait ended: uv exited, it threw, or the launch stopped * waiting for a uv that would not stop. */ type InstallOutcome = @@ -102,9 +151,20 @@ export function planAgentRequirementsInstall( export async function installAgentRequirements( plan: AgentRequirementsInstall, sendOutput: (text: string) => void, - signal?: AbortSignal + signal?: AbortSignal, + onStatus?: (status: AgentInstallStatus) => void ): Promise { sendOutput('\nInstalling agent requirements…\n') + // uv's own output is the only progress signal available: the download is a + // single opaque stretch otherwise, and the row would sit on one caption for + // its whole duration. + const scan = onStatus ? scanForStatus(onStatus) : undefined + const stream = scan + ? (text: string): void => { + scan(text) + sendOutput(text) + } + : sendOutput const uvAbort = new AbortController() const onLaunchAbort = (): void => uvAbort.abort() let timedOut = false @@ -138,7 +198,7 @@ export async function installAgentRequirements( plan.pythonPath, plan.installPath, '.launch-agent-reqs.txt', - sendOutput, + stream, uvAbort.signal, settings.getMirrorConfig() ).then( @@ -152,12 +212,15 @@ export async function installAgentRequirements( // cancellation rather than a failure worth showing. if (signal?.aborted) return if (outcome.kind === 'abandoned') { + onStatus?.({ kind: 'failed' }) sendOutput( `\n⚠ agent requirements install exceeded ${INSTALL_TIMEOUT_MS / 1000}s and uv did not stop; starting ComfyUI anyway\n` ) } else if (outcome.kind === 'failed') { + onStatus?.({ kind: 'failed' }) sendOutput(`⚠ ${AGENT_REQUIREMENTS} failed: ${(outcome.error as Error).message}\n`) } else if (outcome.result.code !== 0) { + onStatus?.({ kind: 'failed' }) // The exit code decides, not the timer that was racing it: an install // that finished inside the grace period succeeded, however close to the // ceiling it landed, and must not be reported as skipped. diff --git a/src/main/lib/ipc/sessionActions/launch.test.ts b/src/main/lib/ipc/sessionActions/launch.test.ts index 953481c8b..6463a6678 100644 --- a/src/main/lib/ipc/sessionActions/launch.test.ts +++ b/src/main/lib/ipc/sessionActions/launch.test.ts @@ -185,6 +185,7 @@ import type * as HardwareTapModule from '../../hardwareTap' import type * as PipModule from '../../pip' import { getUvPath, getVenvPythonPath } from '../../pythonEnv' import { getLogDir } from '../../logRotation' +import en from '../../../../../locales/en.json' const installOf = (sourceId: string) => ({ sourceId }) as InstallationRecord @@ -1706,3 +1707,22 @@ describe('agent requirements at launch', () => { expect(spawned).toBe(0) }) }) + +describe('agent install status strings', () => { + // `t()` resolves nothing under vitest (i18n is never initialised and its + // locales dir does not exist in the source tree), so the lookup itself cannot + // be exercised here. What can break silently is the contract between the + // mapper's keys and the locale file, which is what this pins. Cross-locale + // parity is covered separately by the locale-coverage suite. + const strings = (en as { launch: { agentRequirements?: Record } }).launch + .agentRequirements + + it('defines every key the mapper asks for', () => { + expect(Object.keys(strings ?? {}).sort()).toEqual(['downloading', 'failed', 'installing']) + }) + + it('gives the download caption both placeholders the mapper passes', () => { + expect(strings?.downloading).toContain('{name}') + expect(strings?.downloading).toContain('{size}') + }) +}) diff --git a/src/main/lib/ipc/sessionActions/launch.ts b/src/main/lib/ipc/sessionActions/launch.ts index aa3f93d3f..cb75b3b7b 100644 --- a/src/main/lib/ipc/sessionActions/launch.ts +++ b/src/main/lib/ipc/sessionActions/launch.ts @@ -94,6 +94,7 @@ import { installAgentRequirements, planAgentRequirementsInstall } from '../../agentRequirementsLaunch' +import type { AgentInstallStatus } from '../../agentRequirementsLaunch' import { recoverInterruptedComfyOp } from '../../opMarker' import { waitLaunchSpawnHold } from '../../e2eOverrides' import { migrateEnvLayout } from '../../../sources/standalone/install' @@ -278,6 +279,22 @@ export function emitCoreBetaTelemetry(input: { telemetry.emit('comfy.desktop.core_beta.opt_state', { opted_in: input.optedIn }) } +/** Launch-row text for the agent install. uv already formats the size, so it is + * passed through rather than re-rendered. */ +export function agentInstallStatusText(status: AgentInstallStatus): string { + switch (status.kind) { + case 'downloading': + return i18n.t('launch.agentRequirements.downloading', { + name: status.name, + size: status.size + }) + case 'installing': + return i18n.t('launch.agentRequirements.installing') + case 'failed': + return i18n.t('launch.agentRequirements.failed') + } +} + export interface StorageLaunchState { preLaunchExtras: string[] manageModelFolders: boolean @@ -1036,7 +1053,12 @@ async function runLaunch( await installAgentRequirements( agentRequirements, makeSendOutput(sender, installationId), - abort.signal + abort.signal, + (status) => + sendProgress('agentRequirements', { + percent: -1, + status: agentInstallStatusText(status) + }) ) if (abort.signal.aborted) return { ok: false, cancelled: true } }