From 7d35875c2035b74a2672bc88cce9551dfa2303f9 Mon Sep 17 00:00:00 2001
From: Alex Sorafumo <alex@yuion.net>
Date: Fri, 2 Oct 2026 13:12:35 +1000
Subject: [PATCH 1/2] fix(signage-manager): fix brand kit saves and image
 editor flows

- Keep palette keys when saving branding.
- Write brand kit changes one at a time.
- Prevent double saves and wait for the picked image before saving.
- Cancel the job when the editor closes.
- Show an error when an option fails to load.
- Respect branding-editing for logo changes.
- Show errors that name the failed action.
- Handle three-digit hex colours and avoid extra font redraws.
---
 apps/signage-manager/USER_STORIES.md          |   2 +-
 .../src/app/branding/brand-access.ts          |  22 ++
 .../src/app/branding/branding.component.ts    |  96 ++++++---
 .../image-gen-layer-controls.component.ts     |  16 +-
 .../image-gen/image-gen-layer.component.ts    |  31 ++-
 .../image-gen/image-gen-modal.component.ts    | 178 +++++++++-------
 .../src/app/image-gen/image-gen.service.ts    |  72 +++++--
 .../src/app/image-gen/image-gen.util.ts       |  28 +--
 .../tests/branding/branding.component.spec.ts |  49 ++++-
 ...image-gen-layer-controls.component.spec.ts |  21 ++
 .../image-gen-layer.component.spec.ts         |  44 ++++
 .../image-gen-modal.component.spec.ts         | 194 ++++++++++++++++--
 .../tests/image-gen/image-gen.service.spec.ts | 168 ++++++++++++++-
 .../tests/image-gen/image-gen.util.spec.ts    |  20 +-
 shared/assets/locale/en-AU.json               |   4 +
 15 files changed, 776 insertions(+), 169 deletions(-)
 create mode 100644 apps/signage-manager/src/app/branding/brand-access.ts

diff --git a/apps/signage-manager/USER_STORIES.md b/apps/signage-manager/USER_STORIES.md
index a84e65c4226..62e974e781b 100644
--- a/apps/signage-manager/USER_STORIES.md
+++ b/apps/signage-manager/USER_STORIES.md
@@ -381,7 +381,7 @@ These stories cover the current app workflows:
   - Without `template-editing`, template create, edit, duplicate, delete and layout changes are hidden. Template mappings stay available.
   - Without `ai-generation`, the AI create actions are hidden.
   - Without `ai-editing`, "Edit with AI" and refinement in the AI editor are hidden.
-  - Without `branding-editing`, the branding tab is read-only.
+  - Without `branding-editing`, the branding tab is read-only, and the AI editor cannot change the logo.
 - The All Groups view uses only `app.features`.
 
 ---
diff --git a/apps/signage-manager/src/app/branding/brand-access.ts b/apps/signage-manager/src/app/branding/brand-access.ts
new file mode 100644
index 00000000000..f1c8b9b78ed
--- /dev/null
+++ b/apps/signage-manager/src/app/branding/brand-access.ts
@@ -0,0 +1,22 @@
+import { SignageContextService } from '../signage-context.service';
+
+type BrandAccessContext = Pick<
+    SignageContextService,
+    'is_sys_admin' | 'global_features'
+>;
+
+/**
+ * Whether brand kit edits are on. The kit is for the whole organisation, so
+ * only the global `app.features` setting turns them off, not a group.
+ */
+export function brandEditingOn(context: BrandAccessContext): boolean {
+    return (context.global_features() || []).includes('branding-editing');
+}
+
+/**
+ * Whether this user can change the brand kit, from the branding page or from
+ * the image editor. Call it inside a `computed` so it follows the signals.
+ */
+export function canEditBrandKit(context: BrandAccessContext): boolean {
+    return context.is_sys_admin() && brandEditingOn(context);
+}
diff --git a/apps/signage-manager/src/app/branding/branding.component.ts b/apps/signage-manager/src/app/branding/branding.component.ts
index 506c7081b75..438a4a9433b 100644
--- a/apps/signage-manager/src/app/branding/branding.component.ts
+++ b/apps/signage-manager/src/app/branding/branding.component.ts
@@ -26,12 +26,22 @@ import {
     ImageGenBrandKit,
     ImageGenLogoSlot,
 } from '../image-gen/image-gen.types';
-import { errorMessage } from '../image-gen/image-gen.util';
+import { actionError } from '../image-gen/image-gen.util';
 import { SignageContextService } from '../signage-context.service';
+import { brandEditingOn, canEditBrandKit } from './brand-access';
 import { BRAND_FONTS, ensureBrandFont } from './brand-fonts';
 
 const COLOUR_NAMES = ['primary', 'secondary', 'accent'];
 
