fix: patch four correctness bugs found in state/plugin/API layer

package.json

@@ -1,7 +1,7 @@
 {
   "name": "worldwideview",
 
-  "version": "2.68.25",
+  "version": "2.68.26",
 
   "private": true,
   "license": "EL-2.0",

src/app/api/v1/entities/region/route.ts

@@ -62,7 +62,7 @@ export async function GET(request: NextRequest) {
     const pluginId = searchParams.get("pluginId") ?? undefined;
     const rawLimit = searchParams.get("limit");
     const parsedLimit = rawLimit !== null ? parseInt(rawLimit, 10) : NaN;
-    const limit = Math.min(Number.isNaN(parsedLimit) ? 100 : parsedLimit, 1000);
+    const limit = Math.min(Math.max(Number.isNaN(parsedLimit) ? 100 : parsedLimit, 1), 1000);
 
     try {
         const result = await getEntitiesInRegion({ north, south, east, west, pluginId, limit });

src/core/plugins/PluginManager.test.ts

@@ -150,6 +150,23 @@ describe("PluginManager.enablePlugin", () => {
     });
 });
 
+describe("PluginManager — enable/disable race", () => {
+    it("does not restart polling or re-announce enabled if disabled while enablePlugin was still awaiting", async () => {
+        const layerHandler = vi.fn();
+        await pluginManager.registerPlugin(makePlugin({ id: "race-1" }));
+
+        const enabling = pluginManager.enablePlugin("race-1");
+        // Fast toggle-off before the enable's internal awaits resolve.
+        pluginManager.disablePlugin("race-1");
+        dataBus.on("layerToggled", layerHandler);
+
+        await enabling;
+
+        expect(pluginManager.getPlugin("race-1")?.enabled).toBe(false);
+        expect(layerHandler).not.toHaveBeenCalledWith({ pluginId: "race-1", enabled: true });
+    });
+});
+
 describe("PluginManager.disablePlugin", () => {
     it("clears entities and emits layerToggled(false) + dataUpdated([])", async () => {
         const layerHandler = vi.fn();

src/core/plugins/PluginManager.ts

@@ -40,6 +40,11 @@ class PluginManager {
     private loadedManifests: Map<string, PluginManifest> = new Map();
     private initialized = false;
     private configCacheMaxAge = 3600000;
+    // Bumped by every enablePlugin/disablePlugin call for a given plugin so
+    // an in-flight enablePlugin can detect it was superseded by a later
+    // toggle (e.g. a fast enable->disable) after resuming from an await,
+    // and bail out instead of re-enabling a plugin the user just disabled.
+    private toggleEpoch: Map<string, number> = new Map();
 
     /**
      * Initializes the PluginManager and prepares the persistent cache layer.
@@ -216,10 +221,21 @@ class PluginManager {
     async enablePlugin(pluginId: string): Promise<void> {
         const start = performance.now();
         console.debug(`[PluginManager] enablePlugin called for ${pluginId}`);
+        const epoch = (this.toggleEpoch.get(pluginId) ?? 0) + 1;
+        this.toggleEpoch.set(pluginId, epoch);
+
         // Ensure local manifest is fetched so we don't accidentally fall back to cloud if toggled too fast
         await fetchLocalEngineManifest();
         console.debug(`[PluginManager] Manifest fetched for ${pluginId}. Took ${(performance.now() - start).toFixed(2)}ms`);
 
+        // A disablePlugin() (or a newer enablePlugin()) call may have landed
+        // while we were awaiting above (e.g. the user toggled the layer off
+        // before this resolved). Bail out so we don't re-enable a plugin the
+        // user just disabled, or clobber a newer toggle's state.
+        if (this.toggleEpoch.get(pluginId) !== epoch) {
+            return;
+        }
+
         const managed = this.plugins.get(pluginId);
         if (!managed) {
             console.error(`[PluginManager] Plugin ${pluginId} not found in managed plugins`);
@@ -236,6 +252,11 @@ class PluginManager {
             cached = await cacheLayer.getFromPersistent(pluginId);
         }
 
+        // Re-check after the second await for the same reason as above.
+        if (this.toggleEpoch.get(pluginId) !== epoch) {
+            return;
+        }
+
         // If still enabled and we got cached data, emit it
         if (cached && managed.enabled) {
             managed.entities = cached;
@@ -256,6 +277,7 @@ class PluginManager {
      */
     disablePlugin(pluginId: string): void {
         console.debug(`[PluginManager] disablePlugin called for ${pluginId}`);
+        this.toggleEpoch.set(pluginId, (this.toggleEpoch.get(pluginId) ?? 0) + 1);
         const managed = this.plugins.get(pluginId);
         if (!managed) {
             console.error(`[PluginManager] Plugin ${pluginId} not found during disable`);

src/core/state/dataSlice.test.ts

@@ -6,9 +6,11 @@ import { createStore, type StoreApi } from 'zustand/vanilla';
 import type { GeoEntity } from '@/core/plugins/PluginTypes';
 import { createDataSlice, DataSlice } from './dataSlice';
 
-// We need a mock store that includes selectedEntity to test the cross-slice update behavior
+// We need a mock store that includes selectedEntity and layers to test the
+// cross-slice update behavior and getAllEntities' enabled-flag filtering.
 interface MockAppStore extends DataSlice {
     selectedEntity: GeoEntity | null;
+    layers: Record<string, { enabled: boolean }>;
 }
 
 describe('dataSlice', () => {
@@ -20,6 +22,7 @@ describe('dataSlice', () => {
             return {
                 ...dataSlice,
                 selectedEntity: null,
+                layers: {},
             };
         });
     });
@@ -70,10 +73,16 @@ describe('dataSlice', () => {
         expect(store.getState().entitiesByPlugin['plugin-b']).toEqual(mockEntitiesB);
     });
 
-    it('gets all entities across plugins flattened', () => {
+    it('gets all entities across enabled plugins flattened', () => {
         const mockEntitiesA = [{ id: 'e1', pluginId: 'plugin-a' } as GeoEntity];
         const mockEntitiesB = [{ id: 'e2', pluginId: 'plugin-b' } as GeoEntity];
 
+        store.setState({
+            layers: {
+                'plugin-a': { enabled: true },
+                'plugin-b': { enabled: true },
+            },
+        });
         store.getState().setEntities('plugin-a', mockEntitiesA);
         store.getState().setEntities('plugin-b', mockEntitiesB);
 
@@ -83,4 +92,23 @@ describe('dataSlice', () => {
         expect(all).toContainEqual(mockEntitiesA[0]);
         expect(all).toContainEqual(mockEntitiesB[0]);
     });
+
+    it('excludes entities from disabled plugins', () => {
+        const mockEntitiesA = [{ id: 'e1', pluginId: 'plugin-a' } as GeoEntity];
+        const mockEntitiesB = [{ id: 'e2', pluginId: 'plugin-b' } as GeoEntity];
+
+        store.setState({
+            layers: {
+                'plugin-a': { enabled: true },
+                'plugin-b': { enabled: false },
+            },
+        });
+        store.getState().setEntities('plugin-a', mockEntitiesA);
+        store.getState().setEntities('plugin-b', mockEntitiesB);
+
+        const all = store.getState().getAllEntities();
+
+        expect(all).toHaveLength(1);
+        expect(all).toContainEqual(mockEntitiesA[0]);
+    });
 });

src/core/state/dataSlice.ts

@@ -42,6 +42,8 @@ export const createDataSlice: StateCreator<AppStore, [], [], DataSlice> = (set,
         }),
     getAllEntities: () => {
         const state = get();
-        return Object.values(state.entitiesByPlugin).flat();
+        return Object.entries(state.entitiesByPlugin)
+            .filter(([pluginId]) => state.layers?.[pluginId]?.enabled)
+            .flatMap(([, entities]) => entities);
     },
 });

src/core/state/favoritesSlice.cloud.test.ts

@@ -0,0 +1,77 @@
+/* eslint-disable @typescript-eslint/no-explicit-any */
+import {
+ describe, it, expect, beforeEach, vi, afterEach
+} from 'vitest';
+import { createStore, type StoreApi } from 'zustand/vanilla';
+import type { GeoEntity } from '@/core/plugins/PluginTypes';
+import { createFavoritesSlice, FavoritesSlice } from './favoritesSlice';
+
+// isDemo=false so addFavorite/removeFavorite exercise the fetch (cloud/local
+// edition) branch instead of the cookie branch.
+vi.mock('@/core/edition', () => ({
+    isDemo: false,
+}));
+
+interface MockAppStore extends FavoritesSlice {
+    showErrorToast?: (message: string) => void;
+}
+
+describe('favoritesSlice (cloud mode) — optimistic update rollback', () => {
+    let store: StoreApi<MockAppStore>;
+    let showErrorToast: ReturnType<typeof vi.fn<(message: string) => void>>;
+
+    beforeEach(() => {
+        showErrorToast = vi.fn<(message: string) => void>();
+        store = createStore<MockAppStore>((set, get, api) => ({
+            ...createFavoritesSlice(set as any, get as any, api as any),
+            showErrorToast,
+        }));
+    });
+
+    afterEach(() => {
+        vi.restoreAllMocks();
+        vi.unstubAllGlobals();
+    });
+
+    const mockEntity = { id: 'entity-1', pluginId: 'plugin-a', label: 'Test Entity' } as GeoEntity;
+
+    it('reverts the optimistic add and surfaces an error when the POST fails', async () => {
+        vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 500 }));
+
+        store.getState().addFavorite(mockEntity, 'Plugin A');
+        expect(store.getState().favorites).toHaveLength(1);
+
+        // Let the fetch promise's .then/.catch chain settle.
+        await vi.waitFor(() => {
+            expect(store.getState().favorites).toHaveLength(0);
+        });
+        expect(showErrorToast).toHaveBeenCalled();
+    });
+
+    it('keeps the optimistic add when the POST succeeds', async () => {
+        vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, status: 200 }));
+
+        store.getState().addFavorite(mockEntity, 'Plugin A');
+        await Promise.resolve();
+        await Promise.resolve();
+
+        expect(store.getState().favorites).toHaveLength(1);
+        expect(showErrorToast).not.toHaveBeenCalled();
+    });
+
+    it('reverts the optimistic remove and surfaces an error when the DELETE fails', async () => {
+        vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, status: 200 }));
+        store.getState().initFavorites([{
+            id: 'entity-1', pluginId: 'plugin-a', label: 'Test Entity', pluginName: 'Plugin A', lastSeen: 1,
+        }]);
+
+        vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 500 }));
+        store.getState().removeFavorite('entity-1');
+        expect(store.getState().favorites).toHaveLength(0);
+
+        await vi.waitFor(() => {
+            expect(store.getState().favorites).toHaveLength(1);
+        });
+        expect(showErrorToast).toHaveBeenCalled();
+    });
+});

