Fixes#7439 where successful deployments were being marked as FAILED due to exceptions during old container cleanup.
Root cause: Commit 97550f406 wrapped stop_running_container() in try-catch that re-throws ALL exceptions as DeploymentException. When old containers are already removed (a common scenario), the "No such container" error propagates and marks successful deployments as failed.
Solution: Check if deployment has already succeeded (newVersionIsHealthy || force) before re-throwing exceptions from cleanup operations. Cleanup failures are logged but don't fail the deployment.
- Add conditional handling in stop_running_container() catch block
- Log cleanup warnings with hidden: true to avoid UI clutter
- Only re-throw exceptions if deployment hasn't succeeded yet
- Preserves backward compatibility and expected behavior
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
The regex pattern in injectDockerComposeBuildArgs() was too restrictive
and failed to match `docker compose build servicename` commands. Changed
the lookahead from `(?=\s+(?:--|-)|\s+(?:&&|\|\||;|\|)|$)` to the
simpler `(?=\s|$)` to allow any content after the build command,
including service names with hyphens/underscores and flags.
Also improved the ApplicationDeploymentJob to use the new helper function
and added comprehensive test coverage for service-specific builds.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Refactors generate_buildtime_environment_variables() to use an associative
array (dictionary) approach instead of sequential push() calls. This prevents
duplicate variable declarations in the buildtime.env file.
**Problem:**
After adding nixpacks plan variables to buildtime.env, the same variable
could appear twice in the file:
- Once from nixpacks plan (e.g., NIXPACKS_NODE_VERSION='22')
- Once from user-defined variables (e.g., NIXPACKS_NODE_VERSION="22")
This caused shell errors and undefined behavior during Docker builds.
**Root Cause:**
The push() method adds items sequentially without checking for duplicate
keys. When a variable existed in both nixpacks plan AND user-defined vars,
both would be written to the file.
**Solution:**
- Use associative array ($envs_dict) for automatic deduplication
- Establish clear override precedence:
1. Nixpacks plan variables (lowest priority)
2. COOLIFY_* variables (medium priority)
3. SERVICE_* variables (medium priority)
4. User-defined variables (highest priority - can override everything)
- Convert to collection format at the end
- Add debug logging when user variables override plan variables
**Benefits:**
- Automatic deduplication (array keys are unique by nature)
- User variables properly override nixpacks plan values
- Clear, explicit precedence order
- No breaking changes to existing functionality
Fixes#7114🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Add two new application settings to control Docker build cache invalidation:
- inject_build_args_to_dockerfile (default: true) - Skip Dockerfile ARG injection
- include_source_commit_in_build (default: false) - Exclude SOURCE_COMMIT from build context
These toggles let users preserve Docker cache when SOURCE_COMMIT or custom ARGs change frequently. Development-only logging shows which ARGs are being injected for debugging.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
- Remove COOLIFY_CONTAINER_NAME from build-time ARGs (timestamp-based, breaks cache)
- Use APP_KEY instead of random_bytes for COOLIFY_BUILD_SECRETS_HASH (deterministic)
- Add forBuildTime parameter to generate_coolify_env_variables() to control injection
- Keep COOLIFY_CONTAINER_NAME available at runtime for container identification
- Fix misleading log message about .env file purpose
Fixes#7040🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Fix double-slash issue in Docker Compose preview paths when baseDirectory is "/"
- Normalize baseDirectory using rtrim() to prevent path concatenation issues
- Replace hardcoded '/artifacts/build-time.env' with ApplicationDeploymentJob::BUILD_TIME_ENV_PATH
- Make BUILD_TIME_ENV_PATH constant public for reusability
- Add comprehensive unit tests (11 test cases, 25 assertions)
Fixes preview path generation in:
- getDockerComposeBuildCommandPreviewProperty()
- getDockerComposeStartCommandPreviewProperty()
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
When using a custom Docker Compose build command, environment variables
were being lost because the --env-file flag was not included. This fix
automatically injects the --env-file flag to ensure build-time environment
variables are available during custom builds.
Changes:
- Auto-inject --env-file /artifacts/build-time.env after docker compose
- Respect user-provided --env-file flags (no duplication)
- Append build arguments when not using build secrets
- Update UI helper text to inform users about automatic env injection
- Add comprehensive unit tests (7 test cases, all passing)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Add exception class names to error messages for better debugging
- Mark technical details (error type, code, location, stack trace) as hidden in logs
- Preserve original exception types when wrapping in DeploymentException
- Update ServerManagerJob to include exception class in log messages
- Enhance unit tests to verify hidden log entry behavior
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Wraps rolling_update(), health_check(), stop_running_container(), and
start_by_compose_file() with try-catch to ensure comprehensive error logging
happens in one place. Removes duplicate logging from intermediate catch blocks
since the failed() method already provides full error details including stack trace
and chained exception information.
🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>
Add detection system for PORT environment variable to help users configure applications correctly:
- Add detectPortFromEnvironment() method to Application model to detect PORT env var
- Add getDetectedPortInfoProperty() computed property in General Livewire component
- Display contextual info banners in UI when PORT is detected:
- Warning when PORT exists but ports_exposes is empty
- Warning when PORT doesn't match ports_exposes configuration
- Info message when PORT matches ports_exposes
- Add deployment logging to warn about PORT/ports_exposes mismatches
- Include comprehensive unit tests for port detection logic
The ports_exposes field remains authoritative for proxy configuration, while
PORT detection provides helpful suggestions to users.
- Fix container filtering to properly distinguish base deployments (pullRequestId=0) from PR deployments
- Add deployment cancellation when PR closes via webhook to prevent race conditions
- Prevent CleanupHelperContainersJob from killing active deployment containers
- Enhance error messages with exit codes and actual errors instead of vague "Oops" messages
- Protect status transitions in finally blocks to ensure proper job failure handling
The `is_array` check for `custom_network_aliases_array` was too strict and could lead to issues when the value was an empty string or null. This commit changes the check to `!empty()` for more robust handling.
Additionally, the unit tests for `custom_network_aliases` have been refactored to directly use the `Application::isConfigurationChanged()` method. This provides a more accurate and integrated test of the configuration change detection logic, rather than relying on a manual hash calculatio
The custom_network_aliases attribute in the Application model was being cast to an array directly. This commit refactors the attribute to provide both a string representation (for compatibility with older configurations and hashing) and an array representation for internal use. This ensures that network aliases are correctly parsed and utilized, preventing potential issues during deployment and configuration updates.
The backward-compatible next() method is no longer needed since all
call sites have been updated to use the clearer method names:
- completeDeployment()
- failDeployment()
- transitionToStatus()
This completes the refactoring to make status transitions more explicit
and maintainable.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
**Problem:**
Deployment failure notifications were not being sent due to two bugs:
1. **Timing Issue in next() function:**
- When failed() called next(FAILED), the database still had status "in_progress"
- The notification check looked for ALREADY failed status (not found yet)
- Status was updated AFTER the check, losing the notification
2. **Direct Status Update:**
- Healthcheck failures directly updated status to FAILED
- Bypassed next() entirely, no notification sent
**Solution:**
Refactored status transition logic with clear separation of concerns:
- Moved notification logic AFTER status update (not before)
- Created transitionToStatus() as single source of truth
- Added completeDeployment() and failDeployment() for clarity
- Extracted status-specific side effects into dedicated methods
- Updated healthcheck failure to use failDeployment()
**Benefits:**
- ✅ Notifications sent for ALL failure scenarios
- ✅ Clear, self-documenting method names
- ✅ Single responsibility per method
- ✅ Type-safe using enum instead of strings
- ✅ Harder to bypass notification logic accidentally
- ✅ Backward compatible (old next() preserved)
**Changed:**
- app/Jobs/ApplicationDeploymentJob.php (+101/-21 lines)
Fixes#6911🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Previously, the bash escaping functions (`escapeBashEnvValue()` and `escapeBashDoubleQuoted()`) had strict string type hints that rejected null values, causing deployment failures when environment variables had null values.
Changes:
- Updated both functions to accept nullable strings (`?string $value`)
- Handle null/empty values by returning empty quoted strings (`''` for single quotes, `""` for double quotes)
- Added 3 new tests to cover null and empty value handling
- All 29 tests pass
This fix ensures deployments work correctly even when environment variables have null values, while maintaining the existing behavior for all other cases.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Two improvements to Git deployment handling:
1. **ApplicationDeploymentJob.php**:
- Fixed log message to show actual resolved commit SHA (`$this->commit`)
- Previously showed `$this->application->git_commit_sha` which could be "HEAD"
- Now displays the actual 40-character commit SHA that will be deployed
2. **Application.php (generateGitLsRemoteCommands)**:
- Added `escapeshellarg()` for repository URL in 'other' deployment type
- Prevents shell injection in git ls-remote commands
- Complements existing shell escaping in `generateGitImportCommands`
- Ensures consistent security across all Git operations
**Security Impact:**
- All Git commands now use properly escaped repository URLs
- Prevents command injection through malicious repository URLs
- Consistent escaping in both ls-remote and clone operations
**User Experience:**
- Deployment logs now show exact commit SHA being deployed
- More accurate debugging information for deployment issues
Co-Authored-By: Claude <noreply@anthropic.com>
Fixes pure Dockerfile deployment failing with 'env file not found' error.
The deploy_simple_dockerfile() method was missing the call to
save_runtime_environment_variables() which creates the .env file
needed during the rolling update phase. This call is present in
all other deployment methods (dockerfile, dockercompose, nixpacks,
static) but was missing here.
This ensures the .env file exists when docker compose tries to
use --env-file during the rolling update.
- Strip leading slashes from publish_directory to prevent /app// paths
- Only add slash prefix if directory is not empty
- Ensures clean Docker COPY paths in build output
Fixes deployment failures when Git repositories redirect (e.g., tangled.sh → tangled.org)
and improves security by adding proper shell escaping for repository URLs.
**Root Cause:**
Git redirect warnings can appear on the same line as ls-remote output with no newline:
`warning: redirecting to https://tangled.org/...196d3df... refs/heads/master`
The previous parsing logic split by newlines and extracted text before tabs, which
included the entire warning message instead of just the 40-character commit SHA.
**Changes:**
1. **Fixed commit SHA extraction** (ApplicationDeploymentJob.php):
- Changed from line-based parsing to regex pattern matching
- Uses `/([0-9a-f]{40})\s*\t/` to find valid 40-char hex commit SHA before tab
- Handles warnings on same line, separate lines, multiple warnings, and whitespace
- Added comprehensive Ray debug logs for troubleshooting
2. **Added security fix** (Application.php):
- Added `escapeshellarg()` for repository URLs in 'other' deployment type
- Prevents shell injection and fixes parsing issues with special characters like `@`
- Added Ray debug logs for deployment type tracking
3. **Comprehensive test coverage** (GitLsRemoteParsingTest.php):
- Tests normal output without warnings
- Tests redirect warning on separate line
- Tests redirect warning on same line (actual tangled.sh format)
- Tests multiple warning lines
- Tests extra whitespace handling
**Resolves:**
- Linear issue COOLGH-53: Valid git URLs are rejected as being invalid
- GitHub issue #6568: tangled.sh deployments failing
- Handles Git redirects universally for all Git hosting services
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Updated the should_skip_build method to save runtime environment variables even when the build step is skipped, ensuring that the latest environment settings are preserved.
- Enhanced logging in prepare_builder_image to differentiate between the first attempt and subsequent attempts, improving clarity in deployment logs.
- Implemented a check for the NIXPACKS_NODE_VERSION variable in the ApplicationDeploymentJob.
- Added log entries to notify users when NIXPACKS_NODE_VERSION is not set, indicating that Node.js 18 will be used by default, which is EOL.
- Provided guidance on how to override the default version by setting NIXPACKS_NODE_VERSION in environment variables.
- Changed the .env file reference to use a build-time .env file located in /artifacts, preventing it from being included in Docker images.
- Introduced a new method to wrap Docker build commands with environment variable exports, enhancing the handling of build-time variables.
- Updated logging messages for clarity regarding the creation and usage of the build-time .env file.
