[pjcdawkins]

Types the resources and autoscaling data and Config's lookups, reducing the legacy PHPStan baseline from 1,506 to 1,139 errors. The data is typed once where it is read, and values that are missing or of an unexpected type are left out, so the commands handle their absence.

Autoscaling (Model\AutoscalingSettings, returned by Api::getAutoscalingSettings()):

• autoscaling:set failed with a TypeError when summarizing a change to a service whose settings omit a trigger's duration, a cooldown or an instance limit, e.g. after a partial update.
• autoscaling:get only showed a trigger's enabled state in the rows after the enabled key, so it depended on the order of the API's keys. Directions are now listed up, then down.

Resources (ResourcesUtil::resources(), containerProfiles(), containerProfile(), instanceCount(), disk() and buildResources()):

• resources:build:set compared the CPU option, a float, with the API's integer, so setting the current CPU sent an unchanged value. It also rejected every value when the project's capabilities had no build limits.
• resources:size:list handles a task's default profile, and a profile that isn't in the deployment.
• resources:get no longer copies raw resources values into its table cells.
• QuestionHelper::askInput() returns its validator's type, and ResourcesCommandBase's unused copies of ResourcesUtil methods are removed.

Config:

• Its own lookups use has(), getStr() and a private set() instead of indexing its nested array.
• getStrOrNull() returns an optional string, and getStrList() a list of strings. A string is split into lines, as environment variables set lists that way (e.g. SSH options and host keys).

autoscaling:set's $updates array and its generic askForSetting() are left untyped (about 20 errors): typing them means reworking the interactive flow.

🤖 Generated with Claude Code

[upsun-dispatch]

Warning

Changes suggested — 🟡 1 warning

🔍 Full review · 29 files reviewed

Verification

• Config::getEnv() is only called with addPrefix=false before $this->config is loaded, in the constructor's CLI_CONFIG_FILE lookup, so the new getStrOrNull('application.env_prefix') call does not read uninitialised state.
• The renamed-key migration loop keeps the old isset semantics: has() with its default notNull=true treats null values as unset, as isset did.
• BuildResourcesSetCommand now compares a float CPU option with buildResources()'s float, and askInput turns the int memory default into a string before Symfony validates it, so accepting the current values sends no PATCH.
• ResourcesSetCommand::computeMemoryCPUStorageDiff and the instance-count path read the new flat CurrentService shape (profile_size, instance_count, disk, sizes) consistently. No old ['resources']['profile_size'] reads of $current remain.
• containerProfiles() drops only sizes that lack cpu, memory or cpu_type, and the mock API's OpenAPI spec marks all three as required, so the removed ?? '' fallbacks for cpu_type are safe.

Tests added in the diff: Go integration tests TestAutoscalingSettingsGetDirections and TestAutoscalingSettingsSetPartial (non-interactive only), the build-resources validation case for --cpu 1, and PHPUnit tests ConfigTest::testGetStrList/testGetStrOrNull plus an updated ResourcesSetTest. CI's PHP job runs make lint-php (PHPStan with the reduced baseline) and make test-php; the integration-test job covers the Go tests. No test covers the interactive autoscaling prompts when the defaults are partial.

Review details

• Commit: 228d81f
• Model: claude-opus-5-5

---

Review 1 of 10 for this pull request · View the full run

[upsun-dispatch]

Warning

Changes suggested — 🟡 2 warnings

🔍 Full review · 30 files reviewed

Verification

• requireAnswer() turns Symfony's null answer (an empty reply with no default) into an InvalidArgumentException. All validators for the threshold, duration, cooldown and instance count call it, so a null default no longer reaches a string parameter.
• QuestionHelper::chooseAssoc() returns a string key, so $profileSize !== $currentSize in resources:set compares strings. resources() also casts profile_size to string.
• $currentDisk ?: $resources['default']['disk'] ?? 512 parses as $currentDisk ?: (… ?? 512), because ?? binds tighter than ?:.
• Config::getStrOrNull() returns null for an explicit null value, because has() treats null as undefined. This keeps the old getWithDefault fallbacks for timezone, session_id, user_agent and similar keys.
• The magic container_profile read in ResourcesUtil::containerProfile() works with ??, because ReadOnlyStructureTrait implements __isset.

The diff adds integration tests: autoscaling_get_test.go covers direction order, a partial set and interactive partial defaults, and resources_build_test.go covers comparing --cpu 1 with the current value. It also adds ConfigTest cases for getStrList and getStrOrNull. CI's integration-test and the make lint-php/make test-php steps in ci.yml run them. No test covers a trigger without enabled following an enabled one in autoscaling:get, or a project without build_resources.

Review details

• Commit: f06ac4a
• Model: claude-opus-5-5

---

Review 3 of 10 for this pull request · View the full run

[upsun-dispatch]

Warning

Changes suggested — 🟡 1 warning

🔁 Incremental (head + base moved) · 6 files reviewed

Verification

• AutoscalingSettingsGetCommand now sets $row['enabled'] for every trigger, so a metric that has no enabled no longer takes the previous metric's value.
• buildResources() no longer throws: it returns null for a missing or non-numeric value, and resources:build:get renders that as an empty cell.
• summarizeChanges() leaves out a CPU or memory line only when both the update and the current value are null, and formatChange() accepts a null previous value.
• The base branch's operation:list/run changes touch no symbol this PR uses or changes.

This commit extends TestAutoscalingSettingsGetDirections with a memory trigger that has no enabled, and adds TestBuildResourcesPartial for resources:build:get and for the non-interactive resources:build:set --memory. No test covers the interactive build:set prompt when a value is missing.

Review details

• Commits: f06ac4a..3fe2e40
• Base moved since: 7b036aa
• Model: claude-opus-5-5

---

Review 4 of 10 for this pull request · View the full run

[upsun-dispatch]

🟡 Warning — Interactive resources:build:set crashes with a TypeError on exactly the partial settings this commit sets out to support, instead of asking again.

This commit lets buildResources() return null CPU or memory, and resources:build:set now passes that null as the default to askInput(). That makes a crash reachable that the old code never got to. When the default is null, askInput() does not cast it to a string. If the user presses Enter, Symfony passes null to the validator, but $validateCpu and $validateMemory are declared function (string $v). A user-defined closure does not coerce null to a non-nullable string, even outside strict mode, so PHP throws a TypeError. Symfony's validateAttempts() only catches \Exception, so the TypeError gets past the retry loop and the command crashes with an uncaught error. Example: build_resources is {cpu: 1} (the case TestBuildResourcesPartial covers), the user runs resources:build:set with no options, and presses Enter at the memory prompt (line 114, which passes $current['memory']). The same happens at the CPU prompt when cpu is missing. autoscaling:set handles the same situation with requireAnswer(?string), which shows 'A value is required.' and asks again.