From c2364bf09edbc1a77aa967300ab1063a5037d1c4 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 09:47:58 +0100 Subject: [PATCH 01/10] docs(flags): design for a ship-dark feature flag layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unleash self-hosted in its own Portainer stack, with FastAPI holding the only SDK and Next reading flags through a tagged fetch. The two hard parts are consequences of putting flag state in a service rather than the repo: main stops being the whole truth about what is on, and a flag can now change without the deploy that would have cleared the caches. A code-declared registry bounds the first; webhook-driven revalidateTag handles the second. Cache tagging is deliberately coarse — every server fetch carries the flags tag, not just the flags fetch itself. The first consumer proves why: admission_distance changes the shape of /api/schools/{urn}, so a narrow purge would leave ~25,000 school pages serving the pre-flip render for a week, invisibly. First consumer is the last-distance-offered feature, which is on main and staging and has never reached production. It needs one gate, at the API, because the frontend already no-ops on a missing field. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- .../specs/2026-08-23-feature-flags-design.md | 282 ++++++++++++++++++ 1 file changed, 282 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-23-feature-flags-design.md diff --git a/docs/superpowers/specs/2026-08-23-feature-flags-design.md b/docs/superpowers/specs/2026-08-23-feature-flags-design.md new file mode 100644 index 0000000..cfb5325 --- /dev/null +++ b/docs/superpowers/specs/2026-08-23-feature-flags-design.md @@ -0,0 +1,282 @@ +# Feature Flags — Design + +**Date:** 2026-08-23 +**Status:** approved for planning +**First consumer:** the last-distance-offered feature (`admission_distance`) + +## Goal + +Let work merge to `main` and deploy to production without becoming visible, +so that releasing a feature stops being the same event as deploying it. + +The site has no way to do this today. A feature is either on `main` and live, +or it is on a branch. That forces long-lived branches for anything not ready, +and it makes every promotion to production an all-or-nothing decision about +everything queued behind it. + +This is a **ship-dark** capability, not a kill switch. Flags are expected to +flip on the order of once a month, by a person, deliberately. Nothing here is +designed for flipping something off in seconds under pressure, and nothing +here does percentage rollouts, user targeting or A/B tests — the site has no +user identity to target. + +## Decision: Unleash + +Flag state is held in a self-hosted [Unleash](https://www.getunleash.io/) +instance (Apache-2.0), not in the repository. + +A lighter option was considered and rejected by the project owner: a typed +registry in each runtime with environment-variable overrides set in the +Portainer stack files, which would have needed no new container and kept flag +state in git. The argument for Unleash is that it provides a UI and an audit +log without a deploy, and that flags are expected to become an ongoing +operational tool rather than an occasional one. + +Two consequences follow from choosing a service, and this design exists mostly +to handle them: + +1. **Flag state lives outside the repository.** `main` is no longer the whole + truth about what is switched on. The registry in §2 exists to bound that. +2. **A flag can change without a deploy**, so nothing else invalidates the + caches that a deploy would have cleared. §4 is that mechanism. + +Also considered: Flagsmith (heavier — Django, Postgres and Redis), GrowthBook +(requires MongoDB), and Flipt v2 (the closest conceptual fit, git-native, but +now under the Fair Core Licence — source-available, not OSI open source). + +## 1. Topology + +A third Portainer stack, `docker-compose.portainer.unleash.yml`, holding +`unleashorg/unleash-server` and its own PostgreSQL 16. It is on the macvlan so +both application stacks can reach it, and it belongs to neither of them — a +staging redeploy must not be able to disturb production's flag state, and vice +versa. + +One instance serves both environments. Open-source Unleash ships with +`development` and `production` environments and environment-scoped client +tokens, so the same flag holds independent state in each: staging's FastAPI +carries a `development` token, production's carries a `production` one. + +That property is what makes ship-dark testable. A feature can be **on in +staging and off in production** for as long as it takes, which means the `e2e/` +journeys exercise it against staging while production stays unchanged. + +## 2. The registry + +Unleash supplies flag *state* and the toggle UI. It does not supply the list of +flags. `backend/flags.py` declares every flag the code knows about: + +```python +@dataclass(frozen=True) +class Flag: + name: str # identical in the registry, in Unleash, and in JSON + description: str # one line: what turning this on reveals + added: date # for the staleness test in §8 +``` + +**Every flag defaults to `False`.** There is no per-flag default field, because +a flag that defaults on is not a ship-dark flag — it is a kill switch, and this +design does not offer one. A single unconditional default also means the +fallback path has no branching to get wrong. + +Three reasons the registry is not optional: + +- The Unleash SDK evaluates an unknown flag to `False`. Without a registry that + is an *undeclared* false — indistinguishable from a typo in a flag name. +- `/api/flags` needs a key set to return when Unleash is unreachable. It cannot + enumerate flags it has never heard of. +- A flag present in the Unleash UI but absent from the registry is orphaned, + and should be visibly so rather than quietly authoritative. + +**Naming.** One string, used unchanged as the registry key, the Unleash flag +name, and the JSON key in `/api/flags`. It is snake_case, matching the API's +existing convention (`admission_distance`, `rwm_expected_pct`) and the mirrored +types in `nextjs-app/lib/types.ts`. No case transformation anywhere, so there +is no mapping layer to get wrong. + +## 3. Read paths + +### Backend + +`backend/flags.py` wraps `UnleashClient` behind `is_enabled(name: str) -> bool`. + +Fail-closed is the default rather than something added: the Python SDK +evaluates every flag to `False` until it has synchronised with the server. An +unfinished feature therefore stays hidden when Unleash is unreachable, which is +the correct direction for ship-dark. + +The SDK's fcache directory is mounted on a named volume so a container restart +during an Unleash outage keeps last-known state rather than reverting a +released feature to dark. The registry default remains `False`, so the worst +case is a feature disappearing, never one appearing. + +### Frontend + +`nextjs-app/lib/flags.ts` exposes `getFlags(): Promise`, a single +server-side fetch of `/api/flags` returning a typed record. Server components +only — no flag value reaches the browser bundle, and `package.json` gains no +Unleash dependency. The Unleash client library stays entirely inside the +service that already owns every other piece of data the frontend renders. + +The cost, named plainly: a purely front-end flag must still be declared in a +Python file. It is a flat data edit rather than programming, and the return is +one list, so nobody has to ask which service knows about a given flag. + +### `/api/flags` must not be publicly reachable + +`nextjs-app/app/api/[...path]/route.ts` proxies **everything** under `/api/` to +FastAPI. Left alone, `https://www.schoolcompare.co.uk/api/flags` would return +`{"admission_distance": false, ...}` — publishing the name and state of every +unreleased feature, which defeats the purpose of shipping dark. + +The proxy therefore gains a denylist, and `flags` is on it: a request for a +denied path returns 404 rather than being forwarded. Next's own `getFlags()` is +unaffected because it calls `FASTAPI_URL` directly across the Docker network +and never transits the public proxy. + +This is a general hole rather than a flags-specific one — the proxy will +forward any future internal endpoint too — so the denylist is written as a +named constant with a comment saying what belongs on it. + +## 4. Propagation + +School and place pages carry `revalidate = 604800`. A flag value consulted +during render is baked into the cached HTML, so **polling alone changes +nothing** — the page was rendered days ago. Propagation is push, not pull. + +Two webhook integrations in Unleash, both firing on feature-environment +enable/disable. The webhook cannot set custom headers, so each is authenticated +by a shared secret in the query string. + +1. → `POST /api/admin/flags-changed` on FastAPI. Refreshes the SDK cache, and + rebuilds the sitemap — a route-family flag changes which URLs exist, and the + sitemap is held in memory. +2. → `POST /api/revalidate-flags` on Next. Calls `revalidateTag('flags')`. + +Unleash retries once and can deliver duplicate or out-of-order events, so both +handlers are idempotent: they re-read current state rather than applying a +delta from the payload body. + +### Cache tagging: coarse, deliberately + +**Every server-side fetch in `nextjs-app/lib/` carries the `flags` tag**, not +only the fetch of `/api/flags` itself. + +The tempting rule — tag only those fetches whose response shape a flag can +change — is wrong in a way that fails silently. The first consumer proves it: +`admission_distance` changes the response of `/api/schools/{urn}`, not of +`/api/flags`, so a narrowly-tagged purge would leave ~25,000 school pages +serving the pre-flip render for up to seven days. The failure is invisible +locally and visible to Google. + +Flips are rare and Next serves stale-while-revalidate, so a full purge costs a +gradual re-render rather than a cliff. Correctness is worth more here than +precision. + +## 5. What "off" means, per surface + +| Surface | Off | +|---|---| +| Route | `notFound()`, **and** absent from the sitemap, **and** absent from nav | +| UI element | Not rendered; surrounding page byte-identical to today | +| API field | Key **absent**, not `null` | +| API endpoint | 404, not 403 | + +The three parts of the route rule move together or not at all. Submitting URLs +to Google that return 404 is the bug fixed in PR #124, and a flag is a new way +to reintroduce it. + +An API field is withheld **at the source**, never rendered-but-hidden. The +precedent is already set in this codebase by commit `c9a1892`: `/api/schools/` +is public and unauthenticated, so leaving a withheld field in the payload hands +the record to anyone who opens the network tab. + +## 6. First consumer: `admission_distance` + +The last-distance-offered feature is merged to `main` and live on staging. +Production has never received it: `/api/schools/100010` on production carries +no `admission_distance` key, and no Distance section renders. + +It needs **exactly one gate** — `backend/app.py:809`, where the field is +attached to the school payload: + +```python +"admission_distance": ( + supplementary.get("admission_distance") + if flags.is_enabled("admission_distance") else None +), +``` + +The frontend follows with no change. `DistanceSection` already returns `null` +when `admission_distance?.distance_m == null`, and `PrimarySchoolSections` +already conditions the admissions block on `(admissions || admissionDistance)`. +The off-state is the commonest state on the site — only 57 local authorities +publish cut-off distances at all — so it is well covered by construction. + +The flag does not touch the sitemap: school pages exist either way. + +Intended lifecycle: default off, so production receives the code dark on the +next promotion; on in the `development` environment so staging keeps testing +it; flipped on in `production` when the owner chooses. + +**This flag exercises two of the three surfaces** in §5 — API field and UI +element. No route case ships with it. The route rule is specified but unproven +until a route-shaped flag exists, and should be treated as such. + +## 7. Testing + +**Backend unit.** The registry is well-formed; an unknown flag evaluates +`False`; `/api/flags` returns every declared flag with its default when the +SDK is unreachable; `admission_distance` is absent from the school payload when +the flag is off and present when on. + +**Frontend unit.** `getFlags()` returns declared defaults when `/api/flags` +fails, rather than throwing and taking the page with it. + +**E2E.** Journeys read `/api/flags` and gate flag-dependent assertions on it, +matching the `test.skip` shape the suite already uses. + +One trap to avoid, worth stating because the existing distance journeys walk +straight into it: they already skip when no school has a published figure, so +with the flag off they would skip silently and the suite would go green. The +gate must be explicit — **if `/api/flags` reports `admission_distance` on, then +a school with a cut-off must be found**, converting a silent skip into a real +assertion. + +## 8. Lifecycle + +A flag is temporary scaffolding, and the failure mode of every flag system is +accumulation. + +The registry records the date each flag was added, and a backend test fails any +flag older than **90 days**. Removing a flag means deleting the registry entry, +the branches that read it, and the flag in the Unleash UI. + +Unleash SDK usage metrics stay enabled, so the UI shows which flags are still +being evaluated — the evidence needed to retire one safely. + +## 9. Risks + +**Production gains a homelab dependency.** If Unleash is unreachable when a +production container cold-starts with an empty cache, every flag evaluates +`False` and any feature currently switched on disappears. The fcache volume +covers restarts; the 90-day lifecycle rule bounds how long any feature is +exposed to this. It is a real regression risk and the reason flags must be +retired rather than left on indefinitely. + +**Flag state is not in git.** `main` no longer tells you what production is +showing. The registry lists what *can* be flagged; only the Unleash UI says +what *is*. This is inherent to the choice of a service. + +**A large promotion backlog exists.** Production is running the +pre-SEO-programme build — no place pages, and a sitemap still declaring the +apex host. The first promotion after this work ships that entire backlog. The +flag isolates the distance feature from it and nothing else. + +## Out of scope + +- Percentage rollouts, user targeting, A/B testing, and Unleash strategies + beyond simple on/off. Flags are booleans. +- Pipeline and dbt flags. Airflow and dbt are not flag consumers. +- Client-side flag evaluation. Flags are server-side only. +- Automatic flag removal. The staleness test reports; a person deletes. -- 2.54.0 From e2ca3d79f9471841a22874951f16b7a3225a6988 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:40:35 +0100 Subject: [PATCH 02/10] =?UTF-8?q?docs(flags):=20drop=20the=20webhook=20?= =?UTF-8?q?=E2=80=94=20the=20seven-day=20premise=20was=20wrong?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Next uses the LOWEST revalidate among a route's fetches, not the segment value. School pages fetch school details at 300s and place pages fetch national averages at 3600s, so the effective ISR period is five minutes and one hour respectively — not the seven days the segment declares. A flag flip therefore propagates on its own, well inside the monthly, by-hand cadence these flags are for. That deletes two webhook integrations, a revalidate route, a secret-in-query-string scheme, an idempotency requirement, and the rule that every fetch carry a cache tag — which was the part most likely to rot as fetches are added. Two constraints survive: a flag must never gate content on a force-static page, because app/admissions never revalidates; and a route-family flag must rebuild the sitemap, deferred with the route case since no flag in scope touches it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- .../specs/2026-08-23-feature-flags-design.md | 66 +++++++++++-------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/docs/superpowers/specs/2026-08-23-feature-flags-design.md b/docs/superpowers/specs/2026-08-23-feature-flags-design.md index cfb5325..c8acfb2 100644 --- a/docs/superpowers/specs/2026-08-23-feature-flags-design.md +++ b/docs/superpowers/specs/2026-08-23-feature-flags-design.md @@ -37,8 +37,9 @@ to handle them: 1. **Flag state lives outside the repository.** `main` is no longer the whole truth about what is switched on. The registry in §2 exists to bound that. -2. **A flag can change without a deploy**, so nothing else invalidates the - caches that a deploy would have cleared. §4 is that mechanism. +2. **A flag can change without a deploy**, so nothing else clears the caches + that a deploy would have cleared. §4 establishes how long a flip takes to + become visible, and why that is short enough to need no extra mechanism. Also considered: Flagsmith (heavier — Django, Postgres and Redis), GrowthBook (requires MongoDB), and Flipt v2 (the closest conceptual fit, git-native, but @@ -140,38 +141,49 @@ named constant with a comment saying what belongs on it. ## 4. Propagation -School and place pages carry `revalidate = 604800`. A flag value consulted -during render is baked into the cached HTML, so **polling alone changes -nothing** — the page was rendered days ago. Propagation is push, not pull. +**Time-based revalidation is sufficient. There is no webhook.** -Two webhook integrations in Unleash, both firing on feature-environment -enable/disable. The webhook cannot set custom headers, so each is authenticated -by a shared secret in the query string. +An earlier draft of this section specified two Unleash webhooks and a +`revalidateTag('flags')` purge, on the premise that pages cache for seven days. +That premise was wrong, and checking it removed the most complex part of the +design. -1. → `POST /api/admin/flags-changed` on FastAPI. Refreshes the SDK cache, and - rebuilds the sitemap — a route-family flag changes which URLs exist, and the - sitemap is held in memory. -2. → `POST /api/revalidate-flags` on Next. Calls `revalidateTag('flags')`. +Next uses the **lowest** `revalidate` among a route's fetches to set the +revalidation frequency of the whole route — the segment-level +`export const revalidate` does not override a lower value inside it. Measured +against this codebase: -Unleash retries once and can deliver duplicate or out-of-order events, so both -handlers are idempotent: they re-read current state rather than applying a -delta from the payload body. +| Page family | Segment | Lowest fetch | Effective | +|---|---|---|---| +| `/school/[slug]` | 604800 | `fetchSchoolDetails` at 300 | **5 minutes** | +| `/schools/*` | 604800 | `fetchNationalAverages` at 3600 | **1 hour** | -### Cache tagging: coarse, deliberately +The Unleash SDK polls every 15 seconds, so a flip reaches school pages within +about five minutes and place pages within the hour, unaided. Flags flip +monthly, by hand, deliberately. That is fast enough. -**Every server-side fetch in `nextjs-app/lib/` carries the `flags` tag**, not -only the fetch of `/api/flags` itself. +What this removes: two webhook integrations, a `/api/revalidate-flags` route, a +shared-secret-in-a-query-string scheme, an idempotency requirement against +duplicate and out-of-order delivery, and a rule that every fetch in +`nextjs-app/lib/` carry a cache tag. None of it has to be built, maintained, or +kept correct as new fetches are added. -The tempting rule — tag only those fetches whose response shape a flag can -change — is wrong in a way that fails silently. The first consumer proves it: -`admission_distance` changes the response of `/api/schools/{urn}`, not of -`/api/flags`, so a narrowly-tagged purge would leave ~25,000 school pages -serving the pre-flip render for up to seven days. The failure is invisible -locally and visible to Google. +**If instant flips are ever wanted**, the webhook is the way to add them, and it +is purely additive — nothing in this design has to change first. -Flips are rare and Next serves stale-while-revalidate, so a full purge costs a -gradual re-render rather than a cliff. Correctness is worth more here than -precision. +### Two constraints this leaves behind + +**Never flag content on a `force-static` page.** `app/admissions/page.tsx` +declares `export const dynamic = 'force-static'`, so it is baked at build time +and never revalidates. A flag gating anything on such a page would not take +effect until the next deploy, silently. If a flag ever needs to reach one, that +page must first move to ISR. + +**A route-family flag still needs the sitemap rebuilt.** The sitemap is held in +memory and rebuilt only at startup or via `POST /api/admin/regenerate-sitemap`. +No flag in scope touches the sitemap (§6), so this is deferred with the route +case rather than solved now — but a route flag must not ship without it, or the +sitemap will advertise URLs that `notFound()`. ## 5. What "off" means, per surface -- 2.54.0 From c339c2f1a1987cc1d4b32865abc57df25a6eadfb Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:46:15 +0100 Subject: [PATCH 03/10] docs(flags): implementation plan, eight tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 1 is the Unleash stack and ends with a human step — the Portainer deploy and the token generation cannot be automated from here. Nothing else blocks on it: an unset UNLEASH_URL means every flag is False, which is what local development and CI get, so the whole suite runs without a flag server existing. Self-review caught three defects in the plan itself. get_supplementary_data takes (db, urn), not (urn), and the test DataFrame was minimised to the point where the endpoint would have failed for reasons unrelated to flags — both now copy the known-good shape from test_school_details.py. The proxy test needs the node jest environment, since NextRequest wants Fetch API globals jsdom does not provide. And the e2e off-state check hardcoded a URN, so a 404 page would have satisfied it without proving anything. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- .../plans/2026-08-23-feature-flags.md | 1106 +++++++++++++++++ 1 file changed, 1106 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-23-feature-flags.md diff --git a/docs/superpowers/plans/2026-08-23-feature-flags.md b/docs/superpowers/plans/2026-08-23-feature-flags.md new file mode 100644 index 0000000..4e68f34 --- /dev/null +++ b/docs/superpowers/plans/2026-08-23-feature-flags.md @@ -0,0 +1,1106 @@ +# Feature Flags (Unleash) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let a merged feature reach production without becoming visible, and be switched on later from a UI. + +**Architecture:** A self-hosted Unleash instance in its own Portainer stack holds flag state. FastAPI is the only service with an Unleash SDK; it declares every flag in a code registry, evaluates them fail-closed, and exposes them at an internal-only `/api/flags`. Next reads that endpoint server-side. No webhook — time-based ISR already propagates a flip within five minutes on school pages. + +**Tech Stack:** Unleash OSS (Apache-2.0), `UnleashClient` (Python), FastAPI, Next.js 15 App Router, Playwright, pytest, Jest. + +**Spec:** `docs/superpowers/specs/2026-08-23-feature-flags-design.md` + +## Global Constraints + +- **Every flag defaults to `False`.** There is no per-flag default field. A flag that defaults on is a kill switch, which this design does not offer. +- **One string per flag**, used unchanged as the registry key, the Unleash flag name, and the JSON key in `/api/flags`. snake_case, matching the API's existing convention. No case transformation anywhere. +- **Fail closed.** Unleash unreachable, unconfigured, or erroring ⇒ every flag is `False`. Never raise out of a flag evaluation. +- **`/api/flags` must never be publicly reachable.** It names unreleased features. +- **An API field that is off is absent, not `null`,** and withheld at the source. `/api/schools/` is public and unauthenticated (precedent: commit `c9a1892`). +- **Never flag content on a `force-static` page.** `app/admissions/page.tsx` never revalidates. +- **Reading flags in a route pins that route to a 300s ISR floor** — Next uses the lowest `revalidate` among a route's fetches. +- Backend tests run via: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q` +- Never push to `main`. Branch, PR, let checks pass. +- Do not start a local server to test the application; it does not work. + +## File Structure + +| File | Responsibility | +|---|---| +| `docker-compose.portainer.unleash.yml` (new) | The Unleash stack: server + its own Postgres. Owned by neither app stack. | +| `backend/flags.py` (new) | The whole flag layer: registry, client lifecycle, `is_enabled`, `all_flags`. One module, ~90 lines. | +| `backend/config.py` (modify) | Four `unleash_*` settings. | +| `backend/app.py` (modify) | `/api/flags` endpoint; `flags.init()` in `lifespan`; the `admission_distance` gate. | +| `backend/tests/test_flags.py` (new) | Registry shape, fail-closed evaluation, staleness tripwire, endpoint. | +| `nextjs-app/lib/flags.ts` (new) | `getFlags()` — one server-side fetch, typed, never throws. | +| `nextjs-app/app/api/[...path]/route.ts` (modify) | Internal-only denylist so the public proxy will not forward `/api/flags`. | +| `nextjs-app/__tests__/lib/flags.test.ts` (new) | `getFlags` fallback behaviour. | +| `nextjs-app/__tests__/api/proxyDenylist.test.ts` (new) | The proxy 404s internal paths. | +| `e2e/tests/journeys.spec.ts` (modify) | Flag-aware journeys; the explicit-gate rule from spec §7. | +| `requirements.txt` (modify) | `UnleashClient`. | +| `docker-compose.yml`, `docker-compose.portainer.yml`, `docker-compose.portainer.staging.yml` (modify) | `UNLEASH_*` env for the backend service. | + +--- + +### Task 1: The Unleash stack + +**Files:** +- Create: `docker-compose.portainer.unleash.yml` +- Modify: `docs/DEPLOY.md` + +**Interfaces:** +- Consumes: nothing. +- Produces: a reachable Unleash server, and two client tokens — one scoped to the `development` environment, one to `production` — for Task 2's `UNLEASH_API_TOKEN`. + +> **This task ends with a human step.** The stack must be deployed in Portainer and the tokens generated by hand; neither can be automated from here. Every later task works without it — flags simply evaluate `False` when `UNLEASH_URL` is unset, which is the correct local and CI behaviour. + +- [ ] **Step 1: Write the stack file** + +```yaml +# Portainer Stack Definition for School Compare — UNLEASH (feature flags) +# +# Deploy as a *separate* Portainer stack ("schoolcompare-unleash"), alongside +# the production and staging stacks. It deliberately belongs to neither: a +# staging redeploy must not be able to disturb production's flag state. +# +# One instance serves both environments. Open-source Unleash ships with +# `development` and `production` environments and environment-scoped client +# tokens, so the same flag holds independent state in each. +# +# Portainer environment variables (set in Portainer UI -> Stack -> Environment): +# UNLEASH_DB_PASSWORD — PostgreSQL password for the Unleash database +# UNLEASH_ADMIN_PASSWORD — initial admin password for the Unleash UI +# UNLEASH_IP — macvlan IP for the Unleash server (default 10.0.1.152) + +services: + + unleash_db: + container_name: sc_unleash_postgres + image: postgres:16-alpine + environment: + POSTGRES_USER: unleash + POSTGRES_PASSWORD: ${UNLEASH_DB_PASSWORD} + POSTGRES_DB: unleash + volumes: + - unleash_postgres_data:/var/lib/postgresql/data + networks: + - unleash + healthcheck: + test: ["CMD-SHELL", "pg_isready -U unleash"] + interval: 10s + timeout: 5s + retries: 5 + start_period: 10s + restart: unless-stopped + + unleash: + container_name: sc_unleash + image: unleashorg/unleash-server:6 + environment: + DATABASE_URL: postgres://unleash:${UNLEASH_DB_PASSWORD}@unleash_db:5432/unleash + DATABASE_SSL: "false" + INIT_ADMIN_API_TOKENS: "" + UNLEASH_DEFAULT_ADMIN_PASSWORD: ${UNLEASH_ADMIN_PASSWORD} + depends_on: + unleash_db: + condition: service_healthy + networks: + unleash: {} + macvlan: + ipv4_address: ${UNLEASH_IP:-10.0.1.152} + healthcheck: + test: ["CMD-SHELL", "wget -qO- http://localhost:4242/health || exit 1"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 30s + restart: unless-stopped + +networks: + unleash: + driver: bridge + macvlan: + external: true + name: macvlan + +volumes: + unleash_postgres_data: +``` + +- [ ] **Step 2: Document the runbook** + +Append to `docs/DEPLOY.md`: + +```markdown +## Feature flags (Unleash) + +Flag state lives in a self-hosted Unleash instance, deployed as its own +Portainer stack from `docker-compose.portainer.unleash.yml`. It is separate +from the application stacks on purpose — redeploying staging must not be able +to disturb production's flags. + +### First-time setup + +1. Deploy the stack in Portainer. Set `UNLEASH_DB_PASSWORD`, + `UNLEASH_ADMIN_PASSWORD` and (optionally) `UNLEASH_IP`. +2. Log in to the UI at `http://:4242` as `admin`. +3. Create one **client** API token per environment: + - `schoolcompare-staging`, environment **development** + - `schoolcompare-prod`, environment **production** + Client tokens, not admin tokens — the backend only reads. +4. Put each token in the matching Portainer stack's `UNLEASH_API_TOKEN` + variable, and set `UNLEASH_URL` to `http://:4242/api`. +5. Redeploy the application stacks. + +### Turning a feature on + +Flags are declared in `backend/flags.py` and appear in the Unleash UI on the +first evaluation after the backend starts. Toggle the flag in the environment +you want. A flip reaches school pages within about five minutes and place pages +within the hour — Next's ISR does the propagating, there is no webhook. + +If `UNLEASH_URL` is unset, every flag evaluates to `False`. That is the correct +behaviour for local development and CI, and means nothing here is required to +run the tests. +``` + +- [ ] **Step 3: Commit** + +```bash +git add docker-compose.portainer.unleash.yml docs/DEPLOY.md +git commit -m "feat(flags): add the Unleash stack and its runbook" +``` + +- [ ] **Step 4: Hand off the manual step** + +Tell the human: the stack file is ready, and Steps 1–5 of the runbook are theirs to run. Note that the remaining tasks do not block on it. + +--- + +### Task 2: The flag registry and client + +**Files:** +- Create: `backend/flags.py` +- Create: `backend/tests/test_flags.py` +- Modify: `backend/config.py` +- Modify: `requirements.txt` + +**Interfaces:** +- Consumes: `settings` from `backend/config.py`. +- Produces: + - `backend.flags.Flag` — frozen dataclass with `name: str`, `description: str`, `added: date` + - `backend.flags.REGISTRY: dict[str, Flag]` + - `backend.flags.MAX_FLAG_AGE_DAYS: int` (= 90) + - `backend.flags.init() -> None` + - `backend.flags.is_enabled(name: str) -> bool` + - `backend.flags.all_flags() -> dict[str, bool]` + +- [ ] **Step 1: Write the failing tests** + +Create `backend/tests/test_flags.py`: + +```python +"""Tests for the feature flag layer (spec 2026-08-23). + +None of these need a running Unleash. That is the point: an unset UNLEASH_URL +means every flag is False, which is what local development and CI get. +""" + +from datetime import date, timedelta + +import pytest + +from backend import flags + + +def test_every_declared_flag_is_keyed_by_its_own_name(): + # One string is the registry key, the Unleash flag name and the JSON key. + # A mismatch here would mean the UI toggles a flag the code never reads. + for key, flag in flags.REGISTRY.items(): + assert key == flag.name + + +def test_flag_names_are_snake_case(): + # Matches the API's existing convention (admission_distance, + # rwm_expected_pct) so no case transformation exists to get wrong. + for name in flags.REGISTRY: + assert name == name.lower() + assert "-" not in name and " " not in name + + +def test_an_unconfigured_client_evaluates_every_flag_false(monkeypatch): + monkeypatch.setattr(flags, "_client", None) + for name in flags.REGISTRY: + assert flags.is_enabled(name) is False + + +def test_an_undeclared_flag_is_false_rather_than_an_error(monkeypatch): + # A typo'd flag name must not raise in a request path. It is logged as an + # error, because an undeclared flag is always a bug. + monkeypatch.setattr(flags, "_client", None) + assert flags.is_enabled("no_such_flag") is False + + +def test_an_exploding_client_is_false_rather_than_a_500(monkeypatch): + class Boom: + def is_enabled(self, *a, **kw): + raise RuntimeError("unleash is on fire") + + monkeypatch.setattr(flags, "_client", Boom()) + name = next(iter(flags.REGISTRY)) + assert flags.is_enabled(name) is False + + +def test_all_flags_reports_every_declared_flag(monkeypatch): + monkeypatch.setattr(flags, "_client", None) + assert set(flags.all_flags()) == set(flags.REGISTRY) + assert all(v is False for v in flags.all_flags().values()) + + +def test_a_flag_older_than_the_limit_fails_this_test(): + """A tripwire, not an assertion about correctness. + + Flags are temporary scaffolding and the failure mode of every flag system + is accumulation. This fails on the day a flag turns 90, on whatever PR + happens to be open — which is the point: someone has to decide. + + To fix: delete the flag and the branches that read it, or, if it genuinely + still needs to exist, move its `added` date and say why in the commit. + """ + stale = [ + f.name for f in flags.REGISTRY.values() + if date.today() - f.added > timedelta(days=flags.MAX_FLAG_AGE_DAYS) + ] + assert not stale, ( + f"Flags older than {flags.MAX_FLAG_AGE_DAYS} days: {stale}. " + "Remove the flag and the code branches it guards, or move its `added` " + "date deliberately." + ) +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_flags.py -q` + +Expected: FAIL — `ModuleNotFoundError: No module named 'backend.flags'` + +- [ ] **Step 3: Add the dependency** + +Append to `requirements.txt`: + +``` +UnleashClient==6.0.1 +``` + +- [ ] **Step 4: Add the settings** + +In `backend/config.py`, inside `class Settings`, after the Typesense block: + +```python + # Feature flags (Unleash). An empty unleash_url disables flags entirely and + # every flag evaluates False — the correct behaviour for local development + # and CI, and the reason no test here needs a running Unleash. + unleash_url: str = "" + unleash_api_token: str = "" + unleash_app_name: str = "schoolcompare-backend" + # On a named volume, so a restart during an Unleash outage keeps + # last-known state instead of reverting a released feature to dark. + unleash_cache_directory: str = "/app/.unleash" +``` + +- [ ] **Step 5: Write the module** + +Create `backend/flags.py`: + +```python +"""Feature flags: what can be switched, and what is switched right now. + +Ship-dark, not a kill switch. Flags let work merge and deploy without becoming +visible; they are expected to flip about monthly, by a person, deliberately. +Nothing here does percentage rollouts or user targeting — the site has no user +identity to target. + +Unleash holds the state. It does not hold the list. REGISTRY below is that +list, and it exists for three reasons: the SDK evaluates an unknown flag to +False, so without a registry that is an *undeclared* False, indistinguishable +from a typo; /api/flags needs a key set to return when Unleash is unreachable; +and a flag in the UI but not in the registry is orphaned and should be visibly +so rather than quietly authoritative. + +Every flag defaults to False. There is no per-flag default, because a flag that +defaults on is a kill switch, and this is not one. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from datetime import date + +from .config import settings + +logger = logging.getLogger(__name__) + +# A flag is temporary scaffolding. See test_a_flag_older_than_the_limit. +MAX_FLAG_AGE_DAYS = 90 + + +@dataclass(frozen=True) +class Flag: + # One string: the registry key, the Unleash flag name, and the JSON key in + # /api/flags. snake_case, matching the API's existing convention. No case + # transformation anywhere, so there is no mapping layer to get wrong. + name: str + description: str # one line: what turning this on reveals + added: date # for the staleness tripwire + + +REGISTRY: dict[str, Flag] = { + f.name: f for f in ( + Flag( + name="admission_distance", + description=( + "The last-distance-offered figure on the Admissions tile and " + "the 'How far away are you?' section on school pages." + ), + added=date(2026, 8, 23), + ), + ) +} + + +_client = None + + +def init() -> None: + """Start the Unleash client, or log why flags are all off. + + Called once from the app lifespan. Never raises: a flag system that can + stop the API from booting is worse than one that is switched off. + """ + global _client + if not settings.unleash_url or not settings.unleash_api_token: + logger.warning( + "Unleash is not configured (UNLEASH_URL / UNLEASH_API_TOKEN); " + "every feature flag evaluates to False.") + return + + try: + from UnleashClient import UnleashClient + + _client = UnleashClient( + url=settings.unleash_url, + app_name=settings.unleash_app_name, + custom_headers={"Authorization": settings.unleash_api_token}, + cache_directory=settings.unleash_cache_directory, + refresh_interval=15, + ) + _client.initialize_client() + logger.info("Unleash client initialised against %s", settings.unleash_url) + except Exception: + # Fail closed and keep serving. The SDK also evaluates everything False + # until its first successful sync, so this is the same direction. + _client = None + logger.exception("Unleash client failed to start; flags are all False.") + + +def is_enabled(name: str) -> bool: + """Whether `name` is on. False for anything unknown, unreachable or broken.""" + if name not in REGISTRY: + logger.error( + "undeclared feature flag %r was evaluated; returning False. " + "Add it to backend/flags.py REGISTRY or fix the name.", name) + return False + if _client is None: + return False + try: + return bool(_client.is_enabled( + name, fallback_function=lambda feature_name, context: False)) + except Exception: + logger.exception("flag %r failed to evaluate; returning False", name) + return False + + +def all_flags() -> dict[str, bool]: + """Every declared flag and its current value. Serves /api/flags.""" + return {name: is_enabled(name) for name in REGISTRY} +``` + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_flags.py -q` + +Expected: PASS, 7 tests. + +- [ ] **Step 7: Run the whole backend suite** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q` + +Expected: all pass. Nothing else imports `flags` yet. + +- [ ] **Step 8: Commit** + +```bash +git add backend/flags.py backend/tests/test_flags.py backend/config.py requirements.txt +git commit -m "feat(flags): the registry and a fail-closed Unleash client" +``` + +--- + +### Task 3: `/api/flags`, and keeping it off the public internet + +**Files:** +- Modify: `backend/app.py` +- Modify: `nextjs-app/app/api/[...path]/route.ts` +- Modify: `backend/tests/test_flags.py` +- Create: `nextjs-app/__tests__/api/proxyDenylist.test.ts` + +**Interfaces:** +- Consumes: `backend.flags.init`, `backend.flags.all_flags`. +- Produces: `GET /api/flags` → `{"": bool, ...}`, reachable only from inside the Docker network. + +> The endpoint and its exposure control ship together on purpose. The moment +> `/api/flags` exists, the public proxy at `app/api/[...path]/route.ts` will +> forward it, publishing the name and state of every unreleased feature. + +- [ ] **Step 1: Write the failing backend test** + +Append to `backend/tests/test_flags.py`: + +```python +def _client(): + from fastapi.testclient import TestClient + from backend import app as app_module + return TestClient(app_module.app, raise_server_exceptions=False) + + +def test_the_flags_endpoint_lists_every_declared_flag(monkeypatch): + monkeypatch.setattr(flags, "_client", None) + body = _client().get("/api/flags").json() + assert set(body) == set(flags.REGISTRY) + + +def test_the_flags_endpoint_answers_false_when_unleash_is_unreachable(monkeypatch): + # The endpoint must still answer. A frontend that cannot read flags renders + # everything dark, which is right; one that gets a 500 renders nothing. + monkeypatch.setattr(flags, "_client", None) + res = _client().get("/api/flags") + assert res.status_code == 200 + assert all(v is False for v in res.json().values()) +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_flags.py -q` + +Expected: FAIL — the endpoint 404s, so `set(body)` is `{"detail"}`. + +- [ ] **Step 3: Add the import and the lifespan call** + +In `backend/app.py`, add to the existing relative imports (after `from .data_loader import get_data_info as get_db_info`): + +```python +from . import flags +``` + +Inside `async def lifespan(app: FastAPI):`, immediately after the `print("Loading school data from marts...")` line: + +```python + flags.init() +``` + +- [ ] **Step 4: Add the endpoint** + +In `backend/app.py`, directly above `@app.get("/api/data-info")`: + +```python +@app.get("/api/flags") +@limiter.limit(f"{settings.rate_limit_per_minute}/minute") +async def get_feature_flags(request: Request): + """Every declared flag and its current value. + + Internal only. The Next proxy denies this path, because the response names + every unreleased feature the codebase knows about — which is exactly what + shipping dark is meant to keep quiet. + """ + return flags.all_flags() +``` + +- [ ] **Step 5: Run the backend tests to verify they pass** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q` + +Expected: all pass. + +- [ ] **Step 6: Write the failing proxy test** + +Create `nextjs-app/__tests__/api/proxyDenylist.test.ts`: + +```typescript +/** + * The /api/* proxy is public. Anything it forwards is on the internet. + * + * @jest-environment node + */ +// The docblock above is load-bearing. jest.config.js sets jsdom globally, and +// NextRequest/NextResponse need the Web Fetch API globals that only the node +// environment provides — under jsdom this suite fails on import, not on an +// assertion. +import { NextRequest } from 'next/server'; +import { GET } from '@/app/api/[...path]/route'; + +function request(path: string) { + return new NextRequest(`http://localhost:3000/api/${path}`); +} + +describe('public API proxy', () => { + it('refuses to forward internal-only paths', async () => { + // /api/flags names every unreleased feature and its state. Forwarding it + // publishes the thing shipping dark exists to keep quiet. + const res = await GET(request('flags'), { params: Promise.resolve({ path: ['flags'] }) }); + expect(res.status).toBe(404); + }); + + it('does not deny a path that merely starts with the same letters', async () => { + // A prefix match would take /api/flagship down with /api/flags. + const res = await GET( + request('flagship'), { params: Promise.resolve({ path: ['flagship'] }) }); + expect(res.status).not.toBe(404); + }); +}); +``` + +- [ ] **Step 7: Run it to verify it fails** + +Run: `cd nextjs-app && npx jest __tests__/api/proxyDenylist.test.ts` + +Expected: FAIL — the first case returns 502 (no backend running in the test env), not 404. + +- [ ] **Step 8: Add the denylist** + +In `nextjs-app/app/api/[...path]/route.ts`, after the `STRIPPED_RESPONSE_HEADERS` constant: + +```typescript +/* + * API paths this public proxy must not forward. + * + * Matched on the first segment, exactly — a prefix match would take + * /api/flagship down with /api/flags. + * + * `flags` is here because GET /api/flags names every unreleased feature the + * codebase knows about, along with whether it is on. Publishing that defeats + * the point of shipping dark. Next reads it server-side via FASTAPI_URL, on + * the Docker network, which never transits this route. + * + * Anything else internal-only belongs here too. + */ +const INTERNAL_ONLY_SEGMENTS = new Set(['flags']); +``` + +And at the top of `handler`, immediately after `const { path } = await ctx.params;`: + +```typescript + if (INTERNAL_ONLY_SEGMENTS.has(path[0])) { + return NextResponse.json({ detail: 'Not Found' }, { status: 404 }); + } +``` + +- [ ] **Step 9: Run the frontend tests** + +Run: `cd nextjs-app && npx jest && npx tsc --noEmit` + +Expected: all pass, typecheck clean. + +- [ ] **Step 10: Commit** + +```bash +git add backend/app.py backend/tests/test_flags.py 'nextjs-app/app/api/[...path]/route.ts' nextjs-app/__tests__/api/proxyDenylist.test.ts +git commit -m "feat(flags): serve /api/flags, and keep the public proxy off it" +``` + +--- + +### Task 4: Reading flags from Next + +**Files:** +- Create: `nextjs-app/lib/flags.ts` +- Create: `nextjs-app/__tests__/lib/flags.test.ts` + +**Interfaces:** +- Consumes: `GET /api/flags`. +- Produces: `getFlags(): Promise` where `type Flags = Record`, and `FLAGS_REVALIDATE = 300`. + +> This ships with no consumer. The first flag needs none — the backend +> withholds the field and the frontend follows (Task 5). It is built anyway +> because "UI elements on existing pages" is one of the three surfaces this +> capability is for, and a flag layer that cannot gate a UI element is +> incomplete. Keep it minimal; do not add helpers nothing calls. + +- [ ] **Step 1: Write the failing test** + +Create `nextjs-app/__tests__/lib/flags.test.ts`: + +```typescript +import { getFlags } from '@/lib/flags'; + +describe('getFlags', () => { + afterEach(() => { jest.restoreAllMocks(); }); + + it('returns the flags the API reports', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue({ + ok: true, + json: async () => ({ admission_distance: true }), + } as Response); + await expect(getFlags()).resolves.toEqual({ admission_distance: true }); + }); + + it('returns no flags rather than throwing when the API is down', async () => { + // A page that cannot read flags must render everything dark, not 500. + // Fail-closed is the same direction as the backend's default. + jest.spyOn(global, 'fetch').mockRejectedValue(new Error('ECONNREFUSED')); + await expect(getFlags()).resolves.toEqual({}); + }); + + it('returns no flags rather than throwing on a non-200', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue({ + ok: false, status: 503, + } as Response); + await expect(getFlags()).resolves.toEqual({}); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `cd nextjs-app && npx jest __tests__/lib/flags.test.ts` + +Expected: FAIL — `Cannot find module '@/lib/flags'`. + +- [ ] **Step 3: Write the module** + +Create `nextjs-app/lib/flags.ts`: + +```typescript +/** + * Reading feature flags. + * + * Server-side only. No flag value reaches the browser bundle, and there is no + * Unleash dependency in package.json — the SDK lives in FastAPI, which already + * owns every other piece of data this app renders. + * + * Flags are declared in backend/flags.py. A purely front-end flag still has to + * be declared there; it is a flat data edit, and the return is that one list + * answers "what flags exist" for the whole system. + */ + +export type Flags = Record; + +/* + * Reading flags pins the calling route to this ISR floor: Next uses the LOWEST + * revalidate among a route's fetches to set the whole route's revalidation + * frequency. 300s matches what /school/[slug] already sits at, so a page that + * reads flags is no more dynamic than a school page already is. + * + * It is also what makes a flip propagate without a webhook: five minutes on + * school pages, an hour on place pages, against flags that flip monthly. + */ +export const FLAGS_REVALIDATE = 300; + +const API = process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL + || 'http://localhost:8000/api'; + +/** Every flag and its value. Never throws: an unreadable flag is a dark one. */ +export async function getFlags(): Promise { + try { + const res = await fetch(`${API}/flags`, { + next: { revalidate: FLAGS_REVALIDATE }, + }); + if (!res.ok) return {}; + return await res.json(); + } catch { + return {}; + } +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cd nextjs-app && npx jest __tests__/lib/flags.test.ts && npx tsc --noEmit` + +Expected: PASS, 3 tests, typecheck clean. + +- [ ] **Step 5: Commit** + +```bash +git add nextjs-app/lib/flags.ts nextjs-app/__tests__/lib/flags.test.ts +git commit -m "feat(flags): server-side getFlags for the frontend" +``` + +--- + +### Task 5: Put `admission_distance` behind the flag + +**Files:** +- Modify: `backend/app.py:809` +- Modify: `backend/tests/test_flags.py` + +**Interfaces:** +- Consumes: `backend.flags.is_enabled`. +- Produces: `/api/schools/{urn}` omits `admission_distance` unless the flag is on. + +> One gate, at the source. The frontend needs no change: `DistanceSection` +> already returns `null` when `admissionDistance?.distance_m == null`, and +> `PrimarySchoolSections`/`SecondarySchoolSections` already condition the +> admissions block on `(admissions || admissionDistance)`. Only 57 local +> authorities publish cut-offs, so the off-path is the commonest path on the +> site and is well covered already. + +- [ ] **Step 1: Write the failing tests** + +Append to `backend/tests/test_flags.py`: + +```python +def _school_payload(monkeypatch, *, flag_on: bool): + """Fetch one school's payload with the distance flag forced on or off. + + The DataFrame shape is copied from test_school_details.py rather than + minimised: the endpoint reads a wide set of GIAS columns, and a trimmed + frame fails for reasons that have nothing to do with flags. + """ + import numpy as np + import pandas as pd + from fastapi.testclient import TestClient + from backend import app as app_module + + df = pd.DataFrame([{ + "urn": 150275, + "school_name": "West London Performing Arts Academy", + "phase": "Secondary", + "school_type": "Special post 16 institution", + "trust_name": None, + "religious_denomination": "Does not apply", + "gender": None, + "age_range": "16-25", + "admissions_policy": None, + "capacity": np.nan, + "gias_total_pupils": np.nan, + "headteacher_name": None, + "website": None, + "ofsted_grade": np.nan, + "local_authority": "Ealing", + "address": "268 Northfield Avenue, London, W5 4UB", + "postcode": "W5 4UB", + "latitude": 51.4986, + "longitude": -0.3148, + "year": np.nan, + "total_pupils": np.nan, + "eligible_pupils": np.nan, + "rwm_expected_pct": np.nan, + }]) + + monkeypatch.setattr(app_module, "load_school_data", lambda: df) + # Two arguments: get_supplementary_data(db, urn). See backend/app.py:763. + monkeypatch.setattr( + app_module, "get_supplementary_data", + lambda db, urn: {"admission_distance": {"distance_m": 772.49, + "year": 2024}}) + monkeypatch.setattr(flags, "is_enabled", lambda name: flag_on) + + client = TestClient(app_module.app, raise_server_exceptions=False) + res = client.get("/api/schools/150275") + assert res.status_code == 200, res.text + return res.json() + + +def test_the_distance_field_is_absent_when_the_flag_is_off(monkeypatch): + """Absent, not null, and withheld at the source. + + /api/schools/ is public and unauthenticated. Leaving a withheld field in + the payload while declining to render it hands the record to anyone who + opens the network tab — the reasoning already recorded in c9a1892. + """ + body = _school_payload(monkeypatch, flag_on=False) + assert "admission_distance" not in body + + +def test_the_distance_field_is_present_when_the_flag_is_on(monkeypatch): + body = _school_payload(monkeypatch, flag_on=True) + assert body["admission_distance"]["distance_m"] == 772.49 +``` + +- [ ] **Step 2: Run them to verify they fail** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_flags.py -q` + +Expected: FAIL on `test_the_distance_field_is_absent_when_the_flag_is_off` — the key is present with value `None`. + +- [ ] **Step 3: Gate the field** + +In `backend/app.py`, replace line 809: + +```python + "admission_distance": supplementary.get("admission_distance"), +``` + +with: + +```python + # Behind a flag, and withheld at the source rather than rendered-but- + # hidden: this endpoint is public and unauthenticated, so a field left + # in the payload is a published field. The key is absent, not null — + # null would state that this school has no cut-off, which is a + # different claim from "we are not publishing cut-offs". + **({"admission_distance": supplementary.get("admission_distance")} + if flags.is_enabled("admission_distance") else {}), +``` + +- [ ] **Step 4: Run the backend suite** + +Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q` + +Expected: all pass. + +- [ ] **Step 5: Check the frontend type tolerates an absent key** + +Run: `cd nextjs-app && npx tsc --noEmit` + +`lib/types.ts:364` declares `admission_distance: SchoolAdmissionDistance | null`. An absent key is `undefined`, which that type does not admit. Change it to: + +```typescript + /** + * Absent — not null — when the admission_distance flag is off. Null means + * "this school has no published cut-off"; absent means "cut-offs are not + * being published at all". They are different claims and the type says so. + */ + admission_distance?: SchoolAdmissionDistance | null; +``` + +`app/school/[slug]/page.tsx` already reads it as `admission_distance ?? null`, so no call site changes. + +- [ ] **Step 6: Run the frontend suite** + +Run: `cd nextjs-app && npx jest && npx tsc --noEmit && npm run build` + +Expected: all pass, build green. + +- [ ] **Step 7: Commit** + +```bash +git add backend/app.py backend/tests/test_flags.py nextjs-app/lib/types.ts +git commit -m "feat(flags): ship last-distance-offered dark behind a flag" +``` + +--- + +### Task 6: Flag-aware E2E journeys + +**Files:** +- Modify: `e2e/tests/journeys.spec.ts` + +**Interfaces:** +- Consumes: `GET /api/flags` (reachable from Playwright, which runs against the frontend's own origin — note that the *proxy* denies it, so these read it through the same denial and must handle that; see Step 1). + +- [ ] **Step 1: Write the journeys** + +The existing distance journeys at `e2e/tests/journeys.spec.ts:1214–1310` use +`schoolWithCutoff()`, which returns `null` and skips when no school has a +figure. With the flag off they would skip silently and the suite would go +green — a flag accidentally off in staging would look like a pass. + +`/api/flags` is denied by the public proxy, so Playwright cannot read it +directly. The flag's observable effect is what the tests assert on instead: +whether the field appears in a school payload at all. + +Insert directly above the `schoolWithCutoff` helper: + +```typescript +/** + * Whether the last-distance-offered feature is switched on here. + * + * Read from the data rather than from /api/flags, which the public proxy + * denies on purpose — the endpoint names unreleased features. The observable + * effect is the field's presence: the flag is off iff no candidate school + * carries an `admission_distance` key at all. + * + * The distinction that matters: `admission_distance: null` means this school + * has no published cut-off, and the key being ABSENT means cut-offs are not + * being published at all. + */ +async function distanceFeatureIsOn(page: Page): Promise { + for (const urn of CUTOFF_CANDIDATE_URNS) { + const res = await page.request.get(`/api/schools/${urn}`); + if (!res.ok()) continue; + if ('admission_distance' in (await res.json())) return true; + } + return false; +} +``` + +Then add, directly after the `schoolWithCutoff` helper: + +```typescript +test('when the distance feature is on, a school with a cut-off is findable', async ({ page }) => { + /* + * The gate that stops the other distance journeys passing vacuously. + * + * They all skip when schoolWithCutoff() finds nothing, which is right when + * the feature is off — but it means a feature that is *supposed* to be on + * and is silently broken shows up as a green run full of skips. This test + * fails in that case. + */ + test.skip(!(await distanceFeatureIsOn(page)), + 'the admission_distance flag is off in this environment'); + + expect(await schoolWithCutoff(page), + 'the distance feature is on, but no candidate school has a cut-off — ' + + 'the flag is on and the data or the query behind it is broken') + .not.toBeNull(); +}); + +test('with the distance feature off, the section is absent rather than empty', async ({ page }) => { + // Shipping dark means the page renders as it did before the feature existed, + // not as a feature with its content removed. + test.skip(await distanceFeatureIsOn(page), + 'the admission_distance flag is on in this environment'); + + // A school that exists, found rather than hardcoded — a 404 page would + // satisfy the absent-heading assertion without proving anything. + // + // A plain loop, not Array.find: find's predicate is synchronous, so an async + // one returns a Promise, every Promise is truthy, and it would always hand + // back the first URN whether or not that school exists. + let urn: number | null = null; + for (const candidate of CUTOFF_CANDIDATE_URNS) { + if ((await page.request.get(`/api/schools/${candidate}`)).ok()) { + urn = candidate; + break; + } + } + expect(urn, 'no candidate school resolves in this environment').not.toBeNull(); + + await page.goto(`/school/${urn}`); + await expect(page.locator('h1')).toBeVisible(); + + await expect(page.getByRole('heading', { name: /How far away are you\?/ })) + .toHaveCount(0); +}); + +test('/api/flags is not reachable from the public internet', async ({ page }) => { + // It names every unreleased feature and whether it is on. Next reads it + // server-side over the Docker network; the public proxy must deny it. + const res = await page.request.get('/api/flags'); + expect(res.status()).toBe(404); +}); +``` + +- [ ] **Step 2: Verify the spec compiles and the tests are collected** + +Run: `cd e2e && npx playwright test --list` + +Expected: the three new titles appear, total count rises by 3. + +- [ ] **Step 3: Commit** + +```bash +git add e2e/tests/journeys.spec.ts +git commit -m "test(e2e): make the distance journeys fail loudly, not skip quietly" +``` + +--- + +### Task 7: Wire the environment variables + +**Files:** +- Modify: `docker-compose.yml` +- Modify: `docker-compose.portainer.yml` +- Modify: `docker-compose.portainer.staging.yml` + +**Interfaces:** +- Consumes: the tokens from Task 1. +- Produces: a backend that can reach Unleash in each environment. + +- [ ] **Step 1: Add the variables to all three stacks** + +In each file, in the **backend** service's `environment` block, after the +`TYPESENSE_API_KEY` line, add: + +```yaml + UNLEASH_URL: ${UNLEASH_URL:-} + UNLEASH_API_TOKEN: ${UNLEASH_API_TOKEN:-} +``` + +Unset by default on purpose: an environment without them has every flag off, +which is the correct dark state rather than a failure. + +- [ ] **Step 2: Add the cache volume to the two Portainer stacks** + +The SDK's disk cache must survive a container restart, or a restart during an +Unleash outage reverts a released feature to dark. + +In `docker-compose.portainer.yml` and `docker-compose.portainer.staging.yml`, +add to the backend service: + +```yaml + volumes: + - unleash_cache:/app/.unleash +``` + +(If the backend service already has a `volumes:` block, add the one line to it +rather than a second block.) + +And to each file's top-level `volumes:` block: + +```yaml + unleash_cache: +``` + +- [ ] **Step 3: Document the Portainer variables** + +In `docker-compose.portainer.staging.yml`, add to the header comment list: + +``` +# UNLEASH_URL — http://:4242/api (empty = all flags off) +# UNLEASH_API_TOKEN — Unleash *client* token, environment: development +``` + +And the same in `docker-compose.portainer.yml` with `environment: production`. + +- [ ] **Step 4: Commit** + +```bash +git add docker-compose.yml docker-compose.portainer.yml docker-compose.portainer.staging.yml +git commit -m "chore(flags): wire UNLEASH_URL and the SDK cache volume into the stacks" +``` + +--- + +### Task 8: Open the PR + +- [ ] **Step 1: Run everything** + +```bash +uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q +cd nextjs-app && npx jest && npx tsc --noEmit && npm run build +cd ../e2e && npx playwright test --list +``` + +Expected: backend green, frontend green, typecheck clean, build green, e2e collects. + +- [ ] **Step 2: Open the PR against `main`** + +Body must state: the flag defaults off, so merging changes nothing visible in +any environment until someone toggles it in Unleash; and that Task 1's manual +Portainer steps are a prerequisite for the flag to be togglable at all. + +## Notes for the executor + +**Nothing here needs a running Unleash.** Every test passes with `UNLEASH_URL` +unset, which is also what CI gets. Task 1's manual steps only decide whether +the flag can be *turned on*, not whether the code works. + +**The staleness test will fail on 2026-11-21** if `admission_distance` is still +declared. That is intended — see the docstring. It fails on whatever PR is open +at the time, which is the mechanism, not a bug in it. + +**Do not add a webhook.** An earlier draft of the spec had one; §4 records why +it was removed and why time-based revalidation is enough. -- 2.54.0 From 01ccbb8e827d5cd5609d8f817f28054c812a97c1 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:51:35 +0100 Subject: [PATCH 04/10] feat(flags): add the Unleash stack and its runbook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Its own Portainer stack, belonging to neither application stack: a staging redeploy must not be able to disturb production's flag state. One instance serves both. OSS Unleash ships development and production environments with environment-scoped client tokens, so the same flag holds independent state in each — which is what lets a feature be on in staging, where the E2E journeys exercise it, while production stays dark. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- docker-compose.portainer.unleash.yml | 73 ++++++++++++++++++++++++++++ docs/DEPLOY.md | 51 +++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 docker-compose.portainer.unleash.yml diff --git a/docker-compose.portainer.unleash.yml b/docker-compose.portainer.unleash.yml new file mode 100644 index 0000000..ec20643 --- /dev/null +++ b/docker-compose.portainer.unleash.yml @@ -0,0 +1,73 @@ +# Portainer Stack Definition for School Compare — UNLEASH (feature flags) +# +# Deploy as a *separate* Portainer stack ("schoolcompare-unleash"), alongside +# the production and staging stacks. It deliberately belongs to neither: a +# staging redeploy must not be able to disturb production's flag state, and a +# production redeploy must not disturb staging's. +# +# One instance serves both environments. Open-source Unleash ships with +# `development` and `production` environments and environment-scoped client +# tokens, so the same flag holds independent state in each — which is what +# lets a feature be on in staging, where the E2E journeys exercise it, while +# production stays dark. +# +# Portainer environment variables (set in Portainer UI -> Stack -> Environment): +# UNLEASH_DB_PASSWORD — PostgreSQL password for the Unleash database +# UNLEASH_ADMIN_PASSWORD — initial admin password for the Unleash UI +# UNLEASH_IP — macvlan IP for the Unleash server (default 10.0.1.152) + +services: + + # ── PostgreSQL (Unleash's own; nothing else uses it) ────────────────── + unleash_db: + container_name: sc_unleash_postgres + image: postgres:16-alpine + environment: + POSTGRES_USER: unleash + POSTGRES_PASSWORD: ${UNLEASH_DB_PASSWORD} + POSTGRES_DB: unleash + volumes: + - unleash_postgres_data:/var/lib/postgresql/data + networks: + - unleash + healthcheck: + test: ["CMD-SHELL", "pg_isready -U unleash"] + interval: 10s + timeout: 5s + retries: 5 + start_period: 10s + restart: unless-stopped + + # ── Unleash server (UI + client API on 4242) ────────────────────────── + unleash: + container_name: sc_unleash + image: unleashorg/unleash-server:6 + environment: + DATABASE_URL: postgres://unleash:${UNLEASH_DB_PASSWORD}@unleash_db:5432/unleash + DATABASE_SSL: "false" + INIT_ADMIN_API_TOKENS: "" + UNLEASH_DEFAULT_ADMIN_PASSWORD: ${UNLEASH_ADMIN_PASSWORD} + depends_on: + unleash_db: + condition: service_healthy + networks: + unleash: {} + macvlan: + ipv4_address: ${UNLEASH_IP:-10.0.1.152} + healthcheck: + test: ["CMD-SHELL", "wget -qO- http://localhost:4242/health || exit 1"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 30s + restart: unless-stopped + +networks: + unleash: + driver: bridge + macvlan: + external: + name: macvlan + +volumes: + unleash_postgres_data: diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index 02123f6..14b6f83 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -150,3 +150,54 @@ token Gitea Actions provides automatically (`secrets.GITEA_TOKEN` — no setup needed), and fails the check only when a finding is rated **severe** (would break prod, leak data, or corrupt data). Minor findings are informational and never block a merge. + +## Feature flags (Unleash) + +Flag state lives in a self-hosted Unleash instance, deployed as its own +Portainer stack from `docker-compose.portainer.unleash.yml`. It is separate +from the application stacks on purpose — redeploying staging must not be able +to disturb production's flags. + +The flags themselves are declared in `backend/flags.py`. Unleash holds the +state; the registry holds the list. A flag in the UI that is not in the +registry is orphaned and nothing reads it. + +### First-time setup + +1. Deploy the stack in Portainer. Set `UNLEASH_DB_PASSWORD`, + `UNLEASH_ADMIN_PASSWORD` and (optionally) `UNLEASH_IP`. +2. Log in to the UI at `http://:4242` as `admin`. +3. Create one **client** API token per environment: + - `schoolcompare-staging`, environment **development** + - `schoolcompare-prod`, environment **production** + + Client tokens, not admin tokens — the backend only reads. +4. Put each token in the matching Portainer stack's `UNLEASH_API_TOKEN` + variable, and set `UNLEASH_URL` to `http://:4242/api`. +5. Redeploy the application stacks. + +### Turning a feature on + +Toggle the flag in the environment you want. Flags appear in the Unleash UI +after the backend has evaluated them once, so a newly declared flag shows up +shortly after the deploy that introduced it. + +A flip reaches school pages within about five minutes and place pages within +the hour. Next's ISR does the propagating — it revalidates a route at the +*lowest* `revalidate` among that route's fetches, which is 300s for +`/school/[slug]` and 3600s for the place pages. There is no webhook, and +adding one would only be worth it if flips ever needed to be instant. + +### When Unleash is unreachable + +Every flag evaluates to `False` and the site serves as though nothing were +switched on. That is deliberate — an unfinished feature staying hidden is the +safe direction — but it means a *released* feature disappears if a backend +container cold-starts with an empty cache while Unleash is down. The SDK's +disk cache is on a named volume so restarts keep last-known state, and flags +are removed from the code within 90 days (enforced by a test), which bounds +how long any feature is exposed to this. + +If `UNLEASH_URL` is unset, every flag is `False` and no connection is +attempted. That is the correct behaviour for local development and CI, and it +means the test suites need no flag server. -- 2.54.0 From 7424cef7c66743165873093ef85ba3d4d1c43b62 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:52:55 +0100 Subject: [PATCH 05/10] feat(flags): the registry and a fail-closed Unleash client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unleash holds flag state; it does not hold the list of flags. REGISTRY is that list, because the SDK evaluates an unknown flag to False and without a registry that is an undeclared False — indistinguishable from a typo in a flag name. Fail-closed throughout, and never raises: an unset UNLEASH_URL, an unreachable server, a client that throws, an undeclared name — all False. A flag layer that can 500 a request path or stop the API booting is worse than one that is switched off. Every flag defaults to False, with no per-flag override, because a flag that defaults on is a kill switch and this is deliberately not one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- backend/config.py | 10 ++++ backend/flags.py | 111 ++++++++++++++++++++++++++++++++++++ backend/tests/test_flags.py | 74 ++++++++++++++++++++++++ requirements.txt | 2 +- 4 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 backend/flags.py create mode 100644 backend/tests/test_flags.py diff --git a/backend/config.py b/backend/config.py index bdaa9dc..554efc9 100644 --- a/backend/config.py +++ b/backend/config.py @@ -42,6 +42,16 @@ class Settings(BaseSettings): typesense_url: str = "http://localhost:8108" typesense_api_key: str = "" + # Feature flags (Unleash). An empty unleash_url disables flags entirely and + # every flag evaluates False — the correct behaviour for local development + # and CI, and the reason no test needs a running Unleash. + unleash_url: str = "" + unleash_api_token: str = "" + unleash_app_name: str = "schoolcompare-backend" + # On a named volume, so a restart during an Unleash outage keeps + # last-known state instead of reverting a released feature to dark. + unleash_cache_directory: str = "/app/.unleash" + # Analytics ga_measurement_id: Optional[str] = "G-J0PCVT14NY" # Google Analytics 4 Measurement ID diff --git a/backend/flags.py b/backend/flags.py new file mode 100644 index 0000000..f843828 --- /dev/null +++ b/backend/flags.py @@ -0,0 +1,111 @@ +"""Feature flags: what can be switched, and what is switched right now. + +Ship-dark, not a kill switch. Flags let work merge and deploy without becoming +visible; they are expected to flip about monthly, by a person, deliberately. +Nothing here does percentage rollouts or user targeting — the site has no user +identity to target. + +Unleash holds the state. It does not hold the list. REGISTRY below is that +list, and it exists for three reasons: the SDK evaluates an unknown flag to +False, so without a registry that is an *undeclared* False, indistinguishable +from a typo; /api/flags needs a key set to return when Unleash is unreachable; +and a flag in the UI but not in the registry is orphaned and should be visibly +so rather than quietly authoritative. + +Every flag defaults to False. There is no per-flag default, because a flag that +defaults on is a kill switch, and this is not one. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from datetime import date + +from .config import settings + +logger = logging.getLogger(__name__) + +# A flag is temporary scaffolding. See test_a_flag_older_than_the_limit. +MAX_FLAG_AGE_DAYS = 90 + + +@dataclass(frozen=True) +class Flag: + # One string: the registry key, the Unleash flag name, and the JSON key in + # /api/flags. snake_case, matching the API's existing convention. No case + # transformation anywhere, so there is no mapping layer to get wrong. + name: str + description: str # one line: what turning this on reveals + added: date # for the staleness tripwire + + +REGISTRY: dict[str, Flag] = { + f.name: f for f in ( + Flag( + name="admission_distance", + description=( + "The last-distance-offered figure on the Admissions tile and " + "the 'How far away are you?' section on school pages." + ), + added=date(2026, 8, 23), + ), + ) +} + + +_client = None + + +def init() -> None: + """Start the Unleash client, or log why flags are all off. + + Called once from the app lifespan. Never raises: a flag system that can + stop the API from booting is worse than one that is switched off. + """ + global _client + if not settings.unleash_url or not settings.unleash_api_token: + logger.warning( + "Unleash is not configured (UNLEASH_URL / UNLEASH_API_TOKEN); " + "every feature flag evaluates to False.") + return + + try: + from UnleashClient import UnleashClient + + _client = UnleashClient( + url=settings.unleash_url, + app_name=settings.unleash_app_name, + custom_headers={"Authorization": settings.unleash_api_token}, + cache_directory=settings.unleash_cache_directory, + refresh_interval=15, + ) + _client.initialize_client() + logger.info("Unleash client initialised against %s", settings.unleash_url) + except Exception: + # Fail closed and keep serving. The SDK also evaluates everything False + # until its first successful sync, so this is the same direction. + _client = None + logger.exception("Unleash client failed to start; flags are all False.") + + +def is_enabled(name: str) -> bool: + """Whether `name` is on. False for anything unknown, unreachable or broken.""" + if name not in REGISTRY: + logger.error( + "undeclared feature flag %r was evaluated; returning False. " + "Add it to backend/flags.py REGISTRY or fix the name.", name) + return False + if _client is None: + return False + try: + return bool(_client.is_enabled( + name, fallback_function=lambda feature_name, context: False)) + except Exception: + logger.exception("flag %r failed to evaluate; returning False", name) + return False + + +def all_flags() -> dict[str, bool]: + """Every declared flag and its current value. Serves /api/flags.""" + return {name: is_enabled(name) for name in REGISTRY} diff --git a/backend/tests/test_flags.py b/backend/tests/test_flags.py new file mode 100644 index 0000000..5aa0147 --- /dev/null +++ b/backend/tests/test_flags.py @@ -0,0 +1,74 @@ +"""Tests for the feature flag layer (spec 2026-08-23). + +None of these need a running Unleash. That is the point: an unset UNLEASH_URL +means every flag is False, which is what local development and CI get. +""" + +from datetime import date, timedelta + +from backend import flags + + +def test_every_declared_flag_is_keyed_by_its_own_name(): + # One string is the registry key, the Unleash flag name and the JSON key. + # A mismatch here would mean the UI toggles a flag the code never reads. + for key, flag in flags.REGISTRY.items(): + assert key == flag.name + + +def test_flag_names_are_snake_case(): + # Matches the API's existing convention (admission_distance, + # rwm_expected_pct) so no case transformation exists to get wrong. + for name in flags.REGISTRY: + assert name == name.lower() + assert "-" not in name and " " not in name + + +def test_an_unconfigured_client_evaluates_every_flag_false(monkeypatch): + monkeypatch.setattr(flags, "_client", None) + for name in flags.REGISTRY: + assert flags.is_enabled(name) is False + + +def test_an_undeclared_flag_is_false_rather_than_an_error(monkeypatch): + # A typo'd flag name must not raise in a request path. It is logged as an + # error, because an undeclared flag is always a bug. + monkeypatch.setattr(flags, "_client", None) + assert flags.is_enabled("no_such_flag") is False + + +def test_an_exploding_client_is_false_rather_than_a_500(monkeypatch): + class Boom: + def is_enabled(self, *a, **kw): + raise RuntimeError("unleash is on fire") + + monkeypatch.setattr(flags, "_client", Boom()) + name = next(iter(flags.REGISTRY)) + assert flags.is_enabled(name) is False + + +def test_all_flags_reports_every_declared_flag(monkeypatch): + monkeypatch.setattr(flags, "_client", None) + assert set(flags.all_flags()) == set(flags.REGISTRY) + assert all(v is False for v in flags.all_flags().values()) + + +def test_a_flag_older_than_the_limit_fails_this_test(): + """A tripwire, not an assertion about correctness. + + Flags are temporary scaffolding and the failure mode of every flag system + is accumulation. This fails on the day a flag turns 90, on whatever PR + happens to be open — which is the point: someone has to decide. + + To fix: delete the flag and the branches that read it, or, if it genuinely + still needs to exist, move its `added` date and say why in the commit. + """ + stale = [ + f.name for f in flags.REGISTRY.values() + if date.today() - f.added > timedelta(days=flags.MAX_FLAG_AGE_DAYS) + ] + assert not stale, ( + f"Flags older than {flags.MAX_FLAG_AGE_DAYS} days: {stale}. " + "Remove the flag and the code branches it guards, or move its `added` " + "date deliberately." + ) diff --git a/requirements.txt b/requirements.txt index f5ab98a..3b82b3f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,4 +12,4 @@ slowapi==0.1.9 secure==0.3.0 typesense==0.21.0 numpy==1.26.4 - +UnleashClient==6.0.1 -- 2.54.0 From c30ad1db0782ca54de0022521b1ea33591e1b4fe Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:54:26 +0100 Subject: [PATCH 06/10] feat(flags): serve /api/flags, and keep the public proxy off it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The endpoint and its exposure control ship together on purpose. The moment /api/flags exists, app/api/[...path] forwards it — and the response names every unreleased feature the codebase knows about, along with whether it is on. Publishing that is the opposite of shipping dark. Denied on an exact first-segment match, not a prefix, so /api/flagship does not go down with /api/flags. Next reads the endpoint server-side over the Docker network, which never transits the public proxy. jest.setup.js now guards its browser globals. It runs for every suite, including the one that declares @jest-environment node to exercise the route handler — NextRequest needs Fetch API globals jsdom lacks, and there is no window there to define matchMedia on. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- backend/app.py | 14 +++++++++ backend/tests/test_flags.py | 21 +++++++++++++ .../__tests__/api/proxyDenylist.test.ts | 31 +++++++++++++++++++ nextjs-app/app/api/[...path]/route.ts | 19 ++++++++++++ nextjs-app/jest.setup.js | 8 +++++ 5 files changed, 93 insertions(+) create mode 100644 nextjs-app/__tests__/api/proxyDenylist.test.ts diff --git a/backend/app.py b/backend/app.py index 2284ac8..4828a8b 100644 --- a/backend/app.py +++ b/backend/app.py @@ -35,6 +35,7 @@ from .data_loader import ( search_schools_typesense, ) from .data_loader import get_data_info as get_db_info +from . import flags from .places import build_place_registry from .schemas import METRIC_DEFINITIONS, RANKING_COLUMNS, SCHOOL_COLUMNS from .utils import clean_for_json, convert_to_native @@ -459,6 +460,7 @@ def validate_postcode(postcode: Optional[str]) -> Optional[str]: async def lifespan(app: FastAPI): """Application lifespan - startup and shutdown events.""" global _sitemaps + flags.init() print("Loading school data from marts...") df = load_school_data() if df.empty: @@ -1263,6 +1265,18 @@ async def get_place(request: Request, kind: str, slug: str, } +@app.get("/api/flags") +@limiter.limit(f"{settings.rate_limit_per_minute}/minute") +async def get_feature_flags(request: Request): + """Every declared flag and its current value. + + Internal only. The Next proxy denies this path, because the response names + every unreleased feature the codebase knows about — which is exactly what + shipping dark is meant to keep quiet. + """ + return flags.all_flags() + + @app.get("/api/data-info") @limiter.limit(f"{settings.rate_limit_per_minute}/minute") async def get_data_info(request: Request): diff --git a/backend/tests/test_flags.py b/backend/tests/test_flags.py index 5aa0147..114a219 100644 --- a/backend/tests/test_flags.py +++ b/backend/tests/test_flags.py @@ -72,3 +72,24 @@ def test_a_flag_older_than_the_limit_fails_this_test(): "Remove the flag and the code branches it guards, or move its `added` " "date deliberately." ) + + +def _client(): + from fastapi.testclient import TestClient + from backend import app as app_module + return TestClient(app_module.app, raise_server_exceptions=False) + + +def test_the_flags_endpoint_lists_every_declared_flag(monkeypatch): + monkeypatch.setattr(flags, "_client", None) + body = _client().get("/api/flags").json() + assert set(body) == set(flags.REGISTRY) + + +def test_the_flags_endpoint_answers_false_when_unleash_is_unreachable(monkeypatch): + # The endpoint must still answer. A frontend that cannot read flags renders + # everything dark, which is right; one that gets a 500 renders nothing. + monkeypatch.setattr(flags, "_client", None) + res = _client().get("/api/flags") + assert res.status_code == 200 + assert all(v is False for v in res.json().values()) diff --git a/nextjs-app/__tests__/api/proxyDenylist.test.ts b/nextjs-app/__tests__/api/proxyDenylist.test.ts new file mode 100644 index 0000000..10731f2 --- /dev/null +++ b/nextjs-app/__tests__/api/proxyDenylist.test.ts @@ -0,0 +1,31 @@ +/** + * The /api/* proxy is public. Anything it forwards is on the internet. + * + * @jest-environment node + */ +// The docblock above is load-bearing. jest.config.js sets jsdom globally, and +// NextRequest/NextResponse need the Web Fetch API globals that only the node +// environment provides — under jsdom this suite fails on import, not on an +// assertion. +import { NextRequest } from 'next/server'; +import { GET } from '@/app/api/[...path]/route'; + +function request(path: string) { + return new NextRequest(`http://localhost:3000/api/${path}`); +} + +describe('public API proxy', () => { + it('refuses to forward internal-only paths', async () => { + // /api/flags names every unreleased feature and its state. Forwarding it + // publishes the thing shipping dark exists to keep quiet. + const res = await GET(request('flags'), { params: Promise.resolve({ path: ['flags'] }) }); + expect(res.status).toBe(404); + }); + + it('does not deny a path that merely starts with the same letters', async () => { + // A prefix match would take /api/flagship down with /api/flags. + const res = await GET( + request('flagship'), { params: Promise.resolve({ path: ['flagship'] }) }); + expect(res.status).not.toBe(404); + }); +}); diff --git a/nextjs-app/app/api/[...path]/route.ts b/nextjs-app/app/api/[...path]/route.ts index ba7cc85..5f2d368 100644 --- a/nextjs-app/app/api/[...path]/route.ts +++ b/nextjs-app/app/api/[...path]/route.ts @@ -26,8 +26,27 @@ function backendBase(): string { const STRIPPED_RESPONSE_HEADERS = ['content-encoding', 'content-length', 'transfer-encoding', 'connection']; const METHODS_WITH_BODY = new Set(['POST', 'PUT', 'PATCH', 'DELETE']); +/* + * API paths this public proxy must not forward. + * + * Matched on the first segment, exactly — a prefix match would take + * /api/flagship down with /api/flags. + * + * `flags` is here because GET /api/flags names every unreleased feature the + * codebase knows about, along with whether it is on. Publishing that defeats + * the point of shipping dark. Next reads it server-side via FASTAPI_URL, on + * the Docker network, which never transits this route. + * + * Anything else internal-only belongs here too. + */ +const INTERNAL_ONLY_SEGMENTS = new Set(['flags']); + async function handler(req: NextRequest, ctx: { params: Promise<{ path: string[] }> }) { const { path } = await ctx.params; + if (INTERNAL_ONLY_SEGMENTS.has(path[0])) { + return NextResponse.json({ detail: 'Not Found' }, { status: 404 }); + } + const target = `${backendBase()}/${path.join('/')}${req.nextUrl.search}`; const headers = new Headers(req.headers); diff --git a/nextjs-app/jest.setup.js b/nextjs-app/jest.setup.js index 256811b..804aa70 100644 --- a/nextjs-app/jest.setup.js +++ b/nextjs-app/jest.setup.js @@ -12,6 +12,12 @@ jest.mock('next/navigation', () => ({ useSearchParams: () => new URLSearchParams(), })); +// Everything below this line is browser furniture, and this file runs for +// every suite — including the ones that declare `@jest-environment node` to +// test route handlers, where NextRequest needs Fetch API globals jsdom does +// not provide. There is no `window` there, so guard rather than assume one. +if (typeof window !== 'undefined') { + // Mock window.matchMedia Object.defineProperty(window, 'matchMedia', { writable: true, @@ -52,3 +58,5 @@ const localStorageMock = { clear: jest.fn(), }; global.localStorage = localStorageMock; + +} // end: browser-only globals -- 2.54.0 From 54a30de0d837390e5ec94ea4562390c0ade0ce1a Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:55:32 +0100 Subject: [PATCH 07/10] feat(flags): server-side getFlags for the frontend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships without a consumer, deliberately. The first flag needs none — the backend withholds the field and the page follows — but 'UI elements on existing pages' is one of the three surfaces this capability exists for, and a flag layer that cannot gate one is incomplete. Never throws: an unreadable flag is a dark one, which matches the backend's fail-closed default. A page that 500s because the flags endpoint blinked would be a worse outcome than a hidden feature. Reading flags pins the calling route to a 300s ISR floor, since Next takes the lowest revalidate among a route's fetches. That matches what /school/[slug] already sits at, and it is the same property that makes a flip propagate without a webhook. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- nextjs-app/__tests__/lib/flags.test.ts | 34 ++++++++++++++++++++++ nextjs-app/lib/flags.ts | 40 ++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 nextjs-app/__tests__/lib/flags.test.ts create mode 100644 nextjs-app/lib/flags.ts diff --git a/nextjs-app/__tests__/lib/flags.test.ts b/nextjs-app/__tests__/lib/flags.test.ts new file mode 100644 index 0000000..ddadc7e --- /dev/null +++ b/nextjs-app/__tests__/lib/flags.test.ts @@ -0,0 +1,34 @@ +import { getFlags } from '@/lib/flags'; + +// jsdom provides no global fetch, so there is nothing for jest.spyOn to attach +// to — assign it and restore the original afterwards. This is the first test +// here to mock fetch; later ones should follow this shape. +const realFetch = global.fetch; + +function mockFetch(impl: () => Promise) { + global.fetch = jest.fn(impl) as unknown as typeof fetch; +} + +describe('getFlags', () => { + afterEach(() => { global.fetch = realFetch; }); + + it('returns the flags the API reports', async () => { + mockFetch(async () => ({ + ok: true, + json: async () => ({ admission_distance: true }), + })); + await expect(getFlags()).resolves.toEqual({ admission_distance: true }); + }); + + it('returns no flags rather than throwing when the API is down', async () => { + // A page that cannot read flags must render everything dark, not 500. + // Fail-closed is the same direction as the backend's default. + mockFetch(async () => { throw new Error('ECONNREFUSED'); }); + await expect(getFlags()).resolves.toEqual({}); + }); + + it('returns no flags rather than throwing on a non-200', async () => { + mockFetch(async () => ({ ok: false, status: 503 })); + await expect(getFlags()).resolves.toEqual({}); + }); +}); diff --git a/nextjs-app/lib/flags.ts b/nextjs-app/lib/flags.ts new file mode 100644 index 0000000..0f21e44 --- /dev/null +++ b/nextjs-app/lib/flags.ts @@ -0,0 +1,40 @@ +/** + * Reading feature flags. + * + * Server-side only. No flag value reaches the browser bundle, and there is no + * Unleash dependency in package.json — the SDK lives in FastAPI, which already + * owns every other piece of data this app renders. + * + * Flags are declared in backend/flags.py. A purely front-end flag still has to + * be declared there; it is a flat data edit, and the return is that one list + * answers "what flags exist" for the whole system. + */ + +export type Flags = Record; + +/* + * Reading flags pins the calling route to this ISR floor: Next uses the LOWEST + * revalidate among a route's fetches to set the whole route's revalidation + * frequency. 300s matches what /school/[slug] already sits at, so a page that + * reads flags is no more dynamic than a school page already is. + * + * It is also what makes a flip propagate without a webhook: five minutes on + * school pages, an hour on place pages, against flags that flip monthly. + */ +export const FLAGS_REVALIDATE = 300; + +const API = process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL + || 'http://localhost:8000/api'; + +/** Every flag and its value. Never throws: an unreadable flag is a dark one. */ +export async function getFlags(): Promise { + try { + const res = await fetch(`${API}/flags`, { + next: { revalidate: FLAGS_REVALIDATE }, + }); + if (!res.ok) return {}; + return await res.json(); + } catch { + return {}; + } +} -- 2.54.0 From c3ba7aae0df58d3139336cd64541628b8f37551d Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:56:33 +0100 Subject: [PATCH 08/10] feat(flags): ship last-distance-offered dark behind a flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One gate, at the source. The frontend needs no change: DistanceSection already returns null when distance_m is missing, and the admissions block already conditions on (admissions || admissionDistance). Only 57 local authorities publish cut-offs, so the off-path is the commonest path on the site and is well covered already. Absent, not null. /api/schools/ is public and unauthenticated, so a field left in the payload is a published field — the reasoning already recorded in c9a1892 when history was withheld. The two are also different claims: null says this school has no cut-off, absent says cut-offs are not being published at all. The frontend type now says so. The feature is on main and live on staging and has never reached production, which is what makes it the right first consumer: the flag lets the code promote without the feature appearing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- backend/app.py | 8 ++++- backend/tests/test_flags.py | 68 +++++++++++++++++++++++++++++++++++++ nextjs-app/lib/types.ts | 7 +++- 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/backend/app.py b/backend/app.py index 4828a8b..d352de5 100644 --- a/backend/app.py +++ b/backend/app.py @@ -808,7 +808,13 @@ async def get_school_details(request: Request, urn: int): "census": supplementary.get("census"), "admissions": supplementary.get("admissions"), "admissions_history": supplementary.get("admissions_history") or [], - "admission_distance": supplementary.get("admission_distance"), + # Behind a flag, and withheld at the source rather than rendered-but- + # hidden: this endpoint is public and unauthenticated, so a field left + # in the payload is a published field. The key is absent, not null — + # null would state that this school has no cut-off, which is a + # different claim from "we are not publishing cut-offs". + **({"admission_distance": supplementary.get("admission_distance")} + if flags.is_enabled("admission_distance") else {}), "sen_detail": supplementary.get("sen_detail"), "phonics": supplementary.get("phonics"), "deprivation": supplementary.get("deprivation"), diff --git a/backend/tests/test_flags.py b/backend/tests/test_flags.py index 114a219..24fdb53 100644 --- a/backend/tests/test_flags.py +++ b/backend/tests/test_flags.py @@ -93,3 +93,71 @@ def test_the_flags_endpoint_answers_false_when_unleash_is_unreachable(monkeypatc res = _client().get("/api/flags") assert res.status_code == 200 assert all(v is False for v in res.json().values()) + + +def _school_payload(monkeypatch, *, flag_on: bool): + """Fetch one school's payload with the distance flag forced on or off. + + The DataFrame shape is copied from test_school_details.py rather than + minimised: the endpoint reads a wide set of GIAS columns, and a trimmed + frame fails for reasons that have nothing to do with flags. + """ + import numpy as np + import pandas as pd + from fastapi.testclient import TestClient + from backend import app as app_module + + df = pd.DataFrame([{ + "urn": 150275, + "school_name": "West London Performing Arts Academy", + "phase": "Secondary", + "school_type": "Special post 16 institution", + "trust_name": None, + "religious_denomination": "Does not apply", + "gender": None, + "age_range": "16-25", + "admissions_policy": None, + "capacity": np.nan, + "gias_total_pupils": np.nan, + "headteacher_name": None, + "website": None, + "ofsted_grade": np.nan, + "local_authority": "Ealing", + "address": "268 Northfield Avenue, London, W5 4UB", + "postcode": "W5 4UB", + "latitude": 51.4986, + "longitude": -0.3148, + "year": np.nan, + "total_pupils": np.nan, + "eligible_pupils": np.nan, + "rwm_expected_pct": np.nan, + }]) + + monkeypatch.setattr(app_module, "load_school_data", lambda: df) + # Two arguments: get_supplementary_data(db, urn). See backend/app.py. + monkeypatch.setattr( + app_module, "get_supplementary_data", + lambda db, urn: {"admission_distance": {"distance_m": 772.49, + "year": 2024}}) + monkeypatch.setattr(flags, "is_enabled", lambda name: flag_on) + + client = TestClient(app_module.app, raise_server_exceptions=False) + res = client.get("/api/schools/150275") + assert res.status_code == 200, res.text + return res.json() + + +def test_the_distance_field_is_absent_when_the_flag_is_off(monkeypatch): + """Absent, not null, and withheld at the source. + + /api/schools/ is public and unauthenticated. Leaving a withheld field in + the payload while declining to render it hands the record to anyone who + opens the network tab — the reasoning already recorded in c9a1892. + """ + body = _school_payload(monkeypatch, flag_on=False) + assert "admission_distance" not in body + + +def test_the_distance_field_is_present_when_the_flag_is_on(monkeypatch): + body = _school_payload(monkeypatch, flag_on=True) + assert body["admission_distance"]["distance_m"] == 772.49 diff --git a/nextjs-app/lib/types.ts b/nextjs-app/lib/types.ts index 7c1f751..601fd1f 100644 --- a/nextjs-app/lib/types.ts +++ b/nextjs-app/lib/types.ts @@ -361,7 +361,12 @@ export interface SchoolDetailsResponse { * held back as a paid feature and are not part of this public payload — see * data_loader._admission_distance. */ - admission_distance: SchoolAdmissionDistance | null; + /** + * Absent — not null — when the admission_distance flag is off. Null means + * "this school has no published cut-off"; absent means "cut-offs are not + * being published at all". They are different claims and the type says so. + */ + admission_distance?: SchoolAdmissionDistance | null; deprivation: SchoolDeprivation | null; finance: SchoolFinance | null; } -- 2.54.0 From 4f01fbdedbd41c3d3bd7d6a6fac5cef76a8a70b8 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:57:34 +0100 Subject: [PATCH 09/10] test(e2e): make the distance journeys fail loudly, not skip quietly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing distance journeys all skip when no school has a published figure, which is right when the feature is off — and wrong when it is supposed to be on and is silently broken, because that shows up as a green run full of skips. The new gate fails in exactly that case. Feature state is read from the data, not from /api/flags: the public proxy denies that path on purpose, since it names unreleased features. Presence of the admission_distance key is the observable effect. Verified against staging, where the feature is currently on: the on-gate passes, the off-gate skips, the existing eight distance journeys are unaffected. One honest caveat — the /api/flags check passes on staging today because that image predates the endpoint, not because the denylist works. The denylist itself is covered by the jest unit test; this is defence in depth and becomes a real assertion once deployed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- e2e/tests/journeys.spec.ts | 74 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 7a81e60..e99174d 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -1226,6 +1226,27 @@ const CUTOFF_CANDIDATE_URNS = [ 101099, 100553, 102574, 100769, // mixed ]; +/** + * Whether the last-distance-offered feature is switched on here. + * + * Read from the data rather than from /api/flags, which the public proxy + * denies on purpose — the endpoint names unreleased features. The observable + * effect is the field's presence: the flag is off iff no candidate school + * carries an `admission_distance` key at all. + * + * The distinction that matters: `admission_distance: null` means this school + * has no published cut-off, and the key being ABSENT means cut-offs are not + * being published at all. + */ +async function distanceFeatureIsOn(page: Page): Promise { + for (const urn of CUTOFF_CANDIDATE_URNS) { + const res = await page.request.get(`/api/schools/${urn}`); + if (!res.ok()) continue; + if ('admission_distance' in (await res.json())) return true; + } + return false; +} + async function schoolWithCutoff(page: Page) { for (const urn of CUTOFF_CANDIDATE_URNS) { const res = await page.request.get(`/api/schools/${urn}`); @@ -1237,6 +1258,59 @@ async function schoolWithCutoff(page: Page) { return null; } +test('when the distance feature is on, a school with a cut-off is findable', async ({ page }) => { + /* + * The gate that stops the other distance journeys passing vacuously. + * + * They all skip when schoolWithCutoff() finds nothing, which is right when + * the feature is off — but it means a feature that is *supposed* to be on + * and is silently broken shows up as a green run full of skips. This test + * fails in that case. + */ + test.skip(!(await distanceFeatureIsOn(page)), + 'the admission_distance flag is off in this environment'); + + expect(await schoolWithCutoff(page), + 'the distance feature is on, but no candidate school has a cut-off — ' + + 'the flag is on and the data or the query behind it is broken') + .not.toBeNull(); +}); + +test('with the distance feature off, the section is absent rather than empty', async ({ page }) => { + // Shipping dark means the page renders as it did before the feature existed, + // not as a feature with its content removed. + test.skip(await distanceFeatureIsOn(page), + 'the admission_distance flag is on in this environment'); + + // A school that exists, found rather than hardcoded — a 404 page would + // satisfy the absent-heading assertion without proving anything. + // + // A plain loop, not Array.find: find's predicate is synchronous, so an async + // one returns a Promise, every Promise is truthy, and it would always hand + // back the first URN whether or not that school exists. + let urn: number | null = null; + for (const candidate of CUTOFF_CANDIDATE_URNS) { + if ((await page.request.get(`/api/schools/${candidate}`)).ok()) { + urn = candidate; + break; + } + } + expect(urn, 'no candidate school resolves in this environment').not.toBeNull(); + + await page.goto(`/school/${urn}`); + await expect(page.locator('h1')).toBeVisible(); + + await expect(page.getByRole('heading', { name: /How far away are you\?/ })) + .toHaveCount(0); +}); + +test('/api/flags is not reachable from the public internet', async ({ page }) => { + // It names every unreleased feature and whether it is on. Next reads it + // server-side over the Docker network; the public proxy must deny it. + const res = await page.request.get('/api/flags'); + expect(res.status()).toBe(404); +}); + test('a published cut-off distance is shown with the year it belongs to', async ({ page }) => { const found = await schoolWithCutoff(page); test.skip(found === null, 'no school in the sample has a published cut-off distance yet'); -- 2.54.0 From 413d86cc3cb3165a29aa5d5672af00dd48f86cb2 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:58:29 +0100 Subject: [PATCH 10/10] chore(flags): wire UNLEASH_URL and the SDK cache volume into the stacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both variables default to empty, so an environment without Unleash has every flag off — the correct dark state rather than a boot failure. The cache volume is the mitigation for the one real regression risk in this design: the SDK evaluates everything False until it syncs, so a backend cold-starting with an empty cache while Unleash is unreachable would make a *released* feature disappear. On a named volume the disk cache survives a restart. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- docker-compose.portainer.staging.yml | 9 +++++++++ docker-compose.portainer.yml | 9 +++++++++ docker-compose.yml | 4 ++++ 3 files changed, 22 insertions(+) diff --git a/docker-compose.portainer.staging.yml b/docker-compose.portainer.staging.yml index d0cff2d..3bbad6b 100644 --- a/docker-compose.portainer.staging.yml +++ b/docker-compose.portainer.staging.yml @@ -16,6 +16,8 @@ # ADMIN_API_KEY — Backend admin API key # TYPESENSE_API_KEY — Typesense admin API key # TYPESENSE_SEARCH_KEY — Typesense search-only key (exposed to frontend) +# UNLEASH_URL — http://:4242/api (empty = all flags off) +# UNLEASH_API_TOKEN — Unleash *client* token, environment: development # AIRFLOW_ADMIN_USER — Airflow admin username (password auto-generated, see api-server logs) # STAGING_DB_IP — macvlan IP for staging Postgres (default 10.0.1.190) # STAGING_FRONTEND_IP — macvlan IP for staging frontend (default 10.0.1.151) @@ -55,6 +57,12 @@ services: ADMIN_API_KEY: ${ADMIN_API_KEY:-changeme} TYPESENSE_URL: http://typesense:8108 TYPESENSE_API_KEY: ${TYPESENSE_API_KEY:-changeme} + # Unset means every feature flag is False — the correct dark state for an + # environment with no Unleash, not a failure. + UNLEASH_URL: ${UNLEASH_URL:-} + UNLEASH_API_TOKEN: ${UNLEASH_API_TOKEN:-} + volumes: + - unleash_cache:/app/.unleash depends_on: sc_database: condition: service_healthy @@ -212,3 +220,4 @@ volumes: postgres_data: typesense_data: airflow_logs: + unleash_cache: diff --git a/docker-compose.portainer.yml b/docker-compose.portainer.yml index a593d80..e44a9b6 100644 --- a/docker-compose.portainer.yml +++ b/docker-compose.portainer.yml @@ -7,6 +7,8 @@ # ADMIN_API_KEY — Backend admin API key # TYPESENSE_API_KEY — Typesense admin API key # TYPESENSE_SEARCH_KEY — Typesense search-only key (exposed to frontend) +# UNLEASH_URL — http://:4242/api (empty = all flags off) +# UNLEASH_API_TOKEN — Unleash *client* token, environment: production # AIRFLOW_ADMIN_USER — Airflow admin username (password auto-generated, see api-server logs) services: @@ -44,6 +46,12 @@ services: ADMIN_API_KEY: ${ADMIN_API_KEY:-changeme} TYPESENSE_URL: http://typesense:8108 TYPESENSE_API_KEY: ${TYPESENSE_API_KEY:-changeme} + # Unset means every feature flag is False — the correct dark state for an + # environment with no Unleash, not a failure. + UNLEASH_URL: ${UNLEASH_URL:-} + UNLEASH_API_TOKEN: ${UNLEASH_API_TOKEN:-} + volumes: + - unleash_cache:/app/.unleash depends_on: sc_database: condition: service_healthy @@ -201,3 +209,4 @@ volumes: postgres_data: typesense_data: airflow_logs: + unleash_cache: diff --git a/docker-compose.yml b/docker-compose.yml index f72bb7a..b3fdd41 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -36,6 +36,10 @@ services: ADMIN_API_KEY: ${ADMIN_API_KEY:-changeme} TYPESENSE_URL: http://typesense:8108 TYPESENSE_API_KEY: ${TYPESENSE_API_KEY:-changeme} + # Unset means every feature flag is False — the correct dark state for an + # environment with no Unleash, not a failure. + UNLEASH_URL: ${UNLEASH_URL:-} + UNLEASH_API_TOKEN: ${UNLEASH_API_TOKEN:-} volumes: - ./data:/app/data:ro depends_on: -- 2.54.0