---
id: 11832
title: 'Sub 6: Neo.util.Env THIN substrate primitive (lift EnvConfig.mjs parsers to src/util/Env.mjs)'
state: CLOSED
labels:
  - enhancement
  - ai
  - refactoring
  - architecture
assignees:
  - neo-opus-ada
createdAt: '2026-05-23T11:21:23Z'
updatedAt: '2026-06-07T17:14:46Z'
githubUrl: 'https://github.com/neomjs/neo/issues/11832'
author: neo-opus-ada
commentsCount: 0
parentIssue: 11831
subIssues: []
subIssuesCompleted: 0
subIssuesTotal: 0
blockedBy: []
blocking:
  - '[x] 11833 Sub 1: Orchestrator masterclass-reference refactor (4-way Service-DI + 2-value chain + configure() removal)'
closedAt: '2026-05-23T12:15:09Z'
---
# Sub 6: Neo.util.Env THIN substrate primitive (lift EnvConfig.mjs parsers to src/util/Env.mjs)

## Parent Epic

#11831 — **Sub 6 of 6 (substrate-prerequisite; blocks Sub 1).**

## Scope

THIN substrate-utility primitive. Lift `ai/mcp/server/shared/helpers/EnvConfig.mjs` parsers verbatim to `src/util/Env.mjs` as `Neo.util.Env` (mirrors `Neo.util.String` static-class precedent). Provides Orchestrator-needed parser surface so Sub 1 can implement the 2-value chain via `Neo.util.Env.parseNumber()`.

**Scope-bounded** per Discussion #11828 Cycle-3.1 deferral fixup (avoid recreating the #11075 "start somewhere" failure). DOES NOT include the full 61-file consumer migration — that decomposes into separate Sub 6a/6b/6c follow-up tickets (orchestrator-cluster, mcp-cluster, daemon-cluster, ad-hoc-cluster).

## Acceptance Criteria

1. Create `src/util/Env.mjs` extending `Base`, `Neo.setupClass()` registered as `Neo.util.Env`. Mirrors `src/util/String.mjs` static-class pattern.
2. API surface (lifted from `EnvConfig.mjs` + Orchestrator-needed): `parseNumber(rawValue, envVarName, warn?)`, `parseBool(rawValue, envVarName, warn?)`, `parsePort(rawValue, envVarName, warn?)`, `parseUrl(rawValue, envVarName, warn?)`, `parseString(rawValue)`, `applyEnvBindings(data, bindings, env?, warn?)`, `setDeep(obj, path, value)`.
3. **Boolean compatibility:** `parseBool` accepts permissive token set preserving both MCP (`'true'`/`'false'`) and daemon (`PrimaryRepoSyncService.parseEnabledFlag` `'0'`/`'no'`/`'off'`) semantics. Tokens: true = `'true'`/`'yes'`/`'on'`/`'1'`; false = `'false'`/`'no'`/`'off'`/`'0'`; else warn + undefined. Case-insensitive after trim.
4. Add `Env` entry to `src/util/_export.mjs` barrel.
5. **Availability:** consumers use direct `import Env from '<path>/src/util/Env.mjs'` (NOT relying on `Neo.util.Env` global — `orchestrator-daemon.mjs` boots `src/core/_export.mjs`, not the util barrel).
6. Decoders MUST NOT know AiConfig / defaults / aliases / fallback chains / specific domain consumers (substrate-tier discipline). Fallback to AiConfig stays at consumer call-site.
7. **`Number(undefined) === NaN` gotcha protection:** `parseNumber` returns `undefined` (NOT NaN) for absent input, so `Env.parseNumber(...) ?? AiConfig.X` correctly falls through to AiConfig fallback.
8. **Test surface:** `test/playwright/unit/util/Env.spec.mjs` proves decoder semantics: absent/empty/null → undefined no warn; well-formed → decoded; malformed → warn + undefined; `??` fallback works correctly; boolean tokens (both MCP + daemon sets); port out-of-range; URL normalization; prototype-pollution guards in `setDeep`; `applyEnvBindings` skip-on-absent + skip-on-malformed.

## Out-of-scope (follow-up subs)

- 61-file / 80 env-var consumer migration → Sub 6a (orchestrator-cluster), Sub 6b (mcp-cluster), Sub 6c (daemon-cluster), Sub 6d (ad-hoc-cluster)
- `EnvConfig.mjs` deletion → after Sub 6a/6b consumers rewire
- `CadenceEngine.parseInterval` deletion → after Sub 1 lands
- `PrimaryRepoSyncService.parseEnabledFlag` deletion → after Sub 6c daemon migration

## Files (new + touched)

- NEW: `src/util/Env.mjs` (~190 lines, lifted from `EnvConfig.mjs` + Orchestrator-needed surface)
- EDIT: `src/util/_export.mjs` (add `Env` import + export entry)
- NEW: `test/playwright/unit/util/Env.spec.mjs`
- (no `EnvConfig.mjs` deletion — consumer rewire deferred to follow-up subs to keep this PR thin)

**Authored by:** [Claude Opus 4.7] (Claude Code)

## Timeline

- 2026-05-23T11:21:23Z @neo-opus-ada added the `enhancement` label
- 2026-05-23T11:21:23Z @neo-opus-ada assigned to @neo-opus-ada
- 2026-05-23T11:21:23Z @neo-opus-ada added the `ai` label
- 2026-05-23T11:21:23Z @neo-opus-ada added the `refactoring` label
- 2026-05-23T11:21:24Z @neo-opus-ada added the `architecture` label
- 2026-05-23T11:22:55Z @neo-opus-ada added parent issue #11831
- 2026-05-23T11:23:40Z @neo-opus-ada marked this issue as blocking #11833
- 2026-05-23T11:26:53Z @neo-opus-ada cross-referenced by PR #11838
- 2026-05-23T11:36:34Z @neo-opus-ada referenced in commit `7feef0c` - "refactor(util): delegate Env.setDeep namespace-walk to Neo.ns canonical primitive (#11832)

@tobiu REQUEST CHANGES on PR #11838: setDeep was reinventing namespace-walk
already canonicalized in Neo.ns() at src/Neo.mjs:683.

setDeep now uses Neo.ns(parentKeys, false, obj) for the namespace traversal.
Retains the env-binding-substrate-specific concerns Neo.ns does not provide:

- Prototype-pollution guard for ALL keys (loop validates each key against
  __proto__ / constructor / prototype before delegating to Neo.ns)
- Non-object-intermediate refusal (Neo.ns silently returns undefined on
  non-object intermediates; setDeep wraps that with an explicit warn)

These guards are appropriate because env-binding paths derive from
operator-provided env-binding maps (hostile-input surface), unlike the
trusted class-name input Neo.ns was designed for.

Tests: 23/23 pass (600ms). All previous setDeep behaviors preserved:
- sets value at simple path
- blocks prototype pollution via __proto__
- blocks prototype pollution via constructor
- refuses to traverse non-object intermediate keys"
- 2026-05-23T11:40:18Z @neo-opus-ada referenced in commit `b9ae97e` - "refactor(util): drop setDeep + prototype-pollution guards as over-engineering (#11832)

@tobiu challenge: env values become typed leaf values, never path keys —
the path is developer-authored at source level in envBindings, so there is
no env-value-derived path injection vector. Prototype-pollution guards were
cargo-cult lifted from EnvConfig.mjs original; ~0.01% threat-model relevance
for env-binding-substrate.

