-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat: sync deployment status with GitHub deployments #7238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,15 +33,28 @@ class Source extends Component | |||||||||||||||||||||||||||||||||||||||||||||||||||
| #[Validate(['nullable', 'string'])] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public ?string $gitCommitSha = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[Validate(['nullable', 'string'])] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public ?string $githubDeploymentProductionEnvironment = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[Validate(['nullable', 'string'])] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public ?string $githubDeploymentPreviewEnvironment = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[Locked] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public $sources; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[Locked] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public $availableEnvironments = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| public bool $hasDeploymentsPermission = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| public function mount() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->syncData(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->getPrivateKeys(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->getSources(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->checkGitHubDeploymentsPermission(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->loadGitHubEnvironments(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
typed-sigterm marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (\Throwable $e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| handleError($e, $this); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -71,6 +84,8 @@ public function syncData(bool $toModel = false) | |||||||||||||||||||||||||||||||||||||||||||||||||||
| 'git_branch' => $this->gitBranch, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'git_commit_sha' => $this->gitCommitSha, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'private_key_id' => $this->privateKeyId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'deployment_production_environment' => $this->githubDeploymentProductionEnvironment, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'deployment_preview_environment' => $this->githubDeploymentPreviewEnvironment, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Refresh to get the trimmed values from the model | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->application->refresh(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -81,6 +96,8 @@ public function syncData(bool $toModel = false) | |||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->gitCommitSha = $this->application->git_commit_sha; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->privateKeyId = $this->application->private_key_id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->privateKeyName = data_get($this->application, 'private_key.name'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->githubDeploymentProductionEnvironment = $this->application->deployment_production_environment; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->githubDeploymentPreviewEnvironment = $this->application->deployment_preview_environment; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -157,4 +174,24 @@ public function changeSource($sourceId, $sourceType) | |||||||||||||||||||||||||||||||||||||||||||||||||||
| return handleError($e, $this); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| private function checkGitHubDeploymentsPermission() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ($this->application->is_github_based() && $this->application->source) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| $this->hasDeploymentsPermission = hasGitHubDeploymentsPermission($this->application->source); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+185
to
+196
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 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 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,8 @@ class Change extends Component | |
|
|
||
| public ?bool $preview_deployment_permissions = true; | ||
|
|
||
| public ?bool $deployment_statuses_permissions = true; | ||
|
|
||
|
Comment on lines
+31
to
+32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Deployment permission wiring is fine; consider tightening naming and usage
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 |
||
| public ?bool $administration = false; | ||
|
|
||
| public $parameters; | ||
|
|
@@ -67,6 +69,8 @@ class Change extends Component | |
|
|
||
| public ?string $pullRequests = null; | ||
|
|
||
| public ?string $deployments = null; | ||
|
|
||
| public $applications; | ||
|
|
||
| public $privateKeys; | ||
|
|
@@ -87,6 +91,7 @@ class Change extends Component | |
| 'contents' => 'nullable|string', | ||
| 'metadata' => 'nullable|string', | ||
| 'pullRequests' => 'nullable|string', | ||
| 'deployments' => 'nullable|string', | ||
| 'privateKeyId' => 'nullable|int', | ||
| ]; | ||
|
|
||
|
|
@@ -122,6 +127,7 @@ private function syncData(bool $toModel = false): void | |
| $this->github_app->contents = $this->contents; | ||
| $this->github_app->metadata = $this->metadata; | ||
| $this->github_app->pull_requests = $this->pullRequests; | ||
| $this->github_app->deployments = $this->deployments; | ||
| } else { | ||
| // Sync FROM model (on load/refresh) | ||
| $this->name = $this->github_app->name; | ||
|
|
@@ -140,6 +146,7 @@ private function syncData(bool $toModel = false): void | |
| $this->contents = $this->github_app->contents; | ||
| $this->metadata = $this->github_app->metadata; | ||
| $this->pullRequests = $this->github_app->pull_requests; | ||
| $this->deployments = $this->github_app->deployments; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -162,3 +162,82 @@ function loadRepositoryByPage(GithubApp $source, string $token, int $page) | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'repositories' => $json['repositories'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function hasGitHubDeploymentsPermission(GithubApp $source): bool | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ! empty($source->deployments) && $source->deployments === 'write'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+171
to
+183
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial 🧩 Analysis chainListen 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 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: 💡 Result: Use the REST endpoint: GET /repos/{owner}/{repo}/environments Example cURL: Sources: GitHub API endpoint confirmed—consider adding error logging for consistency. The endpoint you're using ( The error handling structure is solid, but for consistency with other functions in this codebase that use } 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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+185
to
+207
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainI 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 Consider making 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: 💡 Result: Summary — behavior
Sources
Practical best practices
If you want, I can:
🏁 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 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:
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; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+209
to
+243
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainYour 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 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: 💡 Result: Valid values for a deployment status "state" are:
Source: GitHub REST API docs. [1][2] References 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 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| <?php | ||
|
|
||
| use Illuminate\Database\Migrations\Migration; | ||
| use Illuminate\Database\Schema\Blueprint; | ||
| use Illuminate\Support\Facades\Schema; | ||
|
|
||
| return new class extends Migration | ||
| { | ||
| /** | ||
| * Run the migrations. | ||
| */ | ||
| public function up(): void | ||
| { | ||
| Schema::table('applications', function (Blueprint $table) { | ||
| $table->string('deployment_production_environment')->nullable(); | ||
| $table->string('deployment_preview_environment')->nullable(); | ||
| }); | ||
| Schema::table('github_apps', function (Blueprint $table) { | ||
| $table->string('deployments')->nullable(); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Reverse the migrations. | ||
| */ | ||
| public function down(): void | ||
| { | ||
| Schema::table('applications', function (Blueprint $table) { | ||
| $table->dropColumn('deployment_production_environment'); | ||
| $table->dropColumn('deployment_preview_environment'); | ||
| }); | ||
| Schema::table('github_apps', function (Blueprint $table) { | ||
| $table->dropColumn('deployments'); | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
GitHub deployment lifecycle wiring is good, but logging and durability need upgrades
The overall flow is solid:
hasGitHubDeploymentsPermission()passes.createGitHubDeployment()chooses preview vs production environment based onpull_request_idand respects empty envs (no deployment).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:Suggestion: keep the “don’t abort deployment” behavior, but at least log to the deployment queue and/or Laravel logs:
} catch (\Throwable $e) {}
At this point the job may still be using the queued
commitvalue (often'HEAD') and you only resolve the actual SHA later incheck_git_if_build_needed(). GitHub deployments happily accept a branch or SHA, but seeingHEADin 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.
Length of output: 37
🏁 Script executed:
Length of output: 6105
🏁 Script executed:
Length of output: 2273
🏁 Script executed:
Length of output: 975
🏁 Script executed:
Length of output: 767
🏁 Script executed:
Length of output: 41
🏁 Script executed:
Length 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:Fix it: log to the deployment queue, not just the void:
} catch (\Throwable $e) {}
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.