Skip to content

Conversation

@andrasbacsai
Copy link
Member

@andrasbacsai andrasbacsai commented Nov 6, 2025

Summary by CodeRabbit

  • Style

    • Refreshed visual styling and color scheme across authentication views for improved consistency, including login, registration, password reset, and two-factor challenge pages
  • Chores

    • Enhanced deployment infrastructure for improved reliability and process consistency
    • Improved HTML content sanitization and base layout structure enhancements

@andrasbacsai andrasbacsai changed the title Enhance Dockerfile handling for environment variables fix: envs added to the right place in dockerfiles Nov 6, 2025
@andrasbacsai
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@andrasbacsai andrasbacsai linked an issue Nov 6, 2025 that may be closed by this pull request
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

The PR refactors Dockerfile ARG insertion logic in ApplicationDeploymentJob by extracting a helper method to find FROM instructions and reorganizing ARG placement to occur after each FROM line. It introduces comprehensive unit tests for this functionality, updates CSS base styling from neutral-50 to gray-50, applies minor UI/formatting refinements across authentication views, and substantially restructures base.blade.php with DOMPurify hook integration and theme initialization reorganization.

Changes

Cohort / File(s) Summary
Dockerfile Build Logic & Tests
app/Jobs/ApplicationDeploymentJob.php, tests/Unit/DockerfileArgInsertionTest.php
Introduced findFromInstructionLines() helper method to locate FROM instruction positions. Refactored ARG insertion from inline splicing to batch insertion after each FROM instruction. Added comprehensive test suite covering single-stage, multi-stage, mixed-case FROM detection, and edge cases.
Base Styling
resources/css/app.css
Changed base HTML/body background utility from bg-neutral-50 to bg-gray-50 in Tailwind @apply directive.
Authentication Views
resources/views/auth/login.blade.php, resources/views/auth/register.blade.php, resources/views/auth/reset-password.blade.php, resources/views/auth/two-factor-challenge.blade.php
Updated light-mode background styling across auth forms to use bg-gray-50 instead of bg-neutral-50. Applied minor formatting adjustments and line-break restructuring in templates without altering control flow or structure.
Base Layout & JavaScript
resources/views/layouts/base.blade.php
Reformatted JavaScript block with added body classes, idempotent DOMPurify hook registration guarded by window.__dpLinkHook, centralized theme initialization logic, expanded Livewire event handler bindings (info, error, warning, success), and improved code organization without behavioral changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • app/Jobs/ApplicationDeploymentJob.php: Verify ARG insertion logic maintains correct line numbering when iterating in reverse across multiple FROM instructions; ensure batch collection and insertion preserves intended ordering.
  • tests/Unit/DockerfileArgInsertionTest.php: Confirm test coverage adequately validates multi-stage Dockerfiles and edge cases (no FROM, mixed-case variants, comments before FROM).
  • resources/views/layouts/base.blade.php: Validate DOMPurify hook idempotency guard and that theme initialization logic doesn't introduce initialization timing issues; review Livewire event handler expansion for completeness.
  • CSS and view styling changes: Confirm bg-gray-50 consistency across all updated files and verify no unintended visual regressions in light/dark theme scenarios.

Possibly related PRs

  • feat(docker): add build secrets #6629: Modifies ApplicationDeploymentJob.php's ARG handling to add BuildKit/secret-aware paths that also touch add_build_env_variables_to_dockerfile, directly overlapping with this PR's refactoring.
  • v4.0.0-beta.434 #6755: Modifies ApplicationDeploymentJob.php's Dockerfile build-time variable handling to refactor ARG/FROM line detection logic, related to the core refactoring in this PR.

Suggested reviewers

  • peaklabs-dev

Poem

🐰 FROM lines now gathered with care,
ARGs placed after, everywhere!
DOMPurify hooks idempotent and true,
Gray backgrounds unified, fresh and new,
Tests blooming to verify each stage—
A Dockerfile refactor for the ages! 🌱

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request has no description provided, missing all required template sections including Changes and Issues. Add a pull request description following the template: include a Changes section detailing the modifications and an Issues section with the related issue link.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing the placement of environment variables in Dockerfiles after FROM instructions.
✨ 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 feat-add-dockerfile-from-instruction-par

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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23a3b47 and 4968e9f.

