diff --git a/src/Core/Module/ModuleManager.php b/src/Core/Module/ModuleManager.php index dea8c185..b05f85d3 100644 --- a/src/Core/Module/ModuleManager.php +++ b/src/Core/Module/ModuleManager.php @@ -792,6 +792,10 @@ class ModuleManager { $targetVersion = $version ?? $this->manifestVersion($name) ?? $module->getVersion(); $modulePath = $this->modulePathFor($name); + // Capture the admin's on/off choice BEFORE the Installing transition + // overwrites it, or updating a disabled module switches it back on. + $rRestore = $this->stateAfterInstall($name); + $this->setState($name, ModuleState::Installing); try { @@ -812,7 +816,9 @@ class ModuleManager { throw $e; } - $this->setState($name, ModuleState::Enabled); + // writeState, not setState: restoring Disabled is an internal lifecycle + // transition and must not be second-guessed by the dependents guard. + $this->writeState($name, $rRestore); $this->recordInstalledVersion($name, $targetVersion); } @@ -1306,7 +1312,7 @@ class ModuleManager { * @param ModuleState $state Target lifecycle state. */ public function setState(string $name, ModuleState $state): void { - $name = $this->sanitizeModuleName($name); + $name = $this->sanitizeModuleName($name); // Refuse to disable a module that a still-working dependent relies on // (e.g. plex requires watch — watch cannot be disabled under it, or @@ -1324,6 +1330,35 @@ class ModuleManager { } } + $this->writeState($name, $state); + } + + /** + * The state a (re)install should leave the module in. + * + * An admin who switched the module off expects it to stay off across a + * reinstall or a store update; everything else — never installed, or one of + * the transient Installing / Failed states — means Enabled. Reads the raw + * override rather than listModules() precisely so those transient states + * cannot be mistaken for a deliberate "disabled". + */ + private function stateAfterInstall(string $name): ModuleState { + $rRaw = $this->readOverrides()[$name]['state'] ?? null; + + return $rRaw === ModuleState::Disabled->value ? ModuleState::Disabled : ModuleState::Enabled; + } + + /** + * Persist a lifecycle state without the dependents guard. + * + * That guard belongs to a deliberate admin "disable", not to the transitions + * install/uninstall drive themselves: putting a module back into the state it + * already had must never fail because something depends on it. + * + * @param string $name Module name (already sanitized). + * @param ModuleState $state Target lifecycle state. + */ + private function writeState(string $name, ModuleState $state): void { $overrides = $this->readOverrides(); if (!isset($overrides[$name]) || !is_array($overrides[$name])) { diff --git a/tests/Unit/ModuleManagerMigrationsTest.php b/tests/Unit/ModuleManagerMigrationsTest.php index 42854972..6e5925c9 100644 --- a/tests/Unit/ModuleManagerMigrationsTest.php +++ b/tests/Unit/ModuleManagerMigrationsTest.php @@ -348,6 +348,45 @@ final class ModuleManagerMigrationsTest extends TestCase { $this->assertSame([], $byName['ok-consumer']['dependency_warnings']); } + // ── install keeps the admin's on/off decision ───────────────────────── + + public function testReinstallKeepsAModuleTheAdminDisabled(): void { + // A store update lands in installModule(), which used to finish with an + // unconditional Enabled — silently switching a disabled module back on. + $this->createModule('keep-off', '1.0.0'); + $this->writeOverrides(['keep-off' => ['state' => 'disabled', 'installed_version' => '1.0.0']]); + + $this->manager()->installModule('keep-off'); + + $overrides = $this->readOverrides(); + $this->assertSame('disabled', $overrides['keep-off']['state'] ?? null); + $this->assertSame('1.0.0', $overrides['keep-off']['installed_version'] ?? null); + } + + public function testFirstInstallEnablesTheModule(): void { + $this->createModule('turn-on', '1.0.0'); + + $this->manager()->installModule('turn-on'); + + // Enabled is the clean default, recorded by the ABSENCE of a state key. + $this->assertArrayNotHasKey('state', $this->readOverrides()['turn-on']); + } + + public function testReinstallRestoresDisabledEvenWithADependent(): void { + // Restoring Disabled is a lifecycle transition, not an admin "disable", + // so the dependents guard must not veto it and fail the install. + $this->createModuleWithDeps('dep-off', '1.0.0', []); + $this->createModuleWithDeps('dep-user', '1.0.0', ['dep-off']); + $this->writeOverrides([ + 'dep-off' => ['state' => 'disabled', 'installed_version' => '1.0.0'], + 'dep-user' => ['installed_version' => '1.0.0'], + ]); + + $this->manager()->installModule('dep-off'); + + $this->assertSame('disabled', $this->readOverrides()['dep-off']['state'] ?? null); + } + // ── update/uninstall failure containment ────────────────────────────── public function testUpdateKeepsWatermarkAtLastCompletedVersionOnFailure(): void {