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## Middleware Patterns
185
186### Middleware Composition
187
188**CRITICAL: Authentication must precede authorization checks.**
189
190When using multiple middleware functions, order matters:
191
192```typescript
193// ✅ CORRECT: requireAuth runs first, sets c.get("user")
194app.post(
195 "/api/topics",
196 requireAuth(ctx), // Step 1: Restore session, set user
197 requirePermission(ctx, "createTopics"), // Step 2: Check permission
198 async (c) => {
199 const user = c.get("user")!; // Safe - guaranteed by middleware chain
200 // ... handler logic
201 }
202);
203
204// ❌ WRONG: requirePermission runs first, user is undefined
205app.post(
206 "/api/topics",
207 requirePermission(ctx, "createTopics"), // user not set yet!
208 requireAuth(ctx),
209 async (c) => { /* ... */ }
210);
211```
212
213**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.
214
215**Testing middleware composition:**
216```typescript
217it("middleware chain executes in correct order", async () => {
218 // Verify requireAuth sets user before requirePermission checks it
219 const res = await app.request("/api/topics", {
220 method: "POST",
221 headers: { Cookie: "atbb_session=valid_token" }
222 });
223
224 // Should succeed if both middlewares run in order
225 expect(res.status).not.toBe(401);
226});
227```
228
229## Error Handling Standards
230
231Follow these patterns for robust, debuggable production code:
232
233### API Route Handlers
234
235**Required for all database-backed endpoints:**
2361. Validate input parameters before database queries (return 400 for invalid input)
2372. Wrap database queries in try-catch with structured logging
2383. Check resource existence explicitly (return 404 for missing resources)
2394. Return proper HTTP status codes (400/404/500, not always 500)
240
241**Example pattern:**
242```typescript
243export function createForumRoutes(ctx: AppContext) {
244 return new Hono().get("/", async (c) => {
245 try {
246 const [forum] = await ctx.db
247 .select()
248 .from(forums)
249 .where(eq(forums.rkey, "self"))
250 .limit(1);
251
252 if (!forum) {
253 return c.json({ error: "Forum not found" }, 404);
254 }
255
256 return c.json({ /* success response */ });
257 } catch (error) {
258 console.error("Failed to query forum metadata", {
259 operation: "GET /api/forum",
260 error: error instanceof Error ? error.message : String(error),
261 });
262 return c.json(
263 { error: "Failed to retrieve forum metadata. Please try again later." },
264 500
265 );
266 }
267 });
268}
269```
270
271### Catch Block Guidelines
272
273**DO:**
274- Catch specific error types when possible (`instanceof RangeError`, `instanceof SyntaxError`)
275- Re-throw unexpected errors (don't swallow programming bugs like `TypeError`)
276- Log with structured context: operation name, relevant IDs, error message
277- Return user-friendly messages (no stack traces in production)
278- Classify errors by user action (400, 503) vs server bugs (500)
279
280**DON'T:**
281- Use bare `catch` blocks that hide all error types
282- Return generic "try again later" for client errors (400) vs server errors (500)
283- Fabricate data in catch blocks (return null or fail explicitly)
284- Use empty catch blocks or catch without logging
285- 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
286
287**Try Block Granularity:**
288
289When 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:
290
291```typescript
292// ❌ BAD: DB re-query failure reports "Failed to create category" even though
293// the PDS write already succeeded — misleading for operators debugging
294try {
295 const result = await createCategory(...); // PDS write succeeded
296 categoryUri = result.uri;
297 const [cat] = await db.select()...; // DB re-query fails here
298} catch (error) {
299 consola.error("Failed to create category:", ...); // Inaccurate!
300}
301
302// ✅ GOOD: each operation has its own try block and specific error message
303try {
304 const result = await createCategory(...);
305 categoryUri = result.uri;
306} catch (error) {
307 consola.error("Failed to create category:", ...);
308}
309
310try {
311 const [cat] = await db.select()...;
312} catch (error) {
313 consola.error("Failed to look up category ID after creation:", ...);
314}
315```
316
317**Programming Error Re-Throwing Pattern:**
318
319```typescript
320// ✅ CORRECT: Re-throw programming errors, catch runtime errors
321try {
322 const result = await ctx.db.select()...;
323 return processResult(result);
324} catch (error) {
325 // Re-throw programming errors (code bugs) - don't hide them
326 if (error instanceof TypeError ||
327 error instanceof ReferenceError ||
328 error instanceof SyntaxError) {
329 console.error("CRITICAL: Programming error detected", {
330 error: error.message,
331 stack: error.stack,
332 operation: "checkPermission"
333 });
334 throw error; // Let global error handler catch it
335 }
336
337 // Log and handle expected runtime errors (DB failures, network issues)
338 console.error("Database query failed", {
339 operation: "checkPermission",
340 error: error instanceof Error ? error.message : String(error)
341 });
342 return null; // Fail safely for business logic
343}
344```
345
346**Why re-throw programming errors:**
347- `TypeError` = code bug (e.g., `role.permisions.includes()` typo)
348- `ReferenceError` = code bug (e.g., using undefined variable)
349- `SyntaxError` = code bug (e.g., malformed JSON.parse in your code)
350- These should crash during development, not be silently logged
351- Catching them hides bugs and makes debugging impossible
352
353**Error Classification Helper:**
354
355Create helper functions to classify errors consistently:
356
357```typescript
358// File: src/lib/errors.ts
359export function isProgrammingError(error: unknown): boolean {
360 return error instanceof TypeError ||
361 error instanceof ReferenceError ||
362 error instanceof SyntaxError;
363}
364
365export function isNetworkError(error: unknown): boolean {
366 if (!(error instanceof Error)) return false;
367 const msg = error.message.toLowerCase();
368 return msg.includes("fetch failed") ||
369 msg.includes("econnrefused") ||
370 msg.includes("enotfound") ||
371 msg.includes("timeout");
372}
373
374// Usage in route handlers:
375} catch (error) {
376 if (isProgrammingError(error)) {
377 throw error; // Don't catch programming bugs
378 }
379
380 if (isNetworkError(error)) {
381 return c.json({
382 error: "Unable to reach external service. Please try again later."
383 }, 503); // User should retry
384 }
385
386 return c.json({
387 error: "An unexpected error occurred. Please contact support."
388 }, 500); // Server bug, needs investigation
389}
390```
391
392### Helper Functions
393
394**Validation helpers should:**
395- Return `null` for invalid input (not throw)
396- Re-throw unexpected errors
397- Use specific error type checking
398
399**Example:**
400```typescript
401export function parseBigIntParam(value: string): bigint | null {
402 try {
403 return BigInt(value);
404 } catch (error) {
405 if (error instanceof RangeError || error instanceof SyntaxError) {
406 return null; // Expected error for invalid input
407 }
408 throw error; // Unexpected error - let it bubble up
409 }
410}
411```
412
413**Serialization helpers should:**
414- Avoid silent fallbacks (log warnings if fabricating data)
415- Prefer returning `null` over fake values (`"0"`, `new Date()`)
416- Document fallback behavior in JSDoc if unavoidable
417
418## Security-Critical Code Standards
419
420When implementing authentication, authorization, or permission systems, follow these additional requirements:
421
422### 1. Fail-Closed Security
423
424**Security checks must deny access by default when encountering errors.**
425
426```typescript
427// ✅ CORRECT: Fail closed - deny access on any error
428export async function checkPermission(
429 ctx: AppContext,
430 did: string,
431 permission: string
432): Promise<boolean> {
433 try {
434 const [membership] = await ctx.db.select()...;
435 if (!membership || !membership.roleUri) {
436 return false; // No membership = deny access
437 }
438
439 const [role] = await ctx.db.select()...;
440 if (!role) {
441 return false; // Role deleted = deny access
442 }
443
444 // Permissions live in role_permissions join table, not role.permissions array
445 const [match] = await ctx.db
446 .select()
447 .from(rolePermissions)
448 .where(and(
449 eq(rolePermissions.roleId, role.id),
450 or(
451 eq(rolePermissions.permission, permission),
452 eq(rolePermissions.permission, "*")
453 )
454 ))
455 .limit(1);
456
457 return !!match;
458 } catch (error) {
459 if (isProgrammingError(error)) throw error;
460
461 console.error("Failed to check permissions - denying access", {
462 did, permission, error: String(error)
463 });
464 return false; // Error = deny access (fail closed)
465 }
466}
467
468// ❌ WRONG: Fail open - grants access on error
469} catch (error) {
470 console.error("Permission check failed");
471 return true; // SECURITY BUG: grants access on DB error!
472}
473```
474
475**Test fail-closed behavior:**
476```typescript
477it("denies access when database query fails (fail closed)", async () => {
478 vi.spyOn(ctx.db, "select").mockRejectedValueOnce(new Error("DB connection lost"));
479
480 const result = await checkPermission(ctx, "did:plc:user", "createTopics");
481
482 expect(result).toBe(false); // Prove fail-closed behavior
483 expect(console.error).toHaveBeenCalledWith(
484 expect.stringContaining("denying access"),
485 expect.any(Object)
486 );
487});
488```
489
490### 2. Startup Failures for Missing Security Infrastructure
491
492**Security-critical infrastructure must fail fast on startup, not at first request.**
493
494```typescript
495// ✅ CORRECT: Throw error on startup if ForumAgent unavailable
496export async function seedDefaultRoles(ctx: AppContext) {
497 const agent = ctx.forumAgent;
498 if (!agent) {
499 console.error("CRITICAL: ForumAgent not available - role system non-functional", {
500 operation: "seedDefaultRoles",
501 forumDid: ctx.config.forumDid
502 });
503 throw new Error(
504 "Cannot seed roles without ForumAgent - permission system would be broken"
505 );
506 }
507 // ... seeding logic
508}
509
510// ❌ WRONG: Silent failure allows server to start without roles
511if (!agent) {
512 console.warn("ForumAgent not available, skipping role seeding");
513 return { created: 0, skipped: 0 }; // Server starts but is broken!
514}
515```
516
517**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.
518
519## Documentation & Project Tracking
520
521**Keep these synchronized when completing work:**
522
5231. **`docs/atproto-forum-plan.md`** — Master project plan with phase checklist
524 - Mark items complete `[x]` when implementation is done and tested
525 - Add brief status notes with file references and Linear issue IDs
526 - Update immediately after completing milestones
527
5282. **Linear issues** — Task tracker at https://linear.app/atbb
529 - Update status: Backlog → In Progress → Done
530 - Add comments documenting implementation details when marking Done
531 - Keep status in sync with actual codebase state, not planning estimates
532
5333. **`docs/plans/` convention:**
534 - Active/in-progress plan documents go directly in `docs/plans/`
535 - Completed plan documents move to `docs/plans/complete/` when work is shipped
536
5374. **Workflow:** When finishing a task:
538 ```sh
539 # 1. Run tests to verify implementation is correct
540 pnpm test
541
542 # 2. If tests pass, commit your changes
543 git add .
544 git commit -m "feat: your changes"
545
546 # 3. Update plan document: mark [x] and add completion note
547 # 4. Update Linear: change status to Done, add implementation comment
548 # 5. Push and request code review
549 # 6. After review approval: include "docs:" prefix when committing plan updates
550 ```
551
552**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.
553
554## Bruno API Collections
555
556**CRITICAL: Update Bruno collections in the SAME commit as route implementation.**
557
558The `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.
559
560**Common Mistakes:**
561
562**DON'T:**
563- Commit API changes without updating Bruno collections
564- Use hardcoded URLs instead of environment variables (`{{appview_url}}`)
565- Skip documenting error cases (only document 200 responses)
566- Leave placeholder/example data that doesn't match actual API behavior
567- Forget to update assertions when response format changes
568
569**DO:**
570- Update Bruno files in the same commit as route implementation
571- Use environment variables for all URLs and test data
572- Document all HTTP status codes the endpoint can return
573
574## Git Conventions
575
576- Do not include `Co-Authored-By` lines in commit messages.
577- `prior-art/` contains git submodules (Rust AppView, original lexicons, delegation spec) — reference material only, not used at build time.
578- Worktrees with submodules need `submodule deinit --all -f` then `worktree remove --force` to clean up.