Fix TypeError when saving integer CPU core count in global settings by Copilot · Pull Request #376 · R0Wi-DEV/workflow_ocr

Copilot · 2026-03-30T14:03:28Z

Agent-Logs-Url: https://github.com/R0Wi-DEV/workflow_ocr/sessions/e5743218-a210-416d-b782-64fc77d0fb16 Co-authored-by: R0Wi <19730957+R0Wi@users.noreply.github.com>

…erties Agent-Logs-Url: https://github.com/R0Wi-DEV/workflow_ocr/sessions/ca3e8666-830a-42ce-9ae2-c2dbd88e3dc9 Co-authored-by: R0Wi <19730957+R0Wi@users.noreply.github.com>

…ull and improve type management

Copilot

Pull request overview

Fixes a TypeError when persisting integer-valued global settings (CPU core count / timeout) by moving these settings to true integers end-to-end (model → controller/service → appconfig), including a migration for existing string-typed config entries.

Changes:

Convert GlobalSettings numeric fields to ?int and persist them via IAppConfig::getValueInt()/setValueInt() using reflection-based type detection.
Add a migration to convert legacy VALUE_STRING appconfig entries for processorCount/timeout into VALUE_INT.
Update Vue UI + unit tests (PHP + JS) to treat these values as numbers and allow null/unset behavior.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`lib/Model/GlobalSettings.php`	Change global settings fields to typed `?int`.
`lib/Service/GlobalSettingsService.php`	Read/write int settings via `getValueInt`/`setValueInt`, delete keys when nullable and `null`.
`lib/Controller/GlobalSettingsController.php`	Cast incoming request payload values to ints/null.
`lib/Migration/Version1034Date20260331194630.php`	Migrate stored config keys from string type to int type.
`lib/OcrProcessors/Remote/Client/ApiClient.php`	Simplify timeout resolution now that settings are typed ints.
`lib/OcrProcessors/CommandLineUtils.php`	Stop string→int coercion for processor count; treat null as unset.
`psalm.xml`	Exclude `lib/Migration` from Psalm analysis.
`tests/psalm-baseline.xml`	Remove baseline entries for migration unused-class warnings.
`tests/Unit/Service/GlobalSettingsServiceTest.php`	Update mocks/expectations for int config access + null/unset behavior.
`tests/Unit/Controller/GlobalSettingsControllerTest.php`	Add coverage for passing null settings through the controller.
`tests/Unit/OcrProcessors/Remote/Client/ApiClientTest.php`	Update timeout tests to use ints/null.
`src/components/GlobalSettings.vue`	Convert numeric inputs to numbers/null before saving.
`src/test/service/globalSettingsService.spec.js`	Update mocked payload/response types to integers.
`src/test/components/GlobalSettings.spec.js`	Update component tests for numeric values and empty→null behavior.

Copilot · 2026-03-31T20:45:37Z

lib/Controller/GlobalSettingsController.php

 			$globalSettingsObject = new GlobalSettings();
-			$globalSettingsObject->processorCount = $globalSettings['processorCount'];
-			$globalSettingsObject->timeout = $globalSettings['timeout'] ?? null;
+			$globalSettingsObject->processorCount = isset($globalSettings['processorCount']) ? (int)$globalSettings['processorCount'] : null;
+			$globalSettingsObject->timeout = isset($globalSettings['timeout']) ? (int)$globalSettings['timeout'] : null;



isset($globalSettings['processorCount']) will treat an empty string ("" from an empty number input) as set, and (int)'' becomes 0. That ends up persisting a 0 value even though the service treats <= 0 as “not configured”. Consider coercing "" (and other non-numeric inputs) to null, and ideally normalizing <= 0 to null before calling the service (e.g., via a small helper that returns null unless the parsed value is > 0).

Copilot · 2026-03-31T20:45:38Z

lib/Service/GlobalSettingsService.php

