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# Contributing to atBB
2
3This document covers development workflow, testing standards, and coding patterns for contributors. For project-specific gotchas and non-obvious quirks, see [`CLAUDE.md`](CLAUDE.md).
4
5## Testing Workflow
6
7### When to Run Tests
8
9**Before every commit:**
10```sh
11pnpm test
12git add <specific files>
13git commit -m "feat: your changes"
14```
15
16**Before requesting code review:**
17```sh
18pnpm build # Ensure clean build
19pnpm test # Verify all tests pass
20# Only then push and request review
21```
22
23**After fixing review feedback:**
24```sh
25# Make fixes
26pnpm test # Verify tests still pass
27# Push updates
28```
29
30### Test Requirements
31
32All new features must include tests:
33- **API endpoints:** Success cases, error cases, edge cases
34- **Business logic:** All code paths and error conditions
35- **Error handling:** All catch blocks tested
36- **Security features:** Authentication, authorization, input validation
37
38### Test Quality Standards
39
40- Tests must be independent (no shared state between tests)
41- Use descriptive test names that explain what is being tested
42- Mock external dependencies (databases, APIs, network calls)
43- Test error paths, not just happy paths
44- Verify logging and error messages are correct
45
46### Test Coverage Expectations
47
48While we don't enforce strict coverage percentages, aim for:
49- **Critical paths:** 100% coverage (authentication, authorization, data integrity)
50- **Error handling:** All catch blocks should be tested
51- **API endpoints:** All routes should have tests
52- **Business logic:** All functions with branching logic should be tested
53
54Do not:
55- Skip writing tests to "move faster" — untested code breaks in production
56- Write tests after requesting review — tests inform implementation
57- Rely on manual testing alone — automated tests catch regressions
58
59## Error Handling Patterns
60
61### Defensive Programming
62
63All list queries must include defensive limits:
64```typescript
65.from(categories)
66.orderBy(categories.sortOrder)
67.limit(1000); // Prevent memory exhaustion on unbounded queries
68```
69
70Always filter soft-deleted records in read queries:
71```typescript
72.where(and(
73 eq(posts.rootPostId, topicId),
74 eq(posts.deleted, false),
75 eq(posts.bannedByMod, false)
76))
77```
78
79Use explicit ordering for consistent, reproducible results:
80```typescript
81.orderBy(asc(posts.createdAt)) // Chronological order for replies
82```
83
84### Global Error Handler
85
86Every Hono app must have a global error handler as a safety net. See `apps/appview/src/create-app.ts` for the production implementation:
87
88```typescript
89app.onError((err, c) => {
90 console.error("Unhandled error in route handler", {
91 path: c.req.path,
92 method: c.req.method,
93 error: err.message,
94 stack: err.stack,
95 });
96 return c.json(
97 {
98 error: "An internal error occurred. Please try again later.",
99 ...(process.env.NODE_ENV !== "production" && {
100 details: err.message,
101 }),
102 },
103 500
104 );
105});
106```
107
108### Testing Error Classification
109
110Test that errors return the right status code — users need actionable feedback:
111
112| Status | When to return |
113|--------|----------------|
114| 400 | Invalid input, missing required fields, malformed JSON |
115| 404 | Resource doesn't exist |
116| 503 | Network errors, PDS connection failures — user should retry |
117| 500 | Unexpected errors, database errors — needs investigation |
118
119```typescript
120// ✅ Network errors should return 503 (user should retry)
121it("returns 503 when PDS connection fails", async () => {
122 mockPutRecord.mockRejectedValueOnce(new Error("fetch failed"));
123 const res = await app.request("/api/topics", {
124 method: "POST",
125 body: JSON.stringify({ text: "Test" })
126 });
127 expect(res.status).toBe(503);
128});
129
130// ✅ Server errors should return 500 (needs investigation)
131it("returns 500 for unexpected database errors", async () => {
132 mockPutRecord.mockRejectedValueOnce(new Error("Database connection lost"));
133 const res = await app.request("/api/topics", {
134 method: "POST",
135 body: JSON.stringify({ text: "Test" })
136 });
137 expect(res.status).toBe(500);
138});
139
140// ✅ Input validation should return 400
141it("returns 400 for malformed JSON", async () => {
142 const res = await app.request("/api/topics", {
143 method: "POST",
144 headers: { "Content-Type": "application/json" },
145 body: "{ invalid json }"
146 });
147 expect(res.status).toBe(400);
148});
149```
150
151## Security Code Review Checklist
152
153Before requesting review for authentication/authorization code:
154
155- [ ] All permission checks fail closed (deny access on error)
156- [ ] Database errors in security checks are caught and logged
157- [ ] Programming errors (TypeError) are re-thrown, not caught
158- [ ] Privilege escalation is prevented (equal/higher authority blocked)
159- [ ] Tests cover unauthorized (401), forbidden (403), and error cases
160- [ ] Error messages don't leak internal details (priority values, permission names)
161- [ ] Middleware composition is correct (auth before permission checks)
162- [ ] Startup fails fast if security infrastructure is unavailable
163
164**Example test suite structure for security-critical endpoints:**
165```typescript
166describe("POST /api/admin/members/:did/role (security-critical)", () => {
167 describe("Authorization", () => {
168 it("returns 401 when not authenticated", async () => { /* ... */ });
169 it("returns 403 when user lacks manageRoles permission", async () => { /* ... */ });
170 });
171
172 describe("Privilege Escalation Prevention", () => {
173 it("prevents admin from assigning owner role (higher authority)", async () => {
174 // Admin (priority 10) tries to assign Owner (priority 0) → 403
175 });
176 it("allows admin to assign moderator role (lower authority)", async () => {
177 // Admin (priority 10) assigns Moderator (priority 20) → 200
178 });
179 });
180
181 describe("Error Handling", () => {
182 it("returns 503 when PDS connection fails (network error)", async () => { /* ... */ });
183 it("returns 500 when database query fails (server error)", async () => { /* ... */ });
184 it("returns 400 for malformed roleUri (input validation)", async () => { /* ... */ });
185 });
186});
187```
188
189## Bruno API Collections
190
191The `bruno/` directory contains [Bruno](https://www.usebruno.com/) collections for API documentation and interactive testing.
192
193### When to Update
194
195| Event | Action |
196|-------|--------|
197| New endpoint | Create a `.bru` file in `bruno/AppView API/<feature>/` |
198| Modified endpoint | Update the corresponding `.bru` file |
199| Removed endpoint | Delete the `.bru` file; update `bruno/README.md` if referenced |
200| New environment variable | Update `bruno/environments/local.bru` and `bruno/environments/dev.bru`; document in `bruno/README.md` |
201
202Always update Bruno files **in the same commit** as the route implementation.
203
204### File Template
205
206```bru
207meta {
208 name: Endpoint Name
209 type: http
210 seq: 1
211}
212
213get {
214 url: {{appview_url}}/api/path
215}
216
217params:query {
218 param1: {{variable}}
219}
220
221headers {
222 Content-Type: application/json
223}
224
225body:json {
226 {
227 "field": "value"
228 }
229}
230
231assert {
232 res.status: eq 200
233 res.body.field: isDefined
234}
235
236docs {
237 Brief description of what this endpoint does.
238
239 Path/query/body params:
240 - param1: Description (type, required/optional)
241
242 Returns:
243 {
244 "field": "value"
245 }
246
247 Error codes:
248 - 400: Bad request (invalid input)
249 - 401: Unauthorized (requires auth)
250 - 404: Not found
251 - 500: Server error
252
253 Notes:
254 - Any special considerations or validation rules
255}
256```
257
258### Before Committing
259
2601. Open the collection in Bruno
2612. Test each modified request against your local dev server (`pnpm dev`)
2623. Verify assertions pass (green checkmarks)
2634. Verify documentation is accurate and complete
2645. Check that error scenarios are documented (not just happy path)