📒 Files selected for processing (8)
  • app/Jobs/ApplicationDeploymentJob.php (6 hunks)
  • resources/css/app.css (1 hunks)
  • resources/views/auth/login.blade.php (2 hunks)
  • resources/views/auth/register.blade.php (3 hunks)
  • resources/views/auth/reset-password.blade.php (2 hunks)
  • resources/views/auth/two-factor-challenge.blade.php (1 hunks)
  • resources/views/layouts/base.blade.php (3 hunks)
  • tests/Unit/DockerfileArgInsertionTest.php (1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
resources/**

📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)

Store frontend assets and views under resources

Files:

  • resources/css/app.css
  • resources/views/auth/two-factor-challenge.blade.php
  • resources/views/auth/login.blade.php
  • resources/views/auth/register.blade.php
  • resources/views/auth/reset-password.blade.php
  • resources/views/layouts/base.blade.php
resources/**/*.css

📄 CodeRabbit inference engine (.cursor/rules/laravel-boost.mdc)

In Tailwind v4, import Tailwind via @import "tailwindcss" instead of @tailwind directives

Files:

  • resources/css/app.css
**/*.php

📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)

**/*.php: Follow PSR-12 coding standards for all PHP code
Format PHP code with Laravel Pint configuration
Run static analysis with PHPStan to ensure type safety in PHP code
Document complex methods with PHPDoc blocks including parameters, return types, and thrown exceptions

**/*.php: Follow PSR-12 PHP coding standards across the codebase
Prefer eager loading and query optimization to prevent N+1 issues in database interactions
Use Laravel best practices for structure, services, and policies

**/*.php: Always use curly braces for control structures in PHP, even for single-line bodies
Use PHP 8 constructor property promotion in __construct() and avoid empty constructors
Always declare explicit return types for functions and methods; add appropriate parameter type hints
Prefer PHPDoc blocks over inline comments; only add inline code comments for very complex logic
When documenting arrays, add useful array shape types in PHPDoc where appropriate
Enum case names should be TitleCase (e.g., FavoritePerson, Monthly)

Files:

  • resources/views/auth/two-factor-challenge.blade.php
  • tests/Unit/DockerfileArgInsertionTest.php
  • resources/views/auth/login.blade.php
  • resources/views/auth/register.blade.php
  • resources/views/auth/reset-password.blade.php
  • resources/views/layouts/base.blade.php
  • app/Jobs/ApplicationDeploymentJob.php
resources/views/**/*.blade.php

📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)

Use semantic Tailwind CSS classes and consistent spacing in Blade templates

resources/views/**/*.blade.php: In Blade views, prefer using x-forms components with canGate and :canResource instead of wrapping elements in @can/@else blocks
Remove legacy @can/@else blocks around individual form inputs/buttons and migrate to the single-line component pattern
Choose gates consistently in views: use update for configuration changes, deploy for operational actions, view for read-only access, and delete for destructive actions
Always pass the specific resource to :canResource (e.g., :canResource="$application", "$service", "$server"), and use team context for creation permissions (e.g., :canResource="auth()->user()->currentTeam" with canGate="createAnyResource")
Only set autoDisable="false" when also providing explicit :disabled logic on the component
Use single-line authorized components for forms (inputs, selects, checkboxes, buttons) to reduce duplication (e.g., <x-forms.input canGate="update" :canResource="$resource" ... />)

resources/views/**/*.blade.php: In Blade views, prefer x-forms.* components with canGate and :canResource for authorization (autoDisable defaults to true) instead of manual permission blocks.
Do not wrap x-forms.* components in @can/@else to toggle disabled state; use component-level canGate/:canResource instead.
For custom interactive components not using x-forms.* (e.g., Alpine.js widgets), wrap with @can('gate', $resource) and provide a disabled/readonly fallback in @else with opacity-50 and cursor-not-allowed styling.

