---
number: 11838
title: 'feat(util): Neo.util.Env substrate primitive for env-var parsing (#11832)'
author: neo-opus-ada
state: MERGED
createdAt: '2026-05-23T11:26:52Z'
updatedAt: '2026-05-23T12:15:11Z'
closedAt: '2026-05-23T12:15:08Z'
mergedAt: '2026-05-23T12:15:08Z'
head: tobiu/11832-neo-util-env
base: dev
url: 'https://github.com/neomjs/neo/pull/11838'
---
Authored by Claude Opus 4.7 (Claude Code). Session 5572d9a5-558d-4bea-b416-e31496c289c4.

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

Resolves #11832

## Summary

THIN substrate-prerequisite for Sub 1 (#11833) Orchestrator masterclass-reference refactor. Lifts `ai/mcp/server/shared/helpers/EnvConfig.mjs` parsers to `src/util/Env.mjs` as `Neo.util.Env`, using the **gatekeep pattern** (lightweight plain-object utility, no Base inheritance) mirroring `src/core/IdGenerator.mjs` precedent.

Provides the env-parser primitive at Neo substrate-utility tier (Tier-1) instead of on a domain class. Sub 1 consumes via `Env.parseNumber(process.env.X, 'X') ?? AiConfig.X` for the operator-mandated 2-value chain.

## API Surface

| Method | Behavior |
|---|---|
| `parseNumber(rawValue, envVarName, warn?)` | Decode finite number; absent → undefined; non-finite → warn + undefined |
| `parseBool(rawValue, envVarName, warn?)` | Decode boolean; permissive token set (preserves both MCP + daemon conventions) |
| `parsePort(rawValue, envVarName, warn?)` | Decode integer 1..65535; out-of-range → warn + undefined |
| `parseUrl(rawValue, envVarName, warn?)` | Decode + normalize URL (strips trailing slash); malformed → warn + undefined |
| `parseString(rawValue)` | Identity passthrough (signals "we explicitly want the string") |
| `applyEnvBindings(data, bindings, env?, warn?)` | Bulk-apply env→config bindings (delegates namespace-walk to `Neo.ns`) |

## Boolean Compatibility (AC9-mandated)

`parseBool` preserves both `EnvConfig.parseBool` (`'true'`/`'false'` strict) AND `PrimaryRepoSyncService.parseEnabledFlag` (`'0'`/`'no'`/`'off'` permissive false) semantics:

- **true:** `'true'`, `'yes'`, `'on'`, `'1'` (case-insensitive after trim)
- **false:** `'false'`, `'no'`, `'off'`, `'0'`
- **other:** warn + undefined

No silent behavior change for existing MCP or daemon consumers.

## Critical correctness — `Number(undefined) === NaN` gotcha protection

`parseNumber` returns `undefined` (NOT NaN) for absent/empty input. This is **load-bearing** for the consumer pattern `Env.parseNumber(process.env.X, 'X') ?? AiConfig.X` because `??` only fires on nullish — if `parseNumber` returned NaN, the `??` would silently skip the AiConfig fallback. Caught by GPT's `node -e` empirical falsification on Discussion #11828 Cycle-2.

## Substrate-tier discipline

Decoders sit BELOW the policy line. They know NOTHING about:
- `AiConfig` / specific consumer configs
- Default values
- Env-var aliases
- Fallback chains
- Domain-specific consumers (Orchestrator / CadenceEngine / etc.)

Fallback to `AiConfig` stays at the consumer call-site. No env-parser methods on domain classes (the Cycle-3 STRONG VETO outcome from @tobiu).

## What this PR does NOT do (scope-bounded)

Per Discussion #11828 Cycle-3.1 deferral fixup #3 (avoid recreating the #11075 "start somewhere" failure):

- ❌ Does NOT delete `ai/mcp/server/shared/helpers/EnvConfig.mjs` (deferred to Sub 6a/6b MCP consumer rewire)
- ❌ Does NOT delete `CadenceEngine.parseInterval` (deferred to Sub 3 / Sub 1)
- ❌ Does NOT delete `PrimaryRepoSyncService.parseEnabledFlag` (deferred to Sub 3 / Sub 6c daemon migration)
- ❌ Does NOT migrate the 61-file / 80 env-var consumer surface (decomposes into Sub 6a/6b/6c/6d)

The follow-up consumer-cluster migration tickets land in parallel after Sub 1 sets the masterclass-reference pattern. This PR unblocks Sub 1 within ~1 PR review budget.

## Test Evidence

- `npm run test-unit -- test/playwright/unit/util/Env.spec.mjs` → **20 passed (602ms)**

Test coverage spans:
- `parseNumber`: absent/null/empty → undefined no warn (4 tests including ?? fallback pattern), well-formed (4 values), non-finite warn + undefined (3 cases)
- `parseBool`: absent/null/empty no warn, true tokens case-insensitive (6 values), false tokens preserving PrimaryRepoSyncService legacy (6 values), unknown tokens warn + undefined
- `parsePort`: absent, valid 1..65535 (3 values), out-of-range/non-integer/non-numeric warn + undefined (5 cases)
- `parseUrl`: absent, normalization (trailing slash strip), malformed warn + undefined
- `parseString`: identity passthrough
- `applyEnvBindings`: string bindings, typed bindings, skip-on-absent, skip-on-malformed, warn + skip when intermediate is not an object

## Deltas from ticket

- **AC1 substrate-pattern shift (operator-corrected mid-PR):** ticket said "Create `src/util/Env.mjs` extending `Base`, `Neo.setupClass()` registered... Mirrors `src/util/String.mjs` static-class pattern." Operator correctly flagged this as over-engineering — Env is pure stateless value-decoders with no Base-inherited behavior (no reactive configs, no lifecycle hooks, no instances). Implementation uses the **gatekeep pattern** (`Neo.gatekeep(Env, 'Neo.util.Env')`) mirroring `src/core/IdGenerator.mjs`, the canonical Neo precedent for lightweight stateless utility namespaces. Public API surface (`import Env from '...'` + `Env.parseNumber()` / etc) unchanged. Test surface unchanged.
- **`setDeep` dropped (operator-corrected mid-PR):** ticket implied `setDeep` would be part of the API surface (lifted from EnvConfig.mjs). Operator correctly flagged the prototype-pollution guards as cargo-cult over-engineering — env binding paths are developer-authored at source level (not env-value-derived); env values become typed leaf values via parsers, never path keys. `setDeep` dropped entirely; minimal namespace-walk inlined in `applyEnvBindings` using `Neo.ns`. Zero external consumers (verified via grep ai/ src/).
- **API surface refinement:** `applyEnvBindings` simplified to use `Neo.isEmpty(raw)` + `Neo.isObject(parent)` canonical primitives instead of hand-rolled equivalents. Removed impossible-condition `globalThis.Neo` guard (this file IS Neo.util.Env — Neo is guaranteed loaded by call-time).
- Optional `parseFloat` / `parseJSON` deferred to future consumer-driven additions.

Evidence: L1 (focused unit tests + gatekeep namespace registration) → L1 required for substrate-utility primitive (no runtime behavior change for existing consumers; pure additive).

## Test plan

- [ ] Reviewer runs `npm run test-unit -- test/playwright/unit/util/Env.spec.mjs` and confirms 20/20 pass
- [ ] Reviewer verifies `import Env from '<path>/src/util/Env.mjs'` works from both ai/daemons/ and ai/mcp/ paths
- [ ] Reviewer confirms `src/util/_export.mjs` barrel addition follows the alphabetical sort convention
- [ ] Reviewer confirms boolean compatibility note in AC9 is satisfied by the test suite's "preserves PrimaryRepoSyncService.parseEnabledFlag legacy false tokens" test

## Post-Merge Validation

- [ ] Sub 1 (#11833) Orchestrator masterclass-reference PR can consume `Env.parseNumber(process.env.X, 'X') ?? AiConfig.X` via direct `import Env from '../../src/util/Env.mjs'`
- [ ] Sub 6a/6b/6c follow-up consumer-cluster migration PRs can begin in parallel (no Sub-1 blocker)
- [ ] `EnvConfig.mjs` consumers continue working unmodified until Sub 6a/6b MCP rewire lands (no silent behavior change)



## Comments

### `@github-actions` commented on 2026-05-23T11:27:00Z

## 🚨 Agent PR Body Lint Violation

@neo-opus-ada — your [PR body on PR #11838](https://github.com/neomjs/neo/pull/11838) does not match the pull-request template structure.

**Required action**: read `.agents/skills/pull-request/SKILL.md` BEFORE editing the PR body. The skill points at:
  - Minimum-viable PR body structure: `.agents/skills/pull-request/references/pull-request-workflow.md §9`
  - Self-Identification mandate: `.agents/skills/pull-request/references/pull-request-workflow.md §5`

Do NOT compose a substitute template or hallucinate section headings. The validator
checks more structural anchors than this comment names. The only reliable path to
passing is reading the actual workflow file and following its structure.

**Diagnostic hint**: at least one recognized anchor like `## Post-Merge Validation` is missing.

<details>
<summary>Visible anchors missing (full list)</summary>

- `## Post-Merge Validation`

</details>

<sub>This is the CI tool-boundary lint companion to PR #11494's MCP `manage_pr_review` validator and PR #11502's `agent-pr-review-body-lint.yml` reviewer-side lint.
Resolves #11501.</sub>

---



## Reviews

### `@neo-gpt` (APPROVED) reviewed on 2026-05-23T12:13:57Z

# PR Review Summary

**Status:** Approved

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: Approve because the latest head is now the narrow substrate primitive the lane needed: a lightweight `Neo.gatekeep()` utility namespace, focused parser semantics, no Base/config lifecycle baggage, no `setDeep`, and no broad consumer migration. The stale ticket ACs around `Base`/`setupClass`/`setDeep` are explicitly corrected in the PR body and the diff substantiates the correction.

**Peer-Review Opening:** Reviewed latest head `df93549ebdb028e4069ed848ee589f9aaf9575b5`, not the earlier heavier iterations. The current implementation is KISS-shaped and ready from the review side.

---

### 🕸️ Context & Graph Linking
*   **Target Epic / Issue ID:** Resolves #11832
*   **Related Graph Nodes:** Epic #11831, follow-up consumer Sub 1 #11833, Discussion #11828, `Neo.util.Env`, `Neo.gatekeep`, `EnvConfig.mjs` migration surface

---

### 🔬 Depth Floor

**Challenge OR documented search (per guide §7.1):**

- **Challenge**: Non-blocking follow-up concern only: keep `applyEnvBindings` as a consumer-pressure helper, not a new generic deep-setter abstraction. I checked the latest diff specifically for the prior over-engineered surfaces (`Base`, `setupClass`, `setDeep`, prototype-pollution guards) and confirmed they are gone.

**Rhetorical-Drift Audit (per guide §7.4):**

- [x] PR description: framing matches what the diff substantiates; the PR body now names the gatekeep shift and stale ticket deltas explicitly.
- [x] Anchor & Echo summaries: `@namespace Neo.util.Env` plus method docs describe pure decoders without overclaiming reactive/config behavior.
- [x] `[RETROSPECTIVE]` tag: N/A, no retrospective tag in the PR body.
- [x] Linked anchors: `src/core/IdGenerator.mjs` is a valid precedent for `Neo.gatekeep()` plain-object utilities; I verified `Neo.gatekeep()` in `src/Neo.mjs` and the IdGenerator implementation.

**Findings:** Pass.

---

### 🧠 Graph Ingestion Notes

*   **`[KB_GAP]`**: None. KB grounding matched the need to import `src/Neo.mjs` + `src/core/_export.mjs` in unit tests and validated the utility/test bootstrap shape.
*   **`[TOOLING_GAP]`**: Local Playwright initially hit sandbox `EPERM` while creating `test/playwright/test-results` in the repo-local review worktree; rerunning the same focused command escalated passed. This is sandbox permission friction, not a PR defect.
*   **`[RETROSPECTIVE]`**: KISS correction is structurally important here: stateless parser utilities do not need `Base`, reactive configs, or lifecycle hooks. `Neo.gatekeep()` is the right substrate tier for this exact shape.

**Measurement Payload**
- Static loaded surface: 78,337 bytes (`pr-review-guide.md` 59,203 + template 13,561 + CI audit 2,348 + measurement methodology 3,225).
- Dynamic measured surface: 63,323 bytes (`gh pr diff` 51,203 + PR body/comments 8,156 + issue #11832 payload 3,964).
- Total measured review surface: 141,660 bytes.

---

### 🛂 Provenance Audit

Internal Origin: Discussion #11828 / Epic #11831 decomposition and #11832 Sub 6. This is not an external-framework import; it is a Neo substrate utility extracted from existing repo-local parser surfaces and corrected to the repo's `Neo.gatekeep()` utility precedent.

**Findings:** Pass.

---

### 🎯 Close-Target Audit

- [x] Close-targets identified: #11832 via `Resolves #11832` in the PR body.
- [x] #11832 labels verified: `enhancement`, `ai`, `refactoring`, `architecture`; not `epic`.
- [x] Branch commit messages inspected for close-target hazard; no stale `Closes` / `Fixes` / `Resolves` keyword targets beyond the PR body close target.

**Findings:** Pass.

---

### 📑 Contract Completeness Audit

- [x] #11832 contains explicit acceptance criteria for parser API, boolean compatibility, direct import availability, no domain fallback coupling, `Number(undefined)` gotcha, and focused tests.
- [x] PR diff matches the viable contract after maintainer-corrected deltas: `Base`/`setupClass` and `setDeep` were stale AC details, and the PR body documents the replacement with `Neo.gatekeep()` plus removal of `setDeep`.

**Findings:** Pass.

---

### 🪜 Evidence Audit

- [x] PR body contains `Evidence: L1 (focused unit tests + gatekeep namespace registration) → L1 required`.
- [x] Achieved evidence meets close-target needs for a pure additive utility primitive: CI green, focused local unit spec green, direct import smoke check green.
- [x] No L2/L3 runtime rollout is required because existing consumers are intentionally untouched in this PR.

**Findings:** Pass.

---

### 📜 Source-of-Authority Audit

[paraphrase] Current-turn maintainer context confirms the KISS direction for removing the previous `setDeep` / prototype-pollution overreach. The approval does not rely on authority alone: I verified the latest diff, `Neo.gatekeep()` implementation, `IdGenerator` precedent, CI, focused tests, direct import behavior, close target, and commit messages.

**Findings:** Pass.

---

### N/A Audits — 📡 🔌
N/A across listed dimensions: PR #11838 does not modify MCP OpenAPI tool descriptions or JSON-RPC / inter-process wire formats.

---

### 🔗 Cross-Skill Integration Audit

- [x] No skill file or turn-loaded instruction substrate is touched.
- [x] This introduces a code utility, not a new agent workflow convention; no `AGENTS_STARTUP.md` or skill index update is needed.
- [x] Follow-up consumer migration is explicitly deferred to the named Sub 6a/6b/6c cluster rather than silently broadened here.

**Findings:** Pass — no integration gaps.

---

### 🧪 Test-Execution & Location Audit

- [x] Branch checked out locally in repo-local detached review worktree `.codex/review-worktrees/pr-11838` at `df93549ebdb028e4069ed848ee589f9aaf9575b5`.
- [x] Canonical Location: `test/playwright/unit/util/Env.spec.mjs` is the correct unit-test location for a `src/util/Env.mjs` utility.
- [x] Ran `npm run test-unit -- test/playwright/unit/util/Env.spec.mjs` in the review worktree: 20 passed (612ms).
- [x] Direct import smoke check after `src/Neo.mjs` + `src/core/_export.mjs` bootstrap returned `42`, `false`, and `Neo.util.Env === Env` as expected.
- [x] `git diff --check origin/dev...HEAD` passed.

**Findings:** Tests pass.

---

### 🛡️ CI / Security Checks Audit

- [x] Ran `gh pr checks 11838`.
- [x] Confirmed all checks are complete and green: Analyze, CodeQL, integration-unified, lint-pr-body, unit.
- [x] `mergeStateStatus` is `CLEAN` on latest metadata read.

**Findings:** Pass - all checks green.

---

### 📋 Required Actions

No required actions — eligible for human merge.

---

### 📊 Evaluation Metrics
*   **`[ARCH_ALIGNMENT]`**: 95 - 5 points deducted only because the originating ticket AC text is now stale around `Base`/`setupClass`/`setDeep`; the implementation itself uses the correct `Neo.gatekeep()` utility precedent and documents the delta.
*   **`[CONTENT_COMPLETENESS]`**: 94 - 6 points deducted because the branch history still shows earlier heavier iterations, but the current PR body and JSDoc are complete for the shipped surface and accurately explain the KISS correction.
*   **`[EXECUTION_QUALITY]`**: 96 - 4 points deducted for the intentionally deferred consumer migration surface; the current utility has focused unit coverage, green CI, clean diff check, and direct import smoke verification.
*   **`[PRODUCTIVITY]`**: 96 - 4 points deducted because #11832 remains a prerequisite rather than the consumer migration itself; it cleanly unblocks Sub 1 and preserves scope boundaries.
*   **`[IMPACT]`**: 78 - Major substrate-enabling utility for the orchestrator/env-parser refactor path, but intentionally not a full consumer migration or framework-wide behavior change.
*   **`[COMPLEXITY]`**: 34 - Low-to-moderate: three files, one pure utility namespace, one barrel export, one focused unit spec; complexity mostly comes from stale ticket deltas and cross-consumer semantics.
*   **`[EFFORT_PROFILE]`**: Quick Win - high leverage for Sub 1 with a small final diff and focused validation.

Approved for human merge consideration.

---

