From f65de3fea4f64fc14737521ce2bc934871261230 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Thu, 22 Oct 2026 20:14:14 -0700 Subject: [PATCH] fix(jobs): require an admission class, admit holds on entry, true Activity states - JobRegistry.start raises TypeError when neither admission nor record_changing is given, so a new job never runs as "read" beside a scan by omission. - hold() admits when its block is entered, so a hold that is never entered leaves no running records job behind; callers that answer a refusal in their own way check blocker() first, with no await before the block. - A held SSE operation that ends in an error event (thumbnail generation, legacy batch auto-tag) is listed as failed, and refused input (duplicate delete without paths, an unreadable thumbnail body) is answered before the hold, so it is not listed as a job. - Activity keeps a job's cancellable flag (true when an older server omits it) and shows Cancel only for a running, cancellable job, in Recent work and in the job view. A held legacy job no longer offers a Cancel that does nothing. --- py/api/__init__.py | 11 +-- py/api/admin.py | 27 +++--- py/api/autotag_routes.py | 28 +++--- py/api/images.py | 38 ++++---- py/api/jobs.py | 44 ++++++--- py/api/lora_integration.py | 17 ++-- tests/browser/workspace-activity.spec.mjs | 25 +++++ tests/browser/workspace_activity_fixture.py | 35 +++++++ tests/js/workspace-activity-store.test.js | 7 +- tests/test_job_admission.py | 102 ++++++++++++++++---- web/js/workspace/activity-job-view.mjs | 3 +- web/js/workspace/activity-store.mjs | 2 + web/js/workspace/activity.mjs | 8 +- 13 files changed, 252 insertions(+), 95 deletions(-) diff --git a/py/api/__init__.py b/py/api/__init__.py index a7ef474..7735d43 100644 --- a/py/api/__init__.py +++ b/py/api/__init__.py @@ -34,7 +34,7 @@ from .images import ImageRoutesMixin from .admin import AdminRoutesMixin from .job_routes import JobRoutesMixin from .image_tag_jobs import ImageTagJobsMixin -from .jobs import JobBusy, JobRegistry +from .jobs import JobRegistry from .logging_routes import LoggingRoutesMixin from .autotag_routes import AutotagRoutesMixin from .lora_integration import LoraIntegrationMixin @@ -370,11 +370,10 @@ class PromptManagerAPI( legacy 409 envelope names the running job and nothing is done. A response of 400 or above ends the hold ``failed``. """ - try: - held = self._jobs.hold(kind, title, admission=admission) - except JobBusy as busy: - return self._long_job_busy_response(busy.running.title) - async with held as job: + busy = self._jobs.blocker(kind, admission=admission) + if busy is not None: + return self._long_job_busy_response(busy.title) + async with self._jobs.hold(kind, title, admission=admission) as job: response = await operation() if getattr(response, "status", 200) >= 400: job.finish("failed") diff --git a/py/api/admin.py b/py/api/admin.py index 7c1ace6..f8bcdfb 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -825,8 +825,18 @@ class AdminRoutesMixin: """Delete duplicate image files from disk. Single flight with the duplicate scan, and a records hold on the - registry, so no scan, restore or tagging job runs meanwhile. + registry, so no scan, restore or tagging job runs meanwhile. A body + without paths is refused before the hold (it is no job). """ + data, error_response = await _read_json(request) + if error_response is not None: + return error_response + image_paths = data.get("image_paths", []) + if not image_paths or not isinstance(image_paths, list): + return web.json_response( + {"success": False, "error": "No image paths provided"}, + status=400, + ) guard = self._acquire_long_job(self.LONG_JOB_DUPLICATES) if guard is None: return self._long_job_busy_response("Duplicate scan or delete") @@ -834,7 +844,7 @@ class AdminRoutesMixin: async def delete(): # No await since the check above: the lock is still free. async with guard: - return await self._delete_duplicate_images(request) + return await self._delete_duplicate_images(image_paths) return await self._run_held( DUPLICATE_CLEANUP_JOB_KIND, @@ -843,19 +853,8 @@ class AdminRoutesMixin: delete, ) - async def _delete_duplicate_images(self, request): + async def _delete_duplicate_images(self, image_paths): try: - data, error_response = await _read_json(request) - if error_response is not None: - return error_response - image_paths = data.get("image_paths", []) - - if not image_paths or not isinstance(image_paths, list): - return web.json_response( - {"success": False, "error": "No image paths provided"}, - status=400, - ) - output_dir = await self._run_in_executor(self._find_comfyui_output_dir) result = await self._run_in_executor( self._delete_duplicate_images_sync, image_paths, output_dir diff --git a/py/api/autotag_routes.py b/py/api/autotag_routes.py index df922e2..b3b3158 100644 --- a/py/api/autotag_routes.py +++ b/py/api/autotag_routes.py @@ -8,7 +8,7 @@ from pathlib import Path from typing import Optional from urllib.parse import quote -from .jobs import AUTOTAG_BATCH_JOB_KIND, JobBusy +from .jobs import AUTOTAG_BATCH_JOB_KIND from .prompts import thumbnail_url_for from .. import model_gate @@ -426,6 +426,8 @@ class AutotagRoutesMixin: "general_threshold and character_threshold must be numeric" ) + errors = [] # an error event ends the run; Activity lists it failed + async def events(): try: from ..autotag import get_autotag_service @@ -433,9 +435,12 @@ class AutotagRoutesMixin: async for event in self._autotag_batch_events( get_autotag_service(), opts ): + if event.get("type") == "error": + errors.append(event) yield event except Exception: self.logger.exception("AutoTag error") + errors.append(_INTERNAL_ERROR) yield { "type": "error", "message": _INTERNAL_ERROR, @@ -443,21 +448,20 @@ class AutotagRoutesMixin: # The batch writes prompt tags: a records hold on the registry, so it # never runs beside a scan, restore or the workspace tagging job. - try: - held = self._jobs.hold( - AUTOTAG_BATCH_JOB_KIND, - "Auto-tag images (legacy page)", - admission="records", - ) - except JobBusy as busy: - if busy.running.kind == AUTOTAG_BATCH_JOB_KIND: # unchanged payload + busy = self._jobs.blocker(AUTOTAG_BATCH_JOB_KIND, admission="records") + if busy is not None: + if busy.kind == AUTOTAG_BATCH_JOB_KIND: # unchanged payload return web.json_response( {"success": False, "error": _BUSY_ERROR}, status=409 ) - return self._long_job_busy_response(busy.running.title) - async with held as job: + return self._long_job_busy_response(busy.title) + async with self._jobs.hold( + AUTOTAG_BATCH_JOB_KIND, + "Auto-tag images (legacy page)", + admission="records", + ) as job: response = await self._start_autotag_batch(request, opts, events) - if response.status >= 400: + if response.status >= 400 or errors: job.finish("failed") return response diff --git a/py/api/images.py b/py/api/images.py index bf271a5..69ad96f 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -19,7 +19,7 @@ from typing import Callable, NamedTuple from aiohttp import web from PIL import Image, UnidentifiedImageError -from .jobs import THUMBNAILS_JOB_KIND, JobBusy +from .jobs import THUMBNAILS_JOB_KIND from .prompts import bad_request, parse_page_params, publish_image_paths from .prompts import thumbnail_url_for from .prompts import safe_error_message as _safe_error @@ -1744,20 +1744,21 @@ class ImageRoutesMixin: """Generate every thumbnail in one executor job (POST, blocking). Holds the registry as guarded work: refused while a record-changing - job runs, and such jobs are refused while it runs. + job runs, and such jobs are refused while it runs. A body that cannot + be read is refused before the hold, so it is never listed as a job. """ + data, error = await _json_body_or_error(request) + if error is not None: + return error return await self._run_held( THUMBNAILS_JOB_KIND, THUMBNAIL_GENERATION_TITLE, "guarded", - lambda: self._generate_thumbnails(request), + lambda: self._generate_thumbnails(data), ) - async def _generate_thumbnails(self, request): + async def _generate_thumbnails(self, data): try: - data, error = await _json_body_or_error(request) - if error is not None: - return error thumbnail_size = self._thumbnail_size(data.get("quality", "medium")) output_dirs = self._get_all_output_dirs() @@ -1952,31 +1953,32 @@ class ImageRoutesMixin: "Connection": "keep-alive", }, ) - try: - held = self._jobs.hold( - THUMBNAILS_JOB_KIND, THUMBNAIL_GENERATION_TITLE, admission="guarded" - ) - except JobBusy as busy: + busy = self._jobs.blocker(THUMBNAILS_JOB_KIND, admission="guarded") + if busy is not None: # The gallery reads this stream: refuse with its error event. await response.prepare(request) - message = f"{busy.running.title} already running" + message = f"{busy.title} already running" await self._emit_sse( response, "error", {"error": message, "message": message} ) return response - async with held: + async with self._jobs.hold( + THUMBNAILS_JOB_KIND, THUMBNAIL_GENERATION_TITLE, admission="guarded" + ) as job: await response.prepare(request) - await self._thumbnail_progress_stream(response, thumbnail_size) + if not await self._thumbnail_progress_stream(response, thumbnail_size): + job.finish("failed") # an error event ended the stream return response - async def _thumbnail_progress_stream(self, response, thumbnail_size): + async def _thumbnail_progress_stream(self, response, thumbnail_size) -> bool: + """Stream one generation run; False when it ended with an error event.""" try: output_dirs = self._get_all_output_dirs() if not output_dirs: await self._emit_sse( response, "error", {"error": "ComfyUI output directory not found"} ) - return + return False await self._emit_sse( response, @@ -1994,6 +1996,7 @@ class ImageRoutesMixin: remaining=plan["remaining"], ) self.invalidate_gallery_cache() + return True except Exception: self.logger.exception("Thumbnail generation failed") await self._emit_sse( @@ -2004,6 +2007,7 @@ class ImageRoutesMixin: "message": "Thumbnail generation failed. Check server logs.", }, ) + return False async def _stream_thumbnails( self, response, targets, thumbnail_size, skipped=0, remaining=0 diff --git a/py/api/jobs.py b/py/api/jobs.py index 112c464..568a99e 100644 --- a/py/api/jobs.py +++ b/py/api/jobs.py @@ -81,7 +81,7 @@ _REFUSED_BESIDE = { } # Kinds of the legacy-page operations that hold the registry (D80; titles in -# their handlers end in "(admin page)" or "(gallery page)"). +# their handlers end in "(admin page)", "(gallery page)" or "(legacy page)"). RESTORE_JOB_KIND = "restore" MAINTENANCE_JOB_KIND = "maintenance" DUPLICATE_CLEANUP_JOB_KIND = "duplicate_cleanup" @@ -369,11 +369,18 @@ class JobRegistry: ) -> Job: """Create a job and run ``runner(job)`` as a task. - ``admission`` is one of ``ADMISSIONS`` (default ``read``); - ``record_changing=True`` is the older spelling of ``records``. Raises - JobBusy, naming the job in the way, when the admission class refuses - it (see the module docstring). Must be called from the event loop. + ``admission`` is one of ``ADMISSIONS``; ``record_changing=True`` is + the older spelling of ``records`` and ``False`` of ``read``. One of + the two is required (TypeError otherwise), so no new job runs as + ``read`` beside a scan by omission. Raises JobBusy, naming the job in + the way, when the admission class refuses it (see the module + docstring). Must be called from the event loop. """ + if admission is None and record_changing is None: + raise TypeError( + f"JobRegistry.start({kind!r}) needs an admission class " + "(admission= or record_changing=)" + ) job = self._admit( kind, title, @@ -387,18 +394,23 @@ class JobRegistry: return job def hold(self, kind, title, *, admission, scope=None): - """Register a running, non-cancellable job for a legacy-page operation. + """A running, non-cancellable job for a legacy-page operation. - Returns an async context manager yielding the job; the job ends when + Returns an async context manager. Entering it registers the job and + yields it, or raises JobBusy at once, without waiting, when the + admission class refuses it; nothing is registered until then, so a + hold that is never entered leaves no job behind. The job ends when the block exits: ``finished``, or ``failed`` when it raised (unless - the body already ended it). Raises JobBusy at once, without waiting, - when the admission class refuses it. Use it directly in ``async with`` - so the job cannot outlive the operation. + the body already ended it). A caller that answers a refusal in its + own way checks ``blocker(kind, admission=...)`` first, with no await + before the ``async with``. """ - job = self._admit( - kind, title, admission=admission, cancellable=False, scope=scope + resolve_admission(admission) # an unknown class fails at the call + return _Hold( + lambda: self._admit( + kind, title, admission=admission, cancellable=False, scope=scope + ) ) - return _Hold(job) def _admit(self, kind, title, *, admission, record_changing=None, **job_args): resolved = resolve_admission(admission, record_changing) @@ -482,10 +494,12 @@ class JobRegistry: class _Hold: """The async context manager ``JobRegistry.hold`` returns.""" - def __init__(self, job): - self._job = job + def __init__(self, admit): + self._admit = admit + self._job = None async def __aenter__(self): + self._job = self._admit() return self._job async def __aexit__(self, exc_type, exc, tb): diff --git a/py/api/lora_integration.py b/py/api/lora_integration.py index 63a87c3..18da1df 100644 --- a/py/api/lora_integration.py +++ b/py/api/lora_integration.py @@ -6,7 +6,7 @@ from pathlib import Path from aiohttp import web from ..config_store import ConfigUnreadable, RevisionConflict, get_config_store -from .jobs import LORA_IMPORT_JOB_KIND, JobBusy +from .jobs import LORA_IMPORT_JOB_KIND # Flags of POST /lora/enable; each must be a JSON boolean when present. _LORA_FLAGS = ("enabled", "trigger_words_enabled", "clear_civitai_api_key") @@ -286,20 +286,17 @@ class LoraIntegrationMixin: # The import replaces prompts and images: a records hold, so it # never runs beside a scan, restore or tagging job. - try: - held = self._jobs.hold( - LORA_IMPORT_JOB_KIND, - "LoRA import (admin page)", - admission="records", - ) - except JobBusy as busy: + busy = self._jobs.blocker(LORA_IMPORT_JOB_KIND, admission="records") + if busy is not None: await response.prepare(request) - message = f"{busy.running.title} already running" + message = f"{busy.title} already running" await send_progress({"type": "error", "message": message}) await response.write_eof() return response - async with held as job: + async with self._jobs.hold( + LORA_IMPORT_JOB_KIND, "LoRA import (admin page)", admission="records" + ) as job: await response.prepare(request) try: await self._lora_scan_stream(allowed, send_progress) diff --git a/tests/browser/workspace-activity.spec.mjs b/tests/browser/workspace-activity.spec.mjs index 80f9fcd..4479751 100644 --- a/tests/browser/workspace-activity.spec.mjs +++ b/tests/browser/workspace-activity.spec.mjs @@ -678,6 +678,31 @@ test('cancel scan ends cancelled with partial counts', async ({ page, request }) await expect(heading(page).getByRole('button', { name: 'Cancel scan', exact: true })).not.toHaveAttribute('aria-disabled', 'true'); }); +// Truth rule: a held legacy-page job (a restore from the admin page) cannot be cancelled, so neither the Activity +// list nor the job's view offers Cancel; the cancellable scan beside it keeps its own (review I3). +test('a held legacy job shows no Cancel in the list or its view', async ({ page, request }) => { + await page.setViewportSize({ width: 1600, height: 1100 }); + const response = await request.post('/__test__/jobs/hold', { data: { kind: 'restore', title: 'Database restore (admin page)' } }); + await expect(response).toBeOK(); + const held = (await response.json()).job; + expect(held.cancellable).toBe(false); + await page.goto(`${workspace}#/activity`); + const item = items(page).filter({ has: page.getByRole('link', { name: /Database restore \(admin page\)$/ }) }); + await expect(item.locator('.chip')).toHaveText('Running'); + await expect(item.locator('progress')).toHaveCount(1); + await expect(main(page).getByRole('button', { name: 'Cancel job', exact: true })).toHaveCount(0); + + await item.getByRole('link').click(); + await expect(heading(page).locator('h1')).toHaveText('Database restore (admin page)'); + await expect(heading(page).locator('.heading-actions > button')).toHaveText(['Download report']); + await expect(heading(page).getByRole('button', { name: /^Cancel/ })).toHaveCount(0); + + // When the hold ends the view shows the end, still without a Cancel. + await finish(request, held.id); + await expect(heading(page).locator('p.muted')).toHaveText(/^Finished/); + await expect(heading(page).locator('.heading-actions > button')).toHaveText(['Download report']); +}); + test('busy start names the running job', async ({ page, request }) => { await page.setViewportSize({ width: 1600, height: 1100 }); const running = await startJob(request, { title: 'Database maintenance', kind: 'maintenance' }); diff --git a/tests/browser/workspace_activity_fixture.py b/tests/browser/workspace_activity_fixture.py index c2a3d4d..41dba88 100644 --- a/tests/browser/workspace_activity_fixture.py +++ b/tests/browser/workspace_activity_fixture.py @@ -14,6 +14,9 @@ reading files, so a test sets its progress and counters exactly: the progress to every subscriber; - ``POST /__test__/jobs/{id}/finish`` ends it as ``finished``, ``cancelled`` or ``failed``; a cancel through the real route ends it ``cancelled``; +- ``POST /__test__/jobs/hold`` opens a legacy-page hold (``kind``, ``title``): + a running, non-cancellable ``records`` job, as a legacy restore or + maintenance request takes, until ``finish`` ends it; - ``POST /__test__/jobs/restart`` acts out a ComfyUI restart: the open job streams drop without a final event and the registry starts empty. """ @@ -86,6 +89,7 @@ def register_activity_commands(app, routes, api): streams = set() endings = {} + holders = set() @web.middleware async def track_streams(request, handler): @@ -163,6 +167,36 @@ def register_activity_commands(app, routes, api): endings[job.id] = ending return web.json_response({"success": True, "job": job.summary()}) + @routes.post("/__test__/jobs/hold") + async def hold_job(request): + data = await body(request) + loop = asyncio.get_running_loop() + ending = loop.create_future() + entered = loop.create_future() + + async def hold(): + async with api._jobs.hold( + str(data.get("kind", "restore")), + str(data.get("title", "Database restore (admin page)")), + admission="records", + ) as job: + entered.set_result(job) + state = await ending + if state != "finished": + job.finish(state) + + holder = asyncio.ensure_future(hold()) + holders.add(holder) + holder.add_done_callback(holders.discard) + await asyncio.wait({entered, holder}, return_when=asyncio.FIRST_COMPLETED) + if not entered.done(): + if isinstance(holder.exception(), JobBusy): + raise web.HTTPConflict() + raise holder.exception() + job = entered.result() + endings[job.id] = ending + return web.json_response({"success": True, "job": job.summary()}) + @routes.post("/__test__/jobs/{job_id}/step") async def step_job(request): job = job_or_404(request) @@ -210,6 +244,7 @@ def register_activity_commands(app, routes, api): for transport in list(streams): transport.close() tasks = [job.task for job in old.list() if job.task and not job.task.done()] + tasks += [holder for holder in holders if not holder.done()] for task in tasks: task.cancel() await asyncio.gather(*tasks, return_exceptions=True) diff --git a/tests/js/workspace-activity-store.test.js b/tests/js/workspace-activity-store.test.js index a562fec..5fce2cd 100644 --- a/tests/js/workspace-activity-store.test.js +++ b/tests/js/workspace-activity-store.test.js @@ -261,8 +261,13 @@ test('summaries are validated and the copy follows the spec', async () => { const loose = parseJob({ id: ID_A, state: 'running', progress: 140.4, processed: -1, total: 1.5, counters: { added: 'x', errors: 2 }, title: ' ', started_at: 'x', scope: [] }); assert.deepEqual(loose, { id: ID_A, kind: '', title: 'Background job', state: 'running', started_at: null, finished_at: null, - progress: 100, status_line: '', processed: 0, total: 0, counters: { added: 0, errors: 2 }, scope: {}, listed: {}, log_length: 0 }); + progress: 100, status_line: '', processed: 0, total: 0, counters: { added: 0, errors: 2 }, scope: {}, listed: {}, log_length: 0, + cancellable: true }); assert.equal(parseJob(summary({ progress: -3 })).progress, 0); + // A held legacy job cannot be cancelled; an older server sends no key, which keeps Cancel. + assert.equal(parseJob(summary({ cancellable: false })).cancellable, false); + assert.equal(parseJob(summary({ cancellable: true })).cancellable, true); + assert.equal(parseJob(summary()).cancellable, true); const scan = parseJob(summary({ progress: 42, processed: 8240, total: 19622, counters: { added: 1, errors: 1 } })); assert.deepEqual(pillParts(scan), { label: 'Scanning · 42%', files: ' · 8,240 / 19,622', text: 'Scanning · 42% · 8,240 / 19,622' }); diff --git a/tests/test_job_admission.py b/tests/test_job_admission.py index 266d71e..61cf20c 100644 --- a/tests/test_job_admission.py +++ b/tests/test_job_admission.py @@ -62,10 +62,12 @@ class TestAdmissionClasses(unittest.IsolatedAsyncioTestCase): **kwargs, ) - async def test_admission_names_and_default(self): + async def test_admission_names_and_explicit_read(self): self.assertEqual(ADMISSIONS, ("read", "guarded", "records")) registry = JobRegistry() - job = registry.start("report", "Report", scope={}, runner=_noop) + job = registry.start( + "report", "Report", admission="read", scope={}, runner=_noop + ) await asyncio.wait_for(job.task, 2) self.assertEqual(job.admission, "read") self.assertFalse(job.record_changing) @@ -81,6 +83,20 @@ class TestAdmissionClasses(unittest.IsolatedAsyncioTestCase): runner=_noop, ) + async def test_start_requires_an_admission_class(self): + # A job that forgets its class must not silently run as "read" beside + # a scan or restore: every start call decides. + registry = JobRegistry() + with self.assertRaises(TypeError) as raised: + registry.start("review", "Review", scope={}, runner=_noop) + self.assertIn("admission", str(raised.exception)) + self.assertEqual(registry.list(), []) + job = registry.start( + "review", "Review", record_changing=False, scope={}, runner=_noop + ) + await asyncio.wait_for(job.task, 2) + self.assertEqual(job.admission, "read") + async def test_record_changing_is_an_alias_for_records(self): registry = JobRegistry() gate = asyncio.Event() @@ -179,7 +195,8 @@ class TestAdmissionClasses(unittest.IsolatedAsyncioTestCase): gate = asyncio.Event() scan = self._start(registry, "scan", "records", gate) with self.assertRaises(JobBusy) as raised: - registry.hold("maintenance", "Maintenance", admission="records") + async with registry.hold("maintenance", "Maintenance", admission="records"): + self.fail("a refused hold must not run its body") self.assertIs(raised.exception.running, scan) self.assertEqual([j.kind for j in registry.list()], ["scan"]) gate.set() @@ -198,6 +215,19 @@ class TestAdmissionClasses(unittest.IsolatedAsyncioTestCase): second.finish("failed") # the body may end it itself self.assertEqual(second.state, "failed") + async def test_hold_admits_on_entry_not_at_the_call(self): + # A hold created but never entered (an await or an exception between + # the call and ``async with``) must not leave a running records job + # behind that blocks all record work until a restart. + registry = JobRegistry() + held = registry.hold("restore", "Restore", admission="records") + self.assertEqual(registry.list(), []) + self.assertIsNone(registry.blocker("scan", admission="records")) + del held + async with registry.hold("restore", "Restore", admission="records") as job: + self.assertIs(registry.active("restore"), job) + self.assertEqual(job.state, "finished") + def test_job_defaults_stay_read_and_cancellable(self): job = Job("duplicates", "Duplicate scan") self.assertEqual((job.admission, job.cancellable), ("read", True)) @@ -369,27 +399,65 @@ class TestLegacyRoutesHoldTheRegistry(_LegacyHoldCase): (resp.status, await resp.json()), (400, {"success": False, "error": "No image paths provided"}), ) - # Each hold ended with the request; a refused input shows as failed. - listed = ( - await (await self.client.request("GET", "/prompt_manager/jobs")).json() - )["jobs"] + # Each hold ended with the request; refused input never took one. + listed = await self._listed() self.assertEqual( [(j["kind"], j["state"]) for j in listed], - [ - ("duplicate_cleanup", "failed"), - ("maintenance", "finished"), - ("maintenance", "finished"), - ], + [("maintenance", "finished"), ("maintenance", "finished")], ) self.assertEqual( [j["title"] for j in listed], - [ - "Duplicate image delete (gallery page)", - "Prompt cleanup (admin page)", - "Maintenance (admin page)", - ], + ["Prompt cleanup (admin page)", "Maintenance (admin page)"], ) + async def _listed(self): + resp = await self.client.request("GET", "/prompt_manager/jobs") + return (await resp.json())["jobs"] + + +class TestHeldJobStates(_LegacyHoldCase): + """Activity lists what a held legacy operation really did (Minor 7).""" + + async def _listed(self): + resp = await self.client.request("GET", "/prompt_manager/jobs") + return [(j["kind"], j["state"]) for j in (await resp.json())["jobs"]] + + async def test_refused_input_takes_no_hold(self): + resp = await self.client.request( + "POST", "/prompt_manager/delete_duplicate_images", json={} + ) + self.assertEqual(resp.status, 400) + resp = await self.client.request( + "POST", + "/prompt_manager/images/generate-thumbnails", + data=b"{not json", + headers={"Content-Type": "application/json"}, + ) + self.assertEqual(resp.status, 400) + self.assertEqual(await self._listed(), []) + + async def test_thumbnail_stream_that_ends_in_error_is_failed(self): + with patch.object(self.api, "_get_all_output_dirs", return_value=[]): + resp = await self.client.request( + "POST", "/prompt_manager/images/generate-thumbnails/progress" + ) + text = await resp.text() + self.assertEqual(resp.status, 200) + self.assertIn("event: error", text) + self.assertEqual(await self._listed(), [("thumbnails", "failed")]) + + async def test_legacy_autotag_that_ends_in_error_is_failed(self): + service = MagicMock() + service.get_models_status.return_value = {} # model not downloaded + with patch("py.autotag.get_autotag_service", return_value=service): + resp = await self.client.request( + "POST", "/prompt_manager/autotag/start", json={} + ) + text = await resp.text() + self.assertEqual(resp.status, 200) + self.assertIn('"type": "error"', text) + self.assertEqual(await self._listed(), [("autotag", "failed")]) + if __name__ == "__main__": unittest.main() diff --git a/web/js/workspace/activity-job-view.mjs b/web/js/workspace/activity-job-view.mjs index 9808b7a..f853cb4 100644 --- a/web/js/workspace/activity-job-view.mjs +++ b/web/js/workspace/activity-job-view.mjs @@ -274,7 +274,8 @@ export function mountActivityJobView(main, { store, scope, api, jobId }) { const lost = job.state === 'lost'; if (lost && document.activeElement === download) parts.h1.focus(); download.disabled = lost; - if (job.state === 'running') { + // Truth rule: a job that cannot be cancelled (a held legacy-page job) shows no Cancel. + if (job.state === 'running' && job.cancellable) { if (!cancel.isConnected) parts.actions.append(cancel); } else if (cancel.isConnected) { // A focused Cancel that goes away hands its focus to the job's heading, never to the page body. diff --git a/web/js/workspace/activity-store.mjs b/web/js/workspace/activity-store.mjs index 3df6405..72eb6ab 100644 --- a/web/js/workspace/activity-store.mjs +++ b/web/js/workspace/activity-store.mjs @@ -103,6 +103,8 @@ export function parseJob(raw) { scope: plainObject(raw.scope) ? { ...raw.scope } : {}, listed, log_length: count(raw.log_length), + // A held legacy-page job cannot be cancelled; an older server sends no key (cancellable then). + cancellable: raw.cancellable !== false, }; } diff --git a/web/js/workspace/activity.mjs b/web/js/workspace/activity.mjs index 48b83c4..eb7bc14 100644 --- a/web/js/workspace/activity.mjs +++ b/web/js/workspace/activity.mjs @@ -153,11 +153,15 @@ function itemFor(job, scope) { progress.textContent = `${next.progress}%`; progress.setAttribute('aria-label', `${next.title} progress`); if (!progress.isConnected) item.insertBefore(progress, actions.isConnected ? actions : null); - if (!actions.isConnected) item.append(actions); } else { + progress.remove(); + } + // Truth rule: a job that cannot be cancelled (a held legacy-page job) shows no Cancel job. + if (running && next.cancellable) { + if (!actions.isConnected) item.append(actions); + } else if (actions.isConnected) { // A focused Cancel job that goes away hands its focus to the item's title link, never to the page body. if (actions.contains(document.activeElement)) link.focus(); - progress.remove(); actions.remove(); } };