Authored by Claude Opus 4.7 (Claude Code). Session 5572d9a5-558d-4bea-b416-e31496c289c4.
FAIR-band: in-band [verify @ merge-gate]
Resolves #11854
Summary
Sub 13 of Epic #11831 — reverts Sub 1's (#11833) over-engineered Service-DI classification, replaces with the canonical reactive-config-with-default pattern already used in the same file for taskStateService_: TaskStateService and healthService_: HealthService.
Self-critique: I introduced the "4-way classification" framing in Sub 1 to make singleton-alias instance fields look like architecture. The classification was the problem — there are only 2 patterns (reactive-config-with-lifecycle and reactive-config-with-default), not 4. @tobiu caught it on look-back review; the technical debt I introduced in Sub 1 obscured the substrate the entire Epic was supposed to be refactoring.
Evidence: L1 (119/119 orchestrator subsuite + 2007 pass / 0 fail / 2 skip / 13 did-not-run on full unit) → L1 required for substrate refactor (no behavior change; consumer code identical post-rewrite).
Changes
1. Class C field deletion + reactive-config elevation. The 7 "Class C" instance-field aliases (summarizationCoordinator, backupCoordinator, primaryRepoSyncService, dreamService, swarmHeartbeatService, goldenPathSynthesizer, initializeDatabaseFn) deleted as standalone instance fields, re-declared as reactive configs in static config with the singleton import as default. Mirrors the existing taskStateService_: TaskStateService + healthService_: HealthService pattern.
Honest contract: tests inject mocks via Neo.create({collab: mock}) at construction OR instance.collab = mock post-construct — the reactive-config slot is the legitimate test seam (NOT the "Service-DI Class C" framing I invented in Sub 1, which dressed plain instance fields up as architecture without using Neo's class system).
2. Strip historical-context JSDoc. Class @summary previously enumerated a "Service-DI 4-way classification (#11833 / Epic #11831)" with bullet definitions of (A)/(B)/(C)/(D), followed by a "No configure() shadow-resolver. No DEFAULT_X_*_MS constants. No parseInterval..." negative-trailer describing what prior PRs removed. Both are bloat — they describe the design debate not the design intent; git log holds the history. Replaced with intent-driven prose: what the class does, per-task failure isolation, the 2 collaborators with lifecycle vs 7 with singleton defaults.
3. Delete 4 section divider comments. // === Service-DI Class A: ... === etc. removed. The classification framework itself was the anti-pattern; renaming "Class C → delegate field" would have preserved the same reduce-clarity surface.
4. Stale @see + comment cleanup. Removed @see #11833 (masterclass-reference refactor) (history-only). Fixed (\ai/scripts/orchestrator-daemon.mjs`)head comment to new post-Sub-8 pathai/daemons/orchestrator/daemon.mjs`.
5. Consumer code unchanged. All 9 this.<collaborator>.<method>(...) call-sites preserved — they now resolve through the reactive-config getter instead of the deleted plain field. No behavior change at the call surface.
Genuine reactive collaborators preserved
cadenceEngine_ — class reference; beforeSet performs polymorphic class/instance/config-object conversion + destroys prior instance on swapprocessSupervisorService_ — class reference; beforeSet constructs the child from parent-sourced config; parent afterSet* hooks propagate dataDir / taskDefinitions / taskStateService / healthService / spawnFn mutations downstream
These keep their beforeSet / afterSet machinery — those have real lifecycle and parent-propagation reasons. Documented under their reactive-config-with-lifecycle pattern explicitly in the new JSDoc.
Test Evidence
- Orchestrator subsuite (
test/playwright/unit/ai/daemons/orchestrator/): 119/119 pass (5.7s)
- Full unit suite: 2007 pass / 0 fail / 2 skip / 13 did-not-run
Post-Merge Validation
Deltas from ticket
- Implementation expanded beyond the original "delete the fields" prescription to "elevate them to reactive configs with singleton defaults." The original prescription was wrong — deleting the fields without a replacement test seam broke 13 tests that inject mocks via
this.<collab> = mock. The reactive-config-with-default pattern restores test mockability while removing the anti-pattern (plain instance field NOT using Neo's class system). Same logical outcome (no plain field aliases) with a substrate-correct shape.
Depends on
Epic #11831 Subs 8-12 (all merged).
Unblocks
Sub 14 #11855 (centralize env-binding registry) — Sub 13 lands first to avoid Orchestrator merge conflicts on Sub 14's 14-call-site env getter rewrite.
Per @tobiu's "you and me" instruction: no peer-review broadcast on this lane.
Authored by Claude Opus 4.7 (Claude Code). Session 5572d9a5-558d-4bea-b416-e31496c289c4.
FAIR-band: in-band [verify @ merge-gate]
Resolves #11854
Summary
Sub 13 of Epic #11831 — reverts Sub 1's (#11833) over-engineered Service-DI classification, replaces with the canonical reactive-config-with-default pattern already used in the same file for
taskStateService_: TaskStateServiceandhealthService_: HealthService.Self-critique: I introduced the "4-way classification" framing in Sub 1 to make singleton-alias instance fields look like architecture. The classification was the problem — there are only 2 patterns (reactive-config-with-lifecycle and reactive-config-with-default), not 4. @tobiu caught it on look-back review; the technical debt I introduced in Sub 1 obscured the substrate the entire Epic was supposed to be refactoring.
Evidence: L1 (119/119 orchestrator subsuite + 2007 pass / 0 fail / 2 skip / 13 did-not-run on full unit) → L1 required for substrate refactor (no behavior change; consumer code identical post-rewrite).
Changes
1. Class C field deletion + reactive-config elevation. The 7 "Class C" instance-field aliases (
summarizationCoordinator,backupCoordinator,primaryRepoSyncService,dreamService,swarmHeartbeatService,goldenPathSynthesizer,initializeDatabaseFn) deleted as standalone instance fields, re-declared as reactive configs instatic configwith the singleton import as default. Mirrors the existingtaskStateService_: TaskStateService+healthService_: HealthServicepattern.Honest contract: tests inject mocks via
Neo.create({collab: mock})at construction ORinstance.collab = mockpost-construct — the reactive-config slot is the legitimate test seam (NOT the "Service-DI Class C" framing I invented in Sub 1, which dressed plain instance fields up as architecture without using Neo's class system).2. Strip historical-context JSDoc. Class
@summarypreviously enumerated a "Service-DI 4-way classification (#11833 / Epic #11831)" with bullet definitions of (A)/(B)/(C)/(D), followed by a "Noconfigure()shadow-resolver. NoDEFAULT_X_*_MSconstants. NoparseInterval..." negative-trailer describing what prior PRs removed. Both are bloat — they describe the design debate not the design intent; git log holds the history. Replaced with intent-driven prose: what the class does, per-task failure isolation, the 2 collaborators with lifecycle vs 7 with singleton defaults.3. Delete 4 section divider comments.
// === Service-DI Class A: ... ===etc. removed. The classification framework itself was the anti-pattern; renaming "Class C → delegate field" would have preserved the same reduce-clarity surface.4. Stale
@see+ comment cleanup. Removed@see #11833 (masterclass-reference refactor)(history-only). Fixed(\ai/scripts/orchestrator-daemon.mjs`)head comment to new post-Sub-8 pathai/daemons/orchestrator/daemon.mjs`.5. Consumer code unchanged. All 9
this.<collaborator>.<method>(...)call-sites preserved — they now resolve through the reactive-config getter instead of the deleted plain field. No behavior change at the call surface.Genuine reactive collaborators preserved
cadenceEngine_— class reference;beforeSetperforms polymorphic class/instance/config-object conversion + destroys prior instance on swapprocessSupervisorService_— class reference;beforeSetconstructs the child from parent-sourced config; parentafterSet*hooks propagatedataDir/taskDefinitions/taskStateService/healthService/spawnFnmutations downstreamThese keep their
beforeSet/afterSetmachinery — those have real lifecycle and parent-propagation reasons. Documented under their reactive-config-with-lifecycle pattern explicitly in the new JSDoc.Test Evidence
test/playwright/unit/ai/daemons/orchestrator/): 119/119 pass (5.7s)Post-Merge Validation
Deltas from ticket
this.<collab> = mock. The reactive-config-with-default pattern restores test mockability while removing the anti-pattern (plain instance field NOT using Neo's class system). Same logical outcome (no plain field aliases) with a substrate-correct shape.Depends on
Epic #11831 Subs 8-12 (all merged).
Unblocks
Sub 14 #11855 (centralize env-binding registry) — Sub 13 lands first to avoid Orchestrator merge conflicts on Sub 14's 14-call-site env getter rewrite.
Per @tobiu's "you and me" instruction: no peer-review broadcast on this lane.