send queue: list only pending messages; tap a message to see all of it

docs/sending-and-step-up.md

@@ -131,8 +131,16 @@ dispatched agents and the chief panel have no Queue.
   status without its time.
 - 🔴 **A usage limit holds the queue.** See
   [A usage limit holds the queue](#a-usage-limit-holds-the-queue) below.
-- **At most 20** messages can wait per window. Finished ones are shown for a
-  while and deleted after 7 days, or sooner with their **Delete** (below).
+- **At most 20** messages can wait per window. The list shows only what still
+  needs something: messages queued or being sent, and your own failed messages
+  that you have not retried or deleted. A sent or cancelled message leaves the
+  list at once. A failed message of your own stays until you **Retry** or
+  **Delete** it, or until it is deleted 7 days after it failed (every finished
+  message is); only the 20 newest are listed. A failed message your chief
+  routed, or a chief nudge that failed, is not listed: its outcome goes back to
+  the chief, which handles it.
+  A tap on a message in the list shows its whole text (the list shows the first
+  two lines).
 - **Never twice.** A message is claimed (`queued` → `sending`) before it is sent
   and never put back once anything may have reached the window. (The one way
   back to `queued` is the Send-wins case above, where nothing was pasted.) The
@@ -284,14 +292,15 @@ must be fresh like a queue post's. A message that may already have been sent
 asks you to confirm first. A message your chief routed has no Retry: its failed
 route goes back to the chief instead.
 
-A finished message (sent, failed, expired or cancelled) has a **Delete**, which
-removes it from the list on every device at once (`POST /send-queue/{id}/delete`).
-A message still queued has no Delete (Cancel it first), and neither has one
-being sent: the send in progress still holds it. Deleting a sent message does
-not let the next one in early: the next message still waits for the turn the
-deleted one started. Deleting a failed message your chief routed is fine: its
-route is not affected, and if the route times out, the message still goes back
-to the chief.
+A failed message (expired included) has a **Delete**, which removes it from the
+list on every device at once (`POST /send-queue/{id}/delete`). The route deletes
+any finished message (sent, failed or cancelled), though the list draws only
+failed ones. A message still queued has no Delete (Cancel it first), and
+neither has one being sent: the send in progress still holds it. Deleting a
+sent message does not let the next one in early: the next message still waits
+for the turn the deleted one started. A failed message your chief routed is not
+listed, but deleting one through the route is fine: its route is not affected,
+and if the route times out, the message still goes back to the chief.
 
 🔴 **A queued message does not outlive the account that queued it.** When a
 message is sent, the server checks the account again, from the database: if it

e2e/tests/attention.spec.ts

@@ -378,7 +378,7 @@ test('a queued message answers the ask, and its Cancel says the answer stays', a
   await row.getByRole('button', { name: 'More actions for this queued message' }).click();
   await row.getByRole('menuitem', { name: 'Cancel this queued message' }).click();
   await expect(page.locator('#mv-note')).toHaveText(/^Cancelled — but this message had already answered .+'s question, and that answer was delivered: it can't be un-sent\.$/);
-  await expect(row).toHaveAttribute('data-queue-state', 'cancelled');
+  await expect(row, 'cancelled: it leaves the list').toHaveCount(0);
 });
 
 // ANSWERED ON THE ASKING SESSION'S OWN VIEW (attention_asker.go): the

internal/api/send_queue.go

@@ -390,11 +390,11 @@ func (s *Server) sendQueueListView(ctx context.Context, t sendqueue.Target, edit
 			// A cut through a character leaves invalid bytes; they are dropped.
 			preview = strings.ToValidUTF8(preview[:sendQueuePreview], "") + "…"
 		}