+			if ($isNullable && $value === null) {
+				$this->config->deleteKey(Application::APP_NAME, $key);
+			} elseif ($type instanceof \ReflectionNamedType && $type->getName() === 'int') {
+				$this->config->setValueInt(Application::APP_NAME, $key, (int)$value);


getGlobalSettings() maps int values <= 0 to null, but setGlobalSettings() will still persist 0/negative values via setValueInt() when they are provided. This creates inconsistent semantics (stored value vs returned value) and can leave invalid values in appconfig. Consider treating non-positive ints as “unset” here too (delete the key or skip writing) to keep storage consistent with the read-path logic.

Suggested change

$this->config->setValueInt(Application::APP_NAME, $key, (int)$value);

$intValue = (int)$value;

if ($intValue <= 0) {

// Keep storage consistent with read-path semantics: non-positive ints

// are treated as "not configured", so we delete the key instead of

// persisting an invalid/sentinel value.

$this->config->deleteKey(Application::APP_NAME, $key);

} else {

$this->config->setValueInt(Application::APP_NAME, $key, $intValue);

}

Copilot · 2026-03-31T20:45:38Z

lib/Migration/Version1034Date20260331194630.php

+		// requested one, so we must remove the old entry before re-inserting.
+		$this->appConfig->deleteKey(Application::APP_NAME, $key);
+		$this->appConfig->setValueInt(Application::APP_NAME, $key, $intValue);
+
+		$output->info(sprintf(
+			"Migrated config key '%s' for app '%s' from string to int (value: %d).",
+			$key,
+			Application::APP_NAME,
+			$intValue,
+		));


The migration currently re-inserts a VALUE_INT even when the legacy string value is empty/non-numeric (mapped to 0). Since the rest of the code treats <= 0 as “not configured”, consider deleting the old key and only re-inserting when the parsed value is actually > 0; otherwise just delete and leave it unset. This avoids persisting a sentinel/invalid value in appconfig.

Suggested change

// requested one, so we must remove the old entry before re-inserting.

$this->appConfig->deleteKey(Application::APP_NAME, $key);

$this->appConfig->setValueInt(Application::APP_NAME, $key, $intValue);

$output->info(sprintf(

"Migrated config key '%s' for app '%s' from string to int (value: %d).",

$key,

Application::APP_NAME,

$intValue,

));

// requested one, so we must remove the old entry before deciding whether

// to re-insert it as an integer value.

$this->appConfig->deleteKey(Application::APP_NAME, $key);

if ($intValue > 0) {

$this->appConfig->setValueInt(Application::APP_NAME, $key, $intValue);

$output->info(sprintf(

"Migrated config key '%s' for app '%s' from string to int (value: %d).",

$key,

Application::APP_NAME,

$intValue,

));

} else {

// For empty/non-numeric or non-positive values, treat as "not configured"

// and leave the key unset instead of persisting a sentinel 0 value.

$output->info(sprintf(

"Deleted legacy string config key '%s' for app '%s' due to non-positive or invalid value.",

$key,

Application::APP_NAME,

));

}

Copilot · 2026-03-31T20:45:38Z

lib/OcrProcessors/Remote/Client/ApiClient.php

-			if ($timeoutInt > 0) {
-				$timeout = $timeoutInt;
-			}
+		if ($settings->timeout !== null && $settings->timeout > 0) {


getTimeout() is typed as object but directly accesses $settings->timeout without an isset()/property_exists() guard. If anything other than GlobalSettings is ever passed, this will trigger an undefined-property warning. Consider either tightening the parameter type to GlobalSettings (preferred, since that’s what the caller passes) or reintroducing a guard before accessing the property.

Suggested change

if ($settings->timeout !== null && $settings->timeout > 0) {

if (isset($settings->timeout) && $settings->timeout > 0) {

Copilot · 2026-03-31T20:45:38Z

src/components/GlobalSettings.vue

 					<input
-						v-model="settings.processorCount"
+						:value="settings.processorCount"
 						name="processorCount"
 						type="number"
-						@input="save">
+						@input="settings.processorCount = Number($event.target.value) || null; save()">
 				</div>


The input coercion Number($event.target.value) || null only converts falsy values to null; negative numbers (e.g. “-1”) will be kept and sent to the backend, where they can be persisted even though the service treats <= 0 as unset. Consider explicitly normalizing with a > 0 check (and possibly clamping) so that any <= 0 (and NaN) becomes null.

Copilot · 2026-03-31T20:45:39Z

src/components/GlobalSettings.vue

 				<div class="div-table-col">
 					<input
-						v-model="settings.timeout"
+						:value="settings.timeout"
 						name="timeout"
 						type="number"
 						min="1"
-						@input="save">
+						@input="settings.timeout = Number($event.target.value) || null; save()">
 				</div>


Same as processorCount: Number($event.target.value) || null allows negative values to flow through. Even though min="1" exists, users can still type negatives manually and the handler will accept them. Consider normalizing in the handler to null unless the parsed number is > 0 to prevent persisting invalid timeouts.

Copilot · 2026-03-31T20:45:39Z

tests/Unit/Controller/GlobalSettingsControllerTest.php

+			}));
+
+		$this->controller->setGlobalSettings($settings);
+	}


