fix(project): piloter la suppression externe avant d'archiver la ligne

apps/server-nestjs/src/modules/events/app-events.service.spec.ts

@@ -102,8 +102,8 @@ describe('appEventsService', () => {
 
     await service.emitProjectEvent('project.upsert', project, { action: 'Update Project' })
 
-    expect(prisma.project.update).toHaveBeenCalledWith({
-      where: { id: project.id },
+    expect(prisma.project.updateMany).toHaveBeenCalledWith({
+      where: { id: project.id, status: { not: 'archived' } },
       data: { status: 'failed' },
     })
   })
@@ -122,6 +122,20 @@ describe('appEventsService', () => {
     })
   })
 
+  it('does not overwrite an archived project when a replayed delete reports a stale KO', async () => {
+    const project = makeProject({ status: 'archived' })
+    eventEmitter.emitAsync.mockResolvedValue([
+      { keycloak: { status: 'KO', message: 'unknown_error', executionTime: 20, error: new Error('unknown_error') } },
+    ])
+
+    await service.emitProjectEvent('project.delete', project, { action: 'Delete all project resources' })
+
+    expect(prisma.project.updateMany).toHaveBeenCalledWith({
+      where: { id: project.id, status: { not: 'archived' } },
+      data: { status: 'failed' },
+    })
+  })
+
   it('does not touch the project status after a successful delete', async () => {
     const project = makeProject()
     eventEmitter.emitAsync.mockResolvedValue([

apps/server-nestjs/src/modules/events/app-events.service.ts

@@ -87,7 +87,7 @@ export class AppEventsService {
    * Pass a project id in the general case: the project is loaded here, after the
    * emitting transaction commits, so every listener sees the same committed state.
    * Pass an already-loaded snapshot when the row no longer reflects what listeners
-   * must act on (e.g. archiving renames the slug before emitting `project.delete`).
+   * must act on (e.g. archiving emits `project.delete` before renaming the slug).
    */
   async emitProjectEvent(
     event: ProjectEventName,
@@ -167,8 +167,9 @@ export class AppEventsService {
   /**
    * Reflects the listeners' outcome on the project row (legacy hooks behavior):
    * any KO result marks the project `failed`; a fully successful upsert marks it
-   * `created` and records the provisioning version. A successful `project.delete`
-   * leaves the `archived` status set when the project was archived.
+   * `created` and records the provisioning version. `project.delete` results are
+   * not written here: at emit time the row is not yet archived, and a success must
+   * leave the status untouched (the archiving transaction owns it).
    */
   private async updateProjectStatus(
     event: ProjectEventName,
@@ -179,7 +180,13 @@ export class AppEventsService {
 
     if (failed.length) {
       this.logger.warn(`${event} marked project as failed (projectId=${projectId}, failed=${failed.join(',')})`)
-      await this.prisma.project.update({ where: { id: projectId }, data: { status: 'failed' } })
+      // Never overwrite `archived`: a replayed `project.delete` on an already
+      // archived project is a retry of a cleanup that finished since, and its
+      // stale KO must not resurrect the row.
+      await this.prisma.project.updateMany({
+        where: { id: projectId, status: { not: 'archived' } },
+        data: { status: 'failed' },
+      })
       return
     }
 

apps/server-nestjs/src/modules/keycloak/keycloak-client.service.spec.ts

@@ -309,6 +309,18 @@ describe('deleteGroup', () => {
 
     await expect(service.deleteGroup('gone-id')).resolves.toBeUndefined()
   })
+
+  it('should swallow a 500 on delete when the group is gone on re-fetch (concurrent deletion)', async () => {
+    // Keycloak 26.7.0 answers 500 unknown_error instead of 404 when the deletion
+    // races another deletion of the same group
+    server.use(
+      http.get(childrenUrl, () => HttpResponse.json([])),
+      http.delete(groupUrl, () => HttpResponse.json({ error: 'unknown_error' }, { status: 500 })),
+      http.get(groupUrl, () => HttpResponse.json({ error: 'Not found' }, { status: 404 })),
+    )
+
+    await expect(service.deleteGroup('raced-id')).resolves.toBeUndefined()
+  })
 })
 
 describe('getOrCreateGroupByPath root resolution (issue #2518)', () => {

apps/server-nestjs/src/modules/keycloak/keycloak-client.service.ts

@@ -101,9 +101,18 @@ export class KeycloakClientService implements OnModuleInit {
       await this.client.groups.del({ id })
     } catch (err) {
       // The group may already be gone (deleted concurrently or pruned in a
-      // prior pass); a 404 is not a failure worth surfacing to the caller.
-      if (getErrorResponseStatus(err) !== 404) throw err
-      this.logger.warn(`Keycloak group ${id} was already deleted; skipping`)
+      // prior pass). Keycloak answers 404 in the simple case but can also
+      // answer 500 unknown_error when the deletion races another one, so a
+      // 500 is only fatal if the group still resolves on re-fetch.
+      if (getErrorResponseStatus(err) === 404) {
+        this.logger.warn(`Keycloak group ${id} was already deleted; skipping`)
+        return
+      }
+      if (getErrorResponseStatus(err) === 500 && !(await this.client.groups.findOne({ id }))) {
+        this.logger.warn(`Keycloak group ${id} was deleted concurrently (500 on delete, gone on re-fetch); skipping`)
+        return
+      }
+      throw err
     }
   }
 

apps/server-nestjs/src/modules/project/project.service.spec.ts

@@ -6,9 +6,11 @@ import type { UserContext } from '../infrastructure/auth/auth-user.decorator'
 import { faker } from '@faker-js/faker'
 import {
   BadRequestException,
+  ConflictException,
   ForbiddenException,
   InternalServerErrorException,
   NotFoundException,
+  UnprocessableEntityException,
 } from '@nestjs/common'
 import { Test } from '@nestjs/testing'
 import { beforeEach, describe, expect, it } from 'vitest'
@@ -462,30 +464,44 @@ describe('projectService', () => {
   })
 
   describe('archive', () => {
-    it('deletes related data, emits event, renames and archives project', async () => {
+    it('emits the delete event on the intact project, then renames and archives', async () => {
       const projectId = faker.string.uuid()
       const pwd = makeProjectWithDetails({ id: projectId, name: 'myproject', slug: 'myproject' })
+      prisma.project.findUnique.mockResolvedValue(pwd)
       const tx = mockDeep<Prisma.TransactionClient>()
-      tx.project.findUnique.mockResolvedValue(pwd)
       tx.repository.deleteMany.mockResolvedValue({ count: 2 })
       tx.environment.deleteMany.mockResolvedValue({ count: 3 })
       tx.deployment.deleteMany.mockResolvedValue({ count: 1 })
       tx.project.update.mockResolvedValue(makeProject({ id: projectId }))
       prisma.$transaction.mockImplementation(async cb => cb(tx))
+      // the lock write bumps `updatedAt` (@updatedAt); the post-lock row is the
+      // conflict baseline, so the archived row must match it, not the pre-lock snapshot
+      prisma.project.update.mockResolvedValue({ ...pwd, updatedAt: new Date('2026-01-02T00:00:00.000Z') })
+      tx.project.findUnique.mockResolvedValue(makeProjectWithDetails({ id: projectId, updatedAt: new Date('2026-01-02T00:00:00.000Z') }))
+      appEvents.emitProjectEvent.mockResolvedValue({})
 
       const requestId = faker.string.uuid()
       const requestorId = faker.string.uuid()
 
       await service.archive(projectId, requestorId, requestId)
 
-      expect(tx.repository.deleteMany).toHaveBeenCalledWith({ where: { projectId } })
-      expect(tx.environment.deleteMany).toHaveBeenCalledWith({ where: { projectId } })
-      expect(tx.deployment.deleteMany).toHaveBeenCalledWith({ where: { projectId } })
       expect(appEvents.emitProjectEvent).toHaveBeenCalledWith('project.delete', pwd, {
         action: 'Delete all project resources',
         userId: requestorId,
         requestId,
       })
+      // invariant: the row is locked before listeners run, and emit happens before
+      // any database write in the archiving transaction
+      expect(prisma.project.update).toHaveBeenCalledWith({ where: { id: projectId }, data: { locked: true } })
+      expect(prisma.project.update.mock.invocationCallOrder[0]).toBeLessThan(
+        appEvents.emitProjectEvent.mock.invocationCallOrder[0],
+      )
+      expect(appEvents.emitProjectEvent.mock.invocationCallOrder[0]).toBeLessThan(
+        tx.project.update.mock.invocationCallOrder[0],
+      )
+      expect(tx.repository.deleteMany).toHaveBeenCalledWith({ where: { projectId } })
+      expect(tx.environment.deleteMany).toHaveBeenCalledWith({ where: { projectId } })
+      expect(tx.deployment.deleteMany).toHaveBeenCalledWith({ where: { projectId } })
       expect(tx.project.update).toHaveBeenCalledWith(
         expect.objectContaining({
           where: { id: projectId },
@@ -498,30 +514,56 @@ describe('projectService', () => {
       )
     })
 
-    it('throws NotFoundException when project does not exist', async () => {
+    it('leaves the project untouched but locked when a listener reports a KO result, so a retry can re-drive cleanup', async () => {
+      const projectId = faker.string.uuid()
+      const pwd = makeProjectWithDetails({ id: projectId })
+      prisma.project.findUnique.mockResolvedValue(pwd)
+      prisma.project.update.mockResolvedValue(pwd)
+      appEvents.emitProjectEvent.mockResolvedValue({
+        gitlab: { status: 'KO', message: 'boom', executionTime: 1, error: null },
+      })
+
+      await expect(service.archive(projectId))
+        .rejects.toThrow(new UnprocessableEntityException('Echec des services à la suppression du projet'))
+
+      expect(prisma.project.update).toHaveBeenCalledWith({ where: { id: projectId }, data: { locked: true } })
+      expect(prisma.$transaction).not.toHaveBeenCalled()
+    })
+
+    it('keeps the lock set before cleanup when a mutation raced the snapshot (409, no unlock)', async () => {
+      const projectId = faker.string.uuid()
+      const pwd = makeProjectWithDetails({ id: projectId, updatedAt: new Date('2026-01-01T00:00:00.000Z') })
+      prisma.project.findUnique.mockResolvedValue(pwd)
+      prisma.project.update.mockResolvedValue(pwd)
+      appEvents.emitProjectEvent.mockResolvedValue({})
       const tx = mockDeep<Prisma.TransactionClient>()
-      tx.project.findUnique.mockResolvedValue(null)
+      tx.project.findUnique.mockResolvedValue(makeProjectWithDetails({ id: projectId, updatedAt: new Date('2026-01-02T00:00:00.000Z') }))
       prisma.$transaction.mockImplementation(async cb => cb(tx))
 
+      await expect(service.archive(projectId))
+        .rejects.toThrow(new ConflictException('Le projet a été modifié pendant la suppression, rejouez la requête'))
+
+      expect(prisma.project.update).toHaveBeenCalledWith({ where: { id: projectId }, data: { locked: true } })
+      expect(tx.project.update).not.toHaveBeenCalled()
+    })
+
+    it('throws NotFoundException when project does not exist', async () => {
+      prisma.project.findUnique.mockResolvedValue(null)
+
       await expect(service.archive(faker.string.uuid()))
         .rejects.toThrow(NotFoundException)
     })
 
     it('rejects an already archived project without side effects', async () => {
       const projectId = faker.string.uuid()
-      const tx = mockDeep<Prisma.TransactionClient>()
-      tx.project.findUnique.mockResolvedValue(
+      prisma.project.findUnique.mockResolvedValue(
         makeProjectWithDetails({ id: projectId, status: 'archived', locked: true }),
       )
-      prisma.$transaction.mockImplementation(async cb => cb(tx))
 
       await expect(service.archive(projectId))
         .rejects.toThrow(new BadRequestException('Le projet est archivé'))
 
-      expect(tx.repository.deleteMany).not.toHaveBeenCalled()
-      expect(tx.environment.deleteMany).not.toHaveBeenCalled()
-      expect(tx.deployment.deleteMany).not.toHaveBeenCalled()
-      expect(tx.project.update).not.toHaveBeenCalled()
+      expect(prisma.$transaction).not.toHaveBeenCalled()
       expect(appEvents.emitProjectEvent).not.toHaveBeenCalled()
     })
   })

apps/server-nestjs/src/modules/project/project.service.ts

@@ -4,13 +4,14 @@ import type { Prisma } from '@prisma/client'
 import type { UserContext } from '../infrastructure/auth/auth-user.decorator'
 import type { ProjectDataExport, ProjectUpdateContext, ProjectWithDetails } from './project-queries.utils'
 import { AdminAuthorized } from '@cpn-console/shared'
-import { BadRequestException, ForbiddenException, Inject, Injectable, InternalServerErrorException, Logger, NotFoundException } from '@nestjs/common'
+import { BadRequestException, ConflictException, ForbiddenException, Inject, Injectable, InternalServerErrorException, Logger, NotFoundException, UnprocessableEntityException } from '@nestjs/common'
 import { trace } from '@opentelemetry/api'
 import { baseConfigFactory } from '../../config/base.config'
 import { AppEventsService } from '../events/app-events.service'
 import { PrismaService } from '../infrastructure/database/prisma.service'
 import { StartActiveSpan } from '../infrastructure/telemetry/telemetry.decorator'
 import { LogService } from '../log/log.service'
+import { getFailedPlugins } from '../plugin/plugin.utils'
 import { createProjectMember, deleteProjectMember } from '../project-members/project-members-queries.utils'
 import {
   createProject,
@@ -189,10 +190,49 @@ export class ProjectService {
     span?.setAttribute('project.id', projectId)
     this.logger.log(`project.archive started (projectId=${projectId})`)
     try {
-      const project = await this.prisma.$transaction(async (tx) => {
+      const project = await getProject(this.prisma, projectId)
+      if (!project) throw new NotFoundException('Projet introuvable')
+      if (project.status === 'archived') throw new BadRequestException('Le projet est archivé')
+
+      // Serialize external cleanup against project mutations: setting `locked` before
+      // the emit fences every mutating route (`@RequireProjectLocked(false)` and the
+      // in-transaction `locked` re-check in `update`), so no upsert can reconcile
+      // resources behind the cleanup's back. The lock persists on a failed cleanup
+      // (`failed` status): the DELETE is replayable, and so is the unlock by an admin.
+      const locked = await this.prisma.project.update({ where: { id: projectId }, data: { locked: true } })
+      // emit before touching the rest of the database: listeners clean up resources
+      // named after the intact slug and still see repos and environments; a failed
+      // cleanup marks the project `failed` (AppEventsService) and a new DELETE can
+      // retry, listeners are idempotent (ensure-not-exists), the row was never
+      // renamed. The lock is deliberately never released here: it must also hold on
+      // failure (KO -> 422, conflict -> 409) so no mutation can reconcile resources
+      // between a failed cleanup and its replay.
+      const results = await this.appEvents.emitProjectEvent('project.delete', project, {
+        action: 'Delete all project resources',
+        userId: requestorUserId,
+        requestId,
+      })
+      const failed = getFailedPlugins(results)
+      if (failed.length) {
+        this.logger.warn(`project.archive external cleanup failed (projectId=${projectId}, failed=${failed.join(',')})`)
+        throw new UnprocessableEntityException('Echec des services à la suppression du projet')
+      }
+
+      await this.prisma.$transaction(async (tx) => {
         const loaded = await getProject(tx, projectId)
         if (!loaded) throw new NotFoundException('Projet introuvable')
-        if (loaded.status === 'archived') throw new BadRequestException('Le projet est archivé')
+        if (loaded.status === 'archived') {
+          this.logger.warn(`project already archived (projectId=${projectId}), answering 204 without writing`)
+          return
+        }
+        // The cleanup ran against the row as returned by the lock write. If it
+        // moved since (a mutation that raced the lock), resources were reconciled
+        // behind the cleanup: refuse to archive a stale snapshot, the DELETE is
+        // replayable. The lock write itself bumps `updatedAt` (`@updatedAt`), so
+        // the baseline must be the post-lock row, never the pre-lock snapshot.
+        if (loaded.updatedAt.getTime() !== locked.updatedAt.getTime()) {
+          throw new ConflictException('Le projet a été modifié pendant la suppression, rejouez la requête')
+        }
 
         await deleteProjectDependencies(tx, projectId)
 
@@ -204,16 +244,8 @@ export class ProjectService {
           locked: true,
           clusters: { set: [] },
         })
-
-        return loaded
-      })
-      // pass the pre-archive snapshot: the row was renamed (slug suffixed) in the
-      // transaction above, listeners must clean up resources named after the old slug
-      await this.appEvents.emitProjectEvent('project.delete', project, {
-        action: 'Delete all project resources',
-        userId: requestorUserId,
-        requestId,
       })
+
       span?.setAttribute('project.slug', project.slug)
       this.logger.log(`project.archive completed (projectId=${projectId}, slug=${project.slug})`)
     } catch (error) {

packages/shared/src/contracts/project.ts

@@ -161,6 +161,7 @@ export const projectContract = contractInstance.router({
     body: ContractNoBody,
     responses: {
       204: null,
+      422: ErrorSchema,
       500: ErrorSchema,
     },
   },