Authored by Claude Opus 4.7 (Claude Code). Session 5572d9a5-558d-4bea-b416-e31496c289c4.
FAIR-band: in-band [verify @ merge-gate]
Resolves #11833
Summary
Sub 1 of Epic #11831. Refactors ai/daemons/Orchestrator.mjs from 1046 → 760 lines via the 4-way Service-DI classification + 2-value chain pattern graduated from Discussion #11828. Sub 6 #11832 (Neo.util.Env) is the substrate prerequisite (merged).
4-way Service-DI classification applied
- (A)
cadenceEngine_ — class-system-managed reactive collaborator with beforeSet + ClassSystemUtil.beforeSetInstance(value, CadenceEngine, {}). Polymorphic class/instance/config-object input.
- (B)
processSupervisorService_ — parent-configured child collaborator with beforeSet creation from current parent state + parent afterSet* propagation hooks for dataDir / taskDefinitions / taskStateService / healthService / spawnFn. Stable processSupervisorWriteLog seam (no per-call .bind() allocation). start() no longer replays parent config imperatively via processSupervisorService.set({...this...}).
- (C) Simple service singletons — Class C instance fields with direct import (
summarizationCoordinator, backupCoordinator, primaryRepoSyncService, dreamService, swarmHeartbeatService, goldenPathSynthesizer).
- (D) Operator policy values (8 intervals + 5 booleans) — lazy getters reading 2-value chain:
Env.parseNumber(process.env.X, 'X') ?? AiConfig.orchestrator.intervals.X for intervals; Env.parseBool(...) ?? resolveDeploymentEnabled(...) for deployment-aware booleans.
Substrate dependency removals
configure() shadow-resolver REMOVED entirely. Was duplicating initConfig work and shadowing AiConfig precedence.
singleton: true removed from CadenceEngine + ProcessSupervisorService. Per @tobiu: "if a class needs external configs, it should not be a singleton in the first place." Each Orchestrator instance now constructs its own collaborator instances via ClassSystemUtil.
- 8
DEFAULT_*_INTERVAL_MS constants removed from TaskDefinitions.mjs (canonical defaults live in ai/config.template.mjs:79-88 orchestrator.intervals).
DEFAULT_*_INTERVAL_MS re-exports removed from orchestrator-daemon.mjs (no consumers after this refactor; Sub 2's invariant tests will lock this in).
parseInterval / parseEnabledFlag call sites in Orchestrator REMOVED (replaced by Env.parseNumber / Env.parseBool from Sub 6's Neo.util.Env). The parseInterval / parseEnabledFlag method bodies themselves stay until Sub 3 #11835 removes them (other consumers may still use them).
Test refactor
createTestOrchestrator() helper migrated to AiConfig.data mutation + afterEach restore for Class D operator policy values (lazy getters not reachable via Neo.create initConfig). Simple service mocks assigned directly as instance fields after Neo.create.
- 3
configure()-based path/mlx tests migrated to instance-field-assignment + direct buildTaskDefinitions() calls (substrate-correct shape without side-effecting start()).
CadenceEngine.spec.mjs migrated to per-test instance via Neo.create(CadenceEngine) since CadenceEngine is no longer singleton.
orchestrator-daemon.spec.mjs stale DEFAULT_X_INTERVAL_MS imports dropped (were unused).
Deltas from ticket
- AC4 (B) shape clarification: Discussion body's Class B framing required both reactive collaborator AND
singleton: true accommodation. The empirical conflict (singleton namespace lookup returning instance, not class, breaks ClassSystemUtil.beforeSetInstance new cls()) forced removal of singleton: true from both CadenceEngine + ProcessSupervisorService. Operator confirmed this is substrate-correct: classes consuming external config should not be singletons. No deviation in spirit — same pattern, with singleton-incompatible classes corrected to non-singleton.
- Sub 2 #11834 partial absorption: Sub 1 ticket explicitly mentions "refactored
createTestOrchestrator()" — that's done here. Sub 2's broader invariant-test additions (grep-based source-level checks for removed anti-patterns) stay separate.
Evidence: L1 (69/69 unit tests pass) → L1 required for substrate refactor (substrate-shape change, no runtime contract change beyond config-injection path).
Test Evidence
npm run test-unit -- test/playwright/unit/ai/daemons/Orchestrator.spec.mjs test/playwright/unit/ai/daemons/services/ProcessSupervisorService.spec.mjs test/playwright/unit/ai/daemons/services/CadenceEngine.spec.mjs test/playwright/unit/ai/scripts/orchestrator-daemon.spec.mjs test/playwright/unit/ai/config.template.spec.mjs test/playwright/unit/util/Env.spec.mjs → 69/69 pass (1.6s)
Test coverage:
- Orchestrator.spec.mjs (26) — config precedence, heavy-maintenance backpressure, cross-daemon lease, golden-path dependency ordering, swarm-heartbeat lane, path resolution, mlx config flow
- ProcessSupervisorService.spec.mjs (13) — child-process supervision, task recovery, env injection
- CadenceEngine.spec.mjs (5) — parseInterval / shouldRunIntervalTask / runIfDue (per-instance pattern)
- orchestrator-daemon.spec.mjs (10) — task command building, local AI config loading
- config.template.spec.mjs (3) — Tier 1 Config Immutability lock-in
- Env.spec.mjs (20) — Sub 6 substrate primitive (already merged)
Post-Merge Validation
Authored by Claude Opus 4.7 (Claude Code). Session 5572d9a5-558d-4bea-b416-e31496c289c4.
FAIR-band: in-band [verify @ merge-gate]
Resolves #11833
Summary
Sub 1 of Epic #11831. Refactors
ai/daemons/Orchestrator.mjsfrom 1046 → 760 lines via the 4-way Service-DI classification + 2-value chain pattern graduated from Discussion #11828. Sub 6 #11832 (Neo.util.Env) is the substrate prerequisite (merged).4-way Service-DI classification applied
cadenceEngine_— class-system-managed reactive collaborator withbeforeSet+ClassSystemUtil.beforeSetInstance(value, CadenceEngine, {}). Polymorphic class/instance/config-object input.processSupervisorService_— parent-configured child collaborator withbeforeSetcreation from current parent state + parentafterSet*propagation hooks fordataDir/taskDefinitions/taskStateService/healthService/spawnFn. StableprocessSupervisorWriteLogseam (no per-call.bind()allocation).start()no longer replays parent config imperatively viaprocessSupervisorService.set({...this...}).summarizationCoordinator,backupCoordinator,primaryRepoSyncService,dreamService,swarmHeartbeatService,goldenPathSynthesizer).Env.parseNumber(process.env.X, 'X') ?? AiConfig.orchestrator.intervals.Xfor intervals;Env.parseBool(...) ?? resolveDeploymentEnabled(...)for deployment-aware booleans.Substrate dependency removals
configure()shadow-resolver REMOVED entirely. Was duplicating initConfig work and shadowing AiConfig precedence.singleton: trueremoved fromCadenceEngine+ProcessSupervisorService. Per @tobiu: "if a class needs external configs, it should not be a singleton in the first place." Each Orchestrator instance now constructs its own collaborator instances via ClassSystemUtil.DEFAULT_*_INTERVAL_MSconstants removed fromTaskDefinitions.mjs(canonical defaults live inai/config.template.mjs:79-88 orchestrator.intervals).DEFAULT_*_INTERVAL_MSre-exports removed fromorchestrator-daemon.mjs(no consumers after this refactor; Sub 2's invariant tests will lock this in).parseInterval/parseEnabledFlagcall sites in Orchestrator REMOVED (replaced byEnv.parseNumber/Env.parseBoolfrom Sub 6'sNeo.util.Env). TheparseInterval/parseEnabledFlagmethod bodies themselves stay until Sub 3 #11835 removes them (other consumers may still use them).Test refactor
createTestOrchestrator()helper migrated to AiConfig.data mutation +afterEachrestore for Class D operator policy values (lazy getters not reachable viaNeo.createinitConfig). Simple service mocks assigned directly as instance fields afterNeo.create.configure()-based path/mlx tests migrated to instance-field-assignment + directbuildTaskDefinitions()calls (substrate-correct shape without side-effectingstart()).CadenceEngine.spec.mjsmigrated to per-test instance viaNeo.create(CadenceEngine)since CadenceEngine is no longer singleton.orchestrator-daemon.spec.mjsstaleDEFAULT_X_INTERVAL_MSimports dropped (were unused).Deltas from ticket
singleton: trueaccommodation. The empirical conflict (singleton namespace lookup returning instance, not class, breaksClassSystemUtil.beforeSetInstancenew cls()) forced removal ofsingleton: truefrom bothCadenceEngine+ProcessSupervisorService. Operator confirmed this is substrate-correct: classes consuming external config should not be singletons. No deviation in spirit — same pattern, with singleton-incompatible classes corrected to non-singleton.createTestOrchestrator()" — that's done here. Sub 2's broader invariant-test additions (grep-based source-level checks for removed anti-patterns) stay separate.Evidence: L1 (69/69 unit tests pass) → L1 required for substrate refactor (substrate-shape change, no runtime contract change beyond config-injection path).
Test Evidence
npm run test-unit -- test/playwright/unit/ai/daemons/Orchestrator.spec.mjs test/playwright/unit/ai/daemons/services/ProcessSupervisorService.spec.mjs test/playwright/unit/ai/daemons/services/CadenceEngine.spec.mjs test/playwright/unit/ai/scripts/orchestrator-daemon.spec.mjs test/playwright/unit/ai/config.template.spec.mjs test/playwright/unit/util/Env.spec.mjs→ 69/69 pass (1.6s)Test coverage:
Post-Merge Validation
devbranch; verify start-up succeeds with the new lazy-getter substratesummaryInterval=600000ms) post-restart_-configs without hooks; noconfigure(); noDEFAULT_X_INTERVAL_MSin TaskDefinitions; noprocessSupervisorService.set({...this...})in start()) to prevent regressionCadenceEngine.parseInterval+PrimaryRepoSyncService.parseEnabledFlag(now unused by Orchestrator; verify no other consumers before deletion)