Three issues caught by the code review of scripts/gen-icons.mjs:
(1) Missing source SVG validation. Spec requires the script "refuses
to run if any source SVG is missing." Added an existsSync guard at the
top of main() that throws a targeted message rather than letting sharp
fail with a raw ENOENT.
(2) Summary table missing byte sizes. Spec says the summary prints
"each output, its size in pixels, its file size in bytes." trace() now
captures statSync().size; the printout has a size column with
human-readable formatting and a total at the end.
(3) writeMaskablePng's intermediate buffer used .png() without the
compressionLevel/palette options that every other render uses. Zero
functional impact (the buffer is only piped into composite, never
written to disk) but inconsistent and misleading. Aligned with the
rest of the script.
README: replaced 'git add -A' with explicit directory paths matching
the project's convention — the generator writes to a fixed set of
three directories and shouldn't accidentally stage unrelated
working-tree changes during a regen.
Plan updated to match the corrected code so Task 3's source-of-truth
stays consistent.
Single Node ESM script. Reads assets/brand/*.svg, writes the full
desktop + web icon set deterministically. Idempotent; safe to re-run.
Companion README documents the version-bump determinism caveat.
Three semantic copies from Artworks-Backspace/SVG/ — drives the new
icon generator. Element 1 → app-icon (full badge), Element 2 → mark
(bare gradient B, transparent), Element 4 → mark-mono-dark (solid
black for macOS menu-bar template). Element 3 and Element 5 stay in
the archive only.
When autostart is disabled, Windows deletes the Run registry entry, so
deriveStartMinimizedFromArgs(undefined) was returning false and silently
resetting the user's startMinimized preference the next time autostart
was re-enabled. Both the get- and set-auto-launch-settings handlers now
fall back to the disk-cached value when no OS entry exists, matching the
existing macOS/Linux fallback pattern.
Replaces the stale Auto-Launch section with an accurate description of the
Tasks 3–7 implementation: OS-authoritative read model, platform-specific
applyLoginItemSettings() contract, Linux/AppImage-only conditional re-apply,
defence-in-depth hidden-launch detection, and the pure helpers in autoLaunch.ts.
Also updates Startup Sequence step 9 to reflect that unconditional re-apply is gone.
- AutoLaunchSettings: remove pre-await state writes; backend response is
now the sole writer to openAtLogin/startMinimized state, eliminating
the flicker caused when optimistic values diverged from OS truth
- Add busy flag across IPC round-trip; both Toggles receive disabled={busy}
- Start-minimized Toggle additionally receives disabled={!openAtLogin},
preventing interaction when it would have no effect
- Toggle.tsx: add optional disabled prop (forwarded to <button disabled>)
with cursor-not-allowed opacity-50 visual feedback; non-breaking for
all existing callers
macOS: add args:[--hidden] alongside openAsHidden for defence-in-depth detection on macOS 13+.
Windows: add enabled/path/args/name so re-enabling the toggle clears the StartupApproved\Run disable marker.
Linux: add deterministic name:'backspace' so the .desktop filename is stable across Electron/AppImage updates.
Mirrors the REST-login change in f17c46c on the registration insert path.
A successful POST /api/auth/register does not by itself imply a live
WebSocket — the client may never connect (transient network, mobile
background, error path between the 201 and /ws), leaving a permanently
stuck-online row that no disconnect timer can clean up. The schema
default 'offline' is correct; ws/handler.ts flips it to 'online' on real
WS auth.
The federated-stub upgrade path in the same handler is unaffected: it
only updates passwordHash/username/homeUserId/displayName/avatarColor,
leaving the stub's pre-existing 'offline' status (set when the stub was
created via replication) untouched.
Updates docs/systems/auth.md step 7 to reflect the new behavior.
users.status was only flipped back to offline by the WebSocket disconnect
path (5s grace timer in ConnectionManager). Process exits (deploy/crash/OOM)
lose those in-memory timers, freezing any non-offline row at its last value
and making the user appear permanently online to friends and space co-members.
Confirmed in production on the Pi instance: a user appeared online for ~3
days with no live socket.
Add resetStalePresenceOnBoot() in utils/presenceBoot.ts and call it from
index.ts after getDb()/seedDatabase() and before WebSocket route registration.
The reset is federation-safe: it only updates rows where home_instance IS
NULL (replicated stubs are projections of remote presence and must not be
stomped) and is_deleted = 0 (tombstoned users are excluded from broadcasts).
Also remove the redundant status='online' write from POST /api/auth/login.
A successful REST login does not imply a live socket; the WS auth handshake
is the single source of truth. Login alone could otherwise produce the same
stuck-online row when a client logs in and never establishes a WS.
Tests cover: locally-homed online/idle/dnd reset, replicated rows untouched,
tombstoned rows untouched, idempotence, mixed populations.
Updates docs/systems/activity-presence.md (Connect/Disconnect Flow, new Boot
Reset section) and docs/systems/auth.md (login no longer mutates status).
Two correctness/defense fixes plus regression tests in the existing
in-memory drizzle test file.
1. Reverse-direction idempotency. The sender-side path in social.ts
checks BOTH directions of friend_requests and returns 409
incoming_request_exists when an opposite-direction row exists. The
receiver only matched from->to, so cross-fire (alice@A and bob@B both
click "add friend" near-simultaneously) produced two opposite
pending rows on each instance. The receiver now silent-accepts when
either direction matches a pending row, mirroring the sender's
both-direction check.
2. Self-target guard (defense-in-depth). Reject events whose
from-identity equals to-identity (after normalizeOriginForCompare)
with a new receiver-acknowledged 4xx code self_target_invalid.
Sender's local cannot_friend_self should catch this, but the
receiver does not trust upstream validation. Added to
TERMINAL_REJECTION_REASONS so the standard rollback fires
(mapped client-side to peer_rejected). Logged at console.warn.
Spec updates: social.md inbound contract now documents both-direction
idempotency and the self-target guard; federation.md and the
s2s-friend-add design spec list the new terminal rejection reason.
Extends the existing happy-path test (one case) with 14 more covering
every branch of the receiver-side handler: missing payload, attribution
mismatch, tombstoned sender (silent accept), recipient_not_found
(including soft-deleted recipient), already-friends idempotency in both
column orderings, same-direction pending re-delivery idempotency,
declined-then-resent (status filter), reused replicated stub, profile
hydration with URL resolution, explicit and fallback createdAt, and
mixed-batch accept/reject isolation through processRelayEvents.
The third describe block (`ensurePeered needs_attention handling`) was
the last federation test still using the legacy `vi.doMock` +
hand-crafted drizzle query-chain pattern. Convert it to the converged
in-memory better-sqlite3 + real drizzle pattern used by every other
federationPeering.* test (exemplar:
federationPeering.approvalToken.test.ts).
The first two describe blocks (`EnsurePeeredResult type`, `racePeering`)
did not use vi.doMock and remain byte-identical. The needs_attention
test's behavior is preserved 1:1: same `it(...)` description, same
assertions (status === 'rejected', error contains 'needs_attention',
fetch never called).
Module-level `vi.mock` calls now apply to the whole file, but the
type-only and racePeering tests don't exercise the mocked modules, so
there is no behavioral interaction.
No production code changes. No new test scenarios. No skipped/only/
commented blocks. All 352 server tests pass.
Live verification caught a stale-UI bug: when admin denial / approval
fanout / janitor expiry cascaded subscriber rows away, only the
peering_notification_received WS event fired (which only refetches
the notification list). The user's pending-subscriptions section
stayed stale until manual refresh.
Each terminal-state path now also fires peering_subscription_changed
to affected users so their pending list refreshes alongside the new
notification. Fixed in:
- onPeerActivated.fanoutOutboundSubscribers (approval path)
- handleOutboundDeny (admin denial)
- cleanupExpiredApprovalRequests (janitor expiry; also adds the
notification-received broadcast that was previously deferred to
next-page-load only)
Without this, when the local admin gate fires on friend-add, the user
got a generic 'Could not send friend request' fallback. Now they see
'Your admin needs to approve federation with this instance' with a
pointer to Connections settings where Task 12's pending-approvals
section shows their queued request.
Two new inline sections in the user-facing federation/connections settings
panel: 'Recent peering outcomes' (terminal-state notifications with
Retry-for-approved + Dismiss) and 'Pending peering approvals' (active
subscriber rows the user is waiting on, with Cancel). New WS handlers for
peering_subscription_changed and peering_notification_received refresh the
lists in real-time and surface a transient toast for online users. Retry
deep-link for friend_add prefills the friend-add input with the original
target handle (other reasons get Dismiss only — the gate doesn't wire
those paths yet).
PendingApprovals now branches row rendering on direction. Inbound rows
render exactly as before. Outbound rows show '<instance> — N users
want us to peer' with an inline subscriber list (username + reason +
target). ConfirmDialog descriptions branch on direction so admins
see appropriate copy for outbound approve (initiate handshake on
behalf of N users) vs outbound deny (notify requesting users).
- Drops the web-local ApprovalRequest interface in favor of the
canonical shared type (Task 2 already established this; Task 10
reconciles the consumer side).
- Adds five new federation methods wrapping the REST endpoints
added in Tasks 8 and 9: peeringSubscriptions (GET), cancelPeering
Subscription (DELETE), peeringNotifications (GET +/- unreadOnly
filter), markPeeringNotificationRead (POST :id/read), markAll
PeeringNotificationsRead (POST read-all).
db.select() returned every column including userId; spec §4.9
defined the response row WITHOUT userId. The leak is harmless today
(user queries their own rows) but expands the public API surface
beyond the spec, and would become part of the contract once Task 10
generates client types. Switching to explicit column projection.
Task 9's first pass replaced inbound expiry's signed /peer/denied
POST with a plain delete, citing symmetric independent expiry as
the design. The spec at §4.10 explicitly said 'inbound row cleanup
unchanged' — that was scope creep, not a fix. Inbound expiry now
preserves the pre-branch behavior verbatim: signed POST to remote,
delete only on success, retry on failure. Outbound expiry's fanout
logic (the actual Task 9 scope) is unchanged.
- GET /api/federation/peering-notifications (?unread=1 filter)
- POST /api/federation/peering-notifications/:id/read (per-row mark-read)
- POST /api/federation/peering-notifications/read-all (bulk mark-read,
preserves already-read readAt; returns affected count)
- janitor: outbound expired rows fan out kind='expired' notifications
to each subscriber before cascade-deleting parent (replaces Task 1's
scaffolding 'continue' guard); inbound expired rows are deleted outright
(both sides expire independently — no cross-instance network call)
- janitor: 30-day cleanup pass for read notifications
(cleanupReadPeeringNotifications); unread rows persist indefinitely
- cleanupExpiredApprovalRequests is now sync (no async network IO)
Notes:
- No WS broadcast on janitor expiry — offline users see notifications on
next GET, matching the persistence guarantee of the notifications table.
- The previous /peer/denied network call on inbound expiry is removed;
symmetric per-side expiry now handles termination on both sides.
Tests: 12 new (3 + 4 endpoint cases on 3 endpoints; 2 outbound/inbound
janitor expiry cases + 1 not-yet-expired guard + 1 retention sweep);
247 server tests passing total.
User-facing endpoints for the pending-peering-approvals UI surface.
GET lists the user's own pending subscriber rows joined to parent
peering requests. DELETE cancels a single subscription; if it was
the last subscriber, cascade-deletes the parent (admin queue updates
via federation_peers_changed). Authorization: subscriber.userId must
match request.userId.
The fanoutOutboundSubscribers helper is called from onPeerActivated,
the single point all peer activations flow through (queue approval,
/peer/initiate, autoAccept=1 remote, mutual-approval token verification).
Subscribers see kind='approved' notification, parent + subscriber rows
cascade-delete. Critical: cleanup triggers on status->active, not on
the local approve action — when the remote also gates, the peer goes
to awaiting_approval first and subscribers must remain queued.
- social.ts friend-add: user_action, with 409 peer_pending_local_admin
when gate fires
- /peer/ensure: user_action, surfaces peeringStatus: 'admin_required'
- sendCallRelay (typing warm-up + call relay): system intent
- federationWorker resolvePendingPeers: system intent (defensive — gate
is unreachable from here since pending rows already exist)
- CallRelayFailureReason: peer_admin_required added (mapped to
peer_transient_failure on the user-facing event surface, since system
intent should never legitimately surface admin_required)
- Test files: thread intent arg through racePeering and ensurePeered
calls (positional shift from racePeering signature change)
- outboundGate.test.ts: tighten noUncheckedIndexedAccess access via
non-null assertions after toHaveLength()
- docs/systems/social.md: peer_pending_local_admin error code documented
The pre-existing trust-guard's query filtered peer_approval_requests by
origin only. After Task 3 added outbound rows to the same table, the
guard started matching the user's own queued outbound row on retry,
returning 'rejected' with a misleading 'admin must resolve pending
approval' copy instead of the intended 'admin_required'. The variable
name (pendingInbound) and comment block already described the intent
as inbound-only — the query just didn't match. Adding direction='inbound'
to the where clause restores the intended behavior.
- New 'admin_required' EnsurePeeredResult variant.
- Required intent argument (no optional default) prevents future
user-initiated callers from silently getting system behavior.
- Gate runs only when no peer row exists; toggling autoAccept later
does not retroactively gate established peer rows.
- queueOutboundApproval helper upserts parent + subscriber rows and
broadcasts to admins + user.
- Schema enum-narrows direction ('inbound' | 'outbound') and
notifications.kind ('approved' | 'denied' | 'expired') at the column
level; drizzle generate confirmed no migration delta.
- Existing call sites now fail typecheck — fixed in Task 5.
- Symmetric with PeeringNotificationKind at module scope; avoids
future collision with unrelated trigger systems.
- subscribers field doc now explicit: undefined for inbound, present
(possibly []) for outbound. Pins the response contract before Task 7
implements the GET endpoint response shape.
drizzle's snapshot does not encode SQL-level CHECK constraints. Without
this comment, a future migration that recreates the table for unrelated
reasons would silently drop the (direction='inbound' → hmac_secret
NOT NULL) invariant.
Closes the receiver-side trust-bypass class in /peer/accept's
awaiting_approval branch via single-use cryptographic approval tokens.
Builds on the cheap fix (4533e36) that landed earlier today.
Schema: nullable approval_token columns on federation_peers and
peer_approval_requests. Wire format: optional approvalToken field on
/peer/accept request body and 202 response body. Receiver verifies
token before promoting awaiting_approval → active. /approve forwards
the stored token in its outbound /peer/accept; performHandshake,
/peer/initiate, and /approve all capture the returned token from 202.
Backward compat: legacy peers (no token) fall through the existing
autoAccept gate — autoAccept=0 queues, autoAccept=1 promotes (no
regression vs prior behavior). Existing active peers untouched.
Live verification on nova + orbit (autoAccept=0 scenario):
- Bypass attempt without token → 202 (queued), peer row untouched.
- Positive case with matching token → 200, secret rotated, token cleared,
stale approval-request deleted.
Test counts: 289 → 307 (+18 across 4 new test files).
Spec: internal notes
Plan: internal notes
federation.md: new 'Approval Token Verification' subsection covering
issuance (queue path generates token, returns in 202), storage on
initiator (federation_peers.approval_token), forwarding from /approve,
verification on receiver's awaiting_approval branch, single-use lifecycle,
backward compatibility, and threat-model boundary (sender-side outbound
gating tracked separately).
database.md: approval_token column documented on both federation_peers
and peer_approval_requests with cross-references to federation.md.
api.md: /peer/accept request + 202 response now show optional
approvalToken field with pointer to the federation spec.
Three changes to keep the outbound /peer/accept call sites consistent
with the new approval-token mechanism:
1. /approve outbound body now forwards approvalToken from the queued
peer_approval_requests row when present. Receiver's awaiting_approval
branch verifies it and promotes mutual approval. Legacy null-token
rows omit the field; receiver falls through autoAccept gate. Spec §3.7.
2. /approve and /peer/initiate 202 paths now capture the approvalToken
returned by the remote and store it on the local federation_peers row.
Without this, the symmetric autoAccept=0 mutual-approval flow could
not verify on the eventual return /peer/accept. Spec §3.7.
3. /approve and /peer/initiate 200 paths now include approvalToken=null
in the activation UPDATE — single-use lifecycle hygiene per §3.2.
Test coverage: +6 tests (4 in approveOutbound, 2 in peerInitiateOutbound).
Total: 301 → 307. Web tsc clean.
Receiver-side defense — closes the trust-bypass class the cheap fix
(4533e36) cannot cover. The /peer/accept handler's awaiting_approval
branch now requires an approvalToken matching the one stored on the
local peer row before promoting to active. Without a match:
- autoAccept=0 falls through to queueApprovalRequest (no bypass; new
approval-request queued, existing awaiting_approval row untouched).
- autoAccept=1 falls back to permissive promotion (no regression vs
prior behavior, since autoAccept=1 would accept any inbound regardless).
Successful match also deletes any stale approval-request row for the
origin to prevent debris accumulation from prior bypass attempts.
Refactor: queueing path extracted to a top-level queueApprovalRequest()
helper so both the no-existing-peer case and the awaiting_approval
mismatch fallback share one implementation. Helper generates a fresh
single-use token on every call.
Adds 'accept_awaiting_approval_fallback' variant to PeerActivationReason
to distinguish the autoAccept=1 fallback path from the verified path
in onPeerActivated audit logs.
Spec §3.5, §3.6.
Test counts: 289 → 301 (+12).
- 202 response: parse approvalToken from body and store on the local
federation_peers row alongside status='awaiting_approval'. Legacy
receivers that omit the field result in null stored token, handled
gracefully by the verification logic landing in subsequent commits.
- 200 response: clear approvalToken in the same UPDATE that sets
status='active' (single-use consumption per spec §3.2).
Test coverage: 4 tests in federationPeering.approvalToken.test.ts covering
JSON token, missing token (legacy), non-JSON body, and 200 clear after
prior token storage.
Nullable text columns on federation_peers and peer_approval_requests.
Existing rows degrade gracefully (NULL token) per spec §5; the verification
logic landing in subsequent commits routes legacy null-token state through
the existing autoAccept gate.
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