diff --git a/.claude/skills/beacon-collection/SKILL.md b/.claude/skills/beacon-collection/SKILL.md index cec6a34..bcc8c14 100644 --- a/.claude/skills/beacon-collection/SKILL.md +++ b/.claude/skills/beacon-collection/SKILL.md @@ -1,12 +1,21 @@ --- name: beacon-collection -description: How Beacon's collector reaches upstreams and what it does with the results — per-source proxy opt-in and why direct is the default, batched sources sharing one fetch (the 20 Yahoo rows), the Open-Meteo weather provider with its retry policy and alert edge semantics, the 16-day long-range forecast on its own daily gate and the content-gated outlook digest, and the source-health alerting that reports a source gone silent. Load before touching cmd/collector, internal/tools/rateextractor, internal/application/collection, internal/infrastructure/weather, notification.SourceHealthAgent, any rate_sources row or seed migration, anything involving BEACON_PROXY_URL or options.use_proxy, weather alert kinds, the rain/thaw/heat/frost latches, collection.WeatherForecastAgent, OpenMeteo.Forecast or ForecastRange, or the forecast_outlook notify kind and its notify_state signature. +description: Beacon's collection pipeline — source kinds (BID/ASK/LAST) and the options JSON column, per-source proxy opt-in (direct by default; the Telegram bot always bypasses), batched sources sharing one fetch, the Open-Meteo client and its retry policy, the 16-day forecast and its daily gate, the content-gated outlook digest, weather alert latches, source-health alerting, and the cmd/doctor operator umbrella. Load before touching cmd/collector, cmd/doctor, internal/tools/rateextractor or proxyutil, internal/application/collection, internal/infrastructure/weather or telegrambot, any rate_sources row or seed migration, domain.RateSourceOptions, WeatherForecastAgent, BEACON_PROXY_URL or options.use_proxy (HTTPS_PROXY/HTTP_PROXY/NO_PROXY do nothing here), BEACON_CHROMIUM_PATH or fetcher_kind='chromedp' sources, the rain/thaw/heat/frost alert kinds, or the forecast_outlook notify kind. --- # Beacon collection Everything the collector fetches, and what it does with the answer. +## Source rows: `kind` and `options` + +A `rate_sources` row carries a `kind` of `BID`, `ASK`, or `LAST` (equity / last-traded +price). Per-source fetch behaviour — header overrides, the proxy opt-in — lives in the +`options` JSON column, typed as `domain.RateSourceOptions`. + +Several sources may share one URL and therefore one fetch; that batching is load-bearing and +easy to break — see "Batched sources share one fetch" below. + ## Egress: direct by default, per-source opt-in `cmd/collector` reaches every upstream directly unless a source row opts out. Two levels @@ -34,6 +43,16 @@ worth not reversing casually: `cmd/doctor` still honours the proxy unconditionally: it talks to AI providers, which is a different question with different exposure. +**Telegram Bot API traffic bypasses any proxy unconditionally**, and does so in code rather +than by configuration: a hardcoded `Proxy: nil` transport in +`internal/infrastructure/telegrambot/tbotclient.go`. No env var can route the bot through a +proxy, which is deliberate — the bot is the channel that reports collection failures, so it +must not share a failure mode with the thing it reports on. + +The standard `HTTPS_PROXY`, `HTTP_PROXY` and `NO_PROXY` variables are consulted by **no** +component in this project; `BEACON_PROXY_URL`, resolved through `proxyutil.ResolveURL`, is the +only knob. Setting the standard ones changes nothing and looks like it should. + Two things stay direct regardless of the flag. **Chromedp** takes its proxy as a browser-launch argument on one Chromium subprocess shared by the whole tick, so it cannot vary per source — `NewRateAgent` therefore passes it an empty proxy URL on purpose, or a @@ -260,3 +279,18 @@ stopped anyone hearing about it. 180 days. - Weather locations are **not** covered: they write no `execution_history`, so there is no persisted per-location outcome to measure a gap against. + +## Operator tooling: `cmd/doctor` + +`cmd/doctor` is the operator-only umbrella for LLM rule (re)generation and source auditing: +`rulegen` for a single source or `--all`, and `audit --all` / `audit --source `. No +service binary calls it — it runs by hand or from cron, which is why it may take liberties +(the unconditional proxy above) that the collector may not. + +Usage, exit codes, the AI DSN formats and the chromedp/Chromium setup live in +`cmd/doctor/README.md` and the package godoc; `make audit`, `make doctor-help` and +`make audit-help` wrap the common invocations. + +One cross-cutting consequence: `rulegen` persists through `RetainRateSource`, which rewrites +source rows **wholesale**. Anything runtime-valued that has been added to `rate_sources` is +destroyed by an unrelated `rulegen` run — see `beacon-storage`. diff --git a/.claude/skills/beacon-http-api/SKILL.md b/.claude/skills/beacon-http-api/SKILL.md index b5af184..f0eaa20 100644 --- a/.claude/skills/beacon-http-api/SKILL.md +++ b/.claude/skills/beacon-http-api/SKILL.md @@ -1,6 +1,6 @@ --- name: beacon-http-api -description: Beacon's HTTP surface and browser client — endpoint contracts that are not obvious from the router code (chart period whitelist, weather city create validation, the forced alert rows and their 409, the multi-week days array on /weather/current, liveness vs readiness), the content-hashed WASM asset URLs and the nginx location ordering they depend on, and the Mini App's 2x2 screen navigation. Load before adding or changing anything under internal/gateway, cmd/web, cmd/wasm, cmd/web/static, configs/nginx.*, dto.WeatherCurrentItem or WeatherForecastDayItem, the forecast_outlook subscription kind, or any /api/v1/me or /api/v1/public route. +description: Beacon's HTTP surface and browser client — endpoint contracts not obvious from the router code (chart period whitelist, the forced alert rows and their 409, the multi-week days array on /weather/current), Telegram WebApp initData HMAC auth and its single middleware mount, where cmd/web binds and the --api-dsn flag, the PublicError contract and the three assertions every controller error test owes, content-hashed WASM asset URLs and nginx location ordering, and the Mini App's 2x2 navigation. Load before changing anything under internal/gateway, cmd/web, cmd/wasm, cmd/web/static or configs/nginx.*, internal/tools/tgwebapp/initdata.go, middleware.TelegramInitData or routes.MePrefix, internal.PublicError or internal/errors.go, dto.WeatherCurrentItem or WeatherForecastDayItem, or any /api/v1/me or /api/v1/public route. --- # Beacon HTTP API and Mini App @@ -13,9 +13,12 @@ are written down here. - **Auth** — the `/api/v1/me/*` family is the only authenticated surface. The signed Telegram WebApp `initData` is accepted **only** in the `X-Telegram-Init-Data` header, never via - query string (a signed payload in the URL leaks into access logs and `Referer`). The HMAC - algorithm is in CLAUDE.md's Key Patterns; implementation in - `internal/tools/tgwebapp/initdata.go`. + query string (a signed payload in the URL leaks into access logs and `Referer`). + + **The HMAC scheme.** The signature is HMAC-SHA256 under + `secret_key = HMAC_SHA256("WebAppData", botToken)` — the string literal is the *key* and + the bot token is the *message*, which is the way round that is easy to get backwards. + Implementation lives in `internal/tools/tgwebapp/initdata.go`. **How it is enforced.** `NewRouter` registers the family on a private `ServeMux` and mounts it once at `routes.MePrefix` behind `middleware.TelegramInitData`. A route is authenticated @@ -69,6 +72,28 @@ deployment that must publish the port passes `--bind 0.0.0.0` explicitly. Binding loopback is not what stops a *co-hosted* vhost reaching Beacon — that neighbour proxies over loopback too. Only the port or the neighbour's upstream settles that. +**The public HTTPS origin is a CLI flag, not an env var.** `--api-dsn` (format +`https:///`, parsed by `dsninjector.Parse`) is hardcoded in the systemd unit's +`ExecStart` line and never in `.env`, so it travels with the deployment rather than with the +secrets file. `cmd/web/main.go` reads `BEACON_SQLITEDB_DSN` and `BEACON_TELEGRAMBOT_DSN` via +`dsninjector.Unmarshal(envName)` at startup, from the systemd `EnvironmentFile`; all three +configs must be present or the binary calls `log.Fatalf`. + +## Error rendering: what an end user may see + +`internal.PublicError` (in `internal/errors.go`, alongside `TraceError`, `StackTraceError`, +`HttpCodeError`, and the `ErrNotFound` sentinel) carries messages **safe to show to end +users**. Wrap at the point the error is created — the service layer — with +`internal.NewPublicError("...")` when the failure meaningfully tells the user something; +return a plain `error` for everything else (DB down, unexpected nil, ...). The controller +catches every sub-handler error and sends `PublicError.Details()` for a public error, else a +generic fallback constant. + +Every controller test on an error branch must assert three things: (1) a response was +actually sent, so the user is not left in silence; (2) its text equals `PublicError.Details()` +for a public error; (3) its text equals the fallback constant for a plain error. The first is +the one that catches a handler returning early without writing anything at all. + ## The multi-week outlook rides on `/current` `GET /api/v1/me/weather/current` carries a `days` array per city: one entry per city-local diff --git a/.claude/skills/beacon-storage/SKILL.md b/.claude/skills/beacon-storage/SKILL.md index eab5750..ce963ef 100644 --- a/.claude/skills/beacon-storage/SKILL.md +++ b/.claude/skills/beacon-storage/SKILL.md @@ -1,6 +1,6 @@ --- name: beacon-storage -description: Beacon's SQLite storage rules beyond the basics — the hot/archive tiering of rate_values and execution_history (why one file, why reads UNION both tiers and writes touch only hot, roll-over, retention, VACUUM), the migrator contract and the immutable migration filename convention, columns that look droppable but are not, why weather_forecast_days is bounded rather than tiered, why historical migration tests must not seed through a repository, and how to read production data out of a gzipped snapshot. Load before writing or reviewing any query in internal/repository or internal/infrastructure/sqlitedb, adding or altering a migration under ./migrations, touching collection.MaintenanceAgent, sqlitedb.Migrator, Transaction/ReadOnlyTransaction, RetainRateSource, rate_source_health, weather_observations, weather_forecast_days or RetainWeatherForecastDays, writing a test against stubSQLiteDBThrough, or inspecting the production database. +description: Beacon's SQLite storage rules — connection PRAGMAs, the BEGIN IMMEDIATE write / deferred read split, the repository pattern, hot/archive tiering of rate_values and execution_history, source-deletion cascades, the migrator contract and immutable migration filenames, columns that look droppable but are not, historical migration tests, and reading production data from snapshots. Load before writing or reviewing any query in internal/repository or internal/infrastructure/sqlitedb, adding or altering a migration under ./migrations, or touching collection.MaintenanceAgent, sqlitedb.Migrator or RequireMigratedSchema, NewSQLiteClientEx, Transaction/ReadOnlyTransaction, a DSN _pragma= or _txlock setting, RetainRateSource or RemoveRateSource, rate_source_health, weather_forecast_days, or stubSQLiteDBThrough. --- # Beacon storage @@ -8,6 +8,57 @@ description: Beacon's SQLite storage rules beyond the basics — the hot/archive Read this before any repository query, any migration, or any attempt to look at production data. +## Connection, PRAGMAs and transactions + +Engine: SQLite, accessed via the pure-Go `modernc.org/sqlite` driver (no CGO). Three PRAGMAs +are applied on connection open, from two different places for two different reasons: + +- `foreign_keys=ON` and `busy_timeout=5000` are passed as `?_pragma=` query parameters on the + DSN (see `connectionOptions` in `config.go`). The `modernc.org/sqlite` driver re-applies + them in its `Open` hook on every new connection the `database/sql` pool opens, which is the + only way to keep these per-connection settings consistent across `SetMaxOpenConns(N>1)`. +- `journal_mode=WAL` is persisted in the database file header, so it is set once via `db.Exec` + inside `NewSQLiteClientEx`. + +`busy_timeout` (5 s) is the driver-level retry window for lock contention; it must stay +strictly less than the Go-level `Timeout` so the context deadline always fires *after* the +driver retry expires. + +**Writes open `BEGIN IMMEDIATE`; reads stay deferred.** The DSN also carries +`_txlock=immediate`, and the driver applies that begin mode only when +`sql.TxOptions.ReadOnly` is false — so `Transaction` takes the WAL write lock at `BEGIN` +while `ReadOnlyTransaction` keeps a plain deferred `BEGIN` and still runs concurrently with a +writer. + +That split is what makes `busy_timeout` reachable at all. A deferred transaction begins as a +reader and *promotes* at its first write, and SQLite refuses to invoke the busy handler on a +promotion — two connections both waiting to promote would deadlock — so it returns +`SQLITE_BUSY` on the spot. Collector/notifier/web contention therefore lost writes in +milliseconds while a 5 s retry window sat unused (12 rate values and 5 `execution_history` +rows in one production log). Taking the lock at `BEGIN` is not a promotion, so the wait is +real. + +Consequences for new code: + +- **`Transaction` is for paths that write.** Reads go through `ReadOnlyTransaction` or they + serialise against each other for nothing. `SQLiteClient.Rollback` — and `Ping`, and through + it the `/health/check` inspector — is read-only for that reason: a readiness probe queued + behind a collector tick would report a busy database as a dead one. +- **Two write transactions cannot be open at once** in one process; the second `BEGIN` waits + for the first. Open, write and commit inside one function, as every repository method does. + +**The repository pattern.** Each repository type owns its own SQL, migration, and query +helper functions, and runs them inside explicit transactions — `r.db.Transaction(ctx)` to +write, `r.db.ReadOnlyTransaction(ctx)` to read. Repositories are passed as interfaces into +the service and handler layers. + +### Deleting a source cascades into its history + +Foreign keys point from `rate_values`, `rate_user_subscriptions` and `rate_user_events` to +`rate_sources(name)` with `ON DELETE CASCADE`, so deleting a source destroys every dependent +row. Read the warning on `RemoveRateSource` before wiring it to any endpoint. The archive +tier is deliberately exempt — see `rate_values_archive` under the tiering rules below. + ## Hot / archive tiering The two append-only telemetry tables are each split into a bounded **hot** working set and @@ -75,6 +126,10 @@ missing or empty `__schema_migrations` table is fatal: log.Fatalf("schema not initialised: run cmd/migrator before starting the service") ``` +Schema reconciliation is therefore **deploy-time, not startup-time**: `configs/beacon.service` +deliberately carries no `ExecStartPre` migrator, and the `beacon-migrate` one-shot unit runs +as root after the release symlink flips, so the CI deploy user never writes the database. + Migration files live at `./migrations/*.sql`. Filename convention: `....sql` (e.g. `202605.001.rate_sources.table_initiate.sql`). The `` segment is a **global** diff --git a/CLAUDE.md b/CLAUDE.md index 48bdadc..fb46454 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,21 +1,18 @@ # CLAUDE.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -It is deliberately a map, not a manual. It holds what applies to every task plus the rules -whose violation is silent; the depth lives in the project skills listed below and is loaded -on demand. +A map, not a manual: what applies to every task, plus the rules whose violation is silent. +Depth lives in the project skills below, loaded on demand. ## Project skills -Invoke these by name (Skill tool) when the work touches their area. Each is the full canon -for its subject — this file only keeps the tripwire. +Invoke by name (Skill tool) when the work touches their area. Each is the full canon for +its subject; this file keeps only the tripwire. | Skill | Load before touching | |---|---| -| `beacon-collection` | `cmd/collector`, `rateextractor`, `application/collection`, `infrastructure/weather`, `SourceHealthAgent`, `rate_sources` rows, `BEACON_PROXY_URL` / `options.use_proxy`, weather alert kinds, `ForecastRange`, `forecast_outlook` | -| `beacon-storage` | any `internal/repository` query, any `./migrations/*.sql`, `MaintenanceAgent`, `sqlitedb.Migrator`, `weather_forecast_days`, reading the production database | -| `beacon-http-api` | `internal/gateway`, `cmd/web`, `cmd/wasm`, `cmd/web/static`, `configs/nginx.*`, any `/api/v1/me` or `/api/v1/public` route | +| `beacon-collection` | `cmd/collector`, `rateextractor`, `application/collection`, `infrastructure/weather`, `SourceHealthAgent`, `rate_sources` rows, a source `kind` or its `options`, `BEACON_PROXY_URL` / `options.use_proxy`, `BEACON_CHROMIUM_PATH`, `cmd/doctor`, weather alert kinds, `ForecastRange`, `forecast_outlook` | +| `beacon-storage` | any `internal/repository` query, any `./migrations/*.sql`, `MaintenanceAgent`, `sqlitedb.Migrator`, `Transaction` / `ReadOnlyTransaction`, the DSN PRAGMAs, `weather_forecast_days`, reading the production database | +| `beacon-http-api` | `internal/gateway`, `cmd/web`, `cmd/wasm`, `cmd/web/static`, `configs/nginx.*`, `initData` auth, `internal.PublicError`, any `/api/v1/me` or `/api/v1/public` route | | `beacon-forecasting` | `internal/tools/rateforecaster`, `internal/tools/rateanomaly` (load with `knowledge:forecasting`) | | `beacon-data-privacy` | any new column on a user-scoped table, anything captured from a Telegram update, any new log field | @@ -26,210 +23,119 @@ anywhere in this repo. ### Where new canon goes This file is loaded whole into every session and stays there, so its size is a tax on every -conversation regardless of what the task touches. Keep it under **20k chars**; 40k is where -Claude Code warns about performance. Route new documentation by *when the reader needs it*, -not by how important the subject feels: +conversation whatever the task touches. Keep it under **12k bytes** — this project's own +budget, tighter than the 20k default; 40k is where Claude Code warns. Route new +documentation by *when the reader needs it*, not by how important the subject feels: -- **CLAUDE.md** — what applies to every task (the binary map, layer table, key patterns, - env vars, error handling, the working agreement), plus rules whose violation is - **silent**. A tripwire keeps its place here even after its subject has moved out. +- **CLAUDE.md** — what applies to every task (the binary and layer map, startup ordering, + configuration, the gate), plus rules whose violation is **silent**. A tripwire stays + here even after its subject has moved out. - **A project skill** (`.claude/skills//SKILL.md`) — the depth for one subject area. - The `description` frontmatter *is* the load trigger: name the packages, paths, symbols - and env vars that should pull it in. A description that summarises the prose instead of - naming triggers means the skill never loads and the knowledge is lost. -- **Neither** — incident narratives, enumerations derivable from the code, and the - reasoning behind a decision already taken. Those belong in commit bodies, `plans/` and - `docs/decisions/`. - -**Every subject moved into a skill leaves one line behind.** The skill carries the why; -CLAUDE.md carries the sentence that stops someone getting it wrong before they think to -load anything. This is not redundancy — it is the whole reason the split is safe. Reserve -it for failures that do not announce themselves: a read that skips a storage tier returns -partial history without erroring, and an identity-adjacent column is far cheaper to prevent -than to revert from production. - -**Measure, never estimate.** This file is mostly contracts and identifiers, which do not -compress — only the prose around them does, so a guess at what a rewrite will save runs -high. Count with `wc -c` before and after. After moving content, prove nothing was dropped -rather than assuming it: extract every backticked span and figure from the old text, confirm -each still appears somewhere in the new set, and account for every apparent casualty by -name. - -## Build & Run Commands - -Pure-Go build, `CGO_ENABLED=0` by default. Standard `make` targets (`build`, `run`, `test`, `lint`, `format`, `clean`) — see the Makefile; `make test` runs fmt + vet + `go test -race`, `make lint` also checks forbidden imports. - -Gotcha: `-race` needs cgo, so targeted race runs use `CGO_ENABLED=1 go test -race -run TestX .//` (macOS tolerates `0`, Linux does not). Benchmarks (`-bench=.`, no `-race`) don't need cgo. `make test` starts with `go clean -cache`, so a full run rebuilds `modernc.org/sqlite` from scratch — minutes, not seconds. - -## Architecture Overview - -A self-hosted FX-rate monitor. The `collector` binary scrapes each configured rate -source on every invocation (plain HTTP, or a chromedp-driven headless browser for -JS-rendered pages), extracts the numeric rate via per-source rules, and stores it in -SQLite. The `notifier` binary runs a check-agent that evaluates user subscription -conditions (delta / interval / daily / cron) against the latest rates and enqueues -notifications, and a dispatch-agent that drains the pool and sends them over Telegram. -The `web` binary serves a REST API plus an embedded dashboard (HTML and a WASM build) -and routes Telegram callbacks. `migrator` applies schema migrations; `doctor` provides -operator tooling (LLM rule generation and source auditing). - -Sources use a `kind` of `BID`, `ASK`, or `LAST` (equity / last-traded price). Per-source -fetch behaviour — header overrides, the proxy opt-in — lives in the `options` JSON column -(`domain.RateSourceOptions`). Several sources may share one URL and therefore one fetch; -that batching is load-bearing and easy to break. **Skill: `beacon-collection`.** - -**Collection egress is direct by default.** Two levels must agree before anything is -proxied: `BEACON_PROXY_URL` says a proxy exists, `rate_sources.options.use_proxy` says the -source wants it. No source is opted in today, and the default is a measured decision -(issue #16) — do not reverse it casually. Chromedp and weather stay direct regardless. - -**Never widen `OpenMeteo.Forecast`'s `daily` block.** Its index `[0]` *is* today for the -morning summary and all four daily-metric latches. The multi-week fetch is a separate call -(`ForecastRange`, its own table, its own daily cadence) for exactly that reason. - -> `cmd/doctor` is the operator-only umbrella for LLM rule (re)generation and source auditing (`rulegen` single/`--all`, `audit --all`/`--source`). Usage, exit codes, and env vars: `cmd/doctor/README.md` + godoc. - -### Layer Responsibilities + The `description` frontmatter *is* the load trigger: name the packages, paths, symbols and + env vars that pull it in. One that summarises the prose instead means the skill never loads + and the knowledge is lost. +- **Neither** — incident narratives, enumerations derivable from the code, and the reasoning + behind a decision already taken: commit bodies, `plans/` and `docs/decisions/`. + +**Every subject moved into a skill leaves one line behind** — the skill carries the why, +CLAUDE.md the sentence that stops someone getting it wrong before they think to load +anything. That redundancy is what makes the split safe, so reserve it for failures that stay +silent, like a read skipping a storage tier and returning partial history without erroring. +And **measure, never estimate**: identifiers do not compress, so count with `wc -c` before +and after, then prove the move lost nothing by extracting every backticked span and figure +from the old text and accounting for each by name. Full procedure: `standards-layout` R21. + +## Architecture + +A self-hosted FX-rate monitor. Five binaries over one SQLite file. + +| Binary | Role | +|---|---| +| `collector` | Scrapes each rate source per invocation (plain HTTP, or a chromedp-driven headless browser for JS-rendered pages), extracts the numeric rate via per-source rules, stores it; also collects weather | +| `notifier` | Check-agent evaluates subscription conditions (delta / interval / daily / cron) against latest rates and enqueues them; dispatch-agent drains the pool and sends over Telegram | +| `web` | REST API plus embedded dashboard (HTML and a WASM build); routes Telegram callbacks | +| `migrator` | Applies schema migrations — the only thing that mutates schema | +| `doctor` | Operator tooling: LLM rule generation and source auditing | | Layer | Location | Role | |-------|----------|------| -| Entry point | `cmd//` | Composition root per binary (collector, notifier, web, migrator, doctor, wasm) | -| Application | `internal/application/` | What the answer is, free of transport: collection, notification, chart, digest, rulegen, sourceaudit | +| Entry point | `cmd//` | Composition root per binary, plus `wasm` | +| Application | `internal/application/` | What the answer is, free of transport | | Domain | `internal/domain/` | Value objects / models, no logic | | DTO | `internal/dto/` | JSON wire contract shared by the server (gateway) and the WASM client | | Gateway | `internal/gateway/` | Receiving and rendering: HTTP routers, middleware, Telegram update loop | | Repository | `internal/repository/` | Persistence queries | | Infrastructure | `internal/infrastructure/` | External clients (SQLite, Telegram, AI providers) | | Tools | `internal/tools/` | Cross-cutting utilities | -| Frontend | `cmd/wasm/` | GOOS=js GOARCH=wasm dashboard (apiclient, application, ui, dom) | +| Frontend | `cmd/wasm/` | GOOS=js GOARCH=wasm dashboard | -### Key Patterns +**Startup ordering.** Anything that logs or can `log.Fatalf` on bad config belongs in `main` +*after* the logger exists, never in a package initialiser: the cron wrappers discard stderr, +so a line emitted earlier is attributable to nothing. Operators grep the marker sequence +`logger -> settings -> dependencies -> repositories -> runners`. -- **Repository pattern** — each repository type owns its own SQL, migration, and query helper functions. Queries execute inside explicit transactions (`r.db.Transaction(ctx)` to write, `r.db.ReadOnlyTransaction(ctx)` to read). Repositories are passed as interfaces into service and handler layers. -- **Configuration injection** — `BEACON_SQLITEDB_DSN` and `BEACON_TELEGRAMBOT_DSN` are read via `dsninjector.Unmarshal(envName)` at startup in `cmd/web/main.go` and live in the systemd `EnvironmentFile`. The public HTTPS origin is passed via the `--api-dsn` CLI flag (format: `https:///`, parsed by `dsninjector.Parse`) and is hardcoded in the systemd unit's `ExecStart` line — never in `.env`. All three configs must be present at startup; the binary calls `log.Fatalf` on any missing value. -- **Startup ordering** — anything that logs or can `log.Fatalf` on bad config belongs in `main` *after* the logger exists, never in a package initialiser: the cron wrappers discard stderr, so a line emitted earlier is attributable to nothing. Operators grep the marker sequence `logger -> settings -> dependencies -> repositories -> runners`. -- **Embedded assets** — `cmd/web/main.go` embeds the `static/` directory via `//go:embed static`. All static files served by `http.FileServer` live under `cmd/web/static/`. -- **Auth: Telegram WebApp initData HMAC** — the `/api/v1/me/...` endpoint family authenticates callers by verifying the Telegram WebApp `initData` HMAC-SHA256 signature. The signing algorithm uses `secret_key = HMAC_SHA256("WebAppData", botToken)` (the string literal is the key; the token is the message). Implementation lives in `internal/tools/tgwebapp/initdata.go`. The check runs **once**, in `middleware.TelegramInitData`, mounted over `routes.MePrefix` — **a new authenticated route belongs on that inner mux; putting it on the outer one is a bypass, and nothing will say so.** Handlers read the caller via `middleware.UserIDFrom` and refuse without it. No other endpoint requires this auth. +## Tripwires -### HTTP surface +Each fails without an error; the reasoning is in the named skill. -Routes are registered in `internal/gateway/`; wire shapes live in `internal/dto`. Two rules -hold everywhere and are easy to break silently: - -- The `/api/v1/me/*` family is the **only** authenticated surface, and the signed `initData` is - accepted **only** in the `X-Telegram-Init-Data` header — never a query string, which would - leak a signed payload into access logs and `Referer`. -- A `/api/v1/me/*` resource owned by another user returns **404, never 403**. Existence is not - disclosed, anywhere. - -Everything else — per-endpoint contracts, the forced weather subscriptions and their 409, -content-hashed WASM URLs and the nginx location ordering, Mini App navigation — is in the -**`beacon-http-api`** skill. - -`GET /ping` (alias `/healthz`) is liveness and touches no dependency; `GET /health/check` is -readiness and probes every dependency for real. Both are unauthenticated. - -### Database - -Engine: SQLite, accessed via the pure-Go `modernc.org/sqlite` driver (no CGO). - -Three PRAGMAs are applied on connection open: -- `foreign_keys=ON` and `busy_timeout=5000` are passed as `?_pragma=` - query parameters on the DSN (see `connectionOptions` in - `config.go`). The `modernc.org/sqlite` driver re-applies them in - its `Open` hook on every new connection the `database/sql` pool - opens, which is the only way to keep these per-connection settings - consistent across `SetMaxOpenConns(N>1)`. -- `journal_mode=WAL` is persisted in the database file header and is - set once via `db.Exec` inside `NewSQLiteClientEx`. - -`busy_timeout` (5 s) is the driver-level retry window for lock -contention; it must stay strictly less than the Go-level `Timeout` so -the context deadline always fires after the driver retry expires. - -**Writes open `BEGIN IMMEDIATE`; reads stay deferred.** The DSN also carries -`_txlock=immediate`, and the driver applies that begin mode only when -`sql.TxOptions.ReadOnly` is false — so `Transaction` takes the WAL write lock at -`BEGIN` while `ReadOnlyTransaction` keeps a plain deferred `BEGIN` and still runs -concurrently with a writer. - -That split is what makes `busy_timeout` reachable at all. A deferred transaction -begins as a reader and *promotes* at its first write, and SQLite refuses to invoke -the busy handler on a promotion — two connections both waiting to promote would -deadlock — so it returns `SQLITE_BUSY` on the spot. Collector/notifier/web -contention therefore lost writes in milliseconds while a 5 s retry window sat -unused (12 rate values and 5 `execution_history` rows in one production log). -Taking the lock at `BEGIN` is not a promotion, so the wait is real. - -Consequences for new code: - -- **`Transaction` is for paths that write.** Reads go through `ReadOnlyTransaction` or - they serialise against each other for nothing. `SQLiteClient.Rollback` — and `Ping`, and - through it the `/health/check` inspector — is read-only for that reason: a readiness - probe queued behind a collector tick would report a busy database as a dead one. -- **Two write transactions cannot be open at once** in one process; the second `BEGIN` - waits for the first. Open, write and commit inside one function, as every repository - method does. - -Foreign keys point from `rate_values`, `rate_user_subscriptions`, and -`rate_user_events` to `rate_sources(name)` with `ON DELETE CASCADE` — -deleting a source destroys all dependent rows. See the warning on -`RemoveRateSource` before wiring it to any endpoint. - -Two things that look free to change and are not. **`weather_observations.provider` only -ever holds `'open-meteo'` but partitions two composite indexes** — dropping the vestigial -column degrades them. And **runtime state never goes on `rate_sources`**: `RetainRateSource` -rewrites those rows wholesale (`cmd/doctor rulegen` does exactly that), so a column added -there is destroyed by an unrelated config write — which is why the source-health latch lives -in its own `rate_source_health` table. - -**Long-range forecast rows belong in `weather_forecast_days`, never in -`weather_observations`**: the collector sweeps that table by `captured_at` at 48 h on every -tick, so a row describing a day two weeks out is gone a day and a half after it is written, -without an error anywhere. +**Collection egress is direct by default.** Two levels must agree: `BEACON_PROXY_URL` says +a proxy exists, `rate_sources.options.use_proxy` says the source wants it. No source is +opted in today; the default is a measured decision (issue #16) — do not reverse it +casually. Chromedp and weather stay direct regardless. +**Never widen `OpenMeteo.Forecast`'s `daily` block**: index `[0]` *is* today for the morning +summary and all four daily-metric latches, which is why the multi-week fetch is a separate +call (`ForecastRange`, its own table, its own daily cadence). **Skill: `beacon-collection`.** **`rate_values` and `execution_history` are tiered.** Each has an `*_archive` twin in the -same file: reads must span both via `UNION ALL`, writes touch hot only. Getting this wrong -returns partial history without erroring. Schema lives at `./migrations/*.sql` and applied -filenames are **immutable**. Both, plus roll-over, retention, VACUUM and how to read a -production snapshot: **skill `beacon-storage`**. `cmd/migrator` is the only thing that -mutates schema; service binaries call `sqlitedb.RequireMigratedSchema` and refuse to start -against an unmigrated database. - -### Environment Variables - -- `BEACON_SQLITEDB_DSN` — SQLite connection string, parsed via `dsninjector.Unmarshal`. Format: `sqlite://` -- `BEACON_TELEGRAMBOT_DSN` — Telegram bot credentials parsed via `dsninjector.Unmarshal`. Format: `:@` where `Addr()` returns the token and `Login()` returns the admin chat ID. -- `BEACON_PROXY_URL` — optional outbound proxy URL. Format: `://:` (e.g. `http://127.0.0.1:7788`). Resolved through `proxyutil.ResolveURL`. `cmd/doctor` proxies through it unconditionally; `cmd/collector` routes nothing through it on its own — see the egress rule above and the `beacon-collection` skill. Telegram Bot API traffic bypasses any proxy unconditionally, enforced by a hardcoded `Proxy: nil` transport in `internal/infrastructure/telegrambot/tbotclient.go`. Do not configure `HTTPS_PROXY`, `HTTP_PROXY`, or `NO_PROXY` — no component in this project consults them. -- `BEACON_CHROMIUM_PATH` — optional absolute path to the Chromium/Chrome binary for `fetcher_kind='chromedp'` sources. Read by `cmd/collector` and `cmd/doctor`. When unset, chromedp searches PATH (`chromium`, `chromium-browser`, `google-chrome`, `chrome`). -- `BEACON_AI_PRIMARY_DSN` (required) and `BEACON_AI_FALLBACK_DSN` (optional) — AI provider DSNs read only by `cmd/doctor rulegen`. See `cmd/doctor/README.md` for the DSN format and provider details. - -> The public HTTPS origin of the `cmd/web` server is **not** an env var — see the `--api-dsn` CLI flag on the `cmd/web` binary, baked into the systemd unit's `ExecStart` line. - -> Never read or edit `.env` files. - -### Deployment - -Standard release layout: immutable `/opt/beacon/artifacts//` build sets and a `bin/release` channel symlink the units run through. **Security boundary**: the CI deploy user may write only under `artifacts/` and `bin/`; `.env`, the DB, and the base dir are root-owned and out of reach. The `release.yml` job (on an `r_*` tag) uploads a new `artifacts//`, flips the symlink, runs migrations via the **`beacon-migrate` one-shot unit (root, so the deploy user never writes the DB)**, restarts `beacon`, and health-gates on `/health/check` with one-symlink rollback. Schema reconciliation is deploy-time, not startup-time — the service unit has no `ExecStartPre` migrator. `make init` provisions the layout, both units, the narrow sudoers grants, and the nginx vhost; `make deploy-configs` ships later `configs/` changes passwordlessly, except the two sudoers files and the installer script itself — those stay with `init` because an installer that could rewrite its own grant would be passwordless root. See `deploy/README.md`. - -An **`s_*` tag runs the gate only** — lint, tests, production-shape build, no host contact — -for when the full gate will not run locally. Everything below is about `r_*`. +same file: reads must span both via `UNION ALL`, writes touch hot only — getting this wrong +returns partial history without erroring. Writes open `Transaction` (`BEGIN IMMEDIATE`), +reads `ReadOnlyTransaction`; the write path for a read serialises it for nothing. Applied +migration filenames are **immutable**, and service binaries call +`sqlitedb.RequireMigratedSchema`, refusing to start against an unmigrated database. +**Long-range forecast rows belong in `weather_forecast_days`, never `weather_observations`**: +the collector sweeps that table by `captured_at` at 48 h every tick, so a row describing a +day two weeks out is gone a day and a half after it is written, with no error anywhere. Two +columns look free to change and are not — `weather_observations.provider`, and anything +runtime-valued added to `rate_sources`. **Skill: `beacon-storage`.** + +**`/api/v1/me/*` is the only authenticated surface**, and the check runs **once**, in +`middleware.TelegramInitData` mounted over `routes.MePrefix` — **a new authenticated route +belongs on that inner mux; putting it on the outer one is a bypass, and nothing will say +so.** Signed `initData` is accepted **only** in the `X-Telegram-Init-Data` header, never a +query string, which would leak it into access logs and `Referer`. A `/api/v1/me/*` resource +owned by another user returns **404, never 403** — existence is not disclosed, anywhere. +`GET /ping` (alias `/healthz`) is liveness and touches no dependency; `GET /health/check` is +readiness and probes every dependency for real. Both unauthenticated. +**Skill: `beacon-http-api`.** -There is **no staging**: an `r_*` tag, prerelease or not, flips the production symlink and +**There is no staging.** An `r_*` tag, prerelease or not, flips the production symlink and restarts the service. Tags are cut from `alpha`, not `main` — see the working agreement. Do -not tag casually. Delete the superseded alpha tag, local and remote, once the new one is -live. Remote hosts are read-freely, mutate-never without explicit per-action approval. - -## Error Handling - -`internal.PublicError` (in `internal/errors.go`, alongside `TraceError`, `StackTraceError`, `HttpCodeError`, and the `ErrNotFound` sentinel) carries messages **safe to show to end users**. Wrap at the point the error is created (service layer) with `internal.NewPublicError("...")` when the failure meaningfully tells the user something; return a plain `error` for everything else (DB down, unexpected nil, ...). The controller catches every sub-handler error and sends `PublicError.Details()` for a public error, else a generic fallback constant. - -Every controller test on an error branch must assert: (1) a response was actually sent (user not left in silence), (2) its text equals `PublicError.Details()` for a public error, (3) its text equals the fallback constant for a plain error. +not tag casually, and delete the superseded alpha tag, local and remote, once the new one is +live. An `s_*` tag runs the gate only, contacting no host. Release layout, CI security +boundary, `make init` / `make deploy-configs`, one-symlink rollback: `deploy/README.md`. + +## Configuration + +Env var names, formats, which binary needs which, and the Chromium PATH fallback are +declared once in `internal/env.go` (all parsed via `dsninjector.Unmarshal`) — read them +there, not from a copy. `.env.example` has worked DSN examples. + +- **The standard proxy variables do nothing here.** No component consults `HTTPS_PROXY`, + `HTTP_PROXY` or `NO_PROXY`; `BEACON_PROXY_URL`, resolved through `proxyutil.ResolveURL`, is + the only knob, and Telegram Bot API traffic bypasses it unconditionally + (**skill: `beacon-collection`**). +- **The public HTTPS origin is not an env var.** It is the `--api-dsn` flag on `cmd/web` + (format `https:///`, parsed by `dsninjector.Parse`), hardcoded in the systemd unit's + `ExecStart` line — never in `.env`. +- Required config must be present at startup; the binary calls `log.Fatalf` on any missing + value. Never log a settings parser error — it carries the credential (`make lint` fails + on it). +- **Never read or edit `.env` files.** ## Data & Privacy -This project stores the **minimum personal data required** to function as a Telegram bot — -not zero PII, but nothing beyond what delivering notifications requires. +Store the **minimum personal data required** to run as a Telegram bot — not zero PII, but +nothing beyond what delivering notifications requires. Pre-approved for user-scoped tables, no discussion needed: Telegram `chat_id`, IANA timezone, BCP-47 locale, and coordinates of a city the user picked from a geocoding search. @@ -238,10 +144,9 @@ timezone, BCP-47 locale, and coordinates of a city the user picked from a geocod photo, biometrics, device- or IP-derived location, IP address, device fingerprint, user-agent. Same list for log output — `chat=` is fine, nothing else is. -Anything not on either list: **do not persist it yet, ask first.** Identity-adjacent columns -are far easier to prevent than to revert from a production database. Full policy, the -guardrails on each pre-approved field, and how to classify a borderline one: **skill -`beacon-data-privacy`**. +Anything not on either list: **do not persist it yet, ask first** — identity-adjacent +columns are far easier to prevent than to revert from a production database. Full policy, +per-field guardrails, and borderline classification: **skill `beacon-data-privacy`**. ## Constraints @@ -250,35 +155,28 @@ guardrails on each pre-approved field, and how to classify a borderline one: **s Enforced via `make lint`. - **Scratch files** go to `./tmp/` (e.g. `./tmp/probe_*`), never the repo root; bare `go build ./cmd/web` drops a `./web` binary in the root, which is not gitignored. +- **Remote hosts** are read-freely, mutate-never without explicit per-action approval. ## Working agreement -All non-trivial work follows the plan-first pipeline: - -1. **Plan** — the `architect` agent writes `plans/NNN-slug.md` (create via the - `pipeline:new-plan` skill). No source edits before a plan exists. -2. **Implement** — the `engineer` agent executes the plan's tasks with tests. -3. **Review** — three `reviewer` agents launched in parallel in ONE message, each - prompt naming its lens (A: correctness & tests, B: security & operations, - C: performance & architecture) and the changed files. Full three-lens fan-out is - mandatory on the first review; the post-fix re-review is ONE solo reviewer scoped - to the changed lines. -4. **Gate** — `make test` must be green before review; a red tree goes to the - `testdoctor` agent first, at any stage. -5. **Complete** — the orchestrator merges the three reports, deduplicates, resolves - conflicting verdicts (naming what was rejected and why; the user has final say). - P0/P1 findings loop back to the engineer. Only when every P0/P1 is fixed or - explicitly accepted: move the plan via the `pipeline:complete-plan` skill. - -Plans live in `plans/` (active), `plans/completed/` (shipped, `YYMMDD.NNNN.slug.md`), -`plans/history/` (abandoned/superseded). One plan per concern. - -Branch as `type/-` **off `alpha`** and open the PR against `alpha` — work -integrates there and release tags are cut from it. `main` only ever moves to the latest -**non-prerelease** tag, so it trails `alpha` by a whole alpha series. Never commit to -either directly. - -Two silent traps. **A merge into `alpha` does not close its issue** — GitHub honours -`Closes #N` only on the default branch, `main`; close it by hand, naming the squash commit -and its tag. And **`gh pr create` defaults to `main`**, the stale release pointer, so pass -`--base alpha`. +Plan-first pipeline; the canonical procedure is the `pipeline:working-agreement` skill — +load it before starting non-trivial work. Project delta: + +- **Gate:** `make test` (fmt, `go vet`, the full `go test -race` suite, then WASM tests) plus + `make lint-new` — the mergeable gate, linting only what changed since `origin/alpha`, + while `make lint` scans the whole tree as a worklist. Both run **two** steps, + `golangci-lint run` *and* `scripts/lint-checks.sh` — a green `golangci-lint` is **not** a + green `make lint`. `make test` opens with `go clean -cache`, so a full run rebuilds + `modernc.org/sqlite` from scratch: minutes, not seconds. `-race` needs cgo, so a targeted + rerun is `CGO_ENABLED=1 go test -race -run TestX .//` (macOS tolerates `0`, Linux does + not); benchmarks (`-bench=.`, no `-race`) don't. **On the pi5 `make test` dies compiling + `modernc.org/sqlite` under `-race`** — rerun as `go test -race -p 1`, the only route to a + green gate on the one machine that runs it. +- **Lenses:** standard set — see `pipeline:working-agreement`, which includes lens O. +- **Branching:** branch `type/-` **off `alpha`** and open the PR against + `alpha` — work integrates there and release tags are cut from it. `main` only ever moves to + the latest **non-prerelease** tag, so it trails `alpha` by a whole alpha series. Never + commit to either directly. Two silent traps: **a merge into `alpha` does not close its + issue** — GitHub honours `Closes #N` only on the default branch, `main` — so close it by + hand, naming the squash commit and its tag; and **`gh pr create` defaults to `main`**, the + stale release pointer, so pass `--base alpha`.