From 7f53360cf34e887461112acb163a18db7a095a41 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 17:17:06 +0100 Subject: [PATCH 01/14] fix(legacy): handle runtime operations with no stop command `operation:list` rendered the `stop` column by calling `truncateCommand($op->commands['stop'])`. The runtime-operations schema makes `commands.stop` optional (it's `?string` in `Platformsh\Client\Model\Deployment\RuntimeOperation`), and `truncateCommand()` is typed `string $cmd`, so PHP 8 raised a TypeError on any environment that defined an operation without a stop command. Coalesce the value to an empty string before printing or truncating. Integration test added that mocks an operation payload with `"stop": null` and asserts `operation:list` exits cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) --- integration-tests/runtime_operation_test.go | 139 ++++++++++++++++++ .../Command/RuntimeOperation/ListCommand.php | 3 +- 2 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 integration-tests/runtime_operation_test.go diff --git a/integration-tests/runtime_operation_test.go b/integration-tests/runtime_operation_test.go new file mode 100644 index 000000000..5eee7f6a6 --- /dev/null +++ b/integration-tests/runtime_operation_test.go @@ -0,0 +1,139 @@ +package tests + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestRuntimeOperationRun verifies that `operation:run ` passes the +// expected app name (not null) through to execRuntimeOperation, even when the +// --app/--worker options are not provided. PHPStan flagged $appName as +// potentially null at the call site; this test reproduces the exact path. +func TestRuntimeOperationRun(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + envPath := "/projects/" + projectID + "/environments/main" + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + apiHandler.SetEnvironments([]*mockapi.Environment{ + makeEnv(projectID, "main", "production", "active", nil), + }) + + deploymentPath := envPath + "/deployments/current" + + // Mock the deployment with one webapp that has a "migrate" runtime operation. + apiHandler.Get(deploymentPath, func(w http.ResponseWriter, r *http.Request) { + // Build absolute URL so the client treats the #operations link correctly. + scheme := "http" + if r.TLS != nil { + scheme = "https" + } + base := scheme + "://" + r.Host + _ = json.NewEncoder(w).Encode(map[string]any{ + "webapps": map[string]any{ + "app": map[string]any{ + "name": "app", + "type": "golang:1.23", + "operations": map[string]any{ + "migrate": map[string]any{ + "role": "app", + "commands": map[string]any{ + "start": "php migrate.php", + "stop": nil, + }, + }, + }, + }, + }, + "services": map[string]any{}, + "workers": map[string]any{}, + "routes": map[string]any{}, + "_links": map[string]any{ + "self": map[string]any{"href": base + deploymentPath}, + "#operations": map[string]any{"href": base + envPath + "/runtime-operations"}, + }, + }) + }) + + var receivedBody atomic.Value // map[string]any + apiHandler.Post(envPath+"/runtime-operations", func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + var body map[string]any + _ = json.Unmarshal(b, &body) + receivedBody.Store(body) + _ = json.NewEncoder(w).Encode(map[string]any{ + "_embedded": map[string]any{"activities": []any{}}, + }) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + t.Run("list", func(t *testing.T) { + stdout, stderr, err := f.RunCombinedOutput("operation:list", "-p", projectID, "-e", "main") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, stderr, "TypeError") + assert.NotContains(t, stderr, "must be of type string") + assert.Contains(t, stdout, "migrate") + assert.Contains(t, stdout, "app") + }) + + t.Run("run", func(t *testing.T) { + stdout, stderr, err := f.RunCombinedOutput( + "operation:run", "migrate", + "-p", projectID, "-e", "main", + "--no-wait", "--yes", + ) + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, stderr, "TypeError") + assert.NotContains(t, stderr, "must be of type string") + assert.NotContains(t, stderr, "must be of type ?string") + assert.Contains(t, stderr, "Running operation") + assert.Contains(t, stderr, "migrate") + assert.Contains(t, stderr, "app") + + body, ok := receivedBody.Load().(map[string]any) + require.True(t, ok, "operations POST was not received") + assert.Equal(t, "migrate", body["operation"]) + assert.Equal(t, "app", body["service"], "service (app name) must be a non-null string") + }) + + // Drive the not-found branch: this is the path where $appName starts null + // and remains null when the operation name doesn't match. The command + // should exit with an error before reaching execRuntimeOperation. + t.Run("not_found", func(t *testing.T) { + stdout, stderr, err := f.RunCombinedOutput( + "operation:run", "does-not-exist", + "-p", projectID, "-e", "main", + "--no-wait", "--yes", + ) + // Expect non-zero exit because operation isn't defined. + assert.Error(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, stderr, "TypeError") + assert.NotContains(t, stderr, "must be of type string") + assert.Contains(t, stderr, "was not found") + }) +} diff --git a/legacy/src/Command/RuntimeOperation/ListCommand.php b/legacy/src/Command/RuntimeOperation/ListCommand.php index 0334eb980..a961a1d98 100644 --- a/legacy/src/Command/RuntimeOperation/ListCommand.php +++ b/legacy/src/Command/RuntimeOperation/ListCommand.php @@ -77,7 +77,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int $row['service'] = $serviceName; $row['name'] = new AdaptiveTableCell($name, ['wrap' => false]); $row['start'] = $input->getOption('full') ? $op->commands['start'] : $this->truncateCommand($op->commands['start']); - $row['stop'] = $input->getOption('full') ? $op->commands['stop'] : $this->truncateCommand($op->commands['stop']); + $stop = $op->commands['stop'] ?? ''; + $row['stop'] = $input->getOption('full') ? $stop : $this->truncateCommand($stop); $row['role'] = $op->role; $rows[] = $row; } From bde012ce20507be76d1e70b476ec33c597fee210 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 17:17:16 +0100 Subject: [PATCH 02/14] fix(legacy): allow null previous value in autoscaling duration change `AutoscalingSettingsSetCommand::summarizeChangesPerService()` builds the previous-value side of each duration line from `isset(...) ? formatDuration(...) : null`, then passes it to `formatDurationChange()`. The method was typed `int|string` for its first argument, so when an existing autoscaling setting was configured in only one direction (e.g. an `up` trigger but no `down`), `summarizeChangesPerService` raised a TypeError on the missing side. `ResourcesUtil::formatChange()`, which `formatDurationChange()` delegates to, already accepts `int|string|null` and short-circuits on null previous values. Widen the first parameter of `formatDurationChange()` to match. Integration test added: drives `autoscaling:set --metric cpu --duration-down 2m` against a mock whose current `services.app.triggers.cpu` contains only `up`. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../autoscaling_settings_set_test.go | 144 ++++++++++++++++++ .../AutoscalingSettingsSetCommand.php | 4 +- 2 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 integration-tests/autoscaling_settings_set_test.go diff --git a/integration-tests/autoscaling_settings_set_test.go b/integration-tests/autoscaling_settings_set_test.go new file mode 100644 index 000000000..9cb5208ac --- /dev/null +++ b/integration-tests/autoscaling_settings_set_test.go @@ -0,0 +1,144 @@ +package tests + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestAutoscalingSettingsSetMissingDuration verifies behavior of +// AutoscalingSettingsSetCommand when current autoscaling settings are missing +// the "down" trigger sub-array. PHPStan flags this (level 8) because +// summarizeChangesPerService passes the result of a ternary that can yield +// null to formatDurationChange(int|string, int|string). +// +// To hit the path: call autoscaling:set non-interactively with --duration-down +// against a mock whose current settings have triggers.cpu but no +// triggers.cpu.down. summarizeChanges then runs formatDurationChange(null, ...). +func TestAutoscalingSettingsSetMissingDuration(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + main.Links["#autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"} + main.Links["#manage-autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"} + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + apiHandler.Get("/projects/"+projectID+"/capabilities", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "autoscaling": map[string]any{ + "enabled": true, + "supports_horizontal_scaling_services": false, + }, + }) + }) + + deploymentPath := "/projects/" + projectID + "/environments/main/deployments/current" + apiHandler.Get(deploymentPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "webapps": map[string]any{ + "app": map[string]any{ + "name": "app", + "type": "golang:1.23", + "instance_count": 1, + "disk": 512, + "resources": map[string]any{ + "profile_size": "0.1", + }, + }, + }, + "services": map[string]any{}, + "workers": map[string]any{}, + "routes": map[string]any{}, + "_links": mockapi.MakeHALLinks( + "self=" + deploymentPath, + ), + }) + }) + + // Current autoscaling settings — note "triggers.cpu" exists but has no + // "down" sub-array, which is what triggers the formatDurationChange(null) + // call in summarizeChangesPerService. + autoscalingPath := "/projects/" + projectID + "/environments/main/autoscaling" + apiHandler.Get(autoscalingPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "defaults": map[string]any{ + "triggers": map[string]any{ + "cpu": map[string]any{ + "up": map[string]any{"threshold": 80, "duration": 60}, + "down": map[string]any{"threshold": 20, "duration": 60}, + }, + "memory": map[string]any{ + "up": map[string]any{"threshold": 80, "duration": 60}, + "down": map[string]any{"threshold": 20, "duration": 60}, + }, + }, + "scale_cooldown": map[string]any{"up": 300, "down": 300}, + "instances": map[string]any{"min": 1, "max": 10}, + }, + "services": map[string]any{ + "app": map[string]any{ + "enabled": true, + "triggers": map[string]any{ + "cpu": map[string]any{ + "enabled": true, + "up": map[string]any{"threshold": 80, "duration": 60}, + // No "down" key — this is what PHPStan flags. + }, + }, + "instances": map[string]any{"min": 1, "max": 3}, + // Also omit scale_cooldown to hit the cooldown variant. + }, + }, + "_links": mockapi.MakeHALLinks( + "self=" + autoscalingPath, + ), + }) + }) + + apiHandler.Patch(autoscalingPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{}) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + // Use --dry-run to skip the confirmation; we just need summarizeChanges to run. + stdout, stderr, err := f.RunCombinedOutput( + "autoscaling:set", + "-p", projectID, + "-e", "main", + "--service", "app", + "--metric", "cpu", + "--duration-down", "2m", + "--cooldown-down", "5m", + "--dry-run", + ) + + combined := stdout + "\n---\n" + stderr + assert.NotContains(t, combined, "TypeError", "stdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, combined, "must be of type int|string, null given") + assert.NotContains(t, combined, "Fatal error") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) +} diff --git a/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php b/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php index 41c1d88f1..3ffd80adc 100644 --- a/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php +++ b/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php @@ -947,12 +947,12 @@ protected function formatDuration(int $value): string /** * Formats a change in a duration. * - * @param int|string $previousValue + * @param int|string|null $previousValue * @param int|string $newValue * * @return string */ - protected function formatDurationChange(int|string $previousValue, int|string $newValue): string + protected function formatDurationChange(int|string|null $previousValue, int|string $newValue): string { return $this->resourcesUtil->formatChange( $previousValue, From 8a15e4008da3096d2a350012770c118b04ad9c7d Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 17:17:26 +0100 Subject: [PATCH 03/14] fix(legacy): guard ResourcesSet against missing profile sizes `resources:set` displays a per-app summary of CPU/memory changes by formatting `$sizeInfo = ResourcesUtil::sizeInfo(...)`. `sizeInfo()` returns null when the deployment's `container_profile` + `profile_size` combination isn't present in the deployment's `container_profiles` map (which can happen when a deprecated profile size remains set on an older deployment). The previous-CPU branch then passed null to `ResourcesUtil::formatCPU(int|float|string)`, raising a TypeError. Guard each `formatCPU` call behind a `$sizeInfo !== null` check, then hand the (possibly null) formatted previous value to `formatChange()`, which already accepts null and prints just the new value when so. Apply the same shape to the memory branch on the symmetric `$newSizeInfo` path. Integration test added that mocks a deployment whose `app` has `profile_size: 0.5` but a `container_profiles` map containing only `0.1`, then asserts `resources:set --size app:0.1 --dry-run` exits cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) --- integration-tests/resources_set_test.go | 138 ++++++++++++++++++ .../Command/Resources/ResourcesSetCommand.php | 15 +- 2 files changed, 147 insertions(+), 6 deletions(-) create mode 100644 integration-tests/resources_set_test.go diff --git a/integration-tests/resources_set_test.go b/integration-tests/resources_set_test.go new file mode 100644 index 000000000..122d4fcd0 --- /dev/null +++ b/integration-tests/resources_set_test.go @@ -0,0 +1,138 @@ +package tests + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestResourcesSet_CurrentSizeMissingFromContainerProfiles drives +// resources:set into the path PHPStan level 8 flags in +// ResourcesSetCommand::summarizeChangesPerService: the current +// container-profile size is not present in the deployment's +// container_profiles map, so ResourcesUtil::sizeInfo() returns null, +// and the line 436 call formatCPU(null) violates the declared +// int|float|string parameter type. +// +// Setup: a deployment where the app's current profile_size is "0.5" +// but container_profiles["BALANCED"] only advertises "0.1". A +// --size app:0.1 change is then requested with --dry-run, forcing the +// command to print the previous-vs-new summary before exiting. +func TestResourcesSet_CurrentSizeMissingFromContainerProfiles(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + myUserID := "my-user-id" + apiHandler := mockapi.NewHandler(t) + apiHandler.SetMyUser(&mockapi.User{ID: myUserID}) + + orgID := "org-id-1" + apiHandler.SetOrgs([]*mockapi.Org{{ + ID: orgID, + Type: "flexible", + Name: "acme", + Label: "Acme", + Owner: myUserID, + Capabilities: []string{}, + Links: mockapi.MakeHALLinks( + "self=/organizations/"+url.PathEscape(orgID), + "profile=/organizations/"+url.PathEscape(orgID)+"/profile", + ), + }}) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Organization: orgID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + apiHandler.SetEnvironments([]*mockapi.Environment{ + makeEnv(projectID, "main", "production", "active", nil), + }) + + apiHandler.Get("/projects/"+projectID+"/settings", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "sizing_api_enabled": true, + }) + }) + + // Organization profile without a resources_limit: skips the + // trial-limit branch that would otherwise reach into + // $current['sizes'] (a separate nullable path not under test here). + apiHandler.Get("/organizations/"+orgID+"/profile", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{}) + }) + + nextPath := "/projects/" + projectID + "/environments/main/deployments/next" + apiHandler.Get(nextPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "webapps": map[string]any{ + "app": map[string]any{ + "name": "app", + "type": "golang:1.23", + "container_profile": "BALANCED", + "resources": map[string]any{ + // Current size "0.5" is intentionally NOT + // present in container_profiles["BALANCED"] + // below, so sizeInfo() returns null. + "profile_size": "0.5", + }, + "instance_count": 1, + "disk": 512, + }, + }, + "services": map[string]any{}, + "workers": map[string]any{}, + "routes": map[string]any{}, + "project_info": map[string]any{ + "settings": map[string]any{}, + "capabilities": map[string]any{}, + }, + "container_profiles": map[string]any{ + "BALANCED": map[string]any{ + "0.1": map[string]any{ + "cpu": "0.1", + "memory": "256", + "cpu_type": "guaranteed", + }, + }, + }, + }) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + stdout, stderr, err := f.RunCombinedOutput( + "resources:set", + "-p", projectID, + "-e", "main", + "--size", "app:0.1", + "--dry-run", + "--no-wait", + ) + + // The command should not crash with a PHP TypeError. + assert.NotContains(t, stderr, "TypeError") + assert.NotContains(t, stderr, "must be of type") + assert.NotContains(t, stderr, "Fatal error") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + + // The summary should at least mention the new value. + assert.Contains(t, stderr+stdout, "CPU") +} diff --git a/legacy/src/Command/Resources/ResourcesSetCommand.php b/legacy/src/Command/Resources/ResourcesSetCommand.php index 6d76c6597..eedea0bf2 100644 --- a/legacy/src/Command/Resources/ResourcesSetCommand.php +++ b/legacy/src/Command/Resources/ResourcesSetCommand.php @@ -432,13 +432,16 @@ private function summarizeChangesPerService(string $name, WebApp|Worker|Service $newProperties = array_replace_recursive($properties, $updates); $newSizeInfo = $this->resourcesUtil->sizeInfo($newProperties, $containerProfiles); - $this->stdErr->writeln(' CPU: ' . $this->resourcesUtil->formatChange( - $this->resourcesUtil->formatCPU($sizeInfo ? $sizeInfo['cpu'] : null) . ' ' . $this->formatCPUType($sizeInfo), - $this->resourcesUtil->formatCPU($newSizeInfo['cpu']) . ' ' . $this->formatCPUType($newSizeInfo) - )); + $previousCPU = $sizeInfo !== null + ? $this->resourcesUtil->formatCPU($sizeInfo['cpu']) . ' ' . $this->formatCPUType($sizeInfo) + : null; + $newCPU = $newSizeInfo !== null + ? $this->resourcesUtil->formatCPU($newSizeInfo['cpu']) . ' ' . $this->formatCPUType($newSizeInfo) + : ''; + $this->stdErr->writeln(' CPU: ' . $this->resourcesUtil->formatChange($previousCPU, $newCPU)); $this->stdErr->writeln(' Memory: ' . $this->resourcesUtil->formatChange( - $sizeInfo ? $sizeInfo['memory'] : null, - $newSizeInfo['memory'], + $sizeInfo !== null ? $sizeInfo['memory'] : null, + $newSizeInfo !== null ? $newSizeInfo['memory'] : null, ' MB', )); } From 915a95afb775b2da27ee1314bbe64545b55367f2 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 17:17:36 +0100 Subject: [PATCH 04/14] fix(legacy): tolerate null timestamps on activities Two paths crashed on activities with a literal null timestamp: - `Model\Activity::getDuration()` called `strtotime($activity->X)` with `$activity->completed_at`, `updated_at` or `created_at` directly, hitting a TypeError in PHP 8 when any of those was null. - `ActivityLoader::load()` constructed `new DateTime($activity->created_at)` to paginate, same TypeError shape. Both happen on `activity:get`, `activity:list`, and any command that streams activities (e.g. backup-await flows) for an activity returned by the API with missing timestamp fields. Gate each call with `!empty(...)`. In `getDuration()` the existing `$end !== false && $start !== false` guard already handles the missing case; in `ActivityLoader::load()` the previous `$startsAt` value is kept when the most recently loaded activity lacks `created_at` (the loop's `$new` check already detects when pagination has stalled). Integration test added that registers a raw API handler returning an activity with `"completed_at": null, "started_at": null, "created_at": null, "updated_at": null` and exercises `activity:list`, `activity:get`, and `activity:get -P duration`. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../activity_null_timestamp_test.go | 126 ++++++++++++++++++ legacy/src/Model/Activity.php | 8 +- legacy/src/Service/ActivityLoader.php | 4 +- 3 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 integration-tests/activity_null_timestamp_test.go diff --git a/integration-tests/activity_null_timestamp_test.go b/integration-tests/activity_null_timestamp_test.go new file mode 100644 index 000000000..7592534ba --- /dev/null +++ b/integration-tests/activity_null_timestamp_test.go @@ -0,0 +1,126 @@ +package tests + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestActivityNullTimestamp checks whether the legacy PHP code crashes when an +// API activity is returned with null completed_at / updated_at / created_at / +// started_at fields. PHPStan level 8 flags strtotime() calls on those nullable +// properties in legacy/src/Model/Activity.php, and new DateTime() on +// $activity->created_at in legacy/src/Service/ActivityLoader.php. +// +// We bypass mockapi's typed Activity model (which would always emit valid +// timestamps) by wrapping the handler and serving raw JSON for the activities +// endpoints. +func TestActivityNullTimestamp(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + main.Links["#activities"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/activities"} + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + // Raw JSON for a single "complete" activity (completion_percent=100, so + // isComplete() returns true) with completed_at, updated_at, created_at and + // started_at all literal null. This is the worst case for the PHPStan-flagged + // strtotime() and new DateTime() calls. + activityJSON := `{ + "id": "actnull", + "type": "environment.variable.create", + "state": "complete", + "result": "success", + "completion_percent": 100, + "completed_at": null, + "started_at": null, + "created_at": null, + "updated_at": null, + "project": "` + projectID + `", + "environments": ["main"], + "description": "Activity with null timestamps", + "text": "Activity with null timestamps", + "payload": {} + }` + listJSON := "[" + activityJSON + "]" + + listPath := "/projects/" + projectID + "/environments/main/activities" + getPath := "/projects/" + projectID + "/activities/actnull" + getEnvPath := "/projects/" + projectID + "/environments/main/activities/actnull" + + wrapped := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case listPath: + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(listJSON)) + return + case getPath, getEnvPath: + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(activityJSON)) + return + } + apiHandler.ServeHTTP(w, r) + }) + + apiServer := httptest.NewServer(wrapped) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + // Drive activity:list. This exercises ActivityLoader::load() which calls + // new DateTime($activity->created_at) on line 130 when paginating. + stdout, stderr, err := f.RunCombinedOutput("activity:list", "-p", projectID, "-e", "main", "--format", "plain") + t.Logf("activity:list err=%v\nstdout=%s\nstderr=%s", err, stdout, stderr) + + // Drive activity:get. This exercises Model\Activity::getDuration() which + // calls strtotime() on completed_at / updated_at / created_at (lines 17, 24, + // 26). + stdout2, stderr2, err2 := f.RunCombinedOutput("activity:get", "-p", projectID, "-e", "main", "actnull") + t.Logf("activity:get err=%v\nstdout=%s\nstderr=%s", err2, stdout2, stderr2) + + // Also explicitly request the duration property so getDuration() is invoked + // even if the table path skips it. + stdout3, stderr3, err3 := f.RunCombinedOutput( + "activity:get", "-p", projectID, "-e", "main", "actnull", "-P", "duration", + ) + t.Logf("activity:get -P duration err=%v\nstdout=%s\nstderr=%s", err3, stdout3, stderr3) + + // Surface TypeError / fatal errors loudly. + for _, out := range []string{stderr, stderr2, stderr3} { + if strings.Contains(out, "TypeError") || + strings.Contains(out, "must be of type string") || + strings.Contains(out, "Fatal error") || + strings.Contains(out, "Uncaught") { + t.Fatalf("legacy CLI raised a PHP error on null timestamp:\n%s", out) + } + } + + // If we reach here without any error, the strtotime / new DateTime calls + // either tolerated null input or the null was sanitized before reaching them. + if err != nil { + t.Fatalf("activity:list failed unexpectedly: %v\nstderr=%s", err, stderr) + } + if err2 != nil { + t.Fatalf("activity:get failed unexpectedly: %v\nstderr=%s", err2, stderr2) + } + if err3 != nil { + t.Fatalf("activity:get -P duration failed unexpectedly: %v\nstderr=%s", err3, stderr3) + } +} diff --git a/legacy/src/Model/Activity.php b/legacy/src/Model/Activity.php index 3e1100a91..5057b4fde 100644 --- a/legacy/src/Model/Activity.php +++ b/legacy/src/Model/Activity.php @@ -14,16 +14,18 @@ class Activity public function getDuration(ApiActivity $activity, ?int $now = null): float|int|null { if ($activity->isComplete()) { - $end = strtotime($activity->completed_at); + $end = !empty($activity->completed_at) ? strtotime($activity->completed_at) : false; } elseif ($activity->state === ApiActivity::STATE_CANCELLED && $activity->hasProperty('cancelled_at')) { $end = strtotime((string) $activity->getProperty('cancelled_at')); } elseif (!empty($activity->started_at)) { $now = $now === null ? time() : $now; $end = $now; } else { - $end = strtotime($activity->updated_at); + $end = !empty($activity->updated_at) ? strtotime($activity->updated_at) : false; } - $start = !empty($activity->started_at) ? strtotime($activity->started_at) : strtotime($activity->created_at); + $start = !empty($activity->started_at) + ? strtotime($activity->started_at) + : (!empty($activity->created_at) ? strtotime($activity->created_at) : false); return $end !== false && $start !== false && $end - $start > 0 ? $end - $start : null; } diff --git a/legacy/src/Service/ActivityLoader.php b/legacy/src/Service/ActivityLoader.php index 07b925ed4..380cd6c3d 100644 --- a/legacy/src/Service/ActivityLoader.php +++ b/legacy/src/Service/ActivityLoader.php @@ -127,7 +127,9 @@ public function load(HasActivitiesInterface $apiResource, ?int $limit = null, ar $activities = []; while ($limit === null || count($activities) < $limit) { if ($activity = end($activities)) { - $startsAt = new DateTime($activity->created_at); + if (!empty($activity->created_at)) { + $startsAt = new DateTime($activity->created_at); + } } $nextActivities = $apiResource->getActivities($limit ? $limit - count($activities) : 0, $types, $startsAt, $state, $result); $new = false; From e2001d3aaba6e9f69c572af2f32e719d28b0edd4 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 17:17:45 +0100 Subject: [PATCH 05/14] test(integration): cover domain:update, domain:get, org:user:projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These commands were flagged by raising PHPStan to level 8 (nullable selection / member / environment reaching method calls or DB-typed parameters). On inspection the flagged paths are unreachable at runtime — `loadMemberByEmail()` filters nulls before they reach the caller, the `#domains` env branch only runs when the env is set, and the domain argument is `InputArgument::REQUIRED`. Adding tests that exercise the happy paths documents that and locks them down in case future changes break the upstream guarantees the static type checker can't see. No production code changed. Co-Authored-By: Claude Opus 4.7 (1M context) --- integration-tests/domain_update_test.go | 134 ++++++++++++++++++++ integration-tests/org_user_projects_test.go | 120 ++++++++++++++++++ 2 files changed, 254 insertions(+) create mode 100644 integration-tests/domain_update_test.go create mode 100644 integration-tests/org_user_projects_test.go diff --git a/integration-tests/domain_update_test.go b/integration-tests/domain_update_test.go new file mode 100644 index 000000000..3a6545908 --- /dev/null +++ b/integration-tests/domain_update_test.go @@ -0,0 +1,134 @@ +package tests + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestDomainUpdateProjectScope drives the legacy domain:update command with a +// project-only selection (no -e) to verify the project-scoped branch where +// $environment stays null and $project->getDomain($name) is called with the +// argument name. PHPStan flags getDomain($this->domainName) at level 8 because +// $this->domainName is typed ?string; this test asserts that the path runs +// without a TypeError in practice. +func TestDomainUpdateProjectScope(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + "domains=/projects/"+projectID+"/domains", + "#manage-domains=/projects/"+projectID+"/domains", + ), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + apiHandler.Get("/projects/"+projectID+"/capabilities", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "custom_domains": map[string]any{"enabled": true}, + }) + }) + + // Existing project-level domain. Includes _links with self so it can be + // fully formed. + apiHandler.Get("/projects/"+projectID+"/domains/example.com", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "id": "example.com", + "name": "example.com", + "created_at": "2024-01-01T00:00:00Z", + "updated_at": "2024-01-01T00:00:00Z", + "ssl": map[string]any{ + "key": "", + "certificate": "", + "chain": []any{}, + }, + "_links": map[string]any{ + "self": map[string]string{"href": "/projects/" + projectID + "/domains/example.com"}, + }, + }) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + stdout, stderr, err := f.RunCombinedOutput("domain:update", "-p", projectID, "--no-wait", "example.com") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + // With no --cert/--key flags, the command should fall through to the + // "nothing to update" branch after fetching the project-level domain. + assert.Contains(t, stderr, "There is nothing to update.", + "unexpected output - stdout: %s\nstderr: %s", stdout, stderr) +} + +// TestDomainGetProjectScope drives domain:get with project-only selection, +// covering the project-level $project->getDomain($name) branch on line 57 of +// DomainGetCommand. The PHPStan finding at line 56 ($environment->getLink) is +// not reached because $forEnvironment is false when -e is not given. +func TestDomainGetProjectScope(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + "domains=/projects/"+projectID+"/domains", + ), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + apiHandler.Get("/projects/"+projectID+"/domains/example.com", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "id": "example.com", + "name": "example.com", + "created_at": "2024-01-01T00:00:00Z", + "updated_at": "2024-01-01T00:00:00Z", + "ssl": map[string]any{ + "key": "", + "certificate": "", + "chain": []any{}, + }, + "_links": map[string]any{ + "self": map[string]string{"href": "/projects/" + projectID + "/domains/example.com"}, + }, + }) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + stdout, stderr, err := f.RunCombinedOutput("domain:get", "-p", projectID, "example.com") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + // The command renders a simple table with the domain name. Check that it + // printed the domain name. + assert.Contains(t, stdout, "example.com", + "expected domain name in output - stdout: %s\nstderr: %s", stdout, stderr) +} diff --git a/integration-tests/org_user_projects_test.go b/integration-tests/org_user_projects_test.go new file mode 100644 index 000000000..ead6477ef --- /dev/null +++ b/integration-tests/org_user_projects_test.go @@ -0,0 +1,120 @@ +package tests + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestOrgUserProjects verifies the path that PHPStan flags as potentially passing +// a nullable UserRef into Api::getUserRefLabel() and UserRef::email accesses in +// OrganizationUserProjectsCommand (lines 135, 137, 165, 171, 184). +// +// The org+email path uses Api::loadMemberByEmail(), which already filters out +// members whose getUserInfo() is null. The lookup we deliberately make miss for +// one member (by omitting its entry from the ref:users map in the members +// listing) exercises that filter. The remaining matched member always has a +// non-null UserRef at runtime, so the lookup in the command body cannot return +// null. The test confirms the command completes without a TypeError. +func TestOrgUserProjects(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + myUserID := "user-for-oups-test" + targetUserID := "target-user-id" + missingUserID := "missing-ref-user-id" + orgID := "org-id-oups" + orgName := "oups-org" + + apiHandler := mockapi.NewHandler(t) + apiHandler.SetMyUser(&mockapi.User{ID: myUserID}) + + org := makeOrg(orgID, orgName, "OUPS Org", myUserID, "flexible") + org.Links["members"] = mockapi.HALLink{HREF: "/organizations/" + url.PathEscape(orgID) + "/members"} + apiHandler.SetOrgs([]*mockapi.Org{org}) + + projectID := mockapi.ProjectID() + apiHandler.SetProjects([]*mockapi.Project{ + makeProject(projectID, orgID, "test-vendor", "Project 1", "region-1"), + }) + + apiHandler.SetUserGrants([]*mockapi.UserGrant{ + { + ResourceID: projectID, + ResourceType: "project", + OrganizationID: orgID, + UserID: targetUserID, + Permissions: []string{"viewer"}, + }, + }) + + // Mock the organization members listing. The "ref:users" map intentionally + // omits missingUserID, so getUserInfo() returns null for that member and + // loadMemberByEmail filters it out. + apiHandler.Get("/organizations/"+orgID+"/members", func(w http.ResponseWriter, _ *http.Request) { + body := map[string]any{ + "items": []map[string]any{ + { + "id": "member-1", + "organization_id": orgID, + "user_id": targetUserID, + "permissions": []string{"members:viewer"}, + "owner": false, + "created_at": "2024-01-01T00:00:00+00:00", + "updated_at": "2024-01-01T00:00:00+00:00", + }, + { + "id": "member-2", + "organization_id": orgID, + "user_id": missingUserID, + "permissions": []string{"members:viewer"}, + "owner": false, + "created_at": "2024-01-01T00:00:00+00:00", + "updated_at": "2024-01-01T00:00:00+00:00", + }, + }, + "_links": map[string]any{ + "self": map[string]string{"href": "/organizations/" + orgID + "/members"}, + "ref:users:0": map[string]string{"href": "/ref/users?in=" + targetUserID}, + // Deliberately omit missingUserID from the ref:users lookup. + }, + } + _ = json.NewEncoder(w).Encode(body) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + // Run with org+email path; should complete successfully and find the project. + stdout, stderr, err := f.RunCombinedOutput("oups", "-o", orgName, targetUserID+"@example.com") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stderr, "Project access for the user") + assert.Contains(t, stdout, projectID) + + // Run with org+email for the user whose ref:users entry is missing. + // loadMemberByEmail filters that member out, so the CLI reports "User not + // found" and returns 1 — without ever assigning a nullable UserRef into + // the code at lines 135/137/165/171/184. + _, stderr, err = f.RunCombinedOutput("oups", "-o", orgName, missingUserID+"@example.com") + require.Error(t, err) + assert.Contains(t, stderr, "User not found for email address") + + // Also exercise the "no projects" branch (line 135), which is the literal + // PHPStan finding's first occurrence. Use a user that exists but has no + // project grants. + apiHandler.SetUserGrants(nil) + stdout, stderr, err = f.RunCombinedOutput("oups", "-o", orgName, targetUserID+"@example.com") + require.NoError(t, err, "stdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stderr, "No projects were found for the user") + assert.True(t, strings.Contains(stderr, targetUserID+"@example.com")) +} From 9729db8d696f82cce54c0e3b0ffd96bf8b8b4a28 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 17:19:08 +0100 Subject: [PATCH 06/14] chore(legacy): raise PHPStan to level 8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps `legacy/phpstan.neon` from `level: 7` to `level: 8` and regenerates `phpstan-baseline.neon` to suppress the 124 new findings that are not bugs. Background. CLI-145 ran a full pass at level 8 (122 new findings across 57 files at the time) and classified each as bug, annotation, or unclear. Four genuine runtime crashes were fixed in the preceding four commits, with integration coverage. The remaining 124 findings are paths the static type checker can't narrow but that are guarded at runtime — Symfony `Process::getExitCode()` called only after `run()`/`wait()`, `Command::getApplication()` called only inside an `execute()`, `BuildFlavorBase` properties used only after `prepare()`, `preg_replace()` on validated patterns, lazily-init form fields touched only after `configure()`, and so on. Where any of these could plausibly leak in some future refactor the integration tests added in this PR will catch the regression. The Rector visitors (`src/Rector/*`) are also baselined; they are build-time scaffolding and not shipped. A subset of new entries is verified by integration tests added in this PR (domain:update, domain:get, org:user:projects). The rest were reviewed but not driven through end-to-end tests; this baseline gives us a green level-8 gate to ratchet down against. Co-Authored-By: Claude Opus 4.7 (1M context) --- legacy/phpstan-baseline.neon | 450 +++++++++++++++++++++++++++++++++++ legacy/phpstan.neon | 2 +- 2 files changed, 451 insertions(+), 1 deletion(-) diff --git a/legacy/phpstan-baseline.neon b/legacy/phpstan-baseline.neon index a18d4322d..9a6fa14a0 100644 --- a/legacy/phpstan-baseline.neon +++ b/legacy/phpstan-baseline.neon @@ -1,5 +1,15 @@ parameters: ignoreErrors: + - + message: "#^Parameter \\#1 \\$commandName of method Symfony\\\\Component\\\\Console\\\\Application\\:\\:setDefaultCommand\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Application.php + + - + message: "#^Parameter \\#1 \\$name of method Platformsh\\\\Cli\\\\Service\\\\Config\\:\\:isCommandEnabled\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Application.php + - message: "#^Call to an undefined method Platformsh\\\\Client\\\\Connection\\\\ConnectorInterface\\:\\:getOAuth2Provider\\(\\)\\.$#" count: 1 @@ -15,6 +25,16 @@ parameters: count: 3 path: src/Command/Auth/VerifyPhoneNumberCommand.php + - + message: "#^Offset 'triggers' does not exist on array\\\\|null\\.$#" + count: 2 + path: src/Command/Autoscaling/AutoscalingSettingsSetCommand.php + + - + message: "#^Parameter \\#1 \\$value of method Platformsh\\\\Cli\\\\Command\\\\Autoscaling\\\\AutoscalingSettingsSetCommand\\:\\:validateMetric\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Command/Autoscaling/AutoscalingSettingsSetCommand.php + - message: "#^Access to an undefined property Platformsh\\\\Client\\\\Model\\\\Backup\\:\\:\\$safe\\.$#" count: 2 @@ -55,31 +75,166 @@ parameters: count: 1 path: src/Command/BotCommand.php + - + message: "#^Cannot call method getStr\\(\\) on Platformsh\\\\Cli\\\\Service\\\\Config\\|null\\.$#" + count: 2 + path: src/Command/CommandBase.php + + - + message: "#^Cannot call method isCommandEnabled\\(\\) on Platformsh\\\\Cli\\\\Service\\\\Config\\|null\\.$#" + count: 1 + path: src/Command/CommandBase.php + + - + message: "#^Cannot call method isCommandHidden\\(\\) on Platformsh\\\\Cli\\\\Service\\\\Config\\|null\\.$#" + count: 1 + path: src/Command/CommandBase.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Command\\\\CommandBase\\:\\:getPreferredName\\(\\) should return string but returns string\\|null\\.$#" + count: 1 + path: src/Command/CommandBase.php + + - + message: "#^Parameter \\#3 \\$name of static method Platformsh\\\\Cli\\\\Model\\\\EnvironmentDomain\\:\\:add\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Command/Domain/DomainAddCommand.php + + - + message: "#^Cannot call method getLink\\(\\) on Platformsh\\\\Client\\\\Model\\\\Environment\\|null\\.$#" + count: 1 + path: src/Command/Domain/DomainGetCommand.php + + - + message: "#^Parameter \\#1 \\$environment of static method Platformsh\\\\Cli\\\\Model\\\\EnvironmentDomain\\:\\:getList\\(\\) expects Platformsh\\\\Client\\\\Model\\\\Environment, Platformsh\\\\Client\\\\Model\\\\Environment\\|null given\\.$#" + count: 1 + path: src/Command/Domain/DomainGetCommand.php + + - + message: "#^Cannot call method getLink\\(\\) on Platformsh\\\\Client\\\\Model\\\\Environment\\|null\\.$#" + count: 1 + path: src/Command/Domain/DomainUpdateCommand.php + + - + message: "#^Parameter \\#1 \\$id of static method Platformsh\\\\Client\\\\Model\\\\ApiResourceBase\\:\\:get\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Command/Domain/DomainUpdateCommand.php + + - + message: "#^Parameter \\#1 \\$name of method Platformsh\\\\Client\\\\Model\\\\Project\\:\\:getDomain\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Command/Domain/DomainUpdateCommand.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Command\\\\Environment\\\\EnvironmentPushCommand\\:\\:execute\\(\\) should return int but returns int\\|null\\.$#" + count: 1 + path: src/Command/Environment/EnvironmentPushCommand.php + + - + message: "#^Parameter \\#1 \\$id of method Platformsh\\\\Cli\\\\Service\\\\Api\\:\\:getEnvironment\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Command/Environment/EnvironmentSynchronizeCommand.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Command\\\\Environment\\\\EnvironmentXdebugCommand\\:\\:execute\\(\\) should return int but returns int\\|null\\.$#" + count: 1 + path: src/Command/Environment/EnvironmentXdebugCommand.php + + - + message: "#^Cannot call method find\\(\\) on Symfony\\\\Component\\\\Console\\\\Application\\|null\\.$#" + count: 2 + path: src/Command/HelpCommand.php + - message: "#^Call to an undefined method GuzzleHttp\\\\ClientInterface\\:\\:post\\(\\)\\.$#" count: 1 path: src/Command/Integration/IntegrationCommandBase.php + - + message: "#^Cannot call method getProject\\(\\) on Platformsh\\\\Cli\\\\Selector\\\\Selection\\|null\\.$#" + count: 1 + path: src/Command/Integration/IntegrationCommandBase.php + + - + message: "#^Cannot call method hasProject\\(\\) on Platformsh\\\\Cli\\\\Selector\\\\Selection\\|null\\.$#" + count: 2 + path: src/Command/Integration/IntegrationCommandBase.php + - message: "#^Method Platformsh\\\\Cli\\\\Command\\\\Integration\\\\IntegrationCommandBase\\:\\:selectedProjectIntegrationCapabilities\\(\\) should return array\\{enabled\\: bool, config\\?\\: array\\\\} but returns array\\.$#" count: 1 path: src/Command/Integration/IntegrationCommandBase.php + - + message: "#^Parameter \\#1 \\$app of method Platformsh\\\\Client\\\\Model\\\\Environment\\:\\:getSshUrl\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Command/Local/LocalDrushAliasesCommand.php + - message: "#^Cannot call method set\\(\\) on Platformsh\\\\ConsoleForm\\\\Field\\\\Field\\|false\\.$#" count: 1 path: src/Command/Organization/OrganizationCreateCommand.php + - + message: "#^Cannot access property \\$email on Platformsh\\\\Client\\\\Model\\\\Ref\\\\UserRef\\|null\\.$#" + count: 1 + path: src/Command/Organization/User/OrganizationUserProjectsCommand.php + + - + message: "#^Parameter \\#1 \\$userRef of method Platformsh\\\\Cli\\\\Service\\\\Api\\:\\:getUserRefLabel\\(\\) expects Platformsh\\\\Client\\\\Model\\\\Ref\\\\UserRef, Platformsh\\\\Client\\\\Model\\\\Ref\\\\UserRef\\|null given\\.$#" + count: 4 + path: src/Command/Organization/User/OrganizationUserProjectsCommand.php + - message: "#^Call to an undefined method GuzzleHttp\\\\ClientInterface\\:\\:get\\(\\)\\.$#" count: 1 path: src/Command/Project/ProjectCreateCommand.php + - + message: "#^Cannot call method has\\(\\) on Symfony\\\\Component\\\\Console\\\\Application\\|null\\.$#" + count: 1 + path: src/Command/Project/ProjectGetCommand.php + + - + message: "#^Parameter \\#1 \\$rows of method Platformsh\\\\Cli\\\\Service\\\\Table\\:\\:render\\(\\) expects array\\\\|Symfony\\\\Component\\\\Console\\\\Helper\\\\TableSeparator\\>, array\\\\> given\\.$#" + count: 2 + path: src/Command/Project/ProjectListCommand.php + + - + message: "#^Trying to invoke \\(callable\\(\\)\\: mixed\\)\\|null but it might not be a callable\\.$#" + count: 1 + path: src/Command/Resources/ResourcesSetCommand.php + - message: "#^Access to an undefined property Platformsh\\\\Client\\\\Model\\\\Deployment\\\\Service\\|Platformsh\\\\Client\\\\Model\\\\Deployment\\\\WebApp\\|Platformsh\\\\Client\\\\Model\\\\Deployment\\\\Worker\\:\\:\\$container_profile\\.$#" count: 3 path: src/Command/Resources/ResourcesSizeListCommand.php + - + message: "#^Parameter \\#2 \\$service of method Platformsh\\\\Client\\\\Model\\\\Deployment\\\\EnvironmentDeployment\\:\\:execRuntimeOperation\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Command/RuntimeOperation/RunCommand.php + + - + message: "#^Parameter \\#2 \\$content of method Symfony\\\\Component\\\\Filesystem\\\\Filesystem\\:\\:dumpFile\\(\\) expects resource\\|string, string\\|null given\\.$#" + count: 1 + path: src/Command/Self/SelfInstallCommand.php + + - + message: "#^Parameter \\#1 \\$messages of method Symfony\\\\Component\\\\Console\\\\Output\\\\OutputInterface\\:\\:writeln\\(\\) expects iterable\\|string, string\\|null given\\.$#" + count: 1 + path: src/Command/Self/SelfReleaseCommand.php + + - + message: "#^Parameter \\#2 \\$pid of method Platformsh\\\\Cli\\\\Command\\\\Server\\\\ServerCommandBase\\:\\:writeServerInfo\\(\\) expects int, int\\|null given\\.$#" + count: 1 + path: src/Command/Server/ServerRunCommand.php + + - + message: "#^Parameter \\#2 \\$pid of method Platformsh\\\\Cli\\\\Command\\\\Server\\\\ServerCommandBase\\:\\:writeServerInfo\\(\\) expects int, int\\|null given\\.$#" + count: 1 + path: src/Command/Server/ServerStartCommand.php + - message: "#^Parameter \\#1 \\$items of method Platformsh\\\\Cli\\\\Service\\\\QuestionHelper\\:\\:choose\\(\\) expects array\\, array\\ given\\.$#" count: 1 @@ -95,6 +250,11 @@ parameters: count: 1 path: src/Command/Team/Project/TeamProjectDeleteCommand.php + - + message: "#^Cannot call method has\\(\\) on Symfony\\\\Component\\\\Console\\\\Application\\|null\\.$#" + count: 2 + path: src/Command/Team/TeamCommandBase.php + - message: "#^Call to an undefined method GuzzleHttp\\\\ClientInterface\\:\\:get\\(\\)\\.$#" count: 1 @@ -105,21 +265,81 @@ parameters: count: 1 path: src/Command/Team/User/TeamUserAddCommand.php + - + message: "#^Cannot call method has\\(\\) on Symfony\\\\Component\\\\Console\\\\Application\\|null\\.$#" + count: 1 + path: src/Command/Team/User/TeamUserAddCommand.php + + - + message: "#^Cannot access property \\$type on Platformsh\\\\Client\\\\Model\\\\Environment\\|null\\.$#" + count: 1 + path: src/Command/User/UserGetCommand.php + + - + message: "#^Cannot call method getUser\\(\\) on Platformsh\\\\Client\\\\Model\\\\Environment\\|null\\.$#" + count: 1 + path: src/Command/User/UserGetCommand.php + + - + message: "#^Cannot access property \\$id on Platformsh\\\\Client\\\\Model\\\\Environment\\|null\\.$#" + count: 2 + path: src/Command/Variable/VariableCreateCommand.php + + - + message: "#^Cannot call method getFields\\(\\) on Platformsh\\\\ConsoleForm\\\\Form\\|null\\.$#" + count: 1 + path: src/Command/Variable/VariableCreateCommand.php + + - + message: "#^Cannot call method getLink\\(\\) on Platformsh\\\\Client\\\\Model\\\\Environment\\|null\\.$#" + count: 1 + path: src/Command/Variable/VariableCreateCommand.php + + - + message: "#^Cannot call method getVariable\\(\\) on Platformsh\\\\Client\\\\Model\\\\Environment\\|null\\.$#" + count: 1 + path: src/Command/Variable/VariableCreateCommand.php + + - + message: "#^Cannot call method resolveOptions\\(\\) on Platformsh\\\\ConsoleForm\\\\Form\\|null\\.$#" + count: 1 + path: src/Command/Variable/VariableCreateCommand.php + - message: "#^Dead catch \\- Platformsh\\\\ConsoleForm\\\\Exception\\\\ConditionalFieldException is never thrown in the try block\\.$#" count: 1 path: src/Command/Variable/VariableCreateCommand.php + - + message: "#^Cannot call method getFields\\(\\) on Platformsh\\\\ConsoleForm\\\\Form\\|null\\.$#" + count: 1 + path: src/Command/Variable/VariableUpdateCommand.php + - message: "#^Call to an undefined method GuzzleHttp\\\\ClientInterface\\:\\:get\\(\\)\\.$#" count: 1 path: src/Command/Version/VersionListCommand.php + - + message: "#^Parameter \\#1 \\$messages of method Symfony\\\\Component\\\\Console\\\\Output\\\\OutputInterface\\:\\:writeln\\(\\) expects iterable\\|string, string\\|null given\\.$#" + count: 1 + path: src/Command/WelcomeCommand.php + - message: "#^While loop condition is always true\\.$#" count: 1 path: src/Command/WinkyCommand.php + - + message: "#^Method Platformsh\\\\Cli\\\\Console\\\\AdaptiveTable\\:\\:wrapCell\\(\\) should return string but returns string\\|null\\.$#" + count: 1 + path: src/Console/AdaptiveTable.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Console\\\\AdaptiveTable\\:\\:wrapWithDecoration\\(\\) should return string but returns string\\|null\\.$#" + count: 1 + path: src/Console/AdaptiveTable.php + - message: "#^Method Platformsh\\\\Cli\\\\Console\\\\CustomJsonDescriptor\\:\\:describeApplication\\(\\) has parameter \\$options with no value type specified in iterable type array\\.$#" count: 1 @@ -200,6 +420,11 @@ parameters: count: 1 path: src/Console/CustomMarkdownDescriptor.php + - + message: "#^Parameter \\#3 \\$subject of function str_replace expects array\\|string, string\\|null given\\.$#" + count: 1 + path: src/Console/CustomMarkdownDescriptor.php + - message: "#^Method Platformsh\\\\Cli\\\\Console\\\\CustomTextDescriptor\\:\\:describeApplication\\(\\) has parameter \\$options with no value type specified in iterable type array\\.$#" count: 1 @@ -230,6 +455,16 @@ parameters: count: 1 path: src/Console/CustomTextDescriptor.php + - + message: "#^Method Platformsh\\\\Cli\\\\Console\\\\ProcessManager\\:\\:startProcess\\(\\) should return int but returns int\\|null\\.$#" + count: 1 + path: src/Console/ProcessManager.php + + - + message: "#^Parameter \\#1 \\$exitCode of method Platformsh\\\\Cli\\\\Console\\\\ProcessManager\\:\\:getSignal\\(\\) expects int, int\\|null given\\.$#" + count: 1 + path: src/Console/ProcessManager.php + - message: "#^Method Platformsh\\\\Cli\\\\CredentialHelper\\\\SessionStorage\\:\\:load\\(\\) return type has no value type specified in iterable type array\\.$#" count: 1 @@ -305,6 +540,61 @@ parameters: count: 1 path: src/Exception/RootNotFoundException.php + - + message: "#^Cannot call method getName\\(\\) on Platformsh\\\\Cli\\\\Local\\\\LocalApplication\\|null\\.$#" + count: 1 + path: src/Local/BuildFlavor/BuildFlavorBase.php + + - + message: "#^Cannot call method getSharedFileMounts\\(\\) on Platformsh\\\\Cli\\\\Local\\\\LocalApplication\\|null\\.$#" + count: 1 + path: src/Local/BuildFlavor/BuildFlavorBase.php + + - + message: "#^Cannot call method getSourceDir\\(\\) on Platformsh\\\\Cli\\\\Local\\\\LocalApplication\\|null\\.$#" + count: 1 + path: src/Local/BuildFlavor/BuildFlavorBase.php + + - + message: "#^Cannot call method getStr\\(\\) on Platformsh\\\\Cli\\\\Service\\\\Config\\|null\\.$#" + count: 3 + path: src/Local/BuildFlavor/BuildFlavorBase.php + + - + message: "#^Cannot call method isSingle\\(\\) on Platformsh\\\\Cli\\\\Local\\\\LocalApplication\\|null\\.$#" + count: 1 + path: src/Local/BuildFlavor/BuildFlavorBase.php + + - + message: "#^Cannot call method shouldMoveToRoot\\(\\) on Platformsh\\\\Cli\\\\Local\\\\LocalApplication\\|null\\.$#" + count: 1 + path: src/Local/BuildFlavor/BuildFlavorBase.php + + - + message: "#^Parameter \\#1 \\$source of method Platformsh\\\\Cli\\\\Service\\\\Filesystem\\:\\:copyAll\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Local/BuildFlavor/BuildFlavorBase.php + + - + message: "#^Parameter \\#1 \\$string of function substr expects string, string\\|null given\\.$#" + count: 1 + path: src/Local/BuildFlavor/BuildFlavorBase.php + + - + message: "#^Parameter \\#1 \\$target of method Platformsh\\\\Cli\\\\Service\\\\Filesystem\\:\\:symlink\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Local/BuildFlavor/BuildFlavorBase.php + + - + message: "#^Cannot call method getSharedFileMounts\\(\\) on Platformsh\\\\Cli\\\\Local\\\\LocalApplication\\|null\\.$#" + count: 1 + path: src/Local/BuildFlavor/Drupal.php + + - + message: "#^Parameter \\#1 \\$source of method Platformsh\\\\Cli\\\\Service\\\\Filesystem\\:\\:symlinkAll\\(\\) expects string, string\\|null given\\.$#" + count: 2 + path: src/Local/BuildFlavor/Drupal.php + - message: "#^Access to an undefined property PhpParser\\\\Node\\\\Expr\\\\Error\\|PhpParser\\\\Node\\\\Expr\\\\Variable\\:\\:\\$name\\.$#" count: 3 @@ -315,6 +605,11 @@ parameters: count: 2 path: src/Rector/InjectCommandServicesRector.php + - + message: "#^Cannot access property \\$name on PhpParser\\\\Node\\\\Identifier\\|null\\.$#" + count: 2 + path: src/Rector/InjectCommandServicesRector.php + - message: "#^Parameter \\#2 \\$callable of method Rector\\\\Rector\\\\AbstractRector\\:\\:traverseNodesWithCallable\\(\\) expects callable\\(PhpParser\\\\Node\\)\\: \\(array\\\\|int\\|PhpParser\\\\Node\\|null\\), Closure\\(PhpParser\\\\NodeAbstract\\)\\: \\(PhpParser\\\\Node\\\\Expr\\\\Assign\\|null\\) given\\.$#" count: 1 @@ -325,6 +620,11 @@ parameters: count: 1 path: src/Rector/NewServicesRector.php + - + message: "#^Cannot access property \\$name on PhpParser\\\\Node\\\\Identifier\\|null\\.$#" + count: 2 + path: src/Rector/NewServicesRector.php + - message: "#^Parameter \\#2 \\$callable of method Rector\\\\Rector\\\\AbstractRector\\:\\:traverseNodesWithCallable\\(\\) expects callable\\(PhpParser\\\\Node\\)\\: \\(array\\\\|int\\|PhpParser\\\\Node\\|null\\), Closure\\(PhpParser\\\\NodeAbstract\\)\\: \\(PhpParser\\\\Node\\\\Expr\\\\MethodCall\\|null\\) given\\.$#" count: 1 @@ -340,6 +640,16 @@ parameters: count: 1 path: src/Rector/NewServicesRector.php + - + message: "#^Argument of an invalid type array\\\\|null supplied for foreach, only iterables are supported\\.$#" + count: 1 + path: src/Rector/UnnecessaryServiceVariablesRector.php + + - + message: "#^Cannot access property \\$name on PhpParser\\\\Node\\\\Identifier\\|null\\.$#" + count: 1 + path: src/Rector/UnnecessaryServiceVariablesRector.php + - message: "#^Access to an undefined property PhpParser\\\\Node\\\\Arg\\|PhpParser\\\\Node\\\\VariadicPlaceholder\\:\\:\\$value\\.$#" count: 3 @@ -350,11 +660,41 @@ parameters: count: 6 path: src/Rector/UseSelectorServiceRector.php + - + message: "#^Cannot access property \\$name on PhpParser\\\\Node\\\\Identifier\\|null\\.$#" + count: 2 + path: src/Rector/UseSelectorServiceRector.php + - message: "#^Parameter \\#2 \\$callable of method Rector\\\\Rector\\\\AbstractRector\\:\\:traverseNodesWithCallable\\(\\) expects callable\\(PhpParser\\\\Node\\)\\: \\(array\\\\|int\\|PhpParser\\\\Node\\|null\\), Closure\\(PhpParser\\\\NodeAbstract\\)\\: \\(PhpParser\\\\Node\\\\Expr\\\\Assign\\|PhpParser\\\\Node\\\\Expr\\\\MethodCall\\|null\\) given\\.$#" count: 1 path: src/Rector/UseSelectorServiceRector.php + - + message: "#^Cannot call method fetchNextPage\\(\\) on Platformsh\\\\Client\\\\Model\\\\Collection\\|null\\.$#" + count: 1 + path: src/Service/AccessApi.php + + - + message: "#^Cannot call method getData\\(\\) on Platformsh\\\\Client\\\\Model\\\\Collection\\|null\\.$#" + count: 1 + path: src/Service/AccessApi.php + + - + message: "#^Cannot call method hasNextPage\\(\\) on Platformsh\\\\Client\\\\Model\\\\Collection\\|null\\.$#" + count: 1 + path: src/Service/AccessApi.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Service\\\\ActivityMonitor\\:\\:indent\\(\\) should return string but returns string\\|null\\.$#" + count: 1 + path: src/Service/ActivityMonitor.php + + - + message: "#^Parameter \\#1 \\$datetime of function strtotime expects string, string\\|null given\\.$#" + count: 2 + path: src/Service/ActivityMonitor.php + - message: "#^Ternary operator condition is always false\\.$#" count: 1 @@ -375,6 +715,31 @@ parameters: count: 1 path: src/Service/Api.php + - + message: "#^Parameter \\#1 \\$project of method Platformsh\\\\Cli\\\\Service\\\\Api\\:\\:supportsGuaranteedCPU\\(\\) expects Platformsh\\\\Client\\\\Model\\\\Project, Platformsh\\\\Client\\\\Model\\\\Project\\|null given\\.$#" + count: 1 + path: src/Service/Api.php + + - + message: "#^Parameter \\#1 \\$storage of method Platformsh\\\\Client\\\\Session\\\\Session\\:\\:setStorage\\(\\) expects Platformsh\\\\Client\\\\Session\\\\Storage\\\\SessionStorageInterface, Platformsh\\\\Client\\\\Session\\\\Storage\\\\SessionStorageInterface\\|null given\\.$#" + count: 1 + path: src/Service/Api.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Service\\\\CurlCli\\:\\:run\\(\\) should return int but returns int\\|null\\.$#" + count: 1 + path: src/Service/CurlCli.php + + - + message: "#^Parameter \\#2 \\$appName of method Platformsh\\\\Cli\\\\Service\\\\Api\\:\\:getSiteUrl\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Service/Drush.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Service\\\\Filesystem\\:\\:fixTarPath\\(\\) should return string but returns string\\|null\\.$#" + count: 1 + path: src/Service/Filesystem.php + - message: "#^Method Platformsh\\\\Cli\\\\Service\\\\Filesystem\\:\\:remove\\(\\) has parameter \\$files with no value type specified in iterable type iterable\\.$#" count: 1 @@ -385,21 +750,106 @@ parameters: count: 1 path: src/Service/Filesystem.php + - + message: "#^Parameter \\#7 \\$uri of method Platformsh\\\\Cli\\\\Service\\\\Git\\:\\:execute\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Service/Git.php + + - + message: "#^Parameter \\#2 \\$sha of method Platformsh\\\\Cli\\\\Service\\\\GitDataApi\\:\\:getCommitByShaHash\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: src/Service/GitDataApi.php + - message: "#^Call to an undefined method GuzzleHttp\\\\ClientInterface\\:\\:head\\(\\)\\.$#" count: 1 path: src/Service/Identifier.php + - + message: "#^Parameter \\#1 \\$messages of method Symfony\\\\Component\\\\Console\\\\Output\\\\OutputInterface\\:\\:writeln\\(\\) expects iterable\\|string, string\\|null given\\.$#" + count: 1 + path: src/Service/SelfUpdater.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Service\\\\Shell\\:\\:runProcess\\(\\) should return int but returns int\\|null\\.$#" + count: 1 + path: src/Service/Shell.php + + - + message: "#^Parameter \\#1 \\$messages of method Symfony\\\\Component\\\\Console\\\\Output\\\\OutputInterface\\:\\:write\\(\\) expects iterable\\|string, string\\|null given\\.$#" + count: 1 + path: src/Service/Shell.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Service\\\\Ssh\\:\\:getHost\\(\\) should return string\\|false but returns string\\|false\\|null\\.$#" + count: 1 + path: src/Service/Ssh.php + + - + message: "#^Argument of an invalid type array\\\\|null supplied for foreach, only iterables are supported\\.$#" + count: 1 + path: src/Service/TunnelManager.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Service\\\\TunnelManager\\:\\:getTunnels\\(\\) should return array\\ but returns array\\\\|null\\.$#" + count: 1 + path: src/Service/TunnelManager.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Util\\\\Csv\\:\\:formatCell\\(\\) should return string but returns string\\|null\\.$#" + count: 1 + path: src/Util/Csv.php + + - + message: "#^Parameter \\#3 \\$subject of function str_replace expects array\\|string, string\\|null given\\.$#" + count: 1 + path: src/Util/OsUtil.php + + - + message: "#^Method Platformsh\\\\Cli\\\\Util\\\\PlainFormat\\:\\:formatCell\\(\\) should return string but returns string\\|null\\.$#" + count: 1 + path: src/Util/PlainFormat.php + - message: "#^Dead catch \\- InvalidArgumentException is never thrown in the try block\\.$#" count: 1 path: tests/Command/User/UserAddCommandTest.php + - + message: "#^Parameter \\#1 \\$parentDir of method Platformsh\\\\Cli\\\\Tests\\\\Local\\\\BuildFlavor\\\\BuildFlavorTestBase\\:\\:createTempDir\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: tests/Local/BuildFlavor/BuildFlavorTestBase.php + + - + message: "#^Cannot call method getTreeId\\(\\) on Platformsh\\\\Cli\\\\Local\\\\LocalBuild\\|null\\.$#" + count: 2 + path: tests/Local/LocalBuildTest.php + - message: "#^Property Platformsh\\\\Cli\\\\Tests\\\\Local\\\\LocalBuildTest\\:\\:\\$localBuild \\(Platformsh\\\\Cli\\\\Local\\\\LocalBuild\\|null\\) does not accept object\\.$#" count: 1 path: tests/Local/LocalBuildTest.php + - + message: "#^Parameter \\#1 \\$parentDir of method Platformsh\\\\Cli\\\\Tests\\\\Service\\\\DrushServiceTest\\:\\:createTempDir\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: tests/Service/DrushServiceTest.php + + - + message: "#^Parameter \\#1 \\$parentDir of method Platformsh\\\\Cli\\\\Tests\\\\Service\\\\FilesystemServiceTest\\:\\:createTempDir\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: tests/Service/FilesystemServiceTest.php + + - + message: "#^Parameter \\#1 \\$parentDir of method Platformsh\\\\Cli\\\\Tests\\\\Service\\\\GitServiceTest\\:\\:createTempDir\\(\\) expects string, string\\|null given\\.$#" + count: 1 + path: tests/Service/GitServiceTest.php + + - + message: "#^Parameter \\#1 \\$objectOrMethod of class ReflectionMethod constructor expects object\\|string, Platformsh\\\\Cli\\\\Service\\\\Ssh\\|null given\\.$#" + count: 2 + path: tests/Service/SshTest.php + - message: "#^Property Platformsh\\\\Cli\\\\Tests\\\\Service\\\\SshTest\\:\\:\\$ssh \\(Platformsh\\\\Cli\\\\Service\\\\Ssh\\|null\\) does not accept object\\.$#" count: 1 diff --git a/legacy/phpstan.neon b/legacy/phpstan.neon index d73f7a7b6..64da8f9b1 100644 --- a/legacy/phpstan.neon +++ b/legacy/phpstan.neon @@ -1,5 +1,5 @@ parameters: - level: 7 + level: 8 paths: - dist - resources From 177584d643ce740115b7dc56a56b42881f2baf80 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 17:36:15 +0100 Subject: [PATCH 07/14] fix(legacy): require --metric option in non-interactive autoscaling:set Calling `autoscaling:set --service NAME --duration-up 2m` (or any other scaling option without --metric) reached `validateMetric($metric, ...)` with $metric === null, raising an uncaught TypeError. Trigger path: non-interactive mode, $service set, at least one scaling option set, no --metric. Now the command emits a readable error and exits with code 1, matching the message used when --service is omitted. Verified by integration-tests/autoscaling_validate_metric_test.go, which crashed against the unfixed binary and passes after the fix. Corresponding entry removed from legacy/phpstan-baseline.neon. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../autoscaling_validate_metric_test.go | 130 ++++++++++++++++++ legacy/phpstan-baseline.neon | 5 - .../AutoscalingSettingsSetCommand.php | 4 + 3 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 integration-tests/autoscaling_validate_metric_test.go diff --git a/integration-tests/autoscaling_validate_metric_test.go b/integration-tests/autoscaling_validate_metric_test.go new file mode 100644 index 000000000..3a3b431e9 --- /dev/null +++ b/integration-tests/autoscaling_validate_metric_test.go @@ -0,0 +1,130 @@ +package tests + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestAutoscalingSettingsSetMissingMetric verifies that autoscaling:set fails +// cleanly when the user passes --service and another scaling option but omits +// --metric. PHPStan level 8 flags validateMetric($metric, ...) at line 290 +// because $metric is string|null and the method expects a string. Before the +// fix, PHP would throw a TypeError; after the fix the CLI must report a +// readable error and exit non-zero. +func TestAutoscalingSettingsSetMissingMetric(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + main.Links["#autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"} + main.Links["#manage-autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"} + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + apiHandler.Get("/projects/"+projectID+"/capabilities", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "autoscaling": map[string]any{ + "enabled": true, + "supports_horizontal_scaling_services": false, + }, + }) + }) + + deploymentPath := "/projects/" + projectID + "/environments/main/deployments/current" + apiHandler.Get(deploymentPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "webapps": map[string]any{ + "app": map[string]any{ + "name": "app", + "type": "golang:1.23", + "instance_count": 1, + "disk": 512, + "resources": map[string]any{ + "profile_size": "0.1", + }, + }, + }, + "services": map[string]any{}, + "workers": map[string]any{}, + "routes": map[string]any{}, + "_links": mockapi.MakeHALLinks( + "self=" + deploymentPath, + ), + }) + }) + + autoscalingPath := "/projects/" + projectID + "/environments/main/autoscaling" + apiHandler.Get(autoscalingPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "defaults": map[string]any{ + "triggers": map[string]any{ + "cpu": map[string]any{ + "up": map[string]any{"threshold": 80, "duration": 60}, + "down": map[string]any{"threshold": 20, "duration": 60}, + }, + }, + "scale_cooldown": map[string]any{"up": 300, "down": 300}, + "instances": map[string]any{"min": 1, "max": 10}, + }, + "services": map[string]any{ + "app": map[string]any{ + "enabled": true, + "triggers": map[string]any{ + "cpu": map[string]any{ + "enabled": true, + "up": map[string]any{"threshold": 80, "duration": 60}, + "down": map[string]any{"threshold": 20, "duration": 60}, + }, + }, + "instances": map[string]any{"min": 1, "max": 3}, + }, + }, + "_links": mockapi.MakeHALLinks( + "self=" + autoscalingPath, + ), + }) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + // Non-interactive, --service plus another option, no --metric. + // This drives execution past line 289 (!empty($updates[$service])) which + // then invokes validateMetric(null) at line 290. + stdout, stderr, err := f.RunCombinedOutput( + "autoscaling:set", + "-p", projectID, + "-e", "main", + "--service", "app", + "--duration-up", "2m", + "--dry-run", + ) + + combined := stdout + "\n---\n" + stderr + assert.NotContains(t, combined, "TypeError", "stdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, combined, "must be of type string, null given") + assert.NotContains(t, combined, "Fatal error") + // A successful exit (0) or a clean validation error (non-zero with a + // readable message) are both acceptable. A TypeError is not. + _ = err +} diff --git a/legacy/phpstan-baseline.neon b/legacy/phpstan-baseline.neon index 9a6fa14a0..4e5128bc2 100644 --- a/legacy/phpstan-baseline.neon +++ b/legacy/phpstan-baseline.neon @@ -30,11 +30,6 @@ parameters: count: 2 path: src/Command/Autoscaling/AutoscalingSettingsSetCommand.php - - - message: "#^Parameter \\#1 \\$value of method Platformsh\\\\Cli\\\\Command\\\\Autoscaling\\\\AutoscalingSettingsSetCommand\\:\\:validateMetric\\(\\) expects string, string\\|null given\\.$#" - count: 1 - path: src/Command/Autoscaling/AutoscalingSettingsSetCommand.php - - message: "#^Access to an undefined property Platformsh\\\\Client\\\\Model\\\\Backup\\:\\:\\$safe\\.$#" count: 2 diff --git a/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php b/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php index 3ffd80adc..108719675 100644 --- a/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php +++ b/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php @@ -287,6 +287,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int } if (!empty($updates[$service])) { + if ($metric === null) { + $this->stdErr->writeln('The --metric option is required when not running interactively.'); + return 1; + } $metric = $this->validateMetric($metric, $supportedMetrics); // since we have some changes, inject the metric name for them $updates[$service]['metric'] = $metric; From 5358fd5335f7956bd71d2b9ca09f5a84e83aa9ed Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 17:36:21 +0100 Subject: [PATCH 08/14] test(integration): cover autoscaling:set for a service not in autoscaling settings PHPStan level 8 flags two `$current['triggers']` accesses in `summarizeChangesPerService` (AutoscalingSettingsSetCommand.php:599, 601) where $current can be null when $settings[$service] is unset. The trigger path is enabling autoscaling for a service that does not yet appear in the project's autoscaling settings. The CLI does not crash on this path. In PHP 8 the null-offset access emits a warning but yields null, which evaluates as falsy and behaves as expected for a service that does not currently have triggers. The baseline entry for this finding is retained. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../autoscaling_new_service_test.go | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 integration-tests/autoscaling_new_service_test.go diff --git a/integration-tests/autoscaling_new_service_test.go b/integration-tests/autoscaling_new_service_test.go new file mode 100644 index 000000000..34023bc82 --- /dev/null +++ b/integration-tests/autoscaling_new_service_test.go @@ -0,0 +1,118 @@ +package tests + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestAutoscalingSettingsSetNewService verifies that autoscaling:set handles +// the case where the targeted service exists in the deployment but does not +// yet appear in the project's autoscaling settings. PHPStan level 8 flags +// $current['triggers'] on a nullable $current at lines 599/601 of +// AutoscalingSettingsSetCommand.php — summarizeChangesPerService receives +// null when $settings[$service] is unset. +func TestAutoscalingSettingsSetNewService(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + main.Links["#autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"} + main.Links["#manage-autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"} + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + apiHandler.Get("/projects/"+projectID+"/capabilities", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "autoscaling": map[string]any{ + "enabled": true, + "supports_horizontal_scaling_services": false, + }, + }) + }) + + deploymentPath := "/projects/" + projectID + "/environments/main/deployments/current" + apiHandler.Get(deploymentPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "webapps": map[string]any{ + "app": map[string]any{ + "name": "app", + "type": "golang:1.23", + "instance_count": 1, + "disk": 512, + "resources": map[string]any{ + "profile_size": "0.1", + }, + }, + }, + "services": map[string]any{}, + "workers": map[string]any{}, + "routes": map[string]any{}, + "_links": mockapi.MakeHALLinks( + "self=" + deploymentPath, + ), + }) + }) + + // Autoscaling settings with "app" NOT in services — so $current will be + // null when summarizeChangesPerService is invoked for it. + autoscalingPath := "/projects/" + projectID + "/environments/main/autoscaling" + apiHandler.Get(autoscalingPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "defaults": map[string]any{ + "triggers": map[string]any{ + "cpu": map[string]any{ + "up": map[string]any{"threshold": 80, "duration": 60}, + "down": map[string]any{"threshold": 20, "duration": 60}, + }, + }, + "scale_cooldown": map[string]any{"up": 300, "down": 300}, + "instances": map[string]any{"min": 1, "max": 10}, + }, + "services": map[string]any{}, + "_links": mockapi.MakeHALLinks( + "self=" + autoscalingPath, + ), + }) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + // Enable autoscaling for a service not yet in autoscaling settings. + stdout, stderr, err := f.RunCombinedOutput( + "autoscaling:set", + "-p", projectID, + "-e", "main", + "--service", "app", + "--metric", "cpu", + "--enabled", "true", + "--duration-up", "2m", + "--dry-run", + ) + + combined := stdout + "\n---\n" + stderr + assert.NotContains(t, combined, "TypeError", "stdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, combined, "Cannot access offset") + assert.NotContains(t, combined, "Fatal error", "stdout: %s\nstderr: %s", stdout, stderr) + _ = err +} From 12e5b58a70a9bf72936a0ed5e19125c3e4cabce5 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 17:59:13 +0100 Subject: [PATCH 09/14] fix(legacy): handle missing autoscaling defaults from API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AutoscalingSettingsSetCommand assumed $autoscalingSettings['defaults'] was always present and an array. When the autoscaling API returned a settings payload that omitted the "defaults" key (or set it to null), $defaults was null and the next call — getSupportedMetrics($defaults), typed `array $defaults` — raised a TypeError on line 112, crashing the CLI with an unhandled fatal error. Same shape as the null-duration bug fixed in bde012ce: a nested API field assumed present at the top of the command, with no guard before downstream code that expects an array. Treat a missing or non-array `defaults` as the API failing to return the information the command needs, print an error, and exit 1. The surrounding code already exits with similar messages when $autoscalingSettings itself is unavailable or the manage-autoscaling link is missing. Integration test added: drives autoscaling:set against a mock that returns an autoscaling-settings payload with no "defaults" key. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../autoscaling_missing_defaults_test.go | 109 ++++++++++++++++++ .../AutoscalingSettingsSetCommand.php | 6 +- 2 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 integration-tests/autoscaling_missing_defaults_test.go diff --git a/integration-tests/autoscaling_missing_defaults_test.go b/integration-tests/autoscaling_missing_defaults_test.go new file mode 100644 index 000000000..14b49b34a --- /dev/null +++ b/integration-tests/autoscaling_missing_defaults_test.go @@ -0,0 +1,109 @@ +package tests + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestAutoscalingSettingsSetMissingDefaults verifies that autoscaling:set +// fails gracefully when the autoscaling-settings API response omits the +// "defaults" key (or returns null for it). The pre-fix code unconditionally +// dereferenced $defaults['instances']['max'] and called +// getSupportedMetrics($defaults), which is typed array, so PHP raised a +// TypeError on null. +func TestAutoscalingSettingsSetMissingDefaults(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + main.Links["#autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"} + main.Links["#manage-autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"} + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + apiHandler.Get("/projects/"+projectID+"/capabilities", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "autoscaling": map[string]any{ + "enabled": true, + "supports_horizontal_scaling_services": false, + }, + }) + }) + + deploymentPath := "/projects/" + projectID + "/environments/main/deployments/current" + apiHandler.Get(deploymentPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "webapps": map[string]any{ + "app": map[string]any{ + "name": "app", + "type": "golang:1.23", + "instance_count": 1, + "disk": 512, + "resources": map[string]any{ + "profile_size": "0.1", + }, + }, + }, + "services": map[string]any{}, + "workers": map[string]any{}, + "routes": map[string]any{}, + "_links": mockapi.MakeHALLinks( + "self=" + deploymentPath, + ), + }) + }) + + // Autoscaling settings with no "defaults" key — the API payload is + // otherwise valid. The unfixed CLI assumed $defaults was always present. + autoscalingPath := "/projects/" + projectID + "/environments/main/autoscaling" + apiHandler.Get(autoscalingPath, func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "services": map[string]any{}, + "_links": mockapi.MakeHALLinks( + "self=" + autoscalingPath, + ), + }) + }) + + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + stdout, stderr, err := f.RunCombinedOutput( + "autoscaling:set", + "-p", projectID, + "-e", "main", + "--service", "app", + "--metric", "cpu", + "--enabled", "true", + "--dry-run", + ) + + combined := stdout + "\n---\n" + stderr + assert.NotContains(t, combined, "TypeError", "stdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, combined, "must be of type array, null given") + assert.NotContains(t, combined, "Cannot access offset") + assert.NotContains(t, combined, "Fatal error", "stdout: %s\nstderr: %s", stdout, stderr) + // The CLI should exit non-zero with an actionable message. + assert.Error(t, err, "expected non-zero exit when defaults are missing") + assert.Contains(t, stderr, "autoscaling", "stderr: %s", stderr) +} diff --git a/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php b/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php index 108719675..7a191b378 100644 --- a/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php +++ b/legacy/src/Command/Autoscaling/AutoscalingSettingsSetCommand.php @@ -100,7 +100,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int } // Get autoscaling default values - $defaults = $autoscalingSettings['defaults']; + $defaults = $autoscalingSettings['defaults'] ?? null; + if (!is_array($defaults)) { + $this->stdErr->writeln('The autoscaling API did not return any default settings for this environment.'); + return 1; + } // Validate the --service option. $service = $input->getOption('service'); From 0f4ad2c1b18f1f402707a0c63183f4558c66f01c Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 18:15:25 +0100 Subject: [PATCH 10/14] fix(legacy): tolerate null timestamps in ActivityMonitor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 915a95af, which fixed the same null-timestamp crash in `Model\Activity::getDuration()` and `ActivityLoader::load()` but missed `Service\ActivityMonitor`. Two call sites here crashed on an activity whose `created_at` or `started_at` was a literal null from the API: - `getStart()` (line 677) falls back to `strtotime($activity->created_at)` when `started_at` is empty. Reached on every multi-activity wait flow via `waitMultiple()` — `environment:redeploy --wait`, backup-await, snapshot-await, anything streaming activities through the progress bar. - The most-recent-timestamp loop at line 481 calls `strtotime($activity->created_at)` directly. Both now use the same `!empty(...) ? strtotime(...) : false` guard as the earlier fix. Downstream code already handles the `false` return. Integration test added that drives `environment:redeploy --wait` against a mock returning a redeploy activity collection with null timestamps; it reproduced the crash against the unfixed binary and passes after the fix. Surfaced by a skeptical re-review of the level-8 baseline, which the process now requires after a level raise. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../activity_monitor_null_timestamp_test.go | 120 ++++++++++++++++++ legacy/src/Service/ActivityMonitor.php | 7 +- 2 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 integration-tests/activity_monitor_null_timestamp_test.go diff --git a/integration-tests/activity_monitor_null_timestamp_test.go b/integration-tests/activity_monitor_null_timestamp_test.go new file mode 100644 index 000000000..4e9782b5d --- /dev/null +++ b/integration-tests/activity_monitor_null_timestamp_test.go @@ -0,0 +1,120 @@ +package tests + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestActivityMonitorNullTimestamp drives `environment:redeploy` against a +// response whose embedded activities have null started_at and created_at. +// This exercises Platformsh\Cli\Service\ActivityMonitor::getStart() (line 677) +// and the strtotime() call in waitMultiple() (line 481), both of which feed +// $activity->created_at directly to strtotime() without a null guard. The same +// shape of bug was previously fixed in Model\Activity (commit 915a95af) but +// these two sites in ActivityMonitor were missed. +// +// To skip the `count(nonIntegrationActivities) === 1` short-circuit in +// waitMultiple() (which would route through waitAndLog() and getLogStream(), +// requiring more mocking), the redeploy response embeds two non-integration +// activities, so the multi-activity branch (lines 449-515) runs. +func TestActivityMonitorNullTimestamp(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + + apiHandler := mockapi.NewHandler(t) + + projectID := mockapi.ProjectID() + + apiHandler.SetProjects([]*mockapi.Project{{ + ID: projectID, + Links: mockapi.MakeHALLinks( + "self=/projects/"+projectID, + "environments=/projects/"+projectID+"/environments", + ), + DefaultBranch: "main", + }}) + + main := makeEnv(projectID, "main", "production", "active", nil) + main.Links["#redeploy"] = mockapi.HALLink{ + HREF: "/projects/" + projectID + "/environments/main/redeploy", + } + main.Links["#activities"] = mockapi.HALLink{ + HREF: "/projects/" + projectID + "/environments/main/activities", + } + apiHandler.SetEnvironments([]*mockapi.Environment{main}) + + // Two complete non-integration activities with null timestamps. The pair + // avoids the count(nonIntegration)===1 short-circuit and forces the + // multi-activity progress-bar branch in waitMultiple(), which calls + // getStart() (line 456) and strtotime($activity->created_at) (line 481). + // completion_percent=100 lets the wait loop exit on the first iteration. + activityJSON := func(id string) string { + return `{ + "id": "` + id + `", + "type": "environment.redeploy", + "state": "complete", + "result": "success", + "completion_percent": 100, + "completed_at": null, + "started_at": null, + "created_at": null, + "updated_at": null, + "project": "` + projectID + `", + "environments": ["main"], + "description": "Redeploy with null timestamps", + "text": "Redeploy with null timestamps", + "payload": {} + }` + } + redeployResponse := `{ + "_embedded": { + "activities": [` + activityJSON("actA") + `,` + activityJSON("actB") + `] + } + }` + // Project-level activities endpoint, queried by waitMultiple() to refresh + // activity state on each poll (line 495). + projectActivitiesJSON := "[" + activityJSON("actA") + "," + activityJSON("actB") + "]" + + redeployPath := "/projects/" + projectID + "/environments/main/redeploy" + projectActivitiesPath := "/projects/" + projectID + "/activities" + + wrapped := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == redeployPath { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(redeployResponse)) + return + } + if r.Method == http.MethodGet && r.URL.Path == projectActivitiesPath { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(projectActivitiesJSON)) + return + } + apiHandler.ServeHTTP(w, r) + }) + + apiServer := httptest.NewServer(wrapped) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + + stdout, stderr, err := f.RunCombinedOutput( + "environment:redeploy", "-p", projectID, "-e", "main", "-y", "--wait", + ) + t.Logf("environment:redeploy err=%v\nstdout=%s\nstderr=%s", err, stdout, stderr) + + // Surface TypeError / fatal errors loudly. + if strings.Contains(stderr, "TypeError") || + strings.Contains(stderr, "must be of type string") || + strings.Contains(stderr, "Fatal error") || + strings.Contains(stderr, "Uncaught") { + t.Fatalf("legacy CLI raised a PHP error on null timestamp:\n%s", stderr) + } + + if err != nil { + t.Fatalf("environment:redeploy failed unexpectedly: %v\nstderr=%s", err, stderr) + } +} diff --git a/legacy/src/Service/ActivityMonitor.php b/legacy/src/Service/ActivityMonitor.php index 8630b5b03..3ab7b1614 100644 --- a/legacy/src/Service/ActivityMonitor.php +++ b/legacy/src/Service/ActivityMonitor.php @@ -478,7 +478,7 @@ public function waitMultiple(array $activities, Project $project, bool $context // timestamp, so that they can be more efficiently refreshed. $mostRecentTimestamp = 0; foreach ($activities as $activity) { - $created = strtotime($activity->created_at); + $created = !empty($activity->created_at) ? strtotime($activity->created_at) : false; $mostRecentTimestamp = $created > $mostRecentTimestamp ? $created : $mostRecentTimestamp; } @@ -674,7 +674,10 @@ public static function getFormattedDescription(Activity $activity, bool $withDec */ private function getStart(Activity $activity): int|false { - return !empty($activity->started_at) ? strtotime($activity->started_at) : strtotime($activity->created_at); + if (!empty($activity->started_at)) { + return strtotime($activity->started_at); + } + return !empty($activity->created_at) ? strtotime($activity->created_at) : false; } /** From d138992268debcc60c78c5bb7e4b20c48b24b8e7 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 18:22:48 +0100 Subject: [PATCH 11/14] fix(legacy): drop stale ActivityMonitor strtotime baseline entry 0f4ad2c1 fixed the two `strtotime($activity->X)` calls in `Service\ActivityMonitor` but forgot to remove the corresponding baseline entry. PHPStan flags "ignored error pattern not matched" under `lint-phpstan` in CI, which currently fails on PR #90. Co-Authored-By: Claude Opus 4.7 (1M context) --- legacy/phpstan-baseline.neon | 5 ----- 1 file changed, 5 deletions(-) diff --git a/legacy/phpstan-baseline.neon b/legacy/phpstan-baseline.neon index 4e5128bc2..76668f579 100644 --- a/legacy/phpstan-baseline.neon +++ b/legacy/phpstan-baseline.neon @@ -685,11 +685,6 @@ parameters: count: 1 path: src/Service/ActivityMonitor.php - - - message: "#^Parameter \\#1 \\$datetime of function strtotime expects string, string\\|null given\\.$#" - count: 2 - path: src/Service/ActivityMonitor.php - - message: "#^Ternary operator condition is always false\\.$#" count: 1 From d676c96ba570ae9fbe8f408a94979b305422eb39 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 18:23:06 +0100 Subject: [PATCH 12/14] fix(legacy): break ActivityLoader pagination when created_at is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 915a95af guarded the `new DateTime($activity->created_at)` constructor against a null timestamp, but left the pagination cursor unchanged in that case — the next `getActivities()` call would re-request the same window and only stop on the !\$new duplicate-detection check after a redundant round-trip. Break the loop instead: without a valid created_at we can't advance the cursor at all. Per Copilot's review of upsun/cli#90. Co-Authored-By: Claude Opus 4.7 (1M context) --- legacy/src/Service/ActivityLoader.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/legacy/src/Service/ActivityLoader.php b/legacy/src/Service/ActivityLoader.php index 380cd6c3d..55d4d4b08 100644 --- a/legacy/src/Service/ActivityLoader.php +++ b/legacy/src/Service/ActivityLoader.php @@ -127,9 +127,12 @@ public function load(HasActivitiesInterface $apiResource, ?int $limit = null, ar $activities = []; while ($limit === null || count($activities) < $limit) { if ($activity = end($activities)) { - if (!empty($activity->created_at)) { - $startsAt = new DateTime($activity->created_at); + if (empty($activity->created_at)) { + // Can't advance the cursor without a timestamp; the next + // page would be a duplicate of this one. + break; } + $startsAt = new DateTime($activity->created_at); } $nextActivities = $apiResource->getActivities($limit ? $limit - count($activities) : 0, $types, $startsAt, $state, $result); $new = false; From 0cdde3b77cd2c399772714b9b8877d03749ef38f Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 18:24:27 +0100 Subject: [PATCH 13/14] fix(legacy): surface missing profile size in resources:set summary 8a15e400 stopped the TypeError when sizeInfo() returned null for the new (requested) size, but did so by silently substituting an empty string, which made the summary render a confusing blank `` placeholder. Detect the missing-new-size case explicitly and print a one-line notice that names the size and says CPU/memory details aren't available for it. Per Copilot's review of upsun/cli#90. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Command/Resources/ResourcesSetCommand.php | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/legacy/src/Command/Resources/ResourcesSetCommand.php b/legacy/src/Command/Resources/ResourcesSetCommand.php index eedea0bf2..45476e246 100644 --- a/legacy/src/Command/Resources/ResourcesSetCommand.php +++ b/legacy/src/Command/Resources/ResourcesSetCommand.php @@ -432,18 +432,26 @@ private function summarizeChangesPerService(string $name, WebApp|Worker|Service $newProperties = array_replace_recursive($properties, $updates); $newSizeInfo = $this->resourcesUtil->sizeInfo($newProperties, $containerProfiles); - $previousCPU = $sizeInfo !== null - ? $this->resourcesUtil->formatCPU($sizeInfo['cpu']) . ' ' . $this->formatCPUType($sizeInfo) - : null; - $newCPU = $newSizeInfo !== null - ? $this->resourcesUtil->formatCPU($newSizeInfo['cpu']) . ' ' . $this->formatCPUType($newSizeInfo) - : ''; - $this->stdErr->writeln(' CPU: ' . $this->resourcesUtil->formatChange($previousCPU, $newCPU)); - $this->stdErr->writeln(' Memory: ' . $this->resourcesUtil->formatChange( - $sizeInfo !== null ? $sizeInfo['memory'] : null, - $newSizeInfo !== null ? $newSizeInfo['memory'] : null, - ' MB', - )); + if ($newSizeInfo === null) { + // The requested profile_size isn't in the deployment's + // container_profiles catalog, so we can't show CPU/memory + // changes. Surface that rather than printing blank values. + $this->stdErr->writeln(sprintf( + ' Size: %s (CPU/memory details unavailable for this profile size)', + $updates['resources']['profile_size'], + )); + } else { + $previousCPU = $sizeInfo !== null + ? $this->resourcesUtil->formatCPU($sizeInfo['cpu']) . ' ' . $this->formatCPUType($sizeInfo) + : null; + $newCPU = $this->resourcesUtil->formatCPU($newSizeInfo['cpu']) . ' ' . $this->formatCPUType($newSizeInfo); + $this->stdErr->writeln(' CPU: ' . $this->resourcesUtil->formatChange($previousCPU, $newCPU)); + $this->stdErr->writeln(' Memory: ' . $this->resourcesUtil->formatChange( + $sizeInfo !== null ? $sizeInfo['memory'] : null, + $newSizeInfo['memory'], + ' MB', + )); + } } if (isset($updates['instance_count'])) { $this->stdErr->writeln(' Instance count: ' . $this->resourcesUtil->formatChange( From d572bf9e403e02fd43bb0689396bff7006b87cc1 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Tue, 26 May 2026 18:24:48 +0100 Subject: [PATCH 14/14] test(integration): assert request decoding in runtime_operation_test The mock handler ignored the ReadAll() and Unmarshal() errors. If either failed, receivedBody would be unset and downstream assertions could pass or fail for the wrong reason. Per Copilot's review of upsun/cli#90. Co-Authored-By: Claude Opus 4.7 (1M context) --- integration-tests/runtime_operation_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/integration-tests/runtime_operation_test.go b/integration-tests/runtime_operation_test.go index 5eee7f6a6..c55fe242f 100644 --- a/integration-tests/runtime_operation_test.go +++ b/integration-tests/runtime_operation_test.go @@ -78,9 +78,10 @@ func TestRuntimeOperationRun(t *testing.T) { var receivedBody atomic.Value // map[string]any apiHandler.Post(envPath+"/runtime-operations", func(w http.ResponseWriter, r *http.Request) { - b, _ := io.ReadAll(r.Body) + b, err := io.ReadAll(r.Body) + require.NoError(t, err) var body map[string]any - _ = json.Unmarshal(b, &body) + require.NoError(t, json.Unmarshal(b, &body)) receivedBody.Store(body) _ = json.NewEncoder(w).Encode(map[string]any{ "_embedded": map[string]any{"activities": []any{}},