+/** how many palette colours the page shows and edits */
+const MAX_COLOURS = 3;
+
+/** a palette colour and the key it is stored under */
+interface BrandColour {
+    key: string;
+    value: string;
+}
+
 @Component({
     selector: 'app-branding',
     template: `
@@ -100,7 +110,7 @@ const COLOUR_NAMES = ['primary', 'secondary', 'accent'];
                                 class="border-base-content/20 h-10 w-14 rounded border bg-transparent disabled:cursor-not-allowed disabled:opacity-60"
                                 [class.cursor-pointer]="can_edit()"
                                 [disabled]="!can_edit()"
-                                [value]="colour"
+                                [value]="colour.value"
                                 (input)="setColourFromInput($index, $event)"
                                 [attr.aria-label]="
                                     'SIGNAGE_MANAGER.BRAND_COLOURS' | translate
@@ -113,7 +123,7 @@ const COLOUR_NAMES = ['primary', 'secondary', 'accent'];
                             >
                                 <input
                                     matInput
-                                    [ngModel]="colour"
+                                    [ngModel]="colour.value"
                                     (ngModelChange)="setColour($index, $event)"
                                     [disabled]="!can_edit()"
                                     [class.text-error]="colour_errors()[$index]"
@@ -125,7 +135,7 @@ const COLOUR_NAMES = ['primary', 'secondary', 'accent'];
                             </mat-form-field>
                             <span
                                 class="text-base-content/60 text-xs uppercase"
-                                >{{ colourName($index) }}</span
+                                >{{ colour.key }}</span
                             >
                             @if (can_edit()) {
                                 <button
@@ -145,7 +155,7 @@ const COLOUR_NAMES = ['primary', 'secondary', 'accent'];
                             }
                         </div>
                     }
-                    @if (can_edit() && colours().length < 3) {
+                    @if (can_edit() && colours().length < max_colours) {
                         <button
                             mat-stroked-button
                             type="button"
@@ -330,13 +340,8 @@ export class BrandingComponent implements OnInit {
     public readonly fonts = BRAND_FONTS;
     public readonly enabled = this._image_gen.enabled;
 
-    /** The brand kit is for the whole organisation, so only the global
-     * `app.features` setting turns branding edits off */
     public readonly branding_disabled = computed(
-        () =>
-            !(this._context.global_features() || []).includes(
-                'branding-editing',
-            ),
+        () => !brandEditingOn(this._context),
     );
     /** Whether the stored brand kit is read. The form shows only after a
      * read works, so its defaults cannot replace the stored kit. */
@@ -344,14 +349,14 @@ export class BrandingComponent implements OnInit {
         'loading',
     );
     public readonly can_edit = computed(
-        () =>
-            this._context.is_sys_admin() &&
-            !this.branding_disabled() &&
-            this.load_state() === 'ready',
+        () => canEditBrandKit(this._context) && this.load_state() === 'ready',
     );
 
     public readonly organisation = signal('');
-    public readonly colours = signal<string[]>(['#0E6E52']);
+    public readonly colours = signal<BrandColour[]>([
+        { key: 'primary', value: '#0E6E52' },
+    ]);
+    public readonly max_colours = MAX_COLOURS;
     public readonly font = signal('');
     public readonly saving = signal(false);
 
@@ -362,7 +367,7 @@ export class BrandingComponent implements OnInit {
         on_dark: '',
     });
     public readonly derived = signal<ImageGenLogoSlot | ''>('');
-    /** Palette colours after the three that the page edits. A save replaces
+    /** Palette colours after the ones that the page edits. A save replaces
      * the whole kit, so they are saved back as they are. */
     private _extra_palette: Record<string, string> = {};
 
@@ -412,13 +417,31 @@ export class BrandingComponent implements OnInit {
         this.load_state.set('ready');
     }
 
-    public colourName(index: number) {
-        return COLOUR_NAMES[index] || `colour ${index + 1}`;
+    public addColour() {
+        if (this.colours().length >= MAX_COLOURS) return;
+        this.colours.update((list) => [
+            ...list,
+            { key: this._freeKey(), value: '#1B2420' },
+        ]);
     }
 
-    public addColour() {
-        if (this.colours().length >= 3) return;
-        this.colours.update((list) => [...list, '#1B2420']);
+    /** the first palette key not in use, so a new colour replaces nothing */
+    private _freeKey() {
+        const used = new Set([
+            ...this.colours().map((colour) => colour.key),
+            ...Object.keys(this._extra_palette),
+        ]);
+        // one more numbered name than keys in use, so one is always free
+        const names = [
+            ...COLOUR_NAMES,
+            ...Array.from(
+                { length: used.size + 1 },
+                (_, index) => `colour ${index + 1}`,
+            ),
+        ];
+        return (
+            names.find((name) => !used.has(name)) || `colour ${used.size + 1}`
+        );
     }
 
     public removeColour(index: number) {
@@ -430,12 +453,16 @@ export class BrandingComponent implements OnInit {
     public static readonly COLOUR = /^#([0-9a-f]{3}|[0-9a-f]{6})$/i;
 
     public readonly colour_errors = computed(() =>
-        this.colours().map((colour) => !BrandingComponent.COLOUR.test(colour)),
+        this.colours().map(
+            (colour) => !BrandingComponent.COLOUR.test(colour.value),
+        ),
     );
 
     public setColour(index: number, value: string) {
         this.colours.update((list) =>
-            list.map((colour, i) => (i === index ? value : colour)),
+            list.map((colour, i) =>
+                i === index ? { ...colour, value } : colour,
+            ),
         );
     }
 
@@ -487,7 +514,7 @@ export class BrandingComponent implements OnInit {
             notifySuccess(i18n('SIGNAGE_MANAGER.IMAGE_GEN_LOGO_SAVED'));
         } catch (error) {
             notifyError(
-                errorMessage(error, i18n('SIGNAGE_MANAGER.BRAND_SAVE_FAILED')),
+                actionError(error, i18n('SIGNAGE_MANAGER.BRAND_SAVE_FAILED')),
             );
         } finally {
             this.busy.set('');
@@ -504,7 +531,7 @@ export class BrandingComponent implements OnInit {
             notifySuccess(i18n('SIGNAGE_MANAGER.BRAND_LOGO_MADE'));
         } catch (error) {
             notifyError(
-                errorMessage(error, i18n('SIGNAGE_MANAGER.BRAND_SAVE_FAILED')),
+                actionError(error, i18n('SIGNAGE_MANAGER.BRAND_SAVE_FAILED')),
             );
         } finally {
             this.busy.set('');
@@ -519,10 +546,11 @@ export class BrandingComponent implements OnInit {
         }
         this.saving.set(true);
         try {
+            // each colour keeps the key it was read from
             const palette = { ...this._extra_palette };
-            this.colours().forEach((colour, index) => {
-                palette[this.colourName(index)] = colour;
-            });
+            for (const colour of this.colours()) {
+                palette[colour.key] = colour.value;
+            }
             await this._image_gen.saveBrandKit({
                 organisation: this.organisation().trim() || undefined,
                 palette,
@@ -531,7 +559,7 @@ export class BrandingComponent implements OnInit {
             notifySuccess(i18n('SIGNAGE_MANAGER.BRAND_SAVED'));
         } catch (error) {
             notifyError(
-                errorMessage(error, i18n('SIGNAGE_MANAGER.BRAND_SAVE_FAILED')),
+                actionError(error, i18n('SIGNAGE_MANAGER.BRAND_SAVE_FAILED')),
             );
         } finally {
             this.saving.set(false);
@@ -548,10 +576,14 @@ export class BrandingComponent implements OnInit {
             ),
         ];
         if (ordered.length) {
-            this.colours.set(ordered.slice(0, 3).map((key) => palette[key]));
+            this.colours.set(
+                ordered
+                    .slice(0, MAX_COLOURS)
+                    .map((key) => ({ key, value: palette[key] })),
+            );
         }
         this._extra_palette = Object.fromEntries(
-            ordered.slice(3).map((key) => [key, palette[key]]),
+            ordered.slice(MAX_COLOURS).map((key) => [key, palette[key]]),
         );
         const font = brand.font;
         this.font.set(typeof font === 'string' ? font : font?.family || '');
diff --git a/apps/signage-manager/src/app/image-gen/image-gen-layer-controls.component.ts b/apps/signage-manager/src/app/image-gen/image-gen-layer-controls.component.ts
index ecb31322142..e19e4ea76f2 100644
--- a/apps/signage-manager/src/app/image-gen/image-gen-layer-controls.component.ts
+++ b/apps/signage-manager/src/app/image-gen/image-gen-layer-controls.component.ts
@@ -233,10 +233,7 @@ export function newTextBlock(
             >
                 @if (!has_logo()) {
                     <span class="text-sm">{{
-                        (can_set_logo()
-                            ? 'SIGNAGE_MANAGER.IMAGE_GEN_NO_LOGO_YET'
-                            : 'SIGNAGE_MANAGER.IMAGE_GEN_NO_LOGO_ADMIN'
-                        ) | translate
+                        no_logo_note() | translate
                     }}</span>
                     @if (can_set_logo()) {
                         <button
@@ -361,6 +358,8 @@ export class ImageGenLayerControlsComponent {
     public readonly brand = input<ImageGenBrandKit | null>(null);
     public readonly uploading = input(false);
     public readonly can_set_logo = input(true);
+    /** whether the brand kit can be changed by anyone */
+    public readonly branding_editing = input(true);
 
     public readonly changed = output<ImageGenLayerState>();
     public readonly logo_picked = output<File>({ alias: 'logoPicked' });
@@ -372,6 +371,15 @@ export class ImageGenLayerControlsComponent {
         () => !!this.logo_on_light() && !!this.logo_on_dark(),
     );
 
+    /** points at the branding page only when someone can use it */
+    public readonly no_logo_note = computed(() =>
+        this.can_set_logo()
+            ? 'SIGNAGE_MANAGER.IMAGE_GEN_NO_LOGO_YET'
+            : this.branding_editing()
+              ? 'SIGNAGE_MANAGER.IMAGE_GEN_NO_LOGO_ADMIN'
+              : 'SIGNAGE_MANAGER.IMAGE_GEN_NO_LOGO_LOCKED',
+    );
+
     public readonly fonts = BRAND_FONTS;
 
     /** names the face a block falls back to */
diff --git a/apps/signage-manager/src/app/image-gen/image-gen-layer.component.ts b/apps/signage-manager/src/app/image-gen/image-gen-layer.component.ts
index 67e5e9fdbef..a3f139be7bf 100644
--- a/apps/signage-manager/src/app/image-gen/image-gen-layer.component.ts
+++ b/apps/signage-manager/src/app/image-gen/image-gen-layer.component.ts
@@ -131,6 +131,23 @@ export class ImageGenLayerComponent {
         return typeof font === 'string' ? font : font?.family || '';
     });
 
+    /**
+     * Every face in use. Equal by content, so a drag, which makes new blocks
+     * with the same fonts, does not ask for the faces again.
+     */
+    private readonly _families = computed(
+        () =>
+            [
+                ...new Set(
+                    [
+                        this._brand_family(),
+                        ...this.state().blocks.map((block) => block.font),
+                    ].filter(Boolean),
+                ),
+            ].sort(),
+        { equal: (a, b) => a.join('\n') === b.join('\n') },
+    );
+
     constructor() {
         effect(() => {
             const url = this.image_url();
@@ -146,13 +163,7 @@ export class ImageGenLayerComponent {
         });
         effect(() => {
             // a face has to be in the document before a canvas can draw with it
-            const families = new Set(
-                [
-                    this._brand_family(),
-                    ...this.state().blocks.map((block) => block.font),
-                ].filter(Boolean),
-            );
-            for (const family of families) {
+            for (const family of this._families()) {
                 ensureBrandFont(family).then(() => this._draw());
             }
         });
@@ -546,7 +557,11 @@ export class ImageGenLayerComponent {
     }
 
     private _isLight(hex: string) {
-        const value = hex.replace('#', '');
+        let value = hex.replace('#', '');
+        // #rgb is short for #rrggbb
+        if (value.length === 3) {
+            value = [...value].map((digit) => digit + digit).join('');
+        }
         if (value.length < 6) return true;
         const r = parseInt(value.slice(0, 2), 16);
         const g = parseInt(value.slice(2, 4), 16);
diff --git a/apps/signage-manager/src/app/image-gen/image-gen-modal.component.ts b/apps/signage-manager/src/app/image-gen/image-gen-modal.component.ts
index d4f2ed06325..c17e91655dc 100644
--- a/apps/signage-manager/src/app/image-gen/image-gen-modal.component.ts
+++ b/apps/signage-manager/src/app/image-gen/image-gen-modal.component.ts
@@ -1,10 +1,12 @@
 import {
     Component,
     computed,
+    effect,
     inject,
     linkedSignal,
     OnDestroy,
     signal,
+    untracked,
     viewChild,
 } from '@angular/core';
 import { FormsModule } from '@angular/forms';
@@ -29,6 +31,7 @@ import {
 } from '@placeos/components';
 import { SignageMedia } from '@placeos/ts-client';
 
+import { brandEditingOn, canEditBrandKit } from '../branding/brand-access';
 import { SignageMediaService } from '../media/signage-media.service';
 import { SignagePlaylistService } from '../playlists/signage-playlist.service';
 import { SignageContextService } from '../signage-context.service';
@@ -38,7 +41,7 @@ import {
 } from './image-gen-layer-controls.component';
 import { ImageGenLayerComponent } from './image-gen-layer.component';
 import { ImageGenReferencesComponent } from './image-gen-references.component';
-import { ImageGenService, isFinal, MAX_JOB_WAIT_MS } from './image-gen.service';
+import { ImageGenService, isFinal } from './image-gen.service';
 import {
     ImageGenEditRequest,
     ImageGenGenerateRequest,
@@ -46,7 +49,7 @@ import {
     ImageGenLayerState,
     ImageGenReference,
 } from './image-gen.types';
-import { errorMessage, orientationOf } from './image-gen.util';
+import { actionError, orientationOf } from './image-gen.util';
 
 export interface ImageGenModalData {
     /** pre-set from the playlist a user opened this from */
@@ -106,6 +109,10 @@ interface Candidate {
                                 (changed)="layer_state.set($event)"
                                 (failed)="onArtworkFailed()"
                             ></image-gen-layer>
+                        } @else if (selected()) {
+                            <!-- the pick is still being read; the source
+                                 image here would look like the result -->
+                            <mat-spinner diameter="32"></mat-spinner>
                         } @else if (source_url()) {
                             <img
                                 auth
@@ -164,7 +171,7 @@ interface Candidate {
                                 ) {
                                     <button
                                         type="button"
-                                        [disabled]="claim_pending()"
+                                        [disabled]="claim_pending() || saving()"
                                         class="border-base-content/10 h-16 w-28 shrink-0 overflow-hidden rounded border"
                                         [class.ring-2]="
                                             selected()?.upload_id ===
@@ -364,7 +371,8 @@ interface Candidate {
                                             !refinement().trim() ||
                                             !selected() ||
                                             state() === 'generating' ||
-                                            claim_pending()
+                                            claim_pending() ||
+                                            saving()
                                         "
                                         (click)="refine()"
                                     >
@@ -413,6 +421,7 @@ interface Candidate {
                                     [logo_on_dark]="logo_on_dark()"
                                     [brand]="applied_brand()"
                                     [can_set_logo]="can_set_logo()"
+                                    [branding_editing]="branding_editing()"
                                     [uploading]="uploading_logo()"
                                     (changed)="layer_state.set($event)"
                                     (logoPicked)="uploadLogo($event)"
@@ -461,7 +470,7 @@ interface Candidate {
                                 btn
                                 matRipple
                                 class="flex min-w-32 items-center justify-center gap-2"
-                                [disabled]="!selected() || saving()"
+                                [disabled]="!can_save()"
                                 (click)="save()"
                             >
                                 @if (saving()) {
@@ -585,10 +594,20 @@ export class ImageGenModalComponent implements OnDestroy {
         return style ? [style] : [];
     });
     public readonly claim_pending = signal(false);
+    /** only a pick that has loaded, so the person has seen what is saved */
+    public readonly can_save = computed(
+        () => !!this.selected_object_url() && !this.saving(),
+    );
 
     public readonly brand = this._image_gen.brand_kit;
 
-    public readonly can_set_logo = this._context.is_sys_admin;
+    /** the same rule as the branding page, as this writes the same kit */
+    public readonly can_set_logo = computed(() =>
+        canEditBrandKit(this._context),
+    );
+    public readonly branding_editing = computed(() =>
+        brandEditingOn(this._context),
+    );
 
     public readonly group_id = computed(
         () => this._context.selected_group()?.group.id || undefined,
@@ -776,7 +795,7 @@ export class ImageGenModalComponent implements OnDestroy {
             if (token !== this._job_token) return;
             this.state.set('compose');
             notifyError(
-                errorMessage(
+                actionError(
                     error,
                     i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_FAILED'),
                 ),
@@ -812,7 +831,7 @@ export class ImageGenModalComponent implements OnDestroy {
             if (token !== this._job_token) return;
             this.state.set('review');
             notifyError(
-                errorMessage(
+                actionError(
                     error,
                     i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_FAILED'),
                 ),
@@ -822,13 +841,15 @@ export class ImageGenModalComponent implements OnDestroy {
 
     private _select_token = 0;
 
+    /** the pick could not be read, so it is let go rather than saved unseen */
     public onArtworkFailed() {
+        this.selected.set(null);
         this.selected_object_url.set('');
         notifyError(i18n('SIGNAGE_MANAGER.IMAGE_GEN_IMAGE_UNREADABLE'));
     }
 
     public async select(candidate: Candidate) {
-        if (this.claim_pending()) return;
+        if (this.claim_pending() || this.saving()) return;
         const token = ++this._select_token;
         this.selected.set(candidate);
         this.selected_object_url.set('');
@@ -836,6 +857,7 @@ export class ImageGenModalComponent implements OnDestroy {
             .loadImage(candidate.url)
             .catch(() => '');
         if (token !== this._select_token) return;
+        if (!url) return this.onArtworkFailed();
         this.selected_object_url.set(url);
     }
 
@@ -923,9 +945,9 @@ export class ImageGenModalComponent implements OnDestroy {
             }
         } catch (error) {
             notifyError(
-                errorMessage(
+                actionError(
                     error,
-                    i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_FAILED'),
+                    i18n('SIGNAGE_MANAGER.IMAGE_GEN_REFERENCE_UPLOAD_FAILED'),
                 ),
             );
         } finally {
@@ -949,13 +971,21 @@ export class ImageGenModalComponent implements OnDestroy {
         this._closed = true;
         this._stopAwaiting();
 
-        const running = this.state() === 'generating';
-        for (const item of this.references()) {
-            URL.revokeObjectURL(item.url);
-            // a running job reads the reference bytes server side, so those are
-            // left for the housekeeping sweep to clear
-            if (!running) this._image_gen.removeReference(item.id);
+        for (const id of this.job_ids()) {
+            this._image_gen.setJobOnScreen(id, false);
         }
+        const references = this.references();
+        for (const item of references) URL.revokeObjectURL(item.url);
+        const reference_ids = references.map((item) => item.id);
+        if (this.state() !== 'generating') {
+            reference_ids.forEach((id) => this._image_gen.removeReference(id));
+            return;
+        }
+        // nothing can show the result once this closes, so stop the job
+        // rather than spend the quota on images no one can reach. A job the
+        // server has not answered for yet is stopped by _follow.
+        const job = this._image_gen.jobs()[this.current_job_id()];
+        if (job && !isFinal(job)) this._abandon(job.id, reference_ids);
     }
 
     public async uploadLogo(file: File) {
@@ -968,9 +998,9 @@ export class ImageGenModalComponent implements OnDestroy {
             notifySuccess(i18n('SIGNAGE_MANAGER.IMAGE_GEN_LOGO_SAVED'));
         } catch (error) {
             notifyError(
-                errorMessage(
+                actionError(
                     error,
-                    i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_FAILED'),
+                    i18n('SIGNAGE_MANAGER.IMAGE_GEN_LOGO_SAVE_FAILED'),
                 ),
             );
         } finally {
@@ -980,21 +1010,22 @@ export class ImageGenModalComponent implements OnDestroy {
 
     public async save() {
         const candidate = this.selected();
-        if (!candidate) return;
-
-        // Take the composited image before the button swaps to a spinner. A
-        // retry reuses the row the last attempt made, so needs no image.
-        const name = this._name();
-        const overlay = !this._pending && this.has_overlay();
-        const blob = overlay ? await this._layer()?.toBlob() : undefined;
-        if (overlay && !blob) {
-            notifyError(i18n('SIGNAGE_MANAGER.IMAGE_GEN_NO_IMAGE'));
-            return;
-        }
+        if (!candidate || !this.can_save()) return;
 
+        // set before the image is taken, so a second click cannot start a
+        // second save while the canvas is encoded
         this.saving.set(true);
         this._dialog_ref.disableClose = true;
         try {
+            // A retry reuses the row the last attempt made, so needs no image.
+            const name = this._name();
+            const overlay = !this._pending && this.has_overlay();
+            const blob = overlay ? await this._layer()?.toBlob() : undefined;
+            if (overlay && !blob) {
+                notifyError(i18n('SIGNAGE_MANAGER.IMAGE_GEN_NO_IMAGE'));
+                return;
+            }
+
             let pending = this._pending;
             if (!pending) {
                 const media = blob
@@ -1070,9 +1101,9 @@ export class ImageGenModalComponent implements OnDestroy {
             this._dialog_ref.close(media);
         } catch (error) {
             notifyError(
-                errorMessage(
+                actionError(
                     error,
-                    i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_FAILED'),
+                    i18n('SIGNAGE_MANAGER.IMAGE_GEN_SAVE_FAILED'),
                 ),
             );
         } finally {
@@ -1087,63 +1118,68 @@ export class ImageGenModalComponent implements OnDestroy {
      * it rather than making a second. While it is set the pick is locked.
      */
     private _pending: { media: SignageMedia; claimed: boolean } | undefined;
-    private _await_timer: ReturnType<typeof setTimeout> | null = null;
     /** bumped to stop whichever job the modal was following */
     private _job_token = 0;
     private _logo_defaulted = false;
+    /** the job the modal waits on, empty once it ends or is let go */
+    private readonly _awaiting = signal('');
+
+    constructor() {
+        // the service polls the job, and marks it failed if it runs too long
+        effect(() => {
+            const id = this._awaiting();
+            const job = id ? this._image_gen.jobs()[id] : undefined;
+            if (job && isFinal(job)) untracked(() => this._finish(job));
+        });
+    }
 
     /** follow a job the server accepted, unless it was cancelled on the way */
     private _follow(job: ImageGenJob, token: number) {
-        if (this._closed) return;
-        if (token !== this._job_token) {
-            this._image_gen.cancel(job.id);
+        if (this._closed || token !== this._job_token) {
+            this._abandon(job.id, this._closed ? this.reference_ids() : []);
             return;
         }
         this.job_ids.update((ids) => [...ids, job.id]);
+        // the modal shows the result, so the service need not announce it
+        this._image_gen.setJobOnScreen(job.id, true);
         this._awaitJob(job.id);
     }
 
+    /**
+     * Stop a job no one is waiting for, without a notice when it ends. The
+     * references it read can go once it has stopped.
+     */
+    private async _abandon(id: string, reference_ids: string[]) {
+        this._image_gen.unwatch(id);
+        const job = await this._image_gen.cancel(id);
+        if (!isFinal(job)) return;
+        reference_ids.forEach((item) => this._image_gen.removeReference(item));
+    }
+
     private _stopAwaiting() {
         this._job_token++;
-        if (this._await_timer) clearTimeout(this._await_timer);
-        this._await_timer = null;
+        this._awaiting.set('');
     }
 
-    /** poll until the job reaches a final state, then move on */
+    /** wait for the job to reach a final state, then move on */
     private _awaitJob(id: string) {
         this._stopAwaiting();
-        const token = this._job_token;
-        const deadline = Date.now() + MAX_JOB_WAIT_MS;
-        const check = () => {
-            this._await_timer = null;
-            if (this._closed || token !== this._job_token) return;
-            const job = this._image_gen.jobs()[id];
-            if (!job || !isFinal(job)) {
-                if (Date.now() >= deadline) {
-                    this._image_gen.unwatch(id);
-                    this.state.set(this.rail().length ? 'review' : 'compose');
-                    notifyError(i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_FAILED'));
-                    return;
-                }
-                this._await_timer = setTimeout(check, 250);
-                return;
-            }
-            if (job.state === 'failed') {
-                this.state.set(this.rail().length ? 'review' : 'compose');
-                return;
-            }
-            if (job.state === 'cancelled') {
-                this.state.set(this.rail().length ? 'review' : 'compose');
-                return;
-            }
-            const newest = this.rail().filter(
-                (candidate) => candidate.job_id === id,
-            );
-            if (newest.length) this.select(newest[0]);
-            this._loadBrandLogos();
-            this.state.set('review');
-        };
-        check();
+        this._awaiting.set(id);
+    }
+
+    private _finish(job: ImageGenJob) {
+        this._awaiting.set('');
+        if (this._closed) return;
+        if (job.state !== 'done') {
+            this.state.set(this.rail().length ? 'review' : 'compose');
+            return;
+        }
+        const newest = this.rail().find(
+            (candidate) => candidate.job_id === job.id,
+        );
+        if (newest) this.select(newest);
+        this._loadBrandLogos();
+        this.state.set('review');
     }
 
     /** both saved logos, so the toggle in the sidebar has something to show */
diff --git a/apps/signage-manager/src/app/image-gen/image-gen.service.ts b/apps/signage-manager/src/app/image-gen/image-gen.service.ts
index e9c164b6e3b..80c9a42a9cf 100644
--- a/apps/signage-manager/src/app/image-gen/image-gen.service.ts
+++ b/apps/signage-manager/src/app/image-gen/image-gen.service.ts
@@ -28,7 +28,7 @@ import {
     ImageGenJob,
     ImageGenLogoSlot,
 } from './image-gen.types';
-import { errorStatus } from './image-gen.util';
+import { errorStatus, UserFacingError } from './image-gen.util';
 
 const FINAL_STATES = ['done', 'failed', 'cancelled'];
 
@@ -166,6 +166,9 @@ export class ImageGenService extends AsyncHandler {
         file: File,
         derive_other = false,
     ): Promise<ImageGenBrandKit> {
+        // checked before uploading, so a kit that cannot be saved leaves no
+        // stray uploads behind
+        this._assertBrandKitWritable();
         const upload_id = await this._uploads.uploadFileToCompletion(file);
         const changes: Partial<ImageGenBrandKit> = {
             [logoKey(slot)]: upload_id,
@@ -196,12 +199,15 @@ export class ImageGenService extends AsyncHandler {
     public async deriveBrandLogo(
         target: ImageGenLogoSlot,
     ): Promise<ImageGenBrandKit> {
+        this._assertBrandKitWritable();
         const source_id =
             this.brand_kit()?.[
                 logoKey(target === 'on_light' ? 'on_dark' : 'on_light')
             ];
         if (!source_id)
-            throw new Error(i18n('SIGNAGE_MANAGER.IMAGE_GEN_NO_LOGO_YET'));
+            throw new UserFacingError(
+                i18n('SIGNAGE_MANAGER.IMAGE_GEN_NO_LOGO_YET'),
+            );
         const url = await this.loadImage(
             `/api/engine/v2/uploads/${encodeURIComponent(source_id)}/url`,
         );
@@ -239,20 +245,30 @@ export class ImageGenService extends AsyncHandler {
         return removeSignageUpload(id).catch(() => null);
     }
 
+    /** the last brand kit write, so the next one waits for it */
+    private _kit_write: Promise<unknown> = Promise.resolve();
+
     /**
-     * Merge changes into the domain's brand kit.
+     * Merge changes into the domain's brand kit. Writes run one at a time:
+     * each replaces the whole kit, so two at once would lose one's changes.
      */
-    public async saveBrandKit(
+    public saveBrandKit(
         changes: Partial<ImageGenBrandKit>,
     ): Promise<ImageGenBrandKit> {
-        if (!this._org_zone) {
-            throw new Error(i18n('SIGNAGE_MANAGER.IMAGE_GEN_NO_ORG_ZONE'));
-        }
-        if (this.brand_kit_read() !== 'ok') {
-            throw new Error(i18n('SIGNAGE_MANAGER.BRAND_NOT_LOADED'));
-        }
-        const details = { ...(this.brand_kit() || {}), ...changes };
-        for (const key of Object.keys(details)) {
+        const write = this._kit_write.then(() => this._writeBrandKit(changes));
+        this._kit_write = write.catch(() => null);
+        return write;
+    }
+
+    private async _writeBrandKit(
+        changes: Partial<ImageGenBrandKit>,
+    ): Promise<ImageGenBrandKit> {
+        this._assertBrandKitWritable();
+        const details: ImageGenBrandKit = {
+            ...(this.brand_kit() || {}),
+            ...changes,
+        };
+        for (const key of Object.keys(details) as (keyof ImageGenBrandKit)[]) {
             if (details[key] === undefined) delete details[key];
         }
 
@@ -272,6 +288,18 @@ export class ImageGenService extends AsyncHandler {
         return details;
     }
 
+    /** a save needs the organisation zone and the stored kit to merge into */
+    private _assertBrandKitWritable() {
+        if (!this._org_zone) {
+            throw new UserFacingError(
+                i18n('SIGNAGE_MANAGER.IMAGE_GEN_NO_ORG_ZONE'),
+            );
+        }
+        if (this.brand_kit_read() !== 'ok') {
+            throw new UserFacingError(i18n('SIGNAGE_MANAGER.BRAND_NOT_LOADED'));
+        }
+    }
+
     /** re-read the kit, for a page opened before start up finished */
     public async reloadBrandKit(): Promise<ImageGenBrandKit | null> {
         if (!this._org_zone) return null;
@@ -456,14 +484,30 @@ export class ImageGenService extends AsyncHandler {
         }
     }
 
-    /** told once, when a job the user may no longer be watching finishes */
+    /** jobs a screen is showing, so their results need no notice */
+    private readonly _on_screen = new Set<string>();
+
+    /** mark a job as shown, or no longer shown, by an open screen */
+    public setJobOnScreen(id: string, on_screen: boolean) {
+        if (on_screen) this._on_screen.add(id);
+        else this._on_screen.delete(id);
+    }
+
+    /**
+     * Told once, when a job the user may no longer be watching finishes. A
+     * failure is always told, as the screen showing it does not say why.
+     */
     private _announce(job: ImageGenJob) {
         if (job.state === 'failed') {
             notifyError(
                 job.error_message ||
                     i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_FAILED'),
             );
-        } else if (job.state === 'done' && job.images_produced > 0) {
+        } else if (
+            job.state === 'done' &&
+            job.images_produced > 0 &&
+            !this._on_screen.has(job.id)
+        ) {
             notifyInfo(i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_DONE'));
         }
     }
diff --git a/apps/signage-manager/src/app/image-gen/image-gen.util.ts b/apps/signage-manager/src/app/image-gen/image-gen.util.ts
index 2bff1ffc227..9b8d3169dc0 100644
--- a/apps/signage-manager/src/app/image-gen/image-gen.util.ts
+++ b/apps/signage-manager/src/app/image-gen/image-gen.util.ts
@@ -1,19 +1,19 @@
-/** Return the API's string error without leaking response objects to the UI. */
-export function errorMessage(error: unknown, fallback: string): string {
-    if (typeof error === 'string') return error;
-    if (!isRecord(error)) return fallback;
+import { log } from '@placeos/common';
 
-    const nested = error['error'];
-    if (typeof nested === 'string') return nested;
-    if (isRecord(nested)) {
-        const detail = nested['error'];
-        if (typeof detail === 'string') return detail;
-        const nested_message = nested['message'];
-        if (typeof nested_message === 'string') return nested_message;
-    }
+/** An error whose message is written for the person, already translated. */
+export class UserFacingError extends Error {}
 
-    const message = error['message'];
-    return typeof message === 'string' ? message : fallback;
+/**
+ * The text to show when a user action fails. Raw API and upload errors read
+ * as noise ("Creating upload ... failed with status 500"), so the action's own
+ * message shows and the raw error goes to the console. Only a UserFacingError
+ * keeps its message. A failed job's server message is shown elsewhere, as
+ * that one is written for the person.
+ */
+export function actionError(error: unknown, fallback: string): string {
+    if (error instanceof UserFacingError) return error.message;
+    log('ImageGen', fallback, error, 'error', true);
+    return fallback;
 }
 
 function isRecord(value: unknown): value is Record<string, unknown> {
diff --git a/apps/signage-manager/src/tests/branding/branding.component.spec.ts b/apps/signage-manager/src/tests/branding/branding.component.spec.ts
index 4b54be9c8f9..7d25ddb12f6 100644
--- a/apps/signage-manager/src/tests/branding/branding.component.spec.ts
+++ b/apps/signage-manager/src/tests/branding/branding.component.spec.ts
@@ -49,7 +49,9 @@ describe('BrandingComponent', () => {
             target: input,
         } as unknown as Event);
 
-        expect(component.colours()).toEqual(['#123456']);
+        expect(component.colours()).toEqual([
+            { key: 'primary', value: '#123456' },
+        ]);
     });
 
     it('keeps palette colours past the three it shows when saving', async () => {
@@ -64,7 +66,11 @@ describe('BrandingComponent', () => {
         const component = await make();
         await component.ngOnInit();
 
-        expect(component.colours()).toEqual(['#111111', '#222222', '#333333']);
+        expect(component.colours().map((colour) => colour.value)).toEqual([
+            '#111111',
+            '#222222',
+            '#333333',
+        ]);
         await component.save();
 
         expect(image_gen_stub.saveBrandKit).toHaveBeenCalledWith(
@@ -89,4 +95,43 @@ describe('BrandingComponent', () => {
         expect(component.load_state()).toBe('failed');
         expect(component.can_edit()).toBe(false);
     });
+
+    it('saves each colour back under the key it was read from', async () => {
+        const palette = {
+            primary: '#111111',
+            accent: '#333333',
+            highlight: '#444444',
+            extra: '#555555',
+        };
+        image_gen_stub.brand_kit.set({ palette });
+        const component = await make();
+        await component.ngOnInit();
+
+        await component.save();
+
+        expect(image_gen_stub.saveBrandKit).toHaveBeenCalledWith(
+            expect.objectContaining({ palette }),
+        );
+    });
+
+    it('gives an added colour a key that is not in use', async () => {
+        image_gen_stub.brand_kit.set({
+            palette: { primary: '#111111', secondary: '#222222' },
+        });
+        const component = await make();
+        await component.ngOnInit();
+
+        component.addColour();
+        await component.save();
+
+        expect(image_gen_stub.saveBrandKit).toHaveBeenCalledWith(
+            expect.objectContaining({
+                palette: {
+                    primary: '#111111',
+                    secondary: '#222222',
+                    accent: '#1B2420',
+                },
+            }),
+        );
+    });
 });
diff --git a/apps/signage-manager/src/tests/image-gen/image-gen-layer-controls.component.spec.ts b/apps/signage-manager/src/tests/image-gen/image-gen-layer-controls.component.spec.ts
index 9470a391d13..0a4204764d3 100644
--- a/apps/signage-manager/src/tests/image-gen/image-gen-layer-controls.component.spec.ts
+++ b/apps/signage-manager/src/tests/image-gen/image-gen-layer-controls.component.spec.ts
@@ -38,4 +38,25 @@ describe('ImageGenLayerControlsComponent', () => {
             }),
         );
     });
+
+    it('does not point at the branding page when branding changes are off', async () => {
+        await TestBed.configureTestingModule({
+            imports: [ImageGenLayerControlsComponent],
+        })
+            .overrideComponent(ImageGenLayerControlsComponent, {
+                set: { template: '' },
+            })
+            .compileComponents();
+        const fixture = TestBed.createComponent(ImageGenLayerControlsComponent);
+        fixture.componentRef.setInput('can_set_logo', false);
+        expect(fixture.componentInstance.no_logo_note()).toBe(
+            'SIGNAGE_MANAGER.IMAGE_GEN_NO_LOGO_ADMIN',
+        );
+
+        fixture.componentRef.setInput('branding_editing', false);
+
+        expect(fixture.componentInstance.no_logo_note()).toBe(
+            'SIGNAGE_MANAGER.IMAGE_GEN_NO_LOGO_LOCKED',
+        );
+    });
 });
diff --git a/apps/signage-manager/src/tests/image-gen/image-gen-layer.component.spec.ts b/apps/signage-manager/src/tests/image-gen/image-gen-layer.component.spec.ts
index 85c4b4e3af3..14e0464c160 100644
--- a/apps/signage-manager/src/tests/image-gen/image-gen-layer.component.spec.ts
+++ b/apps/signage-manager/src/tests/image-gen/image-gen-layer.component.spec.ts
@@ -1,5 +1,6 @@
 import { TestBed } from '@angular/core/testing';
 
+import { ensureBrandFont } from '../../app/branding/brand-fonts';
 import { ImageGenLayerComponent } from '../../app/image-gen/image-gen-layer.component';
 import { ImageGenLayerState } from '../../app/image-gen/image-gen.types';
 
@@ -12,6 +13,8 @@ interface Box {
 
 /** the private parts these tests drive directly */
 interface LayerInternals {
+    _panelColour: (text_colour: string) => string;
+    _draw: () => void;
     _boxes: Map<string, Box>;
     _logos: Record<'on_light' | 'on_dark', HTMLImageElement | null>;
     _logoFor: (
@@ -109,4 +112,45 @@ describe('ImageGenLayerComponent', () => {
             }),
         );
     });
+
+    it('shades dark short hex text with a light panel', async () => {
+        const fixture = await make('');
+        const layer = fixture.componentInstance as unknown as LayerInternals;
+
+        expect(layer._panelColour('#000')).toBe('rgba(255, 255, 255, 0.6)');
+        expect(layer._panelColour('#fff')).toBe('rgba(0, 0, 0, 0.45)');
+    });
+
+    it('draws once per drag step, not again for each font', async () => {
+        const fixture = await make('');
+        const layer = fixture.componentInstance as unknown as LayerInternals;
+        const block = {
+            id: 'block-1',
+            text: 'Hello',
+            role: 'headline' as const,
+            x: 0.1,
+            y: 0.1,
+            align: 'left' as const,
+            colour: '#FFFFFF',
+            font: 'Inter',
+            panel: false,
+        };
+        fixture.componentRef.setInput('state', layerState({ blocks: [block] }));
+        fixture.detectChanges();
+        // jsdom fetches no stylesheets, so say this one has loaded
+        document.head
+            .querySelector('link[href*="Inter"]')
+            ?.dispatchEvent(new Event('load'));
+        await ensureBrandFont('Inter');
+        const draw = vi.spyOn(layer, '_draw');
+
+        fixture.componentRef.setInput(
+            'state',
+            layerState({ blocks: [{ ...block, x: 0.2 }] }),
+        );
+        fixture.detectChanges();
+        await ensureBrandFont('Inter');
+
+        expect(draw).toHaveBeenCalledTimes(1);
+    });
 });
diff --git a/apps/signage-manager/src/tests/image-gen/image-gen-modal.component.spec.ts b/apps/signage-manager/src/tests/image-gen/image-gen-modal.component.spec.ts
index 5d32c43b573..c55228a0dd2 100644
--- a/apps/signage-manager/src/tests/image-gen/image-gen-modal.component.spec.ts
+++ b/apps/signage-manager/src/tests/image-gen/image-gen-modal.component.spec.ts
@@ -1,6 +1,7 @@
 import { signal } from '@angular/core';
 import { TestBed } from '@angular/core/testing';
 import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog';
+import { i18n, setNotifyOutlet } from '@placeos/common';
 
 import { ImageGenModalComponent } from '../../app/image-gen/image-gen-modal.component';
 import { ImageGenService } from '../../app/image-gen/image-gen.service';
@@ -74,12 +75,27 @@ function capabilities(
 }
 
 describe('ImageGenModalComponent', () => {
+    const notify_open = vi.fn(() => ({
+        onAction: () => ({ subscribe: () => ({ unsubscribe: () => {} }) }),
+        dismiss: vi.fn(),
+    }));
+
     // jsdom has no object URL support and the modal revokes on removal
     beforeAll(() => {
         URL.createObjectURL ??= vi.fn(() => 'blob:mock');
         URL.revokeObjectURL ??= vi.fn();
     });
 
+    beforeEach(() => {
+        notify_open.mockClear();
+        setNotifyOutlet(
+            { open: notify_open } as unknown as Parameters<
+                typeof setNotifyOutlet
+            >[0],
+            true,
+        );
+    });
+
     async function make(data: Record<string, string> = {}) {
         const jobs = signal<Record<string, ImageGenJob>>({});
         const current_capabilities = capabilities();
@@ -120,16 +136,19 @@ describe('ImageGenModalComponent', () => {
             generate,
             cancel: vi.fn(),
             unwatch: vi.fn(),
+            setJobOnScreen: vi.fn(),
             claim: vi.fn().mockResolvedValue({}),
             removeReference: vi.fn(),
-            loadImage: vi.fn().mockResolvedValue(''),
+            loadImage: vi.fn().mockResolvedValue('blob:artwork'),
         };
         const context_stub = {
             is_sys_admin: signal(false),
+            global_features: signal<string[]>(['branding-editing']),
             selected_group: signal({ group: { id: 'group-1' } }),
             hasFeature: vi.fn(() => true),
         };
         const media_stub = {
+            addMedia: vi.fn(),
             addMediaFromUpload: vi.fn(),
             discardCreatedMedia: vi.fn(),
         };
@@ -153,14 +172,32 @@ describe('ImageGenModalComponent', () => {
         const component = TestBed.createComponent(
             ImageGenModalComponent,
         ).componentInstance;
-        return { image_gen, component, dialog_ref, media_stub, playlist_stub };
+        return {
+            image_gen,
+            component,
+            context_stub,
+            dialog_ref,
+            media_stub,
+            playlist_stub,
+        };
     }
 
     afterEach(() => {
         vi.useRealTimers();
         TestBed.resetTestingModule();
+        setNotifyOutlet(null, true);
     });
 
+    const pick = {
+        job_id: 'job-1',
+        index: 0,
+        upload_id: 'upload-1',
+        url: '/uploads/upload-1',
+        width: 1024,
+        height: 1536,
+        version: 1,
+    };
+
     it('does not let a cancelled job take over when it finishes later', async () => {
         vi.useFakeTimers();
         const { image_gen, component } = await make();
@@ -190,7 +227,7 @@ describe('ImageGenModalComponent', () => {
             ...jobs,
             'job-1': job('job-1', {}, [image('upload-1')]),
         }));
-        await vi.advanceTimersByTimeAsync(1_000);
+        TestBed.tick();
 
         expect(component.state()).toBe('generating');
         expect(component.selected()).toBeNull();
@@ -211,7 +248,7 @@ describe('ImageGenModalComponent', () => {
         expect(component.state()).toBe('generating');
 
         image_gen.jobs.set({ 'job-1': job('job-1', {}, [image('upload-1')]) });
-        await vi.advanceTimersByTimeAsync(1_000);
+        TestBed.tick();
 
         expect(component.state()).toBe('review');
     });
@@ -243,8 +280,10 @@ describe('ImageGenModalComponent', () => {
             );
         component.brief.set('A poster for the launch');
         await component.start();
+        TestBed.tick();
         component.refinement.set('Darker');
         await component.refine();
+        TestBed.tick();
 
         // refine option 2 of version 1 after version 2 exists
         await component.select(component.rail()[1]);
@@ -275,18 +314,11 @@ describe('ImageGenModalComponent', () => {
             });
         const media = { id: 'media-1', thumbnail_id: '' };
         media_stub.addMediaFromUpload.mockResolvedValue(media);
+        component.selected_object_url.set('blob:artwork');
         playlist_stub.addMediaToPlaylist
             .mockRejectedValueOnce(new Error('offline'))
             .mockResolvedValueOnce(undefined);
-        component.selected.set({
-            job_id: 'job-1',
-            index: 0,
-            upload_id: 'upload-1',
-            url: '/uploads/upload-1',
-            width: 1024,
-            height: 1536,
-            version: 1,
-        });
+        component.selected.set(pick);
 
         await component.save();
         expect(dialog_ref.close).not.toHaveBeenCalled();
@@ -419,4 +451,140 @@ describe('ImageGenModalComponent', () => {
             expect.not.objectContaining({ aspect_ratio: expect.anything() }),
         );
     });
+
+    it('starts one save when Save is clicked again while the image is encoded', async () => {
+        const { component, media_stub } = await make();
+        media_stub.addMedia.mockResolvedValue({ id: 'media-1' });
+        let encoded: (blob: Blob) => void = () => undefined;
+        const toBlob = vi.fn(
+            () => new Promise<Blob>((resolve) => (encoded = resolve)),
+        );
+        Object.assign(component, { _layer: () => ({ toBlob }) });
+        component.layer_state.update((state) => ({
+            ...state,
+            blocks: [{ ...state.blocks[0], text: 'Launch party' }],
+        }));
+        component.selected.set(pick);
+        component.selected_object_url.set('blob:artwork');
+
+        const first = component.save();
+        const second = component.save();
+        encoded(new Blob(['png']));
+        await Promise.all([first, second]);
+
+        expect(toBlob).toHaveBeenCalledTimes(1);
+        expect(media_stub.addMedia).toHaveBeenCalledTimes(1);
+    });
+
+    it('names the action that failed, not the raw upload error', async () => {
+        const { image_gen, component, context_stub, media_stub } = await make();
+        const raw = new Error('Creating upload failed with status 500: {}');
+        const shown = () =>
+            notify_open.mock.calls.map((call: unknown[]) => call[0]);
+        Object.assign(image_gen, {
+            uploadReference: vi.fn().mockRejectedValue(raw),
+            uploadBrandLogo: vi.fn().mockRejectedValue(raw),
+        });
+        media_stub.addMediaFromUpload.mockRejectedValue(raw);
+        context_stub.is_sys_admin.set(true);
+        component.selected.set(pick);
+        component.selected_object_url.set('blob:artwork');
+
+        await component.addReferences([new File([], 'a.png')], 'include');
+        await component.uploadLogo(new File([], 'logo.png'));
+        await component.save();
+
+        expect(shown()).toEqual([
+            i18n('SIGNAGE_MANAGER.IMAGE_GEN_REFERENCE_UPLOAD_FAILED'),
+            i18n('SIGNAGE_MANAGER.IMAGE_GEN_LOGO_SAVE_FAILED'),
+            i18n('SIGNAGE_MANAGER.IMAGE_GEN_SAVE_FAILED'),
+        ]);
+    });
+
+    it('saves nothing until the picked option has loaded', async () => {
+        const { component, media_stub } = await make();
+        component.selected.set(pick);
+
+        expect(component.can_save()).toBe(false);
+        await component.save();
+
+        expect(media_stub.addMediaFromUpload).not.toHaveBeenCalled();
+    });
+
+    it('tells the service which jobs it shows while open', async () => {
+        const { image_gen, component } = await make();
+        component.brief.set('A poster for the launch');
+        await component.start();
+        expect(image_gen.setJobOnScreen).toHaveBeenCalledWith('job-1', true);
+
+        component.ngOnDestroy();
+
+        expect(image_gen.setJobOnScreen).toHaveBeenCalledWith('job-1', false);
+    });
+
+    it('stops the running job when the modal closes', async () => {
+        const { image_gen, component } = await make();
+        image_gen.generate.mockImplementationOnce(async () => {
+            image_gen.jobs.set({ 'job-1': job('job-1', { state: 'running' }) });
+            return image_gen.jobs()['job-1'];
+        });
+        image_gen.cancel.mockResolvedValue(
+            job('job-1', { state: 'cancelled' }),
+        );
+        component.include_references.set([
+            { id: 'inc-1', name: 'one.png', url: 'blob:one' },
+        ]);
+        component.brief.set('A poster for the launch');
+        await component.start();
+
+        component.ngOnDestroy();
+        // the stopped job no longer reads the reference, so it goes too
+        await vi.waitFor(() =>
+            expect(image_gen.removeReference).toHaveBeenCalledWith('inc-1'),
+        );
+
+        expect(image_gen.cancel).toHaveBeenCalledWith('job-1');
+        expect(image_gen.unwatch).toHaveBeenCalledWith('job-1');
+    });
+
+    it('stops a job the server accepts after the modal closed', async () => {
+        const { image_gen, component } = await make();
+        let accept: (job: ImageGenJob) => void = () => undefined;
+        image_gen.generate.mockImplementationOnce(
+            () => new Promise<ImageGenJob>((resolve) => (accept = resolve)),
+        );
+        component.brief.set('A poster for the launch');
+
+        const started = component.start();
+        component.ngOnDestroy();
+        accept(job('job-1', { state: 'running' }));
+        await started;
+
+        expect(image_gen.cancel).toHaveBeenCalledWith('job-1');
+        expect(image_gen.unwatch).toHaveBeenCalledWith('job-1');
+    });
+
+    it('lets go of an option whose image cannot be read', async () => {
+        const { image_gen, component } = await make();
+        image_gen.loadImage.mockRejectedValueOnce(new Error('offline'));
+
+        await component.select(pick);
+
+        expect(component.selected()).toBeNull();
+        expect(notify_open).toHaveBeenCalledWith(
+            i18n('SIGNAGE_MANAGER.IMAGE_GEN_IMAGE_UNREADABLE'),
+            expect.anything(),
+            expect.anything(),
+        );
+    });
+
+    it('offers logo changes only when branding editing is on', async () => {
+        const { component, context_stub } = await make();
+        context_stub.is_sys_admin.set(true);
+        expect(component.can_set_logo()).toBe(true);
+
+        context_stub.global_features.set([]);
+
+        expect(component.can_set_logo()).toBe(false);
+    });
 });
diff --git a/apps/signage-manager/src/tests/image-gen/image-gen.service.spec.ts b/apps/signage-manager/src/tests/image-gen/image-gen.service.spec.ts
index b0efc18c30c..8b9a67c445a 100644
--- a/apps/signage-manager/src/tests/image-gen/image-gen.service.spec.ts
+++ b/apps/signage-manager/src/tests/image-gen/image-gen.service.spec.ts
@@ -1,12 +1,13 @@
 import { TestBed } from '@angular/core/testing';
-import { UploadsService } from '@placeos/common';
-import { get, post } from '@placeos/ts-client';
+import { i18n, setNotifyOutlet, UploadsService } from '@placeos/common';
+import { get, post, updateMetadata } from '@placeos/ts-client';
 
 import {
     ImageGenService,
     MAX_JOB_WAIT_MS,
 } from '../../app/image-gen/image-gen.service';
 import {
+    ImageGenBrandKit,
     ImageGenCapabilities,
     ImageGenJob,
 } from '../../app/image-gen/image-gen.types';
@@ -24,6 +25,28 @@ function runningJob(id = 'job-1'): ImageGenJob {
 }
 
 vi.mock('@placeos/ts-client', { spy: true });
+/**
+ * A service with a stored kit it can write to. jsdom cannot draw, so a
+ * flipped logo is stood in for by an upload named after its slot.
+ */
+function kitService(kit: ImageGenBrandKit = {}) {
+    const service = TestBed.inject(ImageGenService);
+    Object.assign(service, {
+        _org_zone: 'org-1',
+        _flip: async (_source: File | string, target: string) =>
+            `flipped:${target}`,
+    });
+    service.brand_kit.set(kit);
+    service.brand_kit_read.set('ok');
+    return service;
+}
+
+/** the details of each brand kit write, in order */
+function writtenKits() {
+    return vi
+        .mocked(updateMetadata)
+        .mock.calls.map(([, metadata]) => metadata.details);
+}
 
 /** the overloads type every response as a string; these return JSON */
 type JsonRequest = (...args: unknown[]) => Promise<unknown>;
@@ -38,7 +61,12 @@ describe('ImageGenService', () => {
                 ImageGenService,
                 {
                     provide: UploadsService,
-                    useValue: { uploadFileToCompletion: vi.fn() },
+                    // each file is stored under its own name
+                    useValue: {
+                        uploadFileToCompletion: vi.fn(
+                            async (file: File) => `upload:${file.name}`,
+                        ),
+                    },
                 },
             ],
         });
@@ -46,6 +74,12 @@ describe('ImageGenService', () => {
 
     afterEach(() => vi.useRealTimers());
 
+    beforeEach(() => {
+        vi.mocked(updateMetadata).mockResolvedValue(
+            {} as Awaited<ReturnType<typeof updateMetadata>>,
+        );
+    });
+
     it('reuses a key only when the complete request is unchanged', () => {
         const service = TestBed.inject(ImageGenService);
         const first = service.intentKey('generate', {
@@ -167,4 +201,132 @@ describe('ImageGenService', () => {
 
         expect(service.jobs()[job.id].state).toBe('failed');
     });
+
+    it('writes brand kit changes one at a time, each on top of the last', async () => {
+        const service = kitService({ organisation: 'Acme' });
+        let first_done: () => void = () => undefined;
+        vi.mocked(updateMetadata).mockImplementationOnce(
+            () =>
+                new Promise((resolve) => {
+                    first_done = () =>
+                        resolve(
+                            {} as Awaited<ReturnType<typeof updateMetadata>>,
+                        );
+                }),
+        );
+
+        const palette = service.saveBrandKit({ palette: { primary: '#111' } });
+        const logo = service.saveBrandKit({ logo_upload_id: 'logo-1' });
+        await Promise.resolve();
+        expect(updateMetadata).toHaveBeenCalledTimes(1);
+        first_done();
+        await Promise.all([palette, logo]);
+
+        expect(writtenKits()[1]).toEqual({
+            organisation: 'Acme',
+            palette: { primary: '#111' },
+            logo_upload_id: 'logo-1',
+        });
+    });
+
+    it('uploads nothing when the brand kit cannot be saved', async () => {
+        const service = kitService();
+        service.brand_kit_read.set('failed');
+        const uploads = TestBed.inject(UploadsService);
+
+        await expect(
+            service.replaceBrandLogo(
+                'on_light',
+                new File([], 'logo.png'),
+                true,
+            ),
+        ).rejects.toThrow();
+
+        expect(uploads.uploadFileToCompletion).not.toHaveBeenCalled();
+    });
+
+    it('makes the missing version of a logo from the one uploaded', async () => {
+        const service = kitService();
+
+        await service.replaceBrandLogo(
+            'on_light',
+            new File([], 'logo.png'),
+            true,
+        );
+
+        expect(writtenKits()[0]).toEqual({
+            logo_upload_id: 'upload:logo.png',
+            logo_dark_upload_id: 'flipped:on_dark',
+            logo_derived: 'on_dark',
+        });
+    });
+
+    it('replaces a made version, but not an uploaded one', async () => {
+        const made = kitService({
+            logo_upload_id: 'old-light',
+            logo_dark_upload_id: 'old-dark',
+            logo_derived: 'on_dark',
+        });
+        await made.replaceBrandLogo('on_light', new File([], 'logo.png'), true);
+        expect(writtenKits()[0]).toMatchObject({
+            logo_dark_upload_id: 'flipped:on_dark',
+            logo_derived: 'on_dark',
+        });
+
+        made.brand_kit.set({
+            logo_upload_id: 'old-light',
+            logo_dark_upload_id: 'old-dark',
+        });
+        await made.replaceBrandLogo('on_light', new File([], 'logo.png'), true);
+        expect(writtenKits()[1]).toEqual({
+            logo_upload_id: 'upload:logo.png',
+            logo_dark_upload_id: 'old-dark',
+        });
+    });
+
+    it('stops calling a slot made once a file is uploaded into it', async () => {
+        const service = kitService({
+            logo_upload_id: 'light',
+            logo_dark_upload_id: 'made-dark',
+            logo_derived: 'on_dark',
+        });
+
+        await service.replaceBrandLogo('on_dark', new File([], 'dark.png'));
+
+        expect(writtenKits()[0]).toEqual({
+            logo_upload_id: 'light',
+            logo_dark_upload_id: 'upload:dark.png',
+        });
+    });
+
+    it('does not announce a finished job that an open screen shows', async () => {
+        const notify_open = vi.fn(() => ({
+            onAction: () => ({ subscribe: () => ({ unsubscribe: () => {} }) }),
+            dismiss: vi.fn(),
+        }));
+        setNotifyOutlet(
+            { open: notify_open } as unknown as Parameters<
+                typeof setNotifyOutlet
+            >[0],
+            true,
+        );
+        const done = { ...runningJob(), state: 'done' as const, version: 2 };
+        json_get.mockResolvedValue({ ...done, images_produced: 1 });
+        const service = TestBed.inject(ImageGenService);
+        service.jobs.set({ [done.id]: runningJob() });
+        service.setJobOnScreen(done.id, true);
+        service.watch(done.id);
+
+        await (
+            service as unknown as { _poll: (id: string) => Promise<void> }
+        )._poll(done.id);
+        setNotifyOutlet(null, true);
+
+        expect(service.jobs()[done.id].state).toBe('done');
+        expect(notify_open).not.toHaveBeenCalledWith(
+            i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_DONE'),
+            expect.anything(),
+            expect.anything(),
+        );
+    });
 });
diff --git a/apps/signage-manager/src/tests/image-gen/image-gen.util.spec.ts b/apps/signage-manager/src/tests/image-gen/image-gen.util.spec.ts
index 16d7969ab67..476da5e5871 100644
--- a/apps/signage-manager/src/tests/image-gen/image-gen.util.spec.ts
+++ b/apps/signage-manager/src/tests/image-gen/image-gen.util.spec.ts
@@ -1,19 +1,25 @@
 import {
-    errorMessage,
+    actionError,
     errorStatus,
     orientationOf,
     perceivedLightness,
+    UserFacingError,
 } from '../../app/image-gen/image-gen.util';
 
 describe('image generation utilities', () => {
-    it('reads nested API errors without returning an object', () => {
+    it('names the action rather than a raw error, unless written for people', () => {
         expect(
-            errorMessage(
-                { error: { error: 'Provider rejected the request' } },
-                'Fallback',
+            actionError(
+                new Error('Creating upload for a.png failed with status 500'),
+                'The image could not be attached',
             ),
-        ).toBe('Provider rejected the request');
-        expect(errorMessage({ error: {} }, 'Fallback')).toBe('Fallback');
+        ).toBe('The image could not be attached');
+        expect(
+            actionError(
+                new UserFacingError('This domain has no organisation zone'),
+                'The logo could not be saved',
+            ),
+        ).toBe('This domain has no organisation zone');
     });
 
     it('reads direct and wrapped HTTP status codes', () => {
diff --git a/shared/assets/locale/en-AU.json b/shared/assets/locale/en-AU.json
index 9d9ea9834db..5278dd7ae10 100644
--- a/shared/assets/locale/en-AU.json
+++ b/shared/assets/locale/en-AU.json
@@ -51,10 +51,12 @@
         "IMAGE_GEN_LOGO_AUTO": "Choose for me",
         "IMAGE_GEN_LOGO_VERSION": "Logo version",
         "IMAGE_GEN_LOGO_SAVED": "Logo saved. It will be used on future posters too.",
+        "IMAGE_GEN_LOGO_SAVE_FAILED": "The logo could not be saved",
         "IMAGE_GEN_LOGO_UPLOADING": "Saving logo...",
         "IMAGE_GEN_NO_IMAGE": "There is no image to save",
         "IMAGE_GEN_NO_LOGO_YET": "No logo saved for this organisation yet.",
         "IMAGE_GEN_NO_LOGO_ADMIN": "No logo saved for this organisation yet. An administrator can add one on the branding page.",
+        "IMAGE_GEN_NO_LOGO_LOCKED": "No logo saved for this organisation yet. Branding changes are turned off.",
         "IMAGE_GEN_NO_ORG_ZONE": "This domain has no organisation zone, so a logo cannot be saved",
         "IMAGE_GEN_OPTION": "Generated option",
         "IMAGE_GEN_OPTIONS_COUNT": "Options to generate",
@@ -81,12 +83,14 @@
         "IMAGE_GEN_REFERENCE_NUMBER": "Image {{ number }}",
         "IMAGE_GEN_REFERENCE_REMOVE": "Remove this image",
         "IMAGE_GEN_REFERENCE_UPLOADING": "Attaching...",
+        "IMAGE_GEN_REFERENCE_UPLOAD_FAILED": "The image could not be attached",
         "IMAGE_GEN_REMOVE_TEXT": "Remove this block",
         "IMAGE_GEN_REPLACE_LOGO": "Replace logo",
         "IMAGE_GEN_ROLE_BODY": "Detail",
         "IMAGE_GEN_ROLE_HEADLINE": "Headline",
         "IMAGE_GEN_ROLE_SUBHEADING": "Subheading",
         "IMAGE_GEN_SAVING": "Saving",
+        "IMAGE_GEN_SAVE_FAILED": "The image could not be saved",
         "IMAGE_GEN_SHAPE": "Shape",
         "IMAGE_GEN_SHOW_LOGO": "Show our logo",
         "IMAGE_GEN_SUBHEADING": "Second line",

From fcbcbad95ffed13d9c9ee29daa75a40da531d7d6 Mon Sep 17 00:00:00 2001
From: Alex Sorafumo <alex@yuion.net>
Date: Fri, 2 Oct 2026 13:25:54 +1000
Subject: [PATCH 2/2] fix(signage-manager): clean up image jobs abandoned when
 the editor closes

- ImageGenService.abandon cancels a job and keeps watching it, without
  notices, so a refused cancel is still followed and its attached
  images are removed when it ends.
- A generate or refine request that fails after the editor closed
  removes its attached images.
- A job check that returns after the job was unwatched is ignored, so
  a closed editor does not announce ready images.
---
 .../image-gen/image-gen-modal.component.ts    | 45 +++++-----
 .../src/app/image-gen/image-gen.service.ts    | 30 ++++++-
 .../image-gen-modal.component.spec.ts         | 33 ++++++--
 .../tests/image-gen/image-gen.service.spec.ts | 83 ++++++++++++++++---
 4 files changed, 146 insertions(+), 45 deletions(-)

diff --git a/apps/signage-manager/src/app/image-gen/image-gen-modal.component.ts b/apps/signage-manager/src/app/image-gen/image-gen-modal.component.ts
index c17e91655dc..e906ebd4a04 100644
--- a/apps/signage-manager/src/app/image-gen/image-gen-modal.component.ts
+++ b/apps/signage-manager/src/app/image-gen/image-gen-modal.component.ts
@@ -792,6 +792,8 @@ export class ImageGenModalComponent implements OnDestroy {
             }
             this._follow(job, token);
         } catch (error) {
+            // closed while waiting, and no job will read the images now
+            if (this._closed) return this._removeReferences();
             if (token !== this._job_token) return;
             this.state.set('compose');
             notifyError(
@@ -828,6 +830,8 @@ export class ImageGenModalComponent implements OnDestroy {
             });
             this._follow(job, token);
         } catch (error) {
+            // closed while waiting, and no job will read the images now
+            if (this._closed) return this._removeReferences();
             if (token !== this._job_token) return;
             this.state.set('review');
             notifyError(
@@ -974,18 +978,23 @@ export class ImageGenModalComponent implements OnDestroy {
         for (const id of this.job_ids()) {
             this._image_gen.setJobOnScreen(id, false);
         }
-        const references = this.references();
-        for (const item of references) URL.revokeObjectURL(item.url);
-        const reference_ids = references.map((item) => item.id);
-        if (this.state() !== 'generating') {
-            reference_ids.forEach((id) => this._image_gen.removeReference(id));
-            return;
-        }
+        for (const item of this.references()) URL.revokeObjectURL(item.url);
+        if (this.state() !== 'generating') return this._removeReferences();
         // nothing can show the result once this closes, so stop the job
-        // rather than spend the quota on images no one can reach. A job the
-        // server has not answered for yet is stopped by _follow.
+        // rather than spend the quota on images no one can reach. A request
+        // the server has not answered yet is dealt with by _follow, or by
+        // start and refine if it fails.
         const job = this._image_gen.jobs()[this.current_job_id()];
-        if (job && !isFinal(job)) this._abandon(job.id, reference_ids);
+        if (job && !isFinal(job)) {
+            this._image_gen.abandon(job.id, this.reference_ids());
+        }
+    }
+
+    /** nothing sends the attached images again, so their uploads can go */
+    private _removeReferences() {
+        this.reference_ids().forEach((id) =>
+            this._image_gen.removeReference(id),
+        );
     }
 
     public async uploadLogo(file: File) {
@@ -1136,7 +1145,10 @@ export class ImageGenModalComponent implements OnDestroy {
     /** follow a job the server accepted, unless it was cancelled on the way */
     private _follow(job: ImageGenJob, token: number) {
         if (this._closed || token !== this._job_token) {
-            this._abandon(job.id, this._closed ? this.reference_ids() : []);
+            this._image_gen.abandon(
+                job.id,
+                this._closed ? this.reference_ids() : [],
+            );
             return;
         }
         this.job_ids.update((ids) => [...ids, job.id]);
@@ -1145,17 +1157,6 @@ export class ImageGenModalComponent implements OnDestroy {
         this._awaitJob(job.id);
     }
 
-    /**
-     * Stop a job no one is waiting for, without a notice when it ends. The
-     * references it read can go once it has stopped.
-     */
-    private async _abandon(id: string, reference_ids: string[]) {
-        this._image_gen.unwatch(id);
-        const job = await this._image_gen.cancel(id);
-        if (!isFinal(job)) return;
-        reference_ids.forEach((item) => this._image_gen.removeReference(item));
-    }
-
     private _stopAwaiting() {
         this._job_token++;
         this._awaiting.set('');
diff --git a/apps/signage-manager/src/app/image-gen/image-gen.service.ts b/apps/signage-manager/src/app/image-gen/image-gen.service.ts
index 80c9a42a9cf..0af3e58d598 100644
--- a/apps/signage-manager/src/app/image-gen/image-gen.service.ts
+++ b/apps/signage-manager/src/app/image-gen/image-gen.service.ts
@@ -386,6 +386,30 @@ export class ImageGenService extends AsyncHandler {
         return job;
     }
 
+    /** jobs no screen will show, to the references to clear once each ends */
+    private readonly _abandoned = new Map<string, string[]>();
+
+    /**
+     * Stop a job no screen will show. It ends without a notice, and its
+     * references are cleared once it has stopped. If the server refuses to
+     * cancel, the job stays watched so the clean up still happens when it ends.
+     */
+    public async abandon(id: string, reference_ids: string[]) {
+        this._abandoned.set(id, reference_ids);
+        const job = await this.cancel(id);
+        if (!isFinal(job) || !this._abandoned.has(id)) return;
+        this.unwatch(id);
+        this._ended(job);
+    }
+
+    /** a job reached its end: tell the person, or clear what it was left */
+    private _ended(job: ImageGenJob) {
+        const references = this._abandoned.get(job.id);
+        if (!references) return this._announce(job);
+        this._abandoned.delete(job.id);
+        references.forEach((id) => this.removeReference(id));
+    }
+
     public async claim(id: string, upload_id: string, item_id: string) {
         let last_error: unknown;
         for (const delay of CLAIM_RETRY_DELAYS) {
@@ -442,6 +466,8 @@ export class ImageGenService extends AsyncHandler {
                 wait: POLL_WAIT,
                 since: known,
             }).catch((error: unknown) => ({ error }));
+        // unwatched while this check was waiting on the server
+        if (!this._watching.has(id)) return;
 
         if ('error' in result) {
             const status = errorStatus(result.error);
@@ -462,7 +488,7 @@ export class ImageGenService extends AsyncHandler {
 
         if (isFinal(job)) {
             this.unwatch(id);
-            this._announce(job);
+            this._ended(job);
             this.refreshQuota();
             return;
         }
@@ -522,7 +548,7 @@ export class ImageGenService extends AsyncHandler {
             error_message: i18n('SIGNAGE_MANAGER.IMAGE_GEN_JOB_FAILED'),
         };
         this._merge([failed]);
-        this._announce(failed);
+        this._ended(failed);
     }
 
     private _merge(jobs: ImageGenJob[]) {
diff --git a/apps/signage-manager/src/tests/image-gen/image-gen-modal.component.spec.ts b/apps/signage-manager/src/tests/image-gen/image-gen-modal.component.spec.ts
index c55228a0dd2..f8a372795aa 100644
--- a/apps/signage-manager/src/tests/image-gen/image-gen-modal.component.spec.ts
+++ b/apps/signage-manager/src/tests/image-gen/image-gen-modal.component.spec.ts
@@ -137,6 +137,7 @@ describe('ImageGenModalComponent', () => {
             cancel: vi.fn(),
             unwatch: vi.fn(),
             setJobOnScreen: vi.fn(),
+            abandon: vi.fn(),
             claim: vi.fn().mockResolvedValue({}),
             removeReference: vi.fn(),
             loadImage: vi.fn().mockResolvedValue('blob:artwork'),
@@ -538,13 +539,10 @@ describe('ImageGenModalComponent', () => {
         await component.start();
 
         component.ngOnDestroy();
-        // the stopped job no longer reads the reference, so it goes too
-        await vi.waitFor(() =>
-            expect(image_gen.removeReference).toHaveBeenCalledWith('inc-1'),
-        );
 
-        expect(image_gen.cancel).toHaveBeenCalledWith('job-1');
-        expect(image_gen.unwatch).toHaveBeenCalledWith('job-1');
+        // the service clears the reference once the job has stopped
+        expect(image_gen.abandon).toHaveBeenCalledWith('job-1', ['inc-1']);
+        expect(image_gen.removeReference).not.toHaveBeenCalled();
     });
 
     it('stops a job the server accepts after the modal closed', async () => {
@@ -560,8 +558,27 @@ describe('ImageGenModalComponent', () => {
         accept(job('job-1', { state: 'running' }));
         await started;
 
-        expect(image_gen.cancel).toHaveBeenCalledWith('job-1');
-        expect(image_gen.unwatch).toHaveBeenCalledWith('job-1');
+        expect(image_gen.abandon).toHaveBeenCalledWith('job-1', []);
+    });
+
+    it('clears the attached images when a request fails after the modal closed', async () => {
+        const { image_gen, component } = await make();
+        let refuse: (error: Error) => void = () => undefined;
+        image_gen.generate.mockImplementationOnce(
+            () => new Promise<ImageGenJob>((_, reject) => (refuse = reject)),
+        );
+        component.include_references.set([
+            { id: 'inc-1', name: 'one.png', url: 'blob:one' },
+        ]);
+        component.brief.set('A poster for the launch');
+
+        const started = component.start();
+        component.ngOnDestroy();
+        expect(image_gen.removeReference).not.toHaveBeenCalled();
+        refuse(new Error('offline'));
+        await started;
+
+        expect(image_gen.removeReference).toHaveBeenCalledWith('inc-1');
     });
 
     it('lets go of an option whose image cannot be read', async () => {
diff --git a/apps/signage-manager/src/tests/image-gen/image-gen.service.spec.ts b/apps/signage-manager/src/tests/image-gen/image-gen.service.spec.ts
index 8b9a67c445a..e119408bc5f 100644
--- a/apps/signage-manager/src/tests/image-gen/image-gen.service.spec.ts
+++ b/apps/signage-manager/src/tests/image-gen/image-gen.service.spec.ts
@@ -1,6 +1,6 @@
 import { TestBed } from '@angular/core/testing';
 import { i18n, setNotifyOutlet, UploadsService } from '@placeos/common';
-import { get, post, updateMetadata } from '@placeos/ts-client';
+import { del, get, post, updateMetadata } from '@placeos/ts-client';
 
 import {
     ImageGenService,
@@ -41,6 +41,21 @@ function kitService(kit: ImageGenBrandKit = {}) {
     return service;
 }
 
+/** route notices to a mock, so a test can see what was shown */
+function captureNotices() {
+    const notify_open = vi.fn(() => ({
+        onAction: () => ({ subscribe: () => ({ unsubscribe: () => {} }) }),
+        dismiss: vi.fn(),
+    }));
+    setNotifyOutlet(
+        { open: notify_open } as unknown as Parameters<
+            typeof setNotifyOutlet
+        >[0],
+        true,
+    );
+    return notify_open;
+}
+
 /** the details of each brand kit write, in order */
 function writtenKits() {
     return vi
@@ -72,7 +87,10 @@ describe('ImageGenService', () => {
         });
     });
 
-    afterEach(() => vi.useRealTimers());
+    afterEach(() => {
+        vi.useRealTimers();
+        setNotifyOutlet(null, true);
+    });
 
     beforeEach(() => {
         vi.mocked(updateMetadata).mockResolvedValue(
@@ -300,16 +318,7 @@ describe('ImageGenService', () => {
     });
 
     it('does not announce a finished job that an open screen shows', async () => {
-        const notify_open = vi.fn(() => ({
-            onAction: () => ({ subscribe: () => ({ unsubscribe: () => {} }) }),
-            dismiss: vi.fn(),
-        }));
-        setNotifyOutlet(
-            { open: notify_open } as unknown as Parameters<
-                typeof setNotifyOutlet
-            >[0],
-            true,
-        );
+        const notify_open = captureNotices();
         const done = { ...runningJob(), state: 'done' as const, version: 2 };
         json_get.mockResolvedValue({ ...done, images_produced: 1 });
         const service = TestBed.inject(ImageGenService);
@@ -320,7 +329,6 @@ describe('ImageGenService', () => {
         await (
             service as unknown as { _poll: (id: string) => Promise<void> }
         )._poll(done.id);
-        setNotifyOutlet(null, true);
 
         expect(service.jobs()[done.id].state).toBe('done');
         expect(notify_open).not.toHaveBeenCalledWith(
@@ -329,4 +337,53 @@ describe('ImageGenService', () => {
             expect.anything(),
         );
     });
+
+    it('keeps watching a job it could not cancel, then clears it quietly', async () => {
+        vi.useFakeTimers();
+        const notify_open = captureNotices();
+        json_post.mockRejectedValue(new Error('cancel refused'));
+        vi.mocked(del).mockResolvedValue(undefined);
+        const service = TestBed.inject(ImageGenService);
+        const job = runningJob();
+        service.jobs.set({ [job.id]: job });
+        service.watch(job.id);
+
+        await service.abandon(job.id, ['inc-1']);
+        json_get.mockResolvedValue({
+            ...job,
+            state: 'done',
+            images_produced: 1,
+            version: 2,
+        });
+        await (
+            service as unknown as { _poll: (id: string) => Promise<void> }
+        )._poll(job.id);
+
+        expect(del).toHaveBeenCalledWith(
+            expect.stringContaining('/uploads/inc-1'),
+            expect.anything(),
+        );
+        expect(notify_open).not.toHaveBeenCalled();
+    });
+
+    it('ignores a check that comes back after the job is unwatched', async () => {
+        const notify_open = captureNotices();
+        const job = runningJob();
+        let answer: (value: ImageGenJob) => void = () => undefined;
+        json_get.mockImplementation(
+            () => new Promise((resolve) => (answer = resolve)),
+        );
+        const service = TestBed.inject(ImageGenService);
+        service.jobs.set({ [job.id]: job });
+        service.watch(job.id);
+
+        const check = (
+            service as unknown as { _poll: (id: string) => Promise<void> }
+        )._poll(job.id);
+        service.unwatch(job.id);
+        answer({ ...job, state: 'done', images_produced: 1, version: 2 });
+        await check;
+
+        expect(notify_open).not.toHaveBeenCalled();
+    });
 });