Review fixes and cleanup

apps/server/drizzle/0001_destinations.sql

@@ -1,4 +1,4 @@
--- Chapter 10 — Destinations + Subscriptions migration.
+-- Destinations + Subscriptions migration.
 --
 -- 1. Create `destinations` (named CRUD list).
 -- 2. Backfill destinations from each distinct `subscriptions.destination_chat_id`

apps/server/drizzle/0002_library_filters.sql

@@ -1,4 +1,4 @@
--- Chapter 11 — Library filters + M:N join.
+-- Library filters + M:N join.
 --
 -- Reusable named filter rules attachable to many subscriptions. The
 -- evaluator UNIONs these with per-sub filters at evaluation time.

apps/server/drizzle/0005_drop_tg_session.sql

@@ -2,7 +2,7 @@
 --
 -- Originally created in 0000 as an optional encrypted-at-rest cache for the
 -- gramjs StringSession. The MVP design ended up using `TG_SESSION_STRING`
--- from .env as the source of truth (see PLAN.md "Session storage"). The
+-- from .env as the source of truth. The
 -- table was never written to from app code, and `TG_SESSION_ENCRYPTION_KEY`
 -- — the env knob that would have driven AES around it — was removed in
 -- favour of plain env-only storage.

apps/server/drizzle/0007_filter_mode.sql

@@ -1,12 +1,11 @@
--- Per-filter include/exclude mode (Ch 14).
+-- Per-filter include/exclude mode.
 --
 -- 'include' is the legacy AND-pass default — every include filter must match
 -- for the message to forward. 'exclude' inverts the rule for that one row:
 -- a message is rejected when its rule matches.
 --
--- The CHECK keeps hand-written rows honest (mirrors the precedent set by
--- `forward_log.status` and `library_filters.rule_type`). Existing rows
--- back-fill to 'include' via the DEFAULT, preserving prior behavior.
+-- The CHECK keeps hand-written rows honest. Existing rows back-fill to
+-- 'include' via the DEFAULT, preserving prior behavior.
 
 ALTER TABLE `subscription_filters` ADD COLUMN `mode` TEXT NOT NULL DEFAULT 'include' CHECK (`mode` IN ('include', 'exclude'));
 --> statement-breakpoint

apps/server/src/api/auth.ts

@@ -1,25 +1,4 @@
-/**
- * Auth primitives for the API server.
- *
- * Single-user model. A successful `POST /api/auth/login` mints a fresh
- * opaque random token (256-bit, base64url), stores it in `web_sessions` with
- * an expiry, and sets a signed cookie carrying that token as the value.
- *
- * The signature exists for tamper detection; the *security* comes from the
- * token being unguessable and existing in the DB. Logout deletes the row,
- * so a leaked cookie can be revoked. Sliding refresh on each authed request
- * keeps the user signed in while limiting damage from a long-unused
- * captured cookie.
- *
- * `requireWebAuthEnv` parallels `tg/client.ts#requireTelegramEnv`. Both
- * `WEB_PASSWORD` and `SESSION_SECRET` stay `.optional()` in `config.ts`
- * so `pnpm db:migrate` and `pnpm tg:login` can run without them; only
- * the server boot path requires them.
- *
- * `verifyPassword` hashes both inputs to a fixed-size SHA-256 digest
- * before `timingSafeEqual` so password length isn't observable as a
- * timing signal and the equal-length precondition is automatic.
- */
+// Opaque 256-bit DB-backed token in a signed cookie (the token is the security; the signature is tamper detection); sliding refresh, server-side revoke.
 import { createHash, timingSafeEqual } from 'node:crypto';
 import type { CookieSerializeOptions } from '@fastify/cookie';
 import type { FastifyReply, FastifyRequest } from 'fastify';
@@ -35,7 +14,6 @@ export interface WebAuth {
   sessionSecret: string;
 }
 
-/** Bot token + admin allowlist for the Telegram Web App sign-in route. */
 export interface TelegramAuth {
   botToken: string;
   adminIds: string[];
@@ -58,10 +36,9 @@ export function requireWebAuthEnv(cfg: Config): WebAuth {
 }
 
 export function verifyPassword(plain: string, expected: string): boolean {
-  // Defensive: refuse to match empty/missing credentials even if upstream
-  // validation lets them through. SHA-256 of '' is a deterministic constant —
-  // without this guard, two empty strings would compare equal and grant entry.
+  // Reject empty: SHA-256('')==SHA-256('') would grant entry.
   if (!plain || !expected) return false;
+  // Hash both first so password length isn't a timing signal.
   const a = createHash('sha256').update(plain).digest();
   const b = createHash('sha256').update(expected).digest();
   return timingSafeEqual(a, b);
@@ -87,11 +64,7 @@ export function clearedCookieOptions(isProd: boolean): CookieSerializeOptions {
   };
 }
 
-/**
- * Read a session token from the incoming signed cookie, or null if absent /
- * tampered. Does NOT verify against the DB — the auth pre-handler does that
- * via `sessionStore.verifyAndRefresh`.
- */
+// Unwraps the signed cookie only; the DB check is verifyAndRefresh.
 export function readSessionToken(request: FastifyRequest): string | null {
   const raw = request.cookies[SESSION_COOKIE_NAME];
   if (!raw) return null;
@@ -102,11 +75,6 @@ export function readSessionToken(request: FastifyRequest): string | null {
   return result.value;
 }
 
-/**
- * Build the Fastify pre-handler that enforces auth on a scoped route plugin.
- * The factory takes the session store so the pre-handler closes over a real
- * DB-backed lookup; tests build their own via `buildTestApp`.
- */
 export function makeRequireAuth(sessionStore: SessionStore) {
   return async function requireAuth(request: FastifyRequest, _reply: FastifyReply): Promise<void> {
     const token = readSessionToken(request);

apps/server/src/api/errorHandler.ts

@@ -1,17 +1,4 @@
-/**
- * Single Fastify error handler.
- *
- * All errors thrown from route handlers, pre-handlers, and Fastify itself
- * pass through here. The mapping:
- *
- *   - `ZodError` (route handler called `schema.parse(body)` and it threw)
- *     → 400 with `{ code: 'validation_error', issues }`
- *   - `AppError` subclasses (`UnauthorizedError`, `NotFoundError`, ...)
- *     → `err.statusCode` with `{ code, message, ...(issues if ValidationError) }`
- *   - anything else → 500 generic; original message NOT echoed (no leaking
- *     of internal details). Logged with `request.log.error({ err })` for
- *     diagnosability.
- */
+// Single Fastify error handler: ZodError → 400, AppError → its statusCode, anything else → generic 500 with the original message never echoed.
 import type { FastifyError, FastifyReply, FastifyRequest } from 'fastify';
 import { ZodError } from 'zod';
 import type { ErrorResponse } from '@tg-feed/shared';
@@ -48,10 +35,7 @@ export function makeErrorHandler(logger: Logger): ApiErrorHandler {
       return;
     }
 
-    // Fastify built-in errors (rate-limit, body too large, malformed JSON,
-    // etc.) carry an HTTP statusCode + a stable code. Pass 4xx through with
-    // their own message — they're already client-safe. 5xx still gets the
-    // generic 500 path below to avoid leaking internals.
+    // Fastify built-in 4xx (rate-limit, body too large, bad JSON) are client-safe; pass through. 5xx falls to the generic path.
     if (typeof err.statusCode === 'number' && err.statusCode >= 400 && err.statusCode < 500) {
       const body: ErrorResponse = {
         error: { code: err.code ?? 'client_error', message: err.message },

apps/server/src/api/routes/_params.ts

@@ -1,7 +1,4 @@
-/**
- * Shared route param schemas. The single-`id` shape is used by every CRUD
- * route; compound shapes live in their own route files.
- */
+// Shared by every CRUD route; compound param shapes live in their own route files.
 import { z } from 'zod';
 
 export const idParamsSchema = z.object({

apps/server/src/api/routes/auth.ts

@@ -1,21 +1,3 @@
-/**
- * Auth routes.
- *
- * Split between two registration functions — `registerLoginRoute` for
- * the unauthenticated `POST /api/auth/login`, and `registerAuthRoutes`
- * for the authed-scope `POST /api/auth/logout` and `GET /api/me`. The
- * factory wires each into the appropriate Fastify scope.
- *
- * Login mints an opaque random token (256-bit, base64url) via the session
- * store and sets it as the signed cookie value. Logout deletes the row, so
- * a leaked cookie is revoked on the server — not just cleared on the
- * client. The login-route rate limit only counts failed attempts so a
- * legitimate user mid-debugging doesn't burn the brute-force bucket.
- *
- * `registerTelegramAuthRoute` is the Mini App counterpart: it verifies a
- * Telegram `initData` payload and, if the embedded user is on the admin
- * allowlist, mints a session via the same store as the password login.
- */
 import type { FastifyInstance, FastifyRequest } from 'fastify';
 import {
   loginRequestSchema,
@@ -48,11 +30,7 @@ export function registerLoginRoute(app: FastifyInstance, deps: RegisterLoginDeps
   app.post(
     '/auth/login',
     {
-      // Rate-limit brute-force on the single-user password. Counted per IP
-      // (trustProxy is enabled at the factory level so we key on the real
-      // client IP behind a reverse proxy). `skipOnError: true` and the
-      // `skipSuccessfulRequests`-equivalent below keep the bucket reserved
-      // for failed attempts only.
+      // Brute-force limit on the password, per real client IP; reset on success below so only failures count.
       config: {
         rateLimit: {
           max: 10,
@@ -69,15 +47,12 @@ export function registerLoginRoute(app: FastifyInstance, deps: RegisterLoginDeps
         );
         throw new UnauthorizedError('invalid password');
       }
-      // Successful login — reset the rate-limit bucket for this IP so a
-      // legitimate operator who logged in mid-debugging doesn't get throttled
-      // moments later. `request.rateLimit()` is provided by @fastify/rate-limit
-      // when the route's `config.rateLimit` is set.
+      // Reset the rate-limit bucket for this IP on success so only failures count.
       try {
         const rl = (request as FastifyRequest & { rateLimit?: () => Promise<unknown> }).rateLimit;
         if (typeof rl === 'function') await rl();
       } catch {
-        // Plugin not loaded (test path with custom server). Safe to ignore.
+        // Plugin not loaded (test path with custom server).
       }
       const token = deps.sessionStore.create();
       reply.setCookie(SESSION_COOKIE_NAME, token, signedCookieOptions(deps.isProd));
@@ -91,11 +66,7 @@ export function registerLoginRoute(app: FastifyInstance, deps: RegisterLoginDeps
 const TELEGRAM_INITDATA_MAX_AGE_SEC = 5 * 60;
 
 export interface RegisterTelegramAuthDeps {
-  /**
-   * Live getter for the bot auth config (token + admin allowlist), resolved
-   * DB-over-env. Read per request so a config change made via Settings → Bot
-   * applies without a restart. Yields null when the bot isn't configured.
-   */
+  // read per request (DB-over-env) so a Settings → Bot change applies without a restart; null when unconfigured
   getTelegramAuth: () => TelegramAuth | null;
   isProd: boolean;
   logger: Logger;
@@ -154,7 +125,7 @@ export function registerTelegramAuthRoute(
         const rl = (request as FastifyRequest & { rateLimit?: () => Promise<unknown> }).rateLimit;
         if (typeof rl === 'function') await rl();
       } catch {
-        // Plugin not loaded (test path with custom server). Safe to ignore.
+        // Plugin not loaded (test path with custom server).
       }
 
       const token = deps.sessionStore.create();
@@ -173,9 +144,7 @@ export interface RegisterAuthDeps {
 
 export function registerAuthRoutes(app: FastifyInstance, deps: RegisterAuthDeps): void {
   app.post('/auth/logout', async (request, reply) => {
-    // Read the token from the signed cookie before clearing it; deleting the
-    // server-side row is what actually invalidates the session (the cookie
-    // bytes are otherwise still valid until they expire client-side).
+    // Deleting the server-side row is what invalidates the session; the cookie bytes stay valid until expiry.
     const token = readSessionToken(request);
     if (token) deps.sessionStore.revoke(token);
     reply.clearCookie(SESSION_COOKIE_NAME, clearedCookieOptions(deps.isProd));

apps/server/src/api/routes/botConfig.ts

@@ -1,22 +1,4 @@
-/**
- * Bot configuration routes — Settings → Bot.
- *
- * GET    /api/config/bot                → masked state (sources, key status, botRunning)
- * PUT    /api/config/bot                → partial save (token / admins / publicUrl)
- * DELETE /api/config/bot                → clear the DB row, env fallback resumes
- * POST   /api/config/bot/resolve-admin  → resolve @username → user (preview, no write)
- *
- * The token is encrypted with `TG_SESSION_ENCRYPTION_KEY` before it touches
- * the DB, so a token write refuses with 412 when no key is configured (admins
- * and public URL are not gated). After any change `reloadBot()` live-swaps the
- * long-polling bot so the new config takes effect without a restart. The GET
- * payload is masked — it never returns the token, only whether one exists and
- * from where.
- *
- * `resolve-admin` turns a `@username` into a user id via the universal chat
- * resolver (rejecting channels/groups); the resolved `{ id, displayName,
- * username }` is then saved into `admins`.
- */
+// Token encrypted with TG_SESSION_ENCRYPTION_KEY (token write 412s without a key); GET is masked, never returns the token.
 import type { FastifyInstance } from 'fastify';
 import {
   type BotAdmin,
@@ -36,22 +18,16 @@ import { BOT_TOKEN_AAD, encryptSecret } from '../../lib/sessionCrypto.js';
 import type { ChatResolver } from '../../tg/chatResolver.js';
 
 export interface RegisterBotConfigRoutesDeps {
-  /** Parsed env config — supplies the env fallbacks behind the masked view. */
+  // Supplies the env fallbacks behind the masked view.
   cfg: Config;
   db: Db;
-  /** Returns the configured 32-byte key, or null when unset. */
+  // 32-byte key, or null when unset.
   getEncryptionKey?: () => Buffer | null;
-  /** Live-swaps the bot so a saved config takes effect immediately. */
   reloadBot?: () => Promise<void>;
-  /** Whether the long-polling bot is currently running. */
   getBotRunning?: () => boolean;
-  /**
-   * Lazy lookup for the universal chat resolver — reused by `resolve-admin`
-   * to turn a `@username` into a user id. Read per request (populated after
-   * Telegram init). When absent, the route returns 503.
-   */
+  // Lazy (populated after Telegram init), read per request; absent → 503.
   getChatResolver?: () => ChatResolver | undefined;
-  /** Telegram lifecycle — picks the right 503 (`initializing` vs `unavailable`). */
+  // Picks the right 503: `initializing` vs `unavailable`.
   getTelegramStatus: () => TelegramStatus;
 }
 
@@ -112,8 +88,7 @@ export function registerBotConfigRoutes(
 
     writeBotConfig(db, patch);
     if (reloadBot) {
-      // Live-swap failures are logged inside `reloadBot`; the row is already
-      // saved, so the next restart picks it up. Don't fail the save.
+      // Row is already saved; don't fail the save on a live-swap error (logged in reloadBot).
       await reloadBot().catch(() => {});
     }
     return info();
@@ -134,8 +109,7 @@ export function registerBotConfigRoutes(
     }
     const body = resolveBotAdminRequestSchema.parse(request.body);
     const resolved = await chatResolver(body.query);
-    // Admins are *users* — a bare positive id. Channels/supergroups resolve to
-    // a `-100…` id and unjoined private invites to null; reject both.
+    // Admins are users (bare positive id); reject `-100…` channels/groups and null invites.
     if (!resolved.chatId || resolved.chatId.startsWith('-')) {
       throw new AppError(
         422,

apps/server/src/api/routes/destinations.ts

@@ -1,10 +1,4 @@
-/**
- * Destinations CRUD routes (Chapter 10).
- *
- * Destinations are the named CRUD list subscriptions pick from. Delete is
- * pre-checked for usage in this layer; the FK is `ON DELETE RESTRICT` as a
- * belt-and-suspenders DB-level guard.
- */
+// Delete is usage-checked here; the FK is also ON DELETE RESTRICT as a DB-level guard.
 import { asc, count, eq } from 'drizzle-orm';
 import type { FastifyInstance } from 'fastify';
 import {
@@ -49,36 +43,12 @@ interface DestinationRow {
 
 export interface RegisterDestinationDeps {
   db: Db;
-  /**
-   * Live status getter used to distinguish "Telegram is starting up — try
-   * again in a moment" from "Telegram is not configured at all". Drives
-   * the error code returned when a tg-dep getter yields undefined.
-   */
   getTelegramStatus: () => TelegramStatus;
-  /**
-   * Lazy lookup for the universal "paste-anything" resolver — backs
-   * `POST /destinations/resolve`. Read per request because the boot path
-   * fills it asynchronously after `app.listen()`. When undefined the
-   * route returns 503 with `telegram_initializing` (during the boot
-   * window) or `telegram_unavailable` (steady-state disconnected).
-   */
+  // Lazy getters: filled after app.listen(), so read per request. Undefined → 503.
   getChatResolver?: () => ChatResolver | undefined;
-  /**
-   * Lazy lookup for the `messages.ImportChatInvite` wrapper invoked from
-   * `POST /destinations` when the body carries `inviteHash`. Same
-   * lifecycle as `getChatResolver`.
-   */
   getImportInvite?: () => ImportInviteFn | undefined;
-  /**
-   * Lazy lookup for the best-effort profile-photo fetcher invoked after
-   * the row is inserted. When undefined the row's `iconDataUrl` stays
-   * null and the access monitor's lazy backfill catches it later.
-   */
+  // Undefined leaves iconDataUrl null; the access monitor backfills it later.
   getFetchProfilePhoto?: () => ProfilePhotoFetcher | undefined;
-  /**
-   * Lazy lookup for the forum-topic lister backing `POST /destinations/topics`.
-   * Same lifecycle as `getChatResolver`. When undefined the route returns 503.
-   */
   getListForumTopics?: () => ForumTopicLister | undefined;
 }
 
@@ -146,8 +116,7 @@ export function registerDestinationRoutes(
         throw new UpstreamError('failed to join via invite link', 'invite_join_failed');
       }
       chatId = join.chatId;
-      // We just successfully joined — record access ok now so the access
-      // monitor's first sweep doesn't briefly mis-report.
+      // Just joined — record access ok so the monitor's first sweep doesn't mis-report.
       initialAccessStatus = 'ok';
       accessCheckedAt = new Date();
     } else {
@@ -168,9 +137,7 @@ export function registerDestinationRoutes(
       .returning()
       .all();
     const row = inserted[0]!;
-    // Best-effort: fetch the profile photo and stamp it on the row. The
-    // fetcher already swallows errors and returns null on failure, so we
-    // don't need a try/catch — the response just goes out without an icon.
+    // Best-effort icon: the fetcher swallows errors and returns null.
     let iconDataUrl: string | null = row.iconDataUrl;
     const fetchProfilePhoto = getFetchProfilePhoto?.();
     if (fetchProfilePhoto) {
@@ -244,8 +211,6 @@ export function registerDestinationRoutes(
 }
 
 function listDestinations(db: Db): DestinationRow[] {
-  // One query: destinations LEFT JOIN subscriptions GROUP BY destination,
-  // counting subscription references.
   const rows = db
     .select({
       id: destinations.id,