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### Auto-Fixing Lint Issues
18
19Before committing, auto-fix safe lint violations:
20
21```sh
22# Fix all packages
23pnpm turbo lint:fix
24
25# Fix specific package
26pnpm --filter @atbb/appview lint:fix
27```
28
29## Testing Standards
30
31**CRITICAL: Always run tests before committing code or requesting code review.**
32
33### Environment Variables in Tests
34
35**CRITICAL**: Turbo blocks environment variables by default for cache safety. Tests requiring env vars must declare them in `turbo.json`:
36
37```json
38{
39 "tasks": {
40 "test": {
41 "dependsOn": ["^build"],
42 "env": ["DATABASE_URL"]
43 }
44 }
45}
46```
47
48**Symptoms of missing declaration:**
49- Tests pass when run directly (`pnpm --filter @atbb/appview test`)
50- Tests fail when run via Turbo (`pnpm test`) with undefined env vars
51- CI fails even though env vars are set in workflow
52- Database errors like `database "username" does not exist` (postgres defaults to system username when DATABASE_URL is unset)
53
54**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.
55
56**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.
57
58### When to Run Tests
59
60**Before every commit:**
61```sh
62pnpm test # Verify all tests pass
63git add .
64git commit -m "feat: your changes"
65```
66
67**Before requesting code review:**
68```sh
69pnpm build # Ensure clean build
70pnpm test # Verify all tests pass
71# Only then push and request review
72```
73
74**After fixing review feedback:**
75```sh
76# Make fixes
77pnpm test # Verify tests still pass
78# Push updates
79```
80
81### Test Requirements
82
83**All new features must include tests:**
84- API endpoints: Test success cases, error cases, edge cases
85- Business logic: Test all code paths and error conditions
86- Error handling: Test that errors are caught and logged appropriately
87- Security features: Test authentication, authorization, input validation
88
89**Test quality standards:**
90- Tests must be independent (no shared state between tests)
91- Use descriptive test names that explain what is being tested
92- Mock external dependencies (databases, APIs, network calls)
93- Test error paths, not just happy paths
94- Verify logging and error messages are correct
95
96**Red flags (do not commit):**
97- Skipped tests (`test.skip`, `it.skip`) without Linear issue tracking why
98- Tests that pass locally but fail in CI
99- Tests that require manual setup or specific data
100- Tests with hardcoded timing (`setTimeout`, `sleep`) - use proper mocks
101- Placeholder/stub tests that don't actually test anything
102
103**Placeholder tests are prohibited:**
104```typescript
105// ❌ FORBIDDEN: Stub tests provide false confidence
106it("assigns role successfully when admin has authority", async () => {
107 expect(true).toBe(true); // NOT A REAL TEST
108});
109
110// ✅ REQUIRED: Real tests with actual assertions
111it("assigns role successfully when admin has authority", async () => {
112 const admin = await createUser(ctx, "Admin");
113 const member = await createUser(ctx, "Member");
114 const moderatorRole = await createRole(ctx, "Moderator", [], 20);
115
116 const res = await app.request(`/api/admin/members/${member.did}/role`, {
117 method: "POST",
118 headers: authHeaders(admin),
119 body: JSON.stringify({ roleUri: moderatorRole.uri })
120 });
121
122 expect(res.status).toBe(200);
123 const data = await res.json();
124 expect(data.roleAssigned).toBe("Moderator");
125
126 // Verify database state changed
127 const updatedMember = await getMembership(ctx, member.did);
128 expect(updatedMember.roleUri).toBe(moderatorRole.uri);
129});
130```
131
132**Why this matters:** Stub tests pass in CI, creating false confidence that code is tested. They hide bugs that would be caught by real tests with actual assertions.
133
134**If you're unsure how to test something:** Leave a `// TODO: Add test for X` comment and create a Linear issue. Never commit a stub test that pretends to test something.
135
136
137### Test Coverage Expectations
138
139While we don't enforce strict coverage percentages, aim for:
140- **Critical paths:** 100% coverage (authentication, authorization, data integrity)
141- **Error handling:** All catch blocks should be tested
142- **API endpoints:** All routes should have tests
143- **Business logic:** All functions with branching logic should be tested
144
145**Do not:**
146- Skip writing tests to "move faster" - untested code breaks in production
147- Write tests after requesting review - tests inform implementation
148- Rely on manual testing alone - automated tests catch regressions
149
150### Before Requesting Code Review
151
152**CRITICAL: Run this checklist before requesting review to catch issues early:**
153
154```sh
155# 1. Verify all tests pass
156pnpm test
157
158# 2. Check runtime dependencies are correctly placed
159# (Runtime imports must be in dependencies, not devDependencies)
160grep -r "from 'drizzle-orm'" apps/*/src # If found, verify in dependencies
161grep -r "from 'postgres'" apps/*/src # If found, verify in dependencies
162
163# 3. Verify error test coverage is comprehensive
164# For API endpoints, ensure you have tests for:
165# - Input validation (missing fields, wrong types, malformed JSON)
166# - Error classification (network→503, server→500)
167# - Error message clarity (user-friendly, no stack traces)
168```
169
170**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.
171
172## Lexicon Conventions
173
174- **Source of truth is YAML** in `packages/lexicon/lexicons/`. Never edit generated JSON or TypeScript.
175- **Build pipeline:** YAML → JSON (`scripts/build.ts`) → TypeScript (`@atproto/lex-cli gen-api`).
176- **Adding a new lexicon:** Create a `.yaml` file under `lexicons/space/atbb/`, run `pnpm --filter @atbb/lexicon build`.
177- **Record keys:** Use `key: tid` for collections (multiple records per repo). Use `key: literal:self` for singletons.
178- **References:** Use `com.atproto.repo.strongRef` wrapped in named defs (e.g., `forumRef`, `subjectRef`).
179- **Extensible fields:** Use `knownValues` (not `enum`) for strings that may grow (permissions, reaction types, mod actions).
180- **Record ownership:**
181 - Forum DID owns: `forum.forum`, `forum.category`, `forum.role`, `modAction`
182 - User DID owns: `post`, `membership`, `reaction`
183
184## AT Protocol Conventions
185
186- **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.
187- **Reply chains:** `replyRef` has both `root` (thread starter) and `parent` (direct parent) — same pattern as `app.bsky.feed.post`.
188- **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.
189
190## TypeScript / Hono Gotchas
191
192- **`@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.
193- **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.
194- **HTMX attributes in JSX:** The `typed-htmx` package provides types for `hx-*` attributes. See `apps/web/src/global.d.ts` for the augmentation.
195- **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.
196- **`.env` loading:** Dev scripts use Node's `--env-file=../../.env` flag to load the root `.env` file. No `dotenv` dependency needed.
197- **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`.
198 ```typescript
199 // ❌ BAD: Assumes text is always a string at runtime
200 export function validatePostText(text: string): { valid: boolean } {
201 const trimmed = text.trim(); // Crashes if text is undefined!
202 // ...
203 }
204
205 // ✅ GOOD: Type guard protects against runtime type mismatches
206 export function validatePostText(text: unknown): { valid: boolean } {
207 if (typeof text !== "string") {
208 return { valid: false, error: "Text is required and must be a string" };
209 }
210 const trimmed = text.trim(); // Safe - text is proven to be a string
211 // ...
212 }
213 ```
214- **Hono JSON parsing safety:** `await c.req.json()` throws `SyntaxError` for malformed JSON. Always wrap in try-catch and return 400 for client errors:
215 ```typescript
216 let body: any;
217 try {
218 body = await c.req.json();
219 } catch {
220 return c.json({ error: "Invalid JSON in request body" }, 400);
221 }
222 ```
223
224## Middleware Patterns
225
226### Middleware Composition
227
228**CRITICAL: Authentication must precede authorization checks.**
229
230When using multiple middleware functions, order matters:
231
232```typescript
233// ✅ CORRECT: requireAuth runs first, sets c.get("user")
234app.post(
235 "/api/topics",
236 requireAuth(ctx), // Step 1: Restore session, set user
237 requirePermission(ctx, "createTopics"), // Step 2: Check permission
238 async (c) => {
239 const user = c.get("user")!; // Safe - guaranteed by middleware chain
240 // ... handler logic
241 }
242);
243
244// ❌ WRONG: requirePermission runs first, user is undefined
245app.post(
246 "/api/topics",
247 requirePermission(ctx, "createTopics"), // user not set yet!
248 requireAuth(ctx),
249 async (c) => { /* ... */ }
250);
251```
252
253**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.
254
255**Testing middleware composition:**
256```typescript
257it("middleware chain executes in correct order", async () => {
258 // Verify requireAuth sets user before requirePermission checks it
259 const res = await app.request("/api/topics", {
260 method: "POST",
261 headers: { Cookie: "atbb_session=valid_token" }
262 });
263
264 // Should succeed if both middlewares run in order
265 expect(res.status).not.toBe(401);
266});
267```
268
269## Error Handling Standards
270
271Follow these patterns for robust, debuggable production code:
272
273### API Route Handlers
274
275**Required for all database-backed endpoints:**
2761. Validate input parameters before database queries (return 400 for invalid input)
2772. Wrap database queries in try-catch with structured logging
2783. Check resource existence explicitly (return 404 for missing resources)
2794. Return proper HTTP status codes (400/404/500, not always 500)
280
281**Example pattern:**
282```typescript
283export function createForumRoutes(ctx: AppContext) {
284 return new Hono().get("/", async (c) => {
285 try {
286 const [forum] = await ctx.db
287 .select()
288 .from(forums)
289 .where(eq(forums.rkey, "self"))
290 .limit(1);
291
292 if (!forum) {
293 return c.json({ error: "Forum not found" }, 404);
294 }
295
296 return c.json({ /* success response */ });
297 } catch (error) {
298 console.error("Failed to query forum metadata", {
299 operation: "GET /api/forum",
300 error: error instanceof Error ? error.message : String(error),
301 });
302 return c.json(
303 { error: "Failed to retrieve forum metadata. Please try again later." },
304 500
305 );
306 }
307 });
308}
309```
310
311### Catch Block Guidelines
312
313**DO:**
314- Catch specific error types when possible (`instanceof RangeError`, `instanceof SyntaxError`)
315- Re-throw unexpected errors (don't swallow programming bugs like `TypeError`)
316- Log with structured context: operation name, relevant IDs, error message
317- Return user-friendly messages (no stack traces in production)
318- Classify errors by user action (400, 503) vs server bugs (500)
319
320**DON'T:**
321- Use bare `catch` blocks that hide all error types
322- Return generic "try again later" for client errors (400) vs server errors (500)
323- Fabricate data in catch blocks (return null or fail explicitly)
324- Use empty catch blocks or catch without logging
325- 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
326
327**Try Block Granularity:**
328
329When 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:
330
331```typescript
332// ❌ BAD: DB re-query failure reports "Failed to create category" even though
333// the PDS write already succeeded — misleading for operators debugging
334try {
335 const result = await createCategory(...); // PDS write succeeded
336 categoryUri = result.uri;
337 const [cat] = await db.select()...; // DB re-query fails here
338} catch (error) {
339 consola.error("Failed to create category:", ...); // Inaccurate!
340}
341
342// ✅ GOOD: each operation has its own try block and specific error message
343try {
344 const result = await createCategory(...);
345 categoryUri = result.uri;
346} catch (error) {
347 consola.error("Failed to create category:", ...);
348}
349
350try {
351 const [cat] = await db.select()...;
352} catch (error) {
353 consola.error("Failed to look up category ID after creation:", ...);
354}
355```
356
357**Programming Error Re-Throwing Pattern:**
358
359```typescript
360// ✅ CORRECT: Re-throw programming errors, catch runtime errors
361try {
362 const result = await ctx.db.select()...;
363 return processResult(result);
364} catch (error) {
365 // Re-throw programming errors (code bugs) - don't hide them
366 if (error instanceof TypeError ||
367 error instanceof ReferenceError ||
368 error instanceof SyntaxError) {
369 console.error("CRITICAL: Programming error detected", {
370 error: error.message,
371 stack: error.stack,
372 operation: "checkPermission"
373 });
374 throw error; // Let global error handler catch it
375 }
376
377 // Log and handle expected runtime errors (DB failures, network issues)
378 console.error("Database query failed", {
379 operation: "checkPermission",
380 error: error instanceof Error ? error.message : String(error)
381 });
382 return null; // Fail safely for business logic
383}
384```
385
386**Why re-throw programming errors:**
387- `TypeError` = code bug (e.g., `role.permisions.includes()` typo)
388- `ReferenceError` = code bug (e.g., using undefined variable)
389- `SyntaxError` = code bug (e.g., malformed JSON.parse in your code)
390- These should crash during development, not be silently logged
391- Catching them hides bugs and makes debugging impossible
392
393**Error Classification Helper:**
394
395Create helper functions to classify errors consistently:
396
397```typescript
398// File: src/lib/errors.ts
399export function isProgrammingError(error: unknown): boolean {
400 return error instanceof TypeError ||
401 error instanceof ReferenceError ||
402 error instanceof SyntaxError;
403}
404
405export function isNetworkError(error: unknown): boolean {
406 if (!(error instanceof Error)) return false;
407 const msg = error.message.toLowerCase();
408 return msg.includes("fetch failed") ||
409 msg.includes("econnrefused") ||
410 msg.includes("enotfound") ||
411 msg.includes("timeout");
412}
413
414// Usage in route handlers:
415} catch (error) {
416 if (isProgrammingError(error)) {
417 throw error; // Don't catch programming bugs
418 }
419
420 if (isNetworkError(error)) {
421 return c.json({
422 error: "Unable to reach external service. Please try again later."
423 }, 503); // User should retry
424 }
425
426 return c.json({
427 error: "An unexpected error occurred. Please contact support."
428 }, 500); // Server bug, needs investigation
429}
430```
431
432### Helper Functions
433
434**Validation helpers should:**
435- Return `null` for invalid input (not throw)
436- Re-throw unexpected errors
437- Use specific error type checking
438
439**Example:**
440```typescript
441export function parseBigIntParam(value: string): bigint | null {
442 try {
443 return BigInt(value);
444 } catch (error) {
445 if (error instanceof RangeError || error instanceof SyntaxError) {
446 return null; // Expected error for invalid input
447 }
448 throw error; // Unexpected error - let it bubble up
449 }
450}
451```
452
453**Serialization helpers should:**
454- Avoid silent fallbacks (log warnings if fabricating data)
455- Prefer returning `null` over fake values (`"0"`, `new Date()`)
456- Document fallback behavior in JSDoc if unavoidable
457
458### Defensive Programming
459
460**All list queries must have defensive limits:**
461```typescript
462.from(categories)
463.orderBy(categories.sortOrder)
464.limit(1000); // Prevent memory exhaustion on unbounded queries
465```
466
467**Filter deleted/soft-deleted records:**
468```typescript
469.where(and(
470 eq(posts.rootPostId, topicId),
471 eq(posts.deleted, false) // Never show deleted content to users
472))
473```
474
475**Use ordering for consistent results:**
476```typescript
477.orderBy(asc(posts.createdAt)) // Chronological order for replies
478```
479
480### Global Error Handler
481
482The Hono app must have a global error handler as a safety net:
483```typescript
484app.onError((err, c) => {
485 console.error("Unhandled error in route handler", {
486 path: c.req.path,
487 method: c.req.method,
488 error: err.message,
489 stack: err.stack,
490 });
491 return c.json(
492 {
493 error: "An internal error occurred. Please try again later.",
494 ...(process.env.NODE_ENV !== "production" && {
495 details: err.message,
496 }),
497 },
498 500
499 );
500});
501```
502
503### Testing Error Handling
504
505**Test error classification, not just error catching.** Users need actionable feedback: "retry later" (503) vs "report this bug" (500).
506
507```typescript
508// ✅ Test network errors return 503 (retry later)
509it("returns 503 when PDS connection fails", async () => {
510 mockPutRecord.mockRejectedValueOnce(new Error("fetch failed"));
511 const res = await app.request("/api/topics", {
512 method: "POST",
513 body: JSON.stringify({ text: "Test" })
514 });
515 expect(res.status).toBe(503); // Not 500!
516 const data = await res.json();
517 expect(data.error).toContain("Unable to reach your PDS");
518});
519
520// ✅ Test server errors return 500 (bug report)
521it("returns 500 for unexpected database errors", async () => {
522 mockPutRecord.mockRejectedValueOnce(new Error("Database connection lost"));
523 const res = await app.request("/api/topics", {
524 method: "POST",
525 body: JSON.stringify({ text: "Test" })
526 });
527 expect(res.status).toBe(500); // Not 503!
528 const data = await res.json();
529 expect(data.error).not.toContain("PDS"); // Generic message for server errors
530});
531
532// ✅ Test input validation returns 400
533it("returns 400 for malformed JSON", async () => {
534 const res = await app.request("/api/topics", {
535 method: "POST",
536 headers: { "Content-Type": "application/json" },
537 body: "{ invalid json }"
538 });
539 expect(res.status).toBe(400);
540 const data = await res.json();
541 expect(data.error).toContain("Invalid JSON");
542});
543```
544
545**Error classification patterns to test:**
546- **400 (Bad Request):** Invalid input, missing required fields, malformed JSON
547- **404 (Not Found):** Resource doesn't exist (forum, post, user)
548- **503 (Service Unavailable):** Network errors, PDS connection failures, timeouts — user should retry
549- **500 (Internal Server Error):** Unexpected errors, database errors — needs bug investigation
550
551## Security-Critical Code Standards
552
553When implementing authentication, authorization, or permission systems, follow these additional requirements:
554
555### 1. Fail-Closed Security
556
557**Security checks must deny access by default when encountering errors.**
558
559```typescript
560// ✅ CORRECT: Fail closed - deny access on any error
561export async function checkPermission(
562 ctx: AppContext,
563 did: string,
564 permission: string
565): Promise<boolean> {
566 try {
567 const [membership] = await ctx.db.select()...;
568 if (!membership || !membership.roleUri) {
569 return false; // No membership = deny access
570 }
571
572 const [role] = await ctx.db.select()...;
573 if (!role) {
574 return false; // Role deleted = deny access
575 }
576
577 return role.permissions.includes(permission) ||
578 role.permissions.includes("*");
579 } catch (error) {
580 if (isProgrammingError(error)) throw error;
581
582 console.error("Failed to check permissions - denying access", {
583 did, permission, error: ...
584 });
585 return false; // Error = deny access (fail closed)
586 }
587}
588
589// ❌ WRONG: Fail open - grants access on error
590} catch (error) {
591 console.error("Permission check failed");
592 return true; // SECURITY BUG: grants access on DB error!
593}
594```
595
596**Test fail-closed behavior:**
597```typescript
598it("denies access when database query fails (fail closed)", async () => {
599 vi.spyOn(ctx.db, "select").mockRejectedValueOnce(new Error("DB connection lost"));
600
601 const result = await checkPermission(ctx, "did:plc:user", "createTopics");
602
603 expect(result).toBe(false); // Prove fail-closed behavior
604 expect(console.error).toHaveBeenCalledWith(
605 expect.stringContaining("denying access"),
606 expect.any(Object)
607 );
608});
609```
610
611### 2. Security Test Coverage Requirements
612
613**All security features require comprehensive tests covering:**
614
615- ✅ **Happy path** - Authorized user succeeds
616- ✅ **Unauthorized user** - Returns 401 (not authenticated)
617- ✅ **Forbidden** - Returns 403 (authenticated but lacks permission)
618- ✅ **Privilege escalation prevention** - Cannot grant yourself higher privileges
619- ✅ **Peer protection** - Cannot modify users with equal authority
620- ✅ **Fail-closed behavior** - Database/network errors deny access
621- ✅ **Input validation** - Malformed requests return 400, not 500
622- ✅ **Error classification** - Network errors (503) vs server errors (500)
623
624**Example security test suite structure:**
625```typescript
626describe("POST /api/admin/members/:did/role (security-critical)", () => {
627 describe("Authorization", () => {
628 it("returns 401 when not authenticated", async () => { /* ... */ });
629 it("returns 403 when user lacks manageRoles permission", async () => { /* ... */ });
630 });
631
632 describe("Privilege Escalation Prevention", () => {
633 it("prevents admin from assigning owner role (higher authority)", async () => {
634 // Admin (priority 10) tries to assign Owner (priority 0) → 403
635 });
636
637 it("prevents admin from assigning admin role (equal authority)", async () => {
638 // Admin (priority 10) tries to assign Admin (priority 10) → 403
639 });
640
641 it("allows admin to assign moderator role (lower authority)", async () => {
642 // Admin (priority 10) assigns Moderator (priority 20) → 200
643 });
644 });
645
646 describe("Error Handling", () => {
647 it("returns 503 when PDS connection fails (network error)", async () => { /* ... */ });
648 it("returns 500 when database query fails (server error)", async () => { /* ... */ });
649 it("returns 400 for malformed roleUri (input validation)", async () => { /* ... */ });
650 });
651});
652```
653
654### 3. Startup Failures for Missing Security Infrastructure
655
656**Security-critical infrastructure must fail fast on startup, not at first request.**
657
658```typescript
659// ✅ CORRECT: Throw error on startup if ForumAgent unavailable
660export async function seedDefaultRoles(ctx: AppContext) {
661 const agent = ctx.forumAgent;
662 if (!agent) {
663 console.error("CRITICAL: ForumAgent not available - role system non-functional", {
664 operation: "seedDefaultRoles",
665 forumDid: ctx.config.forumDid
666 });
667 throw new Error(
668 "Cannot seed roles without ForumAgent - permission system would be broken"
669 );
670 }
671 // ... seeding logic
672}
673
674// ❌ WRONG: Silent failure allows server to start without roles
675if (!agent) {
676 console.warn("ForumAgent not available, skipping role seeding");
677 return { created: 0, skipped: 0 }; // Server starts but is broken!
678}
679```
680
681**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.
682
683### 4. Security Code Review Checklist
684
685Before requesting review for authentication/authorization code, verify:
686
687- [ ] All permission checks fail closed (deny access on error)
688- [ ] Database errors in security checks are caught and logged
689- [ ] Programming errors (TypeError) are re-thrown, not caught
690- [ ] Privilege escalation is prevented (equal/higher authority blocked)
691- [ ] Tests cover unauthorized (401), forbidden (403), and error cases
692- [ ] Error messages don't leak internal details (priority values, permission names)
693- [ ] Middleware composition is correct (auth before permission checks)
694- [ ] Startup fails fast if security infrastructure is unavailable
695
696## Documentation & Project Tracking
697
698**Keep these synchronized when completing work:**
699
7001. **`docs/atproto-forum-plan.md`** — Master project plan with phase checklist
701 - Mark items complete `[x]` when implementation is done and tested
702 - Add brief status notes with file references and Linear issue IDs
703 - Update immediately after completing milestones
704
7052. **Linear issues** — Task tracker at https://linear.app/atbb
706 - Update status: Backlog → In Progress → Done
707 - Add comments documenting implementation details when marking Done
708 - Keep status in sync with actual codebase state, not planning estimates
709
7103. **Workflow:** When finishing a task:
711 ```sh
712 # 1. Run tests to verify implementation is correct
713 pnpm test
714
715 # 2. If tests pass, commit your changes
716 git add .
717 git commit -m "feat: your changes"
718
719 # 3. Update plan document: mark [x] and add completion note
720 # 4. Update Linear: change status to Done, add implementation comment
721 # 5. Push and request code review
722 # 6. After review approval: include "docs:" prefix when committing plan updates
723 ```
724
725**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.
726
727## Bruno API Collections
728
729**CRITICAL: Keep Bruno collections synchronized with API changes.**
730
731The `bruno/` directory contains [Bruno](https://www.usebruno.com/) collections that serve dual purpose:
7321. **Interactive API testing** during development
7332. **Version-controlled API documentation** that stays in sync with code
734
735### When to Update Bruno Collections
736
737**When adding a new API endpoint:**
7381. Create a new `.bru` file in the appropriate `bruno/AppView API/` subdirectory
7392. Follow the naming pattern: use descriptive names like `Create Topic.bru`, `Get Forum Metadata.bru`
7403. Include all request details: method, URL with variables, headers, body (if POST/PUT)
7414. Add comprehensive documentation in the `docs` block explaining:
742 - Required/optional parameters
743 - Expected response format with example
744 - All possible error codes (400, 401, 404, 500, 503)
745 - Authentication requirements
746 - Validation rules
7475. Add assertions to validate responses automatically
748
749**When modifying an existing endpoint:**
7501. Update the corresponding `.bru` file in `bruno/AppView API/`
7512. Update parameter descriptions if inputs changed
7523. Update response documentation if output format changed
7534. Update error documentation if new error cases added
7545. Update assertions if validation logic changed
755
756**When removing an endpoint:**
7571. Delete the corresponding `.bru` file
7582. Update `bruno/README.md` if it referenced the removed endpoint
759
760**When adding new environment variables:**
7611. Update `bruno/environments/local.bru` with local development values
7622. Update `bruno/environments/dev.bru` with deployment values
7633. Document the variable in `bruno/README.md` under "Environment Variables Reference"
764
765### Bruno File Template
766
767When creating new `.bru` files, follow this template:
768
769```bru
770meta {
771 name: Endpoint Name
772 type: http
773 seq: 1
774}
775
776get {
777 url: {{appview_url}}/api/path
778}
779
780params:query {
781 param1: {{variable}}
782}
783
784headers {
785 Content-Type: application/json
786}
787
788body:json {
789 {
790 "field": "value"
791 }
792}
793
794assert {
795 res.status: eq 200
796 res.body.field: isDefined
797}
798
799docs {
800 Brief description of what this endpoint does.
801
802 Path/query/body params:
803 - param1: Description (type, required/optional)
804
805 Returns:
806 {
807 "field": "value"
808 }
809
810 Error codes:
811 - 400: Bad request (invalid input)
812 - 401: Unauthorized (requires auth)
813 - 404: Not found
814 - 500: Server error
815
816 Notes:
817 - Any special considerations or validation rules
818}
819```
820
821### Workflow Integration
822
823**When committing API changes, update Bruno collections in the SAME commit:**
824
825```sh
826# Example: Adding a new endpoint
827git add apps/appview/src/routes/my-route.ts
828git add apps/appview/src/routes/__tests__/my-route.test.ts
829git add bruno/AppView\ API/MyRoute/New\ Endpoint.bru
830git commit -m "feat: add new endpoint for X
831
832- Implements POST /api/my-endpoint
833- Adds validation for Y
834- Updates Bruno collection with request documentation"
835```
836
837**Why commit together:** Bruno collections are API documentation. Keeping them in the same commit ensures the documentation is never out of sync with the implementation.
838
839### Testing Bruno Collections
840
841Before committing:
8421. Open the collection in Bruno
8432. Test each modified request against your local dev server (`pnpm dev`)
8443. Verify assertions pass (green checkmarks)
8454. Verify documentation is accurate and complete
8465. Check that error scenarios are documented (not just happy path)
847
848### Common Mistakes
849
850**DON'T:**
851- Commit API changes without updating Bruno collections
852- Use hardcoded URLs instead of environment variables (`{{appview_url}}`)
853- Skip documenting error cases (only document 200 responses)
854- Leave placeholder/example data that doesn't match actual API behavior
855- Forget to update assertions when response format changes
856
857**DO:**
858- Update Bruno files in the same commit as route implementation
859- Use environment variables for all URLs and test data
860- Document all HTTP status codes the endpoint can return
861- Include example request/response bodies that match actual behavior
862- Test requests locally before committing
863
864## Git Conventions
865
866- Do not include `Co-Authored-By` lines in commit messages.
867- `prior-art/` contains git submodules (Rust AppView, original lexicons, delegation spec) — reference material only, not used at build time.
868- Worktrees with submodules need `submodule deinit --all -f` then `worktree remove --force` to clean up.