Fix disableDefaultWorkers() edge case where user-added workers were incorrectly removed (#102)

Author: kayw-geek

config/config.php

@@ -55,10 +55,13 @@
             __DIR__ . '/../packages/Release/ReleaseWorker',
         ]);
 
-    // Always register default workers here. They will be removed by
-    // RemoveDefaultWorkersCompilerPass if user called disableDefaultWorkers()
-    $services->set(TagVersionReleaseWorker::class);
-    $services->set(PushTagReleaseWorker::class);
+    // Register default workers with a special tag to identify them.
+    // RemoveDefaultWorkersCompilerPass will remove services with this tag
+    // if user called disableDefaultWorkers(), but preserve user-registered workers.
+    $services->set(TagVersionReleaseWorker::class)
+        ->tag('monorepo.default_worker');
+    $services->set(PushTagReleaseWorker::class)
+        ->tag('monorepo.default_worker');
 
     $services->load('Symplify\MonorepoBuilder\\', __DIR__ . '/../src')
         ->exclude([

packages-tests/Release/DisableDefaultWorkers/DisableDefaultWorkersTest.php

@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\MonorepoBuilder\Tests\Release\DisableDefaultWorkers;
+
+use PHPUnit\Framework\TestCase;
+use ReflectionClass;
+use Symplify\MonorepoBuilder\Config\MBConfig;
+use Symplify\MonorepoBuilder\Kernel\MonorepoBuilderKernel;
+use Symplify\MonorepoBuilder\Release\ReleaseWorker\PushTagReleaseWorker;
+use Symplify\MonorepoBuilder\Release\ReleaseWorker\TagVersionReleaseWorker;
+
+/**
+ * Tests for MBConfig::disableDefaultWorkers() functionality.
+ *
+ * @see https://github.com/symplify/monorepo-builder/issues/95
+ */
+final class DisableDefaultWorkersTest extends TestCase
+{
+    protected function setUp(): void
+    {
+        // Reset static state before each test
+        $this->resetMBConfigState();
+    }
+
+    protected function tearDown(): void
+    {
+        // Clean up static state after each test
+        $this->resetMBConfigState();
+    }
+
+    /**
+     * Scenario 1: Default behavior - workers should be registered
+     */
+    public function testDefaultWorkersAreRegisteredByDefault(): void
+    {
+        $monorepoBuilderKernel = new MonorepoBuilderKernel();
+        $container = $monorepoBuilderKernel->createFromConfigs([__DIR__ . '/config/default_behavior.php']);
+
+        $this->assertTrue($container->has(TagVersionReleaseWorker::class));
+        $this->assertTrue($container->has(PushTagReleaseWorker::class));
+    }
+
+    /**
+     * Scenario 2: disableDefaultWorkers() removes both default workers
+     */
+    public function testDisableDefaultWorkersRemovesBothWorkers(): void
+    {
+        $monorepoBuilderKernel = new MonorepoBuilderKernel();
+        $container = $monorepoBuilderKernel->createFromConfigs([__DIR__ . '/config/disable_default_workers.php']);
+
+        $this->assertFalse($container->has(TagVersionReleaseWorker::class));
+        $this->assertFalse($container->has(PushTagReleaseWorker::class));
+    }
+
+    /**
+     * Scenario 3: disableDefaultWorkers() + manually add worker preserves user's worker
+     */
+    public function testDisableDefaultWorkersPreservesUserAddedWorker(): void
+    {
+        $monorepoBuilderKernel = new MonorepoBuilderKernel();
+        $container = $monorepoBuilderKernel->createFromConfigs([__DIR__ . '/config/disable_and_add_custom.php']);
+
+        // User explicitly added TagVersionReleaseWorker, so it should be preserved
+        $this->assertTrue($container->has(TagVersionReleaseWorker::class));
+        // User did not add PushTagReleaseWorker, so it should be removed
+        $this->assertFalse($container->has(PushTagReleaseWorker::class));
+    }
+
+    private function resetMBConfigState(): void
+    {
+        $reflectionClass = new ReflectionClass(MBConfig::class);
+        $reflectionProperty = $reflectionClass->getProperty('disableDefaultWorkers');
+        $reflectionProperty->setValue(null, false);
+    }
+}

packages-tests/Release/DisableDefaultWorkers/config/default_behavior.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Symplify\MonorepoBuilder\Config\MBConfig;
+
+return static function (MBConfig $mbConfig): void {
+    // Default behavior - don't disable default workers
+    $mbConfig->defaultBranch('main');
+};

packages-tests/Release/DisableDefaultWorkers/config/disable_and_add_custom.php

@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+use Symplify\MonorepoBuilder\Config\MBConfig;
+use Symplify\MonorepoBuilder\Release\ReleaseWorker\TagVersionReleaseWorker;
+
+return static function (MBConfig $mbConfig): void {
+    // Disable default workers
+    MBConfig::disableDefaultWorkers();
+
+    // But user explicitly wants TagVersionReleaseWorker
+    // This should be preserved because user registration replaces the tagged default
+    $mbConfig->workers([
+        TagVersionReleaseWorker::class,
+    ]);
+};

packages-tests/Release/DisableDefaultWorkers/config/disable_default_workers.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Symplify\MonorepoBuilder\Config\MBConfig;
+
+return static function (MBConfig $mbConfig): void {
+    // Disable default workers - both should be removed
+    MBConfig::disableDefaultWorkers();
+};

src-deps/autowire-array-parameter/src/DependencyInjection/CompilerPass/AutowireArrayParameterCompilerPass.php

@@ -4,16 +4,16 @@
 
 namespace Symplify\AutowireArrayParameter\DependencyInjection\CompilerPass;
 
-use Symfony\Component\Config\Loader\LoaderInterface;
-use Symfony\Component\VarDumper\Dumper\ContextProvider\ContextProviderInterface;
-use Symfony\Component\HttpKernel\KernelInterface;
 use Nette\Utils\Strings;
 use ReflectionClass;
 use ReflectionMethod;
+use Symfony\Component\Config\Loader\LoaderInterface;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\HttpKernel\KernelInterface;
+use Symfony\Component\VarDumper\Dumper\ContextProvider\ContextProviderInterface;
 use Symplify\AutowireArrayParameter\DependencyInjection\DefinitionFinder;
 use Symplify\AutowireArrayParameter\DocBlock\ParamTypeDocBlockResolver;
 use Symplify\AutowireArrayParameter\Skipper\ParameterSkipper;

src-deps/autowire-array-parameter/src/Skipper/ParameterSkipper.php

@@ -4,13 +4,13 @@
 
 namespace Symplify\AutowireArrayParameter\Skipper;
 
-use Symfony\Component\Config\Loader\LoaderInterface;
-use Symfony\Component\VarDumper\Dumper\ContextProvider\ContextProviderInterface;
 use ReflectionMethod;
 use ReflectionNamedType;
 use ReflectionParameter;
 use ReflectionType;
+use Symfony\Component\Config\Loader\LoaderInterface;
 use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\VarDumper\Dumper\ContextProvider\ContextProviderInterface;
 use Symplify\AutowireArrayParameter\TypeResolver\ParameterTypeResolver;
 
 final class ParameterSkipper

src/DependencyInjection/CompilerPass/RemoveDefaultWorkersCompilerPass.php

@@ -7,34 +7,35 @@
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symplify\MonorepoBuilder\Config\MBConfig;
-use Symplify\MonorepoBuilder\Release\ReleaseWorker\PushTagReleaseWorker;
-use Symplify\MonorepoBuilder\Release\ReleaseWorker\TagVersionReleaseWorker;
 
 /**
  * Removes default release workers from the container if MBConfig::disableDefaultWorkers() was called.
- * This needs to happen in a compiler pass rather than during config loading because:
- * 1. Default config is loaded before user config (to allow parameter overriding)
- * 2. User config calls disableDefaultWorkers() after default config has already registered workers
- * 3. Compiler passes run after all configs are loaded, so they can see the final state
+ *
+ * This uses a tag-based approach to distinguish between:
+ * - Default workers registered by config/config.php (tagged with 'monorepo.default_worker')
+ * - User-registered workers (no tag, or re-registered without the tag)
+ *
+ * When user calls disableDefaultWorkers() and then manually registers a worker
+ * (even one with the same class as a default worker), only the tagged default
+ * definition is removed, preserving the user's explicit registration.
  */
 final readonly class RemoveDefaultWorkersCompilerPass implements CompilerPassInterface
 {
+    private const DEFAULT_WORKER_TAG = 'monorepo.default_worker';
+
     public function process(ContainerBuilder $containerBuilder): void
     {
         // Check if user disabled default workers in their config
         if (! MBConfig::isDisableDefaultWorkers()) {
             return;
         }
 
-        // Remove the default workers from the container
-        $defaultWorkers = [
-            TagVersionReleaseWorker::class,
-            PushTagReleaseWorker::class,
-        ];
+        // Find and remove only services tagged as default workers
+        $taggedServices = $containerBuilder->findTaggedServiceIds(self::DEFAULT_WORKER_TAG);
 
-        foreach ($defaultWorkers as $defaultWorker) {
-            if ($containerBuilder->hasDefinition($defaultWorker)) {
-                $containerBuilder->removeDefinition($defaultWorker);
+        foreach (array_keys($taggedServices) as $serviceId) {
+            if ($containerBuilder->hasDefinition($serviceId)) {
+                $containerBuilder->removeDefinition($serviceId);
             }
         }
     }