eleven / ocho Public

server+web: compare-and-swap on the nicknames blob (#652)

Same optimistic versioning as the mini-app state slots: GET returns
{blob, version}; POST carries the base version and 409s (with the
current version) when another device wrote in between — the web client's
journaled read-modify-write already retries over a fresh read, so
conflicts converge instead of clobbering. A version-less POST (a
pre-versioning page still open somewhere) keeps its unconditional write
and advances the counter. Additive migration.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Paul Campbell 4 months ago 0c07ba8ef1147a13594399b339219e60fe2a1031 parent 91599c9

4 files changed, +123 −13

db.go +31 −8
handlers_test.go +64 −0
main.go +19 −4
web/static/shell.js +9 −1

diff --git a/db.go b/db.go

index 7859bbb..1be5674 100644

--- a/db.go

+++ b/db.go

@@ -536,6 +536,10 @@ CREATE TABLE IF NOT EXISTS member_bots (

// blobs above, not even other members can open it; the server holds

// ciphertext either way.

"ALTER TABLE users ADD COLUMN nicknames_blob TEXT",

+ // Compare-and-swap counter for the blob above (same optimistic

+ // versioning as the mini-app state slots): a save from a stale read

+ // 409s instead of clobbering another device's write.

+ "ALTER TABLE users ADD COLUMN nicknames_version INTEGER NOT NULL DEFAULT 0",

// Open "community" groups (docs/community-and-tiers.md): a group any

// member can discover and self-join. visibility is 'invite' (default)

// or 'open'; open_key is the thread's AES key encrypted under the

@@ -849,18 +853,37 @@ func (db *DB) setUserDescriptionBlob(userID, blob string) error {

// setUserNicknamesBlob stores the member's per-viewer name overrides (issue

// #577), sealed to their OWN public key — readable only by their devices,

-// which share the keypair ("" clears it).

-func (db *DB) setUserNicknamesBlob(userID, blob string) error {

- _, err := db.sql.Exec("UPDATE users SET nicknames_blob = ? WHERE id = ? AND COALESCE(is_agent, 0) = 0", blob, userID)

- return err

+// which share the keypair ("" clears it). Optimistic versioning like the

+// mini-app state slots: baseVersion is what the writer last read; a stale base

+// writes nothing (ok=false, current version returned) so the caller re-reads,

+// replays its journaled ops, and retries. baseVersion < 0 skips the check —

+// the pre-versioning client's unconditional write, kept working on purpose.

+func (db *DB) setUserNicknamesBlob(userID, blob string, baseVersion int64) (int64, bool, error) {

+ q := "UPDATE users SET nicknames_blob = ?, nicknames_version = COALESCE(nicknames_version, 0) + 1 WHERE id = ? AND COALESCE(is_agent, 0) = 0"

+ args := []any{blob, userID}

+ if baseVersion >= 0 {

+ q += " AND COALESCE(nicknames_version, 0) = ?"

+ args = append(args, baseVersion)

+ }

+ res, err := db.sql.Exec(q, args...)

+ if err != nil {

+ return 0, false, err

+ }

+ n, err := res.RowsAffected()

+ if err != nil {

+ return 0, false, err

+ }

+ _, v, verr := db.userNicknamesBlob(userID) // where the row (or the winner) is now

+ return v, n > 0, verr

}

// userNicknamesBlob returns the member's own sealed nickname overrides ("" when

-// never set).

-func (db *DB) userNicknamesBlob(userID string) (string, error) {

+// never set) and the blob's version (0 until first write).

+func (db *DB) userNicknamesBlob(userID string) (string, int64, error) {

var blob string

- err := db.sql.QueryRow("SELECT COALESCE(nicknames_blob, '') FROM users WHERE id = ?", userID).Scan(&blob)

- return blob, err

+ var version int64

+ err := db.sql.QueryRow("SELECT COALESCE(nicknames_blob, ''), COALESCE(nicknames_version, 0) FROM users WHERE id = ?", userID).Scan(&blob, &version)

+ return blob, version, err

}

func (db *DB) userExists(id string) bool {

diff --git a/handlers_test.go b/handlers_test.go

index 4a3e55b..5662d23 100644

--- a/handlers_test.go

+++ b/handlers_test.go

@@ -1761,6 +1761,70 @@ func TestNicknamesRoundTrip(t *testing.T) {

}

+// TestNicknamesVersioning covers the compare-and-swap on the nicknames blob:

+// a stale writer 409s (told the current version so it can re-read and retry),

+// a fresh writer advances the version, and a version-less write (a

+// pre-versioning client) stays unconditional.

+func TestNicknamesVersioning(t *testing.T) {

+ ts, srv := newTestServer(t)

+ tok, _, _ := joinedMember(t, srv, "viewer")

+ get := func() (string, int64) {

+ r, err := http.Get(ts.URL + "/api/nicknames?token=" + tok)

+ if err != nil {

+ t.Fatalf("get: %v", err)

+ }

+ var body struct {

+ Blob string `json:"nicknames_blob"`

+ Version int64 `json:"version"`

+ }

+ json.NewDecoder(r.Body).Decode(&body)

+ return body.Blob, body.Version

+ }

+ post := func(payload string) (*http.Response, int64) {

+ r, err := http.Post(ts.URL+"/api/nicknames", "application/json", strings.NewReader(payload))

+ if err != nil {

+ t.Fatalf("post: %v", err)

+ }

+ var body struct {

+ Version int64 `json:"version"`

+ }

+ json.NewDecoder(r.Body).Decode(&body)

+ return r, body.Version

+ }

+ // Fresh row reads version 0; a CAS write from that base lands at 1.

+ if _, v := get(); v != 0 {

+ t.Fatalf("fresh version: want 0, got %d", v)

+ }

+ if r, v := post(`{"token":"` + tok + `","nicknames_blob":"one","version":0}`); r.StatusCode != http.StatusOK || v != 1 {

+ t.Fatalf("first cas write: want 200/v1, got %d/v%d", r.StatusCode, v)

+ }

+ // A second writer still holding base 0 (device B's stale read) conflicts —

+ // and is told where the row is now.

+ if r, v := post(`{"token":"` + tok + `","nicknames_blob":"stale","version":0}`); r.StatusCode != http.StatusConflict || v != 1 {

+ t.Fatalf("stale cas write: want 409/v1, got %d/v%d", r.StatusCode, v)

+ }

+ if blob, _ := get(); blob != "one" {

+ t.Fatalf("stale write clobbered the blob: %q", blob)

+ }

+ // Retrying from the fresh read succeeds.

+ if r, v := post(`{"token":"` + tok + `","nicknames_blob":"two","version":1}`); r.StatusCode != http.StatusOK || v != 2 {

+ t.Fatalf("retry cas write: want 200/v2, got %d/v%d", r.StatusCode, v)

+ }

+ // A pre-versioning client (no version field) writes unconditionally and

+ // still advances the counter, so CAS writers see its write as fresh state.

+ if r, v := post(`{"token":"` + tok + `","nicknames_blob":"legacy"}`); r.StatusCode != http.StatusOK || v != 3 {

+ t.Fatalf("legacy write: want 200/v3, got %d/v%d", r.StatusCode, v)

+ }

+ if blob, v := get(); blob != "legacy" || v != 3 {

+ t.Fatalf("after legacy write: got %q/v%d", blob, v)

+ }

// TestOpenGroups covers the community directory: creating an open group,

// discovering it, self-serve joining, and flipping visibility — including the

// open_key overwrite guard.

diff --git a/main.go b/main.go

index d751d74..da7e4df 100644

--- a/main.go

+++ b/main.go

@@ -1520,18 +1520,22 @@ func (s *Server) getNicknames(w http.ResponseWriter, r *http.Request) {

httpError(w, http.StatusForbidden, "not a member")

return

}

- blob, err := s.db.userNicknamesBlob(uid)

+ blob, version, err := s.db.userNicknamesBlob(uid)

if err != nil {

httpError(w, http.StatusInternalServerError, "db error")

return

}

- writeJSON(w, http.StatusOK, map[string]any{"nicknames_blob": blob})

+ writeJSON(w, http.StatusOK, map[string]any{"nicknames_blob": blob, "version": version})

}

func (s *Server) postNicknames(w http.ResponseWriter, r *http.Request) {

var body struct {

Token string `json:"token"`

NicknamesBlob string `json:"nicknames_blob"`

+ // The version the writer last read (compare-and-swap). A pointer so

+ // absent is distinguishable from 0: a pre-versioning client omits it

+ // and keeps its unconditional write.

+ Version *int64 `json:"version"`

}

if json.NewDecoder(r.Body).Decode(&body) != nil {

httpError(w, http.StatusBadRequest, "bad request")

@@ -1549,11 +1553,22 @@ func (s *Server) postNicknames(w http.ResponseWriter, r *http.Request) {

httpError(w, http.StatusBadRequest, "bad request")

return

}

- if err := s.db.setUserNicknamesBlob(uid, body.NicknamesBlob); err != nil {

+ base := int64(-1)

+ if body.Version != nil && *body.Version >= 0 {

+ base = *body.Version

+ }

+ version, wrote, err := s.db.setUserNicknamesBlob(uid, body.NicknamesBlob, base)

+ if err != nil {

httpError(w, http.StatusInternalServerError, "db error")

return

}

- writeJSON(w, http.StatusOK, map[string]any{"ok": true})

+ if !wrote {

+ // Another device wrote since the caller's read — 409 with where the

+ // row is now, so it can re-read, replay its queued ops, and retry.

+ writeJSON(w, http.StatusConflict, map[string]any{"error": "version conflict", "version": version})

+ return

+ }

+ writeJSON(w, http.StatusOK, map[string]any{"ok": true, "version": version})

}

// postGroupDescription sets the space's description (admin only — the space's

diff --git a/web/static/shell.js b/web/static/shell.js

index bc37f44..7dd1ca1 100644

--- a/web/static/shell.js

+++ b/web/static/shell.js

@@ -1134,6 +1134,9 @@ let nicknames = {};

// a wipe of everything stored. Ops stay queued (and retry) until a save lands.

const nicknameOps = new Map();

let nickRetryTimer = 0;

+// The server blob's version at our last read — sent back with each save so a

+// stale write 409s (compare-and-swap) instead of racing another device.

+let nicknamesVersion = 0;

// The server's current map, or null when it can't be known right now (fetch

// failed, blob won't open, no key). null means "don't write" — never guess.

@@ -1143,6 +1146,7 @@ async function fetchServerNicknames() {

const r = await fetch(`/api/nicknames?token=${encodeURIComponent(session.token)}`);

if (!r.ok) return null;

const body = await r.json();

+ nicknamesVersion = body.version || 0;

if (!body.nicknames_blob) return {};

const obj = await openSealed(body.nicknames_blob, session.userPriv);

return (obj && obj.names) || {};

@@ -1173,6 +1177,7 @@ async function saveNicknamesNow() {

const included = [...nicknameOps.entries()];

const base = await fetchServerNicknames();

if (!base) return queueNickRetry();

+ const baseVersion = nicknamesVersion; // pin the version this merge is built on

const merged = { ...base };

for (const [uid, entry] of included) {

if (entry) merged[uid] = entry;

@@ -1183,9 +1188,12 @@ async function saveNicknamesNow() {

const r = await fetch("/api/nicknames", {

method: "POST",

headers: { "Content-Type": "application/json" },

- body: JSON.stringify({ token: session.token, nicknames_blob: blob }),

+ body: JSON.stringify({ token: session.token, nicknames_blob: blob, version: baseVersion }),

});

+ // A 409 means another device wrote between our read and this write — the

+ // queued ops retry over a fresh read (read-modify-write converges).

if (!r.ok) return queueNickRetry();

+ nicknamesVersion = ((await r.json()) || {}).version || baseVersion + 1;

nicknames = merged; // adopt anything another device stored meanwhile too

// Clear only what this save wrote — an op re-set mid-save stays queued.

for (const [uid, entry] of included) if (nicknameOps.get(uid) === entry) nicknameOps.delete(uid);