refactor(bundle): DurableExtension's registrations move into @internal loaders (#342)

[gplanchat]

Refs #342 (finding M20, M12 residue). C0–C13 of the plan on the issue are in. C14 (the public ids) waits for the user's decision.

What is here

• C0: a container snapshot, before any move.
  • DurableContainerSnapshotTest dumps every definition, alias and parameter the extension registers into Fixtures/container-snapshot.txt.
  • It uses twelve variants: in_memory, dbal+messenger, temporal+guzzle, dbal+Temporal DSN over PSR-18, the legacy mixed path, and application-preset aliases, each with the profiler on and off.
  • It is never skipped. DURABLE_UPDATE_SNAPSHOT=1 rewrites the fixture and still asserts.
• Every registration moves out of DurableExtension into @internal classes under DependencyInjection/Loader/.
  • CoreServices, MessengerServices, EventStores, DbalStores, Observability, Commands.
  • The fixture is untouched by every one of these commits, and each commit is green on its own.
  • DurableExtension.php goes from 979 to 118 lines, and load() is 40.
• C5 (694a136): dbal without a Temporal DSN no longer registers durable.event_store.inner.
  • The snapshot diff is one line: the id leaves variants D and L.
  • It stays on in_memory and wherever a Temporal DSN is set, since both read it.
  • UPGRADE has a line for it.
• C11 (515f17c): on Temporal, WorkflowMetadataStore is an alias, as on the other backends.
  • The in-memory store is registered as durable.workflow_metadata_store.inner from the start.
  • The in-memory and DBAL paths no longer rename a definition to put their decorator in front.
• The public-ids test covers every backend (5a1f4e8). DurableContainerSurfaceTest::testThePublicSurfaceStaysReachable runs in_memory, the legacy DBAL keys, backend: dbal and temporal against the three ids. Cases were only added. Before C11, the temporal metadata row fails.
• C12–C13: definitions move to durable.* ids (af10746, 9d08094, 9562dab, 16cf789).
  • Each definition goes private. Its class or interface id stays, as an alias with the visibility it had.
  • WorkflowTaskProcessor keeps its class id, because DurableContainerSurfaceTest pins it as a private definition.
  • UPGRADE (fc91654) tells compiler passes to use findDefinition() and has().

Deviation from the issue's "Fix"

The loaders are @internal classes, not config files; a config-file form can follow if wanted. Verbatim closure files shared one scope, so their local variables collided. The classes keep each method byte-for-byte, and the snapshot proves the container is unchanged.

Two commits are extractions rather than verbatim moves, to stay under the 200-line commit limit. Both bodies are unchanged, and the snapshot stays byte-identical.

• 0c5aa4c: the Temporal branch of registerEventStore().
• 13f5905: the activity processor, out of registerCommands().

Tests edited

Each id below became an alias, and each edit keeps the same assertion.

• DurableDeclaredWiringTest: getDefinition → findDefinition (ActivityContractResolver, 2 lines).
• DurableProfilerWiringTest: getDefinition → findDefinition (WorkflowResumeDispatcher, 2 lines).
• SetupCommandTest:34: getDefinition → findDefinition.
• SetupCommandTest:33: hasDefinition → has. On an alias, hasDefinition() is always false, so keeping it would have made the assertion vacuous.
• DurableWorkerCommandWiringTest: getDefinition → findDefinition (5 lines).
• DurableDiagnoseCommandWiringTest:47, DurableRunCatalogWiringTest:87, DurableTemporalAssemblyWiringTest:44: getDefinition → findDefinition.
• DurableContainerSurfaceTest: cases added (see above).

C14 (the public ids) waits for the user's decision and is not part of this PR.

Possible follow-up, outside #342: on dbal with a Temporal DSN (variant N), durable.event_store.temporal has no reader, because EventStoreInterface points at DBAL. The compiler drops it as an unused private service.

Done when

• DurableExtension::load() under 150 lines (40); DurableContainerSurfaceTest pins the public ids for the three backends.
• Rector/UPGRADE.md for any removed public id. No public id is removed here, so this box waits for C14.

Reviewer: antoine, per commit. Check that the fixture is untouched by each move commit and that the snapshot is green at each one.

[gplanchat]

Incremental review (antoine), C0–C4, through 7bb18d4. Verdict: OK, no finding.

C0 (snapshot): OK after d5d65a3. The dump now keeps the aliases an application set before load(). Overwriting the app's observer alias, or registering the default redactor over the app's alias, both fail the snapshot.

Per-commit check. I checked out each of the 8 commits alone and ran DurableContainerSnapshotTest there:

Commit	Lines	Fixture	Snapshot
32dfbdf	179	untouched	OK
5bfb590	138	untouched	OK
ba576aa	122	untouched	OK
af02c26	91	untouched	OK
bd90e03	182	untouched	OK
85621b8	46	untouched	OK
0c5aa4c	173	untouched	OK
7bb18d4	46	untouched	OK

Verbatim. For each commit I listed the added lines that do not appear among its removed lines, whitespace ignored. The only ones left are new method signatures, their call sites, and self::isTemporalNative() becoming DurableExtension::isTemporalNative(). Every moved body is unchanged.

0c5aa4c, the one extraction. registerTemporalEventStore() uses only its parameters ($container, $temporalConfig, $dsn, $journal) and locals it defines itself ($fromAssembly, whose closure parameters are $id, $class, $method, $public and $arguments, plus $client, $guzzleClient and $psr18), so no outer variable is lost. The early return; stays in the caller, right after the call. The T and N variants (Temporal, with guzzle_client / psr18_client) run through it and stay byte-identical.

At the head: tests/unit/DurableBundle 211 OK; PHPStan, Psalm and cs:check 0. DurableExtension.php is 564 lines so far; load() under 150 lines is the Done-when still to come.

[gplanchat]

Incremental review (antoine), the push after 7bb18d4, through 694a136. Verdict: OK on the code, with one small "Done when" gap to close.

Per-commit check. I checked out each commit alone and ran DurableContainerSnapshotTest there. It is 8 commits, not 9: the eight you listed.

Commit	Lines	Fixture	Snapshot
7202ad0	130	untouched	OK
b2ce084	193	untouched	OK
9b47d3c	121	untouched	OK
f944589	139	untouched	OK
49706b4	149	untouched	OK
13f5905	96	untouched	OK
65b00e6	169	untouched	OK
694a136	21	changed (intended)	OK

Verbatim. In the seven moves, the only added lines that do not appear among the removed lines (whitespace ignored) are method signatures, call sites and self:: becoming DurableExtension:: calls.

13f5905, the extraction. registerActivityProcessor() uses $container, $config, $isTemporalNative (its parameters) and two locals it assigns itself ($activityTransportConfig, $activityTransportName). The rest of registerCommands() uses $activityTransport, a separate variable computed on its own, so nothing the caller needed was taken away. It is called in place, and the snapshot is identical.

C5, 694a136. The fixture diff is exactly the one line (IDTNLAidtnla becoming I.TN.Ai.tn.a on def durable.event_store.inner), and UPGRADE.md covers it. I looked for a reader on D or L:

• In the tree, every reference to the id is on a path that keeps it: the Temporal read-through (EventStores.php:134, variants T and N) and the in-memory catalog and projecting journal (:193, :201, variants I and A). The reference in DurableRunCatalogWiringTest is the in-memory case.
• In the fixture, every line referencing @durable.event_store.inner carries I....Ai....a or ..TN....tn..; none carries D, L, d or l.
  I found no reader to miss.

"Done when" gap, small: DurableContainerSurfaceTest pins the public ids for one backend, not three

testThePublicSurfaceStaysReachable() loads a single configuration: the legacy event_store: dbal, workflow_metadata: dbal. The box says "pins the public ids for the three backends".

The snapshot does record that the three are public on every variant, with one detail worth pinning. On Temporal, WorkflowMetadataStore is a public definition, not an alias. The fixture has no alias line for it on T and t.

Fix: make that data provider backend × id: in_memory, dbal and temporal (DSN), crossed with the three ids. Assert hasAlias() || hasDefinition() and that the resolved service is public, so the Temporal metadata case passes as a definition.

At the head: DurableExtension.php is 118 lines and load() is 40. tests/unit/DurableBundle 211 OK, the full suite 2,321 OK; PHPStan, Psalm and cs:check 0.

[gplanchat]

Review (antoine), 694a136 → fc91654, C11–C13 and the Done-when fix. Verdict: OK, no finding.

No visibility flip. I parsed the fixture before (694a136) and after (fc91654) per variant and per id:

• Public to private or vanished: 0 of the 228 public (variant, id) pairs. Every class id that was public is still public, now as an alias.
• Private to public: 0.
• New ids: 32, all durable.*, all private. There are 35 def-to-alias conversions. C11's Temporal WorkflowMetadataStore, among others, aliases an id that already existed.
• Every alias resolves: all 56 aliases have a target defined in the same variant, with no dangling durable target.

Nothing reaches a renamed id by definition outside the loaders. I read every getDefinition / hasDefinition in src/:

• ActivityHandlerPass resolves an alias before its hasDefinition() (ActivityExecutor -> durable.activity_executor, a definition).
• RequireLockFactoryPass, NexusHandlerPass and TemporalReceiversPass use durable.* ids, which were not renamed.
• WorkflowPass walks tagged ids.
• The plugin test's RunDashboard belongs to the plugin extension.

Test edits, not weakenings.

• getDefinition becomes findDefinition in 7 files. It resolves the alias, and the same assertion then runs on the same definition.
• SetupCommandTest:33, hasDefinition becomes has. It is stronger: hasDefinition() on what became an alias would always be false, and the assertFalse would have gone vacuous.

The Done-when gap is closed (5a1f4e8). The public ids are pinned on 4 configs × 3 ids. With C11's WorkflowMetadataStore alias on Temporal, no "definition or alias" allowance is needed.

Head: full PHPUnit 2,330 OK (109 skips); PHPStan, Psalm and cs:check 0. From my side, #342's Done-when is met: load() 40 lines, the public ids pinned on every backend, and UPGRADE entries for the ids that changed.