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
feat: persist user handle to DB on login so posts show handles (#63)
* docs: add design for storing user handles at login time
* feat: persist user handle to DB during OAuth login so posts show handles
* fix: address PR review comments on handle persistence
Critical:
- Add isProgrammingError guard to upsert catch so TypeErrors are not swallowed
- Add logger.warn assertion to upsert failure test (per CLAUDE.md logging test requirement)
Important:
- Fix upsert to use COALESCE so a null getProfile result never overwrites a good existing handle
- Add warn log when persisting null handle so operators can detect suspended/migrating accounts
- Add test: getProfile returns undefined → null handle written, login still succeeds
- Add test: existing handle preserved when getProfile returns undefined
- Align log severity — upsert failure now uses warn (consistent with membership failure)
- Fix misleading vi.clearAllMocks() comment; fresh ctx is what makes the spy safe to abandon
- Update design doc snippet to match implementation (use extracted handle variable + COALESCE)
Suggestion:
- Add test: TypeError from upsert is re-thrown and causes 500, not silent redirect
- Hardcode getProfile mock return value instead of deriving from DID string split
+170
-38
apps/appview/src/routes/__tests__/auth.test.ts
+170
-38
apps/appview/src/routes/__tests__/auth.test.ts
···
1
1
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
2
2
import { Hono } from "hono";
3
3
import { createTestContext, type TestContext } from "../../lib/__tests__/test-context.js";
4
+
import { eq } from "drizzle-orm";
5
+
import { users } from "@atbb/db";
6
+
7
+
const TEST_DID = "did:plc:test-oauth";
8
+
const TEST_HANDLE = "test-oauth.test.bsky.social";
4
9
5
10
// Mock createMembershipForUser at module level BEFORE importing routes
6
11
let mockCreateMembership: ReturnType<typeof vi.fn>;
···
9
14
createMembershipForUser: vi.fn((...args) => mockCreateMembership(...args)),
10
15
}));
11
16
12
-
// Mock Agent to avoid real PDS calls
17
+
// Mock Agent to avoid real PDS calls. mockGetProfile is assigned in setupOAuthMocks
18
+
// so individual tests can override it per-call.
19
+
let mockGetProfile: ReturnType<typeof vi.fn>;
20
+
13
21
vi.mock("@atproto/api", () => ({
14
22
Agent: vi.fn((session: any) => ({
15
23
did: session?.did,
16
-
getProfile: vi.fn(async ({ actor }: { actor: string }) => ({
17
-
data: {
18
-
handle: `${actor.split(":").pop()}.test.bsky.social`,
19
-
displayName: "Test User",
20
-
},
21
-
})),
24
+
getProfile: vi.fn((...args: any[]) => mockGetProfile(...args)),
22
25
})),
23
26
}));
24
27
25
28
// Import routes AFTER mocking
26
29
const { createAuthRoutes } = await import("../auth.js");
27
30
31
+
function setupOAuthMocks(ctx: TestContext) {
32
+
(ctx.oauthClient.callback as any) = vi.fn(async () => ({
33
+
session: {
34
+
did: TEST_DID,
35
+
sub: TEST_DID,
36
+
iss: "https://bsky.social",
37
+
aud: "http://localhost:3001",
38
+
exp: Math.floor(Date.now() / 1000) + 3600,
39
+
iat: Math.floor(Date.now() / 1000),
40
+
scope: "atproto transition:generic",
41
+
server: {} as any,
42
+
sessionGetter: {} as any,
43
+
dpopFetch: {} as any,
44
+
serverMetadata: {} as any,
45
+
},
46
+
state: "test-state",
47
+
}));
48
+
49
+
(ctx.oauthStateStore.get as any) = vi.fn(async () => ({
50
+
iss: "https://bsky.social",
51
+
pkceVerifier: "test-verifier",
52
+
dpopKey: undefined,
53
+
}));
54
+
55
+
ctx.cookieSessionStore.set = vi.fn(async () => {});
56
+
57
+
mockGetProfile = vi.fn().mockResolvedValue({
58
+
data: { handle: TEST_HANDLE, displayName: "Test User" },
59
+
});
60
+
}
61
+
28
62
describe("OAuth callback - membership creation error handling", () => {
29
63
let ctx: TestContext;
30
64
let app: Hono;
···
36
70
// Reset mocks
37
71
vi.clearAllMocks();
38
72
39
-
// Mock OAuth client callback to simulate successful OAuth flow
40
-
// Cast to any to bypass complex OAuth type checking in tests
41
-
(ctx.oauthClient.callback as any) = vi.fn(async () => ({
42
-
session: {
43
-
did: "did:plc:test-oauth",
44
-
sub: "did:plc:test-oauth",
45
-
iss: "https://bsky.social",
46
-
aud: "http://localhost:3001",
47
-
exp: Math.floor(Date.now() / 1000) + 3600,
48
-
iat: Math.floor(Date.now() / 1000),
49
-
scope: "atproto transition:generic",
50
-
// Add minimal OAuth session methods to satisfy type
51
-
server: {} as any,
52
-
sessionGetter: {} as any,
53
-
dpopFetch: {} as any,
54
-
serverMetadata: {} as any,
55
-
},
56
-
state: "test-state",
57
-
}));
58
-
59
-
// Mock state store to validate CSRF/PKCE
60
-
(ctx.oauthStateStore.get as any) = vi.fn(async () => ({
61
-
iss: "https://bsky.social",
62
-
pkceVerifier: "test-verifier",
63
-
dpopKey: undefined,
64
-
}));
65
-
66
-
// Mock cookie session store set method (used to create session after OAuth)
67
-
ctx.cookieSessionStore.set = vi.fn(async () => {});
73
+
setupOAuthMocks(ctx);
68
74
});
69
75
70
76
afterEach(async () => {
···
91
97
expect(mockCreateMembership).toHaveBeenCalledWith(
92
98
expect.anything(), // ctx
93
99
expect.anything(), // agent
94
-
"did:plc:test-oauth" // user's DID
100
+
TEST_DID
95
101
);
96
102
});
97
103
···
132
138
// Arrange: Mock membership helper to succeed
133
139
mockCreateMembership = vi.fn().mockResolvedValue({
134
140
created: true,
135
-
uri: "at://did:plc:test-oauth/space.atbb.membership/test123",
141
+
uri: `at://${TEST_DID}/space.atbb.membership/test123`,
136
142
cid: "bafytest123",
137
143
});
138
144
···
148
154
expect(mockCreateMembership).toHaveBeenCalled();
149
155
});
150
156
});
157
+
158
+
describe("OAuth callback - user handle persistence", () => {
159
+
let ctx: TestContext;
160
+
let app: Hono;
161
+
162
+
beforeEach(async () => {
163
+
ctx = await createTestContext();
164
+
app = new Hono().route("/api/auth", createAuthRoutes(ctx));
165
+
vi.clearAllMocks();
166
+
167
+
setupOAuthMocks(ctx);
168
+
mockCreateMembership = vi.fn().mockResolvedValue({ created: false });
169
+
});
170
+
171
+
afterEach(async () => {
172
+
await ctx.cleanup();
173
+
});
174
+
175
+
it("inserts user row with handle when user does not yet exist in DB", async () => {
176
+
const res = await app.request("/api/auth/callback?code=test-code&state=test-state");
177
+
expect(res.status).toBe(302);
178
+
179
+
const [user] = await ctx.db
180
+
.select()
181
+
.from(users)
182
+
.where(eq(users.did, TEST_DID));
183
+
184
+
expect(user).toBeDefined();
185
+
expect(user.handle).toBe(TEST_HANDLE);
186
+
187
+
expect(ctx.cookieSessionStore.set).toHaveBeenCalledWith(
188
+
expect.any(String),
189
+
expect.objectContaining({ handle: TEST_HANDLE })
190
+
);
191
+
});
192
+
193
+
it("updates existing user handle when row was created by firehose with null handle", async () => {
194
+
await ctx.db.insert(users).values({
195
+
did: TEST_DID,
196
+
handle: null,
197
+
indexedAt: new Date(),
198
+
});
199
+
200
+
const res = await app.request("/api/auth/callback?code=test-code&state=test-state");
201
+
expect(res.status).toBe(302);
202
+
203
+
const allUsers = await ctx.db
204
+
.select()
205
+
.from(users)
206
+
.where(eq(users.did, TEST_DID));
207
+
208
+
expect(allUsers).toHaveLength(1);
209
+
expect(allUsers[0].handle).toBe(TEST_HANDLE);
210
+
});
211
+
212
+
it("inserts null handle when getProfile returns no handle (suspended/migrating account)", async () => {
213
+
mockGetProfile = vi.fn().mockResolvedValue({
214
+
data: { handle: undefined, displayName: "Test User" },
215
+
});
216
+
217
+
const res = await app.request("/api/auth/callback?code=test-code&state=test-state");
218
+
expect(res.status).toBe(302);
219
+
expect(res.headers.get("Location")).toBe("/");
220
+
221
+
const [user] = await ctx.db
222
+
.select()
223
+
.from(users)
224
+
.where(eq(users.did, TEST_DID));
225
+
226
+
expect(user).toBeDefined();
227
+
expect(user.handle).toBeNull();
228
+
});
229
+
230
+
it("preserves existing handle when getProfile returns no handle", async () => {
231
+
await ctx.db.insert(users).values({
232
+
did: TEST_DID,
233
+
handle: "alice.bsky.social",
234
+
indexedAt: new Date(),
235
+
});
236
+
237
+
mockGetProfile = vi.fn().mockResolvedValue({
238
+
data: { handle: undefined, displayName: "Test User" },
239
+
});
240
+
241
+
const res = await app.request("/api/auth/callback?code=test-code&state=test-state");
242
+
expect(res.status).toBe(302);
243
+
244
+
const [user] = await ctx.db
245
+
.select()
246
+
.from(users)
247
+
.where(eq(users.did, TEST_DID));
248
+
249
+
expect(user.handle).toBe("alice.bsky.social");
250
+
});
251
+
252
+
it("login still succeeds if user handle DB upsert fails", async () => {
253
+
const loggerWarnSpy = vi.spyOn(ctx.logger, "warn");
254
+
255
+
vi.spyOn(ctx.db, "insert").mockImplementationOnce(() => {
256
+
throw new Error("DB connection lost");
257
+
});
258
+
259
+
const res = await app.request("/api/auth/callback?code=test-code&state=test-state");
260
+
261
+
expect(res.status).toBe(302);
262
+
expect(res.headers.get("Location")).toBe("/");
263
+
expect(res.headers.get("Set-Cookie")).toContain("atbb_session=");
264
+
expect(ctx.db.insert).toHaveBeenCalled(); // Confirms the upsert path was reached before failing
265
+
266
+
expect(loggerWarnSpy).toHaveBeenCalledWith(
267
+
"Failed to persist user handle during login",
268
+
expect.objectContaining({ did: TEST_DID, error: "DB connection lost" })
269
+
);
270
+
// No manual restore needed — beforeEach creates a fresh ctx, so this spy is abandoned.
271
+
});
272
+
273
+
it("returns 500 when upsert throws a TypeError (programming error re-thrown)", async () => {
274
+
vi.spyOn(ctx.db, "insert").mockImplementationOnce(() => {
275
+
throw new TypeError("Cannot read properties of undefined");
276
+
});
277
+
278
+
const res = await app.request("/api/auth/callback?code=test-code&state=test-state");
279
+
280
+
expect(res.status).toBe(500);
281
+
});
282
+
});
+28
apps/appview/src/routes/auth.ts
+28
apps/appview/src/routes/auth.ts
···
1
1
import { Hono } from "hono";
2
2
import { setCookie, getCookie, deleteCookie } from "hono/cookie";
3
3
import { randomBytes } from "crypto";
4
+
import { sql } from "drizzle-orm";
4
5
import { Agent } from "@atproto/api";
5
6
import type { AppContext } from "../lib/app-context.js";
6
7
import { restoreOAuthSession } from "../lib/session.js";
7
8
import { createMembershipForUser } from "../lib/membership.js";
9
+
import { isProgrammingError } from "../lib/errors.js";
10
+
import { users } from "@atbb/db";
8
11
9
12
/**
10
13
* Authentication routes for OAuth flow using @atproto/oauth-client-node.
···
147
150
}
148
151
149
152
ctx.logger.info("OAuth callback success", { did: session.did, handle });
153
+
154
+
// Persist handle to users table so posts display handles instead of DIDs.
155
+
// Non-fatal: login completes even if this fails.
156
+
if (!handle) {
157
+
ctx.logger.warn("Persisting null handle — user will appear as DID in posts", {
158
+
operation: "GET /api/auth/callback",
159
+
did: session.did,
160
+
});
161
+
}
162
+
try {
163
+
await ctx.db
164
+
.insert(users)
165
+
.values({ did: session.did, handle: handle ?? null, indexedAt: new Date() })
166
+
.onConflictDoUpdate({
167
+
target: users.did,
168
+
set: { handle: sql`COALESCE(${handle ?? null}, ${users.handle})` },
169
+
});
170
+
} catch (error) {
171
+
if (isProgrammingError(error)) throw error;
172
+
ctx.logger.warn("Failed to persist user handle during login", {
173
+
operation: "GET /api/auth/callback",
174
+
did: session.did,
175
+
error: error instanceof Error ? error.message : String(error),
176
+
});
177
+
}
150
178
151
179
// Attempt to create membership record
152
180
try {
+42
docs/plans/2026-02-24-show-handles-in-posts-design.md
+42
docs/plans/2026-02-24-show-handles-in-posts-design.md
···
1
+
# Design: Show Handles in Posts
2
+
3
+
**Date:** 2026-02-24
4
+
**Status:** Approved
5
+
6
+
## Problem
7
+
8
+
The `users.handle` column exists in the database but is never populated. The firehose indexer inserts user rows with `handle: null`, and the OAuth login flow fetches the handle via `agent.getProfile()` but only stores it in the in-memory `CookieSession`. As a result, the web frontend always falls back to displaying the author's DID.
9
+
10
+
## Approach
11
+
12
+
Store the authenticated user's handle in the `users` table during the OAuth callback. The handle is already fetched at `apps/appview/src/routes/auth.ts:128-130` — it just needs to be persisted.
13
+
14
+
## Implementation
15
+
16
+
In the OAuth callback handler (`auth.ts`), after `getProfile()` resolves, upsert the user row:
17
+
18
+
```typescript
19
+
await ctx.db
20
+
.insert(users)
21
+
.values({ did: session.did, handle: handle ?? null, indexedAt: new Date() })
22
+
.onConflictDoUpdate({
23
+
target: users.did,
24
+
set: { handle: sql`COALESCE(${handle ?? null}, ${users.handle})` },
25
+
});
26
+
```
27
+
28
+
(`handle` is pre-extracted from `profile.data.handle` earlier in the callback. The `COALESCE` ensures an existing good handle is not overwritten if `getProfile` returns no handle on a subsequent login.)
29
+
30
+
No schema changes are required. This handles both cases:
31
+
- User row already exists (created by firehose): updates `handle`
32
+
- User row does not exist yet: inserts it
33
+
34
+
## Scope and Limitations
35
+
36
+
- Handles are resolved only for users who have logged in at least once.
37
+
- Post authors who have never authenticated will continue to show their DID.
38
+
- This is intentional for MVP — firehose-based handle resolution can be layered on later.
39
+
40
+
## Testing
41
+
42
+
Add assertions to the existing OAuth callback test (`auth.test.ts`) verifying that the `users` table is upserted with the correct handle after a successful login.