From 9380fc8a6f6ed7fc88b73c1924ea28283fcf81c9 Mon Sep 17 00:00:00 2001 From: itinsecurity <98172852+itinsecurity@users.noreply.github.com> Date: Thu, 19 Mar 2026 20:02:08 +0100 Subject: [PATCH 1/5] feat(rendering): force dynamic rendering for all (app) routes Add `export const dynamic = 'force-dynamic'` to all four files in the authenticated (app) route group so Next.js renders them at request time instead of statically pre-rendering during `next build`. This allows the application to build successfully without a database being available. Also adds a Playwright E2E test that verifies the holdings page serves fresh data after an update without requiring a rebuild. Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 2 +- next-env.d.ts | 2 +- .../checklists/requirements.md | 34 ++++ specs/004-dynamic-rendering/data-model.md | 9 ++ .../design-meeting-20260319-171800.md | 150 ++++++++++++++++++ specs/004-dynamic-rendering/plan.md | 72 +++++++++ specs/004-dynamic-rendering/quickstart.md | 50 ++++++ specs/004-dynamic-rendering/research.md | 63 ++++++++ specs/004-dynamic-rendering/spec.md | 71 +++++++++ specs/004-dynamic-rendering/tasks.md | 129 +++++++++++++++ src/app/(app)/holdings/[id]/page.tsx | 2 + src/app/(app)/holdings/page.tsx | 2 + src/app/(app)/layout.tsx | 2 + src/app/(app)/portfolio/page.tsx | 2 + tests/e2e/dynamic-rendering.spec.ts | 39 +++++ tsconfig.json | 2 +- 16 files changed, 628 insertions(+), 3 deletions(-) create mode 100644 specs/004-dynamic-rendering/checklists/requirements.md create mode 100644 specs/004-dynamic-rendering/data-model.md create mode 100644 specs/004-dynamic-rendering/design-meeting-20260319-171800.md create mode 100644 specs/004-dynamic-rendering/plan.md create mode 100644 specs/004-dynamic-rendering/quickstart.md create mode 100644 specs/004-dynamic-rendering/research.md create mode 100644 specs/004-dynamic-rendering/spec.md create mode 100644 specs/004-dynamic-rendering/tasks.md create mode 100644 tests/e2e/dynamic-rendering.spec.ts diff --git a/CLAUDE.md b/CLAUDE.md index c008576..cc8d6fb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,4 +1,4 @@ -# CLAUDE.md +# CLAUDE.md This file provides guidance to Claude Code when working with code in this repository. diff --git a/next-env.d.ts b/next-env.d.ts index 830fb59..9edff1c 100644 --- a/next-env.d.ts +++ b/next-env.d.ts @@ -1,6 +1,6 @@ /// /// -/// +import "./.next/types/routes.d.ts"; // NOTE: This file should not be edited // see https://nextjs.org/docs/app/api-reference/config/typescript for more information. diff --git a/specs/004-dynamic-rendering/checklists/requirements.md b/specs/004-dynamic-rendering/checklists/requirements.md new file mode 100644 index 0000000..9bf4bc6 --- /dev/null +++ b/specs/004-dynamic-rendering/checklists/requirements.md @@ -0,0 +1,34 @@ +# Specification Quality Checklist: Force Dynamic Page Rendering + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-03-19 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All items pass. Specification is ready for `/speckit.clarify` or `/speckit.plan`. diff --git a/specs/004-dynamic-rendering/data-model.md b/specs/004-dynamic-rendering/data-model.md new file mode 100644 index 0000000..9b285f7 --- /dev/null +++ b/specs/004-dynamic-rendering/data-model.md @@ -0,0 +1,9 @@ +# Data Model: Force Dynamic Page Rendering + +**Feature**: 004-dynamic-rendering | **Date**: 2026-03-19 + +## No Changes + +This feature is a rendering configuration change only. No database schema modifications, no new entities, no changed relationships, no new validation rules, and no state transitions. + +The existing data model (holdings, asset profiles, enrichment pipeline) is unaffected. All existing data-fetching functions (`getHoldings`, `createHolding`, `updateHolding`, `deleteHolding`) continue to work identically — they are simply invoked at request time instead of build time. diff --git a/specs/004-dynamic-rendering/design-meeting-20260319-171800.md b/specs/004-dynamic-rendering/design-meeting-20260319-171800.md new file mode 100644 index 0000000..00a38d5 --- /dev/null +++ b/specs/004-dynamic-rendering/design-meeting-20260319-171800.md @@ -0,0 +1,150 @@ +# Design Meeting Summary +## Force Dynamic Page Rendering + +--- + +## Meeting Metadata +- **Feature Branch**: 004-dynamic-rendering +- **Date**: 2026-03-19 17:18 UTC +- **Participants**: Product Owner, Business Sponsor, Scrum Master, UX Designer, Technical Lead +- **Method**: Multi-agent stakeholder review + +--- + +## Executive Summary + +### Amendments Required +1. **Concern 1 — Runtime DB assumption conflicts with edge cases**: Remove or narrow the assumption "database is always available at runtime" to eliminate contradiction with the edge cases section that acknowledges runtime unavailability. Optionally add one acceptance scenario covering what the user sees when the DB is unavailable at runtime. +2. **Concern 2 — Unverified page coverage scope**: Add a pre-implementation verification step to confirm which pages access the database, rather than carrying `(app)` route group coverage as an unchecked assumption. +3. **Concern 3 — SC-004 not falsifiable**: Add a note that SC-004 ("no pages that access the database are statically generated") should include a verification mechanism, even if the specific approach is left to the implementer. + +### Resolved +1. **Concern 4 — Loading state / latency UX**: No spec amendment needed. The spec's explicit assumption that "a small increase in request latency is acceptable given the single-user nature of the application" is sufficient. Unanimous agreement. + +--- + +## Concern Register + +| ID | Severity | Raised By | Spec Item | Description | Resolution | +|----|----------|-----------|-----------|-------------|------------| +| C1 | MEDIUM | PO, BS, SM, UX, TL | Assumptions / Edge Cases | Runtime DB unavailability acknowledged but deferred to unverified "existing error handling"; assumption contradicts edge case | AMENDED | +| C2 | MEDIUM | PO, SM | FR-003 / Assumptions | `(app)` route group assumed to cover all DB-accessing pages without verification | AMENDED | +| C3 | LOW | SM, TL | SC-003 / SC-004 | No new test coverage required; SC-004 has no verification mechanism | AMENDED | +| C4 | LOW | UX | User Story 2 / Assumptions | No loading state or latency threshold defined for dynamic rendering | RESOLVED | + +--- + +# Detailed Discussion + +## Opening Statements + +### Product Owner +**Positives**: The spec is tight and well-scoped. The problem statement is clear, the two user stories map directly to the stated problem, and the acceptance scenarios are concrete and independently testable. The assumptions section is unusually honest — explicitly acknowledging the latency trade-off and single-user context prevents scope creep later. + +**Concerns**: +- CONCERN: Edge Cases section — runtime database unavailability is acknowledged but deferred to "existing error handling behavior" without verifying that behavior is adequate — Impact: If existing error handling is insufficient, the feature ships with a silent regression that only surfaces in production — Severity: MEDIUM +- CONCERN: FR-003 / Assumptions — the spec assumes the `(app)` route group covers all database-accessing pages, but this is stated as an assumption rather than a verified fact — Impact: If any page outside that route group also accesses the database, it will be missed and the build will still fail — Severity: HIGH + +### Business Sponsor +**Positives**: The spec is tightly scoped to a real, blocking problem with a clear business rationale: the app cannot deploy at all without this fix. The assumption that a small latency increase is acceptable for a single-user application is correctly called out and saves scope-creep debates later. + +**Concerns**: +- CONCERN: Assumption "database always available at runtime" — does not define what "available" means or who is responsible for ensuring it — Impact: Runtime failures may appear to be application bugs rather than infrastructure issues — Severity: LOW + +### Scrum Master +**Positives**: The spec is tightly scoped and problem-driven. The input statement maps cleanly to the user stories and requirements. Both user stories include explicit independent test descriptions, enabling sprint-level acceptance without full integration dependency. + +**Concerns**: +- CONCERN: Assumptions bullet 1 — "database is always available at runtime" conflicts with edge case acknowledging runtime unavailability. These two positions are in tension. — Impact: Ambiguity could cause over-engineering or under-engineering of error handling — Severity: MEDIUM +- CONCERN: SC-003 — No mention of new tests for the dynamic rendering behavior itself. If treated as the only test obligation, the rendering change ships with no direct test coverage. — Impact: Regression risk if future changes re-introduce static rendering — Severity: LOW + +### UX Designer +**Positives**: The spec clearly articulates the user-facing impact of data freshness and correctly identifies financial data staleness as a real user concern. The edge cases section is thorough and covers the right scenarios from a user-facing perspective. + +**Concerns**: +- CONCERN: Edge Cases — Runtime database unavailability UX is deferred, not defined. A user mid-session could lose context with no recovery path or message. — Severity: MEDIUM +- CONCERN: User Story 2 / SC-002 — No loading state behavior specified for dynamic rendering latency. Users may perceive sluggishness on slower connections. — Severity: LOW + +### Technical Lead +**Positives**: The spec is well-scoped and the problem statement is clear. The runtime availability assumption is explicitly stated as load-bearing. FR-003 correctly carves out non-database pages, avoiding unnecessary over-engineering. + +**Concerns**: +- CONCERN: Assumptions — "Database is always available at runtime" has no corresponding acceptance scenario. Error behavior is undefined and untested. — Severity: MEDIUM +- CONCERN: SC-004 — No specified mechanism for verifying "no pages that access the database are statically generated" at CI time. Could silently regress. — Severity: LOW + +--- + +## Debate Highlights + +**Concern 2 — Page coverage severity (HIGH vs MEDIUM):** +- Tech Lead challenged Product Owner's HIGH severity rating, arguing that any out-of-group database-accessing page would produce an immediate, visible build failure (the same symptom as the current bug), so the risk of silent regression is low. A pre-implementation route audit is sufficient without amending the spec. +- Product Owner maintained HIGH, arguing this is the core deliverable and the spec should not carry an unverified assumption into implementation. +- Scrum Master supported Product Owner's HIGH rating from a sprint planning standpoint: if the assumption is wrong, the sprint goal fails on first CI run. +- Resolution: AMENDED at MEDIUM severity — the concern is valid and warrants a verification step, but the failure mode is visible rather than silent. + +**Concern 1 — Runtime DB unavailability:** +- All five participants raised this independently. Business Sponsor and Tech Lead argued RESOLVED (defer to existing error handling; out of scope for this fix). Scrum Master and UX Designer argued AMENDED (the assumption contradicts the edge case; at minimum remove the contradiction). Product Owner's original concern aligned with amendment. +- Scrum Master proposed removing or narrowing the assumption. UX Designer proposed adding one acceptance scenario. +- Resolution: AMENDED — remove or narrow the conflicting assumption; optionally add one acceptance scenario. + +**Concern 4 — Loading state UX:** +- Business Sponsor rejected this concern as over-engineering for a single-user app. Product Owner agreed, noting the change affects when data is fetched, not how it is presented. UX Designer conceded, acknowledging the single-user context makes this a non-issue. +- Resolution: RESOLVED unanimously. + +**Concern 3 — CI gate for SC-004:** +- Scrum Master argued SC-004 as written is aspirational rather than falsifiable. Tech Lead accepted the risk as LOW and preferred tracking as a follow-on. Product Owner deferred to Tech Lead on implementation cost. +- Resolution: AMENDED at LOW severity — add a note that SC-004 should include a verification step. + +--- + +## Concern Resolutions + +### C1: Runtime database unavailability — undefined error behavior +**Severity:** MEDIUM +**Raised by:** All participants +**Spec Item:** Assumptions (bullet 1) / Edge Cases + +**Debate Summary:** +All five participants flagged this concern independently. The core issue is a contradiction: the Assumptions section states the database is "always available at runtime," while the Edge Cases section acknowledges runtime unavailability and defers to "existing error handling behavior." Business Sponsor and Tech Lead argued this is out of scope for a deployment-fix feature and should be RESOLVED. Scrum Master argued the conflicting language creates a planning trap — the team will interpret scope differently depending on which section they read. UX Designer argued that without at least one acceptance scenario, there is no way to verify the error experience is acceptable. + +**Resolution:** AMENDED +Remove or narrow the assumption "database is always available at runtime" to eliminate the contradiction with the Edge Cases section. The assumption could be reworded to "the database is expected to be available at runtime; transient unavailability is handled by existing error states." Optionally, add one acceptance scenario covering the user experience when the database is temporarily unavailable at runtime. + +--- + +### C2: Unverified page coverage scope +**Severity:** MEDIUM (downgraded from HIGH after debate) +**Raised by:** Product Owner (HIGH), supported by Scrum Master (HIGH) +**Spec Item:** FR-003 / Assumptions (bullet 4) + +**Debate Summary:** +Product Owner raised this at HIGH severity, arguing the spec should not carry an unverified assumption about which pages access the database. Scrum Master supported HIGH from a sprint planning perspective. Tech Lead challenged the severity, arguing that any missed page would cause an immediate, visible build failure (the same symptom as the current bug), so the failure mode is not silent. The risk is real but the blast radius is contained. + +**Resolution:** AMENDED +Add a pre-implementation verification step to confirm which pages access the database, rather than carrying `(app)` route group coverage as an unchecked assumption. Severity downgraded to MEDIUM because the failure mode is visible, not silent. The spec should note that a route audit is required before implementation begins. + +--- + +### C3: No CI gate or new test coverage for dynamic rendering +**Severity:** LOW +**Raised by:** Scrum Master, Technical Lead +**Spec Item:** SC-003 / SC-004 + +**Debate Summary:** +Scrum Master noted SC-003 only requires existing tests to pass, with no new test obligation for the rendering change itself. Tech Lead noted SC-004 has no mechanism for CI-time verification. Both rated LOW. Product Owner deferred to Tech Lead on implementation cost. Scrum Master argued SC-004 is currently aspirational rather than falsifiable. + +**Resolution:** AMENDED +Add a note to SC-004 that the criterion should be verified through a defined mechanism (e.g., build output check, CI assertion), even if the specific approach is left to the implementer. LOW severity — not blocking, but improves the criterion's usefulness. + +--- + +### C4: No loading state or latency UX defined +**Severity:** LOW +**Raised by:** UX Designer +**Spec Item:** User Story 2 / Assumptions + +**Debate Summary:** +UX Designer raised this noting that dynamic rendering introduces per-request latency with no defined loading behavior. Business Sponsor rejected it as over-engineering for a single-user app. Product Owner agreed, noting the change affects when data is fetched, not how it is presented — existing loading states from prior implementation remain in place. UX Designer conceded, acknowledging the single-user context. + +**Resolution:** RESOLVED +No spec change needed. The spec's explicit assumption that "a small increase in request latency is acceptable given the single-user nature of the application" is sufficient. Existing loading states from the prior implementation are preserved by FR-004. diff --git a/specs/004-dynamic-rendering/plan.md b/specs/004-dynamic-rendering/plan.md new file mode 100644 index 0000000..686459a --- /dev/null +++ b/specs/004-dynamic-rendering/plan.md @@ -0,0 +1,72 @@ +# Implementation Plan: Force Dynamic Page Rendering + +**Branch**: `004-dynamic-rendering` | **Date**: 2026-03-19 | **Spec**: [spec.md](./spec.md) +**Input**: Feature specification from `/specs/004-dynamic-rendering/spec.md` + +## Summary + +The application fails `next build` when no database is available at build time because Next.js attempts to statically pre-render pages that call Prisma. The fix is to add `export const dynamic = 'force-dynamic'` to every page and layout in the authenticated `(app)` route group, ensuring they are rendered at request time instead of build time. This is a configuration-only change — no data fetching logic, UI, or data model changes. + +## Technical Context + +**Language/Version**: TypeScript 5.x, Next.js 16.1.7 (App Router) +**Primary Dependencies**: Next.js, Prisma (with `@prisma/adapter-pg`), Auth.js (next-auth v5) +**Storage**: SQLite (local) / PostgreSQL (production) via Prisma +**Testing**: Vitest (unit + integration), Playwright (E2E), `next build` verification +**Target Platform**: Node.js server (self-hosted) +**Project Type**: Web application (Next.js App Router) +**Performance Goals**: N/A — single-user app; minor latency increase from dynamic rendering is acceptable per spec assumptions +**Constraints**: No database connection at build time; no weakening of security posture +**Scale/Scope**: 5 pages, 3 layouts — only 4 files in the `(app)` route group need modification + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Status | Notes | +|-----------|--------|-------| +| I. Portability | PASS | No provider-specific changes. `force-dynamic` is standard Next.js API. | +| II. Tech Stack Discipline | PASS | No new dependencies or stack changes. | +| III. Security (NON-NEGOTIABLE) | PASS | This change *improves* security by eliminating the need for database credentials at build time. No secrets in code. | +| IV. Testing (NON-NEGOTIABLE) | PASS | Existing tests must continue passing. Build verification (`next build` without DB) is the primary acceptance test. E2E test for dynamic data freshness required. | +| V. Simplicity | PASS | Minimal change: one export line per file. No new abstractions, no caching model changes. | + +**Pre-design gate: PASSED** + +## Project Structure + +### Documentation (this feature) + +```text +specs/004-dynamic-rendering/ +├── plan.md # This file +├── research.md # Phase 0: Next.js 16 dynamic rendering research +├── data-model.md # Phase 1: No changes (configuration-only feature) +├── quickstart.md # Phase 1: Verification guide +└── tasks.md # Phase 2 output (/speckit.tasks command) +``` + +### Source Code (files to modify) + +```text +src/app/(app)/ +├── layout.tsx # ADD: export const dynamic = 'force-dynamic' +├── holdings/ +│ ├── page.tsx # ADD: export const dynamic = 'force-dynamic' +│ └── [id]/ +│ └── page.tsx # ADD: export const dynamic = 'force-dynamic' +└── portfolio/ + └── page.tsx # ADD: export const dynamic = 'force-dynamic' +``` + +**Files NOT modified** (no database access): +- `src/app/page.tsx` — root redirect only +- `src/app/(auth)/login/page.tsx` — no DB access +- `src/app/(auth)/layout.tsx` — no DB access +- `src/app/layout.tsx` — root metadata only + +**Structure Decision**: No new files or directories. This is a configuration-only change to 4 existing files in the `(app)` route group. + +## Complexity Tracking + +No constitution violations. No complexity justification needed. diff --git a/specs/004-dynamic-rendering/quickstart.md b/specs/004-dynamic-rendering/quickstart.md new file mode 100644 index 0000000..828e327 --- /dev/null +++ b/specs/004-dynamic-rendering/quickstart.md @@ -0,0 +1,50 @@ +# Quickstart: Force Dynamic Page Rendering + +**Feature**: 004-dynamic-rendering | **Date**: 2026-03-19 + +## What Changed + +All pages and layouts in the authenticated `(app)` route group now export `dynamic = 'force-dynamic'`, ensuring they are rendered at request time instead of being statically generated during `next build`. + +## Verification + +### 1. Build without database (primary acceptance test) + +```bash +# Unset DATABASE_URL (or ensure it's not configured) +unset DATABASE_URL +npm run build +# Expected: Build completes successfully with zero errors +``` + +The build output should show the `(app)` routes as dynamically rendered (`ƒ` or `λ` symbol) rather than statically generated (`●` symbol). + +### 2. Runtime data freshness + +```bash +# Start with database available +npm run start + +# 1. Navigate to /holdings — verify holdings load +# 2. Modify a holding in the database directly +# 3. Refresh the page — verify updated data appears immediately +``` + +### 3. Existing tests + +```bash +npm run test # Unit + integration tests +npm run test:e2e # Playwright E2E tests (if configured) +npx tsc --noEmit # Type checking +``` + +All existing tests must continue to pass. + +## Files Modified + +| File | Change | +|---|---| +| `src/app/(app)/layout.tsx` | Added `export const dynamic = 'force-dynamic'` | +| `src/app/(app)/holdings/page.tsx` | Added `export const dynamic = 'force-dynamic'` | +| `src/app/(app)/holdings/[id]/page.tsx` | Added `export const dynamic = 'force-dynamic'` | +| `src/app/(app)/portfolio/page.tsx` | Added `export const dynamic = 'force-dynamic'` | diff --git a/specs/004-dynamic-rendering/research.md b/specs/004-dynamic-rendering/research.md new file mode 100644 index 0000000..b2a8953 --- /dev/null +++ b/specs/004-dynamic-rendering/research.md @@ -0,0 +1,63 @@ +# Research: Force Dynamic Page Rendering + +**Feature**: 004-dynamic-rendering | **Date**: 2026-03-19 + +## R1: Why does the build fail? + +**Finding**: During `next build`, Next.js attempts to statically pre-render pages. Pages in `src/app/(app)/` call `getHoldings()` (a server action) which imports `prisma` from `src/lib/db.ts`. The Prisma client is eagerly instantiated at module load via `PrismaPg({ connectionString: process.env.DATABASE_URL })`. When no database is available at build time, this fails. + +**Key detail**: The `(app)/layout.tsx` calls `auth()` which uses `cookies()` internally, which should trigger dynamic rendering. However, the cascade behavior of layout-level dynamic rendering to child pages is undocumented in Next.js (GitHub Discussion #73312). Each page segment may be evaluated independently for static/dynamic determination. + +## R2: Approach — `force-dynamic` vs `connection()` vs `cacheComponents` + +**Decision**: Use `export const dynamic = 'force-dynamic'` on each file in the `(app)` route group. + +**Rationale**: +- Explicit, per-file, well-documented behavior in Next.js 16 +- No dependency on undocumented cascade behavior +- No change to the application's caching model +- One-line addition per file — minimal diff, easy to review + +**Alternatives considered**: + +| Alternative | Why rejected | +|---|---| +| `await connection()` from `next/server` | Function-level opt-in is more granular than needed; would require adding it inside each component body rather than as a clean top-level export | +| `cacheComponents: true` in next.config.ts | Changes the entire caching model (everything dynamic by default, opt-in with `"use cache"`). Overkill for this fix — introduces a paradigm shift when we only need 4 files to be dynamic. Also removes route segment config entirely, which may have unintended side effects. | +| Layout-only `force-dynamic` | Cascade to child pages is undocumented (Next.js GitHub Discussion #73312). Risky to rely on for a build-breaking issue. | +| Prisma lazy initialization | Would defer the connection to query time, but doesn't address the fundamental issue that Next.js would still attempt to execute `getHoldings()` during static generation. Also adds complexity to the DB layer for a rendering configuration problem. | + +## R3: Which files need `force-dynamic`? + +**Decision**: All 4 files in the `(app)` route group — the layout + 3 pages. + +**Analysis per file**: + +| File | DB Access | Other Dynamic Signals | Needs `force-dynamic`? | +|---|---|---|---| +| `(app)/layout.tsx` | `auth()` → cookies | `cookies()` (implicit via auth) | Yes — makes intent explicit, guards against framework changes | +| `(app)/holdings/page.tsx` | `getHoldings()` → Prisma | None | **Yes — critical**. No other dynamic signal; this is the most likely candidate for static generation | +| `(app)/holdings/[id]/page.tsx` | `getHoldings()` → Prisma | `await params` (dynamic segment) | Yes — dynamic segment helps but explicit export is cheap insurance | +| `(app)/portfolio/page.tsx` | `getHoldings()` → Prisma | `await searchParams` | Yes — searchParams helps but explicit export is cheap insurance | + +**Files excluded** (no database access): + +| File | Reason | +|---|---| +| `src/app/page.tsx` | Simple redirect, no data fetching | +| `src/app/(auth)/login/page.tsx` | Reads `NODE_ENV` only, no DB | +| `src/app/(auth)/layout.tsx` | Static wrapper, no DB | +| `src/app/layout.tsx` | Root metadata only | + +## R4: middleware.ts and dynamic rendering + +**Finding**: Middleware runs as a network-level routing layer before page rendering. It does NOT affect whether pages are statically or dynamically rendered during `next build`. The `auth()` call in middleware provides request-time auth checks but does not prevent static pre-rendering of pages. + +**Note**: Next.js 16 deprecated `middleware.ts` in favor of `proxy.ts` (with Node.js runtime). This migration is out of scope for this feature but noted for future work. + +## R5: Next.js 16 breaking changes relevant to this feature + +**Finding**: No breaking changes affect the chosen approach. Key points: +- `export const dynamic = 'force-dynamic'` continues to work in Next.js 16 (when `cacheComponents` is not enabled) +- `params` and `searchParams` must be awaited (already done in our codebase) +- The `cacheComponents` flag exists but is opt-in; we are not enabling it diff --git a/specs/004-dynamic-rendering/spec.md b/specs/004-dynamic-rendering/spec.md new file mode 100644 index 0000000..7050318 --- /dev/null +++ b/specs/004-dynamic-rendering/spec.md @@ -0,0 +1,71 @@ +# Feature Specification: Force Dynamic Page Rendering + +**Feature Branch**: `004-dynamic-rendering` +**Created**: 2026-03-19 +**Status**: Draft +**Input**: User description: "The app is failing build because a page is built before the database is available. Instead of reverting to a less secure database setup, the decision is to make sure the pages of this app are built dynamically on load, not statically during deployment." + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Successful Deployment Without Database at Build Time (Priority: P1) + +As the application owner, I want the application to build and deploy successfully even when no database is available during the build step, so that I can deploy to any environment without exposing database credentials or connections at build time. + +**Why this priority**: This is the core problem — the application currently fails to build in deployment environments where the database is not yet available. Without this fix, the app cannot be deployed at all. + +**Independent Test**: Can be fully tested by running the build process without any database connection configured and verifying it completes successfully. + +**Acceptance Scenarios**: + +1. **Given** the application is being built with no database connection available, **When** the build process runs, **Then** the build completes successfully without errors. +2. **Given** the application has been built without a database, **When** a user navigates to any page at runtime, **Then** the page loads correctly by fetching data from the database at request time. +3. **Given** the application is deployed and running with a database connection, **When** a user visits the holdings page, **Then** holdings data is fetched and displayed on each request (not served from a static build cache). + +--- + +### User Story 2 - All Authenticated Pages Render Fresh Data (Priority: P2) + +As a user viewing my portfolio or holdings, I want every page load to show the most current data from the database, so that I never see stale information from a cached static render. + +**Why this priority**: Even if the build succeeds, pages that are statically rendered at build time would serve stale data. Since this is a financial data application, data freshness is critical. + +**Independent Test**: Can be fully tested by modifying a holding in the database and immediately refreshing the page to confirm the updated data appears. + +**Acceptance Scenarios**: + +1. **Given** a holding has just been updated, **When** the user refreshes the holdings list page, **Then** the updated holding data is displayed immediately. +2. **Given** a new holding has been added, **When** the user navigates to the portfolio page, **Then** the spread analysis reflects the new holding without requiring a rebuild or redeployment. + +--- + +### Edge Cases + +- What happens if the database becomes temporarily unavailable at runtime? Pages should display appropriate error states rather than crashing, consistent with existing error handling behavior. +- What happens to the root redirect page (which does not access the database)? It should continue to work as before — a simple redirect does not require dynamic rendering but must not break during build. +- What happens to the login page? Authentication pages that do not access the database directly should continue to build and function normally. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: All pages that access the database MUST be rendered dynamically at request time, not statically at build time. +- **FR-002**: The application MUST build successfully without any database connection available during the build step. +- **FR-003**: Pages that do not access the database (e.g., the root redirect page, the login page) MUST continue to function correctly and MAY remain statically rendered. +- **FR-004**: The existing data-fetching behavior on each page MUST be preserved — the same data that was fetched before MUST still be fetched, just at request time instead of build time. +- **FR-005**: The application's security posture MUST NOT be weakened — no database credentials or connections should be required or exposed at build time. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: The build process completes successfully with zero database-related errors when no database connection is configured at build time. +- **SC-002**: All existing pages load correctly at runtime and display current data from the database on every request. +- **SC-003**: All existing tests continue to pass after the change. +- **SC-004**: It has been verified that no pages that access the database are statically generated during the build process. + +## Assumptions + +- The database is never available at build time. +- The existing error handling in pages (e.g., displaying "Failed to load holdings" messages) is sufficient for runtime database errors. +- A small increase in request latency from dynamic rendering is acceptable given the single-user nature of the application. +- Pages behind the authenticated layout (`(app)` route group) are the primary targets, as they are the ones accessing the database. diff --git a/specs/004-dynamic-rendering/tasks.md b/specs/004-dynamic-rendering/tasks.md new file mode 100644 index 0000000..d508a06 --- /dev/null +++ b/specs/004-dynamic-rendering/tasks.md @@ -0,0 +1,129 @@ +# Tasks: Force Dynamic Page Rendering + +**Input**: Design documents from `/specs/004-dynamic-rendering/` +**Prerequisites**: plan.md (required) ✓, spec.md (required) ✓, research.md ✓, data-model.md ✓, quickstart.md ✓ + +**Organization**: Tasks grouped by user story. This feature is a configuration-only change — 4 files, 1 export line each. Phases 1 and 2 are skipped: no new dependencies, no new files, no schema changes, no blocking prerequisites. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[Story]**: Which user story this task belongs to (US1, US2) + +--- + +## Phase 1: Setup + +*Skipped — existing project, no new dependencies, files, or infrastructure required.* + +--- + +## Phase 2: Foundational + +*Skipped — all 4 implementation tasks are independent of each other; no blocking prerequisites.* + +--- + +## Phase 3: User Story 1 — Successful Deployment Without Database at Build Time (Priority: P1) MVP + +**Goal**: The application builds successfully when no database is available, by forcing all `(app)` route group files to render dynamically at request time. + +**Independent Test**: Unset `DATABASE_URL` and run `npm run build`. Build must complete with zero errors and show `(app)` routes as dynamically rendered (`f` or `l` symbol) in build output. + +### Implementation + +- [x] T001 [P] [US1] Add `export const dynamic = 'force-dynamic'` after imports in `src/app/(app)/layout.tsx` +- [x] T002 [P] [US1] Add `export const dynamic = 'force-dynamic'` after imports in `src/app/(app)/holdings/page.tsx` +- [x] T003 [P] [US1] Add `export const dynamic = 'force-dynamic'` after imports in `src/app/(app)/holdings/[id]/page.tsx` +- [x] T004 [P] [US1] Add `export const dynamic = 'force-dynamic'` after imports in `src/app/(app)/portfolio/page.tsx` +- [x] T005 [US1] Verify build succeeds without database: unset `DATABASE_URL`, run `npm run build`, confirm zero errors and that `(app)` routes appear as dynamic (not static) in build output + +**Checkpoint**: User Story 1 complete — application builds successfully with no database available. + +--- + +## Phase 4: User Story 2 — All Authenticated Pages Render Fresh Data (Priority: P2) + +**Goal**: Every page load in the `(app)` route group fetches current data from the database at request time; no stale cached data is ever served. + +**Independent Test**: With the app running, modify a holding in the database directly, refresh `/holdings` — updated data must appear immediately without rebuild or redeploy. + +### Implementation + +- [x] T006 [US2] Create Playwright E2E test for dynamic data freshness in `tests/e2e/dynamic-rendering.spec.ts`: navigate to `/holdings`, record a holding value, update that holding via the API or database, navigate back to `/holdings`, assert the updated value is displayed without rebuild + +**Checkpoint**: User Story 2 verified — `(app)` pages always render fresh data from the database on every request. + +--- + +## Phase 5: Polish & Cross-Cutting Concerns + +**Purpose**: Confirm no regressions in type safety or existing tests after the configuration changes. + +- [x] T007 [P] Run `npx tsc --noEmit` and confirm zero TypeScript errors (SC-003) +- [x] T008 [P] Run `npm run test` and confirm all existing unit and integration tests pass (SC-003) + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **User Story 1 (Phase 3)**: No prerequisites — start immediately +- **User Story 2 (Phase 4)**: T006 depends on T001–T004 (same code changes deliver both stories) +- **Polish (Phase 5)**: T007, T008 depend on T001–T004 being complete + +### User Story Dependencies + +- **User Story 1 (P1)**: No dependencies — start immediately +- **User Story 2 (P2)**: Shares code changes with US1; T006 can begin after T001–T004 complete + +### Within Each Phase + +- T001, T002, T003, T004 are fully parallelizable (different files, no shared state) +- T005 depends on T001–T004 (requires all 4 changes before build test) +- T006 depends on T001–T004 (runtime behavior requires all changes in place) +- T007 and T008 can run in parallel (different toolchains) + +--- + +## Parallel Example: User Story 1 + +```bash +# Run all 4 implementation tasks simultaneously (all different files): +Task: "Add force-dynamic to src/app/(app)/layout.tsx" +Task: "Add force-dynamic to src/app/(app)/holdings/page.tsx" +Task: "Add force-dynamic to src/app/(app)/holdings/[id]/page.tsx" +Task: "Add force-dynamic to src/app/(app)/portfolio/page.tsx" + +# After all 4 complete: +Task: "Verify build succeeds without database (T005)" +``` + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete T001–T004 in parallel (4 file changes) +2. Complete T005 (build verification without database) +3. **STOP and VALIDATE**: Build passes without database — MVP delivered + +### Incremental Delivery + +1. T001–T004 in parallel → All `(app)` files export `force-dynamic` +2. T005 → Build verification complete (US1 delivered) +3. T006 → E2E data freshness test (US2 delivered) +4. T007, T008 in parallel → Zero regressions confirmed (Polish complete) + +--- + +## Notes + +- [P] tasks can be executed simultaneously — they touch different files with no shared dependencies +- All 4 implementation tasks (T001–T004) deliver both user stories; they cannot be separated +- Total diff: 1 line added to each of 4 files (`export const dynamic = 'force-dynamic'`) +- Placement: add the export statement immediately after the last `import` statement in each file +- Do NOT add `force-dynamic` to files outside the `(app)` route group (see research.md R3 for rationale) +- See `specs/004-dynamic-rendering/quickstart.md` for manual verification steps diff --git a/src/app/(app)/holdings/[id]/page.tsx b/src/app/(app)/holdings/[id]/page.tsx index 2793b10..797072d 100644 --- a/src/app/(app)/holdings/[id]/page.tsx +++ b/src/app/(app)/holdings/[id]/page.tsx @@ -2,6 +2,8 @@ import { notFound } from "next/navigation"; import { getHoldings } from "@/actions/holdings"; import { HoldingDetailClient } from "./HoldingDetailClient"; +export const dynamic = 'force-dynamic'; + type Props = { params: Promise<{ id: string }>; }; diff --git a/src/app/(app)/holdings/page.tsx b/src/app/(app)/holdings/page.tsx index 8019100..7d032a3 100644 --- a/src/app/(app)/holdings/page.tsx +++ b/src/app/(app)/holdings/page.tsx @@ -1,6 +1,8 @@ import { getHoldings } from "@/actions/holdings"; import { HoldingsClient } from "./HoldingsClient"; +export const dynamic = 'force-dynamic'; + export default async function HoldingsPage() { const result = await getHoldings(); const holdings = result.success ? result.data : []; diff --git a/src/app/(app)/layout.tsx b/src/app/(app)/layout.tsx index 3c285a8..388b273 100644 --- a/src/app/(app)/layout.tsx +++ b/src/app/(app)/layout.tsx @@ -3,6 +3,8 @@ import { auth } from "@/auth"; import { redirect } from "next/navigation"; import LogoutButton from "./LogoutButton"; +export const dynamic = 'force-dynamic'; + export default async function AppLayout({ children, }: { diff --git a/src/app/(app)/portfolio/page.tsx b/src/app/(app)/portfolio/page.tsx index 2137696..ffe2a14 100644 --- a/src/app/(app)/portfolio/page.tsx +++ b/src/app/(app)/portfolio/page.tsx @@ -4,6 +4,8 @@ import { DonutChart } from "@/components/charts/DonutChart"; import { StackedBarChart } from "@/components/charts/StackedBarChart"; import { AccountFilter } from "@/components/ui/AccountFilter"; +export const dynamic = 'force-dynamic'; + type Props = { searchParams: Promise<{ account?: string }>; }; diff --git a/tests/e2e/dynamic-rendering.spec.ts b/tests/e2e/dynamic-rendering.spec.ts new file mode 100644 index 0000000..ca25708 --- /dev/null +++ b/tests/e2e/dynamic-rendering.spec.ts @@ -0,0 +1,39 @@ +import { test, expect } from "@playwright/test"; + +test("holdings page renders fresh data after update without rebuild (dynamic rendering)", async ({ + page, +}) => { + // Step 1: Add a holding with a unique account name so we can track it + const uniqueAccount = `DynTest-${Date.now()}`; + await page.goto("/holdings"); + await page.click('button:has-text("Add Holding")'); + await expect(page.getByRole("heading", { name: "Add New Holding" })).toBeVisible(); + + await page.fill('input[name="instrumentIdentifier"]', "DYNTEST"); + await page.selectOption('select[name="instrumentType"]', "STOCK"); + await page.fill('input[name="accountName"]', uniqueAccount); + await page.fill('input[name="shares"]', "10"); + await page.fill('input[name="pricePerShare"]', "100"); + await page.click('button[type="submit"]:has-text("Add Holding")'); + + // Verify the holding appears in the list + const holdingRow = page.getByRole("row", { name: /DYNTEST/ }); + await expect(holdingRow).toBeVisible(); + + // Step 2: Navigate to the holding's detail page and update the account name + await holdingRow.click(); + await expect(page.getByRole("heading", { name: /DYNTEST/ })).toBeVisible(); + + await page.click('button:has-text("Edit")'); + const updatedAccount = uniqueAccount + "-updated"; + await page.fill('input[name="accountName"]', updatedAccount); + await page.click('button[type="submit"]:has-text("Save Changes")'); + await expect(page.getByRole("heading", { name: "Edit" })).not.toBeVisible(); + + // Step 3: Full server navigation back to /holdings — if pages were statically + // pre-rendered the old account name would appear; force-dynamic ensures fresh data + await page.goto("/holdings"); + + // Step 4: Assert the updated account name is visible (server fetched fresh DB data) + await expect(page.getByText(updatedAccount)).toBeVisible(); +}); diff --git a/tsconfig.json b/tsconfig.json index 33c3869..ee79ebe 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -15,7 +15,7 @@ "moduleResolution": "bundler", "resolveJsonModule": true, "isolatedModules": true, - "jsx": "preserve", + "jsx": "react-jsx", "incremental": true, "plugins": [ { From 9ffdcbde946b04ec1ad28dfd571b43cd86acd01f Mon Sep 17 00:00:00 2001 From: itinsecurity <98172852+itinsecurity@users.noreply.github.com> Date: Thu, 19 Mar 2026 20:15:12 +0100 Subject: [PATCH 2/5] fix(e2e): fix dynamic-rendering spec strict mode and race condition - Scope row selector to uniqueAccount instead of /DYNTEST/ to avoid strict mode violations when prior runs leave DYNTEST rows in the DB - Wait for the Edit button to reappear after saving (proves the server action completed) before navigating to /holdings Co-Authored-By: Claude Sonnet 4.6 --- tests/e2e/dynamic-rendering.spec.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/e2e/dynamic-rendering.spec.ts b/tests/e2e/dynamic-rendering.spec.ts index ca25708..78e987f 100644 --- a/tests/e2e/dynamic-rendering.spec.ts +++ b/tests/e2e/dynamic-rendering.spec.ts @@ -16,8 +16,9 @@ test("holdings page renders fresh data after update without rebuild (dynamic ren await page.fill('input[name="pricePerShare"]', "100"); await page.click('button[type="submit"]:has-text("Add Holding")'); - // Verify the holding appears in the list - const holdingRow = page.getByRole("row", { name: /DYNTEST/ }); + // Use uniqueAccount to scope the row — avoids strict mode violation if previous + // test runs left DYNTEST rows in the database + const holdingRow = page.getByRole("row", { name: new RegExp(uniqueAccount) }); await expect(holdingRow).toBeVisible(); // Step 2: Navigate to the holding's detail page and update the account name @@ -28,7 +29,8 @@ test("holdings page renders fresh data after update without rebuild (dynamic ren const updatedAccount = uniqueAccount + "-updated"; await page.fill('input[name="accountName"]', updatedAccount); await page.click('button[type="submit"]:has-text("Save Changes")'); - await expect(page.getByRole("heading", { name: "Edit" })).not.toBeVisible(); + // Wait for the edit form to close (Edit button reappears) before navigating away + await expect(page.getByRole("button", { name: "Edit" })).toBeVisible(); // Step 3: Full server navigation back to /holdings — if pages were statically // pre-rendered the old account name would appear; force-dynamic ensures fresh data From 8d7fc98c4fb9a2a9055ee7743502261ab0194c66 Mon Sep 17 00:00:00 2001 From: itinsecurity <98172852+itinsecurity@users.noreply.github.com> Date: Thu, 19 Mar 2026 20:21:06 +0100 Subject: [PATCH 3/5] fix(e2e): use exact match for Edit button to avoid dev toolbar collision Playwright name matching is substring by default; "Open in editor" buttons injected by the dev toolbar match /edit/i, causing a strict mode violation. Adding exact: true constrains the locator to the button with accessible name exactly "Edit". Co-Authored-By: Claude Sonnet 4.6 --- tests/e2e/dynamic-rendering.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/dynamic-rendering.spec.ts b/tests/e2e/dynamic-rendering.spec.ts index 78e987f..1522553 100644 --- a/tests/e2e/dynamic-rendering.spec.ts +++ b/tests/e2e/dynamic-rendering.spec.ts @@ -30,7 +30,7 @@ test("holdings page renders fresh data after update without rebuild (dynamic ren await page.fill('input[name="accountName"]', updatedAccount); await page.click('button[type="submit"]:has-text("Save Changes")'); // Wait for the edit form to close (Edit button reappears) before navigating away - await expect(page.getByRole("button", { name: "Edit" })).toBeVisible(); + await expect(page.getByRole("button", { name: "Edit", exact: true })).toBeVisible(); // Step 3: Full server navigation back to /holdings — if pages were statically // pre-rendered the old account name would appear; force-dynamic ensures fresh data From baf3f8bfc391360eb5a41bda780663638be7c173 Mon Sep 17 00:00:00 2001 From: itinsecurity <98172852+itinsecurity@users.noreply.github.com> Date: Thu, 19 Mar 2026 20:27:29 +0100 Subject: [PATCH 4/5] fix(actions): move EDITABLE_PROFILE_FIELDS out of use server file Next.js 16 forbids non-async-function exports from "use server" files. EDITABLE_PROFILE_FIELDS is a constant array, not a function, so it violates this constraint and crashes the holding detail page at runtime. Move the constant to src/lib/profileFields.ts (a plain module with no "use server" directive) and import it in profiles.ts and the test file. Co-Authored-By: Claude Sonnet 4.6 --- src/actions/profiles.ts | 19 +------------------ src/lib/profileFields.ts | 17 +++++++++++++++++ tests/unit/actions/profiles.test.ts | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) create mode 100644 src/lib/profileFields.ts diff --git a/src/actions/profiles.ts b/src/actions/profiles.ts index ea90120..3b3d466 100644 --- a/src/actions/profiles.ts +++ b/src/actions/profiles.ts @@ -3,24 +3,7 @@ import { revalidatePath } from "next/cache"; import { prisma } from "@/lib/db"; import type { ActionResult, FieldSources, AssetProfileData } from "@/types"; - -export const EDITABLE_PROFILE_FIELDS = [ - "name", - "isin", - "ticker", - "exchange", - "country", - "sector", - "industry", - "fundManager", - "fundCategory", - "equityPct", - "bondPct", - "sectorWeightings", - "geographicWeightings", -] as const; - -type EditableField = (typeof EDITABLE_PROFILE_FIELDS)[number]; +import { EDITABLE_PROFILE_FIELDS, type EditableProfileField as EditableField } from "@/lib/profileFields"; const JSON_FIELDS = new Set(["sectorWeightings", "geographicWeightings"]); const NUMERIC_FIELDS = new Set(["equityPct", "bondPct"]); diff --git a/src/lib/profileFields.ts b/src/lib/profileFields.ts new file mode 100644 index 0000000..aa95f8e --- /dev/null +++ b/src/lib/profileFields.ts @@ -0,0 +1,17 @@ +export const EDITABLE_PROFILE_FIELDS = [ + "name", + "isin", + "ticker", + "exchange", + "country", + "sector", + "industry", + "fundManager", + "fundCategory", + "equityPct", + "bondPct", + "sectorWeightings", + "geographicWeightings", +] as const; + +export type EditableProfileField = (typeof EDITABLE_PROFILE_FIELDS)[number]; diff --git a/tests/unit/actions/profiles.test.ts b/tests/unit/actions/profiles.test.ts index 0096748..1c8a686 100644 --- a/tests/unit/actions/profiles.test.ts +++ b/tests/unit/actions/profiles.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from "vitest"; -import { EDITABLE_PROFILE_FIELDS } from "@/actions/profiles"; +import { EDITABLE_PROFILE_FIELDS } from "@/lib/profileFields"; // Mock DB for unit tests — profiles tests mostly cover validation logic vi.mock("@/lib/db", () => ({ From d51d7e89e61e60a7c455663f80467c89c2711db3 Mon Sep 17 00:00:00 2001 From: itinsecurity <98172852+itinsecurity@users.noreply.github.com> Date: Thu, 19 Mar 2026 20:34:14 +0100 Subject: [PATCH 5/5] fix(e2e): scope account name assertion to table cell getByText(updatedAccount) matches both the account filter