Issue #3618870: Move the hold path into an engine of its own, so BookingManager is the contract

BookingManager is 208 lines, from 2,060. It is the fifteen methods of BookingManagerInterface and nothing else: four delegate to HoldEngine, four to BookingAvailability, and seven are the lifecycle transitions. Its constructor is 4 arguments, from 24, because what it no longer does it no longer needs.

HoldEngine is everything reachable from hold(), holdRequests() and holdInTransaction(): 33 methods, plus the three properties they keep their state in.

It moved as one unit, and that is the safety argument

Nothing inside the cluster moved relative to anything else, so the order statements happen in under the locks is exactly what it was. Every one of the 33 methods is byte-identical to its previous text, which I checked rather than assumed: the only two edited are lockedMutate() and assertNotInCheckout(), whose visibility changed from private to public because five lifecycle transitions and cancel() call them. Those two are public for that reason and no other.

The check also covered what a scripted move loses quietly: all 45 methods of the previous file exist in exactly one of the two now (16 and 34, with the four delegations in both), no constant or property was dropped, and every one of the interface's fifteen methods is still on the manager. That check earned itself twice here, catching a class constant left behind and a constructor docblock whose 24 @param tags had been silently dropped.

What this does not do

It does not make the engine smaller. HoldEngine is 2,047 lines, more than the 1,696 the methods measure because each brought its docblock. The mass moved; it did not shrink. What changed is that the contract is now readable on its own, and everything that writes a booking under locks is in one file beside the class that owns everything that reads one.

hold() is still 429 lines. Breaking it up reorders operations under locks and wants its own issue and its own load-rig evidence.

Two things fixed on the way

The engine's four public entry points had {@inheritdoc}, inherited from an interface HoldEngine does not implement, so they carried no documented types at all. They now have the contract's own docblocks, and holdInTransaction() declares what it writes back into $notices, which a caller passing the list on has to know. phpstan found this, as a by-ref type error on the delegation.

reportUnmatchedIgnoredErrors is true, which is phpstan's default and was being switched off here. The single ignore this project has already opts out for itself with reportUnmatched: false, so the setting silenced nothing that exists: only the stale ignores nobody has written yet. It is stated explicitly rather than left to the default, for the same reason the level is, so a contributor reading the file does not have to know what the default is. Confirmed by adding a pattern that matches nothing and watching phpstan report it, then removing it again.

Testing

phpstan level 3 [OK], phpcs Drupal,DrupalPractice over 1,215 files clean, cspell adding no new word. The whole unit suite, 2,477 tests, which is where the last two of these moves broke: reflection and source-scanning tests name methods by string, and the kernel runner never sees them. Plus 24 kernel classes weighted to the hold path, including every query budget and cost test.