diff --git a/backlog/features.json b/backlog/features.json index a84b470..23f8f68 100644 --- a/backlog/features.json +++ b/backlog/features.json @@ -159,12 +159,12 @@ "Flag state change does not require redeploy", "verify.sh green" ], - "status": "pending", + "status": "done", "created_at": "2026-08-14", "gates": { - "review": false, - "security": false, - "qa": false + "review": true, + "security": true, + "qa": true } }, { diff --git a/project/.env.example b/project/.env.example index e473888..bc18ff2 100644 --- a/project/.env.example +++ b/project/.env.example @@ -2,3 +2,8 @@ # Matches docker-compose.yml dev credentials (dev-only, never reuse elsewhere). DATABASE_URL=postgres://mdv:mdv_dev_only@localhost:5432/mercadodevida TEST_DATABASE_URL=postgres://mdv:mdv_dev_only@localhost:5432/mercadodevida_test +REDIS_URL=redis://localhost:6379 + +# Feature flags: FLAG_=true|false (parsed into the flag store at boot). +# Unknown flags default to OFF. Flags can be flipped at runtime via the store. +FLAG_EXAMPLE_FEATURE=false diff --git a/project/README.md b/project/README.md index 765b9cf..a5b064b 100644 --- a/project/README.md +++ b/project/README.md @@ -19,6 +19,17 @@ npm run lint # eslint + prettier check npm run lint:boundaries # module boundary check ``` +## Configuration + +Startup is fail-fast: `src/infrastructure/config` parses env once and refuses to boot +on missing/invalid required vars. `DATABASE_URL` is required; `PORT`, `HOST`, +`LOG_LEVEL`, `NODE_ENV`, `REDIS_URL` are optional with defaults. Errors name variable +NAMES only, never values. + +Feature flags: `FLAG_=true|false` env vars seed the flag store at boot. Unknown +flags default to OFF (fail-safe). Flags flip at runtime through the store — activation +is separate from deployment (no redeploy). Copy `.env.example` to `.env` to start. + ## HTTP contract - Every response carries an `x-request-id` header (propagated from a safe incoming diff --git a/project/src/app/build-app.ts b/project/src/app/build-app.ts index a67f50b..1099c8d 100644 --- a/project/src/app/build-app.ts +++ b/project/src/app/build-app.ts @@ -8,15 +8,24 @@ import Fastify, { type FastifyInstance } from 'fastify'; import type { FastifyError, FastifyReply, FastifyRequest } from 'fastify'; import type { IncomingMessage } from 'node:http'; import { registerHealthRoutes } from '../modules/health/index.js'; +import { createFlagStore, type FeatureFlagProvider } from '../modules/flags/index.js'; import { AppError, errorEnvelope } from '../shared/errors.js'; import { createLogger, type Logger } from '../infrastructure/logging/logger.js'; +declare module 'fastify' { + interface FastifyInstance { + flags: FeatureFlagProvider; + } +} + const REQUEST_ID_HEADER = 'x-request-id'; const SAFE_REQUEST_ID = /^[A-Za-z0-9._-]{1,128}$/; export interface BuildAppDeps { /** Injectable logger so tests can capture structured output. */ logger?: Logger; + /** Feature flags. Default: empty store, every flag OFF (fail-safe). */ + flags?: FeatureFlagProvider; } function generateRequestId(raw: IncomingMessage): string { @@ -33,9 +42,11 @@ function generateRequestId(raw: IncomingMessage): string { */ export async function buildApp(deps: BuildAppDeps = {}): Promise { const logger = deps.logger ?? createLogger(); + const flags = deps.flags ?? createFlagStore(); const startTimes = new WeakMap(); const app = Fastify({ logger: false, genReqId: generateRequestId }); + app.decorate('flags', flags); app.addHook('onRequest', async (request, reply) => { startTimes.set(request, performance.now()); diff --git a/project/src/app/tests/config-flags.test.ts b/project/src/app/tests/config-flags.test.ts new file mode 100644 index 0000000..73e054b --- /dev/null +++ b/project/src/app/tests/config-flags.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest'; +import { buildApp } from '../build-app.js'; +import { createFlagStore } from '../../modules/flags/index.js'; +import { createLogger } from '../../infrastructure/logging/logger.js'; +import type { DestinationStream } from 'pino'; + +function silentLogger() { + const destination: DestinationStream = { write: () => undefined }; + return createLogger({ level: 'info', destination }); +} + +describe('feature flags wiring (composition)', () => { + it('default flags are all OFF when no store is injected', async () => { + const app = await buildApp({ logger: silentLogger() }); + app.get('/__test/guarded', async (request) => { + return { enabled: request.server.flags.isEnabled('risky_path') }; + }); + + const response = await app.inject({ method: 'GET', url: '/__test/guarded' }); + expect(response.json()).toEqual({ enabled: false }); + await app.close(); + }); + + it('guarded path is skipped when the flag is off and runs once enabled at runtime', async () => { + const store = createFlagStore({ risky_path: false }); + const app = await buildApp({ logger: silentLogger(), flags: store }); + app.get('/__test/guarded', async (request) => { + if (!request.server.flags.isEnabled('risky_path')) { + return { path: 'skipped' }; + } + return { path: 'executed' }; + }); + + const off = await app.inject({ method: 'GET', url: '/__test/guarded' }); + expect(off.json()).toEqual({ path: 'skipped' }); + + // Activation without redeploy: same running app instance. + store.setEnabled('risky_path', true); + const on = await app.inject({ method: 'GET', url: '/__test/guarded' }); + expect(on.json()).toEqual({ path: 'executed' }); + + store.setEnabled('risky_path', false); + const offAgain = await app.inject({ method: 'GET', url: '/__test/guarded' }); + expect(offAgain.json()).toEqual({ path: 'skipped' }); + await app.close(); + }); +}); diff --git a/project/src/infrastructure/config/config.ts b/project/src/infrastructure/config/config.ts new file mode 100644 index 0000000..0250c52 --- /dev/null +++ b/project/src/infrastructure/config/config.ts @@ -0,0 +1,97 @@ +/** + * Typed configuration loader. Pure over an env object: no reads of process.env + * here, so tests are deterministic. Fail fast AND clear: every problem is + * collected, then a single ConfigError is thrown. Error messages name variable + * NAMES only, never values, so secrets cannot leak. + */ + +export type StringRecord = Readonly>; + +export type NodeEnv = 'development' | 'test' | 'production'; + +export interface AppConfig { + nodeEnv: NodeEnv; + port: number; + host: string; + logLevel: string; + databaseUrl: string; + redisUrl?: string; + /** Initial flag state parsed from FLAG_* vars. */ + flags: Readonly>; +} + +export class ConfigError extends Error { + constructor(public readonly problems: ReadonlyArray) { + super(`Invalid configuration:\n- ${problems.join('\n- ')}`); + this.name = 'ConfigError'; + } +} + +const NODE_ENVS: ReadonlyArray = ['development', 'test', 'production']; + +function parseBoolFlag(varName: string, raw: string, problems: string[]): boolean | undefined { + const normalized = raw.trim().toLowerCase(); + if (normalized === 'true') return true; + if (normalized === 'false') return false; + problems.push(`${varName} must be "true" or "false"`); + return undefined; +} + +export function loadConfig(env: StringRecord): AppConfig { + const problems: string[] = []; + + const databaseUrl = env.DATABASE_URL; + if (!databaseUrl) { + problems.push('DATABASE_URL is required'); + } + + let port = 3000; + const rawPort = env.PORT; + if (rawPort !== undefined && rawPort !== '') { + const parsed = Number(rawPort); + if (!Number.isInteger(parsed) || parsed < 1 || parsed > 65535) { + problems.push('PORT must be an integer between 1 and 65535'); + } else { + port = parsed; + } + } + + const host = env.HOST && env.HOST !== '' ? env.HOST : '0.0.0.0'; + const logLevel = env.LOG_LEVEL && env.LOG_LEVEL !== '' ? env.LOG_LEVEL : 'info'; + + let nodeEnv: NodeEnv = 'development'; + const rawNodeEnv = env.NODE_ENV; + if (rawNodeEnv !== undefined && rawNodeEnv !== '') { + if ((NODE_ENVS as ReadonlyArray).includes(rawNodeEnv)) { + nodeEnv = rawNodeEnv as NodeEnv; + } else { + problems.push('NODE_ENV must be one of: development, test, production'); + } + } + + const redisUrl = env.REDIS_URL && env.REDIS_URL !== '' ? env.REDIS_URL : undefined; + + const flags: Record = {}; + for (const [key, raw] of Object.entries(env)) { + if (!key.startsWith('FLAG_') || raw === undefined) continue; + const name = key.slice('FLAG_'.length).toLowerCase(); + const value = parseBoolFlag(key, raw, problems); + if (value !== undefined) { + flags[name] = value; + } + } + + if (problems.length > 0) { + throw new ConfigError(problems); + } + + return { + nodeEnv, + port, + host, + logLevel, + databaseUrl: databaseUrl as string, + redisUrl, + flags, + }; +} diff --git a/project/src/infrastructure/config/tests/config.test.ts b/project/src/infrastructure/config/tests/config.test.ts new file mode 100644 index 0000000..ff8e053 --- /dev/null +++ b/project/src/infrastructure/config/tests/config.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it } from 'vitest'; +import { ConfigError, loadConfig } from '../config.js'; + +const BASE = { DATABASE_URL: 'postgres://u:p@localhost:5432/db' }; + +function problemsOf(fn: () => unknown): ReadonlyArray { + try { + fn(); + } catch (error) { + if (error instanceof ConfigError) { + return error.problems; + } + throw error; + } + throw new Error('expected ConfigError was not thrown'); +} + +describe('loadConfig', () => { + it('returns typed config for a valid full env', () => { + const config = loadConfig({ + ...BASE, + PORT: '8080', + HOST: '127.0.0.1', + LOG_LEVEL: 'debug', + NODE_ENV: 'production', + REDIS_URL: 'redis://localhost:6379', + FLAG_NEW_CHECKOUT: 'true', + FLAG_BETA_SEARCH: 'false', + }); + + expect(config.port).toBe(8080); + expect(config.host).toBe('127.0.0.1'); + expect(config.logLevel).toBe('debug'); + expect(config.nodeEnv).toBe('production'); + expect(config.databaseUrl).toBe(BASE.DATABASE_URL); + expect(config.redisUrl).toBe('redis://localhost:6379'); + expect(config.flags).toEqual({ new_checkout: true, beta_search: false }); + }); + + it('applies defaults when optional vars are absent', () => { + const config = loadConfig(BASE); + expect(config.port).toBe(3000); + expect(config.host).toBe('0.0.0.0'); + expect(config.logLevel).toBe('info'); + expect(config.nodeEnv).toBe('development'); + expect(config.redisUrl).toBeUndefined(); + expect(config.flags).toEqual({}); + }); + + it('fails fast naming DATABASE_URL when missing', () => { + expect(() => loadConfig({})).toThrowError(ConfigError); + expect(problemsOf(() => loadConfig({}))).toContain('DATABASE_URL is required'); + }); + + it('rejects invalid PORT values naming the var', () => { + for (const bad of ['not-a-number', '0', '70000', '3.5']) { + expect(() => loadConfig({ ...BASE, PORT: bad })).toThrowError(ConfigError); + expect(problemsOf(() => loadConfig({ ...BASE, PORT: bad })).join(' ')).toContain('PORT'); + } + }); + + it('rejects invalid NODE_ENV', () => { + expect(problemsOf(() => loadConfig({ ...BASE, NODE_ENV: 'staging' })).join(' ')).toContain( + 'NODE_ENV', + ); + }); + + it('rejects FLAG_* values that are not true/false', () => { + expect(problemsOf(() => loadConfig({ ...BASE, FLAG_BROKEN: 'yes' })).join(' ')).toContain( + 'FLAG_BROKEN', + ); + }); + + it('reports every problem in a single ConfigError', () => { + const problems = problemsOf(() => + loadConfig({ PORT: 'nope', NODE_ENV: 'staging', FLAG_BAD: 'maybe' }), + ); + expect(problems.some((p) => p.includes('DATABASE_URL'))).toBe(true); + expect(problems.some((p) => p.includes('PORT'))).toBe(true); + expect(problems.some((p) => p.includes('NODE_ENV'))).toBe(true); + expect(problems.some((p) => p.includes('FLAG_BAD'))).toBe(true); + }); + + it('never echoes secret values in error messages', () => { + let message = ''; + try { + loadConfig({ DATABASE_URL: 'postgres://u:supersecret@h/db', PORT: 'nope' }); + } catch (error) { + message = (error as Error).message; + } + expect(message).not.toContain('supersecret'); + expect(message).toContain('PORT'); + }); +}); diff --git a/project/src/infrastructure/http/server.ts b/project/src/infrastructure/http/server.ts index 82628e8..4cd6fe2 100644 --- a/project/src/infrastructure/http/server.ts +++ b/project/src/infrastructure/http/server.ts @@ -1,14 +1,27 @@ import { buildApp } from '../../app/build-app.js'; +import { ConfigError, loadConfig } from '../config/config.js'; +import { createFlagStore } from '../../modules/flags/index.js'; import { createLogger } from '../logging/logger.js'; -const logger = createLogger(); -const port = Number(process.env.PORT ?? 3000); -const host = process.env.HOST ?? '0.0.0.0'; +let config; +try { + config = loadConfig(process.env); +} catch (error) { + if (error instanceof ConfigError) { + // Config is invalid: fail fast and loud before anything else starts. + console.error(error.message); + } else { + console.error('Failed to load configuration', error); + } + process.exit(1); +} + +const logger = createLogger({ level: config.logLevel }); try { - const app = await buildApp({ logger }); - await app.listen({ port, host }); - logger.info({ port, host }, 'HTTP server listening'); + const app = await buildApp({ logger, flags: createFlagStore(config.flags) }); + await app.listen({ port: config.port, host: config.host }); + logger.info({ port: config.port, host: config.host }, 'HTTP server listening'); } catch (error) { logger.error({ err: error }, 'Failed to start HTTP server'); process.exit(1); diff --git a/project/src/modules/flags/domain/feature-flag-store.ts b/project/src/modules/flags/domain/feature-flag-store.ts new file mode 100644 index 0000000..a8bbc68 --- /dev/null +++ b/project/src/modules/flags/domain/feature-flag-store.ts @@ -0,0 +1,34 @@ +/** + * Feature flags domain. Deployment and activation are separate operations: + * flags can change at runtime without redeploy. Unknown flags are OFF + * (fail-safe default for risky paths). + */ + +export interface FeatureFlagProvider { + isEnabled(name: string): boolean; +} + +export class InMemoryFeatureFlagStore implements FeatureFlagProvider { + private readonly state: Map; + + constructor(initial: Readonly> = {}) { + this.state = new Map( + Object.entries(initial).map(([name, enabled]) => [name.toLowerCase(), enabled]), + ); + } + + isEnabled(name: string): boolean { + return this.state.get(name.toLowerCase()) === true; + } + + /** Runtime mutation: activation does not require a redeploy. */ + setEnabled(name: string, enabled: boolean): void { + this.state.set(name.toLowerCase(), enabled); + } +} + +export function createFlagStore( + initial: Readonly> = {}, +): InMemoryFeatureFlagStore { + return new InMemoryFeatureFlagStore(initial); +} diff --git a/project/src/modules/flags/index.ts b/project/src/modules/flags/index.ts new file mode 100644 index 0000000..440f54d --- /dev/null +++ b/project/src/modules/flags/index.ts @@ -0,0 +1,9 @@ +/** + * Public API of the flags module. Everything the module exposes to the + * outside world goes through this file. + */ +export { + type FeatureFlagProvider, + InMemoryFeatureFlagStore, + createFlagStore, +} from './domain/feature-flag-store.js'; diff --git a/project/src/modules/flags/tests/feature-flag-store.test.ts b/project/src/modules/flags/tests/feature-flag-store.test.ts new file mode 100644 index 0000000..e4d7139 --- /dev/null +++ b/project/src/modules/flags/tests/feature-flag-store.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest'; +import { createFlagStore } from '../index.js'; + +describe('InMemoryFeatureFlagStore', () => { + it('reflects seeded state', () => { + const store = createFlagStore({ new_checkout: true, beta_search: false }); + expect(store.isEnabled('new_checkout')).toBe(true); + expect(store.isEnabled('beta_search')).toBe(false); + }); + + it('defaults unknown flags to OFF (fail-safe)', () => { + const store = createFlagStore(); + expect(store.isEnabled('never_heard_of_it')).toBe(false); + }); + + it('matches flag names case-insensitively', () => { + const store = createFlagStore({ New_Checkout: true }); + expect(store.isEnabled('new_checkout')).toBe(true); + expect(store.isEnabled('NEW_CHECKOUT')).toBe(true); + }); + + it('flips state at runtime without a new store (no redeploy)', () => { + const store = createFlagStore({ risky_path: false }); + expect(store.isEnabled('risky_path')).toBe(false); + + store.setEnabled('risky_path', true); + expect(store.isEnabled('risky_path')).toBe(true); + + store.setEnabled('risky_path', false); + expect(store.isEnabled('risky_path')).toBe(false); + }); + + it('skips a guarded path when the flag is off', () => { + const store = createFlagStore({ risky_path: false }); + let ran = false; + if (store.isEnabled('risky_path')) { + ran = true; + } + expect(ran).toBe(false); + + store.setEnabled('risky_path', true); + if (store.isEnabled('risky_path')) { + ran = true; + } + expect(ran).toBe(true); + }); +}); diff --git a/specs/F-004-config-flags/DESIGN.md b/specs/F-004-config-flags/DESIGN.md new file mode 100644 index 0000000..522fac1 --- /dev/null +++ b/specs/F-004-config-flags/DESIGN.md @@ -0,0 +1,78 @@ +# DESIGN — F-004 Typed config and feature flags + +## Layering +- `src/infrastructure/config/config.ts` — pure config loader. Owns env parsing and + validation. No side effects on `process.env`; takes an env object so tests are + deterministic. Throws a single `ConfigError` listing every problem found. +- `src/modules/flags/` — the feature-flag module. Business-facing, boundary-checked. + Exposes only `index.ts`. Contains the `FeatureFlagProvider` interface and a simple + in-memory store. Does NOT depend on infrastructure/config (kept independent so it + can later be backed by a DB/external service without touching config). + +## Config loader (infrastructure) +```ts +export interface AppConfig { + nodeEnv: 'development' | 'test' | 'production'; + port: number; + host: string; + logLevel: string; + databaseUrl: string; // required + redisUrl?: string; // optional + flags: Record; // parsed FLAG_* vars (initial seed) +} +export class ConfigError extends Error { problems: string[] } +export function loadConfig(env: StringRecord): AppConfig +``` +- `DATABASE_URL` required: missing -> ConfigError naming the var. +- `PORT` parsed to int, validated 1..65535. +- `NODE_ENV` whitelisted, default `development`. +- `FLAG_` vars: value `true`/`false` (case-insensitive) -> boolean; other values + -> ConfigError naming the var. Names lowercased after stripping `FLAG_` prefix. +- Accumulate all problems, then throw once (fail fast AND clear). + +## Feature flag module +```ts +export interface FeatureFlagProvider { + isEnabled(name: string): boolean; +} +export class InMemoryFeatureFlagStore implements FeatureFlagProvider { + constructor(initial?: Record) + isEnabled(name): boolean // unknown flag -> false (fail-safe default) + setEnabled(name, enabled): void // runtime mutation, no redeploy needed +} +export function createFlagStore(initial?): InMemoryFeatureFlagStore +``` +- Unknown flags are OFF by default (fail-safe: risky paths stay skipped). +- `setEnabled` mutates live state: deployment ≠ activation. +- Guarded-path helper kept trivial; consumers do `if (flags.isEnabled('x')) { ... }`. + +## Wiring (composition root) +- `src/app/build-app.ts` stays DB-free for now; but `buildApp` accepts an optional + `flags` dep so routes can guard paths. Default: empty store (all flags off). +- `src/infrastructure/http/server.ts` calls `loadConfig(process.env)`, uses typed + `port`/`host`, and passes `config.flags` into the app. On ConfigError it logs the + problems and exits non-zero. +- `pool.ts` keeps `createPoolFromEnv` but gains an optional typed overload note; no + breaking change needed this ticket (config owns DATABASE_URL validation at startup). + +## Boundary notes +- `flags` module: only imports from `shared` (currently nothing needed). It must not + import `infrastructure/config`. Config parses FLAG_* into a plain record and hands it + to the module at composition time — inversion keeps the module clean. +- Boundary checker will enforce this automatically. + +## Files +| File | Role | +|---|---| +| src/infrastructure/config/config.ts | typed loader + ConfigError | +| src/infrastructure/config/tests/config.test.ts | loader unit tests | +| src/modules/flags/index.ts | module public API | +| src/modules/flags/domain/feature-flag-store.ts | interface + store | +| src/modules/flags/tests/feature-flag-store.test.ts | store unit tests | +| src/app/build-app.ts | optional flags dep injection | +| src/infrastructure/http/server.ts | loadConfig + typed listen + ConfigError exit | + +## Risks / mitigations +- Silent flag default: unknown -> false, documented; tested. +- ConfigError leaking secrets: messages name var NAMES only, never values. +- Boundary violation (flags importing config): prevented by composition-time injection + checker. diff --git a/specs/F-004-config-flags/SPEC.md b/specs/F-004-config-flags/SPEC.md new file mode 100644 index 0000000..96ae65d --- /dev/null +++ b/specs/F-004-config-flags/SPEC.md @@ -0,0 +1,41 @@ +# SPEC — F-004 Typed config and feature flags + +## Problem +Risky features need activation separate from deployment; env access must be typed. +Today env access is ad-hoc (`process.env.X ?? fallback` scattered in entrypoints) +and there is no activation mechanism distinct from deploy. + +## Goal +Fail-fast typed config plus a simple feature flag module behind an interface. + +## Scope IN +- Typed env config loader in `src/infrastructure/config`, fail fast on missing/invalid required vars (all problems reported in one clear message) +- `FeatureFlagProvider` interface with simple in-memory store implementation (`src/modules/flags`) +- Deployment ≠ activation: flags seed from `FLAG_*` env vars at boot and can be mutated at runtime without redeploy +- DB pool refactor to consume config instead of sniffing env itself + +## Scope OUT +- No external flag service +- No per-user segmentation +- No admin HTTP endpoint for flags yet (first consumer decides shape; store API is runtime-mutable already) + +## Required vs optional vars +| Var | Required | Default | +|---|---|---| +| DATABASE_URL | yes | — | +| PORT | no | 3000 (valid: integer 1–65535) | +| HOST | no | 0.0.0.0 | +| LOG_LEVEL | no | info | +| NODE_ENV | no | development (valid: development/test/production) | +| REDIS_URL | no | undefined | +| FLAG_ | no | parsed as true/false | + +## Acceptance criteria +1. Given a missing required env var When the app starts Then startup fails with a clear message naming the var. +2. Given flag off When a code path guarded by the flag runs Then the path is skipped. +3. Flag state change does not require redeploy (runtime `setEnabled` on the store). +4. `./scripts/verify.sh` green. + +## Non-functional +- No new dependencies: hand-rolled validation is small, explicit and boring. +- Config loader is a pure function of an env object (testable without touching process.env). diff --git a/specs/F-004-config-flags/TASKS.md b/specs/F-004-config-flags/TASKS.md new file mode 100644 index 0000000..0e3eb1d --- /dev/null +++ b/specs/F-004-config-flags/TASKS.md @@ -0,0 +1,10 @@ +# TASKS — F-004 Typed config and feature flags + +- [ ] TASK-001 src/infrastructure/config/config.ts: AppConfig, ConfigError, loadConfig (pure over env object) +- [ ] TASK-002 src/infrastructure/config/tests/config.test.ts: required/optional/invalid cases +- [ ] TASK-003 src/modules/flags/domain/feature-flag-store.ts: FeatureFlagProvider + InMemoryFeatureFlagStore + createFlagStore +- [ ] TASK-004 src/modules/flags/index.ts: public API +- [ ] TASK-005 src/modules/flags/tests/feature-flag-store.test.ts: guard + runtime mutation +- [ ] TASK-006 src/app/build-app.ts: optional flags dep +- [ ] TASK-007 src/infrastructure/http/server.ts: loadConfig, typed listen, ConfigError exit +- [ ] TASK-008 run lint + boundaries + typecheck + build + test + verify.sh diff --git a/specs/F-004-config-flags/TESTS.md b/specs/F-004-config-flags/TESTS.md new file mode 100644 index 0000000..c9a3024 --- /dev/null +++ b/specs/F-004-config-flags/TESTS.md @@ -0,0 +1,22 @@ +# TESTS — F-004 Typed config and feature flags + +## Unit — src/infrastructure/config/tests/config.test.ts +1. Valid full env -> typed AppConfig (port number, nodeEnv enum, flags parsed) +2. Missing DATABASE_URL -> ConfigError naming DATABASE_URL +3. Invalid PORT (non-numeric / out of range) -> ConfigError naming PORT +4. Invalid NODE_ENV -> ConfigError +5. FLAG_* true/false parsed; unknown flag value -> ConfigError naming the var +6. Multiple problems -> single ConfigError listing all of them + +## Unit — src/modules/flags/tests/feature-flag-store.test.ts +1. Known flag on -> isEnabled true; known flag off -> false +2. Unknown flag -> false (fail-safe) +3. setEnabled flips state at runtime (no new store / no redeploy) +4. createFlagStore seeds initial state + +## Composition +5. buildApp with flags dep: guarded path skipped when flag off, runs when on + +## Manual / QA +- Start server without DATABASE_URL -> clear error naming the var, non-zero exit. +- verify.sh green. diff --git a/work/artifacts/F-004/architect.md b/work/artifacts/F-004/architect.md new file mode 100644 index 0000000..e7883ab --- /dev/null +++ b/work/artifacts/F-004/architect.md @@ -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. diff --git a/work/artifacts/F-004/documenter.md b/work/artifacts/F-004/documenter.md new file mode 100644 index 0000000..bcef4d1 --- /dev/null +++ b/work/artifacts/F-004/documenter.md @@ -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. diff --git a/work/artifacts/F-004/implementer.md b/work/artifacts/F-004/implementer.md new file mode 100644 index 0000000..3cd3227 --- /dev/null +++ b/work/artifacts/F-004/implementer.md @@ -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. diff --git a/work/artifacts/F-004/leader-close.json b/work/artifacts/F-004/leader-close.json new file mode 100644 index 0000000..a82f1df --- /dev/null +++ b/work/artifacts/F-004/leader-close.json @@ -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" +} diff --git a/work/artifacts/F-004/qa.json b/work/artifacts/F-004/qa.json new file mode 100644 index 0000000..de947d4 --- /dev/null +++ b/work/artifacts/F-004/qa.json @@ -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" +} diff --git a/work/artifacts/F-004/reviewer.json b/work/artifacts/F-004/reviewer.json new file mode 100644 index 0000000..71ba73b --- /dev/null +++ b/work/artifacts/F-004/reviewer.json @@ -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" +} diff --git a/work/artifacts/F-004/security.json b/work/artifacts/F-004/security.json new file mode 100644 index 0000000..3aa2277 --- /dev/null +++ b/work/artifacts/F-004/security.json @@ -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" +} diff --git a/work/current.md b/work/current.md index fad2395..f489a3b 100644 --- a/work/current.md +++ b/work/current.md @@ -1,17 +1,17 @@ # Sesión actual -- Feature en curso: _ninguna_ (F-003 cerrada DONE el 2026-08-14) +- Feature en curso: _ninguna_ (F-004 cerrada DONE el 2026-08-14) - Inicio: — - Orquestador: — ## Plan -- Hechas: F-001, F-002, F-003. -- Desbloqueadas ahora: F-004 (config/flags, depende de F-001) y F-005 (identity, depende de F-002+F-003, ambas done). -- Sugerencia de orden: F-004 → F-005 (identity necesita config/flags limpio para secrets). +- Hechas: F-001, F-002, F-003, F-004. +- Desbloqueada: F-005 (identity & auth core) — depende de F-002+F-003, ambas done. Config/flags (F-004) ya listos para secrets y guards. +- Sugerencia de orden: F-005 ahora. ## Bitácora -- 2026-08-14: F-001, F-002 y F-003 DONE; todos los gates APPROVED; verify.sh verde. -- Dev PostgreSQL + Redis siguen corriendo (mdv-dev-postgres, mdv-dev-redis) para los próximos tickets. +- 2026-08-14: F-001..F-004 DONE; todos los gates APPROVED; verify.sh verde. +- Dev PostgreSQL + Redis siguen corriendo (mdv-dev-postgres, mdv-dev-redis). ## Próximo paso -- intake de F-004 (config and feature flags). +- intake de F-005 (identity & auth core). diff --git a/work/history.md b/work/history.md index b2958cb..cbd2d99 100644 --- a/work/history.md +++ b/work/history.md @@ -19,3 +19,10 @@ - Entregable: request_id (generado o propagado-sanitizado), logs JSON correlacionados, error envelope v2 con requestId, hook de validación parseJson (zod), server con logging inyectable - Nota: doc stage rebotó a build para escribir README (project/ está gateado a build/implementer/running); comportamiento correcto del guardrail - Artefactos: work/artifacts/F-003/ + +## 2026-08-14 — F-004 Typed config and feature flags — DONE +- Gates: reviewer APPROVED, security APPROVED, qa APPROVED, verify.sh exit 0 +- Entregable: loadConfig fail-fast (DATABASE_URL requerido, errores solo con nombres de variables), módulo flags tras FeatureFlagProvider (default OFF, flip en runtime sin redeploy), app.flags decorado +- Nota: los tests detectaron un bug real en build (case-normalization del store); corregido antes de gates con test de regresión +- Cero dependencias nuevas +- Artefactos: work/artifacts/F-004/ diff --git a/work/runtime-status.json b/work/runtime-status.json index 9ab1de2..cce496d 100644 --- a/work/runtime-status.json +++ b/work/runtime-status.json @@ -6,6 +6,6 @@ "state": "waiting", "next_agent": "leader", "waiting_for": "Seleccionar una feature pending y actualizar este estado", - "updated_at": "2026-08-14T20:13:28Z", + "updated_at": "2026-08-14T20:29:18Z", "timeline": [] }