malpercio.dev
WIP! A BB-style forum, on the ATmosphere!
We're still working... we'll be back soon when we have something to show off!
node
typescript
hono
htmx
atproto
1# Claude.md
2
3The role of this file is to describe common mistakes and confusion points that agents might encounter as they work in this project. If you ever encounter something in the project that surprises you, please alert the developer working with you and indicate that this is the case in the Claude.md file to help prevent future agents from having the same issue.
4
5This project is greenfield. It is okay to change the schema entirely or make what might be breaking changes. We will sort out any backfill or backwards compatibility requirements when they are actually needed.
6
7## Development
8
9### Setup
10
11```sh
12devenv shell # enter Nix dev shell (Node.js, pnpm, turbo)
13pnpm install # install all workspace dependencies
14cp .env.example .env # configure environment variables
15```
16
17### Nix Package Hash (`pnpmDeps`)
18
19`nix/package.nix` contains a `hash =` for `pnpm_9.fetchDeps`. This hash must be updated every time `pnpm-lock.yaml` changes.
20
21**Why it changes:** `pnpm_9.fetchDeps` is a Nix fixed-output derivation (FOD) that downloads packages via pnpm in a sandbox. When the lockfile changes, the set of downloaded packages changes, so the hash changes. Platform-specific optional packages (e.g. `@esbuild/linux-arm64`) mean macOS and Linux produce different hashes — the hash in the repo must match what the Linux build produces.
22
23**How to update it:** Run the helper script **on a Linux machine** (or with a Linux remote Nix builder configured):
24
25```sh
26pnpm nix:update-hash
27# or directly:
28bash scripts/update-pnpmdeps-hash.sh
29```
30
31The script:
321. Replaces the current hash with `lib.fakeHash` (a known-invalid placeholder)
332. Runs `nix build .#packages.x86_64-linux.default` — which fails with the correct hash in the error
343. Parses that hash and writes it back to `nix/package.nix`
35
36**On macOS without a Linux builder:** Push your branch to CI, copy the correct hash from the build failure output, and update `nix/package.nix` manually.
37
38### Auto-Fixing Lint Issues
39
40Before committing, auto-fix safe lint violations:
41
42```sh
43# Fix all packages
44pnpm turbo lint:fix
45
46# Fix specific package
47pnpm --filter @atbb/appview lint:fix
48```
49
50## Testing Standards
51
52**CRITICAL: Always run tests before committing code or requesting code review.**
53
54### Environment Variables in Tests
55
56**CRITICAL**: Turbo blocks environment variables by default for cache safety. Tests requiring env vars must declare them in `turbo.json`:
57
58```json
59{
60 "tasks": {
61 "test": {
62 "dependsOn": ["^build"],
63 "env": ["DATABASE_URL"]
64 }
65 }
66}
67```
68
69**Symptoms of missing declaration:**
70- Tests pass when run directly (`pnpm --filter @atbb/appview test`)
71- Tests fail when run via Turbo (`pnpm test`) with undefined env vars
72- CI fails even though env vars are set in workflow
73- Database errors like `database "username" does not exist` (postgres defaults to system username when DATABASE_URL is unset)
74
75**Why this matters:** Turbo's caching requires deterministic inputs. Environment variables that leak into tasks without declaration would make cache hits unpredictable. By explicitly declaring env vars in `turbo.json`, you tell Turbo to include them in the task's input hash and pass them through to the test process.
76
77**When adding new env vars to tests:** Update `turbo.json` immediately, or tests will mysteriously fail when run via Turbo but pass when run directly.
78
79**Placeholder tests are prohibited:**
80```typescript
81// ❌ FORBIDDEN: Stub tests provide false confidence
82it("assigns role successfully when admin has authority", async () => {
83 expect(true).toBe(true); // NOT A REAL TEST
84});
85
86// ✅ REQUIRED: Real tests with actual assertions
87it("assigns role successfully when admin has authority", async () => {
88 const admin = await createUser(ctx, "Admin");
89 const member = await createUser(ctx, "Member");
90 const moderatorRole = await createRole(ctx, "Moderator", [], 20);
91
92 const res = await app.request(`/api/admin/members/${member.did}/role`, {
93 method: "POST",
94 headers: authHeaders(admin),
95 body: JSON.stringify({ roleUri: moderatorRole.uri })
96 });
97
98 expect(res.status).toBe(200);
99 const data = await res.json();
100 expect(data.roleAssigned).toBe("Moderator");
101
102 // Verify database state changed
103 const updatedMember = await getMembership(ctx, member.did);
104 expect(updatedMember.roleUri).toBe(moderatorRole.uri);
105});
106```
107
108**Skipped tests (`test.skip`, `it.skip`) must have a Linear issue tracking why.** Skipping without a tracked reason is prohibited.
109
110### Before Requesting Code Review
111
112**CRITICAL: Run this checklist before requesting review to catch issues early:**
113
114```sh
115# 1. Verify all tests pass
116pnpm test
117
118# 2. Check runtime dependencies are correctly placed
119# (Runtime imports must be in dependencies, not devDependencies)
120grep -r "from 'drizzle-orm'" apps/*/src # If found, verify in dependencies
121grep -r "from 'postgres'" apps/*/src # If found, verify in dependencies
122
123# 3. Verify error test coverage is comprehensive
124# For API endpoints, ensure you have tests for:
125# - Input validation (missing fields, wrong types, malformed JSON)
126# - Error classification (network→503, server→500)
127# - Error message clarity (user-friendly, no stack traces)
128```
129
130**Common mistake:** Adding error tests AFTER review feedback instead of DURING implementation. Write error tests immediately after implementing the happy path — they often reveal bugs in error classification and input validation that are better caught before review.
131
132## Lexicon Conventions
133
134- **Source of truth is YAML** in `packages/lexicon/lexicons/`. Never edit generated JSON or TypeScript.
135- **Build pipeline:** YAML → JSON (`scripts/build.ts`) → TypeScript (`@atproto/lex-cli gen-api`).
136- **Adding a new lexicon:** Create a `.yaml` file under `lexicons/space/atbb/`, run `pnpm --filter @atbb/lexicon build`.
137- **Record keys:** Use `key: tid` for collections (multiple records per repo). Use `key: literal:self` for singletons.
138- **References:** Use `com.atproto.repo.strongRef` wrapped in named defs (e.g., `forumRef`, `subjectRef`).
139- **Extensible fields:** Use `knownValues` (not `enum`) for strings that may grow (permissions, reaction types, mod actions).
140- **Record ownership:**
141 - Forum DID owns: `forum.forum`, `forum.category`, `forum.board`, `forum.role`, `forum.theme`, `forum.themePolicy`, `modAction`
142 - User DID owns: `post`, `membership`, `reaction`
143
144## AT Protocol Conventions
145
146- **Unified post model:** There is no separate "topic" type. A `space.atbb.post` without a `reply` ref is a topic starter; one with a `reply` ref is a reply.
147- **Reply chains:** `replyRef` has both `root` (thread starter) and `parent` (direct parent) — same pattern as `app.bsky.feed.post`.
148- **MVP trust model:** The AppView holds the Forum DID's signing keys directly and writes forum-level records on behalf of admins/mods after verifying their role. This will be replaced by AT Protocol privilege delegation post-MVP.
149
150## TypeScript / Hono Gotchas
151
152- **`@types/node` is required** as a devDependency in every package that uses `process.env` or other Node APIs. `tsx` doesn't need it at runtime, but `tsc` builds will fail without it.
153- **Hono JSX `children`:** Use `PropsWithChildren<T>` from `hono/jsx` for components that accept children. Unlike React, Hono's `FC<T>` does not include `children` implicitly.
154- **HTMX attributes in JSX:** The `typed-htmx` package provides types for `hx-*` attributes. See `apps/web/src/global.d.ts` for the augmentation.
155- **Glob expansion in npm scripts:** `@atproto/lex-cli` needs file paths, not globs. Use `bash -c 'shopt -s globstar && ...'` to expand `**/*.json` in npm scripts.
156- **`.env` loading:** Dev scripts use Node's `--env-file=../../.env` flag to load the root `.env` file. No `dotenv` dependency needed.
157- **API endpoint parameter type guards:** Never trust TypeScript types for user input. Change handler parameter types from `string` to `unknown` and add explicit `typeof` checks. TypeScript types are erased at runtime — a request missing the `text` field will pass type checking but crash with `TypeError: text.trim is not a function`.
158 ```typescript
159 // ❌ BAD: Assumes text is always a string at runtime
160 export function validatePostText(text: string): { valid: boolean } {
161 const trimmed = text.trim(); // Crashes if text is undefined!
162 // ...
163 }
164
165 // ✅ GOOD: Type guard protects against runtime type mismatches
166 export function validatePostText(text: unknown): { valid: boolean } {
167 if (typeof text !== "string") {
168 return { valid: false, error: "Text is required and must be a string" };
169 }
170 const trimmed = text.trim(); // Safe - text is proven to be a string
171 // ...
172 }
173 ```
174- **Hono JSON parsing safety:** `await c.req.json()` throws `SyntaxError` for malformed JSON. Always wrap in try-catch and return 400 for client errors:
175 ```typescript
176 let body: any;
177 try {
178 body = await c.req.json();
179 } catch {
180 return c.json({ error: "Invalid JSON in request body" }, 400);
181 }
182 ```
183
184## Theme Resolution & Cookie Protocol
185
186<!-- freshness: 2026-03-20 -->
187
188The web app resolves which theme to render via a 5-step waterfall in `apps/web/src/lib/theme-resolution.ts`:
189
1901. **Detect color scheme** -- `atbb-color-scheme` cookie or `Sec-CH-Prefers-Color-Scheme` hint (default: `light`)
1912. **Fetch theme policy** -- `GET /api/theme-policy` from AppView (cached in-memory with TTL)
1923. **User preference** -- `atbb-light-theme` / `atbb-dark-theme` cookie (validated against `policy.availableThemes`; ignored when `policy.allowUserChoice` is false)
1934. **Forum default** -- `defaultLightThemeUri` / `defaultDarkThemeUri` from policy
1945. **Hardcoded fallback** -- neobrutal-light or neobrutal-dark bundled presets
195
196**Cookie contracts:**
197
198| Cookie | Written by | Contains | Max-Age |
199|--------|-----------|----------|---------|
200| `atbb-color-scheme` | Client-side JS (toggle button) | `light` or `dark` | 1 year |
201| `atbb-light-theme` | `POST /settings/appearance` | AT URI of chosen light theme | 1 year |
202| `atbb-dark-theme` | `POST /settings/appearance` | AT URI of chosen dark theme | 1 year |
203
204**Invariants:**
205- User preference cookies are always validated against the current `policy.availableThemes` on read. Stale or removed theme URIs silently fall through to the forum default.
206- `resolveTheme()` never throws -- it always returns a usable `ResolvedTheme`.
207- Settings routes (`/settings`, `/settings/appearance`) require authentication. The preview endpoint (`/settings/preview`) is unauthenticated — it returns only public theme data (color swatches). The preference form is only rendered when `policy.allowUserChoice` is true.
208- **Settings routes bypass ThemeCache intentionally.** `GET /settings` and `POST /settings/appearance` call `fetch()` directly (not via `resolveTheme()`), so they always get fresh policy data. This is by design: preference saves must validate against the current policy, not a potentially stale cached copy.
209
210## Middleware Patterns
211
212### Middleware Composition
213
214**CRITICAL: Authentication must precede authorization checks.**
215
216When using multiple middleware functions, order matters:
217
218```typescript
219// ✅ CORRECT: requireAuth runs first, sets c.get("user")
220app.post(
221 "/api/topics",
222 requireAuth(ctx), // Step 1: Restore session, set user
223 requirePermission(ctx, "createTopics"), // Step 2: Check permission
224 async (c) => {
225 const user = c.get("user")!; // Safe - guaranteed by middleware chain
226 // ... handler logic
227 }
228);
229
230// ❌ WRONG: requirePermission runs first, user is undefined
231app.post(
232 "/api/topics",
233 requirePermission(ctx, "createTopics"), // user not set yet!
234 requireAuth(ctx),
235 async (c) => { /* ... */ }
236);
237```
238
239**Why this matters:** `requirePermission` depends on `c.get("user")` being set by `requireAuth`. If authentication middleware doesn't run first, permission checks always fail with 401.
240
241**Testing middleware composition:**
242```typescript
243it("middleware chain executes in correct order", async () => {
244 // Verify requireAuth sets user before requirePermission checks it
245 const res = await app.request("/api/topics", {
246 method: "POST",
247 headers: { Cookie: "atbb_session=valid_token" }
248 });
249
250 // Should succeed if both middlewares run in order
251 expect(res.status).not.toBe(401);
252});
253```
254
255## Error Handling Standards
256
257Follow these patterns for robust, debuggable production code:
258
259### API Route Handlers
260
261**Required for all database-backed endpoints:**
2621. Validate input parameters before database queries (return 400 for invalid input)
2632. Wrap database queries in try-catch with structured logging
2643. Check resource existence explicitly (return 404 for missing resources)
2654. Return proper HTTP status codes (400/404/500, not always 500)
266
267**Example pattern:**
268```typescript
269export function createForumRoutes(ctx: AppContext) {
270 return new Hono().get("/", async (c) => {
271 try {
272 const [forum] = await ctx.db
273 .select()
274 .from(forums)
275 .where(eq(forums.rkey, "self"))
276 .limit(1);
277
278 if (!forum) {
279 return c.json({ error: "Forum not found" }, 404);
280 }
281
282 return c.json({ /* success response */ });
283 } catch (error) {
284 console.error("Failed to query forum metadata", {
285 operation: "GET /api/forum",
286 error: error instanceof Error ? error.message : String(error),
287 });
288 return c.json(
289 { error: "Failed to retrieve forum metadata. Please try again later." },
290 500
291 );
292 }
293 });
294}
295```
296
297### Catch Block Guidelines
298
299**DO:**
300- Catch specific error types when possible (`instanceof RangeError`, `instanceof SyntaxError`)
301- Re-throw unexpected errors (don't swallow programming bugs like `TypeError`)
302- Log with structured context: operation name, relevant IDs, error message
303- Return user-friendly messages (no stack traces in production)
304- Classify errors by user action (400, 503) vs server bugs (500)
305
306**DON'T:**
307- Use bare `catch` blocks that hide all error types
308- Return generic "try again later" for client errors (400) vs server errors (500)
309- Fabricate data in catch blocks (return null or fail explicitly)
310- Use empty catch blocks or catch without logging
311- Put two distinct operations in the same try block when they have different failure semantics — a failure in the second operation will report as failure of the first
312
313**Try Block Granularity:**
314
315When a try block covers multiple distinct operations in sequence, errors from later steps get reported with the wrong context. Split into separate try blocks when operations have meaningfully different failure messages:
316
317```typescript
318// ❌ BAD: DB re-query failure reports "Failed to create category" even though
319// the PDS write already succeeded — misleading for operators debugging
320try {
321 const result = await createCategory(...); // PDS write succeeded
322 categoryUri = result.uri;
323 const [cat] = await db.select()...; // DB re-query fails here
324} catch (error) {
325 consola.error("Failed to create category:", ...); // Inaccurate!
326}
327
328// ✅ GOOD: each operation has its own try block and specific error message
329try {
330 const result = await createCategory(...);
331 categoryUri = result.uri;
332} catch (error) {
333 consola.error("Failed to create category:", ...);
334}
335
336try {
337 const [cat] = await db.select()...;
338} catch (error) {
339 consola.error("Failed to look up category ID after creation:", ...);
340}
341```
342
343**Programming Error Re-Throwing Pattern:**
344
345```typescript
346// ✅ CORRECT: Re-throw programming errors, catch runtime errors
347try {
348 const result = await ctx.db.select()...;
349 return processResult(result);
350} catch (error) {
351 // Re-throw programming errors (code bugs) - don't hide them
352 if (error instanceof TypeError ||
353 error instanceof ReferenceError ||
354 error instanceof SyntaxError) {
355 console.error("CRITICAL: Programming error detected", {
356 error: error.message,
357 stack: error.stack,
358 operation: "checkPermission"
359 });
360 throw error; // Let global error handler catch it
361 }
362
363 // Log and handle expected runtime errors (DB failures, network issues)
364 console.error("Database query failed", {
365 operation: "checkPermission",
366 error: error instanceof Error ? error.message : String(error)
367 });
368 return null; // Fail safely for business logic
369}
370```
371
372**Why re-throw programming errors:**
373- `TypeError` = code bug (e.g., `role.permisions.includes()` typo)
374- `ReferenceError` = code bug (e.g., using undefined variable)
375- `SyntaxError` = code bug (e.g., malformed JSON.parse in your code)
376- These should crash during development, not be silently logged
377- Catching them hides bugs and makes debugging impossible
378
379**Error Classification Helper:**
380
381Create helper functions to classify errors consistently:
382
383```typescript
384// File: src/lib/errors.ts
385export function isProgrammingError(error: unknown): boolean {
386 return error instanceof TypeError ||
387 error instanceof ReferenceError ||
388 error instanceof SyntaxError;
389}
390
391export function isNetworkError(error: unknown): boolean {
392 if (!(error instanceof Error)) return false;
393 const msg = error.message.toLowerCase();
394 return msg.includes("fetch failed") ||
395 msg.includes("econnrefused") ||
396 msg.includes("enotfound") ||
397 msg.includes("timeout");
398}
399
400// Usage in route handlers:
401} catch (error) {
402 if (isProgrammingError(error)) {
403 throw error; // Don't catch programming bugs
404 }
405
406 if (isNetworkError(error)) {
407 return c.json({
408 error: "Unable to reach external service. Please try again later."
409 }, 503); // User should retry
410 }
411
412 return c.json({
413 error: "An unexpected error occurred. Please contact support."
414 }, 500); // Server bug, needs investigation
415}
416```
417
418### Helper Functions
419
420**Validation helpers should:**
421- Return `null` for invalid input (not throw)
422- Re-throw unexpected errors
423- Use specific error type checking
424
425**Example:**
426```typescript
427export function parseBigIntParam(value: string): bigint | null {
428 try {
429 return BigInt(value);
430 } catch (error) {
431 if (error instanceof RangeError || error instanceof SyntaxError) {
432 return null; // Expected error for invalid input
433 }
434 throw error; // Unexpected error - let it bubble up
435 }
436}
437```
438
439**Serialization helpers should:**
440- Avoid silent fallbacks (log warnings if fabricating data)
441- Prefer returning `null` over fake values (`"0"`, `new Date()`)
442- Document fallback behavior in JSDoc if unavoidable
443
444## Security-Critical Code Standards
445
446When implementing authentication, authorization, or permission systems, follow these additional requirements:
447
448### 1. Fail-Closed Security
449
450**Security checks must deny access by default when encountering errors.**
451
452```typescript
453// ✅ CORRECT: Fail closed - deny access on any error
454export async function checkPermission(
455 ctx: AppContext,
456 did: string,
457 permission: string
458): Promise<boolean> {
459 try {
460 const [membership] = await ctx.db.select()...;
461 if (!membership || !membership.roleUri) {
462 return false; // No membership = deny access
463 }
464
465 const [role] = await ctx.db.select()...;
466 if (!role) {
467 return false; // Role deleted = deny access
468 }
469
470 // Permissions live in role_permissions join table, not role.permissions array
471 const [match] = await ctx.db
472 .select()
473 .from(rolePermissions)
474 .where(and(
475 eq(rolePermissions.roleId, role.id),
476 or(
477 eq(rolePermissions.permission, permission),
478 eq(rolePermissions.permission, "*")
479 )
480 ))
481 .limit(1);
482
483 return !!match;
484 } catch (error) {
485 if (isProgrammingError(error)) throw error;
486
487 console.error("Failed to check permissions - denying access", {
488 did, permission, error: String(error)
489 });
490 return false; // Error = deny access (fail closed)
491 }
492}
493
494// ❌ WRONG: Fail open - grants access on error
495} catch (error) {
496 console.error("Permission check failed");
497 return true; // SECURITY BUG: grants access on DB error!
498}
499```
500
501**Test fail-closed behavior:**
502```typescript
503it("denies access when database query fails (fail closed)", async () => {
504 vi.spyOn(ctx.db, "select").mockRejectedValueOnce(new Error("DB connection lost"));
505
506 const result = await checkPermission(ctx, "did:plc:user", "createTopics");
507
508 expect(result).toBe(false); // Prove fail-closed behavior
509 expect(console.error).toHaveBeenCalledWith(
510 expect.stringContaining("denying access"),
511 expect.any(Object)
512 );
513});
514```
515
516### 2. Startup Failures for Missing Security Infrastructure
517
518**Security-critical infrastructure must fail fast on startup, not at first request.**
519
520```typescript
521// ✅ CORRECT: Throw error on startup if ForumAgent unavailable
522export async function seedDefaultRoles(ctx: AppContext) {
523 const agent = ctx.forumAgent;
524 if (!agent) {
525 console.error("CRITICAL: ForumAgent not available - role system non-functional", {
526 operation: "seedDefaultRoles",
527 forumDid: ctx.config.forumDid
528 });
529 throw new Error(
530 "Cannot seed roles without ForumAgent - permission system would be broken"
531 );
532 }
533 // ... seeding logic
534}
535
536// ❌ WRONG: Silent failure allows server to start without roles
537if (!agent) {
538 console.warn("ForumAgent not available, skipping role seeding");
539 return { created: 0, skipped: 0 }; // Server starts but is broken!
540}
541```
542
543**Why this matters:** If the permission system is broken, every request will fail authorization. It's better to fail startup loudly than silently deploy a non-functional system.
544
545## Documentation & Project Tracking
546
547**Keep these synchronized when completing work:**
548
5491. **`docs/atproto-forum-plan.md`** — Master project plan with phase checklist
550 - Mark items complete `[x]` when implementation is done and tested
551 - Add brief status notes with file references and Linear issue IDs
552 - Update immediately after completing milestones
553
5542. **Linear issues** — Task tracker at https://linear.app/atbb
555 - Update status: Backlog → In Progress → Done
556 - Add comments documenting implementation details when marking Done
557 - Keep status in sync with actual codebase state, not planning estimates
558
5593. **`docs/plans/` convention:**
560 - Active/in-progress plan documents go directly in `docs/plans/`
561 - Completed plan documents move to `docs/plans/complete/` when work is shipped
562
5634. **Workflow:** When finishing a task:
564 ```sh
565 # 1. Run tests to verify implementation is correct
566 pnpm test
567
568 # 2. If tests pass, commit your changes
569 git add .
570 git commit -m "feat: your changes"
571
572 # 3. Update plan document: mark [x] and add completion note
573 # 4. Update Linear: change status to Done, add implementation comment
574 # 5. Push and request code review
575 # 6. After review approval: include "docs:" prefix when committing plan updates
576 ```
577
578**Why this matters:** The plan document and Linear can drift from reality as code evolves. Regular synchronization prevents rediscovering completed work and ensures accurate project status.
579
580## Bruno API Collections
581
582**CRITICAL: Update Bruno collections in the SAME commit as route implementation.**
583
584The `bruno/` directory contains [Bruno](https://www.usebruno.com/) collections for API documentation and interactive testing. See `CONTRIBUTING.md` for the full update workflow and file template.
585
586**Common Mistakes:**
587
588**DON'T:**
589- Commit API changes without updating Bruno collections
590- Use hardcoded URLs instead of environment variables (`{{appview_url}}`)
591- Skip documenting error cases (only document 200 responses)
592- Leave placeholder/example data that doesn't match actual API behavior
593- Forget to update assertions when response format changes
594
595**DO:**
596- Update Bruno files in the same commit as route implementation
597- Use environment variables for all URLs and test data
598- Document all HTTP status codes the endpoint can return
599
600## Git Conventions
601
602- Do not include `Co-Authored-By` lines in commit messages.
603- `prior-art/` contains git submodules (Rust AppView, original lexicons, delegation spec) — reference material only, not used at build time.
604- Worktrees with submodules need `submodule deinit --all -f` then `worktree remove --force` to clean up.