src/core/state/favoritesSlice.ts

@@ -85,12 +85,20 @@ export const createFavoritesSlice: StateCreator<AppStore, [], [], FavoritesSlice
                     label: entity.label || entity.id,
                     pluginName
                 })
-            }).catch((e) => console.error("Failed to sync add favorite to DB:", e));
+            }).then((res) => {
+                if (!res.ok) throw new Error(`Request failed with status ${res.status}`);
+            }).catch((e) => {
+                console.error("Failed to sync add favorite to DB:", e);
+                // Revert the optimistic update so UI state matches the backend.
+                set((s) => ({ favorites: s.favorites.filter((f) => f.id !== entity.id) }));
+                get().showErrorToast?.("Failed to save favorite. Please try again.");
+            });
         }
     },
 
     removeFavorite: (id) => {
         const state = get();
+        const removed = state.favorites.find((f) => f.id === id);
         const newFavorites = state.favorites.filter((f) => f.id !== id);
         set({ favorites: newFavorites });
 
@@ -99,7 +107,16 @@ export const createFavoritesSlice: StateCreator<AppStore, [], [], FavoritesSlice
         } else {
             fetch(`/api/user/favorites?entityId=${encodeURIComponent(id)}`, {
                 method: "DELETE"
-            }).catch((e) => console.error("Failed to sync remove favorite from DB:", e));
+            }).then((res) => {
+                if (!res.ok) throw new Error(`Request failed with status ${res.status}`);
+            }).catch((e) => {
+                console.error("Failed to sync remove favorite from DB:", e);
+                // Revert the optimistic update so UI state matches the backend.
+                if (removed) {
+                    set((s) => (s.favorites.some((f) => f.id === id) ? {} : { favorites: [...s.favorites, removed] }));
+                }
+                get().showErrorToast?.("Failed to remove favorite. Please try again.");
+            });
         }
     },
 });

