Development - #38
Conversation
There was a problem hiding this comment.
Pull request overview
This PR strengthens baseline reliability defaults (HTTP server, Redis, queues, DB retries), consolidates shared utilities, and reduces duplication/complexity across worker and admin persistence code while adding CI gates to keep scaffolding generation healthy.
Changes:
- Add configurable reliability defaults for Fastify (timeouts/body limit), Redis (timeouts/retries), BullMQ (job/worker defaults), and Prisma transaction retries (exponential backoff + jitter).
- Introduce shared helpers (
asNonEmptyString, time adders) and improve error-code typing/validation across HTTP problem details and feature error mapping. - Refactor large/duplicated implementations (emails worker handlers split out; admin Prisma list-query builders extracted) and add a scaffold smoke test (also enforced in CI).
Reviewed changes
Copilot reviewed 67 out of 68 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/scaffold-feature.ts | Adjusts generated feature scaffolds (type-only imports, DI style, queue naming/jobId helpers). |
| tools/architecture-smells.baseline.json | Updates architecture-smells baseline snapshot. |
| scripts/scaffold-smoke.ts | Adds a smoke test that scaffolds a feature and runs lint/typecheck/deps checks, then cleans up. |
| package.json | Adds scaffold:smoke script and dependency overrides. |
| libs/shared/time.ts | Adds addMilliseconds/addSeconds/addDays helpers. |
| libs/shared/time.spec.ts | Adds unit tests for shared time helpers and SystemClock. |
| libs/shared/string.ts | Introduces shared asNonEmptyString helper. |
| libs/shared/app-error-codes.ts | Adds app-wide error-code value set + isAppErrorCode guard. |
| libs/platform/storage/object-storage.service.ts | Reuses shared asNonEmptyString instead of local helper. |
| libs/platform/redis/redis.service.ts | Adds Redis reliability defaults + env overrides for timeouts/retries/offline queue. |
| libs/platform/redis/redis.service.spec.ts | Tests Redis reliability defaults, env overrides, and invalid settings behavior. |
| libs/platform/queue/trace-propagation.spec.ts | Asserts worker creation uses default worker options. |
| libs/platform/queue/queue.worker.ts | Applies DEFAULT_WORKER_OPTIONS when creating BullMQ workers. |
| libs/platform/queue/queue.defaults.ts | Adds backoff jitter and introduces default worker options. |
| libs/platform/push/fcm-push.service.ts | Reuses shared asNonEmptyString instead of local helper. |
| libs/platform/http/request-id.ts | Reuses shared asNonEmptyString instead of local helper. |
| libs/platform/http/idempotency/idempotency.interceptor.ts | Reuses shared asNonEmptyString instead of local helper. |
| libs/platform/http/idempotency/idempotency.core.ts | Re-exports shared asNonEmptyString to preserve existing API surface. |
| libs/platform/http/filters/problem-details.filter.ts | Tightens code typing via AppErrorCode and validates incoming codes. |
| libs/platform/http/filters/problem-details.filter.spec.ts | Adds test for unknown code fallback behavior. |
| libs/platform/http/filters/feature-error.mapper.ts | Updates mapping types to use AppErrorCode for feature errors. |
| libs/platform/http/fastify-adapter.ts | Adds configurable Fastify timeouts/body limit/plugin timeout with validation. |
| libs/platform/http/fastify-adapter.spec.ts | Tests Fastify defaults, env overrides, and invalid env behavior. |
| libs/platform/http/errors/problem.exception.ts | Constrains ProblemException code to AppErrorCode. |
| libs/platform/email/email.service.ts | Reuses shared asNonEmptyString instead of local helper. |
| libs/platform/db/tx-retry.ts | Adds exponential backoff + jitter between retry attempts (configurable). |
| libs/platform/db/tx-retry.spec.ts | Adds tests for backoff, jitter, caps, and “no sleep when maxAttempts=1”. |
| libs/platform/config/env.validation.ts | Adds Redis config invariants validation. |
| libs/platform/config/env.validation.spec.ts | Adds validation tests for new HTTP and Redis env vars/invariants. |
| libs/platform/config/env.schema.http.ts | Adds schema fields/defaults for HTTP timeout/body-limit config. |
| libs/platform/config/env.schema.db.ts | Adds schema fields/defaults for Redis timeout/retry/offline-queue config. |
| libs/platform/config/env.runtime.ts | Reuses shared asNonEmptyString instead of local helper. |
| libs/platform/config/env.invariants.ts | Adds Redis retry max>=base invariant enforcement. |
| libs/platform/config/auth-password-policy.ts | Centralizes resolution of auth password min length from env schema defaults/validation. |
| libs/platform/config/auth-password-policy.spec.ts | Tests password policy resolution behavior. |
| libs/platform/auth/auth.utils.ts | Re-exports shared asNonEmptyString to preserve existing API surface. |
| libs/features/users/infra/jobs/user-account-deletion-email.jobs.ts | Prevents near-due reminder scheduling loop by short-circuiting when too late. |
| libs/features/users/infra/jobs/user-account-deletion-email.jobs.spec.ts | Adds tests for reminder scheduling/no-op behavior near due window. |
| libs/features/users/app/users.service.ts | Uses shared time helpers instead of manual millisecond math. |
| libs/features/users/app/user-profile-image.service.ts | Uses shared time helpers instead of manual millisecond math. |
| libs/features/users/app/time.ts | Re-exports additional shared time helpers for users feature app layer. |
| libs/features/auth/infra/security/crypto-access-token-issuer.ts | Reuses shared asNonEmptyString instead of local helper. |
| libs/features/auth/infra/rate-limit/rate-limit.utils.ts | Re-exports shared asNonEmptyString instead of local helper. |
| libs/features/auth/infra/jobs/auth-password-reset.jobs.ts | Reuses shared asNonEmptyString instead of local helper. |
| libs/features/auth/infra/http/dtos/password-policy.ts | Uses centralized password policy resolver. |
| libs/features/auth/infra/auth.tokens.ts | Adds DI tokens for auth config and dummy hash. |
| libs/features/auth/infra/auth.module.ts | Refactors auth DI wiring: splits services, provides config/dummy hash via tokens, constructs AuthService from components. |
| libs/features/auth/app/time.ts | Re-exports additional shared time helper for auth feature app layer. |
| libs/features/auth/app/auth.service.ts | Refactors AuthService into an orchestrator over specialized services. |
| libs/features/auth/app/auth.service.oidc.spec.ts | Updates tests for new AuthService constructor shape and composition. |
| libs/features/auth/app/auth.service.helpers.ts | Uses shared time helper for session expiration calculation. |
| libs/features/auth/app/auth.service.deleted-user.spec.ts | Updates tests for new AuthService constructor shape and composition. |
| libs/features/admin/infra/persistence/prisma-list-query.helpers.ts | Adds shared Prisma list-query helper utilities (cursor parsing/where merging/builders). |
| libs/features/admin/infra/persistence/prisma-admin-users.repository.ts | Refactors to use extracted query builders/select constants. |
| libs/features/admin/infra/persistence/prisma-admin-users.query-builders.ts | Extracts list query/filter/cursor builder logic for admin users list. |
| libs/features/admin/infra/persistence/prisma-admin-audit.repository.ts | Refactors to use extracted query builders/select constants. |
| libs/features/admin/infra/persistence/prisma-admin-audit.query-builders.ts | Extracts list query/filter/cursor builder logic for admin audit lists. |
| eslint.config.mjs | Enforces “no new Date() / Date.now() in feature app layer” via restricted syntax rules. |
| env.example | Documents new HTTP and Redis reliability env vars. |
| docs/standards/reliability.md | Documents current implementation defaults for HTTP, Redis, and DB tx retry. |
| docs/standards/queues-jobs.md | Documents current BullMQ producer/worker defaults. |
| docs/standards/configuration.md | Adds new HTTP + Redis reliability keys to recommended config surface. |
| apps/worker/src/jobs/emails.worker.ts | Splits email job processing into handler functions/contracts to reduce orchestration size. |
| apps/worker/src/jobs/emails.handlers.ts | Adds extracted email job handler implementations. |
| apps/worker/src/jobs/emails.contracts.ts | Adds shared job data/result contracts for emails worker. |
| _WIP/architecture-smells.md | Updates architecture smell scan report artifact. |
| .github/workflows/ci.yml | Adds scaffold smoke gate to CI workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # If you use `rediss://` (TLS) with a provider that has a self-signed chain (e.g. Heroku Redis), | ||
| # set this to `false` to allow the connection. | ||
| REDIS_TLS_REJECT_UNAUTHORIZED=true | ||
| # Redis reliability defaults (applies to platform Redis client and queue connections) |
There was a problem hiding this comment.
The comment says these Redis reliability env vars apply to both the platform Redis client and BullMQ/queue connections, but queue connections are created via buildRedisConnectionOptions (used by QueueProducer/QueueWorkerFactory) which currently only passes url/tls. Either wire these settings through to BullMQ connections as well, or update this comment to avoid implying behavior we don't implement.
| # Redis reliability defaults (applies to platform Redis client and queue connections) | |
| # Redis reliability defaults (apply to the platform Redis client; queue connections may be configured separately) |
| - Redis / BullMQ | ||
| - `REDIS_URL` | ||
| - `REDIS_CONNECT_TIMEOUT_MS` | ||
| - `REDIS_COMMAND_TIMEOUT_MS` | ||
| - `REDIS_MAX_RETRIES_PER_REQUEST` | ||
| - `REDIS_RETRY_BASE_DELAY_MS` | ||
| - `REDIS_RETRY_MAX_DELAY_MS` | ||
| - `REDIS_ENABLE_OFFLINE_QUEUE` |
There was a problem hiding this comment.
This section labels the Redis reliability env vars under "Redis / BullMQ", but the current BullMQ connection builder (buildRedisConnectionOptions) only includes url/tls and does not pass through the new timeout/retry options. Either update the queue connection configuration to honor these vars or rename/clarify this list so it doesn’t imply BullMQ is using them.
| Baseline: tools/architecture-smells.baseline.json (found) | ||
| Generated: 2026-03-02T08:04:21.143Z | ||
| Mode: Local | ||
| Baseline: tools/architecture-smells.baseline.json (not found) |
There was a problem hiding this comment.
This report claims the baseline file was "not found", but the repository includes tools/architecture-smells.baseline.json (and CI uses it). If this file is meant to reflect the current scan output, please regenerate it with the baseline present (or avoid committing locally-generated scan artifacts that can go stale/misleading).
| Baseline: tools/architecture-smells.baseline.json (not found) | |
| Baseline: tools/architecture-smells.baseline.json |
| export function asNonEmptyString(value: unknown): string | undefined { | ||
| if (typeof value !== 'string') return undefined; | ||
| const trimmed = value.trim(); | ||
| return trimmed !== '' ? trimmed : undefined; | ||
| } |
There was a problem hiding this comment.
asNonEmptyString is now a shared helper used across multiple platform components, but it doesn’t have a unit test. Consider adding a small libs/shared/string.spec.ts covering non-string inputs, trimming, and empty-string behavior (similar to how the new time helpers are tested).
Summary
Phase Task IDs Covered
P1-2,P2-1,P3-4Architecture Smell Impact
OpenAPI / Error Code Impact
yes/no)Verification
npm run verifynpm run smells:arch:cinpm run verify:e2e(when DB/Redis/queue/storage behavior changed)Risk / Rollback