feat: implement unique default chatbot agent constraint and enhance related logic for attachment management

JoachimLK · JoachimLK · commit f11a78fced7d · 2026-04-29T06:30:25.000+02:00
diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml
@@ -101,8 +101,15 @@ jobs:
           # cosign signs by immutable digest; the tags only resolve the digest.
           # We sign each unique image reference so verification works against
           # any tag the user pulled (latest, semver, edge, sha-…).
-          for tag in $TAGS; do
-            image="${tag%%:*}"
-            echo "Signing ${image}@${DIGEST}"
-            cosign sign --yes "${image}@${DIGEST}"
-          done
+          # `${tag%:*}` (single %) strips only the trailing :tag so registries
+          # with ports (e.g. registry.local:5000/repo:tag) survive intact.
+          # Pipe through `sort -u` so the same image+digest is signed once
+          # even when the tag list contains many aliases.
+          printf '%s\n' $TAGS \
+            | awk 'NF' \
+            | sed -E 's#:[^:/]+$##' \
+            | sort -u \
+            | while IFS= read -r image; do
+                echo "Signing ${image}@${DIGEST}"
+                cosign sign --yes "${image}@${DIGEST}"
+              done
diff --git a/.github/workflows/pr-title-lint.yml b/.github/workflows/pr-title-lint.yml
@@ -15,7 +15,7 @@ name: PR Title Lint
 #   WIP
 
 on:
-  pull_request_target:
+  pull_request:
     types: [opened, edited, synchronize, reopened]
 
 permissions:
@@ -42,7 +42,6 @@ jobs:
             build
             ci
             chore
-            revert
           requireScope: false
           subjectPattern: ^[A-Za-z0-9].+$
           subjectPatternError: |
diff --git a/.github/workflows/release-verification.yml b/.github/workflows/release-verification.yml
@@ -35,7 +35,7 @@ jobs:
   smoke-test:
     name: Smoke-test published image
     runs-on: ubuntu-latest
-    timeout-minutes: 20
+    timeout-minutes: 35
     steps:
       - name: Resolve release tag
         id: tag
@@ -100,10 +100,20 @@ jobs:
       - name: Assert migrations + S3 bucket ready
         run: |
           set -euo pipefail
-          docker compose -f docker-compose.production.yml logs app | grep -q "Database migrations applied successfully" \
-            || { echo "❌ Migrations message missing"; docker compose -f docker-compose.production.yml logs app; exit 1; }
-          docker compose -f docker-compose.production.yml logs app | grep -q 'S3 bucket "reqcore" is ready' \
-            || { echo "❌ S3 bucket message missing"; docker compose -f docker-compose.production.yml logs app; exit 1; }
+          # Startup messages can land slightly after the HTTP port opens, so
+          # poll instead of one-shot grepping to avoid flaky failures.
+          for i in $(seq 40); do
+            logs="$(docker compose -f docker-compose.production.yml logs app || true)"
+            if grep -q "Database migrations applied successfully" <<<"$logs" \
+              && grep -q 'S3 bucket "reqcore" is ready' <<<"$logs"; then
+              echo "✅ Migrations + S3 ready messages found (attempt $i)"
+              exit 0
+            fi
+            sleep 3
+          done
+          echo "❌ Required startup messages missing after polling"
+          docker compose -f docker-compose.production.yml logs app
+          exit 1
 
       - name: Demote release to pre-release on failure
         if: failure() && github.event_name == 'release'
diff --git a/app/composables/useChatbot.ts b/app/composables/useChatbot.ts
@@ -419,8 +419,13 @@ export function useChatbot() {
     const attachmentIds = userMessage.attachments?.map((a) => a.id) ?? []
     pendingAttachments.value = []
 
+    // Use a per-call controller and only mutate the shared `abortController`
+    // / `isStreaming` state when this call still owns it. This prevents a
+    // late `finally` from a previous (aborted) send from clobbering the
+    // controller and streaming flag of a newer in-flight request.
     isStreaming.value = true
-    abortController = new AbortController()
+    const controller = new AbortController()
+    abortController = controller
 
     try {
       const res = await fetch('/api/chatbot/chat', {
@@ -442,7 +447,7 @@ export function useChatbot() {
                 : {}),
             })),
         }),
-        signal: abortController.signal,
+        signal: controller.signal,
       })
 
       if (!res.ok || !res.body) {
@@ -472,8 +477,11 @@ export function useChatbot() {
     } finally {
       // Force reactivity since we mutated assistantMessage in place.
       messages.value = [...messages.value]
-      isStreaming.value = false
-      abortController = null
+      // Only reset shared state if a newer send hasn't already taken over.
+      if (abortController === controller) {
+        isStreaming.value = false
+        abortController = null
+      }
     }
   }
 