-		body := ""
-		if it.State == sendqueue.Queued {
-			body = it.Body // Copy message copies all of it, not the preview.
-		}
-		row := ui.SendQueueItem{ID: it.ID, State: string(it.State), Preview: preview, Body: body,
+		// The whole text rides in every row: a tap on the preview shows it, and
+		// a queued row's Copy message copies it (the preview is cut). Every row
+		// listed is the caller's own (row-level security), so this is text the
+		// reader already wrote or was routed.
+		row := ui.SendQueueItem{ID: it.ID, State: string(it.State), Preview: preview, Body: it.Body,
 			Detail: it.Detail, Cancellable: it.State == sendqueue.Queued, Answered: answered[it.ID],
 			Retryable: sendQueueRetryable(it), Deletable: it.State.Finished(), MayHaveDelivered: it.MayHaveDelivered,
 			Editable: it.State == sendqueue.Queued && it.RouteID == "" && it.ChiefNudgeID == 0, Held: it.State == sendqueue.Queued && it.Held,

internal/api/send_queue_delete_pg_test.go

@@ -18,8 +18,8 @@ import (
 
 // THE SEND QUEUE'S DELETE (POST /send-queue/{id}/delete), through the real
 // mux and a real Postgres (row-level security included): a FINISHED message —
-// sent, failed (expired included), cancelled — is deleted and leaves the
-// list; a queued one is refused (Cancel it), and so is one being sent (its
+// sent, failed (expired included), cancelled — is deleted (the list draws a
+// Delete only on a failed one: it does not draw the others at all); a queued one is refused (Cancel it), and so is one being sent (its
 // claim still holds the row); another user's is 404 and untouched.
 //
 // RED AT origin/main (0405925): the route is not registered, so every Delete
@@ -151,30 +151,46 @@ func TestAnotherOwnersDeleteIsNotFound(t *testing.T) {
 	}
 }
 
-// TestTheListDrawsDeleteOnFinishedRowsOnly: through GET /send-queue, the
-// finished rows (failed, cancelled, sent) each draw a Delete posting to their
-// own route, and the queued row draws none.
-func TestTheListDrawsDeleteOnFinishedRowsOnly(t *testing.T) {
+// TestTheListDrawsDeleteOnAnUnhandledFailureOnly: through GET /send-queue, a
+// failed row draws a Delete posting to its own route; a sent and a cancelled
+// row are not drawn at all (they are done — the list used to keep them, and
+// each had to be deleted by hand), nor a failed row already retried (its retry
+// row is drawn in its place); and the queued rows draw no Delete.
+func TestTheListDrawsDeleteOnAnUnhandledFailureOnly(t *testing.T) {
 	f := newSQFixture(t)
 	f.mirror.agentState = &mirror.AgentState{Status: mirror.StatusBusy, Since: time.Now().UnixMilli()}
 	failed := f.expired(t, "a failed one")
+	retried := f.expired(t, "a failed one retried")
+	again, err := f.queue.Retry(asQueueUser(1, time.Now()), retried.ID, time.Now(), 0)
+	if err != nil {
+		t.Fatal(err)
+	}
 	cancelled := f.enqueue(t, "a cancelled one", time.Hour, 0)
 	if _, err := f.queue.Cancel(asQueueUser(1, time.Now()), cancelled.ID, time.Now()); err != nil {
 		t.Fatal(err)
 	}
 	sent := f.sent(t, "a sent one")
 	queued := f.enqueue(t, "a queued one", time.Hour, 0)
 	_, page := mirrorGet(t, f.srv, f.ts, ui.SendQueuePath+"?host=ws&session=main&window=%401")
-	for _, id := range []int64{failed.ID, cancelled.ID, sent.ID} {
-		if !strings.Contains(page, `action="`+ui.SendQueueDeletePath(id)+`"`) {
-			t.Fatalf("finished row %d draws no Delete: %s", id, page)
+	if !strings.Contains(page, `action="`+ui.SendQueueDeletePath(failed.ID)+`"`) {
+		t.Fatalf("the failed row draws no Delete: %s", page)
+	}
+	for _, id := range []int64{retried.ID, cancelled.ID, sent.ID} {
+		if strings.Contains(page, `data-queue-id="`+strconv.FormatInt(id, 10)+`"`) {
+			t.Fatalf("handled row %d is drawn: %s", id, page)
 		}
 	}
-	if strings.Contains(page, ui.SendQueueDeletePath(queued.ID)) {
-		t.Fatalf("the queued row draws a Delete: %s", page)
+	// CONTROL: the open rows are drawn — the retry's and the queued one.
+	for _, id := range []int64{again.ID, queued.ID} {
+		if !strings.Contains(page, `data-queue-id="`+strconv.FormatInt(id, 10)+`"`) {
+			t.Fatalf("open row %d is not drawn: %s", id, page)
+		}
+		if strings.Contains(page, ui.SendQueueDeletePath(id)) {
+			t.Fatalf("open row %d draws a Delete: %s", id, page)
+		}
 	}
-	if got := strings.Count(page, `aria-label="`+ui.SendQueueDeleteLabel+`"`); got != 3 {
-		t.Fatalf("%d Delete buttons; want 3 (one per finished row): %s", got, page)
+	if got := strings.Count(page, `aria-label="`+ui.SendQueueDeleteLabel+`"`); got != 1 {
+		t.Fatalf("%d Delete buttons; want 1 (the unhandled failure's): %s", got, page)
 	}
 }
 

internal/api/send_queue_list_test.go

@@ -79,11 +79,45 @@ func TestAFailedRowDrawsRetryOnce(t *testing.T) {
 	}
 }
 
-// TestDeleteIsDrawnOnExactlyTheFinishedStates: through the real list route, a
-// row in each state the schema allows (0019's send_queue_state_check) draws a
-// Delete exactly when it is finished — sent, failed, cancelled — and never
-// when it is queued (that is Cancel) or being sent.
-func TestDeleteIsDrawnOnExactlyTheFinishedStates(t *testing.T) {
+// TestEveryListedRowCarriesItsWholeText: through the real list route, the
+// server cuts each preview at sendQueuePreview bytes, and a queued, a sending
+// and a failed row each carry the WHOLE text in the popover a tap on the
+// preview opens (ui.SendQueueFullID) — the tail past the cut included.
+func TestEveryListedRowCarriesItsWholeText(t *testing.T) {
+	q := &memQueue{}
+	s, ts := queueServer(t, queueMirror(), q, &turnGateway{})
+	states := []sendqueue.State{sendqueue.Queued, sendqueue.Sending, sendqueue.Failed}
+	for _, st := range states {
+		body := strings.Repeat(string(st)+" filler ", 40) + "tail of the " + string(st) + " one"
+		it, err := q.Enqueue(asQueueUser(1, time.Now()), sendqueue.Item{Target: queueTarget, Body: body, StepUpAt: time.Now()})
+		if err != nil {
+			t.Fatal(err)
+		}
+		q.mu.Lock()
+		q.items[it.ID-1].State = st
+		q.mu.Unlock()
+	}
+	_, page := mirrorGet(t, s, ts, ui.SendQueuePath+"?"+url.Values{"host": {"ws"}, "session": {"main"}, "window": {"@1"}}.Encode())
+	for i, st := range states {
+		id := ui.SendQueueFullID(int64(i + 1))
+		at := strings.Index(page, `id="`+id+`"`)
+		if at < 0 {
+			t.Fatalf("the %s row has no #%s: %s", st, id, page)
+		}
+		end := strings.Index(page[at:], "</li>")
+		if end < 0 || !strings.Contains(page[at:at+end], "tail of the "+string(st)+" one") {
+			t.Fatalf("the %s row's popover lacks the tail: %s", st, page[at:])
+		}
+	}
+}
+
+// TestDeleteIsDrawnOnAFailedRowOnly:through the real list route, with a row
+// in each state the schema allows (0019's send_queue_state_check), the failed
+// row draws a Delete, and the queued (that is Cancel) and sending rows do not.
+// The sent and cancelled rows are not drawn at all: the store does not list
+// them (memQueue as PGStore.List: internal/sendqueue's
+// TestListShowsOnlyWhatStillNeedsSomething pins the real one).
+func TestDeleteIsDrawnOnAFailedRowOnly(t *testing.T) {
 	q := &memQueue{}
 	s, ts := queueServer(t, queueMirror(), q, &turnGateway{})
 	states := []sendqueue.State{sendqueue.Queued, sendqueue.Sending, sendqueue.Sent, sendqueue.Failed, sendqueue.Cancelled}
@@ -97,12 +131,16 @@ func TestDeleteIsDrawnOnExactlyTheFinishedStates(t *testing.T) {
 		q.mu.Unlock()
 	}
 	_, page := mirrorGet(t, s, ts, ui.SendQueuePath+"?"+url.Values{"host": {"ws"}, "session": {"main"}, "window": {"@1"}}.Encode())
-	want := map[sendqueue.State]bool{sendqueue.Sent: true, sendqueue.Failed: true, sendqueue.Cancelled: true}
+	want := map[sendqueue.State]bool{sendqueue.Failed: true}
 	for i, st := range states {
 		drawn := strings.Contains(page, `action="`+ui.SendQueueDeletePath(int64(i+1))+`"`)
 		if drawn != want[st] {
 			t.Errorf("a %s row draws Delete: %v; want %v", st, drawn, want[st])
 		}
+		listed := strings.Contains(page, "row "+string(st))
+		if wantListed := st.Open() || st == sendqueue.Failed; listed != wantListed {
+			t.Errorf("a %s row is drawn: %v; want %v", st, listed, wantListed)
+		}
 	}
 	if !strings.Contains(page, "row sending") {
 		t.Fatalf("CONTROL: the list drew no rows: %s", page)

internal/api/send_queue_now_pg_test.go

@@ -234,14 +234,16 @@ func TestSendNowIsRefusedWhereASendIsAndWhereTheDispatchersDeliveryIs(t *testing
 
 // TestTheListDrawsSendNowAndCopyOnQueuedRowsOnly: a queued row's ⋯ menu holds
 // Send now (a real form posting to its route), Copy message (carrying the
-// whole message, not the preview) and Cancel; a finished row has no menu and
+// whole message, not the preview) and Cancel; a failed row has no menu and
 // keeps its own buttons.
 func TestTheListDrawsSendNowAndCopyOnQueuedRowsOnly(t *testing.T) {
 	f := newSQFixture(t)
 	f.mirror.agentState = &mirror.AgentState{Status: mirror.StatusBusy, Since: time.Now().UnixMilli()}
+	// First: expired() runs an idle dispatcher pass, which would send a row
+	// queued before it.
+	failed := f.expired(t, "already failed")
 	long := strings.Repeat("a long queued message ", 20) + "THE END"
 	q := f.enqueue(t, long, time.Minute, 0)
-	sent := f.sent(t, "already sent")
 	_, page := mirrorGet(t, f.srv, f.ts, ui.SendQueuePath+"?"+url.Values{"host": {"ws"}, "session": {"main"}, "window": {"@1"}}.Encode())
 	rowOf := func(id int64) string {
 		i := strings.Index(page, `data-queue-id="`+strconv.FormatInt(id, 10)+`"`)
@@ -251,7 +253,7 @@ func TestTheListDrawsSendNowAndCopyOnQueuedRowsOnly(t *testing.T) {
 		j := strings.Index(page[i:], "</li>")
 		return page[i : i+j]
 	}
-	qr, sr := rowOf(q.ID), rowOf(sent.ID)
+	qr, sr := rowOf(q.ID), rowOf(failed.ID)
 	for _, want := range []string{`aria-haspopup="menu"`, `action="` + sendNowPath(q.ID) + `"`, `action="` + ui.SendQueueCancelPath(q.ID) + `"`,
 		`data-queue-copy`, "THE END"} {
 		if !strings.Contains(qr, want) {
@@ -260,10 +262,10 @@ func TestTheListDrawsSendNowAndCopyOnQueuedRowsOnly(t *testing.T) {
 	}
 	for _, not := range []string{`aria-haspopup="menu"`, "send-now", "data-queue-copy"} {
 		if strings.Contains(sr, not) {
-			t.Errorf("the sent row has %q: %s", not, sr)
+			t.Errorf("the failed row has %q: %s", not, sr)
 		}
 	}
-	if !strings.Contains(sr, `action="`+ui.SendQueueDeletePath(sent.ID)+`"`) {
-		t.Errorf("CONTROL: the sent row lost its Delete: %s", sr)
+	if !strings.Contains(sr, `action="`+ui.SendQueueDeletePath(failed.ID)+`"`) {
+		t.Errorf("CONTROL: the failed row lost its Delete: %s", sr)
 	}
 }

internal/api/send_queue_test.go

@@ -63,6 +63,12 @@ func (m *memQueue) List(ctx context.Context, t sendqueue.Target) ([]sendqueue.It
 	defer m.mu.Unlock()
 	var out []sendqueue.Item
 	for _, it := range m.items {
+		// What PGStore.List lists: the open rows and the person's own
+		// unretried failures — no routed message, no chief nudge (order aside:
+		// these are in queue order).
+		if !it.State.Open() && (it.State != sendqueue.Failed || it.RetriedAs != 0 || it.RouteID != "" || it.ChiefNudgeID != 0) {
+			continue
+		}
 		if it.Owner == queueUser(ctx) && it.Target == t {
 			out = append(out, it)
 		}

internal/api/session_routes_pg_test.go

@@ -158,10 +158,34 @@ func routeArgs(m plugins.Message, sub string) string {
 // queued is the owner's send-queue rows for the routed window.
 func (f *routeFixture) queued(t *testing.T) []sendqueue.Item {
 	t.Helper()
-	items, err := f.queue.List(f.asOwner, sendqueue.Target{Host: "tower", Session: "work", Window: "@4"})
+	// Every row the owner has for the window, oldest first, read as the owner
+	// (row-level security applies) — NOT List, which leaves out what is done
+	// (sent, cancelled, retried), which is what these tests assert on.
+	rows, err := f.pool.Query(f.asOwner, `SELECT id FROM send_queue
+		WHERE mirror_host = 'tower' AND mirror_session = 'work' AND mirror_window = '@4' ORDER BY id`)
 	if err != nil {
 		t.Fatal(err)
 	}
+	var ids []int64
+	for rows.Next() {
+		var id int64
+		if err := rows.Scan(&id); err != nil {
+			t.Fatal(err)
+		}
+		ids = append(ids, id)
+	}
+	rows.Close()
+	if err := rows.Err(); err != nil {
+		t.Fatal(err)
+	}
+	var items []sendqueue.Item
+	for _, id := range ids {
+		it, err := f.queue.Get(f.asOwner, id)
+		if err != nil {
+			t.Fatal(err)
+		}
+		items = append(items, it)
+	}
 	return items
 }
 

internal/sendqueue/store.go

@@ -199,9 +199,16 @@ const MaxOpenPerTarget = 20
 // MaxDetail bounds an outcome's detail (0019's send_queue_detail_cap).
 const MaxDetail = 1024
 
-// HistoryShown is how many finished messages List returns beside the open
-// ones: the recent outcomes a composer shows under its queue.
-const HistoryShown = 5
+// FailedShown bounds the failed messages List returns beside the open ones:
+// the FailedShown most recent of a person's own failures not yet retried,
+// newest first. A sent or cancelled message is never listed — it has been
+// handled — nor a routed message or a chief nudge that failed (its outcome
+// goes back to the chief, which is its handling). A person's own failure is
+// listed until they Retry or Delete it, or Prune deletes it Retention after it
+// failed, and only the newest FailedShown of them; an older one beyond that is
+// not shown. The bound is the open bound's size (TestFailedShownCoversAWholeQueue),
+// so a whole queue that failed is shown whole.
+const FailedShown = MaxOpenPerTarget
 
 var (
 	// ErrFull refuses a message past MaxOpenPerTarget.
@@ -403,17 +410,22 @@ func (s *PGStore) Retry(ctx context.Context, id int64, stepUpAt time.Time, after
 	return out, nil
 }
 
-// List is the caller's messages for t: every open one, oldest first, then
-// the HistoryShown most recently finished, newest first.
+// List is the caller's messages for t that still need something: every open
+// one (queued or being sent), oldest first, then the FailedShown most recently
+// failed of the person's own (no route, no chief nudge) that were not
+// retried, newest first. A sent or cancelled message is not listed, nor a
+// failed one already retried (its retry row, a new message of its own, carries
+// it), nor a failed routed message or chief nudge (handed back to the chief).
 func (s *PGStore) List(ctx context.Context, t Target) ([]Item, error) {
 	if err := t.Valid(); err != nil {
 		return nil, err
 	}
 	where, args := targetWhere(t, 1)
 	rows, err := s.pool.Query(ctx, `(SELECT `+cols+`, 0 AS part FROM send_queue WHERE state IN ('queued', 'sending') AND `+where+`)
 		UNION ALL
-		(SELECT `+cols+`, 1 AS part FROM send_queue WHERE state NOT IN ('queued', 'sending') AND `+where+`
-			ORDER BY finished_at DESC, id DESC LIMIT `+fmt.Sprint(HistoryShown)+`)
+		(SELECT `+cols+`, 1 AS part FROM send_queue WHERE state = 'failed' AND retried_as IS NULL
+			AND route_id IS NULL AND chief_nudge_id IS NULL AND `+where+`
+			ORDER BY finished_at DESC, id DESC LIMIT `+fmt.Sprint(FailedShown)+`)
 		ORDER BY part, id`, args...)
 	if err != nil {
 		return nil, fmt.Errorf("sendqueue: list: %w", err)
@@ -872,8 +884,8 @@ func (s *PGStore) Reject(ctx context.Context, id int64, detail string) (bool, er
 	return tag.RowsAffected() == 1, nil
 }
 
-// Retention is how long a finished message is kept (its outcome is shown
-// under the composer) before Prune deletes it.
+// Retention is how long a finished message is kept (a person's own failure is
+// listed under the composer, List) before Prune deletes it.
 const Retention = 7 * 24 * time.Hour
 
 // Prune deletes every message that finished before before.