diff --git a/.superpowers/sdd/task-6-report.md b/.superpowers/sdd/task-6-report.md index 9b5e18bc..88b7af75 100644 --- a/.superpowers/sdd/task-6-report.md +++ b/.superpowers/sdd/task-6-report.md @@ -49,3 +49,17 @@ - E2E remains environment-blocked until the Playwright Chromium browser is installed. - Existing Vitest runs emit pre-existing MSW unmatched-request and dialog-ref warnings; all assertions pass and this task does not modify those shared test/UI primitives. + +## Review remediation + +- A post-commit review correctly identified that matching `displayName` must never establish + ownership. The predicate now skips confirmation only when `session.author` exactly equals + `principal.subject`; all display-name matches and missing authors are conservative + cross-owner actions. +- Added RED/GREEN regressions where two principals share display name `Alice` but have distinct + subjects: both delete (with another session present, so select-all cannot mask the guard) and + archive require confirmation. +- Added `aria-pressed` to the My sessions / All sessions controls and asserts their selected state + before and after switching. +- Remediation verification: focused regressions passed; full frontend Vitest (44 files / 305 + tests), `npx tsc -b`, `npm run build`, and `git diff --check` all passed. diff --git a/frontend/src/shell/AppShell.session-mgmt.test.tsx b/frontend/src/shell/AppShell.session-mgmt.test.tsx index 00140cc3..d6130b12 100644 --- a/frontend/src/shell/AppShell.session-mgmt.test.tsx +++ b/frontend/src/shell/AppShell.session-mgmt.test.tsx @@ -82,7 +82,7 @@ test("administrators can explicitly switch to all sessions and see owners", asyn let scope = ""; server.use( http.get("http://localhost:8787/me", () => - HttpResponse.json({ issuer: "portal", subject: "alice", displayName: "Alice", isAdmin: true }), + HttpResponse.json({ issuer: "portal", subject: "alice-id", displayName: "Alice", isAdmin: true }), ), http.get("http://localhost:8787/sessions", ({ request }) => { scope = new URL(request.url).searchParams.get("scope") ?? ""; @@ -94,20 +94,25 @@ test("administrators can explicitly switch to all sessions and see owners", asyn ); wrap(); await screen.findByRole("button", { name: "All sessions" }); + expect(screen.getByRole("button", { name: "My sessions" })).toHaveAttribute("aria-pressed", "true"); + expect(screen.getByRole("button", { name: "All sessions" })).toHaveAttribute("aria-pressed", "false"); await userEvent.click(screen.getByRole("button", { name: "All sessions" })); await waitFor(() => expect(scope).toBe("all")); + expect(screen.getByRole("button", { name: "My sessions" })).toHaveAttribute("aria-pressed", "false"); + expect(screen.getByRole("button", { name: "All sessions" })).toHaveAttribute("aria-pressed", "true"); expect(await screen.findByText("Administrator view: all sessions")).toBeInTheDocument(); expect(screen.getByText("Owner: Bob")).toBeInTheDocument(); }); -test("administrator confirms before deleting another owner's session", async () => { +test("administrator confirms before deleting a same-named user's session", async () => { let deletes = 0; server.use( http.get("http://localhost:8787/me", () => - HttpResponse.json({ issuer: "portal", subject: "alice", displayName: "Alice", isAdmin: true }), + HttpResponse.json({ issuer: "portal", subject: "alice-id", displayName: "Alice", isAdmin: true }), ), http.get("http://localhost:8787/sessions", () => HttpResponse.json([ - { ...LIST[0], author: "Bob" }, + { ...LIST[0], author: "Alice" }, + { ...LIST[1], id: "s3", question: "Second session", archived: false, author: "Bob" }, ])), http.delete("http://localhost:8787/sessions/:id", () => { deletes += 1; @@ -116,7 +121,7 @@ test("administrator confirms before deleting another owner's session", async () ); wrap(); await userEvent.click(await screen.findByRole("button", { name: "All sessions" })); - await screen.findByText("Owner: Bob"); + await screen.findByText("Owner: Alice"); await userEvent.click(screen.getByRole("checkbox", { name: "Select Attiva uno" })); await userEvent.click(screen.getByRole("button", { name: "Delete 1 selected sessions" })); expect(deletes).toBe(0); @@ -125,15 +130,15 @@ test("administrator confirms before deleting another owner's session", async () await waitFor(() => expect(deletes).toBe(1)); }); -test("administrator confirms before archiving another owner's session", async () => { +test("administrator confirms before archiving a same-named user's session", async () => { let archives = 0; const confirm = vi.spyOn(window, "confirm").mockReturnValue(false); server.use( http.get("http://localhost:8787/me", () => - HttpResponse.json({ issuer: "portal", subject: "alice", displayName: "Alice", isAdmin: true }), + HttpResponse.json({ issuer: "portal", subject: "alice-id", displayName: "Alice", isAdmin: true }), ), http.get("http://localhost:8787/sessions", () => HttpResponse.json([ - { ...LIST[0], author: "Bob" }, + { ...LIST[0], author: "Alice" }, ])), http.post("http://localhost:8787/sessions/:id/archive", () => { archives += 1; @@ -142,10 +147,10 @@ test("administrator confirms before archiving another owner's session", async () ); wrap(); await userEvent.click(await screen.findByRole("button", { name: "All sessions" })); - await screen.findByText("Owner: Bob"); + await screen.findByText("Owner: Alice"); await userEvent.click(screen.getByRole("button", { name: "Session actions" })); await userEvent.click(await screen.findByText("Archive")); - expect(confirm).toHaveBeenCalledWith("Archive Bob's session?"); + expect(confirm).toHaveBeenCalledWith("Archive Alice's session?"); expect(archives).toBe(0); confirm.mockRestore(); }); diff --git a/frontend/src/shell/AppShell.tsx b/frontend/src/shell/AppShell.tsx index 9caeceb7..784102d3 100644 --- a/frontend/src/shell/AppShell.tsx +++ b/frontend/src/shell/AppShell.tsx @@ -83,7 +83,7 @@ export function AppShell() { const isForeignSession = (session: SessionSummary) => { if (!showingAllSessions || !principal) return false; if (!session.author) return true; - return session.author !== principal.subject && session.author !== principal.displayName; + return session.author !== principal.subject; }; function selectActiveSession(id: string | null) { @@ -526,6 +526,7 @@ export function AppShell() {