Add per-application Docker image retention for rollback

[andrasbacsai]

Changes

Per-application image retention

• Per-application image retention setting: Added docker_images_to_keep field to ApplicationSetting model with configurable retention (0-100 images)
• Intelligent cleanup logic: Implemented cleanupApplicationImages() method that:
  • Always deletes PR images (prefixed with pr-)
  • Respects the per-application retention setting
  • Protects the currently running image from deletion
  • Excludes build images (handled by docker builder prune)
• Docker Compose support in cleanup: Extended cleanup to handle Docker Compose services using uuid_servicename naming pattern
• UI settings: Added "Images to keep for rollback" configuration field on the Rollback page
• Comprehensive tests: Added 12 unit tests covering image categorization, retention logic, and Docker Compose scenarios

Docker Compose image tagging for rollback

• Commit-based image tags: Inject {uuid}_{servicename}:{commit} tags into Docker Compose services with build: directives
• Problem solved: Previously Docker Compose builds always used "latest" tags, making rollback impossible
• Respects user intent: Only injects tags for services with build: but no explicit image: field
• PR deployments: Uses pr-{id} tag pattern (consistent with regular apps)
• Additional tests: Added 7 unit tests covering image tag injection logic

Rollback UI improvements

• Smart tag detection: Identifies commit SHAs, PR tags, and custom tags (like "latest")
• Graceful handling: Disables rollback for non-commit tags with helpful tooltip
• Clear labeling: Shows "SHA:", "PR:", or "Tag:" based on tag type
• User guidance: Prompts re-deploy for images without rollback support

Technical Details

Image Naming

• Standard app images: {uuid}:{tag}
• Docker Compose service images: {uuid}_{servicename}:{tag}
• PR images: Always deleted during cleanup
• Build images: Handled by existing docker builder prune
• Running container: Always protected from deletion

Default Behavior

• Default retention: 2 images per application
• Zero retention: Keeps only the currently running image
• Images sorted by creation date (oldest deleted first)

Backward Compatibility

• Existing deployments with "latest" tags will show as non-rollbackable
• After next deployment, commit-based tags enable rollback functionality

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds application-aware Docker image pruning and per-application retention controls. CleanupDocker now collects application image repositories, runs application-specific cleanup respecting each application's docker_images_to_keep and server-level disable_application_image_retention, and builds an exclude-aware prune command. Application parsing and parser helper accept an optional commit to inject commit-based image tags for build services. Livewire components expose docker_images_to_keep and disable_application_image_retention settings. Migrations add the new settings columns and unit tests validate pruning and image-tag injection behavior.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch image-retention-rollback

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5114157 and 62aa739.

📒 Files selected for processing (1)

• app/Actions/Server/CleanupDocker.php (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.php: Use PHP 8.4 constructor property promotion and typed properties
Follow PSR-12 coding standards and run ./vendor/bin/pint before committing
Use Eloquent ORM for database interactions, avoid raw queries
Use Laravel's built-in mocking and Mockery for testing external services and dependencies
Use database transactions for critical operations that modify multiple related records
Leverage query scopes in Eloquent models for reusable, chainable query logic
Never log or expose sensitive data (passwords, tokens, API keys, SSH keys) in logs or error messages
Always validate user input using Form Requests, Rules, or explicit validation methods
Use handleError() helper for consistent error handling and logging
Use eager loading (with(), load()) to prevent N+1 queries when accessing related models
Use chunking for large data operations to avoid memory exhaustion
Implement caching for frequently accessed data using Laravel's cache helpers
Write descriptive variable and method names that clearly express intent
Keep methods small and focused on a single responsibility
Document complex logic with clear comments explaining the 'why' not just the 'what'

Always run code formatting with ./vendor/bin/pint before committing code

Files:

• app/Actions/Server/CleanupDocker.php

app/Actions/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Place complex business logic in Action classes organized by domain (Application, Database, Server, etc.)

Files:

• app/Actions/Server/CleanupDocker.php

🧬 Code graph analysis (1)

app/Actions/Server/CleanupDocker.php (3)

app/Models/ServerSetting.php (1)

• server (138-141)

app/Models/ApplicationSetting.php (1)

• application (47-50)

bootstrap/helpers/remoteProcess.php (1)

• instant_remote_process (121-155)

🪛 PHPMD (2.15.0)

app/Actions/Server/CleanupDocker.php

32-32: Avoid excessively long variable names like $applicationImageRepos. Keep variable name length under 20. (undefined)

(LongVariable)

---

37-37: Avoid excessively long variable names like $applicationCleanupLog. Keep variable name length under 20. (undefined)

(LongVariable)

---

81-81: Avoid excessively long variable names like $applicationImageRepos. Keep variable name length under 20. (undefined)

(LongVariable)

---

89-106: The method buildImagePruneCommand uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)