src/lib/data-query/service.test.ts

@@ -335,6 +335,23 @@ describe("getEntitiesInRegion", () => {
         expect(result.entities[0].id).toBe("e1");
     });
 
+    it("clamps a negative limit to the floor of 1 instead of producing a negative slice bound", async () => {
+        const entities = [
+            makeEntity({ id: "e1", latitude: 51.5, longitude: -0.1, pluginId: "test-plugin" }),
+            makeEntity({ id: "e2", latitude: 51.6, longitude: -0.2, pluginId: "test-plugin" }),
+        ];
+        mockEngineSnapshot("test-plugin", entities);
+        const result = await getEntitiesInRegion({
+            north: 52,
+            south: 50,
+            east: 1,
+            west: -1,
+            pluginId: "test-plugin",
+            limit: -5,
+        });
+        expect(result.entities).toHaveLength(1);
+    });
+
     it("respects pluginId filter", async () => {
         const entity = makeEntity({ id: "e1", latitude: 51.5, longitude: -0.1, pluginId: "specific-plugin" });
         mockEngineSnapshot("specific-plugin", [entity]);

src/lib/data-query/service.ts

@@ -250,7 +250,7 @@ export async function getEntitiesInRegion(bounds: RegionOptions): Promise<QueryR
         throw new Error("Invalid bounding box: north/south/east/west must all be numbers");
     }
 
-    const effectiveLimit = Math.min(bounds.limit ?? 100, 1000);
+    const effectiveLimit = Math.min(Math.max(bounds.limit ?? 100, 1), 1000);
     const isAntimeridian = east < west;
 
     let snapshotNotStreaming = false;