spawnInstance now sets DISABLE_RATE_LIMITS=1 by default so unrelated tests
don't exhaust the shared 127.0.0.1 per-IP bucket. bootHomePlusRemotes/
bootTwoInstances accept an optional { enableRateLimits } that omits the env
for tests that need real enforcement, exposed via the explicit
bootTwoInstancesWithRateLimits() helper for Test #15 (rate-limit assertion).
Overloading NODE_ENV='test' to silently disable rate limiting layered a
second meaning onto an env that already gates the test-only seed-peer
route. A dedicated DISABLE_RATE_LIMITS env (envBool semantics, matching
DISABLE_FEDERATION_WORKERS) makes intent explicit, defaults off in
production, and leaves room for tests that need to assert real rate-limit
behaviour to opt back in by simply not setting the var.
Also fixes per-IP rate limit exhaustion in test environments: @fastify/rate-limit
v9 has no skip(); use allowList(() => NODE_ENV==='test') which propagates to
per-route overrides via mergeParams Object.assign merge.
The original peerInstances comment framed the two-row pattern as a
band-aid for getOurOrigin's https://${DOMAIN} default. After
investigating a clean collapse to one row (PUBLIC_ORIGIN override on
each spawned instance), the deeper coupling surfaces:
- extractDomain() strips port via new URL().hostname, so unique-port
localhost instances all share hostname '127.0.0.1' and the
receiver's attribution guard
extractDomain(user.homeInstance) === extractDomain(fedHeaders.origin)
becomes ambiguous in any multi-remote configuration.
- The homeInstance validator regex /^[a-zA-Z0-9._-]+$/ in auth.ts
rejects ':', so the port cannot be encoded into homeInstance to
disambiguate.
- Eliminating the second row would require a production refactor of
extractDomain (port-preserving), the attribution check (decoupled
from URL), or the homeInstance validator (allow ':') — all out of
scope.
So the harness DELIBERATELY keeps DOMAIN as a per-instance human label
('home.test.local' / 'remoteN.test.local') for stable identity, and the
two peer rows per direction (transport URL + getOurOrigin URL) are
structural to localhost-port test reality, not a band-aid. Comment
rewritten to reflect this. PUBLIC_ORIGIN remains available in
production code for reverse-proxy / dev-without-TLS deployments.
Adds an explicit override for the federation transport URL returned by
getOurOrigin(). When unset, behaviour is unchanged (https://${DOMAIN} ->
http://localhost:${PORT} dev fallback). Intended for reverse-proxy /
dev-without-TLS deployments where the public origin must be advertised
explicitly (typically http://...) and differs from the bare DOMAIN
value used for federated identity.
Wired via config.publicOrigin (envOptional('PUBLIC_ORIGIN')) so the
override flows through the existing config layer rather than scattering
process.env reads. Trailing slash is stripped for symmetry with
peer.origin storage.
docs/systems/federation.md gets a "Public Origin Override" subsection
under §14 Background Workers documenting the resolution order.
Adds setupFullDeletionFixture (federated user joins remote space, authors 2
messages with reactions, opens 1-on-1 DM with a live observer). Tests #3 (soft
mode: tombstone + dm_members cleared, messages/reactions retained) and #5 (full
mode: tombstone + messages/reactions purged, surviving 1-on-1 DM channel).
Also fixes seedPeer to install both the URL form (outbound lookup on sender)
and the DOMAIN-claim form (inbound auth on receiver) — required because the
test harness's ephemeral http://127.0.0.1 origin and DOMAIN-derived
getOurOrigin() return different strings, while production has them coincide.
This was latent: test #1 (leave mode) skips S2S, so #3 was the first test to
actually exercise the S2S delete path and surfaced the dual-origin gap.
Also fix dbInspect.ts UserRow column aliases: SELECT * returns snake_case
columns (is_deleted, display_name, etc.) but UserRow expected camelCase.
Switch to explicit aliased SELECT so all callers get the documented interface.
Electron derived userData from package.json's `@backspace/desktop` name, leaking
the monorepo's pnpm scope into ~/Library/Application Support/. Now `app.setName`
runs at module load before any userData consumer, and a one-shot migration
atomically moves the historical folder to <appData>/Backspace, cleaning the
empty @backspace/ parent. Conservative on conflict — never clobbers an existing
populated target. EXDEV fallback to recursive copy. Smoke-recovery path flipped
back to Backspace.
CRITICAL BUG. In real SPAs, useEffect fires during document load (microtask
after bundle execute + React render), which is BEFORE did-finish-load fires
(after window.onload). Without this fix, the ping arrived when bootArmed=false
(no-op), then did-finish-load armed a timer nothing would clear → 20s later
every successful packaged build falsely entered recovery.
Caught by smoke scenario 13 (positive control: page that DOES ping should NOT
recover). The smoke proved the page's script ran AND the ping was sent, yet
recovery still fired.
Fix: module-level pingReceivedThisNav flag, reset on did-navigate, set in
handleRendererReady, checked in armBootTimer (early-return if true). Late-ping
case (ping after arm) preserved via existing 'if (bootArmed) clearBootTimer()'.
Also exports resetBootTimerStateForTest() to ensure full module-state isolation
between tests (pingReceivedThisNav is module-level and must not bleed across
test cases in the same run).
3 new tests pin the early-ping, late-ping, and per-nav persistence semantics.
48/48 tests pass. Build clean.
Spec + docs updated.
- App.tsx and main.tsx ErrorBoundary gate rendererReady() on
VITE_FORCE_BOOT_STALL build env var. When set, the ping is suppressed
so the main-process boot timer fires (exercising the renderer-stalled
recovery path without hand-editing source).
- vite-env.d.ts declares the env var type so TS doesn't complain.
- scripts/smoke-recovery.sh automates scenarios 2 (bad URL → load-failed)
and 4 (forced stall → renderer-stalled), grepping stderr for the
[recovery] entered: ... lines added by the prior commit. Backs up and
restores the user's instance-url.json. Manual scenarios documented in
the script header.
UX bug found during smoke testing: clicking Change Instance immediately
deleted the saved instance URL and showed an empty picker, with no way
back if the user changed their mind.
Fix:
- Don't clearInstanceUrl() in recovery action 'change-instance' — picker
is now non-destructive
- Picker pre-fills the input with the current saved URL when present
- Cancel button (shown only when a saved URL exists) returns to current
instance via idempotent setInstanceUrl re-save
- Header copy switches to 'Switch instance' / 'Cancel to stay' framing
when a saved URL is present
- URL only overwrites on explicit Connect to a different instance
Also: add console.log enter/exit lines in enterRecoveryMode and the
clear-recovery-state action handlers, so smoke-test scripts can grep
stderr for recovery activity without UI introspection.
Spec + docs/systems/desktop.md updated.
Two real bugs from final review:
- Clear recovery state on window 'closed' so macOS dock-activate doesn't
drop the recovery surface (window recreated with stale recoveryStore.mode)
- Hoist setOnQuitRequested before createWindow so synchronous boot failures
reach a wired Quit handler
Three polish items:
- Tray's Change Instance now routes through handleRecoveryAction so both
paths share one implementation; recovery action's change-instance also
show()+focus() for hidden-window tray clicks
- install-update action guards against state.updateState !== 'downloaded'
(defense in depth against malicious or buggy renderers)
- Object.freeze rationale documented in RecoveryStateStore.update
Eliminates the brief "Version: loading…" flash in the diagnostic block
on first render. Resolves getRecoveryState + getVersion in parallel via
Promise.all so the first render call has both. Drops the redundant
module-load .then() that set cachedVersion before render ran.
App.tsx useEffect signals boot-completion (success path). ErrorBoundary
componentDidCatch signals it on the caught-error path so the in-app
error UI isn't overridden by native recovery 20s later.
- rendererReady (boot-completion ping)
- getRecoveryState / onRecoveryStateChanged (recovery.html subscribers)
- recoveryAction (button click dispatcher with enum action)
Type declarations kept ambient (no export) to preserve window.backspace
global augmentation — exporting from an ambient .d.ts converts it to a
module and breaks the Window interface extension.
buildAppMenuTemplate's actions param is Partial<MenuActions>, so the
tray-only callbacks (onShow/onHide/onQuit) are simply ignored. Removes
the three-callback duplication between the two objects without changing
behavior. Future onChangeInstance/onCheckForUpdates/onRestartToInstall
changes only need to be made in one place.
- setMainWindow on createWindow, setMainWindow(null) on closed
- attachRecoveryHandlers wires Electron unresponsive/crash/load-fail events
- Store subscriber drives tray context menu + macOS app menu + mode-gated
recovery-state-changed push to renderer; single applyMenusForState
function shared between subscriber and initial fire (no drift)
- Old hard-coded createTray Menu and macOS app-menu construction deleted;
Win/Linux Edit-only menu retained as one-time setup for keyboard accelerators
- requestQuit exported, wired via setOnQuitRequested callback
- Recovery IPC handlers: renderer-ready, recovery-action, get-recovery-state
- setAppUserModelId('com.backspace.desktop') for Win32 notification attribution
- Extended showNotification with optional onClick (existing 2-arg callers unchanged)
Adds buildAppMenuTemplate pure function to recovery.ts that produces the
three-submenu macOS app menu (App/Edit/Window), reusing MenuActions and
checkForUpdatesItem from T4. Includes Restart to Install Update item
conditionally on updateState=downloaded. Tests use destructuring to satisfy
noUncheckedIndexedAccess. 26 tests pass, tsc clean.
Pure buildTrayMenuTemplate function returns MenuItemConstructorOptions[]
without constructing real Menu objects, enabling full test coverage.
All 7 new tests pass (22 total); MenuActions interface and
checkForUpdatesItem helper are intentionally unexported.
- Snapshot listener set before notifying so subscribers can subscribe/
unsubscribe during notification without breaking the pass
- Per-callback try/catch so one throwing subscriber does not silence others
- Object.freeze on each state object so the live reference returned by
get() cannot be accidentally mutated externally (compile-time
Readonly<> is hint only)
- 3 new tests pinning these invariants
Jannis Braun