From cbbc254b552adc8487772455ae0699f911fb2f3e Mon Sep 17 00:00:00 2001 From: Michael Aerni Date: Thu, 14 May 2026 12:00:03 +0200 Subject: [PATCH 1/6] Prevent overlapping CommitJob dispatches Mark CommitJob as ShouldBeUnique so concurrent dispatches collapse to a single queued job. Without this, multiple workers running CommitJobs against the same repo race on `git add` (producing `index.lock: File exists` errors) and on `git push` (producing non-fast-forward rejections because each worker's push uses a stale view of origin). The unique lock TTL is 120s, comfortably outlasting the default queue worker timeout so a hard worker crash recovers within 2 minutes. --- src/Git/CommitJob.php | 8 +++++++- tests/Git/GitTest.php | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/Git/CommitJob.php b/src/Git/CommitJob.php index 726443d76b4..b09c2fb642e 100644 --- a/src/Git/CommitJob.php +++ b/src/Git/CommitJob.php @@ -3,15 +3,21 @@ namespace Statamic\Git; use Illuminate\Bus\Queueable; +use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Statamic\Facades\Git; -class CommitJob implements ShouldQueue +class CommitJob implements ShouldBeUnique, ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable; + /** + * The number of seconds after which the job's unique lock will be released. + */ + public int $uniqueFor = 120; + /** * Create a new job instance. */ diff --git a/tests/Git/GitTest.php b/tests/Git/GitTest.php index 38c1a15b3f9..00e1b86854f 100644 --- a/tests/Git/GitTest.php +++ b/tests/Git/GitTest.php @@ -400,6 +400,18 @@ public function it_dispatches_commit_job() Queue::assertPushed(\Statamic\Git\CommitJob::class, 1); } + #[Test] + public function it_only_dispatches_one_commit_job_at_a_time() + { + Queue::fake(); + + Git::dispatchCommit(); + Git::dispatchCommit(); + Git::dispatchCommit(); + + Queue::assertPushed(\Statamic\Git\CommitJob::class, 1); + } + #[Test] public function it_doesnt_push_by_default() { From a5a9662171e4fc101878d3f5e3b91209a7c18efb Mon Sep 17 00:00:00 2001 From: Michael Aerni Date: Thu, 14 May 2026 12:01:37 +0200 Subject: [PATCH 2/6] Attribute coalesced commits to the configured user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When multiple saves are dispatched within a single CommitJob's lock window, ShouldBeUnique drops the duplicate dispatches and the queued job's `git add` sweeps up everyone's changes — but the queued job still carries the first dispatcher's user. Attributing a multi-author commit to a single user is misleading. Track dispatch attempts in cache via Cache::increment in dispatchCommit, then in CommitJob::handle null out the committer when the pull reveals more than one dispatch occurred. Git's existing fallback then uses the configured user.name / user.email (the bot account) — consistent with how scheduler-driven saves are already attributed. --- src/Git/CommitJob.php | 5 ++++- src/Git/Git.php | 3 +++ tests/Git/GitTest.php | 27 +++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Git/CommitJob.php b/src/Git/CommitJob.php index b09c2fb642e..8d5a651060f 100644 --- a/src/Git/CommitJob.php +++ b/src/Git/CommitJob.php @@ -7,6 +7,7 @@ use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; +use Illuminate\Support\Facades\Cache; use Statamic\Facades\Git; class CommitJob implements ShouldBeUnique, ShouldQueue @@ -30,6 +31,8 @@ public function __construct(public $message = null, public $committer = null) */ public function handle() { - Git::as($this->committer)->commit($this->message); + $committer = Cache::pull('statamic-git-pending-saves', 0) > 1 ? null : $this->committer; + + Git::as($committer)->commit($this->message); } } diff --git a/src/Git/Git.php b/src/Git/Git.php index 473ce1e6ea0..368fd653964 100644 --- a/src/Git/Git.php +++ b/src/Git/Git.php @@ -4,6 +4,7 @@ use Illuminate\Filesystem\Filesystem; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Cache; use Statamic\Console\Processes\Git as GitProcess; use Statamic\Contracts\Auth\User as UserContract; use Statamic\Facades\Antlers; @@ -93,6 +94,8 @@ public function dispatchCommit($message = null) $message = null; } + Cache::increment('statamic-git-pending-saves'); + CommitJob::dispatch($message, $this->authenticatedUser()) ->onConnection(config('statamic.git.queue_connection')) ->delay($delayInMinutes ?? null); diff --git a/tests/Git/GitTest.php b/tests/Git/GitTest.php index 00e1b86854f..f6cf140e892 100644 --- a/tests/Git/GitTest.php +++ b/tests/Git/GitTest.php @@ -3,6 +3,7 @@ namespace Tests\Git; use Illuminate\Filesystem\Filesystem; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Queue; use PHPUnit\Framework\Attributes\Test; use Statamic\Console\Processes\Git as GitProcess; @@ -412,6 +413,32 @@ public function it_only_dispatches_one_commit_job_at_a_time() Queue::assertPushed(\Statamic\Git\CommitJob::class, 1); } + #[Test] + public function it_attributes_coalesced_commits_to_the_configured_user() + { + Cache::put('statamic-git-pending-saves', 3); + + $user = User::make()->email('alice@example.com'); + + Git::shouldReceive('as')->with(null)->andReturnSelf()->once(); + Git::shouldReceive('commit')->with('Entry saved')->once(); + + (new \Statamic\Git\CommitJob('Entry saved', $user))->handle(); + } + + #[Test] + public function it_attributes_non_coalesced_commits_to_the_authenticated_user() + { + Cache::put('statamic-git-pending-saves', 1); + + $user = User::make()->email('alice@example.com'); + + Git::shouldReceive('as')->with($user)->andReturnSelf()->once(); + Git::shouldReceive('commit')->with('Entry saved')->once(); + + (new \Statamic\Git\CommitJob('Entry saved', $user))->handle(); + } + #[Test] public function it_doesnt_push_by_default() { From e1a6fc016ccf503199425997b447d91158fb4b7e Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 10 Jun 2026 14:04:25 -0400 Subject: [PATCH 3/6] Make CommitJob unique lock expiry configurable Co-Authored-By: Claude Sonnet 4.6 --- config/git.php | 14 ++++++++++++++ src/Git/CommitJob.php | 6 ++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/config/git.php b/config/git.php index 7c569b0435f..6edadac32da 100644 --- a/config/git.php +++ b/config/git.php @@ -62,6 +62,20 @@ 'dispatch_delay' => env('STATAMIC_GIT_DISPATCH_DELAY', 0), + /* + |-------------------------------------------------------------------------- + | Unique Lock Expiry + |-------------------------------------------------------------------------- + | + | When commits are queued, a unique lock prevents multiple jobs from + | running concurrently against the same repository. This value (in + | seconds) controls how long that lock is held as a crash-safety + | net. It should exceed your queue worker's configured timeout. + | + */ + + 'unique_lock_expiry' => env('STATAMIC_GIT_UNIQUE_LOCK_EXPIRY', 120), + /* |-------------------------------------------------------------------------- | Git User diff --git a/src/Git/CommitJob.php b/src/Git/CommitJob.php index 8d5a651060f..9e7941e3d8e 100644 --- a/src/Git/CommitJob.php +++ b/src/Git/CommitJob.php @@ -14,16 +14,14 @@ class CommitJob implements ShouldBeUnique, ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable; - /** - * The number of seconds after which the job's unique lock will be released. - */ - public int $uniqueFor = 120; + public int $uniqueFor; /** * Create a new job instance. */ public function __construct(public $message = null, public $committer = null) { + $this->uniqueFor = config('statamic.git.unique_lock_expiry', 120); } /** From 6067a6b77106ad7af59adca0e76b66234c36b7c6 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 10 Jun 2026 14:47:00 -0400 Subject: [PATCH 4/6] Add Co-Authored-By trailers for coalesced multi-user commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When multiple users save during the dispatch delay window, their individual dispatches are coalesced into a single CommitJob. Rather than silently attributing that commit to the configured bot user, this records every contributor as a Co-Authored-By trailer so the commit log reflects who actually made changes. dispatchCommit() now appends {name, email} to a cache array instead of incrementing a counter. CommitJob::handle() pulls that array, deduplicates by email, and when more than one unique author is found it: (a) commits as the bot user, and (b) appends Co-Authored-By lines to the message before handing it to Git::commit(). shellEscape() is updated from escapeshellcmd() to targeted escaping of only \, $, and ` — the three characters that retain special meaning inside a double-quoted shell string per the Bash Reference Manual §3.1.2.3. escapeshellcmd() was over-escaping characters like <, >, ;, and newlines that are literal inside "...", which would corrupt the Co-Authored-By trailer format and was also producing spurious backslashes in git log for ordinary user names and messages. Co-Authored-By: Claude Sonnet 4.6 --- src/Git/CommitJob.php | 15 +++++++++-- src/Git/Git.php | 9 +++++-- tests/Git/GitTest.php | 60 +++++++++++++++++++++++++++++++++++-------- 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/Git/CommitJob.php b/src/Git/CommitJob.php index 9e7941e3d8e..31941328387 100644 --- a/src/Git/CommitJob.php +++ b/src/Git/CommitJob.php @@ -10,6 +10,8 @@ use Illuminate\Support\Facades\Cache; use Statamic\Facades\Git; +use function Statamic\trans as __; + class CommitJob implements ShouldBeUnique, ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable; @@ -29,8 +31,17 @@ public function __construct(public $message = null, public $committer = null) */ public function handle() { - $committer = Cache::pull('statamic-git-pending-saves', 0) > 1 ? null : $this->committer; + $saves = Cache::pull('statamic-git-pending-saves', []); + $users = collect($saves)->unique('email')->values(); + $coalesced = $users->count() > 1; + + $message = $this->message; + + if ($coalesced) { + $trailers = $users->map(fn ($u) => "Co-Authored-By: {$u['name']} <{$u['email']}>")->implode("\n"); + $message = ($message ?? __('Content saved'))."\n\n".$trailers; + } - Git::as($committer)->commit($this->message); + Git::as($coalesced ? null : $this->committer)->commit($message); } } diff --git a/src/Git/Git.php b/src/Git/Git.php index 368fd653964..994c0124a98 100644 --- a/src/Git/Git.php +++ b/src/Git/Git.php @@ -94,7 +94,9 @@ public function dispatchCommit($message = null) $message = null; } - Cache::increment('statamic-git-pending-saves'); + $saves = Cache::get('statamic-git-pending-saves', []); + $saves[] = ['name' => $this->gitUserName(), 'email' => $this->gitUserEmail()]; + Cache::put('statamic-git-pending-saves', $saves); CommitJob::dispatch($message, $this->authenticatedUser()) ->onConnection(config('statamic.git.queue_connection')) @@ -286,8 +288,11 @@ protected function shellEscape(string $string) { $string = str_replace('"', '', $string); $string = str_replace("'", '', $string); + $string = str_replace('\\', '\\\\', $string); + $string = str_replace('$', '\\$', $string); + $string = str_replace('`', '\\`', $string); - return escapeshellcmd($string); + return $string; } /** diff --git a/tests/Git/GitTest.php b/tests/Git/GitTest.php index f6cf140e892..373eefcf4de 100644 --- a/tests/Git/GitTest.php +++ b/tests/Git/GitTest.php @@ -253,13 +253,8 @@ public function it_shell_escapes_git_user_name_and_email() Git::commit('Message"; echo "deleting all your files now"; #'); - $expectedUser = 'Jimmy\; echo deleting all your files now\; \# '; - $expectedMessage = 'Message\; echo deleting all your files now\; \#'; - - if (static::isRunningWindows()) { - $expectedUser = str_replace('\\', '^', $expectedUser); - $expectedMessage = str_replace('\\', '^', $expectedMessage); - } + $expectedUser = 'Jimmy; echo deleting all your files now; # '; + $expectedMessage = 'Message; echo deleting all your files now; #'; $lastCommit = $this->showLastCommit(base_path('content')); @@ -416,12 +411,19 @@ public function it_only_dispatches_one_commit_job_at_a_time() #[Test] public function it_attributes_coalesced_commits_to_the_configured_user() { - Cache::put('statamic-git-pending-saves', 3); + Cache::put('statamic-git-pending-saves', [ + ['name' => 'Alice', 'email' => 'alice@example.com'], + ['name' => 'Bob', 'email' => 'bob@example.com'], + ]); $user = User::make()->email('alice@example.com'); Git::shouldReceive('as')->with(null)->andReturnSelf()->once(); - Git::shouldReceive('commit')->with('Entry saved')->once(); + Git::shouldReceive('commit')->withArgs(function ($message) { + return str_contains($message, 'Entry saved') + && str_contains($message, 'Co-Authored-By: Alice ') + && str_contains($message, 'Co-Authored-By: Bob '); + })->once(); (new \Statamic\Git\CommitJob('Entry saved', $user))->handle(); } @@ -429,7 +431,9 @@ public function it_attributes_coalesced_commits_to_the_configured_user() #[Test] public function it_attributes_non_coalesced_commits_to_the_authenticated_user() { - Cache::put('statamic-git-pending-saves', 1); + Cache::put('statamic-git-pending-saves', [ + ['name' => 'Alice', 'email' => 'alice@example.com'], + ]); $user = User::make()->email('alice@example.com'); @@ -439,6 +443,42 @@ public function it_attributes_non_coalesced_commits_to_the_authenticated_user() (new \Statamic\Git\CommitJob('Entry saved', $user))->handle(); } + #[Test] + public function it_adds_co_authored_by_trailers_for_coalesced_saves() + { + $this->files->put(base_path('content/collections/pages.yaml'), 'title: Pages Title Changed'); + + $message = "Content saved\n\nCo-Authored-By: Alice \nCo-Authored-By: Bob "; + + Git::commit($message); + + $commit = $this->showLastCommit(base_path('content')); + + $this->assertStringContainsString('Co-Authored-By: Alice ', $commit); + $this->assertStringContainsString('Co-Authored-By: Bob ', $commit); + } + + #[Test] + public function it_deduplicates_co_authors_for_repeated_saves_by_the_same_user() + { + Cache::put('statamic-git-pending-saves', [ + ['name' => 'Alice', 'email' => 'alice@example.com'], + ['name' => 'Alice', 'email' => 'alice@example.com'], + ['name' => 'Bob', 'email' => 'bob@example.com'], + ]); + + $user = User::make()->email('alice@example.com'); + + Git::shouldReceive('as')->with(null)->andReturnSelf()->once(); + Git::shouldReceive('commit')->withArgs(function ($message) { + return substr_count($message, 'Co-Authored-By:') === 2 + && str_contains($message, 'Co-Authored-By: Alice ') + && str_contains($message, 'Co-Authored-By: Bob '); + })->once(); + + (new \Statamic\Git\CommitJob('Entry saved', $user))->handle(); + } + #[Test] public function it_doesnt_push_by_default() { From 26f7d666cdb67c8980f1657eb4a9d04a777bcdc3 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 10 Jun 2026 14:52:02 -0400 Subject: [PATCH 5/6] Add end-to-end pipeline test for Co-Authored-By attribution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests the full dispatchCommit → cache accumulation → CommitJob::handle → real git commit path. Two users dispatch; the second is dropped by ShouldBeUnique but their save is still recorded in the cache array. Running handle() directly then verifies both Co-Authored-By trailers appear in the git log. Co-Authored-By: Claude Sonnet 4.6 --- tests/Git/GitTest.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/Git/GitTest.php b/tests/Git/GitTest.php index 373eefcf4de..fdd40f99c69 100644 --- a/tests/Git/GitTest.php +++ b/tests/Git/GitTest.php @@ -479,6 +479,30 @@ public function it_deduplicates_co_authors_for_repeated_saves_by_the_same_user() (new \Statamic\Git\CommitJob('Entry saved', $user))->handle(); } + #[Test] + public function it_wires_dispatch_to_co_authored_by_trailers_end_to_end() + { + Queue::fake(); + + $this->files->put(base_path('content/collections/pages.yaml'), 'title: Pages Title Changed'); + + $alice = User::make()->email('alice@example.com')->data(['name' => 'Alice'])->makeSuper(); + $bob = User::make()->email('bob@example.com')->data(['name' => 'Bob'])->makeSuper(); + + $this->actingAs($alice); + Git::dispatchCommit(); + + $this->actingAs($bob); + Git::dispatchCommit(); // dropped by ShouldBeUnique, but Bob's save is still recorded in cache + + (new \Statamic\Git\CommitJob(null, $alice))->handle(); + + $commit = $this->showLastCommit(base_path('content')); + + $this->assertStringContainsString('Co-Authored-By: Alice ', $commit); + $this->assertStringContainsString('Co-Authored-By: Bob ', $commit); + } + #[Test] public function it_doesnt_push_by_default() { From 6cffab6ac7394a1cee20d84661ed3b96608d264f Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Wed, 10 Jun 2026 15:22:19 -0400 Subject: [PATCH 6/6] Clarify why Queue::fake() still enforces ShouldBeUnique Co-Authored-By: Claude Sonnet 4.6 --- tests/Git/GitTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Git/GitTest.php b/tests/Git/GitTest.php index fdd40f99c69..5facba3e872 100644 --- a/tests/Git/GitTest.php +++ b/tests/Git/GitTest.php @@ -399,6 +399,8 @@ public function it_dispatches_commit_job() #[Test] public function it_only_dispatches_one_commit_job_at_a_time() { + // ShouldBeUnique acquires its cache lock in Bus\Dispatcher before the job + // reaches the queue driver, so Queue::fake() still enforces uniqueness here. Queue::fake(); Git::dispatchCommit();