From a3e22ce3011136621cfbfeb6c799c6b021609c22 Mon Sep 17 00:00:00 2001 From: Ellis Green Date: Tue, 15 Sep 2026 09:27:32 +0100 Subject: [PATCH 1/2] Refetch notes on stage change so obfuscation clears Moving a retro from reflect to group left other people's notes stuck showing reflect-stage obfuscated noise. Obfuscation is recomputed correctly per request from the retro's current status, but nothing re-fetched notes when that status changed - a per-stage remount used to do this as a side effect until "Keep the notes when the stage changes" (dabd046) removed it, and no replacement was added for the already-open board. Wire status_updated to trigger the existing reconnect refetch instead. Also fixes two more instances of the same underlying pattern found while tracing this: - dal.NoteList had no ORDER BY, so a full notes fetch (now happening on every stage change too) had no stable order. - The note_created reducer case always appended the server-confirmed note to the end instead of swapping it in at the optimistic placeholder's own position, which could reorder notes as people created them concurrently. - VotesWithCountFromModel built its result by ranging over a map, so vote-count order in discuss was non-deterministic - the classic Go map-iteration bug. Adds regression coverage at every level: dal/resources/controllers tests against real SQLite, a Vitest reducer case, and a Playwright E2E suite driving two real browser sessions through the actual reflect to group transition (verified this catches the regression: reverting the fix makes it fail showing literal obfuscated noise instead of the typed note). Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 37 +++++++ .gitignore | 3 + CLAUDE.md | 3 +- cmd/thoughts/controllers/notes_test.go | 87 ++++++++++++++++ cmd/thoughts/dal/note.go | 3 +- cmd/thoughts/dal/note_test.go | 107 ++++++++++++++++++++ cmd/thoughts/resources/vote.go | 11 +- cmd/thoughts/resources/vote_test.go | 49 +++++++++ ui/e2e/reflect-to-group-obfuscation.spec.ts | 84 +++++++++++++++ ui/package.json | 2 + ui/playwright.config.ts | 34 +++++++ ui/pnpm-lock.yaml | 28 +++++ ui/src/components/retro/note.tsx | 5 +- ui/src/hooks/use-notes.test.ts | 19 ++++ ui/src/hooks/use-notes.ts | 30 ++++-- ui/tsconfig.e2e.json | 22 ++++ ui/tsconfig.json | 3 +- 17 files changed, 516 insertions(+), 11 deletions(-) create mode 100644 cmd/thoughts/controllers/notes_test.go create mode 100644 cmd/thoughts/dal/note_test.go create mode 100644 cmd/thoughts/resources/vote_test.go create mode 100644 ui/e2e/reflect-to-group-obfuscation.spec.ts create mode 100644 ui/playwright.config.ts create mode 100644 ui/tsconfig.e2e.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 12bdcfc..4391285 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,3 +66,40 @@ jobs: - name: Build run: pnpm build + + e2e: + name: E2E + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: true + + - uses: arduino/setup-task@v2 + with: + version: 3.x + + - uses: pnpm/action-setup@v4 + with: + package_json_file: ui/package.json + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: pnpm + cache-dependency-path: ui/pnpm-lock.yaml + + - name: Install UI dependencies + working-directory: ui + run: pnpm install --frozen-lockfile + + - name: Install Playwright's browser + working-directory: ui + run: pnpm exec playwright install --with-deps chromium + + - name: Run E2E tests + working-directory: ui + run: pnpm test:e2e diff --git a/.gitignore b/.gitignore index 4387f97..01f8fa1 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,6 @@ build/ dist/ .task/ seed*.py +ui/test-results/ +ui/playwright-report/ +ui/blob-report/ diff --git a/CLAUDE.md b/CLAUDE.md index 8c99578..97169c3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,8 @@ cd ui pnpm run dev # Vite on :5173, proxying /api/* to :3000 pnpm run build pnpm run lint -pnpm test +pnpm test # Vitest +pnpm test:e2e # Playwright, against task run's production binary ``` Before saying a change is done: diff --git a/cmd/thoughts/controllers/notes_test.go b/cmd/thoughts/controllers/notes_test.go new file mode 100644 index 0000000..51c8a5a --- /dev/null +++ b/cmd/thoughts/controllers/notes_test.go @@ -0,0 +1,87 @@ +package controllers_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/ellgreen/thoughts/cmd/thoughts/auth" + "github.com/ellgreen/thoughts/cmd/thoughts/controllers" + "github.com/ellgreen/thoughts/cmd/thoughts/dal" + "github.com/ellgreen/thoughts/cmd/thoughts/model" + "github.com/ellgreen/thoughts/cmd/thoughts/testutil" + "github.com/gorilla/mux" +) + +// The frontend now refetches this endpoint on every stage change, on the +// assumption that it always reflects the retro's *current* status rather +// than whatever status was in effect when a note was created. +func TestRetroNotesIndexReObfuscatesFromCurrentStatus(t *testing.T) { + ctx := context.Background() + db := testutil.NewDB(t) + + retro, err := dal.RetroInsert(ctx, db, "A test retro", model.RetroColumns{ + {Title: "Went well", Description: ""}, + }, false) + if err != nil { + t.Fatalf("failed to seed retro: %v", err) + } + + author, err := dal.UserInsert(ctx, db, "Author") + if err != nil { + t.Fatalf("failed to seed author: %v", err) + } + + viewer, err := dal.UserInsert(ctx, db, "Viewer") + if err != nil { + t.Fatalf("failed to seed viewer: %v", err) + } + + column := retro.GetColumns()[0] + + if _, err := dal.NoteInsert(ctx, db, retro.ID, author.ID, column.ID, "a secret thought"); err != nil { + t.Fatalf("failed to insert note: %v", err) + } + + fetchAsViewer := func() string { + t.Helper() + + req := httptest.NewRequest(http.MethodGet, "/api/retros/"+retro.ID.String()+"/notes", nil) + req = mux.SetURLVars(req, map[string]string{"id": retro.ID.String()}) + req = auth.RequestWithUser(req, viewer) + + rec := httptest.NewRecorder() + controllers.RetroNotesIndex(db).ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + + var notes []struct { + Content string `json:"content"` + } + if err := json.Unmarshal(rec.Body.Bytes(), ¬es); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + + if len(notes) != 1 { + t.Fatalf("expected 1 note, got %d", len(notes)) + } + + return notes[0].Content + } + + if got := fetchAsViewer(); got == "a secret thought" { + t.Errorf("expected another viewer's note to be obfuscated during brainstorm, got the real content") + } + + if err := dal.RetroUpdateStatus(ctx, db, retro.ID, model.RetroStatusGroup); err != nil { + t.Fatalf("failed to advance the retro: %v", err) + } + + if got := fetchAsViewer(); got != "a secret thought" { + t.Errorf("expected the note in the clear once brainstorm has ended, got %q", got) + } +} diff --git a/cmd/thoughts/dal/note.go b/cmd/thoughts/dal/note.go index 6845187..7a72335 100644 --- a/cmd/thoughts/dal/note.go +++ b/cmd/thoughts/dal/note.go @@ -142,7 +142,8 @@ func NoteList( retroID uuid.UUID, ) ([]*model.Note, error) { notes := make([]*model.Note, 0) - if err := db.SelectContext(ctx, ¬es, "select * from notes where retro_id = ?", retroID); err != nil { + if err := db.SelectContext(ctx, ¬es, + "select * from notes where retro_id = ? order by created_at asc, id asc", retroID); err != nil { return nil, fmt.Errorf("%w: failed to select notes: %w", ErrExecution, err) } diff --git a/cmd/thoughts/dal/note_test.go b/cmd/thoughts/dal/note_test.go new file mode 100644 index 0000000..7681771 --- /dev/null +++ b/cmd/thoughts/dal/note_test.go @@ -0,0 +1,107 @@ +package dal_test + +import ( + "context" + "testing" + "time" + + "github.com/ellgreen/thoughts/cmd/thoughts/dal" + "github.com/ellgreen/thoughts/cmd/thoughts/model" + "github.com/ellgreen/thoughts/cmd/thoughts/testutil" + "github.com/google/uuid" +) + +func TestNoteListReturnsNotesInCreationOrder(t *testing.T) { + ctx := context.Background() + db := testutil.NewDB(t) + + retro, err := dal.RetroInsert(ctx, db, "A test retro", model.RetroColumns{ + {Title: "Went well", Description: ""}, + }, false) + if err != nil { + t.Fatalf("failed to seed retro: %v", err) + } + + user, err := dal.UserInsert(ctx, db, "Author") + if err != nil { + t.Fatalf("failed to seed user: %v", err) + } + + column := retro.GetColumns()[0] + + var created []uuid.UUID + for _, content := range []string{"first", "second", "third"} { + note, err := dal.NoteInsert(ctx, db, retro.ID, user.ID, column.ID, content) + if err != nil { + t.Fatalf("failed to insert note %q: %v", content, err) + } + + created = append(created, note.ID) + + // NoteList orders by created_at; force distinct timestamps so the + // test doesn't depend on the platform's clock resolution. + time.Sleep(time.Millisecond) + } + + notes, err := dal.NoteList(ctx, db, retro.ID) + if err != nil { + t.Fatalf("failed to list notes: %v", err) + } + + if len(notes) != len(created) { + t.Fatalf("expected %d notes, got %d", len(created), len(notes)) + } + + for i, note := range notes { + if note.ID != created[i] { + t.Errorf("position %d: expected note %s, got %s", i, created[i], note.ID) + } + } +} + +func TestNoteListOrderSurvivesAnUpdate(t *testing.T) { + ctx := context.Background() + db := testutil.NewDB(t) + + retro, err := dal.RetroInsert(ctx, db, "A test retro", model.RetroColumns{ + {Title: "Went well", Description: ""}, + }, false) + if err != nil { + t.Fatalf("failed to seed retro: %v", err) + } + + user, err := dal.UserInsert(ctx, db, "Author") + if err != nil { + t.Fatalf("failed to seed user: %v", err) + } + + column := retro.GetColumns()[0] + + var created []uuid.UUID + for _, content := range []string{"a", "b", "c"} { + note, err := dal.NoteInsert(ctx, db, retro.ID, user.ID, column.ID, content) + if err != nil { + t.Fatalf("failed to insert note %q: %v", content, err) + } + + created = append(created, note.ID) + time.Sleep(time.Millisecond) + } + + // Update the first note, as grouping or moving it during the retro + // would - this is what can shift a row's storage position in SQLite. + if _, err := dal.NoteUpdate(ctx, db, created[0], uuid.Nil, uuid.New(), "a, regrouped", "", false); err != nil { + t.Fatalf("failed to update note: %v", err) + } + + notes, err := dal.NoteList(ctx, db, retro.ID) + if err != nil { + t.Fatalf("failed to list notes: %v", err) + } + + for i, note := range notes { + if note.ID != created[i] { + t.Errorf("position %d: expected note %s, got %s - order changed after an update", i, created[i], note.ID) + } + } +} diff --git a/cmd/thoughts/resources/vote.go b/cmd/thoughts/resources/vote.go index 747feec..37828c7 100644 --- a/cmd/thoughts/resources/vote.go +++ b/cmd/thoughts/resources/vote.go @@ -1,6 +1,9 @@ package resources import ( + "slices" + "strings" + "github.com/ellgreen/thoughts/cmd/thoughts/model" "github.com/google/uuid" "github.com/samber/lo" @@ -30,7 +33,7 @@ func VotesWithCountFromModel(votes []*model.Vote) []*VoteWithCount { groupVoteCounts[vote.GroupID]++ } - votesWithCount := make([]*VoteWithCount, 0) + votesWithCount := make([]*VoteWithCount, 0, len(groupVoteCounts)) for groupID, count := range groupVoteCounts { votesWithCount = append(votesWithCount, &VoteWithCount{ @@ -39,5 +42,11 @@ func VotesWithCountFromModel(votes []*model.Vote) []*VoteWithCount { }) } + // Map iteration order is randomised per run; sort so the response is + // stable instead of shuffling the group order on every request. + slices.SortFunc(votesWithCount, func(a, b *VoteWithCount) int { + return strings.Compare(a.GroupID.String(), b.GroupID.String()) + }) + return votesWithCount } diff --git a/cmd/thoughts/resources/vote_test.go b/cmd/thoughts/resources/vote_test.go new file mode 100644 index 0000000..5819b6a --- /dev/null +++ b/cmd/thoughts/resources/vote_test.go @@ -0,0 +1,49 @@ +package resources_test + +import ( + "testing" + + "github.com/ellgreen/thoughts/cmd/thoughts/model" + "github.com/ellgreen/thoughts/cmd/thoughts/resources" + "github.com/google/uuid" +) + +// Ranging over a map randomises Go's own iteration order, so this asserts +// the same input always produces the same output order - not just that the +// counts are right. +func TestVotesWithCountFromModelIsOrderedDeterministically(t *testing.T) { + groupA := uuid.New() + groupB := uuid.New() + groupC := uuid.New() + + votes := []*model.Vote{ + {GroupID: groupA}, {GroupID: groupB}, {GroupID: groupA}, + {GroupID: groupC}, {GroupID: groupB}, {GroupID: groupB}, + } + + first := resources.VotesWithCountFromModel(votes) + + for i := 0; i < 50; i++ { + got := resources.VotesWithCountFromModel(votes) + + if len(got) != len(first) { + t.Fatalf("run %d: expected %d groups, got %d", i, len(first), len(got)) + } + + for j := range first { + if got[j].GroupID != first[j].GroupID || got[j].Count != first[j].Count { + t.Fatalf("run %d: order changed at position %d: expected %+v, got %+v", + i, j, first[j], got[j]) + } + } + } + + counts := map[uuid.UUID]int{} + for _, v := range first { + counts[v.GroupID] = v.Count + } + + if counts[groupA] != 2 || counts[groupB] != 3 || counts[groupC] != 1 { + t.Errorf("unexpected counts: %+v", counts) + } +} diff --git a/ui/e2e/reflect-to-group-obfuscation.spec.ts b/ui/e2e/reflect-to-group-obfuscation.spec.ts new file mode 100644 index 0000000..9cc6164 --- /dev/null +++ b/ui/e2e/reflect-to-group-obfuscation.spec.ts @@ -0,0 +1,84 @@ +import { + APIRequestContext, + BrowserContext, + expect, + test, +} from "@playwright/test"; + +async function loginAs(context: BrowserContext, name: string) { + const res = await context.request.post("/api/auth/login", { + data: { name }, + }); + + expect(res.ok()).toBeTruthy(); +} + +async function createRetro(request: APIRequestContext, title: string) { + const res = await request.post("/api/retros", { + data: { + title, + columns: [ + { title: "Went well", description: "" }, + { title: "Went badly", description: "" }, + ], + unlisted: true, + tags: [], + }, + }); + + expect(res.ok()).toBeTruthy(); + + const retro = await res.json(); + return retro.id as string; +} + +test("a note stops reading as scrambled noise once the retro leaves reflect, without a reload", async ({ + browser, +}) => { + const author = await browser.newContext(); + const viewer = await browser.newContext(); + + await loginAs(author, `Author ${Date.now()}`); + await loginAs(viewer, `Viewer ${Date.now()}`); + + const retroId = await createRetro( + author.request, + `Obfuscation regression ${Date.now()}`, + ); + + const authorPage = await author.newPage(); + const viewerPage = await viewer.newPage(); + + await authorPage.goto(`/retros/${retroId}`); + await viewerPage.goto(`/retros/${retroId}`); + + const content = "a distinctive thought only the author typed"; + + await authorPage.getByRole("button", { name: "Add a thought" }).first().click(); + await authorPage.getByRole("textbox", { name: "Note" }).fill(content); + await authorPage.getByRole("button", { name: "Save" }).click(); + + // The optimistic placeholder and the server-confirmed note briefly coexist + // mid-animation (the confirmed note swaps in under a new id/key), so wait + // for that to settle before asserting on a single element. + const authorNote = authorPage.getByTestId("note-content"); + await expect(authorNote).toHaveCount(1); + + // The author's own note always renders in the clear on their own screen. + await expect(authorNote).toHaveText(content); + + const viewerNote = viewerPage.getByTestId("note-content"); + await expect(viewerNote).toHaveCount(1); + + // Someone else's note is obfuscated server-side during reflect, so the + // viewer's copy of it should not match what was actually typed. + await expect(viewerNote).toBeVisible(); + await expect(viewerNote).not.toHaveText(content); + + await authorPage.getByRole("button", { name: "Start grouping" }).click(); + await authorPage.getByRole("button", { name: "Move on" }).click(); + + // The viewer's board is already open and never reloads. Without the fix, + // it keeps showing the reflect-stage obfuscated text forever. + await expect(viewerNote).toHaveText(content, { timeout: 10_000 }); +}); diff --git a/ui/package.json b/ui/package.json index 44277f0..4941c15 100644 --- a/ui/package.json +++ b/ui/package.json @@ -9,6 +9,7 @@ "build": "tsc -b && vite build", "lint": "eslint .", "test": "vitest run", + "test:e2e": "playwright test", "preview": "vite preview" }, "dependencies": { @@ -47,6 +48,7 @@ }, "devDependencies": { "@eslint/js": "^9.39.4", + "@playwright/test": "^1.63.0", "@tailwindcss/postcss": "^4.2.2", "@tanstack/router-plugin": "^1.167.0", "@types/node": "^22.19.15", diff --git a/ui/playwright.config.ts b/ui/playwright.config.ts new file mode 100644 index 0000000..65e053f --- /dev/null +++ b/ui/playwright.config.ts @@ -0,0 +1,34 @@ +import { defineConfig } from "@playwright/test"; + +// Runs against the real production binary (task build && task run) rather +// than the Vite dev server, so the WebSocket/session/obfuscation behaviour +// under test is exactly what ships. +// +// Override THOUGHTS_E2E_PORT locally if 3000 is already taken by a dev +// server - task run picks it up as THOUGHTS_ADDRESS. +const port = process.env.THOUGHTS_E2E_PORT ?? "3000"; +const baseURL = `http://localhost:${port}`; + +export default defineConfig({ + testDir: "./e2e", + fullyParallel: true, + forbidOnly: !!process.env.CI, + retries: process.env.CI ? 1 : 0, + reporter: "line", + use: { + baseURL, + trace: "retain-on-failure", + }, + webServer: { + command: "task run", + cwd: "..", + url: baseURL, + reuseExistingServer: !process.env.CI, + timeout: 120_000, + env: { + // Keeps e2e runs off a real dev database. + THOUGHTS_DATA_PATH: "./tmp/e2e-data", + THOUGHTS_ADDRESS: `localhost:${port}`, + }, + }, +}); diff --git a/ui/pnpm-lock.yaml b/ui/pnpm-lock.yaml index f16fb01..5130c2f 100644 --- a/ui/pnpm-lock.yaml +++ b/ui/pnpm-lock.yaml @@ -108,6 +108,9 @@ importers: '@eslint/js': specifier: ^9.39.4 version: 9.39.4 + '@playwright/test': + specifier: ^1.63.0 + version: 1.63.0 '@tailwindcss/postcss': specifier: ^4.2.2 version: 4.2.2 @@ -823,6 +826,11 @@ packages: '@open-draft/until@2.1.0': resolution: {integrity: sha512-U69T3ItWHvLwGg5eJ0n3I62nWuE6ilHlmz7zM0npLBRvPRd7e6NYmg54vvRtP5mZG7kZqZCFVdsTWo7BPtBujg==} + '@playwright/test@1.63.0': + resolution: {integrity: sha512-oxMK4vllB9RK5NQ2l1pq1IfOf2AvnEuj/vYGDj0H2nMtmtZpKtCwt/l00GEO6xjGfpBNAvjovvYdCm50dRQkpQ==} + engines: {node: '>=20'} + hasBin: true + '@radix-ui/number@1.1.1': resolution: {integrity: sha512-MkKCwxlXTgz6CFoJx3pCwn07GKp36+aZyu/u2Ln2VrA5DcdyCZkASEDBTd8x5whTQQL5CiYf4prXKLcgQdv29g==} @@ -3280,6 +3288,16 @@ packages: resolution: {integrity: sha512-wQ0b/W4Fr01qtpHlqSqspcj3EhBvimsdh0KlHhH8HRZnMsEa0ea2fTULOXOS9ccQr3om+GcGRk4e+isrZWV8qQ==} engines: {node: '>=16.20.0'} + playwright-core@1.63.0: + resolution: {integrity: sha512-rYCsBF/M5HjUch52bbtVONEFjv6Xu8sm8h72dNlR5bzIE1fvC/bxgspzkjSfU+MweEMmPM8KJebG6nnyxo5mCg==} + engines: {node: '>=20'} + hasBin: true + + playwright@1.63.0: + resolution: {integrity: sha512-+7ziBLidS4NaNCdt57SUDT+wYmmd5fmiQejUic/kb+YsYSCPyOOE9sebzMjNmQrsnNpDJqd4WHvV/8lfKfUDUg==} + engines: {node: '>=20'} + hasBin: true + postcss-selector-parser@7.1.1: resolution: {integrity: sha512-orRsuYpJVw8LdAwqqLykBj9ecS5/cRHlI5+nvTo8LcCKmzDmqVORXtOIYEEQuL9D4BxtA1lm5isAqzQZCoQ6Eg==} engines: {node: '>=4'} @@ -4505,6 +4523,10 @@ snapshots: '@open-draft/until@2.1.0': {} + '@playwright/test@1.63.0': + dependencies: + playwright: 1.63.0 + '@radix-ui/number@1.1.1': {} '@radix-ui/primitive@1.1.3': {} @@ -6904,6 +6926,12 @@ snapshots: pkce-challenge@5.0.1: {} + playwright-core@1.63.0: {} + + playwright@1.63.0: + dependencies: + playwright-core: 1.63.0 + postcss-selector-parser@7.1.1: dependencies: cssesc: 3.0.0 diff --git a/ui/src/components/retro/note.tsx b/ui/src/components/retro/note.tsx index d32523c..fd8b337 100644 --- a/ui/src/components/retro/note.tsx +++ b/ui/src/components/retro/note.tsx @@ -157,7 +157,10 @@ function NoteBody({ size={16} /> )} -

