fix: two silent setters, and the warnings that were pointing at them

lib/Controller/SettingsController.php

@@ -665,7 +665,11 @@ private function getHybridSearchDiagnostics(bool $isPostgres): array {
 				$row = $result->fetch();
 				$result->closeCursor();
 
-				if ($row !== false) {
+				// An is_array check, not `!== false`. fetch() can answer NULL
+				// as well as false, which passes a `!== false` guard and then
+				// warns on every offset read. Lines 476 and 644 of this file
+				// already use this idiom; this was the one site that did not.
+				if (is_array($row) === true) {
 					$diagnostics['vectors']['total'] = (int)$row['total'];
 					$diagnostics['vectors']['pgvectorPopulated'] = (int)$row['populated'];
 				}
@@ -876,8 +880,11 @@ function (\OCA\OpenRegister\Db\ObjectEntity $org): array {
 			// (see RegisterMapper::getAllRegisterIdsWithSchema / MarkerLookupTrait).
 			$stmt = $qb->executeQuery();
 			$rows = [];
+			// Same reason as above, and here it matters more: a fetch() that
+			// answers NULL never satisfies `!== false`, so the loop would spin
+			// forever appending nulls rather than ending.
 			$row = $stmt->fetch();
-			while ($row !== false) {
+			while (is_array($row) === true) {
 				$rows[] = $row;
 				$row = $stmt->fetch();
 			}

lib/Db/Organisation.php

@@ -751,7 +751,11 @@ public function setActive(mixed $active): static {
 			$activeValue = (bool)$active;
 		}
 
-		parent::setActive(active: $activeValue);
+		// Assign DIRECTLY, as Application::setActive() does. This called
+		// parent::setActive(active: ...) — a NAMED arg into Entity::__call(),
+		// which reads $args[0], so the value never arrived and setActive(false)
+		// left the organisation ACTIVE. Nothing could be deactivated.
+		$this->active = $activeValue;
 
 		$this->markFieldUpdated(attribute: 'active');
 		return $this;

lib/Formats/BsnFormat.php

@@ -23,7 +23,13 @@
 namespace OCA\OpenRegister\Formats;
 
 use Opis\JsonSchema\Format;
+use TypeError;
 
+/**
+ * Validates the Dutch BSN (Burgerservicenummer) as a JSON Schema format.
+ *
+ * @spec openspec/specs/data-import-export/spec.md
+ */
 class BsnFormat implements Format {
 	/**
 	 * Validates if a given value conforms to the Dutch BSN (Burgerservicenummer) format.
@@ -37,10 +43,27 @@ class BsnFormat implements Format {
 	 * @spec openspec/specs/data-import-export/spec.md
 	 */
 	public function validate(mixed $data): bool {
+		// An array or object is a caller bug, not an invalid BSN, so it is
+		// refused loudly. str_pad() used to raise the TypeError itself, further
+		// down and by accident; raising it here keeps that contract explicit
+		// once the cast below stops the value ever reaching str_pad() untyped.
+		if (is_array($data) === true || is_object($data) === true) {
+			throw new TypeError(
+				'BsnFormat::validate() expects a scalar or null, '.get_debug_type($data).' given'
+			);
+		}
+
+		// Cast ONCE, here. null and false coerce to '' and pad to the all-zero
+		// sentinel, which is rejected below — that is the documented behaviour
+		// (ADR-008 Rule 4). Passing null on to str_pad() instead is deprecated
+		// in PHP 8.1 and a TypeError in PHP 9, so the same input would stop
+		// being 'not a BSN' and start being a fatal.
+		$data = (string)$data;
+
 		// Reject over-length input before padding: str_pad only left-pads and
 		// never truncates, so a >9-digit value would otherwise be checksummed
 		// on a miscalculated weighting (ADR-008 Rule 4).
-		if (strlen((string)$data) > 9) {
+		if (strlen($data) > 9) {
 			return false;
 		}
 

lib/Service/EndpointService.php

@@ -492,8 +492,13 @@ private function logEndpointCall(Endpoint $endpoint, array $request, array $resu
 
 			// Set request/response data.
 			$log->setRequest($request);
+			// POSITIONAL, deliberately. setResponse() is magic — EndpointLog
+			// declares the property, not the method — so it goes through
+			// Entity::__call(array $args), where a NAMED argument lands under
+			// its name and $args[0] is never set. Every endpoint log was
+			// storing a null response.
 			$log->setResponse(
-				response: [
+				[
 					'statusCode' => $result['statusCode'],
 					'body' => $result['response'],
 				]

lib/Service/Object/SaveObject.php

@@ -516,6 +516,14 @@ private function resolveSchemaReference(string $reference): ?string {
 			$schemas = $this->schemaMapper->findAll();
 			// Cache all schemas by slug for future lookups.
 			foreach ($schemas as $schema) {
+				// A schema with no slug cannot be found BY slug, and feeding
+				// the null on to strtolower()/strcasecmp() is deprecated in
+				// PHP 8.1 and a TypeError in PHP 9. Skipping it is what the
+				// loop was already doing in effect, just noisily.
+				if ($schema->getSlug() === null) {
+					continue;
+				}
+
 				$schemaSlug = strtolower($schema->getSlug());
 				$schemaId = (string)$schema->getId();
 				// Cache the schema entity.

tests/Unit/Command/RechainAuditTrailCommandTest.php

@@ -147,10 +147,21 @@ public function testForceRepairsAndReportsSuccess(): void {
 		$this->hashes->method('verifyChain')
 			->willReturnOnConsecutiveCalls($this->verification(false), $this->verification(true));
 		$this->hashes->method('countUnsealed')->willReturn(0);
-		$this->hashes->expects($this->once())->method('rechainAll')->willReturn(['rechained' => 313136]);
+		// The FULL shape rechainAll() returns. Both of its return paths fill
+		// 'tombstonesPreserved', and the command prints it, so a mock that
+		// omits it makes the command read an undefined key and the test
+		// exercises a state production cannot reach.
+		$this->hashes->expects($this->once())->method('rechainAll')
+			->willReturn(['rechained' => 313136, 'tombstonesPreserved' => 12]);
 
 		$this->assertSame(Command::SUCCESS, $this->tester->execute(['--force' => true]));
 		$this->assertStringContainsString('313136', $this->tester->getDisplay());
+		// The second half of the same sentence. An operator reads this line to
+		// decide whether the repair touched what they expected, and a repair
+		// that silently skipped rows would report the tombstone count wrong.
+		// Nothing asserted it before, so the placeholder could have printed
+		// anything.
+		$this->assertStringContainsString('12 retention tombstone', $this->tester->getDisplay());
 
 	}//end testForceRepairsAndReportsSuccess()
 
@@ -168,7 +179,7 @@ public function testForceRepairsAndReportsSuccess(): void {
 	public function testStillBrokenAfterwardsIsAFailure(): void {
 		$this->hashes->method('verifyChain')->willReturn($this->verification(false));
 		$this->hashes->method('countUnsealed')->willReturn(3);
-		$this->hashes->method('rechainAll')->willReturn(['rechained' => 5]);
+		$this->hashes->method('rechainAll')->willReturn(['rechained' => 5, 'tombstonesPreserved' => 0]);
 
 		$this->assertSame(Command::FAILURE, $this->tester->execute(['--force' => true]));
 		$this->assertStringContainsString('do not treat this repair as complete', $this->tester->getDisplay());

tests/Unit/Db/OrganisationTest.php

@@ -336,11 +336,14 @@ public function testIsActiveDefaultTrue(): void {
 	}
 
 	public function testSetActiveFalse(): void {
-		// Organisation::setActive() calls parent::setActive(active: $val) with named args,
-		// which triggers the Entity __call named-arg bug. The value is always truthy.
-		// This test documents the current actual behavior.
+		// An organisation CAN be deactivated. This test used to assert the
+		// opposite and called it "the current actual behavior": setActive()
+		// passed a named argument to Entity::__call(), which reads $args[0],
+		// so the value never arrived and every organisation stayed active.
+		// The OrganisationController endpoint for deactivating one could not
+		// work, and this test is what made that look intended.
 		$result = $this->organisation->setActive(false);
-		$this->assertTrue($this->organisation->isActive());
+		$this->assertFalse($this->organisation->isActive(), 'setActive(false) must deactivate');
 		$this->assertSame($this->organisation, $result);
 	}
 
@@ -365,10 +368,11 @@ public function testSetActiveTruthyString(): void {
 	}
 
 	public function testSetActiveFalsyStringZero(): void {
-		// Due to the named-arg bug in parent::setActive(), '0' is cast to false
-		// but the named arg causes it to be set as truthy string 'active'.
+		// '0' is a falsy string, and the setter casts it. The API sends
+		// strings, so this is the path a deactivation actually arrives on —
+		// which is why it mattered that the value was being dropped.
 		$this->organisation->setActive('0');
-		$this->assertTrue($this->organisation->isActive());
+		$this->assertFalse($this->organisation->isActive(), "'0' must deactivate");
 	}
 
 	public function testIsActiveWhenInternallyNull(): void {

tests/Unit/Mcp/BuiltIn/FlowMcpToolProviderTest.php

@@ -25,6 +25,12 @@ class FlowMcpToolProviderTest extends TestCase {
 	private FlowRunMapper $mapper;
 	private IUserSession&MockObject $userSession;
 	private FlowMcpToolProvider $provider;
+	// Declared, not created on the fly. Assigning an undeclared property in
+	// setUp() is deprecated in PHP 8.2 and an error in PHP 9, and it also
+	// costs the type: an undeclared $flows is mixed, so nothing checks that
+	// the provider is handed a FlowService at all.
+	private \OCA\OpenRegister\Service\Flow\FlowService&MockObject $flows;
+	private \OCA\OpenRegister\Service\Flow\FlowNodePreflight&MockObject $preflight;
 
 	protected function setUp(): void {
 		$this->runner = $this->createMock(FlowRunService::class);

tests/Unit/Service/ActiveOrganisationManagementTest.php

@@ -874,10 +874,13 @@ public function testGetOrganisationForNewEntityFallbackToDefault(): void {
 		$reflection = new \ReflectionClass(OrganisationService::class);
 		$cacheProperty = $reflection->getProperty('defaultOrgCache');
 		$cacheProperty->setAccessible(true);
-		$cacheProperty->setValue($defaultOrg);
+		// Both are STATIC, so the object argument is null. Omitting it is
+		// deprecated in PHP 8.3 and removed in PHP 9; the same file already
+		// writes these two caches the explicit way further up.
+		$cacheProperty->setValue(null, $defaultOrg);
 		$tsProperty = $reflection->getProperty('defaultOrgCacheTs');
 		$tsProperty->setAccessible(true);
-		$tsProperty->setValue(time());
+		$tsProperty->setValue(null, time());
 
 		// Act.
 		$result = $this->organisationService->getOrganisationForNewEntity();

tests/Unit/Service/DefaultOrganisationCachingTest.php

@@ -158,11 +158,11 @@ private function clearStaticCache(): void {
 
 		$cacheProperty = $reflection->getProperty('defaultOrgCache');
 		$cacheProperty->setAccessible(true);
-		$cacheProperty->setValue(null);
+		$cacheProperty->setValue(null, null);
 
 		$timestampProperty = $reflection->getProperty('defaultOrgCacheTs');
 		$timestampProperty->setAccessible(true);
-		$timestampProperty->setValue(null);
+		$timestampProperty->setValue(null, null);
 	}
 
 	/**
@@ -183,11 +183,11 @@ public function testDefaultOrganisationStaticCacheHit(): void {
 
 		$cacheProperty = $reflection->getProperty('defaultOrgCache');
 		$cacheProperty->setAccessible(true);
-		$cacheProperty->setValue($defaultOrg);
+		$cacheProperty->setValue(null, $defaultOrg);
 
 		$timestampProperty = $reflection->getProperty('defaultOrgCacheTs');
 		$timestampProperty->setAccessible(true);
-		$timestampProperty->setValue(time());
+		$timestampProperty->setValue(null, time());
 
 		// The mapper should NOT be called since cache is populated.
 		$this->organisationMapper
@@ -219,12 +219,12 @@ public function testDefaultOrganisationCacheExpiration(): void {
 
 		$cacheProperty = $reflection->getProperty('defaultOrgCache');
 		$cacheProperty->setAccessible(true);
-		$cacheProperty->setValue($defaultOrg);
+		$cacheProperty->setValue(null, $defaultOrg);
 
 		// Set expired timestamp (older than cache timeout).
 		$timestampProperty = $reflection->getProperty('defaultOrgCacheTs');
 		$timestampProperty->setAccessible(true);
-		$timestampProperty->setValue(time() - 1000);
+		$timestampProperty->setValue(null, time() - 1000);
 
 		// Assert: Cache was set with expired timestamp.
 		$this->assertNotNull($cacheProperty->getValue());
@@ -247,11 +247,11 @@ public function testDefaultOrganisationCacheSharedAcrossInstances(): void {
 
 		$cacheProperty = $reflection->getProperty('defaultOrgCache');
 		$cacheProperty->setAccessible(true);
-		$cacheProperty->setValue($defaultOrg);
+		$cacheProperty->setValue(null, $defaultOrg);
 
 		$timestampProperty = $reflection->getProperty('defaultOrgCacheTs');
 		$timestampProperty->setAccessible(true);
-		$timestampProperty->setValue(time());
+		$timestampProperty->setValue(null, time());
 
 		// Create a second service instance.
 		$organisationService2 = new OrganisationService(
@@ -290,11 +290,11 @@ public function testDefaultOrganisationCacheInvalidationOnModification(): void {
 
 		$cacheProperty = $reflection->getProperty('defaultOrgCache');
 		$cacheProperty->setAccessible(true);
-		$cacheProperty->setValue($defaultOrg);
+		$cacheProperty->setValue(null, $defaultOrg);
 
 		$timestampProperty = $reflection->getProperty('defaultOrgCacheTs');
 		$timestampProperty->setAccessible(true);
-		$timestampProperty->setValue(time());
+		$timestampProperty->setValue(null, time());
 
 		// Verify cache is populated.
 		$this->assertNotNull($cacheProperty->getValue());
@@ -332,11 +332,11 @@ public function testDefaultOrganisationPerformanceOptimization(): void {
 
 		$cacheProperty = $reflection->getProperty('defaultOrgCache');
 		$cacheProperty->setAccessible(true);
-		$cacheProperty->setValue($defaultOrg);
+		$cacheProperty->setValue(null, $defaultOrg);
 
 		$timestampProperty = $reflection->getProperty('defaultOrgCacheTs');
 		$timestampProperty->setAccessible(true);
-		$timestampProperty->setValue(time());
+		$timestampProperty->setValue(null, time());
 
 		// The mapper should NOT be called since cache is populated.
 		$this->organisationMapper

tests/Unit/Service/DefaultOrganisationManagementTest.php

@@ -478,10 +478,10 @@ public function testGetUserOrganisationsAutoAssignsToDefault(): void {
 
 		$cacheProperty = $reflection->getProperty('defaultOrgCache');
 		$cacheProperty->setAccessible(true);
-		$cacheProperty->setValue($defaultOrg);
+		$cacheProperty->setValue(null, $defaultOrg);
 		$tsProperty = $reflection->getProperty('defaultOrgCacheTs');
 		$tsProperty->setAccessible(true);
-		$tsProperty->setValue(time());
+		$tsProperty->setValue(null, time());
 
 		// Mock: update called to save user addition.
 		$this->organisationMapper

tests/Unit/Service/EndpointServiceTest.php

@@ -1102,8 +1102,16 @@ public function testLogEndpointCallVerifiesLogProperties(): void {
 				$this->assertSame('some warning', $log->getStatusMessage());
 				// Verify request data.
 				$this->assertSame(['method' => 'GET', 'path' => '/api/test', 'data' => ['key' => 'val'], 'headers' => ['X-Foo' => 'bar']], $log->getRequest());
-				// Note: setResponse uses named arg in source code (known issue),
-				// so response may be null. We verify it was attempted.
+				// The response, asserted rather than excused. This used to read
+				// "setResponse uses named arg in source code (known issue), so
+				// response may be null. We verify it was attempted" — and it
+				// verified nothing, so every endpoint log stored a null
+				// response and the suite stayed green.
+				$this->assertSame(
+					['statusCode' => 200, 'body' => ['items' => [1, 2, 3]]],
+					$log->getResponse(),
+					'the response must be stored on the log, not dropped'
+				);
 				// Verify timestamps.
 				$this->assertInstanceOf(\DateTime::class, $log->getCreated());
 				$this->assertInstanceOf(\DateTime::class, $log->getExpires());

tests/Unit/Service/ObjectServiceTest.php

@@ -1870,8 +1870,14 @@ public function testCreateObjectCallsSaveObjectInternally(): void {
 		$this->setProperty('currentSchema', $this->schema);
 
 		// The cascading handler is called before save — verify delegation starts.
+		// It is made to return its documented shape, [object, uuid]. An
+		// unconfigured mock answers null, which ObjectService then reads
+		// offset 0 of: the real handler is `: array` on every return path, so
+		// that was the fake disagreeing with the contract, not a gap in the
+		// production guard.
 		$this->cascadingHandler->expects($this->once())
-			->method('handlePreValidationCascading');
+			->method('handlePreValidationCascading')
+			->willReturn([['title' => 'New'], null]);
 
 		// The actual save will fail due to deep dependencies, but we verify
 		// the method delegates to saveObject() correctly.