Changes:
- Drop Env.setDeep entirely (zero external consumers per grep ai/ src/)
- Inline minimal namespace-walk in applyEnvBindings using Neo.ns
- Add explicit comment documenting the threat-model decision (so future
  agents don't re-add the guards thinking they were missed)
- Drop 4 setDeep tests (simple path, __proto__, constructor, non-object
  intermediate); keep 1 new applyEnvBindings test exercising the non-object-
  intermediate warn path through the actual public API consumer

Tests: 20/20 pass (602ms). Net: 23 → 20 tests, ~30 lines removed from Env.mjs."
- 2026-05-23T11:44:56Z @neo-opus-ada referenced in commit `7ad1c7a` - "refactor(util): drop impossible-condition globalThis.Neo guard in applyEnvBindings (#11832)

@tobiu: src/util/Env.mjs IS Neo.util.Env — registered via Neo.setupClass().
By the time applyEnvBindings runs, Neo is guaranteed loaded. The dual-branch
`if (globalThis.Neo && Neo.isEmpty(raw))` / `if (!globalThis.Neo && ...)`
pattern was impossible-condition cargo-cult lifted from EnvConfig.mjs
(which sat in ai/mcp/server/shared/helpers/ with plausibly-pre-Neo
MCP-standalone context — that context no longer applies).

Simplifies to single line: `if (Neo.isEmpty(raw)) continue;`

Tests: 20/20 pass (619ms)."
- 2026-05-23T11:49:23Z @neo-opus-ada referenced in commit `8c2b038` - "refactor(util): use Neo.isEmpty + Neo.isObject + drop explicit return undefined (#11832)

3 operator corrections in one pass:

1. `if (rawValue === undefined || rawValue === null || rawValue === '')`
   replaced with `if (Neo.isEmpty(rawValue))` — Neo.isEmpty handles all three
   (null, undefined, '') canonically via existing primitive.

2. `if (parent && typeof parent === 'object')` replaced with
   `if (Neo.isObject(parent))` — uses Neo.isObject canonical check;
   handles null safely via constructor.name optional chaining.

3. Explicit `return undefined;` at end of methods (and in guard branches)
   dropped — bare `return;` is the JS-idiomatic shape; default return value
   is already undefined.

Tests: 20/20 pass (615ms).

Net: ~10 lines removed across parsePort, parseUrl, parseBool, parseNumber,
applyEnvBindings."
- 2026-05-23T11:52:56Z @neo-opus-ada referenced in commit `df93549` - "refactor(util): switch Env from setupClass to gatekeep pattern (#11832)

@tobiu: Env is pure stateless value-decoders — no Base inheritance, no
reactive configs, no lifecycle hooks. Lightweight gatekeep pattern is the
right fit, mirroring src/core/IdGenerator.mjs precedent.

Drops:
- import Base from '../core/Base.mjs'
- class Env extends Base
- static config = { className: 'Neo.util.Env' }
- Neo.setupClass(Env) export
- all `static` keywords on methods

Adopts:
- Plain object with methods (alphabetically sorted)
- Neo.gatekeep(Env, 'Neo.util.Env') export

Public API unchanged — `import Env from '<path>/src/util/Env.mjs'` and
`Env.parseNumber()` / etc work identically. Test surface untouched
(20/20 pass, 601ms)."
- 2026-05-23T12:15:10Z @tobiu referenced in commit `07e4389` - "feat(util): Neo.util.Env substrate primitive for env-var parsing (#11832) (#11838)

* feat(util): add Neo.util.Env substrate primitive for env-var parsing (#11832)

Lifts ai/mcp/server/shared/helpers/EnvConfig.mjs parsers verbatim to
src/util/Env.mjs as Neo.util.Env, mirroring Neo.util.String static-class
precedent. THIN substrate-prerequisite unblocking Sub 1 (#11833) Orchestrator
masterclass-reference refactor — provides Env.parseNumber + Env.parseBool +
other decoders for the 2-value chain pattern (Env.parseNumber(process.env.X)
?? AiConfig.X) without coupling daemon code to a domain-class env parser.

API surface (lifted + Orchestrator-needed):
- parseNumber, parseBool, parsePort, parseUrl, parseString
- applyEnvBindings, setDeep (with prototype-pollution guards)

Boolean compatibility preserves both MCP ('true'/'false') AND daemon
(PrimaryRepoSyncService.parseEnabledFlag '0'/'no'/'off') conventions:
- true:  'true'/'yes'/'on'/'1' (case-insensitive after trim)
- false: 'false'/'no'/'off'/'0'
- other: warn + undefined

parseNumber returns undefined (NOT NaN) for absent input — load-bearing
for the Env.parseNumber(...) ?? AiConfig.X consumer pattern because ??
only fires on nullish, not NaN.

Decoders sit BELOW the policy line — know nothing about AiConfig / defaults
/ aliases / fallback chains / specific domain consumers. Fallback to AiConfig
stays at consumer call-site.

Test surface: test/playwright/unit/util/Env.spec.mjs proves decoder semantics
(23 tests, 611ms): absent/empty/null to undefined no warn; well-formed to
decoded; malformed to warn + undefined; ?? fallback pattern works; boolean
tokens (both MCP + daemon sets, case-insensitive); port out-of-range; URL
normalization; prototype-pollution guards in setDeep; applyEnvBindings
skip-on-absent + skip-on-malformed.

DOES NOT include the full 61-file / 80-env-var consumer migration — that
decomposes into separate Sub 6a/6b/6c follow-up tickets (orchestrator-cluster,
mcp-cluster, daemon-cluster) to avoid recreating the #11075 'start somewhere'
failure. EnvConfig.mjs retention deferred to Sub 6a/6b consumer rewire.

Graduates from Discussion #11828 (Cycle-3.3 GRADUATION_APPROVED at body
anchor 2026-05-23T10:58:31Z by @neo-gpt @ DC_kwDODSospM4BA-Ic).

* refactor(util): delegate Env.setDeep namespace-walk to Neo.ns canonical primitive (#11832)

@tobiu REQUEST CHANGES on PR #11838: setDeep was reinventing namespace-walk
already canonicalized in Neo.ns() at src/Neo.mjs:683.

setDeep now uses Neo.ns(parentKeys, false, obj) for the namespace traversal.
Retains the env-binding-substrate-specific concerns Neo.ns does not provide:

- Prototype-pollution guard for ALL keys (loop validates each key against
  __proto__ / constructor / prototype before delegating to Neo.ns)
- Non-object-intermediate refusal (Neo.ns silently returns undefined on
  non-object intermediates; setDeep wraps that with an explicit warn)

These guards are appropriate because env-binding paths derive from
operator-provided env-binding maps (hostile-input surface), unlike the
trusted class-name input Neo.ns was designed for.

Tests: 23/23 pass (600ms). All previous setDeep behaviors preserved:
- sets value at simple path
- blocks prototype pollution via __proto__
- blocks prototype pollution via constructor
- refuses to traverse non-object intermediate keys

* refactor(util): drop setDeep + prototype-pollution guards as over-engineering (#11832)

@tobiu challenge: env values become typed leaf values, never path keys —
the path is developer-authored at source level in envBindings, so there is
no env-value-derived path injection vector. Prototype-pollution guards were
cargo-cult lifted from EnvConfig.mjs original; ~0.01% threat-model relevance
for env-binding-substrate.

Changes:
- Drop Env.setDeep entirely (zero external consumers per grep ai/ src/)
- Inline minimal namespace-walk in applyEnvBindings using Neo.ns
- Add explicit comment documenting the threat-model decision (so future
  agents don't re-add the guards thinking they were missed)
- Drop 4 setDeep tests (simple path, __proto__, constructor, non-object
  intermediate); keep 1 new applyEnvBindings test exercising the non-object-
  intermediate warn path through the actual public API consumer

Tests: 20/20 pass (602ms). Net: 23 → 20 tests, ~30 lines removed from Env.mjs.

* refactor(util): drop impossible-condition globalThis.Neo guard in applyEnvBindings (#11832)

@tobiu: src/util/Env.mjs IS Neo.util.Env — registered via Neo.setupClass().
By the time applyEnvBindings runs, Neo is guaranteed loaded. The dual-branch
`if (globalThis.Neo && Neo.isEmpty(raw))` / `if (!globalThis.Neo && ...)`
pattern was impossible-condition cargo-cult lifted from EnvConfig.mjs
(which sat in ai/mcp/server/shared/helpers/ with plausibly-pre-Neo
MCP-standalone context — that context no longer applies).

Simplifies to single line: `if (Neo.isEmpty(raw)) continue;`

Tests: 20/20 pass (619ms).

* refactor(util): use Neo.isEmpty + Neo.isObject + drop explicit return undefined (#11832)

3 operator corrections in one pass:

1. `if (rawValue === undefined || rawValue === null || rawValue === '')`
   replaced with `if (Neo.isEmpty(rawValue))` — Neo.isEmpty handles all three
   (null, undefined, '') canonically via existing primitive.

2. `if (parent && typeof parent === 'object')` replaced with
   `if (Neo.isObject(parent))` — uses Neo.isObject canonical check;
   handles null safely via constructor.name optional chaining.

3. Explicit `return undefined;` at end of methods (and in guard branches)
   dropped — bare `return;` is the JS-idiomatic shape; default return value
   is already undefined.

Tests: 20/20 pass (615ms).

Net: ~10 lines removed across parsePort, parseUrl, parseBool, parseNumber,
applyEnvBindings.

* refactor(util): switch Env from setupClass to gatekeep pattern (#11832)

@tobiu: Env is pure stateless value-decoders — no Base inheritance, no
reactive configs, no lifecycle hooks. Lightweight gatekeep pattern is the
right fit, mirroring src/core/IdGenerator.mjs precedent.

Drops:
- import Base from '../core/Base.mjs'
- class Env extends Base
- static config = { className: 'Neo.util.Env' }
- Neo.setupClass(Env) export
- all `static` keywords on methods

Adopts:
- Plain object with methods (alphabetically sorted)
- Neo.gatekeep(Env, 'Neo.util.Env') export

Public API unchanged — `import Env from '<path>/src/util/Env.mjs'` and
`Env.parseNumber()` / etc work identically. Test surface untouched
(20/20 pass, 601ms).

---------

Co-authored-by: tobiu <tobiasuhlig78@gmail.com>"
- 2026-05-23T12:15:10Z @tobiu closed this issue
- 2026-05-23T13:03:13Z @neo-opus-ada cross-referenced by PR #11842
- 2026-05-23T13:10:37Z @tobiu referenced in commit `27f7a65` - "refactor(agentos): Orchestrator masterclass-reference (4-way Service-DI + Neo.util.Env + configure() removal) (#11833) (#11842)

Sub 1 of Epic #11831. Refactors ai/daemons/Orchestrator.mjs from 1046 lines
to 760 lines via the 4-way Service-DI classification + 2-value chain pattern
graduated from Discussion #11828.

Architecture changes:
- 30+ cargo-cult `_`-suffix configs replaced with lazy getters (Class D):
  `Env.parseNumber(process.env.X, 'X') ?? AiConfig.orchestrator.intervals.X`
  for 8 interval values, `Env.parseBool(...) ?? resolveDeploymentEnabled(...)`
  for 5 boolean values. No setter overhead, no DEFAULT_X constants, no
  per-property fallback chain.
- cadenceEngine_ (Class A) becomes proper class-system-managed reactive
  collaborator with `beforeSet` + ClassSystemUtil.beforeSetInstance for
  polymorphic class/instance/config-object input.
- processSupervisorService_ (Class B) becomes parent-configured child
  collaborator with `beforeSet` creation from parent state + parent
  afterSet* propagation hooks for dataDir / taskDefinitions / taskStateService
  / healthService / spawnFn. Stable processSupervisorWriteLog seam avoids
  per-call .bind() allocation. start() no longer replays parent config
  imperatively via processSupervisorService.set({...this...}).
- Simple service singletons become Class C instance fields (direct import,
  no reactive config slot).
- configure() shadow-resolver REMOVED entirely (was duplicating
  initConfig's work and shadowing AiConfig precedence).

Substrate dependency removals:
- CadenceEngine + ProcessSupervisorService no longer `singleton: true`
  (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_MS constants (8) 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.

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
  (parseInterval queued for removal by Sub 3 #11835).

Tests: 69/69 pass (1.6s) across 6 affected spec files: Orchestrator.spec.mjs
(26), ProcessSupervisorService.spec.mjs (13), CadenceEngine.spec.mjs (5),
orchestrator-daemon.spec.mjs (10), config.template.spec.mjs (3), Env.spec.mjs
(20 from Sub 6 #11832).

File-size delta: Orchestrator.mjs 1046 → 760 lines (-286 / -27%). Net
project delta: +349 / -416 = -67 lines across 8 files.

Graduates from Discussion #11828 (Cycle-3.3 GRADUATION_APPROVED at body
anchor 2026-05-23T10:58:31Z by @neo-gpt @ DC_kwDODSospM4BA-Ic).

Co-authored-by: tobiu <tobiasuhlig78@gmail.com>"
- 2026-05-23T13:16:47Z @neo-opus-ada cross-referenced by PR #11843
- 2026-05-23T13:28:43Z @tobiu referenced in commit `7e7adb2` - "refactor(agentos): delete obsolete CadenceEngine.parseInterval + PrimaryRepoSyncService.parseEnabledFlag (#11835) (#11843)

* refactor(agentos): delete obsolete CadenceEngine.parseInterval + PrimaryRepoSyncService.parseEnabledFlag (#11835)

Sub 3 of Epic #11831. Removes two parser surfaces whose sole runtime consumer
(Orchestrator) was rewired in Sub 1 (#11833) to use Neo.util.Env from Sub 6
(#11832). Both functions had zero remaining non-test consumers per grep.

Removed:
- CadenceEngine.parseInterval() — Orchestrator now uses
  Env.parseNumber(process.env.X, 'X') ?? AiConfig.X via lazy getters.
  Math.max(0, n) clamp re-applied at caller if non-negative semantics needed
  (Orchestrator does not — intervals already validated by AiConfig schema).
- PrimaryRepoSyncService.parseEnabledFlag() — Orchestrator now uses
  Env.parseBool(process.env.X, 'X') ?? resolveDeploymentEnabled('X')
  via lazy getters. Env.parseBool preserves the legacy false tokens
  ('0', 'false', 'no', 'off') per Sub 6 (#11832) AC9 boolean compatibility,
  so the runtime behavior is unchanged.

Test surface migration:
- CadenceEngine.spec.mjs — 3 parseInterval tests removed (coverage lives in
  test/playwright/unit/util/Env.spec.mjs ?? fallback + boolean tests).
- PrimaryRepoSyncService.spec.mjs — parseEnabledFlag import + 4 inline
  assertions removed; the existing "preserves PrimaryRepoSyncService.parseEnabledFlag
  legacy false tokens" test in Env.spec.mjs documents the lift.

Out of scope (deferred to Sub 4 #11836 sibling-daemon propagation):
- SwarmHeartbeatService.mjs:37 LOCAL DEFAULT_POLL_INTERVAL_MS (consumer
  rewire requires the full Sub-1-style daemon refactor).
- KbAlertingService.mjs:13/202 DEFAULT_COOLDOWN_MS (same — full daemon refactor).
- #11827 audit-record closure (will be handled post-merge as `gh issue close
  #11827 --reason 'superseded by Epic #11831 Sub 3 #11835'`).

Tests: 49/49 pass (1.3s) across 3 affected specs.

* refactor(agentos): drop historical removal-tombstone comments (#11835)

Per @tobiu: "removed per Epic X" inline comments are bloat that provide zero
value to future readers. Git log + commit message hold the rationale.

Removes the 4 tombstone comment blocks added in the prior commit on this
branch. Net: -19 lines bloat.

Tests: 23/23 pass.

---------

Co-authored-by: tobiu <tobiasuhlig78@gmail.com>"
- 2026-06-06T21:08:07Z @neo-gpt cross-referenced by #11831

