feat(F-004): typed fail-fast config and feature flag module
- loadConfig: pure over env object, accumulates all problems, names var names only - DATABASE_URL now required at startup; PORT/HOST/LOG_LEVEL/NODE_ENV/REDIS_URL defaulted - flags module behind FeatureFlagProvider; unknown flags OFF; runtime setEnabled (no redeploy) - buildApp decorates app.flags; server.ts fail-fast before app boot - tests caught and fixed flag-store case-normalization bug before gates - zero new dependencies; all gates approved; verify.sh green
This commit is contained in:
33
work/artifacts/F-004/architect.md
Normal file
33
work/artifacts/F-004/architect.md
Normal file
@@ -0,0 +1,33 @@
|
||||
# Architect — F-004 Typed config and feature flags
|
||||
|
||||
done -> work/artifacts/F-004/architect.md
|
||||
|
||||
## Deliverables
|
||||
- specs/F-004-config-flags/SPEC.md
|
||||
- specs/F-004-config-flags/DESIGN.md
|
||||
- specs/F-004-config-flags/TASKS.md
|
||||
- specs/F-004-config-flags/TESTS.md
|
||||
|
||||
## Key decisions
|
||||
1. **Config loader lives in `infrastructure/config`** and is a pure function over an env
|
||||
object. No reads of `process.env` inside the loader → deterministic unit tests. It
|
||||
accumulates every problem and throws ONE `ConfigError` (fail fast AND clear). It names
|
||||
var NAMES only — never values — so secrets cannot leak into error messages.
|
||||
2. **Feature flags are a real module (`src/modules/flags`)** behind a
|
||||
`FeatureFlagProvider` interface with an in-memory store. Unknown flags default to OFF
|
||||
(fail-safe). `setEnabled` mutates live state → activation ≠ deployment, no redeploy.
|
||||
3. **Inversion to keep the module clean:** `infrastructure/config` parses `FLAG_*` into a
|
||||
plain record and the composition root hands it to the flags store. The flags module never
|
||||
imports config. The boundary checker enforces this.
|
||||
4. **buildApp gains an optional `flags` dep**, decorated onto the instance (`app.flags`),
|
||||
establishing the wiring point future risky routes will consume. Default: empty store →
|
||||
everything off.
|
||||
5. **No new dependencies.** Hand-rolled validation is small and boring.
|
||||
|
||||
## Boundaries
|
||||
- `flags` module imports nothing outside its subtree (no shared needed). Config → flags
|
||||
only via composition root. server.ts pulls config + passes flags into buildApp.
|
||||
|
||||
## Risks
|
||||
- server now requires DATABASE_URL to start (intentional fail-fast). Smoke/QA must pass it.
|
||||
- ConfigError must not echo values. Covered by name-only messages + test.
|
||||
19
work/artifacts/F-004/documenter.md
Normal file
19
work/artifacts/F-004/documenter.md
Normal file
@@ -0,0 +1,19 @@
|
||||
# Documenter — F-004 Typed config and feature flags
|
||||
|
||||
done -> work/artifacts/F-004/documenter.md
|
||||
|
||||
## Why documentation applies
|
||||
F-004 changes the startup contract (DATABASE_URL now required, fail-fast behavior) and introduces the flag mechanism every future risky feature will use.
|
||||
|
||||
## Documents reviewed/produced
|
||||
- `project/README.md`: "Configuration" section added at build stage — required/optional vars, fail-fast behavior, FLAG_* semantics, fail-safe defaults, activation ≠ deployment.
|
||||
- `.env.example`: REDIS_URL + FLAG_EXAMPLE_FEATURE added with explanatory comments.
|
||||
- `spec/tech.md`: no new dependency rows needed — ticket added zero dependencies by design (noted for audit trail).
|
||||
- `specs/F-004-config-flags/` SPEC/DESIGN/TASKS/TESTS consistent with implementation.
|
||||
|
||||
## Verification
|
||||
- README Configuration section cross-checked against config.ts behavior (required DATABASE_URL; PORT/HOST/LOG_LEVEL/NODE_ENV/REDIS_URL defaults; FLAG_* parsing).
|
||||
- Startup contract change (DATABASE_URL required) is called out explicitly so operators are not surprised.
|
||||
|
||||
## Pending docs (not this ticket)
|
||||
- Flag admin transport documented when first consumer lands.
|
||||
44
work/artifacts/F-004/implementer.md
Normal file
44
work/artifacts/F-004/implementer.md
Normal file
@@ -0,0 +1,44 @@
|
||||
# Implementer — F-004 Typed config and feature flags
|
||||
|
||||
done -> work/artifacts/F-004/implementer.md
|
||||
|
||||
## Files created
|
||||
- src/infrastructure/config/config.ts (loadConfig pure over env object; ConfigError accumulates all problems; names var names only)
|
||||
- src/infrastructure/config/tests/config.test.ts (8 tests)
|
||||
- src/modules/flags/domain/feature-flag-store.ts (FeatureFlagProvider + InMemoryFeatureFlagStore + createFlagStore)
|
||||
- src/modules/flags/index.ts (module public API)
|
||||
- src/modules/flags/tests/feature-flag-store.test.ts (5 tests)
|
||||
- src/app/tests/config-flags.test.ts (2 composition tests: default OFF, runtime flip without redeploy)
|
||||
|
||||
## Files modified
|
||||
- src/app/build-app.ts: optional flags dep; `app.flags` decorated on the instance (typed via module augmentation)
|
||||
- src/infrastructure/http/server.ts: loadConfig fail-fast before anything starts; typed port/host/logLevel; flags seeded from FLAG_* env
|
||||
- .env.example: REDIS_URL + FLAG_EXAMPLE_FEATURE example
|
||||
- README.md: Configuration section
|
||||
|
||||
## API changes
|
||||
- Server now REQUIRES DATABASE_URL at startup (fail-fast; intentional per spec).
|
||||
|
||||
## Bug found by tests during build (fixed before gates)
|
||||
- Flag store constructor did not lowercase keys while isEnabled lowercased lookups -> case-insensitive seed test failed. Fixed by normalizing keys in the constructor. Regression test retained.
|
||||
|
||||
## Tests passed (evidence)
|
||||
```
|
||||
npm run lint -> OK
|
||||
npm run lint:boundaries -> Boundary check OK: 22 file(s) checked (flags module imports nothing outside its subtree)
|
||||
npm run typecheck -> exit 0
|
||||
npm run build -> exit 0
|
||||
npm test -> 9 files passed, 2 skipped (integration without DB); 38 passed | 6 skipped
|
||||
npm run test:integration-> 2 files, 6 passed (no regression)
|
||||
live smoke:
|
||||
env -u DATABASE_URL node dist/.../server.js -> "Invalid configuration:\n- DATABASE_URL is required", exit 1 (AC1)
|
||||
with DATABASE_URL + FLAG_EXAMPLE_FEATURE=true -> health=200, "HTTP server listening"
|
||||
./scripts/verify.sh -> exit 0
|
||||
```
|
||||
|
||||
## Known limitations
|
||||
- No admin endpoint to flip flags yet (scope out); store API is runtime-mutable so the first consumer only adds transport.
|
||||
- Flags live in memory: restart reseeds from env (documented as expected; persistence is a later ticket if needed).
|
||||
|
||||
## Follow-up work
|
||||
- F-005 identity consumes config.databaseUrl through the pool and can guard risky paths via app.flags.
|
||||
33
work/artifacts/F-004/leader-close.json
Normal file
33
work/artifacts/F-004/leader-close.json
Normal file
@@ -0,0 +1,33 @@
|
||||
{
|
||||
"feature_id": "F-004",
|
||||
"agent": "leader",
|
||||
"verdict": "APPROVED",
|
||||
"summary": "F-004 closed. Typed fail-fast config and a boundary-clean feature flag module behind an interface. Deployment ≠ activation proven at runtime. All gates APPROVED, verify.sh exit 0, zero new dependencies.",
|
||||
"gates": {
|
||||
"reviewer": "APPROVED (reviewer.json)",
|
||||
"security": "APPROVED (security.json)",
|
||||
"qa": "APPROVED (qa.json)",
|
||||
"verify_sh": "exit 0"
|
||||
},
|
||||
"deliverables": [
|
||||
"src/infrastructure/config/config.ts (pure loader, accumulated problems, name-only errors)",
|
||||
"src/modules/flags (FeatureFlagProvider + InMemoryFeatureFlagStore, fail-safe OFF default)",
|
||||
"buildApp flags dep decorated as app.flags; server.ts fail-fast startup",
|
||||
"15 new tests incl. runtime flip without redeploy and secret non-leakage",
|
||||
"README Configuration section + .env.example FLAG_* example"
|
||||
],
|
||||
"process_notes": [
|
||||
"Tests caught a real bug during build (flag store case normalization); fixed before gates with regression test retained. Exactly the loop we want."
|
||||
],
|
||||
"next_feature_hint": "F-005 identity now has all dependencies done (F-002, F-003) plus config/flags ready -> recommended next",
|
||||
"evidence": [
|
||||
"work/artifacts/F-004/architect.md",
|
||||
"work/artifacts/F-004/implementer.md",
|
||||
"work/artifacts/F-004/reviewer.json",
|
||||
"work/artifacts/F-004/security.json",
|
||||
"work/artifacts/F-004/qa.json",
|
||||
"work/artifacts/F-004/documenter.md",
|
||||
"./scripts/verify.sh exit 0 at close"
|
||||
],
|
||||
"timestamp": "2026-08-14T20:30:00Z"
|
||||
}
|
||||
35
work/artifacts/F-004/qa.json
Normal file
35
work/artifacts/F-004/qa.json
Normal file
@@ -0,0 +1,35 @@
|
||||
{
|
||||
"feature_id": "F-004",
|
||||
"agent": "qa",
|
||||
"verdict": "APPROVED",
|
||||
"summary": "All 4 acceptance criteria verified with fresh executions (live process + fresh test runs).",
|
||||
"traceability": [
|
||||
{
|
||||
"criterion": "AC1: missing required env var -> startup fails with clear message",
|
||||
"test": "fresh live run: env -u DATABASE_URL node dist/infrastructure/http/server.js",
|
||||
"result": "PASS (prints 'Invalid configuration: - DATABASE_URL is required', exit code 1)"
|
||||
},
|
||||
{
|
||||
"criterion": "AC2: flag off -> guarded path skipped",
|
||||
"test": "feature-flag-store.test.ts 'skips a guarded path when the flag is off' + config-flags.test.ts",
|
||||
"result": "PASS"
|
||||
},
|
||||
{
|
||||
"criterion": "AC3: flag state change does not require redeploy",
|
||||
"test": "'flips state at runtime without a new store' + composition 'runs once enabled at runtime' on the SAME app instance",
|
||||
"result": "PASS"
|
||||
},
|
||||
{
|
||||
"criterion": "AC4: verify.sh green",
|
||||
"test": "./scripts/verify.sh",
|
||||
"result": "PASS (exit 0)"
|
||||
}
|
||||
],
|
||||
"regressions": "PASS - full unit suite 38 passed | 6 skipped; F-002 integration 6 passed; /health returns 200 under valid config",
|
||||
"evidence": [
|
||||
"fresh live fail-fast run (exit 1, clear message naming DATABASE_URL)",
|
||||
"fresh vitest run of flags unit + composition tests: 7 passed",
|
||||
"./scripts/verify.sh exit 0"
|
||||
],
|
||||
"timestamp": "2026-08-14T20:28:00Z"
|
||||
}
|
||||
26
work/artifacts/F-004/reviewer.json
Normal file
26
work/artifacts/F-004/reviewer.json
Normal file
@@ -0,0 +1,26 @@
|
||||
{
|
||||
"feature_id": "F-004",
|
||||
"agent": "reviewer",
|
||||
"verdict": "APPROVED",
|
||||
"summary": "Typed config + feature flags match specs/F-004 DESIGN.md. Loader is pure and fail-fast, flags module is boundary-clean behind an interface, activation is decoupled from deployment. No regressions.",
|
||||
"checks": {
|
||||
"design_conformance": "PASS: loadConfig pure over env object (no process.env reads), ConfigError accumulates all problems, FLAG_* parsed into a record handed to the store at composition time",
|
||||
"boundary_rules": "PASS: lint:boundaries clean over 22 files; flags module imports only its own subtree (vitest in tests only); flags never imports config (inversion upheld)",
|
||||
"fail_safe_defaults": "PASS: unknown flag -> OFF; missing optional vars -> sane defaults; missing DATABASE_URL -> startup aborts before app is built",
|
||||
"test_coverage": "PASS: 15 new tests map to specs/F-004 TESTS.md incl. multi-problem aggregation and secret non-leakage; composition test proves runtime flip without redeploy",
|
||||
"regression": "PASS: unit 38 passed | 6 skipped; F-002 integration 6 passed; /health unaffected"
|
||||
},
|
||||
"findings": [
|
||||
{
|
||||
"severity": "info",
|
||||
"note": "Flags are in-memory only; restart reseeds from env. Documented as expected for this ticket; persistence is a later concern if ever needed."
|
||||
}
|
||||
],
|
||||
"evidence": [
|
||||
"npm run lint / typecheck / lint:boundaries (22 files) / test / test:integration -> all green",
|
||||
"grep: flags module has no imports outside its subtree; config.ts has no process.env read",
|
||||
"live smoke at build stage: missing DATABASE_URL -> exit 1 with clear message; valid env -> listening + health 200",
|
||||
"files reviewed: src/infrastructure/config/**, src/modules/flags/**, src/app/build-app.ts, src/infrastructure/http/server.ts, src/app/tests/config-flags.test.ts"
|
||||
],
|
||||
"timestamp": "2026-08-14T20:24:00Z"
|
||||
}
|
||||
22
work/artifacts/F-004/security.json
Normal file
22
work/artifacts/F-004/security.json
Normal file
@@ -0,0 +1,22 @@
|
||||
{
|
||||
"feature_id": "F-004",
|
||||
"agent": "security",
|
||||
"verdict": "APPROVED",
|
||||
"summary": "Security gate passed. No new dependencies (0 vulnerabilities), no dangerous patterns, config errors name variable NAMES only (secret leakage structurally impossible), .env hygiene intact.",
|
||||
"checks": {
|
||||
"dependencies": "PASS: npm audit -> 0 vulnerabilities; ticket added zero dependencies by design",
|
||||
"secret_leakage": "PASS: every problems.push uses var names/static text only; dedicated unit test asserts a secret in DATABASE_URL never appears in the thrown message",
|
||||
"dangerous_patterns": "PASS: no eval / new Function / child_process in new code",
|
||||
"env_hygiene": "PASS: .env gitignored; .env.example trackable and carries only dev-only/public values",
|
||||
"input_surfaces": "PASS: PORT integer-range validated; FLAG_* parsed to booleans only (no string passthrough); unknown flags fail-safe OFF so risky paths stay skipped"
|
||||
},
|
||||
"findings": [],
|
||||
"evidence": [
|
||||
"npm audit -> found 0 vulnerabilities",
|
||||
"grep eval|new Function|child_process over config+flags -> none",
|
||||
"grep problems.push -> name-only messages (4 sites inspected)",
|
||||
"git check-ignore .env -> ignored; .env.example -> tracked",
|
||||
"unit test 'never echoes secret values in error messages' passes"
|
||||
],
|
||||
"timestamp": "2026-08-14T20:26:00Z"
|
||||
}
|
||||
Reference in New Issue
Block a user