William Valentin
19287c4cad
- SUMMARY.md with 2 task commits, 10 new tests, 3 min duration - STATE.md updated: Phase 2 in_progress, 02-01 done - ROADMAP.md updated: 02-01 checked off
114 lines
4.5 KiB
Markdown
114 lines
4.5 KiB
Markdown
---
|
|
phase: 02-config-overlays
|
|
plan: 01
|
|
subsystem: config
|
|
tags: [yaml, deep-merge, overlay, environment-config, FLYNN_ENV]
|
|
|
|
# Dependency graph
|
|
requires:
|
|
- phase: 01-daemon-decomposition
|
|
provides: decomposed daemon modules with config loader at src/config/loader.ts
|
|
provides:
|
|
- deepMerge utility for recursive object merging
|
|
- overlay-aware loadConfig (optional overlayPath parameter)
|
|
- resolveOverlayPath for FLYNN_ENV → overlay file resolution
|
|
- overlay-aware loadConfigSafe for all CLI commands
|
|
affects: [02-config-overlays, 03-live-ops-dashboard]
|
|
|
|
# Tech tracking
|
|
tech-stack:
|
|
added: []
|
|
patterns: [deep-merge-before-validation, optional-overlay-parameter, env-to-file-resolution]
|
|
|
|
key-files:
|
|
created: []
|
|
modified:
|
|
- src/config/loader.ts
|
|
- src/config/loader.test.ts
|
|
- src/config/index.ts
|
|
- src/cli/shared.ts
|
|
|
|
key-decisions:
|
|
- "Merge happens before env var expansion and Zod validation — overlays don't need required fields"
|
|
- "Arrays in overlays replace base arrays (not concatenated) — explicit override semantics"
|
|
- "resolveOverlayPath does NOT check file existence — callers decide error handling (loadConfig throws, doctor reports)"
|
|
- "Env var expansion test uses string field (model name) not number field (port) since YAML env vars expand to strings"
|
|
|
|
patterns-established:
|
|
- "Optional overlay parameter: loadConfig(base, overlay?) — backward compatible by default"
|
|
- "Environment resolution: FLYNN_ENV maps to {configDir}/{env}.yaml sibling file"
|
|
|
|
# Metrics
|
|
duration: 3min
|
|
completed: 2026-02-10
|
|
---
|
|
|
|
# Phase 2 Plan 1: Core Overlay Merge Summary
|
|
|
|
**deepMerge utility + overlay-aware loadConfig + FLYNN_ENV resolution with 10 new tests proving merge behavior and backward compatibility**
|
|
|
|
## Performance
|
|
|
|
- **Duration:** 3 min
|
|
- **Started:** 2026-02-10T04:54:19Z
|
|
- **Completed:** 2026-02-10T04:57:21Z
|
|
- **Tasks:** 2
|
|
- **Files modified:** 4
|
|
|
|
## Accomplishments
|
|
- deepMerge function with recursive object merging (arrays replace, null overwrites, 3+ level nesting)
|
|
- loadConfig accepts optional overlayPath — merges before env expansion and Zod validation
|
|
- resolveOverlayPath maps FLYNN_ENV to sibling YAML file path
|
|
- loadConfigSafe automatically passes overlay to loadConfig — all CLI commands get overlay support
|
|
- 10 new tests (6 deepMerge unit + 4 overlay integration), all 1087 tests still pass
|
|
|
|
## Task Commits
|
|
|
|
Each task was committed atomically:
|
|
|
|
1. **Task 1: Implement deepMerge and overlay-aware loadConfig with tests** - `c2cc052` (feat)
|
|
2. **Task 2: Wire FLYNN_ENV resolution into shared.ts and update loadConfigSafe** - `29bc185` (feat)
|
|
|
|
## Files Created/Modified
|
|
- `src/config/loader.ts` - Added deepMerge export + overlay-aware loadConfig with optional overlayPath
|
|
- `src/config/loader.test.ts` - Added 10 new tests: 6 for deepMerge, 4 for overlay integration
|
|
- `src/config/index.ts` - Re-exports deepMerge
|
|
- `src/cli/shared.ts` - Added resolveOverlayPath + updated loadConfigSafe to pass overlay
|
|
|
|
## Decisions Made
|
|
- Merge before validation: overlay files don't need to repeat required base fields (telegram, models)
|
|
- Arrays replace (not concat): `allowed_chat_ids: [999]` in overlay replaces base `[123]`, not appends
|
|
- resolveOverlayPath returns path even if file doesn't exist — loadConfig throws on missing file, doctor can report gracefully
|
|
- Used string field (model name) for env var expansion test since YAML `${VAR}` produces strings, not numbers
|
|
|
|
## Deviations from Plan
|
|
|
|
### Auto-fixed Issues
|
|
|
|
**1. [Rule 1 - Bug] Fixed env var expansion test using number field**
|
|
- **Found during:** Task 1 (GREEN phase)
|
|
- **Issue:** Plan specified testing `server.port: ${TEST_OVERLAY_PORT}` but env vars expand to strings, and Zod `z.number()` rejects string "7777"
|
|
- **Fix:** Changed test to use `models.default.model: ${TEST_OVERLAY_MODEL}` (string field) instead of port (number field)
|
|
- **Files modified:** src/config/loader.test.ts
|
|
- **Verification:** All 13 tests in loader.test.ts pass
|
|
- **Committed in:** c2cc052 (Task 1 commit)
|
|
|
|
---
|
|
|
|
**Total deviations:** 1 auto-fixed (1 bug in test spec)
|
|
**Impact on plan:** Minor test adjustment. The env var expansion in overlay is still fully tested, just using a string-typed field instead of a number-typed field.
|
|
|
|
## Issues Encountered
|
|
None
|
|
|
|
## User Setup Required
|
|
None - no external service configuration required.
|
|
|
|
## Next Phase Readiness
|
|
- Core overlay merge complete, ready for Plan 02-02 (doctor overlay validation check)
|
|
- All existing CLI commands automatically get overlay support via loadConfigSafe
|
|
|
|
---
|
|
*Phase: 02-config-overlays*
|
|
*Completed: 2026-02-10*
|