resources/views/**/*.blade.php: Prefer named routes and the route() helper when generating links in views
Dispatch events using $this->dispatch() (not emit/dispatchBrowserEvent) in Livewire 3 templates/components
Do not manually include Alpine.js; Livewire 3 bundles Alpine with required plugins

Files:

  • resources/views/auth/two-factor-challenge.blade.php
  • resources/views/auth/login.blade.php
  • resources/views/auth/register.blade.php
  • resources/views/auth/reset-password.blade.php
  • resources/views/layouts/base.blade.php
resources/**/*.{blade.php,vue}

📄 CodeRabbit inference engine (.cursor/rules/laravel-boost.mdc)

Use Tailwind gap utilities for spacing lists instead of margins when listing items

Files:

  • resources/views/auth/two-factor-challenge.blade.php
  • resources/views/auth/login.blade.php
  • resources/views/auth/register.blade.php
  • resources/views/auth/reset-password.blade.php
  • resources/views/layouts/base.blade.php
tests/Unit/**/*.php

📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)

Place unit tests under tests/Unit and keep them focused on individual classes/methods

tests/Unit/**/*.php: Unit tests (tests/Unit) must not use database connections or model factories
Unit tests must use mocking for models and external dependencies

Files:

  • tests/Unit/DockerfileArgInsertionTest.php
tests/**/*.php

📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)

Write feature tests for API endpoints using Pest; mock external services and assert JSON responses and side effects

Add tests validating that components respect authorization (e.g., unauthorized users see disabled inputs; checkbox instantSave becomes false)

Include security tests covering SQL injection resistance, XSS validation, and enforcement of team isolation (403 on cross-team access).

Write tests using Pest (feature/unit) and include Laravel Dusk for browser scenarios as needed

Prefer Pest PHP as the primary testing framework for writing tests

Files:

  • tests/Unit/DockerfileArgInsertionTest.php
tests/**

📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)

Keep automated tests (Pest/Dusk) under tests

Files:

  • tests/Unit/DockerfileArgInsertionTest.php
tests/{Feature,Unit}/**/*.php

📄 CodeRabbit inference engine (.cursor/rules/laravel-boost.mdc)

tests/{Feature,Unit}/**/*.php: All tests must be written using Pest and live under tests/Feature or tests/Unit
Use specific response assertion helpers (e.g., assertForbidden, assertNotFound) instead of generic assertStatus
Use datasets in Pest to reduce duplication where suitable (e.g., validation tests)
Write or update unit/feature tests for every change and run the affected tests

Files:

  • tests/Unit/DockerfileArgInsertionTest.php
app/Jobs/**/*.php

📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)

Queueable background work should be implemented as Jobs under app/Jobs; Actions may expose asJob to dispatch

app/Jobs/**/*.php: Queue jobs should implement ShouldQueue and define retry strategy (tries, maxExceptions, backoff)
Queue jobs must handle success/failure state updates and broadcast corresponding events; implement failed(Throwable) for final failure handling

Queue jobs should update status, wrap external calls in try/catch, set failure status on exceptions, and rethrow

Files:

  • app/Jobs/ApplicationDeploymentJob.php
app/Jobs/*.php

📄 CodeRabbit inference engine (.cursor/rules/deployment-architecture.mdc)

Implement background deployment, monitoring, backup, and notification work as Laravel Job classes under app/Jobs

Files:

  • app/Jobs/ApplicationDeploymentJob.php
app/**/*.php

📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)

Use database transactions to group related write operations for consistency in services/controllers/jobs

Files:

  • app/Jobs/ApplicationDeploymentJob.php
app/Jobs/**

📄 CodeRabbit inference engine (.cursor/rules/project-overview.mdc)

Implement background jobs under app/Jobs

Files:

  • app/Jobs/ApplicationDeploymentJob.php
🧠 Learnings (9)
📚 Learning: 2025-10-15T09:16:25.875Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/laravel-boost.mdc:0-0
Timestamp: 2025-10-15T09:16:25.875Z
Learning: Applies to resources/**/*.css : In Tailwind v4, import Tailwind via import "tailwindcss" instead of tailwind directives

