- Hide raw database connections (DbDep) from routers by removing from public exports - Maintain DbDep as deprecated export for backward compatibility - Add _DbDep internal dependency for use by other dependencies like require_auth - Update module docstring to explain dependency layering rules - Add comprehensive documentation section on dependency layering to Backend-Development.md This enforces the architectural boundary where: - Routers depend on repository dependencies (SessionRepoDep, BlocklistRepositoryDep, etc) - Services orchestrate operations through repositories - Only repositories execute SQL queries The repository boundary is now technically enforced through the dependency injection system, making it impossible for routers to accidentally bypass repositories and access the database directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
23 KiB
23 KiB
6) Raw DB connection exposed as dependency for all routes
- Where found:
- Why this is needed:
- Architectural boundary relies on convention, not enforcement.
- Goal:
- Enforce repository boundary for persistence access.
- What to do:
- Prefer repository dependencies in routers.
- Restrict direct DB usage to repository/service internals.
- Possible traps and issues:
- Large refactor may touch many endpoint signatures.
- Docs changes needed:
- Add dependency layering rule to backend guidelines.
- Doc references:
7) Service layer coupled to response/presentation models
- Where found:
- Why this is needed:
- Domain logic becomes tied to API shape and slows model evolution.
- Goal:
- Keep service returns domain-centered; map to API models at router boundary.
- What to do:
- Introduce service DTO/domain objects.
- Map to response models in routers or dedicated mappers.
- Possible traps and issues:
- Temporary duplicate model definitions during migration.
- Docs changes needed:
- Add layer responsibilities and mapping policy.
- Doc references:
8) Inconsistent modeling style (TypedDict vs Pydantic)
- Where found:
- Why this is needed:
- Mixed validation/serialization behavior increases maintenance cost.
- Goal:
- Standardize model type usage by layer.
- What to do:
- Define when TypedDict is allowed.
- Convert external-facing structures to Pydantic consistently.
- Possible traps and issues:
- Performance and strictness differences may alter runtime behavior.
- Docs changes needed:
- Add modeling conventions section.
- Doc references:
9) Repository protocol coverage is incomplete
- Where found:
- Why this is needed:
- Missing protocols reduce mockability and static contract checks.
- Goal:
- Full protocol coverage for repository interfaces.
- What to do:
- Add protocol definitions for missing repositories.
- Validate implementation compatibility in tests/CI.
- Possible traps and issues:
- Protocol drift if methods evolve without synchronized updates.
- Docs changes needed:
- Add repository protocol checklist.
- Doc references:
10) Startup sequence depends on implicit ordering
- Where found:
- Why this is needed:
- Hidden order dependencies make lifecycle changes risky.
- Goal:
- Explicitly define resource/task startup graph and required prerequisites.
- What to do:
- Document dependency graph.
- Add startup assertions and health checks for each stage.
- Possible traps and issues:
- Partial startup failures may leave stale scheduler or open sessions.
- Docs changes needed:
- Add startup DAG and failure-mode behavior.
- Doc references:
11) Logging semantics are inconsistent across backend modules
- Where found:
- Why this is needed:
- Uneven level usage reduces observability quality.
- Goal:
- Standardize logging levels and event naming.
- What to do:
- Define logging conventions.
- Align service/task logging with that convention.
- Possible traps and issues:
- Excessive log volume if levels are set too low globally.
- Docs changes needed:
- Add logging policy and examples.
- Doc references:
12) Prop drilling in jail overview page
- Where found:
- Why this is needed:
- Large prop chains increase coupling and refactor cost.
- Goal:
- Move jail state/actions into dedicated context or controller hook.
- What to do:
- Introduce JailContext (or equivalent local state container).
- Remove multi-hop prop forwarding.
- Possible traps and issues:
- Context overuse can trigger broad rerenders if not split.
- Docs changes needed:
- Add frontend state ownership notes.
- Doc references:
13) Config page is over-centralized
- Where found:
- Why this is needed:
- Tab orchestration and UI concerns are too concentrated.
- Goal:
- Decompose page into focused route/tab controllers.
- What to do:
- Split tab state/routing logic from rendering components.
- Extract domain-specific subcontainers.
- Possible traps and issues:
- Shared state sync across tabs can regress.
- Docs changes needed:
- Add config page composition map.
- Doc references:
14) Error boundary granularity is too coarse
- Where found:
- Why this is needed:
- Single top boundary causes full app fallback on local component failures.
- Goal:
- Add page-level and section-level boundaries.
- What to do:
- Wrap risky pages/widgets with local boundaries.
- Preserve shell/navigation when a section fails.
- Possible traps and issues:
- Too many boundaries can complicate fallback UX consistency.
- Docs changes needed:
- Add frontend resilience/fallback strategy.
- Doc references:
15) Fragmented async error UX handling in components
- Where found:
- Why this is needed:
- Localized ad-hoc error handling leads to inconsistent user feedback.
- Goal:
- Centralized error reporting + consistent UI feedback channels.
- What to do:
- Introduce notification/error service.
- Standardize form operation error patterns.
- Possible traps and issues:
- Duplicate messaging if local and global handlers fire together.
- Docs changes needed:
- Add frontend error handling guideline.
- Doc references:
16) API usage pattern is inconsistent across components/hooks
- Where found:
- Why this is needed:
- Data access logic is spread, reducing composability and testability.
- Goal:
- Use a consistent hook/service boundary for API calls.
- What to do:
- Define data-fetching conventions and keep component files UI-focused.
- Possible traps and issues:
- Over-abstracting can obscure simple endpoint interactions.
- Docs changes needed:
- Add API usage layering section.
- Doc references:
17) Weak typed error contracts in generic hooks
- Where found:
- Why this is needed:
- Unknown-only error handling weakens actionable UX and diagnostics.
- Goal:
- Standardize API error typing and hook-level error model.
- What to do:
- Introduce discriminated error payloads.
- Map ApiError and network errors consistently.
- Possible traps and issues:
- Type expansion can touch many call sites.
- Docs changes needed:
- Add typed error model examples.
- Doc references:
18) Duplicate polling/list loading behavior across hooks
- Where found:
- Why this is needed:
- Duplication multiplies maintenance bugs.
- Goal:
- Share a composable base for fetch lifecycle, cancellation, and polling.
- What to do:
- Extract shared primitives and keep hook-specific selectors minimal.
- Possible traps and issues:
- Generic abstractions can become too complex.
- Docs changes needed:
- Add hook architecture overview.
- Doc references:
19) Provider dependency chain is implicit
- Where found:
- Why this is needed:
- Order-sensitive providers can fail silently during future refactors.
- Goal:
- Make provider dependency order explicit and tested.
- What to do:
- Document provider order rationale.
- Add provider composition tests.
- Possible traps and issues:
- Hidden assumptions in auth/timezone/theme initialization.
- Docs changes needed:
- Add provider order contract.
- Doc references:
20) Loading UX lacks progressive/skeleton states
- Where found:
- Why this is needed:
- Blank loading regions reduce perceived responsiveness.
- Goal:
- Introduce standardized loading placeholders.
- What to do:
- Build shared skeleton components for tables/charts/forms.
- Possible traps and issues:
- Skeleton mismatch with actual layout can cause content shift.
- Docs changes needed:
- Add loading UX component guidance.
- Doc references:
21) Silent auth error swallow in fetch error utility
- Where found:
- Why this is needed:
- Silent return can drop actionable errors when no auth handler is registered.
- Goal:
- Ensure auth errors are handled deterministically with fallback logging/handling.
- What to do:
- Enforce handler registration or fallback behavior.
- Possible traps and issues:
- Duplicate redirect flows if multiple auth handlers exist.
- Docs changes needed:
- Add auth error handling contract for utilities.
- Doc references:
22) Magic strings are scattered in frontend storage keys
- Where found:
- Why this is needed:
- Repeated literals invite drift and typo regressions.
- Goal:
- Centralize user/session/local storage keys.
- What to do:
- Consolidate into a single constants module.
- Possible traps and issues:
- Existing tests may assume current literal values.
- Docs changes needed:
- Add storage key registry note.
- Doc references:
23) No global cancellation policy on route transitions
- Where found:
- Why this is needed:
- Many hooks cancel individually, but route-wide cancellation remains inconsistent.
- Goal:
- Provide a global request lifecycle cancellation mechanism.
- What to do:
- Introduce navigation-aware cancellation context/manager.
- Possible traps and issues:
- Over-cancel can break long-lived background fetches unintentionally.
- Docs changes needed:
- Add request lifecycle policy.
- Doc references:
24) API response wrapper shape is inconsistent
- Where found:
- Why this is needed:
- Inconsistent payload envelopes increase frontend branching and integration bugs.
- Goal:
- Define and enforce a consistent response envelope policy.
- What to do:
- Standardize endpoint response forms.
- Align frontend typing and parsing strategy.
- Possible traps and issues:
- Breaking contract for existing clients.
- Docs changes needed:
- Add API response style guide.
- Doc references:
25) No canonical snake_case/camelCase serialization policy
- Where found:
- Why this is needed:
- Naming convention drift causes brittle frontend-backend contracts.
- Goal:
- Enforce one API field naming policy.
- What to do:
- Configure model aliasing strategy and update frontend contracts.
- Possible traps and issues:
- Partial migration can leave mixed payload formats.
- Docs changes needed:
- Add naming convention section for API fields.
- Doc references:
26) Pagination contract is not standardized across endpoints
- Where found:
- Why this is needed:
- Different pagination patterns increase frontend complexity.
- Goal:
- Use one shared paginated response model.
- What to do:
- Introduce common pagination schema and query parameter policy.
- Possible traps and issues:
- Existing endpoint consumers may require transition period.
- Docs changes needed:
- Add pagination API spec.
- Doc references:
27) Error response body shape is inconsistent
- Where found:
- Why this is needed:
- Frontend cannot reliably branch on machine-readable error codes.
- Goal:
- Standard error response schema with code + detail + metadata.
- What to do:
- Add shared error model and update handlers.
- Possible traps and issues:
- Legacy consumers parsing detail strings may break.
- Docs changes needed:
- Add backend error schema and mapping table.
- Doc references:
28) Login failure delay can enable app-layer DoS
- Where found:
- Why this is needed:
- Fixed 10-second await for invalid login attempts can amplify load impact.
- Goal:
- Keep brute-force resistance without exhausting request capacity.
- What to do:
- Replace fixed sleep with limiter-backed penalty strategy and concurrency protection.
- Possible traps and issues:
- Too little penalty weakens brute-force protection.
- Docs changes needed:
- Document authentication throttling strategy.
- Doc references:
29) Blocklist URL validation has DNS-rebinding window
- Where found:
- Why this is needed:
- Validation at create/update does not guarantee safe runtime destination.
- Goal:
- Enforce SSRF safety at connect/request time as well.
- What to do:
- Add runtime destination/IP allow-deny validation in HTTP layer.
- Possible traps and issues:
- Resolver/cache behavior may vary across environments.
- Docs changes needed:
- Add blocklist network security notes and runtime checks.
- Doc references:
30) Setup persistence is non-atomic across DB contexts
- Where found:
- Why this is needed:
- Partial commits during setup can leave inconsistent state.
- Goal:
- Make setup operations transactional and crash-safe.
- What to do:
- Introduce staged setup state and transaction boundaries.
- Possible traps and issues:
- SQLite transaction handling across multiple DB files is limited.
- Docs changes needed:
- Add setup state machine and rollback behavior.
- Doc references:
31) Fire-and-forget reschedule may fail silently
- Where found:
- Why this is needed:
- Schedule update requests can succeed while background reschedule fails.
- Goal:
- Make schedule updates deterministic and observable.
- What to do:
- Await reschedule path or persist task outcome status and surface errors.
- Possible traps and issues:
- Blocking request path might add latency if scheduler is busy.
- Docs changes needed:
- Document scheduling reliability guarantees.
- Doc references:
32) RateLimiter cleanup function is not scheduled/used
- Where found:
- Why this is needed:
- Rate limiter state can grow over long runtimes.
- Goal:
- Ensure periodic cleanup or bounded memory strategy.
- What to do:
- Add scheduled cleanup or auto-eviction structure.
- Possible traps and issues:
- Cleanup cadence too frequent can add overhead.
- Docs changes needed:
- Add operational notes for auth throttling lifecycle.
- Doc references:
33) Trusted proxy configuration is hardcoded in auth router
- Where found:
- Why this is needed:
- Incorrect client IP extraction can break per-IP rate limiting behind proxies.
- Goal:
- Move trusted proxies to validated runtime config.
- What to do:
- Add settings for trusted proxy IPs/CIDRs.
- Validate and use these in client IP extraction.
- Possible traps and issues:
- Over-trusting headers can enable spoofing.
- Docs changes needed:
- Add reverse-proxy deployment configuration section.
- Doc references:
34) Setup redirect allowlist uses broad prefix matching
- Where found:
- Why this is needed:
- Prefix-based allow rules are fragile for future route additions.
- Goal:
- Use exact path or route-level allow policy.
- What to do:
- Replace startswith matching with explicit allowlist checks.
- Possible traps and issues:
- API docs and setup flow paths must remain reachable.
- Docs changes needed:
- Add setup guard route policy documentation.
- Doc references:
35) API client sends JSON and CSRF header for every request method
- Where found:
- Why this is needed:
- Extra headers on GET increase unnecessary CORS preflights and noise.
- Goal:
- Apply headers by method/body requirements.
- What to do:
- Only set Content-Type for requests with JSON body.
- Send CSRF header for mutating cookie-authenticated requests only.
- Possible traps and issues:
- CSRF protection assumptions must still hold for all mutating paths.
- Docs changes needed:
- Update frontend API client contract and CSRF notes.
- Doc references:
36) Polling continues when tab is not visible
- Where found:
- Why this is needed:
- Unnecessary backend load and client resource usage in background tabs.
- Goal:
- Pause/reduce polling when page is hidden.
- What to do:
- Add visibility-aware polling strategy and optional backoff.
- Possible traps and issues:
- Data may appear stale immediately after tab restore if refresh is delayed.
- Docs changes needed:
- Add frontend polling lifecycle policy.
- Doc references:
37) Multi-worker safety check depends on one environment variable
- Where found:
- Why this is needed:
- Other process managers can still launch multiple workers without this variable.
- Goal:
- Enforce scheduler single-executor safety regardless of launcher.
- What to do:
- Add robust single-run lock/leader mechanism for scheduler ownership.
- Possible traps and issues:
- Locking strategy must be reliable in container orchestration.
- Docs changes needed:
- Expand deployment constraints and supported run modes.
- Doc references:
38) History archive query paths may need explicit indexing plan
- Where found:
- Why this is needed:
- Large archive datasets can degrade filter/sort performance.
- Goal:
- Add indexes aligned with real query patterns.
- What to do:
- Benchmark common history queries.
- Add migration with targeted indexes.
- Possible traps and issues:
- Extra indexes increase write cost and DB size.
- Docs changes needed:
- Add DB performance/indexing section for history.
- Doc references:
39) No explicit DI container strategy for backend service graph
- Where found:
- Why this is needed:
- Dependency construction and lifecycle are partly implicit.
- Goal:
- Define a clear dependency wiring pattern for services and repositories.
- What to do:
- Create service composition root pattern and document usage.
- Possible traps and issues:
- Over-engineering if container abstraction is too heavy for current size.
- Docs changes needed:
- Add dependency wiring chapter.
- Doc references:
40) Frontend and backend observability are not aligned
- Where found:
- Why this is needed:
- Backend uses structured logging while frontend error telemetry is mostly local and ad-hoc.
- Goal:
- Define unified error telemetry and correlation approach.
- What to do:
- Introduce frontend error reporting pipeline and request correlation IDs.
- Possible traps and issues:
- PII/sensitive payload leakage risk in client-side telemetry.
- Docs changes needed:
- Add observability and privacy-safe logging guidelines.
- Doc references: