Skip to main content

max / makenotwork

21.7 KB · 500 lines History Blame Raw
1 # Contributing to MNW Server
2
3 Patterns, conventions, and rules for working on the MNW server codebase. Read this before making changes.
4
5 ## Project Structure
6
7 ```
8 server/
9 src/
10 main.rs # Entry point
11 lib.rs # Library root, AppState definition
12 config.rs # Configuration from environment
13 error.rs # AppError enum, HTTP error responses
14 auth.rs # Session auth extractors (AuthUser, MaybeUser, AdminUser)
15 csrf.rs # CSRF middleware + token management
16 helpers.rs # Shared utilities (format_price, get_initials, etc.)
17 db/ # Database queries, one file per domain
18 types/ # View types + Db→View conversions
19 routes/ # HTTP handlers, grouped by domain
20 templates/ # Askama template structs (Rust side)
21 payments/ # Stripe integration
22 email/ # Postmark transactional email
23 scanning/ # File scanning (ClamAV, YARA, hash lookup)
24 storage.rs # S3 storage abstraction
25 synckit_auth.rs # SyncKit JWT auth
26 migrations/ # PostgreSQL migrations (numbered, auto-applied on boot)
27 templates/ # Askama HTML templates (Jinja2-like)
28 static/ # CSS, JS, fonts, images
29 tests/ # Integration tests
30 deploy/ # Deployment scripts and configs
31 ```
32
33 ## Error Handling
34
35 All handlers return `Result<T, AppError>`. The `AppError` enum (`src/error.rs`) maps each variant to an HTTP status code, a Sentry tag, and a user-facing message. Internal details (DB errors, storage errors) are logged but never exposed to users.
36
37 ```rust
38 #[derive(Debug, thiserror::Error)]
39 pub enum AppError {
40 #[error("Not found")]
41 NotFound, // 404
42 #[error("Bad request: {0}")]
43 BadRequest(String), // 400, message shown to user
44 #[error("Database error: {0}")]
45 Database(#[from] sqlx::Error), // 500, generic message to user
46 #[error("Internal server error")]
47 Internal(#[from] anyhow::Error), // 500, generic message to user
48 // ... see error.rs for full list
49 }
50 ```
51
52 **Rules:**
53 - Use `?` for error propagation. Never `.unwrap()` in production code.
54 - Use `.ok_or(AppError::NotFound)?` when an optional DB result must exist.
55 - Use `AppError::BadRequest("message")` for user-caused errors. The string is shown directly.
56 - Use `AppError::Validation("message")` for form validation failures (returns 422).
57 - Never expose internal error details to users. `Database` and `Internal` variants always show "Something went wrong."
58 - Convert external errors with `From` impls, not string formatting. Add `#[from]` to AppError variants for automatic conversion.
59
60 On API routes (`/api/*`), a middleware layer (`json_error_layer`) automatically converts HTML error responses to `{"error": "message"}` JSON. Handlers don't need to handle this. It's transparent.
61
62 ## Route Handlers
63
64 ### Signature Pattern
65
66 Every handler follows this structure:
67
68 ```rust
69 #[tracing::instrument(skip_all, name = "module::handler_name")]
70 async fn handler_name(
71 State(state): State<AppState>, // App state (DB pool, config, etc.)
72 session: Session, // Session store
73 AuthUser(user): AuthUser, // Or MaybeUser(maybe_user), or AdminUser
74 Path(slug): Path<String>, // URL parameters
75 Query(params): Query<FilterQuery>, // Query string
76 Form(form): Form<CreateForm>, // POST body (form-encoded)
77 ) -> Result<impl IntoResponse> {
78 let csrf_token = get_csrf_token(&session).await;
79 // ... business logic ...
80 Ok(MyTemplate { csrf_token, session_user: Some(user.into()), /* ... */ })
81 }
82 ```
83
84 **Rules:**
85 - Every handler gets `#[tracing::instrument(skip_all, name = "...")]` for structured logging.
86 - Return type is always `Result<impl IntoResponse>`.
87 - Extract auth requirements via the type system: `AuthUser` (login required), `MaybeUser` (optional), `AdminUser` (admin only, returns 404 to hide admin routes from non-admins).
88 - Askama templates implement `IntoResponse`. Return the struct directly.
89 - CSRF token goes into every template that renders forms.
90
91 ### HTMX Responses
92
93 Full-page handlers extend `base.html` and include `session_user`, `csrf_token`, navigation, etc. HTMX handlers return partial templates (HTML fragments without the base layout).
94
95 ```rust
96 // Full page, extends base.html
97 Ok(FullPageTemplate { csrf_token, session_user: maybe_user, /* ... */ })
98
99 // HTMX fragment, standalone partial, no base layout
100 Ok(FilteredEntriesTemplate { items, current_page, total_pages })
101 ```
102
103 In templates, HTMX attributes trigger fragment requests:
104 ```html
105 <button hx-get="/dashboard/items?page=2"
106 hx-target="#item-list"
107 hx-swap="innerHTML">Next</button>
108 ```
109
110 The CSRF token is included in HTMX requests via the `X-CSRF-Token` header, set globally from the `<meta name="csrf-token">` tag.
111
112 ### Route Module Organization
113
114 Routes are grouped by domain under `src/routes/`. Each domain is either a single file (for small domains) or a directory module (when it exceeds ~500 lines).
115
116 ```
117 routes/
118 mod.rs # Declares modules + re-exports *_routes() functions
119 auth.rs # Login, signup, logout
120 admin.rs # Admin panel (or admin/ directory)
121 pages/ # All public HTML pages (directory module)
122 mod.rs # Composes sub-routers
123 public/ # Public-facing pages
124 dashboard/ # Creator dashboard + HTMX tabs
125 api/ # JSON API endpoints
126 stripe/ # Stripe webhooks + checkout
127 synckit/ # SyncKit API
128 ```
129
130 Each module exposes a `*_routes()` function that returns an `axum::Router`:
131 ```rust
132 pub fn page_routes() -> Router<AppState> {
133 Router::new()
134 .route("/", get(home))
135 .route("/:username", get(profile))
136 // ...
137 }
138 ```
139
140 These are composed in `main.rs` via `.merge()` or `.nest()`.
141
142 ## Database Layer
143
144 ### Query Pattern
145
146 All queries use sqlx with compile-time checking. Queries live in `src/db/`, one file per domain (e.g., `db/users.rs`, `db/items.rs`, `db/synckit.rs`).
147
148 ```rust
149 pub async fn get_user_by_id(pool: &PgPool, id: UserId) -> Result<Option<DbUser>> {
150 let user = sqlx::query_as::<_, DbUser>(
151 "SELECT * FROM users WHERE id = $1"
152 )
153 .bind(id)
154 .fetch_optional(pool)
155 .await?;
156 Ok(user)
157 }
158 ```
159
160 **Rules:**
161 - Always use positional parameters (`$1`, `$2`, ...). Never interpolate values into SQL strings.
162 - Use `sqlx::query_as::<_, DbRow>` for typed results. Use `sqlx::query!` only when the macro's compile-time checking is needed.
163 - `.fetch_one()` when exactly one row expected (errors on zero), `.fetch_optional()` when zero or one, `.fetch_all()` for lists.
164 - Newtype ID wrappers (`UserId`, `ProjectId`, etc.) work directly with `.bind()`. They implement sqlx's `Encode`/`Decode`.
165 - Multi-line SQL uses `r#"..."#` raw strings.
166
167 ### DB Row Types vs View Types
168
169 Database rows are `Db*` structs (e.g., `DbUser`, `DbProject`) in `src/db/`. View types for templates are in `src/types/` (e.g., `User`, `Project`). Conversions between them use `From` trait impls in `src/types/conversions.rs`.
170
171 ```rust
172 // In src/types/conversions.rs
173 impl From<&db::DbUser> for User {
174 fn from(u: &db::DbUser) -> Self {
175 User {
176 username: u.username.to_string(),
177 avatar_initials: get_initials(u.display_name.as_deref().unwrap_or(&u.username)),
178 stripe_connected: u.stripe_account_id.is_some(),
179 // ... computed fields
180 }
181 }
182 }
183 ```
184
185 This separation means:
186 - `Db*` types mirror the database schema exactly (derive `sqlx::FromRow`).
187 - View types have display-ready fields: formatted dates, computed booleans, pre-rendered HTML.
188 - Handlers call `let user: User = (&db_user).into();` to convert.
189
190 ## Type Safety
191
192 ### ID Newtypes
193
194 All database IDs are newtype wrappers defined via the `define_pg_uuid_id!` macro in `src/db/id_types.rs`:
195
196 ```rust
197 define_pg_uuid_id!(UserId, ProjectId, ItemId, VersionId, /* ... */);
198 ```
199
200 This generates `UserId(Uuid)` with `Display`, `FromStr`, `sqlx::Type`, `Encode`, `Decode`, `Serialize`, `Deserialize`, and `Default` (generates new v4 UUID). Use these everywhere. Never pass raw `Uuid` or `String` for IDs.
201
202 ### String Enums
203
204 Domain enums that map to database TEXT columns use the `impl_str_enum!` macro in `src/db/enums.rs`:
205
206 ```rust
207 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
208 pub enum ItemType { Audio, Video, Text, Software, /* ... */ }
209
210 impl_str_enum!(ItemType {
211 Audio => "audio",
212 Video => "video",
213 Text => "text",
214 Software => "software",
215 });
216 ```
217
218 This generates `Display`, `FromStr`, and sqlx `Type`/`Encode`/`Decode`. Add `#[serde(rename_all = "lowercase")]` if the JSON representation should match the database strings.
219
220 ## Migrations
221
222 Migrations live in `server/migrations/` and are numbered sequentially (e.g., `001_initial_schema.sql`, `053_add_video_support.sql`). They run automatically on application boot via sqlx.
223
224 **Rules:**
225 - Use `IF NOT EXISTS` guards wherever possible (tables, indexes, extensions).
226 - Use `TIMESTAMPTZ` for all timestamps (UTC-aware).
227 - Use `gen_random_uuid()` for UUID primary keys.
228 - Always add `DEFAULT` values for `NOT NULL` columns.
229 - Create indexes after the table definition, in the same migration.
230 - Prefer additive migrations (add columns, add tables). Destructive changes (drop columns, rename tables) need careful planning.
231 - Name migrations descriptively: `NNN_what_it_does.sql`.
232 - **Indexes on growth tables must be `CONCURRENTLY`.** A plain `CREATE INDEX` takes an `ACCESS EXCLUSIVE` lock and blocks writes for the whole build: fine on a small table, a production write-stall on `transactions`/`page_views`/`subscriptions`/etc. `CREATE INDEX CONCURRENTLY` cannot run inside a transaction, so a migration that uses it **must start with the exact line `-- no-transaction`** (sqlx runs that file outside its per-migration transaction). Note the trade-off: a `-- no-transaction` migration is not atomic, so keep it to the single concurrent index build.
233
234 ```sql
235 -- no-transaction
236 CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_transactions_seller_created
237 ON transactions (seller_user_id, created_at DESC);
238 ```
239
240 The `migration_hygiene` test (`tests/migration_hygiene.rs`) enforces both rules (concurrent-on-growth-table and `IF NOT EXISTS`) for every migration past the frozen high-water mark (historical migrations can't change: sqlx checksums applied files). Bump `HIGH_WATER` there only after deliberately reviewing the migrations you're grandfathering.
241
242 ```sql
243 -- Example: 004_file_scan_status.sql
244 ALTER TABLE items ADD COLUMN scan_status TEXT NOT NULL DEFAULT 'pending';
245
246 CREATE TABLE file_scan_results (
247 id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
248 s3_key TEXT NOT NULL,
249 scan_status TEXT NOT NULL,
250 scanned_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
251 );
252
253 CREATE INDEX idx_file_scan_results_s3_key ON file_scan_results(s3_key);
254 ```
255
256 ## Templates
257
258 Askama templates (Jinja2-like syntax) live in `server/templates/`. Template structs (Rust side) live in `server/src/templates/`.
259
260 ### Layout Inheritance
261
262 All full pages extend `base.html`:
263 ```html
264 {% extends "base.html" %}
265 {% block title %}Page Title{% endblock %}
266 {% block content %}
267 {% include "partials/site_header.html" %}
268 <!-- page content -->
269 {% endblock %}
270 ```
271
272 Blocks available in `base.html`: `title`, `meta_description`, `head` (extra CSS/meta), `body_attrs`, `content`, `scripts`.
273
274 ### HTMX Partials
275
276 HTMX fragment templates do NOT extend `base.html`. They render standalone HTML fragments:
277 ```html
278 {# No extends. This is a partial #}
279 {% for item in items %}
280 <div class="item-row">{{ item.title }}</div>
281 {% endfor %}
282 ```
283
284 ### Template Variables
285
286 Every full-page template needs at minimum:
287 - `csrf_token: Option<String>`, for the CSRF meta tag
288 - `session_user: Option<SessionUser>`, for the header (login state, avatar)
289
290 ## Frontend Performance
291
292 These rules apply to all HTML templates. The goal is zero layout shift, no wasted pixels, and instant-feeling interactions.
293
294 ### Images
295
296 Every `<img>` must have explicit dimensions to prevent layout shift:
297 ```html
298 <!-- Inline style with both width and height -->
299 <img src="{{ url }}" alt="{{ title }}"
300 style="width: 120px; height: 120px; object-fit: cover;">
301
302 <!-- Or use aspect-ratio for responsive images -->
303 <img src="{{ url }}" alt="{{ title }}"
304 style="width: 100%; aspect-ratio: 1 / 1; object-fit: cover;">
305 ```
306
307 Never use `loading="lazy"` for images that may appear above the fold (the first visible screen before scrolling).
308
309 ### Navigation and Reloads
310
311 Never use `window.location.reload()` when an HTMX partial swap can do the job. Full-page reloads discard all client state, re-parse all JS/CSS, and feel slow.
312
313 ```html
314 <!-- Bad: full-page reload after action -->
315 <button hx-put="/api/items/{{ id }}"
316 hx-on::after-request="if(event.detail.successful) window.location.reload()">
317
318 <!-- Good: re-fetch the relevant tab/section -->
319 <button hx-put="/api/items/{{ id }}"
320 hx-on::after-request="if(event.detail.successful) document.getElementById('tab-settings').click()">
321 ```
322
323 Legitimate uses of `window.location.href`:
324 - Navigating to a different page entirely (login, checkout redirect, file download)
325 - After destructive account actions (delete account → login page)
326
327 ### Loading States
328
329 Prefer server-complete responses over client-side loading placeholders. The server should wait for data and send a complete fragment in one paint, rather than sending a skeleton and filling it in later.
330
331 Use `hx-trigger="revealed"` with a loading placeholder only when the data is expensive to fetch AND hidden behind a `<details>` element (e.g., 2FA status, passkey list). Never use loading placeholders for content that's visible on initial tab load.
332
333 ### Static JavaScript
334
335 JavaScript lives in `server/static/`, one file per feature area:
336
337 ```
338 static/
339 mnw.js core utilities (CSRF, toasts, tabs, shortcuts), loaded globally
340 upload.js S3 upload (S3Uploader, initDropzone), loaded globally
341 passkey.js WebAuthn registration/login
342 insertions.js clip management
343 wizard.js wizard navigation
344 docs-search.js doc search index
345 item-details.js bundle, section, tag management
346 item-upload.js audio + version upload flows
347 blog-editor.js blog save/autosave/publish
348 style.css main stylesheet
349 wizard.css wizard-specific styles
350 ```
351
352 Only `mnw.js`, `upload.js`, and `htmx.min.js` are loaded globally (in `base.html` / `_head_assets.html`). All other JS files are loaded via `{% block scripts %}` in the page that needs them.
353
354 **Passing server data to static JS:** Use `data-*` attributes on the feature's container element. The JS file reads them on init.
355
356 ```html
357 <!-- In template -->
358 <div id="audio-upload" data-item-id="{{ item.id }}">
359 <!-- upload UI -->
360 </div>
361
362 <!-- In {% block scripts %} -->
363 <script src="/static/item-upload.js"></script>
364 ```
365
366 ```javascript
367 // static/item-upload.js
368 (function() {
369 var el = document.getElementById('audio-upload');
370 if (!el) return;
371 var itemId = el.dataset.itemId;
372 // ...
373 })();
374 ```
375
376 For complex structured data (JSON arrays/objects that can't fit in an attribute), use a minimal inline script:
377
378 ```html
379 <script>window.MNW = window.MNW || {}; window.MNW.pageData = { segments: {{ segments_json|safe }} };</script>
380 <script src="/static/audio-player.js"></script>
381 ```
382
383 **HTMX partial re-initialization:** Static JS loaded in `{% block scripts %}` runs once on page load. For HTMX partials (tab content swapped dynamically), use `htmx:afterSwap` to re-initialize:
384
385 ```javascript
386 document.body.addEventListener('htmx:afterSwap', function(e) {
387 if (e.detail.target.id === 'tab-content') {
388 initMyFeature();
389 }
390 });
391 ```
392
393 **When inline is OK:** Under 20 lines, no template variables that could be data attributes, and tightly coupled to a single template's DOM structure (e.g., tab overflow close handler, single-use form validation).
394
395 ### Inline CSS
396
397 Page-specific `<style>` blocks in `{% block head %}` are acceptable when the styles are truly unique to that page. Shared patterns (form layouts, tables, status badges, cards) belong in `style.css`.
398
399 ### Information Density
400
401 Show more data in less space. Prefer tight rows over hero cards. Let users compare by scanning, not by clicking into detail pages. A page that shows 15 items is more useful than one that shows 3.
402
403 Use typography (weight, size) and whitespace for hierarchy instead of borders and background colors. A 16px gap groups elements as well as a 1px border, with less visual noise.
404
405 ## CSRF Protection
406
407 The CSRF middleware (`src/csrf.rs`) validates all state-changing requests (POST, PUT, PATCH, DELETE) except exempted paths (webhooks, auth endpoints, OAuth).
408
409 **For HTMX requests:** The CSRF token is sent via the `X-CSRF-Token` header, read from the `<meta name="csrf-token">` tag by client-side JS.
410
411 **For vanilla forms:** Include a hidden `_csrf` field:
412 ```html
413 <input type="hidden" name="_csrf" value="{{ csrf_token.as_deref().unwrap_or_default() }}">
414 ```
415
416 Token comparison uses constant-time comparison to prevent timing attacks.
417
418 ## Tracing
419
420 Every handler and significant function gets `#[tracing::instrument(skip_all, name = "...")]`. The `name` parameter uses the pattern `module::function_name` (e.g., `"pages::blog_post_page"`, `"admin::approve_user"`).
421
422 `skip_all` prevents large structs (State, Session, request bodies) from being serialized into spans. Add specific fields if needed:
423 ```rust
424 #[tracing::instrument(skip_all, fields(user_id = %user.id))]
425 ```
426
427 Internal errors are logged at `error!` level. Security events (failed CSRF, malware detection) at `warn!`.
428
429 ## Testing
430
431 ### Unit Tests
432
433 In-file `#[cfg(test)]` modules. No database needed.
434
435 ```rust
436 #[cfg(test)]
437 mod tests {
438 use super::*;
439
440 #[test]
441 fn status_code_not_found() {
442 assert_eq!(AppError::NotFound.status_code(), StatusCode::NOT_FOUND);
443 }
444 }
445 ```
446
447 ### Integration Tests
448
449 Each integration test creates and drops its own PostgreSQL database. Requires `TEST_DATABASE_URL` pointing to a PostgreSQL instance with `CREATE DATABASE` permission.
450
451 ```bash
452 TEST_DATABASE_URL="postgres:///postgres" cargo test --test integration
453 ```
454
455 On Astra, use `--test-threads=8` (or the `RUST_TEST_THREADS=8` env var) to avoid overwhelming PostgreSQL with concurrent database creation.
456
457 ### Seal Tests
458
459 `assumptions.rs`, `migration_hygiene.rs`, `frontend_globals.rs`, `test_hygiene.rs`, and `workflows/enum_drift.rs` are ratchets: they read the repo (the TOML corpus, the migration set, `static/*.js`, the test suite itself, the live schema) and fail when a convention is broken or a `HIGH_WATER` count rises. Add one whenever a rule is worth enforcing but too tedious to catch in review, and write the failure message so it names the number to set.
460
461 `test_hygiene.rs` enforces the conventions below. Two of its rules are hard (every workflow module has a `//!` header; every `#[ignore]` carries a reason) and three are ratchets over counts: loose status assertions, `test_`-prefixed names, and modules over 800 lines. Lower a `HIGH_WATER` when you clean a file up; the failure message names the number.
462
463 The loose-status count went 830 to 6 on 2026-08-06, so that one is close to a hard rule now. The method is worth reusing on the other two: rather than reason about what each site should assert, a temporary `#[track_caller]` probe stood in for the predicate, the suite ran once, and every site whose observations agreed on a single status was pinned to it. Measuring beats guessing at that volume, and the sites left over are exactly the ones that needed a human.
464
465 ### Conventions
466
467 - **Name the behavior, not the function.** `lookups_return_none_when_absent`, not `test_lookup`. No `test_` prefix; the attribute already says it.
468 - **Assert the exact status code.** `assert_eq!(resp.status, 403)` pins the contract. `is_client_error()` also passes on the 404 you get when the route silently disappears. Use the loose form only where the code genuinely is not contracted.
469 - **Carry the body in the failure message**: `assert_eq!(resp.status, 200, "publish should succeed: {} {}", resp.status, resp.text)`.
470 - **Shared setup lives in `tests/harness/`.** A per-file helper is fine when it is a thin wrapper over harness calls; the moment a second file wants it, promote it rather than copy it. One helper name means one return shape.
471 - **Return named structs past two fields** (`CreatorSetup` is the model), not long tuples.
472 - **Never sleep to wait for background work.** Drain it deterministically (`drain_scan_jobs`, `drain_s3_deletions`).
473 - **`#[ignore]` needs a reason string** saying how to run the test instead.
474 - Group integration tests by feature domain, one module per domain, each opening with a `//!` comment on what surface it covers. Split a module before it passes ~800 lines.
475
476 ## Rust Edition and Style
477
478 - **Rust 2024 edition** (Rust 1.85+). Uses `gen` keyword restrictions and other 2024 features.
479 - No `.unwrap()` in production code. Use `?`, `.ok_or()`, or `unwrap_or_default()`.
480 - Prefer `Option::and_then`/`map` over `if let Some`/`match` for simple transforms.
481 - File size guideline per root `CONTRIBUTING.md`: 500-line limit on branching logic, flat lists exempt. Route files follow the same rule. Split into directory modules when they grow beyond 500 lines.
482
483 ## Dependencies
484
485 Always use the latest stable release of every dependency. When upgrading introduces breaking API changes, update the code. Never pin old versions to avoid migration work.
486
487 ## Deployment
488
489 Deploys go through Sando, the pipeline controller (`sandod`, on fw13), which
490 builds the server natively, runs the gate tiers, and swaps the release on the
491 prod host. Never deploy by hand: no `deploy.sh`, no `scp`, no building on the
492 prod box. The operator procedure lives in `../sando/deploy/README.md`.
493
494 The version is read from `Cargo.toml` and compiled into the binary via
495 `env!("CARGO_PKG_VERSION")` for Sentry release strings. Bump the version in
496 `Cargo.toml` before every production deploy.
497
498 `server/deploy/archive/deploy.sh.legacy` is a retained cutover reference, not a
499 supported path.
500