fix: untag reviewed image names during prune (#1836)

backend/src/__tests__/fleet-action-card-endpoints.test.ts

@@ -1301,6 +1301,7 @@ describe('POST /api/fleet/labels/fleet-prune with dryRun: true', () => {
     executePrunePlan.mockResolvedValue({
       success: true,
       reclaimedBytes: 0,
+      mutated: true,
       outcomes: [{ target: 'images', id: 'image', status: 'removed' }],
     });
     const res = await request(app)
@@ -1337,7 +1338,12 @@ describe('POST /api/fleet/labels/fleet-prune with dryRun: true', () => {
         nodeId: local.id, scope: 'managed', targets: ['images'], items: [...testCase.items],
         reclaimableBytes: 0, fingerprint, createdAt: 1,
       });
-      executePrunePlan.mockResolvedValue({ success: true, reclaimedBytes: 0, outcomes: [...testCase.outcomes] });
+      executePrunePlan.mockResolvedValue({
+        success: true,
+        reclaimedBytes: 0,
+        mutated: false,
+        outcomes: [...testCase.outcomes],
+      });
       invalidateNodeCaches.mockClear();
       const res = await request(app)
         .post('/api/fleet/labels/fleet-prune')

backend/src/__tests__/fleet-prune.test.ts

@@ -3,6 +3,7 @@ import request from 'supertest';
 import jwt from 'jsonwebtoken';
 import { cleanupTestDb, setupTestDb, TEST_JWT_SECRET, TEST_USERNAME } from './helpers/setupTestDb';
 import type { PruneItemOutcome, PrunePlan, PrunePlanItem } from '../services/prunePlan';
+import { CacheService } from '../services/CacheService';
 
 let tmpDir: string;
 let app: import('express').Express;
@@ -62,9 +63,11 @@ function mockLocal(planFactory: (nodeId: number) => PrunePlan = (nodeId) => plan
       success: boolean;
       reclaimedBytes: number;
       outcomes: PruneItemOutcome[];
+      mutated: boolean;
     }> => ({
       success: true,
       reclaimedBytes: reviewedPlan.reclaimableBytes,
+      mutated: reviewedPlan.items.length > 0,
       outcomes: reviewedPlan.items.map((entry) => ({
         id: entry.id,
         target: entry.target,
@@ -212,6 +215,30 @@ describe('POST /api/fleet/labels/fleet-prune', () => {
     expect(activeBulkActions.size).toBe(0);
   });
 
+  it('invalidates local node caches when a failed image outcome is mutated', async () => {
+    const fake = mockLocal();
+    fake.executePrunePlan.mockResolvedValue({
+      success: false,
+      reclaimedBytes: 0,
+      mutated: true,
+      outcomes: [{ target: 'images', id: 'sha256:image', status: 'failed', error: 'tags remain' }],
+    });
+    const invalidate = vi.spyOn(CacheService.getInstance(), 'invalidate');
+    const local = DatabaseService.getInstance().getNodes().find((node) => node.type === 'local')!;
+    const response = await request(app)
+      .post('/api/fleet/labels/fleet-prune')
+      .set('Authorization', authHeader)
+      .send({
+        targets: ['images'], scope: 'managed', dryRun: false,
+        reviewedNodes: [{ nodeId: local.id, reachable: true }],
+        plans: [{ nodeId: local.id, fingerprint: `fingerprint-${local.id}` }],
+      });
+    expect(response.status).toBe(200);
+    expect(response.body.results[0].outcomes[0].status).toBe('failed');
+    expect(invalidate).toHaveBeenCalledWith(`stats:${local.id}`);
+    expect(invalidate).toHaveBeenCalledWith(`stack-statuses:${local.id}`);
+  });
+
   it('fails closed when a local prune lock is active', async () => {
     const fake = mockLocal();
     const { local, body } = localReview(`fingerprint-${DatabaseService.getInstance().getNodes()[0].id}`);
@@ -440,6 +467,7 @@ describe('POST /api/fleet/labels/fleet-prune', () => {
     fake.executePrunePlan.mockResolvedValue({
       success: false,
       reclaimedBytes: 100,
+      mutated: true,
       outcomes: [
         { target: 'images', id: 'removed', status: 'removed', sizeBytes: 100 },
         { target: 'images', id: 'skipped', status: 'skipped', reason: 'became active' },

backend/src/__tests__/prune-plan.test.ts

@@ -659,9 +659,77 @@ describe('DockerController.buildPrunePlan', () => {
 
     expect(plan.items.map((i) => i.id)).toEqual(['img-free']);
   });
+
+  it('excludes a fully synthetic sencho-rb hold image from the plan even without a DB hold', async () => {
+    mockDocker.listImages.mockResolvedValue([
+      {
+        Id: 'img-orphan-hold',
+        RepoTags: ['sencho-rb/abc123456789/web:hold', 'sencho-rb/abc123456789/api:hold'],
+        Size: 50,
+        Containers: 0,
+      },
+      { Id: 'img-free', RepoTags: ['app:2'], Size: 100, Containers: 0 },
+    ]);
+
+    const dc = DockerController.getInstance(1);
+    const plan = await dc.buildPrunePlan(['images'], 'all', [], 1, () => false);
+
+    expect(plan.items.map((i) => i.id)).toEqual(['img-free']);
+  });
+
+  it('still plans a dual-tagged image that carries a registry tag and a sencho-rb hold tag', async () => {
+    mockDocker.listImages.mockResolvedValue([
+      {
+        Id: 'img-dual',
+        RepoTags: ['myregistry/app:1.4', 'sencho-rb/abc123456789/app:hold'],
+        Size: 100,
+        Containers: 0,
+      },
+    ]);
+
+    const dc = DockerController.getInstance(1);
+    const plan = await dc.buildPrunePlan(['images'], 'all', [], 1);
+
+    expect(plan.items.map((i) => i.id)).toEqual(['img-dual']);
+    expect(plan.items[0]?.image?.references).toEqual([
+      'myregistry/app:1.4',
+      'sencho-rb/abc123456789/app:hold',
+    ]);
+  });
 });
 
 describe('DockerController.executePrunePlan', () => {
+  const multiTagImage = {
+    Id: 'img-multi',
+    RepoTags: ['qa1769/app:v0', 'qa1769-external/keep:v0'],
+    Size: 50,
+    Containers: 0,
+  };
+
+  function mockImageRemove(
+    onRemove?: (name: string, opts?: { force?: boolean }) => void | Promise<void>,
+  ): Array<{ name: string; force?: boolean }> {
+    const removes: Array<{ name: string; force?: boolean }> = [];
+    mockDocker.getImage.mockImplementation((name: string) => ({
+      remove: vi.fn().mockImplementation(async (opts?: { force?: boolean }) => {
+        removes.push({ name, force: opts?.force });
+        await onRemove?.(name, opts);
+      }),
+    }));
+    return removes;
+  }
+
+  async function executeForcedFreshImagePlan(listings: Array<unknown[] | Error>) {
+    for (const listing of listings) {
+      if (listing instanceof Error) mockDocker.listImages.mockRejectedValueOnce(listing);
+      else mockDocker.listImages.mockResolvedValueOnce(listing);
+    }
+    const dc = DockerController.getInstance(1);
+    const plan = await dc.buildPrunePlan(['images'], 'all', [], 1);
+    vi.spyOn(dc, 'assertPlanFresh').mockResolvedValue(plan);
+    return dc.executePrunePlan(plan, []);
+  }
+
   it('throws PrunePlanStaleError when the fingerprint no longer matches', async () => {
     mockDocker.listContainers.mockResolvedValue([
       {
@@ -852,6 +920,49 @@ describe('DockerController.executePrunePlan', () => {
     ]);
   });
 
+  it('skips a fully synthetic hold image even under a forced-fresh plan', async () => {
+    const holdImg = {
+      Id: 'img-orphan-hold',
+      RepoTags: ['sencho-rb/abc123456789/web:hold', 'sencho-rb/abc123456789/api:hold'],
+      Size: 50,
+      Containers: 0,
+    };
+    mockDocker.listImages.mockResolvedValue([holdImg]);
+    const removes = mockImageRemove();
+
+    const dc = DockerController.getInstance(1);
+    const plan = {
+      scope: 'all' as const,
+      targets: ['images' as const],
+      items: [{
+        target: 'images' as const,
+        id: holdImg.Id,
+        name: holdImg.RepoTags[0],
+        sizeBytes: 50,
+        managed: false,
+        reason: 'Image is not used by any container',
+        image: { references: holdImg.RepoTags },
+      }],
+      reclaimableBytes: 50,
+      fingerprint: 'forced-hold',
+      createdAt: Date.now(),
+      nodeId: 1,
+    };
+    vi.spyOn(dc, 'assertPlanFresh').mockResolvedValue(plan);
+    const result = await dc.executePrunePlan(plan, []);
+
+    expect(removes).toEqual([]);
+    expect(result.mutated).toBe(false);
+    expect(result.outcomes).toEqual([
+      expect.objectContaining({
+        id: holdImg.Id,
+        target: 'images',
+        status: 'skipped',
+        reason: 'Sencho rollback-hold image',
+      }),
+    ]);
+  });
+
   it('marks the plan stale when a free image gains a new RepoTag between plan and rebuild', async () => {
     const img = {
       Id: 'img-retag',
@@ -896,29 +1007,177 @@ describe('DockerController.executePrunePlan', () => {
     }));
   });
 
-  it('surfaces multi-repository refuse without a silent partial untag report', async () => {
-    mockDocker.listImages.mockResolvedValue([
-      {
-        Id: 'img-multi',
-        RepoTags: ['qa1769/app:v0', 'qa1769-external/keep:v0'],
-        Size: 50,
-        Containers: 0,
-      },
-    ]);
-    const imageRemove = vi.fn().mockRejectedValue(
-      Object.assign(new Error('conflict: unable to delete (must be forced) - image is referenced in multiple repositories'), { statusCode: 409 }),
-    );
-    mockDocker.getImage.mockReturnValue({ remove: imageRemove });
+  it('untags each reviewed name and reports removed once the image is gone', async () => {
+    const removes = mockImageRemove();
+    const result = await executeForcedFreshImagePlan([[multiTagImage], [multiTagImage], []]);
+
+    expect(removes.map((entry) => entry.name)).toEqual(['qa1769/app:v0', 'qa1769-external/keep:v0']);
+    expect(result.outcomes[0]).toEqual(expect.objectContaining({
+      id: 'img-multi',
+      status: 'removed',
+    }));
+    expect(result.mutated).toBe(true);
+    expect(result.success).toBe(true);
+  });
+
+  it('treats a first-ref 404 as absent and still removes when a later reviewed tag succeeds', async () => {
+    const removes = mockImageRemove(async (name) => {
+      if (name === 'qa1769/app:v0') {
+        throw Object.assign(new Error('No such image: qa1769/app:v0'), { statusCode: 404 });
+      }
+    });
+    const result = await executeForcedFreshImagePlan([[multiTagImage], [multiTagImage], []]);
+
+    expect(removes).toEqual([
+      { name: 'qa1769/app:v0', force: false },
+      { name: 'qa1769-external/keep:v0', force: false },
+    ]);
+    expect(result.outcomes[0]).toEqual(expect.objectContaining({ status: 'removed' }));
+    expect(result.mutated).toBe(true);
+    expect(result.success).toBe(true);
+  });
+
+  it('does not report removed when a later 404 leaves another reviewed tag present', async () => {
+    mockImageRemove(async (name) => {
+      if (name === 'qa1769-external/keep:v0') {
+        throw Object.assign(new Error('No such image'), { statusCode: 404 });
+      }
+    });
+    const result = await executeForcedFreshImagePlan([
+      [multiTagImage],
+      [multiTagImage],
+      [{ ...multiTagImage, RepoTags: ['qa1769/app:v0'] }],
+    ]);
 
-    const dc = DockerController.getInstance(1);
-    const plan = await dc.buildPrunePlan(['images'], 'all', [], 1);
-    vi.spyOn(dc, 'assertPlanFresh').mockResolvedValue(plan);
-    const result = await dc.executePrunePlan(plan, []);
-    expect(imageRemove).toHaveBeenCalled();
     expect(result.outcomes[0]).toEqual(expect.objectContaining({
       status: 'failed',
-      error: expect.stringMatching(/multiple repositories/i),
+      error: expect.stringMatching(/qa1769\/app:v0/),
     }));
-    expect(String(result.outcomes[0] && 'error' in result.outcomes[0] ? result.outcomes[0].error : '')).toMatch(/qa1769\/app:v0/);
+    expect(result.outcomes[0]).not.toEqual(expect.objectContaining({ status: 'removed' }));
+    expect(result.mutated).toBe(true);
+    expect(result.success).toBe(false);
+  });
+
+  it('stops on a hard untag error, lists remaining tags, and marks mutated', async () => {
+    const removes = mockImageRemove(async (name) => {
+      if (name === 'qa1769-external/keep:v0') {
+        throw Object.assign(new Error('conflict: unable to delete (must be forced)'), { statusCode: 409 });
+      }
+    });
+    const result = await executeForcedFreshImagePlan([
+      [multiTagImage],
+      [multiTagImage],
+      [{ ...multiTagImage, RepoTags: ['qa1769-external/keep:v0'] }],
+    ]);
+
+    expect(removes.map((entry) => entry.name)).toEqual(['qa1769/app:v0', 'qa1769-external/keep:v0']);
+    expect(result.outcomes[0]).toEqual(expect.objectContaining({
+      status: 'failed',
+      error: expect.stringMatching(/qa1769-external\/keep:v0/),
+    }));
+    expect(result.mutated).toBe(true);
+    expect(result.success).toBe(false);
+  });
+
+  it('never removes an unexpected tag that appears during execution', async () => {
+    const img = { Id: 'img-multi', RepoTags: ['qa1769/app:v0'], Size: 50, Containers: 0 };
+    const removes = mockImageRemove();
+    const result = await executeForcedFreshImagePlan([
+      [img],
+      [img],
+      [{ ...img, RepoTags: ['qa1769-external/keep:v0'] }],
+    ]);
+
+    expect(removes.map((entry) => entry.name)).toEqual(['qa1769/app:v0']);
+    expect(removes.map((entry) => entry.name)).not.toContain('qa1769-external/keep:v0');
+    expect(result.outcomes[0]).toEqual(expect.objectContaining({
+      status: 'failed',
+      error: expect.stringMatching(/not in the reviewed set/i),
+    }));
+    expect(result.mutated).toBe(true);
+  });
+
+  it('removes a dangling planned image by id with force false', async () => {
+    const img = { Id: 'img-dang', RepoTags: ['<none>:<none>'], Size: 20, Containers: 0 };
+    const removes = mockImageRemove();
+    const result = await executeForcedFreshImagePlan([[img], [img], [img]]);
+
+    expect(removes).toEqual([{ name: 'img-dang', force: false }]);
+    expect(result.outcomes[0]).toEqual(expect.objectContaining({ status: 'removed' }));
+    expect(result.mutated).toBe(true);
+    expect(result.success).toBe(true);
+  });
+
+  it('id-removes leftover none tags after reviewed names are gone', async () => {
+    const img = { Id: 'img-multi', RepoTags: ['qa1769/app:v0'], Size: 50, Containers: 0 };
+    const removes = mockImageRemove();
+    const result = await executeForcedFreshImagePlan([
+      [img],
+      [img],
+      [{ ...img, RepoTags: ['<none>:<none>'] }],
+    ]);
+
+    expect(removes).toEqual([
+      { name: 'qa1769/app:v0', force: false },
+      { name: 'img-multi', force: false },
+    ]);
+    expect(result.outcomes[0]).toEqual(expect.objectContaining({ status: 'removed' }));
+    expect(result.mutated).toBe(true);
+    expect(result.success).toBe(true);
+  });
+
+  it('does not treat a 409 as missing when the conflict text contains 404', async () => {
+    const img = {
+      Id: 'sha256:abc404def',
+      RepoTags: ['qa1769/app:v0', 'qa1769/app:latest'],
+      Size: 50,
+      Containers: 0,
+    };
+    const removes = mockImageRemove(async () => {
+      throw Object.assign(
+        new Error('(HTTP code 409) conflict - image sha256:abc404def is being used by running container'),
+        { statusCode: 409 },
+      );
+    });
+    const result = await executeForcedFreshImagePlan([[img], [img], [img]]);
+
+    expect(removes).toEqual([{ name: 'qa1769/app:v0', force: false }]);
+    expect(result.outcomes[0]).toEqual(expect.objectContaining({
+      status: 'failed',
+      error: expect.stringMatching(/qa1769\/app:v0/),
+    }));
+    expect(result.mutated).toBe(false);
+    expect(result.success).toBe(false);
+  });
+
+  it('does not report dangling id-remove 409 as removed when the message contains 404', async () => {
+    const img = { Id: 'sha256:abc404def', RepoTags: ['<none>:<none>'], Size: 20, Containers: 0 };
+    mockImageRemove(async () => {
+      throw Object.assign(
+        new Error('(HTTP code 409) conflict - unable to delete sha256:abc404def (must be forced)'),
+        { statusCode: 409 },
+      );
+    });
+    const result = await executeForcedFreshImagePlan([[img], [img], [img]]);
+
+    expect(result.outcomes[0]).toEqual(expect.objectContaining({
+      status: 'failed',
+      error: expect.stringMatching(/dangling image could not be removed/i),
+    }));
+    expect(result.mutated).toBe(false);
+    expect(result.success).toBe(false);
+  });
+
+  it('fails honestly when completion re-list throws after a successful untag', async () => {
+    const img = { Id: 'img-multi', RepoTags: ['qa1769/app:v0'], Size: 50, Containers: 0 };
+    mockImageRemove();
+    const result = await executeForcedFreshImagePlan([[img], [img], new Error('daemon busy')]);
+
+    expect(result.outcomes[0]).toEqual(expect.objectContaining({
+      status: 'failed',
+      error: expect.stringMatching(/could not confirm remaining/i),
+    }));
+    expect(result.mutated).toBe(true);
+    expect(result.success).toBe(false);
   });
 });

backend/src/__tests__/system-maintenance-prune.test.ts

@@ -174,6 +174,7 @@ describe('Prune plan routes', () => {
       outcomes: [{ id: 'v1', target: 'volumes', status: 'removed', sizeBytes: 42 }],
       reclaimedBytes: 42,
       success: true,
+      mutated: true,
     });
     vi.spyOn(DockerController, 'getInstance').mockReturnValue({
       buildPrunePlan: vi.fn().mockResolvedValue(plan),
@@ -190,6 +191,7 @@ describe('Prune plan routes', () => {
     expect(res.body.success).toBe(true);
     expect(res.body.reclaimedBytes).toBe(42);
     expect(res.body.outcomes).toHaveLength(1);
+    expect(res.body).not.toHaveProperty('mutated');
     expect(executePrunePlan).toHaveBeenCalled();
     expect(invalidate).toHaveBeenCalledWith('stats:1');
     expect(invalidate).toHaveBeenCalledWith('stack-statuses:1');
@@ -268,4 +270,80 @@ describe('Prune plan routes', () => {
     expect(res.body.items).toHaveLength(1);
     expect(executePrunePlan).not.toHaveBeenCalled();
   });
+
+  it('invalidates node caches when a failed image outcome is mutated', async () => {
+    stubFsStacks();
+    const plan = samplePlan('fp-partial');
+    vi.spyOn(DockerController, 'getInstance').mockReturnValue({
+      buildPrunePlan: vi.fn().mockResolvedValue(plan),
+      executePrunePlan: vi.fn().mockResolvedValue({
+        outcomes: [{ id: 'img-multi', target: 'images', status: 'failed', error: 'tags remain' }],
+        reclaimedBytes: 0,
+        success: false,
+        mutated: true,
+      }),
+    } as unknown as ReturnType<typeof DockerController.getInstance>);
+    const invalidate = vi.spyOn(CacheService.getInstance(), 'invalidate');
+
+    const res = await request(app)
+      .post('/api/system/prune/system')
+      .set('Authorization', authHeader)
+      .send({ target: 'volumes', scope: 'managed', planFingerprint: 'fp-partial' });
+
+    expect(res.status).toBe(200);
+    expect(res.body.success).toBe(false);
+    expect(res.body.outcomes[0].status).toBe('failed');
+    expect(invalidate).toHaveBeenCalledWith('stats:1');
+    expect(invalidate).toHaveBeenCalledWith('stack-statuses:1');
+  });
+
+  it('does not invalidate node caches when a failed outcome is not mutated', async () => {
+    stubFsStacks();
+    const plan = samplePlan('fp-nomut');
+    vi.spyOn(DockerController, 'getInstance').mockReturnValue({
+      buildPrunePlan: vi.fn().mockResolvedValue(plan),
+      executePrunePlan: vi.fn().mockResolvedValue({
+        outcomes: [{ id: 'img-multi', target: 'images', status: 'failed', error: 'references changed' }],
+        reclaimedBytes: 0,
+        success: false,
+        mutated: false,
+      }),
+    } as unknown as ReturnType<typeof DockerController.getInstance>);
+    const invalidate = vi.spyOn(CacheService.getInstance(), 'invalidate');
+
+    const res = await request(app)
+      .post('/api/system/prune/system')
+      .set('Authorization', authHeader)
+      .send({ target: 'volumes', scope: 'managed', planFingerprint: 'fp-nomut' });
+
+    expect(res.status).toBe(200);
+    expect(invalidate).not.toHaveBeenCalled();
+  });
+
+  it('legacy no-fingerprint containers prune omits mutated from the JSON body', async () => {
+    stubFsStacks();
+    const executePrunePlan = vi.fn().mockResolvedValue({
+      outcomes: [{ id: 'c1', target: 'containers', status: 'removed' }],
+      reclaimedBytes: 0,
+      success: true,
+      mutated: true,
+    });
+    vi.spyOn(DockerController, 'getInstance').mockReturnValue({
+      buildPrunePlan: vi.fn().mockResolvedValue(samplePlan('unused')),
+      executePrunePlan,
+    } as unknown as ReturnType<typeof DockerController.getInstance>);
+
+    const res = await request(app)
+      .post('/api/system/prune/system')
+      .set('Authorization', authHeader)
+      .send({ target: 'containers', scope: 'managed' });
+
+    expect(res.status).toBe(200);
+    expect(res.body.success).toBe(true);
+    expect(res.body.reclaimedBytes).toBe(0);
+    expect(res.body.outcomes).toEqual([
+      { id: 'c1', target: 'containers', status: 'removed' },
+    ]);
+    expect(res.body).not.toHaveProperty('mutated');
+  });
 });