amadan
web: reopen the existing DM instead of starting a second (#32)
The server already dedupes DMs (one per pair), but the client always went through createThread, which had two problems: it offered to "start a chat" with someone you already DM, and — worse — when the server returned the existing thread, createThread overwrote that thread's real key with the throwaway key it had just generated, corrupting decryption for the DM. Both DM entry points (New chat picker, View-members tap) now reopen the existing DM directly when one exists, and createThread no longer stores a key when the server responds existing:true. Add a handler test that locks in the dedupe contract the client depends on (repeat POST returns the same thread_id with existing:true).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 files changed,
+90
−3
handlers_test.go+62 −0web/static/chat.js+28 −3
diff --git a/handlers_test.go b/handlers_test.go| index 57950b2..ad92161 100644 |
| --- a/handlers_test.go |
| +++ b/handlers_test.go |
| @@ -482,6 +482,68 @@ func TestGroupNameEscapedInHTML(t *testing.T) { |
| } |
| } |
| +// Starting a DM you already have must reopen the same thread, not spawn a |
| +// second. The client relies on the server returning the existing thread with |
| +// existing:true on a repeat POST (see createThread / existingDM in chat.js). |
| +func TestPostThreadDedupesDM(t *testing.T) { |
| + ts, srv := newTestServer(t) |
| + |
| + mkUser := func(label string) (tok, uid string) { |
| + u, err := srv.db.createUser(label) |
| + if err != nil { |
| + t.Fatalf("createUser %s: %v", label, err) |
| + } |
| + if err := srv.db.createKeyInvite("inv-"+label, u, "wrap", 0); err != nil { |
| + t.Fatalf("createKeyInvite %s: %v", label, err) |
| + } |
| + tk, ok, err := srv.db.claimKeyInvite("inv-"+label, label) |
| + if err != nil || !ok { |
| + t.Fatalf("claimKeyInvite %s: ok=%v err=%v", label, ok, err) |
| + } |
| + if err := srv.db.setUserPublicKey(u, "pub-"+label); err != nil { |
| + t.Fatalf("setUserPublicKey %s: %v", label, err) |
| + } |
| + return tk, u |
| + } |
| + tokA, uidA := mkUser("Alice") |
| + _, uidB := mkUser("Bob") |
| + |
| + body := `{"token":"` + tokA + `","kind":"dm","members":[` + |
| + `{"user_id":"` + uidA + `","wrapped":"wa"},` + |
| + `{"user_id":"` + uidB + `","wrapped":"wb"}]}` |
| + |
| + post := func() (string, bool) { |
| + resp, err := http.Post(ts.URL+"/api/threads", "application/json", strings.NewReader(body)) |
| + if err != nil { |
| + t.Fatalf("post: %v", err) |
| + } |
| + defer resp.Body.Close() |
| + if resp.StatusCode != http.StatusOK { |
| + t.Fatalf("status %d", resp.StatusCode) |
| + } |
| + var out struct { |
| + ThreadID string `json:"thread_id"` |
| + Existing bool `json:"existing"` |
| + } |
| + if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { |
| + t.Fatalf("decode: %v", err) |
| + } |
| + return out.ThreadID, out.Existing |
| + } |
| + |
| + id1, existing1 := post() |
| + if id1 == "" || existing1 { |
| + t.Fatalf("first create: id=%q existing=%v (want a fresh id, existing=false)", id1, existing1) |
| + } |
| + id2, existing2 := post() |
| + if id2 != id1 || !existing2 { |
| + t.Fatalf("repeat create should dedupe: id=%q existing=%v (want %q, true)", id2, existing2, id1) |
| + } |
| + if got, ok, _ := srv.db.dmThreadFor(uidA, uidB); !ok || got != id1 { |
| + t.Fatalf("dmThreadFor: got %q ok=%v want %q", got, ok, id1) |
| + } |
| +} |
| + |
| // joinedMember sets up a real "joined" member — a user with a device token and a |
| // registered public key — plus a thread they belong to, for exercising the |
| // thread-scoped APIs. Returns the device token, the user id, and the thread id. |
diff --git a/web/static/chat.js b/web/static/chat.js| index 13eaa18..d8f5877 100644 |
| --- a/web/static/chat.js |
| +++ b/web/static/chat.js |
| @@ -2428,6 +2428,11 @@ function showMembers() { |
| if (!me) |
| row.onclick = async () => { |
| backdrop.remove(); |
| + const dm = existingDM(uid); |
| + if (dm) { |
| + openThread(dm.id); // already have a DM — reopen it, don't start a second |
| + return; |
| + } |
| try { |
| const id = await createThread([uid], { group: false }); |
| await loadThreads(); |
| @@ -2559,6 +2564,13 @@ async function leaveGroup() { |
| // ---- new chat (roster picker) ------------------------------------------- |
| +// The existing DM thread with a person, if any. There's exactly one DM per pair |
| +// (the server enforces it too), so starting a chat with someone you already DM |
| +// should just reopen it — never spawn a second. |
| +function existingDM(userId) { |
| + return threads.find((t) => t.kind === "dm" && (t.members || []).includes(userId)); |
| +} |
| + |
| function showNewChat() { |
| const others = roster.filter((e) => e.user_id !== myUserId); |
| const backdrop = modal(); |
| @@ -2658,7 +2670,15 @@ function showNewChat() { |
| const b = document.createElement("button"); |
| b.className = "list-row"; |
| b.textContent = e.label; |
| - b.onclick = () => go(createThread([e.user_id], { group: false }), b); |
| + b.onclick = () => { |
| + const dm = existingDM(e.user_id); |
| + if (dm) { |
| + backdrop.remove(); |
| + openThread(dm.id); // already chatting — just reopen it |
| + return; |
| + } |
| + go(createThread([e.user_id], { group: false }), b); |
| + }; |
| list.appendChild(b); |
| } |
| } |
| @@ -2706,8 +2726,13 @@ async function createThread(otherIds, opts = {}) { |
| }) |
| ).json(); |
| if (!r.thread_id) throw new Error("no thread id"); |
| - threadKeys[r.thread_id] = key; |
| - threadKeyB64[r.thread_id] = b64; |
| + // The server dedupes DMs: if one already exists it returns it with existing:true. |
| + // Keep that thread's real key — don't overwrite it with the throwaway key we |
| + // generated for the would-be new thread (which no member could decrypt). |
| + if (!r.existing) { |
| + threadKeys[r.thread_id] = key; |
| + threadKeyB64[r.thread_id] = b64; |
| + } |
| return r.thread_id; |
| } |