diff --git a/nuxt.config.ts b/nuxt.config.ts
@@ -94,7 +94,7 @@ export default defineNuxtConfig({
   // Enable source maps so PostHog error tracking can display readable stack traces
   sourcemap: { client: "hidden" },
 
-  // @ts-expect-error - posthogConfig types only available when @posthog/nuxt module is loaded
+  // @ts-ignore - posthogConfig types only available when @posthog/nuxt module is loaded
   posthogConfig: {
     publicKey: process.env.POSTHOG_PUBLIC_KEY || "",
     host: process.env.POSTHOG_HOST || "https://eu.i.posthog.com",
@@ -244,7 +244,14 @@ export default defineNuxtConfig({
        * Self-hosters use these to enable/disable flags without running PostHog.
        * See `shared/feature-flags.ts` for the full registry and resolution order.
        */
-      featureFlagOverrides: readEnvFlagOverrides(),
+      // Cast: Nuxt narrows public runtime config from the registry's literal
+      // `defaultValue` types (boolean here), but env overrides can also be
+      // multivariate strings — and entries are partial. The override map is
+      // validated at runtime by `parseFlagOverride`, so the cast is safe.
+      featureFlagOverrides: readEnvFlagOverrides() as Record<
+        string,
+        boolean | string
+      >,
     },
   },
 
diff --git a/server/api/ai-config/[id]/set-default.post.ts b/server/api/ai-config/[id]/set-default.post.ts
@@ -1,4 +1,4 @@
-import { and, eq } from 'drizzle-orm'
+import { and, eq, sql } from 'drizzle-orm'
 import { z } from 'zod'
 import { aiConfig } from '../../../database/schema'
 import { setAiConfigDefaultSchema } from '../../../utils/schemas/scoring'
@@ -9,8 +9,11 @@ const paramsSchema = z.object({ id: z.string().min(1) })
  * POST /api/ai-config/:id/set-default
  *
  * Atomically claims one or more "default" slots (chatbot, analysis) for this
- * configuration. Clears the same flag on every other config in the org so
- * exactly one default exists per purpose.
+ * configuration. Uses a single UPDATE per purpose that sets the flag to true
+ * for the chosen row and false for every other row in the same organization,
+ * so the "exactly one default per purpose" invariant is preserved even under
+ * concurrent requests. The partial unique indexes on `is_default_chatbot` and
+ * `is_default_analysis` provide a DB-level backstop.
  */
 export default defineEventHandler(async (event) => {
   const session = await requirePermission(event, { scoring: ['create'] })
@@ -27,22 +30,20 @@ export default defineEventHandler(async (event) => {
   await db.transaction(async (tx) => {
     if (body.purposes.includes('chatbot')) {
       await tx.update(aiConfig)
-        .set({ isDefaultChatbot: false })
+        .set({
+          isDefaultChatbot: sql`${aiConfig.id} = ${id}`,
+          updatedAt: new Date(),
+        })
         .where(eq(aiConfig.organizationId, orgId))
     }
     if (body.purposes.includes('analysis')) {
       await tx.update(aiConfig)
-        .set({ isDefaultAnalysis: false })
+        .set({
+          isDefaultAnalysis: sql`${aiConfig.id} = ${id}`,
+          updatedAt: new Date(),
+        })
         .where(eq(aiConfig.organizationId, orgId))
     }
-
-    const promote: Record<string, unknown> = { updatedAt: new Date() }
-    if (body.purposes.includes('chatbot')) promote.isDefaultChatbot = true
-    if (body.purposes.includes('analysis')) promote.isDefaultAnalysis = true
-
-    await tx.update(aiConfig)
-      .set(promote)
-      .where(and(eq(aiConfig.id, id), eq(aiConfig.organizationId, orgId)))
   })
 
   recordActivity({
diff --git a/server/api/chatbot/agents/[id].patch.ts b/server/api/chatbot/agents/[id].patch.ts
@@ -41,16 +41,6 @@ export default defineEventHandler(async (event): Promise<{ agent: ChatbotAgent }
     throw createError({ statusCode: 404, statusMessage: 'Agent not found.' })
   }
 
-  // If switching this agent to default, clear other defaults first.
-  if (body.isDefault === true && !existing.isDefault) {
-    await db.update(chatbotAgent)
-      .set({ isDefault: false, updatedAt: new Date() })
-      .where(and(
-        eq(chatbotAgent.organizationId, orgId),
-        eq(chatbotAgent.userId, userId),
-      ))
-  }
-
   const updates: Partial<typeof chatbotAgent.$inferInsert> = { updatedAt: new Date() }
   if (body.name !== undefined) updates.name = body.name
   if (body.description !== undefined) updates.description = body.description
@@ -61,14 +51,29 @@ export default defineEventHandler(async (event): Promise<{ agent: ChatbotAgent }
   }
   if (body.isDefault !== undefined) updates.isDefault = body.isDefault
 
-  const [updated] = await db.update(chatbotAgent)
-    .set(updates)
-    .where(and(
-      eq(chatbotAgent.id, id),
-      eq(chatbotAgent.organizationId, orgId),
-      eq(chatbotAgent.userId, userId),
-    ))
-    .returning()
+  // Clear-and-set must be atomic so two concurrent "promote to default"
+  // requests cannot both observe `existing.isDefault === false` and leave
+  // multiple defaults. The partial unique index
+  // `chatbot_agent_default_per_user_idx` is the DB-level backstop.
+  const [updated] = await db.transaction(async (tx) => {
+    if (body.isDefault === true && !existing.isDefault) {
+      await tx.update(chatbotAgent)
+        .set({ isDefault: false, updatedAt: new Date() })
+        .where(and(
+          eq(chatbotAgent.organizationId, orgId),
+          eq(chatbotAgent.userId, userId),
+        ))
+    }
+
+    return tx.update(chatbotAgent)
+      .set(updates)
+      .where(and(
+        eq(chatbotAgent.id, id),
+        eq(chatbotAgent.organizationId, orgId),
+        eq(chatbotAgent.userId, userId),
+      ))
+      .returning()
+  })
 
   if (!updated) {
     throw createError({ statusCode: 500, statusMessage: 'Failed to update agent.' })
diff --git a/server/api/chatbot/agents/index.post.ts b/server/api/chatbot/agents/index.post.ts
@@ -1,4 +1,4 @@
-import { and, count, eq } from 'drizzle-orm'
+import { and, count, eq, sql } from 'drizzle-orm'
 import { z } from 'zod'
 import { chatbotAgent } from '../../../database/schema'
 import { requireChatbotAccess } from '../../../utils/chatbotAccess'
@@ -21,6 +21,14 @@ const bodySchema = z.object({
  * POST /api/chatbot/agents
  *
  * Create a custom AI agent (system prompt + persona) for the current user.
+ *
+ * The cap-check, default-clearing, and insert run inside a single
+ * transaction guarded by a Postgres advisory lock keyed on (org, user).
+ * The lock serialises concurrent create requests for the same user so the
+ * `CHATBOT_AGENT_MAX_PER_USER` cap cannot be exceeded by overlapping calls,
+ * and so a previous default is reliably cleared before a new default is
+ * inserted. The partial unique index `chatbot_agent_default_per_user_idx`
+ * is the DB-level backstop on the default invariant.
  */
 export default defineEventHandler(async (event): Promise<{ agent: ChatbotAgent }> => {
   const session = await requireChatbotAccess(event)
@@ -29,42 +37,52 @@ export default defineEventHandler(async (event): Promise<{ agent: ChatbotAgent }
 
   const body = await readValidatedBody(event, bodySchema.parse)
 
-  // Enforce per-user cap.
-  const [{ value: existing } = { value: 0 }] = await db
-    .select({ value: count() })
-    .from(chatbotAgent)
-    .where(and(
-      eq(chatbotAgent.organizationId, orgId),
-      eq(chatbotAgent.userId, userId),
-    ))
-
-  if (existing >= CHATBOT_AGENT_MAX_PER_USER) {
-    throw createError({
-      statusCode: 422,
-      statusMessage: `Agent limit reached (${CHATBOT_AGENT_MAX_PER_USER}). Delete an agent before adding another.`,
-    })
-  }
+  const created = await db.transaction(async (tx) => {
+    // Serialise concurrent create requests for the same (org, user) so the
+    // count → insert sequence is race-free. The lock is released on commit.
+    await tx.execute(
+      sql`select pg_advisory_xact_lock(hashtext(${`chatbot_agent:${orgId}:${userId}`}))`,
+    )
 
-  // If marking this agent as default, unset any previous default.
-  if (body.isDefault) {
-    await db.update(chatbotAgent)
-      .set({ isDefault: false, updatedAt: new Date() })
+    // Enforce per-user cap (now race-safe under the advisory lock).
+    const [{ value: existing } = { value: 0 }] = await tx
+      .select({ value: count() })
+      .from(chatbotAgent)
       .where(and(
         eq(chatbotAgent.organizationId, orgId),
         eq(chatbotAgent.userId, userId),
       ))
-  }
 
-  const [created] = await db.insert(chatbotAgent).values({
-    organizationId: orgId,
-    userId,
-    name: body.name,
-    description: body.description ?? null,
-    icon: body.icon ?? null,
-    systemPrompt: body.systemPrompt,
-    temperature: typeof body.temperature === 'number' ? String(body.temperature) : null,
-    isDefault: body.isDefault === true,
-  }).returning()
+    if (existing >= CHATBOT_AGENT_MAX_PER_USER) {
+      throw createError({
+        statusCode: 422,
+        statusMessage: `Agent limit reached (${CHATBOT_AGENT_MAX_PER_USER}). Delete an agent before adding another.`,
+      })
+    }
+
+    // If marking this agent as default, unset any previous default.
+    if (body.isDefault) {
+      await tx.update(chatbotAgent)
+        .set({ isDefault: false, updatedAt: new Date() })
+        .where(and(
+          eq(chatbotAgent.organizationId, orgId),
+          eq(chatbotAgent.userId, userId),
+        ))
+    }
+
+    const [row] = await tx.insert(chatbotAgent).values({
+      organizationId: orgId,
+      userId,
+      name: body.name,
+      description: body.description ?? null,
+      icon: body.icon ?? null,
+      systemPrompt: body.systemPrompt,
+      temperature: typeof body.temperature === 'number' ? String(body.temperature) : null,
+      isDefault: body.isDefault === true,
+    }).returning()
+
+    return row
+  })
 
   if (!created) {
     throw createError({ statusCode: 500, statusMessage: 'Failed to create agent.' })
diff --git a/server/api/chatbot/chat.post.ts b/server/api/chatbot/chat.post.ts
@@ -182,7 +182,7 @@ export default defineEventHandler(async (event) => {
   }
   const attachmentIds = lastUser.attachmentIds ?? []
   const attachmentRecords = attachmentIds.length
-    ? getChatbotAttachments(session.user.id, attachmentIds)
+    ? getChatbotAttachments(orgId, session.user.id, attachmentIds)
     : []
 
   if (attachmentIds.length > 0 && attachmentRecords.length === 0) {
diff --git a/server/database/migrations/0027_chatbot_agent_default_unique.sql b/server/database/migrations/0027_chatbot_agent_default_unique.sql
@@ -0,0 +1,14 @@
+-- Migration: enforce single default chatbot agent per (organization, user).
+--
+-- The chatbot_agent table allows one "default" agent per user inside an
+-- organisation (the agent pre-selected when starting a new conversation).
+-- The application code clears any prior default before promoting a new one,
+-- but this is racy under concurrent requests. This partial unique index
+-- enforces the invariant at the DB layer: any concurrent transaction that
+-- would create a second default row will fail with a unique-constraint
+-- violation, preserving correctness even if application-level coordination
+-- breaks down.
+
+CREATE UNIQUE INDEX IF NOT EXISTS "chatbot_agent_default_per_user_idx"
+  ON "chatbot_agent" ("organization_id", "user_id")
+  WHERE "is_default" = true;
diff --git a/server/database/migrations/meta/_journal.json b/server/database/migrations/meta/_journal.json
@@ -190,6 +190,13 @@
       "when": 1777200000000,
       "tag": "0026_multi_ai_configs",
       "breakpoints": true
+    },
+    {
+      "idx": 27,
+      "version": "7",
+      "when": 1777300000000,
+      "tag": "0027_chatbot_agent_default_unique",
+      "breakpoints": true
     }
   ]
 }
diff --git a/server/database/schema/app.ts b/server/database/schema/app.ts
@@ -788,6 +788,11 @@ export const chatbotAgent = pgTable('chatbot_agent', {
   updatedAt: timestamp('updated_at').notNull().defaultNow(),
 }, (t) => ([
   index('chatbot_agent_org_user_idx').on(t.organizationId, t.userId),
+  // Enforce single default agent per (org, user) at the DB layer to backstop
+  // the application-level clear-then-set logic against concurrent requests.
+  uniqueIndex('chatbot_agent_default_per_user_idx')
+    .on(t.organizationId, t.userId)
+    .where(sql`${t.isDefault} = true`),
 ]))
 
 /**
diff --git a/server/utils/chatbotAttachments.ts b/server/utils/chatbotAttachments.ts
diff --git a/server/utils/featureFlags.ts b/server/utils/featureFlags.ts
diff --git a/shared/feature-flags.ts b/shared/feature-flags.ts

Original file line number	Diff line number	Diff line change
`@@ -182,7 +182,7 @@ export default defineEventHandler(async (event) => {`
`182`	`182`	`}`
`183`	`183`	`const attachmentIds = lastUser.attachmentIds ?? []`
`184`	`184`	`const attachmentRecords = attachmentIds.length`
`185`		`- ? getChatbotAttachments(session.user.id, attachmentIds)`
	`185`	`+ ? getChatbotAttachments(orgId, session.user.id, attachmentIds)`
`186`	`186`	`: []`
`187`	`187`
`188`	`188`	`if (attachmentIds.length > 0 && attachmentRecords.length === 0) {`