diff --git a/SPEC.md b/SPEC.md index 57d8a43..f3e2016 100644 --- a/SPEC.md +++ b/SPEC.md @@ -714,6 +714,47 @@ The lighter half ships the structural shape — frontmatter, consent, resolution, revocation. The heavier half ships the runtime hardening. +### 6.8 Sign-in state resume (roadmap item #29, v0.23.0) + +Signing in lands the user back on their most recently-viewed app +state instead of always on the empty-state home view. The model is +**per-user**, not per-device — the safe default the roadmap calls +for: a sign-in on any device resumes the most-recently-recorded +route. A per-device split and a profile-settings toggle UI are +follow-ups; v0.23.0 ships the column-level opt-out flag +(`resume_enabled`, default on) and the default-on behavior. + +**Storage.** A single row per user in `user_session_state` +(migration 022): `user_id` (PK, FK → `users.id`, cascade-delete), +`last_route` (TEXT, the frontend pathname), `last_route_state` +(TEXT, JSON-encoded — SQLite has no native JSONB, so JSON-as-TEXT +matches how the app stores every other JSON blob), `resume_enabled` +(INTEGER, default 1), and `last_updated_at`. + +**Wiring.** A debounced (~1s) frontend route-change hook posts the +current route to `PUT /api/me/last-state` for authenticated users +(anonymous: no-op). The stored `last_route` is folded onto the +existing `/api/auth/me` payload (no extra round-trip); on sign-in, +after the §21-Part-C Amplitude `identify` fires, the frontend +`navigate()`s to it. The identify-then-redirect ordering is +preserved: the redirect is gated on identify having fired. + +**Stale state.** If the stored route is an RFC since withdrawn or +one the user lost rights to read, the redirect is a graceful no-op: +`navigate(last_route)` lands on whatever that route renders today, +and the existing routing already falls through to the +catalog/empty-state for a missing/unreadable RFC. No special-casing +on the server. + +**Privacy (binding).** The stored state is **route + light view +state ONLY** — scroll anchors, open-tab selection, filter chips, and +the like. It **MUST NOT** carry draft-buffer contents, PR/comment +draft text, or any user-typed content. The frontend never sends such +content; the `last_route_state` column comment in migration 022 and +this paragraph are the contract. Resume state is purposely cheap to +discard: a deliberate "clear" (or `resume_enabled = 0`) drops the +user back to today's empty-state behavior. + --- ## 7. The left pane diff --git a/backend/app/api.py b/backend/app/api.py index 09a66bc..32756fa 100644 --- a/backend/app/api.py +++ b/backend/app/api.py @@ -67,6 +67,18 @@ class FunderCredentialBody(BaseModel): api_key: str = Field(min_length=1, max_length=2048) +class LastStateBody(BaseModel): + # v0.23.0 / roadmap item #29: server-side sign-in state resume. + # `route` is a frontend pathname the user was last on (bounded so a + # hostile client can't stuff arbitrary blobs through). `state` is an + # optional bag of *light* view state (scroll anchors, open tab, + # filter chips). PRIVACY: it MUST NOT carry draft-buffer contents — + # the frontend only ever sends ephemeral view state, and the column + # comment in migration 022 + SPEC §6.2 are the binding contract. + route: str = Field(min_length=1, max_length=2048) + state: dict[str, Any] | None = None + + class BetaRequestBody(BaseModel): # v0.8.0 — captured on the first OTC sign-in. All three fields are # required so the admin queue has a coherent triage shape. @@ -354,6 +366,28 @@ def make_router( ) has_passcode = bool(row and row["passcode_hash"]) passcode_set_at = row["passcode_set_at"] if (row and has_passcode) else None + # v0.23.0 / item #29: fold the sign-in-resume state onto the + # same round-trip the frontend already makes on boot. When + # resume is disabled (resume_enabled = 0) we hand back a null + # route so the client never redirects; the stored row stays put + # so re-enabling later resumes the last-known route. + state_row = db.conn().execute( + "SELECT last_route, last_route_state, resume_enabled " + "FROM user_session_state WHERE user_id = ?", + (user.user_id,), + ).fetchone() + resume_enabled = bool(state_row["resume_enabled"]) if state_row else True + last_route = ( + state_row["last_route"] + if (state_row and resume_enabled) + else None + ) + last_route_state = None + if state_row and resume_enabled and state_row["last_route_state"]: + try: + last_route_state = json.loads(state_row["last_route_state"]) + except (ValueError, TypeError): + last_route_state = None return { "authenticated": True, "user": { @@ -370,6 +404,10 @@ def make_router( "needs_profile": needs_profile, "has_passcode": has_passcode, "passcode_set_at": passcode_set_at, + # v0.23.0 / item #29 — sign-in state resume. + "resume_enabled": resume_enabled, + "last_route": last_route, + "last_route_state": last_route_state, }, } @@ -439,6 +477,52 @@ def make_router( notify.fan_out_new_beta_request(requester_user_id=user.user_id) return {"ok": True} + # --------------------------------------------------------------- + # v0.23.0 (§6.2, roadmap item #29): server-side sign-in state + # resume. The frontend debounce-posts the user's current route + + # a small bag of light view state here on every route change; the + # next sign-in reads `last_route` off `/api/auth/me` and redirects. + # + # Per-user (NOT per-device) — one row per user, keyed on user_id. + # `resume_enabled` is the opt-out flag (default on); when it's 0 + # this endpoint no-ops so a user who turned resume off doesn't keep + # silently rewriting their stored route. PRIVACY: the body carries + # route + light state ONLY, never draft-buffer contents (migration + # 022 column comment + SPEC §6.2 are the binding contract). + # + # `require_user` (not `require_contributor`) — a pending/granted + # distinction is irrelevant for "remember where I was", and a + # pending user navigating read-only surfaces should still resume. + # Anonymous callers get the 401 `require_user` raises. + # --------------------------------------------------------------- + + @router.put("/api/me/last-state") + async def put_last_state(body: LastStateBody, request: Request) -> dict[str, Any]: + user = auth.require_user(request) + # Respect the opt-out: if a row already exists with resume + # disabled, leave it untouched and report the no-op. A first- + # ever POST (no row yet) defaults to enabled and stores. + existing = db.conn().execute( + "SELECT resume_enabled FROM user_session_state WHERE user_id = ?", + (user.user_id,), + ).fetchone() + if existing is not None and not existing["resume_enabled"]: + return {"ok": True, "stored": False} + state_json = json.dumps(body.state) if body.state is not None else None + db.conn().execute( + """ + INSERT INTO user_session_state + (user_id, last_route, last_route_state, last_updated_at) + VALUES (?, ?, ?, datetime('now')) + ON CONFLICT(user_id) DO UPDATE SET + last_route = excluded.last_route, + last_route_state = excluded.last_route_state, + last_updated_at = excluded.last_updated_at + """, + (user.user_id, body.route, state_json), + ) + return {"ok": True, "stored": True} + # --------------------------------------------------------------- # v0.11.0: trust device for 30 days (§6.2, roadmap item #9). # diff --git a/backend/migrations/022_user_session_state.sql b/backend/migrations/022_user_session_state.sql new file mode 100644 index 0000000..6ec7936 --- /dev/null +++ b/backend/migrations/022_user_session_state.sql @@ -0,0 +1,54 @@ +-- v0.23.0 / roadmap item #29: server-side sign-in state resume. +-- +-- Track each authenticated user's last-viewed route + a small bag of +-- "light" component state so that the *next* sign-in can land the user +-- back where they left off, rather than always dropping them on the +-- empty-state home view. +-- +-- Storage shape (one row per user — per-user, NOT per-device, per the +-- #29 "safe default"): +-- +-- * `user_id` — PRIMARY KEY and FK into users(id) with cascade on +-- delete. INTEGER to match users.id (INTEGER PRIMARY KEY +-- AUTOINCREMENT). A deleted user automatically loses their stored +-- resume state. One row per user means a later sign-in on any +-- device resumes the most-recently-recorded route — the per-user +-- model the roadmap asks for. +-- +-- * `last_route` — the frontend pathname the user was last on +-- (e.g. "/rfc/open-human-model"). TEXT, nullable until the first +-- route-change POST lands. NEVER contains draft-buffer contents — +-- it is a route only. See SPEC §6.2 "Sign-in state resume +-- (privacy)". +-- +-- * `last_route_state` — a JSON-encoded bag of *light* component +-- state (scroll anchors, open-tab selection, filter chips, etc.). +-- SQLite has no native JSONB; we store JSON as TEXT exactly as the +-- rest of the app stores its JSON blobs (json.dumps / json.loads, +-- cf. permission_events.details, actions.details). Nullable. +-- PRIVACY INVARIANT: this column MUST NOT carry draft-buffer text, +-- PR bodies, comment drafts, or any user-typed content — only +-- ephemeral view state safe to replay. The PUT handler is the +-- enforcement point; the column comment is the contract. +-- +-- * `resume_enabled` — the per-user opt-out flag. 1 (default) means +-- "resume me where I left off"; 0 means "always land on home". The +-- PUT handler no-ops the upsert when this is 0, and the read path +-- refuses to hand back a stored route when this is 0. A +-- profile-settings toggle UI to flip this is a follow-up (the +-- column + default-on behavior ship now); see CHANGELOG v0.23.0. +-- +-- * `last_updated_at` — TEXT timestamp, app convention +-- `datetime('now')`, matching device_trust.last_seen_at / +-- users.last_seen_at. Refreshed on every successful upsert. +-- +-- No new env vars. The debounce interval for the frontend route-change +-- POST is a frontend constant (~1s), not a server knob. + +CREATE TABLE user_session_state ( + user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE, + last_route TEXT, + last_route_state TEXT, -- JSON-encoded light state, nullable + resume_enabled INTEGER NOT NULL DEFAULT 1, + last_updated_at TEXT NOT NULL DEFAULT (datetime('now')) +); diff --git a/backend/tests/test_session_resume_vertical.py b/backend/tests/test_session_resume_vertical.py new file mode 100644 index 0000000..801fd97 --- /dev/null +++ b/backend/tests/test_session_resume_vertical.py @@ -0,0 +1,140 @@ +"""End-to-end integration tests for the v0.23.0 sign-in state-resume +vertical (§6.2, roadmap item #29). + +New behavior: each authenticated user's last-viewed route + a small bag +of light view state is tracked server-side, so the next sign-in can land +them back where they left off rather than on the empty-state home view. + +The tests below prove: + + * `PUT /api/me/last-state` requires auth — an anonymous client gets + 401, and nothing is stored. + * An authenticated PUT upserts the route, and the stored route is + read back for that user off `GET /api/auth/me` (`last_route`). + * A second PUT overwrites (upsert, one row per user) — the latest + route wins. + * `last_route_state` round-trips as decoded JSON on `/api/auth/me`. + * Per-user isolation: user A's stored route is not visible to user B. + * `resume_enabled = 0` disables resume: the PUT no-ops (does not + rewrite the stored route) and `/api/auth/me` hands back a null + `last_route` even though a stored row exists. + +The fakes from `test_propose_vertical` give us a working app harness + +the `sign_in_as` / `provision_user_row` seams. +""" +from __future__ import annotations + +from test_propose_vertical import ( # noqa: F401 + FakeGitea, + app_with_fake_gitea, + provision_user_row, + sign_in_as, + tmp_env, +) + + +def test_put_last_state_requires_auth(app_with_fake_gitea): + from fastapi.testclient import TestClient + from app import db + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + # Anonymous — no session cookie set. + r = client.put("/api/me/last-state", json={"route": "/rfc/open-human-model"}) + assert r.status_code == 401 + # Nothing landed in the table. + row = db.conn().execute("SELECT COUNT(*) AS n FROM user_session_state").fetchone() + assert row["n"] == 0 + + +def test_put_last_state_upserts_and_reads_back(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="alice", role="contributor") + sign_in_as(client, user_id=2, gitea_login="alice", display_name="Alice", role="contributor", email="alice@test") + + # First POST stores a route + light state. + r = client.put( + "/api/me/last-state", + json={"route": "/rfc/open-human-model", "state": {"tab": "discussion", "scroll": 420}}, + ) + assert r.status_code == 200 + assert r.json()["stored"] is True + + # /api/auth/me hands the route + decoded state back. + me = client.get("/api/auth/me").json() + assert me["authenticated"] is True + assert me["user"]["resume_enabled"] is True + assert me["user"]["last_route"] == "/rfc/open-human-model" + assert me["user"]["last_route_state"] == {"tab": "discussion", "scroll": 420} + + # A later POST overwrites — one row per user, latest wins. + r = client.put("/api/me/last-state", json={"route": "/proposals/7"}) + assert r.status_code == 200 + me = client.get("/api/auth/me").json() + assert me["user"]["last_route"] == "/proposals/7" + # state was omitted on the second POST → cleared to null. + assert me["user"]["last_route_state"] is None + + +def test_last_state_is_per_user(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="alice", role="contributor") + provision_user_row(user_id=3, login="bob", role="contributor") + + sign_in_as(client, user_id=2, gitea_login="alice", display_name="Alice", role="contributor", email="alice@test") + client.put("/api/me/last-state", json={"route": "/rfc/alice-route"}) + + # Switch to bob — he has no stored route yet. + sign_in_as(client, user_id=3, gitea_login="bob", display_name="Bob", role="contributor", email="bob@test") + me = client.get("/api/auth/me").json() + assert me["user"]["last_route"] is None + + client.put("/api/me/last-state", json={"route": "/rfc/bob-route"}) + me = client.get("/api/auth/me").json() + assert me["user"]["last_route"] == "/rfc/bob-route" + + # Back to alice — her route is untouched by bob's write. + sign_in_as(client, user_id=2, gitea_login="alice", display_name="Alice", role="contributor", email="alice@test") + me = client.get("/api/auth/me").json() + assert me["user"]["last_route"] == "/rfc/alice-route" + + +def test_resume_disabled_no_ops_put_and_hides_route(app_with_fake_gitea): + from fastapi.testclient import TestClient + from app import db + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="alice", role="contributor") + sign_in_as(client, user_id=2, gitea_login="alice", display_name="Alice", role="contributor", email="alice@test") + + # Seed a stored row, then flip resume_enabled off directly (the + # profile-settings toggle UI to do this from the client is a + # follow-up; the column + behavior ship now). + client.put("/api/me/last-state", json={"route": "/rfc/before-disable"}) + db.conn().execute( + "UPDATE user_session_state SET resume_enabled = 0 WHERE user_id = ?", + (2,), + ) + + # /api/auth/me reports resume off and hands back a null route + # even though a stored row exists. + me = client.get("/api/auth/me").json() + assert me["user"]["resume_enabled"] is False + assert me["user"]["last_route"] is None + + # A PUT while disabled no-ops: stored=False and the stored route + # is NOT rewritten. + r = client.put("/api/me/last-state", json={"route": "/rfc/after-disable"}) + assert r.status_code == 200 + assert r.json()["stored"] is False + row = db.conn().execute( + "SELECT last_route FROM user_session_state WHERE user_id = ?", (2,) + ).fetchone() + assert row["last_route"] == "/rfc/before-disable" diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index e7dfb88..47c4a96 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -2,6 +2,7 @@ import { useEffect, useRef, useState } from 'react' import { Routes, Route, Link, Navigate, useLocation, useNavigate } from 'react-router-dom' import { getMe, subscribeToNotifications } from './api' import { anonymize, EVENTS, identify, track } from './lib/analytics' +import { useLastState } from './lib/useLastState' import Catalog from './components/Catalog.jsx' import Inbox from './components/Inbox.jsx' import RFCView from './components/RFCView.jsx' @@ -42,6 +43,12 @@ export default function App() { // "Privacy & cookies" tab dispatches a `rfc-app:cookie-consent-reopen` // event that bumps this. const [consentReopenTick, setConsentReopenTick] = useState(0) + // v0.23.0 / item #29 — flips true once the #21-Part-C identify effect + // has fired (or once we've confirmed there's no authenticated user to + // identify). useLastState gates its resume redirect on this so the + // redirect always happens AFTER identify, preserving identify-then- + // track ordering. + const [identifyReady, setIdentifyReady] = useState(false) const navigate = useNavigate() const location = useLocation() // v0.15.0 — Page Viewed event taxonomy. We fire on every @@ -86,6 +93,9 @@ export default function App() { if (viewer?.first_sign_in_at) props.first_sign_in_at = ['__setOnce__', viewer.first_sign_in_at] if (viewer?.created_at) props.account_created_at = ['__setOnce__', viewer.created_at] identify({ user_id: String(uid), properties: props }) + // v0.23.0 / item #29 — identify has now fired for this sign-in; + // release useLastState's resume redirect (it waits on this). + setIdentifyReady(true) } else if (uid == null && lastUserIdRef.current != null) { // Sign-out edge — App-level reset is handled separately by the // sign-out gesture that fires User Signed Out. Clear our local @@ -94,6 +104,14 @@ export default function App() { } }, [me?.authenticated, me?.user?.id, me?.user?.role, me?.user?.permission_state, me?.user?.passcode_set, me?.user?.device_trusted]) + // v0.23.0 / item #29 — once `me` has resolved, if there's no + // authenticated user there is nothing to identify, so release the + // resume gate immediately (anonymous boots have no resume to do, but + // the hook still needs the gate resolved to be a clean no-op). + useEffect(() => { + if (me != null && !me.authenticated) setIdentifyReady(true) + }, [me]) + useEffect(() => { const handler = () => setConsentReopenTick(t => t + 1) window.addEventListener('rfc-app:cookie-consent-reopen', handler) @@ -107,6 +125,18 @@ export default function App() { .finally(() => setLoading(false)) }, []) + // v0.23.0 / item #29 — server-side sign-in state resume. The hook + // debounce-posts the current route for authenticated users and, once + // identify has fired, redirects a fresh sign-in (which hard-lands on + // "/") to the user's stored last route. Anonymous users: no-op. + useLastState({ + authenticated: !!me?.authenticated, + pathname: location.pathname, + identifyReady, + lastRoute: me?.authenticated ? me.user?.last_route : null, + navigate, + }) + // §15.3 — subscribe to the live SSE stream for authenticated viewers // so the badge counter and the toast surface stay in lockstep with // the inbox. Tabs that miss an event because they were closed pick diff --git a/frontend/src/api.js b/frontend/src/api.js index 27e6881..7b7b174 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -25,6 +25,25 @@ export async function getMe() { return jsonOrThrow(res) } +// ── v0.23.0: sign-in state resume (§6.2, roadmap item #29) ─────────────── +// +// The route-change hook (useLastState) debounce-posts the user's current +// route + a small bag of *light* view state here for authenticated users. +// The next sign-in reads `last_route` off `/api/auth/me` and redirects. +// Privacy: `state` carries ephemeral view state ONLY — never draft-buffer +// contents (see SPEC §6.2). Best-effort: callers ignore failures (an +// offline/401 POST must never disrupt navigation). +export async function putLastState(route, state) { + const body = { route } + if (state != null) body.state = state + const res = await fetch('/api/me/last-state', { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }) + return jsonOrThrow(res) +} + // ── v0.7.0: email + one-time-code sign-in (§6.2) ───────────────────────── // // The legacy /auth/login → /auth/callback OAuth flow remains during the diff --git a/frontend/src/lib/useLastState.js b/frontend/src/lib/useLastState.js new file mode 100644 index 0000000..394ee55 --- /dev/null +++ b/frontend/src/lib/useLastState.js @@ -0,0 +1,91 @@ +// useLastState — v0.23.0 / roadmap item #29: server-side sign-in state +// resume. Two responsibilities, kept in one small hook so App.jsx's +// surface stays minimal: +// +// 1. Debounce-post the authenticated user's current route to +// `PUT /api/me/last-state` on every route change (~1s debounce so +// a fast click-through doesn't spray the endpoint). Anonymous +// users: no-op. Best-effort: a failed POST never disrupts nav. +// +// 2. On the first authenticated load, read the stored `last_route` +// (handed back on `/api/auth/me`) and `navigate()` to it ONCE. +// Stale routes (a withdrawn RFC, a slug the user lost rights to) +// are not special-cased here: navigating lands on whatever that +// route renders today, and the existing routing already falls +// through to the catalog/empty-state for a missing RFC. Keeping +// this dumb is deliberate (see SPEC §6.2 + the #29 scope note). +// +// Ordering with #21 Part C (Amplitude identify): App.jsx fires +// `identify` in its own effect when `me.user.id` first appears. This +// hook's resume redirect is gated on `identifyReady` — App.jsx flips it +// true only after the identify effect has run — so the redirect always +// happens AFTER identify, preserving the identify-then-track ordering +// the roadmap calls out. +import { useEffect, useRef } from 'react' +import { putLastState } from '../api' + +// ~1s debounce on the route-change POST. A frontend constant, not a +// server knob — the backend takes whatever lands. +const DEBOUNCE_MS = 1000 + +// Routes we never want to resume *to* — auth/landing surfaces that +// would be nonsensical or hostile to drop a returning user onto. We +// still record them (cheap, and the user may legitimately be sitting on +// /docs), but the resume redirect skips them and falls through to the +// default landing. Anything not listed resumes normally. +const NON_RESUMABLE_PREFIXES = ['/login', '/welcome', '/invites/', '/invitations/'] + +function isResumable(route) { + if (!route || typeof route !== 'string') return false + if (route === '/') return false // "/" is already the default landing + return !NON_RESUMABLE_PREFIXES.some(p => route.startsWith(p)) +} + +/** + * @param {object} opts + * @param {boolean} opts.authenticated whether a viewer is signed in + * @param {string} opts.pathname current location.pathname + * @param {boolean} opts.identifyReady App flips true after identify fires + * @param {string|null} opts.lastRoute stored route from /api/auth/me + * @param {function} opts.navigate react-router navigate() + */ +export function useLastState({ authenticated, pathname, identifyReady, lastRoute, navigate }) { + // ── 1. Debounced route-change POST ──────────────────────────────── + const timerRef = useRef(null) + useEffect(() => { + if (!authenticated) return undefined + if (timerRef.current) clearTimeout(timerRef.current) + timerRef.current = setTimeout(() => { + // Best-effort — swallow failures so an offline/401 POST never + // surfaces as a navigation error. + putLastState(pathname).catch(() => {}) + }, DEBOUNCE_MS) + return () => { + if (timerRef.current) clearTimeout(timerRef.current) + } + }, [authenticated, pathname]) + + // ── 2. One-time resume redirect ─────────────────────────────────── + // Fires once, after identify is ready, when there's a resumable + // stored route AND the user is currently sitting on the default + // landing ("/"). We only redirect from "/" so we never yank a user + // who deep-linked somewhere specific (or refreshed mid-RFC) back to + // their last route. + const resumedRef = useRef(false) + useEffect(() => { + if (resumedRef.current) return + if (!authenticated || !identifyReady) return + // Only resume when the app booted on the default landing — a hard + // sign-in nav lands on "/", which is exactly the case we want. + if (pathname !== '/') { + resumedRef.current = true // user deep-linked; don't resume later either + return + } + if (isResumable(lastRoute)) { + resumedRef.current = true + navigate(lastRoute, { replace: true }) + } else { + resumedRef.current = true // nothing to resume to; keep default landing + } + }, [authenticated, identifyReady, lastRoute, pathname, navigate]) +}