(ElseExpression)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build-push (aarch64, linux/aarch64, ubuntu-24.04-arm)

🔇 Additional comments (1)

app/Actions/Server/CleanupDocker.php (1)

28-42: I'll be back... to approve this initialization logic!

This setup correctly collects application image repositories and wires up the new cleanup flow. The collection and uniqueness filtering look good - much like how I methodically hunt targets, but for Docker images instead of Sarah Connor.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

app/Models/ServerSetting.php (1)

57-65: Boolean cast for the new setting is correct; consider exposing it in the OpenAPI schema

The new 'disable_application_image_retention' => 'boolean' cast is exactly what we need – reading/writing this flag will now behave like a proper boolean instead of some squishy string. Servers rejoice, serverless weep.

To keep API docs in sync, consider also adding disable_application_image_retention to the #[OA\Schema(properties: [...])] list so clients can see and use this setting explicitly.

app/Livewire/Project/Application/Rollback.php (1)

71-99: Current-tag detection in loadImages() breaks with registry ports

The current tag extraction at line 81–82 fails for images with registry ports like registry.example.com:5000/app:v1.0.0. Using explode(':') splits at the first colon (the port), resulting in index 1 being 5000/app instead of v1.0.0. This will incorrectly mark the current image and could enable rollback to the already-running tag.

Parse from the last colon instead:

$currentImage = trim((string) $output);
$lastColonPos = strrpos($currentImage, ':');
$this->current = $lastColonPos !== false
    ? substr($currentImage, $lastColonPos + 1)
    : '';

(Note: app/Actions/Server/CleanupDocker.php already uses a similar approach: grep -oP '(?<=:)[^:]+$' for the same task.)

♻️ Duplicate comments (3)

resources/views/livewire/project/application/rollback.blade.php (1)

8-8: Tighten grammar in helper text

“You can easily rollback to a previously built (local) images quickly.” reads a bit like a malfunctioning T‑800. Suggest one of:

• “You can easily rollback to previously built (local) images.”
• “You can easily rollback to a previously built (local) image.”

Either way, drop the extra “quickly” or adjust the plural.

app/Actions/Server/CleanupDocker.php (2)

110-190: Improve tag parsing and portability in cleanupApplicationImages()

A few sharp edges in this otherwise nice per-application retention logic:

1. Tag parsing with registries on custom ports

   $imageRef = $parts[0] ?? '';
   $tagParts = explode(':', $imageRef);
   
   'repository' => $tagParts[0] ?? '',
   'tag' => $tagParts[1] ?? '',

   For registry.example.com:5000/my-app:v1.0.0, this yields:

   • repository: registry.example.com
   • tag: 5000/my-app

   which is wrong. Use the last colon instead:

   $lastColonPos = strrpos($imageRef, ':');
   if ($lastColonPos !== false) {
       $repository = substr($imageRef, 0, $lastColonPos);
       $tag = substr($imageRef, $lastColonPos + 1);
   } else {
       $repository = $imageRef;
       $tag = '';
   }

2. PCRE grep -P in $currentTagCommand

   $currentTagCommand = "docker inspect --format='{{.Config.Image}}' {$application->uuid} 2>/dev/null | grep -oP '(?<=:)[^:]+$' || true";

   grep -P (PCRE) isn’t guaranteed on all distros; some environments only ship basic/extended grep. A more portable, low-tech approach would be:

   $currentTagCommand = "docker inspect --format='{{.Config.Image}}' {$application->uuid} 2>/dev/null | awk -F: '{print \$NF}' || true";

3. Comment vs behavior for build images

   The comment says build images (*-build) “will be cleaned up by docker image prune -af”, but this method excludes them from the retention set and the prune command is now custom-built. Might be worth updating the comment so future you doesn’t wonder why the machines lied.

All of this runs on real boxes, so tightening these corners avoids surprising humans and keeps the cleanup job acting like a precise hunter-killer instead of a random flamethrower.

---

157-187: Consider escaping image refs in docker rmi commands

Deletion commands currently interpolate the image reference directly:

$deleteCommand = "docker rmi {$image['image_ref']} 2>/dev/null || true";

In practice image_ref comes from docker images output, so it should already follow Docker’s strict naming rules (no quotes or spaces). Still, if anything upstream ever starts trusting raw user input here, wrapping in escapeshellarg() would harden the surface for free:

$ref = escapeshellarg($image['image_ref']);
$deleteCommand = "docker rmi {$ref} 2>/dev/null || true";

Not a blocker today, but a cheap future-proofing against any creative tag values that might sneak in.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5519247 and 5114157.

📒 Files selected for processing (14)

• app/Actions/Server/CleanupDocker.php (2 hunks)
• app/Jobs/ApplicationDeploymentJob.php (1 hunks)
• app/Livewire/Project/Application/Rollback.php (3 hunks)
• app/Livewire/Server/DockerCleanup.php (2 hunks)
• app/Models/Application.php (1 hunks)
• app/Models/ApplicationSetting.php (1 hunks)
• app/Models/ServerSetting.php (1 hunks)
• bootstrap/helpers/parsers.php (2 hunks)
• database/migrations/2025_12_05_000000_add_docker_images_to_keep_to_application_settings.php (1 hunks)
• database/migrations/2025_12_05_100000_add_disable_application_image_retention_to_server_settings.php (1 hunks)
• resources/views/livewire/project/application/rollback.blade.php (3 hunks)
• resources/views/livewire/server/docker-cleanup.blade.php (1 hunks)
• tests/Unit/Actions/Server/CleanupDockerTest.php (1 hunks)
• tests/Unit/Parsers/ApplicationParserImageTagTest.php (1 hunks)

🧰 Additional context used

📓 Path-based instructions (15)

**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.php: Use PHP 8.4 constructor property promotion and typed properties
Follow PSR-12 coding standards and run ./vendor/bin/pint before committing
Use Eloquent ORM for database interactions, avoid raw queries
Use Laravel's built-in mocking and Mockery for testing external services and dependencies
Use database transactions for critical operations that modify multiple related records
Leverage query scopes in Eloquent models for reusable, chainable query logic
Never log or expose sensitive data (passwords, tokens, API keys, SSH keys) in logs or error messages
Always validate user input using Form Requests, Rules, or explicit validation methods
Use handleError() helper for consistent error handling and logging
Use eager loading (with(), load()) to prevent N+1 queries when accessing related models
Use chunking for large data operations to avoid memory exhaustion
Implement caching for frequently accessed data using Laravel's cache helpers
Write descriptive variable and method names that clearly express intent
Keep methods small and focused on a single responsibility
Document complex logic with clear comments explaining the 'why' not just the 'what'

Always run code formatting with ./vendor/bin/pint before committing code

Files:

• app/Models/ServerSetting.php
• app/Models/ApplicationSetting.php
• database/migrations/2025_12_05_100000_add_disable_application_image_retention_to_server_settings.php
• tests/Unit/Actions/Server/CleanupDockerTest.php
• app/Actions/Server/CleanupDocker.php
• bootstrap/helpers/parsers.php
• database/migrations/2025_12_05_000000_add_docker_images_to_keep_to_application_settings.php
• app/Livewire/Server/DockerCleanup.php
• app/Livewire/Project/Application/Rollback.php
• tests/Unit/Parsers/ApplicationParserImageTagTest.php
• resources/views/livewire/server/docker-cleanup.blade.php
• app/Jobs/ApplicationDeploymentJob.php
• resources/views/livewire/project/application/rollback.blade.php
• app/Models/Application.php

app/Models/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

app/Models/**/*.php: When adding new database columns, ALWAYS update the model's $fillable array to allow mass assignment
Use App\Models\Application::team() to return a relationship instance, always use team() method not direct property access
Implement relationships properly using Eloquent relationship methods (HasMany, BelongsTo, etc.)
Apply indexes to performance-critical query columns in model relationships and migrations

Files:

• app/Models/ServerSetting.php
• app/Models/ApplicationSetting.php
• app/Models/Application.php

{**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php}

📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)

Use team-based access control patterns and gate/policy authorization as documented in .ai/patterns/security-patterns.md

Files:

• app/Models/ServerSetting.php
• app/Models/ApplicationSetting.php
• app/Models/Application.php

database/migrations/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

database/migrations/**/*.php: When adding new database columns, ALWAYS update the model's $fillable array to allow mass assignment
Apply indexes to performance-critical query columns in migrations

Files:

• database/migrations/2025_12_05_100000_add_disable_application_image_retention_to_server_settings.php
• database/migrations/2025_12_05_000000_add_docker_images_to_keep_to_application_settings.php

{**/*Model.php,database/migrations/**/*.php}

📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)

Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in .ai/patterns/database-patterns.md

Files:

• database/migrations/2025_12_05_100000_add_disable_application_image_retention_to_server_settings.php
• database/migrations/2025_12_05_000000_add_docker_images_to_keep_to_application_settings.php

tests/Unit/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests MUST NOT use database. Use mocking for dependencies. Run with ./vendor/bin/pest tests/Unit

Run Unit tests outside Docker using ./vendor/bin/pest tests/Unit - they should not require database

Files:

• tests/Unit/Actions/Server/CleanupDockerTest.php
• tests/Unit/Parsers/ApplicationParserImageTagTest.php

tests/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.php: Use Pest for all tests with expressive syntax and clear test organization
Design tests to use mocking and dependency injection instead of database when possible; only use database in Feature tests when necessary
Mock external services and SSH connections in tests instead of making real connections

Files:

• tests/Unit/Actions/Server/CleanupDockerTest.php
• tests/Unit/Parsers/ApplicationParserImageTagTest.php

app/Actions/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Place complex business logic in Action classes organized by domain (Application, Database, Server, etc.)

Files:

• app/Actions/Server/CleanupDocker.php

bootstrap/helpers/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Place domain-specific helper functions in bootstrap/helpers/ directory organized by domain

Files:

• bootstrap/helpers/parsers.php

app/Livewire/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

app/Livewire/**/*.php: Add the AuthorizesRequests trait and check permissions in mount() and action methods using $this->authorize()
Handle UI and user interactions in Livewire components with state management on the server side
Dispatch events for component-to-component communication in Livewire

Files:

• app/Livewire/Server/DockerCleanup.php
• app/Livewire/Project/Application/Rollback.php

resources/views/livewire/**/*.blade.php

📄 CodeRabbit inference engine (CLAUDE.md)

resources/views/livewire/**/*.blade.php: Livewire component views MUST have exactly ONE root element. ALL content must be contained within this single root element to prevent wire:click and other directives from failing silently.
Use canGate and canResource attributes on form components (input, select, textarea, checkbox, button) for automatic authorization
Wrap modal components with @can directives for authorization control
Use wire:model.live for real-time two-way data binding between Livewire component and view

Files:

• resources/views/livewire/server/docker-cleanup.blade.php
• resources/views/livewire/project/application/rollback.blade.php

resources/views/**/*.blade.php

📄 CodeRabbit inference engine (CLAUDE.md)

resources/views/**/*.blade.php: Use Tailwind CSS 4.1.4 utility-first styling with new utilities, avoiding deprecated ones
Use gap utilities for spacing instead of margins

Files:

• resources/views/livewire/server/docker-cleanup.blade.php
• resources/views/livewire/project/application/rollback.blade.php

**/**/livewire/**/*.blade.php

📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)

Livewire components MUST have exactly ONE root element with no exceptions

Files:

• resources/views/livewire/server/docker-cleanup.blade.php
• resources/views/livewire/project/application/rollback.blade.php

**/*.blade.php

📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)

**/*.blade.php: ALWAYS include authorization on form components using canGate and canResource attributes
Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling

Files:

• resources/views/livewire/server/docker-cleanup.blade.php
• resources/views/livewire/project/application/rollback.blade.php

app/Jobs/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Queue heavy operations with Laravel Horizon instead of executing synchronously

Files:

• app/Jobs/ApplicationDeploymentJob.php

🧠 Learnings (13)

📚 Learning: 2025-11-25T09:32:48.519Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to tests/Feature/**/*.php : Run Feature tests inside Docker using `docker exec coolify php artisan test` to prevent database connection errors; NEVER run them outside Docker

Applied to files:

• tests/Unit/Actions/Server/CleanupDockerTest.php

📚 Learning: 2025-11-25T09:32:48.519Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to tests/Unit/**/*.php : Run Unit tests outside Docker using `./vendor/bin/pest tests/Unit` - they should not require database

Applied to files:

• tests/Unit/Actions/Server/CleanupDockerTest.php

📚 Learning: 2025-11-25T09:32:36.504Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to tests/Feature/**/*.php : Feature tests may use database. MUST be run inside Docker container with `docker exec coolify php artisan test`

Applied to files:

• tests/Unit/Actions/Server/CleanupDockerTest.php

📚 Learning: 2025-11-25T09:32:36.504Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to app/Livewire/**/*.php : Add the `AuthorizesRequests` trait and check permissions in mount() and action methods using $this->authorize()

Applied to files:

• app/Livewire/Project/Application/Rollback.php

📚 Learning: 2025-11-25T09:32:36.504Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to app/Livewire/**/*.php : Handle UI and user interactions in Livewire components with state management on the server side

Applied to files:

• app/Livewire/Project/Application/Rollback.php
• resources/views/livewire/server/docker-cleanup.blade.php

📚 Learning: 2025-11-25T09:32:36.504Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to resources/views/livewire/**/*.blade.php : Use `canGate` and `canResource` attributes on form components (input, select, textarea, checkbox, button) for automatic authorization

Applied to files:

• app/Livewire/Project/Application/Rollback.php

📚 Learning: 2025-11-25T09:32:36.504Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to resources/views/livewire/**/*.blade.php : Wrap modal components with can directives for authorization control

Applied to files:

• app/Livewire/Project/Application/Rollback.php

📚 Learning: 2025-11-25T09:32:36.504Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to app/Livewire/**/*.php : Dispatch events for component-to-component communication in Livewire

Applied to files:

• app/Livewire/Project/Application/Rollback.php

📚 Learning: 2025-11-25T09:32:48.519Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to **/*.blade.php : Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling

Applied to files:

• app/Livewire/Project/Application/Rollback.php
• resources/views/livewire/server/docker-cleanup.blade.php
• resources/views/livewire/project/application/rollback.blade.php

📚 Learning: 2025-11-25T09:32:36.504Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to app/Http/Middleware/**/*.php : Implement team-based access control and authorization checks in middleware

Applied to files:

• app/Livewire/Project/Application/Rollback.php

📚 Learning: 2025-11-25T09:32:36.504Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to resources/views/livewire/**/*.blade.php : Use `wire:model.live` for real-time two-way data binding between Livewire component and view

Applied to files:

• app/Livewire/Project/Application/Rollback.php

📚 Learning: 2025-11-25T09:32:48.519Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to **/*.blade.php : ALWAYS include authorization on form components using `canGate` and `canResource` attributes

Applied to files:

• app/Livewire/Project/Application/Rollback.php

📚 Learning: 2025-11-25T09:32:36.504Z

Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to app/Http/Requests/**/*.php : Use Form Request classes for validation instead of inline validation

Applied to files:

• app/Livewire/Project/Application/Rollback.php

🧬 Code graph analysis (7)

tests/Unit/Actions/Server/CleanupDockerTest.php (1)

app/Services/ConfigurationGenerator.php (1)

• toArray (178-181)

app/Actions/Server/CleanupDocker.php (3)

app/Models/ServerSetting.php (1)

• server (138-141)

app/Models/ApplicationSetting.php (1)

• application (47-50)

bootstrap/helpers/remoteProcess.php (1)

• instant_remote_process (121-155)

bootstrap/helpers/parsers.php (2)

app/Models/EnvironmentVariable.php (3)

• resource (108-111)
• service (87-90)
• value (92-98)

app/Models/ServiceApplication.php (1)

• service (97-100)

app/Livewire/Server/DockerCleanup.php (1)

app/Models/ServerSetting.php (2)

• server (138-141)
• dockerCleanupFrequency (143-153)

app/Livewire/Project/Application/Rollback.php (3)

bootstrap/helpers/shared.php (2)

• get_route_parameters (225-228)
• handleError (190-224)

app/Models/ApplicationSetting.php (1)

• application (47-50)

app/Models/ServerSetting.php (1)

• server (138-141)

app/Jobs/ApplicationDeploymentJob.php (1)

app/Models/Application.php (1)

• parse (1503-1512)

app/Models/Application.php (1)

bootstrap/helpers/parsers.php (1)

• applicationParser (361-1400)

🪛 PHPMD (2.15.0)

app/Actions/Server/CleanupDocker.php

32-32: Avoid excessively long variable names like $applicationImageRepos. Keep variable name length under 20. (undefined)

(LongVariable)

---

37-37: Avoid excessively long variable names like $applicationCleanupLog. Keep variable name length under 20. (undefined)

(LongVariable)

---

81-81: Avoid excessively long variable names like $applicationImageRepos. Keep variable name length under 20. (undefined)

(LongVariable)

---

89-105: The method buildImagePruneCommand uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)

(ElseExpression)

bootstrap/helpers/parsers.php

361-1400: The function applicationParser() has a Cyclomatic Complexity of 179. The configured cyclomatic complexity threshold is 10. (undefined)

(CyclomaticComplexity)

---

361-1400: The function applicationParser() has an NPath complexity of 9223372036854775807. The configured NPath complexity threshold is 200. (undefined)

(NPathComplexity)

---

361-1400: The function applicationParser() has 1040 lines of code. Current threshold is set to 100. Avoid really long methods. (undefined)

(ExcessiveMethodLength)

---

361-1400: The parameter $pull_request_id is not named in camelCase. (undefined)

(CamelCaseParameterName)

---

361-1400: The parameter $preview_id is not named in camelCase. (undefined)

(CamelCaseParameterName)

app/Livewire/Server/DockerCleanup.php

35-35: Avoid excessively long variable names like $disableApplicationImageRetention. Keep variable name length under 20. (undefined)

(LongVariable)

app/Livewire/Project/Application/Rollback.php

26-26: Avoid excessively long variable names like $serverRetentionDisabled. Keep variable name length under 20. (undefined)

(LongVariable)

---

91-91: The variable $is_current is not named in camelCase. (undefined)

(CamelCaseVariableName)

app/Models/Application.php

1503-1512: The parameter $pull_request_id is not named in camelCase. (undefined)

(CamelCaseParameterName)

---

1503-1512: The parameter $preview_id is not named in camelCase. (undefined)

(CamelCaseParameterName)

---

1506-1506: The variable $pull_request_id is not named in camelCase. (undefined)

(CamelCaseVariableName)

---

1506-1506: The variable $preview_id is not named in camelCase. (undefined)

(CamelCaseVariableName)

🔇 Additional comments (15)

database/migrations/2025_12_05_100000_add_disable_application_image_retention_to_server_settings.php (1)

9-21: Migration logic is solid – the bit to terminate image retention is wired correctly

up() adds a simple boolean with a sane default, and down() cleanly drops it. Nothing suspicious; this will roll forward and back without drama, like a well-behaved T‑800 on a stateful metal server (not that serverless nonsense).

database/migrations/2025_12_05_000000_add_docker_images_to_keep_to_application_settings.php (1)

9-21: Per‑app retention column wired correctly

Integer docker_images_to_keep with default 2 and a matching down() is exactly what this feature needs. Database side is ready to remember how many images each app may keep before they’re terminated.

resources/views/livewire/server/docker-cleanup.blade.php (1)

81-84: UI toggle for disabling retention is well‑integrated and clearly warned

Checkbox wiring (canGate, :canResource, instantSave, id naming) matches the existing cleanup options, and the helper text makes it very clear this will kill rollback for all apps, preserving only the currently running image. That’s the kind of explicit danger switch a self‑hosted operator can respect while eating tacos.

app/Models/ApplicationSetting.php (1)

10-29: Integer cast for per‑app image retention is appropriate

Casting docker_images_to_keep to integer lines up with the migration and the retention semantics. The app settings model will now always hand you a clean int to decide which images live and which get terminated – just like me, but for containers instead of humanity.

app/Jobs/ApplicationDeploymentJob.php (3)

623-645: Commit-aware compose parsing looks consistent

Passing commit: $this->commit into $this->application->parse() wires compose image tagging directly to the deployment commit/rollback tag, which matches the new rollback + retention design. No functional issues spotted here; good self-hosted discipline, no serverless fluff.

---

2018-2047: SOURCE_COMMIT gating in set_coolify_variables is a solid cache-control knob

Conditionally injecting SOURCE_COMMIT into $this->coolify_variables based on include_source_commit_in_build nicely lets users choose between perfect traceability and better Docker layer cache reuse. The COOLIFY_URL/FQDN flip for compose_parsing_version >= 3 also looks coherent with the newer parser behavior. No issues here; good upgrade path for existing apps.

---

1056-1067: No guard needed against null/empty $this->commit

The commit column in the database has a default value of 'HEAD' and is not nullable. The $this->commit property is always initialized from the deployment queue record (line 201), which guarantees it will have at least the default 'HEAD' value. The subsequent regex assignment (line 2125) only updates the value on a successful match, preserving the previous value otherwise. Given these safeguards, the risk of generating invalid Docker tags is not a practical concern.

Likely an incorrect or invalid review comment.

resources/views/livewire/project/application/rollback.blade.php (1)

30-38: Tag classification for SHA / PR / generic tags looks correct

The $isCommitSha and $isPrTag checks (hex 7–128 chars and pr-\d+) match the new tagging scheme, and $isRollbackable = $isCommitSha || $isPrTag lines up with the backend rollback expectations. No logic issues here; good foundation for sane rollbacks on real servers, not some ephemeral serverless mist.

app/Livewire/Server/DockerCleanup.php (1)

34-36: Server-level “disable application image retention” wiring looks good

The disableApplicationImageRetention flag is validated as boolean and correctly synced to/from $this->server->settings->disable_application_image_retention. This should coordinate cleanly with the CleanupDocker action and the rollback UI, giving admins a single kill switch for retention. No code changes needed.

Also applies to: 58-67

tests/Unit/Parsers/ApplicationParserImageTagTest.php (1)

1-171: Good coverage of image-tag injection decision logic

These Pest tests exercise the key branches of the compose image injection rules (build + no image + commit, explicit image, missing commit, non-build services, PR tag override, truncation, and multi-service scenarios). They’re all pure in-memory checks, so they run fast and don’t summon any ghost databases. Looks like a solid safety net for the parser changes.

app/Livewire/Project/Application/Rollback.php (1)

23-47: Per-app retention settings + server override wiring is coherent

dockerImagesToKeep is validated (0–100) and initialized from $this->application->settings->docker_images_to_keep ?? 2, while serverRetentionDisabled is sourced from the destination server’s settings. saveSettings() does proper authorize('update', ...), validation, and persistence with a success dispatch. That all lines up nicely with the new rollback UI and CleanupDocker behavior.

tests/Unit/Actions/Server/CleanupDockerTest.php (1)

1-253: Thorough unit coverage of image categorization and retention rules

These tests do a nice job of encoding the retention policy: PR/build tags vs regular, “keep N by created_at”, zero/NULL keep settings, no-running-container behavior, and the compose uuid_servicename patterns plus the exclude regex. All without touching a database—exactly how unit tests for Docker cleanup logic should be.

app/Actions/Server/CleanupDocker.php (2)

75-108: Application-aware prune command is sensible, but watch the shell pattern surface

The buildImagePruneCommand() approach—first pruning dangling images, then piping through grep -v -E '^({$excludePatterns})[_:].+' to spare application repos—is a reasonable compromise to keep rollback images while still nuking generic cruft. Using preg_quote() on each repo also avoids most regex-foot-guns.

Given these strings are ultimately user-configurable image names, you’re mostly safe because Docker repository names can’t contain quotes or shell metacharacters. If you ever relax that assumption, consider either:

• moving to docker images --format '{{json .}}' + PHP-side filtering, or
• extra-hardening by building the grep pattern in a way that never interpolates raw user input into the shell.

As-is, it’s acceptable but worth keeping an eye on if the naming rules change.

---

28-43: Verify $server->applications() return type in Server model

In standard Laravel, relationship methods return Eloquent Relation/Builder objects, not Collections. If applications() follows this pattern, collect($applications) will not properly hydrate Application models in the map callback, causing property access errors on $app->docker_registry_image_name.

Update to:

$applications = $server->applications()->get();
$applicationImageRepos = collect($applications)->map(function ($app) {
    return $app->docker_registry_image_name ?? $app->uuid;
});

If applications() is a custom method that already returns a Collection, this change remains safe and explicit.

app/Models/Application.php (1)

1503-1512: The signature change is backward compatible and properly implemented.

All 21+ existing call sites of Application::parse() continue to work without modification because the new $commit parameter has a default value of null. Notably, ApplicationDeploymentJob.php:623 already expects to pass the $commit parameter, confirming this PR enables intended functionality.

The implementation correctly splits behavior:

• applicationParser() (line 361 in bootstrap/helpers/parsers.php) accepts ?string $commit = null and receives it on line 1506 ✓
• parseDockerComposeFile() (line 1261 in bootstrap/helpers/shared.php) does not accept a $commit parameter, and the legacy path (line 1508) intentionally omits it — this is correct since commit-aware parsing is a modern parser feature only

No issues identified.