Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/comfybuilder/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* const client = new ComfyBuilderClient({ baseUrl, auth: tokenStoreAdapter })
* const builds = await client.listBuilds() // render tiles
* const { artifacts } = await client.getVersion(versionId)
* const artifact = selectArtifactForHost(artifacts, { os: hostOs(), gpu })
* const artifact = selectArtifactForHost(artifacts, { os: hostOs(), arch: process.arch, gpu })
* await installArtifact({ artifact, client, installPath, cacheDir, onProgress })
* const launch = buildLaunchSpec(installPath, { launchArgs })
* ```
Expand Down
56 changes: 44 additions & 12 deletions src/main/comfybuilder/targets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,40 @@ describe('hostOs', () => {

describe('selectArtifactForHost', () => {
it.each<[string, Host, string | null]>([
['exact os+gpu (windows/nvidia)', { os: 'windows', gpu: 'nvidia' }, 'windows-nvidia'],
['cpu fallback (windows host, cpu gpu)', { os: 'windows', gpu: 'cpu' }, 'windows-cpu'],
['os filter rejects a mac nvidia host', { os: 'mac', gpu: 'nvidia' }, null],
['no artifact for the host os (mac)', { os: 'mac', gpu: 'mps' }, null],
['linux nvidia', { os: 'linux', gpu: 'nvidia' }, 'linux-nvidia']
[
'exact os+gpu (windows/nvidia)',
{ os: 'windows', arch: 'x64', gpu: 'nvidia' },
'windows-nvidia'
],
[
'cpu fallback (windows host, cpu gpu)',
{ os: 'windows', arch: 'x64', gpu: 'cpu' },
'windows-cpu'
],
['os filter rejects a mac nvidia host', { os: 'mac', arch: 'arm64', gpu: 'nvidia' }, null],
['no artifact for the host os (mac)', { os: 'mac', arch: 'arm64', gpu: 'mps' }, null],
['linux nvidia', { os: 'linux', arch: 'x64', gpu: 'nvidia' }, 'linux-nvidia']
])('%s', (_name, host, expectedId) => {
expect(selectArtifactForHost(catalog, host)?.id ?? null).toBe(expectedId)
})

it('prefers exact gpu over the cpu fallback', () => {
const both = [art('linux', 'cpu'), art('linux', 'nvidia')]
expect(selectArtifactForHost(both, { os: 'linux', gpu: 'nvidia' })?.gpu).toBe('nvidia')
expect(selectArtifactForHost(both, { os: 'linux', arch: 'x64', gpu: 'nvidia' })?.gpu).toBe(
'nvidia'
)
})

it('ignores non-ready artifacts', () => {
const notReady = [art('linux', 'nvidia', { status: 'building' })]
expect(selectArtifactForHost(notReady, { os: 'linux', gpu: 'nvidia' })).toBeNull()
expect(selectArtifactForHost(notReady, { os: 'linux', arch: 'x64', gpu: 'nvidia' })).toBeNull()
})

it('an nvidia host still installs a cpu-only build', () => {
const cpuOnly = [art('windows', 'cpu')]
expect(selectArtifactForHost(cpuOnly, { os: 'windows', gpu: 'nvidia' })?.gpu).toBe('cpu')
expect(selectArtifactForHost(cpuOnly, { os: 'windows', arch: 'x64', gpu: 'nvidia' })?.gpu).toBe(
'cpu'
)
})

it('prefers the matching accelVariant among same-gpu builds', () => {
Expand All @@ -61,20 +73,40 @@ describe('selectArtifactForHost', () => {
art('linux', 'nvidia', { id: 'cu128', accelVariant: 'cu128' })
]
expect(
selectArtifactForHost(cudas, { os: 'linux', gpu: 'nvidia', accelVariant: 'cu128' })?.id
selectArtifactForHost(cudas, {
os: 'linux',
arch: 'x64',
gpu: 'nvidia',
accelVariant: 'cu128'
})?.id
).toBe('cu128')
})

it('is deterministic (not input-order dependent) when accel ties', () => {
const a = art('linux', 'nvidia', { id: 'cu118', accelVariant: 'cu118' })
const b = art('linux', 'nvidia', { id: 'cu128', accelVariant: 'cu128' })
const host = { os: 'linux', gpu: 'nvidia' } as const
const host = { os: 'linux', arch: 'x64', gpu: 'nvidia' } as const
expect(selectArtifactForHost([a, b], host)?.id).toBe('cu128')
expect(selectArtifactForHost([b, a], host)?.id).toBe('cu128')
})
})

describe('compatibleArtifactsForHost', () => {
it.each<Host>([
{ os: 'windows', arch: 'arm64', gpu: 'nvidia' },
{ os: 'windows', arch: 'arm64', gpu: 'cpu' },
{ os: 'linux', arch: 'arm64', gpu: 'nvidia' },
{ os: 'linux', arch: 'arm64', gpu: 'cpu' },
{ os: 'linux', arch: 'riscv64', gpu: 'cpu' },
{ os: 'windows', arch: 'ia32', gpu: 'cpu' },
{ os: 'mac', arch: 'arm64', gpu: 'mps' },
{ os: 'mac', arch: 'x64', gpu: 'cpu' }
])('rejects unverified architectures: $os/$arch/$gpu', (host) => {
const targets = [art(host.os, host.gpu), art(host.os, 'cpu')]
expect(compatibleArtifactsForHost(targets, host)).toEqual([])
expect(selectArtifactForHost(targets, host)).toBeNull()
})

it('returns exact GPU targets before CPU fallbacks', () => {
const targets = [
art('windows', 'cpu', { id: 'cpu' }),
Expand All @@ -83,7 +115,7 @@ describe('compatibleArtifactsForHost', () => {
]

expect(
compatibleArtifactsForHost(targets, { os: 'windows', gpu: 'nvidia' }).map(
compatibleArtifactsForHost(targets, { os: 'windows', arch: 'x64', gpu: 'nvidia' }).map(
(artifact) => artifact.id
)
).toEqual(['cuda-128', 'cuda-118', 'cpu'])
Expand All @@ -98,7 +130,7 @@ describe('compatibleArtifactsForHost', () => {
]

expect(
compatibleArtifactsForHost(targets, { os: 'windows', gpu: 'nvidia' }).map(
compatibleArtifactsForHost(targets, { os: 'windows', arch: 'x64', gpu: 'nvidia' }).map(
(artifact) => artifact.id
)
).toEqual(['ready'])
Expand Down
20 changes: 17 additions & 3 deletions src/main/comfybuilder/targets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,23 @@
*
* A version fans out into per-target artifacts (os x gpu x accel). This module
* identifies and ranks every artifact the host can run. Pure functions, no I/O;
* the caller supplies the host GPU (Desktop already detects it).
* the caller supplies the host architecture and GPU (Desktop already detects them).
*/
import type { Artifact, ArtifactGpu, ArtifactOs, Host } from './types'

/**
* Builder's Artifact API has no CPU architecture field. Its curated base images
* currently ship x64 Python for Windows and Linux; macOS is not buildable.
* Keep unknown targets blocked until Builder exposes their architecture.
* Source: Comfy-Org/cloud services/comfy-builder/sandbox-images/catalog/base_images.yaml
* and apiserver/menus/build_targets_helpers.go.
*/
const BUILDER_ARCHITECTURES: Record<ArtifactOs, NodeJS.Architecture | null> = {
linux: 'x64',
windows: 'x64',
mac: null
}

/** The host OS as a build-target token, from Node's `process.platform`. */
export function hostOs(): ArtifactOs {
switch (process.platform) {
Expand All @@ -21,7 +34,7 @@ export function hostOs(): ArtifactOs {

/**
* Rank an artifact's GPU against the host's, higher is better. An exact match
* wins; a CPU artifact is the universal fallback (every host can run it); an
* wins; a CPU artifact is the fallback within a matching OS and architecture; an
* NVIDIA host tolerates a CPU build but never the reverse.
*/
function gpuScore(artifactGpu: ArtifactGpu, hostGpu: ArtifactGpu): number {
Expand Down Expand Up @@ -50,6 +63,7 @@ function score(a: Artifact, host: Host): number {
export function compatibleArtifactsForHost(artifacts: readonly Artifact[], host: Host): Artifact[] {
return artifacts
.filter((artifact) => artifact.status === 'ready' && artifact.os === host.os)
.filter((artifact) => BUILDER_ARCHITECTURES[artifact.os] === host.arch)
.filter((artifact) => score(artifact, host) > 0)
.sort((a, b) => {
const scoreDifference = score(b, host) - score(a, host)
Expand All @@ -61,7 +75,7 @@ export function compatibleArtifactsForHost(artifacts: readonly Artifact[], host:
}

/**
* Pick the best `ready` artifact for the host: OS must match, then GPU fit
* Pick the best `ready` artifact for the host: OS and architecture must match, then GPU fit
* (exact, else CPU fallback), then a preferred `accelVariant`, then a
* deterministic tie-break. Returns null when the version has no runnable
* artifact for this machine (e.g. a windows-only build on mac).
Expand Down
1 change: 1 addition & 0 deletions src/main/comfybuilder/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export interface Artifact {
/** The machine an install targets: which artifact to pick. */
export interface Host {
os: ArtifactOs
arch: NodeJS.Architecture
gpu: ArtifactGpu
/** Preferred accelerator build (e.g. `cu128`) when a gpu ships several. Optional. */
accelVariant?: string
Expand Down
56 changes: 55 additions & 1 deletion src/main/devplatform/builds.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,18 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
import {
listCompleteVersions,
listBuildRows,
resolveHost,
resolveHostArtifact,
resolveHostArtifactForVersion,
resolveSelectedHostArtifact
} from './builds'
import { clearVersionCache, getCachedVersions } from './versionCache'
import type { Artifact, Build, BuildVersion, Host } from '../comfybuilder'
import { detectGPUCached } from '../lib/gpu'

const HOST: Host = { os: 'linux', gpu: 'nvidia' }
vi.mock('../lib/gpu', () => ({ detectGPUCached: vi.fn() }))

const HOST: Host = { os: 'linux', arch: 'x64', gpu: 'nvidia' }

beforeEach(() => clearVersionCache())

Expand Down Expand Up @@ -46,6 +50,56 @@ function stubClient(opts: {
}
}

describe('resolveHost', () => {
it('includes the process architecture alongside detected OS and GPU', async () => {
vi.mocked(detectGPUCached).mockResolvedValueOnce({ id: 'nvidia', label: 'NVIDIA', model: null })
expect(await resolveHost()).toEqual({
os:
process.platform === 'win32' ? 'windows' : process.platform === 'darwin' ? 'mac' : 'linux',
arch: process.arch,
gpu: 'nvidia'
})
})
})

describe.each(['windows', 'linux'] as const)('Managed Builds on %s ARM64', (os) => {
const host: Host = { os, arch: 'arm64', gpu: 'nvidia' }
const client = () =>
stubClient({
builds: [{ id: 'd1', name: 'Build' }],
versionsByBuild: { d1: [version(2, 'complete')] },
artifactsByVersion: {
v2: [
artifact({ id: 'cuda', os }),
artifact({ id: 'cpu', os, gpu: 'cpu', accelVariant: 'cpu' })
]
}
})

it.each([undefined, 1])(
'blocks installation and updates (installed version: %s)',
async (installedVersion) => {
const installed =
installedVersion === undefined ? undefined : new Map([['d1', installedVersion]])
const [row] = await listBuildRows(client() as never, host, installed)
expect(row).toMatchObject({
state: 'platform-mismatch',
blockedReason: 'noArtifactForMachine'
})
expect(row?.releaseTargets).toBeUndefined()
}
)

it('rejects latest, explicitly selected, and update artifacts', async () => {
const api = client()
expect(await resolveHostArtifact(api as never, host, 'd1')).toBeNull()
for (const id of ['cuda', 'cpu']) {
expect(await resolveSelectedHostArtifact(api as never, host, 'd1', 2, id)).toBeNull()
}
expect(await resolveHostArtifactForVersion(api as never, host, 'd1', 2)).toBeNull()
})
})

describe('listBuildRows', () => {
it('marks a build installable when the latest complete version has a host artifact', async () => {
const client = stubClient({
Expand Down
6 changes: 3 additions & 3 deletions src/main/devplatform/builds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ export interface ResolvedHostArtifact {
version: number
}

/** The signed-in host's build target: OS from the platform, GPU from detection. */
/** The signed-in host's build target: OS/architecture from Node, GPU from detection. */
export async function resolveHost(): Promise<Host> {
const gpu = await detectGPUCached()
// The library targets nvidia/amd/cpu/mps; an Intel dGPU (or none) maps to the
// universal CPU build, which `selectArtifactForHost` treats as the fallback.
// CPU build, which `selectArtifactForHost` treats as the fallback on the same architecture.
const mapped = gpu?.id === 'nvidia' || gpu?.id === 'amd' || gpu?.id === 'mps' ? gpu.id : 'cpu'
return { os: hostOs(), gpu: mapped }
return { os: hostOs(), arch: process.arch, gpu: mapped }
}

/** Latest complete version, or null. `complete` is the only terminal status in
Expand Down
4 changes: 2 additions & 2 deletions src/main/lib/ipc/registerDevPlatformHandlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const mocks = vi.hoisted(() => ({
})),
listBuilds: vi.fn(),
createBuildDraft: vi.fn(),
resolveHost: vi.fn(async () => ({ os: 'linux', gpu: 'nvidia' })),
resolveHost: vi.fn(async () => ({ os: 'linux', arch: 'x64', gpu: 'nvidia' })),
resolveBuildRows: vi.fn(),
resolveHostArtifact: vi.fn(),
resolveSelectedHostArtifact: vi.fn(),
Expand Down Expand Up @@ -726,7 +726,7 @@ describe('registerDevPlatformHandlers', () => {
expect(result).toMatchObject({ ok: true })
expect(mocks.resolveSelectedHostArtifact).toHaveBeenCalledWith(
expect.anything(),
{ os: 'linux', gpu: 'nvidia' },
{ os: 'linux', arch: 'x64', gpu: 'nvidia' },
'd1',
7,
'art-cpu'
Expand Down
24 changes: 22 additions & 2 deletions src/main/sources/comfybuilder/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ vi.mock('./modelStagingTask', () => ({
restageBuildModelsIfNeeded: vi.fn()
}))
vi.mock('../../devplatform/builds', () => ({
resolveHost: vi.fn(async () => ({ os: 'linux', gpu: 'nvidia' })),
resolveHost: vi.fn(async () => ({ os: 'linux', arch: 'x64', gpu: 'nvidia' })),
resolveHostArtifactForVersion: vi.fn(),
listCompleteVersions: vi.fn(async () => [])
}))
Expand All @@ -61,7 +61,11 @@ import fs, { promises as fsp } from 'fs'
import os from 'os'
import path from 'path'
import { installArtifact, stageModels, resolveModelManifest, venvPython } from '../../comfybuilder'
import { listCompleteVersions, resolveHostArtifactForVersion } from '../../devplatform/builds'
import {
listCompleteVersions,
resolveHost,
resolveHostArtifactForVersion
} from '../../devplatform/builds'
import {
clearVersionCache,
getCachedVersions,
Expand Down Expand Up @@ -168,6 +172,22 @@ describe('comfybuilder.install wiring', () => {
expect(releaseInstallTerminalForFsOp).toHaveBeenCalledWith('i1')
})

it.each(['windows', 'linux'] as const)(
'rejects a saved %s x64 selection on ARM64 before changing the environment',
async (os) => {
vi.mocked(resolveHost).mockResolvedValueOnce({ os, arch: 'arm64', gpu: 'nvidia' })
await expect(
comfybuilder.install!(record({ artifactOs: os, status: 'failed' }), fakeTools())
).rejects.toThrow('This build is not compatible with this machine.')
expect(installArtifact).not.toHaveBeenCalled()
expect(releaseInstallTerminalForFsOp).not.toHaveBeenCalled()
expect(rm).not.toHaveBeenCalled()
expect(rename).not.toHaveBeenCalled()
expect(writeFile).not.toHaveBeenCalled()
expect(startModelStaging).not.toHaveBeenCalled()
}
)

it('updates code while models and user data remain at stable paths', async () => {
access.mockImplementation(realFsp.access)
mkdir.mockImplementation(realFsp.mkdir)
Expand Down
8 changes: 7 additions & 1 deletion src/main/sources/comfybuilder/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import type {
ModelDescriptor
} from '../../comfybuilder'
import { getBuilderClient } from '../../devplatform/session'
import { selectArtifactForHost } from '../../comfybuilder/targets'
import {
listCompleteVersions,
resolveHost,
Expand Down Expand Up @@ -405,8 +406,13 @@ async function installEnvironment(
},
onTransactionStarted?: () => Promise<void>
): Promise<readonly ModelDescriptor[]> {
releaseInstallTerminalForFsOp(installation.id)
const artifact = artifactFromRecord(installation)
// Retried installs carry a persisted selection and bypass catalog resolution.
// Revalidate before releasing terminals or touching the existing environment.
if (!selectArtifactForHost([artifact], await resolveHost())) {
throw new Error('This build is not compatible with this machine.')
}
releaseInstallTerminalForFsOp(installation.id)
const client = getBuilderClient()
const paths = environmentPaths(installation.installPath)
const interrupted = await hasEnvironmentBackups(installation.installPath)
Expand Down
Loading