Closes the auto-reconnect trust-bypass: any code path calling
ensurePeered(remote) on an instance with autoAcceptPeering=0 could
previously bypass the admin gate by initiating a fresh handshake to the
remote, which the remote then accepted against its existing
awaiting_approval row.
The trigger surfaced was stores/instanceStore.ts:1010 — the silent
.catch(() => {}) auto-reconnect that fires for any user with the
remote in their replicatedInstances (commonly: any admin). Anyone with
that profile reloading their session activated peering on both sides
without any admin approval action.
Surgical fix: ensurePeered now returns rejected when an unresolved
inbound peer_approval_requests row exists for the target origin. The
legitimate admin-approve flow (routes/federation.ts:1089) does not call
ensurePeered; it deletes the approval-request and does its own direct
fetch to /peer/accept, so this check does not block legitimate approvals.
The receiver-side trust assumption at routes/federation.ts:619-645
(awaiting_approval branch in /peer/accept) still has the same flaw
— an adversarial peer that knows the timing could re-handshake at the
right moment to flip the receiver to active. That deeper trust-model
rework is plan-grade work tracked at internal notes
2026-04-26-peer-handshake-trust-model.md.
The T19/T20 catch blocks looked for an `.body` property on thrown errors
to extract the structured error code. The shared API client (api/client.ts:298)
actually throws `new Error(body.error)` — the code lives in `err.message`,
and there's no `.body` attached.
Live E2E (T22 scenario 2) caught this: typing alice@orbit against an
awaiting_approval peer surfaced the raw code 'peer_pending_approval' as
the toast text instead of the human-readable mapServerErrorToMessage
output. Same defect would have hit every server-error toast on both
FriendsPage (AddFriend + UserDiscoverCard) and UserProfileModal.
Catch blocks now use err.message as both the code and the fallback text;
the inline comment points at the API client throw site so the contract
is documented at the consumer.
Same pattern as FriendsPage cleanup (T19): the friend-add catch no longer
branches on the deleted InstanceNotConnectedError / InstanceDisconnectedError;
the ConnectInstanceModal trigger is removed since the server handles
all routing/peering. Errors surface via toast using mapServerErrorToMessage.
This restores the web package to a compileable state.
Removes try/catch on the deleted InstanceNotConnectedError/Disconnected
classes (T17). Server now returns structured error codes; client maps
them to human-readable toasts via the new mapServerErrorToMessage helper.
The friend-add flow no longer triggers ConnectInstanceModal — the server
handles all routing/peering/lookup. The modal itself stays for Connections
settings and space-join flows.
Multi-tab sync: friend_request_sent appends the new outbound request to
socialStore (deduped by id+origin), no toast.
Async rollback: friend_request_relay_failed removes the row by id and
surfaces a warning toast with the target handle and reason. Wires the
client side of the rollback hook from T10.
Server now handles all parsing/routing/peering/lookup (T11-T14). Client
sends the trimmed username verbatim to /api/social/requests and surfaces
server errors via toast (added in T18-T20).
Note: FriendsPage.tsx and UserProfileModal.tsx will fail to compile
until T19 and T20 remove their now-dead try/catch blocks for the
deleted error classes. TypeScript catches it; pnpm dev will not start
until those tasks land.
The friend contextId must be byte-identical on both peers for a given
canonical pair regardless of argument order. Initial-sync backfill keys
on contextId; if the two sides computed different values for the same
friendship, missed events would never reconcile.
The local branch was reusing friendRequestPayload (built for
friend_request_received with user=sender) for the sender-side
friend_request_sent broadcast. Tab B on the sender would render
the sender's own avatar where the target's should appear.
Match the federated branch — sent broadcast carries target.
Refactors the POST handler into handleLocalFriendRequest + handleFederatedFriendRequest helpers. The federated branch resolves the target domain, ensures peering, looks up the remote user, creates/hydrates a replicated stub, and writes a transactional (friend_requests + mutation_log + outbox) event with relayMessageId set. Also exports hydrateReplicatedUserProfile from federation.ts and adds the T11 happy-path test.
The T3 commit (e4086fe) generated the SQL migration but missed the
drizzle meta files that track migration state. Without these, future
db:generate runs would re-emit or skew migration ordering.
Nullable indexed column tracking the entityId of the originating relay
event for federated friend requests. Used by the rollback hook in
federationRollback.ts to locate and delete rows when a federated
friend_request_create is permanently rejected (spec §5).
Sections 3 and 12 now describe the discover-equivalent filter set
applied to /api/social/search and the widened Direct-Add row
contract (always-visible for well-formed input, resolved-form
display, server-side username normalization).
Per code-review: directAddDisplay now reads directAt === -1
instead of re-deriving includes('@'); add a one-line comment on
showDirectAdd so the predicate's intent is obvious at first read.
The Send-Friend-Request action row in the Add Friend tab previously
appeared only when the typed query contained a non-edge @, leaving
no way to fire a blind request for a bare local handle. Widen the
gate to allow non-empty bare handles, keep the malformed @ shapes
(@, @bob, bob@) hidden. When the typed query has no @, display the
resolved form <query>@<window.location.host> so the user sees which
instance the request will hit. Submission string is unchanged.
Updates FriendsPage.test.tsx: inverts the now-stale 'does not show
Direct Add row for plain usernames' test into the new positive
assertion, and adds a separate test for the malformed @ shapes.
Per code-review:
- Drop the inline comment in sendFriendRequest; the commit message
for the prior commit already covers the why and CLAUDE.md prefers
no comments when the code is self-explanatory.
- Add writable: true to the window.location defineProperty in the
test so re-firing beforeEach across jsdom version drift is safe.
Hostnames are case-insensitive (RFC 4343), and both right-hand
sides of the routing comparisons (window.location.host and
URL.host) are already canonical lowercase. The user-typed domain
substring was compared with strict ===, so ORBIT.ddns.net
failed to match an existing connected peer and popped a spurious
Connect Instance modal. Normalize at parse time.
Per code-review suggestion: add a brief comment explaining why
module-level sqlite/testDb/app reassignment works (mock getter
closes over the current binding) and what would break it
(top-level it, .concurrent describe). Prevents a future foot-gun.
Registration canonicalizes usernames to lowercase (auth.ts:32),
but the friend-request endpoint compared with strict eq() against
raw user input, so 'Bob' returned 404 even when 'bob' existed.
Trim and lowercase before lookup, matching the rest of the auth
boundary. Empty-after-trim now returns 400 (was: 404).
Tests appended to social.test.ts as a second describe block
sharing the harness from Task 1.
Drop the file-path reference per code-review suggestion (paths in
comments rot); keep the load-bearing why — substring of the domain
matches every stub because stubs are stored as <homeUserId>@<domain>.
Tombstoned users, replicated federated stubs, and users with
discoverable=0 were all surfacing in Add Friend search results.
Add the three WHERE filters that /api/social/discover already
applies. Federated users continue to be surfaced via the
client-side cross-instance fan-out in socialStore.searchUsers.
New tests: social.test.ts covers all five filter cases plus
existing self-exclusion and displayName-match behaviours.
Final-review reviewer flagged two minor staleness items:
- embeds.md §10 ImageEmbed bullets still described the pre-Task-1
shape (no wrapper, no aspect-ratio). Replaced with the actual
current shape, with an explicit pointer to the Dimension
reservation contract section that explains why the dims-null
branch deliberately has no fallback.
- message-list.md said VideoEmbed uses "padding-bottom" without
noting the direct-video branch uses aspectRatio. Now describes
both branches explicitly.
No code changes; both are documentation-only touch-ups.
Reviewer caught two small gaps after Task 3:
- Effect A's smooth-scroll path on new messages is intentionally NOT
instrumented with the sentinel (the animation lands asynchronously
across frames; no intermediate scrollTop is worth pinning to). The
doc now records this so the reader's intuition matches the code.
- The sentinel-branch comment in MessageList.tsx pointed at "spec §2",
which is the planning doc rather than the durable subsystem spec.
Pointed at docs/systems/message-list.md instead.
Tracks the post-clamp scrollTop of every programmatic scroll-to-bottom in
lastProgrammaticBottomScrollRef. handleScroll skips the at-bottom flip and
re-pins when the event's scrollTop matches the sentinel — i.e., the event
was queued by our own command and layout grew underneath. User scrolls
break the match (scrollTop changes) and flow through the normal path.
This complements the 2026-03-25 race-fix (which gated auxiliary effects on
isAtBottomRef) by also preventing handleScroll from flipping that ref to
false based on a post-growth distance measurement of our own scroll.
Restores the dimension reservation reverted in dae6f2d, scoped to
the embed.width && embed.height case only. No fallback aspect-ratio
when dims are null — that was the source of the dark letterbox bars
on Tenor/Klipy GIFs and OG-less images that triggered the revert.
The server-side probe in embedResolver.ts:196 already populates dims
for all image-type embeds; this change makes the client honor them.
Third initiator path that calls remote /peer/accept. Mirrors performHandshake (auto-peer) and /peer/initiate (admin-initiate) — same try/catch parse, same null-or-non-empty-string guard. Caught in final review of #33; same root cause as Bug #1, bundled rather than fragmented to a new backlog item.
Fresh-create returned {id, ownerId, federatedId, createdAt, members, lastMessage}; the existing-DM path returned the same shape minus federatedId. Inconsistency was a footgun for any future feature reading federatedId from this response — fresh-create tests would pass while idempotent path would break. One-line addition to the result builder.
Mirrors the previous performHandshake fix for the admin-initiated path. /peer/initiate now parses the remote's instanceName from the /peer/accept response body and writes it alongside status='active'.
Initiator side of the bidirectional handshake exchange. /peer/accept now returns instanceName in the response body (prior commit); performHandshake parses it and persists alongside status='active'. Tolerates missing field (older peers) and non-JSON bodies.
Bidirectional handshake exchange. Today the responder learns the initiator's instance name from request body but the initiator never learns the responder's. Adding {instanceName} to the response body lets the initiator persist it on its side (next commit). Field is optional so older peers omitting it cause no ill effect.
Previously /peer/accept read body.instanceName only when queueing for admin approval. The four paths that mutate federation_peers (rejected→active override, awaiting_approval→active, pending→active, new-peer create) all wrote status='active' without persisting instance_name. Result: every peer established via direct handshake had instance_name = NULL forever. Anywhere peerLabel was rendered fell back to origin hostname.
Active/needs_attention idempotent early-return path deliberately left alone — same security posture that already refuses to overwrite hmac_secret from unauthenticated requests on already-active peers.
Existing live NULL rows are repaired post-deploy via manual UPDATE statements (see plan).
Jannis Braun