diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 17676b8..3a5210c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -97,6 +97,11 @@ jobs: PM_COMFY_ROOT: ${{ github.workspace }}/.ci-comfyui PM_TEST_PYTHON: python run: npm run --prefix tools/vendor test:comfyui + - name: Real cached and headless execution history + env: + PM_COMFY_ROOT: ${{ github.workspace }}/.ci-comfyui + PM_TEST_PYTHON: python + run: npm run --prefix tools/vendor test:executions - name: Independent and combined pack compatibility env: PM_COMFY_ROOT: ${{ github.workspace }}/.ci-comfyui diff --git a/TESTING.md b/TESTING.md index dc1f5c9..9213a43 100644 --- a/TESTING.md +++ b/TESTING.md @@ -10,7 +10,7 @@ reviewed. Add a regression test for each bug; prove it fails before the fix. | Python tests | Domain logic, SQLite, API handlers, filesystem checks | ComfyUI host and ML runtimes where needed | | JavaScript tests | Frontend modules and controller methods | DOM/network fixtures for focused edge cases | | Viewer checks | Production Viewer.js and frontend methods in Chromium desktop/touch | Controlled images and page fixtures | -| Real ComfyUI | Pinned CPU host, actual extension, node definitions, widgets, routes and SQLite | Inference is not queued; browser startup settings and host preference writes are isolated | +| Real ComfyUI | Pinned CPU host, actual extension, node definitions, widgets, routes, SQLite and real execution lifecycle | Stock UI cases isolate host writes; the execution suite queues deterministic test-only nodes without models | | Page workflows | Production HTML, startup, click handlers, HTTP routes, SQLite, PNG files, thumbnails | ComfyUI host services and the tagging model | Page workflows cover save/edit/reload, duplicate-edit rejection, gallery image @@ -106,7 +106,7 @@ and failure screenshots/traces. Open traces with Playwright's `show-trace` tool. The real-host suite covers both Classic and Nodes 2.0 startup, switching both ways, actual database prompt selection, execution-text serialization, workflow reload, widget/style cleanup, node schemas and standalone-page opening. It does -not queue inference or certify every existing feature, arbitrary host versions, +not certify every existing feature, arbitrary host versions, GPU/model changes, cross-pack installation or visual design changes. Those still need targeted validation. @@ -197,3 +197,61 @@ Viewer, including nested modal focus, mobile layouts and per-role sending. `tests/comfyui/prompt-sender.spec.mjs` sends through the actual workspace UI into real Classic and Nodes 2.0 widgets and verifies workflow serialization without loading a workflow or queueing generation. + +## Durable execution ledger + +`npm run --prefix tools/vendor test:executions` adds ten real execution cases +(five each in Classic and Nodes 2.0) to the existing 78 stock host cases and six +pack compatibility cases. Run these suites serially because they share report +cleanup and owned host resources. Use the same pinned disposable `PM_COMFY_ROOT`, +`PM_TEST_PYTHON` and Playwright browser installation as `test:comfyui`: + +```sh +PM_COMFY_ROOT=/path/to/disposable/ComfyUI \ +PM_TEST_PYTHON=/path/to/cpu-env/bin/python \ +PLAYWRIGHT_BROWSERS_PATH=/path/to/playwright-browsers \ +npm run --prefix tools/vendor test:executions +``` + +This suite reserves `http://127.0.0.1:8196/` and never attaches to an existing +server. Each renderer worker launches a CPU/offline host with independent +configuration, database, user and output directories. The shared launcher installs +tracked snapshots of PromptManager and a tiny temporary Git-tracked fixture pack. +Only that owned test pack provides cached output, deliberate failure, an +interruptible queue gate and gate-control routes; production has no test nodes or +endpoints. Cleanup always releases the gate, stops the host and removes both +owned directories. Logs and Playwright failures appear under +`tools/vendor/test-results/executions/` and the existing CI artifact upload. + +The request-only case creates no browser or WebSocket client. Other cases verify +real `execution_cached` node lists after a failed job, both renderer switches and +workflow reload, repeated public UUIDs, interruption, graph rejection, and queued +removal before dequeue. Actual host history is checked alongside finite polling +of SQLite after the background writer. The read helper uses SQLite read-only +mode, exact UUID job/session filters and a 256-row limit; absent ledger schema is +explicit, while missing/corrupt databases and other read errors fail the suite. +Sensitive sentinels in graph inputs, extra data and error messages must never +appear in any stored row. + +For a meaningful historical failure, create a separate disposable detached +checkout at `fdba6fd0126a8a40cbd6f3e50ed4c5de8991efd4` and pass its path to +`launchExecutionHost({ repository })`. That base also predates the #211 config +import fix: after confirming an actual headless job succeeded, use +`readExecutionRuns` on its explicitly constructed owned path +`host.stateDirectory/custom_nodes/promptmanager/prompts.db`. It reports +`schemaAvailable: false` and empty rows, failing the expected succeeded-row +assertion. This historical inspection stays outside the ordinary suite, which +requires the configured database path. The launcher also accepts +`PM_EXECUTION_REPOSITORY`; omit it for the implementation worktree. Stage new +files first because snapshots use `git ls-files`. Never reset the implementation +checkout or point this harness at a live ComfyUI checkout. + +The package-qualified config import also corrects #211: the configured database +path is honored when PromptManager runs as a ComfyUI custom node. Existing files +are not moved automatically; deployments that previously used the fallback path +should set the intended database path before restarting. + +The ledger records only new dequeued jobs and observed outcomes. Existing +counters, prompts, pairs and image links are preserved, with no reconstructed old +runs. Prompt contexts, output associations and the chronological timeline remain +future work under #209. Deploying the Python changes requires a ComfyUI restart. diff --git a/docs/application-redesign.md b/docs/application-redesign.md index f1fa0f0..3107e9a 100644 --- a/docs/application-redesign.md +++ b/docs/application-redesign.md @@ -9,8 +9,8 @@ code. This plan proposes implementation order and acceptance gates. - Foundation merged in [#198](https://github.com/ComfyAssets/ComfyUI_PromptManager/pull/198). - [#200](https://github.com/ComfyAssets/ComfyUI_PromptManager/issues/200) delivers native Images browsing, physical-file details, infinite scroll, Grid/List/Masonry, Viewer.js and video playback. The workspace opens on Images; original application entry points remain available. - Shared role-aware sending and the physical-file prompt inspector merged in [#203](https://github.com/ComfyAssets/ComfyUI_PromptManager/pull/203) and [#206](https://github.com/ComfyAssets/ComfyUI_PromptManager/pull/206). -- [#208](https://github.com/ComfyAssets/ComfyUI_PromptManager/issues/208) adds the native Prompt Library: one positive parent, saved negative variants, full selected text and a shared filtered image gallery. It does not invent chronological runs from counters or scan dates. [Durable history storage](prompt-history-storage.md) must be designed and tested before the execution timeline. -- Execution history, tagging/review and System consolidation remain subsequent slices under #196. +- [#208](https://github.com/ComfyAssets/ComfyUI_PromptManager/issues/208) adds the native Prompt Library: one positive parent, saved negative variants, full selected text and a shared filtered image gallery. It does not invent chronological runs from counters or scan dates. [Durable history storage](prompt-history-storage.md) now records new job outcomes; prompt contexts, output associations and the execution timeline remain pending under #209. +- Prompt execution contexts and timeline, tagging/review and System consolidation remain subsequent slices under #196; #209 now records durable job-level outcomes. ## Scope and compatibility diff --git a/docs/prompt-history-storage.md b/docs/prompt-history-storage.md index 79b86ac..2e5440c 100644 --- a/docs/prompt-history-storage.md +++ b/docs/prompt-history-storage.md @@ -1,46 +1,62 @@ # Durable Prompt History -Status: future proposal tracked in [#209](https://github.com/ComfyAssets/ComfyUI_PromptManager/issues/209); the native library does not migrate execution storage. +Status: the durable job ledger is implemented as the first slice of +[#209](https://github.com/ComfyAssets/ComfyUI_PromptManager/issues/209). Prompt +contexts, output associations and the chronological timeline remain pending; +#209 stays open. -## What the current database proves +## Recorded jobs -- `prompts.run_count` and `prompt_pairs.run_count` are aggregate usage counters. - `utils/usage_tracking.py` counts known recipes when ComfyUI dequeues a job, - including cached node executions. Dequeue does not establish successful completion. -- `last_used_at` records the latest aggregate update. The legacy prompt migration - estimated usage from linked images, with a minimum of one; no flag distinguishes - those estimates from newer recorded uses (`database/models.py`). -- `generated_images` identifies linked files; distinct image IDs are image counts, - never run counts. Its default `generation_time` is database insertion time. -- `image_prompt_pairs` stores immutable association provenance. More than one - provenance row can describe the same image and recipe. Its `created_at` is the - association time, potentially an old-image scan. -- `prompt_pairs.created_at` is the saved recipe date. Execution PNG evidence in - `utils/pair_execution.py` records distinct text/composition values without a - durable job ID or timestamp. `PromptTracker` execution IDs remain process-local. +New jobs observed at the ComfyUI queue's dequeue boundary receive an immutable +run identity and a host-session/queue-item identity. The public ComfyUI job UUID +is a correlation field: submitting it twice records two attempts. Rejected graphs +and jobs removed from the queue before dequeue create no execution rows. Cached +jobs are recorded even when no PromptManager node runs, and headless jobs do not +require a browser or WebSocket client. -The current UI can show saved negative variants, linked images and aggregate uses -with an estimate caveat. It cannot honestly show chronological execution dots, -repeat-run branches or completed-run counts. +The additive `execution_runs` table stores dequeue time, observed callback time, +optional executor event times and an explicit outcome: `observed`, `succeeded`, +`failed`, `interrupted` or `unknown`. Matching stored terminal events determine +outcomes. Missing or conflicting terminal evidence remains unknown; a process +crash can leave an observed row without an invented completion time. Bounded +safe diagnostics retain node ID/type and exception type, never graph text, +arbitrary extra data, credentials, exception messages, traceback, inputs or +output payloads. A bounded background writer keeps storage failures from +changing host execution and reports capture gaps rather than inventing rows. -## Smallest useful future migration +Existing databases begin with zero execution rows. Migration and backup restore +preserve prompts, pairs, image links and aggregate counters; storage epochs keep +stale queued writes from being replayed into restored data. There is no +historical backfill from counters, scan dates, image timestamps or saved recipes. -Add a durable `run_events` table using a stable host-session and ComfyUI job ID -plus recipe identity. Create started events at dequeue, rather than node encoding, -so cached executions remain visible. Retain nullable recipe references and immutable -raw/effective positive, negative and composition snapshots. Record completion, -failure and interruption only when observed; otherwise retain unknown status. +[Testing](../TESTING.md#durable-execution-ledger) describes the real CPU host +matrix, historical failure proof, privacy checks and cleanup. Python deployment +requires restarting ComfyUI. -Add a deduplicated `run_images(run_id, image_id)` relationship. Multiple output -images can belong to one run, and one physical file must not manufacture multiple -runs through repeated metadata evidence. Preserve aggregate counters and existing -image provenance. Do not manufacture historical events from counters, file dates -or scans. +## What the existing UI proves -## Required migration and integration tests +`prompts.run_count` and `prompt_pairs.run_count` remain aggregate usage counters, +not completed-run counts. `last_used_at` is the latest aggregate update; legacy +migration may have estimated counts from linked images. `generated_images` +identifies linked files; image counts and insertion times are not run counts or +execution times. Image/pair association creation time can reflect an old-image +scan. A saved pair's creation time is its recipe date. -Verify old databases upgrade transactionally and idempotently, with rollback on -failure; unchanged counters and associations; cached repeat executions; failed or -cancelled jobs; batched outputs; duplicate callbacks; process restart; unknown -connected inputs; and metadata rescans that create no execution events. Cover real -Classic and Nodes 2.0 hosts before exposing run chronology. +The native Prompt Library shows saved negative variants, linked images and +aggregate uses. It does not expose the ledger as chronological dots or repeat-run +branches. `PromptTracker` identities and PNG recipe evidence remain separate +from durable host job identity. + +## Remaining history work + +Design bounded execution contexts with nullable recipe references, immutable +raw/effective positive, negative and composition snapshots and explicit +inferred/recorded/unknown provenance. Cached nodes must not inherit unrelated +recent prompt evidence. Define deletion and restore behavior before connecting +saved recipes to historical snapshots. + +Add exact output references and a deduplicated run/image relationship without +claiming cached outputs were newly created. Preserve existing image provenance +and same-path replacement protection. Only after those semantics are tested can +the timeline honestly show prompt-level execution chronology, batched outputs +and repeat-run branches. diff --git a/tests/comfyui/execution-fixture.mjs b/tests/comfyui/execution-fixture.mjs new file mode 100644 index 0000000..1966f65 --- /dev/null +++ b/tests/comfyui/execution-fixture.mjs @@ -0,0 +1,47 @@ +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import { fileURLToPath } from 'node:url'; +import { access } from 'node:fs/promises'; +import { join } from 'node:path'; +import assert from 'node:assert/strict'; +import { test as base, expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { launchExecutionHost } from './execution-host.mjs'; +import runtime from '../../tools/vendor/comfyui-runtime.json' with { type: 'json' }; + +export { launchExecutionHost }; +const run = promisify(execFile); +const reader = fileURLToPath(new URL('./read_execution_history.py', import.meta.url)); + +export async function readExecutionRuns(databasePath, jobId, sessionId) { + const args = [reader, databasePath, jobId]; + if (sessionId !== undefined) args.push('--session-id', sessionId); + const { stdout } = await run(process.env.PM_TEST_PYTHON || 'python', args, { timeout: 5000, maxBuffer: 1024 * 1024 }); + return JSON.parse(stdout); +} + +export const test = base.extend({ + execution: [async ({ playwright }, use, workerInfo) => { + const execution = await launchExecutionHost({ + logPath: fileURLToPath(new URL(`../../tools/vendor/test-results/executions/${workerInfo.project.name}-host.log`, import.meta.url)), + }); + let request; + try { + request = await playwright.request.newContext({ baseURL: 'http://127.0.0.1:8196/' }); + await expect.poll(async () => { + if (execution.host.child.exitCode !== null) throw new Error('Owned execution host exited before readiness'); + try { return (await request.get('/system_stats')).ok(); } catch { return false; } + }, { timeout: 90000 }).toBe(true); + const info = (await (await request.get('/system_stats')).json()).system; + expect(info.comfyui_version).toBe(runtime.serverVersion); + expect(info.pytorch_version).toBe(`${runtime.torchVersion}+cpu`); + expect(info.required_frontend_version).toBe(runtime.frontendVersion); + expect((await request.get('/prompt_manager/stats')).ok()).toBe(true); + await access(execution.databasePath); + await assert.rejects(access(join(execution.host.stateDirectory, 'custom_nodes/promptmanager/prompts.db')), { code: 'ENOENT' }); + await use({ ...execution, request }); + } finally { + try { if (request) await request.post('/pm_execution_fixture/gate/release', { timeout: 5000 }); } + finally { try { await request?.dispose(); } finally { await execution.dispose(); } } + } + }, { scope: 'worker', timeout: 120000 }], +}); diff --git a/tests/comfyui/execution-history.spec.mjs b/tests/comfyui/execution-history.spec.mjs new file mode 100644 index 0000000..f39fe00 --- /dev/null +++ b/tests/comfyui/execution-history.spec.mjs @@ -0,0 +1,168 @@ +import { randomUUID } from 'node:crypto'; +import { expect } from '../../tools/vendor/node_modules/@playwright/test/index.mjs'; +import { test, readExecutionRuns } from './execution-fixture.mjs'; + +const sensitive = ['PM_SECRET_PROMPT_209', 'PM_SECRET_TOKEN_209', 'PM_SECRET_TRACE_209']; +const cached = value => ({ '1': { class_type: 'PMHistoryCached', inputs: { value } } }); +const gate = value => ({ '1': { class_type: 'PMHistoryGate', inputs: { value } } }); + +async function submit(request, prompt, promptId = randomUUID()) { + const response = await request.post('/prompt', { data: { prompt, prompt_id: promptId, + extra_data: { token: sensitive[1], extra_pnginfo: { private: sensitive[2] } } } }); + expect(response.ok()).toBe(true); + expect((await response.json()).prompt_id).toBe(promptId); + return promptId; +} + +async function history(request, jobId, terminal = 'execution_success') { + let item; + await expect.poll(async () => { + const response = await request.get(`/history/${jobId}`); + expect(response.ok()).toBe(true); + item = (await response.json())[jobId]; + return item?.status?.messages?.some(([kind]) => kind === terminal) || false; + }, { timeout: 15000 }).toBe(true); + expect(item.prompt[1]).toBe(jobId); + return item; +} + +async function rows(execution, jobId, outcomes) { + let result; + await expect.poll(async () => { + result = await readExecutionRuns(execution.databasePath, jobId); + return { schemaAvailable: result.schemaAvailable, outcomes: result.rows.map(row => row.outcome) }; + }, { timeout: 10000, message: `Persisted exact-job outcomes for ${jobId}` }).toEqual({ schemaAvailable: true, outcomes }); + for (const sentinel of sensitive) expect(JSON.stringify(result.rows)).not.toContain(sentinel); + for (const row of result.rows) { + expect(row.observed_dequeue_at).toBeGreaterThan(0); + expect(row.observed_callback_at).toBeGreaterThanOrEqual(row.observed_dequeue_at); + expect(row.executor_started_at).toBeGreaterThan(0); + expect(row.terminal_event_at).toBeGreaterThanOrEqual(row.executor_started_at); + } + return result.rows; +} + +async function control(request, action) { + expect((await request.post(`/pm_execution_fixture/gate/${action}`)).ok()).toBe(true); +} + +async function entered(request) { + await expect.poll(async () => (await (await request.get('/pm_execution_fixture/gate')).json()).entered, + { timeout: 10000 }).toBe(true); +} + +test('headless actual queue execution persists without a browser or WebSocket client', async ({ execution }) => { + const id = await submit(execution.request, cached(`headless ${sensitive[0]}`)); + const item = await history(execution.request, id); + expect(item.status.status_str).toBe('success'); + expect((await rows(execution, id, ['succeeded']))[0].outcome).toBe('succeeded'); +}); + +test('cached reruns after failure survive renderer switches both ways and workflow reload', async ({ execution, page, context }, testInfo) => { + const request = execution.request; + await context.route('**/*', async route => { + const path = new URL(route.request().url()).pathname.replace(/^\/api\//, '/'); + if (path === '/settings' && route.request().method() === 'GET') { + const response = await route.fetch(); + return route.fulfill({ json: { ...await response.json(), + 'Comfy.VueNodes.Enabled': testInfo.project.metadata.initialRenderer, + 'Comfy.TutorialCompleted': true, 'Comfy.Workflow.Persist': false, + 'Comfy.Notification.ShowVersionUpdates': false } }); + } + return route.continue(); + }); + const errors = []; + page.on('pageerror', error => errors.push(error.message)); + await page.goto('/', { waitUntil: 'domcontentloaded' }); + await expect(page.getByRole('status', { name: 'Loading ComfyUI' })).toBeHidden({ timeout: 30000 }); + await page.waitForFunction(async () => { + const { app } = await import('/scripts/app.js'); + return !!app.graph && !!globalThis.LiteGraph?.registered_node_types?.PMHistoryCached; + }); + await expect.poll(() => page.evaluate(() => LiteGraph.vueNodesMode)).toBe(testInfo.project.metadata.initialRenderer); + await page.evaluate(async value => { + const { app } = await import('/scripts/app.js'); + app.graph.clear(); + const node = LiteGraph.createNode('PMHistoryCached'); + app.graph.add(node); + node.widgets.find(widget => widget.name === 'value').value = value; + }, `cache ${sensitive[0]}`); + const graph = await page.evaluate(async () => (await (await import('/scripts/app.js')).app.graphToPrompt()).output); + expect(Object.values(graph).map(node => node.class_type)).toEqual(['PMHistoryCached']); + const cachedNode = Object.keys(graph)[0]; + await history(request, await submit(request, graph)); + const failureGraph = { ...graph, '999': { class_type: 'PMHistoryFailure', inputs: { value: sensitive[2] } } }; + const failed = await submit(request, failureGraph); + const failureHistory = await history(request, failed, 'execution_error'); + expect(failureHistory.status.messages.find(([kind]) => kind === 'execution_error')[1].exception_message).toContain(sensitive[2]); + const failedRows = await rows(execution, failed, ['failed']); + expect(failedRows[0]).toMatchObject({ outcome: 'failed', node_id: '999', node_type: 'PMHistoryFailure', exception_type: 'ValueError' }); + for (const renderer of [!testInfo.project.metadata.initialRenderer, testInfo.project.metadata.initialRenderer]) { + await page.evaluate(async renderer => { + const { app } = await import('/scripts/app.js'); + await app.ui.settings.setSettingValue('Comfy.VueNodes.Enabled', renderer); + await app.loadGraphData(app.graph.serialize()); + }, renderer); + await expect.poll(() => page.evaluate(() => LiteGraph.vueNodesMode)).toBe(renderer); + const reloaded = await page.evaluate(async () => (await (await import('/scripts/app.js')).app.graphToPrompt()).output); + const id = await submit(request, reloaded); + const item = await history(request, id); + expect(item.status.messages.find(([kind]) => kind === 'execution_cached')[1].nodes).toContain(cachedNode); + expect((await rows(execution, id, ['succeeded']))[0].outcome).toBe('succeeded'); + } + expect(errors).toEqual([]); +}); + +test('reusing a public UUID creates two distinct persisted attempts', async ({ execution }) => { + const id = randomUUID(); + await history(execution.request, await submit(execution.request, cached('first reused UUID'), id)); + await rows(execution, id, ['succeeded']); + const graph = cached('second reused UUID'); + await submit(execution.request, graph, id); + await expect.poll(async () => { + const response = await execution.request.get(`/history/${id}`); + return (await response.json())[id]?.prompt[2]?.['1']?.inputs?.value; + }, { timeout: 15000 }).toBe('second reused UUID'); + const attempts = await rows(execution, id, ['succeeded', 'succeeded']); + expect(attempts).toHaveLength(2); + expect(new Set(attempts.map(row => row.run_id)).size).toBe(2); + expect(new Set(attempts.map(row => row.queue_item_id)).size).toBe(2); + expect(new Set(attempts.map(row => row.session_id)).size).toBe(1); +}); + +test('real worker interruption persists an interrupted outcome', async ({ execution }) => { + const request = execution.request; + await control(request, 'hold'); + try { + const id = await submit(request, gate(`interrupt ${sensitive[0]}`)); + await entered(request); + expect((await request.post('/interrupt', { data: {} })).ok()).toBe(true); + await history(request, id, 'execution_interrupted'); + expect((await rows(execution, id, ['interrupted']))[0].outcome).toBe('interrupted'); + } finally { await control(request, 'release'); } +}); + +test('rejected and removed-before-dequeue jobs create no persisted attempts', async ({ execution }) => { + const request = execution.request; + const rejected = randomUUID(); + const response = await request.post('/prompt', { data: { prompt_id: rejected, + prompt: { '1': { class_type: 'PMHistoryMissing', inputs: {} } } } }); + expect(response.status()).toBe(400); + await control(request, 'hold'); + try { + const blocking = await submit(request, gate('hold worker until queued item is deleted')); + await entered(request); + const removed = await submit(request, cached('must never dequeue')); + const queued = (await (await request.get('/queue')).json()).queue_pending; + expect(queued.some(item => item[1] === removed)).toBe(true); + expect((await request.post('/queue', { data: { delete: [removed] } })).ok()).toBe(true); + expect((await (await request.get('/queue')).json()).queue_pending.some(item => item[1] === removed)).toBe(false); + await control(request, 'release'); + await history(request, blocking); + await rows(execution, blocking, ['succeeded']); + for (const id of [rejected, removed]) { + expect((await (await request.get(`/history/${id}`)).json())[id]).toBeUndefined(); + expect((await rows(execution, id, [])).length).toBe(0); + } + } finally { await control(request, 'release'); } +}); diff --git a/tests/comfyui/execution-host.mjs b/tests/comfyui/execution-host.mjs new file mode 100644 index 0000000..0e86aa8 --- /dev/null +++ b/tests/comfyui/execution-host.mjs @@ -0,0 +1,41 @@ +import { execFileSync } from 'node:child_process'; +import { copyFile, mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { createServer } from 'node:net'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { launchSharedHost as launchComfyHost, preparePromptManagerPlugin } from '../../tools/vendor/comfyui-host.mjs'; +import runtime from '../../tools/vendor/comfyui-runtime.json' with { type: 'json' }; + +const repositoryRoot = fileURLToPath(new URL('../../', import.meta.url)); + +export async function launchExecutionHost({ checkout = process.env.PM_COMFY_ROOT, + repository = process.env.PM_EXECUTION_REPOSITORY || repositoryRoot, + python = process.env.PM_TEST_PYTHON || 'python', port = 8196, pins = runtime, + logPath = join(repositoryRoot, 'tools/vendor/test-results/executions/host.log'), forwardOutput = true } = {}) { + if (port !== 8196) throw new Error('Execution tests reserve managed loopback port 8196'); + await new Promise((resolve, reject) => { + const probe = createServer(); + probe.once('error', error => reject(new Error('Execution loopback port 8196 is already in use', { cause: error }))); + probe.listen({ host: '127.0.0.1', port, exclusive: true }, () => probe.close(error => error ? reject(error) : resolve())); + }); + const fixtureDirectory = await mkdtemp(join(tmpdir(), 'pm-execution-fixture-')); + let host; + try { + await copyFile(new URL('./fixtures/ledger_nodes.py', import.meta.url), join(fixtureDirectory, '__init__.py')); + execFileSync('git', ['init', '--quiet', fixtureDirectory]); + execFileSync('git', ['-C', fixtureDirectory, 'add', '__init__.py']); + host = await launchComfyHost({ checkout, repository, python, port, pins, logPath, forwardOutput, + pluginName: 'promptmanager', preparePlugin: preparePromptManagerPlugin, + additionalPlugins: [{ pluginName: 'pm_execution_fixture', repository: fixtureDirectory }], + }); + } catch (error) { + await rm(fixtureDirectory, { recursive: true, force: true }); + throw error; + } + host.finished.finally(() => rm(fixtureDirectory, { recursive: true, force: true })).catch(() => {}); + return { host, fixtureDirectory, databasePath: join(host.stateDirectory, 'prompts.db'), async dispose() { + try { await host.stop(); } + finally { await rm(fixtureDirectory, { recursive: true, force: true }); } + } }; +} diff --git a/tests/comfyui/fixtures/ledger_nodes.py b/tests/comfyui/fixtures/ledger_nodes.py new file mode 100644 index 0000000..785fa21 --- /dev/null +++ b/tests/comfyui/fixtures/ledger_nodes.py @@ -0,0 +1,71 @@ +"""Deterministic nodes and queue controls installed only in an owned test host.""" + +import threading +import time + +from aiohttp import web +import comfy.model_management +from server import PromptServer + +_GATE = threading.Event() +_GATE.set() +_ENTERED = threading.Event() + + +@PromptServer.instance.routes.post("/pm_execution_fixture/gate/{action}") +async def control_gate(request: web.Request) -> web.Response: + action = request.match_info["action"] + if action == "hold": + _ENTERED.clear() + _GATE.clear() + elif action == "release": + _GATE.set() + else: + raise web.HTTPBadRequest() + return web.json_response({"held": not _GATE.is_set()}) + + +@PromptServer.instance.routes.get("/pm_execution_fixture/gate") +async def gate_status(request: web.Request) -> web.Response: + return web.json_response({"entered": _ENTERED.is_set(), "held": not _GATE.is_set()}) + + +class PMHistoryCached: + @classmethod + def INPUT_TYPES(cls) -> dict: + return {"required": {"value": ("STRING", {"default": "deterministic"})}} + + RETURN_TYPES = ("STRING",) + FUNCTION = "execute" + CATEGORY = "testing/execution-history" + OUTPUT_NODE = True + + def execute(self, value: str) -> dict: + return {"ui": {"text": [value]}, "result": (value,)} + + +class PMHistoryFailure(PMHistoryCached): + def execute(self, value: str) -> dict: + raise ValueError(value) + + +class PMHistoryGate(PMHistoryCached): + @classmethod + def IS_CHANGED(cls, **kwargs: object) -> float: + return float("nan") + + def execute(self, value: str) -> dict: + _ENTERED.set() + deadline = time.monotonic() + 30 + while not _GATE.wait(0.05): + comfy.model_management.throw_exception_if_processing_interrupted() + if time.monotonic() >= deadline: + raise TimeoutError("Owned execution fixture gate timed out") + return super().execute(value) + + +NODE_CLASS_MAPPINGS = { + "PMHistoryCached": PMHistoryCached, + "PMHistoryFailure": PMHistoryFailure, + "PMHistoryGate": PMHistoryGate, +} diff --git a/tests/comfyui/read_execution_history.py b/tests/comfyui/read_execution_history.py new file mode 100644 index 0000000..f2ac22a --- /dev/null +++ b/tests/comfyui/read_execution_history.py @@ -0,0 +1,46 @@ +"""Bounded read-only SQLite inspection for the owned execution fixture.""" + +import argparse +import json +from pathlib import Path +import sqlite3 +from uuid import UUID + + +def read_runs(database: str, job_id: str, session_id: str | None = None) -> dict: + job_id = str(UUID(job_id)) + if session_id is not None: + session_id = str(UUID(session_id)) + uri = Path(database).resolve().as_uri() + "?mode=ro" + with sqlite3.connect(uri, uri=True, timeout=0.25) as connection: + connection.row_factory = sqlite3.Row + connection.execute("PRAGMA query_only=ON") + table = connection.execute( + "SELECT 1 FROM sqlite_master WHERE type='table' AND name='execution_runs'" + ).fetchone() + if table is None: + return {"schemaAvailable": False, "rows": []} + sql = "SELECT * FROM execution_runs WHERE job_id=?" + parameters = [job_id] + if session_id is not None: + sql += " AND session_id=?" + parameters.append(session_id) + rows = connection.execute( + sql + " ORDER BY rowid LIMIT 257", parameters + ).fetchall() + if len(rows) > 256: + raise ValueError("Execution query exceeds the 256-row bound") + return {"schemaAvailable": True, "rows": [dict(row) for row in rows]} + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("database") + parser.add_argument("job_id") + parser.add_argument("--session-id") + arguments = parser.parse_args() + print( + json.dumps( + read_runs(arguments.database, arguments.job_id, arguments.session_id) + ) + ) diff --git a/tests/vendor/comfyui-config.test.mjs b/tests/vendor/comfyui-config.test.mjs index 3895637..48c6a6c 100644 --- a/tests/vendor/comfyui-config.test.mjs +++ b/tests/vendor/comfyui-config.test.mjs @@ -2,6 +2,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { execFileSync } from 'node:child_process'; import { fileURLToPath } from 'node:url'; +import { readFileSync } from 'node:fs'; const root = fileURLToPath(new URL('../../', import.meta.url)); test('real-ComfyUI suite registers Classic and Nodes 2.0 startup projects', () => { @@ -13,3 +14,34 @@ test('real-ComfyUI suite registers Classic and Nodes 2.0 startup projects', () = assert.match(output, /\[classic-start\]/); assert.match(output, /\[nodes2-start\]/); }); + +test('execution script discovers only the new matrix and preserves all 78 stock cases', () => { + const manifest = JSON.parse(readFileSync(new URL('../../tools/vendor/package.json', import.meta.url), 'utf8')); + assert.equal(manifest.scripts['test:executions'], 'playwright test --config playwright.execution.config.mjs'); + const list = config => execFileSync(process.execPath, ['tools/vendor/node_modules/@playwright/test/cli.js', + 'test', '--config', config, '--list'], { cwd: root, encoding: 'utf8', + env: { ...process.env, PM_COMFY_ROOT: '/unused-pinned-checkout' } }); + const stock = list('tools/vendor/comfyui.config.mjs'); + assert.match(stock, /Total: 78 tests in 7 files/); + assert.doesNotMatch(stock, /execution-history/); + const executions = list('tools/vendor/playwright.execution.config.mjs'); + assert.match(executions, /\[classic-start\]/); + assert.match(executions, /\[nodes2-start\]/); + assert.match(executions, /Total: 10 tests in 1 file/); + assert.doesNotMatch(executions, /prompt-manager.spec/); +}); + +test('execution config refuses live-host URLs and binds a managed loopback port', () => { + for (const url of ['http://127.0.0.1:8188/', 'https://127.0.0.1:8196/', 'http://localhost:8196/', 'http://example.com:8196/']) { + assert.throws(() => execFileSync(process.execPath, ['--input-type=module', '-e', + "await import('./tools/vendor/playwright.execution.config.mjs')"], { cwd: root, stdio: 'pipe', + env: { ...process.env, PM_COMFY_ROOT: '/unused', PM_EXECUTION_URL: url } }), /loopback|8196/); + } +}); + +test('CI retains stock, execution and pack suites in order with uploaded execution reports', () => { + const workflow = readFileSync(new URL('../../.github/workflows/test.yml', import.meta.url), 'utf8'); + const commands = [...workflow.matchAll(/^ run: npm run --prefix tools\/vendor (test:[a-z]+)$/gm)].map(match => match[1]); + assert.deepEqual(commands, ['test:comfyui', 'test:executions', 'test:compat']); + assert.match(workflow, /path: tools\/vendor\/test-results\//); +}); diff --git a/tests/vendor/comfyui-host.test.mjs b/tests/vendor/comfyui-host.test.mjs index fb5c9cd..8bbfa19 100644 --- a/tests/vendor/comfyui-host.test.mjs +++ b/tests/vendor/comfyui-host.test.mjs @@ -1,10 +1,11 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { execFileSync } from 'node:child_process'; -import { mkdtemp, writeFile, mkdir, readFile, realpath, rm, access } from 'node:fs/promises'; +import { mkdtemp, writeFile, mkdir, readFile, realpath, rm, access, readdir } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { once } from 'node:events'; +import { createServer } from 'node:net'; import { launchComfyHost } from '../../tools/vendor/comfyui-host.mjs'; async function fixture(t, program) { @@ -59,3 +60,71 @@ test('PromptManager adapter isolates database, configuration and monitored direc await access(join(options.repository, '__init__.py')); }); + +test('execution host adds only the tracked fixture pack and removes all owned state', async t => { + const { launchExecutionHost } = await import('../comfyui/execution-host.mjs'); + const options = await fixture(t, idle); + const execution = await launchExecutionHost({ ...options, port: 8196 }); + t.after(() => execution.dispose()); + await once(execution.host.child.stdout, 'data'); + const args = execution.host.child.spawnargs; + assert.deepEqual(args.slice(args.indexOf('--whitelist-custom-nodes') + 1), ['promptmanager', 'pm_execution_fixture']); + assert.equal(execution.databasePath, join(execution.host.stateDirectory, 'prompts.db')); + const installed = join(execution.host.stateDirectory, 'custom_nodes/pm_execution_fixture'); + assert.match(await readFile(join(installed, '__init__.py'), 'utf8'), /PMHistoryCached/); + assert.deepEqual(await readdir(installed), ['__init__.py']); + const source = execution.fixtureDirectory; + assert.notEqual(await realpath(source), await realpath(options.repository)); + await execution.dispose(); + await assert.rejects(access(execution.host.stateDirectory), { code: 'ENOENT' }); + await assert.rejects(access(source), { code: 'ENOENT' }); + await access(join(options.repository, '__init__.py')); +}); + +test('read-only ledger helper distinguishes absent schema from missing, corrupt and oversized data', async t => { + const { readExecutionRuns } = await import('../comfyui/execution-fixture.mjs'); + const directory = await mkdtemp(join(tmpdir(), 'pm-ledger-read-')); + t.after(() => rm(directory, { recursive: true, force: true })); + const database = join(directory, 'prompts.db'); + const python = process.env.PM_TEST_PYTHON || 'python'; + const job = '10000000-0000-4000-8000-000000000001'; + const session = '20000000-0000-4000-8000-000000000001'; + execFileSync(python, ['-c', 'import sqlite3,sys; sqlite3.connect(sys.argv[1]).close()', database]); + assert.deepEqual(await readExecutionRuns(database, job), { schemaAvailable: false, rows: [] }); + execFileSync(python, ['-c', `import sqlite3,sys +c=sqlite3.connect(sys.argv[1]) +c.execute('CREATE TABLE execution_runs (run_id TEXT, job_id TEXT, session_id TEXT, outcome TEXT)') +c.executemany('INSERT INTO execution_runs VALUES (?,?,?,?)', [('first',sys.argv[2],sys.argv[3],'succeeded'),('other','30000000-0000-4000-8000-000000000001',sys.argv[3],'failed')]) +c.commit()`, database, job, session]); + const before = await readFile(database); + assert.deepEqual(await readExecutionRuns(database, job, session), { schemaAvailable: true, rows: [ + { run_id: 'first', job_id: job, session_id: session, outcome: 'succeeded' }, + ] }); + assert.deepEqual((await readExecutionRuns(database, job, '40000000-0000-4000-8000-000000000001')).rows, []); + assert.deepEqual(await readFile(database), before); + await assert.rejects(readExecutionRuns(database, "' OR 1=1 --")); + execFileSync(python, ['-c', `import sqlite3,sys +c=sqlite3.connect(sys.argv[1]) +c.executemany('INSERT INTO execution_runs VALUES (?,?,?,?)', [(str(i),sys.argv[2],sys.argv[3],'succeeded') for i in range(256)]) +c.commit()`, database, job, session]); + await assert.rejects(readExecutionRuns(database, job), /bound|256/); + await assert.rejects(readExecutionRuns(join(directory, 'missing.db'), job)); + await writeFile(database, 'corrupt database'); + await assert.rejects(readExecutionRuns(database, job)); +}); + + +test('execution launcher refuses an occupied port without touching an existing server', async t => { + const { launchExecutionHost } = await import('../comfyui/execution-host.mjs'); + const options = await fixture(t, idle); + const server = createServer(); + await new Promise((resolve, reject) => { server.once('error', reject); server.listen(8196, '127.0.0.1', resolve); }); + let execution; + try { + await assert.rejects(async () => { execution = await launchExecutionHost({ ...options, port: 8196 }); }, /already in use/); + assert.equal(server.listening, true); + } finally { + await execution?.dispose(); + await new Promise(resolve => server.close(resolve)); + } +}); diff --git a/tools/vendor/comfyui-host.mjs b/tools/vendor/comfyui-host.mjs index 33b5168..a545487 100644 --- a/tools/vendor/comfyui-host.mjs +++ b/tools/vendor/comfyui-host.mjs @@ -4,6 +4,8 @@ import { fileURLToPath, pathToFileURL } from 'node:url'; import { launchComfyHost as launchSharedHost } from '@comfyassets/toolkit/testing/host'; import runtime from './comfyui-runtime.json' with { type: 'json' }; +export { launchSharedHost }; + const repositoryRoot = fileURLToPath(new URL('../../', import.meta.url)); export async function preparePromptManagerPlugin({ installed, stateDirectory }) { diff --git a/tools/vendor/comfyui.config.mjs b/tools/vendor/comfyui.config.mjs index c1a0c1a..1d2f206 100644 --- a/tools/vendor/comfyui.config.mjs +++ b/tools/vendor/comfyui.config.mjs @@ -14,6 +14,7 @@ const servers = [{ export default defineConfig({ testDir: '../../tests/comfyui', + testIgnore: 'execution-history.spec.mjs', outputDir: './test-results/comfyui', timeout: 60000, workers: 1, diff --git a/tools/vendor/package.json b/tools/vendor/package.json index 0a4c465..29aa97f 100644 --- a/tools/vendor/package.json +++ b/tools/vendor/package.json @@ -12,6 +12,7 @@ "test:browser": "playwright test --config playwright.config.mjs", "test:workflows": "playwright test --config playwright.workflows.config.mjs", "test:comfyui": "playwright test --config comfyui.config.mjs", + "test:executions": "playwright test --config playwright.execution.config.mjs", "test:coverage": "node frontend-coverage.mjs", "coverage:update": "node frontend-coverage.mjs --update-baseline", "test:compat": "playwright test --config pack-compat.config.mjs" diff --git a/tools/vendor/playwright.execution.config.mjs b/tools/vendor/playwright.execution.config.mjs new file mode 100644 index 0000000..c57349b --- /dev/null +++ b/tools/vendor/playwright.execution.config.mjs @@ -0,0 +1,21 @@ +import { defineConfig } from './node_modules/@playwright/test/index.mjs'; + +if (!process.env.PM_COMFY_ROOT) throw new Error('Set PM_COMFY_ROOT to a pinned disposable ComfyUI checkout'); +const host = new URL(process.env.PM_EXECUTION_URL || 'http://127.0.0.1:8196/'); +if (host.href !== 'http://127.0.0.1:8196/') throw new Error('Execution tests require managed loopback http://127.0.0.1:8196/'); + +export default defineConfig({ + testDir: '../../tests/comfyui', + testMatch: 'execution-history.spec.mjs', + outputDir: './test-results/executions/cases', + timeout: 60000, + workers: 1, + forbidOnly: Boolean(process.env.CI), + retries: 0, + use: { baseURL: host.href, browserName: 'chromium', viewport: { width: 1600, height: 1100 }, + trace: 'retain-on-failure', screenshot: 'only-on-failure' }, + projects: [ + { name: 'classic-start', metadata: { initialRenderer: false } }, + { name: 'nodes2-start', metadata: { initialRenderer: true } }, + ], +});