Fedibook Code Review 1 — Follow-up

Reviewed against the current fix/review-1 branch (2026-04-05, updated 2026-04-05). This document goes through every item in the original review and records its current status.

Priority List Items

✅ 1. Mixed-language error messages

All Danish user-facing strings have been replaced with English. The fix covered:

• service/post.go — 3 strings (post content, image count)
• service/friendship.go — 3 strings (user not found, self-request, duplicate request) + 1 notification message
• service/group.go — 1 string (member not found)
• handler/post.go — 1 "Konflikt" title + Danish body
• handler/friendship.go — 1 "Konflikt" title

The only remaining Danish text is in service/fbimport.go (Facebook export category keys), which is correct — those are data values, not error messages.

✅ 2. AP actor URL duplication

activitypub/urls.go provides UserActorURL, GroupActorURL, UserKeyID, GroupKeyID, and friends. These are now used consistently in handler/activitypub.go, handler/user.go, service/ap.go, service/ap_group.go, and internal/worker/handlers.go.

One remaining inline construction: handler/webfinger.go still assembles actor URLs by hand in two places (lines 69 and 124): "https://" + instanceDomain + "/ap/users/" + profile.Username. The rest of the codebase uses the helpers.

✅ 3. Worker main.go decomposition

All 12 task handler implementations have been moved to internal/worker/handlers.go. The Handlers struct holds the shared dependencies; each handler is a method with the asynq.HandlerFunc signature, so registration is a clean mux.HandleFunc(type, h.MethodName) call. cmd/worker/main.go is now 106 lines (down from ~900) and contains only startup, wiring, and the mux registrations.

✅ 4. Middleware error format

internal/middleware/problem.go was created with a local writeProblem function that sets Content-Type: application/problem+json and encodes a proper RFC 7807 struct. All five http.Error() calls in jwt.go and the one in ratelimit.go were replaced with calls to this helper.

✅ 5. fediverse_handle display bug

postResponse (handler/post.go:586) and commentResponse (handler/post.go:607) now call fediverseHandle(username, isLocal, instanceDomain) instead of the inline "@" + username + "@" + domain formula. Remote users (whose username is already stored as user@domain) now produce the correct @user@domain handle.

Consistency

✅ Mixed-language error messages

See priority item 1 above.

✅ Auth middleware uses a different error format

See priority item 4 above.

⬜ Inconsistent limit parsing

Not addressed. Two issues remain:

• handler/user.go Search (lines 204–208): uses a bare strconv.Atoi(l) with no upper-bound check. parseCursorParams in handler/post.go:662 enforces 0 < limit <= 100; this handler does not.
• handler/friendship.go List (lines 51–52) and ListRequests (lines 85–86): call _, limit := parseCursorParams(r) but then re-read the after cursor manually with r.URL.Query().Get("after"). The after value from parseCursorParams is discarded.

⬜ Group actor document is a raw map

Not addressed. handler/activitypub.go:GroupActor still builds the actor document as map[string]any (lines 277–303), while WebFingerHandler.GetActor uses the typed activitypub.Actor struct. The activitypub.Actor struct also still lacks an Image field for cover photos.

✅ NodeInfo ignores registration mode

Fixed. APHandler now has an openRegistrations bool field set at construction time. cmd/api/main.go passes cfg.RegistrationMode == "open" so instances in "approval" or "closed" mode correctly advertise false.

Complexity

✅ In-function regexp compilation

Fixed. A package-level var slugifyRE = regexp.MustCompile("[^a-z0-9]+") was added alongside the existing handleRE in service/group.go. slugifyHandle now calls slugifyRE.ReplaceAllString(...) instead of compiling on every invocation.

✅ Worker main.go decomposition

See priority item 3 above.

⬜ actorURLsToMentions vs actorURLsToMentionMaps duplication

Not addressed. Both functions still exist with equivalent logic but different return types:

• actorURLsToMentions in internal/worker/handlers.go:795 returns []activitypub.Mention
• actorURLsToMentionMaps in service/ap_group.go:1335 returns []map[string]string

The underlying divergence — that ap_group.go builds AP payloads as map[string]any while the worker uses typed structs — is what forces the duplication.

⬜ Opaque group membership status encoding

Not addressed. repository/group.go still concatenates role || '_' || status in SQL (line 64) to produce strings like "member_pending" and "admin_invited". The service layer still decodes these with a switch statement. A typo in either the SQL or the switch silently falls through to "none".

Structure

✅ Duplicate constructors

Fixed. NewPostHandler and NewGroupHandler (the short forms, neither of which had any call sites) have been deleted. Only NewPostHandlerWithUploadDir and NewGroupHandlerWithUploadDir remain, which is what cmd/api/main.go already used.

✅ HTTP signature verification is duplicated

Fixed. A readAndVerifyBody(w, r, logTag string) ([]byte, bool) method was extracted on APHandler. Both Inbox and GroupInbox now delegate to it, replacing the 10-line duplicated block in each.

⬜ Setter methods for optional dependencies

Not addressed. SetAPMemberInviter, SetGroupResolver, SetGroupFinder, SetGroupJoiner, SetLocalInviter in handler/group.go, handler/webfinger.go, service/ap_group.go, and service/channel.go still assign without validating. A missing call produces a nil-pointer panic at the first relevant request, not a startup error.

⬜ UserIDFromContext wrapper

Not addressed. handler/auth.go:267–270 still re-exports middleware.UserIDFromContext as a handler-package function. All handler files call the wrapper rather than the original directly.

Dead Code and Duplication

⬜ summaryMap vs userSummaryResponse

Not addressed. Both functions exist in the handler package and produce structurally identical map[string]any objects for a user summary:

• summaryMap(u *domain.UserSummary) — handler/friendship.go:306
• userSummaryResponse(u *domain.User, instanceDomain string) — handler/user.go:294

✅ fediverse_handle display bug

See priority item 5 above.

⚠️ AP actor URL constructed in five places

Partially addressed. The activitypub URL helpers are used in most places. The remaining exception is handler/webfinger.go, which still inlines the URL construction at two points (lines 69 and 124). All other files (handler/activitypub.go, handler/user.go, service/ap.go, service/ap_group.go, internal/worker/handlers.go) use activitypub.UserActorURL and activitypub.GroupActorURL.

⬜ processGroupAnnounce legacy path

Not addressed. service/ap.go:775 still contains the handler for the old Announce{Create{Note}} format. The comment notes it is kept for in-flight activities. No deprecation window or removal ticket exists.

ActivityPub Specifics

⚠️ Scattered AP URL construction

Same status as the dead-code item above — mostly resolved, with handler/webfinger.go as the remaining outlier.

⬜ Fragile fallback URL in ServeNote

Not addressed. handler/activitypub.go:ServeNote still reconstructs the actor URL from the username when post.Author.APID is nil, using the Mastodon-specific /users/ path segment. The correct fix is to ensure UpsertRemoteUser always populates ap_id, removing the need for the fallback entirely.

✅ openRegistrations hardcoded

Same as the NodeInfo consistency item above — fixed.

Summary

All five priority-list items and four further quick wins are resolved. No correctness bugs remain among the outstanding items — they are all structural or consistency concerns.