+

{note.content}

diff --git a/ui/src/hooks/use-notes.test.ts b/ui/src/hooks/use-notes.test.ts index dcdb2a1..68a63a5 100644 --- a/ui/src/hooks/use-notes.test.ts +++ b/ui/src/hooks/use-notes.test.ts @@ -84,6 +84,25 @@ describe("notesReducer", () => { expect(state.rollbacks).toEqual({}); }); + it("keeps a confirmed note in its own slot even when another note is confirmed first", () => { + const state = replay( + { name: "note_create", payload: { column_id: "column-1", content: "mine", ref: "ref-1" } }, + { name: "note_created", payload: note({ id: "theirs", created_by_me: false }) }, + { name: "note_created", payload: { ...note({ id: "server-1" }), ref: "ref-1" } }, + ); + + expect(state.notes.map((n) => n.id)).toEqual(["server-1", "theirs"]); + }); + + it("still appends a confirmed note with no matching placeholder", () => { + const state = replay({ + name: "note_created", + payload: { ...note({ id: "server-1" }), ref: "ref-1" }, + }); + + expect(state.notes.map((n) => n.id)).toEqual(["server-1"]); + }); + it("does not leak the ref onto the stored note", () => { const state = replay({ name: "note_created", diff --git a/ui/src/hooks/use-notes.ts b/ui/src/hooks/use-notes.ts index 50ee987..693f1b5 100644 --- a/ui/src/hooks/use-notes.ts +++ b/ui/src/hooks/use-notes.ts @@ -127,14 +127,22 @@ function notesReducer(state: NotesState, event: SocketEvent): NotesState { case "note_created": { const payload = event.payload as Note & Partial; + const confirmed = toNote(payload); - const notes = payload.ref - ? state.notes.filter((note) => note.id !== payload.ref) - : state.notes; + // Swapped in at the placeholder's own index rather than filtered out + // and appended, so it doesn't jump past a note that got confirmed + // while this one was in flight. Falls back to upsert when there is no + // placeholder to swap (e.g. the ref already resolved). + const hasPlaceholder = + !!payload.ref && state.notes.some((note) => note.id === payload.ref); + + const notes = hasPlaceholder + ? state.notes.map((note) => (note.id === payload.ref ? confirmed : note)) + : upsert(state.notes, confirmed); return { ...state, - notes: upsert(notes, toNote(payload)), + notes, rollbacks: forget(state.rollbacks, payload.ref), }; } @@ -286,14 +294,24 @@ export function useNotesState(loaded: Note[]): NotesValue { [send, user?.name], ); - useSocketEvent(dispatch); - const load = useCallback(() => { api.get(`/api/retros/${retro.id}/notes`).then((res) => { dispatch({ name: "note_index", payload: res.data }); }); }, [retro.id]); + useSocketEvent((event: SocketEvent) => { + dispatch(event); + + // Obfuscation is decided per-request from the retro's current status, and + // nothing re-sends a note's content when that status changes - without + // this, another person's notes stay stuck showing whatever was obfuscated + // (or not) as of the last fetch, straight through a stage change. + if (event.name === "status_updated") { + load(); + } + }); + // Nothing replays what the socket missed while it was down, and this hook no // longer remounts per stage to refetch by accident, so a reconnect has to ask // the server for the list again. The route loader covers the first connection. diff --git a/ui/tsconfig.e2e.json b/ui/tsconfig.e2e.json new file mode 100644 index 0000000..e53d14c --- /dev/null +++ b/ui/tsconfig.e2e.json @@ -0,0 +1,22 @@ +{ + "compilerOptions": { + "tsBuildInfoFile": "./node_modules/.tmp/tsconfig.e2e.tsbuildinfo", + "target": "ES2022", + "lib": ["ES2023"], + "module": "ESNext", + "skipLibCheck": true, + + "moduleResolution": "Bundler", + "allowImportingTsExtensions": true, + "isolatedModules": true, + "moduleDetection": "force", + "noEmit": true, + + "strict": true, + "noUnusedLocals": true, + "noUnusedParameters": true, + "noFallthroughCasesInSwitch": true, + "noUncheckedSideEffectImports": true + }, + "include": ["playwright.config.ts", "e2e"] +} diff --git a/ui/tsconfig.json b/ui/tsconfig.json index fec8c8e..275a255 100644 --- a/ui/tsconfig.json +++ b/ui/tsconfig.json @@ -2,7 +2,8 @@ "files": [], "references": [ { "path": "./tsconfig.app.json" }, - { "path": "./tsconfig.node.json" } + { "path": "./tsconfig.node.json" }, + { "path": "./tsconfig.e2e.json" } ], "compilerOptions": { "baseUrl": ".", From aa1316f503a3cfba6519163c3d783a9b2791ea93 Mon Sep 17 00:00:00 2001 From: Ellis Green Date: Tue, 15 Sep 2026 09:38:32 +0100 Subject: [PATCH 2/2] Create the e2e data directory before starting the server task run's data path is never created by the app itself - task dev relies on the committed ./data, and the Dockerfile does its own mkdir. CI's isolated THOUGHTS_DATA_PATH had nothing creating it, so the server exited on startup before Playwright could reach it. Co-Authored-By: Claude Sonnet 5 --- ui/playwright.config.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/playwright.config.ts b/ui/playwright.config.ts index 65e053f..36e2f80 100644 --- a/ui/playwright.config.ts +++ b/ui/playwright.config.ts @@ -20,7 +20,10 @@ export default defineConfig({ trace: "retain-on-failure", }, webServer: { - command: "task run", + // The app never creates its own data directory (task dev/run rely on the + // committed ./data, and the Dockerfile does its own mkdir), so this has + // to exist before the binary starts. + command: "mkdir -p tmp/e2e-data && task run", cwd: "..", url: baseURL, reuseExistingServer: !process.env.CI,