The controller now casts values to int/null, but there’s no unit test covering the API behavior when the client submits an empty string (""), which is a common payload from cleared number inputs and is called out in the PR description. Adding a test case for "" → null (or "" → delete/unset) would protect the intended behavior.

Suggested change

}

}

public function testSetSettingsConvertsEmptyStringsToNull() {

$settings = [

'processorCount' => '',

'timeout' => '',

];

$this->globalSettingsService->expects($this->once())

->method('setGlobalSettings')

->with($this->callback(function (GlobalSettings $settings) {

return $settings->processorCount === null && $settings->timeout === null;

}));

$this->controller->setGlobalSettings($settings);

}

Initial plan

e65da91

Copilot AI assigned Copilot and R0Wi Mar 30, 2026

Copilot started work on behalf of R0Wi March 30, 2026 14:03 View session

Copilot AI linked an issue Mar 30, 2026 that may be closed by this pull request

Max CPU usage cannot be set, int is not valid, string expected #375

Open

fix: cast GlobalSettings values to string before calling setValueString

020c004

Agent-Logs-Url: https://github.com/R0Wi-DEV/workflow_ocr/sessions/e5743218-a210-416d-b782-64fc77d0fb16 Co-authored-by: R0Wi <19730957+R0Wi@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix max CPU usage setting to accept string instead of int~~ Fix TypeError when saving integer CPU core count in global settings Mar 30, 2026

Copilot finished work on behalf of R0Wi March 30, 2026 14:06

Copilot AI requested a review from R0Wi March 30, 2026 14:06

Copilot started work on behalf of R0Wi March 30, 2026 19:57 View session

refactor: use setValueInt/getValueInt for GlobalSettings integer prop…

40cd602

…erties Agent-Logs-Url: https://github.com/R0Wi-DEV/workflow_ocr/sessions/ca3e8666-830a-42ce-9ae2-c2dbd88e3dc9 Co-authored-by: R0Wi <19730957+R0Wi@users.noreply.github.com>

Copilot finished work on behalf of R0Wi March 30, 2026 20:06

fix: Add DB migration for processorCount and timeout

2f65fec

R0Wi force-pushed the copilot/fix-max-cpu-usage-setting branch from 13d0e3f to 2f65fec Compare March 30, 2026 20:45

R0Wi added 2 commits March 31, 2026 20:37

refactor: Update GlobalSettings handling to convert empty inputs to n…

5da3670

…ull and improve type management

Exclude migrations from PSALM

983b1b1

R0Wi requested a review from Copilot March 31, 2026 20:42

Copilot started reviewing on behalf of R0Wi March 31, 2026 20:42 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix TypeError when saving integer CPU core count in global settings#376

Fix TypeError when saving integer CPU core count in global settings#376
Copilot wants to merge 6 commits intomasterfrom
copilot/fix-max-cpu-usage-setting

Copilot AI commented Mar 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-				$this->config->setValueInt(Application::APP_NAME, $key, (int)$value);
+				$intValue = (int)$value;
+				if ($intValue <= 0) {
+					// Keep storage consistent with read-path semantics: non-positive ints
+					// are treated as "not configured", so we delete the key instead of
+					// persisting an invalid/sentinel value.
+					$this->config->deleteKey(Application::APP_NAME, $key);
+				} else {
+					$this->config->setValueInt(Application::APP_NAME, $key, $intValue);
+				}

	if ($settings->timeout !== null && $settings->timeout > 0) {
	if (isset($settings->timeout) && $settings->timeout > 0) {

-	}
+	}
+	public function testSetSettingsConvertsEmptyStringsToNull() {
+		$settings = [
+			'processorCount' => '',
+			'timeout' => '',
+		];
+		$this->globalSettingsService->expects($this->once())
+			->method('setGlobalSettings')
+			->with($this->callback(function (GlobalSettings $settings) {
+				return $settings->processorCount === null && $settings->timeout === null;
+			}));
+		$this->controller->setGlobalSettings($settings);
+	}

Conversation

Copilot AI commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Mar 30, 2026 •

edited

Loading