fix: eliminate $_SESSION drift in SessionProvider and add notification UI states

Pass AppContainer to SessionProvider::clear() so all module providers use
SessionStoreInterface consistently instead of raw $_SESSION. Also add
loading and error states to the notification bell dropdown (GR-UI-014).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

lib/App/Module/Contracts/SessionProvider.php

@@ -22,5 +22,5 @@ interface SessionProvider
     /**
      * Remove session keys owned by this module (called on logout).
      */
-    public function clear(): void;
+    public function clear(AppContainer $container): void;
 }

lib/Service/Auth/AuthSessionTenantContextService.php

@@ -54,7 +54,7 @@ class AuthSessionTenantContextService
     public function clearModuleSessionData(): void
     {
         foreach ($this->moduleSessionProviders() as $provider) {
-            $provider->clear();
+            $provider->clear($this->container);
         }
     }
 

modules/addressbook/lib/Module/AddressBook/Providers/AddressBookSessionProvider.php

@@ -15,15 +15,23 @@ final class AddressBookSessionProvider implements SessionProvider
 {
     public function populate(array $user, AppContainer $container): void
     {
-        $userId = (int) ($user['id'] ?? 0);
-        if ($userId <= 0) {
-            unset($_SESSION['module.addressbook.departments_by_tenant']);
+        $sessionStore = $container->get(SessionStoreInterface::class);
+        if (!$sessionStore instanceof SessionStoreInterface) {
             return;
         }
 
-        $tenantContext = $container->get(UserTenantContextService::class);
-        $sessionStore = $container->get(SessionStoreInterface::class);
-        if (!$tenantContext instanceof UserTenantContextService || !$sessionStore instanceof SessionStoreInterface) {
+        $userId = (int) ($user['id'] ?? 0);
+        if ($userId <= 0) {
+            $sessionStore->remove('module.addressbook.departments_by_tenant');
+            return;
+        }
+
+        try {
+            $tenantContext = $container->get(UserTenantContextService::class);
+        } catch (\Throwable) {
+            return;
+        }
+        if (!$tenantContext instanceof UserTenantContextService) {
             return;
         }
 
@@ -33,8 +41,11 @@ final class AddressBookSessionProvider implements SessionProvider
         );
     }
 
-    public function clear(): void
+    public function clear(AppContainer $container): void
     {
-        unset($_SESSION['module.addressbook.departments_by_tenant']);
+        $sessionStore = $container->get(SessionStoreInterface::class);
+        if ($sessionStore instanceof SessionStoreInterface) {
+            $sessionStore->remove('module.addressbook.departments_by_tenant');
+        }
     }
 }

modules/bookmarks/lib/Module/Bookmarks/Providers/BookmarksSessionProvider.php

@@ -13,23 +13,30 @@ final class BookmarksSessionProvider implements SessionProvider
 
     public function populate(array $user, AppContainer $container): void
     {
-        $userId = (int) ($user['id'] ?? 0);
-        if ($userId <= 0) {
-            unset($_SESSION[self::SESSION_KEY]);
+        $sessionStore = $container->get(SessionStoreInterface::class);
+        if (!$sessionStore instanceof SessionStoreInterface) {
+            return;
+        }
+
+        $userId = (int) ($user['id'] ?? 0);
+        if ($userId <= 0) {
+            $sessionStore->remove(self::SESSION_KEY);
             return;
         }
 
-        $sessionStore = $container->get(SessionStoreInterface::class);
         $bookmarkService = $container->get(BookmarkService::class);
-        if (!$sessionStore instanceof SessionStoreInterface || !$bookmarkService instanceof BookmarkService) {
+        if (!$bookmarkService instanceof BookmarkService) {
             return;
         }
 
         $sessionStore->set(self::SESSION_KEY, $bookmarkService->listGroupedForUser($userId));
     }
 
-    public function clear(): void
+    public function clear(AppContainer $container): void
     {
-        unset($_SESSION[self::SESSION_KEY]);
+        $sessionStore = $container->get(SessionStoreInterface::class);
+        if ($sessionStore instanceof SessionStoreInterface) {
+            $sessionStore->remove(self::SESSION_KEY);
+        }
     }
 }

modules/notifications/i18n/default_de.json

@@ -5,6 +5,8 @@
   "Mark as read": "Als gelesen markieren",
   "Dismiss": "Verwerfen",
   "Action failed": "Aktion fehlgeschlagen",
+  "Loading notifications…": "Benachrichtigungen werden geladen…",
+  "Failed to load notifications": "Benachrichtigungen konnten nicht geladen werden",
   "New user: %s": "Neuer Benutzer: %s",
   "User deleted: %s": "Benutzer gelöscht: %s",
   "User activated: %s": "Benutzer aktiviert: %s",

modules/notifications/i18n/default_en.json

@@ -5,6 +5,8 @@
   "Mark as read": "Mark as read",
   "Dismiss": "Dismiss",
   "Action failed": "Action failed",
+  "Loading notifications…": "Loading notifications…",
+  "Failed to load notifications": "Failed to load notifications",
   "New user: %s": "New user: %s",
   "User deleted: %s": "User deleted: %s",
   "User activated: %s": "User activated: %s",

modules/notifications/lib/Module/Notifications/Providers/NotificationsSessionProvider.php

@@ -13,15 +13,19 @@ final class NotificationsSessionProvider implements SessionProvider
 
     public function populate(array $user, AppContainer $container): void
     {
-        $userId = (int) ($user['id'] ?? 0);
-        if ($userId <= 0) {
-            unset($_SESSION[self::SESSION_KEY]);
+        $sessionStore = $container->get(SessionStoreInterface::class);
+        if (!$sessionStore instanceof SessionStoreInterface) {
+            return;
+        }
+
+        $userId = (int) ($user['id'] ?? 0);
+        if ($userId <= 0) {
+            $sessionStore->remove(self::SESSION_KEY);
             return;
         }
 
-        $sessionStore = $container->get(SessionStoreInterface::class);
         $service = $container->get(NotificationService::class);
-        if (!$sessionStore instanceof SessionStoreInterface || !$service instanceof NotificationService) {
+        if (!$service instanceof NotificationService) {
             return;
         }
 
@@ -30,8 +34,11 @@ final class NotificationsSessionProvider implements SessionProvider
         $sessionStore->set(self::SESSION_KEY, $service->unreadCount($userId, $tenantId > 0 ? $tenantId : null));
     }
 
-    public function clear(): void
+    public function clear(AppContainer $container): void
     {
-        unset($_SESSION[self::SESSION_KEY]);
+        $sessionStore = $container->get(SessionStoreInterface::class);
+        if ($sessionStore instanceof SessionStoreInterface) {
+            $sessionStore->remove(self::SESSION_KEY);
+        }
     }
 }

modules/notifications/templates/topbar-notification-bell.phtml

@@ -26,6 +26,14 @@ $unreadCount = (int) ($notifNav['unread_count'] ?? 0);
                 </button>
             </li>
             <li class="app-notification-dropdown__body" data-notification-list>
+                <div class="app-notification-loading" data-notification-loading style="display:none;">
+                    <span class="app-notification-loading__spinner"></span>
+                    <span class="app-notification-loading__text"><?php e(t('Loading notifications…')); ?></span>
+                </div>
+                <div class="app-notification-error" data-notification-error style="display:none;">
+                    <i class="bi bi-exclamation-triangle"></i>
+                    <span><?php e(t('Failed to load notifications')); ?></span>
+                </div>
                 <?php
                 $emptyState = [
                     'message' => t('No notifications'),

modules/notifications/web/css/components/app-notification-dropdown.css

@@ -156,4 +156,33 @@
     color: var(--app-notification-text);
     background: var(--app-notification-border);
   }
+
+  .app-notification-loading,
+  .app-notification-error {
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    gap: 8px;
+    padding: 24px 16px;
+    font-size: 13px;
+    color: var(--app-notification-muted);
+  }
+
+  .app-notification-error {
+    color: var(--app-danger, #c0392b);
+  }
+
+  .app-notification-loading__spinner {
+    display: inline-block;
+    width: 16px;
+    height: 16px;
+    border: 2px solid var(--app-notification-border);
+    border-top-color: var(--app-primary);
+    border-radius: 50%;
+    animation: app-notification-spin 0.6s linear infinite;
+  }
+
+  @keyframes app-notification-spin {
+    to { transform: rotate(360deg); }
+  }
 }

modules/notifications/web/js/components/app-notification-bell.js

@@ -59,6 +59,8 @@ export function initNotificationBell(root = document, config = {}) {
     const badge = details.querySelector('[data-notification-badge]');
     const list = dropdown.querySelector('[data-notification-list]');
     const emptyEl = list?.querySelector('.app-empty-state');
+    const loadingEl = list?.querySelector('[data-notification-loading]');
+    const errorEl = list?.querySelector('[data-notification-error]');
     const markAllBtn = dropdown.querySelector('[data-notification-mark-all]');
 
     const ac = new AbortController();
@@ -77,18 +79,25 @@ export function initNotificationBell(root = document, config = {}) {
         }
     }
 
+    // --- State management ---
+    function showState(state) {
+        if (loadingEl) loadingEl.style.display = state === 'loading' ? '' : 'none';
+        if (errorEl) errorEl.style.display = state === 'error' ? '' : 'none';
+        if (emptyEl) emptyEl.style.display = state === 'empty' ? '' : 'none';
+    }
+
     // --- Render ---
     function renderNotifications(notifications) {
         if (!list) return;
 
-        // Remove rendered items, keep empty state partial
+        // Remove rendered items, keep state elements
         list.querySelectorAll('.app-notification-item').forEach(el => el.remove());
 
         if (notifications.length === 0) {
-            if (emptyEl) emptyEl.style.display = '';
+            showState('empty');
             return;
         }
-        if (emptyEl) emptyEl.style.display = 'none';
+        showState('success');
 
         const frag = document.createDocumentFragment();
         for (const n of notifications) {
@@ -161,14 +170,19 @@ export function initNotificationBell(root = document, config = {}) {
 
     // --- API ---
     async function loadNotifications() {
+        showState('loading');
+        list?.querySelectorAll('.app-notification-item').forEach(el => el.remove());
         try {
             const data = await fetchJson(endpoint('notifications/list-data'), { signal });
             if (data.ok) {
                 renderNotifications(data.notifications || []);
                 updateBadge(data.unread_count ?? 0);
+            } else {
+                showState('error');
             }
         } catch (err) {
             if (err instanceof DOMException && err.name === 'AbortError') return;
+            showState('error');
             telemetry.capture('warn_once', {
                 message: 'Notification bell: load failed',
                 meta: { module: 'notifications', component: 'notification-bell', action: 'load' },

tests/Service/Auth/AuthSessionTenantContextServiceTest.php

@@ -144,7 +144,7 @@ final class DummyAuthSessionProvider implements SessionProvider
         $_SESSION['dummy_provider_user_id'] = (int) ($user['id'] ?? 0);
     }
 
-    public function clear(): void
+    public function clear(AppContainer $container): void
     {
         self::$clearCalls++;
         unset($_SESSION['dummy_provider_user_id']);

tests/Unit/Module/SessionProviderTest.php

@@ -3,6 +3,8 @@
 namespace MintyPHP\Tests\Unit\Module;
 
 use MintyPHP\App\AppContainer;
+use MintyPHP\Http\SessionStore;
+use MintyPHP\Http\SessionStoreInterface;
 use MintyPHP\Module\AddressBook\Providers\AddressBookSessionProvider;
 use PHPUnit\Framework\TestCase;
 
@@ -21,13 +23,11 @@ final class SessionProviderTest extends TestCase
         $_SESSION = [];
     }
 
-    /**
-     * Create a real AppContainer that returns null for any get() call.
-     * AppContainer is final and cannot be mocked.
-     */
-    private function emptyContainer(): AppContainer
+    private function containerWithSessionStore(): AppContainer
     {
-        return new AppContainer();
+        $container = new AppContainer();
+        $container->set(SessionStoreInterface::class, static fn (): SessionStore => new SessionStore());
+        return $container;
     }
 
     public function testPopulateWithInvalidUserClearsSessionKey(): void
@@ -35,7 +35,7 @@ final class SessionProviderTest extends TestCase
         $_SESSION['module.addressbook.departments_by_tenant'] = [['tenant' => 'old']];
 
         $provider = new AddressBookSessionProvider();
-        $provider->populate(['id' => 0], $this->emptyContainer());
+        $provider->populate(['id' => 0], $this->containerWithSessionStore());
 
         self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION);
     }
@@ -45,22 +45,17 @@ final class SessionProviderTest extends TestCase
         $_SESSION['module.addressbook.departments_by_tenant'] = [['tenant' => 'old']];
 
         $provider = new AddressBookSessionProvider();
-        $provider->populate([], $this->emptyContainer());
+        $provider->populate([], $this->containerWithSessionStore());
 
         self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION);
     }
 
     public function testPopulateReturnsEarlyWhenContainerMissesDependencies(): void
     {
+        $container = $this->containerWithSessionStore();
         $provider = new AddressBookSessionProvider();
-        // Container has no bindings → get() will throw, but provider guards with instanceof check
-        // The provider should handle this gracefully (no session key set)
-        try {
-            $provider->populate(['id' => 42], $this->emptyContainer());
-        } catch (\Throwable) {
-            // Provider may throw if container has no binding — that's acceptable;
-            // in production, bindings are always registered before providers run.
-        }
+        // Container has SessionStore but no UserTenantContextService → provider returns early
+        $provider->populate(['id' => 42], $container);
 
         self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION);
     }
@@ -72,7 +67,7 @@ final class SessionProviderTest extends TestCase
         ];
 
         $provider = new AddressBookSessionProvider();
-        $provider->clear();
+        $provider->clear($this->containerWithSessionStore());
 
         self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION);
     }
@@ -81,7 +76,7 @@ final class SessionProviderTest extends TestCase
     {
         // Key doesn't exist — clear() should not throw
         $provider = new AddressBookSessionProvider();
-        $provider->clear();
+        $provider->clear($this->containerWithSessionStore());
 
         self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION);
     }