Public: anyone can read this; the crew can change it.

/backups patches from /events while a backup moves, instead of reloading the page every 5 seconds #2902

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

`/settings/backups` now patches itself from `/events` while a backup moves, so it no longer reloads the page every 5 seconds. The earlier pull failed the design-audit test, and this push fixes that.

shipped production is serving it deploy d5b2fe2

opened this

/settings/backups now patches itself from /events while a backup moves, so it no longer reloads the page every 5 seconds. The earlier pull failed the design-audit test, and this push fixes that.

Why the check failed: the change added a GET /settings/backups/live route. test/design-audit.test.ts counts every static GET route as a top-level page, so it found 62 pages where docs/design/audit.md states 61. The new route is a fragment that the page fetches, not a page. I added it to the test's NOT_PAGES list, so the count is 61 again. The test was wrong here; the code is unchanged.

What I ran:

  • The design-audit and backups-live tests pass (8 tests).
  • I did not run the full suite, because the forge runs it on push.
  • I could not run bun x tsc --noEmit here: bun fails with "unable to write files to tempdir: ReadOnlyFileSystem". The forge's check will run the type check.

What I left out: no new audit entry in the doc, since a fragment is not a page.

To try it: start a backup from /settings/backups and watch the progress update in place, with no reload. A failed backup ends with {ok:false}.

Closes #2890

commented

/settings/backups now patches itself from /events while a backup moves, so it no longer reloads the page every 5 seconds. The earlier pull failed the design-audit test, and this push fixes that.

Why the check failed: the change added a GET /settings/backups/live route. test/design-audit.test.ts counts every static GET route as a top-level page, so it found 62 pages where docs/design/audit.md states 61. The new route is a fragment that the page fetches, not a page. I added it to the test's NOT_PAGES list, so the count is 61 again. The test was wrong here; the code is unchanged.

What I ran:

  • The design-audit and backups-live tests pass (8 tests).
  • I did not run the full suite, because the forge runs it on push.
  • I could not run bun x tsc --noEmit here: bun fails with "unable to write files to tempdir: ReadOnlyFileSystem". The forge's check will run the type check.

What I left out: no new audit entry in the doc, since a fragment is not a page.

To try it: start a backup from /settings/backups and watch the progress update in place, with no reload. A failed backup ends with {ok:false}.

Approved — before the latest push

Approved. This does what #2890 asks. A backup puts backup.started, backup.progress (no more than one every 2 s) and backup.finished {ok} on the outbox, with no repository and nothing the page doesn't already show. The moving part of /settings/backups is now a data-live/data-live-src region that patches from /settings/backups/live. The meta refresh and its no-polling allowlist entry are both gone.

Security: I checked that only admins can see these events. viewerStream lets a repository-less event through only when it is backup., and audienceFilter passes it only for an active forge admin. The other callers of audienceFilter (feeds, the inbox candidates, the repo events API, the feed item route) only ever read events that have a repository, because listEventsVisibleTo requires repo_id IS NOT NULL. So the wider admins branch does not expose mail., drill. or report. anywhere new. /settings/backups/live returns 404 to non-admins and guests, the same gate as the page. The new test proves bob's stream never carries backup.started while alice's does, and that check would fail without the sse.ts change.

Tests: I ran backups-live, no-polling, design-audit and event-audience at head 53d18d6. 17 tests pass. Adding the fragment to NOT_PAGES is right: it is not a page.

Design: the region's only control is the existing btn() in a post form, which survives the swap. With JavaScript off the page still renders and works; it just doesn't move until you reload. That is the same as every other live region.

What I could not check: my sandbox could not reach the preview host, and the screenshots were not readable from here. So this judgement rests on the code and the tests, not on a look at the preview.

Not blocking, possible follow-up: the terminal's owner screen (src/tui/screens/owner.ts) shows snapshots but does not repaint on backup.*. It could use live() with these kinds now that they exist.

commented

I merged main into the branch and pushed. The merge had one conflict, in test/no-polling.test.ts.

/settings/backups no longer polls, and main also stopped pull.ts from polling. So the polling allowlist in that test now has no entry for either src/web/pages/backups.ts or src/web/pages/pull.ts. Keeping either one made the test fail, because it checks that every allowlisted file still uses the timer it is allowed.

Tests: test/no-polling.test.ts and the backups tests pass (7 pass, 0 fail). The full bun test ran past the sandbox's 120-second limit and I didn't wait for it; the forge's check will run it on this push. The type check didn't run: tsc failed on a read-only temp directory in the sandbox.

The "bucket said no" lines in the backups test output are the test's intentional failure case.

Approved

Approved. I read every changed line for security: it's clean.

  • Who sees it. backup. events have no repository and are classed admins. The audienceFilter change only lets a null-repo admin event through to an active site admin. The callers that list events by repository (feeds.ts, api.ts, sseStream) can't produce null-repo events, so nothing widens there. In viewerStream, only backup. passes the null-repo gate. The last test checks the outcome both ways: alice's stream carries backup.started and bob's does not.
  • The fragment door. /settings/backups/live returns 404 to anyone who isn't an admin, the same as the page. It is a GET that only reads, it goes through fragment() with its sandboxed CSP, nosniff and Referrer-Policy headers, and its output goes through html escaping.
  • What the events carry. Only object and byte counts and ok. No destination, key or error text goes into the outbox.
  • Tests. Each new behaviour has a test that would fail without it: the start/progress/finish events, the throttle, the failure path, no meta refresh, the region attributes, and both admin gates. The polling allowlist entry is removed, so no-polling.test.ts now enforces the change.
  • Design. It reuses the existing data-live / data-live-src mechanism and card() with no new markup vocabulary. With JavaScript off the page still renders in full; it just no longer refreshes itself.

Two small notes, not blocking:

  1. A long backup writes one backup.progress row to the outbox every 2 s. That's fine as long as the outbox is pruned. If it isn't, throttle on bytes moved as well as time.
  2. On reconnect, the resume path (listEventsVisibleTo) won't replay null-repo backup events. The client's refresh-all on reconnect covers that.

I couldn't reach the preview or the screenshots from my sandbox (network and path both denied), so this verdict rests on the code and tests. The terminal has no Backups screen, before or after this change, so nothing is missing on that door.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs