fix(sync): validate labelRules on read and fix two mappings-dashboard bugs

apps/web/src/__tests__/api-route-auth.test.ts

@@ -14,9 +14,19 @@ const mockAccountFindMany = vi.fn();
 const mockTicketGroupBy = vi.fn();
 const mockExternalIdentityFindMany = vi.fn();
 const mockSystemConfigFindUnique = vi.fn().mockResolvedValue(null);
+const mockSystemConfigUpsert = vi.fn().mockResolvedValue({});
 
 vi.mock('@copilotkit/outpost/db', () => ({
     prisma: {
+        // The mappings PUT reads and writes in one transaction so a concurrent save
+        // cannot drop label rules; the callback receives the same mocked client.
+        $transaction: async (fn: (tx: unknown) => unknown) =>
+            fn({
+                systemConfig: {
+                    findUnique: (...args: unknown[]) => mockSystemConfigFindUnique(...args),
+                    upsert: (...args: unknown[]) => mockSystemConfigUpsert(...args),
+                },
+            }),
         account: {
             create: (...args: unknown[]) => mockAccountCreate(...args),
             findMany: (...args: unknown[]) => mockAccountFindMany(...args),

apps/web/src/__tests__/sync-api.test.ts

@@ -15,6 +15,15 @@ const mockTicketExternalLinkFindFirst = vi.fn();
 
 vi.mock('@copilotkit/outpost/db', () => ({
     prisma: {
+        // The mappings PUT reads and writes in one transaction so a concurrent save
+        // cannot drop label rules; the callback receives the same mocked client.
+        $transaction: async (fn: (tx: unknown) => unknown) =>
+            fn({
+                systemConfig: {
+                    findUnique: (...args: unknown[]) => mockSystemConfigFindUnique(...args),
+                    upsert: (...args: unknown[]) => mockSystemConfigUpsert(...args),
+                },
+            }),
         syncEvent: {
             findMany: (...args: unknown[]) => mockSyncEventFindMany(...args),
             findFirst: (...args: unknown[]) => mockSyncEventFindFirst(...args),

apps/web/src/__tests__/sync-mappings-config-read.test.ts

@@ -7,9 +7,10 @@
  * as "never configured", because that renders the code defaults as though they
  * were the admin's saved settings.
  */
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 
 const mockSystemConfigFindUnique = vi.fn();
+const mockSystemConfigUpsert = vi.fn();
 const mockExternalIdentityFindMany = vi.fn();
 
 vi.mock('@copilotkit/outpost/db', () => ({
@@ -20,6 +21,15 @@ vi.mock('@copilotkit/outpost/db', () => ({
         externalIdentity: {
             findMany: (...args: unknown[]) => mockExternalIdentityFindMany(...args),
         },
+        // The PUT path reads and writes in one transaction so a concurrent save cannot
+        // drop label rules. The callback receives the same mocked client.
+        $transaction: async (fn: (tx: unknown) => unknown) =>
+            fn({
+                systemConfig: {
+                    findUnique: (...a: unknown[]) => mockSystemConfigFindUnique(...a),
+                    upsert: (...a: unknown[]) => mockSystemConfigUpsert(...a),
+                },
+            }),
     },
 }));
 
@@ -46,6 +56,12 @@ describe('GET /api/sync/mappings — persisted config read path', () => {
         mockExternalIdentityFindMany.mockResolvedValue([]);
     });
 
+    // Restored here, not at the end of each test body: a failed assertion would skip an
+    // inline mockRestore() and leave console.error stubbed for the rest of the file.
+    afterEach(() => {
+        vi.restoreAllMocks();
+    });
+
     it('reports defaults as defaults when no row exists', async () => {
         mockSystemConfigFindUnique.mockResolvedValue(null);
 
@@ -75,7 +91,6 @@ describe('GET /api/sync/mappings — persisted config read path', () => {
         expect(body.configSource).toBe('defaults');
         expect(body.configError).toContain('JSON');
         expect(errorSpy.mock.calls.flat().join(' ')).toContain('sync.mappingConfig');
-        errorSpy.mockRestore();
     });
 
     // A row written by an older version of the code, or hand-edited in the DB,
@@ -104,6 +119,41 @@ describe('GET /api/sync/mappings — persisted config read path', () => {
         // ...while the good one is still the admin's saved config.
         expect(body.priorityMappings).toEqual(VALID_CONFIG.priorityMappings);
         expect(errorSpy.mock.calls.flat().join(' ')).toContain('statusMappings');
-        errorSpy.mockRestore();
+    });
+
+    it('validates labelRules on read instead of passing a malformed value through', async () => {
+        // labelRules used to be served raw: a malformed value reached LabelRulesPanel,
+        // which calls ruleList.map(...) on it, and it was absent from invalidSections
+        // so nothing reported the problem.
+        const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+        mockSystemConfigFindUnique.mockResolvedValue({
+            value: JSON.stringify({
+                ...VALID_CONFIG,
+                labelRules: { linear: 'not-an-array' },
+            }),
+        });
+
+        const body = await (await GET()).json();
+
+        expect(body.invalidSections).toContain('labelRules');
+        // Positive assertion against the defaults this endpoint serves when nothing is
+        // persisted. `not.toEqual` would also pass if labelRules were undefined, which is
+        // a different bug wearing the same green tick.
+        mockSystemConfigFindUnique.mockResolvedValue(null);
+        const defaults = await (await GET()).json();
+        expect(body.labelRules).toEqual(defaults.labelRules);
+        expect(body.labelRules).toBeTruthy();
+        // The good sections are untouched.
+        expect(body.statusMappings).toEqual(VALID_CONFIG.statusMappings);
+        expect(errorSpy.mock.calls.flat().join(' ')).toContain('labelRules');
+    });
+
+    it('treats an absent labelRules as valid, since the section is optional', async () => {
+        mockSystemConfigFindUnique.mockResolvedValue({ value: JSON.stringify(VALID_CONFIG) });
+
+        const body = await (await GET()).json();
+
+        expect(body.invalidSections ?? []).not.toContain('labelRules');
+        expect(body.configSource).not.toBe('defaults');
     });
 });

apps/web/src/__tests__/sync-mappings-roundtrip.test.ts

@@ -11,7 +11,7 @@
  * That is exactly what shipped: the Linear priority defaults read '0 (None)'..
  * '4 (Low)' while LinearAdapter maps with String(data.priority) -> '0'..'4'.
  */
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { TicketPriority, TicketStatus } from '@copilotkit/outpost/shared';
 import {
     loadStatusMap,
@@ -25,6 +25,7 @@ import {
 const mockSystemConfigFindUnique = vi.fn();
 const mockSystemConfigUpsert = vi.fn();
 const mockExternalIdentityFindMany = vi.fn();
+const mockTransaction = vi.fn();
 const mockGetServerSession = vi.fn();
 
 vi.mock('@copilotkit/outpost/db', () => ({
@@ -34,6 +35,17 @@ vi.mock('@copilotkit/outpost/db', () => ({
             upsert: (...a: unknown[]) => mockSystemConfigUpsert(...a),
         },
         externalIdentity: { findMany: (...a: unknown[]) => mockExternalIdentityFindMany(...a) },
+        // The PUT path reads and writes in one transaction so a concurrent save cannot
+        // drop label rules. The callback receives the same mocked client.
+        $transaction: async (fn: (tx: unknown) => unknown) => {
+            mockTransaction(fn);
+            return fn({
+                systemConfig: {
+                    findUnique: (...a: unknown[]) => mockSystemConfigFindUnique(...a),
+                    upsert: (...a: unknown[]) => mockSystemConfigUpsert(...a),
+                },
+            });
+        },
     },
 }));
 
@@ -125,3 +137,188 @@ describe('mappings round-trip: GET defaults -> PUT -> load as the worker does',
         expect(loaded.toOutpost(['wontfix'])).toEqual([]);
     });
 });
+
+describe('PUT /api/sync/mappings — labelRules preservation and empty-array rejection', () => {
+    const VALID_STATUS = { linear: [{ externalStatus: 'Done', outpostStatus: 'RESOLVED' }] };
+    const VALID_PRIORITY = { linear: [{ externalPriority: '1', outpostPriority: 'CRITICAL' }] };
+    const SAVED_LABEL_RULES = { github: [{ externalPrefix: 'bug', outpostPrefix: 'defect' }] };
+
+    beforeEach(() => {
+        vi.clearAllMocks();
+        mockExternalIdentityFindMany.mockResolvedValue([]);
+        mockSystemConfigUpsert.mockResolvedValue({});
+    });
+
+    function put(body: Record<string, unknown>) {
+        return PUT(
+            new Request('http://localhost:3000/api/sync/mappings', {
+                method: 'PUT',
+                headers: { 'Content-Type': 'application/json' },
+                body: JSON.stringify(body),
+            }) as never,
+        );
+    }
+
+    function persistedValue() {
+        return JSON.parse(mockSystemConfigUpsert.mock.calls[0][0].update.value);
+    }
+
+    it('carries forward existing labelRules when the PUT omits the key', async () => {
+        // The upsert replaces the whole row, so omitting labelRules used to drop
+        // previously persisted rules — a silent wipe reachable through a door the
+        // `labelRules: {}` rejection did not cover.
+        mockSystemConfigFindUnique.mockResolvedValue({
+            value: JSON.stringify({
+                statusMappings: VALID_STATUS,
+                priorityMappings: VALID_PRIORITY,
+                labelRules: SAVED_LABEL_RULES,
+            }),
+        });
+
+        const res = await put({
+            statusMappings: VALID_STATUS,
+            priorityMappings: VALID_PRIORITY,
+        });
+
+        expect(res.status).toBe(200);
+        expect(persistedValue().labelRules).toEqual(SAVED_LABEL_RULES);
+    });
+
+    it('rejects an empty per-plugin labelRules array, as the sibling validator does', async () => {
+        // loadLabelMapper treats [] as "nothing persisted, use defaults", so saving
+        // it would show an empty list while the worker kept applying built-in rules.
+        mockSystemConfigFindUnique.mockResolvedValue(null);
+
+        const res = await put({
+            statusMappings: VALID_STATUS,
+            priorityMappings: VALID_PRIORITY,
+            labelRules: { linear: [] },
+        });
+
+        expect(res.status).toBe(400);
+        expect(mockSystemConfigUpsert).not.toHaveBeenCalled();
+    });
+
+    it('still writes an explicitly supplied labelRules value', async () => {
+        mockSystemConfigFindUnique.mockResolvedValue(null);
+
+        const res = await put({
+            statusMappings: VALID_STATUS,
+            priorityMappings: VALID_PRIORITY,
+            labelRules: SAVED_LABEL_RULES,
+        });
+
+        expect(res.status).toBe(200);
+        expect(persistedValue().labelRules).toEqual(SAVED_LABEL_RULES);
+    });
+});
+
+describe('PUT /api/sync/mappings — atomicity, un-carriable rows, and label validation', () => {
+    const VALID_STATUS = { linear: [{ externalStatus: 'Done', outpostStatus: 'RESOLVED' }] };
+    const VALID_PRIORITY = { linear: [{ externalPriority: '1', outpostPriority: 'CRITICAL' }] };
+    const SAVED_LABEL_RULES = { github: [{ externalPrefix: 'bug', outpostPrefix: 'defect' }] };
+
+    beforeEach(() => {
+        vi.clearAllMocks();
+        mockExternalIdentityFindMany.mockResolvedValue([]);
+        mockSystemConfigUpsert.mockResolvedValue({});
+    });
+
+    afterEach(() => {
+        vi.restoreAllMocks();
+    });
+
+    function put(body: Record<string, unknown>) {
+        return PUT(
+            new Request('http://localhost:3000/api/sync/mappings', {
+                method: 'PUT',
+                headers: { 'Content-Type': 'application/json' },
+                body: JSON.stringify(body),
+            }) as never,
+        );
+    }
+
+    it('carries labelRules forward inside a transaction, not as two statements', async () => {
+        // Read-then-write as separate statements lets a concurrent PUT land between them
+        // and lose its rules. Pin that both happen through $transaction.
+        mockSystemConfigFindUnique.mockResolvedValue({
+            value: JSON.stringify({
+                statusMappings: VALID_STATUS,
+                priorityMappings: VALID_PRIORITY,
+                labelRules: SAVED_LABEL_RULES,
+            }),
+        });
+
+        const res = await put({ statusMappings: VALID_STATUS, priorityMappings: VALID_PRIORITY });
+
+        expect(res.status).toBe(200);
+        expect(mockTransaction).toHaveBeenCalledTimes(1);
+        const persisted = JSON.parse(mockSystemConfigUpsert.mock.calls[0][0].update.value);
+        expect(persisted.labelRules).toEqual(SAVED_LABEL_RULES);
+    });
+
+    it('says so when existing labelRules are unusable and cannot be carried', async () => {
+        const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+        mockSystemConfigFindUnique.mockResolvedValue({
+            value: JSON.stringify({
+                statusMappings: VALID_STATUS,
+                priorityMappings: VALID_PRIORITY,
+                labelRules: { linear: 'not-an-array' },
+            }),
+        });
+
+        const res = await put({ statusMappings: VALID_STATUS, priorityMappings: VALID_PRIORITY });
+
+        expect(res.status).toBe(200);
+        const persisted = JSON.parse(mockSystemConfigUpsert.mock.calls[0][0].update.value);
+        // Unusable rules are dropped rather than persisted...
+        expect(persisted.labelRules).toBeUndefined();
+        // ...but the drop is reported, so it is not silent.
+        expect(errorSpy.mock.calls.flat().join(' ')).toContain('labelRules');
+    });
+
+    it('returns the persisted config so the client can store what was actually saved', async () => {
+        mockSystemConfigFindUnique.mockResolvedValue({
+            value: JSON.stringify({
+                statusMappings: VALID_STATUS,
+                priorityMappings: VALID_PRIORITY,
+                labelRules: SAVED_LABEL_RULES,
+            }),
+        });
+
+        const body = await (
+            await put({ statusMappings: VALID_STATUS, priorityMappings: VALID_PRIORITY })
+        ).json();
+
+        // The request omitted labelRules; the response must still show them, otherwise the
+        // dashboard drops rules that are saved.
+        expect(body.labelRules).toEqual(SAVED_LABEL_RULES);
+    });
+
+    it('rejects a non-string label on a priority entry', async () => {
+        mockSystemConfigFindUnique.mockResolvedValue(null);
+
+        const res = await put({
+            statusMappings: VALID_STATUS,
+            priorityMappings: {
+                linear: [{ externalPriority: '1', outpostPriority: 'CRITICAL', label: 42 }],
+            },
+        });
+
+        expect(res.status).toBe(400);
+        expect(mockSystemConfigUpsert).not.toHaveBeenCalled();
+    });
+
+    it('accepts a string label', async () => {
+        mockSystemConfigFindUnique.mockResolvedValue(null);
+
+        const res = await put({
+            statusMappings: VALID_STATUS,
+            priorityMappings: {
+                linear: [{ externalPriority: '1', outpostPriority: 'CRITICAL', label: 'Urgent' }],
+            },
+        });
+
+        expect(res.status).toBe(200);
+    });
+});