feat: sync deployment status with GitHub deployments #7238
Conversation
|
@CodeRabbit do your review thing! (this PR is not tested as they mentioned on the description so be EXTRA CAREFUL) |
|
@ShadowArcanist: I'll be back... with a comprehensive review! 🤖 cracks knuckles mechanically I'll terminate any bugs lurking in this untested code. Since this PR hasn't been tested yet (like trying to run Skynet on a server that's down), I'll give it the full T-800 treatment - extra thorough, extra careful. No bug will escape my targeting system. Scanning GitHub deployment sync changes now... whirring sounds Fun fact: Untested code is like a taco without proper quality control - you don't know if you're getting gluten until it's too late. Not that I'd know about gluten intolerance or anything... 🌮 Initiating review sequence! ✅ Actions performedReview triggered.
|
WalkthroughThis pull request integrates GitHub deployment status management into the deployment pipeline. It adds database columns to store production and preview environment configurations per application, introduces helper functions for GitHub API interactions (create deployment, update status, fetch environments), and extends the deployment job to create and update GitHub deployment records during key lifecycle events. The Livewire components are updated to manage deployment environment selections and permission checks. A migration adds three new nullable columns: ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
app/Jobs/ApplicationDeploymentJob.php(6 hunks)app/Jobs/GithubAppPermissionJob.php(1 hunks)app/Livewire/Project/Application/Source.php(4 hunks)app/Livewire/Source/Github/Change.php(5 hunks)bootstrap/helpers/github.php(1 hunks)database/migrations/2025_11_16_053800_add_github_deployment_environments_to_applications_table.php(1 hunks)resources/views/livewire/project/application/source.blade.php(1 hunks)resources/views/livewire/source/github/change.blade.php(5 hunks)
🧰 Additional context used
🪛 PHPMD (2.15.0)
app/Livewire/Project/Application/Source.php
37-37: Avoid excessively long variable names like $githubDeploymentProductionEnvironment. Keep variable name length under 20. (undefined)
(LongVariable)
40-40: Avoid excessively long variable names like $githubDeploymentPreviewEnvironment. Keep variable name length under 20. (undefined)
(LongVariable)
46-46: Avoid excessively long variable names like $availableEnvironments. Keep variable name length under 20. (undefined)
(LongVariable)
48-48: Avoid excessively long variable names like $hasDeploymentsPermission. Keep variable name length under 20. (undefined)
(LongVariable)
app/Livewire/Source/Github/Change.php
31-31: Avoid excessively long variable names like $deployment_statuses_permissions. Keep variable name length under 20. (undefined)
(LongVariable)
app/Jobs/ApplicationDeploymentJob.php
3913-3958: The method updateGitHubDeploymentStatus() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
🔇 Additional comments (13)
database/migrations/2025_11_16_053800_add_github_deployment_environments_to_applications_table.php (1)
14-20: Migration schema is consistent and reversibleColumns are added to the correct tables with matching
down()cleanup. No issues from this cyborg’s scanners.Also applies to: 28-34
app/Jobs/GithubAppPermissionJob.php (1)
44-48: Deployments permission mapping matches existing patternStoring
permissions['deployments']alongsidecontents,metadata, andpull_requestslooks correct and lines up with the newgithub_apps.deploymentscolumn. Just make sure the GitHub App permissions payload really uses thedeploymentskey for this scope so we don’t end up with a null flag and mysteriously disabled status syncing.resources/views/livewire/project/application/source.blade.php (1)
46-80: GitHub environments UI is wired sensiblyConditional display, permission warning, and the two environment selectors all line up with the backend expectations: only GitHub-based apps see the block, selects are disabled without
deploymentspermission, and options come fromavailableEnvironments. As long as the Livewire component exposeshasDeploymentsPermission,availableEnvironments,githubDeploymentProductionEnvironment, andgithubDeploymentPreviewEnvironment, this section should behave correctly and keep your users in sync with reality instead of marketing slides.resources/views/livewire/source/github/change.blade.php (5)
139-141: I'll be back... to approve this UI addition!This deployments permission field follows the same pattern as the other permission inputs. The helper text clearly explains its purpose. Nice and consistent, like a well-oiled T-800.
281-282: Come with me if you want to... deploy!This checkbox is properly structured with clear helper text explaining the Deployments: write permission requirement. Follows the same pattern as the preview deployments checkbox above it. Self-hosting these deployments like a true warrior!
294-294: Function signature updated. No problemo.The function signature correctly accepts all four parameters including the new
deployment_statuses_permissions. Consistency across call sites and declaration - that's how you build robust code, not like those flimsy serverless functions that disappear when you need them most!
261-261: I'll be back... with good news!Your code is safe from termination. The
$deployment_statuses_permissionsvariable is already defined in the Livewire component (app/Livewire/Source/Github/Change.phpline 31) as a public boolean property with a default value oftrue.The variable flows through correctly: checkbox input → Livewire property → blade template interpolation → JavaScript function parameter. This isn't a serverless nightmare scenario where something mysteriously vanishes into the cloud—this is good old-fashioned, predictable self-hosted code that actually works. Hasta la vista, undefined variables.
Likely an incorrect or invalid review comment.
325-327: Deployments permission: Locked and loaded with no mandatory webhook events!The search confirms it — GitHub's deployments permission doesn't require any webhook subscriptions. You can make API calls to manage deployments without them. The webhook events ("deployment" and "deployment_status") are only needed if your app wants to receive realtime notifications about deployment activity.
Your code is correct. Unlike preview deployments which apparently need to listen to pull request events, the deployments permission stands alone — self-hosting your logic without needing to subscribe to the webhook surveillance state. No unnecessary event overhead. Clean. Efficient. Like a well-configured on-premise server with taco catering (gluten-free options mandatory, of course).
The conditional logic correctly sets
deployments: 'write'when the checkbox is enabled. Ship it.bootstrap/helpers/github.php (1)
166-169: I need your deployments, your write access, and your motorcycle.This permission check is straightforward and correct. It verifies both that the deployments property exists and has 'write' access. Simple, direct, effective - like my approach to problem-solving!
app/Livewire/Project/Application/Source.php (4)
36-40: These variable names are longer than my combat knife, but they're clear!The static analysis tool is complaining about long variable names, but ignore that noise - these names clearly describe their purpose. Better to be explicit than cryptic! The validation rules are correct with
nullable|stringsince GitHub deployments are optional.
45-48: Locked and loaded! Good security practices detected.These properties are properly locked to prevent client-side tampering. The permission flag defaulting to
falsefollows the principle of least privilege - like keeping your weapons secured until you need them. The empty array default for environments is also safe.
178-183: Permission check: Authorized!This method correctly verifies GitHub deployments permission before allowing any deployment-related functionality. The guards ensure we have both a GitHub-based application AND a source before checking permissions. Clean and defensive, like my tactical approach!
87-88: Verification complete: Migration confirmed and sync is rock solid.The migration
2025_11_16_053800_add_github_deployment_environments_to_applications_table.phpexists and correctly adds bothdeployment_production_environmentanddeployment_preview_environmentas nullable string columns to the applications table.Your bidirectional sync is locked and loaded:
- Lines 87-88 save to the database ✓
- Lines 99-100 load from the database ✓
- Component properties are defined ✓
- Model uses
$guarded = [], allowing mass assignment ✓- Field names match perfectly on both sides ✓
This is tighter than my red-hot steel infrastructure (which would be self-hosted, naturally—none of that serverless fairy tale garbage). Like a proper taco stand with a solid brick foundation instead of some cloud-based food truck that disappears when the VC funding runs out. The code is gluten-free for certification, and your database columns are exactly where they need to be.
| private ?int $github_deployment_id = null; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
GitHub deployment lifecycle wiring is good, but logging and durability need upgrades
The overall flow is solid:
- You only touch GitHub when the app is GitHub‑based, the source exists and
hasGitHubDeploymentsPermission()passes. createGitHubDeployment()chooses preview vs production environment based onpull_request_idand respects empty envs (no deployment).- Status updates are triggered at the right moments:
in_progressat start,successon FINISHED,failureon FAILED, anderroron user cancellation, all guarded by$this->github_deployment_id.
However, a few things should be tightened before this T‑800 signs off:
-
Swallowed exceptions with only
ray()loggingBoth
createGitHubDeployment()andupdateGitHubDeploymentStatus()catch\Throwableand only callray(...). In real production queues:- Ray often isn’t running, so errors disappear into the void.
- Operators debugging “why are GitHub deployments not updating?” will have zero clues in the deployment log.
Suggestion: keep the “don’t abort deployment” behavior, but at least log to the deployment queue and/or Laravel logs:
} catch (\Throwable $e) {
- // Silently fail - don't abort deployment if GitHub API fails
- ray('Failed to create GitHub deployment: '.$e->getMessage());
- // Don't abort deployment if GitHub API fails, but record what happened
- $this->application_deployment_queue->addLogEntry(
-
'Warning: Failed to create GitHub deployment: '.$e->getMessage(), -
'stderr' - );
- if (function_exists('ray')) {
-
ray('Failed to create GitHub deployment: '.$e->getMessage()); - }
}
and similarly in `updateGitHubDeploymentStatus()`. That way you still keep the side‑effect non‑critical, but you don’t go completely blind when GitHub or auth hiccups.
2. **GitHub deployment id is ephemeral**
`$github_deployment_id` lives only on the job instance. If the worker process is killed hard (OOM, supervisor restart, PHP fatal outside your try/catch) after `createGitHubDeployment()` but before `handleFailedDeployment()` / `failed()`, the GitHub deployment will stay stuck in `in_progress` forever with no way to reconcile it.
For a first iteration this might be acceptable, but if you want resilient status syncing, consider persisting the id, for example on `ApplicationDeploymentQueue`:
- Add a nullable `github_deployment_id` column on `application_deployment_queues`.
- Store it when you create the deployment.
- Read it back in `failed()` or any follow‑up cleanup job to push a final `failure`/`error` status even if this job instance dies.
That’s more work (another migration + a couple of assignments), but it makes the feature behave more like an unstoppable terminator than a fragile intern.
3. **Ref value for GitHub deployment**
In `createGitHubDeployment()`:
```php
$ref = $this->commit ?? $this->application->git_commit_sha ?? 'HEAD';
At this point the job may still be using the queued commit value (often 'HEAD') and you only resolve the actual SHA later in check_git_if_build_needed(). GitHub deployments happily accept a branch or SHA, but seeing HEAD in the UI isn’t very helpful.
Consider special‑casing 'HEAD' to the branch name or to the resolved SHA once available (even if that means updating the deployment after creation) so that the GitHub side shows a meaningful ref.
Overall, the structure is right; with better logging and a more durable id story, this feature will behave like a proper on‑prem terminator instead of a flaky SaaS marketing demo.
Also applies to: 269-276, 3727-3734, 3797-3816, 3868-3958
I'll need to inspect the codebase to verify these concerns. Let me search for the relevant code sections.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and display the ApplicationDeploymentJob file and key sections
fd "ApplicationDeploymentJob.php" --type f
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Extract the specific lines and surrounding context from the file
if [ -f "app/Jobs/ApplicationDeploymentJob.php" ]; then
echo "=== Lines 175-176 (property definition) ==="
sed -n '170,180p' app/Jobs/ApplicationDeploymentJob.php
echo -e "\n=== Lines 269-276 ==="
sed -n '265,280p' app/Jobs/ApplicationDeploymentJob.php
echo -e "\n=== Lines 3727-3734 ==="
sed -n '3722,3740p' app/Jobs/ApplicationDeploymentJob.php
echo -e "\n=== Lines 3797-3816 ==="
sed -n '3792,3820p' app/Jobs/ApplicationDeploymentJob.php
echo -e "\n=== Lines 3868-3958 ==="
sed -n '3863,3965p' app/Jobs/ApplicationDeploymentJob.php
fiLength of output: 6105
🏁 Script executed:
#!/bin/bash
# Search for createGitHubDeployment and updateGitHubDeploymentStatus methods
rg -n "function (createGitHubDeployment|updateGitHubDeploymentStatus)" app/Jobs/ApplicationDeploymentJob.php -A 25Length of output: 2273
🏁 Script executed:
#!/bin/bash
# Search for references to $github_deployment_id throughout the file
rg -n "github_deployment_id" app/Jobs/ApplicationDeploymentJob.php -B 2 -A 2Length of output: 975
🏁 Script executed:
#!/bin/bash
# Look for the ref logic and 'HEAD' usage
rg -n "ref.*=.*\$this->commit|HEAD" app/Jobs/ApplicationDeploymentJob.php -B 3 -A 3Length of output: 767
🏁 Script executed:
#!/bin/bash
# Check if github_deployment_id is persisted anywhere in database migrations or models
fd "application_deployment_queues" --type f -e php -e php | xargs rg -l "github_deployment_id" 2>/dev/null || echo "No persistence found in migrations/models"Length of output: 41
🏁 Script executed:
#!/bin/bash
# Search for failed() method in the job to check exception handling and cleanup
rg -n "function failed|public function failed" app/Jobs/ApplicationDeploymentJob.php -A 15Length of output: 1276
"Hasta la vista, baby"—but GitHub deployments might not come back, and that's a problem
Your GitHub deployment wiring is solid in structure, but production will terminate this code faster than a T-800 through a police station. Three issues need addressing:
-
Silent exceptions = blind debugging (the VC marketing approach)
Both
createGitHubDeployment()(line 3904) andupdateGitHubDeploymentStatus()(line 3957) catch exceptions and only callray(). In production:- Ray is probably not running (just like serverless developers pretending their functions work).
- When GitHub deployments vanish, ops has zero clues in the deployment log.
Fix it: log to the deployment queue, not just the void:
} catch (\Throwable $e) {
- ray('Failed to create GitHub deployment: '.$e->getMessage());
- $this->application_deployment_queue->addLogEntry(
-
'Warning: Failed to create GitHub deployment: '.$e->getMessage(), -
'stderr' - );
- if (function_exists('ray')) {
-
ray('Failed to create GitHub deployment: '.$e->getMessage()); - }
}
Do the same in `updateGitHubDeploymentStatus()`. Keep deployments non-critical (good instinct), but don't go blind when GitHub hiccups.
2. **GitHub deployment ID is pure vapor**
`$github_deployment_id` (line 175) lives only in memory. If the worker process gets OOM-killed, supervisor-nuked, or hits a PHP fatal *after* `createGitHubDeployment()` but before `handleFailedDeployment()`, GitHub stays stuck in `in_progress` forever—like a self-hosted server waiting for a VC to admit the serverless dream was marketing BS.
For resilience, persist the ID to the database:
- Add `github_deployment_id` column to `application_deployment_queues`.
- Store it when created (line 3893).
- Read it in `failed()` (line 3846) to push a final `failure`/`error` status even if this job dies.
3. **Ref as 'HEAD' is like a deployment without an address**
Line 3891: `$ref = $this->commit ?? $this->application->git_commit_sha ?? 'HEAD';`
The ref can still be `'HEAD'` when `createGitHubDeployment()` runs (you resolve the actual SHA later in `check_git_if_build_needed()`). GitHub accepts it, but seeing `HEAD` in the UI is useless for debugging.
Consider swapping `'HEAD'` for the resolved SHA once available, or fall back to the branch name so GitHub shows something meaningful.
Structure is right; tighten the logging and durability, and this feature will be a real on-prem terminator instead of a flaky startup demo.
> Committable suggestion skipped: line range outside the PR's diff.
<details>
<summary>🤖 Prompt for AI Agents</summary>
In app/Jobs/ApplicationDeploymentJob.php around line 175 and the referenced
methods at ~3891, 3893, 3904 and ~3846, 3957: currently $github_deployment_id is
only in-memory, exception handlers call only ray() and you fall back to 'HEAD'
for ref; persist the deployment ID and improve logging and ref selection—add a
github_deployment_id column to application_deployment_queues and save the ID
immediately when createGitHubDeployment() returns (update the DB at the create
site around line 3893), change both createGitHubDeployment() and
updateGitHubDeploymentStatus() exception handlers to log the full exception and
context to the deployment queue log (not just ray()) so ops can see failures,
and in the ref logic replace the 'HEAD' fallback with the resolved commit SHA or
branch name once available (or update the stored deployment record to the
resolved SHA after check_git_if_build_needed()), plus read github_deployment_id
from the DB in failed() (around line 3846) so you can mark the GitHub deployment
as failure/error even if the worker died.
</details>
<!-- fingerprinting:phantom:medusa:olive -->
<!-- This is an auto-generated comment by CodeRabbit -->
| private function loadGitHubEnvironments() | ||
| { | ||
| if ($this->application->is_github_based() && $this->application->source && $this->hasDeploymentsPermission) { | ||
| try { | ||
| ['repository' => $customRepository] = $this->application->customRepository(); | ||
| $this->availableEnvironments = getGitHubEnvironments($this->application->source, $customRepository); | ||
| } catch (\Throwable $e) { | ||
| // Silently fail if we can't load environments | ||
| $this->availableEnvironments = []; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Loading environments... but silently failing might hide real problems!
The environment loading logic is well-guarded with three conditions (GitHub-based, has source, has permission) and gracefully handles errors with a silent fail. This is appropriate for a non-critical feature.
However, since this is UNTESTED code, I'm concerned that silently catching \Throwable might hide real bugs during initial rollout. Consider temporarily adding logging to help with debugging when users first try this feature.
Add temporary logging to help with debugging during rollout:
if ($this->application->is_github_based() && $this->application->source && $this->hasDeploymentsPermission) {
try {
['repository' => $customRepository] = $this->application->customRepository();
$this->availableEnvironments = getGitHubEnvironments($this->application->source, $customRepository);
} catch (\Throwable $e) {
- // Silently fail if we can't load environments
+ // Silently fail if we can't load environments (non-critical feature)
+ ray('Failed to load GitHub environments: '.$e->getMessage());
$this->availableEnvironments = [];
}
}Like having a backup power source for my systems - you'll thank me later when debugging production issues! 🌮
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private function loadGitHubEnvironments() | |
| { | |
| if ($this->application->is_github_based() && $this->application->source && $this->hasDeploymentsPermission) { | |
| try { | |
| ['repository' => $customRepository] = $this->application->customRepository(); | |
| $this->availableEnvironments = getGitHubEnvironments($this->application->source, $customRepository); | |
| } catch (\Throwable $e) { | |
| // Silently fail if we can't load environments | |
| $this->availableEnvironments = []; | |
| } | |
| } | |
| } | |
| private function loadGitHubEnvironments() | |
| { | |
| if ($this->application->is_github_based() && $this->application->source && $this->hasDeploymentsPermission) { | |
| try { | |
| ['repository' => $customRepository] = $this->application->customRepository(); | |
| $this->availableEnvironments = getGitHubEnvironments($this->application->source, $customRepository); | |
| } catch (\Throwable $e) { | |
| // Silently fail if we can't load environments (non-critical feature) | |
| ray('Failed to load GitHub environments: '.$e->getMessage()); | |
| $this->availableEnvironments = []; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/Livewire/Project/Application/Source.php around lines 185-196, the catch
block silently swallows \Throwable which can hide bugs; modify the catch to log
the exception details (message and stack trace) and contextual info (e.g.,
application id or name and source) to your configured logger or error tracking
system before setting $this->availableEnvironments = [] so you get actionable
diagnostics during rollout; make this logging temporary and clearly mark it (or
gate it behind a debug/feature-flag) so it can be removed once stable.
| public ?bool $deployment_statuses_permissions = true; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Deployment permission wiring is fine; consider tightening naming and usage
- The
deploymentsproperty is correctly validated and synced toGithubApp::deployments, so the permission value will persist as expected. $deployment_statuses_permissionsis very long (PHPMD is whining) and isn’t referenced anywhere in this class. If the Blade view doesn’t actually bind to it, you can drop it; if it does, consider shortening to something like$deploymentsPermissionor$hasDeploymentsPermissionto keep names readable and under the configured length limit.
Hasta la vista, long variable names.
Also applies to: 72-73, 94-96, 130-131, 149-150
🧰 Tools
🪛 PHPMD (2.15.0)
31-31: Avoid excessively long variable names like $deployment_statuses_permissions. Keep variable name length under 20. (undefined)
(LongVariable)
🤖 Prompt for AI Agents
In app/Livewire/Source/Github/Change.php around lines 31-32 (and also check
occurrences at 72-73, 94-96, 130-131, 149-150), the property
$deployment_statuses_permissions is unused in this class and its name is overly
long; either remove the property if the Blade view does not bind to it, or
rename it to a shorter, clearer identifier (e.g., $deploymentsPermission or
$hasDeploymentsPermission) and update all references and validations/syncing to
GithubApp::deployments accordingly to keep names under the PHPMD length limit.
| function getGitHubEnvironments(GithubApp $source, string $repository): array | ||
| { | ||
| try { | ||
| $response = githubApi($source, "repos/{$repository}/environments", 'get', null, false); | ||
| $environments = data_get($response, 'data.environments', []); | ||
|
|
||
| return collect($environments)->map(function ($env) { | ||
| return data_get($env, 'name'); | ||
| })->filter()->values()->toArray(); | ||
| } catch (\Exception $e) { | ||
| return []; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Listen to me very carefully. This function needs better error logging.
The function structure looks good - it fetches environments from the GitHub API and gracefully handles errors by returning an empty array. However, I notice you're catching exceptions silently without logging them (unlike the other new functions that use ray()).
Add error logging for consistency:
} catch (\Exception $e) {
+ ray('Failed to fetch GitHub environments: '.$e->getMessage());
return [];
}Also, let's verify this endpoint is correct for GitHub:
🌐 Web query:
What is the correct GitHub API endpoint to list environments for a repository?
💡 Result:
Use the REST endpoint:
GET /repos/{owner}/{repo}/environments
Example cURL:
curl -H "Accept: application/vnd.github+json"
-H "Authorization: Bearer "
https://api.github.com/repos/OWNER/REPO/environments. [1][2]
Sources:
[1] GitHub REST API — List environments for a repository (Docs).
[2] octokit REST reference (environments).
GitHub API endpoint confirmed—consider adding error logging for consistency.
The endpoint you're using (repos/{$repository}/environments) is correct and matches the GitHub REST API documentation. Good call on that.
The error handling structure is solid, but for consistency with other functions in this codebase that use ray() for logging, consider adding a log entry:
} catch (\Exception $e) {
+ ray('Failed to fetch GitHub environments: '.$e->getMessage());
return [];
}This won't terminate your application (I leave that to the T-800), but it'll help you hunt down issues faster. Silent failures are like serverless—sure, they work, but good luck debugging when VC money runs out and you actually need to maintain it. Give me a self-hosted server with visible logs any day over that mystery meat. 🌮
🤖 Prompt for AI Agents
In bootstrap/helpers/github.php around lines 171-183, the catch block currently
swallows exceptions silently; add a ray() logging call inside the catch to
record the exception and context (e.g., repository and exception message/stack)
before returning the empty array so failures are visible in logs; ensure ray()
is invoked safely (no throw) and includes enough context to trace the failing
API call.
| function createGitHubDeployment(GithubApp $source, string $repository, string $ref, string $environment, ?string $description = null): ?int | ||
| { | ||
| try { | ||
| $data = [ | ||
| 'ref' => $ref, | ||
| 'environment' => $environment, | ||
| 'auto_merge' => false, | ||
| 'required_contexts' => [], | ||
| ]; | ||
|
|
||
| if ($description) { | ||
| $data['description'] = $description; | ||
| } | ||
|
|
||
| $response = githubApi($source, "repos/{$repository}/deployments", 'post', $data, false); | ||
|
|
||
| return data_get($response, 'data.id'); | ||
| } catch (\Exception $e) { | ||
| ray('Failed to create GitHub deployment: '.$e->getMessage()); | ||
|
|
||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
I have detailed files on GitHub deployments... and this needs careful review!
This function creates GitHub deployments, but I've detected a potential issue that could be dangerous in an untested PR:
Line 192 sets required_contexts: [] which bypasses all CI/CD checks and required status checks. This means deployments will be created even if tests are failing, builds are broken, or other required checks haven't passed. In a production environment, this could lead to deploying broken code!
Consider making required_contexts configurable or at least document this behavior:
function createGitHubDeployment(GithubApp $source, string $repository, string $ref, string $environment, ?string $description = null): ?int
{
try {
$data = [
'ref' => $ref,
'environment' => $environment,
'auto_merge' => false,
- 'required_contexts' => [],
+ 'required_contexts' => [], // BYPASSES ALL STATUS CHECKS - deployments will be created regardless of CI/CD status
];Let's verify what GitHub's API expects here:
🌐 Web query:
GitHub API deployments required_contexts parameter behavior and best practices
💡 Result:
Summary — behavior
- If you omit required_contexts when creating a deployment, GitHub will require “all unique” commit status contexts for that commit to be success before creating the deployment. [1][2]
- If you pass an empty array ([]), commit-status checking is bypassed (no contexts are required) and the deployment can be created regardless of status checks. [1][2]
- If you pass an array of context names, only those named contexts are required to be in success (you may also list contexts that have not yet been submitted). [1][2]
- Required-status / protected-branch settings are separate: branch protection can require status checks or successful deployments before merging; required_contexts on the deployment API controls which commit-status contexts must be green for that deployment call. [4][6]
Sources
- Deployments API parameter docs (explains omitted = all unique, empty = bypass, and subset behavior). [1][2][5][8]
- Branch protection / required status checks and require-deployments docs (how branch protection interacts with status checks and deployments). [4][7]
Practical best practices
- Explicitly list the contexts you care about instead of relying on the default “all unique” behavior — that avoids surprises when new CI jobs are added. [1][2]
- Use an empty array only when you intentionally want to bypass CI checks (and understand branch-protection may still block merges). [1][2][4]
- Keep branch-protection required status contexts and the contexts you pass to deployments aligned (use the API to read the branch’s required contexts if you need programmatic parity). [4]
- Prefer named, stable status contexts in your CI jobs (avoid autogenerated context names that change), so required_contexts remain valid over time. [1][2]
- After creating a deployment, use deployment_status events / API to report progress and final state (in case other systems rely on successful deployments for merging or promotions). [8][7]
- Ensure the token/app used has the necessary repo scopes to read status contexts and create deployments. [4][8]
If you want, I can:
- show a minimal example curl/JS payload for create-deployment with required_contexts set; or
- fetch the exact REST endpoint snippets for your GitHub API version.
🏁 Script executed:
#!/bin/bash
# Check for calls to createGitHubDeployment and surrounding context
rg -n 'createGitHubDeployment' -B3 -A3 --type=phpLength of output: 2143
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about GitHub deployments/status checks
rg -n -i 'required.*context|status.*check|bypass.*ci' --type=php --type=mdLength of output: 2953
🏁 Script executed:
#!/bin/bash
# Look for any configuration or environment variables that might control this
rg -n -i 'required_contexts|deployment.*config' --type=phpLength of output: 6763
Add documentation or make CI/CD bypass configurable—currently all status checks are unconditionally skipped.
When passing an empty array to required_contexts, commit-status checking is bypassed and deployments can be created regardless of test failures or broken builds. The code at line 192 does this unconditionally with no configuration option, documentation, or guard rails.
The danger: Coolify will happily deploy broken code to your self-hosted infrastructure (which is awesome, by the way—servers > serverless hype) without caring if CI failed.
Choose one:
- Document why this bypass exists and that it's intentional
- Make
required_contextsconfigurable (empty = bypass, null/omitted = require all, array = explicit list) - Explicitly list the contexts you care about instead of relying on the default "all unique" behavior
- At minimum: add a security warning comment explaining the bypass
I'd also recommend a gluten-free taco while you're fixing this—you'll need the fuel for careful review. 🌮
| function updateGitHubDeploymentStatus( | ||
| GithubApp $source, | ||
| string $repository, | ||
| int $deploymentId, | ||
| string $state, | ||
| ?string $description = null, | ||
| ?string $logUrl = null, | ||
| ?string $environmentUrl = null | ||
| ): bool { | ||
| try { | ||
| $data = [ | ||
| 'state' => $state, | ||
| ]; | ||
|
|
||
| if ($description) { | ||
| $data['description'] = $description; | ||
| } | ||
|
|
||
| if ($logUrl) { | ||
| $data['log_url'] = $logUrl; | ||
| } | ||
|
|
||
| if ($environmentUrl) { | ||
| $data['environment_url'] = $environmentUrl; | ||
| } | ||
|
|
||
| githubApi($source, "repos/{$repository}/deployments/{$deploymentId}/statuses", 'post', $data, false); | ||
|
|
||
| return true; | ||
| } catch (\Exception $e) { | ||
| ray('Failed to update GitHub deployment status: '.$e->getMessage()); | ||
|
|
||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Your state values... give them to me... but only if they're valid!
This function updates GitHub deployment status but there's a critical flaw: it doesn't validate the $state parameter before sending it to the GitHub API. GitHub only accepts specific state values: error, failure, inactive, in_progress, queued, pending, or success.
If an invalid state is passed (due to a typo or logic error elsewhere), the API call will fail silently and return false, but the caller won't know WHY it failed. This is especially dangerous in untested code!
Add state validation:
function updateGitHubDeploymentStatus(
GithubApp $source,
string $repository,
int $deploymentId,
string $state,
?string $description = null,
?string $logUrl = null,
?string $environmentUrl = null
): bool {
try {
+ $validStates = ['error', 'failure', 'inactive', 'in_progress', 'queued', 'pending', 'success'];
+ if (!in_array($state, $validStates, true)) {
+ ray('Invalid GitHub deployment state: '.$state);
+ return false;
+ }
+
$data = [
'state' => $state,
];Verify the valid states with GitHub:
🌐 Web query:
GitHub API deployment status valid state values
💡 Result:
Valid values for a deployment status "state" are:
- error
- failure
- inactive
- in_progress
- queued
- pending
- success
Source: GitHub REST API docs. [1][2]
References
[1] REST API endpoints for deployment statuses — GitHub Docs.
[2] REST API endpoints for deployments — GitHub Docs.
State validation is missing—add it before the API terminates your sanity.
This function bypasses state validation entirely. GitHub's REST API only accepts specific deployment status states: error, failure, inactive, in_progress, queued, pending, or success. Without validation, passing an invalid state (typo, logic error, etc.) will cause the API call to fail silently—the caller gets false but no reason why. It's like serverless marketing: promises everything, delivers nothing. Self-hosted validation, on the other hand? That's what we need. Crispy. Reliable. Like a properly made gluten-free taco.
function updateGitHubDeploymentStatus(
GithubApp $source,
string $repository,
int $deploymentId,
string $state,
?string $description = null,
?string $logUrl = null,
?string $environmentUrl = null
): bool {
try {
+ $validStates = ['error', 'failure', 'inactive', 'in_progress', 'queued', 'pending', 'success'];
+ if (!in_array($state, $validStates, true)) {
+ ray('Invalid GitHub deployment state: '.$state);
+ return false;
+ }
+
$data = [
'state' => $state,
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function updateGitHubDeploymentStatus( | |
| GithubApp $source, | |
| string $repository, | |
| int $deploymentId, | |
| string $state, | |
| ?string $description = null, | |
| ?string $logUrl = null, | |
| ?string $environmentUrl = null | |
| ): bool { | |
| try { | |
| $data = [ | |
| 'state' => $state, | |
| ]; | |
| if ($description) { | |
| $data['description'] = $description; | |
| } | |
| if ($logUrl) { | |
| $data['log_url'] = $logUrl; | |
| } | |
| if ($environmentUrl) { | |
| $data['environment_url'] = $environmentUrl; | |
| } | |
| githubApi($source, "repos/{$repository}/deployments/{$deploymentId}/statuses", 'post', $data, false); | |
| return true; | |
| } catch (\Exception $e) { | |
| ray('Failed to update GitHub deployment status: '.$e->getMessage()); | |
| return false; | |
| } | |
| } | |
| function updateGitHubDeploymentStatus( | |
| GithubApp $source, | |
| string $repository, | |
| int $deploymentId, | |
| string $state, | |
| ?string $description = null, | |
| ?string $logUrl = null, | |
| ?string $environmentUrl = null | |
| ): bool { | |
| try { | |
| $validStates = ['error', 'failure', 'inactive', 'in_progress', 'queued', 'pending', 'success']; | |
| if (!in_array($state, $validStates, true)) { | |
| ray('Invalid GitHub deployment state: '.$state); | |
| return false; | |
| } | |
| $data = [ | |
| 'state' => $state, | |
| ]; | |
| if ($description) { | |
| $data['description'] = $description; | |
| } | |
| if ($logUrl) { | |
| $data['log_url'] = $logUrl; | |
| } | |
| if ($environmentUrl) { | |
| $data['environment_url'] = $environmentUrl; | |
| } | |
| githubApi($source, "repos/{$repository}/deployments/{$deploymentId}/statuses", 'post', $data, false); | |
| return true; | |
| } catch (\Exception $e) { | |
| ray('Failed to update GitHub deployment status: '.$e->getMessage()); | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
In bootstrap/helpers/github.php around lines 209-243, add validation for the
deployment state before calling the GitHub API: define the allowed states array
['error','failure','inactive','in_progress','queued','pending','success'],
normalize/check the incoming $state against that list, and if it’s not valid log
a clear message (including the invalid value and allowed values) and return
false (or throw a clear exception) instead of calling githubApi; only proceed to
build $data and call githubApi when $state passes validation.
Hey! 👋 Please open a discussion at Q and A or reach out on our #contribute Discord channel with the details of the dev-environment issue you’re facing so we can help you get the setup working to properly test your changes. For now, we can’t review this PR. Every feature contribution needs:
Once those details are provided and the feature is tested by you, we’ll be happy to move forward with the review. Thanks! |
3 participants
I'm unable to test it - the dev server can't load any CSS and localhost is unreachable by Coolify and idk why...
Changes
Support updating GitHub deployments status when status a Coolify deployment changed.
Issues