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.
Line 4 pointed at a stale `runMigrations()` name and omitted the drizzle-kit
generate step entirely. Replace with a description of the real workflow:
`pnpm db:generate` produces SQL from `schema.ts`, `initDatabase()` runs
`drizzle.migrate()` + `ensureDefaults()` on startup. Note the 2026-04-24
baseline squash for future readers.
Delete the "Migration flags (internal)" line — those flags belonged to
pre-drizzle data-fix migrations deleted wholesale in 3acaea2 (2026-04-09)
and are historical trivia with no present referent.
Refs backlog #31 Phase 2.
dm-system.md: fold the voice.md cross-reference into the paragraph so it
renders as part of the Cross-instance access explanation instead of an
orphaned bullet.
websocket.md: add 'host_unreachable' to the dm_call_undeliverable phase
union — stale since #32 was merged (docs drift noted in Task 8 review).
federation.md — new undeliverable bucket subsection with three-way
classification table, Path A/B semantics, and wire backward-compat note.
voice.md — no_recipient row in failure-surface table.
websocket.md — DmCallUndeliverableReason union updated to include no_recipient.
dm-system.md — cross-reference to voice.md for no_recipient reason.
instanceStore registers three resolver functions at module load —
setApiForOriginResolver, setUserIdForOriginResolver,
setOriginFromHostnameResolver — whose backing `let` bindings used to
live in spaceStore. When the module graph was entered from
instanceStore (e.g. JoinSpaceModal importing useInstanceStore) the
order became spaceStore → chatStore → useWebSocket → socialStore →
instanceStore (top-level setter call) while spaceStore was still
paused on its line-8 chatStore import, so the backing `let` had not
been reached yet and the setter crashed with
`Cannot access '_getApiForOrigin' before initialization`. This left
InviteModal.test.tsx and JoinSpace.test.tsx unable to even load their
suites once AudioManager was mocked away.
Move the three `let` bindings, their setters, their pure getters, plus
the WS-populated user-ID cache (`_myUserIdByOrigin`, setMyUserIdForOrigin,
getCachedUserIdForOrigin, clearMyUserIdCache) into
`packages/web/src/utils/crossStoreResolvers.ts`. The utility imports
nothing from `./stores/*`, so no back-edge exists. spaceStore re-exports
the public surface for backward compatibility with the many existing
import sites; instanceStore imports the setters directly from the
utility (the in-cycle re-export path does not resolve at module-init
time under vite-ssr, so a direct import is required for the top-level
setter calls).
spaceStore's remaining wrappers (resolveUserOrigin, getLayoutHomeOrigin,
getMyUserIdForOrigin) stay where they are — they combine the utility's
pure lookups with authStore state — but now delegate to the utility.
Also adds the AudioManager mock to InviteModal.test.tsx and
JoinSpace.test.tsx so their suites actually load (same pattern already
used in 5 other test files). Net test-suite result: 127/131 pass (up
from 121/121 — +6 newly unlockable). The 4 remaining JoinSpace
failures are pre-existing stale UI-text assertions (the placeholder was
expanded and the submit button was made disable-when-empty) made
visible by the suite now loading; they're orthogonal to this change
and handed back for a separate triage.
Closes backlog #27.
New subsection under client-federation.md's origin-aware routing section
covering dmAlternatives, failoverDmOriginsFromDisconnected, rekey flow,
trigger points, the intentional cache-flush trade-off, voice-out-of-scope,
no-re-home policy, and the WS routing contract. dm-system.md gets a
one-line cross-reference from the Client routing bullet.
Duplicate rejection means the peer already has the message (e.g.,
delivered earlier via outbox AND pulled via sync in the same
window). Retrying will fail identically forever until TTL expires.
Before this patch: duplicate-rejected outbox entries were retained
with attempts++ and exponential backoff, creating log noise and
outbox bloat for up to 30 days.
After: duplicate-rejected entityIds join the terminal set alongside
accepted ones and are deleted from the outbox. Logged at info level
('outbox entry removed (terminal)') to distinguish from warn-level
transient-rejection retries.
Other rejection reasons (attribution_mismatch, processing_error,
etc.) stay on the retry path; some may also be terminal but are
deferred until observed accumulating.
Replace the batch-level processRelayEvents call with a per-event
loop wrapped in try/catch. On exception: log event type, messageId,
timestamp, peer origin, and the error message; continue to the next
event.
Previously, a single poison-pill event (e.g., UNIQUE conflict from
a malformed relay payload) would throw, be caught by the outer
try/catch, and block lastSyncedAt from advancing — causing every
future activation to retry the same broken window indefinitely.
The final 'replayed N events' log line now reports '(K skipped due
to errors)' when K > 0, surfacing the count to operators. Individual
event failures are logged via console.error with enough context to
debug or replay manually.
Trade-off documented in docs/systems/federation.md: forward progress
of the sync pipeline takes priority over strict at-least-once
delivery. An event that fails to process is lost to the receiver
unless replayed manually.
Adds 'Peer Activation Recovery' subsection with call-site roster,
peer-state x outbox-enqueue x recovery matrix, mutation log
coverage table, /api/federation/sync contextType filter values,
and a Known Issues note about the poison-pill edge case.
Also updates stale references to runInitialSyncForNewPeers (removed
in commit 02a1ed7) to point at the unified startupBootstrapSync
path.
/peer/initiate checked `response.ok` to decide whether to activate the
local peer. `response.ok` is true for the full 2xx range, so a remote
that returned 202 (queued for admin approval — autoAcceptPeering off
on their side) caused the local peer to flip to `active` while the
remote had us `awaiting_approval`. The split only self-healed when
the remote admin approved and pushed us an `awaiting_approval → active`
override via the peer_approval_requests inbound path.
The auto-peer flow in federationPeering.ts:performHandshake already
had the correct 202 branch: set local status to awaiting_approval,
broadcast federation_peers_changed, surface a pending outcome. Mirror
it here:
- Check response.status === 202 BEFORE the !response.ok branch so the
fall-through can't reach the activation code.
- Transition local peer to awaiting_approval (not active).
- Broadcast federation_peers_changed so other admin tabs refresh.
- Return 202 with the sanitized peer so the client observes the
queued state distinctly from both success and failure.
Also added the missing federation_peers_changed broadcast on the
activation (200) path for parity with every other peer-state-change
site in the codebase — it was a pre-existing drift that would leave
sibling admin tabs stale after an initiate. Pattern-aligned with
federationPeering.ts:160 and the rest of routes/federation.ts.
Docs: expanded Phase 1 bullets in docs/systems/federation.md to cover
the 200 / 202 / other non-2xx / network-error branches explicitly and
reference the mirrored auto-peer branch.
Verified: pnpm -r typecheck clean (shared + server), vitest 70/70
pass.
Closes#21 from S2S DM unification backlog.
The column was `integer DEFAULT 0` (nullable) since the initial schema.
Counters should not be nullable — the semantics are a count, not an
optional measurement. `consecutive_auth_failures` (added later) was
correctly declared NOT NULL; tightening `consecutive_failures` to match
removes the drift and eliminates the "|null" burden everywhere the value
is read.
SQLite does not support in-place ALTER … SET NOT NULL, so drizzle-kit
cannot auto-generate this. The manual migration uses the standard
SQLite recreate pattern (new table + INSERT SELECT + DROP + RENAME +
recreate index) under `PRAGMA defer_foreign_keys = ON` so the existing
federation_outbox → federation_peers FK survives the swap. The COPY
step coalesces any hypothetical NULL to 0 defensively; live probes on
both test instances (nova, orbit) showed zero NULL rows so no
actual backfill is required.
Verified by applying the full migration chain against a copy of the VM's
live DB: column ends as `notnull=1 dflt=0`, the peer row is preserved,
the unique index on origin is recreated, NULL inserts are rejected, and
`PRAGMA foreign_key_check` reports no violations.
Server `SanitizedPeer.consecutiveFailures` tightened to `number` to
match the new drizzle inference and the shared `FederationPeer` shape.
Follow-up #22 from S2S DM unification backlog.
HEALTH_CHECK_INTERVAL_MS was 1 h, but ROTATION_GRACE_PERIOD_MS is 15 min.
Phase skew between two peers' health-check ticks could stretch rotation
finalization desync up to ~1 h, during which signatures from the already-
finalized side verify against the other side's primary-only secret (grace
has expired; verifyPeerSignature stops trying the pending secret). With
AUTH_FAILURE_THRESHOLD = 5 and the existing backoff schedule, this
occasionally tripped legitimate rotations into needs_attention.
Setting the interval to 15 min (= ROTATION_GRACE_PERIOD_MS) guarantees a
finalization tick fires within one grace window on each side, so the
cross-verification window where one peer signs with NEW while the other
still treats NEW as pending cannot outlast the grace period.
Per-tick cost is negligible for the worker's steady state: the only
network fetches are per-active-peer /peer/rotate calls when the 90-day
rotation interval hits (rare) and per-unreachable-peer /instance/info
health pings (bounded by outage count). Going lower than 15 min would
reduce the residual desync but increase tick overhead with diminishing
returns; 15 min is the grace-period-aligned value that the original spec
("runs hourly") deviated from without justification.
Follow-up #20 from S2S DM unification backlog; reduces #19 false-positive
rate (outbox auth-failure transition) on legitimate rotations.
processMemberAddEvent, processMemberRemoveEvent, and processOwnershipTransferEvent inserted system messages unconditionally. Outbox retries and initial-sync replays (triggered whenever an admin re-approves a peering request, which recreates the peer row with lastSyncedAt=0) duplicated the system message on every delivery. Each new snowflake ID exceeded the user's last_read_message_id, flipping the channel back to unread after every deploy.
Processors now short-circuit on a matching (source_instance, source_message_id) row, and persist those fields when inserting. processMemberAddEvent emits the tagged system message in both bootstrap and incremental paths so bootstrap replays don't fall through and insert a second one; the bootstrap's dm_channel_created broadcast carries that message as lastMessage so sidebar previews and unread anchors agree across instances.
When two instances each have a native user with the same username, the
Add Friend search card for the federated one sent its request to the
local namesake instead of the intended remote user.
Root cause: `isNative = !homeUserId` in socialStore's searchUsers and
loadFriends dedup. The server backfills native users' homeUserId to
their own id so federation tier-1 lookups succeed, so `homeUserId` is
set on natives too. Only `homeInstance` distinguishes native (null)
from replicated stubs. With the wrong check, no entry was ever "native"
and the home-origin stub of the remote user was kept over the true
native record — leaving `_instanceOrigin=''`, which caused the Send
button handler to drop the domain suffix and POST to the home API,
where "nova" resolved to a completely different local user.
Also fixes loadRequests dedup to prefer the target-native record so the
search card correctly flips to "Request Pending" after sending.
Remove the owner-only gate on POST /api/dm/:id/members. The S2S relay
already accepts member_add from any HMAC-verified peer, and the UI
already shows the add button to all group DM members. Only the
server-side check was blocking non-owners.
Add federation.md section 8b covering dm_close/dm_reopen relay events:
payload, outbound queueing via queueDmCloseRelay, inbound processDmCloseEvent/
processDmReopenEvent handlers (lookup-only identity resolution, silent
no-ops on missing channel/user/membership), and the processCreateEvent
closed-state reopen bug fix.
Update dm-system.md soft-close section with federation behaviour: relay
to peers for both close and reopen, relayed-message reopen in
processCreateEvent, and the federatedId-only guard for legacy DMs.
The client-side federation model (instanceStore, federated accounts,
multi-instance connections, origin-aware routing) was completely
undocumented. An agent reading only federation.md would understand S2S
relay but have no knowledge of how the client connects to multiple
instances, creates federated accounts with real credentials, or routes
API/WS calls to the correct instance.
New spec covers: instanceStore architecture, federated account creation
(username@instance format), Connections UI, auto-connect lifecycle,
channelOriginMap routing, WebSocket multiplexing, cross-instance
identity resolution, and the relationship between client-side and S2S
federation.
CLAUDE.md subsystem table updated. Both federation docs cross-reference
each other.
- Spec: status → Implemented, fix PATCH peer as new (not pre-existing),
correct API paths, update component breakdown to match actual structure
- Federation docs: update relay rate limit from 30 to 90 req/min
Prevent malicious peers from forging events attributed to users on other
instances. Every relay event processor now verifies the acting user's
homeInstance (from payload) matches X-Federation-Origin (from HMAC-verified
header) via verifyAttribution(), normalized to bare domain.
- Add verifyAttribution() helper using extractDomain normalization
- Guard all 13 event processors before any user resolution or DB writes
- Add homeInstance to FederationRelayReaction type + outbound payloads
- Replace unnormalized string equality in friend handlers
- Log mismatched values on rejection for debugging
Jannis Braun