LearnNewsExamplesServices
Frontmatter
title>-
authorneo-opus-ada
stateMerged
createdAtMay 23, 2026, 11:42 PM
updatedAtMay 23, 2026, 11:54 PM
closedAtMay 23, 2026, 11:54 PM
mergedAtMay 23, 2026, 11:54 PM
branchesdevtobiu/11855-env-binding-registry
urlhttps://github.com/neomjs/neo/pull/11868
Merged
neo-opus-ada
neo-opus-ada commented on May 23, 2026, 11:42 PM

Authored by Claude Opus 4.7 (Claude Code). Session 5572d9a5-558d-4bea-b416-e31496c289c4.

FAIR-band: in-band [verify @ merge-gate]

Resolves #11855 (Sub 14 of Epic #11831)

Evidence: L1 (6/6 new env.spec + 2049/2049 full unit suite, 0 failures) → L1 required for binding-registry extraction (pure-function consumer-pattern collapse; env-registry shape is single eager-bound object). No residuals.

Summary

Single-source binding registry for all NEO_ORCHESTRATOR_* env vars consumed by Orchestrator.mjs. Replaces 13 inline Env.parseX(process.env.NAME, 'NAME') ?? AiConfig.X call-sites with env.NAME ?? AiConfig.X. Adds dotenv/config at the registry module load so local .env files auto-populate process.env.

Why: rename drift (change the env-var name in one spot, miss the matching string literal at the consumer); audit opacity (no single file enumerated which env vars the system reads); no .env file integration (dotenv was already a devDependency but consumers read process.env.X directly).

New files

  • ai/config/env.mjs (64 LOC) — bindings map (13 NEO_ORCHESTRATOR_* vars + parsers), buildEnv({bindings, source}) pure function (extracted for testability), default-export env object eagerly bound at module load after dotenv/config.
  • test/playwright/unit/ai/config/env.spec.mjs (77 LOC, 6 tests) — buildEnv pure-function coverage: absent var → undefined, valid parse, invalid parse + warn, ?? fallback chain, no-leakage from source env, mixed bool+number parsers.

Touched files

  • ai/daemons/orchestrator/Orchestrator.mjs (-77 +25 net) — Dropped import Env; added import env from '../../config/env.mjs'. 13 lazy getters collapsed from 4-line invocations to single-line env.NAME ?? AiConfig.X reads. JSDoc Class-D description updated to reflect the new pattern. Line 416 init-failure path: process.env.NEO_ORCHESTRATOR_SWARM_HEARTBEAT_ENABLED = 'false'env.NEO_ORCHESTRATOR_SWARM_HEARTBEAT_ENABLED = false (mutates the registry object; getter re-reads).
  • src/util/Env.mjs — JSDoc updated to point at the registry pattern over the per-call-site Env.parseX(process.env.X, 'X') invocation.
  • test/playwright/unit/util/Env.spec.mjs — Test comment updated for consistency.

Design choices

buildEnv({bindings, source}) pure function lifted out of the default-export construction so unit tests don't need to fight the module-cache-busting + env-mutation-before-import dance that the eager-bound singleton would otherwise require. The module-level env is buildEnv({bindings}) against process.env.

Registry keyed by env-var name (env.NEO_ORCHESTRATOR_KB_SYNC_ENABLED) not semantic name (env.kbSyncEnabled). Per ticket's avoided traps: the semantic mapping is the consumer's call (different consumers may want different defaults via ??); the registry's job is the typed env-var slot, not the policy.

Eager binding at module load preserved per ticket guidance (env vars don't change at runtime in production). The one runtime-mutation site (swarmHeartbeatEnabled init-failure path) now mutates the registry object directly — observable behavior identical because the object is mutable and the getter re-reads on every call.

Scope discipline: 16 grep matches for Env.parseX(process.env...) — 13 in Orchestrator.mjs getters (migrated), 3 in JSDoc/comments (updated). The broader pattern of bare process.env.NEO_X || default direct reads in ai/config.template.mjs, ai/daemons/bridge/daemon.mjs, KbAlertingService, TaskDefinitions.mjs is out of scope — those follow a different shape (config-source defaults, not consumer fallbacks).

Test Evidence

  • npm run test-unit -- --grep="ai/config/env"6/6 pass (2.0s)
  • npm run test-unit -- --grep="ai/config/env|Orchestrator|Env"231 passed, 0 failed (8.7s)
  • npm run test-unit (full suite) → 2049 passed, 2 skipped, 6 did not run, 0 failed (56.4s)

Post-Merge Validation

  • No behavioral change observable in orchestrator boot or scheduling (consumer pattern preserves env.X ?? AiConfig.X chain semantics)
  • Local .env files now auto-load via dotenv/config — pre-existing prod boots unaffected (no .env file present → dotenv silently no-ops)

AC mapping

AC Status
1. ai/config/env.mjs with binding registry covering NEO_* vars discovered via pre-survey ✓ 13 NEO_ORCHESTRATOR_* vars covered (grep-clean for Env.parseX(process.env...) pattern in ai/)
2. dotenv/config imported at module load — .env auto-load ✓ Line 1 of ai/config/env.mjs
3. All consumers migrated from Env.parseX(process.env.NAME, 'NAME')env.NAME ✓ 13/13 getters in Orchestrator.mjs migrated
4. src/util/Env.mjs:12 JSDoc updated — recommends registry pattern ✓ JSDoc now points at the binding registry shape
5. Unit test for ai/config/env.mjs covering absent/valid/invalid/leakage/mixed-parser paths ✓ 6 tests cover all required paths; module-cache-busting + .env-load tests deferred per Deltas
6. Zero remaining Env.parseX(process.env.NAME, 'NAME') in ai/ git grep -E "Env\.parse(Bool|Number|Port|Url|String)\(process\.env" -- 'ai/' returns 0
7. All existing unit + integration tests pass ✓ 2049/2049 full unit suite, 0 failures

Deltas from ticket

  • AC 5 sub-bullets .env-file load + override precedence: those test dotenv's contract, not this PR's code. The buildEnv({bindings, source}) pure-function shape lets the spec cover the registry logic without re-validating upstream dotenv. If operator wants explicit dotenv-integration tests, separate ticket would add them via dotenv.config({path}) programmatic API.
  • Avoided-trap "Don't pre-suppose the registry's filesystem location": went with the ticket's proposed ai/config/env.mjs (sibling to ai/config.template.mjs — consistent thematic bucket). Happy to move if operator prefers another path.

Depends on

Epic #11831. Independent of Sub 19 #11861 (different surface — Sub 19 touches the §E-H methods; Sub 14 touches the §D lazy getters). Either ordering merges cleanly.

Unblocks

Closes the env-binding anti-pattern that survived the Sub-1 masterclass refactor — completes the Epic #11831 cleanup pass at this layer (Sub 18 #11862 then closes the remaining Round-2 surface).

Authority

Discussion #11857 Cycle-3.5 graduation + operator architectural ratify. Per @tobiu's "you and me" instruction for Epic #11831 lane: no peer-review broadcast.

tobiu
tobiu APPROVED reviewed on May 23, 2026, 11:53 PM

No review body provided.