Applied to files:

  • resources/css/app.css
📚 Learning: 2025-10-15T09:17:35.563Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/testing-patterns.mdc:0-0
Timestamp: 2025-10-15T09:17:35.563Z
Learning: Feature tests must run inside a Docker container

Applied to files:

  • tests/Unit/DockerfileArgInsertionTest.php
📚 Learning: 2025-08-27T15:02:37.804Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/form-components.mdc:0-0
Timestamp: 2025-08-27T15:02:37.804Z
Learning: Applies to resources/views/**/*.blade.php : Use single-line authorized components for forms (inputs, selects, checkboxes, buttons) to reduce duplication (e.g., <x-forms.input canGate="update" :canResource="$resource" ... />)

Applied to files:

  • resources/views/auth/login.blade.php
  • resources/views/auth/register.blade.php
📚 Learning: 2025-08-27T15:02:37.804Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/form-components.mdc:0-0
Timestamp: 2025-08-27T15:02:37.804Z
Learning: Applies to resources/views/**/*.blade.php : Remove legacy can/else blocks around individual form inputs/buttons and migrate to the single-line component pattern

Applied to files:

  • resources/views/auth/login.blade.php
  • resources/views/auth/register.blade.php
  • resources/views/auth/reset-password.blade.php
  • resources/views/layouts/base.blade.php
📚 Learning: 2025-08-27T15:01:58.710Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/development-workflow.mdc:0-0
Timestamp: 2025-08-27T15:01:58.710Z
Learning: Applies to resources/views/**/*.blade.php : Use semantic Tailwind CSS classes and consistent spacing in Blade templates

Applied to files:

  • resources/views/layouts/base.blade.php
📚 Learning: 2025-10-15T09:16:25.875Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/laravel-boost.mdc:0-0
Timestamp: 2025-10-15T09:16:25.875Z
Learning: Applies to resources/views/**/*.blade.php : Do not manually include Alpine.js; Livewire 3 bundles Alpine with required plugins

Applied to files:

  • resources/views/layouts/base.blade.php
📚 Learning: 2025-08-27T15:04:22.911Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/project-overview.mdc:0-0
Timestamp: 2025-08-27T15:04:22.911Z
Learning: Use Laravel for the backend with Livewire for the frontend

Applied to files:

  • resources/views/layouts/base.blade.php
📚 Learning: 2025-08-27T14:59:11.004Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/application-architecture.mdc:0-0
Timestamp: 2025-08-27T14:59:11.004Z
Learning: Applies to app/Livewire/**/*.php : Define Livewire components in app/Livewire and keep HTTP/UI concerns there

Applied to files:

  • resources/views/layouts/base.blade.php
📚 Learning: 2025-08-27T15:06:41.387Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/technology-stack.mdc:0-0
Timestamp: 2025-08-27T15:06:41.387Z
Learning: Applies to app/Livewire/**/*.php : Place Livewire components under app/Livewire/ (e.g., Dashboard.php, ActivityMonitor.php, MonacoEditor.php)

Applied to files:

  • resources/views/layouts/base.blade.php
🔇 Additional comments (1)
tests/Unit/DockerfileArgInsertionTest.php (1)

9-21: Tests crash: protected method is inaccessible

findFromInstructionLines is protected, but the tests call it directly on the partial mock. Without shouldAllowMockingProtectedMethods() (or an exposed proxy), PHP throws “Call to protected method”. The suite never reaches its assertions.

Please expose the helper via a test seam (e.g., subclass wrapper or reflection) before calling it, or switch to exercising the public API that uses this helper.

⛔ Skipped due to learnings
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/testing-patterns.mdc:0-0
Timestamp: 2025-10-15T09:17:35.563Z
Learning: Applies to tests/Unit/**/*.php : Unit tests must use mocking for models and external dependencies

@andrasbacsai andrasbacsai merged commit 395d225 into next Nov 6, 2025
5 checks passed
@andrasbacsai andrasbacsai deleted the feat-add-dockerfile-from-instruction-par branch November 6, 2025 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Build arguments added incorrectly to Dockerfile

2 participants