diff --git a/src/ImplementationAgent/Facade/Message/ImplementIssueMessage.php b/src/ImplementationAgent/Facade/Message/ImplementIssueMessage.php index eae660e..602bbdb 100644 --- a/src/ImplementationAgent/Facade/Message/ImplementIssueMessage.php +++ b/src/ImplementationAgent/Facade/Message/ImplementIssueMessage.php @@ -17,6 +17,7 @@ public function __construct( public string $runId = '', public ?string $agentApiKeyOverride = null, public ?string $agentProviderOverride = null, + public int $retryCount = 0, ) { } } diff --git a/src/ImplementationAgent/Infrastructure/Service/ImplementationRunExecutor.php b/src/ImplementationAgent/Infrastructure/Service/ImplementationRunExecutor.php index e911752..3c02ddc 100644 --- a/src/ImplementationAgent/Infrastructure/Service/ImplementationRunExecutor.php +++ b/src/ImplementationAgent/Infrastructure/Service/ImplementationRunExecutor.php @@ -20,10 +20,14 @@ use App\Workflow\Facade\WorkflowConcurrencyFacadeInterface; use App\WorkspaceManagement\Facade\WorkspaceManagementFacadeInterface; use Psr\Log\LoggerInterface; +use Symfony\Component\Messenger\MessageBusInterface; +use Symfony\Component\Uid\Uuid; use Throwable; readonly class ImplementationRunExecutor implements ImplementationRunExecutorInterface { + private const int MAX_RETRIES = 2; + private const string OUTPUT_FORMAT_INSTRUCTIONS = <<<'INSTRUCTIONS' ## Required Output Format @@ -61,6 +65,7 @@ public function __construct( private LlmIntegrationFacadeInterface $llmIntegrationFacade, private LoggerInterface $logger, private MessengerLogContext $logContext, + private MessageBusInterface $messageBus, ) { } @@ -113,6 +118,9 @@ private function doExecute(ImplementIssueMessage $message, ProductConfigDto $con return; } + $agentErrorText = null; + $exceptionMessage = null; + try { $issueDto = $this->findIssue($config->githubUrl, $config->githubToken, $message->issueNumber); if ($issueDto === null) { @@ -181,15 +189,19 @@ private function doExecute(ImplementIssueMessage $message, ProductConfigDto $con 'durationMs' => $agentResult->durationMs, ]); - $this->handleOutcome( - $outcome, - $agentResult->resultText, - $config->githubUrl, - $config->githubToken, - $message->issueNumber, - $issueDto, - $message->prNumber, - ); + if ($outcome === ImplementationOutcome::Error) { + $agentErrorText = $this->stripMarkers($agentResult->resultText); + } else { + $this->handleOutcome( + $outcome, + $agentResult->resultText, + $config->githubUrl, + $config->githubToken, + $message->issueNumber, + $issueDto, + $message->prNumber, + ); + } } catch (Throwable $e) { $this->logger->error('[ImplementationAgent] Unhandled exception', [ 'issueNumber' => $message->issueNumber, @@ -197,14 +209,7 @@ private function doExecute(ImplementIssueMessage $message, ProductConfigDto $con 'error' => $e->getMessage(), ]); - $this->applyErrorLabels($config->githubUrl, $config->githubToken, $message->issueNumber); - - $this->githubIntegrationFacade->postIssueComment( - $config->githubUrl, - $config->githubToken, - $message->issueNumber, - "**ProductBuilder Implementation Error**\n\nAn unexpected error occurred during implementation:\n\n> " . $e->getMessage(), - ); + $exceptionMessage = $e->getMessage(); } finally { if ($workspaceInfo !== null) { $this->workspaceManagementFacade->releaseWorkspace($workspaceInfo); @@ -217,6 +222,53 @@ private function doExecute(ImplementIssueMessage $message, ProductConfigDto $con $message->runId, ); } + + if ($agentErrorText !== null || $exceptionMessage !== null) { + if ($message->retryCount < self::MAX_RETRIES) { + $newRunId = Uuid::v4()->toRfc4122(); + + if ($this->workflowConcurrencyFacade->tryCreateRunClaim( + $message->productConfigId, + $message->issueNumber, + WorkflowRunPhase::Implementation, + $newRunId, + )) { + $this->logger->warning('[ImplementationAgent] Error on attempt, scheduling retry', [ + 'issueNumber' => $message->issueNumber, + 'attempt' => $message->retryCount + 1, + ]); + + $this->messageBus->dispatch(new ImplementIssueMessage( + $message->productConfigId, + $message->issueNumber, + $message->isRevision, + $message->prBranchName, + $message->prNumber, + $newRunId, + $message->agentApiKeyOverride, + $message->agentProviderOverride, + $message->retryCount + 1, + )); + + return; + } + } + + $this->githubIntegrationFacade->postIssueComment( + $config->githubUrl, + $config->githubToken, + $message->issueNumber, + $exceptionMessage !== null + ? "**ProductBuilder Implementation Error**\n\nAn unexpected error occurred during implementation:\n\n> " . $exceptionMessage + : "**ProductBuilder Implementation Error**\n\nThe implementation agent encountered an error:\n\n" . $agentErrorText, + ); + + $this->applyErrorLabels($config->githubUrl, $config->githubToken, $message->issueNumber); + + $this->logger->error('[ImplementationAgent] Implementation failed after all retries', [ + 'issueNumber' => $message->issueNumber, + ]); + } } private function findIssue(string $githubUrl, string $githubToken, int $issueNumber): ?GithubIssueDto @@ -328,7 +380,7 @@ private function handleOutcome( match ($outcome) { ImplementationOutcome::PrCreated => $this->handlePrCreated($resultText, $githubUrl, $githubToken, $issueNumber, $issueDto), ImplementationOutcome::RevisionPushed => $this->handleRevisionPushed($githubUrl, $githubToken, $issueNumber, $prNumber), - ImplementationOutcome::Error => $this->handleError($resultText, $githubUrl, $githubToken, $issueNumber), + ImplementationOutcome::Error => null, // unreachable; error is captured and handled after finally blocks }; } @@ -341,14 +393,7 @@ private function handlePrCreated( ): void { $metadata = ImplementationOutcome::extractPrMetadata($resultText); if ($metadata === null) { - $this->handleError( - 'Agent indicated PR readiness but did not return valid PR metadata markers.', - $githubUrl, - $githubToken, - $issueNumber, - ); - - return; + throw new \RuntimeException('Agent indicated PR readiness but did not return valid PR metadata markers.'); } $body = $metadata->body; @@ -416,24 +461,6 @@ private function handleRevisionPushed(string $githubUrl, string $githubToken, in ]); } - private function handleError(string $resultText, string $githubUrl, string $githubToken, int $issueNumber): void - { - $cleanedText = $this->stripMarkers($resultText); - - $this->githubIntegrationFacade->postIssueComment( - $githubUrl, - $githubToken, - $issueNumber, - "**ProductBuilder Implementation Error**\n\nThe implementation agent encountered an error:\n\n" . $cleanedText, - ); - - $this->applyErrorLabels($githubUrl, $githubToken, $issueNumber); - - $this->logger->error('[ImplementationAgent] Implementation errored', [ - 'issueNumber' => $issueNumber, - ]); - } - private function stripMarkers(string $text): string { $cleaned = str_replace(['[PR_READY]', '[REVISION_PUSHED]'], '', $text); diff --git a/src/PlanningAgent/Facade/Message/PlanIssueMessage.php b/src/PlanningAgent/Facade/Message/PlanIssueMessage.php index 4ba7ba7..4cc7054 100644 --- a/src/PlanningAgent/Facade/Message/PlanIssueMessage.php +++ b/src/PlanningAgent/Facade/Message/PlanIssueMessage.php @@ -15,6 +15,7 @@ public function __construct( public string $runId, public ?string $agentApiKeyOverride = null, public ?string $agentProviderOverride = null, + public int $retryCount = 0, ) { } } diff --git a/src/PlanningAgent/Infrastructure/Service/PlanningRunExecutor.php b/src/PlanningAgent/Infrastructure/Service/PlanningRunExecutor.php index c61fdc9..4d7d6b9 100644 --- a/src/PlanningAgent/Infrastructure/Service/PlanningRunExecutor.php +++ b/src/PlanningAgent/Infrastructure/Service/PlanningRunExecutor.php @@ -20,10 +20,14 @@ use App\Workflow\Facade\WorkflowConcurrencyFacadeInterface; use App\WorkspaceManagement\Facade\WorkspaceManagementFacadeInterface; use Psr\Log\LoggerInterface; +use Symfony\Component\Messenger\MessageBusInterface; +use Symfony\Component\Uid\Uuid; use Throwable; readonly class PlanningRunExecutor implements PlanningRunExecutorInterface { + private const int MAX_RETRIES = 2; + private const string OUTPUT_FORMAT_INSTRUCTIONS = <<<'INSTRUCTIONS' ## Required Output Format @@ -50,6 +54,7 @@ public function __construct( private LlmIntegrationFacadeInterface $llmIntegrationFacade, private LoggerInterface $logger, private MessengerLogContext $logContext, + private MessageBusInterface $messageBus, ) { } @@ -99,6 +104,9 @@ private function doExecute(PlanIssueMessage $message, ProductConfigDto $config): return; } + $agentErrorText = null; + $exceptionMessage = null; + try { $issueDto = $this->findIssue($config->githubUrl, $config->githubToken, $message->issueNumber); if ($issueDto === null) { @@ -149,27 +157,24 @@ private function doExecute(PlanIssueMessage $message, ProductConfigDto $config): 'durationMs' => $agentResult->durationMs, ]); - $this->handleOutcome( - $outcome, - $agentResult->resultText, - $config->githubUrl, - $config->githubToken, - $message->issueNumber, - ); + if ($outcome === PlanningOutcome::Error) { + $agentErrorText = $this->stripMarkers($agentResult->resultText); + } else { + $this->handleOutcome( + $outcome, + $agentResult->resultText, + $config->githubUrl, + $config->githubToken, + $message->issueNumber, + ); + } } catch (Throwable $e) { $this->logger->error('[PlanningAgent] Unhandled exception', [ 'issueNumber' => $message->issueNumber, 'error' => $e->getMessage(), ]); - $this->applyErrorLabels($config->githubUrl, $config->githubToken, $message->issueNumber); - - $this->githubIntegrationFacade->postIssueComment( - $config->githubUrl, - $config->githubToken, - $message->issueNumber, - "**ProductBuilder Planning Error**\n\nAn unexpected error occurred during planning:\n\n> " . $e->getMessage(), - ); + $exceptionMessage = $e->getMessage(); } finally { if ($workspaceInfo !== null) { $this->workspaceManagementFacade->releaseWorkspace($workspaceInfo); @@ -182,6 +187,51 @@ private function doExecute(PlanIssueMessage $message, ProductConfigDto $config): $message->runId, ); } + + if ($agentErrorText !== null || $exceptionMessage !== null) { + if ($message->retryCount < self::MAX_RETRIES) { + $newRunId = Uuid::v4()->toRfc4122(); + + if ($this->workflowConcurrencyFacade->tryCreateRunClaim( + $message->productConfigId, + $message->issueNumber, + WorkflowRunPhase::Planning, + $newRunId, + )) { + $this->logger->warning('[PlanningAgent] Error on attempt, scheduling retry', [ + 'issueNumber' => $message->issueNumber, + 'attempt' => $message->retryCount + 1, + ]); + + $this->messageBus->dispatch(new PlanIssueMessage( + $message->productConfigId, + $message->issueNumber, + $message->isResume, + $newRunId, + $message->agentApiKeyOverride, + $message->agentProviderOverride, + $message->retryCount + 1, + )); + + return; + } + } + + $this->githubIntegrationFacade->postIssueComment( + $config->githubUrl, + $config->githubToken, + $message->issueNumber, + $exceptionMessage !== null + ? "**ProductBuilder Planning Error**\n\nAn unexpected error occurred during planning:\n\n> " . $exceptionMessage + : "**ProductBuilder Planning Error**\n\nThe planning agent encountered an error:\n\n" . $agentErrorText, + ); + + $this->applyErrorLabels($config->githubUrl, $config->githubToken, $message->issueNumber); + + $this->logger->error('[PlanningAgent] Planning failed after all retries', [ + 'issueNumber' => $message->issueNumber, + ]); + } } private function findIssue(string $githubUrl, string $githubToken, int $issueNumber): ?GithubIssueDto @@ -246,7 +296,7 @@ private function handleOutcome( match ($outcome) { PlanningOutcome::FeedbackNeeded => $this->handleFeedbackNeeded($cleanedText, $githubUrl, $githubToken, $issueNumber), PlanningOutcome::PlanProduced => $this->handlePlanProduced($cleanedText, $githubUrl, $githubToken, $issueNumber), - PlanningOutcome::Error => $this->handleError($cleanedText, $githubUrl, $githubToken, $issueNumber), + PlanningOutcome::Error => null, // unreachable; error is captured and handled after finally blocks }; } @@ -290,22 +340,6 @@ private function handlePlanProduced(string $text, string $githubUrl, string $git ]); } - private function handleError(string $text, string $githubUrl, string $githubToken, int $issueNumber): void - { - $this->githubIntegrationFacade->postIssueComment( - $githubUrl, - $githubToken, - $issueNumber, - "**ProductBuilder Planning Error**\n\nThe planning agent encountered an error:\n\n" . $text, - ); - - $this->applyErrorLabels($githubUrl, $githubToken, $issueNumber); - - $this->logger->error('[PlanningAgent] Planning errored', [ - 'issueNumber' => $issueNumber, - ]); - } - private function applyErrorLabels(string $githubUrl, string $githubToken, int $issueNumber): void { $this->githubIntegrationFacade->removeLabelFromIssue($githubUrl, $githubToken, $issueNumber, GithubLabel::PlanningOngoing); diff --git a/tests/Unit/ImplementationAgent/ImplementationRunExecutorTest.php b/tests/Unit/ImplementationAgent/ImplementationRunExecutorTest.php new file mode 100644 index 0000000..450199d --- /dev/null +++ b/tests/Unit/ImplementationAgent/ImplementationRunExecutorTest.php @@ -0,0 +1,580 @@ +tryStartRunResult = false; + $llm = new FakeLlmIntegrationFacadeForImplementationRunExecutor(); + $bus = new FakeMessageBusForImplementationRunExecutor(); + + $executor = new ImplementationRunExecutor( + $productConfig, + $github, + $workspace, + $concurrency, + $llm, + new NullLogger(), + new MessengerLogContext(), + $bus, + ); + + $executor->execute(new ImplementIssueMessage('cfg-1', 77, false, null, null, 'run-1')); + + expect($github->getIssueByNumberCalls)->toBe(0); + expect($workspace->acquireCalls)->toBe(0); + expect($concurrency->releasedClaims)->toBe([]); + }); + + it('returns early when product config is missing', function (): void { + $productConfig = new FakeProductConfigFacadeForImplementationRunExecutor(null); + $github = new FakeGithubIntegrationFacadeForImplementationRunExecutor(); + $workspace = new FakeWorkspaceManagementFacadeForImplementationRunExecutor(); + $concurrency = new FakeWorkflowConcurrencyFacadeForImplementationRunExecutor(); + $llm = new FakeLlmIntegrationFacadeForImplementationRunExecutor(); + $bus = new FakeMessageBusForImplementationRunExecutor(); + + $executor = new ImplementationRunExecutor( + $productConfig, + $github, + $workspace, + $concurrency, + $llm, + new NullLogger(), + new MessengerLogContext(), + $bus, + ); + + $executor->execute(new ImplementIssueMessage('cfg-404', 77, false, null, null, 'run-1')); + + expect($concurrency->tryStartCalls)->toBe(0); + expect($concurrency->releasedClaims)->toBe([]); + }); + + it('opens PR and posts comment on successful implementation', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-ok')); + + expect($state->github->postedComments)->toHaveCount(1); + expect($state->github->postedComments[0]['body'])->toContain('PR Opened'); + expect($state->github->addedLabels)->toContain(GithubLabel::ImplementationDone->value); + expect($state->github->removedLabels)->toContain(GithubLabel::ImplementationOngoing->value); + expect($state->workspace->releaseCalls)->toBe(1); + expect($state->concurrency->releasedClaims)->toHaveCount(1); + }); + + it('applies error labels and error comment for error outcome after max retries', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-err', null, null, 2)); + + expect($state->github->postedComments)->toHaveCount(1); + expect($state->github->postedComments[0]['body'])->toContain('Implementation Error'); + expect($state->github->addedLabels)->toContain(GithubLabel::ImplementationErrored->value); + expect($state->workspace->releaseCalls)->toBe(1); + expect($state->concurrency->releasedClaims)->toHaveCount(1); + expect($state->bus->dispatchedMessages)->toHaveCount(0); + }); + + it('handles unhandled exception by applying error labels, posting comment, and releasing resources after max retries', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + $state->llm->throwOnRun = true; + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-ex', null, null, 2)); + + expect($state->github->postedComments)->toHaveCount(1); + expect($state->github->postedComments[0]['body'])->toContain('unexpected error occurred'); + expect($state->github->addedLabels)->toContain(GithubLabel::ImplementationErrored->value); + expect($state->workspace->releaseCalls)->toBe(1); + expect($state->concurrency->releasedClaims)->toHaveCount(1); + expect($state->bus->dispatchedMessages)->toHaveCount(0); + }); + + it('dispatches retry message on error outcome with retryCount=0, no error labels applied', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-r0')); + + expect($state->github->addedLabels)->not->toContain(GithubLabel::ImplementationErrored->value); + expect($state->github->postedComments)->toHaveCount(0); + expect($state->concurrency->tryCreateRunClaimCalls)->toBe(1); + expect($state->bus->dispatchedMessages)->toHaveCount(1); + + $dispatched = $state->bus->dispatchedMessages[0]; + expect($dispatched)->toBeInstanceOf(ImplementIssueMessage::class); + expect($dispatched->retryCount)->toBe(1); + expect($dispatched->productConfigId)->toBe('cfg-1'); + expect($dispatched->issueNumber)->toBe(99); + }); + + it('dispatches retry message on error outcome with retryCount=1', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-r1', null, null, 1)); + + expect($state->github->addedLabels)->not->toContain(GithubLabel::ImplementationErrored->value); + expect($state->github->postedComments)->toHaveCount(0); + expect($state->bus->dispatchedMessages)->toHaveCount(1); + expect($state->bus->dispatchedMessages[0]->retryCount)->toBe(2); + }); + + it('applies error labels on error outcome with retryCount=2 (max retries exhausted)', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-r2', null, null, 2)); + + expect($state->github->addedLabels)->toContain(GithubLabel::ImplementationErrored->value); + expect($state->github->postedComments)->toHaveCount(1); + expect($state->github->postedComments[0]['body'])->toContain('Implementation Error'); + expect($state->bus->dispatchedMessages)->toHaveCount(0); + }); + + it('dispatches retry message on throwable with retryCount=0, no error labels applied', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + $state->llm->throwOnRun = true; + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-ex0')); + + expect($state->github->addedLabels)->not->toContain(GithubLabel::ImplementationErrored->value); + expect($state->github->postedComments)->toHaveCount(0); + expect($state->bus->dispatchedMessages)->toHaveCount(1); + expect($state->bus->dispatchedMessages[0]->retryCount)->toBe(1); + }); + + it('applies error labels on throwable with retryCount=2 (max retries exhausted)', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + $state->llm->throwOnRun = true; + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-ex2', null, null, 2)); + + expect($state->github->addedLabels)->toContain(GithubLabel::ImplementationErrored->value); + expect($state->github->postedComments)->toHaveCount(1); + expect($state->bus->dispatchedMessages)->toHaveCount(0); + }); + + it('falls back to final error handling when tryCreateRunClaim fails during retry', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + $state->concurrency->tryCreateRunClaimResult = false; + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-claim-fail')); + + expect($state->github->addedLabels)->toContain(GithubLabel::ImplementationErrored->value); + expect($state->github->postedComments)->toHaveCount(1); + expect($state->bus->dispatchedMessages)->toHaveCount(0); + }); + + it('dispatches retry message when PR metadata extraction fails (retryCount=0)', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + // Agent returns PrCreated outcome but no metadata markers + $state->llm->result = new AgentRunResultDto(true, '[PR_READY] no metadata here', 'session-1', 1500); + + $state->executor->execute(new ImplementIssueMessage('cfg-1', 99, false, null, null, 'run-meta-fail')); + + expect($state->github->addedLabels)->not->toContain(GithubLabel::ImplementationErrored->value); + expect($state->github->postedComments)->toHaveCount(0); + expect($state->bus->dispatchedMessages)->toHaveCount(1); + expect($state->bus->dispatchedMessages[0]->retryCount)->toBe(1); + }); + + it('preserves isRevision, prBranchName, prNumber fields in retry message', function (): void { + $state = createImplementationRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + + $state->executor->execute(new ImplementIssueMessage( + 'cfg-1', + 99, + true, + 'feature/branch', + 42, + 'run-rev', + )); + + expect($state->bus->dispatchedMessages)->toHaveCount(1); + $dispatched = $state->bus->dispatchedMessages[0]; + expect($dispatched)->toBeInstanceOf(ImplementIssueMessage::class); + expect($dispatched->isRevision)->toBeTrue(); + expect($dispatched->prBranchName)->toBe('feature/branch'); + expect($dispatched->prNumber)->toBe(42); + expect($dispatched->retryCount)->toBe(1); + }); +}); + +function createImplementationRunExecutorHappyPathState(): ImplementationRunExecutorHappyPathState +{ + $productConfig = new FakeProductConfigFacadeForImplementationRunExecutor( + new ProductConfigDto('cfg-1', 'Demo', 'https://github.com/acme/demo', 'cursor-key', 'gh-token', '', 'anthropic-key'), + ); + $github = new FakeGithubIntegrationFacadeForImplementationRunExecutor(); + $github->issue = new GithubIssueDto( + 99, + 'Issue title', + 'Issue body', + [GithubLabel::ImplementationOngoing->value, GithubLabel::CoordinationOngoing->value], + [], + 'author', + ); + $github->comments = [ + new GithubCommentDto(1, 'plan comment', DateAndTimeService::getDateTimeImmutable(), 'bot'), + ]; + + $workspace = new FakeWorkspaceManagementFacadeForImplementationRunExecutor(); + $concurrency = new FakeWorkflowConcurrencyFacadeForImplementationRunExecutor(); + $llm = new FakeLlmIntegrationFacadeForImplementationRunExecutor(); + $bus = new FakeMessageBusForImplementationRunExecutor(); + + $executor = new ImplementationRunExecutor( + $productConfig, + $github, + $workspace, + $concurrency, + $llm, + new NullLogger(), + new MessengerLogContext(), + $bus, + ); + + return new ImplementationRunExecutorHappyPathState($executor, $github, $workspace, $concurrency, $llm, $bus); +} + +final readonly class ImplementationRunExecutorHappyPathState +{ + public function __construct( + public ImplementationRunExecutor $executor, + public FakeGithubIntegrationFacadeForImplementationRunExecutor $github, + public FakeWorkspaceManagementFacadeForImplementationRunExecutor $workspace, + public FakeWorkflowConcurrencyFacadeForImplementationRunExecutor $concurrency, + public FakeLlmIntegrationFacadeForImplementationRunExecutor $llm, + public FakeMessageBusForImplementationRunExecutor $bus, + ) { + } +} + +final class FakeProductConfigFacadeForImplementationRunExecutor implements ProductConfigFacadeInterface +{ + public function __construct( + private readonly ?ProductConfigDto $config, + ) { + } + + public function getAllProductConfigs(): array + { + if ($this->config === null) { + return []; + } + + return [$this->config]; + } + + public function getProductConfigById(string $id): ?ProductConfigDto + { + if ($this->config === null) { + return null; + } + + return $this->config->id === $id ? $this->config : null; + } + + public function findMatchingProductConfigs(string $githubOwner, string $githubRepo): array + { + return []; + } +} + +final class FakeGithubIntegrationFacadeForImplementationRunExecutor implements GithubIntegrationFacadeInterface +{ + public ?GithubIssueDto $issue = null; + /** @var list */ + public array $comments = []; + public int $getIssueByNumberCalls = 0; + /** @var list */ + public array $postedComments = []; + /** @var list */ + public array $addedLabels = []; + /** @var list */ + public array $removedLabels = []; + + public function parseIssueUrl(string $issueUrl): ?GithubIssueReferenceDto + { + return null; + } + + public function getIssuesWithLabel(string $githubUrl, string $githubToken, GithubLabel $label, string $state = 'open'): array + { + return []; + } + + public function getIssueByNumber(string $githubUrl, string $githubToken, int $issueNumber): ?GithubIssueDto + { + ++$this->getIssueByNumberCalls; + + return $this->issue; + } + + public function addLabelToIssue(string $githubUrl, string $githubToken, int $issueNumber, GithubLabel $label): void + { + $this->addedLabels[] = $label->value; + } + + public function removeLabelFromIssue(string $githubUrl, string $githubToken, int $issueNumber, GithubLabel $label): void + { + $this->removedLabels[] = $label->value; + } + + public function postIssueComment(string $githubUrl, string $githubToken, int $issueNumber, string $body): void + { + $this->postedComments[] = ['issue' => $issueNumber, 'body' => $body]; + } + + public function addIssueAssignee(string $githubUrl, string $githubToken, int $issueNumber, string $assigneeLogin): void + { + } + + public function ensureLabelsExist(string $githubUrl, string $githubToken): void + { + } + + public function getLabelAddedAt(string $githubUrl, string $githubToken, int $issueNumber, GithubLabel $label): ?DateTimeImmutable + { + return null; + } + + public function getLabelLatestActorLogin(string $githubUrl, string $githubToken, int $issueNumber, GithubLabel $label): ?string + { + return null; + } + + public function getIssueComments(string $githubUrl, string $githubToken, int $issueNumber): array + { + return $this->comments; + } + + public function getAuthenticatedUserLogin(string $githubToken): string + { + return 'bot'; + } + + public function getOpenPullRequestForIssue(string $githubUrl, string $githubToken, int $issueNumber): ?GithubPullRequestDto + { + return null; + } + + public function getPullRequestComments(string $githubUrl, string $githubToken, int $prNumber): array + { + return []; + } + + public function getPullRequestLatestCommitCommittedAt(string $githubUrl, string $githubToken, int $prNumber): ?DateTimeImmutable + { + return null; + } + + public function isIssueClosed(string $githubUrl, string $githubToken, int $issueNumber): bool + { + return false; + } + + public function requestPullRequestReviewer(string $githubUrl, string $githubToken, int $prNumber, string $reviewerLogin): void + { + } + + public function createPullRequest( + string $githubUrl, + string $githubToken, + string $title, + string $body, + string $headBranch, + ?string $baseBranch = null, + bool $draft = false, + ): GithubPullRequestDto { + return new GithubPullRequestDto( + 1, + $title, + $headBranch, + 'https://github.com/acme/repo/pull/1', + false, + ); + } +} + +final class FakeWorkspaceManagementFacadeForImplementationRunExecutor implements WorkspaceManagementFacadeInterface +{ + public int $acquireCalls = 0; + public int $releaseCalls = 0; + + public function acquireWorkspace( + string $productConfigId, + int $issueNumber, + string $githubUrl, + string $githubToken, + ?string $branchName = null, + ): WorkspaceInfoDto { + ++$this->acquireCalls; + + return new WorkspaceInfoDto('/tmp/workspace-impl-executor', null); + } + + public function releaseWorkspace(WorkspaceInfoDto $workspaceInfo): void + { + ++$this->releaseCalls; + } + + public function destroyWorkspace(WorkspaceInfoDto $workspaceInfo): void + { + } + + public function cleanupWorkspaceForIssue(string $productConfigId, int $issueNumber): void + { + } + + public function cleanupWorkspaceById(string $workspaceId): void + { + } + + public function lookupWorkspaceInfoByPrefix(string $workspaceIdPrefix): array + { + return []; + } + + public function createWorkspace(string $githubUrl, string $githubToken): WorkspaceInfoDto + { + return new WorkspaceInfoDto('/tmp/workspace-impl-executor', null); + } + + public function createWorkspaceOnBranch(string $githubUrl, string $githubToken, string $branchName): WorkspaceInfoDto + { + return new WorkspaceInfoDto('/tmp/workspace-impl-executor', null); + } +} + +final class FakeWorkflowConcurrencyFacadeForImplementationRunExecutor implements WorkflowConcurrencyFacadeInterface +{ + public bool $tryStartRunResult = true; + public bool $tryCreateRunClaimResult = true; + public int $tryStartCalls = 0; + public int $tryCreateRunClaimCalls = 0; + /** @var list */ + public array $releasedClaims = []; + + public function tryCreateRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId): bool + { + ++$this->tryCreateRunClaimCalls; + + return $this->tryCreateRunClaimResult; + } + + public function tryStartRun(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId): bool + { + ++$this->tryStartCalls; + + return $this->tryStartRunResult; + } + + public function releaseRunClaim(string $productConfigId, int $issueNumber, string $runId): void + { + $this->releasedClaims[] = [ + 'productConfigId' => $productConfigId, + 'issueNumber' => $issueNumber, + 'runId' => $runId, + ]; + } + + public function clearRunClaimsForIssue(string $productConfigId, int $issueNumber): int + { + return 0; + } + + public function resetIssueLocksAndClaims(string $productConfigId, int $issueNumber, string $teamleaderIssueLockKey): void + { + } +} + +final class FakeLlmIntegrationFacadeForImplementationRunExecutor implements LlmIntegrationFacadeInterface +{ + public bool $throwOnRun = false; + public AgentRunResultDto $result; + public ?string $lastCursorApiKey = null; + public ?string $lastAnthropicApiKey = null; + public ?string $lastAgentProviderOverride = null; + + public function __construct() + { + $this->result = new AgentRunResultDto( + true, + "[PR_READY]\nPR_HEAD_BRANCH: feature/issue-99\nPR_TITLE: Implement issue 99\nPR_BODY_BEGIN\nImplementation done.\n\nCloses #99\nPR_BODY_END", + 'session-1', + 1000, + ); + } + + public function runAgent( + AgentRole $role, + string $prompt, + string $workspacePath, + AgentCredentialsDto $credentials, + string $githubToken, + ?string $containerName = null, + ?string $agentProviderOverride = null, + ): AgentRunResultDto { + $this->lastCursorApiKey = $credentials->cursorApiKey; + $this->lastAnthropicApiKey = $credentials->anthropicApiKey; + $this->lastAgentProviderOverride = $agentProviderOverride; + + if ($this->throwOnRun) { + throw new RuntimeException('llm failed'); + } + + return $this->result; + } +} + +final class FakeMessageBusForImplementationRunExecutor implements MessageBusInterface +{ + /** @var list */ + public array $dispatchedMessages = []; + + /** + * @param list $stamps + */ + public function dispatch(object $message, array $stamps = []): Envelope + { + $this->dispatchedMessages[] = $message; + + return new Envelope($message, $stamps); + } +} diff --git a/tests/Unit/PlanningAgent/PlanningRunExecutorTest.php b/tests/Unit/PlanningAgent/PlanningRunExecutorTest.php index 5d96bed..66295f5 100644 --- a/tests/Unit/PlanningAgent/PlanningRunExecutorTest.php +++ b/tests/Unit/PlanningAgent/PlanningRunExecutorTest.php @@ -23,6 +23,8 @@ use App\WorkspaceManagement\Facade\WorkspaceManagementFacadeInterface; use EnterpriseToolingForSymfony\SharedBundle\DateAndTime\Service\DateAndTimeService; use Psr\Log\NullLogger; +use Symfony\Component\Messenger\Envelope; +use Symfony\Component\Messenger\MessageBusInterface; describe('PlanningRunExecutor', function (): void { it('skips stale messages when run start claim cannot transition', function (): void { @@ -34,6 +36,7 @@ $concurrency = new FakeWorkflowConcurrencyFacadeForPlanningRunExecutor(); $concurrency->tryStartRunResult = false; $llm = new FakeLlmIntegrationFacadeForPlanningRunExecutor(); + $bus = new FakeMessageBusForPlanningRunExecutor(); $executor = new PlanningRunExecutor( $productConfig, @@ -43,6 +46,7 @@ $llm, new NullLogger(), new MessengerLogContext(), + $bus, ); $executor->execute(new PlanIssueMessage('cfg-1', 77, false, 'run-1')); @@ -58,6 +62,7 @@ $workspace = new FakeWorkspaceManagementFacadeForPlanningRunExecutor(); $concurrency = new FakeWorkflowConcurrencyFacadeForPlanningRunExecutor(); $llm = new FakeLlmIntegrationFacadeForPlanningRunExecutor(); + $bus = new FakeMessageBusForPlanningRunExecutor(); $executor = new PlanningRunExecutor( $productConfig, @@ -67,6 +72,7 @@ $llm, new NullLogger(), new MessengerLogContext(), + $bus, ); $executor->execute(new PlanIssueMessage('cfg-404', 77, false, 'run-1')); @@ -83,6 +89,7 @@ $workspace = new FakeWorkspaceManagementFacadeForPlanningRunExecutor(); $concurrency = new FakeWorkflowConcurrencyFacadeForPlanningRunExecutor(); $llm = new FakeLlmIntegrationFacadeForPlanningRunExecutor(); + $bus = new FakeMessageBusForPlanningRunExecutor(); $executor = new PlanningRunExecutor( $productConfig, @@ -92,6 +99,7 @@ $llm, new NullLogger(), new MessengerLogContext(), + $bus, ); $executor->execute(new PlanIssueMessage('cfg-1', 88, false, 'run-2')); @@ -143,7 +151,7 @@ expect($state->concurrency->releasedClaims)->toHaveCount(1); }); - it('applies error labels and error comment for error outcome', function (): void { + it('applies error labels and error comment for error outcome after max retries', function (): void { $state = createPlanningRunExecutorHappyPathState(); $state->llm->result = new AgentRunResultDto( false, @@ -152,7 +160,7 @@ 1500, ); - $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-5')); + $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-5', null, null, 2)); expect($state->github->postedComments)->toHaveCount(1); expect($state->github->postedComments[0]['body'])->toContain('Planning Error'); @@ -161,19 +169,21 @@ expect($state->github->addedLabels)->toContain(GithubLabel::PlanningErrored->value); expect($state->workspace->releaseCalls)->toBe(1); expect($state->concurrency->releasedClaims)->toHaveCount(1); + expect($state->bus->dispatchedMessages)->toHaveCount(0); }); - it('handles unhandled exception by applying error labels, posting comment, and releasing resources', function (): void { + it('handles unhandled exception by applying error labels, posting comment, and releasing resources after max retries', function (): void { $state = createPlanningRunExecutorHappyPathState(); $state->llm->throwOnRun = true; - $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-6')); + $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-6', null, null, 2)); expect($state->github->postedComments)->toHaveCount(1); expect($state->github->postedComments[0]['body'])->toContain('unexpected error occurred'); expect($state->github->addedLabels)->toContain(GithubLabel::PlanningErrored->value); expect($state->workspace->releaseCalls)->toBe(1); expect($state->concurrency->releasedClaims)->toHaveCount(1); + expect($state->bus->dispatchedMessages)->toHaveCount(0); }); it('releases run claim in finally after successful run', function (): void { @@ -217,6 +227,83 @@ expect($state->llm->lastAgentProviderOverride)->toBe('claude-code-cli'); }); + + it('dispatches retry message on error outcome with retryCount=0, no error labels applied', function (): void { + $state = createPlanningRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + + $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-r0')); + + expect($state->github->addedLabels)->not->toContain(GithubLabel::PlanningErrored->value); + expect($state->github->postedComments)->toHaveCount(0); + expect($state->concurrency->tryCreateRunClaimCalls)->toBe(1); + expect($state->bus->dispatchedMessages)->toHaveCount(1); + + $dispatched = $state->bus->dispatchedMessages[0]; + expect($dispatched)->toBeInstanceOf(PlanIssueMessage::class); + expect($dispatched->retryCount)->toBe(1); + expect($dispatched->productConfigId)->toBe('cfg-1'); + expect($dispatched->issueNumber)->toBe(99); + }); + + it('dispatches retry message on error outcome with retryCount=1, no error labels applied', function (): void { + $state = createPlanningRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + + $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-r1', null, null, 1)); + + expect($state->github->addedLabels)->not->toContain(GithubLabel::PlanningErrored->value); + expect($state->github->postedComments)->toHaveCount(0); + expect($state->bus->dispatchedMessages)->toHaveCount(1); + expect($state->bus->dispatchedMessages[0]->retryCount)->toBe(2); + }); + + it('applies error labels on error outcome with retryCount=2 (max retries exhausted)', function (): void { + $state = createPlanningRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + + $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-r2', null, null, 2)); + + expect($state->github->addedLabels)->toContain(GithubLabel::PlanningErrored->value); + expect($state->github->postedComments)->toHaveCount(1); + expect($state->github->postedComments[0]['body'])->toContain('Planning Error'); + expect($state->bus->dispatchedMessages)->toHaveCount(0); + }); + + it('dispatches retry message on throwable with retryCount=0, no error labels applied', function (): void { + $state = createPlanningRunExecutorHappyPathState(); + $state->llm->throwOnRun = true; + + $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-ex0')); + + expect($state->github->addedLabels)->not->toContain(GithubLabel::PlanningErrored->value); + expect($state->github->postedComments)->toHaveCount(0); + expect($state->bus->dispatchedMessages)->toHaveCount(1); + expect($state->bus->dispatchedMessages[0]->retryCount)->toBe(1); + }); + + it('applies error labels on throwable with retryCount=2 (max retries exhausted)', function (): void { + $state = createPlanningRunExecutorHappyPathState(); + $state->llm->throwOnRun = true; + + $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-ex2', null, null, 2)); + + expect($state->github->addedLabels)->toContain(GithubLabel::PlanningErrored->value); + expect($state->github->postedComments)->toHaveCount(1); + expect($state->bus->dispatchedMessages)->toHaveCount(0); + }); + + it('falls back to final error handling when tryCreateRunClaim fails during retry', function (): void { + $state = createPlanningRunExecutorHappyPathState(); + $state->llm->result = new AgentRunResultDto(false, 'agent failed', 'session-1', 1500); + $state->concurrency->tryCreateRunClaimResult = false; + + $state->executor->execute(new PlanIssueMessage('cfg-1', 99, false, 'run-claim-fail')); + + expect($state->github->addedLabels)->toContain(GithubLabel::PlanningErrored->value); + expect($state->github->postedComments)->toHaveCount(1); + expect($state->bus->dispatchedMessages)->toHaveCount(0); + }); }); function createPlanningRunExecutorHappyPathState(): PlanningRunExecutorHappyPathState @@ -240,6 +327,7 @@ function createPlanningRunExecutorHappyPathState(): PlanningRunExecutorHappyPath $workspace = new FakeWorkspaceManagementFacadeForPlanningRunExecutor(); $concurrency = new FakeWorkflowConcurrencyFacadeForPlanningRunExecutor(); $llm = new FakeLlmIntegrationFacadeForPlanningRunExecutor(); + $bus = new FakeMessageBusForPlanningRunExecutor(); $executor = new PlanningRunExecutor( $productConfig, @@ -249,9 +337,10 @@ function createPlanningRunExecutorHappyPathState(): PlanningRunExecutorHappyPath $llm, new NullLogger(), new MessengerLogContext(), + $bus, ); - return new PlanningRunExecutorHappyPathState($executor, $github, $workspace, $concurrency, $llm); + return new PlanningRunExecutorHappyPathState($executor, $github, $workspace, $concurrency, $llm, $bus); } final readonly class PlanningRunExecutorHappyPathState @@ -262,6 +351,7 @@ public function __construct( public FakeWorkspaceManagementFacadeForPlanningRunExecutor $workspace, public FakeWorkflowConcurrencyFacadeForPlanningRunExecutor $concurrency, public FakeLlmIntegrationFacadeForPlanningRunExecutor $llm, + public FakeMessageBusForPlanningRunExecutor $bus, ) { } } @@ -465,14 +555,18 @@ public function createWorkspaceOnBranch(string $githubUrl, string $githubToken, final class FakeWorkflowConcurrencyFacadeForPlanningRunExecutor implements WorkflowConcurrencyFacadeInterface { - public bool $tryStartRunResult = true; - public int $tryStartCalls = 0; + public bool $tryStartRunResult = true; + public bool $tryCreateRunClaimResult = true; + public int $tryStartCalls = 0; + public int $tryCreateRunClaimCalls = 0; /** @var list */ public array $releasedClaims = []; public function tryCreateRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId): bool { - return true; + ++$this->tryCreateRunClaimCalls; + + return $this->tryCreateRunClaimResult; } public function tryStartRun(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId): bool @@ -534,3 +628,19 @@ public function runAgent( return $this->result; } } + +final class FakeMessageBusForPlanningRunExecutor implements MessageBusInterface +{ + /** @var list */ + public array $dispatchedMessages = []; + + /** + * @param list $stamps + */ + public function dispatch(object $message, array $stamps = []): Envelope + { + $this->dispatchedMessages[] = $message; + + return new Envelope($message, $stamps); + } +}