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.
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).
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.
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).
baselineExistingInstall, healInitialSchemaDrift, and healRenamedColumns
existed only because of pre-squash intermediate states: dev DBs drifted
against drizzle-kit's assumed 0000 baseline, or carried pre-rename
columns from the old manual migration system. Under a single squashed
baseline there are no intermediate states to drift against, and these
functions are unreachable.
initDatabase() is now: pragmas -> drizzle() -> migrate() -> ensureDefaults().
ensureDefaults stays (idempotent startup seeding for settings row, worker
ID, first-admin promotion).
Refs backlog #31 Phase 2.
Follow-up from Task 2 code review. Keeps the test mock aligned with
the widened return type even though vi.mock doesn't structurally
typecheck the factory.
sendFederatedCallStart now treats a 200-with-undeliverable-messageId as
a peer-failure instead of unconditional success. Feeds the existing
failures[] array and terminal-determination machinery from #16.
New sendFederatedCallStartForTest export mirrors the existing
handleDm*ForTest pattern. TDD — three tests cover single-peer terminal
no_recipient, group-DM mixed delivered+undeliverable non-terminal, and
the happy-path (empty undeliverable → no event).
Also hardens sendCallRelay's response parse: validates undeliverable
is an Array and entries are well-shaped, logs protocol drift at warn/debug
rather than silently falling back to old-peer semantics.
CallRelayResult success arm gains undeliverable: string[]. sendCallRelay
parses FederationRelayResponse.undeliverable (when present) and returns
the messageIds so sendFederatedCallStart can reclassify per-peer results.
Old peers that omit the field → empty array → today's behavior.
TDD — three tests cover old-peer, new-peer-with-undeliverable, and 5xx paths.
processDmCallStartEvent Path A now skips offline local members (matching
Path B's pre-existing per-member check) and pushes undeliverable when no
member could be rung, instead of creating a stranded FederatedCallEntry.
TDD — two new tests cover zero-online and mixed-online cases.
processDmCallStartEvent Path B no longer silently accepts when no local
participant is reachable. Pushes {messageId, reason: 'no_recipient'} to
the undeliverable ack bucket so the caller can surface fast-fail.
TDD — test asserts undeliverable push + no FederatedCallEntry.
Additive plumbing. No behavior change — every existing event-type path
continues to push to accepted/rejected only. Response serializes the new
bucket only when non-empty (byte-identical responses in the normal case).
Tasks 3-4 add actual undeliverable pushes for dm_call_start paths.
The admin reset endpoint (DELETE-pattern gated on peer.status !== 'needs_attention')
doesn't transition status — it deletes the row of an already-deactivated peer.
onPeerDeactivated already fired at the earlier needs_attention transition, so
the reset site correctly has no hook. The enum value was defensive-unused; per
project principles (no backwards-compat shims, no placeholders) drop it.
Admin-revoke endpoint (DELETE /api/federation/peers/:id): fires
onPeerDeactivated(id, 'admin_revoked') after the status write to
'revoked', evicting any in-flight federated calls for the now-revoked
peer.
Admin-reset endpoint (POST /api/federation/peers/:id/reset): hook
SKIPPED. The reset endpoint is guarded to only run when status is
already 'needs_attention' (active peers are rejected at the boundary
with a 400). Because the peer was already deactivated before reset is
called, onPeerDeactivated was already fired at the active→needs_attention
transition. The reset deletes the row entirely rather than writing a new
status; it does not represent a transition OUT OF active, so wiring it
here would be a semantic error — double-evicting an already-deactivated
peer.
Code-review catch: the Path-2 accept-rollback previously emitted
dm_call_undeliverable { terminal: true } via sendToFederatedCallUsers,
which broadcasts to every ringedUserIds entry. In a group DM this
would prematurely tear down non-accepting ringees whose own accept /
reject / timeout paths should govern their state. Switch to
sendToUser(acceptorId) so only the acting user gets the terminal
signal. Reorder the clearFederatedCall to happen before the emit so a
concurrent end-handler sees a cleared entry (clearFederatedCall is
idempotent). Spec updated, test extended to assert the scoping with a
two-ringee group-DM fixture.
Companion to #29. `healInitialSchemaDrift` can only ADD columns, so it
skips NOT NULL-without-default columns like `federation_outbox.entity_id`
/ `context_id` — which on some old pre-drizzle dev DBs carry the
pre-rename names `message_id` / `dm_channel_id` instead. The tables
load but the outbox worker fails every tick with "no such column:
federation_outbox.context_id" once the server is up.
Adds healRenamedColumns() — a second pass that runs right after
`healInitialSchemaDrift`. For each table whose physical column set is
*missing* columns declared by the current-migration-state snapshot AND
which holds zero rows, it DROPs the table and rebuilds it from the
snapshot's JSON: columns, defaults, foreign keys, composite PKs,
unique constraints, indexes.
Key design decisions:
- **Target is the current-migration-state snapshot, not the latest on
disk.** The current state is determined by the highest
`__drizzle_migrations.created_at` matched against `_journal.json`'s
`when` timestamps (with backward walk for idx values that lack a
snapshot, like the hand-written 0002). Rebuilding to a *future*
snapshot would introduce columns that drizzle's migrator is about
to add via ALTER TABLE ADD COLUMN, causing duplicate-column errors.
Rebuilding to the *current* snapshot preserves the invariant that
drizzle's pending migrations can run cleanly afterwards.
- **Missing-column gate, not extra-column.** Extra columns alone don't
break anything at runtime (the ORM ignores them); they're leftover
from pre-drizzle manual migrations and might matter to the operator.
Missing columns DO break runtime queries, so only those trigger
rebuild.
- **Empty-table gate.** Non-empty tables log a warning and skip —
data preservation wins over heal, and this path should only ever
hit a pre-drizzle dev DB that never exercised the affected tables
in the first place.
- **Transactional rebuild.** DROP + CREATE + index reinstatement wrap
in a single `db.transaction()` so a partial rebuild rolls back.
Verified against three scenarios via in-memory simulation:
(A) fresh install — heal no-op, drizzle creates everything; (B) pre-
drizzle dev DB with fed_outbox/fed_mutation_log rename drift —
tables rebuilt to 0000 snapshot, drizzle then applies 0001–0004
successfully to reach the current target schema; (C) post-migration-
correct (production-like) — heal no-op, drizzle no-op, schema
unchanged. Live boot on my actual dev DB: migrations complete
silently, server binds :3005, no outbox worker errors. Server tests
110/110, web 131/131, typecheck clean.
No migration files changed. Deployed Pi+VM instances are unaffected
(their schema matches the snapshot exactly — heal won't touch
anything).
Closes backlog #30.
Jannis Braun