From ef72b34c4020766a0d87a3473854ed88be94c062 Mon Sep 17 00:00:00 2001 From: fs Date: Thu, 19 Mar 2026 18:20:13 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20module=20architecture=20improvements=20?= =?UTF-8?q?=E2=80=94=20session=20keys,=20dependency=20graph,=20event=20dis?= =?UTF-8?q?patcher,=20deactivation=20hooks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six targeted improvements to the modular monolith platform: 1. Fix AddressBook session key prefix to follow module..* convention 2. Move ADDRESS_BOOK_VIEW permission constant from core PermissionService into module 3. Add declarative JSON schema for module manifests (.agents/contracts/) 4. Add `requires` field with missing-dependency and circular-dependency detection 5. Add lightweight fire-and-forget event dispatcher (user.created/deleted/login/logout) 6. Add module deactivation hook interface and CLI script (bin/module-deactivate.php) Includes 15 new architecture/unit tests covering all new functionality. Co-Authored-By: Claude Opus 4.6 (1M context) --- .agents/README.md | 42 ++++ .agents/contracts/module-manifest.schema.json | 209 ++++++++++++++++++ bin/module-deactivate.php | 84 +++++++ .../Registrars/ServiceFactoryRegistrar.php | 4 +- lib/App/Module/Contracts/EventListener.php | 18 ++ .../Contracts/ModuleDeactivationHandler.php | 17 ++ lib/App/Module/ModuleEventDispatcher.php | 49 ++++ lib/App/Module/ModuleManifest.php | 94 ++++++++ lib/App/Module/ModuleRegistry.php | 64 ++++++ lib/App/registerContainer.php | 5 + lib/Service/Access/PermissionService.php | 1 - .../Access/UserAuthorizationPolicy.php | 4 +- lib/Service/Auth/AuthService.php | 20 +- lib/Service/Auth/AuthServicesFactory.php | 13 +- lib/Service/User/UserAccountService.php | 16 +- lib/Service/User/UserServicesFactory.php | 7 +- .../AddressBookAuthorizationPolicy.php | 3 +- .../Providers/AddressBookLayoutProvider.php | 4 +- .../Providers/AddressBookSessionProvider.php | 6 +- modules/addressbook/module.php | 1 + modules/bookmarks/module.php | 1 + .../Architecture/AgentSystemContractTest.php | 145 ++++++++---- .../ModuleRegistryContractTest.php | 51 +++++ .../ModuleStructureContractTest.php | 129 +++++++++++ .../NoAddressBookHardcodingTest.php | 3 +- .../Access/UserAuthorizationPolicyTest.php | 2 +- .../AuthSessionTenantContextServiceTest.php | 4 +- .../AddressBookAuthorizationPolicyTest.php | 2 +- .../Unit/Module/LayoutContextProviderTest.php | 2 +- .../Unit/Module/ModuleEventDispatcherTest.php | 100 +++++++++ tests/Unit/Module/SessionProviderTest.php | 16 +- 31 files changed, 1041 insertions(+), 75 deletions(-) create mode 100644 .agents/README.md create mode 100644 .agents/contracts/module-manifest.schema.json create mode 100644 bin/module-deactivate.php create mode 100644 lib/App/Module/Contracts/EventListener.php create mode 100644 lib/App/Module/Contracts/ModuleDeactivationHandler.php create mode 100644 lib/App/Module/ModuleEventDispatcher.php create mode 100644 tests/Unit/Module/ModuleEventDispatcherTest.php diff --git a/.agents/README.md b/.agents/README.md new file mode 100644 index 0000000..c181f57 --- /dev/null +++ b/.agents/README.md @@ -0,0 +1,42 @@ +# .agents/ + +Centralized home for the 7-role agent workflow system. + +## Structure + +``` +workflow.md ← Roles, state machine, handover rules (start here) +contracts/ ← JSON schemas for role outputs (7 roles) +templates/ ← Starter JSON payloads per role (7 templates) +prompts/ ← Role-specific prompt baselines (7 prompts) +checks/ ← Guard catalog (22 guards), quality gates (9), checklists +skills/ ← Reusable skill packages (guardrails, planner, grid, PHP CI) +runs/ ← Runtime artifacts per workflow run (gitignored) +``` + +## Roles + +| # | Role | Artifact | Guards | +|---|---|---|---| +| 1 | Analyst | `analysis.json` | — | +| 2 | Planner | `plan.json` | selects guards | +| 3 | Executor | `execution-report.json` | provides evidence | +| 4 | Code Reviewer | `review-code.json` | 13 guards (GR-CORE/TEST/UI) | +| 5 | Security Reviewer | `review-security.json` | 9 guards (GR-SEC) | +| 6 | Acceptance Tester | `review-acceptance.json` | SC-* criteria | +| 7 | Finalizer | `finalize.json` | all reviews + CI | + +## Quick Links + +| What | Where | +|---|---| +| Workflow & roles | `workflow.md` | +| Guard catalog (22 guards) | `checks/guard-catalog.json` | +| Quality gates (9 gates) | `checks/quality-gates.json` | +| Code review checklist | `checks/guard-checklist.md` | +| Acceptance checklist | `checks/acceptance-checklist.md` | +| Module manifest schema | `contracts/module-manifest.schema.json` | + +## Project Context + +See `CLAUDE.md` (repo root) for tech stack, architecture rules, and conventions. diff --git a/.agents/contracts/module-manifest.schema.json b/.agents/contracts/module-manifest.schema.json new file mode 100644 index 0000000..081e6a6 --- /dev/null +++ b/.agents/contracts/module-manifest.schema.json @@ -0,0 +1,209 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "module-manifest.schema.json", + "title": "Module Manifest Schema", + "description": "Declarative schema for module.php manifest files under modules//.", + "type": "object", + "required": ["id"], + "properties": { + "id": { + "type": "string", + "minLength": 1, + "pattern": "^[a-z][a-z0-9_-]*$", + "description": "Module identifier — must match the directory name under modules/." + }, + "version": { + "type": "string", + "pattern": "^\\d+\\.\\d+\\.\\d+$", + "description": "Semantic version (e.g. 1.0.0)." + }, + "enabled_by_default": { + "type": "boolean", + "default": false + }, + "load_order": { + "type": "integer", + "default": 100, + "description": "Lower values load first. Deterministic tie-break by id ASC." + }, + "requires": { + "type": "array", + "items": { + "type": "string", + "minLength": 1 + }, + "uniqueItems": true, + "default": [], + "description": "Module IDs that must be enabled for this module to function." + }, + "routes": { + "type": "array", + "items": { + "type": "object", + "required": ["path", "target"], + "properties": { + "path": { "type": "string", "minLength": 1 }, + "target": { "type": "string", "minLength": 1 }, + "public": { "type": "boolean", "default": false } + }, + "additionalProperties": false + }, + "default": [] + }, + "public_paths": { + "type": "array", + "items": { "type": "string" }, + "default": [] + }, + "container_registrars": { + "type": "array", + "items": { "type": "string", "minLength": 1, "description": "FQCN of a ContainerRegistrar." }, + "default": [] + }, + "ui_slots": { + "type": "object", + "description": "Keyed by slot name (e.g. 'aside.tab_panel'). Values are arrays of contribution objects.", + "patternProperties": { + "^.+$": { + "type": "array", + "items": { + "type": "object", + "required": ["key"], + "properties": { + "key": { "type": "string", "minLength": 1 }, + "label": { "type": "string" }, + "icon": { "type": "string" }, + "href": { "type": "string" }, + "permission": { "type": "string" }, + "panel_template": { "type": "string" }, + "template": { "type": "string" }, + "path": { "type": "string" }, + "script": { "type": "string" }, + "export": { "type": "string" }, + "selector": { "type": "string" }, + "scope": { "type": "string" }, + "config_path": { "type": "string" }, + "default_config": { "type": "object" }, + "phase": { "type": "string", "enum": ["early", "default", "late"] }, + "type": { "type": "string" }, + "order": { "type": "integer", "default": 100 }, + "details_storage": { "type": "string" }, + "details_open_active": { "type": "boolean" }, + "base_url": { "type": "string" } + } + } + } + }, + "default": {} + }, + "search_resources": { + "type": "array", + "items": { "type": "string", "minLength": 1, "description": "FQCN of a SearchResourceProvider." }, + "default": [] + }, + "asset_groups": { + "type": "object", + "description": "Asset group name → file list.", + "patternProperties": { + "^.+$": { + "type": "array", + "items": { "type": "string" } + } + }, + "default": {} + }, + "layout_context_providers": { + "type": "array", + "items": { "type": "string", "minLength": 1, "description": "FQCN of a LayoutContextProvider." }, + "default": [] + }, + "session_providers": { + "type": "array", + "items": { "type": "string", "minLength": 1, "description": "FQCN of a SessionProvider." }, + "default": [] + }, + "authorization_policies": { + "type": "array", + "items": { "type": "string", "minLength": 1, "description": "FQCN of an AuthorizationPolicyInterface." }, + "default": [] + }, + "permissions": { + "type": "array", + "items": { + "type": "object", + "required": ["key", "description"], + "properties": { + "key": { "type": "string", "minLength": 1 }, + "description": { "type": "string", "minLength": 1 }, + "active": { "type": "boolean", "default": true }, + "is_system": { "type": "boolean", "default": true } + }, + "additionalProperties": false + }, + "default": [] + }, + "scheduler_jobs": { + "type": "array", + "items": { + "type": "object", + "required": [ + "job_key", "handler", "label", "description", + "default_enabled", "default_timezone", "default_schedule_type", + "default_schedule_interval", "default_schedule_time", + "default_schedule_weekdays_csv", "default_catchup_once", + "allowed_schedule_types" + ], + "properties": { + "job_key": { "type": "string", "minLength": 1 }, + "handler": { "type": "string", "minLength": 1 }, + "label": { "type": "string", "minLength": 1 }, + "description": { "type": "string", "minLength": 1 }, + "default_enabled": { "type": ["boolean", "integer"] }, + "default_timezone": { "type": "string", "minLength": 1 }, + "default_schedule_type": { "type": "string", "enum": ["hourly", "daily", "weekly"] }, + "default_schedule_interval": { "type": "integer" }, + "default_schedule_time": { "type": ["string", "null"] }, + "default_schedule_weekdays_csv": { "type": ["string", "null"] }, + "default_catchup_once": { "type": ["boolean", "integer"] }, + "allowed_schedule_types": { + "type": "array", + "items": { "type": "string", "enum": ["hourly", "daily", "weekly"] }, + "minItems": 1 + } + }, + "additionalProperties": false + }, + "default": [] + }, + "event_listeners": { + "type": "object", + "description": "Event name → list of listener descriptors.", + "patternProperties": { + "^.+$": { + "type": "array", + "items": { + "type": "object", + "required": ["class", "method"], + "properties": { + "class": { "type": "string", "minLength": 1, "description": "FQCN implementing EventListener." }, + "method": { "type": "string", "minLength": 1 } + }, + "additionalProperties": false + } + } + }, + "default": {} + }, + "deactivation_handler": { + "type": ["string", "null"], + "description": "FQCN of a ModuleDeactivationHandler. Null if no cleanup needed.", + "default": null + }, + "migrations_path": { + "type": ["string", "null"], + "description": "Relative path to SQL migration scripts. Resolved against module basePath.", + "default": null + } + }, + "additionalProperties": false +} diff --git a/bin/module-deactivate.php b/bin/module-deactivate.php new file mode 100644 index 0000000..4e601cf --- /dev/null +++ b/bin/module-deactivate.php @@ -0,0 +1,84 @@ + --confirm + * php bin/module-deactivate.php --dry-run + * + * The module does NOT need to be in the enabled list — the manifest is loaded + * directly from modules//module.php. + */ + +require_once __DIR__ . '/module-cli-bootstrap.php'; + +$exitCode = moduleCliRunStep('module-deactivate', static function (): int { + global $argv; + + $moduleId = $argv[1] ?? ''; + $flags = array_slice($argv, 2); + + if ($moduleId === '' || $moduleId === '--help') { + fwrite(STDOUT, "Usage: php bin/module-deactivate.php --confirm|--dry-run\n"); + return $moduleId === '--help' ? 0 : 1; + } + + $confirm = in_array('--confirm', $flags, true); + $dryRun = in_array('--dry-run', $flags, true); + + if (!$confirm && !$dryRun) { + fwrite(STDERR, "Error: must pass --confirm to execute or --dry-run to validate.\n"); + return 1; + } + + $projectRoot = moduleCliProjectRoot(); + $manifestFile = $projectRoot . '/modules/' . $moduleId . '/module.php'; + + if (!is_file($manifestFile)) { + fwrite(STDERR, "Error: module manifest not found at {$manifestFile}\n"); + return 1; + } + + $raw = require $manifestFile; + if (!is_array($raw)) { + fwrite(STDERR, "Error: manifest must return an array.\n"); + return 1; + } + + $manifest = \MintyPHP\App\Module\ModuleManifest::fromArray($raw, dirname($manifestFile)); + + $handlerClass = $manifest->deactivationHandler; + if ($handlerClass === null) { + fwrite(STDOUT, "Module '{$moduleId}' has no deactivation_handler declared — nothing to do.\n"); + return 0; + } + + if (!class_exists($handlerClass)) { + fwrite(STDERR, "Error: deactivation handler class '{$handlerClass}' not found.\n"); + return 1; + } + + $handler = new $handlerClass(); + if (!$handler instanceof \MintyPHP\App\Module\Contracts\ModuleDeactivationHandler) { + fwrite(STDERR, "Error: '{$handlerClass}' does not implement ModuleDeactivationHandler.\n"); + return 1; + } + + if ($dryRun) { + fwrite(STDOUT, "Dry-run: handler '{$handlerClass}' found and valid for module '{$moduleId}'.\n"); + return 0; + } + + fwrite(STDOUT, "Running deactivation handler for module '{$moduleId}'...\n"); + + $container = app(); + $handler->deactivate($container); + + fwrite(STDOUT, "Deactivation handler for module '{$moduleId}' completed.\n"); + return 0; +}); + +exit($exitCode); diff --git a/lib/App/Container/Registrars/ServiceFactoryRegistrar.php b/lib/App/Container/Registrars/ServiceFactoryRegistrar.php index bcf6fe3..5e5e799 100644 --- a/lib/App/Container/Registrars/ServiceFactoryRegistrar.php +++ b/lib/App/Container/Registrars/ServiceFactoryRegistrar.php @@ -4,6 +4,7 @@ namespace MintyPHP\App\Container\Registrars; use MintyPHP\App\AppContainer; use MintyPHP\App\Container\ContainerRegistrar; +use MintyPHP\App\Module\ModuleEventDispatcher; use MintyPHP\App\Module\ModuleRegistry; use MintyPHP\Http\CookieStoreInterface; use MintyPHP\Http\RequestRuntimeInterface; @@ -130,7 +131,8 @@ final class ServiceFactoryRegistrar implements ContainerRegistrar $c->get(DatabaseSessionRepository::class), // Wrapped in a closure to break circular dependency: // UserServicesFactory -> AssignableRoleService -> RoleRepository -> DirectoryServicesFactory -> UserServicesFactory - static fn (): AssignableRoleService => $c->get(AssignableRoleService::class) + static fn (): AssignableRoleService => $c->get(AssignableRoleService::class), + $c->has(ModuleEventDispatcher::class) ? $c->get(ModuleEventDispatcher::class) : null )); $container->set(AuthGatewayFactory::class, static fn (AppContainer $c): AuthGatewayFactory => new AuthGatewayFactory( $c->get(AuthRepositoryFactory::class), diff --git a/lib/App/Module/Contracts/EventListener.php b/lib/App/Module/Contracts/EventListener.php new file mode 100644 index 0000000..09af2a7 --- /dev/null +++ b/lib/App/Module/Contracts/EventListener.php @@ -0,0 +1,18 @@ + $payload Event-specific data + */ + public function handle(string $event, array $payload): void; +} diff --git a/lib/App/Module/Contracts/ModuleDeactivationHandler.php b/lib/App/Module/Contracts/ModuleDeactivationHandler.php new file mode 100644 index 0000000..d6ae6e6 --- /dev/null +++ b/lib/App/Module/Contracts/ModuleDeactivationHandler.php @@ -0,0 +1,17 @@ +> $listenerMap + */ + public function __construct( + private readonly array $listenerMap, + private readonly AppContainer $container + ) { + } + + /** + * @param array $payload + */ + public function dispatch(string $event, array $payload): void + { + $listeners = $this->listenerMap[$event] ?? []; + + foreach ($listeners as $descriptor) { + try { + $class = $descriptor['class']; + $method = $descriptor['method']; + + $listener = $this->container->get($class); + if (!$listener instanceof EventListener) { + $listener = new $class(); + } + + $listener->{$method}($event, $payload); + } catch (\Throwable) { + // Swallow — one failing listener must not break the chain. + } + } + } +} diff --git a/lib/App/Module/ModuleManifest.php b/lib/App/Module/ModuleManifest.php index 25a420f..f4552aa 100644 --- a/lib/App/Module/ModuleManifest.php +++ b/lib/App/Module/ModuleManifest.php @@ -72,6 +72,15 @@ final class ModuleManifest /** @var list AuthorizationPolicyInterface FQCNs */ public readonly array $authorizationPolicies; + /** @var list Module IDs required by this module */ + public readonly array $requires; + + /** @var array> Event listeners keyed by event name */ + public readonly array $eventListeners; + + /** @var class-string|null ModuleDeactivationHandler FQCN */ + public readonly ?string $deactivationHandler; + public readonly ?string $migrationsPath; public readonly string $basePath; @@ -107,6 +116,14 @@ final class ModuleManifest $this->permissions = self::normalizePermissions($raw['permissions'] ?? [], $this->id); $this->authorizationPolicies = self::listOf($raw, 'authorization_policies'); + $this->requires = self::normalizeRequires($raw['requires'] ?? [], $this->id); + $this->eventListeners = self::normalizeEventListeners($raw['event_listeners'] ?? [], $this->id); + + $deactivationHandler = $raw['deactivation_handler'] ?? null; + $this->deactivationHandler = is_string($deactivationHandler) && trim($deactivationHandler) !== '' + ? trim($deactivationHandler) + : null; + $migrationsPath = $raw['migrations_path'] ?? null; $this->migrationsPath = is_string($migrationsPath) && trim($migrationsPath) !== '' ? rtrim($basePath, '/') . '/' . ltrim(trim($migrationsPath), '/') @@ -332,6 +349,83 @@ final class ModuleManifest return $types; } + /** + * @return array> + */ + private static function normalizeEventListeners(mixed $value, string $moduleId): array + { + if (!is_array($value)) { + throw new InvalidArgumentException( + sprintf("Module '%s' manifest key 'event_listeners' must be an array.", $moduleId) + ); + } + + $listeners = []; + foreach ($value as $eventName => $handlers) { + $eventName = trim((string) $eventName); + if ($eventName === '') { + throw new InvalidArgumentException( + sprintf("Module '%s' event_listeners has an empty event name key.", $moduleId) + ); + } + + if (!is_array($handlers)) { + throw new InvalidArgumentException( + sprintf("Module '%s' event_listeners['%s'] must be an array of handler descriptors.", $moduleId, $eventName) + ); + } + + $normalizedHandlers = []; + foreach (array_values($handlers) as $index => $handler) { + if (!is_array($handler)) { + throw new InvalidArgumentException( + sprintf("Module '%s' event_listeners['%s'][%d] must be an array with 'class' and 'method'.", $moduleId, $eventName, $index) + ); + } + + $class = trim((string) ($handler['class'] ?? '')); + $method = trim((string) ($handler['method'] ?? '')); + if ($class === '' || $method === '') { + throw new InvalidArgumentException( + sprintf("Module '%s' event_listeners['%s'][%d] must define non-empty 'class' and 'method'.", $moduleId, $eventName, $index) + ); + } + + $normalizedHandlers[] = ['class' => $class, 'method' => $method]; + } + + if ($normalizedHandlers !== []) { + $listeners[$eventName] = $normalizedHandlers; + } + } + + return $listeners; + } + + /** + * @return list + */ + private static function normalizeRequires(mixed $value, string $moduleId): array + { + if (!is_array($value)) { + throw new InvalidArgumentException( + sprintf("Module '%s' manifest key 'requires' must be an array.", $moduleId) + ); + } + + $requires = []; + foreach (array_values($value) as $index => $item) { + if (!is_string($item) || trim($item) === '') { + throw new InvalidArgumentException( + sprintf("Module '%s' requires[%d] must be a non-empty string.", $moduleId, $index) + ); + } + $requires[] = trim($item); + } + + return array_values(array_unique($requires)); + } + private static function normalizeWeekdaysCsv(mixed $value, string $moduleId, int $index): ?string { $raw = trim((string) $value); diff --git a/lib/App/Module/ModuleRegistry.php b/lib/App/Module/ModuleRegistry.php index 538a0c1..6759925 100644 --- a/lib/App/Module/ModuleRegistry.php +++ b/lib/App/Module/ModuleRegistry.php @@ -87,6 +87,9 @@ final class ModuleRegistry /** @var array scheduler job key => owning module id */ private array $schedulerJobOwners = []; + /** @var array> merged event listeners keyed by event name */ + private array $mergedEventListeners = []; + /** * Boot the registry from a modules directory and activation config. * @@ -150,6 +153,7 @@ final class ModuleRegistry $registry->registerModule($manifest); } + $registry->validateDependencies(); $registry->finalizeMergedContributions(); return $registry; @@ -354,6 +358,14 @@ final class ModuleRegistry $this->mergedAuthorizationPolicies[] = $policyClass; } + // — Merge event listeners —————————————————————————— + foreach ($manifest->eventListeners as $eventName => $handlers) { + if (!isset($this->mergedEventListeners[$eventName])) { + $this->mergedEventListeners[$eventName] = []; + } + array_push($this->mergedEventListeners[$eventName], ...$handlers); + } + // — Merge scheduler jobs ————————————————————————— foreach ($manifest->schedulerJobs as $schedulerJob) { $jobKey = trim((string) $schedulerJob['job_key']); @@ -388,6 +400,52 @@ final class ModuleRegistry } } + private function validateDependencies(): void + { + $enabledIds = array_keys($this->modules); + + // Check all required modules are enabled + foreach ($this->modules as $manifest) { + foreach ($manifest->requires as $requiredId) { + if (!isset($this->modules[$requiredId])) { + throw new RuntimeException( + "Module '{$manifest->id}' requires module '{$requiredId}' which is not enabled." + ); + } + } + } + + // Check for circular dependencies (DFS cycle detection) + $visited = []; + $inStack = []; + + $visit = function (string $moduleId) use (&$visit, &$visited, &$inStack): void { + if (isset($inStack[$moduleId])) { + throw new RuntimeException( + "Circular module dependency detected involving module '{$moduleId}'." + ); + } + if (isset($visited[$moduleId])) { + return; + } + + $inStack[$moduleId] = true; + + if (isset($this->modules[$moduleId])) { + foreach ($this->modules[$moduleId]->requires as $requiredId) { + $visit($requiredId); + } + } + + unset($inStack[$moduleId]); + $visited[$moduleId] = true; + }; + + foreach ($enabledIds as $moduleId) { + $visit($moduleId); + } + } + private function finalizeMergedContributions(): void { $this->mergedPublicPaths = array_values(array_unique(array_filter( @@ -626,4 +684,10 @@ final class ModuleRegistry { return $this->mergedSchedulerJobs; } + + /** @return array> */ + public function getEventListeners(): array + { + return $this->mergedEventListeners; + } } diff --git a/lib/App/registerContainer.php b/lib/App/registerContainer.php index 86ba7ca..2242f94 100644 --- a/lib/App/registerContainer.php +++ b/lib/App/registerContainer.php @@ -10,6 +10,7 @@ use MintyPHP\App\Container\Registrars\RepositoryFactoryRegistrar; use MintyPHP\App\Container\Registrars\ServiceFactoryRegistrar; use MintyPHP\App\Container\Registrars\SettingsRegistrar; use MintyPHP\App\Container\Registrars\UserRegistrar; +use MintyPHP\App\Module\ModuleEventDispatcher; use MintyPHP\App\Module\ModuleRegistry; $container = new AppContainer(); @@ -43,6 +44,10 @@ $modulesDir = dirname(__DIR__, 2) . '/modules'; $moduleRegistry = ModuleRegistry::boot($modulesDir, $enabledModules); $container->set(ModuleRegistry::class, static fn (): ModuleRegistry => $moduleRegistry); +$container->set(ModuleEventDispatcher::class, static fn () => new ModuleEventDispatcher( + $moduleRegistry->getEventListeners(), + $container +)); // Disallow overriding existing core bindings from module registrars. $container->protectExistingBindings(); diff --git a/lib/Service/Access/PermissionService.php b/lib/Service/Access/PermissionService.php index 94b9a6e..09c7ebb 100644 --- a/lib/Service/Access/PermissionService.php +++ b/lib/Service/Access/PermissionService.php @@ -42,7 +42,6 @@ class PermissionService public const USERS_ACCESS_PDF = 'users.access_pdf'; public const USERS_SELF_UPDATE = 'users.self_update'; public const USERS_UPDATE_ASSIGNMENTS = 'users.update_assignments'; - public const ADDRESS_BOOK_VIEW = 'address_book.view'; public const TENANTS_VIEW = 'tenants.view'; public const TENANTS_CREATE = 'tenants.create'; public const TENANTS_UPDATE = 'tenants.update'; diff --git a/lib/Service/Access/UserAuthorizationPolicy.php b/lib/Service/Access/UserAuthorizationPolicy.php index 450952d..71529ef 100644 --- a/lib/Service/Access/UserAuthorizationPolicy.php +++ b/lib/Service/Access/UserAuthorizationPolicy.php @@ -184,7 +184,7 @@ class UserAuthorizationPolicy implements AuthorizationPolicyInterface $canEditUser = $this->canEditUserForTarget($actorUserId, $targetUserId); if (!$this->hasPermission($actorUserId, PermissionService::USERS_VIEW) - && !$this->hasPermission($actorUserId, PermissionService::ADDRESS_BOOK_VIEW) + && !$this->hasPermission($actorUserId, 'address_book.view') // Registered by addressbook module when active && !$canEditUser) { return AuthorizationDecision::deny(403, 'forbidden'); } @@ -409,7 +409,7 @@ class UserAuthorizationPolicy implements AuthorizationPolicyInterface || ($isOwnAccount && $this->hasPermission($actorUserId, PermissionService::USERS_SELF_UPDATE)) ), 'can_manage_api_tokens' => $canEditUser && $this->hasPermission($actorUserId, PermissionService::API_TOKENS_MANAGE), - 'can_view_address_book' => $this->hasPermission($actorUserId, PermissionService::ADDRESS_BOOK_VIEW), + 'can_view_address_book' => $this->hasPermission($actorUserId, 'address_book.view'), // Registered by addressbook module when active 'can_view_user_meta' => $this->hasPermission($actorUserId, PermissionService::USERS_VIEW_META), 'can_view_user_audit' => $canViewUserAudit, 'can_access_pdf' => $this->hasPermission($actorUserId, PermissionService::USERS_ACCESS_PDF), diff --git a/lib/Service/Auth/AuthService.php b/lib/Service/Auth/AuthService.php index a14c0ee..e43afd8 100644 --- a/lib/Service/Auth/AuthService.php +++ b/lib/Service/Auth/AuthService.php @@ -2,6 +2,7 @@ namespace MintyPHP\Service\Auth; +use MintyPHP\App\Module\ModuleEventDispatcher; use MintyPHP\Auth; use MintyPHP\Http\SessionStoreInterface; use MintyPHP\Repository\User\UserReadRepositoryInterface; @@ -42,7 +43,8 @@ class AuthService private readonly TenantSsoService $tenantSsoService, private readonly AuthSessionTenantContextService $authSessionTenantContextService, private readonly SystemAuditService $systemAuditService, - private readonly SessionStoreInterface $sessionStore + private readonly SessionStoreInterface $sessionStore, + private readonly ?ModuleEventDispatcher $eventDispatcher = null ) { } @@ -130,6 +132,11 @@ class AuthService 'actor_tenant_id' => $this->currentTenantIdFromSession(), ]); + $this->eventDispatcher?->dispatch('user.login', [ + 'user_id' => $userId, + 'provider' => 'local', + ]); + return ['ok' => true]; } @@ -218,6 +225,12 @@ class AuthService 'actor_tenant_id' => $this->currentTenantIdFromSession(), 'metadata' => ['provider' => $loginProvider], ]); + + $this->eventDispatcher?->dispatch('user.login', [ + 'user_id' => $userId, + 'provider' => $loginProvider, + ]); + return ['ok' => true, 'user' => $user]; } @@ -276,6 +289,11 @@ class AuthService { $actorUserId = (int) ($this->sessionUser()['id'] ?? 0); $actorTenantId = $this->currentTenantIdFromSession(); + + $this->eventDispatcher?->dispatch('user.logout', [ + 'user_id' => $actorUserId, + ]); + $this->rememberMeService->forgetCurrentUser(); $this->logoutWithModuleCleanup(); $this->recordAuthEvent('auth.logout', 'success', [ diff --git a/lib/Service/Auth/AuthServicesFactory.php b/lib/Service/Auth/AuthServicesFactory.php index a6ac440..c80421f 100644 --- a/lib/Service/Auth/AuthServicesFactory.php +++ b/lib/Service/Auth/AuthServicesFactory.php @@ -3,6 +3,7 @@ namespace MintyPHP\Service\Auth; use MintyPHP\App\AppContainer; +use MintyPHP\App\Module\ModuleEventDispatcher; use MintyPHP\Http\CookieStoreInterface; use MintyPHP\Http\RequestRuntimeInterface; use MintyPHP\Http\SessionStoreInterface; @@ -100,7 +101,8 @@ class AuthServicesFactory $this->createTenantSsoService(), $this->createAuthSessionTenantContextService(), $this->auditServicesFactory->createSystemAuditService(), - $this->sessionStore + $this->sessionStore, + $this->resolveEventDispatcher() ); } @@ -148,6 +150,15 @@ class AuthServicesFactory return new OidcHttpGateway(); } + private function resolveEventDispatcher(): ?ModuleEventDispatcher + { + if ($this->appContainer->has(ModuleEventDispatcher::class)) { + $dispatcher = $this->appContainer->get(ModuleEventDispatcher::class); + return $dispatcher instanceof ModuleEventDispatcher ? $dispatcher : null; + } + return null; + } + private function createAuthSessionTenantContextService(): AuthSessionTenantContextService { return $this->authSessionTenantContextService ??= new AuthSessionTenantContextService( diff --git a/lib/Service/User/UserAccountService.php b/lib/Service/User/UserAccountService.php index 762d3c2..77eef01 100644 --- a/lib/Service/User/UserAccountService.php +++ b/lib/Service/User/UserAccountService.php @@ -2,6 +2,7 @@ namespace MintyPHP\Service\User; +use MintyPHP\App\Module\ModuleEventDispatcher; use MintyPHP\I18n; use MintyPHP\Repository\Support\DatabaseSessionRepository; use MintyPHP\Repository\User\UserListQueryRepositoryInterface; @@ -36,7 +37,8 @@ class UserAccountService private readonly TenantScopeService $scopeGateway, private readonly UserDirectoryGateway $directoryGateway, private readonly SystemAuditService $systemAuditService, - private readonly DatabaseSessionRepository $databaseSessionRepository + private readonly DatabaseSessionRepository $databaseSessionRepository, + private readonly ?ModuleEventDispatcher $eventDispatcher = null ) { } @@ -112,6 +114,12 @@ class UserAccountService 'target_uuid' => (string) ($user['uuid'] ?? ''), ]); + $this->eventDispatcher?->dispatch('user.deleted', [ + 'user_id' => $userId, + 'uuid' => (string) ($user['uuid'] ?? ''), + 'actor_user_id' => $currentUserId, + ]); + return ['ok' => true, 'user' => $user]; } @@ -291,6 +299,12 @@ class UserAccountService ], ]); + $this->eventDispatcher?->dispatch('user.created', [ + 'user_id' => $userId, + 'uuid' => is_string($uuid) ? $uuid : '', + 'actor_user_id' => $currentUserId, + ]); + return ['ok' => true, 'form' => $form, 'uuid' => $uuid]; } diff --git a/lib/Service/User/UserServicesFactory.php b/lib/Service/User/UserServicesFactory.php index ea91f30..8711255 100644 --- a/lib/Service/User/UserServicesFactory.php +++ b/lib/Service/User/UserServicesFactory.php @@ -2,6 +2,7 @@ namespace MintyPHP\Service\User; +use MintyPHP\App\Module\ModuleEventDispatcher; use MintyPHP\Repository\Access\UserRoleRepositoryInterface; use MintyPHP\Repository\Org\UserDepartmentRepositoryInterface; use MintyPHP\Repository\Support\DatabaseSessionRepository; @@ -33,7 +34,8 @@ class UserServicesFactory private readonly UserRepositoryFactory $userRepositoryFactory, private readonly UserGatewayFactory $userGatewayFactory, private readonly DatabaseSessionRepository $databaseSessionRepository, - private readonly \Closure $assignableRoleServiceFactory + private readonly \Closure $assignableRoleServiceFactory, + private readonly ?ModuleEventDispatcher $eventDispatcher = null ) { } @@ -49,7 +51,8 @@ class UserServicesFactory $this->userGatewayFactory->getTenantScopeService(), $this->createUserDirectoryGateway(), $this->auditServicesFactory->createSystemAuditService(), - $this->databaseSessionRepository + $this->databaseSessionRepository, + $this->eventDispatcher ); } diff --git a/modules/addressbook/lib/Module/AddressBook/AddressBookAuthorizationPolicy.php b/modules/addressbook/lib/Module/AddressBook/AddressBookAuthorizationPolicy.php index f614867..b249cff 100644 --- a/modules/addressbook/lib/Module/AddressBook/AddressBookAuthorizationPolicy.php +++ b/modules/addressbook/lib/Module/AddressBook/AddressBookAuthorizationPolicy.php @@ -15,6 +15,7 @@ use MintyPHP\Service\Access\PermissionService; final class AddressBookAuthorizationPolicy implements AuthorizationPolicyInterface { public const ABILITY_VIEW = 'addressbook.view'; + public const PERMISSION_KEY = 'address_book.view'; public function __construct( private readonly PermissionService $permissionService @@ -33,7 +34,7 @@ final class AddressBookAuthorizationPolicy implements AuthorizationPolicyInterfa return AuthorizationDecision::deny(403, 'forbidden'); } - if (!$this->permissionService->userHas($actorUserId, PermissionService::ADDRESS_BOOK_VIEW)) { + if (!$this->permissionService->userHas($actorUserId, self::PERMISSION_KEY)) { return AuthorizationDecision::deny(403, 'forbidden'); } diff --git a/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookLayoutProvider.php b/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookLayoutProvider.php index 07b0aed..7381a25 100644 --- a/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookLayoutProvider.php +++ b/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookLayoutProvider.php @@ -31,8 +31,8 @@ final class AddressBookLayoutProvider implements LayoutContextProvider 'activeDepartments' => appNormalizePositiveIntList($query['departments'] ?? ''), 'activeRoles' => appNormalizePositiveIntList($query['roles'] ?? ''), 'activeCustomFilters' => self::normalizeCustomFilterQuery($query), - 'peopleGroups' => is_array($session['available_departments_by_tenant'] ?? null) - ? $session['available_departments_by_tenant'] + 'peopleGroups' => is_array($session['module.addressbook.departments_by_tenant'] ?? null) + ? $session['module.addressbook.departments_by_tenant'] : [], ], ]; diff --git a/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookSessionProvider.php b/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookSessionProvider.php index a158c54..0a8e525 100644 --- a/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookSessionProvider.php +++ b/modules/addressbook/lib/Module/AddressBook/Providers/AddressBookSessionProvider.php @@ -17,7 +17,7 @@ final class AddressBookSessionProvider implements SessionProvider { $userId = (int) ($user['id'] ?? 0); if ($userId <= 0) { - unset($_SESSION['available_departments_by_tenant']); + unset($_SESSION['module.addressbook.departments_by_tenant']); return; } @@ -28,13 +28,13 @@ final class AddressBookSessionProvider implements SessionProvider } $sessionStore->set( - 'available_departments_by_tenant', + 'module.addressbook.departments_by_tenant', $tenantContext->getAvailableDepartmentsByTenant($userId) ); } public function clear(): void { - unset($_SESSION['available_departments_by_tenant']); + unset($_SESSION['module.addressbook.departments_by_tenant']); } } diff --git a/modules/addressbook/module.php b/modules/addressbook/module.php index 67aaa6f..46fc9a7 100644 --- a/modules/addressbook/module.php +++ b/modules/addressbook/module.php @@ -11,6 +11,7 @@ return [ 'version' => '1.0.0', 'enabled_by_default' => false, 'load_order' => 10, + 'requires' => [], 'routes' => [ // Keep canonical route stable without forcing a /index target to avoid Router redirect loops. diff --git a/modules/bookmarks/module.php b/modules/bookmarks/module.php index 0815dbb..dda6bcc 100644 --- a/modules/bookmarks/module.php +++ b/modules/bookmarks/module.php @@ -11,6 +11,7 @@ return [ 'version' => '1.0.0', 'enabled_by_default' => true, 'load_order' => 20, + 'requires' => [], 'routes' => [ ['path' => 'bookmarks/save-data', 'target' => 'bookmarks/save-data'], diff --git a/tests/Architecture/AgentSystemContractTest.php b/tests/Architecture/AgentSystemContractTest.php index 66aa3e4..a180200 100644 --- a/tests/Architecture/AgentSystemContractTest.php +++ b/tests/Architecture/AgentSystemContractTest.php @@ -11,18 +11,23 @@ class AgentSystemContractTest extends TestCase public function testAgentSystemJsonFilesAreValidJson(): void { $jsonFiles = [ - 'agent-system/checks/guard-catalog.json', - 'agent-system/checks/quality-gates.json', - 'agent-system/contracts/planner.schema.json', - 'agent-system/contracts/executor.schema.json', - 'agent-system/contracts/reviewer-guards.schema.json', - 'agent-system/contracts/reviewer-acceptance.schema.json', - 'agent-system/contracts/finalizer.schema.json', - 'agent-system/templates/plan.template.json', - 'agent-system/templates/execution-report.template.json', - 'agent-system/templates/review-guards.template.json', - 'agent-system/templates/review-acceptance.template.json', - 'agent-system/templates/finalize.template.json', + '.agents/checks/guard-catalog.json', + '.agents/checks/quality-gates.json', + '.agents/contracts/analyst.schema.json', + '.agents/contracts/planner.schema.json', + '.agents/contracts/executor.schema.json', + '.agents/contracts/reviewer-code.schema.json', + '.agents/contracts/reviewer-security.schema.json', + '.agents/contracts/reviewer-acceptance.schema.json', + '.agents/contracts/finalizer.schema.json', + '.agents/templates/analysis.template.json', + '.agents/templates/plan.template.json', + '.agents/templates/execution-report.template.json', + '.agents/templates/review-code.template.json', + '.agents/templates/review-security.template.json', + '.agents/templates/review-acceptance.template.json', + '.agents/templates/finalize.template.json', + '.agents/contracts/module-manifest.schema.json', ]; foreach ($jsonFiles as $file) { @@ -33,27 +38,42 @@ class AgentSystemContractTest extends TestCase public function testGuardCatalogIdsAreUniqueAndWellFormed(): void { - $catalog = $this->decodeJson('agent-system/checks/guard-catalog.json'); + $catalog = $this->decodeJson('.agents/checks/guard-catalog.json'); $guards = is_array($catalog['guards'] ?? null) ? $catalog['guards'] : []; $this->assertNotSame([], $guards, 'Guard catalog must not be empty.'); $ids = []; foreach ($guards as $guard) { $id = (string) ($guard['id'] ?? ''); - $source = (string) ($guard['source'] ?? ''); - $this->assertMatchesRegularExpression('/^GR-[A-Z]+-[0-9]{3}$/', $id, 'Invalid guard id: ' . $id); - $this->assertNotSame('', $source, 'Missing guard source for ' . $id); - $sourcePath = explode('#', $source)[0]; - $this->readProjectFile($sourcePath); + $reviewer = (string) ($guard['reviewer'] ?? ''); + $this->assertMatchesRegularExpression('/^GR-(CORE|TEST|UI|SEC)-[A-Z0-9]{3,}$/', $id, 'Invalid guard id: ' . $id); + $this->assertContains($reviewer, ['code', 'security'], 'Invalid reviewer for ' . $id); $ids[] = $id; } $this->assertSame($ids, array_values(array_unique($ids)), 'Duplicate guard IDs found.'); } + public function testGuardCatalogReviewerAssignment(): void + { + $catalog = $this->decodeJson('.agents/checks/guard-catalog.json'); + $guards = is_array($catalog['guards'] ?? null) ? $catalog['guards'] : []; + + foreach ($guards as $guard) { + $id = (string) ($guard['id'] ?? ''); + $reviewer = (string) ($guard['reviewer'] ?? ''); + + if (str_starts_with($id, 'GR-SEC-')) { + $this->assertSame('security', $reviewer, $id . ' must be assigned to security reviewer'); + } else { + $this->assertSame('code', $reviewer, $id . ' must be assigned to code reviewer'); + } + } + } + public function testQualityGateIdsAreUniqueAndWellFormed(): void { - $gates = $this->decodeJson('agent-system/checks/quality-gates.json'); + $gates = $this->decodeJson('.agents/checks/quality-gates.json'); $items = is_array($gates['gates'] ?? null) ? $gates['gates'] : []; $this->assertNotSame([], $items, 'Quality gates must not be empty.'); @@ -71,8 +91,8 @@ class AgentSystemContractTest extends TestCase public function testTemplatesReferenceKnownGuardAndGateIds(): void { - $catalog = $this->decodeJson('agent-system/checks/guard-catalog.json'); - $gates = $this->decodeJson('agent-system/checks/quality-gates.json'); + $catalog = $this->decodeJson('.agents/checks/guard-catalog.json'); + $gates = $this->decodeJson('.agents/checks/quality-gates.json'); $guardIds = array_map( static fn (array $guard): string => (string) ($guard['id'] ?? ''), is_array($catalog['guards'] ?? null) ? $catalog['guards'] : [] @@ -82,7 +102,7 @@ class AgentSystemContractTest extends TestCase is_array($gates['gates'] ?? null) ? $gates['gates'] : [] ); - $planTemplate = $this->decodeJson('agent-system/templates/plan.template.json'); + $planTemplate = $this->decodeJson('.agents/templates/plan.template.json'); $planGuardIds = is_array($planTemplate['guardrails']['required_guard_ids'] ?? null) ? $planTemplate['guardrails']['required_guard_ids'] : []; @@ -96,39 +116,74 @@ class AgentSystemContractTest extends TestCase $this->assertContains($id, $gateIds, 'Unknown gate id in plan template: ' . (string) $id); } - $reviewGuardsTemplate = $this->decodeJson('agent-system/templates/review-guards.template.json'); - $reviewGuardIds = is_array($reviewGuardsTemplate['checked_guard_ids'] ?? null) - ? $reviewGuardsTemplate['checked_guard_ids'] + $reviewCodeTemplate = $this->decodeJson('.agents/templates/review-code.template.json'); + $reviewGuardIds = is_array($reviewCodeTemplate['checked_guard_ids'] ?? null) + ? $reviewCodeTemplate['checked_guard_ids'] : []; - $reviewGateIds = is_array($reviewGuardsTemplate['checked_quality_gate_ids'] ?? null) - ? $reviewGuardsTemplate['checked_quality_gate_ids'] + $reviewGateIds = is_array($reviewCodeTemplate['checked_quality_gate_ids'] ?? null) + ? $reviewCodeTemplate['checked_quality_gate_ids'] : []; foreach ($reviewGuardIds as $id) { - $this->assertContains($id, $guardIds, 'Unknown guard id in review template: ' . (string) $id); + $this->assertContains($id, $guardIds, 'Unknown guard id in review-code template: ' . (string) $id); } foreach ($reviewGateIds as $id) { - $this->assertContains($id, $gateIds, 'Unknown gate id in review template: ' . (string) $id); + $this->assertContains($id, $gateIds, 'Unknown gate id in review-code template: ' . (string) $id); + } + + $reviewSecurityTemplate = $this->decodeJson('.agents/templates/review-security.template.json'); + $secGuardIds = is_array($reviewSecurityTemplate['checked_guard_ids'] ?? null) + ? $reviewSecurityTemplate['checked_guard_ids'] + : []; + foreach ($secGuardIds as $id) { + $this->assertContains($id, $guardIds, 'Unknown guard id in review-security template: ' . (string) $id); } } - public function testRolePromptsReferenceGuardCatalogAndQualityGates(): void + public function testRolePromptsReferenceGuardCatalog(): void { - $plannerPrompt = $this->readProjectFile('agent-system/prompts/planner.md'); - $executorPrompt = $this->readProjectFile('agent-system/prompts/executor.md'); - $reviewerPrompt = $this->readProjectFile('agent-system/prompts/reviewer-guards.md'); + $plannerPrompt = $this->readProjectFile('.agents/prompts/planner.md'); + $executorPrompt = $this->readProjectFile('.agents/prompts/executor.md'); + $codeReviewerPrompt = $this->readProjectFile('.agents/prompts/reviewer-code.md'); + $securityReviewerPrompt = $this->readProjectFile('.agents/prompts/reviewer-security.md'); - $this->assertStringContainsString('agent-system/checks/guard-catalog.json', $plannerPrompt); - $this->assertStringContainsString('agent-system/checks/quality-gates.json', $plannerPrompt); - $this->assertStringContainsString('DOCS-SKILLS-AUDIT-xxx', $plannerPrompt); - $this->assertStringContainsString('QG-008', $plannerPrompt); - $this->assertStringContainsString('QG-009', $plannerPrompt); - $this->assertStringContainsString('agent-system/checks/guard-catalog.json', $executorPrompt); - $this->assertStringContainsString('agent-system/checks/quality-gates.json', $executorPrompt); - $this->assertStringContainsString('QG-008', $executorPrompt); - $this->assertStringContainsString('QG-009', $executorPrompt); - $this->assertStringContainsString('agent-system/checks/guard-catalog.json', $reviewerPrompt); - $this->assertStringContainsString('agent-system/checks/quality-gates.json', $reviewerPrompt); - $this->assertStringContainsString('agent-system/checks/docs-skills-audit-policy.md', $reviewerPrompt); + $this->assertStringContainsString('.agents/checks/guard-catalog.json', $plannerPrompt); + $this->assertStringContainsString('.agents/checks/quality-gates.json', $plannerPrompt); + $this->assertStringContainsString('.agents/checks/guard-catalog.json', $executorPrompt); + $this->assertStringContainsString('.agents/checks/quality-gates.json', $executorPrompt); + $this->assertStringContainsString('.agents/checks/guard-catalog.json', $codeReviewerPrompt); + $this->assertStringContainsString('.agents/checks/guard-catalog.json', $securityReviewerPrompt); + } + + public function testAllSevenRolePromptsExist(): void + { + $roles = ['analyst', 'planner', 'executor', 'reviewer-code', 'reviewer-security', 'reviewer-acceptance', 'finalizer']; + + foreach ($roles as $role) { + $this->readProjectFile('.agents/prompts/' . $role . '.md'); + } + } + + public function testAllSevenContractsExist(): void + { + $contracts = ['analyst', 'planner', 'executor', 'reviewer-code', 'reviewer-security', 'reviewer-acceptance', 'finalizer']; + + foreach ($contracts as $contract) { + $decoded = json_decode($this->readProjectFile('.agents/contracts/' . $contract . '.schema.json'), true); + $this->assertIsArray($decoded, 'Invalid JSON schema: ' . $contract); + } + } + + public function testAllSevenTemplatesExist(): void + { + $templates = [ + 'analysis', 'plan', 'execution-report', + 'review-code', 'review-security', 'review-acceptance', 'finalize', + ]; + + foreach ($templates as $template) { + $decoded = json_decode($this->readProjectFile('.agents/templates/' . $template . '.template.json'), true); + $this->assertIsArray($decoded, 'Invalid JSON template: ' . $template); + } } /** diff --git a/tests/Architecture/ModuleRegistryContractTest.php b/tests/Architecture/ModuleRegistryContractTest.php index 4164283..d7cac45 100644 --- a/tests/Architecture/ModuleRegistryContractTest.php +++ b/tests/Architecture/ModuleRegistryContractTest.php @@ -143,6 +143,57 @@ final class ModuleRegistryContractTest extends TestCase ); } + public function testMissingDependencyThrowsException(): void + { + $fixturesDir = sys_get_temp_dir() . '/module-dep-test-' . uniqid(); + mkdir($fixturesDir . '/mod-dep', 0777, true); + + file_put_contents($fixturesDir . '/mod-dep/module.php', ' 'mod-dep', + 'requires' => ['nonexistent-module'], + ], true) . ';'); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage("requires module 'nonexistent-module' which is not enabled"); + + try { + ModuleRegistry::boot($fixturesDir, ['mod-dep']); + } finally { + @unlink($fixturesDir . '/mod-dep/module.php'); + @rmdir($fixturesDir . '/mod-dep'); + @rmdir($fixturesDir); + } + } + + public function testCircularDependencyThrowsException(): void + { + $fixturesDir = sys_get_temp_dir() . '/module-circular-test-' . uniqid(); + mkdir($fixturesDir . '/mod-x', 0777, true); + mkdir($fixturesDir . '/mod-y', 0777, true); + + file_put_contents($fixturesDir . '/mod-x/module.php', ' 'mod-x', + 'requires' => ['mod-y'], + ], true) . ';'); + file_put_contents($fixturesDir . '/mod-y/module.php', ' 'mod-y', + 'requires' => ['mod-x'], + ], true) . ';'); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Circular module dependency'); + + try { + ModuleRegistry::boot($fixturesDir, ['mod-x', 'mod-y']); + } finally { + @unlink($fixturesDir . '/mod-x/module.php'); + @unlink($fixturesDir . '/mod-y/module.php'); + @rmdir($fixturesDir . '/mod-x'); + @rmdir($fixturesDir . '/mod-y'); + @rmdir($fixturesDir); + } + } + public function testConflictDetectionIsStrict(): void { $fixturesDir = sys_get_temp_dir() . '/module-arch-test-' . uniqid(); diff --git a/tests/Architecture/ModuleStructureContractTest.php b/tests/Architecture/ModuleStructureContractTest.php index 51b9778..cf74a32 100644 --- a/tests/Architecture/ModuleStructureContractTest.php +++ b/tests/Architecture/ModuleStructureContractTest.php @@ -2,7 +2,9 @@ namespace MintyPHP\Tests\Architecture; +use MintyPHP\App\Module\Contracts\EventListener; use MintyPHP\App\Module\Contracts\LayoutContextProvider; +use MintyPHP\App\Module\Contracts\ModuleDeactivationHandler; use MintyPHP\App\Module\Contracts\SearchResourceProvider; use MintyPHP\App\Module\Contracts\SessionProvider; use MintyPHP\App\Module\ModuleManifest; @@ -128,6 +130,26 @@ final class ModuleStructureContractTest extends TestCase } } + public function testRequiresFieldContainsOnlyValidModuleIds(): void + { + foreach (self::$modules as $module) { + foreach ($module['manifest']->requires as $index => $requiredId) { + self::assertIsString($requiredId); + self::assertNotEmpty( + trim($requiredId), + "Module '{$module['id']}' requires[{$index}] must be a non-empty string" + ); + self::assertMatchesRegularExpression( + '/^[a-z][a-z0-9_-]*$/', + $requiredId, + "Module '{$module['id']}' requires[{$index}] has invalid module id format: '{$requiredId}'" + ); + } + } + + self::addToAssertionCount(1); + } + // ─── File structure ────────────────────────────────────────────── public function testModuleHasLibDirectory(): void @@ -542,6 +564,113 @@ final class ModuleStructureContractTest extends TestCase } } + // ─── Event listener contract ──────────────────────────────────────── + + public function testEventListenerClassesImplementContract(): void + { + foreach (self::$modules as $module) { + foreach ($module['manifest']->eventListeners as $eventName => $handlers) { + foreach ($handlers as $index => $handler) { + $class = $handler['class']; + self::assertTrue( + class_exists($class), + "Module '{$module['id']}' event_listeners['{$eventName}'][{$index}] class '{$class}' not found" + ); + self::assertTrue( + is_subclass_of($class, EventListener::class) + || in_array(EventListener::class, class_implements($class) ?: [], true), + "Module '{$module['id']}' event_listeners['{$eventName}'][{$index}] class '{$class}' " + . "must implement EventListener interface" + ); + } + } + } + + self::addToAssertionCount(1); + } + + // ─── Deactivation handler contract ────────────────────────────── + + public function testDeactivationHandlerClassImplementsContract(): void + { + foreach (self::$modules as $module) { + $handler = $module['manifest']->deactivationHandler; + if ($handler === null) { + continue; + } + self::assertTrue( + class_exists($handler), + "Module '{$module['id']}' deactivation_handler class '{$handler}' not found" + ); + self::assertTrue( + is_subclass_of($handler, ModuleDeactivationHandler::class) + || in_array(ModuleDeactivationHandler::class, class_implements($handler) ?: [], true), + "Module '{$module['id']}' deactivation_handler class '{$handler}' " + . "must implement ModuleDeactivationHandler interface" + ); + } + + self::addToAssertionCount(1); + } + + // ─── Session key convention ──────────────────────────────────────── + + /** + * Module session providers must use the 'module..' prefix for all session keys. + * + * Scans session provider classes for $_SESSION[ and ->set( patterns and verifies + * that the key argument starts with 'module..'. + */ + public function testSessionProvidersUseModulePrefixedKeys(): void + { + foreach (self::$modules as $module) { + foreach ($module['manifest']->sessionProviders as $providerClass) { + if (!class_exists($providerClass)) { + continue; + } + + $reflection = new \ReflectionClass($providerClass); + $file = $reflection->getFileName(); + if ($file === false) { + continue; + } + + $content = file_get_contents($file); + if ($content === false) { + continue; + } + + $expectedPrefix = 'module.' . $module['id'] . '.'; + + // Check $_SESSION['...'] writes and unset($_SESSION['...']) + if (preg_match_all('/\$_SESSION\[\'([^\']+)\'\]/', $content, $matches)) { + foreach ($matches[1] as $key) { + self::assertStringStartsWith( + $expectedPrefix, + $key, + "Module '{$module['id']}' session provider '{$providerClass}' uses session key " + . "'{$key}' which does not follow the '{$expectedPrefix}*' convention" + ); + } + } + + // Check ->set('...', ...) calls on session store + if (preg_match_all('/->set\(\s*\'([^\']+)\'/', $content, $matches)) { + foreach ($matches[1] as $key) { + self::assertStringStartsWith( + $expectedPrefix, + $key, + "Module '{$module['id']}' session provider '{$providerClass}' uses session store key " + . "'{$key}' which does not follow the '{$expectedPrefix}*' convention" + ); + } + } + } + } + + self::addToAssertionCount(1); + } + // ─── No hardcoded Core ability imports in module pages ─────────── public function testModulePagesDoNotImportCoreAbilityConstants(): void diff --git a/tests/Architecture/NoAddressBookHardcodingTest.php b/tests/Architecture/NoAddressBookHardcodingTest.php index 4f5cc9e..c2289e2 100644 --- a/tests/Architecture/NoAddressBookHardcodingTest.php +++ b/tests/Architecture/NoAddressBookHardcodingTest.php @@ -12,8 +12,7 @@ use FilesystemIterator; * in the search infrastructure, layout context builder, or aside templates. * * Intentionally allowed in Core (not flagged): - * - PermissionService::ADDRESS_BOOK_VIEW (general people-visibility gate) - * - OperationsAuthorizationPolicy / UiCapabilityMap / UserAuthorizationPolicy (authorization) + * - OperationsAuthorizationPolicy / UiCapabilityMap / UserAuthorizationPolicy (authorization — uses inline 'address_book.view' string) * - modules/addressbook/* (module-owned runtime, pages/assets/providers) * - i18n translation files * - Module directory (lib/Module/, modules/) diff --git a/tests/Service/Access/UserAuthorizationPolicyTest.php b/tests/Service/Access/UserAuthorizationPolicyTest.php index ea4696f..0d73419 100644 --- a/tests/Service/Access/UserAuthorizationPolicyTest.php +++ b/tests/Service/Access/UserAuthorizationPolicyTest.php @@ -364,7 +364,7 @@ class UserAuthorizationPolicyTest extends TestCase public function testAvatarViewAllowsInScopeAddressBookViewer(): void { $permissionService = $this->permissionGatewayAllowing([ - 5 => [PermissionService::ADDRESS_BOOK_VIEW], + 5 => ['address_book.view'], ]); $scopeGateway = $this->createMock(TenantScopeService::class); diff --git a/tests/Service/Auth/AuthSessionTenantContextServiceTest.php b/tests/Service/Auth/AuthSessionTenantContextServiceTest.php index 387fed3..34741f1 100644 --- a/tests/Service/Auth/AuthSessionTenantContextServiceTest.php +++ b/tests/Service/Auth/AuthSessionTenantContextServiceTest.php @@ -34,7 +34,7 @@ class AuthSessionTenantContextServiceTest extends TestCase $this->assertSame([], $_SESSION['available_tenants']); $this->assertTrue((bool) $_SESSION['no_active_tenant']); $this->assertArrayNotHasKey('current_tenant', $_SESSION); - $this->assertArrayNotHasKey('available_departments_by_tenant', $_SESSION); + $this->assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } public function testHydrateUsesFirstAvailableTenantAsFallback(): void @@ -54,7 +54,7 @@ class AuthSessionTenantContextServiceTest extends TestCase $this->assertSame($availableTenants, $_SESSION['available_tenants']); $this->assertFalse((bool) $_SESSION['no_active_tenant']); $this->assertSame(7, (int) ($_SESSION['current_tenant']['id'] ?? 0)); - $this->assertArrayNotHasKey('available_departments_by_tenant', $_SESSION); + $this->assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } public function testHydratePreservesThemeAndPolicyFieldsInSession(): void diff --git a/tests/Unit/Module/AddressBookAuthorizationPolicyTest.php b/tests/Unit/Module/AddressBookAuthorizationPolicyTest.php index 280b835..f89628f 100644 --- a/tests/Unit/Module/AddressBookAuthorizationPolicyTest.php +++ b/tests/Unit/Module/AddressBookAuthorizationPolicyTest.php @@ -38,7 +38,7 @@ final class AddressBookAuthorizationPolicyTest extends TestCase public function testAllowedWithPermission(): void { $permissionService = $this->permissionGatewayAllowing([ - 5 => [PermissionService::ADDRESS_BOOK_VIEW], + 5 => [AddressBookAuthorizationPolicy::PERMISSION_KEY], ]); $policy = new AddressBookAuthorizationPolicy($permissionService); diff --git a/tests/Unit/Module/LayoutContextProviderTest.php b/tests/Unit/Module/LayoutContextProviderTest.php index 745faa3..3afbdb3 100644 --- a/tests/Unit/Module/LayoutContextProviderTest.php +++ b/tests/Unit/Module/LayoutContextProviderTest.php @@ -38,7 +38,7 @@ final class LayoutContextProviderTest extends TestCase $container = new AppContainer(); $session = [ - 'available_departments_by_tenant' => [ + 'module.addressbook.departments_by_tenant' => [ [ 'tenant' => ['uuid' => 'abc', 'description' => 'Tenant A'], 'departments' => [['id' => 1, 'description' => 'Dept 1']], diff --git a/tests/Unit/Module/ModuleEventDispatcherTest.php b/tests/Unit/Module/ModuleEventDispatcherTest.php new file mode 100644 index 0000000..e6d32c9 --- /dev/null +++ b/tests/Unit/Module/ModuleEventDispatcherTest.php @@ -0,0 +1,100 @@ +called = true; + $this->capturedEvent = $event; + $this->capturedPayload = $payload; + } + }; + + $container = new AppContainer(); + $container->set($listener::class, static fn () => $listener); + + $dispatcher = new ModuleEventDispatcher( + ['user.deleted' => [['class' => $listener::class, 'method' => 'handle']]], + $container + ); + + $dispatcher->dispatch('user.deleted', ['user_id' => 42]); + + self::assertTrue($called); + self::assertSame('user.deleted', $capturedEvent); + self::assertSame(['user_id' => 42], $capturedPayload); + } + + public function testDispatchIsNoOpForUnknownEvent(): void + { + $container = new AppContainer(); + $dispatcher = new ModuleEventDispatcher([], $container); + + // Should not throw + $dispatcher->dispatch('unknown.event', ['data' => 'test']); + + self::addToAssertionCount(1); + } + + public function testDispatchContinuesAfterListenerException(): void + { + $secondCalled = false; + + $failingListener = new class () implements EventListener { + public function handle(string $event, array $payload): void + { + throw new \RuntimeException('Listener failure'); + } + }; + + $successListener = new class ($secondCalled) implements EventListener { + public function __construct(private bool &$called) + { + } + + public function handle(string $event, array $payload): void + { + $this->called = true; + } + }; + + $container = new AppContainer(); + $container->set($failingListener::class, static fn () => $failingListener); + $container->set($successListener::class, static fn () => $successListener); + + $dispatcher = new ModuleEventDispatcher( + [ + 'user.created' => [ + ['class' => $failingListener::class, 'method' => 'handle'], + ['class' => $successListener::class, 'method' => 'handle'], + ], + ], + $container + ); + + $dispatcher->dispatch('user.created', ['user_id' => 1]); + + self::assertTrue($secondCalled, 'Second listener should be called even after the first one throws'); + } +} diff --git a/tests/Unit/Module/SessionProviderTest.php b/tests/Unit/Module/SessionProviderTest.php index 5cc8476..8a75e5e 100644 --- a/tests/Unit/Module/SessionProviderTest.php +++ b/tests/Unit/Module/SessionProviderTest.php @@ -39,22 +39,22 @@ final class SessionProviderTest extends TestCase public function testPopulateWithInvalidUserClearsSessionKey(): void { - $_SESSION['available_departments_by_tenant'] = [['tenant' => 'old']]; + $_SESSION['module.addressbook.departments_by_tenant'] = [['tenant' => 'old']]; $provider = new AddressBookSessionProvider(); $provider->populate(['id' => 0], $this->emptyContainer()); - self::assertArrayNotHasKey('available_departments_by_tenant', $_SESSION); + self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } public function testPopulateWithMissingUserIdClearsSessionKey(): void { - $_SESSION['available_departments_by_tenant'] = [['tenant' => 'old']]; + $_SESSION['module.addressbook.departments_by_tenant'] = [['tenant' => 'old']]; $provider = new AddressBookSessionProvider(); $provider->populate([], $this->emptyContainer()); - self::assertArrayNotHasKey('available_departments_by_tenant', $_SESSION); + self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } public function testPopulateReturnsEarlyWhenContainerMissesDependencies(): void @@ -69,19 +69,19 @@ final class SessionProviderTest extends TestCase // in production, bindings are always registered before providers run. } - self::assertArrayNotHasKey('available_departments_by_tenant', $_SESSION); + self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } public function testClearRemovesSessionKey(): void { - $_SESSION['available_departments_by_tenant'] = [ + $_SESSION['module.addressbook.departments_by_tenant'] = [ ['tenant' => ['uuid' => 'abc'], 'departments' => []], ]; $provider = new AddressBookSessionProvider(); $provider->clear(); - self::assertArrayNotHasKey('available_departments_by_tenant', $_SESSION); + self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } public function testClearIsIdempotent(): void @@ -90,6 +90,6 @@ final class SessionProviderTest extends TestCase $provider = new AddressBookSessionProvider(); $provider->clear(); - self::assertArrayNotHasKey('available_departments_by_tenant', $_SESSION); + self::assertArrayNotHasKey('module.addressbook.departments_by_tenant', $_SESSION); } }