fix: harden workspace and agent concurrency safeguards

Add distributed workspace/cache locking and atomic workspace/runtime/metadata updates to prevent corruption during parallel runs. Improve GitHub API resilience and stale-label recovery so failed workers do not leave issues permanently wedged.

Author: manuelkiessling

.github/workflows/ci.yml

@@ -9,6 +9,10 @@ on:
         branches:
             - "**"
 
+concurrency:
+    group: ${{ github.workflow }}-${{ github.ref }}
+    cancel-in-progress: true
+
 env:
     ETFS_PROJECT_NAME: ci
     APP_IMAGE: ghcr.io/${{ github.repository }}/app:ci

docs/devbook.md

@@ -5,22 +5,22 @@ How do I solve recurring tasks and problems during development?
 
 ## Building the frontend
 
-- `bash bin/build-frontend.sh`
+- `mise run frontend`
 
 
 ## Updating all dependencies
 
-- `composer update --with-dependencies`
-- `nvm use && npm update`
-- `php bin/console importmap:update`
+- PHP deps: `mise run composer update --with-dependencies`
+- Node deps: `mise run npm update` (or `mise run npm install`)
+- Importmap: `mise run console importmap:update`
 
 
 ## Changing the database schema with migrations
 
 - Create new or edit existing entities
-- Run `php bin/console make:migration`
+- Run `mise run console make:migration`
 
 
 ## Connect to the local database
 
-- `bash bin/connect-to-db.sh`
+- `mise run db`

src/GithubIntegration/Infrastructure/Service/GithubApiClient.php

@@ -10,12 +10,20 @@
 use DateTimeImmutable;
 use Psr\Log\LoggerInterface;
 use RuntimeException;
+use Symfony\Contracts\HttpClient\Exception\ClientExceptionInterface;
+use Symfony\Contracts\HttpClient\Exception\ServerExceptionInterface;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 use Symfony\Contracts\HttpClient\ResponseInterface;
 
 class GithubApiClient
 {
-    private const string BASE_URL = 'https://api.github.com';
+    private const string BASE_URL                    = 'https://api.github.com';
+    private const float DEFAULT_TIMEOUT_SECONDS      = 15.0;
+    private const float DEFAULT_MAX_DURATION_SECONDS = 30.0;
+    private const int DEFAULT_GET_RETRY_ATTEMPTS     = 3;
+    private const int RETRY_BASE_DELAY_MS            = 200;
+    private const int RETRY_MAX_DELAY_MS             = 2_000;
 
     public function __construct(
         private readonly HttpClientInterface $httpClient,
@@ -32,7 +40,8 @@ public function getIssuesWithLabel(string $owner, string $repo, string $token, s
         $page   = 1;
 
         do {
-            $response = $this->request('GET', "/repos/{$owner}/{$repo}/issues", $token, [
+            /** @var list<array<string, mixed>> $data */
+            $data = $this->requestJsonWithRetry('GET', "/repos/{$owner}/{$repo}/issues", $token, [
                 'query' => [
                     'labels'   => $labelName,
                     'state'    => 'open',
@@ -41,9 +50,6 @@ public function getIssuesWithLabel(string $owner, string $repo, string $token, s
                 ],
             ]);
 
-            /** @var list<array<string, mixed>> $data */
-            $data = $response->toArray();
-
             foreach ($data as $issueData) {
                 if (array_key_exists('pull_request', $issueData)) {
                     continue;
@@ -74,16 +80,14 @@ public function getIssueComments(string $owner, string $repo, string $token, int
         $page     = 1;
 
         do {
-            $response = $this->request('GET', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/comments", $token, [
+            /** @var list<array<string, mixed>> $data */
+            $data = $this->requestJsonWithRetry('GET', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/comments", $token, [
                 'query' => [
                     'per_page' => 100,
                     'page'     => $page,
                 ],
             ]);
 
-            /** @var list<array<string, mixed>> $data */
-            $data = $response->toArray();
-
             foreach ($data as $commentData) {
                 /** @var array<string, mixed>|null $userData */
                 $userData = $commentData['user'] ?? null;
@@ -104,9 +108,10 @@ public function getIssueComments(string $owner, string $repo, string $token, int
 
     public function addLabelToIssue(string $owner, string $repo, string $token, int $issueNumber, string $labelName): void
     {
-        $this->request('POST', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/labels", $token, [
+        $response = $this->request('POST', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/labels", $token, [
             'json' => ['labels' => [$labelName]],
         ]);
+        $response->getContent();
 
         $this->logger->info('Added label to issue', [
             'owner' => $owner,
@@ -121,15 +126,16 @@ public function removeLabelFromIssue(string $owner, string $repo, string $token,
         $encodedLabel = rawurlencode($labelName);
 
         try {
-            $this->request('DELETE', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/labels/{$encodedLabel}", $token);
+            $response = $this->request('DELETE', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/labels/{$encodedLabel}", $token);
+            $response->getContent();
 
             $this->logger->info('Removed label from issue', [
                 'owner' => $owner,
                 'repo'  => $repo,
                 'issue' => $issueNumber,
                 'label' => $labelName,
             ]);
-        } catch (\Symfony\Component\HttpClient\Exception\ClientException $e) {
+        } catch (ClientExceptionInterface $e) {
             if ($e->getResponse()->getStatusCode() === 404) {
                 $this->logger->debug('Label not present on issue, nothing to remove', [
                     'issue' => $issueNumber,
@@ -146,20 +152,21 @@ public function removeLabelFromIssue(string $owner, string $repo, string $token,
     public function createLabel(string $owner, string $repo, string $token, string $labelName, string $color, string $description = ''): void
     {
         try {
-            $this->request('POST', "/repos/{$owner}/{$repo}/labels", $token, [
+            $response = $this->request('POST', "/repos/{$owner}/{$repo}/labels", $token, [
                 'json' => [
                     'name'        => $labelName,
                     'color'       => $color,
                     'description' => $description,
                 ],
             ]);
+            $response->getContent();
 
             $this->logger->info('Created label', [
                 'owner' => $owner,
                 'repo'  => $repo,
                 'label' => $labelName,
             ]);
-        } catch (\Symfony\Component\HttpClient\Exception\ClientException $e) {
+        } catch (ClientExceptionInterface $e) {
             if ($e->getResponse()->getStatusCode() === 422) {
                 $this->logger->debug('Label already exists', [
                     'owner' => $owner,
@@ -176,9 +183,10 @@ public function createLabel(string $owner, string $repo, string $token, string $
 
     public function postIssueComment(string $owner, string $repo, string $token, int $issueNumber, string $body): void
     {
-        $this->request('POST', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/comments", $token, [
+        $response = $this->request('POST', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/comments", $token, [
             'json' => ['body' => $body],
         ]);
+        $response->getContent();
 
         $this->logger->info('Posted comment on issue', [
             'owner'      => $owner,
@@ -190,10 +198,8 @@ public function postIssueComment(string $owner, string $repo, string $token, int
 
     public function getAuthenticatedUserLogin(string $token): string
     {
-        $response = $this->request('GET', '/user', $token);
-
         /** @var array<string, mixed> $data */
-        $data = $response->toArray();
+        $data = $this->requestJsonWithRetry('GET', '/user', $token);
 
         return $this->stringVal($data, 'login');
     }
@@ -204,17 +210,15 @@ public function findOpenPullRequestForIssue(string $owner, string $repo, string
         $page          = 1;
 
         do {
-            $response = $this->request('GET', "/repos/{$owner}/{$repo}/pulls", $token, [
+            /** @var list<array<string, mixed>> $data */
+            $data = $this->requestJsonWithRetry('GET', "/repos/{$owner}/{$repo}/pulls", $token, [
                 'query' => [
                     'state'    => 'open',
                     'per_page' => 100,
                     'page'     => $page,
                 ],
             ]);
 
-            /** @var list<array<string, mixed>> $data */
-            $data = $response->toArray();
-
             foreach ($data as $prData) {
                 $body = $this->stringVal($prData, 'body');
                 if (str_contains($body, $closesPattern)) {
@@ -247,16 +251,14 @@ public function getPullRequestReviewComments(string $owner, string $repo, string
         $page     = 1;
 
         do {
-            $response = $this->request('GET', "/repos/{$owner}/{$repo}/pulls/{$prNumber}/comments", $token, [
+            /** @var list<array<string, mixed>> $data */
+            $data = $this->requestJsonWithRetry('GET', "/repos/{$owner}/{$repo}/pulls/{$prNumber}/comments", $token, [
                 'query' => [
                     'per_page' => 100,
                     'page'     => $page,
                 ],
             ]);
 
-            /** @var list<array<string, mixed>> $data */
-            $data = $response->toArray();
-
             foreach ($data as $commentData) {
                 /** @var array<string, mixed>|null $userData */
                 $userData = $commentData['user'] ?? null;
@@ -281,16 +283,14 @@ public function getPullRequestLatestCommitCommittedAt(string $owner, string $rep
         $page              = 1;
 
         do {
-            $response = $this->request('GET', "/repos/{$owner}/{$repo}/pulls/{$prNumber}/commits", $token, [
+            /** @var list<array<string, mixed>> $data */
+            $data = $this->requestJsonWithRetry('GET', "/repos/{$owner}/{$repo}/pulls/{$prNumber}/commits", $token, [
                 'query' => [
                     'per_page' => 100,
                     'page'     => $page,
                 ],
             ]);
 
-            /** @var list<array<string, mixed>> $data */
-            $data = $response->toArray();
-
             foreach ($data as $commitData) {
                 /** @var array<string, mixed>|null $commitMetadata */
                 $commitMetadata = $commitData['commit'] ?? null;
@@ -324,18 +324,16 @@ public function getPullRequestLatestCommitCommittedAt(string $owner, string $rep
     public function getIssueByNumber(string $owner, string $repo, string $token, int $issueNumber): ?RawGithubIssue
     {
         try {
-            $response = $this->request('GET', "/repos/{$owner}/{$repo}/issues/{$issueNumber}", $token);
-        } catch (\Symfony\Component\HttpClient\Exception\ClientException $e) {
+            /** @var array<string, mixed> $data */
+            $data = $this->requestJsonWithRetry('GET', "/repos/{$owner}/{$repo}/issues/{$issueNumber}", $token);
+        } catch (ClientExceptionInterface $e) {
             if ($e->getResponse()->getStatusCode() === 404) {
                 return null;
             }
 
             throw $e;
         }
 
-        /** @var array<string, mixed> $data */
-        $data = $response->toArray();
-
         if (array_key_exists('pull_request', $data)) {
             return null;
         }
@@ -350,10 +348,8 @@ public function getIssueByNumber(string $owner, string $repo, string $token, int
 
     public function getIssueState(string $owner, string $repo, string $token, int $issueNumber): string
     {
-        $response = $this->request('GET', "/repos/{$owner}/{$repo}/issues/{$issueNumber}", $token);
-
         /** @var array<string, mixed> $data */
-        $data = $response->toArray();
+        $data = $this->requestJsonWithRetry('GET', "/repos/{$owner}/{$repo}/issues/{$issueNumber}", $token);
 
         return $this->stringVal($data, 'state');
     }
@@ -364,16 +360,14 @@ public function getLabelAddedAt(string $owner, string $repo, string $token, int
         $page          = 1;
 
         do {
-            $response = $this->request('GET', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/events", $token, [
+            /** @var list<array<string, mixed>> $data */
+            $data = $this->requestJsonWithRetry('GET', "/repos/{$owner}/{$repo}/issues/{$issueNumber}/events", $token, [
                 'query' => [
                     'per_page' => 100,
                     'page'     => $page,
                 ],
             ]);
 
-            /** @var list<array<string, mixed>> $data */
-            $data = $response->toArray();
-
             foreach ($data as $event) {
                 $eventType = $this->stringVal($event, 'event');
                 /** @var array<string, mixed>|null $labelData */
@@ -469,6 +463,14 @@ private function parseDateTimeImmutable(string $isoDate): DateTimeImmutable
      */
     private function request(string $method, string $path, string $token, array $options = []): ResponseInterface
     {
+        if (!array_key_exists('timeout', $options)) {
+            $options['timeout'] = self::DEFAULT_TIMEOUT_SECONDS;
+        }
+
+        if (!array_key_exists('max_duration', $options)) {
+            $options['max_duration'] = self::DEFAULT_MAX_DURATION_SECONDS;
+        }
+
         /** @var array<string, string> $existingHeaders */
         $existingHeaders    = is_array($options['headers'] ?? null) ? $options['headers'] : [];
         $options['headers'] = array_merge($existingHeaders, [
@@ -479,4 +481,89 @@ private function request(string $method, string $path, string $token, array $opt
 
         return $this->httpClient->request($method, self::BASE_URL . $path, $options);
     }
+
+    /**
+     * @param array<string, mixed> $options
+     *
+     * @return array<mixed>
+     */
+    private function requestJsonWithRetry(string $method, string $path, string $token, array $options = []): array
+    {
+        $maxAttempts = strtoupper($method) === 'GET' ? self::DEFAULT_GET_RETRY_ATTEMPTS : 1;
+        $attempt     = 0;
+
+        while (true) {
+            ++$attempt;
+
+            try {
+                $response = $this->request($method, $path, $token, $options);
+
+                /** @var array<mixed> $data */
+                $data = $response->toArray();
+
+                return $data;
+            } catch (TransportExceptionInterface $e) {
+                if ($attempt >= $maxAttempts) {
+                    throw $e;
+                }
+
+                $this->logger->warning('GitHub API transport error, retrying', [
+                    'method'  => $method,
+                    'path'    => $path,
+                    'attempt' => $attempt,
+                    'error'   => $e->getMessage(),
+                ]);
+
+                $this->sleepBeforeRetry($attempt);
+            } catch (ServerExceptionInterface $e) {
+                if ($attempt >= $maxAttempts) {
+                    throw $e;
+                }
+
+                $this->logger->warning('GitHub API server error, retrying', [
+                    'method'     => $method,
+                    'path'       => $path,
+                    'attempt'    => $attempt,
+                    'statusCode' => $e->getResponse()->getStatusCode(),
+                    'error'      => $e->getMessage(),
+                ]);
+
+                $this->sleepBeforeRetry($attempt);
+            } catch (ClientExceptionInterface $e) {
+                $statusCode = $e->getResponse()->getStatusCode();
+
+                if ($attempt < $maxAttempts && $this->shouldRetryClientStatusCode($statusCode)) {
+                    $this->logger->warning('GitHub API client error, retrying', [
+                        'method'     => $method,
+                        'path'       => $path,
+                        'attempt'    => $attempt,
+                        'statusCode' => $statusCode,
+                        'error'      => $e->getMessage(),
+                    ]);
+
+                    $this->sleepBeforeRetry($attempt);
+                    continue;
+                }
+
+                throw $e;
+            }
+        }
+    }
+
+    private function shouldRetryClientStatusCode(int $statusCode): bool
+    {
+        return $statusCode === 429;
+    }
+
+    private function sleepBeforeRetry(int $attempt): void
+    {
+        $delayMs = (int) min(
+            self::RETRY_MAX_DELAY_MS,
+            self::RETRY_BASE_DELAY_MS * (2 ** ($attempt - 1)),
+        );
+
+        $jitterMs = random_int(0, max(1, (int) ($delayMs * 0.2)));
+
+        usleep(($delayMs + $jitterMs) * 1000);
+    }
 }