diff --git a/config/services.yaml b/config/services.yaml index 9266d27..31c89f9 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -27,5 +27,15 @@ services: arguments: - "%env(resolve:DATABASE_PRODUCT)%://%env(resolve:DATABASE_USER)%:%env(resolve:DATABASE_PASSWORD)%@%env(resolve:DATABASE_HOST)%:%env(resolve:DATABASE_PORT)%/%env(resolve:DATABASE_DB)%?serverVersion=%env(resolve:DATABASE_SERVERVERSION)%" + # LlmIntegration: bind interface to implementation per consumer (two preparators exist) + App\LlmIntegration\Infrastructure\Service\Cursor\CursorCliBinaryManager: + arguments: + $configWriter: '@App\LlmIntegration\Infrastructure\Service\Cursor\CursorCliConfigWriter' + $runtimePreparator: '@App\LlmIntegration\Infrastructure\Service\Cursor\CursorWorkspaceRuntimePreparator' + + App\LlmIntegration\Infrastructure\Service\ClaudeCode\ClaudeCodeBinaryManager: + arguments: + $runtimePreparator: '@App\LlmIntegration\Infrastructure\Service\ClaudeCode\ClaudeCodeWorkspaceRuntimePreparator' + # add more service definitions when explicit configuration is needed # please note that last definitions always *replace* previous ones diff --git a/src/ImplementationAgent/Infrastructure/Service/ImplementationRunExecutor.php b/src/ImplementationAgent/Infrastructure/Service/ImplementationRunExecutor.php index e911752..c347eeb 100644 --- a/src/ImplementationAgent/Infrastructure/Service/ImplementationRunExecutor.php +++ b/src/ImplementationAgent/Infrastructure/Service/ImplementationRunExecutor.php @@ -96,126 +96,117 @@ public function execute(ImplementIssueMessage $message): void private function doExecute(ImplementIssueMessage $message, ProductConfigDto $config): void { - $runStarted = $this->workflowConcurrencyFacade->tryStartRun( + $didRun = $this->workflowConcurrencyFacade->executeWithRunClaim( $message->productConfigId, $message->issueNumber, WorkflowRunPhase::Implementation, $message->runId, - ); - - if (!$runStarted) { - $this->logger->info('[ImplementationAgent] Skipping stale or duplicate implementation message', [ - 'productConfigId' => $message->productConfigId, - 'issueNumber' => $message->issueNumber, - 'runId' => $message->runId, - ]); - - return; - } + function () use ($message, $config): void { + $issueDto = $this->findIssue($config->githubUrl, $config->githubToken, $message->issueNumber); + if ($issueDto === null) { + $this->logger->error('[ImplementationAgent] Could not fetch issue data', [ + 'issueNumber' => $message->issueNumber, + ]); + + return; + } - try { - $issueDto = $this->findIssue($config->githubUrl, $config->githubToken, $message->issueNumber); - if ($issueDto === null) { - $this->logger->error('[ImplementationAgent] Could not fetch issue data', [ - 'issueNumber' => $message->issueNumber, - ]); + $issueComments = $this->githubIntegrationFacade->getIssueComments( + $config->githubUrl, + $config->githubToken, + $message->issueNumber, + ); - return; - } + $workspaceInfo = null; + + try { + if ($message->isRevision && $message->prBranchName !== null) { + $workspaceInfo = $this->workspaceManagementFacade->acquireWorkspace( + $config->id, + $message->issueNumber, + $config->githubUrl, + $config->githubToken, + $message->prBranchName, + ); + + $prComments = $message->prNumber !== null + ? $this->githubIntegrationFacade->getPullRequestComments($config->githubUrl, $config->githubToken, $message->prNumber) + : []; + + $prompt = $this->buildRevisionPrompt($issueDto, $issueComments, $prComments, $message->prNumber); + } else { + $workspaceInfo = $this->workspaceManagementFacade->acquireWorkspace( + $config->id, + $message->issueNumber, + $config->githubUrl, + $config->githubToken, + ); + + $prompt = $this->buildPrompt($issueDto, $issueComments); + } + + $agentApiKeyOverride = $message->agentApiKeyOverride; + $credentials = $agentApiKeyOverride !== null + ? new AgentCredentialsDto($agentApiKeyOverride, $agentApiKeyOverride) + : new AgentCredentialsDto($config->cursorAgentApiKey, $config->anthropicApiKey); + + $agentResult = $this->llmIntegrationFacade->runAgent( + AgentRole::Implementation, + $prompt, + $workspaceInfo->workspacePath, + $credentials, + $config->githubToken, + $workspaceInfo->containerName, + $message->agentProviderOverride, + ); - $issueComments = $this->githubIntegrationFacade->getIssueComments( - $config->githubUrl, - $config->githubToken, - $message->issueNumber, - ); + $outcome = ImplementationOutcome::fromAgentOutput($agentResult->success, $agentResult->resultText); - $workspaceInfo = null; + $this->logger->info('[ImplementationAgent] Agent completed', [ + 'issueNumber' => $message->issueNumber, + 'outcome' => $outcome->value, + 'isRevision' => $message->isRevision, + 'durationMs' => $agentResult->durationMs, + ]); - try { - if ($message->isRevision && $message->prBranchName !== null) { - $workspaceInfo = $this->workspaceManagementFacade->acquireWorkspace( - $config->id, - $message->issueNumber, + $this->handleOutcome( + $outcome, + $agentResult->resultText, $config->githubUrl, $config->githubToken, - $message->prBranchName, + $message->issueNumber, + $issueDto, + $message->prNumber, ); + } catch (Throwable $e) { + $this->logger->error('[ImplementationAgent] Unhandled exception', [ + 'issueNumber' => $message->issueNumber, + 'isRevision' => $message->isRevision, + 'error' => $e->getMessage(), + ]); - $prComments = $message->prNumber !== null - ? $this->githubIntegrationFacade->getPullRequestComments($config->githubUrl, $config->githubToken, $message->prNumber) - : []; + $this->applyErrorLabels($config->githubUrl, $config->githubToken, $message->issueNumber); - $prompt = $this->buildRevisionPrompt($issueDto, $issueComments, $prComments, $message->prNumber); - } else { - $workspaceInfo = $this->workspaceManagementFacade->acquireWorkspace( - $config->id, - $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(), ); - - $prompt = $this->buildPrompt($issueDto, $issueComments); + } finally { + if ($workspaceInfo !== null) { + $this->workspaceManagementFacade->releaseWorkspace($workspaceInfo); + } } + }, + ); - $agentApiKeyOverride = $message->agentApiKeyOverride; - $credentials = $agentApiKeyOverride !== null - ? new AgentCredentialsDto($agentApiKeyOverride, $agentApiKeyOverride) - : new AgentCredentialsDto($config->cursorAgentApiKey, $config->anthropicApiKey); - - $agentResult = $this->llmIntegrationFacade->runAgent( - AgentRole::Implementation, - $prompt, - $workspaceInfo->workspacePath, - $credentials, - $config->githubToken, - $workspaceInfo->containerName, - $message->agentProviderOverride, - ); - - $outcome = ImplementationOutcome::fromAgentOutput($agentResult->success, $agentResult->resultText); - - $this->logger->info('[ImplementationAgent] Agent completed', [ - 'issueNumber' => $message->issueNumber, - 'outcome' => $outcome->value, - 'isRevision' => $message->isRevision, - 'durationMs' => $agentResult->durationMs, - ]); - - $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, - 'isRevision' => $message->isRevision, - '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(), - ); - } finally { - if ($workspaceInfo !== null) { - $this->workspaceManagementFacade->releaseWorkspace($workspaceInfo); - } - } - } finally { - $this->workflowConcurrencyFacade->releaseRunClaim( - $message->productConfigId, - $message->issueNumber, - $message->runId, - ); + if (!$didRun) { + $this->logger->info('[ImplementationAgent] Skipping stale or duplicate implementation message', [ + 'productConfigId' => $message->productConfigId, + 'issueNumber' => $message->issueNumber, + 'runId' => $message->runId, + ]); } } diff --git a/src/LlmIntegration/Infrastructure/Service/AgentStreamParser.php b/src/LlmIntegration/Infrastructure/Service/AgentStreamParser.php index 648256f..8f56c6d 100644 --- a/src/LlmIntegration/Infrastructure/Service/AgentStreamParser.php +++ b/src/LlmIntegration/Infrastructure/Service/AgentStreamParser.php @@ -10,18 +10,19 @@ use Symfony\Component\Process\Process; /** - * Parses NDJSON stream output shared by Cursor CLI and Claude Code CLI agents. + * Orchestrates process execution and NDJSON stream handling: runs the process, + * parses lines via NdjsonStreamLineParser, and logs events / assembles CliAgentRunResult. * - * Both CLIs emit identical event types: system (init), assistant, tool_call - * (started/completed), and result. This service handles all stream parsing - * and logging for any CLI agent provider. + * Both Cursor CLI and Claude Code CLI emit identical event types: system (init), + * assistant, tool_call (started/completed), and result. */ class AgentStreamParser { private const float ASSISTANT_MAX_FLUSH_DELAY_SECONDS = 0.6; public function __construct( - private readonly LoggerInterface $logger, + private readonly NdjsonStreamLineParser $lineParser, + private readonly LoggerInterface $logger, ) { } @@ -144,19 +145,28 @@ public function executeAndParseStream(Process $process, string $providerLabel): } /** - * Parses a single NDJSON line, logs a human-readable summary, and returns the decoded data. + * Parses a single NDJSON line (via line parser), logs a human-readable summary, and returns the decoded data. * * @return array|null */ private function logStreamEvent(string $jsonLine, string $providerLabel): ?array { + $parsed = $this->lineParser->parseLine($jsonLine); + if ($parsed === null) { + return null; + } + try { - /** @var array $event */ - $event = json_decode($jsonLine, true, 512, JSON_THROW_ON_ERROR); + /** @var array|null $event */ + $event = json_decode($parsed->jsonLine, true, 512, JSON_THROW_ON_ERROR); } catch (JsonException) { return null; } + if (!is_array($event)) { + return null; + } + $type = is_string($event['type'] ?? null) ? $event['type'] : ''; $subtype = is_string($event['subtype'] ?? null) ? $event['subtype'] : ''; @@ -481,18 +491,30 @@ private function shouldFlushAssistantBufferByBoundary(string $assistantLogBuffer private function parseResultLine(string $jsonLine, string $providerLabel): CliAgentRunResult { + $parsed = $this->lineParser->parseLine($jsonLine); + if ($parsed === null) { + $this->logger->warning('[' . $providerLabel . '] Could not parse result JSON line', [ + 'preview' => mb_substr($jsonLine, 0, 500), + ]); + + return new CliAgentRunResult(true, $jsonLine, null, null); + } + try { - /** @var array $data */ - $data = json_decode($jsonLine, true, 512, JSON_THROW_ON_ERROR); - } catch (JsonException $e) { + /** @var array|null $data */ + $data = json_decode($parsed->jsonLine, true, 512, JSON_THROW_ON_ERROR); + } catch (JsonException) { $this->logger->warning('[' . $providerLabel . '] Could not parse result JSON line', [ - 'error' => $e->getMessage(), 'preview' => mb_substr($jsonLine, 0, 500), ]); return new CliAgentRunResult(true, $jsonLine, null, null); } + if (!is_array($data)) { + return new CliAgentRunResult(true, $jsonLine, null, null); + } + $isError = (bool) ($data['is_error'] ?? false); $resultText = is_string($data['result'] ?? null) ? $data['result'] : $jsonLine; $sessionId = is_string($data['session_id'] ?? null) ? $data['session_id'] : null; diff --git a/src/LlmIntegration/Infrastructure/Service/ClaudeCode/ClaudeCodeBinaryManager.php b/src/LlmIntegration/Infrastructure/Service/ClaudeCode/ClaudeCodeBinaryManager.php index 063e13e..f54f4d6 100644 --- a/src/LlmIntegration/Infrastructure/Service/ClaudeCode/ClaudeCodeBinaryManager.php +++ b/src/LlmIntegration/Infrastructure/Service/ClaudeCode/ClaudeCodeBinaryManager.php @@ -4,9 +4,9 @@ namespace App\LlmIntegration\Infrastructure\Service\ClaudeCode; +use App\LlmIntegration\Infrastructure\Service\WorkspaceRuntimePreparatorInterface; use Psr\Log\LoggerInterface; use RuntimeException; -use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\Process\Process; class ClaudeCodeBinaryManager @@ -25,7 +25,8 @@ class ClaudeCodeBinaryManager private ?string $resolvedPath = null; public function __construct( - private readonly LoggerInterface $logger, + private readonly WorkspaceRuntimePreparatorInterface $runtimePreparator, + private readonly LoggerInterface $logger, ) { } @@ -67,71 +68,7 @@ public function findOrInstall(): string public function prepareWorkspaceRuntime(string $workspacePath): void { $binaryPath = $this->findOrInstall(); - $sourceDir = $this->resolveSourceDirectory($binaryPath); - - $runtimeDir = $workspacePath . '/' . self::RUNTIME_DIR; - $tmpRuntimeDir = $runtimeDir . '.tmp-' . getmypid() . '-' . bin2hex(random_bytes(4)); - $binDir = $tmpRuntimeDir . '/bin'; - - $filesystem = new Filesystem(); - - if (file_exists($tmpRuntimeDir)) { - $filesystem->remove($tmpRuntimeDir); - } - - $filesystem->mkdir([$binDir], 0o755); - - $this->mirrorDirectoryContents($sourceDir, $binDir); - $this->copyNodeBinaryIfAvailable(dirname($binaryPath), $binDir); - - $claudeSymlink = $binDir . '/claude'; - if (!is_file($claudeSymlink) && !is_link($claudeSymlink)) { - $realBinaryName = basename((string) realpath($binaryPath)); - if ($realBinaryName !== 'claude') { - symlink($realBinaryName, $claudeSymlink); - } - } - - $backupRuntimeDir = null; - if (file_exists($runtimeDir) && !is_dir($runtimeDir)) { - $filesystem->remove($runtimeDir); - } - if (is_dir($runtimeDir)) { - $backupRuntimeDir = $runtimeDir . '.old-' . gmdate('YmdHis') . '-' . bin2hex(random_bytes(2)); - - if (!rename($runtimeDir, $backupRuntimeDir)) { - $filesystem->remove($tmpRuntimeDir); - - throw new RuntimeException(sprintf( - 'Failed to move existing workspace runtime out of the way ("%s" -> "%s")', - $runtimeDir, - $backupRuntimeDir, - )); - } - } - - if (!rename($tmpRuntimeDir, $runtimeDir)) { - if ($backupRuntimeDir !== null && is_dir($backupRuntimeDir)) { - rename($backupRuntimeDir, $runtimeDir); - } - - $filesystem->remove($tmpRuntimeDir); - - throw new RuntimeException(sprintf( - 'Failed to move prepared workspace runtime into place ("%s" -> "%s")', - $tmpRuntimeDir, - $runtimeDir, - )); - } - - if ($backupRuntimeDir !== null && is_dir($backupRuntimeDir)) { - $filesystem->remove($backupRuntimeDir); - } - - $this->logger->info('[ClaudeCodeBinary] Workspace runtime prepared', [ - 'runtimeDir' => $runtimeDir, - 'sourceDir' => $sourceDir, - ]); + $this->runtimePreparator->prepare($workspacePath, $binaryPath); } public static function getWorkspaceAgentBinaryPath(string $workspacePath): string @@ -222,154 +159,6 @@ private function installBinary(): void ]); } - private function resolveSourceDirectory(string $binaryPath): string - { - $resolved = realpath($binaryPath); - - if ($resolved === false) { - throw new RuntimeException(sprintf( - 'Could not resolve real path of Claude Code binary "%s"', - $binaryPath, - )); - } - - return dirname($resolved); - } - - private function mirrorDirectoryContents(string $sourceDir, string $targetDir): void - { - $entries = scandir($sourceDir); - - if ($entries === false) { - throw new RuntimeException(sprintf( - 'Could not scan Claude Code source directory "%s"', - $sourceDir, - )); - } - - $copiedCount = 0; - - foreach ($entries as $entry) { - if ($entry === '.' || $entry === '..') { - continue; - } - - $src = $sourceDir . '/' . $entry; - $dst = $targetDir . '/' . $entry; - - $copiedCount += $this->copyPathRecursive($src, $dst); - } - - $this->logger->info('[ClaudeCodeBinary] Copied runtime files', [ - 'sourceDir' => $sourceDir, - 'targetDir' => $targetDir, - 'fileCount' => $copiedCount, - ]); - } - - private function copyPathRecursive(string $sourcePath, string $targetPath): int - { - if (is_link($sourcePath)) { - $linkTarget = readlink($sourcePath); - if (!is_string($linkTarget)) { - throw new RuntimeException(sprintf('Failed to read symlink target: "%s"', $sourcePath)); - } - - if (file_exists($targetPath) || is_link($targetPath)) { - $filesystem = new Filesystem(); - $filesystem->remove($targetPath); - } - - symlink($linkTarget, $targetPath); - - return 1; - } - - if (is_dir($sourcePath)) { - if (!is_dir($targetPath)) { - mkdir($targetPath, 0o755, true); - } - - $entries = scandir($sourcePath); - if ($entries === false) { - throw new RuntimeException(sprintf( - 'Could not scan directory "%s"', - $sourcePath, - )); - } - - $count = 0; - foreach ($entries as $entry) { - if ($entry === '.' || $entry === '..') { - continue; - } - - $count += $this->copyPathRecursive( - $sourcePath . '/' . $entry, - $targetPath . '/' . $entry, - ); - } - - return $count; - } - - if (!is_file($sourcePath)) { - return 0; - } - - if (!copy($sourcePath, $targetPath)) { - throw new RuntimeException(sprintf( - 'Failed to copy "%s" to "%s"', - $sourcePath, - $targetPath, - )); - } - - if (is_executable($sourcePath)) { - chmod($targetPath, 0o755); - } - - return 1; - } - - /** - * Copies the `node` binary from the same bin directory as the `claude` - * wrapper into the workspace runtime so the shebang can resolve inside - * the project container. - */ - private function copyNodeBinaryIfAvailable(string $claudeBinDir, string $targetBinDir): void - { - $nodeSrc = $claudeBinDir . '/node'; - - if (!is_file($nodeSrc)) { - $this->logger->warning('[ClaudeCodeBinary] node binary not found next to claude, container execution may fail', [ - 'expected' => $nodeSrc, - ]); - - return; - } - - $nodeDst = $targetBinDir . '/node'; - if (is_file($nodeDst)) { - return; - } - - if (!copy($nodeSrc, $nodeDst)) { - throw new RuntimeException(sprintf( - 'Failed to copy node binary "%s" to "%s"', - $nodeSrc, - $nodeDst, - )); - } - - chmod($nodeDst, 0o755); - - $this->logger->info('[ClaudeCodeBinary] Copied node binary into workspace runtime', [ - 'source' => $nodeSrc, - 'target' => $nodeDst, - ]); - } - private function getHome(): string { $home = getenv('HOME'); diff --git a/src/LlmIntegration/Infrastructure/Service/ClaudeCode/ClaudeCodeWorkspaceRuntimePreparator.php b/src/LlmIntegration/Infrastructure/Service/ClaudeCode/ClaudeCodeWorkspaceRuntimePreparator.php new file mode 100644 index 0000000..881f4a8 --- /dev/null +++ b/src/LlmIntegration/Infrastructure/Service/ClaudeCode/ClaudeCodeWorkspaceRuntimePreparator.php @@ -0,0 +1,232 @@ +resolveSourceDirectory($agentBinaryPath); + $claudeBinDir = dirname($agentBinaryPath); + $runtimeDir = $workspacePath . '/' . self::RUNTIME_DIR; + $tmpRuntimeDir = $runtimeDir . '.tmp-' . getmypid() . '-' . bin2hex(random_bytes(4)); + $binDir = $tmpRuntimeDir . '/bin'; + $filesystem = new Filesystem(); + + if (file_exists($tmpRuntimeDir)) { + $filesystem->remove($tmpRuntimeDir); + } + + $filesystem->mkdir([$binDir], 0o755); + + $this->mirrorDirectoryContents($sourceDir, $binDir); + $this->copyNodeBinaryIfAvailable($claudeBinDir, $binDir); + + $claudeSymlink = $binDir . '/claude'; + if (!is_file($claudeSymlink) && !is_link($claudeSymlink)) { + $realBinaryName = basename((string) realpath($agentBinaryPath)); + if ($realBinaryName !== 'claude') { + symlink($realBinaryName, $claudeSymlink); + } + } + + $backupRuntimeDir = null; + if (file_exists($runtimeDir) && !is_dir($runtimeDir)) { + $filesystem->remove($runtimeDir); + } + if (is_dir($runtimeDir)) { + $backupRuntimeDir = $runtimeDir . '.old-' . gmdate('YmdHis') . '-' . bin2hex(random_bytes(2)); + + if (!rename($runtimeDir, $backupRuntimeDir)) { + $filesystem->remove($tmpRuntimeDir); + + throw new RuntimeException(sprintf( + 'Failed to move existing workspace runtime out of the way ("%s" -> "%s")', + $runtimeDir, + $backupRuntimeDir, + )); + } + } + + if (!rename($tmpRuntimeDir, $runtimeDir)) { + if ($backupRuntimeDir !== null && is_dir($backupRuntimeDir)) { + rename($backupRuntimeDir, $runtimeDir); + } + + $filesystem->remove($tmpRuntimeDir); + + throw new RuntimeException(sprintf( + 'Failed to move prepared workspace runtime into place ("%s" -> "%s")', + $tmpRuntimeDir, + $runtimeDir, + )); + } + + if ($backupRuntimeDir !== null && is_dir($backupRuntimeDir)) { + $filesystem->remove($backupRuntimeDir); + } + + $this->logger->info('[ClaudeCodeBinary] Workspace runtime prepared', [ + 'runtimeDir' => $runtimeDir, + 'sourceDir' => $sourceDir, + ]); + } + + private function resolveSourceDirectory(string $binaryPath): string + { + $resolved = realpath($binaryPath); + + if ($resolved === false) { + throw new RuntimeException(sprintf( + 'Could not resolve real path of Claude Code binary "%s"', + $binaryPath, + )); + } + + return dirname($resolved); + } + + private function mirrorDirectoryContents(string $sourceDir, string $targetDir): void + { + $entries = scandir($sourceDir); + + if ($entries === false) { + throw new RuntimeException(sprintf( + 'Could not scan Claude Code source directory "%s"', + $sourceDir, + )); + } + + $copiedCount = 0; + $filesystem = new Filesystem(); + + foreach ($entries as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + + $src = $sourceDir . '/' . $entry; + $dst = $targetDir . '/' . $entry; + + $copiedCount += $this->copyPathRecursive($src, $dst, $filesystem); + } + + $this->logger->info('[ClaudeCodeBinary] Copied runtime files', [ + 'sourceDir' => $sourceDir, + 'targetDir' => $targetDir, + 'fileCount' => $copiedCount, + ]); + } + + private function copyPathRecursive(string $sourcePath, string $targetPath, Filesystem $filesystem): int + { + if (is_link($sourcePath)) { + $linkTarget = readlink($sourcePath); + if (!is_string($linkTarget)) { + throw new RuntimeException(sprintf('Failed to read symlink target: "%s"', $sourcePath)); + } + + if (file_exists($targetPath) || is_link($targetPath)) { + $filesystem->remove($targetPath); + } + + symlink($linkTarget, $targetPath); + + return 1; + } + + if (is_dir($sourcePath)) { + if (!is_dir($targetPath)) { + mkdir($targetPath, 0o755, true); + } + + $entries = scandir($sourcePath); + if ($entries === false) { + throw new RuntimeException(sprintf( + 'Could not scan directory "%s"', + $sourcePath, + )); + } + + $count = 0; + foreach ($entries as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + + $count += $this->copyPathRecursive( + $sourcePath . '/' . $entry, + $targetPath . '/' . $entry, + $filesystem, + ); + } + + return $count; + } + + if (!is_file($sourcePath)) { + return 0; + } + + if (!copy($sourcePath, $targetPath)) { + throw new RuntimeException(sprintf( + 'Failed to copy "%s" to "%s"', + $sourcePath, + $targetPath, + )); + } + + if (is_executable($sourcePath)) { + chmod($targetPath, 0o755); + } + + return 1; + } + + private function copyNodeBinaryIfAvailable(string $claudeBinDir, string $targetBinDir): void + { + $nodeSrc = $claudeBinDir . '/node'; + + if (!is_file($nodeSrc)) { + $this->logger->warning('[ClaudeCodeBinary] node binary not found next to claude, container execution may fail', [ + 'expected' => $nodeSrc, + ]); + + return; + } + + $nodeDst = $targetBinDir . '/node'; + if (is_file($nodeDst)) { + return; + } + + if (!copy($nodeSrc, $nodeDst)) { + throw new RuntimeException(sprintf( + 'Failed to copy node binary "%s" to "%s"', + $nodeSrc, + $nodeDst, + )); + } + + chmod($nodeDst, 0o755); + + $this->logger->info('[ClaudeCodeBinary] Copied node binary into workspace runtime', [ + 'source' => $nodeSrc, + 'target' => $nodeDst, + ]); + } +} diff --git a/src/LlmIntegration/Infrastructure/Service/CliConfigWriterInterface.php b/src/LlmIntegration/Infrastructure/Service/CliConfigWriterInterface.php new file mode 100644 index 0000000..b2dad00 --- /dev/null +++ b/src/LlmIntegration/Infrastructure/Service/CliConfigWriterInterface.php @@ -0,0 +1,10 @@ +writeCliConfigTo($configPath); + $this->configWriter->writeTo($configPath); } public function writeCliConfigTo(string $configPath): void { - $config = [ - 'version' => 1, - 'editor' => ['vimMode' => false], - 'permissions' => [ - 'allow' => [ - 'Shell(*)', - 'Read(**)', - 'Write(**)', - 'WebFetch(*)', - 'Mcp(*:*)', - ], - 'deny' => [], - ], - 'attribution' => [ - 'attributeCommitsToAgent' => false, - 'attributePRsToAgent' => false, - ], - ]; - - file_put_contents( - $configPath, - json_encode($config, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES) . "\n", - ); - - $this->logger->info('[CursorBinary] Wrote CLI config', ['path' => $configPath]); + $this->configWriter->writeTo($configPath); } /** @@ -109,72 +88,7 @@ public function writeCliConfigTo(string $configPath): void public function prepareWorkspaceRuntime(string $workspacePath): void { $agentBinaryPath = $this->findOrInstall(); - $sourceDir = $this->resolveAgentSourceDirectory($agentBinaryPath); - - $runtimeDir = $workspacePath . '/' . self::RUNTIME_DIR; - $tmpRuntimeDir = $runtimeDir . '.tmp-' . getmypid() . '-' . bin2hex(random_bytes(4)); - - $binDir = $tmpRuntimeDir . '/bin'; - $cursorDir = $tmpRuntimeDir . '/.cursor'; - - $filesystem = new Filesystem(); - - if (file_exists($tmpRuntimeDir)) { - $filesystem->remove($tmpRuntimeDir); - } - - $filesystem->mkdir([$binDir, $cursorDir], 0o755); - - $this->mirrorDirectoryContents($sourceDir, $binDir); - - $agentSymlink = $binDir . '/agent'; - if (is_link($agentSymlink) || is_file($agentSymlink)) { - unlink($agentSymlink); - } - symlink('cursor-agent', $agentSymlink); - - $this->writeCliConfigTo($cursorDir . '/cli-config.json'); - - $backupRuntimeDir = null; - if (file_exists($runtimeDir) && !is_dir($runtimeDir)) { - $filesystem->remove($runtimeDir); - } - if (is_dir($runtimeDir)) { - $backupRuntimeDir = $runtimeDir . '.old-' . gmdate('YmdHis') . '-' . bin2hex(random_bytes(2)); - - if (!rename($runtimeDir, $backupRuntimeDir)) { - $filesystem->remove($tmpRuntimeDir); - - throw new RuntimeException(sprintf( - 'Failed to move existing workspace runtime out of the way ("%s" -> "%s")', - $runtimeDir, - $backupRuntimeDir, - )); - } - } - - if (!rename($tmpRuntimeDir, $runtimeDir)) { - if ($backupRuntimeDir !== null && is_dir($backupRuntimeDir)) { - rename($backupRuntimeDir, $runtimeDir); - } - - $filesystem->remove($tmpRuntimeDir); - - throw new RuntimeException(sprintf( - 'Failed to move prepared workspace runtime into place ("%s" -> "%s")', - $tmpRuntimeDir, - $runtimeDir, - )); - } - - if ($backupRuntimeDir !== null && is_dir($backupRuntimeDir)) { - $filesystem->remove($backupRuntimeDir); - } - - $this->logger->info('[CursorBinary] Workspace runtime prepared', [ - 'runtimeDir' => $runtimeDir, - 'sourceDir' => $sourceDir, - ]); + $this->runtimePreparator->prepare($workspacePath, $agentBinaryPath); } public static function getWorkspaceAgentBinaryPath(string $workspacePath): string @@ -236,125 +150,6 @@ private function installBinary(): void ]); } - private function resolveAgentSourceDirectory(string $agentBinaryPath): string - { - $resolved = realpath($agentBinaryPath); - - if ($resolved === false) { - throw new RuntimeException(sprintf( - 'Could not resolve real path of agent binary "%s"', - $agentBinaryPath, - )); - } - - $sourceDir = dirname($resolved); - - if (!is_file($sourceDir . '/node') || !is_file($sourceDir . '/index.js')) { - throw new RuntimeException(sprintf( - 'Agent source directory "%s" does not contain expected files (node, index.js)', - $sourceDir, - )); - } - - return $sourceDir; - } - - private function mirrorDirectoryContents(string $sourceDir, string $targetDir): void - { - $entries = scandir($sourceDir); - - if ($entries === false) { - throw new RuntimeException(sprintf( - 'Could not scan agent source directory "%s"', - $sourceDir, - )); - } - - $copiedCount = 0; - - foreach ($entries as $entry) { - if ($entry === '.' || $entry === '..') { - continue; - } - - $src = $sourceDir . '/' . $entry; - $dst = $targetDir . '/' . $entry; - - $copiedCount += $this->copyPathRecursive($src, $dst); - } - - $this->logger->info('[CursorBinary] Copied agent runtime files', [ - 'sourceDir' => $sourceDir, - 'targetDir' => $targetDir, - 'fileCount' => $copiedCount, - ]); - } - - private function copyPathRecursive(string $sourcePath, string $targetPath): int - { - if (is_link($sourcePath)) { - $linkTarget = readlink($sourcePath); - if (!is_string($linkTarget)) { - throw new RuntimeException(sprintf('Failed to read symlink target: "%s"', $sourcePath)); - } - - if (file_exists($targetPath) || is_link($targetPath)) { - $filesystem = new Filesystem(); - $filesystem->remove($targetPath); - } - - symlink($linkTarget, $targetPath); - - return 1; - } - - if (is_dir($sourcePath)) { - if (!is_dir($targetPath)) { - mkdir($targetPath, 0o755, true); - } - - $entries = scandir($sourcePath); - if ($entries === false) { - throw new RuntimeException(sprintf( - 'Could not scan directory "%s"', - $sourcePath, - )); - } - - $count = 0; - foreach ($entries as $entry) { - if ($entry === '.' || $entry === '..') { - continue; - } - - $count += $this->copyPathRecursive( - $sourcePath . '/' . $entry, - $targetPath . '/' . $entry, - ); - } - - return $count; - } - - if (!is_file($sourcePath)) { - return 0; - } - - if (!copy($sourcePath, $targetPath)) { - throw new RuntimeException(sprintf( - 'Failed to copy "%s" to "%s"', - $sourcePath, - $targetPath, - )); - } - - if (is_executable($sourcePath)) { - chmod($targetPath, 0o755); - } - - return 1; - } - private function getHome(): string { $home = getenv('HOME'); diff --git a/src/LlmIntegration/Infrastructure/Service/Cursor/CursorCliConfigWriter.php b/src/LlmIntegration/Infrastructure/Service/Cursor/CursorCliConfigWriter.php new file mode 100644 index 0000000..f722734 --- /dev/null +++ b/src/LlmIntegration/Infrastructure/Service/Cursor/CursorCliConfigWriter.php @@ -0,0 +1,50 @@ + 1, + 'editor' => ['vimMode' => false], + 'permissions' => [ + 'allow' => [ + 'Shell(*)', + 'Read(**)', + 'Write(**)', + 'WebFetch(*)', + 'Mcp(*:*)', + ], + 'deny' => [], + ], + 'attribution' => [ + 'attributeCommitsToAgent' => false, + 'attributePRsToAgent' => false, + ], + ]; + + try { + $json = json_encode($config, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES) . "\n"; + } catch (JsonException $e) { + throw new RuntimeException('Failed to encode Cursor CLI config: ' . $e->getMessage(), 0, $e); + } + + file_put_contents($configPath, $json); + + $this->logger->info('[CursorBinary] Wrote CLI config', ['path' => $configPath]); + } +} diff --git a/src/LlmIntegration/Infrastructure/Service/Cursor/CursorWorkspaceRuntimePreparator.php b/src/LlmIntegration/Infrastructure/Service/Cursor/CursorWorkspaceRuntimePreparator.php new file mode 100644 index 0000000..b28133f --- /dev/null +++ b/src/LlmIntegration/Infrastructure/Service/Cursor/CursorWorkspaceRuntimePreparator.php @@ -0,0 +1,209 @@ +resolveAgentSourceDirectory($agentBinaryPath); + $runtimeDir = $workspacePath . '/' . self::RUNTIME_DIR; + $tmpRuntimeDir = $runtimeDir . '.tmp-' . getmypid() . '-' . bin2hex(random_bytes(4)); + $binDir = $tmpRuntimeDir . '/bin'; + $cursorDir = $tmpRuntimeDir . '/.cursor'; + $filesystem = new Filesystem(); + + if (file_exists($tmpRuntimeDir)) { + $filesystem->remove($tmpRuntimeDir); + } + + $filesystem->mkdir([$binDir, $cursorDir], 0o755); + + $this->mirrorDirectoryContents($sourceDir, $binDir); + + $agentSymlink = $binDir . '/agent'; + if (is_link($agentSymlink) || is_file($agentSymlink)) { + unlink($agentSymlink); + } + symlink('cursor-agent', $agentSymlink); + + $this->configWriter->writeTo($cursorDir . '/cli-config.json'); + + $backupRuntimeDir = null; + if (file_exists($runtimeDir) && !is_dir($runtimeDir)) { + $filesystem->remove($runtimeDir); + } + if (is_dir($runtimeDir)) { + $backupRuntimeDir = $runtimeDir . '.old-' . gmdate('YmdHis') . '-' . bin2hex(random_bytes(2)); + + if (!rename($runtimeDir, $backupRuntimeDir)) { + $filesystem->remove($tmpRuntimeDir); + + throw new RuntimeException(sprintf( + 'Failed to move existing workspace runtime out of the way ("%s" -> "%s")', + $runtimeDir, + $backupRuntimeDir, + )); + } + } + + if (!rename($tmpRuntimeDir, $runtimeDir)) { + if ($backupRuntimeDir !== null && is_dir($backupRuntimeDir)) { + rename($backupRuntimeDir, $runtimeDir); + } + + $filesystem->remove($tmpRuntimeDir); + + throw new RuntimeException(sprintf( + 'Failed to move prepared workspace runtime into place ("%s" -> "%s")', + $tmpRuntimeDir, + $runtimeDir, + )); + } + + if ($backupRuntimeDir !== null && is_dir($backupRuntimeDir)) { + $filesystem->remove($backupRuntimeDir); + } + + $this->logger->info('[CursorBinary] Workspace runtime prepared', [ + 'runtimeDir' => $runtimeDir, + 'sourceDir' => $sourceDir, + ]); + } + + private function resolveAgentSourceDirectory(string $agentBinaryPath): string + { + $resolved = realpath($agentBinaryPath); + + if ($resolved === false) { + throw new RuntimeException(sprintf( + 'Could not resolve real path of agent binary "%s"', + $agentBinaryPath, + )); + } + + $sourceDir = dirname($resolved); + + if (!is_file($sourceDir . '/node') || !is_file($sourceDir . '/index.js')) { + throw new RuntimeException(sprintf( + 'Agent source directory "%s" does not contain expected files (node, index.js)', + $sourceDir, + )); + } + + return $sourceDir; + } + + private function mirrorDirectoryContents(string $sourceDir, string $targetDir): void + { + $entries = scandir($sourceDir); + + if ($entries === false) { + throw new RuntimeException(sprintf( + 'Could not scan agent source directory "%s"', + $sourceDir, + )); + } + + $copiedCount = 0; + $filesystem = new Filesystem(); + + foreach ($entries as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + + $src = $sourceDir . '/' . $entry; + $dst = $targetDir . '/' . $entry; + + $copiedCount += $this->copyPathRecursive($src, $dst, $filesystem); + } + + $this->logger->info('[CursorBinary] Copied agent runtime files', [ + 'sourceDir' => $sourceDir, + 'targetDir' => $targetDir, + 'fileCount' => $copiedCount, + ]); + } + + private function copyPathRecursive(string $sourcePath, string $targetPath, Filesystem $filesystem): int + { + if (is_link($sourcePath)) { + $linkTarget = readlink($sourcePath); + if (!is_string($linkTarget)) { + throw new RuntimeException(sprintf('Failed to read symlink target: "%s"', $sourcePath)); + } + + if (file_exists($targetPath) || is_link($targetPath)) { + $filesystem->remove($targetPath); + } + + symlink($linkTarget, $targetPath); + + return 1; + } + + if (is_dir($sourcePath)) { + if (!is_dir($targetPath)) { + mkdir($targetPath, 0o755, true); + } + + $entries = scandir($sourcePath); + if ($entries === false) { + throw new RuntimeException(sprintf( + 'Could not scan directory "%s"', + $sourcePath, + )); + } + + $count = 0; + foreach ($entries as $entry) { + if ($entry === '.' || $entry === '..') { + continue; + } + + $count += $this->copyPathRecursive( + $sourcePath . '/' . $entry, + $targetPath . '/' . $entry, + $filesystem, + ); + } + + return $count; + } + + if (!is_file($sourcePath)) { + return 0; + } + + if (!copy($sourcePath, $targetPath)) { + throw new RuntimeException(sprintf( + 'Failed to copy "%s" to "%s"', + $sourcePath, + $targetPath, + )); + } + + if (is_executable($sourcePath)) { + chmod($targetPath, 0o755); + } + + return 1; + } +} diff --git a/src/LlmIntegration/Infrastructure/Service/Dto/ParsedStreamEvent.php b/src/LlmIntegration/Infrastructure/Service/Dto/ParsedStreamEvent.php new file mode 100644 index 0000000..72fd697 --- /dev/null +++ b/src/LlmIntegration/Infrastructure/Service/Dto/ParsedStreamEvent.php @@ -0,0 +1,17 @@ +workflowConcurrencyFacade->tryStartRun( + $didRun = $this->workflowConcurrencyFacade->executeWithRunClaim( $message->productConfigId, $message->issueNumber, WorkflowRunPhase::Planning, $message->runId, - ); - - if (!$runStarted) { - $this->logger->info('[PlanningAgent] Skipping stale or duplicate planning message', [ - 'productConfigId' => $message->productConfigId, - 'issueNumber' => $message->issueNumber, - 'runId' => $message->runId, - ]); - - return; - } - - try { - $issueDto = $this->findIssue($config->githubUrl, $config->githubToken, $message->issueNumber); - if ($issueDto === null) { - $this->logger->error('[PlanningAgent] Could not fetch issue data', [ - 'issueNumber' => $message->issueNumber, - ]); - - return; - } - - $comments = $this->githubIntegrationFacade->getIssueComments( - $config->githubUrl, - $config->githubToken, - $message->issueNumber, - ); - - $workspaceInfo = null; - - try { - $workspaceInfo = $this->workspaceManagementFacade->acquireWorkspace( - $config->id, - $message->issueNumber, - $config->githubUrl, - $config->githubToken, - ); - - $prompt = $this->buildPrompt($issueDto, $comments, $message->isResume); - $agentApiKeyOverride = $message->agentApiKeyOverride; - $credentials = $agentApiKeyOverride !== null - ? new AgentCredentialsDto($agentApiKeyOverride, $agentApiKeyOverride) - : new AgentCredentialsDto($config->cursorAgentApiKey, $config->anthropicApiKey); - - $agentResult = $this->llmIntegrationFacade->runAgent( - AgentRole::Planning, - $prompt, - $workspaceInfo->workspacePath, - $credentials, - $config->githubToken, - $workspaceInfo->containerName, - $message->agentProviderOverride, - ); - - $outcome = PlanningOutcome::fromAgentOutput($agentResult->success, $agentResult->resultText); - - $this->logger->info('[PlanningAgent] Agent completed', [ - 'issueNumber' => $message->issueNumber, - 'outcome' => $outcome->value, - 'durationMs' => $agentResult->durationMs, - ]); + function () use ($message, $config): void { + $issueDto = $this->findIssue($config->githubUrl, $config->githubToken, $message->issueNumber); + if ($issueDto === null) { + $this->logger->error('[PlanningAgent] Could not fetch issue data', [ + 'issueNumber' => $message->issueNumber, + ]); + + return; + } - $this->handleOutcome( - $outcome, - $agentResult->resultText, + $comments = $this->githubIntegrationFacade->getIssueComments( $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(), - ); - } finally { - if ($workspaceInfo !== null) { - $this->workspaceManagementFacade->releaseWorkspace($workspaceInfo); + $workspaceInfo = null; + + try { + $workspaceInfo = $this->workspaceManagementFacade->acquireWorkspace( + $config->id, + $message->issueNumber, + $config->githubUrl, + $config->githubToken, + ); + + $prompt = $this->buildPrompt($issueDto, $comments, $message->isResume); + $agentApiKeyOverride = $message->agentApiKeyOverride; + $credentials = $agentApiKeyOverride !== null + ? new AgentCredentialsDto($agentApiKeyOverride, $agentApiKeyOverride) + : new AgentCredentialsDto($config->cursorAgentApiKey, $config->anthropicApiKey); + + $agentResult = $this->llmIntegrationFacade->runAgent( + AgentRole::Planning, + $prompt, + $workspaceInfo->workspacePath, + $credentials, + $config->githubToken, + $workspaceInfo->containerName, + $message->agentProviderOverride, + ); + + $outcome = PlanningOutcome::fromAgentOutput($agentResult->success, $agentResult->resultText); + + $this->logger->info('[PlanningAgent] Agent completed', [ + 'issueNumber' => $message->issueNumber, + 'outcome' => $outcome->value, + 'durationMs' => $agentResult->durationMs, + ]); + + $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(), + ); + } finally { + if ($workspaceInfo !== null) { + $this->workspaceManagementFacade->releaseWorkspace($workspaceInfo); + } } - } - } finally { - $this->workflowConcurrencyFacade->releaseRunClaim( - $message->productConfigId, - $message->issueNumber, - $message->runId, - ); + }, + ); + + if (!$didRun) { + $this->logger->info('[PlanningAgent] Skipping stale or duplicate planning message', [ + 'productConfigId' => $message->productConfigId, + 'issueNumber' => $message->issueNumber, + 'runId' => $message->runId, + ]); } } diff --git a/src/Workflow/Facade/WorkflowConcurrencyFacade.php b/src/Workflow/Facade/WorkflowConcurrencyFacade.php index 04ca63f..858f150 100644 --- a/src/Workflow/Facade/WorkflowConcurrencyFacade.php +++ b/src/Workflow/Facade/WorkflowConcurrencyFacade.php @@ -76,6 +76,21 @@ public function releaseRunClaim(string $productConfigId, int $issueNumber, strin ); } + public function executeWithRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId, callable $work): bool + { + if (!$this->tryStartRun($productConfigId, $issueNumber, $phase, $runId)) { + return false; + } + + try { + $work(); + + return true; + } finally { + $this->releaseRunClaim($productConfigId, $issueNumber, $runId); + } + } + public function clearRunClaimsForIssue(string $productConfigId, int $issueNumber): int { $deletedRows = $this->connection->executeStatement( diff --git a/src/Workflow/Facade/WorkflowConcurrencyFacadeInterface.php b/src/Workflow/Facade/WorkflowConcurrencyFacadeInterface.php index 2a264e2..438d784 100644 --- a/src/Workflow/Facade/WorkflowConcurrencyFacadeInterface.php +++ b/src/Workflow/Facade/WorkflowConcurrencyFacadeInterface.php @@ -14,6 +14,13 @@ public function tryStartRun(string $productConfigId, int $issueNumber, WorkflowR public function releaseRunClaim(string $productConfigId, int $issueNumber, string $runId): void; + /** + * Starts the run (tryStartRun), executes the callable, and releases the claim in finally. + * Returns false if tryStartRun failed (stale/duplicate); otherwise runs $work and returns true. + * Exceptions from $work propagate; releaseRunClaim is still called in finally. + */ + public function executeWithRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId, callable $work): bool; + public function clearRunClaimsForIssue(string $productConfigId, int $issueNumber): int; public function resetIssueLocksAndClaims(string $productConfigId, int $issueNumber, string $teamleaderIssueLockKey): void; diff --git a/src/Workflow/Facade/WorkflowFacadeInterface.php b/src/Workflow/Facade/WorkflowFacadeInterface.php deleted file mode 100644 index e69de29..0000000 diff --git a/src/Workflow/Infrastructure/Service/ImplementationRunStarter.php b/src/Workflow/Infrastructure/Service/ImplementationRunStarter.php index 468153d..aa66eb4 100644 --- a/src/Workflow/Infrastructure/Service/ImplementationRunStarter.php +++ b/src/Workflow/Infrastructure/Service/ImplementationRunStarter.php @@ -4,36 +4,23 @@ namespace App\Workflow\Infrastructure\Service; -use App\GithubIntegration\Facade\Enum\GithubLabel; -use App\GithubIntegration\Facade\GithubIntegrationFacadeInterface; use App\Workflow\Facade\Enum\WorkflowRunPhase; -use App\Workflow\Facade\WorkflowConcurrencyFacadeInterface; -use Symfony\Component\Uid\Uuid; -use Throwable; readonly class ImplementationRunStarter implements ImplementationRunStarterInterface { public function __construct( - private WorkflowConcurrencyFacadeInterface $workflowConcurrencyFacade, - private GithubIntegrationFacadeInterface $githubIntegrationFacade, + private PhaseRunStarter $phaseRunStarter, ) { } public function startImplementation(string $githubUrl, string $githubToken, string $productConfigId, int $issueNumber): ?string { - $runId = Uuid::v4()->toRfc4122(); - if (!$this->workflowConcurrencyFacade->tryCreateRunClaim($productConfigId, $issueNumber, WorkflowRunPhase::Implementation, $runId)) { - return null; - } - - try { - $this->githubIntegrationFacade->addLabelToIssue($githubUrl, $githubToken, $issueNumber, GithubLabel::CoordinationOngoing); - $this->githubIntegrationFacade->addLabelToIssue($githubUrl, $githubToken, $issueNumber, GithubLabel::ImplementationOngoing); - } catch (Throwable $e) { - $this->workflowConcurrencyFacade->releaseRunClaim($productConfigId, $issueNumber, $runId); - throw $e; - } - - return $runId; + return $this->phaseRunStarter->startRun( + WorkflowRunPhase::Implementation, + $githubUrl, + $githubToken, + $productConfigId, + $issueNumber, + ); } } diff --git a/src/Workflow/Infrastructure/Service/PhaseRunStarter.php b/src/Workflow/Infrastructure/Service/PhaseRunStarter.php new file mode 100644 index 0000000..b5120ea --- /dev/null +++ b/src/Workflow/Infrastructure/Service/PhaseRunStarter.php @@ -0,0 +1,56 @@ +toRfc4122(); + if (!$this->workflowConcurrencyFacade->tryCreateRunClaim($productConfigId, $issueNumber, $phase, $runId)) { + return null; + } + + try { + $this->githubIntegrationFacade->addLabelToIssue($githubUrl, $githubToken, $issueNumber, GithubLabel::CoordinationOngoing); + $this->githubIntegrationFacade->addLabelToIssue($githubUrl, $githubToken, $issueNumber, $this->phaseOngoingLabel($phase)); + } catch (Throwable $e) { + $this->workflowConcurrencyFacade->releaseRunClaim($productConfigId, $issueNumber, $runId); + throw $e; + } + + return $runId; + } + + private function phaseOngoingLabel(WorkflowRunPhase $phase): GithubLabel + { + return match ($phase) { + WorkflowRunPhase::Planning => GithubLabel::PlanningOngoing, + WorkflowRunPhase::Implementation => GithubLabel::ImplementationOngoing, + }; + } +} diff --git a/src/Workflow/Infrastructure/Service/PlanningRunStarter.php b/src/Workflow/Infrastructure/Service/PlanningRunStarter.php index df4c9f5..8c689f9 100644 --- a/src/Workflow/Infrastructure/Service/PlanningRunStarter.php +++ b/src/Workflow/Infrastructure/Service/PlanningRunStarter.php @@ -4,36 +4,23 @@ namespace App\Workflow\Infrastructure\Service; -use App\GithubIntegration\Facade\Enum\GithubLabel; -use App\GithubIntegration\Facade\GithubIntegrationFacadeInterface; use App\Workflow\Facade\Enum\WorkflowRunPhase; -use App\Workflow\Facade\WorkflowConcurrencyFacadeInterface; -use Symfony\Component\Uid\Uuid; -use Throwable; readonly class PlanningRunStarter implements PlanningRunStarterInterface { public function __construct( - private WorkflowConcurrencyFacadeInterface $workflowConcurrencyFacade, - private GithubIntegrationFacadeInterface $githubIntegrationFacade, + private PhaseRunStarter $phaseRunStarter, ) { } public function startPlanning(string $githubUrl, string $githubToken, string $productConfigId, int $issueNumber): ?string { - $runId = Uuid::v4()->toRfc4122(); - if (!$this->workflowConcurrencyFacade->tryCreateRunClaim($productConfigId, $issueNumber, WorkflowRunPhase::Planning, $runId)) { - return null; - } - - try { - $this->githubIntegrationFacade->addLabelToIssue($githubUrl, $githubToken, $issueNumber, GithubLabel::CoordinationOngoing); - $this->githubIntegrationFacade->addLabelToIssue($githubUrl, $githubToken, $issueNumber, GithubLabel::PlanningOngoing); - } catch (Throwable $e) { - $this->workflowConcurrencyFacade->releaseRunClaim($productConfigId, $issueNumber, $runId); - throw $e; - } - - return $runId; + return $this->phaseRunStarter->startRun( + WorkflowRunPhase::Planning, + $githubUrl, + $githubToken, + $productConfigId, + $issueNumber, + ); } } diff --git a/src/WorkspaceManagement/Facade/WorkspaceManagementFacade.php b/src/WorkspaceManagement/Facade/WorkspaceManagementFacade.php index 9d03398..8575fea 100644 --- a/src/WorkspaceManagement/Facade/WorkspaceManagementFacade.php +++ b/src/WorkspaceManagement/Facade/WorkspaceManagementFacade.php @@ -6,16 +6,16 @@ use App\WorkspaceManagement\Facade\Dto\WorkspaceInfoDto; use App\WorkspaceManagement\Facade\Dto\WorkspaceInfoLookupDto; -use App\WorkspaceManagement\Infrastructure\Service\GitCloneService; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceAcquisitionInterface; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceInfoLookupService; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceSweepCleanupService; readonly class WorkspaceManagementFacade implements WorkspaceManagementFacadeInterface { public function __construct( - private GitCloneService $gitCloneService, - private WorkspaceSweepCleanupService $workspaceSweepCleanupService, - private WorkspaceInfoLookupService $workspaceInfoLookupService, + private WorkspaceAcquisitionInterface $workspaceAcquisition, + private WorkspaceSweepCleanupService $workspaceSweepCleanupService, + private WorkspaceInfoLookupService $workspaceInfoLookupService, ) { } @@ -26,7 +26,7 @@ public function acquireWorkspace( string $githubToken, ?string $branchName = null, ): WorkspaceInfoDto { - return $this->gitCloneService->acquireWorkspace( + return $this->workspaceAcquisition->acquireWorkspace( $productConfigId, $issueNumber, $githubUrl, @@ -37,12 +37,12 @@ public function acquireWorkspace( public function releaseWorkspace(WorkspaceInfoDto $workspaceInfo): void { - $this->gitCloneService->releaseWorkspace($workspaceInfo->workspacePath); + $this->workspaceAcquisition->releaseWorkspace($workspaceInfo->workspacePath); } public function destroyWorkspace(WorkspaceInfoDto $workspaceInfo): void { - $this->gitCloneService->removeWorkspace($workspaceInfo->workspacePath, $workspaceInfo->containerName); + $this->workspaceAcquisition->removeWorkspace($workspaceInfo->workspacePath, $workspaceInfo->containerName); } public function cleanupWorkspaceForIssue(string $productConfigId, int $issueNumber): void @@ -65,11 +65,11 @@ public function lookupWorkspaceInfoByPrefix(string $workspaceIdPrefix): array public function createWorkspace(string $githubUrl, string $githubToken): WorkspaceInfoDto { - return $this->gitCloneService->cloneRepository($githubUrl, $githubToken); + return $this->workspaceAcquisition->cloneRepository($githubUrl, $githubToken); } public function createWorkspaceOnBranch(string $githubUrl, string $githubToken, string $branchName): WorkspaceInfoDto { - return $this->gitCloneService->cloneRepositoryOnBranch($githubUrl, $githubToken, $branchName); + return $this->workspaceAcquisition->cloneRepositoryOnBranch($githubUrl, $githubToken, $branchName); } } diff --git a/src/WorkspaceManagement/Infrastructure/Command/WorkspaceCleanupCommand.php b/src/WorkspaceManagement/Infrastructure/Command/WorkspaceCleanupCommand.php index 5732dea..44c8ec3 100644 --- a/src/WorkspaceManagement/Infrastructure/Command/WorkspaceCleanupCommand.php +++ b/src/WorkspaceManagement/Infrastructure/Command/WorkspaceCleanupCommand.php @@ -7,9 +7,9 @@ use App\GithubIntegration\Facade\GithubIntegrationFacadeInterface; use App\ProductConfig\Facade\ProductConfigFacadeInterface; use App\WorkspaceManagement\Facade\Dto\WorkspaceInfoDto; -use App\WorkspaceManagement\Infrastructure\Service\GitCloneService; -use App\WorkspaceManagement\Infrastructure\Service\ProjectContainerManager; -use App\WorkspaceManagement\Infrastructure\Service\WorkspaceMetadataManager; +use App\WorkspaceManagement\Infrastructure\Service\ProjectContainerManagerInterface; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceAcquisitionInterface; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceMetadataManagerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; @@ -27,13 +27,13 @@ class WorkspaceCleanupCommand extends Command { public function __construct( - private readonly GitCloneService $gitCloneService, - private readonly ProjectContainerManager $containerManager, - private readonly WorkspaceMetadataManager $metadataManager, - private readonly ProductConfigFacadeInterface $productConfigFacade, - private readonly GithubIntegrationFacadeInterface $githubIntegrationFacade, - private readonly LockFactory $lockFactory, - private readonly LoggerInterface $logger, + private readonly WorkspaceAcquisitionInterface $workspaceAcquisition, + private readonly ProjectContainerManagerInterface $containerManager, + private readonly WorkspaceMetadataManagerInterface $metadataManager, + private readonly ProductConfigFacadeInterface $productConfigFacade, + private readonly GithubIntegrationFacadeInterface $githubIntegrationFacade, + private readonly LockFactory $lockFactory, + private readonly LoggerInterface $logger, ) { parent::__construct(); } @@ -57,7 +57,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $io->note('Running in dry-run mode — no workspaces will be removed.'); } - $workspaceBaseDir = $this->gitCloneService->resolveWorkspaceBaseDir(); + $workspaceBaseDir = $this->workspaceAcquisition->resolveWorkspaceBaseDir(); if (!is_dir($workspaceBaseDir)) { $io->success('No workspace directory found. Nothing to clean up.'); @@ -134,7 +134,7 @@ private function processWorkspace(SymfonyStyle $io, string $workspacePath, bool return 'skipped'; } - $lock = $this->lockFactory->createLock(GitCloneService::WORKSPACE_LOCK_PREFIX . $workspaceId, 600.0); + $lock = $this->lockFactory->createLock(WorkspaceAcquisitionInterface::WORKSPACE_LOCK_PREFIX . $workspaceId, 600.0); if (!$lock->acquire(false)) { $io->text(sprintf( @@ -251,14 +251,14 @@ private function destroyWorkspace(SymfonyStyle $io, string $workspacePath, strin } try { - $repoPath = GitCloneService::repoPath($workspacePath); + $repoPath = $this->workspaceAcquisition->repoPath($workspacePath); $containerName = $this->containerManager->hasProjectContainerConfig($repoPath) ? 'pb-workspace-' . $workspaceId : null; $workspaceInfo = new WorkspaceInfoDto($workspacePath, $containerName); - $this->gitCloneService->removeWorkspace( + $this->workspaceAcquisition->removeWorkspace( $workspaceInfo->workspacePath, $workspaceInfo->containerName, ); diff --git a/src/WorkspaceManagement/Infrastructure/Service/GitCloneService.php b/src/WorkspaceManagement/Infrastructure/Service/GitCloneService.php index 4e50adb..43ba940 100644 --- a/src/WorkspaceManagement/Infrastructure/Service/GitCloneService.php +++ b/src/WorkspaceManagement/Infrastructure/Service/GitCloneService.php @@ -13,7 +13,7 @@ use Symfony\Component\Process\Process; use Throwable; -class GitCloneService +class GitCloneService implements WorkspaceAcquisitionInterface { public const string WORKSPACE_LOCK_PREFIX = 'ws-'; private const float WORKSPACE_LOCK_TTL_SECONDS = 7200.0; @@ -24,11 +24,12 @@ class GitCloneService private array $heldWorkspaceLocks = []; public function __construct( - private readonly Filesystem $filesystem, - private readonly ProjectContainerManager $containerManager, - private readonly WorkspaceMetadataManager $metadataManager, - private readonly LockFactory $lockFactory, - private readonly LoggerInterface $logger, + private readonly Filesystem $filesystem, + private readonly ProjectContainerManagerInterface $containerManager, + private readonly WorkspaceMetadataManagerInterface $metadataManager, + private readonly LockFactory $lockFactory, + private readonly LoggerInterface $logger, + private readonly WorkspaceRemovalInterface $workspaceRemoval, ) { } @@ -129,26 +130,11 @@ public function removeWorkspace(string $workspacePath, ?string $containerName): $workspaceId = $this->deriveWorkspaceIdFromPath($workspacePath); try { - if ($containerName !== null) { - $this->containerManager->stopProjectContainer($containerName, $workspacePath); - } - - $this->assertWorkspacePathIsWithinBaseDir($workspacePath); - - $resolvedPath = realpath($workspacePath); - if ($resolvedPath === false) { - $this->logger->info('[WorkspaceManagement] Workspace path already removed, nothing to do', [ - 'workspacePath' => $workspacePath, - ]); - - return; - } - - $this->filesystem->remove($resolvedPath); - - $this->logger->info('[WorkspaceManagement] Workspace removed', [ - 'workspacePath' => $workspacePath, - ]); + $this->workspaceRemoval->removeWorkspace( + $workspacePath, + $containerName, + $this->resolveWorkspaceBaseDir(), + ); } finally { $this->releaseWorkspaceLock($workspaceId); } @@ -164,7 +150,7 @@ public function cloneRepository(string $githubUrl, string $githubToken): Workspa $workspaceId = bin2hex(random_bytes(16)); $workspacePath = $workspaceBaseDir . '/' . $workspaceId; - $repoPath = self::repoPath($workspacePath); + $repoPath = $this->repoPath($workspacePath); $authenticatedUrl = $this->buildAuthenticatedUrl($githubUrl, $githubToken); $this->logger->info('[WorkspaceManagement] Cloning repository', [ @@ -192,7 +178,7 @@ public function cloneRepositoryOnBranch(string $githubUrl, string $githubToken, $workspaceId = bin2hex(random_bytes(16)); $workspacePath = $workspaceBaseDir . '/' . $workspaceId; - $repoPath = self::repoPath($workspacePath); + $repoPath = $this->repoPath($workspacePath); $authenticatedUrl = $this->buildAuthenticatedUrl($githubUrl, $githubToken); $this->logger->info('[WorkspaceManagement] Cloning repository (full clone for branch checkout)', [ @@ -245,7 +231,7 @@ private function reuseWorkspace( string $authenticatedUrl, ?string $branchName, ): WorkspaceInfoDto { - $repoPath = self::repoPath($workspacePath); + $repoPath = $this->repoPath($workspacePath); $this->logger->info('[WorkspaceManagement] Reusing existing workspace', [ 'workspacePath' => $workspacePath, @@ -512,14 +498,14 @@ private function checkoutBranch(string $workspacePath, string $branchName): void ]); } - public static function repoPath(string $workspacePath): string + public function repoPath(string $workspacePath): string { return $workspacePath . '/' . WorkspaceInfoDto::REPO_SUBDIR; } private function isEnvelopeWithRepo(string $workspacePath): bool { - return is_dir(self::repoPath($workspacePath) . '/.git'); + return is_dir($this->repoPath($workspacePath) . '/.git'); } private function buildCorruptWorkspacePath(string $workspacePath): string @@ -577,37 +563,9 @@ private function releaseWorkspaceLock(string $workspaceId): void unset($this->heldWorkspaceLocks[$workspaceId]); } - private function assertWorkspacePathIsWithinBaseDir(string $workspacePath): void - { - $workspaceBaseDir = $this->resolveWorkspaceBaseDir(); - $baseReal = realpath($workspaceBaseDir); - - if ($baseReal === false) { - throw new RuntimeException(sprintf( - 'Cannot resolve workspace base directory realpath: "%s"', - $workspaceBaseDir, - )); - } - - $pathReal = realpath($workspacePath); - if ($pathReal === false) { - return; - } - - $basePrefix = rtrim($baseReal, '/') . '/'; - - if (!str_starts_with($pathReal, $basePrefix)) { - throw new RuntimeException(sprintf( - 'Refusing to remove path outside workspace base directory: "%s" (resolved to "%s")', - $workspacePath, - $pathReal, - )); - } - } - private function maybeStartProjectContainer(string $workspacePath, string $workspaceId): ?string { - $repoPath = self::repoPath($workspacePath); + $repoPath = $this->repoPath($workspacePath); if (!$this->containerManager->hasProjectContainerConfig($repoPath)) { $this->logger->info('[WorkspaceManagement] No .productbuilder/ config found, agent will run locally'); diff --git a/src/WorkspaceManagement/Infrastructure/Service/ProjectContainerManager.php b/src/WorkspaceManagement/Infrastructure/Service/ProjectContainerManager.php index c093631..51fa207 100644 --- a/src/WorkspaceManagement/Infrastructure/Service/ProjectContainerManager.php +++ b/src/WorkspaceManagement/Infrastructure/Service/ProjectContainerManager.php @@ -11,7 +11,7 @@ use Symfony\Component\Process\Process; use Throwable; -class ProjectContainerManager +class ProjectContainerManager implements ProjectContainerManagerInterface { private const string DOCKERIMAGE_FILE = '.productbuilder/dockerimage'; private const string STARTUP_SCRIPT = '.productbuilder/startup.sh'; diff --git a/src/WorkspaceManagement/Infrastructure/Service/ProjectContainerManagerInterface.php b/src/WorkspaceManagement/Infrastructure/Service/ProjectContainerManagerInterface.php new file mode 100644 index 0000000..00ce0cd --- /dev/null +++ b/src/WorkspaceManagement/Infrastructure/Service/ProjectContainerManagerInterface.php @@ -0,0 +1,16 @@ +gitCloneService->resolveWorkspaceBaseDir(); + $workspaceBaseDir = $this->workspaceAcquisition->resolveWorkspaceBaseDir(); if (!is_dir($workspaceBaseDir)) { return []; } diff --git a/src/WorkspaceManagement/Infrastructure/Service/WorkspaceMetadataManager.php b/src/WorkspaceManagement/Infrastructure/Service/WorkspaceMetadataManager.php index 5000725..ea38ae7 100644 --- a/src/WorkspaceManagement/Infrastructure/Service/WorkspaceMetadataManager.php +++ b/src/WorkspaceManagement/Infrastructure/Service/WorkspaceMetadataManager.php @@ -11,10 +11,8 @@ use JsonException; use RuntimeException; -class WorkspaceMetadataManager +class WorkspaceMetadataManager implements WorkspaceMetadataManagerInterface { - public const string METADATA_FILE = '.workspace-meta.json'; - public function write(string $workspacePath, string $productConfigId, int $issueNumber): void { $now = DateAndTimeService::getDateTimeImmutable(); diff --git a/src/WorkspaceManagement/Infrastructure/Service/WorkspaceMetadataManagerInterface.php b/src/WorkspaceManagement/Infrastructure/Service/WorkspaceMetadataManagerInterface.php new file mode 100644 index 0000000..07001be --- /dev/null +++ b/src/WorkspaceManagement/Infrastructure/Service/WorkspaceMetadataManagerInterface.php @@ -0,0 +1,18 @@ +containerManager->stopProjectContainer($containerName, $workspacePath); + } + + $this->assertWorkspacePathIsWithinBaseDir($workspacePath, $workspaceBaseDir); + + $resolvedPath = realpath($workspacePath); + if ($resolvedPath === false) { + $this->logger->info('[WorkspaceManagement] Workspace path already removed, nothing to do', [ + 'workspacePath' => $workspacePath, + ]); + + return; + } + + $this->filesystem->remove($resolvedPath); + + $this->logger->info('[WorkspaceManagement] Workspace removed', [ + 'workspacePath' => $workspacePath, + ]); + } + + private function assertWorkspacePathIsWithinBaseDir(string $workspacePath, string $workspaceBaseDir): void + { + $baseReal = realpath($workspaceBaseDir); + + if ($baseReal === false) { + throw new RuntimeException(sprintf( + 'Cannot resolve workspace base directory realpath: "%s"', + $workspaceBaseDir, + )); + } + + $pathReal = realpath($workspacePath); + if ($pathReal === false) { + return; + } + + $basePrefix = rtrim($baseReal, '/') . '/'; + + if (!str_starts_with($pathReal, $basePrefix)) { + throw new RuntimeException(sprintf( + 'Refusing to remove path outside workspace base directory: "%s" (resolved to "%s")', + $workspacePath, + $pathReal, + )); + } + } +} diff --git a/src/WorkspaceManagement/Infrastructure/Service/WorkspaceSweepCleanupService.php b/src/WorkspaceManagement/Infrastructure/Service/WorkspaceSweepCleanupService.php index d03dca9..e21d92e 100644 --- a/src/WorkspaceManagement/Infrastructure/Service/WorkspaceSweepCleanupService.php +++ b/src/WorkspaceManagement/Infrastructure/Service/WorkspaceSweepCleanupService.php @@ -11,7 +11,7 @@ class WorkspaceSweepCleanupService { public function __construct( - private readonly GitCloneService $gitCloneService, + private readonly WorkspaceAcquisitionInterface $workspaceAcquisition, private readonly Filesystem $filesystem, private readonly WorkspaceCleanupCommandRunnerInterface $commandRunner, private readonly LoggerInterface $logger, @@ -20,13 +20,13 @@ public function __construct( public function cleanupByIssue(string $productConfigId, int $issueNumber): void { - $workspaceId = $this->gitCloneService->deriveWorkspaceId($productConfigId, $issueNumber); + $workspaceId = $this->workspaceAcquisition->deriveWorkspaceId($productConfigId, $issueNumber); $this->cleanupByWorkspaceId($workspaceId); } public function cleanupByWorkspaceId(string $workspaceId): void { - $workspaceBase = $this->gitCloneService->resolveWorkspaceBaseDir(); + $workspaceBase = $this->workspaceAcquisition->resolveWorkspaceBaseDir(); $matchedContainerIds = $this->findMatchingContainerIds($workspaceId); $imageIds = $this->findMatchingImageIds($workspaceId, $matchedContainerIds); diff --git a/tests/Unit/ImplementationAgent/ImplementationOutcomeTest.php b/tests/Unit/ImplementationAgent/ImplementationOutcomeTest.php index 8863d25..8cfcd4f 100644 --- a/tests/Unit/ImplementationAgent/ImplementationOutcomeTest.php +++ b/tests/Unit/ImplementationAgent/ImplementationOutcomeTest.php @@ -3,7 +3,6 @@ declare(strict_types=1); use App\ImplementationAgent\Domain\Enum\ImplementationOutcome; -use RuntimeException; describe('ImplementationOutcome::fromAgentOutput', function (): void { describe('explicit markers', function (): void { diff --git a/tests/Unit/LlmIntegration/AgentStreamParserTest.php b/tests/Unit/LlmIntegration/AgentStreamParserTest.php index d90d90e..ba6f180 100644 --- a/tests/Unit/LlmIntegration/AgentStreamParserTest.php +++ b/tests/Unit/LlmIntegration/AgentStreamParserTest.php @@ -4,6 +4,7 @@ use App\LlmIntegration\Infrastructure\Service\AgentStreamParser; use App\LlmIntegration\Infrastructure\Service\Dto\CliAgentRunResult; +use App\LlmIntegration\Infrastructure\Service\NdjsonStreamLineParser; use Monolog\Formatter\LineFormatter; use Monolog\Handler\StreamHandler; use Monolog\Level; @@ -41,7 +42,7 @@ $scriptLines[] = 'echo ' . escapeshellarg($resultLine); $logger = new RecordingLoggerForAgentStreamParserTest(); - $parser = new AgentStreamParser($logger->logger); + $parser = new AgentStreamParser(new NdjsonStreamLineParser(), $logger->logger); $result = runParserWithScript( $parser, implode("\n", $scriptLines), @@ -78,7 +79,7 @@ $resultLine = buildResultLineForAgentStreamParserTest(false, 'ok', 'sess-timeout', 222); $logger = new RecordingLoggerForAgentStreamParserTest(); - $parser = new AgentStreamParser($logger->logger); + $parser = new AgentStreamParser(new NdjsonStreamLineParser(), $logger->logger); $result = runParserWithScript( $parser, sprintf( @@ -118,7 +119,7 @@ ], JSON_THROW_ON_ERROR); $logger = new RecordingLoggerForAgentStreamParserTest(); - $parser = new AgentStreamParser($logger->logger); + $parser = new AgentStreamParser(new NdjsonStreamLineParser(), $logger->logger); $result = runParserWithScript( $parser, sprintf("echo %s\necho %s", escapeshellarg($assistantLine), escapeshellarg($resultLine)), @@ -184,7 +185,7 @@ ], JSON_THROW_ON_ERROR); $logger = new RecordingLoggerForAgentStreamParserTest(); - $parser = new AgentStreamParser($logger->logger); + $parser = new AgentStreamParser(new NdjsonStreamLineParser(), $logger->logger); $result = runParserWithScript( $parser, sprintf( @@ -223,7 +224,7 @@ ], JSON_THROW_ON_ERROR); $logger = new RecordingLoggerForAgentStreamParserTest(); - $parser = new AgentStreamParser($logger->logger); + $parser = new AgentStreamParser(new NdjsonStreamLineParser(), $logger->logger); $result = runParserWithScript( $parser, sprintf('printf %%s %s', escapeshellarg($resultLine)), diff --git a/tests/Unit/LlmIntegration/CursorCliAgentBinaryManagerTest.php b/tests/Unit/LlmIntegration/CursorCliAgentBinaryManagerTest.php index 16ebc9e..dc9aa87 100644 --- a/tests/Unit/LlmIntegration/CursorCliAgentBinaryManagerTest.php +++ b/tests/Unit/LlmIntegration/CursorCliAgentBinaryManagerTest.php @@ -3,8 +3,21 @@ declare(strict_types=1); use App\LlmIntegration\Infrastructure\Service\Cursor\CursorCliBinaryManager; +use App\LlmIntegration\Infrastructure\Service\Cursor\CursorCliConfigWriter; +use App\LlmIntegration\Infrastructure\Service\Cursor\CursorWorkspaceRuntimePreparator; use Psr\Log\NullLogger; +function createCursorCliBinaryManager(): CursorCliBinaryManager +{ + $logger = new NullLogger(); + + return new CursorCliBinaryManager( + new CursorCliConfigWriter($logger), + new CursorWorkspaceRuntimePreparator(new CursorCliConfigWriter($logger), $logger), + $logger, + ); +} + describe('CursorCliBinaryManager', function (): void { describe('ensureCliConfig', function (): void { it('creates cli-config.json in the cursor config directory', function (): void { @@ -15,7 +28,7 @@ putenv('HOME=' . $tempHome); try { - $manager = new CursorCliBinaryManager(new NullLogger()); + $manager = createCursorCliBinaryManager(); $manager->ensureCliConfig(); $configPath = $tempHome . '/.cursor/cli-config.json'; @@ -56,7 +69,7 @@ putenv('HOME=' . $tempHome); try { - $manager = new CursorCliBinaryManager(new NullLogger()); + $manager = createCursorCliBinaryManager(); $manager->ensureCliConfig(); expect(file_get_contents($configPath))->not->toBe($customContent); @@ -119,7 +132,7 @@ putenv('HOME=' . $tempHome); try { - $manager = new CursorCliBinaryManager(new NullLogger()); + $manager = createCursorCliBinaryManager(); $manager->prepareWorkspaceRuntime($workspacePath); $runtimeBin = $workspacePath . '/runtime/bin'; @@ -173,7 +186,7 @@ putenv('HOME=' . $tempHome); try { - $manager = new CursorCliBinaryManager(new NullLogger()); + $manager = createCursorCliBinaryManager(); $result = $manager->findOrInstall(); expect($result)->toBe($agentPath); @@ -204,7 +217,7 @@ putenv('HOME=' . $tempHome); try { - $manager = new CursorCliBinaryManager(new NullLogger()); + $manager = createCursorCliBinaryManager(); $first = $manager->findOrInstall(); $second = $manager->findOrInstall(); diff --git a/tests/Unit/LlmIntegration/NdjsonStreamLineParserTest.php b/tests/Unit/LlmIntegration/NdjsonStreamLineParserTest.php new file mode 100644 index 0000000..9e40f0f --- /dev/null +++ b/tests/Unit/LlmIntegration/NdjsonStreamLineParserTest.php @@ -0,0 +1,66 @@ +parseLine($line); + + expect($parsed)->not->toBeNull(); + assert($parsed !== null); + expect($parsed->jsonLine)->toBe($line); + $data = json_decode($parsed->jsonLine, true, 512, JSON_THROW_ON_ERROR); + assert(is_array($data)); + expect($data['type'])->toBe('result'); + expect($data['subtype'])->toBe('success'); + expect($data['result'])->toBe('ok'); + expect($data['session_id'])->toBe('sess-1'); + }); + + it('returns a DTO with the trimmed line for valid assistant JSON', function (): void { + $line = '{"type":"assistant","message":{"role":"assistant","content":[{"type":"text","text":"Hello"}]},"session_id":"sess-2"}'; + $parser = new NdjsonStreamLineParser(); + + $parsed = $parser->parseLine($line); + + expect($parsed)->not->toBeNull(); + assert($parsed !== null); + expect($parsed->jsonLine)->toBe($line); + $data = json_decode($parsed->jsonLine, true, 512, JSON_THROW_ON_ERROR); + assert(is_array($data)); + expect($data['type'])->toBe('assistant'); + $message = $data['message'] ?? null; + assert(is_array($message)); + $content = $message['content'] ?? null; + assert(is_array($content)); + $first = $content[0] ?? null; + assert(is_array($first)); + expect($first['text'] ?? null)->toBe('Hello'); + }); + + it('returns null for empty or whitespace-only line', function (): void { + $parser = new NdjsonStreamLineParser(); + + expect($parser->parseLine(''))->toBeNull(); + expect($parser->parseLine(' '))->toBeNull(); + }); + + it('returns a DTO for non-empty line even when not valid JSON (consumer decodes)', function (): void { + $parser = new NdjsonStreamLineParser(); + + $parsed = $parser->parseLine('not json'); + expect($parsed)->not->toBeNull(); + assert($parsed !== null); + expect($parsed->jsonLine)->toBe('not json'); + + $parsed2 = $parser->parseLine('{"type":'); + expect($parsed2)->not->toBeNull(); + assert($parsed2 !== null); + expect($parsed2->jsonLine)->toBe('{"type":'); + }); +}); diff --git a/tests/Unit/LlmIntegration/RealCursorCliAgentDriverTest.php b/tests/Unit/LlmIntegration/RealCursorCliAgentDriverTest.php index 32ffaca..7f44352 100644 --- a/tests/Unit/LlmIntegration/RealCursorCliAgentDriverTest.php +++ b/tests/Unit/LlmIntegration/RealCursorCliAgentDriverTest.php @@ -4,12 +4,25 @@ use App\LlmIntegration\Infrastructure\Service\AgentStreamParser; use App\LlmIntegration\Infrastructure\Service\Cursor\CursorCliBinaryManager; +use App\LlmIntegration\Infrastructure\Service\Cursor\CursorCliConfigWriter; use App\LlmIntegration\Infrastructure\Service\Cursor\CursorCliDriver; +use App\LlmIntegration\Infrastructure\Service\Cursor\CursorWorkspaceRuntimePreparator; use App\LlmIntegration\Infrastructure\Service\Dto\TokenOwnerProfile; use App\LlmIntegration\Infrastructure\Service\GitIdentityResolver; use App\WorkspaceManagement\Facade\Dto\WorkspaceInfoDto; use Psr\Log\NullLogger; +function createCursorCliBinaryManagerForDriverTest(): CursorCliBinaryManager +{ + $logger = new NullLogger(); + + return new CursorCliBinaryManager( + new CursorCliConfigWriter($logger), + new CursorWorkspaceRuntimePreparator(new CursorCliConfigWriter($logger), $logger), + $logger, + ); +} + /** * Creates a fake agent script that emits NDJSON lines (stream-json format), * sets up HOME and a workspace envelope with a repo/ subdir, runs the test, and cleans up. @@ -158,7 +171,7 @@ function buildDriver(CursorCliBinaryManager $binaryManager, ?TokenOwnerProfile $ ); $logger = new NullLogger(); - $streamParser = new AgentStreamParser($logger); + $streamParser = new AgentStreamParser(new App\LlmIntegration\Infrastructure\Service\NdjsonStreamLineParser(), $logger); $gitIdentityResolver = new class($logger, $profile) extends GitIdentityResolver { public function __construct( @@ -190,7 +203,7 @@ protected function fetchGithubTokenOwnerProfile(string $githubToken): TokenOwner ]; runWithFakeAgent($ndjsonLines, function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager); $result = $driver->run('Test prompt', $workspacePath, 'fake-api-key', 'fake-github-token'); @@ -208,7 +221,7 @@ protected function fetchGithubTokenOwnerProfile(string $githubToken): TokenOwner ]; runWithFakeAgent($ndjsonLines, function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager); $result = $driver->run('Test prompt', $workspacePath, 'fake-api-key', 'fake-github-token'); @@ -227,7 +240,7 @@ protected function fetchGithubTokenOwnerProfile(string $githubToken): TokenOwner ]; runWithFakeAgent($ndjsonLines, function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager); $result = $driver->run('Test prompt', $workspacePath, 'fake-api-key', 'fake-github-token'); @@ -242,7 +255,7 @@ protected function fetchGithubTokenOwnerProfile(string $githubToken): TokenOwner describe('failed agent execution', function (): void { it('returns failure result when the process exits with non-zero code', function (): void { runWithFakeAgent([], function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager); $result = $driver->run('Test prompt', $workspacePath, 'bad-api-key', 'fake-github-token'); @@ -259,7 +272,7 @@ protected function fetchGithubTokenOwnerProfile(string $githubToken): TokenOwner ]; runWithFakeAgent($ndjsonLines, function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager); $result = $driver->run('Test prompt', $workspacePath, 'bad-api-key', 'fake-github-token'); @@ -278,7 +291,7 @@ protected function fetchGithubTokenOwnerProfile(string $githubToken): TokenOwner ]; runWithFakeAgent($ndjsonLines, function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager); $result = $driver->run('Test prompt', $workspacePath, 'fake-api-key', 'fake-github-token'); @@ -296,7 +309,7 @@ protected function fetchGithubTokenOwnerProfile(string $githubToken): TokenOwner ]; runWithFakeAgent($ndjsonLines, function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager); $result = $driver->run('Test prompt', $workspacePath, 'fake-api-key', 'fake-github-token'); @@ -311,7 +324,7 @@ protected function fetchGithubTokenOwnerProfile(string $githubToken): TokenOwner runWithCustomFakeAgent( 'echo "A=${GIT_AUTHOR_NAME};AE=${GIT_AUTHOR_EMAIL};C=${GIT_COMMITTER_NAME};CE=${GIT_COMMITTER_EMAIL};GH=${GH_TOKEN};GT=${GITHUB_TOKEN}"', function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager, new TokenOwnerProfile( 42, 'octocat', @@ -336,7 +349,7 @@ function (string $workspacePath): void { runWithCustomFakeAgent( 'echo "A=${GIT_AUTHOR_NAME};AE=${GIT_AUTHOR_EMAIL};C=${GIT_COMMITTER_NAME};CE=${GIT_COMMITTER_EMAIL}"', function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager, new TokenOwnerProfile( 42, 'octocat', @@ -361,7 +374,7 @@ function (string $workspacePath): void { runWithCustomFakeAgent( 'echo "$@"', function (string $workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager); $result = $driver->run('Test prompt', $workspacePath, 'fake-api-key', 'fake-github-token'); @@ -379,7 +392,7 @@ function (string $workspacePath): void { withFakeDocker( 'echo "$@"', function () use ($workspacePath): void { - $binaryManager = new CursorCliBinaryManager(new NullLogger()); + $binaryManager = createCursorCliBinaryManagerForDriverTest(); $driver = buildDriver($binaryManager); $result = $driver->run('Test prompt', $workspacePath, 'fake-api-key', 'fake-github-token', 'test-container'); diff --git a/tests/Unit/PlanningAgent/PlanningRunExecutorTest.php b/tests/Unit/PlanningAgent/PlanningRunExecutorTest.php index 5d96bed..9542c91 100644 --- a/tests/Unit/PlanningAgent/PlanningRunExecutorTest.php +++ b/tests/Unit/PlanningAgent/PlanningRunExecutorTest.php @@ -491,6 +491,21 @@ public function releaseRunClaim(string $productConfigId, int $issueNumber, strin ]; } + public function executeWithRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId, callable $work): bool + { + if (!$this->tryStartRun($productConfigId, $issueNumber, $phase, $runId)) { + return false; + } + + try { + $work(); + + return true; + } finally { + $this->releaseRunClaim($productConfigId, $issueNumber, $runId); + } + } + public function clearRunClaimsForIssue(string $productConfigId, int $issueNumber): int { return 0; diff --git a/tests/Unit/TeamleaderAgent/ProcessProductConfigHandlerTest.php b/tests/Unit/TeamleaderAgent/ProcessProductConfigHandlerTest.php index 469e669..b9e2756 100644 --- a/tests/Unit/TeamleaderAgent/ProcessProductConfigHandlerTest.php +++ b/tests/Unit/TeamleaderAgent/ProcessProductConfigHandlerTest.php @@ -711,6 +711,21 @@ public function releaseRunClaim(string $productConfigId, int $issueNumber, strin { } + public function executeWithRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId, callable $work): bool + { + if (!$this->tryStartRun($productConfigId, $issueNumber, $phase, $runId)) { + return false; + } + + try { + $work(); + + return true; + } finally { + $this->releaseRunClaim($productConfigId, $issueNumber, $runId); + } + } + public function clearRunClaimsForIssue(string $productConfigId, int $issueNumber): int { return 0; diff --git a/tests/Unit/Workflow/ImplementationRunStarterTest.php b/tests/Unit/Workflow/ImplementationRunStarterTest.php index b6c3544..89e3256 100644 --- a/tests/Unit/Workflow/ImplementationRunStarterTest.php +++ b/tests/Unit/Workflow/ImplementationRunStarterTest.php @@ -10,12 +10,14 @@ use App\Workflow\Facade\Enum\WorkflowRunPhase; use App\Workflow\Facade\WorkflowConcurrencyFacadeInterface; use App\Workflow\Infrastructure\Service\ImplementationRunStarter; +use App\Workflow\Infrastructure\Service\PhaseRunStarter; describe('ImplementationRunStarter', function (): void { it('returns null when run claim cannot be created', function (): void { - $concurrency = new FakeWorkflowConcurrencyFacadeForImplementationRunStarter(false); - $github = new FakeGithubIntegrationFacadeForImplementationRunStarter(); - $starter = new ImplementationRunStarter($concurrency, $github); + $concurrency = new FakeWorkflowConcurrencyFacadeForImplementationRunStarter(false); + $github = new FakeGithubIntegrationFacadeForImplementationRunStarter(); + $phaseStarter = new PhaseRunStarter($concurrency, $github); + $starter = new ImplementationRunStarter($phaseStarter); $runId = $starter->startImplementation('https://github.com/acme/repo', 'token', 'cfg-1', 123); @@ -25,9 +27,10 @@ }); it('adds coordination and implementation labels when claim is created', function (): void { - $concurrency = new FakeWorkflowConcurrencyFacadeForImplementationRunStarter(true); - $github = new FakeGithubIntegrationFacadeForImplementationRunStarter(); - $starter = new ImplementationRunStarter($concurrency, $github); + $concurrency = new FakeWorkflowConcurrencyFacadeForImplementationRunStarter(true); + $github = new FakeGithubIntegrationFacadeForImplementationRunStarter(); + $phaseStarter = new PhaseRunStarter($concurrency, $github); + $starter = new ImplementationRunStarter($phaseStarter); $runId = $starter->startImplementation('https://github.com/acme/repo', 'token', 'cfg-1', 123); @@ -43,7 +46,8 @@ $concurrency = new FakeWorkflowConcurrencyFacadeForImplementationRunStarter(true); $github = new FakeGithubIntegrationFacadeForImplementationRunStarter(); $github->throwOnLabel = GithubLabel::ImplementationOngoing; - $starter = new ImplementationRunStarter($concurrency, $github); + $phaseStarter = new PhaseRunStarter($concurrency, $github); + $starter = new ImplementationRunStarter($phaseStarter); $invoke = static fn () => $starter->startImplementation('https://github.com/acme/repo', 'token', 'cfg-1', 123); @@ -94,6 +98,21 @@ public function releaseRunClaim(string $productConfigId, int $issueNumber, strin ]; } + public function executeWithRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId, callable $work): bool + { + if (!$this->tryStartRun($productConfigId, $issueNumber, $phase, $runId)) { + return false; + } + + try { + $work(); + + return true; + } finally { + $this->releaseRunClaim($productConfigId, $issueNumber, $runId); + } + } + public function clearRunClaimsForIssue(string $productConfigId, int $issueNumber): int { return 0; diff --git a/tests/Unit/Workflow/PlanningRunStarterTest.php b/tests/Unit/Workflow/PlanningRunStarterTest.php index 186b8eb..12e537a 100644 --- a/tests/Unit/Workflow/PlanningRunStarterTest.php +++ b/tests/Unit/Workflow/PlanningRunStarterTest.php @@ -9,13 +9,15 @@ use App\GithubIntegration\Facade\GithubIntegrationFacadeInterface; use App\Workflow\Facade\Enum\WorkflowRunPhase; use App\Workflow\Facade\WorkflowConcurrencyFacadeInterface; +use App\Workflow\Infrastructure\Service\PhaseRunStarter; use App\Workflow\Infrastructure\Service\PlanningRunStarter; describe('PlanningRunStarter', function (): void { it('returns null when run claim cannot be created', function (): void { - $concurrency = new FakeWorkflowConcurrencyFacadeForPlanningRunStarter(false); - $github = new FakeGithubIntegrationFacadeForPlanningRunStarter(); - $starter = new PlanningRunStarter($concurrency, $github); + $concurrency = new FakeWorkflowConcurrencyFacadeForPlanningRunStarter(false); + $github = new FakeGithubIntegrationFacadeForPlanningRunStarter(); + $phaseStarter = new PhaseRunStarter($concurrency, $github); + $starter = new PlanningRunStarter($phaseStarter); $runId = $starter->startPlanning('https://github.com/acme/repo', 'token', 'cfg-1', 123); @@ -25,9 +27,10 @@ }); it('adds coordination and planning labels when claim is created', function (): void { - $concurrency = new FakeWorkflowConcurrencyFacadeForPlanningRunStarter(true); - $github = new FakeGithubIntegrationFacadeForPlanningRunStarter(); - $starter = new PlanningRunStarter($concurrency, $github); + $concurrency = new FakeWorkflowConcurrencyFacadeForPlanningRunStarter(true); + $github = new FakeGithubIntegrationFacadeForPlanningRunStarter(); + $phaseStarter = new PhaseRunStarter($concurrency, $github); + $starter = new PlanningRunStarter($phaseStarter); $runId = $starter->startPlanning('https://github.com/acme/repo', 'token', 'cfg-1', 123); @@ -43,7 +46,8 @@ $concurrency = new FakeWorkflowConcurrencyFacadeForPlanningRunStarter(true); $github = new FakeGithubIntegrationFacadeForPlanningRunStarter(); $github->throwOnLabel = GithubLabel::PlanningOngoing; - $starter = new PlanningRunStarter($concurrency, $github); + $phaseStarter = new PhaseRunStarter($concurrency, $github); + $starter = new PlanningRunStarter($phaseStarter); $invoke = static fn () => $starter->startPlanning('https://github.com/acme/repo', 'token', 'cfg-1', 123); @@ -94,6 +98,21 @@ public function releaseRunClaim(string $productConfigId, int $issueNumber, strin ]; } + public function executeWithRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId, callable $work): bool + { + if (!$this->tryStartRun($productConfigId, $issueNumber, $phase, $runId)) { + return false; + } + + try { + $work(); + + return true; + } finally { + $this->releaseRunClaim($productConfigId, $issueNumber, $runId); + } + } + public function clearRunClaimsForIssue(string $productConfigId, int $issueNumber): int { return 0; diff --git a/tests/Unit/Workflow/RunImplementationAgentCommandTest.php b/tests/Unit/Workflow/RunImplementationAgentCommandTest.php index 40ef8bc..49a5e50 100644 --- a/tests/Unit/Workflow/RunImplementationAgentCommandTest.php +++ b/tests/Unit/Workflow/RunImplementationAgentCommandTest.php @@ -504,6 +504,21 @@ public function releaseRunClaim(string $productConfigId, int $issueNumber, strin { } + public function executeWithRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId, callable $work): bool + { + if (!$this->tryStartRun($productConfigId, $issueNumber, $phase, $runId)) { + return false; + } + + try { + $work(); + + return true; + } finally { + $this->releaseRunClaim($productConfigId, $issueNumber, $runId); + } + } + public function clearRunClaimsForIssue(string $productConfigId, int $issueNumber): int { ++$this->clearClaimsCalls; diff --git a/tests/Unit/Workflow/RunPlanningAgentCommandTest.php b/tests/Unit/Workflow/RunPlanningAgentCommandTest.php index 7dd29ea..6d90399 100644 --- a/tests/Unit/Workflow/RunPlanningAgentCommandTest.php +++ b/tests/Unit/Workflow/RunPlanningAgentCommandTest.php @@ -501,6 +501,21 @@ public function releaseRunClaim(string $productConfigId, int $issueNumber, strin { } + public function executeWithRunClaim(string $productConfigId, int $issueNumber, WorkflowRunPhase $phase, string $runId, callable $work): bool + { + if (!$this->tryStartRun($productConfigId, $issueNumber, $phase, $runId)) { + return false; + } + + try { + $work(); + + return true; + } finally { + $this->releaseRunClaim($productConfigId, $issueNumber, $runId); + } + } + public function clearRunClaimsForIssue(string $productConfigId, int $issueNumber): int { ++$this->clearClaimsCalls; diff --git a/tests/Unit/WorkspaceManagement/GitCloneServiceTest.php b/tests/Unit/WorkspaceManagement/GitCloneServiceTest.php index 2257da8..5adabc3 100644 --- a/tests/Unit/WorkspaceManagement/GitCloneServiceTest.php +++ b/tests/Unit/WorkspaceManagement/GitCloneServiceTest.php @@ -5,6 +5,7 @@ use App\WorkspaceManagement\Infrastructure\Service\GitCloneService; use App\WorkspaceManagement\Infrastructure\Service\ProjectContainerManager; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceMetadataManager; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceRemover; use Psr\Log\NullLogger; use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\Lock\LockFactory; @@ -57,12 +58,14 @@ $run(['git', '-C', $sourcePath, 'push', '-u', 'origin', 'main'], $tempRoot); $run(['git', '-C', $remotePath, 'symbolic-ref', 'HEAD', 'refs/heads/main'], $tempRoot); - $service = new GitCloneService( + $containerManager = new ProjectContainerManager(new LockFactory(new InMemoryStore()), new NullLogger()); + $service = new GitCloneService( $fs, - new ProjectContainerManager(new LockFactory(new InMemoryStore()), new NullLogger()), + $containerManager, new WorkspaceMetadataManager(), new LockFactory(new FlockStore($locksDir)), new NullLogger(), + new WorkspaceRemover($containerManager, $fs, new NullLogger()), ); $workspaceInfo = $service->acquireWorkspace('cfg-1', 1, $remotePath, 'token'); diff --git a/tests/Unit/WorkspaceManagement/WorkspaceCleanupCommandTest.php b/tests/Unit/WorkspaceManagement/WorkspaceCleanupCommandTest.php index 7369d0a..b02d706 100644 --- a/tests/Unit/WorkspaceManagement/WorkspaceCleanupCommandTest.php +++ b/tests/Unit/WorkspaceManagement/WorkspaceCleanupCommandTest.php @@ -14,6 +14,7 @@ use App\WorkspaceManagement\Infrastructure\Service\GitCloneService; use App\WorkspaceManagement\Infrastructure\Service\ProjectContainerManager; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceMetadataManager; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceRemovalInterface; use EnterpriseToolingForSymfony\SharedBundle\DateAndTime\Service\DateAndTimeService; use Psr\Log\NullLogger; use Symfony\Component\Console\Tester\CommandTester; @@ -135,6 +136,15 @@ final class FakeGitCloneServiceForCleanup extends GitCloneService public function __construct( private string $workspaceBaseDir, ) { + $fs = new Filesystem(); + parent::__construct( + $fs, + new ProjectContainerManager(new LockFactory(new InMemoryStore()), new NullLogger()), + new WorkspaceMetadataManager(), + new LockFactory(new InMemoryStore()), + new NullLogger(), + new NoOpWorkspaceRemoverForCleanupTest(), + ); } public function resolveWorkspaceBaseDir(): string @@ -329,3 +339,10 @@ public function requestPullRequestReviewer(string $githubUrl, string $githubToke { } } + +final class NoOpWorkspaceRemoverForCleanupTest implements WorkspaceRemovalInterface +{ + public function removeWorkspace(string $workspacePath, ?string $containerName, string $workspaceBaseDir): void + { + } +} diff --git a/tests/Unit/WorkspaceManagement/WorkspaceInfoLookupServiceTest.php b/tests/Unit/WorkspaceManagement/WorkspaceInfoLookupServiceTest.php index 16d2412..7282156 100644 --- a/tests/Unit/WorkspaceManagement/WorkspaceInfoLookupServiceTest.php +++ b/tests/Unit/WorkspaceManagement/WorkspaceInfoLookupServiceTest.php @@ -5,9 +5,14 @@ use App\ProductConfig\Facade\Dto\ProductConfigDto; use App\ProductConfig\Facade\ProductConfigFacadeInterface; use App\WorkspaceManagement\Infrastructure\Service\GitCloneService; +use App\WorkspaceManagement\Infrastructure\Service\ProjectContainerManager; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceInfoLookupService; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceMetadataManager; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceRemovalInterface; +use Psr\Log\NullLogger; use Symfony\Component\Filesystem\Filesystem; +use Symfony\Component\Lock\LockFactory; +use Symfony\Component\Lock\Store\InMemoryStore; describe('WorkspaceInfoLookupService', function (): void { it('returns enriched matches for workspace id prefix', function (): void { @@ -53,6 +58,15 @@ final class FakeGitCloneServiceForWorkspaceInfoLookup extends GitCloneService public function __construct( private readonly string $workspaceBaseDir, ) { + $fs = new Filesystem(); + parent::__construct( + $fs, + new ProjectContainerManager(new LockFactory(new InMemoryStore()), new NullLogger()), + new WorkspaceMetadataManager(), + new LockFactory(new InMemoryStore()), + new NullLogger(), + new NoOpWorkspaceRemoverForLookupTest(), + ); } public function resolveWorkspaceBaseDir(): string @@ -92,3 +106,10 @@ public function findMatchingProductConfigs(string $githubOwner, string $githubRe return []; } } + +final class NoOpWorkspaceRemoverForLookupTest implements WorkspaceRemovalInterface +{ + public function removeWorkspace(string $workspacePath, ?string $containerName, string $workspaceBaseDir): void + { + } +} diff --git a/tests/Unit/WorkspaceManagement/WorkspaceManagementFacadeTest.php b/tests/Unit/WorkspaceManagement/WorkspaceManagementFacadeTest.php index 862ba99..d9035c3 100644 --- a/tests/Unit/WorkspaceManagement/WorkspaceManagementFacadeTest.php +++ b/tests/Unit/WorkspaceManagement/WorkspaceManagementFacadeTest.php @@ -7,18 +7,24 @@ use App\WorkspaceManagement\Facade\WorkspaceManagementFacade; use App\WorkspaceManagement\Infrastructure\Service\GitCloneService; use App\WorkspaceManagement\Infrastructure\Service\ProcessWorkspaceCleanupCommandRunner; +use App\WorkspaceManagement\Infrastructure\Service\ProjectContainerManager; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceAcquisitionInterface; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceCleanupCommandResult; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceCleanupCommandRunnerInterface; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceInfoLookupService; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceMetadataManager; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceRemovalInterface; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceSweepCleanupService; use Psr\Log\NullLogger; use Symfony\Component\Filesystem\Filesystem; +use Symfony\Component\Lock\LockFactory; +use Symfony\Component\Lock\Store\InMemoryStore; describe('WorkspaceManagementFacade', function (): void { - it('delegates createWorkspace to GitCloneService', function (): void { + it('delegates createWorkspace to workspace acquisition (interface)', function (): void { $expectedInfo = new WorkspaceInfoDto('/var/tmp/productbuilder-workspaces/abc123', null); - $gitCloneService = new class($expectedInfo) extends GitCloneService { + $acquisition = new class($expectedInfo) implements WorkspaceAcquisitionInterface { public string $receivedUrl = ''; public string $receivedToken = ''; @@ -27,6 +33,24 @@ public function __construct( ) { } + public function acquireWorkspace( + string $productConfigId, + int $issueNumber, + string $githubUrl, + string $githubToken, + ?string $branchName = null, + ): WorkspaceInfoDto { + return $this->infoToReturn; + } + + public function releaseWorkspace(string $workspacePath): void + { + } + + public function removeWorkspace(string $workspacePath, ?string $containerName): void + { + } + public function cloneRepository(string $githubUrl, string $githubToken): WorkspaceInfoDto { $this->receivedUrl = $githubUrl; @@ -35,19 +59,36 @@ public function cloneRepository(string $githubUrl, string $githubToken): Workspa return $this->infoToReturn; } - public function removeWorkspace(string $workspacePath, ?string $containerName): void + public function cloneRepositoryOnBranch(string $githubUrl, string $githubToken, string $branchName): WorkspaceInfoDto + { + return $this->infoToReturn; + } + + public function resolveWorkspaceBaseDir(): string { + return '/var/tmp'; + } + + public function deriveWorkspaceId(string $productConfigId, int $issueNumber): string + { + return 'derived'; + } + + public function repoPath(string $workspacePath): string + { + return $workspacePath . '/repo'; } }; + $sweepService = new WorkspaceSweepCleanupService( + $acquisition, + new Filesystem(), + new ProcessWorkspaceCleanupCommandRunner(), + new NullLogger(), + ); $facade = new WorkspaceManagementFacade( - $gitCloneService, - new WorkspaceSweepCleanupService( - $gitCloneService, - new Filesystem(), - new ProcessWorkspaceCleanupCommandRunner(), - new NullLogger(), - ), + $acquisition, + $sweepService, new class extends WorkspaceInfoLookupService { public function __construct() { @@ -60,8 +101,8 @@ public function __construct() expect($result->workspacePath)->toBe('/var/tmp/productbuilder-workspaces/abc123'); expect($result->repoPath)->toBe('/var/tmp/productbuilder-workspaces/abc123/repo'); expect($result->containerName)->toBeNull(); - expect($gitCloneService->receivedUrl)->toBe('https://github.com/owner/repo'); - expect($gitCloneService->receivedToken)->toBe('ghp_token'); + expect($acquisition->receivedUrl)->toBe('https://github.com/owner/repo'); + expect($acquisition->receivedToken)->toBe('ghp_token'); }); it('delegates createWorkspace and returns container name when set', function (): void { @@ -110,6 +151,15 @@ public function __construct() public function __construct() { + $fs = new Filesystem(); + parent::__construct( + $fs, + new ProjectContainerManager(new LockFactory(new InMemoryStore()), new NullLogger()), + new WorkspaceMetadataManager(), + new LockFactory(new InMemoryStore()), + new NullLogger(), + new NoOpWorkspaceRemoverForFacadeTest(), + ); } public function cloneRepository(string $githubUrl, string $githubToken): WorkspaceInfoDto @@ -151,6 +201,15 @@ public function __construct() public function __construct() { + $fs = new Filesystem(); + parent::__construct( + $fs, + new ProjectContainerManager(new LockFactory(new InMemoryStore()), new NullLogger()), + new WorkspaceMetadataManager(), + new LockFactory(new InMemoryStore()), + new NullLogger(), + new NoOpWorkspaceRemoverForFacadeTest(), + ); } public function cloneRepository(string $githubUrl, string $githubToken): WorkspaceInfoDto @@ -189,6 +248,15 @@ public function __construct() $gitCloneService = new class extends GitCloneService { public function __construct() { + $fs = new Filesystem(); + parent::__construct( + $fs, + new ProjectContainerManager(new LockFactory(new InMemoryStore()), new NullLogger()), + new WorkspaceMetadataManager(), + new LockFactory(new InMemoryStore()), + new NullLogger(), + new NoOpWorkspaceRemoverForFacadeTest(), + ); } }; @@ -265,3 +333,10 @@ public function findByWorkspaceIdPrefix(string $workspaceIdPrefix): array expect($lookup[0]->issueNumber)->toBe(42); }); }); + +final class NoOpWorkspaceRemoverForFacadeTest implements WorkspaceRemovalInterface +{ + public function removeWorkspace(string $workspacePath, ?string $containerName, string $workspaceBaseDir): void + { + } +} diff --git a/tests/Unit/WorkspaceManagement/WorkspaceRemoverTest.php b/tests/Unit/WorkspaceManagement/WorkspaceRemoverTest.php new file mode 100644 index 0000000..07bab71 --- /dev/null +++ b/tests/Unit/WorkspaceManagement/WorkspaceRemoverTest.php @@ -0,0 +1,160 @@ +removeWorkspace($workspacePath, null, $baseDir); + + expect(is_dir($workspacePath))->toBeFalse(); + expect(is_dir($baseDir))->toBeTrue(); + + (new Filesystem())->remove($baseDir); + }); + + it('calls stopProjectContainer when container name is given', function (): void { + $baseDir = sys_get_temp_dir() . '/pb-remover-test-' . bin2hex(random_bytes(4)); + $workspacePath = $baseDir . '/ws-456'; + mkdir($workspacePath, 0o755, true); + + $capture = new stdClass(); + $capture->container = null; + $capture->path = null; + $containerManager = new class($capture) implements ProjectContainerManagerInterface { + public function __construct( + private readonly stdClass $capture, + ) { + } + + public function hasProjectContainerConfig(string $workspacePath): bool + { + return false; + } + + public function startProjectContainer(string $workspacePath, string $workspaceId): string + { + return ''; + } + + public function isContainerRunning(string $containerName): bool + { + return false; + } + + public function stopProjectContainer(string $containerName, string $workspacePath): void + { + $this->capture->container = $containerName; + $this->capture->path = $workspacePath; + } + }; + + $remover = new WorkspaceRemover($containerManager, new Filesystem(), new NullLogger()); + $remover->removeWorkspace($workspacePath, 'pb-workspace-456', $baseDir); + + expect($capture->container)->toBe('pb-workspace-456'); + expect($capture->path)->toBe($workspacePath); + expect(is_dir($workspacePath))->toBeFalse(); + + (new Filesystem())->remove($baseDir); + }); + + it('throws when workspace path is outside base directory', function (): void { + $baseDir = sys_get_temp_dir() . '/pb-remover-base-' . bin2hex(random_bytes(4)); + $outsidePath = sys_get_temp_dir() . '/pb-remover-outside-' . bin2hex(random_bytes(4)); + mkdir($baseDir, 0o755, true); + mkdir($outsidePath, 0o755, true); + + $containerManager = new class implements ProjectContainerManagerInterface { + public function hasProjectContainerConfig(string $workspacePath): bool + { + return false; + } + + public function startProjectContainer(string $workspacePath, string $workspaceId): string + { + return ''; + } + + public function isContainerRunning(string $containerName): bool + { + return false; + } + + public function stopProjectContainer(string $containerName, string $workspacePath): void + { + } + }; + + $remover = new WorkspaceRemover($containerManager, new Filesystem(), new NullLogger()); + + expect(fn () => $remover->removeWorkspace($outsidePath, null, $baseDir)) + ->toThrow(RuntimeException::class, 'Refusing to remove path outside workspace base directory'); + + (new Filesystem())->remove([$baseDir, $outsidePath]); + }); + + it('does nothing when workspace path already does not exist', function (): void { + $baseDir = sys_get_temp_dir() . '/pb-remover-base-' . bin2hex(random_bytes(4)); + $nonExistentPath = $baseDir . '/does-not-exist'; + mkdir($baseDir, 0o755, true); + + $containerManager = new class implements ProjectContainerManagerInterface { + public function hasProjectContainerConfig(string $workspacePath): bool + { + return false; + } + + public function startProjectContainer(string $workspacePath, string $workspaceId): string + { + return ''; + } + + public function isContainerRunning(string $containerName): bool + { + return false; + } + + public function stopProjectContainer(string $containerName, string $workspacePath): void + { + } + }; + + $remover = new WorkspaceRemover($containerManager, new Filesystem(), new NullLogger()); + $remover->removeWorkspace($nonExistentPath, null, $baseDir); + + expect(is_dir($baseDir))->toBeTrue(); + (new Filesystem())->remove($baseDir); + }); +}); diff --git a/tests/Unit/WorkspaceManagement/WorkspaceSweepCleanupServiceTest.php b/tests/Unit/WorkspaceManagement/WorkspaceSweepCleanupServiceTest.php index 558c20e..994e4da 100644 --- a/tests/Unit/WorkspaceManagement/WorkspaceSweepCleanupServiceTest.php +++ b/tests/Unit/WorkspaceManagement/WorkspaceSweepCleanupServiceTest.php @@ -3,11 +3,16 @@ declare(strict_types=1); use App\WorkspaceManagement\Infrastructure\Service\GitCloneService; +use App\WorkspaceManagement\Infrastructure\Service\ProjectContainerManager; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceCleanupCommandResult; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceCleanupCommandRunnerInterface; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceMetadataManager; +use App\WorkspaceManagement\Infrastructure\Service\WorkspaceRemovalInterface; use App\WorkspaceManagement\Infrastructure\Service\WorkspaceSweepCleanupService; use Psr\Log\NullLogger; use Symfony\Component\Filesystem\Filesystem; +use Symfony\Component\Lock\LockFactory; +use Symfony\Component\Lock\Store\InMemoryStore; describe('WorkspaceSweepCleanupService', function (): void { it('removes only matching containers, targeted images, and workspace directories', function (): void { @@ -122,6 +127,15 @@ public function __construct( private readonly string $workspaceBase, private readonly string $workspaceId, ) { + $fs = new Filesystem(); + parent::__construct( + $fs, + new ProjectContainerManager(new LockFactory(new InMemoryStore()), new NullLogger()), + new WorkspaceMetadataManager(), + new LockFactory(new InMemoryStore()), + new NullLogger(), + new NoOpWorkspaceRemoverForSweepTest(), + ); } public function deriveWorkspaceId(string $productConfigId, int $issueNumber): string @@ -134,3 +148,10 @@ public function resolveWorkspaceBaseDir(): string return $this->workspaceBase; } } + +final class NoOpWorkspaceRemoverForSweepTest implements WorkspaceRemovalInterface +{ + public function removeWorkspace(string $workspacePath, ?string $containerName, string $workspaceBaseDir): void + { + } +}