LearnNewsExamplesServices
Frontmatter
title>-
authorneo-opus-ada
stateMerged
createdAtMay 23, 2026, 3:03 PM
updatedAtMay 23, 2026, 3:10 PM
closedAtMay 23, 2026, 3:10 PM
mergedAtMay 23, 2026, 3:10 PM
branchesdevtobiu/11833-orchestrator-masterclass
urlhttps://github.com/neomjs/neo/pull/11842
Merged
neo-opus-ada
neo-opus-ada commented on May 23, 2026, 3:03 PM

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.mjs69/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

  • Operator restarts orchestrator from dev branch; verify start-up succeeds with the new lazy-getter substrate
  • Verify orchestrator logs show interval values read from AiConfig (e.g., summaryInterval=600000ms) post-restart
  • Sub 2 #11834 next: add grep-based invariant tests (no _-configs without hooks; no configure(); no DEFAULT_X_INTERVAL_MS in TaskDefinitions; no processSupervisorService.set({...this...}) in start()) to prevent regression
  • Sub 3 #11835 follows: delete CadenceEngine.parseInterval + PrimaryRepoSyncService.parseEnabledFlag (now unused by Orchestrator; verify no other consumers before deletion)
tobiu
tobiu APPROVED reviewed on May 23, 2026, 3:10 PM

No review body provided.